[bug report] phy: apple: Add Apple Type-C PHY

public inbox for asahi@lists.linux.dev
 help / color / mirror / Atom feed

* [bug report] phy: apple: Add Apple Type-C PHY
       [not found] <caa37f28-a2e8-4e0a-a9ce-a365ce805e4b@stanley.mountain>
@ 2026-02-06 13:40 ` Dan Carpenter
  2026-02-06 21:47   ` Janne Grunau
  0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2026-02-06 13:40 UTC (permalink / raw)
  To: Sven Peter
  Cc: Neal Gompa, Neil Armstrong, asahi, linux-arm-kernel, linux-phy,
	linux-kernel

[ Smatch checking is paused while we raise funding.  #SadFace
  https://lore.kernel.org/all/aTaiGSbWZ9DJaGo7@stanley.mountain/ -dan ]

Hello Sven Peter,

Commit 8e98ca1e74db ("phy: apple: Add Apple Type-C PHY") from Dec 14,
2025 (linux-next), leads to the following Smatch static checker
warning:

	drivers/phy/apple/atc.c:2209 atcphy_map_resources()
	warn: 'resources[i]->addr' isn't an ERR_PTR

drivers/phy/apple/atc.c
    2191 static int atcphy_map_resources(struct platform_device *pdev, struct apple_atcphy *atcphy)
    2192 {
    2193         struct {
    2194                 const char *name;
    2195                 void __iomem **addr;
    2196                 struct resource **res;
    2197         } resources[] = {
    2198                 { "core", &atcphy->regs.core, &atcphy->res.core },
    2199                 { "lpdptx", &atcphy->regs.lpdptx, NULL },
    2200                 { "axi2af", &atcphy->regs.axi2af, &atcphy->res.axi2af },
    2201                 { "usb2phy", &atcphy->regs.usb2phy, NULL },
    2202                 { "pipehandler", &atcphy->regs.pipehandler, NULL },
    2203         };
    2204         struct resource *res;
    2205 
    2206         for (int i = 0; i < ARRAY_SIZE(resources); i++) {
    2207                 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, resources[i].name);
    2208                 *resources[i].addr = devm_ioremap_resource(&pdev->dev, res);
--> 2209                 if (IS_ERR(resources[i].addr))

This is checking the wrong variable.  The * is missing.
if (IS_ERR(*resources[i].addr)) {

    2210                         return dev_err_probe(atcphy->dev, PTR_ERR(resources[i].addr),
    2211                                              "Unable to map %s regs", resources[i].name);
    2212 
    2213                 if (resources[i].res)
    2214                         *resources[i].res = res;
    2215         }
    2216 
    2217         return 0;
    2218 }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [bug report] phy: apple: Add Apple Type-C PHY
  2026-02-06 13:40 ` [bug report] phy: apple: Add Apple Type-C PHY Dan Carpenter
@ 2026-02-06 21:47   ` Janne Grunau
  2026-02-06 21:48     ` Sven Peter
  0 siblings, 1 reply; 3+ messages in thread
From: Janne Grunau @ 2026-02-06 21:47 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Sven Peter, Neal Gompa, Neil Armstrong, asahi, linux-arm-kernel,
	linux-phy, linux-kernel

On Fri, Feb 06, 2026 at 04:40:47PM +0300, Dan Carpenter wrote:
> [ Smatch checking is paused while we raise funding.  #SadFace
>   https://lore.kernel.org/all/aTaiGSbWZ9DJaGo7@stanley.mountain/ -dan ]

This is unfortunate, there have been useful bug reports.

> Commit 8e98ca1e74db ("phy: apple: Add Apple Type-C PHY") from Dec 14,
> 2025 (linux-next), leads to the following Smatch static checker
> warning:
> 
> 	drivers/phy/apple/atc.c:2209 atcphy_map_resources()
> 	warn: 'resources[i]->addr' isn't an ERR_PTR
> 
> drivers/phy/apple/atc.c
>     2191 static int atcphy_map_resources(struct platform_device *pdev, struct apple_atcphy *atcphy)
>     2192 {
>     2193         struct {
>     2194                 const char *name;
>     2195                 void __iomem **addr;
>     2196                 struct resource **res;
>     2197         } resources[] = {
>     2198                 { "core", &atcphy->regs.core, &atcphy->res.core },
>     2199                 { "lpdptx", &atcphy->regs.lpdptx, NULL },
>     2200                 { "axi2af", &atcphy->regs.axi2af, &atcphy->res.axi2af },
>     2201                 { "usb2phy", &atcphy->regs.usb2phy, NULL },
>     2202                 { "pipehandler", &atcphy->regs.pipehandler, NULL },
>     2203         };
>     2204         struct resource *res;
>     2205 
>     2206         for (int i = 0; i < ARRAY_SIZE(resources); i++) {
>     2207                 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, resources[i].name);
>     2208                 *resources[i].addr = devm_ioremap_resource(&pdev->dev, res);
> --> 2209                 if (IS_ERR(resources[i].addr))
> 
> This is checking the wrong variable.  The * is missing.
> if (IS_ERR(*resources[i].addr)) {

This issue was identified by testing and is fixed in next by commit
7d55b44e2be1 ("phy: apple: atc: Actually check return value of
devm_apple_tunable_parse").

https://lore.kernel.org/all/20260104-atcphy-tunable-fix-v2-1-84e5c2a57aaa@kernel.org/

Thanks for the report

Janne

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [bug report] phy: apple: Add Apple Type-C PHY
  2026-02-06 21:47   ` Janne Grunau
@ 2026-02-06 21:48     ` Sven Peter
  0 siblings, 0 replies; 3+ messages in thread
From: Sven Peter @ 2026-02-06 21:48 UTC (permalink / raw)
  To: Janne Grunau, Dan Carpenter
  Cc: Neal Gompa, Neil Armstrong, asahi, linux-arm-kernel, linux-phy,
	linux-kernel

On 06.02.26 22:47, Janne Grunau wrote:
> On Fri, Feb 06, 2026 at 04:40:47PM +0300, Dan Carpenter wrote:
>> [ Smatch checking is paused while we raise funding.  #SadFace
>>    https://lore.kernel.org/all/aTaiGSbWZ9DJaGo7@stanley.mountain/ -dan ]
> 
> This is unfortunate, there have been useful bug reports.
> 
>> Commit 8e98ca1e74db ("phy: apple: Add Apple Type-C PHY") from Dec 14,
>> 2025 (linux-next), leads to the following Smatch static checker
>> warning:
>>
>> 	drivers/phy/apple/atc.c:2209 atcphy_map_resources()
>> 	warn: 'resources[i]->addr' isn't an ERR_PTR
>>
>> drivers/phy/apple/atc.c
>>      2191 static int atcphy_map_resources(struct platform_device *pdev, struct apple_atcphy *atcphy)
>>      2192 {
>>      2193         struct {
>>      2194                 const char *name;
>>      2195                 void __iomem **addr;
>>      2196                 struct resource **res;
>>      2197         } resources[] = {
>>      2198                 { "core", &atcphy->regs.core, &atcphy->res.core },
>>      2199                 { "lpdptx", &atcphy->regs.lpdptx, NULL },
>>      2200                 { "axi2af", &atcphy->regs.axi2af, &atcphy->res.axi2af },
>>      2201                 { "usb2phy", &atcphy->regs.usb2phy, NULL },
>>      2202                 { "pipehandler", &atcphy->regs.pipehandler, NULL },
>>      2203         };
>>      2204         struct resource *res;
>>      2205
>>      2206         for (int i = 0; i < ARRAY_SIZE(resources); i++) {
>>      2207                 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, resources[i].name);
>>      2208                 *resources[i].addr = devm_ioremap_resource(&pdev->dev, res);
>> --> 2209                 if (IS_ERR(resources[i].addr))
>>
>> This is checking the wrong variable.  The * is missing.
>> if (IS_ERR(*resources[i].addr)) {
> 
> This issue was identified by testing and is fixed in next by commit
> 7d55b44e2be1 ("phy: apple: atc: Actually check return value of
> devm_apple_tunable_parse").
> 
> https://lore.kernel.org/all/20260104-atcphy-tunable-fix-v2-1-84e5c2a57aaa@kernel.org/

I think I actually messed this up *twice*! Once for the tunables and 
once again for the resources here :(


Sven



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-02-06 21:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <caa37f28-a2e8-4e0a-a9ce-a365ce805e4b@stanley.mountain>
2026-02-06 13:40 ` [bug report] phy: apple: Add Apple Type-C PHY Dan Carpenter
2026-02-06 21:47   ` Janne Grunau
2026-02-06 21:48     ` Sven Peter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox