| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2025062058-caddie-rendering-3bf1@gregkh>
Date: Fri, 20 Jun 2025 07:02:19 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Ming Yu <a0282524688@...il.com>
Cc: Guenter Roeck <linux@...ck-us.net>, Lee Jones <lee@...nel.org>,
linus.walleij@...aro.org, brgl@...ev.pl, andi.shyti@...nel.org,
mkl@...gutronix.de, mailhol.vincent@...adoo.fr,
andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, wim@...ux-watchdog.org,
jdelvare@...e.com, alexandre.belloni@...tlin.com,
linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
linux-i2c@...r.kernel.org, linux-can@...r.kernel.org,
netdev@...r.kernel.org, linux-watchdog@...r.kernel.org,
linux-hwmon@...r.kernel.org, linux-rtc@...r.kernel.org,
linux-usb@...r.kernel.org, Ming Yu <tmyu0@...oton.com>
Subject: Re: [PATCH v12 1/7] mfd: Add core driver for Nuvoton NCT6694
On Fri, Jun 20, 2025 at 10:54:44AM +0800, Ming Yu wrote:
> Dear Guenter and Greg,
>
> Thank you for reviewing,
>
> Greg KH <gregkh@...uxfoundation.org> 於 2025年6月20日 週五 上午1:18寫道：
> >
> > On Thu, Jun 19, 2025 at 09:58:04AM -0700, Guenter Roeck wrote:
> > > On 6/19/25 09:20, Greg KH wrote:
> > > > On Fri, Jun 20, 2025 at 12:03:01AM +0800, Ming Yu wrote:
> > > > > Lee Jones <lee@...nel.org> 於 2025年6月19日 週四 下午11:28寫道：
> > > > > >
> > > > > > On Thu, 19 Jun 2025, Ming Yu wrote:
> > > > > >
> > > > > > > Lee Jones <lee@...nel.org> 於 2025年6月19日 週四 下午7:53寫道：
> > > > > > > >
> > > > > > > > On Fri, 13 Jun 2025, Ming Yu wrote:
> > > > > > > >
> > > > > > > > > Lee Jones <lee@...nel.org> 於 2025年6月13日 週五 下午9:11寫道：
> > > > > > > > > >
> > > > > > > > > > On Fri, 13 Jun 2025, Ming Yu wrote:
> > > > > > > > > >
> > > > > > > > > > > Lee Jones <lee@...nel.org> 於 2025年6月12日 週四 下午11:23寫道：
> > > > > > > > > > > >
> > > > > > > > > > > > On Thu, 12 Jun 2025, Ming Yu wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > Dear Lee,
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thank you for reviewing,
> > > > > > > > > > > > >
> > > > > > > > > > > > > Lee Jones <lee@...nel.org> 於 2025年6月12日 週四 下午10:00寫道：
> > > > > > > > > > > > > >
> > > > > > > > > > > > > ...
> > > > > > > > > > > > > > > +static const struct mfd_cell nct6694_devs[] = {
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 0),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 1),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 2),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 3),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 4),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 5),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 6),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 7),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 8),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 9),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 10),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 11),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 12),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 13),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 14),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 15),
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 0),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 1),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 2),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 3),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 4),
> > > > > > > > > > > > > > > + MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 5),
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Why have we gone back to this silly numbering scheme?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > What happened to using IDA in the child driver?
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > In a previous version, I tried to maintain a static IDA in each
> > > > > > > > > > > > > sub-driver. However, I didn’t consider the case where multiple NCT6694
> > > > > > > > > > > > > devices are bound to the same driver — in that case, the IDs are not
> > > > > > > > > > > > > fixed and become unusable for my purpose.
> > > > > > > > > > > >
> > > > > > > > > > > > Not sure I understand.
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > As far as I know, if I maintain the IDA in the sub-drivers and use
> > > > > > > > > > > multiple MFD_CELL_NAME("nct6694-gpio") entries in the MFD, the first
> > > > > > > > > > > NCT6694 device bound to the GPIO driver will receive IDs 0~15.
> > > > > > > > > > > However, when a second NCT6694 device is connected to the system, it
> > > > > > > > > > > will receive IDs 16~31.
> > > > > > > > > > > Because of this behavior, I switched back to using platform_device->id.
> > > > > > > > > >
> > > > > > > > > > Each of the devices will probe once.
> > > > > > > > > >
> > > > > > > > > > The first one will be given 0, the second will be given 1, etc.
> > > > > > > > > >
> > > > > > > > > > Why would you give multiple IDs to a single device bound to a driver?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > The device exposes multiple peripherals — 16 GPIO controllers, 6 I2C
> > > > > > > > > adapters, 2 CAN FD controllers, and 2 watchdog timers. Each peripheral
> > > > > > > > > is independently addressable, has its own register region, and can
> > > > > > > > > operate in isolation. The IDs are used to distinguish between these
> > > > > > > > > instances.
> > > > > > > > > For example, the GPIO driver will be probed 16 times, allocating 16
> > > > > > > > > separate gpio_chip instances to control 8 GPIO lines each.
> > > > > > > > >
> > > > > > > > > If another device binds to this driver, it is expected to expose
> > > > > > > > > peripherals with the same structure and behavior.
> > > > > > > >
> > > > > > > > I still don't see why having a per-device IDA wouldn't render each
> > > > > > > > probed device with its own ID. Just as you have above.
> > > > > > > >
> > > > > > >
> > > > > > > For example, when the MFD driver and the I2C sub-driver are loaded,
> > > > > > > connecting the first NCT6694 USB device to the system results in 6
> > > > > > > nct6694-i2c platform devices being created and bound to the
> > > > > > > i2c-nct6694 driver. These devices receive IDs 0 through 5 via the IDA.
> > > > > > >
> > > > > > > However, when a second NCT6694 USB device is connected, its
> > > > > > > corresponding nct6694-i2c platform devices receive IDs 6 through 11 —
> > > > > > > instead of 0 through 5 as I originally expected.
> > > > > > >
> > > > > > > If I've misunderstood something, please feel free to correct me. Thank you!
> > > > > >
> > > > > > In the code above you register 6 I2C devices. Each device will be
> > > > > > assigned a platform ID 0 through 5. The .probe() function in the I2C
> > > > > > driver will be executed 6 times. In each of those calls to .probe(),
> > > > > > instead of pre-allocating a contiguous assignment of IDs here, you
> > > > > > should be able to use IDA in .probe() to allocate those same device IDs
> > > > > > 0 through 5.
> > > > > >
> > > > > > What am I missing here?
> > > > > >
> > > > >
> > > > > You're absolutely right in the scenario where a single NCT6694 device
> > > > > is present. However, I’m wondering how we should handle the case where
> > > > > a second or even third NCT6694 device is bound to the same MFD driver.
> > > > > In that situation, the sub-drivers using a static IDA will continue
> > > > > allocating increasing IDs, rather than restarting from 0 for each
> > > > > device. How should this be handled?
> > > >
> > > > What is wrong with increasing ids? The id value means nothing, they
> > > > just have to be unique.
> > > >
> > >
> > > Unless they are used in the client driver as index into an array, as in
> > > "this is the Nth instance of this device for this chip". There has to be
> > > _some_ means to pass N to the client driver.
> >
> > Ick, that should just be walking the list of child devices instead, as
> > obviously no one is hard coding array sizes for devices these days,
> > right? :)
> >
> > Anyway, sure, if you _have_ to have a specific id, then use a specific
> > id, but really, it should not matter.
> >
>
> I need fixed IDs in order to communicate with the sub-devices
> correctly. For instance, the I2C driver registers 6 devices, and the
> userspace interface needs to know which specific I2C controller (e.g.,
> index 0 ~ 5) to target with custom commands. Using fixed IDs allow the
> driver to maintain a consistent mapping between device instances and
> register sets.
>
> I'm open to better alternatives, but so far, using fixed
> platform_device->id has been the most straightforward way to achieve
> this.
The kernel is not responsible for numbering devices in a determinisitic
way like this, so be very careful. Userspace can implement whatever
policy it wants to figure out what device id is what actual device, by
using tools like udev and the like. See how it does this today by
looking at your /dev/disks/ directory on your system at the symlinks.
So good luck with this, but again, it's not the kernel's job to keep
device numbering static across boots, that is not something we guarantee
at all.
Perhaps you should not be using mfd for this? Perhaps something like
the auxbus would work better?
thanks,
greg k-h
Powered by blists - more mailing lists