| Message ID | 20240228023925.2814638-2-hamish.martin@alliedtelesis.co.nz |
|---|---|
| State | Superseded |
| Headers | show |
| Series | I2C mux ACPI SSDT overlay removal issue | expand |
On Wed, Feb 28, 2024 at 03:39:25PM +1300, Hamish Martin wrote: > Unbind any i2c_adapters matching an ACPI device being removed. This > ensures that the linkage between the ACPI device and the i2c_adapter is > correctly broken before the ACPI device is destroyed. I suggest to move some of the information from the cover letter here to make sure it is not lost. It includes good analysis of the problem so should be part of the changelog. > Signed-off-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz> > --- > drivers/i2c/i2c-core-acpi.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c > index d6037a328669..67fa8deccef6 100644 > --- a/drivers/i2c/i2c-core-acpi.c > +++ b/drivers/i2c/i2c-core-acpi.c > @@ -445,6 +445,11 @@ static struct i2c_client *i2c_acpi_find_client_by_adev(struct acpi_device *adev) > return i2c_find_device_by_fwnode(acpi_fwnode_handle(adev)); > } > > +static struct i2c_adapter *i2c_acpi_find_adapter_by_adev(struct acpi_device *adev) > +{ > + return i2c_find_adapter_by_fwnode(acpi_fwnode_handle(adev)); > +} > + > static int i2c_acpi_notify(struct notifier_block *nb, unsigned long value, > void *arg) > { > @@ -471,11 +476,17 @@ static int i2c_acpi_notify(struct notifier_block *nb, unsigned long value, > break; > > client = i2c_acpi_find_client_by_adev(adev); > - if (!client) > - break; > + if (client) { > + i2c_unregister_device(client); > + put_device(&client->dev); > + } > + > + adapter = i2c_acpi_find_adapter_by_adev(adev); > + if (adapter) { > + acpi_device_notify_remove(&adapter->dev); > + put_device(&adapter->dev); > + } This looks good to me. I'm just wondering whether we have the same issue on DT side too? > > - i2c_unregister_device(client); > - put_device(&client->dev); > break; > } > > -- > 2.43.0
On Thu, 2024-02-29 at 11:54 +0200, Mika Westerberg wrote: > On Wed, Feb 28, 2024 at 03:39:25PM +1300, Hamish Martin wrote: > > Unbind any i2c_adapters matching an ACPI device being removed. This > > ensures that the linkage between the ACPI device and the > > i2c_adapter is > > correctly broken before the ACPI device is destroyed. > I suggest to move some of the information from the cover letter here > to > make sure it is not lost. It includes good analysis of the problem so > should be part of the changelog. OK, thanks Mika. I will do that. > > > Signed-off-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz> > > --- > > drivers/i2c/i2c-core-acpi.c | 19 +++++++++++++++---- > > 1 file changed, 15 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core- > > acpi.c > > index d6037a328669..67fa8deccef6 100644 > > --- a/drivers/i2c/i2c-core-acpi.c > > +++ b/drivers/i2c/i2c-core-acpi.c > > @@ -445,6 +445,11 @@ static struct i2c_client > > *i2c_acpi_find_client_by_adev(struct acpi_device *adev) > > return i2c_find_device_by_fwnode(acpi_fwnode_handle(adev)); > > } > > > > +static struct i2c_adapter *i2c_acpi_find_adapter_by_adev(struct > > acpi_device *adev) > > +{ > > + return > > i2c_find_adapter_by_fwnode(acpi_fwnode_handle(adev)); > > +} > > + > > static int i2c_acpi_notify(struct notifier_block *nb, unsigned > > long value, > > void *arg) > > { > > @@ -471,11 +476,17 @@ static int i2c_acpi_notify(struct > > notifier_block *nb, unsigned long value, > > break; > > > > client = i2c_acpi_find_client_by_adev(adev); > > - if (!client) > > - break; > > + if (client) { > > + i2c_unregister_device(client); > > + put_device(&client->dev); > > + } > > + > > + adapter = i2c_acpi_find_adapter_by_adev(adev); > > + if (adapter) { > > + acpi_device_notify_remove(&adapter->dev); > > + put_device(&adapter->dev); > > + } > > This looks good to me. I'm just wondering whether we have the same > issue > on DT side too? We don't see that. We have plenty of systems with the same basic hardware layout using device tree and don't observe this kind of issue. > > > > > - i2c_unregister_device(client); > > - put_device(&client->dev); > > break; > > } > > > > -- > > 2.43.0
diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c index d6037a328669..67fa8deccef6 100644 --- a/drivers/i2c/i2c-core-acpi.c +++ b/drivers/i2c/i2c-core-acpi.c @@ -445,6 +445,11 @@ static struct i2c_client *i2c_acpi_find_client_by_adev(struct acpi_device *adev) return i2c_find_device_by_fwnode(acpi_fwnode_handle(adev)); } +static struct i2c_adapter *i2c_acpi_find_adapter_by_adev(struct acpi_device *adev) +{ + return i2c_find_adapter_by_fwnode(acpi_fwnode_handle(adev)); +} + static int i2c_acpi_notify(struct notifier_block *nb, unsigned long value, void *arg) { @@ -471,11 +476,17 @@ static int i2c_acpi_notify(struct notifier_block *nb, unsigned long value, break; client = i2c_acpi_find_client_by_adev(adev); - if (!client) - break; + if (client) { + i2c_unregister_device(client); + put_device(&client->dev); + } + + adapter = i2c_acpi_find_adapter_by_adev(adev); + if (adapter) { + acpi_device_notify_remove(&adapter->dev); + put_device(&adapter->dev); + } - i2c_unregister_device(client); - put_device(&client->dev); break; }
Unbind any i2c_adapters matching an ACPI device being removed. This ensures that the linkage between the ACPI device and the i2c_adapter is correctly broken before the ACPI device is destroyed. Signed-off-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz> --- drivers/i2c/i2c-core-acpi.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)