[PATCH 0/4] amlogic SoC's power-domains fixes

[PATCH 0/4] amlogic SoC's power-domains fixes

[George Stark]

Here's some fixes to the bindings and device tree related to Amlogic A1 SoC.
The SoC provides dedicated power domain for for almost all periphery.

George Stark (4):
  dt-bindings: spi: amlogic,a1-spifc: make power-domains required
  dt-bindings: i2c: amlogic,meson6-i2c: add optional power-domains
  dt-bindings: thermal: amlogic,thermal: add power-domains
  arm64: dts: meson: a1: bind power domain to temperature sensor

 .../bindings/i2c/amlogic,meson6-i2c.yaml           |  3 +++
 .../devicetree/bindings/spi/amlogic,a1-spifc.yaml  |  4 ++++
 .../bindings/thermal/amlogic,thermal.yaml          | 14 ++++++++++++++
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi          |  1 +
 4 files changed, 22 insertions(+)

--
2.25.1

[PATCH 1/4] dt-bindings: spi: amlogic,a1-spifc: make power-domains required

[George Stark]

SPI Flash Controller has dedicated power domain so make the
corresponding property required.

Signed-off-by: George Stark <gnstark@salutedevices.com>
---
 Documentation/devicetree/bindings/spi/amlogic,a1-spifc.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/amlogic,a1-spifc.yaml b/Documentation/devicetree/bindings/spi/amlogic,a1-spifc.yaml
index 043879b434ac..e704aff8a862 100644
--- a/Documentation/devicetree/bindings/spi/amlogic,a1-spifc.yaml
+++ b/Documentation/devicetree/bindings/spi/amlogic,a1-spifc.yaml
@@ -30,15 +30,19 @@ required:
   - compatible
   - reg
   - clocks
+  - power-domains
 
 unevaluatedProperties: false
 
 examples:
   - |
+    #include <dt-bindings/power/meson-a1-power.h>
+
     spi@fd000400 {
         compatible = "amlogic,a1-spifc";
         reg = <0xfd000400 0x290>;
         clocks = <&clkc_clkid_spifc>;
         #address-cells = <1>;
         #size-cells = <0>;
+        power-domains = <&pwrc PWRC_SPIFC_ID>;
     };
-- 
2.25.1

Re: [PATCH 1/4] dt-bindings: spi: amlogic,a1-spifc: make power-domains required

[Krzysztof Kozlowski]

On 08/07/2024 21:48, George Stark wrote:
> SPI Flash Controller has dedicated power domain so make the
> corresponding property required.

That's an ABI break. Being part of power domain does not necessarily
mean it must be required, so your commit msg lacks rationale. Especially
on the ABI part.



Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: spi: amlogic,a1-spifc: make power-domains required

[George Stark]

Hello Krzysztof, Neil, Rob

Thanks for your reviews.

I'd just like to clarify things a little on that matter. On A1 most of
periphery has either dedicated power domain (ir, spifc, spi, cpu etc) or
several peripheries share single PD e.g. pwm, temp sensor, i2c, adc
share PD 12. Appropriate PD must be turned on before the periphery is
touched and be on while periphery is used. Experiments confirm it. So
power-domains property in dts nodes is really required. It makes kernel
to control PDs when bound periphery is used.

If I understand bindings thing correctly we should make power-domains
property required in bindings too in that case, at least for those
peripheries which has *a1* compatibles (some peripheries use compatible
from older SoCs without PDs).

What you think?

On 7/8/24 22:51, Krzysztof Kozlowski wrote:
> On 08/07/2024 21:48, George Stark wrote:
>> SPI Flash Controller has dedicated power domain so make the
>> corresponding property required.
> 
> That's an ABI break. Being part of power domain does not necessarily
> mean it must be required, so your commit msg lacks rationale. Especially
> on the ABI part.
> 
> 
> 
> Best regards,
> Krzysztof
> 

-- 
Best regards
George

Re: [PATCH 1/4] dt-bindings: spi: amlogic,a1-spifc: make power-domains required

[neil.armstrong]

