RE: Microchip KSZ* DSA drivers Re: [PATCH v1 RFC 1/1] Add Microchip KSZ8795 DSA driver

[Tristram.Ha]

> Subject: Microchip KSZ* DSA drivers Re: [PATCH v1 RFC 1/1] Add Microchip
> KSZ8795 DSA driver
> 
> Hi!
> 
> Are there any news here? Is there new release planned? Is there a git
> tree somewhere? I probably should get it working, soon.. so I guess I
> can help with testing.
> 

Reviewed patches will be submitted formally to net-next within this week.

BTW, how is the KSZ8895 driver working for you?  Are there some issues that
prevent you from using it?

RE: [PATCH v1 RFC 1/1] Add Microchip KSZ8795 DSA driver

[Tristram.Ha]

> > +static void ksz8795_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
> > + u64 *cnt)
> > +{
> > +   u32 data;
> > +   u16 ctrl_addr;
> > +   u8 check;
> > +   int loop;
> > +
> > +   ctrl_addr = addr + SWITCH_COUNTER_NUM * port;
> > +   ctrl_addr |= IND_ACC_TABLE(TABLE_MIB | TABLE_READ);
> > +
> > +   mutex_lock(&dev->alu_mutex);
> > +   ksz_write16(dev, REG_IND_CTRL_0, ctrl_addr);
> > +
> > +   /* It is almost guaranteed to always read the valid bit because of
> > +* slow SPI speed.
> > +*/
> > +   for (loop = 2; loop > 0; loop--) {
> > +   ksz_read8(dev, REG_IND_MIB_CHECK, &check);
> > +
> > +   if (check & MIB_COUNTER_VALID) {
> > +   ksz_read32(dev, REG_IND_DATA_LO, &data);
> > +   if (check & MIB_COUNTER_OVERFLOW)
> > +   *cnt += MIB_COUNTER_VALUE + 1;
> > +   *cnt += data & MIB_COUNTER_VALUE;
> > +   break;
> > +   }
> > +   }
> 
> Hmm. Maybe, but should not this at least warn if if it can not get
> valid counter?
>

The checking of valid bit is implemented because of the chip datasheet.
But in my experience it never happens.   A warning will be added, although I
do not see any benefit of it.  It this warning ever comes up it just means
somehow the SPI access is completely broken down.
 
> > +   /* It is almost guaranteed to always read the valid bit because of
> > +* slow SPI speed.
> > +*/
> > +   for (loop = 2; loop > 0; loop--) {
> > +   ksz_read8(dev, REG_IND_MIB_CHECK, &check);
> > +
> > +   if (check & MIB_COUNTER_VALID) {
> > +   ksz_read32(dev, REG_IND_DATA_LO, &data);
> > +   if (addr < 2) {
> > +   u64 total;
> > +
> > +   total = check & MIB_TOTAL_BYTES_H;
> > +   total <<= 32;
> > +   *cnt += total;
> > +   *cnt += data;
> > +   if (check & MIB_COUNTER_OVERFLOW) {
> > +   total = MIB_TOTAL_BYTES_H + 1;
> > +   total <<= 32;
> > +   *cnt += total;
> > +   }
> > +   } else {
> > +   if (check & MIB_COUNTER_OVERFLOW)
> > +   *cnt += MIB_PACKET_DROPPED + 1;
> > +   *cnt += data & MIB_PACKET_DROPPED;
> > +   }
> > +   break;
> > +   }
> > +   }
> 
> Same here. Plus, is overflow handling correct? There may be more than
> MIB_PACKET_DROPPED + 1 packets dropped between the checks.
> 

MIB_PACKET_DROPPED is the maximum count like 0x.  Plus 1 means 0x1.
It is assumed the checking should be done fast enough  to avoid overflow.  This 
is
just assuming one overflow has been missed.  To tell you the truth this code is
never checked for correctness.

> > +static void ksz8795_r_table(struct ksz_device *dev, int table, u16 addr,
> > +   u64 *data)
> > +{
> > +   u16 ctrl_addr;
> > +
> > +   ctrl_addr = IND_ACC_TABLE(table | TABLE_READ) | addr;
> > +
> > +   mutex_lock(&dev->alu_mutex);
> > +   ksz_write16(dev, REG_IND_CTRL_0, ctrl_addr);
> > +   ksz_get(dev, REG_IND_DATA_HI, data, sizeof(u64));
> > +   mutex_unlock(&dev->alu_mutex);
> > +   *data = be64_to_cpu(*data);
> > +}
> 
> It would be a tiny bit nicer to have be64 temporary variable and use
> it; having *data change endianness at runtime is "interesting".

> > +   /* At least one valid entry in the table. */
> > +   } else {
> > +   u64 buf;
> > +   int cnt;
> > +
> > +   ksz_get(dev, REG_IND_DATA_HI, &buf, sizeof(buf));
> > +   buf = be64_to_cpu(buf);
> 
> Would it make sense to convert endianness inside ksz_get?

The ksz_get function utilizes the chip's capability to automatically
increase the register index so that their values can be retrieved in
one SPI transfer.  It is mostly used for debugging for dumping chip
registers or programming MAC address (6 bytes).  Here it is also used for
reading 8 bytes.  If you do not want to see the conversion then another
access functions like ksz_read64 and ksz_write64 will need to be implemented.

RE: [PATCH v1 RFC 7/7] Modify tag_ksz.c so that tail tag code can be used by other KSZ switch drivers

[Tristram.Ha]

> > +#define KSZ9477_INGRESS_TAG_LEN2
> > +#define KSZ9477_PTP_TAG_LEN4
> > +#define KSZ9477_PTP_TAG_INDICATION 0x80
> > +
> > +#define KSZ9477_TAIL_TAG_OVERRIDE  BIT(9)
> > +#define KSZ9477_TAIL_TAG_LOOKUPBIT(10)
> > +
> > +static int ksz9477_get_tag(u8 *tag, int *port)
> > +{
> > +   int len = KSZ_EGRESS_TAG_LEN;
> > +
> > +   /* Extra 4-bytes PTP timestamp */
> > +   if (tag[0] & KSZ9477_PTP_TAG_INDICATION)
> > +   len += KSZ9477_PTP_TAG_LEN;
> > +   *port = tag[0] & 7;
> > +   return len;
> > +}
> > +
> > +static void ksz9477_set_tag(void *ptr, u8 *addr, int p)
> > +{
> > +   u16 *tag = (u16 *)ptr;
> > +
> > +   *tag = 1 << p;
> > +   if (!memcmp(addr, special_mult_addr, ETH_ALEN))
> > +   *tag |= KSZ9477_TAIL_TAG_OVERRIDE;
> > +   *tag = cpu_to_be16(*tag);
> > +}
> 
> These are new features that were not there before, right?

Although it is the correct procedure, they will be removed until
this tail tag code has access to the main switch driver.

RE: [PATCH net-next v3 00/12] net: dsa: microchip: PTP support for KSZ956x

[Tristram.Ha]

> On Thursday, 19 November 2020, 00:40:18 CET, Vladimir Oltean wrote:
> > On Wed, Nov 18, 2020 at 09:30:01PM +0100, Christian Eggers wrote:
> > > This series adds support for PTP to the KSZ956x and KSZ9477 devices.
> > >
> > > There is only little documentation for PTP available on the data sheet
> > > [1] (more or less only the register reference). Questions to the
> > > Microchip support were seldom answered comprehensively or in
> reasonable
> > > time. So this is more or less the result of reverse engineering.
> >
> > [...]
> > One thing that should definitely not be part of this series though is
> > patch 11/12. Christian, given the conversation we had on your previous
> > patch:
> > https://lore.kernel.org/netdev/20201113025311.jpkplhmacjz6lkc5@skbuf/
> sorry, I didn't read that carefully enough. Some of the other requested
> changes
> were quite challenging for me. Additionally, finding the UDP checksum bug
> needed some time for identifying because I didn't recognize that when it got
> introduced.
> 
> > as well as the documentation patch that was submitted in the meantime:
> > https://lore.kernel.org/netdev/20201117213826.18235-1-
> a.fat...@pengutronix.de/
> I am not subscribed to the list.
> 
> > obviously you chose to completely disregard that. May we know why? How
> > are you even making use of the PTP_CLK_REQ_PPS feature?
> Of course I will drop that patch from the next series.

These are general comments about this PTP patch.

The initial proposal in tag_ksz.c is for the switch driver to provide callback 
functions
to handle receiving and transmitting.  Then each switch driver can be added to
process the tail tag in its own driver and leave tag_ksz.c unchanged.

It was rejected because of wanting to keep tag_ksz.c code and switch driver code
separate and concern about performance.

Now tag_ksz.c is filled with PTP code that is not relevant for other switches 
and will
need to be changed again when another switch driver with PTP function is added.

Can we implement that callback mechanism?

One issue with transmission with PTP enabled is that the tail tag needs to 
contain 4
additional bytes.  When the PTP function is off the bytes are not added.  This 
should
be monitored all the time.

The extra 4 bytes are only used for 1-step Pdelay_Resp.  It should contain the 
receive
timestamp of previous Pdelay_Req with latency adjusted.  The correction field in
Pdelay_Resp should be zero.  It may be a hardware bug to have wrong UDP checksum
when the message is sent.

I think the right implementation is for the driver to remember this receive 
timestamp
of Pdelay_Req and puts it in the tail tag when it sees a 1-step Pdelay_Resp is 
sent.

There is one more requirement that is a little difficult to do.  The calculated 
peer delay
needs to be programmed in hardware register, but the regular PTP stack has no 
way to
send that command.  I think the driver has to do its own calculation by 
snooping on the
Pdelay_Req/Pdelay_Resp/Pdelay_Resp_Follow_Up messages.

The receive and transmit latencies are different for different connected speed. 
 So the
driver needs to change them when the link changes.  For that reason the PTP 
stack
should not use its own latency values as generally the application does not 
care about
the linked speed.
 

RE: [PATCH net-next v3 00/12] net: dsa: microchip: PTP support for KSZ956x

[Tristram.Ha]

> Hi Microchip,
> 
> as ACL based blocking of PTP traffic seems not to work, I tried to install MAC
> based static lookup rules on the switch I successfully managed to block other
> non-PTP traffic, but for PTP the lookup table entry (see below) seems not to
> work. Incoming SYNC messages on port are still forwarded to port 2.
> 
> The table entry is based on the multicast MAC used for PTP. With PTP
> domains!=0
> there could be 128 possible MAC addresses that needs to blocked (but the
> switch
> has only 16 entries in the static table). Is there any way to block the whole
> PTP multicast address range (01:00:5E:00:01:81-01:00:5E:00:01:ff)? The data
> sheet
> mentions that the static address table can be used for multicast addresses,
> so there should be a way.
> 
> Alternatively, is there a hidden "disable TC" setting which disables the
> transparent clock entirely?

The 1588 PTP engine in the KSZ switches was designed to be controlled closely by
a PTP stack, so it is a little difficult to use when there is a layer of kernel 
support
between the application and the driver.

The default mode to use should be 1-step E2E where the switch acts as an E2E
Transparent Clock.

The 16-bit register 0x514 specifies the basic operation mode of the switch.

Bit 0 is for 1-step clock mode.
Bit 1 is for master mode, which should be off when the clock is acting as a 
master.
Bit 2 is for P2P mode.
Bit 7 stops the automatic forwarding and every PTP message goes to the host 
port.
This is the mode to use when the switch acts as a Boundary Clock or 2-step 
Clock.

When master mode is on Delay_Resp will not be forwarded to the host port.
When master mode is off Delay_Req will not be forwarded to the host port.

When P2P mode is off Pdelay_Req/Pdelay_Resp/Pdelay_Resp_Follow_Up will not
be forwarded to the host port.
When P2P mode is on those messages can be sent and received even though the port
Is closed for normal communication.

Bit 5 recognizes L2 PTP messages and the switch acts accordingly.
Bit 4 is for UDPv4 while bit 3 is for UDPv6.

It is rather pointless to actively filter certain PTP messages through other 
means.
It is better to leave the kernel PTP receive filter as coarse as possible.

When using P2P in 1-step clock mode the port id in the PTP header is 
automatically
changed by hardware to be the same as the real port, so it is useless to 
arbitrarily
use a different port id.  The original intent is to send 1 Pdelay_Req and 
receive
several Pdelay_Resp in each port.

The calculated peer delay from the port needs to be programmed to the port
register so that the Sync message can be compensated correctly while it travels
through the switches.  This poses a problem as the driver normally does not do 
the
calculation.

The 2-step clock mode avoids some of the mentioned issues.  However there are 
some
hardware bugs associated with this operation mode and it is not recommended to 
use.

For some profiles that require 2-step operation like gPTP there are ways to 
workaround.

For Sync it is quite simple to send Follow_Up after it even though the Sync 
contains the
transmit timestamp.  The Follow_Up just repeats that information.

For 2-step Pdelay_Resp it is harder as the hardware puts the turnaround time in
the correction field.  The driver workaround is to report the transmit timestamp
differently such that it is the same as Pdelay_Req receive timestamp so that 
the net
calculation of the peer delay is just the same as receiving 1-step Pdelay_Resp.

I will try my own implementation to see how these steps can be done.

RE: [PATCH 3/4] net: dsa: microchip: fix interrupt mask

[Tristram.Ha]

> On 27/08/2019 15:51, Andrew Lunn wrote:
> >
> > On Tue, Aug 27, 2019 at 12:31:09PM +0300, Razvan Stefanescu wrote:
> >> Global Interrupt Mask Register comprises of Lookup Engine (LUE)
> Interrupt
> >> Mask (bit 31) and GPIO Pin Output Trigger and Timestamp Unit Interrupt
> >> Mask (bit 29).
> >>
> >> This corrects LUE bit.
> >
> > Hi Razvan
> >
> > Is this a fix? Something that should be back ported to old kernels?
> 
> Hello,
> 
> During testing I did not observed any issues with the old value. So I am
> not sure how the switch is affected by the incorrect setting.
> 
> Maybe maintainers will be able to make a better assessment if this needs
> back-porting. And I will be happy to do it if it is necessary.
> 

I do not think the change has any effect as the interrupt handling is not 
implemented in the driver, unless I am mistaken and do not know about the new 
code.

Currently those 3 interrupts do not do anything that are required in normal 
operation.

The first one LUE_INT notifies the driver when there are learn/write fails in 
the MAC table.  This condition rarely happens unless the switch is going 
through stress test.  When this interrupt happens software cannot do anything 
to resolve the issue.  It may become a denial of service if the MAC table keeps 
triggering learn fail.

The second one is used by PTP code, which is not implemented.

The third one is triggered when register access space does not exist.  It is 
useful during development so driver knows it is accessing the wrong register.  
It can also become a denial of service if someone keeps accessing wrong 
registers.  But then that person can do anything with the chip.

RE: [PATCH 4/4] net: dsa: microchip: avoid hard-codded port count

[Tristram.Ha]

> Subject: [PATCH 4/4] net: dsa: microchip: avoid hard-codded port count
> 
> Use port_cnt value to disable interrupts on switch reset.
> 
> Signed-off-by: Razvan Stefanescu 
> ---
>  drivers/net/dsa/microchip/ksz9477.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz9477.c
> b/drivers/net/dsa/microchip/ksz9477.c
> index 187be42de5f1..54fc05595d48 100644
> --- a/drivers/net/dsa/microchip/ksz9477.c
> +++ b/drivers/net/dsa/microchip/ksz9477.c
> @@ -213,7 +213,7 @@ static int ksz9477_reset_switch(struct ksz_device
> *dev)
> 
>   /* disable interrupts */
>   ksz_write32(dev, REG_SW_INT_MASK__4, SWITCH_INT_MASK);
> - ksz_write32(dev, REG_SW_PORT_INT_MASK__4, 0x7F);
> + ksz_write32(dev, REG_SW_PORT_INT_MASK__4, dev->port_cnt);
>   ksz_read32(dev, REG_SW_PORT_INT_STATUS__4, &data32);
> 
>   /* set broadcast storm protection 10% rate */

The register value is a portmap, so using port_cnt may be wrong.

The chip is a 7-port switch.  There is a 6-port variant, but it is okay to 
write 0x7F.

There is also a 3-port variant which uses a different design.  It is a bit of 
stretch to use 0x7F on it.

It is more a code readability or correctness than incorrect hardware operation.

RE: [PATCH] DSA support for Micrel KSZ8895

[Tristram.Ha]

> On Mon 2017-08-28 16:09:27, Andrew Lunn wrote:
> > > I may be confused here, but AFAICT:
> > >
> > > 1) Yes, it has standard layout when accessed over MDIO.
> >
> >
> > Section 4.8 of the datasheet says:
> >
> > All the registers defined in this section can be also accessed
> > via the SPI interface.
> >
> > Meaning all PHY registers can be access via the SPI interface. So you
> > should be able to make a standard Linux MDIO bus driver which performs
> > SPI reads.
> 
> As far as I can tell (and their driver confirms) -- yes, all those registers 
> can be
> accessed over the SPI, they are just shuffled around... hence MDIO
> emulation code. I copied it from their code (see the copyrights) so no, I 
> don't
> believe there's nicer solution.
> 
> Best regards,
> 
>   Pavel

Can you hold on your developing work on KSZ8895 driver?  I am afraid your 
effort may be in vain.  We at Microchip are planning to release DSA drivers for 
all KSZ switches, starting at KSZ8795, then KSZ8895, and KSZ8863.

The driver files all follow the structures of the current KSZ9477 DSA driver, 
and the file tag_ksz.c will be updated to handle the tail tag of different 
chips, which requires including the ksz_priv.h header.  That is required 
nevertheless to support using the offload_fwd_mark indication.

The KSZ8795 driver will be submitted after Labor Day (9/4) if testing reveals 
no problem.  The KSZ8895 driver will be submitted right after that.  You should 
have no problem using the driver right away.

Tristram Ha
Principal Software Engineer
Microchip Technology Inc.

RE: [PATCH RFC 6/6] Modify tag_ksz.c to support other KSZ switch drivers

[Tristram.Ha]

Sorry for the late response.

> > diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c index 010ca0a..d5faf14
> 100644
> > --- a/net/dsa/tag_ksz.c
> > +++ b/net/dsa/tag_ksz.c
> >  static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device
> *dev)  {
> > struct dsa_slave_priv *p = netdev_priv(dev);
> > +   struct dsa_switch *ds = p->dp->ds;
> > +   struct ksz_device *swdev = ds->priv;
> > struct sk_buff *nskb;
> > +   int len;
> > int padlen;
> > -   u8 *tag;
> >
> > padlen = (skb->len >= ETH_ZLEN) ? 0 : ETH_ZLEN - skb->len;
> >
> > -   if (skb_tailroom(skb) >= padlen + KSZ_INGRESS_TAG_LEN) {
> > +   len = swdev->dev_ops->get_tx_len(swdev);
> 
> This is ugly. We have a clean separation between a switch driver and a
> tag driver. Here you are mixing them together. Don't. Look at the
> mailing lists for what Florian and I suggested to Pavel. It will also
> solve your include file issue above.

It seems to me all tag_*.c are hard-coded.  They do not have access to
the switch device and just use the bit definitions defined in the top to
do the job.

This creates a problem for all KSZ switch devices as they have different
tail tag formats and lengths.  There will be potentially 4 additional
DSA_TAG_PROT_KSZ* just to handle them.  Extra files will be added
with about the same code.

The KSZ9477 driver has its own problem as the tail tag length is dynamic
and not fixed.  Right now the function feature that affects this behavior is
not turned on and so the problem does not happen.

But the most important thing is how do we support the offload_fwd_mark
bit in skb when the only place that has access to skb does not have access to
the switch hardware?  I thought this bit is important for the new switchdev 
model.

A little out-of-topic is the MAC driver may have problem receiving the frame if
the tail tag length is too big.  Most of the MAC drivers have big enough receive
buffer or require 64-bytes multiple so there are extra room to accommodate
the big tail tag at the end of the frame.  Still some MAC drivers use exactly 
1500
MTU and some additional length and may run into this problem.  Currently the
Atmel Cadence MAC driver has this potential problem when certain conditions
are met.

RE: [PATCH RFC 3/5] Add KSZ8795 switch driver

[Tristram.Ha]

> > +/**
> > + * Some counters do not need to be read too often because they are less
> likely
> > + * to increase much.
> > + */
> 
> What does comment mean? Are you caching statistics, and updating
> different values at different rates?
> 

There are 34 counters.  In normal case using generic bus I/O or PCI to read them
is very quick, but the switch is mostly accessed using SPI, or even I2C.  As 
the SPI
access is very slow and cannot run in interrupt context I keep worrying reading
the MIB counters in a loop for 5 or more ports will prevent other critical 
hardware
access from executing soon enough.  These accesses can be getting 1588 PTP
timestamps and opening/closing ports.  (RSTP Conformance Test sends test traffic
to port supposed to be closed/opened after receiving specific RSTP BPDU.)

> > +static const struct {
> > +   int interval;
> > +   char string[ETH_GSTRING_LEN];
> > +} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
> > +   { 1, "rx_hi" },
> > +   { 4, "rx_undersize" },
> > +   { 4, "rx_fragments" },
> > +   { 4, "rx_oversize" },
> > +   { 4, "rx_jabbers" },
> > +   { 4, "rx_symbol_err" },
> > +   { 4, "rx_crc_err" },
> > +   { 4, "rx_align_err" },
> > +   { 4, "rx_mac_ctrl" },
> > +   { 1, "rx_pause" },
> > +   { 1, "rx_bcast" },
> > +   { 1, "rx_mcast" },
> > +   { 1, "rx_ucast" },
> > +   { 2, "rx_64_or_less" },
> > +   { 2, "rx_65_127" },
> > +   { 2, "rx_128_255" },
> > +   { 2, "rx_256_511" },
> > +   { 2, "rx_512_1023" },
> > +   { 2, "rx_1024_1522" },
> > +   { 2, "rx_1523_2000" },
> > +   { 2, "rx_2001" },
> > +   { 1, "tx_hi" },
> > +   { 4, "tx_late_col" },
> > +   { 1, "tx_pause" },
> > +   { 1, "tx_bcast" },
> > +   { 1, "tx_mcast" },
> > +   { 1, "tx_ucast" },
> > +   { 4, "tx_deferred" },
> > +   { 4, "tx_total_col" },
> > +   { 4, "tx_exc_col" },
> > +   { 4, "tx_single_col" },
> > +   { 4, "tx_mult_col" },
> > +   { 1, "rx_total" },
> > +   { 1, "tx_total" },
> > +   { 2, "rx_discards" },
> > +   { 2, "tx_discards" },
> > +};

> > +{
> > +   u32 data;
> > +   u16 ctrl_addr;
> > +   u8 check;
> > +   int timeout;
> > +
> > +   ctrl_addr = addr + SWITCH_COUNTER_NUM * port;
> > +
> > +   ctrl_addr |= IND_ACC_TABLE(TABLE_MIB | TABLE_READ);
> > +   mutex_lock(&dev->stats_mutex);
> > +   ksz_write16(dev, REG_IND_CTRL_0, ctrl_addr);
> > +
> > +   for (timeout = 1; timeout > 0; timeout--) {
> 
> A rather short timeount!
> 
> > +   ksz_read8(dev, REG_IND_MIB_CHECK, &check);
> > +
> > +   if (check & MIB_COUNTER_VALID) {
> > +   ksz_read32(dev, REG_IND_DATA_LO, &data);
> > +   if (check & MIB_COUNTER_OVERFLOW)
> > +   *cnt += MIB_COUNTER_VALUE + 1;
> > +   *cnt += data & MIB_COUNTER_VALUE;
> > +   break;
> > +   }
> 
> Should there be a dev_err() here about timing out?
> 

The timeout value should be at least 2.  Hardware datasheet says the
valid bit should be checked, but in my experience it is always there.
Maybe a very slow SPI access speed can make that happen, but it is
pointless to run in slow speed if the system can run in the fastest speed
possible.
 
> > +static void ksz_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
> > +{
> > +   struct ksz_port *port;
> > +   u8 ctrl;
> > +   u8 restart;
> > +   u8 link;
> > +   u8 speed;
> > +   u8 force;
> > +   u8 p = phy;
> > +   u16 data = 0;
> > +   int processed = true;
> > +
> > +   port = &dev->ports[p];
> > +   switch (reg) {
> > +   case PHY_REG_CTRL:
> > +   ksz_pread8(dev, p, P_LOCAL_CTRL, &ctrl);
> > +   ksz_pread8(dev, p, P_NEG_RESTART_CTRL, &restart);
> > +   ksz_pread8(dev, p, P_SPEED_STATUS, &speed);
> > +   ksz_pread8(dev, p, P_FORCE_CTRL, &force);
> > +   if (restart & PORT_PHY_LOOPBACK)
> > +   data |= PHY_LOOPBACK;
> > +   if (force & PORT_FORCE_100_MBIT)
> > +   data |= PHY_SPEED_100MBIT;
> > +   if (!(force & PORT_AUTO_NEG_DISABLE))
> > +   data |= PHY_AUTO_NEG_ENABLE;
> > +   if (restart & PORT_POWER_DOWN)
> > +   data |= PHY_POWER_DOWN;
> 
> Same questions i asked Pavel
> 
> Is there a way to access the PHY registers over SPI? The datasheet
> suggests there is, but it does not say how, as far as i can tell.
> 
> Ideally, we want to use just a normal PHY driver.
>

The switch does not have actual PHY registers when used in SPI mode,
so the PHY register access has to be simulated.

A side-note is the PHY device id returned will pair the PHY with one of
the Micrel PHYs defined in the kernel.  I just found out the new phylib
code removed the Asymmetric/Symmetric Pause support in the PHY
driver and let the MAC driver indicate the support.  This is reasonable
as the MAC is responsible for sending and processing PAUSE frames.
However, the PHY in the switch port is not connected to the MAC.
Is there a way to indicate this support in the DSA model?

Ideally the switch should have c

RE: [PATCH RFC 6/6] Modify tag_ksz.c to support other KSZ switch drivers

[Tristram.Ha]

> > > This is ugly. We have a clean separation between a switch driver and a
> > > tag driver. Here you are mixing them together. Don't. Look at the
> > > mailing lists for what Florian and I suggested to Pavel. It will also
> > > solve your include file issue above.
> >
> > It seems to me all tag_*.c are hard-coded.  They do not have access to
> > the switch device and just use the bit definitions defined in the top to
> > do the job.
> >
> > This creates a problem for all KSZ switch devices as they have different
> > tail tag formats and lengths.  There will be potentially 4 additional
> > DSA_TAG_PROT_KSZ* just to handle them.  Extra files will be added
> > with about the same code.
> 
> Hi Tristram
> 
> Think about factoring out the common code into a shared functions.
>

I am a little unsure what you have in mind.  Can you elaborate?

Like creating an additional ksz_xmit function and passing a function pointer
to it to do the job?
 
> > But the most important thing is how do we support the offload_fwd_mark
> > bit in skb when the only place that has access to skb does not have access
> to
> > the switch hardware?
> 
> Well, so far, nothing has set offload_fwd_mark. This is about to
> change, since i need it for IGMP snooping on the brX interface, for
> Marvell at least. Bit i just need to set it to true for all frames.
> 
> What use cases do you have in mind where you need information from the
> switch driver to decide on its value?
>

In the old DSA implementation all the ports are partitioned into its own device
and the bridge joining them will do all the forwarding.  This is useful for 
quick
testing with some protocols like RSTP but it is probably useless for real
operation.

The new switchdev model tries to use the switch hardware as much as
possible.  This offload_fwd_mark bit means the frame is forwarded by the
hardware switch, so the software bridge does not need to do it again.  Without
this bit there will be duplicated multicast frames coming out the ports if 
internal
forwarding is enabled.

When the DSA device is first created all ports are independent devices.  When
a bridge is created and those devices are attached the ports share the same
membership so all frames will be forwarded internally.

When RSTP is used the port can be put in blocked state and so the forwarding
will stop for that port.   Currently the switch driver will check that 
membership
to decide whether to set that bit.

> > A little out-of-topic is the MAC driver may have problem receiving the
> frame if
> > the tail tag length is too big.  Most of the MAC drivers have big enough
> receive
> > buffer or require 64-bytes multiple so there are extra room to
> accommodate
> > the big tail tag at the end of the frame.  Still some MAC drivers use 
> > exactly
> 1500
> > MTU and some additional length and may run into this problem.  Currently
> the
> > Atmel Cadence MAC driver has this potential problem when certain
> conditions
> > are met.
> 
> This is a know problem. I just fixed the FEC driver which also
> discarded DSA tagged frames when they were bigger than the MTU.
> 
> So far, it has not been too much of an issue, because most boards with
> a switch, use an Ethernet interface from the same manufacture. Marvell
> Switches with a Marvell Ethernet interface. Broadcom switch with a
> Broadcom Interface. Atheros switch with an Atheros interface. And
> naturally, these combinations just work. But Freescale FEC and Marvell
> had not been combined before, and it was broken.
> 
> So i would not be surprised if the Cadence MAC driver had an issue.

The KSZ switches never have a built-in MAC controller, so it is always a support
issue for us.

RE: [PATCH RFC 6/6] Modify tag_ksz.c to support other KSZ switch drivers

[Tristram.Ha]

> > In the old DSA implementation all the ports are partitioned into its own
> device
> > and the bridge joining them will do all the forwarding.  This is useful for
> quick
> > testing with some protocols like RSTP but it is probably useless for real
> > operation.
> 
> It is a good minimal driver, to get something into the kernel. You can
> then add features to it.
> 
> > The new switchdev model tries to use the switch hardware as much as
> > possible.  This offload_fwd_mark bit means the frame is forwarded by the
> > hardware switch, so the software bridge does not need to do it again.
> Without
> > this bit there will be duplicated multicast frames coming out the ports if
> internal
> > forwarding is enabled.
> 
> Correct. Once you switch driver is clever enough, you can enable
> offload_fwd_mark.
> 
> > When RSTP is used the port can be put in blocked state and so the
> forwarding
> > will stop for that port.   Currently the switch driver will check that
> membership
> > to decide whether to set that bit.
> 
> This i don't get. RSTP or STP just break loops. How does RSTP vs STP
> mean you need to set offload_fwd_mark differently?
> 

The logic of the switch driver is if the membership of the port receiving
the frame contains other ports--not counting cpu port--the bit
offload_fwd_mark is set.  In RSTP closing the blocked port is generally good
enough, but there are exceptions, so the port is removed from the
membership of other forwarding ports.  A disabled port will have its
membership completely reset so it cannot receive anything.  It does not
matter much in RSTP as the software bridge should know whether to forward
the frame or not.

We are back to square one.  Is there any plan to add this offload_fwd_mark
support to DSA driver so that it can be reported properly?  It can be set all 
the
time, except during port initialization or before bridge creation the forwarding
state does not reflect reality.

If not the port membership can be fixed and there is no internal switch
forwarding, leaving everything handled by the software bridge.

RE: [PATCH RFC 6/6] Modify tag_ksz.c to support other KSZ switch drivers

[Tristram.Ha]

> I am not really sure why this is such a concern for you so soon when
> your driver is not even included yet. You should really aim for baby
> steps here: get the basic driver(s) included, with a limited set of
> features, and gradually add more features to the driver. When
> fwd_offload_mark and RSTP become a real problem, we can most
> definitively find a way to fix those in DSA and depending drivers.

I was under the impression that there is a new push of this new switchdev
model and so the DSA model was overhauled to support that.

The KSZ9477 driver is already in the kernel, and its register access is actually
much different from the other older switches.  There are not much common
code to be reused.  I always know this tail tag handling is the sticking point.
I will submit a much simplified driver and wait for switch access in the future.

RE: [PATCH RFC 5/5] Add KSZ8795 SPI driver

[Tristram.Ha]

> -Original Message-
> From: Pavel Machek [mailto:pa...@ucw.cz]
> Sent: Friday, September 08, 2017 2:26 AM
> To: Tristram Ha - C24268
> Cc: and...@lunn.ch; muva...@gmail.com; nathan.leigh.con...@gmail.com;
> vivien.dide...@savoirfairelinux.com; f.faine...@gmail.com;
> net...@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC 5/5] Add KSZ8795 SPI driver
> 
> Hi!
> 
> 
> > +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_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;
> > +}
> 
> Please format according to CodingStyle. (Not only this.)
> 
> And this will be common for more drivers. Can it go to a header file
> and be included...?
> 

Sorry about the formatting.  It seems my e-mail system needs to be checked
to make sure it does not auto-format the contents again.

About the SPI access functions they are the same for each driver except the
low level ksz_spi_read_reg and ksz_spi_write_reg.  The dev_io_ops structure
should contain only those 2 and ksz_spi_get and ksz_spi_set.

But that requires changing ksz_spi.c.  The idea was to keep the code of
KSZ9477 driver with little change as possible while introducing another driver.

The KSZ9477 driver will need to be updated with some of the code in KSZ8795
driver regarding port membership and MIB counter reading.

RE: [PATCH RFC 3/5] Add KSZ8795 switch driver

[Tristram.Ha]

> -Original Message-
> From: Pavel Machek [mailto:pa...@ucw.cz]
> Sent: Friday, September 08, 2017 2:19 AM
> To: Tristram Ha - C24268
> Cc: and...@lunn.ch; muva...@gmail.com; nathan.leigh.con...@gmail.com;
> vivien.dide...@savoirfairelinux.com; f.faine...@gmail.com;
> net...@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC 3/5] Add KSZ8795 switch driver
> 
> On Thu 2017-09-07 21:17:16, tristram...@microchip.com wrote:
> > From: Tristram Ha 
> >
> > Add KSZ8795 switch support with function code.
> 
> English? "Add KSZ8795 switch support." ?
> 
> > Signed-off-by: Tristram Ha 
> > ---
> > diff --git a/drivers/net/dsa/microchip/ksz8795.c
> b/drivers/net/dsa/microchip/ksz8795.c
> > new file mode 100644
> > index 000..e4d4e6a
> > --- /dev/null
> > +++ b/drivers/net/dsa/microchip/ksz8795.c
> > @@ -0,0 +1,2066 @@
> > +/*
> > + * Microchip KSZ8795 switch driver
> > + *
> > + * Copyright (C) 2017 Microchip Technology Inc.
> > + * Tristram Ha 
> > + *
> > + * 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.
> > + */
> 
> This is not exactly GPL, right? But tagging below says it is
> GPL. Please fix one.
> 

This boilerplate paragraph was copied from the KSZ9477 driver, although I did
wonder why this was used.

> > +static int ksz_reset_switch(struct ksz_device *dev)
> > +{
> > +   /* reset switch */
> > +   ksz_write8(dev, REG_POWER_MANAGEMENT_1,
> > +  SW_SOFTWARE_POWER_DOWN <<
> SW_POWER_MANAGEMENT_MODE_S);
> > +   ksz_write8(dev, REG_POWER_MANAGEMENT_1, 0);
> > +
> > +   return 0;
> > +}
> 
> This is going to be same in other drivers, right?
> 

The switch reset is different in each chip, although KSZ8795 and
KSZ8895 use the same mechanism.

> > +
> > +   for (timeout = 1; timeout > 0; timeout--) {
> > +   ksz_read8(dev, REG_IND_MIB_CHECK, &check);
> > +
> > +   if (check & MIB_COUNTER_VALID) {
> > +   ksz_read32(dev, REG_IND_DATA_LO, &data);
> > +   if (addr < 2) {
> > +   u64 total;
> > +
> > +   total = check & MIB_TOTAL_BYTES_H;
> > +   total <<= 32;
> > +   *cnt += total;
> > +   *cnt += data;
> > +   if (check & MIB_COUNTER_OVERFLOW) {
> > +   total = MIB_TOTAL_BYTES_H + 1;
> > +   total <<= 32;
> > +   *cnt += total;
> > +   }
> > +   } else {
> > +   if (check & MIB_COUNTER_OVERFLOW)
> > +   *cnt += MIB_PACKET_DROPPED + 1;
> > +   *cnt += data & MIB_PACKET_DROPPED;
> > +   }
> > +   break;
> > +   }
> > +   }
> 
> Why do you need a loop here? This is quite strange code. (And you have
> similar strangeness elsewhere. Please fix.)
> 

The MIB_COUNTER_VALID bit may be invalid on first read, although in slow
SPI speed it never happens.  The timeout value should be increased to 2.

> > +static int valid_dyn_entry(struct ksz_device *dev, u8 *data)
> > +{
> > +   int timeout = 100;
> > +
> > +   do {
> > +   ksz_read8(dev, REG_IND_DATA_CHECK, data);
> > +   timeout--;
> > +   } while ((*data & DYNAMIC_MAC_TABLE_NOT_READY) && timeout);
> > +
> > +   /* Entry is not ready for accessing. */
> > +   if (*data & DYNAMIC_MAC_TABLE_NOT_READY) {
> > +   return 1;
> > +   /* Entry is ready for accessing. */
> > +   } else {
> > +   ksz_read8(dev, REG_IND_DATA_8, data);
> > +
> > +   /* There is no valid entry in the table. */
> > +   if (*data & DYNAMIC_MAC_TABLE_MAC_EMPTY)
> > +   return 2;
> > +   }
> > +   return 0;
> > +}
> 
> Normal calling convention is 0 / -ERROR, not 0,1,2.
>

This is an internal function that is not returning any error.  It just reports
different conditions so the calling function decides what to do.
 
> > +static void ksz_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
> > +{
> > +   struct ksz_port *port;
> > +   u8 ctrl;
> > +   u8 restart;
> > +   u8 link;
> > +   u8 speed;
> > +   u8 for

RE: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added

[Tristram.Ha]

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Friday, September 08, 2017 7:12 AM
> To: Maxim Uvarov
> Cc: Tristram Ha - C24268; Pavel Machek; Nathan Conrad; Vivien Didelot; Florian
> Fainelli; netdev; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new
> drivers can be added
> 
> On Fri, Sep 08, 2017 at 04:32:35PM +0300, Maxim Uvarov wrote:
> > 2017-09-08 0:54 GMT+03:00 Andrew Lunn :
> > >> -- compatible: For external switch chips, compatible string must be 
> > >> exactly
> one
> > >> -  of: "microchip,ksz9477"
> > >> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip,
> > >> +   "microchip,ksz8795" for KSZ8795 chip,
> > >> +   "microchip,ksz8794" for KSZ8794 chip,
> > >> +   "microchip,ksz8765" for KSZ8765 chip,
> > >> +   "microchip,ksz8895" for KSZ8895 chip,
> > >> +   "microchip,ksz8864" for KSZ8864 chip,
> > >> +   "microchip,ksz8873" for KSZ8873 chip,
> > >> +   "microchip,ksz8863" for KSZ8863 chip,
> > >> +   "microchip,ksz8463" for KSZ8463 chip
> > >
> >
> > all that chips have the same spi access to get chip id on probe(). I
> > prefer common microship,ksz-spi rather than somebody will always
> > maintain that list.
> 
> The Marvell DSA driver is similar. The compatibility string tells you
> enough to go find the switch ID in the switch itself.
> 
> I suppose this comes down to, is there going to be one SPI driver for
> all the devices, or lots of drivers? In general, DSA has one driver
> for lots of devices. The mv88e6xxx supports around 25 devices. The b53
> has around 17, etc.
> 
> So i would suggest one driver supporting all the different devices.
> 

There will be 5 drivers to support these devices:

ksz9477.c - KSZ9893/KSZ9897/KSZ9567/KSZ9566/KSZ9477
ksz8795.c - KSZ8795/KSZ8795/KSZ8765
ksz8895.c - KSZ8895/KSZ8864
ksz8863.c - KSZ8863/KSZ8873
ksz8463.c - KSZ8463

These chips have different SPI access mechanisms, MIB counter reading,
and register set.  These can be combined into one single driver using
function pointers, at least for ksz8795/ksz8895/ksz8863/ksz8463.  My
only concern is the memory footprint.  The customer may not want a
big driver to cover all the switches while only one is used.

Out of topic I have a question to ask the community regarding the DSA
port creation:

Port 1 can be specified using the reg parameter specifying 0; port 2, 1;
and so on.  The KSZ8794 is a variant of KSZ8795 with port 4 disabled.
So
lan1 = 0, lan2 = 1, lan3 = 2, cpu = 4.
But our company Marketing does not want to promote that fact but treat
KSZ8794 as a distinct product.
So
lan1 = 0, lan2 = 1, lan3 = 2, cpu = 3.
Is this okay to hide this information inside the driver?  This is manageable
for KSZ8794 but for KSZ8864 the first port is disabled:
lan1 = 1, lan2 = 2, lan3 = 3, cpu = 4.

I am not sure whether DSA has or will have a way to display the port
mapping to regular users.

RE: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added

[Tristram.Ha]

> -Original Message-
> From: Maxim Uvarov [mailto:muva...@gmail.com]
> Sent: Friday, September 08, 2017 12:00 PM
> To: Florian Fainelli
> Cc: Tristram Ha - C24268; Andrew Lunn; Pavel Machek; Nathan Conrad; Vivien
> Didelot; netdev; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new
> drivers can be added
> 
> 2017-09-08 21:48 GMT+03:00 Florian Fainelli :
> > On 09/07/2017 02:11 PM, tristram...@microchip.com wrote:
> >> From: Tristram Ha 
> >>
> >> Add other KSZ switches support so that patch check does not complain.
> >>
> >> Signed-off-by: Tristram Ha 
> >> ---
> >>  Documentation/devicetree/bindings/net/dsa/ksz.txt | 117
> >> --
> >>  1 file changed, 62 insertions(+), 55 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt
> >> b/Documentation/devicetree/bindings/net/dsa/ksz.txt
> >> index 0ab8b39..34af0e0 100644
> >> --- a/Documentation/devicetree/bindings/net/dsa/ksz.txt
> >> +++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt
> >> @@ -3,8 +3,15 @@ Microchip KSZ Series Ethernet switches
> >>
> >>  Required properties:
> >>
> >> -- compatible: For external switch chips, compatible string must be
> >> exactly one
> >> -  of: "microchip,ksz9477"
> >> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip,
> >> +   "microchip,ksz8795" for KSZ8795 chip,
> >> +   "microchip,ksz8794" for KSZ8794 chip,
> >> +   "microchip,ksz8765" for KSZ8765 chip,
> >> +   "microchip,ksz8895" for KSZ8895 chip,
> >> +   "microchip,ksz8864" for KSZ8864 chip,
> >> +   "microchip,ksz8873" for KSZ8873 chip,
> >> +   "microchip,ksz8863" for KSZ8863 chip,
> >> +   "microchip,ksz8463" for KSZ8463 chip
> >
> 
> 
> Tristram, does any of this devices support chaining?
> 
> Maxim.

They do not if you mean daisy-chaining the switches together.

There is always a problem that once tail tagging mode is enabled
sending a frame through the MAC without going through the DSA
layer will cause the frame to be dropped.

RE: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added

[Tristram.Ha]

> -Original Message-
> From: Florian Fainelli [mailto:f.faine...@gmail.com]
> Sent: Friday, September 08, 2017 12:54 PM
> To: Tristram Ha - C24268; muva...@gmail.com
> Cc: and...@lunn.ch; pa...@ucw.cz; nathan.leigh.con...@gmail.com;
> vivien.dide...@savoirfairelinux.com; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new
> drivers can be added
> 
> On 09/08/2017 12:48 PM, tristram...@microchip.com wrote:
> >> -Original Message-
> >> From: Maxim Uvarov [mailto:muva...@gmail.com]
> >> Sent: Friday, September 08, 2017 12:00 PM
> >> To: Florian Fainelli
> >> Cc: Tristram Ha - C24268; Andrew Lunn; Pavel Machek; Nathan Conrad; Vivien
> >> Didelot; netdev; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> >> Subject: Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that
> new
> >> drivers can be added
> >>
> >> 2017-09-08 21:48 GMT+03:00 Florian Fainelli :
> >>> On 09/07/2017 02:11 PM, tristram...@microchip.com wrote:
>  From: Tristram Ha 
> 
>  Add other KSZ switches support so that patch check does not complain.
> 
>  Signed-off-by: Tristram Ha 
>  ---
>   Documentation/devicetree/bindings/net/dsa/ksz.txt | 117
>  --
>   1 file changed, 62 insertions(+), 55 deletions(-)
> 
>  diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt
>  b/Documentation/devicetree/bindings/net/dsa/ksz.txt
>  index 0ab8b39..34af0e0 100644
>  --- a/Documentation/devicetree/bindings/net/dsa/ksz.txt
>  +++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt
>  @@ -3,8 +3,15 @@ Microchip KSZ Series Ethernet switches
> 
>   Required properties:
> 
>  -- compatible: For external switch chips, compatible string must be
>  exactly one
>  -  of: "microchip,ksz9477"
>  +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip,
>  +   "microchip,ksz8795" for KSZ8795 chip,
>  +   "microchip,ksz8794" for KSZ8794 chip,
>  +   "microchip,ksz8765" for KSZ8765 chip,
>  +   "microchip,ksz8895" for KSZ8895 chip,
>  +   "microchip,ksz8864" for KSZ8864 chip,
>  +   "microchip,ksz8873" for KSZ8873 chip,
>  +   "microchip,ksz8863" for KSZ8863 chip,
>  +   "microchip,ksz8463" for KSZ8463 chip
> >>>
> >>
> >>
> >> Tristram, does any of this devices support chaining?
> >>
> >> Maxim.
> >
> > They do not if you mean daisy-chaining the switches together.
> 
> Marvell tags allow you to specify both a port and switch index
> destination after setting up an appropriate routing table, I am assuming
> this is not supported.
> 
> What happens though if I connect two KSZ switches ones to another say:
> 
> eth0
>   -> KSZ8463
>   -> KSZ8463
> 
> Will the first switch terminates KSZ tag if it sees two tags
> encapsulated in another, something like this:
> 
> | MAC DA | MAC SA |  payload | Inner KSZ tag | Inner KSZ tag | FCS |
> 

In theory it is doable by adding more tags and remember which port
is connected to the cpu port of another switch, but there is no switch
forwarding and everything is handled by software.

> >
> > There is always a problem that once tail tagging mode is enabled
> > sending a frame through the MAC without going through the DSA
> > layer will cause the frame to be dropped.
> 
> Yes, once the master network device is used for DSA, it is still usable
> directly by e.g: applications, but it won't do anything if the switch is
> configured such that it drops ingressing frames not having the proper
> tag. We documented that here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docume
> ntation/networking/dsa/dsa.txt#n275
> --

As the DSA was developed for the Marvell switches I assumed they can
forward frame without a tag.

RE: [PATCH RFC 0/6] Modify KSZ9477 DSA driver in preparation to add other KSZ switch drivers.

[Tristram.Ha]

> -Original Message-
> From: Pavel Machek [mailto:pa...@ucw.cz]
> Sent: Friday, September 08, 2017 1:54 AM
> To: Tristram Ha - C24268
> Cc: and...@lunn.ch; muva...@gmail.com; nathan.leigh.con...@gmail.com;
> vivien.dide...@savoirfairelinux.com; f.faine...@gmail.com;
> net...@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh -
> C21699
> Subject: Re: [PATCH RFC 0/6] Modify KSZ9477 DSA driver in preparation to
> add other KSZ switch drivers.
> 
> Hi!
> 
> > From: Tristram Ha 
> >
> > This series of patches is to modify the original KSZ9477 DSA driver so that
> other KSZ switch drivers can be added and use the common code.
> >
> 
> Please wrap the lines from time to time...
> 
> 
> > This patch set is against net-next.
> >
> >  drivers/net/dsa/microchip/Makefile |2 +-
> >  drivers/net/dsa/microchip/ksz9477.c| 1317
> 
> 
> We already have ksz_9477_reg.h. So should this be ksz_9477.c for
> consistency?

The product name is KSZ9477 and other switches are also like KSZ,
so I would prefer to have no separation between KSZ and the product
number.  I think the file ksz_9477_reg.h was named that way because
the other files were named ksz_common.c and ksz_spi.c.  If need to
we can change the file name.

RE: [PATCH RFC 3/5] Add KSZ8795 switch driver

[Tristram.Ha]

> -Original Message-
> From: Pavel Machek [mailto:pa...@ucw.cz]
> Sent: Friday, September 08, 2017 2:58 PM
> To: Tristram Ha - C24268
> Cc: and...@lunn.ch; muva...@gmail.com; nathan.leigh.con...@gmail.com;
> vivien.dide...@savoirfairelinux.com; f.faine...@gmail.com;
> net...@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh -
> C21699
> Subject: Re: [PATCH RFC 3/5] Add KSZ8795 switch driver
> 
> Hi!
> 
> > > > +   default:
> > > > +   processed = false;
> > > > +   break;
> > > > +   }
> > > > +   if (processed)
> > > > +   *val = data;
> > > > +}
> > >
> > > Similar code will be needed by other drivers, right?
> >
> > Although KSZ8795 and KSZ8895 may use the same code, the other
> > chips will have different code.
> 
> Ok, please make sure code is shared between these two.

The exact function probably cannot be shared between KSZ8795
and KSZ8895 drivers because although the register constants look
the same but their exact locations are different in the 2 chips.

RE: [PATCH] DSA support for Micrel KSZ8895

[Tristram.Ha]

> -Original Message-
> From: Maxim Uvarov [mailto:muva...@gmail.com]
> Sent: Wednesday, September 06, 2017 2:15 AM
> To: Tristram Ha - C24268
> Cc: Pavel Machek; Woojung Huh - C21699; Nathan Conrad; Vivien Didelot;
> Florian Fainelli; netdev; linux-kernel@vger.kernel.org; Andrew Lunn
> Subject: Re: [PATCH] DSA support for Micrel KSZ8895
> 
> 2017-08-31 0:32 GMT+03:00  :
> >> On Mon 2017-08-28 16:09:27, Andrew Lunn wrote:
> >> > > I may be confused here, but AFAICT:
> >> > >
> >> > > 1) Yes, it has standard layout when accessed over MDIO.
> >> >
> >> >
> >> > Section 4.8 of the datasheet says:
> >> >
> >> > All the registers defined in this section can be also accessed
> >> > via the SPI interface.
> >> >
> >> > Meaning all PHY registers can be access via the SPI interface. So
> >> > you should be able to make a standard Linux MDIO bus driver which
> >> > performs SPI reads.
> >>
> >> As far as I can tell (and their driver confirms) -- yes, all those
> >> registers can be accessed over the SPI, they are just shuffled
> >> around... hence MDIO emulation code. I copied it from their code (see
> >> the copyrights) so no, I don't believe there's nicer solution.
> >>
> >> Best regards,
> >>
> >>
> >> Pavel
> >
> > Can you hold on your developing work on KSZ8895 driver?  I am afraid your
> effort may be in vain.  We at Microchip are planning to release DSA drivers
> for all KSZ switches, starting at KSZ8795, then KSZ8895, and KSZ8863.
> >
> > The driver files all follow the structures of the current KSZ9477 DSA 
> > driver,
> and the file tag_ksz.c will be updated to handle the tail tag of different 
> chips,
> which requires including the ksz_priv.h header.  That is required
> nevertheless to support using the offload_fwd_mark indication.
> >
> > The KSZ8795 driver will be submitted after Labor Day (9/4) if testing 
> > reveals
> no problem.  The KSZ8895 driver will be submitted right after that.  You
> should have no problem using the driver right away.
> >
> 
> Hello Tristram, is there any update for that driver?
> 
> Maxim.
> 

