| Message ID | 1614773878-8058-1-git-send-email-rnayak@codeaurora.org |
|---|---|
| Headers | show |
| Series | Add binding updates and DT files for SC7280 SoC | expand |
Quoting Rajendra Nayak (2021-03-03 04:17:49) > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > index 4a56d9c..21c2399 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > @@ -30,6 +31,18 @@ > }; > }; > > + reserved_memory: reserved-memory { Do we plan to use this label at any point? I'd prefer we remove this until it becomes useful. > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + aop_cmd_db_mem: memory@80860000 { > + reg = <0x0 0x80860000 0x0 0x20000>; > + compatible = "qcom,cmd-db"; > + no-map; > + }; > + }; > + > cpus { > #address-cells = <2>; > #size-cells = <0>; > @@ -203,6 +229,7 @@ > interrupt-controller; > #interrupt-cells = <2>; > gpio-ranges = <&tlmm 0 0 175>; > + wakeup-parent = <&pdc>; > > qup_uart5_default: qup-uart5-default { > pins = "gpio46", "gpio47"; > @@ -287,6 +314,23 @@ > status = "disabled"; > }; > }; > + > + apps_rsc: rsc@18200000 { Any better name than 'rsc'? Maybe 'power-controller'? > + compatible = "qcom,rpmh-rsc"; > + reg = <0 0x18200000 0 0x10000>, > + <0 0x18210000 0 0x10000>, > + <0 0x18220000 0 0x10000>; > + reg-names = "drv-0", "drv-1", "drv-2"; > + interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; > + qcom,tcs-offset = <0xd00>; > + qcom,drv-id = <2>; > + qcom,tcs-config = <ACTIVE_TCS 2>, > + <SLEEP_TCS 3>, > + <WAKE_TCS 3>, > + <CONTROL_TCS 1>; > + }; > };
Quoting Rajendra Nayak (2021-03-03 04:17:50) > Add rpmhcc clock controller node for SC7280. Also add references to > rpmhcc clocks in gcc. > > Signed-off-by: Taniya Das <tdas@codeaurora.org> > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Quoting Rajendra Nayak (2021-03-03 04:17:47) > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > new file mode 100644 > index 0000000..4a56d9c > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > @@ -0,0 +1,299 @@ > +// SPDX-License-Identifier: BSD-3-Clause > +/* > + * sc7280 SoC device tree source > + * > + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved. > + */ > + > +#include <dt-bindings/clock/qcom,gcc-sc7280.h> > +#include <dt-bindings/interrupt-controller/arm-gic.h> > + > +/ { > + interrupt-parent = <&intc>; > + > + #address-cells = <2>; > + #size-cells = <2>; > + > + chosen { }; > + > + clocks { > + xo_board: xo-board { > + compatible = "fixed-clock"; > + clock-frequency = <76800000>; If this is the correct frequency I think we need to update the rpmh clk driver to use the correct divider? Right now I think it is a 2 when it should be 4? > + #clock-cells = <0>; > + }; > + > + sleep_clk: sleep-clk { > + compatible = "fixed-clock"; > + clock-frequency = <32000>; > + #clock-cells = <0>; > + }; > + };
Quoting Rajendra Nayak (2021-03-03 04:17:56) > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > index fe4fdb9..aa6f847 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > @@ -239,6 +239,25 @@ > interrupt-controller; > }; > > + spmi_bus: spmi@c440000 { > + compatible = "qcom,spmi-pmic-arb"; > + reg = <0 0x0c440000 0 0x1100>, > + <0 0x0c600000 0 0x2000000>, > + <0 0x0e600000 0 0x100000>, > + <0 0x0e700000 0 0xa0000>, > + <0 0x0c40a000 0 0x26000>; > + reg-names = "core", "chnls", "obsrvr", "intr", "cnfg"; > + interrupt-names = "periph_irq"; > + interrupts-extended = <&pdc 1 IRQ_TYPE_LEVEL_HIGH>; > + qcom,ee = <0>; > + qcom,channel = <0>; > + #address-cells = <1>; > + #size-cells = <1>; I see the binding says these should be 2 instead of 1 but I suspect that is incorrect. > + interrupt-controller; > + #interrupt-cells = <4>; > + cell-index = <0>; Is cell-index used? Please remove as I don't see it used anywhere and not in the binding. > + }; > + > tlmm: pinctrl@f100000 { > compatible = "qcom,sc7280-pinctrl"; > reg = <0 0x0f100000 0 0x1000000>;
Quoting Rajendra Nayak (2021-03-03 04:17:57) > From: Maulik Shah <mkshah@codeaurora.org> > > Add cpuidle states for little and big cpus. Please also say "The latency values are preliminary placeholders and will be updated once testing provides the real numbers". > > Signed-off-by: Maulik Shah <mkshah@codeaurora.org> > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > --- With that commit text update Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Quoting Rajendra Nayak (2021-03-03 04:17:58) > Add the DT node for the rpmhpd power controller on SC7280 SoCs. > > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> > --- Reviewed-by: Stephen Boyd <swboyd@chromium.org>
On 3/4/2021 5:34 AM, Stephen Boyd wrote: > Quoting Rajendra Nayak (2021-03-03 04:17:49) >> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> index 4a56d9c..21c2399 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> @@ -30,6 +31,18 @@ >> }; >> }; >> >> + reserved_memory: reserved-memory { > > Do we plan to use this label at any point? I'd prefer we remove this > until it becomes useful. sure, i'll drop it > >> + #address-cells = <2>; >> + #size-cells = <2>; >> + ranges; >> + >> + aop_cmd_db_mem: memory@80860000 { >> + reg = <0x0 0x80860000 0x0 0x20000>; >> + compatible = "qcom,cmd-db"; >> + no-map; >> + }; >> + }; >> + >> cpus { >> #address-cells = <2>; >> #size-cells = <0>; >> @@ -203,6 +229,7 @@ >> interrupt-controller; >> #interrupt-cells = <2>; >> gpio-ranges = <&tlmm 0 0 175>; >> + wakeup-parent = <&pdc>; >> >> qup_uart5_default: qup-uart5-default { >> pins = "gpio46", "gpio47"; >> @@ -287,6 +314,23 @@ >> status = "disabled"; >> }; >> }; >> + >> + apps_rsc: rsc@18200000 { > > Any better name than 'rsc'? Maybe 'power-controller'? hmm, Maulik, any thoughts? This would perhaps need the bindings docs to be updated as well (and maybe the existing platform DTs using rsc too) > >> + compatible = "qcom,rpmh-rsc"; >> + reg = <0 0x18200000 0 0x10000>, >> + <0 0x18210000 0 0x10000>, >> + <0 0x18220000 0 0x10000>; >> + reg-names = "drv-0", "drv-1", "drv-2"; >> + interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; >> + qcom,tcs-offset = <0xd00>; >> + qcom,drv-id = <2>; >> + qcom,tcs-config = <ACTIVE_TCS 2>, >> + <SLEEP_TCS 3>, >> + <WAKE_TCS 3>, >> + <CONTROL_TCS 1>; >> + }; >> };
On 3/4/2021 5:42 AM, Stephen Boyd wrote: > Quoting Rajendra Nayak (2021-03-03 04:17:56) >> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> index fe4fdb9..aa6f847 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> @@ -239,6 +239,25 @@ >> interrupt-controller; >> }; >> >> + spmi_bus: spmi@c440000 { >> + compatible = "qcom,spmi-pmic-arb"; >> + reg = <0 0x0c440000 0 0x1100>, >> + <0 0x0c600000 0 0x2000000>, >> + <0 0x0e600000 0 0x100000>, >> + <0 0x0e700000 0 0xa0000>, >> + <0 0x0c40a000 0 0x26000>; >> + reg-names = "core", "chnls", "obsrvr", "intr", "cnfg"; >> + interrupt-names = "periph_irq"; >> + interrupts-extended = <&pdc 1 IRQ_TYPE_LEVEL_HIGH>; >> + qcom,ee = <0>; >> + qcom,channel = <0>; >> + #address-cells = <1>; >> + #size-cells = <1>; > > I see the binding says these should be 2 instead of 1 but I suspect that > is incorrect. yeah looks like the bindings need to be fixed > >> + interrupt-controller; >> + #interrupt-cells = <4>; >> + cell-index = <0>; > > Is cell-index used? Please remove as I don't see it used anywhere and > not in the binding. I'll drop it. thanks > >> + }; >> + >> tlmm: pinctrl@f100000 { >> compatible = "qcom,sc7280-pinctrl"; >> reg = <0 0x0f100000 0 0x1000000>;
On 3/4/2021 5:43 AM, Stephen Boyd wrote: > Quoting Rajendra Nayak (2021-03-03 04:17:57) >> From: Maulik Shah <mkshah@codeaurora.org> >> >> Add cpuidle states for little and big cpus. > > Please also say "The latency values are preliminary placeholders and will be updated > once testing provides the real numbers". will do when I respin, thanks. > >> >> Signed-off-by: Maulik Shah <mkshah@codeaurora.org> >> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> >> --- > > With that commit text update > > Reviewed-by: Stephen Boyd <swboyd@chromium.org> >
Hi, On 3/5/2021 11:12 AM, Rajendra Nayak wrote: > > On 3/4/2021 5:34 AM, Stephen Boyd wrote: >> Quoting Rajendra Nayak (2021-03-03 04:17:49) >>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi >>> b/arch/arm64/boot/dts/qcom/sc7280.dtsi >>> index 4a56d9c..21c2399 100644 >>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi >>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi >>> @@ -30,6 +31,18 @@ >>> }; >>> }; >>> + reserved_memory: reserved-memory { >> >> Do we plan to use this label at any point? I'd prefer we remove this >> until it becomes useful. > > sure, i'll drop it > >> >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + ranges; >>> + >>> + aop_cmd_db_mem: memory@80860000 { >>> + reg = <0x0 0x80860000 0x0 0x20000>; >>> + compatible = "qcom,cmd-db"; >>> + no-map; >>> + }; >>> + }; >>> + >>> cpus { >>> #address-cells = <2>; >>> #size-cells = <0>; >>> @@ -203,6 +229,7 @@ >>> interrupt-controller; >>> #interrupt-cells = <2>; >>> gpio-ranges = <&tlmm 0 0 175>; >>> + wakeup-parent = <&pdc>; >>> qup_uart5_default: qup-uart5-default { >>> pins = "gpio46", "gpio47"; >>> @@ -287,6 +314,23 @@ >>> status = "disabled"; >>> }; >>> }; >>> + >>> + apps_rsc: rsc@18200000 { >> >> Any better name than 'rsc'? Maybe 'power-controller'? > > hmm, Maulik, any thoughts? This would perhaps need the bindings docs > to be updated as well (and maybe the existing platform DTs using rsc too) I think we should be good with rsc (resource-state-coordinator). RSC itself don't do any resource power management. Thanks, Maulik > >> >>> + compatible = "qcom,rpmh-rsc"; >>> + reg = <0 0x18200000 0 0x10000>, >>> + <0 0x18210000 0 0x10000>, >>> + <0 0x18220000 0 0x10000>; >>> + reg-names = "drv-0", "drv-1", "drv-2"; >>> + interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>, >>> + <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>, >>> + <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; >>> + qcom,tcs-offset = <0xd00>; >>> + qcom,drv-id = <2>; >>> + qcom,tcs-config = <ACTIVE_TCS 2>, >>> + <SLEEP_TCS 3>, >>> + <WAKE_TCS 3>, >>> + <CONTROL_TCS 1>; >>> + }; >>> }; >
On 3/4/2021 5:37 AM, Stephen Boyd wrote: > Quoting Rajendra Nayak (2021-03-03 04:17:47) >> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> new file mode 100644 >> index 0000000..4a56d9c >> --- /dev/null >> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> @@ -0,0 +1,299 @@ >> +// SPDX-License-Identifier: BSD-3-Clause >> +/* >> + * sc7280 SoC device tree source >> + * >> + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved. >> + */ >> + >> +#include <dt-bindings/clock/qcom,gcc-sc7280.h> >> +#include <dt-bindings/interrupt-controller/arm-gic.h> >> + >> +/ { >> + interrupt-parent = <&intc>; >> + >> + #address-cells = <2>; >> + #size-cells = <2>; >> + >> + chosen { }; >> + >> + clocks { >> + xo_board: xo-board { >> + compatible = "fixed-clock"; >> + clock-frequency = <76800000>; > > If this is the correct frequency I think we need to update the rpmh clk > driver to use the correct divider? Right now I think it is a 2 when it > should be 4? Looks like this is fixed now [1] [1] https://lore.kernel.org/patchwork/patch/1393159/ > >> + #clock-cells = <0>; >> + }; >> + >> + sleep_clk: sleep-clk { >> + compatible = "fixed-clock"; >> + clock-frequency = <32000>; >> + #clock-cells = <0>; >> + }; >> + };
Quoting Maulik Shah (2021-03-07 21:21:04) > Hi, > > On 3/5/2021 11:12 AM, Rajendra Nayak wrote: > > > > On 3/4/2021 5:34 AM, Stephen Boyd wrote: > >> Quoting Rajendra Nayak (2021-03-03 04:17:49) > >>> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi > >>> b/arch/arm64/boot/dts/qcom/sc7280.dtsi > >>> index 4a56d9c..21c2399 100644 > >>> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > >>> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > >>> @@ -30,6 +31,18 @@ > >>> }; > >>> }; > >>> + reserved_memory: reserved-memory { > >> > >> Do we plan to use this label at any point? I'd prefer we remove this > >> until it becomes useful. > > > > sure, i'll drop it > > > >> > >>> + #address-cells = <2>; > >>> + #size-cells = <2>; > >>> + ranges; > >>> + > >>> + aop_cmd_db_mem: memory@80860000 { > >>> + reg = <0x0 0x80860000 0x0 0x20000>; > >>> + compatible = "qcom,cmd-db"; > >>> + no-map; > >>> + }; > >>> + }; > >>> + > >>> cpus { > >>> #address-cells = <2>; > >>> #size-cells = <0>; > >>> @@ -203,6 +229,7 @@ > >>> interrupt-controller; > >>> #interrupt-cells = <2>; > >>> gpio-ranges = <&tlmm 0 0 175>; > >>> + wakeup-parent = <&pdc>; > >>> qup_uart5_default: qup-uart5-default { > >>> pins = "gpio46", "gpio47"; > >>> @@ -287,6 +314,23 @@ > >>> status = "disabled"; > >>> }; > >>> }; > >>> + > >>> + apps_rsc: rsc@18200000 { > >> > >> Any better name than 'rsc'? Maybe 'power-controller'? > > > > hmm, Maulik, any thoughts? This would perhaps need the bindings docs > > to be updated as well (and maybe the existing platform DTs using rsc too) > > I think we should be good with rsc (resource-state-coordinator). RSC > itself don't do any resource power management. > Maybe 'mailbox' then? Or 'remoteproc'? I am not "good" with rsc as it isn't part of the standardized nodes names per the DT spec.