Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1504053 > unrolled thread

[PATCH 0/3] Support H3 SPI controller

Started byMilo Kim <woogyom.kim@gmail.com>
First post2016-10-19 18:20 +0200
Last post2016-10-20 18:00 +0200
Articles 8 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/3] Support H3 SPI controller Milo Kim <woogyom.kim@gmail.com> - 2016-10-19 18:20 +0200
    [PATCH 3/3] spi: sun6i: Use the driver data to get the buffer size Milo Kim <woogyom.kim@gmail.com> - 2016-10-19 18:20 +0200
      Re: [PATCH 3/3] spi: sun6i: Use the driver data to get the buffer  size Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-20 18:00 +0200
        Re: [PATCH 3/3] spi: sun6i: Use the driver data to get the buffer  size Milo Kim <woogyom.kim@gmail.com> - 2016-10-21 15:50 +0200
    [PATCH 2/3] spi: sun6i: Support Allwinner H3 SPI controller Milo Kim <woogyom.kim@gmail.com> - 2016-10-19 18:30 +0200
      Re: [PATCH 2/3] spi: sun6i: Support Allwinner H3 SPI controller Chen-Yu Tsai <wens@csie.org> - 2016-10-20 18:00 +0200
        Re: [PATCH 2/3] spi: sun6i: Support Allwinner H3 SPI controller Milo Kim <woogyom.kim@gmail.com> - 2016-10-21 15:40 +0200
      Re: [PATCH 2/3] spi: sun6i: Support Allwinner H3 SPI controller Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-20 18:00 +0200

#1504053 — [PATCH 0/3] Support H3 SPI controller

FromMilo Kim <woogyom.kim@gmail.com>
Date2016-10-19 18:20 +0200
Subject[PATCH 0/3] Support H3 SPI controller
Message-ID<su1Bg-3zK-23@gated-at.bofh.it>
Allwinner H3 SPI controller has same architecture as A31.
So most configuration is identical except one thing - FIFO capacity.

                         A31    H3
------------------------------------
Number of controllers     4      2
Number of FIFO depth     128    64
Transfer bits             8      8

Register maps are sharable, so sun6i SPI driver is reusable with
device configuration.

Tested on Nano Pi M1 and SPI slave device is TI LP8860.

Milo Kim (3):
  spi: sun6i: Add binding for Allwinner H3 SPI controller
  spi: sun6i: Support Allwinner H3 SPI controller
  spi: sun6i: Use the driver data to get the buffer size

 .../devicetree/bindings/spi/spi-sun6i.txt          | 25 +++++++++++-
 drivers/spi/Kconfig                                |  4 +-
 drivers/spi/spi-sun6i.c                            | 44 +++++++++++++++-------
 3 files changed, 56 insertions(+), 17 deletions(-)

-- 
2.9.3

[toc] | [next] | [standalone]


#1504055 — [PATCH 3/3] spi: sun6i: Use the driver data to get the buffer size

FromMilo Kim <woogyom.kim@gmail.com>
Date2016-10-19 18:20 +0200
Subject[PATCH 3/3] spi: sun6i: Use the driver data to get the buffer size
Message-ID<su1KW-3Hg-15@gated-at.bofh.it>
In reply to#1504053
The argument can be removed because the driver private data has it.

Cc: Mark Brown <broonie@kernel.org>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
---
 drivers/spi/spi-sun6i.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 91235b2..c638f5d 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -105,8 +105,9 @@ static inline void sun6i_spi_write(struct sun6i_spi *sspi, u32 reg, u32 value)
 	writel(value, sspi->base_addr + reg);
 }
 
-static inline void sun6i_spi_drain_fifo(struct sun6i_spi *sspi, int len)
+static inline void sun6i_spi_drain_fifo(struct sun6i_spi *sspi)
 {
+	int len = sspi->fifo_depth;
 	u32 reg, cnt;
 	u8 byte;
 
@@ -125,8 +126,9 @@ static inline void sun6i_spi_drain_fifo(struct sun6i_spi *sspi, int len)
 	}
 }
 
