linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v12 0/2] spi: loongson: add bus driver for the loongson spi
@ 2023-06-08  7:28 Yinbo Zhu
  2023-06-08  7:28 ` [PATCH v12 1/2] spi: add loongson spi bindings Yinbo Zhu
  2023-06-08  7:28 ` [PATCH v12 2/2] spi: loongson: add bus driver for the loongson spi controller Yinbo Zhu
  0 siblings, 2 replies; 29+ messages in thread
From: Yinbo Zhu @ 2023-06-08  7:28 UTC (permalink / raw)
  To: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi,
	devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, Yinbo Zhu

Loongson platform support spi hardware controller and this series patch
was to add spi driver and binding support.

Change in v2:
		1. This [PATCH v2 1/2] dt-bindings patch need depend on clk patch:
	 	   https://
		   lore.kernel.org/all/20230307115022.12846-1-zhuyinbo@loongson.cn/
		2. Remove the clock-names in spi yaml file.
		3. Add "loongson,ls7a-spi" compatible in spi yaml file.
		4. Add an || COMPILE_TEST and drop && PCI then add some CONFIG_PCI
		   macro to limit some pci code.
		5. Make the spi driver top code comment block that use C++ style.
		6. Drop spi->max_speed_hz.
		7. Add a spin_lock for loongson_spi_setup.
		8. Add a timeout and cpu_relax() in loongson_spi_write_read_8bit.
		9. Add spi_transfer_one and drop transfer and rework entire spi
		   driver that include some necessary changes.
		10. Use module_init replace subsys_initcall.
		11. About PM interface that I don't find any issue so I don't add
		    any changes.
Change in v3:
		1. This [PATCH v3 1/2] dt-bindings patch need depend on clk patch:
		   https://
		   lore.kernel.org/all/20230323025229.2971-1-zhuyinbo@loongson.cn/
		2. Drop the unused blank line in loongson,ls-spi.yaml file.
		3. Replace clock minItems with clock maxItems in yaml file.
		4. Separate spi driver into platform module, pci module and core
		   module.
		5. Replace DIV_ROUND_UP with DIV_ROUND_UP_ULL to fix compile error
		   "undefined reference to `__aeabi_uldivmod'" and  "__udivdi3 undefined"
		   that reported by test robot.
		6. Remove the spin lock.
		7. Clear the loongson_spi->hz and loongson_spi->mode in setup to fixup
		   the issue that multiple spi device transfer that maybe cause spi was
		   be misconfigured.
Change in v4:
		1. This [PATCH v4 1/2] dt-bindings patch need depend on clk patch:
		   https://
		   lore.kernel.org/all/20230323025229.2971-1-zhuyinbo@loongson.cn/
		2. Add "#include <linux/io.h>" in spi-loongson-core.c for fix the compile
		   issue which devm_ioremap no declaration.
		3. Add "EXPORT_SYMBOL_GPL(loongson_spi_dev_pm_ops)" in
		   spi-loongson-core.c for fix the compile issue which
		   loongson_spi_dev_pm_ops undefined.
Change in v5:
		1. Get rid of the clock patch's dependency and open-code the clock IDs.
		2. Fixup checkpatch issue that by installed ply and gitpython package
		   locally, but this series of patch's code doesn't have any change.
Change in v6:
		1. Remove the "#include <dt-bindings/clock/loongson,ls2k-clk.h>" in
		   yaml file.
Change in v7:
		1. Remove the "loongson,ls7a-spi" and change yaml file name as
		   "loongson,ls2k-spi.yaml".
		2. Use module_pci_driver and module_platform_driver to replace
		   module_init and module_exit.
		3. Drop ".owner	= THIS_MODULE" in spi platform driver.
		4. Add devm_spi_alloc_master devm_spi_register_master to simplify code.
		5. Add pci_disable_device() in loongson_spi_pci_unregister.
Change in v8:
		1. Add reviewed-by information for spi bindings patch.
		2. Fixup the uncorrect spi yaml file path in MAINTAINERS file.
		3. Add spi_master_suspend and spi_master_resume in spi pm function.
Change in v9:
		1. Make spi_master_suspend go first in pm suspend.
Change in v10:
		1. Fix the compile issue about of_node_get and of_get_property no
		   declaration.
		2. set config SPI_LOONGSON_CORE invisible.
		3. Captial "spi" in commit log and Kconfig file.
		4. Write header files in alphabetical order.
		5. Use clamp_val, GENMASK() and BIT() in spi clock setting.
		6. Optimize clock and mode setting code.
		7. Use readb_poll_timeout in loongson_spi_write_read_8bit.
		8. Remove some useless dmesg print.
		9. Use device_set_node replace of_node_get.
		10. Use dev_err_probe in code.
		11. Use devm_clk_get_optional replace devm_clk_get.
		12. Remove SPI_NO_CS for drop 2k500 non common type spi.
		13. Use pcim_enable_device() and pcim_iomap_regions() in spi pci
		    driver.
		14. Passing the remapped address in loongson_spi_init_master.
		15. Remove the useless goto flag "err_out".
		16. Use pci vendor id in pci_ids.h.
		17. Use devm_platform_ioremap_resource in spi platform driver.
		18. Remove the useless item in pci_device_id.
		19. Remove the inned comma in of_device_id.
		20. Add some headfile in spi_loongson.h.
		21. Remove the useless extern for loongson_spi_init_master in
		    spi_loongson.h.
Change in v11:
		1. Use spi_get_chipselect() to replace all spi->chip_select in
		   spi driver
Change in v12:
		1. Reword the dt-bindings patch title.
		2. Use a specific spi compatible in dt-bindings and spi driver.
		3. Add Cc list for the reviewers of the previous version.
		4. Add a static for rdiv[12] array in loongson_spi_set_clk.
		5. Fixup the compile warning for spi HZ that reported by robot.
		6. Use "#define LOONGSON_... BIT(0)" in readb_poll_timeout.
		7. Add a error code return that when write spi failed.
		8. Use spi_controller* instead of spi_master* in all cases.
		9. Check for the error first which for clock gain.
		10. Drop the ->remove() in spi pci driver.
		11. Drop the comma for the terminator entry in pci_device_id.
		12. Adjust the head file in spi driver.
		13. Use forward declarations for device and spi_controller.

Yinbo Zhu (2):
  spi: add loongson spi bindings
  spi: loongson: add bus driver for the loongson spi controller

 .../bindings/spi/loongson,ls2k-spi.yaml       |  41 +++
 MAINTAINERS                                   |  10 +
 drivers/spi/Kconfig                           |  26 ++
 drivers/spi/Makefile                          |   3 +
 drivers/spi/spi-loongson-core.c               | 275 ++++++++++++++++++
 drivers/spi/spi-loongson-pci.c                |  55 ++++
 drivers/spi/spi-loongson-plat.c               |  47 +++
 drivers/spi/spi-loongson.h                    |  49 ++++
 8 files changed, 506 insertions(+)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-08  7:28 [PATCH v12 0/2] spi: loongson: add bus driver for the loongson spi Yinbo Zhu
@ 2023-06-08  7:28 ` Yinbo Zhu
  2023-06-08  7:45   ` Krzysztof Kozlowski
  2023-06-08  7:28 ` [PATCH v12 2/2] spi: loongson: add bus driver for the loongson spi controller Yinbo Zhu
  1 sibling, 1 reply; 29+ messages in thread
From: Yinbo Zhu @ 2023-06-08  7:28 UTC (permalink / raw)
  To: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi,
	devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel,
	Yinbo Zhu, Krzysztof Kozlowski

Add the Loongson platform spi binding with DT schema format using
json-schema.

Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/spi/loongson,ls2k-spi.yaml       | 41 +++++++++++++++++++
 MAINTAINERS                                   |  6 +++
 2 files changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml

diff --git a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
new file mode 100644
index 000000000000..423ee851edd5
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/loongson,ls2k-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Loongson SPI controller
+
+maintainers:
+  - Yinbo Zhu <zhuyinbo@loongson.cn>
+
+allOf:
+  - $ref: /schemas/spi/spi-controller.yaml#
+
+properties:
+  compatible:
+    enum:
+      - loongson,ls2k1000-spi
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi0: spi@1fff0220{
+        compatible = "loongson,ls2k1000-spi";
+        reg = <0x1fff0220 0x10>;
+        clocks = <&clk 17>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index bc201627c2e0..5e604dddd87b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12186,6 +12186,12 @@ F:	Documentation/devicetree/bindings/clock/loongson,ls2k-clk.yaml
 F:	drivers/clk/clk-loongson2.c
 F:	include/dt-bindings/clock/loongson,ls2k-clk.h
 
+LOONGSON SPI DRIVER
+M:	Yinbo Zhu <zhuyinbo@loongson.cn>
+L:	linux-spi@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
+
 LOONGSON-2 SOC SERIES GUTS DRIVER
 M:	Yinbo Zhu <zhuyinbo@loongson.cn>
 L:	loongarch@lists.linux.dev
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH v12 2/2] spi: loongson: add bus driver for the loongson spi controller
  2023-06-08  7:28 [PATCH v12 0/2] spi: loongson: add bus driver for the loongson spi Yinbo Zhu
  2023-06-08  7:28 ` [PATCH v12 1/2] spi: add loongson spi bindings Yinbo Zhu
@ 2023-06-08  7:28 ` Yinbo Zhu
  2023-06-08 10:15   ` Andy Shevchenko
  1 sibling, 1 reply; 29+ messages in thread
From: Yinbo Zhu @ 2023-06-08  7:28 UTC (permalink / raw)
  To: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi,
	devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel,
	Yinbo Zhu, Andy Shevchenko

This bus driver supports the Loongson SPI hardware controller in the
Loongson platforms and supports to use DTS and PCI framework to
register SPI device resources.

Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
---
 MAINTAINERS                     |   4 +
 drivers/spi/Kconfig             |  26 +++
 drivers/spi/Makefile            |   3 +
 drivers/spi/spi-loongson-core.c | 275 ++++++++++++++++++++++++++++++++
 drivers/spi/spi-loongson-pci.c  |  55 +++++++
 drivers/spi/spi-loongson-plat.c |  47 ++++++
 drivers/spi/spi-loongson.h      |  49 ++++++
 7 files changed, 459 insertions(+)
 create mode 100644 drivers/spi/spi-loongson-core.c
 create mode 100644 drivers/spi/spi-loongson-pci.c
 create mode 100644 drivers/spi/spi-loongson-plat.c
 create mode 100644 drivers/spi/spi-loongson.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 5e604dddd87b..69cb8fb2a0e1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12191,6 +12191,10 @@ M:	Yinbo Zhu <zhuyinbo@loongson.cn>
 L:	linux-spi@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
+F:	drivers/spi/spi-loongson-core.c
+F:	drivers/spi/spi-loongson-pci.c
+F:	drivers/spi/spi-loongson-plat.c
+F:	drivers/spi/spi-loongson.h
 
 LOONGSON-2 SOC SERIES GUTS DRIVER
 M:	Yinbo Zhu <zhuyinbo@loongson.cn>
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 3de2ebe8294a..6b953904792e 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -516,6 +516,32 @@ config SPI_LM70_LLP
 	  which interfaces to an LM70 temperature sensor using
 	  a parallel port.
 
+config SPI_LOONGSON_CORE
+	tristate
+	depends on LOONGARCH || COMPILE_TEST
+
+config SPI_LOONGSON_PCI
+	tristate "Loongson SPI Controller PCI Driver Support"
+	select SPI_LOONGSON_CORE
+	depends on PCI && (LOONGARCH || COMPILE_TEST)
+	help
+	  This bus driver supports the Loongson SPI hardware controller in
+	  the Loongson platforms and supports to use PCI framework to
+	  register SPI device resources.
+	  Say Y or M here if you want to use the SPI controller on
+	  Loongson platform.
+
+config SPI_LOONGSON_PLATFORM
+	tristate "Loongson SPI Controller Platform Driver Support"
+	select SPI_LOONGSON_CORE
+	depends on OF && (LOONGARCH || COMPILE_TEST)
+	help
+	  This bus driver supports the Loongson SPI hardware controller in
+	  the Loongson platforms and supports to use DTS framework to
+	  register SPI device resources.
+	  Say Y or M here if you want to use the SPI controller on
+	  Loongson platform.
+
 config SPI_LP8841_RTC
 	tristate "ICP DAS LP-8841 SPI Controller for RTC"
 	depends on MACH_PXA27X_DT || COMPILE_TEST
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 28c4817a8a74..3e933064d237 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -71,6 +71,9 @@ obj-$(CONFIG_SPI_INTEL_PLATFORM)	+= spi-intel-platform.o
 obj-$(CONFIG_SPI_LANTIQ_SSC)		+= spi-lantiq-ssc.o
 obj-$(CONFIG_SPI_JCORE)			+= spi-jcore.o
 obj-$(CONFIG_SPI_LM70_LLP)		+= spi-lm70llp.o
+obj-$(CONFIG_SPI_LOONGSON_CORE)		+= spi-loongson-core.o
+obj-$(CONFIG_SPI_LOONGSON_PCI)		+= spi-loongson-pci.o
+obj-$(CONFIG_SPI_LOONGSON_PLATFORM)	+= spi-loongson-plat.o
 obj-$(CONFIG_SPI_LP8841_RTC)		+= spi-lp8841-rtc.o
 obj-$(CONFIG_SPI_MESON_SPICC)		+= spi-meson-spicc.o
 obj-$(CONFIG_SPI_MESON_SPIFC)		+= spi-meson-spifc.o
