[PATCH 0/3] Improve SPI support for Ingenic SoCs.

[PATCH 0/3] Improve SPI support for Ingenic SoCs.

[周琰杰 (Zhou Yanjie)]

1.Add support for using GPIOs as chip select lines on Ingenic SoCs.
2.Add support for probing the spi-ingenic driver on the JZ4775 SoC,
  the X1000 SoC, and the X2000 SoC.
3.Modify annotation texts to be more in line with the current state.

周琰杰 (Zhou Yanjie) (3):
  SPI: Ingenic: Add support for use GPIO as chip select line.
  dt-bindings: SPI: Add bindings for new Ingenic SoCs.
  SPI: Ingenic: Add support for new Ingenic SoCs.

 .../devicetree/bindings/spi/ingenic,spi.yaml       |  3 ++
 drivers/spi/Kconfig                                |  4 +-
 drivers/spi/spi-ingenic.c                          | 49 +++++++++++++++++++---
 3 files changed, 49 insertions(+), 7 deletions(-)

-- 
2.7.4

[PATCH 1/3] SPI: Ingenic: Add support for use GPIO as chip select line.

[周琰杰 (Zhou Yanjie)]

Add support for using GPIOs as chip select lines on Ingenic SoCs.

Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---
 drivers/spi/spi-ingenic.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-ingenic.c b/drivers/spi/spi-ingenic.c
index 03077a7..672e4ed 100644
--- a/drivers/spi/spi-ingenic.c
+++ b/drivers/spi/spi-ingenic.c
@@ -380,7 +380,7 @@ static int spi_ingenic_probe(struct platform_device *pdev)
 	struct spi_controller *ctlr;
 	struct ingenic_spi *priv;
 	void __iomem *base;
-	int ret;
+	int num_cs, ret;
 
 	pdata = of_device_get_match_data(dev);
 	if (!pdata) {
@@ -416,6 +416,11 @@ static int spi_ingenic_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->flen_field))
 		return PTR_ERR(priv->flen_field);
 
+	if (of_property_read_u32(dev->of_node, "num-cs", &num_cs)) {
+		dev_warn(dev, "Number of chip select lines not specified.\n");
+		num_cs = 2;
+	}
+
 	platform_set_drvdata(pdev, ctlr);
 
 	ctlr->prepare_transfer_hardware = spi_ingenic_prepare_hardware;
@@ -429,7 +434,9 @@ static int spi_ingenic_probe(struct platform_device *pdev)
 	ctlr->bits_per_word_mask = pdata->bits_per_word_mask;
 	ctlr->min_speed_hz = 7200;
 	ctlr->max_speed_hz = 54000000;
-	ctlr->num_chipselect = 2;
+	ctlr->use_gpio_descriptors = true;
+	ctlr->max_native_cs = 2;
+	ctlr->num_chipselect = num_cs;
 	ctlr->dev.of_node = pdev->dev.of_node;
 
 	if (spi_ingenic_request_dma(ctlr, dev))
-- 
2.7.4

[PATCH 2/3] dt-bindings: SPI: Add bindings for new Ingenic SoCs.

[周琰杰 (Zhou Yanjie)]

Add the SPI bindings for the JZ4775 SoC, the X1000 SoC,
and the X2000 SoC from Ingenic.

Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---
 Documentation/devicetree/bindings/spi/ingenic,spi.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/ingenic,spi.yaml b/Documentation/devicetree/bindings/spi/ingenic,spi.yaml
index 5b1c7a2..360f76c 100644
--- a/Documentation/devicetree/bindings/spi/ingenic,spi.yaml
+++ b/Documentation/devicetree/bindings/spi/ingenic,spi.yaml
@@ -18,7 +18,10 @@ properties:
     oneOf:
       - enum:
           - ingenic,jz4750-spi
+          - ingenic,jz4775-spi
           - ingenic,jz4780-spi
+          - ingenic,x1000-spi
+          - ingenic,x2000-spi
       - items:
           - enum:
               - ingenic,jz4760-spi
-- 
2.7.4

[PATCH 3/3] SPI: Ingenic: Add support for new Ingenic SoCs.

[周琰杰 (Zhou Yanjie)]

1.Since it would be dangerous to specify a newer SoC's compatible
  string as the fallback of an older SoC's compatible string, we
  add support for the "ingenic,jz4775-spi" compatible string in
  the driver.

  This will permit to support the JZ4775 by having:
  compatible = "ingenic,jz4775-spi";

  Instead of doing:
  compatible = "ingenic,jz4775-spi", "ingenic,jz4780-spi";

2.Add support for probing the spi-ingenic driver on the X1000 SoC
  from Ingenic. From the X1000 SoC onwards, the maximum frequency
  allowed by the SSI module of Ingenic SoCs has been changed from
  54MHz to 50MHz. So "max_speed_hz" is introduced in "jz_soc_info"
  to set different maximum frequency values.

