Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220330190846.13997-1-wens@kernel.org> <20220330190846.13997-3-wens@kernel.org>
 <CABxcv==csvqsxM46ce2LecDh4E-UxxD2DG+3E-hCFoyrdtRv7A@mail.gmail.com>
 <CAGb2v64VQPjan=EUkd8UhRZfV0g1GqBwPqhxQakS=7YhgvVDQA@mail.gmail.com> <92a46ea2-e23e-b7c8-ea5f-35d458ee1b76@redhat.com>
In-Reply-To: <92a46ea2-e23e-b7c8-ea5f-35d458ee1b76@redhat.com>
Reply-To: wens@kernel.org
From:   Chen-Yu Tsai <wens@kernel.org>
Date:   Tue, 5 Apr 2022 00:11:05 +0800
Message-ID: <CAGb2v66zj4x7zx4ht+6ojjKRxEe+M4bTc_mKRgf8wd_TK+Wf4g@mail.gmail.com>
Subject: Re: [PATCH 2/4] dt-bindings: display: ssd1307fb: Add entry for SINO
 WEALTH SH1106
To:     Javier Martinez Canillas <javierm@redhat.com>
Cc:     Javier Martinez Canillas <javier@dowhile0.org>,
        Maxime Ripard <mripard@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzk+dt@kernel.org>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        devicetree <devicetree@vger.kernel.org>,
        Linux Kernel <linux-kernel@vger.kernel.org>,
        dri-devel <dri-devel@lists.freedesktop.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Apr 4, 2022 at 11:48 PM Javier Martinez Canillas
<javierm@redhat.com> 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 <javierm@redhat.com>
> >
> > 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/20220209091204.2513437-1-javierm@redhat.com/#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