diff --git a/drivers/spi/spi-loongson-core.c b/drivers/spi/spi-loongson-core.c
new file mode 100644
index 000000000000..403b735bd0cf
--- /dev/null
+++ b/drivers/spi/spi-loongson-core.c
@@ -0,0 +1,275 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Loongson SPI Support
+// Copyright (C) 2023 Loongson Technology Corporation Limited
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+
+#include "spi-loongson.h"
+
+static inline void loongson_spi_write_reg(struct loongson_spi *spi, unsigned char reg,
+					  unsigned char data)
+{
+	writeb(data, spi->base + reg);
+}
+
+static inline char loongson_spi_read_reg(struct loongson_spi *spi, unsigned char reg)
+{
+	return readb(spi->base + reg);
+}
+
+static void loongson_spi_set_cs(struct spi_device *spi, bool val)
+{
+	int cs;
+	struct loongson_spi *loongson_spi = spi_controller_get_devdata(spi->controller);
+
+	cs = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SFCS_REG)
+					   & ~(0x11 << spi_get_chipselect(spi, 0));
+	loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SFCS_REG,
+				       (val ? (0x11 << spi_get_chipselect(spi, 0)) :
+				       (0x1 << spi_get_chipselect(spi, 0))) | cs);
+}
+
+static void loongson_spi_set_clk(struct loongson_spi *loongson_spi, unsigned int hz)
+{
+	unsigned char val;
+	unsigned int div, div_tmp;
+	static const char rdiv[12] = {0, 1, 4, 2, 3, 5, 6, 7, 8, 9, 10, 11};
+
+	div = clamp_val(DIV_ROUND_UP_ULL(loongson_spi->clk_rate, hz), 2, 4096);
+	div_tmp = rdiv[fls(div - 1)];
+	loongson_spi->spcr = (div_tmp & GENMASK(1, 0)) >> 0;
+	loongson_spi->sper = (div_tmp & GENMASK(3, 2)) >> 2;
+	val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPCR_REG);
+	loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, (val & ~3) |
+			       loongson_spi->spcr);
+	val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPER_REG);
+	loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPER_REG, (val & ~3) |
+			       loongson_spi->sper);
+	loongson_spi->hz = hz;
+}
+
+static void loongson_spi_set_mode(struct loongson_spi *loongson_spi,
+				  struct spi_device *spi)
+{
+	unsigned char val;
+
+	val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPCR_REG);
+	val &= ~(LOONGSON_SPI_SPCR_CPOL | LOONGSON_SPI_SPCR_CPHA);
+	if (spi->mode & SPI_CPOL)
+		val |= LOONGSON_SPI_SPCR_CPOL;
+	if (spi->mode & SPI_CPHA)
+		val |= LOONGSON_SPI_SPCR_CPHA;
+
+	loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, val);
+	loongson_spi->mode |= spi->mode;
+}
+
+static int loongson_spi_update_state(struct loongson_spi *loongson_spi,
+				struct spi_device *spi, struct spi_transfer *t)
+{
+	if (t && loongson_spi->hz != t->speed_hz)
+		loongson_spi_set_clk(loongson_spi, t->speed_hz);
+
+	if ((spi->mode ^ loongson_spi->mode) & SPI_MODE_X_MASK)
+		loongson_spi_set_mode(loongson_spi, spi);
+
+	return 0;
+}
+
+static int loongson_spi_setup(struct spi_device *spi)
+{
+	struct loongson_spi *loongson_spi;
+
+	loongson_spi = spi_controller_get_devdata(spi->controller);
+	if (spi->bits_per_word % 8)
+		return -EINVAL;
+
+	if (spi_get_chipselect(spi, 0) >= spi->controller->num_chipselect)
+		return -EINVAL;
+
+	loongson_spi->hz = 0;
+	loongson_spi_set_cs(spi, 1);
+
+	return 0;
+}
+
+static int loongson_spi_write_read_8bit(struct spi_device *spi, const u8 **tx_buf,
+					u8 **rx_buf, unsigned int num)
+{
+	int ret;
+	struct loongson_spi *loongson_spi = spi_controller_get_devdata(spi->controller);
+
+	if (tx_buf && *tx_buf)
+		loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_FIFO_REG, *((*tx_buf)++));
+	else
+		loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_FIFO_REG, 0);
+	ret = readb_poll_timeout(loongson_spi->base + LOONGSON_SPI_SPSR_REG, loongson_spi->spsr,
+			(loongson_spi->spsr & 0x1) != LOONGSON_SPI_SPSR_RFEMPTY, 1, MSEC_PER_SEC);
+
+	if (rx_buf && *rx_buf)
+		*(*rx_buf)++ = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_FIFO_REG);
+	else
+		loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_FIFO_REG);
+
+	return ret;
+}
+
+static int loongson_spi_write_read(struct spi_device *spi, struct spi_transfer *xfer)
+{
+	int ret;
+	unsigned int count;
+	const u8 *tx = xfer->tx_buf;
+	u8 *rx = xfer->rx_buf;
+
+	count = xfer->len;
+
+	do {
+		ret = loongson_spi_write_read_8bit(spi, &tx, &rx, count);
+		if (ret)
+			break;
+	} while (--count);
+
+	return ret;
+}
+
+static int loongson_spi_prepare_message(struct spi_controller *ctlr, struct spi_message *m)
+{
+	struct loongson_spi *loongson_spi = spi_controller_get_devdata(ctlr);
+
+	loongson_spi->para = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_PARA_REG);
+	loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_PARA_REG, loongson_spi->para & ~1);
+
+	return 0;
+}
+
+static int loongson_spi_transfer_one(struct spi_controller *ctrl, struct spi_device *spi,
+				     struct spi_transfer *xfer)
+{
+	struct loongson_spi *loongson_spi = spi_controller_get_devdata(spi->controller);
+
+	loongson_spi_update_state(loongson_spi, spi, xfer);
+	if (xfer->len)
+		return loongson_spi_write_read(spi, xfer);
+
+	return 0;
+}
+
+static int loongson_spi_unprepare_message(struct spi_controller *ctrl, struct spi_message *m)
+{
+	struct loongson_spi *loongson_spi = spi_controller_get_devdata(ctrl);
+
+	loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_PARA_REG, loongson_spi->para);
+
+	return 0;
+}
+
+static void loongson_spi_reginit(struct loongson_spi *loongson_spi_dev)
+{
+	unsigned char val;
+
+	val = loongson_spi_read_reg(loongson_spi_dev, LOONGSON_SPI_SPCR_REG);
+	val &= ~LOONGSON_SPI_SPCR_SPE;
+	loongson_spi_write_reg(loongson_spi_dev, LOONGSON_SPI_SPCR_REG, val);
+
+	loongson_spi_write_reg(loongson_spi_dev, LOONGSON_SPI_SPSR_REG,
+			       (LOONGSON_SPI_SPSR_SPIF | LOONGSON_SPI_SPSR_WCOL));
+
+	val = loongson_spi_read_reg(loongson_spi_dev, LOONGSON_SPI_SPCR_REG);
+	val |= LOONGSON_SPI_SPCR_SPE;
+	loongson_spi_write_reg(loongson_spi_dev, LOONGSON_SPI_SPCR_REG, val);
+}
+
+int loongson_spi_init_controller(struct device *dev, void __iomem *regs)
+{
+	struct spi_controller *controller;
+	struct loongson_spi *spi;
+	struct clk *clk;
+
+	controller = devm_spi_alloc_host(dev, sizeof(struct loongson_spi));
+	if (controller == NULL)
+		return -ENOMEM;
+
+	controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
+	controller->setup = loongson_spi_setup;
+	controller->prepare_message = loongson_spi_prepare_message;
+	controller->transfer_one = loongson_spi_transfer_one;
+	controller->unprepare_message = loongson_spi_unprepare_message;
+	controller->set_cs = loongson_spi_set_cs;
+	controller->num_chipselect = 4;
+	device_set_node(&controller->dev, dev_fwnode(dev));
+	dev_set_drvdata(dev, controller);
+
+	spi = spi_controller_get_devdata(controller);
+	spi->base = regs;
+	spi->controller = controller;
+
+	clk = devm_clk_get_optional(dev, NULL);
+	if (IS_ERR(clk))
+		return dev_err_probe(dev, PTR_ERR(clk), "unable to get clock\n");
+
+	spi->clk_rate = clk_get_rate(clk);
+	loongson_spi_reginit(spi);
+
+	spi->mode = 0;
+
+	return devm_spi_register_controller(dev, controller);
+}
+EXPORT_SYMBOL_NS_GPL(loongson_spi_init_controller, SPI_LOONGSON_CORE);
+
+static int __maybe_unused loongson_spi_suspend(struct device *dev)
+{
+	struct loongson_spi *loongson_spi;
+	struct spi_controller *controller;
+
+	controller = dev_get_drvdata(dev);
+	spi_controller_suspend(controller);
+
+	loongson_spi = spi_controller_get_devdata(controller);
+
+	loongson_spi->spcr = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPCR_REG);
+	loongson_spi->sper = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPER_REG);
+	loongson_spi->spsr = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPSR_REG);
+	loongson_spi->para = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_PARA_REG);
+	loongson_spi->sfcs = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SFCS_REG);
+	loongson_spi->timi = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_TIMI_REG);
+
+	return 0;
+}
+
+static int __maybe_unused loongson_spi_resume(struct device *dev)
+{
+	struct loongson_spi *loongson_spi;
+	struct spi_controller *controller;
+
+	controller = dev_get_drvdata(dev);
+	loongson_spi = spi_controller_get_devdata(controller);
+
+	loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, loongson_spi->spcr);
+	loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPER_REG, loongson_spi->sper);
+	loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPSR_REG, loongson_spi->spsr);
+	loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_PARA_REG, loongson_spi->para);
+	loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SFCS_REG, loongson_spi->sfcs);
+	loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_TIMI_REG, loongson_spi->timi);
+
+	spi_controller_resume(controller);
+
+	return 0;
+}
+
+const struct dev_pm_ops loongson_spi_dev_pm_ops = {
+	.suspend = loongson_spi_suspend,
+	.resume = loongson_spi_resume,
+};
+EXPORT_SYMBOL_NS_GPL(loongson_spi_dev_pm_ops, SPI_LOONGSON_CORE);
+
+MODULE_DESCRIPTION("Loongson SPI core driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/spi/spi-loongson-pci.c b/drivers/spi/spi-loongson-pci.c
new file mode 100644
index 000000000000..134cda0c13a5
--- /dev/null
+++ b/drivers/spi/spi-loongson-pci.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0+
+// PCI interface driver for Loongson SPI Support
+// Copyright (C) 2023 Loongson Technology Corporation Limited
+
+#include <linux/mod_devicetable.h>
+#include <linux/pci.h>
+
+#include "spi-loongson.h"
+
+static int loongson_spi_pci_register(struct pci_dev *pdev,
+			const struct pci_device_id *ent)
+{
+	int ret;
+	void __iomem *reg_base;
+	struct device *dev = &pdev->dev;
+	int pci_bar = 0;
+
+	ret = pcim_enable_device(pdev);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "cannot enable pci device\n");
+
+	ret = pcim_iomap_regions(pdev, BIT(pci_bar), pci_name(pdev));
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to request and remap memory\n");
+
+	reg_base = pcim_iomap_table(pdev)[pci_bar];
+
+	ret = loongson_spi_init_controller(dev, reg_base);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to initialize controller\n");
+
+	return 0;
+}
+
+static struct pci_device_id loongson_spi_devices[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x7a0b) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x7a1b) },
+	{ }
+};
+MODULE_DEVICE_TABLE(pci, loongson_spi_devices);
+
+static struct pci_driver loongson_spi_pci_driver = {
+	.name       = "loongson-spi-pci",
+	.id_table   = loongson_spi_devices,
+	.probe      = loongson_spi_pci_register,
+	.driver	= {
+		.bus = &pci_bus_type,
+		.pm = &loongson_spi_dev_pm_ops,
+	},
+};
+module_pci_driver(loongson_spi_pci_driver);
+
+MODULE_DESCRIPTION("Loongson spi pci driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(SPI_LOONGSON_CORE);
diff --git a/drivers/spi/spi-loongson-plat.c b/drivers/spi/spi-loongson-plat.c
new file mode 100644
index 000000000000..c066e5f5891e
--- /dev/null
+++ b/drivers/spi/spi-loongson-plat.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Platform driver for Loongson SPI Support
+// Copyright (C) 2023 Loongson Technology Corporation Limited
+
+#include <linux/err.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+
+#include "spi-loongson.h"
+
+static int loongson_spi_platform_probe(struct platform_device *pdev)
+{
+	int ret;
+	void __iomem *reg_base;
+	struct device *dev = &pdev->dev;
+
+	reg_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(reg_base))
+		return PTR_ERR(reg_base);
+
+	ret = loongson_spi_init_controller(dev, reg_base);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to initialize controller\n");
+
+	return 0;
+}
+
+static const struct of_device_id loongson_spi_id_table[] = {
+	{ .compatible = "loongson,ls2k1000-spi" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, loongson_spi_id_table);
+
+static struct platform_driver loongson_spi_plat_driver = {
+	.probe = loongson_spi_platform_probe,
+	.driver	= {
+		.name	= "loongson-spi",
+		.bus = &platform_bus_type,
+		.pm = &loongson_spi_dev_pm_ops,
+		.of_match_table = loongson_spi_id_table,
+	},
+};
+module_platform_driver(loongson_spi_plat_driver);
+
+MODULE_DESCRIPTION("Loongson spi platform driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(SPI_LOONGSON_CORE);
diff --git a/drivers/spi/spi-loongson.h b/drivers/spi/spi-loongson.h
new file mode 100644
index 000000000000..35f95b161842
--- /dev/null
+++ b/drivers/spi/spi-loongson.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Header File for Loongson SPI Driver. */
+/* Copyright (C) 2023 Loongson Technology Corporation Limited */
+
+#ifndef __LINUX_SPI_LOONGSON_H
+#define __LINUX_SPI_LOONGSON_H
+
+#include <linux/bits.h>
+#include <linux/pm.h>
+#include <linux/types.h>
+
+#define	LOONGSON_SPI_SPCR_REG	0x00
+#define	LOONGSON_SPI_SPSR_REG	0x01
+#define	LOONGSON_SPI_FIFO_REG	0x02
+#define	LOONGSON_SPI_SPER_REG	0x03
+#define	LOONGSON_SPI_PARA_REG	0x04
+#define	LOONGSON_SPI_SFCS_REG	0x05
+#define	LOONGSON_SPI_TIMI_REG	0x06
+
+/* Bits definition for Loongson SPI register */
+#define	LOONGSON_SPI_PARA_MEM_EN	BIT(0)
+#define	LOONGSON_SPI_SPCR_CPHA	BIT(2)
+#define	LOONGSON_SPI_SPCR_CPOL	BIT(3)
+#define	LOONGSON_SPI_SPCR_SPE	BIT(6)
+#define	LOONGSON_SPI_SPSR_RFEMPTY	BIT(0)
+#define	LOONGSON_SPI_SPSR_WCOL	BIT(6)
+#define	LOONGSON_SPI_SPSR_SPIF	BIT(7)
+
+struct device;
+struct spi_controller;
+
+struct loongson_spi {
+	struct	spi_controller	*controller;
+	void __iomem		*base;
+	int			cs_active;
+	unsigned int		hz;
+	unsigned char		spcr;
+	unsigned char		sper;
+	unsigned char		spsr;
+	unsigned char		para;
+	unsigned char		sfcs;
+	unsigned char		timi;
+	unsigned int		mode;
+	u64			clk_rate;
+};
+
+int loongson_spi_init_controller(struct device *dev, void __iomem *reg);
+extern const struct dev_pm_ops loongson_spi_dev_pm_ops;
+#endif /* __LINUX_SPI_LOONGSON_H */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-08  7:28 ` [PATCH v12 1/2] spi: add loongson spi bindings Yinbo Zhu
@ 2023-06-08  7:45   ` Krzysztof Kozlowski
  2023-06-08  8:39     ` zhuyinbo
  0 siblings, 1 reply; 29+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-08  7:45 UTC (permalink / raw)
  To: Yinbo Zhu, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel

On 08/06/2023 09:28, Yinbo Zhu wrote:
> Add the Loongson platform spi binding with DT schema format using
> json-schema.

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching.

The prefix for SPI should be: "spi: dt-bindings: ". In the same time
last "bindings" are redundant.

> 
> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../bindings/spi/loongson,ls2k-spi.yaml       | 41 +++++++++++++++++++
>  MAINTAINERS                                   |  6 +++
>  2 files changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
> new file mode 100644
> index 000000000000..423ee851edd5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml

Filename based on compatible.

> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/loongson,ls2k-spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Loongson SPI controller
> +
> +maintainers:
> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-controller.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - loongson,ls2k1000-spi

No compatibles for other devices? Didn't we have big discussion about this?

https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-08  7:45   ` Krzysztof Kozlowski
@ 2023-06-08  8:39     ` zhuyinbo
  2023-06-08  8:53       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 29+ messages in thread
From: zhuyinbo @ 2023-06-08  8:39 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo



在 2023/6/8 下午3:45, Krzysztof Kozlowski 写道:
> On 08/06/2023 09:28, Yinbo Zhu wrote:
>> Add the Loongson platform spi binding with DT schema format using
>> json-schema.
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching.
> 
> The prefix for SPI should be: "spi: dt-bindings: ". In the same time
> last "bindings" are redundant.


okay, I got it.

> 
>>
>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>   .../bindings/spi/loongson,ls2k-spi.yaml       | 41 +++++++++++++++++++
>>   MAINTAINERS                                   |  6 +++
>>   2 files changed, 47 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>> new file mode 100644
>> index 000000000000..423ee851edd5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
> 
> Filename based on compatible.


There will be more ls2k series SoC spi device in the future thus I still
use "loongson,ls2k-spi.yaml" for cover it.

> 
>> @@ -0,0 +1,41 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/spi/loongson,ls2k-spi.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Loongson SPI controller
>> +
>> +maintainers:
>> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
>> +
>> +allOf:
>> +  - $ref: /schemas/spi/spi-controller.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - loongson,ls2k1000-spi
> 
> No compatibles for other devices? Didn't we have big discussion about this?
> 
> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42


There are other ls2k SPI devices compatible, such as,
"loongson,ls2k0500-spi", "loongson,ls2k2000-spi" but currently I plan to
add ls2k1000 spi device first, Other ls2k SoC spi device adaptation may
require some additional work and I will add it later.

Thanks


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-08  8:39     ` zhuyinbo
@ 2023-06-08  8:53       ` Krzysztof Kozlowski
  2023-06-08 10:00         ` zhuyinbo
  0 siblings, 1 reply; 29+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-08  8:53 UTC (permalink / raw)
  To: zhuyinbo, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel

On 08/06/2023 10:39, zhuyinbo wrote:
>>>
>>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> ---
>>>   .../bindings/spi/loongson,ls2k-spi.yaml       | 41 +++++++++++++++++++
>>>   MAINTAINERS                                   |  6 +++
>>>   2 files changed, 47 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>> new file mode 100644
>>> index 000000000000..423ee851edd5
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>
>> Filename based on compatible.
> 
> 
> There will be more ls2k series SoC spi device in the future thus I still
> use "loongson,ls2k-spi.yaml" for cover it.

Add them now.

> 
>>
>>> @@ -0,0 +1,41 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/spi/loongson,ls2k-spi.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Loongson SPI controller
>>> +
>>> +maintainers:
>>> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
>>> +
>>> +allOf:
>>> +  - $ref: /schemas/spi/spi-controller.yaml#
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - loongson,ls2k1000-spi
>>
>> No compatibles for other devices? Didn't we have big discussion about this?
>>
>> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42
> 
> 
> There are other ls2k SPI devices compatible, such as,
> "loongson,ls2k0500-spi", "loongson,ls2k2000-spi" but currently I plan to
> add ls2k1000 spi device first, Other ls2k SoC spi device adaptation may
> require some additional work and I will add it later.

Previously you claimed this serves entire family, so I don't understand
why you need to fix something. Why previously it was working for entire
family but now it does not?

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-08  8:53       ` Krzysztof Kozlowski
@ 2023-06-08 10:00         ` zhuyinbo
  2023-06-08 10:02           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 29+ messages in thread
From: zhuyinbo @ 2023-06-08 10:00 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo



在 2023/6/8 下午4:53, Krzysztof Kozlowski 写道:
> On 08/06/2023 10:39, zhuyinbo wrote:
>>>>
>>>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> ---
>>>>    .../bindings/spi/loongson,ls2k-spi.yaml       | 41 +++++++++++++++++++
>>>>    MAINTAINERS                                   |  6 +++
>>>>    2 files changed, 47 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>> new file mode 100644
>>>> index 000000000000..423ee851edd5
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>
>>> Filename based on compatible.
>>
>>
>> There will be more ls2k series SoC spi device in the future thus I still
>> use "loongson,ls2k-spi.yaml" for cover it.
> 
> Add them now.


The 2k0500 doesn't support CCF and not use CCF to gain clock and We
internally tend to prioritize supporting 2k1000.

> 
>>
>>>
>>>> @@ -0,0 +1,41 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/spi/loongson,ls2k-spi.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Loongson SPI controller
>>>> +
>>>> +maintainers:
>>>> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
>>>> +
>>>> +allOf:
>>>> +  - $ref: /schemas/spi/spi-controller.yaml#
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - loongson,ls2k1000-spi
>>>
>>> No compatibles for other devices? Didn't we have big discussion about this?
>>>
>>> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42
>>
>>
>> There are other ls2k SPI devices compatible, such as,
>> "loongson,ls2k0500-spi", "loongson,ls2k2000-spi" but currently I plan to
>> add ls2k1000 spi device first, Other ls2k SoC spi device adaptation may
>> require some additional work and I will add it later.
> 
> Previously you claimed this serves entire family, so I don't understand
> why you need to fix something. Why previously it was working for entire
> family but now it does not?


It can work was for ls2k1000 and ls2k0500 and it specifically refers to
spi driver. but 2k0500 doesn't implementing a clock driver and doesn't
use CCF to gain clock but can use "clock-frequency".  Is it necessary to
obtain a clock based on CCF? If it's necessary, then it seems that it
can only added 2k1000 spi first.

Thanks,
Yinbo




^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-08 10:00         ` zhuyinbo
@ 2023-06-08 10:02           ` Krzysztof Kozlowski
  2023-06-08 11:42             ` zhuyinbo
  0 siblings, 1 reply; 29+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-08 10:02 UTC (permalink / raw)
  To: zhuyinbo, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel

On 08/06/2023 12:00, zhuyinbo wrote:
> 
> 
> 在 2023/6/8 下午4:53, Krzysztof Kozlowski 写道:
>> On 08/06/2023 10:39, zhuyinbo wrote:
>>>>>
>>>>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>> ---
>>>>>    .../bindings/spi/loongson,ls2k-spi.yaml       | 41 +++++++++++++++++++
>>>>>    MAINTAINERS                                   |  6 +++
>>>>>    2 files changed, 47 insertions(+)
>>>>>    create mode 100644 Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..423ee851edd5
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>
>>>> Filename based on compatible.
>>>
>>>
>>> There will be more ls2k series SoC spi device in the future thus I still
>>> use "loongson,ls2k-spi.yaml" for cover it.
>>
>> Add them now.
> 
> 
> The 2k0500 doesn't support CCF and not use CCF to gain clock and We
> internally tend to prioritize supporting 2k1000.

Don't you refer now to drivers? Because how hardware can not support
clocks if it has them? How CCF is anyhow related to hardware?

> 
>>
>>>
>>>>
>>>>> @@ -0,0 +1,41 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/spi/loongson,ls2k-spi.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Loongson SPI controller
>>>>> +
>>>>> +maintainers:
>>>>> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
>>>>> +
>>>>> +allOf:
>>>>> +  - $ref: /schemas/spi/spi-controller.yaml#
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    enum:
>>>>> +      - loongson,ls2k1000-spi
>>>>
>>>> No compatibles for other devices? Didn't we have big discussion about this?
>>>>
>>>> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42
>>>
>>>
>>> There are other ls2k SPI devices compatible, such as,
>>> "loongson,ls2k0500-spi", "loongson,ls2k2000-spi" but currently I plan to
>>> add ls2k1000 spi device first, Other ls2k SoC spi device adaptation may
>>> require some additional work and I will add it later.
>>
>> Previously you claimed this serves entire family, so I don't understand
>> why you need to fix something. Why previously it was working for entire
>> family but now it does not?
> 
> 
> It can work was for ls2k1000 and ls2k0500 and it specifically refers to
> spi driver. but 2k0500 doesn't implementing a clock driver and doesn't

We do not discuss here drivers, but bindings. Whatever your drivers are
not supporting, matters less.

> use CCF to gain clock but can use "clock-frequency".  Is it necessary to
> obtain a clock based on CCF? If it's necessary, then it seems that it
> can only added 2k1000 spi first.

Not related to bindings...

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 2/2] spi: loongson: add bus driver for the loongson spi controller
  2023-06-08  7:28 ` [PATCH v12 2/2] spi: loongson: add bus driver for the loongson spi controller Yinbo Zhu
@ 2023-06-08 10:15   ` Andy Shevchenko
  2023-06-08 10:18     ` Andy Shevchenko
                       ` (2 more replies)
  0 siblings, 3 replies; 29+ messages in thread
From: Andy Shevchenko @ 2023-06-08 10:15 UTC (permalink / raw)
  To: Yinbo Zhu
  Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi,
	devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao,
	loongson-kernel

On Thu, Jun 8, 2023 at 10:28 AM Yinbo Zhu <zhuyinbo@loongson.cn> wrote:
>
> This bus driver supports the Loongson SPI hardware controller in the
> Loongson platforms and supports to use DTS and PCI framework to

the use

> register SPI device resources.

Thank you for an update. I have a few nit-picks below, but in general
this version is good (esp. if you address them)
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> ---
>  MAINTAINERS                     |   4 +
>  drivers/spi/Kconfig             |  26 +++
>  drivers/spi/Makefile            |   3 +
>  drivers/spi/spi-loongson-core.c | 275 ++++++++++++++++++++++++++++++++
>  drivers/spi/spi-loongson-pci.c  |  55 +++++++
>  drivers/spi/spi-loongson-plat.c |  47 ++++++
>  drivers/spi/spi-loongson.h      |  49 ++++++
>  7 files changed, 459 insertions(+)
>  create mode 100644 drivers/spi/spi-loongson-core.c
>  create mode 100644 drivers/spi/spi-loongson-pci.c
>  create mode 100644 drivers/spi/spi-loongson-plat.c
>  create mode 100644 drivers/spi/spi-loongson.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5e604dddd87b..69cb8fb2a0e1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12191,6 +12191,10 @@ M:     Yinbo Zhu <zhuyinbo@loongson.cn>
>  L:     linux-spi@vger.kernel.org
>  S:     Maintained
>  F:     Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
> +F:     drivers/spi/spi-loongson-core.c
> +F:     drivers/spi/spi-loongson-pci.c
> +F:     drivers/spi/spi-loongson-plat.c
> +F:     drivers/spi/spi-loongson.h
>
>  LOONGSON-2 SOC SERIES GUTS DRIVER
>  M:     Yinbo Zhu <zhuyinbo@loongson.cn>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 3de2ebe8294a..6b953904792e 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -516,6 +516,32 @@ config SPI_LM70_LLP
>           which interfaces to an LM70 temperature sensor using
>           a parallel port.
>
> +config SPI_LOONGSON_CORE
> +       tristate
> +       depends on LOONGARCH || COMPILE_TEST
> +
> +config SPI_LOONGSON_PCI
> +       tristate "Loongson SPI Controller PCI Driver Support"
> +       select SPI_LOONGSON_CORE
> +       depends on PCI && (LOONGARCH || COMPILE_TEST)
> +       help
> +         This bus driver supports the Loongson SPI hardware controller in
> +         the Loongson platforms and supports to use PCI framework to
> +         register SPI device resources.
> +         Say Y or M here if you want to use the SPI controller on
> +         Loongson platform.
> +
> +config SPI_LOONGSON_PLATFORM
> +       tristate "Loongson SPI Controller Platform Driver Support"
> +       select SPI_LOONGSON_CORE
> +       depends on OF && (LOONGARCH || COMPILE_TEST)
> +       help
> +         This bus driver supports the Loongson SPI hardware controller in
> +         the Loongson platforms and supports to use DTS framework to
> +         register SPI device resources.
> +         Say Y or M here if you want to use the SPI controller on
> +         Loongson platform.
> +
>  config SPI_LP8841_RTC
>         tristate "ICP DAS LP-8841 SPI Controller for RTC"
>         depends on MACH_PXA27X_DT || COMPILE_TEST
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index 28c4817a8a74..3e933064d237 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -71,6 +71,9 @@ obj-$(CONFIG_SPI_INTEL_PLATFORM)      += spi-intel-platform.o
>  obj-$(CONFIG_SPI_LANTIQ_SSC)           += spi-lantiq-ssc.o
>  obj-$(CONFIG_SPI_JCORE)                        += spi-jcore.o
>  obj-$(CONFIG_SPI_LM70_LLP)             += spi-lm70llp.o
> +obj-$(CONFIG_SPI_LOONGSON_CORE)                += spi-loongson-core.o
> +obj-$(CONFIG_SPI_LOONGSON_PCI)         += spi-loongson-pci.o
> +obj-$(CONFIG_SPI_LOONGSON_PLATFORM)    += spi-loongson-plat.o
>  obj-$(CONFIG_SPI_LP8841_RTC)           += spi-lp8841-rtc.o
>  obj-$(CONFIG_SPI_MESON_SPICC)          += spi-meson-spicc.o
>  obj-$(CONFIG_SPI_MESON_SPIFC)          += spi-meson-spifc.o
> diff --git a/drivers/spi/spi-loongson-core.c b/drivers/spi/spi-loongson-core.c
> new file mode 100644
> index 000000000000..403b735bd0cf
> --- /dev/null
> +++ b/drivers/spi/spi-loongson-core.c
> @@ -0,0 +1,275 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Loongson SPI Support
> +// Copyright (C) 2023 Loongson Technology Corporation Limited
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +
> +#include "spi-loongson.h"
> +
> +static inline void loongson_spi_write_reg(struct loongson_spi *spi, unsigned char reg,
> +                                         unsigned char data)
> +{
> +       writeb(data, spi->base + reg);
> +}
> +
> +static inline char loongson_spi_read_reg(struct loongson_spi *spi, unsigned char reg)
> +{
> +       return readb(spi->base + reg);
> +}
> +
> +static void loongson_spi_set_cs(struct spi_device *spi, bool val)
> +{
> +       int cs;
> +       struct loongson_spi *loongson_spi = spi_controller_get_devdata(spi->controller);
> +
> +       cs = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SFCS_REG)
> +                                          & ~(0x11 << spi_get_chipselect(spi, 0));
> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SFCS_REG,
> +                                      (val ? (0x11 << spi_get_chipselect(spi, 0)) :
> +                                      (0x1 << spi_get_chipselect(spi, 0))) | cs);

