diff mbox series

[v3,2/9] fpga: dfl: omit unneeded null pointer check from {afu,fme}_open()

Message ID 20240919203430.1278067-3-peter.colberg@intel.com (mailing list archive)
State New
Headers show
Series fpga: dfl: fix kernel warning on port release/assign for SRIOV | expand

Commit Message

Colberg, Peter Sept. 19, 2024, 8:34 p.m. UTC 
The feature platform device is guaranteed to have an associated platform
data. Refactor dfl_fpga_inode_to_feature_dev_data() to directly return
the platform data and retrieve the device from the data.

Signed-off-by: Peter Colberg <peter.colberg@intel.com>
Reviewed-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
---
 drivers/fpga/dfl-afu-main.c | 8 ++------
 drivers/fpga/dfl-fme-main.c | 7 ++-----
 drivers/fpga/dfl.h          | 6 +++---
 3 files changed, 7 insertions(+), 14 deletions(-)

Comments

Xu Yilun Sept. 24, 2024, 6:28 a.m. UTC | #1 
On Thu, Sep 19, 2024 at 04:34:23PM -0400, Peter Colberg wrote:
> The feature platform device is guaranteed to have an associated platform
> data. Refactor dfl_fpga_inode_to_feature_dev_data() to directly return
> the platform data and retrieve the device from the data.

The code is good. But please elaborate on the purpose of these
intermediate change.

Thanks,
Yilun
Xu Yilun Sept. 24, 2024, 6:41 a.m. UTC | #2 
On Thu, Sep 19, 2024 at 04:34:23PM -0400, Peter Colberg wrote:
> The feature platform device is guaranteed to have an associated platform
> data. Refactor dfl_fpga_inode_to_feature_dev_data() to directly return
> the platform data and retrieve the device from the data.

These changelog better describes the change, but the short log does not.
Please update.

Thanks,
Yilun

