[PATCH 1/2] dt-bindings: pinctrl: add syscon property

Troy Mitchell posted 2 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH 1/2] dt-bindings: pinctrl: add syscon property
Posted by Troy Mitchell 1 month, 2 weeks ago
In order to access the protected IO power domain registers, a valid
unlock sequence must be performed by writing the required keys to the
AIB Secure Access Register (ASAR).

The ASAR register resides within the APBC register address space.
A corresponding syscon property is added to allow the pinctrl driver
to access this register.

Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
---
 .../devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml      | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
index c5b0218ad6251f97b1f27089ffff724a7b0f69ae..4dc49c2cc1d52008ad89896ae0419091802cd2c4 100644
--- a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
@@ -32,6 +32,15 @@ properties:
   resets:
     maxItems: 1
 
+  spacemit,apbc:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: phandle to syscon that access the protected register
+          - description: offset of access secure registers
+    description:
+      A phandle to syscon with byte offset to access the protected register
+
 patternProperties:
   '-cfg$':
     type: object
@@ -111,6 +120,7 @@ required:
   - reg
   - clocks
   - clock-names
+  - spacemit,apbc
 
 additionalProperties: false
 
@@ -128,6 +138,7 @@ examples:
             clocks = <&syscon_apbc 42>,
                      <&syscon_apbc 94>;
             clock-names = "func", "bus";
+            spacemit,apbc = <&syscon_apbc 0x50>;
 
             uart0_2_cfg: uart0-2-cfg {
                 uart0-2-pins {

-- 
2.52.0
Re: [PATCH 1/2] dt-bindings: pinctrl: add syscon property
Posted by Linus Walleij 1 month, 1 week ago
Hi Troy,

thanks for your patch!

On Tue, Dec 23, 2025 at 10:11 AM Troy Mitchell
<troy.mitchell@linux.spacemit.com> wrote:

> +  spacemit,apbc:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: phandle to syscon that access the protected register
> +          - description: offset of access secure registers
(...)
> +            spacemit,apbc = <&syscon_apbc 0x50>;

Isn't the offset implicit from the compatible of the pin controller or
the syscon or any other compatible?

It's easy to check compatibles in the device tree and just say
this offset is 0x50 if compatible is so-or-so, and something else
for another compatible and just give an error if an unknown
compatible appears.

So: please try to avoid to put things the code can easily look
up into the device tree.

Yours,
Linus Walleij
Re: [PATCH 1/2] dt-bindings: pinctrl: add syscon property
Posted by Troy Mitchell 1 month ago
On Thu, Jan 01, 2026 at 11:54:02PM +0100, Linus Walleij wrote:
> Hi Troy,
> 
> thanks for your patch!
> 
> On Tue, Dec 23, 2025 at 10:11 AM Troy Mitchell
> <troy.mitchell@linux.spacemit.com> wrote:
> 
> > +  spacemit,apbc:
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    items:
> > +      - items:
> > +          - description: phandle to syscon that access the protected register
> > +          - description: offset of access secure registers
> (...)
> > +            spacemit,apbc = <&syscon_apbc 0x50>;
> 
> Isn't the offset implicit from the compatible of the pin controller or
> the syscon or any other compatible?
> 
> It's easy to check compatibles in the device tree and just say
> this offset is 0x50 if compatible is so-or-so, and something else
> for another compatible and just give an error if an unknown
> compatible appears.
> 
> So: please try to avoid to put things the code can easily look
> up into the device tree.
Thanks for you pointing it out. I'll remove it.

                          - Troy
> 
> Yours,
> Linus Walleij
> 
Re: [PATCH 1/2] dt-bindings: pinctrl: add syscon property
Posted by Krzysztof Kozlowski 1 month, 2 weeks ago
On Tue, Dec 23, 2025 at 05:11:11PM +0800, Troy Mitchell wrote:
> In order to access the protected IO power domain registers, a valid
> unlock sequence must be performed by writing the required keys to the
> AIB Secure Access Register (ASAR).
> 
> The ASAR register resides within the APBC register address space.
> A corresponding syscon property is added to allow the pinctrl driver
> to access this register.
> 
> Signed-off-by: Troy Mitchell <troy.mitchell@linux.spacemit.com>
> ---
>  .../devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml      | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
> index c5b0218ad6251f97b1f27089ffff724a7b0f69ae..4dc49c2cc1d52008ad89896ae0419091802cd2c4 100644
> --- a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
> @@ -32,6 +32,15 @@ properties:
>    resets:
>      maxItems: 1
>  
> +  spacemit,apbc:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: phandle to syscon that access the protected register
> +          - description: offset of access secure registers
> +    description:
> +      A phandle to syscon with byte offset to access the protected register

Say here for what purpose.

> +
>  patternProperties:
>    '-cfg$':
>      type: object
> @@ -111,6 +120,7 @@ required:
>    - reg
>    - clocks
>    - clock-names
> +  - spacemit,apbc

That's ABI break without justification.

>  
>  additionalProperties: false
>  
> @@ -128,6 +138,7 @@ examples:
>              clocks = <&syscon_apbc 42>,
>                       <&syscon_apbc 94>;
>              clock-names = "func", "bus";
> +            spacemit,apbc = <&syscon_apbc 0x50>;
>  
>              uart0_2_cfg: uart0-2-cfg {
>                  uart0-2-pins {
> 
> -- 
> 2.52.0
>
Re: [PATCH 1/2] dt-bindings: pinctrl: add syscon property
Posted by Krzysztof Kozlowski 1 month, 2 weeks ago
On Tue, Dec 23, 2025 at 05:11:11PM +0800, Troy Mitchell wrote:
> In order to access the protected IO power domain registers, a valid
> unlock sequence must be performed by writing the required keys to the
> AIB Secure Access Register (ASAR).
> 
> The ASAR register resides within the APBC register address space.
> A corresponding syscon property is added to allow the pinctrl driver
> to access this register.
>

Also:

Please use subject prefixes matching the subsystem. You can get them for
example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

You are not adding syscon to all pinctrls.

Best regards,
Krzysztof
Re: [PATCH 1/2] dt-bindings: pinctrl: add syscon property
Posted by Troy Mitchell 1 month ago
On Sat, Dec 27, 2025 at 01:58:52PM +0100, Krzysztof Kozlowski wrote:
> On Tue, Dec 23, 2025 at 05:11:11PM +0800, Troy Mitchell wrote:
> > In order to access the protected IO power domain registers, a valid
> > unlock sequence must be performed by writing the required keys to the
> > AIB Secure Access Register (ASAR).
> > 
> > The ASAR register resides within the APBC register address space.
> > A corresponding syscon property is added to allow the pinctrl driver
> > to access this register.
> >
> 
> Also:
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> 
> You are not adding syscon to all pinctrls.
I lost "spacemit" prefix. I'll add it in the next version.
Thanks!

                        - Troy
> 
> Best regards,
> Krzysztof
> 
>