On 10/07/2024 18:23, George Stark wrote:
> Hello Krzysztof, Neil, Rob
> 
> Thanks for your reviews.
> 
> I'd just like to clarify things a little on that matter. On A1 most of
> periphery has either dedicated power domain (ir, spifc, spi, cpu etc) or
> several peripheries share single PD e.g. pwm, temp sensor, i2c, adc
> share PD 12. Appropriate PD must be turned on before the periphery is
> touched and be on while periphery is used. Experiments confirm it. So
> power-domains property in dts nodes is really required. It makes kernel
> to control PDs when bound periphery is used.
> 
> If I understand bindings thing correctly we should make power-domains
> property required in bindings too in that case, at least for those
> peripheries which has *a1* compatibles (some peripheries use compatible
> from older SoCs without PDs).
> 
> What you think?

Yes, but if the compatible has already been defined without a required
power-domain, then it's too late to mark it as required since it's an ABI
break.

So next time a new compatible is added, it's the appropriate time to mark
it as required.

But in any case it's not a big deal because if the power-domain is not
specified it won't work, like if you use wrong register address or the
wrong interrupt line.

The fact power-domain can be specified is enough.

Neil

> 
> On 7/8/24 22:51, Krzysztof Kozlowski wrote:
>> On 08/07/2024 21:48, George Stark wrote:
>>> SPI Flash Controller has dedicated power domain so make the
>>> corresponding property required.
>>
>> That's an ABI break. Being part of power domain does not necessarily
>> mean it must be required, so your commit msg lacks rationale. Especially
>> on the ABI part.
>>
>>
>>
>> Best regards,
>> Krzysztof
>>
> 

Re: [PATCH 1/4] dt-bindings: spi: amlogic,a1-spifc: make power-domains required

[Rob Herring (Arm)]

On Mon, 08 Jul 2024 22:48:05 +0300, George Stark wrote:
> SPI Flash Controller has dedicated power domain so make the
> corresponding property required.
> 
> Signed-off-by: George Stark <gnstark@salutedevices.com>
> ---
>  Documentation/devicetree/bindings/spi/amlogic,a1-spifc.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/amlogic,a1-spifc.example.dtb: spi@fd000400: Unevaluated properties are not allowed ('power-domains' was unexpected)
	from schema $id: http://devicetree.org/schemas/spi/amlogic,a1-spifc.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240708194808.1819185-2-gnstark@salutedevices.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

[PATCH 2/4] dt-bindings: i2c: amlogic,meson6-i2c: add optional power-domains

[George Stark]

On newer SoCs, the I2C hardware can require a power domain to operate.
Since the same compatible is used for older and newer SoCs make
power-domains property optional.

Signed-off-by: George Stark <gnstark@salutedevices.com>
---
 Documentation/devicetree/bindings/i2c/amlogic,meson6-i2c.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/amlogic,meson6-i2c.yaml b/Documentation/devicetree/bindings/i2c/amlogic,meson6-i2c.yaml
index 26bed558c6b8..c4cc8af18280 100644
--- a/Documentation/devicetree/bindings/i2c/amlogic,meson6-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/amlogic,meson6-i2c.yaml
@@ -30,6 +30,9 @@ properties:
   clocks:
     minItems: 1
 
+  power-domains:
+    maxItems: 1
+
 required:
   - compatible
   - reg
-- 
2.25.1

Re: [PATCH 2/4] dt-bindings: i2c: amlogic,meson6-i2c: add optional power-domains

[neil.armstrong]

On 08/07/2024 21:48, George Stark wrote:
> On newer SoCs, the I2C hardware can require a power domain to operate.
> Since the same compatible is used for older and newer SoCs make
> power-domains property optional.
> 
> Signed-off-by: George Stark <gnstark@salutedevices.com>
> ---
>   Documentation/devicetree/bindings/i2c/amlogic,meson6-i2c.yaml | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/amlogic,meson6-i2c.yaml b/Documentation/devicetree/bindings/i2c/amlogic,meson6-i2c.yaml
> index 26bed558c6b8..c4cc8af18280 100644
> --- a/Documentation/devicetree/bindings/i2c/amlogic,meson6-i2c.yaml
> +++ b/Documentation/devicetree/bindings/i2c/amlogic,meson6-i2c.yaml
> @@ -30,6 +30,9 @@ properties:
>     clocks:
>       minItems: 1
>   
> +  power-domains:
> +    maxItems: 1
> +
>   required:
>     - compatible
>     - reg

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Re: [PATCH 2/4] dt-bindings: i2c: amlogic,meson6-i2c: add optional power-domains

[Rob Herring (Arm)]

