[PATCH net v2] net: dpaa: fix mode setting

[PATCH net v2] net: dpaa: fix mode setting

[Michael Walle]

Before converting to the phylink interface, the init function would have
set the correct I/F mode depending on the maximum link speed of an
interface. After converting to phylink, the established link speed
is used to determine this setting and is set in the .link_up()
callback. The callback isn't called because the link is never
established between the PCS and a connected SGMII PHY.
To fix it, don't use the current speed, but set the mode depending on
the interface (which implies the maximum speed) in .mac_config().

Fixes: 5d93cfcf7360 ("net: dpaa: Convert to phylink")
Suggested-by: Sean Anderson <sean.anderson@linux.dev>
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
FWIW, I dropped setting a non-reserved mode in init(). The hardware
default is 0 and the mac_config() will set a valid mode anyway.

Changes in v2:
 - the setting is/was based on the maximum speed, not the current
   speed. thus, move the setting into mac_config().
 - Link to v1: https://lore.kernel.org/r/20260706121011.1948906-1-mwalle@kernel.org/

 .../net/ethernet/freescale/fman/fman_dtsec.c  | 26 ++++++++++---------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fman/fman_dtsec.c b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
index fe35703c509e..7075f93bab49 100644
--- a/drivers/net/ethernet/freescale/fman/fman_dtsec.c
+++ b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
@@ -900,22 +900,28 @@ static void dtsec_mac_config(struct phylink_config *config, unsigned int mode,
 {
 	struct mac_device *mac_dev = fman_config_to_mac(config);
 	struct dtsec_regs __iomem *regs = mac_dev->fman_mac->regs;
-	u32 tmp;
+	u32 ecntrl, maccfg2;
+
+	maccfg2 = ioread32be(&regs->maccfg2);
+	maccfg2 &= ~(MACCFG2_NIBBLE_MODE | MACCFG2_BYTE_MODE);
 
 	switch (state->interface) {
 	case PHY_INTERFACE_MODE_RMII:
-		tmp = DTSEC_ECNTRL_RMM;
+		ecntrl = DTSEC_ECNTRL_RMM;
+		maccfg2 |= MACCFG2_NIBBLE_MODE;
 		break;
 	case PHY_INTERFACE_MODE_RGMII:
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		tmp = DTSEC_ECNTRL_GMIIM | DTSEC_ECNTRL_RPM;
+		ecntrl = DTSEC_ECNTRL_GMIIM | DTSEC_ECNTRL_RPM;
+		maccfg2 |= MACCFG2_BYTE_MODE;
 		break;
 	case PHY_INTERFACE_MODE_SGMII:
 	case PHY_INTERFACE_MODE_1000BASEX:
 	case PHY_INTERFACE_MODE_2500BASEX:
-		tmp = DTSEC_ECNTRL_TBIM | DTSEC_ECNTRL_SGMIIM;
+		ecntrl = DTSEC_ECNTRL_TBIM | DTSEC_ECNTRL_SGMIIM;
+		maccfg2 |= MACCFG2_BYTE_MODE;
 		break;
 	default:
 		dev_warn(mac_dev->dev, "cannot configure dTSEC for %s\n",
@@ -923,7 +929,8 @@ static void dtsec_mac_config(struct phylink_config *config, unsigned int mode,
 		return;
 	}
 
-	iowrite32be(tmp, &regs->ecntrl);
+	iowrite32be(ecntrl, &regs->ecntrl);
+	iowrite32be(maccfg2, &regs->maccfg2);
 }
 
 static void dtsec_link_up(struct phylink_config *config, struct phy_device *phy,
@@ -948,15 +955,10 @@ static void dtsec_link_up(struct phylink_config *config, struct phy_device *phy,
 	iowrite32be(tmp, &regs->ecntrl);
 
 	tmp = ioread32be(&regs->maccfg2);
-	tmp &= ~(MACCFG2_NIBBLE_MODE | MACCFG2_BYTE_MODE | MACCFG2_FULL_DUPLEX);
-	if (speed >= SPEED_1000)
-		tmp |= MACCFG2_BYTE_MODE;
-	else
-		tmp |= MACCFG2_NIBBLE_MODE;
-
 	if (duplex == DUPLEX_FULL)
 		tmp |= MACCFG2_FULL_DUPLEX;
-
+	else
+		tmp &= ~MACCFG2_FULL_DUPLEX;
 	iowrite32be(tmp, &regs->maccfg2);
 
 	mac_dev->update_speed(mac_dev, speed);
-- 
2.47.3

Re: [PATCH net v2] net: dpaa: fix mode setting

[Sean Anderson]

On 7/10/26 10:22, Michael Walle wrote:
> Before converting to the phylink interface, the init function would have
> set the correct I/F mode depending on the maximum link speed of an
> interface. After converting to phylink, the established link speed
> is used to determine this setting and is set in the .link_up()
> callback. The callback isn't called because the link is never
> established between the PCS and a connected SGMII PHY.
> To fix it, don't use the current speed, but set the mode depending on
> the interface (which implies the maximum speed) in .mac_config().
> 
> Fixes: 5d93cfcf7360 ("net: dpaa: Convert to phylink")
> Suggested-by: Sean Anderson <sean.anderson@linux.dev>
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> ---
> FWIW, I dropped setting a non-reserved mode in init(). The hardware
> default is 0 and the mac_config() will set a valid mode anyway.
> 
> Changes in v2:
>   - the setting is/was based on the maximum speed, not the current
>     speed. thus, move the setting into mac_config().
>   - Link to v1: https://lore.kernel.org/r/20260706121011.1948906-1-mwalle@kernel.org/
> 
>   .../net/ethernet/freescale/fman/fman_dtsec.c  | 26 ++++++++++---------
>   1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fman/fman_dtsec.c b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
> index fe35703c509e..7075f93bab49 100644
> --- a/drivers/net/ethernet/freescale/fman/fman_dtsec.c
> +++ b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
> @@ -900,22 +900,28 @@ static void dtsec_mac_config(struct phylink_config *config, unsigned int mode,
>   {
>   	struct mac_device *mac_dev = fman_config_to_mac(config);
>   	struct dtsec_regs __iomem *regs = mac_dev->fman_mac->regs;
> -	u32 tmp;
> +	u32 ecntrl, maccfg2;
> +
> +	maccfg2 = ioread32be(&regs->maccfg2);
> +	maccfg2 &= ~(MACCFG2_NIBBLE_MODE | MACCFG2_BYTE_MODE);
>   
>   	switch (state->interface) {
>   	case PHY_INTERFACE_MODE_RMII:
> -		tmp = DTSEC_ECNTRL_RMM;
> +		ecntrl = DTSEC_ECNTRL_RMM;
> +		maccfg2 |= MACCFG2_NIBBLE_MODE;
>   		break;
>   	case PHY_INTERFACE_MODE_RGMII:
>   	case PHY_INTERFACE_MODE_RGMII_ID:
>   	case PHY_INTERFACE_MODE_RGMII_RXID:
>   	case PHY_INTERFACE_MODE_RGMII_TXID:
> -		tmp = DTSEC_ECNTRL_GMIIM | DTSEC_ECNTRL_RPM;
> +		ecntrl = DTSEC_ECNTRL_GMIIM | DTSEC_ECNTRL_RPM;
> +		maccfg2 |= MACCFG2_BYTE_MODE;
>   		break;
>   	case PHY_INTERFACE_MODE_SGMII:
>   	case PHY_INTERFACE_MODE_1000BASEX:
>   	case PHY_INTERFACE_MODE_2500BASEX:
> -		tmp = DTSEC_ECNTRL_TBIM | DTSEC_ECNTRL_SGMIIM;
> +		ecntrl = DTSEC_ECNTRL_TBIM | DTSEC_ECNTRL_SGMIIM;
> +		maccfg2 |= MACCFG2_BYTE_MODE;
>   		break;
>   	default:
>   		dev_warn(mac_dev->dev, "cannot configure dTSEC for %s\n",
> @@ -923,7 +929,8 @@ static void dtsec_mac_config(struct phylink_config *config, unsigned int mode,
>   		return;
>   	}
>   
> -	iowrite32be(tmp, &regs->ecntrl);
> +	iowrite32be(ecntrl, &regs->ecntrl);
> +	iowrite32be(maccfg2, &regs->maccfg2);
>   }
>   
>   static void dtsec_link_up(struct phylink_config *config, struct phy_device *phy,
> @@ -948,15 +955,10 @@ static void dtsec_link_up(struct phylink_config *config, struct phy_device *phy,
>   	iowrite32be(tmp, &regs->ecntrl);
>   
>   	tmp = ioread32be(&regs->maccfg2);
> -	tmp &= ~(MACCFG2_NIBBLE_MODE | MACCFG2_BYTE_MODE | MACCFG2_FULL_DUPLEX);
> -	if (speed >= SPEED_1000)
> -		tmp |= MACCFG2_BYTE_MODE;
> -	else
> -		tmp |= MACCFG2_NIBBLE_MODE;
> -
>   	if (duplex == DUPLEX_FULL)
>   		tmp |= MACCFG2_FULL_DUPLEX;
> -
> +	else
> +		tmp &= ~MACCFG2_FULL_DUPLEX;

Did you test this when forcing 10/100 speed?

>   	iowrite32be(tmp, &regs->maccfg2);
>   
>   	mac_dev->update_speed(mac_dev, speed);

If so,

Reviewed-by: Sean Anderson <sean.anderson@linux.dev>

Re: [PATCH net v2] net: dpaa: fix mode setting

[Michael Walle]

On Fri Jul 10, 2026 at 4:39 PM CEST, Sean Anderson wrote:
> On 7/10/26 10:22, Michael Walle wrote:
>> Before converting to the phylink interface, the init function would have
>> set the correct I/F mode depending on the maximum link speed of an
>> interface. After converting to phylink, the established link speed
>> is used to determine this setting and is set in the .link_up()
>> callback. The callback isn't called because the link is never
>> established between the PCS and a connected SGMII PHY.
>> To fix it, don't use the current speed, but set the mode depending on
>> the interface (which implies the maximum speed) in .mac_config().
>> 
>> Fixes: 5d93cfcf7360 ("net: dpaa: Convert to phylink")
>> Suggested-by: Sean Anderson <sean.anderson@linux.dev>
>> Signed-off-by: Michael Walle <mwalle@kernel.org>
>> ---
>> FWIW, I dropped setting a non-reserved mode in init(). The hardware
>> default is 0 and the mac_config() will set a valid mode anyway.
>> 
>> Changes in v2:
>>   - the setting is/was based on the maximum speed, not the current
>>     speed. thus, move the setting into mac_config().
>>   - Link to v1: https://lore.kernel.org/r/20260706121011.1948906-1-mwalle@kernel.org/
>> 
>>   .../net/ethernet/freescale/fman/fman_dtsec.c  | 26 ++++++++++---------
>>   1 file changed, 14 insertions(+), 12 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/freescale/fman/fman_dtsec.c b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
>> index fe35703c509e..7075f93bab49 100644
>> --- a/drivers/net/ethernet/freescale/fman/fman_dtsec.c
>> +++ b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
>> @@ -900,22 +900,28 @@ static void dtsec_mac_config(struct phylink_config *config, unsigned int mode,
>>   {
>>   	struct mac_device *mac_dev = fman_config_to_mac(config);
>>   	struct dtsec_regs __iomem *regs = mac_dev->fman_mac->regs;
>> -	u32 tmp;
>> +	u32 ecntrl, maccfg2;
>> +
>> +	maccfg2 = ioread32be(&regs->maccfg2);
>> +	maccfg2 &= ~(MACCFG2_NIBBLE_MODE | MACCFG2_BYTE_MODE);
>>   
>>   	switch (state->interface) {
>>   	case PHY_INTERFACE_MODE_RMII:
>> -		tmp = DTSEC_ECNTRL_RMM;
>> +		ecntrl = DTSEC_ECNTRL_RMM;
>> +		maccfg2 |= MACCFG2_NIBBLE_MODE;
>>   		break;
>>   	case PHY_INTERFACE_MODE_RGMII:
>>   	case PHY_INTERFACE_MODE_RGMII_ID:
>>   	case PHY_INTERFACE_MODE_RGMII_RXID:
>>   	case PHY_INTERFACE_MODE_RGMII_TXID:
>> -		tmp = DTSEC_ECNTRL_GMIIM | DTSEC_ECNTRL_RPM;
>> +		ecntrl = DTSEC_ECNTRL_GMIIM | DTSEC_ECNTRL_RPM;
>> +		maccfg2 |= MACCFG2_BYTE_MODE;
>>   		break;
>>   	case PHY_INTERFACE_MODE_SGMII:
>>   	case PHY_INTERFACE_MODE_1000BASEX:
>>   	case PHY_INTERFACE_MODE_2500BASEX:
>> -		tmp = DTSEC_ECNTRL_TBIM | DTSEC_ECNTRL_SGMIIM;
>> +		ecntrl = DTSEC_ECNTRL_TBIM | DTSEC_ECNTRL_SGMIIM;
>> +		maccfg2 |= MACCFG2_BYTE_MODE;
>>   		break;
>>   	default:
>>   		dev_warn(mac_dev->dev, "cannot configure dTSEC for %s\n",
>> @@ -923,7 +929,8 @@ static void dtsec_mac_config(struct phylink_config *config, unsigned int mode,
>>   		return;
>>   	}
>>   
>> -	iowrite32be(tmp, &regs->ecntrl);
>> +	iowrite32be(ecntrl, &regs->ecntrl);
>> +	iowrite32be(maccfg2, &regs->maccfg2);
>>   }
>>   
>>   static void dtsec_link_up(struct phylink_config *config, struct phy_device *phy,
>> @@ -948,15 +955,10 @@ static void dtsec_link_up(struct phylink_config *config, struct phy_device *phy,
>>   	iowrite32be(tmp, &regs->ecntrl);
>>   
>>   	tmp = ioread32be(&regs->maccfg2);
>> -	tmp &= ~(MACCFG2_NIBBLE_MODE | MACCFG2_BYTE_MODE | MACCFG2_FULL_DUPLEX);
>> -	if (speed >= SPEED_1000)
>> -		tmp |= MACCFG2_BYTE_MODE;
>> -	else
>> -		tmp |= MACCFG2_NIBBLE_MODE;
>> -
>>   	if (duplex == DUPLEX_FULL)
>>   		tmp |= MACCFG2_FULL_DUPLEX;
>> -
>> +	else
>> +		tmp &= ~MACCFG2_FULL_DUPLEX;
>
> Did you test this when forcing 10/100 speed?

No I didn't. Well I can't. I have a very weird board which only
supports 1000base-X (and copper SFPs in 1000basex autoneg mode). On
top of that there is a Marvell 88E1112 in between the SFP and the
MAC, for which the PHY driver is completely broken. Long story
short, I'm not able to test that (yet/at all? Not sure).

-michael

>
>>   	iowrite32be(tmp, &regs->maccfg2);
>>   
>>   	mac_dev->update_speed(mac_dev, speed);
>
> If so,
>
> Reviewed-by: Sean Anderson <sean.anderson@linux.dev>

Re: [PATCH net v2] net: dpaa: fix mode setting

[Paolo Abeni]

On 7/10/26 4:47 PM, Michael Walle wrote:
> On Fri Jul 10, 2026 at 4:39 PM CEST, Sean Anderson wrote:
>> On 7/10/26 10:22, Michael Walle wrote:
>>> Before converting to the phylink interface, the init function would have
>>> set the correct I/F mode depending on the maximum link speed of an
>>> interface. After converting to phylink, the established link speed
>>> is used to determine this setting and is set in the .link_up()
>>> callback. The callback isn't called because the link is never
>>> established between the PCS and a connected SGMII PHY.
>>> To fix it, don't use the current speed, but set the mode depending on
>>> the interface (which implies the maximum speed) in .mac_config().
>>>
>>> Fixes: 5d93cfcf7360 ("net: dpaa: Convert to phylink")
>>> Suggested-by: Sean Anderson <sean.anderson@linux.dev>
>>> Signed-off-by: Michael Walle <mwalle@kernel.org>
>>> ---
>>> FWIW, I dropped setting a non-reserved mode in init(). The hardware
>>> default is 0 and the mac_config() will set a valid mode anyway.
>>>
>>> Changes in v2:
>>>   - the setting is/was based on the maximum speed, not the current
>>>     speed. thus, move the setting into mac_config().
>>>   - Link to v1: https://lore.kernel.org/r/20260706121011.1948906-1-mwalle@kernel.org/
>>>
>>>   .../net/ethernet/freescale/fman/fman_dtsec.c  | 26 ++++++++++---------
>>>   1 file changed, 14 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/freescale/fman/fman_dtsec.c b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
>>> index fe35703c509e..7075f93bab49 100644
>>> --- a/drivers/net/ethernet/freescale/fman/fman_dtsec.c
>>> +++ b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
>>> @@ -900,22 +900,28 @@ static void dtsec_mac_config(struct phylink_config *config, unsigned int mode,
>>>   {
>>>   	struct mac_device *mac_dev = fman_config_to_mac(config);
>>>   	struct dtsec_regs __iomem *regs = mac_dev->fman_mac->regs;
>>> -	u32 tmp;
>>> +	u32 ecntrl, maccfg2;
>>> +
>>> +	maccfg2 = ioread32be(&regs->maccfg2);
>>> +	maccfg2 &= ~(MACCFG2_NIBBLE_MODE | MACCFG2_BYTE_MODE);
>>>   
>>>   	switch (state->interface) {
>>>   	case PHY_INTERFACE_MODE_RMII:
>>> -		tmp = DTSEC_ECNTRL_RMM;
>>> +		ecntrl = DTSEC_ECNTRL_RMM;
>>> +		maccfg2 |= MACCFG2_NIBBLE_MODE;
>>>   		break;
>>>   	case PHY_INTERFACE_MODE_RGMII:
>>>   	case PHY_INTERFACE_MODE_RGMII_ID:
>>>   	case PHY_INTERFACE_MODE_RGMII_RXID:
>>>   	case PHY_INTERFACE_MODE_RGMII_TXID:
>>> -		tmp = DTSEC_ECNTRL_GMIIM | DTSEC_ECNTRL_RPM;
>>> +		ecntrl = DTSEC_ECNTRL_GMIIM | DTSEC_ECNTRL_RPM;
>>> +		maccfg2 |= MACCFG2_BYTE_MODE;
>>>   		break;
>>>   	case PHY_INTERFACE_MODE_SGMII:
>>>   	case PHY_INTERFACE_MODE_1000BASEX:
>>>   	case PHY_INTERFACE_MODE_2500BASEX:
>>> -		tmp = DTSEC_ECNTRL_TBIM | DTSEC_ECNTRL_SGMIIM;
>>> +		ecntrl = DTSEC_ECNTRL_TBIM | DTSEC_ECNTRL_SGMIIM;
>>> +		maccfg2 |= MACCFG2_BYTE_MODE;
>>>   		break;
>>>   	default:
>>>   		dev_warn(mac_dev->dev, "cannot configure dTSEC for %s\n",
>>> @@ -923,7 +929,8 @@ static void dtsec_mac_config(struct phylink_config *config, unsigned int mode,
>>>   		return;
>>>   	}
>>>   
>>> -	iowrite32be(tmp, &regs->ecntrl);
>>> +	iowrite32be(ecntrl, &regs->ecntrl);
>>> +	iowrite32be(maccfg2, &regs->maccfg2);
>>>   }
>>>   
>>>   static void dtsec_link_up(struct phylink_config *config, struct phy_device *phy,
>>> @@ -948,15 +955,10 @@ static void dtsec_link_up(struct phylink_config *config, struct phy_device *phy,
>>>   	iowrite32be(tmp, &regs->ecntrl);
>>>   
>>>   	tmp = ioread32be(&regs->maccfg2);
>>> -	tmp &= ~(MACCFG2_NIBBLE_MODE | MACCFG2_BYTE_MODE | MACCFG2_FULL_DUPLEX);
>>> -	if (speed >= SPEED_1000)
>>> -		tmp |= MACCFG2_BYTE_MODE;
>>> -	else
>>> -		tmp |= MACCFG2_NIBBLE_MODE;
>>> -
>>>   	if (duplex == DUPLEX_FULL)
>>>   		tmp |= MACCFG2_FULL_DUPLEX;
>>> -
>>> +	else
>>> +		tmp &= ~MACCFG2_FULL_DUPLEX;
>>
>> Did you test this when forcing 10/100 speed?
> 
> No I didn't. Well I can't. I have a very weird board which only
> supports 1000base-X (and copper SFPs in 1000basex autoneg mode). On
> top of that there is a Marvell 88E1112 in between the SFP and the
> MAC, for which the PHY driver is completely broken. Long story
> short, I'm not able to test that (yet/at all? Not sure).
FTR, sashiko suspect this patch will broke such setup:

https://sashiko.dev/#/patchset/20260710143430.2276141-1-mwalle%40kernel.org

I think we need either an explicit test from a 3rd party or a new revision.

/P

Re: [PATCH net v2] net: dpaa: fix mode setting

[Michael Walle]

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

On Fri Jul 17, 2026 at 12:01 PM CEST, Paolo Abeni wrote:
> On 7/10/26 4:47 PM, Michael Walle wrote:
>> On Fri Jul 10, 2026 at 4:39 PM CEST, Sean Anderson wrote:
>>> On 7/10/26 10:22, Michael Walle wrote:
>>>> Before converting to the phylink interface, the init function would have
>>>> set the correct I/F mode depending on the maximum link speed of an
>>>> interface. After converting to phylink, the established link speed
>>>> is used to determine this setting and is set in the .link_up()
>>>> callback. The callback isn't called because the link is never
>>>> established between the PCS and a connected SGMII PHY.
>>>> To fix it, don't use the current speed, but set the mode depending on
>>>> the interface (which implies the maximum speed) in .mac_config().
>>>>
>>>> Fixes: 5d93cfcf7360 ("net: dpaa: Convert to phylink")
>>>> Suggested-by: Sean Anderson <sean.anderson@linux.dev>
>>>> Signed-off-by: Michael Walle <mwalle@kernel.org>
>>>> ---
>>>> FWIW, I dropped setting a non-reserved mode in init(). The hardware
>>>> default is 0 and the mac_config() will set a valid mode anyway.
>>>>
>>>> Changes in v2:
>>>>   - the setting is/was based on the maximum speed, not the current
>>>>     speed. thus, move the setting into mac_config().
>>>>   - Link to v1: https://lore.kernel.org/r/20260706121011.1948906-1-mwalle@kernel.org/
>>>>
>>>>   .../net/ethernet/freescale/fman/fman_dtsec.c  | 26 ++++++++++---------
>>>>   1 file changed, 14 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/freescale/fman/fman_dtsec.c b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
>>>> index fe35703c509e..7075f93bab49 100644
>>>> --- a/drivers/net/ethernet/freescale/fman/fman_dtsec.c
>>>> +++ b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
>>>> @@ -900,22 +900,28 @@ static void dtsec_mac_config(struct phylink_config *config, unsigned int mode,
>>>>   {
>>>>   	struct mac_device *mac_dev = fman_config_to_mac(config);
>>>>   	struct dtsec_regs __iomem *regs = mac_dev->fman_mac->regs;
>>>> -	u32 tmp;
>>>> +	u32 ecntrl, maccfg2;
>>>> +
>>>> +	maccfg2 = ioread32be(&regs->maccfg2);
>>>> +	maccfg2 &= ~(MACCFG2_NIBBLE_MODE | MACCFG2_BYTE_MODE);
>>>>   
>>>>   	switch (state->interface) {
>>>>   	case PHY_INTERFACE_MODE_RMII:
>>>> -		tmp = DTSEC_ECNTRL_RMM;
>>>> +		ecntrl = DTSEC_ECNTRL_RMM;
>>>> +		maccfg2 |= MACCFG2_NIBBLE_MODE;
>>>>   		break;
>>>>   	case PHY_INTERFACE_MODE_RGMII:
>>>>   	case PHY_INTERFACE_MODE_RGMII_ID:
>>>>   	case PHY_INTERFACE_MODE_RGMII_RXID:
>>>>   	case PHY_INTERFACE_MODE_RGMII_TXID:
>>>> -		tmp = DTSEC_ECNTRL_GMIIM | DTSEC_ECNTRL_RPM;
>>>> +		ecntrl = DTSEC_ECNTRL_GMIIM | DTSEC_ECNTRL_RPM;
>>>> +		maccfg2 |= MACCFG2_BYTE_MODE;
>>>>   		break;
>>>>   	case PHY_INTERFACE_MODE_SGMII:
>>>>   	case PHY_INTERFACE_MODE_1000BASEX:
>>>>   	case PHY_INTERFACE_MODE_2500BASEX:
>>>> -		tmp = DTSEC_ECNTRL_TBIM | DTSEC_ECNTRL_SGMIIM;
>>>> +		ecntrl = DTSEC_ECNTRL_TBIM | DTSEC_ECNTRL_SGMIIM;
>>>> +		maccfg2 |= MACCFG2_BYTE_MODE;
>>>>   		break;
>>>>   	default:
>>>>   		dev_warn(mac_dev->dev, "cannot configure dTSEC for %s\n",
>>>> @@ -923,7 +929,8 @@ static void dtsec_mac_config(struct phylink_config *config, unsigned int mode,
>>>>   		return;
>>>>   	}
>>>>   
>>>> -	iowrite32be(tmp, &regs->ecntrl);
>>>> +	iowrite32be(ecntrl, &regs->ecntrl);
>>>> +	iowrite32be(maccfg2, &regs->maccfg2);
>>>>   }
>>>>   
>>>>   static void dtsec_link_up(struct phylink_config *config, struct phy_device *phy,
>>>> @@ -948,15 +955,10 @@ static void dtsec_link_up(struct phylink_config *config, struct phy_device *phy,
>>>>   	iowrite32be(tmp, &regs->ecntrl);
>>>>   
>>>>   	tmp = ioread32be(&regs->maccfg2);
>>>> -	tmp &= ~(MACCFG2_NIBBLE_MODE | MACCFG2_BYTE_MODE | MACCFG2_FULL_DUPLEX);
>>>> -	if (speed >= SPEED_1000)
>>>> -		tmp |= MACCFG2_BYTE_MODE;
>>>> -	else
>>>> -		tmp |= MACCFG2_NIBBLE_MODE;
>>>> -
>>>>   	if (duplex == DUPLEX_FULL)
>>>>   		tmp |= MACCFG2_FULL_DUPLEX;
>>>> -
>>>> +	else
>>>> +		tmp &= ~MACCFG2_FULL_DUPLEX;
>>>
>>> Did you test this when forcing 10/100 speed?
>> 
>> No I didn't. Well I can't. I have a very weird board which only
>> supports 1000base-X (and copper SFPs in 1000basex autoneg mode). On
>> top of that there is a Marvell 88E1112 in between the SFP and the
>> MAC, for which the PHY driver is completely broken. Long story
>> short, I'm not able to test that (yet/at all? Not sure).
> FTR, sashiko suspect this patch will broke such setup:
> https://sashiko.dev/#/patchset/20260710143430.2276141-1-mwalle%40kernel.org

I've seen that, but.. that was the actual change between v1 and v2
as suggested by Sean. It does not depend on the actual link speed,
but the maximum link speed. So it is not relevant if the link
negotiates to a slower speed or not. At least that now matches the
behavior prior to the phylink conversion. If that was working -
that I can't tell you.

> I think we need either an explicit test from a 3rd party or a new revision.
I'm not sure what you mean with a v3. Going back to v1? 

-michael

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

Re: [PATCH net v2] net: dpaa: fix mode setting

[Paolo Abeni]

On 7/17/26 12:13 PM, Michael Walle wrote:
> On Fri Jul 17, 2026 at 12:01 PM CEST, Paolo Abeni wrote:
>> On 7/10/26 4:47 PM, Michael Walle wrote:
>>> On Fri Jul 10, 2026 at 4:39 PM CEST, Sean Anderson wrote:
>>>> On 7/10/26 10:22, Michael Walle wrote:
>>>>> Before converting to the phylink interface, the init function would have
>>>>> set the correct I/F mode depending on the maximum link speed of an
>>>>> interface. After converting to phylink, the established link speed
>>>>> is used to determine this setting and is set in the .link_up()
>>>>> callback. The callback isn't called because the link is never
>>>>> established between the PCS and a connected SGMII PHY.
>>>>> To fix it, don't use the current speed, but set the mode depending on
>>>>> the interface (which implies the maximum speed) in .mac_config().
>>>>>
>>>>> Fixes: 5d93cfcf7360 ("net: dpaa: Convert to phylink")
>>>>> Suggested-by: Sean Anderson <sean.anderson@linux.dev>
>>>>> Signed-off-by: Michael Walle <mwalle@kernel.org>
>>>>> ---
>>>>> FWIW, I dropped setting a non-reserved mode in init(). The hardware
>>>>> default is 0 and the mac_config() will set a valid mode anyway.
>>>>>
>>>>> Changes in v2:
>>>>>   - the setting is/was based on the maximum speed, not the current
>>>>>     speed. thus, move the setting into mac_config().
>>>>>   - Link to v1: https://lore.kernel.org/r/20260706121011.1948906-1-mwalle@kernel.org/
>>>>>
>>>>>   .../net/ethernet/freescale/fman/fman_dtsec.c  | 26 ++++++++++---------
>>>>>   1 file changed, 14 insertions(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/freescale/fman/fman_dtsec.c b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
>>>>> index fe35703c509e..7075f93bab49 100644
>>>>> --- a/drivers/net/ethernet/freescale/fman/fman_dtsec.c
>>>>> +++ b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
>>>>> @@ -900,22 +900,28 @@ static void dtsec_mac_config(struct phylink_config *config, unsigned int mode,
>>>>>   {
>>>>>   	struct mac_device *mac_dev = fman_config_to_mac(config);
>>>>>   	struct dtsec_regs __iomem *regs = mac_dev->fman_mac->regs;
>>>>> -	u32 tmp;
>>>>> +	u32 ecntrl, maccfg2;
>>>>> +
>>>>> +	maccfg2 = ioread32be(&regs->maccfg2);
>>>>> +	maccfg2 &= ~(MACCFG2_NIBBLE_MODE | MACCFG2_BYTE_MODE);
>>>>>   
>>>>>   	switch (state->interface) {
>>>>>   	case PHY_INTERFACE_MODE_RMII:
>>>>> -		tmp = DTSEC_ECNTRL_RMM;
>>>>> +		ecntrl = DTSEC_ECNTRL_RMM;
>>>>> +		maccfg2 |= MACCFG2_NIBBLE_MODE;
>>>>>   		break;
>>>>>   	case PHY_INTERFACE_MODE_RGMII:
>>>>>   	case PHY_INTERFACE_MODE_RGMII_ID:
>>>>>   	case PHY_INTERFACE_MODE_RGMII_RXID:
>>>>>   	case PHY_INTERFACE_MODE_RGMII_TXID:
>>>>> -		tmp = DTSEC_ECNTRL_GMIIM | DTSEC_ECNTRL_RPM;
>>>>> +		ecntrl = DTSEC_ECNTRL_GMIIM | DTSEC_ECNTRL_RPM;
>>>>> +		maccfg2 |= MACCFG2_BYTE_MODE;
>>>>>   		break;
>>>>>   	case PHY_INTERFACE_MODE_SGMII:
>>>>>   	case PHY_INTERFACE_MODE_1000BASEX:
>>>>>   	case PHY_INTERFACE_MODE_2500BASEX:
>>>>> -		tmp = DTSEC_ECNTRL_TBIM | DTSEC_ECNTRL_SGMIIM;
>>>>> +		ecntrl = DTSEC_ECNTRL_TBIM | DTSEC_ECNTRL_SGMIIM;
>>>>> +		maccfg2 |= MACCFG2_BYTE_MODE;
>>>>>   		break;
>>>>>   	default:
>>>>>   		dev_warn(mac_dev->dev, "cannot configure dTSEC for %s\n",
>>>>> @@ -923,7 +929,8 @@ static void dtsec_mac_config(struct phylink_config *config, unsigned int mode,
>>>>>   		return;
>>>>>   	}
>>>>>   
>>>>> -	iowrite32be(tmp, &regs->ecntrl);
>>>>> +	iowrite32be(ecntrl, &regs->ecntrl);
>>>>> +	iowrite32be(maccfg2, &regs->maccfg2);
>>>>>   }
>>>>>   
>>>>>   static void dtsec_link_up(struct phylink_config *config, struct phy_device *phy,
>>>>> @@ -948,15 +955,10 @@ static void dtsec_link_up(struct phylink_config *config, struct phy_device *phy,
>>>>>   	iowrite32be(tmp, &regs->ecntrl);
>>>>>   
>>>>>   	tmp = ioread32be(&regs->maccfg2);
>>>>> -	tmp &= ~(MACCFG2_NIBBLE_MODE | MACCFG2_BYTE_MODE | MACCFG2_FULL_DUPLEX);
>>>>> -	if (speed >= SPEED_1000)
>>>>> -		tmp |= MACCFG2_BYTE_MODE;
>>>>> -	else
>>>>> -		tmp |= MACCFG2_NIBBLE_MODE;
>>>>> -
>>>>>   	if (duplex == DUPLEX_FULL)
>>>>>   		tmp |= MACCFG2_FULL_DUPLEX;
>>>>> -
>>>>> +	else
>>>>> +		tmp &= ~MACCFG2_FULL_DUPLEX;
>>>>
>>>> Did you test this when forcing 10/100 speed?
>>>
>>> No I didn't. Well I can't. I have a very weird board which only
>>> supports 1000base-X (and copper SFPs in 1000basex autoneg mode). On
>>> top of that there is a Marvell 88E1112 in between the SFP and the
>>> MAC, for which the PHY driver is completely broken. Long story
>>> short, I'm not able to test that (yet/at all? Not sure).
>> FTR, sashiko suspect this patch will broke such setup:
>> https://sashiko.dev/#/patchset/20260710143430.2276141-1-mwalle%40kernel.org
> 
> I've seen that, but.. that was the actual change between v1 and v2
> as suggested by Sean. It does not depend on the actual link speed,
> but the maximum link speed. So it is not relevant if the link
> negotiates to a slower speed or not. At least that now matches the
> behavior prior to the phylink conversion. If that was working -
> that I can't tell you.

I'm sorry, following all cross revision discussion is a bit hard here.

I don't understand if the 'link never established' is specific of your
board, or it a constant with this driver. Could you please clarify?

/P

Re: [PATCH net v2] net: dpaa: fix mode setting

[Sean Anderson]

On 7/17/26 07:11, Paolo Abeni wrote:
> On 7/17/26 12:13 PM, Michael Walle wrote:
>> On Fri Jul 17, 2026 at 12:01 PM CEST, Paolo Abeni wrote:
>>> On 7/10/26 4:47 PM, Michael Walle wrote:
>>>> On Fri Jul 10, 2026 at 4:39 PM CEST, Sean Anderson wrote:
>>>>> On 7/10/26 10:22, Michael Walle wrote:
>>>>>> Before converting to the phylink interface, the init function would have
>>>>>> set the correct I/F mode depending on the maximum link speed of an
>>>>>> interface. After converting to phylink, the established link speed
>>>>>> is used to determine this setting and is set in the .link_up()
>>>>>> callback. The callback isn't called because the link is never
>>>>>> established between the PCS and a connected SGMII PHY.
>>>>>> To fix it, don't use the current speed, but set the mode depending on
>>>>>> the interface (which implies the maximum speed) in .mac_config().
>>>>>>
>>>>>> Fixes: 5d93cfcf7360 ("net: dpaa: Convert to phylink")
>>>>>> Suggested-by: Sean Anderson <sean.anderson@linux.dev>
>>>>>> Signed-off-by: Michael Walle <mwalle@kernel.org>
>>>>>> ---
>>>>>> FWIW, I dropped setting a non-reserved mode in init(). The hardware
>>>>>> default is 0 and the mac_config() will set a valid mode anyway.
>>>>>>
>>>>>> Changes in v2:
>>>>>>    - the setting is/was based on the maximum speed, not the current
>>>>>>      speed. thus, move the setting into mac_config().
>>>>>>    - Link to v1: https://lore.kernel.org/r/20260706121011.1948906-1-mwalle@kernel.org/
>>>>>>
>>>>>>    .../net/ethernet/freescale/fman/fman_dtsec.c  | 26 ++++++++++---------
>>>>>>    1 file changed, 14 insertions(+), 12 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/ethernet/freescale/fman/fman_dtsec.c b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
>>>>>> index fe35703c509e..7075f93bab49 100644
>>>>>> --- a/drivers/net/ethernet/freescale/fman/fman_dtsec.c
>>>>>> +++ b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
>>>>>> @@ -900,22 +900,28 @@ static void dtsec_mac_config(struct phylink_config *config, unsigned int mode,
>>>>>>    {
>>>>>>    	struct mac_device *mac_dev = fman_config_to_mac(config);
>>>>>>    	struct dtsec_regs __iomem *regs = mac_dev->fman_mac->regs;
>>>>>> -	u32 tmp;
>>>>>> +	u32 ecntrl, maccfg2;
>>>>>> +
>>>>>> +	maccfg2 = ioread32be(&regs->maccfg2);
>>>>>> +	maccfg2 &= ~(MACCFG2_NIBBLE_MODE | MACCFG2_BYTE_MODE);
>>>>>>    
>>>>>>    	switch (state->interface) {
>>>>>>    	case PHY_INTERFACE_MODE_RMII:
>>>>>> -		tmp = DTSEC_ECNTRL_RMM;
>>>>>> +		ecntrl = DTSEC_ECNTRL_RMM;
>>>>>> +		maccfg2 |= MACCFG2_NIBBLE_MODE;
>>>>>>    		break;
>>>>>>    	case PHY_INTERFACE_MODE_RGMII:
>>>>>>    	case PHY_INTERFACE_MODE_RGMII_ID:
>>>>>>    	case PHY_INTERFACE_MODE_RGMII_RXID:
>>>>>>    	case PHY_INTERFACE_MODE_RGMII_TXID:
>>>>>> -		tmp = DTSEC_ECNTRL_GMIIM | DTSEC_ECNTRL_RPM;
>>>>>> +		ecntrl = DTSEC_ECNTRL_GMIIM | DTSEC_ECNTRL_RPM;
>>>>>> +		maccfg2 |= MACCFG2_BYTE_MODE;
>>>>>>    		break;
>>>>>>    	case PHY_INTERFACE_MODE_SGMII:
>>>>>>    	case PHY_INTERFACE_MODE_1000BASEX:
>>>>>>    	case PHY_INTERFACE_MODE_2500BASEX:
>>>>>> -		tmp = DTSEC_ECNTRL_TBIM | DTSEC_ECNTRL_SGMIIM;
>>>>>> +		ecntrl = DTSEC_ECNTRL_TBIM | DTSEC_ECNTRL_SGMIIM;
>>>>>> +		maccfg2 |= MACCFG2_BYTE_MODE;
>>>>>>    		break;
>>>>>>    	default:
>>>>>>    		dev_warn(mac_dev->dev, "cannot configure dTSEC for %s\n",
>>>>>> @@ -923,7 +929,8 @@ static void dtsec_mac_config(struct phylink_config *config, unsigned int mode,
>>>>>>    		return;
>>>>>>    	}
>>>>>>    
>>>>>> -	iowrite32be(tmp, &regs->ecntrl);
>>>>>> +	iowrite32be(ecntrl, &regs->ecntrl);
>>>>>> +	iowrite32be(maccfg2, &regs->maccfg2);
>>>>>>    }
>>>>>>    
>>>>>>    static void dtsec_link_up(struct phylink_config *config, struct phy_device *phy,
>>>>>> @@ -948,15 +955,10 @@ static void dtsec_link_up(struct phylink_config *config, struct phy_device *phy,
>>>>>>    	iowrite32be(tmp, &regs->ecntrl);
>>>>>>    
>>>>>>    	tmp = ioread32be(&regs->maccfg2);
>>>>>> -	tmp &= ~(MACCFG2_NIBBLE_MODE | MACCFG2_BYTE_MODE | MACCFG2_FULL_DUPLEX);
>>>>>> -	if (speed >= SPEED_1000)
>>>>>> -		tmp |= MACCFG2_BYTE_MODE;
>>>>>> -	else
>>>>>> -		tmp |= MACCFG2_NIBBLE_MODE;
>>>>>> -
>>>>>>    	if (duplex == DUPLEX_FULL)
>>>>>>    		tmp |= MACCFG2_FULL_DUPLEX;
>>>>>> -
>>>>>> +	else
>>>>>> +		tmp &= ~MACCFG2_FULL_DUPLEX;
>>>>>
>>>>> Did you test this when forcing 10/100 speed?
>>>>
>>>> No I didn't. Well I can't. I have a very weird board which only
>>>> supports 1000base-X (and copper SFPs in 1000basex autoneg mode). On
>>>> top of that there is a Marvell 88E1112 in between the SFP and the
>>>> MAC, for which the PHY driver is completely broken. Long story
>>>> short, I'm not able to test that (yet/at all? Not sure).
>>> FTR, sashiko suspect this patch will broke such setup:
>>> https://sashiko.dev/#/patchset/20260710143430.2276141-1-mwalle%40kernel.org
>>
>> I've seen that, but.. that was the actual change between v1 and v2
>> as suggested by Sean. It does not depend on the actual link speed,
>> but the maximum link speed. So it is not relevant if the link
>> negotiates to a slower speed or not. At least that now matches the
>> behavior prior to the phylink conversion. If that was working -
>> that I can't tell you.
> 
> I'm sorry, following all cross revision discussion is a bit hard here.
> 
> I don't understand if the 'link never established' is specific of your
> board, or it a constant with this driver. Could you please clarify?

I think we should set byte/nibble mode in link_up as well as mac_config. That
should fix the problem where the link never comes up (possibly broken since
the phylink conversion). I think it's unlikely that this config is supposed
to match the phy interface speed and not the link speed, but you never know...

--Sean

Re: [PATCH net v2] net: dpaa: fix mode setting

[Paolo Abeni]

On 7/17/26 2:05 PM, Sean Anderson wrote:
> On 7/17/26 07:11, Paolo Abeni wrote:
>> On 7/17/26 12:13 PM, Michael Walle wrote:
>>> I've seen that, but.. that was the actual change between v1 and v2
>>> as suggested by Sean. It does not depend on the actual link speed,
>>> but the maximum link speed. So it is not relevant if the link
>>> negotiates to a slower speed or not. At least that now matches the
>>> behavior prior to the phylink conversion. If that was working -
>>> that I can't tell you.
>>
>> I'm sorry, following all cross revision discussion is a bit hard here.
>>
>> I don't understand if the 'link never established' is specific of your
>> board, or it a constant with this driver. Could you please clarify?
> 
> I think we should set byte/nibble mode in link_up as well as mac_config. That
> should fix the problem where the link never comes up (possibly broken since
> the phylink conversion). I think it's unlikely that this config is supposed
> to match the phy interface speed and not the link speed, but you never know...
FTR, the above does not answer my question ;)

Still I think (mostly guess) bringing the mode initialization in both
link_up and mac_config should be safer (that is, I agree with the above
plan).

/P

Re: [PATCH net v2] net: dpaa: fix mode setting

[Michael Walle]

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

On Fri Jul 17, 2026 at 2:44 PM CEST, Paolo Abeni wrote:
> On 7/17/26 2:05 PM, Sean Anderson wrote:
>> On 7/17/26 07:11, Paolo Abeni wrote:
>>> On 7/17/26 12:13 PM, Michael Walle wrote:
>>>> I've seen that, but.. that was the actual change between v1 and v2
>>>> as suggested by Sean. It does not depend on the actual link speed,
>>>> but the maximum link speed. So it is not relevant if the link
>>>> negotiates to a slower speed or not. At least that now matches the
>>>> behavior prior to the phylink conversion. If that was working -
>>>> that I can't tell you.
>>>
>>> I'm sorry, following all cross revision discussion is a bit hard here.
>>>
>>> I don't understand if the 'link never established' is specific of your
>>> board, or it a constant with this driver. Could you please clarify?

I don't think it's related to my board, but rather to the link being
a SGMII one on my board. I assume Sean had an RGMII link while
converting the driver (?) and moreover, that it worked correctly
there :)

>> I think we should set byte/nibble mode in link_up as well as mac_config. That
>> should fix the problem where the link never comes up (possibly broken since
>> the phylink conversion). I think it's unlikely that this config is supposed
>> to match the phy interface speed and not the link speed, but you never know...
> FTR, the above does not answer my question ;)
>
> Still I think (mostly guess) bringing the mode initialization in both
> link_up and mac_config should be safer (that is, I agree with the above
> plan).

I agree. I looked at the original driver and there adjust_link()
will also select between byte and nibble mode. I'll prepare a v3.

Sorry, I misunderstood Sean's intention/suggestion, thanks for
pushing on this. FWIW, I also tried with nibble mode as the initial
configuration on my board and link_up() is getting called. Whereas
if it's 0 (as in being documented as "reserved", though it's the
default), .link_up() is never called. Thus, it seems that it just
has to be any valid setting until link_up() will set the correct
value.

-michael

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

Re: [PATCH net v2] net: dpaa: fix mode setting

[Sean Anderson]

On 7/17/26 08:57, Michael Walle wrote:
> On Fri Jul 17, 2026 at 2:44 PM CEST, Paolo Abeni wrote:
>> On 7/17/26 2:05 PM, Sean Anderson wrote:
>>> On 7/17/26 07:11, Paolo Abeni wrote:
>>>> On 7/17/26 12:13 PM, Michael Walle wrote:
>>>>> I've seen that, but.. that was the actual change between v1 and v2
>>>>> as suggested by Sean. It does not depend on the actual link speed,
>>>>> but the maximum link speed. So it is not relevant if the link
>>>>> negotiates to a slower speed or not. At least that now matches the
>>>>> behavior prior to the phylink conversion. If that was working -
>>>>> that I can't tell you.
>>>>
>>>> I'm sorry, following all cross revision discussion is a bit hard here.
>>>>
>>>> I don't understand if the 'link never established' is specific of your
>>>> board, or it a constant with this driver. Could you please clarify?
> 
> I don't think it's related to my board, but rather to the link being
> a SGMII one on my board. I assume Sean had an RGMII link while
> converting the driver (?) and moreover, that it worked correctly
> there :)

I didn't have a link at all! I only personally tested this conversion on
layerscape (memac). I'm pleased to learn that there were only two major
bugs, but a little concerned that it took four years to discover one of them.

>>> I think we should set byte/nibble mode in link_up as well as mac_config. That
>>> should fix the problem where the link never comes up (possibly broken since
>>> the phylink conversion). I think it's unlikely that this config is supposed
>>> to match the phy interface speed and not the link speed, but you never know...
>> FTR, the above does not answer my question ;)
>>
>> Still I think (mostly guess) bringing the mode initialization in both
>> link_up and mac_config should be safer (that is, I agree with the above
>> plan).
> 
> I agree. I looked at the original driver and there adjust_link()
> will also select between byte and nibble mode. I'll prepare a v3.
> 
> Sorry, I misunderstood Sean's intention/suggestion, thanks for
> pushing on this. FWIW, I also tried with nibble mode as the initial
> configuration on my board and link_up() is getting called. Whereas
> if it's 0 (as in being documented as "reserved", though it's the
> default), .link_up() is never called. Thus, it seems that it just
> has to be any valid setting until link_up() will set the correct
> value.
> 
> -michael