Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v3 net-next 01/11] net: stmmac: prepare dma op mode config for multiple queues
From: Jan Kiszka @ 2017-05-08 12:36 UTC (permalink / raw)
  To: Joao Pinto, Andy Shevchenko
  Cc: David S. Miller, Giuseppe CAVALLARO, Alexandre TORGUE, netdev,
	Linux Kernel Mailing List
In-Reply-To: <54cd5a46-eed3-bce7-5280-c5e95957e8e6@synopsys.com>

On 2017-05-08 14:02, Joao Pinto wrote:
> Às 12:56 PM de 5/8/2017, Andy Shevchenko escreveu:
>> On Mon, May 8, 2017 at 2:40 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>>> Às 12:34 PM de 5/8/2017, Andy Shevchenko escreveu:
>>>> On Mon, May 8, 2017 at 1:42 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>>>>> Às 11:12 AM de 5/8/2017, Andy Shevchenko escreveu:
>>>>>> On Mon, May 8, 2017 at 12:54 PM, Joao Pinto <Joao.Pinto@synopsys.com> wrote:
>>>>>>> Às 10:36 AM de 5/8/2017, Andy Shevchenko escreveu:
>>
>>>>
>>>> [   44.374161] stmmac_dvr_probe <<< 0 0
>>>>
>>>
>>> Ok, so this is the cause of the problem. The driver is geting 0 for real RX and
>>> TX queues.
>>>
>>> Your setup uses standard DT parsing from stmmac_platform or a custom one?
>>>
>>> If you are using stmmac_probe_config_dt():
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n363&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=fJQj7RiT2sksJYOAZ9VSJUDnxPR7RlE6Fw_cTV0_Mqc&s=KhdAPUtP0twDkibE89cLYs8JjnxEvBgav5uf08WL_e8&e= 
>>>
>>> You will find a function named stmmac_mtl_setup() being called:
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_torvalds_linux.git_tree_drivers_net_ethernet_stmicro_stmmac_stmmac-5Fplatform.c-23n492&d=DwIFaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=fJQj7RiT2sksJYOAZ9VSJUDnxPR7RlE6Fw_cTV0_Mqc&s=rTxn0fwdudwq9XAquH60xNHN538KBQ6_n4wODdLoyA0&e= 
>>>
>>> In this function, the number of RX and TX queues is being set to 1 by default.
>>
>> Ah-ha, now I know how it's happened.
>> You forget to update all setup() hooks in PCI bus driver :-)
>>
>> I will prepare a fix.
>> Just tell me should I put Fixes tag or not? And if yes, what commit
>> should I refer to?
>>
> 
> Great, you can use this commit:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c?id=26d6851fd24ed5d88580d66b4c8384947d5ca29b
> 
> Thanks!
> 
> Joao
> 

Perfect, looking forward to try out a fix. Thanks, folks!

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply

* Re: [ISSUE: sky2 - rx error] Link stops working under heavy traffic load connected to a mv88e6176
From: Andrew Lunn @ 2017-05-08 12:38 UTC (permalink / raw)
  To: Rafa Corvillo; +Cc: Stephen Hemminger, netdev
In-Reply-To: <59105EA3.9030203@aoifes.com>

> >static unsigned sky2_get_rx_threshold(struct sky2_port *sky2)
> >{
> >         unsigned size;
> >
> >         /* Space needed for frame data + headers rounded up */
> >         size = roundup(sky2->netdev->mtu + ETH_HLEN + VLAN_HLEN, 8);
> >
> >         /* Stopping point for hardware truncation */
> >         return (size - 8) / sizeof(u32);
> >}
> >
> >This is not going to be big enough for a frame with a DSA header.
> >
> 
> Then, would be a good fix add 8 bytes to the size variable in this function?

Yes. Also look at the transmit code, is there again a limit based on
the MTU.

> Settings for marvell:
>         Supported ports: [ TP ]
>         Supported link modes:   10baseT/Half 10baseT/Full
>                                 100baseT/Half 100baseT/Full
>                                 1000baseT/Half 1000baseT/Full
>         Supported pause frame use: No
>         Supports auto-negotiation: Yes
>         Advertised link modes:  10baseT/Half 10baseT/Full
>                                 100baseT/Half 100baseT/Full
>                                 1000baseT/Half 1000baseT/Full
>         Advertised pause frame use: No
>         Advertised auto-negotiation: No
>         Speed: 1000Mb/s
>         Duplex: Full
>         Port: Twisted Pair
>         PHYAD: 0
>         Transceiver: internal
>         Auto-negotiation: on
>         MDI-X: Unknown
>         Supports Wake-on: pg
>         Wake-on: d
>         Current message level: 0x000000ff (255)
>                                drv probe link timer ifdown ifup
> rx_err tx_err
>         Link detected: yes
> 

So this suggests there is a real PHY there, and it is
auto-negotiating.

What we cannot see is the status for the PHY it connects to. But since
this PHY has established a link, the other PHY is probably O.K. It is
just a bit unsafe, since you are relying on reset behaviour. There is
nothing in software configuring the second PHY to make it
auto-negotiate.

	Andrew

^ permalink raw reply

* RE: [PATCH v4 net-next 04/10] net/ncsi: Ethtool operation to get NCSI topology
From: David Laight @ 2017-05-08 12:40 UTC (permalink / raw)
  To: 'Gavin Shan'
  Cc: Stephen Hemminger, netdev@vger.kernel.org, joe@perches.com,
	kubakici@wp.pl, f.fainelli@gmail.com, davem@davemloft.net
In-Reply-To: <20170508001955.GA5787@gwshan>

From: Gavin Shan
> Sent: 08 May 2017 01:20
...
> >Why 16 bits?
> >You are just making life hard for the compiler and possibly generating
> >random padding.
> >
> 
> It's because there are 256 NCSI channels to maximal degree. So 16-bits
> is the minial data width to hold it in signed format. Yes, I think
> __s32 would be better in this case. However, I would like to discard
> the negotiation mechanism in next respin.

Just because the domain of a value fits in 16 bits doesn't mean
that a 16bit type is appropriate.

It is generally much better to use 32 (aka machine word) sized
items unless you have an array or are trying to fit a lot of
items into a small memory area.

	David

^ permalink raw reply

* [PATCH V2 0/1] net: cdc_ncm: Fix TX zero padding
From: Jim Baxter @ 2017-05-08 12:49 UTC (permalink / raw)
  To: linux-usb, netdev, linux-kernel, Oliver Neukum; +Cc: jim_baxter

Analysis
--------

The zero padding that is added to NTB's does not zero
the memory correctly.
This happens because the skb_put called within the memset in
the line:
memset(skb_put(skb_out, ctx->tx_max - skb_out->len),
       0, ctx->tx_max - skb_out->len);
causes the value of skb_out->len to be modified during
the two uses of it within the above line.
This causes non-zeroed data at the end of skb_out.

This issue was found when connecting between an ARM
Sabre SD Host platform and a test box that was
dropping the NDP's due to the non zeroed memory being
identified as an error.

Solution
--------

To resolve this I have cached the value of
ctx->tx_max - skb_out->len before the memset operation.

----

V1: Sent to linux-use for review.
V2: Added netdev mailing list as it was missed for V1.

Jim Baxter (1):
  net: cdc_ncm: Fix TX zero padding

 drivers/net/usb/cdc_ncm.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH V2 1/1] net: cdc_ncm: Fix TX zero padding
From: Jim Baxter @ 2017-05-08 12:49 UTC (permalink / raw)
  To: linux-usb, netdev, linux-kernel, Oliver Neukum; +Cc: jim_baxter
In-Reply-To: <1494247797-1732-1-git-send-email-jim_baxter@mentor.com>

The zero padding that is added to NTB's does
not zero the memory correctly.
This is because the skb_put modifies the value
of skb_out->len which results in the memset
command not setting any memory to zero as
(ctx->tx_max - skb_out->len) == 0.

I have resolved this by storing the size of
the memory to be zeroed before the skb_put
and using this in the memset call.

Signed-off-by: Jim Baxter <jim_baxter@mentor.com>
Reviewed-by: Bjørn Mork <bjorn@mork.no>
---

V1: Sent to linux-use for review.
V2: Added netdev mailing list as it was missed for V1.

 drivers/net/usb/cdc_ncm.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index f317984..e2a48d7 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1087,6 +1087,7 @@ struct sk_buff *
 	u16 n = 0, index, ndplen;
 	u8 ready2send = 0;
 	u32 delayed_ndp_size;
+	size_t padding_count;
 
 	/* When our NDP gets written in cdc_ncm_ndp(), then skb_out->len gets updated
 	 * accordingly. Otherwise, we should check here.
@@ -1243,11 +1244,13 @@ struct sk_buff *
 	 * a ZLP after full sized NTBs.
 	 */
 	if (!(dev->driver_info->flags & FLAG_SEND_ZLP) &&
-	    skb_out->len > ctx->min_tx_pkt)
-		memset(skb_put(skb_out, ctx->tx_max - skb_out->len), 0,
-		       ctx->tx_max - skb_out->len);
-	else if (skb_out->len < ctx->tx_max && (skb_out->len % dev->maxpacket) == 0)
+	    skb_out->len > ctx->min_tx_pkt) {
+		padding_count = ctx->tx_max - skb_out->len;
+		memset(skb_put(skb_out, padding_count), 0, padding_count);
+	} else if (skb_out->len < ctx->tx_max &&
+		   (skb_out->len % dev->maxpacket) == 0) {
 		*skb_put(skb_out, 1) = 0;	/* force short packet */
+	}
 
 	/* set final frame length */
 	nth16 = (struct usb_cdc_ncm_nth16 *)skb_out->data;
-- 
1.9.1

^ permalink raw reply related

* Re: [RFC iproute2 0/8] RDMA tool
From: Knut Omang @ 2017-05-08 13:04 UTC (permalink / raw)
  To: Leon Romanovsky, Jiri Pirko
  Cc: Jiri Benc, Stephen Hemminger, Doug Ledford, Jiri Pirko,
	Ariel Almog, Dennis Dalessandro, Ram Amrani, Bart Van Assche,
	Sagi Grimberg, Jason Gunthorpe, Christoph Hellwig, Or Gerlitz,
	Linux RDMA, Linux Netdev
In-Reply-To: <20170507063329.GL22833@mtr-leonro.local>

On Sun, 2017-05-07 at 09:33 +0300, Leon Romanovsky wrote:
> On Sat, May 06, 2017 at 12:48:26PM +0200, Jiri Pirko wrote:
> > Fri, May 05, 2017 at 03:17:54PM CEST, leon@kernel.org wrote:
> > >On Fri, May 05, 2017 at 08:54:57AM +0200, Jiri Benc wrote:
> > >> On Thu,  4 May 2017 21:02:08 +0300, Leon Romanovsky wrote:
> > >> > In order to close object model, ensure reuse of existing code and make this
> > >> > tool usable from day one, we decided to implement wrappers over legacy sysfs
> > >> > prior to implementing netlink functionality. As a nice bonus, it will allow
> > >> > to use this tool with old kernels too.
> > >>
> > >> This sounds wrong. We don't support legacy ioctl interface for the 'ip'
> > >> command, either. I think rdma should be converted to netlink first and
> > >> the new tool should only use netlink.
> > >
> > >RDMA in slightly different situation than "ip" tool was. "ip" was implemented
> > >when tools like ifconfig existed. It allowed to old and new systems to be
> > >configured to some degree. In RDMA community, there are no similar tools like
> > >"ifconfig". Implementation in netlink-only interface will leave old systems without
> > >common tool at all.
> > >
> > >As an upstream-oriented person, I personally fine with that, but anyway would
> > >like to get wider agreement/disagreement on that, before removing sysfs
> > >parsing logic from the rdmatool.
> >
> > I tend to agree with Jiri Benc. I fear that supporting sysfs + netlink
> > api later on for the same things will make the code unnecessary complex.
> > Also, the legacy sysfs will most likely stay there forever so there will
> > be no actual motivation to port the existing things to the new netlink
> > api.
> >
> > For the prototyping purposes, I belive that what you did makes perfect
> > sense. But for the actual mergable version, my feeling is that we need
> > to strictly stick with new netlink rdma interface and just forget about
> > the old sysfs one. Distros would have to backport the new kernel
> > rdma netlink api.
> 
> Thanks,
> It looks like that most of the comments are in favor of netlink-only
> solution.

Leon, I like the thought bw comp support. After all this is a user level tool so it should
be possible to make a clean implementation that makes the old stuff easy to remove 
at some point. It will also attract users much sooner than if they have to have 
their own if-then-else logic around everything to be able to support old and new.

> > Yes, this will be little bit more painful at the beginning, but in the
> > long run, I believe it will save some severe headaches.
> >

IMHO, some headache will be there anyway, just a matter of how how far out it gets.

Knut

^ permalink raw reply

* RE: [PATCH net-next 2/5] phy: micrel: add Microchip KSZ 9477 Switch PHY support
From: Woojung.Huh @ 2017-05-08 13:31 UTC (permalink / raw)
  To: andrew; +Cc: f.fainelli, vivien.didelot, netdev, davem, UNGLinuxDriver
In-Reply-To: <20170508001207.GE27709@lunn.ch>

