From: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
To: "Uwe Kleine-König" <uwe@kleine-koenig.org>
Cc: "Bjorn Helgaas" <helgaas@kernel.org>,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
linux-pci@vger.kernel.org, kernel@pengutronix.de,
"Herbert Xu" <herbert@gondor.apana.org.au>,
"David S. Miller" <davem@davemloft.net>,
"Tomaszx Kowalik" <tomaszx.kowalik@intel.com>,
"Fiona Trahe" <fiona.trahe@intel.com>,
"Marco Chiappero" <marco.chiappero@intel.com>,
"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
"Wojciech Ziemba" <wojciech.ziemba@intel.com>,
"Jack Xu" <jack.xu@intel.com>,
qat-linux@intel.com, linux-crypto@vger.kernel.org
Subject: Re: [PATCH v4 6/8] crypto: qat - simplify adf_enable_aer()
Date: Tue, 28 Sep 2021 12:11:38 +0100 [thread overview]
Message-ID: <YVL4aoKjUT2kvHip@silpixa00400314> (raw)
In-Reply-To: <20210927204326.612555-7-uwe@kleine-koenig.org>
Hi Uwe,
On Mon, Sep 27, 2021 at 10:43:24PM +0200, Uwe Kleine-König wrote:
> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>
> A struct pci_driver is supposed to be constant and assigning .err_handler
> once per bound device isn't really sensible. Also as the function returns
> zero unconditionally let it return no value instead and simplify the
> callers accordingly.
>
> As a side effect this removes one user of struct pci_dev::driver. This
> member is planned to be removed.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
this patch does not build.
drivers/crypto/qat/qat_c3xxx/adf_drv.c:36:24: error: ‘adf_err_handler’ undeclared here (not in a function)
36 | .err_handler = adf_err_handler,
| ^~~~~~~~~~~~~~~
drivers/crypto/qat/qat_4xxx/adf_drv.c:303:24: error: ‘adf_err_handler’ undeclared here (not in a function)
303 | .err_handler = adf_err_handler,
| ^~~~~~~~~~~~~~~
drivers/crypto/qat/qat_c62x/adf_drv.c:36:24: error: ‘adf_err_handler’ undeclared here (not in a function)
36 | .err_handler = adf_err_handler,
| ^~~~~~~~~~~~~~~
drivers/crypto/qat/qat_dh895xcc/adf_drv.c:36:24: error: ‘adf_err_handler’ undeclared here (not in a function)
36 | .err_handler = adf_err_handler,
| ^~~~~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:277: drivers/crypto/qat/qat_c3xxx/adf_drv.o] Error 1
Below is an updated version of your patch that fixes the issues.
After fixing the patch you can add:
Acked-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Regards,
GC
----8<----
From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
Date: Mon, 27 Sep 2021 22:43:24 +0200
Subject: [PATCH] crypto: qat - simplify adf_enable_aer()
A struct pci_driver is supposed to be constant and assigning .err_handler
once per bound device isn't really sensible. Also as the function returns
zero unconditionally let it return no value instead and simplify the
callers accordingly.
As a side effect this removes one user of struct pci_dev::driver. This
member is planned to be removed.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/crypto/qat/qat_4xxx/adf_drv.c | 7 ++-----
drivers/crypto/qat/qat_c3xxx/adf_drv.c | 7 ++-----
drivers/crypto/qat/qat_c62x/adf_drv.c | 7 ++-----
drivers/crypto/qat/qat_common/adf_aer.c | 10 +++-------
drivers/crypto/qat/qat_common/adf_common_drv.h | 4 +++-
drivers/crypto/qat/qat_dh895xcc/adf_drv.c | 7 ++-----
6 files changed, 14 insertions(+), 28 deletions(-)
diff --git a/drivers/crypto/qat/qat_4xxx/adf_drv.c b/drivers/crypto/qat/qat_4xxx/adf_drv.c
index 359fb7989dfb..71ef065914b2 100644
--- a/drivers/crypto/qat/qat_4xxx/adf_drv.c
+++ b/drivers/crypto/qat/qat_4xxx/adf_drv.c
@@ -247,11 +247,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
pci_set_master(pdev);
- if (adf_enable_aer(accel_dev)) {
- dev_err(&pdev->dev, "Failed to enable aer.\n");
- ret = -EFAULT;
- goto out_err;
- }
+ adf_enable_aer(accel_dev);
if (pci_save_state(pdev)) {
dev_err(&pdev->dev, "Failed to save pci state.\n");
@@ -304,6 +300,7 @@ static struct pci_driver adf_driver = {
.probe = adf_probe,
.remove = adf_remove,
.sriov_configure = adf_sriov_configure,
+ .err_handler = &adf_err_handler,
};
module_pci_driver(adf_driver);
diff --git a/drivers/crypto/qat/qat_c3xxx/adf_drv.c b/drivers/crypto/qat/qat_c3xxx/adf_drv.c
index cc6e75dc60de..2aef0bb791df 100644
--- a/drivers/crypto/qat/qat_c3xxx/adf_drv.c
+++ b/drivers/crypto/qat/qat_c3xxx/adf_drv.c
@@ -33,6 +33,7 @@ static struct pci_driver adf_driver = {
.probe = adf_probe,
.remove = adf_remove,
.sriov_configure = adf_sriov_configure,
+ .err_handler = &adf_err_handler,
};
static void adf_cleanup_pci_dev(struct adf_accel_dev *accel_dev)
@@ -192,11 +193,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
}
pci_set_master(pdev);
- if (adf_enable_aer(accel_dev)) {
- dev_err(&pdev->dev, "Failed to enable aer\n");
- ret = -EFAULT;
- goto out_err_free_reg;
- }
+ adf_enable_aer(accel_dev);
if (pci_save_state(pdev)) {
dev_err(&pdev->dev, "Failed to save pci state\n");
diff --git a/drivers/crypto/qat/qat_c62x/adf_drv.c b/drivers/crypto/qat/qat_c62x/adf_drv.c
index bf251dfe74b3..56163083f161 100644
--- a/drivers/crypto/qat/qat_c62x/adf_drv.c
+++ b/drivers/crypto/qat/qat_c62x/adf_drv.c
@@ -33,6 +33,7 @@ static struct pci_driver adf_driver = {
.probe = adf_probe,
.remove = adf_remove,
.sriov_configure = adf_sriov_configure,
+ .err_handler = &adf_err_handler,
};
static void adf_cleanup_pci_dev(struct adf_accel_dev *accel_dev)
@@ -192,11 +193,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
}
pci_set_master(pdev);
- if (adf_enable_aer(accel_dev)) {
- dev_err(&pdev->dev, "Failed to enable aer\n");
- ret = -EFAULT;
- goto out_err_free_reg;
- }
+ adf_enable_aer(accel_dev);
if (pci_save_state(pdev)) {
dev_err(&pdev->dev, "Failed to save pci state\n");
diff --git a/drivers/crypto/qat/qat_common/adf_aer.c b/drivers/crypto/qat/qat_common/adf_aer.c
index ed3e40bc56eb..fe9bb2f3536a 100644
--- a/drivers/crypto/qat/qat_common/adf_aer.c
+++ b/drivers/crypto/qat/qat_common/adf_aer.c
@@ -166,11 +166,12 @@ static void adf_resume(struct pci_dev *pdev)
dev_info(&pdev->dev, "Device is up and running\n");
}
-static const struct pci_error_handlers adf_err_handler = {
+const struct pci_error_handlers adf_err_handler = {
.error_detected = adf_error_detected,
.slot_reset = adf_slot_reset,
.resume = adf_resume,
};
+EXPORT_SYMBOL_GPL(adf_err_handler);
/**
* adf_enable_aer() - Enable Advance Error Reporting for acceleration device
@@ -179,17 +180,12 @@ static const struct pci_error_handlers adf_err_handler = {
* Function enables PCI Advance Error Reporting for the
* QAT acceleration device accel_dev.
* To be used by QAT device specific drivers.
- *
- * Return: 0 on success, error code otherwise.
*/
-int adf_enable_aer(struct adf_accel_dev *accel_dev)
+void adf_enable_aer(struct adf_accel_dev *accel_dev)
{
struct pci_dev *pdev = accel_to_pci_dev(accel_dev);
- struct pci_driver *pdrv = pdev->driver;
- pdrv->err_handler = &adf_err_handler;
pci_enable_pcie_error_reporting(pdev);
- return 0;
}
EXPORT_SYMBOL_GPL(adf_enable_aer);
diff --git a/drivers/crypto/qat/qat_common/adf_common_drv.h b/drivers/crypto/qat/qat_common/adf_common_drv.h
index 4261749fae8d..2141e0b22315 100644
--- a/drivers/crypto/qat/qat_common/adf_common_drv.h
+++ b/drivers/crypto/qat/qat_common/adf_common_drv.h
@@ -95,7 +95,9 @@ void adf_ae_fw_release(struct adf_accel_dev *accel_dev);
int adf_ae_start(struct adf_accel_dev *accel_dev);
int adf_ae_stop(struct adf_accel_dev *accel_dev);
-int adf_enable_aer(struct adf_accel_dev *accel_dev);
+extern const struct pci_error_handlers adf_err_handler;
+
+void adf_enable_aer(struct adf_accel_dev *accel_dev);
void adf_disable_aer(struct adf_accel_dev *accel_dev);
void adf_reset_sbr(struct adf_accel_dev *accel_dev);
void adf_reset_flr(struct adf_accel_dev *accel_dev);
diff --git a/drivers/crypto/qat/qat_dh895xcc/adf_drv.c b/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
index 3976a81bd99b..acca56752aa0 100644
--- a/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
+++ b/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
@@ -33,6 +33,7 @@ static struct pci_driver adf_driver = {
.probe = adf_probe,
.remove = adf_remove,
.sriov_configure = adf_sriov_configure,
+ .err_handler = &adf_err_handler,
};
static void adf_cleanup_pci_dev(struct adf_accel_dev *accel_dev)
@@ -192,11 +193,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
}
pci_set_master(pdev);
- if (adf_enable_aer(accel_dev)) {
- dev_err(&pdev->dev, "Failed to enable aer\n");
- ret = -EFAULT;
- goto out_err_free_reg;
- }
+ adf_enable_aer(accel_dev);
if (pci_save_state(pdev)) {
dev_err(&pdev->dev, "Failed to save pci state\n");
--
2.31.1
next prev parent reply other threads:[~2021-09-28 11:12 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-27 20:43 [PATCH v4 0/8] PCI: Drop duplicated tracking of a pci_dev's bound driver Uwe Kleine-König
2021-09-27 20:43 ` [PATCH v4 1/8] PCI: Simplify pci_device_remove() Uwe Kleine-König
2021-09-27 20:43 ` [PATCH v4 2/8] PCI: Drop useless check from pci_device_probe() Uwe Kleine-König
2021-09-27 20:43 ` [PATCH v4 3/8] xen/pci: Drop some checks that are always true Uwe Kleine-König
2021-09-27 20:43 ` [PATCH v4 4/8] PCI: replace pci_dev::driver usage that gets the driver name Uwe Kleine-König
2021-09-28 8:28 ` Kalle Valo
2021-09-28 10:01 ` Simon Horman
2021-09-28 10:31 ` Uwe Kleine-König
2021-09-29 8:05 ` Simon Horman
2021-09-29 9:04 ` Uwe Kleine-König
2021-09-28 17:17 ` Bjorn Helgaas
2021-09-28 19:29 ` Uwe Kleine-König
2021-09-28 20:08 ` Bjorn Helgaas
2021-09-27 20:43 ` [PATCH v4 5/8] scsi: message: fusion: Remove unused parameter of mpt_pci driver's probe() Uwe Kleine-König
2021-09-27 20:43 ` [PATCH v4 6/8] crypto: qat - simplify adf_enable_aer() Uwe Kleine-König
2021-09-28 11:11 ` Giovanni Cabiddu [this message]
2021-09-28 11:26 ` Uwe Kleine-König
2021-09-28 13:57 ` Uwe Kleine-König
2021-09-28 15:01 ` Giovanni Cabiddu
2021-09-28 16:08 ` Uwe Kleine-König
2021-09-27 20:43 ` [PATCH v4 7/8] PCI: Replace pci_dev::driver usage by pci_dev::dev.driver Uwe Kleine-König
2021-09-27 20:43 ` [PATCH v4 8/8] PCI: Drop duplicated tracking of a pci_dev's bound driver Uwe Kleine-König
2021-09-27 20:59 ` [PATCH v4 0/8] " Uwe Kleine-König
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YVL4aoKjUT2kvHip@silpixa00400314 \
--to=giovanni.cabiddu@intel.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=davem@davemloft.net \
--cc=fiona.trahe@intel.com \
--cc=helgaas@kernel.org \
--cc=herbert@gondor.apana.org.au \
--cc=jack.xu@intel.com \
--cc=kernel@pengutronix.de \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=marco.chiappero@intel.com \
--cc=qat-linux@intel.com \
--cc=tomaszx.kowalik@intel.com \
--cc=u.kleine-koenig@pengutronix.de \
--cc=uwe@kleine-koenig.org \
--cc=wojciech.ziemba@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).