The patches are under review internally and will need to be updated and 
approved by Woojung before formal submission.  Problem is although KSZ8795 and 
KSZ8895 drivers are new code and will be submitted as RFC, they depend on the 
change of KSZ9477 driver currently in the kernel, which require more rigorous 
review.

[PATCH RFC 2/6] Create new file ksz9477.c from KSZ9477 code in ksz_common.c

[Tristram.Ha]

From: Tristram Ha 

Create new ksz9477.c file from original ksz_common.c.

Signed-off-by: Tristram Ha 
---
diff --git a/drivers/net/dsa/microchip/ksz9477.c 
b/drivers/net/dsa/microchip/ksz9477.c
new file mode 100644
index 000..bc722b4
--- /dev/null
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -0,0 +1,1317 @@
+/*
+ * Microchip switch driver main logic
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ksz_priv.h"
+#include "ksz_9477_reg.h"
+
+static const struct {
+   int index;
+   char string[ETH_GSTRING_LEN];
+} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
+   { 0x00, "rx_hi" },
+   { 0x01, "rx_undersize" },
+   { 0x02, "rx_fragments" },
+   { 0x03, "rx_oversize" },
+   { 0x04, "rx_jabbers" },
+   { 0x05, "rx_symbol_err" },
+   { 0x06, "rx_crc_err" },
+   { 0x07, "rx_align_err" },
+   { 0x08, "rx_mac_ctrl" },
+   { 0x09, "rx_pause" },
+   { 0x0A, "rx_bcast" },
+   { 0x0B, "rx_mcast" },
+   { 0x0C, "rx_ucast" },
+   { 0x0D, "rx_64_or_less" },
+   { 0x0E, "rx_65_127" },
+   { 0x0F, "rx_128_255" },
+   { 0x10, "rx_256_511" },
+   { 0x11, "rx_512_1023" },
+   { 0x12, "rx_1024_1522" },
+   { 0x13, "rx_1523_2000" },
+   { 0x14, "rx_2001" },
+   { 0x15, "tx_hi" },
+   { 0x16, "tx_late_col" },
+   { 0x17, "tx_pause" },
+   { 0x18, "tx_bcast" },
+   { 0x19, "tx_mcast" },
+   { 0x1A, "tx_ucast" },
+   { 0x1B, "tx_deferred" },
+   { 0x1C, "tx_total_col" },
+   { 0x1D, "tx_exc_col" },
+   { 0x1E, "tx_single_col" },
+   { 0x1F, "tx_mult_col" },
+   { 0x80, "rx_total" },
+   { 0x81, "tx_total" },
+   { 0x82, "rx_discards" },
+   { 0x83, "tx_discards" },
+};
+
+static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, 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, bool 
+set) {
+   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,
+bool 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, bool 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 int wait_vlan_ctrl_ready(struct ksz_device *dev, u32 waiton, int 
+timeout) {
+   u8 data;
+
+   do {
+   ksz_read8(dev, REG_SW_VLAN_CTRL, &data);
+   if (!(data & waiton))
+   break;
+   usleep_range(1, 10);
+   } while (timeout-- > 0);
+
+   if (timeout <= 0)
+   return -ETIMEDOUT;
+
+   return 0;
+}
+
+static int get_vlan_table(struct dsa_switch *ds, u16 vid, u32 
+*vlan_table) {
+   struct ksz_device *dev = ds->priv;
+   int ret;
+
+   mutex_lock(&dev->vlan_mutex);
+
+   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 */
+   ret = wait_vlan_ctrl_ready(dev, VLAN_START, 1000);
+   if (ret < 0) {
+   dev_dbg(dev->dev, "Failed to read vlan table\n");
+   goto exit;
+   }
+
+   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

[PATCH RFC 6/6] Modify tag_ksz.c to support other KSZ switch drivers

[Tristram.Ha]

From: Tristram Ha 

The KSZ tail tag code can be used by different KSZ switch drivers.

Signed-off-by: Tristram Ha 
---
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c index 010ca0a..d5faf14 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -13,35 +13,21 @@
 #include 
 #include 
 #include "dsa_priv.h"
-
-/* For Ingress (Host -> KSZ), 2 bytes are added before FCS.
- * ---
- * DA(6bytes)|SA(6bytes)||Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes)
- * ---
- * tag0 : Prioritization (not used now)
- * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x10=port5)
- *
- * For Egress (KSZ -> Host), 1 byte is added before FCS.
- * ---
- * DA(6bytes)|SA(6bytes)||Data(nbytes)|tag0(1byte)|FCS(4bytes)
- * ---
- * tag0 : zero-based value represents port
- *   (eg, 0x00=port1, 0x02=port3, 0x06=port7)
- */
-
-#defineKSZ_INGRESS_TAG_LEN 2
-#defineKSZ_EGRESS_TAG_LEN  1
+#include "../../drivers/net/dsa/microchip/ksz_priv.h"
 
 static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)  {
struct dsa_slave_priv *p = netdev_priv(dev);
+   struct dsa_switch *ds = p->dp->ds;
+   struct ksz_device *swdev = ds->priv;
struct sk_buff *nskb;
+   int len;
int padlen;
-   u8 *tag;
 
padlen = (skb->len >= ETH_ZLEN) ? 0 : ETH_ZLEN - skb->len;
 
-   if (skb_tailroom(skb) >= padlen + KSZ_INGRESS_TAG_LEN) {
+   len = swdev->dev_ops->get_tx_len(swdev);
+   if (skb_tailroom(skb) >= padlen + len) {
/* Let dsa_slave_xmit() free skb */
if (__skb_put_padto(skb, skb->len + padlen, false))
return NULL;
@@ -49,7 +35,7 @@ static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct 
net_device *dev)
nskb = skb;
} else {
nskb = alloc_skb(NET_IP_ALIGN + skb->len +
-padlen + KSZ_INGRESS_TAG_LEN, GFP_ATOMIC);
+padlen + len, GFP_ATOMIC);
if (!nskb)
return NULL;
skb_reserve(nskb, NET_IP_ALIGN);
@@ -70,9 +56,7 @@ static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct 
net_device *dev)
consume_skb(skb);
}
 
