On Wed, 2017-03-22 at 11:23 +0100, Peter Rosin wrote:
> On 2017-03-21 20:13, Andy Shevchenko wrote:
> > In ACPI world any ID should be carefully chosen and registered
> > officially. The commit bbf9d262a147 seems did a wrong assumption
> > because
> > PCA is the registered PNP ID for "PHILIPS BU ADD ON CARD". I'm
> > pretty
> > sure this prefix has nothing to do with the driver in question.
>
> [Cc: leds people, in case they know any details]
>
> Hmmm, a couple of questions about that "pretty sure"...
I didn't neither see the *real* excerpt from DSDT nor hear anything
about official IDs from Phillips.
> Philips and NXP are pretty much just different faces of the same coin,
> IIUC.
Good to know.
While I might be mistaken, I would like to remove a confusion until we
get an official confirmation either in *real* existing product on the
market or letter from Phillips representatives (see above).
> But, what do I know? Also, what about the leds drivers for NXP PCA955x
> and
> NXP PCA963x? Do they suffer from the same crap? And if not, why is
> that
> any different?
They pretty much do.
Yesterday I send a patch to remove LP3952 invented ID which TI didn't
confirm to exists.
My scope now is limited by the ACPI-enabled drivers which are using
*gpiod_get*() functions.
> From drivers/leds/leds-pca955x.c
>
> static const struct acpi_device_id pca955x_acpi_ids[] = {
> { "PCA9550", pca9550 },
> { "PCA9551", pca9551 },
> { "PCA9552", pca9552 },
> { "PCA9553", pca9553 },
> { }
> };
> MODULE_DEVICE_TABLE(acpi, pca955x_acpi_ids);
>
> and from drivers/leds/leds-pca963x.c
>
> static const struct acpi_device_id pca963x_acpi_ids[] = {
> { "PCA9632", pca9633 },
> { "PCA9633", pca9633 },
> { "PCA9634", pca9634 },
> { "PCA9635", pca9635 },
> { }
> };
> MODULE_DEVICE_TABLE(acpi, pca963x_acpi_ids);
>
> But maybe I'm full of it and these led chips really are part of
> "PHILIPS
> BU ADD ON CARD", while the muxer chips are not? I doubt it though...
> But again, what do I know?
Thanks for input to this topic. As I said above I might be mistaken too,
though we can't just wilfully invent ACPI IDs without vendors' approvals
/ confirmations.
>
> Cheers,
> peda
>
> > Moreover, newer ACPI specification has a support of _DSD method and
> > special device IDs to allow drivers be enumerated via compatible
> > string.
> > The slight change to support this kind of enumeration will be added
> > in
> > sequential patch against pca954x.c.
> >
> > Revert the commit bbf9d262a147 for good.
> >
> > Cc: Tin Huynh <tnhuynh@xxxxxxx>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > ---
> > drivers/i2c/muxes/i2c-mux-pca954x.c | 28 +-------------------------
> > --
> > 1 file changed, 1 insertion(+), 27 deletions(-)
> >
> > diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c
> > b/drivers/i2c/muxes/i2c-mux-pca954x.c
> > index dfc1c0e37c40..333a3918b656 100644
> > --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> > +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> > @@ -35,7 +35,6 @@
> > * warranty of any kind, whether express or implied.
> > */
> >
> > -#include <linux/acpi.h>
> > #include <linux/device.h>
> > #include <linux/gpio/consumer.h>
> > #include <linux/i2c.h>
> > @@ -141,21 +140,6 @@ static const struct i2c_device_id pca954x_id[]
> > = {
> > };
> > MODULE_DEVICE_TABLE(i2c, pca954x_id);
> >
> > -#ifdef CONFIG_ACPI
> > -static const struct acpi_device_id pca954x_acpi_ids[] = {
> > - { .id = "PCA9540", .driver_data = pca_9540 },
> > - { .id = "PCA9542", .driver_data = pca_9542 },
> > - { .id = "PCA9543", .driver_data = pca_9543 },
> > - { .id = "PCA9544", .driver_data = pca_9544 },
> > - { .id = "PCA9545", .driver_data = pca_9545 },
> > - { .id = "PCA9546", .driver_data = pca_9545 },
> > - { .id = "PCA9547", .driver_data = pca_9547 },
> > - { .id = "PCA9548", .driver_data = pca_9548 },
> > - { }
> > -};
> > -MODULE_DEVICE_TABLE(acpi, pca954x_acpi_ids);
> > -#endif
> > -
> > #ifdef CONFIG_OF
> > static const struct of_device_id pca954x_of_match[] = {
> > { .compatible = "nxp,pca9540", .data = &chips[pca_9540] },
> > @@ -393,17 +377,8 @@ static int pca954x_probe(struct i2c_client
> > *client,
> > match = of_match_device(of_match_ptr(pca954x_of_match),
> > &client->dev);
> > if (match)
> > data->chip = of_device_get_match_data(&client-
> > >dev);
> > - else if (id)
> > + else
> > data->chip = &chips[id->driver_data];
> > - else {
> > - const struct acpi_device_id *acpi_id;
> > -
> > - acpi_id =
> > acpi_match_device(ACPI_PTR(pca954x_acpi_ids),
> > - &client->dev);
> > - if (!acpi_id)
> > - return -ENODEV;
> > - data->chip = &chips[acpi_id->driver_data];
> > - }
> >
> > data->last_chan = 0; /* force the first
> > selection */
> >
> > @@ -492,7 +467,6 @@ static struct i2c_driver pca954x_driver = {
> > .name = "pca954x",
> > .pm = &pca954x_pm,
> > .of_match_table = of_match_ptr(pca954x_of_match),
> > - .acpi_match_table = ACPI_PTR(pca954x_acpi_ids),
> > },
> > .probe = pca954x_probe,
> > .remove = pca954x_remove,
> >
>
>
--
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy
