All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lee Jones <lee@kernel.org>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Guenter Roeck <linux@roeck-us.net>,
	Linus Walleij <linusw@kernel.org>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>, Peter Rosin <peda@axentia.se>,
	kernel@pengutronix.de, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linux-hwmon@vger.kernel.org,
	linux-gpio@vger.kernel.org, David Jander <david@protonic.nl>
Subject: Re: [PATCH v15 2/6] platform: misc: add NXP MC33978/MC34978 core driver
Date: Thu, 23 Jul 2026 15:53:00 +0100	[thread overview]
Message-ID: <20260723145300.GO3363113@google.com> (raw)
In-Reply-To: <20260710101358.2606941-3-o.rempel@pengutronix.de>

On Fri, 10 Jul 2026, Oleksij Rempel wrote:

> Add the core support module for the NXP MC33978 and MC34978 Multiple
> Switch Detection Interfaces (MSDI).
> 
> The MC33978/MC34978 devices provide 22 switch detection inputs, analog
> multiplexing (AMUX), and comprehensive hardware fault detection.
> 
> This core module handles:
> - SPI communications via a custom regmap bus to support the device's
>   pipelined two-frame MISO response requirement.
> - Interrupt demultiplexing, utilizing an irq_domain to provide 22 virtual
>   IRQs for switch state changes and 1 virtual IRQ for hardware faults.
> - Inline status harvesting from the SPI MSB to detect and trigger events
>   without requiring dedicated status register polling.
> 
> It exports mc33978_core_init(), called by the MFD driver added in the
> following patch. CONFIG_MC33978_CORE carries no prompt and is selected
> by CONFIG_MFD_MC33978, so this patch alone builds nothing new.
> 
> Note: The device currently lacks suspend/resume power management
> callbacks. If the system enters a sleep state cutting power to
> VDDQ/VBATP, the device will wake up in POR state with hardware interrupt
> masks reset. Power management support is intentionally deferred for now.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> changes v15:
> - Split out of the MFD patch, as requested by Lee Jones. The register
>   definitions in include/linux/mfd/mc33978.h are carried here rather than
>   with the MFD driver because this module includes them, keeping every
>   commit individually buildable.

I can't help feeling that this is a hack.

When I suggested moving the functional parts out, I meant properly
separating off and compartmentalising.  Instead, a huge slice has been
taken out of the initial submission's MFD driver and dumped into the
wild west that is drivers/platform.  Worse still; we're masquerading as
the MFD since the MFD's 'dev' pointer is being passed through so
everything here is operating as though it's the parent device.  You've
created half library / half MFD.

I get that we're on v15 and there's still a lot to do, but I guess
that's what happens when 3500 lines of code is submitted at the same
time.

My suggestion is to return to first principles; what lives where?

Allocating of shared resources, including the various regmaps, IRQs and
domains should live in the MFD subsystem - that's literally what it's
for.  Anything that does-a-thing, should be allocated a proper subsystem
and platform drivers should be created.

Complicated platform / embedded-controller type work (events,
conditions, bespoke messaging, etc) that truly cannot live anywhere else
can live in drivers/platform, but perhaps don't invent a new
sub-sub-system here or create an extended MFD device here - it should be
a separate entity.

Take a look to see how the other drivers/platform dumpers work.

-- 
Lee Jones

  parent reply	other threads:[~2026-07-23 14:53 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-10 10:13 [PATCH v15 0/6] mfd: Add support for NXP MC33978/MC34978 MSDI Oleksij Rempel
2026-07-10 10:13 ` [PATCH v15 1/6] dt-bindings: pinctrl: add " Oleksij Rempel
2026-07-10 10:19   ` sashiko-bot
2026-07-10 10:13 ` [PATCH v15 2/6] platform: misc: add NXP MC33978/MC34978 core driver Oleksij Rempel
2026-07-10 10:24   ` sashiko-bot
2026-07-23 14:53   ` Lee Jones [this message]
2026-07-23 15:04     ` Oleksij Rempel
2026-07-23 16:09       ` Lee Jones
2026-07-23 16:34         ` Oleksij Rempel
2026-07-10 10:13 ` [PATCH v15 3/6] mfd: mc33978: Add NXP MC33978/MC34978 MSDI support Oleksij Rempel
2026-07-10 10:25   ` sashiko-bot
2026-07-10 10:13 ` [PATCH v15 4/6] pinctrl: add NXP MC33978/MC34978 pinctrl driver Oleksij Rempel
2026-07-10 10:27   ` sashiko-bot
2026-07-10 15:22   ` Alvin Šipraga
2026-07-11  5:21     ` Oleksij Rempel
2026-07-15 14:25       ` Alvin Šipraga
2026-07-10 10:13 ` [PATCH v15 5/6] hwmon: add NXP MC33978/MC34978 driver Oleksij Rempel
2026-07-10 10:23   ` sashiko-bot
2026-07-12 22:20   ` Uwe Kleine-König
2026-07-13 13:20     ` Guenter Roeck
2026-07-13 15:13       ` Uwe Kleine-König
2026-07-10 10:13 ` [PATCH v15 6/6] mux: add NXP MC33978/MC34978 AMUX driver Oleksij Rempel
2026-07-10 10:25   ` sashiko-bot
2026-07-10 12:53   ` Alvin Šipraga
2026-07-12 22:18   ` Uwe Kleine-König

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=20260723145300.GO3363113@google.com \
    --to=lee@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=david@protonic.nl \
    --cc=devicetree@vger.kernel.org \
    --cc=kernel@pengutronix.de \
    --cc=krzk+dt@kernel.org \
    --cc=linusw@kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=o.rempel@pengutronix.de \
    --cc=peda@axentia.se \
    --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 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.