Re: [PATCH 1/3] dt-bindings: display: bridge: add sam9x7-lvds compatible

[Dharma.B]

Hi Conor,

On 22/01/24 10:07 pm, Conor Dooley wrote:
> On Mon, Jan 22, 2024 at 04:51:16PM +0100, Krzysztof Kozlowski wrote:
>> On 22/01/2024 09:29, Dharma Balasubiramani wrote:
>>> Add the 'sam9x7-lvds' compatible binding, which describes the
>>> Low Voltage Differential Signaling (LVDS) Controller found on Microchip's
>>> sam9x7 series System-on-Chip (SoC) devices. This binding will be used to
>>> define the properties and configuration for the LVDS Controller in DT.
>>>
>>> Signed-off-by: Dharma Balasubiramani<dharma.b@xxxxxxxxxxxxx>
>>> ---
>>>   .../display/bridge/microchip,sam9x7-lvds.yaml | 59 +++++++++++++++++++
>>>   1 file changed, 59 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/display/bridge/microchip,sam9x7-lvds.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/bridge/microchip,sam9x7-lvds.yaml b/Documentation/devicetree/bindings/display/bridge/microchip,sam9x7-lvds.yaml
>>> new file mode 100644
>>> index 000000000000..8c2c5b858c85
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/display/bridge/microchip,sam9x7-lvds.yaml
>>> @@ -0,0 +1,59 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id:http://devicetree.org/schemas/display/bridge/microchip,sam9x7-lvds.yaml#
>>> +$schema:http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Microchip SAM9X7 LVDS Controller
>> What is the "X"?
>>
>>> +
>>> +maintainers:
>>> +  - Dharma Balasubiramani<dharma.b@xxxxxxxxxxxxx>
>>> +
>>> +description: |
>> Do not need '|' unless you need to preserve formatting.
>>
>>> +  The Low Voltage Differential Signaling Controller (LVDSC) manages data
>>> +  format conversion from the LCD Controller internal DPI bus to OpenLDI
>>> +  LVDS output signals. LVDSC functions include bit mapping, balanced mode
>>> +  management, and serializer.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: microchip,sam9x7-lvds
>> What is "x"? Wildcard? Then no, don't use it and instead use proper SoC
>> version number.
> These SoCs actually do have an x in their name. However, and I do always
> get confused here, the sam9x7 is a series of SoCs (the cover letter does
> say this) rather than a specific device.
> I think the series current consists of a sam9x70 sam9x72 and a sam9x75.
> The devices are largely similar, but I am not sure if the sam9x70
> supports LVDS at all. Having a compatible for the series does not seem
> correct to me.
Yes, you are correct. Only sam9x72 and sam9x75 have LVDS support, while 
sam9x70 does not. I will revise the compatibility to include both 
sam9x72 and sam9x75, as outlined below:

properties:
   compatible:
     enum:
       - microchip,sam9x72-lvds
       - microchip,sam9x75-lvds

Additionally, I will update the driver accordingly. Thank you.

-- 
With Best Regards,
Dharma B.
> 
> Cheers,
> Conor.