mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Jean-François Lessard" <jefflessard3@gmail.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Andy Shevchenko <andriy.shevchenko@intel.com>,
	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 16:21:01 -0400	[thread overview]
Message-ID: <81E65D49-79BD-4F61-A32F-D01494D22C1B@gmail.com> (raw)
In-Reply-To: <CAHp75Veb_tQ3QYybDqYvNDrfX36Ft_RM6LwwCie2qYdoZGZOQw@mail.gmail.com>

Le 26 août 2025 14 h 26 min 37 s HAE, Andy Shevchenko <andy.shevchenko@gmail.com> a écrit :
>On Tue, Aug 26, 2025 at 8:38 PM Jean-François Lessard
><jefflessard3@gmail.com> wrote:
>> 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.
>
>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.


  reply	other threads:[~2025-08-26 20:21 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
2025-08-26 18:26           ` Andy Shevchenko
2025-08-26 20:21             ` Jean-François Lessard [this message]
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=81E65D49-79BD-4F61-A32F-D01494D22C1B@gmail.com \
    --to=jefflessard3@gmail.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=andy.shevchenko@gmail.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

all inboxes | Powered by JetHome®