> > >> +}, {
> > >> +     .phy_id         = PHY_ID_KSZ9477,
> > >> +     .phy_id_mask    = MICREL_PHY_ID_MASK,
> > >> +     .name           = "Microchip KSZ9477",
> > >> +     .features       = PHY_GBIT_FEATURES,
> > >> +     .flags          = PHY_HAS_MAGICANEG,
> > >
> > >Is this magic still used anywhere? I could not find anything.
> > >
> > Hi Andrew,
> >
> > Really no usage in define and flags.
> > Will double check and remove it.
> 
> Hi Woojung
> 
> I just created a patch to remove PHY_HAS_MAGICANEG from the kernel.
> Once net-next has reopened when the merge window closes, i will submit
> it. So please don't add it here.
Hi Andrew,

Thanks you. I'll remove it.

Woojung

^ permalink raw reply

* RE: [PATCH net-next 3/5] dsa: add DSA switch driver for Microchip KSZ9477
From: Woojung.Huh @ 2017-05-08 13:47 UTC (permalink / raw)
  To: f.fainelli, andrew, vivien.didelot; +Cc: netdev, davem, UNGLinuxDriver
In-Reply-To: <34dd65c8-21e1-9cdf-ab47-89134743ea7d@gmail.com>

Hi Florian,

> > The KSZ9477 is a fully integrated layer 2, managed, 7 ports GigE switch
> > with numerous advanced features. 5 ports incorporate
> > 10/100/1000 Mbps PHYs. The other 2 ports have interfaces that can be
> > configured as SGMII, RGMII, MII or RMII.
> > Either of these may connect directly to a host processor or to
> > an external PHY. The SGMII port may interface to a fiber optic transceiver.
> >
> > This driver currently supports vlan, fdb, mdb & mirror dsa switch operations.
> >
> > Signed-off-by: Woojung Huh <Woojung.Huh@microchip.com>
> > ---
> 
> Overall, this looks really nice and clean, there are a few minor nits,
> most of them being that busy polling loop have no timeout to make sure
> they terminate in a finite amount of time.
Thanks for your full review on this patch.
Will add limit in the loop and update per your other comments below.
Added my comment in few section.

> For VLAN programming, can you describe how the CPU port works? Is it
> similar to B53 where we explictly need to have the CPU port be part of a
> VLAN entry (including tagged/untagged attribute) or is it similar to
> Marvell where it seems to be smarter and requires little to no
> configuration?
Can you explain little bit this question? I understand that how this chip handles
VLAN in CPU port. Am I understanding your question?

> > +
> > +static u64 mib_value[TOTAL_SWITCH_COUNTER_NUM];
> 
> This probably needs to be driver instance specific and in the ksz_device
> structure?
> 
> > +
> > +static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, int set)
> > +{
> 
> bool set?
> 
> > +	u8 data;
> > +
> > +	ksz_read8(dev, addr, &data);
> > +	if (set)
> > +		data |= bits;
> > +	else
> > +		data &= ~bits;
> > +	ksz_write8(dev, addr, data);
> > +}
> > +
> > +static void ksz_cfg32(struct ksz_device *dev, u32 addr, u32 bits, int set)
> > +{
> 
> Same thing
> 
> > +	u32 data;
> > +
> > +	ksz_read32(dev, addr, &data);
> > +	if (set)
> > +		data |= bits;
> > +	else
> > +		data &= ~bits;
> > +	ksz_write32(dev, addr, data);
> > +}
> > +
> > +static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
> > +			 int set)
> > +{
> > +	u32 addr;
> > +	u8 data;
> > +
> > +	addr = PORT_CTRL_ADDR(port, offset);
> > +	ksz_read8(dev, addr, &data);
> > +
> > +	if (set)
> > +		data |= bits;
> > +	else
> > +		data &= ~bits;
> > +
> > +	ksz_write8(dev, addr, data);
> > +}
> > +
> > +static void ksz_port_cfg32(struct ksz_device *dev, int port, int offset,
> > +			   u32 bits, int set)
> > +{
> > +	u32 addr;
> > +	u32 data;
> > +
> > +	addr = PORT_CTRL_ADDR(port, offset);
> > +	ksz_read32(dev, addr, &data);
> > +
> > +	if (set)
> > +		data |= bits;
> > +	else
> > +		data &= ~bits;
> > +
> > +	ksz_write32(dev, addr, data);
> > +}
> > +
> > +static void get_vlan_table(struct dsa_switch *ds, u16 vid, u32 *vlan_table)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	u8 data;
> > +
> > +	ksz_write16(dev, REG_SW_VLAN_ENTRY_INDEX__2, vid &
> VLAN_INDEX_M);
> > +	ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_READ | VLAN_START);
> > +	/* wait to be cleared */
> > +	data = 0;
> > +	do {
> > +		ksz_read8(dev, REG_SW_VLAN_CTRL, &data);
> > +	} while (data & VLAN_START);
> 
> You need to put an upper boundary on how this can take, potentially
> spinning forever is not very safe.
> 
> > +
> > +	ksz_read32(dev, REG_SW_VLAN_ENTRY__4, &vlan_table[0]);
> > +	ksz_read32(dev, REG_SW_VLAN_ENTRY_UNTAG__4,
> &vlan_table[1]);
> > +	ksz_read32(dev, REG_SW_VLAN_ENTRY_PORTS__4, &vlan_table[2]);
> > +
> > +	ksz_write8(dev, REG_SW_VLAN_CTRL, 0);
> > +}
> > +
> > +static void set_vlan_table(struct dsa_switch *ds, u16 vid, u32 *vlan_table)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	u8 data;
> > +
> > +	ksz_write32(dev, REG_SW_VLAN_ENTRY__4, vlan_table[0]);
> > +	ksz_write32(dev, REG_SW_VLAN_ENTRY_UNTAG__4, vlan_table[1]);
> > +	ksz_write32(dev, REG_SW_VLAN_ENTRY_PORTS__4, vlan_table[2]);
> > +
> > +	ksz_write16(dev, REG_SW_VLAN_ENTRY_INDEX__2, vid &
> VLAN_INDEX_M);
> > +	ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_START | VLAN_WRITE);
> > +
> > +	do {
> > +		ksz_read8(dev, REG_SW_VLAN_CTRL, &data);
> > +	} while (data & VLAN_START);
> 
> Same here.
> 
> > +
> > +	ksz_write8(dev, REG_SW_VLAN_CTRL, 0);
> > +
> > +	/* update vlan cache table */
> > +	dev->vlan_cache[vid].table[0] = vlan_table[0];
> > +	dev->vlan_cache[vid].table[1] = vlan_table[1];
> > +	dev->vlan_cache[vid].table[2] = vlan_table[2];
> > +}
> > +
> > +static void read_table(struct dsa_switch *ds, u32 *table)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +
> > +	ksz_read32(dev, REG_SW_ALU_VAL_A, &table[0]);
> > +	ksz_read32(dev, REG_SW_ALU_VAL_B, &table[1]);
> > +	ksz_read32(dev, REG_SW_ALU_VAL_C, &table[2]);
> > +	ksz_read32(dev, REG_SW_ALU_VAL_D, &table[3]);
> > +}
> > +
> > +static void write_table(struct dsa_switch *ds, u32 *table)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +
> > +	ksz_write32(dev, REG_SW_ALU_VAL_A, table[0]);
> > +	ksz_write32(dev, REG_SW_ALU_VAL_B, table[1]);
> > +	ksz_write32(dev, REG_SW_ALU_VAL_C, table[2]);
> > +	ksz_write32(dev, REG_SW_ALU_VAL_D, table[3]);
> > +}
> > +
> > +static int ksz_reset_switch(struct dsa_switch *ds)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	u8 data8;
> > +	u16 data16;
> > +	u32 data32;
> > +
> > +	/* reset switch */
> > +	ksz_cfg(dev, REG_SW_OPERATION, SW_RESET, true);
> > +
> > +	/* turn off SPI DO Edge select */
> > +	ksz_read8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, &data8);
> > +	data8 &= ~SPI_AUTO_EDGE_DETECTION;
> > +	ksz_write8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, data8);
> > +
> > +	/* default configuration */
> > +	ksz_read8(dev, REG_SW_LUE_CTRL_1, &data8);
> > +	data8 = SW_AGING_ENABLE | SW_LINK_AUTO_AGING |
> > +	      SW_SRC_ADDR_FILTER | SW_FLUSH_STP_TABLE |
> SW_FLUSH_MSTP_TABLE;
> > +	ksz_write8(dev, REG_SW_LUE_CTRL_1, data8);
> > +
> > +	/* disable interrupts */
> > +	ksz_write32(dev, REG_SW_INT_MASK__4, SWITCH_INT_MASK);
> > +	ksz_write32(dev, REG_SW_PORT_INT_MASK__4, 0x7F);
> > +	ksz_read32(dev, REG_SW_PORT_INT_STATUS__4, &data32);
> > +
> > +	/* set broadcast storm protection 10% rate */
> > +	ksz_read16(dev, REG_SW_MAC_CTRL_2, &data16);
> > +	data16 &= ~BROADCAST_STORM_RATE;
> > +	data16 |= (BROADCAST_STORM_VALUE *
> BROADCAST_STORM_PROT_RATE) / 100;
> > +	ksz_write16(dev, REG_SW_MAC_CTRL_2, data16);
> > +
> > +	memset(dev->vlan_cache, 0, sizeof(*dev->vlan_cache) * dev-
> >num_vlans);
> > +
> > +	return 0;
> > +}
> > +
> > +static int ksz_setup_ports(struct dsa_switch *ds)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	u8 data8;
> > +	u16 data16;
> > +	int i;
> > +
> > +	ds->num_ports = dev->port_cnt;
> > +
> > +	for (i = 0; i < ds->num_ports; i++) {
> > +		if (dsa_is_cpu_port(ds, i)) {
> > +			dev->cpu_port = i;
> > +			/* enable tag tail for host port */
> > +			ksz_port_cfg(dev, i, REG_PORT_CTRL_0,
> > +				     PORT_TAIL_TAG_ENABLE, true);
> > +		}
> 
> With the exception of this configuration above, which is specific to the
> CPU port
> 
> > +
> > +		ksz_port_cfg(dev, i, REG_PORT_MAC_CTRL_1,
> > +			     PORT_BACK_PRESSURE, true);
> > +		ksz_port_cfg(dev, i, REG_PORT_CTRL_0,
> > +			     PORT_FORCE_TX_FLOW_CTRL |
> PORT_FORCE_RX_FLOW_CTRL,
> > +			     false);
> > +
> > +		/* configure MAC to 1G & RGMII mode */
> > +		ksz_pread8(dev, i, REG_PORT_XMII_CTRL_1, &data8);
> > +		data8 |= PORT_RGMII_ID_EG_ENABLE;
> > +		data8 &= ~PORT_MII_NOT_1GBIT;
> > +		data8 &= ~PORT_MII_SEL_M;
> > +		data8 |= PORT_RGMII_SEL;
> > +		ksz_pwrite8(dev, i, REG_PORT_XMII_CTRL_1, data8);
> > +
> > +		/* clear pending interrupts */
> > +		ksz_pread16(dev, i, REG_PORT_PHY_INT_ENABLE, &data16);
> 
> All of this configuration should probably be moved to the port_enable
> method.
Will test whether need any settings before enabling port.

> NB: in the original DSA drivers, there was no per-port setup function,
> so we should think about migrating existing drivers to per-port
> port_enable instead of keeping this here.
Agree.

> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int ksz_setup(struct dsa_switch *ds)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	int ret = 0;
> > +
> > +	dev->vlan_cache = devm_kmalloc_array(dev->dev,
> > +					     sizeof(struct vlan_table),
> > +					     dev->num_vlans, GFP_KERNEL);
> > +	if (!dev->vlan_cache)
> > +		return -ENOMEM;
> > +
> > +	ret = ksz_reset_switch(ds);
> > +	if (ret) {
> > +		dev_err(ds->dev, "failed to reset switch\n");
> > +		return ret;
> > +	}
> > +
> > +	/* accept packet up to 2000bytes */
> > +	ksz_cfg(dev, REG_SW_MAC_CTRL_1, SW_LEGAL_PACKET_DISABLE,
> true);
> > +
> > +	ksz_setup_ports(ds);
> > +
> > +	ksz_cfg(dev, REG_SW_MAC_CTRL_1, MULTICAST_STORM_DISABLE,
> true);
> > +
> > +	/* queue based egress rate limit */
> > +	ksz_cfg(dev, REG_SW_MAC_CTRL_5,
> SW_OUT_RATE_LIMIT_QUEUE_BASED, true);
> > +
> > +	/* start switch */
> > +	ksz_cfg(dev, REG_SW_OPERATION, SW_START, true);
> > +
> > +	return 0;
> > +}
> > +
> > +static enum dsa_tag_protocol ksz_get_tag_protocol(struct dsa_switch
> *ds)
> > +{
> > +	return DSA_TAG_PROTO_KSZ;
> > +}
> > +
> > +static int ksz_phy_read16(struct dsa_switch *ds, int addr, int reg)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	u16 val = 0;
> > +
> > +	ksz_pread16(dev, addr, 0x100 + (reg << 1), &val);
> > +
> > +	return (int)val;
> 
> The cast is probably not necessary here.
> 
> > +}
> > +
> > +static int ksz_phy_write16(struct dsa_switch *ds, int addr, int reg, u16 val)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +
> > +	ksz_pwrite16(dev, addr, 0x100 + (reg << 1), val);
> > +
> > +	return 0;
> > +}
> > +
> > +static int ksz_enable_port(struct dsa_switch *ds, int port,
> > +			   struct phy_device *phy)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +
> > +	ksz_port_cfg(dev, port, REG_PORT_CTRL_0, PORT_MAC_LOOPBACK,
> false);
> > +
> > +	/* set back pressure */
> > +	ksz_port_cfg(dev, port, REG_PORT_MAC_CTRL_1,
> PORT_BACK_PRESSURE, true);
> > +
> > +	/* set flow control */
> > +	ksz_port_cfg(dev, port, REG_PORT_CTRL_0,
> > +		     PORT_FORCE_TX_FLOW_CTRL |
> PORT_FORCE_RX_FLOW_CTRL, true);
> > +
> > +	/* enable boradcast storm limit */
> > +	ksz_port_cfg(dev, port, P_BCAST_STORM_CTRL,
> PORT_BROADCAST_STORM, true);
> > +
> > +	/* disable DiffServ priority */
> > +	ksz_port_cfg(dev, port, P_PRIO_CTRL,
> PORT_DIFFSERV_PRIO_ENABLE, false);
> > +
> > +	/* replace priority */
> > +	ksz_port_cfg(dev, port, REG_PORT_MRI_MAC_CTRL,
> > +		     PORT_USER_PRIO_CEILING, false);
> > +	ksz_port_cfg32(dev, port, REG_PORT_MTI_QUEUE_CTRL_0__4,
> > +		       MTI_PVID_REPLACE, false);
> > +
> > +	/* enable 802.1p priority */
> > +	ksz_port_cfg(dev, port, P_PRIO_CTRL, PORT_802_1P_PRIO_ENABLE,
> true);
> > +
> > +	return 0;
> > +}
> > +
> > +static void ksz_disable_port(struct dsa_switch *ds, int port,
> > +			     struct phy_device *phy)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +
> > +	/* there is no port disable */
> > +	ksz_port_cfg(dev, port, REG_PORT_CTRL_0, PORT_MAC_LOOPBACK,
> true);
> > +}
> > +
> > +static int ksz_sset_count(struct dsa_switch *ds)
> > +{
> > +	return TOTAL_SWITCH_COUNTER_NUM;
> > +}
> > +
> > +static void ksz_get_strings(struct dsa_switch *ds, int port, uint8_t *buf)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
> > +		memcpy(buf + (i * ETH_GSTRING_LEN), mib_names[i].string,
> > +		       ETH_GSTRING_LEN);
> 
> Parenthesis around i * ETH_GSTRINGS_LEN should not be needed.
> 
> > +	}
> > +}
> > +
> > +static void ksz_get_ethtool_stats(struct dsa_switch *ds, int port,
> > +				  uint64_t *buf)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	int i;
> > +	u32 data;
> > +
> > +	mutex_lock(&dev->stats_mutex);
> > +
> > +	for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
> > +		data = MIB_COUNTER_READ;
> > +		data |= ((mib_names[i].index & 0xFF) <<
> MIB_COUNTER_INDEX_S);
> > +		ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4,
> data);
> > +
> > +		do {
> > +			ksz_pread32(dev, port,
> REG_PORT_MIB_CTRL_STAT__4,
> > +				    &data);
> > +			usleep_range(10, 50);
> > +		} while (data & MIB_COUNTER_READ);
> 
> Same thing here we need to bound this with a timeout count of some sort.
> 
> > +
> > +		/* count resets upon read */
> > +		ksz_pread32(dev, port, REG_PORT_MIB_DATA, &data);
> > +
> > +		mib_value[i] += (uint64_t)data;
> > +		buf[i] = mib_value[i];
> > +	}
> > +
> > +	mutex_unlock(&dev->stats_mutex);
> > +}
> > +
> > +static void ksz_port_stp_state_set(struct dsa_switch *ds, int port, u8
> state)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	u8 data;
> > +
> > +	ksz_pread8(dev, port, P_STP_CTRL, &data);
> > +	switch (state) {
> > +	case BR_STATE_DISABLED:
> > +		data &= ~(PORT_TX_ENABLE | PORT_RX_ENABLE);
> > +		data |= PORT_LEARN_DISABLE;
> > +		break;
> > +	case BR_STATE_LISTENING:
> > +		data &= ~PORT_TX_ENABLE;
> > +		data |= (PORT_RX_ENABLE | PORT_LEARN_DISABLE);
> > +		break;
> > +	case BR_STATE_LEARNING:
> > +		data &= ~(PORT_TX_ENABLE | PORT_LEARN_DISABLE);
> > +		data |= PORT_RX_ENABLE;
> > +		break;
> > +	case BR_STATE_FORWARDING:
> > +		data &= ~PORT_LEARN_DISABLE;
> > +		data |= (PORT_TX_ENABLE | PORT_RX_ENABLE);
> > +		break;
> > +	case BR_STATE_BLOCKING:
> > +		data &= ~(PORT_TX_ENABLE | PORT_RX_ENABLE);
> > +		data |= PORT_LEARN_DISABLE;
> > +		break;
> > +	default:
> > +		dev_err(ds->dev, "invalid STP state: %d\n", state);
> > +		return;
> > +	}
> 
> It would be clearer to move the masking of all bits outside of the
> switch()/case statement, have the switch()/case just assign values, and
> do the logial oring at the end of this statement.
> 
> > +
> > +	ksz_pwrite8(dev, port, P_STP_CTRL, data);
> > +}
> > +
> > +static void ksz_port_fast_age(struct dsa_switch *ds, int port)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	u8 data8;
> > +
> > +	ksz_read8(dev, REG_SW_LUE_CTRL_1, &data8);
> > +	data8 |= SW_FAST_AGING;
> > +	ksz_write8(dev, REG_SW_LUE_CTRL_1, data8);
> > +
> > +	data8 &= ~SW_FAST_AGING;
> > +	ksz_write8(dev, REG_SW_LUE_CTRL_1, data8);
> > +}
> > +
> > +static int ksz_port_vlan_filtering(struct dsa_switch *ds, int port, bool flag)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +
> > +	if (flag) {
> > +		ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL,
> > +			     PORT_VLAN_LOOKUP_VID_0, true);
> > +		ksz_cfg32(dev, REG_SW_QM_CTRL__4,
> UNICAST_VLAN_BOUNDARY, true);
> > +		ksz_cfg(dev, REG_SW_LUE_CTRL_0, SW_VLAN_ENABLE,
> true);
> > +	} else {
> > +		ksz_cfg(dev, REG_SW_LUE_CTRL_0, SW_VLAN_ENABLE,
> false);
> > +		ksz_cfg32(dev, REG_SW_QM_CTRL__4,
> UNICAST_VLAN_BOUNDARY, false);
> > +		ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL,
> > +			     PORT_VLAN_LOOKUP_VID_0, false);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int ksz_port_vlan_prepare(struct dsa_switch *ds, int port,
> > +				 const struct switchdev_obj_port_vlan *vlan,
> > +				 struct switchdev_trans *trans)
> > +{
> > +	/* nothing needed */
> > +
> > +	return 0;
> > +}
> > +
> > +static void ksz_port_vlan_add(struct dsa_switch *ds, int port,
> > +			      const struct switchdev_obj_port_vlan *vlan,
> > +			      struct switchdev_trans *trans)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	u32 vlan_table[3];
> > +	u16 vid;
> > +	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
> > +
> > +	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
> > +		get_vlan_table(ds, vid, vlan_table);
> > +
> > +		vlan_table[0] = VLAN_VALID | (vid & VLAN_FID_M);
> > +		if (untagged)
> > +			vlan_table[1] |= BIT(port);
> > +		else
> > +			vlan_table[1] &= ~BIT(port);
> > +		vlan_table[1] &= ~(BIT(dev->cpu_port));
> > +
> > +		vlan_table[2] |= BIT(port) | BIT(dev->cpu_port);
> > +
> > +		set_vlan_table(ds, vid, vlan_table);
> > +
> > +		/* change PVID */
> > +		if (vlan->flags & BRIDGE_VLAN_INFO_PVID)
> > +			ksz_pwrite16(dev, port, REG_PORT_DEFAULT_VID,
> vid);
> > +	}
> > +}
> > +
> > +static int ksz_port_vlan_del(struct dsa_switch *ds, int port,
> > +			     const struct switchdev_obj_port_vlan *vlan)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
> > +	u32 vlan_table[3];
> > +	u16 vid;
> > +	u16 pvid;
> > +
> > +	ksz_pread16(dev, port, REG_PORT_DEFAULT_VID, &pvid);
> > +	pvid = pvid & 0xFFF;
> > +
> > +	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
> > +		get_vlan_table(ds, vid, vlan_table);
> > +
> > +		vlan_table[2] &= ~BIT(port);
> > +
> > +		if (pvid == vid)
> > +			pvid = 1;
> > +
> > +		if (untagged)
> > +			vlan_table[1] &= ~BIT(port);
> > +
> > +		set_vlan_table(ds, vid, vlan_table);
> > +	}
> > +
> > +	ksz_pwrite16(dev, port, REG_PORT_DEFAULT_VID, pvid);
> > +
> > +	return 0;
> > +}
> > +
> > +static int ksz_port_vlan_dump(struct dsa_switch *ds, int port,
> > +			      struct switchdev_obj_port_vlan *vlan,
> > +			      int (*cb)(struct switchdev_obj *obj))
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	u16 vid;
> > +	u16 data;
> > +	struct vlan_table *vlan_cache;
> > +	int err = 0;
> > +
> > +	/* use dev->vlan_cache due to lack of searching valid vlan entry */
> > +	for (vid = vlan->vid_begin; vid < dev->num_vlans; vid++) {
> > +		vlan_cache = &dev->vlan_cache[vid];
> > +
> > +		if (!(vlan_cache->table[0] & VLAN_VALID))
> > +			continue;
> > +
> > +		vlan->vid_begin = vid;
> > +		vlan->vid_end = vid;
> > +		vlan->flags = 0;
> > +		if (vlan_cache->table[2] & BIT(port)) {
> > +			if (vlan_cache->table[1] & BIT(port))
> > +				vlan->flags |=
> BRIDGE_VLAN_INFO_UNTAGGED;
> > +			ksz_pread16(dev, port, REG_PORT_DEFAULT_VID,
> &data);
> > +			if (vid == (data & 0xFFFFF))
> > +				vlan->flags |= BRIDGE_VLAN_INFO_PVID;
> > +
> > +			err = cb(&vlan->obj);
> > +			if (err)
> > +				break;
> > +		}
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +static int ksz_port_fdb_prepare(struct dsa_switch *ds, int port,
> > +				const struct switchdev_obj_port_fdb *fdb,
> > +				struct switchdev_trans *trans)
> > +{
> > +	/* nothing needed */
> > +
> > +	return 0;
> > +}
> > +
> > +struct alu_struct {
> > +	/* entry 1 */
> > +	u8	is_static:1;
> > +	u8	is_src_filter:1;
> > +	u8	is_dst_filter:1;
> > +	u8	prio_age:3;
> > +	u32	_reserv_0_1:23;
> > +	u8	mstp:3;
> > +	/* entry 2 */
> > +	u8	is_override:1;
> > +	u8	is_use_fid:1;
> > +	u32	_reserv_1_1:23;
> > +	u8	port_forward:7;
> > +	/* entry 3 & 4*/
> > +	u32	_reserv_2_1:9;
> > +	u8	fid:7;
> > +	u8	mac[ETH_ALEN];
> > +};
> > +
> > +static void ksz_port_fdb_add(struct dsa_switch *ds, int port,
> > +			     const struct switchdev_obj_port_fdb *fdb,
> > +			     struct switchdev_trans *trans)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	u32 alu_table[4];
> > +	u32 data;
> > +
> > +	mutex_lock(&dev->alu_mutex);
> > +
> > +	/* find any entry with mac & vid */
> > +	data = fdb->vid << ALU_FID_INDEX_S;
> > +	data |= ((fdb->addr[0] << 8) | fdb->addr[1]);
> > +	ksz_write32(dev, REG_SW_ALU_INDEX_0, data);
> > +
> > +	data = ((fdb->addr[2] << 24) | (fdb->addr[3] << 16));
> > +	data |= ((fdb->addr[4] << 8) | fdb->addr[5]);
> > +	ksz_write32(dev, REG_SW_ALU_INDEX_1, data);
> > +
> > +	/* start read operation */
> > +	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_READ | ALU_START);
> > +
> > +	/* wait to be finished */
> > +	do {
> > +		ksz_read32(dev, REG_SW_ALU_CTRL__4, &data);
> > +	} while (data & ALU_START);
> > +
> > +	/* read ALU entry */
> > +	read_table(ds, alu_table);
> > +
> > +	mutex_unlock(&dev->alu_mutex);
> > +
> > +	/* update ALU entry */
> > +	alu_table[0] = ALU_V_STATIC_VALID;
> > +	alu_table[1] |= BIT(port);
> > +	if (fdb->vid)
> > +		alu_table[1] |= ALU_V_USE_FID;
> > +	alu_table[2] = (fdb->vid << ALU_V_FID_S);
> > +	alu_table[2] |= ((fdb->addr[0] << 8) | fdb->addr[1]);
> > +	alu_table[3] = ((fdb->addr[2] << 24) | (fdb->addr[3] << 16));
> > +	alu_table[3] |= ((fdb->addr[4] << 8) | fdb->addr[5]);
> > +
> > +	mutex_lock(&dev->alu_mutex);
> > +
> > +	write_table(ds, alu_table);
> > +
> > +	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_WRITE | ALU_START);
> > +
> > +	/* wait to be finished */
> > +	do {
> > +		ksz_read32(dev, REG_SW_ALU_CTRL__4, &data);
> > +	} while (data & ALU_START);
> 
> Same thing here we also need a timeout count, there are more occurences
> of this throughout the entire driver.
> 
> > +
> > +	mutex_unlock(&dev->alu_mutex);
> > +}
> > +
> > +static int ksz_port_fdb_del(struct dsa_switch *ds, int port,
> > +			    const struct switchdev_obj_port_fdb *fdb)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	u32 alu_table[4];
> > +	u32 data;
> > +
> > +	mutex_lock(&dev->alu_mutex);
> > +
> > +	/* read any entry with mac & vid */
> > +	data = fdb->vid << ALU_FID_INDEX_S;
> > +	data |= ((fdb->addr[0] << 8) | fdb->addr[1]);
> > +	ksz_write32(dev, REG_SW_ALU_INDEX_0, data);
> > +
> > +	data = ((fdb->addr[2] << 24) | (fdb->addr[3] << 16));
> > +	data |= ((fdb->addr[4] << 8) | fdb->addr[5]);
> > +	ksz_write32(dev, REG_SW_ALU_INDEX_1, data);
> > +
> > +	/* start read operation */
> > +	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_READ | ALU_START);
> > +
> > +	/* wait to be finished */
> > +	do {
> > +		ksz_read32(dev, REG_SW_ALU_CTRL__4, &data);
> > +	} while (data & ALU_START);
> > +
> > +	ksz_read32(dev, REG_SW_ALU_VAL_A, &alu_table[0]);
> > +	if (alu_table[0] & ALU_V_STATIC_VALID) {
> > +		ksz_read32(dev, REG_SW_ALU_VAL_B, &alu_table[1]);
> > +		ksz_read32(dev, REG_SW_ALU_VAL_C, &alu_table[2]);
> > +		ksz_read32(dev, REG_SW_ALU_VAL_D, &alu_table[3]);
> > +
> > +		/* clear forwarding port */
> > +		alu_table[2] &= ~BIT(port);
> > +
> > +		/* if there is no port to forward, clear table */
> > +		if ((alu_table[2] & ALU_V_PORT_MAP) == 0) {
> > +			alu_table[0] = 0;
> > +			alu_table[1] = 0;
> > +			alu_table[2] = 0;
> > +			alu_table[3] = 0;
> > +		}
> > +	} else {
> > +		alu_table[0] = 0;
> > +		alu_table[1] = 0;
> > +		alu_table[2] = 0;
> > +		alu_table[3] = 0;
> > +	}
> > +
> > +	write_table(ds, alu_table);
> > +
> > +	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_WRITE | ALU_START);
> > +
> > +	/* wait to be finished */
> > +	do {
> > +		ksz_read32(dev, REG_SW_ALU_CTRL__4, &data);
> > +	} while (data & ALU_START);
> > +
> > +	mutex_unlock(&dev->alu_mutex);
> > +
> > +	return 0;
> > +}
> > +
> > +static void convert_alu(struct alu_struct *alu, u32 *alu_table)
> > +{
> > +	alu->is_static = !!(alu_table[0] & ALU_V_STATIC_VALID);
> > +	alu->is_src_filter = !!(alu_table[0] & ALU_V_SRC_FILTER);
> > +	alu->is_dst_filter = !!(alu_table[0] & ALU_V_DST_FILTER);
> > +	alu->prio_age = (alu_table[0] >> ALU_V_PRIO_AGE_CNT_S) &
> > +			ALU_V_PRIO_AGE_CNT_M;
> > +	alu->mstp = alu_table[0] & ALU_V_MSTP_M;
> > +
> > +	alu->is_override = !!(alu_table[1] & ALU_V_OVERRIDE);
> > +	alu->is_use_fid = !!(alu_table[1] & ALU_V_USE_FID);
> > +	alu->port_forward = alu_table[1] & ALU_V_PORT_MAP;
> > +
> > +	alu->fid = (alu_table[2] >> ALU_V_FID_S) & ALU_V_FID_M;
> > +
> > +	alu->mac[0] = (alu_table[2] >> 8) & 0xFF;
> > +	alu->mac[1] = alu_table[2] & 0xFF;
> > +	alu->mac[2] = (alu_table[3] >> 24) & 0xFF;
> > +	alu->mac[3] = (alu_table[3] >> 16) & 0xFF;
> > +	alu->mac[4] = (alu_table[3] >> 8) & 0xFF;
> > +	alu->mac[5] = alu_table[3] & 0xFF;
> > +}
> > +
> > +static int ksz_port_fdb_dump(struct dsa_switch *ds, int port,
> > +			     struct switchdev_obj_port_fdb *fdb,
> > +			     int (*cb)(struct switchdev_obj *obj))
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	int ret = 0;
> > +	u32 data;
> > +	u32 alu_table[4];
> > +	struct alu_struct alu;
> > +
> > +	mutex_lock(&dev->alu_mutex);
> > +
> > +	/* start ALU search */
> > +	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_START |
> ALU_SEARCH);
> > +
> > +	do {
> > +		do {
> > +			ksz_read32(dev, REG_SW_ALU_CTRL__4, &data);
> > +		} while (!(data & ALU_VALID) && (data & ALU_START));
> > +
> > +		/* read ALU table */
> > +		read_table(ds, alu_table);
> > +
> > +		convert_alu(&alu, alu_table);
> > +
> > +		if (alu.port_forward & BIT(port)) {
> > +			fdb->vid = alu.fid;
> > +			if (alu.is_static)
> > +				fdb->ndm_state = NUD_NOARP;
> > +			else
> > +				fdb->ndm_state = NUD_REACHABLE;
> > +			ether_addr_copy(fdb->addr, alu.mac);
> > +
> > +			ret = cb(&fdb->obj);
> > +			if (ret)
> > +				goto exit;
> > +		}
> > +	} while (data & ALU_START);
> > +
> > +exit:
> > +
> > +	/* stop ALU search */
> > +	ksz_write32(dev, REG_SW_ALU_CTRL__4, 0);
> > +
> > +	mutex_unlock(&dev->alu_mutex);
> > +
> > +	return ret;
> > +}
> > +
> > +static int ksz_port_mdb_prepare(struct dsa_switch *ds, int port,
> > +				const struct switchdev_obj_port_mdb *mdb,
> > +				struct switchdev_trans *trans)
> > +{
> > +	/* nothing to do */
> > +	return 0;
> > +}
> > +
> > +static void ksz_port_mdb_add(struct dsa_switch *ds, int port,
> > +			     const struct switchdev_obj_port_mdb *mdb,
> > +			     struct switchdev_trans *trans)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	u32 static_table[4];
> > +	u32 data;
> > +	int index;
> > +	u32 mac_hi, mac_lo;
> > +
> > +	mac_hi = ((mdb->addr[0] << 8) | mdb->addr[1]);
> > +	mac_lo = ((mdb->addr[2] << 24) | (mdb->addr[3] << 16));
> > +	mac_lo |= ((mdb->addr[4] << 8) | mdb->addr[5]);
> > +
> > +	for (index = 0; index < dev->num_statics; index++) {
> > +		mutex_lock(&dev->alu_mutex);
> > +
> > +		/* find empty slot first */
> > +		data = (index << ALU_STAT_INDEX_S) |
> > +			ALU_STAT_READ | ALU_STAT_START;
> > +		ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
> > +
> > +		/* wait to be finished */
> > +		do {
> > +			ksz_read32(dev, REG_SW_ALU_STAT_CTRL__4,
> &data);
> > +		} while (data & ALU_STAT_START);
> > +
> > +		/* read ALU static table */
> > +		read_table(ds, static_table);
> > +
> > +		mutex_unlock(&dev->alu_mutex);
> > +
> > +		if (static_table[0] & ALU_V_STATIC_VALID) {
> > +			/* check this has same vid & mac address */
> > +
> > +			if (((static_table[2] >> ALU_V_FID_S) == (mdb->vid))
> &&
> > +			    ((static_table[2] & ALU_V_MAC_ADDR_HI) ==
> mac_hi) &&
> > +			    (static_table[3] == mac_lo)) {
> > +				/* found matching one */
> > +				break;
> > +			}
> > +		} else {
> > +			/* found empty one */
> > +			break;
> > +		}
> > +	}
> > +
> > +	/* no available entry */
> > +	if (index == dev->num_statics)
> > +		return;
> > +
> > +	/* add entry */
> > +	static_table[0] = ALU_V_STATIC_VALID;
> > +	static_table[1] |= BIT(port);
> > +	if (mdb->vid)
> > +		static_table[1] |= ALU_V_USE_FID;
> > +	static_table[2] = (mdb->vid << ALU_V_FID_S);
> > +	static_table[2] |= mac_hi;
> > +	static_table[3] = mac_lo;
> > +
> > +	mutex_lock(&dev->alu_mutex);
> > +
> > +	write_table(ds, static_table);
> > +
> > +	data = (index << ALU_STAT_INDEX_S) | ALU_STAT_START;
> > +	ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
> > +
> > +	/* wait to be finished */
> > +	do {
> > +		ksz_read32(dev, REG_SW_ALU_STAT_CTRL__4, &data);
> > +	} while (data & ALU_STAT_START);
> > +
> > +	mutex_unlock(&dev->alu_mutex);
> > +}
> > +
> > +static int ksz_port_mdb_del(struct dsa_switch *ds, int port,
> > +			    const struct switchdev_obj_port_mdb *mdb)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	u32 static_table[4];
> > +	u32 data;
> > +	int index;
> > +	u32 mac_hi, mac_lo;
> > +
> > +	mac_hi = ((mdb->addr[0] << 8) | mdb->addr[1]);
> > +	mac_lo = ((mdb->addr[2] << 24) | (mdb->addr[3] << 16));
> > +	mac_lo |= ((mdb->addr[4] << 8) | mdb->addr[5]);
> > +
> > +	for (index = 0; index < dev->num_statics; index++) {
> > +		mutex_lock(&dev->alu_mutex);
> > +
> > +		/* find empty slot first */
> > +		data = (index << ALU_STAT_INDEX_S) |
> > +			ALU_STAT_READ | ALU_STAT_START;
> > +		ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
> > +
> > +		/* wait to be finished */
> > +		do {
> > +			ksz_read32(dev, REG_SW_ALU_STAT_CTRL__4,
> &data);
> > +		} while (data & ALU_STAT_START);
> > +
> > +		/* read ALU static table */
> > +		read_table(ds, static_table);
> > +
> > +		mutex_unlock(&dev->alu_mutex);
> > +
> > +		if (static_table[0] & ALU_V_STATIC_VALID) {
> > +			/* check this has same vid & mac address */
> > +
> > +			if (((static_table[2] >> ALU_V_FID_S) == (mdb->vid))
> &&
> > +			    ((static_table[2] & ALU_V_MAC_ADDR_HI) ==
> mac_hi) &&
> > +			    (static_table[3] == mac_lo)) {
> > +				/* found matching one */
> > +				break;
> > +			}
> > +		}
> > +	}
> > +
> > +	/* no available entry */
> > +	if (index == dev->num_statics)
> > +		return -EINVAL;
> > +
> > +	/* clear port */
> > +	static_table[1] &= ~BIT(port);
> > +
> > +	if ((static_table[1] & ALU_V_PORT_MAP) == 0) {
> > +		/* delete entry */
> > +		static_table[0] = 0;
> > +		static_table[1] = 0;
> > +		static_table[2] = 0;
> > +		static_table[3] = 0;
> > +	}
> > +
> > +	mutex_lock(&dev->alu_mutex);
> > +
> > +	write_table(ds, static_table);
> > +
> > +	data = (index << ALU_STAT_INDEX_S) | ALU_STAT_START;
> > +	ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
> > +
> > +	/* wait to be finished */
> > +	do {
> > +		ksz_read32(dev, REG_SW_ALU_STAT_CTRL__4, &data);
> > +	} while (data & ALU_STAT_START);
> > +
> > +	mutex_unlock(&dev->alu_mutex);
> > +
> > +	return 0;
> > +}
> > +
> > +static int ksz_port_mdb_dump(struct dsa_switch *ds, int port,
> > +			     struct switchdev_obj_port_mdb *mdb,
> > +			     int (*cb)(struct switchdev_obj *obj))
> > +{
> > +	/* this is not called by switch layer */
> > +	return 0;
> > +}
> > +
> > +static int ksz_port_mirror_add(struct dsa_switch *ds, int port,
> > +			       struct dsa_mall_mirror_tc_entry *mirror,
> > +			       bool ingress)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +
> > +	if (ingress)
> > +		ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_RX,
> true);
> > +	else
> > +		ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_TX,
> true);
> > +
> > +	ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_SNIFFER,
> false);
> > +
> > +	/* configure mirror port */
> > +	ksz_port_cfg(dev, mirror->to_local_port, P_MIRROR_CTRL,
> > +		     PORT_MIRROR_SNIFFER, true);
> > +
> > +	ksz_cfg(dev, S_MIRROR_CTRL, SW_MIRROR_RX_TX, false);
> > +
> > +	return 0;
> > +}
> > +
> > +static void ksz_port_mirror_del(struct dsa_switch *ds, int port,
> > +				struct dsa_mall_mirror_tc_entry *mirror)
> > +{
> > +	struct ksz_device *dev = ds->priv;
> > +	u8 data;
> > +
> > +	if (mirror->ingress)
> > +		ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_RX,
> false);
> > +	else
> > +		ksz_port_cfg(dev, port, P_MIRROR_CTRL, PORT_MIRROR_TX,
> false);
> > +
> > +	ksz_pread8(dev, port, P_MIRROR_CTRL, &data);
> > +
> > +	if (!(data & (PORT_MIRROR_RX | PORT_MIRROR_TX)))
> > +		ksz_port_cfg(dev, mirror->to_local_port, P_MIRROR_CTRL,
> > +			     PORT_MIRROR_SNIFFER, false);
> > +}
> > +
> > +static const struct dsa_switch_ops ksz_switch_ops = {
> > +	.get_tag_protocol	= ksz_get_tag_protocol,
> > +	.setup			= ksz_setup,
> > +	.phy_read		= ksz_phy_read16,
> > +	.phy_write		= ksz_phy_write16,
> > +	.port_enable		= ksz_enable_port,
> > +	.port_disable		= ksz_disable_port,
> > +	.get_strings		= ksz_get_strings,
> > +	.get_ethtool_stats	= ksz_get_ethtool_stats,
> > +	.get_sset_count		= ksz_sset_count,
> > +	.port_stp_state_set	= ksz_port_stp_state_set,
> > +	.port_fast_age		= ksz_port_fast_age,
> > +	.port_vlan_filtering	= ksz_port_vlan_filtering,
> > +	.port_vlan_prepare	= ksz_port_vlan_prepare,
> > +	.port_vlan_add		= ksz_port_vlan_add,
> > +	.port_vlan_del		= ksz_port_vlan_del,
> > +	.port_vlan_dump		= ksz_port_vlan_dump,
> > +	.port_fdb_prepare	= ksz_port_fdb_prepare,
> > +	.port_fdb_dump		= ksz_port_fdb_dump,
> > +	.port_fdb_add		= ksz_port_fdb_add,
> > +	.port_fdb_del		= ksz_port_fdb_del,
> > +	.port_mdb_prepare       = ksz_port_mdb_prepare,
> > +	.port_mdb_add           = ksz_port_mdb_add,
> > +	.port_mdb_del           = ksz_port_mdb_del,
> > +	.port_mdb_dump          = ksz_port_mdb_dump,
> > +	.port_mirror_add	= ksz_port_mirror_add,
> > +	.port_mirror_del	= ksz_port_mirror_del,
> > +};
> > +
> > +struct ksz_chip_data {
> > +	u32 chip_id;
> > +	const char *dev_name;
> > +	int num_vlans;
> > +	int num_alus;
> > +	int num_statics;
> > +	u16 enabled_ports;
> > +	int cpu_port;
> > +	int port_cnt;
> > +	int phy_port_cnt;
> > +};
> > +
> > +static const struct ksz_chip_data ksz_switch_chips[] = {
> > +	{
> > +		.chip_id = 0x00947700,
> > +		.dev_name = "KSZ9477",
> > +		.num_vlans = 4096,
> > +		.num_alus = 4096,
> > +		.num_statics = 16,
> > +		.enabled_ports = 0x1F,	/* port0-4 */
> > +		.cpu_port = 5,		/* port5 (RGMII) */
> > +		.port_cnt = 7,
> > +		.phy_port_cnt = 5,
> > +	},
> > +};
> 
> Do you have an idea of which other models would be largely software
> compatible?
Yes. Mostly num of port & configuration differences.

