[PATCH] regulator: twl-regulator - fix twlreg_set_mode

[PATCH] regulator: twl-regulator - fix twlreg_set_mode

[Axel Lin]

The Singular Message is 16 bits:
        DEV_GRP[15:13] MT[12]  RES_ID[11:4]  RES_STATE[3:0]

Current implementation return immedially after sucessfuly write MSB part.
To properly set mode, we need to write the complete message ( MSB and LSB ).

In twl.h, now we have defines for PM Master module register offsets,
use it instead of hard coded 0x15/0x16.

Use "message & 0xff" to ensure we send correct value for LSB.

Signed-off-by: Axel Lin <axel.lin@gmail.com>
---

I don't have this hardware handy, I appreciate if someone who has the
hardware can test this patch.

Regards,
Axel

 drivers/regulator/twl-regulator.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c
index 7e5892e..a57262a 100644
--- a/drivers/regulator/twl-regulator.c
+++ b/drivers/regulator/twl-regulator.c
@@ -219,12 +219,12 @@ static int twlreg_set_mode(struct regulator_dev *rdev, unsigned mode)
 		return -EACCES;
 
 	status = twl_i2c_write_u8(TWL_MODULE_PM_MASTER,
-			message >> 8, 0x15 /* PB_WORD_MSB */ );
-	if (status >= 0)
+			message >> 8, TWL4030_PM_MASTER_PB_WORD_MSB);
+	if (status < 0)
 		return status;
 
 	return twl_i2c_write_u8(TWL_MODULE_PM_MASTER,
-			message, 0x16 /* PB_WORD_LSB */ );
+			message & 0xff, TWL4030_PM_MASTER_PB_WORD_LSB);
 }
 
 /*----------------------------------------------------------------------*/
-- 
1.7.2

Re: [PATCH] regulator: twl-regulator - fix twlreg_set_mode

[Mark Brown]

On Fri, Oct 22, 2010 at 04:38:22PM +0800, Axel Lin wrote:
> The Singular Message is 16 bits:
>         DEV_GRP[15:13] MT[12]  RES_ID[11:4]  RES_STATE[3:0]
> 
> Current implementation return immedially after sucessfuly write MSB part.
> To properly set mode, we need to write the complete message ( MSB and LSB ).
> 
> In twl.h, now we have defines for PM Master module register offsets,
> use it instead of hard coded 0x15/0x16.
> 
> Use "message & 0xff" to ensure we send correct value for LSB.
> 
> Signed-off-by: Axel Lin <axel.lin@gmail.com>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

> I don't have this hardware handy, I appreciate if someone who has the
> hardware can test this patch.

I beleive Liam has appropriate hardware.

Re: [PATCH] regulator: twl-regulator - fix twlreg_set_mode

[Liam Girdwood]

On Fri, 2010-10-22 at 08:30 -0700, Mark Brown wrote:
> On Fri, Oct 22, 2010 at 04:38:22PM +0800, Axel Lin wrote:
> > The Singular Message is 16 bits:
> >         DEV_GRP[15:13] MT[12]  RES_ID[11:4]  RES_STATE[3:0]
> > 
> > Current implementation return immedially after sucessfuly write MSB part.
> > To properly set mode, we need to write the complete message ( MSB and LSB ).
> > 
> > In twl.h, now we have defines for PM Master module register offsets,
> > use it instead of hard coded 0x15/0x16.
> > 
> > Use "message & 0xff" to ensure we send correct value for LSB.
> > 
> > Signed-off-by: Axel Lin <axel.lin@gmail.com>
> 
> Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> 
> > I don't have this hardware handy, I appreciate if someone who has the
> > hardware can test this patch.
> 
> I beleive Liam has appropriate hardware.

Rejendra, care to test this ?

Thanks

Liam 
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

RE: [PATCH] regulator: twl-regulator - fix twlreg_set_mode

[Nayak, Rajendra]