3.Add support for probing the spi-ingenic driver on the X2000 SoC
  from Ingenic. The X2000 SoC has only one native chip select line,
  so "max_native_cs" is introduced in "jz_soc_info" to set different
  maximum number of native chip select lines.

4.Because of the introduction of support for the X-series SoCs, the
  current driver is not only applicable to the JZ-series SoCs, so
  the description texts has been modified to avoid misunderstanding.

Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---
 drivers/spi/Kconfig       |  4 ++--
 drivers/spi/spi-ingenic.c | 42 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index d2815eb..cca92a8 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -419,10 +419,10 @@ config SPI_IMX
 	  This enables support for the Freescale i.MX SPI controllers.
 
 config SPI_INGENIC
-	tristate "Ingenic JZ47xx SoCs SPI controller"
+	tristate "Ingenic SoCs SPI controller"
 	depends on MACH_INGENIC || COMPILE_TEST
 	help
-	  This enables support for the Ingenic JZ47xx SoCs SPI controller.
+	  This enables support for the Ingenic SoCs SPI controller.
 
 	  To compile this driver as a module, choose M here: the module
 	  will be called spi-ingenic.
diff --git a/drivers/spi/spi-ingenic.c b/drivers/spi/spi-ingenic.c
index 672e4ed..ff507c8 100644
--- a/drivers/spi/spi-ingenic.c
+++ b/drivers/spi/spi-ingenic.c
@@ -1,8 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * SPI bus driver for the Ingenic JZ47xx SoCs
+ * SPI bus driver for the Ingenic SoCs
  * Copyright (c) 2017-2021 Artur Rojek <contact@artur-rojek.eu>
  * Copyright (c) 2017-2021 Paul Cercueil <paul@crapouillou.net>
+ * Copyright (c) 2022 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
  */
 
 #include <linux/clk.h>
@@ -52,6 +53,9 @@ struct jz_soc_info {
 	u32 bits_per_word_mask;
 	struct reg_field flen_field;
 	bool has_trendian;
+
+	unsigned int max_speed_hz;
+	unsigned int max_native_cs;
 };
 
 struct ingenic_spi {
@@ -418,7 +422,7 @@ static int spi_ingenic_probe(struct platform_device *pdev)
 
 	if (of_property_read_u32(dev->of_node, "num-cs", &num_cs)) {
 		dev_warn(dev, "Number of chip select lines not specified.\n");
-		num_cs = 2;
+		num_cs = pdata->max_native_cs;
 	}
 
 	platform_set_drvdata(pdev, ctlr);
@@ -433,9 +437,9 @@ static int spi_ingenic_probe(struct platform_device *pdev)
 	ctlr->max_dma_len = SPI_INGENIC_FIFO_SIZE;
 	ctlr->bits_per_word_mask = pdata->bits_per_word_mask;
 	ctlr->min_speed_hz = 7200;
-	ctlr->max_speed_hz = 54000000;
+	ctlr->max_speed_hz = pdata->max_speed_hz;
 	ctlr->use_gpio_descriptors = true;
-	ctlr->max_native_cs = 2;
+	ctlr->max_native_cs = pdata->max_native_cs;
 	ctlr->num_chipselect = num_cs;
 	ctlr->dev.of_node = pdev->dev.of_node;
 
@@ -459,17 +463,44 @@ static const struct jz_soc_info jz4750_soc_info = {
 	.bits_per_word_mask = SPI_BPW_RANGE_MASK(2, 17),
 	.flen_field = REG_FIELD(REG_SSICR1, 4, 7),
 	.has_trendian = false,
+
+	.max_speed_hz = 54000000,
+	.max_native_cs = 2,
 };
 
 static const struct jz_soc_info jz4780_soc_info = {
 	.bits_per_word_mask = SPI_BPW_RANGE_MASK(2, 32),
 	.flen_field = REG_FIELD(REG_SSICR1, 3, 7),
 	.has_trendian = true,
+
+	.max_speed_hz = 54000000,
+	.max_native_cs = 2,
+};
+
+static const struct jz_soc_info x1000_soc_info = {
+	.bits_per_word_mask = SPI_BPW_RANGE_MASK(2, 32),
+	.flen_field = REG_FIELD(REG_SSICR1, 3, 7),
+	.has_trendian = true,
+
+	.max_speed_hz = 50000000,
+	.max_native_cs = 2,
+};
+
+static const struct jz_soc_info x2000_soc_info = {
+	.bits_per_word_mask = SPI_BPW_RANGE_MASK(2, 32),
+	.flen_field = REG_FIELD(REG_SSICR1, 3, 7),
+	.has_trendian = true,
+
+	.max_speed_hz = 50000000,
+	.max_native_cs = 1,
 };
 
 static const struct of_device_id spi_ingenic_of_match[] = {
 	{ .compatible = "ingenic,jz4750-spi", .data = &jz4750_soc_info },
+	{ .compatible = "ingenic,jz4775-spi", .data = &jz4780_soc_info },
 	{ .compatible = "ingenic,jz4780-spi", .data = &jz4780_soc_info },
+	{ .compatible = "ingenic,x1000-spi", .data = &x1000_soc_info },
+	{ .compatible = "ingenic,x2000-spi", .data = &x2000_soc_info },
 	{}
 };
 MODULE_DEVICE_TABLE(of, spi_ingenic_of_match);
@@ -483,7 +514,8 @@ static struct platform_driver spi_ingenic_driver = {
 };
 
 module_platform_driver(spi_ingenic_driver);
-MODULE_DESCRIPTION("SPI bus driver for the Ingenic JZ47xx SoCs");
+MODULE_DESCRIPTION("SPI bus driver for the Ingenic SoCs");
 MODULE_AUTHOR("Artur Rojek <contact@artur-rojek.eu>");
 MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
+MODULE_AUTHOR("周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>");
 MODULE_LICENSE("GPL");
-- 
2.7.4

Re: [PATCH 1/3] SPI: Ingenic: Add support for use GPIO as chip select line.

[Paul Cercueil]

Hi Zhou,

Le ven., avril 15 2022 at 22:22:06 +0800, 周琰杰 (Zhou Yanjie) 
<zhouyanjie@wanyeetech.com> a écrit :
> Add support for using GPIOs as chip select lines on Ingenic SoCs.
> 
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> ---
>  drivers/spi/spi-ingenic.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi-ingenic.c b/drivers/spi/spi-ingenic.c
> index 03077a7..672e4ed 100644
> --- a/drivers/spi/spi-ingenic.c
> +++ b/drivers/spi/spi-ingenic.c
> @@ -380,7 +380,7 @@ static int spi_ingenic_probe(struct 
> platform_device *pdev)
>  	struct spi_controller *ctlr;
>  	struct ingenic_spi *priv;
>  	void __iomem *base;
> -	int ret;
> +	int num_cs, ret;
> 
>  	pdata = of_device_get_match_data(dev);
>  	if (!pdata) {
> @@ -416,6 +416,11 @@ static int spi_ingenic_probe(struct 
> platform_device *pdev)
>  	if (IS_ERR(priv->flen_field))
>  		return PTR_ERR(priv->flen_field);
> 
> +	if (of_property_read_u32(dev->of_node, "num-cs", &num_cs)) {
> +		dev_warn(dev, "Number of chip select lines not specified.\n");
> +		num_cs = 2;
> +	}
> +
>  	platform_set_drvdata(pdev, ctlr);
> 
>  	ctlr->prepare_transfer_hardware = spi_ingenic_prepare_hardware;
> @@ -429,7 +434,9 @@ static int spi_ingenic_probe(struct 
> platform_device *pdev)
>  	ctlr->bits_per_word_mask = pdata->bits_per_word_mask;
>  	ctlr->min_speed_hz = 7200;
>  	ctlr->max_speed_hz = 54000000;
> -	ctlr->num_chipselect = 2;
> +	ctlr->use_gpio_descriptors = true;

I wonder if this should be set conditionally instead. Maybe set it to 
"true" if the "num-cs" property exists?

The rest looks good to me.

Cheers,
-Paul

> +	ctlr->max_native_cs = 2;
> +	ctlr->num_chipselect = num_cs;
>  	ctlr->dev.of_node = pdev->dev.of_node;
> 
>  	if (spi_ingenic_request_dma(ctlr, dev))
> --
> 2.7.4
> 

Re: [PATCH 2/3] dt-bindings: SPI: Add bindings for new Ingenic SoCs.

[Paul Cercueil]

Le ven., avril 15 2022 at 22:22:07 +0800, 周琰杰 (Zhou Yanjie) 
<zhouyanjie@wanyeetech.com> a écrit :
> Add the SPI bindings for the JZ4775 SoC, the X1000 SoC,
> and the X2000 SoC from Ingenic.
> 
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>

Acked-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul

> ---
>  Documentation/devicetree/bindings/spi/ingenic,spi.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/ingenic,spi.yaml 
> b/Documentation/devicetree/bindings/spi/ingenic,spi.yaml
> index 5b1c7a2..360f76c 100644
> --- a/Documentation/devicetree/bindings/spi/ingenic,spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/ingenic,spi.yaml
> @@ -18,7 +18,10 @@ properties:
>      oneOf:
>        - enum:
>            - ingenic,jz4750-spi
> +          - ingenic,jz4775-spi
>            - ingenic,jz4780-spi
> +          - ingenic,x1000-spi
> +          - ingenic,x2000-spi
>        - items:
>            - enum:
>                - ingenic,jz4760-spi
> --
> 2.7.4
> 

Re: [PATCH 3/3] SPI: Ingenic: Add support for new Ingenic SoCs.

[Paul Cercueil]

Le ven., avril 15 2022 at 22:22:08 +0800, 周琰杰 (Zhou Yanjie) 
<zhouyanjie@wanyeetech.com> a écrit :
> 1.Since it would be dangerous to specify a newer SoC's compatible
>   string as the fallback of an older SoC's compatible string, we
>   add support for the "ingenic,jz4775-spi" compatible string in
>   the driver.
> 
>   This will permit to support the JZ4775 by having:
>   compatible = "ingenic,jz4775-spi";
> 
>   Instead of doing:
>   compatible = "ingenic,jz4775-spi", "ingenic,jz4780-spi";
> 
> 2.Add support for probing the spi-ingenic driver on the X1000 SoC
>   from Ingenic. From the X1000 SoC onwards, the maximum frequency
>   allowed by the SSI module of Ingenic SoCs has been changed from
>   54MHz to 50MHz. So "max_speed_hz" is introduced in "jz_soc_info"
>   to set different maximum frequency values.
> 
> 3.Add support for probing the spi-ingenic driver on the X2000 SoC
>   from Ingenic. The X2000 SoC has only one native chip select line,
>   so "max_native_cs" is introduced in "jz_soc_info" to set different
>   maximum number of native chip select lines.
> 
> 4.Because of the introduction of support for the X-series SoCs, the
>   current driver is not only applicable to the JZ-series SoCs, so
>   the description texts has been modified to avoid misunderstanding.
> 
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>

Reviewed-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul

> ---
>  drivers/spi/Kconfig       |  4 ++--
>  drivers/spi/spi-ingenic.c | 42 
> +++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 39 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index d2815eb..cca92a8 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -419,10 +419,10 @@ config SPI_IMX
>  	  This enables support for the Freescale i.MX SPI controllers.
> 
>  config SPI_INGENIC
> -	tristate "Ingenic JZ47xx SoCs SPI controller"
> +	tristate "Ingenic SoCs SPI controller"
>  	depends on MACH_INGENIC || COMPILE_TEST
>  	help
> -	  This enables support for the Ingenic JZ47xx SoCs SPI controller.
> +	  This enables support for the Ingenic SoCs SPI controller.
> 
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called spi-ingenic.
> diff --git a/drivers/spi/spi-ingenic.c b/drivers/spi/spi-ingenic.c
> index 672e4ed..ff507c8 100644
> --- a/drivers/spi/spi-ingenic.c
> +++ b/drivers/spi/spi-ingenic.c
> @@ -1,8 +1,9 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * SPI bus driver for the Ingenic JZ47xx SoCs
> + * SPI bus driver for the Ingenic SoCs
>   * Copyright (c) 2017-2021 Artur Rojek <contact@artur-rojek.eu>
>   * Copyright (c) 2017-2021 Paul Cercueil <paul@crapouillou.net>
> + * Copyright (c) 2022 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@wanyeetech.com>
>   */
> 
>  #include <linux/clk.h>
> @@ -52,6 +53,9 @@ struct jz_soc_info {
>  	u32 bits_per_word_mask;
>  	struct reg_field flen_field;
>  	bool has_trendian;
> +
> +	unsigned int max_speed_hz;
> +	unsigned int max_native_cs;
>  };
> 
>  struct ingenic_spi {
> @@ -418,7 +422,7 @@ static int spi_ingenic_probe(struct 
> platform_device *pdev)
> 
>  	if (of_property_read_u32(dev->of_node, "num-cs", &num_cs)) {
>  		dev_warn(dev, "Number of chip select lines not specified.\n");
> -		num_cs = 2;
> +		num_cs = pdata->max_native_cs;
>  	}
> 
>  	platform_set_drvdata(pdev, ctlr);
> @@ -433,9 +437,9 @@ static int spi_ingenic_probe(struct 
> platform_device *pdev)
>  	ctlr->max_dma_len = SPI_INGENIC_FIFO_SIZE;
>  	ctlr->bits_per_word_mask = pdata->bits_per_word_mask;
>  	ctlr->min_speed_hz = 7200;
> -	ctlr->max_speed_hz = 54000000;
> +	ctlr->max_speed_hz = pdata->max_speed_hz;
>  	ctlr->use_gpio_descriptors = true;
> -	ctlr->max_native_cs = 2;
> +	ctlr->max_native_cs = pdata->max_native_cs;
>  	ctlr->num_chipselect = num_cs;
>  	ctlr->dev.of_node = pdev->dev.of_node;
> 
> @@ -459,17 +463,44 @@ static const struct jz_soc_info jz4750_soc_info 
> = {
>  	.bits_per_word_mask = SPI_BPW_RANGE_MASK(2, 17),
>  	.flen_field = REG_FIELD(REG_SSICR1, 4, 7),
>  	.has_trendian = false,
> +
> +	.max_speed_hz = 54000000,
> +	.max_native_cs = 2,
>  };
> 
>  static const struct jz_soc_info jz4780_soc_info = {
>  	.bits_per_word_mask = SPI_BPW_RANGE_MASK(2, 32),
>  	.flen_field = REG_FIELD(REG_SSICR1, 3, 7),
>  	.has_trendian = true,
> +
> +	.max_speed_hz = 54000000,
> +	.max_native_cs = 2,
> +};
> +
> +static const struct jz_soc_info x1000_soc_info = {
> +	.bits_per_word_mask = SPI_BPW_RANGE_MASK(2, 32),
> +	.flen_field = REG_FIELD(REG_SSICR1, 3, 7),
> +	.has_trendian = true,
> +
> +	.max_speed_hz = 50000000,
> +	.max_native_cs = 2,
> +};
> +
> +static const struct jz_soc_info x2000_soc_info = {
> +	.bits_per_word_mask = SPI_BPW_RANGE_MASK(2, 32),
> +	.flen_field = REG_FIELD(REG_SSICR1, 3, 7),
> +	.has_trendian = true,
> +
> +	.max_speed_hz = 50000000,
> +	.max_native_cs = 1,
>  };
> 
>  static const struct of_device_id spi_ingenic_of_match[] = {
>  	{ .compatible = "ingenic,jz4750-spi", .data = &jz4750_soc_info },
> +	{ .compatible = "ingenic,jz4775-spi", .data = &jz4780_soc_info },
>  	{ .compatible = "ingenic,jz4780-spi", .data = &jz4780_soc_info },
> +	{ .compatible = "ingenic,x1000-spi", .data = &x1000_soc_info },
> +	{ .compatible = "ingenic,x2000-spi", .data = &x2000_soc_info },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, spi_ingenic_of_match);
> @@ -483,7 +514,8 @@ static struct platform_driver spi_ingenic_driver 
> = {
>  };
> 
>  module_platform_driver(spi_ingenic_driver);
> -MODULE_DESCRIPTION("SPI bus driver for the Ingenic JZ47xx SoCs");
> +MODULE_DESCRIPTION("SPI bus driver for the Ingenic SoCs");
>  MODULE_AUTHOR("Artur Rojek <contact@artur-rojek.eu>");
>  MODULE_AUTHOR("Paul Cercueil <paul@crapouillou.net>");
> +MODULE_AUTHOR("周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>");
>  MODULE_LICENSE("GPL");
> --
> 2.7.4
> 

Re: [PATCH 1/3] SPI: Ingenic: Add support for use GPIO as chip select line.

[Zhou Yanjie]

Hi Paul,

On 2022/4/15 下午11:00, Paul Cercueil wrote:
> Hi Zhou,
>
> Le ven., avril 15 2022 at 22:22:06 +0800, 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@wanyeetech.com> a écrit :
>> Add support for using GPIOs as chip select lines on Ingenic SoCs.
>>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> ---
>>  drivers/spi/spi-ingenic.c | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/spi/spi-ingenic.c b/drivers/spi/spi-ingenic.c
>> index 03077a7..672e4ed 100644
>> --- a/drivers/spi/spi-ingenic.c
>> +++ b/drivers/spi/spi-ingenic.c
>> @@ -380,7 +380,7 @@ static int spi_ingenic_probe(struct 
>> platform_device *pdev)
>>      struct spi_controller *ctlr;
>>      struct ingenic_spi *priv;
>>      void __iomem *base;
>> -    int ret;
>> +    int num_cs, ret;
>>
>>      pdata = of_device_get_match_data(dev);
>>      if (!pdata) {
>> @@ -416,6 +416,11 @@ static int spi_ingenic_probe(struct 
>> platform_device *pdev)
>>      if (IS_ERR(priv->flen_field))
>>          return PTR_ERR(priv->flen_field);
>>
>> +    if (of_property_read_u32(dev->of_node, "num-cs", &num_cs)) {
>> +        dev_warn(dev, "Number of chip select lines not specified.\n");
>> +        num_cs = 2;
>> +    }
>> +
>>      platform_set_drvdata(pdev, ctlr);
>>
>>      ctlr->prepare_transfer_hardware = spi_ingenic_prepare_hardware;
>> @@ -429,7 +434,9 @@ static int spi_ingenic_probe(struct 
>> platform_device *pdev)
>>      ctlr->bits_per_word_mask = pdata->bits_per_word_mask;
>>      ctlr->min_speed_hz = 7200;
>>      ctlr->max_speed_hz = 54000000;
>> -    ctlr->num_chipselect = 2;
>> +    ctlr->use_gpio_descriptors = true;
>
> I wonder if this should be set conditionally instead. Maybe set it to 
> "true" if the "num-cs" property exists?
>

I'm not too sure, but it seems some other drivers like "spi-sun6i.c", 
"spi-stm32.c", "spi-s3c64xx.c", "spi-pic32.c", etc. set it unconditionally.


> The rest looks good to me.
>
> Cheers,
> -Paul
>
>> +    ctlr->max_native_cs = 2;
>> +    ctlr->num_chipselect = num_cs;
>>      ctlr->dev.of_node = pdev->dev.of_node;
>>
>>      if (spi_ingenic_request_dma(ctlr, dev))
>> -- 
>> 2.7.4
>>
>

Re: [PATCH 1/3] SPI: Ingenic: Add support for use GPIO as chip select line.

[Paul Cercueil]

Hi Zhou,

Le sam., avril 16 2022 at 19:55:13 +0800, Zhou Yanjie 
<zhouyanjie@wanyeetech.com> a écrit :
> Hi Paul,
> 
> On 2022/4/15 下午11:00, Paul Cercueil wrote:
>> Hi Zhou,
>> 
>> Le ven., avril 15 2022 at 22:22:06 +0800, 周琰杰 (Zhou Yanjie) 
>> \x7f<zhouyanjie@wanyeetech.com> a écrit :
>>> Add support for using GPIOs as chip select lines on Ingenic SoCs.
>>> 
>>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>> ---
>>>  drivers/spi/spi-ingenic.c | 11 +++++++++--
>>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/drivers/spi/spi-ingenic.c b/drivers/spi/spi-ingenic.c
>>> index 03077a7..672e4ed 100644
>>> --- a/drivers/spi/spi-ingenic.c
>>> +++ b/drivers/spi/spi-ingenic.c
>>> @@ -380,7 +380,7 @@ static int spi_ingenic_probe(struct 
>>> \x7f\x7fplatform_device *pdev)
>>>      struct spi_controller *ctlr;
>>>      struct ingenic_spi *priv;
>>>      void __iomem *base;
>>> -    int ret;
>>> +    int num_cs, ret;
>>> 
>>>      pdata = of_device_get_match_data(dev);
>>>      if (!pdata) {
>>> @@ -416,6 +416,11 @@ static int spi_ingenic_probe(struct 
>>> \x7f\x7fplatform_device *pdev)
>>>      if (IS_ERR(priv->flen_field))
>>>          return PTR_ERR(priv->flen_field);
>>> 
>>> +    if (of_property_read_u32(dev->of_node, "num-cs", &num_cs)) {
>>> +        dev_warn(dev, "Number of chip select lines not 
>>> specified.\n");
>>> +        num_cs = 2;
>>> +    }
>>> +
>>>      platform_set_drvdata(pdev, ctlr);
>>> 
>>>      ctlr->prepare_transfer_hardware = spi_ingenic_prepare_hardware;
>>> @@ -429,7 +434,9 @@ static int spi_ingenic_probe(struct 
>>> \x7f\x7fplatform_device *pdev)
>>>      ctlr->bits_per_word_mask = pdata->bits_per_word_mask;
>>>      ctlr->min_speed_hz = 7200;
>>>      ctlr->max_speed_hz = 54000000;
>>> -    ctlr->num_chipselect = 2;
>>> +    ctlr->use_gpio_descriptors = true;
>> 
>> I wonder if this should be set conditionally instead. Maybe set it 
>> to \x7f"true" if the "num-cs" property exists?
>> 
> 
> I'm not too sure, but it seems some other drivers like "spi-sun6i.c", 
> "spi-stm32.c", "spi-s3c64xx.c", "spi-pic32.c", etc. set it 
> unconditionally.

Ok, maybe Mark can enlighten us here.

Cheers,
-Paul


>> The rest looks good to me.
>> 
>> Cheers,
>> -Paul
>> 
>>> +    ctlr->max_native_cs = 2;
>>> +    ctlr->num_chipselect = num_cs;
>>>      ctlr->dev.of_node = pdev->dev.of_node;
>>> 
>>>      if (spi_ingenic_request_dma(ctlr, dev))
>>> --
>>> 2.7.4
>>> 
>> 

Re: [PATCH 2/3] dt-bindings: SPI: Add bindings for new Ingenic SoCs.

[Krzysztof Kozlowski]

On 15/04/2022 16:22, 周琰杰 (Zhou Yanjie) wrote:
> Add the SPI bindings for the JZ4775 SoC, the X1000 SoC,
> and the X2000 SoC from Ingenic.
> 
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH 1/3] SPI: Ingenic: Add support for use GPIO as chip select line.

[Mark Brown]

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

On Sat, Apr 16, 2022 at 05:36:46PM +0100, Paul Cercueil wrote:
> Le sam., avril 16 2022 at 19:55:13 +0800, Zhou Yanjie
> > On 2022/4/15 下午11:00, Paul Cercueil wrote:

> > > > -    ctlr->num_chipselect = 2;
> > > > +    ctlr->use_gpio_descriptors = true;

> > > I wonder if this should be set conditionally instead. Maybe set it
> > > to \x7f"true" if the "num-cs" property exists?

> > I'm not too sure, but it seems some other drivers like "spi-sun6i.c",
> > "spi-stm32.c", "spi-s3c64xx.c", "spi-pic32.c", etc. set it
> > unconditionally.

> Ok, maybe Mark can enlighten us here.

use_gpio_descriptions is just selecting which version of the GPIO APIs
we should use if we're handling GPIOs rather than if we should handle
them.  We've got one last driver using the numerical GPIO APIs, once
that one is converted we should just be able to drop the flag since
everything will be using descriptors.

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

Re: [PATCH 1/3] SPI: Ingenic: Add support for use GPIO as chip select line.

[Paul Cercueil]

Hi,

Le mar., avril 19 2022 at 18:00:41 +0100, Mark Brown 
<broonie@kernel.org> a écrit :
> On Sat, Apr 16, 2022 at 05:36:46PM +0100, Paul Cercueil wrote:
>>  Le sam., avril 16 2022 at 19:55:13 +0800, Zhou Yanjie
>>  > On 2022/4/15 下午11:00, Paul Cercueil wrote:
> 
>>  > > > -    ctlr->num_chipselect = 2;
>>  > > > +    ctlr->use_gpio_descriptors = true;
> 
>>  > > I wonder if this should be set conditionally instead. Maybe set 
>> it
>>  > > to \x7f"true" if the "num-cs" property exists?
> 
>>  > I'm not too sure, but it seems some other drivers like 
>> "spi-sun6i.c",
>>  > "spi-stm32.c", "spi-s3c64xx.c", "spi-pic32.c", etc. set it
>>  > unconditionally.
> 
>>  Ok, maybe Mark can enlighten us here.
> 
> use_gpio_descriptions is just selecting which version of the GPIO APIs
> we should use if we're handling GPIOs rather than if we should handle
> them.  We've got one last driver using the numerical GPIO APIs, once
> that one is converted we should just be able to drop the flag since
> everything will be using descriptors.

Thank you Mark.

Cheers,
-Paul

Re: [PATCH 1/3] SPI: Ingenic: Add support for use GPIO as chip select line.

[Paul Cercueil]

Hi Zhou,

Le ven., avril 15 2022 at 22:22:06 +0800, 周琰杰 (Zhou Yanjie) 
<zhouyanjie@wanyeetech.com> a écrit :
> Add support for using GPIOs as chip select lines on Ingenic SoCs.
> 
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> ---
>  drivers/spi/spi-ingenic.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi-ingenic.c b/drivers/spi/spi-ingenic.c
> index 03077a7..672e4ed 100644
> --- a/drivers/spi/spi-ingenic.c
> +++ b/drivers/spi/spi-ingenic.c
> @@ -380,7 +380,7 @@ static int spi_ingenic_probe(struct 
> platform_device *pdev)
>  	struct spi_controller *ctlr;
>  	struct ingenic_spi *priv;
>  	void __iomem *base;
> -	int ret;
> +	int num_cs, ret;
> 
>  	pdata = of_device_get_match_data(dev);
>  	if (!pdata) {
> @@ -416,6 +416,11 @@ static int spi_ingenic_probe(struct 
> platform_device *pdev)
>  	if (IS_ERR(priv->flen_field))
>  		return PTR_ERR(priv->flen_field);
> 
> +	if (of_property_read_u32(dev->of_node, "num-cs", &num_cs)) {

One small comment here - I think it would be better to use 
device_property_read_u32().

The driver should also use device_get_match_data() instead of 
of_device_get_match_data(), but that's a cleanup that can be done later.

Cheers,
-Paul

> +		dev_warn(dev, "Number of chip select lines not specified.\n");
> +		num_cs = 2;
> +	}
> +
>  	platform_set_drvdata(pdev, ctlr);
> 
>  	ctlr->prepare_transfer_hardware = spi_ingenic_prepare_hardware;
> @@ -429,7 +434,9 @@ static int spi_ingenic_probe(struct 
> platform_device *pdev)
>  	ctlr->bits_per_word_mask = pdata->bits_per_word_mask;
>  	ctlr->min_speed_hz = 7200;
>  	ctlr->max_speed_hz = 54000000;
> -	ctlr->num_chipselect = 2;
> +	ctlr->use_gpio_descriptors = true;
> +	ctlr->max_native_cs = 2;
> +	ctlr->num_chipselect = num_cs;
>  	ctlr->dev.of_node = pdev->dev.of_node;
> 
>  	if (spi_ingenic_request_dma(ctlr, dev))
> --
> 2.7.4
> 

Re: [PATCH 1/3] SPI: Ingenic: Add support for use GPIO as chip select line.

[Zhou Yanjie]

Hi Mark,

On 2022/4/20 上午1:00, Mark Brown wrote:
> On Sat, Apr 16, 2022 at 05:36:46PM +0100, Paul Cercueil wrote:
>> Le sam., avril 16 2022 at 19:55:13 +0800, Zhou Yanjie
>>> On 2022/4/15 下午11:00, Paul Cercueil wrote:
>>>>> -    ctlr->num_chipselect = 2;
>>>>> +    ctlr->use_gpio_descriptors = true;
>>>> I wonder if this should be set conditionally instead. Maybe set it
>>>> to \x7f"true" if the "num-cs" property exists?
>>> I'm not too sure, but it seems some other drivers like "spi-sun6i.c",
>>> "spi-stm32.c", "spi-s3c64xx.c", "spi-pic32.c", etc. set it
>>> unconditionally.
>> Ok, maybe Mark can enlighten us here.
> use_gpio_descriptions is just selecting which version of the GPIO APIs
> we should use if we're handling GPIOs rather than if we should handle
> them.  We've got one last driver using the numerical GPIO APIs, once
> that one is converted we should just be able to drop the flag since
> everything will be using descriptors.


Thanks for your answer!

Re: [PATCH 1/3] SPI: Ingenic: Add support for use GPIO as chip select line.

[Zhou Yanjie]

Hi Paul,

On 2022/4/20 上午1:13, Paul Cercueil wrote:
> Hi Zhou,
>
> Le ven., avril 15 2022 at 22:22:06 +0800, 周琰杰 (Zhou Yanjie) 
> <zhouyanjie@wanyeetech.com> a écrit :
>> Add support for using GPIOs as chip select lines on Ingenic SoCs.
>>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> ---
>>  drivers/spi/spi-ingenic.c | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/spi/spi-ingenic.c b/drivers/spi/spi-ingenic.c
>> index 03077a7..672e4ed 100644
>> --- a/drivers/spi/spi-ingenic.c
>> +++ b/drivers/spi/spi-ingenic.c
>> @@ -380,7 +380,7 @@ static int spi_ingenic_probe(struct 
>> platform_device *pdev)
>>      struct spi_controller *ctlr;
>>      struct ingenic_spi *priv;
>>      void __iomem *base;
>> -    int ret;
>> +    int num_cs, ret;
>>
>>      pdata = of_device_get_match_data(dev);
>>      if (!pdata) {
>> @@ -416,6 +416,11 @@ static int spi_ingenic_probe(struct 
>> platform_device *pdev)
>>      if (IS_ERR(priv->flen_field))
>>          return PTR_ERR(priv->flen_field);
>>
>> +    if (of_property_read_u32(dev->of_node, "num-cs", &num_cs)) {
>
> One small comment here - I think it would be better to use 
> device_property_read_u32().
>
> The driver should also use device_get_match_data() instead of 
> of_device_get_match_data(), but that's a cleanup that can be done later.
>

Sure, I will send v2.


Thanks and best regards!


> Cheers,
> -Paul
>
>> +        dev_warn(dev, "Number of chip select lines not specified.\n");
>> +        num_cs = 2;
>> +    }
>> +
>>      platform_set_drvdata(pdev, ctlr);
>>
>>      ctlr->prepare_transfer_hardware = spi_ingenic_prepare_hardware;
>> @@ -429,7 +434,9 @@ static int spi_ingenic_probe(struct 
>> platform_device *pdev)
>>      ctlr->bits_per_word_mask = pdata->bits_per_word_mask;
>>      ctlr->min_speed_hz = 7200;
>>      ctlr->max_speed_hz = 54000000;
>> -    ctlr->num_chipselect = 2;
>> +    ctlr->use_gpio_descriptors = true;
>> +    ctlr->max_native_cs = 2;
>> +    ctlr->num_chipselect = num_cs;
>>      ctlr->dev.of_node = pdev->dev.of_node;
>>
>>      if (spi_ingenic_request_dma(ctlr, dev))
>> -- 
>> 2.7.4
>>
>