Re: [PATCH 1/3] dt-bindings: power: add Amlogic secure power domains bindings
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
>> Signed-off-by: Zhiqiang Liang
>> ---
>> .../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 ..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
>> +%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
>> +
>> +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 ..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
>> + */
>> +
>> +#ifndef _DT_BINDINGS_MESON_A1_POWER_H
>> +#define _DT_BINDINGS_MESON_A1_POWER_H
>> +
>> +#define PWRC_DSPA_ID8
>> +#define PWRC_DSPB_ID9
>> +#define PWRC_UART_ID10
>> +#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_ID19
>> +#define PWRC_RAMB_ID20
>> +#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
>>
>
> .
>
Re: [PATCH 1/3] dt-bindings: power: add Amlogic secure power domains bindings
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
> Signed-off-by: Zhiqiang Liang
> ---
> .../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 ..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
> +%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
> +
> +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 ..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
> + */
> +
> +#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_ID13
> +#define PWRC_ACODEC_ID 14
> +#define PWRC_AUDIO_ID15
> +#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_ID22
> +#define PWRC_SPIFC_ID23
> +#define PWRC_USB_ID 24
> +#define PWRC_NIC_ID 25
> +#define PWRC_PDMIN_ID26
> +#define PWRC_RSA_ID 27
> +#define PWRC_MAX_ID 28
> +
> +#endif
> --
> 2.7.4
>
Re: [PATCH 1/3] dt-bindings: power: add Amlogic secure power domains bindings
Hi Martin,
On 2019/9/20 4:06, Martin Blumenstingl wrote:
> Hi Jianxin,
>
> On Thu, Sep 19, 2019 at 2:11 PM 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
>> Signed-off-by: Zhiqiang Liang
>> ---
>> .../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 ..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.
> .
>
Re: [PATCH 1/3] dt-bindings: power: add Amlogic secure power domains bindings
Hi Jianxin,
On Thu, Sep 19, 2019 at 2:11 PM 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
> Signed-off-by: Zhiqiang Liang
> ---
> .../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 ..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
> +%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
> +
> +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
[PATCH 1/3] dt-bindings: power: add Amlogic secure power domains bindings
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
Signed-off-by: Zhiqiang Liang
---
.../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 ..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
+%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
+
+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 ..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
+ */
+
+#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_ID11
+#define PWRC_I2C_ID12
+#define PWRC_PSRAM_ID 13
+#define PWRC_ACODEC_ID 14
+#define PWRC_AUDIO_ID 15
+#define PWRC_OTP_ID16
+#define PWRC_DMA_ID17
+#define PWRC_SD_EMMC_ID18
+#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_ID24
+#define PWRC_NIC_ID25
+#define PWRC_PDMIN_ID 26
+#define PWRC_RSA_ID27
+#define PWRC_MAX_ID28
+
+#endif
--
2.7.4
