Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1605472 > unrolled thread

[PATCH net-next v2 0/5] net-next: dsa: add Mediatek MT7530 support

Back to article view | Back to linux.kernel

Contents

  [PATCH net-next v2 0/5] net-next: dsa: add Mediatek MT7530 support <sean.wang@mediatek.com> - 2017-03-21 10:40 +0100
    [PATCH net-next v2 1/5] dt-bindings: net: dsa: add Mediatek MT7530 binding <sean.wang@mediatek.com> - 2017-03-21 10:40 +0100
      Re: [PATCH net-next v2 1/5] dt-bindings: net: dsa: add Mediatek  MT7530 binding Rob Herring <robh@kernel.org> - 2017-03-24 17:20 +0100
    [PATCH net-next v2 3/5] net-next: ethernet: mediatek: add CDM able to recognize the tag for DSA <sean.wang@mediatek.com> - 2017-03-21 10:50 +0100
      Re: [PATCH net-next v2 3/5] net-next: ethernet: mediatek: add CDM  able to recognize the tag for DSA Andrew Lunn <andrew@lunn.ch> - 2017-03-22 18:50 +0100
      Re: [PATCH net-next v2 3/5] net-next: ethernet: mediatek: add CDM  able to recognize the tag for DSA Florian Fainelli <f.fainelli@gmail.com> - 2017-03-22 19:30 +0100
    [PATCH net-next v2 4/5] net-next: ethernet: mediatek: add device_node of GMAC pointing into the netdev instance <sean.wang@mediatek.com> - 2017-03-21 10:50 +0100
      Re: [PATCH net-next v2 4/5] net-next: ethernet: mediatek: add  device_node of GMAC pointing into the netdev instance Andrew Lunn <andrew@lunn.ch> - 2017-03-22 19:20 +0100
      Re: [PATCH net-next v2 4/5] net-next: ethernet: mediatek: add  device_node of GMAC pointing into the netdev instance Florian Fainelli <f.fainelli@gmail.com> - 2017-03-22 19:30 +0100
    [PATCH net-next v2 2/5] net-next: dsa: add Mediatek tag RX/TX handler <sean.wang@mediatek.com> - 2017-03-21 10:50 +0100
    Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for  Mediatek MT7530 switch Florian Fainelli <f.fainelli@gmail.com> - 2017-03-22 19:40 +0100
    Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for  Mediatek MT7530 switch Andrew Lunn <andrew@lunn.ch> - 2017-03-22 19:40 +0100
    Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for  Mediatek MT7530 switch Andrew Lunn <andrew@lunn.ch> - 2017-03-23 08:50 +0100
      Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for  Mediatek MT7530 switch Sean Wang <sean.wang@mediatek.com> - 2017-03-23 09:10 +0100
        Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for  Mediatek MT7530 switch Felix Fietkau <nbd@nbd.name> - 2017-03-23 15:10 +0100
          Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for  Mediatek MT7530 switch John Crispin <john@phrozen.org> - 2017-03-23 15:30 +0100
            Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for  Mediatek MT7530 switch Felix Fietkau <nbd@nbd.name> - 2017-03-23 15:40 +0100
        Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for  Mediatek MT7530 switch Andrew Lunn <andrew@lunn.ch> - 2017-03-24 09:00 +0100
    Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for  Mediatek MT7530 switch Andrew Lunn <andrew@lunn.ch> - 2017-03-24 15:10 +0100
    Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for  Mediatek MT7530 switch Andrew Lunn <andrew@lunn.ch> - 2017-03-24 15:30 +0100

#1605472 — [PATCH net-next v2 0/5] net-next: dsa: add Mediatek MT7530 support

From: Sean Wang <sean.wang@mediatek.com>

MT7530 is a 7-ports Gigabit Ethernet Switch that could be found on
Mediatek router platforms such as MT7623A or MT7623N which includes 7-port
Gigabit Ethernet MAC and 5-port Gigabit Ethernet PHY. Among these ports,
The port from 0 to 4 are the user ports connecting with the remote devices
while the port 5 and 6 are the CPU ports connecting into Mediatek Ethernet
GMAC.

The patch series integrated Mediatek MT7530 into DSA support which
includes the most of the essential callbacks such as tag insertion for
port distinguishing, port control, bridge offloading, STP setup and
ethtool operations to allow DSA to model each user port into independently
standalone netdevice as the other DSA driver had done.

Changes since v1:
- rebased into 4.11-rc1
- refined binding document including below five items 
- changed the type of mediatek,mcm into bool
- used reset controller binding for MCM reset and removed "mediatek,ethsys"
  property from binding
- reused CPU port's ethernet Phandle instead of creating new one and removed
  "mediatek,ethernet" property from binding
- aligned naming for GPIO reset with dsa/marvell.txt
- added phy-mode as required property child nodes within ports container
- handled gpio reset with devm_gpiod_* API
- refined comment words
- removed condition for CDM setting since the setup looks both fine for all cases
- allowed of_find_net_device_by_node() working with pointing the device node into
  real netdev instance
- fixed Kbuild warnings

Sean Wang (5):
  dt-bindings: net: dsa: add Mediatek MT7530 binding
  net-next: dsa: add Mediatek tag RX/TX handler
  net-next: ethernet: mediatek: add CDM able to recognize the tag for
    DSA
  net-next: ethernet: mediatek: add device_node of GMAC pointing into
    the netdev instance
  net-next: dsa: add dsa support for Mediatek MT7530 switch

 .../devicetree/bindings/net/dsa/mt7530.txt         |   92 ++
 drivers/net/dsa/Kconfig                            |    8 +
 drivers/net/dsa/Makefile                           |    2 +-
 drivers/net/dsa/mt7530.c                           | 1172 ++++++++++++++++++++
 drivers/net/dsa/mt7530.h                           |  382 +++++++
 drivers/net/ethernet/mediatek/mtk_eth_soc.c        |    8 +
 drivers/net/ethernet/mediatek/mtk_eth_soc.h        |    4 +
 include/net/dsa.h                                  |    1 +
 net/dsa/Kconfig                                    |    2 +
 net/dsa/Makefile                                   |    1 +
 net/dsa/dsa.c                                      |    3 +
 net/dsa/dsa_priv.h                                 |    3 +
 net/dsa/tag_mtk.c                                  |  117 ++
 13 files changed, 1794 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/net/dsa/mt7530.txt
 create mode 100644 drivers/net/dsa/mt7530.c
 create mode 100644 drivers/net/dsa/mt7530.h
 create mode 100644 net/dsa/tag_mtk.c

-- 
1.9.1

[toc] | [next] | [standalone]

#1605473 — [PATCH net-next v2 1/5] dt-bindings: net: dsa: add Mediatek MT7530 binding

