* [PATCH 1/5] i2c: mv64xxx: put offload check into offload prepare function
2014-02-13 20:36 ` [PATCH 0/5] mv64xxx updates Wolfram Sang
@ 2014-02-13 20:36 ` Wolfram Sang
2014-02-14 11:38 ` Gregory CLEMENT
2014-02-13 20:36 ` [PATCH 2/5] i2c: mv64xxx: refactor message start to ensure proper initialization Wolfram Sang
` (5 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Wolfram Sang @ 2014-02-13 20:36 UTC (permalink / raw)
To: linux-arm-kernel
It makes code simpler to read and prepares a reorganization needed for a
bugfix.
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
drivers/i2c/busses/i2c-mv64xxx.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index b8c5187..ba64978 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -204,6 +204,9 @@ static int mv64xxx_i2c_offload_msg(struct mv64xxx_i2c_data *drv_data)
unsigned long ctrl_reg;
struct i2c_msg *msg = drv_data->msgs;
+ if (!drv_data->offload_enabled)
+ return -EOPNOTSUPP;
+
drv_data->msg = msg;
drv_data->byte_posn = 0;
drv_data->bytes_left = msg->len;
@@ -433,8 +436,7 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
drv_data->msgs++;
drv_data->num_msgs--;
- if (!(drv_data->offload_enabled &&
- mv64xxx_i2c_offload_msg(drv_data))) {
+ if (mv64xxx_i2c_offload_msg(drv_data) < 0) {
drv_data->cntl_bits |= MV64XXX_I2C_REG_CONTROL_START;
writel(drv_data->cntl_bits,
drv_data->reg_base + drv_data->reg_offsets.control);
--
1.8.5.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 1/5] i2c: mv64xxx: put offload check into offload prepare function
2014-02-13 20:36 ` [PATCH 1/5] i2c: mv64xxx: put offload check into offload prepare function Wolfram Sang
@ 2014-02-14 11:38 ` Gregory CLEMENT
0 siblings, 0 replies; 22+ messages in thread
From: Gregory CLEMENT @ 2014-02-14 11:38 UTC (permalink / raw)
To: linux-arm-kernel
On 13/02/2014 21:36, Wolfram Sang wrote:
> It makes code simpler to read and prepares a reorganization needed for a
> bugfix.
>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
As expected it didn't fix the issue I had, but it didn't introduce
any visible regression too.
> ---
> drivers/i2c/busses/i2c-mv64xxx.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
> index b8c5187..ba64978 100644
> --- a/drivers/i2c/busses/i2c-mv64xxx.c
> +++ b/drivers/i2c/busses/i2c-mv64xxx.c
> @@ -204,6 +204,9 @@ static int mv64xxx_i2c_offload_msg(struct mv64xxx_i2c_data *drv_data)
> unsigned long ctrl_reg;
> struct i2c_msg *msg = drv_data->msgs;
>
> + if (!drv_data->offload_enabled)
> + return -EOPNOTSUPP;
> +
> drv_data->msg = msg;
> drv_data->byte_posn = 0;
> drv_data->bytes_left = msg->len;
> @@ -433,8 +436,7 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
>
> drv_data->msgs++;
> drv_data->num_msgs--;
> - if (!(drv_data->offload_enabled &&
> - mv64xxx_i2c_offload_msg(drv_data))) {
> + if (mv64xxx_i2c_offload_msg(drv_data) < 0) {
> drv_data->cntl_bits |= MV64XXX_I2C_REG_CONTROL_START;
> writel(drv_data->cntl_bits,
> drv_data->reg_base + drv_data->reg_offsets.control);
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/5] i2c: mv64xxx: refactor message start to ensure proper initialization
2014-02-13 20:36 ` [PATCH 0/5] mv64xxx updates Wolfram Sang
2014-02-13 20:36 ` [PATCH 1/5] i2c: mv64xxx: put offload check into offload prepare function Wolfram Sang
@ 2014-02-13 20:36 ` Wolfram Sang
2014-02-14 11:39 ` Gregory CLEMENT
2014-02-13 20:36 ` [PATCH 3/5] i2c: mv64xxx: refactor send_start Wolfram Sang
` (4 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Wolfram Sang @ 2014-02-13 20:36 UTC (permalink / raw)
To: linux-arm-kernel
Because the offload mechanism can fall back to a standard transfer,
having two seperate initialization states is unfortunate. Let's just
have one state which does things consistently. This fixes a bug where
some preparation was missing when the fallback happened. And it makes
the code much easier to follow.
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
drivers/i2c/busses/i2c-mv64xxx.c | 27 ++++++++++-----------------
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index ba64978..d52d849 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -97,7 +97,6 @@ enum {
enum {
MV64XXX_I2C_ACTION_INVALID,
MV64XXX_I2C_ACTION_CONTINUE,
- MV64XXX_I2C_ACTION_OFFLOAD_SEND_START,
MV64XXX_I2C_ACTION_SEND_START,
MV64XXX_I2C_ACTION_SEND_RESTART,
MV64XXX_I2C_ACTION_OFFLOAD_RESTART,
@@ -460,15 +459,14 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
drv_data->reg_base + drv_data->reg_offsets.control);
break;
- case MV64XXX_I2C_ACTION_OFFLOAD_SEND_START:
- if (!mv64xxx_i2c_offload_msg(drv_data))
- break;
- else
- drv_data->action = MV64XXX_I2C_ACTION_SEND_START;
- /* FALLTHRU */
case MV64XXX_I2C_ACTION_SEND_START:
- writel(drv_data->cntl_bits | MV64XXX_I2C_REG_CONTROL_START,
- drv_data->reg_base + drv_data->reg_offsets.control);
+ /* Can we offload this msg ? */
+ if (mv64xxx_i2c_offload_msg(drv_data) < 0) {
+ /* No, switch to standard path */
+ mv64xxx_i2c_prepare_for_io(drv_data, drv_data->msgs);
+ writel(drv_data->cntl_bits | MV64XXX_I2C_REG_CONTROL_START,
+ drv_data->reg_base + drv_data->reg_offsets.control);
+ }
break;
case MV64XXX_I2C_ACTION_SEND_ADDR_1:
@@ -627,15 +625,10 @@ mv64xxx_i2c_execute_msg(struct mv64xxx_i2c_data *drv_data, struct i2c_msg *msg,
unsigned long flags;
spin_lock_irqsave(&drv_data->lock, flags);
- if (drv_data->offload_enabled) {
- drv_data->action = MV64XXX_I2C_ACTION_OFFLOAD_SEND_START;
- drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_START_COND;
- } else {
- mv64xxx_i2c_prepare_for_io(drv_data, msg);
- drv_data->action = MV64XXX_I2C_ACTION_SEND_START;
- drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_START_COND;
- }
+ drv_data->action = MV64XXX_I2C_ACTION_SEND_START;
+ drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_START_COND;
+
drv_data->send_stop = is_last;
drv_data->block = 1;
mv64xxx_i2c_do_action(drv_data);
--
1.8.5.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 2/5] i2c: mv64xxx: refactor message start to ensure proper initialization
2014-02-13 20:36 ` [PATCH 2/5] i2c: mv64xxx: refactor message start to ensure proper initialization Wolfram Sang
@ 2014-02-14 11:39 ` Gregory CLEMENT
2014-02-15 14:44 ` Wolfram Sang
0 siblings, 1 reply; 22+ messages in thread
From: Gregory CLEMENT @ 2014-02-14 11:39 UTC (permalink / raw)
To: linux-arm-kernel
On 13/02/2014 21:36, Wolfram Sang wrote:
> Because the offload mechanism can fall back to a standard transfer,
> having two seperate initialization states is unfortunate. Let's just
> have one state which does things consistently. This fixes a bug where
> some preparation was missing when the fallback happened. And it makes
> the code much easier to follow.
>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
With this one the bug we observed was fixed.
Thanks,
Gregory
> ---
> drivers/i2c/busses/i2c-mv64xxx.c | 27 ++++++++++-----------------
> 1 file changed, 10 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
> index ba64978..d52d849 100644
> --- a/drivers/i2c/busses/i2c-mv64xxx.c
> +++ b/drivers/i2c/busses/i2c-mv64xxx.c
> @@ -97,7 +97,6 @@ enum {
> enum {
> MV64XXX_I2C_ACTION_INVALID,
> MV64XXX_I2C_ACTION_CONTINUE,
> - MV64XXX_I2C_ACTION_OFFLOAD_SEND_START,
> MV64XXX_I2C_ACTION_SEND_START,
> MV64XXX_I2C_ACTION_SEND_RESTART,
> MV64XXX_I2C_ACTION_OFFLOAD_RESTART,
> @@ -460,15 +459,14 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
> drv_data->reg_base + drv_data->reg_offsets.control);
> break;
>
> - case MV64XXX_I2C_ACTION_OFFLOAD_SEND_START:
> - if (!mv64xxx_i2c_offload_msg(drv_data))
> - break;
> - else
> - drv_data->action = MV64XXX_I2C_ACTION_SEND_START;
> - /* FALLTHRU */
> case MV64XXX_I2C_ACTION_SEND_START:
> - writel(drv_data->cntl_bits | MV64XXX_I2C_REG_CONTROL_START,
> - drv_data->reg_base + drv_data->reg_offsets.control);
> + /* Can we offload this msg ? */
> + if (mv64xxx_i2c_offload_msg(drv_data) < 0) {
> + /* No, switch to standard path */
> + mv64xxx_i2c_prepare_for_io(drv_data, drv_data->msgs);
> + writel(drv_data->cntl_bits | MV64XXX_I2C_REG_CONTROL_START,
> + drv_data->reg_base + drv_data->reg_offsets.control);
> + }
> break;
>
> case MV64XXX_I2C_ACTION_SEND_ADDR_1:
> @@ -627,15 +625,10 @@ mv64xxx_i2c_execute_msg(struct mv64xxx_i2c_data *drv_data, struct i2c_msg *msg,
> unsigned long flags;
>
> spin_lock_irqsave(&drv_data->lock, flags);
> - if (drv_data->offload_enabled) {
> - drv_data->action = MV64XXX_I2C_ACTION_OFFLOAD_SEND_START;
> - drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_START_COND;
> - } else {
> - mv64xxx_i2c_prepare_for_io(drv_data, msg);
>
> - drv_data->action = MV64XXX_I2C_ACTION_SEND_START;
> - drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_START_COND;
> - }
> + drv_data->action = MV64XXX_I2C_ACTION_SEND_START;
> + drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_START_COND;
> +
> drv_data->send_stop = is_last;
> drv_data->block = 1;
> mv64xxx_i2c_do_action(drv_data);
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 2/5] i2c: mv64xxx: refactor message start to ensure proper initialization
2014-02-14 11:39 ` Gregory CLEMENT
@ 2014-02-15 14:44 ` Wolfram Sang
0 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2014-02-15 14:44 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Feb 14, 2014 at 12:39:02PM +0100, Gregory CLEMENT wrote:
> On 13/02/2014 21:36, Wolfram Sang wrote:
> > Because the offload mechanism can fall back to a standard transfer,
> > having two seperate initialization states is unfortunate. Let's just
> > have one state which does things consistently. This fixes a bug where
> > some preparation was missing when the fallback happened. And it makes
> > the code much easier to follow.
> >
> > Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
>
>
> Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>
> With this one the bug we observed was fixed.
I squashed patches 1+2 (easier handling for stable) and applied to
for-current, thanks!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140215/5266b037/attachment.sig>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/5] i2c: mv64xxx: refactor send_start
2014-02-13 20:36 ` [PATCH 0/5] mv64xxx updates Wolfram Sang
2014-02-13 20:36 ` [PATCH 1/5] i2c: mv64xxx: put offload check into offload prepare function Wolfram Sang
2014-02-13 20:36 ` [PATCH 2/5] i2c: mv64xxx: refactor message start to ensure proper initialization Wolfram Sang
@ 2014-02-13 20:36 ` Wolfram Sang
2014-02-14 11:42 ` Gregory CLEMENT
2014-02-13 20:36 ` [PATCH 4/5] i2c: mv64xxx: directly call send_start when initializing transfer Wolfram Sang
` (3 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Wolfram Sang @ 2014-02-13 20:36 UTC (permalink / raw)
To: linux-arm-kernel
For start and restart, we are doing the same thing. Let's consolidate
that.
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
drivers/i2c/busses/i2c-mv64xxx.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index d52d849..9c37b59 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -419,6 +419,17 @@ mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status)
}
}
+static void mv64xxx_i2c_send_start(struct mv64xxx_i2c_data *drv_data)
+{
+ /* Can we offload this msg ? */
+ if (mv64xxx_i2c_offload_msg(drv_data) < 0) {
+ /* No, switch to standard path */
+ mv64xxx_i2c_prepare_for_io(drv_data, drv_data->msgs);
+ writel(drv_data->cntl_bits | MV64XXX_I2C_REG_CONTROL_START,
+ drv_data->reg_base + drv_data->reg_offsets.control);
+ }
+}
+
static void
mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
{
@@ -435,14 +446,11 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
drv_data->msgs++;
drv_data->num_msgs--;
- if (mv64xxx_i2c_offload_msg(drv_data) < 0) {
- drv_data->cntl_bits |= MV64XXX_I2C_REG_CONTROL_START;
- writel(drv_data->cntl_bits,
- drv_data->reg_base + drv_data->reg_offsets.control);
+ // CHECKME: Does it work? Order of writel and prepare_for_io is
+ // exchanged. Also, do we need to change cntl_bits in drv_data
+ // with |= MV64XXX_I2C_REG_CONTROL_START?
+ mv64xxx_i2c_send_start(drv_data);
- /* Setup for the next message */
- mv64xxx_i2c_prepare_for_io(drv_data, drv_data->msgs);
- }
if (drv_data->errata_delay)
udelay(5);
@@ -460,13 +468,7 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
break;
case MV64XXX_I2C_ACTION_SEND_START:
- /* Can we offload this msg ? */
- if (mv64xxx_i2c_offload_msg(drv_data) < 0) {
- /* No, switch to standard path */
- mv64xxx_i2c_prepare_for_io(drv_data, drv_data->msgs);
- writel(drv_data->cntl_bits | MV64XXX_I2C_REG_CONTROL_START,
- drv_data->reg_base + drv_data->reg_offsets.control);
- }
+ mv64xxx_i2c_send_start(drv_data);
break;
case MV64XXX_I2C_ACTION_SEND_ADDR_1:
--
1.8.5.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 3/5] i2c: mv64xxx: refactor send_start
2014-02-13 20:36 ` [PATCH 3/5] i2c: mv64xxx: refactor send_start Wolfram Sang
@ 2014-02-14 11:42 ` Gregory CLEMENT
0 siblings, 0 replies; 22+ messages in thread
From: Gregory CLEMENT @ 2014-02-14 11:42 UTC (permalink / raw)
To: linux-arm-kernel
On 13/02/2014 21:36, Wolfram Sang wrote:
> For start and restart, we are doing the same thing. Let's consolidate
> that.
>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
With my first tests it continue to work with this change,
but I want to have a closer look on it
Thanks,
Gregory
> ---
> drivers/i2c/busses/i2c-mv64xxx.c | 30 ++++++++++++++++--------------
> 1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
> index d52d849..9c37b59 100644
> --- a/drivers/i2c/busses/i2c-mv64xxx.c
> +++ b/drivers/i2c/busses/i2c-mv64xxx.c
> @@ -419,6 +419,17 @@ mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status)
> }
> }
>
> +static void mv64xxx_i2c_send_start(struct mv64xxx_i2c_data *drv_data)
> +{
> + /* Can we offload this msg ? */
> + if (mv64xxx_i2c_offload_msg(drv_data) < 0) {
> + /* No, switch to standard path */
> + mv64xxx_i2c_prepare_for_io(drv_data, drv_data->msgs);
> + writel(drv_data->cntl_bits | MV64XXX_I2C_REG_CONTROL_START,
> + drv_data->reg_base + drv_data->reg_offsets.control);
> + }
> +}
> +
> static void
> mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
> {
> @@ -435,14 +446,11 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
>
> drv_data->msgs++;
> drv_data->num_msgs--;
> - if (mv64xxx_i2c_offload_msg(drv_data) < 0) {
> - drv_data->cntl_bits |= MV64XXX_I2C_REG_CONTROL_START;
> - writel(drv_data->cntl_bits,
> - drv_data->reg_base + drv_data->reg_offsets.control);
> + // CHECKME: Does it work? Order of writel and prepare_for_io is
> + // exchanged. Also, do we need to change cntl_bits in drv_data
> + // with |= MV64XXX_I2C_REG_CONTROL_START?
> + mv64xxx_i2c_send_start(drv_data);
>
> - /* Setup for the next message */
> - mv64xxx_i2c_prepare_for_io(drv_data, drv_data->msgs);
> - }
> if (drv_data->errata_delay)
> udelay(5);
>
> @@ -460,13 +468,7 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
> break;
>
> case MV64XXX_I2C_ACTION_SEND_START:
> - /* Can we offload this msg ? */
> - if (mv64xxx_i2c_offload_msg(drv_data) < 0) {
> - /* No, switch to standard path */
> - mv64xxx_i2c_prepare_for_io(drv_data, drv_data->msgs);
> - writel(drv_data->cntl_bits | MV64XXX_I2C_REG_CONTROL_START,
> - drv_data->reg_base + drv_data->reg_offsets.control);
> - }
> + mv64xxx_i2c_send_start(drv_data);
> break;
>
> case MV64XXX_I2C_ACTION_SEND_ADDR_1:
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/5] i2c: mv64xxx: directly call send_start when initializing transfer
2014-02-13 20:36 ` [PATCH 0/5] mv64xxx updates Wolfram Sang
` (2 preceding siblings ...)
2014-02-13 20:36 ` [PATCH 3/5] i2c: mv64xxx: refactor send_start Wolfram Sang
@ 2014-02-13 20:36 ` Wolfram Sang
2014-02-13 20:36 ` [PATCH 5/5] i2c: mv64xxx: refactor initialization for new msgs Wolfram Sang
` (2 subsequent siblings)
6 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2014-02-13 20:36 UTC (permalink / raw)
To: linux-arm-kernel
Calling the state machine with a definite state which is only used in
this context is superfluous. Do it directly.
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
drivers/i2c/busses/i2c-mv64xxx.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 9c37b59..3af5266 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -97,7 +97,6 @@ enum {
enum {
MV64XXX_I2C_ACTION_INVALID,
MV64XXX_I2C_ACTION_CONTINUE,
- MV64XXX_I2C_ACTION_SEND_START,
MV64XXX_I2C_ACTION_SEND_RESTART,
MV64XXX_I2C_ACTION_OFFLOAD_RESTART,
MV64XXX_I2C_ACTION_SEND_ADDR_1,
@@ -467,10 +466,6 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
drv_data->reg_base + drv_data->reg_offsets.control);
break;
- case MV64XXX_I2C_ACTION_SEND_START:
- mv64xxx_i2c_send_start(drv_data);
- break;
-
case MV64XXX_I2C_ACTION_SEND_ADDR_1:
writel(drv_data->addr1,
drv_data->reg_base + drv_data->reg_offsets.data);
@@ -628,12 +623,11 @@ mv64xxx_i2c_execute_msg(struct mv64xxx_i2c_data *drv_data, struct i2c_msg *msg,
spin_lock_irqsave(&drv_data->lock, flags);
- drv_data->action = MV64XXX_I2C_ACTION_SEND_START;
drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_START_COND;
drv_data->send_stop = is_last;
drv_data->block = 1;
- mv64xxx_i2c_do_action(drv_data);
+ mv64xxx_i2c_send_start(drv_data);
spin_unlock_irqrestore(&drv_data->lock, flags);
mv64xxx_i2c_wait_for_completion(drv_data);
--
1.8.5.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 5/5] i2c: mv64xxx: refactor initialization for new msgs
2014-02-13 20:36 ` [PATCH 0/5] mv64xxx updates Wolfram Sang
` (3 preceding siblings ...)
2014-02-13 20:36 ` [PATCH 4/5] i2c: mv64xxx: directly call send_start when initializing transfer Wolfram Sang
@ 2014-02-13 20:36 ` Wolfram Sang
2014-02-13 20:41 ` [PATCH 0/5] mv64xxx updates Gregory CLEMENT
2014-02-13 20:51 ` Thomas Petazzoni
6 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2014-02-13 20:36 UTC (permalink / raw)
To: linux-arm-kernel
We now have a central place to put this code to.
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
drivers/i2c/busses/i2c-mv64xxx.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 3af5266..be4b0a1 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -175,11 +175,6 @@ mv64xxx_i2c_prepare_for_io(struct mv64xxx_i2c_data *drv_data,
{
u32 dir = 0;
- drv_data->msg = msg;
- drv_data->byte_posn = 0;
- drv_data->bytes_left = msg->len;
- drv_data->aborting = 0;
- drv_data->rc = 0;
drv_data->cntl_bits = MV64XXX_I2C_REG_CONTROL_ACK |
MV64XXX_I2C_REG_CONTROL_INTEN | MV64XXX_I2C_REG_CONTROL_TWSIEN;
@@ -205,11 +200,6 @@ static int mv64xxx_i2c_offload_msg(struct mv64xxx_i2c_data *drv_data)
if (!drv_data->offload_enabled)
return -EOPNOTSUPP;
- drv_data->msg = msg;
- drv_data->byte_posn = 0;
- drv_data->bytes_left = msg->len;
- drv_data->aborting = 0;
- drv_data->rc = 0;
/* Only regular transactions can be offloaded */
if ((msg->flags & ~(I2C_M_TEN | I2C_M_RD)) != 0)
return -EINVAL;
@@ -420,6 +410,12 @@ mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status)
static void mv64xxx_i2c_send_start(struct mv64xxx_i2c_data *drv_data)
{
+ drv_data->msg = drv_data->msgs;
+ drv_data->byte_posn = 0;
+ drv_data->bytes_left = drv_data->msg->len;
+ drv_data->aborting = 0;
+ drv_data->rc = 0;
+
/* Can we offload this msg ? */
if (mv64xxx_i2c_offload_msg(drv_data) < 0) {
/* No, switch to standard path */
--
1.8.5.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 0/5] mv64xxx updates
2014-02-13 20:36 ` [PATCH 0/5] mv64xxx updates Wolfram Sang
` (4 preceding siblings ...)
2014-02-13 20:36 ` [PATCH 5/5] i2c: mv64xxx: refactor initialization for new msgs Wolfram Sang
@ 2014-02-13 20:41 ` Gregory CLEMENT
2014-03-10 16:25 ` Wolfram Sang
2014-02-13 20:51 ` Thomas Petazzoni
6 siblings, 1 reply; 22+ messages in thread
From: Gregory CLEMENT @ 2014-02-13 20:41 UTC (permalink / raw)
To: linux-arm-kernel
Hi Wolfram,
On 13/02/2014 21:36, Wolfram Sang wrote:
> So, this is a series I came up with trying to fix the issue found by Kevin.
> Patches 1+2 are hopefully fixing the bug (in theory, I don't have the HW).
> Patches 3-5 are RFC, and if patch 3 actually works (see the CHECKME), then 4+5
> are further cleanup possibilities. And there is still more potential, I mainly
> wanted to give some inspiration and awareness that the driver could need some
> more love. Please test at least 1+2, comments to 3-5 very welcome.
>
> Sorry for the delay, I got distracted by an NMI.
>
Thanks for this series, indeed the code looks better.
I will test it tomorrow and let you know if it fixed the bug.
I will also take time to review the RFC patches.
Greogry
> Wolfram Sang (5):
> i2c: mv64xxx: put offload check into offload prepare function
> i2c: mv64xxx: refactor message start to ensure proper initialization
> i2c: mv64xxx: refactor send_start
> i2c: mv64xxx: directly call send_start when initializing transfer
> i2c: mv64xxx: refactor initialization for new msgs
>
> drivers/i2c/busses/i2c-mv64xxx.c | 67 ++++++++++++++++------------------------
> 1 file changed, 27 insertions(+), 40 deletions(-)
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 0/5] mv64xxx updates
2014-02-13 20:36 ` [PATCH 0/5] mv64xxx updates Wolfram Sang
` (5 preceding siblings ...)
2014-02-13 20:41 ` [PATCH 0/5] mv64xxx updates Gregory CLEMENT
@ 2014-02-13 20:51 ` Thomas Petazzoni
2014-02-18 11:29 ` Maxime Ripard
6 siblings, 1 reply; 22+ messages in thread
From: Thomas Petazzoni @ 2014-02-13 20:51 UTC (permalink / raw)
To: linux-arm-kernel
Dear Wolfram Sang,
On Thu, 13 Feb 2014 21:36:28 +0100, Wolfram Sang wrote:
> So, this is a series I came up with trying to fix the issue found by Kevin.
> Patches 1+2 are hopefully fixing the bug (in theory, I don't have the HW).
> Patches 3-5 are RFC, and if patch 3 actually works (see the CHECKME), then 4+5
> are further cleanup possibilities. And there is still more potential, I mainly
> wanted to give some inspiration and awareness that the driver could need some
> more love. Please test at least 1+2, comments to 3-5 very welcome.
>
> Sorry for the delay, I got distracted by an NMI.
I'm adding Maxime Ripard in Cc, since he is the maintainer of the
Allwinner platform, which also uses this I2C driver. So I guess he
would like to be notified of such changes in order to test that the
driver still works for him.
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 0/5] mv64xxx updates
2014-02-13 20:51 ` Thomas Petazzoni
@ 2014-02-18 11:29 ` Maxime Ripard
0 siblings, 0 replies; 22+ messages in thread
From: Maxime Ripard @ 2014-02-18 11:29 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Thu, Feb 13, 2014 at 09:51:00PM +0100, Thomas Petazzoni wrote:
> Dear Wolfram Sang,
>
> On Thu, 13 Feb 2014 21:36:28 +0100, Wolfram Sang wrote:
> > So, this is a series I came up with trying to fix the issue found by Kevin.
> > Patches 1+2 are hopefully fixing the bug (in theory, I don't have the HW).
> > Patches 3-5 are RFC, and if patch 3 actually works (see the CHECKME), then 4+5
> > are further cleanup possibilities. And there is still more potential, I mainly
> > wanted to give some inspiration and awareness that the driver could need some
> > more love. Please test at least 1+2, comments to 3-5 very welcome.
> >
> > Sorry for the delay, I got distracted by an NMI.
>
> I'm adding Maxime Ripard in Cc, since he is the maintainer of the
> Allwinner platform, which also uses this I2C driver. So I guess he
> would like to be notified of such changes in order to test that the
> driver still works for him.
I just gave a try to these patches, on my A31 board with the extra
patches sent previously.
Tested-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140218/3a3f8b4c/attachment.sig>
^ permalink raw reply [flat|nested] 22+ messages in thread