Re: [PATCH v2] dt-bindings: net: can: atmel: Convert to json schema

[Krzysztof Kozlowski <krzk@kernel.org>]

On 13/11/2024 06:30, Charan.Pedumuru@microchip.com wrote:
> On 03/10/24 14:04, Krzysztof Kozlowski wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On Thu, Oct 03, 2024 at 10:37:03AM +0530, Charan Pedumuru wrote:
>>> Convert atmel-can documentation to yaml format
>>>
>>> Signed-off-by: Charan Pedumuru <charan.pedumuru@microchip.com>
>>> ---
>>> Changes in v2:
>>> - Renamed the title to "Microchip AT91 CAN controller"
>>> - Removed the unnecessary labels and add clock properties to examples
>>> - Removed if condition statements and made clock properties as default required properties
>>> - Link to v1: https://lore.kernel.org/r/20240912-can-v1-1-c5651b1809bb@microchip.com
>>> ---
>>>   .../bindings/net/can/atmel,at91sam9263-can.yaml    | 58 ++++++++++++++++++++++
>>>   .../devicetree/bindings/net/can/atmel-can.txt      | 15 ------
>>>   2 files changed, 58 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/can/atmel,at91sam9263-can.yaml b/Documentation/devicetree/bindings/net/can/atmel,at91sam9263-can.yaml
>>> new file mode 100644
>>> index 000000000000..c818c01a718b
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/can/atmel,at91sam9263-can.yaml
>>> @@ -0,0 +1,58 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/net/can/atmel,at91sam9263-can.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Microchip AT91 CAN Controller
>>> +
>>> +maintainers:
>>> +  - Nicolas Ferre <nicolas.ferre@microchip.com>
>>> +
>>> +allOf:
>>> +  - $ref: can-controller.yaml#
>>> +
>>> +properties:
>>> +  compatible:
>>> +    oneOf:
>>> +      - enum:
>>> +          - atmel,at91sam9263-can
>>> +          - atmel,at91sam9x5-can
>>> +      - items:
>>> +          - enum:
>>> +              - microchip,sam9x60-can
>>> +          - const: atmel,at91sam9x5-can
>> That is not what old binding said.
> 
> Apologies for the late reply, the driver doesn't have compatible with 
> "microchip,sam9x60-can",
> so I made "atmel,at91sam9x5-can" as fallback driver

Any changes to the binding must be clearly expressed in the commit msg,
with appropriate reasoning.

> 
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: can_clk
>> These are new...
> 
> These were already defined in the previous revision.

Any changes to the binding must be clearly expressed in the commit msg,
with appropriate reasoning.


> 
>>
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - interrupts
>>> +  - clocks
>>> +  - clock-names
>> Here the same. Each change to the binding should be explained (answer
>> to the: why) in commit msg.
> 
> Sure, I will include the reason for changes in commit message for the 
> next revision.
> 

Why am I repeating myself...


Best regards,
Krzysztof