| 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
| ||
|
Message-ID:
<TYZPR06MB5203E7912C75298F2149A022B28BA@TYZPR06MB5203.apcprd06.prod.outlook.com>
Date: Sun, 18 Jan 2026 12:39:59 +0000
From: Chin-Ting Kuo <chin-ting_kuo@...eedtech.com>
To: Paul Menzel <pmenzel@...gen.mpg.de>
CC: "clg@...d.org" <clg@...d.org>, "broonie@...nel.org" <broonie@...nel.org>,
"boris.brezillon@...tlin.com" <boris.brezillon@...tlin.com>, "joel@....id.au"
<joel@....id.au>, "andrew@...econstruct.com.au"
<andrew@...econstruct.com.au>, "linux-aspeed@...ts.ozlabs.org"
<linux-aspeed@...ts.ozlabs.org>, "openbmc@...ts.ozlabs.org"
<openbmc@...ts.ozlabs.org>, "linux-spi@...r.kernel.org"
<linux-spi@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, BMC-SW <BMC-SW@...eedtech.com>
Subject: RE: [PATCH v2 2/2] spi: aspeed: Add support for non-spi-mem devices
Hi Paul,
Thanks for the review.
> -----Original Message-----
> From: Paul Menzel <pmenzel@...gen.mpg.de>
> Sent: Sunday, January 18, 2026 7:49 AM
> Subject: Re: [PATCH v2 2/2] spi: aspeed: Add support for non-spi-mem
> devices
>
> Dear Chin-Ting,
>
>
> Thank you for your patch.
>
> Am 17.01.26 um 14:42 schrieb Chin-Ting Kuo:
> > The ASPEED FMC/SPI controller may be shared by spi-mem devices and
> > other SPI peripherals that do not use the spi-mem framework.
> >
> > The driver currently assumes spi-mem semantics for all devices, while
> > the controller also supports direct user mode access commonly used by
> > non-spi-mem devices. This mismatch can result in incorrect behavior
> > when different types of devices share the same controller.
> >
> > Update the driver to properly handle non-spi-mem devices, allowing
> > them to operate correctly in pure user mode alongside spi-mem devices
> > on a shared SPI controller.
>
> It’d be great if you added a paragraph on how the driver ist updated.
>
Okay, it will be added in the next patch version.
> Also, documented on how to test your change, would be great.
>
Okay.
> > Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@...eedtech.com>
> > ---
> > drivers/spi/spi-aspeed-smc.c | 134
> +++++++++++++++++++++++++++++++++--
> > 1 file changed, 128 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/spi/spi-aspeed-smc.c
> > b/drivers/spi/spi-aspeed-smc.c index db3e096f2eb0..3949f94b6667 100644
> > --- a/drivers/spi/spi-aspeed-smc.c
> > +++ b/drivers/spi/spi-aspeed-smc.c
> > @@ -48,6 +48,8 @@
> > /* CEx Address Decoding Range Register */
> > #define CE0_SEGMENT_ADDR_REG 0x30
> >
> > +#define FULL_DUPLEX_RX_DATA 0x1e4
> > +
> > /* CEx Read timing compensation register */
> > #define CE0_TIMING_COMPENSATION_REG 0x94
> >
> > @@ -81,6 +83,7 @@ struct aspeed_spi_data {
> > u32 hclk_mask;
> > u32 hdiv_max;
> > u32 min_window_size;
> > + bool full_duplex;
> >
> > phys_addr_t (*segment_start)(struct aspeed_spi *aspi, u32 reg);
> > phys_addr_t (*segment_end)(struct aspeed_spi *aspi, u32 reg);
> @@
> > -105,6 +108,7 @@ struct aspeed_spi {
> >
> > struct clk *clk;
> > u32 clk_freq;
> > + u8 cs_change;
> >
> > struct aspeed_spi_chip chips[ASPEED_SPI_MAX_NUM_CS];
> > };
> > @@ -280,7 +284,8 @@ static ssize_t aspeed_spi_write_user(struct
> aspeed_spi_chip *chip,
> > }
> >
> > /* support for 1-1-1, 1-1-2 or 1-1-4 */ -static bool
> > aspeed_spi_supports_op(struct spi_mem *mem, const struct spi_mem_op
> > *op)
> > +static bool aspeed_spi_supports_mem_op(struct spi_mem *mem,
> > + const struct spi_mem_op *op)
> > {
> > if (op->cmd.buswidth > 1)
> > return false;
> > @@ -305,7 +310,8 @@ static bool aspeed_spi_supports_op(struct
> spi_mem
> > *mem, const struct spi_mem_op
> >
> > static const struct aspeed_spi_data ast2400_spi_data;
> >
> > -static int do_aspeed_spi_exec_op(struct spi_mem *mem, const struct
> > spi_mem_op *op)
> > +static int do_aspeed_spi_exec_mem_op(struct spi_mem *mem,
> > + const struct spi_mem_op *op)
> > {
> > struct aspeed_spi *aspi =
> spi_controller_get_devdata(mem->spi->controller);
> > struct aspeed_spi_chip *chip =
> > &aspi->chips[spi_get_chipselect(mem->spi, 0)]; @@ -367,11 +373,12 @@
> static int do_aspeed_spi_exec_op(struct spi_mem *mem, const struct
> spi_mem_op *o
> > return ret;
> > }
> >
> > -static int aspeed_spi_exec_op(struct spi_mem *mem, const struct
> > spi_mem_op *op)
> > +static int aspeed_spi_exec_mem_op(struct spi_mem *mem,
> > + const struct spi_mem_op *op)
> > {
> > int ret;
> >
> > - ret = do_aspeed_spi_exec_op(mem, op);
> > + ret = do_aspeed_spi_exec_mem_op(mem, op);
> > if (ret)
> > dev_err(&mem->spi->dev, "operation failed: %d\n", ret);
> > return ret;
> > @@ -773,8 +780,8 @@ static ssize_t aspeed_spi_dirmap_read(struct
> spi_mem_dirmap_desc *desc,
> > }
> >
> > static const struct spi_controller_mem_ops aspeed_spi_mem_ops = {
> > - .supports_op = aspeed_spi_supports_op,
> > - .exec_op = aspeed_spi_exec_op,
> > + .supports_op = aspeed_spi_supports_mem_op,
> > + .exec_op = aspeed_spi_exec_mem_op,
> > .get_name = aspeed_spi_get_name,
> > .dirmap_create = aspeed_spi_dirmap_create,
> > .dirmap_read = aspeed_spi_dirmap_read, @@ -843,6 +850,110
> @@ static
> > void aspeed_spi_enable(struct aspeed_spi *aspi, bool enable)
> > aspeed_spi_chip_enable(aspi, cs, enable);
> > }
> >
> > +static int aspeed_spi_user_prepare_msg(struct spi_controller *ctlr,
> > + struct spi_message *msg)
> > +{
> > + struct aspeed_spi *aspi =
> > + (struct aspeed_spi *)spi_controller_get_devdata(ctlr);
> > + const struct aspeed_spi_data *data = aspi->data;
> > + struct spi_device *spi = msg->spi;
> > + u32 cs = spi_get_chipselect(spi, 0);
> > + struct aspeed_spi_chip *chip = &aspi->chips[cs];
> > + u32 ctrl_val;
> > + u32 clk_div = data->get_clk_div(chip, spi->max_speed_hz);
> > +
> > + ctrl_val = chip->ctl_val[ASPEED_SPI_BASE];
> > + ctrl_val &= ~CTRL_IO_MODE_MASK & data->hclk_mask;
> > + ctrl_val |= clk_div;
> > + chip->ctl_val[ASPEED_SPI_BASE] = ctrl_val;
> > +
> > + if (aspi->cs_change == 0)
> > + aspeed_spi_start_user(chip);
> > +
> > + return 0;
> > +}
> > +
> > +static int aspeed_spi_user_unprepare_msg(struct spi_controller *ctlr,
> > + struct spi_message *msg)
> > +{
> > + struct aspeed_spi *aspi =
> > + (struct aspeed_spi *)spi_controller_get_devdata(ctlr);
> > + struct spi_device *spi = msg->spi;
> > + u32 cs = spi_get_chipselect(spi, 0);
> > + struct aspeed_spi_chip *chip = &aspi->chips[cs];
> > +
> > + if (aspi->cs_change == 0)
> > + aspeed_spi_stop_user(chip);
> > +
> > + return 0;
> > +}
> > +
> > +static void aspeed_spi_user_transfer_tx(struct aspeed_spi *aspi,
> > + struct spi_device *spi,
> > + const u8 *tx_buf, u8 *rx_buf,
> > + void *dst, u32 len)
> > +{
> > + const struct aspeed_spi_data *data = aspi->data;
> > + bool full_duplex_transfer = data->full_duplex && tx_buf == rx_buf;
> > + u32 i;
> > +
> > + if (full_duplex_transfer &&
> > + !!(spi->mode & (SPI_TX_DUAL | SPI_TX_QUAD |
> > + SPI_RX_DUAL | SPI_RX_QUAD))) {
> > + dev_err(aspi->dev,
> > + "full duplex is only supported for single IO mode\n");
> > + return;
> > + }
> > +
> > + for (i = 0; i < len; i++) {
> > + writeb(tx_buf[i], dst);
> > + if (full_duplex_transfer)
> > + rx_buf[i] = readb(aspi->regs + FULL_DUPLEX_RX_DATA);
> > + }
> > +}
> > +
> > +static int aspeed_spi_user_transfer(struct spi_controller *ctlr,
> > + struct spi_device *spi,
> > + struct spi_transfer *xfer)
> > +{
> > + struct aspeed_spi *aspi =
> > + (struct aspeed_spi *)spi_controller_get_devdata(ctlr);
> > + u32 cs = spi_get_chipselect(spi, 0);
> > + struct aspeed_spi_chip *chip = &aspi->chips[cs];
> > + void __iomem *ahb_base = aspi->chips[cs].ahb_base;
> > + const u8 *tx_buf = xfer->tx_buf;
> > + u8 *rx_buf = xfer->rx_buf;
> > +
> > + dev_dbg(aspi->dev,
> > + "[cs%d] xfer: width %d, len %u, tx %p, rx %p\n",
> > + cs, xfer->bits_per_word, xfer->len,
> > + tx_buf, rx_buf);
> > +
> > + if (tx_buf) {
> > + if (spi->mode & SPI_TX_DUAL)
> > + aspeed_spi_set_io_mode(chip, CTRL_IO_DUAL_DATA);
> > + else if (spi->mode & SPI_TX_QUAD)
> > + aspeed_spi_set_io_mode(chip, CTRL_IO_QUAD_DATA);
> > +
> > + aspeed_spi_user_transfer_tx(aspi, spi, tx_buf, rx_buf,
> > + (void *)ahb_base, xfer->len);
> > + }
> > +
> > + if (rx_buf && rx_buf != tx_buf) {
> > + if (spi->mode & SPI_RX_DUAL)
> > + aspeed_spi_set_io_mode(chip, CTRL_IO_DUAL_DATA);
> > + else if (spi->mode & SPI_RX_QUAD)
> > + aspeed_spi_set_io_mode(chip, CTRL_IO_QUAD_DATA);
>
> Should other modes be handled?
>
> > +
> > + ioread8_rep(ahb_base, rx_buf, xfer->len);
> > + }
> > +
> > + xfer->error = 0;
> > + aspi->cs_change = xfer->cs_change;
> > +
> > + return 0;
> > +}
> > +
> > static int aspeed_spi_probe(struct platform_device *pdev)
> > {
> > struct device *dev = &pdev->dev;
> > @@ -899,6 +1010,9 @@ static int aspeed_spi_probe(struct
> platform_device *pdev)
> > ctlr->cleanup = aspeed_spi_cleanup;
> > ctlr->num_chipselect =
> of_get_available_child_count(dev->of_node);
> > ctlr->dev.of_node = dev->of_node;
> > + ctlr->prepare_message = aspeed_spi_user_prepare_msg;
> > + ctlr->unprepare_message = aspeed_spi_user_unprepare_msg;
> > + ctlr->transfer_one = aspeed_spi_user_transfer;
> >
> > aspi->num_cs = ctlr->num_chipselect;
> >
> > @@ -1455,6 +1569,7 @@ static const struct aspeed_spi_data
> ast2400_fmc_data = {
> > .hclk_mask = 0xfffff0ff,
> > .hdiv_max = 1,
> > .min_window_size = 0x800000,
> > + .full_duplex = false,
> > .calibrate = aspeed_spi_calibrate,
> > .get_clk_div = aspeed_get_clk_div_ast2400,
> > .segment_start = aspeed_spi_segment_start, @@ -1471,6 +1586,7
> @@
> > static const struct aspeed_spi_data ast2400_spi_data = {
> > .timing = 0x14,
> > .hclk_mask = 0xfffff0ff,
> > .hdiv_max = 1,
> > + .full_duplex = false,
> > .get_clk_div = aspeed_get_clk_div_ast2400,
> > .calibrate = aspeed_spi_calibrate,
> > /* No segment registers */
> > @@ -1485,6 +1601,7 @@ static const struct aspeed_spi_data
> ast2500_fmc_data = {
> > .hclk_mask = 0xffffd0ff,
> > .hdiv_max = 1,
> > .min_window_size = 0x800000,
> > + .full_duplex = false,
> > .get_clk_div = aspeed_get_clk_div_ast2500,
> > .calibrate = aspeed_spi_calibrate,
> > .segment_start = aspeed_spi_segment_start, @@ -1502,6 +1619,7
> @@
> > static const struct aspeed_spi_data ast2500_spi_data = {
> > .hclk_mask = 0xffffd0ff,
> > .hdiv_max = 1,
> > .min_window_size = 0x800000,
> > + .full_duplex = false,
> > .get_clk_div = aspeed_get_clk_div_ast2500,
> > .calibrate = aspeed_spi_calibrate,
> > .segment_start = aspeed_spi_segment_start, @@ -1520,6 +1638,7
> @@
> > static const struct aspeed_spi_data ast2600_fmc_data = {
> > .hclk_mask = 0xf0fff0ff,
> > .hdiv_max = 2,
> > .min_window_size = 0x200000,
> > + .full_duplex = false,
> > .get_clk_div = aspeed_get_clk_div_ast2600,
> > .calibrate = aspeed_spi_ast2600_calibrate,
> > .segment_start = aspeed_spi_segment_ast2600_start, @@ -1538,6
> > +1657,7 @@ static const struct aspeed_spi_data ast2600_spi_data = {
> > .hclk_mask = 0xf0fff0ff,
> > .hdiv_max = 2,
> > .min_window_size = 0x200000,
> > + .full_duplex = false,
> > .get_clk_div = aspeed_get_clk_div_ast2600,
> > .calibrate = aspeed_spi_ast2600_calibrate,
> > .segment_start = aspeed_spi_segment_ast2600_start, @@ -1556,6
> > +1676,7 @@ static const struct aspeed_spi_data ast2700_fmc_data = {
> > .hclk_mask = 0xf0fff0ff,
> > .hdiv_max = 2,
> > .min_window_size = 0x10000,
> > + .full_duplex = true,
> > .get_clk_div = aspeed_get_clk_div_ast2600,
> > .calibrate = aspeed_spi_ast2600_calibrate,
> > .segment_start = aspeed_spi_segment_ast2700_start, @@ -1573,6
> > +1694,7 @@ static const struct aspeed_spi_data ast2700_spi_data = {
> > .hclk_mask = 0xf0fff0ff,
> > .hdiv_max = 2,
> > .min_window_size = 0x10000,
> > + .full_duplex = true,
> > .get_clk_div = aspeed_get_clk_div_ast2600,
> > .calibrate = aspeed_spi_ast2600_calibrate,
> > .segment_start = aspeed_spi_segment_ast2700_start,
>
>
> Kind regards,
>
> Paul
Powered by blists - more mailing lists