> -----Original Message-----
> From: Liam Girdwood [mailto:lrg@slimlogic.co.uk]
> Sent: Saturday, October 23, 2010 7:17 PM
> To: Nayak, Rajendra
> Cc: Axel Lin; linux-kernel; Juha Keski-Saari; Mark Brown
> Subject: Re: [PATCH] regulator: twl-regulator - fix twlreg_set_mode
> 
> On Fri, 2010-10-22 at 08:30 -0700, Mark Brown wrote:
> > On Fri, Oct 22, 2010 at 04:38:22PM +0800, Axel Lin wrote:
> > > The Singular Message is 16 bits:
> > >         DEV_GRP[15:13] MT[12]  RES_ID[11:4]  RES_STATE[3:0]
> > >
> > > Current implementation return immedially after sucessfuly write MSB part.
> > > To properly set mode, we need to write the complete message ( MSB and LSB ).
> > >
> > > In twl.h, now we have defines for PM Master module register offsets,
> > > use it instead of hard coded 0x15/0x16.
> > >
> > > Use "message & 0xff" to ensure we send correct value for LSB.
> > >
> > > Signed-off-by: Axel Lin <axel.lin@gmail.com>
> >
> > Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> >
> > > I don't have this hardware handy, I appreciate if someone who has the
> > > hardware can test this patch.
> >
> > I beleive Liam has appropriate hardware.
> 
> Rejendra, care to test this ?

Sure, either me or Lesly will test this on one of the twl4030 based platforms
and update.

> 
> Thanks
> 
> Liam
> --
> Freelance Developer, SlimLogic Ltd
> ASoC and Voltage Regulator Maintainer.
> http://www.slimlogic.co.uk

RE: [PATCH] regulator: twl-regulator - fix twlreg_set_mode

[Lesly Arackal Manuel]

I have tested it with MMC card inserted on OMAP3630SDP board (using OMAP3
integration tree).

The changes seem to be valid.

The i2c_transfer() returns '1' on success and the twl_i2c_write() which
calls i2c_transfer returns '0' if success and err value if it fails.

After writing the MSB if it returns with check:
if (status >= 0)
	return status;
It is not writing the LSB.

So it should be:
if (status < 0)
	return status;

Regards,
Lesly AM

> -----Original Message-----
> From: Nayak, Rajendra [mailto:rnayak@ti.com]
> Sent: Tuesday, October 26, 2010 3:40 PM
> To: Liam Girdwood
> Cc: Axel Lin; linux-kernel; Juha Keski-Saari; Mark Brown; Manuel, Lesly
> Arackal
> Subject: RE: [PATCH] regulator: twl-regulator - fix twlreg_set_mode
> 
> 
> 
> > -----Original Message-----
> > From: Liam Girdwood [mailto:lrg@slimlogic.co.uk]
> > Sent: Saturday, October 23, 2010 7:17 PM
> > To: Nayak, Rajendra
> > Cc: Axel Lin; linux-kernel; Juha Keski-Saari; Mark Brown
> > Subject: Re: [PATCH] regulator: twl-regulator - fix twlreg_set_mode
> >
> > On Fri, 2010-10-22 at 08:30 -0700, Mark Brown wrote:
> > > On Fri, Oct 22, 2010 at 04:38:22PM +0800, Axel Lin wrote:
> > > > The Singular Message is 16 bits:
> > > >         DEV_GRP[15:13] MT[12]  RES_ID[11:4]  RES_STATE[3:0]
> > > >
> > > > Current implementation return immedially after sucessfuly write MSB
> part.
> > > > To properly set mode, we need to write the complete message ( MSB
> and LSB ).
> > > >
> > > > In twl.h, now we have defines for PM Master module register offsets,
> > > > use it instead of hard coded 0x15/0x16.
> > > >
> > > > Use "message & 0xff" to ensure we send correct value for LSB.
> > > >
> > > > Signed-off-by: Axel Lin <axel.lin@gmail.com>
> > >
> > > Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> > >
> > > > I don't have this hardware handy, I appreciate if someone who has
> the
> > > > hardware can test this patch.
> > >
> > > I beleive Liam has appropriate hardware.
> >
> > Rejendra, care to test this ?
> 
> Sure, either me or Lesly will test this on one of the twl4030 based
> platforms
> and update.
> 
> >
> > Thanks
> >
> > Liam
> > --
> > Freelance Developer, SlimLogic Ltd
> > ASoC and Voltage Regulator Maintainer.
> > http://www.slimlogic.co.uk

RE: [PATCH] regulator: twl-regulator - fix twlreg_set_mode

[Lesly Arackal Manuel]

Forgot to mention-

Only omap hsmmc driver is using regulator_set_mode() which will call
twlreg_set_mode().

> -----Original Message-----
> From: Lesly Arackal Manuel [mailto:leslyam@ti.com]
> Sent: Thursday, November 04, 2010 3:01 PM
> To: 'Nayak, Rajendra'; 'Liam Girdwood'
> Cc: 'Axel Lin'; 'linux-kernel'; 'Juha Keski-Saari'; 'Mark Brown'
> Subject: RE: [PATCH] regulator: twl-regulator - fix twlreg_set_mode
> 
> 
> I have tested it with MMC card inserted on OMAP3630SDP board (using OMAP3
> integration tree).
> 
> The changes seem to be valid.
> 
> The i2c_transfer() returns '1' on success and the twl_i2c_write() which
> calls i2c_transfer returns '0' if success and err value if it fails.
> 
> After writing the MSB if it returns with check:
> if (status >= 0)
> 	return status;
> It is not writing the LSB.
> 
> So it should be:
> if (status < 0)
> 	return status;
> 
> Regards,
> Lesly AM
> 
> > -----Original Message-----
> > From: Nayak, Rajendra [mailto:rnayak@ti.com]
> > Sent: Tuesday, October 26, 2010 3:40 PM
> > To: Liam Girdwood
> > Cc: Axel Lin; linux-kernel; Juha Keski-Saari; Mark Brown; Manuel, Lesly
> > Arackal
> > Subject: RE: [PATCH] regulator: twl-regulator - fix twlreg_set_mode
> >
> >
> >
> > > -----Original Message-----
> > > From: Liam Girdwood [mailto:lrg@slimlogic.co.uk]
> > > Sent: Saturday, October 23, 2010 7:17 PM
> > > To: Nayak, Rajendra
> > > Cc: Axel Lin; linux-kernel; Juha Keski-Saari; Mark Brown
> > > Subject: Re: [PATCH] regulator: twl-regulator - fix twlreg_set_mode
> > >
> > > On Fri, 2010-10-22 at 08:30 -0700, Mark Brown wrote:
> > > > On Fri, Oct 22, 2010 at 04:38:22PM +0800, Axel Lin wrote:
> > > > > The Singular Message is 16 bits:
> > > > >         DEV_GRP[15:13] MT[12]  RES_ID[11:4]  RES_STATE[3:0]
> > > > >
> > > > > Current implementation return immedially after sucessfuly write
> MSB
> > part.
> > > > > To properly set mode, we need to write the complete message ( MSB
> > and LSB ).
> > > > >
> > > > > In twl.h, now we have defines for PM Master module register
> offsets,
> > > > > use it instead of hard coded 0x15/0x16.
> > > > >
> > > > > Use "message & 0xff" to ensure we send correct value for LSB.
> > > > >
> > > > > Signed-off-by: Axel Lin <axel.lin@gmail.com>
> > > >
> > > > Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> > > >
> > > > > I don't have this hardware handy, I appreciate if someone who has
> > the
> > > > > hardware can test this patch.
> > > >
> > > > I beleive Liam has appropriate hardware.
> > >
> > > Rejendra, care to test this ?
> >
> > Sure, either me or Lesly will test this on one of the twl4030 based
> > platforms
> > and update.
> >
> > >
> > > Thanks
> > >
> > > Liam
> > > --
> > > Freelance Developer, SlimLogic Ltd
> > > ASoC and Voltage Regulator Maintainer.
> > > http://www.slimlogic.co.uk

RE: [PATCH] regulator: twl-regulator - fix twlreg_set_mode

[Liam Girdwood]

On Thu, 2010-11-04 at 15:08 +0530, Lesly Arackal Manuel wrote:
> Forgot to mention-
> 
> Only omap hsmmc driver is using regulator_set_mode() which will call
> twlreg_set_mode().
> 
> > -----Original Message-----
> > From: Lesly Arackal Manuel [mailto:leslyam@ti.com]
> > Sent: Thursday, November 04, 2010 3:01 PM
> > To: 'Nayak, Rajendra'; 'Liam Girdwood'
> > Cc: 'Axel Lin'; 'linux-kernel'; 'Juha Keski-Saari'; 'Mark Brown'
> > Subject: RE: [PATCH] regulator: twl-regulator - fix twlreg_set_mode
> > 
> > 
> > I have tested it with MMC card inserted on OMAP3630SDP board (using OMAP3
> > integration tree).
> > 
> > The changes seem to be valid.
> > 
> > The i2c_transfer() returns '1' on success and the twl_i2c_write() which
> > calls i2c_transfer returns '0' if success and err value if it fails.
> > 
> > After writing the MSB if it returns with check:
> > if (status >= 0)
> > 	return status;
> > It is not writing the LSB.
> > 
> > So it should be:
> > if (status < 0)
> > 	return status;
> > 
> > Regards,
> > Lesly AM
> > 

Applied.

Thanks

Liam
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk