[PATCH net v2 1/4] auxiliary: Support hexadecimal ids

Sean Anderson posted 4 patches 2 months, 3 weeks ago
There is a newer version of this series
[PATCH net v2 1/4] auxiliary: Support hexadecimal ids
Posted by Sean Anderson 2 months, 3 weeks ago
Support creating auxiliary devices with the id included as part of the
name. This allows for hexadecimal ids, which may be more appropriate for
auxiliary devices created as children of memory-mapped devices. If an
auxiliary device's id is set to AUXILIARY_DEVID_NONE, the name must
be of the form "name.id".

With this patch, dmesg logs from an auxiliary device might look 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

this is especially useful when compared to what might happen if there is
an error before userspace has the chance to assign a name to the netdev:

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

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

Changes in v2:
- Add example log output to commit message

 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 v2 1/4] auxiliary: Support hexadecimal ids
Posted by Greg Kroah-Hartman 2 months, 3 weeks ago
On Tue, Jul 15, 2025 at 08:01:07PM -0400, Sean Anderson wrote:
> Support creating auxiliary devices with the id included as part of the
> name. This allows for hexadecimal ids, which may be more appropriate for
> auxiliary devices created as children of memory-mapped devices. If an
> auxiliary device's id is set to AUXILIARY_DEVID_NONE, the name must
> be of the form "name.id".
> 
> With this patch, dmesg logs from an auxiliary device might look 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
> 
> this is especially useful when compared to what might happen if there is
> an error before userspace has the chance to assign a name to the netdev:
> 
> [    4.947215] xilinx_emac.mac xilinx_emac.mac.1 (unnamed net_device) (uninitialized): incorrect link mode  for in-band status
> 
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
> 
> Changes in v2:
> - Add example log output to commit message

I rejected v1, why is this being sent again?

confused,

greg k-h
Re: [PATCH net v2 1/4] auxiliary: Support hexadecimal ids
Posted by Sean Anderson 2 months, 3 weeks ago
On 7/16/25 01:09, Greg Kroah-Hartman wrote:
> On Tue, Jul 15, 2025 at 08:01:07PM -0400, Sean Anderson wrote:
>> Support creating auxiliary devices with the id included as part of the
>> name. This allows for hexadecimal ids, which may be more appropriate for
>> auxiliary devices created as children of memory-mapped devices. If an
>> auxiliary device's id is set to AUXILIARY_DEVID_NONE, the name must
>> be of the form "name.id".
>> 
>> With this patch, dmesg logs from an auxiliary device might look 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
>> 
>> this is especially useful when compared to what might happen if there is
>> an error before userspace has the chance to assign a name to the netdev:
>> 
>> [    4.947215] xilinx_emac.mac xilinx_emac.mac.1 (unnamed net_device) (uninitialized): incorrect link mode  for in-band status
>> 
>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>> ---
>> 
>> Changes in v2:
>> - Add example log output to commit message
> 
> I rejected v1, why is this being sent again?

You asked for explanation, I provided it. I specifically pointed out why
I wanted to do things this way. But I got no response. So here in v2.

--Sean
Re: [PATCH net v2 1/4] auxiliary: Support hexadecimal ids
Posted by Greg Kroah-Hartman 2 months, 3 weeks ago
On Thu, Jul 17, 2025 at 11:49:37AM -0400, Sean Anderson wrote:
> On 7/16/25 01:09, Greg Kroah-Hartman wrote:
> > On Tue, Jul 15, 2025 at 08:01:07PM -0400, Sean Anderson wrote:
> >> Support creating auxiliary devices with the id included as part of the
> >> name. This allows for hexadecimal ids, which may be more appropriate for
> >> auxiliary devices created as children of memory-mapped devices. If an
> >> auxiliary device's id is set to AUXILIARY_DEVID_NONE, the name must
> >> be of the form "name.id".
> >> 
> >> With this patch, dmesg logs from an auxiliary device might look 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
> >> 
> >> this is especially useful when compared to what might happen if there is
> >> an error before userspace has the chance to assign a name to the netdev:
> >> 
> >> [    4.947215] xilinx_emac.mac xilinx_emac.mac.1 (unnamed net_device) (uninitialized): incorrect link mode  for in-band status
> >> 
> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> >> ---
> >> 
> >> Changes in v2:
> >> - Add example log output to commit message
> > 
> > I rejected v1, why is this being sent again?
> 
> You asked for explanation, I provided it. I specifically pointed out why
> I wanted to do things this way. But I got no response. So here in v2.

Again, I said, "do not do that, this is not how ids work in the driver
model", and you tried to show lots of reasons why you wanted to do it
this way despite me saying so.

So again, no, sorry, this isn't ok.  Don't attempt to encode information
in a device id like you are trying to do here, that's not what a device
id is for at all.  I need to go dig up my old patch that made all device
ids random numbers just to see what foolish assumptions busses and
userspace tools are making....

thanks,

greg k-h
Re: [PATCH net v2 1/4] auxiliary: Support hexadecimal ids
Posted by Sean Anderson 2 months, 3 weeks ago
On 7/17/25 11:59, Greg Kroah-Hartman wrote:
> On Thu, Jul 17, 2025 at 11:49:37AM -0400, Sean Anderson wrote:
>> On 7/16/25 01:09, Greg Kroah-Hartman wrote:
>> > On Tue, Jul 15, 2025 at 08:01:07PM -0400, Sean Anderson wrote:
>> >> Support creating auxiliary devices with the id included as part of the
>> >> name. This allows for hexadecimal ids, which may be more appropriate for
>> >> auxiliary devices created as children of memory-mapped devices. If an
>> >> auxiliary device's id is set to AUXILIARY_DEVID_NONE, the name must
>> >> be of the form "name.id".
>> >> 
>> >> With this patch, dmesg logs from an auxiliary device might look 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
>> >> 
>> >> this is especially useful when compared to what might happen if there is
>> >> an error before userspace has the chance to assign a name to the netdev:
>> >> 
>> >> [    4.947215] xilinx_emac.mac xilinx_emac.mac.1 (unnamed net_device) (uninitialized): incorrect link mode  for in-band status
>> >> 
>> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>> >> ---
>> >> 
>> >> Changes in v2:
>> >> - Add example log output to commit message
>> > 
>> > I rejected v1, why is this being sent again?
>> 
>> You asked for explanation, I provided it. I specifically pointed out why
>> I wanted to do things this way. But I got no response. So here in v2.
> 
> Again, I said, "do not do that, this is not how ids work in the driver
> model", and you tried to show lots of reasons why you wanted to do it
> this way despite me saying so.
> 
> So again, no, sorry, this isn't ok.  Don't attempt to encode information
> in a device id like you are trying to do here, that's not what a device
> id is for at all.  I need to go dig up my old patch that made all device
> ids random numbers just to see what foolish assumptions busses and
> userspace tools are making....

But it *is* how ids work in platform devices. And because my auxiliary
devices are created by a platform device, it is guaranteed that the
platform device id is unique and that it will also be unique for
auxiliary devices. So there is no assumption here about the uniqueness
of any given id.

--Sean
Re: [PATCH net v2 1/4] auxiliary: Support hexadecimal ids
Posted by Greg Kroah-Hartman 2 months, 3 weeks ago
On Thu, Jul 17, 2025 at 12:04:15PM -0400, Sean Anderson wrote:
> On 7/17/25 11:59, Greg Kroah-Hartman wrote:
> > On Thu, Jul 17, 2025 at 11:49:37AM -0400, Sean Anderson wrote:
> >> On 7/16/25 01:09, Greg Kroah-Hartman wrote:
> >> > On Tue, Jul 15, 2025 at 08:01:07PM -0400, Sean Anderson wrote:
> >> >> Support creating auxiliary devices with the id included as part of the
> >> >> name. This allows for hexadecimal ids, which may be more appropriate for
> >> >> auxiliary devices created as children of memory-mapped devices. If an
> >> >> auxiliary device's id is set to AUXILIARY_DEVID_NONE, the name must
> >> >> be of the form "name.id".
> >> >> 
> >> >> With this patch, dmesg logs from an auxiliary device might look 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
> >> >> 
> >> >> this is especially useful when compared to what might happen if there is
> >> >> an error before userspace has the chance to assign a name to the netdev:
> >> >> 
> >> >> [    4.947215] xilinx_emac.mac xilinx_emac.mac.1 (unnamed net_device) (uninitialized): incorrect link mode  for in-band status
> >> >> 
> >> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> >> >> ---
> >> >> 
> >> >> Changes in v2:
> >> >> - Add example log output to commit message
> >> > 
> >> > I rejected v1, why is this being sent again?
> >> 
> >> You asked for explanation, I provided it. I specifically pointed out why
> >> I wanted to do things this way. But I got no response. So here in v2.
> > 
> > Again, I said, "do not do that, this is not how ids work in the driver
> > model", and you tried to show lots of reasons why you wanted to do it
> > this way despite me saying so.
> > 
> > So again, no, sorry, this isn't ok.  Don't attempt to encode information
> > in a device id like you are trying to do here, that's not what a device
> > id is for at all.  I need to go dig up my old patch that made all device
> > ids random numbers just to see what foolish assumptions busses and
> > userspace tools are making....
> 
> But it *is* how ids work in platform devices.

No one should ever use platform devices/bus as an excuse to do anything,
it's "wrong" in so many ways, but needs to be because of special
reasons.  No other bus should work like that, sorry.

> And because my auxiliary
> devices are created by a platform device, it is guaranteed that the
> platform device id is unique and that it will also be unique for
> auxiliary devices. So there is no assumption here about the uniqueness
> of any given id.

Then perhaps use the faux device api instead?

thanks,