Can be done as

static void loongson_spi_set_cs(struct spi_device *spi, bool en)

    unsigned char mask = (BIT(4) | BIT(0)) << spi_get_chipselect(spi, 0);
    unsigned char val = en ? mask :  (BIT(0) << spi_get_chipselect(spi, 0));

    cs = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SFCS_REG) & ~mask;
    loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SFCS_REG, val | cs);

(Renamed variables to be consistent with the other uses in the driver below)

> +}
> +
> +static void loongson_spi_set_clk(struct loongson_spi *loongson_spi, unsigned int hz)
> +{
> +       unsigned char val;
> +       unsigned int div, div_tmp;
> +       static const char rdiv[12] = {0, 1, 4, 2, 3, 5, 6, 7, 8, 9, 10, 11};
> +
> +       div = clamp_val(DIV_ROUND_UP_ULL(loongson_spi->clk_rate, hz), 2, 4096);
> +       div_tmp = rdiv[fls(div - 1)];
> +       loongson_spi->spcr = (div_tmp & GENMASK(1, 0)) >> 0;
> +       loongson_spi->sper = (div_tmp & GENMASK(3, 2)) >> 2;
> +       val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPCR_REG);

    val &= GENMASK(1, 0);

> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, (val & ~3) |
> +                              loongson_spi->spcr);

       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, val
