On Wed, May 11, 2022 at 12:31 AM Moses Christopher Bollavarapu <mosescb.dev@xxxxxxxxx> wrote: I like the idea and the patch, but a few comments below. > remove of_gpio.h header file, replace of_* functions and structs > with appropriate alternatives Please respect English grammar, i.e. Capital letters at the beginning of the sentences and period at the ends. ... > struct zevio_gpio { > spinlock_t lock; > - struct of_mm_gpio_chip chip; > + struct gpio_chip chip; > + void __iomem *regs; While at it, please move the chip member to be the first one. It will optimize container_of() in case it's being called against a zevio_gpio object instance. > }; ... > + controller->regs = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(controller->regs)) { > + dev_err(&pdev->dev, "failed to ioremap memory resource\n"); > + return PTR_ERR(controller->regs); return dev_err_probe(&pdev->dev, ...); > + } ... > + status = devm_gpiochip_add_data(&pdev->dev, > + &(controller->chip), Too many parentheses. > + controller); Also, combine all of them to be located on a single line. -- With Best Regards, Andy Shevchenko