Re: [DO NOT MERGE v6 22/37] dt-bindings: display: smi,sm501: SMI SM501 binding json-schema
flat view
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2024-01-15 09:52:49
Also in:
dri-devel, linux-clk, linux-devicetree, linux-ide, linux-pci, linux-renesas-soc, linux-serial, linux-sh, lkml
Hi Sato-san, On Tue, Jan 9, 2024 at 9:24 AM Yoshinori Sato [off-list ref] wrote:
Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
Thanks for your patch!
quoted hunk ↗ jump to hunk
--- .../bindings/display/smi,sm501.yaml | 417 ++++++++++++++++++ 1 file changed, 417 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/smi,sm501.yaml
Surely Documentation/devicetree/bindings/display/sm501fb.txt should be removed, too?
quoted hunk ↗ jump to hunk
--- /dev/null +++ b/Documentation/devicetree/bindings/display/smi,sm501.yaml
quoted hunk ↗ jump to hunk
+ crt: + type: object + description: CRT output control + properties: + edid: + $ref: /schemas/types.yaml#/definitions/uint8-array + description: | + verbatim EDID data block describing attached display. + Data from the detailed timing descriptor will be used to + program the display controller. + + smi,flags: + $ref: /schemas/types.yaml#/definitions/string-array + description: Display control flags. + items: + anyOf: + - const: use-init-done + - const: disable-at-exit + - const: use-hwcursor + - const: use-hwaccel
The "use-*" flags look like software policy, not hardware description, and thus do not belong in DT?
quoted hunk ↗ jump to hunk
+ - const: panel-no-fpen + - const: panel-no-vbiasen + - const: panel-inv-fpen + - const: panel-inv-vbiasen + maxItems: 8 + + bpp: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Color depth + + panel: + type: object + description: Panel output control + properties: + edid: + $ref: /schemas/types.yaml#/definitions/uint8-array + description: | + verbatim EDID data block describing attached display. + Data from the detailed timing descriptor will be used to + program the display controller. + + smi,flags: + $ref: /schemas/types.yaml#/definitions/string-array + description: Display control flags. + items: + anyOf: + - const: use-init-done + - const: disable-at-exit + - const: use-hwcursor + - const: use-hwaccel
The "use-*" flags look like software policy, not hardware description, and thus do not belong in DT?
quoted hunk ↗ jump to hunk
+ - const: panel-no-fpen + - const: panel-no-vbiasen + - const: panel-inv-fpen + - const: panel-inv-vbiasen + maxItems: 8 + + bpp: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Color depth + + smi,devices: + $ref: /schemas/types.yaml#/definitions/string-array + description: Select SM501 device functions. + items: + anyOf: + - const: usb-host + - const: usb-slave + - const: ssp0 + - const: ssp1 + - const: uart0 + - const: uart1 + - const: fbaccel + - const: ac97 + - const: i2s + - const: gpio + minItems: 1 + maxItems: 10
I think it would be better to have individual subnodes for the sub devices, with status = "ok"/"disabled". If you go that route, you do need some fallback code to handle the lack of subnodes in the existing user in arch/powerpc/boot/dts/charon.dts. BTW, why can sm501_pci_initdata get away with setting ".devices = SM501_USE_ALL"? Or, would it hurt to enable all subdevices unconditionally?
quoted hunk ↗ jump to hunk
+ + smi,mclk: + $ref: /schemas/types.yaml#/definitions/uint32 + description: mclk frequency. + + smi,m1xclk: + $ref: /schemas/types.yaml#/definitions/uint32 + description: m1xclk frequency.
These two should be clock specifiers (i.e. phandles pointing to clock nodes + optional clock indices).
quoted hunk ↗ jump to hunk
+ + misc-timing: + type: object + description: Miscellaneous Timing register values. + properties: + ex: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Extend bus holding time. + enum: [0, 16, 32, 48, 64, 80, 96, 112, 128, 144, 160, 176, 192, 208, 224, 240] + + xc: + $ref: /schemas/types.yaml#/definitions/string + description: Xscale clock input select. + items: + enum: + - internal-pll + - hclk + - gpio33
Software policy instead of hardware description again?
I am not familiar with how the SM501 works, so I cannot comment on
the other properties, but several of them look like they need rework.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds