From: "Jean-François Lessard" <jefflessard3@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: Andy Shevchenko <andy@kernel.org>,
Geert Uytterhoeven <geert@linux-m68k.org>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org,
devicetree@vger.kernel.org
Subject: Re: [PATCH v4 5/6] auxdisplay: TM16xx: Add support for I2C-based controllers
Date: Tue, 26 Aug 2025 13:38:07 -0400 [thread overview]
Message-ID: <A045103F-1F73-4AC7-9316-1AF906ECDC9E@gmail.com> (raw)
In-Reply-To: <aK3TIVbmFgv1ZiYs@smile.fi.intel.com>
Le 26 août 2025 11 h 30 min 41 s HAE, Andy Shevchenko <andriy.shevchenko@intel.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@intel.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.
next prev parent reply other threads:[~2025-08-26 17:38 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-25 3:32 [PATCH v4 0/6] auxdisplay: Add TM16xx 7-segment LED matrix display controllers driver Jean-François Lessard
2025-08-25 3:32 ` [PATCH v4 1/6] dt-bindings: vendor-prefixes: Add fdhisi, titanmec, princeton, winrise, wxicore Jean-François Lessard
2025-08-25 13:53 ` Andy Shevchenko
2025-08-26 2:57 ` Jean-François Lessard
2025-08-25 3:32 ` [PATCH v4 2/6] dt-bindings: auxdisplay: add Titan Micro Electronics TM16xx Jean-François Lessard
2025-08-25 18:26 ` Rob Herring
2025-08-26 1:33 ` Jean-François Lessard
2025-08-26 14:37 ` Jean-François Lessard
2025-08-29 15:26 ` Rob Herring
2025-08-29 16:26 ` Jean-François Lessard
2025-08-25 19:08 ` Per Larsson
2025-08-26 1:53 ` Jean-François Lessard
2025-08-25 3:32 ` [PATCH v4 3/6] auxdisplay: Add TM16xx 7-segment LED matrix display controllers driver Jean-François Lessard
2025-08-25 15:14 ` Andy Shevchenko
2025-08-25 17:48 ` Jean-François Lessard
2025-08-26 15:22 ` Andy Shevchenko
2025-08-26 20:44 ` Jean-François Lessard
2025-08-27 18:37 ` Jean-François Lessard
2025-09-01 6:04 ` Andy Shevchenko
2025-08-25 3:32 ` [PATCH v4 4/6] auxdisplay: TM16xx: Add keypad support for scanning matrix keys Jean-François Lessard
2025-08-25 3:32 ` [PATCH v4 5/6] auxdisplay: TM16xx: Add support for I2C-based controllers Jean-François Lessard
2025-08-25 15:18 ` Andy Shevchenko
2025-08-26 4:01 ` Jean-François Lessard
2025-08-26 15:30 ` Andy Shevchenko
2025-08-26 17:38 ` Jean-François Lessard [this message]
2025-08-26 18:26 ` Andy Shevchenko
2025-08-26 20:21 ` Jean-François Lessard
2025-08-25 3:32 ` [PATCH v4 6/6] auxdisplay: TM16xx: Add support for SPI-based controllers Jean-François Lessard
2025-08-25 15:19 ` Andy Shevchenko
2025-08-26 4:04 ` Jean-François Lessard
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=A045103F-1F73-4AC7-9316-1AF906ECDC9E@gmail.com \
--to=jefflessard3@gmail.com \
--cc=andriy.shevchenko@intel.com \
--cc=andy@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=geert@linux-m68k.org \
--cc=krzk+dt@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).