From: Chen-Yu Tsai <[email protected]>
The SINO WEALTH SH1106 is an OLED display driver that is somewhat
compatible with the SSD1306. It supports a slightly wider display,
at 132 instead of 128 pixels. The basic commands are the same, but
the SH1106 doesn't support the horizontal or vertical address modes.
Add a compatible string for it.
Signed-off-by: Chen-Yu Tsai <[email protected]>
---
Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
index 9baafd0c42dd..1ac016a2d847 100644
--- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
+++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
@@ -13,6 +13,7 @@ maintainers:
properties:
compatible:
enum:
+ - sinowealth,sh1106-i2c
- solomon,ssd1305fb-i2c
- solomon,ssd1306fb-i2c
- solomon,ssd1307fb-i2c
--
2.34.1
Hello Chen-Yu,
Thanks a lot for your patch.
On Wed, Mar 30, 2022 at 9:09 PM Chen-Yu Tsai <[email protected]> wrote:
>
> From: Chen-Yu Tsai <[email protected]>
>
> The SINO WEALTH SH1106 is an OLED display driver that is somewhat
> compatible with the SSD1306. It supports a slightly wider display,
> at 132 instead of 128 pixels. The basic commands are the same, but
> the SH1106 doesn't support the horizontal or vertical address modes.
>
> Add a compatible string for it.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
> ---
> Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> index 9baafd0c42dd..1ac016a2d847 100644
> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> @@ -13,6 +13,7 @@ maintainers:
> properties:
> compatible:
> enum:
> + - sinowealth,sh1106-i2c
I like that you didn't include a "fb" suffix for this, the existing
ones are cargo culting from the previous fbdev driver to make existing
DTBs compatible with the DRM driver.
I've been thinking if I should post a patch to compatible strings
without the "fb" and mark the current ones as deprecated...
Reviewed-by: Javier Martinez Canillas <[email protected]>
Best regards,
Javier
On 30/03/2022 21:08, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <[email protected]>
>
> The SINO WEALTH SH1106 is an OLED display driver that is somewhat
> compatible with the SSD1306. It supports a slightly wider display,
> at 132 instead of 128 pixels. The basic commands are the same, but
> the SH1106 doesn't support the horizontal or vertical address modes.
>
> Add a compatible string for it.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
> ---
> Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> index 9baafd0c42dd..1ac016a2d847 100644
> --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> @@ -13,6 +13,7 @@ maintainers:
> properties:
> compatible:
> enum:
> + - sinowealth,sh1106-i2c
> - solomon,ssd1305fb-i2c
> - solomon,ssd1306fb-i2c
> - solomon,ssd1307fb-i2c
Please update the allOf:if: blocks.
Best regards,
Krzysztof
Hello Chen-Yu,
On 4/4/22 17:06, Chen-Yu Tsai wrote:
[snip]
>>> enum:
>>> + - sinowealth,sh1106-i2c
>>
>> I like that you didn't include a "fb" suffix for this, the existing
>> ones are cargo culting from the previous fbdev driver to make existing
>> DTBs compatible with the DRM driver.
>>
>> I've been thinking if I should post a patch to compatible strings
>> without the "fb" and mark the current ones as deprecated...
>>
>> Reviewed-by: Javier Martinez Canillas <[email protected]>
>
> I also thought about dropping the "-i2c" suffix, but then thought
> there might be a case where someone wanted to search the device
> tree specifically for an I2C connected node using said compatible
> string.
>
> What do you think?
>
>
tl; dr: unfortunately we can't do it due how SPI and I2C report module
aliases. Otherwise module auto loading will not work. I wrote a much
longer explanation with some details not so long ago:
https://patchwork.kernel.org/project/dri-devel/patch/[email protected]/#24730793
BTW, I bought a SSD1306 SPI controller and go it working this weekend.
I plan to post the patches once yours land, to avoid in-flight series
that may conflict. And what I did is mark the -fb as deprecated, then
added "ssd130x-i2c" and "ssd130x-spi" compatibles strings.
The WIP patches can be found here in case you are interested:
https://github.com/martinezjavier/linux/tree/drm-ssd130x-spi
> ChenYu
>
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
On Fri, Apr 1, 2022 at 5:32 PM Javier Martinez Canillas
<[email protected]> wrote:
>
> Hello Chen-Yu,
>
> Thanks a lot for your patch.
>
> On Wed, Mar 30, 2022 at 9:09 PM Chen-Yu Tsai <[email protected]> wrote:
> >
> > From: Chen-Yu Tsai <[email protected]>
> >
> > The SINO WEALTH SH1106 is an OLED display driver that is somewhat
> > compatible with the SSD1306. It supports a slightly wider display,
> > at 132 instead of 128 pixels. The basic commands are the same, but
> > the SH1106 doesn't support the horizontal or vertical address modes.
> >
> > Add a compatible string for it.
> >
> > Signed-off-by: Chen-Yu Tsai <[email protected]>
> > ---
> > Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> > index 9baafd0c42dd..1ac016a2d847 100644
> > --- a/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> > +++ b/Documentation/devicetree/bindings/display/solomon,ssd1307fb.yaml
> > @@ -13,6 +13,7 @@ maintainers:
> > properties:
> > compatible:
> > enum:
> > + - sinowealth,sh1106-i2c
>
> I like that you didn't include a "fb" suffix for this, the existing
> ones are cargo culting from the previous fbdev driver to make existing
> DTBs compatible with the DRM driver.
>
> I've been thinking if I should post a patch to compatible strings
> without the "fb" and mark the current ones as deprecated...
>
> Reviewed-by: Javier Martinez Canillas <[email protected]>
I also thought about dropping the "-i2c" suffix, but then thought
there might be a case where someone wanted to search the device
tree specifically for an I2C connected node using said compatible
string.
What do you think?
ChenYu
On Mon, Apr 4, 2022 at 11:48 PM Javier Martinez Canillas
<[email protected]> wrote:
>
> Hello Chen-Yu,
>
> On 4/4/22 17:06, Chen-Yu Tsai wrote:
>
> [snip]
>
> >>> enum:
> >>> + - sinowealth,sh1106-i2c
> >>
> >> I like that you didn't include a "fb" suffix for this, the existing
> >> ones are cargo culting from the previous fbdev driver to make existing
> >> DTBs compatible with the DRM driver.
> >>
> >> I've been thinking if I should post a patch to compatible strings
> >> without the "fb" and mark the current ones as deprecated...
> >>
> >> Reviewed-by: Javier Martinez Canillas <[email protected]>
> >
> > I also thought about dropping the "-i2c" suffix, but then thought
> > there might be a case where someone wanted to search the device
> > tree specifically for an I2C connected node using said compatible
> > string.
> >
> > What do you think?
> >
> >
>
> tl; dr: unfortunately we can't do it due how SPI and I2C report module
> aliases. Otherwise module auto loading will not work. I wrote a much
> longer explanation with some details not so long ago:
>
> https://patchwork.kernel.org/project/dri-devel/patch/[email protected]/#24730793
Right. I think that crossed my mind at some point, but didn't stick.
Thanks for raising it again. :)
> BTW, I bought a SSD1306 SPI controller and go it working this weekend.
>
> I plan to post the patches once yours land, to avoid in-flight series
> that may conflict. And what I did is mark the -fb as deprecated, then
> added "ssd130x-i2c" and "ssd130x-spi" compatibles strings.
>
> The WIP patches can be found here in case you are interested:
>
> https://github.com/martinezjavier/linux/tree/drm-ssd130x-spi
I took a quick look and a couple things stood out:
1. Would 3-wire SPI ever be considered? If so, might want to
rename some of variables/functions as *_spi_4wire_* to
begin with.
2. Maybe we should move the ssd130x_deviceinfo stuff into the
core module, and define an enum to use for matching compatible
strings across the modules to their respective device info
entries? FYI we are doing this in drivers/mfd/axp20x* .
I think a friend of mine has a SPI based SH1106 module that I
could borrow and test/work on, but that's a big if.
Regards
ChenYu
On 4/4/22 18:11, Chen-Yu Tsai wrote:
[snip]
>>>
>>
>> tl; dr: unfortunately we can't do it due how SPI and I2C report module
>> aliases. Otherwise module auto loading will not work. I wrote a much
>> longer explanation with some details not so long ago:
>>
>> https://patchwork.kernel.org/project/dri-devel/patch/[email protected]/#24730793
>
> Right. I think that crossed my mind at some point, but didn't stick.
> Thanks for raising it again. :)
>
>> BTW, I bought a SSD1306 SPI controller and go it working this weekend.
>>
>> I plan to post the patches once yours land, to avoid in-flight series
>> that may conflict. And what I did is mark the -fb as deprecated, then
>> added "ssd130x-i2c" and "ssd130x-spi" compatibles strings.
>>
>> The WIP patches can be found here in case you are interested:
>>
>> https://github.com/martinezjavier/linux/tree/drm-ssd130x-spi
>
> I took a quick look and a couple things stood out:
>
> 1. Would 3-wire SPI ever be considered? If so, might want to
> rename some of variables/functions as *_spi_4wire_* to
> begin with.
>
That's a good question and something that I considered too. I have to
admit that never had a SPI device that uses the 3-wire scheme though
so I don't know how common that is.
The ssd1306 datasheet mentions that the chip supports it but I could
not find one to buy. Read that should be able to do it by soldering
some pads in the board but that wold be more hustle that would like.
For that reason I just went with only supporting 4-wire and someone
if really like could provide patches for 3-wire SPI.
> 2. Maybe we should move the ssd130x_deviceinfo stuff into the
> core module, and define an enum to use for matching compatible
> strings across the modules to their respective device info
> entries? FYI we are doing this in drivers/mfd/axp20x* .
>
Yes, that's a good idea. I'll add that refactoring as a part of the
SPI series. Thanks a lot for the suggestion, it was very useful.
> I think a friend of mine has a SPI based SH1106 module that I
> could borrow and test/work on, but that's a big if.
>
>
Cool. If it uses 4-wire too then I believe that would mostly work
out-of-the-box if you add a compatible string for it. I didn't
have to do any change in the core ssd103x driver for the ssd1306
SPI to work.
> Regards
> ChenYu
>
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat