Message ID | cover.1704788539.git.ysato@users.sourceforge.jp |
---|---|
Headers | show |
Series | Device Tree support for SH7751 based board | expand |
On Wed, Jan 10, 2024 at 04:11:44PM +0000, Conor Dooley wrote: > On Wed, Jan 10, 2024 at 12:23:37PM +0100, Geert Uytterhoeven wrote: > > Hi Conor, > > > > On Tue, Jan 9, 2024 at 7:06 PM Conor Dooley <conor@kernel.org> wrote: > > > On Tue, Jan 09, 2024 at 05:23:23PM +0900, Yoshinori Sato wrote: > > > > Add Silicon Mortion Technology Corporation > > > > Motion > > > > > > https://www.siliconmotion.com/ > > > > > > > > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> > > > > --- > > > > Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml > > > > index 94ed63d9f7de..a338bdd743ab 100644 > > > > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml > > > > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml > > > > @@ -1283,6 +1283,8 @@ patternProperties: > > > > description: Skyworks Solutions, Inc. > > > > "^smartlabs,.*": > > > > description: SmartLabs LLC > > > > + "^smi,.*": > > > > + description: Silicon Motion Technology Corporation > > > > > > How come "smi" is used for a company with this name? > > > Why is it not something like SMTC? There's probably some history here > > > that I am unaware of. > > > > See Documentation/devicetree/bindings/display/sm501fb.txt > > The stock ticker is "SIMO", though. > > https://www.nasdaq.com/market-activity/stocks/simo > > If there's an existing user, there's little reason to stand in the way I > think. > Acked-by: Conor Dooley <conor.dooley@microchip.com> Or reason not to apply, so I'm applying this. BTW, 'RFC' is the standard way to say 'DO NOT MERGE'. Rob
Hi Sato-san, On Tue, Jan 9, 2024 at 9:24 AM Yoshinori Sato <ysato@users.sourceforge.jp> wrote: > Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp> Thanks for your patch! > --- > .../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? > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/smi,sm501.yaml > + 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? > + - 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? > + - 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? > + > + 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). > + > + 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