public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Markus Probst" <markus.probst@posteo.de>
Cc: "Markus Probst via B4 Relay"
	<devnull+markus.probst.posteo.de@kernel.org>,
	"Lee Jones" <lee@kernel.org>, "Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Igor Korotin" <igor.korotin.linux@gmail.com>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	"Pavel Machek" <pavel@kernel.org>, "Len Brown" <lenb@kernel.org>,
	"Robert Moore" <robert.moore@intel.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	rust-for-linux@vger.kernel.org, driver-core@lists.linux.dev,
	linux-pci@vger.kernel.org, linux-leds@vger.kernel.org,
	linux-acpi@vger.kernel.org, acpica-devel@lists.linux.dev
Subject: Re: [PATCH v3 7/7] leds: add synology microp led driver
Date: Sun, 15 Mar 2026 20:41:06 +0100	[thread overview]
Message-ID: <DH3M1023PCBI.1HYYZU93NS1JX@kernel.org> (raw)
In-Reply-To: <eb2f7498c5f3247265effc47b3445a04ac71956e.camel@posteo.de>

On Sun Mar 15, 2026 at 7:47 PM CET, Markus Probst wrote:
> On Sun, 2026-03-15 at 19:20 +0100, Danilo Krummrich wrote:
>> Isn't this handled through IRQs, i.e. you device issues an IRQ and then you read
>> from the serial bus?
>> 
>> (I'm asking since such chips can usually be connected via different busses, e.g.
>> serial and I2C. And with I2C the slave can't issue a transfer by itself.)
>> 
>> Other MFD drivers register their own IRQ chip for this. I.e. one would register
>> an IRQ chip in the MFD driver and pass it to the sub-devices created through
>> mfd_add_devices(). Then the sub-device receives an IRQ and reads the regmap.
> You mean registering a virtual IRQ and triggering it on data receival?

Not really virtual, there are a lot of MFD drivers that register their own IRQ
chip to forward only relevant IRQs to the sub-device.

What you say should work as well, but as mentioned below, I feel like that's
overkill.

> Could you provide an example driver in the tree?

One example would be drivers/mfd/palmas.c, but there should be many more.

>> Now, if you don't have IRQs at all and the only event you get is through
>> receive_buf() (which implies that the chip is only compatible with a serial bus)
>> this technically still works, but might be a bit overkill.
>
> There is a physical IRQ, but the serial device bus abstracts that so
> the driver only has the receive_buf() function. The driver it self is
> not aware of an IRQs.

I think you are confusing the IRQ of the serial bus controller with a device
IRQ. The serial bus controller the device is connected to has an IRQ itself, but
what I mean is a device IRQ line.

This is very common for devices on busses such as I2C, SPI, etc., as they have
master/slave semantics. I.e. the device issues an IRQ and the kernel reads a
register subsequently.

UART does not force master/slave sematics on a bus level though.

That's why I asked whether the device is UART only, or if it supports other
busses as well.

> Having like a reverse regmap would be great (in addition), in which the
> mfd device is the one who calls write and the sub-device has to handle
> it. But I don't think something like this exists in the kernel.

I mean, it's not really that the kernel exposes registers to the device. The
device just uses the fact that the UART is not a master/slave bus and pushes a
single byte to the kernel to signal that a button has been pressed. So, it's
still "IRQ semantics".

(But I see that on abstract level one could argue in this direction.)

TBH, I think that the combination of this chip supporting multiple functions and
being connected through UART, where the device pushes bytes through the UART to
signal events is a bit of an edge case.

As mentioned, if it would be connected through I2C instead, it would be simple:
forward the IRQ and use a regmap, and you can do it entirely with generic
infrastructure and no custom APIs, which in the end is the idea of MFD. I.e. you
can describe the whole sub-device with a struct mfd_cell.

And while we could technically "emulate" this, it remains to be odd and has
unnecessary overhead.

I've seen one other case in the kernel, which is drivers/mfd/rave-sp.c. But this
driver doesn't use MFD infrastructure at all and just goes for a custom API,
which clearly defeats the purpose of MFD in the first place. I.e. it shouldn't
even live under drivers/mfd/.

Greg already mentioned the auxiliary bus, which for a custom API clearly is the
better choice.

But to be honest, the more I hear about this device, the more I feel like a
monolithic driver is all that's needed, as everything else sounds like overhead
that doesn't really provide any value.

I.e. if we can't (easily) use mfd cells and would need a custom API, then why
even split it up at all, given that splitting it up would probably the most
complicated part of the whole driver.

Greg, what do you think?

*me runs away*

  reply	other threads:[~2026-03-15 20:41 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-13 19:03 [PATCH v3 0/7] Introduce Synology Microp driver Markus Probst via B4 Relay
2026-03-13 19:03 ` [PATCH v3 1/7] rust: Add `parent_unchecked` function to `Device` Markus Probst via B4 Relay
2026-03-13 19:03 ` [PATCH v3 2/7] rust: add basic mfd abstractions Markus Probst via B4 Relay
2026-03-13 19:03 ` [PATCH v3 3/7] acpi: add acpi_of_match_device_ids Markus Probst via B4 Relay
2026-03-23 19:57   ` Rafael J. Wysocki
2026-03-24 15:30     ` Markus Probst
2026-03-24 16:01       ` Rafael J. Wysocki
2026-03-24 16:26         ` Markus Probst
2026-03-24 17:39           ` Rafael J. Wysocki
2026-03-13 19:03 ` [PATCH v3 4/7] mfd: match acpi devices against PRP0001 Markus Probst via B4 Relay
2026-03-13 19:03 ` [PATCH v3 5/7] dt-bindings: mfd: Add synology,microp device Markus Probst via B4 Relay
2026-03-13 19:37   ` Krzysztof Kozlowski
2026-03-13 20:29     ` Markus Probst
2026-03-14  8:49       ` Krzysztof Kozlowski
2026-03-14 12:31         ` Markus Probst
2026-03-14 13:59           ` Krzysztof Kozlowski
2026-03-14 14:54             ` Markus Probst
2026-03-13 19:03 ` [PATCH v3 6/7] mfd: Add synology microp core driver Markus Probst via B4 Relay
2026-03-13 19:03 ` [PATCH v3 7/7] leds: add synology microp led driver Markus Probst via B4 Relay
2026-03-13 21:00   ` Danilo Krummrich
2026-03-13 21:10     ` Markus Probst
2026-03-15 15:15     ` Markus Probst
2026-03-15 18:20       ` Danilo Krummrich
2026-03-15 18:47         ` Markus Probst
2026-03-15 19:41           ` Danilo Krummrich [this message]
2026-03-16  6:33             ` Greg Kroah-Hartman
2026-03-16 13:43               ` Markus Probst
2026-03-16 13:58                 ` Greg Kroah-Hartman
2026-03-16 18:06                   ` Markus Probst

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=DH3M1023PCBI.1HYYZU93NS1JX@kernel.org \
    --to=dakr@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=acpica-devel@lists.linux.dev \
    --cc=aliceryhl@google.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=devicetree@vger.kernel.org \
    --cc=devnull+markus.probst.posteo.de@kernel.org \
    --cc=driver-core@lists.linux.dev \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=igor.korotin.linux@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=kwilczynski@kernel.org \
    --cc=lee@kernel.org \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=markus.probst@posteo.de \
    --cc=ojeda@kernel.org \
    --cc=pavel@kernel.org \
    --cc=rafael@kernel.org \
    --cc=robert.moore@intel.com \
    --cc=robh@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /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