| loongson_spi->spcr);

> +       val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPER_REG);

    val &= GENMASK(1, 0);

> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPER_REG, (val & ~3) |
> +                              loongson_spi->sper);

       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPER_REG, val
| loongson_spi->sper);

> +       loongson_spi->hz = hz;
> +}
> +
> +static void loongson_spi_set_mode(struct loongson_spi *loongson_spi,
> +                                 struct spi_device *spi)
> +{
> +       unsigned char val;
> +
> +       val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPCR_REG);
> +       val &= ~(LOONGSON_SPI_SPCR_CPOL | LOONGSON_SPI_SPCR_CPHA);
> +       if (spi->mode & SPI_CPOL)
> +               val |= LOONGSON_SPI_SPCR_CPOL;
> +       if (spi->mode & SPI_CPHA)
> +               val |= LOONGSON_SPI_SPCR_CPHA;
> +
> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, val);
> +       loongson_spi->mode |= spi->mode;
> +}
> +
> +static int loongson_spi_update_state(struct loongson_spi *loongson_spi,
> +                               struct spi_device *spi, struct spi_transfer *t)
> +{
> +       if (t && loongson_spi->hz != t->speed_hz)
> +               loongson_spi_set_clk(loongson_spi, t->speed_hz);
> +
> +       if ((spi->mode ^ loongson_spi->mode) & SPI_MODE_X_MASK)
> +               loongson_spi_set_mode(loongson_spi, spi);
> +
> +       return 0;
> +}
> +
> +static int loongson_spi_setup(struct spi_device *spi)
> +{
> +       struct loongson_spi *loongson_spi;
> +
> +       loongson_spi = spi_controller_get_devdata(spi->controller);
> +       if (spi->bits_per_word % 8)
> +               return -EINVAL;
> +
> +       if (spi_get_chipselect(spi, 0) >= spi->controller->num_chipselect)
> +               return -EINVAL;
> +
> +       loongson_spi->hz = 0;
> +       loongson_spi_set_cs(spi, 1);
> +
> +       return 0;
> +}
> +
> +static int loongson_spi_write_read_8bit(struct spi_device *spi, const u8 **tx_buf,
> +                                       u8 **rx_buf, unsigned int num)
> +{
> +       int ret;
> +       struct loongson_spi *loongson_spi = spi_controller_get_devdata(spi->controller);
> +
> +       if (tx_buf && *tx_buf)
> +               loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_FIFO_REG, *((*tx_buf)++));
> +       else
> +               loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_FIFO_REG, 0);

+ Blank line

> +       ret = readb_poll_timeout(loongson_spi->base + LOONGSON_SPI_SPSR_REG, loongson_spi->spsr,
> +                       (loongson_spi->spsr & 0x1) != LOONGSON_SPI_SPSR_RFEMPTY, 1, MSEC_PER_SEC);

                       (loongson_spi->spsr &
LOONGSON_SPI_SPSR_RFEMPTY) != LOONGSON_SPI_SPSR_RFEMPTY,
                       1, MSEC_PER_SEC);

> +       if (rx_buf && *rx_buf)
> +               *(*rx_buf)++ = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_FIFO_REG);
> +       else
> +               loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_FIFO_REG);
> +
> +       return ret;
> +}
> +
> +static int loongson_spi_write_read(struct spi_device *spi, struct spi_transfer *xfer)
> +{
> +       int ret;
> +       unsigned int count;
> +       const u8 *tx = xfer->tx_buf;
> +       u8 *rx = xfer->rx_buf;
> +
> +       count = xfer->len;

> +

Unneeded blank line.

> +       do {
> +               ret = loongson_spi_write_read_8bit(spi, &tx, &rx, count);
> +               if (ret)
> +                       break;
> +       } while (--count);
> +
> +       return ret;
> +}
> +
> +static int loongson_spi_prepare_message(struct spi_controller *ctlr, struct spi_message *m)
> +{
> +       struct loongson_spi *loongson_spi = spi_controller_get_devdata(ctlr);

> +       loongson_spi->para = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_PARA_REG);

> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_PARA_REG, loongson_spi->para & ~1);

BIT(0) ?
LOONGSON_SPI_PARA_MEM_EN ?

