| Message ID | 20210811080637.2596434-7-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: Drop duplicated tracking of a pci_dev's bound driver | expand |
On Wed, Aug 11, 2021 at 10:06:35AM +0200, Uwe Kleine-König wrote: > 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> Thanks Uwe for the rework. > --- > 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 | 2 +- > drivers/crypto/qat/qat_dh895xcc/adf_drv.c | 7 ++----- > 6 files changed, 12 insertions(+), 28 deletions(-) > > diff --git a/drivers/crypto/qat/qat_4xxx/adf_drv.c b/drivers/crypto/qat/qat_4xxx/adf_drv.c > index a8805c815d16..1620281a9ed8 100644 > --- a/drivers/crypto/qat/qat_4xxx/adf_drv.c > +++ b/drivers/crypto/qat/qat_4xxx/adf_drv.c > @@ -253,11 +253,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); After seeing your patch, I'm thinking to get rid of both adf_enable_aer() (and adf_disable_aer()) and call directly pci_enable_pcie_error_reporting() here. There is another patch around this area that I reworked (but not sent yet - https://patchwork.kernel.org/project/linux-crypto/patch/a19132d07a65dbef5e818f84b2bc971f26cc3805.1625983602.git.christophe.jaillet@wanadoo.fr/). Would you mind if your patch goes through Herbert's tree? If you want I can also send a reworked version. > if (pci_save_state(pdev)) { > dev_err(&pdev->dev, "Failed to save pci state.\n"); > @@ -310,6 +306,7 @@ static struct pci_driver adf_driver = { > .probe = adf_probe, > .remove = adf_remove, > .sriov_configure = adf_sriov_configure, > + .err_handler = adf_err_handler, Compilation fails here: drivers/crypto/qat/qat_4xxx/adf_drv.c:309:24: error: ‘adf_err_handler’ undeclared here (not in a function) 309 | .err_handler = adf_err_handler, | ^~~~~~~~~~~~~~~ Were you thinking to change it this way? --- a/drivers/crypto/qat/qat_common/adf_common_drv.h +++ b/drivers/crypto/qat/qat_common/adf_common_drv.h @@ -95,8 +95,11 @@ 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); +extern const struct pci_error_handlers adf_err_handler; --- a/drivers/crypto/qat/qat_4xxx/adf_drv.c +++ b/drivers/crypto/qat/qat_4xxx/adf_drv.c @@ -306,7 +306,7 @@ static struct pci_driver adf_driver = { .probe = adf_probe, .remove = adf_remove, .sriov_configure = adf_sriov_configure, - .err_handler = adf_err_handler, + .err_handler = &adf_err_handler, }; I think the main reason why the driver was dereferencing the pci_driver in adf_enable_aer() was to avoid that extern struct in adf_common_drv.h. I am ok with that. Any objections from others? Regards,
On Wed, Aug 11, 2021 at 12:56:39PM +0100, Giovanni Cabiddu wrote: > On Wed, Aug 11, 2021 at 10:06:35AM +0200, Uwe Kleine-König wrote: > > 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> > Thanks Uwe for the rework. > > > --- > > 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 | 2 +- > > drivers/crypto/qat/qat_dh895xcc/adf_drv.c | 7 ++----- > > 6 files changed, 12 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/crypto/qat/qat_4xxx/adf_drv.c b/drivers/crypto/qat/qat_4xxx/adf_drv.c > > index a8805c815d16..1620281a9ed8 100644 > > --- a/drivers/crypto/qat/qat_4xxx/adf_drv.c > > +++ b/drivers/crypto/qat/qat_4xxx/adf_drv.c > > @@ -253,11 +253,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); > After seeing your patch, I'm thinking to get rid of both adf_enable_aer() > (and adf_disable_aer()) and call directly pci_enable_pcie_error_reporting() > here. > > There is another patch around this area that I reworked (but not sent > yet - https://patchwork.kernel.org/project/linux-crypto/patch/a19132d07a65dbef5e818f84b2bc971f26cc3805.1625983602.git.christophe.jaillet@wanadoo.fr/). > Would you mind if your patch goes through Herbert's tree? > If you want I can also send a reworked version. As patch #8 of my series depends on this one I think the best option is to create a tag and pull that into both, the pci and the crypto tree?! @Bjorn: Would you agree to this procedure? There has to be a v4, if it helps I can provide a signed tag for pulling?! Just tell me what would be helpful here. > > if (pci_save_state(pdev)) { > > dev_err(&pdev->dev, "Failed to save pci state.\n"); > > @@ -310,6 +306,7 @@ static struct pci_driver adf_driver = { > > .probe = adf_probe, > > .remove = adf_remove, > > .sriov_configure = adf_sriov_configure, > > + .err_handler = adf_err_handler, > Compilation fails here: > drivers/crypto/qat/qat_4xxx/adf_drv.c:309:24: error: ‘adf_err_handler’ undeclared here (not in a function) > 309 | .err_handler = adf_err_handler, > | ^~~~~~~~~~~~~~~ > > Were you thinking to change it this way? > > --- a/drivers/crypto/qat/qat_common/adf_common_drv.h > +++ b/drivers/crypto/qat/qat_common/adf_common_drv.h > @@ -95,8 +95,11 @@ 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); > > +extern const struct pci_error_handlers adf_err_handler; > > --- a/drivers/crypto/qat/qat_4xxx/adf_drv.c > +++ b/drivers/crypto/qat/qat_4xxx/adf_drv.c > @@ -306,7 +306,7 @@ static struct pci_driver adf_driver = { > .probe = adf_probe, > .remove = adf_remove, > .sriov_configure = adf_sriov_configure, > - .err_handler = adf_err_handler, > + .err_handler = &adf_err_handler, > }; Yeah, the other three drivers need an adaption, too. I will send a v4 in the next few days. The current state is available at https://git.pengutronix.de/git/ukl/linux pci-dedup Best regards Uwe
On Wed, Aug 11, 2021 at 11:34:05PM +0200, Uwe Kleine-König wrote: > On Wed, Aug 11, 2021 at 12:56:39PM +0100, Giovanni Cabiddu wrote: > > On Wed, Aug 11, 2021 at 10:06:35AM +0200, Uwe Kleine-König wrote: > > > 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> > > Thanks Uwe for the rework. > > > > > --- > > > 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 | 2 +- > > > drivers/crypto/qat/qat_dh895xcc/adf_drv.c | 7 ++----- > > > 6 files changed, 12 insertions(+), 28 deletions(-) > > > > > > diff --git a/drivers/crypto/qat/qat_4xxx/adf_drv.c b/drivers/crypto/qat/qat_4xxx/adf_drv.c > > > index a8805c815d16..1620281a9ed8 100644 > > > --- a/drivers/crypto/qat/qat_4xxx/adf_drv.c > > > +++ b/drivers/crypto/qat/qat_4xxx/adf_drv.c > > > @@ -253,11 +253,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); > > After seeing your patch, I'm thinking to get rid of both adf_enable_aer() > > (and adf_disable_aer()) and call directly pci_enable_pcie_error_reporting() > > here. I'm going to rework this in a subsequent patchset. > > There is another patch around this area that I reworked (but not sent > > yet - https://patchwork.kernel.org/project/linux-crypto/patch/a19132d07a65dbef5e818f84b2bc971f26cc3805.1625983602.git.christophe.jaillet@wanadoo.fr/). > > Would you mind if your patch goes through Herbert's tree? > > If you want I can also send a reworked version. > > As patch #8 of my series depends on this one I think the best option is > to create a tag and pull that into both, the pci and the crypto tree?! Probably there is no need for that. Your patch applies cleanly on top of this series: https://patchwork.kernel.org/project/linux-crypto/list/?series=530407 > > @Bjorn: Would you agree to this procedure? There has to be a v4, if it > helps I can provide a signed tag for pulling?! Just tell me what would > be helpful here. > > > > if (pci_save_state(pdev)) { > > > dev_err(&pdev->dev, "Failed to save pci state.\n"); > > > @@ -310,6 +306,7 @@ static struct pci_driver adf_driver = { > > > .probe = adf_probe, > > > .remove = adf_remove, > > > .sriov_configure = adf_sriov_configure, > > > + .err_handler = adf_err_handler, > > Compilation fails here: > > drivers/crypto/qat/qat_4xxx/adf_drv.c:309:24: error: ‘adf_err_handler’ undeclared here (not in a function) > > 309 | .err_handler = adf_err_handler, > > | ^~~~~~~~~~~~~~~ > > > > Were you thinking to change it this way? > > > > --- a/drivers/crypto/qat/qat_common/adf_common_drv.h > > +++ b/drivers/crypto/qat/qat_common/adf_common_drv.h > > @@ -95,8 +95,11 @@ 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); > > > > +extern const struct pci_error_handlers adf_err_handler; > > > > --- a/drivers/crypto/qat/qat_4xxx/adf_drv.c > > +++ b/drivers/crypto/qat/qat_4xxx/adf_drv.c > > @@ -306,7 +306,7 @@ static struct pci_driver adf_driver = { > > .probe = adf_probe, > > .remove = adf_remove, > > .sriov_configure = adf_sriov_configure, > > - .err_handler = adf_err_handler, > > + .err_handler = &adf_err_handler, > > }; > > Yeah, the other three drivers need an adaption, too. I will send a v4 in > the next few days. The current state is available at > > https://git.pengutronix.de/git/ukl/linux pci-dedup I fixed that and tested on c62x. Here is an updated patch: ----8<---- From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de> Date: Wed, 11 Aug 2021 10:06:35 +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> Co-developed-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com> Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com> --- 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 | 5 ++++- drivers/crypto/qat/qat_dh895xcc/adf_drv.c | 7 ++----- 6 files changed, 15 insertions(+), 28 deletions(-) diff --git a/drivers/crypto/qat/qat_4xxx/adf_drv.c b/drivers/crypto/qat/qat_4xxx/adf_drv.c index a8805c815d16..e863f7ac9c44 100644 --- a/drivers/crypto/qat/qat_4xxx/adf_drv.c +++ b/drivers/crypto/qat/qat_4xxx/adf_drv.c @@ -253,11 +253,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"); @@ -310,6 +306,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 7fb3343ae8b0..c62330357e63 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) @@ -199,11 +200,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 1f5de442e1e6..ad9f18a9eb1d 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) @@ -199,11 +200,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 d2ae293d0df6..9284d09e3af8 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 c61476553728..3d7fb9715cf8 100644 --- a/drivers/crypto/qat/qat_common/adf_common_drv.h +++ b/drivers/crypto/qat/qat_common/adf_common_drv.h @@ -95,8 +95,11 @@ 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); void adf_dev_restore(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 a9ec4357144c..9e9d495271b5 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) @@ -199,11 +200,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");
Hello Giovanni, On Thu, Aug 12, 2021 at 03:43:09PM +0100, Giovanni Cabiddu wrote: > On Wed, Aug 11, 2021 at 11:34:05PM +0200, Uwe Kleine-König wrote: > > Yeah, the other three drivers need an adaption, too. I will send a v4 in > > the next few days. The current state is available at > > > > https://git.pengutronix.de/git/ukl/linux pci-dedup > > I fixed that and tested on c62x. Here is an updated patch: Apart from ordering in drivers/crypto/qat/qat_common/adf_common_drv.h and some whitespace differences there is already an equivalent commit in my branch. Best regards Uwe
diff --git a/drivers/crypto/qat/qat_4xxx/adf_drv.c b/drivers/crypto/qat/qat_4xxx/adf_drv.c index a8805c815d16..1620281a9ed8 100644 --- a/drivers/crypto/qat/qat_4xxx/adf_drv.c +++ b/drivers/crypto/qat/qat_4xxx/adf_drv.c @@ -253,11 +253,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"); @@ -310,6 +306,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 7fb3343ae8b0..ceca372506a0 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) @@ -199,11 +200,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 1f5de442e1e6..9176be253cc0 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) @@ -199,11 +200,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 d2ae293d0df6..9284d09e3af8 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 c61476553728..d25934e97b55 100644 --- a/drivers/crypto/qat/qat_common/adf_common_drv.h +++ b/drivers/crypto/qat/qat_common/adf_common_drv.h @@ -95,7 +95,7 @@ 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); +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 a9ec4357144c..e1254bb83fc3 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) @@ -199,11 +200,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");
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 | 2 +- drivers/crypto/qat/qat_dh895xcc/adf_drv.c | 7 ++----- 6 files changed, 12 insertions(+), 28 deletions(-)