[PATCH AUTOSEL 4.9 01/13] spi: spi-cadence: Fix SPI CS gets toggling sporadically

[PATCH AUTOSEL 4.9 01/13] spi: spi-cadence: Fix SPI CS gets toggling sporadically

[Sasha Levin]

From: Sai Krishna Potthuri <lakshmi.sai.krishna.potthuri@xilinx.com>

[ Upstream commit 21b511ddee09a78909035ec47a6a594349fe3296 ]

As part of unprepare_transfer_hardware, SPI controller will be disabled
which will indirectly deassert the CS line. This will create a problem
in some of the devices where message will be transferred with
cs_change flag set(CS should not be deasserted).
As per SPI controller implementation, if SPI controller is disabled then
all output enables are inactive and all pins are set to input mode which
means CS will go to default state high(deassert). This leads to an issue
when core explicitly ask not to deassert the CS (cs_change = 1). This
patch fix the above issue by checking the Slave select status bits from
configuration register before disabling the SPI.

Signed-off-by: Sai Krishna Potthuri <lakshmi.sai.krishna.potthuri@xilinx.com>
Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@xilinx.com>
Link: https://lore.kernel.org/r/20220606062525.18447-1-amit.kumar-mahapatra@xilinx.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/spi/spi-cadence.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-cadence.c b/drivers/spi/spi-cadence.c
index e383c63689157..6d294a1fa5e58 100644
--- a/drivers/spi/spi-cadence.c
+++ b/drivers/spi/spi-cadence.c
@@ -72,6 +72,7 @@
 #define CDNS_SPI_BAUD_DIV_SHIFT		3 /* Baud rate divisor shift in CR */
 #define CDNS_SPI_SS_SHIFT		10 /* Slave Select field shift in CR */
 #define CDNS_SPI_SS0			0x1 /* Slave Select zero */
+#define CDNS_SPI_NOSS			0x3C /* No Slave select */
 
 /*
  * SPI Interrupt Registers bit Masks
@@ -444,15 +445,20 @@ static int cdns_prepare_transfer_hardware(struct spi_master *master)
  * @master:	Pointer to the spi_master structure which provides
  *		information about the controller.
  *
- * This function disables the SPI master controller.
+ * This function disables the SPI master controller when no slave selected.
  *
  * Return:	0 always
  */
 static int cdns_unprepare_transfer_hardware(struct spi_master *master)
 {
 	struct cdns_spi *xspi = spi_master_get_devdata(master);
+	u32 ctrl_reg;
 
-	cdns_spi_write(xspi, CDNS_SPI_ER, CDNS_SPI_ER_DISABLE);
+	/* Disable the SPI if slave is deselected */
+	ctrl_reg = cdns_spi_read(xspi, CDNS_SPI_CR);
+	ctrl_reg = (ctrl_reg & CDNS_SPI_CR_SSCTRL) >>  CDNS_SPI_SS_SHIFT;
+	if (ctrl_reg == CDNS_SPI_NOSS)
+		cdns_spi_write(xspi, CDNS_SPI_ER, CDNS_SPI_ER_DISABLE);
 
 	return 0;
 }
-- 
2.35.1

[PATCH AUTOSEL 4.9 02/13] spi: cadence: Detect transmit FIFO depth

[Sasha Levin]

From: Lars-Peter Clausen <lars@metafoo.de>

[ Upstream commit 7b40322f7183a92c4303457528ae7cda571c60b9 ]

The depth of the transmit FIFO for the Cadence SPI controller is currently
hardcoded to 128. But the depth is a synthesis configuration parameter of
the core and can vary between different SoCs.

If the configured FIFO size is less than 128 the driver will busy loop in
the cdns_spi_fill_tx_fifo() function waiting for FIFO space to become
available.

Depending on the length and speed of the transfer it can spin for a
significant amount of time. The cdns_spi_fill_tx_fifo() function is called
from the drivers interrupt handler, so it can leave interrupts disabled for
a prolonged amount of time.

In addition the read FIFO will also overflow and data will be discarded.

To avoid this detect the actual size of the FIFO and use that rather than
the hardcoded value.

