Message-ID: <A045103F-1F73-4AC7-9316-1AF906ECDC9E@gmail.com>
Date: Tue, 26 Aug 2025 13:38:07 -0400
From: Jean-François Lessard <jefflessard3@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
CC: 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

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.

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

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.

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.

Powered by blists - more mailing lists