[PATCH v1 0/8] riscv: Add SPI support for Kendryte K210

[PATCH v1 0/8] riscv: Add SPI support for Kendryte K210

[Sean Anderson]

This patch adds support for SPI on the Kendryte K210. This includes the MMC slot
and SPI flash on the Sipeed Maix Bit.

This patch series was previously part of
https://patchwork.ozlabs.org/project/uboot/list/?series=161576

This patch series depends on
https://patchwork.ozlabs.org/project/uboot/list/?series=162646

Known bugs:
- The MMC cannot be accessed with the dw_spi driver

Sean Anderson (8):
  doc: Fix typo in FIT documentation
  spi: dw: Add device tree properties for fields in CTRL0
  spi: dw: Rename "cs-gpio" to "cs-gpios"
  spi: dw: Use generic function to read reg address
  spi: dw: Speed up transfer loops
  spi: dw: Add mem_ops
  riscv: Add device tree bindings for SPI
  riscv: Add support for SPI on Kendryte K210

 arch/arc/dts/axs10x_mb.dtsi                   |   3 +-
 arch/arc/dts/hsdk.dts                         |   3 +-
 arch/riscv/dts/k210-maix-bit.dts              |  90 ++++++++
 arch/riscv/dts/k210.dtsi                      |  12 +
 board/sipeed/maix/Kconfig                     |  10 +
 configs/sipeed_maix_bitm_defconfig            |   8 +
 doc/board/sipeed/maix.rst                     |  94 +++++---
 .../spi/snps,dw-apb-ssi.txt                   |  43 ++++
 doc/uImage.FIT/source_file_format.txt         |   2 +-
 drivers/spi/designware_spi.c                  | 211 ++++++++++++++----
 10 files changed, 395 insertions(+), 81 deletions(-)
 create mode 100644 doc/device-tree-bindings/spi/snps,dw-apb-ssi.txt

-- 
2.25.0

[PATCH v1 1/8] doc: Fix typo in FIT documentation

[Sean Anderson]

u_boot should be u-boot

Signed-off-by: Sean Anderson <seanga2@gmail.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---

 doc/uImage.FIT/source_file_format.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/uImage.FIT/source_file_format.txt b/doc/uImage.FIT/source_file_format.txt
index 18d2aedcb7..00c4dc337c 100644
--- a/doc/uImage.FIT/source_file_format.txt
+++ b/doc/uImage.FIT/source_file_format.txt
@@ -172,7 +172,7 @@ the '/images' node should have the following layout:
   - os : OS name, mandatory for types "kernel" and "ramdisk". Valid OS names
     are: "openbsd", "netbsd", "freebsd", "4_4bsd", "linux", "svr4", "esix",
     "solaris", "irix", "sco", "dell", "ncr", "lynxos", "vxworks", "psos", "qnx",
-    "u_boot", "rtems", "unity", "integrity".
+    "u-boot", "rtems", "unity", "integrity".
   - arch : Architecture name, mandatory for types: "standalone", "kernel",
     "firmware", "ramdisk" and "fdt". Valid architecture names are: "alpha",
     "arm", "i386", "ia64", "mips", "mips64", "ppc", "s390", "sh", "sparc",
-- 
2.25.0

[PATCH v1 1/8] doc: Fix typo in FIT documentation

[Marek Vasut]

On 3/5/20 8:19 PM, Sean Anderson wrote:
> u_boot should be u-boot
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>  doc/uImage.FIT/source_file_format.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/doc/uImage.FIT/source_file_format.txt b/doc/uImage.FIT/source_file_format.txt
> index 18d2aedcb7..00c4dc337c 100644
> --- a/doc/uImage.FIT/source_file_format.txt
> +++ b/doc/uImage.FIT/source_file_format.txt
> @@ -172,7 +172,7 @@ the '/images' node should have the following layout:
>    - os : OS name, mandatory for types "kernel" and "ramdisk". Valid OS names
>      are: "openbsd", "netbsd", "freebsd", "4_4bsd", "linux", "svr4", "esix",
>      "solaris", "irix", "sco", "dell", "ncr", "lynxos", "vxworks", "psos", "qnx",
> -    "u_boot", "rtems", "unity", "integrity".
> +    "u-boot", "rtems", "unity", "integrity".

I wonder why 4_4bsd in that list is also spelled with an underscore ?

[PATCH v1 1/8] doc: Fix typo in FIT documentation

[Tom Rini]

On Sun, Mar 22, 2020 at 02:52:59AM +0100, Marek Vasut wrote:
> On 3/5/20 8:19 PM, Sean Anderson wrote:
> > u_boot should be u-boot
> > 
> > Signed-off-by: Sean Anderson <seanga2@gmail.com>
> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
> > 
> >  doc/uImage.FIT/source_file_format.txt | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/doc/uImage.FIT/source_file_format.txt b/doc/uImage.FIT/source_file_format.txt
> > index 18d2aedcb7..00c4dc337c 100644
> > --- a/doc/uImage.FIT/source_file_format.txt
> > +++ b/doc/uImage.FIT/source_file_format.txt
> > @@ -172,7 +172,7 @@ the '/images' node should have the following layout:
> >    - os : OS name, mandatory for types "kernel" and "ramdisk". Valid OS names
> >      are: "openbsd", "netbsd", "freebsd", "4_4bsd", "linux", "svr4", "esix",
> >      "solaris", "irix", "sco", "dell", "ncr", "lynxos", "vxworks", "psos", "qnx",
> > -    "u_boot", "rtems", "unity", "integrity".
> > +    "u-boot", "rtems", "unity", "integrity".
> 
> I wonder why 4_4bsd in that list is also spelled with an underscore ?

Matches the code:
common/image.c:	{	IH_OS_4_4BSD,	"4_4bsd",	"4_4BSD",		},
Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200323/6df09482/attachment.sig>

[PATCH v1 2/8] spi: dw: Add device tree properties for fields in CTRL0

[Sean Anderson]

Some devices have different layouts for the fields in CTRL0 (e.g. the
Kendryte K210). Allow this layout to be configurable from the device tree.
The documentation has been taken from Linux.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

 .../spi/snps,dw-apb-ssi.txt                   | 43 +++++++++++++++++++
 drivers/spi/designware_spi.c                  | 40 ++++++++++-------
 2 files changed, 68 insertions(+), 15 deletions(-)
 create mode 100644 doc/device-tree-bindings/spi/snps,dw-apb-ssi.txt

diff --git a/doc/device-tree-bindings/spi/snps,dw-apb-ssi.txt b/doc/device-tree-bindings/spi/snps,dw-apb-ssi.txt
new file mode 100644
index 0000000000..4b6152f6b7
--- /dev/null
+++ b/doc/device-tree-bindings/spi/snps,dw-apb-ssi.txt
@@ -0,0 +1,43 @@
+Synopsys DesignWare AMBA 2.0 Synchronous Serial Interface.
+
+Required properties:
+- compatible : "snps,dw-apb-ssi"
+- reg : The register base for the controller. For "mscc,<soc>-spi", a second
+  register set is required (named ICPU_CFG:SPI_MST)
+- #address-cells : <1>, as required by generic SPI binding.
+- #size-cells : <0>, also as required by generic SPI binding.
+- clocks : phandles for the clocks, see the description of clock-names below.
+   The phandle for the "ssi_clk" is required. The phandle for the "pclk" clock
+   is optional. If a single clock is specified but no clock-name, it is the
+   "ssi_clk" clock. If both clocks are listed, the "ssi_clk" must be first.
+
+Optional properties:
+- clock-names : Contains the names of the clocks:
+    "ssi_clk", for the core clock used to generate the external SPI clock.
+    "pclk", the interface clock, required for register access.
+- cs-gpios : Specifies the gpio pins to be used for chipselects.
+- num-cs : The number of chipselects. If omitted, this will default to 4.
+- reg-io-width : The I/O register width (in bytes) implemented by this
+  device.  Supported values are 2 or 4 (the default).
+- snps,dfs-offset The offset in bits of the DFS field in CTRL0, defaulting to 0
+- snps,frf-offset The offset in bits of the FRF field in CTRL0, defaulting to 4
+- snps,tmod-offset The offset in bits of the tmode field in CTRL0, defaulting
+  to 6
+- snps,mode-offset The offset in bits of the work mode field in CTRL0,
+  defaulting to 8
+
+Child nodes as per the generic SPI binding.
+
+Example:
+
+	spi at fff00000 {
+		compatible = "snps,dw-apb-ssi";
+		reg = <0xfff00000 0x1000>;
+		interrupts = <0 154 4>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		clocks = <&spi_m_clk>;
+		num-cs = <2>;
+		cs-gpios = <&gpio0 13 0>,
+			   <&gpio0 14 0>;
+	};
diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 2dc16736a3..6e1c289297 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -3,6 +3,7 @@
  * Designware master SPI core controller driver
  *
  * Copyright (C) 2014 Stefan Roese <sr@denx.de>
+ * Copyright (C) 2020 Sean Anderson <seanga2@gmail.com>
  *
  * Very loosely based on the Linux driver:
  * drivers/spi/spi-dw.c, which is:
@@ -51,20 +52,14 @@
 #define DW_SPI_DR			0x60
 
 /* Bit fields in CTRLR0 */
-#define SPI_DFS_OFFSET			0
-
-#define SPI_FRF_OFFSET			4
 #define SPI_FRF_SPI			0x0
 #define SPI_FRF_SSP			0x1
 #define SPI_FRF_MICROWIRE		0x2
 #define SPI_FRF_RESV			0x3
 
