| Message ID | 20230731112755.1943630-1-ruanjinjie@huawei.com |
|---|---|
| State | Accepted |
| Delegated to: | Andi Shyti |
| Headers | show |
| Series | [-next] i2c: stm32: fix the return value handle for platform_get_irq() | expand |
Hi Ruan, On Mon, Jul 31, 2023 at 07:27:55PM +0800, Ruan Jinjie wrote: > There is no possible for platform_get_irq() to return 0, > and the return value of platform_get_irq() is more sensible > to show the error reason. > > Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> > --- > drivers/i2c/busses/i2c-stm32f7.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c > index e897d9101434..579b30581725 100644 > --- a/drivers/i2c/busses/i2c-stm32f7.c > +++ b/drivers/i2c/busses/i2c-stm32f7.c > @@ -2121,12 +2121,12 @@ static int stm32f7_i2c_probe(struct platform_device *pdev) > phy_addr = (dma_addr_t)res->start; > > irq_event = platform_get_irq(pdev, 0); > - if (irq_event <= 0) > - return irq_event ? : -ENOENT; > + if (irq_event < 0) > + return irq_event; > > irq_error = platform_get_irq(pdev, 1); > - if (irq_error <= 0) > - return irq_error ? : -ENOENT; > + if (irq_error < 0) > + return irq_error; Correct, from patch ce753ad1549cbe ("platform: finally disallow IRQ0 in platform_get_irq() and its ilk") this check is already done inside platform_get_irq(); Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Andi > > i2c_dev->wakeup_src = of_property_read_bool(pdev->dev.of_node, > "wakeup-source"); > -- > 2.34.1 >
Hi Ruan, Just a nitpick here that does not affect my r-b... > On Mon, Jul 31, 2023 at 07:27:55PM +0800, Ruan Jinjie wrote: > > There is no possible for platform_get_irq() to return 0, > > and the return value of platform_get_irq() is more sensible > > to show the error reason. can we rephrase the whole commit to: i2c: stm32: Do not check for 0 return after calling platform_get_irq() It is not possible for platform_get_irq() to return 0. Use the return value from platform_get_irq(). Two notes on the commit log: - Use capital letter after "i2c: stm32: Fix..." - This is not a fix, so that we shouldn't use the word "fix" in the title. just little notes for future patches :) Thanks, Andi > > Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> > > --- > > drivers/i2c/busses/i2c-stm32f7.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c > > index e897d9101434..579b30581725 100644 > > --- a/drivers/i2c/busses/i2c-stm32f7.c > > +++ b/drivers/i2c/busses/i2c-stm32f7.c > > @@ -2121,12 +2121,12 @@ static int stm32f7_i2c_probe(struct platform_device *pdev) > > phy_addr = (dma_addr_t)res->start; > > > > irq_event = platform_get_irq(pdev, 0); > > - if (irq_event <= 0) > > - return irq_event ? : -ENOENT; > > + if (irq_event < 0) > > + return irq_event; > > > > irq_error = platform_get_irq(pdev, 1); > > - if (irq_error <= 0) > > - return irq_error ? : -ENOENT; > > + if (irq_error < 0) > > + return irq_error; > > Correct, from patch ce753ad1549cbe ("platform: finally disallow > IRQ0 in platform_get_irq() and its ilk") this check is already > done inside platform_get_irq(); > > Reviewed-by: Andi Shyti <andi.shyti@kernel.org> > > Andi > > > > > i2c_dev->wakeup_src = of_property_read_bool(pdev->dev.of_node, > > "wakeup-source"); > > -- > > 2.34.1 > >
On 2023/8/2 7:52, Andi Shyti wrote: > Hi Ruan, > > Just a nitpick here that does not affect my r-b... > >> On Mon, Jul 31, 2023 at 07:27:55PM +0800, Ruan Jinjie wrote: >>> There is no possible for platform_get_irq() to return 0, >>> and the return value of platform_get_irq() is more sensible >>> to show the error reason. > > can we rephrase the whole commit to: > > i2c: stm32: Do not check for 0 return after calling platform_get_irq() > > It is not possible for platform_get_irq() to return 0. Use the > return value from platform_get_irq(). > > Two notes on the commit log: > > - Use capital letter after "i2c: stm32: Fix..." > - This is not a fix, so that we shouldn't use the word "fix" in > the title. > > just little notes for future patches :) Thank you for your valuable advice! I will keep an eye out for these in future patches. > > Thanks, > Andi > >>> Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> >>> --- >>> drivers/i2c/busses/i2c-stm32f7.c | 8 ++++---- >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c >>> index e897d9101434..579b30581725 100644 >>> --- a/drivers/i2c/busses/i2c-stm32f7.c >>> +++ b/drivers/i2c/busses/i2c-stm32f7.c >>> @@ -2121,12 +2121,12 @@ static int stm32f7_i2c_probe(struct platform_device *pdev) >>> phy_addr = (dma_addr_t)res->start; >>> >>> irq_event = platform_get_irq(pdev, 0); >>> - if (irq_event <= 0) >>> - return irq_event ? : -ENOENT; >>> + if (irq_event < 0) >>> + return irq_event; >>> >>> irq_error = platform_get_irq(pdev, 1); >>> - if (irq_error <= 0) >>> - return irq_error ? : -ENOENT; >>> + if (irq_error < 0) >>> + return irq_error; >> >> Correct, from patch ce753ad1549cbe ("platform: finally disallow >> IRQ0 in platform_get_irq() and its ilk") this check is already >> done inside platform_get_irq(); >> >> Reviewed-by: Andi Shyti <andi.shyti@kernel.org> >> >> Andi >> >>> >>> i2c_dev->wakeup_src = of_property_read_bool(pdev->dev.of_node, >>> "wakeup-source"); >>> -- >>> 2.34.1 >>>
Hi Ruan, thanks for your patch. On Mon, Jul 31, 2023 at 07:27:55PM +0800, Ruan Jinjie wrote: > There is no possible for platform_get_irq() to return 0, > and the return value of platform_get_irq() is more sensible > to show the error reason. > > Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> > --- > drivers/i2c/busses/i2c-stm32f7.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c > index e897d9101434..579b30581725 100644 > --- a/drivers/i2c/busses/i2c-stm32f7.c > +++ b/drivers/i2c/busses/i2c-stm32f7.c > @@ -2121,12 +2121,12 @@ static int stm32f7_i2c_probe(struct platform_device *pdev) > phy_addr = (dma_addr_t)res->start; > > irq_event = platform_get_irq(pdev, 0); > - if (irq_event <= 0) > - return irq_event ? : -ENOENT; > + if (irq_event < 0) > + return irq_event; > > irq_error = platform_get_irq(pdev, 1); > - if (irq_error <= 0) > - return irq_error ? : -ENOENT; > + if (irq_error < 0) > + return irq_error; > > i2c_dev->wakeup_src = of_property_read_bool(pdev->dev.of_node, > "wakeup-source"); Acked-by: Alain Volmat <alain.volmat@foss.st.com> Regards Alain > -- > 2.34.1 >
Hi On Mon, 31 Jul 2023 19:27:55 +0800, Ruan Jinjie wrote: > There is no possible for platform_get_irq() to return 0, > and the return value of platform_get_irq() is more sensible > to show the error reason. > > With the commit log fixed, applied to i2c/andi-for-next on https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git Please note that this patch may still undergo further evaluation and the final decision will be made in collaboration with Wolfram. Thank you, Andi Patches applied =============== [1/1] i2c: stm32: fix the return value handle for platform_get_irq() commit: 603b3cf89d8a96cc0e51eb15853f28944eb78f31
On 2023/8/5 9:29, Andi Shyti wrote: > Hi > > On Mon, 31 Jul 2023 19:27:55 +0800, Ruan Jinjie wrote: >> There is no possible for platform_get_irq() to return 0, >> and the return value of platform_get_irq() is more sensible >> to show the error reason. >> >> > > With the commit log fixed, applied to i2c/andi-for-next on Thank you! > > https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git > > Please note that this patch may still undergo further evaluation > and the final decision will be made in collaboration with > Wolfram. > > Thank you, > Andi > > Patches applied > =============== > [1/1] i2c: stm32: fix the return value handle for platform_get_irq() > commit: 603b3cf89d8a96cc0e51eb15853f28944eb78f31 >
On Sat, Aug 05, 2023 at 03:29:12AM +0200, Andi Shyti wrote: > Hi > > On Mon, 31 Jul 2023 19:27:55 +0800, Ruan Jinjie wrote: > > There is no possible for platform_get_irq() to return 0, > > and the return value of platform_get_irq() is more sensible > > to show the error reason. > > > > Applied to for-next (via Andi's branch), thanks!
diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c index e897d9101434..579b30581725 100644 --- a/drivers/i2c/busses/i2c-stm32f7.c +++ b/drivers/i2c/busses/i2c-stm32f7.c @@ -2121,12 +2121,12 @@ static int stm32f7_i2c_probe(struct platform_device *pdev) phy_addr = (dma_addr_t)res->start; irq_event = platform_get_irq(pdev, 0); - if (irq_event <= 0) - return irq_event ? : -ENOENT; + if (irq_event < 0) + return irq_event; irq_error = platform_get_irq(pdev, 1); - if (irq_error <= 0) - return irq_error ? : -ENOENT; + if (irq_error < 0) + return irq_error; i2c_dev->wakeup_src = of_property_read_bool(pdev->dev.of_node, "wakeup-source");
There is no possible for platform_get_irq() to return 0, and the return value of platform_get_irq() is more sensible to show the error reason. Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com> --- drivers/i2c/busses/i2c-stm32f7.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)