[U-Boot] [PATCH 0/1] mvtwsi: Fix breakage introduced by "Fix mvtwsi not working on sun6i and newer sunxi SoCs"

[U-Boot] [PATCH 0/1] mvtwsi: Fix breakage introduced by "Fix mvtwsi not working on sun6i and newer sunxi SoCs"

[Hans de Goede]

Hi Heiko,

Your fix for the checkpatch issues in one of my patches (sorry about those)
has broken mvtwsi support.

This patch fixes this, I'm currently preparing a sunxi pull-req, and I
plan to include this fix, so this is mostly FYI, you do not have to
take any action.

Regards,

Hans

[U-Boot] [PATCH] mvtwsi: Fix breakage introduced by "Fix mvtwsi not working on sun6i and newer sunxi SoCs"

[Hans de Goede]

"Fix mvtwsi not working on sun6i and newer sunxi SoCs" includes the following:

@@ -189,7 +200,8 @@ static int twsi_start(struct i2c_adapter *adap, int expected_status)
 	/* globally set TWSIEN in case it was not */
 	twsi_control_flags |= MVTWSI_CONTROL_TWSIEN;
 	/* assert START */
-	writel(twsi_control_flags | MVTWSI_CONTROL_START, &twsi->control);
+	twsi_control_flags |= MVTWSI_CONTROL_START | MVTWSI_CONTROL_CLEAR_IFLG;
+	writel(twsi_control_flags, &twsi->control);
 	/* wait for controller to process START */
 	return twsi_wait(adap, expected_status);
 }

The modification of twsi_control_flags done here was introduced while
merging to fix a line > 80 chars, but twsi_control_flags is a global variable
and should not be modified like this here, this commit fixes this, restoring
mvtwsi functionality.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/i2c/mvtwsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index 5f993b9..221ff4f 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -204,8 +204,8 @@ static int twsi_start(struct i2c_adapter *adap, int expected_status)
 	/* globally set TWSIEN in case it was not */
 	twsi_control_flags |= MVTWSI_CONTROL_TWSIEN;
 	/* assert START */
-	twsi_control_flags |= MVTWSI_CONTROL_START | MVTWSI_CONTROL_CLEAR_IFLG;
-	writel(twsi_control_flags, &twsi->control);
+	writel(twsi_control_flags | MVTWSI_CONTROL_START |
+				    MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control);
 	/* wait for controller to process START */
 	return twsi_wait(adap, expected_status);
 }
-- 
2.5.0

[U-Boot] [PATCH 0/1] mvtwsi: Fix breakage introduced by "Fix mvtwsi not working on sun6i and newer sunxi SoCs"

[Heiko Schocher]

Hello Hans,

Am 26.01.2016 um 18:05 schrieb Hans de Goede:
> Hi Heiko,
>
> Your fix for the checkpatch issues in one of my patches (sorry about those)
> has broken mvtwsi support.

Uh! Sorry!

> This patch fixes this, I'm currently preparing a sunxi pull-req, and I
> plan to include this fix, so this is mostly FYI, you do not have to
> take any action.

Thanks!

bye,
Heiko
-- 
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] mvtwsi: Fix breakage introduced by "Fix mvtwsi not working on sun6i and newer sunxi SoCs"

[Heiko Schocher]

Hello Hans,

Am 26.01.2016 um 18:05 schrieb Hans de Goede:
> "Fix mvtwsi not working on sun6i and newer sunxi SoCs" includes the following:
>
> @@ -189,7 +200,8 @@ static int twsi_start(struct i2c_adapter *adap, int expected_status)
>   	/* globally set TWSIEN in case it was not */
>   	twsi_control_flags |= MVTWSI_CONTROL_TWSIEN;
>   	/* assert START */
> -	writel(twsi_control_flags | MVTWSI_CONTROL_START, &twsi->control);
> +	twsi_control_flags |= MVTWSI_CONTROL_START | MVTWSI_CONTROL_CLEAR_IFLG;
> +	writel(twsi_control_flags, &twsi->control);
>   	/* wait for controller to process START */
>   	return twsi_wait(adap, expected_status);
>   }
>
> The modification of twsi_control_flags done here was introduced while
> merging to fix a line > 80 chars, but twsi_control_flags is a global variable
> and should not be modified like this here, this commit fixes this, restoring
> mvtwsi functionality.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   drivers/i2c/mvtwsi.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Thanks!

Acked-by: Heiko Schocher <hs@denx.de>

If we would have automated tests, such cases would hopefully not happen.

Maybe you find time to look into:

[1] [U-Boot] [PATCH] test, tools: introduce tbot README
http://lists.denx.de/pipermail/u-boot/2016-January/243248.html

With tbot we could setup a test setup, where we can run testcases on
boards wherever they are availiable for tests. In [1] search for
"automated Testsetup with buildbot and tbot doing cyclic tests"

With this setup I detected your checkpatch error, and thought its
easy to fix (never think its easy ... again sorry). If I/we would
have a sunxi SoC HW with i2c testcases my bad fix would have automatically
poped up ...

As tbot is written in python it would be easy to send an automated
EMail if tbot detects such problems with a patch ... so, I never
would have tried to fix and break it ... but I tend to say, sending
EMails should be done by humans ...

bye,
Heiko
>
> diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
> index 5f993b9..221ff4f 100644
> --- a/drivers/i2c/mvtwsi.c
> +++ b/drivers/i2c/mvtwsi.c
> @@ -204,8 +204,8 @@ static int twsi_start(struct i2c_adapter *adap, int expected_status)
>   	/* globally set TWSIEN in case it was not */
>   	twsi_control_flags |= MVTWSI_CONTROL_TWSIEN;
>   	/* assert START */
> -	twsi_control_flags |= MVTWSI_CONTROL_START | MVTWSI_CONTROL_CLEAR_IFLG;
> -	writel(twsi_control_flags, &twsi->control);
> +	writel(twsi_control_flags | MVTWSI_CONTROL_START |
> +				    MVTWSI_CONTROL_CLEAR_IFLG, &twsi->control);
>   	/* wait for controller to process START */
>   	return twsi_wait(adap, expected_status);
>   }
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany