linux-kernel - Re: [PATCH net-next v4 1/4] net: mdio: common handling of phy reset properties

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251028182240.17da8177@kernel.org>
Date: Tue, 28 Oct 2025 18:22:40 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Buday Csaba <buday.csaba@...lan.hu>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
 Russell King <linux@...linux.org.uk>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni
 <pabeni@...hat.com>, Philipp Zabel <p.zabel@...gutronix.de>,
 <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next v4 1/4] net: mdio: common handling of phy reset
 properties

Let me give you some nit picky comments, maybe v5 will have more luck
attracting area experts :(

On Wed, 22 Oct 2025 11:08:50 +0200 Buday Csaba wrote:
> Reset properties of an `mdio_device` are initialized in multiple
> source files and multiple functions:
>   - `reset_assert_delay` and `reset_deassert_delay` are in
>     fwnode_mdio.c
>   - `reset_gpio` and `reset_ctrl` are in mdio_bus.c, but handled by
>     different functions
> 
> This patch unifies the handling of all these properties into two
> functions.

Use imperative mood. "This patch unifies" -> "Unify"
Please check all commits msgs for this (eg glancing at patch 2
"Changed" -> "Change")

> mdiobus_register_gpiod() and mdiobus_register_reset() are removed,
> while mdio_device_register_reset() and mdio_device_unregister_reset()
> are introduced instead.
> These functions handle both reset-controllers and reset-gpios, and
> also read the corresponding properties from the device tree.
> These changes should make tracking the reset properties easier.

> +	/* reset-gpio, bring up deasserted */
> +	mdiodev->reset_gpio = gpiod_get_optional(&mdiodev->dev, "reset",
> +						 GPIOD_OUT_LOW);
> +

no empty lines between call and its error check pls

> +	if (IS_ERR(mdiodev->reset_gpio))
> +		return PTR_ERR(mdiodev->reset_gpio);
> +
> +	if (mdiodev->reset_gpio)
> +		gpiod_set_consumer_name(mdiodev->reset_gpio, "PHY reset");
> +
> +	reset = reset_control_get_optional_exclusive(&mdiodev->dev, "phy");
> +	if (IS_ERR(reset))
> +		return PTR_ERR(reset);
> +
> +	mdiodev->reset_ctrl = reset;
> +
> +	/* Assert the reset signal */
> +	mdio_device_reset(mdiodev, 1);
> +
> +	return 0;
> +}