> +       return 0;
> +}
> +
> +static int loongson_spi_transfer_one(struct spi_controller *ctrl, struct spi_device *spi,
> +                                    struct spi_transfer *xfer)
> +{
> +       struct loongson_spi *loongson_spi = spi_controller_get_devdata(spi->controller);
> +
> +       loongson_spi_update_state(loongson_spi, spi, xfer);
> +       if (xfer->len)
> +               return loongson_spi_write_read(spi, xfer);
> +
> +       return 0;
> +}
> +
> +static int loongson_spi_unprepare_message(struct spi_controller *ctrl, struct spi_message *m)
> +{
> +       struct loongson_spi *loongson_spi = spi_controller_get_devdata(ctrl);
> +
> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_PARA_REG, loongson_spi->para);
> +
> +       return 0;
> +}
> +
> +static void loongson_spi_reginit(struct loongson_spi *loongson_spi_dev)
> +{
> +       unsigned char val;
> +
> +       val = loongson_spi_read_reg(loongson_spi_dev, LOONGSON_SPI_SPCR_REG);
> +       val &= ~LOONGSON_SPI_SPCR_SPE;
> +       loongson_spi_write_reg(loongson_spi_dev, LOONGSON_SPI_SPCR_REG, val);
> +
> +       loongson_spi_write_reg(loongson_spi_dev, LOONGSON_SPI_SPSR_REG,
> +                              (LOONGSON_SPI_SPSR_SPIF | LOONGSON_SPI_SPSR_WCOL));
> +
> +       val = loongson_spi_read_reg(loongson_spi_dev, LOONGSON_SPI_SPCR_REG);
> +       val |= LOONGSON_SPI_SPCR_SPE;
> +       loongson_spi_write_reg(loongson_spi_dev, LOONGSON_SPI_SPCR_REG, val);
> +}
> +
> +int loongson_spi_init_controller(struct device *dev, void __iomem *regs)
> +{
> +       struct spi_controller *controller;
> +       struct loongson_spi *spi;
> +       struct clk *clk;
> +
> +       controller = devm_spi_alloc_host(dev, sizeof(struct loongson_spi));
> +       if (controller == NULL)
> +               return -ENOMEM;
> +
> +       controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;

   ... = SPI_MODE_X_MASK | SPI_CS_HIGH;

> +       controller->setup = loongson_spi_setup;
> +       controller->prepare_message = loongson_spi_prepare_message;
> +       controller->transfer_one = loongson_spi_transfer_one;
> +       controller->unprepare_message = loongson_spi_unprepare_message;
> +       controller->set_cs = loongson_spi_set_cs;
> +       controller->num_chipselect = 4;
> +       device_set_node(&controller->dev, dev_fwnode(dev));
> +       dev_set_drvdata(dev, controller);
> +
> +       spi = spi_controller_get_devdata(controller);
> +       spi->base = regs;
> +       spi->controller = controller;
> +
> +       clk = devm_clk_get_optional(dev, NULL);
> +       if (IS_ERR(clk))
> +               return dev_err_probe(dev, PTR_ERR(clk), "unable to get clock\n");
> +
> +       spi->clk_rate = clk_get_rate(clk);
> +       loongson_spi_reginit(spi);
> +
> +       spi->mode = 0;
> +
> +       return devm_spi_register_controller(dev, controller);
> +}
> +EXPORT_SYMBOL_NS_GPL(loongson_spi_init_controller, SPI_LOONGSON_CORE);
> +
> +static int __maybe_unused loongson_spi_suspend(struct device *dev)
> +{
> +       struct loongson_spi *loongson_spi;
> +       struct spi_controller *controller;
> +
> +       controller = dev_get_drvdata(dev);
> +       spi_controller_suspend(controller);
> +
> +       loongson_spi = spi_controller_get_devdata(controller);
> +
> +       loongson_spi->spcr = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPCR_REG);
> +       loongson_spi->sper = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPER_REG);
> +       loongson_spi->spsr = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPSR_REG);
> +       loongson_spi->para = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_PARA_REG);
> +       loongson_spi->sfcs = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SFCS_REG);
> +       loongson_spi->timi = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_TIMI_REG);
> +
> +       return 0;
> +}
> +
> +static int __maybe_unused loongson_spi_resume(struct device *dev)
> +{
> +       struct loongson_spi *loongson_spi;
> +       struct spi_controller *controller;
> +
> +       controller = dev_get_drvdata(dev);
> +       loongson_spi = spi_controller_get_devdata(controller);
> +
> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, loongson_spi->spcr);
> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPER_REG, loongson_spi->sper);
> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPSR_REG, loongson_spi->spsr);
> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_PARA_REG, loongson_spi->para);
> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SFCS_REG, loongson_spi->sfcs);
> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_TIMI_REG, loongson_spi->timi);
> +
> +       spi_controller_resume(controller);
> +
> +       return 0;
> +}
> +
> +const struct dev_pm_ops loongson_spi_dev_pm_ops = {
> +       .suspend = loongson_spi_suspend,
> +       .resume = loongson_spi_resume,
> +};
> +EXPORT_SYMBOL_NS_GPL(loongson_spi_dev_pm_ops, SPI_LOONGSON_CORE);
> +
> +MODULE_DESCRIPTION("Loongson SPI core driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/spi/spi-loongson-pci.c b/drivers/spi/spi-loongson-pci.c
> new file mode 100644
> index 000000000000..134cda0c13a5
> --- /dev/null
> +++ b/drivers/spi/spi-loongson-pci.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// PCI interface driver for Loongson SPI Support
> +// Copyright (C) 2023 Loongson Technology Corporation Limited
> +
> +#include <linux/mod_devicetable.h>
> +#include <linux/pci.h>
> +
> +#include "spi-loongson.h"
> +
> +static int loongson_spi_pci_register(struct pci_dev *pdev,
> +                       const struct pci_device_id *ent)
> +{
> +       int ret;
> +       void __iomem *reg_base;
> +       struct device *dev = &pdev->dev;
> +       int pci_bar = 0;
> +
> +       ret = pcim_enable_device(pdev);
> +       if (ret < 0)
> +               return dev_err_probe(dev, ret, "cannot enable pci device\n");
> +
> +       ret = pcim_iomap_regions(pdev, BIT(pci_bar), pci_name(pdev));
> +       if (ret)
> +               return dev_err_probe(dev, ret, "failed to request and remap memory\n");
> +
> +       reg_base = pcim_iomap_table(pdev)[pci_bar];
> +
> +       ret = loongson_spi_init_controller(dev, reg_base);
> +       if (ret)
> +               return dev_err_probe(dev, ret, "failed to initialize controller\n");
> +
> +       return 0;
> +}
> +
> +static struct pci_device_id loongson_spi_devices[] = {
> +       { PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x7a0b) },
> +       { PCI_DEVICE(PCI_VENDOR_ID_LOONGSON, 0x7a1b) },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(pci, loongson_spi_devices);
> +
> +static struct pci_driver loongson_spi_pci_driver = {
> +       .name       = "loongson-spi-pci",
> +       .id_table   = loongson_spi_devices,
> +       .probe      = loongson_spi_pci_register,
> +       .driver = {
> +               .bus = &pci_bus_type,
> +               .pm = &loongson_spi_dev_pm_ops,
> +       },
> +};
> +module_pci_driver(loongson_spi_pci_driver);
> +
> +MODULE_DESCRIPTION("Loongson spi pci driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(SPI_LOONGSON_CORE);
> diff --git a/drivers/spi/spi-loongson-plat.c b/drivers/spi/spi-loongson-plat.c
> new file mode 100644
> index 000000000000..c066e5f5891e
> --- /dev/null
> +++ b/drivers/spi/spi-loongson-plat.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Platform driver for Loongson SPI Support
> +// Copyright (C) 2023 Loongson Technology Corporation Limited
> +
> +#include <linux/err.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/platform_device.h>
> +
> +#include "spi-loongson.h"
> +
> +static int loongson_spi_platform_probe(struct platform_device *pdev)
> +{
> +       int ret;
> +       void __iomem *reg_base;
> +       struct device *dev = &pdev->dev;
> +
> +       reg_base = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(reg_base))
> +               return PTR_ERR(reg_base);
> +
> +       ret = loongson_spi_init_controller(dev, reg_base);
> +       if (ret)
> +               return dev_err_probe(dev, ret, "failed to initialize controller\n");
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id loongson_spi_id_table[] = {
> +       { .compatible = "loongson,ls2k1000-spi" },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, loongson_spi_id_table);
> +
> +static struct platform_driver loongson_spi_plat_driver = {
> +       .probe = loongson_spi_platform_probe,
> +       .driver = {
> +               .name   = "loongson-spi",
> +               .bus = &platform_bus_type,
> +               .pm = &loongson_spi_dev_pm_ops,
> +               .of_match_table = loongson_spi_id_table,
> +       },
> +};
> +module_platform_driver(loongson_spi_plat_driver);
> +
> +MODULE_DESCRIPTION("Loongson spi platform driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(SPI_LOONGSON_CORE);
> diff --git a/drivers/spi/spi-loongson.h b/drivers/spi/spi-loongson.h
> new file mode 100644
> index 000000000000..35f95b161842
> --- /dev/null
> +++ b/drivers/spi/spi-loongson.h
> @@ -0,0 +1,49 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/* Header File for Loongson SPI Driver. */
> +/* Copyright (C) 2023 Loongson Technology Corporation Limited */
> +
> +#ifndef __LINUX_SPI_LOONGSON_H
> +#define __LINUX_SPI_LOONGSON_H
> +
> +#include <linux/bits.h>
> +#include <linux/pm.h>
> +#include <linux/types.h>
> +
> +#define        LOONGSON_SPI_SPCR_REG   0x00
> +#define        LOONGSON_SPI_SPSR_REG   0x01
> +#define        LOONGSON_SPI_FIFO_REG   0x02
> +#define        LOONGSON_SPI_SPER_REG   0x03
> +#define        LOONGSON_SPI_PARA_REG   0x04
> +#define        LOONGSON_SPI_SFCS_REG   0x05
> +#define        LOONGSON_SPI_TIMI_REG   0x06
> +
> +/* Bits definition for Loongson SPI register */
> +#define        LOONGSON_SPI_PARA_MEM_EN        BIT(0)
> +#define        LOONGSON_SPI_SPCR_CPHA  BIT(2)
> +#define        LOONGSON_SPI_SPCR_CPOL  BIT(3)
> +#define        LOONGSON_SPI_SPCR_SPE   BIT(6)
> +#define        LOONGSON_SPI_SPSR_RFEMPTY       BIT(0)
> +#define        LOONGSON_SPI_SPSR_WCOL  BIT(6)
> +#define        LOONGSON_SPI_SPSR_SPIF  BIT(7)
> +
> +struct device;
> +struct spi_controller;
> +
> +struct loongson_spi {
> +       struct  spi_controller  *controller;
> +       void __iomem            *base;
> +       int                     cs_active;
> +       unsigned int            hz;
> +       unsigned char           spcr;
> +       unsigned char           sper;
> +       unsigned char           spsr;
> +       unsigned char           para;
> +       unsigned char           sfcs;
> +       unsigned char           timi;
> +       unsigned int            mode;
> +       u64                     clk_rate;
> +};
> +
> +int loongson_spi_init_controller(struct device *dev, void __iomem *reg);
> +extern const struct dev_pm_ops loongson_spi_dev_pm_ops;
> +#endif /* __LINUX_SPI_LOONGSON_H */
> --
> 2.20.1
>


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 2/2] spi: loongson: add bus driver for the loongson spi controller
  2023-06-08 10:15   ` Andy Shevchenko
@ 2023-06-08 10:18     ` Andy Shevchenko
  2023-06-09  7:13       ` zhuyinbo
  2023-06-08 10:29     ` Mark Brown
  2023-06-09  7:10     ` zhuyinbo
  2 siblings, 1 reply; 29+ messages in thread
From: Andy Shevchenko @ 2023-06-08 10:18 UTC (permalink / raw)
  To: Yinbo Zhu
  Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi,
	devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao,
	loongson-kernel

On Thu, Jun 8, 2023 at 1:15 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Jun 8, 2023 at 10:28 AM Yinbo Zhu <zhuyinbo@loongson.cn> wrote:




> > +       ret = readb_poll_timeout(loongson_spi->base + LOONGSON_SPI_SPSR_REG, loongson_spi->spsr,
> > +                       (loongson_spi->spsr & 0x1) != LOONGSON_SPI_SPSR_RFEMPTY, 1, MSEC_PER_SEC);
>
>                        (loongson_spi->spsr &
> LOONGSON_SPI_SPSR_RFEMPTY) != LOONGSON_SPI_SPSR_RFEMPTY,
>                        1, MSEC_PER_SEC);

Actually the last should be USEC_PER_MSEC, as the parameter is in microseconds.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 2/2] spi: loongson: add bus driver for the loongson spi controller
  2023-06-08 10:15   ` Andy Shevchenko
  2023-06-08 10:18     ` Andy Shevchenko
@ 2023-06-08 10:29     ` Mark Brown
  2023-06-08 11:45       ` zhuyinbo
  2023-06-09  7:10     ` zhuyinbo
  2 siblings, 1 reply; 29+ messages in thread
From: Mark Brown @ 2023-06-08 10:29 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Yinbo Zhu, Rob Herring, Krzysztof Kozlowski, linux-spi,
	devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao,
	loongson-kernel

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

On Thu, Jun 08, 2023 at 01:15:39PM +0300, Andy Shevchenko wrote:
> On Thu, Jun 8, 2023 at 10:28 AM Yinbo Zhu <zhuyinbo@loongson.cn> wrote:
> >
> > This bus driver supports the Loongson SPI hardware controller in the
> > Loongson platforms and supports to use DTS and PCI framework to

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-08 10:02           ` Krzysztof Kozlowski
@ 2023-06-08 11:42             ` zhuyinbo
  2023-06-08 11:45               ` Krzysztof Kozlowski
  0 siblings, 1 reply; 29+ messages in thread
From: zhuyinbo @ 2023-06-08 11:42 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo



在 2023/6/8 下午6:02, Krzysztof Kozlowski 写道:
> On 08/06/2023 12:00, zhuyinbo wrote:
>>
>>
>> 在 2023/6/8 下午4:53, Krzysztof Kozlowski 写道:
>>> On 08/06/2023 10:39, zhuyinbo wrote:
>>>>>>
>>>>>> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
>>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>> ---
>>>>>>     .../bindings/spi/loongson,ls2k-spi.yaml       | 41 +++++++++++++++++++
>>>>>>     MAINTAINERS                                   |  6 +++
>>>>>>     2 files changed, 47 insertions(+)
>>>>>>     create mode 100644 Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>> new file mode 100644
>>>>>> index 000000000000..423ee851edd5
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>
>>>>> Filename based on compatible.
>>>>
>>>>
>>>> There will be more ls2k series SoC spi device in the future thus I still
>>>> use "loongson,ls2k-spi.yaml" for cover it.
>>>
>>> Add them now.
>>
>>
>> The 2k0500 doesn't support CCF and not use CCF to gain clock and We
>> internally tend to prioritize supporting 2k1000.
> 
> Don't you refer now to drivers? Because how hardware can not support
> clocks if it has them? How CCF is anyhow related to hardware?


The CCF (common clock framework) driver only affects the clock parameter
pass method and isn't related to clock hardware. and if dts pass a
"clock-frequency" that not need a clock driver but if dts pass a
"clocks" that need a clock driver. Currently, only 2k1000 has
implemented a clock driver.

> 
>>
>>>
>>>>
>>>>>
>>>>>> @@ -0,0 +1,41 @@
>>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id: http://devicetree.org/schemas/spi/loongson,ls2k-spi.yaml#
>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: Loongson SPI controller
>>>>>> +
>>>>>> +maintainers:
>>>>>> +  - Yinbo Zhu <zhuyinbo@loongson.cn>
>>>>>> +
>>>>>> +allOf:
>>>>>> +  - $ref: /schemas/spi/spi-controller.yaml#
>>>>>> +
>>>>>> +properties:
>>>>>> +  compatible:
>>>>>> +    enum:
>>>>>> +      - loongson,ls2k1000-spi
>>>>>
>>>>> No compatibles for other devices? Didn't we have big discussion about this?
>>>>>
>>>>> https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42
>>>>
>>>>
>>>> There are other ls2k SPI devices compatible, such as,
>>>> "loongson,ls2k0500-spi", "loongson,ls2k2000-spi" but currently I plan to
>>>> add ls2k1000 spi device first, Other ls2k SoC spi device adaptation may
>>>> require some additional work and I will add it later.
>>>
>>> Previously you claimed this serves entire family, so I don't understand
>>> why you need to fix something. Why previously it was working for entire
>>> family but now it does not?
>>
>>
>> It can work was for ls2k1000 and ls2k0500 and it specifically refers to
>> spi driver. but 2k0500 doesn't implementing a clock driver and doesn't
> 
> We do not discuss here drivers, but bindings. Whatever your drivers are
> not supporting, matters less.
> 
>> use CCF to gain clock but can use "clock-frequency".  Is it necessary to
>> obtain a clock based on CCF? If it's necessary, then it seems that it
>> can only added 2k1000 spi first.
> 
> Not related to bindings...


I may understand that what you said, and the dt-bindings only cover 
hardware and not involve the drivers. if so, I will add following:


--- a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
+++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
@@ -16,6 +16,7 @@ properties:
    compatible:
      enum:
        - loongson,ls2k1000-spi
+      - loongson,ls2k0500-spi


Thanks,
Yinbo


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-08 11:42             ` zhuyinbo
@ 2023-06-08 11:45               ` Krzysztof Kozlowski
  2023-06-08 12:10                 ` zhuyinbo
  0 siblings, 1 reply; 29+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-08 11:45 UTC (permalink / raw)
  To: zhuyinbo, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel

On 08/06/2023 13:42, zhuyinbo wrote:
>>
>>> It can work was for ls2k1000 and ls2k0500 and it specifically refers to
>>> spi driver. but 2k0500 doesn't implementing a clock driver and doesn't
>>
>> We do not discuss here drivers, but bindings. Whatever your drivers are
>> not supporting, matters less.
>>
>>> use CCF to gain clock but can use "clock-frequency".  Is it necessary to
>>> obtain a clock based on CCF? If it's necessary, then it seems that it
>>> can only added 2k1000 spi first.
>>
>> Not related to bindings...
> 
> 
> I may understand that what you said, and the dt-bindings only cover 
> hardware and not involve the drivers. if so, I will add following:
> 
> 
> --- a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
> @@ -16,6 +16,7 @@ properties:
>     compatible:
>       enum:
>         - loongson,ls2k1000-spi
> +      - loongson,ls2k0500-spi

Aren't they compatible?

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 2/2] spi: loongson: add bus driver for the loongson spi controller
  2023-06-08 10:29     ` Mark Brown
@ 2023-06-08 11:45       ` zhuyinbo
  2023-06-08 11:53         ` Mark Brown
  0 siblings, 1 reply; 29+ messages in thread
From: zhuyinbo @ 2023-06-08 11:45 UTC (permalink / raw)
  To: Mark Brown, Andy Shevchenko
  Cc: Rob Herring, Krzysztof Kozlowski, linux-spi, devicetree,
	linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao,
	loongson-kernel, zhuyinbo



在 2023/6/8 下午6:29, Mark Brown 写道:
> On Thu, Jun 08, 2023 at 01:15:39PM +0300, Andy Shevchenko wrote:
>> On Thu, Jun 8, 2023 at 10:28 AM Yinbo Zhu <zhuyinbo@loongson.cn> wrote:
>>>
>>> This bus driver supports the Loongson SPI hardware controller in the
>>> Loongson platforms and supports to use DTS and PCI framework to
> 
> Please delete unneeded context from mails when replying.  Doing this
> makes it much easier to find your reply in the message, helping ensure
> it won't be missed by people scrolling through the irrelevant quoted
> material.


okay, I got it.

Thanks,
Yinbo


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 2/2] spi: loongson: add bus driver for the loongson spi controller
  2023-06-08 11:45       ` zhuyinbo
@ 2023-06-08 11:53         ` Mark Brown
  2023-06-09  3:17           ` zhuyinbo
  0 siblings, 1 reply; 29+ messages in thread
From: Mark Brown @ 2023-06-08 11:53 UTC (permalink / raw)
  To: zhuyinbo
  Cc: Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, linux-spi,
	devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao,
	loongson-kernel

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

On Thu, Jun 08, 2023 at 07:45:49PM +0800, zhuyinbo wrote:
> 在 2023/6/8 下午6:29, Mark Brown 写道:
> > On Thu, Jun 08, 2023 at 01:15:39PM +0300, Andy Shevchenko wrote:
> > > On Thu, Jun 8, 2023 at 10:28 AM Yinbo Zhu <zhuyinbo@loongson.cn> wrote:

> > > > This bus driver supports the Loongson SPI hardware controller in the
> > > > Loongson platforms and supports to use DTS and PCI framework to

> > Please delete unneeded context from mails when replying.  Doing this
> > makes it much easier to find your reply in the message, helping ensure
> > it won't be missed by people scrolling through the irrelevant quoted
> > material.

> okay, I got it.

That was more directed at Andy than you!

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-08 11:45               ` Krzysztof Kozlowski
@ 2023-06-08 12:10                 ` zhuyinbo
  2023-06-08 13:26                   ` Krzysztof Kozlowski
  0 siblings, 1 reply; 29+ messages in thread
From: zhuyinbo @ 2023-06-08 12:10 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo



在 2023/6/8 下午7:45, Krzysztof Kozlowski 写道:
> On 08/06/2023 13:42, zhuyinbo wrote:
>> --- a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>> @@ -16,6 +16,7 @@ properties:
>>      compatible:
>>        enum:
>>          - loongson,ls2k1000-spi
>> +      - loongson,ls2k0500-spi
> 
> Aren't they compatible?
> 


Are you saying that the spi driver is compatible with 2k0500 ?
Yes.  and the 2k1000 spi hardware was same with 2k0500 common type spi
hardware.

but afterwards, it may be necessary to implement a clock drvier for
2k0500, because the spi driver was use "devm_clk_get_optional()" to
get clock and not use "of_property_read_u32(np, "clock-frequency",
&clk)",  But this seems to have nothing to do with bindings.


Thanks,
Yinbo


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-08 12:10                 ` zhuyinbo
@ 2023-06-08 13:26                   ` Krzysztof Kozlowski
  2023-06-09  3:13                     ` zhuyinbo
  0 siblings, 1 reply; 29+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-08 13:26 UTC (permalink / raw)
  To: zhuyinbo, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel

On 08/06/2023 14:10, zhuyinbo wrote:
> 
> 
> 在 2023/6/8 下午7:45, Krzysztof Kozlowski 写道:
>> On 08/06/2023 13:42, zhuyinbo wrote:
>>> --- a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>> @@ -16,6 +16,7 @@ properties:
>>>      compatible:
>>>        enum:
>>>          - loongson,ls2k1000-spi
>>> +      - loongson,ls2k0500-spi
>>
>> Aren't they compatible?
>>
> 
> 
> Are you saying that the spi driver is compatible with 2k0500 ?

Didn't you say this through 11 previous revisions?

> Yes.  and the 2k1000 spi hardware was same with 2k0500 common type spi
> hardware.
> 
> but afterwards, it may be necessary to implement a clock drvier for
> 2k0500, because the spi driver was use "devm_clk_get_optional()" to
> get clock and not use "of_property_read_u32(np, "clock-frequency",
> &clk)",  But this seems to have nothing to do with bindings.
>

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-08 13:26                   ` Krzysztof Kozlowski
@ 2023-06-09  3:13                     ` zhuyinbo
  2023-06-09 16:45                       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 29+ messages in thread
