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@xxxxxxxxxxxxxxxxxxx>
> > ---
> > 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