> 
> Signed-off-by: Peter Colberg <peter.colberg@intel.com>
> Reviewed-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> ---
>  drivers/fpga/dfl-afu-main.c | 8 ++------
>  drivers/fpga/dfl-fme-main.c | 7 ++-----
>  drivers/fpga/dfl.h          | 6 +++---
>  3 files changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
> index 6b97c073849e..6125e2faada8 100644
> --- a/drivers/fpga/dfl-afu-main.c
> +++ b/drivers/fpga/dfl-afu-main.c
> @@ -595,14 +595,10 @@ static struct dfl_feature_driver port_feature_drvs[] = {
>  
>  static int afu_open(struct inode *inode, struct file *filp)
>  {
> -	struct platform_device *fdev = dfl_fpga_inode_to_feature_dev(inode);
> -	struct dfl_feature_platform_data *pdata;
> +	struct dfl_feature_platform_data *pdata = dfl_fpga_inode_to_feature_dev_data(inode);
> +	struct platform_device *fdev = pdata->dev;
>  	int ret;
>  
> -	pdata = dev_get_platdata(&fdev->dev);
> -	if (WARN_ON(!pdata))
> -		return -ENODEV;
> -
>  	mutex_lock(&pdata->lock);
>  	ret = dfl_feature_dev_use_begin(pdata, filp->f_flags & O_EXCL);
>  	if (!ret) {
> diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
> index 864924f68f5e..480a187289bb 100644
> --- a/drivers/fpga/dfl-fme-main.c
> +++ b/drivers/fpga/dfl-fme-main.c
> @@ -598,13 +598,10 @@ static long fme_ioctl_check_extension(struct dfl_feature_platform_data *pdata,
>  
>  static int fme_open(struct inode *inode, struct file *filp)
>  {
> -	struct platform_device *fdev = dfl_fpga_inode_to_feature_dev(inode);
> -	struct dfl_feature_platform_data *pdata = dev_get_platdata(&fdev->dev);
> +	struct dfl_feature_platform_data *pdata = dfl_fpga_inode_to_feature_dev_data(inode);
> +	struct platform_device *fdev = pdata->dev;
>  	int ret;
>  
> -	if (WARN_ON(!pdata))
> -		return -ENODEV;
> -
>  	mutex_lock(&pdata->lock);
>  	ret = dfl_feature_dev_use_begin(pdata, filp->f_flags & O_EXCL);
>  	if (!ret) {
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index 5063d73b0d82..2285215f444e 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -398,14 +398,14 @@ int dfl_fpga_dev_ops_register(struct platform_device *pdev,
>  			      struct module *owner);
>  void dfl_fpga_dev_ops_unregister(struct platform_device *pdev);
>  
> -static inline
> -struct platform_device *dfl_fpga_inode_to_feature_dev(struct inode *inode)
> +static inline struct dfl_feature_platform_data *
> +dfl_fpga_inode_to_feature_dev_data(struct inode *inode)
>  {
>  	struct dfl_feature_platform_data *pdata;
>  
>  	pdata = container_of(inode->i_cdev, struct dfl_feature_platform_data,
>  			     cdev);
> -	return pdata->dev;
> +	return pdata;
>  }
>  
>  #define dfl_fpga_dev_for_each_feature(pdata, feature)			    \
> -- 
> 2.46.1
> 
>
Colberg, Peter Oct. 25, 2024, 10:39 p.m. UTC | #3 
On Tue, 2024-09-24 at 14:28 +0800, Xu Yilun wrote:
> On Thu, Sep 19, 2024 at 04:34:23PM -0400, Peter Colberg wrote:
> > The feature platform device is guaranteed to have an associated platform
> > data. Refactor dfl_fpga_inode_to_feature_dev_data() to directly return
> > the platform data and retrieve the device from the data.
> 
> The code is good. But please elaborate on the purpose of these
> intermediate change.
> 
> Thanks,
> Yilun

This has been done in the revised patch "fpga: dfl: return platform
data from dfl_fpga_inode_to_feature_dev_data()".

Thanks,
Peter
Colberg, Peter Oct. 25, 2024, 10:40 p.m. UTC | #4 
On Tue, 2024-09-24 at 14:41 +0800, Xu Yilun wrote:
> On Thu, Sep 19, 2024 at 04:34:23PM -0400, Peter Colberg wrote:
> > The feature platform device is guaranteed to have an associated platform
> > data. Refactor dfl_fpga_inode_to_feature_dev_data() to directly return
> > the platform data and retrieve the device from the data.
> 
> These changelog better describes the change, but the short log does not.
> Please update.
> 
> Thanks,
> Yilun

The subject has been revised in the patch "fpga: dfl: return platform
data from dfl_fpga_inode_to_feature_dev_data()".

Thanks,
Peter

> 
> > 
> > Signed-off-by: Peter Colberg <peter.colberg@intel.com>
> > Reviewed-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > ---
> >  drivers/fpga/dfl-afu-main.c | 8 ++------
> >  drivers/fpga/dfl-fme-main.c | 7 ++-----
> >  drivers/fpga/dfl.h          | 6 +++---
> >  3 files changed, 7 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
> > index 6b97c073849e..6125e2faada8 100644
> > --- a/drivers/fpga/dfl-afu-main.c
> > +++ b/drivers/fpga/dfl-afu-main.c
> > @@ -595,14 +595,10 @@ static struct dfl_feature_driver port_feature_drvs[] = {
> >  
> >  static int afu_open(struct inode *inode, struct file *filp)
> >  {
> > -	struct platform_device *fdev = dfl_fpga_inode_to_feature_dev(inode);
> > -	struct dfl_feature_platform_data *pdata;
> > +	struct dfl_feature_platform_data *pdata = dfl_fpga_inode_to_feature_dev_data(inode);
> > +	struct platform_device *fdev = pdata->dev;
> >  	int ret;
> >  
> > -	pdata = dev_get_platdata(&fdev->dev);
> > -	if (WARN_ON(!pdata))
> > -		return -ENODEV;
> > -
> >  	mutex_lock(&pdata->lock);
> >  	ret = dfl_feature_dev_use_begin(pdata, filp->f_flags & O_EXCL);
> >  	if (!ret) {
> > diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
> > index 864924f68f5e..480a187289bb 100644
> > --- a/drivers/fpga/dfl-fme-main.c
> > +++ b/drivers/fpga/dfl-fme-main.c
> > @@ -598,13 +598,10 @@ static long fme_ioctl_check_extension(struct dfl_feature_platform_data *pdata,
> >  
> >  static int fme_open(struct inode *inode, struct file *filp)
> >  {
> > -	struct platform_device *fdev = dfl_fpga_inode_to_feature_dev(inode);
> > -	struct dfl_feature_platform_data *pdata = dev_get_platdata(&fdev->dev);
> > +	struct dfl_feature_platform_data *pdata = dfl_fpga_inode_to_feature_dev_data(inode);
> > +	struct platform_device *fdev = pdata->dev;
> >  	int ret;
> >  
> > -	if (WARN_ON(!pdata))
> > -		return -ENODEV;
> > -
> >  	mutex_lock(&pdata->lock);
> >  	ret = dfl_feature_dev_use_begin(pdata, filp->f_flags & O_EXCL);
> >  	if (!ret) {
> > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> > index 5063d73b0d82..2285215f444e 100644
> > --- a/drivers/fpga/dfl.h
> > +++ b/drivers/fpga/dfl.h
> > @@ -398,14 +398,14 @@ int dfl_fpga_dev_ops_register(struct platform_device *pdev,
> >  			      struct module *owner);
> >  void dfl_fpga_dev_ops_unregister(struct platform_device *pdev);
> >  
> > -static inline
> > -struct platform_device *dfl_fpga_inode_to_feature_dev(struct inode *inode)
> > +static inline struct dfl_feature_platform_data *
> > +dfl_fpga_inode_to_feature_dev_data(struct inode *inode)
> >  {
> >  	struct dfl_feature_platform_data *pdata;
> >  
> >  	pdata = container_of(inode->i_cdev, struct dfl_feature_platform_data,
> >  			     cdev);
> > -	return pdata->dev;
> > +	return pdata;
> >  }
> >  
> >  #define dfl_fpga_dev_for_each_feature(pdata, feature)			    \
> > -- 
> > 2.46.1
> > 
> >
diff mbox series

Patch

diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
index 6b97c073849e..6125e2faada8 100644
--- a/drivers/fpga/dfl-afu-main.c
+++ b/drivers/fpga/dfl-afu-main.c
@@ -595,14 +595,10 @@  static struct dfl_feature_driver port_feature_drvs[] = {
 
 static int afu_open(struct inode *inode, struct file *filp)
 {
-	struct platform_device *fdev = dfl_fpga_inode_to_feature_dev(inode);
-	struct dfl_feature_platform_data *pdata;
+	struct dfl_feature_platform_data *pdata = dfl_fpga_inode_to_feature_dev_data(inode);
+	struct platform_device *fdev = pdata->dev;
 	int ret;
 
-	pdata = dev_get_platdata(&fdev->dev);
-	if (WARN_ON(!pdata))
-		return -ENODEV;
-
 	mutex_lock(&pdata->lock);
 	ret = dfl_feature_dev_use_begin(pdata, filp->f_flags & O_EXCL);
 	if (!ret) {
diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
index 864924f68f5e..480a187289bb 100644
--- a/drivers/fpga/dfl-fme-main.c
+++ b/drivers/fpga/dfl-fme-main.c
@@ -598,13 +598,10 @@  static long fme_ioctl_check_extension(struct dfl_feature_platform_data *pdata,
 
 static int fme_open(struct inode *inode, struct file *filp)
 {
-	struct platform_device *fdev = dfl_fpga_inode_to_feature_dev(inode);
-	struct dfl_feature_platform_data *pdata = dev_get_platdata(&fdev->dev);
+	struct dfl_feature_platform_data *pdata = dfl_fpga_inode_to_feature_dev_data(inode);
+	struct platform_device *fdev = pdata->dev;
 	int ret;
 
-	if (WARN_ON(!pdata))
-		return -ENODEV;
-
 	mutex_lock(&pdata->lock);
 	ret = dfl_feature_dev_use_begin(pdata, filp->f_flags & O_EXCL);
 	if (!ret) {
diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 5063d73b0d82..2285215f444e 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -398,14 +398,14 @@  int dfl_fpga_dev_ops_register(struct platform_device *pdev,
 			      struct module *owner);
 void dfl_fpga_dev_ops_unregister(struct platform_device *pdev);
 
-static inline
-struct platform_device *dfl_fpga_inode_to_feature_dev(struct inode *inode)
+static inline struct dfl_feature_platform_data *
+dfl_fpga_inode_to_feature_dev_data(struct inode *inode)
 {
 	struct dfl_feature_platform_data *pdata;
 
 	pdata = container_of(inode->i_cdev, struct dfl_feature_platform_data,
 			     cdev);
-	return pdata->dev;
+	return pdata;
 }
 
 #define dfl_fpga_dev_for_each_feature(pdata, feature)			    \