Re: [PATCH 02/10] dt-bindings: net: can: grcan: Convert GRCAN CAN controllers binding from txt to YAML

[Krzysztof Kozlowski]

On 11/12/2025 11:11, Arun Muthusamy wrote:
> Hi Krzysztof,
> 
> Thank you for your thorough review. I’d like to clarify a few points 
> regarding the DT binding and get your guidance.
> 
> On 11/18/25 12:01, Krzysztof Kozlowski wrote:
>> On 18/11/2025 10:21, Arun Muthusamy wrote:
>>> +      Fallback on node name matching for systems that don't provide compatible.
>>> +    enum:
>>> +      - GAISLER_GRCAN
>>> +      - 01_03d
>>> +      - GAISLER_GRHCAN
>>> +      - "01_034"
>> This does not really work. Are you really defining here "name" property?
> 
> The driver supports two of the platforms which are LEON and NOEL 

I don't care much about driver. Please describe here hardware.

> platforms. PROM-based *LEON* systems identify uses the "node name" 
> property, while DTS based *NOEL* systems use proper "|compatible"|strings. On LEON (SPARC32), AMBA Plug & Play information creates the DT 
> properties, and drivers historically match devices based on node names.
> To reflect this, I updated the $nodename pattern to support LEON-style 
> node names: properties:
>    $nodename:
>      pattern: "^(GAISLER_GRCAN|01_03d|GAISLER_GRHCAN|01_034)$" I’d 
> appreciate any suggestions on the preferred way to describe this node 
> name for PROM-based LEON.
> 
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  freq:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description: |
>>> +      Frequency of the external oscillator clock in Hz (the frequency of the
>>> +      amba bus in the ordinary case).
>>> +      This property should be used by systems that utilize the common clock
>>> +      framework is not supported.
>> Missing systemid. Your commit msg must explain any changes done to the
>> binding during conversion.
> 
> The driver now reads systemid directly from /ambapp0, so the property no 
> longer needs to be defined in the DTS. The previous documentation was 
> outdated and should have been updated after commit:
> 1e93ed26acf0 ("can: grcan: grcan_probe(): fix broken system id check for 
> errata workaround needs").

Read again what I wrote.

> 
>>> +  - reg
>>> +  - interrupts
>> Where is freq? It was required in the old binding. Again, you need to
>> explain the changes.
> 
> LEON: relies on the freq property
> NOEL: uses a standard clocks binding
> Because of this dual approach, the freq property is no longer required 
> in the DTS binding itself as theAMBA Plug & Play creates the DT properties.


Please read my comment carefully: you need to explain the changes. But
not to me, to everyone in the commit msg.

Best regards,
Krzysztof