diff mbox series

[v3,2/5] spi: sun6i: change OF match data to a struct

Message ID 20230506232616.1792109-3-bigunclemax@gmail.com
State New
Headers show
Series Allwinner R329/D1/R528/T113s SPI support | expand

Commit Message

Maksim Kiselev May 6, 2023, 11:26 p.m. UTC 
From: Icenowy Zheng <icenowy@aosc.io>

As we're adding more properties to the OF match data, convert it to a
struct now.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
Reviewed-by: Samuel Holland <samuel@sholland.org>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/spi/spi-sun6i.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

Comments

Maksim Kiselev May 10, 2023, 8:33 a.m. UTC | #1 
Hi, David

> Is it worth doing a structure copy at this point?
> The 'cfg' data is short and constant and it would make
> the code that uses it smaller and faster.

I'm sorry but I don't fully understand what you are suggesting.
In patch 3\5 we extend the sun6i_spi_cfg structure with the has_clk_ctl field.

пн, 8 мая 2023 г. в 12:47, David Laight <David.Laight@aculab.com>:
>
> From: Maksim Kiselev
> > Sent: 07 May 2023 00:26
> >
> > As we're adding more properties to the OF match data, convert it to a
> > struct now.
> >
> ...
> > -     sspi->fifo_depth = (unsigned long)of_device_get_match_data(&pdev->dev);
> > +     sspi->cfg = of_device_get_match_data(&pdev->dev);
>
> Is it worth doing a structure copy at this point?
> The 'cfg' data is short and constant and it would make
> the code that uses it smaller and faster.
>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
David Laight May 10, 2023, 8:55 a.m. UTC | #2 
From: Maxim Kiselev
> Sent: 10 May 2023 09:34
> 
> Hi, David
> 
> > Is it worth doing a structure copy at this point?
> > The 'cfg' data is short and constant and it would make
> > the code that uses it smaller and faster.
> 
> I'm sorry but I don't fully understand what you are suggesting.
> In patch 3\5 we extend the sun6i_spi_cfg structure with the has_clk_ctl field.

You are still only adding a second integer.

I'm suggesting that instead of sspi->cfg being a pointer to the
config data it is a copy of the config data.
So the assignment below becomes (ignoring error checks)
	memcpy(&sspi->cfg, of_device_get_match_data(&pdev->dev), sizeof sspi->cfg);
and the code that needs the values is:
	sspi->cfg.fifo_depth
(etc)

	David

> 
> пн, 8 мая 2023 г. в 12:47, David Laight <David.Laight@aculab.com>:
> >
> > From: Maksim Kiselev
> > > Sent: 07 May 2023 00:26
> > >
> > > As we're adding more properties to the OF match data, convert it to a
> > > struct now.
> > >
> > ...
> > > -     sspi->fifo_depth = (unsigned long)of_device_get_match_data(&pdev->dev);
> > > +     sspi->cfg = of_device_get_match_data(&pdev->dev);
> >
> > Is it worth doing a structure copy at this point?
> > The 'cfg' data is short and constant and it would make
> > the code that uses it smaller and faster.
> >
> >         David
> >
> > -
> > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > Registration No: 1397386 (Wales)
> >

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
diff mbox series

Patch

diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 7532c85a352c..01a01cd86db5 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -85,6 +85,10 @@ 
 #define SUN6I_TXDATA_REG		0x200
 #define SUN6I_RXDATA_REG		0x300
 
