| Message ID | 20240122082947.21645-2-dharma.b@microchip.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | LVDS Controller Support for SAM9X7 SoC | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
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@microchip.com> > --- > .../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@microchip.com> > + > +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. > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + items: > + - description: Peripheral Bus Clock > + > + clock-names: > + items: > + - const: pclk > + - const: gclk > + minItems: 1 No, you just said you have one clock. > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - clock-names > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/at91.h> > + #include <dt-bindings/dma/at91.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> This header is not used. Include only used ones (and missing interrupt). > + > + lvds-controller@f8060000 { > + compatible = "microchip,sam9x7-lvds"; > + reg = <0xf8060000 0x100>; > + interrupts = <56 IRQ_TYPE_LEVEL_HIGH 0>; What is "0"? > + clocks = <&pmc PMC_TYPE_PERIPHERAL 56>; > + clock-names = "pclk"; > + }; Best regards, Krzysztof
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@microchip.com> > > --- > > .../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@microchip.com> > > + > > +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. Cheers, Conor.
Hi Krzysztof, On 22/01/24 9:21 pm, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > 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@microchip.com> >> --- >> .../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 Answered below > >> + >> +maintainers: >> + - Dharma Balasubiramani <dharma.b@microchip.com> >> + >> +description: | > > Do not need '|' unless you need to preserve formatting. Sure, I will drop it. > >> + 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. The term 'X' doesn't serve as a wildcard; rather, it directly represents the name of the SoC series, I should use sam9x75,sam9x72 instead of sam9x7. > >> + >> + reg: >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + clocks: >> + items: >> + - description: Peripheral Bus Clock >> + >> + clock-names: >> + items: >> + - const: pclk >> + - const: gclk >> + minItems: 1 > > No, you just said you have one clock. Certainly, I need to exclude the gclk. Thanks. > >> + >> +required: >> + - compatible >> + - reg >> + - interrupts >> + - clocks >> + - clock-names >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/clock/at91.h> >> + #include <dt-bindings/dma/at91.h> >> + #include <dt-bindings/interrupt-controller/arm-gic.h> > > This header is not used. Include only used ones (and missing interrupt). > >> + >> + lvds-controller@f8060000 { >> + compatible = "microchip,sam9x7-lvds"; >> + reg = <0xf8060000 0x100>; >> + interrupts = <56 IRQ_TYPE_LEVEL_HIGH 0>; > > What is "0"? Please refer "Documentation/devicetree/bindings/interrupt-controller/atmel,aic.txt" The third cell is used to specify the irq priority from 0 (lowest) to 7(highest).
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@microchip.com> >>> --- >>> .../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@microchip.com> >>> + >>> +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.
On Tue, Jan 23, 2024 at 03:39:13AM +0000, Dharma.B@microchip.com wrote: > 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@microchip.com> > >>> --- > >>> .../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@microchip.com> > >>> + > >>> +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 I would presume these 2 are the same, but the above implies they aren't. I think what you had is fine assuming these are all fundamentally the same part with just packaging or fused off h/w differences. Rob
On 31/01/24 12:42 am, Rob Herring wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Tue, Jan 23, 2024 at 03:39:13AM +0000, Dharma.B@microchip.com wrote: >> 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@microchip.com> >>>>> --- >>>>> .../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@microchip.com> >>>>> + >>>>> +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 > > I would presume these 2 are the same, but the above implies they > aren't. I think what you had is fine assuming these are all > fundamentally the same part with just packaging or fused off h/w > differences. Yes, so is it okay to have compatible for a series? Shall I go ahead with " compatible: const: microchip,sam9x7-lvds " itself?
On 01/02/2024 05:10, Dharma.B@microchip.com wrote: > On 31/01/24 12:42 am, Rob Herring wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> On Tue, Jan 23, 2024 at 03:39:13AM +0000, Dharma.B@microchip.com wrote: >>> 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@microchip.com> >>>>>> --- >>>>>> .../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@microchip.com> >>>>>> + >>>>>> +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 >> >> I would presume these 2 are the same, but the above implies they >> aren't. I think what you had is fine assuming these are all >> fundamentally the same part with just packaging or fused off h/w >> differences. > > Yes, so is it okay to have compatible for a series? Shall I go ahead with > " > compatible: > const: microchip,sam9x7-lvds You said 9x70, which would match such 9x7 "series", is different, so I still think series should not be used. I don't know much about Microchip naming scheme, so this x is always confusing. However if these are the same, maybe just use sam9x72? Best regards, Krzysztof
On 01/02/24 1:09 pm, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 01/02/2024 05:10, Dharma.B@microchip.com wrote: >> On 31/01/24 12:42 am, Rob Herring wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> On Tue, Jan 23, 2024 at 03:39:13AM +0000, Dharma.B@microchip.com wrote: >>>> 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@microchip.com> >>>>>>> --- >>>>>>> .../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@microchip.com> >>>>>>> + >>>>>>> +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 >>> >>> I would presume these 2 are the same, but the above implies they >>> aren't. I think what you had is fine assuming these are all >>> fundamentally the same part with just packaging or fused off h/w >>> differences. >> >> Yes, so is it okay to have compatible for a series? Shall I go ahead with >> " >> compatible: >> const: microchip,sam9x7-lvds > > You said 9x70, which would match such 9x7 "series", is different, so I > still think series should not be used. I don't know much about Microchip > naming scheme, so this x is always confusing. However if these are the > same, maybe just use sam9x72? sam9x75 is the first board to be available publicly; hence, I shall use the "microchip,sam9x75-lvds" compatible and reuse the same in other boards that (features same IP) will come later. > > Best regards, > Krzysztof >
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 + +maintainers: + - Dharma Balasubiramani <dharma.b@microchip.com> + +description: | + 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 + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + items: + - description: Peripheral Bus Clock + + clock-names: + items: + - const: pclk + - const: gclk + minItems: 1 + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/at91.h> + #include <dt-bindings/dma/at91.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + + lvds-controller@f8060000 { + compatible = "microchip,sam9x7-lvds"; + reg = <0xf8060000 0x100>; + interrupts = <56 IRQ_TYPE_LEVEL_HIGH 0>; + clocks = <&pmc PMC_TYPE_PERIPHERAL 56>; + clock-names = "pclk"; + };
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@microchip.com> --- .../display/bridge/microchip,sam9x7-lvds.yaml | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/microchip,sam9x7-lvds.yaml