| Message ID | 20210308233858.24741-2-daniel.lezcano@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RESEND,v5,1/4] dt-bindings: devfreq: rk3399_dmc: Add rockchip,pmu phandle. | expand |
Hi Daniel, Some comments. Have a look if it's useful or that you disagree with. New nodes should be verifiable if possible. Especially with so many properties. Could you convert rockchip-dfi.txt and rk3399_dmc.txt to yaml instead of changing old txt documents? Add rockchip-dfi.yaml and rk3399_dmc.yaml before this patch in version 6. Nodes and properties have a sort order. Please fix. Some goes for [RESEND PATCH v5 3/4]. (This is a generic dtsi. How about cooling and dmc ??) ---- Heiko rules: compatible reg interrupts [alphabetical] status [if needed] ---- My incomplete list: For nodes: If exists on top: model, compatible and chosen. Sort things without reg alphabetical first, then sort the rest by reg address. Inside nodes: If exists on top: compatible, reg and interrupts. In alphabetical order the required properties. Then in alphabetical order the other properties. And as last things that start with '#' in alphabetical order. Add status below all other properties for soc internal components with any board-specifics. Keep an empty line between properties and nodes. Exceptions: Sort pinctrl-0 above pinctrl-names, so it stays in line with clock-names and dma-names. Sort simple-audio-card,name above other simple-audio-card properties. Sort regulator-name above other regulator properties. Sort regulator-min-microvolt above regulator-max-microvolt. On 3/9/21 12:38 AM, Daniel Lezcano wrote: > From: Lin Huang <hl@rock-chips.com> > > These are required to support DDR DVFS on rk3399 platform. > > Signed-off-by: Lin Huang <hl@rock-chips.com> > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> > Signed-off-by: Gaël PORTAY <gael.portay@collabora.com> > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > --- > arch/arm64/boot/dts/rockchip/rk3399.dtsi | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > index edbbf35fe19e..6f23d99236fe 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi > @@ -1937,6 +1937,25 @@ > status = "disabled"; > }; > > + dfi: dfi@ff630000 { > + reg = <0x00 0xff630000 0x00 0x4000>; > + compatible = "rockchip,rk3399-dfi"; > + rockchip,pmu = <&pmugrf>; > + interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH 0>; > + clocks = <&cru PCLK_DDR_MON>; > + clock-names = "pclk_ddr_mon"; > + status = "disabled"; > + }; > + > + dmc: dmc { > + compatible = "rockchip,rk3399-dmc"; > + rockchip,pmu = <&pmugrf>; > + devfreq-events = <&dfi>; > + clocks = <&cru SCLK_DDRC>; > + clock-names = "dmc_clk"; > + status = "disabled"; > + }; > + > pinctrl: pinctrl { > compatible = "rockchip,rk3399-pinctrl"; > rockchip,grf = <&grf>; >
On 09/03/2021 12:42, Johan Jonker wrote: > Hi Daniel, > > Some comments. Have a look if it's useful or that you disagree with. > > New nodes should be verifiable if possible. > Especially with so many properties. > Could you convert rockchip-dfi.txt and rk3399_dmc.txt to yaml instead of > changing old txt documents? > Add rockchip-dfi.yaml and rk3399_dmc.yaml before this patch in version 6. I don't have a lot of bandwidth to do it but I will give a try. Thanks for reviewing -- Daniel > Nodes and properties have a sort order. Please fix. > Some goes for [RESEND PATCH v5 3/4]. > > (This is a generic dtsi. How about cooling and dmc ??) > > ---- > Heiko rules: > > compatible > reg > interrupts > [alphabetical] > status [if needed] > > ---- > My incomplete list: > > For nodes: > If exists on top: model, compatible and chosen. > Sort things without reg alphabetical first, > then sort the rest by reg address. > > Inside nodes: > If exists on top: compatible, reg and interrupts. > In alphabetical order the required properties. > Then in alphabetical order the other properties. > And as last things that start with '#' in alphabetical order. > Add status below all other properties for soc internal components with > any board-specifics. > Keep an empty line between properties and nodes. > > Exceptions: > Sort pinctrl-0 above pinctrl-names, so it stays in line with clock-names > and dma-names. > Sort simple-audio-card,name above other simple-audio-card properties. > Sort regulator-name above other regulator properties. > Sort regulator-min-microvolt above regulator-max-microvolt. > > On 3/9/21 12:38 AM, Daniel Lezcano wrote: >> From: Lin Huang <hl@rock-chips.com> >> >> These are required to support DDR DVFS on rk3399 platform. >> >> Signed-off-by: Lin Huang <hl@rock-chips.com> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> >> Signed-off-by: Gaël PORTAY <gael.portay@collabora.com> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> >> --- >> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi >> index edbbf35fe19e..6f23d99236fe 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi >> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi >> @@ -1937,6 +1937,25 @@ >> status = "disabled"; >> }; >> >> + dfi: dfi@ff630000 { >> + reg = <0x00 0xff630000 0x00 0x4000>; >> + compatible = "rockchip,rk3399-dfi"; >> + rockchip,pmu = <&pmugrf>; >> + interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH 0>; >> + clocks = <&cru PCLK_DDR_MON>; >> + clock-names = "pclk_ddr_mon"; >> + status = "disabled"; >> + }; >> + >> + dmc: dmc { >> + compatible = "rockchip,rk3399-dmc"; >> + rockchip,pmu = <&pmugrf>; >> + devfreq-events = <&dfi>; >> + clocks = <&cru SCLK_DDRC>; >> + clock-names = "dmc_clk"; >> + status = "disabled"; >> + }; >> + >> pinctrl: pinctrl { >> compatible = "rockchip,rk3399-pinctrl"; >> rockchip,grf = <&grf>; >> > -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
On 09/03/2021 12:42, Johan Jonker wrote: > Hi Daniel, > > (This is a generic dtsi. How about cooling and dmc ??) Yeah, I will add it after. I need to figure out the right dynamic power coefficient from the different places around (android / chromeos / etc...). Any help for that is welcome. [ ... ] -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index edbbf35fe19e..6f23d99236fe 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi @@ -1937,6 +1937,25 @@ status = "disabled"; }; + dfi: dfi@ff630000 { + reg = <0x00 0xff630000 0x00 0x4000>; + compatible = "rockchip,rk3399-dfi"; + rockchip,pmu = <&pmugrf>; + interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH 0>; + clocks = <&cru PCLK_DDR_MON>; + clock-names = "pclk_ddr_mon"; + status = "disabled"; + }; + + dmc: dmc { + compatible = "rockchip,rk3399-dmc"; + rockchip,pmu = <&pmugrf>; + devfreq-events = <&dfi>; + clocks = <&cru SCLK_DDRC>; + clock-names = "dmc_clk"; + status = "disabled"; + }; + pinctrl: pinctrl { compatible = "rockchip,rk3399-pinctrl"; rockchip,grf = <&grf>;