-   tag = skb_put(nskb, KSZ_INGRESS_TAG_LEN);
-   tag[0] = 0;
-   tag[1] = 1 << p->dp->index; /* destination port */
+   swdev->dev_ops->add_tail_tag(swdev, nskb, p->dp->index);
 
return nskb;
 }
@@ -83,16 +67,16 @@ static struct sk_buff *ksz_rcv(struct sk_buff *skb, struct 
net_device *dev,
struct dsa_switch_tree *dst = dev->dsa_ptr;
struct dsa_port *cpu_dp = dsa_get_cpu_port(dst);
struct dsa_switch *ds = cpu_dp->ds;
-   u8 *tag;
+   struct ksz_device *swdev = ds->priv;
+   int len;
int source_port;
 
-   tag = skb_tail_pointer(skb) - KSZ_EGRESS_TAG_LEN;
+   len = swdev->dev_ops->get_tail_tag(swdev, skb, &source_port);
 
-   source_port = tag[0] & 7;
if (source_port >= ds->num_ports || !ds->ports[source_port].netdev)
return NULL;
 
-   pskb_trim_rcsum(skb, skb->len - KSZ_EGRESS_TAG_LEN);
+   pskb_trim_rcsum(skb, skb->len - len);
 
skb->dev = ds->ports[source_port].netdev;

[PATCH RFC 0/6] Modify KSZ9477 DSA driver in preparation to add other KSZ switch drivers.

[Tristram.Ha]

From: Tristram Ha 

This series of patches is to modify the original KSZ9477 DSA driver so that 
other KSZ switch drivers can be added and use the common code.

This patch set is against net-next.

 drivers/net/dsa/microchip/Makefile |2 +-
 drivers/net/dsa/microchip/ksz9477.c| 1317 
 drivers/net/dsa/microchip/ksz_common.c | 1156 +---
 drivers/net/dsa/microchip/ksz_priv.h   |  105 ++-
 drivers/net/dsa/microchip/ksz_spi.c|   13 +-
 net/dsa/tag_ksz.c  |   40 +-
 6 files changed, 1458 insertions(+), 1175 deletions(-)  create mode 100644 
drivers/net/dsa/microchip/ksz9477.c

[PATCH RFC 4/6] The common header ksz_priv.h does not contain chip specific data

[Tristram.Ha]

From: Tristram Ha 

The header ksz_priv.h is used by all KSZ switch drivers so chip specific data 
are removed and commonly used variables are added.

Signed-off-by: Tristram Ha 
---
diff --git a/drivers/net/dsa/microchip/ksz_priv.h 
b/drivers/net/dsa/microchip/ksz_priv.h
index 2a98dbd..343b509 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -25,14 +25,56 @@
 #include 
 #include 
 
-#include "ksz_9477_reg.h"
-
 struct ksz_io_ops;
 
 struct vlan_table {
u32 table[3];
 };
 
+enum {
+   PHY_NO_FLOW_CTRL,
+   PHY_FLOW_CTRL,
+   PHY_TX_ONLY,
+   PHY_RX_ONLY
+};
+
+struct ksz_port_mib {
+   u8 cnt_ptr;
+   u8 ctrl;
+   unsigned long time;
+
+   struct ksz_port_mib_info {
+   u64 counter;
+   u8 read_cnt;
+   u8 read_max;
+   } *info;
+};
+
+struct ksz_port {
+   u16 member;
+   u16 vid_member;
+   int stp_state;
+   u16 speed;
+   u8 duplex;
+   u8 advertised;
+   u8 flow_ctrl;
+   u8 link;
+   u8 partner;
+
+   u32 on:1;   /* port is not disabled by hardware */
+   u32 fiber:1;/* port is fiber */
+   u32 sgmii:1;/* port is SGMII */
+   u32 force:1;
+   u32 link_down:1;/* link just goes down */
+   u32 link_up:1;  /* link is up */
+
+   struct ksz_port_mib mib;
+};
+
+#define USE_FEWER_PORTSBIT(18)
+
+#define PTP_TAGBIT(29)
+
 struct ksz_device {
struct dsa_switch *ds;
struct ksz_platform_data *pdata;
@@ -43,6 +85,7 @@ struct ksz_device {
struct mutex alu_mutex; /* ALU access */
struct mutex vlan_mutex;/* vlan access */
const struct ksz_io_ops *ops;
+   const struct ksz_dev_ops *dev_ops;
 
struct device *dev;
 
@@ -56,10 +99,32 @@ struct ksz_device {
int cpu_port;   /* port connected to CPU */
int cpu_ports;  /* port bitmap can be cpu port */
int port_cnt;
+   int mib_cnt;
+   int mib_port_cnt;
+   int last_port;  /* ports after that not used */
+   int interface;
 
struct vlan_table *vlan_cache;
 
-   u64 mib_value[TOTAL_SWITCH_COUNTER_NUM];
+   u8 *txbuf;
+
+   struct ksz_port *ports;
+   struct timer_list mib_read_timer;
+   struct work_struct mib_read;
+   spinlock_t mib_read_lock;   /* use to update read_cnt */
+   unsigned long MIB_READ_INTERVAL;
+   u32 features;
+   u32 overrides;
+   u16 br_member;
+   u16 member;
+   u16 live_ports;
+   u16 on_ports;   /* ports enabled by DSA */
+   u16 rx_ports;
+   u16 tx_ports;
+   u16 mirror_rx;
+   u16 mirror_tx;
+   u16 HOST_MASK;
+   u16 PORT_MASK;
 };
 
 struct ksz_io_ops {
@@ -75,12 +140,30 @@ struct ksz_io_ops {
  u16 *value);
int (*phy_write16)(struct ksz_device *dev, int addr, int reg,
   u16 value);
+   unsigned int (*get)(struct ksz_device *dev, u32 reg, void *data,
+   unsigned int len);
+   unsigned int (*set)(struct ksz_device *dev, u32 reg, void *data,
+   unsigned int len);
+};
+
+struct ksz_dev_ops {
+   u32 (*get_port_addr)(int port, int offset);
+   int (*reset)(struct ksz_device *dev);
+   int (*get_rx_len)(struct ksz_device *dev);
+   int (*get_tx_len)(struct ksz_device *dev);
+   void (*add_tail_tag)(struct ksz_device *dev, struct sk_buff *skb,
+int port);
+   int (*get_tail_tag)(struct ksz_device *dev, struct sk_buff *skb,
+   int *port);
+   int (*detect)(struct ksz_device *dev);
+   int (*init)(struct ksz_device *dev);
+   void (*exit)(struct ksz_device *dev);
 };
 
 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);
+int ksz_switch_register(struct ksz_device *dev,
+   const struct ksz_dev_ops *ops);
 void ksz_switch_remove(struct ksz_device *dev);
 
 static inline int ksz_read8(struct ksz_device *dev, u32 reg, u8 *val) @@ 
-174,37 +257,37 @@ static inline int ksz_write32(struct ksz_device *dev, u32 
reg, u32 value)  static inline void ksz_pread8(struct ksz_device *dev, int 
port, int offset,
  u8 *data)
 {
-   ksz_read8(dev, PORT_CTRL_ADDR(port, offset), data);
+   ksz_read8(dev, dev->dev_ops->get_port_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);
+

[PATCH RFC 5/6] Switch SPI driver calls its own driver switch register function

[Tristram.Ha]

From: Tristram Ha 

SPI driver calls own specific switch register function.
Shutdown callback function is added to reset switch to default state.

Signed-off-by: Tristram Ha 
---
diff --git a/drivers/net/dsa/microchip/ksz_spi.c 
b/drivers/net/dsa/microchip/ksz_spi.c
index c519469..d03eb83 100644
--- a/drivers/net/dsa/microchip/ksz_spi.c
+++ b/drivers/net/dsa/microchip/ksz_spi.c
@@ -25,6 +25,8 @@
 
 #include "ksz_priv.h"
 
+int ksz9477_switch_register(struct ksz_device *dev);
+
 /* SPI frame opcodes */
 #define KS_SPIOP_RD3
 #define KS_SPIOP_WR2
@@ -174,7 +176,7 @@ static int ksz_spi_probe(struct spi_device *spi)
if (spi->dev.platform_data)
dev->pdata = spi->dev.platform_data;
 
-   ret = ksz_switch_register(dev);
+   ret = ksz9477_switch_register(dev);
if (ret)
return ret;
 
@@ -193,6 +195,14 @@ static int ksz_spi_remove(struct spi_device *spi)
return 0;
 }
 
+static void ksz_spi_shutdown(struct spi_device *spi) {
+   struct ksz_device *dev = spi_get_drvdata(spi);
+
+   if (dev)
+   dev->dev_ops->reset(dev);
+}
+
 static const struct of_device_id ksz_dt_ids[] = {
{ .compatible = "microchip,ksz9477" },
{},
@@ -207,6 +217,7 @@ static int ksz_spi_remove(struct spi_device *spi)
},
.probe  = ksz_spi_probe,
.remove = ksz_spi_remove,
+   .shutdown = ksz_spi_shutdown,
 };
 
 module_spi_driver(ksz_spi_driver);

[PATCH RFC 3/6] The file ksz_common.c contains common code shared by all KSZ switch drivers

[Tristram.Ha]

From: Tristram Ha 

The file ksz_common.c only holds common code used by all KSZ switch drivers.

Signed-off-by: Tristram Ha 
---
diff --git a/drivers/net/dsa/microchip/ksz_common.c 
b/drivers/net/dsa/microchip/ksz_common.c
index 56cd6d3..bebcc65 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -25,1114 +25,12 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
 #include "ksz_priv.h"
 
-static const struct {
-   int index;
-   char string[ETH_GSTRING_LEN];
-} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
-   { 0x00, "rx_hi" },
-   { 0x01, "rx_undersize" },
-   { 0x02, "rx_fragments" },
-   { 0x03, "rx_oversize" },
-   { 0x04, "rx_jabbers" },
-   { 0x05, "rx_symbol_err" },
-   { 0x06, "rx_crc_err" },
-   { 0x07, "rx_align_err" },
-   { 0x08, "rx_mac_ctrl" },
-   { 0x09, "rx_pause" },
-   { 0x0A, "rx_bcast" },
-   { 0x0B, "rx_mcast" },
-   { 0x0C, "rx_ucast" },
-   { 0x0D, "rx_64_or_less" },
-   { 0x0E, "rx_65_127" },
-   { 0x0F, "rx_128_255" },
-   { 0x10, "rx_256_511" },
-   { 0x11, "rx_512_1023" },
-   { 0x12, "rx_1024_1522" },
-   { 0x13, "rx_1523_2000" },
-   { 0x14, "rx_2001" },
-   { 0x15, "tx_hi" },
-   { 0x16, "tx_late_col" },
-   { 0x17, "tx_pause" },
-   { 0x18, "tx_bcast" },
-   { 0x19, "tx_mcast" },
-   { 0x1A, "tx_ucast" },
-   { 0x1B, "tx_deferred" },
-   { 0x1C, "tx_total_col" },
-   { 0x1D, "tx_exc_col" },
-   { 0x1E, "tx_single_col" },
-   { 0x1F, "tx_mult_col" },
-   { 0x80, "rx_total" },
-   { 0x81, "tx_total" },
-   { 0x82, "rx_discards" },
-   { 0x83, "tx_discards" },
-};
-
-static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, 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, bool set) -{
-   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,
-bool 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, bool 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 int wait_vlan_ctrl_ready(struct ksz_device *dev, u32 waiton, int 
timeout) -{
-   u8 data;
-
-   do {
-   ksz_read8(dev, REG_SW_VLAN_CTRL, &data);
-   if (!(data & waiton))
-   break;
-   usleep_range(1, 10);
-   } while (timeout-- > 0);
-
-   if (timeout <= 0)
-   return -ETIMEDOUT;
-
-   return 0;
-}
-
-static int get_vlan_table(struct dsa_switch *ds, u16 vid, u32 *vlan_table) -{
-   struct ksz_device *dev = ds->priv;
-   int ret;
-
-   mutex_lock(&dev->vlan_mutex);
-
-   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 */
-   ret = wait_vlan_ctrl_ready(dev, VLAN_START, 1000);
-   if (ret < 0) {
-   dev_dbg(dev->dev, "Failed to read vlan table\n");
-   goto exit;
-   }
-
-   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);
-
-exit:
-   mutex_unlock(&dev->vlan_mutex);
-
-   return ret;
-}
-
-static int set_vlan_table(struct dsa_switch *ds, u16 vid, u32 *vlan_table) -{
-   struct ksz_device *dev = ds->priv;
-   int ret;
-
-   mutex_lock(&dev->vlan_mutex);
-
-   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);
-
-   /* wait to be cleared */
-   ret = wait_vlan_ctrl_ready(dev, VLAN_START, 1000);
-   if (ret < 0) {
-   dev_dbg(dev->dev, "Failed to write vla

[PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ switch drivers.

[Tristram.Ha]

From: Tristram Ha 

Break ksz_common.c into 2 files so that the common code can be used by other 
KSZ switch drivers.

Signed-off-by: Tristram Ha 
---
diff --git a/drivers/net/dsa/microchip/Makefile 
b/drivers/net/dsa/microchip/Makefile
index ed335e2..0961c30 100644
--- a/drivers/net/dsa/microchip/Makefile
+++ b/drivers/net/dsa/microchip/Makefile
@@ -1,2 +1,2 @@
-obj-$(CONFIG_MICROCHIP_KSZ)+= ksz_common.o
+obj-$(CONFIG_MICROCHIP_KSZ)+= ksz9477.o ksz_common.o
 obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o

[PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added

[Tristram.Ha]

From: Tristram Ha 

Add other KSZ switches support so that patch check does not complain.

Signed-off-by: Tristram Ha 
---
 Documentation/devicetree/bindings/net/dsa/ksz.txt | 117 --
 1 file changed, 62 insertions(+), 55 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt 
b/Documentation/devicetree/bindings/net/dsa/ksz.txt
index 0ab8b39..34af0e0 100644
--- a/Documentation/devicetree/bindings/net/dsa/ksz.txt
+++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt
@@ -3,8 +3,15 @@ Microchip KSZ Series Ethernet switches
 
 Required properties:
 
-- compatible: For external switch chips, compatible string must be exactly one
-  of: "microchip,ksz9477"
+- compatible: Should be "microchip,ksz9477" for KSZ9477 chip,
+ "microchip,ksz8795" for KSZ8795 chip,
+ "microchip,ksz8794" for KSZ8794 chip,
+ "microchip,ksz8765" for KSZ8765 chip,
+ "microchip,ksz8895" for KSZ8895 chip,
+ "microchip,ksz8864" for KSZ8864 chip,
+ "microchip,ksz8873" for KSZ8873 chip,
+ "microchip,ksz8863" for KSZ8863 chip,
+ "microchip,ksz8463" for KSZ8463 chip
 
 See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional  
required and optional properties.
@@ -13,60 +20,60 @@ Examples:
 
 Ethernet switch connected via SPI to the host, CPU port wired to eth0:
 
- eth0: ethernet@10001000 {
- fixed-link {
- speed = <1000>;
- full-duplex;
- };
- };
+   eth0: ethernet@10001000 {
+   fixed-link {
+   speed = <1000>;
+   full-duplex;
+   };
+   };
 
- spi1: spi@f8008000 {
- pinctrl-0 = <&pinctrl_spi_ksz>;
- cs-gpios = <&pioC 25 0>;
- id = <1>;
- status = "okay";
+   spi1: spi@f8008000 {
+   cs-gpios = <&pioC 25 0>;
+   id = <1>;
+   status = "okay";
 
- ksz9477: ksz9477@0 {
- compatible = 
"microchip,ksz9477";
- reg = <0>;
+   ksz9477: ksz9477@0 {
+   compatible = "microchip,ksz9477";
+   reg = <0>;
 
- spi-max-frequency 
= <4400>;
- spi-cpha;
- spi-cpol;
+   spi-max-frequency = <4400>;
+   spi-cpha;
+   spi-cpol;
+
+   status = "okay";
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   port@0 {
+   reg = <0>;
+   label = "lan1";
+   };
+   port@1 {
+   reg = <1>;
+   label = "lan2";
+   };
+   port@2 {
+   reg = <2>;
+   label = "lan3";
+   };
+   port@3 {
+   reg = <3>;
+   label = "lan4";
+   };
+   port@4 {
+   reg = <4>;
+   label = "lan5";
+   };
+   port@5 {
+   reg = <5>;
+   label = "cpu";
+   ethernet = <ð0>;
+   fixed-link {
+   speed = <1000>;
+   full-duplex;
+   };
+   };
+   };
+   };
+   };
 
- status = "okay";
- ports {
- 
#address-cells = <1>;
-  

[PATCH RFC 1/5] Add KSZ8795 switch driver support in Kconfig

[Tristram.Ha]

From: Tristram Ha 

Add KSZ8795 switch support with SPI access.

Signed-off-by: Tristram Ha 
---
diff --git a/drivers/net/dsa/microchip/Kconfig 
b/drivers/net/dsa/microchip/Kconfig
index a8b8f59..0b6225e 100644
--- a/drivers/net/dsa/microchip/Kconfig
+++ b/drivers/net/dsa/microchip/Kconfig
@@ -10,3 +10,21 @@ config MICROCHIP_KSZ_SPI_DRIVER
depends on MICROCHIP_KSZ && SPI
help
  Select to enable support for registering switches configured through 
SPI.
+
+menuconfig MICROCHIP_KSZ8795
+   tristate "Microchip KSZ8795 series switch support"
+   depends on NET_DSA
+   select NET_DSA_TAG_KSZ
+   help
+ This driver adds support for Microchip KSZ8795 switch chips.
+
+config MICROCHIP_KSZ8795_SPI_DRIVER
+   tristate "KSZ8795 series SPI connected switch driver"
+   depends on MICROCHIP_KSZ8795 && SPI
+   default y
+   help
+ This driver accesses KSZ8795 chip through SPI.
+
+ It is required to use the KSZ8795 switch driver as the only access
+ is through SPI.
+

[PATCH RFC 0/5] Add DSA driver support for KSZ8795 switch

[Tristram.Ha]

From: Tristram Ha 

This series of patches is to add DSA driver support for KSZ8795 switch.

Previous patches for KSZ9477 have to be applied first before these.

This patch set is against net-next.

 drivers/net/dsa/microchip/Kconfig   |   18 +
 drivers/net/dsa/microchip/Makefile  |2 +
 drivers/net/dsa/microchip/ksz8795.c | 2066 +++
 drivers/net/dsa/microchip/ksz8795.h | 1015 +++
 drivers/net/dsa/microchip/ksz8795_spi.c |  229 
 5 files changed, 3330 insertions(+)
 create mode 100644 drivers/net/dsa/microchip/ksz8795.c
 create mode 100644 drivers/net/dsa/microchip/ksz8795.h
 create mode 100644 drivers/net/dsa/microchip/ksz8795_spi.c

[PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile

[Tristram.Ha]

From: Tristram Ha 

Add KSZ8795 switch support with SPI access.

Signed-off-by: Tristram Ha 
---
diff --git a/drivers/net/dsa/microchip/Makefile 
b/drivers/net/dsa/microchip/Makefile
index 0961c30..0d8ba48 100644
--- a/drivers/net/dsa/microchip/Makefile
+++ b/drivers/net/dsa/microchip/Makefile
@@ -1,2 +1,4 @@
 obj-$(CONFIG_MICROCHIP_KSZ)+= ksz9477.o ksz_common.o
 obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
+obj-$(CONFIG_MICROCHIP_KSZ8795)+= ksz8795.o ksz_common.o
+obj-$(CONFIG_MICROCHIP_KSZ8795_SPI_DRIVER) += ksz8795_spi.o

[PATCH RFC 3/5] Add KSZ8795 switch driver

[Tristram.Ha]

From: Tristram Ha 

Add KSZ8795 switch support with function code.

Signed-off-by: Tristram Ha 
---
diff --git a/drivers/net/dsa/microchip/ksz8795.c 
b/drivers/net/dsa/microchip/ksz8795.c
new file mode 100644
index 000..e4d4e6a
--- /dev/null
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -0,0 +1,2066 @@
+/*
+ * Microchip KSZ8795 switch driver
+ *
+ * Copyright (C) 2017 Microchip Technology Inc.
+ * Tristram Ha 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ksz_priv.h"
+#include "ksz8795.h"
+
+/**
+ * Some counters do not need to be read too often because they are less likely
+ * to increase much.
+ */
+static const struct {
+   int interval;
+   char string[ETH_GSTRING_LEN];
+} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
+   { 1, "rx_hi" },
+   { 4, "rx_undersize" },
+   { 4, "rx_fragments" },
+   { 4, "rx_oversize" },
+   { 4, "rx_jabbers" },
+   { 4, "rx_symbol_err" },
+   { 4, "rx_crc_err" },
+   { 4, "rx_align_err" },
+   { 4, "rx_mac_ctrl" },
+   { 1, "rx_pause" },
+   { 1, "rx_bcast" },
+   { 1, "rx_mcast" },
+   { 1, "rx_ucast" },
+   { 2, "rx_64_or_less" },
+   { 2, "rx_65_127" },
+   { 2, "rx_128_255" },
+   { 2, "rx_256_511" },
+   { 2, "rx_512_1023" },
+   { 2, "rx_1024_1522" },
+   { 2, "rx_1523_2000" },
+   { 2, "rx_2001" },
+   { 1, "tx_hi" },
+   { 4, "tx_late_col" },
+   { 1, "tx_pause" },
+   { 1, "tx_bcast" },
+   { 1, "tx_mcast" },
+   { 1, "tx_ucast" },
+   { 4, "tx_deferred" },
+   { 4, "tx_total_col" },
+   { 4, "tx_exc_col" },
+   { 4, "tx_single_col" },
+   { 4, "tx_mult_col" },
+   { 1, "rx_total" },
+   { 1, "tx_total" },
+   { 2, "rx_discards" },
+   { 2, "tx_discards" },
+};
+
+static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set)
+{
+   u8 data;
+
+   ksz_read8(dev, addr, &data);
+   if (set)
+   data |= bits;
+   else
+   data &= ~bits;
+   ksz_write8(dev, addr, data);
+}
+
+static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
+bool 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 int chk_last_port(struct ksz_device *dev, int p)
+{
+   if (dev->last_port && p == dev->last_port)
+   p = dev->cpu_port;
+   return p;
+}
+
+static int ksz_reset_switch(struct ksz_device *dev)
+{
+   /* reset switch */
+   ksz_write8(dev, REG_POWER_MANAGEMENT_1,
+  SW_SOFTWARE_POWER_DOWN << SW_POWER_MANAGEMENT_MODE_S);
+   ksz_write8(dev, REG_POWER_MANAGEMENT_1, 0);
+
+   return 0;
+}
+
+static void port_set_prio_queue(struct ksz_device *dev, int port, int queue)
+{
+   u8 hi;
+   u8 lo;
+
+   switch (queue) {
+   case 4:
+   case 3:
+   queue = PORT_QUEUE_SPLIT_4;
+   break;
+   case 2:
+   queue = PORT_QUEUE_SPLIT_2;
+   break;
+   default:
+   queue = PORT_QUEUE_SPLIT_1;
+   }
+   ksz_pread8(dev, port, REG_PORT_CTRL_0, &lo);
+   ksz_pread8(dev, port, P_DROP_TAG_CTRL, &hi);
+   lo &= ~PORT_QUEUE_SPLIT_L;
+   if (queue & 1)
+   lo |= PORT_QUEUE_SPLIT_L;
+   hi &= ~PORT_QUEUE_SPLIT_H;
+   if (queue & 2)
+   hi |= PORT_QUEUE_SPLIT_H;
+   ksz_pwrite8(dev, port, REG_PORT_CTRL_0, lo);
+   ksz_pwrite8(dev, port, P_DROP_TAG_CTRL, hi);
+
+   /* Default is port based for egress rate limit. */
+   if (queue)
+   ksz_cfg(dev, REG_SW_CTRL_19, SW_OUT_RATE_LIMIT_QUEUE_BASED,
+   true);
+}
+
+static void port_r_mib_cnt(struct ksz_device *dev, int port, u16 addr, u64 
*cnt)
+{
+   u32 data;
+   u16 ctrl_addr;
+   u8 check;
+   int timeout;
+
+   ctrl_addr = addr + SWITCH_COUNTER_NUM * port;
+
+   ctrl_addr |= IND_ACC_TABLE(TABLE_MIB | TABLE_READ);
+  

[PATCH RFC 5/5] Add KSZ8795 SPI driver

[Tristram.Ha]

From: Tristram Ha 

Add KSZ8795 switch support with SPI access.

Signed-off-by: Tristram Ha 
---
diff --git a/drivers/net/dsa/microchip/ksz8795_spi.c 
b/drivers/net/dsa/microchip/ksz8795_spi.c
new file mode 100644
index 000..0f9c731
--- /dev/null
+++ b/drivers/net/dsa/microchip/ksz8795_spi.c
@@ -0,0 +1,229 @@
+/*
+ * Microchip KSZ8795 series register access through SPI
+ *
+ * Copyright (C) 2017 Microchip Technology Inc.
+ * Tristram Ha 
+ *
+ * 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 
+
+#include 
+#include 
+#include 
+#include 
+
+#include "ksz_priv.h"
+
+int ksz8795_switch_register(struct ksz_device *dev);
+
+/* SPI frame opcodes */
+#define KS_SPIOP_RD3
+#define KS_SPIOP_WR2
+
+#define SPI_ADDR_S 12
+#define SPI_ADDR_M (BIT(SPI_ADDR_S) - 1)
+#define SPI_TURNAROUND_S   1
+
+/* Enough to read all switch registers. */
+#define SPI_TX_BUF_LEN 0x100
+
+static int ksz_spi_read_reg(struct spi_device *spi, u32 reg, u8 *val,
+   unsigned int len)
+{
+   u16 txbuf;
+   int ret;
+
+   txbuf = reg & SPI_ADDR_M;
+   txbuf |= KS_SPIOP_RD << SPI_ADDR_S;
+   txbuf <<= SPI_TURNAROUND_S;
+   txbuf = cpu_to_be16(txbuf);
+
+   ret = spi_write_then_read(spi, &txbuf, 2, 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_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 int ksz_spi_write_reg(struct spi_device *spi, u32 reg, u8 *val,
+unsigned int len)
+{
+   u16 *txbuf = (u16 *)val;
+
+   *txbuf = reg & SPI_ADDR_M;
+   *txbuf |= (KS_SPIOP_WR << SPI_ADDR_S);
+   *txbuf <<= SPI_TURNAROUND_S;
+   *txbuf = cpu_to_be16(*txbuf);
+
+   return spi_write(spi, txbuf, 2 + len); }
+
+static int ksz_spi_write(struct ksz_device *dev, u32 reg, void *data,
+unsigned int len)
+{
+   struct spi_device *spi = dev->priv;
+
+   if (len > SPI_TX_BUF_LEN)
+   len = SPI_TX_BUF_LEN;
+   memcpy(&dev->txbuf[2], data, len);
+   return ksz_spi_write_reg(spi, reg, dev->txbuf, len); }
+
+static unsigned int ksz_spi_get(struct ksz_device *dev, u32 reg, void *data,
+   unsigned int len)
+{
+   int err;
+
+   err = ksz_spi_read(dev, reg, data, len);
+   if (err)
+   return 0;
+   return len;
+}
+
+static unsigned int ksz_spi_set(struct ksz_device *dev, u32 reg, void *data,
+   unsigned int len)
+{
+   int err;
+
+   err = ksz_spi_write(dev, reg, data, len);
+   if (err)
+   return 0;
+   return len;
+}
+
+static int ksz_spi_write8(struct ksz_device *dev, u32 reg, u8 value) {
+   return ksz_spi_write(dev, reg, &value, 1); }
+
+static int ksz_spi_write16(struct ksz_device *dev, u32 reg, u16 value) 
+{
+   value = cpu_to_be16(value);
+   return ksz_spi_write(dev, reg, &value, 2); }
+
+static int ksz_spi_write32(struct ksz_device *dev, u32 reg, u32 value) 
+{
+   value = cpu_to_be32(value);
+   return ksz_spi_write(dev, reg, &value, 4); }
+
+static const struct ksz_io_ops ksz_spi_ops = {
+   .read8 = ksz_spi_read8,
+   .read16 = ksz_spi_read16,
+   .read32 = ksz_spi_read32,
+   .write8 = ksz_spi_write8,
+   .write16 = ksz_spi_write16,
+   .write32 = ksz_spi_write32,
+   .get = ksz_spi_get,
+   .set = ksz_spi_set,
+};
+
+static int ksz_spi_probe(struct spi_device *spi) {
+   struct ksz_device *dev;
+   int ret;
+

[PATCH RFC 4/5] Add KSZ8795 register definitions

[Tristram.Ha]

From: Tristram Ha 

Add KSZ8795 switch support with register definitions.

Signed-off-by: Tristram Ha 
---
diff --git a/drivers/net/dsa/microchip/ksz8795.h 
b/drivers/net/dsa/microchip/ksz8795.h
new file mode 100644
index 000..005eda5
--- /dev/null
+++ b/drivers/net/dsa/microchip/ksz8795.h
@@ -0,0 +1,1015 @@
+/**
+ * Microchip KSZ8795 definition file
+ *
+ * Copyright (c) 2017 Microchip Technology Inc.
+ * Tristram Ha 
+ *
+ * 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 __KSZ8795_H
+#define __KSZ8795_H
+
+#define KS_PORT_M  0x1F
+
+#define KS_PRIO_M  0x3
+#define KS_PRIO_S  2
+
+#define REG_CHIP_ID0   0x00
+
+#define FAMILY_ID  0x87
+
+#define REG_CHIP_ID1   0x01
+
+#define SW_CHIP_ID_M   0xF0
+#define SW_CHIP_ID_S   4
+#define SW_REVISION_M  0x0E
+#define SW_REVISION_S  1
+#define SW_START   0x01
+
+#define CHIP_ID_94 0x60
+#define CHIP_ID_95 0x90
+
+#define REG_SW_CTRL_0  0x02
+
+#define SW_NEW_BACKOFF BIT(7)
+#define SW_GLOBAL_RESETBIT(6)
+#define SW_FLUSH_DYN_MAC_TABLE BIT(5)
+#define SW_FLUSH_STA_MAC_TABLE BIT(4)
+#define SW_LINK_AUTO_AGING BIT(0)
+
+#define REG_SW_CTRL_1  0x03
+
+#define SW_HUGE_PACKET BIT(6)
+#define SW_TX_FLOW_CTRL_DISABLEBIT(5)
+#define SW_RX_FLOW_CTRL_DISABLEBIT(4)
+#define SW_CHECK_LENGTHBIT(3)
+#define SW_AGING_ENABLEBIT(2)
+#define SW_FAST_AGING  BIT(1)
+#define SW_AGGR_BACKOFFBIT(0)
+
+#define REG_SW_CTRL_2  0x04
+
+#define UNICAST_VLAN_BOUNDARY  BIT(7)
+#define MULTICAST_STORM_DISABLEBIT(6)
+#define SW_BACK_PRESSURE   BIT(5)
+#define FAIR_FLOW_CTRL BIT(4)
+#define NO_EXC_COLLISION_DROP  BIT(3)
+#define SW_LEGAL_PACKET_DISABLEBIT(1)
+
+#define REG_SW_CTRL_3  0x05
+ #define WEIGHTED_FAIR_QUEUE_ENABLEBIT(3)
+
+#define SW_VLAN_ENABLE BIT(7)
+#define SW_IGMP_SNOOP  BIT(6)
+#define SW_MIRROR_RX_TXBIT(0)
+
+#define REG_SW_CTRL_4  0x06
+
+#define SW_HALF_DUPLEX_FLOW_CTRL   BIT(7)
+#define SW_HALF_DUPLEX BIT(6)
+#define SW_FLOW_CTRL   BIT(5)
+#define SW_10_MBIT BIT(4)
+#define SW_REPLACE_VID BIT(3)
+#define BROADCAST_STORM_RATE_HI0x07
+
+#define REG_SW_CTRL_5  0x07
+
+#define BROADCAST_STORM_RATE_LO0xFF
+#define BROADCAST_STORM_RATE   0x07FF
+
+#define REG_SW_CTRL_6  0x08
+
+#define SW_MIB_COUNTER_FLUSH   BIT(7)
+#define SW_MIB_COUNTER_FREEZE  BIT(6)
+#define SW_MIB_COUNTER_CTRL_ENABLE KS_PORT_M
+
+#define REG_SW_CTRL_9  0x0B
+
+#define SPI_CLK_125_MHZ0x80
+#define SPI_CLK_62_5_MHZ   0x40
+#define SPI_CLK_31_25_MHZ  0x00
+
+#define SW_LED_MODE_M  0x3
+#define SW_LED_MODE_S  4
+#define SW_LED_LINK_ACT_SPEED  0
+#define SW_LED_LINK_ACT1
+#define SW_LED_LINK_ACT_DUPLEX 2
+#define SW_LED_LINK_DUPLEX 3
+
+#define REG_SW_CTRL_10 0x0C
+
+#define SW_TAIL_TAG_ENABLE BIT(1)
+#define SW_PASS_PAUSE  BIT(0)
+
+#define REG_SW_CTRL_11 0x0D
+
+#define REG_POWER_MANAGEMENT_1 0x0E
+
+#define SW_PLL_POWER_DOWN  BIT(5)
+#define SW_POWER_MANAGEMENT_MODE_M 0x3
+#define SW_POWER_MANAGEMENT_MODE_S 3
+#define SW_POWER_NORMAL0
+#define SW_ENERGY_DETECTION1
+#define SW_SOFTWARE_POWER_DOWN 2
+
+#define REG_POWER_MANAGEMENT_2 0x0F
+
+#define REG_PORT_1_CTRL_0  0x10
+#define REG_PORT_2_CTRL_0  0x20
+#define REG_PORT_3_CTRL_0  0x30
+#define REG_PORT_4_CTRL_0  0x40
+#define REG_P

RE: [PATCH RFC 2/6] Create new file ksz9477.c from KSZ9477 code in ksz_common.c

[Tristram.Ha]

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Thursday, September 07, 2017 2:33 PM
> To: Tristram Ha - C24268
> Cc: muva...@gmail.com; pa...@ucw.cz; nathan.leigh.con...@gmail.com;
> vivien.dide...@savoirfairelinux.com; f.faine...@gmail.com;
> net...@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC 2/6] Create new file ksz9477.c from KSZ9477 code in
> ksz_common.c
> 
> On Thu, Sep 07, 2017 at 09:09:04PM +, tristram...@microchip.com wrote:
> > From: Tristram Ha 
> >
> > Create new ksz9477.c file from original ksz_common.c.
> >
> > Signed-off-by: Tristram Ha 
> > ---
> > diff --git a/drivers/net/dsa/microchip/ksz9477.c
> > b/drivers/net/dsa/microchip/ksz9477.c
> > new file mode 100644
> > index 000..bc722b4
> > --- /dev/null
> > +++ b/drivers/net/dsa/microchip/ksz9477.c
> > @@ -0,0 +1,1317 @@
> > +/*
> > + * Microchip switch driver main logic
> > + *
> > + * 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
> 
> Tristram
> 
> It looks like something hand mangled this comment. "any" and "above"
> appear to be on a line on there own.
> 
> > + * 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 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "ksz_priv.h"
> > +#include "ksz_9477_reg.h"
> > +
> > +static const struct {
> > +   int index;
> > +   char string[ETH_GSTRING_LEN];
> > +} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
> > +   { 0x00, "rx_hi" },
> > +   { 0x01, "rx_undersize" },
> > +   { 0x02, "rx_fragments" },
> > +   { 0x03, "rx_oversize" },
> > +   { 0x04, "rx_jabbers" },
> > +   { 0x05, "rx_symbol_err" },
> > +   { 0x06, "rx_crc_err" },
> > +   { 0x07, "rx_align_err" },
> > +   { 0x08, "rx_mac_ctrl" },
> > +   { 0x09, "rx_pause" },
> > +   { 0x0A, "rx_bcast" },
> > +   { 0x0B, "rx_mcast" },
> > +   { 0x0C, "rx_ucast" },
> > +   { 0x0D, "rx_64_or_less" },
> > +   { 0x0E, "rx_65_127" },
> > +   { 0x0F, "rx_128_255" },
> > +   { 0x10, "rx_256_511" },
> > +   { 0x11, "rx_512_1023" },
> > +   { 0x12, "rx_1024_1522" },
> > +   { 0x13, "rx_1523_2000" },
> > +   { 0x14, "rx_2001" },
> > +   { 0x15, "tx_hi" },
> > +   { 0x16, "tx_late_col" },
> > +   { 0x17, "tx_pause" },
> > +   { 0x18, "tx_bcast" },
> > +   { 0x19, "tx_mcast" },
> > +   { 0x1A, "tx_ucast" },
> > +   { 0x1B, "tx_deferred" },
> > +   { 0x1C, "tx_total_col" },
> > +   { 0x1D, "tx_exc_col" },
> > +   { 0x1E, "tx_single_col" },
> > +   { 0x1F, "tx_mult_col" },
> > +   { 0x80, "rx_total" },
> > +   { 0x81, "tx_total" },
> > +   { 0x82, "rx_discards" },
> > +   { 0x83, "tx_discards" },
> > +};
> > +
> > +static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool
> > +set) {
> 
> More mangling? Should set be on the end of the previous line?
> 
> > +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]); }
> 
> More mangling? } at the end of a line?
> 
> I will stop reading now and wait for a v2 this is not corrupt.
> 
>   Andrew

Sorry about that.  It seems my e-mail system wraps the line too soon.

RE: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ switch drivers.

[Tristram.Ha]

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Thursday, September 07, 2017 2:25 PM
> To: Tristram Ha - C24268
> Cc: muva...@gmail.com; pa...@ucw.cz; nathan.leigh.con...@gmail.com;
> vivien.dide...@savoirfairelinux.com; f.faine...@gmail.com;
> net...@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ
> switch drivers.
> 
> On Thu, Sep 07, 2017 at 09:08:58PM +, tristram...@microchip.com wrote:
> > From: Tristram Ha 
> >
> > Break ksz_common.c into 2 files so that the common code can be used by other
> KSZ switch drivers.
> >
> > Signed-off-by: Tristram Ha 
> > ---
> > diff --git a/drivers/net/dsa/microchip/Makefile
> > b/drivers/net/dsa/microchip/Makefile
> > index ed335e2..0961c30 100644
> > --- a/drivers/net/dsa/microchip/Makefile
> > +++ b/drivers/net/dsa/microchip/Makefile
> > @@ -1,2 +1,2 @@
> > -obj-$(CONFIG_MICROCHIP_KSZ)+= ksz_common.o
> > +obj-$(CONFIG_MICROCHIP_KSZ)+= ksz9477.o ksz_common.o
> >  obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
> 
> Hi Tristram
> 
> I would of thought this would break the build. You don't add ksz9477.c until 
> the
> next patch.
> 
> Each patch needs to compile, otherwise you break git bisect.
> 
>  Andrew

Eventually the file will need to be broken in two, so you would like to see all 
3 changes (Makefile, ksz_common.c, and ksz9477.c) in 1 patch file?

RE: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile

[Tristram.Ha]

> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Thursday, September 07, 2017 2:56 PM
> To: Tristram Ha - C24268
> Cc: muva...@gmail.com; pa...@ucw.cz; nathan.leigh.con...@gmail.com;
> vivien.dide...@savoirfairelinux.com; f.faine...@gmail.com;
> net...@vger.kernel.org; linux-kernel@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile
> 
> On Thu, Sep 07, 2017 at 09:17:10PM +, tristram...@microchip.com wrote:
> > From: Tristram Ha 
> >
> > Add KSZ8795 switch support with SPI access.
> >
> > Signed-off-by: Tristram Ha 
> > ---
> > diff --git a/drivers/net/dsa/microchip/Makefile
> b/drivers/net/dsa/microchip/Makefile
> > index 0961c30..0d8ba48 100644
> > --- a/drivers/net/dsa/microchip/Makefile
> > +++ b/drivers/net/dsa/microchip/Makefile
> > @@ -1,2 +1,4 @@
> >  obj-$(CONFIG_MICROCHIP_KSZ)+= ksz9477.o ksz_common.o
> >  obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
> > +obj-$(CONFIG_MICROCHIP_KSZ8795)+= ksz8795.o ksz_common.o
> > +obj-$(CONFIG_MICROCHIP_KSZ8795_SPI_DRIVER) += ksz8795_spi.o
> 
> I've not tried it, but i think this breaks the build
> 
>  Andrew

So you would like to have all 5 patches in 1 patch file?

RE: [PATCH RFC 3/5] Add KSZ8795 switch driver

[Tristram.Ha]

The actual SPI access performance will depend on the SPI host controller.
The SPI access speed, ranging from 12 MHz to 50 MHz depending on the
chip, is a factor but the performance of the SPI host controller is more
important.  Generally the SPI host controller scales down the clock by a
factor of 2.  So if the maximum 50 Mhz does not work for the chip the next
speed is 25 Mhz.  Most of the SPI host controllers work in range of 20-30 Mhz
with Microchip SPI switches.

For example, Raspberry Pi 2 has 2 SPI host controllers.  The new code uses the
newer host controller which has better performance even when running in
slower SPI speed.

It is hard to measure the actual SPI performance of the switch, but for SPI
Ethernet controller the performance can be viewed by running throughput test
as SPI is responsible for passing network frames between system and device.

The ODROID C1 (A Raspberry Pi like SoC) has the best SPI performance, even not
running in the highest SPI speed.

Typical SPI register read takes about 120 microseconds.

Now back to my concern about SPI access.  It is not accessible in interrupt 
context.
Even in a timer callback a work queue has to be scheduled to program the 
hardware
registers.  My concern is if a task is already running with SPI access to a lot 
of registers
like reading the 32 MIB counters in every port of the switch, another register 
access
has to wait until they are finished.  This normally does not pose a problem in
regular switch operation, but there are some situations it will create a 
problem.

One of the situations is running RSTP Conformance Test.  The test case sends a 
BPDU
to open/close the port and then send traffic to test if the port is really 
opened/closed.
For software implementation which receives the BPDU and all network traffic it 
is
reasonable to expect the software opens/closes the port and then can regulate 
the
network traffic whatever it wants, but for a hardware implementation which 
programs
a register to open/close the port then it is critical this register write can 
be executed as
soon as possible.

Another situation is getting the PTP transmit timestamp of a PTP event message.
The Microchip PTP switch uses registers to store the Sync, Delay_Req, and 
Pdelay_Resp
timestamps.  If this register is not read as soon as possible and another 
message of the
same type is sent, the last timestamp is lost.  Software can regulate the 
sending of
these messages and this situation does not happen in normal operation.  But in 
a stress
test this PTP operation definitely cannot handle it.

I know this MIB counter reading implementation cannot guarantee those urgent 
register
access to happen promptly, but it minimizes the chance of blocking those 
accesses in normal
operation.


> -Original Message-
> From: Andrew Lunn [mailto:and...@lunn.ch]
> Sent: Friday, September 29, 2017 5:12 AM
> To: David Laight
> Cc: Tristram Ha - C24268; muva...@gmail.com; pa...@ucw.cz;
> nathan.leigh.con...@gmail.com; vivien.dide...@savoirfairelinux.com;
> f.faine...@gmail.com; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC 3/5] Add KSZ8795 switch driver
> 
> On Fri, Sep 29, 2017 at 09:14:26AM +, David Laight wrote:
> > From: Andrew Lunn
> > > Sent: 28 September 2017 20:34
> > ...
> > > > There are 34 counters.  In normal case using generic bus I/O or PCI to
> read them
> > > > is very quick, but the switch is mostly accessed using SPI, or even I2C.
> As the SPI
> > > > access is very slow.
> > >
> > > How slow is it? The Marvell switches all use MDIO. It is probably a
> > > bit faster than I2C, but it is a lot slower than MMIO or PCI.
> > >
> > > ethtool -S lan0 takes about 25ms.
> >
> > Is the SPI access software bit-banged?
> 
> That will depend on the board design. I've used mdio bit banging, and
> that was painfully slow for stats.
> 
> But we should primarily think about average hardware. It is going to
> have hardware SPI or I2C. If statistics reading with hardware I2C is
> reasonable, i would avoid caching, and just ensure other accesses are
> permitted between individual statistic reads.
> 
> It also requires Microchip also post new code. They have been very
> silent for quite a while
> 
> Andrew

RE: [PATCH RFC 3/5] Add KSZ8795 switch driver

[Tristram.Ha]

> > > > > Similar code will be needed by other drivers, right?
> > > >
> > > > Although KSZ8795 and KSZ8895 may use the same code, the other
> > > > chips will have different code.
> > >
> > > Ok, please make sure code is shared between these two.
> >
> > The exact function probably cannot be shared between KSZ8795
> > and KSZ8895 drivers because although the register constants look
> > the same but their exact locations are different in the 2 chips.
> 
> Put the code into header as static inline and include it from both
> places?
> 

Although KSZ8795 and KSZ8895 share the same code when simulating
the PHY register access, even though the exact registers are not the
same, this code needs a little modification for another chip.  It also looks
too large to put in a header.

RE: [PATCH RFC 3/5] Add KSZ8795 switch driver

[Tristram.Ha]

> On Mon 2017-09-18 20:27:13, tristram...@microchip.com wrote:
> > > > +/**
> > > > + * Some counters do not need to be read too often because they are
> less
> > > likely
> > > > + * to increase much.
> > > > + */
> > >
> > > What does comment mean? Are you caching statistics, and updating
> > > different values at different rates?
> > >
> >
> > There are 34 counters.  In normal case using generic bus I/O or PCI to read
> them
> > is very quick, but the switch is mostly accessed using SPI, or even I2C.  As
> the SPI
> > access is very slow and cannot run in interrupt context I keep worrying
> reading
> > the MIB counters in a loop for 5 or more ports will prevent other critical
> hardware
> > access from executing soon enough.  These accesses can be getting 1588
> PTP
> > timestamps and opening/closing ports.  (RSTP Conformance Test sends test
> traffic
> > to port supposed to be closed/opened after receiving specific RSTP
> > BPDU.)
> 
> Hmm. Ok, interesting.
> 
> I wonder how well this is going to work if userspace actively 'does
> something' with the switch.
> 
> It seems to me that even if your statistics code is careful not to do
> 'a lot' of accesses at the same time, userspace can use other parts of
> the driver to do the same, and thus cause same unwanted effects...

If the user calls "ethtool -S" in a tight loop the system will waste a lot of
CPU time, but this is more like a user error.
Another solution is not to schedule to read the MIB counters in that
function call.  I think I was doing a favor to update the MIB counters
sooner as the user probably wants to find out what is wrong with the
switch by reading the MIB counters and checking them several times.
For system tracking like SNMP I think it is likely a separate mechanism
is used to gather those information.  If I am wrong that function definitely
needs to be modified.

RE: [PATCH RFC 3/5] Add KSZ8795 switch driver

[Tristram.Ha]

> > My concern is if a task is already running with SPI access to a lot
> > of registers like reading the 32 MIB counters in every port of the
> > switch, another register access has to wait until they are finished.
> 
> Why does it have to wait? Looking at the code in
> ksz_get_ethtool_stats(), you don't take any mutex which will prevent
> others from using the SPI bus. All there is is a mutex which prevents
> two sets of ksz_get_ethtool_stats() at the same time.
> 
> So a PTP read could happen in parallel, and will not be blocked by MIB
> reads. They should get interleaved access to the SPI bus.
> 

The MIB counters are read in the background.  For multiple CPU cores 2
tasks may run in the same time allowing SPI access one after another.
For single core I am not sure an SPI access like coming from an interrupt
routine can jump ahead from one in a background task.

I know nowadays SoCs are powerful enough to do amazing things.  It is
just I spent a long time using a low-powered SoC doing switch driver
development.

[PATCH v1 RFC 1/7] Replace license with GPL

[Tristram.Ha]

From: Tristram Ha 

Replace license with GPL.

Signed-off-by: Tristram Ha 
---
 drivers/net/dsa/microchip/ksz_9477_reg.h | 23 ---
 drivers/net/dsa/microchip/ksz_common.c   | 23 ---
 drivers/net/dsa/microchip/ksz_priv.h | 23 ---
 drivers/net/dsa/microchip/ksz_spi.c  | 23 ---
 4 files changed, 48 insertions(+), 44 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_9477_reg.h 
b/drivers/net/dsa/microchip/ksz_9477_reg.h
index 6aa6752..26a0e4b 100644
--- a/drivers/net/dsa/microchip/ksz_9477_reg.h
+++ b/drivers/net/dsa/microchip/ksz_9477_reg.h
@@ -1,19 +1,20 @@
 /*
  * Microchip KSZ9477 register definitions
  *
- * Copyright (C) 2017
+ * Copyright (C) 2017 Microchip Technology Inc.
  *
- * 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.
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
  *
- * 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.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see .
  */
 
 #ifndef __KSZ9477_REGS_H
diff --git a/drivers/net/dsa/microchip/ksz_common.c 
b/drivers/net/dsa/microchip/ksz_common.c
index 56cd6d3..685dafd 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1,19 +1,20 @@
 /*
  * Microchip switch driver main logic
  *
- * Copyright (C) 2017
+ * Copyright (C) 2017 Microchip Technology Inc.
  *
- * 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.
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
  *
- * 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.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see .
  */
 
 #include 
diff --git a/drivers/net/dsa/microchip/ksz_priv.h 
b/drivers/net/dsa/microchip/ksz_priv.h
index 2a98dbd..d461468 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -1,19 +1,20 @@
 /*
  * Microchip KSZ series switch common definitions
  *
- * Copyright (C) 2017
+ * Copyright (C) 2017 Microchip Technology Inc.
  *
- * 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.
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
  *
- * 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 N

[PATCH v1 RFC 4/7] Rename ksz_spi.c to ksz9477_spi.c

[Tristram.Ha]

From: Tristram Ha 

Rename ksz_spi.c to ksz9477_spi.c and update Kconfig in preparation to add
more KSZ switch drivers.

Signed-off-by: Tristram Ha 
---
 drivers/net/dsa/microchip/Kconfig  | 12 ++--
 drivers/net/dsa/microchip/Makefile |  4 ++--
 drivers/net/dsa/microchip/{ksz_spi.c => ksz9477_spi.c} |  0
 3 files changed, 8 insertions(+), 8 deletions(-)
 rename drivers/net/dsa/microchip/{ksz_spi.c => ksz9477_spi.c} (100%)

diff --git a/drivers/net/dsa/microchip/Kconfig 
b/drivers/net/dsa/microchip/Kconfig
index a8b8f59..5a8660d 100644
--- a/drivers/net/dsa/microchip/Kconfig
+++ b/drivers/net/dsa/microchip/Kconfig
@@ -1,12 +1,12 @@
-menuconfig MICROCHIP_KSZ
-   tristate "Microchip KSZ series switch support"
+menuconfig MICROCHIP_KSZ9477
+   tristate "Microchip KSZ9477 series switch support"
depends on NET_DSA
select NET_DSA_TAG_KSZ
help
- This driver adds support for Microchip KSZ switch chips.
+ This driver adds support for Microchip KSZ9477 switch chips.
 
-config MICROCHIP_KSZ_SPI_DRIVER
-   tristate "KSZ series SPI connected switch driver"
-   depends on MICROCHIP_KSZ && SPI
+config MICROCHIP_KSZ9477_SPI_DRIVER
+   tristate "KSZ9477 series SPI connected switch driver"
+   depends on MICROCHIP_KSZ9477 && SPI
help
  Select to enable support for registering switches configured through 
SPI.
diff --git a/drivers/net/dsa/microchip/Makefile 
b/drivers/net/dsa/microchip/Makefile
index ed335e2..5b6325b 100644
--- a/drivers/net/dsa/microchip/Makefile
+++ b/drivers/net/dsa/microchip/Makefile
@@ -1,2 +1,2 @@
-obj-$(CONFIG_MICROCHIP_KSZ)+= ksz_common.o
-obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
+obj-$(CONFIG_MICROCHIP_KSZ9477)+= ksz_common.o
+obj-$(CONFIG_MICROCHIP_KSZ9477_SPI_DRIVER) += ksz9477_spi.o
diff --git a/drivers/net/dsa/microchip/ksz_spi.c 
b/drivers/net/dsa/microchip/ksz9477_spi.c
similarity index 100%
rename from drivers/net/dsa/microchip/ksz_spi.c
rename to drivers/net/dsa/microchip/ksz9477_spi.c
-- 
1.9.1

[PATCH RFC] Add other KSZ switch support so that patch check does not complain

[Tristram.Ha]

From: Tristram Ha 

Add other KSZ switch support so that patch check does not complain.

Signed-off-by: Tristram Ha 
---
 Documentation/devicetree/bindings/net/dsa/ksz.txt | 189 --
 1 file changed, 136 insertions(+), 53 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt 
b/Documentation/devicetree/bindings/net/dsa/ksz.txt
index fd23904..705f9d9 100644
--- a/Documentation/devicetree/bindings/net/dsa/ksz.txt
+++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt
@@ -3,8 +3,15 @@ Microchip KSZ Series Ethernet switches
 
 Required properties:
 
-- compatible: For external switch chips, compatible string must be exactly one
-  of: "microchip,ksz9477"
+- compatible: "microchip,ksz9477",
+ "microchip,ksz8795",
+ "microchip,ksz8794",
+ "microchip,ksz8765",
+ "microchip,ksz8895",
+ "microchip,ksz8864",
+ "microchip,ksz8873",
+ "microchip,ksz8863",
+ "microchip,ksz8463"
 
 See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional
 required and optional properties.
@@ -13,58 +20,134 @@ Examples:
 
 Ethernet switch connected via SPI to the host, CPU port wired to eth0:
 
- eth0: ethernet@10001000 {
- fixed-link {
- speed = <1000>;
- full-duplex;
- };
- };
+   eth0: ethernet@10001000 {
+   fixed-link {
+   speed = <1000>;
+   full-duplex;
+   };
+   };
 
- spi1: spi@f8008000 {
- pinctrl-0 = <&pinctrl_spi_ksz>;
- cs-gpios = <&pioC 25 0>;
- id = <1>;
+   spi1: spi@f8008000 {
+   cs-gpios = <&pioC 25 0>;
+   id = <1>;
 
- ksz9477: ksz9477@0 {
- compatible = 
"microchip,ksz9477";
- reg = <0>;
+   ksz9477: ksz9477@0 {
+   compatible = "microchip,ksz9477";
+   reg = <0>;
 
- spi-max-frequency 
= <4400>;
- spi-cpha;
- spi-cpol;
+   spi-max-frequency = <4400>;
+   spi-cpha;
+   spi-cpol;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   port@0 {
+   reg = <0>;
+   label = "lan1";
+   };
+   port@1 {
+   reg = <1>;
+   label = "lan2";
+   };
+   port@2 {
+   reg = <2>;
+   label = "lan3";
+   };
+   port@3 {
+   reg = <3>;
+   label = "lan4";
+   };
+   port@4 {
+   reg = <4>;
+   label = "lan5";
+   };
+   port@5 {
+   reg = <5>;
+   label = "cpu";
+   ethernet = <ð0>;
+   fixed-link {
+   speed = <1000>;
+   full-duplex;
+   };
+   };
+   port@6 {
+   reg = <6>;
+   label = "lan6";
+   fixed-link {
+   speed = <1000>;
+   full-duplex;
+   };
+   };
+   };
+   };
+   ksz8794: ksz8794@0 {
+   compatible = "microchip,ksz8794";
+   reg = <0>;
+
+   spi-max-frequency = <3000>;
+  

[PATCH v1 RFC 7/7] Modify tag_ksz.c so that tail tag code can be used by other KSZ switch drivers

[Tristram.Ha]

From: Tristram Ha 

Modify tag_ksz.c so that tail tag code can be used by other KSZ switch
drivers.

Signed-off-by: Tristram Ha 
---
 drivers/net/dsa/microchip/Kconfig   |   2 +-
 drivers/net/dsa/microchip/ksz9477.c |   2 +-
 include/net/dsa.h   |   2 +-
 net/dsa/Kconfig |   4 ++
 net/dsa/dsa.c   |   4 +-
 net/dsa/dsa_priv.h  |   2 +-
 net/dsa/tag_ksz.c   | 107 ++--
 7 files changed, 88 insertions(+), 35 deletions(-)

diff --git a/drivers/net/dsa/microchip/Kconfig 
b/drivers/net/dsa/microchip/Kconfig
index 5a8660d..ab8f9f6 100644
--- a/drivers/net/dsa/microchip/Kconfig
+++ b/drivers/net/dsa/microchip/Kconfig
@@ -1,7 +1,7 @@
 menuconfig MICROCHIP_KSZ9477
tristate "Microchip KSZ9477 series switch support"
depends on NET_DSA
-   select NET_DSA_TAG_KSZ
+   select NET_DSA_TAG_KSZ9477
help
  This driver adds support for Microchip KSZ9477 switch chips.
 
diff --git a/drivers/net/dsa/microchip/ksz9477.c 
b/drivers/net/dsa/microchip/ksz9477.c
index 9579d03..e6d2956 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -342,7 +342,7 @@ static void ksz9477_port_init_cnt(struct ksz_device *dev, 
int port)
 
 static enum dsa_tag_protocol ksz9477_get_tag_protocol(struct dsa_switch *ds)
 {
-   return DSA_TAG_PROTO_KSZ;
+   return DSA_TAG_PROTO_KSZ9477;
 }
 
 static int ksz9477_phy_read16(struct dsa_switch *ds, int addr, int reg)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 10dcecc..28cdc5e 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -31,7 +31,7 @@ enum dsa_tag_protocol {
DSA_TAG_PROTO_BRCM,
DSA_TAG_PROTO_DSA,
DSA_TAG_PROTO_EDSA,
-   DSA_TAG_PROTO_KSZ,
+   DSA_TAG_PROTO_KSZ9477,
DSA_TAG_PROTO_LAN9303,
DSA_TAG_PROTO_MTK,
DSA_TAG_PROTO_QCA,
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index cc5f8f9..d2bbd21 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -28,6 +28,10 @@ config NET_DSA_TAG_EDSA
 config NET_DSA_TAG_KSZ
bool
 
+config NET_DSA_TAG_KSZ9477
+   bool
+   select NET_DSA_TAG_KSZ
+
 config NET_DSA_TAG_LAN9303
bool
 
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 51ca2a5..cc03a09 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -49,8 +49,8 @@ static struct sk_buff *dsa_slave_notag_xmit(struct sk_buff 
*skb,
 #ifdef CONFIG_NET_DSA_TAG_EDSA
[DSA_TAG_PROTO_EDSA] = &edsa_netdev_ops,
 #endif
-#ifdef CONFIG_NET_DSA_TAG_KSZ
-   [DSA_TAG_PROTO_KSZ] = &ksz_netdev_ops,
+#ifdef CONFIG_NET_DSA_TAG_KSZ9477
+   [DSA_TAG_PROTO_KSZ9477] = &ksz9477_netdev_ops,
 #endif
 #ifdef CONFIG_NET_DSA_TAG_LAN9303
[DSA_TAG_PROTO_LAN9303] = &lan9303_netdev_ops,
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 2850077c..e0dc2d4 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -183,7 +183,7 @@ int dsa_port_vlan_del(struct dsa_port *dp,
 extern const struct dsa_device_ops edsa_netdev_ops;
 
 /* tag_ksz.c */
-extern const struct dsa_device_ops ksz_netdev_ops;
+extern const struct dsa_device_ops ksz9477_netdev_ops;
 
 /* tag_lan9303.c */
 extern const struct dsa_device_ops lan9303_netdev_ops;
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index b241c99..1bb5b7d 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -14,34 +14,28 @@
 #include 
 #include "dsa_priv.h"
 
-/* For Ingress (Host -> KSZ), 2 bytes are added before FCS.
- * ---
- * DA(6bytes)|SA(6bytes)||Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes)
- * ---
- * tag0 : Prioritization (not used now)
- * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x10=port5)
- *
- * For Egress (KSZ -> Host), 1 byte is added before FCS.
- * ---
- * DA(6bytes)|SA(6bytes)||Data(nbytes)|tag0(1byte)|FCS(4bytes)
- * ---
- * tag0 : zero-based value represents port
- *   (eg, 0x00=port1, 0x02=port3, 0x06=port7)
- */
+/* Typically only one byte is used for tail tag. */
+#defineKSZ_EGRESS_TAG_LEN  1
+#defineKSZ_INGRESS_TAG_LEN 1
 
-#defineKSZ_INGRESS_TAG_LEN 2
-#defineKSZ_EGRESS_TAG_LEN  1
+/* Frames with following addresse may need to be sent even when the port is
+ * closed.
+ */
+static const u8 special_mult_addr[] = {
+   0x01, 0x80, 0xC2, 0x00, 0x00, 0x00
+};
 
-static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
+static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev,
+   int len,
+   void (*set_tag)(void *ptr, u8 *addr, int p))
 {
struct dsa_slave_priv *p = n

[PATCH v1 RFC 0/1] Add Microchip KSZ8795 DSA driver

[Tristram.Ha]

From: Tristram Ha 

This patch requires the previous patches for Microchip KSZ9477 DSA driver.

v1
- Return error codes instead of numbers
- Add more comments to clarify operation
- Use ksz8795 prefix to indicate KSZ8795 specific code
- Simplify MIB counter reading code
- Switch driver code is not accessed from tag_ksz.c

Tristram Ha (1):
  Add Microchip KSZ8795 DSA driver.

 drivers/net/dsa/microchip/Kconfig   |   17 +
 drivers/net/dsa/microchip/Makefile  |2 +
 drivers/net/dsa/microchip/ksz8795.c | 1372 +++
 drivers/net/dsa/microchip/ksz8795_reg.h | 1016 +++
 drivers/net/dsa/microchip/ksz8795_spi.c |  166 
 drivers/net/dsa/microchip/ksz_priv.h|1 +
 include/net/dsa.h   |1 +
 net/dsa/Kconfig |4 +
 net/dsa/dsa.c   |3 +
 net/dsa/dsa_priv.h  |1 +
 net/dsa/tag_ksz.c   |   37 +
 11 files changed, 2620 insertions(+)
 create mode 100644 drivers/net/dsa/microchip/ksz8795.c
 create mode 100644 drivers/net/dsa/microchip/ksz8795_reg.h
 create mode 100644 drivers/net/dsa/microchip/ksz8795_spi.c

-- 
1.9.1

[PATCH v1 RFC 1/1] Add Microchip KSZ8795 DSA driver

[Tristram.Ha]

From: Tristram Ha 

Add Microchip KSZ8795 DSA driver.

Signed-off-by: Tristram Ha 
---
 drivers/net/dsa/microchip/Kconfig   |   17 +
 drivers/net/dsa/microchip/Makefile  |2 +
 drivers/net/dsa/microchip/ksz8795.c | 1372 +++
 drivers/net/dsa/microchip/ksz8795_reg.h | 1016 +++
 drivers/net/dsa/microchip/ksz8795_spi.c |  166 
 drivers/net/dsa/microchip/ksz_priv.h|1 +
 include/net/dsa.h   |1 +
 net/dsa/Kconfig |4 +
 net/dsa/dsa.c   |3 +
 net/dsa/dsa_priv.h  |1 +
 net/dsa/tag_ksz.c   |   37 +
 11 files changed, 2620 insertions(+)
 create mode 100644 drivers/net/dsa/microchip/ksz8795.c
 create mode 100644 drivers/net/dsa/microchip/ksz8795_reg.h
 create mode 100644 drivers/net/dsa/microchip/ksz8795_spi.c

diff --git a/drivers/net/dsa/microchip/Kconfig 
b/drivers/net/dsa/microchip/Kconfig
index ab8f9f6..cb95d3d 100644
--- a/drivers/net/dsa/microchip/Kconfig
+++ b/drivers/net/dsa/microchip/Kconfig
@@ -10,3 +10,20 @@ config MICROCHIP_KSZ9477_SPI_DRIVER
depends on MICROCHIP_KSZ9477 && SPI
help
  Select to enable support for registering switches configured through 
SPI.
+
+menuconfig MICROCHIP_KSZ8795
+   tristate "Microchip KSZ8795 series switch support"
+   depends on NET_DSA
+   select NET_DSA_TAG_KSZ8795
+   help
+ This driver adds support for Microchip KSZ8795 switch chips.
+
+config MICROCHIP_KSZ8795_SPI_DRIVER
+   tristate "KSZ8795 series SPI connected switch driver"
+   depends on MICROCHIP_KSZ8795 && SPI
+   default y
+   help
+ This driver accesses KSZ8795 chip through SPI.
+
+ It is required to use the KSZ8795 switch driver as the only access
+ is through SPI.
diff --git a/drivers/net/dsa/microchip/Makefile 
b/drivers/net/dsa/microchip/Makefile
index 13dd8f0..99a283e 100644
--- a/drivers/net/dsa/microchip/Makefile
+++ b/drivers/net/dsa/microchip/Makefile
@@ -1,2 +1,4 @@
 obj-$(CONFIG_MICROCHIP_KSZ9477)+= ksz9477.o ksz_common.o
 obj-$(CONFIG_MICROCHIP_KSZ9477_SPI_DRIVER) += ksz9477_spi.o
+obj-$(CONFIG_MICROCHIP_KSZ8795)+= ksz8795.o ksz_common.o
+obj-$(CONFIG_MICROCHIP_KSZ8795_SPI_DRIVER) += ksz8795_spi.o
diff --git a/drivers/net/dsa/microchip/ksz8795.c 
b/drivers/net/dsa/microchip/ksz8795.c
new file mode 100644
index 000..7e727d3
--- /dev/null
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -0,0 +1,1372 @@
+/*
+ * Microchip KSZ8795 switch driver
+ *
+ * Copyright (C) 2017 Microchip Technology Inc.
+ * Tristram Ha 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ksz_priv.h"
+#include "ksz_common.h"
+#include "ksz8795_reg.h"
+
+static const struct {
+   char string[ETH_GSTRING_LEN];
+} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
+   { "rx_hi" },
+   { "rx_undersize" },
+   { "rx_fragments" },
+   { "rx_oversize" },
+   { "rx_jabbers" },
+   { "rx_symbol_err" },
+   { "rx_crc_err" },
+   { "rx_align_err" },
+   { "rx_mac_ctrl" },
+   { "rx_pause" },
+   { "rx_bcast" },
+   { "rx_mcast" },
+   { "rx_ucast" },
+   { "rx_64_or_less" },
+   { "rx_65_127" },
+   { "rx_128_255" },
+   { "rx_256_511" },
+   { "rx_512_1023" },
+   { "rx_1024_1522" },
+   { "rx_1523_2000" },
+   { "rx_2001" },
+   { "tx_hi" },
+   { "tx_late_col" },
+   { "tx_pause" },
+   { "tx_bcast" },
+   { "tx_mcast" },
+   { "tx_ucast" },
+   { "tx_deferred" },
+   { "tx_total_col" },
+   { "tx_exc_col" },
+   { "tx_single_col" },
+   { "tx_mult_col" },
+   { "rx_total" },
+   { "tx_total" },
+   { "rx_discards" },
+   { "tx_discards" },
+};
+
+static int ksz8795_reset_switch(struct ksz_device *dev)
+{
+   /* reset switch */
+   ksz_write8(dev, REG_POWER_MANAGEMENT_1,
+  SW_SOFTWARE_POWER_DOWN << SW_POWER_MANAGEMENT_MODE_S);
+   ksz_write8(dev, REG_POWER_MANAGEMENT_1, 0);
+
+   return 0;
+}
+
+static void ksz8795_set_prio_queue(struct ksz_device *dev, int port, int queue)
+{
+   u8 hi;
+   u8 lo;
+
+  

[PATCH RFC 0/1] Add Microchip KSZ8895 DSA driver

[Tristram.Ha]

From: Tristram Ha 

This patch requires the previous patch for Microchip KSZ8795 DSA driver.

Tristram Ha (1):
  Add Microchip KSZ8895 DSA driver.

 drivers/net/dsa/microchip/Kconfig   |   17 +
 drivers/net/dsa/microchip/Makefile  |2 +
 drivers/net/dsa/microchip/ksz8895.c | 1288 +++
 drivers/net/dsa/microchip/ksz8895_reg.h |  824 
 drivers/net/dsa/microchip/ksz8895_spi.c |  157 
 drivers/net/dsa/microchip/ksz_priv.h|1 +
 6 files changed, 2289 insertions(+)
 create mode 100644 drivers/net/dsa/microchip/ksz8895.c
 create mode 100644 drivers/net/dsa/microchip/ksz8895_reg.h
 create mode 100644 drivers/net/dsa/microchip/ksz8895_spi.c

-- 
1.9.1

[PATCH RFC] Add Microchip KSZ8895 DSA driver

[Tristram.Ha]

From: Tristram Ha 

Add Microchip KSZ8895 DSA driver.

Signed-off-by: Tristram Ha 
---
 drivers/net/dsa/microchip/Kconfig   |   17 +
 drivers/net/dsa/microchip/Makefile  |2 +
 drivers/net/dsa/microchip/ksz8895.c | 1288 +++
 drivers/net/dsa/microchip/ksz8895_reg.h |  824 
 drivers/net/dsa/microchip/ksz8895_spi.c |  157 
 drivers/net/dsa/microchip/ksz_priv.h|1 +
 6 files changed, 2289 insertions(+)
 create mode 100644 drivers/net/dsa/microchip/ksz8895.c
 create mode 100644 drivers/net/dsa/microchip/ksz8895_reg.h
 create mode 100644 drivers/net/dsa/microchip/ksz8895_spi.c

diff --git a/drivers/net/dsa/microchip/Kconfig 
b/drivers/net/dsa/microchip/Kconfig
index cb95d3d..b854c4b 100644
--- a/drivers/net/dsa/microchip/Kconfig
+++ b/drivers/net/dsa/microchip/Kconfig
@@ -27,3 +27,20 @@ config MICROCHIP_KSZ8795_SPI_DRIVER
 
  It is required to use the KSZ8795 switch driver as the only access
  is through SPI.
+
+menuconfig MICROCHIP_KSZ8895
+   tristate "Microchip KSZ8895 series switch support"
+   depends on NET_DSA
+   select NET_DSA_TAG_KSZ8795
+   help
+ This driver adds support for Microchip KSZ8895 switch chips.
+
+config MICROCHIP_KSZ8895_SPI_DRIVER
+   tristate "KSZ8895 series SPI connected switch driver"
+   depends on MICROCHIP_KSZ8895 && SPI
+   default y
+   help
+ This driver accesses KSZ8895 chip through SPI.
+
+ It is required to use the KSZ8895 switch driver as the only access
+ is through SPI.
diff --git a/drivers/net/dsa/microchip/Makefile 
b/drivers/net/dsa/microchip/Makefile
index 99a283e..8dd6312 100644
--- a/drivers/net/dsa/microchip/Makefile
+++ b/drivers/net/dsa/microchip/Makefile
@@ -2,3 +2,5 @@ obj-$(CONFIG_MICROCHIP_KSZ9477) += ksz9477.o 
ksz_common.o
 obj-$(CONFIG_MICROCHIP_KSZ9477_SPI_DRIVER) += ksz9477_spi.o
 obj-$(CONFIG_MICROCHIP_KSZ8795)+= ksz8795.o ksz_common.o
 obj-$(CONFIG_MICROCHIP_KSZ8795_SPI_DRIVER) += ksz8795_spi.o
+obj-$(CONFIG_MICROCHIP_KSZ8895)+= ksz8895.o ksz_common.o
+obj-$(CONFIG_MICROCHIP_KSZ8895_SPI_DRIVER) += ksz8895_spi.o
diff --git a/drivers/net/dsa/microchip/ksz8895.c 
b/drivers/net/dsa/microchip/ksz8895.c
new file mode 100644
index 000..947638b
--- /dev/null
+++ b/drivers/net/dsa/microchip/ksz8895.c
@@ -0,0 +1,1288 @@
+/*
+ * Microchip KSZ8895 switch driver
+ *
+ * Copyright (C) 2017 Microchip Technology Inc.
+ * Tristram Ha 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ksz_priv.h"
+#include "ksz_common.h"
+#include "ksz8895_reg.h"
+
+static const struct {
+   char string[ETH_GSTRING_LEN];
+} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
+   { "rx" },
+   { "rx_hi" },
+   { "rx_undersize" },
+   { "rx_fragments" },
+   { "rx_oversize" },
+   { "rx_jabbers" },
+   { "rx_symbol_err" },
+   { "rx_crc_err" },
+   { "rx_align_err" },
+   { "rx_mac_ctrl" },
+   { "rx_pause" },
+   { "rx_bcast" },
+   { "rx_mcast" },
+   { "rx_ucast" },
+   { "rx_64_or_less" },
+   { "rx_65_127" },
+   { "rx_128_255" },
+   { "rx_256_511" },
+   { "rx_512_1023" },
+   { "rx_1024_1522" },
+   { "tx" },
+   { "tx_hi" },
+   { "tx_late_col" },
+   { "tx_pause" },
+   { "tx_bcast" },
+   { "tx_mcast" },
+   { "tx_ucast" },
+   { "tx_deferred" },
+   { "tx_total_col" },
+   { "tx_exc_col" },
+   { "tx_single_col" },
+   { "tx_mult_col" },
+   { "rx_discards" },
+   { "tx_discards" },
+};
+
+static int ksz8895_reset_switch(struct ksz_device *dev)
+{
+   /* reset switch */
+   ksz_write8(dev, REG_POWER_MANAGEMENT_1,
+  SW_SOFTWARE_POWER_DOWN << SW_POWER_MANAGEMENT_MODE_S);
+   ksz_write8(dev, REG_POWER_MANAGEMENT_1, 0);
+
+   return 0;
+}
+
+static void ksz8895_set_prio_queue(struct ksz_device *dev, int port, int queue)
+{
+   u8 hi;
+   u8 lo;
+
+   /* Number of queues can only be 1, 2, or 4. */
+   switch (queue) {
+   case 4:
+   case 3:
+   queue = PORT_QUEUE_SPLIT_4;
+   break;
+   case 2:
+   queue = PORT_Q

[PATCH v1 RFC 6/7] Add MIB counter reading support

[Tristram.Ha]

From: Tristram Ha 

Add MIB counter reading support.
Rename ksz_9477_reg.h to ksz9477_reg.h for consistency as the product
name is always KSZ.
Header file ksz_priv.h no longer contains any chip specific data.

Signed-off-by: Tristram Ha 
---
 drivers/net/dsa/microchip/ksz9477.c| 130 ++---
 .../microchip/{ksz_9477_reg.h => ksz9477_reg.h}|   0
 drivers/net/dsa/microchip/ksz_common.c | 122 +++
 drivers/net/dsa/microchip/ksz_common.h |   4 +
 drivers/net/dsa/microchip/ksz_priv.h   |   8 +-
 5 files changed, 219 insertions(+), 45 deletions(-)
 rename drivers/net/dsa/microchip/{ksz_9477_reg.h => ksz9477_reg.h} (100%)

diff --git a/drivers/net/dsa/microchip/ksz9477.c 
b/drivers/net/dsa/microchip/ksz9477.c
index 214d380..9579d03 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -31,7 +31,7 @@
 
 #include "ksz_priv.h"
 #include "ksz_common.h"
-#include "ksz_9477_reg.h"
+#include "ksz9477_reg.h"
 
 static const struct {
int index;
@@ -272,6 +272,74 @@ static int ksz9477_reset_switch(struct ksz_device *dev)
return 0;
 }
 
+static void ksz9477_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
+ u64 *cnt)
+{
+   u32 data;
+   int timeout;
+
+   /* retain the flush/freeze bit */
+   ksz_pread32(dev, port, REG_PORT_MIB_CTRL_STAT__4, &data);
+   data &= MIB_COUNTER_FLUSH_FREEZE;
+
+   data |= MIB_COUNTER_READ;
+   data |= (addr << MIB_COUNTER_INDEX_S);
+   ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, data);
+
+   timeout = 1000;
+   do {
+   ksz_pread32(dev, port, REG_PORT_MIB_CTRL_STAT__4,
+   &data);
+   usleep_range(1, 10);
+   if (!(data & MIB_COUNTER_READ))
+   break;
+   } while (timeout-- > 0);
+
+   /* failed to read MIB. get out of loop */
+   if (!timeout) {
+   dev_dbg(dev->dev, "Failed to get MIB\n");
+   return;
+   }
+
+   /* count resets upon read */
+   ksz_pread32(dev, port, REG_PORT_MIB_DATA, &data);
+   *cnt += data;
+}
+
+static void ksz9477_r_mib_pkt(struct ksz_device *dev, int port, u16 addr,
+ u64 *dropped, u64 *cnt)
+{
+   addr = mib_names[addr].index;
+   ksz9477_r_mib_cnt(dev, port, addr, cnt);
+}
+
+static void ksz9477_freeze_mib(struct ksz_device *dev, int port, bool freeze)
+{
+   /* enable the port for flush/freeze function */
+   if (freeze)
+   ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4,
+MIB_COUNTER_FLUSH_FREEZE);
+   ksz_cfg(dev, REG_SW_MAC_CTRL_6, SW_MIB_COUNTER_FREEZE, freeze);
+
+   /* disable the port after freeze is done */
+   if (!freeze)
+   ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, 0);
+}
+
+static void ksz9477_port_init_cnt(struct ksz_device *dev, int port)
+{
+   struct ksz_port_mib *mib = &dev->ports[port].mib;
+
+   /* flush all enabled port MIB counters */
+   ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4,
+MIB_COUNTER_FLUSH_FREEZE);
+   ksz_write8(dev, REG_SW_MAC_CTRL_6, SW_MIB_COUNTER_FLUSH);
+   ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, 0);
+
+   mib->cnt_ptr = 0;
+   memset(mib->counters, 0, dev->mib_cnt * sizeof(u64));
+}
+
 static enum dsa_tag_protocol ksz9477_get_tag_protocol(struct dsa_switch *ds)
 {
return DSA_TAG_PROTO_KSZ;
@@ -350,46 +418,6 @@ static void ksz9477_get_strings(struct dsa_switch *ds, int 
port, uint8_t *buf)
}
 }
 
-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;
-   int timeout;
-
-   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);
-
-   timeout = 1000;
-   do {
-   ksz_pread32(dev, port, REG_PORT_MIB_CTRL_STAT__4,
-   &data);
-   usleep_range(1, 10);
-   if (!(data & MIB_COUNTER_READ))
-   break;
-   } while (timeout-- > 0);
-
-   /* failed to read MIB. get out of loop */
-   if (!timeout) {
-   dev_dbg(dev->dev, "Failed to get MIB\n");
-   break;
-   }
-
-   /* count resets upon read */
-   ksz_pread32(dev, port, REG_PORT_MIB_DATA, &data);
-
-   dev->mib_value[i] += (uint64_t)data;
-   buf[i] = dev->mib_value[i];
-   }
-
-   mutex

[PATCH v1 RFC 5/7] Break KSZ9477 DSA driver into two files

[Tristram.Ha]

From: Tristram Ha 

Break KSZ9477 DSA driver into two files in preparation to add more KSZ
switch drivers.
Add common functions in ksz_common.h so that other KSZ switch drivers
can access code in ksz_common.c.
Add ksz_spi.h for common functions used by KSZ switch SPI drivers.

Signed-off-by: Tristram Ha 
---
 drivers/net/dsa/microchip/Makefile  |2 +-
 drivers/net/dsa/microchip/ksz9477.c | 1328 +++
 drivers/net/dsa/microchip/ksz9477_spi.c |  143 ++--
 drivers/net/dsa/microchip/ksz_common.c  | 1133 +++---
 drivers/net/dsa/microchip/ksz_common.h  |  230 ++
 drivers/net/dsa/microchip/ksz_priv.h|  229 +++---
 drivers/net/dsa/microchip/ksz_spi.h |   82 ++
 7 files changed, 1926 insertions(+), 1221 deletions(-)
 create mode 100644 drivers/net/dsa/microchip/ksz9477.c
 create mode 100644 drivers/net/dsa/microchip/ksz_common.h
 create mode 100644 drivers/net/dsa/microchip/ksz_spi.h

diff --git a/drivers/net/dsa/microchip/Makefile 
b/drivers/net/dsa/microchip/Makefile
index 5b6325b..13dd8f0 100644
--- a/drivers/net/dsa/microchip/Makefile
+++ b/drivers/net/dsa/microchip/Makefile
@@ -1,2 +1,2 @@
-obj-$(CONFIG_MICROCHIP_KSZ9477)+= ksz_common.o
+obj-$(CONFIG_MICROCHIP_KSZ9477)+= ksz9477.o ksz_common.o
 obj-$(CONFIG_MICROCHIP_KSZ9477_SPI_DRIVER) += ksz9477_spi.o
diff --git a/drivers/net/dsa/microchip/ksz9477.c 
b/drivers/net/dsa/microchip/ksz9477.c
new file mode 100644
index 000..214d380
--- /dev/null
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -0,0 +1,1328 @@
+/*
+ * Microchip KSZ9477 switch driver main logic
+ *
+ * Copyright (C) 2017 Microchip Technology Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ksz_priv.h"
+#include "ksz_common.h"
+#include "ksz_9477_reg.h"
+
+static const struct {
+   int index;
+   char string[ETH_GSTRING_LEN];
+} mib_names[TOTAL_SWITCH_COUNTER_NUM] = {
+   { 0x00, "rx_hi" },
+   { 0x01, "rx_undersize" },
+   { 0x02, "rx_fragments" },
+   { 0x03, "rx_oversize" },
+   { 0x04, "rx_jabbers" },
+   { 0x05, "rx_symbol_err" },
+   { 0x06, "rx_crc_err" },
+   { 0x07, "rx_align_err" },
+   { 0x08, "rx_mac_ctrl" },
+   { 0x09, "rx_pause" },
+   { 0x0A, "rx_bcast" },
+   { 0x0B, "rx_mcast" },
+   { 0x0C, "rx_ucast" },
+   { 0x0D, "rx_64_or_less" },
+   { 0x0E, "rx_65_127" },
+   { 0x0F, "rx_128_255" },
+   { 0x10, "rx_256_511" },
+   { 0x11, "rx_512_1023" },
+   { 0x12, "rx_1024_1522" },
+   { 0x13, "rx_1523_2000" },
+   { 0x14, "rx_2001" },
+   { 0x15, "tx_hi" },
+   { 0x16, "tx_late_col" },
+   { 0x17, "tx_pause" },
+   { 0x18, "tx_bcast" },
+   { 0x19, "tx_mcast" },
+   { 0x1A, "tx_ucast" },
+   { 0x1B, "tx_deferred" },
+   { 0x1C, "tx_total_col" },
+   { 0x1D, "tx_exc_col" },
+   { 0x1E, "tx_single_col" },
+   { 0x1F, "tx_mult_col" },
+   { 0x80, "rx_total" },
+   { 0x81, "tx_total" },
+   { 0x82, "rx_discards" },
+   { 0x83, "tx_discards" },
+};
+
+static void ksz_cfg32(struct ksz_device *dev, u32 addr, u32 bits, bool set)
+{
+   u32 data;
+
+   ksz_read32(dev, addr, &data);
+   if (set)
+   data |= bits;
+   else
+   data &= ~bits;
+   ksz_write32(dev, addr, data);
+}
+
+static void ksz_port_cfg32(struct ksz_device *dev, int port, int offset,
+  u32 bits, bool 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 int wait_vlan_ctrl_ready(struct ksz_device *dev, u32 waiton, int 
timeout)
+{
+   u8 data;
+
+   do {
+   ksz_read8(dev, REG_SW_VLAN_CTRL, &data);
+   if (!(data & waiton))
+   break;
+   usleep_range(1, 10);
+   } while (timeout-- > 0);
+
+   if (timeout <= 0)
+   return -ETIMEDOUT;
+
+   return 0;
+}
+
+static int get_vlan_table(struct dsa_switch *ds, u16 vid, u32 *vlan_table)
+{
+   struct ksz_device *dev = 

[PATCH v1 RFC 3/7] Rename some functions with ksz9477 prefix

[Tristram.Ha]

From: Tristram Ha 

Rename some functions with ksz9477 prefix to separate chip specific code
from common code.

Signed-off-by: Tristram Ha 
---
 drivers/net/dsa/microchip/ksz_common.c | 116 +
 1 file changed, 59 insertions(+), 57 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c 
b/drivers/net/dsa/microchip/ksz_common.c
index d72aad7..7a0d10f 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -265,9 +265,8 @@ static int wait_alu_sta_ready(struct ksz_device *dev, u32 
waiton, int timeout)
return 0;
 }
 
-static int ksz_reset_switch(struct dsa_switch *ds)
+static int ksz9477_reset_switch(struct ksz_device *dev)
 {
-   struct ksz_device *dev = ds->priv;
u8 data8;
u16 data16;
u32 data32;
@@ -300,7 +299,7 @@ static int ksz_reset_switch(struct dsa_switch *ds)
return 0;
 }
 
-static void port_setup(struct ksz_device *dev, int port, bool cpu_port)
+static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port)
 {
u8 data8;
u16 data16;
@@ -346,7 +345,7 @@ static void port_setup(struct ksz_device *dev, int port, 
bool cpu_port)
ksz_pread16(dev, port, REG_PORT_PHY_INT_ENABLE, &data16);
 }
 
-static void ksz_config_cpu_port(struct dsa_switch *ds)
+static void ksz9477_config_cpu_port(struct dsa_switch *ds)
 {
struct ksz_device *dev = ds->priv;
int i;
@@ -358,12 +357,12 @@ static void ksz_config_cpu_port(struct dsa_switch *ds)
dev->cpu_port = i;
 
/* enable cpu port */
-   port_setup(dev, i, true);
+   ksz9477_port_setup(dev, i, true);
}
}
 }
 
