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 <[email protected]>
---
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
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 <[email protected]>
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.
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 <[email protected]>
>
> 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 "[email protected]" is not contained in the following list.
"David S. Miller" <[email protected]> (maintainer:NETWORKING DRIVERS)
Eric Dumazet <[email protected]> (maintainer:NETWORKING DRIVERS)
Jakub Kicinski <[email protected]> (maintainer:NETWORKING DRIVERS,commit_signer:5/6=83%,authored:1/6=17%,removed_lines:1/20=5%)
Paolo Abeni <[email protected]> (maintainer:NETWORKING DRIVERS)
Asmaa Mnebhi <[email protected]> (commit_signer:4/6=67%)
David Thompson <[email protected]> (commit_signer:4/6=67%,authored:4/6=67%,added_lines:94/99=95%,removed_lines:19/20=95%)
Marc Kleine-Budde <[email protected]> (commit_signer:1/6=17%)
[email protected] (open list:NETWORKING DRIVERS)
[email protected] (open list)
There may be a problem with the script.
The best way is to fix it.
Thanks,
Jiang
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 <[email protected]>
> >
> > 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 "[email protected]" is not contained in the following list.
>
> "David S. Miller" <[email protected]> (maintainer:NETWORKING DRIVERS)
> Eric Dumazet <[email protected]> (maintainer:NETWORKING DRIVERS)
> Jakub Kicinski <[email protected]> (maintainer:NETWORKING DRIVERS,commit_signer:5/6=83%,authored:1/6=17%,removed_lines:1/20=5%)
> Paolo Abeni <[email protected]> (maintainer:NETWORKING DRIVERS)
> Asmaa Mnebhi <[email protected]> (commit_signer:4/6=67%)
> David Thompson <[email protected]> (commit_signer:4/6=67%,authored:4/6=67%,added_lines:94/99=95%,removed_lines:19/20=95%)
> Marc Kleine-Budde <[email protected]> (commit_signer:1/6=17%)
> [email protected] (open list:NETWORKING DRIVERS)
> [email protected] (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 <[email protected]>
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" <[email protected]> (maintainer:NETWORKING DRIVERS,blamed_fixes:1/1=100%)
Eric Dumazet <[email protected]> (maintainer:NETWORKING DRIVERS)
Jakub Kicinski <[email protected]> (maintainer:NETWORKING DRIVERS,commit_signer:5/5=100%)
Paolo Abeni <[email protected]> (maintainer:NETWORKING DRIVERS)
Asmaa Mnebhi <[email protected]> (commit_signer:4/5=80%,blamed_fixes:1/1=100%)
David Thompson <[email protected]> (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 <[email protected]> (blamed_fixes:1/1=100%)
[email protected] (open list:NETWORKING DRIVERS)
[email protected] (open list)
N.B.: The script excludes [email protected]
As an aside. This patch is missing v2.
Subject: [PATCH v2] ...
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 "[email protected]" 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" <[email protected]> (maintainer:NETWORKING DRIVERS,blamed_fixes:1/1=100%)
Eric Dumazet <[email protected]> (maintainer:NETWORKING DRIVERS)
Jakub Kicinski <[email protected]> (maintainer:NETWORKING DRIVERS,commit_signer:5/6=83%,authored:1/6=17%,removed_lines:1/20=5%)
Paolo Abeni <[email protected]> (maintainer:NETWORKING DRIVERS)
Asmaa Mnebhi <[email protected]> (commit_signer:4/6=67%,blamed_fixes:1/1=100%)
David Thompson <[email protected]> (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 <[email protected]> (commit_signer:1/6=17%)
Jiasheng Jiang <[email protected]> (commit_signer:1/6=17%,authored:1/6=17%)
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
Liming Sun <[email protected]> (blamed_fixes:1/1=100%)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[email protected] (open list:NETWORKING DRIVERS)
[email protected] (open list)
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 <[email protected]>
> 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" <[email protected]> (maintainer:NETWORKING DRIVERS,blamed_fixes:1/1=100%)
> Eric Dumazet <[email protected]> (maintainer:NETWORKING DRIVERS)
> Jakub Kicinski <[email protected]> (maintainer:NETWORKING DRIVERS,commit_signer:5/5=100%)
> Paolo Abeni <[email protected]> (maintainer:NETWORKING DRIVERS)
> Asmaa Mnebhi <[email protected]> (commit_signer:4/5=80%,blamed_fixes:1/1=100%)
> David Thompson <[email protected]> (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 <[email protected]> (blamed_fixes:1/1=100%)
> [email protected] (open list:NETWORKING DRIVERS)
> [email protected] (open list)
>
> N.B.: The script excludes [email protected]
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
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 "[email protected]" 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" <[email protected]> (maintainer:NETWORKING DRIVERS,blamed_fixes:1/1=100%)
> Eric Dumazet <[email protected]> (maintainer:NETWORKING DRIVERS)
> Jakub Kicinski <[email protected]> (maintainer:NETWORKING DRIVERS,commit_signer:5/6=83%,authored:1/6=17%,removed_lines:1/20=5%)
> Paolo Abeni <[email protected]> (maintainer:NETWORKING DRIVERS)
> Asmaa Mnebhi <[email protected]> (commit_signer:4/6=67%,blamed_fixes:1/1=100%)
> David Thompson <[email protected]> (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 <[email protected]> (commit_signer:1/6=17%)
> Jiasheng Jiang <[email protected]> (commit_signer:1/6=17%,authored:1/6=17%)
> vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
> Liming Sun <[email protected]> (blamed_fixes:1/1=100%)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> [email protected] (open list:NETWORKING DRIVERS)
> [email protected] (open list)
I have got it. I will run the script on the patch.
Thanks,
Jiang
> -----Original Message-----
> From: Jiasheng Jiang <[email protected]>
> Sent: Thursday, June 1, 2023 2:19 AM
> To: [email protected]; [email protected]; [email protected];
> [email protected]; David Thompson <[email protected]>; Asmaa
> Mnebhi <[email protected]>; [email protected]; Liming Sun
> <[email protected]>
> Cc: [email protected]; [email protected]; Jiasheng Jiang
> <[email protected]>
> 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 <[email protected]>
> ---
> 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