-static inline void sun6i_spi_fill_fifo(struct sun6i_spi *sspi, int len)
+static inline void sun6i_spi_fill_fifo(struct sun6i_spi *sspi)
 {
+	int len = sspi->fifo_depth;
 	u8 byte;
 
 	if (len > sspi->len)
@@ -270,7 +272,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
 			SUN6I_BURST_CTL_CNT_STC(tx_len));
 
 	/* Fill the TX FIFO */
-	sun6i_spi_fill_fifo(sspi, sspi->fifo_depth);
+	sun6i_spi_fill_fifo(sspi);
 
 	/* Enable the interrupts */
 	sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, SUN6I_INT_CTL_TC);
@@ -293,7 +295,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
 		goto out;
 	}
 
-	sun6i_spi_drain_fifo(sspi, sspi->fifo_depth);
+	sun6i_spi_drain_fifo(sspi);
 
 out:
 	sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, 0);
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]


#1505056 — Re: [PATCH 3/3] spi: sun6i: Use the driver data to get the buffer size

FromMaxime Ripard <maxime.ripard@free-electrons.com>
Date2016-10-20 18:00 +0200
SubjectRe: [PATCH 3/3] spi: sun6i: Use the driver data to get the buffer size
Message-ID<sunV7-161-17@gated-at.bofh.it>
In reply to#1504055

[Multipart message — attachments visible in raw view] — view raw

On Wed, Oct 19, 2016 at 11:02:34PM +0900, Milo Kim wrote:
> The argument can be removed because the driver private data has it.
> 
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
> ---
>  drivers/spi/spi-sun6i.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
> index 91235b2..c638f5d 100644
> --- a/drivers/spi/spi-sun6i.c
> +++ b/drivers/spi/spi-sun6i.c
> @@ -105,8 +105,9 @@ static inline void sun6i_spi_write(struct sun6i_spi *sspi, u32 reg, u32 value)
>  	writel(value, sspi->base_addr + reg);
>  }
>  
> -static inline void sun6i_spi_drain_fifo(struct sun6i_spi *sspi, int len)
> +static inline void sun6i_spi_drain_fifo(struct sun6i_spi *sspi)
>  {
> +	int len = sspi->fifo_depth;
>  	u32 reg, cnt;
>  	u8 byte;

I'm not so sure about this one. This function was initially to drain
any given number of bytes, which might be of interest at some point.

Removing that argument also remove that ability... Any particular
reason you want to change it?

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]


#1505964 — Re: [PATCH 3/3] spi: sun6i: Use the driver data to get the buffer size

FromMilo Kim <woogyom.kim@gmail.com>
Date2016-10-21 15:50 +0200
SubjectRe: [PATCH 3/3] spi: sun6i: Use the driver data to get the buffer size
Message-ID<suImR-6aO-13@gated-at.bofh.it>
In reply to#1505056
On 10/21/2016 12:56 AM, Maxime Ripard wrote:
>> -static inline void sun6i_spi_drain_fifo(struct sun6i_spi *sspi, int len)
>> > +static inline void sun6i_spi_drain_fifo(struct sun6i_spi *sspi)
>> >  {
>> > +	int len = sspi->fifo_depth;
>> >  	u32 reg, cnt;
>> >  	u8 byte;
> I'm not so sure about this one. This function was initially to drain
> any given number of bytes, which might be of interest at some point.
>
> Removing that argument also remove that ability... Any particular
> reason you want to change it?

I just wanted to remove duplicate variable but I agree with you. Let me 
drop this in the next patch-set. Thanks for your comments.

Best regards,
Milo

[toc] | [prev] | [next] | [standalone]


#1504103 — [PATCH 2/3] spi: sun6i: Support Allwinner H3 SPI controller

FromMilo Kim <woogyom.kim@gmail.com>
Date2016-10-19 18:30 +0200
Subject[PATCH 2/3] spi: sun6i: Support Allwinner H3 SPI controller
Message-ID<su1UD-3KZ-65@gated-at.bofh.it>
In reply to#1504053
H3 has two SPI controllers. The size of the buffer is 64 * 8.
(8 bit transfer by 64 entry FIFO)
A31 has four controllers. The size of the buffer is 128 * 8.
(8 bit transfer by 128 entry FIFO)

Register maps are sharable, so sun6i SPI driver is reusable with
device configuration.

Use the variable, 'fifo_depth' instead of fixed value to support both SPI
controllers.

Cc: Mark Brown <broonie@kernel.org>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
---
 drivers/spi/Kconfig     |  4 ++--
 drivers/spi/spi-sun6i.c | 38 +++++++++++++++++++++++++++-----------
 2 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index b799547..06d0845 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -610,11 +610,11 @@ config SPI_SUN4I
 	  SPI driver for Allwinner sun4i, sun5i and sun7i SoCs
 
 config SPI_SUN6I
-	tristate "Allwinner A31 SPI controller"
+	tristate "Allwinner A31/H3 SPI controller"
 	depends on ARCH_SUNXI || COMPILE_TEST
 	depends on RESET_CONTROLLER
 	help
-	  This enables using the SPI controller on the Allwinner A31 SoCs.
+	  This enables using the SPI controller on the Allwinner A31/H3 SoCs.
 
 config SPI_MXS
 	tristate "Freescale MXS SPI controller"
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
index 9918a57..91235b2 100644
--- a/drivers/spi/spi-sun6i.c
+++ b/drivers/spi/spi-sun6i.c
@@ -17,6 +17,7 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/reset.h>
@@ -24,6 +25,7 @@
 #include <linux/spi/spi.h>
 
 #define SUN6I_FIFO_DEPTH		128
+#define SUN8I_FIFO_DEPTH		64
 
 #define SUN6I_GBL_CTL_REG		0x04
 #define SUN6I_GBL_CTL_BUS_ENABLE		BIT(0)
@@ -90,6 +92,7 @@ struct sun6i_spi {
 	const u8		*tx_buf;
 	u8			*rx_buf;
 	int			len;
+	int			fifo_depth;
 };
 
 static inline u32 sun6i_spi_read(struct sun6i_spi *sspi, u32 reg)
@@ -155,7 +158,9 @@ static void sun6i_spi_set_cs(struct spi_device *spi, bool enable)
 
 static size_t sun6i_spi_max_transfer_size(struct spi_device *spi)
 {
-	return SUN6I_FIFO_DEPTH - 1;
+	struct sun6i_spi *sspi = spi_master_get_devdata(spi->master);
+
+	return sspi->fifo_depth - 1;
 }
 
 static int sun6i_spi_transfer_one(struct spi_master *master,
@@ -170,7 +175,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
 	u32 reg;
 
 	/* We don't support transfer larger than the FIFO */
-	if (tfr->len > SUN6I_FIFO_DEPTH)
+	if (tfr->len > sspi->fifo_depth)
 		return -EINVAL;
 
 	reinit_completion(&sspi->done);
@@ -265,7 +270,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
 			SUN6I_BURST_CTL_CNT_STC(tx_len));
 
 	/* Fill the TX FIFO */
-	sun6i_spi_fill_fifo(sspi, SUN6I_FIFO_DEPTH);
+	sun6i_spi_fill_fifo(sspi, sspi->fifo_depth);
 
 	/* Enable the interrupts */
 	sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, SUN6I_INT_CTL_TC);
@@ -288,7 +293,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
 		goto out;
 	}
 
-	sun6i_spi_drain_fifo(sspi, SUN6I_FIFO_DEPTH);
+	sun6i_spi_drain_fifo(sspi, sspi->fifo_depth);
 
 out:
 	sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, 0);
@@ -360,8 +365,16 @@ static int sun6i_spi_runtime_suspend(struct device *dev)
 	return 0;
 }
 
+static const struct of_device_id sun6i_spi_match[] = {
+	{ .compatible = "allwinner,sun6i-a31-spi", .data = (int *)SUN6I_FIFO_DEPTH },
+	{ .compatible = "allwinner,sun8i-h3-spi", .data = (int *)SUN8I_FIFO_DEPTH },
+	{}
+};
+MODULE_DEVICE_TABLE(of, sun6i_spi_match);
+
 static int sun6i_spi_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *id;
 	struct spi_master *master;
 	struct sun6i_spi *sspi;
 	struct resource	*res;
@@ -398,6 +411,15 @@ static int sun6i_spi_probe(struct platform_device *pdev)
 	}
 
 	sspi->master = master;
+
+	id = of_match_device(sun6i_spi_match, &pdev->dev);
+	if (!id) {
+		dev_err(&pdev->dev, "Cannot get device ID\n");
+		goto err_free_master;
+	}
+
+	sspi->fifo_depth = (int)id->data;
+
 	master->max_speed_hz = 100 * 1000 * 1000;
 	master->min_speed_hz = 3 * 1000;
 	master->set_cs = sun6i_spi_set_cs;
@@ -469,12 +491,6 @@ static int sun6i_spi_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id sun6i_spi_match[] = {
-	{ .compatible = "allwinner,sun6i-a31-spi", },
-	{}
-};
-MODULE_DEVICE_TABLE(of, sun6i_spi_match);
-
 static const struct dev_pm_ops sun6i_spi_pm_ops = {
 	.runtime_resume		= sun6i_spi_runtime_resume,
 	.runtime_suspend	= sun6i_spi_runtime_suspend,
@@ -493,5 +509,5 @@ module_platform_driver(sun6i_spi_driver);
 
 MODULE_AUTHOR("Pan Nan <pannan@allwinnertech.com>");
 MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
-MODULE_DESCRIPTION("Allwinner A31 SPI controller driver");
+MODULE_DESCRIPTION("Allwinner A31/H3 SPI controller driver");
 MODULE_LICENSE("GPL");
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]


#1505053 — Re: [PATCH 2/3] spi: sun6i: Support Allwinner H3 SPI controller