greg k-h
Re: [PATCH net v2 1/4] auxiliary: Support hexadecimal ids
Posted by Sean Anderson 2 months, 3 weeks ago
On 7/17/25 12:21, Greg Kroah-Hartman wrote:
> On Thu, Jul 17, 2025 at 12:04:15PM -0400, Sean Anderson wrote:
>> On 7/17/25 11:59, Greg Kroah-Hartman wrote:
>> > On Thu, Jul 17, 2025 at 11:49:37AM -0400, Sean Anderson wrote:
>> >> On 7/16/25 01:09, Greg Kroah-Hartman wrote:
>> >> > On Tue, Jul 15, 2025 at 08:01:07PM -0400, Sean Anderson wrote:
>> >> >> Support creating auxiliary devices with the id included as part of the
>> >> >> name. This allows for hexadecimal ids, which may be more appropriate for
>> >> >> auxiliary devices created as children of memory-mapped devices. If an
>> >> >> auxiliary device's id is set to AUXILIARY_DEVID_NONE, the name must
>> >> >> be of the form "name.id".
>> >> >> 
>> >> >> With this patch, dmesg logs from an auxiliary device might look 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
>> >> >> 
>> >> >> this is especially useful when compared to what might happen if there is
>> >> >> an error before userspace has the chance to assign a name to the netdev:
>> >> >> 
>> >> >> [    4.947215] xilinx_emac.mac xilinx_emac.mac.1 (unnamed net_device) (uninitialized): incorrect link mode  for in-band status
>> >> >> 
>> >> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>> >> >> ---
>> >> >> 
>> >> >> Changes in v2:
>> >> >> - Add example log output to commit message
>> >> > 
>> >> > I rejected v1, why is this being sent again?
>> >> 
>> >> You asked for explanation, I provided it. I specifically pointed out why
>> >> I wanted to do things this way. But I got no response. So here in v2.
>> > 
>> > Again, I said, "do not do that, this is not how ids work in the driver
>> > model", and you tried to show lots of reasons why you wanted to do it
>> > this way despite me saying so.
>> > 
>> > So again, no, sorry, this isn't ok.  Don't attempt to encode information
>> > in a device id like you are trying to do here, that's not what a device
>> > id is for at all.  I need to go dig up my old patch that made all device
>> > ids random numbers just to see what foolish assumptions busses and
>> > userspace tools are making....
>> 
>> But it *is* how ids work in platform devices.
> 
> No one should ever use platform devices/bus as an excuse to do anything,
> it's "wrong" in so many ways, but needs to be because of special
> reasons.  No other bus should work like that, sorry.
> 
>> And because my auxiliary
>> devices are created by a platform device, it is guaranteed that the
>> platform device id is unique and that it will also be unique for
>> auxiliary devices. So there is no assumption here about the uniqueness
>> of any given id.
> 
> Then perhaps use the faux device api instead?

There's *another* pseudo bus? OK the reason why is that faux was added
four months ago and there is nothing under Documentation for it. So I
had no idea it existed. I will have a look, but perhaps you should write
up some documentation about why someone might want to use a "faux" bus
over the auxiliary bus or MFD.

--Sean
Re: [PATCH net v2 1/4] auxiliary: Support hexadecimal ids
Posted by Greg Kroah-Hartman 2 months, 3 weeks ago
On Thu, Jul 17, 2025 at 12:27:44PM -0400, Sean Anderson wrote:
> On 7/17/25 12:21, Greg Kroah-Hartman wrote:
> > On Thu, Jul 17, 2025 at 12:04:15PM -0400, Sean Anderson wrote:
> >> On 7/17/25 11:59, Greg Kroah-Hartman wrote:
> >> > On Thu, Jul 17, 2025 at 11:49:37AM -0400, Sean Anderson wrote:
> >> >> On 7/16/25 01:09, Greg Kroah-Hartman wrote:
> >> >> > On Tue, Jul 15, 2025 at 08:01:07PM -0400, Sean Anderson wrote:
> >> >> >> Support creating auxiliary devices with the id included as part of the
> >> >> >> name. This allows for hexadecimal ids, which may be more appropriate for
> >> >> >> auxiliary devices created as children of memory-mapped devices. If an
> >> >> >> auxiliary device's id is set to AUXILIARY_DEVID_NONE, the name must
> >> >> >> be of the form "name.id".
> >> >> >> 
> >> >> >> With this patch, dmesg logs from an auxiliary device might look 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
> >> >> >> 
> >> >> >> this is especially useful when compared to what might happen if there is
> >> >> >> an error before userspace has the chance to assign a name to the netdev:
> >> >> >> 
> >> >> >> [    4.947215] xilinx_emac.mac xilinx_emac.mac.1 (unnamed net_device) (uninitialized): incorrect link mode  for in-band status
> >> >> >> 
> >> >> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> >> >> >> ---
> >> >> >> 
> >> >> >> Changes in v2:
> >> >> >> - Add example log output to commit message
> >> >> > 
> >> >> > I rejected v1, why is this being sent again?
> >> >> 
> >> >> You asked for explanation, I provided it. I specifically pointed out why
> >> >> I wanted to do things this way. But I got no response. So here in v2.
> >> > 
> >> > Again, I said, "do not do that, this is not how ids work in the driver
> >> > model", and you tried to show lots of reasons why you wanted to do it
> >> > this way despite me saying so.
> >> > 
> >> > So again, no, sorry, this isn't ok.  Don't attempt to encode information
> >> > in a device id like you are trying to do here, that's not what a device
> >> > id is for at all.  I need to go dig up my old patch that made all device
> >> > ids random numbers just to see what foolish assumptions busses and
> >> > userspace tools are making....
> >> 
> >> But it *is* how ids work in platform devices.
> > 
> > No one should ever use platform devices/bus as an excuse to do anything,
> > it's "wrong" in so many ways, but needs to be because of special
> > reasons.  No other bus should work like that, sorry.
> > 
> >> And because my auxiliary
> >> devices are created by a platform device, it is guaranteed that the
> >> platform device id is unique and that it will also be unique for
> >> auxiliary devices. So there is no assumption here about the uniqueness
> >> of any given id.
> > 
> > Then perhaps use the faux device api instead?
> 
> There's *another* pseudo bus? OK the reason why is that faux was added
> four months ago and there is nothing under Documentation for it. So I
> had no idea it existed. I will have a look, but perhaps you should write
> up some documentation about why someone might want to use a "faux" bus
> over the auxiliary bus or MFD.