From: zhuyinbo @ 2023-06-09  3:13 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo



在 2023/6/8 下午9:26, Krzysztof Kozlowski 写道:
> On 08/06/2023 14:10, zhuyinbo wrote:
>>
>>
>> 在 2023/6/8 下午7:45, Krzysztof Kozlowski 写道:
>>> On 08/06/2023 13:42, zhuyinbo wrote:
>>>> --- a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>> @@ -16,6 +16,7 @@ properties:
>>>>       compatible:
>>>>         enum:
>>>>           - loongson,ls2k1000-spi
>>>> +      - loongson,ls2k0500-spi
>>>
>>> Aren't they compatible?
>>>
>>
>>
>> Are you saying that the spi driver is compatible with 2k0500 ?
> 
> Didn't you say this through 11 previous revisions?


Yes, did I understand your meaning incorrectly ?

Thanks,
Yinbo
> 
>> Yes.  and the 2k1000 spi hardware was same with 2k0500 common type spi
>> hardware.
>>
>> but afterwards, it may be necessary to implement a clock drvier for
>> 2k0500, because the spi driver was use "devm_clk_get_optional()" to
>> get clock and not use "of_property_read_u32(np, "clock-frequency",
>> &clk)",  But this seems to have nothing to do with bindings.


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 2/2] spi: loongson: add bus driver for the loongson spi controller
  2023-06-08 11:53         ` Mark Brown
@ 2023-06-09  3:17           ` zhuyinbo
  0 siblings, 0 replies; 29+ messages in thread
From: zhuyinbo @ 2023-06-09  3:17 UTC (permalink / raw)
  To: Mark Brown
  Cc: Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, linux-spi,
	devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao,
	loongson-kernel, zhuyinbo



在 2023/6/8 下午7:53, Mark Brown 写道:
> On Thu, Jun 08, 2023 at 07:45:49PM +0800, zhuyinbo wrote:
>> 在 2023/6/8 下午6:29, Mark Brown 写道:
>>> On Thu, Jun 08, 2023 at 01:15:39PM +0300, Andy Shevchenko wrote:
>>>> On Thu, Jun 8, 2023 at 10:28 AM Yinbo Zhu <zhuyinbo@loongson.cn> wrote:
> 
>>>>> This bus driver supports the Loongson SPI hardware controller in the
>>>>> Loongson platforms and supports to use DTS and PCI framework to
> 
>>> Please delete unneeded context from mails when replying.  Doing this
>>> makes it much easier to find your reply in the message, helping ensure
>>> it won't be missed by people scrolling through the irrelevant quoted
>>> material.
> 
>> okay, I got it.
> 
> That was more directed at Andy than you!


Okay, I will learn from Andy.

Thanks
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 2/2] spi: loongson: add bus driver for the loongson spi controller
  2023-06-08 10:15   ` Andy Shevchenko
  2023-06-08 10:18     ` Andy Shevchenko
  2023-06-08 10:29     ` Mark Brown
@ 2023-06-09  7:10     ` zhuyinbo
  2 siblings, 0 replies; 29+ messages in thread
From: zhuyinbo @ 2023-06-09  7:10 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi,
	devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao,
	loongson-kernel, zhuyinbo



在 2023/6/8 下午6:15, Andy Shevchenko 写道:
> On Thu, Jun 8, 2023 at 10:28 AM Yinbo Zhu <zhuyinbo@loongson.cn> wrote:
>>
>> This bus driver supports the Loongson SPI hardware controller in the
>> Loongson platforms and supports to use DTS and PCI framework to
> 
> the use


okay, I got it.

> 
>> register SPI device resources.
> 
> Thank you for an update. I have a few nit-picks below, but in general
> this version is good (esp. if you address them)
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 


You're welcome, I will add the reviewed-by in v13.

...

>> +static void loongson_spi_set_cs(struct spi_device *spi, bool val)
>> +{
>> +       int cs;
>> +       struct loongson_spi *loongson_spi = spi_controller_get_devdata(spi->controller);
>> +
>> +       cs = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SFCS_REG)
>> +                                          & ~(0x11 << spi_get_chipselect(spi, 0));
>> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SFCS_REG,
>> +                                      (val ? (0x11 << spi_get_chipselect(spi, 0)) :
>> +                                      (0x1 << spi_get_chipselect(spi, 0))) | cs);
> 
> Can be done as
> 
> static void loongson_spi_set_cs(struct spi_device *spi, bool en)
> 
>      unsigned char mask = (BIT(4) | BIT(0)) << spi_get_chipselect(spi, 0);
>      unsigned char val = en ? mask :  (BIT(0) << spi_get_chipselect(spi, 0));
> 
>      cs = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SFCS_REG) & ~mask;
>      loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SFCS_REG, val | cs);


okay, I will do it.

> 
> (Renamed variables to be consistent with the other uses in the driver below)


sorry, I don't got it. Are you referring to the following changes ?
loongson_spi_set_cs(spi, 1) => loongson_spi_set_cs(spi, true)

> 
>> +}
>> +
>> +static void loongson_spi_set_clk(struct loongson_spi *loongson_spi, unsigned int hz)
>> +{
>> +       unsigned char val;
>> +       unsigned int div, div_tmp;
>> +       static const char rdiv[12] = {0, 1, 4, 2, 3, 5, 6, 7, 8, 9, 10, 11};
>> +
>> +       div = clamp_val(DIV_ROUND_UP_ULL(loongson_spi->clk_rate, hz), 2, 4096);
>> +       div_tmp = rdiv[fls(div - 1)];
>> +       loongson_spi->spcr = (div_tmp & GENMASK(1, 0)) >> 0;
>> +       loongson_spi->sper = (div_tmp & GENMASK(3, 2)) >> 2;
>> +       val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPCR_REG);
> 
>      val &= GENMASK(1, 0);


This seems to be "val &= ~GENMASK(1, 0);"

> 
>> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, (val & ~3) |
>> +                              loongson_spi->spcr);
> 
>         loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, val
> | loongson_spi->spcr);