+struct sun6i_spi_cfg {
+	unsigned long		fifo_depth;
+};
+
 struct sun6i_spi {
 	struct spi_master	*master;
 	void __iomem		*base_addr;
@@ -99,7 +103,7 @@  struct sun6i_spi {
 	const u8		*tx_buf;
 	u8			*rx_buf;
 	int			len;
-	unsigned long		fifo_depth;
+	const struct sun6i_spi_cfg *cfg;
 };
 
 static inline u32 sun6i_spi_read(struct sun6i_spi *sspi, u32 reg)
@@ -156,7 +160,7 @@  static inline void sun6i_spi_fill_fifo(struct sun6i_spi *sspi)
 	u8 byte;
 
 	/* See how much data we can fit */
-	cnt = sspi->fifo_depth - sun6i_spi_get_tx_fifo_count(sspi);
+	cnt = sspi->cfg->fifo_depth - sun6i_spi_get_tx_fifo_count(sspi);
 
 	len = min((int)cnt, sspi->len);
 
@@ -289,14 +293,14 @@  static int sun6i_spi_transfer_one(struct spi_master *master,
 		 * the hardcoded value used in old generation of Allwinner
 		 * SPI controller. (See spi-sun4i.c)
 		 */
-		trig_level = sspi->fifo_depth / 4 * 3;
+		trig_level = sspi->cfg->fifo_depth / 4 * 3;
 	} else {
 		/*
 		 * Setup FIFO DMA request trigger level
 		 * We choose 1/2 of the full fifo depth, that value will
 		 * be used as DMA burst length.
 		 */
-		trig_level = sspi->fifo_depth / 2;
+		trig_level = sspi->cfg->fifo_depth / 2;
 
 		if (tfr->tx_buf)
 			reg |= SUN6I_FIFO_CTL_TF_DRQ_EN;
@@ -410,9 +414,9 @@  static int sun6i_spi_transfer_one(struct spi_master *master,
 	reg = SUN6I_INT_CTL_TC;
 
 	if (!use_dma) {
-		if (rx_len > sspi->fifo_depth)
+		if (rx_len > sspi->cfg->fifo_depth)
 			reg |= SUN6I_INT_CTL_RF_RDY;
-		if (tx_len > sspi->fifo_depth)
+		if (tx_len > sspi->cfg->fifo_depth)
 			reg |= SUN6I_INT_CTL_TF_ERQ;
 	}
 
@@ -543,7 +547,7 @@  static bool sun6i_spi_can_dma(struct spi_master *master,
 	 * the fifo length we can just fill the fifo and wait for a single
 	 * irq, so don't bother setting up dma
 	 */
-	return xfer->len > sspi->fifo_depth;
+	return xfer->len > sspi->cfg->fifo_depth;
 }
 
 static int sun6i_spi_probe(struct platform_device *pdev)
@@ -582,7 +586,7 @@  static int sun6i_spi_probe(struct platform_device *pdev)
 	}
 
 	sspi->master = master;
-	sspi->fifo_depth = (unsigned long)of_device_get_match_data(&pdev->dev);
+	sspi->cfg = of_device_get_match_data(&pdev->dev);
 
 	master->max_speed_hz = 100 * 1000 * 1000;
 	master->min_speed_hz = 3 * 1000;
@@ -695,9 +699,17 @@  static void sun6i_spi_remove(struct platform_device *pdev)
 		dma_release_channel(master->dma_rx);
 }
 
+static const struct sun6i_spi_cfg sun6i_a31_spi_cfg = {
+	.fifo_depth	= SUN6I_FIFO_DEPTH,
+};
+
+static const struct sun6i_spi_cfg sun8i_h3_spi_cfg = {
+	.fifo_depth	= SUN8I_FIFO_DEPTH,
+};
+
 static const struct of_device_id sun6i_spi_match[] = {
-	{ .compatible = "allwinner,sun6i-a31-spi", .data = (void *)SUN6I_FIFO_DEPTH },
-	{ .compatible = "allwinner,sun8i-h3-spi",  .data = (void *)SUN8I_FIFO_DEPTH },
+	{ .compatible = "allwinner,sun6i-a31-spi", .data = &sun6i_a31_spi_cfg },
+	{ .compatible = "allwinner,sun8i-h3-spi",  .data = &sun8i_h3_spi_cfg },
 	{}
 };
 MODULE_DEVICE_TABLE(of, sun6i_spi_match);