-static int ksz_setup(struct dsa_switch *ds)
+static int ksz9477_setup(struct dsa_switch *ds)
 {
struct ksz_device *dev = ds->priv;
int ret = 0;
@@ -373,7 +372,7 @@ static int ksz_setup(struct dsa_switch *ds)
if (!dev->vlan_cache)
return -ENOMEM;
 
-   ret = ksz_reset_switch(ds);
+   ret = ksz9477_reset_switch(dev);
if (ret) {
dev_err(ds->dev, "failed to reset switch\n");
return ret;
@@ -382,7 +381,7 @@ static int ksz_setup(struct dsa_switch *ds)
/* accept packet up to 2000bytes */
ksz_cfg(dev, REG_SW_MAC_CTRL_1, SW_LEGAL_PACKET_DISABLE, true);
 
-   ksz_config_cpu_port(ds);
+   ksz9477_config_cpu_port(ds);
 
ksz_cfg(dev, REG_SW_MAC_CTRL_1, MULTICAST_STORM_DISABLE, true);
 
@@ -395,12 +394,12 @@ static int ksz_setup(struct dsa_switch *ds)
return 0;
 }
 
-static enum dsa_tag_protocol ksz_get_tag_protocol(struct dsa_switch *ds)
+static enum dsa_tag_protocol ksz9477_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)
+static int ksz9477_phy_read16(struct dsa_switch *ds, int addr, int reg)
 {
struct ksz_device *dev = ds->priv;
u16 val = 0;
@@ -410,7 +409,8 @@ static int ksz_phy_read16(struct dsa_switch *ds, int addr, 
int reg)
return val;
 }
 
