Fix Regression with AXP20X for 6.13-rc1

From: Chris Morgan <[email protected]>

After performing a git bisect, I identified a commit that broke the battery and charger driver for my AXP717 PMIC. This was caused by commit e37ec3218870 ("mfd: axp20x: Allow multiple regulators").

After digging into it, it appears when mfd_add_devices was called with a platform ID of PLATFORM_DEVID_NONE, the devm_iio_channel_get() call made by the various AXP20X power drivers would not be able to generate a dev_name(dev) for some reason, and the iio_channel_get() call used in the devm_ helper would fall back to making a iio_channel_get_sys() call. After the platform ID was updated, now iio_channel_get() is no longer falling back to iio_channel_get_sys(). At least this is my limited understanding of what happened.

To fix this, I added a new devm_ helper of devm_iio_channel_get_sys() that directly calls iio_channel_get_sys(), and I updated all the affected drivers with the new routine. I then no longer experienced any issues with the drivers on my devices.

Chris Morgan (2):
iio: core: Add devm_ API for iio_channel_get_sys
power: supply: axp20x: Use devm_iio_channel_get_sys() for iio chans

 drivers/iio/inkern.c                    | 18 ++++++++++++++++++
 drivers/power/supply/axp20x_ac_power.c  |  4 ++--
 drivers/power/supply/axp20x_battery.c   | 16 ++++++++--------
 drivers/power/supply/axp20x_usb_power.c |  6 +++---
 include/linux/iio/consumer.h            | 20 ++++++++++++++++++++
 5 files changed, 51 insertions(+), 13 deletions(-)

Quoted 6 lines · On Tue, 10 Dec 2024 16:48:57 -0600 Chris Morgan <[email protected]> wrote:

From: Chris Morgan <[email protected]>

After performing a git bisect, I identified a commit that broke the battery and charger driver for my AXP717 PMIC. This was caused by commit e37ec3218870 ("mfd: axp20x: Allow multiple regulators").

Quoted 8 lines

After digging into it, it appears when mfd_add_devices was called with a platform ID of PLATFORM_DEVID_NONE, the devm_iio_channel_get() call made by the various AXP20X power drivers would not be able to generate a dev_name(dev) for some reason, and the iio_channel_get() call used in the devm_ helper would fall back to making a iio_channel_get_sys() call. After the platform ID was updated, now iio_channel_get() is no longer falling back to iio_channel_get_sys(). At least this is my limited understanding of what happened.

The dev_name(dev) not getting a name doesn't sound quite right to me.

Time to look at the ancient creaking ghost that is the iio_map handling.

struct iio_channel *iio_channel_get(struct device *dev,
const char *channel_name)

{
	const char *name = dev ? dev_name(dev) : NULL;
	struct iio_channel *channel;

	if (dev) {
		channel = fwnode_iio_channel_get_by_name(dev_fwnode(dev),
							 channel_name);
		if (!IS_ERR(channel) || PTR_ERR(channel) != -ENODEV)
			return channel;
	}

	return iio_channel_get_sys(name, channel_name);
}

EXPORT_SYMBOL_GPL(iio_channel_get);

We didn't invent the relevant phandle stuff in DT via the patch you point at so all that matters is what gets passed to that iio_channel_get_sys()

So key here is that dev should be set, so we are passing dev_name(dev) into iio_channel_get_sys()
I'm guessing that changed...

Ah. The iio_maps in
https://elixir.bootlin.com/linux/v6.12.4/source/drivers/iio/adc/axp20x_adc.c#L158
are our problem. Those hardcode the consumer_dev name. The fix just changed those names. Back when this infrastructure was written we were in the world of board files, so everything was hard coded in them - or in an MFD like this it was treated as a singleton device.

So as to how to fix it... Assuming the new device names are the same for all the mfd parts that make up each pmic, then you should be able to figure out the
extra the number and build the channel maps to allow you to find the numbered
devices.

That's a lot lighter change than moving over to DT based phandles for all this. (which is the modern way to handle it).

As a cheeky check, just edit those maps to whatever IDs you have and see if it works. Probably not an upstreamable solution but will confirm we have it correct.

