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

[Parvathi Pudi <parvathi@couthit.com>]

Hi Markus,
 
> 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?
> 

We have an ICSSM PRUETH Ethernet use case. The existing static MDIO pinmux
configuration has been hard-coded for CPSW. Based on the hardware jumper
setting we configure the MDIO either for CPSW or for the ICSSM PRUETH use case.

>>
>> 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.
> 

We will fix this in the next version.

>> +
>> +#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.
> 

Sure, we will clean it up in the next version.

>> +			/* 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.
> 

We will fix this in the next version.

>> +			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?
> 

enable_board_pin_mux() runs before the jumper state is known, so we can't
use it here. We'll reuse the existing macros from mux.h instead to avoid
the redefinition's.

>> +		}
>> +
> 
> 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?
> 

We have not changed the original pinmux setup.
Can you please elaborate bit more on this?

Thanks and Regards,
Parvathi