[PATCH net 1/4] auxiliary: Allow empty id

Sean Anderson posted 4 patches 3 months, 3 weeks ago
There is a newer version of this series
[PATCH net 1/4] auxiliary: Allow empty id
Posted by Sean Anderson 3 months, 3 weeks ago
Support creating auxiliary devices with the id included as part of the
name. This allows for non-decimal ids, which may be more appropriate for
auxiliary devices created as children of memory-mapped devices. For
example, a name like "xilinx_emac.mac.802c0000" could be achieved by
setting .name to "mac.802c0000" and .id to AUXILIARY_DEVID_NONE.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 drivers/base/auxiliary.c      | 6 +++++-
 include/linux/auxiliary_bus.h | 4 +++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
index dba7c8e13a53..64a0d5e2eb83 100644
--- a/drivers/base/auxiliary.c
+++ b/drivers/base/auxiliary.c
@@ -331,7 +331,11 @@ int __auxiliary_device_add(struct auxiliary_device *auxdev, const char *modname)
 		return -EINVAL;
 	}
 
-	ret = dev_set_name(dev, "%s.%s.%d", modname, auxdev->name, auxdev->id);
+	if (auxdev->id == AUXILIARY_DEVID_NONE)
+		ret = dev_set_name(dev, "%s.%s", modname, auxdev->name);
+	else
+		ret = dev_set_name(dev, "%s.%s.%d", modname, auxdev->name,
+				   auxdev->id);
 	if (ret) {
 		dev_err(dev, "auxiliary device dev_set_name failed: %d\n", ret);
 		return ret;
diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
index 4086afd0cc6b..76904cf2c3dd 100644
--- a/include/linux/auxiliary_bus.h
+++ b/include/linux/auxiliary_bus.h
@@ -51,6 +51,8 @@
  * unregisters the auxiliary device.
  */
 
+#define AUXILIARY_DEVID_NONE	(-1)
+
 /**
  * struct auxiliary_device - auxiliary device object.
  * @dev: Device,
@@ -269,7 +271,7 @@ struct auxiliary_device *__devm_auxiliary_device_create(struct device *dev,
 
 #define devm_auxiliary_device_create(dev, devname, platform_data)     \
 	__devm_auxiliary_device_create(dev, KBUILD_MODNAME, devname,  \
-				       platform_data, 0)
+				       platform_data, AUXILIARY_DEVID_NONE)
 
 /**
  * module_auxiliary_driver() - Helper macro for registering an auxiliary driver
-- 
2.35.1.1320.gc452695387.dirty
Re: [PATCH net 1/4] auxiliary: Allow empty id
Posted by Greg Kroah-Hartman 3 months, 2 weeks ago
On Thu, Jun 19, 2025 at 04:05:34PM -0400, Sean Anderson wrote:
> Support creating auxiliary devices with the id included as part of the
> name. This allows for non-decimal ids, which may be more appropriate for
> auxiliary devices created as children of memory-mapped devices. For
> example, a name like "xilinx_emac.mac.802c0000" could be achieved by
> setting .name to "mac.802c0000" and .id to AUXILIARY_DEVID_NONE.

I don't see the justification for this, sorry.  An id is just an id, it
doesn't matter what is is and nothing should be relying on it to be the
same across reboots or anywhere else.  The only requirement is that it
be unique at this point in time in the system.

We're having this same discussion on a different thread for a different
bus as well.  This isn't something new, it's been hashed out and
resolved 20+ years ago...

So no, this change isn't ok to make at this point in time, sorry.

greg k-h
Re: [PATCH net 1/4] auxiliary: Allow empty id
Posted by Sean Anderson 3 months, 2 weeks ago
On 6/20/25 01:13, Greg Kroah-Hartman wrote:
> On Thu, Jun 19, 2025 at 04:05:34PM -0400, Sean Anderson wrote:
>> Support creating auxiliary devices with the id included as part of the
>> name. This allows for non-decimal ids, which may be more appropriate for
>> auxiliary devices created as children of memory-mapped devices. For
>> example, a name like "xilinx_emac.mac.802c0000" could be achieved by
>> setting .name to "mac.802c0000" and .id to AUXILIARY_DEVID_NONE.
> 
> I don't see the justification for this, sorry.  An id is just an id, it
> doesn't matter what is is and nothing should be relying on it to be the
> same across reboots or anywhere else.  The only requirement is that it
> be unique at this point in time in the system.

It identifies the device in log messages. Without this you have to read
sysfs to determine what device is (for example) producing an error. This
may be inconvenient to do if the error prevents the system from booting.
This series converts a platform device with a legible ID like
"802c0000.ethernet" to an auxiliary device, and I believe descriptive
device names produce a better developer experience.

This is also shorter and simpler than auto-generated IDs.

--Sean
Re: [PATCH net 1/4] auxiliary: Allow empty id
Posted by Greg Kroah-Hartman 3 months, 2 weeks ago
On Fri, Jun 20, 2025 at 11:37:40AM -0400, Sean Anderson wrote:
> On 6/20/25 01:13, Greg Kroah-Hartman wrote:
> > On Thu, Jun 19, 2025 at 04:05:34PM -0400, Sean Anderson wrote:
> >> Support creating auxiliary devices with the id included as part of the
> >> name. This allows for non-decimal ids, which may be more appropriate for
> >> auxiliary devices created as children of memory-mapped devices. For
> >> example, a name like "xilinx_emac.mac.802c0000" could be achieved by
> >> setting .name to "mac.802c0000" and .id to AUXILIARY_DEVID_NONE.
> > 
> > I don't see the justification for this, sorry.  An id is just an id, it
> > doesn't matter what is is and nothing should be relying on it to be the
> > same across reboots or anywhere else.  The only requirement is that it
> > be unique at this point in time in the system.
> 
> It identifies the device in log messages. Without this you have to read
> sysfs to determine what device is (for example) producing an error.

That's fine, read sysfs :)

> This
> may be inconvenient to do if the error prevents the system from booting.
> This series converts a platform device with a legible ID like
> "802c0000.ethernet" to an auxiliary device, and I believe descriptive
> device names produce a better developer experience.

You can still have 802c0000.ethernet be the prefix of the name, that's
fine.

> This is also shorter and simpler than auto-generated IDs.

Please stick with auto-generated ids, they will work properly here.

thanks,

greg k-h
Re: [PATCH net 1/4] auxiliary: Allow empty id
Posted by Sean Anderson 3 months, 2 weeks ago
On 6/20/25 12:02, Greg Kroah-Hartman wrote:
> On Fri, Jun 20, 2025 at 11:37:40AM -0400, Sean Anderson wrote:
>> On 6/20/25 01:13, Greg Kroah-Hartman wrote:
>> > On Thu, Jun 19, 2025 at 04:05:34PM -0400, Sean Anderson wrote:
>> >> Support creating auxiliary devices with the id included as part of the
>> >> name. This allows for non-decimal ids, which may be more appropriate for
>> >> auxiliary devices created as children of memory-mapped devices. For
>> >> example, a name like "xilinx_emac.mac.802c0000" could be achieved by
>> >> setting .name to "mac.802c0000" and .id to AUXILIARY_DEVID_NONE.
>> > 
>> > I don't see the justification for this, sorry.  An id is just an id, it
>> > doesn't matter what is is and nothing should be relying on it to be the
>> > same across reboots or anywhere else.  The only requirement is that it
>> > be unique at this point in time in the system.
>> 
>> It identifies the device in log messages. Without this you have to read
>> sysfs to determine what device is (for example) producing an error.
> 
> That's fine, read sysfs :)

I should not have to read sysfs to decode boot output. If there is an
error during boot I should be able to determine the offending device.
This very important when the boot process fails before init is started,
and very convenient otherwise. 

>> This
>> may be inconvenient to do if the error prevents the system from booting.
>> This series converts a platform device with a legible ID like
>> "802c0000.ethernet" to an auxiliary device, and I believe descriptive
>> device names produce a better developer experience.
> 
> You can still have 802c0000.ethernet be the prefix of the name, that's
> fine.

This is not possible due to how the auxiliary bus works. If device's
name is in the form "foo.id", then the driver must have an
auxiliary_device_id in its id_table with .name = "foo". So the address
*must* come after the last period in the name.

--Sean

>> This is also shorter and simpler than auto-generated IDs.
> 
> Please stick with auto-generated ids, they will work properly here.
> 
> thanks,
> 
> greg k-h
Re: [PATCH net 1/4] auxiliary: Allow empty id
Posted by Greg Kroah-Hartman 3 months, 2 weeks ago
On Fri, Jun 20, 2025 at 12:09:29PM -0400, Sean Anderson wrote:
> On 6/20/25 12:02, Greg Kroah-Hartman wrote:
> > On Fri, Jun 20, 2025 at 11:37:40AM -0400, Sean Anderson wrote:
> >> On 6/20/25 01:13, Greg Kroah-Hartman wrote:
> >> > On Thu, Jun 19, 2025 at 04:05:34PM -0400, Sean Anderson wrote:
> >> >> Support creating auxiliary devices with the id included as part of the
> >> >> name. This allows for non-decimal ids, which may be more appropriate for
> >> >> auxiliary devices created as children of memory-mapped devices. For
> >> >> example, a name like "xilinx_emac.mac.802c0000" could be achieved by
> >> >> setting .name to "mac.802c0000" and .id to AUXILIARY_DEVID_NONE.
> >> > 
> >> > I don't see the justification for this, sorry.  An id is just an id, it
> >> > doesn't matter what is is and nothing should be relying on it to be the
> >> > same across reboots or anywhere else.  The only requirement is that it
> >> > be unique at this point in time in the system.
> >> 
> >> It identifies the device in log messages. Without this you have to read
> >> sysfs to determine what device is (for example) producing an error.
> > 
> > That's fine, read sysfs :)
> 
> I should not have to read sysfs to decode boot output. If there is an
> error during boot I should be able to determine the offending device.
> This very important when the boot process fails before init is started,
> and very convenient otherwise. 

The boot log will show you the name of the device that is having a
problem.  And you get to pick a portion of that name to make it make
some kind of sense to users if you want.

> >> This
> >> may be inconvenient to do if the error prevents the system from booting.
> >> This series converts a platform device with a legible ID like
> >> "802c0000.ethernet" to an auxiliary device, and I believe descriptive
> >> device names produce a better developer experience.
> > 
> > You can still have 802c0000.ethernet be the prefix of the name, that's
> > fine.
> 
> This is not possible due to how the auxiliary bus works. If device's
> name is in the form "foo.id", then the driver must have an
> auxiliary_device_id in its id_table with .name = "foo". So the address
> *must* come after the last period in the name.

So what is the new name without this aux patch that looks so wrong?
What is the current log line before and after the change you made?

thanks,

greg k-h
Re: [PATCH net 1/4] auxiliary: Allow empty id
Posted by Sean Anderson 3 months, 2 weeks ago
On 6/20/25 12:15, Greg Kroah-Hartman wrote:
> On Fri, Jun 20, 2025 at 12:09:29PM -0400, Sean Anderson wrote:
>> On 6/20/25 12:02, Greg Kroah-Hartman wrote:
>> > On Fri, Jun 20, 2025 at 11:37:40AM -0400, Sean Anderson wrote:
>> >> On 6/20/25 01:13, Greg Kroah-Hartman wrote:
>> >> > On Thu, Jun 19, 2025 at 04:05:34PM -0400, Sean Anderson wrote:
>> >> >> Support creating auxiliary devices with the id included as part of the
>> >> >> name. This allows for non-decimal ids, which may be more appropriate for
>> >> >> auxiliary devices created as children of memory-mapped devices. For
>> >> >> example, a name like "xilinx_emac.mac.802c0000" could be achieved by
>> >> >> setting .name to "mac.802c0000" and .id to AUXILIARY_DEVID_NONE.
>> >> > 
>> >> > I don't see the justification for this, sorry.  An id is just an id, it
>> >> > doesn't matter what is is and nothing should be relying on it to be the
>> >> > same across reboots or anywhere else.  The only requirement is that it
>> >> > be unique at this point in time in the system.
>> >> 
>> >> It identifies the device in log messages. Without this you have to read
>> >> sysfs to determine what device is (for example) producing an error.
>> > 
>> > That's fine, read sysfs :)
>> 
>> I should not have to read sysfs to decode boot output. If there is an
>> error during boot I should be able to determine the offending device.
>> This very important when the boot process fails before init is started,
>> and very convenient otherwise. 
> 
> The boot log will show you the name of the device that is having a
> problem.  And you get to pick a portion of that name to make it make
> some kind of sense to users if you want.

As noted below, I can't! The name has to be in a very particular format
which does not allow for any differentiation *except* in the "id" portion.
Really the only thing I want to do is print the id in hexadecimal rather
than decimal.

>> >> This
>> >> may be inconvenient to do if the error prevents the system from booting.
>> >> This series converts a platform device with a legible ID like
>> >> "802c0000.ethernet" to an auxiliary device, and I believe descriptive
>> >> device names produce a better developer experience.
>> > 
>> > You can still have 802c0000.ethernet be the prefix of the name, that's
>> > fine.
>> 
>> This is not possible due to how the auxiliary bus works. If device's
>> name is in the form "foo.id", then the driver must have an
>> auxiliary_device_id in its id_table with .name = "foo". So the address
>> *must* come after the last period in the name.
> 
> So what is the new name without this aux patch that looks so wrong?
> What is the current log line before and after the change you made?

Well, without this patch if you have multiple devices the subsequent
ones can't be created because they all have id 0 and this conflicts in sysfs.

With this patch it looks something like

[    4.781268] xilinx_axienet 80200000.ethernet: autodetected 64-bit DMA range
[   21.889563] xilinx_emac.mac xilinx_emac.mac.80200000 net4: renamed from eth0
[   32.296965] xilinx_emac.mac xilinx_emac.mac.80200000 net4: PHY [axienet-80200000:05] driver [RTL8211F Gigabit Ethernet] (irq=70)
[   32.313456] xilinx_emac.mac xilinx_emac.mac.80200000 net4: configuring for inband/sgmii link mode
[   65.095419] xilinx_emac.mac xilinx_emac.mac.80200000 net4: Link is Up - 1Gbps/Full - flow control rx/tx

I also prototyped a version of PLATFORM_DEVID_AUTO which looks roughly
like:

[    5.424220] xilinx_axienet 80240000.ethernet: autodetected 64-bit DMA range
[  178.249494] xilinx_emac.mac xilinx_emac.mac.1-auto net4: renamed from eth0
[  178.714048] xilinx_emac.mac xilinx_emac.mac.1-auto net4: PHY [axienet-80200000:05] driver [RTL8211F Gigabit Ethernet] (irq=70)
[  178.731272] xilinx_emac.mac xilinx_emac.mac.1-auto net4: configuring for inband/sgmii link mode
[  182.818831] xilinx_emac.mac xilinx_emac.mac.1-auto net4: Link is Up - 1Gbps/Full - flow control rx/tx

The only identifying part of the name is the "net4" part of the netdev.
However, if there's a failure before creating the netdev then userspace
will never have a chance to rename it. For example, you might get

[    4.947215] xilinx_emac.mac xilinx_emac.mac.1-auto (unnamed net_device) (uninitialized): incorrect link mode  for in-band status

which leaves you with no clue as to what device went wrong.

--Sean