On Fri, Oct 08, 2021 at 10:17:36AM +0200, Bartosz Golaszewski wrote: > Implement a new, modern GPIO testing module controlled by configfs > attributes instead of module parameters. The goal of this driver is > to provide a replacement for gpio-mockup that will be easily extensible > with new features and doesn't require reloading the module to change > the setup. > > Signed-off-by: Bartosz Golaszewski <brgl@xxxxxxxx> > [Andy: Initialize attribute allocated on the heap] > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > [Colin: Fix dereference of free'd pointer config] > Signed-off-by: Colin Ian King <colin.king@xxxxxxxxxxxxx> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx> Some nit-picks below, up to you to address. ... > + ret = gpio_sim_setup_sysfs(chip); > + if (ret) > + return ret; > + > + return 0; return gpio_sim_...(chip); ? ... > + Redundant empty line. > +CONFIGFS_ATTR_RO(gpio_sim_config_, dev_name); ... > + Ditto. > +CONFIGFS_ATTR_RO(gpio_sim_config_, chip_name); ... > + Ditto. > +CONFIGFS_ATTR(gpio_sim_config_, label); ... > + Ditto. > +CONFIGFS_ATTR(gpio_sim_config_, num_lines); ... > + Ditto. > +CONFIGFS_ATTR(gpio_sim_config_, line_names); ... > + fwnode = fwnode_create_software_node(properties, NULL); > + if (IS_ERR(fwnode)) > + return PTR_ERR(fwnode); > + fwnode = dev_fwnode(&config->pdev->dev); > + platform_device_unregister(config->pdev); > + fwnode_remove_software_node(fwnode); This seems correct, thank you for modifying the code. ... > + config->pdev = NULL; > + mutex_unlock(&config->lock); mutex_destroy() ? Or is it done in the upper level? -- With Best Regards, Andy Shevchenko