To detect the FIFO size the FIFO threshold register is used. The register
is sized so that it can hold FIFO size - 1 as its maximum value. Bits that
are not needed to hold the threshold value will always read 0. By writing
0xffff to the register and then reading back the value in the register we
get the FIFO size.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Link: https://lore.kernel.org/r/20220527091143.3780378-1-lars@metafoo.de
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/spi/spi-cadence.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-cadence.c b/drivers/spi/spi-cadence.c
index 6d294a1fa5e58..733724e71a30c 100644
--- a/drivers/spi/spi-cadence.c
+++ b/drivers/spi/spi-cadence.c
@@ -96,9 +96,6 @@
 #define CDNS_SPI_ER_ENABLE	0x00000001 /* SPI Enable Bit Mask */
 #define CDNS_SPI_ER_DISABLE	0x0 /* SPI Disable Bit Mask */
 
-/* SPI FIFO depth in bytes */
-#define CDNS_SPI_FIFO_DEPTH	128
-
 /* Default number of chip select lines */
 #define CDNS_SPI_DEFAULT_NUM_CS		4
 
@@ -114,6 +111,7 @@
  * @rx_bytes:		Number of bytes requested
  * @dev_busy:		Device busy flag
  * @is_decoded_cs:	Flag for decoder property set or not
+ * @tx_fifo_depth:	Depth of the TX FIFO
  */
 struct cdns_spi {
 	void __iomem *regs;
@@ -127,6 +125,7 @@ struct cdns_spi {
 	int rx_bytes;
 	u8 dev_busy;
 	u32 is_decoded_cs;
+	unsigned int tx_fifo_depth;
 };
 
 /* Macros for the SPI controller read/write */
@@ -308,7 +307,7 @@ static void cdns_spi_fill_tx_fifo(struct cdns_spi *xspi)
 {
 	unsigned long trans_cnt = 0;
 
-	while ((trans_cnt < CDNS_SPI_FIFO_DEPTH) &&
+	while ((trans_cnt < xspi->tx_fifo_depth) &&
 	       (xspi->tx_bytes > 0)) {
 		if (xspi->txbuf)
 			cdns_spi_write(xspi, CDNS_SPI_TXD, *xspi->txbuf++);
@@ -463,6 +462,24 @@ static int cdns_unprepare_transfer_hardware(struct spi_master *master)
 	return 0;
 }
 
+/**
+ * cdns_spi_detect_fifo_depth - Detect the FIFO depth of the hardware
+ * @xspi:	Pointer to the cdns_spi structure
+ *
+ * The depth of the TX FIFO is a synthesis configuration parameter of the SPI
+ * IP. The FIFO threshold register is sized so that its maximum value can be the
+ * FIFO size - 1. This is used to detect the size of the FIFO.
+ */
+static void cdns_spi_detect_fifo_depth(struct cdns_spi *xspi)
+{
+	/* The MSBs will get truncated giving us the size of the FIFO */
+	cdns_spi_write(xspi, CDNS_SPI_THLD, 0xffff);
+	xspi->tx_fifo_depth = cdns_spi_read(xspi, CDNS_SPI_THLD) + 1;
+
+	/* Reset to default */
+	cdns_spi_write(xspi, CDNS_SPI_THLD, 0x1);
+}
+
 /**
  * cdns_spi_probe - Probe method for the SPI driver
  * @pdev:	Pointer to the platform_device structure
@@ -536,6 +553,8 @@ static int cdns_spi_probe(struct platform_device *pdev)
 	if (ret < 0)
 		xspi->is_decoded_cs = 0;
 
+	cdns_spi_detect_fifo_depth(xspi);
+
 	/* SPI controller initializations */
 	cdns_spi_init_hw(xspi);
 
-- 
2.35.1

Re: [PATCH AUTOSEL 4.9 01/13] spi: spi-cadence: Fix SPI CS gets toggling sporadically

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2628 bytes --]

Hi!

> From: Sai Krishna Potthuri <lakshmi.sai.krishna.potthuri@xilinx.com>
> 
> [ Upstream commit 21b511ddee09a78909035ec47a6a594349fe3296 ]
> 
> As part of unprepare_transfer_hardware, SPI controller will be disabled
> which will indirectly deassert the CS line. This will create a problem
> in some of the devices where message will be transferred with
> cs_change flag set(CS should not be deasserted).
> As per SPI controller implementation, if SPI controller is disabled then
> all output enables are inactive and all pins are set to input mode which
> means CS will go to default state high(deassert). This leads to an issue
> when core explicitly ask not to deassert the CS (cs_change = 1). This
> patch fix the above issue by checking the Slave select status bits from
> configuration register before disabling the SPI.

My records say this was already submitted to AUTOSEL at "Jun
27". There are more patches from that era that were reviewed in
AUTOSEL but not merged anywhere. Can you investigate?

Best regards,
								Pavel

a 4.9 01/13] spi: spi-cadence: Fix SPI CS gets toggling sporadic
a 4.9 02/13] spi: cadence: Detect transmit FIFO depth
n unused preparation 4.9 03/13] drm/vc4: crtc: Use an union to store the page f\
l
a 4.9 04/13] drivers/net/ethernet/neterion/vxge: Fix a use-af
n just a comment fix 4.9 05/13] video: fbdev: skeletonfb: Fix syntax errors in \
c
a just a printk tweak 4.9 06/13] video: fbdev: intelfb: Use aperture size from \
pc
a 4.9 07/13] video: fbdev: pxa3xx-gcu: Fix integer overflow i
n just a cleanup 4.9 08/13] video: fbdev: simplefb: Check before clk_put() n
a 4.9 09/13] mips: lantiq: falcon: Fix refcount leak bug in s
a 4.9 10/13] mips: lantiq: xway: Fix refcount leak bug in sys
n we still have reference to the name, it is not safe to put it 4.9 11/13] mips\
/pic32/pic32mzda: Fix refcount leak bugs
a 4.9 12/13] mips: lantiq: Add missing of_node_put() in irq.c

