Messages in this thread | ![/](/images/icornerl.gif) | | From | David Thompson <> | Subject | RE: [PATCH] mlxbf_gige: Add missing check for platform_get_irq | Date | Fri, 2 Jun 2023 14:56:49 +0000 |
| |
> -----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
| ![\](/images/icornerr.gif) |