On Thu, Dec 8, 2016 at 10:12 PM, Jeremy Kerr <jk@xxxxxxxxxx> wrote: > Hi Chris, > >> +static ssize_t store_scan(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, >> + size_t count) >> +{ >> + struct fsi_master_gpio *master = dev_get_drvdata(dev); >> + >> + fsi_master_gpio_init(master); >> + >> + /* clear out any old scan data if present */ >> + fsi_master_unregister(&master->master); >> + fsi_master_register(&master->master); >> + >> + return count; >> +} >> + >> +static DEVICE_ATTR(scan, 0200, NULL, store_scan); > > I think it would make more sense to have the scan attribute populated by > the fsi core; we want this on all masters, not just GPIO. > Hi Jeremy, Sure, will move that to the core. > Currently, the only GPIO-master-specific functionality here is the > fsi_master_gpio_init() - but isn't this something that we can do at > probe time instead? > Yes that can be done at probe time. Will change. >> +static int fsi_master_gpio_probe(struct platform_device *pdev) >> +{ >> + struct fsi_master_gpio *master; >> + struct gpio_desc *gpio; >> + >> + master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL); >> + if (!master) >> + return -ENOMEM; > > We should be populating master->dev.parent, see > > https://github.com/jk-ozlabs/linux/commit/5225d6c47 > > Will make the change. >> + /* Optional pins */ >> + >> + gpio = devm_gpiod_get(&pdev->dev, "trans", 0); >> + if (IS_ERR(gpio)) >> + dev_dbg(&pdev->dev, "probe: failed to get trans pin\n"); >> + else >> + master->gpio_trans = gpio; > > I found devm_gpiod_get_optional(), which might make this a little > neater. Will make this change. Thanks, Chris > > Cheers, > > > Jeremy -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html