| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3c0f31ce-38a8-4f1e-8c39-6aa6ac879dc6@hauke-m.de>
Date: Sun, 24 Aug 2025 18:05:28 +0200
From: Hauke Mehrtens <hauke@...ke-m.de>
To: Heiner Kallweit <hkallweit1@...il.com>, Greg Ungerer
<gerg@...ux-m68k.org>, Geert Uytterhoeven <geert@...ux-m68k.org>,
Rafał Miłecki <zajec5@...il.com>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>, Andrew Lunn
<andrew@...n.ch>, Andrew Lunn <andrew+netdev@...n.ch>,
Russell King - ARM Linux <linux@...linux.org.uk>,
Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, David Miller <davem@...emloft.net>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
linux-m68k@...ts.linux-m68k.org, linux-mips@...r.kernel.org,
Takumi Sueda <puhitaku@...il.com>
Subject: Re: [PATCH net-next] net: phy: fixed: let fixed_phy_add always use
addr 0 and remove return value
On 8/22/25 22:36, Heiner Kallweit wrote:
> We have only two users of fixed_phy_add(), both use address 0 and
> ignore the return value. So simplify fixed_phy_add() accordingly.
>
> Whilst at it, constify the fixed_phy_status configs.
>
> Note:
> fixed_phy_add() is a legacy function which shouldn't be used in new
> code, as it's use may be problematic:
> - No check whether a fixed phy exists already at the given address
> - If fixed_phy_register() is called afterwards by any other driver,
> then it will also use phy_addr 0, because fixed_phy_add() ignores
> the ida which manages address assignment
> Drivers using a fixed phy created by fixed_phy_add() in platform code,
> should dynamically create a fixed phy with fixed_phy_register()
> instead.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
> ---
> arch/m68k/coldfire/m5272.c | 4 ++--
> arch/mips/bcm47xx/setup.c | 4 ++--
> drivers/net/phy/fixed_phy.c | 4 ++--
> include/linux/phy_fixed.h | 8 ++------
> 4 files changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/arch/m68k/coldfire/m5272.c b/arch/m68k/coldfire/m5272.c
> index 5b70dfdab..918e2a323 100644
> --- a/arch/m68k/coldfire/m5272.c
> +++ b/arch/m68k/coldfire/m5272.c
> @@ -108,7 +108,7 @@ void __init config_BSP(char *commandp, int size)
> * an ethernet switch. In this case we need to use the fixed phy type,
> * and we need to declare it early in boot.
> */
> -static struct fixed_phy_status nettel_fixed_phy_status __initdata = {
> +static const struct fixed_phy_status nettel_fixed_phy_status __initconst = {
> .link = 1,
> .speed = 100,
> .duplex = 0,
> @@ -119,7 +119,7 @@ static struct fixed_phy_status nettel_fixed_phy_status __initdata = {
> static int __init init_BSP(void)
> {
> m5272_uarts_init();
> - fixed_phy_add(0, &nettel_fixed_phy_status);
> + fixed_phy_add(&nettel_fixed_phy_status);
> clkdev_add_table(m5272_clk_lookup, ARRAY_SIZE(m5272_clk_lookup));
> return 0;
> }
> diff --git a/arch/mips/bcm47xx/setup.c b/arch/mips/bcm47xx/setup.c
> index de426a474..a93a4266d 100644
> --- a/arch/mips/bcm47xx/setup.c
> +++ b/arch/mips/bcm47xx/setup.c
> @@ -256,7 +256,7 @@ static int __init bcm47xx_cpu_fixes(void)
> }
> arch_initcall(bcm47xx_cpu_fixes);
>
> -static struct fixed_phy_status bcm47xx_fixed_phy_status __initdata = {
> +static const struct fixed_phy_status bcm47xx_fixed_phy_status __initconst = {
> .link = 1,
> .speed = SPEED_100,
> .duplex = DUPLEX_FULL,
> @@ -282,7 +282,7 @@ static int __init bcm47xx_register_bus_complete(void)
> bcm47xx_leds_register();
> bcm47xx_workarounds();
>
> - fixed_phy_add(0, &bcm47xx_fixed_phy_status);
> + fixed_phy_add(&bcm47xx_fixed_phy_status);
> return 0;
> }
> device_initcall(bcm47xx_register_bus_complete);
> diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c
> index 7902b35c5..b39532abf 100644
> --- a/drivers/net/phy/fixed_phy.c
> +++ b/drivers/net/phy/fixed_phy.c
> @@ -153,9 +153,9 @@ static int fixed_phy_add_gpiod(unsigned int irq, int phy_addr,
> return 0;
> }
>
> -int fixed_phy_add(int phy_addr, const struct fixed_phy_status *status)
> +void fixed_phy_add(const struct fixed_phy_status *status)
> {
> - return fixed_phy_add_gpiod(PHY_POLL, phy_addr, status, NULL);
> + fixed_phy_add_gpiod(PHY_POLL, 0, status, NULL);
> }
> EXPORT_SYMBOL_GPL(fixed_phy_add);
>
> diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h
> index 5399b9e41..6227a1bde 100644
> --- a/include/linux/phy_fixed.h
> +++ b/include/linux/phy_fixed.h
> @@ -17,7 +17,7 @@ struct net_device;
>
> #if IS_ENABLED(CONFIG_FIXED_PHY)
> extern int fixed_phy_change_carrier(struct net_device *dev, bool new_carrier);
> -int fixed_phy_add(int phy_id, const struct fixed_phy_status *status);
> +void fixed_phy_add(const struct fixed_phy_status *status);
> struct phy_device *fixed_phy_register(const struct fixed_phy_status *status,
> struct device_node *np);
>
> @@ -26,11 +26,7 @@ extern int fixed_phy_set_link_update(struct phy_device *phydev,
> int (*link_update)(struct net_device *,
> struct fixed_phy_status *));
> #else
> -static inline int fixed_phy_add(int phy_id,
> - const struct fixed_phy_status *status)
> -{
> - return -ENODEV;
> -}
> +static inline void fixed_phy_add(const struct fixed_phy_status *status) {}
> static inline struct phy_device *
> fixed_phy_register(const struct fixed_phy_status *status,
> struct device_node *np)
Hi,
I do not use this hardware any more, but Takumi reported that
fixed_phy_add() is not working for the PHY registration on brcm47xx any
more and we have to use fixed_phy_register(), see:
https://github.com/openwrt/openwrt/pull/19610
Does this need a bigger refactoring anyway?
Hauke
Powered by blists - more mailing lists