| Message ID | E1pVXJK-00CTAl-V7@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Various mtk_eth_soc cleanups | expand |
On Fri, Feb 24, 2023 at 12:36:26PM +0000, Russell King (Oracle) wrote: Hi Russell, I think it would be good to add a patch description here. Code change looks good to me. > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > --- > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 ++ > 1 file changed, 2 insertions(+) ...
On Sat, Feb 25, 2023 at 05:49:18PM +0100, Simon Horman wrote: > On Fri, Feb 24, 2023 at 12:36:26PM +0000, Russell King (Oracle) wrote: > > Hi Russell, > > I think it would be good to add a patch description here. > > Code change looks good to me. As noted in the cover message, this is to highlight the issue to hopefully get folk to think what we should do about RMII and REVMII in this driver - basically, should we continue to support them, or remove it completely. Either way, this patch won't hit net-next in its current form. Essentially, the choice is either we remove these two switch cases, or we add these interface modes to the supported_interfaces array. I'd rather those with mtk_eth_soc made the decision, even though it is highly unlikely that these modes are used on the hardware they have - as I don't have any mediatek hardware. Thanks.
On Sat, Feb 25, 2023 at 07:11:44PM +0000, Russell King (Oracle) wrote: > On Sat, Feb 25, 2023 at 05:49:18PM +0100, Simon Horman wrote: > > On Fri, Feb 24, 2023 at 12:36:26PM +0000, Russell King (Oracle) wrote: > > > > Hi Russell, > > > > I think it would be good to add a patch description here. > > > > Code change looks good to me. > > As noted in the cover message, this is to highlight the issue to > hopefully get folk to think what we should do about RMII and REVMII > in this driver - basically, should we continue to support them, or > remove it completely. > > Either way, this patch won't hit net-next in its current form. > > Essentially, the choice is either we remove these two switch cases, > or we add these interface modes to the supported_interfaces array. > > I'd rather those with mtk_eth_soc made the decision, even though it > is highly unlikely that these modes are used on the hardware they > have - as I don't have any mediatek hardware. > > Thanks. Thanks, understood. Sorry for missing this earlier.
To any of those with Mediatek hardware... Any opinions on what we should do concerning these modes? If I don't get a reply, then I'll delete these. There's no point having this code if it's been unreachable for a while and no one's complained. Thanks. On Fri, Feb 24, 2023 at 12:36:26PM +0000, Russell King (Oracle) wrote: > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > --- > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > index f78810717f66..280490942fa4 100644 > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > @@ -530,9 +530,11 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode, > case PHY_INTERFACE_MODE_GMII: > ge_mode = 1; > break; > + /* FIXME: RMII is not set in the phylink capabilities */ > case PHY_INTERFACE_MODE_REVMII: > ge_mode = 2; > break; > + /* FIXME: RMII is not set in the phylink capabilities */ > case PHY_INTERFACE_MODE_RMII: > if (mac->id) > goto err_phy; > -- > 2.30.2 > >
On Tue, Mar 07, 2023 at 12:01:17PM +0000, Russell King (Oracle) wrote: > To any of those with Mediatek hardware... Any opinions on what we should > do concerning these modes? > > If I don't get a reply, then I'll delete these. There's no point having > this code if it's been unreachable for a while and no one's complained. A quick grep through the device trees of the more than 650 ramips and mediatek boards we support in OpenWrt has revealed that *none* of them uses either reduced-MII or reverse-MII PHY modes. I could imaging that some more specialized ramips boards may use the RMII 100M PHY mode to connect with exotic PHYs for industrial or automotive applications (think: for 100BASE-T1 PHY connected via RMII). I have never seen or touched such boards, but there are hints that they do exist. For reverse-MII there are cases in which the Ralink SoC (Rt305x, for example) is used in iNIC mode, ie. connected as a PHY to another SoC, and running only a minimal firmware rather than running Linux. Due to the lack of external DRAM for the Ralink SoC on this kind of boards, the Ralink SoC there will anyway never be able to boot Linux. I've seen this e.g. in multimedia devices like early WiFi-connected not-yet-so-smart TVs. Tl;dr: I'd drop them. If anyone really needs them, it would be easy to add them again and then also add them to the phylink capability mask. > > Thanks. > > On Fri, Feb 24, 2023 at 12:36:26PM +0000, Russell King (Oracle) wrote: > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > --- > > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > > index f78810717f66..280490942fa4 100644 > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > > @@ -530,9 +530,11 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode, > > case PHY_INTERFACE_MODE_GMII: > > ge_mode = 1; > > break; > > + /* FIXME: RMII is not set in the phylink capabilities */ > > case PHY_INTERFACE_MODE_REVMII: > > ge_mode = 2; > > break; > > + /* FIXME: RMII is not set in the phylink capabilities */ > > case PHY_INTERFACE_MODE_RMII: > > if (mac->id) > > goto err_phy; > > -- > > 2.30.2 > > > > > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On Tue, Mar 07, 2023 at 01:25:23PM +0000, Daniel Golle wrote: > A quick grep through the device trees of the more than 650 ramips and > mediatek boards we support in OpenWrt has revealed that *none* of them > uses either reduced-MII or reverse-MII PHY modes. I could imaging that > some more specialized ramips boards may use the RMII 100M PHY mode to > connect with exotic PHYs for industrial or automotive applications > (think: for 100BASE-T1 PHY connected via RMII). I have never seen or > touched such boards, but there are hints that they do exist. > > For reverse-MII there are cases in which the Ralink SoC (Rt305x, for > example) is used in iNIC mode, ie. connected as a PHY to another SoC, > and running only a minimal firmware rather than running Linux. Due to > the lack of external DRAM for the Ralink SoC on this kind of boards, > the Ralink SoC there will anyway never be able to boot Linux. > I've seen this e.g. in multimedia devices like early WiFi-connected > not-yet-so-smart TVs. > > Tl;dr: I'd drop them. If anyone really needs them, it would be easy to > add them again and then also add them to the phylink capability mask. Thanks! That seems to be well reasoned. Would you have any objection to using the above as part of the commit message removing these modes?
On Tue, Mar 07, 2023 at 02:04:59PM +0000, Russell King (Oracle) wrote: > On Tue, Mar 07, 2023 at 01:25:23PM +0000, Daniel Golle wrote: > > A quick grep through the device trees of the more than 650 ramips and > > mediatek boards we support in OpenWrt has revealed that *none* of them > > uses either reduced-MII or reverse-MII PHY modes. I could imaging that > > some more specialized ramips boards may use the RMII 100M PHY mode to > > connect with exotic PHYs for industrial or automotive applications > > (think: for 100BASE-T1 PHY connected via RMII). I have never seen or > > touched such boards, but there are hints that they do exist. > > > > For reverse-MII there are cases in which the Ralink SoC (Rt305x, for > > example) is used in iNIC mode, ie. connected as a PHY to another SoC, > > and running only a minimal firmware rather than running Linux. Due to > > the lack of external DRAM for the Ralink SoC on this kind of boards, > > the Ralink SoC there will anyway never be able to boot Linux. > > I've seen this e.g. in multimedia devices like early WiFi-connected > > not-yet-so-smart TVs. > > > > Tl;dr: I'd drop them. If anyone really needs them, it would be easy to > > add them again and then also add them to the phylink capability mask. > > Thanks! That seems to be well reasoned. Would you have any objection to > using the above as part of the commit message removing these modes? Sure, go ahead, sounds good to me.
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index f78810717f66..280490942fa4 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -530,9 +530,11 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode, case PHY_INTERFACE_MODE_GMII: ge_mode = 1; break; + /* FIXME: RMII is not set in the phylink capabilities */ case PHY_INTERFACE_MODE_REVMII: ge_mode = 2; break; + /* FIXME: RMII is not set in the phylink capabilities */ case PHY_INTERFACE_MODE_RMII: if (mac->id) goto err_phy;
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 ++ 1 file changed, 2 insertions(+)