Re: [PATCH net-next v3 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch
> I want to see each ports behind the bridge as independent ethernet > devices. If I understand RTFM only devlink may expose this > configuration. DSA and switchdev in general gives you a linux interface per user switch port. So you can see each interface, get stats, ethtools, mii-tool, etc per interface. devlink is not needed for this. Andrew
Re: [PATCH net-next v3 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch
2017-05-26 8:54 GMT+03:00 Florian Fainelli : > On May 24, 2017 4:17:39 AM PDT, "Andrey Jr. Melnikov" > wrote: >>In gmane.linux.kernel sean.w...@mediatek.com wrote: >>> From: Sean Wang >> >>> 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. >> >>What about JUMBO frames and large MTU support? devlink support? > > We don't have a ndo_change_mtu callback implemented for DSA network devices > but that is probably how we should do it. > Regarding devlink, Andrew added basic support for it but unlike mlxsw it is > currently of mild interest. Do you have something specific in mind with > devlink? I want to see each ports behind the bridge as independent ethernet devices. If I understand RTFM only devlink may expose this configuration.
Re: [PATCH net-next v3 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch
On May 24, 2017 4:17:39 AM PDT, "Andrey Jr. Melnikov" wrote: >In gmane.linux.kernel sean.w...@mediatek.com wrote: >> From: Sean Wang > >> 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. > >What about JUMBO frames and large MTU support? devlink support? We don't have a ndo_change_mtu callback implemented for DSA network devices but that is probably how we should do it. Regarding devlink, Andrew added basic support for it but unlike mlxsw it is currently of mild interest. Do you have something specific in mind with devlink? -- Florian
Re: [PATCH net-next v3 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch
2017-05-24 16:20 GMT+03:00 Andrew Lunn : >> What about JUMBO frames and large MTU support? devlink support? > > To get the driver merged, it is better to start small, a minimal set > of features. Other features can be added later. If these features are > important to you, please feel free to submit patches. There is no documentation from mediatek about this feature. Driver from SDK v4.x contains only switching 19'th bit in GDMA1_FWD_CFG and nothing more. So, how to write patches? Peeking random bits in random SOC config registers?
Re: [PATCH net-next v3 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch
> What about JUMBO frames and large MTU support? devlink support? To get the driver merged, it is better to start small, a minimal set of features. Other features can be added later. If these features are important to you, please feel free to submit patches. Andrew
Re: [PATCH net-next v3 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch
In gmane.linux.kernel sean.w...@mediatek.com wrote: > From: Sean Wang > 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. What about JUMBO frames and large MTU support? devlink support?
Re: [PATCH net-next v3 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch
Hi Sean, [auto build test ERROR on net-next/master] url: https://github.com/0day-ci/linux/commits/sean-wang-mediatek-com/net-next-dsa-add-Mediatek-MT7530-support/20170330-135532 config: m68k-allmodconfig (attached as .config) compiler: m68k-linux-gcc (GCC) 4.9.0 reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=m68k All errors (new ones prefixed by >>): In file included from net//dsa/tag_mtk.c:16:0: net//dsa/dsa_priv.h:51:16: warning: 'struct dsa_switch' declared inside parameter list struct dsa_port *dport, int port); ^ net//dsa/dsa_priv.h:51:16: warning: its scope is only this definition or declaration, which is probably not what you want net//dsa/dsa_priv.h:54:39: warning: 'struct dsa_switch' declared inside parameter list int dsa_cpu_port_ethtool_setup(struct dsa_switch *ds); ^ net//dsa/dsa_priv.h:55:42: warning: 'struct dsa_switch' declared inside parameter list void dsa_cpu_port_ethtool_restore(struct dsa_switch *ds); ^ net//dsa/dsa_priv.h:59:36: warning: 'struct dsa_switch' declared inside parameter list void dsa_slave_mii_bus_init(struct dsa_switch *ds); ^ net//dsa/dsa_priv.h:62:8: warning: 'struct dsa_switch' declared inside parameter list int port, const char *name); ^ net//dsa/dsa_priv.h:70:41: warning: 'struct dsa_switch' declared inside parameter list int dsa_switch_register_notifier(struct dsa_switch *ds); ^ net//dsa/dsa_priv.h:71:44: warning: 'struct dsa_switch' declared inside parameter list void dsa_switch_unregister_notifier(struct dsa_switch *ds); ^ net//dsa/tag_mtk.c: In function 'mtk_tag_xmit': >> net//dsa/tag_mtk.c:38:26: error: dereferencing pointer to incomplete type mtk_tag[1] = (1 << p->dp->index) & MTK_HDR_XMIT_DP_BIT_MASK; ^ net//dsa/tag_mtk.c: In function 'mtk_tag_rcv': net//dsa/tag_mtk.c:85:10: error: dereferencing pointer to incomplete type ds = dst->ds[0]; ^ net//dsa/tag_mtk.c:91:9: error: dereferencing pointer to incomplete type if (!ds->ports[port].netdev) ^ net//dsa/tag_mtk.c:98:15: error: dereferencing pointer to incomplete type skb->dev = ds->ports[port].netdev; ^ vim +38 net//dsa/tag_mtk.c 2faad9d7 Sean Wang 2017-03-29 22 static struct sk_buff *mtk_tag_xmit(struct sk_buff *skb, 2faad9d7 Sean Wang 2017-03-29 23 struct net_device *dev) 2faad9d7 Sean Wang 2017-03-29 24 { 2faad9d7 Sean Wang 2017-03-29 25 struct dsa_slave_priv *p = netdev_priv(dev); 2faad9d7 Sean Wang 2017-03-29 26 u8 *mtk_tag; 2faad9d7 Sean Wang 2017-03-29 27 2faad9d7 Sean Wang 2017-03-29 28 if (skb_cow_head(skb, MTK_HDR_LEN) < 0) 2faad9d7 Sean Wang 2017-03-29 29 goto out_free; 2faad9d7 Sean Wang 2017-03-29 30 2faad9d7 Sean Wang 2017-03-29 31 skb_push(skb, MTK_HDR_LEN); 2faad9d7 Sean Wang 2017-03-29 32 2faad9d7 Sean Wang 2017-03-29 33 memmove(skb->data, skb->data + MTK_HDR_LEN, 2 * ETH_ALEN); 2faad9d7 Sean Wang 2017-03-29 34 2faad9d7 Sean Wang 2017-03-29 35 /* Build the tag after the MAC Source Address */ 2faad9d7 Sean Wang 2017-03-29 36 mtk_tag = skb->data + 2 * ETH_ALEN; 2faad9d7 Sean Wang 2017-03-29 37 mtk_tag[0] = 0; 2faad9d7 Sean Wang 2017-03-29 @38 mtk_tag[1] = (1 << p->dp->index) & MTK_HDR_XMIT_DP_BIT_MASK; 2faad9d7 Sean Wang 2017-03-29 39 mtk_tag[2] = 0; 2faad9d7 Sean Wang 2017-03-29 40 mtk_tag[3] = 0; 2faad9d7 Sean Wang 2017-03-29 41 2faad9d7 Sean Wang 2017-03-29 42 return skb; 2faad9d7 Sean Wang 2017-03-29 43 2faad9d7 Sean Wang 2017-03-29 44 out_free: 2faad9d7 Sean Wang 2017-03-29 45 kfree_skb(skb); 2faad9d7 Sean Wang 2017-03-29 46 return NULL; :: The code at line 38 was first introduced by commit :: 2faad9d71e4c0544e3cf43b24439517c95df301f net-next: dsa: add Mediatek tag RX/TX handler :: TO: Sean Wang :: CC: 0day robot --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH net-next v3 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch
Hi Sean, [auto build test ERROR on net-next/master] url: https://github.com/0day-ci/linux/commits/sean-wang-mediatek-com/net-next-dsa-add-Mediatek-MT7530-support/20170330-135532 config: arm-allmodconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705 reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=arm All errors (new ones prefixed by >>): In file included from net/dsa/tag_mtk.c:16:0: net/dsa/dsa_priv.h:50:30: warning: 'struct dsa_switch' declared inside parameter list will not be visible outside of this definition or declaration int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev, ^~ net/dsa/dsa_priv.h:54:39: warning: 'struct dsa_switch' declared inside parameter list will not be visible outside of this definition or declaration int dsa_cpu_port_ethtool_setup(struct dsa_switch *ds); ^~ net/dsa/dsa_priv.h:55:42: warning: 'struct dsa_switch' declared inside parameter list will not be visible outside of this definition or declaration void dsa_cpu_port_ethtool_restore(struct dsa_switch *ds); ^~ net/dsa/dsa_priv.h:59:36: warning: 'struct dsa_switch' declared inside parameter list will not be visible outside of this definition or declaration void dsa_slave_mii_bus_init(struct dsa_switch *ds); ^~ net/dsa/dsa_priv.h:61:29: warning: 'struct dsa_switch' declared inside parameter list will not be visible outside of this definition or declaration int dsa_slave_create(struct dsa_switch *ds, struct device *parent, ^~ net/dsa/dsa_priv.h:70:41: warning: 'struct dsa_switch' declared inside parameter list will not be visible outside of this definition or declaration int dsa_switch_register_notifier(struct dsa_switch *ds); ^~ net/dsa/dsa_priv.h:71:44: warning: 'struct dsa_switch' declared inside parameter list will not be visible outside of this definition or declaration void dsa_switch_unregister_notifier(struct dsa_switch *ds); ^~ net/dsa/tag_mtk.c: In function 'mtk_tag_xmit': >> net/dsa/tag_mtk.c:38:26: error: dereferencing pointer to incomplete type >> 'struct dsa_port' mtk_tag[1] = (1 << p->dp->index) & MTK_HDR_XMIT_DP_BIT_MASK; ^~ net/dsa/tag_mtk.c: In function 'mtk_tag_rcv': >> net/dsa/tag_mtk.c:85:10: error: dereferencing pointer to incomplete type >> 'struct dsa_switch_tree' ds = dst->ds[0]; ^~ >> net/dsa/tag_mtk.c:91:9: error: dereferencing pointer to incomplete type >> 'struct dsa_switch' if (!ds->ports[port].netdev) ^~ vim +91 net/dsa/tag_mtk.c 2faad9d7 Sean Wang 2017-03-29 32 2faad9d7 Sean Wang 2017-03-29 33 memmove(skb->data, skb->data + MTK_HDR_LEN, 2 * ETH_ALEN); 2faad9d7 Sean Wang 2017-03-29 34 2faad9d7 Sean Wang 2017-03-29 35 /* Build the tag after the MAC Source Address */ 2faad9d7 Sean Wang 2017-03-29 36 mtk_tag = skb->data + 2 * ETH_ALEN; 2faad9d7 Sean Wang 2017-03-29 37 mtk_tag[0] = 0; 2faad9d7 Sean Wang 2017-03-29 @38 mtk_tag[1] = (1 << p->dp->index) & MTK_HDR_XMIT_DP_BIT_MASK; 2faad9d7 Sean Wang 2017-03-29 39 mtk_tag[2] = 0; 2faad9d7 Sean Wang 2017-03-29 40 mtk_tag[3] = 0; 2faad9d7 Sean Wang 2017-03-29 41 2faad9d7 Sean Wang 2017-03-29 42 return skb; 2faad9d7 Sean Wang 2017-03-29 43 2faad9d7 Sean Wang 2017-03-29 44 out_free: 2faad9d7 Sean Wang 2017-03-29 45 kfree_skb(skb); 2faad9d7 Sean Wang 2017-03-29 46 return NULL; 2faad9d7 Sean Wang 2017-03-29 47 } 2faad9d7 Sean Wang 2017-03-29 48 2faad9d7 Sean Wang 2017-03-29 49 static int mtk_tag_rcv(struct sk_buff *skb, struct net_device *dev, 2faad9d7 Sean Wang 2017-03-29 50 struct packet_type *pt, struct net_device *orig_dev) 2faad9d7 Sean Wang 2017-03-29 51 { 2faad9d7 Sean Wang 2017-03-29 52 struct dsa_switch_tree *dst = dev->dsa_ptr; 2faad9d7 Sean Wang 2017-03-29 53 struct dsa_switch *ds; 2faad9d7 Sean Wang 2017-03-29 54 int port; 2faad9d7 Sean Wang 2017-03-29 55 __be16 *phdr, hdr; 2faad9d7 Sean Wang 2017-03-29 56 2faad9d7 Sean Wang 2017-03-29 57 if (unlikely(!dst)) 2faad9d7 Sean Wang 2017-03-29 58 goto out_drop; 2faad9d7 Sean Wang 2017-03-29 59 2faad9d7 Sean Wang 2017-03-29 60 skb = skb_unshare(skb, GFP_ATOMIC); 2faad9d7 Sean Wang 2017-03-29 61 if (!skb) 2faad9d7 Sean Wang 2017-03-29 62 goto out; 2faad9d7 Sean Wang 2017-03-29 63 2faad9d7 Sean Wan
Re: [PATCH net-next v3 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch
> +static struct mt7530_priv *lpriv; Hi Sean Why do you need this global variable? What if somebody has two switches connected? > +static void mt7530_port_disable(struct dsa_switch *ds, int port, > + struct phy_device *phy); > +static int mt7530_cpu_port_enable(struct mt7530_priv *priv, > + int port); It is better to move the functions around than have forward declarations. > +static u32 > +_mt7530_read(u32 reg) > +{ > + struct mt7530_priv *priv = lpriv; > + struct mii_bus *bus = priv->bus; > + u32 val; > + > + mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED); > + > + val = mt7530_mii_read(priv, reg); > + > + mutex_unlock(&bus->mdio_lock); > + > + return val; > +} > + > +static u32 > +mt7530_read(struct mt7530_priv *priv, u32 reg) > +{ > + return _mt7530_read(reg); > +} So here you would pass priv to _mt7530_read(). > +static int > +mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp) > +{ > + u32 val; > + int ret; > + > + /* Set the command operating upon the MAC address entries */ > + val = ATC_BUSY | ATC_MAT(0) | cmd; > + mt7530_write(priv, MT7530_ATC, val); > + > + ret = readx_poll_timeout(_mt7530_read, MT7530_ATC, val, > + !(val & ATC_BUSY), 20, 2); This is where your need for lpriv comes, you can only pass a single argument. But you can work around it. readx_poll_timeout is a #define. So there is no type check for _mt7530_read, or the arg passed to it. So pass a struct containing the address and priv. I think you can then kill of your global lpriv. Andrew