linux-kernel - Re: [PATCH v4 5/6] auxdisplay: TM16xx: Add support for I2C-based controllers

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: <CAHp75Veb_tQ3QYybDqYvNDrfX36Ft_RM6LwwCie2qYdoZGZOQw@mail.gmail.com>
Date: Tue, 26 Aug 2025 21:26:37 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Jean-François Lessard <jefflessard3@...il.com>
Cc: Andy Shevchenko <andriy.shevchenko@...el.com>, Andy Shevchenko <andy@...nel.org>, 
	Geert Uytterhoeven <geert@...ux-m68k.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, linux-kernel@...r.kernel.org, 
	linux-leds@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v4 5/6] auxdisplay: TM16xx: Add support for I2C-based controllers

On Tue, Aug 26, 2025 at 8:38 PM Jean-François Lessard
<jefflessard3@...il.com> wrote:
> Le 26 août 2025 11 h 30 min 41 s HAE, Andy Shevchenko <andriy.shevchenko@...el.com> a écrit :
> >On Tue, Aug 26, 2025 at 12:01:57AM -0400, Jean-François Lessard wrote:
> >> Le 25 août 2025 11 h 18 min 27 s HAE, Andy Shevchenko <andriy.shevchenko@...el.com> a écrit :
> >> >On Sun, Aug 24, 2025 at 11:32:31PM -0400, Jean-François Lessard wrote:

...

> >> >Can we use regmap for all parts of the driver? Why not?
> >>
> >> These controllers implement custom 2-wire/3-wire protocols that share
> >> sufficient commonalities with I2C/SPI to leverage those subsystems, but are not
> >> fully compliant with standard register-based access patterns.
> >>
> >> Specific regmap incompatibilities:
> >>
> >> I2C protocol:
> >> - Dynamic addressing: slave address embedded in command byte (data[0] >> 1)
> >
> >Isn't this called paging? Or actually we have also non-standard
> >(non-power-of-2) regmap implementations, perhaps one of them
> >(7 + 9) if exists is what you need?
> >
> >> - Custom message flags: requires I2C_M_NO_RD_ACK for reads
> >
> >Hmm... If we have more than one device like this, we might implement the
> >support in regmap. Or, perhaps, the custom regmap IO accessors can solve this.
> >
> >> SPI protocol:
> >> - Inter-transfer timing: mandatory TM16XX_SPI_TWAIT_US delay between
> >> command/data
> >
> >One may implement custom regmap IO accessors.
> >
> >> - CS control: requires cs_change = 0 to maintain assertion across phases
> >>
> >> Regmap's I2C/SPI bus implementations use fixed addressing and standard transfer
> >> patterns without support for these protocol-specific requirements. A custom
> >> regmap bus would internally call these same helper functions without providing
> >> practical benefit.
> >
> >regmap provides a few benefits on top of the raw implementations. First of all,
> >it takes care about synchronisation (and as a side effect enables
> >configurations of the multi-functional HW, if ever needed in this case). It also
> >gives a debugfs implementation, and paging support (if it's what we need).
> >And many more...
> >
> >> The explicit transfer approach better reflects the actual hardware protocol
> >> requirements.
> >
> >That said, please, try to look into it closer.
> >
>
> I investigated your regmap suggestions thoroughly:
>
> Custom IO accessors:
> While technically possible, TM16xx protocols create significant implementation
> challenges:
>
> - TM1650: Commands 0x48 (control) and 0x4F (key read) appear as I2C addresses
> but represent completely different operations with different data structures.
> Custom accessors would need complex command routing logic.

So, to me it sounds like mutli-functional I²C device with two clients,
hence would be two drivers under the same umbrella.

> - TM1628: Requires coordinated command sequences (mode -> write command ->
> control command -> data transfers). A single regmap_write() call can't express
> this multi-step initialization.

I believe we have something like that done in a few cases in the
kernel, but I don't remember for sure.

> Paging/non-standard addressing:
> TM1650's 0x68-0x6E digit commands could theoretically map to regmap pages, but
> the 0x48/0x4F control/read commands break the model since they're fundamentally
> different operations, not register variants.

Paging can be partial.

> You're correct that regmap provides valuable synchronization, debugfs, and
> abstraction benefits. However, implementing custom accessors for TM16xx would
> essentially recreate the existing controller functions while forcing them into
> register semantics they don't naturally fit.
>
> Custom regmap implementation is possible but would add significant complexity
> to achieve register abstraction over inherently command-based protocols, while
> the current approach directly expresses the hardware's actual command structure.

Okay, if you still think regmap doesn't fit this case, please provide
a summary in the cover letter to describe all your discoveries and
thoughts.

-- 
With Best Regards,
Andy Shevchenko