From: Sean Wang <sean.wang@mediatek.com>

Add device-tree binding for Mediatek MT7530 switch.

Cc: devicetree@vger.kernel.org
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 .../devicetree/bindings/net/dsa/mt7530.txt         | 92 ++++++++++++++++++++++
 1 file changed, 92 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/dsa/mt7530.txt

diff --git a/Documentation/devicetree/bindings/net/dsa/mt7530.txt b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
new file mode 100644
index 0000000..a9bc27b
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dsa/mt7530.txt
@@ -0,0 +1,92 @@
+Mediatek MT7530 Ethernet switch
+================================
+
+Required properties:
+
+- compatible: Must be compatible = "mediatek,mt7530";
+- #address-cells: Must be 1.
+- #size-cells: Must be 0.
+- mediatek,mcm: Boolean; if defined, indicates that either MT7530 is the part
+	on multi-chip module belong to MT7623A has or the remotely standalone
+	chip as the function MT7623N reference board provided for.
+- core-supply: Phandle to the regulator node necessary for the core power.
+- io-supply: Phandle to the regulator node necessary for the I/O power.
+	See Documentation/devicetree/bindings/regulator/mt6323-regulator.txt
+	for details for the regulator setup on these boards.
+
+If the property mediatek,mcm isn't defined, following property is required
+
+- reset-gpios: Should be a gpio specifier for a reset line.
+
+Else, following properties are required
+
+- resets : Phandle pointing to the system reset controller with
+	line index for the ethsys.
+- reset-names : Should be set to "mcm".
+
+Required properties for the child nodes within ports container:
+
+- reg: Port address described must be 6 for CPU port and from 0 to 5 for
+	user ports.
+- phy-mode: String, must be either "trgmii" or "rgmii" for port labeled
+	 "cpu".
+
+See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional
+required, optional properties and how the integrated switch subnodes must
+be specified.
+
+Example:
+
+	&mdio0 {
+		switch@0 {
+			compatible = "mediatek,mt7530";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0>;
+
+			core-supply = <&mt6323_vpa_reg>;
+			io-supply = <&mt6323_vemc3v3_reg>;
+			reset-gpios = <&pio 33 0>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0>;
+				port@0 {
+					reg = <0>;
+					label = "lan0";
+				};
+
+				port@1 {
+					reg = <1>;
+					label = "lan1";
+				};
+
+				port@2 {
+					reg = <2>;
+					label = "lan2";
+				};
+
+				port@3 {
+					reg = <3>;
+					label = "lan3";
+				};
+
+				port@4 {
+					reg = <4>;
+					label = "wan";
+				};
+
+				port@6 {
+					reg = <6>;
+					label = "cpu";
+					ethernet = <&gmac0>;
+					phy-mode = "trgmii";
+					fixed-link {
+						speed = <1000>;
+						full-duplex;
+					};
+				};
+			};
+		};
+	};
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1608627 — Re: [PATCH net-next v2 1/5] dt-bindings: net: dsa: add Mediatek MT7530 binding

On Tue, Mar 21, 2017 at 05:35:06PM +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> Add device-tree binding for Mediatek MT7530 switch.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  .../devicetree/bindings/net/dsa/mt7530.txt         | 92 ++++++++++++++++++++++
>  1 file changed, 92 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/dsa/mt7530.txt

Acked-by: Rob Herring <robh@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1605474 — [PATCH net-next v2 3/5] net-next: ethernet: mediatek: add CDM able to recognize the tag for DSA

From: Sean Wang <sean.wang@mediatek.com>

The patch adds the setup for allowing CDM can recognize these packets with
carrying port-distinguishing tag. Otherwise, these tagging packets will be
handled incorrectly by CDM. The setup is working out for general untag
packets as well.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Signed-off-by: Landen Chao <Landen.Chao@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 6 ++++++
 drivers/net/ethernet/mediatek/mtk_eth_soc.h | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 9e75768..c21ed99 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1846,6 +1846,12 @@ static int mtk_hw_init(struct mtk_eth *eth)
 	/* GE2, Force 1000M/FD, FC ON */
 	mtk_w32(eth, MAC_MCR_FIXED_LINK, MTK_MAC_MCR(1));
 
+	/* Indicates CDM to parse the MTK special tag from CPU
+	 * which also is working out for untag packets.
+	 */
+	val = mtk_r32(eth, MTK_CDMQ_IG_CTRL);
+	mtk_w32(eth, val | MTK_CDMQ_STAG_EN, MTK_CDMQ_IG_CTRL);
+
 	/* Enable RX VLan Offloading */
 	mtk_w32(eth, 1, MTK_CDMP_EG_CTRL);
 
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 99b1c8e..996024d 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -70,6 +70,10 @@
 /* Frame Engine Interrupt Grouping Register */
 #define MTK_FE_INT_GRP		0x20
 
+/* CDMP Ingress Control Register */
+#define MTK_CDMQ_IG_CTRL	0x1400
+#define MTK_CDMQ_STAG_EN	BIT(0)
+
 /* CDMP Exgress Control Register */
 #define MTK_CDMP_EG_CTRL	0x404
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1606831 — Re: [PATCH net-next v2 3/5] net-next: ethernet: mediatek: add CDM able to recognize the tag for DSA

On Tue, Mar 21, 2017 at 05:35:08PM +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> The patch adds the setup for allowing CDM can recognize these packets with
> carrying port-distinguishing tag. Otherwise, these tagging packets will be
> handled incorrectly by CDM. The setup is working out for general untag
> packets as well.
> 
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> Signed-off-by: Landen Chao <Landen.Chao@mediatek.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[toc] | [prev] | [next] | [standalone]

#1606891 — Re: [PATCH net-next v2 3/5] net-next: ethernet: mediatek: add CDM able to recognize the tag for DSA

On 03/21/2017 02:35 AM, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> The patch adds the setup for allowing CDM can recognize these packets with
> carrying port-distinguishing tag. Otherwise, these tagging packets will be
> handled incorrectly by CDM. The setup is working out for general untag
> packets as well.
> 
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> Signed-off-by: Landen Chao <Landen.Chao@mediatek.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[toc] | [prev] | [next] | [standalone]

#1605476 — [PATCH net-next v2 4/5] net-next: ethernet: mediatek: add device_node of GMAC pointing into the netdev instance

From: Sean Wang <sean.wang@mediatek.com>

the patch adds the setup of the corresponding device node of GMAC into the
netdev instance which could allow other modules such as DSA to find the
instance through the node in dt-bindings using of_find_net_device_by_node()
call.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 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 c21ed99..84b09a4 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2323,6 +2323,8 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
 	eth->netdev[id]->ethtool_ops = &mtk_ethtool_ops;
 
 	eth->netdev[id]->irq = eth->irq[0];
+	eth->netdev[id]->dev.of_node = np;
+
 	return 0;
 
 free_netdev:
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1606884 — Re: [PATCH net-next v2 4/5] net-next: ethernet: mediatek: add device_node of GMAC pointing into the netdev instance

On Tue, Mar 21, 2017 at 05:35:09PM +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> the patch adds the setup of the corresponding device node of GMAC into the
> netdev instance which could allow other modules such as DSA to find the
> instance through the node in dt-bindings using of_find_net_device_by_node()
> call.
> 
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  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 c21ed99..84b09a4 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -2323,6 +2323,8 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
>  	eth->netdev[id]->ethtool_ops = &mtk_ethtool_ops;
>  
>  	eth->netdev[id]->irq = eth->irq[0];
> +	eth->netdev[id]->dev.of_node = np;
> +

Humm, O.K. This is not obvious, until you look at of_dev_node_match()
in net-sysfs.c.

Most Ethernet drivers don't set netdev.dev.of_node. But they do call
SET_NETDEV_DEV(), which sets netdev.dev.parent.

of_dev_node_match() first looks at netdev.dev.parent->of_node, and if that
does not match, then looks at netdev.dev.of_node.

For the mtk Ethernet driver, the parent is not going to work, because
of the sub devices. So netdev.dev.of_node does need to be set.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[toc] | [prev] | [next] | [standalone]

#1606888 — Re: [PATCH net-next v2 4/5] net-next: ethernet: mediatek: add device_node of GMAC pointing into the netdev instance

On 03/21/2017 02:35 AM, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> the patch adds the setup of the corresponding device node of GMAC into the
> netdev instance which could allow other modules such as DSA to find the
> instance through the node in dt-bindings using of_find_net_device_by_node()
> call.
> 
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[toc] | [prev] | [next] | [standalone]

#1605477 — [PATCH net-next v2 2/5] net-next: dsa: add Mediatek tag RX/TX handler

From: Sean Wang <sean.wang@mediatek.com>

Add the support for the 4-bytes tag for DSA port distinguishing inserted
allowing receiving and transmitting the packet via the particular port.
The tag is being added after the source MAC address in the ethernet
header.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Signed-off-by: Landen Chao <Landen.Chao@mediatek.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/net/dsa.h  |   1 +
 net/dsa/Kconfig    |   2 +
 net/dsa/Makefile   |   1 +
 net/dsa/dsa.c      |   3 ++
 net/dsa/dsa_priv.h |   3 ++
 net/dsa/tag_mtk.c  | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 127 insertions(+)
 create mode 100644 net/dsa/tag_mtk.c

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 4e13e69..3276547 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -31,6 +31,7 @@ enum dsa_tag_protocol {
 	DSA_TAG_PROTO_EDSA,
 	DSA_TAG_PROTO_BRCM,
 	DSA_TAG_PROTO_QCA,
+	DSA_TAG_PROTO_MTK,
 	DSA_TAG_LAST,		/* MUST BE LAST */
 };
 
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 9649238..d78789b 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -31,4 +31,6 @@ config NET_DSA_TAG_TRAILER
 config NET_DSA_TAG_QCA
 	bool
 
+config NET_DSA_TAG_MTK
+	bool
 endif
diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index 31d3437..9b1d478 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -8,3 +8,4 @@ dsa_core-$(CONFIG_NET_DSA_TAG_DSA) += tag_dsa.o
 dsa_core-$(CONFIG_NET_DSA_TAG_EDSA) += tag_edsa.o
 dsa_core-$(CONFIG_NET_DSA_TAG_TRAILER) += tag_trailer.o
 dsa_core-$(CONFIG_NET_DSA_TAG_QCA) += tag_qca.o
+dsa_core-$(CONFIG_NET_DSA_TAG_MTK) += tag_mtk.o
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index b6d4f6a..617f736 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -53,6 +53,9 @@ static struct sk_buff *dsa_slave_notag_xmit(struct sk_buff *skb,
 #ifdef CONFIG_NET_DSA_TAG_QCA
 	[DSA_TAG_PROTO_QCA] = &qca_netdev_ops,
 #endif
+#ifdef CONFIG_NET_DSA_TAG_MTK
+	[DSA_TAG_PROTO_MTK] = &mtk_netdev_ops,
+#endif
 	[DSA_TAG_PROTO_NONE] = &none_ops,
 };
 
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 0706a51..2a31399 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -85,4 +85,7 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
 /* tag_qca.c */
 extern const struct dsa_device_ops qca_netdev_ops;
 
+/* tag_mtk.c */
+extern const struct dsa_device_ops mtk_netdev_ops;
+
 #endif
diff --git a/net/dsa/tag_mtk.c b/net/dsa/tag_mtk.c
new file mode 100644
index 0000000..833a9d6
--- /dev/null
+++ b/net/dsa/tag_mtk.c
@@ -0,0 +1,117 @@
+/*
+ * Mediatek DSA Tag support
+ * Copyright (C) 2017 Landen Chao <landen.chao@mediatek.com>
+ *		      Sean Wang <sean.wang@mediatek.com>
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/etherdevice.h>
+#include "dsa_priv.h"
+
+#define MTK_HDR_LEN		4
+#define MTK_HDR_RECV_SOURCE_PORT_MASK	GENMASK(2, 0)
+#define MTK_HDR_XMIT_DP_BIT_MASK	GENMASK(5, 0)
+
+static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb,
+				    struct net_device *dev)
+{
+	struct dsa_slave_priv *p = netdev_priv(dev);
+	u8 *mtk_tag;
+
+	if (skb_cow_head(skb, MTK_HDR_LEN) < 0)
+		goto out_free;
+
+	skb_push(skb, MTK_HDR_LEN);
+
+	memmove(skb->data, skb->data + MTK_HDR_LEN, 2 * ETH_ALEN);
+
+	/* Build the tag after the MAC Source Address */
+	mtk_tag = skb->data + 2 * ETH_ALEN;
+	mtk_tag[0] = 0;
+	mtk_tag[1] = (1 << p->dp->index) & MTK_HDR_XMIT_DP_BIT_MASK;
+	mtk_tag[2] = 0;
+	mtk_tag[3] = 0;
+
+	return skb;
+
+out_free:
+	kfree_skb(skb);
+	return NULL;
+}
+
+static int mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev,
+		       struct packet_type *pt, struct net_device *orig_dev)
+{
+	struct dsa_switch_tree *dst = dev->dsa_ptr;
+	struct dsa_switch *ds;
+	int port;
+	__be16 *phdr, hdr;
+
+	if (unlikely(!dst))
+		goto out_drop;
+
+	skb = skb_unshare(skb, GFP_ATOMIC);
+	if (!skb)
+		goto out;
+
+	if (unlikely(!pskb_may_pull(skb, MTK_HDR_LEN)))
+		goto out_drop;
+
+	/* The MTK header is added by the switch between src addr
+	 * and ethertype at this point, skb->data points to 2 bytes
+	 * after src addr so header should be 2 bytes right before.
+	 */
+	phdr = (__be16 *)(skb->data - 2);
+	hdr = ntohs(*phdr);
+
+	/* Remove MTK tag and recalculate checksum. */
+	skb_pull_rcsum(skb, MTK_HDR_LEN);
+
+	memmove(skb->data - ETH_HLEN,
+		skb->data - ETH_HLEN - MTK_HDR_LEN,
+		2 * ETH_ALEN);
+
+	/* This protocol doesn't support cascading multiple
+	 * switches so it's safe to assume the switch is first
+	 * in the tree.
+	 */
+	ds = dst->ds[0];
+	if (!ds)
+		goto out_drop;
+
+	/* Get source port information */
+	port = (hdr & MTK_HDR_RECV_SOURCE_PORT_MASK);
+	if (!ds->ports[port].netdev)
+		goto out_drop;
+
+	/* Update skb & forward the frame accordingly */
+	skb_push(skb, ETH_HLEN);
+
+	skb->pkt_type = PACKET_HOST;
+	skb->dev = ds->ports[port].netdev;
+	skb->protocol = eth_type_trans(skb, skb->dev);
+
+	skb->dev->stats.rx_packets++;
+	skb->dev->stats.rx_bytes += skb->len;
+
+	netif_receive_skb(skb);
+
+	return 0;
+
+out_drop:
+	kfree_skb(skb);
+out:
+	return 0;
+}
+
+const struct dsa_device_ops mtk_netdev_ops = {
+	.xmit	= mtk_tag_xmit,
+	.rcv	= mtk_tag_rcv,
+};
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1606895 — Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

