On Fri, Mar 19, 2021 at 10:10 AM Ran Wang <ran.wang_1@xxxxxxx> wrote: > > Current implementation only supports DT, now add ACPI support. Thanks for an update, my comments below. ... > +#include <linux/acpi.h> Missed property.h mod_devicetable.h. ... > - mpc8xxx_gc->regs = of_iomap(np, 0); > + mpc8xxx_gc->regs = devm_platform_ioremap_resource(pdev, 0); > if (!mpc8xxx_gc->regs) This is wrong now. > return -ENOMEM; This too. ... > + fwnode = dev_fwnode(&pdev->dev); > if (of_device_is_compatible(np, "fsl,qoriq-gpio") || > of_device_is_compatible(np, "fsl,ls1028a-gpio") || > - of_device_is_compatible(np, "fsl,ls1088a-gpio")) > + of_device_is_compatible(np, "fsl,ls1088a-gpio") || > + !(IS_ERR_OR_NULL(fwnode) || is_of_node(fwnode))) Since you left acpi.h inclusion, you may switch this to is_acpi_node(fwnode) or drop fwnode and use has_acpi_companion(&pdev->dev) > gc->write_reg(mpc8xxx_gc->regs + GPIO_IBE, 0xffffffff); -- With Best Regards, Andy Shevchenko