* [PATCH 0/3] arm64: meson: add support for A1 Power Domains
@ 2019-09-19 12:11 Jianxin Pan
2019-09-19 12:11 ` [PATCH 1/3] dt-bindings: power: add Amlogic secure power domains bindings Jianxin Pan
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Jianxin Pan @ 2019-09-19 12:11 UTC (permalink / raw)
To: Kevin Hilman, linux-amlogic
Cc: Jianxin Pan, Rob Herring, Neil Armstrong, Jerome Brunet,
Martin Blumenstingl, linux-pm, linux-kernel, linux-arm-kernel,
devicetree, Jian Hu, Hanjie Lin, Victor Wan, Xingyu Chen
This patchset introduces a "Secure Power Doamin Controller". In A1/C1, power
controller registers such as PWRCTRL_FOCRSTN, PWRCTRL_PWR_OFF, PWRCTRL_MEM_PD
and PWRCTRL_ISO_EN, are in the secure domain, and should be accessed from ATF
by smc.
This patchset is based on A1 DTB series at [0].
[0] https://lore.kernel.org/linux-amlogic/1568276370-54181-1-git-send-email-jianxin.pan@amlogic.com
Jianxin Pan (3):
dt-bindings: power: add Amlogic secure power domains bindings
soc: amlogic: Add support for Secure power domains controller
arm64: dts: meson: a1: add secure power domain controller
.../bindings/power/amlogic,meson-sec-pwrc.yaml | 32 ++++
arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 6 +
drivers/soc/amlogic/Kconfig | 13 ++
drivers/soc/amlogic/Makefile | 1 +
drivers/soc/amlogic/meson-secure-pwrc.c | 182 +++++++++++++++++++++
include/dt-bindings/power/meson-a1-power.h | 32 ++++
6 files changed, 266 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml
create mode 100644 drivers/soc/amlogic/meson-secure-pwrc.c
create mode 100644 include/dt-bindings/power/meson-a1-power.h
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/3] dt-bindings: power: add Amlogic secure power domains bindings 2019-09-19 12:11 [PATCH 0/3] arm64: meson: add support for A1 Power Domains Jianxin Pan @ 2019-09-19 12:11 ` Jianxin Pan 2019-09-19 20:06 ` Martin Blumenstingl 2019-10-01 22:09 ` Rob Herring 2019-09-19 12:11 ` [PATCH 2/3] soc: amlogic: Add support for Secure power domains controller Jianxin Pan 2019-09-19 12:11 ` [PATCH 3/3] arm64: dts: meson: a1: add secure power domain controller Jianxin Pan 2 siblings, 2 replies; 13+ messages in thread From: Jianxin Pan @ 2019-09-19 12:11 UTC (permalink / raw) To: Kevin Hilman, linux-amlogic Cc: Jianxin Pan, Zhiqiang Liang, Rob Herring, Neil Armstrong, Jerome Brunet, Martin Blumenstingl, linux-pm, linux-kernel, linux-arm-kernel, devicetree, Jian Hu, Hanjie Lin, Victor Wan, Xingyu Chen Add the bindings for the Amlogic Secure power domains, controlling the secure power domains. The bindings targets the Amlogic A1 and C1 compatible SoCs, in which the power domain registers are in secure world. Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com> Signed-off-by: Zhiqiang Liang <zhiqiang.liang@amlogic.com> --- .../bindings/power/amlogic,meson-sec-pwrc.yaml | 32 ++++++++++++++++++++++ include/dt-bindings/power/meson-a1-power.h | 32 ++++++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml create mode 100644 include/dt-bindings/power/meson-a1-power.h diff --git a/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml b/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml new file mode 100644 index 00000000..327e0d9 --- /dev/null +++ b/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml @@ -0,0 +1,32 @@ +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) +# Copyright (c) 2019 Amlogic, Inc +# Author: Jianxin Pan <jianxin.pan@amlogic.com> +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/power/amlogic,meson-sec-pwrc.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Amlogic Meson Secure Power Domains + +maintainers: + - Jianxin Pan <jianxin.pan@amlogic.com> + +description: |+ + A1/C1 series The Secure Power Domains node should be the child of a syscon + node with the required property. + +properties: + compatible: + enum: + - amlogic,meson-a1-pwrc + +required: + - compatible + +examples: + - | + pwrc: power-controller { + compatible = "amlogic,meson-a1-pwrc"; + }; + + diff --git a/include/dt-bindings/power/meson-a1-power.h b/include/dt-bindings/power/meson-a1-power.h new file mode 100644 index 00000000..6cf50bf --- /dev/null +++ b/include/dt-bindings/power/meson-a1-power.h @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: (GPL-2.0+ or MIT) */ +/* + * Copyright (c) 2019 Amlogic, Inc. + * Author: Jianxin Pan <jianxin.pan@amlogic.com> + */ + +#ifndef _DT_BINDINGS_MESON_A1_POWER_H +#define _DT_BINDINGS_MESON_A1_POWER_H + +#define PWRC_DSPA_ID 8 +#define PWRC_DSPB_ID 9 +#define PWRC_UART_ID 10 +#define PWRC_DMC_ID 11 +#define PWRC_I2C_ID 12 +#define PWRC_PSRAM_ID 13 +#define PWRC_ACODEC_ID 14 +#define PWRC_AUDIO_ID 15 +#define PWRC_OTP_ID 16 +#define PWRC_DMA_ID 17 +#define PWRC_SD_EMMC_ID 18 +#define PWRC_RAMA_ID 19 +#define PWRC_RAMB_ID 20 +#define PWRC_IR_ID 21 +#define PWRC_SPICC_ID 22 +#define PWRC_SPIFC_ID 23 +#define PWRC_USB_ID 24 +#define PWRC_NIC_ID 25 +#define PWRC_PDMIN_ID 26 +#define PWRC_RSA_ID 27 +#define PWRC_MAX_ID 28 + +#endif -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] dt-bindings: power: add Amlogic secure power domains bindings 2019-09-19 12:11 ` [PATCH 1/3] dt-bindings: power: add Amlogic secure power domains bindings Jianxin Pan @ 2019-09-19 20:06 ` Martin Blumenstingl 2019-09-20 7:37 ` Jianxin Pan 2019-10-01 22:09 ` Rob Herring 1 sibling, 1 reply; 13+ messages in thread From: Martin Blumenstingl @ 2019-09-19 20:06 UTC (permalink / raw) To: Jianxin Pan Cc: Kevin Hilman, linux-amlogic, Zhiqiang Liang, Rob Herring, Neil Armstrong, Jerome Brunet, linux-pm, linux-kernel, linux-arm-kernel, devicetree, Jian Hu, Hanjie Lin, Victor Wan, Xingyu Chen Hi Jianxin, On Thu, Sep 19, 2019 at 2:11 PM Jianxin Pan <jianxin.pan@amlogic.com> wrote: > > Add the bindings for the Amlogic Secure power domains, controlling the > secure power domains. > > The bindings targets the Amlogic A1 and C1 compatible SoCs, in which the > power domain registers are in secure world. > > Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com> > Signed-off-by: Zhiqiang Liang <zhiqiang.liang@amlogic.com> > --- > .../bindings/power/amlogic,meson-sec-pwrc.yaml | 32 ++++++++++++++++++++++ > include/dt-bindings/power/meson-a1-power.h | 32 ++++++++++++++++++++++ > 2 files changed, 64 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml > create mode 100644 include/dt-bindings/power/meson-a1-power.h > > diff --git a/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml b/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml > new file mode 100644 > index 00000000..327e0d9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml > @@ -0,0 +1,32 @@ > +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +# Copyright (c) 2019 Amlogic, Inc > +# Author: Jianxin Pan <jianxin.pan@amlogic.com> > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/power/amlogic,meson-sec-pwrc.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: Amlogic Meson Secure Power Domains > + > +maintainers: > + - Jianxin Pan <jianxin.pan@amlogic.com> > + > +description: |+ > + A1/C1 series The Secure Power Domains node should be the child of a syscon > + node with the required property. > + > +properties: > + compatible: > + enum: > + - amlogic,meson-a1-pwrc > + > +required: > + - compatible > + > +examples: > + - | > + pwrc: power-controller { > + compatible = "amlogic,meson-a1-pwrc"; > + }; not a comment about this binding but about the secure monitor in general: there's a recent discussion about the secure monitor in the nvmem bindings: [0] Martin [0] https://www.spinics.net/lists/arm-kernel/msg750010.html ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] dt-bindings: power: add Amlogic secure power domains bindings 2019-09-19 20:06 ` Martin Blumenstingl @ 2019-09-20 7:37 ` Jianxin Pan 0 siblings, 0 replies; 13+ messages in thread From: Jianxin Pan @ 2019-09-20 7:37 UTC (permalink / raw) To: Martin Blumenstingl Cc: Kevin Hilman, linux-amlogic, Zhiqiang Liang, Rob Herring, Neil Armstrong, Jerome Brunet, linux-pm, linux-kernel, linux-arm-kernel, devicetree, Jian Hu, Hanjie Lin, Victor Wan, Xingyu Chen Hi Martin, On 2019/9/20 4:06, Martin Blumenstingl wrote: > Hi Jianxin, > > On Thu, Sep 19, 2019 at 2:11 PM Jianxin Pan <jianxin.pan@amlogic.com> wrote: >> >> Add the bindings for the Amlogic Secure power domains, controlling the >> secure power domains. >> >> The bindings targets the Amlogic A1 and C1 compatible SoCs, in which the >> power domain registers are in secure world. >> >> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com> >> Signed-off-by: Zhiqiang Liang <zhiqiang.liang@amlogic.com> >> --- >> .../bindings/power/amlogic,meson-sec-pwrc.yaml | 32 ++++++++++++++++++++++ >> include/dt-bindings/power/meson-a1-power.h | 32 ++++++++++++++++++++++ >> 2 files changed, 64 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml >> create mode 100644 include/dt-bindings/power/meson-a1-power.h >> >> diff --git a/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml b/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml >> new file mode 100644 >> index 00000000..327e0d9 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml >> @@ -0,0 +1,32 @@ >> +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) [...]>> +examples: >> + - | >> + pwrc: power-controller { >> + compatible = "amlogic,meson-a1-pwrc"; >> + }; > not a comment about this binding but about the secure monitor in general: > there's a recent discussion about the secure monitor in the nvmem bindings: [0] > > > Martin > > > [0] https://www.spinics.net/lists/arm-kernel/msg750010.html > Thanks for your remind,I will update and send the second version after Rework of secure-monitor driver is applied. > . > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] dt-bindings: power: add Amlogic secure power domains bindings 2019-09-19 12:11 ` [PATCH 1/3] dt-bindings: power: add Amlogic secure power domains bindings Jianxin Pan 2019-09-19 20:06 ` Martin Blumenstingl @ 2019-10-01 22:09 ` Rob Herring 2019-10-10 3:31 ` Jianxin Pan 1 sibling, 1 reply; 13+ messages in thread From: Rob Herring @ 2019-10-01 22:09 UTC (permalink / raw) To: Jianxin Pan Cc: Kevin Hilman, linux-amlogic, Zhiqiang Liang, Neil Armstrong, Jerome Brunet, Martin Blumenstingl, linux-pm, linux-kernel, linux-arm-kernel, devicetree, Jian Hu, Hanjie Lin, Victor Wan, Xingyu Chen On Thu, Sep 19, 2019 at 08:11:02AM -0400, Jianxin Pan wrote: > Add the bindings for the Amlogic Secure power domains, controlling the > secure power domains. > > The bindings targets the Amlogic A1 and C1 compatible SoCs, in which the > power domain registers are in secure world. > > Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com> > Signed-off-by: Zhiqiang Liang <zhiqiang.liang@amlogic.com> > --- > .../bindings/power/amlogic,meson-sec-pwrc.yaml | 32 ++++++++++++++++++++++ > include/dt-bindings/power/meson-a1-power.h | 32 ++++++++++++++++++++++ > 2 files changed, 64 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml > create mode 100644 include/dt-bindings/power/meson-a1-power.h > > diff --git a/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml b/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml > new file mode 100644 > index 00000000..327e0d9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml > @@ -0,0 +1,32 @@ > +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +# Copyright (c) 2019 Amlogic, Inc > +# Author: Jianxin Pan <jianxin.pan@amlogic.com> > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/power/amlogic,meson-sec-pwrc.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: Amlogic Meson Secure Power Domains > + > +maintainers: > + - Jianxin Pan <jianxin.pan@amlogic.com> > + > +description: |+ > + A1/C1 series The Secure Power Domains node should be the child of a syscon > + node with the required property. 'a syscon node' is not specific enough. It must be a specific node. > + > +properties: > + compatible: > + enum: > + - amlogic,meson-a1-pwrc > + > +required: > + - compatible > + > +examples: > + - | > + pwrc: power-controller { > + compatible = "amlogic,meson-a1-pwrc"; But why do you need this node? It has no resources. #power-domain-cells needed? > + }; > + > + > diff --git a/include/dt-bindings/power/meson-a1-power.h b/include/dt-bindings/power/meson-a1-power.h > new file mode 100644 > index 00000000..6cf50bf > --- /dev/null > +++ b/include/dt-bindings/power/meson-a1-power.h > @@ -0,0 +1,32 @@ > +/* SPDX-License-Identifier: (GPL-2.0+ or MIT) */ > +/* > + * Copyright (c) 2019 Amlogic, Inc. > + * Author: Jianxin Pan <jianxin.pan@amlogic.com> > + */ > + > +#ifndef _DT_BINDINGS_MESON_A1_POWER_H > +#define _DT_BINDINGS_MESON_A1_POWER_H > + > +#define PWRC_DSPA_ID 8 > +#define PWRC_DSPB_ID 9 > +#define PWRC_UART_ID 10 > +#define PWRC_DMC_ID 11 > +#define PWRC_I2C_ID 12 > +#define PWRC_PSRAM_ID 13 > +#define PWRC_ACODEC_ID 14 > +#define PWRC_AUDIO_ID 15 > +#define PWRC_OTP_ID 16 > +#define PWRC_DMA_ID 17 > +#define PWRC_SD_EMMC_ID 18 > +#define PWRC_RAMA_ID 19 > +#define PWRC_RAMB_ID 20 > +#define PWRC_IR_ID 21 > +#define PWRC_SPICC_ID 22 > +#define PWRC_SPIFC_ID 23 > +#define PWRC_USB_ID 24 > +#define PWRC_NIC_ID 25 > +#define PWRC_PDMIN_ID 26 > +#define PWRC_RSA_ID 27 > +#define PWRC_MAX_ID 28 > + > +#endif > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] dt-bindings: power: add Amlogic secure power domains bindings 2019-10-01 22:09 ` Rob Herring @ 2019-10-10 3:31 ` Jianxin Pan 0 siblings, 0 replies; 13+ messages in thread From: Jianxin Pan @ 2019-10-10 3:31 UTC (permalink / raw) To: Rob Herring Cc: Kevin Hilman, linux-amlogic, Zhiqiang Liang, Neil Armstrong, Jerome Brunet, Martin Blumenstingl, linux-pm, linux-kernel, linux-arm-kernel, devicetree, Jian Hu, Hanjie Lin, Victor Wan, Xingyu Chen Hi Rob， Thanks for your review. I'm sorry to reply so late, for I've been on vacation in the last week. On 2019/10/2 6:09, Rob Herring wrote: > On Thu, Sep 19, 2019 at 08:11:02AM -0400, Jianxin Pan wrote: >> Add the bindings for the Amlogic Secure power domains, controlling the >> secure power domains. >> >> The bindings targets the Amlogic A1 and C1 compatible SoCs, in which the >> power domain registers are in secure world. >> >> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com> >> Signed-off-by: Zhiqiang Liang <zhiqiang.liang@amlogic.com> >> --- >> .../bindings/power/amlogic,meson-sec-pwrc.yaml | 32 ++++++++++++++++++++++ >> include/dt-bindings/power/meson-a1-power.h | 32 ++++++++++++++++++++++ >> 2 files changed, 64 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml >> create mode 100644 include/dt-bindings/power/meson-a1-power.h >> >> diff --git a/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml b/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml >> new file mode 100644 >> index 00000000..327e0d9 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/power/amlogic,meson-sec-pwrc.yaml >> @@ -0,0 +1,32 @@ >> +# SPDX-License-Identifier: (GPL-2.0+ OR MIT) >> +# Copyright (c) 2019 Amlogic, Inc >> +# Author: Jianxin Pan <jianxin.pan@amlogic.com> >> +%YAML 1.2 >> +--- >> +$id: "http://devicetree.org/schemas/power/amlogic,meson-sec-pwrc.yaml#" >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" >> + >> +title: Amlogic Meson Secure Power Domains >> + >> +maintainers: >> + - Jianxin Pan <jianxin.pan@amlogic.com> >> + >> +description: |+ >> + A1/C1 series The Secure Power Domains node should be the child of a syscon >> + node with the required property. > > 'a syscon node' is not specific enough. It must be a specific node. > I will fix this. In A1/C1, power control is in secure domain, and syscon parent is not needed. >> + >> +properties: >> + compatible: >> + enum: >> + - amlogic,meson-a1-pwrc >> + >> +required: >> + - compatible >> + >> +examples: >> + - | >> + pwrc: power-controller { >> + compatible = "amlogic,meson-a1-pwrc"; > > But why do you need this node? It has no resources. > > #power-domain-cells needed? I will add #power-domain-cells and secure-monitor here. Thank you for the review. > >> + }; >> + >> + >> diff --git a/include/dt-bindings/power/meson-a1-power.h b/include/dt-bindings/power/meson-a1-power.h >> new file mode 100644 >> index 00000000..6cf50bf >> --- /dev/null >> +++ b/include/dt-bindings/power/meson-a1-power.h >> @@ -0,0 +1,32 @@ >> +/* SPDX-License-Identifier: (GPL-2.0+ or MIT) */ >> +/* >> + * Copyright (c) 2019 Amlogic, Inc. >> + * Author: Jianxin Pan <jianxin.pan@amlogic.com> >> + */ >> + >> +#ifndef _DT_BINDINGS_MESON_A1_POWER_H >> +#define _DT_BINDINGS_MESON_A1_POWER_H >> + >> +#define PWRC_DSPA_ID 8 >> +#define PWRC_DSPB_ID 9 >> +#define PWRC_UART_ID 10 >> +#define PWRC_DMC_ID 11 >> +#define PWRC_I2C_ID 12 >> +#define PWRC_PSRAM_ID 13 >> +#define PWRC_ACODEC_ID 14 >> +#define PWRC_AUDIO_ID 15 >> +#define PWRC_OTP_ID 16 >> +#define PWRC_DMA_ID 17 >> +#define PWRC_SD_EMMC_ID 18 >> +#define PWRC_RAMA_ID 19 >> +#define PWRC_RAMB_ID 20 >> +#define PWRC_IR_ID 21 >> +#define PWRC_SPICC_ID 22 >> +#define PWRC_SPIFC_ID 23 >> +#define PWRC_USB_ID 24 >> +#define PWRC_NIC_ID 25 >> +#define PWRC_PDMIN_ID 26 >> +#define PWRC_RSA_ID 27 >> +#define PWRC_MAX_ID 28 >> + >> +#endif >> -- >> 2.7.4 >> > > . > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] soc: amlogic: Add support for Secure power domains controller 2019-09-19 12:11 [PATCH 0/3] arm64: meson: add support for A1 Power Domains Jianxin Pan 2019-09-19 12:11 ` [PATCH 1/3] dt-bindings: power: add Amlogic secure power domains bindings Jianxin Pan @ 2019-09-19 12:11 ` Jianxin Pan 2019-09-19 20:03 ` Martin Blumenstingl 2019-09-25 22:41 ` Kevin Hilman 2019-09-19 12:11 ` [PATCH 3/3] arm64: dts: meson: a1: add secure power domain controller Jianxin Pan 2 siblings, 2 replies; 13+ messages in thread From: Jianxin Pan @ 2019-09-19 12:11 UTC (permalink / raw) To: Kevin Hilman, linux-amlogic Cc: Jianxin Pan, Zhiqiang Liang, Rob Herring, Neil Armstrong, Jerome Brunet, Martin Blumenstingl, linux-pm, linux-kernel, linux-arm-kernel, devicetree, Jian Hu, Hanjie Lin, Victor Wan, Xingyu Chen Add support for the Amlogic Secure Power controller. In A1/C1 series, power control registers are in secure domain, and should be accessed by smc. Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com> Signed-off-by: Zhiqiang Liang <zhiqiang.liang@amlogic.com> --- drivers/soc/amlogic/Kconfig | 13 +++ drivers/soc/amlogic/Makefile | 1 + drivers/soc/amlogic/meson-secure-pwrc.c | 182 ++++++++++++++++++++++++++++++++ 3 files changed, 196 insertions(+) create mode 100644 drivers/soc/amlogic/meson-secure-pwrc.c diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig index bc2c912..6cb06e7 100644 --- a/drivers/soc/amlogic/Kconfig +++ b/drivers/soc/amlogic/Kconfig @@ -48,6 +48,19 @@ config MESON_EE_PM_DOMAINS Say yes to expose Amlogic Meson Everything-Else Power Domains as Generic Power Domains. +config MESON_SECURE_PM_DOMAINS + bool "Amlogic Meson Secure Power Domains driver" + depends on ARCH_MESON || COMPILE_TEST + depends on PM && OF + depends on HAVE_ARM_SMCCC + default ARCH_MESON + select PM_GENERIC_DOMAINS + select PM_GENERIC_DOMAINS_OF + help + Support for the power controller on Amlogic A1/C1 series. + Say yes to expose Amlogic Meson Secure Power Domains as Generic + Power Domains. + config MESON_MX_SOCINFO bool "Amlogic Meson MX SoC Information driver" depends on ARCH_MESON || COMPILE_TEST diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile index de79d044..7b8c5d3 100644 --- a/drivers/soc/amlogic/Makefile +++ b/drivers/soc/amlogic/Makefile @@ -5,3 +5,4 @@ obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o obj-$(CONFIG_MESON_GX_PM_DOMAINS) += meson-gx-pwrc-vpu.o obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o obj-$(CONFIG_MESON_EE_PM_DOMAINS) += meson-ee-pwrc.o +obj-$(CONFIG_MESON_SECURE_PM_DOMAINS) += meson-secure-pwrc.o diff --git a/drivers/soc/amlogic/meson-secure-pwrc.c b/drivers/soc/amlogic/meson-secure-pwrc.c new file mode 100644 index 00000000..00c7232 --- /dev/null +++ b/drivers/soc/amlogic/meson-secure-pwrc.c @@ -0,0 +1,182 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright (c) 2019 Amlogic, Inc. + * Author: Jianxin Pan <jianxin.pan@amlogic.com> + */ +#include <linux/io.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/pm_domain.h> +#include <dt-bindings/power/meson-a1-power.h> +#include <linux/arm-smccc.h> + +#define PWRC_ON 1 +#define PWRC_OFF 0 +#define SMC_PWRC_SET 0x82000093 +#define SMC_PWRC_GET 0x82000095 + +struct meson_secure_pwrc_domain { + struct generic_pm_domain base; + unsigned int index; +}; + +struct meson_secure_pwrc { + struct meson_secure_pwrc_domain *domains; + struct genpd_onecell_data xlate; +}; + +struct meson_secure_pwrc_domain_desc { + unsigned int index; + unsigned int flags; + char *name; + bool (*get_power)(struct meson_secure_pwrc_domain *pwrc_domain); +}; + +struct meson_secure_pwrc_domain_data { + unsigned int count; + struct meson_secure_pwrc_domain_desc *domains; +}; + +static bool pwrc_secure_get_power(struct meson_secure_pwrc_domain *pwrc_domain) +{ + struct arm_smccc_res res; + + arm_smccc_smc(SMC_PWRC_GET, pwrc_domain->index, 0, + 0, 0, 0, 0, 0, &res); + + return res.a0 & 0x1; +} + +static int meson_secure_pwrc_off(struct generic_pm_domain *domain) +{ + struct arm_smccc_res res; + struct meson_secure_pwrc_domain *pwrc_domain = + container_of(domain, struct meson_secure_pwrc_domain, base); + + arm_smccc_smc(SMC_PWRC_SET, pwrc_domain->index, PWRC_OFF, + 0, 0, 0, 0, 0, &res); + + return 0; +} + +static int meson_secure_pwrc_on(struct generic_pm_domain *domain) +{ + struct arm_smccc_res res; + struct meson_secure_pwrc_domain *pwrc_domain = + container_of(domain, struct meson_secure_pwrc_domain, base); + + arm_smccc_smc(SMC_PWRC_SET, pwrc_domain->index, PWRC_ON, + 0, 0, 0, 0, 0, &res); + + return 0; +} + +#define SEC_PD(__name, __flag) \ +{ \ + .name = #__name, \ + .index = PWRC_##__name##_ID, \ + .get_power = pwrc_secure_get_power, \ + .flags = __flag, \ +} + +static struct meson_secure_pwrc_domain_desc a1_pwrc_domains[] = { + SEC_PD(DSPA, 0), + SEC_PD(DSPB, 0), + SEC_PD(UART, GENPD_FLAG_ALWAYS_ON), + SEC_PD(DMC, GENPD_FLAG_ALWAYS_ON), + SEC_PD(I2C, 0), + SEC_PD(PSRAM, 0), + SEC_PD(ACODEC, 0), + SEC_PD(AUDIO, 0), + SEC_PD(OTP, 0), + SEC_PD(DMA, 0), + SEC_PD(SD_EMMC, 0), + SEC_PD(RAMA, 0), + SEC_PD(RAMB, GENPD_FLAG_ALWAYS_ON), + SEC_PD(IR, 0), + SEC_PD(SPICC, 0), + SEC_PD(SPIFC, 0), + SEC_PD(USB, 0), + SEC_PD(NIC, GENPD_FLAG_ALWAYS_ON), + SEC_PD(PDMIN, 0), + SEC_PD(RSA, 0), +}; + +static int meson_secure_pwrc_probe(struct platform_device *pdev) +{ + const struct meson_secure_pwrc_domain_data *match; + struct meson_secure_pwrc *pwrc; + int i; + + match = of_device_get_match_data(&pdev->dev); + if (!match) { + dev_err(&pdev->dev, "failed to get match data\n"); + return -ENODEV; + } + + pwrc = devm_kzalloc(&pdev->dev, sizeof(*pwrc), GFP_KERNEL); + if (!pwrc) + return -ENOMEM; + + pwrc->xlate.domains = devm_kcalloc(&pdev->dev, match->count, + sizeof(*pwrc->xlate.domains), + GFP_KERNEL); + if (!pwrc->xlate.domains) + return -ENOMEM; + + pwrc->domains = devm_kcalloc(&pdev->dev, match->count, + sizeof(*pwrc->domains), GFP_KERNEL); + if (!pwrc->domains) + return -ENOMEM; + + pwrc->xlate.num_domains = match->count; + platform_set_drvdata(pdev, pwrc); + + for (i = 0 ; i < match->count ; ++i) { + struct meson_secure_pwrc_domain *dom = &pwrc->domains[i]; + + if (!match->domains[i].index) + continue; + + dom->index = match->domains[i].index; + dom->base.name = match->domains[i].name; + dom->base.flags = match->domains[i].flags; + dom->base.power_on = meson_secure_pwrc_on; + dom->base.power_off = meson_secure_pwrc_off; + + pm_genpd_init(&dom->base, NULL, + (match->domains[i].get_power ? + match->domains[i].get_power(dom) : true)); + + pwrc->xlate.domains[i] = &dom->base; + } + + return of_genpd_add_provider_onecell(pdev->dev.of_node, &pwrc->xlate); +} + +static struct meson_secure_pwrc_domain_data meson_secure_a1_pwrc_data = { + .domains = a1_pwrc_domains, + .count = ARRAY_SIZE(a1_pwrc_domains), +}; + +static const struct of_device_id meson_secure_pwrc_match_table[] = { + { + .compatible = "amlogic,meson-a1-pwrc", + .data = &meson_secure_a1_pwrc_data, + }, + { } +}; + +static struct platform_driver meson_secure_pwrc_driver = { + .probe = meson_secure_pwrc_probe, + .driver = { + .name = "meson_secure_pwrc", + .of_match_table = meson_secure_pwrc_match_table, + }, +}; + +static int meson_secure_pwrc_init(void) +{ + return platform_driver_register(&meson_secure_pwrc_driver); +} +arch_initcall_sync(meson_secure_pwrc_init); -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] soc: amlogic: Add support for Secure power domains controller 2019-09-19 12:11 ` [PATCH 2/3] soc: amlogic: Add support for Secure power domains controller Jianxin Pan @ 2019-09-19 20:03 ` Martin Blumenstingl 2019-09-20 7:58 ` Jianxin Pan 2019-09-25 22:41 ` Kevin Hilman 1 sibling, 1 reply; 13+ messages in thread From: Martin Blumenstingl @ 2019-09-19 20:03 UTC (permalink / raw) To: Jianxin Pan Cc: Kevin Hilman, linux-amlogic, Zhiqiang Liang, Rob Herring, Neil Armstrong, Jerome Brunet, linux-pm, linux-kernel, linux-arm-kernel, devicetree, Jian Hu, Hanjie Lin, Victor Wan, Xingyu Chen Hi Jianxin, I added three comments below from a quick glance at this driver (I didn't have time for a complete review) On Thu, Sep 19, 2019 at 2:11 PM Jianxin Pan <jianxin.pan@amlogic.com> wrote: [...] > + pm_genpd_init(&dom->base, NULL, > + (match->domains[i].get_power ? > + match->domains[i].get_power(dom) : true)); .get_power is never NULL in this driver so the ": true" part is effectively a no-op [...] > +static const struct of_device_id meson_secure_pwrc_match_table[] = { > + { > + .compatible = "amlogic,meson-a1-pwrc", > + .data = &meson_secure_a1_pwrc_data, > + }, > + { } many drivers use a /* sentinel */ comment inside { } [...] > +arch_initcall_sync(meson_secure_pwrc_init); why arch_initcall_sync instead of builtin_platform_driver? $ grep -R arch_initcall_sync drivers/soc/ $ Martin ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] soc: amlogic: Add support for Secure power domains controller 2019-09-19 20:03 ` Martin Blumenstingl @ 2019-09-20 7:58 ` Jianxin Pan 0 siblings, 0 replies; 13+ messages in thread From: Jianxin Pan @ 2019-09-20 7:58 UTC (permalink / raw) To: Martin Blumenstingl Cc: Kevin Hilman, linux-amlogic, Zhiqiang Liang, Rob Herring, Neil Armstrong, Jerome Brunet, linux-pm, linux-kernel, linux-arm-kernel, devicetree, Jian Hu, Hanjie Lin, Victor Wan, Xingyu Chen Hi Martin, On 2019/9/20 4:03, Martin Blumenstingl wrote: > Hi Jianxin, > > I added three comments below from a quick glance at this driver (I > didn't have time for a complete review) > > On Thu, Sep 19, 2019 at 2:11 PM Jianxin Pan <jianxin.pan@amlogic.com> wrote: > [...] >> + pm_genpd_init(&dom->base, NULL, >> + (match->domains[i].get_power ? >> + match->domains[i].get_power(dom) : true)); > .get_power is never NULL in this driver so the ": true" part is > effectively a no-op > OK, I will remove it. Thanks for your time. > [...] >> +static const struct of_device_id meson_secure_pwrc_match_table[] = { >> + { >> + .compatible = "amlogic,meson-a1-pwrc", >> + .data = &meson_secure_a1_pwrc_data, >> + }, >> + { } > many drivers use a /* sentinel */ comment inside { } > OK, I will add this comment line. > [...] >> +arch_initcall_sync(meson_secure_pwrc_init); > why arch_initcall_sync instead of builtin_platform_driver? > $ grep -R arch_initcall_sync drivers/soc/ > $ > > > Martin > The power-domain is depended by many other drivers, arch_initcall_sync is used to make power-domain probe earlier. Maybe I need to switch back to builtin_platform_driver when use APIs from meson_sm.c. > . > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] soc: amlogic: Add support for Secure power domains controller 2019-09-19 12:11 ` [PATCH 2/3] soc: amlogic: Add support for Secure power domains controller Jianxin Pan 2019-09-19 20:03 ` Martin Blumenstingl @ 2019-09-25 22:41 ` Kevin Hilman 2019-09-26 9:17 ` Jianxin Pan 1 sibling, 1 reply; 13+ messages in thread From: Kevin Hilman @ 2019-09-25 22:41 UTC (permalink / raw) To: Jianxin Pan, linux-amlogic Cc: Jianxin Pan, Zhiqiang Liang, Rob Herring, Neil Armstrong, Jerome Brunet, Martin Blumenstingl, linux-pm, linux-kernel, linux-arm-kernel, devicetree, Jian Hu, Hanjie Lin, Victor Wan, Xingyu Chen Hi Jianxin, Jianxin Pan <jianxin.pan@amlogic.com> writes: > Add support for the Amlogic Secure Power controller. In A1/C1 series, power > control registers are in secure domain, and should be accessed by smc. > > Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com> > Signed-off-by: Zhiqiang Liang <zhiqiang.liang@amlogic.com> Thanks for the new power domain driver. > --- > drivers/soc/amlogic/Kconfig | 13 +++ > drivers/soc/amlogic/Makefile | 1 + > drivers/soc/amlogic/meson-secure-pwrc.c | 182 ++++++++++++++++++++++++++++++++ > 3 files changed, 196 insertions(+) > create mode 100644 drivers/soc/amlogic/meson-secure-pwrc.c > > diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig > index bc2c912..6cb06e7 100644 > --- a/drivers/soc/amlogic/Kconfig > +++ b/drivers/soc/amlogic/Kconfig > @@ -48,6 +48,19 @@ config MESON_EE_PM_DOMAINS > Say yes to expose Amlogic Meson Everything-Else Power Domains as > Generic Power Domains. > > +config MESON_SECURE_PM_DOMAINS > + bool "Amlogic Meson Secure Power Domains driver" > + depends on ARCH_MESON || COMPILE_TEST > + depends on PM && OF > + depends on HAVE_ARM_SMCCC > + default ARCH_MESON > + select PM_GENERIC_DOMAINS > + select PM_GENERIC_DOMAINS_OF > + help > + Support for the power controller on Amlogic A1/C1 series. > + Say yes to expose Amlogic Meson Secure Power Domains as Generic > + Power Domains. > + > config MESON_MX_SOCINFO > bool "Amlogic Meson MX SoC Information driver" > depends on ARCH_MESON || COMPILE_TEST > diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile > index de79d044..7b8c5d3 100644 > --- a/drivers/soc/amlogic/Makefile > +++ b/drivers/soc/amlogic/Makefile > @@ -5,3 +5,4 @@ obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o > obj-$(CONFIG_MESON_GX_PM_DOMAINS) += meson-gx-pwrc-vpu.o > obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o > obj-$(CONFIG_MESON_EE_PM_DOMAINS) += meson-ee-pwrc.o > +obj-$(CONFIG_MESON_SECURE_PM_DOMAINS) += meson-secure-pwrc.o > diff --git a/drivers/soc/amlogic/meson-secure-pwrc.c b/drivers/soc/amlogic/meson-secure-pwrc.c > new file mode 100644 > index 00000000..00c7232 > --- /dev/null > +++ b/drivers/soc/amlogic/meson-secure-pwrc.c > @@ -0,0 +1,182 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright (c) 2019 Amlogic, Inc. > + * Author: Jianxin Pan <jianxin.pan@amlogic.com> > + */ > +#include <linux/io.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/pm_domain.h> > +#include <dt-bindings/power/meson-a1-power.h> > +#include <linux/arm-smccc.h> > + > +#define PWRC_ON 1 > +#define PWRC_OFF 0 > +#define SMC_PWRC_SET 0x82000093 > +#define SMC_PWRC_GET 0x82000095 > + > +struct meson_secure_pwrc_domain { > + struct generic_pm_domain base; > + unsigned int index; > +}; > + > +struct meson_secure_pwrc { > + struct meson_secure_pwrc_domain *domains; > + struct genpd_onecell_data xlate; > +}; > + > +struct meson_secure_pwrc_domain_desc { > + unsigned int index; > + unsigned int flags; > + char *name; > + bool (*get_power)(struct meson_secure_pwrc_domain *pwrc_domain); > +}; > + > +struct meson_secure_pwrc_domain_data { > + unsigned int count; > + struct meson_secure_pwrc_domain_desc *domains; > +}; > + > +static bool pwrc_secure_get_power(struct meson_secure_pwrc_domain *pwrc_domain) > +{ > + struct arm_smccc_res res; > + > + arm_smccc_smc(SMC_PWRC_GET, pwrc_domain->index, 0, > + 0, 0, 0, 0, 0, &res); > + > + return res.a0 & 0x1; Please use a #define with a readable name for this mask. > +} What does the return value for this function mean? Does true mean "powered off" or "powered on"? See the rename I just did on the ee-pwrc driver: https://lore.kernel.org/linux-amlogic/20190925213528.21515-2-khilman@kernel.org/ > +static int meson_secure_pwrc_off(struct generic_pm_domain *domain) > +{ > + struct arm_smccc_res res; > + struct meson_secure_pwrc_domain *pwrc_domain = > + container_of(domain, struct meson_secure_pwrc_domain, base); > + > + arm_smccc_smc(SMC_PWRC_SET, pwrc_domain->index, PWRC_OFF, > + 0, 0, 0, 0, 0, &res); > + > + return 0; > +} > + > +static int meson_secure_pwrc_on(struct generic_pm_domain *domain) > +{ > + struct arm_smccc_res res; > + struct meson_secure_pwrc_domain *pwrc_domain = > + container_of(domain, struct meson_secure_pwrc_domain, base); > + > + arm_smccc_smc(SMC_PWRC_SET, pwrc_domain->index, PWRC_ON, > + 0, 0, 0, 0, 0, &res); > + > + return 0; > +} > + > +#define SEC_PD(__name, __flag) \ > +{ \ > + .name = #__name, \ > + .index = PWRC_##__name##_ID, \ > + .get_power = pwrc_secure_get_power, \ > + .flags = __flag, \ > +} > + > +static struct meson_secure_pwrc_domain_desc a1_pwrc_domains[] = { > + SEC_PD(DSPA, 0), > + SEC_PD(DSPB, 0), > + SEC_PD(UART, GENPD_FLAG_ALWAYS_ON), This flag should only be used for domains where there are no linux drivers. Rather than using this flag, you need to add a 'power-domain' property to the uart driver in DT, and then update the meson_uart driver to use the runtime PM API so that the domain is enabled whenever the UART is in use. > + SEC_PD(DMC, GENPD_FLAG_ALWAYS_ON), Please explain the need for ALWAYS_ON. > + SEC_PD(I2C, 0), > + SEC_PD(PSRAM, 0), > + SEC_PD(ACODEC, 0), > + SEC_PD(AUDIO, 0), > + SEC_PD(OTP, 0), > + SEC_PD(DMA, 0), > + SEC_PD(SD_EMMC, 0), > + SEC_PD(RAMA, 0), > + SEC_PD(RAMB, GENPD_FLAG_ALWAYS_ON), Please explain the need for ALWAYS_ON. > + SEC_PD(IR, 0), > + SEC_PD(SPICC, 0), > + SEC_PD(SPIFC, 0), > + SEC_PD(USB, 0), > + SEC_PD(NIC, GENPD_FLAG_ALWAYS_ON), Please explain the need for ALWAYS_ON. > + SEC_PD(PDMIN, 0), > + SEC_PD(RSA, 0), > +}; > + > +static int meson_secure_pwrc_probe(struct platform_device *pdev) > +{ > + const struct meson_secure_pwrc_domain_data *match; > + struct meson_secure_pwrc *pwrc; > + int i; > + > + match = of_device_get_match_data(&pdev->dev); > + if (!match) { > + dev_err(&pdev->dev, "failed to get match data\n"); > + return -ENODEV; > + } > + > + pwrc = devm_kzalloc(&pdev->dev, sizeof(*pwrc), GFP_KERNEL); > + if (!pwrc) > + return -ENOMEM; > + > + pwrc->xlate.domains = devm_kcalloc(&pdev->dev, match->count, > + sizeof(*pwrc->xlate.domains), > + GFP_KERNEL); > + if (!pwrc->xlate.domains) > + return -ENOMEM; > + > + pwrc->domains = devm_kcalloc(&pdev->dev, match->count, > + sizeof(*pwrc->domains), GFP_KERNEL); > + if (!pwrc->domains) > + return -ENOMEM; > + > + pwrc->xlate.num_domains = match->count; > + platform_set_drvdata(pdev, pwrc); > + > + for (i = 0 ; i < match->count ; ++i) { > + struct meson_secure_pwrc_domain *dom = &pwrc->domains[i]; > + > + if (!match->domains[i].index) > + continue; > + > + dom->index = match->domains[i].index; > + dom->base.name = match->domains[i].name; > + dom->base.flags = match->domains[i].flags; > + dom->base.power_on = meson_secure_pwrc_on; > + dom->base.power_off = meson_secure_pwrc_off; > + > + pm_genpd_init(&dom->base, NULL, > + (match->domains[i].get_power ? > + match->domains[i].get_power(dom) : true)); > + > + pwrc->xlate.domains[i] = &dom->base; > + } > + > + return of_genpd_add_provider_onecell(pdev->dev.of_node, &pwrc->xlate); > +} > + > +static struct meson_secure_pwrc_domain_data meson_secure_a1_pwrc_data = { > + .domains = a1_pwrc_domains, > + .count = ARRAY_SIZE(a1_pwrc_domains), > +}; > + > +static const struct of_device_id meson_secure_pwrc_match_table[] = { > + { > + .compatible = "amlogic,meson-a1-pwrc", > + .data = &meson_secure_a1_pwrc_data, > + }, > + { } as mentioned by Martin, please add the sentinel string here. Helps for readability. > +}; > + > +static struct platform_driver meson_secure_pwrc_driver = { > + .probe = meson_secure_pwrc_probe, > + .driver = { > + .name = "meson_secure_pwrc", > + .of_match_table = meson_secure_pwrc_match_table, > + }, > +}; > + > +static int meson_secure_pwrc_init(void) > +{ > + return platform_driver_register(&meson_secure_pwrc_driver); > +} > +arch_initcall_sync(meson_secure_pwrc_init); Please use builtin_platform_driver() or explain in detail why the initcall is needed. Thanks, Kevin ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] soc: amlogic: Add support for Secure power domains controller 2019-09-25 22:41 ` Kevin Hilman @ 2019-09-26 9:17 ` Jianxin Pan 2019-09-26 16:23 ` Kevin Hilman 0 siblings, 1 reply; 13+ messages in thread From: Jianxin Pan @ 2019-09-26 9:17 UTC (permalink / raw) To: Kevin Hilman, linux-amlogic Cc: Zhiqiang Liang, Rob Herring, Neil Armstrong, Jerome Brunet, Martin Blumenstingl, linux-pm, linux-kernel, linux-arm-kernel, devicetree, Jian Hu, Hanjie Lin, Victor Wan, Xingyu Chen Hi Kevin, Thanks for your review. Please see my comments below. On 2019/9/26 6:41, Kevin Hilman wrote: > Hi Jianxin, > > Jianxin Pan <jianxin.pan@amlogic.com> writes: > >> Add support for the Amlogic Secure Power controller. In A1/C1 series, power >> control registers are in secure domain, and should be accessed by smc. >> >> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com> >> Signed-off-by: Zhiqiang Liang <zhiqiang.liang@amlogic.com> > > Thanks for the new power domain driver. > >> --- >> drivers/soc/amlogic/Kconfig | 13 +++ >> drivers/soc/amlogic/Makefile | 1 + >> drivers/soc/amlogic/meson-secure-pwrc.c | 182 ++++++++++++++++++++++++++++++++ >> 3 files changed, 196 insertions(+) >> create mode 100644 drivers/soc/amlogic/meson-secure-pwrc.c >> >> diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig >> index bc2c912..6cb06e7 100644 >> --- a/drivers/soc/amlogic/Kconfig >> +++ b/drivers/soc/amlogic/Kconfig >> @@ -48,6 +48,19 @@ config MESON_EE_PM_DOMAINS >> Say yes to expose Amlogic Meson Everything-Else Power Domains as >> Generic Power Domains. >> >> +config MESON_SECURE_PM_DOMAINS >> + bool "Amlogic Meson Secure Power Domains driver" >> + depends on ARCH_MESON || COMPILE_TEST >> + depends on PM && OF >> + depends on HAVE_ARM_SMCCC >> + default ARCH_MESON >> + select PM_GENERIC_DOMAINS >> + select PM_GENERIC_DOMAINS_OF >> + help >> + Support for the power controller on Amlogic A1/C1 series. >> + Say yes to expose Amlogic Meson Secure Power Domains as Generic >> + Power Domains. >> + >> config MESON_MX_SOCINFO >> bool "Amlogic Meson MX SoC Information driver" >> depends on ARCH_MESON || COMPILE_TEST >> diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile >> index de79d044..7b8c5d3 100644 >> --- a/drivers/soc/amlogic/Makefile >> +++ b/drivers/soc/amlogic/Makefile >> @@ -5,3 +5,4 @@ obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o >> obj-$(CONFIG_MESON_GX_PM_DOMAINS) += meson-gx-pwrc-vpu.o >> obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o >> obj-$(CONFIG_MESON_EE_PM_DOMAINS) += meson-ee-pwrc.o >> +obj-$(CONFIG_MESON_SECURE_PM_DOMAINS) += meson-secure-pwrc.o >> diff --git a/drivers/soc/amlogic/meson-secure-pwrc.c b/drivers/soc/amlogic/meson-secure-pwrc.c >> new file mode 100644 >> index 00000000..00c7232 >> --- /dev/null >> +++ b/drivers/soc/amlogic/meson-secure-pwrc.c >> @@ -0,0 +1,182 @@ >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) [...] >> + >> +static bool pwrc_secure_get_power(struct meson_secure_pwrc_domain *pwrc_domain) >> +{ >> + struct arm_smccc_res res; >> + >> + arm_smccc_smc(SMC_PWRC_GET, pwrc_domain->index, 0, >> + 0, 0, 0, 0, 0, &res); >> + >> + return res.a0 & 0x1; > > Please use a #define with a readable name for this mask. > The return type of this smc is bool. I will remove 0x1 mask in next version. Another question about smc: In this driver, no share memory is needed, and I use arm_smccc_smc() directly. Should I add secure-monitor = <&sm> in dtb and use meson_sm_call() from sm driver instead? >> +} > > What does the return value for this function mean? Does true mean > "powered off" or "powered on"> The return vaule for SMC_PWRC_GET : 0 -> power on 1 -> power off> See the rename I just did on the ee-pwrc driver: > https://lore.kernel.org/linux-amlogic/20190925213528.21515-2-khilman@kernel.org/ > I will follow and rename to _is_off() in the next verson. >> +static int meson_secure_pwrc_off(struct generic_pm_domain *domain) >> +{ >> + struct arm_smccc_res res; >> + struct meson_secure_pwrc_domain *pwrc_domain = [...] >> + >> +#define SEC_PD(__name, __flag) \ >> +{ \ >> + .name = #__name, \ >> + .index = PWRC_##__name##_ID, \ >> + .get_power = pwrc_secure_get_power, \ >> + .flags = __flag, \ >> +} >> + >> +static struct meson_secure_pwrc_domain_desc a1_pwrc_domains[] = { >> + SEC_PD(DSPA, 0), >> + SEC_PD(DSPB, 0), >> + SEC_PD(UART, GENPD_FLAG_ALWAYS_ON), > > This flag should only be used for domains where there are no linux > drivers. > > Rather than using this flag, you need to add a 'power-domain' property > to the uart driver in DT, and then update the meson_uart driver to use > the runtime PM API so that the domain is enabled whenever the UART is in > use. PM_UART Power domain is shared by uart, msr, jtag and cec. Uart should keep working in BL31, after kernel suspend and before kernel resume. > >> + SEC_PD(DMC, GENPD_FLAG_ALWAYS_ON), > > Please explain the need for ALWAYS_ON. > PM_DMC is used for DDR PHY ana/dig and DMC. There is no linux drver for them, and it should be always on. I will add comments for all these always on domains. >> + SEC_PD(I2C, 0), >> + SEC_PD(PSRAM, 0), >> + SEC_PD(ACODEC, 0), >> + SEC_PD(AUDIO, 0), >> + SEC_PD(OTP, 0), >> + SEC_PD(DMA, 0), >> + SEC_PD(SD_EMMC, 0), >> + SEC_PD(RAMA, 0), >> + SEC_PD(RAMB, GENPD_FLAG_ALWAYS_ON), > > Please explain the need for ALWAYS_ON. > In A1, SRAMB is used for bl31 ATF. >> + SEC_PD(IR, 0), >> + SEC_PD(SPICC, 0), >> + SEC_PD(SPIFC, 0), >> + SEC_PD(USB, 0), >> + SEC_PD(NIC, GENPD_FLAG_ALWAYS_ON), > > Please explain the need for ALWAYS_ON. > PD_NIC is used for NIC400, and should keep on. >> + SEC_PD(PDMIN, 0), >> + SEC_PD(RSA, 0), >> +}; >> + >> +static int meson_secure_pwrc_probe(struct platform_device *pdev) >> +{ >> + const struct meson_secure_pwrc_domain_data *match; >> + struct meson_secure_pwrc *pwrc; >> + int i; [...] >> + >> + return of_genpd_add_provider_onecell(pdev->dev.of_node, &pwrc->xlate); >> +} >> + >> +static struct meson_secure_pwrc_domain_data meson_secure_a1_pwrc_data = { >> + .domains = a1_pwrc_domains, >> + .count = ARRAY_SIZE(a1_pwrc_domains), >> +}; >> + >> +static const struct of_device_id meson_secure_pwrc_match_table[] = { >> + { >> + .compatible = "amlogic,meson-a1-pwrc", >> + .data = &meson_secure_a1_pwrc_data, >> + }, >> + { } > > as mentioned by Martin, please add the sentinel string here. Helps for > readability. > OK, I will fix it. Thank you. >> +}; >> + >> +static struct platform_driver meson_secure_pwrc_driver = { >> + .probe = meson_secure_pwrc_probe, >> + .driver = { >> + .name = "meson_secure_pwrc", >> + .of_match_table = meson_secure_pwrc_match_table, >> + }, >> +}; >> + >> +static int meson_secure_pwrc_init(void) >> +{ >> + return platform_driver_register(&meson_secure_pwrc_driver); >> +} >> +arch_initcall_sync(meson_secure_pwrc_init); > > Please use builtin_platform_driver() or explain in detail why the > initcall is needed. > OK, I will use builtin_platform_driver instead. > Thanks, > > Kevin > > . > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] soc: amlogic: Add support for Secure power domains controller 2019-09-26 9:17 ` Jianxin Pan @ 2019-09-26 16:23 ` Kevin Hilman 0 siblings, 0 replies; 13+ messages in thread From: Kevin Hilman @ 2019-09-26 16:23 UTC (permalink / raw) To: Jianxin Pan, linux-amlogic Cc: Zhiqiang Liang, Rob Herring, Neil Armstrong, Jerome Brunet, Martin Blumenstingl, linux-pm, linux-kernel, linux-arm-kernel, devicetree, Jian Hu, Hanjie Lin, Victor Wan, Xingyu Chen Jianxin Pan <jianxin.pan@amlogic.com> writes: > Hi Kevin, > > Thanks for your review. Please see my comments below. > > > On 2019/9/26 6:41, Kevin Hilman wrote: >> Hi Jianxin, >> >> Jianxin Pan <jianxin.pan@amlogic.com> writes: >> >>> Add support for the Amlogic Secure Power controller. In A1/C1 series, power >>> control registers are in secure domain, and should be accessed by smc. >>> >>> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com> >>> Signed-off-by: Zhiqiang Liang <zhiqiang.liang@amlogic.com> >> >> Thanks for the new power domain driver. >> >>> --- >>> drivers/soc/amlogic/Kconfig | 13 +++ >>> drivers/soc/amlogic/Makefile | 1 + >>> drivers/soc/amlogic/meson-secure-pwrc.c | 182 ++++++++++++++++++++++++++++++++ >>> 3 files changed, 196 insertions(+) >>> create mode 100644 drivers/soc/amlogic/meson-secure-pwrc.c >>> >>> diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig >>> index bc2c912..6cb06e7 100644 >>> --- a/drivers/soc/amlogic/Kconfig >>> +++ b/drivers/soc/amlogic/Kconfig >>> @@ -48,6 +48,19 @@ config MESON_EE_PM_DOMAINS >>> Say yes to expose Amlogic Meson Everything-Else Power Domains as >>> Generic Power Domains. >>> >>> +config MESON_SECURE_PM_DOMAINS >>> + bool "Amlogic Meson Secure Power Domains driver" >>> + depends on ARCH_MESON || COMPILE_TEST >>> + depends on PM && OF >>> + depends on HAVE_ARM_SMCCC >>> + default ARCH_MESON >>> + select PM_GENERIC_DOMAINS >>> + select PM_GENERIC_DOMAINS_OF >>> + help >>> + Support for the power controller on Amlogic A1/C1 series. >>> + Say yes to expose Amlogic Meson Secure Power Domains as Generic >>> + Power Domains. >>> + >>> config MESON_MX_SOCINFO >>> bool "Amlogic Meson MX SoC Information driver" >>> depends on ARCH_MESON || COMPILE_TEST >>> diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile >>> index de79d044..7b8c5d3 100644 >>> --- a/drivers/soc/amlogic/Makefile >>> +++ b/drivers/soc/amlogic/Makefile >>> @@ -5,3 +5,4 @@ obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o >>> obj-$(CONFIG_MESON_GX_PM_DOMAINS) += meson-gx-pwrc-vpu.o >>> obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o >>> obj-$(CONFIG_MESON_EE_PM_DOMAINS) += meson-ee-pwrc.o >>> +obj-$(CONFIG_MESON_SECURE_PM_DOMAINS) += meson-secure-pwrc.o >>> diff --git a/drivers/soc/amlogic/meson-secure-pwrc.c b/drivers/soc/amlogic/meson-secure-pwrc.c >>> new file mode 100644 >>> index 00000000..00c7232 >>> --- /dev/null >>> +++ b/drivers/soc/amlogic/meson-secure-pwrc.c >>> @@ -0,0 +1,182 @@ >>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > [...] >>> + >>> +static bool pwrc_secure_get_power(struct meson_secure_pwrc_domain *pwrc_domain) >>> +{ >>> + struct arm_smccc_res res; >>> + >>> + arm_smccc_smc(SMC_PWRC_GET, pwrc_domain->index, 0, >>> + 0, 0, 0, 0, 0, &res); >>> + >>> + return res.a0 & 0x1; >> >> Please use a #define with a readable name for this mask. >> The return type of this smc is bool. I will remove 0x1 mask in next version. > > Another question about smc: > In this driver, no share memory is needed, and I use arm_smccc_smc() directly. > Should I add secure-monitor = <&sm> in dtb and use meson_sm_call() from sm driver instead? Yes, that would be preferred. >>> +} >> >> What does the return value for this function mean? Does true mean >> "powered off" or "powered on"> > The return vaule for SMC_PWRC_GET : > 0 -> power on > 1 -> power off> See the rename I just did on the ee-pwrc driver: >> https://lore.kernel.org/linux-amlogic/20190925213528.21515-2-khilman@kernel.org/ >> I will follow and rename to _is_off() in the next verson. >>> +static int meson_secure_pwrc_off(struct generic_pm_domain *domain) >>> +{ >>> + struct arm_smccc_res res; >>> + struct meson_secure_pwrc_domain *pwrc_domain = > [...] >>> + >>> +#define SEC_PD(__name, __flag) \ >>> +{ \ >>> + .name = #__name, \ >>> + .index = PWRC_##__name##_ID, \ >>> + .get_power = pwrc_secure_get_power, \ >>> + .flags = __flag, \ >>> +} >>> + >>> +static struct meson_secure_pwrc_domain_desc a1_pwrc_domains[] = { >>> + SEC_PD(DSPA, 0), >>> + SEC_PD(DSPB, 0), >>> + SEC_PD(UART, GENPD_FLAG_ALWAYS_ON), >> >> This flag should only be used for domains where there are no linux >> drivers. >> >> Rather than using this flag, you need to add a 'power-domain' property >> to the uart driver in DT, and then update the meson_uart driver to use >> the runtime PM API so that the domain is enabled whenever the UART is in >> use. > > PM_UART Power domain is shared by uart, msr, jtag and cec. > Uart should keep working in BL31, after kernel suspend and before kernel resume. OK. >> >>> + SEC_PD(DMC, GENPD_FLAG_ALWAYS_ON), >> >> Please explain the need for ALWAYS_ON. >> > PM_DMC is used for DDR PHY ana/dig and DMC. > There is no linux drver for them, and it should be always on. > > I will add comments for all these always on domains. OK, thank you. Kevin ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] arm64: dts: meson: a1: add secure power domain controller 2019-09-19 12:11 [PATCH 0/3] arm64: meson: add support for A1 Power Domains Jianxin Pan 2019-09-19 12:11 ` [PATCH 1/3] dt-bindings: power: add Amlogic secure power domains bindings Jianxin Pan 2019-09-19 12:11 ` [PATCH 2/3] soc: amlogic: Add support for Secure power domains controller Jianxin Pan @ 2019-09-19 12:11 ` Jianxin Pan 2 siblings, 0 replies; 13+ messages in thread From: Jianxin Pan @ 2019-09-19 12:11 UTC (permalink / raw) To: Kevin Hilman, linux-amlogic Cc: Jianxin Pan, Zhiqiang Liang, Rob Herring, Neil Armstrong, Jerome Brunet, Martin Blumenstingl, linux-pm, linux-kernel, linux-arm-kernel, devicetree, Jian Hu, Hanjie Lin, Victor Wan, Xingyu Chen Enable power domain controller for Meson A1 SoC. Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com> Signed-off-by: Zhiqiang Liang <zhiqiang.liang@amlogic.com> --- arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi index 7210ad0..d689e5c 100644 --- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi @@ -93,6 +93,12 @@ clock-names = "xtal", "pclk", "baud"; status = "disabled"; }; + + pwrc: power-controller { + compatible = "amlogic,meson-a1-pwrc"; + #power-domain-cells = <1>; + status = "okay"; + }; }; gic: interrupt-controller@ff901000 { -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-10-10 3:31 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-19 12:11 [PATCH 0/3] arm64: meson: add support for A1 Power Domains Jianxin Pan 2019-09-19 12:11 ` [PATCH 1/3] dt-bindings: power: add Amlogic secure power domains bindings Jianxin Pan 2019-09-19 20:06 ` Martin Blumenstingl 2019-09-20 7:37 ` Jianxin Pan 2019-10-01 22:09 ` Rob Herring 2019-10-10 3:31 ` Jianxin Pan 2019-09-19 12:11 ` [PATCH 2/3] soc: amlogic: Add support for Secure power domains controller Jianxin Pan 2019-09-19 20:03 ` Martin Blumenstingl 2019-09-20 7:58 ` Jianxin Pan 2019-09-25 22:41 ` Kevin Hilman 2019-09-26 9:17 ` Jianxin Pan 2019-09-26 16:23 ` Kevin Hilman 2019-09-19 12:11 ` [PATCH 3/3] arm64: dts: meson: a1: add secure power domain controller Jianxin Pan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).