On 03/21/2017 02:35 AM, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> MT7530 is a 7-ports Gigabit Ethernet Switch that could be found on
> Mediatek router platforms such as MT7623A or MT7623N platform which
> includes 7-port Gigabit Ethernet MAC and 5-port Gigabit Ethernet PHY.
> Among these ports, The port from 0 to 4 are the user ports connecting
> with the remote devices while the port 5 and 6 are the CPU ports
> connecting into Mediatek Ethernet GMAC.
> 
> For port 6, it can communicate with the CPU via Mediatek Ethernet GMAC
> through either the TRGMII or RGMII which could be controlled by phy-mode
> in the dt-bindings to specify which mode is preferred to use. And for
> port 5, only RGMII can be specified. However, currently, only port 6 is
> being supported in this DSA driver.
> 
> The driver is made with the reference to qca8k and other existing DSA
> driver. The most of the essential callbacks of the DSA are already
> support in the driver, including tag insert for user port distinguishing,
> port control, bridge offloading, STP setup and ethtool operation to allow
> DSA to model each user port into a standalone netdevice as the other DSA
> driver had done.

Overall, this looks pretty nice and clean, a few comments below

> 
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> Signed-off-by: Landen Chao <Landen.Chao@mediatek.com>
> ---

> +static void
> +mt7530_fdb_read(struct mt7530_priv *priv, struct mt7530_fdb *fdb)
> +{
> +	u32 reg[3];
> +	int i;
> +
> +	/* Read from ARL table into an array */
> +	for (i = 0; i < 3; i++) {
> +		reg[i] = mt7530_read(priv, MT7530_TSRA1 + (i * 4));
> +
> +		dev_dbg(priv->dev, "%s(%d) reg[%d]=0x%x\n",
> +			__func__, __LINE__, i, reg[i]);
> +	}
> +
> +	/* vid - 11:0 on reg[1] */
> +	fdb->vid = (reg[1] >> 0) & 0xfff;
> +	/* aging - 31:24 on reg[2] */
> +	fdb->aging = (reg[2] >> 24) & 0xff;
> +	/* portmask - 11:4 on reg[2] */
> +	fdb->port_mask = (reg[2] >> 4) & 0xff;
> +	/* mac - 31:0 on reg[0] and 31:16 on reg[1] */
> +	fdb->mac[0] = (reg[0] >> 24) & 0xff;
> +	fdb->mac[1] = (reg[0] >> 16) & 0xff;
> +	fdb->mac[2] = (reg[0] >>  8) & 0xff;
> +	fdb->mac[3] = (reg[0] >>  0) & 0xff;
> +	fdb->mac[4] = (reg[1] >> 24) & 0xff;
> +	fdb->mac[5] = (reg[1] >> 16) & 0xff;
> +	/* noarp - 3:2 on reg[2] */
> +	fdb->noarp = ((reg[2] >> 2) & 0x3) == STATIC_ENT;

Could you add some definitions for the bits and masks that you are
shifting here?

> +}
> +
> +static void
> +mt7530_fdb_write(struct mt7530_priv *priv, u16 vid,
> +		 u8 port_mask, const u8 *mac,
> +		 u8 aging, u8 type)
> +{
> +	u32 reg[3] = { 0 };
> +	int i;
> +
> +	/* vid - 11:0 on reg[1] */
> +	reg[1] |= (vid & 0xfff) << 0;
> +	/* aging - 31:25 on reg[2] */
> +	reg[2] |= (aging & 0xff) << 24;
> +	/* portmask - 11:4 on reg[2] */
> +	reg[2] |= (port_mask & 0xff) << 4;
> +	/* type - 3 indicate that entry is static wouldn't
> +	 * be aged out and 0 specified as erasing an entry
> +	 */
> +	reg[2] |= (type & 0x3) << 2;
> +	/* mac - 31:0 on reg[0] and 31:16 on reg[1] */
> +	reg[1] |= mac[5] << 16;
> +	reg[1] |= mac[4] << 24;
> +	reg[0] |= mac[3] << 0;
> +	reg[0] |= mac[2] << 8;
> +	reg[0] |= mac[1] << 16;
> +	reg[0] |= mac[0] << 24;
> +
> +	/* Wrirte array into the ARL table */
> +	for (i = 0; i < 3; i++)
> +		mt7530_write(priv, MT7530_ATA1 + (i * 4), reg[i]);
> +}

Same here.