> > +
> > +static int ksz_switch_init(struct ksz_device *dev)
> > +{
> > +	int i;
> > +
> > +	mutex_init(&dev->reg_mutex);
> > +	mutex_init(&dev->stats_mutex);
> > +	mutex_init(&dev->alu_mutex);
> > +
> > +	dev->ds->ops = &ksz_switch_ops;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(ksz_switch_chips); i++) {
> > +		const struct ksz_chip_data *chip = &ksz_switch_chips[i];
> > +
> > +		if (dev->chip_id == chip->chip_id) {
> > +			if (!dev->enabled_ports)
> > +				dev->enabled_ports = chip->enabled_ports;
> > +			dev->name = chip->dev_name;
> > +			dev->num_vlans = chip->num_vlans;
> > +			dev->num_alus = chip->num_alus;
> > +			dev->num_statics = chip->num_statics;
> > +			/* default cpu port */
> > +			dev->cpu_port = chip->cpu_port;
> > +			dev->port_cnt = chip->port_cnt;
> > +
> > +			break;
> > +		}
> > +	}
> > +
> > +	/* no switch found */
> > +	if (!dev->port_cnt)
> > +		return -ENODEV;
> > +
> > +	return 0;
> > +}
> > +
> > +struct ksz_device *ksz_switch_alloc(struct device *base,
> > +				    const struct ksz_io_ops *ops,
> > +				    void *priv)
> > +{
> > +	struct dsa_switch *ds;
> > +	struct ksz_device *swdev;
> > +
> > +	ds = dsa_switch_alloc(base, DSA_MAX_PORTS);
> > +	if (!ds)
> > +		return NULL;
> > +
> > +	swdev = devm_kzalloc(base, sizeof(*swdev), GFP_KERNEL);
> > +	if (!swdev)
> > +		return NULL;
> > +
> > +	ds->priv = swdev;
> > +	swdev->dev = base;
> > +
> > +	swdev->ds = ds;
> > +	swdev->priv = priv;
> > +	swdev->ops = ops;
> > +
> > +	return swdev;
> > +}
> > +EXPORT_SYMBOL(ksz_switch_alloc);
> > +
> > +int ksz_switch_detect(struct ksz_device *dev)
> > +{
> > +	u8 data8;
> > +	u32 id32;
> > +	int ret;
> > +
> > +	/* turn off SPI DO Edge select */
> > +	ret = ksz_read8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, &data8);
> > +	if (ret)
> > +		return ret;
> > +
> > +	data8 &= ~SPI_AUTO_EDGE_DETECTION;
> > +	ret = ksz_write8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, data8);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* read chip id */
> > +	ret = ksz_read32(dev, REG_CHIP_ID0__1, &id32);
> > +	if (ret)
> > +		return ret;
> > +
> > +	dev->chip_id = id32;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(ksz_switch_detect);
> > +
> > +int ksz_switch_register(struct ksz_device *dev)
> > +{
> > +	int ret;
> > +
> > +	if (dev->pdata) {
> > +		dev->chip_id = dev->pdata->chip_id;
> > +		dev->enabled_ports = dev->pdata->enabled_ports;
> > +	}
> > +
> > +	if (!dev->chip_id && ksz_switch_detect(dev))
> > +		return -EINVAL;
> > +
> > +	ret = ksz_switch_init(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return dsa_register_switch(dev->ds, dev->ds->dev);
> > +}
> > +EXPORT_SYMBOL(ksz_switch_register);
> > +
> > +void ksz_switch_remove(struct ksz_device *dev)
> > +{
> > +	dsa_unregister_switch(dev->ds);
> > +}
> > +EXPORT_SYMBOL(ksz_switch_remove);
> > +
> > +MODULE_AUTHOR("Woojung Huh <Woojung.Huh@microchip.com>");
> > +MODULE_DESCRIPTION("Microchip KSZ Series Switch DSA Driver");
> > diff --git a/drivers/net/dsa/microchip/ksz_priv.h
> b/drivers/net/dsa/microchip/ksz_priv.h
> > new file mode 100644
> > index 0000000..5619acd
> > --- /dev/null
> > +++ b/drivers/net/dsa/microchip/ksz_priv.h
> > @@ -0,0 +1,207 @@
> > +/*
> > + * Microchip KSZ series switch common definitions
> > + *
> > + * Copyright (C) 2017
> > + *
> > + * Permission to use, copy, modify, and/or distribute this software for any
> > + * purpose with or without fee is hereby granted, provided that the above
> > + * copyright notice and this permission notice appear in all copies.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
> WARRANTIES
> > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
> WARRANTIES OF
> > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE
> LIABLE FOR
> > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR
> ANY DAMAGES
> > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS,
> WHETHER IN AN
> > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> ARISING OUT OF
> > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS
> SOFTWARE.
> > + */
> > +
> > +#ifndef __KSZ_PRIV_H
> > +#define __KSZ_PRIV_H
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/mutex.h>
> > +#include <linux/phy.h>
> > +#include <linux/etherdevice.h>
> > +#include <net/dsa.h>
> > +
> > +#include "ksz_9477_reg.h"
> > +
> > +struct ksz_io_ops;
> > +
> > +struct vlan_table {
> > +	u32 table[3];
> > +};
> > +
> > +struct ksz_device {
> > +	struct dsa_switch *ds;
> > +	struct ksz_platform_data *pdata;
> > +	const char *name;
> > +
> > +	struct mutex reg_mutex;		/* register access */
> > +	struct mutex stats_mutex;	/* status access */
> > +	struct mutex alu_mutex;		/* ALU access */
> > +	const struct ksz_io_ops *ops;
> > +
> > +	struct device *dev;
> > +
> > +	void *priv;
> > +
> > +	/* chip specific data */
> > +	u32 chip_id;
> > +	u16 enabled_ports;
> > +	int num_vlans;
> > +	int num_alus;
> > +	int num_statics;
> > +	int cpu_port;
> > +	int port_cnt;
> > +
> > +	struct vlan_table *vlan_cache;
> > +};
> > +
> > +struct ksz_io_ops {
> > +	int (*read8)(struct ksz_device *dev, u32 reg, u8 *value);
> > +	int (*read16)(struct ksz_device *dev, u32 reg, u16 *value);
> > +	int (*read24)(struct ksz_device *dev, u32 reg, u32 *value);
> > +	int (*read32)(struct ksz_device *dev, u32 reg, u32 *value);
> > +	int (*write8)(struct ksz_device *dev, u32 reg, u8 value);
> > +	int (*write16)(struct ksz_device *dev, u32 reg, u16 value);
> > +	int (*write24)(struct ksz_device *dev, u32 reg, u32 value);
> > +	int (*write32)(struct ksz_device *dev, u32 reg, u32 value);
> > +	int (*phy_read16)(struct ksz_device *dev, int addr, int reg,
> > +			  u16 *value);
> > +	int (*phy_write16)(struct ksz_device *dev, int addr, int reg,
> > +			   u16 value);
> > +};
> > +
> > +struct ksz_device *ksz_switch_alloc(struct device *base,
> > +				    const struct ksz_io_ops *ops, void *priv);
> > +int ksz_switch_detect(struct ksz_device *dev);
> > +int ksz_switch_register(struct ksz_device *dev);
> > +void ksz_switch_remove(struct ksz_device *dev);
> > +
> > +static inline int ksz_read8(struct ksz_device *dev, u32 reg, u8 *val)
> > +{
> > +	int ret;
> > +
> > +	mutex_lock(&dev->reg_mutex);
> > +	ret = dev->ops->read8(dev, reg, val);
> > +	mutex_unlock(&dev->reg_mutex);
> > +
> > +	return ret;
> > +}
> > +
> > +static inline int ksz_read16(struct ksz_device *dev, u32 reg, u16 *val)
> > +{
> > +	int ret;
> > +
> > +	mutex_lock(&dev->reg_mutex);
> > +	ret = dev->ops->read16(dev, reg, val);
> > +	mutex_unlock(&dev->reg_mutex);
> > +
> > +	return ret;
> > +}
> > +
> > +static inline int ksz_read24(struct ksz_device *dev, u32 reg, u32 *val)
> > +{
> > +	int ret;
> > +
> > +	mutex_lock(&dev->reg_mutex);
> > +	ret = dev->ops->read24(dev, reg, val);
> > +	mutex_unlock(&dev->reg_mutex);
> > +
> > +	return ret;
> > +}
> > +
> > +static inline int ksz_read32(struct ksz_device *dev, u32 reg, u32 *val)
> > +{
> > +	int ret;
> > +
> > +	mutex_lock(&dev->reg_mutex);
> > +	ret = dev->ops->read32(dev, reg, val);
> > +	mutex_unlock(&dev->reg_mutex);
> > +
> > +	return ret;
> > +}
> > +
> > +static inline int ksz_write8(struct ksz_device *dev, u32 reg, u8 value)
> > +{
> > +	int ret;
> > +
> > +	mutex_lock(&dev->reg_mutex);
> > +	ret = dev->ops->write8(dev, reg, value);
> > +	mutex_unlock(&dev->reg_mutex);
> > +
> > +	return ret;
> > +}
> > +
> > +static inline int ksz_write16(struct ksz_device *dev, u32 reg, u16 value)
> > +{
> > +	int ret;
> > +
> > +	mutex_lock(&dev->reg_mutex);
> > +	ret = dev->ops->write16(dev, reg, value);
> > +	mutex_unlock(&dev->reg_mutex);
> > +
> > +	return ret;
> > +}
> > +
> > +static inline int ksz_write24(struct ksz_device *dev, u32 reg, u32 value)
> > +{
> > +	int ret;
> > +
> > +	mutex_lock(&dev->reg_mutex);
> > +	ret = dev->ops->write24(dev, reg, value);
> > +	mutex_unlock(&dev->reg_mutex);
> > +
> > +	return ret;
> > +}
> > +
> > +static inline int ksz_write32(struct ksz_device *dev, u32 reg, u32 value)
> > +{
> > +	int ret;
> > +
> > +	mutex_lock(&dev->reg_mutex);
> > +	ret = dev->ops->write32(dev, reg, value);
> > +	mutex_unlock(&dev->reg_mutex);
> > +
> > +	return ret;
> > +}
> > +
> > +static inline void ksz_pread8(struct ksz_device *dev, int port, int offset,
> > +			      u8 *data)
> > +{
> > +	ksz_read8(dev, PORT_CTRL_ADDR(port, offset), data);
> > +}
> > +
> > +static inline void ksz_pread16(struct ksz_device *dev, int port, int offset,
> > +			       u16 *data)
> > +{
> > +	ksz_read16(dev, PORT_CTRL_ADDR(port, offset), data);
> > +}
> > +
> > +static inline void ksz_pread32(struct ksz_device *dev, int port, int offset,
> > +			       u32 *data)
> > +{
> > +	ksz_read32(dev, PORT_CTRL_ADDR(port, offset), data);
> > +}
> > +
> > +static inline void ksz_pwrite8(struct ksz_device *dev, int port, int offset,
> > +			       u8 data)
> > +{
> > +	ksz_write8(dev, PORT_CTRL_ADDR(port, offset), data);
> > +}
> > +
> > +static inline void ksz_pwrite16(struct ksz_device *dev, int port, int offset,
> > +				u16 data)
> > +{
> > +	ksz_write16(dev, PORT_CTRL_ADDR(port, offset), data);
> > +}
> > +
> > +static inline void ksz_pwrite32(struct ksz_device *dev, int port, int offset,
> > +				u32 data)
> > +{
> > +	ksz_write32(dev, PORT_CTRL_ADDR(port, offset), data);
> > +}
> > +
> > +#endif
> > diff --git a/drivers/net/dsa/microchip/ksz_spi.c
> b/drivers/net/dsa/microchip/ksz_spi.c
> > new file mode 100644
> > index 0000000..f92e41e
> > --- /dev/null
> > +++ b/drivers/net/dsa/microchip/ksz_spi.c
> > @@ -0,0 +1,215 @@
> > +/*
> > + * Microchip KSZ series register access through SPI
> > + *
> > + * Copyright (C) 2017
> > + *
> > + * Permission to use, copy, modify, and/or distribute this software for any
> > + * purpose with or without fee is hereby granted, provided that the above
> > + * copyright notice and this permission notice appear in all copies.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
> WARRANTIES
> > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
> WARRANTIES OF
> > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE
> LIABLE FOR
> > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR
> ANY DAMAGES
> > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS,
> WHETHER IN AN
> > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> ARISING OUT OF
> > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS
> SOFTWARE.
> > + */
> > +
> > +#include <asm/unaligned.h>
> > +
> > +#include <linux/delay.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/spi/spi.h>
> > +
> > +#include "ksz_priv.h"
> > +
> > +/* SPI frame opcodes */
> > +#define KS_SPIOP_RD			3
> > +#define KS_SPIOP_WR			2
> > +
> > +#define SPI_ADDR_SHIFT			24
> > +#define SPI_ADDR_MASK			(BIT(SPI_ADDR_SHIFT) - 1)
> > +#define SPI_TURNAROUND_SHIFT		5
> > +
> > +static inline int ksz_spi_read_reg(struct spi_device *spi, u32 reg, u8 *val,
> > +				   unsigned int len)
> > +{
> > +	u32 txbuf;
> > +	int ret;
> > +
> > +	txbuf = reg & SPI_ADDR_MASK;
> > +	txbuf |= KS_SPIOP_RD << SPI_ADDR_SHIFT;
> > +	txbuf <<= SPI_TURNAROUND_SHIFT;
> > +	txbuf = cpu_to_be32(txbuf);
> > +
> > +	ret = spi_write_then_read(spi, &txbuf, 4, val, len);
> > +	return ret;
> > +}
> > +
> > +static int ksz_spi_read(struct ksz_device *dev, u32 reg, u8 *data,
> > +			unsigned int len)
> > +{
> > +	struct spi_device *spi = dev->priv;
> > +
> > +	return ksz_spi_read_reg(spi, reg, data, len);
> > +}
> > +
> > +static int ksz_spi_read8(struct ksz_device *dev, u32 reg, u8 *val)
> > +{
> > +	return ksz_spi_read(dev, reg, val, 1);
> > +}
> > +
> > +static int ksz_spi_read16(struct ksz_device *dev, u32 reg, u16 *val)
> > +{
> > +	int ret = ksz_spi_read(dev, reg, (u8 *)val, 2);
> > +
> > +	if (!ret)
> > +		*val = be16_to_cpu(*val);
> > +
> > +	return ret;
> > +}
> > +
> > +static int ksz_spi_read24(struct ksz_device *dev, u32 reg, u32 *val)
> > +{
> > +	int ret;
> > +
> > +	*val = 0;
> > +	ret = ksz_spi_read(dev, reg, (u8 *)val, 3);
> > +	if (!ret) {
> > +		*val = be32_to_cpu(*val);
> > +		/* convert to 24bit */
> > +		*val >>= 8;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static int ksz_spi_read32(struct ksz_device *dev, u32 reg, u32 *val)
> > +{
> > +	int ret = ksz_spi_read(dev, reg, (u8 *)val, 4);
> > +
> > +	if (!ret)
> > +		*val = be32_to_cpu(*val);
> > +
> > +	return ret;
> > +}
> > +
> > +static inline int ksz_spi_write_reg(struct spi_device *spi, u32 reg, u8 *val,
> > +				    unsigned int len)
> > +{
> > +	u32 txbuf;
> > +	u8 data[12];
> > +	int i;
> > +
> > +	txbuf = reg & SPI_ADDR_MASK;
> > +	txbuf |= (KS_SPIOP_WR << SPI_ADDR_SHIFT);
> > +	txbuf <<= SPI_TURNAROUND_SHIFT;
> > +	txbuf = cpu_to_be32(txbuf);
> > +
> > +	data[0] = txbuf & 0xFF;
> > +	data[1] = (txbuf & 0xFF00) >> 8;
> > +	data[2] = (txbuf & 0xFF0000) >> 16;
> > +	data[3] = (txbuf & 0xFF000000) >> 24;
> > +	for (i = 0; i < len; i++)
> > +		data[i + 4] = val[i];
> > +
> > +	return spi_write(spi, &data, 4 + len);
> > +}
> > +
> > +static int ksz_spi_write8(struct ksz_device *dev, u32 reg, u8 value)
> > +{
> > +	struct spi_device *spi = dev->priv;
> > +
> > +	return ksz_spi_write_reg(spi, reg, &value, 1);
> > +}
> > +
> > +static int ksz_spi_write16(struct ksz_device *dev, u32 reg, u16 value)
> > +{
> > +	struct spi_device *spi = dev->priv;
> > +
> > +	value = cpu_to_be16(value);
> > +	return ksz_spi_write_reg(spi, reg, (u8 *)&value, 2);
> > +}
> > +
> > +static int ksz_spi_write24(struct ksz_device *dev, u32 reg, u32 value)
> > +{
> > +	struct spi_device *spi = dev->priv;
> > +
> > +	/* make it to big endian 24bit from MSB */
> > +	value <<= 8;
> > +	value = cpu_to_be32(value);
> > +	return ksz_spi_write_reg(spi, reg, (u8 *)&value, 3);
> > +}
> > +
> > +static int ksz_spi_write32(struct ksz_device *dev, u32 reg, u32 value)
> > +{
> > +	struct spi_device *spi = dev->priv;
> > +
> > +	value = cpu_to_be32(value);
> > +	return ksz_spi_write_reg(spi, reg, (u8 *)&value, 4);
> > +}
> > +
> > +static const struct ksz_io_ops ksz_spi_ops = {
> > +	.read8 = ksz_spi_read8,
> > +	.read16 = ksz_spi_read16,
> > +	.read24 = ksz_spi_read24,
> > +	.read32 = ksz_spi_read32,
> > +	.write8 = ksz_spi_write8,
> > +	.write16 = ksz_spi_write16,
> > +	.write24 = ksz_spi_write24,
> > +	.write32 = ksz_spi_write32,
> > +};
> > +
> > +static int ksz_spi_probe(struct spi_device *spi)
> > +{
> > +	struct ksz_device *dev;
> > +	int ret;
> > +
> > +	dev = ksz_switch_alloc(&spi->dev, &ksz_spi_ops, spi);
> > +	if (!dev)
> > +		return -ENOMEM;
> > +
> > +	if (spi->dev.platform_data)
> > +		dev->pdata = spi->dev.platform_data;
> > +
> > +	ret = ksz_switch_register(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	spi_set_drvdata(spi, dev);
> > +
> > +	return 0;
> > +}
> > +
> > +static int ksz_spi_remove(struct spi_device *spi)
> > +{
> > +	struct ksz_device *dev = spi_get_drvdata(spi);
> > +
> > +	if (dev)
> > +		ksz_switch_remove(dev);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id ksz_dt_ids[] = {
> > +	{ .compatible = "microchip,ksz9477" },
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, ksz_dt_ids);
> > +
> > +static struct spi_driver ksz_spi_driver = {
> > +	.driver = {
> > +		.name	= "ksz9477-switch",
> > +		.owner	= THIS_MODULE,
> > +		.of_match_table = of_match_ptr(ksz_dt_ids),
> > +	},
> > +	.probe	= ksz_spi_probe,
> > +	.remove	= ksz_spi_remove,
> > +};
> > +
> > +module_spi_driver(ksz_spi_driver);
> > +
> > +MODULE_AUTHOR("Woojung Huh <Woojung.Huh@microchip.com>");
> > +MODULE_DESCRIPTION("Microchip KSZ Series Switch SPI access Driver");
> > diff --git a/include/linux/platform_data/microchip-ksz.h
> b/include/linux/platform_data/microchip-ksz.h
> > new file mode 100644
> > index 0000000..84789ca
> > --- /dev/null
> > +++ b/include/linux/platform_data/microchip-ksz.h
> > @@ -0,0 +1,29 @@
> > +/*
> > + * Microchip KSZ series switch platform data
> > + *
> > + * Copyright (C) 2017
> > + *
> > + * Permission to use, copy, modify, and/or distribute this software for any
> > + * purpose with or without fee is hereby granted, provided that the above
> > + * copyright notice and this permission notice appear in all copies.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
> WARRANTIES
> > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
> WARRANTIES OF
> > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE
> LIABLE FOR
> > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR
> ANY DAMAGES
> > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS,
> WHETHER IN AN
> > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> ARISING OUT OF
> > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS
> SOFTWARE.
> > + */
> > +
> > +#ifndef __MICROCHIP_KSZ_H
> > +#define __MICROCHIP_KSZ_H
> > +
> > +#include <linux/kernel.h>
> > +
> > +struct ksz_platform_data {
> > +	u32 chip_id;
> > +	u16 enabled_ports;
> > +};
> > +
> > +#endif
> >
> 
> 
> --
> Florian

Thanks.
Woojung

^ permalink raw reply

* Re: Network cooling device and how to control NIC speed on thermal condition
From: Andrew Lunn @ 2017-05-08 14:02 UTC (permalink / raw)
  To: Waldemar Rymarkiewicz; +Cc: Alan Cox, Florian Fainelli, netdev, linux-kernel
In-Reply-To: <CAHKzcENk8USj1kjFco=a+vdXeX-FycM1G4hPm6JnotEUtNi99Q@mail.gmail.com>

> However, the fact is that PHYs having active 1G/s link generate much
> more heat than having 100M/s link independently from network traffic.

Yes, this is true. I got an off-list email suggesting this power
difference is very significant, more so than actually processing
packets.

> All cooling methods impact host only, but "net cooling" impacts remote
> side in addition, which seems to me to be a problem sometimes. Also,
> the moment of link renegotiation blocks rx/tx for upper layers, so the
> user sees a pause when streaming a video for example. However, if a
> system is under a thermal condition, does it really matter?

I don't know the cooling subsystem too well. Can you express a 'cost'
for making a change, as well as the likely result in making the
change. You might want to make the cost high, so it is used as a last
resort if other methods cannot give enough cooling.

       Andrew

^ permalink raw reply

* [PATCH v1 0/4] stmmac: pci: Fix crash on Intel Galileo Gen2
From: Andy Shevchenko @ 2017-05-08 14:14 UTC (permalink / raw)
  To: Joao Pinto, Jan Kiszka, netdev, Giuseppe CAVALLARO,
	David S. Miller
  Cc: Andy Shevchenko

Due to misconfiguration of PCI driver for Intel Quark the user will get
a kernel crash:

# udhcpc -i eth0
udhcpc: started, v1.26.2
stmmaceth 0000:00:14.6 eth0: device MAC address 98:4f:ee:05:ac:47
Generic PHY stmmac-a6:01: attached PHY driver [Generic PHY] (mii_bus:phy_addr=stmmac-a6:01, irq=-1)
stmmaceth 0000:00:14.6 eth0: IEEE 1588-2008 Advanced Timestamp supported
stmmaceth 0000:00:14.6 eth0: registered PTP clock
IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready

udhcpc: sending discover

stmmaceth 0000:00:14.6 eth0: Link is Up - 100Mbps/Full - flow control off
IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: stmmac_xmit+0xf1/0x1080

Fix this by adding necessary settings.

P.S. I split fix to three patches according to what each of them adds.

Andy Shevchenko (4):
  stmmac: pci: set default number of rx and tx queues
  stmmac: pci: TX and RX queue priority configuration
  stmmac: pci: RX queue routing configuration
  stmmac: pci: split out common_default_data() helper

 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 41 +++++++++++-------------
 1 file changed, 18 insertions(+), 23 deletions(-)

-- 
2.11.0

^ permalink raw reply

* [PATCH v1 2/4] stmmac: pci: TX and RX queue priority configuration
From: Andy Shevchenko @ 2017-05-08 14:14 UTC (permalink / raw)
  To: Joao Pinto, Jan Kiszka, netdev, Giuseppe CAVALLARO,
	David S. Miller
  Cc: Andy Shevchenko
In-Reply-To: <20170508141422.39612-1-andriy.shevchenko@linux.intel.com>

The commit a8f5102af2a7

	("net: stmmac: TX and RX queue priority configuration")

missed Intel Quark configuration. Append it here.

Fixes: a8f5102af2a7 ("net: stmmac: TX and RX queue priority configuration")
Cc: Joao Pinto <Joao.Pinto@synopsys.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index ae3e836f9bb6..c015a715a8ac 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -149,6 +149,10 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
 	plat->tx_queues_to_use = 1;
 	plat->rx_queues_to_use = 1;
 
+	/* Disable Priority config by default */
+	plat->tx_queues_cfg[0].use_prio = false;
+	plat->rx_queues_cfg[0].use_prio = false;
+
 	return 0;
 }
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH v1 3/4] stmmac: pci: RX queue routing configuration
From: Andy Shevchenko @ 2017-05-08 14:14 UTC (permalink / raw)
  To: Joao Pinto, Jan Kiszka, netdev, Giuseppe CAVALLARO,
	David S. Miller
  Cc: Andy Shevchenko
In-Reply-To: <20170508141422.39612-1-andriy.shevchenko@linux.intel.com>

The commit abe80fdc6ee6

    ("net: stmmac: RX queue routing configuration")

missed Intel Quark configuration. Append it here.

Fixes: abe80fdc6ee6 ("net: stmmac: RX queue routing configuration")
Cc: Joao Pinto <Joao.Pinto@synopsys.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index c015a715a8ac..2456e0a945ef 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -153,6 +153,9 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
 	plat->tx_queues_cfg[0].use_prio = false;
 	plat->rx_queues_cfg[0].use_prio = false;
 
+	/* Disable RX queues routing by default */
+	plat->rx_queues_cfg[0].pkt_route = 0x0;
+
 	return 0;
 }
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH v1 1/4] stmmac: pci: set default number of rx and tx queues
From: Andy Shevchenko @ 2017-05-08 14:14 UTC (permalink / raw)
  To: Joao Pinto, Jan Kiszka, netdev, Giuseppe CAVALLARO,
	David S. Miller
  Cc: Andy Shevchenko
In-Reply-To: <20170508141422.39612-1-andriy.shevchenko@linux.intel.com>

The commit 26d6851fd24e

	("net: stmmac: set default number of rx and tx queues in stmmac_pci")

missed Intel Quark configuration. Append it here.

Fixes: 26d6851fd24e ("net: stmmac: set default number of rx and tx queues in stmmac_pci")
Cc: Joao Pinto <Joao.Pinto@synopsys.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 39be96779145..ae3e836f9bb6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -145,6 +145,10 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
 	/* Set the maxmtu to a default of JUMBO_LEN */
 	plat->maxmtu = JUMBO_LEN;
 
+	/* Set default number of RX and TX queues to use */
+	plat->tx_queues_to_use = 1;
+	plat->rx_queues_to_use = 1;
+
 	return 0;
 }
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH v1 4/4] stmmac: pci: split out common_default_data() helper
From: Andy Shevchenko @ 2017-05-08 14:14 UTC (permalink / raw)
  To: Joao Pinto, Jan Kiszka, netdev, Giuseppe CAVALLARO,
	David S. Miller
  Cc: Andy Shevchenko
