[PATCH RFC net-next 0/4] Various mtk_eth_soc cleanups

[PATCH RFC net-next 0/4] Various mtk_eth_soc cleanups

[Russell King (Oracle)]

Hi,

These are a number of patches that do a bit of cleanup to mtk_eth_soc,
including one (the last) which seems to indicate that I somehow missed
adding RMII and REVMII to the supported_interfaces - but no one has
complained, and checking the DT files in the kernel, no one uses these
modes (so the driver is probably untested for these.)

The first patch cleans up mtk_gmac0_rgmii_adjust(), which is the
troublesome function preventing the driver becoming a post-March2020
phylink driver. It doesn't solve that problem, merely makes the code
easier to follow by getting rid of repeated tenary operators.

The second patch moves the check for DDR2 memory to the initialisation
of phylink's supported_interfaces - if TRGMII is not possible for some
reason, we should not be erroring out in phylink MAC operations when
that can be determined prior to phylink creation.

The third patch removes checks from mtk_mac_config() that are done
when initialising supported_interfaces - phylink will not call
mtk_mac_config() with an interface that was not marked as supported,
so these checks are redundant.

The last patch adds comments for REVMII and RMII. As I note, these
seem to be unused in the DT files, and as no one has reported that
these don't work, I suggest that no one uses them. Should we drop
support for these modes, or add them to supported_interfaces?

Please review, and I'll collect attributations for resending after
net-next has re-opened.

Thanks.

 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 71 ++++++++++++++---------------
 1 file changed, 34 insertions(+), 37 deletions(-)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

[PATCH RFC net-next 1/4] net: mtk_eth_soc: tidy mtk_gmac0_rgmii_adjust()

[Russell King (Oracle)]