> +
> +static int
> +mt7530_pad_clk_setup(struct dsa_switch *ds, int mode)
> +{
> +	struct mt7530_priv *priv = ds->priv;
> +	u32 ncpo1, ssc_delta, trgint, i;
> +
> +	switch (mode) {
> +	case PHY_INTERFACE_MODE_RGMII:
> +		trgint = 0;
> +		ncpo1 = 0x0c80;
> +		ssc_delta = 0x87;
> +		break;
> +	case PHY_INTERFACE_MODE_TRGMII:
> +		trgint = 1;
> +		ncpo1 = 0x1400;
> +		ssc_delta = 0x57;
> +		break;
> +	default:
> +		pr_err("xMII mode %d not supported\n", mode);
> +		return -EINVAL;
> +	}

You may be able to move this to an adjust_link callback that the PHY
library would call when the PHY gets setup and the port is finally used,
as opposed to doing this upfront during driver initialization.


> +mt7530_setup(struct dsa_switch *ds)
> +{
> +	struct mt7530_priv *priv = ds->priv;
> +	int ret, i, phy_mode;
> +	u8  cpup_mask = 0;
> +	u32 id, val;
> +	struct regmap *regmap;
> +	struct device_node *dn;
> +
> +	/* Make sure that cpu port specfied on the dt is appropriate */
> +	if (!dsa_is_cpu_port(ds, MT7530_CPU_PORT)) {
> +		dev_err(priv->dev, "port not matched with the CPU port\n");
> +		return -EINVAL;
> +	}

This is kind of a hard error, in that case, a sensible thing to do could
be issue a warning to the user telling that the configuration does not
permit the use of Mediatek tags. Your get_tag_protocol() function could
then return DSA_TAG_PROTO_NONE here instead of DSA_TAG_PROTO_MTK. It
would still allow an user to utilize the switch, and we would know what
is wrong with the configuration/board setup though.


> +
> +	/* The parent node of master_netdev which holds the common system
> +	 * controller also is the container for two GMACs nodes representing
> +	 * as two netdev instances.
> +	 */
> +	dn = ds->master_netdev->dev.of_node->parent;
> +	priv->ethernet = syscon_node_to_regmap(dn);
> +	if (IS_ERR(priv->ethernet))
> +		return PTR_ERR(priv->ethernet);
> +
> +	regmap = devm_regmap_init(ds->dev, NULL, priv,
> +				  &mt7530_regmap_config);
> +	if (IS_ERR(regmap))
> +		dev_warn(priv->dev, "phy regmap initialization failed");
> +
> +	phy_mode = of_get_phy_mode(ds->ports[ds->dst->cpu_port].dn);
> +	if (phy_mode < 0) {
> +		dev_err(priv->dev, "Can't find phy-mode for master device\n");
> +		return phy_mode;
> +	}
> +	dev_info(priv->dev, "phy-mode for master device = %x\n", phy_mode);

An adjust_link() callback which has a proper PHY device structure would
be more appropriate to look up the phy_mode rather than doing this here.

[snip]

> +	mt7530_clear(priv, MT7530_MFC, UNU_FFP_MASK);
> +
> +	/* Fabric setup for the cpu port */
> +	for (i = 0; i < MT7530_NUM_PORTS; i++)

Are not you missing an opening parenthesis here in the for() statement?

> +		if (dsa_is_cpu_port(ds, i)) {
> +			/* Enable Mediatek header mode on the cpu port */
> +			mt7530_write(priv, MT7530_PVC_P(i),
> +				     PORT_SPEC_TAG);
> +
> +			/* Setup the MAC by default for the cpu port */
> +			mt7530_write(priv, MT7530_PMCR_P(i), PMCR_CPUP_LINK);
> +
> +			/* Disable auto learning on the cpu port */
> +			mt7530_set(priv, MT7530_PSC_P(i), SA_DIS);
> +
> +			/* Unknown unicast frame fordwarding to the cpu port */
> +			mt7530_set(priv, MT7530_MFC, UNU_FFP(BIT(i)));
> +
> +			/* CPU port gets connected to all user ports of
> +			 * the switch
> +			 */
> +			mt7530_write(priv, MT7530_PCR_P(i),
> +				     PCR_MATRIX(ds->enabled_port_mask));
> +
> +			cpup_mask |= BIT(i);
> +		}
> +
> +	/* Fabric setup for the all user ports */
> +	for (i = 0; i < MT7530_NUM_PORTS; i++)
> +		if (ds->enabled_port_mask & BIT(i)) {
> +			/* Setup the MAC by default for all user ports */
> +			mt7530_write(priv, MT7530_PMCR_P(i),
> +				     PMCR_USERP_LINK);
> +
> +			/* The user port gets connected to the cpu port only */
> +			mt7530_write(priv, MT7530_PCR_P(i),
> +				     PCR_MATRIX(cpup_mask));
> +		}

This should be moved to the port_enable() function.

[snip]

> +#define wait_condition_timeout(condition, timeout)			\
> +({									\
> +	long __ret = 0;							\
> +	unsigned long toj;						\
> +	toj = jiffies + msecs_to_jiffies(timeout);			\
> +									\
> +	for ( ; !(condition) && !time_after_eq(jiffies, toj) ; )	\
> +		cond_resched();						\
> +									\
> +	if (time_after_eq(jiffies, toj))				\
> +		__ret = -ETIMEDOUT;					\
> +	__ret;								\
> +})

Can you use read*_poll_timeout() instead of this?


> +/* struct mt7530_priv -	This is the main datasructure for holding the state
> + *			of the driver
> + * @dev:		The device pointer
> + * @ds:			The pointer to the dsa core structure
> + * @bus:		The bus used for the device and built-in PHY
> + * @ethsys:		The regmap used for enabling the necessary PLL
> + * @ethernet:		The regmap used for access TRGMII-based registers
> + * @core_pwr:		The power supplied into the core
> + * @io_pwr:		The power supplied into the I/O
> + * @mcm:		Flag for distinguishing if standalone IC or module
> + *			coupling
> + * @reset:		The descriptor for GPIO line tied to its reset pin
> + * @phy_mode:		The xMII for cpu port used
> + * @ports:		Holding the state amongs ports
> + * @reg_mutex:		The lock for protecting among process accessing
> + *			registers
> + */

Kudos for using kernel doc here.
-- 
Florian

[toc] | [prev] | [next] | [standalone]

#1606898 — Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

> +static int
> +core_read_mmd_indirect(struct mt7530_priv *priv, int prtad, int devad)
> +{
> +	struct mii_bus *bus = priv->bus;
> +	int value, ret;
> +
> +	/* Write the desired MMD Devad */
> +	ret = bus->write(bus, 0, MII_MMD_CTRL, devad);
> +	if (ret < 0)
> +		goto err;
> +
> +	/* Write the desired MMD register address */
> +	ret = bus->write(bus, 0, MII_MMD_DATA, prtad);
> +	if (ret < 0)
> +		goto err;
> +
> +	/* Select the Function : DATA with no post increment */
> +	ret = bus->write(bus, 0, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
> +	if (ret < 0)
> +		goto err;

Hi Sean

This appear to be a copy of mmd_phy_indirect(). There are patches from
Russell King which made this a public function. Once Russell patches
make net-next, it would be nice to use mmd_phy_indirect(). But that
might be as a follow up patch, rather than now, depending on the order
of acceptance of the patches.

   Andrew

[toc] | [prev] | [next] | [standalone]

#1607204 — Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

> +static int
> +mt7623_trgmii_write(struct mt7530_priv *priv,  u32 reg, u32 val)
> +{
> +	int ret;
> +
> +	ret =  regmap_write(priv->ethernet, TRGMII_BASE(reg), val);
> +	if (ret < 0)
> +		dev_err(priv->dev,
> +			"failed to priv write register\n");
> +	return ret;
> +}
> +
> +static u32
> +mt7623_trgmii_read(struct mt7530_priv *priv, u32 reg)
> +{
> +	int ret;
> +	u32 val;
> +
> +	ret = regmap_read(priv->ethernet, TRGMII_BASE(reg), &val);
> +	if (ret < 0) {
> +		dev_err(priv->dev,
> +			"failed to priv read register\n");
> +		return ret;
> +	}
> +
> +	return val;
> +}

Hi Sean

These appear to be the only two accessors which use the regmap.

> +static int
> +mt7530_regmap_read(void *ctx, uint32_t reg, uint32_t *val)
> +{
> +	struct mt7530_priv *priv = (struct mt7530_priv *)ctx;
> +
> +	/* BIT(15) is used as indication for pseudo registers
> +	 * which would be translated into the general MDIO
> +	 * access to leverage the unique regmap sys interface.
> +	 */
> +	if (reg & BIT(15))
> +		*val = mdiobus_read_nested(priv->bus,
> +					   (reg & 0xf00) >> 8,
> +					   (reg & 0xff) >> 2);
> +	else
> +		*val = mt7530_read(priv, reg);
> +
> +	return 0;
> +}

.....

> +static const struct regmap_range mt7530_readable_ranges[] = {
> +	regmap_reg_range(0x0000, 0x00ac), /* Global control */
> +	regmap_reg_range(0x2000, 0x202c), /* Port Control - P0 */
> +	regmap_reg_range(0x2100, 0x212c), /* Port Control - P1 */
> +	regmap_reg_range(0x2200, 0x222c), /* Port Control - P2 */
> +	regmap_reg_range(0x2300, 0x232c), /* Port Control - P3 */
> +	regmap_reg_range(0x2400, 0x242c), /* Port Control - P4 */
> +	regmap_reg_range(0x2500, 0x252c), /* Port Control - P5 */
> +	regmap_reg_range(0x2600, 0x262c), /* Port Control - P6 */
> +	regmap_reg_range(0x30e0, 0x30f8), /* Port MAC - SYS */
> +	regmap_reg_range(0x3000, 0x3014), /* Port MAC - P0 */
> +	regmap_reg_range(0x3100, 0x3114), /* Port MAC - P1 */
> +	regmap_reg_range(0x3200, 0x3214), /* Port MAC - P2*/
> +	regmap_reg_range(0x3300, 0x3314), /* Port MAC - P3*/
> +	regmap_reg_range(0x3400, 0x3414), /* Port MAC - P4 */
> +	regmap_reg_range(0x3500, 0x3514), /* Port MAC - P5 */
> +	regmap_reg_range(0x3600, 0x3614), /* Port MAC - P6 */
> +	regmap_reg_range(0x4000, 0x40d4), /* MIB - P0 */
> +	regmap_reg_range(0x4100, 0x41d4), /* MIB - P1 */
> +	regmap_reg_range(0x4200, 0x42d4), /* MIB - P2 */
> +	regmap_reg_range(0x4300, 0x43d4), /* MIB - P3 */
> +	regmap_reg_range(0x4400, 0x44d4), /* MIB - P4 */
> +	regmap_reg_range(0x4500, 0x45d4), /* MIB - P5 */
> +	regmap_reg_range(0x4600, 0x46d4), /* MIB - P6 */
> +	regmap_reg_range(0x4fe0, 0x4ff4), /* SYS */
> +	regmap_reg_range(0x7000, 0x700c), /* SYS 2 */
> +	regmap_reg_range(0x7018, 0x7028), /* SYS 3 */
> +	regmap_reg_range(0x7800, 0x7830), /* SYS 4 */
> +	regmap_reg_range(0x7a00, 0x7a7c), /* TRGMII */
> +	regmap_reg_range(0x8000, 0x8078), /* Psedo address for Phy - P0 */
> +	regmap_reg_range(0x8100, 0x8178), /* Psedo address for Phy - P1 */
> +	regmap_reg_range(0x8200, 0x8278), /* Psedo address for Phy - P2 */
> +	regmap_reg_range(0x8300, 0x8378), /* Psedo address for Phy - P3 */
> +	regmap_reg_range(0x8400, 0x8478), /* Psedo address for Phy - P4 */
> +};

It looks like your regmap accessor are only used for 0x7a00 to 0x7a7c.

It is not clear why you even bother with a regmap. If you have it, why
not use it for all registers within the regmap?

    Andrew

[toc] | [prev] | [next] | [standalone]

#1607234 — Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

Hi Andrew,

The purpose for the regmap table registered is to 

provide a way which helps us to look up a specific 

register on the switch through regmap-debugfs.


And not all ranges of register is defined

so I only include the meaningful ones in a sparse way 

for the table.

	Sean


On Thu, 2017-03-23 at 08:22 +0100, Andrew Lunn wrote:
> > +static int
> > +mt7623_trgmii_write(struct mt7530_priv *priv,  u32 reg, u32 val)
> > +{
> > +	int ret;
> > +
> > +	ret =  regmap_write(priv->ethernet, TRGMII_BASE(reg), val);
> > +	if (ret < 0)
> > +		dev_err(priv->dev,
> > +			"failed to priv write register\n");
> > +	return ret;
> > +}
> > +
> > +static u32
> > +mt7623_trgmii_read(struct mt7530_priv *priv, u32 reg)
> > +{
> > +	int ret;
> > +	u32 val;
> > +
> > +	ret = regmap_read(priv->ethernet, TRGMII_BASE(reg), &val);
> > +	if (ret < 0) {
> > +		dev_err(priv->dev,
> > +			"failed to priv read register\n");
> > +		return ret;
> > +	}
> > +
> > +	return val;
> > +}
> 
> Hi Sean
> 
> These appear to be the only two accessors which use the regmap.
> 
> > +static int
> > +mt7530_regmap_read(void *ctx, uint32_t reg, uint32_t *val)
> > +{
> > +	struct mt7530_priv *priv = (struct mt7530_priv *)ctx;
> > +
> > +	/* BIT(15) is used as indication for pseudo registers
> > +	 * which would be translated into the general MDIO
> > +	 * access to leverage the unique regmap sys interface.
> > +	 */
> > +	if (reg & BIT(15))
> > +		*val = mdiobus_read_nested(priv->bus,
> > +					   (reg & 0xf00) >> 8,
> > +					   (reg & 0xff) >> 2);
> > +	else
> > +		*val = mt7530_read(priv, reg);
> > +
> > +	return 0;
> > +}
> 
> .....
> 
> > +static const struct regmap_range mt7530_readable_ranges[] = {
> > +	regmap_reg_range(0x0000, 0x00ac), /* Global control */
> > +	regmap_reg_range(0x2000, 0x202c), /* Port Control - P0 */
> > +	regmap_reg_range(0x2100, 0x212c), /* Port Control - P1 */
> > +	regmap_reg_range(0x2200, 0x222c), /* Port Control - P2 */
> > +	regmap_reg_range(0x2300, 0x232c), /* Port Control - P3 */
> > +	regmap_reg_range(0x2400, 0x242c), /* Port Control - P4 */
> > +	regmap_reg_range(0x2500, 0x252c), /* Port Control - P5 */
> > +	regmap_reg_range(0x2600, 0x262c), /* Port Control - P6 */
> > +	regmap_reg_range(0x30e0, 0x30f8), /* Port MAC - SYS */
> > +	regmap_reg_range(0x3000, 0x3014), /* Port MAC - P0 */
> > +	regmap_reg_range(0x3100, 0x3114), /* Port MAC - P1 */
> > +	regmap_reg_range(0x3200, 0x3214), /* Port MAC - P2*/
> > +	regmap_reg_range(0x3300, 0x3314), /* Port MAC - P3*/
> > +	regmap_reg_range(0x3400, 0x3414), /* Port MAC - P4 */
> > +	regmap_reg_range(0x3500, 0x3514), /* Port MAC - P5 */
> > +	regmap_reg_range(0x3600, 0x3614), /* Port MAC - P6 */
> > +	regmap_reg_range(0x4000, 0x40d4), /* MIB - P0 */
> > +	regmap_reg_range(0x4100, 0x41d4), /* MIB - P1 */
> > +	regmap_reg_range(0x4200, 0x42d4), /* MIB - P2 */
> > +	regmap_reg_range(0x4300, 0x43d4), /* MIB - P3 */
> > +	regmap_reg_range(0x4400, 0x44d4), /* MIB - P4 */
> > +	regmap_reg_range(0x4500, 0x45d4), /* MIB - P5 */
> > +	regmap_reg_range(0x4600, 0x46d4), /* MIB - P6 */
> > +	regmap_reg_range(0x4fe0, 0x4ff4), /* SYS */
> > +	regmap_reg_range(0x7000, 0x700c), /* SYS 2 */
> > +	regmap_reg_range(0x7018, 0x7028), /* SYS 3 */
> > +	regmap_reg_range(0x7800, 0x7830), /* SYS 4 */
> > +	regmap_reg_range(0x7a00, 0x7a7c), /* TRGMII */
> > +	regmap_reg_range(0x8000, 0x8078), /* Psedo address for Phy - P0 */
> > +	regmap_reg_range(0x8100, 0x8178), /* Psedo address for Phy - P1 */
> > +	regmap_reg_range(0x8200, 0x8278), /* Psedo address for Phy - P2 */
> > +	regmap_reg_range(0x8300, 0x8378), /* Psedo address for Phy - P3 */
> > +	regmap_reg_range(0x8400, 0x8478), /* Psedo address for Phy - P4 */
> > +};
> 
> It looks like your regmap accessor are only used for 0x7a00 to 0x7a7c.
> 
> It is not clear why you even bother with a regmap. If you have it, why
> not use it for all registers within the regmap?
> 
>     Andrew

[toc] | [prev] | [next] | [standalone]

#1607526 — Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

On 2017-03-23 09:06, Sean Wang wrote:
> Hi Andrew,
> 
> The purpose for the regmap table registered is to 
> 
> provide a way which helps us to look up a specific 
> 
> register on the switch through regmap-debugfs.
> 
> 
> And not all ranges of register is defined
> 
> so I only include the meaningful ones in a sparse way 
> 
> for the table.
I think in that case it might be nice to make regmap support optional in
order to avoid pulling in bloat on platforms that don't need it.

- Felix

[toc] | [prev] | [next] | [standalone]

#1607539 — Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

On 23/03/17 15:09, Felix Fietkau wrote:
> On 2017-03-23 09:06, Sean Wang wrote:
>> Hi Andrew,
>>
>> The purpose for the regmap table registered is to
>>
>> provide a way which helps us to look up a specific
>>
>> register on the switch through regmap-debugfs.
>>
>>
>> And not all ranges of register is defined
>>
>> so I only include the meaningful ones in a sparse way
>>
>> for the table.
> I think in that case it might be nice to make regmap support optional in
> order to avoid pulling in bloat on platforms that don't need it.
>
> - Felix
>
The 2 relevant platforms are mips/ralink and arm/mediatek. both require 
regmap for the eth_sysctl syscon if they want to utilize the mtk_soc_eth 
driver which is a prereq for mt7530. so regmap cannot be optional here.

     John


>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

[toc] | [prev] | [next] | [standalone]

#1607551 — Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

On 2017-03-23 15:25, John Crispin wrote:
> 
> 
> On 23/03/17 15:09, Felix Fietkau wrote:
>> On 2017-03-23 09:06, Sean Wang wrote:
>>> Hi Andrew,
>>>
>>> The purpose for the regmap table registered is to
>>>
>>> provide a way which helps us to look up a specific
>>>
>>> register on the switch through regmap-debugfs.
>>>
>>>
>>> And not all ranges of register is defined
>>>
>>> so I only include the meaningful ones in a sparse way
>>>
>>> for the table.
>> I think in that case it might be nice to make regmap support optional in
>> order to avoid pulling in bloat on platforms that don't need it.
>>
>> - Felix
>>
> The 2 relevant platforms are mips/ralink and arm/mediatek. both require 
> regmap for the eth_sysctl syscon if they want to utilize the mtk_soc_eth 
> driver which is a prereq for mt7530. so regmap cannot be optional here.
Makes sense, thanks.

- Felix

[toc] | [prev] | [next] | [standalone]

#1608188 — Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

On Thu, Mar 23, 2017 at 04:06:56PM +0800, Sean Wang wrote:
> Hi Andrew,
> 
> The purpose for the regmap table registered is to 
> 
> provide a way which helps us to look up a specific 
> 
> register on the switch through regmap-debugfs.
> 
> 
> And not all ranges of register is defined
> 
> so I only include the meaningful ones in a sparse way 
> 
> for the table.

Many of there registers can be dumped using existing tools.  You can
dump the port registers using ethtool -r, if you implement
get_regs/get_regs_len in your driver. mii-tool can dump the PHY
registers.

What you cannot see are global registers, so i can understand the
usage of regmap-debugfs. However, i have a hard time with you only
actually using the regmap for get/set for one small subset of
registers. Either you need to use regmap to get/set all registers, or
you remove it from the mainline driver, and keep it as a private patch
which you use for your development work. For the Marvell driver we
have an out of tree patch which exports a log of information via
debugfs.

	Andrew

[toc] | [prev] | [next] | [standalone]

#1608426 — Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

Hi Sean

> +	regmap = devm_regmap_init(ds->dev, NULL, priv,
> +				  &mt7530_regmap_config);
> +	if (IS_ERR(regmap))
> +		dev_warn(priv->dev, "phy regmap initialization failed");
> +

Shouldn't this be a fatal error? If you keep going when there is an
error, what happens when you actually try to use the regmap?

> +	phy_mode = of_get_phy_mode(ds->ports[ds->dst->cpu_port].dn);
> +	if (phy_mode < 0) {
> +		dev_err(priv->dev, "Can't find phy-mode for master device\n");
> +		return phy_mode;
> +	}
> +	dev_info(priv->dev, "phy-mode for master device = %x\n", phy_mode);

dev_dbg? 

> +
> +	id = mt7530_read(priv, MT7530_CREV);
> +	id >>= CHIP_NAME_SHIFT;
> +	if (id != MT7530_ID)
> +		return -ENODEV;

It might be helpful to say what ID has been found, if it is not the
supported ID.

	Andrew

[toc] | [prev] | [next] | [standalone]

#1608487 — Re: [PATCH net-next v2 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

On Tue, Mar 21, 2017 at 05:35:10PM +0800, sean.wang@mediatek.com wrote:

Hi Sean

> +	/* Lower Tx Driving */
> +	for (i = 0 ; i < 6 ; i++)

Could MT7530_CPU_PORT be used here? 

> +		mt7530_write(priv, MT7530_TRGMII_TD_ODT(i),
> +			     TD_DM_DRVP(8) | TD_DM_DRVN(8));
> +
> +	/* Setup MT7530 core clock */
> +	if (!trgint) {
> +		/* Disable MT7530 core clock */
> +		core_clear(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
> +
> +		/* Disable MT7530 PLL, since phy_device has not yet been
> +		 * created when this function is called. So we provide
> +		 * core_write_mmd_indirect to complete this function
> +		 */
> +		core_write_mmd_indirect(priv,
> +					CORE_GSWPLL_GRP1,
> +					MDIO_MMD_VEND2,
> +					0);
> +
> +		/* Setup MT7530 core clock into 500Mhz */
> +		core_write(priv, CORE_GSWPLL_GRP2,
> +			   RG_GSWPLL_POSDIV_500M(1) |
> +			   RG_GSWPLL_FBKDIV_500M(25));
> +
> +		/* Enable MT7530 PLL */
> +		core_write(priv, CORE_GSWPLL_GRP1,
> +			   RG_GSWPLL_EN_PRE |
> +			   RG_GSWPLL_POSDIV_200M(2) |
> +			   RG_GSWPLL_FBKDIV_200M(32));
> +
> +		/* Enable MT7530 core clock */
> +		core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
> +	}
> +
> +	/* Setup the MT7530 TRGMII Tx Clock */
> +	core_set(priv, CORE_TRGMII_GSW_CLK_CG, REG_GSWCK_EN);
> +	core_write(priv, CORE_PLL_GROUP5, RG_LCDDS_PCW_NCPO1(ncpo1));
> +	core_write(priv, CORE_PLL_GROUP6, RG_LCDDS_PCW_NCPO0(0));
> +	core_write(priv, CORE_PLL_GROUP10, RG_LCDDS_SSC_DELTA(ssc_delta));
> +	core_write(priv, CORE_PLL_GROUP11, RG_LCDDS_SSC_DELTA1(ssc_delta));
> +	core_write(priv, CORE_PLL_GROUP4,
> +		   RG_SYSPLL_DDSFBK_EN | RG_SYSPLL_BIAS_EN |
> +		   RG_SYSPLL_BIAS_LPF_EN);
> +	core_write(priv, CORE_PLL_GROUP2,
> +		   RG_SYSPLL_EN_NORMAL | RG_SYSPLL_VODEN |
> +		   RG_SYSPLL_POSDIV(1));
> +	core_write(priv, CORE_PLL_GROUP7,
> +		   RG_LCDDS_PCW_NCPO_CHG | RG_LCCDS_C(3) |
> +		   RG_LCDDS_PWDB | RG_LCDDS_ISO_EN);
> +	core_set(priv, CORE_TRGMII_GSW_CLK_CG,
> +		 REG_GSWCK_EN | REG_TRGMIICK_EN);
> +
> +	if (!trgint)
> +		for (i = 0 ; i < 5 ; i++)

Why only 5 here? All other similar loops are to 6.  Replacing 5 with a
#define might help make this more readable.

> +			mt7530_rmw(priv, MT7530_TRGMII_RD(i),
> +				   RD_TAP_MASK, RD_TAP(16));
> +	else
> +		mt7623_trgmii_set(priv, GSW_INTF_MODE, INTF_MODE_TRGMII);
> +
> +	return 0;
> +}
> +
> +static int
> +mt7623_pad_clk_setup(struct dsa_switch *ds)
> +{
> +	struct mt7530_priv *priv = ds->priv;
> +	int i;
> +
> +	for (i = 0 ; i < 6; i++)

MT7530_CPU_PORT?

> +
> +/* Registers to mac forward conrol for unknown frames */

/conrol/control

> +
> +/* Registor for port control */

Register

> +/* Regiser for TOP signal control */

Register

> +/* struct mt7530_priv -	This is the main datasructure for holding the state

data structure

> + *			of the driver
> + * @dev:		The device pointer
> + * @ds:			The pointer to the dsa core structure
> + * @bus:		The bus used for the device and built-in PHY
> + * @ethsys:		The regmap used for enabling the necessary PLL
> + * @ethernet:		The regmap used for access TRGMII-based registers
> + * @core_pwr:		The power supplied into the core
> + * @io_pwr:		The power supplied into the I/O
> + * @mcm:		Flag for distinguishing if standalone IC or module
> + *			coupling
> + * @reset:		The descriptor for GPIO line tied to its reset pin
> + * @phy_mode:		The xMII for cpu port used
> + * @ports:		Holding the state amongs ports

among

	Andrew

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web