In-Reply-To: <20170508141422.39612-1-andriy.shevchenko@linux.intel.com>

New helper is added in order to prevent misconfiguration happened
for one of the platforms when configuration data is expanded.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 52 ++++++++----------------
 1 file changed, 18 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 2456e0a945ef..22f910795be4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -70,11 +70,8 @@ static int stmmac_pci_find_phy_addr(struct stmmac_pci_info *info)
 	return -ENODEV;
 }
 
-static void stmmac_default_data(struct plat_stmmacenet_data *plat)
+static void common_default_data(struct plat_stmmacenet_data *plat)
 {
-	plat->bus_id = 1;
-	plat->phy_addr = 0;
-	plat->interface = PHY_INTERFACE_MODE_GMII;
 	plat->clk_csr = 2;	/* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */
 	plat->has_gmac = 1;
 	plat->force_sf_dma_mode = 1;
@@ -82,10 +79,6 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat)
 	plat->mdio_bus_data->phy_reset = NULL;
 	plat->mdio_bus_data->phy_mask = 0;
 
-	plat->dma_cfg->pbl = 32;
-	plat->dma_cfg->pblx8 = true;
-	/* TODO: AXI */
-
 	/* Set default value for multicast hash bins */
 	plat->multicast_filter_bins = HASH_TABLE_SIZE;
 
@@ -107,12 +100,29 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat)
 	plat->rx_queues_cfg[0].pkt_route = 0x0;
 }
 
+static void stmmac_default_data(struct plat_stmmacenet_data *plat)
+{
+	/* Set common default data first */
+	common_default_data(plat);
+
+	plat->bus_id = 1;
+	plat->phy_addr = 0;
+	plat->interface = PHY_INTERFACE_MODE_GMII;
+
+	plat->dma_cfg->pbl = 32;
+	plat->dma_cfg->pblx8 = true;
+	/* TODO: AXI */
+}
+
 static int quark_default_data(struct plat_stmmacenet_data *plat,
 			      struct stmmac_pci_info *info)
 {
 	struct pci_dev *pdev = info->pdev;
 	int ret;
 
+	/* Set common default data first */
+	common_default_data(plat);
+
 	/*
 	 * Refuse to load the driver and register net device if MAC controller
 	 * does not connect to any PHY interface.
@@ -124,38 +134,12 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
 	plat->bus_id = PCI_DEVID(pdev->bus->number, pdev->devfn);
 	plat->phy_addr = ret;
 	plat->interface = PHY_INTERFACE_MODE_RMII;
-	plat->clk_csr = 2;
-	plat->has_gmac = 1;
-	plat->force_sf_dma_mode = 1;
-
-	plat->mdio_bus_data->phy_reset = NULL;
-	plat->mdio_bus_data->phy_mask = 0;
 
 	plat->dma_cfg->pbl = 16;
 	plat->dma_cfg->pblx8 = true;
 	plat->dma_cfg->fixed_burst = 1;
 	/* AXI (TODO) */
 
-	/* Set default value for multicast hash bins */
-	plat->multicast_filter_bins = HASH_TABLE_SIZE;
-
-	/* Set default value for unicast filter entries */
-	plat->unicast_filter_entries = 1;
-
-	/* Set the maxmtu to a default of JUMBO_LEN */
-	plat->maxmtu = JUMBO_LEN;
-
-	/* Set default number of RX and TX queues to use */
-	plat->tx_queues_to_use = 1;
-	plat->rx_queues_to_use = 1;
-
-	/* Disable Priority config by default */
-	plat->tx_queues_cfg[0].use_prio = false;
-	plat->rx_queues_cfg[0].use_prio = false;
-
-	/* Disable RX queues routing by default */
-	plat->rx_queues_cfg[0].pkt_route = 0x0;
-
 	return 0;
 }
 
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH v1 1/4] stmmac: pci: set default number of rx and tx queues
From: Joao Pinto @ 2017-05-08 14:18 UTC (permalink / raw)
  To: Andy Shevchenko, Joao Pinto, Jan Kiszka, netdev,
	Giuseppe CAVALLARO, David S. Miller
