* [PATCH] rtc: armada38x: remove unneeded call to platform_set_drvdata()
@ 2023-08-07 12:48 ` Andrei Coardos
0 siblings, 0 replies; 6+ messages in thread
From: Andrei Coardos @ 2023-08-07 12:48 UTC (permalink / raw)
To: linux-kernel, linux-rtc, linux-arm-kernel
Cc: alexandre.belloni, a.zummo, sebastian.hesselbarth,
gregory.clement, andrew, alex, Andrei Coardos
This function call was found to be unnecessary as there is no equivalent
platform_get_drvdata() call to access the private data of the driver. Also,
the private data is defined in this driver, so there is no risk of it being
accessed outside of this driver file.
Signed-off-by: Andrei Coardos <aboutphysycs@gmail.com>
---
drivers/rtc/rtc-armada38x.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/rtc/rtc-armada38x.c b/drivers/rtc/rtc-armada38x.c
index b4139c200676..5ed028429565 100644
--- a/drivers/rtc/rtc-armada38x.c
+++ b/drivers/rtc/rtc-armada38x.c
@@ -527,7 +527,6 @@ static __init int armada38x_rtc_probe(struct platform_device *pdev)
dev_warn(&pdev->dev, "Interrupt not available.\n");
rtc->irq = -1;
}
- platform_set_drvdata(pdev, rtc);
if (rtc->irq != -1)
device_init_wakeup(&pdev->dev, 1);
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] rtc: armada38x: remove unneeded call to platform_set_drvdata()
2023-08-07 12:48 ` Andrei Coardos
@ 2023-08-07 17:32 ` Alexandru Ardelean
-1 siblings, 0 replies; 6+ messages in thread
From: Alexandru Ardelean @ 2023-08-07 17:32 UTC (permalink / raw)
To: Andrei Coardos
Cc: linux-kernel, linux-rtc, linux-arm-kernel, alexandre.belloni,
a.zummo, sebastian.hesselbarth, gregory.clement, andrew
On Mon, Aug 7, 2023 at 3:48 PM Andrei Coardos <aboutphysycs@gmail.com> wrote:
>
> This function call was found to be unnecessary as there is no equivalent
> platform_get_drvdata() call to access the private data of the driver. Also,
> the private data is defined in this driver, so there is no risk of it being
> accessed outside of this driver file.
>
NAK on this.
This still calls dev_get_drvdata() in PM routines to access the private data.
> Signed-off-by: Andrei Coardos <aboutphysycs@gmail.com>
> ---
> drivers/rtc/rtc-armada38x.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/rtc/rtc-armada38x.c b/drivers/rtc/rtc-armada38x.c
> index b4139c200676..5ed028429565 100644
> --- a/drivers/rtc/rtc-armada38x.c
> +++ b/drivers/rtc/rtc-armada38x.c
> @@ -527,7 +527,6 @@ static __init int armada38x_rtc_probe(struct platform_device *pdev)
> dev_warn(&pdev->dev, "Interrupt not available.\n");
> rtc->irq = -1;
> }
> - platform_set_drvdata(pdev, rtc);
>
> if (rtc->irq != -1)
> device_init_wakeup(&pdev->dev, 1);
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rtc: armada38x: remove unneeded call to platform_set_drvdata()
@ 2023-08-07 17:32 ` Alexandru Ardelean
0 siblings, 0 replies; 6+ messages in thread
From: Alexandru Ardelean @ 2023-08-07 17:32 UTC (permalink / raw)
To: Andrei Coardos
Cc: linux-kernel, linux-rtc, linux-arm-kernel, alexandre.belloni,
a.zummo, sebastian.hesselbarth, gregory.clement, andrew
On Mon, Aug 7, 2023 at 3:48 PM Andrei Coardos <aboutphysycs@gmail.com> wrote:
>
> This function call was found to be unnecessary as there is no equivalent
> platform_get_drvdata() call to access the private data of the driver. Also,
> the private data is defined in this driver, so there is no risk of it being
> accessed outside of this driver file.
>
NAK on this.
This still calls dev_get_drvdata() in PM routines to access the private data.
> Signed-off-by: Andrei Coardos <aboutphysycs@gmail.com>
> ---
> drivers/rtc/rtc-armada38x.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/rtc/rtc-armada38x.c b/drivers/rtc/rtc-armada38x.c
> index b4139c200676..5ed028429565 100644
> --- a/drivers/rtc/rtc-armada38x.c
> +++ b/drivers/rtc/rtc-armada38x.c
> @@ -527,7 +527,6 @@ static __init int armada38x_rtc_probe(struct platform_device *pdev)
> dev_warn(&pdev->dev, "Interrupt not available.\n");
> rtc->irq = -1;
> }
> - platform_set_drvdata(pdev, rtc);
>
> if (rtc->irq != -1)
> device_init_wakeup(&pdev->dev, 1);
> --
> 2.34.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rtc: armada38x: remove unneeded call to platform_set_drvdata()
2023-08-07 12:48 ` Andrei Coardos
@ 2023-08-07 22:36 ` Russell King (Oracle)
-1 siblings, 0 replies; 6+ messages in thread
From: Russell King (Oracle) @ 2023-08-07 22:36 UTC (permalink / raw)
To: Andrei Coardos
Cc: linux-kernel, linux-rtc, linux-arm-kernel, alexandre.belloni,
a.zummo, sebastian.hesselbarth, gregory.clement, andrew, alex
On Mon, Aug 07, 2023 at 03:48:11PM +0300, Andrei Coardos wrote:
> This function call was found to be unnecessary as there is no equivalent
> platform_get_drvdata() call to access the private data of the driver. Also,
> the private data is defined in this driver, so there is no risk of it being
> accessed outside of this driver file.
I hope you're not going about doing this _broken_ thing and thinking
"if there's no platform_get_drvdata(), then there shouldn't be any
platform_set_drvdata()" because that is WRONG. Wrong wrong wrong wrong
wrong.
platform_set_drvdata(pdev, foo);
sets the driver data on dev, and is basically implemented as:
dev_set_drvdata(&pdev->dev, foo);
So, to access the driver data, one can either use:
platform_get_drvdata(pdev)
or:
dev_get_drvdata(dev)
where one is dealing with a "struct device" pointer. So, the lack of
any platform_get_drvdata() does *NOT* mean that platform_set_drvdata()
can be removed.
If that's what you've been doing, you are *creating* bugs - and you
need to stop until you properly understand what you are doing.
Thanks anyway, but... your change is totally wrong and will wreck the
driver, causing it to oops the kernel.
Strong and definite NAK.
I do *NOT* expect to ever see a patch like this again for this driver.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rtc: armada38x: remove unneeded call to platform_set_drvdata()
@ 2023-08-07 22:36 ` Russell King (Oracle)
0 siblings, 0 replies; 6+ messages in thread
From: Russell King (Oracle) @ 2023-08-07 22:36 UTC (permalink / raw)
To: Andrei Coardos
Cc: linux-kernel, linux-rtc, linux-arm-kernel, alexandre.belloni,
a.zummo, sebastian.hesselbarth, gregory.clement, andrew, alex
On Mon, Aug 07, 2023 at 03:48:11PM +0300, Andrei Coardos wrote:
> This function call was found to be unnecessary as there is no equivalent
> platform_get_drvdata() call to access the private data of the driver. Also,
> the private data is defined in this driver, so there is no risk of it being
> accessed outside of this driver file.
I hope you're not going about doing this _broken_ thing and thinking
"if there's no platform_get_drvdata(), then there shouldn't be any
platform_set_drvdata()" because that is WRONG. Wrong wrong wrong wrong
wrong.
platform_set_drvdata(pdev, foo);
sets the driver data on dev, and is basically implemented as:
dev_set_drvdata(&pdev->dev, foo);
So, to access the driver data, one can either use:
platform_get_drvdata(pdev)
or:
dev_get_drvdata(dev)
where one is dealing with a "struct device" pointer. So, the lack of
any platform_get_drvdata() does *NOT* mean that platform_set_drvdata()
can be removed.
If that's what you've been doing, you are *creating* bugs - and you
need to stop until you properly understand what you are doing.
Thanks anyway, but... your change is totally wrong and will wreck the
driver, causing it to oops the kernel.
Strong and definite NAK.
I do *NOT* expect to ever see a patch like this again for this driver.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread