| 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
| ||
|
Message-ID: <4d210cf326023c8e4d7bfe6e71a304e68247eedd.camel@intel.com>
Date: Thu, 19 Sep 2024 21:42:53 +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: [RFC PATCH v2 8/9] fpga: dfl: migrate dfl_get_feature_by_id() to
dfl_feature_dev_data
On Tue, 2024-04-23 at 23:16 +0800, Xu Yilun wrote:
> 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.
Thank you for the suggestion; this has been done in the patch "fpga:
dfl: pass feature platform data instead of device as argument".
>
> > {
> > 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.
This has been done in the patch "fpga: dfl: pass feature platform data
instead of device as argument".
Thanks,
Peter
>
> Thanks,
> Yilun
Powered by blists - more mailing lists