In-Reply-To: <20170508141422.39612-2-andriy.shevchenko@linux.intel.com>

Às 3:14 PM de 5/8/2017, Andy Shevchenko escreveu:
> The commit 26d6851fd24e
> 
> 	("net: stmmac: set default number of rx and tx queues in stmmac_pci")
> 
> missed Intel Quark configuration. Append it here.
> 
> Fixes: 26d6851fd24e ("net: stmmac: set default number of rx and tx queues in stmmac_pci")
> Cc: Joao Pinto <Joao.Pinto@synopsys.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> index 39be96779145..ae3e836f9bb6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> @@ -145,6 +145,10 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
>  	/* Set the maxmtu to a default of JUMBO_LEN */
>  	plat->maxmtu = JUMBO_LEN;
>  
> +	/* Set default number of RX and TX queues to use */
> +	plat->tx_queues_to_use = 1;
> +	plat->rx_queues_to_use = 1;
> +
>  	return 0;
>  }
>  
> 

Acked-by: Joao Pinto <jpinto@synopsys.com>

^ permalink raw reply

* Re: [PATCH v1 2/4] stmmac: pci: TX and RX queue priority configuration
From: Joao Pinto @ 2017-05-08 14:18 UTC (permalink / raw)
  To: Andy Shevchenko, Joao Pinto, Jan Kiszka, netdev,
	Giuseppe CAVALLARO, David S. Miller
In-Reply-To: <20170508141422.39612-3-andriy.shevchenko@linux.intel.com>

Às 3:14 PM de 5/8/2017, Andy Shevchenko escreveu:
> The commit a8f5102af2a7
> 
> 	("net: stmmac: TX and RX queue priority configuration")
> 
> missed Intel Quark configuration. Append it here.
> 
> Fixes: a8f5102af2a7 ("net: stmmac: TX and RX queue priority configuration")
> Cc: Joao Pinto <Joao.Pinto@synopsys.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> index ae3e836f9bb6..c015a715a8ac 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> @@ -149,6 +149,10 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
>  	plat->tx_queues_to_use = 1;
>  	plat->rx_queues_to_use = 1;
>  
> +	/* Disable Priority config by default */
> +	plat->tx_queues_cfg[0].use_prio = false;
> +	plat->rx_queues_cfg[0].use_prio = false;
> +
>  	return 0;
>  }
>  
> 

Acked-by: Joao Pinto <jpinto@synopsys.com>

^ permalink raw reply

* Re: [PATCH v1 3/4] stmmac: pci: RX queue routing configuration
From: Joao Pinto @ 2017-05-08 14:18 UTC (permalink / raw)
  To: Andy Shevchenko, Joao Pinto, Jan Kiszka, netdev,
	Giuseppe CAVALLARO, David S. Miller
In-Reply-To: <20170508141422.39612-4-andriy.shevchenko@linux.intel.com>

Às 3:14 PM de 5/8/2017, Andy Shevchenko escreveu:
> The commit abe80fdc6ee6
> 
>     ("net: stmmac: RX queue routing configuration")
> 
> missed Intel Quark configuration. Append it here.
> 
> Fixes: abe80fdc6ee6 ("net: stmmac: RX queue routing configuration")
> Cc: Joao Pinto <Joao.Pinto@synopsys.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> index c015a715a8ac..2456e0a945ef 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> @@ -153,6 +153,9 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
>  	plat->tx_queues_cfg[0].use_prio = false;
>  	plat->rx_queues_cfg[0].use_prio = false;
>  
> +	/* Disable RX queues routing by default */
> +	plat->rx_queues_cfg[0].pkt_route = 0x0;
> +
>  	return 0;
>  }
>  
> 