FromChen-Yu Tsai <wens@csie.org>
Date2016-10-20 18:00 +0200
SubjectRe: [PATCH 2/3] spi: sun6i: Support Allwinner H3 SPI controller
Message-ID<sunV7-161-3@gated-at.bofh.it>
In reply to#1504103
On Wed, Oct 19, 2016 at 10:02 PM, Milo Kim <woogyom.kim@gmail.com> wrote:
> H3 has two SPI controllers. The size of the buffer is 64 * 8.
> (8 bit transfer by 64 entry FIFO)
> A31 has four controllers. The size of the buffer is 128 * 8.
> (8 bit transfer by 128 entry FIFO)
>
> Register maps are sharable, so sun6i SPI driver is reusable with
> device configuration.
>
> Use the variable, 'fifo_depth' instead of fixed value to support both SPI
> controllers.
>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
> ---
>  drivers/spi/Kconfig     |  4 ++--
>  drivers/spi/spi-sun6i.c | 38 +++++++++++++++++++++++++++-----------
>  2 files changed, 29 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index b799547..06d0845 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -610,11 +610,11 @@ config SPI_SUN4I
>           SPI driver for Allwinner sun4i, sun5i and sun7i SoCs
>
>  config SPI_SUN6I
> -       tristate "Allwinner A31 SPI controller"
> +       tristate "Allwinner A31/H3 SPI controller"
>         depends on ARCH_SUNXI || COMPILE_TEST
>         depends on RESET_CONTROLLER
>         help
> -         This enables using the SPI controller on the Allwinner A31 SoCs.
> +         This enables using the SPI controller on the Allwinner A31/H3 SoCs.
>
>  config SPI_MXS
>         tristate "Freescale MXS SPI controller"
> diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
> index 9918a57..91235b2 100644
> --- a/drivers/spi/spi-sun6i.c
> +++ b/drivers/spi/spi-sun6i.c
> @@ -17,6 +17,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/reset.h>
> @@ -24,6 +25,7 @@
>  #include <linux/spi/spi.h>
>
>  #define SUN6I_FIFO_DEPTH               128
> +#define SUN8I_FIFO_DEPTH               64
>
>  #define SUN6I_GBL_CTL_REG              0x04
>  #define SUN6I_GBL_CTL_BUS_ENABLE               BIT(0)
> @@ -90,6 +92,7 @@ struct sun6i_spi {
>         const u8                *tx_buf;
>         u8                      *rx_buf;
>         int                     len;
> +       int                     fifo_depth;
>  };
>
>  static inline u32 sun6i_spi_read(struct sun6i_spi *sspi, u32 reg)
> @@ -155,7 +158,9 @@ static void sun6i_spi_set_cs(struct spi_device *spi, bool enable)
>
>  static size_t sun6i_spi_max_transfer_size(struct spi_device *spi)
>  {
> -       return SUN6I_FIFO_DEPTH - 1;
> +       struct sun6i_spi *sspi = spi_master_get_devdata(spi->master);
> +
> +       return sspi->fifo_depth - 1;
>  }
>
>  static int sun6i_spi_transfer_one(struct spi_master *master,
> @@ -170,7 +175,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
>         u32 reg;
>
>         /* We don't support transfer larger than the FIFO */
> -       if (tfr->len > SUN6I_FIFO_DEPTH)
> +       if (tfr->len > sspi->fifo_depth)
>                 return -EINVAL;
>
>         reinit_completion(&sspi->done);
> @@ -265,7 +270,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
>                         SUN6I_BURST_CTL_CNT_STC(tx_len));
>
>         /* Fill the TX FIFO */
> -       sun6i_spi_fill_fifo(sspi, SUN6I_FIFO_DEPTH);
> +       sun6i_spi_fill_fifo(sspi, sspi->fifo_depth);
>
>         /* Enable the interrupts */
>         sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, SUN6I_INT_CTL_TC);
> @@ -288,7 +293,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
>                 goto out;
>         }
>
> -       sun6i_spi_drain_fifo(sspi, SUN6I_FIFO_DEPTH);
> +       sun6i_spi_drain_fifo(sspi, sspi->fifo_depth);
>
>  out:
>         sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, 0);
> @@ -360,8 +365,16 @@ static int sun6i_spi_runtime_suspend(struct device *dev)
>         return 0;
>  }
>
> +static const struct of_device_id sun6i_spi_match[] = {
> +       { .compatible = "allwinner,sun6i-a31-spi", .data = (int *)SUN6I_FIFO_DEPTH },
> +       { .compatible = "allwinner,sun8i-h3-spi", .data = (int *)SUN8I_FIFO_DEPTH },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, sun6i_spi_match);
> +
>  static int sun6i_spi_probe(struct platform_device *pdev)
>  {
> +       const struct of_device_id *id;
>         struct spi_master *master;
>         struct sun6i_spi *sspi;
>         struct resource *res;
> @@ -398,6 +411,15 @@ static int sun6i_spi_probe(struct platform_device *pdev)
>         }
>
>         sspi->master = master;
> +
> +       id = of_match_device(sun6i_spi_match, &pdev->dev);

Use of_device_get_match_data, then you don't even have to move sun6i_spi_match
above this function.

ChenYu