-static int ksz_phy_write16(struct dsa_switch *ds, int addr, int reg, u16 val)
+static int ksz9477_phy_write16(struct dsa_switch *ds, int addr, int reg,
+  u16 val)
 {
struct ksz_device *dev = ds->priv;
 
@@ -425,7 +425,7 @@ static int ksz_enable_port(struct dsa_switch *ds, int port,
struct ksz_device *dev = ds->priv;
 
/* setup slave port */
-   port_setup(dev, port, false);
+   ksz9477_port_setup(dev, port, false);
 
return 0;
 }
@@ -444,7 +444,7 @@ 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)
+static void ksz9477_get_strings(struct dsa_switch *ds, int port, uint8_t *buf)
 {
int i;
 
@@ -494,7 +494,8 @@ static void ksz_get_ethtool_stats(struct dsa_switch *ds, 
int port,
mutex_unlock(&dev->stats_mutex);
 }
 
-static void ksz_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
+static void ksz9477_port_stp_state_set(struct dsa_switch *ds, int port,
+  u8 state)
 {
struct ksz_device *dev = ds->priv;
u8 data;
@@ -539,7 +540,8 @@ static void ksz_port_fast_age(struct dsa_switch *ds, int 
port)
ksz_write8(dev, REG_SW_LUE_CTRL_1, data8);
 }
 
-static int ksz_port_vlan_filtering(struct dsa_switch *ds, int port, bool flag)
+static int ksz9477_port_vlan_filtering(struct dsa_switch *ds, int port,
+  bool flag)
 {
struct ksz_device *dev = ds->priv;
 
@@ -567,9 +569,9 @@ static int ksz_port_vlan_prepare(struct dsa_switch *ds, int 
port,
return 0;
 }
 
-static void ksz_port_vlan_add(struct dsa_switch *ds, int port,
- const struct switchdev_obj_port_vlan 

[PATCH v1 RFC 2/7] Clean up code according to patch check suggestions

[Tristram.Ha]

From: Tristram Ha 

Clean up code according to patch check suggestions.

Signed-off-by: Tristram Ha 
---
 drivers/net/dsa/microchip/ksz_common.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c 
b/drivers/net/dsa/microchip/ksz_common.c
index 685dafd..d72aad7 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -898,9 +898,9 @@ static void ksz_port_mdb_add(struct dsa_switch *ds, int 
port,
 
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)) &&
+   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)) {
+   static_table[3] == mac_lo) {
/* found matching one */
break;
}
@@ -971,9 +971,9 @@ static int ksz_port_mdb_del(struct dsa_switch *ds, int port,
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)) &&
+   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)) {
+   static_table[3] == mac_lo) {
/* found matching one */
break;
}
-- 
1.9.1

