* [PATCH 0/2] fpga: add attribute groups @ 2017-08-22 15:53 Alan Tull 2017-08-22 15:53 ` [PATCH 1/2] " Alan Tull 2017-08-22 15:53 ` [PATCH 2/2] fpga: altera-cvp: register attribute groups using ops Alan Tull 0 siblings, 2 replies; 5+ messages in thread From: Alan Tull @ 2017-08-22 15:53 UTC (permalink / raw) To: Moritz Fischer, Anatolij Gustschin; +Cc: Alan Tull, linux-kernel, linux-fpga Make it easy to add device attribute groups when registering an FPGA manager, bridge, or region. This way, the sysfs attributes are added in device_add instead of afterwards which can lead to race conditions [1]. Patch 1: add groups to the structs that are used when a manager, bridge, or region is registered. Patch 2: changes to altera cvp FPGA manager to use this method of registering. I've tested this method on platform devices. Patch 2 builds but is untested. This patchset is dependent on my "non-dt support" patchset currently under review. [1] http://www.kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/ Alan Tull (2): fpga: add attribute groups fpga: altera-cvp: register attribute groups using ops drivers/fpga/altera-cvp.c | 39 +++++++++++++++++++-------------------- drivers/fpga/fpga-bridge.c | 1 + drivers/fpga/fpga-mgr.c | 1 + drivers/fpga/fpga-region.c | 1 + include/linux/fpga/fpga-bridge.h | 2 ++ include/linux/fpga/fpga-mgr.h | 2 ++ include/linux/fpga/fpga-region.h | 2 ++ 7 files changed, 28 insertions(+), 20 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] fpga: add attribute groups 2017-08-22 15:53 [PATCH 0/2] fpga: add attribute groups Alan Tull @ 2017-08-22 15:53 ` Alan Tull 2017-08-22 15:53 ` [PATCH 2/2] fpga: altera-cvp: register attribute groups using ops Alan Tull 1 sibling, 0 replies; 5+ messages in thread From: Alan Tull @ 2017-08-22 15:53 UTC (permalink / raw) To: Moritz Fischer, Anatolij Gustschin; +Cc: Alan Tull, linux-kernel, linux-fpga Make it easy to add attributes to low level FPGA drivers the right way. Add attribute groups pointers to structures that are used when registering a manager, bridge, or group. When the low level driver registers, set the device attribute group. The attributes are created in device_add. Signed-off-by: Alan Tull <atull@kernel.org> --- drivers/fpga/fpga-bridge.c | 1 + drivers/fpga/fpga-mgr.c | 1 + drivers/fpga/fpga-region.c | 1 + include/linux/fpga/fpga-bridge.h | 2 ++ include/linux/fpga/fpga-mgr.h | 2 ++ include/linux/fpga/fpga-region.h | 2 ++ 6 files changed, 9 insertions(+) diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c index af6d97e..a189638 100644 --- a/drivers/fpga/fpga-bridge.c +++ b/drivers/fpga/fpga-bridge.c @@ -367,6 +367,7 @@ int fpga_bridge_register(struct device *dev, const char *name, bridge->priv = priv; device_initialize(&bridge->dev); + bridge->dev.groups = br_ops->groups; bridge->dev.class = fpga_bridge_class; bridge->dev.parent = dev; bridge->dev.of_node = dev->of_node; diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c index be13cce..b54f6f0 100644 --- a/drivers/fpga/fpga-mgr.c +++ b/drivers/fpga/fpga-mgr.c @@ -560,6 +560,7 @@ int fpga_mgr_register(struct device *dev, const char *name, device_initialize(&mgr->dev); mgr->dev.class = fpga_mgr_class; + mgr->dev.groups = mops->groups; mgr->dev.parent = dev; mgr->dev.of_node = dev->of_node; mgr->dev.id = id; diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c index afc6188..edab2a2 100644 --- a/drivers/fpga/fpga-region.c +++ b/drivers/fpga/fpga-region.c @@ -173,6 +173,7 @@ int fpga_region_register(struct device *dev, struct fpga_region *region) mutex_init(®ion->mutex); INIT_LIST_HEAD(®ion->bridge_list); device_initialize(®ion->dev); + region->dev.groups = region->groups; region->dev.class = fpga_region_class; region->dev.parent = dev; region->dev.of_node = dev->of_node; diff --git a/include/linux/fpga/fpga-bridge.h b/include/linux/fpga/fpga-bridge.h index 9f6696b..652160b 100644 --- a/include/linux/fpga/fpga-bridge.h +++ b/include/linux/fpga/fpga-bridge.h @@ -11,11 +11,13 @@ struct fpga_bridge; * @enable_show: returns the FPGA bridge's status * @enable_set: set a FPGA bridge as enabled or disabled * @fpga_bridge_remove: set FPGA into a specific state during driver remove + * @groups: optional attribute groups. */ struct fpga_bridge_ops { int (*enable_show)(struct fpga_bridge *bridge); int (*enable_set)(struct fpga_bridge *bridge, bool enable); void (*fpga_bridge_remove)(struct fpga_bridge *bridge); + const struct attribute_group **groups; }; /** diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h index 50954cb..bb061a5 100644 --- a/include/linux/fpga/fpga-mgr.h +++ b/include/linux/fpga/fpga-mgr.h @@ -113,6 +113,7 @@ struct fpga_image_info { * @write_sg: write the scatter list of configuration data to the FPGA * @write_complete: set FPGA to operating state after writing is done * @fpga_remove: optional: Set FPGA into a specific state during driver remove + * @groups: optional attribute groups. * * fpga_manager_ops are the low level functions implemented by a specific * fpga manager driver. The optional ones are tested for NULL before being @@ -129,6 +130,7 @@ struct fpga_manager_ops { int (*write_complete)(struct fpga_manager *mgr, struct fpga_image_info *info); void (*fpga_remove)(struct fpga_manager *mgr); + const struct attribute_group **groups; }; /** diff --git a/include/linux/fpga/fpga-region.h b/include/linux/fpga/fpga-region.h index f2eecdd..4a5562f 100644 --- a/include/linux/fpga/fpga-region.h +++ b/include/linux/fpga/fpga-region.h @@ -14,6 +14,7 @@ * @info: FPGA image info * @priv: private data * @get_bridges: optional function to get bridges to a list + * @groups: optional attribute groups. */ struct fpga_region { struct device dev; @@ -23,6 +24,7 @@ struct fpga_region { struct fpga_image_info *info; void *priv; int (*get_bridges)(struct fpga_region *region); + const struct attribute_group **groups; }; #define to_fpga_region(d) container_of(d, struct fpga_region, dev) -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] fpga: altera-cvp: register attribute groups using ops 2017-08-22 15:53 [PATCH 0/2] fpga: add attribute groups Alan Tull 2017-08-22 15:53 ` [PATCH 1/2] " Alan Tull @ 2017-08-22 15:53 ` Alan Tull 2017-08-22 20:23 ` Alan Tull 1 sibling, 1 reply; 5+ messages in thread From: Alan Tull @ 2017-08-22 15:53 UTC (permalink / raw) To: Moritz Fischer, Anatolij Gustschin; +Cc: Alan Tull, linux-kernel, linux-fpga --- drivers/fpga/altera-cvp.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c index 08629ee..a498022 100644 --- a/drivers/fpga/altera-cvp.c +++ b/drivers/fpga/altera-cvp.c @@ -354,20 +354,14 @@ static int altera_cvp_write_complete(struct fpga_manager *mgr, return ret; } -static const struct fpga_manager_ops altera_cvp_ops = { - .state = altera_cvp_state, - .write_init = altera_cvp_write_init, - .write = altera_cvp_write, - .write_complete = altera_cvp_write_complete, -}; - -static ssize_t show_chkcfg(struct device_driver *dev, char *buf) +static ssize_t show_chkcfg(struct device *dev, + struct device_attribute *attr, char *buf) { return snprintf(buf, 3, "%d\n", altera_cvp_chkcfg); } -static ssize_t store_chkcfg(struct device_driver *drv, const char *buf, - size_t count) +static ssize_t store_chkcfg(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) { int ret; @@ -378,7 +372,21 @@ static ssize_t store_chkcfg(struct device_driver *drv, const char *buf, return count; } -static DRIVER_ATTR(chkcfg, 0600, show_chkcfg, store_chkcfg); +static DEVICE_ATTR(chkcfg, 0600, show_chkcfg, store_chkcfg); + +static struct attribute *altera_cvp_attrs[] = { + &dev_attr_chkcfg.attr, + NULL, +}; +ATTRIBUTE_GROUPS(altera_cvp); + +static const struct fpga_manager_ops altera_cvp_ops = { + .state = altera_cvp_state, + .write_init = altera_cvp_write_init, + .write = altera_cvp_write, + .write_complete = altera_cvp_write_complete, + .groups = altera_cvp_groups, +}; static int altera_cvp_probe(struct pci_dev *pdev, const struct pci_device_id *dev_id); @@ -459,14 +467,6 @@ static int altera_cvp_probe(struct pci_dev *pdev, if (ret) goto err_unmap; - ret = driver_create_file(&altera_cvp_driver.driver, - &driver_attr_chkcfg); - if (ret) { - dev_err(&pdev->dev, "Can't create sysfs chkcfg file\n"); - fpga_mgr_unregister(&pdev->dev); - goto err_unmap; - } - return 0; err_unmap: @@ -484,7 +484,6 @@ static void altera_cvp_remove(struct pci_dev *pdev) struct altera_cvp_conf *conf = mgr->priv; u16 cmd; - driver_remove_file(&altera_cvp_driver.driver, &driver_attr_chkcfg); fpga_mgr_unregister(&pdev->dev); pci_iounmap(pdev, conf->map); pci_release_region(pdev, CVP_BAR); -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] fpga: altera-cvp: register attribute groups using ops 2017-08-22 15:53 ` [PATCH 2/2] fpga: altera-cvp: register attribute groups using ops Alan Tull @ 2017-08-22 20:23 ` Alan Tull 2017-08-23 17:54 ` Alan Tull 0 siblings, 1 reply; 5+ messages in thread From: Alan Tull @ 2017-08-22 20:23 UTC (permalink / raw) To: Moritz Fischer, Anatolij Gustschin; +Cc: Alan Tull, linux-kernel, linux-fpga On Tue, Aug 22, 2017 at 10:53 AM, Alan Tull <atull@kernel.org> wrote: OK so obviously, this patch doesn't have the necessary header or signoff. Will fix that. Alan > --- > drivers/fpga/altera-cvp.c | 39 +++++++++++++++++++-------------------- > 1 file changed, 19 insertions(+), 20 deletions(-) > > diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c > index 08629ee..a498022 100644 > --- a/drivers/fpga/altera-cvp.c > +++ b/drivers/fpga/altera-cvp.c > @@ -354,20 +354,14 @@ static int altera_cvp_write_complete(struct fpga_manager *mgr, > return ret; > } > > -static const struct fpga_manager_ops altera_cvp_ops = { > - .state = altera_cvp_state, > - .write_init = altera_cvp_write_init, > - .write = altera_cvp_write, > - .write_complete = altera_cvp_write_complete, > -}; > - > -static ssize_t show_chkcfg(struct device_driver *dev, char *buf) > +static ssize_t show_chkcfg(struct device *dev, > + struct device_attribute *attr, char *buf) > { > return snprintf(buf, 3, "%d\n", altera_cvp_chkcfg); > } > > -static ssize_t store_chkcfg(struct device_driver *drv, const char *buf, > - size_t count) > +static ssize_t store_chkcfg(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > { > int ret; > > @@ -378,7 +372,21 @@ static ssize_t store_chkcfg(struct device_driver *drv, const char *buf, > return count; > } > > -static DRIVER_ATTR(chkcfg, 0600, show_chkcfg, store_chkcfg); > +static DEVICE_ATTR(chkcfg, 0600, show_chkcfg, store_chkcfg); > + > +static struct attribute *altera_cvp_attrs[] = { > + &dev_attr_chkcfg.attr, > + NULL, > +}; > +ATTRIBUTE_GROUPS(altera_cvp); > + > +static const struct fpga_manager_ops altera_cvp_ops = { > + .state = altera_cvp_state, > + .write_init = altera_cvp_write_init, > + .write = altera_cvp_write, > + .write_complete = altera_cvp_write_complete, > + .groups = altera_cvp_groups, > +}; > > static int altera_cvp_probe(struct pci_dev *pdev, > const struct pci_device_id *dev_id); > @@ -459,14 +467,6 @@ static int altera_cvp_probe(struct pci_dev *pdev, > if (ret) > goto err_unmap; > > - ret = driver_create_file(&altera_cvp_driver.driver, > - &driver_attr_chkcfg); > - if (ret) { > - dev_err(&pdev->dev, "Can't create sysfs chkcfg file\n"); > - fpga_mgr_unregister(&pdev->dev); > - goto err_unmap; > - } > - > return 0; > > err_unmap: > @@ -484,7 +484,6 @@ static void altera_cvp_remove(struct pci_dev *pdev) > struct altera_cvp_conf *conf = mgr->priv; > u16 cmd; > > - driver_remove_file(&altera_cvp_driver.driver, &driver_attr_chkcfg); > fpga_mgr_unregister(&pdev->dev); > pci_iounmap(pdev, conf->map); > pci_release_region(pdev, CVP_BAR); > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] fpga: altera-cvp: register attribute groups using ops 2017-08-22 20:23 ` Alan Tull @ 2017-08-23 17:54 ` Alan Tull 0 siblings, 0 replies; 5+ messages in thread From: Alan Tull @ 2017-08-23 17:54 UTC (permalink / raw) To: Moritz Fischer, Anatolij Gustschin; +Cc: Alan Tull, linux-kernel, linux-fpga On Tue, Aug 22, 2017 at 3:23 PM, Alan Tull <atull@kernel.org> wrote: > On Tue, Aug 22, 2017 at 10:53 AM, Alan Tull <atull@kernel.org> wrote: On second thought, this patch is probably not really helpful as it changes the path of the existing sysfs file from /sys/bus/pci/drivers/altera-cvp/chkcfg to /sys/class/fpga_manager/fpga*/chkcfg which there is no need to do. Patch 1/2 is still fine for manager drivers in the future that are adding device attributes. Alan > > OK so obviously, this patch doesn't have the necessary header or > signoff. Will fix that. > > Alan > >> --- >> drivers/fpga/altera-cvp.c | 39 +++++++++++++++++++-------------------- >> 1 file changed, 19 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/fpga/altera-cvp.c b/drivers/fpga/altera-cvp.c >> index 08629ee..a498022 100644 >> --- a/drivers/fpga/altera-cvp.c >> +++ b/drivers/fpga/altera-cvp.c >> @@ -354,20 +354,14 @@ static int altera_cvp_write_complete(struct fpga_manager *mgr, >> return ret; >> } >> >> -static const struct fpga_manager_ops altera_cvp_ops = { >> - .state = altera_cvp_state, >> - .write_init = altera_cvp_write_init, >> - .write = altera_cvp_write, >> - .write_complete = altera_cvp_write_complete, >> -}; >> - >> -static ssize_t show_chkcfg(struct device_driver *dev, char *buf) >> +static ssize_t show_chkcfg(struct device *dev, >> + struct device_attribute *attr, char *buf) >> { >> return snprintf(buf, 3, "%d\n", altera_cvp_chkcfg); >> } >> >> -static ssize_t store_chkcfg(struct device_driver *drv, const char *buf, >> - size_t count) >> +static ssize_t store_chkcfg(struct device *dev, struct device_attribute *attr, >> + const char *buf, size_t count) >> { >> int ret; >> >> @@ -378,7 +372,21 @@ static ssize_t store_chkcfg(struct device_driver *drv, const char *buf, >> return count; >> } >> >> -static DRIVER_ATTR(chkcfg, 0600, show_chkcfg, store_chkcfg); >> +static DEVICE_ATTR(chkcfg, 0600, show_chkcfg, store_chkcfg); >> + >> +static struct attribute *altera_cvp_attrs[] = { >> + &dev_attr_chkcfg.attr, >> + NULL, >> +}; >> +ATTRIBUTE_GROUPS(altera_cvp); >> + >> +static const struct fpga_manager_ops altera_cvp_ops = { >> + .state = altera_cvp_state, >> + .write_init = altera_cvp_write_init, >> + .write = altera_cvp_write, >> + .write_complete = altera_cvp_write_complete, >> + .groups = altera_cvp_groups, >> +}; >> >> static int altera_cvp_probe(struct pci_dev *pdev, >> const struct pci_device_id *dev_id); >> @@ -459,14 +467,6 @@ static int altera_cvp_probe(struct pci_dev *pdev, >> if (ret) >> goto err_unmap; >> >> - ret = driver_create_file(&altera_cvp_driver.driver, >> - &driver_attr_chkcfg); >> - if (ret) { >> - dev_err(&pdev->dev, "Can't create sysfs chkcfg file\n"); >> - fpga_mgr_unregister(&pdev->dev); >> - goto err_unmap; >> - } >> - >> return 0; >> >> err_unmap: >> @@ -484,7 +484,6 @@ static void altera_cvp_remove(struct pci_dev *pdev) >> struct altera_cvp_conf *conf = mgr->priv; >> u16 cmd; >> >> - driver_remove_file(&altera_cvp_driver.driver, &driver_attr_chkcfg); >> fpga_mgr_unregister(&pdev->dev); >> pci_iounmap(pdev, conf->map); >> pci_release_region(pdev, CVP_BAR); >> -- >> 2.7.4 >> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-08-23 17:54 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-08-22 15:53 [PATCH 0/2] fpga: add attribute groups Alan Tull 2017-08-22 15:53 ` [PATCH 1/2] " Alan Tull 2017-08-22 15:53 ` [PATCH 2/2] fpga: altera-cvp: register attribute groups using ops Alan Tull 2017-08-22 20:23 ` Alan Tull 2017-08-23 17:54 ` Alan Tull
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).