Re: [PATCH V2 03/12] net-next: mediatek: add embedded switch driver (ESW)
On 26/02/16 07:24, John Crispin wrote: > > Hi, > > would the series be ok if we just dropped those parts and then have a > driver in the kernel that wont do much with the out of tree patches ? > > the problem here is that on one side people complain about vendors not > sending code upstream. once they start being a good citizen and provide > funding to send stuff upstream the feedback tends to be very bad as seen > here. I agree with David here, the feedback from Andrew is very constructive, you just don't like the feedback you are being given, which is a different thing. You can't always get a 12 series patches adding a new driver accepted after second try, look at all the recent submissions that occured, it took 5-6-7 maybe more submissions until things were in a shape where they could be merged. If for your next submission you get the feedback that switchdev/DSA is deprecated, and something new needs to be used, then I would agree that feedback is not acceptable, I doubt this will be the case unless we wait another 10 years to get these patches out. > we are planning on doing a DSA driver but one step at a time. this > kind of feedback will inevitably lead to vendors doing second thoughts > of upstream contributions. If you are planning on a DSA driver, which sounds like a good plan, then maybe drop the integrated switch parts for now, keep it as a local set of patches for your testing, and just get the basic CPU Ethernet MAC driver to work for data movement, so that part gets in, and later on, when your DSA driver is ready, that's one less thing to take care of. They ultimately are logically spearated drivers if you use DSA, a little less if you use switchdev. -- Florian
Re: [PATCH V2 03/12] net-next: mediatek: add embedded switch driver (ESW)
From: Andrew Lunn Date: Fri, 26 Feb 2016 18:05:45 +0100 > I think it is great a vendor is providing funding to get code > upstream. However, that code needs to conform with current kernel > architecture and design philosophy. > > We as a community also need to be consistent. We have recently push > back on Microchip with there LAN9352 who want to do something very > similar, introduce the MAC and a very dumb switch driver. They are now > looking at what it means to do a DSA driver. There is also talk of > writing a DSA driver for the ks8995 family. > > As David said recently, a year ago this probably would of been > accepted. But now, switches need to be DSA or switchdev. +1
Re: [PATCH V2 03/12] net-next: mediatek: add embedded switch driver (ESW)
From: Felix Fietkau Date: Fri, 26 Feb 2016 17:25:38 +0100 > In my opinion, leaving this part out does not make much sense and > neither does deferring the entire patch series until we have a > switchdev/DSA capable driver. This is just a starting point, which will > be turned into a proper driver with the right APIs later. I disagree, and we want people to concentrate on writing proper switchdev/DSA drivers. People like Andrew have offered to help in any way possible to make this as easy as possible, so please take this seriously. I would have accepted your arguments a year ago when we didn't have the right infrastructure, but now we do and there is no real excuse to submit partial or bastardized drivers for these kinds of hardware anymore Thanks in advance for your understanding, and I'm plan to stand very firm on this.
Re: [PATCH V2 03/12] net-next: mediatek: add embedded switch driver (ESW)
From: John Crispin Date: Fri, 26 Feb 2016 16:24:47 +0100 > the problem here is that on one side people complain about vendors not > sending code upstream. once they start being a good citizen and provide > funding to send stuff upstream the feedback tends to be very bad as seen > here. The feedback is not bad, on the contrary it is very positive and people like Andrew want to help people like you write proper switch drivers. If you were ignored, or rejected purely on the grounds of coding style issues, that would be "very bad" feedback.
Re: [PATCH V2 03/12] net-next: mediatek: add embedded switch driver (ESW)
From: Andrew Lunn Date: Fri, 26 Feb 2016 16:18:13 +0100 > On Fri, Feb 26, 2016 at 03:21:35PM +0100, John Crispin wrote: >> The ESW is found in many of the old 100mbit MIPS based SoCs. it has 5 >> external ports, 1 cpu port and 1 further port that the internal HW >> offloading engine connects to. >> >> This driver is very basic and only provides basic init and irq support. >> The SoC and switch core both have support for a special tag making DSA >> support possible. > > Hi Crispin > > There was recently a discussion about adding switches without using > DSA or switchdev. It was pretty much decided we would not accept such > drivers. > > Sorry +1
Re: [PATCH V2 03/12] net-next: mediatek: add embedded switch driver (ESW)
On Fri, Feb 26, 2016 at 05:25:38PM +0100, Felix Fietkau wrote: > On 2016-02-26 16:18, Andrew Lunn wrote: > > On Fri, Feb 26, 2016 at 03:21:35PM +0100, John Crispin wrote: > >> The ESW is found in many of the old 100mbit MIPS based SoCs. it has 5 > >> external ports, 1 cpu port and 1 further port that the internal HW > >> offloading engine connects to. > >> > >> This driver is very basic and only provides basic init and irq support. > >> The SoC and switch core both have support for a special tag making DSA > >> support possible. > > > > Hi Crispin > > > > There was recently a discussion about adding switches without using > > DSA or switchdev. It was pretty much decided we would not accept such > > drivers. > For exactly this reason, the code does not provide any non-standard API > for allowing the user to configure the switch. The hardware needs to be > programmed with some defaults for the driver to be functional (since the > switch logic is built into the SoC). > > > In my opinion, leaving this part out does not make much sense and > neither does deferring the entire patch series until we have a > switchdev/DSA capable driver. This is just a starting point, which will > be turned into a proper driver with the right APIs later. You are however introducing APIs which become things you need to support. Your device tree binding for example. The device tree maintainers consider this a stable API you need to maintain forever. Can you guarantee that you can maintain this binding, and also support the DSA binding? How messy is it going to make your code supporting two bindings? You currently have one netdev interface for five switch ports. When you have a DSA driver, you have 5 additional netdev interfaces, one per port, and your original interface is now unusable. Isn't that an API change. You need to find incremental steps towards a switchdev/DSA driver which are not going to hinder you in the long run. Andrew
Re: [PATCH V2 03/12] net-next: mediatek: add embedded switch driver (ESW)
> the problem here is that on one side people complain about vendors not > sending code upstream. once they start being a good citizen and provide > funding to send stuff upstream the feedback tends to be very bad as seen > here. we are planning on doing a DSA driver but one step at a time. this > kind of feedback will inevitably lead to vendors doing second thoughts > of upstream contributions. I think it is great a vendor is providing funding to get code upstream. However, that code needs to conform with current kernel architecture and design philosophy. We as a community also need to be consistent. We have recently push back on Microchip with there LAN9352 who want to do something very similar, introduce the MAC and a very dumb switch driver. They are now looking at what it means to do a DSA driver. There is also talk of writing a DSA driver for the ks8995 family. As David said recently, a year ago this probably would of been accepted. But now, switches need to be DSA or switchdev. Andrew
Re: [PATCH V2 03/12] net-next: mediatek: add embedded switch driver (ESW)
On 2016-02-26 16:18, Andrew Lunn wrote: > On Fri, Feb 26, 2016 at 03:21:35PM +0100, John Crispin wrote: >> The ESW is found in many of the old 100mbit MIPS based SoCs. it has 5 >> external ports, 1 cpu port and 1 further port that the internal HW >> offloading engine connects to. >> >> This driver is very basic and only provides basic init and irq support. >> The SoC and switch core both have support for a special tag making DSA >> support possible. > > Hi Crispin > > There was recently a discussion about adding switches without using > DSA or switchdev. It was pretty much decided we would not accept such > drivers. For exactly this reason, the code does not provide any non-standard API for allowing the user to configure the switch. The hardware needs to be programmed with some defaults for the driver to be functional (since the switch logic is built into the SoC). In my opinion, leaving this part out does not make much sense and neither does deferring the entire patch series until we have a switchdev/DSA capable driver. This is just a starting point, which will be turned into a proper driver with the right APIs later. - Felix
Re: [PATCH V2 03/12] net-next: mediatek: add embedded switch driver (ESW)
On 26/02/2016 16:18, Andrew Lunn wrote: > On Fri, Feb 26, 2016 at 03:21:35PM +0100, John Crispin wrote: >> The ESW is found in many of the old 100mbit MIPS based SoCs. it has 5 >> external ports, 1 cpu port and 1 further port that the internal HW >> offloading engine connects to. >> >> This driver is very basic and only provides basic init and irq support. >> The SoC and switch core both have support for a special tag making DSA >> support possible. > > Hi Crispin > > There was recently a discussion about adding switches without using > DSA or switchdev. It was pretty much decided we would not accept such > drivers. > > Sorry > > Andrew > > ___ > Linux-mediatek mailing list > linux-media...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek > Hi, would the series be ok if we just dropped those parts and then have a driver in the kernel that wont do much with the out of tree patches ? the problem here is that on one side people complain about vendors not sending code upstream. once they start being a good citizen and provide funding to send stuff upstream the feedback tends to be very bad as seen here. we are planning on doing a DSA driver but one step at a time. this kind of feedback will inevitably lead to vendors doing second thoughts of upstream contributions. John
Re: [PATCH V2 03/12] net-next: mediatek: add embedded switch driver (ESW)
On Fri, Feb 26, 2016 at 03:21:35PM +0100, John Crispin wrote: > The ESW is found in many of the old 100mbit MIPS based SoCs. it has 5 > external ports, 1 cpu port and 1 further port that the internal HW > offloading engine connects to. > > This driver is very basic and only provides basic init and irq support. > The SoC and switch core both have support for a special tag making DSA > support possible. Hi Crispin There was recently a discussion about adding switches without using DSA or switchdev. It was pretty much decided we would not accept such drivers. Sorry Andrew
[PATCH V2 03/12] net-next: mediatek: add embedded switch driver (ESW)
The ESW is found in many of the old 100mbit MIPS based SoCs. it has 5 external ports, 1 cpu port and 1 further port that the internal HW offloading engine connects to. This driver is very basic and only provides basic init and irq support. The SoC and switch core both have support for a special tag making DSA support possible. I have managed to work out all the names of the registers of the ESW, however the init for the phys uses lots of voodoo magic which is based on the SDK driver code. It will be hard to find out the real names for the phy registers and their bits. Signed-off-by: John Crispin --- drivers/net/ethernet/mediatek/esw_rt3050.c | 642 drivers/net/ethernet/mediatek/esw_rt3050.h | 21 + 2 files changed, 663 insertions(+) create mode 100644 drivers/net/ethernet/mediatek/esw_rt3050.c create mode 100644 drivers/net/ethernet/mediatek/esw_rt3050.h diff --git a/drivers/net/ethernet/mediatek/esw_rt3050.c b/drivers/net/ethernet/mediatek/esw_rt3050.c new file mode 100644 index 000..8da4bec --- /dev/null +++ b/drivers/net/ethernet/mediatek/esw_rt3050.c @@ -0,0 +1,642 @@ +/* This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License + * + * 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. + * + * Copyright (C) 2009-2016 John Crispin + * Copyright (C) 2009-2016 Felix Fietkau + * Copyright (C) 2013-2016 Michael Lee + */ + +#include +#include +#include +#include +#include +#include + +#include "mtk_eth_soc.h" + +/* HW limitations for this switch: + * - No large frame support (PKT_MAX_LEN at most 1536) + * - Can't have untagged vlan and tagged vlan on one port at the same time, + * though this might be possible using the undocumented PPE. + */ + +#define RT305X_ESW_REG_ISR 0x00 +#define RT305X_ESW_REG_IMR 0x04 +#define RT305X_ESW_REG_FCT00x08 +#define RT305X_ESW_REG_PFC10x14 +#define RT305X_ESW_REG_ATS 0x24 +#define RT305X_ESW_REG_ATS00x28 +#define RT305X_ESW_REG_ATS10x2c +#define RT305X_ESW_REG_ATS20x30 +#define RT305X_ESW_REG_PVIDC(_n) (0x40 + 4 * (_n)) +#define RT305X_ESW_REG_VLANI(_n) (0x50 + 4 * (_n)) +#define RT305X_ESW_REG_VMSC(_n)(0x70 + 4 * (_n)) +#define RT305X_ESW_REG_POA 0x80 +#define RT305X_ESW_REG_FPA 0x84 +#define RT305X_ESW_REG_SOCPC 0x8c +#define RT305X_ESW_REG_POC00x90 +#define RT305X_ESW_REG_POC10x94 +#define RT305X_ESW_REG_POC20x98 +#define RT305X_ESW_REG_SGC 0x9c +#define RT305X_ESW_REG_STRT0xa0 +#define RT305X_ESW_REG_PCR00xc0 +#define RT305X_ESW_REG_PCR10xc4 +#define RT305X_ESW_REG_FPA20xc8 +#define RT305X_ESW_REG_FCT20xcc +#define RT305X_ESW_REG_SGC20xe4 +#define RT305X_ESW_REG_P0LED 0xa4 +#define RT305X_ESW_REG_P1LED 0xa8 +#define RT305X_ESW_REG_P2LED 0xac +#define RT305X_ESW_REG_P3LED 0xb0 +#define RT305X_ESW_REG_P4LED 0xb4 +#define RT305X_ESW_REG_PXPC(_x)(0xe8 + (4 * _x)) +#define RT305X_ESW_REG_P1PC0xec +#define RT305X_ESW_REG_P2PC0xf0 +#define RT305X_ESW_REG_P3PC0xf4 +#define RT305X_ESW_REG_P4PC0xf8 +#define RT305X_ESW_REG_P5PC0xfc + +#define RT305X_ESW_LED_LINK0 +#define RT305X_ESW_LED_100M1 +#define RT305X_ESW_LED_DUPLEX 2 +#define RT305X_ESW_LED_ACTIVITY3 +#define RT305X_ESW_LED_COLLISION 4 +#define RT305X_ESW_LED_LINKACT 5 +#define RT305X_ESW_LED_DUPLCOLL6 +#define RT305X_ESW_LED_10MACT 7 +#define RT305X_ESW_LED_100MACT 8 +/* Additional led states not in datasheet: */ +#define RT305X_ESW_LED_BLINK 10 +#define RT305X_ESW_LED_ON 12 + +#define RT305X_ESW_LINK_S 25 +#define RT305X_ESW_DUPLEX_S9 +#define RT305X_ESW_SPD_S 0 + +#define RT305X_ESW_PCR0_WT_NWAY_DATA_S 16 +#define RT305X_ESW_PCR0_WT_PHY_CMD BIT(13) +#define RT305X_ESW_PCR0_CPU_PHY_REG_S 8 + +#define RT305X_ESW_PCR1_WT_DONEBIT(0) + +#define RT305X_ESW_ATS_TIMEOUT (5 * HZ) +#define RT305X_ESW_PHY_TIMEOUT (5 * HZ) + +#define RT305X_ESW_PVIDC_PVID_M0xfff +#define RT305X_ESW_PVIDC_PVID_S12 + +#define RT305X_ESW_VLANI_VID_M 0xfff +#define RT305X_ESW_VLANI_VID_S 12 + +#define RT305X_ESW_VMSC_MSC_M 0xff +#define RT305X_ESW_VMSC_MSC_S 8 + +#define RT305X_ESW_SOCPC_DISUN