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@xxxxxxxxxxxxxx> > 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@xxxxxxxxxx/). > 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 -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature