[v2,5/5] ARM: dts: ls1021a-tsn: Use the DSPI controller in poll mode
diff mbox series

Message ID 20190822211514.19288-6-olteanv@gmail.com
State New, archived
Headers show
Series
  • Poll mode for NXP DSPI driver
Related show

Commit Message

Vladimir Oltean Aug. 22, 2019, 9:15 p.m. UTC
Connected to the LS1021A DSPI is the SJA1105 DSA switch. This
constitutes 4 of the 6 Ethernet ports on this board.

As the SJA1105 is a PTP switch, constant disciplining of its PTP clock
is necessary, and that translates into a lot of SPI I/O even when
otherwise idle.

Switching to using the DSPI in poll mode has several distinct
benefits:

- With interrupts, the DSPI driver in TCFQ mode raises an IRQ after each
  transmitted byte. There is more time wasted for the "waitq" event than
  for actual I/O. And the DSPI IRQ count is by far the largest in
  /proc/interrupts on this board (larger than Ethernet). I should
  mention that due to various LS1021A errata, other operating modes than
  TCFQ are not available.

- The SPI I/O time is both lower, and more consistently so. For a TSN
  switch it is important that all SPI transfers take a deterministic
  time to complete.
  Reading the PTP clock is an important example.
  Egressing through the switch requires some setup in advance (an SPI
  write command). Without this patch, that operation required a
  --tx_timestamp_timeout 50 (ms), now it can be done with
  --tx_timestamp_timeout 10.
  Yet another example is reconstructing timestamps, which has a hard
  deadline because the PTP timestamping counter wraps around in 0.135
  seconds. Combined with other I/O needed for that to happen, there is
  a real risk that the deadline is not always met.

See drivers/net/dsa/sja1105/ for more info about the above.

Cc: Rob Herring <robh@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 arch/arm/boot/dts/ls1021a-tsn.dts | 1 +
 1 file changed, 1 insertion(+)

Comments

Vladimir Oltean Aug. 26, 2019, 1:10 p.m. UTC | #1
Hi Mark,

