In the LAN966x PCI device use case, syscon cannot be used as syscon
devices do not support removal [1]. A syscon device is a core "system"
device and not a device available in some addon boards and so, it is not
supposed to be removed.

In order to remove the syscon device usage, allow the reset controller
to have a direct access to the address range it needs to use.

Link: https://lore.kernel.org/all/20240923100741.11277439@bootlin.com/ [1]
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 .../bindings/reset/microchip,rst.yaml         | 35 ++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/reset/microchip,rst.yaml b/Documentation/devicetree/bindings/reset/microchip,rst.yaml
index f2da0693b05a..5164239a372c 100644
--- a/Documentation/devicetree/bindings/reset/microchip,rst.yaml
+++ b/Documentation/devicetree/bindings/reset/microchip,rst.yaml
@@ -25,12 +25,16 @@ properties:
       - microchip,lan966x-switch-reset
 
   reg:
+    minItems: 1
     items:
       - description: global control block registers
+      - description: cpu system block registers
 
   reg-names:
+    minItems: 1
     items:
       - const: gcb
+      - const: cpu
 
   "#reset-cells":
     const: 1
@@ -39,12 +43,29 @@ properties:
     $ref: /schemas/types.yaml#/definitions/phandle
     description: syscon used to access CPU reset
 
+allOf:
+  # Allow to use the second reg item instead of cpu-syscon
+  - if:
+      required:
+        - cpu-syscon
+    then:
+      properties:
+        reg:
+          maxItems: 1
+        reg-names:
+          maxItems: 1
+    else:
+      properties:
+        reg:
+          minItems: 2
+        reg-names:
+          minItems: 2
+
 required:
   - compatible
   - reg
   - reg-names
   - "#reset-cells"
-  - cpu-syscon
 
 additionalProperties: false
 
@@ -57,3 +78,15 @@ examples:
         #reset-cells = <1>;
         cpu-syscon = <&cpu_ctrl>;
     };
+
+    /*
+     * The following construction can be used if the cpu-syscon device is not
+     * present. This is the case when the LAN966x is used as a PCI device.
+     */
+    reset-controller@22010008 {
+        compatible = "microchip,lan966x-switch-reset";
+        reg = <0xe200400c 0x4>,
+              <0xe00c0000 0xa8>;
+        reg-names = "gcb", "cpu";
+        #reset-cells = <1>;
+    };
-- 
2.46.1

On Mon, Sep 30, 2024 at 02:15:41PM +0200, Herve Codina wrote:
> In the LAN966x PCI device use case, syscon cannot be used as syscon
> devices do not support removal [1]. A syscon device is a core "system"
> device and not a device available in some addon boards and so, it is not
> supposed to be removed.

That's not accurate. syscon is our own, Linux term which means also
anything exposing set of registers.

If you need to unload syscons, implement it. syscon is the same resource
as all others so should be handled same way.

> 
> In order to remove the syscon device usage, allow the reset controller
> to have a direct access to the address range it needs to use.

So you map same address twice? That's not good, because you have no
locking over concurrent register accesses.

> 
> Link: https://lore.kernel.org/all/20240923100741.11277439@bootlin.com/ [1]
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  .../bindings/reset/microchip,rst.yaml         | 35 ++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/reset/microchip,rst.yaml b/Documentation/devicetree/bindings/reset/microchip,rst.yaml
> index f2da0693b05a..5164239a372c 100644
> --- a/Documentation/devicetree/bindings/reset/microchip,rst.yaml
> +++ b/Documentation/devicetree/bindings/reset/microchip,rst.yaml
> @@ -25,12 +25,16 @@ properties:
>        - microchip,lan966x-switch-reset
>  
>    reg:
> +    minItems: 1
>      items:
>        - description: global control block registers
> +      - description: cpu system block registers
>  
>    reg-names:
> +    minItems: 1
>      items:
>        - const: gcb
> +      - const: cpu
>  
>    "#reset-cells":
>      const: 1
> @@ -39,12 +43,29 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/phandle
>      description: syscon used to access CPU reset
>  
> +allOf:
> +  # Allow to use the second reg item instead of cpu-syscon
> +  - if:
> +      required:
> +        - cpu-syscon
> +    then:
> +      properties:
> +        reg:
> +          maxItems: 1
> +        reg-names:
> +          maxItems: 1
> +    else:
> +      properties:
> +        reg:
> +          minItems: 2
> +        reg-names:
> +          minItems: 2
> +
>  required:
>    - compatible
>    - reg
>    - reg-names
>    - "#reset-cells"
> -  - cpu-syscon
>  
>  additionalProperties: false
>  
> @@ -57,3 +78,15 @@ examples:
>          #reset-cells = <1>;
>          cpu-syscon = <&cpu_ctrl>;
>      };
> +
> +    /*
> +     * The following construction can be used if the cpu-syscon device is not
> +     * present. This is the case when the LAN966x is used as a PCI device.
> +     */
> +    reset-controller@22010008 {
> +        compatible = "microchip,lan966x-switch-reset";
> +        reg = <0xe200400c 0x4>,
> +              <0xe00c0000 0xa8>;

If you have here CPU address, then syscon device is present...

Best regards,
Krzysztof

Hi Krystoff,

On Tue, 1 Oct 2024 08:43:23 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On Mon, Sep 30, 2024 at 02:15:41PM +0200, Herve Codina wrote:
> > In the LAN966x PCI device use case, syscon cannot be used as syscon
> > devices do not support removal [1]. A syscon device is a core "system"
> > device and not a device available in some addon boards and so, it is not
> > supposed to be removed.  
> 
> That's not accurate. syscon is our own, Linux term which means also
> anything exposing set of registers.
> 
> If you need to unload syscons, implement it. syscon is the same resource
> as all others so should be handled same way.
> 
> > 
> > In order to remove the syscon device usage, allow the reset controller
> > to have a direct access to the address range it needs to use.  
> 
> So you map same address twice? That's not good, because you have no
> locking over concurrent register accesses.
> 

I will remove this patch and keep using the syscon node in the next
iteration.

Best regards,
Hervé