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. 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? > +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 > + /* 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. 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