RE: [PATCH v4 06/24] fpga: dfl: adds fpga_cdev_find_port
> On Tue, Feb 13, 2018 at 3:24 AM, Wu Hao wrote: > > Hi Hao, > > > For feature devices, we need a method to find the port dedicated > > to the device. This patch adds a function fpga_cdev_find_port > > for this purpose. e.g FPGA Management Engine (FME) Partial > > Reconfiguration sub feature, it uses this function to find > > dedicated port on the device for PR function implementation. > > OK, that is very clear now, thanks! Ack with the minor doc changes below. > > > > > Signed-off-by: Tim Whisonant > > Signed-off-by: Enno Luebbers > > Signed-off-by: Shiva Rao > > Signed-off-by: Christopher Rauer > > Signed-off-by: Xiao Guangrong > > Signed-off-by: Wu Hao > Acked-by: Alan Tull > > > --- > > v3: s/fpga_for_each_port/fpga_cdev_find_port/ > > move fpga_cdev_find_port to fpga-dfl module. > > v4: improve description in commit message. > > add comments to remind user to put_device after use this function. > > --- > > drivers/fpga/dfl.c | 34 ++ > > drivers/fpga/dfl.h | 20 > > 2 files changed, 54 insertions(+) > > > > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c > > index dcfe5ab..38dc819 100644 > > --- a/drivers/fpga/dfl.c > > +++ b/drivers/fpga/dfl.c > > @@ -783,6 +783,40 @@ void fpga_remove_feature_devs(struct fpga_cdev > *cdev) > > } > > EXPORT_SYMBOL_GPL(fpga_remove_feature_devs); > > > > +/** > > + * __fpga_cdev_find_port - find a port under given container device > > + * > > + * @cdev: container device > > + * @data: data passed to match function > > + * @match: match function used to find specific port from the port device > > list > > + * > > + * Find a port device under container device. This function needs to be > > + * invoked with lock held. > > + * > > + * This function returns NULL if the device doesn't match and non-zero if > > it > > + * does. > > Good but this would be more brief and more specific and more in line > with recommended format in Documentation/kernel-doc-nano-HOWTO.txt: > > * Return: pointer to port's platform device if successful, NULL otherwise. > > > If this function returns non-zero and a reference to the platform > > + * device of port can be obtained. > > This if..then type sentence is not complete (missing the 'then' part). > I see where you might have gotten inspiration for it from core.c. > Probably you could just delete the sentence and all would be clear. Thanks for the review and the suggestion on this, will update the comments to fix it. Hao > > > + * > > + * NOTE: you will need to drop the device reference with put_device() after > use. > > + */ > > Good > > Thanks, > Alan > > > +struct platform_device * > > +__fpga_cdev_find_port(struct fpga_cdev *cdev, void *data, > > + int (*match)(struct platform_device *, void *)) > > +{ > > + struct feature_platform_data *pdata; > > + struct platform_device *port_dev; > > + > > + list_for_each_entry(pdata, &cdev->port_dev_list, node) { > > + port_dev = pdata->dev; > > + > > + if (match(port_dev, data) && get_device(&port_dev->dev)) > > + return port_dev; > > + } > > + > > + return NULL; > > +} > > +EXPORT_SYMBOL_GPL(__fpga_cdev_find_port); > > + > > int fpga_port_id(struct platform_device *pdev) > > { > > void __iomem *base = get_feature_ioaddr_by_id(&pdev->dev, > > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h > > index 9b19399..d6ecda1 100644 > > --- a/drivers/fpga/dfl.h > > +++ b/drivers/fpga/dfl.h > > @@ -356,4 +356,24 @@ struct fpga_cdev { > > struct fpga_cdev *fpga_enumerate_feature_devs(struct fpga_enum_info > *info); > > void fpga_remove_feature_devs(struct fpga_cdev *cdev); > > > > +/* > > + * need to drop the device reference with put_device() after use port > platform > > + * device returned by __fpga_cdev_find_port and fpga_cdev_find_port > functions. > > + */ > > +struct platform_device * > > +__fpga_cdev_find_port(struct fpga_cdev *cdev, void *data, > > + int (*match)(struct platform_device *, void *)); > > + > > +static inline struct platform_device * > > +fpga_cdev_find_port(struct fpga_cdev *cdev, void *data, > > + int (*match)(struct platform_device *, void *)) > > +{ > > + struct platform_device *pdev; > > + > > + mutex_lock(&cdev->lock); > > + pdev = __fpga_cdev_find_port(cdev, data, match); > > + mutex_unlock(&cdev->lock); > > + > > + return pdev; > > +} > > #endif /* __FPGA_DFL_H */ > > -- > > 2.7.4 > >
Re: [PATCH v4 06/24] fpga: dfl: adds fpga_cdev_find_port
Hi Hao, On Tue, Feb 13, 2018 at 05:24:35PM +0800, Wu Hao wrote: > For feature devices, we need a method to find the port dedicated > to the device. This patch adds a function fpga_cdev_find_port > for this purpose. e.g FPGA Management Engine (FME) Partial > Reconfiguration sub feature, it uses this function to find > dedicated port on the device for PR function implementation. > > Signed-off-by: Tim Whisonant > Signed-off-by: Enno Luebbers > Signed-off-by: Shiva Rao > Signed-off-by: Christopher Rauer > Signed-off-by: Xiao Guangrong > Signed-off-by: Wu Hao Acked-by: Moritz Fischer > --- > v3: s/fpga_for_each_port/fpga_cdev_find_port/ > move fpga_cdev_find_port to fpga-dfl module. > v4: improve description in commit message. > add comments to remind user to put_device after use this function. > --- > drivers/fpga/dfl.c | 34 ++ > drivers/fpga/dfl.h | 20 > 2 files changed, 54 insertions(+) > > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c > index dcfe5ab..38dc819 100644 > --- a/drivers/fpga/dfl.c > +++ b/drivers/fpga/dfl.c > @@ -783,6 +783,40 @@ void fpga_remove_feature_devs(struct fpga_cdev *cdev) > } > EXPORT_SYMBOL_GPL(fpga_remove_feature_devs); > > +/** > + * __fpga_cdev_find_port - find a port under given container device > + * > + * @cdev: container device > + * @data: data passed to match function > + * @match: match function used to find specific port from the port device > list > + * > + * Find a port device under container device. This function needs to be > + * invoked with lock held. > + * > + * This function returns NULL if the device doesn't match and non-zero if it > + * does. If this function returns non-zero and a reference to the platform > + * device of port can be obtained. > + * > + * NOTE: you will need to drop the device reference with put_device() after > use. > + */ > +struct platform_device * > +__fpga_cdev_find_port(struct fpga_cdev *cdev, void *data, > + int (*match)(struct platform_device *, void *)) > +{ > + struct feature_platform_data *pdata; > + struct platform_device *port_dev; > + > + list_for_each_entry(pdata, &cdev->port_dev_list, node) { > + port_dev = pdata->dev; > + > + if (match(port_dev, data) && get_device(&port_dev->dev)) > + return port_dev; > + } > + > + return NULL; > +} > +EXPORT_SYMBOL_GPL(__fpga_cdev_find_port); > + > int fpga_port_id(struct platform_device *pdev) > { > void __iomem *base = get_feature_ioaddr_by_id(&pdev->dev, > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h > index 9b19399..d6ecda1 100644 > --- a/drivers/fpga/dfl.h > +++ b/drivers/fpga/dfl.h > @@ -356,4 +356,24 @@ struct fpga_cdev { > struct fpga_cdev *fpga_enumerate_feature_devs(struct fpga_enum_info *info); > void fpga_remove_feature_devs(struct fpga_cdev *cdev); > > +/* > + * need to drop the device reference with put_device() after use port > platform > + * device returned by __fpga_cdev_find_port and fpga_cdev_find_port > functions. > + */ > +struct platform_device * > +__fpga_cdev_find_port(struct fpga_cdev *cdev, void *data, > + int (*match)(struct platform_device *, void *)); > + > +static inline struct platform_device * > +fpga_cdev_find_port(struct fpga_cdev *cdev, void *data, > + int (*match)(struct platform_device *, void *)) > +{ > + struct platform_device *pdev; > + > + mutex_lock(&cdev->lock); > + pdev = __fpga_cdev_find_port(cdev, data, match); > + mutex_unlock(&cdev->lock); > + > + return pdev; > +} > #endif /* __FPGA_DFL_H */ > -- > 2.7.4 > Thanks, Moritz signature.asc Description: PGP signature
Re: [PATCH v4 06/24] fpga: dfl: adds fpga_cdev_find_port
On Tue, Feb 13, 2018 at 3:24 AM, Wu Hao wrote: Hi Hao, > For feature devices, we need a method to find the port dedicated > to the device. This patch adds a function fpga_cdev_find_port > for this purpose. e.g FPGA Management Engine (FME) Partial > Reconfiguration sub feature, it uses this function to find > dedicated port on the device for PR function implementation. OK, that is very clear now, thanks! Ack with the minor doc changes below. > > Signed-off-by: Tim Whisonant > Signed-off-by: Enno Luebbers > Signed-off-by: Shiva Rao > Signed-off-by: Christopher Rauer > Signed-off-by: Xiao Guangrong > Signed-off-by: Wu Hao Acked-by: Alan Tull > --- > v3: s/fpga_for_each_port/fpga_cdev_find_port/ > move fpga_cdev_find_port to fpga-dfl module. > v4: improve description in commit message. > add comments to remind user to put_device after use this function. > --- > drivers/fpga/dfl.c | 34 ++ > drivers/fpga/dfl.h | 20 > 2 files changed, 54 insertions(+) > > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c > index dcfe5ab..38dc819 100644 > --- a/drivers/fpga/dfl.c > +++ b/drivers/fpga/dfl.c > @@ -783,6 +783,40 @@ void fpga_remove_feature_devs(struct fpga_cdev *cdev) > } > EXPORT_SYMBOL_GPL(fpga_remove_feature_devs); > > +/** > + * __fpga_cdev_find_port - find a port under given container device > + * > + * @cdev: container device > + * @data: data passed to match function > + * @match: match function used to find specific port from the port device > list > + * > + * Find a port device under container device. This function needs to be > + * invoked with lock held. > + * > + * This function returns NULL if the device doesn't match and non-zero if it > + * does. Good but this would be more brief and more specific and more in line with recommended format in Documentation/kernel-doc-nano-HOWTO.txt: * Return: pointer to port's platform device if successful, NULL otherwise. > If this function returns non-zero and a reference to the platform > + * device of port can be obtained. This if..then type sentence is not complete (missing the 'then' part). I see where you might have gotten inspiration for it from core.c. Probably you could just delete the sentence and all would be clear. > + * > + * NOTE: you will need to drop the device reference with put_device() after > use. > + */ Good Thanks, Alan > +struct platform_device * > +__fpga_cdev_find_port(struct fpga_cdev *cdev, void *data, > + int (*match)(struct platform_device *, void *)) > +{ > + struct feature_platform_data *pdata; > + struct platform_device *port_dev; > + > + list_for_each_entry(pdata, &cdev->port_dev_list, node) { > + port_dev = pdata->dev; > + > + if (match(port_dev, data) && get_device(&port_dev->dev)) > + return port_dev; > + } > + > + return NULL; > +} > +EXPORT_SYMBOL_GPL(__fpga_cdev_find_port); > + > int fpga_port_id(struct platform_device *pdev) > { > void __iomem *base = get_feature_ioaddr_by_id(&pdev->dev, > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h > index 9b19399..d6ecda1 100644 > --- a/drivers/fpga/dfl.h > +++ b/drivers/fpga/dfl.h > @@ -356,4 +356,24 @@ struct fpga_cdev { > struct fpga_cdev *fpga_enumerate_feature_devs(struct fpga_enum_info *info); > void fpga_remove_feature_devs(struct fpga_cdev *cdev); > > +/* > + * need to drop the device reference with put_device() after use port > platform > + * device returned by __fpga_cdev_find_port and fpga_cdev_find_port > functions. > + */ > +struct platform_device * > +__fpga_cdev_find_port(struct fpga_cdev *cdev, void *data, > + int (*match)(struct platform_device *, void *)); > + > +static inline struct platform_device * > +fpga_cdev_find_port(struct fpga_cdev *cdev, void *data, > + int (*match)(struct platform_device *, void *)) > +{ > + struct platform_device *pdev; > + > + mutex_lock(&cdev->lock); > + pdev = __fpga_cdev_find_port(cdev, data, match); > + mutex_unlock(&cdev->lock); > + > + return pdev; > +} > #endif /* __FPGA_DFL_H */ > -- > 2.7.4 >