| Message ID | 20250411131423.3802611-2-guodong@riscstar.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | pwm: Update PWM_PXA driver for SpacemiT K1 | expand |
| Context | Check | Description |
|---|---|---|
| bjorn/pre-ci_am | fail | Failed to apply series |
On Fri, Apr 11, 2025 at 09:14:15PM +0800, Guodong Xu wrote: > Add an optional resets property for the Marvell PWM PXA binding. > > Signed-off-by: Guodong Xu <guodong@riscstar.com> > --- > Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml > index 9ee1946dc2e1..9640d4b627c2 100644 > --- a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml > +++ b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml > @@ -31,6 +31,9 @@ properties: > clocks: > maxItems: 1 > > + resets: > + maxItems: 1 Do any of the currently supported devices use a reset? If not, then add this in tandem with the new compatible and only allow it there please. > + > required: > - compatible > - reg > -- > 2.43.0 >
On Fr, 2025-04-11 at 17:44 +0100, Conor Dooley wrote: > On Fri, Apr 11, 2025 at 09:14:15PM +0800, Guodong Xu wrote: > > Add an optional resets property for the Marvell PWM PXA binding. > > > > Signed-off-by: Guodong Xu <guodong@riscstar.com> > > --- > > Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml > > index 9ee1946dc2e1..9640d4b627c2 100644 > > --- a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml > > +++ b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml > > @@ -31,6 +31,9 @@ properties: > > clocks: > > maxItems: 1 > > > > + resets: > > + maxItems: 1 > > Do any of the currently supported devices use a reset? If not, then add > this in tandem with the new compatible and only allow it there please. Also, if spacemit,k1-pwm can not work without the reset being deasserted, mark it as required. The driver can still use reset_control_get_optional. regards Philipp
On Tue, Apr 15, 2025 at 4:53 PM Philipp Zabel <p.zabel@pengutronix.de> wrote: > > On Fr, 2025-04-11 at 17:44 +0100, Conor Dooley wrote: > > On Fri, Apr 11, 2025 at 09:14:15PM +0800, Guodong Xu wrote: > > > Add an optional resets property for the Marvell PWM PXA binding. > > > > > > Signed-off-by: Guodong Xu <guodong@riscstar.com> > > > --- > > > Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml > > > index 9ee1946dc2e1..9640d4b627c2 100644 > > > --- a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml > > > +++ b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml > > > @@ -31,6 +31,9 @@ properties: > > > clocks: > > > maxItems: 1 > > > > > > + resets: > > > + maxItems: 1 > > > > Do any of the currently supported devices use a reset? If not, then add > > this in tandem with the new compatible and only allow it there please. > > Also, if spacemit,k1-pwm can not work without the reset being > deasserted, mark it as required. > Thank you Philipp. spacemit,k1-pwm can not work without the reset. I will add that in the next version. -Guodong > The driver can still use reset_control_get_optional. > > regards > Philipp
Hi Philipp, On 17:54 Tue 15 Apr , Guodong Xu wrote: > On Tue, Apr 15, 2025 at 4:53 PM Philipp Zabel <p.zabel@pengutronix.de> wrote: > > > > On Fr, 2025-04-11 at 17:44 +0100, Conor Dooley wrote: > > > On Fri, Apr 11, 2025 at 09:14:15PM +0800, Guodong Xu wrote: > > > > Add an optional resets property for the Marvell PWM PXA binding. > > > > > > > > Signed-off-by: Guodong Xu <guodong@riscstar.com> > > > > --- > > > > Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml > > > > index 9ee1946dc2e1..9640d4b627c2 100644 > > > > --- a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml > > > > +++ b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml > > > > @@ -31,6 +31,9 @@ properties: > > > > clocks: > > > > maxItems: 1 > > > > > > > > + resets: > > > > + maxItems: 1 > > > > > > Do any of the currently supported devices use a reset? If not, then add > > > this in tandem with the new compatible and only allow it there please. > > > > Also, if spacemit,k1-pwm can not work without the reset being > > deasserted, mark it as required. If I inerpret correctly, only reset requires explicitly being de-asserted, need to mark as required? that's being said, if reset comes out as de-asserted by default after power reset, then not necessary? (in other cases, some device block is in asserted state by default) thanks > > > > Thank you Philipp. spacemit,k1-pwm can not work without the reset. > I will add that in the next version. > > -Guodong > > > The driver can still use reset_control_get_optional. > > > > regards > > Philipp
On 4/15/25 5:12 AM, Yixun Lan wrote: > Hi Philipp, > > On 17:54 Tue 15 Apr , Guodong Xu wrote: >> On Tue, Apr 15, 2025 at 4:53 PM Philipp Zabel <p.zabel@pengutronix.de> wrote: >>> >>> On Fr, 2025-04-11 at 17:44 +0100, Conor Dooley wrote: >>>> On Fri, Apr 11, 2025 at 09:14:15PM +0800, Guodong Xu wrote: >>>>> Add an optional resets property for the Marvell PWM PXA binding. >>>>> >>>>> Signed-off-by: Guodong Xu <guodong@riscstar.com> >>>>> --- >>>>> Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml | 3 +++ >>>>> 1 file changed, 3 insertions(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml >>>>> index 9ee1946dc2e1..9640d4b627c2 100644 >>>>> --- a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml >>>>> +++ b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml >>>>> @@ -31,6 +31,9 @@ properties: >>>>> clocks: >>>>> maxItems: 1 >>>>> >>>>> + resets: >>>>> + maxItems: 1 >>>> >>>> Do any of the currently supported devices use a reset? If not, then add >>>> this in tandem with the new compatible and only allow it there please. >>> >>> Also, if spacemit,k1-pwm can not work without the reset being >>> deasserted, mark it as required. > > If I inerpret correctly, only reset requires explicitly being de-asserted, > need to mark as required? that's being said, if reset comes out as de-asserted > by default after power reset, then not necessary? > (in other cases, some device block is in asserted state by default) We can often benefit from the state that the boot loader has left things in, but I think it's better not to assume it if possible. I suppose it might not be required though. Anyway, the reset line is available to use; why not require it? -Alex > thanks >>> >> >> Thank you Philipp. spacemit,k1-pwm can not work without the reset. >> I will add that in the next version. >> >> -Guodong >> >>> The driver can still use reset_control_get_optional. >>> >>> regards >>> Philipp >
Hi Alex, On 07:11 Tue 15 Apr , Alex Elder wrote: > On 4/15/25 5:12 AM, Yixun Lan wrote: > > Hi Philipp, > > > > On 17:54 Tue 15 Apr , Guodong Xu wrote: > >> On Tue, Apr 15, 2025 at 4:53 PM Philipp Zabel <p.zabel@pengutronix.de> wrote: > >>> > >>> On Fr, 2025-04-11 at 17:44 +0100, Conor Dooley wrote: > >>>> On Fri, Apr 11, 2025 at 09:14:15PM +0800, Guodong Xu wrote: > >>>>> Add an optional resets property for the Marvell PWM PXA binding. > >>>>> > >>>>> Signed-off-by: Guodong Xu <guodong@riscstar.com> > >>>>> --- > >>>>> Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml | 3 +++ > >>>>> 1 file changed, 3 insertions(+) > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml > >>>>> index 9ee1946dc2e1..9640d4b627c2 100644 > >>>>> --- a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml > >>>>> +++ b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml > >>>>> @@ -31,6 +31,9 @@ properties: > >>>>> clocks: > >>>>> maxItems: 1 > >>>>> > >>>>> + resets: > >>>>> + maxItems: 1 > >>>> > >>>> Do any of the currently supported devices use a reset? If not, then add > >>>> this in tandem with the new compatible and only allow it there please. > >>> > >>> Also, if spacemit,k1-pwm can not work without the reset being > >>> deasserted, mark it as required. > > > > If I inerpret correctly, only reset requires explicitly being de-asserted, > > need to mark as required? that's being said, if reset comes out as de-asserted > > by default after power reset, then not necessary? > > (in other cases, some device block is in asserted state by default) > > We can often benefit from the state that the boot loader has left > things in, but I think it's better not to assume it if possible. right, I agree mostly > I suppose it might not be required though. > > Anyway, the reset line is available to use; why not require it? > maybe there are cases that users don't want to issue a reset.. so, want to make it optional.. I can think one example that, display controller is up and working from bootloader to linux, reset it will got a flicker picture.. anyway, my question was trying to understand the policy of writting DTS hehind.. > > > thanks > >>> > >> > >> Thank you Philipp. spacemit,k1-pwm can not work without the reset. > >> I will add that in the next version. > >> > >> -Guodong > >> > >>> The driver can still use reset_control_get_optional. > >>> > >>> regards > >>> Philipp > > >
Hello, On Tue, Apr 15, 2025 at 12:28:07PM +0000, Yixun Lan wrote: > maybe there are cases that users don't want to issue a reset.. > so, want to make it optional.. I can think one example that, > display controller is up and working from bootloader to linux, > reset it will got a flicker picture.. Agreed. You can just deassert the reset at probe time. That shouldn't interfere with a PWM that is already producing an output. > GPG Key ID AABEFD55 If you advertise your OpenPGP certificate, I recommend using the long id. See for example https://keys.openpgp.org/search?q=AABEFD55.
On 4/16/25 12:18 AM, Uwe Kleine-König wrote: > Hello, > > On Tue, Apr 15, 2025 at 12:28:07PM +0000, Yixun Lan wrote: >> maybe there are cases that users don't want to issue a reset.. >> so, want to make it optional.. I can think one example that, >> display controller is up and working from bootloader to linux, >> reset it will got a flicker picture.. > > Agreed. You can just deassert the reset at probe time. That shouldn't > interfere with a PWM that is already producing an output. I think you're saying reset can be a required property, to be harmlessly deasserted at probe time? Yixun was suggesting it should not be required, because it might already be deasserted. Anyway, I don't feel strongly either way. Maybe the DTS maintainers can recommend what to do. -Alex > >> GPG Key ID AABEFD55 > > If you advertise your OpenPGP certificate, I recommend using the long > id. See for example https://keys.openpgp.org/search?q=AABEFD55.
diff --git a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml index 9ee1946dc2e1..9640d4b627c2 100644 --- a/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml +++ b/Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml @@ -31,6 +31,9 @@ properties: clocks: maxItems: 1 + resets: + maxItems: 1 + required: - compatible - reg
Add an optional resets property for the Marvell PWM PXA binding. Signed-off-by: Guodong Xu <guodong@riscstar.com> --- Documentation/devicetree/bindings/pwm/marvell,pxa-pwm.yaml | 3 +++ 1 file changed, 3 insertions(+)