[PATCH 0/2] fbdev: ssd1307fb: add SH1107 support

[PATCH 0/2] fbdev: ssd1307fb: add SH1107 support

[Jakub Turek]

This series adds support for the Sino Wealth SH1107 OLED controller
to the ssd1307fb driver.

Patch 1 implements SH1107 support in the ssd1307 driver.
Patch 2 adds the device tree compatible string.

Tested on:
- imx7d-colibri

Signed-off-by: Jakub Turek <jakub.turek@elsta.tech>
---
Jakub Turek (2):
      fbdev: ssd1307fb: add support for Sino Wealth SH1107 controller
      dt-bindings: display: solomon,ssd1307fb: add SH1107 compatible

 .../bindings/display/solomon,ssd1307fb.yaml        |  17 ++
 drivers/video/fbdev/ssd1307fb.c                    | 234 ++++++++++++++-------
 2 files changed, 172 insertions(+), 79 deletions(-)
---
base-commit: 37e2f878a7a660a216cc7a60459995fefd150f25
change-id: 20260731-feat-sh1107-c76f7d8d93d5

Best regards,
--  
Jakub Turek <jakub.turek@elsta.tech>

[PATCH 1/2] fbdev: ssd1307fb: add support for Sino Wealth SH1107 controller

[Jakub Turek]

The SH1107 display controller is very similar to SSD1307 but the main
difference is that SH1107 has different memory layout and uses page mode
adressation only.

The SH1107 relies primarily on vertical page addressing mode where the
internal 128x128 GDDRAM is split into 16 vertical pages, rather than
the horizontal/tile addressing modes natively utilized by the
SSD1306/SSD1307.

Signed-off-by: Jakub Turek  <jakub.turek@elsta.tech>
---
 drivers/video/fbdev/ssd1307fb.c | 234 ++++++++++++++++++++++++++--------------
 1 file changed, 155 insertions(+), 79 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 644b8d97b381..4930d5c999ff 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -17,29 +17,31 @@
 #include <linux/uaccess.h>
 #include <linux/regulator/consumer.h>
 
-#define SSD1307FB_DATA			0x40
-#define SSD1307FB_COMMAND		0x80
-
-#define SSD1307FB_SET_ADDRESS_MODE	0x20
-#define SSD1307FB_SET_ADDRESS_MODE_HORIZONTAL	(0x00)
-#define SSD1307FB_SET_ADDRESS_MODE_VERTICAL	(0x01)
-#define SSD1307FB_SET_ADDRESS_MODE_PAGE		(0x02)
-#define SSD1307FB_SET_COL_RANGE		0x21
-#define SSD1307FB_SET_PAGE_RANGE	0x22
-#define SSD1307FB_CONTRAST		0x81
-#define SSD1307FB_SET_LOOKUP_TABLE	0x91
-#define	SSD1307FB_CHARGE_PUMP		0x8d
-#define SSD1307FB_SEG_REMAP_ON		0xa1
-#define SSD1307FB_DISPLAY_OFF		0xae
-#define SSD1307FB_SET_MULTIPLEX_RATIO	0xa8
-#define SSD1307FB_DISPLAY_ON		0xaf
-#define SSD1307FB_START_PAGE_ADDRESS	0xb0
-#define SSD1307FB_SET_DISPLAY_OFFSET	0xd3
-#define	SSD1307FB_SET_CLOCK_FREQ	0xd5
-#define	SSD1307FB_SET_AREA_COLOR_MODE	0xd8
-#define	SSD1307FB_SET_PRECHARGE_PERIOD	0xd9
-#define	SSD1307FB_SET_COM_PINS_CONFIG	0xda
-#define	SSD1307FB_SET_VCOMH		0xdb
+#define SSD1307FB_DATA 0x40
+#define SSD1307FB_COMMAND 0x80
+
+#define SSD1307FB_PAGE_COL_START_LOW 0x00
+#define SSD1307FB_PAGE_COL_START_HIGH 0x10
+#define SSD1307FB_SET_ADDRESS_MODE 0x20
+#define SSD1307FB_SET_ADDRESS_MODE_HORIZONTAL (0x00)
+#define SSD1307FB_SET_ADDRESS_MODE_VERTICAL (0x01)
+#define SSD1307FB_SET_ADDRESS_MODE_PAGE (0x02)
+#define SSD1307FB_SET_COL_RANGE 0x21
+#define SSD1307FB_SET_PAGE_RANGE 0x22
+#define SSD1307FB_CONTRAST 0x81
+#define SSD1307FB_SET_LOOKUP_TABLE 0x91
+#define SSD1307FB_CHARGE_PUMP 0x8d
+#define SSD1307FB_SEG_REMAP_ON 0xa1
+#define SSD1307FB_DISPLAY_OFF 0xae
+#define SSD1307FB_SET_MULTIPLEX_RATIO 0xa8
+#define SSD1307FB_DISPLAY_ON 0xaf
+#define SSD1307FB_START_PAGE_ADDRESS 0xb0
+#define SSD1307FB_SET_DISPLAY_OFFSET 0xd3
+#define SSD1307FB_SET_CLOCK_FREQ 0xd5
+#define SSD1307FB_SET_AREA_COLOR_MODE 0xd8
+#define SSD1307FB_SET_PRECHARGE_PERIOD 0xd9
+#define SSD1307FB_SET_COM_PINS_CONFIG 0xda
+#define SSD1307FB_SET_VCOMH 0xdb
 
 #define MAX_CONTRAST 255
 
