Messages in this thread | | | From | Andy Shevchenko <> | Date | Sun, 14 Mar 2021 15:51:54 +0200 | Subject | Re: [PATCH] gpio: mpc8xxx: Add ACPI support |
| |
On Fri, Mar 12, 2021 at 8:53 AM Ran Wang <ran.wang_1@nxp.com> wrote:
First of all, please add me to the Cc list for the next version of the patch.
> Current implementation only supports DT, now add ACPI support. > > Note that compared to device of 'fsl,qoriq-gpio', LS1028A and
to devices
> LS1088A's GPIO have no extra programming, so simplify related checking.
...
> +#include <linux/acpi.h>
Nope, you rather need property.h and mod_devicetable.h.
...
> + if (pdev->dev.of_node) { > + devtype = of_device_get_match_data(&pdev->dev); > + } else { > + acpi_id = acpi_match_device(pdev->dev.driver->acpi_match_table, > + &pdev->dev); > + if (acpi_id) > + devtype = (struct mpc8xxx_gpio_devtype *) acpi_id->driver_data; > + }
No, please use device_get_match_data() instead of the entire conditional block.
> + if (pdev->dev.of_node) { > + if (of_device_is_compatible(np, "fsl,qoriq-gpio")) > + gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 0xffffffff); > + } else { > + if (acpi_match_device(pdev->dev.driver->acpi_match_table, > + &pdev->dev)) > + gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 0xffffffff); > + }
Yeah, no need to call acpi_match_device() here. Instead use stuff from OF, like
if (of_device_is_compatible() || !(IS_ERR_OR_NULL(fwnode) || is_of_node(fwnode))) (check the logic)
...
> +#ifdef CONFIG_ACPI
No ugly ifdeffery, please.
> +static const struct acpi_device_id gpio_acpi_ids[] = { > + {"NXP0031",}, > + { } > +}; > +MODULE_DEVICE_TABLE(acpi, gpio_acpi_ids); > +#endif > + > static struct platform_driver mpc8xxx_plat_driver = { > .probe = mpc8xxx_probe, > .remove = mpc8xxx_remove, > .driver = { > .name = "gpio-mpc8xxx", > .of_match_table = mpc8xxx_gpio_ids,
> + .acpi_match_table = ACPI_PTR(gpio_acpi_ids),
Drop ACPI_PTR().
> }, > };
-- With Best Regards, Andy Shevchenko
| |