linux-kernel - Re: [PATCH v3 6/9] fpga: dfl: factor out feature data creation from build_info_commit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2cb783bc7d3a85226d8dfbcded551ef5b93582a6.camel@intel.com>
Date: Fri, 25 Oct 2024 22:43:28 +0000
From: "Colberg, Peter" <peter.colberg@...el.com>
To: "yilun.xu@...ux.intel.com" <yilun.xu@...ux.intel.com>
CC: "Xu, Yilun" <yilun.xu@...el.com>, "linux-fpga@...r.kernel.org"
	<linux-fpga@...r.kernel.org>, "mdf@...nel.org" <mdf@...nel.org>, "Wu, Hao"
	<hao.wu@...el.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "russ.weight@...ux.dev"
	<russ.weight@...ux.dev>, "Pagani, Marco" <marpagan@...hat.com>,
	"trix@...hat.com" <trix@...hat.com>, "matthew.gerlach@...ux.intel.com"
	<matthew.gerlach@...ux.intel.com>
Subject: Re: [PATCH v3 6/9] fpga: dfl: factor out feature data creation from
 build_info_commit_dev()

On Tue, 2024-09-24 at 15:15 +0800, Xu Yilun wrote:
> On Thu, Sep 19, 2024 at 04:34:27PM -0400, Peter Colberg wrote:
> > Add a separate function binfo_create_feature_dev_data() which allocates
> > and populates the feature platform data, and call the function from
> > build_info_commit_dev() which registers the feature platform device.
> > 
> > Signed-off-by: Peter Colberg <peter.colberg@...el.com>
> > Reviewed-by: Matthew Gerlach <matthew.gerlach@...ux.intel.com>
> > ---
> >  drivers/fpga/dfl.c | 74 ++++++++++++++++++++++++++--------------------
> >  1 file changed, 42 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> > index 4c79d433d216..e644eb9fde39 100644
> > --- a/drivers/fpga/dfl.c
> > +++ b/drivers/fpga/dfl.c
> > @@ -749,12 +749,8 @@ static void dfl_fpga_cdev_add_port_data(struct dfl_fpga_cdev *cdev,
> >  	mutex_unlock(&cdev->lock);
> >  }
> >  
> > -/*
> > - * register current feature device, it is called when we need to switch to
> > - * another feature parsing or we have parsed all features on given device
> > - * feature list.
> > - */
> > -static int build_info_commit_dev(struct build_feature_devs_info *binfo)
> > +static struct dfl_feature_platform_data *
> > +binfo_create_feature_dev_data(struct build_feature_devs_info *binfo)
> >  {
> >  	struct platform_device *fdev = binfo->feature_dev;
> >  	struct dfl_feature_platform_data *pdata;
> > @@ -764,7 +760,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
> >  
> >  	type = feature_dev_id_type(fdev);
> >  	if (WARN_ON_ONCE(type >= DFL_ID_MAX))
> > -		return -EINVAL;
> > +		return ERR_PTR(-EINVAL);
> >  
> >  	/*
> >  	 * we do not need to care for the memory which is associated with
> > @@ -774,7 +770,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
> >  	 */
> >  	pdata = kzalloc(struct_size(pdata, features, binfo->feature_num), GFP_KERNEL);
> >  	if (!pdata)
> > -		return -ENOMEM;
> > +		return ERR_PTR(-ENOMEM);
> >  
> >  	pdata->dev = fdev;
> >  	pdata->num = binfo->feature_num;
> > @@ -799,7 +795,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
> >  	fdev->resource = kcalloc(binfo->feature_num, sizeof(*fdev->resource),
> >  				 GFP_KERNEL);
> >  	if (!fdev->resource)
> > -		return -ENOMEM;
> > +		return ERR_PTR(-ENOMEM);
> >  
> >  	/* fill features and resource information for feature dev */
> >  	list_for_each_entry_safe(finfo, p, &binfo->sub_features, node) {
> > @@ -818,7 +814,7 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
> >  						       finfo->params, finfo->param_size,
> >  						       GFP_KERNEL);
> >  			if (!feature->params)
> > -				return -ENOMEM;
> > +				return ERR_PTR(-ENOMEM);
> >  
> >  			feature->param_size = finfo->param_size;
> >  		}
> > @@ -834,8 +830,10 @@ static int build_info_commit_dev(struct build_feature_devs_info *binfo)
> >  			feature->ioaddr =
> >  				devm_ioremap_resource(binfo->dev,
> >  						      &finfo->mmio_res);
> > -			if (IS_ERR(feature->ioaddr))
> > -				return PTR_ERR(feature->ioaddr);
> > +			if (IS_ERR(feature->ioaddr)) {
> > +				ret = PTR_ERR(feature->ioaddr);
> > +				return ERR_PTR(ret);
> 
> Why change the type back and forth?
> 
>   return feature->ioaddr;
> 
> is it OK?
> 
> Thanks,
> Yilun

Yes, this has been done in the revised patch "fpga: dfl: factor out
feature data creation from build_info_commit_dev()".

Thanks,
Peter