@@ -54,9 +56,11 @@ struct ssd1307fb_deviceinfo {
 	u32 default_dclk_frq;
 	bool need_pwm;
 	bool need_chargepump;
+	bool page_mode_only;
 };
 
 struct ssd1307fb_par {
+	unsigned page_address_mode : 1;
 	unsigned area_color_enable : 1;
 	unsigned com_invdir : 1;
 	unsigned com_lrremap : 1;
@@ -90,22 +94,22 @@ struct ssd1307fb_par {
 };
 
 struct ssd1307fb_array {
-	u8	type;
-	u8	data[];
+	u8 type;
+	u8 data[];
 };
 
 static const struct fb_fix_screeninfo ssd1307fb_fix = {
-	.id		= "Solomon SSD1307",
-	.type		= FB_TYPE_PACKED_PIXELS,
-	.visual		= FB_VISUAL_MONO10,
-	.xpanstep	= 0,
-	.ypanstep	= 0,
-	.ywrapstep	= 0,
-	.accel		= FB_ACCEL_NONE,
+	.id = "Solomon SSD1307",
+	.type = FB_TYPE_PACKED_PIXELS,
+	.visual = FB_VISUAL_MONO10,
+	.xpanstep = 0,
+	.ypanstep = 0,
+	.ywrapstep = 0,
+	.accel = FB_ACCEL_NONE,
 };
 
 static const struct fb_var_screeninfo ssd1307fb_var = {
-	.bits_per_pixel	= 1,
+	.bits_per_pixel = 1,
 	.red = { .length = 1 },
 	.green = { .length = 1 },
 	.blue = { .length = 1 },
@@ -115,7 +119,7 @@ static struct ssd1307fb_array *ssd1307fb_alloc_array(u32 len, u8 type)
 {
 	struct ssd1307fb_array *array;
 
-	array = kzalloc(sizeof(struct ssd1307fb_array) + len, GFP_KERNEL);
+	array = kzalloc(sizeof(*array) + len, GFP_KERNEL);
 	if (!array)
 		return NULL;
 
@@ -209,6 +213,33 @@ static int ssd1307fb_set_page_range(struct ssd1307fb_par *par, u8 page_start,
 	return 0;
 }
 
+static int ssd1307fb_set_page_pos(struct ssd1307fb_par *par, u8 page_start,
+				  u8 col_start)
+{
+	int ret;
+	u8 page;
+	u8 col_low;
+	u8 col_high;
+
+	page = SSD1307FB_START_PAGE_ADDRESS | (page_start & 0x0f);
+	col_low = SSD1307FB_PAGE_COL_START_LOW | (col_start & 0x0f);
+	col_high = SSD1307FB_PAGE_COL_START_HIGH | ((col_start >> 4) & 0x0f);
+
+	ret = ssd1307fb_write_cmd(par->client, page);
+	if (ret < 0)
+		return ret;
+
+	ret = ssd1307fb_write_cmd(par->client, col_low);
+	if (ret < 0)
+		return ret;
+
+	ret = ssd1307fb_write_cmd(par->client, col_high);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 static int ssd1307fb_update_rect(struct ssd1307fb_par *par, unsigned int x,
 				 unsigned int y, unsigned int width,
 				 unsigned int height)
@@ -253,13 +284,16 @@ static int ssd1307fb_update_rect(struct ssd1307fb_par *par, unsigned int x,
 	 *  (5) A4 B4 C4 D4 E4 F4 G4 H4
 	 */
 
-	ret = ssd1307fb_set_col_range(par, par->col_offset + x, width);
-	if (ret < 0)
-		goto out_free;
+	if (!par->page_address_mode) {
+		ret = ssd1307fb_set_col_range(par, par->col_offset + x, width);
+		if (ret < 0)
+			goto out_free;
 
-	ret = ssd1307fb_set_page_range(par, par->page_offset + y / 8, pages);
-	if (ret < 0)
-		goto out_free;
+		ret = ssd1307fb_set_page_range(par, par->page_offset + y / 8,
+					       pages);
+		if (ret < 0)
+			goto out_free;
+	}
 
 	for (i = y / 8; i < y / 8 + pages; i++) {
 		int m = 8;
@@ -271,16 +305,31 @@ static int ssd1307fb_update_rect(struct ssd1307fb_par *par, unsigned int x,
 			u8 data = 0;
 
 			for (k = 0; k < m; k++) {
-				u8 byte = vmem[(8 * i + k) * line_length +
-					       j / 8];
+				u8 byte =
+					vmem[(8 * i + k) * line_length + j / 8];
 				u8 bit = (byte >> (j % 8)) & 1;
+
 				data |= bit << k;
 			}
 			array->data[array_idx++] = data;
 		}
+
+		if (par->page_address_mode) {
+			ret = ssd1307fb_set_page_pos(par, par->page_offset + i,
+						     par->col_offset + x);
+			if (ret < 0)
+				goto out_free;
+
+			ret = ssd1307fb_write_array(par->client, array, width);
+			if (ret < 0)
+				goto out_free;
+
+			array_idx = 0;
+		}
 	}
 
-	ret = ssd1307fb_write_array(par->client, array, width * pages);
+	if (!par->page_address_mode)
+		ret = ssd1307fb_write_array(par->client, array, width * pages);
 
 out_free:
 	kfree(array);
@@ -302,7 +351,8 @@ static int ssd1307fb_blank(int blank_mode, struct fb_info *info)
 		return ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_ON);
 }
 
-static void ssd1307fb_defio_damage_range(struct fb_info *info, off_t off, size_t len)
+static void ssd1307fb_defio_damage_range(struct fb_info *info, off_t off,
+					 size_t len)
 {
 	struct ssd1307fb_par *par = info->par;
 
@@ -317,17 +367,17 @@ static void ssd1307fb_defio_damage_area(struct fb_info *info, u32 x, u32 y,
 	ssd1307fb_update_rect(par, x, y, width, height);
 }
 
-FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(ssd1307fb,
-				   ssd1307fb_defio_damage_range,
+FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS(ssd1307fb, ssd1307fb_defio_damage_range,
 				   ssd1307fb_defio_damage_area)
 
 static const struct fb_ops ssd1307fb_ops = {
-	.owner		= THIS_MODULE,
+	.owner = THIS_MODULE,
 	FB_DEFAULT_DEFERRED_OPS(ssd1307fb),
-	.fb_blank	= ssd1307fb_blank,
+	.fb_blank = ssd1307fb_blank,
 };
 
-static void ssd1307fb_deferred_io(struct fb_info *info, struct list_head *pagereflist)
+static void ssd1307fb_deferred_io(struct fb_info *info,
+				  struct list_head *pagereflist)
 {
 	ssd1307fb_update_display(info->par);
 }
@@ -341,7 +391,8 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 	if (par->device_info->need_pwm) {
 		par->pwm = pwm_get(&par->client->dev, NULL);
 		if (IS_ERR(par->pwm)) {
-			dev_err(&par->client->dev, "Could not get PWM from device tree!\n");
+			dev_err(&par->client->dev,
+				"Could not get PWM from device tree!\n");
 			return PTR_ERR(par->pwm);
 		}
 
@@ -352,8 +403,9 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 		/* Enable the PWM */
 		pwm_enable(par->pwm);
 
-		dev_dbg(&par->client->dev, "Using PWM %s with a %lluns period.\n",
-			par->pwm->label, pwm_get_period(par->pwm));
+		dev_dbg(&par->client->dev,
+			"Using PWM %s with a %lluns period.\n", par->pwm->label,
+			pwm_get_period(par->pwm));
 	}
 
 	/* Set initial contrast */
@@ -374,7 +426,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 
 	/* Set COM direction */
 	com_invdir = 0xc0 | par->com_invdir << 3;
-	ret = ssd1307fb_write_cmd(par->client,  com_invdir);
+	ret = ssd1307fb_write_cmd(par->client, com_invdir);
 	if (ret < 0)
 		return ret;
 
@@ -416,7 +468,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 			return ret;
 
 		mode = (par->area_color_enable ? 0x30 : 0) |
-			(par->low_power ? 5 : 0);
+		       (par->low_power ? 5 : 0);
 		ret = ssd1307fb_write_cmd(par->client, mode);
 		if (ret < 0)
 			return ret;
@@ -457,7 +509,7 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 		return ret;
 
 	ret = ssd1307fb_write_cmd(par->client,
-		BIT(4) | (par->device_info->need_chargepump ? BIT(2) : 0));
+				  BIT(4) | (par->device_info->need_chargepump ? BIT(2) : 0));
 	if (ret < 0)
 		return ret;
 
@@ -483,13 +535,16 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 		}
 	}
 
-	/* Switch to horizontal addressing mode */
 	ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_ADDRESS_MODE);
 	if (ret < 0)
 		return ret;
 
-	ret = ssd1307fb_write_cmd(par->client,
-				  SSD1307FB_SET_ADDRESS_MODE_HORIZONTAL);
+	if (par->page_address_mode)
+		ret = ssd1307fb_write_cmd(par->client,
+					  SSD1307FB_SET_ADDRESS_MODE_PAGE);
+	else
+		ret = ssd1307fb_write_cmd(par->client,
+					  SSD1307FB_SET_ADDRESS_MODE_HORIZONTAL);
 	if (ret < 0)
 		return ret;
 
@@ -531,9 +586,9 @@ static int ssd1307fb_get_brightness(struct backlight_device *bdev)
 }
 
 static const struct backlight_ops ssd1307fb_bl_ops = {
-	.options	= BL_CORE_SUSPENDRESUME,
-	.update_status	= ssd1307fb_update_bl,
-	.get_brightness	= ssd1307fb_get_brightness,
+	.options = BL_CORE_SUSPENDRESUME,
+	.update_status = ssd1307fb_update_bl,
+	.get_brightness = ssd1307fb_get_brightness,
 };
 
 static struct ssd1307fb_deviceinfo ssd1307fb_ssd1305_deviceinfo = {
@@ -562,6 +617,13 @@ static struct ssd1307fb_deviceinfo ssd1307fb_ssd1309_deviceinfo = {
 	.default_dclk_frq = 10,
 };
 
+static struct ssd1307fb_deviceinfo ssd1307fb_sh1107_deviceinfo = {
+	.default_vcomh = 0x35,
+	.default_dclk_div = 1,
+	.default_dclk_frq = 8,
+	.page_mode_only = 1,
+};
+
 static const struct of_device_id ssd1307fb_of_match[] = {
 	{
 		.compatible = "solomon,ssd1305fb-i2c",
@@ -579,6 +641,10 @@ static const struct of_device_id ssd1307fb_of_match[] = {
 		.compatible = "solomon,ssd1309fb-i2c",
 		.data = (void *)&ssd1307fb_ssd1309_deviceinfo,
 	},
+	{
+		.compatible = "sinowealth,sh1107",
+		.data = (void *)&ssd1307fb_sh1107_deviceinfo,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, ssd1307fb_of_match);
@@ -617,7 +683,8 @@ static int ssd1307fb_probe(struct i2c_client *client)
 		if (ret == -ENODEV) {
 			par->vbat_reg = NULL;
 		} else {
-			dev_err_probe(dev, ret, "failed to get VBAT regulator\n");
+			dev_err_probe(dev, ret,
+				      "failed to get VBAT regulator\n");
 			goto fb_alloc_error;
 		}
 	}
@@ -628,19 +695,24 @@ static int ssd1307fb_probe(struct i2c_client *client)
 	if (device_property_read_u32(dev, "solomon,height", &par->height))
 		par->height = 16;
 
-	if (device_property_read_u32(dev, "solomon,page-offset", &par->page_offset))
+	if (device_property_read_u32(dev, "solomon,page-offset",
+				     &par->page_offset))
 		par->page_offset = 1;
 
-	if (device_property_read_u32(dev, "solomon,col-offset", &par->col_offset))
+	if (device_property_read_u32(dev, "solomon,col-offset",
+				     &par->col_offset))
 		par->col_offset = 0;
 
-	if (device_property_read_u32(dev, "solomon,com-offset", &par->com_offset))
+	if (device_property_read_u32(dev, "solomon,com-offset",
+				     &par->com_offset))
 		par->com_offset = 0;
 
-	if (device_property_read_u32(dev, "solomon,prechargep1", &par->prechargep1))
+	if (device_property_read_u32(dev, "solomon,prechargep1",
+				     &par->prechargep1))
 		par->prechargep1 = 2;
 
-	if (device_property_read_u32(dev, "solomon,prechargep2", &par->prechargep2))
+	if (device_property_read_u32(dev, "solomon,prechargep2",
+				     &par->prechargep2))
 		par->prechargep2 = 2;
 
 	if (!device_property_read_u8_array(dev, "solomon,lookup-table",
@@ -648,9 +720,11 @@ static int ssd1307fb_probe(struct i2c_client *client)
 					   ARRAY_SIZE(par->lookup_table)))
 		par->lookup_table_set = 1;
 
-	par->seg_remap = !device_property_read_bool(dev, "solomon,segment-no-remap");
+	par->seg_remap =
+		!device_property_read_bool(dev, "solomon,segment-no-remap");
 	par->com_seq = device_property_read_bool(dev, "solomon,com-seq");
-	par->com_lrremap = device_property_read_bool(dev, "solomon,com-lrremap");
+	par->com_lrremap =
+		device_property_read_bool(dev, "solomon,com-lrremap");
 	par->com_invdir = device_property_read_bool(dev, "solomon,com-invdir");
 	par->area_color_enable =
 		device_property_read_bool(dev, "solomon,area-color-enable");
@@ -658,6 +732,7 @@ static int ssd1307fb_probe(struct i2c_client *client)
 
 	par->contrast = 127;
 	par->vcomh = par->device_info->default_vcomh;
+	par->page_address_mode = par->device_info->page_mode_only;
 
 	/* Setup display timing */
 	if (device_property_read_u32(dev, "solomon,dclk-div", &par->dclk_div))
@@ -675,8 +750,8 @@ static int ssd1307fb_probe(struct i2c_client *client)
 		goto fb_alloc_error;
 	}
 
-	ssd1307fb_defio = devm_kzalloc(dev, sizeof(*ssd1307fb_defio),
-				       GFP_KERNEL);
+	ssd1307fb_defio =
+		devm_kzalloc(dev, sizeof(*ssd1307fb_defio), GFP_KERNEL);
 	if (!ssd1307fb_defio) {
 		dev_err(dev, "Couldn't allocate deferred io.\n");
 		ret = -ENOMEM;
@@ -725,8 +800,8 @@ static int ssd1307fb_probe(struct i2c_client *client)
 	if (ret)
 		goto regulator_enable_error;
 
-	bl = backlight_device_register("ssd1307fb-bl", dev, par, &ssd1307fb_bl_ops,
-				       NULL);
+	bl = backlight_device_register("ssd1307fb-bl", dev, par,
+				       &ssd1307fb_bl_ops, NULL);
 	if (IS_ERR(bl)) {
 		ret = PTR_ERR(bl);
 		dev_err(dev, "unable to register backlight device: %d\n", ret);
@@ -742,8 +817,11 @@ static int ssd1307fb_probe(struct i2c_client *client)
 
 	bl->props.brightness = par->contrast;
 	bl->props.max_brightness = MAX_CONTRAST;
+	info->bl_dev = bl;
 
-	dev_info(dev, "fb%d: %s framebuffer device registered, using %d bytes of video memory\n", info->node, info->fix.id, vmem_size);
+	dev_info(dev,
+		 "fb%d: %s framebuffer device registered, using %d bytes of video memory\n",
+		 info->node, info->fix.id, vmem_size);
 
 	return 0;
 
@@ -784,11 +862,9 @@ static void ssd1307fb_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id ssd1307fb_i2c_id[] = {
-	{ .name = "ssd1305fb" },
-	{ .name = "ssd1306fb" },
-	{ .name = "ssd1307fb" },
-	{ .name = "ssd1309fb" },
-	{ }
+	{ .name = "ssd1305fb" }, { .name = "ssd1306fb" },
+	{ .name = "ssd1307fb" }, { .name = "ssd1309fb" },
+	{ .name = "sh1107fb" },	 {}
 };
 MODULE_DEVICE_TABLE(i2c, ssd1307fb_i2c_id);
 

-- 
2.34.1

[PATCH 2/2] dt-bindings: display: solomon,ssd1307fb: add SH1107 compatible

[Jakub Turek]

Update the device tree documentation to include "sinowealth,sh1107" as a
recognized compatible string.

Signed-off-by: Jakub Turek  <jakub.turek@elsta.tech>
---
 .../devicetree/bindings/display/solomon,ssd1307fb.yaml  | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
index 153ff86fb405..6b17a701bbaa 100644
--- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
+++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
@@ -22,6 +22,7 @@ properties:
         deprecated: true
       - enum:
           - sinowealth,sh1106
+          - sinowealth,sh1107
           - solomon,ssd1305
           - solomon,ssd1306
           - solomon,ssd1307
@@ -140,6 +141,22 @@ allOf:
         solomon,dclk-frq:
           default: 5
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: sinowealth,sh1107
+    then:
+      properties:
+        solomon,width:
+          default: 128
+        solomon,height:
+          default: 64
+        solomon,dclk-div:
+          default: 1
+        solomon,dclk-frq:
+          default: 8
+
   - if:
       properties:
         compatible:

-- 
2.34.1

Re: [PATCH 1/2] fbdev: ssd1307fb: add support for Sino Wealth SH1107 controller

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider:

New issues:
- [High] Invalid initialization command sequence sent to SH1107 displays.
- [Medium] The local variable `ret` can be returned uninitialized in `ssd1307fb_update_rect` when `par->page_address_mode` is true and `pages` evaluates to 0.
- [Low] Suboptimal and wasteful memory allocation for `array` when `page_address_mode` is enabled.

Pre-existing issues:
- [Critical] The driver frees `fb_info` and graphical memory immediately during device removal, leading to a Use-After-Free if userspace holds open file descriptors.
- [High] Kernel crash during probe if the device is matched via the legacy I2C ID table.
- [High] Short I2C writes are silently treated as successes, potentially corrupting the display.
- [High] Missing lock around display update sequence allows concurrent I2C transactions to interleave and corrupts the hardware state cache.
- [High] The driver may sleep in atomic context when fbcon attempts to update the display during a kernel panic or oops.
- [High] Unvalidated `solomon,width` and `solomon,height` device tree properties can cause integer overflows when allocating memory, leading to heap buffer overflows.
- [High] The `refreshrate` module parameter can be set to 0, which causes a division by zero when configuring the deferred I/O delay.
- [Low] The `i2c_device_id` array terminator does not use named initializers.
--

--- Patch [1]: [PATCH 1/2] fbdev: ssd1307fb: add support for Sino Wealth SH1107 controller ---
Note: The format of this review has been altered to a free-form summary due
to recitation restrictions. Original patch code is not quoted.

Commit: 1ab3e53993e2ff1e1a60665597dfc8bf1a7a4cd1
Author: Jakub Turek <jakub.turek@elsta.tech>
Subject: fbdev: ssd1307fb: add support for Sino Wealth SH1107 controller

This commit adds support for the SH1107 display controller to the ssd1307fb
driver. It introduces a page_address_mode flag to handle the vertical page
addressing mode utilized by the SH1107.

[Severity: Medium]
In ssd1307fb_update_rect(), if par->page_address_mode is true and the
calculated pages evaluates to 0 (for example, if invoked with a damage
rectangle where height is 0), both initialization blocks for ret are
bypassed. Could this result in returning an uninitialized stack variable?

[Severity: Low]
In ssd1307fb_update_rect(), when page_address_mode is enabled, the code
allocates width * pages bytes for the array. However, it only populates
and writes width bytes per loop iteration, resetting the index each time.
Is this allocation larger than necessary?

[Severity: High]
In ssd1307fb_init(), the SSD1307FB_SET_ADDRESS_MODE (0x20) command is
sent unconditionally. On the SH1107 controller, 0x20 is a 1-byte DC-DC
voltage setting command rather than a 2-byte addressing mode command. Will
the hardware incorrectly interpret the subsequent byte 0x02 as a standalone
command instead of an addressing mode parameter?

[Severity: High]
This is a pre-existing issue, but in ssd1307fb_probe(), device_info is
fetched using device_get_match_data(). If the driver is bound via the
legacy I2C sysfs interface, this returns NULL because the entries in the
ssd1307fb_i2c_id array lack a .driver_data field. Could this cause a kernel
panic when par->device_info->default_vcomh is dereferenced?

[Severity: High]
This isn't a bug introduced by this patch, but in ssd1307fb_write_array(),
if i2c_master_send() returns a short write (a positive value less than len),
this positive value is returned instead of a negative error. Since call
sites like ssd1307fb_update_rect() only test for ret < 0, do short writes
get incorrectly treated as successes, potentially misaligning subsequent
hardware commands?

[Severity: High]
This is a pre-existing issue, but ssd1307fb_update_rect() performs multiple
I2C writes and updates caching variables without holding any driver-level
lock. Could concurrent access from fbcon, delayed work queues, and userspace
writes to /dev/fb0 interleave I2C messages and corrupt the hardware state?

[Severity: High]
This isn't a bug introduced by this patch, but during a system crash, fbcon
flushes output in atomic context and invokes .fb_imageblit, which eventually
calls ssd1307fb_update_rect(). This function calls kzalloc() with GFP_KERNEL
and the sleepable i2c_master_send(). Does this cause a deadlock or nested
panic preventing oops messages from printing?

[Severity: Critical]
This is a pre-existing issue, but ssd1307fb_remove() unconditionally calls
__free_pages() and framebuffer_release() directly inline. If a userspace
process holds an open file descriptor to /dev/fb0 during device unbind, will
subsequent operations access freed memory since the driver doesn't wait for
fbdev core reference counting to reach zero?

[Severity: High]
This isn't a bug introduced by this patch, but in ssd1307fb_probe(),
unvalidated solomon,width and solomon,height properties from the device tree
are multiplied to calculate vmem_size. Could maliciously crafted large
values cause an integer overflow leading to a small memory allocation and
subsequent heap buffer overflows in ssd1307fb_update_rect()?

[Severity: High]
This is a pre-existing issue, but the refreshrate module parameter can be
set to 0. When configuring the deferred I/O delay in ssd1307fb_probe(), does
the expression HZ / refreshrate evaluate to a division by zero and crash the
kernel?

[Severity: Low]
This isn't a bug introduced by this patch, but the ssd1307fb_i2c_id array
concludes with an empty terminator instead of using a named initializer.
Does this violate the I2C subsystem API guidelines for struct i2c_device_id
arrays?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-feat-sh1107-v1-0-e10977f76e6b@elsta.tech?part=1

Re: [PATCH 1/2] fbdev: ssd1307fb: add support for Sino Wealth SH1107 controller

[Helge Deller]

On 7/31/26 12:46, Jakub Turek wrote:
> The SH1107 display controller is very similar to SSD1307 but the main
> difference is that SH1107 has different memory layout and uses page mode
> adressation only.
> 
> The SH1107 relies primarily on vertical page addressing mode where the
> internal 128x128 GDDRAM is split into 16 vertical pages, rather than
> the horizontal/tile addressing modes natively utilized by the
> SSD1306/SSD1307.
> 
> Signed-off-by: Jakub Turek  <jakub.turek@elsta.tech>
> ---
>   drivers/video/fbdev/ssd1307fb.c | 234 ++++++++++++++++++++++++++--------------
>   1 file changed, 155 insertions(+), 79 deletions(-)

This patch has many unnecessary white space cleanups.

Please split it up in at least two patches: One with real code additions and fixes,
the other (if you like) a patch to clean up indenting and such.

Helge

Re: [PATCH 1/2] fbdev: ssd1307fb: add support for Sino Wealth SH1107 controller

[Javier Martinez Canillas]

Jakub Turek <jakub.turek@elsta.tech> writes:

Hello Jakub,

> The SH1107 display controller is very similar to SSD1307 but the main
> difference is that SH1107 has different memory layout and uses page mode
> adressation only.
>
> The SH1107 relies primarily on vertical page addressing mode where the
> internal 128x128 GDDRAM is split into 16 vertical pages, rather than
> the horizontal/tile addressing modes natively utilized by the
> SSD1306/SSD1307.
>
> Signed-off-by: Jakub Turek  <jakub.turek@elsta.tech>
> ---


The fbdev driver is obsolete and likely to be removed soon. Instead please
use the KMS driver (drivers/gpu/drm/solomon/ssd130x*). This already has
support for the sh1106, so adding ssh1307 support should be trivial

(basically just adding an entry to the device ID tables I believe).

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat