diff mbox series

[DO,NOT,MERGE,v5,22/37] dt-bindings: display: smi,sm501: SMI SM501 binding json-schema

Message ID f671beae8a8ebfd361f4c903bccce713135a169f.1701768028.git.ysato@users.sourceforge.jp
State Changes Requested
Headers show
Series Device Tree support for SH7751 based board | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dt-meta-schema fail build log

Commit Message

Yoshinori Sato Dec. 5, 2023, 9:45 a.m. UTC
Define SM501 functions and modes.

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 .../bindings/display/smi,sm501.yaml           | 134 ++++++++++++++++++
 include/dt-bindings/display/sm501.h           |  25 ++++
 2 files changed, 159 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/smi,sm501.yaml
 create mode 100644 include/dt-bindings/display/sm501.h

Comments

Rob Herring (Arm) Dec. 5, 2023, 1:36 p.m. UTC | #1
On Tue, 05 Dec 2023 18:45:41 +0900, Yoshinori Sato wrote:
> Define SM501 functions and modes.
> 
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>  .../bindings/display/smi,sm501.yaml           | 134 ++++++++++++++++++
>  include/dt-bindings/display/sm501.h           |  25 ++++
>  2 files changed, 159 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/smi,sm501.yaml
>  create mode 100644 include/dt-bindings/display/sm501.h
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/smi,sm501.yaml: interrupt-name: missing type definition
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/smi,sm501.yaml: edid: missing type definition
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/smi,sm501.yaml: crt: missing type definition

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/f671beae8a8ebfd361f4c903bccce713135a169f.1701768028.git.ysato@users.sourceforge.jp

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Arnd Bergmann Dec. 5, 2023, 1:36 p.m. UTC | #2
On Tue, Dec 5, 2023, at 10:45, Yoshinori Sato wrote:
> Define SM501 functions and modes.
>
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> ---
>  .../bindings/display/smi,sm501.yaml           | 134 ++++++++++++++++++
>  include/dt-bindings/display/sm501.h           |  25 ++++

It looks like we already have a binding at
Documentation/devicetree/bindings/display/sm501fb.txt

> +  little-endian:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: available on big endian systems, to set different 
> foreign endian.
> +  big-endian:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: available on little endian systems, to set different 
> foreign endian.
> +
> +  swap-fb-endian:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: swap framebuffer byteorder.

Why do you need both the "swap" and the specific little/big
properties?

> +  crt:
> +    description: CRT output control
> +
> +  panel:
> +    description: Panel output control

What type are these?

> +  smi,misc-timing:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Miscellaneous Timing reg value.
> +
> +  smi,misc-control:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Miscellaneous Control reg value.
> +
> +  smi,gpio-low:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: GPIO0 to 31 Control reg value.
> +
> +  smi,gpio-high:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: GPIO32 to 63 Control reg value.

Register values should generally not go into DT


> +
> +  smi,gpio-i2c:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 5
> +    description: |
> +      GPIO I2C bus number
> +      1st field - I2C bus number
> +      2nd Field - GPIO SDA
> +      3rd Field - GPIO SCL
> +      4th Field - Timeout
> +      5th Field - udelay

Instead of a bus number and other fields, I think
this should reference an i2c device.

      Arnd
Krzysztof Kozlowski Dec. 5, 2023, 3:56 p.m. UTC | #3
On 05/12/2023 14:36, Arnd Bergmann wrote:
> On Tue, Dec 5, 2023, at 10:45, Yoshinori Sato wrote:
>> Define SM501 functions and modes.
>>
>> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
>> ---
>>  .../bindings/display/smi,sm501.yaml           | 134 ++++++++++++++++++
>>  include/dt-bindings/display/sm501.h           |  25 ++++
> 
> It looks like we already have a binding at
> Documentation/devicetree/bindings/display/sm501fb.txt

