Re: [PATCH 1/3] dt-bindings: display: ssd1307fb: Change "solomon,page-offset" default value

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1914 bytes --]

On Thu, Nov 16, 2023 at 07:07:37PM +0100, Javier Martinez Canillas wrote:
> This is used to specify the page start address offset of the display RAM.
> 
> The value is used as offset when setting the page start address with the
> SSD130X_SET_PAGE_RANGE command, and the driver currently sets its value to
> 1 if the property is not present in the Device Tree.
> 
> But the datasheet mentions that the value on reset for the page start is a
> 0, so it makes more sense to also have 0 as the default value for the page
> offset if the property is not present.
> 
> In fact, using a default value of 1 leads to the display not working when
> the fbdev is attached to the framebuffer console.
> 
> Reported-by: Sahaj Sarup <sahaj.sarup@linaro.org>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> 
>  .../devicetree/bindings/display/solomon,ssd1307fb.yaml          | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> index 3afbb52d1b7f..badd81459aaa 100644
> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> @@ -35,7 +35,7 @@ properties:
>  
>    solomon,page-offset:
>      $ref: /schemas/types.yaml#/definitions/uint32
> -    default: 1
> +    default: 0

I think I saw it pointed out by Maxime elsewhere that this breaks the
ABI. It would be nice if DT defaults matched the hardware's, but I don't
really think it is worth breaking the ABI here. Expanding the property
description to explain the impact of the particular values might help
with incorrect usage.

Thanks,
Conor.

>      description:
>        Offset of pages (band of 8 pixels) that the screen is mapped to
>  
> -- 
> 2.41.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]