* [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