"faux" is for when platform devices were being abused because someone
just wanted a device in the device tree, and did not use any of the
platform device resources.

Yes, more documentation is always a good idea, someday...

thanks,

greg k-h
Re: [PATCH net v2 1/4] auxiliary: Support hexadecimal ids
Posted by Sean Anderson 2 months, 3 weeks ago
On 7/17/25 12:33, Greg Kroah-Hartman wrote:
> On Thu, Jul 17, 2025 at 12:27:44PM -0400, Sean Anderson wrote:
>> On 7/17/25 12:21, Greg Kroah-Hartman wrote:
>> > On Thu, Jul 17, 2025 at 12:04:15PM -0400, Sean Anderson wrote:
>> >> On 7/17/25 11:59, Greg Kroah-Hartman wrote:
>> >> > On Thu, Jul 17, 2025 at 11:49:37AM -0400, Sean Anderson wrote:
>> >> >> On 7/16/25 01:09, Greg Kroah-Hartman wrote:
>> >> >> > On Tue, Jul 15, 2025 at 08:01:07PM -0400, Sean Anderson wrote:
>> >> >> >> Support creating auxiliary devices with the id included as part of the
>> >> >> >> name. This allows for hexadecimal ids, which may be more appropriate for
>> >> >> >> auxiliary devices created as children of memory-mapped devices. If an
>> >> >> >> auxiliary device's id is set to AUXILIARY_DEVID_NONE, the name must
>> >> >> >> be of the form "name.id".
>> >> >> >> 
>> >> >> >> With this patch, dmesg logs from an auxiliary device might look 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
>> >> >> >> 
>> >> >> >> this is especially useful when compared to what might happen if there is
>> >> >> >> an error before userspace has the chance to assign a name to the netdev:
>> >> >> >> 
>> >> >> >> [    4.947215] xilinx_emac.mac xilinx_emac.mac.1 (unnamed net_device) (uninitialized): incorrect link mode  for in-band status
>> >> >> >> 
>> >> >> >> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>> >> >> >> ---
>> >> >> >> 
>> >> >> >> Changes in v2:
>> >> >> >> - Add example log output to commit message
>> >> >> > 
>> >> >> > I rejected v1, why is this being sent again?
>> >> >> 
>> >> >> You asked for explanation, I provided it. I specifically pointed out why
>> >> >> I wanted to do things this way. But I got no response. So here in v2.
>> >> > 
>> >> > Again, I said, "do not do that, this is not how ids work in the driver
>> >> > model", and you tried to show lots of reasons why you wanted to do it
>> >> > this way despite me saying so.
>> >> > 
>> >> > So again, no, sorry, this isn't ok.  Don't attempt to encode information
>> >> > in a device id like you are trying to do here, that's not what a device
>> >> > id is for at all.  I need to go dig up my old patch that made all device
>> >> > ids random numbers just to see what foolish assumptions busses and
>> >> > userspace tools are making....
>> >> 
>> >> But it *is* how ids work in platform devices.
>> > 
>> > No one should ever use platform devices/bus as an excuse to do anything,
>> > it's "wrong" in so many ways, but needs to be because of special
>> > reasons.  No other bus should work like that, sorry.
>> > 
>> >> And because my auxiliary
>> >> devices are created by a platform device, it is guaranteed that the
>> >> platform device id is unique and that it will also be unique for
>> >> auxiliary devices. So there is no assumption here about the uniqueness
>> >> of any given id.
>> > 
>> > Then perhaps use the faux device api instead?
>> 
>> There's *another* pseudo bus? OK the reason why is that faux was added
>> four months ago and there is nothing under Documentation for it. So I
>> had no idea it existed. I will have a look, but perhaps you should write
>> up some documentation about why someone might want to use a "faux" bus
>> over the auxiliary bus or MFD.
> 
> "faux" is for when platform devices were being abused because someone
> just wanted a device in the device tree, and did not use any of the
> platform device resources.

OK, well that's not this. These are real devices and there may be more
than one per system. Actually, that's the primary problem I wanted to
address with this patch: you can't create more than one device with
devm_auxiliary_device_create because they will have the same id (0).

Anyway, if you really think ids should be random or whatever, why not
just ida_alloc one in axiliary_device_init and ignore whatever's
provided? I'd say around half the auxiliary drivers just use 0 (or some
other constant), which is just as deterministic as using the device
address. Another third use ida_alloc (or xa_alloc) so all that could be
removed. And the remainder just use an address of some kind (which ends
up be formatted as decimal).

--Sean
Re: [PATCH net v2 1/4] auxiliary: Support hexadecimal ids
Posted by Leon Romanovsky 2 months, 2 weeks ago
On Thu, Jul 17, 2025 at 01:12:08PM -0400, Sean Anderson wrote:
> On 7/17/25 12:33, Greg Kroah-Hartman wrote:

<...>

> Anyway, if you really think ids should be random or whatever, why not
> just ida_alloc one in axiliary_device_init and ignore whatever's
> provided? I'd say around half the auxiliary drivers just use 0 (or some
> other constant), which is just as deterministic as using the device
> address.

I would say that auxiliary bus is not right fit for such devices. This
bus was introduced for more complex devices, like the one who has their
own ida_alloc logic.


> Another third use ida_alloc (or xa_alloc) so all that could be
> removed.

These ID numbers need to be per-device.

Thanks

> 
> --Sean
Re: [PATCH net v2 1/4] auxiliary: Support hexadecimal ids
Posted by Sean Anderson 2 months, 2 weeks ago
On 7/20/25 04:17, Leon Romanovsky wrote:
> On Thu, Jul 17, 2025 at 01:12:08PM -0400, Sean Anderson wrote:
>> On 7/17/25 12:33, Greg Kroah-Hartman wrote:
> 
> <...>
> 
>> Anyway, if you really think ids should be random or whatever, why not
>> just ida_alloc one in axiliary_device_init and ignore whatever's
>> provided? I'd say around half the auxiliary drivers just use 0 (or some
>> other constant), which is just as deterministic as using the device
>> address.
> 
> I would say that auxiliary bus is not right fit for such devices. This
> bus was introduced for more complex devices, like the one who has their
> own ida_alloc logic.

I'd say that around 2/3 of the auxiliary drivers that have non-constant
ids use ida_alloc solely for the auxiliary bus and for no other purpose.
I don't think that's the kind of complexity you're referring to.

>> Another third use ida_alloc (or xa_alloc) so all that could be
>> removed.
> 
> These ID numbers need to be per-device.

Why? They are arbitrary with no semantic meaning, right?

--Sean
Re: [PATCH net v2 1/4] auxiliary: Support hexadecimal ids
Posted by Leon Romanovsky 2 months, 2 weeks ago
On Mon, Jul 21, 2025 at 10:29:32AM -0400, Sean Anderson wrote:
> On 7/20/25 04:17, Leon Romanovsky wrote:
> > On Thu, Jul 17, 2025 at 01:12:08PM -0400, Sean Anderson wrote:
> >> On 7/17/25 12:33, Greg Kroah-Hartman wrote:
> > 
> > <...>
> > 
> >> Anyway, if you really think ids should be random or whatever, why not
> >> just ida_alloc one in axiliary_device_init and ignore whatever's
> >> provided? I'd say around half the auxiliary drivers just use 0 (or some
> >> other constant), which is just as deterministic as using the device
> >> address.
> > 
> > I would say that auxiliary bus is not right fit for such devices. This
> > bus was introduced for more complex devices, like the one who has their
> > own ida_alloc logic.
> 
> I'd say that around 2/3 of the auxiliary drivers that have non-constant
> ids use ida_alloc solely for the auxiliary bus and for no other purpose.
> I don't think that's the kind of complexity you're referring to.
> 
> >> Another third use ida_alloc (or xa_alloc) so all that could be
> >> removed.
> > 
> > These ID numbers need to be per-device.
> 
> Why? They are arbitrary with no semantic meaning, right?

Yes, officially there is no meaning, and this is how we would like to
keep it.

Right now, they are very correlated with with their respective PCI function number.
Is it important? No, however it doesn't mean that we should proactively harm user
experience just because we can do it.

[leonro@c ~]$ l /sys/bus/auxiliary/devices/
,,,
rwxrwxrwx 1 root root 0 Jul 21 15:25 mlx5_core.rdma.0 -> ../../../devices/pci0000:00/0000:00:02.7/0000:0
8:00.0/mlx5_core.rdma.0
lrwxrwxrwx 1 root root 0 Jul 21 15:25 mlx5_core.rdma.1 -> ../../../devices/pci0000:00/0000:00:02.7/0000:0
8:00.1/mlx5_core.rdma
...

Thanks