a 4.19 03/22] ALSA: usb-audio: US16x08: Move overflow check b
n unused preparation 4.19 05/22] drm/vc4: crtc: Move the BO handling out of com\
m
! do we have everything? 4.19 06/22] ALSA: x86: intel_hdmi_audio: enable pm_run\
time
! 4.19 07/22] hamradio: 6pack: fix array-index-out-of-bounds
a 4.19 13/22] arch: mips: generic: Add missing of_node_put()
a 4.19 14/22] mips: mti-malta: Fix refcount leak in malta-tim
a 4.19 15/22] mips: ralink: Fix refcount leak in of.c
a 4.19 20/22] drm/sun4i: Add DMA mask and segment size
! 4.19 21/22] drm/amdgpu: Adjust logic around GTT size (v3)

-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH AUTOSEL 4.9 01/13] spi: spi-cadence: Fix SPI CS gets toggling sporadically

[Sasha Levin]

On Tue, Sep 20, 2022 at 03:48:32PM +0200, Pavel Machek wrote:
>Hi!
>
>> From: Sai Krishna Potthuri <lakshmi.sai.krishna.potthuri@xilinx.com>
>>
>> [ Upstream commit 21b511ddee09a78909035ec47a6a594349fe3296 ]
>>
>> As part of unprepare_transfer_hardware, SPI controller will be disabled
>> which will indirectly deassert the CS line. This will create a problem
>> in some of the devices where message will be transferred with
>> cs_change flag set(CS should not be deasserted).
>> As per SPI controller implementation, if SPI controller is disabled then
>> all output enables are inactive and all pins are set to input mode which
>> means CS will go to default state high(deassert). This leads to an issue
>> when core explicitly ask not to deassert the CS (cs_change = 1). This
>> patch fix the above issue by checking the Slave select status bits from
>> configuration register before disabling the SPI.
>
>My records say this was already submitted to AUTOSEL at "Jun
>27". There are more patches from that era that were reviewed in
>AUTOSEL but not merged anywhere. Can you investigate?

Yup, there was a batch that never went in, going to queue it up for the
next round of releases.

-- 
Thanks,
Sasha

AUTOSEL: batches that did not make it in was Re: [PATCH AUTOSEL 4.9 01/13] spi: spi-cadence: Fix SPI CS gets toggling sporadically

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 5811 bytes --]

Hi!

> > > From: Sai Krishna Potthuri <lakshmi.sai.krishna.potthuri@xilinx.com>
> > > 
> > > [ Upstream commit 21b511ddee09a78909035ec47a6a594349fe3296 ]
> > > 
> > > As part of unprepare_transfer_hardware, SPI controller will be disabled
> > > which will indirectly deassert the CS line. This will create a problem
> > > in some of the devices where message will be transferred with
> > > cs_change flag set(CS should not be deasserted).
> > > As per SPI controller implementation, if SPI controller is disabled then
> > > all output enables are inactive and all pins are set to input mode which
> > > means CS will go to default state high(deassert). This leads to an issue
> > > when core explicitly ask not to deassert the CS (cs_change = 1). This
> > > patch fix the above issue by checking the Slave select status bits from
> > > configuration register before disabling the SPI.
> > 
> > My records say this was already submitted to AUTOSEL at "Jun
> > 27". There are more patches from that era that were reviewed in
> > AUTOSEL but not merged anywhere. Can you investigate?
> 
> Yup, there was a batch that never went in, going to queue it up for the
> next round of releases.

