Re: [PATCH 2/2] dt-bindings: trivial-devices: Add a reference to spi-peripheral-props.yaml

[Krzysztof Kozlowski <krzk@xxxxxxxxxx>]

On 30/08/2024 20:05, Rob Herring wrote:
> On Fri, Aug 30, 2024 at 04:17:02PM +0100, Conor Dooley wrote:
>> On Fri, Aug 30, 2024 at 12:05:20PM -0300, Fabio Estevam wrote:
>>> Hi Conor,
>>>
>>> On Fri, Aug 30, 2024 at 11:14 AM Conor Dooley <conor@xxxxxxxxxx> wrote:
>>>
>>>> Since those don't come from spi-peripheral-props, not really the correct
>>>> justification (although why they don't, I'm not sure). If you still saw
>>>> dtbs_check complaints after the first patch, I maybe the controller
>>>> schema is missing a reference to spi-controller.yaml?
>>>
>>> I changed the first patch as suggested:
>>>
>>> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>>> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>>> @@ -29,6 +29,10 @@ properties:
>>>      description:
>>>        Chip select used by the device.
>>>
>>> +  spi-cpha: true
>>> +
>>> +  spi-cpol: true
>>> +
>>>    spi-cs-high:
>>>      $ref: /schemas/types.yaml#/definitions/flag
>>>      description:
>>>
>>> spi-rockchip.yaml does reference spi-controller.yaml, but I still get
>>> dtbs_check complaints after the first patch.
>>>
>>> $ make CHECK_DTBS=y rockchip/rv1108-elgin-r1.dtb -j12
>>>   UPD     include/config/kernel.release
>>>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
>>>   DTC [C] arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dtb
>>> /home/fabio/linux-next/arch/arm/boot/dts/rockchip/rv1108-elgin-r1.dtb:
>>> display@0: 'spi-cpha', 'spi-cpol' do not match any of the regexes:
>>> 'pinctrl-[0-9]+'
>>> from schema $id: http://devicetree.org/schemas/trivial-devices.yaml#
>>>
>>> I would appreciate some suggestions on how to fix this warning.
>>
>> Ah, I think I suggested something garbage, because I misread the diff,
>> as my quoted mail evidences. I was really trying to suggest putting
>> spi-cpha: true
>> spi-cpol: true
>> in trivial-devices.yaml, but I didn't notice that the patch was to
>> spi-peripheral-props rather than trivial-devices. These properties are
>> defined (for reasons I don't quite understand) in spi-controller.yaml
>> and applied to children of the controller node by that binding and I
>> wanted to avoid the redefinition.
> 
> I steered Fabio wrong...
> 
> I think we originally had these in spi-peripheral-props, but then 
> decided they are properties of the device, not the controller. These 
> properties should really only be needed if the device supports different 
> modes. If what a device supports is fixed, then that can be implicit.
> 
> There's one other case I see with "dh,dhcom-board". So I guess add 
> spi-cpha and spi-cpol directly to trivial-devices.yaml.

I responded to v2 before reaching here. Are you sure that's intended?
It's almost equal to the patch here: all trivial devices will have it,
even if they do not need it. For a device requiring CPOL/CPHA, not
having it (or the reverse) is a bug in DTS and basically we would miss
such check for all trivial devices.

Best regards,
Krzysztof