okay, I got it.

> 
>> +       val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPER_REG);
> 
>      val &= GENMASK(1, 0);


This seems to be "val &= ~GENMASK(1, 0);"

> 
>> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPER_REG, (val & ~3) |
>> +                              loongson_spi->sper);
> 
>         loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPER_REG, val
> | loongson_spi->sper);
> 


okay,  I got it.

>> +       loongson_spi->hz = hz;
>> +}
>> +
>> +static void loongson_spi_set_mode(struct loongson_spi *loongson_spi,
>> +                                 struct spi_device *spi)
>> +{
>> +       unsigned char val;
>> +
>> +       val = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_SPCR_REG);
>> +       val &= ~(LOONGSON_SPI_SPCR_CPOL | LOONGSON_SPI_SPCR_CPHA);
>> +       if (spi->mode & SPI_CPOL)
>> +               val |= LOONGSON_SPI_SPCR_CPOL;
>> +       if (spi->mode & SPI_CPHA)
>> +               val |= LOONGSON_SPI_SPCR_CPHA;
>> +
>> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_SPCR_REG, val);
>> +       loongson_spi->mode |= spi->mode;
>> +}
>> +
>> +static int loongson_spi_update_state(struct loongson_spi *loongson_spi,
>> +                               struct spi_device *spi, struct spi_transfer *t)
>> +{
>> +       if (t && loongson_spi->hz != t->speed_hz)
>> +               loongson_spi_set_clk(loongson_spi, t->speed_hz);
>> +
>> +       if ((spi->mode ^ loongson_spi->mode) & SPI_MODE_X_MASK)
>> +               loongson_spi_set_mode(loongson_spi, spi);
>> +
>> +       return 0;
>> +}
>> +
>> +static int loongson_spi_setup(struct spi_device *spi)
>> +{
>> +       struct loongson_spi *loongson_spi;
>> +
>> +       loongson_spi = spi_controller_get_devdata(spi->controller);
>> +       if (spi->bits_per_word % 8)
>> +               return -EINVAL;
>> +
>> +       if (spi_get_chipselect(spi, 0) >= spi->controller->num_chipselect)
>> +               return -EINVAL;
>> +
>> +       loongson_spi->hz = 0;
>> +       loongson_spi_set_cs(spi, 1);
>> +
>> +       return 0;
>> +}
>> +
>> +static int loongson_spi_write_read_8bit(struct spi_device *spi, const u8 **tx_buf,
>> +                                       u8 **rx_buf, unsigned int num)
>> +{
>> +       int ret;
>> +       struct loongson_spi *loongson_spi = spi_controller_get_devdata(spi->controller);
>> +
>> +       if (tx_buf && *tx_buf)
>> +               loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_FIFO_REG, *((*tx_buf)++));
>> +       else
>> +               loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_FIFO_REG, 0);
> 
> + Blank line


okay, I will add the blank line.

> 
>> +       ret = readb_poll_timeout(loongson_spi->base + LOONGSON_SPI_SPSR_REG, loongson_spi->spsr,
>> +                       (loongson_spi->spsr & 0x1) != LOONGSON_SPI_SPSR_RFEMPTY, 1, MSEC_PER_SEC);
> 
>                         (loongson_spi->spsr &
> LOONGSON_SPI_SPSR_RFEMPTY) != LOONGSON_SPI_SPSR_RFEMPTY,
>                         1, MSEC_PER_SEC);


okay, I got it.

> 
>> +       if (rx_buf && *rx_buf)
>> +               *(*rx_buf)++ = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_FIFO_REG);
>> +       else
>> +               loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_FIFO_REG);
>> +
>> +       return ret;
>> +}
>> +
>> +static int loongson_spi_write_read(struct spi_device *spi, struct spi_transfer *xfer)
>> +{
>> +       int ret;
>> +       unsigned int count;
>> +       const u8 *tx = xfer->tx_buf;
>> +       u8 *rx = xfer->rx_buf;
>> +
>> +       count = xfer->len;
> 
>> +
> 
> Unneeded blank line.


okay, I will remove the blank line.

> 
>> +       do {
>> +               ret = loongson_spi_write_read_8bit(spi, &tx, &rx, count);
>> +               if (ret)
>> +                       break;
>> +       } while (--count);
>> +
>> +       return ret;
>> +}
>> +
>> +static int loongson_spi_prepare_message(struct spi_controller *ctlr, struct spi_message *m)
>> +{
>> +       struct loongson_spi *loongson_spi = spi_controller_get_devdata(ctlr);
> 
>> +       loongson_spi->para = loongson_spi_read_reg(loongson_spi, LOONGSON_SPI_PARA_REG);
> 
>> +       loongson_spi_write_reg(loongson_spi, LOONGSON_SPI_PARA_REG, loongson_spi->para & ~1);
> 
> BIT(0) ?
> LOONGSON_SPI_PARA_MEM_EN ?


I will use LOONGSON_SPI_PARA_MEM_EN.

> 
>> +       return 0;
>> +}
>> +

...

>> +int loongson_spi_init_controller(struct device *dev, void __iomem *regs)
>> +{
>> +       struct spi_controller *controller;
>> +       struct loongson_spi *spi;
>> +       struct clk *clk;
>> +
>> +       controller = devm_spi_alloc_host(dev, sizeof(struct loongson_spi));
>> +       if (controller == NULL)
>> +               return -ENOMEM;
>> +
>> +       controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
> 
>     ... = SPI_MODE_X_MASK | SPI_CS_HIGH;


okay, I got it.


Thanks,
Yinbo


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 2/2] spi: loongson: add bus driver for the loongson spi controller
  2023-06-08 10:18     ` Andy Shevchenko
@ 2023-06-09  7:13       ` zhuyinbo
  0 siblings, 0 replies; 29+ messages in thread
From: zhuyinbo @ 2023-06-09  7:13 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Mark Brown, Rob Herring, Krzysztof Kozlowski, linux-spi,
	devicetree, linux-kernel, Jianmin Lv, wanghongliang, Liu Peibao,
	loongson-kernel, zhuyinbo



在 2023/6/8 下午6:18, Andy Shevchenko 写道:
> On Thu, Jun 8, 2023 at 1:15 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Thu, Jun 8, 2023 at 10:28 AM Yinbo Zhu <zhuyinbo@loongson.cn> wrote:
>>> +       ret = readb_poll_timeout(loongson_spi->base + LOONGSON_SPI_SPSR_REG, loongson_spi->spsr,
>>> +                       (loongson_spi->spsr & 0x1) != LOONGSON_SPI_SPSR_RFEMPTY, 1, MSEC_PER_SEC);
>>
>>                         (loongson_spi->spsr &
>> LOONGSON_SPI_SPSR_RFEMPTY) != LOONGSON_SPI_SPSR_RFEMPTY,
>>                         1, MSEC_PER_SEC);
> 
> Actually the last should be USEC_PER_MSEC, as the parameter is in microseconds.


okay, I got it.

Thanks
> 


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-09  3:13                     ` zhuyinbo
@ 2023-06-09 16:45                       ` Krzysztof Kozlowski
  2023-06-12  7:13                         ` zhuyinbo
  0 siblings, 1 reply; 29+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-09 16:45 UTC (permalink / raw)
  To: zhuyinbo, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel

On 09/06/2023 05:13, zhuyinbo wrote:
> 
> 
> 在 2023/6/8 下午9:26, Krzysztof Kozlowski 写道:
>> On 08/06/2023 14:10, zhuyinbo wrote:
>>>
>>>
>>> 在 2023/6/8 下午7:45, Krzysztof Kozlowski 写道:
>>>> On 08/06/2023 13:42, zhuyinbo wrote:
>>>>> --- a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>> @@ -16,6 +16,7 @@ properties:
>>>>>       compatible:
>>>>>         enum:
>>>>>           - loongson,ls2k1000-spi
>>>>> +      - loongson,ls2k0500-spi
>>>>
>>>> Aren't they compatible?
>>>>
>>>
>>>
>>> Are you saying that the spi driver is compatible with 2k0500 ?
>>
>> Didn't you say this through 11 previous revisions?
> 
> 
> Yes, did I understand your meaning incorrectly ?

If they are compatible, then they are not part of one enum. They could
not be as this would easily fail in testing of your DTS.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-09 16:45                       ` Krzysztof Kozlowski
@ 2023-06-12  7:13                         ` zhuyinbo
  2023-06-12  7:17                           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 29+ messages in thread
From: zhuyinbo @ 2023-06-12  7:13 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, zhuyinbo, Liu Peibao, loongson-kernel



在 2023/6/10 上午12:45, Krzysztof Kozlowski 写道:
> On 09/06/2023 05:13, zhuyinbo wrote:
>>
>>
>> 在 2023/6/8 下午9:26, Krzysztof Kozlowski 写道:
>>> On 08/06/2023 14:10, zhuyinbo wrote:
>>>>
>>>>
>>>> 在 2023/6/8 下午7:45, Krzysztof Kozlowski 写道:
>>>>> On 08/06/2023 13:42, zhuyinbo wrote:
>>>>>> --- a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>> @@ -16,6 +16,7 @@ properties:
>>>>>>        compatible:
>>>>>>          enum:
>>>>>>            - loongson,ls2k1000-spi
>>>>>> +      - loongson,ls2k0500-spi
>>>>>
>>>>> Aren't they compatible?
>>>>>
>>>>
>>>>
>>>> Are you saying that the spi driver is compatible with 2k0500 ?
>>>
>>> Didn't you say this through 11 previous revisions?
>>
>>
>> Yes, did I understand your meaning incorrectly ?
> 
> If they are compatible, then they are not part of one enum. They could
> not be as this would easily fail in testing of your DTS.
> 


The "loongson,ls2k0500-spi" wasn't a compatible in previous version and
I will add "loongson,ls2k0500-spi" as a compatible in spi driver and
added it as a part of the one enum in dt-binding.

Thanks,
Yinbo


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-12  7:13                         ` zhuyinbo
@ 2023-06-12  7:17                           ` Krzysztof Kozlowski
  2023-06-12  7:40                             ` zhuyinbo
  0 siblings, 1 reply; 29+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-12  7:17 UTC (permalink / raw)
  To: zhuyinbo, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel

On 12/06/2023 09:13, zhuyinbo wrote:
> 
> 
> 在 2023/6/10 上午12:45, Krzysztof Kozlowski 写道:
>> On 09/06/2023 05:13, zhuyinbo wrote:
>>>
>>>
>>> 在 2023/6/8 下午9:26, Krzysztof Kozlowski 写道:
>>>> On 08/06/2023 14:10, zhuyinbo wrote:
>>>>>
>>>>>
>>>>> 在 2023/6/8 下午7:45, Krzysztof Kozlowski 写道:
>>>>>> On 08/06/2023 13:42, zhuyinbo wrote:
>>>>>>> --- a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>>> @@ -16,6 +16,7 @@ properties:
>>>>>>>        compatible:
>>>>>>>          enum:
>>>>>>>            - loongson,ls2k1000-spi
>>>>>>> +      - loongson,ls2k0500-spi
>>>>>>
>>>>>> Aren't they compatible?
>>>>>>
>>>>>
>>>>>
>>>>> Are you saying that the spi driver is compatible with 2k0500 ?
>>>>
>>>> Didn't you say this through 11 previous revisions?
>>>
>>>
>>> Yes, did I understand your meaning incorrectly ?
>>
>> If they are compatible, then they are not part of one enum. They could
>> not be as this would easily fail in testing of your DTS.
>>
> 
> 
> The "loongson,ls2k0500-spi" wasn't a compatible in previous version and
> I will add "loongson,ls2k0500-spi" as a compatible in spi driver and
> added it as a part of the one enum in dt-binding.

No, because you claimed - if I understood correctly - that they are
compatible. Don't add fake entries to the driver.


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-12  7:17                           ` Krzysztof Kozlowski
@ 2023-06-12  7:40                             ` zhuyinbo
  2023-06-12  8:16                               ` Krzysztof Kozlowski
  0 siblings, 1 reply; 29+ messages in thread
From: zhuyinbo @ 2023-06-12  7:40 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo



在 2023/6/12 下午3:17, Krzysztof Kozlowski 写道:
> On 12/06/2023 09:13, zhuyinbo wrote:
>>
>>
>> 在 2023/6/10 上午12:45, Krzysztof Kozlowski 写道:
>>> On 09/06/2023 05:13, zhuyinbo wrote:
>>>>
>>>>
>>>> 在 2023/6/8 下午9:26, Krzysztof Kozlowski 写道:
>>>>> On 08/06/2023 14:10, zhuyinbo wrote:
>>>>>>
>>>>>>
>>>>>> 在 2023/6/8 下午7:45, Krzysztof Kozlowski 写道:
>>>>>>> On 08/06/2023 13:42, zhuyinbo wrote:
>>>>>>>> --- a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>>>> @@ -16,6 +16,7 @@ properties:
>>>>>>>>         compatible:
>>>>>>>>           enum:
>>>>>>>>             - loongson,ls2k1000-spi
>>>>>>>> +      - loongson,ls2k0500-spi
>>>>>>>
>>>>>>> Aren't they compatible?
>>>>>>>
>>>>>>
>>>>>>
>>>>>> Are you saying that the spi driver is compatible with 2k0500 ?
>>>>>
>>>>> Didn't you say this through 11 previous revisions?
>>>>
>>>>
>>>> Yes, did I understand your meaning incorrectly ?
>>>
>>> If they are compatible, then they are not part of one enum. They could
>>> not be as this would easily fail in testing of your DTS.
>>>
>>
>>
>> The "loongson,ls2k0500-spi" wasn't a compatible in previous version and
>> I will add "loongson,ls2k0500-spi" as a compatible in spi driver and
>> added it as a part of the one enum in dt-binding.
> 
> No, because you claimed - if I understood correctly - that they are
> compatible. Don't add fake entries to the driver.
> 


I'm a bit confused, and I just need to add 'loongson,ls2k0500-spi' as
one enum in dt-bindings, but driver don't add this entry ?

Thanks,
Yinbo


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-12  7:40                             ` zhuyinbo
@ 2023-06-12  8:16                               ` Krzysztof Kozlowski
  2023-06-12 11:29                                 ` zhuyinbo
  0 siblings, 1 reply; 29+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-12  8:16 UTC (permalink / raw)
  To: zhuyinbo, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel

On 12/06/2023 09:40, zhuyinbo wrote:
> 
> 
> 在 2023/6/12 下午3:17, Krzysztof Kozlowski 写道:
>> On 12/06/2023 09:13, zhuyinbo wrote:
>>>
>>>
>>> 在 2023/6/10 上午12:45, Krzysztof Kozlowski 写道:
>>>> On 09/06/2023 05:13, zhuyinbo wrote:
>>>>>
>>>>>
>>>>> 在 2023/6/8 下午9:26, Krzysztof Kozlowski 写道:
>>>>>> On 08/06/2023 14:10, zhuyinbo wrote:
>>>>>>>
>>>>>>>
>>>>>>> 在 2023/6/8 下午7:45, Krzysztof Kozlowski 写道:
>>>>>>>> On 08/06/2023 13:42, zhuyinbo wrote:
>>>>>>>>> --- a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>>>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>>>>> @@ -16,6 +16,7 @@ properties:
>>>>>>>>>         compatible:
>>>>>>>>>           enum:
>>>>>>>>>             - loongson,ls2k1000-spi
>>>>>>>>> +      - loongson,ls2k0500-spi
>>>>>>>>
>>>>>>>> Aren't they compatible?
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Are you saying that the spi driver is compatible with 2k0500 ?
>>>>>>
>>>>>> Didn't you say this through 11 previous revisions?
>>>>>
>>>>>
>>>>> Yes, did I understand your meaning incorrectly ?
>>>>
>>>> If they are compatible, then they are not part of one enum. They could
>>>> not be as this would easily fail in testing of your DTS.
>>>>
>>>
>>>
>>> The "loongson,ls2k0500-spi" wasn't a compatible in previous version and
>>> I will add "loongson,ls2k0500-spi" as a compatible in spi driver and
>>> added it as a part of the one enum in dt-binding.
>>
>> No, because you claimed - if I understood correctly - that they are
>> compatible. Don't add fake entries to the driver.
>>
> 
> 
> I'm a bit confused, and I just need to add 'loongson,ls2k0500-spi' as
> one enum in dt-bindings, but driver don't add this entry ?

Compatibility is expressed with a list:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#compatible
so it cannot be just one enum, but "items". There are hundreds of
examples including example-schema.


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-12  8:16                               ` Krzysztof Kozlowski
@ 2023-06-12 11:29                                 ` zhuyinbo
  2023-06-12 18:03                                   ` Krzysztof Kozlowski
  0 siblings, 1 reply; 29+ messages in thread
From: zhuyinbo @ 2023-06-12 11:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo



在 2023/6/12 下午4:16, Krzysztof Kozlowski 写道:
>>>>>>>> 在 2023/6/8 下午7:45, Krzysztof Kozlowski 写道:
>>>>>>>>> On 08/06/2023 13:42, zhuyinbo wrote:
>>>>>>>>>> --- a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>>>>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>>>>>> @@ -16,6 +16,7 @@ properties:
>>>>>>>>>>          compatible:
>>>>>>>>>>            enum:
>>>>>>>>>>              - loongson,ls2k1000-spi
>>>>>>>>>> +      - loongson,ls2k0500-spi
>>>>>>>>>
>>>>>>>>> Aren't they compatible?
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Are you saying that the spi driver is compatible with 2k0500 ?
>>>>>>>
>>>>>>> Didn't you say this through 11 previous revisions?
>>>>>>
>>>>>>
>>>>>> Yes, did I understand your meaning incorrectly ?
>>>>>
>>>>> If they are compatible, then they are not part of one enum. They could
>>>>> not be as this would easily fail in testing of your DTS.
>>>>>
>>>>
>>>>
>>>> The "loongson,ls2k0500-spi" wasn't a compatible in previous version and
>>>> I will add "loongson,ls2k0500-spi" as a compatible in spi driver and
>>>> added it as a part of the one enum in dt-binding.
>>>
>>> No, because you claimed - if I understood correctly - that they are
>>> compatible. Don't add fake entries to the driver.
>>>
>>
>>
>> I'm a bit confused, and I just need to add 'loongson,ls2k0500-spi' as
>> one enum in dt-bindings, but driver don't add this entry ?
> 
> Compatibility is expressed with a list:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#compatible
> so it cannot be just one enum, but "items". There are hundreds of
> examples including example-schema.


Is it a description like the following?

  properties:
    compatible:
-    enum:
-      - loongson,ls2k1000-spi
+    oneOf:
+      - enum:
+          - loongson,ls2k1000-spi
+      - items:
+          - enum:
+              - loongson,ls2k1000-spi
+          - const: loongson,ls2k1000-spi
+      - items:
+          - enum:
+              - loongson,ls2k0500-spi
+          - const: loongson,ls2k1000-spi

    reg:


Thanks,
Yinbo


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-12 11:29                                 ` zhuyinbo
@ 2023-06-12 18:03                                   ` Krzysztof Kozlowski
  2023-06-13  2:04                                     ` zhuyinbo
  0 siblings, 1 reply; 29+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-12 18:03 UTC (permalink / raw)
  To: zhuyinbo, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel

On 12/06/2023 13:29, zhuyinbo wrote:
> 
> 
> 在 2023/6/12 下午4:16, Krzysztof Kozlowski 写道:
>>>>>>>>> 在 2023/6/8 下午7:45, Krzysztof Kozlowski 写道:
>>>>>>>>>> On 08/06/2023 13:42, zhuyinbo wrote:
>>>>>>>>>>> --- a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>>>>>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>>>>>>> @@ -16,6 +16,7 @@ properties:
>>>>>>>>>>>          compatible:
>>>>>>>>>>>            enum:
>>>>>>>>>>>              - loongson,ls2k1000-spi
>>>>>>>>>>> +      - loongson,ls2k0500-spi
>>>>>>>>>>
>>>>>>>>>> Aren't they compatible?
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Are you saying that the spi driver is compatible with 2k0500 ?
>>>>>>>>
>>>>>>>> Didn't you say this through 11 previous revisions?
>>>>>>>
>>>>>>>
>>>>>>> Yes, did I understand your meaning incorrectly ?
>>>>>>
>>>>>> If they are compatible, then they are not part of one enum. They could
>>>>>> not be as this would easily fail in testing of your DTS.
>>>>>>
>>>>>
>>>>>
>>>>> The "loongson,ls2k0500-spi" wasn't a compatible in previous version and
>>>>> I will add "loongson,ls2k0500-spi" as a compatible in spi driver and
>>>>> added it as a part of the one enum in dt-binding.
>>>>
>>>> No, because you claimed - if I understood correctly - that they are
>>>> compatible. Don't add fake entries to the driver.
>>>>
>>>
>>>
>>> I'm a bit confused, and I just need to add 'loongson,ls2k0500-spi' as
>>> one enum in dt-bindings, but driver don't add this entry ?
>>
>> Compatibility is expressed with a list:
>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#compatible
>> so it cannot be just one enum, but "items". There are hundreds of
>> examples including example-schema.
> 
> 
> Is it a description like the following?
> 
>   properties:
>     compatible:
> -    enum:
> -      - loongson,ls2k1000-spi
> +    oneOf:
> +      - enum:
> +          - loongson,ls2k1000-spi
> +      - items:
> +          - enum:
> +              - loongson,ls2k1000-spi
> +          - const: loongson,ls2k1000-spi

Remove this items part - it does not make sense. Device is not
compatible with itself. Rest looks ok.



Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH v12 1/2] spi: add loongson spi bindings
  2023-06-12 18:03                                   ` Krzysztof Kozlowski
@ 2023-06-13  2:04                                     ` zhuyinbo
  0 siblings, 0 replies; 29+ messages in thread
From: zhuyinbo @ 2023-06-13  2:04 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, linux-spi, devicetree, linux-kernel
  Cc: Jianmin Lv, wanghongliang, Liu Peibao, loongson-kernel, zhuyinbo



在 2023/6/13 上午2:03, Krzysztof Kozlowski 写道:
> On 12/06/2023 13:29, zhuyinbo wrote:
>>
>>
>> 在 2023/6/12 下午4:16, Krzysztof Kozlowski 写道:
>>>>>>>>>> 在 2023/6/8 下午7:45, Krzysztof Kozlowski 写道:
>>>>>>>>>>> On 08/06/2023 13:42, zhuyinbo wrote:
>>>>>>>>>>>> --- a/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>>>>>>>> +++ b/Documentation/devicetree/bindings/spi/loongson,ls2k-spi.yaml
>>>>>>>>>>>> @@ -16,6 +16,7 @@ properties:
>>>>>>>>>>>>           compatible:
>>>>>>>>>>>>             enum:
>>>>>>>>>>>>               - loongson,ls2k1000-spi
>>>>>>>>>>>> +      - loongson,ls2k0500-spi
>>>>>>>>>>>
>>>>>>>>>>> Aren't they compatible?
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Are you saying that the spi driver is compatible with 2k0500 ?
>>>>>>>>>
>>>>>>>>> Didn't you say this through 11 previous revisions?
>>>>>>>>
>>>>>>>>
>>>>>>>> Yes, did I understand your meaning incorrectly ?
>>>>>>>
>>>>>>> If they are compatible, then they are not part of one enum. They could
>>>>>>> not be as this would easily fail in testing of your DTS.
>>>>>>>
>>>>>>
>>>>>>
>>>>>> The "loongson,ls2k0500-spi" wasn't a compatible in previous version and
>>>>>> I will add "loongson,ls2k0500-spi" as a compatible in spi driver and
>>>>>> added it as a part of the one enum in dt-binding.
>>>>>
>>>>> No, because you claimed - if I understood correctly - that they are
>>>>> compatible. Don't add fake entries to the driver.
>>>>>
>>>>
>>>>
>>>> I'm a bit confused, and I just need to add 'loongson,ls2k0500-spi' as
>>>> one enum in dt-bindings, but driver don't add this entry ?
>>>
>>> Compatibility is expressed with a list:
>>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#compatible
>>> so it cannot be just one enum, but "items". There are hundreds of
>>> examples including example-schema.
>>
>>
>> Is it a description like the following?
>>
>>    properties:
>>      compatible:
>> -    enum:
>> -      - loongson,ls2k1000-spi
>> +    oneOf:
>> +      - enum:
>> +          - loongson,ls2k1000-spi
>> +      - items:
>> +          - enum:
>> +              - loongson,ls2k1000-spi
>> +          - const: loongson,ls2k1000-spi
> 
> Remove this items part - it does not make sense. Device is not
> compatible with itself. Rest looks ok.


okay, I got it.

Thanks,
Yinbo


^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2023-06-13  2:05 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-08  7:28 [PATCH v12 0/2] spi: loongson: add bus driver for the loongson spi Yinbo Zhu
2023-06-08  7:28 ` [PATCH v12 1/2] spi: add loongson spi bindings Yinbo Zhu
2023-06-08  7:45   ` Krzysztof Kozlowski
2023-06-08  8:39     ` zhuyinbo
2023-06-08  8:53       ` Krzysztof Kozlowski
2023-06-08 10:00         ` zhuyinbo
2023-06-08 10:02           ` Krzysztof Kozlowski
2023-06-08 11:42             ` zhuyinbo
2023-06-08 11:45               ` Krzysztof Kozlowski
2023-06-08 12:10                 ` zhuyinbo
2023-06-08 13:26                   ` Krzysztof Kozlowski
2023-06-09  3:13                     ` zhuyinbo
2023-06-09 16:45                       ` Krzysztof Kozlowski
2023-06-12  7:13                         ` zhuyinbo
2023-06-12  7:17                           ` Krzysztof Kozlowski
2023-06-12  7:40                             ` zhuyinbo
2023-06-12  8:16                               ` Krzysztof Kozlowski
2023-06-12 11:29                                 ` zhuyinbo
2023-06-12 18:03                                   ` Krzysztof Kozlowski
2023-06-13  2:04                                     ` zhuyinbo
2023-06-08  7:28 ` [PATCH v12 2/2] spi: loongson: add bus driver for the loongson spi controller Yinbo Zhu
2023-06-08 10:15   ` Andy Shevchenko
2023-06-08 10:18     ` Andy Shevchenko
2023-06-09  7:13       ` zhuyinbo
2023-06-08 10:29     ` Mark Brown
2023-06-08 11:45       ` zhuyinbo
2023-06-08 11:53         ` Mark Brown
2023-06-09  3:17           ` zhuyinbo
2023-06-09  7:10     ` zhuyinbo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).