For the record, these are batches that did not seem to make it in
(along with my notes, ignore those).

Best regards,
								Pavel

Jun 21

a 4.9 1/3] mei: me: set internal pg flag to off on hardware reset
a relatively important performance tweak 4.9 2/3] ext4: improve write performance with disabled delal
a 4.9 3/3] ext4: correct the judgment of BUG in ext4_mb_normal

a 4.19 1/7] genirq: PM: Use runtime PM for chained interrupts
a 4.19 2/7] irqchip/uniphier-aidet: Add compatible string for
a just a printk tweak 4.19 4/7] nvme-pci: add trouble shooting steps for timeouts
a 4.19 6/7] blk-mq: protect q->elevator by ->sysfs_lock in blk

a 5.10 05/11] nvme-pci: phison e12 has bogus namespace ids
a 5.10 06/11] nvme-pci: smi has bogus namespace ids
a 5.10 07/11] nvme-pci: sk hynix p31 has bogus namespace ids
a 5.10 10/11] blk-mq: don't clear flush_rq from tags->rqs[]

Jun 27

a 4.19 03/22] ALSA: usb-audio: US16x08: Move overflow check b
n unused preparation 4.19 05/22] drm/vc4: crtc: Move the BO handling out of comm
! do we have everything? 4.19 06/22] ALSA: x86: intel_hdmi_audio: enable pm_runtime
! 4.19 07/22] hamradio: 6pack: fix array-index-out-of-bounds
a 4.19 13/22] arch: mips: generic: Add missing of_node_put()
a 4.19 14/22] mips: mti-malta: Fix refcount leak in malta-tim
a 4.19 15/22] mips: ralink: Fix refcount leak in of.c
a 4.19 20/22] drm/sun4i: Add DMA mask and segment size
! 4.19 21/22] drm/amdgpu: Adjust logic around GTT size (v3)

a 5.10 03/34] regulator: qcom_smd: correct MP5496 ranges
a just a printk tweak 5.10 05/34] bus: bt1-apb: Don't print error on -EPROBE_DEFE
a just a printk tweak 5.10 06/34] bus: bt1-axi: Don't print error on -EPROBE_DEFE
n unused preparation 5.10 09/34] scsi: ufs: Simplify ufshcd_clear_cmd()
n unused preparation 5.10 10/34] scsi: ufs: Support clearing multiple commands a
! c/e 5.10 12/34] ALSA: x86: intel_hdmi_audio: use pm_runtime_res
! needed? 5.10 15/34] powerpc/prom_init: Fix build failure with GCC_P
! c/e, just a robustness 5.10 20/34] btrfs: do not BUG_ON() on failure to migrate sp
a 5.10 29/34] drm/sun4i: Return if frontend is not present
n just a workaround for bug 5.10 is not triggering -- we don't have 281d0c962752 ("fortify: Add Clang support") 5.10 30/34] hin\
ic: Replace memcpy() with direct assignment
a 5.10 32/34] nvme: add a bogus subsystem NQN quirk for Micro
a 5.10 33/34] gpio: grgpio: Fix device removing


Aug 11

a just a warning fix 4.9 01/12] drm/r128: Fix undefined behavior due to shift overf
a 4.9 02/12] drm/radeon: Initialize fences array entries in r
a could be cleaned up 4.9 03/12] media: airspy: respect the DMA coherency rules
a 4.9 04/12] media: pvrusb2: fix memory leak in pvr_probe
n preparation for patches not queued here 4.9 05/12] mlxsw: cmd: Increase 'config_profile.flood_mode'
! do they in 4.9? 4.9 06/12] iov_iter_get_pages{,_alloc}(): cap the maxsize w
a just a warning fix 4.9 07/12] crypto: vmx - Fix warning on p8_ghash_alg
n preparation for patches not queued, not needed in 4.9/5.10 4.9 08/12] can: sja1000: Add Quirk for RZ/N1 SJA1000 CAN co
! performance tweaks, known to cause regressions 4.9 09/12] net/cdc_ncm: Increase NTB max RX/TX values to 64
a 4.9 10/12] wifi: rtl8xxxu: Fix the error handling of the pr
a 4.9 11/12] d_add_ci(): make sure we don't miss d_lookup_don
n PREEMPT_RT does not exist on 4.9 and 4.19 4.9 12/12] fs/dcache: Disable preemption on i_dir_seq write

a 4.19 02/14] drm/radeon: integer overflow in radeon_mode_dum
! check, is it using endpoing after that? 4.19 04/14] media: davinci: vpif: add missing of_node_put()
a 4.19 09/14] drm/nouveau/nvkm: use list_add_tail() when buil

! check requisites 5.10 02/46] ath10k: htt_tx: do not interpret Eth frames as
a 5.10 03/46] ath10k: fix misreported tx bandwidth for 160Mhz
a 5.10 04/46] drm/nouveau: clear output poll workers before n
n preparation for patches not queued here 5.10 05/46] drm/panfrost: Handle HW_ISSUE_TTRX_2968_TTRX_31
! just a cleanup 5.10 06/46] drm/panfrost: Don't set L2_MMU_CONFIG quirks
a 5.10 07/46] ath10k: fix regdomain info of iw reg set/get
a 5.10 08/46] drm/amd/display: Detect dpcd_rev when hotplug m
 5.10 09/46] drm/amd/display: Fix dpp dto for disabled pipes
 5.10 12/46] udmabuf: Set the DMA mask for the udmabuf devic
 5.10 13/46] net/mlx5: Add HW definitions of vport debug cou
 5.10 14/46] drm/amd/display: Fix monitor flash issue


-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[PATCH AUTOSEL 4.9 01/13] spi: spi-cadence: Fix SPI CS gets toggling sporadically

[Sasha Levin]

From: Sai Krishna Potthuri <lakshmi.sai.krishna.potthuri@xilinx.com>

[ Upstream commit 21b511ddee09a78909035ec47a6a594349fe3296 ]

As part of unprepare_transfer_hardware, SPI controller will be disabled
which will indirectly deassert the CS line. This will create a problem
in some of the devices where message will be transferred with
cs_change flag set(CS should not be deasserted).
As per SPI controller implementation, if SPI controller is disabled then
all output enables are inactive and all pins are set to input mode which
means CS will go to default state high(deassert). This leads to an issue
when core explicitly ask not to deassert the CS (cs_change = 1). This
patch fix the above issue by checking the Slave select status bits from
configuration register before disabling the SPI.

Signed-off-by: Sai Krishna Potthuri <lakshmi.sai.krishna.potthuri@xilinx.com>
Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@xilinx.com>
Link: https://lore.kernel.org/r/20220606062525.18447-1-amit.kumar-mahapatra@xilinx.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/spi/spi-cadence.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-cadence.c b/drivers/spi/spi-cadence.c
index e383c6368915..6d294a1fa5e5 100644
--- a/drivers/spi/spi-cadence.c
+++ b/drivers/spi/spi-cadence.c
@@ -72,6 +72,7 @@
 #define CDNS_SPI_BAUD_DIV_SHIFT		3 /* Baud rate divisor shift in CR */
 #define CDNS_SPI_SS_SHIFT		10 /* Slave Select field shift in CR */
 #define CDNS_SPI_SS0			0x1 /* Slave Select zero */
+#define CDNS_SPI_NOSS			0x3C /* No Slave select */
 
 /*
  * SPI Interrupt Registers bit Masks
@@ -444,15 +445,20 @@ static int cdns_prepare_transfer_hardware(struct spi_master *master)
  * @master:	Pointer to the spi_master structure which provides
  *		information about the controller.
  *
- * This function disables the SPI master controller.
+ * This function disables the SPI master controller when no slave selected.
  *
  * Return:	0 always
  */
 static int cdns_unprepare_transfer_hardware(struct spi_master *master)
 {
 	struct cdns_spi *xspi = spi_master_get_devdata(master);
+	u32 ctrl_reg;
 
-	cdns_spi_write(xspi, CDNS_SPI_ER, CDNS_SPI_ER_DISABLE);
+	/* Disable the SPI if slave is deselected */
+	ctrl_reg = cdns_spi_read(xspi, CDNS_SPI_CR);
+	ctrl_reg = (ctrl_reg & CDNS_SPI_CR_SSCTRL) >>  CDNS_SPI_SS_SHIFT;
+	if (ctrl_reg == CDNS_SPI_NOSS)
+		cdns_spi_write(xspi, CDNS_SPI_ER, CDNS_SPI_ER_DISABLE);
 
 	return 0;
 }
-- 
2.35.1