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

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 <[email protected]>

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/[email protected]?part=2