All of lore.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: 37+ 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 ` Markus Probst
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   ` Markus Probst
2026-03-13 19:03 ` [PATCH v3 2/7] rust: add basic mfd abstractions Markus Probst via B4 Relay
2026-03-13 19:03   ` Markus Probst
2026-03-13 19:03 ` [PATCH v3 3/7] acpi: add acpi_of_match_device_ids Markus Probst via B4 Relay
2026-03-13 19:03   ` Markus Probst
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   ` Markus Probst
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:03   ` Markus Probst
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   ` Markus Probst
2026-03-13 19:03 ` [PATCH v3 7/7] leds: add synology microp led driver Markus Probst via B4 Relay
2026-03-13 19:03   ` Markus Probst
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.