Acked-by: Joao Pinto <jpinto@synopsys.com>

^ permalink raw reply

* Re: [PATCH v1 4/4] stmmac: pci: split out common_default_data() helper
From: Joao Pinto @ 2017-05-08 14:19 UTC (permalink / raw)
  To: Andy Shevchenko, Joao Pinto, Jan Kiszka, netdev,
	Giuseppe CAVALLARO, David S. Miller
In-Reply-To: <20170508141422.39612-5-andriy.shevchenko@linux.intel.com>

Às 3:14 PM de 5/8/2017, Andy Shevchenko escreveu:
> New helper is added in order to prevent misconfiguration happened
> for one of the platforms when configuration data is expanded.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 52 ++++++++----------------
>  1 file changed, 18 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> index 2456e0a945ef..22f910795be4 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> @@ -70,11 +70,8 @@ static int stmmac_pci_find_phy_addr(struct stmmac_pci_info *info)
>  	return -ENODEV;
>  }
>  
> -static void stmmac_default_data(struct plat_stmmacenet_data *plat)
> +static void common_default_data(struct plat_stmmacenet_data *plat)
>  {
> -	plat->bus_id = 1;
> -	plat->phy_addr = 0;
> -	plat->interface = PHY_INTERFACE_MODE_GMII;
>  	plat->clk_csr = 2;	/* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */
>  	plat->has_gmac = 1;
>  	plat->force_sf_dma_mode = 1;
> @@ -82,10 +79,6 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat)
>  	plat->mdio_bus_data->phy_reset = NULL;
>  	plat->mdio_bus_data->phy_mask = 0;
>  
> -	plat->dma_cfg->pbl = 32;
> -	plat->dma_cfg->pblx8 = true;
> -	/* TODO: AXI */
> -
>  	/* Set default value for multicast hash bins */
>  	plat->multicast_filter_bins = HASH_TABLE_SIZE;
>  
> @@ -107,12 +100,29 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat)
>  	plat->rx_queues_cfg[0].pkt_route = 0x0;
>  }
>  
> +static void stmmac_default_data(struct plat_stmmacenet_data *plat)
> +{
> +	/* Set common default data first */
> +	common_default_data(plat);
> +
> +	plat->bus_id = 1;
> +	plat->phy_addr = 0;
> +	plat->interface = PHY_INTERFACE_MODE_GMII;
> +
> +	plat->dma_cfg->pbl = 32;
> +	plat->dma_cfg->pblx8 = true;
> +	/* TODO: AXI */
> +}
> +
>  static int quark_default_data(struct plat_stmmacenet_data *plat,
>  			      struct stmmac_pci_info *info)
>  {
>  	struct pci_dev *pdev = info->pdev;
>  	int ret;
>  
> +	/* Set common default data first */
> +	common_default_data(plat);
> +
>  	/*
>  	 * Refuse to load the driver and register net device if MAC controller
>  	 * does not connect to any PHY interface.
> @@ -124,38 +134,12 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
>  	plat->bus_id = PCI_DEVID(pdev->bus->number, pdev->devfn);
>  	plat->phy_addr = ret;
>  	plat->interface = PHY_INTERFACE_MODE_RMII;
> -	plat->clk_csr = 2;
> -	plat->has_gmac = 1;
> -	plat->force_sf_dma_mode = 1;
> -
> -	plat->mdio_bus_data->phy_reset = NULL;
> -	plat->mdio_bus_data->phy_mask = 0;
>  
>  	plat->dma_cfg->pbl = 16;
>  	plat->dma_cfg->pblx8 = true;
>  	plat->dma_cfg->fixed_burst = 1;
>  	/* AXI (TODO) */
>  
> -	/* Set default value for multicast hash bins */
> -	plat->multicast_filter_bins = HASH_TABLE_SIZE;
> -
> -	/* Set default value for unicast filter entries */
> -	plat->unicast_filter_entries = 1;
> -
> -	/* Set the maxmtu to a default of JUMBO_LEN */
> -	plat->maxmtu = JUMBO_LEN;
> -
> -	/* Set default number of RX and TX queues to use */
> -	plat->tx_queues_to_use = 1;
> -	plat->rx_queues_to_use = 1;
> -
> -	/* Disable Priority config by default */
> -	plat->tx_queues_cfg[0].use_prio = false;
> -	plat->rx_queues_cfg[0].use_prio = false;
> -
> -	/* Disable RX queues routing by default */
> -	plat->rx_queues_cfg[0].pkt_route = 0x0;
> -
>  	return 0;
>  }
>  
> 

Makes sense!

Acked-by: Joao Pinto <jpinto@synopsys.com>

^ permalink raw reply

* Re: [PATCH v1 1/4] stmmac: pci: set default number of rx and tx queues
From: Giuseppe CAVALLARO @ 2017-05-08 14:22 UTC (permalink / raw)
  To: Andy Shevchenko, Joao Pinto, Jan Kiszka, netdev, David S. Miller
In-Reply-To: <20170508141422.39612-2-andriy.shevchenko@linux.intel.com>

On 5/8/2017 4:14 PM, Andy Shevchenko wrote:
> The commit 26d6851fd24e
>
> 	("net: stmmac: set default number of rx and tx queues in stmmac_pci")
>
> missed Intel Quark configuration. Append it here.
>
> Fixes: 26d6851fd24e ("net: stmmac: set default number of rx and tx queues in stmmac_pci")
> Cc: Joao Pinto <Joao.Pinto@synopsys.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> index 39be96779145..ae3e836f9bb6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> @@ -145,6 +145,10 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
>   	/* Set the maxmtu to a default of JUMBO_LEN */
>   	plat->maxmtu = JUMBO_LEN;
>   
> +	/* Set default number of RX and TX queues to use */
> +	plat->tx_queues_to_use = 1;
> +	plat->rx_queues_to_use = 1;
> +
>   	return 0;
>   }
>   

For the Series, please consider my

Acked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>


thx a lot

^ permalink raw reply

* Re: [PATCH] net: ethernet: stmmac: properly set PS bit in MII configurations during reset
From: Giuseppe CAVALLARO @ 2017-05-08 14:28 UTC (permalink / raw)
  To: Corentin Labbe; +Cc: Thomas Petazzoni, Alexandre Torgue, netdev, stable
In-Reply-To: <20170503143032.GA22106@Red>

Hello

On 5/3/2017 4:30 PM, Corentin Labbe wrote:
> On Wed, May 03, 2017 at 10:13:56AM +0200, Giuseppe CAVALLARO wrote:
>> Hello Thomas
>>
>> this was initially set by using the hw->link.port; both the core_init
>> and adjust callback
>> should invoke the hook and tuning the PS bit according to the speed and
>> mode.
>> So maybe the ->set_ps is superfluous and you could reuse the existent hook
>>
>> let me know
>>
>> Regards
>> peppe
>>
> I just see that GMAC_CONTROL and MAC_CTRL_REG are the same, so why not create a custom adjust_link for each dwmac type ?
> This will permit to call it instead of set_ps() and remove lots of if (has_gmac) and co in stmmac_adjust_link()
> Basicly replace all between "ctrl = readl()... and writel(ctrl)" by a sot of priv->hw->mac->adjust_link()
>
> It will also help a lot for my dwmac-sun8i inclusion (since I add some if has_sun8i:))

Corentin, I think this is a good idea and maybe necessary now that the 
driver is supporting a lot of chips.
In the past it was sufficient to have a adjust link function and a 
stmmac_hw_fix_mac_speed
to invoke dedicated hook shared between MAC10/100 and GMAC inside STM 
platforms.

Thomas, I wonder if you could take a look at the 
priv->plat->fix_mac_speed. This can be used
for setting internal registers too.

Regards
Peppe

