[PATCH 1/2] dt-bindings: soc: samsung: exynos-pmu: remove syscon for google,gs101-pmu

Peter Griffin posted 2 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH 1/2] dt-bindings: soc: samsung: exynos-pmu: remove syscon for google,gs101-pmu
Posted by Peter Griffin 3 months, 1 week ago
Since commit ba5095ebbc7a ("mfd: syscon: Allow syscon nodes without a
"syscon" compatible") it is possible to register a regmap without the
syscon compatible in the node.

Update the bindings for google,gs101-pmu so that the syscon compatible is
no longer required. As it isn't really correct to claim we are compatible with
syscon (as a mmio regmap created by syscon will not work on gs101).

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 Documentation/devicetree/bindings/soc/samsung/exynos-pmu.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/soc/samsung/exynos-pmu.yaml b/Documentation/devicetree/bindings/soc/samsung/exynos-pmu.yaml
index be1441193feeea1c12d0d5370168e9316c8dcf76..c5dd132a2b6948f8eda93e4a32ddd1b57a83fcc0 100644
--- a/Documentation/devicetree/bindings/soc/samsung/exynos-pmu.yaml
+++ b/Documentation/devicetree/bindings/soc/samsung/exynos-pmu.yaml
@@ -34,9 +34,10 @@ select:
 properties:
   compatible:
     oneOf:
+      - items:
+          - const: google,gs101-pmu
       - items:
           - enum:
-              - google,gs101-pmu
               - samsung,exynos3250-pmu
               - samsung,exynos4210-pmu
               - samsung,exynos4212-pmu

-- 
2.51.1.930.gacf6e81ea2-goog
Re: [PATCH 1/2] dt-bindings: soc: samsung: exynos-pmu: remove syscon for google,gs101-pmu
Posted by Krzysztof Kozlowski 2 months, 4 weeks ago
On Mon, Nov 03, 2025 at 08:03:10AM +0000, Peter Griffin wrote:
> Since commit ba5095ebbc7a ("mfd: syscon: Allow syscon nodes without a
> "syscon" compatible") it is possible to register a regmap without the
> syscon compatible in the node.
> 
> Update the bindings for google,gs101-pmu so that the syscon compatible is
> no longer required. As it isn't really correct to claim we are compatible with
> syscon (as a mmio regmap created by syscon will not work on gs101).
> 
> Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  Documentation/devicetree/bindings/soc/samsung/exynos-pmu.yaml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/soc/samsung/exynos-pmu.yaml b/Documentation/devicetree/bindings/soc/samsung/exynos-pmu.yaml
> index be1441193feeea1c12d0d5370168e9316c8dcf76..c5dd132a2b6948f8eda93e4a32ddd1b57a83fcc0 100644
> --- a/Documentation/devicetree/bindings/soc/samsung/exynos-pmu.yaml
> +++ b/Documentation/devicetree/bindings/soc/samsung/exynos-pmu.yaml
> @@ -34,9 +34,10 @@ select:
>  properties:
>    compatible:
>      oneOf:
> +      - items:

I expect new version (see other comment), so also please change items
into enum and drop const from below. items is redundant for one entry in
compatible (by convention) and having it as enum already makes it ready
for growing the enumeration for future devices.

> +          - const: google,gs101-pmu

Best regards,
Krzysztof
Re: [PATCH 1/2] dt-bindings: soc: samsung: exynos-pmu: remove syscon for google,gs101-pmu
Posted by Krzysztof Kozlowski 2 months, 4 weeks ago
On 12/11/2025 11:22, Krzysztof Kozlowski wrote:
>>      oneOf:
>> +      - items:
> 
> I expect new version (see other comment), so also please change items
> into enum and drop const from below. items is redundant for one entry in
> compatible (by convention) and having it as enum already makes it ready
> for growing the enumeration for future devices.
> 
>> +          - const: google,gs101-pmu
> 


Ah, and there is also checkpatch warning about length of commit msg.

Best regards,
Krzysztof
Re: [PATCH 1/2] dt-bindings: soc: samsung: exynos-pmu: remove syscon for google,gs101-pmu
Posted by Peter Griffin 2 months, 3 weeks ago
Hi Krzysztof,

Thanks for the review feedback!

On Wed, 12 Nov 2025 at 10:23, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 12/11/2025 11:22, Krzysztof Kozlowski wrote:
> >>      oneOf:
> >> +      - items:
> >
> > I expect new version (see other comment), so also please change items
> > into enum and drop const from below. items is redundant for one entry in
> > compatible (by convention) and having it as enum already makes it ready
> > for growing the enumeration for future devices.
> >
> >> +          - const: google,gs101-pmu
> >
>
>
> Ah, and there is also checkpatch warning about length of commit msg.

I just sent a new version addressing this and the other comments.

Thanks,

Peter
Re: [PATCH 1/2] dt-bindings: soc: samsung: exynos-pmu: remove syscon for google,gs101-pmu
Posted by Krzysztof Kozlowski 3 months ago
On Mon, Nov 03, 2025 at 08:03:10AM +0000, Peter Griffin wrote:
> Since commit ba5095ebbc7a ("mfd: syscon: Allow syscon nodes without a
> "syscon" compatible") it is possible to register a regmap without the
> syscon compatible in the node.
> 
> Update the bindings for google,gs101-pmu so that the syscon compatible is

That's an ABI break...

> no longer required. As it isn't really correct to claim we are compatible with
> syscon (as a mmio regmap created by syscon will not work on gs101).

... with kind of a reason, but then the question I have: was the
standard MMIO regmap exposed via syscon ever working for any part of
this PMU?

Original posting here:
https://lore.kernel.org/all/20231209233106.147416-2-peter.griffin@linaro.org/
did not change PMU driver and did not express that the syscon is broken.

Best regards,
Krzysztof
Re: [PATCH 1/2] dt-bindings: soc: samsung: exynos-pmu: remove syscon for google,gs101-pmu
Posted by Peter Griffin 3 months ago
Hi Krzysztof,

On Wed, 5 Nov 2025 at 08:55, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Mon, Nov 03, 2025 at 08:03:10AM +0000, Peter Griffin wrote:
> > Since commit ba5095ebbc7a ("mfd: syscon: Allow syscon nodes without a
> > "syscon" compatible") it is possible to register a regmap without the
> > syscon compatible in the node.
> >
> > Update the bindings for google,gs101-pmu so that the syscon compatible is
>
> That's an ABI break...
>
> > no longer required. As it isn't really correct to claim we are compatible with
> > syscon (as a mmio regmap created by syscon will not work on gs101).
>
> ... with kind of a reason, but then the question I have: was the
> standard MMIO regmap exposed via syscon ever working for any part of
> this PMU?
>
> Original posting here:
> https://lore.kernel.org/all/20231209233106.147416-2-peter.griffin@linaro.org/
> did not change PMU driver and did not express that the syscon is broken.

With the benefit of hindsight, no, PMU write register access was never
working for any part of the PMU.

On the initial posting I didn't hit this as we were only disabling the
Watchdog on boot (which doesn't write any PMU registers). Once the
issue with accessing PMU regs became clear, I first suspected some
missing clock. Then later it became apparent we needed a SMC call and
after some initial proposals the syscon driver was enhanced with the
of_syscon_register_regmap() API in 769cb63166d9 ("mfd: syscon: Add
of_syscon_register_regmap() API"). At the time that commit was merged,
syscon driver required a "syscon" compatible, but since ba5095ebbc7a
("mfd: syscon: Allow syscon nodes without a "syscon" compatible") that
is no longer the case.

Peter.
Re: [PATCH 1/2] dt-bindings: soc: samsung: exynos-pmu: remove syscon for google,gs101-pmu
Posted by Krzysztof Kozlowski 2 months, 4 weeks ago
On Tue, Nov 11, 2025 at 10:18:47AM +0000, Peter Griffin wrote:
> Hi Krzysztof,
> 
> On Wed, 5 Nov 2025 at 08:55, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On Mon, Nov 03, 2025 at 08:03:10AM +0000, Peter Griffin wrote:
> > > Since commit ba5095ebbc7a ("mfd: syscon: Allow syscon nodes without a
> > > "syscon" compatible") it is possible to register a regmap without the
> > > syscon compatible in the node.
> > >
> > > Update the bindings for google,gs101-pmu so that the syscon compatible is
> >
> > That's an ABI break...
> >
> > > no longer required. As it isn't really correct to claim we are compatible with
> > > syscon (as a mmio regmap created by syscon will not work on gs101).
> >
> > ... with kind of a reason, but then the question I have: was the
> > standard MMIO regmap exposed via syscon ever working for any part of
> > this PMU?
> >
> > Original posting here:
> > https://lore.kernel.org/all/20231209233106.147416-2-peter.griffin@linaro.org/
> > did not change PMU driver and did not express that the syscon is broken.
> 
> With the benefit of hindsight, no, PMU write register access was never
> working for any part of the PMU.

Please add it to the last paragraph of your commit msg, that this was
never working in practice thus changing ABI is justified.

Best regards,
Krzysztof