Re: [PATCH v2 5/5] dt-bindings: pinctrl: Convert Atmel PIO3 pinctrl to json-schema

[<Manikandan.M@microchip.com>]

Hi Rob,

On 01/07/24 8:33 pm, Rob Herring wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Tue, Jun 25, 2024 at 10:05:25AM +0530, Manikandan Muralidharan wrote:
>> Convert Atmel PIO3 pinctrl binding document to DT schema format
>> json-schema.
>>
>> Signed-off-by: Manikandan Muralidharan <manikandan.m@microchip.com>
>> ---
>> changes in v2:
>>
>> - Fix bot errors by fixing issues in 4/5
>> - remove qoutes from $ref
>> ---
>>   .../bindings/pinctrl/atmel,at91-pinctrl.txt   | 178 ----------------
>>   .../pinctrl/atmel,at91rm9200-pinctrl.yaml     | 194 ++++++++++++++++++
>>   2 files changed, 194 insertions(+), 178 deletions(-)
>>   delete mode 100644 Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>>   create mode 100644 Documentation/devicetree/bindings/pinctrl/atmel,at91rm9200-pinctrl.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>> deleted file mode 100644
>> index 0aa1a53012d6..000000000000
>> --- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
>> +++ /dev/null
>> @@ -1,178 +0,0 @@
>> -* Atmel AT91 Pinmux Controller
>> -
>> -The AT91 Pinmux Controller, enables the IC
>> -to share one PAD to several functional blocks. The sharing is done by
>> -multiplexing the PAD input/output signals. For each PAD there are up to
>> -8 muxing options (called periph modes). Since different modules require
>> -different PAD settings (like pull up, keeper, etc) the controller controls
>> -also the PAD settings parameters.
>> -
>> -Please refer to pinctrl-bindings.txt in this directory for details of the
>> -common pinctrl bindings used by client devices, including the meaning of the
>> -phrase "pin configuration node".
>> -
>> +

[ .. ]

>> +patternProperties:
>> +  '^[a-z0-9-_]+$':
>> +    description:
>> +      A pinctrl node should contain at least one subnode representing the
>> +      pinctrl group available on the machine.
> 
>         type: object
> 
I tried to add the "type: object" for the above regex pattern and got 
hit by DTC_CHK errors,

pinctrl@fffff400: compatible: ['atmel,at91rm9200-pinctrl', 'simple-mfd'] 
is not of type 'object'
	from schema $id: 
http://devicetree.org/schemas/pinctrl/atmel,at91rm9200-pinctrl.yaml#
  pinctrl@fffff400: ranges: [[0, 1073872896, 12288]] is not of type 'object'
	from schema $id: 
http://devicetree.org/schemas/pinctrl/atmel,at91rm9200-pinctrl.yaml#

The pinctrl subnode string in the at91 DT's takes any format, 
combination of lowercase alphabets  or combination of alphabets + 
numbers or with some spl characters like '-' or '_' .
No fixed regex format is defined for pinctrl subnode group.
Referring to other SoC yaml files, I have removed the regex for pinmux 
subnode in v3 + addressing the other comments.
Kindly have a look, TIA.
>> +    additionalProperties: false
>> +
>> +    patternProperties:
>> +      '^[a-z0-9-_]+$':
>> +        type: object
>> +        properties:
>> +          atmel,pins:
>> +            $ref: /schemas/types.yaml#/definitions/uint32-matrix
>> +            description: |
>> +              Each entry consists of 4 integers and represents the pins
>> +              mux and config setting.The format is
>> +              atmel,pins = <PIN_BANK PIN_BANK_NUM PERIPH CONFIG>.
>> +              Supported pin number and mux varies for different SoCs, and
>> +              are defined in <include/dt-bindings/pinctrl/at91.h>.
>> +              items:
>> +                items:
>> +                  - description:
>> +                      Pin bank
>> +                  - description:
>> +                      Pin bank index
>> +                  - description:
>> +                      Peripheral function
>> +                  - description:
>> +                      Pad configuration
>> +
>> +        required:
>> +          - atmel,pins
>> +
>> +        additionalProperties: false
>> +
>> +  'gpio@[0-9a-f]*$':
> 
> '*' means 0 or more. You need '+' for 1 or more
> 
>> +    $ref: "/schemas/gpio/atmel,at91rm9200-gpio.yaml"
> 
> Don't need quotes. yamllint should have told you this.
> 
>> +    unevaluatedProperties: false
>> +
>> +allOf:
>> +  - $ref: pinctrl.yaml#
>> +
>> +required:
>> +  - compatible
>> +  - ranges
>> +  - "#address-cells"
>> +  - "#size-cells"
>> +  - atmel,mux-mask
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/at91.h>
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    #include <dt-bindings/pinctrl/at91.h>
>> +
>> +    pinctrl@fffff400 {
>> +      #address-cells = <1>;
>> +      #size-cells = <1>;
>> +      compatible = "atmel,at91rm9200-pinctrl", "simple-mfd";
>> +      ranges = <0xfffff400 0xfffff400 0x600>;
>> +
>> +      atmel,mux-mask = <
>> +        /*    A         B     */
>> +        0xffffffff 0xffc00c3b  /* pioA */
>> +        0xffffffff 0x7fff3ccf  /* pioB */
>> +        0xffffffff 0x007fffff  /* pioC */
>> +        >;
>> +
>> +      dbgu {
>> +        pinctrl_dbgu: dbgu-0 {
>> +                        atmel,pins =
>> +                          <AT91_PIOB 14 AT91_PERIPH_A AT91_PINCTRL_PULL_UP
>> +                           AT91_PIOB 15 AT91_PERIPH_A AT91_PINCTRL_NONE>;
>> +        };
>> +      };
>> +
>> +      pioA: gpio@fffff400 {
>> +              compatible = "atmel,at91rm9200-gpio";
>> +              reg = <0xfffff400 0x200>;
>> +              interrupts = <2 IRQ_TYPE_LEVEL_HIGH 1>;
>> +              #gpio-cells = <2>;
>> +              gpio-controller;
>> +              interrupt-controller;
>> +              #interrupt-cells = <2>;
>> +              clocks = <&pmc PMC_TYPE_PERIPHERAL 2>;
>> +      };
>> +    };
>> +...
>> --
>> 2.25.1
>>

-- 
Thanks and Regards,
Manikandan M.