Re: [U-Boot] [PATCH] power: twl6030: fix code refactoring

2016-10-17 Thread Tom Rini
On Mon, Oct 17, 2016 at 08:03:23PM +0300, Nicolae Rosia wrote:
> Hi,
> 
> On Sat, Oct 15, 2016 at 8:48 PM, Tom Rini  wrote:
> > I'll take this and do you want to do a follow up to make the file
> > checkpatch clean?  Thanks!
> Either way works for me, I can do the follow up to make the file
> checkpatch clean.

Follow up patch it is, thanks!

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] power: twl6030: fix code refactoring

2016-10-17 Thread Nicolae Rosia
Hi,

On Sat, Oct 15, 2016 at 8:48 PM, Tom Rini  wrote:
> I'll take this and do you want to do a follow up to make the file
> checkpatch clean?  Thanks!
Either way works for me, I can do the follow up to make the file
checkpatch clean.

Thanks,
Nicolae
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] power: twl6030: fix code refactoring

2016-10-15 Thread Paul Kocialkowski
Hi,

Le samedi 15 octobre 2016 à 20:41 +0300, Nicolae Rosia a écrit :
> Hi,
> 
> On Fri, Oct 14, 2016 at 4:28 PM, Paul Kocialkowski  wrote:
> > 
> > Le jeudi 13 octobre 2016 à 13:47 +0300, Nicolae Rosia a écrit :
> > > 
> > > From: Nicolae Rosia 
> > > 
> > > Commit a85362fb3e1fc7833723accddbbae431091d06b8 refactored the code
> > > but the register read ended up in the wrong if branch.
> > > Currently, the else branch checks a variable which is always 0.
> > 
> > Good catch! Sorry for including that regression in the first place, I should
> > have checked the patch more thoroughly.
> No worries!
> 
> > 
> > See comment below.
> > The indentation before &value is inconsistent with what is done in the rest
> > of
> > the file. I think you should either just move the line as it was or only use
> > one
> > extra tab indent for the new line. Since this file breaks the 80 chars limit
> > in
> > a few places, I would tend to prefer the former.
> > 
> checkpatch.pl was complaining, that's why I formatted it.

Then I think you should fix the rest of the file accordingly.

It doesn't make sense to introduce a variation in the coding style, as it
creates inconsistency. IMO this is worse than having only subsequent changes
conform to the global U-Boot coding style.

> @Tom Rini, can you apply it either way?
> 
> Best regards,
> Nicolae
-- 
Paul Kocialkowski, developer of free digital technology at the lower levels

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

signature.asc
Description: This is a digitally signed message part
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] power: twl6030: fix code refactoring

2016-10-15 Thread Tom Rini
On Sat, Oct 15, 2016 at 08:41:35PM +0300, Nicolae Rosia wrote:
> Hi,
> 
> On Fri, Oct 14, 2016 at 4:28 PM, Paul Kocialkowski  wrote:
> > Le jeudi 13 octobre 2016 à 13:47 +0300, Nicolae Rosia a écrit :
> >> From: Nicolae Rosia 
> >>
> >> Commit a85362fb3e1fc7833723accddbbae431091d06b8 refactored the code
> >> but the register read ended up in the wrong if branch.
> >> Currently, the else branch checks a variable which is always 0.
> >
> > Good catch! Sorry for including that regression in the first place, I should
> > have checked the patch more thoroughly.
> No worries!
> 
> > See comment below.
> > The indentation before &value is inconsistent with what is done in the rest 
> > of
> > the file. I think you should either just move the line as it was or only 
> > use one
> > extra tab indent for the new line. Since this file breaks the 80 chars 
> > limit in
> > a few places, I would tend to prefer the former.
> >
> checkpatch.pl was complaining, that's why I formatted it.
> @Tom Rini, can you apply it either way?

I'll take this and do you want to do a follow up to make the file
checkpatch clean?  Thanks!

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] power: twl6030: fix code refactoring

2016-10-15 Thread Nicolae Rosia
Hi,

On Fri, Oct 14, 2016 at 4:28 PM, Paul Kocialkowski  wrote:
> Le jeudi 13 octobre 2016 à 13:47 +0300, Nicolae Rosia a écrit :
>> From: Nicolae Rosia 
>>
>> Commit a85362fb3e1fc7833723accddbbae431091d06b8 refactored the code
>> but the register read ended up in the wrong if branch.
>> Currently, the else branch checks a variable which is always 0.
>
> Good catch! Sorry for including that regression in the first place, I should
> have checked the patch more thoroughly.
No worries!