[PATCH v1 RFC 0/7] Modify KSZ9477 DSA driver in preparation to add other KSZ switch drivers

[Tristram.Ha]

From: Tristram Ha 

This series of patches is to modify the original KSZ9477 DSA driver so
that other KSZ switch drivers can be added and use the common code.

There are several steps to accomplish this achievement.  First is to
rename some function names with a prefix to indicate chip specific
function.  Second is to move common code into header that can be shared.
Last is to modify tag_ksz.c so that it can handle many tail tag formats
used by different KSZ switch drivers.

ksz_common.c will contain the common code used by all KSZ switch drivers.
ksz9477.c will contain KSZ9477 code from the original ksz_common.c.
ksz9477_spi.c is renamed from ksz_spi.c.
ksz9477_reg.h is renamed from ksz_9477_reg.h.
ksz_common.h is added to provide common code access to KSZ switch
drivers.
ksz_spi.h is added to provide common SPI access functions to KSZ SPI
drivers.

v1
- Each patch in the set is self-contained
- Use ksz9477 prefix to indicate KSZ9477 specific code
- Simplify MIB counter reading code
- Switch driver code is not accessed from tag_ksz.c

Tristram Ha (7):
  Replace license with GPL.
  Clean up code according to patch check suggestions.
  Rename some functions with ksz9477 prefix to separate chip specific
