[PATCH v2 0/5] drm/solomon: Add SSD130x OLED displays SPI support

[PATCH v2 0/5] drm/solomon: Add SSD130x OLED displays SPI support

[Javier Martinez Canillas]

Hello,

This series adds a ssd130x-spi driver that provides a 4-wire SPI transport
support for SSD130x OLED controllers that can be accessed through a SPI.

The driver is quite similar to existing ssd130x-i2c driver that is used by
I2C controllers, but there is a difference in the protocol used by SSD130x
depending on the transport used. The details are in patch #4 description.

Patch #1 just makes the current ssd130x-i2c compatible strings in the DT
binding to be deprecated, and add new ones that don't have an "fb-i2c".

Patch #2 extends the DT binding with the properties needed to support SPI.

Patch #3 adds the new compatible strings to the OF device ID table in the
ssd130x-i2c DRM driver and deprecate the old ones.

Patch #4 moves the device info for the different SSD130x variants from
the ssd130x-i2c transport driver to the ssd130x core driver.

Finally patch #5 adds the ssd130x-spi DRM driver for the OLED controllers
that come with a 4-wire SPI interface, instead of an I2C interface.

This is a v2 that addresses the issues pointed out in v1.

Best regards,
Javier

Changes in v2:
- Drop the -i2c suffixes from the compatible strings too (Geert Uytterhoeven).
- Don't add compatible strings with an "-spi" suffix (Geert Uytterhoeven).
- Drop ssd13x_variant_to_info() and just use the array index (Neil Armstrong).
- Add Mark Brown's Acked-by tag to all patches.

Javier Martinez Canillas (5):
  dt-bindings: display: ssd1307fb: Deprecate "-i2c" compatible strings
  dt-bindings: display: ssd1307fb: Extend schema for SPI controllers
  drm/solomon: Add ssd130x new compatible strings and deprecate old
    ones.
  drm/solomon: Move device info from ssd130x-i2c to the core driver
  drm/solomon: Add SSD130x OLED displays SPI support

 .../bindings/display/solomon,ssd1307fb.yaml   |  74 +++++--
 drivers/gpu/drm/solomon/Kconfig               |   9 +
 drivers/gpu/drm/solomon/Makefile              |   1 +
 drivers/gpu/drm/solomon/ssd130x-i2c.c         |  64 +++---
 drivers/gpu/drm/solomon/ssd130x-spi.c         | 184 ++++++++++++++++++
 drivers/gpu/drm/solomon/ssd130x.c             |  45 ++++-
 drivers/gpu/drm/solomon/ssd130x.h             |  12 ++
 7 files changed, 332 insertions(+), 57 deletions(-)
 create mode 100644 drivers/gpu/drm/solomon/ssd130x-spi.c

-- 
2.35.1

[PATCH v2 0/5] drm/solomon: Add SSD130x OLED displays SPI support

[Javier Martinez Canillas]

Hello,

This series adds a ssd130x-spi driver that provides a 4-wire SPI transport
support for SSD130x OLED controllers that can be accessed through a SPI.

The driver is quite similar to existing ssd130x-i2c driver that is used by
I2C controllers, but there is a difference in the protocol used by SSD130x
depending on the transport used. The details are in patch #4 description.

Patch #1 just makes the current ssd130x-i2c compatible strings in the DT
binding to be deprecated, and add new ones that don't have an "fb-i2c".

Patch #2 extends the DT binding with the properties needed to support SPI.

Patch #3 adds the new compatible strings to the OF device ID table in the
ssd130x-i2c DRM driver and deprecate the old ones.

Patch #4 moves the device info for the different SSD130x variants from
the ssd130x-i2c transport driver to the ssd130x core driver.

Finally patch #5 adds the ssd130x-spi DRM driver for the OLED controllers
that come with a 4-wire SPI interface, instead of an I2C interface.

This is a v2 that addresses the issues pointed out in v1.

Best regards,
Javier

Changes in v2:
- Drop the -i2c suffixes from the compatible strings too (Geert Uytterhoeven).
- Don't add compatible strings with an "-spi" suffix (Geert Uytterhoeven).
- Drop ssd13x_variant_to_info() and just use the array index (Neil Armstrong).
- Add Mark Brown's Acked-by tag to all patches.

Javier Martinez Canillas (5):
  dt-bindings: display: ssd1307fb: Deprecate "-i2c" compatible strings
  dt-bindings: display: ssd1307fb: Extend schema for SPI controllers
  drm/solomon: Add ssd130x new compatible strings and deprecate old
    ones.
  drm/solomon: Move device info from ssd130x-i2c to the core driver
  drm/solomon: Add SSD130x OLED displays SPI support

 .../bindings/display/solomon,ssd1307fb.yaml   |  74 +++++--
 drivers/gpu/drm/solomon/Kconfig               |   9 +
 drivers/gpu/drm/solomon/Makefile              |   1 +
 drivers/gpu/drm/solomon/ssd130x-i2c.c         |  64 +++---
 drivers/gpu/drm/solomon/ssd130x-spi.c         | 184 ++++++++++++++++++
 drivers/gpu/drm/solomon/ssd130x.c             |  45 ++++-
 drivers/gpu/drm/solomon/ssd130x.h             |  12 ++
 7 files changed, 332 insertions(+), 57 deletions(-)
 create mode 100644 drivers/gpu/drm/solomon/ssd130x-spi.c

-- 
2.35.1

[PATCH v2 1/5] dt-bindings: display: ssd1307fb: Deprecate "-i2c" compatible strings

[Javier Martinez Canillas]

The current compatible strings for SSD130x I2C controllers contain both an
"fb" and "-i2c" suffixes. It seems to indicate that are for a fbdev driver
and also that are for devices that can be accessed over an I2C bus.

But a DT is supposed to describe the hardware and not Linux implementation
details. So let's deprecate those compatible strings and add new ones that
only contain the vendor and device name, without any of these suffixes.

These will just describe the device and can be matched by both I2C and SPI
DRM drivers.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Mark Brown <broonie@kernel.org>
---

Changes in v2:
- Drop the -i2c suffixes from the compatible strings too (Geert Uytterhoeven).

 .../bindings/display/solomon,ssd1307fb.yaml   | 37 ++++++++++++-------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
index ade61d502edd..6b9d0c72739a 100644
--- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
+++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
@@ -12,12 +12,23 @@ maintainers:
 
 properties:
   compatible:
-    enum:
-      - sinowealth,sh1106-i2c
-      - solomon,ssd1305fb-i2c
-      - solomon,ssd1306fb-i2c
-      - solomon,ssd1307fb-i2c
-      - solomon,ssd1309fb-i2c
+    oneOf:
+      # Deprecated compatible strings
+      - items:
+          - enum:
+              - sinowealth,sh1106-i2c
+              - solomon,ssd1305fb-i2c
+              - solomon,ssd1306fb-i2c
+              - solomon,ssd1307fb-i2c
+              - solomon,ssd1309fb-i2c
+        deprecated: true
+      - items:
+          - enum:
+              - sinowealth,sh1106
+              - solomon,ssd1305
+              - solomon,ssd1306
+              - solomon,ssd1307
+              - solomon,ssd1309
 
   reg:
     maxItems: 1
@@ -136,7 +147,7 @@ allOf:
       properties:
         compatible:
           contains:
-            const: sinowealth,sh1106-i2c
+            const: sinowealth,sh1106
     then:
       properties:
         solomon,dclk-div:
@@ -148,7 +159,7 @@ allOf:
       properties:
         compatible:
           contains:
-            const: solomon,ssd1305fb-i2c
+            const: solomon,ssd1305
     then:
       properties:
         solomon,dclk-div:
@@ -160,7 +171,7 @@ allOf:
       properties:
         compatible:
           contains:
-            const: solomon,ssd1306fb-i2c
+            const: solomon,ssd1306
     then:
       properties:
         solomon,dclk-div:
@@ -172,7 +183,7 @@ allOf:
       properties:
         compatible:
           contains:
-            const: solomon,ssd1307fb-i2c
+            const: solomon,ssd1307
     then:
       properties:
         solomon,dclk-div:
@@ -186,7 +197,7 @@ allOf:
       properties:
         compatible:
           contains:
-            const: solomon,ssd1309fb-i2c
+            const: solomon,ssd1309
     then:
       properties:
         solomon,dclk-div:
@@ -203,14 +214,14 @@ examples:
             #size-cells = <0>;
 
             ssd1307: oled@3c {
-                    compatible = "solomon,ssd1307fb-i2c";
+                    compatible = "solomon,ssd1307";
                     reg = <0x3c>;
                     pwms = <&pwm 4 3000>;
                     reset-gpios = <&gpio2 7>;
             };
 
             ssd1306: oled@3d {
-                    compatible = "solomon,ssd1306fb-i2c";
+                    compatible = "solomon,ssd1306";
                     reg = <0x3c>;
                     pwms = <&pwm 4 3000>;
                     reset-gpios = <&gpio2 7>;
-- 
2.35.1

[PATCH v2 1/5] dt-bindings: display: ssd1307fb: Deprecate "-i2c" compatible strings

[Javier Martinez Canillas]

The current compatible strings for SSD130x I2C controllers contain both an
"fb" and "-i2c" suffixes. It seems to indicate that are for a fbdev driver
and also that are for devices that can be accessed over an I2C bus.

But a DT is supposed to describe the hardware and not Linux implementation
details. So let's deprecate those compatible strings and add new ones that
only contain the vendor and device name, without any of these suffixes.

These will just describe the device and can be matched by both I2C and SPI
DRM drivers.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Mark Brown <broonie@kernel.org>
---

Changes in v2:
- Drop the -i2c suffixes from the compatible strings too (Geert Uytterhoeven).

 .../bindings/display/solomon,ssd1307fb.yaml   | 37 ++++++++++++-------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
index ade61d502edd..6b9d0c72739a 100644
--- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
+++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
@@ -12,12 +12,23 @@ maintainers:
 
 properties:
   compatible:
-    enum:
-      - sinowealth,sh1106-i2c
-      - solomon,ssd1305fb-i2c
-      - solomon,ssd1306fb-i2c
-      - solomon,ssd1307fb-i2c
-      - solomon,ssd1309fb-i2c
+    oneOf:
+      # Deprecated compatible strings
+      - items:
+          - enum:
+              - sinowealth,sh1106-i2c
+              - solomon,ssd1305fb-i2c
+              - solomon,ssd1306fb-i2c
+              - solomon,ssd1307fb-i2c
+              - solomon,ssd1309fb-i2c
+        deprecated: true
+      - items:
+          - enum:
+              - sinowealth,sh1106
+              - solomon,ssd1305
+              - solomon,ssd1306
+              - solomon,ssd1307
+              - solomon,ssd1309
 
   reg:
     maxItems: 1
@@ -136,7 +147,7 @@ allOf:
       properties:
         compatible:
           contains:
-            const: sinowealth,sh1106-i2c
+            const: sinowealth,sh1106
     then:
       properties:
         solomon,dclk-div:
@@ -148,7 +159,7 @@ allOf:
       properties:
         compatible:
           contains:
-            const: solomon,ssd1305fb-i2c
+            const: solomon,ssd1305
     then:
       properties:
         solomon,dclk-div:
@@ -160,7 +171,7 @@ allOf:
       properties:
         compatible:
           contains:
-            const: solomon,ssd1306fb-i2c
+            const: solomon,ssd1306
     then:
       properties:
         solomon,dclk-div:
@@ -172,7 +183,7 @@ allOf:
       properties:
         compatible:
           contains:
-            const: solomon,ssd1307fb-i2c
+            const: solomon,ssd1307
     then:
       properties:
         solomon,dclk-div:
@@ -186,7 +197,7 @@ allOf:
       properties:
         compatible:
           contains:
-            const: solomon,ssd1309fb-i2c
+            const: solomon,ssd1309
     then:
       properties:
         solomon,dclk-div:
@@ -203,14 +214,14 @@ examples:
             #size-cells = <0>;
 
             ssd1307: oled@3c {
-                    compatible = "solomon,ssd1307fb-i2c";
+                    compatible = "solomon,ssd1307";
                     reg = <0x3c>;
                     pwms = <&pwm 4 3000>;
                     reset-gpios = <&gpio2 7>;
             };
 
             ssd1306: oled@3d {
-                    compatible = "solomon,ssd1306fb-i2c";
+                    compatible = "solomon,ssd1306";
                     reg = <0x3c>;
                     pwms = <&pwm 4 3000>;
                     reset-gpios = <&gpio2 7>;
-- 
2.35.1

[PATCH v2 2/5] dt-bindings: display: ssd1307fb: Extend schema for SPI controllers

[Javier Martinez Canillas]

The Solomon SSD130x OLED displays can either have an I2C or SPI interface,
add to the schema the properties and examples for OLED devices under SPI.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Mark Brown <broonie@kernel.org>
---

Changes in v2:
- Don't add compatible strings with an "-spi" suffix (Geert Uytterhoeven).

 .../bindings/display/solomon,ssd1307fb.yaml   | 37 ++++++++++++++++++-
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
index 6b9d0c72739a..b58d2d7bd8c1 100644
--- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
+++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
@@ -39,9 +39,14 @@ properties:
   reset-gpios:
     maxItems: 1
 
+  dc-gpios:
+    maxItems: 1
+
   vbat-supply:
     description: The supply for VBAT
 
+  spi-max-frequency: true
+
   solomon,height:
     $ref: /schemas/types.yaml#/definitions/uint32
     default: 16
@@ -160,6 +165,7 @@ allOf:
         compatible:
           contains:
             const: solomon,ssd1305
+
     then:
       properties:
         solomon,dclk-div:
@@ -213,14 +219,14 @@ examples:
             #address-cells = <1>;
             #size-cells = <0>;
 
-            ssd1307: oled@3c {
+            ssd1307_i2c: oled@3c {
                     compatible = "solomon,ssd1307";
                     reg = <0x3c>;
                     pwms = <&pwm 4 3000>;
                     reset-gpios = <&gpio2 7>;
             };
 
-            ssd1306: oled@3d {
+            ssd1306_i2c: oled@3d {
                     compatible = "solomon,ssd1306";
                     reg = <0x3c>;
                     pwms = <&pwm 4 3000>;
@@ -231,3 +237,30 @@ examples:
                     solomon,lookup-table = /bits/ 8 <0x3f 0x3f 0x3f 0x3f>;
             };
     };
+  - |
+    spi {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            ssd1307_spi: oled@0 {
+                    compatible = "solomon,ssd1307";
+                    reg = <0x0>;
+                    pwms = <&pwm 4 3000>;
+                    reset-gpios = <&gpio2 7>;
+                    dc-gpios = <&gpio2 8>;
+                    spi-max-frequency = <10000000>;
+            };
+
+            ssd1306_spi: oled@1 {
+                    compatible = "solomon,ssd1306";
+                    reg = <0x1>;
+                    pwms = <&pwm 4 3000>;
+                    reset-gpios = <&gpio2 7>;
+                    dc-gpios = <&gpio2 8>;
+                    spi-max-frequency = <10000000>;
+                    solomon,com-lrremap;
+                    solomon,com-invdir;
+                    solomon,com-offset = <32>;
+                    solomon,lookup-table = /bits/ 8 <0x3f 0x3f 0x3f 0x3f>;
+            };
+    };
-- 
2.35.1

[PATCH v2 2/5] dt-bindings: display: ssd1307fb: Extend schema for SPI controllers

[Javier Martinez Canillas]

The Solomon SSD130x OLED displays can either have an I2C or SPI interface,
add to the schema the properties and examples for OLED devices under SPI.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Mark Brown <broonie@kernel.org>
---

Changes in v2:
- Don't add compatible strings with an "-spi" suffix (Geert Uytterhoeven).

 .../bindings/display/solomon,ssd1307fb.yaml   | 37 ++++++++++++++++++-
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
index 6b9d0c72739a..b58d2d7bd8c1 100644
--- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
+++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
@@ -39,9 +39,14 @@ properties:
   reset-gpios:
     maxItems: 1
 
+  dc-gpios:
+    maxItems: 1
+
   vbat-supply:
     description: The supply for VBAT
 
+  spi-max-frequency: true
+
   solomon,height:
     $ref: /schemas/types.yaml#/definitions/uint32
     default: 16
@@ -160,6 +165,7 @@ allOf:
         compatible:
           contains:
             const: solomon,ssd1305
+
     then:
       properties:
         solomon,dclk-div:
@@ -213,14 +219,14 @@ examples:
             #address-cells = <1>;
             #size-cells = <0>;
 
-            ssd1307: oled@3c {
+            ssd1307_i2c: oled@3c {
                     compatible = "solomon,ssd1307";
                     reg = <0x3c>;
                     pwms = <&pwm 4 3000>;
                     reset-gpios = <&gpio2 7>;
             };
 
-            ssd1306: oled@3d {
+            ssd1306_i2c: oled@3d {
                     compatible = "solomon,ssd1306";
                     reg = <0x3c>;
                     pwms = <&pwm 4 3000>;
@@ -231,3 +237,30 @@ examples:
                     solomon,lookup-table = /bits/ 8 <0x3f 0x3f 0x3f 0x3f>;
             };
     };
+  - |
+    spi {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            ssd1307_spi: oled@0 {
+                    compatible = "solomon,ssd1307";
+                    reg = <0x0>;
+                    pwms = <&pwm 4 3000>;
+                    reset-gpios = <&gpio2 7>;
+                    dc-gpios = <&gpio2 8>;
+                    spi-max-frequency = <10000000>;
+            };
+
+            ssd1306_spi: oled@1 {
+                    compatible = "solomon,ssd1306";
+                    reg = <0x1>;
+                    pwms = <&pwm 4 3000>;
+                    reset-gpios = <&gpio2 7>;
+                    dc-gpios = <&gpio2 8>;
+                    spi-max-frequency = <10000000>;
+                    solomon,com-lrremap;
+                    solomon,com-invdir;
+                    solomon,com-offset = <32>;
+                    solomon,lookup-table = /bits/ 8 <0x3f 0x3f 0x3f 0x3f>;
+            };
+    };
-- 
2.35.1

[PATCH v2 3/5] drm/solomon: Add ssd130x new compatible strings and deprecate old ones.

[Javier Martinez Canillas]

The current compatible strings for SSD130x I2C controllers contain an "fb"
and "-i2c" suffixes. These have been deprecated and more correct ones were
added, that don't encode a subsystem or bus used to interface the devices.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Mark Brown <broonie@kernel.org>
---

(no changes since v1)

 drivers/gpu/drm/solomon/ssd130x-i2c.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/solomon/ssd130x-i2c.c b/drivers/gpu/drm/solomon/ssd130x-i2c.c
index d099b241dd3f..87abe1fe31fc 100644
--- a/drivers/gpu/drm/solomon/ssd130x-i2c.c
+++ b/drivers/gpu/drm/solomon/ssd130x-i2c.c
@@ -87,6 +87,27 @@ static struct ssd130x_deviceinfo ssd130x_ssd1309_deviceinfo = {
 };
 
 static const struct of_device_id ssd130x_of_match[] = {
+	{
+		.compatible = "sinowealth,sh1106",
+		.data = &ssd130x_sh1106_deviceinfo,
+	},
+	{
+		.compatible = "solomon,ssd1305",
+		.data = &ssd130x_ssd1305_deviceinfo,
+	},
+	{
+		.compatible = "solomon,ssd1306",
+		.data = &ssd130x_ssd1306_deviceinfo,
+	},
+	{
+		.compatible = "solomon,ssd1307",
+		.data = &ssd130x_ssd1307_deviceinfo,
+	},
+	{
+		.compatible = "solomon,ssd1309",
+		.data = &ssd130x_ssd1309_deviceinfo,
+	},
+	/* Deprecated but kept for backward compatibility */
 	{
 		.compatible = "sinowealth,sh1106-i2c",
 		.data = &ssd130x_sh1106_deviceinfo,
-- 
2.35.1

[PATCH v2 3/5] drm/solomon: Add ssd130x new compatible strings and deprecate old ones.

[Javier Martinez Canillas]

The current compatible strings for SSD130x I2C controllers contain an "fb"
and "-i2c" suffixes. These have been deprecated and more correct ones were
added, that don't encode a subsystem or bus used to interface the devices.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Mark Brown <broonie@kernel.org>
---

(no changes since v1)

 drivers/gpu/drm/solomon/ssd130x-i2c.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/solomon/ssd130x-i2c.c b/drivers/gpu/drm/solomon/ssd130x-i2c.c
index d099b241dd3f..87abe1fe31fc 100644
--- a/drivers/gpu/drm/solomon/ssd130x-i2c.c
+++ b/drivers/gpu/drm/solomon/ssd130x-i2c.c
@@ -87,6 +87,27 @@ static struct ssd130x_deviceinfo ssd130x_ssd1309_deviceinfo = {
 };
 
 static const struct of_device_id ssd130x_of_match[] = {
+	{
+		.compatible = "sinowealth,sh1106",
+		.data = &ssd130x_sh1106_deviceinfo,
+	},
+	{
+		.compatible = "solomon,ssd1305",
+		.data = &ssd130x_ssd1305_deviceinfo,
+	},
+	{
+		.compatible = "solomon,ssd1306",
+		.data = &ssd130x_ssd1306_deviceinfo,
+	},
+	{
+		.compatible = "solomon,ssd1307",
+		.data = &ssd130x_ssd1307_deviceinfo,
+	},
+	{
+		.compatible = "solomon,ssd1309",
+		.data = &ssd130x_ssd1309_deviceinfo,
+	},
+	/* Deprecated but kept for backward compatibility */
 	{
 		.compatible = "sinowealth,sh1106-i2c",
 		.data = &ssd130x_sh1106_deviceinfo,
-- 
2.35.1

[PATCH v2 4/5] drm/solomon: Move device info from ssd130x-i2c to the core driver

[Javier Martinez Canillas]

These are declared in the ssd130x-i2c transport driver but the information
is not I2C specific, and could be used by other SSD130x transport drivers.

Move them to the ssd130x core driver and just set the OF device entries to
an ID that could be used to lookup the correct device info from an array.

While being there, also move the SSD130X_DATA and SSD130X_COMMAND control
bytes. Since even though are used by the I2C interface, it could also be
useful for other transport protocols such as SPI.

Suggested-by: Chen-Yu Tsai <wens@kernel.org>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Mark Brown <broonie@kernel.org>
---

Changes in v2:
- Drop ssd13x_variant_to_info() and just use the array index (Neil Armstrong).

 drivers/gpu/drm/solomon/ssd130x-i2c.c | 53 +++++----------------------
 drivers/gpu/drm/solomon/ssd130x.c     | 45 +++++++++++++++++++++--
 drivers/gpu/drm/solomon/ssd130x.h     | 12 ++++++
 3 files changed, 63 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/solomon/ssd130x-i2c.c b/drivers/gpu/drm/solomon/ssd130x-i2c.c
index 87abe1fe31fc..8259b10db966 100644
--- a/drivers/gpu/drm/solomon/ssd130x-i2c.c
+++ b/drivers/gpu/drm/solomon/ssd130x-i2c.c
@@ -53,80 +53,47 @@ static void ssd130x_i2c_shutdown(struct i2c_client *client)
 	ssd130x_shutdown(ssd130x);
 }
 
-static struct ssd130x_deviceinfo ssd130x_sh1106_deviceinfo = {
-	.default_vcomh = 0x40,
-	.default_dclk_div = 1,
-	.default_dclk_frq = 5,
-	.page_mode_only = 1,
-};
-
-static struct ssd130x_deviceinfo ssd130x_ssd1305_deviceinfo = {
-	.default_vcomh = 0x34,
-	.default_dclk_div = 1,
-	.default_dclk_frq = 7,
-};
-
-static struct ssd130x_deviceinfo ssd130x_ssd1306_deviceinfo = {
-	.default_vcomh = 0x20,
-	.default_dclk_div = 1,
-	.default_dclk_frq = 8,
-	.need_chargepump = 1,
-};
-
-static struct ssd130x_deviceinfo ssd130x_ssd1307_deviceinfo = {
-	.default_vcomh = 0x20,
-	.default_dclk_div = 2,
-	.default_dclk_frq = 12,
-	.need_pwm = 1,
-};
-
-static struct ssd130x_deviceinfo ssd130x_ssd1309_deviceinfo = {
-	.default_vcomh = 0x34,
-	.default_dclk_div = 1,
-	.default_dclk_frq = 10,
-};
-
 static const struct of_device_id ssd130x_of_match[] = {
 	{
 		.compatible = "sinowealth,sh1106",
-		.data = &ssd130x_sh1106_deviceinfo,
+		.data = (void *)SH1106_ID,
 	},
 	{
 		.compatible = "solomon,ssd1305",
-		.data = &ssd130x_ssd1305_deviceinfo,
+		.data = (void *)SSD1305_ID,
 	},
 	{
 		.compatible = "solomon,ssd1306",
-		.data = &ssd130x_ssd1306_deviceinfo,
+		.data = (void *)SSD1306_ID,
 	},
 	{
 		.compatible = "solomon,ssd1307",
-		.data = &ssd130x_ssd1307_deviceinfo,
+		.data = (void *)SSD1307_ID,
 	},
 	{
 		.compatible = "solomon,ssd1309",
-		.data = &ssd130x_ssd1309_deviceinfo,
+		.data = (void *)SSD1309_ID,
 	},
 	/* Deprecated but kept for backward compatibility */
 	{
 		.compatible = "sinowealth,sh1106-i2c",
-		.data = &ssd130x_sh1106_deviceinfo,
+		.data = (void *)SH1106_ID,
 	},
 	{
 		.compatible = "solomon,ssd1305fb-i2c",
-		.data = &ssd130x_ssd1305_deviceinfo,
+		.data = (void *)SSD1305_ID,
 	},
 	{
 		.compatible = "solomon,ssd1306fb-i2c",
-		.data = &ssd130x_ssd1306_deviceinfo,
+		.data = (void *)SSD1306_ID,
 	},
 	{
 		.compatible = "solomon,ssd1307fb-i2c",
-		.data = &ssd130x_ssd1307_deviceinfo,
+		.data = (void *)SSD1307_ID,
 	},
 	{
 		.compatible = "solomon,ssd1309fb-i2c",
-		.data = &ssd130x_ssd1309_deviceinfo,
+		.data = (void *)SSD1309_ID,
 	},
 	{ /* sentinel */ }
 };
diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index a7e784518c69..7d5b43023213 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -39,11 +39,9 @@
 #define DRIVER_MAJOR	1
 #define DRIVER_MINOR	0
 
-#define SSD130X_DATA				0x40
-#define SSD130X_COMMAND				0x80
-
 #define SSD130X_PAGE_COL_START_LOW		0x00
 #define SSD130X_PAGE_COL_START_HIGH		0x10
+
 #define SSD130X_SET_ADDRESS_MODE		0x20
 #define SSD130X_SET_COL_RANGE			0x21
 #define SSD130X_SET_PAGE_RANGE			0x22
@@ -94,6 +92,37 @@
 
 #define MAX_CONTRAST 255
 
+static const struct ssd130x_deviceinfo ssd130x_variants[] = {
+	[SH1106_ID] = {
+		.default_vcomh = 0x40,
+		.default_dclk_div = 1,
+		.default_dclk_frq = 5,
+		.page_mode_only = 1,
+	},
+	[SSD1305_ID] = {
+		.default_vcomh = 0x34,
+		.default_dclk_div = 1,
+		.default_dclk_frq = 7,
+	},
+	[SSD1306_ID] = {
+		.default_vcomh = 0x20,
+		.default_dclk_div = 1,
+		.default_dclk_frq = 8,
+		.need_chargepump = 1,
+	},
+	[SSD1307_ID] = {
+		.default_vcomh = 0x20,
+		.default_dclk_div = 2,
+		.default_dclk_frq = 12,
+		.need_pwm = 1,
+	},
+	[SSD1309_ID] = {
+		.default_vcomh = 0x34,
+		.default_dclk_div = 1,
+		.default_dclk_frq = 10,
+	}
+};
+
 static inline struct ssd130x_device *drm_to_ssd130x(struct drm_device *drm)
 {
 	return container_of(drm, struct ssd130x_device, drm);
@@ -846,6 +875,7 @@ static int ssd130x_get_resources(struct ssd130x_device *ssd130x)
 struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap)
 {
 	struct ssd130x_device *ssd130x;
+	enum ssd130x_variants variant;
 	struct backlight_device *bl;
 	struct drm_device *drm;
 	int ret;
@@ -860,7 +890,14 @@ struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap)
 
 	ssd130x->dev = dev;
 	ssd130x->regmap = regmap;
-	ssd130x->device_info = device_get_match_data(dev);
+
+	variant = (enum ssd130x_variants)device_get_match_data(dev);
+
+	if (variant >= NR_SSD130X_VARIANTS)
+		return ERR_PTR(dev_err_probe(dev, -EINVAL,
+					     "Invalid SSD130x variant\n"));
+
+	ssd130x->device_info = &ssd130x_variants[variant];
 
 	if (ssd130x->device_info->page_mode_only)
 		ssd130x->page_address_mode = 1;
diff --git a/drivers/gpu/drm/solomon/ssd130x.h b/drivers/gpu/drm/solomon/ssd130x.h
index f5b062576fdf..ec349239e945 100644
--- a/drivers/gpu/drm/solomon/ssd130x.h
+++ b/drivers/gpu/drm/solomon/ssd130x.h
@@ -18,6 +18,18 @@
 
 #include <linux/regmap.h>
 
+#define SSD130X_DATA				0x40
+#define SSD130X_COMMAND				0x80
+
+enum ssd130x_variants {
+	SH1106_ID,
+	SSD1305_ID,
+	SSD1306_ID,
+	SSD1307_ID,
+	SSD1309_ID,
+	NR_SSD130X_VARIANTS
+};
+
 struct ssd130x_deviceinfo {
 	u32 default_vcomh;
 	u32 default_dclk_div;
-- 
2.35.1

[PATCH v2 4/5] drm/solomon: Move device info from ssd130x-i2c to the core driver

[Javier Martinez Canillas]

These are declared in the ssd130x-i2c transport driver but the information
is not I2C specific, and could be used by other SSD130x transport drivers.

Move them to the ssd130x core driver and just set the OF device entries to
an ID that could be used to lookup the correct device info from an array.

While being there, also move the SSD130X_DATA and SSD130X_COMMAND control
bytes. Since even though are used by the I2C interface, it could also be
useful for other transport protocols such as SPI.

Suggested-by: Chen-Yu Tsai <wens@kernel.org>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Mark Brown <broonie@kernel.org>
---

Changes in v2:
- Drop ssd13x_variant_to_info() and just use the array index (Neil Armstrong).

 drivers/gpu/drm/solomon/ssd130x-i2c.c | 53 +++++----------------------
 drivers/gpu/drm/solomon/ssd130x.c     | 45 +++++++++++++++++++++--
 drivers/gpu/drm/solomon/ssd130x.h     | 12 ++++++
 3 files changed, 63 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/solomon/ssd130x-i2c.c b/drivers/gpu/drm/solomon/ssd130x-i2c.c
index 87abe1fe31fc..8259b10db966 100644
--- a/drivers/gpu/drm/solomon/ssd130x-i2c.c
+++ b/drivers/gpu/drm/solomon/ssd130x-i2c.c
@@ -53,80 +53,47 @@ static void ssd130x_i2c_shutdown(struct i2c_client *client)
 	ssd130x_shutdown(ssd130x);
 }
 
-static struct ssd130x_deviceinfo ssd130x_sh1106_deviceinfo = {
-	.default_vcomh = 0x40,
-	.default_dclk_div = 1,
-	.default_dclk_frq = 5,
-	.page_mode_only = 1,
-};
-
-static struct ssd130x_deviceinfo ssd130x_ssd1305_deviceinfo = {
-	.default_vcomh = 0x34,
-	.default_dclk_div = 1,
-	.default_dclk_frq = 7,
-};
-
-static struct ssd130x_deviceinfo ssd130x_ssd1306_deviceinfo = {
-	.default_vcomh = 0x20,
-	.default_dclk_div = 1,
-	.default_dclk_frq = 8,
-	.need_chargepump = 1,
-};
-
-static struct ssd130x_deviceinfo ssd130x_ssd1307_deviceinfo = {
-	.default_vcomh = 0x20,
-	.default_dclk_div = 2,
-	.default_dclk_frq = 12,
-	.need_pwm = 1,
-};
-
-static struct ssd130x_deviceinfo ssd130x_ssd1309_deviceinfo = {
-	.default_vcomh = 0x34,
-	.default_dclk_div = 1,
-	.default_dclk_frq = 10,
-};
-
 static const struct of_device_id ssd130x_of_match[] = {
 	{
 		.compatible = "sinowealth,sh1106",
-		.data = &ssd130x_sh1106_deviceinfo,
+		.data = (void *)SH1106_ID,
 	},
 	{
 		.compatible = "solomon,ssd1305",
-		.data = &ssd130x_ssd1305_deviceinfo,
+		.data = (void *)SSD1305_ID,
 	},
 	{
 		.compatible = "solomon,ssd1306",
-		.data = &ssd130x_ssd1306_deviceinfo,
+		.data = (void *)SSD1306_ID,
 	},
 	{
 		.compatible = "solomon,ssd1307",
-		.data = &ssd130x_ssd1307_deviceinfo,
+		.data = (void *)SSD1307_ID,
 	},
 	{
 		.compatible = "solomon,ssd1309",
-		.data = &ssd130x_ssd1309_deviceinfo,
+		.data = (void *)SSD1309_ID,
 	},
 	/* Deprecated but kept for backward compatibility */
 	{
 		.compatible = "sinowealth,sh1106-i2c",
-		.data = &ssd130x_sh1106_deviceinfo,
+		.data = (void *)SH1106_ID,
 	},
 	{
 		.compatible = "solomon,ssd1305fb-i2c",
-		.data = &ssd130x_ssd1305_deviceinfo,
+		.data = (void *)SSD1305_ID,
 	},
 	{
 		.compatible = "solomon,ssd1306fb-i2c",
-		.data = &ssd130x_ssd1306_deviceinfo,
+		.data = (void *)SSD1306_ID,
 	},
 	{
 		.compatible = "solomon,ssd1307fb-i2c",
-		.data = &ssd130x_ssd1307_deviceinfo,
+		.data = (void *)SSD1307_ID,
 	},
 	{
 		.compatible = "solomon,ssd1309fb-i2c",
-		.data = &ssd130x_ssd1309_deviceinfo,
+		.data = (void *)SSD1309_ID,
 	},
 	{ /* sentinel */ }
 };
diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index a7e784518c69..7d5b43023213 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -39,11 +39,9 @@
 #define DRIVER_MAJOR	1
 #define DRIVER_MINOR	0
 
-#define SSD130X_DATA				0x40
-#define SSD130X_COMMAND				0x80
-
 #define SSD130X_PAGE_COL_START_LOW		0x00
 #define SSD130X_PAGE_COL_START_HIGH		0x10
+
 #define SSD130X_SET_ADDRESS_MODE		0x20
 #define SSD130X_SET_COL_RANGE			0x21
 #define SSD130X_SET_PAGE_RANGE			0x22
@@ -94,6 +92,37 @@
 
 #define MAX_CONTRAST 255
 
+static const struct ssd130x_deviceinfo ssd130x_variants[] = {
+	[SH1106_ID] = {
+		.default_vcomh = 0x40,
+		.default_dclk_div = 1,
+		.default_dclk_frq = 5,
+		.page_mode_only = 1,
+	},
+	[SSD1305_ID] = {
+		.default_vcomh = 0x34,
+		.default_dclk_div = 1,
+		.default_dclk_frq = 7,
+	},
+	[SSD1306_ID] = {
+		.default_vcomh = 0x20,
+		.default_dclk_div = 1,
+		.default_dclk_frq = 8,
+		.need_chargepump = 1,
+	},
+	[SSD1307_ID] = {
+		.default_vcomh = 0x20,
+		.default_dclk_div = 2,
+		.default_dclk_frq = 12,
+		.need_pwm = 1,
+	},
+	[SSD1309_ID] = {
+		.default_vcomh = 0x34,
+		.default_dclk_div = 1,
+		.default_dclk_frq = 10,
+	}
+};
+
 static inline struct ssd130x_device *drm_to_ssd130x(struct drm_device *drm)
 {
 	return container_of(drm, struct ssd130x_device, drm);
@@ -846,6 +875,7 @@ static int ssd130x_get_resources(struct ssd130x_device *ssd130x)
 struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap)
 {
 	struct ssd130x_device *ssd130x;
+	enum ssd130x_variants variant;
 	struct backlight_device *bl;
 	struct drm_device *drm;
 	int ret;
@@ -860,7 +890,14 @@ struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap)
 
 	ssd130x->dev = dev;
 	ssd130x->regmap = regmap;
-	ssd130x->device_info = device_get_match_data(dev);
+
+	variant = (enum ssd130x_variants)device_get_match_data(dev);
+
+	if (variant >= NR_SSD130X_VARIANTS)
+		return ERR_PTR(dev_err_probe(dev, -EINVAL,
+					     "Invalid SSD130x variant\n"));
+
+	ssd130x->device_info = &ssd130x_variants[variant];
 
 	if (ssd130x->device_info->page_mode_only)
 		ssd130x->page_address_mode = 1;
diff --git a/drivers/gpu/drm/solomon/ssd130x.h b/drivers/gpu/drm/solomon/ssd130x.h
index f5b062576fdf..ec349239e945 100644
--- a/drivers/gpu/drm/solomon/ssd130x.h
+++ b/drivers/gpu/drm/solomon/ssd130x.h
@@ -18,6 +18,18 @@
 
 #include <linux/regmap.h>
 
+#define SSD130X_DATA				0x40
+#define SSD130X_COMMAND				0x80
+
+enum ssd130x_variants {
+	SH1106_ID,
+	SSD1305_ID,
+	SSD1306_ID,
+	SSD1307_ID,
+	SSD1309_ID,
+	NR_SSD130X_VARIANTS
+};
+
 struct ssd130x_deviceinfo {
 	u32 default_vcomh;
 	u32 default_dclk_div;
-- 
2.35.1

[PATCH v2 5/5] drm/solomon: Add SSD130x OLED displays SPI support

[Javier Martinez Canillas]

The ssd130x driver only provides the core support for these devices but it
does not have any bus transport logic. Add a driver to interface over SPI.

There is a difference in the communication protocol when using 4-wire SPI
instead of I2C. For the latter, a control byte that contains a D/C# field
has to be sent. This field tells the controller whether the data has to be
written to the command register or to the graphics display data memory.

But for 4-wire SPI that control byte is not used, instead a real D/C# line
must be pulled HIGH for commands data and LOW for graphics display data.

For this reason the standard SPI regmap can't be used and a custom .write
bus handler is needed.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Mark Brown <broonie@kernel.org>
---

(no changes since v1)

 drivers/gpu/drm/solomon/Kconfig       |   9 ++
 drivers/gpu/drm/solomon/Makefile      |   1 +
 drivers/gpu/drm/solomon/ssd130x-spi.c | 184 ++++++++++++++++++++++++++
 3 files changed, 194 insertions(+)
 create mode 100644 drivers/gpu/drm/solomon/ssd130x-spi.c

diff --git a/drivers/gpu/drm/solomon/Kconfig b/drivers/gpu/drm/solomon/Kconfig
index 8c0a0c788385..e170716d976b 100644
--- a/drivers/gpu/drm/solomon/Kconfig
+++ b/drivers/gpu/drm/solomon/Kconfig
@@ -20,3 +20,12 @@ config DRM_SSD130X_I2C
 	  I2C bus.
 
 	  If M is selected the module will be called ssd130x-i2c.
+
+config DRM_SSD130X_SPI
+	tristate "DRM support for Solomon SSD130X OLED displays (SPI bus)"
+	depends on DRM_SSD130X && SPI
+	select REGMAP
+	help
+	  Say Y here if the SSD130x OLED display is connected via SPI bus.
+
+	  If M is selected the module will be called ssd130x-spi.
diff --git a/drivers/gpu/drm/solomon/Makefile b/drivers/gpu/drm/solomon/Makefile
index 4bfc5acb0447..b5fc792257d7 100644
--- a/drivers/gpu/drm/solomon/Makefile
+++ b/drivers/gpu/drm/solomon/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_DRM_SSD130X)	+= ssd130x.o
 obj-$(CONFIG_DRM_SSD130X_I2C)	+= ssd130x-i2c.o
+obj-$(CONFIG_DRM_SSD130X_SPI)	+= ssd130x-spi.o
diff --git a/drivers/gpu/drm/solomon/ssd130x-spi.c b/drivers/gpu/drm/solomon/ssd130x-spi.c
new file mode 100644
index 000000000000..b6fee66a0c01
--- /dev/null
+++ b/drivers/gpu/drm/solomon/ssd130x-spi.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * DRM driver for Solomon SSD130X OLED displays (SPI bus)
+ *
+ * Copyright 2022 Red Hat Inc.
+ * Authors: Javier Martinez Canillas <javierm@redhat.com>
+ */
+#include <linux/spi/spi.h>
+#include <linux/module.h>
+
+#include "ssd130x.h"
+
+#define DRIVER_NAME	"ssd130x-spi"
+#define DRIVER_DESC	"DRM driver for Solomon SSD130X OLED displays (SPI)"
+
+struct ssd130x_spi_transport {
+	struct spi_device *spi;
+	struct gpio_desc *dc;
+};
+
+static const struct regmap_config ssd130x_spi_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
+/*
+ * The regmap bus .write handler, it is just a wrapper around spi_write()
+ * but toggling the Data/Command control pin (D/C#). Since for 4-wire SPI
+ * a D/C# pin is used, in contrast with I2C where a control byte is sent,
+ * prior to every data byte, that contains a bit with the D/C# value.
+ *
+ * These control bytes are considered registers by the ssd130x core driver
+ * and can be used by the ssd130x SPI driver to determine if the data sent
+ * is for a command register or for the Graphic Display Data RAM (GDDRAM).
+ */
+static int ssd130x_spi_write(void *context, const void *data, size_t count)
+{
+	struct ssd130x_spi_transport *t = context;
+	struct spi_device *spi = t->spi;
+	const u8 *reg = data;
+
+	if (*reg == SSD130X_COMMAND)
+		gpiod_set_value_cansleep(t->dc, 0);
+
+	if (*reg == SSD130X_DATA)
+		gpiod_set_value_cansleep(t->dc, 1);
+
+	/* Remove the control byte since is not used by the 4-wire SPI */
+	return spi_write(spi, ((u8 *)data) + 1, count - 1);
+}
+
+/* The ssd130x driver does not read registers but regmap expects a .read */
+static int ssd130x_spi_read(void *context, const void *reg, size_t reg_size,
+			    void *val, size_t val_size)
+{
+	return -EOPNOTSUPP;
+}
+
+/*
+ * A custom bus is needed due the special write that toggles a D/C# pin,
+ * another option could be to just have a .reg_write() callback but that
+ * will prevent to do data writes in bulk.
+ *
+ * Once the regmap API is extended to support defining a bulk write handler
+ * in the struct regmap_config, this can be simplified and the bus dropped.
+ */
+static struct regmap_bus regmap_ssd130x_spi_bus = {
+	.write = ssd130x_spi_write,
+	.read = ssd130x_spi_read,
+};
+
+static struct gpio_desc *ssd130x_spi_get_dc(struct device *dev)
+{
+	struct gpio_desc *dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW);
+
+	if (IS_ERR(dc))
+		return ERR_PTR(dev_err_probe(dev, PTR_ERR(dc), "Failed to get dc gpio\n"));
+
+	return dc;
+}
+
+static int ssd130x_spi_probe(struct spi_device *spi)
+{
+	struct ssd130x_spi_transport *t;
+	struct ssd130x_device *ssd130x;
+	struct regmap *regmap;
+	struct device *dev = &spi->dev;
+
+	t = devm_kzalloc(dev, sizeof(*t), GFP_KERNEL);
+	if (!t)
+		return dev_err_probe(dev, -ENOMEM,
+				     "Failed to allocate SPI transport data\n");
+
+	t->spi = spi;
+
+	t->dc = ssd130x_spi_get_dc(&spi->dev);
+	if (IS_ERR(t->dc))
+		return PTR_ERR(t->dc);
+
+	regmap = devm_regmap_init(dev, &regmap_ssd130x_spi_bus, t,
+				  &ssd130x_spi_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	ssd130x = ssd130x_probe(dev, regmap);
+	if (IS_ERR(ssd130x))
+		return PTR_ERR(ssd130x);
+
+	spi_set_drvdata(spi, ssd130x);
+
+	return 0;
+}
+
+static void ssd130x_spi_remove(struct spi_device *spi)
+{
+	struct ssd130x_device *ssd130x = spi_get_drvdata(spi);
+
+	ssd130x_remove(ssd130x);
+}
+
+static void ssd130x_spi_shutdown(struct spi_device *spi)
+{
+	struct ssd130x_device *ssd130x = spi_get_drvdata(spi);
+
+	ssd130x_shutdown(ssd130x);
+}
+
+static const struct of_device_id ssd130x_of_match[] = {
+	{
+		.compatible = "sinowealth,sh1106",
+		.data = (void *)SH1106_ID,
+	},
+	{
+		.compatible = "solomon,ssd1305",
+		.data = (void *)SSD1305_ID,
+	},
+	{
+		.compatible = "solomon,ssd1306",
+		.data =  (void *)SSD1306_ID,
+	},
+	{
+		.compatible = "solomon,ssd1307",
+		.data =  (void *)SSD1307_ID,
+	},
+	{
+		.compatible = "solomon,ssd1309",
+		.data =  (void *)SSD1309_ID,
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ssd130x_of_match);
+
+/*
+ * The SPI core always reports a MODALIAS uevent of the form "spi:<dev>", even
+ * if the device was registered via OF. This means that the module will not be
+ * auto loaded, unless it contains an alias that matches the MODALIAS reported.
+ *
+ * To workaround this issue, add a SPI device ID table. Even when this should
+ * not be needed for this driver to match the registered SPI devices.
+ */
+static const struct spi_device_id ssd130x_spi_table[] = {
+	{ "sh1106",  SH1106_ID },
+	{ "ssd1305", SSD1305_ID },
+	{ "ssd1306", SSD1306_ID },
+	{ "ssd1307", SSD1307_ID },
+	{ "ssd1309", SSD1309_ID },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(spi, ssd130x_spi_table);
+
+static struct spi_driver ssd130x_spi_driver = {
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table = ssd130x_of_match,
+	},
+	.probe = ssd130x_spi_probe,
+	.remove = ssd130x_spi_remove,
+	.shutdown = ssd130x_spi_shutdown,
+};
+module_spi_driver(ssd130x_spi_driver);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_AUTHOR("Javier Martinez Canillas <javierm@redhat.com>");
+MODULE_LICENSE("GPL");
-- 
2.35.1

[PATCH v2 5/5] drm/solomon: Add SSD130x OLED displays SPI support

[Javier Martinez Canillas]

The ssd130x driver only provides the core support for these devices but it
does not have any bus transport logic. Add a driver to interface over SPI.

There is a difference in the communication protocol when using 4-wire SPI
instead of I2C. For the latter, a control byte that contains a D/C# field
has to be sent. This field tells the controller whether the data has to be
written to the command register or to the graphics display data memory.

But for 4-wire SPI that control byte is not used, instead a real D/C# line
must be pulled HIGH for commands data and LOW for graphics display data.

For this reason the standard SPI regmap can't be used and a custom .write
bus handler is needed.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Mark Brown <broonie@kernel.org>
---

(no changes since v1)

 drivers/gpu/drm/solomon/Kconfig       |   9 ++
 drivers/gpu/drm/solomon/Makefile      |   1 +
 drivers/gpu/drm/solomon/ssd130x-spi.c | 184 ++++++++++++++++++++++++++
 3 files changed, 194 insertions(+)
 create mode 100644 drivers/gpu/drm/solomon/ssd130x-spi.c

diff --git a/drivers/gpu/drm/solomon/Kconfig b/drivers/gpu/drm/solomon/Kconfig
index 8c0a0c788385..e170716d976b 100644
--- a/drivers/gpu/drm/solomon/Kconfig
+++ b/drivers/gpu/drm/solomon/Kconfig
@@ -20,3 +20,12 @@ config DRM_SSD130X_I2C
 	  I2C bus.
 
 	  If M is selected the module will be called ssd130x-i2c.
+
+config DRM_SSD130X_SPI
+	tristate "DRM support for Solomon SSD130X OLED displays (SPI bus)"
+	depends on DRM_SSD130X && SPI
+	select REGMAP
+	help
+	  Say Y here if the SSD130x OLED display is connected via SPI bus.
+
+	  If M is selected the module will be called ssd130x-spi.
diff --git a/drivers/gpu/drm/solomon/Makefile b/drivers/gpu/drm/solomon/Makefile
index 4bfc5acb0447..b5fc792257d7 100644
--- a/drivers/gpu/drm/solomon/Makefile
+++ b/drivers/gpu/drm/solomon/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_DRM_SSD130X)	+= ssd130x.o
 obj-$(CONFIG_DRM_SSD130X_I2C)	+= ssd130x-i2c.o
+obj-$(CONFIG_DRM_SSD130X_SPI)	+= ssd130x-spi.o
diff --git a/drivers/gpu/drm/solomon/ssd130x-spi.c b/drivers/gpu/drm/solomon/ssd130x-spi.c
new file mode 100644
index 000000000000..b6fee66a0c01
--- /dev/null
+++ b/drivers/gpu/drm/solomon/ssd130x-spi.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * DRM driver for Solomon SSD130X OLED displays (SPI bus)
+ *
+ * Copyright 2022 Red Hat Inc.
+ * Authors: Javier Martinez Canillas <javierm@redhat.com>
+ */
+#include <linux/spi/spi.h>
+#include <linux/module.h>
+
+#include "ssd130x.h"
+
+#define DRIVER_NAME	"ssd130x-spi"
+#define DRIVER_DESC	"DRM driver for Solomon SSD130X OLED displays (SPI)"
+
+struct ssd130x_spi_transport {
+	struct spi_device *spi;
+	struct gpio_desc *dc;
+};
+
+static const struct regmap_config ssd130x_spi_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
+/*
+ * The regmap bus .write handler, it is just a wrapper around spi_write()
+ * but toggling the Data/Command control pin (D/C#). Since for 4-wire SPI
+ * a D/C# pin is used, in contrast with I2C where a control byte is sent,
+ * prior to every data byte, that contains a bit with the D/C# value.
+ *
+ * These control bytes are considered registers by the ssd130x core driver
+ * and can be used by the ssd130x SPI driver to determine if the data sent
+ * is for a command register or for the Graphic Display Data RAM (GDDRAM).
+ */
+static int ssd130x_spi_write(void *context, const void *data, size_t count)
+{
+	struct ssd130x_spi_transport *t = context;
+	struct spi_device *spi = t->spi;
+	const u8 *reg = data;
+
+	if (*reg == SSD130X_COMMAND)
+		gpiod_set_value_cansleep(t->dc, 0);
+
+	if (*reg == SSD130X_DATA)
+		gpiod_set_value_cansleep(t->dc, 1);
+
+	/* Remove the control byte since is not used by the 4-wire SPI */
+	return spi_write(spi, ((u8 *)data) + 1, count - 1);
+}
+
+/* The ssd130x driver does not read registers but regmap expects a .read */
+static int ssd130x_spi_read(void *context, const void *reg, size_t reg_size,
+			    void *val, size_t val_size)
+{
+	return -EOPNOTSUPP;
+}
+
+/*
+ * A custom bus is needed due the special write that toggles a D/C# pin,
+ * another option could be to just have a .reg_write() callback but that
+ * will prevent to do data writes in bulk.
+ *
+ * Once the regmap API is extended to support defining a bulk write handler
+ * in the struct regmap_config, this can be simplified and the bus dropped.
+ */
+static struct regmap_bus regmap_ssd130x_spi_bus = {
+	.write = ssd130x_spi_write,
+	.read = ssd130x_spi_read,
+};
+
+static struct gpio_desc *ssd130x_spi_get_dc(struct device *dev)
+{
+	struct gpio_desc *dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW);
+
+	if (IS_ERR(dc))
+		return ERR_PTR(dev_err_probe(dev, PTR_ERR(dc), "Failed to get dc gpio\n"));
+
+	return dc;
+}
+
+static int ssd130x_spi_probe(struct spi_device *spi)
+{
+	struct ssd130x_spi_transport *t;
+	struct ssd130x_device *ssd130x;
+	struct regmap *regmap;
+	struct device *dev = &spi->dev;
+
+	t = devm_kzalloc(dev, sizeof(*t), GFP_KERNEL);
+	if (!t)
+		return dev_err_probe(dev, -ENOMEM,
+				     "Failed to allocate SPI transport data\n");
+
+	t->spi = spi;
+
+	t->dc = ssd130x_spi_get_dc(&spi->dev);
+	if (IS_ERR(t->dc))
+		return PTR_ERR(t->dc);
+
+	regmap = devm_regmap_init(dev, &regmap_ssd130x_spi_bus, t,
+				  &ssd130x_spi_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	ssd130x = ssd130x_probe(dev, regmap);
+	if (IS_ERR(ssd130x))
+		return PTR_ERR(ssd130x);
+
+	spi_set_drvdata(spi, ssd130x);
+
+	return 0;
+}
+
+static void ssd130x_spi_remove(struct spi_device *spi)
+{
+	struct ssd130x_device *ssd130x = spi_get_drvdata(spi);
+
+	ssd130x_remove(ssd130x);
+}
+
+static void ssd130x_spi_shutdown(struct spi_device *spi)
+{
+	struct ssd130x_device *ssd130x = spi_get_drvdata(spi);
+
+	ssd130x_shutdown(ssd130x);
+}
+
+static const struct of_device_id ssd130x_of_match[] = {
+	{
+		.compatible = "sinowealth,sh1106",
+		.data = (void *)SH1106_ID,
+	},
+	{
+		.compatible = "solomon,ssd1305",
+		.data = (void *)SSD1305_ID,
+	},
+	{
+		.compatible = "solomon,ssd1306",
+		.data =  (void *)SSD1306_ID,
+	},
+	{
+		.compatible = "solomon,ssd1307",
+		.data =  (void *)SSD1307_ID,
+	},
+	{
+		.compatible = "solomon,ssd1309",
+		.data =  (void *)SSD1309_ID,
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ssd130x_of_match);
+
+/*
+ * The SPI core always reports a MODALIAS uevent of the form "spi:<dev>", even
+ * if the device was registered via OF. This means that the module will not be
+ * auto loaded, unless it contains an alias that matches the MODALIAS reported.
+ *
+ * To workaround this issue, add a SPI device ID table. Even when this should
+ * not be needed for this driver to match the registered SPI devices.
+ */
+static const struct spi_device_id ssd130x_spi_table[] = {
+	{ "sh1106",  SH1106_ID },
+	{ "ssd1305", SSD1305_ID },
+	{ "ssd1306", SSD1306_ID },
+	{ "ssd1307", SSD1307_ID },
+	{ "ssd1309", SSD1309_ID },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(spi, ssd130x_spi_table);
+
+static struct spi_driver ssd130x_spi_driver = {
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table = ssd130x_of_match,
+	},
+	.probe = ssd130x_spi_probe,
+	.remove = ssd130x_spi_remove,
+	.shutdown = ssd130x_spi_shutdown,
+};
+module_spi_driver(ssd130x_spi_driver);
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_AUTHOR("Javier Martinez Canillas <javierm@redhat.com>");
+MODULE_LICENSE("GPL");
-- 
2.35.1

Re: [PATCH v2 1/5] dt-bindings: display: ssd1307fb: Deprecate "-i2c" compatible strings

[Geert Uytterhoeven]

On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> The current compatible strings for SSD130x I2C controllers contain both an
> "fb" and "-i2c" suffixes. It seems to indicate that are for a fbdev driver
> and also that are for devices that can be accessed over an I2C bus.
>
> But a DT is supposed to describe the hardware and not Linux implementation
> details. So let's deprecate those compatible strings and add new ones that
> only contain the vendor and device name, without any of these suffixes.
>
> These will just describe the device and can be matched by both I2C and SPI
> DRM drivers.
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Mark Brown <broonie@kernel.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

Re: [PATCH v2 1/5] dt-bindings: display: ssd1307fb: Deprecate "-i2c" compatible strings

[Geert Uytterhoeven]

On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> The current compatible strings for SSD130x I2C controllers contain both an
> "fb" and "-i2c" suffixes. It seems to indicate that are for a fbdev driver
> and also that are for devices that can be accessed over an I2C bus.
>
> But a DT is supposed to describe the hardware and not Linux implementation
> details. So let's deprecate those compatible strings and add new ones that
> only contain the vendor and device name, without any of these suffixes.
>
> These will just describe the device and can be matched by both I2C and SPI
> DRM drivers.
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Mark Brown <broonie@kernel.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

Re: [PATCH v2 2/5] dt-bindings: display: ssd1307fb: Extend schema for SPI controllers

[Geert Uytterhoeven]

Hi Javier,

On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> The Solomon SSD130x OLED displays can either have an I2C or SPI interface,
> add to the schema the properties and examples for OLED devices under SPI.
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Mark Brown <broonie@kernel.org>
> ---
>
> Changes in v2:
> - Don't add compatible strings with an "-spi" suffix (Geert Uytterhoeven).

Thanks for the update!

> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> @@ -39,9 +39,14 @@ properties:
>    reset-gpios:
>      maxItems: 1
>
> +  dc-gpios:
> +    maxItems: 1
> +

Perhaps add a description, and clarify this is for SPI only?

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

Re: [PATCH v2 2/5] dt-bindings: display: ssd1307fb: Extend schema for SPI controllers

[Geert Uytterhoeven]

Hi Javier,

On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> The Solomon SSD130x OLED displays can either have an I2C or SPI interface,
> add to the schema the properties and examples for OLED devices under SPI.
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Mark Brown <broonie@kernel.org>
> ---
>
> Changes in v2:
> - Don't add compatible strings with an "-spi" suffix (Geert Uytterhoeven).

Thanks for the update!

> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> @@ -39,9 +39,14 @@ properties:
>    reset-gpios:
>      maxItems: 1
>
> +  dc-gpios:
> +    maxItems: 1
> +

Perhaps add a description, and clarify this is for SPI only?

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

Re: [PATCH v2 3/5] drm/solomon: Add ssd130x new compatible strings and deprecate old ones.

[Geert Uytterhoeven]

Hi Javier,

On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> The current compatible strings for SSD130x I2C controllers contain an "fb"
> and "-i2c" suffixes. These have been deprecated and more correct ones were
> added, that don't encode a subsystem or bus used to interface the devices.
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Mark Brown <broonie@kernel.org>
> ---
>
> (no changes since v1)

That's not really true ;-)

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

Re: [PATCH v2 3/5] drm/solomon: Add ssd130x new compatible strings and deprecate old ones.

[Geert Uytterhoeven]

Hi Javier,

On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> The current compatible strings for SSD130x I2C controllers contain an "fb"
> and "-i2c" suffixes. These have been deprecated and more correct ones were
> added, that don't encode a subsystem or bus used to interface the devices.
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Mark Brown <broonie@kernel.org>
> ---
>
> (no changes since v1)

That's not really true ;-)

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

Re: [PATCH v2 4/5] drm/solomon: Move device info from ssd130x-i2c to the core driver

[Geert Uytterhoeven]

Hi Javier,

Thanks for your patch!

On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> These are declared in the ssd130x-i2c transport driver but the information
> is not I2C specific, and could be used by other SSD130x transport drivers.
>
> Move them to the ssd130x core driver and just set the OF device entries to
> an ID that could be used to lookup the correct device info from an array.
>
> While being there, also move the SSD130X_DATA and SSD130X_COMMAND control
> bytes. Since even though are used by the I2C interface, it could also be

though they are ... they could

> useful for other transport protocols such as SPI.
>
> Suggested-by: Chen-Yu Tsai <wens@kernel.org>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

> --- a/drivers/gpu/drm/solomon/ssd130x.c
> +++ b/drivers/gpu/drm/solomon/ssd130x.c

> @@ -860,7 +890,14 @@ struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap)
>
>         ssd130x->dev = dev;
>         ssd130x->regmap = regmap;
> -       ssd130x->device_info = device_get_match_data(dev);
> +
> +       variant = (enum ssd130x_variants)device_get_match_data(dev);

(uintptr_t), to avoid a cast from pointer to integer of different
size warning.

> +

Please drop the blank line.

> +       if (variant >= NR_SSD130X_VARIANTS)
> +               return ERR_PTR(dev_err_probe(dev, -EINVAL,
> +                                            "Invalid SSD130x variant\n"));
> +
> +       ssd130x->device_info = &ssd130x_variants[variant];
>
>         if (ssd130x->device_info->page_mode_only)
>                 ssd130x->page_address_mode = 1;

With the above fixed:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

Re: [PATCH v2 4/5] drm/solomon: Move device info from ssd130x-i2c to the core driver

[Geert Uytterhoeven]

Hi Javier,

Thanks for your patch!

On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> These are declared in the ssd130x-i2c transport driver but the information
> is not I2C specific, and could be used by other SSD130x transport drivers.
>
> Move them to the ssd130x core driver and just set the OF device entries to
> an ID that could be used to lookup the correct device info from an array.
>
> While being there, also move the SSD130X_DATA and SSD130X_COMMAND control
> bytes. Since even though are used by the I2C interface, it could also be

though they are ... they could

> useful for other transport protocols such as SPI.
>
> Suggested-by: Chen-Yu Tsai <wens@kernel.org>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

> --- a/drivers/gpu/drm/solomon/ssd130x.c
> +++ b/drivers/gpu/drm/solomon/ssd130x.c

> @@ -860,7 +890,14 @@ struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap)
>
>         ssd130x->dev = dev;
>         ssd130x->regmap = regmap;
> -       ssd130x->device_info = device_get_match_data(dev);
> +
> +       variant = (enum ssd130x_variants)device_get_match_data(dev);

(uintptr_t), to avoid a cast from pointer to integer of different
size warning.

> +

Please drop the blank line.

> +       if (variant >= NR_SSD130X_VARIANTS)
> +               return ERR_PTR(dev_err_probe(dev, -EINVAL,
> +                                            "Invalid SSD130x variant\n"));
> +
> +       ssd130x->device_info = &ssd130x_variants[variant];
>
>         if (ssd130x->device_info->page_mode_only)
>                 ssd130x->page_address_mode = 1;

With the above fixed:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

Re: [PATCH v2 4/5] drm/solomon: Move device info from ssd130x-i2c to the core driver

[kernel test robot]

Hi Javier,

I love your patch! Perhaps something to improve:

[auto build test WARNING on drm-tip/drm-tip]
[also build test WARNING on next-20220411]
[cannot apply to drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next tegra-drm/drm/tegra/for-next linus/master linux/master v5.18-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Javier-Martinez-Canillas/drm-solomon-Add-SSD130x-OLED-displays-SPI-support/20220412-051518
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-randconfig-a001-20220411 (https://download.01.org/0day-ci/archive/20220412/202204121542.aU2BiYXN-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project fe2478d44e4f7f191c43fef629ac7a23d0251e72)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/ac5a07cda8a0f8d4948e6a01d0b3bb6ce9fe7830
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Javier-Martinez-Canillas/drm-solomon-Add-SSD130x-OLED-displays-SPI-support/20220412-051518
        git checkout ac5a07cda8a0f8d4948e6a01d0b3bb6ce9fe7830
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/solomon/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/solomon/ssd130x.c:894:12: warning: cast to smaller integer type 'enum ssd130x_variants' from 'const void *' [-Wvoid-pointer-to-enum-cast]
           variant = (enum ssd130x_variants)device_get_match_data(dev);
                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +894 drivers/gpu/drm/solomon/ssd130x.c

   874	
   875	struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap)
   876	{
   877		struct ssd130x_device *ssd130x;
   878		enum ssd130x_variants variant;
   879		struct backlight_device *bl;
   880		struct drm_device *drm;
   881		int ret;
   882	
   883		ssd130x = devm_drm_dev_alloc(dev, &ssd130x_drm_driver,
   884					     struct ssd130x_device, drm);
   885		if (IS_ERR(ssd130x))
   886			return ERR_PTR(dev_err_probe(dev, PTR_ERR(ssd130x),
   887						     "Failed to allocate DRM device\n"));
   888	
   889		drm = &ssd130x->drm;
   890	
   891		ssd130x->dev = dev;
   892		ssd130x->regmap = regmap;
   893	
 > 894		variant = (enum ssd130x_variants)device_get_match_data(dev);
   895	
   896		if (variant >= NR_SSD130X_VARIANTS)
   897			return ERR_PTR(dev_err_probe(dev, -EINVAL,
   898						     "Invalid SSD130x variant\n"));
   899	
   900		ssd130x->device_info = &ssd130x_variants[variant];
   901	
   902		if (ssd130x->device_info->page_mode_only)
   903			ssd130x->page_address_mode = 1;
   904	
   905		ssd130x_parse_properties(ssd130x);
   906	
   907		ret = ssd130x_get_resources(ssd130x);
   908		if (ret)
   909			return ERR_PTR(ret);
   910	
   911		bl = devm_backlight_device_register(dev, dev_name(dev), dev, ssd130x,
   912						    &ssd130xfb_bl_ops, NULL);
   913		if (IS_ERR(bl))
   914			return ERR_PTR(dev_err_probe(dev, PTR_ERR(bl),
   915						     "Unable to register backlight device\n"));
   916	
   917		bl->props.brightness = ssd130x->contrast;
   918		bl->props.max_brightness = MAX_CONTRAST;
   919		ssd130x->bl_dev = bl;
   920	
   921		ret = ssd130x_init_modeset(ssd130x);
   922		if (ret)
   923			return ERR_PTR(ret);
   924	
   925		ret = drm_dev_register(drm, 0);
   926		if (ret)
   927			return ERR_PTR(dev_err_probe(dev, ret, "DRM device register failed\n"));
   928	
   929		drm_fbdev_generic_setup(drm, 0);
   930	
   931		return ssd130x;
   932	}
   933	EXPORT_SYMBOL_GPL(ssd130x_probe);
   934	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v2 4/5] drm/solomon: Move device info from ssd130x-i2c to the core driver

[kernel test robot]

Hi Javier,

I love your patch! Perhaps something to improve:

[auto build test WARNING on drm-tip/drm-tip]
[also build test WARNING on next-20220411]
[cannot apply to drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next tegra-drm/drm/tegra/for-next linus/master linux/master v5.18-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Javier-Martinez-Canillas/drm-solomon-Add-SSD130x-OLED-displays-SPI-support/20220412-051518
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-randconfig-a001-20220411 (https://download.01.org/0day-ci/archive/20220412/202204121542.aU2BiYXN-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project fe2478d44e4f7f191c43fef629ac7a23d0251e72)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/ac5a07cda8a0f8d4948e6a01d0b3bb6ce9fe7830
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Javier-Martinez-Canillas/drm-solomon-Add-SSD130x-OLED-displays-SPI-support/20220412-051518
        git checkout ac5a07cda8a0f8d4948e6a01d0b3bb6ce9fe7830
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/solomon/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/solomon/ssd130x.c:894:12: warning: cast to smaller integer type 'enum ssd130x_variants' from 'const void *' [-Wvoid-pointer-to-enum-cast]
           variant = (enum ssd130x_variants)device_get_match_data(dev);
                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +894 drivers/gpu/drm/solomon/ssd130x.c

   874	
   875	struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap)
   876	{
   877		struct ssd130x_device *ssd130x;
   878		enum ssd130x_variants variant;
   879		struct backlight_device *bl;
   880		struct drm_device *drm;
   881		int ret;
   882	
   883		ssd130x = devm_drm_dev_alloc(dev, &ssd130x_drm_driver,
   884					     struct ssd130x_device, drm);
   885		if (IS_ERR(ssd130x))
   886			return ERR_PTR(dev_err_probe(dev, PTR_ERR(ssd130x),
   887						     "Failed to allocate DRM device\n"));
   888	
   889		drm = &ssd130x->drm;
   890	
   891		ssd130x->dev = dev;
   892		ssd130x->regmap = regmap;
   893	
 > 894		variant = (enum ssd130x_variants)device_get_match_data(dev);
   895	
   896		if (variant >= NR_SSD130X_VARIANTS)
   897			return ERR_PTR(dev_err_probe(dev, -EINVAL,
   898						     "Invalid SSD130x variant\n"));
   899	
   900		ssd130x->device_info = &ssd130x_variants[variant];
   901	
   902		if (ssd130x->device_info->page_mode_only)
   903			ssd130x->page_address_mode = 1;
   904	
   905		ssd130x_parse_properties(ssd130x);
   906	
   907		ret = ssd130x_get_resources(ssd130x);
   908		if (ret)
   909			return ERR_PTR(ret);
   910	
   911		bl = devm_backlight_device_register(dev, dev_name(dev), dev, ssd130x,
   912						    &ssd130xfb_bl_ops, NULL);
   913		if (IS_ERR(bl))
   914			return ERR_PTR(dev_err_probe(dev, PTR_ERR(bl),
   915						     "Unable to register backlight device\n"));
   916	
   917		bl->props.brightness = ssd130x->contrast;
   918		bl->props.max_brightness = MAX_CONTRAST;
   919		ssd130x->bl_dev = bl;
   920	
   921		ret = ssd130x_init_modeset(ssd130x);
   922		if (ret)
   923			return ERR_PTR(ret);
   924	
   925		ret = drm_dev_register(drm, 0);
   926		if (ret)
   927			return ERR_PTR(dev_err_probe(dev, ret, "DRM device register failed\n"));
   928	
   929		drm_fbdev_generic_setup(drm, 0);
   930	
   931		return ssd130x;
   932	}
   933	EXPORT_SYMBOL_GPL(ssd130x_probe);
   934	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v2 5/5] drm/solomon: Add SSD130x OLED displays SPI support

[Geert Uytterhoeven]

Hi Javier,

On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> The ssd130x driver only provides the core support for these devices but it
> does not have any bus transport logic. Add a driver to interface over SPI.
>
> There is a difference in the communication protocol when using 4-wire SPI
> instead of I2C. For the latter, a control byte that contains a D/C# field
> has to be sent. This field tells the controller whether the data has to be
> written to the command register or to the graphics display data memory.
>
> But for 4-wire SPI that control byte is not used, instead a real D/C# line
> must be pulled HIGH for commands data and LOW for graphics display data.
>
> For this reason the standard SPI regmap can't be used and a custom .write
> bus handler is needed.
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Mark Brown <broonie@kernel.org>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/gpu/drm/solomon/ssd130x-spi.c

> +static struct gpio_desc *ssd130x_spi_get_dc(struct device *dev)
> +{
> +       struct gpio_desc *dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW);
> +
> +       if (IS_ERR(dc))
> +               return ERR_PTR(dev_err_probe(dev, PTR_ERR(dc), "Failed to get dc gpio\n"));
> +
> +       return dc;
> +}
> +
> +static int ssd130x_spi_probe(struct spi_device *spi)
> +{
> +       struct ssd130x_spi_transport *t;
> +       struct ssd130x_device *ssd130x;
> +       struct regmap *regmap;
> +       struct device *dev = &spi->dev;
> +
> +       t = devm_kzalloc(dev, sizeof(*t), GFP_KERNEL);
> +       if (!t)
> +               return dev_err_probe(dev, -ENOMEM,
> +                                    "Failed to allocate SPI transport data\n");
> +
> +       t->spi = spi;
> +
> +       t->dc = ssd130x_spi_get_dc(&spi->dev);
> +       if (IS_ERR(t->dc))
> +               return PTR_ERR(t->dc);

This can be simplified (no need for the PTR_ERR(ERR_PTR(...) dance)
by open-coding ssd130x_spi_get_dc() here.

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

Re: [PATCH v2 5/5] drm/solomon: Add SSD130x OLED displays SPI support

[Geert Uytterhoeven]

Hi Javier,

On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> The ssd130x driver only provides the core support for these devices but it
> does not have any bus transport logic. Add a driver to interface over SPI.
>
> There is a difference in the communication protocol when using 4-wire SPI
> instead of I2C. For the latter, a control byte that contains a D/C# field
> has to be sent. This field tells the controller whether the data has to be
> written to the command register or to the graphics display data memory.
>
> But for 4-wire SPI that control byte is not used, instead a real D/C# line
> must be pulled HIGH for commands data and LOW for graphics display data.
>
> For this reason the standard SPI regmap can't be used and a custom .write
> bus handler is needed.
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Mark Brown <broonie@kernel.org>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/gpu/drm/solomon/ssd130x-spi.c

> +static struct gpio_desc *ssd130x_spi_get_dc(struct device *dev)
> +{
> +       struct gpio_desc *dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW);
> +
> +       if (IS_ERR(dc))
> +               return ERR_PTR(dev_err_probe(dev, PTR_ERR(dc), "Failed to get dc gpio\n"));
> +
> +       return dc;
> +}
> +
> +static int ssd130x_spi_probe(struct spi_device *spi)
> +{
> +       struct ssd130x_spi_transport *t;
> +       struct ssd130x_device *ssd130x;
> +       struct regmap *regmap;
> +       struct device *dev = &spi->dev;
> +
> +       t = devm_kzalloc(dev, sizeof(*t), GFP_KERNEL);
> +       if (!t)
> +               return dev_err_probe(dev, -ENOMEM,
> +                                    "Failed to allocate SPI transport data\n");
> +
> +       t->spi = spi;
> +
> +       t->dc = ssd130x_spi_get_dc(&spi->dev);
> +       if (IS_ERR(t->dc))
> +               return PTR_ERR(t->dc);

This can be simplified (no need for the PTR_ERR(ERR_PTR(...) dance)
by open-coding ssd130x_spi_get_dc() here.

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

Re: [PATCH v2 2/5] dt-bindings: display: ssd1307fb: Extend schema for SPI controllers

[Javier Martinez Canillas]

Hello Geert,

On 4/12/22 09:16, Geert Uytterhoeven wrote:
> Hi Javier,
> 
> On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> The Solomon SSD130x OLED displays can either have an I2C or SPI interface,
>> add to the schema the properties and examples for OLED devices under SPI.
>>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>> Acked-by: Mark Brown <broonie@kernel.org>
>> ---
>>
>> Changes in v2:
>> - Don't add compatible strings with an "-spi" suffix (Geert Uytterhoeven).
> 
> Thanks for the update!
>

You are welcome and thanks for your prompt review and feedback!
 
>> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
>> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
>> @@ -39,9 +39,14 @@ properties:
>>    reset-gpios:
>>      maxItems: 1
>>
>> +  dc-gpios:
>> +    maxItems: 1
>> +
> 
> Perhaps add a description, and clarify this is for SPI only?
> 

I wondered how to make it required for SPI but couldn't find another binding
that did the same and I'm not that familiar with DT schemas to figure it out.

Before, when I had compatible strings just for SPI I could do the following:

  - if:
      properties:
        compatible:
          contains:
            enum:
              - sinowealth,sh1106-spi
              - solomon,ssd1305-spi
              - solomon,ssd1306-spi
              - solomon,ssd1307-spi
              - solomon,ssd1309-spi
    then:
      required:
        - spi-max-frequency
        - dc-gpios

but now that we are using the same compatible strings for I2C and SPI, the
compatible string can't be used anymore as an indication to make required.

Do you have any hints here on how I should enforce this in the schema ?

Or if you think that a comment is enough, then I will add it in v3.

> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>

Thanks!

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 2/5] dt-bindings: display: ssd1307fb: Extend schema for SPI controllers

[Javier Martinez Canillas]

Hello Geert,

On 4/12/22 09:16, Geert Uytterhoeven wrote:
> Hi Javier,
> 
> On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> The Solomon SSD130x OLED displays can either have an I2C or SPI interface,
>> add to the schema the properties and examples for OLED devices under SPI.
>>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>> Acked-by: Mark Brown <broonie@kernel.org>
>> ---
>>
>> Changes in v2:
>> - Don't add compatible strings with an "-spi" suffix (Geert Uytterhoeven).
> 
> Thanks for the update!
>

You are welcome and thanks for your prompt review and feedback!
 
>> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
>> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
>> @@ -39,9 +39,14 @@ properties:
>>    reset-gpios:
>>      maxItems: 1
>>
>> +  dc-gpios:
>> +    maxItems: 1
>> +
> 
> Perhaps add a description, and clarify this is for SPI only?
> 

I wondered how to make it required for SPI but couldn't find another binding
that did the same and I'm not that familiar with DT schemas to figure it out.

Before, when I had compatible strings just for SPI I could do the following:

  - if:
      properties:
        compatible:
          contains:
            enum:
              - sinowealth,sh1106-spi
              - solomon,ssd1305-spi
              - solomon,ssd1306-spi
              - solomon,ssd1307-spi
              - solomon,ssd1309-spi
    then:
      required:
        - spi-max-frequency
        - dc-gpios

but now that we are using the same compatible strings for I2C and SPI, the
compatible string can't be used anymore as an indication to make required.

Do you have any hints here on how I should enforce this in the schema ?

Or if you think that a comment is enough, then I will add it in v3.

> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>

Thanks!

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 3/5] drm/solomon: Add ssd130x new compatible strings and deprecate old ones.

[Javier Martinez Canillas]

On 4/12/22 09:19, Geert Uytterhoeven wrote:
> Hi Javier,
> 
> On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> The current compatible strings for SSD130x I2C controllers contain an "fb"
>> and "-i2c" suffixes. These have been deprecated and more correct ones were
>> added, that don't encode a subsystem or bus used to interface the devices.
>>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>> Acked-by: Mark Brown <broonie@kernel.org>
>> ---
>>
>> (no changes since v1)
> 
> That's not really true ;-)
>

Ups, I did indeed update the compatible strings to drop the "-i2c" as you
asked. I'll add that in the patch history for v2 when posting v3.
 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 

Thanks!

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 3/5] drm/solomon: Add ssd130x new compatible strings and deprecate old ones.

[Javier Martinez Canillas]

On 4/12/22 09:19, Geert Uytterhoeven wrote:
> Hi Javier,
> 
> On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> The current compatible strings for SSD130x I2C controllers contain an "fb"
>> and "-i2c" suffixes. These have been deprecated and more correct ones were
>> added, that don't encode a subsystem or bus used to interface the devices.
>>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>> Acked-by: Mark Brown <broonie@kernel.org>
>> ---
>>
>> (no changes since v1)
> 
> That's not really true ;-)
>

Ups, I did indeed update the compatible strings to drop the "-i2c" as you
asked. I'll add that in the patch history for v2 when posting v3.
 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 

Thanks!

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 4/5] drm/solomon: Move device info from ssd130x-i2c to the core driver

[Javier Martinez Canillas]

On 4/12/22 09:23, Geert Uytterhoeven wrote:
> Hi Javier,
> 
> Thanks for your patch!
> 
> On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> These are declared in the ssd130x-i2c transport driver but the information
>> is not I2C specific, and could be used by other SSD130x transport drivers.
>>
>> Move them to the ssd130x core driver and just set the OF device entries to
>> an ID that could be used to lookup the correct device info from an array.
>>
>> While being there, also move the SSD130X_DATA and SSD130X_COMMAND control
>> bytes. Since even though are used by the I2C interface, it could also be
> 
> though they are ... they could
>

Right, will fix it.
 
>> useful for other transport protocols such as SPI.
>>
>> Suggested-by: Chen-Yu Tsai <wens@kernel.org>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> 
>> --- a/drivers/gpu/drm/solomon/ssd130x.c
>> +++ b/drivers/gpu/drm/solomon/ssd130x.c
> 
>> @@ -860,7 +890,14 @@ struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap)
>>
>>         ssd130x->dev = dev;
>>         ssd130x->regmap = regmap;
>> -       ssd130x->device_info = device_get_match_data(dev);
>> +
>> +       variant = (enum ssd130x_variants)device_get_match_data(dev);
> 
> (uintptr_t), to avoid a cast from pointer to integer of different
> size warning.
>

Indeed. The kernel test robot reported the same.
 
>> +
> 
> Please drop the blank line.
>

Ok.
 
>> +       if (variant >= NR_SSD130X_VARIANTS)
>> +               return ERR_PTR(dev_err_probe(dev, -EINVAL,
>> +                                            "Invalid SSD130x variant\n"));
>> +
>> +       ssd130x->device_info = &ssd130x_variants[variant];
>>
>>         if (ssd130x->device_info->page_mode_only)
>>                 ssd130x->page_address_mode = 1;
> 
> With the above fixed:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>

Thanks!

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 4/5] drm/solomon: Move device info from ssd130x-i2c to the core driver

[Javier Martinez Canillas]

On 4/12/22 09:23, Geert Uytterhoeven wrote:
> Hi Javier,
> 
> Thanks for your patch!
> 
> On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
> <javierm@redhat.com> wrote:
>> These are declared in the ssd130x-i2c transport driver but the information
>> is not I2C specific, and could be used by other SSD130x transport drivers.
>>
>> Move them to the ssd130x core driver and just set the OF device entries to
>> an ID that could be used to lookup the correct device info from an array.
>>
>> While being there, also move the SSD130X_DATA and SSD130X_COMMAND control
>> bytes. Since even though are used by the I2C interface, it could also be
> 
> though they are ... they could
>

Right, will fix it.
 
>> useful for other transport protocols such as SPI.
>>
>> Suggested-by: Chen-Yu Tsai <wens@kernel.org>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> 
>> --- a/drivers/gpu/drm/solomon/ssd130x.c
>> +++ b/drivers/gpu/drm/solomon/ssd130x.c
> 
>> @@ -860,7 +890,14 @@ struct ssd130x_device *ssd130x_probe(struct device *dev, struct regmap *regmap)
>>
>>         ssd130x->dev = dev;
>>         ssd130x->regmap = regmap;
>> -       ssd130x->device_info = device_get_match_data(dev);
>> +
>> +       variant = (enum ssd130x_variants)device_get_match_data(dev);
> 
> (uintptr_t), to avoid a cast from pointer to integer of different
> size warning.
>

Indeed. The kernel test robot reported the same.
 
>> +
> 
> Please drop the blank line.
>

Ok.
 
>> +       if (variant >= NR_SSD130X_VARIANTS)
>> +               return ERR_PTR(dev_err_probe(dev, -EINVAL,
>> +                                            "Invalid SSD130x variant\n"));
>> +
>> +       ssd130x->device_info = &ssd130x_variants[variant];
>>
>>         if (ssd130x->device_info->page_mode_only)
>>                 ssd130x->page_address_mode = 1;
> 
> With the above fixed:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>

Thanks!

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 2/5] dt-bindings: display: ssd1307fb: Extend schema for SPI controllers

[Geert Uytterhoeven]

Hi Javier,

On Tue, Apr 12, 2022 at 10:01 AM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> On 4/12/22 09:16, Geert Uytterhoeven wrote:
> > On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
> > <javierm@redhat.com> wrote:
> >> The Solomon SSD130x OLED displays can either have an I2C or SPI interface,
> >> add to the schema the properties and examples for OLED devices under SPI.
> >>
> >> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

> >> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> >> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> >> @@ -39,9 +39,14 @@ properties:
> >>    reset-gpios:
> >>      maxItems: 1
> >>
> >> +  dc-gpios:
> >> +    maxItems: 1
> >> +
> >
> > Perhaps add a description, and clarify this is for SPI only?
>
> I wondered how to make it required for SPI but couldn't find another binding
> that did the same and I'm not that familiar with DT schemas to figure it out.
>
> Before, when I had compatible strings just for SPI I could do the following:
>
>   - if:
>       properties:
>         compatible:
>           contains:
>             enum:
>               - sinowealth,sh1106-spi
>               - solomon,ssd1305-spi
>               - solomon,ssd1306-spi
>               - solomon,ssd1307-spi
>               - solomon,ssd1309-spi
>     then:
>       required:
>         - spi-max-frequency
>         - dc-gpios
>
> but now that we are using the same compatible strings for I2C and SPI, the
> compatible string can't be used anymore as an indication to make required.
>
> Do you have any hints here on how I should enforce this in the schema ?
>
> Or if you think that a comment is enough, then I will add it in v3.

I don't know how to make it required for SPI, if possible at all.

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

Re: [PATCH v2 2/5] dt-bindings: display: ssd1307fb: Extend schema for SPI controllers

[Geert Uytterhoeven]

Hi Javier,

On Tue, Apr 12, 2022 at 10:01 AM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> On 4/12/22 09:16, Geert Uytterhoeven wrote:
> > On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
> > <javierm@redhat.com> wrote:
> >> The Solomon SSD130x OLED displays can either have an I2C or SPI interface,
> >> add to the schema the properties and examples for OLED devices under SPI.
> >>
> >> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

> >> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> >> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> >> @@ -39,9 +39,14 @@ properties:
> >>    reset-gpios:
> >>      maxItems: 1
> >>
> >> +  dc-gpios:
> >> +    maxItems: 1
> >> +
> >
> > Perhaps add a description, and clarify this is for SPI only?
>
> I wondered how to make it required for SPI but couldn't find another binding
> that did the same and I'm not that familiar with DT schemas to figure it out.
>
> Before, when I had compatible strings just for SPI I could do the following:
>
>   - if:
>       properties:
>         compatible:
>           contains:
>             enum:
>               - sinowealth,sh1106-spi
>               - solomon,ssd1305-spi
>               - solomon,ssd1306-spi
>               - solomon,ssd1307-spi
>               - solomon,ssd1309-spi
>     then:
>       required:
>         - spi-max-frequency
>         - dc-gpios
>
> but now that we are using the same compatible strings for I2C and SPI, the
> compatible string can't be used anymore as an indication to make required.
>
> Do you have any hints here on how I should enforce this in the schema ?
>
> Or if you think that a comment is enough, then I will add it in v3.

I don't know how to make it required for SPI, if possible at all.

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

Re: [PATCH v2 5/5] drm/solomon: Add SSD130x OLED displays SPI support

[Javier Martinez Canillas]

On 4/12/22 09:31, Geert Uytterhoeven wrote:

[snip]

>> +
>> +       t->spi = spi;
>> +
>> +       t->dc = ssd130x_spi_get_dc(&spi->dev);
>> +       if (IS_ERR(t->dc))
>> +               return PTR_ERR(t->dc);
> 
> This can be simplified (no need for the PTR_ERR(ERR_PTR(...) dance)
> by open-coding ssd130x_spi_get_dc() here.
>

Right, that will be better indeed.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 5/5] drm/solomon: Add SSD130x OLED displays SPI support

[Javier Martinez Canillas]

On 4/12/22 09:31, Geert Uytterhoeven wrote:

[snip]

>> +
>> +       t->spi = spi;
>> +
>> +       t->dc = ssd130x_spi_get_dc(&spi->dev);
>> +       if (IS_ERR(t->dc))
>> +               return PTR_ERR(t->dc);
> 
> This can be simplified (no need for the PTR_ERR(ERR_PTR(...) dance)
> by open-coding ssd130x_spi_get_dc() here.
>

Right, that will be better indeed.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 2/5] dt-bindings: display: ssd1307fb: Extend schema for SPI controllers

[Javier Martinez Canillas]

On 4/12/22 10:07, Geert Uytterhoeven wrote:
> Hi Javier,

[snip]

>>
>> Do you have any hints here on how I should enforce this in the schema ?
>>
>> Or if you think that a comment is enough, then I will add it in v3.
> 
> I don't know how to make it required for SPI, if possible at all.
>

I see. Let's go with a comment then and we can later enforce it, if someone
knows if is possible / how to do it.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 2/5] dt-bindings: display: ssd1307fb: Extend schema for SPI controllers

