diff mbox series

nfc: st95hf: Make spi remove() callback return zero

Message ID 20211019204916.3834554-1-u.kleine-koenig@pengutronix.de (mailing list archive)
State Accepted
Commit 641e3fd1a038c68045bbf89d78502f4b4bbc7284
Delegated to: Netdev Maintainers
Headers show
Series nfc: st95hf: Make spi remove() callback return zero | expand

Checks

Context Check Description
netdev/cover_letter success Single patches do not need cover letters
netdev/fixes_present success Fixes tag not required for -next series
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cc_maintainers warning 2 maintainers not CCed: wengjianfeng@yulong.com kuba@kernel.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success No Fixes tag
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 20 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success No static functions without inline keyword in header files

Commit Message

Uwe Kleine-König Oct. 19, 2021, 8:49 p.m. UTC
If something goes wrong in the remove callback, returning an error code
just results in an error message. The device still disappears.

So don't skip disabling the regulator in st95hf_remove() if resetting
the controller via spi fails. Also don't return an error code which just
results in two error messages.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/nfc/st95hf/core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Krzysztof Kozlowski Oct. 20, 2021, 6:55 a.m. UTC | #1
On 19/10/2021 22:49, Uwe Kleine-König wrote:
> If something goes wrong in the remove callback, returning an error code
> just results in an error message. The device still disappears.
> 
> So don't skip disabling the regulator in st95hf_remove() if resetting
> the controller via spi fails. Also don't return an error code which just
> results in two error messages.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/nfc/st95hf/core.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>


Best regards,
Krzysztof
Uwe Kleine-König Oct. 20, 2021, 7:05 a.m. UTC | #2
Hello Krzysztof,

On Wed, Oct 20, 2021 at 08:55:51AM +0200, Krzysztof Kozlowski wrote:
> On 19/10/2021 22:49, Uwe Kleine-König wrote:
> > If something goes wrong in the remove callback, returning an error code
> > just results in an error message. The device still disappears.
> > 
> > So don't skip disabling the regulator in st95hf_remove() if resetting
> > the controller via spi fails. Also don't return an error code which just
> > results in two error messages.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  drivers/nfc/st95hf/core.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

Given you're the listed maintainer for NFC, I wonder who will take this
patch? I expected you to take this patch and not "only" give your
Reviewed-by tag.

Best regards
Uwe
Krzysztof Kozlowski Oct. 20, 2021, 7:09 a.m. UTC | #3
On 20/10/2021 09:05, Uwe Kleine-König wrote:
> Hello Krzysztof,
> 
> On Wed, Oct 20, 2021 at 08:55:51AM +0200, Krzysztof Kozlowski wrote:
>> On 19/10/2021 22:49, Uwe Kleine-König wrote:
>>> If something goes wrong in the remove callback, returning an error code
>>> just results in an error message. The device still disappears.
>>>
>>> So don't skip disabling the regulator in st95hf_remove() if resetting
>>> the controller via spi fails. Also don't return an error code which just
>>> results in two error messages.
>>>
>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>>> ---
>>>  drivers/nfc/st95hf/core.c | 6 ++----
>>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> 
> Given you're the listed maintainer for NFC, I wonder who will take this
> patch? I expected you to take this patch and not "only" give your
> Reviewed-by tag.
> 

Yeah, it's not that obvious. Maybe I should write subsystem/maintainer
guide for NFC...

All NFC patches are taken by netdev folks (David and Jakub) via
patchwork. You did not CC them here, but you CC-ed the netdev, so let's
hope it is enough. You also skipped linux-nfc, so you might need a file:

  $ cat .get_maintainer.conf
  --s

Best regards,
Krzysztof
Uwe Kleine-König Oct. 20, 2021, 9:29 a.m. UTC | #4
On Wed, Oct 20, 2021 at 09:09:04AM +0200, Krzysztof Kozlowski wrote:
> On 20/10/2021 09:05, Uwe Kleine-König wrote:
> > Hello Krzysztof,
> > 
> > On Wed, Oct 20, 2021 at 08:55:51AM +0200, Krzysztof Kozlowski wrote:
> >> On 19/10/2021 22:49, Uwe Kleine-König wrote:
> >>> If something goes wrong in the remove callback, returning an error code
> >>> just results in an error message. The device still disappears.
> >>>
> >>> So don't skip disabling the regulator in st95hf_remove() if resetting
> >>> the controller via spi fails. Also don't return an error code which just
> >>> results in two error messages.
> >>>
> >>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> >>> ---
> >>>  drivers/nfc/st95hf/core.c | 6 ++----
> >>>  1 file changed, 2 insertions(+), 4 deletions(-)
> >>
> >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> > 
> > Given you're the listed maintainer for NFC, I wonder who will take this
> > patch? I expected you to take this patch and not "only" give your
> > Reviewed-by tag.
> > 
> 
> Yeah, it's not that obvious. Maybe I should write subsystem/maintainer
> guide for NFC...
> 
> All NFC patches are taken by netdev folks (David and Jakub) via
> patchwork. You did not CC them here, but you CC-ed the netdev, so let's
> hope it is enough. You also skipped linux-nfc, so you might need a file:
> 
>   $ cat .get_maintainer.conf
>   --s

Ah, I handpicked the recipents from the get_maintainer.pl output. (My
intention was to pick linux-nfc, not netdev. Either I misremember or cut
the wrong line.)

Anyhow, I added David and Jakub to Cc: to maybe increase my chances this
patch is noticed by them.

Thanks
Uwe
patchwork-bot+netdevbpf@kernel.org Oct. 20, 2021, 1:50 p.m. UTC | #5
Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Tue, 19 Oct 2021 22:49:16 +0200 you wrote:
> If something goes wrong in the remove callback, returning an error code
> just results in an error message. The device still disappears.
> 
> So don't skip disabling the regulator in st95hf_remove() if resetting
> the controller via spi fails. Also don't return an error code which just
> results in two error messages.
> 
> [...]

Here is the summary with links:
  - nfc: st95hf: Make spi remove() callback return zero
    https://git.kernel.org/netdev/net/c/641e3fd1a038

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
diff mbox series

Patch

diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index d16cf3ff644e..b23f47936473 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -1226,11 +1226,9 @@  static int st95hf_remove(struct spi_device *nfc_spi_dev)
 				 &reset_cmd,
 				 ST95HF_RESET_CMD_LEN,
 				 ASYNC);
-	if (result) {
+	if (result)
 		dev_err(&spictx->spidev->dev,
 			"ST95HF reset failed in remove() err = %d\n", result);
-		return result;
-	}
 
 	/* wait for 3 ms to complete the controller reset process */
 	usleep_range(3000, 4000);
@@ -1239,7 +1237,7 @@  static int st95hf_remove(struct spi_device *nfc_spi_dev)
 	if (stcontext->st95hf_supply)
 		regulator_disable(stcontext->st95hf_supply);
 
-	return result;
+	return 0;
 }
 
 /* Register as SPI protocol driver */