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

[PATCH v2 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.

Changes in v2:
- Removed unrelated whitespace cleanups from the driver patch.
- Kept only functional SH1107 changes.

Signed-off-by: Jakub Turek <jakub.turek@elsta.tech>

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                    | 87 +++++++++++++++++++---
 2 files changed, 93 insertions(+), 11 deletions(-)
---
base-commit: 8ba098e6b6ff0db8edf28528d1552be261af30d4
change-id: 20260731-feat-sh1107-859027285291

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

[PATCH v2 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 | 87 +++++++++++++++++++++++++++++++++++------
 1 file changed, 76 insertions(+), 11 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 644b8d97b381..19dc7671fa7a 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -20,6 +20,8 @@
 #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)
@@ -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;
@@ -209,6 +213,34 @@ 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 +285,15 @@ 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;
@@ -274,13 +308,28 @@ static int ssd1307fb_update_rect(struct ssd1307fb_par *par, unsigned int x,
 				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);
@@ -483,13 +532,17 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
 		}
 	}
 
-	/* Switch to horizontal addressing mode */
+	/* Switch to 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;
 
@@ -562,6 +615,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 +639,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);
@@ -658,6 +722,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))
@@ -678,7 +743,6 @@ static int ssd1307fb_probe(struct i2c_client *client)
 	ssd1307fb_defio = devm_kzalloc(dev, sizeof(*ssd1307fb_defio),
 				       GFP_KERNEL);
 	if (!ssd1307fb_defio) {
-		dev_err(dev, "Couldn't allocate deferred io.\n");
 		ret = -ENOMEM;
 		goto fb_defio_error;
 	}
@@ -788,6 +852,7 @@ static const struct i2c_device_id ssd1307fb_i2c_id[] = {
 	{ .name = "ssd1306fb" },
 	{ .name = "ssd1307fb" },
 	{ .name = "ssd1309fb" },
+	{ .name = "sh1107fb" },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ssd1307fb_i2c_id);

-- 
2.34.1

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

[sashiko-bot]

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

New issues:
- [Medium] Uninitialized variable `ret` returned when `pages == 0` and `page_address_mode` is enabled.

Pre-existing issues:
- [High] Passing a virtual address to `__free_pages()` instead of a `struct page *`.
- [High] Use-After-Free of physical framebuffer memory due to unrevoked mmap mappings on device removal.
--

--- Patch [1]: [PATCH v2 1/2] fbdev: ssd1307fb: add support for Sino Wealth SH1107 controller ---
commit e1d9663a6aba54b57c6911f220e7fb969872972f
Author: Jakub Turek <jakub.turek@elsta.tech>

fbdev: ssd1307fb: add support for Sino Wealth SH1107 controller

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.

> diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
> index 644b8d97b3811..19dc7671fa7ac 100644
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c

[ ... ]

> @@ -274,13 +308,28 @@ static int ssd1307fb_update_rect(struct ssd1307fb_par *par, unsigned int x,
>  				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);
>  	return ret;

[Severity: Medium]
Can this return an uninitialized variable from ssd1307fb_update_rect() in
drivers/video/fbdev/ssd1307fb.c?

When the rectangle has a height of 0 and y % 8 is 0, the calculated pages
evaluates to 0. If par->page_address_mode is true, both of the
if (!par->page_address_mode) blocks and the for loop are skipped entirely.

This leaves ret completely uninitialized. Is it possible for
ssd1307fb_defio_fillrect to call the __damage_area callback even if
sys_fillrect aborted on a 0-height rectangle, returning uninitialized memory?

[ ... ]

> @@ -658,6 +722,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))

[Severity: High]
This isn't a bug introduced by this patch, but does the probe error path in
drivers/video/fbdev/ssd1307fb.c pass a virtual address to a function expecting
a struct page pointer?

In ssd1307fb_probe():

        vmem = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
                                        get_order(vmem_size));
...
fb_defio_error:
        __free_pages(vmem, get_order(vmem_size));

Since __get_free_pages() returns a virtual address, passing it to
__free_pages() (which expects a struct page pointer) will misinterpret the
virtual address as a page descriptor. Could this cause memory corruption or a
kernel panic when manipulating buddy allocator lists?

A similar pattern appears in ssd1307fb_remove():

        __free_pages(__va(info->fix.smem_start), get_order(info->fix.smem_len));

[Severity: High]
This is a pre-existing issue, but does removing the device cause a physical
memory use-after-free if userspace has an active mmap?

During device unbinding in ssd1307fb_remove() in
drivers/video/fbdev/ssd1307fb.c, the driver frees the physical framebuffer
memory:

        __free_pages(__va(info->fix.smem_start), get_order(info->fix.smem_len));

If an unprivileged userspace process mapped the device memory via mmap, the
page table entries are never shot down using unmap_mapping_range(). Does
this allow userspace to retain read/write access to physical pages that have
been returned to the kernel allocator?

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

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

[Helge Deller]

On 7/31/26 16:45, 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 | 87 +++++++++++++++++++++++++++++++++++------
>   1 file changed, 76 insertions(+), 11 deletions(-)
series applied.

Thanks!
Helge

[PATCH v2 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 v2 2/2] dt-bindings: display: solomon,ssd1307fb: add SH1107 compatible

[sashiko-bot]

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

Pre-existing issues:
- [High] The fbdev driver ignores the device-specific default dimensions documented in the device tree bindings, falling back to a hardcoded 96x16 for all devices including the newly documented sinowealth,sh1107.
--

--- Patch [2]: [PATCH v2 2/2] dt-bindings: display: solomon,ssd1307fb: add SH1107 compatible ---
commit 6de7ab88b3219b227a8443711b01ecabe4a747f9
Author: Jakub Turek <jakub.turek@elsta.tech>

dt-bindings: display: solomon,ssd1307fb: add SH1107 compatible

This commit updates the device tree documentation to include the
"sinowealth,sh1107" compatible string. It also configures default values for
the display dimensions and clock settings when this compatible is used.

> diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> index 153ff86fb4059..6b17a701bbaa5 100644
> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
[ ... ]
> @@ -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

[Severity: High]
This isn't a bug introduced by this patch, but does the fbdev driver actually
respect these default dimensions?

Looking at ssd1307fb_probe() in drivers/video/fbdev/ssd1307fb.c, the driver
appears to hardcode the fallback values to 96x16:

drivers/video/fbdev/ssd1307fb.c:ssd1307fb_probe() {
    ...
	if (device_property_read_u32(dev, "solomon,width", &par->width))
		par->width = 96;

	if (device_property_read_u32(dev, "solomon,height", &par->height))
		par->height = 16;
    ...
}

If a device tree node for sinowealth,sh1107 is created without the
solomon,width and solomon,height properties, relying on the schema's
defaults, will the driver initialize the display with the hardcoded 96x16
resolution instead?

> +        solomon,dclk-div:
> +          default: 1
> +        solomon,dclk-frq:
> +          default: 8

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-feat-sh1107-v2-0-0d2893e44ebd@elsta.tech?part=2