[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20251024-st7571-split-v1-4-d3092b98130f@gmail.com>
Date: Fri, 24 Oct 2025 12:56:55 +0200
From: Marcus Folkesson <marcus.folkesson@...il.com>
To: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
Marcus Folkesson <marcus.folkesson@...il.com>
Subject: [PATCH 4/6] drm/sitronix/st7571-i2c: make probe independent of hw
interface
Create a interface independent layer for the probe function. This to
make it possible to add support for other interfaces.
Signed-off-by: Marcus Folkesson <marcus.folkesson@...il.com>
---
drivers/gpu/drm/sitronix/st7571-i2c.c | 165 +++++++++++++++++++++++-----------
drivers/gpu/drm/sitronix/st7571.h | 25 +-----
2 files changed, 113 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/sitronix/st7571-i2c.c b/drivers/gpu/drm/sitronix/st7571-i2c.c
index af27658a5e152534d445bc623893eee6b3ca00d5..f994ace407390dba30c0968ca99f437382badfab 100644
--- a/drivers/gpu/drm/sitronix/st7571-i2c.c
+++ b/drivers/gpu/drm/sitronix/st7571-i2c.c
@@ -80,6 +80,33 @@
#define DRIVER_MAJOR 1
#define DRIVER_MINOR 0
+struct st7571_i2c_transport {
+ struct i2c_client *client;
+
+ /*
+ * Depending on the hardware design, the acknowledge signal may be hard to
+ * recognize as a valid logic "0" level.
+ * Therefor, ignore NAK if possible to stay compatible with most hardware designs
+ * and off-the-shelf panels out there.
+ *
+ * From section 6.4 MICROPOCESSOR INTERFACE section in the datasheet:
+ *
+ * "By connecting SDA_OUT to SDA_IN externally, the SDA line becomes fully
+ * I2C interface compatible.
+ * Separating acknowledge-output from serial data
+ * input is advantageous for chip-on-glass (COG) applications. In COG
+ * applications, the ITO resistance and the pull-up resistor will form a
+ * voltage divider, which affects acknowledge-signal level. Larger ITO
+ * resistance will raise the acknowledged-signal level and system cannot
+ * recognize this level as a valid logic “0” level. By separating SDA_IN from
+ * SDA_OUT, the IC can be used in a mode that ignores the acknowledge-bit.
+ * For applications which check acknowledge-bit, it is necessary to minimize
+ * the ITO resistance of the SDA_OUT trace to guarantee a valid low level."
+ *
+ */
+ bool ignore_nak;
+};
+
static inline struct st7571_device *drm_to_st7571(struct drm_device *drm)
{
return container_of(drm, struct st7571_device, drm);
@@ -87,18 +114,17 @@ static inline struct st7571_device *drm_to_st7571(struct drm_device *drm)
static int st7571_regmap_write(void *context, const void *data, size_t count)
{
- struct i2c_client *client = context;
- struct st7571_device *st7571 = i2c_get_clientdata(client);
+ struct st7571_i2c_transport *t = context;
int ret;
struct i2c_msg msg = {
- .addr = st7571->client->addr,
- .flags = st7571->ignore_nak ? I2C_M_IGNORE_NAK : 0,
+ .addr = t->client->addr,
+ .flags = t->ignore_nak ? I2C_M_IGNORE_NAK : 0,
.len = count,
.buf = (u8 *)data
};
- ret = i2c_transfer(st7571->client->adapter, &msg, 1);
+ ret = i2c_transfer(t->client->adapter, &msg, 1);
/*
* Unfortunately, there is no way to check if the transfer failed because of
@@ -106,7 +132,7 @@ static int st7571_regmap_write(void *context, const void *data, size_t count)
*
* However, if the transfer fails and ignore_nak is set, we know it is an error.
*/
- if (ret < 0 && st7571->ignore_nak)
+ if (ret < 0 && t->ignore_nak)
return ret;
return 0;
@@ -845,102 +871,135 @@ static int st7571_lcd_init(struct st7571_device *st7571)
return st7571_send_command_list(st7571, commands, ARRAY_SIZE(commands));
}
-static int st7571_probe(struct i2c_client *client)
+static struct st7571_device *st7571_probe(struct device *dev,
+ struct regmap *regmap)
{
struct st7571_device *st7571;
struct drm_device *drm;
int ret;
- st7571 = devm_drm_dev_alloc(&client->dev, &st7571_driver,
+ st7571 = devm_drm_dev_alloc(dev, &st7571_driver,
struct st7571_device, drm);
if (IS_ERR(st7571))
- return PTR_ERR(st7571);
+ return st7571;
drm = &st7571->drm;
- st7571->dev = &client->dev;
- st7571->client = client;
- i2c_set_clientdata(client, st7571);
+ st7571->dev = dev;
st7571->pdata = device_get_match_data(st7571->dev);
ret = st7571->pdata->parse_dt(st7571);
if (ret)
- return ret;
+ return ERR_PTR(ret);
ret = st7571_validate_parameters(st7571);
if (ret)
- return ret;
+ return ERR_PTR(ret);
st7571->mode = st7571_mode(st7571);
+ st7571->regmap = regmap;
- /*
- * The hardware design could make it hard to detect a NAK on the I2C bus.
- * If the adapter does not support protocol mangling do
- * not set the I2C_M_IGNORE_NAK flag at the expense * of possible
- * cruft in the logs.
- */
- if (i2c_check_functionality(client->adapter, I2C_FUNC_PROTOCOL_MANGLING))
- st7571->ignore_nak = true;
-
- st7571->regmap = devm_regmap_init(st7571->dev, &st7571_regmap_bus,
- client, &st7571_regmap_config);
- if (IS_ERR(st7571->regmap)) {
- return dev_err_probe(st7571->dev, PTR_ERR(st7571->regmap),
- "Failed to initialize regmap\n");
- }
st7571->hwbuf = devm_kzalloc(st7571->dev,
(st7571->nlines * st7571->ncols * st7571->bpp) / 8,
GFP_KERNEL);
if (!st7571->hwbuf)
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
st7571->row = devm_kzalloc(st7571->dev,
(st7571->ncols * st7571->bpp),
GFP_KERNEL);
if (!st7571->row)
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
ret = st7571_mode_config_init(st7571);
- if (ret)
- return dev_err_probe(st7571->dev, ret,
- "Failed to initialize mode config\n");
+ if (ret) {
+ dev_err(st7571->dev, "Failed to initialize mode config\n");
+ return ERR_PTR(ret);
+ }
ret = st7571_plane_init(st7571, st7571->pformat);
- if (ret)
- return dev_err_probe(st7571->dev, ret,
- "Failed to initialize primary plane\n");
+ if (ret) {
+ dev_err(st7571->dev, "Failed to initialize primary plane\n");
+ return ERR_PTR(ret);
+ }
ret = st7571_crtc_init(st7571);
- if (ret < 0)
- return dev_err_probe(st7571->dev, ret,
- "Failed to initialize CRTC\n");
+ if (ret < 0) {
+ dev_err(st7571->dev, "Failed to initialize CRTC\n");
+ return ERR_PTR(ret);
+ }
ret = st7571_encoder_init(st7571);
- if (ret < 0)
- return dev_err_probe(st7571->dev, ret,
- "Failed to initialize encoder\n");
+ if (ret < 0) {
+ dev_err(st7571->dev, "Failed to initialize encoder\n");
+ return ERR_PTR(ret);
+ }
ret = st7571_connector_init(st7571);
- if (ret < 0)
- return dev_err_probe(st7571->dev, ret,
- "Failed to initialize connector\n");
+ if (ret < 0) {
+ dev_err(st7571->dev, "Failed to initialize connector\n");
+ return ERR_PTR(ret);
+ }
drm_mode_config_reset(drm);
ret = drm_dev_register(drm, 0);
- if (ret)
- return dev_err_probe(st7571->dev, ret,
- "Failed to register DRM device\n");
+ if (ret) {
+ dev_err(st7571->dev, "Failed to register DRM device\n");
+ return ERR_PTR(ret);
+ }
drm_client_setup(drm, NULL);
+ return st7571;
+}
+
+static void st7571_remove(struct st7571_device *st7571)
+{
+ drm_dev_unplug(&st7571->drm);
+}
+
+static int st7571_i2c_probe(struct i2c_client *client)
+{
+ struct st7571_device *st7571;
+ struct st7571_i2c_transport *t;
+ struct regmap *regmap;
+
+ t = devm_kzalloc(&client->dev, sizeof(*t), GFP_KERNEL);
+ if (!t)
+ return -ENOMEM;
+
+ t->client = client;
+
+ /*
+ * The hardware design could make it hard to detect a NAK on the I2C bus.
+ * If the adapter does not support protocol mangling do
+ * not set the I2C_M_IGNORE_NAK flag at the expense * of possible
+ * cruft in the logs.
+ */
+ if (i2c_check_functionality(client->adapter, I2C_FUNC_PROTOCOL_MANGLING))
+ t->ignore_nak = true;
+
+ regmap = devm_regmap_init(&client->dev, &st7571_regmap_bus,
+ t, &st7571_regmap_config);
+ if (IS_ERR(regmap)) {
+ return dev_err_probe(&client->dev, PTR_ERR(regmap),
+ "Failed to initialize regmap\n");
+ }
+
+ st7571 = st7571_probe(&client->dev, regmap);
+ if (IS_ERR(st7571))
+ return dev_err_probe(&client->dev, PTR_ERR(st7571),
+ "Failed to initialize regmap\n");
+
+ i2c_set_clientdata(client, st7571);
return 0;
}
-static void st7571_remove(struct i2c_client *client)
+static void st7571_i2c_remove(struct i2c_client *client)
{
struct st7571_device *st7571 = i2c_get_clientdata(client);
- drm_dev_unplug(&st7571->drm);
+ st7571_remove(st7571);
}
static const struct st7571_panel_data st7567_config = {
@@ -986,8 +1045,8 @@ static struct i2c_driver st7571_i2c_driver = {
.name = "st7571",
.of_match_table = st7571_of_match,
},
- .probe = st7571_probe,
- .remove = st7571_remove,
+ .probe = st7571_i2c_probe,
+ .remove = st7571_i2c_remove,
.id_table = st7571_id,
};
diff --git a/drivers/gpu/drm/sitronix/st7571.h b/drivers/gpu/drm/sitronix/st7571.h
index c6fd6f1d3aa33d6b43330ce8f2cb2d3f2321b29b..f62c57ddb99ebe82f63048cc2ccf75a81518d717 100644
--- a/drivers/gpu/drm/sitronix/st7571.h
+++ b/drivers/gpu/drm/sitronix/st7571.h
@@ -13,6 +13,7 @@
#include <drm/drm_crtc.h>
#include <drm/drm_drv.h>
#include <drm/drm_encoder.h>
+#include <drm/drm_format_helper.h>
#include <linux/regmap.h>
@@ -62,33 +63,9 @@ struct st7571_device {
const struct st7571_panel_format *pformat;
const struct st7571_panel_data *pdata;
- struct i2c_client *client;
struct gpio_desc *reset;
struct regmap *regmap;
- /*
- * Depending on the hardware design, the acknowledge signal may be hard to
- * recognize as a valid logic "0" level.
- * Therefor, ignore NAK if possible to stay compatible with most hardware designs
- * and off-the-shelf panels out there.
- *
- * From section 6.4 MICROPOCESSOR INTERFACE section in the datasheet:
- *
- * "By connecting SDA_OUT to SDA_IN externally, the SDA line becomes fully
- * I2C interface compatible.
- * Separating acknowledge-output from serial data
- * input is advantageous for chip-on-glass (COG) applications. In COG
- * applications, the ITO resistance and the pull-up resistor will form a
- * voltage divider, which affects acknowledge-signal level. Larger ITO
- * resistance will raise the acknowledged-signal level and system cannot
- * recognize this level as a valid logic “0” level. By separating SDA_IN from
- * SDA_OUT, the IC can be used in a mode that ignores the acknowledge-bit.
- * For applications which check acknowledge-bit, it is necessary to minimize
- * the ITO resistance of the SDA_OUT trace to guarantee a valid low level."
- *
- */
- bool ignore_nak;
-
bool grayscale;
bool inverted;
u32 height_mm;
--
2.50.1
Powered by blists - more mailing lists