Date:   Wed, 22 Feb 2023 21:11:05 +0200
From:   Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc:     Robin van der Gracht <robin@protonic.nl>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>,
        Raul E Rangel <rrangel@chromium.org>,
        Wolfram Sang <wsa@kernel.org>, linux-kernel@vger.kernel.org,
        linux-i2c@vger.kernel.org, linux-usb@vger.kernel.org,
        Miguel Ojeda <ojeda@kernel.org>,
        Heikki Krogerus <heikki.krogerus@linux.intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        devicetree@vger.kernel.org
Subject: Re: [PATCH v1 2/3] auxdisplay: ht16k33: Make use of
 device_get_match_data()
Message-ID: <Y/ZoyaV10TCWhloT@smile.fi.intel.com>
References: <20230221133307.20287-1-andriy.shevchenko@linux.intel.com>
 <20230221133307.20287-3-andriy.shevchenko@linux.intel.com>
 <Y/TJs+Arban0ats8@smile.fi.intel.com>
 <be203dfd290e67c8ce74d11c5c9478a4@protonic.nl>
 <Y/UD3HWNy8uKYShC@smile.fi.intel.com>
 <0235f0fed989a8b027db720663699f5d@protonic.nl>
 <Y/ZKdN4nuHcL4DgE@smile.fi.intel.com>
 <Y/ZOyGo8X7r258EC@smile.fi.intel.com>
 <06f29d66-f16a-039c-ecd0-155bdcce00c1@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <06f29d66-f16a-039c-ecd0-155bdcce00c1@linaro.org>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Precedence: bulk

On Wed, Feb 22, 2023 at 07:46:25PM +0100, Krzysztof Kozlowski wrote:
> On 22/02/2023 18:20, Andy Shevchenko wrote:
> >>
> >>> Which effectively breaks i.e. user-space instantiation for other display
> >>> types which now do work due to i2c_of_match_device().
> >>> (so my suggestion above is not sufficient).
> >>>
> >>> Are you proposing extending and searching the I2C ID table to work around
> >>> that?
> >>
> >> See (1) above. This is the downside I have noticed after sending this series.
> >> So, the I?C ID table match has to be restored, but the above mentioned issues
> >> with existing table are not gone, hence they need to be addressed in the next
> >> version.
> > 
> > I see now what you mean. So, we have even more issues in this driver:
> > - I?C table is not in sync with all devices supported
> 
> Does anything actually rely on i2c_device_id table? ACPI would match
> either via ACPI or OF tables. All modern ARM systems (e.g. imx6) are
> DT-based. Maybe just drop the I2C ID table?

For I?C it's still possible to enumerate the device via sysfs, which is ABI.

> > - the OF ID table seems has something really badly formed for adafruit
> >   (just a number after a comma)
> 
> Maybe it is a model number? It was documented:
> Documentation/devicetree/bindings/auxdisplay/holtek,ht16k33.yaml

Yes, it's not a problem for ACPI/DT platforms, the problem is for the above
way of enumeration, so if we have more than 1 manufacturer that uses plain
numbers for the model, I?C framework may not distinguish which driver to use.

I.o.w. the part after comma in the compatible strings of the I?C devices must
be unique globally to make that enumeration disambiguous.

> > The latter shows how broken it is. The I?C ID table mechanism is used as
> > a backward compatibility to the OF. Unfortunately, user space may not provide
> > the data except in form of DT overlays, so for the legacy enumeration we
> > have only device name, which is a set of 4 digits for adafruit case.
> > 
> > Now imagine if by some reason we will get adafruit2 (you name it) with
> > the same schema. How I?C framework can understand that you meant adafruit
> > and not adafruit2? Or did I miss something?

-- 
With Best Regards,
Andy Shevchenko