[PATCH v1 0/2] Use ACPI_COMPANION macro to obtain acpi_device in cs35l41_hda

[PATCH v1 0/2] Use ACPI_COMPANION macro to obtain acpi_device in cs35l41_hda

[Stefan Binding]

Currently, in cs35l41_hda driver, we use acpi_dev_get_first_match_dev to obtain
the acpi_device used to obtain the properties and gpios.

It is better to use the ACPI_COMPANION macro to do this, since it guarentees
that we get the correct acpi_device for the device.

However, the cs35l41_hda driver uses the serial-multi-instantiate driver to
enumerate, and whilst the ACPI_CONPANION macro works with spi, it does not work
with i2c. This is fixed by setting the fwnode for i2c.

Stefan Binding (2):
  platform/x86: serial-multi-instantiate: Set fwnode for i2c
  ALSA: hda: cs35l41: Use ACPI_COMPANION to read acpi properties

 .../platform/x86/serial-multi-instantiate.c   |  1 +
 sound/pci/hda/cs35l41_hda.c                   | 50 ++++++++-----------
 2 files changed, 22 insertions(+), 29 deletions(-)

-- 
2.34.1

[PATCH v1 1/2] platform/x86: serial-multi-instantiate: Set fwnode for i2c

[Stefan Binding]

This allows the i2c driver to obtain the ACPI_COMPANION.

Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 drivers/platform/x86/serial-multi-instantiate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
index 5362f1a7b77c..15ef2f3c442e 100644
--- a/drivers/platform/x86/serial-multi-instantiate.c
+++ b/drivers/platform/x86/serial-multi-instantiate.c
@@ -194,6 +194,7 @@ static int smi_i2c_probe(struct platform_device *pdev, struct smi *smi,
 		strscpy(board_info.type, inst_array[i].type, I2C_NAME_SIZE);
 		snprintf(name, sizeof(name), "%s-%s.%d", dev_name(dev), inst_array[i].type, i);
 		board_info.dev_name = name;
+		board_info.fwnode = acpi_fwnode_handle(adev);
 
 		ret = smi_get_irq(pdev, adev, &inst_array[i]);
 		if (ret < 0)
-- 
2.34.1

Re: [PATCH v1 1/2] platform/x86: serial-multi-instantiate: Set fwnode for i2c

[Andy Shevchenko]

On Thu, Nov 24, 2022 at 1:07 PM Stefan Binding
<sbinding@opensource.cirrus.com> wrote:
>
> This allows the i2c driver to obtain the ACPI_COMPANION.

As far as I get how it's done in the SPI case the real fix should lie
among i2c_acpi_new_device_by_fwnode(), right?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/2] platform/x86: serial-multi-instantiate: Set fwnode for i2c

[Hans de Goede]

Hi,

On 11/24/22 12:35, Andy Shevchenko wrote:
> On Thu, Nov 24, 2022 at 1:07 PM Stefan Binding
> <sbinding@opensource.cirrus.com> wrote:
>>
>> This allows the i2c driver to obtain the ACPI_COMPANION.
> 
> As far as I get how it's done in the SPI case the real fix should lie
> among i2c_acpi_new_device_by_fwnode(), right?

Eventually maybe, but not for the initial change.

It is complicated, making this change has side-effects
and we want to limit those side-effects to only i2c-clients
instantiated from serial-multi-instantiate for now, see
my other reply to this patch.

I do believe that we eventually want to make this change,
to easily give drivers access to all sorts of info
(e.g. _DSM methods) from the matching ACPI fw-node,
but as I said it is complicated...

Regards,

Hans

Re: [PATCH v1 1/2] platform/x86: serial-multi-instantiate: Set fwnode for i2c

[Hans de Goede]

Hi Stefan,

On 11/24/22 12:07, Stefan Binding wrote:
> This allows the i2c driver to obtain the ACPI_COMPANION.
> 
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
> ---
>  drivers/platform/x86/serial-multi-instantiate.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
> index 5362f1a7b77c..15ef2f3c442e 100644
> --- a/drivers/platform/x86/serial-multi-instantiate.c
> +++ b/drivers/platform/x86/serial-multi-instantiate.c
> @@ -194,6 +194,7 @@ static int smi_i2c_probe(struct platform_device *pdev, struct smi *smi,
>  		strscpy(board_info.type, inst_array[i].type, I2C_NAME_SIZE);
>  		snprintf(name, sizeof(name), "%s-%s.%d", dev_name(dev), inst_array[i].type, i);
>  		board_info.dev_name = name;
> +		board_info.fwnode = acpi_fwnode_handle(adev);
>  
>  		ret = smi_get_irq(pdev, adev, &inst_array[i]);
>  		if (ret < 0)

I'm afraid that making this change is not as straight forward as it looks.

I know that I have tried to do this in the past and it failed.

IIRC there were 3 problems:

1. I was expecting this to also allow the driver for the instantiated
i2c-client to be able to bind using an acpi_match_table but that
unfortunately does not work. acpi_match_table matches only work for
the first physical_node linked under
/sys/bus/acpi/devices/xxxx:xx/physical_node and that is the platform
device to which serial-multi-instantiate.c binds. The i2c_client becomes
the second physical node.  Note this is not really an issue,
just something to be aware of.


2. This causes the i2c-core to use the first IRQ resource in the ACPI
fwnode as client->irq for any clients for which we do not set an
IRQ when instantiating. Which may very well be wrong. Sometimes that
IRQ is only valid for the first i2c-client which we instantiate; and
not for the others! And sometimes it is a problem because it may
point to an irqchip for which we never wrote a driver leading to
all probes of the i2c-client failing with -EPROBE_DEFER, see:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d1d84bb95364ed604015c2b788caaf3dbca0262f

Note that patch has been reverted since that specific -EPROBE_DEFER
issue has been solved by making the ACPI core instantiate a
platform_device instead of an i2c_client (in this case we
did not need the actual i2c_client at all).

The current i2c-core code has a (!client-irq) test guarding its
code of trying to use the first ACPI fwnode IRQ resource.

So we could disable this by setting client->irq = -ENOENT in
serial-multi-instantiate.c when (inst->flags & IRQ_RESOURCE_TYPE) ==
IRQ_RESOURCE_NONE). But that will introduce a new problem. Many
i2c-drivers check if there is an IRQ for them to use by doing:
"if (client->irq) request_irq(client->irq, ...)" but then with
error checking/so setting client->irq to -ENOENT will cause
the request_irq to fail, leading the probe to fail.

So before you can write a patch setting client->irq = -ENOENT
when (inst->flags & IRQ_RESOURCE_TYPE) == IRQ_RESOURCE_NONE),
you would first need to patch all i2c-drivers for clients
instantiated through serial-multi-instantiate.c changing:

	if (client->irq) {
		...
	}

to:

	if (client->irq > 0) {
		...
	}

Note this is not as bad as it sounds, since there are only
a few drivers for clients instantiated by serial-multi-instantiate.c .


3. Some drivers may check for an ACPI companion device and then
change their behavior. So all drivers for clients instantiated
through serial-multi-instantiate.c will need to be audited for
this (and a summary of this audit needs to be added to the commit
msg).

Regards,

Hans

Re: [PATCH v1 1/2] platform/x86: serial-multi-instantiate: Set fwnode for i2c

[Hans de Goede]

Hi,