Get rid of the multiple tenary operators in mtk_gmac0_rgmii_adjust()
replacing them with a single if(), thus making the code easier to read.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 34 ++++++++++++---------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 14be6ea51b88..c63f17929ccf 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -397,38 +397,42 @@ static int mt7621_gmac0_rgmii_adjust(struct mtk_eth *eth,
 static void mtk_gmac0_rgmii_adjust(struct mtk_eth *eth,
 				   phy_interface_t interface, int speed)
 {
-	u32 val;
+	unsigned long rate;
+	u32 tck, rck, intf;
 	int ret;
 
 	if (interface == PHY_INTERFACE_MODE_TRGMII) {
 		mtk_w32(eth, TRGMII_MODE, INTF_MODE);
-		val = 500000000;
-		ret = clk_set_rate(eth->clks[MTK_CLK_TRGPLL], val);
+		ret = clk_set_rate(eth->clks[MTK_CLK_TRGPLL], 500000000);
 		if (ret)
 			dev_err(eth->dev, "Failed to set trgmii pll: %d\n", ret);
 		return;
 	}
 
-	val = (speed == SPEED_1000) ?
-		INTF_MODE_RGMII_1000 : INTF_MODE_RGMII_10_100;
-	mtk_w32(eth, val, INTF_MODE);
+	if (speed == SPEED_1000) {
+		intf = INTF_MODE_RGMII_1000;
+		rate = 250000000;
+		rck = RCK_CTRL_RGMII_1000;
+		tck = TCK_CTRL_RGMII_1000;
+	} else {
+		intf = INTF_MODE_RGMII_10_100;
+		rate = 500000000;
+		rck = RCK_CTRL_RGMII_10_100;
+		tck = TCK_CTRL_RGMII_10_100;
+	}
+
+	mtk_w32(eth, intf, INTF_MODE);
 
 	regmap_update_bits(eth->ethsys, ETHSYS_CLKCFG0,
 			   ETHSYS_TRGMII_CLK_SEL362_5,
 			   ETHSYS_TRGMII_CLK_SEL362_5);
 
-	val = (speed == SPEED_1000) ? 250000000 : 500000000;
-	ret = clk_set_rate(eth->clks[MTK_CLK_TRGPLL], val);
+	ret = clk_set_rate(eth->clks[MTK_CLK_TRGPLL], rate);
 	if (ret)
 		dev_err(eth->dev, "Failed to set trgmii pll: %d\n", ret);
 
-	val = (speed == SPEED_1000) ?
-		RCK_CTRL_RGMII_1000 : RCK_CTRL_RGMII_10_100;
-	mtk_w32(eth, val, TRGMII_RCK_CTRL);
-
-	val = (speed == SPEED_1000) ?
-		TCK_CTRL_RGMII_1000 : TCK_CTRL_RGMII_10_100;
-	mtk_w32(eth, val, TRGMII_TCK_CTRL);
+	mtk_w32(eth, rck, TRGMII_RCK_CTRL);
+	mtk_w32(eth, tck, TRGMII_TCK_CTRL);
 }
 
 static struct phylink_pcs *mtk_mac_select_pcs(struct phylink_config *config,
-- 
2.30.2

Re: [PATCH RFC net-next 1/4] net: mtk_eth_soc: tidy mtk_gmac0_rgmii_adjust()

[Simon Horman]

On Fri, Feb 24, 2023 at 12:36:11PM +0000, Russell King (Oracle) wrote:
> Get rid of the multiple tenary operators in mtk_gmac0_rgmii_adjust()
> replacing them with a single if(), thus making the code easier to read.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

[PATCH RFC net-next 2/4] net: mtk_eth_soc: move trgmii ddr2 check to probe function

[Russell King (Oracle)]

If TRGMII mode is not permitted when using DDR2 mode, we should handle
that when setting up phylink's ->supported_interfaces so phylink knows
that this is not supported by the hardware. Move this check to
mtk_add_mac().

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index c63f17929ccf..1b385dfe620f 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -374,17 +374,6 @@ static int mt7621_gmac0_rgmii_adjust(struct mtk_eth *eth,
 {
 	u32 val;
 
-	/* Check DDR memory type.
-	 * Currently TRGMII mode with DDR2 memory is not supported.
-	 */
-	regmap_read(eth->ethsys, ETHSYS_SYSCFG, &val);
-	if (interface == PHY_INTERFACE_MODE_TRGMII &&
-	    val & SYSCFG_DRAM_TYPE_DDR2) {
-		dev_err(eth->dev,
-			"TRGMII mode with DDR2 memory is not supported!\n");
-		return -EOPNOTSUPP;
-	}
-
 	val = (interface == PHY_INTERFACE_MODE_TRGMII) ?
 		ETHSYS_TRGMII_MT7621_DDR_PLL : 0;
 
@@ -4333,6 +4322,7 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 	struct mtk_mac *mac;
 	int id, err;
 	int txqs = 1;
+	u32 val;
 
 	if (!_id) {
 		dev_err(eth->dev, "missing mac id\n");
@@ -4409,6 +4399,15 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 		__set_bit(PHY_INTERFACE_MODE_TRGMII,
 			  mac->phylink_config.supported_interfaces);
 
+	/* TRGMII is not permitted on MT7621 if using DDR2 */
+	if (MTK_HAS_CAPS(mac->hw->soc->caps, MTK_GMAC1_TRGMII) &&
+	    MTK_HAS_CAPS(mac->hw->soc->caps, MTK_TRGMII_MT7621_CLK)) {
+		regmap_read(eth->ethsys, ETHSYS_SYSCFG, &val);
+		if (val & SYSCFG_DRAM_TYPE_DDR2)
+			__clear_bit(PHY_INTERFACE_MODE_TRGMII,
+				    mac->phylink_config.supported_interfaces);
+	}
+
 	if (MTK_HAS_CAPS(mac->hw->soc->caps, MTK_SGMII)) {
 		__set_bit(PHY_INTERFACE_MODE_SGMII,
 			  mac->phylink_config.supported_interfaces);
-- 
2.30.2

Re: [PATCH RFC net-next 2/4] net: mtk_eth_soc: move trgmii ddr2 check to probe function

[Simon Horman]

On Fri, Feb 24, 2023 at 12:36:16PM +0000, Russell King (Oracle) wrote:
> If TRGMII mode is not permitted when using DDR2 mode, we should handle
> that when setting up phylink's ->supported_interfaces so phylink knows
> that this is not supported by the hardware. Move this check to
> mtk_add_mac().
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

[PATCH RFC net-next 3/4] net: mtk_eth_soc: remove unnecessary checks in mtk_mac_config()

[Russell King (Oracle)]

mtk_mac_config() checks that the interface mode is permitted for the
capabilities, but we already do these checks in mtk_add_mac() when
initialising phylink's supported_interfaces bitmap. Remove the
unnecessary tests.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 1b385dfe620f..f78810717f66 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -458,12 +458,6 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode,
 		/* Setup soc pin functions */
 		switch (state->interface) {
 		case PHY_INTERFACE_MODE_TRGMII:
-			if (mac->id)
-				goto err_phy;
-			if (!MTK_HAS_CAPS(mac->hw->soc->caps,
-					  MTK_GMAC1_TRGMII))
-				goto err_phy;
-			fallthrough;
 		case PHY_INTERFACE_MODE_RGMII_TXID:
 		case PHY_INTERFACE_MODE_RGMII_RXID:
 		case PHY_INTERFACE_MODE_RGMII_ID:
@@ -480,11 +474,9 @@ static void mtk_mac_config(struct phylink_config *config, unsigned int mode,
 		case PHY_INTERFACE_MODE_1000BASEX:
 		case PHY_INTERFACE_MODE_2500BASEX:
 		case PHY_INTERFACE_MODE_SGMII:
-			if (MTK_HAS_CAPS(eth->soc->caps, MTK_SGMII)) {
-				err = mtk_gmac_sgmii_path_setup(eth, mac->id);
-				if (err)
-					goto init_err;
-			}
+			err = mtk_gmac_sgmii_path_setup(eth, mac->id);
+			if (err)
+				goto init_err;
 			break;
 		case PHY_INTERFACE_MODE_GMII:
 			if (MTK_HAS_CAPS(eth->soc->caps, MTK_GEPHY)) {
-- 
2.30.2

Re: [PATCH RFC net-next 3/4] net: mtk_eth_soc: remove unnecessary checks in mtk_mac_config()

[Simon Horman]

On Fri, Feb 24, 2023 at 12:36:21PM +0000, Russell King (Oracle) wrote:
> mtk_mac_config() checks that the interface mode is permitted for the
> capabilities, but we already do these checks in mtk_add_mac() when
> initialising phylink's supported_interfaces bitmap. Remove the
> unnecessary tests.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

[PATCH RFC net-next 4/4] net: mtk_eth_soc: note interface modes not set in supported_interfaces

[Russell King (Oracle)]

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

Re: [PATCH RFC net-next 4/4] net: mtk_eth_soc: note interface modes not set in supported_interfaces

[Simon Horman]

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(+)

...

Re: [PATCH RFC net-next 4/4] net: mtk_eth_soc: note interface modes not set in supported_interfaces

[Russell King (Oracle)]

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.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH RFC net-next 4/4] net: mtk_eth_soc: note interface modes not set in supported_interfaces

[Simon Horman]

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.

Re: [PATCH RFC net-next 4/4] net: mtk_eth_soc: note interface modes not set in supported_interfaces

[Russell King (Oracle)]

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

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH RFC net-next 4/4] net: mtk_eth_soc: note interface modes not set in supported_interfaces

[Daniel Golle]

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!

Re: [PATCH RFC net-next 4/4] net: mtk_eth_soc: note interface modes not set in supported_interfaces

[Russell King (Oracle)]

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?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH RFC net-next 4/4] net: mtk_eth_soc: note interface modes not set in supported_interfaces

[Daniel Golle]

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.