* [PATCH] ata: libahci_platform: use of_platform_device_create() return value
@ 2026-07-09 1:42 Rosen Penev
2026-07-09 1:46 ` Damien Le Moal
2026-07-09 1:50 ` sashiko-bot
0 siblings, 2 replies; 6+ messages in thread
From: Rosen Penev @ 2026-07-09 1:42 UTC (permalink / raw)
To: linux-ide; +Cc: Hans de Goede, Damien Le Moal, Niklas Cassel, open list
of_platform_device_create() already returns the struct platform_device
pointer, so there is no need for a separate of_find_device_by_node()
call. Use the return value directly.
Avoids having to free the reference from of_find_device_by_node().
Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/ata/libahci_platform.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index 6e072d681341..2aa3df58fc9a 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -617,10 +617,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
mask_port_map |= BIT(port);
#ifdef CONFIG_OF_ADDRESS
- of_platform_device_create(child, NULL, NULL);
-
- port_dev = of_find_device_by_node(child);
-
+ port_dev = of_platform_device_create(child, NULL, NULL);
if (port_dev) {
rc = ahci_platform_get_regulator(hpriv, port,
&port_dev->dev);
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ata: libahci_platform: use of_platform_device_create() return value
2026-07-09 1:42 [PATCH] ata: libahci_platform: use of_platform_device_create() return value Rosen Penev
@ 2026-07-09 1:46 ` Damien Le Moal
2026-07-09 1:50 ` sashiko-bot
1 sibling, 0 replies; 6+ messages in thread
From: Damien Le Moal @ 2026-07-09 1:46 UTC (permalink / raw)
To: Rosen Penev, linux-ide; +Cc: Hans de Goede, Niklas Cassel, open list
On 7/9/26 10:42, Rosen Penev wrote:
> of_platform_device_create() already returns the struct platform_device
> pointer, so there is no need for a separate of_find_device_by_node()
> call. Use the return value directly.
>
> Avoids having to free the reference from of_find_device_by_node().
Looks OK to me, but how is the above comment relevant? I do not see any change
that can be described with this sentence. Maybe remove it? (I can do it when
applying)
>
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
> drivers/ata/libahci_platform.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
> index 6e072d681341..2aa3df58fc9a 100644
> --- a/drivers/ata/libahci_platform.c
> +++ b/drivers/ata/libahci_platform.c
> @@ -617,10 +617,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
> mask_port_map |= BIT(port);
>
> #ifdef CONFIG_OF_ADDRESS
> - of_platform_device_create(child, NULL, NULL);
> -
> - port_dev = of_find_device_by_node(child);
> -
> + port_dev = of_platform_device_create(child, NULL, NULL);
> if (port_dev) {
> rc = ahci_platform_get_regulator(hpriv, port,
> &port_dev->dev);
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ata: libahci_platform: use of_platform_device_create() return value
2026-07-09 1:42 [PATCH] ata: libahci_platform: use of_platform_device_create() return value Rosen Penev
2026-07-09 1:46 ` Damien Le Moal
@ 2026-07-09 1:50 ` sashiko-bot
2026-07-09 1:59 ` Rosen Penev
1 sibling, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-07-09 1:50 UTC (permalink / raw)
To: Rosen Penev; +Cc: cassel, linux-ide, dlemoal
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Using the return value of `of_platform_device_create()` breaks target regulator fetching during driver rebind or probe deferral, leading to unpowered SATA ports.
--
--- Patch [1]: [PATCH] ata: libahci_platform: use of_platform_device_create() return value ---
commit b10e7693c196a90e6a36f9889202414e7cb20fc9
Author: Rosen Penev <rosenp@gmail.com>
ata: libahci_platform: use of_platform_device_create() return value
This commit updates ahci_platform_get_resources() to use the return value
of of_platform_device_create() directly, removing the subsequent call to
of_find_device_by_node() to avoid needing to free the reference.
> --- a/drivers/ata/libahci_platform.c
> +++ b/drivers/ata/libahci_platform.c
> @@ -617,10 +617,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
> mask_port_map |= BIT(port);
>
> #ifdef CONFIG_OF_ADDRESS
> - of_platform_device_create(child, NULL, NULL);
> -
> - port_dev = of_find_device_by_node(child);
> -
> + port_dev = of_platform_device_create(child, NULL, NULL);
> if (port_dev) {
> rc = ahci_platform_get_regulator(hpriv, port,
> &port_dev->dev);
[Severity: High]
Does this change break regulator fetching during a probe deferral?
When of_platform_device_create() successfully creates the device on the first
probe attempt, it sets the OF_POPULATED flag on the child device node.
If a subsequent initialization step triggers a probe deferral (for instance,
returning -EPROBE_DEFER), the probe is aborted. However, the driver's cleanup
path does not depopulate the platform device, so the OF_POPULATED flag remains
set on the child node.
During the second probe attempt, of_platform_device_create() sees the
OF_POPULATED flag and returns NULL.
Because port_dev is now NULL, the block above is bypassed, and
ahci_platform_get_regulator() is silently skipped. This leaves the SATA port
without its required target power regulator.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709014230.1905756-1-rosenp@gmail.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ata: libahci_platform: use of_platform_device_create() return value
2026-07-09 1:50 ` sashiko-bot
@ 2026-07-09 1:59 ` Rosen Penev
2026-07-09 2:00 ` Damien Le Moal
0 siblings, 1 reply; 6+ messages in thread
From: Rosen Penev @ 2026-07-09 1:59 UTC (permalink / raw)
To: sashiko-reviews; +Cc: cassel, linux-ide, dlemoal
On Wed, Jul 8, 2026 at 6:50 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Using the return value of `of_platform_device_create()` breaks target regulator fetching during driver rebind or probe deferral, leading to unpowered SATA ports.
> --
>
> --- Patch [1]: [PATCH] ata: libahci_platform: use of_platform_device_create() return value ---
> commit b10e7693c196a90e6a36f9889202414e7cb20fc9
> Author: Rosen Penev <rosenp@gmail.com>
>
> ata: libahci_platform: use of_platform_device_create() return value
>
> This commit updates ahci_platform_get_resources() to use the return value
> of of_platform_device_create() directly, removing the subsequent call to
> of_find_device_by_node() to avoid needing to free the reference.
>
> > --- a/drivers/ata/libahci_platform.c
> > +++ b/drivers/ata/libahci_platform.c
> > @@ -617,10 +617,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
> > mask_port_map |= BIT(port);
> >
> > #ifdef CONFIG_OF_ADDRESS
> > - of_platform_device_create(child, NULL, NULL);
> > -
> > - port_dev = of_find_device_by_node(child);
> > -
> > + port_dev = of_platform_device_create(child, NULL, NULL);
> > if (port_dev) {
> > rc = ahci_platform_get_regulator(hpriv, port,
> > &port_dev->dev);
>
> [Severity: High]
> Does this change break regulator fetching during a probe deferral?
>
> When of_platform_device_create() successfully creates the device on the first
> probe attempt, it sets the OF_POPULATED flag on the child device node.
>
> If a subsequent initialization step triggers a probe deferral (for instance,
> returning -EPROBE_DEFER), the probe is aborted. However, the driver's cleanup
> path does not depopulate the platform device, so the OF_POPULATED flag remains
> set on the child node.
>
> During the second probe attempt, of_platform_device_create() sees the
> OF_POPULATED flag and returns NULL.
>
> Because port_dev is now NULL, the block above is bypassed, and
> ahci_platform_get_regulator() is silently skipped. This leaves the SATA port
> without its required target power regulator.
Welp. So much for that.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260709014230.1905756-1-rosenp@gmail.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ata: libahci_platform: use of_platform_device_create() return value
2026-07-09 1:59 ` Rosen Penev
@ 2026-07-09 2:00 ` Damien Le Moal
2026-07-09 2:01 ` Rosen Penev
0 siblings, 1 reply; 6+ messages in thread
From: Damien Le Moal @ 2026-07-09 2:00 UTC (permalink / raw)
To: Rosen Penev, sashiko-reviews; +Cc: cassel, linux-ide
On 7/9/26 10:59, Rosen Penev wrote:
> On Wed, Jul 8, 2026 at 6:50 PM <sashiko-bot@kernel.org> wrote:
>>
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [High] Using the return value of `of_platform_device_create()` breaks target regulator fetching during driver rebind or probe deferral, leading to unpowered SATA ports.
>> --
>>
>> --- Patch [1]: [PATCH] ata: libahci_platform: use of_platform_device_create() return value ---
>> commit b10e7693c196a90e6a36f9889202414e7cb20fc9
>> Author: Rosen Penev <rosenp@gmail.com>
>>
>> ata: libahci_platform: use of_platform_device_create() return value
>>
>> This commit updates ahci_platform_get_resources() to use the return value
>> of of_platform_device_create() directly, removing the subsequent call to
>> of_find_device_by_node() to avoid needing to free the reference.
>>
>>> --- a/drivers/ata/libahci_platform.c
>>> +++ b/drivers/ata/libahci_platform.c
>>> @@ -617,10 +617,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
>>> mask_port_map |= BIT(port);
>>>
>>> #ifdef CONFIG_OF_ADDRESS
>>> - of_platform_device_create(child, NULL, NULL);
>>> -
>>> - port_dev = of_find_device_by_node(child);
>>> -
>>> + port_dev = of_platform_device_create(child, NULL, NULL);
>>> if (port_dev) {
>>> rc = ahci_platform_get_regulator(hpriv, port,
>>> &port_dev->dev);
>>
>> [Severity: High]
>> Does this change break regulator fetching during a probe deferral?
>>
>> When of_platform_device_create() successfully creates the device on the first
>> probe attempt, it sets the OF_POPULATED flag on the child device node.
>>
>> If a subsequent initialization step triggers a probe deferral (for instance,
>> returning -EPROBE_DEFER), the probe is aborted. However, the driver's cleanup
>> path does not depopulate the platform device, so the OF_POPULATED flag remains
>> set on the child node.
>>
>> During the second probe attempt, of_platform_device_create() sees the
>> OF_POPULATED flag and returns NULL.
>>
>> Because port_dev is now NULL, the block above is bypassed, and
>> ahci_platform_get_regulator() is silently skipped. This leaves the SATA port
>> without its required target power regulator.
> Welp. So much for that.
Indeed. Not a great API... So we would need a pattern like:
if (of_node_check_flag(child, OF_POPULATED))
port_dev = of_find_device_by_node(child);
else
port_dev = of_platform_device_create(child, NULL, NULL);
Which kind of makes the patch not needed. Let's drop it ?
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ata: libahci_platform: use of_platform_device_create() return value
2026-07-09 2:00 ` Damien Le Moal
@ 2026-07-09 2:01 ` Rosen Penev
0 siblings, 0 replies; 6+ messages in thread
From: Rosen Penev @ 2026-07-09 2:01 UTC (permalink / raw)
To: Damien Le Moal; +Cc: sashiko-reviews, cassel, linux-ide
On Wed, Jul 8, 2026 at 7:00 PM Damien Le Moal <dlemoal@kernel.org> wrote:
>
> On 7/9/26 10:59, Rosen Penev wrote:
> > On Wed, Jul 8, 2026 at 6:50 PM <sashiko-bot@kernel.org> wrote:
> >>
> >> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> >> - [High] Using the return value of `of_platform_device_create()` breaks target regulator fetching during driver rebind or probe deferral, leading to unpowered SATA ports.
> >> --
> >>
> >> --- Patch [1]: [PATCH] ata: libahci_platform: use of_platform_device_create() return value ---
> >> commit b10e7693c196a90e6a36f9889202414e7cb20fc9
> >> Author: Rosen Penev <rosenp@gmail.com>
> >>
> >> ata: libahci_platform: use of_platform_device_create() return value
> >>
> >> This commit updates ahci_platform_get_resources() to use the return value
> >> of of_platform_device_create() directly, removing the subsequent call to
> >> of_find_device_by_node() to avoid needing to free the reference.
> >>
> >>> --- a/drivers/ata/libahci_platform.c
> >>> +++ b/drivers/ata/libahci_platform.c
> >>> @@ -617,10 +617,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev,
> >>> mask_port_map |= BIT(port);
> >>>
> >>> #ifdef CONFIG_OF_ADDRESS
> >>> - of_platform_device_create(child, NULL, NULL);
> >>> -
> >>> - port_dev = of_find_device_by_node(child);
> >>> -
> >>> + port_dev = of_platform_device_create(child, NULL, NULL);
> >>> if (port_dev) {
> >>> rc = ahci_platform_get_regulator(hpriv, port,
> >>> &port_dev->dev);
> >>
> >> [Severity: High]
> >> Does this change break regulator fetching during a probe deferral?
> >>
> >> When of_platform_device_create() successfully creates the device on the first
> >> probe attempt, it sets the OF_POPULATED flag on the child device node.
> >>
> >> If a subsequent initialization step triggers a probe deferral (for instance,
> >> returning -EPROBE_DEFER), the probe is aborted. However, the driver's cleanup
> >> path does not depopulate the platform device, so the OF_POPULATED flag remains
> >> set on the child node.
> >>
> >> During the second probe attempt, of_platform_device_create() sees the
> >> OF_POPULATED flag and returns NULL.
> >>
> >> Because port_dev is now NULL, the block above is bypassed, and
> >> ahci_platform_get_regulator() is silently skipped. This leaves the SATA port
> >> without its required target power regulator.
> > Welp. So much for that.
>
> Indeed. Not a great API... So we would need a pattern like:
>
> if (of_node_check_flag(child, OF_POPULATED))
> port_dev = of_find_device_by_node(child);
> else
> port_dev = of_platform_device_create(child, NULL, NULL);
>
> Which kind of makes the patch not needed. Let's drop it ?
Yes.
>
> --
> Damien Le Moal
> Western Digital Research
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-09 2:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-09 1:42 [PATCH] ata: libahci_platform: use of_platform_device_create() return value Rosen Penev
2026-07-09 1:46 ` Damien Le Moal
2026-07-09 1:50 ` sashiko-bot
2026-07-09 1:59 ` Rosen Penev
2026-07-09 2:00 ` Damien Le Moal
2026-07-09 2:01 ` Rosen Penev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox