* [PATCH] regulator: tps6586x: Having slew rate settings for other than SM0/1 is not fatal
@ 2013-02-16 11:50 Axel Lin
2013-02-19 16:47 ` Stephen Warren
0 siblings, 1 reply; 7+ messages in thread
From: Axel Lin @ 2013-02-16 11:50 UTC (permalink / raw)
To: Mark Brown; +Cc: Mike Rapoport, Laxman Dewangan, Liam Girdwood, linux-kernel
Ignore the setting and show "Only SM0/SM1 can set slew rate" warning is enough,
then we can return 0 instead of -EINVAL in tps6586x_regulator_set_slew_rate().
Otherwise, probe() fails.
Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
drivers/regulator/tps6586x-regulator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/regulator/tps6586x-regulator.c b/drivers/regulator/tps6586x-regulator.c
index e68382d..51da3f9 100644
--- a/drivers/regulator/tps6586x-regulator.c
+++ b/drivers/regulator/tps6586x-regulator.c
@@ -246,7 +246,7 @@ static int tps6586x_regulator_set_slew_rate(struct platform_device *pdev,
break;
default:
dev_warn(&pdev->dev, "Only SM0/SM1 can set slew rate\n");
- return -EINVAL;
+ return 0;
}
return tps6586x_write(parent, reg,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] regulator: tps6586x: Having slew rate settings for other than SM0/1 is not fatal
2013-02-16 11:50 [PATCH] regulator: tps6586x: Having slew rate settings for other than SM0/1 is not fatal Axel Lin
@ 2013-02-19 16:47 ` Stephen Warren
2013-02-19 18:26 ` Mark Brown
2013-02-20 0:53 ` Axel Lin
0 siblings, 2 replies; 7+ messages in thread
From: Stephen Warren @ 2013-02-19 16:47 UTC (permalink / raw)
To: Axel Lin
Cc: Mark Brown, Mike Rapoport, Laxman Dewangan, Liam Girdwood,
linux-kernel
On 02/16/2013 04:50 AM, Axel Lin wrote:
> Ignore the setting and show "Only SM0/SM1 can set slew rate" warning is enough,
> then we can return 0 instead of -EINVAL in tps6586x_regulator_set_slew_rate().
>
> Otherwise, probe() fails.
Why does probe() fail; what is trying to set a slew rate on a regulator
that doesn't support it? At least a few days ago in linux-next, this
patch wasn't needed AFAIK. Is the problem something new?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] regulator: tps6586x: Having slew rate settings for other than SM0/1 is not fatal
2013-02-19 16:47 ` Stephen Warren
@ 2013-02-19 18:26 ` Mark Brown
2013-02-19 19:32 ` Stephen Warren
2013-02-20 0:53 ` Axel Lin
1 sibling, 1 reply; 7+ messages in thread
From: Mark Brown @ 2013-02-19 18:26 UTC (permalink / raw)
To: Stephen Warren
Cc: Axel Lin, Mike Rapoport, Laxman Dewangan, Liam Girdwood,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 772 bytes --]
On Tue, Feb 19, 2013 at 09:47:29AM -0700, Stephen Warren wrote:
> On 02/16/2013 04:50 AM, Axel Lin wrote:
> > Ignore the setting and show "Only SM0/SM1 can set slew rate" warning is enough,
> > then we can return 0 instead of -EINVAL in tps6586x_regulator_set_slew_rate().
> >
> > Otherwise, probe() fails.
> Why does probe() fail; what is trying to set a slew rate on a regulator
> that doesn't support it? At least a few days ago in linux-next, this
> patch wasn't needed AFAIK. Is the problem something new?
I rather suspect Axel is doing this based on code inspection and review
rather than testing (either that or he has an enormous lab somewhere
full of all sorts of hardware!) - what he's saying is that the error
handling here seems excessive.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] regulator: tps6586x: Having slew rate settings for other than SM0/1 is not fatal
2013-02-19 18:26 ` Mark Brown
@ 2013-02-19 19:32 ` Stephen Warren
2013-02-19 20:07 ` Mark Brown
0 siblings, 1 reply; 7+ messages in thread
From: Stephen Warren @ 2013-02-19 19:32 UTC (permalink / raw)
To: Mark Brown
Cc: Axel Lin, Mike Rapoport, Laxman Dewangan, Liam Girdwood,
linux-kernel
On 02/19/2013 11:26 AM, Mark Brown wrote:
> On Tue, Feb 19, 2013 at 09:47:29AM -0700, Stephen Warren wrote:
>> On 02/16/2013 04:50 AM, Axel Lin wrote:
>
>>> Ignore the setting and show "Only SM0/SM1 can set slew rate"
>>> warning is enough, then we can return 0 instead of -EINVAL in
>>> tps6586x_regulator_set_slew_rate().
>>>
>>> Otherwise, probe() fails.
>
>> Why does probe() fail; what is trying to set a slew rate on a
>> regulator that doesn't support it? At least a few days ago in
>> linux-next, this patch wasn't needed AFAIK. Is the problem
>> something new?
>
> I rather suspect Axel is doing this based on code inspection and
> review rather than testing (either that or he has an enormous lab
> somewhere full of all sorts of hardware!)
Makes sense.
> - what he's saying is that the error handling here seems
> excessive.
Why shouldn't the driver return an error if it's asked to do something
that's impossible?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] regulator: tps6586x: Having slew rate settings for other than SM0/1 is not fatal
2013-02-19 19:32 ` Stephen Warren
@ 2013-02-19 20:07 ` Mark Brown
0 siblings, 0 replies; 7+ messages in thread
From: Mark Brown @ 2013-02-19 20:07 UTC (permalink / raw)
To: Stephen Warren
Cc: Axel Lin, Mike Rapoport, Laxman Dewangan, Liam Girdwood,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 519 bytes --]
On Tue, Feb 19, 2013 at 12:32:50PM -0700, Stephen Warren wrote:
> On 02/19/2013 11:26 AM, Mark Brown wrote:
> > - what he's saying is that the error handling here seems
> > excessive.
> Why shouldn't the driver return an error if it's asked to do something
> that's impossible?
I'm having a hard time caring too much to be honest. On the one hand it
means that if that feature gets added for some reason new DTs won't work
on older kernels, on the other hand perhaps the hardware doesn't support
doing this at all.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] regulator: tps6586x: Having slew rate settings for other than SM0/1 is not fatal
2013-02-19 16:47 ` Stephen Warren
2013-02-19 18:26 ` Mark Brown
@ 2013-02-20 0:53 ` Axel Lin
2013-02-20 2:12 ` Stephen Warren
1 sibling, 1 reply; 7+ messages in thread
From: Axel Lin @ 2013-02-20 0:53 UTC (permalink / raw)
To: Stephen Warren
Cc: Mark Brown, Mike Rapoport, Laxman Dewangan, Liam Girdwood,
linux-kernel
2013/2/20 Stephen Warren <swarren@wwwdotorg.org>:
> On 02/16/2013 04:50 AM, Axel Lin wrote:
>> Ignore the setting and show "Only SM0/SM1 can set slew rate" warning is enough,
>> then we can return 0 instead of -EINVAL in tps6586x_regulator_set_slew_rate().
>>
>> Otherwise, probe() fails.
>
> Why does probe() fail; what is trying to set a slew rate on a regulator
> that doesn't support it? At least a few days ago in linux-next, this
> patch wasn't needed AFAIK. Is the problem something new?
>
Oh, sorry for my poor Engilish.
I mean probe fails because of having slew rate settings for other than SM0/1
seems not necessary.
In tps6586x_regulator_set_slew_rate() it uses dev_warn rather than dev_err
for the default case.
We can either using "dev_warn with return 0" or use "dev_err with
return -EINVAL"
in tps6586x_regulator_set_slew_rate().
It looks to me that having slew rate settings for other than SM0/1 should be ok
if it actually is harmless ( because we can just ignore the setting ).
BTW, I read the code but I don't have this hardware.
Axel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] regulator: tps6586x: Having slew rate settings for other than SM0/1 is not fatal
2013-02-20 0:53 ` Axel Lin
@ 2013-02-20 2:12 ` Stephen Warren
0 siblings, 0 replies; 7+ messages in thread
From: Stephen Warren @ 2013-02-20 2:12 UTC (permalink / raw)
To: Axel Lin
Cc: Mark Brown, Mike Rapoport, Laxman Dewangan, Liam Girdwood,
linux-kernel
On 02/19/2013 05:53 PM, Axel Lin wrote:
> 2013/2/20 Stephen Warren <swarren@wwwdotorg.org>:
>> On 02/16/2013 04:50 AM, Axel Lin wrote:
>>> Ignore the setting and show "Only SM0/SM1 can set slew rate" warning is enough,
>>> then we can return 0 instead of -EINVAL in tps6586x_regulator_set_slew_rate().
>>>
>>> Otherwise, probe() fails.
>>
>> Why does probe() fail; what is trying to set a slew rate on a regulator
>> that doesn't support it? At least a few days ago in linux-next, this
>> patch wasn't needed AFAIK. Is the problem something new?
>>
>
> Oh, sorry for my poor Engilish.
> I mean probe fails because of having slew rate settings for other than SM0/1
> seems not necessary.
>
> In tps6586x_regulator_set_slew_rate() it uses dev_warn rather than dev_err
> for the default case.
> We can either using "dev_warn with return 0" or use "dev_err with
> return -EINVAL"
> in tps6586x_regulator_set_slew_rate().
Oh right, I understand. s/dev_warn/dev_err/ seems more appropriate to
me, I think.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-02-20 2:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-16 11:50 [PATCH] regulator: tps6586x: Having slew rate settings for other than SM0/1 is not fatal Axel Lin
2013-02-19 16:47 ` Stephen Warren
2013-02-19 18:26 ` Mark Brown
2013-02-19 19:32 ` Stephen Warren
2013-02-19 20:07 ` Mark Brown
2013-02-20 0:53 ` Axel Lin
2013-02-20 2:12 ` Stephen Warren
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox