* [PATCH] regulator: max8649 - fix setting extclk_freq
@ 2010-10-01 5:56 Axel Lin
2010-10-01 7:19 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Axel Lin @ 2010-10-01 5:56 UTC (permalink / raw)
To: linux-kernel; +Cc: Haojian Zhuang, Liam Girdwood, Mark Brown
The SYNC bits are BIT6 and BIT7 of MAX8649_SYNC register.
pdata->extclk_freq could be [0|1|2].
(MAX8649_EXTCLK_26MHZ|MAX8649_EXTCLK_13MHZ|MAX8649_EXTCLK_19MHZ)
It requires to left shift 6 bits to properly set extclk_freq.
Signed-off-by: Axel Lin <axel.lin@gmail.com>
---
drivers/regulator/max8649.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/regulator/max8649.c b/drivers/regulator/max8649.c
index 4520ace..6b60a9c 100644
--- a/drivers/regulator/max8649.c
+++ b/drivers/regulator/max8649.c
@@ -330,7 +330,7 @@ static int __devinit max8649_regulator_probe(struct i2c_client *client,
/* set external clock frequency */
info->extclk_freq = pdata->extclk_freq;
max8649_set_bits(info->i2c, MAX8649_SYNC, MAX8649_EXT_MASK,
- info->extclk_freq);
+ info->extclk_freq << 6);
}
if (pdata->ramp_timing) {
--
1.7.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] regulator: max8649 - fix setting extclk_freq
2010-10-01 5:56 [PATCH] regulator: max8649 - fix setting extclk_freq Axel Lin
@ 2010-10-01 7:19 ` Mark Brown
2010-10-01 7:53 ` Axel Lin
2010-10-01 11:18 ` Axel Lin
0 siblings, 2 replies; 6+ messages in thread
From: Mark Brown @ 2010-10-01 7:19 UTC (permalink / raw)
To: Axel Lin; +Cc: linux-kernel, Haojian Zhuang, Liam Girdwood
On Fri, Oct 01, 2010 at 01:56:27PM +0800, Axel Lin wrote:
> The SYNC bits are BIT6 and BIT7 of MAX8649_SYNC register.
> pdata->extclk_freq could be [0|1|2].
> (MAX8649_EXTCLK_26MHZ|MAX8649_EXTCLK_13MHZ|MAX8649_EXTCLK_19MHZ)
> It requires to left shift 6 bits to properly set extclk_freq.
>
> Signed-off-by: Axel Lin <axel.lin@gmail.com>
Are there existing users of the driver? It's not always 100% clear if
people are expecting bitfield values that will be shifted in or values
that can be ored directly in. I don't really mind either way but I'd
rather be careful.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] regulator: max8649 - fix setting extclk_freq
2010-10-01 7:19 ` Mark Brown
@ 2010-10-01 7:53 ` Axel Lin
2010-10-01 11:18 ` Axel Lin
1 sibling, 0 replies; 6+ messages in thread
From: Axel Lin @ 2010-10-01 7:53 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Haojian Zhuang, Liam Girdwood
2010/10/1 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Fri, Oct 01, 2010 at 01:56:27PM +0800, Axel Lin wrote:
>> The SYNC bits are BIT6 and BIT7 of MAX8649_SYNC register.
>> pdata->extclk_freq could be [0|1|2].
>> (MAX8649_EXTCLK_26MHZ|MAX8649_EXTCLK_13MHZ|MAX8649_EXTCLK_19MHZ)
>> It requires to left shift 6 bits to properly set extclk_freq.
>>
>> Signed-off-by: Axel Lin <axel.lin@gmail.com>
>
> Are there existing users of the driver? It's not always 100% clear if
> people are expecting bitfield values that will be shifted in or values
> that can be ored directly in. I don't really mind either way but I'd
> rather be careful.
>
max8649 regulator is used in arch/arm/mach-mmp/jasper.c (Marvell
Jasper Development Platform)
I checked include/linux/regulator/max8649.h
extclk_freq only takes 2 bits:
unsigned extclk_freq:2;
People should set extclk_freq to
[MAX8649_EXTCLK_26MHZ|MAX8649_EXTCLK_13MHZ|MAX8649_EXTCLK_19MHZ],
so we need to do 6 bits left shift to properly set extclk_freq.
Regards,
Axel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] regulator: max8649 - fix setting extclk_freq
2010-10-01 7:19 ` Mark Brown
2010-10-01 7:53 ` Axel Lin
@ 2010-10-01 11:18 ` Axel Lin
2010-10-01 17:36 ` Mark Brown
1 sibling, 1 reply; 6+ messages in thread
From: Axel Lin @ 2010-10-01 11:18 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, Haojian Zhuang, Liam Girdwood
2010/10/1 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Fri, Oct 01, 2010 at 01:56:27PM +0800, Axel Lin wrote:
>> The SYNC bits are BIT6 and BIT7 of MAX8649_SYNC register.
>> pdata->extclk_freq could be [0|1|2].
>> (MAX8649_EXTCLK_26MHZ|MAX8649_EXTCLK_13MHZ|MAX8649_EXTCLK_19MHZ)
>> It requires to left shift 6 bits to properly set extclk_freq.
>>
>> Signed-off-by: Axel Lin <axel.lin@gmail.com>
>
> Are there existing users of the driver? It's not always 100% clear if
> people are expecting bitfield values that will be shifted in or values
> that can be ored directly in. I don't really mind either way but I'd
> rather be careful.
>
I think I should explain this way:
In max8649.h , we have
enum {
MAX8649_EXTCLK_26MHZ = 0,
MAX8649_EXTCLK_13MHZ,
MAX8649_EXTCLK_19MHZ, /* 19.2MHz */
};
Thus, I think users will set
extclk_freq = MAX8649_EXTCLK_26MHZ;
but not
extclk_freq = (MAX8649_EXTCLK_26MHZ << 6);
Regards,
Axel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] regulator: max8649 - fix setting extclk_freq
2010-10-01 11:18 ` Axel Lin
@ 2010-10-01 17:36 ` Mark Brown
2010-10-02 13:22 ` Liam Girdwood
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2010-10-01 17:36 UTC (permalink / raw)
To: Axel Lin; +Cc: linux-kernel, Haojian Zhuang, Liam Girdwood
On Fri, Oct 01, 2010 at 07:18:14PM +0800, Axel Lin wrote:
> 2010/10/1 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> > Are there existing users of the driver? It's not always 100% clear if
> > people are expecting bitfield values that will be shifted in or values
> > that can be ored directly in. I don't really mind either way but I'd
> > rather be careful.
> I think I should explain this way:
> In max8649.h , we have
> enum {
> MAX8649_EXTCLK_26MHZ = 0,
> MAX8649_EXTCLK_13MHZ,
> MAX8649_EXTCLK_19MHZ, /* 19.2MHz */
> };
OK, that's clearer. When changing the interpretation of existing
platform data like this it's good to note this sort of thing in the
changelog since merge issues can be a real pain.
Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] regulator: max8649 - fix setting extclk_freq
2010-10-01 17:36 ` Mark Brown
@ 2010-10-02 13:22 ` Liam Girdwood
0 siblings, 0 replies; 6+ messages in thread
From: Liam Girdwood @ 2010-10-02 13:22 UTC (permalink / raw)
To: Mark Brown; +Cc: Axel Lin, linux-kernel, Haojian Zhuang
On Fri, 2010-10-01 at 10:36 -0700, Mark Brown wrote:
> On Fri, Oct 01, 2010 at 07:18:14PM +0800, Axel Lin wrote:
> > 2010/10/1 Mark Brown <broonie@opensource.wolfsonmicro.com>:
>
> > > Are there existing users of the driver? It's not always 100% clear if
> > > people are expecting bitfield values that will be shifted in or values
> > > that can be ored directly in. I don't really mind either way but I'd
> > > rather be careful.
>
> > I think I should explain this way:
>
> > In max8649.h , we have
> > enum {
> > MAX8649_EXTCLK_26MHZ = 0,
> > MAX8649_EXTCLK_13MHZ,
> > MAX8649_EXTCLK_19MHZ, /* 19.2MHz */
> > };
>
> OK, that's clearer. When changing the interpretation of existing
> platform data like this it's good to note this sort of thing in the
> changelog since merge issues can be a real pain.
>
> Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> --
Applied.
Thanks
Liam
--
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-02 13:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-01 5:56 [PATCH] regulator: max8649 - fix setting extclk_freq Axel Lin
2010-10-01 7:19 ` Mark Brown
2010-10-01 7:53 ` Axel Lin
2010-10-01 11:18 ` Axel Lin
2010-10-01 17:36 ` Mark Brown
2010-10-02 13:22 ` Liam Girdwood
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox