Re: [PATCH 05/11] dt-bindings: serio: add Arm PL050 DT schema

[Andre Przywara <andre.przywara@arm.com>]

On Thu, 28 Apr 2022 09:07:53 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

Hi Krzysztof,

many thanks for having a look and your comments (here and on the other
patches). I will fix them and send a new version ASAP.
One thing below:

> On 27/04/2022 13:25, Andre Przywara wrote:
> > The Arm PL050 "Keyboard/Mouse Interface" is an Arm system IP providing a
> > PS/2 compatible serial interface.
> > 
> > Add a simple DT schema binding, based on the TRM[1], the existing DTs and
> > the Linux driver.
> > 
> > [1] https://developer.arm.com/documentation/ddi0143/latest
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  .../devicetree/bindings/serio/amba-pl050.yaml | 67 +++++++++++++++++++
> >  1 file changed, 67 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/serio/amba-pl050.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/serio/amba-pl050.yaml b/Documentation/devicetree/bindings/serio/amba-pl050.yaml
> > new file mode 100644
> > index 0000000000000..9732a84550098
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/serio/amba-pl050.yaml  
> 
> Filename: vendor,device
> 
> > @@ -0,0 +1,67 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/serio/amba-pl050.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Arm Ltd. PrimeCell PL050 PS/2 Keyboard/Mouse Interface
> > +
> > +maintainers:
> > +  - Andre Przywara <andre.przywara@arm.com>
> > +
> > +description:
> > +  The Arm PrimeCell PS2 Keyboard/Mouse Interface (KMI) is an AMBA compliant
> > +  peripheral that can be used to implement a keyboard or mouse interface that
> > +  is IBM PS2 or AT compatible.
> > +
> > +# We need a select here so we don't match all nodes with 'arm,primecell'
> > +select:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        const: arm,pl050
> > +  required:
> > +    - compatible
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: arm,pl050
> > +      - const: arm,primecell
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: KMI reference clock, used to generate the bus timing
> > +      - description: APB register access clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: KMIREFCLK  
> 
> lowercase letters only

I am afraid this ship has sailed: the various DTs in the kernel tree use
it in that way, and the Linux driver insists on that spelling. So by
changing this we would break both the existing DT's compliance and also
existing Linux kernels.
So is lowercase something that is mandated by DT schema, or can we just
make an exception here?

Cheers,
Andre

> > +      - const: apb_pclk
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    kmi@70000 {  
> 
> Generic node names, so "serio".
> 
> > +            compatible = "arm,pl050", "arm,primecell";
> > +            reg = <0x070000 0x1000>;
> > +            interrupts = <8>;
> > +            clocks = <&mb_clk24mhz>, <&soc_smc50mhz>;
> > +            clock-names = "KMIREFCLK", "apb_pclk";
> > +    };
> > +
> > +...  
> 
> 
> Best regards,
> Krzysztof