Your patch works because we allow for some fuzzy matching (I can't remember why) that doesn't use the consumer device name.
That works as long as there is only one instance. I'm guessing all this mess came about because someone has a board with two of these devices. On such a board we need the precise matching including the device name.

Jonathan

Quoted 17 lines

To fix this, I added a new devm_ helper of devm_iio_channel_get_sys() that directly calls iio_channel_get_sys(), and I updated all the affected drivers with the new routine. I then no longer experienced any issues with the drivers on my devices.

Chris Morgan (2):
iio: core: Add devm_ API for iio_channel_get_sys
power: supply: axp20x: Use devm_iio_channel_get_sys() for iio chans

 drivers/iio/inkern.c                    | 18 ++++++++++++++++++
 drivers/power/supply/axp20x_ac_power.c  |  4 ++--
 drivers/power/supply/axp20x_battery.c   | 16 ++++++++--------
 drivers/power/supply/axp20x_usb_power.c |  6 +++---
 include/linux/iio/consumer.h            | 20 ++++++++++++++++++++
 5 files changed, 51 insertions(+), 13 deletions(-)

Quoted 6 lines · On Tue, 10 Dec 2024 16:48:57 -0600 Chris Morgan <[email protected]> wrote:

From: Chris Morgan <[email protected]>

After performing a git bisect, I identified a commit that broke the battery and charger driver for my AXP717 PMIC. This was caused by commit e37ec3218870 ("mfd: axp20x: Allow multiple regulators").

Quoted 8 lines

After digging into it, it appears when mfd_add_devices was called with a platform ID of PLATFORM_DEVID_NONE, the devm_iio_channel_get() call made by the various AXP20X power drivers would not be able to generate a dev_name(dev) for some reason, and the iio_channel_get() call used in the devm_ helper would fall back to making a iio_channel_get_sys() call. After the platform ID was updated, now iio_channel_get() is no longer falling back to iio_channel_get_sys(). At least this is my limited understanding of what happened.

The dev_name(dev) not getting a name doesn't sound quite right to me.

Time to look at the ancient creaking ghost that is the iio_map handling.

struct iio_channel *iio_channel_get(struct device *dev,
const char *channel_name)

{
	const char *name = dev ? dev_name(dev) : NULL;
	struct iio_channel *channel;

	if (dev) {
		channel = fwnode_iio_channel_get_by_name(dev_fwnode(dev),
							 channel_name);
		if (!IS_ERR(channel) || PTR_ERR(channel) != -ENODEV)
			return channel;
	}

	return iio_channel_get_sys(name, channel_name);
}

EXPORT_SYMBOL_GPL(iio_channel_get);

We didn't invent the relevant phandle stuff in DT via the patch you point at so all that matters is what gets passed to that iio_channel_get_sys()

So key here is that dev should be set, so we are passing dev_name(dev) into iio_channel_get_sys()
I'm guessing that changed...

Ah. The iio_maps in
https://elixir.bootlin.com/linux/v6.12.4/source/drivers/iio/adc/axp20x_adc.c#L158
are our problem. Those hardcode the consumer_dev name. The fix just changed those names. Back when this infrastructure was written we were in the world of board files, so everything was hard coded in them - or in an MFD like this it was treated as a singleton device.

So as to how to fix it... Assuming the new device names are the same for all the mfd parts that make up each pmic, then you should be able to figure out the
extra the number and build the channel maps to allow you to find the numbered
devices.

That's a lot lighter change than moving over to DT based phandles for all this. (which is the modern way to handle it).

As a cheeky check, just edit those maps to whatever IDs you have and see if it works. Probably not an upstreamable solution but will confirm we have it correct.

Your patch works because we allow for some fuzzy matching (I can't remember why) that doesn't use the consumer device name.
That works as long as there is only one instance. I'm guessing all this mess came about because someone has a board with two of these devices. On such a board we need the precise matching including the device name.

Jonathan

Quoted 17 lines

To fix this, I added a new devm_ helper of devm_iio_channel_get_sys() that directly calls iio_channel_get_sys(), and I updated all the affected drivers with the new routine. I then no longer experienced any issues with the drivers on my devices.

Chris Morgan (2):
iio: core: Add devm_ API for iio_channel_get_sys
power: supply: axp20x: Use devm_iio_channel_get_sys() for iio chans

 drivers/iio/inkern.c                    | 18 ++++++++++++++++++
 drivers/power/supply/axp20x_ac_power.c  |  4 ++--
 drivers/power/supply/axp20x_battery.c   | 16 ++++++++--------
 drivers/power/supply/axp20x_usb_power.c |  6 +++---
 include/linux/iio/consumer.h            | 20 ++++++++++++++++++++
 5 files changed, 51 insertions(+), 13 deletions(-)

Quoted 53 lines · Jonathan Cameron

Quoted 6 lines · On Tue, 10 Dec 2024 16:48:57 -0600 Chris Morgan <[email protected]> wrote:

From: Chris Morgan <[email protected]>

After performing a git bisect, I identified a commit that broke the battery and charger driver for my AXP717 PMIC. This was caused by commit e37ec3218870 ("mfd: axp20x: Allow multiple regulators").

Quoted 8 lines

After digging into it, it appears when mfd_add_devices was called with a platform ID of PLATFORM_DEVID_NONE, the devm_iio_channel_get() call made by the various AXP20X power drivers would not be able to generate a dev_name(dev) for some reason, and the iio_channel_get() call used in the devm_ helper would fall back to making a iio_channel_get_sys() call. After the platform ID was updated, now iio_channel_get() is no longer falling back to iio_channel_get_sys(). At least this is my limited understanding of what happened.

The dev_name(dev) not getting a name doesn't sound quite right to me.

Time to look at the ancient creaking ghost that is the iio_map handling.

struct iio_channel *iio_channel_get(struct device *dev,
const char *channel_name)

{
      const char *name = dev ? dev_name(dev) : NULL;
      struct iio_channel *channel;

      if (dev) {
              channel = fwnode_iio_channel_get_by_name(dev_fwnode(dev),
                                                       channel_name);
              if (!IS_ERR(channel) || PTR_ERR(channel) != -ENODEV)
                      return channel;
      }

      return iio_channel_get_sys(name, channel_name);
}

EXPORT_SYMBOL_GPL(iio_channel_get);

We didn't invent the relevant phandle stuff in DT via the patch you point at so all that matters is what gets passed to that iio_channel_get_sys()

So key here is that dev should be set, so we are passing dev_name(dev) into iio_channel_get_sys()
I'm guessing that changed...

Ah. The iio_maps in
https://elixir.bootlin.com/linux/v6.12.4/source/drivers/iio/adc/axp20x_adc.c#L158
are our problem. Those hardcode the consumer_dev name. The fix just changed those names. Back when this infrastructure was written we were in the world of board files, so everything was hard coded in them - or in an MFD like this it was treated as a singleton device.

Quoted 4 lines

So as to how to fix it... Assuming the new device names are the same for all the mfd parts that make up each pmic, then you should be able to figure out the
extra the number and build the channel maps to allow you to find the numbered
devices.

Is there a way to figure out the device number at runtime? The issue is each time the device attempts to probe and fails, the device number increments, making it a "hitting a moving target" problem.

Thank you,
Chris

Quoted 34 lines

That's a lot lighter change than moving over to DT based phandles for all this. (which is the modern way to handle it).

As a cheeky check, just edit those maps to whatever IDs you have and see if it works. Probably not an upstreamable solution but will confirm we have it correct.

Your patch works because we allow for some fuzzy matching (I can't remember why) that doesn't use the consumer device name.
That works as long as there is only one instance. I'm guessing all this mess came about because someone has a board with two of these devices. On such a board we need the precise matching including the device name.

Jonathan

Quoted 17 lines

To fix this, I added a new devm_ helper of devm_iio_channel_get_sys() that directly calls iio_channel_get_sys(), and I updated all the affected drivers with the new routine. I then no longer experienced any issues with the drivers on my devices.

Chris Morgan (2):
iio: core: Add devm_ API for iio_channel_get_sys
power: supply: axp20x: Use devm_iio_channel_get_sys() for iio chans

 drivers/iio/inkern.c                    | 18 ++++++++++++++++++
 drivers/power/supply/axp20x_ac_power.c  |  4 ++--
 drivers/power/supply/axp20x_battery.c   | 16 ++++++++--------
 drivers/power/supply/axp20x_usb_power.c |  6 +++---
 include/linux/iio/consumer.h            | 20 ++++++++++++++++++++
 5 files changed, 51 insertions(+), 13 deletions(-)

Quoted 60 lines · Chris Morgan

Quoted 57 lines · Jonathan Cameron

Quoted 6 lines · On Tue, 10 Dec 2024 16:48:57 -0600 Chris Morgan <[email protected]> wrote:

From: Chris Morgan <[email protected]>

After performing a git bisect, I identified a commit that broke the battery and charger driver for my AXP717 PMIC. This was caused by commit e37ec3218870 ("mfd: axp20x: Allow multiple regulators").

Quoted 8 lines

After digging into it, it appears when mfd_add_devices was called with a platform ID of PLATFORM_DEVID_NONE, the devm_iio_channel_get() call made by the various AXP20X power drivers would not be able to generate a dev_name(dev) for some reason, and the iio_channel_get() call used in the devm_ helper would fall back to making a iio_channel_get_sys() call. After the platform ID was updated, now iio_channel_get() is no longer falling back to iio_channel_get_sys(). At least this is my limited understanding of what happened.

The dev_name(dev) not getting a name doesn't sound quite right to me.

Time to look at the ancient creaking ghost that is the iio_map handling.

struct iio_channel *iio_channel_get(struct device *dev,
const char *channel_name)

{
      const char *name = dev ? dev_name(dev) : NULL;
      struct iio_channel *channel;

      if (dev) {
              channel = fwnode_iio_channel_get_by_name(dev_fwnode(dev),
                                                       channel_name);
              if (!IS_ERR(channel) || PTR_ERR(channel) != -ENODEV)
                      return channel;
      }

      return iio_channel_get_sys(name, channel_name);
}

EXPORT_SYMBOL_GPL(iio_channel_get);

We didn't invent the relevant phandle stuff in DT via the patch you point at so all that matters is what gets passed to that iio_channel_get_sys()

So key here is that dev should be set, so we are passing dev_name(dev) into iio_channel_get_sys()
I'm guessing that changed...

Ah. The iio_maps in
https://elixir.bootlin.com/linux/v6.12.4/source/drivers/iio/adc/axp20x_adc.c#L158
are our problem. Those hardcode the consumer_dev name. The fix just changed those names. Back when this infrastructure was written we were in the world of board files, so everything was hard coded in them - or in an MFD like this it was treated as a singleton device.

So as to how to fix it... Assuming the new device names are the same for all the mfd parts that make up each pmic, then you should be able to figure out the
extra the number and build the channel maps to allow you to find the numbered
devices.

Quoted 3 lines

Is there a way to figure out the device number at runtime? The issue is each time the device attempts to probe and fails, the device number increments, making it a "hitting a moving target" problem.

The ADC device is a mfd cell or child device of the PMIC mfd device. So you should be able to use dev->parent to get it directly? We do that at least for the regulator driver.

ChenYu

Quoted 37 lines

Thank you,
Chris

Quoted 34 lines

That's a lot lighter change than moving over to DT based phandles for all this. (which is the modern way to handle it).

As a cheeky check, just edit those maps to whatever IDs you have and see if it works. Probably not an upstreamable solution but will confirm we have it correct.

Your patch works because we allow for some fuzzy matching (I can't remember why) that doesn't use the consumer device name.
That works as long as there is only one instance. I'm guessing all this mess came about because someone has a board with two of these devices. On such a board we need the precise matching including the device name.

Jonathan

Quoted 17 lines

To fix this, I added a new devm_ helper of devm_iio_channel_get_sys() that directly calls iio_channel_get_sys(), and I updated all the affected drivers with the new routine. I then no longer experienced any issues with the drivers on my devices.

Chris Morgan (2):
iio: core: Add devm_ API for iio_channel_get_sys
power: supply: axp20x: Use devm_iio_channel_get_sys() for iio chans

 drivers/iio/inkern.c                    | 18 ++++++++++++++++++
 drivers/power/supply/axp20x_ac_power.c  |  4 ++--
 drivers/power/supply/axp20x_battery.c   | 16 ++++++++--------
 drivers/power/supply/axp20x_usb_power.c |  6 +++---
 include/linux/iio/consumer.h            | 20 ++++++++++++++++++++
 5 files changed, 51 insertions(+), 13 deletions(-)

Quoted 61 lines · Chen-Yu Tsai

Quoted 60 lines · Chris Morgan

Quoted 57 lines · Jonathan Cameron

Quoted 6 lines · On Tue, 10 Dec 2024 16:48:57 -0600 Chris Morgan <[email protected]> wrote:

From: Chris Morgan <[email protected]>

After performing a git bisect, I identified a commit that broke the battery and charger driver for my AXP717 PMIC. This was caused by commit e37ec3218870 ("mfd: axp20x: Allow multiple regulators").

Quoted 8 lines

After digging into it, it appears when mfd_add_devices was called with a platform ID of PLATFORM_DEVID_NONE, the devm_iio_channel_get() call made by the various AXP20X power drivers would not be able to generate a dev_name(dev) for some reason, and the iio_channel_get() call used in the devm_ helper would fall back to making a iio_channel_get_sys() call. After the platform ID was updated, now iio_channel_get() is no longer falling back to iio_channel_get_sys(). At least this is my limited understanding of what happened.

The dev_name(dev) not getting a name doesn't sound quite right to me.

Time to look at the ancient creaking ghost that is the iio_map handling.

struct iio_channel *iio_channel_get(struct device *dev,
const char *channel_name)

{
      const char *name = dev ? dev_name(dev) : NULL;
      struct iio_channel *channel;

      if (dev) {
              channel = fwnode_iio_channel_get_by_name(dev_fwnode(dev),
                                                       channel_name);
              if (!IS_ERR(channel) || PTR_ERR(channel) != -ENODEV)
                      return channel;
      }

      return iio_channel_get_sys(name, channel_name);
}

EXPORT_SYMBOL_GPL(iio_channel_get);

We didn't invent the relevant phandle stuff in DT via the patch you point at so all that matters is what gets passed to that iio_channel_get_sys()

So key here is that dev should be set, so we are passing dev_name(dev) into iio_channel_get_sys()
I'm guessing that changed...

Ah. The iio_maps in
https://elixir.bootlin.com/linux/v6.12.4/source/drivers/iio/adc/axp20x_adc.c#L158
are our problem. Those hardcode the consumer_dev name. The fix just changed those names. Back when this infrastructure was written we were in the world of board files, so everything was hard coded in them - or in an MFD like this it was treated as a singleton device.

So as to how to fix it... Assuming the new device names are the same for all the mfd parts that make up each pmic, then you should be able to figure out the
extra the number and build the channel maps to allow you to find the numbered
devices.

Quoted 7 lines

Quoted 3 lines

Is there a way to figure out the device number at runtime? The issue is each time the device attempts to probe and fails, the device number increments, making it a "hitting a moving target" problem.

The ADC device is a mfd cell or child device of the PMIC mfd device. So you should be able to use dev->parent to get it directly? We do that at least for the regulator driver.

Sorry to dig up such an old thread. I'm taking a look at this one again and can confirm passing pdev->dev.parent to devm_iio_channel_get() is insufficient to get the driver to find the correct ADC channel. Would there be another/better way to do this other than the devm_iio_channel_get_sys() as proposed? Or should we be walking the parent somehow looking for the named ADC channel on each child device (which would also require a new symbol most likely)?

Thank you,
Chris

Quoted 40 lines

ChenYu

Quoted 37 lines

Thank you,
Chris

Quoted 34 lines

That's a lot lighter change than moving over to DT based phandles for all this. (which is the modern way to handle it).

As a cheeky check, just edit those maps to whatever IDs you have and see if it works. Probably not an upstreamable solution but will confirm we have it correct.

Your patch works because we allow for some fuzzy matching (I can't remember why) that doesn't use the consumer device name.
That works as long as there is only one instance. I'm guessing all this mess came about because someone has a board with two of these devices. On such a board we need the precise matching including the device name.

Jonathan

Quoted 17 lines

To fix this, I added a new devm_ helper of devm_iio_channel_get_sys() that directly calls iio_channel_get_sys(), and I updated all the affected drivers with the new routine. I then no longer experienced any issues with the drivers on my devices.

Chris Morgan (2):
iio: core: Add devm_ API for iio_channel_get_sys
power: supply: axp20x: Use devm_iio_channel_get_sys() for iio chans

 drivers/iio/inkern.c                    | 18 ++++++++++++++++++
 drivers/power/supply/axp20x_ac_power.c  |  4 ++--
 drivers/power/supply/axp20x_battery.c   | 16 ++++++++--------
 drivers/power/supply/axp20x_usb_power.c |  6 +++---
 include/linux/iio/consumer.h            | 20 ++++++++++++++++++++
 5 files changed, 51 insertions(+), 13 deletions(-)

Quoted 70 lines · On Tue, 4 Mar 2025 12:18:10 -0600 Chris Morgan <[email protected]> wrote:

Quoted 61 lines · Chen-Yu Tsai

Quoted 60 lines · Chris Morgan

Quoted 57 lines · Jonathan Cameron

Quoted 6 lines · On Tue, 10 Dec 2024 16:48:57 -0600 Chris Morgan <[email protected]> wrote:

From: Chris Morgan <[email protected]>

After performing a git bisect, I identified a commit that broke the battery and charger driver for my AXP717 PMIC. This was caused by commit e37ec3218870 ("mfd: axp20x: Allow multiple regulators").

Quoted 8 lines

After digging into it, it appears when mfd_add_devices was called with a platform ID of PLATFORM_DEVID_NONE, the devm_iio_channel_get() call made by the various AXP20X power drivers would not be able to generate a dev_name(dev) for some reason, and the iio_channel_get() call used in the devm_ helper would fall back to making a iio_channel_get_sys() call. After the platform ID was updated, now iio_channel_get() is no longer falling back to iio_channel_get_sys(). At least this is my limited understanding of what happened.

The dev_name(dev) not getting a name doesn't sound quite right to me.

Time to look at the ancient creaking ghost that is the iio_map handling.

struct iio_channel *iio_channel_get(struct device *dev,
const char *channel_name)

{
      const char *name = dev ? dev_name(dev) : NULL;
      struct iio_channel *channel;

      if (dev) {
              channel = fwnode_iio_channel_get_by_name(dev_fwnode(dev),
                                                       channel_name);
              if (!IS_ERR(channel) || PTR_ERR(channel) != -ENODEV)
                      return channel;
      }

      return iio_channel_get_sys(name, channel_name);
}

EXPORT_SYMBOL_GPL(iio_channel_get);

We didn't invent the relevant phandle stuff in DT via the patch you point at so all that matters is what gets passed to that iio_channel_get_sys()

So key here is that dev should be set, so we are passing dev_name(dev) into iio_channel_get_sys()
I'm guessing that changed...

Ah. The iio_maps in
https://elixir.bootlin.com/linux/v6.12.4/source/drivers/iio/adc/axp20x_adc.c#L158
are our problem. Those hardcode the consumer_dev name. The fix just changed those names. Back when this infrastructure was written we were in the world of board files, so everything was hard coded in them - or in an MFD like this it was treated as a singleton device.

So as to how to fix it... Assuming the new device names are the same for all the mfd parts that make up each pmic, then you should be able to figure out the
extra the number and build the channel maps to allow you to find the numbered
devices.

Quoted 7 lines

Quoted 3 lines

Is there a way to figure out the device number at runtime? The issue is each time the device attempts to probe and fails, the device number increments, making it a "hitting a moving target" problem.

The ADC device is a mfd cell or child device of the PMIC mfd device. So you should be able to use dev->parent to get it directly? We do that at least for the regulator driver.

Quoted 7 lines

Sorry to dig up such an old thread. I'm taking a look at this one again and can confirm passing pdev->dev.parent to devm_iio_channel_get() is insufficient to get the driver to find the correct ADC channel. Would there be another/better way to do this other than the devm_iio_channel_get_sys() as proposed? Or should we be walking the parent somehow looking for the named ADC channel on each child device (which would also require a new symbol most likely)?

Hi Chris

From what I recall the point was to get the number from pdev->dev.parent rather than simply passing it to the devm_iio_channel_get() call.

That should allow building of channel maps that provide the correct device name to allow the provider to be found.

Jonathan

Quoted 46 lines

Thank you,
Chris

Quoted 40 lines

ChenYu

Quoted 37 lines

Thank you,
Chris

Quoted 34 lines

That's a lot lighter change than moving over to DT based phandles for all this. (which is the modern way to handle it).

As a cheeky check, just edit those maps to whatever IDs you have and see if it works. Probably not an upstreamable solution but will confirm we have it correct.

Your patch works because we allow for some fuzzy matching (I can't remember why) that doesn't use the consumer device name.
That works as long as there is only one instance. I'm guessing all this mess came about because someone has a board with two of these devices. On such a board we need the precise matching including the device name.

Jonathan

Quoted 17 lines

To fix this, I added a new devm_ helper of devm_iio_channel_get_sys() that directly calls iio_channel_get_sys(), and I updated all the affected drivers with the new routine. I then no longer experienced any issues with the drivers on my devices.

Chris Morgan (2):
iio: core: Add devm_ API for iio_channel_get_sys
power: supply: axp20x: Use devm_iio_channel_get_sys() for iio chans

 drivers/iio/inkern.c                    | 18 ++++++++++++++++++
 drivers/power/supply/axp20x_ac_power.c  |  4 ++--
 drivers/power/supply/axp20x_battery.c   | 16 ++++++++--------
 drivers/power/supply/axp20x_usb_power.c |  6 +++---
 include/linux/iio/consumer.h            | 20 ++++++++++++++++++++
 5 files changed, 51 insertions(+), 13 deletions(-)

Quoted 83 lines · Jonathan Cameron

Quoted 70 lines · On Tue, 4 Mar 2025 12:18:10 -0600 Chris Morgan <[email protected]> wrote:

Quoted 61 lines · Chen-Yu Tsai

Quoted 60 lines · Chris Morgan

Quoted 57 lines · Jonathan Cameron

Quoted 6 lines · On Tue, 10 Dec 2024 16:48:57 -0600 Chris Morgan <[email protected]> wrote:

From: Chris Morgan <[email protected]>

After performing a git bisect, I identified a commit that broke the battery and charger driver for my AXP717 PMIC. This was caused by commit e37ec3218870 ("mfd: axp20x: Allow multiple regulators").

Quoted 8 lines

After digging into it, it appears when mfd_add_devices was called with a platform ID of PLATFORM_DEVID_NONE, the devm_iio_channel_get() call made by the various AXP20X power drivers would not be able to generate a dev_name(dev) for some reason, and the iio_channel_get() call used in the devm_ helper would fall back to making a iio_channel_get_sys() call. After the platform ID was updated, now iio_channel_get() is no longer falling back to iio_channel_get_sys(). At least this is my limited understanding of what happened.

The dev_name(dev) not getting a name doesn't sound quite right to me.

Time to look at the ancient creaking ghost that is the iio_map handling.

struct iio_channel *iio_channel_get(struct device *dev,
const char *channel_name)

{
      const char *name = dev ? dev_name(dev) : NULL;
      struct iio_channel *channel;

      if (dev) {
              channel = fwnode_iio_channel_get_by_name(dev_fwnode(dev),
                                                       channel_name);
              if (!IS_ERR(channel) || PTR_ERR(channel) != -ENODEV)
                      return channel;
      }

      return iio_channel_get_sys(name, channel_name);
}

EXPORT_SYMBOL_GPL(iio_channel_get);

We didn't invent the relevant phandle stuff in DT via the patch you point at so all that matters is what gets passed to that iio_channel_get_sys()

So key here is that dev should be set, so we are passing dev_name(dev) into iio_channel_get_sys()
I'm guessing that changed...

Ah. The iio_maps in
https://elixir.bootlin.com/linux/v6.12.4/source/drivers/iio/adc/axp20x_adc.c#L158
are our problem. Those hardcode the consumer_dev name. The fix just changed those names. Back when this infrastructure was written we were in the world of board files, so everything was hard coded in them - or in an MFD like this it was treated as a singleton device.

So as to how to fix it... Assuming the new device names are the same for all the mfd parts that make up each pmic, then you should be able to figure out the
extra the number and build the channel maps to allow you to find the numbered
devices.

Quoted 7 lines

Quoted 3 lines

Is there a way to figure out the device number at runtime? The issue is each time the device attempts to probe and fails, the device number increments, making it a "hitting a moving target" problem.

The ADC device is a mfd cell or child device of the PMIC mfd device. So you should be able to use dev->parent to get it directly? We do that at least for the regulator driver.

Quoted 7 lines

Sorry to dig up such an old thread. I'm taking a look at this one again and can confirm passing pdev->dev.parent to devm_iio_channel_get() is insufficient to get the driver to find the correct ADC channel. Would there be another/better way to do this other than the devm_iio_channel_get_sys() as proposed? Or should we be walking the parent somehow looking for the named ADC channel on each child device (which would also require a new symbol most likely)?

Hi Chris

Quoted 7 lines

From what I recall the point was to get the number from pdev->dev.parent rather than simply passing it to the devm_iio_channel_get() call.

That should allow building of channel maps that provide the correct device name to allow the provider to be found.

Jonathan

I don't suppose you have a few more tips, do you? I'm still kind of lost. I was able to crawl the parent node until I gathered the correct device struct pointer of the adc device, but even passing it to the devm_iio_channel_get() still doesn't work. I'm thinking that maybe it's because devm_iio_channel_get() then calls iio_channel_get() which then calls fwnode_iio_channel_get_by_name() which then calls __fwnode_iio_channel_get_by_name() which fails because there is no "io-channel-names" property in the device tree (because the driver hard codes the channel names). Now a call to iio_channel_get_sys() would work for me in this case, but I'm starting to wonder how it might work if we had more than one PMIC (which is the whole point of what the original fix and revert was trying to solve) so we might have to figure out a more elegant solution..

Thank you,
Chris

Quoted 47 lines

Quoted 46 lines

Thank you,
Chris

Quoted 40 lines

ChenYu

Quoted 37 lines

Thank you,
Chris

Quoted 34 lines

That's a lot lighter change than moving over to DT based phandles for all this. (which is the modern way to handle it).

As a cheeky check, just edit those maps to whatever IDs you have and see if it works. Probably not an upstreamable solution but will confirm we have it correct.

Your patch works because we allow for some fuzzy matching (I can't remember why) that doesn't use the consumer device name.
That works as long as there is only one instance. I'm guessing all this mess came about because someone has a board with two of these devices. On such a board we need the precise matching including the device name.

Jonathan

Quoted 17 lines

To fix this, I added a new devm_ helper of devm_iio_channel_get_sys() that directly calls iio_channel_get_sys(), and I updated all the affected drivers with the new routine. I then no longer experienced any issues with the drivers on my devices.

Chris Morgan (2):
iio: core: Add devm_ API for iio_channel_get_sys
power: supply: axp20x: Use devm_iio_channel_get_sys() for iio chans

 drivers/iio/inkern.c                    | 18 ++++++++++++++++++
 drivers/power/supply/axp20x_ac_power.c  |  4 ++--
 drivers/power/supply/axp20x_battery.c   | 16 ++++++++--------
 drivers/power/supply/axp20x_usb_power.c |  6 +++---
 include/linux/iio/consumer.h            | 20 ++++++++++++++++++++
 5 files changed, 51 insertions(+), 13 deletions(-)