[Javier Martinez Canillas]

On 4/12/22 10:07, Geert Uytterhoeven wrote:
> Hi Javier,

[snip]

>>
>> Do you have any hints here on how I should enforce this in the schema ?
>>
>> Or if you think that a comment is enough, then I will add it in v3.
> 
> I don't know how to make it required for SPI, if possible at all.
>

I see. Let's go with a comment then and we can later enforce it, if someone
knows if is possible / how to do it.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 5/5] drm/solomon: Add SSD130x OLED displays SPI support

[kernel test robot]

Hi Javier,

I love your patch! Perhaps something to improve:

[auto build test WARNING on drm-tip/drm-tip]
[also build test WARNING on next-20220412]
[cannot apply to drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next tegra-drm/drm/tegra/for-next linus/master linux/master airlied/drm-next v5.18-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Javier-Martinez-Canillas/drm-solomon-Add-SSD130x-OLED-displays-SPI-support/20220412-051518
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: hexagon-allyesconfig (https://download.01.org/0day-ci/archive/20220412/202204121654.38UTab7Q-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project fe2478d44e4f7f191c43fef629ac7a23d0251e72)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/46bbef7fc1afeb9bc8241fe7636e77b5096e3d22
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Javier-Martinez-Canillas/drm-solomon-Add-SSD130x-OLED-displays-SPI-support/20220412-051518
        git checkout 46bbef7fc1afeb9bc8241fe7636e77b5096e3d22
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/gpu/drm/solomon/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/solomon/ssd130x-spi.c:161:35: warning: unused variable 'ssd130x_spi_table' [-Wunused-const-variable]
   static const struct spi_device_id ssd130x_spi_table[] = {
                                     ^
   1 warning generated.


vim +/ssd130x_spi_table +161 drivers/gpu/drm/solomon/ssd130x-spi.c

   152	
   153	/*
   154	 * The SPI core always reports a MODALIAS uevent of the form "spi:<dev>", even
   155	 * if the device was registered via OF. This means that the module will not be
   156	 * auto loaded, unless it contains an alias that matches the MODALIAS reported.
   157	 *
   158	 * To workaround this issue, add a SPI device ID table. Even when this should
   159	 * not be needed for this driver to match the registered SPI devices.
   160	 */
 > 161	static const struct spi_device_id ssd130x_spi_table[] = {
   162		{ "sh1106",  SH1106_ID },
   163		{ "ssd1305", SSD1305_ID },
   164		{ "ssd1306", SSD1306_ID },
   165		{ "ssd1307", SSD1307_ID },
   166		{ "ssd1309", SSD1309_ID },
   167		{ /* sentinel */ }
   168	};
   169	MODULE_DEVICE_TABLE(spi, ssd130x_spi_table);
   170	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v2 5/5] drm/solomon: Add SSD130x OLED displays SPI support

[kernel test robot]

Hi Javier,

I love your patch! Perhaps something to improve:

[auto build test WARNING on drm-tip/drm-tip]
[also build test WARNING on next-20220412]
[cannot apply to drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next tegra-drm/drm/tegra/for-next linus/master linux/master airlied/drm-next v5.18-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Javier-Martinez-Canillas/drm-solomon-Add-SSD130x-OLED-displays-SPI-support/20220412-051518
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: hexagon-allyesconfig (https://download.01.org/0day-ci/archive/20220412/202204121654.38UTab7Q-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project fe2478d44e4f7f191c43fef629ac7a23d0251e72)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/46bbef7fc1afeb9bc8241fe7636e77b5096e3d22
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Javier-Martinez-Canillas/drm-solomon-Add-SSD130x-OLED-displays-SPI-support/20220412-051518
        git checkout 46bbef7fc1afeb9bc8241fe7636e77b5096e3d22
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/gpu/drm/solomon/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/solomon/ssd130x-spi.c:161:35: warning: unused variable 'ssd130x_spi_table' [-Wunused-const-variable]
   static const struct spi_device_id ssd130x_spi_table[] = {
                                     ^
   1 warning generated.


vim +/ssd130x_spi_table +161 drivers/gpu/drm/solomon/ssd130x-spi.c

   152	
   153	/*
   154	 * The SPI core always reports a MODALIAS uevent of the form "spi:<dev>", even
   155	 * if the device was registered via OF. This means that the module will not be
   156	 * auto loaded, unless it contains an alias that matches the MODALIAS reported.
   157	 *
   158	 * To workaround this issue, add a SPI device ID table. Even when this should
   159	 * not be needed for this driver to match the registered SPI devices.
   160	 */
 > 161	static const struct spi_device_id ssd130x_spi_table[] = {
   162		{ "sh1106",  SH1106_ID },
   163		{ "ssd1305", SSD1305_ID },
   164		{ "ssd1306", SSD1306_ID },
   165		{ "ssd1307", SSD1307_ID },
   166		{ "ssd1309", SSD1309_ID },
   167		{ /* sentinel */ }
   168	};
   169	MODULE_DEVICE_TABLE(spi, ssd130x_spi_table);
   170	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v2 4/5] drm/solomon: Move device info from ssd130x-i2c to the core driver

[Andy Shevchenko]

On Tue, Apr 12, 2022 at 10:07:02AM +0200, Javier Martinez Canillas wrote:
> On 4/12/22 09:23, Geert Uytterhoeven wrote:
> > On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
> > <javierm@redhat.com> wrote:

...

> >> -       ssd130x->device_info = device_get_match_data(dev);
> >> +
> >> +       variant = (enum ssd130x_variants)device_get_match_data(dev);
> > 
> > (uintptr_t), to avoid a cast from pointer to integer of different
> > size warning.
> >
> 
> Indeed. The kernel test robot reported the same.

Not only because of this, but also with the non-NULL pointers I prefer the old
style without ugly castings.

Instead, you may export the array (in the driver's namespace) and use
&info[ID] pointer for the specific device info.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 4/5] drm/solomon: Move device info from ssd130x-i2c to the core driver

[Andy Shevchenko]

On Tue, Apr 12, 2022 at 10:07:02AM +0200, Javier Martinez Canillas wrote:
> On 4/12/22 09:23, Geert Uytterhoeven wrote:
> > On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
> > <javierm@redhat.com> wrote:

...

> >> -       ssd130x->device_info = device_get_match_data(dev);
> >> +
> >> +       variant = (enum ssd130x_variants)device_get_match_data(dev);
> > 
> > (uintptr_t), to avoid a cast from pointer to integer of different
> > size warning.
> >
> 
> Indeed. The kernel test robot reported the same.

Not only because of this, but also with the non-NULL pointers I prefer the old
style without ugly castings.

Instead, you may export the array (in the driver's namespace) and use
&info[ID] pointer for the specific device info.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 4/5] drm/solomon: Move device info from ssd130x-i2c to the core driver

[Andy Shevchenko]

On Tue, Apr 12, 2022 at 02:21:08PM +0300, Andy Shevchenko wrote:
> On Tue, Apr 12, 2022 at 10:07:02AM +0200, Javier Martinez Canillas wrote:
> > On 4/12/22 09:23, Geert Uytterhoeven wrote:
> > > On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
> > > <javierm@redhat.com> wrote:

...

> > >> -       ssd130x->device_info = device_get_match_data(dev);
> > >> +
> > >> +       variant = (enum ssd130x_variants)device_get_match_data(dev);
> > > 
> > > (uintptr_t), to avoid a cast from pointer to integer of different
> > > size warning.
> > >
> > 
> > Indeed. The kernel test robot reported the same.
> 
> Not only because of this, but also with the non-NULL pointers I prefer the old
> style without ugly castings.
> 
> Instead, you may export the array (in the driver's namespace) and use
> &info[ID] pointer for the specific device info.

Note that device_get_match_data() has no clue if the data is absent or
data == (void *)0.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 4/5] drm/solomon: Move device info from ssd130x-i2c to the core driver

[Andy Shevchenko]

On Tue, Apr 12, 2022 at 02:21:08PM +0300, Andy Shevchenko wrote:
> On Tue, Apr 12, 2022 at 10:07:02AM +0200, Javier Martinez Canillas wrote:
> > On 4/12/22 09:23, Geert Uytterhoeven wrote:
> > > On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
> > > <javierm@redhat.com> wrote:

...

> > >> -       ssd130x->device_info = device_get_match_data(dev);
> > >> +
> > >> +       variant = (enum ssd130x_variants)device_get_match_data(dev);
> > > 
> > > (uintptr_t), to avoid a cast from pointer to integer of different
> > > size warning.
> > >
> > 
> > Indeed. The kernel test robot reported the same.
> 
> Not only because of this, but also with the non-NULL pointers I prefer the old
> style without ugly castings.
> 
> Instead, you may export the array (in the driver's namespace) and use
> &info[ID] pointer for the specific device info.

Note that device_get_match_data() has no clue if the data is absent or
data == (void *)0.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/5] dt-bindings: display: ssd1307fb: Deprecate "-i2c" compatible strings

[Maxime Ripard]

On Mon, Apr 11, 2022 at 11:12:39PM +0200, Javier Martinez Canillas wrote:
> The current compatible strings for SSD130x I2C controllers contain both an
> "fb" and "-i2c" suffixes. It seems to indicate that are for a fbdev driver
> and also that are for devices that can be accessed over an I2C bus.
> 
> But a DT is supposed to describe the hardware and not Linux implementation
> details. So let's deprecate those compatible strings and add new ones that
> only contain the vendor and device name, without any of these suffixes.
> 
> These will just describe the device and can be matched by both I2C and SPI
> DRM drivers.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Mark Brown <broonie@kernel.org>
> ---
> 
> Changes in v2:
> - Drop the -i2c suffixes from the compatible strings too (Geert Uytterhoeven).
> 
>  .../bindings/display/solomon,ssd1307fb.yaml   | 37 ++++++++++++-------
>  1 file changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> index ade61d502edd..6b9d0c72739a 100644
> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> @@ -12,12 +12,23 @@ maintainers:
>  
>  properties:
>    compatible:
> -    enum:
> -      - sinowealth,sh1106-i2c
> -      - solomon,ssd1305fb-i2c
> -      - solomon,ssd1306fb-i2c
> -      - solomon,ssd1307fb-i2c
> -      - solomon,ssd1309fb-i2c
> +    oneOf:
> +      # Deprecated compatible strings
> +      - items:
> +          - enum:
> +              - sinowealth,sh1106-i2c
> +              - solomon,ssd1305fb-i2c
> +              - solomon,ssd1306fb-i2c
> +              - solomon,ssd1307fb-i2c
> +              - solomon,ssd1309fb-i2c
> +        deprecated: true
> +      - items:
> +          - enum:
> +              - sinowealth,sh1106
> +              - solomon,ssd1305
> +              - solomon,ssd1306
> +              - solomon,ssd1307
> +              - solomon,ssd1309
>  
>    reg:
>      maxItems: 1
> @@ -136,7 +147,7 @@ allOf:
>        properties:
>          compatible:
>            contains:
> -            const: sinowealth,sh1106-i2c
> +            const: sinowealth,sh1106

I think we should have both in the condition here, but it looks good
otherwise.

Maxime

Re: [PATCH v2 1/5] dt-bindings: display: ssd1307fb: Deprecate "-i2c" compatible strings

[Maxime Ripard]

On Mon, Apr 11, 2022 at 11:12:39PM +0200, Javier Martinez Canillas wrote:
> The current compatible strings for SSD130x I2C controllers contain both an
> "fb" and "-i2c" suffixes. It seems to indicate that are for a fbdev driver
> and also that are for devices that can be accessed over an I2C bus.
> 
> But a DT is supposed to describe the hardware and not Linux implementation
> details. So let's deprecate those compatible strings and add new ones that
> only contain the vendor and device name, without any of these suffixes.
> 
> These will just describe the device and can be matched by both I2C and SPI
> DRM drivers.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Mark Brown <broonie@kernel.org>
> ---
> 
> Changes in v2:
> - Drop the -i2c suffixes from the compatible strings too (Geert Uytterhoeven).
> 
>  .../bindings/display/solomon,ssd1307fb.yaml   | 37 ++++++++++++-------
>  1 file changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> index ade61d502edd..6b9d0c72739a 100644
> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> @@ -12,12 +12,23 @@ maintainers:
>  
>  properties:
>    compatible:
> -    enum:
> -      - sinowealth,sh1106-i2c
> -      - solomon,ssd1305fb-i2c
> -      - solomon,ssd1306fb-i2c
> -      - solomon,ssd1307fb-i2c
> -      - solomon,ssd1309fb-i2c
> +    oneOf:
> +      # Deprecated compatible strings
> +      - items:
> +          - enum:
> +              - sinowealth,sh1106-i2c
> +              - solomon,ssd1305fb-i2c
> +              - solomon,ssd1306fb-i2c
> +              - solomon,ssd1307fb-i2c
> +              - solomon,ssd1309fb-i2c
> +        deprecated: true
> +      - items:
> +          - enum:
> +              - sinowealth,sh1106
> +              - solomon,ssd1305
> +              - solomon,ssd1306
> +              - solomon,ssd1307
> +              - solomon,ssd1309
>  
>    reg:
>      maxItems: 1
> @@ -136,7 +147,7 @@ allOf:
>        properties:
>          compatible:
>            contains:
> -            const: sinowealth,sh1106-i2c
> +            const: sinowealth,sh1106

I think we should have both in the condition here, but it looks good
otherwise.

Maxime

Re: [PATCH v2 1/5] dt-bindings: display: ssd1307fb: Deprecate "-i2c" compatible strings

[Chen-Yu Tsai]

On Tue, Apr 12, 2022 at 5:12 AM Javier Martinez Canillas
<javierm@redhat.com> wrote:
>
> The current compatible strings for SSD130x I2C controllers contain both an
> "fb" and "-i2c" suffixes. It seems to indicate that are for a fbdev driver
> and also that are for devices that can be accessed over an I2C bus.
>
> But a DT is supposed to describe the hardware and not Linux implementation
> details. So let's deprecate those compatible strings and add new ones that
> only contain the vendor and device name, without any of these suffixes.
>
> These will just describe the device and can be matched by both I2C and SPI
> DRM drivers.
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Mark Brown <broonie@kernel.org>
> ---
>
> Changes in v2:
> - Drop the -i2c suffixes from the compatible strings too (Geert Uytterhoeven).
>
>  .../bindings/display/solomon,ssd1307fb.yaml   | 37 ++++++++++++-------
>  1 file changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> index ade61d502edd..6b9d0c72739a 100644
> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> @@ -12,12 +12,23 @@ maintainers:
>
>  properties:
>    compatible:
> -    enum:
> -      - sinowealth,sh1106-i2c
> -      - solomon,ssd1305fb-i2c
> -      - solomon,ssd1306fb-i2c
> -      - solomon,ssd1307fb-i2c
> -      - solomon,ssd1309fb-i2c
> +    oneOf:
> +      # Deprecated compatible strings
> +      - items:
> +          - enum:
> +              - sinowealth,sh1106-i2c

I think you can just drop this one, since it was just merged and isn't
part of any release yet. It's not even in -rc.

ChenYu

Re: [PATCH v2 1/5] dt-bindings: display: ssd1307fb: Deprecate "-i2c" compatible strings

[Chen-Yu Tsai]

On Tue, Apr 12, 2022 at 5:12 AM Javier Martinez Canillas
<javierm@redhat.com> wrote:
>
> The current compatible strings for SSD130x I2C controllers contain both an
> "fb" and "-i2c" suffixes. It seems to indicate that are for a fbdev driver
> and also that are for devices that can be accessed over an I2C bus.
>
> But a DT is supposed to describe the hardware and not Linux implementation
> details. So let's deprecate those compatible strings and add new ones that
> only contain the vendor and device name, without any of these suffixes.
>
> These will just describe the device and can be matched by both I2C and SPI
> DRM drivers.
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Mark Brown <broonie@kernel.org>
> ---
>
> Changes in v2:
> - Drop the -i2c suffixes from the compatible strings too (Geert Uytterhoeven).
>
>  .../bindings/display/solomon,ssd1307fb.yaml   | 37 ++++++++++++-------
>  1 file changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> index ade61d502edd..6b9d0c72739a 100644
> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> @@ -12,12 +12,23 @@ maintainers:
>
>  properties:
>    compatible:
> -    enum:
> -      - sinowealth,sh1106-i2c
> -      - solomon,ssd1305fb-i2c
> -      - solomon,ssd1306fb-i2c
> -      - solomon,ssd1307fb-i2c
> -      - solomon,ssd1309fb-i2c
> +    oneOf:
> +      # Deprecated compatible strings
> +      - items:
> +          - enum:
> +              - sinowealth,sh1106-i2c

I think you can just drop this one, since it was just merged and isn't
part of any release yet. It's not even in -rc.

ChenYu

Re: [PATCH v2 4/5] drm/solomon: Move device info from ssd130x-i2c to the core driver

[Javier Martinez Canillas]

Hello Andy,

Thanks for your feedback.

On 4/12/22 13:21, Andy Shevchenko wrote:
> On Tue, Apr 12, 2022 at 10:07:02AM +0200, Javier Martinez Canillas wrote:
>> On 4/12/22 09:23, Geert Uytterhoeven wrote:
>>> On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
>>> <javierm@redhat.com> wrote:
> 
> ...
> 
>>>> -       ssd130x->device_info = device_get_match_data(dev);
>>>> +
>>>> +       variant = (enum ssd130x_variants)device_get_match_data(dev);
>>>
>>> (uintptr_t), to avoid a cast from pointer to integer of different
>>> size warning.
>>>
>>
>> Indeed. The kernel test robot reported the same.
> 
> Not only because of this, but also with the non-NULL pointers I prefer the old
> style without ugly castings.
> 
> Instead, you may export the array (in the driver's namespace) and use
> &info[ID] pointer for the specific device info.
> 

That's a great idea! I'll do that in v3.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 4/5] drm/solomon: Move device info from ssd130x-i2c to the core driver

[Javier Martinez Canillas]

Hello Andy,

Thanks for your feedback.

On 4/12/22 13:21, Andy Shevchenko wrote:
> On Tue, Apr 12, 2022 at 10:07:02AM +0200, Javier Martinez Canillas wrote:
>> On 4/12/22 09:23, Geert Uytterhoeven wrote:
>>> On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
>>> <javierm@redhat.com> wrote:
> 
> ...
> 
>>>> -       ssd130x->device_info = device_get_match_data(dev);
>>>> +
>>>> +       variant = (enum ssd130x_variants)device_get_match_data(dev);
>>>
>>> (uintptr_t), to avoid a cast from pointer to integer of different
>>> size warning.
>>>
>>
>> Indeed. The kernel test robot reported the same.
> 
> Not only because of this, but also with the non-NULL pointers I prefer the old
> style without ugly castings.
> 
> Instead, you may export the array (in the driver's namespace) and use
> &info[ID] pointer for the specific device info.
> 

That's a great idea! I'll do that in v3.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 4/5] drm/solomon: Move device info from ssd130x-i2c to the core driver

[Javier Martinez Canillas]

On 4/12/22 13:22, Andy Shevchenko wrote:
> On Tue, Apr 12, 2022 at 02:21:08PM +0300, Andy Shevchenko wrote:
>> On Tue, Apr 12, 2022 at 10:07:02AM +0200, Javier Martinez Canillas wrote:
>>> On 4/12/22 09:23, Geert Uytterhoeven wrote:
>>>> On Mon, Apr 11, 2022 at 11:12 PM Javier Martinez Canillas
>>>> <javierm@redhat.com> wrote:
> 
> ...
> 
>>>>> -       ssd130x->device_info = device_get_match_data(dev);
>>>>> +
>>>>> +       variant = (enum ssd130x_variants)device_get_match_data(dev);
>>>>
>>>> (uintptr_t), to avoid a cast from pointer to integer of different
>>>> size warning.
>>>>
>>>
>>> Indeed. The kernel test robot reported the same.
>>
>> Not only because of this, but also with the non-NULL pointers I prefer the old
>> style without ugly castings.
>>
>> Instead, you may export the array (in the driver's namespace) and use
>> &info[ID] pointer for the specific device info.
> 
> Note that device_get_match_data() has no clue if the data is absent or
> data == (void *)0.
>

Yep, we could make the enum start at 1 and check for !variant but that's
something that will also be prevented by your suggestion to just use the
&info[ID] instead.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 1/5] dt-bindings: display: ssd1307fb: Deprecate "-i2c" compatible strings

[Javier Martinez Canillas]

Hello Maxime,

On 4/12/22 13:28, Maxime Ripard wrote:
> On Mon, Apr 11, 2022 at 11:12:39PM +0200, Javier Martinez Canillas wrote:

[snip]

>>  
>>    reg:
>>      maxItems: 1
>> @@ -136,7 +147,7 @@ allOf:
>>        properties:
>>          compatible:
>>            contains:
>> -            const: sinowealth,sh1106-i2c
>> +            const: sinowealth,sh1106
> 
> I think we should have both in the condition here, but it looks good
> otherwise.
>

Right, we want to keep enforcing for the deprecated compatible strings.
 
> Maxime
> 

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 1/5] dt-bindings: display: ssd1307fb: Deprecate "-i2c" compatible strings

[Javier Martinez Canillas]

Hello Maxime,

On 4/12/22 13:28, Maxime Ripard wrote:
> On Mon, Apr 11, 2022 at 11:12:39PM +0200, Javier Martinez Canillas wrote:

[snip]

>>  
>>    reg:
>>      maxItems: 1
>> @@ -136,7 +147,7 @@ allOf:
>>        properties:
>>          compatible:
>>            contains:
>> -            const: sinowealth,sh1106-i2c
>> +            const: sinowealth,sh1106
> 
> I think we should have both in the condition here, but it looks good
> otherwise.
>

Right, we want to keep enforcing for the deprecated compatible strings.
 
> Maxime
> 

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 1/5] dt-bindings: display: ssd1307fb: Deprecate "-i2c" compatible strings

[Javier Martinez Canillas]

Hello Chen-Yu,

On 4/12/22 14:07, Chen-Yu Tsai wrote:
> On Tue, Apr 12, 2022 at 5:12 AM Javier Martinez Canillas
> <javierm@redhat.com> wrote:

[snip]

> 
> I think you can just drop this one, since it was just merged and isn't
> part of any release yet. It's not even in -rc.
>

I believe you are correct and we could just drop that one.
 
> ChenYu
> 

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 1/5] dt-bindings: display: ssd1307fb: Deprecate "-i2c" compatible strings

[Javier Martinez Canillas]

Hello Chen-Yu,

On 4/12/22 14:07, Chen-Yu Tsai wrote:
> On Tue, Apr 12, 2022 at 5:12 AM Javier Martinez Canillas
> <javierm@redhat.com> wrote:

[snip]

> 
> I think you can just drop this one, since it was just merged and isn't
> part of any release yet. It's not even in -rc.
>

I believe you are correct and we could just drop that one.
 
> ChenYu
> 

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat

Re: [PATCH v2 2/5] dt-bindings: display: ssd1307fb: Extend schema for SPI controllers

[Rob Herring]

On Tue, Apr 12, 2022 at 10:12:32AM +0200, Javier Martinez Canillas wrote:
> On 4/12/22 10:07, Geert Uytterhoeven wrote:
> > Hi Javier,
> 
> [snip]
> 
> >>
> >> Do you have any hints here on how I should enforce this in the schema ?
> >>
> >> Or if you think that a comment is enough, then I will add it in v3.
> > 
> > I don't know how to make it required for SPI, if possible at all.
> >
> 
> I see. Let's go with a comment then and we can later enforce it, if someone
> knows if is possible / how to do it.

It's not possible unless we annotate child nodes with something from the 
parent. '$nodename' is the one and only example of that. 

Really, this falls below the bar of what I'm concerned about checking. 
Ask me again when the conversions are all done (only ~2K to go :( ).

Rob

Re: [PATCH v2 2/5] dt-bindings: display: ssd1307fb: Extend schema for SPI controllers

[Rob Herring]

On Tue, Apr 12, 2022 at 10:12:32AM +0200, Javier Martinez Canillas wrote:
> On 4/12/22 10:07, Geert Uytterhoeven wrote:
> > Hi Javier,
> 
> [snip]
> 
> >>
> >> Do you have any hints here on how I should enforce this in the schema ?
> >>
> >> Or if you think that a comment is enough, then I will add it in v3.
> > 
> > I don't know how to make it required for SPI, if possible at all.
> >
> 
> I see. Let's go with a comment then and we can later enforce it, if someone
> knows if is possible / how to do it.

It's not possible unless we annotate child nodes with something from the 
parent. '$nodename' is the one and only example of that. 

Really, this falls below the bar of what I'm concerned about checking. 
Ask me again when the conversions are all done (only ~2K to go :( ).

Rob