[PATCH] mlxbf_gige: Add missing check for platform_get_irq

[PATCH] mlxbf_gige: Add missing check for platform_get_irq

[Jiasheng Jiang]

Add the check for the return value of the platform_get_irq and
return error if it fails.

Fixes: f92e1869d74e ("Add Mellanox BlueField Gigabit Ethernet driver")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
index 694de9513b9f..a38e1c68874f 100644
--- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
+++ b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
@@ -427,6 +427,10 @@ static int mlxbf_gige_probe(struct platform_device *pdev)
 	priv->error_irq = platform_get_irq(pdev, MLXBF_GIGE_ERROR_INTR_IDX);
 	priv->rx_irq = platform_get_irq(pdev, MLXBF_GIGE_RECEIVE_PKT_INTR_IDX);
 	priv->llu_plu_irq = platform_get_irq(pdev, MLXBF_GIGE_LLU_PLU_INTR_IDX);
+	if (priv->error_irq < 0 || priv->rx_irq < 0 || priv->llu_plu_irq < 0) {
+		err = -ENODEV;
+		goto out;
+	}
 
 	phy_irq = acpi_dev_gpio_irq_get_by(ACPI_COMPANION(&pdev->dev), "phy-gpios", 0);
 	if (phy_irq < 0) {
-- 
2.25.1

Re: [PATCH] mlxbf_gige: Add missing check for platform_get_irq

[Jakub Kicinski]

On Wed, 31 May 2023 15:54:51 +0800 Jiasheng Jiang wrote:
> Add the check for the return value of the platform_get_irq and
> return error if it fails.
> 
> Fixes: f92e1869d74e ("Add Mellanox BlueField Gigabit Ethernet driver")

You need to CC all the authors of the patch limings@nvidia.com seems 
to be missing.
-- 
pw-bot: cr

[PATCH] mlxbf_gige: Add missing check for platform_get_irq

[Jiasheng Jiang]

Add the check for the return value of the platform_get_irq and
return error if it fails.

Fixes: f92e1869d74e ("Add Mellanox BlueField Gigabit Ethernet driver")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
index 694de9513b9f..a38e1c68874f 100644
--- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
+++ b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
@@ -427,6 +427,10 @@ static int mlxbf_gige_probe(struct platform_device *pdev)
 	priv->error_irq = platform_get_irq(pdev, MLXBF_GIGE_ERROR_INTR_IDX);
 	priv->rx_irq = platform_get_irq(pdev, MLXBF_GIGE_RECEIVE_PKT_INTR_IDX);
 	priv->llu_plu_irq = platform_get_irq(pdev, MLXBF_GIGE_LLU_PLU_INTR_IDX);
+	if (priv->error_irq < 0 || priv->rx_irq < 0 || priv->llu_plu_irq < 0) {
+		err = -ENODEV;
+		goto out;
+	}
 
 	phy_irq = acpi_dev_gpio_irq_get_by(ACPI_COMPANION(&pdev->dev), "phy-gpios", 0);
 	if (phy_irq < 0) {
-- 
2.25.1

Re: [PATCH] mlxbf_gige: Add missing check for platform_get_irq

[Jakub Kicinski]

On Thu,  1 Jun 2023 14:19:08 +0800 Jiasheng Jiang wrote:
> Add the check for the return value of the platform_get_irq and
> return error if it fails.
> 
> Fixes: f92e1869d74e ("Add Mellanox BlueField Gigabit Ethernet driver")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>

BTW I looked thru my sent messages and I complained to you about not
CCing people at least twice before. Please start paying attention or
we'll stop paying attention to your patches.

Re: [PATCH] mlxbf_gige: Add missing check for platform_get_irq

[Jiasheng Jiang]

On Thu,  1 Jun 2023 14:27:21 +0800 Jakub Kicinski wrote:
> On Thu,  1 Jun 2023 14:19:08 +0800 Jiasheng Jiang wrote:
>> Add the check for the return value of the platform_get_irq and
>> return error if it fails.
>> 
>> Fixes: f92e1869d74e ("Add Mellanox BlueField Gigabit Ethernet driver")
>> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> 
> BTW I looked thru my sent messages and I complained to you about not
> CCing people at least twice before. Please start paying attention or
> we'll stop paying attention to your patches.

According to the documentation of submitting patches
(Link: https://docs.kernel.org/process/submitting-patches.html),
I used "scripts/get_maintainer.pl" to gain the appropriate recipients
for my patch.
However, the "limings@nvidia.com" is not contained in the following list.

"David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING DRIVERS)
Eric Dumazet <edumazet@google.com> (maintainer:NETWORKING DRIVERS)
Jakub Kicinski <kuba@kernel.org> (maintainer:NETWORKING DRIVERS,commit_signer:5/6=83%,authored:1/6=17%,removed_lines:1/20=5%)
Paolo Abeni <pabeni@redhat.com> (maintainer:NETWORKING DRIVERS)
Asmaa Mnebhi <asmaa@nvidia.com> (commit_signer:4/6=67%)
David Thompson <davthompson@nvidia.com> (commit_signer:4/6=67%,authored:4/6=67%,added_lines:94/99=95%,removed_lines:19/20=95%)
Marc Kleine-Budde <mkl@pengutronix.de> (commit_signer:1/6=17%)
netdev@vger.kernel.org (open list:NETWORKING DRIVERS)
linux-kernel@vger.kernel.org (open list)

There may be a problem with the script.
The best way is to fix it.

Thanks,
Jiang

Re: [PATCH] mlxbf_gige: Add missing check for platform_get_irq

[Simon Horman]

On Thu, Jun 01, 2023 at 02:58:08PM +0800, Jiasheng Jiang wrote:
> On Thu,  1 Jun 2023 14:27:21 +0800 Jakub Kicinski wrote:
> > On Thu,  1 Jun 2023 14:19:08 +0800 Jiasheng Jiang wrote:
> >> Add the check for the return value of the platform_get_irq and
> >> return error if it fails.
> >> 
> >> Fixes: f92e1869d74e ("Add Mellanox BlueField Gigabit Ethernet driver")
> >> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> > 
> > BTW I looked thru my sent messages and I complained to you about not
> > CCing people at least twice before. Please start paying attention or
> > we'll stop paying attention to your patches.
> 
> According to the documentation of submitting patches
> (Link: https://docs.kernel.org/process/submitting-patches.html),
> I used "scripts/get_maintainer.pl" to gain the appropriate recipients
> for my patch.
> However, the "limings@nvidia.com" is not contained in the following list.
> 
> "David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING DRIVERS)
> Eric Dumazet <edumazet@google.com> (maintainer:NETWORKING DRIVERS)
> Jakub Kicinski <kuba@kernel.org> (maintainer:NETWORKING DRIVERS,commit_signer:5/6=83%,authored:1/6=17%,removed_lines:1/20=5%)
> Paolo Abeni <pabeni@redhat.com> (maintainer:NETWORKING DRIVERS)
> Asmaa Mnebhi <asmaa@nvidia.com> (commit_signer:4/6=67%)
> David Thompson <davthompson@nvidia.com> (commit_signer:4/6=67%,authored:4/6=67%,added_lines:94/99=95%,removed_lines:19/20=95%)
> Marc Kleine-Budde <mkl@pengutronix.de> (commit_signer:1/6=17%)
> netdev@vger.kernel.org (open list:NETWORKING DRIVERS)
> linux-kernel@vger.kernel.org (open list)
> 
> There may be a problem with the script.
> The best way is to fix it.

Let's take a step back.

The script is here, so you can take a look at what it does.
And I dare say that changes can be proposed.

  https://github.com/kuba-moo/nipa/blob/master/tests/patch/cc_maintainers/test.py

I'd also say that the problem here is that Liming Sun <limings@nvidia.com>
appears in the above mentioned commit that is being fixed.

I think that get_maintainer will dell you this if you run it
on your patch. Which is what the script appears to do.

Locally, I see:

  $ ./scripts/get_maintainer.pl --git-min-percent 25 this.patch
  "David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING DRIVERS,blamed_fixes:1/1=100%)
  Eric Dumazet <edumazet@google.com> (maintainer:NETWORKING DRIVERS)
  Jakub Kicinski <kuba@kernel.org> (maintainer:NETWORKING DRIVERS,commit_signer:5/5=100%)
  Paolo Abeni <pabeni@redhat.com> (maintainer:NETWORKING DRIVERS)
  Asmaa Mnebhi <asmaa@nvidia.com> (commit_signer:4/5=80%,blamed_fixes:1/1=100%)
  David Thompson <davthompson@nvidia.com> (commit_signer:4/5=80%,authored:4/5=80%,added_lines:94/95=99%,removed_lines:19/20=95%,blamed_fixes:1/1=100%)
  Liming Sun <limings@nvidia.com> (blamed_fixes:1/1=100%)
  netdev@vger.kernel.org (open list:NETWORKING DRIVERS)
  linux-kernel@vger.kernel.org (open list)

N.B.: The script excludes linux-kernel@vger.kernel.org


As an aside. This patch is missing v2.

  Subject: [PATCH v2] ...

Re: [PATCH] mlxbf_gige: Add missing check for platform_get_irq

[Jakub Kicinski]

On Thu,  1 Jun 2023 14:58:08 +0800 Jiasheng Jiang wrote:
> According to the documentation of submitting patches
> (Link: https://docs.kernel.org/process/submitting-patches.html),
> I used "scripts/get_maintainer.pl" to gain the appropriate recipients
> for my patch.
> However, the "limings@nvidia.com" is not contained in the following list.

And I told you already to run the script on the _patch_ not on the file
path.

$ ./scripts/get_maintainer.pl 0001-mlxbf_gige-Add-missing-check-for-platform_get_irq.patch
"David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING DRIVERS,blamed_fixes:1/1=100%)
Eric Dumazet <edumazet@google.com> (maintainer:NETWORKING DRIVERS)
Jakub Kicinski <kuba@kernel.org> (maintainer:NETWORKING DRIVERS,commit_signer:5/6=83%,authored:1/6=17%,removed_lines:1/20=5%)
Paolo Abeni <pabeni@redhat.com> (maintainer:NETWORKING DRIVERS)
Asmaa Mnebhi <asmaa@nvidia.com> (commit_signer:4/6=67%,blamed_fixes:1/1=100%)
David Thompson <davthompson@nvidia.com> (commit_signer:4/6=67%,authored:4/6=67%,added_lines:94/99=95%,removed_lines:19/20=95%,blamed_fixes:1/1=100%)
Marc Kleine-Budde <mkl@pengutronix.de> (commit_signer:1/6=17%)
Jiasheng Jiang <jiasheng@iscas.ac.cn> (commit_signer:1/6=17%,authored:1/6=17%)
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
Liming Sun <limings@nvidia.com> (blamed_fixes:1/1=100%)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
netdev@vger.kernel.org (open list:NETWORKING DRIVERS)
linux-kernel@vger.kernel.org (open list)

Re: [PATCH] mlxbf_gige: Add missing check for platform_get_irq

[Jiasheng Jiang]

On Thu, 1 Jun 2023 23:54:02 +0800 Jakub Kicinski wrote:
> On Thu,  1 Jun 2023 14:58:08 +0800 Jiasheng Jiang wrote:
>> According to the documentation of submitting patches
>> (Link: https://docs.kernel.org/process/submitting-patches.html),
>> I used "scripts/get_maintainer.pl" to gain the appropriate recipients
>> for my patch.
>> However, the "limings@nvidia.com" is not contained in the following list.
> 
> And I told you already to run the script on the _patch_ not on the file
> path.
> 
> $ ./scripts/get_maintainer.pl 0001-mlxbf_gige-Add-missing-check-for-platform_get_irq.patch
> "David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING DRIVERS,blamed_fixes:1/1=100%)
> Eric Dumazet <edumazet@google.com> (maintainer:NETWORKING DRIVERS)
> Jakub Kicinski <kuba@kernel.org> (maintainer:NETWORKING DRIVERS,commit_signer:5/6=83%,authored:1/6=17%,removed_lines:1/20=5%)
> Paolo Abeni <pabeni@redhat.com> (maintainer:NETWORKING DRIVERS)
> Asmaa Mnebhi <asmaa@nvidia.com> (commit_signer:4/6=67%,blamed_fixes:1/1=100%)
> David Thompson <davthompson@nvidia.com> (commit_signer:4/6=67%,authored:4/6=67%,added_lines:94/99=95%,removed_lines:19/20=95%,blamed_fixes:1/1=100%)
> Marc Kleine-Budde <mkl@pengutronix.de> (commit_signer:1/6=17%)
> Jiasheng Jiang <jiasheng@iscas.ac.cn> (commit_signer:1/6=17%,authored:1/6=17%)
> vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
> Liming Sun <limings@nvidia.com> (blamed_fixes:1/1=100%)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> netdev@vger.kernel.org (open list:NETWORKING DRIVERS)
> linux-kernel@vger.kernel.org (open list)

I have got it. I will run the script on the patch.

Thanks,
Jiang

Re: [PATCH] mlxbf_gige: Add missing check for platform_get_irq

[Jiasheng Jiang]

On Thu, Jun 01, 2023 at 23:08:54 +0800, Simon Horman wrote:
> Let's take a step back.
> 
> The script is here, so you can take a look at what it does.
> And I dare say that changes can be proposed.
> 
>   https://github.com/kuba-moo/nipa/blob/master/tests/patch/cc_maintainers/test.py
> 
> I'd also say that the problem here is that Liming Sun <limings@nvidia.com>
> appears in the above mentioned commit that is being fixed.
> 
> I think that get_maintainer will dell you this if you run it
> on your patch. Which is what the script appears to do.
> 
> Locally, I see:
> 
>   $ ./scripts/get_maintainer.pl --git-min-percent 25 this.patch
>   "David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING DRIVERS,blamed_fixes:1/1=100%)
>   Eric Dumazet <edumazet@google.com> (maintainer:NETWORKING DRIVERS)
>   Jakub Kicinski <kuba@kernel.org> (maintainer:NETWORKING DRIVERS,commit_signer:5/5=100%)
>   Paolo Abeni <pabeni@redhat.com> (maintainer:NETWORKING DRIVERS)
>   Asmaa Mnebhi <asmaa@nvidia.com> (commit_signer:4/5=80%,blamed_fixes:1/1=100%)
>   David Thompson <davthompson@nvidia.com> (commit_signer:4/5=80%,authored:4/5=80%,added_lines:94/95=99%,removed_lines:19/20=95%,blamed_fixes:1/1=100%)
>   Liming Sun <limings@nvidia.com> (blamed_fixes:1/1=100%)
>   netdev@vger.kernel.org (open list:NETWORKING DRIVERS)
>   linux-kernel@vger.kernel.org (open list)
> 
> N.B.: The script excludes linux-kernel@vger.kernel.org

I have got it. I will run the script on the patch.

> 
> 
> As an aside. This patch is missing v2.
> 
>   Subject: [PATCH v2] ...

The patch is not changed. I only add a recipient.
Therefore, I think it is not a v2.

Thanks,
Jiasheng

RE: [PATCH] mlxbf_gige: Add missing check for platform_get_irq

[David Thompson]

> -----Original Message-----
> From: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> Sent: Thursday, June 1, 2023 2:19 AM
> To: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; David Thompson <davthompson@nvidia.com>; Asmaa
> Mnebhi <asmaa@nvidia.com>; mkl@pengutronix.de; Liming Sun
> <limings@nvidia.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Jiasheng Jiang
> <jiasheng@iscas.ac.cn>
> Subject: [PATCH] mlxbf_gige: Add missing check for platform_get_irq
> 
> Add the check for the return value of the platform_get_irq and return error if it
> fails.
> 
> Fixes: f92e1869d74e ("Add Mellanox BlueField Gigabit Ethernet driver")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> ---
>  drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> index 694de9513b9f..a38e1c68874f 100644
> --- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> +++ b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> @@ -427,6 +427,10 @@ static int mlxbf_gige_probe(struct platform_device
> *pdev)
>  	priv->error_irq = platform_get_irq(pdev,
> MLXBF_GIGE_ERROR_INTR_IDX);
>  	priv->rx_irq = platform_get_irq(pdev,
> MLXBF_GIGE_RECEIVE_PKT_INTR_IDX);
>  	priv->llu_plu_irq = platform_get_irq(pdev,
> MLXBF_GIGE_LLU_PLU_INTR_IDX);
> +	if (priv->error_irq < 0 || priv->rx_irq < 0 || priv->llu_plu_irq < 0) {
> +		err = -ENODEV;
> +		goto out;
> +	}
> 

It is preferred to maintain the error code from "platform_get_irq" and return
that instead of -ENODEV.  Please review the API for "platform_get_irq" :

/**
 * platform_get_irq - get an IRQ for a device
 * @dev: platform device
 * @num: IRQ number index
 *
 * Gets an IRQ for a platform device and prints an error message if finding the
 * IRQ fails. Device drivers should check the return value for errors so as to
 * not pass a negative integer value to the request_irq() APIs.
 *
 * For example::
 *
 *              int irq = platform_get_irq(pdev, 0);
 *              if (irq < 0)
 *                      return irq;
 *
 * Return: non-zero IRQ number on success, negative error number on failure.
 */

So, the code could look something like this, for each of 3 IRQs:
	
	priv->error_irq = platform_get_irq(...)
	if (priv->error_irq < 0) {
		err = priv->error_irq;
		goto out;
	}

Just a thought.

Thanks,
- Dave

>  	phy_irq = acpi_dev_gpio_irq_get_by(ACPI_COMPANION(&pdev->dev),
> "phy-gpios", 0);
>  	if (phy_irq < 0) {
> --
> 2.25.1