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@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 Answered below > >> + >> +maintainers: >> + - Dharma Balasubiramani <dharma.b@xxxxxxxxxxxxx> >> + >> +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). -- With Best Regards, Dharma B. > >> + clocks = <&pmc PMC_TYPE_PERIPHERAL 56>; >> + clock-names = "pclk"; >> + }; > > Best regards, > Krzysztof >