On 11/24/22 12:47, Hans de Goede wrote:
> Hi Stefan,
> 
> On 11/24/22 12:07, Stefan Binding wrote:
>> This allows the i2c driver to obtain the ACPI_COMPANION.
>>
>> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
>> ---
>>  drivers/platform/x86/serial-multi-instantiate.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
>> index 5362f1a7b77c..15ef2f3c442e 100644
>> --- a/drivers/platform/x86/serial-multi-instantiate.c
>> +++ b/drivers/platform/x86/serial-multi-instantiate.c
>> @@ -194,6 +194,7 @@ static int smi_i2c_probe(struct platform_device *pdev, struct smi *smi,
>>  		strscpy(board_info.type, inst_array[i].type, I2C_NAME_SIZE);
>>  		snprintf(name, sizeof(name), "%s-%s.%d", dev_name(dev), inst_array[i].type, i);
>>  		board_info.dev_name = name;
>> +		board_info.fwnode = acpi_fwnode_handle(adev);
>>  
>>  		ret = smi_get_irq(pdev, adev, &inst_array[i]);
>>  		if (ret < 0)
> 
> I'm afraid that making this change is not as straight forward as it looks.
> 
> I know that I have tried to do this in the past and it failed.
> 
> IIRC there were 3 problems:
> 
> 1. I was expecting this to also allow the driver for the instantiated
> i2c-client to be able to bind using an acpi_match_table but that
> unfortunately does not work. acpi_match_table matches only work for
> the first physical_node linked under
> /sys/bus/acpi/devices/xxxx:xx/physical_node and that is the platform
> device to which serial-multi-instantiate.c binds. The i2c_client becomes
> the second physical node.  Note this is not really an issue,
> just something to be aware of.
> 
> 
> 2. This causes the i2c-core to use the first IRQ resource in the ACPI
> fwnode as client->irq for any clients for which we do not set an
> IRQ when instantiating. Which may very well be wrong. Sometimes that
> IRQ is only valid for the first i2c-client which we instantiate; and
> not for the others! And sometimes it is a problem because it may
> point to an irqchip for which we never wrote a driver leading to
> all probes of the i2c-client failing with -EPROBE_DEFER, see:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d1d84bb95364ed604015c2b788caaf3dbca0262f
> 
> Note that patch has been reverted since that specific -EPROBE_DEFER
> issue has been solved by making the ACPI core instantiate a
> platform_device instead of an i2c_client (in this case we
> did not need the actual i2c_client at all).
> 
> The current i2c-core code has a (!client-irq) test guarding its
> code of trying to use the first ACPI fwnode IRQ resource.
> 
> So we could disable this by setting client->irq = -ENOENT in
> serial-multi-instantiate.c when (inst->flags & IRQ_RESOURCE_TYPE) ==
> IRQ_RESOURCE_NONE). But that will introduce a new problem. Many
> i2c-drivers check if there is an IRQ for them to use by doing:
> "if (client->irq) request_irq(client->irq, ...)" but then with
> error checking/so setting client->irq to -ENOENT will cause
> the request_irq to fail, leading the probe to fail.
> 
> So before you can write a patch setting client->irq = -ENOENT
> when (inst->flags & IRQ_RESOURCE_TYPE) == IRQ_RESOURCE_NONE),
> you would first need to patch all i2c-drivers for clients
> instantiated through serial-multi-instantiate.c changing:
> 
> 	if (client->irq) {
> 		...
> 	}
> 
> to:
> 
> 	if (client->irq > 0) {
> 		...
> 	}
> 
> Note this is not as bad as it sounds, since there are only
> a few drivers for clients instantiated by serial-multi-instantiate.c .

Possibly a  nicer way to fix this would be to make the i2c-core change
client->irq to 0 if it is -ENOENT before calling the i2c_driver's
probe method, thus fixing things centrally for all i2c-drivers without
needing to audit/patch them all. Specifically in:

drivers/i2c/i2c-core-base.c: i2c_device_probe() change:

	if (!client->irq) {
		...
	}

to:

	if (!client->irq) {
		...
	} else if (client->irq == -ENOENT) {
		client->irq = 0; /* Drivers expect 0 for "no-IRQ" */
	}

And maybe as Andy suggested, handle at least the IRQ in
i2c_acpi_new_device_by_fwnode() by adding something like that there:

	/* Disable the i2c-core attempting to get an IRQ from ACPI itself */
	if (!board_info->irq)
		board_info->irq= -ENOENT;

I also agree with Andy that setting board_info->fw_node would be done
there ideally too. But then you would need to extend the audit of
impacted drivers mentioned below to also include drivers for
i2c-clients instantiated through other code-paths calling
i2c_acpi_new_device_by_fwnode()  (of which there are not many,
but there are a few others).

> 3. Some drivers may check for an ACPI companion device and then
> change their behavior. So all drivers for clients instantiated
> through serial-multi-instantiate.c will need to be audited for
> this (and a summary of this audit needs to be added to the commit
> msg).

Regards,

Hans

[PATCH v1 2/2] ALSA: hda: cs35l41: Use ACPI_COMPANION to read acpi properties

[Stefan Binding]

Currently the driver finds the acpi_device used to read certain
properties using the HID, however, this is not necessary, as the
acpi_device can be obtained from the device itself.

With the ACPI_COMPANION correctly set, we can also simplify how
we obtain the reset gpio.

Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 sound/pci/hda/cs35l41_hda.c | 50 ++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 29 deletions(-)

diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
index e5f0549bf06d..50cbbcce4946 100644
--- a/sound/pci/hda/cs35l41_hda.c
+++ b/sound/pci/hda/cs35l41_hda.c
@@ -1214,16 +1214,15 @@ static int cs35l41_get_speaker_id(struct device *dev, int amp_index,
  * And devm functions expect that the device requesting the resource has the correct
  * fwnode.
  */
-static int cs35l41_no_acpi_dsd(struct cs35l41_hda *cs35l41, struct device *physdev, int id,
-			       const char *hid)
+static int cs35l41_no_acpi_dsd(struct cs35l41_hda *cs35l41, int id, const char *hid)
 {
 	struct cs35l41_hw_cfg *hw_cfg = &cs35l41->hw_cfg;
 
 	/* check I2C address to assign the index */
 	cs35l41->index = id == 0x40 ? 0 : 1;
 	cs35l41->channel_index = 0;
-	cs35l41->reset_gpio = gpiod_get_index(physdev, NULL, 0, GPIOD_OUT_HIGH);
-	cs35l41->speaker_id = cs35l41_get_speaker_id(physdev, 0, 0, 2);
+	cs35l41->reset_gpio = gpiod_get_index(cs35l41->dev, NULL, 0, GPIOD_OUT_HIGH);
+	cs35l41->speaker_id = cs35l41_get_speaker_id(cs35l41->dev, 0, 0, 2);
 	hw_cfg->spk_pos = cs35l41->index;
 	hw_cfg->gpio2.func = CS35L41_INTERRUPT;
 	hw_cfg->gpio2.valid = true;
@@ -1255,39 +1254,36 @@ static int cs35l41_hda_read_acpi(struct cs35l41_hda *cs35l41, const char *hid, i
 	struct cs35l41_hw_cfg *hw_cfg = &cs35l41->hw_cfg;
 	u32 values[HDA_MAX_COMPONENTS];
 	struct acpi_device *adev;
-	struct device *physdev;
+
 	const char *sub;
 	char *property;
 	size_t nval;
 	int i, ret;
 
-	adev = acpi_dev_get_first_match_dev(hid, NULL, -1);
+	adev = ACPI_COMPANION(cs35l41->dev);
 	if (!adev) {
-		dev_err(cs35l41->dev, "Failed to find an ACPI device for %s\n", hid);
+		dev_err(cs35l41->dev, "Failed to find an ACPI device for %s\n",
+			dev_name(cs35l41->dev));
 		return -ENODEV;
 	}
 
-	physdev = get_device(acpi_get_first_physical_node(adev));
-	acpi_dev_put(adev);
-
-	sub = acpi_get_subsystem_id(ACPI_HANDLE(physdev));
+	sub = acpi_get_subsystem_id(ACPI_HANDLE(cs35l41->dev));
 	if (IS_ERR(sub))
 		sub = NULL;
 	cs35l41->acpi_subsystem_id = sub;
 
 	property = "cirrus,dev-index";
-	ret = device_property_count_u32(physdev, property);
-	if (ret <= 0) {
-		ret = cs35l41_no_acpi_dsd(cs35l41, physdev, id, hid);
-		goto err_put_physdev;
-	}
+	ret = device_property_count_u32(cs35l41->dev, property);
+	if (ret <= 0)
+		return cs35l41_no_acpi_dsd(cs35l41, id, hid);
+
 	if (ret > ARRAY_SIZE(values)) {
 		ret = -EINVAL;
 		goto err;
 	}
 	nval = ret;
 
-	ret = device_property_read_u32_array(physdev, property, values, nval);
+	ret = device_property_read_u32_array(cs35l41->dev, property, values, nval);
 	if (ret)
 		goto err;
 
@@ -1307,11 +1303,10 @@ static int cs35l41_hda_read_acpi(struct cs35l41_hda *cs35l41, const char *hid, i
 	/* To use the same release code for all laptop variants we can't use devm_ version of
 	 * gpiod_get here, as CLSA010* don't have a fully functional bios with an _DSD node
 	 */
-	cs35l41->reset_gpio = fwnode_gpiod_get_index(acpi_fwnode_handle(adev), "reset", cs35l41->index,
-						     GPIOD_OUT_LOW, "cs35l41-reset");
+	cs35l41->reset_gpio = gpiod_get_index(cs35l41->dev, "reset", cs35l41->index, GPIOD_OUT_LOW);
 
 	property = "cirrus,speaker-position";
-	ret = device_property_read_u32_array(physdev, property, values, nval);
+	ret = device_property_read_u32_array(cs35l41->dev, property, values, nval);
 	if (ret)
 		goto err;
 	hw_cfg->spk_pos = values[cs35l41->index];
@@ -1322,41 +1317,41 @@ static int cs35l41_hda_read_acpi(struct cs35l41_hda *cs35l41, const char *hid, i
 			cs35l41->channel_index++;
 
 	property = "cirrus,gpio1-func";
-	ret = device_property_read_u32_array(physdev, property, values, nval);
+	ret = device_property_read_u32_array(cs35l41->dev, property, values, nval);
 	if (ret)
 		goto err;
 	hw_cfg->gpio1.func = values[cs35l41->index];
 	hw_cfg->gpio1.valid = true;
 
 	property = "cirrus,gpio2-func";
-	ret = device_property_read_u32_array(physdev, property, values, nval);
+	ret = device_property_read_u32_array(cs35l41->dev, property, values, nval);
 	if (ret)
 		goto err;
 	hw_cfg->gpio2.func = values[cs35l41->index];
 	hw_cfg->gpio2.valid = true;
 
 	property = "cirrus,boost-peak-milliamp";
-	ret = device_property_read_u32_array(physdev, property, values, nval);
+	ret = device_property_read_u32_array(cs35l41->dev, property, values, nval);
 	if (ret == 0)
 		hw_cfg->bst_ipk = values[cs35l41->index];
 	else
 		hw_cfg->bst_ipk = -1;
 
 	property = "cirrus,boost-ind-nanohenry";
-	ret = device_property_read_u32_array(physdev, property, values, nval);
+	ret = device_property_read_u32_array(cs35l41->dev, property, values, nval);
 	if (ret == 0)
 		hw_cfg->bst_ind = values[cs35l41->index];
 	else
 		hw_cfg->bst_ind = -1;
 
 	property = "cirrus,boost-cap-microfarad";
-	ret = device_property_read_u32_array(physdev, property, values, nval);
+	ret = device_property_read_u32_array(cs35l41->dev, property, values, nval);
 	if (ret == 0)
 		hw_cfg->bst_cap = values[cs35l41->index];
 	else
 		hw_cfg->bst_cap = -1;
 
-	cs35l41->speaker_id = cs35l41_get_speaker_id(physdev, cs35l41->index, nval, -1);
+	cs35l41->speaker_id = cs35l41_get_speaker_id(cs35l41->dev, cs35l41->index, nval, -1);
 
 	if (hw_cfg->bst_ind > 0 || hw_cfg->bst_cap > 0 || hw_cfg->bst_ipk > 0)
 		hw_cfg->bst_type = CS35L41_INT_BOOST;
@@ -1364,14 +1359,11 @@ static int cs35l41_hda_read_acpi(struct cs35l41_hda *cs35l41, const char *hid, i
 		hw_cfg->bst_type = CS35L41_EXT_BOOST;
 
 	hw_cfg->valid = true;
-	put_device(physdev);
 
 	return 0;
 
 err:
 	dev_err(cs35l41->dev, "Failed property %s: %d\n", property, ret);
-err_put_physdev:
-	put_device(physdev);
 
 	return ret;
 }
-- 
2.34.1

Re: [PATCH v1 2/2] ALSA: hda: cs35l41: Use ACPI_COMPANION to read acpi properties

[Andy Shevchenko]

On Thu, Nov 24, 2022 at 1:07 PM Stefan Binding
<sbinding@opensource.cirrus.com> wrote:
>
> Currently the driver finds the acpi_device used to read certain
> properties using the HID, however, this is not necessary, as the
> acpi_device can be obtained from the device itself.
>
> With the ACPI_COMPANION correctly set, we can also simplify how

ACPI companion device

> we obtain the reset gpio.

GPIO

...

The idea seems to be an improvement to me. Thanks.

But I have side question, are you going to address the
https://bugzilla.kernel.org/show_bug.cgi?id=215993

P.S. It would be nice if you have an account there, so I can reassign
that to you.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/2] ALSA: hda: cs35l41: Use ACPI_COMPANION to read acpi properties

[Hans de Goede]

Hi,

On 11/24/22 12:07, Stefan Binding wrote:
> Currently the driver finds the acpi_device used to read certain
> properties using the HID, however, this is not necessary, as the
> acpi_device can be obtained from the device itself.
> 
> With the ACPI_COMPANION correctly set, we can also simplify how
> we obtain the reset gpio.

Typically when you write "also do ..." in a commit message
that is a hint to yourself that it might be better to split
the commit into 2 commits which each do only 1 thing, for easier
review. But e.g. also to easier see what is going on if a bisect
points out the commit as being the first bad one.

So once the issues with patch 1/2 are resolved, please consider
splitting this patch into 2 smaller patches.

Regards,

Hans


> 
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
> ---
>  sound/pci/hda/cs35l41_hda.c | 50 ++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 29 deletions(-)
> 
> diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
> index e5f0549bf06d..50cbbcce4946 100644
> --- a/sound/pci/hda/cs35l41_hda.c
> +++ b/sound/pci/hda/cs35l41_hda.c
> @@ -1214,16 +1214,15 @@ static int cs35l41_get_speaker_id(struct device *dev, int amp_index,
>   * And devm functions expect that the device requesting the resource has the correct
>   * fwnode.
>   */
> -static int cs35l41_no_acpi_dsd(struct cs35l41_hda *cs35l41, struct device *physdev, int id,
> -			       const char *hid)
> +static int cs35l41_no_acpi_dsd(struct cs35l41_hda *cs35l41, int id, const char *hid)
>  {
>  	struct cs35l41_hw_cfg *hw_cfg = &cs35l41->hw_cfg;
>  
>  	/* check I2C address to assign the index */
>  	cs35l41->index = id == 0x40 ? 0 : 1;
>  	cs35l41->channel_index = 0;
> -	cs35l41->reset_gpio = gpiod_get_index(physdev, NULL, 0, GPIOD_OUT_HIGH);
> -	cs35l41->speaker_id = cs35l41_get_speaker_id(physdev, 0, 0, 2);
> +	cs35l41->reset_gpio = gpiod_get_index(cs35l41->dev, NULL, 0, GPIOD_OUT_HIGH);
> +	cs35l41->speaker_id = cs35l41_get_speaker_id(cs35l41->dev, 0, 0, 2);
>  	hw_cfg->spk_pos = cs35l41->index;
>  	hw_cfg->gpio2.func = CS35L41_INTERRUPT;
>  	hw_cfg->gpio2.valid = true;
> @@ -1255,39 +1254,36 @@ static int cs35l41_hda_read_acpi(struct cs35l41_hda *cs35l41, const char *hid, i
>  	struct cs35l41_hw_cfg *hw_cfg = &cs35l41->hw_cfg;
>  	u32 values[HDA_MAX_COMPONENTS];
>  	struct acpi_device *adev;
> -	struct device *physdev;
> +
>  	const char *sub;
>  	char *property;
>  	size_t nval;
>  	int i, ret;
>  
> -	adev = acpi_dev_get_first_match_dev(hid, NULL, -1);
> +	adev = ACPI_COMPANION(cs35l41->dev);
>  	if (!adev) {
> -		dev_err(cs35l41->dev, "Failed to find an ACPI device for %s\n", hid);
> +		dev_err(cs35l41->dev, "Failed to find an ACPI device for %s\n",
> +			dev_name(cs35l41->dev));
>  		return -ENODEV;
>  	}
>  
> -	physdev = get_device(acpi_get_first_physical_node(adev));
> -	acpi_dev_put(adev);
> -
> -	sub = acpi_get_subsystem_id(ACPI_HANDLE(physdev));
> +	sub = acpi_get_subsystem_id(ACPI_HANDLE(cs35l41->dev));
>  	if (IS_ERR(sub))
>  		sub = NULL;
>  	cs35l41->acpi_subsystem_id = sub;
>  
>  	property = "cirrus,dev-index";
> -	ret = device_property_count_u32(physdev, property);
> -	if (ret <= 0) {
> -		ret = cs35l41_no_acpi_dsd(cs35l41, physdev, id, hid);
> -		goto err_put_physdev;
> -	}
> +	ret = device_property_count_u32(cs35l41->dev, property);
> +	if (ret <= 0)
> +		return cs35l41_no_acpi_dsd(cs35l41, id, hid);
> +
>  	if (ret > ARRAY_SIZE(values)) {
>  		ret = -EINVAL;
>  		goto err;
>  	}
>  	nval = ret;
>  
> -	ret = device_property_read_u32_array(physdev, property, values, nval);
> +	ret = device_property_read_u32_array(cs35l41->dev, property, values, nval);
>  	if (ret)
>  		goto err;
>  
> @@ -1307,11 +1303,10 @@ static int cs35l41_hda_read_acpi(struct cs35l41_hda *cs35l41, const char *hid, i
>  	/* To use the same release code for all laptop variants we can't use devm_ version of
>  	 * gpiod_get here, as CLSA010* don't have a fully functional bios with an _DSD node
>  	 */
> -	cs35l41->reset_gpio = fwnode_gpiod_get_index(acpi_fwnode_handle(adev), "reset", cs35l41->index,
> -						     GPIOD_OUT_LOW, "cs35l41-reset");
> +	cs35l41->reset_gpio = gpiod_get_index(cs35l41->dev, "reset", cs35l41->index, GPIOD_OUT_LOW);
>  
>  	property = "cirrus,speaker-position";
> -	ret = device_property_read_u32_array(physdev, property, values, nval);
> +	ret = device_property_read_u32_array(cs35l41->dev, property, values, nval);
>  	if (ret)
>  		goto err;
>  	hw_cfg->spk_pos = values[cs35l41->index];
> @@ -1322,41 +1317,41 @@ static int cs35l41_hda_read_acpi(struct cs35l41_hda *cs35l41, const char *hid, i
>  			cs35l41->channel_index++;
>  
>  	property = "cirrus,gpio1-func";
> -	ret = device_property_read_u32_array(physdev, property, values, nval);
> +	ret = device_property_read_u32_array(cs35l41->dev, property, values, nval);
>  	if (ret)
>  		goto err;
>  	hw_cfg->gpio1.func = values[cs35l41->index];
>  	hw_cfg->gpio1.valid = true;
>  
>  	property = "cirrus,gpio2-func";
> -	ret = device_property_read_u32_array(physdev, property, values, nval);
> +	ret = device_property_read_u32_array(cs35l41->dev, property, values, nval);
>  	if (ret)
>  		goto err;
>  	hw_cfg->gpio2.func = values[cs35l41->index];
>  	hw_cfg->gpio2.valid = true;
>  
>  	property = "cirrus,boost-peak-milliamp";
> -	ret = device_property_read_u32_array(physdev, property, values, nval);
> +	ret = device_property_read_u32_array(cs35l41->dev, property, values, nval);
>  	if (ret == 0)
>  		hw_cfg->bst_ipk = values[cs35l41->index];
>  	else
>  		hw_cfg->bst_ipk = -1;
>  
>  	property = "cirrus,boost-ind-nanohenry";
> -	ret = device_property_read_u32_array(physdev, property, values, nval);
> +	ret = device_property_read_u32_array(cs35l41->dev, property, values, nval);
>  	if (ret == 0)
>  		hw_cfg->bst_ind = values[cs35l41->index];
>  	else
>  		hw_cfg->bst_ind = -1;
>  
>  	property = "cirrus,boost-cap-microfarad";
> -	ret = device_property_read_u32_array(physdev, property, values, nval);
> +	ret = device_property_read_u32_array(cs35l41->dev, property, values, nval);
>  	if (ret == 0)
>  		hw_cfg->bst_cap = values[cs35l41->index];
>  	else
>  		hw_cfg->bst_cap = -1;
>  
> -	cs35l41->speaker_id = cs35l41_get_speaker_id(physdev, cs35l41->index, nval, -1);
> +	cs35l41->speaker_id = cs35l41_get_speaker_id(cs35l41->dev, cs35l41->index, nval, -1);
>  
>  	if (hw_cfg->bst_ind > 0 || hw_cfg->bst_cap > 0 || hw_cfg->bst_ipk > 0)
>  		hw_cfg->bst_type = CS35L41_INT_BOOST;
> @@ -1364,14 +1359,11 @@ static int cs35l41_hda_read_acpi(struct cs35l41_hda *cs35l41, const char *hid, i
>  		hw_cfg->bst_type = CS35L41_EXT_BOOST;
>  
>  	hw_cfg->valid = true;
> -	put_device(physdev);
>  
>  	return 0;
>  
>  err:
>  	dev_err(cs35l41->dev, "Failed property %s: %d\n", property, ret);
> -err_put_physdev:
> -	put_device(physdev);
>  
>  	return ret;
>  }