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

Groups > linux.kernel > #1741792 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH RFC 3/5] Add KSZ8795 switch driver Andrew Lunn <andrew@lunn.ch> - 2017-09-28 21:40 +0200
    RE: [PATCH RFC 3/5] Add KSZ8795 switch driver David Laight <David.Laight@ACULAB.COM> - 2017-09-29 11:20 +0200
      Re: [PATCH RFC 3/5] Add KSZ8795 switch driver Andrew Lunn <andrew@lunn.ch> - 2017-09-29 14:20 +0200
        RE: [PATCH RFC 3/5] Add KSZ8795 switch driver <Tristram.Ha@microchip.com> - 2017-09-29 20:30 +0200
          Re: [PATCH RFC 3/5] Add KSZ8795 switch driver Andrew Lunn <andrew@lunn.ch> - 2017-09-29 21:00 +0200
            RE: [PATCH RFC 3/5] Add KSZ8795 switch driver <Tristram.Ha@microchip.com> - 2017-09-29 21:20 +0200
              Re: [PATCH RFC 3/5] Add KSZ8795 switch driver Andrew Lunn <andrew@lunn.ch> - 2017-09-29 22:50 +0200

#1741792 — Re: [PATCH RFC 3/5] Add KSZ8795 switch driver

On Mon, Sep 18, 2017 at 08:27:13PM +0000, Tristram.Ha@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.

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.

No other driver does caching. So i'm hesitant to add one which does.

>  These accesses can be getting 1588 PTP timestamps and opening/closing ports.

You could drop the mutex between each statistic read, so allowing
something else access to the switch. That should reduce the jitter PTP
experiences.

	Andrew

[toc] | [next] | [standalone]

#1742004

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?
Doing that with software delays isn't friendly to the rest of the system.
(Hardware guys please note...)

One possibility is to rate-limit the stats reading.
Then an application cannot completely 'hog' the SPI bandwidth.

	David

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

#1742073

On Fri, Sep 29, 2017 at 09:14:26AM +0000, 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

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

#1742268

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:andrew@lunn.ch]
> Sent: Friday, September 29, 2017 5:12 AM
> To: David Laight
> Cc: Tristram Ha - C24268; muvarov@gmail.com; pavel@ucw.cz;
> nathan.leigh.conrad@gmail.com; vivien.didelot@savoirfairelinux.com;
> f.fainelli@gmail.com; netdev@vger.kernel.org; linux-
> kernel@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 +0000, 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

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

#1742287

> 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.

       Andrew

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

#1742295

> > 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.

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

#1742339

On Fri, Sep 29, 2017 at 07:19:17PM +0000, Tristram.Ha@microchip.com wrote:
> > > 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.

The SPI subsystem has a mutex per controller. When starting a
transfer, it takes the mutex and release it once the transfer has
completed. There is also a reschedule point at the end of a
transfer. So even on your single core CPU, there can be multi tasking
going on.

      Andrew

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web