| Message ID | 20200322112437.18070-1-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: dts: Fix leftover entry-methods for PSCI | expand |
On Sun, Mar 22, 2020 at 4:55 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > These two device trees were either missed or added after > the commit correcting the "entry-method" from > "arm,psci" to just "psci" as per the binding. My patch went in, in 4.18. The FSL entry went in, in 4.20 and the spreadtrum one in 5.5. > Fixes: commit e9880240e4f4 ("arm64: dts: Fix various entry-method properties to reflect documentation") So only minor comment is that it isn't really a fix. We've tried improving the text in the binding too but somehow people still get confused. Converting the binding to YAML and enforcing it seems to be the only course of action left now. Otherwise, Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org> > Cc: Amit Kucheria <amit.kucheria@linaro.org> > Cc: Sudeep Holla <sudeep.holla@arm.com> > Cc: Fabio Estevam <festevam@gmail.com> > Cc: Shawn Guo <shawnguo@kernel.org> > Cc: Chunyan Zhang <chunyan.zhang@unisoc.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > ARM SoC folks: if this is fine just apply it to the > tree where appropriate please. > --- > arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 2 +- > arch/arm64/boot/dts/sprd/sc9863a.dtsi | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi > index 0bf375ec959b..55b71bb4baf8 100644 > --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi > @@ -53,7 +53,7 @@ > * PSCI node is not added default, U-boot will add missing > * parts if it determines to use PSCI. > */ > - entry-method = "arm,psci"; > + entry-method = "psci"; > > CPU_PW20: cpu-pw20 { > compatible = "arm,idle-state"; > diff --git a/arch/arm64/boot/dts/sprd/sc9863a.dtsi b/arch/arm64/boot/dts/sprd/sc9863a.dtsi > index cd80756c888d..2c590ca1d079 100644 > --- a/arch/arm64/boot/dts/sprd/sc9863a.dtsi > +++ b/arch/arm64/boot/dts/sprd/sc9863a.dtsi > @@ -108,7 +108,7 @@ > }; > > idle-states { > - entry-method = "arm,psci"; > + entry-method = "psci"; > CORE_PD: core-pd { > compatible = "arm,idle-state"; > entry-latency-us = <4000>; > -- > 2.21.1 >
On Sun, Mar 22, 2020 at 12:49 PM Amit Kucheria <amit.kucheria@linaro.org> wrote: > My patch went in, in 4.18. > > The FSL entry went in, in 4.20 and the spreadtrum one in 5.5. > > > Fixes: commit e9880240e4f4 ("arm64: dts: Fix various entry-method properties to reflect documentation") > > So only minor comment is that it isn't really a fix. OK I'll resend a v2 without the Fixes and your reviewed-by. > We've tried > improving the text in the binding too but somehow people still get > confused. Converting the binding to YAML and enforcing it seems to be > the only course of action left now. Since nothing in the kernel checks for entry-method right now, maybe we could just add code to do that and warn in dmesg if entry-method is set to anything else than "psci"? Yours, Linus Walleij
On Sun, Mar 22, 2020 at 5:26 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Sun, Mar 22, 2020 at 12:49 PM Amit Kucheria <amit.kucheria@linaro.org> wrote: > > > My patch went in, in 4.18. > > > > The FSL entry went in, in 4.20 and the spreadtrum one in 5.5. > > > > > Fixes: commit e9880240e4f4 ("arm64: dts: Fix various entry-method properties to reflect documentation") > > > > So only minor comment is that it isn't really a fix. > > OK I'll resend a v2 without the Fixes and your reviewed-by. > > > We've tried > > improving the text in the binding too but somehow people still get > > confused. Converting the binding to YAML and enforcing it seems to be > > the only course of action left now. > > Since nothing in the kernel checks for entry-method right now, maybe > we could just add code to do that and warn in dmesg if entry-method > is set to anything else than "psci"? Hi Linus, Documentation/devicetree/bindings/arm/idle-states.yaml already triggers an error on 'make dtbs_check'. Perhaps we just keep an eye on these? db845c-check-2020-02-24-222456.log:/home/amit/work/builds/build-check/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dt.yaml: idle-states: entry-method:0: 'psci' was expected db845c-check-2020-02-24-222456.log:/home/amit/work/builds/build-check/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dt.yaml: idle-states: entry-method:0: 'psci' was expected db845c-check-2020-02-24-222456.log:/home/amit/work/builds/build-check/arch/arm64/boot/dts/sprd/sp9863a-1h10.dt.yaml: idle-states: entry-method:0: 'psci' was expected Regards, Amit
On Tue, Mar 24, 2020 at 6:56 AM Amit Kucheria <amit.kucheria@linaro.org> wrote: > On Sun, Mar 22, 2020 at 5:26 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Sun, Mar 22, 2020 at 12:49 PM Amit Kucheria <amit.kucheria@linaro.org> wrote: > > > > > My patch went in, in 4.18. > > > > > > The FSL entry went in, in 4.20 and the spreadtrum one in 5.5. > > > > > > > Fixes: commit e9880240e4f4 ("arm64: dts: Fix various entry-method properties to reflect documentation") > > > > > > So only minor comment is that it isn't really a fix. > > > > OK I'll resend a v2 without the Fixes and your reviewed-by. > > > > > We've tried > > > improving the text in the binding too but somehow people still get > > > confused. Converting the binding to YAML and enforcing it seems to be > > > the only course of action left now. > > > > Since nothing in the kernel checks for entry-method right now, maybe > > we could just add code to do that and warn in dmesg if entry-method > > is set to anything else than "psci"? > > Hi Linus, > > Documentation/devicetree/bindings/arm/idle-states.yaml already > triggers an error on 'make dtbs_check'. Perhaps we just keep an eye on > these? > > db845c-check-2020-02-24-222456.log:/home/amit/work/builds/build-check/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dt.yaml: > idle-states: entry-method:0: 'psci' was expected > db845c-check-2020-02-24-222456.log:/home/amit/work/builds/build-check/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dt.yaml: > idle-states: entry-method:0: 'psci' was expected > db845c-check-2020-02-24-222456.log:/home/amit/work/builds/build-check/arch/arm64/boot/dts/sprd/sp9863a-1h10.dt.yaml: > idle-states: entry-method:0: 'psci' was expected Aha that's pretty awesome actually. I think right now we have a bit too many warnings coming out of the YAML schema but sooner or later we'll actually start to plow through the backlog of warnings and fix stuff up... :) Yours, Linus Walleij
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi index 0bf375ec959b..55b71bb4baf8 100644 --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi @@ -53,7 +53,7 @@ * PSCI node is not added default, U-boot will add missing * parts if it determines to use PSCI. */ - entry-method = "arm,psci"; + entry-method = "psci"; CPU_PW20: cpu-pw20 { compatible = "arm,idle-state"; diff --git a/arch/arm64/boot/dts/sprd/sc9863a.dtsi b/arch/arm64/boot/dts/sprd/sc9863a.dtsi index cd80756c888d..2c590ca1d079 100644 --- a/arch/arm64/boot/dts/sprd/sc9863a.dtsi +++ b/arch/arm64/boot/dts/sprd/sc9863a.dtsi @@ -108,7 +108,7 @@ }; idle-states { - entry-method = "arm,psci"; + entry-method = "psci"; CORE_PD: core-pd { compatible = "arm,idle-state"; entry-latency-us = <4000>;
These two device trees were either missed or added after the commit correcting the "entry-method" from "arm,psci" to just "psci" as per the binding. Fixes: commit e9880240e4f4 ("arm64: dts: Fix various entry-method properties to reflect documentation") Cc: Amit Kucheria <amit.kucheria@linaro.org> Cc: Sudeep Holla <sudeep.holla@arm.com> Cc: Fabio Estevam <festevam@gmail.com> Cc: Shawn Guo <shawnguo@kernel.org> Cc: Chunyan Zhang <chunyan.zhang@unisoc.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- ARM SoC folks: if this is fine just apply it to the tree where appropriate please. --- arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 2 +- arch/arm64/boot/dts/sprd/sc9863a.dtsi | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)