linux-kernel - Re: [PATCH v2 2/5] dt-bindings: display: ssd1307fb: Remove default width and height values

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d4828a3d-639a-a207-ff36-45c8c5d4d311@suse.de>
Date:   Mon, 12 Jun 2023 09:47:12 +0200
From:   Thomas Zimmermann <tzimmermann@...e.de>
To:     Javier Martinez Canillas <javierm@...hat.com>,
        Conor Dooley <conor@...nel.org>
Cc:     devicetree@...r.kernel.org, Conor Dooley <conor+dt@...nel.org>,
        linux-kernel@...r.kernel.org, Maxime Ripard <mripard@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        dri-devel@...ts.freedesktop.org,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
Subject: Re: [PATCH v2 2/5] dt-bindings: display: ssd1307fb: Remove default
 width and height values

Hi

Am 11.06.23 um 01:18 schrieb Javier Martinez Canillas:
> Conor Dooley <conor@...nel.org> writes:
> 
>> On Sat, Jun 10, 2023 at 07:51:35PM +0200, Javier Martinez Canillas wrote:
>>> Conor Dooley <conor@...nel.org> writes:
>>>
>>>> On Fri, Jun 09, 2023 at 07:09:37PM +0200, Javier Martinez Canillas wrote:
>>>>> A default resolution in the ssd130x driver isn't set to an arbitrary 96x16
>>>>> anymore. Instead is set to a width and height that's controller dependent.
>>>>
>>>> Did that change to the driver not break backwards compatibility with
>>>> existing devicetrees that relied on the default values to get 96x16?
>>>>
>>>
>>> It would but I don't think it is an issue in pratice. Most users of these
>>> panels use one of the multiple libraries on top of the spidev interface.
>>>
>>> For the small userbase that don't, I believe that they will use the rpif
>>> kernel and ssd1306-overlay.dtbo DTB overlay, which defaults to width=128
>>> and height=64 [1]. So those users will have to explicitly set a width and
>>> height for a 96x16 panel anyways.
>>>
>>> The intersection of users that have a 96x16 panel, assumed that default
>>> and consider the DTB a stable ABI, and only update their kernel but not
>>> the  DTB should be very small IMO.
>>
>> It's the adding of new defaults that makes it a bit messier, since you
>> can't even revert without potentially breaking a newer user. I'd be more
>> inclined to require the properties, rather change their defaults in the
>> binding, lest there are people relying on them.
> 
> I think that's OK, the old drivers/video/fbdev/ssd1307fb.c fbdev driver
> still has the old behaviour so it will only be a problem for users that
> want to move to the new ssd130x driver as well.
> 
> By looking at the git log history, the 96x16 resolution was chosen when
> the driver was merged because Maxime tested it on a CFA10036 board [1]
> that has a 96x16 panel that uses an SSD1307 controller.
> 
> But as mentioned, that chip can drive up to 128x39 pixels so the maximum
> makes more sense as default to me.
> 
> [1]: https://www.crystalfontz.com/product/cfa10036
> 
>> If you and the other knowledgeable folk in the area really do know such
>> users do not exist then I suppose it is fine to do.
> 
> I believe is fine, since as explained above that change was only done in
> the ssd130x DRM driver, not the ssd1307fb fbdev driver whose default is
> still 96x16. Both drivers share the same DT binding scheme, I was asked
> to do that to make it as much backward compatible as possible with the
> old fbdev driver.
> 
> But I will be OK to drop the "solomon,ssd130?fb-i2c" compatible strings
> from the DRM driver and only match against the new "solomon,ssd130?-i2c"
> compatible strings. And add a different DT binding schema for the ssd130x
> driver, if that would mean being able to fix things like the one mentioned
> in this patch.
> 
> In my opinion, trying to always make the drivers backward compatible with
> old DTBs only makes the drivers code more complicated for unclear benefit.
> 
> Usually this just ends being code that is neither used nor tested. Because
> in practice most people update the DTBs and kernels, instead of trying to
> make the DTB a stable ABI like firmware.
> 

 From my understanding, fixing the resolution is the correct thing to do 
here. Userspace needs to be able to handle these differences.

Best regards
Thomas

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (841 bytes)