> 
> --Sean
>
Re: [PATCH net v2 1/4] auxiliary: Support hexadecimal ids
Posted by Sean Anderson 2 months, 2 weeks ago
On 7/23/25 04:13, Leon Romanovsky wrote:
> On Mon, Jul 21, 2025 at 10:29:32AM -0400, Sean Anderson wrote:
>> On 7/20/25 04:17, Leon Romanovsky wrote:
>> > On Thu, Jul 17, 2025 at 01:12:08PM -0400, Sean Anderson wrote:
>> >> On 7/17/25 12:33, Greg Kroah-Hartman wrote:
>> > 
>> > <...>
>> > 
>> >> Anyway, if you really think ids should be random or whatever, why not
>> >> just ida_alloc one in axiliary_device_init and ignore whatever's
>> >> provided? I'd say around half the auxiliary drivers just use 0 (or some
>> >> other constant), which is just as deterministic as using the device
>> >> address.
>> > 
>> > I would say that auxiliary bus is not right fit for such devices. This
>> > bus was introduced for more complex devices, like the one who has their
>> > own ida_alloc logic.
>> 
>> I'd say that around 2/3 of the auxiliary drivers that have non-constant
>> ids use ida_alloc solely for the auxiliary bus and for no other purpose.
>> I don't think that's the kind of complexity you're referring to.
>> 
>> >> Another third use ida_alloc (or xa_alloc) so all that could be
>> >> removed.
>> > 
>> > These ID numbers need to be per-device.
>> 
>> Why? They are arbitrary with no semantic meaning, right?
> 
> Yes, officially there is no meaning, and this is how we would like to
> keep it.
> 
> Right now, they are very correlated with with their respective PCI function number.
> Is it important? No, however it doesn't mean that we should proactively harm user
> experience just because we can do it.
> 
> [leonro@c ~]$ l /sys/bus/auxiliary/devices/
> ,,,
> rwxrwxrwx 1 root root 0 Jul 21 15:25 mlx5_core.rdma.0 -> ../../../devices/pci0000:00/0000:00:02.7/0000:0
> 8:00.0/mlx5_core.rdma.0
> lrwxrwxrwx 1 root root 0 Jul 21 15:25 mlx5_core.rdma.1 -> ../../../devices/pci0000:00/0000:00:02.7/0000:0
> 8:00.1/mlx5_core.rdma

Well, I would certainly like to have semantic meaning for ids. But apparently
that is only allowed if you can sneak it past the review process.

--Sean
Re: [PATCH net v2 1/4] auxiliary: Support hexadecimal ids
Posted by Greg Kroah-Hartman 2 months, 1 week ago
On Thu, Jul 24, 2025 at 09:55:59AM -0400, Sean Anderson wrote:
> On 7/23/25 04:13, Leon Romanovsky wrote:
> > On Mon, Jul 21, 2025 at 10:29:32AM -0400, Sean Anderson wrote:
> >> On 7/20/25 04:17, Leon Romanovsky wrote:
> >> > On Thu, Jul 17, 2025 at 01:12:08PM -0400, Sean Anderson wrote:
> >> >> On 7/17/25 12:33, Greg Kroah-Hartman wrote:
> >> > 
> >> > <...>
> >> > 
> >> >> Anyway, if you really think ids should be random or whatever, why not
> >> >> just ida_alloc one in axiliary_device_init and ignore whatever's
> >> >> provided? I'd say around half the auxiliary drivers just use 0 (or some
> >> >> other constant), which is just as deterministic as using the device
> >> >> address.
> >> > 
> >> > I would say that auxiliary bus is not right fit for such devices. This
> >> > bus was introduced for more complex devices, like the one who has their
> >> > own ida_alloc logic.
> >> 
> >> I'd say that around 2/3 of the auxiliary drivers that have non-constant
> >> ids use ida_alloc solely for the auxiliary bus and for no other purpose.
> >> I don't think that's the kind of complexity you're referring to.
> >> 
> >> >> Another third use ida_alloc (or xa_alloc) so all that could be
> >> >> removed.
> >> > 
> >> > These ID numbers need to be per-device.
> >> 
> >> Why? They are arbitrary with no semantic meaning, right?
> > 
> > Yes, officially there is no meaning, and this is how we would like to
> > keep it.
> > 
> > Right now, they are very correlated with with their respective PCI function number.
> > Is it important? No, however it doesn't mean that we should proactively harm user
> > experience just because we can do it.
> > 
> > [leonro@c ~]$ l /sys/bus/auxiliary/devices/
> > ,,,
> > rwxrwxrwx 1 root root 0 Jul 21 15:25 mlx5_core.rdma.0 -> ../../../devices/pci0000:00/0000:00:02.7/0000:0
> > 8:00.0/mlx5_core.rdma.0
> > lrwxrwxrwx 1 root root 0 Jul 21 15:25 mlx5_core.rdma.1 -> ../../../devices/pci0000:00/0000:00:02.7/0000:0
> > 8:00.1/mlx5_core.rdma
> 
> Well, I would certainly like to have semantic meaning for ids. But apparently
> that is only allowed if you can sneak it past the review process.

Do I need to dust off my "make all ids random" patch again and actually
merge it just to prevent this from happening?