On Mon, 08 Jul 2024 22:48:06 +0300, George Stark wrote:
> On newer SoCs, the I2C hardware can require a power domain to operate.
> Since the same compatible is used for older and newer SoCs make
> power-domains property optional.
> 
> Signed-off-by: George Stark <gnstark@salutedevices.com>
> ---
>  Documentation/devicetree/bindings/i2c/amlogic,meson6-i2c.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

[PATCH 3/4] dt-bindings: thermal: amlogic,thermal: add power-domains

[George Stark]

On newer SoCs, the thermal hardware can require a power domain to
operate so add corresponding property as optional by default and as
required for Meson A1 due to it's temperature sensor has dedicated
power domain.

Signed-off-by: George Stark <gnstark@salutedevices.com>
---
 .../bindings/thermal/amlogic,thermal.yaml          | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
index 01fccdfc4178..5c9147e3b734 100644
--- a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
@@ -30,6 +30,9 @@ properties:
   clocks:
     maxItems: 1
 
+  power-domains:
+    maxItems: 1
+
   amlogic,ao-secure:
     description: phandle to the ao-secure syscon
     $ref: /schemas/types.yaml#/definitions/phandle
@@ -44,6 +47,17 @@ required:
   - clocks
   - amlogic,ao-secure
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - amlogic,a1-cpu-thermal
+    then:
+      required:
+        - power-domains
+
 additionalProperties: false
 
 examples:
-- 
2.25.1

Re: [PATCH 3/4] dt-bindings: thermal: amlogic,thermal: add power-domains

[Krzysztof Kozlowski]

On 08/07/2024 21:48, George Stark wrote:
> On newer SoCs, the thermal hardware can require a power domain to
> operate so add corresponding property as optional by default and as
> required for Meson A1 due to it's temperature sensor has dedicated
> power domain.
> 

That's again ABI break with a reason "there is power domain", so not
really sufficient.

Best regards,
Krzysztof

[PATCH 4/4] arm64: dts: meson: a1: bind power domain to temperature sensor

[George Stark]

Meson A1 temperature sensor has dedicated power domain so bind it
to the device node.

Signed-off-by: George Stark <gnstark@salutedevices.com>
---
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index dd5695963caa..86d77f51c25c 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -919,6 +919,7 @@ cpu_temp: temperature-sensor@4c00 {
 				assigned-clock-rates = <500000>;
 				#thermal-sensor-cells = <0>;
 				amlogic,ao-secure = <&sec_AO>;
+				power-domains = <&pwrc PWRC_I2C_ID>;
 			};
 
 			hwrng: rng@5118 {
-- 
2.25.1

Re: [PATCH 4/4] arm64: dts: meson: a1: bind power domain to temperature sensor

[neil.armstrong]

On 08/07/2024 21:48, George Stark wrote:
> Meson A1 temperature sensor has dedicated power domain so bind it
> to the device node.
> 
> Signed-off-by: George Stark <gnstark@salutedevices.com>
> ---
>   arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> index dd5695963caa..86d77f51c25c 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> @@ -919,6 +919,7 @@ cpu_temp: temperature-sensor@4c00 {
>   				assigned-clock-rates = <500000>;
>   				#thermal-sensor-cells = <0>;
>   				amlogic,ao-secure = <&sec_AO>;
> +				power-domains = <&pwrc PWRC_I2C_ID>;
>   			};
>   
>   			hwrng: rng@5118 {

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Re: [PATCH 0/4] amlogic SoC's power-domains fixes

[neil.armstrong]

Hi,

On 08/07/2024 21:48, George Stark wrote:
> Here's some fixes to the bindings and device tree related to Amlogic A1 SoC.
> The SoC provides dedicated power domain for for almost all periphery.
> 
> George Stark (4):
>    dt-bindings: spi: amlogic,a1-spifc: make power-domains required
>    dt-bindings: i2c: amlogic,meson6-i2c: add optional power-domains
>    dt-bindings: thermal: amlogic,thermal: add power-domains
>    arm64: dts: meson: a1: bind power domain to temperature sensor
> 
>   .../bindings/i2c/amlogic,meson6-i2c.yaml           |  3 +++
>   .../devicetree/bindings/spi/amlogic,a1-spifc.yaml  |  4 ++++
>   .../bindings/thermal/amlogic,thermal.yaml          | 14 ++++++++++++++
>   arch/arm64/boot/dts/amlogic/meson-a1.dtsi          |  1 +
>   4 files changed, 22 insertions(+)
> 
> --
> 2.25.1
> 

You can drop patch 1 & drop required on patch 3, and it will be good to go.

Thanks,
Neil