[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0iKLtQpUnhMqB6zgwbURXGFZkje5rNORS9MLqYN=13nWg@mail.gmail.com>
Date: Fri, 23 Jun 2023 16:43:55 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-i2c@...r.kernel.org, acpica-devel@...ts.linuxfoundation.org,
Len Brown <lenb@...nel.org>,
Andi Shyti <andi.shyti@...nel.org>,
Robert Moore <robert.moore@...el.com>,
Michael Brunner <michael.brunner@...tron.com>
Subject: Re: [PATCH v2 1/2] ACPI: platform: Ignore SMB0001 only when it has resources
On Thu, Jun 22, 2023 at 8:19 PM Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> wrote:
>
> On Thu, Jun 22, 2023 at 05:53:13PM +0200, Rafael J. Wysocki wrote:
> > On Wed, Jun 21, 2023 at 5:16 PM Andy Shevchenko
> > <andriy.shevchenko@...ux.intel.com> wrote:
> > >
> > > After switching i2c-scmi driver to be a plaform one, it stopped
> >
> > "platform"
> >
> > > being enumerated on number of Kontron platforms, because it's
> > > listed in the forbidden_id_list.
> > >
> > > To resolve the situation, split the list to generic one and
> > > another that holds devices that has to be skipped if and only
> >
> > "have"
> >
> > > if they have bogus resources attached (_CRS method returns some).
>
> Thanks for the typo fixes!
>
> ...
>
> > > +static const struct acpi_device_id forbidden_id_with_resourses[] = {
> >
> > I don't quite like this name and the driver_data field could be used
> > to indicate the need to check the resources.
>
> Okay, something like
>
> /* Check if the device has resources provided by _CRS method */
> #define ACPI_PLATFORM_CHECK_RES BIT(0)
>
> ?
Could be, but this is specific to forbidden_ids_list[]. Maybe
ACPI_ALLOW_WO_RESOURCES or similar.
> > > + {"SMB0001", 0}, /* ACPI SMBUS virtual device */
> > > + { }
> > > +};
>
> ...
>
> > > +static int acpi_platform_resource_count(struct acpi_resource *ares, void *data)
> > > +{
> > > + int *count = data;
> > > +
> > > + *count = *count + 1;
> >
> > Why not (*count)++?
>
> Can be that way, I just copied'n'pasted from the existing code.
>
> > > + return 1;
> > > +}
BTW, this doesn't need to increment the count even. It could just
terminate the walk on the first valid resource found and tell the
caller to return true in that case.
Powered by blists - more mailing lists