| Message ID | 20231107202818.4005791-2-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hwtracing: Convert to platform remove callback returning void | expand |
On 2023/11/8 4:28, Uwe Kleine-König wrote: > If smb_config_inport() fails it's still necessary to unregister all > resources. As smb_config_inport() already emits an error message on > failure, there is no need to add another one. By not returning the error > code, a second error message (about the return value being ignored) is > suppressed. > > Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System Memory Buffer driver") > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Hi, thanks, for the report. Can you try this patch and help review it? https://lore.kernel.org/linux-arm-kernel/20231021083822.18239-3-hejunhao3@huawei.com/ This patch should have fixed this issue. Best regards, Junhao. > --- > drivers/hwtracing/coresight/ultrasoc-smb.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c > index e9a32a97fbee..10e7d4852112 100644 > --- a/drivers/hwtracing/coresight/ultrasoc-smb.c > +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c > @@ -613,11 +613,8 @@ static int smb_probe(struct platform_device *pdev) > static int smb_remove(struct platform_device *pdev) > { > struct smb_drv_data *drvdata = platform_get_drvdata(pdev); > - int ret; > > - ret = smb_config_inport(&pdev->dev, false); > - if (ret) > - return ret; > + smb_config_inport(&pdev->dev, false); > > smb_unregister_sink(drvdata); >
Hello, On Thu, Nov 09, 2023 at 05:41:55PM +0800, hejunhao wrote: > On 2023/11/8 4:28, Uwe Kleine-König wrote: > > If smb_config_inport() fails it's still necessary to unregister all > > resources. As smb_config_inport() already emits an error message on > > failure, there is no need to add another one. By not returning the error > > code, a second error message (about the return value being ignored) is > > suppressed. > > > > Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System Memory Buffer driver") > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > thanks, for the report. Can you try this patch and help review it? > > https://lore.kernel.org/linux-arm-kernel/20231021083822.18239-3-hejunhao3@huawei.com/ Well, I cannot try it, but it improves things a bit. The remaining problem is that smb_remove() should return 0 even if smb_config_inport() fails. Best regards Uwe
On 2023/11/9 17:53, Uwe Kleine-König wrote: > Hello, > > On Thu, Nov 09, 2023 at 05:41:55PM +0800, hejunhao wrote: >> On 2023/11/8 4:28, Uwe Kleine-König wrote: >>> If smb_config_inport() fails it's still necessary to unregister all >>> resources. As smb_config_inport() already emits an error message on >>> failure, there is no need to add another one. By not returning the error >>> code, a second error message (about the return value being ignored) is >>> suppressed. >>> >>> Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System Memory Buffer driver") >>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> >> thanks, for the report. Can you try this patch and help review it? >> >> https://lore.kernel.org/linux-arm-kernel/20231021083822.18239-3-hejunhao3@huawei.com/ > Well, I cannot try it, but it improves things a bit. The remaining > problem is that smb_remove() should return 0 even if smb_config_inport() > fails. > > Best regards > Uwe Yes, I will do that. By the way, Can I add patch 8.8 to my patchset? I think this is a good way to resolve patch conflicts. Best regards, Junhao. >
On Thu, Nov 09, 2023 at 08:14:34PM +0800, hejunhao wrote: > > On 2023/11/9 17:53, Uwe Kleine-König wrote: > > Hello, > > > > On Thu, Nov 09, 2023 at 05:41:55PM +0800, hejunhao wrote: > > > On 2023/11/8 4:28, Uwe Kleine-König wrote: > > > > If smb_config_inport() fails it's still necessary to unregister all > > > > resources. As smb_config_inport() already emits an error message on > > > > failure, there is no need to add another one. By not returning the error > > > > code, a second error message (about the return value being ignored) is > > > > suppressed. > > > > > > > > Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System Memory Buffer driver") > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > > thanks, for the report. Can you try this patch and help review it? > > > > > > https://lore.kernel.org/linux-arm-kernel/20231021083822.18239-3-hejunhao3@huawei.com/ > > Well, I cannot try it, but it improves things a bit. The remaining > > problem is that smb_remove() should return 0 even if smb_config_inport() > > fails. > > > > Best regards > > Uwe > > Yes, I will do that. > By the way, Can I add patch 8.8 to my patchset? I think this is a good way > to resolve patch conflicts. I don't know how patch application works in drivers/hwtracing. I will probably check if these patches are in next in a few weeks and resend if they are not. If you adding patch #8 to your patch set needs more coordination, please tell me. Best regards Uwe
Hi Uwe, On 2023/11/9 21:11, Uwe Kleine-König wrote: > On Thu, Nov 09, 2023 at 08:14:34PM +0800, hejunhao wrote: >> On 2023/11/9 17:53, Uwe Kleine-König wrote: >>> Hello, >>> >>> On Thu, Nov 09, 2023 at 05:41:55PM +0800, hejunhao wrote: >>>> On 2023/11/8 4:28, Uwe Kleine-König wrote: >>>>> If smb_config_inport() fails it's still necessary to unregister all >>>>> resources. As smb_config_inport() already emits an error message on >>>>> failure, there is no need to add another one. By not returning the error >>>>> code, a second error message (about the return value being ignored) is >>>>> suppressed. >>>>> >>>>> Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System Memory Buffer driver") >>>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> >>>> thanks, for the report. Can you try this patch and help review it? >>>> >>>> https://lore.kernel.org/linux-arm-kernel/20231021083822.18239-3-hejunhao3@huawei.com/ >>> Well, I cannot try it, but it improves things a bit. The remaining >>> problem is that smb_remove() should return 0 even if smb_config_inport() >>> fails. >>> >>> Best regards >>> Uwe >> Yes, I will do that. >> By the way, Can I add patch 8.8 to my patchset? I think this is a good way >> to resolve patch conflicts. > I don't know how patch application works in drivers/hwtracing. I will > probably check if these patches are in next in a few weeks and resend if > they are not. If you adding patch #8 to your patch set needs more > coordination, please tell me. > > Best regards > Uwe Forgive me for interrupting. I sent v3 and it ignored the return value of smb_config_inport(). Can you rebase to that?:-) https://lore.kernel.org/lkml/20231114133346.30489-1-hejunhao3@huawei.com/ Best regards, Junhao.
On 16/11/2023 12:35, hejunhao wrote: > Hi Uwe, > > On 2023/11/9 21:11, Uwe Kleine-K�nig wrote: >> On Thu, Nov 09, 2023 at 08:14:34PM +0800, hejunhao wrote: >>> On 2023/11/9 17:53, Uwe Kleine-K�nig wrote: >>>> Hello, >>>> >>>> On Thu, Nov 09, 2023 at 05:41:55PM +0800, hejunhao wrote: >>>>> On 2023/11/8 4:28, Uwe Kleine-K�nig wrote: >>>>>> If smb_config_inport() fails it's still necessary to unregister all >>>>>> resources. As smb_config_inport() already emits an error message on >>>>>> failure, there is no need to add another one. By not returning the >>>>>> error >>>>>> code, a second error message (about the return value being >>>>>> ignored) is >>>>>> suppressed. >>>>>> >>>>>> Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System >>>>>> Memory Buffer driver") >>>>>> Signed-off-by: Uwe Kleine-K�nig <u.kleine-koenig@pengutronix.de> >>>>> thanks, for the report. Can you try this patch and help review it? >>>>> >>>>> https://lore.kernel.org/linux-arm-kernel/20231021083822.18239-3-hejunhao3@huawei.com/ >>>> Well, I cannot try it, but it improves things a bit. The remaining >>>> problem is that smb_remove() should return 0 even if >>>> smb_config_inport() >>>> fails. >>>> >>>> Best regards >>>> Uwe >>> Yes, I will do that. >>> By the way, Can I add patch 8.8 to my patchset? I think this is a >>> good way >>> to resolve patch conflicts. >> I don't know how patch application works in drivers/hwtracing. I will >> probably check if these patches are in next in a few weeks and resend if >> they are not. If you adding patch #8 to your patch set needs more >> coordination, please tell me. >> >> Best regards >> Uwe > > Forgive me for interrupting. > I sent v3 and it ignored the return value of smb_config_inport(). Can > you rebase to that?:-) > > https://lore.kernel.org/lkml/20231114133346.30489-1-hejunhao3@huawei.com/ fyi, I have queued all the patches in the link above, to fixes branch on coresight and it would soon appear on next branch too. So you should be able to rebase it on coresight/next. Suzuki > > Best regards, > Junhao. >
diff --git a/drivers/hwtracing/coresight/ultrasoc-smb.c b/drivers/hwtracing/coresight/ultrasoc-smb.c index e9a32a97fbee..10e7d4852112 100644 --- a/drivers/hwtracing/coresight/ultrasoc-smb.c +++ b/drivers/hwtracing/coresight/ultrasoc-smb.c @@ -613,11 +613,8 @@ static int smb_probe(struct platform_device *pdev) static int smb_remove(struct platform_device *pdev) { struct smb_drv_data *drvdata = platform_get_drvdata(pdev); - int ret; - ret = smb_config_inport(&pdev->dev, false); - if (ret) - return ret; + smb_config_inport(&pdev->dev, false); smb_unregister_sink(drvdata);
If smb_config_inport() fails it's still necessary to unregister all resources. As smb_config_inport() already emits an error message on failure, there is no need to add another one. By not returning the error code, a second error message (about the return value being ignored) is suppressed. Fixes: 06f5c2926aaa ("drivers/coresight: Add UltraSoc System Memory Buffer driver") Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/hwtracing/coresight/ultrasoc-smb.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)