Message-ID: <81E65D49-79BD-4F61-A32F-D01494D22C1B@gmail.com>
Date: Tue, 26 Aug 2025 16:21:01 -0400
From: Jean-François Lessard <jefflessard3@...il.com>
To: Andy Shevchenko <andy.shevchenko@...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

Le 26 août 2025 14 h 26 min 37 s HAE, Andy Shevchenko <andy.shevchenko@...il.com> a écrit :
>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.
>

Understood. I'll provide a summary of the regmap evaluation and design
decisions in the cover letter of the next submission.

Powered by blists - more mailing lists