[PATCH -next 1/5] misc: microchip: pci1xxxx: fix error handling in gp_aux_bus_probe()

[PATCH -next 1/5] misc: microchip: pci1xxxx: fix error handling in gp_aux_bus_probe()

[Wei Yongjun]

From: Wei Yongjun <weiyongjun1@huawei.com>

In some error handling path, resoures alloced may not released.
This patch fix them.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
index bfc03028b34d..11f79f239006 100644
--- a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
+++ b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
@@ -87,12 +87,13 @@ static int gp_aux_bus_probe(struct pci_dev *pdev, const struct pci_device_id *id
 	retval = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
 
 	if (retval < 0)
-		return retval;
+		goto err_ida_alloc_1;
 
-	pdev->irq = pci_irq_vector(pdev, 0);
-	if (pdev->irq < 0)
-		return retval;
+	retval = pci_irq_vector(pdev, 0);
+	if (retval < 0)
+		goto err_aux_dev_init_1;
 
+	pdev->irq = retval;
 	aux_bus->aux_device_wrapper[1]->gp_aux_data.irq_num = pdev->irq;
 
 	retval = auxiliary_device_init(&aux_bus->aux_device_wrapper[1]->aux_dev);
-- 
2.34.1

[PATCH -next 2/5] misc: microchip: pci1xxxx: Fix missing spin_lock_init()

[Wei Yongjun]

From: Wei Yongjun <weiyongjun1@huawei.com>

The driver allocates the spinlock but not initialize it.
Use spin_lock_init() on it to initialize it correctly.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c
index 230503cca2ff..47e6e87938ae 100644
--- a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c
+++ b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c
@@ -383,6 +383,7 @@ static int pci1xxxx_gpio_probe(struct auxiliary_device *aux_dev,
 	if (!priv)
 		return -ENOMEM;
 
+	spin_lock_init(&priv->lock);
 	priv->aux_dev = aux_dev;
 
 	if (!devm_request_mem_region(&aux_dev->dev, pdata->region_start, 0x800, aux_dev->name))
-- 
2.34.1

[PATCH -next 3/5] misc: microchip: pci1xxxx: Make symbol 'pci1xxxx_gpio_auxiliary_id_table' static

[Wei Yongjun]

From: Wei Yongjun <weiyongjun1@huawei.com>

The sparse tool complains as follows:

drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c:409:34: warning:
 symbol 'pci1xxxx_gpio_auxiliary_id_table' was not declared. Should it be static?

This symbol is not used outside of mchp_pci1xxxx_gpio.c, so marks it static.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c
index 47e6e87938ae..4f26871994ee 100644
--- a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c
+++ b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c
@@ -407,7 +407,7 @@ static int pci1xxxx_gpio_probe(struct auxiliary_device *aux_dev,
 
 static SIMPLE_DEV_PM_OPS(pci1xxxx_gpio_pm_ops, pci1xxxx_gpio_suspend, pci1xxxx_gpio_resume);
 
-const struct auxiliary_device_id pci1xxxx_gpio_auxiliary_id_table[] = {
+static const struct auxiliary_device_id pci1xxxx_gpio_auxiliary_id_table[] = {
 	{.name = "mchp_pci1xxxx_gp.gp_gpio"},
 	{}
 };
-- 
2.34.1

[PATCH -next 4/5] misc: microchip: pci1xxxx: Add missing MODULE_DEVICE_TABLE

[Wei Yongjun]

From: Wei Yongjun <weiyongjun1@huawei.com>

This patch adds missing MODULE_DEVICE_TABLE definition which generates
correct modalias for automatic loading of this driver when it is built
as an external module.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c
index 4f26871994ee..fa80a7788596 100644
--- a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c
+++ b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c
@@ -411,6 +411,7 @@ static const struct auxiliary_device_id pci1xxxx_gpio_auxiliary_id_table[] = {
 	{.name = "mchp_pci1xxxx_gp.gp_gpio"},
 	{}
 };
+MODULE_DEVICE_TABLE(auxiliary, pci1xxxx_gpio_auxiliary_id_table);
 
 static struct auxiliary_driver pci1xxxx_gpio_driver = {
 	.driver = {
-- 
2.34.1

[PATCH -next 5/5] misc: microchip: pci1xxxx: use module_auxiliary_driver

[Wei Yongjun]

From: Wei Yongjun <weiyongjun1@huawei.com>

Use the module_auxiliary_driver() macro to make the code simpler
by eliminating module_init and module_exit calls.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c
index fa80a7788596..9cc771c604ed 100644
--- a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c
+++ b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c
@@ -421,19 +421,7 @@ static struct auxiliary_driver pci1xxxx_gpio_driver = {
 	.probe = pci1xxxx_gpio_probe,
 	.id_table = pci1xxxx_gpio_auxiliary_id_table
 };
-
-static int __init pci1xxxx_gpio_driver_init(void)
-{
-	return auxiliary_driver_register(&pci1xxxx_gpio_driver);
-}
-
-static void __exit pci1xxxx_gpio_driver_exit(void)
-{
-	auxiliary_driver_unregister(&pci1xxxx_gpio_driver);
-}
-
-module_init(pci1xxxx_gpio_driver_init);
-module_exit(pci1xxxx_gpio_driver_exit);
+module_auxiliary_driver(pci1xxxx_gpio_driver);
 
 MODULE_DESCRIPTION("Microchip Technology Inc. PCI1xxxx GPIO controller");
 MODULE_AUTHOR("Kumaravel Thiagarajan <kumaravel.thiagarajan@microchip.com>");
-- 
2.34.1

RE: [PATCH -next 1/5] misc: microchip: pci1xxxx: fix error handling in gp_aux_bus_probe()

[Kumaravel.Thiagarajan]

> -----Original Message-----
> From: Wei Yongjun <weiyongjun@huaweicloud.com>
> Sent: Wednesday, September 7, 2022 2:05 PM
> To: Kumaravel Thiagarajan - I21417
> <Kumaravel.Thiagarajan@microchip.com>; Arnd Bergmann
> <arnd@arndb.de>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Wei Yongjun <weiyongjun1@huawei.com>; linux-gpio@vger.kernel.org;
> kernel-janitors@vger.kernel.org
> Subject: [PATCH -next 1/5] misc: microchip: pci1xxxx: fix error handling in
> gp_aux_bus_probe()
> 
> 
> From: Wei Yongjun <weiyongjun1@huawei.com>
> 
> In some error handling path, resoures alloced may not released.
> This patch fix them.
Thanks for your patch. But I have a minor correction below. Please review.
> 
Fixes: 393fc2f5948f ("misc: microchip: pci1xxxx: load auxiliary bus driver for the PIO function in the multi-function endpoint of pci1xxxx device.")?
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>  drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
> b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
> index bfc03028b34d..11f79f239006 100644
> --- a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
> +++ b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
> @@ -87,12 +87,13 @@ static int gp_aux_bus_probe(struct pci_dev *pdev,
> const struct pci_device_id *id
>         retval = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
> 
>         if (retval < 0)
> -               return retval;
> +               goto err_ida_alloc_1;
By the time the execution reaches here, second ida_alloc (ida_alloc_1) is successful.
It must go to err_aux_dev_init_1 too.

Thank You.

Regards,
Kumaravel

RE: [PATCH -next 2/5] misc: microchip: pci1xxxx: Fix missing spin_lock_init()

[Kumaravel.Thiagarajan]

> -----Original Message-----
> From: Wei Yongjun <weiyongjun@huaweicloud.com>
> Sent: Wednesday, September 7, 2022 2:05 PM
> To: Kumaravel Thiagarajan - I21417
> <Kumaravel.Thiagarajan@microchip.com>; Arnd Bergmann
> <arnd@arndb.de>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Wei Yongjun <weiyongjun1@huawei.com>; linux-gpio@vger.kernel.org;
> kernel-janitors@vger.kernel.org
> Subject: [PATCH -next 2/5] misc: microchip: pci1xxxx: Fix missing
> spin_lock_init()
> 
> 
> From: Wei Yongjun <weiyongjun1@huawei.com>
> 
> The driver allocates the spinlock but not initialize it.
> Use spin_lock_init() on it to initialize it correctly.
Thanks for your patch ! This is required but I have missed this.
> 
Add this tag -> Fixes: 7d3e4d807df2 ("misc: microchip: pci1xxxx: load gpio driver for the gpio controller auxiliary device enumerated by the auxiliary bus driver.")?
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>  drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c | 1 +
>  1 file changed, 1 insertion(+)
> 

Thank You.

Regards,
Kumaravel

RE: [PATCH -next 3/5] misc: microchip: pci1xxxx: Make symbol 'pci1xxxx_gpio_auxiliary_id_table' static

[Kumaravel.Thiagarajan]

> -----Original Message-----
> From: Wei Yongjun <weiyongjun@huaweicloud.com>
> Sent: Wednesday, September 7, 2022 2:05 PM
> To: Kumaravel Thiagarajan - I21417
> <Kumaravel.Thiagarajan@microchip.com>; Arnd Bergmann
> <arnd@arndb.de>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Wei Yongjun <weiyongjun1@huawei.com>; linux-gpio@vger.kernel.org;
> kernel-janitors@vger.kernel.org
> Subject: [PATCH -next 3/5] misc: microchip: pci1xxxx: Make symbol
> 'pci1xxxx_gpio_auxiliary_id_table' static
> 
> 
> From: Wei Yongjun <weiyongjun1@huawei.com>
> 
> The sparse tool complains as follows:
> 
> drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gpio.c:409:34: warning:
>  symbol 'pci1xxxx_gpio_auxiliary_id_table' was not declared. Should it be
> static?
> 
> This symbol is not used outside of mchp_pci1xxxx_gpio.c, so marks it static.
Yes. This was first reported to me by "kernel test robot <lkp@intel.com>".
Thanks for your patch.
> 
Add this tag -> Fixes: 7d3e4d807df2 ("misc: microchip: pci1xxxx: load gpio driver for the gpio controller auxiliary device enumerated by the auxiliary bus driver.")?
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---

Thank You.

Regards,
Kumaravel

RE: [PATCH -next 4/5] misc: microchip: pci1xxxx: Add missing MODULE_DEVICE_TABLE

[Kumaravel.Thiagarajan]

> -----Original Message-----
> From: Wei Yongjun <weiyongjun@huaweicloud.com>
> Sent: Wednesday, September 7, 2022 2:05 PM
> To: Kumaravel Thiagarajan - I21417
> <Kumaravel.Thiagarajan@microchip.com>; Arnd Bergmann
> <arnd@arndb.de>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Wei Yongjun <weiyongjun1@huawei.com>; linux-gpio@vger.kernel.org;
> kernel-janitors@vger.kernel.org
> Subject: [PATCH -next 4/5] misc: microchip: pci1xxxx: Add missing
> MODULE_DEVICE_TABLE
> 
>  
> This patch adds missing MODULE_DEVICE_TABLE definition which generates
> correct modalias for automatic loading of this driver when it is built as an
> external module.
I noticed during testing that my auxiliary device's drivers were not getting loaded
automatically whereas the auxiliary bus driver for the PIO function was getting loaded
automatically. Neither did I expect it to load automatically nor did know this was the cause :)
Thanks for your patch.
> 
Add this tag -> Fixes: 7d3e4d807df2 ("misc: microchip: pci1xxxx: load gpio driver for the gpio controller auxiliary device enumerated by the auxiliary bus driver.")?
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---

Thank You.

Regards,
Kumaravel

RE: [PATCH -next 5/5] misc: microchip: pci1xxxx: use module_auxiliary_driver

[Kumaravel.Thiagarajan]

> -----Original Message-----
> From: Wei Yongjun <weiyongjun@huaweicloud.com>
> Sent: Wednesday, September 7, 2022 2:05 PM
> To: Kumaravel Thiagarajan - I21417
> <Kumaravel.Thiagarajan@microchip.com>; Arnd Bergmann
> <arnd@arndb.de>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Wei Yongjun <weiyongjun1@huawei.com>; linux-gpio@vger.kernel.org;
> kernel-janitors@vger.kernel.org
> Subject: [PATCH -next 5/5] misc: microchip: pci1xxxx: use
> module_auxiliary_driver
> 
> 
> Use the module_auxiliary_driver() macro to make the code simpler by
> eliminating module_init and module_exit calls.
I needed this during the experimentation stage. But now these functions only
do register and unregister. Hence, can be replaced with module_auxiliary_driver.
Thanks for your patch.
> 
Add this tag -> Fixes: 7d3e4d807df2 ("misc: microchip: pci1xxxx: load gpio driver for the gpio controller auxiliary device enumerated by the auxiliary bus driver.")?
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---

Thank You.

Regards,
Kumaravel

Re: [PATCH -next 1/5] misc: microchip: pci1xxxx: fix error handling in gp_aux_bus_probe()

[Wei Yongjun]

On 2022/9/7 19:56, Kumaravel.Thiagarajan@microchip.com wrote:
>> -----Original Message-----
>> From: Wei Yongjun <weiyongjun@huaweicloud.com>
>> Sent: Wednesday, September 7, 2022 2:05 PM
>> To: Kumaravel Thiagarajan - I21417
>> <Kumaravel.Thiagarajan@microchip.com>; Arnd Bergmann
>> <arnd@arndb.de>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Wei Yongjun <weiyongjun1@huawei.com>; linux-gpio@vger.kernel.org;
>> kernel-janitors@vger.kernel.org
>> Subject: [PATCH -next 1/5] misc: microchip: pci1xxxx: fix error handling in
>> gp_aux_bus_probe()
>>
>>
>> From: Wei Yongjun <weiyongjun1@huawei.com>
>>
>> In some error handling path, resoures alloced may not released.
>> This patch fix them.
> Thanks for your patch. But I have a minor correction below. Please review.
> Fixes: 393fc2f5948f ("misc: microchip: pci1xxxx: load auxiliary bus driver for the PIO function in the multi-function endpoint of pci1xxxx device.")?

Will add it.

>> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
>> ---
>>  drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
>> b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
>> index bfc03028b34d..11f79f239006 100644
>> --- a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
>> +++ b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
>> @@ -87,12 +87,13 @@ static int gp_aux_bus_probe(struct pci_dev *pdev,
>> const struct pci_device_id *id
>>         retval = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
>>
>>         if (retval < 0)
>> -               return retval;
>> +               goto err_ida_alloc_1;
> By the time the execution reaches here, second ida_alloc (ida_alloc_1) is successful.
> It must go to err_aux_dev_init_1 too.

Yes, you are right. will fix in next version.

Thanks,
Wei Yongjun


>
> Thank You.
>
> Regards,
> Kumaravel

Re: [PATCH -next 5/5] misc: microchip: pci1xxxx: use module_auxiliary_driver

[Greg KH]

On Wed, Sep 07, 2022 at 02:35:14PM +0000, Kumaravel.Thiagarajan@microchip.com wrote:
> > -----Original Message-----
> > From: Wei Yongjun <weiyongjun@huaweicloud.com>
> > Sent: Wednesday, September 7, 2022 2:05 PM
> > To: Kumaravel Thiagarajan - I21417
> > <Kumaravel.Thiagarajan@microchip.com>; Arnd Bergmann
> > <arnd@arndb.de>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Wei Yongjun <weiyongjun1@huawei.com>; linux-gpio@vger.kernel.org;
> > kernel-janitors@vger.kernel.org
> > Subject: [PATCH -next 5/5] misc: microchip: pci1xxxx: use
> > module_auxiliary_driver
> > 
> > 
> > Use the module_auxiliary_driver() macro to make the code simpler by
> > eliminating module_init and module_exit calls.
> I needed this during the experimentation stage. But now these functions only
> do register and unregister. Hence, can be replaced with module_auxiliary_driver.
> Thanks for your patch.
> > 
> Add this tag -> Fixes: 7d3e4d807df2 ("misc: microchip: pci1xxxx: load gpio driver for the gpio controller auxiliary device enumerated by the auxiliary bus driver.")?

As you are the maintainer of this driver, you can add these markings to
the patches and sign off on them when you forward them on to me.  No
need to force the original developer to do this if you are already going
to do it as well.

thanks,

greg k-h

RE: [PATCH -next 5/5] misc: microchip: pci1xxxx: use module_auxiliary_driver

[Kumaravel.Thiagarajan]

> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Wednesday, September 7, 2022 8:17 PM
> To: Kumaravel Thiagarajan - I21417 <Kumaravel.Thiagarajan@microchip.com>
> Cc: weiyongjun@huaweicloud.com; arnd@arndb.de;
> weiyongjun1@huawei.com; linux-gpio@vger.kernel.org; kernel-
> janitors@vger.kernel.org
> Subject: Re: [PATCH -next 5/5] misc: microchip: pci1xxxx: use
> module_auxiliary_driver
> 
> On Wed, Sep 07, 2022 at 02:35:14PM +0000,
> Kumaravel.Thiagarajan@microchip.com wrote:
> > > -----Original Message-----
> > > From: Wei Yongjun <weiyongjun@huaweicloud.com>
> > > Sent: Wednesday, September 7, 2022 2:05 PM
> > > To: Kumaravel Thiagarajan - I21417
> > > <Kumaravel.Thiagarajan@microchip.com>; Arnd Bergmann
> > > <arnd@arndb.de>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: Wei Yongjun <weiyongjun1@huawei.com>;
> > > linux-gpio@vger.kernel.org; kernel-janitors@vger.kernel.org
> > > Subject: [PATCH -next 5/5] misc: microchip: pci1xxxx: use
> > > module_auxiliary_driver
> > >
> > >
> > > Use the module_auxiliary_driver() macro to make the code simpler by
> > > eliminating module_init and module_exit calls.
> > I needed this during the experimentation stage. But now these
> > functions only do register and unregister. Hence, can be replaced with
> module_auxiliary_driver.
> > Thanks for your patch.
> > >
> > Add this tag -> Fixes: 7d3e4d807df2 ("misc: microchip: pci1xxxx: load gpio
> driver for the gpio controller auxiliary device enumerated by the auxiliary bus
> driver.")?
> 
> As you are the maintainer of this driver, you can add these markings to the
> patches and sign off on them when you forward them on to me.  No need to
> force the original developer to do this if you are already going to do it as well.
Sure. Understand this now. Thank you, Greg.

Regards,
Kumaravel