> +       if (!id) {
> +               dev_err(&pdev->dev, "Cannot get device ID\n");
> +               goto err_free_master;
> +       }
> +
> +       sspi->fifo_depth = (int)id->data;
> +
>         master->max_speed_hz = 100 * 1000 * 1000;
>         master->min_speed_hz = 3 * 1000;
>         master->set_cs = sun6i_spi_set_cs;
> @@ -469,12 +491,6 @@ static int sun6i_spi_remove(struct platform_device *pdev)
>         return 0;
>  }
>
> -static const struct of_device_id sun6i_spi_match[] = {
> -       { .compatible = "allwinner,sun6i-a31-spi", },
> -       {}
> -};
> -MODULE_DEVICE_TABLE(of, sun6i_spi_match);
> -
>  static const struct dev_pm_ops sun6i_spi_pm_ops = {
>         .runtime_resume         = sun6i_spi_runtime_resume,
>         .runtime_suspend        = sun6i_spi_runtime_suspend,
> @@ -493,5 +509,5 @@ module_platform_driver(sun6i_spi_driver);
>
>  MODULE_AUTHOR("Pan Nan <pannan@allwinnertech.com>");
>  MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
> -MODULE_DESCRIPTION("Allwinner A31 SPI controller driver");
> +MODULE_DESCRIPTION("Allwinner A31/H3 SPI controller driver");
>  MODULE_LICENSE("GPL");
> --
> 2.9.3
>

[toc] | [prev] | [next] | [standalone]


#1505952 — Re: [PATCH 2/3] spi: sun6i: Support Allwinner H3 SPI controller

FromMilo Kim <woogyom.kim@gmail.com>
Date2016-10-21 15:40 +0200
SubjectRe: [PATCH 2/3] spi: sun6i: Support Allwinner H3 SPI controller
Message-ID<suIdb-67f-3@gated-at.bofh.it>
In reply to#1505053
On 10/21/2016 12:53 AM, Chen-Yu Tsai wrote:
> Use of_device_get_match_data, then you don't even have to move sun6i_spi_match
> above this function.

Oh, good. Thanks for introducing this helper.

Best regards,
Milo

[toc] | [prev] | [next] | [standalone]


#1505060 — Re: [PATCH 2/3] spi: sun6i: Support Allwinner H3 SPI controller

FromMaxime Ripard <maxime.ripard@free-electrons.com>
Date2016-10-20 18:00 +0200
SubjectRe: [PATCH 2/3] spi: sun6i: Support Allwinner H3 SPI controller
Message-ID<sunV7-161-27@gated-at.bofh.it>
In reply to#1504103

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Wed, Oct 19, 2016 at 11:02:33PM +0900, Milo Kim wrote:
> H3 has two SPI controllers. The size of the buffer is 64 * 8.
> (8 bit transfer by 64 entry FIFO)
> A31 has four controllers. The size of the buffer is 128 * 8.
> (8 bit transfer by 128 entry FIFO)
> 
> Register maps are sharable, so sun6i SPI driver is reusable with
> device configuration.
> 
> Use the variable, 'fifo_depth' instead of fixed value to support both SPI
> controllers.
> 
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
> ---
>  drivers/spi/Kconfig     |  4 ++--
>  drivers/spi/spi-sun6i.c | 38 +++++++++++++++++++++++++++-----------
>  2 files changed, 29 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index b799547..06d0845 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -610,11 +610,11 @@ config SPI_SUN4I
>  	  SPI driver for Allwinner sun4i, sun5i and sun7i SoCs
>  
>  config SPI_SUN6I
> -	tristate "Allwinner A31 SPI controller"
> +	tristate "Allwinner A31/H3 SPI controller"
>  	depends on ARCH_SUNXI || COMPILE_TEST
>  	depends on RESET_CONTROLLER
>  	help
> -	  This enables using the SPI controller on the Allwinner A31 SoCs.
> +	  This enables using the SPI controller on the Allwinner A31/H3 SoCs.

Usually, we don't care about maintaining this. This is just going to
be a list that grows all the time, and is wrong most of the time.

>  config SPI_MXS
>  	tristate "Freescale MXS SPI controller"
> diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
> index 9918a57..91235b2 100644
> --- a/drivers/spi/spi-sun6i.c
> +++ b/drivers/spi/spi-sun6i.c
> @@ -17,6 +17,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/reset.h>
> @@ -24,6 +25,7 @@
>  #include <linux/spi/spi.h>
>  
>  #define SUN6I_FIFO_DEPTH		128
> +#define SUN8I_FIFO_DEPTH		64
>  
>  #define SUN6I_GBL_CTL_REG		0x04
>  #define SUN6I_GBL_CTL_BUS_ENABLE		BIT(0)
> @@ -90,6 +92,7 @@ struct sun6i_spi {
>  	const u8		*tx_buf;
>  	u8			*rx_buf;
>  	int			len;
> +	int			fifo_depth;
>  };
>  
>  static inline u32 sun6i_spi_read(struct sun6i_spi *sspi, u32 reg)
> @@ -155,7 +158,9 @@ static void sun6i_spi_set_cs(struct spi_device *spi, bool enable)
>  
>  static size_t sun6i_spi_max_transfer_size(struct spi_device *spi)
>  {
> -	return SUN6I_FIFO_DEPTH - 1;
> +	struct sun6i_spi *sspi = spi_master_get_devdata(spi->master);
> +
> +	return sspi->fifo_depth - 1;
>  }
>  
>  static int sun6i_spi_transfer_one(struct spi_master *master,
> @@ -170,7 +175,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
>  	u32 reg;
>  
>  	/* We don't support transfer larger than the FIFO */
> -	if (tfr->len > SUN6I_FIFO_DEPTH)
> +	if (tfr->len > sspi->fifo_depth)
>  		return -EINVAL;
>  
>  	reinit_completion(&sspi->done);
> @@ -265,7 +270,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
>  			SUN6I_BURST_CTL_CNT_STC(tx_len));
>  
>  	/* Fill the TX FIFO */
> -	sun6i_spi_fill_fifo(sspi, SUN6I_FIFO_DEPTH);
> +	sun6i_spi_fill_fifo(sspi, sspi->fifo_depth);
>  
>  	/* Enable the interrupts */
>  	sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, SUN6I_INT_CTL_TC);
> @@ -288,7 +293,7 @@ static int sun6i_spi_transfer_one(struct spi_master *master,
>  		goto out;
>  	}
>  
> -	sun6i_spi_drain_fifo(sspi, SUN6I_FIFO_DEPTH);
> +	sun6i_spi_drain_fifo(sspi, sspi->fifo_depth);
>  
>  out:
>  	sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, 0);
> @@ -360,8 +365,16 @@ static int sun6i_spi_runtime_suspend(struct device *dev)
>  	return 0;
>  }
>  
> +static const struct of_device_id sun6i_spi_match[] = {
> +	{ .compatible = "allwinner,sun6i-a31-spi", .data = (int *)SUN6I_FIFO_DEPTH },
> +	{ .compatible = "allwinner,sun8i-h3-spi", .data = (int *)SUN8I_FIFO_DEPTH },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, sun6i_spi_match);
> +
>  static int sun6i_spi_probe(struct platform_device *pdev)
>  {
> +	const struct of_device_id *id;
>  	struct spi_master *master;
>  	struct sun6i_spi *sspi;
>  	struct resource	*res;
> @@ -398,6 +411,15 @@ static int sun6i_spi_probe(struct platform_device *pdev)
>  	}
>  
>  	sspi->master = master;
> +
> +	id = of_match_device(sun6i_spi_match, &pdev->dev);
> +	if (!id) {
> +		dev_err(&pdev->dev, "Cannot get device ID\n");
> +		goto err_free_master;
> +	}
> +
> +	sspi->fifo_depth = (int)id->data;
> +
>  	master->max_speed_hz = 100 * 1000 * 1000;
>  	master->min_speed_hz = 3 * 1000;
>  	master->set_cs = sun6i_spi_set_cs;
> @@ -469,12 +491,6 @@ static int sun6i_spi_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static const struct of_device_id sun6i_spi_match[] = {
> -	{ .compatible = "allwinner,sun6i-a31-spi", },
> -	{}
> -};
> -MODULE_DEVICE_TABLE(of, sun6i_spi_match);
> -
>  static const struct dev_pm_ops sun6i_spi_pm_ops = {
>  	.runtime_resume		= sun6i_spi_runtime_resume,
>  	.runtime_suspend	= sun6i_spi_runtime_suspend,
> @@ -493,5 +509,5 @@ module_platform_driver(sun6i_spi_driver);
>  
>  MODULE_AUTHOR("Pan Nan <pannan@allwinnertech.com>");
>  MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com>");
> -MODULE_DESCRIPTION("Allwinner A31 SPI controller driver");
> +MODULE_DESCRIPTION("Allwinner A31/H3 SPI controller driver");

Ditto.

It looks good otherwise.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web