Re: [PATCH] board: ti: am335x: Conditional MDIO PAD configuration instead of static for AM335_ICE

["Markus Schneider-Pargmann" <msp@baylibre.com>]

[-- Attachment #1: Type: text/plain, Size: 3063 bytes --]

Hi Parvathi,

On Tue Apr 7, 2026 at 10:22 AM CEST, Parvathi Pudi wrote:
> This patch removes the static MDIO pinmux configuration from
> rmii1_pin_mux[] and instead configures the MDIO pins conditionally
> during board_init(). Previously, the MDIO_CLK and MDIO_DATA pins
> were always configured for CPSW in mux.c, which could lead to
> unnecessary pin ownership and conflicts in scenarios where CPSW
> is not used.
>
> With this change, the MDIO pins are configured only when required,
> ensuring that CPSW Ethernet functionality in U-Boot remains unaffected.
> This approach keeps Ethernet boot behavior intact and provides cleaner
> separation between CPSW and other Ethernet use cases.

Do you have a specific use case here?

>
> Signed-off-by: Parvathi Pudi <parvathi@couthit.com>
> ---
>  board/ti/am335x/board.c | 22 ++++++++++++++++++++++
>  board/ti/am335x/mux.c   |  2 --
>  2 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c
> index 90e37a8d913..abeab809387 100644
> --- a/board/ti/am335x/board.c
> +++ b/board/ti/am335x/board.c
> @@ -61,6 +61,18 @@ DECLARE_GLOBAL_DATA_PTR;
>  #define GPIO_ETH0_MODE		GPIO_TO_PIN(0, 11)
>  #define GPIO_ETH1_MODE		GPIO_TO_PIN(1, 26)
>  
> +#define AM335X_PIN_MDIO	0x948
> +#define AM335X_PIN_MDC		0x94c

These two are not aligned when applied.

> +
> +#define GPIO_MDIO_DATA		CTRL_BASE + AM335X_PIN_MDIO
> +#define GPIO_MDIO_CLK		CTRL_BASE + AM335X_PIN_MDC
> +
> +/* Enabling MDIO_DATA by setting MUX_MODE to 0, RXACTIVE, PULLUP_EN bits */
> +#define PAD_CONFIG_MDIO_DATA	0x30
> +
> +/* Enabling MDIO_CLK by setting MUX_MODE to 0, PULLUP_EN bit */
> +#define PAD_CONFIG_MDIO_CLK	0x10
> +
>  static struct ctrl_dev *cdev = (struct ctrl_dev *)CTRL_DEVICE_BASE;
>  
>  #define GPIO0_RISINGDETECT	(AM33XX_GPIO0_BASE + OMAP_GPIO_RISINGDETECT)
> @@ -779,6 +791,16 @@ int board_init(void)
>  			hang();
>  		}
>  
> +		if (!eth0_is_mii || !eth1_is_mii) {

eth0_is_mii and eth1_is_mii is the same, as it was checked in the
if condition above this one. You could make this an else if as well if
you like.

> +			/* Set the Mux Mode to MDIO_DATA */
> +			reg = readl(GPIO_MDIO_DATA);
> +			writel(reg & PAD_CONFIG_MDIO_DATA, GPIO_MDIO_DATA);
> +
> +			/* Set the Mux Mode to MDIO_CLK */

Please remove comments for code that shows the same thing.

> +			reg = readl(GPIO_MDIO_CLK);
> +			writel(reg & PAD_CONFIG_MDIO_CLK, GPIO_MDIO_CLK);

Could you do the same thing in enable_board_pin_mux()? Or would it be
possible to reuse mux.h here to avoid redefining values etc.?

Why do you use reg & PAD_CONFIG_MDIO_CLK instead of writing
PAD_CONFIG_MDIO_CLK directly and skip the read?

> +		}
> +

This snippet is in the section guarded by these:

  #if defined(CONFIG_CLOCK_SYNTHESIZER) && (!defined(CONFIG_XPL_BUILD) || \
    (defined(CONFIG_SPL_ETH) && defined(CONFIG_XPL_BUILD)))
    if (board_is_icev2()) {

Is this the same as for the original pinmux setup?

Best
Markus

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 289 bytes --]