| Message ID | 20190822211514.19288-6-olteanv@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Poll mode for NXP DSPI driver | expand |
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
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.
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?
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.
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.
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.
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
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
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";
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(+)