greg k-h
Re: [PATCH net v2 1/4] auxiliary: Support hexadecimal ids
Posted by Leon Romanovsky 2 months, 1 week ago
On Fri, Jul 25, 2025 at 07:02:24AM +0200, Greg Kroah-Hartman wrote:
> On Thu, Jul 24, 2025 at 09:55:59AM -0400, Sean Anderson wrote:
> > On 7/23/25 04:13, Leon Romanovsky wrote:
> > > On Mon, Jul 21, 2025 at 10:29:32AM -0400, Sean Anderson wrote:
> > >> On 7/20/25 04:17, Leon Romanovsky wrote:
> > >> > On Thu, Jul 17, 2025 at 01:12:08PM -0400, Sean Anderson wrote:
> > >> >> On 7/17/25 12:33, Greg Kroah-Hartman wrote:
> > >> > 
> > >> > <...>
> > >> > 
> > >> >> Anyway, if you really think ids should be random or whatever, why not
> > >> >> just ida_alloc one in axiliary_device_init and ignore whatever's
> > >> >> provided? I'd say around half the auxiliary drivers just use 0 (or some
> > >> >> other constant), which is just as deterministic as using the device
> > >> >> address.
> > >> > 
> > >> > I would say that auxiliary bus is not right fit for such devices. This
> > >> > bus was introduced for more complex devices, like the one who has their
> > >> > own ida_alloc logic.
> > >> 
> > >> I'd say that around 2/3 of the auxiliary drivers that have non-constant
> > >> ids use ida_alloc solely for the auxiliary bus and for no other purpose.
> > >> I don't think that's the kind of complexity you're referring to.
> > >> 
> > >> >> Another third use ida_alloc (or xa_alloc) so all that could be
> > >> >> removed.
> > >> > 
> > >> > These ID numbers need to be per-device.
> > >> 
> > >> Why? They are arbitrary with no semantic meaning, right?
> > > 
> > > Yes, officially there is no meaning, and this is how we would like to
> > > keep it.
> > > 
> > > Right now, they are very correlated with with their respective PCI function number.
> > > Is it important? No, however it doesn't mean that we should proactively harm user
> > > experience just because we can do it.
> > > 
> > > [leonro@c ~]$ l /sys/bus/auxiliary/devices/
> > > ,,,
> > > rwxrwxrwx 1 root root 0 Jul 21 15:25 mlx5_core.rdma.0 -> ../../../devices/pci0000:00/0000:00:02.7/0000:0
> > > 8:00.0/mlx5_core.rdma.0
> > > lrwxrwxrwx 1 root root 0 Jul 21 15:25 mlx5_core.rdma.1 -> ../../../devices/pci0000:00/0000:00:02.7/0000:0
> > > 8:00.1/mlx5_core.rdma
> > 
> > Well, I would certainly like to have semantic meaning for ids. But apparently
> > that is only allowed if you can sneak it past the review process.
> 
> Do I need to dust off my "make all ids random" patch again and actually
> merge it just to prevent this from happening?

After weekend thoughts on it. IDs need to be removed from the driver
access. Let's make them global at least.

Thanks

> 
> greg k-h
>
Re: [PATCH net v2 1/4] auxiliary: Support hexadecimal ids
Posted by Greg Kroah-Hartman 2 months, 1 week ago
On Sun, Jul 27, 2025 at 11:07:05AM +0300, Leon Romanovsky wrote:
> On Fri, Jul 25, 2025 at 07:02:24AM +0200, Greg Kroah-Hartman wrote:
> > On Thu, Jul 24, 2025 at 09:55:59AM -0400, Sean Anderson wrote:
> > > On 7/23/25 04:13, Leon Romanovsky wrote:
> > > > On Mon, Jul 21, 2025 at 10:29:32AM -0400, Sean Anderson wrote:
> > > >> On 7/20/25 04:17, Leon Romanovsky wrote:
> > > >> > On Thu, Jul 17, 2025 at 01:12:08PM -0400, Sean Anderson wrote:
> > > >> >> On 7/17/25 12:33, Greg Kroah-Hartman wrote:
> > > >> > 
> > > >> > <...>
> > > >> > 
> > > >> >> Anyway, if you really think ids should be random or whatever, why not
> > > >> >> just ida_alloc one in axiliary_device_init and ignore whatever's
> > > >> >> provided? I'd say around half the auxiliary drivers just use 0 (or some
> > > >> >> other constant), which is just as deterministic as using the device
> > > >> >> address.
> > > >> > 
> > > >> > I would say that auxiliary bus is not right fit for such devices. This
> > > >> > bus was introduced for more complex devices, like the one who has their
> > > >> > own ida_alloc logic.
> > > >> 
> > > >> I'd say that around 2/3 of the auxiliary drivers that have non-constant
> > > >> ids use ida_alloc solely for the auxiliary bus and for no other purpose.
> > > >> I don't think that's the kind of complexity you're referring to.
> > > >> 
> > > >> >> Another third use ida_alloc (or xa_alloc) so all that could be
> > > >> >> removed.
> > > >> > 
> > > >> > These ID numbers need to be per-device.
> > > >> 
> > > >> Why? They are arbitrary with no semantic meaning, right?
> > > > 
> > > > Yes, officially there is no meaning, and this is how we would like to
> > > > keep it.
> > > > 
> > > > Right now, they are very correlated with with their respective PCI function number.
> > > > Is it important? No, however it doesn't mean that we should proactively harm user
> > > > experience just because we can do it.
> > > > 
> > > > [leonro@c ~]$ l /sys/bus/auxiliary/devices/
> > > > ,,,
> > > > rwxrwxrwx 1 root root 0 Jul 21 15:25 mlx5_core.rdma.0 -> ../../../devices/pci0000:00/0000:00:02.7/0000:0
> > > > 8:00.0/mlx5_core.rdma.0
> > > > lrwxrwxrwx 1 root root 0 Jul 21 15:25 mlx5_core.rdma.1 -> ../../../devices/pci0000:00/0000:00:02.7/0000:0
> > > > 8:00.1/mlx5_core.rdma
> > > 
> > > Well, I would certainly like to have semantic meaning for ids. But apparently
> > > that is only allowed if you can sneak it past the review process.
> > 
> > Do I need to dust off my "make all ids random" patch again and actually
> > merge it just to prevent this from happening?
> 
> After weekend thoughts on it. IDs need to be removed from the driver
> access. Let's make them global at least.

