[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250904101705.GH2764654@google.com>
Date: Thu, 4 Sep 2025 11:17:05 +0100
From: Lee Jones <lee@...nel.org>
To: Marcos Del Sol Vives <marcos@...a.pet>
Cc: linux-kernel@...r.kernel.org, Linus Walleij <linus.walleij@...aro.org>,
Bartosz Golaszewski <brgl@...ev.pl>,
Michael Walle <mwalle@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>, linux-gpio@...r.kernel.org,
linux-pci@...r.kernel.org
Subject: Re: [PATCH v4 3/3] mfd: vortex: implement new driver for Vortex
southbridges
On Wed, 03 Sep 2025, Marcos Del Sol Vives wrote:
> El 03/09/2025 a las 16:01, Lee Jones escribió:
> >> patch series and thus make it a proper MFD (at the cost of delaying
> >> even further the GPIO inclusion), or keep the struct mfd_cell array
> >> as a single-element array and implement the watchdog later on another
> >> merge request, using that very same array.
> >>
> >> I am however not okay with wasting my time rewriting that to bypass
> >> the MFD API for this, so I can waste even more time later
> >> implementing again the MFD API, just because linguistically
> >> one (right now) is technically not "multi".
> >
> > I don't get this. If you implement the WDT now, you will be "multi", so
> > what are you protesting against?
>
> That GPIO is something required to perform the poweroff sequence, a must
> for any machine, while WDT is just a "nice to have".
>
> Implementing now the WDT just because of a linguistic preference means
> delaying something more important in favour of a "nice to have".
You use the word "delaying" here. What's the rush?
If you only need a GPIO driver, then you don't need the MFD part.
> >> That seems very unreasonable, specially when it wouldn't be a first
> >> since at least these other devices are also using MFD with a single
> >> device:
> >>
> >> - 88pm80
> >
> > % grep name drivers/mfd/88pm800.c
> > .name = "88pm80x-rtc",
> > .name = "88pm80x-onkey",
> > .name = "88pm80x-regulator",
> > .name = "88pm800",
>
> If you open the file, you'll see it uses five single-element arrays.
Which is fine.
> >> - 88pm805
> >
> > % grep name drivers/mfd/88pm805.c
> > .name = "88pm80x-codec",
> > .name = "88pm805",
> >
>
> Same as above.
>
> >> - at91-usart
> >
> > % grep NAME drivers/mfd/at91-usart.c
> > MFD_CELL_NAME("at91_usart_spi");
> > MFD_CELL_NAME("atmel_usart_serial");
>
> Has two single-element arrays. It registers one or the other, never both
> (just like my patch does!)
Fair point.
I guess this is more of a selector than a true concurrent MFD.
> >> - stw481x
> >
> > * Copyright (C) 2013 ST-Ericsson SA
> >
> >> - vx855
> >
> > * Copyright (C) 2009 VIA Technologies, Inc.
> >
> >> - wm8400
> >
> > * Copyright 2008 Wolfson Microelectronics PLC.
> >
>
> To my knowledge the definition of "multi" has not been changed
> since any of those years.
If they were submitted during my tenure, it is likely that they would
not have been accepted. Besides, past mistakes is no excuse for making
them in the present.
--
Lee Jones [李琼斯]
Powered by blists - more mailing lists