| Message ID | 20220110214456.67087-1-sander@svanheule.net |
|---|---|
| State | Accepted |
| Commit | 2ba144e68edb4987a2fe1b1cf418b58cbdc4ee96 |
| Headers | show |
| Series | dt-bindings: power: reset: gpio-restart: Correct default priority | expand |
On Mon, 10 Jan 2022 22:44:56 +0100, Sander Vanheule wrote: > Commit bcd56fe1aa97 ("power: reset: gpio-restart: increase priority > slightly") changed the default restart priority 129, but did not update > the documentation. Correct this, so the driver and documentation have > the same default value. > > Signed-off-by: Sander Vanheule <sander@svanheule.net> > --- > This is a resubmission of RFC: > https://lore.kernel.org/all/cfcd00257daba5aa30b8d20a62ba542be1a6914c.1640887456.git.sander@svanheule.net/ > > The commit message for bcd56fe1aa97 mentions that it is a workaround for > rk3288-veryon boards. However, commit e28ea9dbc52d3 ("ARM: dts: > rockchip: add shared rk3288-veyron files") later adds a gpio-restart > node with a priority value of <200> for those boards, effectively > rendering bcd56fe1aa97 obsolete (for their use case). > > Perhaps bcd56fe1aa97 could just be reverted instead of updating the > documentation. > > An argument against reverting (a 6 year old patch) is that other boards > may have come to depend on the default value of 129. I don't know about > out-of-tree user of gpio-restart, but there are a few in-tree users of > gpio-restart /without/ an explicit priority: > > arch/arm/boot/dts/imx53-ppd.dts (commit 2952d67637716) > DTS submitted after changed default, but DTS copyright predates the > changed default. > > arch/microblaze/boot/dts/system.dts (commit 7cca9b8b7c5bc) > The original DTS commit predates the changed default, but didn't use > gpio-restart. The commit adding gpio-restart appears to indicate no > other restart handlers are present on this platform, although it could > be these were just being shadowed by the custom restart code. > > arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts (commit 0a91330b2af9f) > Recently added board; couldn't find any obvious alternative restart > handlers. > > Best, > Sander > > .../devicetree/bindings/power/reset/gpio-restart.yaml | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Reviewed-by: Rob Herring <robh@kernel.org>
Hi Rob, On Tue, 2022-01-11 at 20:18 -0600, Rob Herring wrote: > On Mon, 10 Jan 2022 22:44:56 +0100, Sander Vanheule wrote: > > Commit bcd56fe1aa97 ("power: reset: gpio-restart: increase priority > > slightly") changed the default restart priority 129, but did not update > > the documentation. Correct this, so the driver and documentation have > > the same default value. > > > > Signed-off-by: Sander Vanheule <sander@svanheule.net> > > --- > > This is a resubmission of RFC: > > https://lore.kernel.org/all/cfcd00257daba5aa30b8d20a62ba542be1a6914c.1640887456.git.sander@svanheule.net/ > > > > The commit message for bcd56fe1aa97 mentions that it is a workaround for > > rk3288-veryon boards. However, commit e28ea9dbc52d3 ("ARM: dts: > > rockchip: add shared rk3288-veyron files") later adds a gpio-restart > > node with a priority value of <200> for those boards, effectively > > rendering bcd56fe1aa97 obsolete (for their use case). > > > > Perhaps bcd56fe1aa97 could just be reverted instead of updating the > > documentation. > > > > An argument against reverting (a 6 year old patch) is that other boards > > may have come to depend on the default value of 129. I don't know about > > out-of-tree user of gpio-restart, but there are a few in-tree users of > > gpio-restart /without/ an explicit priority: > > > > arch/arm/boot/dts/imx53-ppd.dts (commit 2952d67637716) > > DTS submitted after changed default, but DTS copyright predates the > > changed default. > > > > arch/microblaze/boot/dts/system.dts (commit 7cca9b8b7c5bc) > > The original DTS commit predates the changed default, but didn't use > > gpio-restart. The commit adding gpio-restart appears to indicate no > > other restart handlers are present on this platform, although it could > > be these were just being shadowed by the custom restart code. > > > > arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts (commit 0a91330b2af9f) > > Recently added board; couldn't find any obvious alternative restart > > handlers. > > > > Best, > > Sander > > > > .../devicetree/bindings/power/reset/gpio-restart.yaml | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > Reviewed-by: Rob Herring <robh@kernel.org> Thanks for the review! I've noticed the devicetree patchwork has this patch marked as "Not applicable", but linux-pm patchwork has it marked "Handled elsewhere". Since you merged the gpio-restart conversion patch, can you also take this one, Rob? Best, Sander
On Wed, Jan 12, 2022 at 2:49 PM Sander Vanheule <sander@svanheule.net> wrote: > > Hi Rob, > > On Tue, 2022-01-11 at 20:18 -0600, Rob Herring wrote: > > On Mon, 10 Jan 2022 22:44:56 +0100, Sander Vanheule wrote: > > > Commit bcd56fe1aa97 ("power: reset: gpio-restart: increase priority > > > slightly") changed the default restart priority 129, but did not update > > > the documentation. Correct this, so the driver and documentation have > > > the same default value. > > > > > > Signed-off-by: Sander Vanheule <sander@svanheule.net> > > > --- > > > This is a resubmission of RFC: > > > https://lore.kernel.org/all/cfcd00257daba5aa30b8d20a62ba542be1a6914c.1640887456.git.sander@svanheule.net/ > > > > > > The commit message for bcd56fe1aa97 mentions that it is a workaround for > > > rk3288-veryon boards. However, commit e28ea9dbc52d3 ("ARM: dts: > > > rockchip: add shared rk3288-veyron files") later adds a gpio-restart > > > node with a priority value of <200> for those boards, effectively > > > rendering bcd56fe1aa97 obsolete (for their use case). > > > > > > Perhaps bcd56fe1aa97 could just be reverted instead of updating the > > > documentation. > > > > > > An argument against reverting (a 6 year old patch) is that other boards > > > may have come to depend on the default value of 129. I don't know about > > > out-of-tree user of gpio-restart, but there are a few in-tree users of > > > gpio-restart /without/ an explicit priority: > > > > > > arch/arm/boot/dts/imx53-ppd.dts (commit 2952d67637716) > > > DTS submitted after changed default, but DTS copyright predates the > > > changed default. > > > > > > arch/microblaze/boot/dts/system.dts (commit 7cca9b8b7c5bc) > > > The original DTS commit predates the changed default, but didn't use > > > gpio-restart. The commit adding gpio-restart appears to indicate no > > > other restart handlers are present on this platform, although it could > > > be these were just being shadowed by the custom restart code. > > > > > > arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts (commit 0a91330b2af9f) > > > Recently added board; couldn't find any obvious alternative restart > > > handlers. > > > > > > Best, > > > Sander > > > > > > .../devicetree/bindings/power/reset/gpio-restart.yaml | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > Thanks for the review! > > I've noticed the devicetree patchwork has this patch marked as "Not applicable", but > linux-pm patchwork has it marked "Handled elsewhere". Since you merged the gpio-restart > conversion patch, can you also take this one, Rob? Yes, I'll pick it up. Rob
diff --git a/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml b/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml index 3dd22220cb5f..a72d5c721516 100644 --- a/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml +++ b/Documentation/devicetree/bindings/power/reset/gpio-restart.yaml @@ -43,7 +43,7 @@ properties: priority: $ref: /schemas/types.yaml#/definitions/uint32 description: | - A priority ranging from 0 to 255 (default 128) according to the following guidelines: + A priority ranging from 0 to 255 (default 129) according to the following guidelines: 0: Restart handler of last resort, with limited restart capabilities. 128: Default restart handler; use if no other restart handler is expected to be available, @@ -51,7 +51,7 @@ properties: 255: Highest priority restart handler, will preempt all other restart handlers. minimum: 0 maximum: 255 - default: 128 + default: 129 active-delay: $ref: /schemas/types.yaml#/definitions/uint32
Commit bcd56fe1aa97 ("power: reset: gpio-restart: increase priority slightly") changed the default restart priority 129, but did not update the documentation. Correct this, so the driver and documentation have the same default value. Signed-off-by: Sander Vanheule <sander@svanheule.net> --- This is a resubmission of RFC: https://lore.kernel.org/all/cfcd00257daba5aa30b8d20a62ba542be1a6914c.1640887456.git.sander@svanheule.net/ The commit message for bcd56fe1aa97 mentions that it is a workaround for rk3288-veryon boards. However, commit e28ea9dbc52d3 ("ARM: dts: rockchip: add shared rk3288-veyron files") later adds a gpio-restart node with a priority value of <200> for those boards, effectively rendering bcd56fe1aa97 obsolete (for their use case). Perhaps bcd56fe1aa97 could just be reverted instead of updating the documentation. An argument against reverting (a 6 year old patch) is that other boards may have come to depend on the default value of 129. I don't know about out-of-tree user of gpio-restart, but there are a few in-tree users of gpio-restart /without/ an explicit priority: arch/arm/boot/dts/imx53-ppd.dts (commit 2952d67637716) DTS submitted after changed default, but DTS copyright predates the changed default. arch/microblaze/boot/dts/system.dts (commit 7cca9b8b7c5bc) The original DTS commit predates the changed default, but didn't use gpio-restart. The commit adding gpio-restart appears to indicate no other restart handlers are present on this platform, although it could be these were just being shadowed by the custom restart code. arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts (commit 0a91330b2af9f) Recently added board; couldn't find any obvious alternative restart handlers. Best, Sander .../devicetree/bindings/power/reset/gpio-restart.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)