>
> Regards
>
>> On 4/27/2017 11:45 AM, Thomas Petazzoni wrote:
>>> On the SPEAr600 SoC, which has the dwmac1000 variant of the IP block,
>>> the DMA reset never succeeds when a MII PHY is used (no problem with a
>>> GMII PHY). The dwmac_dma_reset() function sets the
>>> DMA_BUS_MODE_SFT_RESET bit in the DMA_BUS_MODE register, and then
>>> polls until this bit clears. When a MII PHY is used, with the current
>>> driver, this bit never clears and the driver therefore doesn't work.
>>>
>>> The reason is that the PS bit of the GMAC_CONTROL register should be
>>> correctly configured for the DMA reset to work. When the PS bit is 0,
>>> it tells the MAC we have a GMII PHY, when the PS bit is 1, it tells
>>> the MAC we have a MII PHY.
>>>
>>> Doing a DMA reset clears all registers, so the PS bit is cleared as
>>> well. This makes the DMA reset work fine with a GMII PHY. However,
>>> with MII PHY, the PS bit should be set.
>>>
>>> We have identified this issue thanks to two SPEAr600 platform:
>>>
>>>    - One equipped with a GMII PHY, with which the existing driver was
>>>      working fine.
>>>
>>>    - One equipped with a MII PHY, where the current driver fails because
>>>      the DMA reset times out.
>>>
>>> This patch fixes the problem for the MII PHY configuration, and has
>>> been tested with a GMII PHY configuration as well.
>>>
>>> In terms of implement, since the ->reset() hook is implemented in the
>>> DMA related code, we do not want to touch directly from this function
>>> the MAC registers. Therefore, a ->set_ps() hook has been added to
>>> stmmac_ops, which gets called between the moment the reset is asserted
>>> and the polling loop waiting for the reset bit to clear.
>>>
>>> In order for this ->set_ps() hook to decide what to do, we pass it the
>>> "struct mac_device_info" so it can access the MAC registers, and the
>>> PHY interface type so it knows if we're using a MII PHY or not.
>>>
>>> The ->set_ps() hook is only implemented for the dwmac1000 case.
>>>
>>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>>> Cc: <stable@vger.kernel.org>
>>> ---
>>> Do not hesitate to suggest ideas for alternative implementations, I'm
>>> not sure if the current proposal is the one that fits best with the
>>> current design of the driver.
>>> ---
>>>    drivers/net/ethernet/stmicro/stmmac/common.h         | 12 +++++++++---
>>>    drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 16 ++++++++++++++++
>>>    drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h     |  3 ++-
>>>    drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c     |  7 ++++++-
>>>    drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h      |  3 ++-
>>>    drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c      |  6 +++++-
>>>    drivers/net/ethernet/stmicro/stmmac/stmmac_main.c    |  3 ++-
>>>    7 files changed, 42 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
>>> index 04d9245..d576f95 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
>>> @@ -407,10 +407,13 @@ struct stmmac_desc_ops {
>>>    extern const struct stmmac_desc_ops enh_desc_ops;
>>>    extern const struct stmmac_desc_ops ndesc_ops;
>>>    
>>> +struct mac_device_info;
>>> +
>>>    /* Specific DMA helpers */
>>>    struct stmmac_dma_ops {
>>>    	/* DMA core initialization */
>>> -	int (*reset)(void __iomem *ioaddr);
>>> +	int (*reset)(void __iomem *ioaddr, struct mac_device_info *hw,
>>> +		     phy_interface_t interface);
>>>    	void (*init)(void __iomem *ioaddr, struct stmmac_dma_cfg *dma_cfg,
>>>    		     u32 dma_tx, u32 dma_rx, int atds);
>>>    	/* Configure the AXI Bus Mode Register */
>>> @@ -445,12 +448,15 @@ struct stmmac_dma_ops {
>>>    	void (*enable_tso)(void __iomem *ioaddr, bool en, u32 chan);
>>>    };
>>>    
>>> -struct mac_device_info;
>>> -
>>>    /* Helpers to program the MAC core */
>>>    struct stmmac_ops {
>>>    	/* MAC core initialization */
>>>    	void (*core_init)(struct mac_device_info *hw, int mtu);
>>> +	/* Set port select. Called between asserting DMA reset and
>>> +	 * waiting for the reset bit to clear.
>>> +	 */
>>> +	void (*set_ps)(struct mac_device_info *hw,
>>> +		       phy_interface_t interface);
>>>    	/* Enable and verify that the IPC module is supported */
>>>    	int (*rx_ipc)(struct mac_device_info *hw);
>>>    	/* Enable RX Queues */
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>> index 19b9b308..dfcbb5b 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
>>> @@ -75,6 +75,21 @@ static void dwmac1000_core_init(struct mac_device_info *hw, int mtu)
>>>    #endif
>>>    }
>>>    
>>> +static void dwmac1000_set_ps(struct mac_device_info *hw,
>>> +			     phy_interface_t interface)
>>> +{
>>> +	void __iomem *ioaddr = hw->pcsr;
>>> +	u32 value = readl(ioaddr + GMAC_CONTROL);
>>> +
>>> +	/* When a MII PHY is used, we must set the PS bit for the DMA
>>> +	 * reset to succeed.
>>> +	 */
>>> +	if (interface == PHY_INTERFACE_MODE_MII)
>>> +		value |= GMAC_CONTROL_PS;
>>> +
>>> +	writel(value, ioaddr + GMAC_CONTROL);
>>> +}
>>> +
>>>    static int dwmac1000_rx_ipc_enable(struct mac_device_info *hw)
>>>    {
>>>    	void __iomem *ioaddr = hw->pcsr;
>>> @@ -488,6 +503,7 @@ static void dwmac1000_debug(void __iomem *ioaddr, struct stmmac_extra_stats *x)
>>>    
>>>    static const struct stmmac_ops dwmac1000_ops = {
>>>    	.core_init = dwmac1000_core_init,
>>> +	.set_ps = dwmac1000_set_ps,
>>>    	.rx_ipc = dwmac1000_rx_ipc_enable,
>>>    	.dump_regs = dwmac1000_dump_regs,
>>>    	.host_irq_status = dwmac1000_irq_status,
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h
>>> index 1b06df7..e9c6c49 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h
>>> @@ -183,7 +183,8 @@
>>>    #define DMA_CHAN0_DBG_STAT_RPS		GENMASK(11, 8)
>>>    #define DMA_CHAN0_DBG_STAT_RPS_SHIFT	8
>>>    
>>> -int dwmac4_dma_reset(void __iomem *ioaddr);
>>> +int dwmac4_dma_reset(void __iomem *ioaddr, struct mac_device_info *hw,
>>> +		     phy_interface_t interface);
>>>    void dwmac4_enable_dma_transmission(void __iomem *ioaddr, u32 tail_ptr);
>>>    void dwmac4_enable_dma_irq(void __iomem *ioaddr);
>>>    void dwmac410_enable_dma_irq(void __iomem *ioaddr);
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
>>> index c7326d5..485eecb 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
>>> @@ -14,7 +14,8 @@
>>>    #include "dwmac4_dma.h"
>>>    #include "dwmac4.h"
>>>    
>>> -int dwmac4_dma_reset(void __iomem *ioaddr)
>>> +int dwmac4_dma_reset(void __iomem *ioaddr, struct mac_device_info *hw,
>>> +		     phy_interface_t interface)
>>>    {
>>>    	u32 value = readl(ioaddr + DMA_BUS_MODE);
>>>    	int limit;
>>> @@ -22,6 +23,10 @@ int dwmac4_dma_reset(void __iomem *ioaddr)
>>>    	/* DMA SW reset */
>>>    	value |= DMA_BUS_MODE_SFT_RESET;
>>>    	writel(value, ioaddr + DMA_BUS_MODE);
>>> +
>>> +	if (hw->mac->set_ps)
>>> +		hw->mac->set_ps(hw, interface);
>>> +
>>>    	limit = 10;
>>>    	while (limit--) {
>>>    		if (!(readl(ioaddr + DMA_BUS_MODE) & DMA_BUS_MODE_SFT_RESET))
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h b/drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h
>>> index 56e485f..25ae028 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h
>>> @@ -144,6 +144,7 @@ void dwmac_dma_stop_tx(void __iomem *ioaddr);
>>>    void dwmac_dma_start_rx(void __iomem *ioaddr);
>>>    void dwmac_dma_stop_rx(void __iomem *ioaddr);
>>>    int dwmac_dma_interrupt(void __iomem *ioaddr, struct stmmac_extra_stats *x);
>>> -int dwmac_dma_reset(void __iomem *ioaddr);
>>> +int dwmac_dma_reset(void __iomem *ioaddr, struct mac_device_info *hw,
>>> +		    phy_interface_t interface);
>>>    
>>>    #endif /* __DWMAC_DMA_H__ */
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
>>> index e60bfca..1a17df5 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
>>> @@ -23,7 +23,8 @@
>>>    
>>>    #define GMAC_HI_REG_AE		0x80000000
>>>    
>>> -int dwmac_dma_reset(void __iomem *ioaddr)
>>> +int dwmac_dma_reset(void __iomem *ioaddr, struct mac_device_info *hw,
>>> +		    phy_interface_t interface)
>>>    {
>>>    	u32 value = readl(ioaddr + DMA_BUS_MODE);
>>>    	int err;
>>> @@ -32,6 +33,9 @@ int dwmac_dma_reset(void __iomem *ioaddr)
>>>    	value |= DMA_BUS_MODE_SFT_RESET;
>>>    	writel(value, ioaddr + DMA_BUS_MODE);
>>>    
>>> +	if (hw->mac->set_ps)
>>> +		hw->mac->set_ps(hw, interface);
>>> +
>>>    	err = readl_poll_timeout(ioaddr + DMA_BUS_MODE, value,
>>>    				 !(value & DMA_BUS_MODE_SFT_RESET),
>>>    				 100000, 10000);
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> index 4498a38..66bc218 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> @@ -1585,7 +1585,8 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
>>>    	if (priv->extend_desc && (priv->mode == STMMAC_RING_MODE))
>>>    		atds = 1;
>>>    
>>> -	ret = priv->hw->dma->reset(priv->ioaddr);
>>> +	ret = priv->hw->dma->reset(priv->ioaddr, priv->hw,
>>> +				   priv->plat->interface);
>>>    	if (ret) {
>>>    		dev_err(priv->device, "Failed to reset the dma\n");
>>>    		return ret;
>>

^ permalink raw reply

* Re: [PATCH 1/2] PCI: Add new PCIe Fabric End Node flag, PCI_DEV_FLAGS_NO_RELAXED_ORDERING
From: Ding Tianhong @ 2017-05-08 14:33 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Mark Rutland, Gabriele Paoloni, Asit K Mallick, Catalin Marinas,
	Will Deacon, LinuxArm, Raj, Ashok, Bjorn Helgaas, Ganesh GR,
	Jeff Kirsher, Bob Shaw, Casey Leedom, Patrick J Cramer, Arjun V.,
	Michael Werner, linux-arm-kernel@lists.infradead.org, Amir Ancel,
	Netdev, David Laight, Suravee Suthikulpanit, Robin Murphy,
	David Miller, h
In-Reply-To: <CAKgT0Ufg6uebqj79e1FOHoPpF-GxPYMkp353r3Ok_=nM8wF6qA@mail.gmail.com>



On 2017/5/7 2:07, Alexander Duyck wrote:
> On Fri, May 5, 2017 at 8:08 PM, Ding Tianhong <dingtianhong@huawei.com> wrote:
>>
>>
>> On 2017/5/5 22:04, Alexander Duyck wrote:
>>> On Thu, May 4, 2017 at 2:01 PM, Casey Leedom <leedom@chelsio.com> wrote:
>>>> | From: Alexander Duyck <alexander.duyck@gmail.com>
>>>> | Sent: Wednesday, May 3, 2017 9:02 AM
>>>> | ...
>>>> | It sounds like we are more or less in agreement. My only concern is
>>>> | really what we default this to. On x86 I would say we could probably
>>>> | default this to disabled for existing platforms since my understanding
>>>> | is that relaxed ordering doesn't provide much benefit on what is out
>>>> | there right now when performing DMA through the root complex. As far
>>>> | as peer-to-peer I would say we should probably look at enabling the
>>>> | ability to have Relaxed Ordering enabled for some channels but not
>>>> | others. In those cases the hardware needs to be smart enough to allow
>>>> | for you to indicate you want it disabled by default for most of your
>>>> | DMA channels, and then enabled for the select channels that are
>>>> | handling the peer-to-peer traffic.
>>>>
>>>>   Yes, I think that we are mostly in agreement.  I had just wanted to make
>>>> sure that whatever scheme was developed would allow for simultaneously
>>>> supporting non-Relaxed Ordering for some PCIe End Points and Relaxed
>>>> Ordering for others within the same system.  I.e. not simply
>>>> enabling/disabling/etc.  based solely on System Platform Architecture.
>>>>
>>>>   By the way, I've started our QA folks off looking at what things look like
>>>> in Linux Virtual Machines under different Hypervisors to see what
>>>> information they may provide to the VM in the way of what Root Complex Port
>>>> is being used, etc.  So far they've got Windows HyperV done and there
>>>> there's no PCIe Fabric exposed in any way: just the attached device.  I'll
>>>> have to see what pci_find_pcie_root_port() returns in that environment.
>>>> Maybe NULL?
>>>
>>> I believe NULL is one of the options. It all depends on what qemu is
>>> emulating. Most likely you won't find a pcie root port on KVM because
>>> the default is to emulate an older system that only supports PCI.
>>>
>>>>   With your reservations (which I also share), I think that it probably
>>>> makes sense to have a per-architecture definition of the "Can I Use Relaxed
>>>> Ordering With TLPs Directed At This End Point" predicate, with the default
>>>> being "No" for any architecture which doesn't implement the predicate.  And
>>>> if the specified (struct pci_dev *) End Node is NULL, it ought to return
>>>> False for that as well.  I can't see any reason to pass in the Source End
>>>> Node but I may be missing something.
>>>>
>>>>   At this point, this is pretty far outside my level of expertise.  I'm
>>>> happy to give it a go, but I'd be even happier if someone with a lot more
>>>> experience in the PCIe Infrastructure were to want to carry the ball
>>>> forward.  I'm not super familiar with the Linux Kernel "Rules Of
>>>> Engagement", so let me know what my next step should be.  Thanks.
>>>>
>>>> Casey
>>>
>>> For now we can probably keep this on the linux-pci mailing list. Going
>>> that route is the most straight forward for now since step one is
>>> probably just making sure we are setting the relaxed ordering bit in
>>> the setups that make sense. I would say we could probably keep it
>>> simple. We just need to enable relaxed ordering by default for SPARC
>>> architectures, on most others we can probably default it to off.
>>>
>>
>> Casey, Alexander:
>>
>> Thanks for the wonderful discussion, it is more clearly that what to do next,
>> I agree that enable relaxed ordering by default only for SPARC and ARM64
>> is more safe for all the other platform, as no one want to break anything.
>>
>>> I believe this all had started as Ding Tianhong was hoping to enable
>>> this for the ARM architecture. That is the only one I can think of
>>> where it might be difficult to figure out which way to default as we
>>> were attempting to follow the same code that was enabled for SPARC and
>>> that is what started this tug-of-war about how this should be done.
>>> What we might do is take care of this in two phases. The first one
>>> enables the infrastructure generically but leaves it defaulted to off
>>> for everyone but SPARC. Then we can go through and start enabling it
>>> for other platforms such as some of those on ARM in the platforms that
>>> Ding Tianhong was working with.
>>>
>>
>> According the suggestion, I could only think of this code:
>>
>> @@ -3979,6 +3979,15 @@ static void quirk_tw686x_class(struct pci_dev *pdev)
>>  DECLARE_PCI_FIXUP_CLASS_EARLY(0x1797, 0x6869, PCI_CLASS_NOT_DEFINED, 8,
>>                               quirk_tw686x_class);
>>
>> +static void quirk_relaxedordering_disable(struct pci_dev *dev)
>> +{
>> + if (dev->vendor != PCI_VENDOR_ID_HUAWEI &&
>> +     dev->vendor != PCI_VENDOR_ID_SUN)
>> +         dev->dev_flags |= PCI_DEV_FLAGS_NO_RELAXED_ORDERING;
>> +}
>> +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_INTEL_ID, PCI_ANY_ID, PCI_CLASS_NOT_DEFINED, 8,
>> +                       quirk_relaxedordering_disable);
>> +
>>  /*
>>   * Per PCIe r3.0, sec 2.2.9, "Completion headers must supply the same
>>   * values for the Attribute as were supplied in the header of the
>>
>>
>> What do you think of it?
>>
>> Thanks
>> Ding
>>
> 
> This is a bit simplistic but it is a start.
> 
> The other bit I was getting at is that we need to update the core PCIe
> code so that when we configure devices and the root complex reports no
> support for relaxed ordering it should be clearing the relaxed
> ordering bits in the PCIe configuration registers on the upstream
> facing devices.

How about this:
rename the PCI_DEV_FLAGS_NO_RELAXED_ORDERIN to PCI_DEV_FLAGS_RELAXED_ORDERIN, only enable it
when pcie root configure if it support the RO mode, otherwise we will not set it to indicate
that the pcie dev did not support RO mode.

> 
> The last bit we need in all this is a way to allow for setups where
> peer-to-peer wants to perform relaxed ordering but for writes to the
> host we have to not use relaxed ordering. For that we need to enable a
> special case and that isn't handled right now in any of the solutions
> we have coded up so far.
> 

Sorry I am not clear of this way, can you explain more about this or give me
a special case, thanks a lot.

Ding

> - Alex
> 
> .
> 

^ permalink raw reply

* pull-request: mac80211 2017-05-08
From: Johannes Berg @ 2017-05-08 14:46 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-wireless

Hi Dave,

Thanks for merging the rate fix quickly the other day. I've got a
few more fixes lined up, so this time as a pull request.

Please pull and let me know if there's any problem.

Thanks,
johannes



The following changes since commit 4ac4d584886a4f47f8ff3bca0f32ff9a2987d3e5:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2017-05-04 12:26:43 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-davem-2017-05-08

for you to fetch changes up to 6406c91943a0f29b6e8786921aaa038663e08055:

  cfg80211: fix multi scheduled scan kernel-doc (2017-05-08 13:09:38 +0200)

----------------------------------------------------------------
A couple more fixes:
 * don't try to authenticate during reconfiguration, which causes
   drivers to get confused
 * fix a kernel-doc warning for a recently merged change
 * fix MU-MIMO group configuration (relevant only for monitor mode)
 * more rate flags fix: remove stray RX_ENC_FLAG_40MHZ
 * fix IBSS probe response allocation size

----------------------------------------------------------------
Johannes Berg (4):
      mac80211: properly remove RX_ENC_FLAG_40MHZ
      nl80211: correctly validate MU-MIMO groups
      mac80211: fix IBSS presp allocation size
      cfg80211: fix multi scheduled scan kernel-doc

Luca Coelho (1):
      mac80211: bail out from prep_connection() if a reconfig is ongoing

 drivers/net/wireless/ath/ath9k/ar9003_mac.c    | 2 +-
 drivers/net/wireless/ath/ath9k/mac.c           | 4 ++--
 drivers/net/wireless/intel/iwlegacy/4965-mac.c | 4 +++-
 drivers/net/wireless/intel/iwlwifi/dvm/rx.c    | 4 +++-
 drivers/net/wireless/mac80211_hwsim.c          | 8 +++++++-
 include/net/cfg80211.h                         | 2 +-
 include/net/mac80211.h                         | 2 --
 net/mac80211/ibss.c                            | 2 ++
 net/mac80211/mlme.c                            | 4 ++++
 net/wireless/nl80211.c                         | 4 ++--
 10 files changed, 25 insertions(+), 11 deletions(-)

^ permalink raw reply

* Re: [PATCH net-next ] tcp: add new parameter tcp_inherit_buffsize to control the initial buffer size for new passive connetion
From: Eric Dumazet @ 2017-05-08 14:46 UTC (permalink / raw)
  To: Shan Wei; +Cc: netdev, David Miller
In-Reply-To: <c42195b5-7c77-fb8d-9ea6-214ad762aab8@gmail.com>

On Mon, 2017-05-08 at 15:49 +0800, Shan Wei wrote:

> Indeed,  HZ=250.  What is the problem about autotuning with HZ=250?
> 

You might look at recent patches.

commit 645f4c6f2ebd040688cc2a5f626ffc909e66ccf2
("tcp: switch rcv_rtt_est and rcvq_space to high resolution timestamps")

Thanks.

^ permalink raw reply

* Re: [PATCH net] tcp: init tcp_options before using it.
From: Eric Dumazet @ 2017-05-08 14:49 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: netdev
In-Reply-To: <1494237477-19023-1-git-send-email-liuhangbin@gmail.com>

On Mon, 2017-05-08 at 17:57 +0800, Hangbin Liu wrote:
> I searched 4308fc58dced ("tcp: Document use of undefined variable") in
> archive list, but did not find the thread. So I'm not sure why we only
> add a description about un-initialized value.
> 
> Even we don't use tmp_opt.sack_ok, I think it would be more safe to
> initialize the value before using it. Just as other caller did.

Patch is not needed at all.

Comment and code are pretty clear.

This part of the code uses a generic function ( tcp_parse_options()) to
decode TCP options, but we are only caring about TS one.

^ permalink raw reply

* Re: [PATCH net] ipv6: make sure dev is not NULL before call ip6_frag_reasm
From: Eric Dumazet @ 2017-05-08 14:50 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: netdev
In-Reply-To: <1494212964-17861-1-git-send-email-liuhangbin@gmail.com>

On Mon, 2017-05-08 at 11:09 +0800, Hangbin Liu wrote:
> Since ip6_frag_reasm() will call __in6_dev_get(dev), which will access
> dev->ip6_ptr. We need to make sure dev is not NULL.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  net/ipv6/reassembly.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> index e1da5b8..e3ebd62 100644
> --- a/net/ipv6/reassembly.c
> +++ b/net/ipv6/reassembly.c
> @@ -348,7 +348,7 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
>  		fq->q.flags |= INET_FRAG_FIRST_IN;
>  	}
>  
> -	if (fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) &&
> +	if (dev && fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) &&
>  	    fq->q.meat == fq->q.len) {
>  		int res;
>  		unsigned long orefdst = skb->_skb_refdst;


How dev could be possibly NULL here ?

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox