Re: [PATCH v6 3/4] fpga: dfl: add basic support for DFHv1

From: Xu Yilun <yilun.xu@intel.com>
To: matthew.gerlach@linux.intel.com
Cc: hao.wu@intel.com, russell.h.weight@intel.com,
	basheer.ahmed.muddebihal@intel.com, trix@redhat.com,
	mdf@kernel.org, linux-fpga@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	tianfei.zhang@intel.com, corbet@lwn.net,
	gregkh@linuxfoundation.org, linux-serial@vger.kernel.org,
	jirislaby@kernel.org, geert+renesas@glider.be,
	andriy.shevchenko@linux.intel.com,
	niklas.soderlund+renesas@ragnatech.se, macro@orcam.me.uk,
	johan@kernel.org, lukas@wunner.de, ilpo.jarvinen@linux.intel.com,
	marpagan@redhat.com
Subject: Re: [PATCH v6 3/4] fpga: dfl: add basic support for DFHv1
Date: Tue, 13 Dec 2022 13:26:20 +0800	[thread overview]
Message-ID: <Y5gM/L3wufo0/m0y@yilunxu-OptiPlex-7050> (raw)
In-Reply-To: <20221209214523.3484193-4-matthew.gerlach@linux.intel.com>

On 2022-12-09 at 13:45:22 -0800, matthew.gerlach@linux.intel.com wrote:
> From: Matthew Gerlach <matthew.gerlach@linux.intel.com>

[...]

>  /*
>   * when create sub feature instances, for private features, it doesn't need
>   * to provide resource size and feature id as they could be read from DFH
> @@ -1023,39 +1125,69 @@ static int
>  create_feature_instance(struct build_feature_devs_info *binfo,
>  			resource_size_t ofst, resource_size_t size, u16 fid)
>  {
> -	unsigned int irq_base, nr_irqs;
>  	struct dfl_feature_info *finfo;
> +	resource_size_t start, end;
> +	int dfh_psize = 0;
>  	u8 revision = 0;
> +	u64 v, addr_off;
> +	u8 dfh_ver = 0;
>  	int ret;
> -	u64 v;
>  
>  	if (fid != FEATURE_ID_AFU) {
>  		v = readq(binfo->ioaddr + ofst);
>  		revision = FIELD_GET(DFH_REVISION, v);
> -
> +		dfh_ver = FIELD_GET(DFH_VERSION, v);
>  		/* read feature size and id if inputs are invalid */
>  		size = size ? size : feature_size(v);
>  		fid = fid ? fid : feature_id(v);
> +		if (dfh_ver == 1) {
> +			dfh_psize = dfh_get_psize(binfo->ioaddr + ofst, size);
> +			if (dfh_psize < 0) {
> +				dev_err(binfo->dev,
> +					"failed to read size of DFHv1 parameters %d\n",
> +					dfh_psize);
> +				return dfh_psize;
> +			}
> +			dev_dbg(binfo->dev, "dfhv1_psize %d\n", dfh_psize);
> +		}
>  	}
>  
>  	if (binfo->len - ofst < size)
>  		return -EINVAL;
>  
> -	ret = parse_feature_irqs(binfo, ofst, fid, &irq_base, &nr_irqs);
> -	if (ret)
> -		return ret;
> -
> -	finfo = kzalloc(sizeof(*finfo), GFP_KERNEL);
> +	finfo = kzalloc(sizeof(*finfo) + dfh_psize, GFP_KERNEL);

Please use size_add(). See Documentation/process/deprecated.rst for
details.

Others look good to me.

Yilun