linux-kernel - Re: [PATCH v3 2/3] spi: spacemit: introduce SpacemiT K1 SPI controller driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250928123622-GYA1347020@gentoo.org>
Date: Sun, 28 Sep 2025 20:36:22 +0800
From: Yixun Lan <dlan@...too.org>
To: Alex Elder <elder@...cstar.com>
Cc: broonie@...nel.org, robh@...nel.org, krzk+dt@...nel.org,
	conor+dt@...nel.org, ziyao@...root.org, linux-spi@...r.kernel.org,
	devicetree@...r.kernel.org, paul.walmsley@...ive.com,
	palmer@...belt.com, aou@...s.berkeley.edu, alex@...ti.fr,
	p.zabel@...gutronix.de, spacemit@...ts.linux.dev,
	linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/3] spi: spacemit: introduce SpacemiT K1 SPI
 controller driver

Hi Alex,

On 07:49 Tue 23 Sep     , Alex Elder wrote:
> On 9/22/25 6:06 PM, Yixun Lan wrote:
> > Hi Alex,
> > 
> > On 11:17 Mon 22 Sep     , Alex Elder wrote:
> >> This patch introduces the driver for the SPI controller found in the
> >> SpacemiT K1 SoC.  Currently the driver supports master mode only.
> >> The SPI hardware implements RX and TX FIFOs, 32 entries each, and
> >> supports both PIO and DMA mode transfers.
> >>
> >> Signed-off-by: Alex Elder <elder@...cstar.com>
> >> ---
..
> 
> >> diff --git a/drivers/spi/spi-spacemit-k1.c b/drivers/spi/spi-spacemit-k1.c
> >> new file mode 100644
> >> index 0000000000000..2b932d80cc510
> >> --- /dev/null
> >> +++ b/drivers/spi/spi-spacemit-k1.c
> >> @@ -0,0 +1,965 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Support for SpacemiT K1 SPI controller
> >> + *
> >> + * Copyright (C) 2025 by RISCstar Solutions Corporation.  All rights reserved.
> >> + * Copyright (c) 2023, spacemit Corporation.
> >> + */
> 
> . . .
> 
> >> +static irqreturn_t k1_spi_ssp_isr(int irq, void *dev_id)
> >> +{
> >> +	struct k1_spi_driver_data *drv_data = dev_id;
> >> +	bool rx_done;
> >> +	bool tx_done;
> >> +	u32 val;
> >> +
> >> +	/* Get status and clear pending interrupts */
> >> +	val = readl(drv_data->base + SSP_STATUS);
> >> +	writel(val, drv_data->base + SSP_STATUS);
> >> +
> >> +	if (!drv_data->message)
> >> +		return IRQ_NONE;
> >> +
> >> +	/* Check for a TX underrun or RX underrun first */
> > s/RX underrun/RX overrun/
> 
> OK.
> 
> >> +	if (val & (SSP_STATUS_TUR | SSP_STATUS_ROR)) {
> >> +		/* Disable all interrupts on error */
> >> +		writel(0, drv_data->base + SSP_INT_EN);
> > should clear status of SSP_STATUS instead of disabling ISR, see commet below
> 
> The status is cleared immediately after reading, above.  We hold
> the status value so we can act on the current state of the FIFOs.
> 
I'm surprised by this, do you mean the status will be cleared by reading?
can you double checck and prove it? by reading twice and compare?

while according to the docs - 18.2.4.6 SSSR REGISTERS, the status bits has
two types: 
  R - Read only
  R/W1C - Read only, write 1 to clear

if you're right, then the docs should be fixed.

-- 
Yixun Lan (dlan)