Re: [PATCH v3] dt-bindings: display: atmel,lcdc: convert to dtschema

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dharma, all,

On 06/03/2024 at 15:35, Dharma B - I70843 wrote:

On 05/03/24 3:31 am, Rob Herring wrote:
On Mon, Mar 04, 2024 at 08:00:03PM +0530, Dharma Balasubiramani wrote:
Convert the atmel,lcdc bindings to DT schema.
Changes during conversion: add missing clocks and clock-names properties.

Signed-off-by: Dharma Balasubiramani <dharma.b@xxxxxxxxxxxxx>
---
This patch converts the existing lcdc display text binding to JSON schema.
The binding is split into two namely
lcdc.yaml
- Holds the frame buffer properties
lcdc-display.yaml
- Holds the display panel properties which is a phandle to the display
property in lcdc fb node.

These bindings are tested using the following command.
'make DT_CHECKER_FLAGS=-m dt_binding_check'
---
Changes in v3:
- Remove the generic property "bits-per-pixel"
- Link to v2: https://lore.kernel.org/r/20240304-lcdc-fb-v2-1-a14b463c157a@xxxxxxxxxxxxx

Changes in v2:
- Run checkpatch and remove whitespace errors.
- Add the standard interrupt flags.
- Split the binding into two, namely lcdc.yaml and lcdc-display.yaml.
- Link to v1: https://lore.kernel.org/r/20240223-lcdc-fb-v1-1-4c64cb6277df@xxxxxxxxxxxxx
---
   .../bindings/display/atmel,lcdc-display.yaml       | 97 ++++++++++++++++++++++
   .../devicetree/bindings/display/atmel,lcdc.txt     | 87 -------------------
   .../devicetree/bindings/display/atmel,lcdc.yaml    | 70 ++++++++++++++++
   3 files changed, 167 insertions(+), 87 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/atmel,lcdc-display.yaml b/Documentation/devicetree/bindings/display/atmel,lcdc-display.yaml
new file mode 100644
index 000000000000..5e0b706d695d

[..]

+  atmel,lcd-wiring-mode:
+    $ref: /schemas/types.yaml#/definitions/non-unique-string-array

Isn't this just a single string rather than an array?

+    description: lcd wiring mode "RGB" or "BRG"

enum:
    - RGB
    - BRG

No BGR?

In the current driver implementation, we have interpreted the wiring
mode represented by ATMEL_LCDC_WIRING_BGR as 'BRG' in the array
atmel_lcdfb_wiring_modes. Considering conventional color representation,
would it be appropriate to consider modifying the existing driver to use
the 'BGR' string instead of 'BRG' for better alignment with standard
naming conventions?

This "BRG" thing is definitively a typo. We never had such thing as a BRG color wiring but did have BGR wiring mode.

static const char *atmel_lcdfb_wiring_modes[] = {
          [ATMEL_LCDC_WIRING_BGR] = "BRG",
          [ATMEL_LCDC_WIRING_RGB] = "RGB",

The thing is that we have one DT using that:
arch/arm/boot/dts/at91sam9261ek.dts

So, either I would leave it like that: it's only old product using it.
Or just focus on first character in the string so that it works for both "BRG" or "BGR", and maintains the backward compatibility.

Regards,
  Nicolas


};



But wait, the example shows the value is '1'. That should fail testing.
It didn't, but I've now fixed that.

It seems correctly configured in our dts files but didn't noticed the
same in the bindings example, thanks for letting me know, I will correct
it in the next revision.





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux