Re: [linux-sunxi] Re: [PATCH 52/54] arm64: dts: allwinner: Remove regulator-ramp-delay

2021-08-06 Thread Icenowy Zheng



于 2021年8月6日 GMT+08:00 下午8:05:56, Chen-Yu Tsai  写到:
>On Fri, Aug 6, 2021 at 7:49 PM Icenowy Zheng  wrote:
>>
>> 在 2021-07-22星期四的 10:16 +0200,Maxime Ripard写道:
>> > On Thu, Jul 22, 2021 at 12:55:53AM -0500, Samuel Holland wrote:
>> > > On 7/21/21 9:04 AM, Maxime Ripard wrote:
>> > > > The regulator-ramp-delay property isn't documented in the binding
>> > > > for
>> > > > the AXP806, and it's ignored by the driver. Remove those
>> > > > properties.
>> > >
>> > > This is a generic regulator property, parsed by
>> > > of_get_regulation_constraints, which is called by
>> > > regulator_of_get_init_data in the regulator core. And it appears in
>> > > bindings/regulator/regulator.yaml. I believe the binding needs to be
>> > > fixed, not the device trees.
>> >
>> > It's indeed parsed by the regulator framework, but then it calls into
>> > the driver if that property is set using set_ramp_delay if it's set.
>>
>> Not only is it used for set_ramp_delay, but it's also used to calculate
>> a post-change delay, see the following position (it can be overrided by
>> a custom set_voltage_time in the driver):
>>
>> https://elixir.bootlin.com/linux/latest/source/drivers/regulator/core.c#L3339
>
>Having just dug through the regulator core code at work, I agree.
>
>Furthermore, the commit log for this addition specifically mentions the
>reason for adding this property is to provide a (guessed) ramp rate for
>the core to do a proper delay for the regulator to ramp up, not to set
>the actual ramp rate in hardware.

Well we must agree that we have some more suitable property that delays
for a constant span, see lines below in the function I mentioned.

>
>
>ChenYu
>
>
>[1] https://git.kernel.org/torvalds/c/fe79ea577be81e1e71642826ab00e676dc59c194
>
>> >
>> > https://elixir.bootlin.com/linux/latest/source/drivers/regulator/core.c#L1378
>> >
>> > We don't set that hook for the AXP806 DCDC-A and DCDC-E regulators
>> > (that
>> > use AXP_DESC_RANGES) at all:
>> >
>> > https://elixir.bootlin.com/linux/latest/source/drivers/regulator/axp20x-regulator.c#L343
>> >
>> > And the only implementation we have (set for AXP_DESC and AXP_DESC_IO)
>> > works only for the AXP209:
>> >
>> > https://elixir.bootlin.com/linux/latest/source/drivers/regulator/axp20x-regulator.c#L368
>> >
>> > So, it just looks like those properties have never been tested since
>> > they were just ignored.
>> >
>> > Maxime
>> >
>>
>> --
>> You received this message because you are subscribed to the Google Groups 
>> "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to linux-sunxi+unsubscr...@googlegroups.com.
>> To view this discussion on the web, visit 
>> https://groups.google.com/d/msgid/linux-sunxi/68e4820ead3107aa4e80dcaf9243bd11de5fce98.camel%40aosc.io.
>

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/0308F3FA-B5DE-4FFC-AD22-4F5E94CDF466%40aosc.io.


Re: [linux-sunxi] Re: [PATCH 52/54] arm64: dts: allwinner: Remove regulator-ramp-delay

2021-08-06 Thread Chen-Yu Tsai
On Fri, Aug 6, 2021 at 7:49 PM Icenowy Zheng  wrote:
>
> 在 2021-07-22星期四的 10:16 +0200,Maxime Ripard写道:
> > On Thu, Jul 22, 2021 at 12:55:53AM -0500, Samuel Holland wrote:
> > > On 7/21/21 9:04 AM, Maxime Ripard wrote:
> > > > The regulator-ramp-delay property isn't documented in the binding
> > > > for
> > > > the AXP806, and it's ignored by the driver. Remove those
> > > > properties.
> > >
> > > This is a generic regulator property, parsed by
> > > of_get_regulation_constraints, which is called by
> > > regulator_of_get_init_data in the regulator core. And it appears in
> > > bindings/regulator/regulator.yaml. I believe the binding needs to be
> > > fixed, not the device trees.
> >
> > It's indeed parsed by the regulator framework, but then it calls into
> > the driver if that property is set using set_ramp_delay if it's set.
>
> Not only is it used for set_ramp_delay, but it's also used to calculate
> a post-change delay, see the following position (it can be overrided by
> a custom set_voltage_time in the driver):
>
> https://elixir.bootlin.com/linux/latest/source/drivers/regulator/core.c#L3339

