[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZifQz/QJvYpFljMv@yilunxu-OptiPlex-7050>
Date: Tue, 23 Apr 2024 23:16:31 +0800
From: Xu Yilun <yilun.xu@...ux.intel.com>
To: Peter Colberg <peter.colberg@...el.com>
Cc: Wu Hao <hao.wu@...el.com>, Tom Rix <trix@...hat.com>,
Moritz Fischer <mdf@...nel.org>, Xu Yilun <yilun.xu@...el.com>,
linux-fpga@...r.kernel.org, linux-kernel@...r.kernel.org,
Russ Weight <russ.weight@...ux.dev>,
Marco Pagani <marpagan@...hat.com>,
Matthew Gerlach <matthew.gerlach@...ux.intel.com>
Subject: Re: [RFC PATCH v2 8/9] fpga: dfl: migrate dfl_get_feature_by_id() to
dfl_feature_dev_data
On Tue, Apr 09, 2024 at 07:39:41PM -0400, Peter Colberg wrote:
> This change separates out most of the symbol name changes required by this
> patch series for the function: dfl_get_feature_by_id(). This is done to
> split a single monolithic change into multiple, smaller patches at the
> request of the maintainer.
>
> Signed-off-by: Peter Colberg <peter.colberg@...el.com>
> ---
> v2:
> - Split monolithic patch into series at request of maintainer
> ---
> drivers/fpga/dfl-afu-error.c | 59 +++++++------
> drivers/fpga/dfl-afu-main.c | 166 ++++++++++++++++++-----------------
> drivers/fpga/dfl-afu.h | 26 +++---
> drivers/fpga/dfl-fme-error.c | 98 +++++++++++----------
> drivers/fpga/dfl-fme-main.c | 18 ++--
> drivers/fpga/dfl-fme-pr.c | 6 +-
> drivers/fpga/dfl.c | 3 +-
> drivers/fpga/dfl.h | 13 ++-
> 8 files changed, 203 insertions(+), 186 deletions(-)
>
> diff --git a/drivers/fpga/dfl-afu-error.c b/drivers/fpga/dfl-afu-error.c
> index ab7be6217368..0f392d1f6d45 100644
> --- a/drivers/fpga/dfl-afu-error.c
> +++ b/drivers/fpga/dfl-afu-error.c
> @@ -28,37 +28,36 @@
> #define ERROR_MASK GENMASK_ULL(63, 0)
>
> /* mask or unmask port errors by the error mask register. */
> -static void __afu_port_err_mask(struct device *dev, bool mask)
> +static void __afu_port_err_mask(struct dfl_feature_dev_data *fdata, bool mask)
Maybe first replace all "struct device *dev" arguments with "struct
dfl_feature_platform data *pdata", then you could do simple
pdata->fdata replacement the same as other patches.
> {
> void __iomem *base;
>
> - base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_ERROR);
> + base = dfl_get_feature_ioaddr_by_id(fdata, PORT_FEATURE_ID_ERROR);
>
> writeq(mask ? ERROR_MASK : 0, base + PORT_ERROR_MASK);
> }
>
> static void afu_port_err_mask(struct device *dev, bool mask)
> {
> - struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
> + struct dfl_feature_dev_data *fdata = to_dfl_feature_dev_data(dev);
>
> - mutex_lock(&pdata->lock);
> - __afu_port_err_mask(dev, mask);
> - mutex_unlock(&pdata->lock);
> + mutex_lock(&fdata->lock);
> + __afu_port_err_mask(fdata, mask);
> + mutex_unlock(&fdata->lock);
> }
>
> /* clear port errors. */
> static int afu_port_err_clear(struct device *dev, u64 err)
> {
> - struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
> - struct platform_device *pdev = to_platform_device(dev);
> + struct dfl_feature_dev_data *fdata = to_dfl_feature_dev_data(dev);
> void __iomem *base_err, *base_hdr;
> int enable_ret = 0, ret = -EBUSY;
> u64 v;
>
> - base_err = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_ERROR);
> - base_hdr = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_HEADER);
> + base_err = dfl_get_feature_ioaddr_by_id(fdata, PORT_FEATURE_ID_ERROR);
> + base_hdr = dfl_get_feature_ioaddr_by_id(fdata, PORT_FEATURE_ID_HEADER);
>
> - mutex_lock(&pdata->lock);
> + mutex_lock(&fdata->lock);
>
> /*
> * clear Port Errors
> @@ -80,12 +79,12 @@ static int afu_port_err_clear(struct device *dev, u64 err)
> }
>
> /* Halt Port by keeping Port in reset */
> - ret = __afu_port_disable(pdev);
> + ret = __afu_port_disable(fdata);
> if (ret)
> goto done;
>
> /* Mask all errors */
> - __afu_port_err_mask(dev, true);
> + __afu_port_err_mask(fdata, true);
>
> /* Clear errors if err input matches with current port errors.*/
> v = readq(base_err + PORT_ERROR);
> @@ -102,28 +101,28 @@ static int afu_port_err_clear(struct device *dev, u64 err)
> }
>
> /* Clear mask */
> - __afu_port_err_mask(dev, false);
> + __afu_port_err_mask(fdata, false);
>
> /* Enable the Port by clearing the reset */
> - enable_ret = __afu_port_enable(pdev);
> + enable_ret = __afu_port_enable(fdata);
>
> done:
> - mutex_unlock(&pdata->lock);
> + mutex_unlock(&fdata->lock);
> return enable_ret ? enable_ret : ret;
> }
>
> static ssize_t errors_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> - struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
> + struct dfl_feature_dev_data *fdata = to_dfl_feature_dev_data(dev);
> void __iomem *base;
> u64 error;
>
> - base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_ERROR);
> + base = dfl_get_feature_ioaddr_by_id(fdata, PORT_FEATURE_ID_ERROR);
>
> - mutex_lock(&pdata->lock);
> + mutex_lock(&fdata->lock);
> error = readq(base + PORT_ERROR);
> - mutex_unlock(&pdata->lock);
> + mutex_unlock(&fdata->lock);
>
> return sprintf(buf, "0x%llx\n", (unsigned long long)error);
> }
> @@ -146,15 +145,15 @@ static DEVICE_ATTR_RW(errors);
> static ssize_t first_error_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
> + struct dfl_feature_dev_data *fdata = to_dfl_feature_dev_data(dev);
> void __iomem *base;
> u64 error;
>
> - base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_ERROR);
> + base = dfl_get_feature_ioaddr_by_id(fdata, PORT_FEATURE_ID_ERROR);
>
> - mutex_lock(&pdata->lock);
> + mutex_lock(&fdata->lock);
> error = readq(base + PORT_FIRST_ERROR);
> - mutex_unlock(&pdata->lock);
> + mutex_unlock(&fdata->lock);
>
> return sprintf(buf, "0x%llx\n", (unsigned long long)error);
> }
> @@ -164,16 +163,16 @@ static ssize_t first_malformed_req_show(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> {
> - struct dfl_feature_platform_data *pdata = dev_get_platdata(dev);
> + struct dfl_feature_dev_data *fdata = to_dfl_feature_dev_data(dev);
> void __iomem *base;
> u64 req0, req1;
>
> - base = dfl_get_feature_ioaddr_by_id(dev, PORT_FEATURE_ID_ERROR);
> + base = dfl_get_feature_ioaddr_by_id(fdata, PORT_FEATURE_ID_ERROR);
>
> - mutex_lock(&pdata->lock);
> + mutex_lock(&fdata->lock);
> req0 = readq(base + PORT_MALFORMED_REQ0);
> req1 = readq(base + PORT_MALFORMED_REQ1);
> - mutex_unlock(&pdata->lock);
> + mutex_unlock(&fdata->lock);
>
> return sprintf(buf, "0x%016llx%016llx\n",
> (unsigned long long)req1, (unsigned long long)req0);
> @@ -191,12 +190,14 @@ static umode_t port_err_attrs_visible(struct kobject *kobj,
> struct attribute *attr, int n)
> {
> struct device *dev = kobj_to_dev(kobj);
> + struct dfl_feature_dev_data *fdata;
>
> + fdata = to_dfl_feature_dev_data(dev);
> /*
> * sysfs entries are visible only if related private feature is
> * enumerated.
> */
> - if (!dfl_get_feature_by_id(dev, PORT_FEATURE_ID_ERROR))
> + if (!dfl_get_feature_by_id(fdata, PORT_FEATURE_ID_ERROR))
> return 0;
>
> return attr->mode;
> diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
> index 61868cdd5b0b..42928cc7e42b 100644
> --- a/drivers/fpga/dfl-afu-main.c
> +++ b/drivers/fpga/dfl-afu-main.c
> @@ -26,7 +26,7 @@
>
> /**
> * __afu_port_enable - enable a port by clear reset
> - * @pdev: port platform device.
> + * @fdata: port feature dev data.
> *
> * Enable Port by clear the port soft reset bit, which is set by default.
> * The AFU is unable to respond to any MMIO access while in reset.
> @@ -35,18 +35,17 @@
> *
> * The caller needs to hold lock for protection.
> */
> -int __afu_port_enable(struct platform_device *pdev)
> +int __afu_port_enable(struct dfl_feature_dev_data *fdata)
Same suggestion. Replace "struct platform_device *pdev" with "struct
dfl_feature_platform_data *pdata" first. Then do massive pdata->fdata
replacement.
Thanks,
Yilun
Powered by blists - more mailing lists