Which I asked to do three months ago. Several comments were simply
ignored and never responded to.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/smi,sm501.yaml b/Documentation/devicetree/bindings/display/smi,sm501.yaml
new file mode 100644
index 000000000000..df46600b8d4a
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/smi,sm501.yaml
@@ -0,0 +1,134 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/smi,sm501.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Silicon Motion SM501 Mobile Multimedia Companion Chip
+
+maintainers:
+  - Yoshinori Sato <ysato@user.sourceforge.jp>
+
+description: |
+  These DT bindings describe the SM501.
+
+properties:
+  compatible:
+    const:
+      smi,sm501
+
+  reg:
+    maxItems: 2
+    description: |
+     First entry: System Configuration register
+     Second entry: IO space (Display Controller register)
+
+  interrupts:
+    description: SM501 interrupt to the cpu should be described here.
+
+  interrupt-name: true
+
+  mode:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: select a video mode
+
+  edid:
+    description: |
+      verbatim EDID data block describing attached display.
+      Data from the detailed timing descriptor will be used to
+      program the display controller.
+
+  little-endian:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: available on big endian systems, to set different foreign endian.
+  big-endian:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: available on little endian systems, to set different foreign endian.
+
+  swap-fb-endian:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: swap framebuffer byteorder.
+
+  route-crt-panel:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: Panel output merge to CRT.
+
+  crt:
+    description: CRT output control
+
+  panel:
+    description: Panel output control
+
+  bpp:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Color depth
+
+  smi,flags:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Display control flags.
+
+  smi,devices:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: SM501 device function select.
+
+  smi,mclk:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: mclk frequency.
+
+  smi,m1xclk:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: m1xclk frequency.
+
+  smi,misc-timing:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Miscellaneous Timing reg value.
+
+  smi,misc-control:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Miscellaneous Control reg value.
+
+  smi,gpio-low:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: GPIO0 to 31 Control reg value.
+
+  smi,gpio-high:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: GPIO32 to 63 Control reg value.
+
+  smi,gpio-i2c:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 5
+    description: |
+      GPIO I2C bus number
+      1st field - I2C bus number
+      2nd Field - GPIO SDA
+      3rd Field - GPIO SCL
+      4th Field - Timeout
+      5th Field - udelay
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+
+examples:
+  # MPC5200
+  - |
+    display@1,0 {
+        compatible = "smi,sm501";
+        reg = <0x00000000 0x00800000
+               0x03e00000 0x00200000>;
+        interrupts = <1 1 3>;
+        mode = "640x480-32@60";
+        edid = [00 ff ff ff ff ff ff 00 00 00 00 00 00 00 00 00
+                00 00 01 04 00 00 00 00 00 00 00 00 00 00 00 00
+                00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+                00 00 00 00 00 00 f0 0a 80 fb 20 e0 25 10 32 60
+                02 00 00 00 00 00 00 06 00 00 00 00 00 00 00 00
+                00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+                00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+                00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 bd];
+    };
diff --git a/include/dt-bindings/display/sm501.h b/include/dt-bindings/display/sm501.h
new file mode 100644
index 000000000000..1be8490d7635
--- /dev/null
+++ b/include/dt-bindings/display/sm501.h
@@ -0,0 +1,25 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+
+/* Platform data definitions */
+
+#define SM501FB_FLAG_USE_INIT_MODE	(1<<0)
+#define SM501FB_FLAG_DISABLE_AT_EXIT	(1<<1)
+#define SM501FB_FLAG_USE_HWCURSOR	(1<<2)
+#define SM501FB_FLAG_USE_HWACCEL	(1<<3)
+#define SM501FB_FLAG_PANEL_NO_FPEN	(1<<4)
+#define SM501FB_FLAG_PANEL_NO_VBIASEN	(1<<5)
+#define SM501FB_FLAG_PANEL_INV_FPEN	(1<<6)
+#define SM501FB_FLAG_PANEL_INV_VBIASEN	(1<<7)
+
+#define SM501_USE_USB_HOST	(1<<0)
+#define SM501_USE_USB_SLAVE	(1<<1)
+#define SM501_USE_SSP0		(1<<2)
+#define SM501_USE_SSP1		(1<<3)
+#define SM501_USE_UART0		(1<<4)
+#define SM501_USE_UART1		(1<<5)
+#define SM501_USE_FBACCEL	(1<<6)
+#define SM501_USE_AC97		(1<<7)
+#define SM501_USE_I2S		(1<<8)
+#define SM501_USE_GPIO		(1<<9)
+
+#define SM501_USE_ALL		(0xffffffff)