-#define SPI_MODE_OFFSET			6
-#define SPI_SCPH_OFFSET			6
-#define SPI_SCOL_OFFSET			7
+#define SPI_MODE_SCPH			0x1
+#define SPI_MODE_SCOL			0x2
 
-#define SPI_TMOD_OFFSET			8
-#define SPI_TMOD_MASK			(0x3 << SPI_TMOD_OFFSET)
 #define	SPI_TMOD_TR			0x0		/* xmit & recv */
 #define SPI_TMOD_TO			0x1		/* xmit only */
 #define SPI_TMOD_RO			0x2		/* recv only */
@@ -89,6 +84,12 @@
 struct dw_spi_platdata {
 	s32 frequency;		/* Default clock frequency, -1 for none */
 	void __iomem *regs;
+
+	/* Offsets in CTRL0 */
+	u8 dfs_off;
+	u8 frf_off;
+	u8 tmod_off;
+	u8 mode_off;
 };
 
 struct dw_spi_priv {
@@ -115,6 +116,15 @@ struct dw_spi_priv {
 	struct reset_ctl_bulk	resets;
 };
 
+static inline u32 GEN_CTRL0(struct dw_spi_priv *priv,
+			    struct dw_spi_platdata *plat)
+{
+	return ((priv->bits_per_word - 1) << plat->dfs_off |
+	      (priv->type << plat->frf_off) |
+	      (priv->mode << plat->mode_off) |
+	      (priv->tmode << plat->tmod_off));
+}
+
 static inline u32 dw_read(struct dw_spi_priv *priv, u32 offset)
 {
 	return __raw_readl(priv->regs + offset);
@@ -160,6 +170,10 @@ static int dw_spi_ofdata_to_platdata(struct udevice *bus)
 	/* Use 500KHz as a suitable default */
 	plat->frequency = dev_read_u32_default(bus, "spi-max-frequency",
 					       500000);
+	plat->dfs_off = dev_read_u32_default(bus, "snps,dfs-offset", 0);
+	plat->frf_off = dev_read_u32_default(bus, "snps,frf-offset", 4);
+	plat->mode_off = dev_read_u32_default(bus, "snps,mode-offset", 6);
+	plat->tmod_off = dev_read_u32_default(bus, "snps,tmod-offset", 8);
 	debug("%s: regs=%p max-frequency=%d\n", __func__, plat->regs,
 	      plat->frequency);
 
@@ -388,6 +402,7 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen,
 		       const void *dout, void *din, unsigned long flags)
 {
 	struct udevice *bus = dev->parent;
+	struct dw_spi_platdata *plat = dev_get_platdata(bus);
 	struct dw_spi_priv *priv = dev_get_priv(bus);
 	const u8 *tx = dout;
 	u8 *rx = din;
@@ -406,10 +421,6 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen,
 	if (flags & SPI_XFER_BEGIN)
 		external_cs_manage(dev, false);
 
-	cr0 = (priv->bits_per_word - 1) | (priv->type << SPI_FRF_OFFSET) |
-		(priv->mode << SPI_MODE_OFFSET) |
-		(priv->tmode << SPI_TMOD_OFFSET);
-
 	if (rx && tx)
 		priv->tmode = SPI_TMOD_TR;
 	else if (rx)
@@ -421,8 +432,7 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen,
 		 */
 		priv->tmode = SPI_TMOD_TR;
 
-	cr0 &= ~SPI_TMOD_MASK;
-	cr0 |= (priv->tmode << SPI_TMOD_OFFSET);
+	cr0 = GEN_CTRL0(priv, plat);
 
 	priv->len = bitlen >> 3;
 	debug("%s: rx=%p tx=%p len=%d [bytes]\n", __func__, rx, tx, priv->len);
@@ -476,7 +486,7 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen,
 
 static int dw_spi_set_speed(struct udevice *bus, uint speed)
 {
-	struct dw_spi_platdata *plat = bus->platdata;
+	struct dw_spi_platdata *plat = dev_get_platdata(bus);
 	struct dw_spi_priv *priv = dev_get_priv(bus);
 	u16 clk_div;
 
-- 
2.25.0

[PATCH v1 2/8] spi: dw: Add device tree properties for fields in CTRL0

[Marek Vasut]

On 3/5/20 8:19 PM, Sean Anderson wrote:
> Some devices have different layouts for the fields in CTRL0 (e.g. the
> Kendryte K210). Allow this layout to be configurable from the device tree.
> The documentation has been taken from Linux.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Can't you just have different compatible string for each SoC and derive
the various fields based on that compatible string, instead of
describing all the register bitfields in DT ?

What does Linux do ?

[PATCH v1 2/8] spi: dw: Add device tree properties for fields in CTRL0

[Sean Anderson]

On 3/21/20 9:51 PM, Marek Vasut wrote:
> On 3/5/20 8:19 PM, Sean Anderson wrote:
>> Some devices have different layouts for the fields in CTRL0 (e.g. the
>> Kendryte K210). Allow this layout to be configurable from the device tree.
>> The documentation has been taken from Linux.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> Can't you just have different compatible string for each SoC and derive
> the various fields based on that compatible string, instead of
> describing all the register bitfields in DT ?
> 
> What does Linux do ?
> 

Linux only supports socfpga boards. I don't know if there is any
rhyme/reason to the shifting around of these fields. It is possible to
add several compatible strings like "kendryte,k210-spi3". I chose this
method because the bitfields are different for spi0 and spi1, spi2, and
spi3. If there are other incompatibilities discovered, then it may make
more sense to use different strings. Another option could have been to
use the DW_SPI_VERSION field to detect different controllers, but it is
the same among all the controllers on the K210. 

--Sean

[PATCH v1 2/8] spi: dw: Add device tree properties for fields in CTRL0

[Marek Vasut]

On 3/22/20 3:36 AM, Sean Anderson wrote:
> On 3/21/20 9:51 PM, Marek Vasut wrote:
>> On 3/5/20 8:19 PM, Sean Anderson wrote:
>>> Some devices have different layouts for the fields in CTRL0 (e.g. the
>>> Kendryte K210). Allow this layout to be configurable from the device tree.
>>> The documentation has been taken from Linux.
>>>
>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> Can't you just have different compatible string for each SoC and derive
>> the various fields based on that compatible string, instead of
>> describing all the register bitfields in DT ?
>>
>> What does Linux do ?
>>
> 
> Linux only supports socfpga boards. I don't know if there is any
> rhyme/reason to the shifting around of these fields.

Could be a different revision of the IP. This is usually handled by
using SoC-specific compatible strings, see e.g. Linux
Documentation/devicetree/bindings/usb/renesas,usb3-peri.yaml

You don't want to encode register layout in the DT.

> It is possible to
> add several compatible strings like "kendryte,k210-spi3".

Why ?

> I chose this
> method because the bitfields are different for spi0 and spi1, spi2, and
> spi3. If there are other incompatibilities discovered, then it may make
> more sense to use different strings. Another option could have been to
> use the DW_SPI_VERSION field to detect different controllers, but it is
> the same among all the controllers on the K210. 

The controllers on the same SoC have different register layout ?

-- 
Best regards,
Marek Vasut

[PATCH v1 2/8] spi: dw: Add device tree properties for fields in CTRL0

[Sean Anderson]

On 3/21/20 11:04 PM, Marek Vasut wrote:
> On 3/22/20 3:36 AM, Sean Anderson wrote:
>> On 3/21/20 9:51 PM, Marek Vasut wrote:
>>> On 3/5/20 8:19 PM, Sean Anderson wrote:
>>>> Some devices have different layouts for the fields in CTRL0 (e.g. the
>>>> Kendryte K210). Allow this layout to be configurable from the device tree.
>>>> The documentation has been taken from Linux.
>>>>
>>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>
>>> Can't you just have different compatible string for each SoC and derive
>>> the various fields based on that compatible string, instead of
>>> describing all the register bitfields in DT ?
>>>
>>> What does Linux do ?
>>>
>>
>> Linux only supports socfpga boards. I don't know if there is any
>> rhyme/reason to the shifting around of these fields.
> 
> Could be a different revision of the IP. This is usually handled by
> using SoC-specific compatible strings, see e.g. Linux
> Documentation/devicetree/bindings/usb/renesas,usb3-peri.yaml
> 
> You don't want to encode register layout in the DT.

Ok, then I think adding compatible strings would be the cleanest.

> 
>> It is possible to
>> add several compatible strings like "kendryte,k210-spi3".
> 
> Why ?

See below.

> 
>> I chose this
>> method because the bitfields are different for spi0 and spi1, spi2, and
>> spi3. If there are other incompatibilities discovered, then it may make
>> more sense to use different strings. Another option could have been to
>> use the DW_SPI_VERSION field to detect different controllers, but it is
>> the same among all the controllers on the K210. 
> 
> The controllers on the same SoC have different register layout ?
> 

Yup!

Don't ask me why.

--Sean

[PATCH v1 2/8] spi: dw: Add device tree properties for fields in CTRL0

[Marek Vasut]

On 3/22/20 4:08 AM, Sean Anderson wrote:
> On 3/21/20 11:04 PM, Marek Vasut wrote:
>> On 3/22/20 3:36 AM, Sean Anderson wrote:
>>> On 3/21/20 9:51 PM, Marek Vasut wrote:
>>>> On 3/5/20 8:19 PM, Sean Anderson wrote:
>>>>> Some devices have different layouts for the fields in CTRL0 (e.g. the
>>>>> Kendryte K210). Allow this layout to be configurable from the device tree.
>>>>> The documentation has been taken from Linux.
>>>>>
>>>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>>
>>>> Can't you just have different compatible string for each SoC and derive
>>>> the various fields based on that compatible string, instead of
>>>> describing all the register bitfields in DT ?
>>>>
>>>> What does Linux do ?
>>>>
>>>
>>> Linux only supports socfpga boards. I don't know if there is any
>>> rhyme/reason to the shifting around of these fields.
>>
>> Could be a different revision of the IP. This is usually handled by
>> using SoC-specific compatible strings, see e.g. Linux
>> Documentation/devicetree/bindings/usb/renesas,usb3-peri.yaml
>>
>> You don't want to encode register layout in the DT.
> 
> Ok, then I think adding compatible strings would be the cleanest.
> 
>>
>>> It is possible to
>>> add several compatible strings like "kendryte,k210-spi3".
>>
>> Why ?
> 
> See below.
> 
>>
>>> I chose this
>>> method because the bitfields are different for spi0 and spi1, spi2, and
>>> spi3. If there are other incompatibilities discovered, then it may make
>>> more sense to use different strings. Another option could have been to
>>> use the DW_SPI_VERSION field to detect different controllers, but it is
>>> the same among all the controllers on the K210. 
>>
>> The controllers on the same SoC have different register layout ?
>>
> 
> Yup!
> 
> Don't ask me why.

Now that is truly an odd design. Is there a datasheet for this SoC ?

You might be able to somehow enumerate those controllers in DT and
derive their layout from that enumeration or somesuch I think.

[PATCH v1 2/8] spi: dw: Add device tree properties for fields in CTRL0

[Sean Anderson]

On 3/21/20 11:13 PM, Marek Vasut wrote:
> On 3/22/20 4:08 AM, Sean Anderson wrote:
>> On 3/21/20 11:04 PM, Marek Vasut wrote:
>>> Could be a different revision of the IP. This is usually handled by
>>> using SoC-specific compatible strings, see e.g. Linux
>>> Documentation/devicetree/bindings/usb/renesas,usb3-peri.yaml
>>>
>>> You don't want to encode register layout in the DT.
>>
>> Ok, then I think adding compatible strings would be the cleanest.
>>
>>>
>>>> It is possible to
>>>> add several compatible strings like "kendryte,k210-spi3".
>>>
>>> Why ?
>>
>> See below.
>>
>>>
>>>> I chose this
>>>> method because the bitfields are different for spi0 and spi1, spi2, and
>>>> spi3. If there are other incompatibilities discovered, then it may make
>>>> more sense to use different strings. Another option could have been to
>>>> use the DW_SPI_VERSION field to detect different controllers, but it is
>>>> the same among all the controllers on the K210. 
>>>
>>> The controllers on the same SoC have different register layout ?
>>>
>>
>> Yup!
>>
>> Don't ask me why.
> 
> Now that is truly an odd design. Is there a datasheet for this SoC ?

Nope. Everything has been done referencing their sdk.

In hindsight, porting this board was a poor decision because of all the
hoops I've had to jump through due to the absence of documentation.
Unfortunately, it's probably the only budget risc-v board with S-mode
until the end of the year or so.

> You might be able to somehow enumerate those controllers in DT and
> derive their layout from that enumeration or somesuch I think.

So like derive it from the sequence number? I'd rather do this in a more
standard way, if possible.

--Sean

[PATCH v1 2/8] spi: dw: Add device tree properties for fields in CTRL0

[Marek Vasut]

On 3/22/20 4:33 AM, Sean Anderson wrote:
> On 3/21/20 11:13 PM, Marek Vasut wrote:
>> On 3/22/20 4:08 AM, Sean Anderson wrote:
>>> On 3/21/20 11:04 PM, Marek Vasut wrote:
>>>> Could be a different revision of the IP. This is usually handled by
>>>> using SoC-specific compatible strings, see e.g. Linux
>>>> Documentation/devicetree/bindings/usb/renesas,usb3-peri.yaml
>>>>
>>>> You don't want to encode register layout in the DT.
>>>
>>> Ok, then I think adding compatible strings would be the cleanest.
>>>
>>>>
>>>>> It is possible to
>>>>> add several compatible strings like "kendryte,k210-spi3".
>>>>
>>>> Why ?
>>>
>>> See below.
>>>
>>>>
>>>>> I chose this
>>>>> method because the bitfields are different for spi0 and spi1, spi2, and
>>>>> spi3. If there are other incompatibilities discovered, then it may make
>>>>> more sense to use different strings. Another option could have been to
>>>>> use the DW_SPI_VERSION field to detect different controllers, but it is
>>>>> the same among all the controllers on the K210. 
>>>>
>>>> The controllers on the same SoC have different register layout ?
>>>>
>>>
>>> Yup!
>>>
>>> Don't ask me why.
>>
>> Now that is truly an odd design. Is there a datasheet for this SoC ?
> 
> Nope. Everything has been done referencing their sdk.
> 
> In hindsight, porting this board was a poor decision because of all the
> hoops I've had to jump through due to the absence of documentation.
> Unfortunately, it's probably the only budget risc-v board with S-mode
> until the end of the year or so.
> 
>> You might be able to somehow enumerate those controllers in DT and
>> derive their layout from that enumeration or somesuch I think.
> 
> So like derive it from the sequence number? I'd rather do this in a more
> standard way, if possible.

I would rather like to know why do the controllers have different
register layout.

[PATCH v1 3/8] spi: dw: Rename "cs-gpio" to "cs-gpios"

[Sean Anderson]

This property is named differently than other SPI drivers with the same
property, as well as the property as used in Linux.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 arch/arc/dts/axs10x_mb.dtsi  |  3 ++-
 arch/arc/dts/hsdk.dts        |  3 ++-
 drivers/spi/designware_spi.c | 10 +++-------
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/arch/arc/dts/axs10x_mb.dtsi b/arch/arc/dts/axs10x_mb.dtsi
index 5b77642b8d..50e62788cf 100644
--- a/arch/arc/dts/axs10x_mb.dtsi
+++ b/arch/arc/dts/axs10x_mb.dtsi
@@ -97,7 +97,8 @@
 			spi-max-frequency = <4000000>;
 			clocks = <&apbclk>;
 			clock-names = "spi_clk";
-			cs-gpio = <&cs_gpio 0>;
+			num-cs = <1>;
+			cs-gpios = <&cs_gpio 0>;
 			spi_flash at 0 {
 				compatible = "jedec,spi-nor";
 				reg = <0>;
diff --git a/arch/arc/dts/hsdk.dts b/arch/arc/dts/hsdk.dts
index 34ef3a620a..719ffdbce5 100644
--- a/arch/arc/dts/hsdk.dts
+++ b/arch/arc/dts/hsdk.dts
@@ -120,7 +120,8 @@
 		spi-max-frequency = <4000000>;
 		clocks = <&cgu_clk CLK_SYS_SPI_REF>;
 		clock-names = "spi_clk";
-		cs-gpio = <&cs_gpio 0>;
+		num-cs = <1>;
+		cs-gpios = <&cs_gpio 0>;
 		spi_flash at 0 {
 			compatible = "jedec,spi-nor";
 			reg = <0>;
diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 6e1c289297..765fa2f582 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -142,7 +142,8 @@ static int request_gpio_cs(struct udevice *bus)
 	int ret;
 
 	/* External chip select gpio line is optional */
-	ret = gpio_request_by_name(bus, "cs-gpio", 0, &priv->cs_gpio, 0);
+	ret = gpio_request_by_name(bus, "cs-gpios", 0, &priv->cs_gpio,
+				   GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
 	if (ret == -ENOENT)
 		return 0;
 
@@ -151,12 +152,7 @@ static int request_gpio_cs(struct udevice *bus)
 		return ret;
 	}
 
-	if (dm_gpio_is_valid(&priv->cs_gpio)) {
-		dm_gpio_set_dir_flags(&priv->cs_gpio,
-				      GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
-	}
-
-	debug("%s: used external gpio for CS management\n", __func__);
+	debug("%s: using external gpio for CS management\n", __func__);
 #endif
 	return 0;
 }
-- 
2.25.0

[PATCH v1 3/8] spi: dw: Rename "cs-gpio" to "cs-gpios"

[Jagan Teki]

On Fri, Mar 6, 2020 at 12:49 AM Sean Anderson <seanga2@gmail.com> wrote:
>
> This property is named differently than other SPI drivers with the same
> property, as well as the property as used in Linux.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
>  arch/arc/dts/axs10x_mb.dtsi  |  3 ++-
>  arch/arc/dts/hsdk.dts        |  3 ++-
>  drivers/spi/designware_spi.c | 10 +++-------

Better separate the dts changes and add sha1 from Linux if it synced.

[PATCH v1 3/8] spi: dw: Rename "cs-gpio" to "cs-gpios"

[Sean Anderson]

On 4/2/20 8:17 AM, Jagan Teki wrote:
> On Fri, Mar 6, 2020 at 12:49 AM Sean Anderson <seanga2@gmail.com> wrote:
>>
>> This property is named differently than other SPI drivers with the same
>> property, as well as the property as used in Linux.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>>
>>  arch/arc/dts/axs10x_mb.dtsi  |  3 ++-
>>  arch/arc/dts/hsdk.dts        |  3 ++-
>>  drivers/spi/designware_spi.c | 10 +++-------
> 
> Better separate the dts changes and add sha1 from Linux if it synced.
> 

This is not from a specific patch; I just noticed that it was different
and changed it. I think it is clearer to include dts changes with the
code change. That way if someone is reviewing the device tree, it is
obvious why the change was made. Splitting the changes would also make
the code not run properly for the intermediate commit. This can be
annoying when bisecting. However, if you would still like it split, I
can do that.

--Sean

[PATCH v1 4/8] spi: dw: Use generic function to read reg address

[Sean Anderson]

Using an fdt-specific function causes problems when compiled with a live
tree.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/spi/designware_spi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 765fa2f582..38c24fe550 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -161,7 +161,9 @@ static int dw_spi_ofdata_to_platdata(struct udevice *bus)
 {
 	struct dw_spi_platdata *plat = bus->platdata;
 
-	plat->regs = (struct dw_spi *)devfdt_get_addr(bus);
+	plat->regs = dev_read_addr_ptr(bus);
+	if (!plat->regs)
+		return -EINVAL;
 
 	/* Use 500KHz as a suitable default */
 	plat->frequency = dev_read_u32_default(bus, "spi-max-frequency",
-- 
2.25.0

[PATCH v1 4/8] spi: dw: Use generic function to read reg address

[Jagan Teki]

On Fri, Mar 6, 2020 at 12:49 AM Sean Anderson <seanga2@gmail.com> wrote:
>
> Using an fdt-specific function causes problems when compiled with a live
> tree.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
>  drivers/spi/designware_spi.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
> index 765fa2f582..38c24fe550 100644
> --- a/drivers/spi/designware_spi.c
> +++ b/drivers/spi/designware_spi.c
> @@ -161,7 +161,9 @@ static int dw_spi_ofdata_to_platdata(struct udevice *bus)
>  {
>         struct dw_spi_platdata *plat = bus->platdata;
>
> -       plat->regs = (struct dw_spi *)devfdt_get_addr(bus);
> +       plat->regs = dev_read_addr_ptr(bus);
> +       if (!plat->regs)
> +               return -EINVAL;

Better check the returned regs with FDT_ADDR_T_NONE

[PATCH v1 4/8] spi: dw: Use generic function to read reg address

[Sean Anderson]

On 4/2/20 8:24 AM, Jagan Teki wrote:
> On Fri, Mar 6, 2020 at 12:49 AM Sean Anderson <seanga2@gmail.com> wrote:
>>
>> Using an fdt-specific function causes problems when compiled with a live
>> tree.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>>
>>  drivers/spi/designware_spi.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
>> index 765fa2f582..38c24fe550 100644
>> --- a/drivers/spi/designware_spi.c
>> +++ b/drivers/spi/designware_spi.c
>> @@ -161,7 +161,9 @@ static int dw_spi_ofdata_to_platdata(struct udevice *bus)
>>  {
>>         struct dw_spi_platdata *plat = bus->platdata;
>>
>> -       plat->regs = (struct dw_spi *)devfdt_get_addr(bus);
>> +       plat->regs = dev_read_addr_ptr(bus);
>> +       if (!plat->regs)
>> +               return -EINVAL;
> 
> Better check the returned regs with FDT_ADDR_T_NONE
> 

This series depends on the series for k210 support which includes the
patch "dm: Fix error handling for dev_read_addr_ptr" [1]. This patch changes
the semantics for that function to be consistent regardless of whether a
live device tree is in use.

[1] https://patchwork.ozlabs.org/patch/1258450/

--Sean

[PATCH v1 5/8] spi: dw: Speed up transfer loops

[Sean Anderson]

The transfer loops are very tight on some platforms (especially on higher
speeds). If we don't read/write fast enough we can run into over-/under-
flow problems. This patch removes several divisions and log statements,
and simplifies the read logic.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/spi/designware_spi.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 38c24fe550..613eb0d0e6 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -304,7 +304,7 @@ static inline u32 tx_max(struct dw_spi_priv *priv)
 {
 	u32 tx_left, tx_room, rxtx_gap;
 
-	tx_left = (priv->tx_end - priv->tx) / (priv->bits_per_word >> 3);
+	tx_left = priv->tx_end - priv->tx;
 	tx_room = priv->fifo_len - dw_read(priv, DW_SPI_TXFLR);
 
 	/*
@@ -315,8 +315,7 @@ static inline u32 tx_max(struct dw_spi_priv *priv)
 	 * shift registers. So a control from sw point of
 	 * view is taken.
 	 */
-	rxtx_gap = ((priv->rx_end - priv->rx) - (priv->tx_end - priv->tx)) /
-		(priv->bits_per_word >> 3);
+	rxtx_gap = ((priv->rx_end - priv->rx) - (priv->tx_end - priv->tx));
 
 	return min3(tx_left, tx_room, (u32)(priv->fifo_len - rxtx_gap));
 }
@@ -324,7 +323,7 @@ static inline u32 tx_max(struct dw_spi_priv *priv)
 /* Return the max entries we should read out of rx fifo */
 static inline u32 rx_max(struct dw_spi_priv *priv)
 {
-	u32 rx_left = (priv->rx_end - priv->rx) / (priv->bits_per_word >> 3);
+	u32 rx_left = priv->rx_end - priv->rx;
 
 	return min_t(u32, rx_left, dw_read(priv, DW_SPI_RXFLR));
 }
@@ -336,15 +335,10 @@ static void dw_writer(struct dw_spi_priv *priv)
 
 	while (max--) {
 		/* Set the tx word if the transfer's original "tx" is not null */
-		if (priv->tx_end - priv->len) {
-			if (priv->bits_per_word == 8)
-				txw = *(u8 *)(priv->tx);
-			else
-				txw = *(u16 *)(priv->tx);
-		}
+		if (priv->tx_end - priv->len)
+			txw = *(u8 *)(priv->tx);
 		dw_write(priv, DW_SPI_DR, txw);
-		debug("%s: tx=0x%02x\n", __func__, txw);
-		priv->tx += priv->bits_per_word >> 3;
+		priv->tx++;
 	}
 }
 
@@ -355,16 +349,11 @@ static void dw_reader(struct dw_spi_priv *priv)
 
 	while (max--) {
 		rxw = dw_read(priv, DW_SPI_DR);
-		debug("%s: rx=0x%02x\n", __func__, rxw);
 
 		/* Care about rx if the transfer's original "rx" is not null */
-		if (priv->rx_end - priv->len) {
-			if (priv->bits_per_word == 8)
-				*(u8 *)(priv->rx) = rxw;
-			else
-				*(u16 *)(priv->rx) = rxw;
-		}
-		priv->rx += priv->bits_per_word >> 3;
+		if (priv->rx_end - priv->len)
+			*(u8 *)(priv->rx) = rxw;
+		priv->rx++;
 	}
 }
 
-- 
2.25.0

[PATCH v1 5/8] spi: dw: Speed up transfer loops

[Marek Vasut]

On 3/5/20 8:19 PM, Sean Anderson wrote:
> The transfer loops are very tight on some platforms (especially on higher
> speeds). If we don't read/write fast enough we can run into over-/under-
> flow problems. This patch removes several divisions and log statements,
> and simplifies the read logic.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
> 
>  drivers/spi/designware_spi.c | 29 +++++++++--------------------
>  1 file changed, 9 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
> index 38c24fe550..613eb0d0e6 100644
> --- a/drivers/spi/designware_spi.c
> +++ b/drivers/spi/designware_spi.c
> @@ -304,7 +304,7 @@ static inline u32 tx_max(struct dw_spi_priv *priv)
>  {
>  	u32 tx_left, tx_room, rxtx_gap;
>  
> -	tx_left = (priv->tx_end - priv->tx) / (priv->bits_per_word >> 3);
> +	tx_left = priv->tx_end - priv->tx;
>  	tx_room = priv->fifo_len - dw_read(priv, DW_SPI_TXFLR);
>  
>  	/*
> @@ -315,8 +315,7 @@ static inline u32 tx_max(struct dw_spi_priv *priv)
>  	 * shift registers. So a control from sw point of
>  	 * view is taken.
>  	 */
> -	rxtx_gap = ((priv->rx_end - priv->rx) - (priv->tx_end - priv->tx)) /
> -		(priv->bits_per_word >> 3);
> +	rxtx_gap = ((priv->rx_end - priv->rx) - (priv->tx_end - priv->tx));
>  
>  	return min3(tx_left, tx_room, (u32)(priv->fifo_len - rxtx_gap));
>  }
> @@ -324,7 +323,7 @@ static inline u32 tx_max(struct dw_spi_priv *priv)
>  /* Return the max entries we should read out of rx fifo */
>  static inline u32 rx_max(struct dw_spi_priv *priv)
>  {
> -	u32 rx_left = (priv->rx_end - priv->rx) / (priv->bits_per_word >> 3);
> +	u32 rx_left = priv->rx_end - priv->rx;
>  
>  	return min_t(u32, rx_left, dw_read(priv, DW_SPI_RXFLR));
>  }
> @@ -336,15 +335,10 @@ static void dw_writer(struct dw_spi_priv *priv)
>  
>  	while (max--) {
>  		/* Set the tx word if the transfer's original "tx" is not null */
> -		if (priv->tx_end - priv->len) {
> -			if (priv->bits_per_word == 8)
> -				txw = *(u8 *)(priv->tx);
> -			else
> -				txw = *(u16 *)(priv->tx);
> -		}
> +		if (priv->tx_end - priv->len)
> +			txw = *(u8 *)(priv->tx);
>  		dw_write(priv, DW_SPI_DR, txw);
> -		debug("%s: tx=0x%02x\n", __func__, txw);
> -		priv->tx += priv->bits_per_word >> 3;
> +		priv->tx++;

This breaks 16 bits per word transfers, NAK.

The compiler should be able to figure out the rest of the optimizations
in this patch.

[PATCH v1 5/8] spi: dw: Speed up transfer loops

[Sean Anderson]

On 3/21/20 9:49 PM, Marek Vasut wrote:
> On 3/5/20 8:19 PM, Sean Anderson wrote:
>> The transfer loops are very tight on some platforms (especially on higher
>> speeds). If we don't read/write fast enough we can run into over-/under-
>> flow problems. This patch removes several divisions and log statements,
>> and simplifies the read logic.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>>
>>  drivers/spi/designware_spi.c | 29 +++++++++--------------------
>>  1 file changed, 9 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
>> index 38c24fe550..613eb0d0e6 100644
>> --- a/drivers/spi/designware_spi.c
>> +++ b/drivers/spi/designware_spi.c
>> @@ -304,7 +304,7 @@ static inline u32 tx_max(struct dw_spi_priv *priv)
>>  {
>>  	u32 tx_left, tx_room, rxtx_gap;
>>  
>> -	tx_left = (priv->tx_end - priv->tx) / (priv->bits_per_word >> 3);
>> +	tx_left = priv->tx_end - priv->tx;
>>  	tx_room = priv->fifo_len - dw_read(priv, DW_SPI_TXFLR);
>>  
>>  	/*
>> @@ -315,8 +315,7 @@ static inline u32 tx_max(struct dw_spi_priv *priv)
>>  	 * shift registers. So a control from sw point of
>>  	 * view is taken.
>>  	 */
>> -	rxtx_gap = ((priv->rx_end - priv->rx) - (priv->tx_end - priv->tx)) /
>> -		(priv->bits_per_word >> 3);
>> +	rxtx_gap = ((priv->rx_end - priv->rx) - (priv->tx_end - priv->tx));
>>  
>>  	return min3(tx_left, tx_room, (u32)(priv->fifo_len - rxtx_gap));
>>  }
>> @@ -324,7 +323,7 @@ static inline u32 tx_max(struct dw_spi_priv *priv)
>>  /* Return the max entries we should read out of rx fifo */
>>  static inline u32 rx_max(struct dw_spi_priv *priv)
>>  {
>> -	u32 rx_left = (priv->rx_end - priv->rx) / (priv->bits_per_word >> 3);
>> +	u32 rx_left = priv->rx_end - priv->rx;
>>  
>>  	return min_t(u32, rx_left, dw_read(priv, DW_SPI_RXFLR));
>>  }
>> @@ -336,15 +335,10 @@ static void dw_writer(struct dw_spi_priv *priv)
>>  
>>  	while (max--) {
>>  		/* Set the tx word if the transfer's original "tx" is not null */
>> -		if (priv->tx_end - priv->len) {
>> -			if (priv->bits_per_word == 8)
>> -				txw = *(u8 *)(priv->tx);
>> -			else
>> -				txw = *(u16 *)(priv->tx);
>> -		}
>> +		if (priv->tx_end - priv->len)
>> +			txw = *(u8 *)(priv->tx);
>>  		dw_write(priv, DW_SPI_DR, txw);
>> -		debug("%s: tx=0x%02x\n", __func__, txw);
>> -		priv->tx += priv->bits_per_word >> 3;
>> +		priv->tx++;
> 
> This breaks 16 bits per word transfers, NAK.
> 
> The compiler should be able to figure out the rest of the optimizations
> in this patch.
> 

Hm, I will try with just the debug symbols removed.

--Sean

[PATCH v1 6/8] spi: dw: Add mem_ops

[Sean Anderson]

The designware ssi device has "broken" chip select behaviour [1], and needs
specific manipulation to use the built-in chip select. The existing fix is
to use an external GPIO for chip select, but typically the K210 has SPI3
directly connected to a flash chip with dedicated pins. This makes it
impossible to use the spi_xfer function to use spi, since the CS is
de-asserted in between calls.  This patch adds an implementation of
exec_op, which gives correct behaviour when reading/writing spi flash.

Work on this device has been difficult because the only example code I have
to work off is Kendryte's sdk, and I do not have access to the datasheet
(if anyone does, I would love to have a look!). The MMC device is still not
working, but I have been making progress.

[1] https://lkml.org/lkml/2015/12/23/132

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/spi/designware_spi.c | 128 ++++++++++++++++++++++++++++++++++-
 1 file changed, 126 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 613eb0d0e6..298d1dbab5 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -17,6 +17,7 @@
 #include <errno.h>
 #include <malloc.h>
 #include <spi.h>
+#include <spi-mem.h>
 #include <fdtdec.h>
 #include <reset.h>
 #include <dm/device_compat.h>
@@ -108,8 +109,8 @@ struct dw_spi_priv {
 	int len;
 
 	u32 fifo_len;		/* depth of the FIFO buffer */
-	void *tx;
-	void *tx_end;
+	const void *tx;
+	const void *tx_end;
 	void *rx;
 	void *rx_end;
 
@@ -471,6 +472,124 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen,
 	return ret;
 }
 
+/*
+ * This function is necessary for reading SPI flash with the native CS
+ * c.f. https://lkml.org/lkml/2015/12/23/132
+ */
+static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
+{
+	bool read = op->data.dir == SPI_MEM_DATA_IN;
+	int pos, i, ret = 0;
+	struct udevice *bus = slave->dev->parent;
+	struct dw_spi_platdata *plat = dev_get_platdata(bus);
+	struct dw_spi_priv *priv = dev_get_priv(bus);
+	u8 op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes;
+	u8 op_buf[op_len];
+	u32 cr0;
+
+	if (read)
+		priv->tmode = SPI_TMOD_EPROMREAD;
+	else
+		priv->tmode = SPI_TMOD_TO;
+
+	debug("%s: buf=%p len=%u [bytes]\n",
+	      __func__, op->data.buf.in, op->data.nbytes);
+
+	cr0 = GEN_CTRL0(priv, plat);
+	debug("%s: cr0=%08x\n", __func__, cr0);
+
+	spi_enable_chip(priv, 0);
+	dw_write(priv, DW_SPI_CTRL0, cr0);
+	if (read)
+		dw_write(priv, DW_SPI_CTRL1, op->data.nbytes - 1);
+	spi_enable_chip(priv, 1);
+
+	/* From spi_mem_exec_op */
+	pos = 0;
+	op_buf[pos++] = op->cmd.opcode;
+	if (op->addr.nbytes) {
+		for (i = 0; i < op->addr.nbytes; i++)
+			op_buf[pos + i] = op->addr.val >>
+				(8 * (op->addr.nbytes - i - 1));
+
+		pos += op->addr.nbytes;
+	}
+	if (op->dummy.nbytes)
+		memset(op_buf + pos, 0xff, op->dummy.nbytes);
+
+	priv->tx = &op_buf;
+	priv->tx_end = priv->tx + op_len;
+	priv->rx = NULL;
+	priv->rx_end = NULL;
+	while (priv->tx != priv->tx_end) {
+		dw_writer(priv);
+		/* This loop needs a delay otherwise we can hang */
+		udelay(1);
+	}
+
+	/*
+	 * XXX: The following are tight loops! Enabling debug messages may cause
+	 * them to fail because we are not reading/writing the fifo fast enough.
+	 *
+	 * We heuristically break out of the loop when we stop getting data.
+	 * This is to stop us from hanging if the device doesn't send any data
+	 * (either at all, or after sending a response). For example, one flash
+	 * chip I tested did not send anything back after the first 64K of data.
+	 */
+	if (read) {
+		/* If we have gotten any data back yet */
+		bool got_data = false;
+		/* How many times we have looped without reading anything */
+		int loops_since_read = 0;
+		struct spi_mem_op *mut_op = (struct spi_mem_op *)op;
+
+		priv->rx = op->data.buf.in;
+		priv->rx_end = priv->rx + op->data.nbytes;
+
+		dw_write(priv, DW_SPI_SER, 1 << spi_chip_select(slave->dev));
+		while (priv->rx != priv->rx_end) {
+			void *last_rx = priv->rx;
+
+			dw_reader(priv);
+			if (priv->rx == last_rx) {
+				loops_since_read++;
+				/* Thresholds are arbitrary */
+				if (loops_since_read > 256)
+					break;
+				else if (got_data && loops_since_read > 32)
+					break;
+			} else {
+				got_data = true;
+				loops_since_read = 0;
+			}
+		}
+
+		/* Update with the actual amount of data read */
+		mut_op->data.nbytes -= priv->rx_end - priv->rx;
+	} else {
+		u32 val;
+
+		priv->tx = op->data.buf.out;
+		priv->tx_end = priv->tx + op->data.nbytes;
+
+		/* Fill up the write fifo before starting the transfer */
+		dw_writer(priv);
+		dw_write(priv, DW_SPI_SER, 1 << spi_chip_select(slave->dev));
+		while (priv->tx != priv->tx_end)
+			dw_writer(priv);
+
+		if (readl_poll_timeout(priv->regs + DW_SPI_SR, val,
+				       (val & SR_TF_EMPT) && !(val & SR_BUSY),
+				       RX_TIMEOUT * 1000)) {
+			ret = -ETIMEDOUT;
+		}
+	}
+
+	dw_write(priv, DW_SPI_SER, 0);
+	debug("%s: %u bytes xfered\n", __func__, op->data.nbytes);
+	return ret;
+}
+
 static int dw_spi_set_speed(struct udevice *bus, uint speed)
 {
 	struct dw_spi_platdata *plat = dev_get_platdata(bus);
@@ -534,8 +653,13 @@ static int dw_spi_remove(struct udevice *bus)
 	return 0;
 }
 
+static const struct spi_controller_mem_ops dw_spi_mem_ops = {
+	.exec_op = dw_spi_exec_op,
+};
+
 static const struct dm_spi_ops dw_spi_ops = {
 	.xfer		= dw_spi_xfer,
+	.mem_ops	= &dw_spi_mem_ops,
 	.set_speed	= dw_spi_set_speed,
 	.set_mode	= dw_spi_set_mode,
 	/*
-- 
2.25.0

[PATCH v1 6/8] spi: dw: Add mem_ops

[Eugeniy Paltsev]

Hi Sean,

do you have branch with this code (all dw spi changes) in some public repo?
I would like to test it with our board (which have DW SPI).

---
 Eugeniy Paltsev


________________________________________
From: Sean Anderson <seanga2@gmail.com>
Sent: Thursday, March 5, 2020 22:19
To: u-boot at lists.denx.de
Cc: Simon Glass; Jagan Teki; Sean Anderson; Eugeniy Paltsev
Subject: [PATCH v1 6/8] spi: dw: Add mem_ops

The designware ssi device has "broken" chip select behaviour [1], and needs
specific manipulation to use the built-in chip select. The existing fix is
to use an external GPIO for chip select, but typically the K210 has SPI3
directly connected to a flash chip with dedicated pins. This makes it
impossible to use the spi_xfer function to use spi, since the CS is
de-asserted in between calls.  This patch adds an implementation of
exec_op, which gives correct behaviour when reading/writing spi flash.

Work on this device has been difficult because the only example code I have
to work off is Kendryte's sdk, and I do not have access to the datasheet
(if anyone does, I would love to have a look!). The MMC device is still not
working, but I have been making progress.

[1] https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2015_12_23_132&d=DwIDAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=ZlJN1MriPUTkBKCrPSx67GmaplEUGcAEk9yPtCLdUXI&m=WVwKMzBXasdUvVoschXNTC5C7t39Ta0E7Ly3uEPRT_0&s=7dEhNfWpefL5yAqFi1Ztrve9SBPM_n3OoIJoH40hgpQ&e=

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 drivers/spi/designware_spi.c | 128 ++++++++++++++++++++++++++++++++++-
 1 file changed, 126 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 613eb0d0e6..298d1dbab5 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -17,6 +17,7 @@
 #include <errno.h>
 #include <malloc.h>
 #include <spi.h>
+#include <spi-mem.h>
 #include <fdtdec.h>
 #include <reset.h>
 #include <dm/device_compat.h>
@@ -108,8 +109,8 @@ struct dw_spi_priv {
        int len;

        u32 fifo_len;           /* depth of the FIFO buffer */
-       void *tx;
-       void *tx_end;
+       const void *tx;
+       const void *tx_end;
        void *rx;
        void *rx_end;

@@ -471,6 +472,124 @@ static int dw_spi_xfer(struct udevice *dev, unsigned int bitlen,
        return ret;
 }

+/*
+ * This function is necessary for reading SPI flash with the native CS
+ * c.f. https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2015_12_23_132&d=DwIDAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=ZlJN1MriPUTkBKCrPSx67GmaplEUGcAEk9yPtCLdUXI&m=WVwKMzBXasdUvVoschXNTC5C7t39Ta0E7Ly3uEPRT_0&s=7dEhNfWpefL5yAqFi1Ztrve9SBPM_n3OoIJoH40hgpQ&e=
+ */
+static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
+{
+       bool read = op->data.dir == SPI_MEM_DATA_IN;
+       int pos, i, ret = 0;
+       struct udevice *bus = slave->dev->parent;
+       struct dw_spi_platdata *plat = dev_get_platdata(bus);
+       struct dw_spi_priv *priv = dev_get_priv(bus);
+       u8 op_len = sizeof(op->cmd.opcode) + op->addr.nbytes + op->dummy.nbytes;
+       u8 op_buf[op_len];
+       u32 cr0;
+
+       if (read)
+               priv->tmode = SPI_TMOD_EPROMREAD;
+       else
+               priv->tmode = SPI_TMOD_TO;
+
+       debug("%s: buf=%p len=%u [bytes]\n",
+             __func__, op->data.buf.in, op->data.nbytes);
+
+       cr0 = GEN_CTRL0(priv, plat);
+       debug("%s: cr0=%08x\n", __func__, cr0);
+
+       spi_enable_chip(priv, 0);
+       dw_write(priv, DW_SPI_CTRL0, cr0);
+       if (read)
+               dw_write(priv, DW_SPI_CTRL1, op->data.nbytes - 1);
+       spi_enable_chip(priv, 1);
+
+       /* From spi_mem_exec_op */
+       pos = 0;
+       op_buf[pos++] = op->cmd.opcode;
+       if (op->addr.nbytes) {
+               for (i = 0; i < op->addr.nbytes; i++)
+                       op_buf[pos + i] = op->addr.val >>
+                               (8 * (op->addr.nbytes - i - 1));
+
+               pos += op->addr.nbytes;
+       }
+       if (op->dummy.nbytes)
+               memset(op_buf + pos, 0xff, op->dummy.nbytes);
+
+       priv->tx = &op_buf;
+       priv->tx_end = priv->tx + op_len;
+       priv->rx = NULL;
+       priv->rx_end = NULL;
+       while (priv->tx != priv->tx_end) {
+               dw_writer(priv);
+               /* This loop needs a delay otherwise we can hang */
+               udelay(1);
+       }
+
+       /*
+        * XXX: The following are tight loops! Enabling debug messages may cause
+        * them to fail because we are not reading/writing the fifo fast enough.
+        *
+        * We heuristically break out of the loop when we stop getting data.
+        * This is to stop us from hanging if the device doesn't send any data
+        * (either at all, or after sending a response). For example, one flash
+        * chip I tested did not send anything back after the first 64K of data.
+        */
+       if (read) {
+               /* If we have gotten any data back yet */
+               bool got_data = false;
+               /* How many times we have looped without reading anything */
+               int loops_since_read = 0;
+               struct spi_mem_op *mut_op = (struct spi_mem_op *)op;
+
+               priv->rx = op->data.buf.in;
+               priv->rx_end = priv->rx + op->data.nbytes;
+
+               dw_write(priv, DW_SPI_SER, 1 << spi_chip_select(slave->dev));
+               while (priv->rx != priv->rx_end) {
+                       void *last_rx = priv->rx;
+
+                       dw_reader(priv);
+                       if (priv->rx == last_rx) {
+                               loops_since_read++;
+                               /* Thresholds are arbitrary */
+                               if (loops_since_read > 256)
+                                       break;
+                               else if (got_data && loops_since_read > 32)
+                                       break;
+                       } else {
+                               got_data = true;
+                               loops_since_read = 0;
+                       }
+               }
+
+               /* Update with the actual amount of data read */
+               mut_op->data.nbytes -= priv->rx_end - priv->rx;
+       } else {
+               u32 val;
+
+               priv->tx = op->data.buf.out;
+               priv->tx_end = priv->tx + op->data.nbytes;
+
+               /* Fill up the write fifo before starting the transfer */
+               dw_writer(priv);
+               dw_write(priv, DW_SPI_SER, 1 << spi_chip_select(slave->dev));
+               while (priv->tx != priv->tx_end)
+                       dw_writer(priv);
+
+               if (readl_poll_timeout(priv->regs + DW_SPI_SR, val,
+                                      (val & SR_TF_EMPT) && !(val & SR_BUSY),
+                                      RX_TIMEOUT * 1000)) {
+                       ret = -ETIMEDOUT;
+               }
+       }
+
+       dw_write(priv, DW_SPI_SER, 0);
+       debug("%s: %u bytes xfered\n", __func__, op->data.nbytes);
+       return ret;
+}
+
 static int dw_spi_set_speed(struct udevice *bus, uint speed)
 {
        struct dw_spi_platdata *plat = dev_get_platdata(bus);
@@ -534,8 +653,13 @@ static int dw_spi_remove(struct udevice *bus)
        return 0;
 }

+static const struct spi_controller_mem_ops dw_spi_mem_ops = {
+       .exec_op = dw_spi_exec_op,
+};
+
 static const struct dm_spi_ops dw_spi_ops = {
        .xfer           = dw_spi_xfer,
+       .mem_ops        = &dw_spi_mem_ops,
        .set_speed      = dw_spi_set_speed,
        .set_mode       = dw_spi_set_mode,
        /*
--
2.25.0

[PATCH v1 6/8] spi: dw: Add mem_ops

[Sean Anderson]

On 3/5/20 5:15 PM, Eugeniy Paltsev wrote:
> Hi Sean,
> 
> do you have branch with this code (all dw spi changes) in some public repo?
> I would like to test it with our board (which have DW SPI).
> 
> ---
>  Eugeniy Paltsev

A full tree is available at
https://github.com/Forty-Bot/u-boot/tree/maix_spi

--Sean

[PATCH v1 6/8] spi: dw: Add mem_ops

[Eugeniy Paltsev]

Hi Sean,

I've tested the SPI (with SPI flash) on HSDK board (which have DW SPI) with your changes.
It completely break SPI on HSDK:

--------------------->8--------------------------
# sf probe 
unrecognized JEDEC id bytes: ff, ff, ff
Failed to initialize SPI flash at 0:0 (error -2)
--------------------->8--------------------------

Tested from maix_spi HEAD (e338571bf528f58b3ced7fbd0c1f5d923caa1cfd)

---
 Eugeniy Paltsev


________________________________________
From: Sean Anderson <seanga2@gmail.com>
Sent: Friday, March 6, 2020 03:48
To: Eugeniy Paltsev; u-boot at lists.denx.de
Cc: Simon Glass; Jagan Teki; Alexey Brodkin
Subject: Re: [PATCH v1 6/8] spi: dw: Add mem_ops

On 3/5/20 5:15 PM, Eugeniy Paltsev wrote:
> Hi Sean,
>
> do you have branch with this code (all dw spi changes) in some public repo?
> I would like to test it with our board (which have DW SPI).
>
> ---
>  Eugeniy Paltsev

A full tree is available at
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Forty-2DBot_u-2Dboot_tree_maix-5Fspi&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=ZlJN1MriPUTkBKCrPSx67GmaplEUGcAEk9yPtCLdUXI&m=EmfUzvFjhqLxmNa2GVtV9Ph9bW4C8gFi0EhXqFqucSU&s=23UJFSwfnLIPXRyp6h_whXzJJJCbiceE8X6ydfb11_A&e=

--Sean

[PATCH v1 6/8] spi: dw: Add mem_ops

[Sean Anderson]

On 3/6/20 12:03 PM, Eugeniy Paltsev wrote:
> Hi Sean,
> 
> I've tested the SPI (with SPI flash) on HSDK board (which have DW SPI) with your changes.
> It completely break SPI on HSDK:
> 
> --------------------->8--------------------------
> # sf probe 
> unrecognized JEDEC id bytes: ff, ff, ff
> Failed to initialize SPI flash at 0:0 (error -2)
> --------------------->8--------------------------
> 
> Tested from maix_spi HEAD (e338571bf528f58b3ced7fbd0c1f5d923caa1cfd)
> 
> ---
>  Eugeniy Paltsev

Can you try testing without this patch, but with all the other patches
in the series? Alternatively, can you try testing with this patch, but
withpout the previous one ([PATCH v1 5/8] spi: dw: Speed up transfer
loops), and with logging/debug statements enabled?

--Sean

[PATCH v1 6/8] spi: dw: Add mem_ops

[Sean Anderson]

On 3/6/20 12:03 PM, Eugeniy Paltsev wrote:
> Hi Sean,
> 
> I've tested the SPI (with SPI flash) on HSDK board (which have DW SPI) with your changes.
> It completely break SPI on HSDK:
> 
> --------------------->8--------------------------
> # sf probe 
> unrecognized JEDEC id bytes: ff, ff, ff
> Failed to initialize SPI flash at 0:0 (error -2)
> --------------------->8--------------------------
> 
> Tested from maix_spi HEAD (e338571bf528f58b3ced7fbd0c1f5d923caa1cfd)
> 
> ---
>  Eugeniy Paltsev

Actually, it just occured to me that I did not add support for a gpio cs
to the xfer function, so it's likely that the chip is not enabling the
cs. Can you try testing with the following patch

---
 drivers/spi/designware_spi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 298d1dbab5..a4e3dbd948 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -517,6 +517,8 @@ static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
 	if (op->dummy.nbytes)
 		memset(op_buf + pos, 0xff, op->dummy.nbytes);
 
+	external_cs_manage(dev, false);
+
 	priv->tx = &op_buf;
 	priv->tx_end = priv->tx + op_len;
 	priv->rx = NULL;
@@ -586,6 +588,8 @@ static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
 	}
 
 	dw_write(priv, DW_SPI_SER, 0);
+	external_cs_manage(dev, true);
+
 	debug("%s: %u bytes xfered\n", __func__, op->data.nbytes);
 	return ret;
 }
-- 
2.25.0

[PATCH v1 6/8] spi: dw: Add mem_ops

[Eugeniy Paltsev]

Hi Sean,

even after fixing build errors with this patch I still have issues with communications to flash IC.

Now it is detected correctly (JEDEC id is valid) but I have issues with read or write.
I need to look to that more intently. 

---
 Eugeniy Paltsev


________________________________________
From: Sean Anderson <seanga2@gmail.com>
Sent: Saturday, March 7, 2020 00:20
To: Eugeniy Paltsev; u-boot at lists.denx.de
Cc: Simon Glass; Jagan Teki; Alexey Brodkin
Subject: Re: [PATCH v1 6/8] spi: dw: Add mem_ops

On 3/6/20 12:03 PM, Eugeniy Paltsev wrote:
> Hi Sean,
>
> I've tested the SPI (with SPI flash) on HSDK board (which have DW SPI) with your changes.
> It completely break SPI on HSDK:
>
> --------------------->8--------------------------
> # sf probe
> unrecognized JEDEC id bytes: ff, ff, ff
> Failed to initialize SPI flash at 0:0 (error -2)
> --------------------->8--------------------------
>
> Tested from maix_spi HEAD (e338571bf528f58b3ced7fbd0c1f5d923caa1cfd)
>
> ---
>  Eugeniy Paltsev

Actually, it just occured to me that I did not add support for a gpio cs
to the xfer function, so it's likely that the chip is not enabling the
cs. Can you try testing with the following patch

---
 drivers/spi/designware_spi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
index 298d1dbab5..a4e3dbd948 100644
--- a/drivers/spi/designware_spi.c
+++ b/drivers/spi/designware_spi.c
@@ -517,6 +517,8 @@ static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
        if (op->dummy.nbytes)
                memset(op_buf + pos, 0xff, op->dummy.nbytes);

+       external_cs_manage(dev, false);
+
        priv->tx = &op_buf;
        priv->tx_end = priv->tx + op_len;
        priv->rx = NULL;
@@ -586,6 +588,8 @@ static int dw_spi_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
        }

        dw_write(priv, DW_SPI_SER, 0);
+       external_cs_manage(dev, true);
+
        debug("%s: %u bytes xfered\n", __func__, op->data.nbytes);
        return ret;
 }
--
2.25.0

[PATCH v1 7/8] riscv: Add device tree bindings for SPI

[Sean Anderson]

This patch adds bindings for the MMC slot and SPI flash on the Sipeed Maix
Bit.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 arch/riscv/dts/k210-maix-bit.dts | 90 ++++++++++++++++++++++++++++++++
 arch/riscv/dts/k210.dtsi         | 12 +++++
 2 files changed, 102 insertions(+)

diff --git a/arch/riscv/dts/k210-maix-bit.dts b/arch/riscv/dts/k210-maix-bit.dts
index c0ec572552..cb4dba5e13 100644
--- a/arch/riscv/dts/k210-maix-bit.dts
+++ b/arch/riscv/dts/k210-maix-bit.dts
@@ -235,9 +235,99 @@
 			pins = "IO_47";
 		};
 	};
+
+	fpioa_spi0: spi0 {
+		cs0 {
+			function = "GPIOHS28";
+			pins = "IO_36";
+		};
+		rst {
+			function = "GPIOHS29";
+			pins = "IO_37";
+		};
+		dc {
+			function = "GPIOHS30";
+			pins = "IO_38";
+		};
+		wr {
+			function = "SPI0_SCK";
+			pins = "IO_39";
+		};
+	};
+
+	fpioa_spi1: spi1 {
+		miso {
+			function = "SPI1_D1";
+			pins = "IO_26";
+		};
+		clk {
+			function = "SPI1_SCLK";
+			pins = "IO_27";
+		};
+		mosi {
+			function = "SPI1_D0";
+			pins = "IO_28";
+		};
+		cs0 {
+			function = "GPIOHS31";
+			pins = "IO_29";
+		};
+	};
+};
+
+&wdt0 {
+	status = "okay";
 };
 
 &dvp0 {
 	pinctrl-0 = <&fpioa_dvp>;
 	pinctrl-names = "default";
 };
+
+&spi0 {
+	pinctrl-0 = <&fpioa_spi0>;
+	pinctrl-names = "default";
+	num-cs = <1>;
+	cs-gpios = <&gpio0 28 0>;
+
+	panel at 0 {
+		compatible = "sitronix,st8898v";
+		reg = <0>;
+		reset-gpios = <&gpio0 29 GPIO_ACTIVE_LOW>;
+		spi-max-frequency = <15000000>;
+		status = "disabled";
+	};
+};
+
+&spi1 {
+	pinctrl-0 = <&fpioa_spi1>;
+	pinctrl-names = "default";
+	num-cs = <1>;
+	cs-gpios = <&gpio0 31 0>;
+	status = "okay";
+
+	slot at 0 {
+		compatible = "mmc-spi-slot";
+		reg = <0>;
+		spi-max-frequency = <25000000>;
+		voltage-ranges = <3300 3300>;
+		broken-cd;
+		disable-wp;
+	};
+};
+
+&spi3 {
+	status = "okay";
+
+	spi-flash at 0 {
+		compatible = "winbond,w25q128fw", "jedec,spi-nor";
+		reg = <0>;
+		/*
+		 * Datasheet says it should work at 100 MHz with fast read, but
+		 * after testing it doesn't probe at that frequency
+		 */
+		spi-max-frequency = <50000000>;
+		m25p,fast-read;
+		broken-flash-reset;
+	};
+};
diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi
index 0192ce5eae..a74620281c 100644
--- a/arch/riscv/dts/k210.dtsi
+++ b/arch/riscv/dts/k210.dtsi
@@ -592,6 +592,10 @@
 				spi-max-frequency = <25000000>;
 				num-cs = <4>;
 				reg-io-width = <4>;
+				snps,dfs-offset = <16>;
+				snps,frf-offset = <21>;
+				snps,tmod-offset = <8>;
+				snps,mode-offset = <6>;
 				status = "disabled";
 			};
 
@@ -608,6 +612,10 @@
 				spi-max-frequency = <25000000>;
 				num-cs = <4>;
 				reg-io-width = <4>;
+				snps,dfs-offset = <16>;
+				snps,frf-offset = <21>;
+				snps,tmod-offset = <8>;
+				snps,mode-offset = <6>;
 				status = "disabled";
 			};
 
@@ -625,6 +633,10 @@
 				spi-max-frequency = <100000000>;
 				num-cs = <4>;
 				reg-io-width = <4>;
+				snps,dfs-offset = <0>;
+				snps,frf-offset = <22>;
+				snps,tmod-offset = <10>;
+				snps,mode-offset = <8>;
 				status = "disabled";
 			};
 		};
-- 
2.25.0

[PATCH v1 7/8] riscv: Add device tree bindings for SPI

[Marek Vasut]

On 3/5/20 8:19 PM, Sean Anderson wrote:
> This patch adds bindings for the MMC slot and SPI flash on the Sipeed Maix
> Bit.

The mail subject should say this is for specific SoC.
[...]

> +&wdt0 {
> +	status = "okay";
>  };

This shouldn't be here I guess ?

You should split this into the .dtsi addition for SoC and board specific
patch.

[PATCH v1 7/8] riscv: Add device tree bindings for SPI

[Sean Anderson]

On 3/21/20 9:55 PM, Marek Vasut wrote:
> On 3/5/20 8:19 PM, Sean Anderson wrote:
>> This patch adds bindings for the MMC slot and SPI flash on the Sipeed Maix
>> Bit.
> 
> The mail subject should say this is for specific SoC
Oh, whoops. This will be fixed in the next revision.

> [...]
> 
>> +&wdt0 {
>> +	status = "okay";
>>  };
> 
> This shouldn't be here I guess ?

Yeah, this was meant to be added in a separate series for the wdt.

> 
> You should split this into the .dtsi addition for SoC and board specific
> patch.

Ok.

--Sean

[PATCH v1 8/8] riscv: Add support for SPI on Kendryte K210

[Sean Anderson]

This patch enables configs necessary for usign SPI. It also adds some
documentation.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

 board/sipeed/maix/Kconfig          | 10 ++++
 configs/sipeed_maix_bitm_defconfig |  8 +++
 doc/board/sipeed/maix.rst          | 94 +++++++++++++++++++-----------
 3 files changed, 79 insertions(+), 33 deletions(-)

diff --git a/board/sipeed/maix/Kconfig b/board/sipeed/maix/Kconfig
index a5a095d4ff..8ce7a11a9f 100644
--- a/board/sipeed/maix/Kconfig
+++ b/board/sipeed/maix/Kconfig
@@ -57,4 +57,14 @@ config BOARD_SPECIFIC_OPTIONS
 	imply CMD_GPIO
 	imply LED
 	imply LED_GPIO
+	imply SPI
+	imply DESIGNWARE_SPI
+	imply SPI_FLASH_WINBOND
+	imply DM_MTD
+	imply SPI_FLASH_MTD
+	imply CMD_MTD
+	imply ENV_IS_IN_SPI_FLASH
+	imply MMC
+	imply MMC_BROKEN_CD
+	imply MMC_SPI
 endif
diff --git a/configs/sipeed_maix_bitm_defconfig b/configs/sipeed_maix_bitm_defconfig
index f48f7f06e9..7f644e7a37 100644
--- a/configs/sipeed_maix_bitm_defconfig
+++ b/configs/sipeed_maix_bitm_defconfig
@@ -1,6 +1,14 @@
 CONFIG_RISCV=y
+CONFIG_ENV_SIZE=0x2000
+CONFIG_ENV_SECT_SIZE=0x1000
+CONFIG_ENV_OFFSET=0x7C000
+CONFIG_ENV_OFFSET_REDUND=0x7E000
 CONFIG_TARGET_SIPEED_MAIX=y
 CONFIG_ARCH_RV64I=y
+CONFIG_USE_BOOTCOMMAND=y
+CONFIG_BOOTCOMMAND="sf probe;mtd read kernel 80000000;go 80000000"
+CONFIG_MTDIDS_DEFAULT="nor0=spi3.0"
+CONFIG_MTDPARTS_DEFAULT="spi3.0:496k(u-boot),16k(env),5632k(kernel),10240k(data)"
 CONFIG_SYS_REDUNDAND_ENVIRONMENT=y
 # CONFIG_NET is not set
 # CONFIG_INPUT is not set
diff --git a/doc/board/sipeed/maix.rst b/doc/board/sipeed/maix.rst
index 312da1e7bb..0216f9b893 100644
--- a/doc/board/sipeed/maix.rst
+++ b/doc/board/sipeed/maix.rst
@@ -46,42 +46,14 @@ Boot output should look like the following:
     U-Boot 2020.04-rc2-00087-g2221cc09c1-dirty (Feb 28 2020 - 13:53:09 -0500)
 
     DRAM:  8 MiB
+    MMC:   spi at 53000000:slot at 0: 0
     In:    serial at 38000000
     Out:   serial at 38000000
     Err:   serial at 38000000
-    =>
-
-Loading Images
-^^^^^^^^^^^^^^
-
-To load a kernel, transfer it over serial.
-
-.. code-block:: none
-
-    => loady 80000000 1500000
-    ## Switch baudrate to 1500000 bps and press ENTER ...
-
-    *** baud: 1500000
-
-    *** baud: 1500000 ***
-    ## Ready for binary (ymodem) download to 0x80000000 at 1500000 bps...
-    C
-    *** file: loader.bin
-    $ sz -vv loader.bin
-    Sending: loader.bin
-    Bytes Sent:2478208   BPS:72937
-    Sending:
-    Ymodem sectors/kbytes sent:   0/ 0k
-    Transfer complete
-
-    *** exit status: 0 ***
-    ## Total Size      = 0x0025d052 = 2478162 Bytes
-    ## Switch baudrate to 115200 bps and press ESC ...
-
-    *** baud: 115200
-
-    *** baud: 115200 ***
-    =>
+    Hit any key to stop autoboot:  0
+    SF: Detected w25q128fw with page size 256 Bytes, erase size 4 KiB, total 16 MiB
+    Reading 5242880 byte(s) at offset 0x00000000
+    ## Starting application at 0x80000000 ...
 
 Running Programs
 ^^^^^^^^^^^^^^^^
@@ -163,6 +135,62 @@ To run legacy images, use the ``bootm`` command:
     argv[0] = "<NULL>"
     Hit any key to exit ...
 
+Flashing Images
+---------------
+
+To flash a kernel, transfer it over serial, then write it to the kernel
+partition.
+
+.. code-block:: none
+
+    => loady 80000000 1500000
+    ## Switch baudrate to 1500000 bps and press ENTER ...
+
+    *** baud: 1500000
+
+    *** baud: 1500000 ***
+    ## Ready for binary (ymodem) download to 0x80000000 at 1500000 bps...
+    C
+    *** file: loader.bin
+    $ sz -vv loader.bin
+    Sending: loader.bin
+    Bytes Sent:2478208   BPS:72937
+    Sending:
+    Ymodem sectors/kbytes sent:   0/ 0k
+    Transfer complete
+
+    *** exit status: 0 ***
+    ## Total Size      = 0x0025d052 = 2478162 Bytes
+    ## Switch baudrate to 115200 bps and press ESC ...
+
+    *** baud: 115200
+
+    *** baud: 115200 ***
+    => sf probe
+    SF: Detected w25q128fw with page size 256 Bytes, erase size 4 KiB, total 16 MiB
+    => mtd write kernel 80000000 0 25d052
+    Writing 2478162 byte(s) at offset 0x00000000
+
+Partition Scheme
+^^^^^^^^^^^^^^^^
+
+There is no partition scheme specified by the manufacturer. The only requirement
+imposed by the firmware is that offset 0 will be loaded and ran. The default
+partition scheme is
+
+========= ======== ======
+Partition Offset   Size
+========= ======== ======
+u-boot    0x000000 496k
+env       0x07C000 16k
+kernel    0x080000 5M
+data      0x580000 10.5M
+========= ======== ======
+
+**NB:** kflash adds a 5-byte header to payloads (and a 32-byte trailer) to all
+payloads it flashes. If you use kflash to flash your payload, you will need to
+account for this header when specifying what offset in spi flash to load from.
+
 Pin Assignment
 --------------
 
-- 
2.25.0