> See comment below.
> The indentation before &value is inconsistent with what is done in the rest of
> the file. I think you should either just move the line as it was or only use 
> one
> extra tab indent for the new line. Since this file breaks the 80 chars limit 
> in
> a few places, I would tend to prefer the former.
>
checkpatch.pl was complaining, that's why I formatted it.
@Tom Rini, can you apply it either way?

Best regards,
Nicolae
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] power: twl6030: fix code refactoring

2016-10-14 Thread Paul Kocialkowski
Le jeudi 13 octobre 2016 à 13:47 +0300, Nicolae Rosia a écrit :
> From: Nicolae Rosia 
> 
> Commit a85362fb3e1fc7833723accddbbae431091d06b8 refactored the code
> but the register read ended up in the wrong if branch.
> Currently, the else branch checks a variable which is always 0.

Good catch! Sorry for including that regression in the first place, I should
have checked the patch more thoroughly.

See comment below.

> Signed-off-by: Nicolae Rosia 
> ---
>  drivers/power/twl6030.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/power/twl6030.c b/drivers/power/twl6030.c
> index 05c79be..cd53200 100644
> --- a/drivers/power/twl6030.c
> +++ b/drivers/power/twl6030.c
> @@ -231,9 +231,9 @@ void twl6030_power_mmc_init(int dev_index)
>   /* Enable P1 output for VMMC */
>   twl6030_i2c_write_u8(TWL6030_CHIP_PM, TWL6030_VMMC_CFG_STATE,
>   TWL6030_CFG_STATE_P1 | TWL6030_CFG_STATE_ON);
> -
> - twl6030_i2c_read_u8(TWL6030_CHIP_PM, TWL6030_PH_STS_BOOT,
> &value);
>   } else if (dev_index == 1) {
> + twl6030_i2c_read_u8(TWL6030_CHIP_PM, TWL6030_PH_STS_BOOT,
> + &value);

The indentation before &value is inconsistent with what is done in the rest of
the file. I think you should either just move the line as it was or only use one
extra tab indent for the new line. Since this file breaks the 80 chars limit in
a few places, I would tend to prefer the former.

>   /* BOOT2 indicates 1.8V/2.8V VAUX1 for eMMC */
>   if (value & TWL6030_PH_STS_BOOT2) {
>   /* 1.8V voltage output for VAUX1 */

-- 
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

signature.asc
Description: This is a digitally signed message part
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH] power: twl6030: fix code refactoring

2016-10-13 Thread Tom Rini
On Thu, Oct 13, 2016 at 01:47:53PM +0300, Nicolae Rosia wrote:

> From: Nicolae Rosia 
> 
> Commit a85362fb3e1fc7833723accddbbae431091d06b8 refactored the code
> but the register read ended up in the wrong if branch.
> Currently, the else branch checks a variable which is always 0.
> 
> Signed-off-by: Nicolae Rosia 

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH] power: twl6030: fix code refactoring

2016-10-13 Thread Nicolae Rosia
From: Nicolae Rosia 

Commit a85362fb3e1fc7833723accddbbae431091d06b8 refactored the code
but the register read ended up in the wrong if branch.
Currently, the else branch checks a variable which is always 0.

Signed-off-by: Nicolae Rosia 
---
 drivers/power/twl6030.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/power/twl6030.c b/drivers/power/twl6030.c
index 05c79be..cd53200 100644
--- a/drivers/power/twl6030.c
+++ b/drivers/power/twl6030.c
@@ -231,9 +231,9 @@ void twl6030_power_mmc_init(int dev_index)
/* Enable P1 output for VMMC */
twl6030_i2c_write_u8(TWL6030_CHIP_PM, TWL6030_VMMC_CFG_STATE,
TWL6030_CFG_STATE_P1 | TWL6030_CFG_STATE_ON);
-
-   twl6030_i2c_read_u8(TWL6030_CHIP_PM, TWL6030_PH_STS_BOOT, 
&value);
} else if (dev_index == 1) {
+   twl6030_i2c_read_u8(TWL6030_CHIP_PM, TWL6030_PH_STS_BOOT,
+   &value);
/* BOOT2 indicates 1.8V/2.8V VAUX1 for eMMC */
if (value & TWL6030_PH_STS_BOOT2) {
/* 1.8V voltage output for VAUX1 */
-- 
2.5.5

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot