All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] mmc: Add LiteSDCard mmc driver
@ 2021-12-08 13:20 Gabriel Somlo
  2021-12-08 13:20 ` [PATCH v3 1/3] MAINTAINERS: co-maintain LiteX platform Gabriel Somlo
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Gabriel Somlo @ 2021-12-08 13:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: robh+dt, devicetree, ulf.hansson, linux-mmc, kgugala, mholenko,
	krakoczy, mdudek, paulus, joel, shorne, geert,
	david.abdurachmanov, florent, rdunlap

Add support for the LiteX SD-Card device, LiteSDCard.

LiteSDCard is a simple SD-Card interface available as part of the LiteX
environment, used with various RISC-V and other FPGA based SoCs.

New in v3:

  MAINTAINERS:

  - picked up acked-by Joel
  - added listing for liteeth driver
  - added Joel as additional co-maintainer (thanks!)

  Doc/dt/bindings/mmc/litex,mmc.yaml:

  - picked up r/b Geert Uytterhoeven <geert@linux-m68k.org> in DT
    bindings document (please let me know if that was premature, and
    happy to take further review if needed :)
  - add dedicated DT property for source clock frequency

  drivers/mmc/host/litex_mmc.c:

  - fixed function signature (no line split), and naming (litex_mmc_*)
  - more informative MODULE_AUTHOR() entries
    - also added matching "Copyright" entries in file header
  - fixed description in Kconfig
  - fixed DT documentation
  - removed magic constants
  - removed litex_map_status(), have sdcard_wait_done() return *real*
    error codes directly instead.
  - streamlined litex_mmc_reponse_len()
  - call litex_mmc_set_bus_width() only once, and ensure it returns
    correct error code(s)
  - use readx_poll_timeout() -- more concise -- instead of
    read_poll_timeout()
  - use dev_err() in litex_mmc_send_cmd() (instead of pr_err())
  - litex_mmc_setclk() will update host->clock before returning
  - separate irq initialization into its own function,
    litex_mmc_irq_init()
  - document rationale for f_min, f_max
  - use dmam_alloc_coherent(), which simplifies cleanup significantly
  - large `if (data) { ... }` block in litex_mmc_request() left as-is,
    there are too many variables shared with the rest of the parent
    function body to easily separate (e.g., `len`, `transfer`, `direct`).
    If this is indeed a blocker, I can take another shot at refactoring
    it in a future revision!
  - bump dma_set_mask_and_coherent() to 64-bits on suitable
    architectures
  - clock source picked up from dedicated DT clock reference property
  - remove gpio card-detect logic (needs testing and a dt binding
    example before being eligible for upstream inclusion)

> New in v2:
>   - reword info message in litex_set_clk()
>   - streamline code in litex_map_status()
>   - fix typos in Kconfig (thanks Randy Dunlap <rdunlap@infradead.org>)
>   - improvements suggested by Stafford Horne <shorne@gmail.com>
>     - allow COMPILE_TEST in Kconfig
>     - use read_poll_timeout() when waiting for cmd/data/DMA
>       xfer completion
>   - include interrupt.h (thanks kernel test robot <lkp@intel.com>)

Gabriel Somlo (3):
  MAINTAINERS: co-maintain LiteX platform
  dt-bindings: mmc: Add bindings for LiteSDCard
  mmc: Add driver for LiteX's LiteSDCard interface

 .../devicetree/bindings/mmc/litex,mmc.yaml    |  72 ++
 MAINTAINERS                                   |   9 +-
 drivers/mmc/host/Kconfig                      |   9 +
 drivers/mmc/host/Makefile                     |   1 +
 drivers/mmc/host/litex_mmc.c                  | 644 ++++++++++++++++++
 5 files changed, 733 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/litex,mmc.yaml
 create mode 100644 drivers/mmc/host/litex_mmc.c

-- 
2.31.1


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

* [PATCH v3 1/3] MAINTAINERS: co-maintain LiteX platform
  2021-12-08 13:20 [PATCH v3 0/3] mmc: Add LiteSDCard mmc driver Gabriel Somlo
@ 2021-12-08 13:20 ` Gabriel Somlo
  2021-12-08 13:20 ` [PATCH v3 2/3] dt-bindings: mmc: Add bindings for LiteSDCard Gabriel Somlo
  2021-12-08 13:20 ` [PATCH v3 3/3] mmc: Add driver for LiteX's LiteSDCard interface Gabriel Somlo
  2 siblings, 0 replies; 9+ messages in thread
From: Gabriel Somlo @ 2021-12-08 13:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: robh+dt, devicetree, ulf.hansson, linux-mmc, kgugala, mholenko,
	krakoczy, mdudek, paulus, joel, shorne, geert,
	david.abdurachmanov, florent, rdunlap

Add the litex_mmc (LiteSDCard) and LiteETH drivers to the list of files
maintained under LiteX.

Add Gabriel Somlo and Joel Stanley as maintainers; Joel authored
the LiteETH driver, and Gabriel is currently curating the LiteX
out-of-tree device drivers as they are tested and prepared for
upstream submission, having also co-authored a number of them.

Cc: Karol Gugala <kgugala@antmicro.com>
Cc: Mateusz Holenko <mholenko@antmicro.com>
Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
Acked-by: Joel Stanley <joel@jms.id.au>
---

New in v3:
  - picked up acked-by Joel
  - added listing for liteeth driver
  - added Joel as additional co-maintainer (thanks!)

 MAINTAINERS | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 43007f2d29e0..477d993f70df 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11012,12 +11012,17 @@ F:	lib/list-test.c
 LITEX PLATFORM
 M:	Karol Gugala <kgugala@antmicro.com>
 M:	Mateusz Holenko <mholenko@antmicro.com>
+M:	Gabriel Somlo <gsomlo@gmail.com>
+M:	Joel Stanley <joel@jms.id.au>
 S:	Maintained
 F:	Documentation/devicetree/bindings/*/litex,*.yaml
 F:	arch/openrisc/boot/dts/or1klitex.dts
-F:	drivers/soc/litex/litex_soc_ctrl.c
-F:	drivers/tty/serial/liteuart.c
 F:	include/linux/litex.h
+F:	drivers/tty/serial/liteuart.c
+F:	drivers/soc/litex/*
+F:	drivers/net/ethernet/litex/*
+F:	drivers/mmc/host/litex_mmc.c
+N:	litex
 
 LIVE PATCHING
 M:	Josh Poimboeuf <jpoimboe@redhat.com>
-- 
2.31.1


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

* [PATCH v3 2/3] dt-bindings: mmc: Add bindings for LiteSDCard
  2021-12-08 13:20 [PATCH v3 0/3] mmc: Add LiteSDCard mmc driver Gabriel Somlo
  2021-12-08 13:20 ` [PATCH v3 1/3] MAINTAINERS: co-maintain LiteX platform Gabriel Somlo
@ 2021-12-08 13:20 ` Gabriel Somlo
  2021-12-08 23:06   ` Rob Herring
  2021-12-08 13:20 ` [PATCH v3 3/3] mmc: Add driver for LiteX's LiteSDCard interface Gabriel Somlo
  2 siblings, 1 reply; 9+ messages in thread
From: Gabriel Somlo @ 2021-12-08 13:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: robh+dt, devicetree, ulf.hansson, linux-mmc, kgugala, mholenko,
	krakoczy, mdudek, paulus, joel, shorne, geert,
	david.abdurachmanov, florent, rdunlap

LiteSDCard is a small footprint, configurable SDCard core for FPGA
based system on chips.

Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
---

New in v3:
  - picked up r/b Geert Uytterhoeven <geert@linux-m68k.org> in DT
    bindings document (please let me know if that was premature, and
    happy to take further review if needed :)
  - add dedicated DT property for source clock frequency

 .../devicetree/bindings/mmc/litex,mmc.yaml    | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/litex,mmc.yaml

diff --git a/Documentation/devicetree/bindings/mmc/litex,mmc.yaml b/Documentation/devicetree/bindings/mmc/litex,mmc.yaml
new file mode 100644
index 000000000000..9b8d4ec30375
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/litex,mmc.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: GPL-2.0-or-later OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/litex,mmc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LiteX LiteSDCard device
+
+maintainers:
+  - Gabriel Somlo <gsomlo@gmail.com>
+
+description: |
+  LiteSDCard is a small footprint, configurable SDCard core for FPGA based
+  system on chips.
+
+  The hardware source is Open Source and can be found on at
+  https://github.com/enjoy-digital/litesdcard/.
+
+allOf:
+  - $ref: mmc-controller.yaml#
+
+properties:
+  compatible:
+    const: litex,mmc
+
+  reg:
+    items:
+      - description: PHY registers
+      - description: CORE registers
+      - description: DMA Reader buffer
+      - description: DMA Writer buffer
+      - description: IRQ registers (optional)
+    minItems: 4
+    maxItems: 5
+
+  reg-names:
+    items:
+      - const: phy
+      - const: core
+      - const: reader
+      - const: writer
+      - const: irq (optional)
+    minItems: 4
+    maxItems: 5
+
+  clocks:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    mmc: mmc@12005000 {
+        compatible = "litex,mmc";
+        reg = <0x12005000 0x100>,
+              <0x12003800 0x100>,
+              <0x12003000 0x100>,
+              <0x12004800 0x100>,
+              <0x12004000 0x100>;
+        reg-names = "phy", "core", "reader", "writer", "irq";
+        clocks = <&reference_clk>;
+        interrupts = <4>;
+    };
-- 
2.31.1


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

* [PATCH v3 3/3] mmc: Add driver for LiteX's LiteSDCard interface
  2021-12-08 13:20 [PATCH v3 0/3] mmc: Add LiteSDCard mmc driver Gabriel Somlo
  2021-12-08 13:20 ` [PATCH v3 1/3] MAINTAINERS: co-maintain LiteX platform Gabriel Somlo
  2021-12-08 13:20 ` [PATCH v3 2/3] dt-bindings: mmc: Add bindings for LiteSDCard Gabriel Somlo
@ 2021-12-08 13:20 ` Gabriel Somlo
  2021-12-25 16:39   ` Andy Shevchenko
  2 siblings, 1 reply; 9+ messages in thread
From: Gabriel Somlo @ 2021-12-08 13:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: robh+dt, devicetree, ulf.hansson, linux-mmc, kgugala, mholenko,
	krakoczy, mdudek, paulus, joel, shorne, geert,
	david.abdurachmanov, florent, rdunlap

LiteX (https://github.com/enjoy-digital/litex) is a SoC framework
that targets FPGAs. LiteSDCard is a small footprint, configurable
SDCard core commonly used in LiteX designs.

The driver was first written in May 2020 and has been maintained
cooperatively by the LiteX community. Thanks to all contributors!

Co-developed-by: Kamil Rakoczy <krakoczy@antmicro.com>
Signed-off-by: Kamil Rakoczy <krakoczy@antmicro.com>
Co-developed-by: Maciej Dudek <mdudek@internships.antmicro.com>
Signed-off-by: Maciej Dudek <mdudek@internships.antmicro.com>
Co-developed-by: Paul Mackerras <paulus@ozlabs.org>
Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
Cc: Mateusz Holenko <mholenko@antmicro.com>
Cc: Karol Gugala <kgugala@antmicro.com>
Cc: Joel Stanley <joel@jms.id.au>
Cc: Stafford Horne <shorne@gmail.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: David Abdurachmanov <david.abdurachmanov@sifive.com>
Cc: Florent Kermarrec <florent@enjoy-digital.fr>
---

New in v3:
  - fixed function signature (no line split), and naming (litex_mmc_*)
  - more informative MODULE_AUTHOR() entries
    - also added matching "Copyright" entries in file header
  - fixed description and dependencies in Kconfig
  - removed magic constants
  - removed litex_map_status(), have sdcard_wait_done() return *real*
    error codes directly instead.
  - streamlined litex_mmc_reponse_len()
  - call litex_mmc_set_bus_width() only once, and ensure it returns
    correct error code(s)
  - use readx_poll_timeout() -- more concise -- instead of
    read_poll_timeout()
  - use dev_err() in litex_mmc_send_cmd() (instead of pr_err())
  - litex_mmc_setclk() will update host->clock before returning
  - separate irq initialization into its own function,
    litex_mmc_irq_init()
  - document rationale for f_min, f_max
  - use dmam_alloc_coherent(), which simplifies cleanup significantly
  - bump dma_set_mask_and_coherent() to 64-bits on suitable
    architectures
  - clock source picked up from dedicated DT clock reference property
  - remove gpio card-detect logic (needs testing and a dt binding
    example before being eligible for upstream inclusion)
  - large `if (data) { ... }` block in litex_mmc_request() left as-is,
    there are too many variables shared with the rest of the parent
    function body to easily separate (e.g., `len`, `transfer`, `direct`).
    If this is indeed a blocker, I can take another shot at refactoring
    it in a future revision!

 drivers/mmc/host/Kconfig     |   9 +
 drivers/mmc/host/Makefile    |   1 +
 drivers/mmc/host/litex_mmc.c | 644 +++++++++++++++++++++++++++++++++++
 3 files changed, 654 insertions(+)
 create mode 100644 drivers/mmc/host/litex_mmc.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 5af8494c31b5..c1b66d06d1c9 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -1093,3 +1093,12 @@ config MMC_OWL
 
 config MMC_SDHCI_EXTERNAL_DMA
 	bool
+
+config MMC_LITEX
+	tristate "LiteX MMC Host Controller support"
+	depends on OF
+	depends on PPC_MICROWATT || LITEX || COMPILE_TEST
+	help
+	  This selects support for the MMC Host Controller found in LiteX SoCs.
+
+	  If unsure, say N.
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index ea36d379bd3c..4e4ceb32c4b4 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_MMC_CQHCI)			+= cqhci.o
 cqhci-y					+= cqhci-core.o
 cqhci-$(CONFIG_MMC_CRYPTO)		+= cqhci-crypto.o
 obj-$(CONFIG_MMC_HSQ)			+= mmc_hsq.o
+obj-$(CONFIG_MMC_LITEX)			+= litex_mmc.o
 
 ifeq ($(CONFIG_CB710_DEBUG),y)
 	CFLAGS-cb710-mmc	+= -DDEBUG
diff --git a/drivers/mmc/host/litex_mmc.c b/drivers/mmc/host/litex_mmc.c
new file mode 100644
index 000000000000..cef24a3475bd
--- /dev/null
+++ b/drivers/mmc/host/litex_mmc.c
@@ -0,0 +1,644 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LiteX LiteSDCard driver
+ *
+ * Copyright (C) 2019-2020 Antmicro <contact@antmicro.com>
+ * Copyright (C) 2019-2020 Kamil Rakoczy <krakoczy@antmicro.com>
+ * Copyright (C) 2019-2020 Maciej Dudek <mdudek@internships.antmicro.com>
+ * Copyright (C) 2020 Paul Mackerras <paulus@ozlabs.org>
+ * Copyright (C) 2020-2021 Gabriel Somlo <gsomlo@gmail.com>
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/litex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/iopoll.h>
+#include <linux/mmc/sd.h>
+#include <linux/mmc/mmc.h>
+#include <linux/mmc/host.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+
+#define LITEX_PHY_CARDDETECT  0x00
+#define LITEX_PHY_CLOCKERDIV  0x04
+#define LITEX_PHY_INITIALIZE  0x08
+#define LITEX_PHY_WRITESTATUS 0x0C
+#define LITEX_CORE_CMDARG     0x00
+#define LITEX_CORE_CMDCMD     0x04
+#define LITEX_CORE_CMDSND     0x08
+#define LITEX_CORE_CMDRSP     0x0C
+#define LITEX_CORE_CMDEVT     0x1C
+#define LITEX_CORE_DATAEVT    0x20
+#define LITEX_CORE_BLKLEN     0x24
+#define LITEX_CORE_BLKCNT     0x28
+#define LITEX_BLK2MEM_BASE    0x00
+#define LITEX_BLK2MEM_LEN     0x08
+#define LITEX_BLK2MEM_ENA     0x0C
+#define LITEX_BLK2MEM_DONE    0x10
+#define LITEX_BLK2MEM_LOOP    0x14
+#define LITEX_MEM2BLK_BASE    0x00
+#define LITEX_MEM2BLK_LEN     0x08
+#define LITEX_MEM2BLK_ENA     0x0C
+#define LITEX_MEM2BLK_DONE    0x10
+#define LITEX_MEM2BLK_LOOP    0x14
+#define LITEX_MEM2BLK         0x18
+#define LITEX_IRQ_STATUS      0x00
+#define LITEX_IRQ_PENDING     0x04
+#define LITEX_IRQ_ENABLE      0x08
+
+#define SDCARD_CTRL_DATA_TRANSFER_NONE  0
+#define SDCARD_CTRL_DATA_TRANSFER_READ  1
+#define SDCARD_CTRL_DATA_TRANSFER_WRITE 2
+
+#define SDCARD_CTRL_RESPONSE_NONE       0
+#define SDCARD_CTRL_RESPONSE_SHORT      1
+#define SDCARD_CTRL_RESPONSE_LONG       2
+#define SDCARD_CTRL_RESPONSE_SHORT_BUSY 3
+
+#define SD_BIT_DONE    BIT(0)
+#define SD_BIT_WR_ERR  BIT(1)
+#define SD_BIT_TIMEOUT BIT(2)
+#define SD_BIT_CRC_ERR BIT(3)
+
+#define SD_SLEEP_US       5
+#define SD_TIMEOUT_US 20000
+
+#define SDIRQ_CARD_DETECT    1
+#define SDIRQ_SD_TO_MEM_DONE 2
+#define SDIRQ_MEM_TO_SD_DONE 4
+#define SDIRQ_CMD_DONE       8
+
+struct litex_mmc_host {
+	struct mmc_host *mmc;
+	struct platform_device *dev;
+
+	void __iomem *sdphy;
+	void __iomem *sdcore;
+	void __iomem *sdreader;
+	void __iomem *sdwriter;
+	void __iomem *sdirq;
+
+	void *buffer;
+	size_t buf_size;
+	dma_addr_t dma;
+
+	struct completion cmd_done;
+	int irq;
+
+	unsigned int ref_clk;
+	unsigned int sd_clk;
+	bool is_bus_width_set;
+	bool app_cmd;
+
+	u32 resp[4];
+	u16 rca;
+};
+
+static int litex_mmc_sdcard_wait_done(void __iomem *reg)
+{
+	u8 evt;
+	int ret;
+
+	ret = readx_poll_timeout(litex_read8, reg, evt, (evt & SD_BIT_DONE),
+				 SD_SLEEP_US, SD_TIMEOUT_US);
+	if (ret || (evt & SD_BIT_TIMEOUT))
+		return -ETIMEDOUT;
+	if (evt == SD_BIT_DONE)
+		return 0;
+	if (evt & SD_BIT_WR_ERR)
+		return -EIO;
+	if (evt & SD_BIT_CRC_ERR)
+		return -EILSEQ;
+	pr_err("%s: unknown error evt=%x\n", __func__, evt);
+	return -EINVAL;
+}
+
+static int litex_mmc_send_cmd(struct litex_mmc_host *host,
+			      u8 cmd, u32 arg, u8 response_len, u8 transfer)
+{
+	struct device *dev = mmc_dev(host->mmc);
+	void __iomem *reg;
+	int ret;
+	u8 evt;
+
+	litex_write32(host->sdcore + LITEX_CORE_CMDARG, arg);
+	litex_write32(host->sdcore + LITEX_CORE_CMDCMD,
+		      cmd << 8 | transfer << 5 | response_len);
+	litex_write8(host->sdcore + LITEX_CORE_CMDSND, 1);
+
+	/* Wait for an interrupt if we have an interrupt and either there is
+	 * data to be transferred, or if the card can report busy via DAT0.
+	 */
+	if (host->irq > 0 &&
+	    (transfer != SDCARD_CTRL_DATA_TRANSFER_NONE ||
+	     response_len == SDCARD_CTRL_RESPONSE_SHORT_BUSY)) {
+		reinit_completion(&host->cmd_done);
+		litex_write32(host->sdirq + LITEX_IRQ_ENABLE,
+			      SDIRQ_CMD_DONE | SDIRQ_CARD_DETECT);
+		wait_for_completion(&host->cmd_done);
+	}
+
+	ret = litex_mmc_sdcard_wait_done(host->sdcore + LITEX_CORE_CMDEVT);
+	if (ret) {
+		dev_err(dev, "Command (cmd %d) error, status %d\n", cmd, ret);
+		return ret;
+	}
+
+	if (response_len != SDCARD_CTRL_RESPONSE_NONE) {
+		int i;
+
+		reg = host->sdcore + LITEX_CORE_CMDRSP;
+		for (i = 0; i < 4; i++) {
+			host->resp[i] = litex_read32(reg);
+			reg += sizeof(u32);
+		}
+	}
+
+	if (!host->app_cmd && cmd == SD_SEND_RELATIVE_ADDR)
+		host->rca = (host->resp[3] >> 16) & 0xffff;
+
+	host->app_cmd = (cmd == MMC_APP_CMD);
+
+	if (transfer == SDCARD_CTRL_DATA_TRANSFER_NONE)
+		return ret; /* OK from prior litex_mmc_sdcard_wait_done() */
+
+	ret = litex_mmc_sdcard_wait_done(host->sdcore + LITEX_CORE_DATAEVT);
+	if (ret) {
+		dev_err(dev, "Data xfer (cmd %d) error, status %d\n", cmd, ret);
+		return ret;
+	}
+
+	/* wait for completion of (read or write) DMA transfer */
+	reg = (transfer == SDCARD_CTRL_DATA_TRANSFER_READ) ?
+		host->sdreader + LITEX_BLK2MEM_DONE :
+		host->sdwriter + LITEX_MEM2BLK_DONE;
+	ret = readx_poll_timeout(litex_read8, reg, evt, (evt & SD_BIT_DONE),
+				 SD_SLEEP_US, SD_TIMEOUT_US);
+	if (ret)
+		dev_err(dev, "DMA timeout (cmd %d)\n", cmd);
+
+	return ret;
+}
+
+static int litex_mmc_send_app_cmd(struct litex_mmc_host *host)
+{
+	return litex_mmc_send_cmd(host, MMC_APP_CMD, host->rca << 16,
+				  SDCARD_CTRL_RESPONSE_SHORT,
+				  SDCARD_CTRL_DATA_TRANSFER_NONE);
+}
+
+static int litex_mmc_send_set_bus_w_cmd(struct litex_mmc_host *host, u32 width)
+{
+	return litex_mmc_send_cmd(host, SD_APP_SET_BUS_WIDTH, width,
+				  SDCARD_CTRL_RESPONSE_SHORT,
+				  SDCARD_CTRL_DATA_TRANSFER_NONE);
+}
+
+static int litex_mmc_set_bus_width(struct litex_mmc_host *host)
+{
+	bool app_cmd_sent = host->app_cmd; /* was preceding command app_cmd? */
+	int ret;
+
+	/* ensure 'app_cmd' precedes 'app_set_bus_width_cmd' */
+	if (!app_cmd_sent) {
+		ret = litex_mmc_send_app_cmd(host);
+		if (ret)
+			goto out;
+	}
+
+	/* litesdcard only supports 4-bit bus width */
+	ret = litex_mmc_send_set_bus_w_cmd(host, MMC_BUS_WIDTH_4);
+	if (ret)
+		goto out;
+
+	/* re-send 'app_cmd' if necessary */
+	if (app_cmd_sent)
+		ret = litex_mmc_send_app_cmd(host);
+
+out:
+	return ret;
+}
+
+static int litex_mmc_get_cd(struct mmc_host *mmc)
+{
+	struct litex_mmc_host *host = mmc_priv(mmc);
+	int ret;
+
+	if (!mmc_card_is_removable(mmc))
+		return 1;
+
+	ret = !litex_read8(host->sdphy + LITEX_PHY_CARDDETECT);
+
+	/* ensure bus width will be set (again) upon card (re)insertion */
+	if (ret == 0)
+		host->is_bus_width_set = false;
+
+	return ret;
+}
+
+static irqreturn_t litex_mmc_interrupt(int irq, void *arg)
+{
+	struct mmc_host *mmc = arg;
+	struct litex_mmc_host *host = mmc_priv(mmc);
+	u32 pending = litex_read32(host->sdirq + LITEX_IRQ_PENDING);
+
+	/* Check for card change interrupt */
+	if (pending & SDIRQ_CARD_DETECT) {
+		litex_write32(host->sdirq + LITEX_IRQ_PENDING,
+			      SDIRQ_CARD_DETECT);
+		mmc_detect_change(mmc, msecs_to_jiffies(10));
+	}
+
+	/* Check for command completed */
+	if (pending & SDIRQ_CMD_DONE) {
+		/* Disable it so it doesn't keep interrupting */
+		litex_write32(host->sdirq + LITEX_IRQ_ENABLE,
+			      SDIRQ_CARD_DETECT);
+		complete(&host->cmd_done);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static u32 litex_mmc_response_len(struct mmc_command *cmd)
+{
+	if (cmd->flags & MMC_RSP_136)
+		return SDCARD_CTRL_RESPONSE_LONG;
+	if (!(cmd->flags & MMC_RSP_PRESENT))
+		return SDCARD_CTRL_RESPONSE_NONE;
+	if (cmd->flags & MMC_RSP_BUSY)
+		return SDCARD_CTRL_RESPONSE_SHORT_BUSY;
+	return SDCARD_CTRL_RESPONSE_SHORT;
+}
+
+static void litex_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+	struct litex_mmc_host *host = mmc_priv(mmc);
+	struct device *dev = mmc_dev(mmc);
+	struct mmc_command *cmd = mrq->cmd;
+	struct mmc_command *sbc = mrq->sbc;
+	struct mmc_data *data = mrq->data;
+	struct mmc_command *stop = mrq->stop;
+	unsigned int retries = cmd->retries;
+	unsigned int len = 0;
+	bool direct = false;
+	dma_addr_t dma;
+	int sg_count;
+
+	u32 response_len = litex_mmc_response_len(cmd);
+	u32 transfer = SDCARD_CTRL_DATA_TRANSFER_NONE;
+
+	/* First check that the card is still there */
+	if (!litex_mmc_get_cd(mmc)) {
+		cmd->error = -ENOMEDIUM;
+		mmc_request_done(mmc, mrq);
+		return;
+	}
+
+	/* Send set-block-count command if needed */
+	if (sbc) {
+		sbc->error = litex_mmc_send_cmd(host, sbc->opcode, sbc->arg,
+						litex_mmc_response_len(sbc),
+						SDCARD_CTRL_DATA_TRANSFER_NONE);
+		if (sbc->error) {
+			host->is_bus_width_set = false;
+			mmc_request_done(mmc, mrq);
+			return;
+		}
+	}
+
+	if (data) {
+		/* LiteSDCard only supports 4-bit bus width; therefore, we MUST
+		 * inject a SET_BUS_WIDTH (acmd6) before the very first data
+		 * transfer, earlier than when the mmc subsystem would normally
+		 * get around to it!
+		 */
+		if (!host->is_bus_width_set) {
+			cmd->error = litex_mmc_set_bus_width(host);
+			if (cmd->error) {
+				dev_err(dev, "Can't set bus width!\n");
+				mmc_request_done(mmc, mrq);
+				return;
+			}
+			host->is_bus_width_set = true;
+		}
+
+		/* Try to DMA directly to/from the data buffer.
+		 * We can do that if the buffer can be mapped for DMA
+		 * in one contiguous chunk.
+		 */
+		dma = host->dma;
+		len = data->blksz * data->blocks;
+		sg_count = dma_map_sg(dev, data->sg, data->sg_len,
+				      mmc_get_dma_dir(data));
+		if (sg_count == 1) {
+			dma = sg_dma_address(data->sg);
+			len = sg_dma_len(data->sg);
+			direct = true;
+		} else if (len > host->buf_size)
+			len = host->buf_size;
+
+		if (data->flags & MMC_DATA_READ) {
+			litex_write8(host->sdreader + LITEX_BLK2MEM_ENA, 0);
+			litex_write64(host->sdreader + LITEX_BLK2MEM_BASE, dma);
+			litex_write32(host->sdreader + LITEX_BLK2MEM_LEN, len);
+			litex_write8(host->sdreader + LITEX_BLK2MEM_ENA, 1);
+
+			transfer = SDCARD_CTRL_DATA_TRANSFER_READ;
+		} else if (data->flags & MMC_DATA_WRITE) {
+			if (!direct)
+				sg_copy_to_buffer(data->sg, data->sg_len,
+						  host->buffer, len);
+
+			litex_write8(host->sdwriter + LITEX_MEM2BLK_ENA, 0);
+			litex_write64(host->sdwriter + LITEX_MEM2BLK_BASE, dma);
+			litex_write32(host->sdwriter + LITEX_MEM2BLK_LEN, len);
+			litex_write8(host->sdwriter + LITEX_MEM2BLK_ENA, 1);
+
+			transfer = SDCARD_CTRL_DATA_TRANSFER_WRITE;
+		} else {
+			dev_warn(dev, "Data present w/o read or write flag.\n");
+			/* Continue: set cmd status, mark req done */
+		}
+
+		litex_write16(host->sdcore + LITEX_CORE_BLKLEN, data->blksz);
+		litex_write32(host->sdcore + LITEX_CORE_BLKCNT, data->blocks);
+	}
+
+	do {
+		cmd->error = litex_mmc_send_cmd(host, cmd->opcode, cmd->arg,
+						response_len, transfer);
+	} while (cmd->error && retries-- > 0);
+
+	if (cmd->error) {
+		/* card may be gone; don't assume bus width is still set */
+		host->is_bus_width_set = false;
+	}
+
+	if (response_len == SDCARD_CTRL_RESPONSE_SHORT) {
+		/* pull short response fields from appropriate host registers */
+		cmd->resp[0] = host->resp[3];
+		cmd->resp[1] = host->resp[2] & 0xFF;
+	} else if (response_len == SDCARD_CTRL_RESPONSE_LONG) {
+		cmd->resp[0] = host->resp[0];
+		cmd->resp[1] = host->resp[1];
+		cmd->resp[2] = host->resp[2];
+		cmd->resp[3] = host->resp[3];
+	}
+
+	/* Send stop-transmission command if required */
+	if (stop && (cmd->error || !sbc)) {
+		stop->error = litex_mmc_send_cmd(host, stop->opcode, stop->arg,
+						litex_mmc_response_len(stop),
+						SDCARD_CTRL_DATA_TRANSFER_NONE);
+		if (stop->error)
+			host->is_bus_width_set = false;
+	}
+
+	if (data) {
+		dma_unmap_sg(dev, data->sg, data->sg_len,
+			     mmc_get_dma_dir(data));
+	}
+
+	if (!cmd->error && transfer != SDCARD_CTRL_DATA_TRANSFER_NONE) {
+		data->bytes_xfered = min(len, mmc->max_req_size);
+		if (transfer == SDCARD_CTRL_DATA_TRANSFER_READ && !direct) {
+			sg_copy_from_buffer(data->sg, sg_nents(data->sg),
+					    host->buffer, data->bytes_xfered);
+		}
+	}
+
+	mmc_request_done(mmc, mrq);
+}
+
+static void litex_mmc_setclk(struct litex_mmc_host *host, unsigned int freq)
+{
+	struct device *dev = mmc_dev(host->mmc);
+	u32 div;
+
+	div = freq ? host->ref_clk / freq : 256U;
+	div = roundup_pow_of_two(div);
+	div = min(max(div, 2U), 256U);
+	dev_dbg(dev, "sd_clk_freq=%d: set to %d via div=%d\n",
+		freq, host->ref_clk / div, div);
+	litex_write16(host->sdphy + LITEX_PHY_CLOCKERDIV, div);
+	host->sd_clk = freq;
+}
+
+static void litex_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+	struct litex_mmc_host *host = mmc_priv(mmc);
+
+	/* NOTE: Ignore any ios->bus_width updates; they occur right after
+	 * the mmc core sends its own acmd6 bus-width change notification,
+	 * which is redundant since we snoop on the command flow and inject
+	 * an early acmd6 before the first data transfer command is sent!
+	 */
+
+	/* update sd_clk */
+	if (ios->clock != host->sd_clk)
+		litex_mmc_setclk(host, ios->clock);
+}
+
+static const struct mmc_host_ops litex_mmc_ops = {
+	.get_cd = litex_mmc_get_cd,
+	.request = litex_mmc_request,
+	.set_ios = litex_mmc_set_ios,
+};
+
+static int litex_mmc_irq_init(struct litex_mmc_host *host)
+{
+	struct device *dev = mmc_dev(host->mmc);
+	int ret;
+
+	host->irq = platform_get_irq_optional(host->dev, 0);
+	if (host->irq <= 0) {
+		dev_warn(dev, "Failed to get IRQ, using polling\n");
+		goto use_polling;
+	}
+
+	host->sdirq = devm_platform_ioremap_resource_byname(host->dev, "irq");
+	if (IS_ERR(host->sdirq))
+		return PTR_ERR(host->sdirq);
+
+	ret = request_irq(host->irq, litex_mmc_interrupt, 0,
+			  "litex-mmc", host->mmc);
+	if (ret < 0) {
+		dev_warn(dev, "IRQ request error %d, using polling\n", ret);
+		goto use_polling;
+	}
+
+	/* clear & enable card-change interrupts */
+	litex_write32(host->sdirq + LITEX_IRQ_PENDING, SDIRQ_CARD_DETECT);
+	litex_write32(host->sdirq + LITEX_IRQ_ENABLE, SDIRQ_CARD_DETECT);
+
+	return 0;
+
+use_polling:
+	host->mmc->caps |= MMC_CAP_NEEDS_POLL;
+	host->irq = 0;
+	return 0;
+}
+
+static int litex_mmc_probe(struct platform_device *pdev)
+{
+	struct litex_mmc_host *host;
+	struct mmc_host *mmc;
+	struct clk *clk;
+	int ret;
+
+	mmc = mmc_alloc_host(sizeof(struct litex_mmc_host), &pdev->dev);
+	/* NOTE: defaults to max_[req,seg]_size=PAGE_SIZE, max_blk_size=512,
+	 * and max_blk_count accordingly set to 8;
+	 * If for some reason we need to modify max_blk_count, we must also
+	 * re-calculate `max_[req,seg]_size = max_blk_size * max_blk_count;`
+	 */
+	if (!mmc)
+		return -ENOMEM;
+
+	host = mmc_priv(mmc);
+	host->mmc = mmc;
+	host->dev = pdev;
+
+	/* initialize clock source */
+	clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk)) {
+		ret = dev_err_probe(&pdev->dev,
+				    PTR_ERR(clk), "can't get clock\n");
+		goto err;
+	}
+	host->ref_clk = clk_get_rate(clk);
+	host->sd_clk = 0;
+
+	/* LiteSDCard only supports 4-bit bus width; therefore, we MUST inject
+	 * a SET_BUS_WIDTH (acmd6) before the very first data transfer, earlier
+	 * than when the mmc subsystem would normally get around to it!
+	 */
+	host->is_bus_width_set = false;
+	host->app_cmd = false;
+
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+	/* increase from default 32 on 64-bit-DMA capable architectures */
+	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
+	if (ret)
+		goto err;
+#endif
+
+	host->buf_size = mmc->max_req_size * 2;
+	host->buffer = dmam_alloc_coherent(&pdev->dev, host->buf_size,
+					   &host->dma, GFP_DMA);
+	if (host->buffer == NULL) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	host->sdphy = devm_platform_ioremap_resource_byname(pdev, "phy");
+	if (IS_ERR(host->sdphy)) {
+		ret = PTR_ERR(host->sdphy);
+		goto err;
+	}
+
+	host->sdcore = devm_platform_ioremap_resource_byname(pdev, "core");
+	if (IS_ERR(host->sdcore)) {
+		ret = PTR_ERR(host->sdcore);
+		goto err;
+	}
+
+	host->sdreader = devm_platform_ioremap_resource_byname(pdev, "reader");
+	if (IS_ERR(host->sdreader)) {
+		ret = PTR_ERR(host->sdreader);
+		goto err;
+	}
+
+	host->sdwriter = devm_platform_ioremap_resource_byname(pdev, "writer");
+	if (IS_ERR(host->sdwriter)) {
+		ret = PTR_ERR(host->sdwriter);
+		goto err;
+	}
+
+	/* ensure DMA bus masters are disabled */
+	litex_write8(host->sdreader + LITEX_BLK2MEM_ENA, 0);
+	litex_write8(host->sdwriter + LITEX_MEM2BLK_ENA, 0);
+
+	init_completion(&host->cmd_done);
+	ret = litex_mmc_irq_init(host);
+	if (ret)
+		goto err;
+
+	mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+	mmc->ops = &litex_mmc_ops;
+
+	/* set default sd_clk frequency range based on empirical observations
+	 * of LiteSDCard gateware behavior on typical SDCard media
+	 */
+	mmc->f_min = 12.5e6;
+	mmc->f_max = 50e6;
+
+	ret = mmc_of_parse(mmc);
+	if (ret)
+		goto err;
+
+	/* force 4-bit bus_width (only width supported by hardware) */
+	mmc->caps &= ~MMC_CAP_8_BIT_DATA;
+	mmc->caps |= MMC_CAP_4_BIT_DATA;
+
+	/* set default capabilities */
+	mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY |
+		     MMC_CAP_DRIVER_TYPE_D |
+		     MMC_CAP_CMD23;
+	mmc->caps2 |= MMC_CAP2_NO_WRITE_PROTECT |
+		      MMC_CAP2_FULL_PWR_CYCLE |
+		      MMC_CAP2_NO_SDIO;
+
+	platform_set_drvdata(pdev, host);
+
+	ret = mmc_add_host(mmc);
+	if (ret < 0)
+		goto err;
+
+	return 0;
+
+err:
+	mmc_free_host(mmc);
+	return ret;
+}
+
+static int litex_mmc_remove(struct platform_device *pdev)
+{
+	struct litex_mmc_host *host = dev_get_drvdata(&pdev->dev);
+
+	if (host->irq > 0)
+		free_irq(host->irq, host->mmc);
+	mmc_remove_host(host->mmc);
+	mmc_free_host(host->mmc);
+
+	return 0;
+}
+
+static const struct of_device_id litex_match[] = {
+	{ .compatible = "litex,mmc" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, litex_match);
+
+static struct platform_driver litex_mmc_driver = {
+	.probe = litex_mmc_probe,
+	.remove = litex_mmc_remove,
+	.driver = {
+		.name = "litex-mmc",
+		.of_match_table = of_match_ptr(litex_match),
+	},
+};
+module_platform_driver(litex_mmc_driver);
+
+MODULE_DESCRIPTION("LiteX SDCard driver");
+MODULE_AUTHOR("Antmicro <contact@antmicro.com>");
+MODULE_AUTHOR("Kamil Rakoczy <krakoczy@antmicro.com>");
+MODULE_AUTHOR("Maciej Dudek <mdudek@internships.antmicro.com>");
+MODULE_AUTHOR("Paul Mackerras <paulus@ozlabs.org>");
+MODULE_AUTHOR("Gabriel Somlo <gsomlo@gmail.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.31.1


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

* Re: [PATCH v3 2/3] dt-bindings: mmc: Add bindings for LiteSDCard
  2021-12-08 13:20 ` [PATCH v3 2/3] dt-bindings: mmc: Add bindings for LiteSDCard Gabriel Somlo
@ 2021-12-08 23:06   ` Rob Herring
  2021-12-09  1:08     ` Gabriel L. Somlo
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2021-12-08 23:06 UTC (permalink / raw)
  To: Gabriel Somlo
  Cc: linux-kernel, kgugala, mdudek, rdunlap, paulus, joel, geert,
	david.abdurachmanov, florent, linux-mmc, shorne, devicetree,
	robh+dt, krakoczy, ulf.hansson, mholenko

On Wed, 08 Dec 2021 08:20:41 -0500, Gabriel Somlo wrote:
> LiteSDCard is a small footprint, configurable SDCard core for FPGA
> based system on chips.
> 
> Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> 
> New in v3:
>   - picked up r/b Geert Uytterhoeven <geert@linux-m68k.org> in DT
>     bindings document (please let me know if that was premature, and
>     happy to take further review if needed :)
>   - add dedicated DT property for source clock frequency
> 
>  .../devicetree/bindings/mmc/litex,mmc.yaml    | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/litex,mmc.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/litex,mmc.yaml: properties:reg-names:items: 'oneOf' conditional failed, one must be fixed:
	[{'const': 'phy'}, {'const': 'core'}, {'const': 'reader'}, {'const': 'writer'}, {'const': 'irq (optional)'}] is not of type 'object'
	'irq (optional)' does not match '^[a-zA-Z0-9,.\\-_ #+/]+$'
	from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/litex,mmc.yaml: properties:reg: {'items': [{'description': 'PHY registers'}, {'description': 'CORE registers'}, {'description': 'DMA Reader buffer'}, {'description': 'DMA Writer buffer'}, {'description': 'IRQ registers (optional)'}], 'minItems': 4, 'maxItems': 5} should not be valid under {'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/litex,mmc.yaml: properties:reg-names: {'items': [{'const': 'phy'}, {'const': 'core'}, {'const': 'reader'}, {'const': 'writer'}, {'const': 'irq (optional)'}], 'minItems': 4, 'maxItems': 5} should not be valid under {'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/litex,mmc.yaml: ignoring, error in schema: properties: reg-names: items
warning: no schema found in file: ./Documentation/devicetree/bindings/mmc/litex,mmc.yaml
Documentation/devicetree/bindings/mmc/litex,mmc.example.dt.yaml:0:0: /example-0/mmc@12005000: failed to match any schema with compatible: ['litex,mmc']

doc reference errors (make refcheckdocs):


See https://patchwork.ozlabs.org/patch/1565210

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


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

* Re: [PATCH v3 2/3] dt-bindings: mmc: Add bindings for LiteSDCard
  2021-12-08 23:06   ` Rob Herring
@ 2021-12-09  1:08     ` Gabriel L. Somlo
  2021-12-09  8:37       ` Geert Uytterhoeven
  0 siblings, 1 reply; 9+ messages in thread
From: Gabriel L. Somlo @ 2021-12-09  1:08 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel, kgugala, mdudek, rdunlap, paulus, joel, geert,
	david.abdurachmanov, florent, linux-mmc, shorne, devicetree,
	robh+dt, krakoczy, ulf.hansson, mholenko

On Wed, Dec 08, 2021 at 05:06:46PM -0600, Rob Herring wrote:
> On Wed, 08 Dec 2021 08:20:41 -0500, Gabriel Somlo wrote:
> > LiteSDCard is a small footprint, configurable SDCard core for FPGA
> > based system on chips.
> > 
> > Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
> > Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > ---
> > 
> > New in v3:
> >   - picked up r/b Geert Uytterhoeven <geert@linux-m68k.org> in DT
> >     bindings document (please let me know if that was premature, and
> >     happy to take further review if needed :)
> >   - add dedicated DT property for source clock frequency
> > 
> >  .../devicetree/bindings/mmc/litex,mmc.yaml    | 72 +++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mmc/litex,mmc.yaml
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/litex,mmc.yaml: properties:reg-names:items: 'oneOf' conditional failed, one must be fixed:
> 	[{'const': 'phy'}, {'const': 'core'}, {'const': 'reader'}, {'const': 'writer'}, {'const': 'irq (optional)'}] is not of type 'object'
> 	'irq (optional)' does not match '^[a-zA-Z0-9,.\\-_ #+/]+$'
> 	from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/litex,mmc.yaml: properties:reg: {'items': [{'description': 'PHY registers'}, {'description': 'CORE registers'}, {'description': 'DMA Reader buffer'}, {'description': 'DMA Writer buffer'}, {'description': 'IRQ registers (optional)'}], 'minItems': 4, 'maxItems': 5} should not be valid under {'required': ['maxItems']}
> 	hint: "maxItems" is not needed with an "items" list
> 	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/litex,mmc.yaml: properties:reg-names: {'items': [{'const': 'phy'}, {'const': 'core'}, {'const': 'reader'}, {'const': 'writer'}, {'const': 'irq (optional)'}], 'minItems': 4, 'maxItems': 5} should not be valid under {'required': ['maxItems']}
> 	hint: "maxItems" is not needed with an "items" list
> 	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/litex,mmc.yaml: ignoring, error in schema: properties: reg-names: items
> warning: no schema found in file: ./Documentation/devicetree/bindings/mmc/litex,mmc.yaml
> Documentation/devicetree/bindings/mmc/litex,mmc.example.dt.yaml:0:0: /example-0/mmc@12005000: failed to match any schema with compatible: ['litex,mmc']
> 
> doc reference errors (make refcheckdocs):
> 
> 
> See https://patchwork.ozlabs.org/patch/1565210
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.
> 

Thanks! 

I made the following changes:

--- a/Documentation/devicetree/bindings/mmc/litex,mmc.yaml
+++ b/Documentation/devicetree/bindings/mmc/litex,mmc.yaml
@@ -29,9 +29,8 @@ properties:
       - description: CORE registers
       - description: DMA Reader buffer
       - description: DMA Writer buffer
-      - description: IRQ registers (optional)
+      - description: IRQ registers
     minItems: 4
-    maxItems: 5

   reg-names:
     items:
@@ -39,12 +38,13 @@ properties:
       - const: core
       - const: reader
       - const: writer
-      - const: irq (optional)
+      - const: irq
     minItems: 4
-    maxItems: 5

   clocks:
     maxItems: 1
+    description:
+      Handle to reference clock.

   interrupts:
     maxItems: 1

... which took care of the bulk of the error messages reported. However,
I'm still getting the one below, whether or not I leave the `maxItems 1`
line there under `clocks:`

$ make ARCH=riscv CROSS_COMPILE=riscv64-unknown-linux-gnu-  dt_binding_check
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema-examples.json
/home/somlo/linux/Documentation/devicetree/bindings/clock/litex,clock.yaml: properties:clock-output-names: {'description': 'List of strings of clock output signal names indexed by the first cell in the clock specifier.', 'minItems': 1, 'maxItems': 7, 'items': [{'const': 'CLKOUT0'}, {'const': 'CLKOUT1'}, {'const': 'CLKOUT2'}, {'const': 'CLKOUT3'}, {'const': 'CLKOUT4'}, {'const': 'CLKOUT5'}, {'const': 'CLKOUT6'}]} should not be valid under {'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
  SCHEMA  Documentation/devicetree/bindings/processed-schema-examples.json
/home/somlo/linux/Documentation/devicetree/bindings/clock/litex,clock.yaml: ignoring, error in schema: properties: clock-output-names
warning: no schema found in file: ./Documentation/devicetree/bindings/clock/litex,clock.yaml
  DTEX    Documentation/devicetree/bindings/mmc/litex,mmc.example.dts
  DTEX    Documentation/devicetree/bindings/media/renesas,imr.example.dts
  ...

It appears as though `make dt_binding_check` is trying to read from
`Documentation/devicetree/bindings/clock/litex,clock.yaml`, which
does not exist. The clock reference I'm talking about could be *any*
clock elsewhere in the dts!

This wasn't part of the originally reported errors, not sure why I'm
seeing it now. Also, not sure what (if anything) I still need to do
about it, any advice much appreciated!

Thanks,
--Gabriel

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

* Re: [PATCH v3 2/3] dt-bindings: mmc: Add bindings for LiteSDCard
  2021-12-09  1:08     ` Gabriel L. Somlo
@ 2021-12-09  8:37       ` Geert Uytterhoeven
  2021-12-09 18:25         ` Gabriel L. Somlo
  0 siblings, 1 reply; 9+ messages in thread
From: Geert Uytterhoeven @ 2021-12-09  8:37 UTC (permalink / raw)
  To: Gabriel L. Somlo
  Cc: Rob Herring, Linux Kernel Mailing List, Karol Gugala, mdudek,
	Randy Dunlap, Paul Mackerras, Joel Stanley, david.abdurachmanov,
	Florent Kermarrec, Linux MMC List, Stafford Horne,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Rob Herring, Kamil Rakoczy, Ulf Hansson, Mateusz Holenko

Hi Gabriel,

On Thu, Dec 9, 2021 at 2:08 AM Gabriel L. Somlo <gsomlo@gmail.com> wrote:
> ... which took care of the bulk of the error messages reported. However,
> I'm still getting the one below, whether or not I leave the `maxItems 1`
> line there under `clocks:`
>
> $ make ARCH=riscv CROSS_COMPILE=riscv64-unknown-linux-gnu-  dt_binding_check
>   LINT    Documentation/devicetree/bindings
>   CHKDT   Documentation/devicetree/bindings/processed-schema-examples.json
> /home/somlo/linux/Documentation/devicetree/bindings/clock/litex,clock.yaml: properties:clock-output-names: {'description': 'List of strings of clock output signal names indexed by the first cell in the clock specifier.', 'minItems': 1, 'maxItems': 7, 'items': [{'const': 'CLKOUT0'}, {'const': 'CLKOUT1'}, {'const': 'CLKOUT2'}, {'const': 'CLKOUT3'}, {'const': 'CLKOUT4'}, {'const': 'CLKOUT5'}, {'const': 'CLKOUT6'}]} should not be valid under {'required': ['maxItems']}
>         hint: "maxItems" is not needed with an "items" list
>         from schema $id: http://devicetree.org/meta-schemas/items.yaml#
>   SCHEMA  Documentation/devicetree/bindings/processed-schema-examples.json
> /home/somlo/linux/Documentation/devicetree/bindings/clock/litex,clock.yaml: ignoring, error in schema: properties: clock-output-names
> warning: no schema found in file: ./Documentation/devicetree/bindings/clock/litex,clock.yaml
>   DTEX    Documentation/devicetree/bindings/mmc/litex,mmc.example.dts
>   DTEX    Documentation/devicetree/bindings/media/renesas,imr.example.dts
>   ...

--- a/Documentation/devicetree/bindings/clock/litex,clock.yaml
+++ b/Documentation/devicetree/bindings/clock/litex,clock.yaml
@@ -45,7 +45,6 @@ properties:
       List of strings of clock output signal names indexed
       by the first cell in the clock specifier.
     minItems: 1
-    maxItems: 7
     items:
       - const: CLKOUT0
       - const: CLKOUT1

I have that in my local tree, but hadn't sent it to you yet, because
litex,clock definitely need more work.

> It appears as though `make dt_binding_check` is trying to read from
> `Documentation/devicetree/bindings/clock/litex,clock.yaml`, which
> does not exist. The clock reference I'm talking about could be *any*

Oh, it does exist in your tree ;-)
To check the examples, it has to apply all other binding files that
might apply, hence some checks are always run.

You can avoid some (but not all) such checks by adding

    DT_SCHEMA_FILES=Documentation/devicetree/bindings/path/to/binding.yaml

> clock elsewhere in the dts!
>
> This wasn't part of the originally reported errors, not sure why I'm
> seeing it now. Also, not sure what (if anything) I still need to do
> about it, any advice much appreciated!

Of course, as Rob doesn't have that file in his tree.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH v3 2/3] dt-bindings: mmc: Add bindings for LiteSDCard
  2021-12-09  8:37       ` Geert Uytterhoeven
@ 2021-12-09 18:25         ` Gabriel L. Somlo
  0 siblings, 0 replies; 9+ messages in thread
From: Gabriel L. Somlo @ 2021-12-09 18:25 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Rob Herring, Linux Kernel Mailing List, Karol Gugala, mdudek,
	Randy Dunlap, Paul Mackerras, Joel Stanley, david.abdurachmanov,
	Florent Kermarrec, Linux MMC List, Stafford Horne,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Rob Herring, Kamil Rakoczy, Ulf Hansson, Mateusz Holenko

On Thu, Dec 09, 2021 at 09:37:27AM +0100, Geert Uytterhoeven wrote:
> Hi Gabriel,
> 
> On Thu, Dec 9, 2021 at 2:08 AM Gabriel L. Somlo <gsomlo@gmail.com> wrote:
> > ... which took care of the bulk of the error messages reported. However,
> > I'm still getting the one below, whether or not I leave the `maxItems 1`
> > line there under `clocks:`
> >
> > $ make ARCH=riscv CROSS_COMPILE=riscv64-unknown-linux-gnu-  dt_binding_check
> >   LINT    Documentation/devicetree/bindings
> >   CHKDT   Documentation/devicetree/bindings/processed-schema-examples.json
> > /home/somlo/linux/Documentation/devicetree/bindings/clock/litex,clock.yaml: properties:clock-output-names: {'description': 'List of strings of clock output signal names indexed by the first cell in the clock specifier.', 'minItems': 1, 'maxItems': 7, 'items': [{'const': 'CLKOUT0'}, {'const': 'CLKOUT1'}, {'const': 'CLKOUT2'}, {'const': 'CLKOUT3'}, {'const': 'CLKOUT4'}, {'const': 'CLKOUT5'}, {'const': 'CLKOUT6'}]} should not be valid under {'required': ['maxItems']}
> >         hint: "maxItems" is not needed with an "items" list
> >         from schema $id: http://devicetree.org/meta-schemas/items.yaml#
> >   SCHEMA  Documentation/devicetree/bindings/processed-schema-examples.json
> > /home/somlo/linux/Documentation/devicetree/bindings/clock/litex,clock.yaml: ignoring, error in schema: properties: clock-output-names
> > warning: no schema found in file: ./Documentation/devicetree/bindings/clock/litex,clock.yaml
> >   DTEX    Documentation/devicetree/bindings/mmc/litex,mmc.example.dts
> >   DTEX    Documentation/devicetree/bindings/media/renesas,imr.example.dts
> >   ...
> 
> --- a/Documentation/devicetree/bindings/clock/litex,clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/litex,clock.yaml
> @@ -45,7 +45,6 @@ properties:
>        List of strings of clock output signal names indexed
>        by the first cell in the clock specifier.
>      minItems: 1
> -    maxItems: 7
>      items:
>        - const: CLKOUT0
>        - const: CLKOUT1
> 
> I have that in my local tree, but hadn't sent it to you yet, because
> litex,clock definitely need more work.
> 
> > It appears as though `make dt_binding_check` is trying to read from
> > `Documentation/devicetree/bindings/clock/litex,clock.yaml`, which
> > does not exist. The clock reference I'm talking about could be *any*
> 
> Oh, it does exist in your tree ;-)
> To check the examples, it has to apply all other binding files that
> might apply, hence some checks are always run.
> 
> You can avoid some (but not all) such checks by adding
> 
>     DT_SCHEMA_FILES=Documentation/devicetree/bindings/path/to/binding.yaml
> 
> > clock elsewhere in the dts!
> >
> > This wasn't part of the originally reported errors, not sure why I'm
> > seeing it now. Also, not sure what (if anything) I still need to do
> > about it, any advice much appreciated!
> 
> Of course, as Rob doesn't have that file in his tree.

Oh, I'm working on the `litex-rebase` branch, which does have the
litex,clock file. Running the check on the Linus master with litex_mmc
v4 on top now passes the check without any errors or warnings. I'll
incorporate the fixes pointed out by Rob when I publish v4.

Sorry for the misunderstanding, thanks Geert for pointing out the
source of my confusion -- I think all's well now on the dt-bindings
front w.r.t. litex_mmc... :)

Cheers,
--Gabriel

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

* Re: [PATCH v3 3/3] mmc: Add driver for LiteX's LiteSDCard interface
  2021-12-08 13:20 ` [PATCH v3 3/3] mmc: Add driver for LiteX's LiteSDCard interface Gabriel Somlo
@ 2021-12-25 16:39   ` Andy Shevchenko
  0 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2021-12-25 16:39 UTC (permalink / raw)
  To: gsomlo
  Cc: Linux Kernel Mailing List, Rob Herring, devicetree, Ulf Hansson,
	linux-mmc, Karol Gugala, Mateusz Holenko, krakoczy, mdudek,
	paulus, Joel Stanley, Stafford Horne, Geert Uytterhoeven,
	david.abdurachmanov, florent, Randy Dunlap

On Wed, Dec 8, 2021 at 6:15 PM Gabriel Somlo <gsomlo@gmail.com> wrote:
>
> LiteX (https://github.com/enjoy-digital/litex) is a SoC framework
> that targets FPGAs. LiteSDCard is a small footprint, configurable
> SDCard core commonly used in LiteX designs.

...

> +       int ret;
> +
> +       host->irq = platform_get_irq_optional(host->dev, 0);
> +       if (host->irq <= 0) {
> +               dev_warn(dev, "Failed to get IRQ, using polling\n");
> +               goto use_polling;
> +       }

This is wrong. It missed the deferred probe, for example.

The best approach is

ret = platform_get_irq_optional(...);
if (ret < 0 && ret != -ENXIO)
  return ret;
if (ret > 0)
  ...we got it...

It will allow the future API fix of platform_get_irq_optional() to be
really optional.




--
With Best Regards,
Andy Shevchenko

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

end of thread, other threads:[~2021-12-25 16:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-08 13:20 [PATCH v3 0/3] mmc: Add LiteSDCard mmc driver Gabriel Somlo
2021-12-08 13:20 ` [PATCH v3 1/3] MAINTAINERS: co-maintain LiteX platform Gabriel Somlo
2021-12-08 13:20 ` [PATCH v3 2/3] dt-bindings: mmc: Add bindings for LiteSDCard Gabriel Somlo
2021-12-08 23:06   ` Rob Herring
2021-12-09  1:08     ` Gabriel L. Somlo
2021-12-09  8:37       ` Geert Uytterhoeven
2021-12-09 18:25         ` Gabriel L. Somlo
2021-12-08 13:20 ` [PATCH v3 3/3] mmc: Add driver for LiteX's LiteSDCard interface Gabriel Somlo
2021-12-25 16:39   ` Andy Shevchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.