Having just dug through the regulator core code at work, I agree.

Furthermore, the commit log for this addition specifically mentions the
reason for adding this property is to provide a (guessed) ramp rate for
the core to do a proper delay for the regulator to ramp up, not to set
the actual ramp rate in hardware.


ChenYu


[1] https://git.kernel.org/torvalds/c/fe79ea577be81e1e71642826ab00e676dc59c194

> >
> > https://elixir.bootlin.com/linux/latest/source/drivers/regulator/core.c#L1378
> >
> > We don't set that hook for the AXP806 DCDC-A and DCDC-E regulators
> > (that
> > use AXP_DESC_RANGES) at all:
> >
> > https://elixir.bootlin.com/linux/latest/source/drivers/regulator/axp20x-regulator.c#L343
> >
> > And the only implementation we have (set for AXP_DESC and AXP_DESC_IO)
> > works only for the AXP209:
> >
> > https://elixir.bootlin.com/linux/latest/source/drivers/regulator/axp20x-regulator.c#L368
> >
> > So, it just looks like those properties have never been tested since
> > they were just ignored.
> >
> > Maxime
> >
>
> --
> You received this message because you are subscribed to the Google Groups 
> "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-sunxi+unsubscr...@googlegroups.com.
> To view this discussion on the web, visit 
> https://groups.google.com/d/msgid/linux-sunxi/68e4820ead3107aa4e80dcaf9243bd11de5fce98.camel%40aosc.io.

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/CAGb2v65dAAc274JiZyTswuqGHn1tE9urJpTBv%3Dis2CG1UT2CTA%40mail.gmail.com.


Re: [linux-sunxi] Re: [PATCH 52/54] arm64: dts: allwinner: Remove regulator-ramp-delay

2021-08-06 Thread Icenowy Zheng
在 2021-07-22星期四的 10:16 +0200,Maxime Ripard写道:
> On Thu, Jul 22, 2021 at 12:55:53AM -0500, Samuel Holland wrote:
> > On 7/21/21 9:04 AM, Maxime Ripard wrote:
> > > The regulator-ramp-delay property isn't documented in the binding
> > > for
> > > the AXP806, and it's ignored by the driver. Remove those
> > > properties.
> > 
> > This is a generic regulator property, parsed by
> > of_get_regulation_constraints, which is called by
> > regulator_of_get_init_data in the regulator core. And it appears in
> > bindings/regulator/regulator.yaml. I believe the binding needs to be
> > fixed, not the device trees.
> 
> It's indeed parsed by the regulator framework, but then it calls into
> the driver if that property is set using set_ramp_delay if it's set.

Not only is it used for set_ramp_delay, but it's also used to calculate
a post-change delay, see the following position (it can be overrided by
a custom set_voltage_time in the driver):

https://elixir.bootlin.com/linux/latest/source/drivers/regulator/core.c#L3339

> 
> https://elixir.bootlin.com/linux/latest/source/drivers/regulator/core.c#L1378
> 
> We don't set that hook for the AXP806 DCDC-A and DCDC-E regulators
> (that
> use AXP_DESC_RANGES) at all:
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/regulator/axp20x-regulator.c#L343
> 
> And the only implementation we have (set for AXP_DESC and AXP_DESC_IO)
> works only for the AXP209:
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/regulator/axp20x-regulator.c#L368
> 
> So, it just looks like those properties have never been tested since
> they were just ignored.
> 
> Maxime
> 

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit 
https://groups.google.com/d/msgid/linux-sunxi/68e4820ead3107aa4e80dcaf9243bd11de5fce98.camel%40aosc.io.