On Fri, 23 Aug 2019 at 00:15, Vladimir Oltean <olteanv@gmail.com> wrote:
>
> Connected to the LS1021A DSPI is the SJA1105 DSA switch. This
> constitutes 4 of the 6 Ethernet ports on this board.
>
> As the SJA1105 is a PTP switch, constant disciplining of its PTP clock
> is necessary, and that translates into a lot of SPI I/O even when
> otherwise idle.
>
> Switching to using the DSPI in poll mode has several distinct
> benefits:
>
> - With interrupts, the DSPI driver in TCFQ mode raises an IRQ after each
>   transmitted byte. There is more time wasted for the "waitq" event than
>   for actual I/O. And the DSPI IRQ count is by far the largest in
>   /proc/interrupts on this board (larger than Ethernet). I should
>   mention that due to various LS1021A errata, other operating modes than
>   TCFQ are not available.
>
> - The SPI I/O time is both lower, and more consistently so. For a TSN
>   switch it is important that all SPI transfers take a deterministic
>   time to complete.
>   Reading the PTP clock is an important example.
>   Egressing through the switch requires some setup in advance (an SPI
>   write command). Without this patch, that operation required a
>   --tx_timestamp_timeout 50 (ms), now it can be done with
>   --tx_timestamp_timeout 10.
>   Yet another example is reconstructing timestamps, which has a hard
>   deadline because the PTP timestamping counter wraps around in 0.135
>   seconds. Combined with other I/O needed for that to happen, there is
>   a real risk that the deadline is not always met.
>
> See drivers/net/dsa/sja1105/ for more info about the above.
>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
> ---
>  arch/arm/boot/dts/ls1021a-tsn.dts | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/ls1021a-tsn.dts b/arch/arm/boot/dts/ls1021a-tsn.dts
> index 5b7689094b70..1c09cfc766af 100644
> --- a/arch/arm/boot/dts/ls1021a-tsn.dts
> +++ b/arch/arm/boot/dts/ls1021a-tsn.dts
> @@ -33,6 +33,7 @@
>  };
>
>  &dspi0 {
> +       /delete-property/ interrupts;
>         bus-num = <0>;
>         status = "okay";
>
> --
> 2.17.1
>

I noticed you skipped applying this patch, and I'm not sure that Shawn
will review it/take it.
Do you have a better suggestion how I can achieve putting the DSPI
driver in poll mode for this board? A Kconfig option maybe?

Regards,
-Vladimir
Mark Brown Aug. 27, 2019, 6:05 p.m. UTC | #2
On Mon, Aug 26, 2019 at 04:10:51PM +0300, Vladimir Oltean wrote:

> I noticed you skipped applying this patch, and I'm not sure that Shawn
> will review it/take it.
> Do you have a better suggestion how I can achieve putting the DSPI
> driver in poll mode for this board? A Kconfig option maybe?

DT changes go through the relevant platform trees, not the
subsystem trees, so it's not something I'd expect to apply.
Vladimir Oltean Aug. 27, 2019, 6:06 p.m. UTC | #3
On Tue, 27 Aug 2019 at 21:05, Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, Aug 26, 2019 at 04:10:51PM +0300, Vladimir Oltean wrote:
>
> > I noticed you skipped applying this patch, and I'm not sure that Shawn
> > will review it/take it.
> > Do you have a better suggestion how I can achieve putting the DSPI
> > driver in poll mode for this board? A Kconfig option maybe?
>
> DT changes go through the relevant platform trees, not the
> subsystem trees, so it's not something I'd expect to apply.

But at least is it something that you expect to see done through a
device tree change?
Mark Brown Aug. 27, 2019, 6:13 p.m. UTC | #4
On Tue, Aug 27, 2019 at 09:06:14PM +0300, Vladimir Oltean wrote:
> On Tue, 27 Aug 2019 at 21:05, Mark Brown <broonie@kernel.org> wrote:
> > On Mon, Aug 26, 2019 at 04:10:51PM +0300, Vladimir Oltean wrote:

> > > I noticed you skipped applying this patch, and I'm not sure that Shawn
> > > will review it/take it.
> > > Do you have a better suggestion how I can achieve putting the DSPI
> > > driver in poll mode for this board? A Kconfig option maybe?

> > DT changes go through the relevant platform trees, not the
> > subsystem trees, so it's not something I'd expect to apply.

> But at least is it something that you expect to see done through a
> device tree change?

Well, it's not ideal - if it performs better all the time the
driver should probably just do it unconditionally.  If there's
some threashold where it tends to perform better then the driver
should check for that but IIRC it sounds like the interrupt just
isn't at all helpful here.
Vladimir Oltean Aug. 27, 2019, 6:16 p.m. UTC | #5
On Tue, 27 Aug 2019 at 21:13, Mark Brown <broonie@kernel.org> wrote:
>
> On Tue, Aug 27, 2019 at 09:06:14PM +0300, Vladimir Oltean wrote:
> > On Tue, 27 Aug 2019 at 21:05, Mark Brown <broonie@kernel.org> wrote:
> > > On Mon, Aug 26, 2019 at 04:10:51PM +0300, Vladimir Oltean wrote:
>
> > > > I noticed you skipped applying this patch, and I'm not sure that Shawn
> > > > will review it/take it.
> > > > Do you have a better suggestion how I can achieve putting the DSPI
> > > > driver in poll mode for this board? A Kconfig option maybe?
>
> > > DT changes go through the relevant platform trees, not the
> > > subsystem trees, so it's not something I'd expect to apply.
>
> > But at least is it something that you expect to see done through a
> > device tree change?
>
> Well, it's not ideal - if it performs better all the time the
> driver should probably just do it unconditionally.  If there's
> some threashold where it tends to perform better then the driver
> should check for that but IIRC it sounds like the interrupt just
> isn't at all helpful here.

I can't seem to find any situation where it performs worse. Hence my
question on whether it's a better idea to condition this behavior on a
Kconfig option rather than a DT blob which may or may not be in sync.
Mark Brown Aug. 27, 2019, 6:31 p.m. UTC | #6
On Tue, Aug 27, 2019 at 09:16:39PM +0300, Vladimir Oltean wrote:

> I can't seem to find any situation where it performs worse. Hence my
> question on whether it's a better idea to condition this behavior on a
> Kconfig option rather than a DT blob which may or may not be in sync.

If it's unconditionally worse then it shouldn't even be a Kconfig
option, just make the driver just never use the interrupt.
Shawn Guo Sept. 11, 2019, 6:33 a.m. UTC | #7
On Tue, Aug 27, 2019 at 09:16:39PM +0300, Vladimir Oltean wrote:
> On Tue, 27 Aug 2019 at 21:13, Mark Brown <broonie@kernel.org> wrote:
> >
> > On Tue, Aug 27, 2019 at 09:06:14PM +0300, Vladimir Oltean wrote:
> > > On Tue, 27 Aug 2019 at 21:05, Mark Brown <broonie@kernel.org> wrote:
> > > > On Mon, Aug 26, 2019 at 04:10:51PM +0300, Vladimir Oltean wrote:
> >
> > > > > I noticed you skipped applying this patch, and I'm not sure that Shawn
> > > > > will review it/take it.
> > > > > Do you have a better suggestion how I can achieve putting the DSPI
> > > > > driver in poll mode for this board? A Kconfig option maybe?
> >
> > > > DT changes go through the relevant platform trees, not the
> > > > subsystem trees, so it's not something I'd expect to apply.
> >
> > > But at least is it something that you expect to see done through a
> > > device tree change?
> >
> > Well, it's not ideal - if it performs better all the time the
> > driver should probably just do it unconditionally.  If there's
> > some threashold where it tends to perform better then the driver
> > should check for that but IIRC it sounds like the interrupt just
> > isn't at all helpful here.
> 
> I can't seem to find any situation where it performs worse. Hence my
> question on whether it's a better idea to condition this behavior on a
> Kconfig option rather than a DT blob which may or may not be in sync.

DT is a description of hardware not condition for software behavior,
where module parameter is usually used for.

Shawn
Geert Uytterhoeven Sept. 11, 2019, 7 a.m. UTC | #8
On Wed, Sep 11, 2019 at 8:34 AM Shawn Guo <shawnguo@kernel.org> wrote:
> On Tue, Aug 27, 2019 at 09:16:39PM +0300, Vladimir Oltean wrote:
> > On Tue, 27 Aug 2019 at 21:13, Mark Brown <broonie@kernel.org> wrote:
> > > On Tue, Aug 27, 2019 at 09:06:14PM +0300, Vladimir Oltean wrote:
> > > > On Tue, 27 Aug 2019 at 21:05, Mark Brown <broonie@kernel.org> wrote:
> > > > > On Mon, Aug 26, 2019 at 04:10:51PM +0300, Vladimir Oltean wrote:
> > >
> > > > > > I noticed you skipped applying this patch, and I'm not sure that Shawn
> > > > > > will review it/take it.
> > > > > > Do you have a better suggestion how I can achieve putting the DSPI
> > > > > > driver in poll mode for this board? A Kconfig option maybe?
> > >
> > > > > DT changes go through the relevant platform trees, not the
> > > > > subsystem trees, so it's not something I'd expect to apply.
> > >
> > > > But at least is it something that you expect to see done through a
> > > > device tree change?
> > >
> > > Well, it's not ideal - if it performs better all the time the
> > > driver should probably just do it unconditionally.  If there's
> > > some threashold where it tends to perform better then the driver
> > > should check for that but IIRC it sounds like the interrupt just
> > > isn't at all helpful here.
> >
> > I can't seem to find any situation where it performs worse. Hence my
> > question on whether it's a better idea to condition this behavior on a
> > Kconfig option rather than a DT blob which may or may not be in sync.
>
> DT is a description of hardware not condition for software behavior,
> where module parameter is usually used for.

+1

DT says the interrupt line is wired.
The driver should know if it should make use of the interrupt, or not.

Gr{oetje,eeting}s,

                        Geert

Patch
diff mbox series

diff --git a/arch/arm/boot/dts/ls1021a-tsn.dts b/arch/arm/boot/dts/ls1021a-tsn.dts
index 5b7689094b70..1c09cfc766af 100644
--- a/arch/arm/boot/dts/ls1021a-tsn.dts
+++ b/arch/arm/boot/dts/ls1021a-tsn.dts
@@ -33,6 +33,7 @@ 
 };
 
 &dspi0 {
+	/delete-property/ interrupts;
 	bus-num = <0>;
 	status = "okay";