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

From: Neil Armstrong
Date: Fri Apr 08 2022 - 03:49:19 EST


Hi,

On 07/04/2022 22:02, Javier Martinez Canillas 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 into 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@xxxxxxxxxx>
Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
---

drivers/gpu/drm/solomon/ssd130x-i2c.c | 51 ++++-------------------
drivers/gpu/drm/solomon/ssd130x.c | 60 +++++++++++++++++++++++++--
drivers/gpu/drm/solomon/ssd130x.h | 13 ++++++
3 files changed, 78 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/solomon/ssd130x-i2c.c b/drivers/gpu/drm/solomon/ssd130x-i2c.c
index a469679548f8..aa6cc2cb54f9 100644
--- a/drivers/gpu/drm/solomon/ssd130x-i2c.c
+++ b/drivers/gpu/drm/solomon/ssd130x-i2c.c
@@ -53,76 +53,43 @@ 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-i2c",
- .data = &ssd130x_sh1106_deviceinfo,
+ .data = SH1106_ID,
},
{
.compatible = "solomon,ssd1305-i2c",
- .data = &ssd130x_ssd1305_deviceinfo,
+ .data = (void *)SSD1305_ID,
},
{
.compatible = "solomon,ssd1306-i2c",
- .data = &ssd130x_ssd1306_deviceinfo,
+ .data = (void *)SSD1306_ID,
},
{
.compatible = "solomon,ssd1307-i2c",
- .data = &ssd130x_ssd1307_deviceinfo,
+ .data = (void *)SSD1307_ID,
},
{
.compatible = "solomon,ssd1309-i2c",
- .data = &ssd130x_ssd1309_deviceinfo,
+ .data = (void *)SSD1309_ID,
},
/* Deprecated but remain for backward compatibility */
{
.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..1f00fd3c0023 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,55 @@
#define MAX_CONTRAST 255
+static struct ssd130x_deviceinfo ssd130x_variants[] = {
+ {
+ .default_vcomh = 0x40,
+ .default_dclk_div = 1,
+ .default_dclk_frq = 5,
+ .page_mode_only = 1,
+ },

Why not [SH1106_ID] = {

and later:

if (variant < NR_SSD130X_VARIANTS)
ssd130x->device_info = ssd130x_variants[variant];

instead of less efficient ssd13x_variant_to_info ?

+ {
+ .variant = SSD1305_ID,
+ .default_vcomh = 0x34,
+ .default_dclk_div = 1,
+ .default_dclk_frq = 7,
+ },
+ {
+ .variant = SSD1306_ID,
+ .default_vcomh = 0x20,
+ .default_dclk_div = 1,
+ .default_dclk_frq = 8,
+ .need_chargepump = 1,
+ },
+ {
+ .variant = SSD1307_ID,
+ .default_vcomh = 0x20,
+ .default_dclk_div = 2,
+ .default_dclk_frq = 12,
+ .need_pwm = 1,
+ },
+ {
+ .variant = SSD1309_ID,
+ .default_vcomh = 0x34,
+ .default_dclk_div = 1,
+ .default_dclk_frq = 10,
+ }
+};
+
+static const struct ssd130x_deviceinfo *ssd13x_variant_to_info(enum ssd130x_variants variant)
+{
+ int i;
+ const struct ssd130x_deviceinfo *info;
+
+ for (i = 0; i < ARRAY_SIZE(ssd130x_variants); i++) {
+ info = &ssd130x_variants[i];
+ if (info->variant == variant)
+ return info;
+ }
+
+ return NULL;
+}
+
static inline struct ssd130x_device *drm_to_ssd130x(struct drm_device *drm)
{
return container_of(drm, struct ssd130x_device, drm);
@@ -846,6 +893,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 +908,11 @@ 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);
+ ssd130x->device_info = ssd13x_variant_to_info(variant);
+ if (!ssd130x->device_info)
+ return ERR_PTR(-EINVAL);
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..4e0b62a41aa3 100644
--- a/drivers/gpu/drm/solomon/ssd130x.h
+++ b/drivers/gpu/drm/solomon/ssd130x.h
@@ -18,7 +18,20 @@
#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 {
+ enum ssd130x_variants variant;
u32 default_vcomh;
u32 default_dclk_div;
u32 default_dclk_frq;

Neil