Great, no objection from me, want to send a patch we can queue up for
6.18-rc1?
Re: [PATCH net v2 1/4] auxiliary: Support hexadecimal ids
Posted by Leon Romanovsky 2 months, 1 week ago
On Sun, Jul 27, 2025 at 10:57:09AM +0200, Greg Kroah-Hartman wrote:
> On Sun, Jul 27, 2025 at 11:07:05AM +0300, Leon Romanovsky wrote:
> > On Fri, Jul 25, 2025 at 07:02:24AM +0200, Greg Kroah-Hartman wrote:
> > > On Thu, Jul 24, 2025 at 09:55:59AM -0400, Sean Anderson wrote:
> > > > On 7/23/25 04:13, Leon Romanovsky wrote:
> > > > > On Mon, Jul 21, 2025 at 10:29:32AM -0400, Sean Anderson wrote:
> > > > >> On 7/20/25 04:17, Leon Romanovsky wrote:
> > > > >> > On Thu, Jul 17, 2025 at 01:12:08PM -0400, Sean Anderson wrote:
> > > > >> >> On 7/17/25 12:33, Greg Kroah-Hartman wrote:
> > > > >> > 
> > > > >> > <...>
> > > > >> > 
> > > > >> >> Anyway, if you really think ids should be random or whatever, why not
> > > > >> >> just ida_alloc one in axiliary_device_init and ignore whatever's
> > > > >> >> provided? I'd say around half the auxiliary drivers just use 0 (or some
> > > > >> >> other constant), which is just as deterministic as using the device
> > > > >> >> address.
> > > > >> > 
> > > > >> > I would say that auxiliary bus is not right fit for such devices. This
> > > > >> > bus was introduced for more complex devices, like the one who has their
> > > > >> > own ida_alloc logic.
> > > > >> 
> > > > >> I'd say that around 2/3 of the auxiliary drivers that have non-constant
> > > > >> ids use ida_alloc solely for the auxiliary bus and for no other purpose.
> > > > >> I don't think that's the kind of complexity you're referring to.
> > > > >> 
> > > > >> >> Another third use ida_alloc (or xa_alloc) so all that could be
> > > > >> >> removed.
> > > > >> > 
> > > > >> > These ID numbers need to be per-device.
> > > > >> 
> > > > >> Why? They are arbitrary with no semantic meaning, right?
> > > > > 
> > > > > Yes, officially there is no meaning, and this is how we would like to
> > > > > keep it.
> > > > > 
> > > > > Right now, they are very correlated with with their respective PCI function number.
> > > > > Is it important? No, however it doesn't mean that we should proactively harm user
> > > > > experience just because we can do it.
> > > > > 
> > > > > [leonro@c ~]$ l /sys/bus/auxiliary/devices/
> > > > > ,,,
> > > > > rwxrwxrwx 1 root root 0 Jul 21 15:25 mlx5_core.rdma.0 -> ../../../devices/pci0000:00/0000:00:02.7/0000:0
> > > > > 8:00.0/mlx5_core.rdma.0
> > > > > lrwxrwxrwx 1 root root 0 Jul 21 15:25 mlx5_core.rdma.1 -> ../../../devices/pci0000:00/0000:00:02.7/0000:0
> > > > > 8:00.1/mlx5_core.rdma
> > > > 
> > > > Well, I would certainly like to have semantic meaning for ids. But apparently
> > > > that is only allowed if you can sneak it past the review process.
> > > 
> > > Do I need to dust off my "make all ids random" patch again and actually
> > > merge it just to prevent this from happening?
> > 
> > After weekend thoughts on it. IDs need to be removed from the driver
> > access. Let's make them global at least.
> 
> Great, no objection from me, want to send a patch we can queue up for
> 6.18-rc1?

Sean proposed the initial idea to move IDs to aux core. Let's wait for him
to see if he wants to do such patch. If not, I'll send.

Thanks