On Tue, Mar 28, 2023 at 08:10:04PM +0530, Vaibhaav Ram T.L wrote:
> From: Kumaravel Thiagarajan <[email protected]>
>
> Removed unnecessary header files.
That's not a "fix", it is a cleanup.
> Fix error handling path in probe function.
> Add pci_free_irq_vectors and auxiliary_device_delete in
> error handling path.
All of these should be individual patches, right?
And you have trailing whitespace here :(
>
> Fixes: 393fc2f5948f ("misc: microchip: pci1xxxx: load auxiliary bus driver for the PIO function in the multi-function endpoint of pci1xxxx device.")
Is this really a fix of that commit? What was wrong there, just the
error handling?
> Reported by: Christophe JAILLET <[email protected]>
>
> Co-developed-by: Tharun Kumar P <[email protected]>
No blank line there please.
> Signed-off-by: Tharun Kumar P <[email protected]>
> Signed-off-by: Kumaravel Thiagarajan <[email protected]>
> Signed-off-by: Vaibhaav Ram T.L <[email protected]>
> ---
> drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c | 104 +++++++++---------
> 1 file changed, 55 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
> index 32af2b14ff34..64302fdfbefc 100644
> --- a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
> +++ b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
> @@ -1,16 +1,15 @@
> // SPDX-License-Identifier: GPL-2.0
> -// Copyright (C) 2022 Microchip Technology Inc.
> +// Copyright (C) 2022-2023 Microchip Technology Inc.
>
> -#include <linux/mfd/core.h>
> +#include <linux/interrupt.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> -#include <linux/spinlock.h>
> -#include <linux/gpio/driver.h>
> -#include <linux/interrupt.h>
> -#include <linux/io.h>
> #include <linux/idr.h>
> +#include <linux/io.h>
> #include "mchp_pci1xxxx_gp.h"
>
> +#define PCI_DRIVER_NAME "PCI1xxxxGP"
This is not a "fix" but rather a new change.
All of the changes in here are not related, break this up into "one
logical change per patch" please.
thanks,
greg k-h
On Wed, 2023-03-29 at 11:59 +0200, Greg KH wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> On Tue, Mar 28, 2023 at 08:10:04PM +0530, Vaibhaav Ram T.L wrote:
> > From: Kumaravel Thiagarajan <[email protected]>
> >
> > Removed unnecessary header files.
>
> That's not a "fix", it is a cleanup.
>
> > Fix error handling path in probe function.
> > Add pci_free_irq_vectors and auxiliary_device_delete in
> > error handling path.
>
> All of these should be individual patches, right?
Ok. I will split this patch into multiple patches.
>
> And you have trailing whitespace here :(
Ok. I will correct this.
>
> >
> > Fixes: 393fc2f5948f ("misc: microchip: pci1xxxx: load auxiliary bus
> > driver for the PIO function in the multi-function endpoint of
> > pci1xxxx device.")
>
> Is this really a fix of that commit? What was wrong there, just the
> error handling?
Yes, There are two errors in the error handling path.
>
> > Reported by: Christophe JAILLET <[email protected]>
> >
> > Co-developed-by: Tharun Kumar P
> > <[email protected]>
>
> No blank line there please.
Ok.
>
> > Signed-off-by: Tharun Kumar P
> > <[email protected]>
> > Signed-off-by: Kumaravel Thiagarajan
> > <[email protected]>
> > Signed-off-by: Vaibhaav Ram T.L <[email protected]>
> > ---
> > drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c | 104 +++++++++-----
> > ----
> > 1 file changed, 55 insertions(+), 49 deletions(-)
> >
> > diff --git a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
> > b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
> > index 32af2b14ff34..64302fdfbefc 100644
> > --- a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
> > +++ b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
> > @@ -1,16 +1,15 @@
> > // SPDX-License-Identifier: GPL-2.0
> > -// Copyright (C) 2022 Microchip Technology Inc.
> > +// Copyright (C) 2022-2023 Microchip Technology Inc.
> >
> > -#include <linux/mfd/core.h>
> > +#include <linux/interrupt.h>
> > #include <linux/module.h>
> > #include <linux/pci.h>
> > -#include <linux/spinlock.h>
> > -#include <linux/gpio/driver.h>
> > -#include <linux/interrupt.h>
> > -#include <linux/io.h>
> > #include <linux/idr.h>
> > +#include <linux/io.h>
> > #include "mchp_pci1xxxx_gp.h"
> >
> > +#define PCI_DRIVER_NAME "PCI1xxxxGP"
>
> This is not a "fix" but rather a new change.
Ok. Will correct this.
>
> All of the changes in here are not related, break this up into "one
> logical change per patch" please.
Ok Greg. I understand this now.
>
> Thank You.
Regards,
Vaibhaav Ram