code from common code.
  Rename ksz_spi.c to ksz9477_spi.c and update Kconfig in preparation to
add more KSZ switch drivers.
  Break KSZ9477 DSA driver into two files in preparation to add more KSZ
switch drivers.  Add common functions in ksz_common.h so that other
KSZ switch drivers can access code in ksz_common.c.  Add ksz_spi.h
for common functions used by KSZ switch SPI drivers.
  Add MIB counter reading support.  Rename ksz_9477_reg.h to
ksz9477_reg.h for consistency as the product name is always KSZ.
Header file ksz_priv.h no longer contains any chip specific data.
  Modify tag_ksz.c so that tail tag code can be used by other KSZ switch
drivers.

 drivers/net/dsa/microchip/Kconfig  |   14 +-
 drivers/net/dsa/microchip/Makefile |4 +-
 drivers/net/dsa/microchip/ksz9477.c| 1376 
 .../microchip/{ksz_9477_reg.h => ksz9477_reg.h}|   23 +-
 drivers/net/dsa/microchip/ksz9477_spi.c|  188 +++
 drivers/net/dsa/microchip/ksz_common.c | 1210 -
 drivers/net/dsa/microchip/ksz_common.h |  234 
 drivers/net/dsa/microchip/ksz_priv.h   |  258 ++--
 drivers/net/dsa/microchip/ksz_spi.c|  216 ---
 drivers/net/dsa/microchip/ksz_spi.h|   82 ++
 include/net/dsa.h  |2 +-
 net/dsa/Kconfig|4 +
 net/dsa/dsa.c  |4 +-
 net/dsa/dsa_priv.h |2 +-
 net/dsa/tag_ksz.c  |  107 +-
 15 files changed, 2331 insertions(+), 1393 deletions(-)
 create mode 100644 drivers/net/dsa/microchip/ksz9477.c
 rename drivers/net/dsa/microchip/{ksz_9477_reg.h => ksz9477_reg.h} (98%)
 create mode 100644 drivers/net/dsa/microchip/ksz9477_spi.c
 create mode 100644 drivers/net/dsa/microchip/ksz_common.h
 delete mode 100644 drivers/net/dsa/microchip/ksz_spi.c
 create mode 100644 drivers/net/dsa/microchip/ksz_spi.h

-- 
1.9.1

RE: [PATCH v1 RFC 1/1] Add Microchip KSZ8795 DSA driver

[Tristram.Ha]

> in previous version I see that transit traffic (ping) goes to cpu,
> then from cpu back to destination port. I.e. it works but with cpu
> involving. Is this version supposed to work like that?

Yes, it works in the old DSA way such that a software bridge is
responsible to forward every packet.

Now if the ksz_update_port_member function is called inside
the ksz8795_port_stp_state_set function the switch will forward
packets itself.  Because of that the offload_fwd_mark bit should be
set in the socket buffer so that the software bridge does not
forward the packet (mostly multicast) again.  However, that
indication cannot be set in the switch driver but in the tail tag code
in tag_ksz.c.  Right now there is no easy way for that code to know
the bit should be set because the switch is in forwarding mode.

RE: [PATCH v1 RFC 1/7] Replace license with GPL

[Tristram.Ha]

> From: tristram...@microchip.com
> > Sent: 06 October 2017 21:33
> > Replace license with GPL.
> 
> Don't you need permission from all the people who have updated
> the files in order to make this change?
> 
>   David

I am a little confused by your comment.  The 4 original KSZ9477 DSA
driver files were written by Woojung at Microchip Technology Inc.
There was a complaint the "AS IS" license is not exactly GPL.

It should be submitted formally to net-next instead of a RFC, but it
is probably pointless to do that when there is no code change.

I am hoping these drastic changes of KSZ9477 driver can be accepted
so that the patches can be submitted formally to net-next.