[PATCH v2 0/3] HiSilicon v3xx SFC driver

[PATCH v2 0/3] HiSilicon v3xx SFC driver

[John Garry]

This patchset introduces support for the HiSilicon SFC V3XX driver.

Whilst the kernel tree already includes support for a "HiSilicon SFC
driver", that is for different HW. Indeed, as mentioned in patch #1, the
naming for that driver could be better, as it should support more memory
technologies than SPI NOR (as I have been told), and it is actually known
internally as FMC. As such, maybe "hisi-fmc" would have been better, but
we can't change that now.

I used V3XX in this driver name, as that is the unique versioning for
this HW.

As for the driver itself, it is quite simple. Only ACPI firmware is
supported, and we assume m25p80 compatible SPI NOR part will be used.

DMA is not supported, and we just use polling mode for operation
completion notification. The driver uses the SPI MEM OPs.

Changes from v1:
- Add ACPI kconfig dependency
- Fix up header comment style
- Change macros naming style
- Try to enforce aligned accesses in hisi_sfc_v3xx_adjust_op_size()

John Garry (3):
  mtd: spi-nor: hisi-sfc: Try to provide some clarity on which SFC we
    are
  spi: Add HiSilicon v3xx SPI NOR flash controller driver
  MAINTAINERS: Add a maintainer for the HiSilicon v3xx SFC driver

 MAINTAINERS                     |   6 +
 drivers/mtd/spi-nor/Kconfig     |   4 +-
 drivers/mtd/spi-nor/hisi-sfc.c  |   2 +-
 drivers/spi/Kconfig             |   9 +
 drivers/spi/Makefile            |   1 +
 drivers/spi/spi-hisi-sfc-v3xx.c | 284 ++++++++++++++++++++++++++++++++
 6 files changed, 303 insertions(+), 3 deletions(-)
 create mode 100644 drivers/spi/spi-hisi-sfc-v3xx.c

-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 1/3] mtd: spi-nor: hisi-sfc: Try to provide some clarity on which SFC we are

[John Garry]

The driver is for the HiSilicon FMC (Flash Memory Controller), which
supports SPI NOR in addition other memory technologies, like SPI NAND.

Indeed, the naming in the driver is a little inappropriate, especially
considering that there is already another HiSilicon SPI NOR flash
controller (which I believe the FMC is derived from).

Since we now want to provide software support for this other HiSilicon
controller, update code comments to at least try to make it clear that
this driver is for the FMC.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/mtd/spi-nor/Kconfig    | 4 ++--
 drivers/mtd/spi-nor/hisi-sfc.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index f237fcdf7f86..c1eda67d1ad2 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -46,11 +46,11 @@ config SPI_CADENCE_QUADSPI
 	  Flash as an MTD device.
 
 config SPI_HISI_SFC
-	tristate "Hisilicon SPI-NOR Flash Controller(SFC)"
+	tristate "Hisilicon FMC SPI-NOR Flash Controller(SFC)"
 	depends on ARCH_HISI || COMPILE_TEST
 	depends on HAS_IOMEM
 	help
-	  This enables support for hisilicon SPI-NOR flash controller.
+	  This enables support for HiSilicon FMC SPI-NOR flash controller.
 
 config SPI_MTK_QUADSPI
 	tristate "MediaTek Quad SPI controller"
diff --git a/drivers/mtd/spi-nor/hisi-sfc.c b/drivers/mtd/spi-nor/hisi-sfc.c
index a1258216f89d..6db964bf3b0e 100644
--- a/drivers/mtd/spi-nor/hisi-sfc.c
+++ b/drivers/mtd/spi-nor/hisi-sfc.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * HiSilicon SPI Nor Flash Controller Driver
+ * HiSilicon FMC SPI-NOR flash controller driver
  *
  * Copyright (c) 2015-2016 HiSilicon Technologies Co., Ltd.
  */
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 1/3] mtd: spi-nor: hisi-sfc: Try to provide some clarity on which SFC we are

[Tudor.Ambarus]

On Monday, December 9, 2019 4:08:08 PM EET John Garry wrote:
> The driver is for the HiSilicon FMC (Flash Memory Controller), which
> supports SPI NOR in addition other memory technologies, like SPI NAND.
> 
> Indeed, the naming in the driver is a little inappropriate, especially
> considering that there is already another HiSilicon SPI NOR flash
> controller (which I believe the FMC is derived from).
> 
> Since we now want to provide software support for this other HiSilicon
> controller, update code comments to at least try to make it clear that
> this driver is for the FMC.
> 
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  drivers/mtd/spi-nor/Kconfig    | 4 ++--
>  drivers/mtd/spi-nor/hisi-sfc.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 

Applied to spi-nor/next.

Thanks,
ta


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[John Garry]

Add the driver for the HiSilicon v3xx SPI NOR flash controller, commonly
found in hi16xx chipsets.

This is a different controller than that in drivers/mtd/spi-nor/hisi-sfc.c;
indeed, the naming for that driver is poor, since it is really known as
FMC, and can support other memory technologies.

The driver module name is "hisi-sfc-v3xx", as recommended by HW designer,
being an attempt to provide a distinct name - v3xx being the unique
controller versioning.

Only ACPI firmware is supported.

DMA is not supported, and we just use polling mode for operation
completion notification.

The driver uses the SPI MEM OPs.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/spi/Kconfig             |   9 +
 drivers/spi/Makefile            |   1 +
 drivers/spi/spi-hisi-sfc-v3xx.c | 284 ++++++++++++++++++++++++++++++++
 3 files changed, 294 insertions(+)
 create mode 100644 drivers/spi/spi-hisi-sfc-v3xx.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 870f7797b56b..d6ed0c355954 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -281,6 +281,15 @@ config SPI_FSL_QUADSPI
 	  This controller does not support generic SPI messages. It only
 	  supports the high-level SPI memory interface.
 
+config SPI_HISI_SFC_V3XX
+	tristate "HiSilicon SPI-NOR Flash Controller for Hi16XX chipsets"
+	depends on (ARM64 && ACPI) || COMPILE_TEST
+	depends on HAS_IOMEM
+	select CONFIG_MTD_SPI_NOR
+	help
+	  This enables support for HiSilicon v3xx SPI-NOR flash controller
+	  found in hi16xx chipsets.
+
 config SPI_NXP_FLEXSPI
 	tristate "NXP Flex SPI controller"
 	depends on ARCH_LAYERSCAPE || HAS_IOMEM
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index bb49c9e6d0a0..9b65ec5afc5e 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_SPI_FSL_LPSPI)		+= spi-fsl-lpspi.o
 obj-$(CONFIG_SPI_FSL_QUADSPI)		+= spi-fsl-qspi.o
 obj-$(CONFIG_SPI_FSL_SPI)		+= spi-fsl-spi.o
 obj-$(CONFIG_SPI_GPIO)			+= spi-gpio.o
+obj-$(CONFIG_SPI_HISI_SFC_V3XX)		+= spi-hisi-sfc-v3xx.o
 obj-$(CONFIG_SPI_IMG_SPFI)		+= spi-img-spfi.o
 obj-$(CONFIG_SPI_IMX)			+= spi-imx.o
 obj-$(CONFIG_SPI_LANTIQ_SSC)		+= spi-lantiq-ssc.o
diff --git a/drivers/spi/spi-hisi-sfc-v3xx.c b/drivers/spi/spi-hisi-sfc-v3xx.c
new file mode 100644
index 000000000000..4cf8fc80a7b7
--- /dev/null
+++ b/drivers/spi/spi-hisi-sfc-v3xx.c
@@ -0,0 +1,284 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// HiSilicon SPI NOR V3XX Flash Controller Driver for hi16xx chipsets
+//
+// Copyright (c) 2019 HiSilicon Technologies Co., Ltd.
+// Author: John Garry <john.garry@huawei.com>
+
+#include <linux/acpi.h>
+#include <linux/bitops.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
+
+#define HISI_SFC_V3XX_VERSION (0x1f8)
+
+#define HISI_SFC_V3XX_CMD_CFG (0x300)
+#define HISI_SFC_V3XX_CMD_CFG_DATA_CNT_OFF 9
+#define HISI_SFC_V3XX_CMD_CFG_RW_MSK BIT(8)
+#define HISI_SFC_V3XX_CMD_CFG_DATA_EN_MSK BIT(7)
+#define HISI_SFC_V3XX_CMD_CFG_DUMMY_CNT_OFF 4
+#define HISI_SFC_V3XX_CMD_CFG_ADDR_EN_MSK BIT(3)
+#define HISI_SFC_V3XX_CMD_CFG_CS_SEL_OFF 1
+#define HISI_SFC_V3XX_CMD_CFG_START_MSK BIT(0)
+#define HISI_SFC_V3XX_CMD_INS (0x308)
+#define HISI_SFC_V3XX_CMD_ADDR (0x30c)
+#define HISI_SFC_V3XX_CMD_DATABUF0 (0x400)
+
+struct hisi_sfc_v3xx_host {
+	struct device *dev;
+	void __iomem *regbase;
+	int max_cmd_dword;
+};
+
+#define HISI_SFC_V3XX_WAIT_TIMEOUT_US		1000000
+#define HISI_SFC_V3XX_WAIT_POLL_INTERVAL_US	10
+
+static int hisi_sfc_v3xx_wait_cmd_idle(struct hisi_sfc_v3xx_host *host)
+{
+	u32 reg;
+
+	return readl_poll_timeout(host->regbase + HISI_SFC_V3XX_CMD_CFG, reg,
+				  !(reg & HISI_SFC_V3XX_CMD_CFG_START_MSK),
+				  HISI_SFC_V3XX_WAIT_POLL_INTERVAL_US,
+				  HISI_SFC_V3XX_WAIT_TIMEOUT_US);
+}
+
+static int hisi_sfc_v3xx_adjust_op_size(struct spi_mem *mem,
+					struct spi_mem_op *op)
+{
+	struct spi_device *spi = mem->spi;
+	struct hisi_sfc_v3xx_host *host;
+	uintptr_t addr = (uintptr_t)op->data.buf.in;
+	int max_byte_count;
+
+	host = spi_controller_get_devdata(spi->master);
+
+	max_byte_count = host->max_cmd_dword * 4;
+
+	if (!IS_ALIGNED(addr, 4) && op->data.nbytes >= 4)
+		op->data.nbytes = 4 - (addr % 4);
+	else if (op->data.nbytes > max_byte_count)
+		op->data.nbytes = max_byte_count;
+
+	return 0;
+}
+
+/*
+ * memcpy_{to,from}io doesn't gurantee 32b accesses - which we require for the
+ * DATABUF registers -so use __io{read,write}32_copy when possible. For
+ * trailing bytes, copy them byte-by-byte from the DATABUF register, as we
+ * can't clobber outside the source/dest buffer.
+ *
+ * For efficient data read/write, we try to put any start 32b unaligned data
+ * into a separate transaction in hisi_sfc_v3xx_adjust_op_size().
+ */
+static void hisi_sfc_v3xx_read_databuf(struct hisi_sfc_v3xx_host *host,
+				       u8 *to, unsigned int len)
+{
+	void __iomem *from;
+	int i;
+
+	from = host->regbase + HISI_SFC_V3XX_CMD_DATABUF0;
+
+	if (IS_ALIGNED((uintptr_t)to, 4)) {
+		int words = len / 4;
+
+		__ioread32_copy(to, from, words);
+
+		len -= words * 4;
+		if (len) {
+			u32 val;
+
+			to += words * 4;
+			from += words * 4;
+
+			val = __raw_readl(from);
+
+			for (i = 0; i < len; i++, val >>= 8, to++)
+				*to = (u8)val;
+		}
+	} else {
+		for (i = 0; i < DIV_ROUND_UP(len, 4); i++, from += 4) {
+			u32 val = __raw_readl(from);
+			int j;
+
+			for (j = 0; j < 4 && (j + (i * 4) < len);
+			     to++, val >>= 8, j++)
+				*to = (u8)val;
+		}
+	}
+}
+
+static void hisi_sfc_v3xx_write_databuf(struct hisi_sfc_v3xx_host *host,
+					const u8 *from, unsigned int len)
+{
+	void __iomem *to;
+	int i;
+
+	to = host->regbase + HISI_SFC_V3XX_CMD_DATABUF0;
+
+	if (IS_ALIGNED((uintptr_t)from, 4)) {
+		int words = len / 4;
+
+		__iowrite32_copy(to, from, words);
+
+		len -= words * 4;
+		if (len) {
+			u32 val = 0;
+
+			to += words * 4;
+			from += words * 4;
+
+			for (i = 0; i < len; i++, from++)
+				val |= *from << i * 8;
+			__raw_writel(val, to);
+		}
+
+	} else {
+		for (i = 0; i < DIV_ROUND_UP(len, 4); i++, to += 4) {
+			u32 val = 0;
+			int j;
+
+			for (j = 0; j < 4 && (j + (i * 4) < len);
+			     from++, j++)
+				val |= *from << j * 8;
+			__raw_writel(val, to);
+		}
+	}
+}
+
+static int hisi_sfc_v3xx_generic_exec_op(struct hisi_sfc_v3xx_host *host,
+					 const struct spi_mem_op *op,
+					 u8 chip_select)
+{
+	int ret, len = op->data.nbytes;
+	u32 config = 0;
+
+	if (op->addr.nbytes)
+		config |= HISI_SFC_V3XX_CMD_CFG_ADDR_EN_MSK;
+
+	if (op->data.dir != SPI_MEM_NO_DATA) {
+		config |= (len - 1) << HISI_SFC_V3XX_CMD_CFG_DATA_CNT_OFF;
+		config |= HISI_SFC_V3XX_CMD_CFG_DATA_EN_MSK;
+	}
+
+	if (op->data.dir == SPI_MEM_DATA_OUT)
+		hisi_sfc_v3xx_write_databuf(host, op->data.buf.out, len);
+	else if (op->data.dir == SPI_MEM_DATA_IN)
+		config |= HISI_SFC_V3XX_CMD_CFG_RW_MSK;
+
+	config |= op->dummy.nbytes << HISI_SFC_V3XX_CMD_CFG_DUMMY_CNT_OFF |
+		  chip_select << HISI_SFC_V3XX_CMD_CFG_CS_SEL_OFF |
+		  HISI_SFC_V3XX_CMD_CFG_START_MSK;
+
+	writel(op->addr.val, host->regbase + HISI_SFC_V3XX_CMD_ADDR);
+	writel(op->cmd.opcode, host->regbase + HISI_SFC_V3XX_CMD_INS);
+
+	writel(config, host->regbase + HISI_SFC_V3XX_CMD_CFG);
+
+	ret = hisi_sfc_v3xx_wait_cmd_idle(host);
+	if (ret)
+		return ret;
+
+	if (op->data.dir == SPI_MEM_DATA_IN)
+		hisi_sfc_v3xx_read_databuf(host, op->data.buf.in, len);
+
+	return 0;
+}
+
+static int hisi_sfc_v3xx_exec_op(struct spi_mem *mem,
+				 const struct spi_mem_op *op)
+{
+	struct hisi_sfc_v3xx_host *host;
+	struct spi_device *spi = mem->spi;
+	u8 chip_select = spi->chip_select;
+
+	host = spi_controller_get_devdata(spi->master);
+
+	return hisi_sfc_v3xx_generic_exec_op(host, op, chip_select);
+}
+
+static const struct spi_controller_mem_ops hisi_sfc_v3xx_mem_ops = {
+	.adjust_op_size = hisi_sfc_v3xx_adjust_op_size,
+	.exec_op = hisi_sfc_v3xx_exec_op,
+};
+
+static int hisi_sfc_v3xx_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct hisi_sfc_v3xx_host *host;
+	struct spi_controller *ctlr;
+	u32 version;
+	int ret;
+
+	ctlr = spi_alloc_master(&pdev->dev, sizeof(*host));
+	if (!ctlr)
+		return -ENOMEM;
+
+	ctlr->mode_bits = SPI_RX_DUAL | SPI_RX_QUAD |
+			  SPI_TX_DUAL | SPI_TX_QUAD;
+
+	host = spi_controller_get_devdata(ctlr);
+	host->dev = dev;
+
+	platform_set_drvdata(pdev, host);
+
+	host->regbase = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(host->regbase)) {
+		ret = PTR_ERR(host->regbase);
+		goto err_put_master;
+	}
+
+	ctlr->bus_num = -1;
+	ctlr->num_chipselect = 1;
+	ctlr->mem_ops = &hisi_sfc_v3xx_mem_ops;
+
+	version = readl(host->regbase + HISI_SFC_V3XX_VERSION);
+
+	switch (version) {
+	case 0x351:
+		host->max_cmd_dword = 64;
+		break;
+	default:
+		host->max_cmd_dword = 16;
+		break;
+	}
+
+	ret = devm_spi_register_controller(dev, ctlr);
+	if (ret)
+		goto err_put_master;
+
+	dev_info(&pdev->dev, "hw version 0x%x\n", version);
+
+	return 0;
+
+err_put_master:
+	spi_master_put(ctlr);
+	return ret;
+}
+
+#if IS_ENABLED(CONFIG_ACPI)
+static const struct acpi_device_id hisi_sfc_v3xx_acpi_ids[] = {
+	{"HISI0341", 0},
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, hisi_sfc_v3xx_acpi_ids);
+#endif
+
+static struct platform_driver hisi_sfc_v3xx_spi_driver = {
+	.driver = {
+		.name	= "hisi-sfc-v3xx",
+		.acpi_match_table = ACPI_PTR(hisi_sfc_v3xx_acpi_ids),
+	},
+	.probe	= hisi_sfc_v3xx_probe,
+};
+
+module_platform_driver(hisi_sfc_v3xx_spi_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("John Garry <john.garry@huawei.com>");
+MODULE_DESCRIPTION("HiSilicon SPI NOR V3XX Flash Controller Driver for hi16xx chipsets");
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[John Garry]

On 09/12/2019 14:08, John Garry wrote:
> +	if (ret)
> +		return ret;
> +
> +	if (op->data.dir == SPI_MEM_DATA_IN)
> +		hisi_sfc_v3xx_read_databuf(host, op->data.buf.in, len);
> +
> +	return 0;
> +}
> +
> +static int hisi_sfc_v3xx_exec_op(struct spi_mem *mem,
> +				 const struct spi_mem_op *op)
> +{
> +	struct hisi_sfc_v3xx_host *host;
> +	struct spi_device *spi = mem->spi;
> +	u8 chip_select = spi->chip_select;
> +
> +	host = spi_controller_get_devdata(spi->master);
> +
> +	return hisi_sfc_v3xx_generic_exec_op(host, op, chip_select);
> +}
> +
> +static const struct spi_controller_mem_ops hisi_sfc_v3xx_mem_ops = {
> +	.adjust_op_size = hisi_sfc_v3xx_adjust_op_size,
> +	.exec_op = hisi_sfc_v3xx_exec_op,
> +};
> +
> +static int hisi_sfc_v3xx_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct hisi_sfc_v3xx_host *host;
> +	struct spi_controller *ctlr;
> +	u32 version;
> +	int ret;
> +
> +	ctlr = spi_alloc_master(&pdev->dev, sizeof(*host));
> +	if (!ctlr)
> +		return -ENOMEM;
> +


Hi Mark,

> +	ctlr->mode_bits = SPI_RX_DUAL | SPI_RX_QUAD |
> +			  SPI_TX_DUAL | SPI_TX_QUAD;

I have an issue with dual/quad support. I naively thought that setting 
these bits would give me the highest protocol available.

However, now I notice that spi_device.mode needs to be set for supported 
protocols for the slave - I'm using the generic spi mem ops to check if 
protocols are supported based on this value.

 From checking acpi_spi_add_resource() or anywhere else, I cannot see 
how SPI_RX_DUAL or the others are set for spi_device.mode. What am I 
missing? Are these just not supported yet for ACPI? Or should the 
spi-nor code not be relying on this since we should be able to get this 
info from the SPI NOR part?

Cheers,
John

> +
> +	host = spi_controller_get_devdata(ctlr);
> +	host->dev = dev;
> +
> +	platform_set_drvdata(pdev, host);
> +
> +	host->regbase = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(host->regbase)) {
> +		ret = PTR_ERR(host->regbase);
> +		goto err_put_master;


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 957 bytes --]

On Thu, Jan 09, 2020 at 03:54:00PM +0000, John Garry wrote:

> From checking acpi_spi_add_resource() or anywhere else, I cannot see how
> SPI_RX_DUAL or the others are set for spi_device.mode. What am I missing?
> Are these just not supported yet for ACPI? Or should the spi-nor code not be
> relying on this since we should be able to get this info from the SPI NOR
> part?

I'm not aware of any work on integrating this sort of stuff into ACPI
platforms so I think it's just not yet supported in ACPI.  I'm not
really sure what would be idiomatic for ACPI, figuring it out from what
the part supports might well be idiomatic there though I don't know how
common it is for people not to wire up all the data lines even if both
controller and device support wider transfers.  I've got a horrible
feeling that the idiomatic thing is a combination of that and a bunch of
per-device quirks.  There may be a spec I'm not aware of though I'd be a
bit surprised.

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

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[John Garry]

On 09/01/2020 21:28, Mark Brown wrote:
> On Thu, Jan 09, 2020 at 03:54:00PM +0000, John Garry wrote:
> 
>>  From checking acpi_spi_add_resource() or anywhere else, I cannot see how
>> SPI_RX_DUAL or the others are set for spi_device.mode. What am I missing?
>> Are these just not supported yet for ACPI? Or should the spi-nor code not be
>> relying on this since we should be able to get this info from the SPI NOR
>> part?
> 

Hi Mark,

> I'm not aware of any work on integrating this sort of stuff into ACPI
> platforms so I think it's just not yet supported in ACPI.  I'm not
> really sure what would be idiomatic for ACPI, figuring it out from what
> the part supports might well be idiomatic there though I don't know how
> common it is for people not to wire up all the data lines even if both
> controller and device support wider transfers. 

OK, so I guess that is why we require the width property from the FW and 
can't blindly rely on SFDP.

  I've got a horrible
> feeling that the idiomatic thing is a combination of that and a bunch of
> per-device quirks.  There may be a spec I'm not aware of though I'd be a
> bit surprised.
> 

I'm not sure on that. I don't see anything in the ACPI spec.

I will note that PRP0001+"jedec,spi-nor" compatible DSD seems to be the 
defacto method to describe the SPI NOR-compat part for ACPI - that's 
what I'm using. We could add properties there, but that seems improper.

I'll continue to look....

Thanks,
John

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 513 bytes --]

On Fri, Jan 10, 2020 at 11:55:37AM +0000, John Garry wrote:

> I will note that PRP0001+"jedec,spi-nor" compatible DSD seems to be the
> defacto method to describe the SPI NOR-compat part for ACPI - that's what
> I'm using. We could add properties there, but that seems improper.

OK, so that's just reusing the DT binding in which case everything
that's valid for the DT binding should also be valid for ACPI - I
thought that actually worked automatically without you having to do
anything in the code but ICBW.

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

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[John Garry]

On 10/01/2020 14:07, Mark Brown wrote:

+ Mika, Andy

> On Fri, Jan 10, 2020 at 11:55:37AM +0000, John Garry wrote: >
Hi Mark,

>> I will note that PRP0001+"jedec,spi-nor" compatible DSD seems to be the
>> defacto method to describe the SPI NOR-compat part for ACPI - that's what
>> I'm using. We could add properties there, but that seems improper.
> 
> OK, so that's just reusing the DT binding in which case everything
> that's valid for the DT binding should also be valid for ACPI - I
> thought that actually worked automatically without you having to do
> anything in the code but ICBW.

I thought that it would be improper as we could be mixing ACPI methods 
to describe the serial bus (SPI Serial Bus Connection Resource 
Descriptor) and also DT properties which could conflict, like CS active 
high.

However I do see extra properties than "compatible" being added in DSD 
for PRP0001:
https://patchwork.ozlabs.org/patch/662813/ (see EEPROM part)

And if we were to do this, I think that we would need to add some 
device_property_read_u32("spi-rx-bus-width", ...), etc calls in the SPI 
FW parsing for ACPI path - I couldn't see that.

Thanks,
John

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1276 bytes --]

On Fri, Jan 10, 2020 at 02:58:54PM +0000, John Garry wrote:
> On 10/01/2020 14:07, Mark Brown wrote:
> > On Fri, Jan 10, 2020 at 11:55:37AM +0000, John Garry wrote: >

> > OK, so that's just reusing the DT binding in which case everything
> > that's valid for the DT binding should also be valid for ACPI - I
> > thought that actually worked automatically without you having to do
> > anything in the code but ICBW.

> I thought that it would be improper as we could be mixing ACPI methods to
> describe the serial bus (SPI Serial Bus Connection Resource Descriptor) and
> also DT properties which could conflict, like CS active high.

Yes, that's one of the issues with importing bits of DT into ACPI
unfortunately - you will get conflicts, it's not clear it's a good idea
to be using PRP0001 for SPI stuff given that there's bus level bindings
for both ACPI and SPI and they don't line up exactly.

> However I do see extra properties than "compatible" being added in DSD for
> PRP0001:
> https://patchwork.ozlabs.org/patch/662813/ (see EEPROM part)

> And if we were to do this, I think that we would need to add some
> device_property_read_u32("spi-rx-bus-width", ...), etc calls in the SPI FW
> parsing for ACPI path - I couldn't see that.

You'd need parsing code, yes.

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

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[John Garry]

On 10/01/2020 15:12, Mark Brown wrote:
> On Fri, Jan 10, 2020 at 02:58:54PM +0000, John Garry wrote:
>> On 10/01/2020 14:07, Mark Brown wrote:
>>> On Fri, Jan 10, 2020 at 11:55:37AM +0000, John Garry wrote: >
> 

Hi Mark,

>>> OK, so that's just reusing the DT binding in which case everything
>>> that's valid for the DT binding should also be valid for ACPI - I
>>> thought that actually worked automatically without you having to do
>>> anything in the code but ICBW.
> 
>> I thought that it would be improper as we could be mixing ACPI methods to
>> describe the serial bus (SPI Serial Bus Connection Resource Descriptor) and
>> also DT properties which could conflict, like CS active high.
> 
> Yes, that's one of the issues with importing bits of DT into ACPI
> unfortunately - you will get conflicts, it's not clear it's a good idea
> to be using PRP0001 for SPI stuff given that there's bus level bindings
> for both ACPI and SPI and they don't line up exactly.

Yeah, I'm not entirely comfortable with this yet.

> 
>> However I do see extra properties than "compatible" being added in DSD for
>> PRP0001:
>> https://patchwork.ozlabs.org/patch/662813/ (see EEPROM part)
> 
>> And if we were to do this, I think that we would need to add some
>> device_property_read_u32("spi-rx-bus-width", ...), etc calls in the SPI FW
>> parsing for ACPI path - I couldn't see that.
> 
> You'd need parsing code, yes.
> 

I'll continue to check the options.

Thanks,
john

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[Andy Shevchenko]

On Fri, Jan 10, 2020 at 02:58:54PM +0000, John Garry wrote:
> On 10/01/2020 14:07, Mark Brown wrote:
> > On Fri, Jan 10, 2020 at 11:55:37AM +0000, John Garry wrote: >

...

> > > I will note that PRP0001+"jedec,spi-nor" compatible DSD seems to be the
> > > defacto method to describe the SPI NOR-compat part for ACPI - that's what
> > > I'm using. We could add properties there, but that seems improper.
> > 
> > OK, so that's just reusing the DT binding in which case everything
> > that's valid for the DT binding should also be valid for ACPI - I
> > thought that actually worked automatically without you having to do
> > anything in the code but ICBW.
> 
> I thought that it would be improper as we could be mixing ACPI methods to
> describe the serial bus (SPI Serial Bus Connection Resource Descriptor) and
> also DT properties which could conflict, like CS active high.
> 
> However I do see extra properties than "compatible" being added in DSD for
> PRP0001:
> https://patchwork.ozlabs.org/patch/662813/ (see EEPROM part)

PRP method is only for vendors to *test* the hardware in ACPI environment.
The proper method is to allocate correct ACPI ID.

Properties (_DSD in ACPI) may be used in the same way as for DT if we have no
other means in ACPI specification for them.

> And if we were to do this, I think that we would need to add some
> device_property_read_u32("spi-rx-bus-width", ...), etc calls in the SPI FW
> parsing for ACPI path - I couldn't see that.

It's okay as long as you have ACPI ID.

P.S. Most of the sensor drivers were updated in order to support ACPI PRP
method due to DIY hobbyist on IoT sector and embedded devices. This should not
be an official way how we support hardware on ACPI-based platforms.

-- 
With Best Regards,
Andy Shevchenko



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[John Garry]

On 10/01/2020 19:31, Andy Shevchenko wrote:
> On Fri, Jan 10, 2020 at 02:58:54PM +0000, John Garry wrote:
>> On 10/01/2020 14:07, Mark Brown wrote:
>>> On Fri, Jan 10, 2020 at 11:55:37AM +0000, John Garry wrote: >
> 
> ...
> 
>>>> I will note that PRP0001+"jedec,spi-nor" compatible DSD seems to be the
>>>> defacto method to describe the SPI NOR-compat part for ACPI - that's what
>>>> I'm using. We could add properties there, but that seems improper.
>>>
>>> OK, so that's just reusing the DT binding in which case everything
>>> that's valid for the DT binding should also be valid for ACPI - I
>>> thought that actually worked automatically without you having to do
>>> anything in the code but ICBW.
>>
>> I thought that it would be improper as we could be mixing ACPI methods to
>> describe the serial bus (SPI Serial Bus Connection Resource Descriptor) and
>> also DT properties which could conflict, like CS active high.
>>
>> However I do see extra properties than "compatible" being added in DSD for
>> PRP0001:
>> https://patchwork.ozlabs.org/patch/662813/ (see EEPROM part)
> 

Hi Andy,

> PRP method is only for vendors to *test* the hardware in ACPI environment.
> The proper method is to allocate correct ACPI ID.

Yes, that would seem the proper thing to do. So the SPI NOR driver is 
based on micron m25p80 and compatible string is "jedec,spi-nor", so I 
don't know who should or would do this registration.

> 
> Properties (_DSD in ACPI) may be used in the same way as for DT if we have no
> other means in ACPI specification for them.
> 
>> And if we were to do this, I think that we would need to add some
>> device_property_read_u32("spi-rx-bus-width", ...), etc calls in the SPI FW
>> parsing for ACPI path - I couldn't see that.
> 
> It's okay as long as you have ACPI ID.

Well there is none AFAIK.

> 
> P.S. Most of the sensor drivers were updated in order to support ACPI PRP
> method due to DIY hobbyist on IoT sector and embedded devices. This should not
> be an official way how we support hardware on ACPI-based platforms.

Yeah, so we could do this. But, as I mentioned already, this could mean 
that we conflicting properties. For this the kernel driver prob should 
only pay attention to properties which ACPI cannot describe.

Even better would be to update the ACPI spec, especially for something 
general like SPI bus width

BTW, Do any of these sensors you mention have any ACPI standardization?

Thanks,
John

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 953 bytes --]

On Mon, Jan 13, 2020 at 10:09:27AM +0000, John Garry wrote:
> On 10/01/2020 19:31, Andy Shevchenko wrote:

> > PRP method is only for vendors to *test* the hardware in ACPI environment.
> > The proper method is to allocate correct ACPI ID.

> Yes, that would seem the proper thing to do. So the SPI NOR driver is based
> on micron m25p80 and compatible string is "jedec,spi-nor", so I don't know
> who should or would do this registration.

The idiomatic approach appears to be for individual board vendors
to allocate IDs, you do end up with multiple IDs from multiple
vendors for the same thing.

> BTW, Do any of these sensors you mention have any ACPI standardization?

In general there's not really much standardizaiton for devices,
the bindings that do exist aren't really centrally documented and
the Windows standard is just to have the basic device
registration in the firmware and do all properties based on
quirking based on DMI information.

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

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[John Garry]

On 13/01/2020 11:42, Mark Brown wrote:
> On Mon, Jan 13, 2020 at 10:09:27AM +0000, John Garry wrote:
>> On 10/01/2020 19:31, Andy Shevchenko wrote:
> 
>>> PRP method is only for vendors to *test* the hardware in ACPI environment.
>>> The proper method is to allocate correct ACPI ID.
> 
>> Yes, that would seem the proper thing to do. So the SPI NOR driver is based
>> on micron m25p80 and compatible string is "jedec,spi-nor", so I don't know
>> who should or would do this registration.
> 

Hi Mark,

> The idiomatic approach appears to be for individual board vendors
> to allocate IDs, you do end up with multiple IDs from multiple
> vendors for the same thing.

So we see sort of approach a lot when vendors integrate 3rd party IP 
into a SoC and then assign some vendor specific ID for that.

But I am not sure how appropriate that same approach would be for some 
3rd party memory part which we're simply wiring up on our board. Maybe 
it is.

> 
>> BTW, Do any of these sensors you mention have any ACPI standardization?
> 
> In general there's not really much standardizaiton for devices,
> the bindings that do exist aren't really centrally documented and
> the Windows standard is just to have the basic device
> registration in the firmware and do all properties based on
> quirking based on DMI information.
> 

OK, so there is always DMI. I hoped to avoid this sort of thing in the 
linux driver :)

Cheers,
John

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1056 bytes --]

On Mon, Jan 13, 2020 at 01:01:06PM +0000, John Garry wrote:
> On 13/01/2020 11:42, Mark Brown wrote:

> > The idiomatic approach appears to be for individual board vendors
> > to allocate IDs, you do end up with multiple IDs from multiple
> > vendors for the same thing.

> But I am not sure how appropriate that same approach would be for some 3rd
> party memory part which we're simply wiring up on our board. Maybe it is.

It seems to be quite common for Intel reference designs to assign
Intel IDs to non-Intel parts on the board (which is where I
became aware of this practice).  

> > In general there's not really much standardizaiton for devices,
> > the bindings that do exist aren't really centrally documented and
> > the Windows standard is just to have the basic device
> > registration in the firmware and do all properties based on
> > quirking based on DMI information.

> OK, so there is always DMI. I hoped to avoid this sort of thing in the linux
> driver :)

Yes, there are some merits to an approach like that.

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

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[Andy Shevchenko]

On Mon, Jan 13, 2020 at 4:07 PM Mark Brown <broonie@kernel.org> wrote:
> On Mon, Jan 13, 2020 at 01:01:06PM +0000, John Garry wrote:
> > On 13/01/2020 11:42, Mark Brown wrote:
>
> > > The idiomatic approach appears to be for individual board vendors
> > > to allocate IDs, you do end up with multiple IDs from multiple
> > > vendors for the same thing.
>
> > But I am not sure how appropriate that same approach would be for some 3rd
> > party memory part which we're simply wiring up on our board. Maybe it is.
>
> It seems to be quite common for Intel reference designs to assign
> Intel IDs to non-Intel parts on the board (which is where I
> became aware of this practice).

Basically vendor of component in question is responsible for ID, but
it seems they simple don't care.

-- 
With Best Regards,
Andy Shevchenko

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1008 bytes --]

On Mon, Jan 13, 2020 at 04:17:32PM +0200, Andy Shevchenko wrote:
> On Mon, Jan 13, 2020 at 4:07 PM Mark Brown <broonie@kernel.org> wrote:
> > On Mon, Jan 13, 2020 at 01:01:06PM +0000, John Garry wrote:
> > > On 13/01/2020 11:42, Mark Brown wrote:

> > > > The idiomatic approach appears to be for individual board vendors
> > > > to allocate IDs, you do end up with multiple IDs from multiple
> > > > vendors for the same thing.

> > > But I am not sure how appropriate that same approach would be for some 3rd
> > > party memory part which we're simply wiring up on our board. Maybe it is.

> > It seems to be quite common for Intel reference designs to assign
> > Intel IDs to non-Intel parts on the board (which is where I
> > became aware of this practice).

> Basically vendor of component in question is responsible for ID, but
> it seems they simple don't care.

AFAICT a lot of the time it seems to be that whoever is writing
the software ends up assigning an ID, that may not be the silicon
vendor.

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

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[Andy Shevchenko]

On Mon, Jan 13, 2020 at 02:27:54PM +0000, Mark Brown wrote:
> On Mon, Jan 13, 2020 at 04:17:32PM +0200, Andy Shevchenko wrote:
> > On Mon, Jan 13, 2020 at 4:07 PM Mark Brown <broonie@kernel.org> wrote:
> > > On Mon, Jan 13, 2020 at 01:01:06PM +0000, John Garry wrote:
> > > > On 13/01/2020 11:42, Mark Brown wrote:
> 
> > > > > The idiomatic approach appears to be for individual board vendors
> > > > > to allocate IDs, you do end up with multiple IDs from multiple
> > > > > vendors for the same thing.
> 
> > > > But I am not sure how appropriate that same approach would be for some 3rd
> > > > party memory part which we're simply wiring up on our board. Maybe it is.
> 
> > > It seems to be quite common for Intel reference designs to assign
> > > Intel IDs to non-Intel parts on the board (which is where I
> > > became aware of this practice).
> 
> > Basically vendor of component in question is responsible for ID, but
> > it seems they simple don't care.
> 
> AFAICT a lot of the time it seems to be that whoever is writing
> the software ends up assigning an ID, that may not be the silicon
> vendor.

...which is effectively abusing the ACPI ID allocation procedure.

(And yes, Intel itself did it in the past — see badly created ACPI IDs
 in the drivers)

-- 
With Best Regards,
Andy Shevchenko



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[John Garry]

On 13/01/2020 14:34, Andy Shevchenko wrote:
> On Mon, Jan 13, 2020 at 02:27:54PM +0000, Mark Brown wrote:
>> On Mon, Jan 13, 2020 at 04:17:32PM +0200, Andy Shevchenko wrote:
>>> On Mon, Jan 13, 2020 at 4:07 PM Mark Brown <broonie@kernel.org> wrote:
>>>> On Mon, Jan 13, 2020 at 01:01:06PM +0000, John Garry wrote:
>>>>> On 13/01/2020 11:42, Mark Brown wrote:
>>
>>>>>> The idiomatic approach appears to be for individual board vendors
>>>>>> to allocate IDs, you do end up with multiple IDs from multiple
>>>>>> vendors for the same thing.
>>
>>>>> But I am not sure how appropriate that same approach would be for some 3rd
>>>>> party memory part which we're simply wiring up on our board. Maybe it is.
>>
>>>> It seems to be quite common for Intel reference designs to assign
>>>> Intel IDs to non-Intel parts on the board (which is where I
>>>> became aware of this practice).
>>
>>> Basically vendor of component in question is responsible for ID, but
>>> it seems they simple don't care.
>>
>> AFAICT a lot of the time it seems to be that whoever is writing
>> the software ends up assigning an ID, that may not be the silicon
>> vendor.
> 
> ...which is effectively abusing the ACPI ID allocation procedure.
> 
> (And yes, Intel itself did it in the past — see badly created ACPI IDs
>   in the drivers)
> 

Hi Mark,

About this topic of ACPI having no method to describe device buswidth in 
the resource descriptor, it may be an idea for me to raise a Tianocore 
feature request @ https://bugzilla.tianocore.org/

There seems to be an avenue there for raising new features for the spec. 
I (or my org) can't participate in AWSG.

I would have no concrete proposal for spec update for now, though. 
Hopefully others with more expertise could contribute.

In the meantime, I have an RFC for using DMI to quirk support for this 
on the driver - I can share when ready.

Thanks,
John

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[Andy Shevchenko]

On Fri, Jan 31, 2020 at 12:08 PM John Garry <john.garry@huawei.com> wrote:
>
> On 13/01/2020 14:34, Andy Shevchenko wrote:
> > On Mon, Jan 13, 2020 at 02:27:54PM +0000, Mark Brown wrote:
> >> On Mon, Jan 13, 2020 at 04:17:32PM +0200, Andy Shevchenko wrote:
> >>> On Mon, Jan 13, 2020 at 4:07 PM Mark Brown <broonie@kernel.org> wrote:
> >>>> On Mon, Jan 13, 2020 at 01:01:06PM +0000, John Garry wrote:
> >>>>> On 13/01/2020 11:42, Mark Brown wrote:
> >>
> >>>>>> The idiomatic approach appears to be for individual board vendors
> >>>>>> to allocate IDs, you do end up with multiple IDs from multiple
> >>>>>> vendors for the same thing.
> >>
> >>>>> But I am not sure how appropriate that same approach would be for some 3rd
> >>>>> party memory part which we're simply wiring up on our board. Maybe it is.
> >>
> >>>> It seems to be quite common for Intel reference designs to assign
> >>>> Intel IDs to non-Intel parts on the board (which is where I
> >>>> became aware of this practice).
> >>
> >>> Basically vendor of component in question is responsible for ID, but
> >>> it seems they simple don't care.
> >>
> >> AFAICT a lot of the time it seems to be that whoever is writing
> >> the software ends up assigning an ID, that may not be the silicon
> >> vendor.
> >
> > ...which is effectively abusing the ACPI ID allocation procedure.
> >
> > (And yes, Intel itself did it in the past — see badly created ACPI IDs
> >   in the drivers)
> >
>
> Hi Mark,
>
> About this topic of ACPI having no method to describe device buswidth in
> the resource descriptor, it may be an idea for me to raise a Tianocore
> feature request @ https://bugzilla.tianocore.org/
>

The 19.6.126 describes the SPI resource, in particular:

---8<---8<---
DataBitLength is the size, in bits, of the smallest transfer unit for
this connection. _LEN is automatically
created to refer to this portion of the resource descriptor.
---8<---8<---

Is it what you are looking for? (As far as I know most of the
firmwares simple abuse this field among others)

> There seems to be an avenue there for raising new features for the spec.
> I (or my org) can't participate in AWSG.

But have you read 19.6.126?

> I would have no concrete proposal for spec update for now, though.
> Hopefully others with more expertise could contribute.


-- 
With Best Regards,
Andy Shevchenko

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[John Garry]

On 31/01/2020 11:39, Andy Shevchenko wrote:
> On Fri, Jan 31, 2020 at 12:08 PM John Garry <john.garry@huawei.com> wrote:
>>
>> On 13/01/2020 14:34, Andy Shevchenko wrote:
>>> On Mon, Jan 13, 2020 at 02:27:54PM +0000, Mark Brown wrote:
>>>> On Mon, Jan 13, 2020 at 04:17:32PM +0200, Andy Shevchenko wrote:
>>>>> On Mon, Jan 13, 2020 at 4:07 PM Mark Brown <broonie@kernel.org> wrote:
>>>>>> On Mon, Jan 13, 2020 at 01:01:06PM +0000, John Garry wrote:
>>>>>>> On 13/01/2020 11:42, Mark Brown wrote:
>>>>
>>>>>>>> The idiomatic approach appears to be for individual board vendors
>>>>>>>> to allocate IDs, you do end up with multiple IDs from multiple
>>>>>>>> vendors for the same thing.
>>>>
>>>>>>> But I am not sure how appropriate that same approach would be for some 3rd
>>>>>>> party memory part which we're simply wiring up on our board. Maybe it is.
>>>>
>>>>>> It seems to be quite common for Intel reference designs to assign
>>>>>> Intel IDs to non-Intel parts on the board (which is where I
>>>>>> became aware of this practice).
>>>>
>>>>> Basically vendor of component in question is responsible for ID, but
>>>>> it seems they simple don't care.
>>>>
>>>> AFAICT a lot of the time it seems to be that whoever is writing
>>>> the software ends up assigning an ID, that may not be the silicon
>>>> vendor.
>>>
>>> ...which is effectively abusing the ACPI ID allocation procedure.
>>>
>>> (And yes, Intel itself did it in the past — see badly created ACPI IDs
>>>    in the drivers)
>>>
>>
>> Hi Mark,
>>

Hi Andy,

>> About this topic of ACPI having no method to describe device buswidth in
>> the resource descriptor, it may be an idea for me to raise a Tianocore
>> feature request @ https://bugzilla.tianocore.org/
>>
> 
> The 19.6.126 describes the SPI resource, in particular:
> 
> ---8<---8<---
> DataBitLength is the size, in bits, of the smallest transfer unit for
> this connection. _LEN is automatically
> created to refer to this portion of the resource descriptor.
> ---8<---8<---
> 
> Is it what you are looking for? (As far as I know most of the
> firmwares simple abuse this field among others)

I didn't think so - I thought that there was a distinction between width 
and length in SPI terms.

So how do you find that most firmwares abuse this field? AFAICS, linux 
kernel doesn't interpret this field at all.

> 
>> There seems to be an avenue there for raising new features for the spec.
>> I (or my org) can't participate in AWSG.
> 
> But have you read 19.6.126?
> 

Maybe some clarification at least could be achieved :)

Cheers,
John

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[Andy Shevchenko]

On Fri, Jan 31, 2020 at 2:03 PM John Garry <john.garry@huawei.com> wrote:
> On 31/01/2020 11:39, Andy Shevchenko wrote:
> > On Fri, Jan 31, 2020 at 12:08 PM John Garry <john.garry@huawei.com> wrote:
> >> On 13/01/2020 14:34, Andy Shevchenko wrote:

...

> >> About this topic of ACPI having no method to describe device buswidth in
> >> the resource descriptor, it may be an idea for me to raise a Tianocore
> >> feature request @ https://bugzilla.tianocore.org/
> >>
> >
> > The 19.6.126 describes the SPI resource, in particular:
> >
> > ---8<---8<---
> > DataBitLength is the size, in bits, of the smallest transfer unit for
> > this connection. _LEN is automatically
> > created to refer to this portion of the resource descriptor.
> > ---8<---8<---
> >
> > Is it what you are looking for? (As far as I know most of the
> > firmwares simple abuse this field among others)
>
> I didn't think so - I thought that there was a distinction between width
> and length in SPI terms.

My interpretation of this field is a data width of the slave.
Basically what we have as transfer->size inside SPI in the Linux
kernel.

> So how do you find that most firmwares abuse this field? AFAICS, linux
> kernel doesn't interpret this field at all.

From all tables I have this is the result of appearance (some of the
tables are like 10x times present in my data base, but nevertheless)

    140 SpiSerialBusV2(0x0000,PolarityHigh,FourWireMode,0x08,
   411 SpiSerialBusV2(0x0000,PolarityLow,FourWireMode,0x08,
     1 SpiSerialBusV2(0x0000,PolarityLow,FourWireMode,0x08,
    36 SpiSerialBusV2(0x0000,PolarityLow,FourWireMode,0x10,
    35 SpiSerialBusV2(0x0000,PolarityLow,FourWireMode,0x18,
    35 SpiSerialBusV2(0x0000,PolarityLow,FourWireMode,0x20,
     1 SpiSerialBusV2(0x0000,PolarityLow,ThreeWireMode,0x10,
     8 SpiSerialBusV2(0x0001,PolarityLow,FourWireMode,0x08,
     1 SpiSerialBusV2(0x0001,PolarityLow,FourWireMode,0x10,

So, it seems I stand corrected, the field is in right use, although
cases like 0x10 and 0x20 should be carefully checked.

We may teach kernel to get something meaningful out of it.

-- 
With Best Regards,
Andy Shevchenko

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[John Garry]

On 31/01/2020 15:46, Andy Shevchenko wrote:
> On Fri, Jan 31, 2020 at 2:03 PM John Garry <john.garry@huawei.com> wrote:
>> On 31/01/2020 11:39, Andy Shevchenko wrote:
>>> On Fri, Jan 31, 2020 at 12:08 PM John Garry <john.garry@huawei.com> wrote:
>>>> On 13/01/2020 14:34, Andy Shevchenko wrote:
> 
> ...
> 
>>>> About this topic of ACPI having no method to describe device buswidth in
>>>> the resource descriptor, it may be an idea for me to raise a Tianocore
>>>> feature request @ https://bugzilla.tianocore.org/
>>>>
>>>
>>> The 19.6.126 describes the SPI resource, in particular:
>>>
>>> ---8<---8<---
>>> DataBitLength is the size, in bits, of the smallest transfer unit for
>>> this connection. _LEN is automatically
>>> created to refer to this portion of the resource descriptor.
>>> ---8<---8<---
>>>

Hi Andy,

>>> Is it what you are looking for? (As far as I know most of the
>>> firmwares simple abuse this field among others)
>>
>> I didn't think so - I thought that there was a distinction between width
>> and length in SPI terms.
> 
> My interpretation of this field is a data width of the slave.
> Basically what we have as transfer->size inside SPI in the Linux
> kernel.
> 
>> So how do you find that most firmwares abuse this field? AFAICS, linux
>> kernel doesn't interpret this field at all.
> 
>>From all tables I have this is the result of appearance (some of the
> tables are like 10x times present in my data base, but nevertheless)
> 
>      140 SpiSerialBusV2(0x0000,PolarityHigh,FourWireMode,0x08,
>     411 SpiSerialBusV2(0x0000,PolarityLow,FourWireMode,0x08,
>       1 SpiSerialBusV2(0x0000,PolarityLow,FourWireMode,0x08,
>      36 SpiSerialBusV2(0x0000,PolarityLow,FourWireMode,0x10,
>      35 SpiSerialBusV2(0x0000,PolarityLow,FourWireMode,0x18,
>      35 SpiSerialBusV2(0x0000,PolarityLow,FourWireMode,0x20,
>       1 SpiSerialBusV2(0x0000,PolarityLow,ThreeWireMode,0x10,
>       8 SpiSerialBusV2(0x0001,PolarityLow,FourWireMode,0x08,
>       1 SpiSerialBusV2(0x0001,PolarityLow,FourWireMode,0x10,
> 
> So, it seems I stand corrected, the field is in right use, although
> cases like 0x10 and 0x20 should be carefully checked.
> 
> We may teach kernel to get something meaningful out of it.
> 

It seems that someone already had a go at that:
https://lore.kernel.org/lkml/20170317212143.bogj6efzyvvf24yd@sirena.org.uk/

Thanks,
John


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 997 bytes --]

On Fri, Jan 31, 2020 at 04:26:46PM +0000, John Garry wrote:
> On 31/01/2020 15:46, Andy Shevchenko wrote:

> > So, it seems I stand corrected, the field is in right use, although
> > cases like 0x10 and 0x20 should be carefully checked.

> > We may teach kernel to get something meaningful out of it.

> It seems that someone already had a go at that:
> https://lore.kernel.org/lkml/20170317212143.bogj6efzyvvf24yd@sirena.org.uk/

This is a discussion about supporting DT bindings for
bits-per-word which is a different thing again, that's the size
of a data word which is not connected with the physical wiring.

Please include human readable descriptions of things like commits and
issues being discussed in e-mail in your mails, this makes them much
easier for humans to read especially when they have no internet access.
I do frequently catch up on my mail on flights or while otherwise
travelling so this is even more pressing for me than just being about
making things a bit easier to read.

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

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 2/3] spi: Add HiSilicon v3xx SPI NOR flash controller driver

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1488 bytes --]

On Fri, Jan 31, 2020 at 05:46:39PM +0200, Andy Shevchenko wrote:
> On Fri, Jan 31, 2020 at 2:03 PM John Garry <john.garry@huawei.com> wrote:
> > On 31/01/2020 11:39, Andy Shevchenko wrote:

> > > DataBitLength is the size, in bits, of the smallest transfer unit for
> > > this connection. _LEN is automatically
> > > created to refer to this portion of the resource descriptor.

> > > Is it what you are looking for? (As far as I know most of the
> > > firmwares simple abuse this field among others)

> > I didn't think so - I thought that there was a distinction between width
> > and length in SPI terms.

> My interpretation of this field is a data width of the slave.
> Basically what we have as transfer->size inside SPI in the Linux
> kernel.

This discussion is about the number of data lines, SPI_TX_QUAD
and friends.

>      1 SpiSerialBusV2(0x0000,PolarityLow,FourWireMode,0x08,
>     36 SpiSerialBusV2(0x0000,PolarityLow,FourWireMode,0x10,
>     35 SpiSerialBusV2(0x0000,PolarityLow,FourWireMode,0x18,
>     35 SpiSerialBusV2(0x0000,PolarityLow,FourWireMode,0x20,
>      1 SpiSerialBusV2(0x0000,PolarityLow,ThreeWireMode,0x10,
>      8 SpiSerialBusV2(0x0001,PolarityLow,FourWireMode,0x08,
>      1 SpiSerialBusV2(0x0001,PolarityLow,FourWireMode,0x10,

> So, it seems I stand corrected, the field is in right use, although
> cases like 0x10 and 0x20 should be carefully checked.

Those look like they're mainly controlling SPI_3WIRE so it does
look like a reasonable fit, yes.

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

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Applied "spi: Add HiSilicon v3xx SPI NOR flash controller driver" to the spi tree

[Mark Brown]

The patch

   spi: Add HiSilicon v3xx SPI NOR flash controller driver

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.6

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From a2ca53b52e007de81752bbb443d828f5950d6d04 Mon Sep 17 00:00:00 2001
From: John Garry <john.garry@huawei.com>
Date: Mon, 9 Dec 2019 22:08:09 +0800
Subject: [PATCH] spi: Add HiSilicon v3xx SPI NOR flash controller driver

Add the driver for the HiSilicon v3xx SPI NOR flash controller, commonly
found in hi16xx chipsets.

This is a different controller than that in drivers/mtd/spi-nor/hisi-sfc.c;
indeed, the naming for that driver is poor, since it is really known as
FMC, and can support other memory technologies.

The driver module name is "hisi-sfc-v3xx", as recommended by HW designer,
being an attempt to provide a distinct name - v3xx being the unique
controller versioning.

Only ACPI firmware is supported.

DMA is not supported, and we just use polling mode for operation
completion notification.

The driver uses the SPI MEM OPs.

Signed-off-by: John Garry <john.garry@huawei.com>
Link: https://lore.kernel.org/r/1575900490-74467-3-git-send-email-john.garry@huawei.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/Kconfig             |   9 +
 drivers/spi/Makefile            |   1 +
 drivers/spi/spi-hisi-sfc-v3xx.c | 284 ++++++++++++++++++++++++++++++++
 3 files changed, 294 insertions(+)
 create mode 100644 drivers/spi/spi-hisi-sfc-v3xx.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 870f7797b56b..d6ed0c355954 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -281,6 +281,15 @@ config SPI_FSL_QUADSPI
 	  This controller does not support generic SPI messages. It only
 	  supports the high-level SPI memory interface.
 
+config SPI_HISI_SFC_V3XX
+	tristate "HiSilicon SPI-NOR Flash Controller for Hi16XX chipsets"
+	depends on (ARM64 && ACPI) || COMPILE_TEST
+	depends on HAS_IOMEM
+	select CONFIG_MTD_SPI_NOR
+	help
+	  This enables support for HiSilicon v3xx SPI-NOR flash controller
+	  found in hi16xx chipsets.
+
 config SPI_NXP_FLEXSPI
 	tristate "NXP Flex SPI controller"
 	depends on ARCH_LAYERSCAPE || HAS_IOMEM
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index bb49c9e6d0a0..9b65ec5afc5e 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_SPI_FSL_LPSPI)		+= spi-fsl-lpspi.o
 obj-$(CONFIG_SPI_FSL_QUADSPI)		+= spi-fsl-qspi.o
 obj-$(CONFIG_SPI_FSL_SPI)		+= spi-fsl-spi.o
 obj-$(CONFIG_SPI_GPIO)			+= spi-gpio.o
+obj-$(CONFIG_SPI_HISI_SFC_V3XX)		+= spi-hisi-sfc-v3xx.o
 obj-$(CONFIG_SPI_IMG_SPFI)		+= spi-img-spfi.o
 obj-$(CONFIG_SPI_IMX)			+= spi-imx.o
 obj-$(CONFIG_SPI_LANTIQ_SSC)		+= spi-lantiq-ssc.o
diff --git a/drivers/spi/spi-hisi-sfc-v3xx.c b/drivers/spi/spi-hisi-sfc-v3xx.c
new file mode 100644
index 000000000000..4cf8fc80a7b7
--- /dev/null
+++ b/drivers/spi/spi-hisi-sfc-v3xx.c
@@ -0,0 +1,284 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// HiSilicon SPI NOR V3XX Flash Controller Driver for hi16xx chipsets
+//
+// Copyright (c) 2019 HiSilicon Technologies Co., Ltd.
+// Author: John Garry <john.garry@huawei.com>
+
+#include <linux/acpi.h>
+#include <linux/bitops.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
+
+#define HISI_SFC_V3XX_VERSION (0x1f8)
+
+#define HISI_SFC_V3XX_CMD_CFG (0x300)
+#define HISI_SFC_V3XX_CMD_CFG_DATA_CNT_OFF 9
+#define HISI_SFC_V3XX_CMD_CFG_RW_MSK BIT(8)
+#define HISI_SFC_V3XX_CMD_CFG_DATA_EN_MSK BIT(7)
+#define HISI_SFC_V3XX_CMD_CFG_DUMMY_CNT_OFF 4
+#define HISI_SFC_V3XX_CMD_CFG_ADDR_EN_MSK BIT(3)
+#define HISI_SFC_V3XX_CMD_CFG_CS_SEL_OFF 1
+#define HISI_SFC_V3XX_CMD_CFG_START_MSK BIT(0)
+#define HISI_SFC_V3XX_CMD_INS (0x308)
+#define HISI_SFC_V3XX_CMD_ADDR (0x30c)
+#define HISI_SFC_V3XX_CMD_DATABUF0 (0x400)
+
+struct hisi_sfc_v3xx_host {
+	struct device *dev;
+	void __iomem *regbase;
+	int max_cmd_dword;
+};
+
+#define HISI_SFC_V3XX_WAIT_TIMEOUT_US		1000000
+#define HISI_SFC_V3XX_WAIT_POLL_INTERVAL_US	10
+
+static int hisi_sfc_v3xx_wait_cmd_idle(struct hisi_sfc_v3xx_host *host)
+{
+	u32 reg;
+
+	return readl_poll_timeout(host->regbase + HISI_SFC_V3XX_CMD_CFG, reg,
+				  !(reg & HISI_SFC_V3XX_CMD_CFG_START_MSK),
+				  HISI_SFC_V3XX_WAIT_POLL_INTERVAL_US,
+				  HISI_SFC_V3XX_WAIT_TIMEOUT_US);
+}
+
+static int hisi_sfc_v3xx_adjust_op_size(struct spi_mem *mem,
+					struct spi_mem_op *op)
+{
+	struct spi_device *spi = mem->spi;
+	struct hisi_sfc_v3xx_host *host;
+	uintptr_t addr = (uintptr_t)op->data.buf.in;
+	int max_byte_count;
+
+	host = spi_controller_get_devdata(spi->master);
+
+	max_byte_count = host->max_cmd_dword * 4;
+
+	if (!IS_ALIGNED(addr, 4) && op->data.nbytes >= 4)
+		op->data.nbytes = 4 - (addr % 4);
+	else if (op->data.nbytes > max_byte_count)
+		op->data.nbytes = max_byte_count;
+
+	return 0;
+}
+
+/*
+ * memcpy_{to,from}io doesn't gurantee 32b accesses - which we require for the
+ * DATABUF registers -so use __io{read,write}32_copy when possible. For
+ * trailing bytes, copy them byte-by-byte from the DATABUF register, as we
+ * can't clobber outside the source/dest buffer.
+ *
+ * For efficient data read/write, we try to put any start 32b unaligned data
+ * into a separate transaction in hisi_sfc_v3xx_adjust_op_size().
+ */
+static void hisi_sfc_v3xx_read_databuf(struct hisi_sfc_v3xx_host *host,
+				       u8 *to, unsigned int len)
+{
+	void __iomem *from;
+	int i;
+
+	from = host->regbase + HISI_SFC_V3XX_CMD_DATABUF0;
+
+	if (IS_ALIGNED((uintptr_t)to, 4)) {
+		int words = len / 4;
+
+		__ioread32_copy(to, from, words);
+
+		len -= words * 4;
+		if (len) {
+			u32 val;
+
+			to += words * 4;
+			from += words * 4;
+
+			val = __raw_readl(from);
+
+			for (i = 0; i < len; i++, val >>= 8, to++)
+				*to = (u8)val;
+		}
+	} else {
+		for (i = 0; i < DIV_ROUND_UP(len, 4); i++, from += 4) {
+			u32 val = __raw_readl(from);
+			int j;
+
+			for (j = 0; j < 4 && (j + (i * 4) < len);
+			     to++, val >>= 8, j++)
+				*to = (u8)val;
+		}
+	}
+}
+
+static void hisi_sfc_v3xx_write_databuf(struct hisi_sfc_v3xx_host *host,
+					const u8 *from, unsigned int len)
+{
+	void __iomem *to;
+	int i;
+
+	to = host->regbase + HISI_SFC_V3XX_CMD_DATABUF0;
+
+	if (IS_ALIGNED((uintptr_t)from, 4)) {
+		int words = len / 4;
+
+		__iowrite32_copy(to, from, words);
+
+		len -= words * 4;
+		if (len) {
+			u32 val = 0;
+
+			to += words * 4;
+			from += words * 4;
+
+			for (i = 0; i < len; i++, from++)
+				val |= *from << i * 8;
+			__raw_writel(val, to);
+		}
+
+	} else {
+		for (i = 0; i < DIV_ROUND_UP(len, 4); i++, to += 4) {
+			u32 val = 0;
+			int j;
+
+			for (j = 0; j < 4 && (j + (i * 4) < len);
+			     from++, j++)
+				val |= *from << j * 8;
+			__raw_writel(val, to);
+		}
+	}
+}
+
+static int hisi_sfc_v3xx_generic_exec_op(struct hisi_sfc_v3xx_host *host,
+					 const struct spi_mem_op *op,
+					 u8 chip_select)
+{
+	int ret, len = op->data.nbytes;
+	u32 config = 0;
+
+	if (op->addr.nbytes)
+		config |= HISI_SFC_V3XX_CMD_CFG_ADDR_EN_MSK;
+
+	if (op->data.dir != SPI_MEM_NO_DATA) {
+		config |= (len - 1) << HISI_SFC_V3XX_CMD_CFG_DATA_CNT_OFF;
+		config |= HISI_SFC_V3XX_CMD_CFG_DATA_EN_MSK;
+	}
+
+	if (op->data.dir == SPI_MEM_DATA_OUT)
+		hisi_sfc_v3xx_write_databuf(host, op->data.buf.out, len);
+	else if (op->data.dir == SPI_MEM_DATA_IN)
+		config |= HISI_SFC_V3XX_CMD_CFG_RW_MSK;
+
+	config |= op->dummy.nbytes << HISI_SFC_V3XX_CMD_CFG_DUMMY_CNT_OFF |
+		  chip_select << HISI_SFC_V3XX_CMD_CFG_CS_SEL_OFF |
+		  HISI_SFC_V3XX_CMD_CFG_START_MSK;
+
+	writel(op->addr.val, host->regbase + HISI_SFC_V3XX_CMD_ADDR);
+	writel(op->cmd.opcode, host->regbase + HISI_SFC_V3XX_CMD_INS);
+
+	writel(config, host->regbase + HISI_SFC_V3XX_CMD_CFG);
+
+	ret = hisi_sfc_v3xx_wait_cmd_idle(host);
+	if (ret)
+		return ret;
+
+	if (op->data.dir == SPI_MEM_DATA_IN)
+		hisi_sfc_v3xx_read_databuf(host, op->data.buf.in, len);
+
+	return 0;
+}
+
+static int hisi_sfc_v3xx_exec_op(struct spi_mem *mem,
+				 const struct spi_mem_op *op)
+{
+	struct hisi_sfc_v3xx_host *host;
+	struct spi_device *spi = mem->spi;
+	u8 chip_select = spi->chip_select;
+
+	host = spi_controller_get_devdata(spi->master);
+
+	return hisi_sfc_v3xx_generic_exec_op(host, op, chip_select);
+}
+
+static const struct spi_controller_mem_ops hisi_sfc_v3xx_mem_ops = {
+	.adjust_op_size = hisi_sfc_v3xx_adjust_op_size,
+	.exec_op = hisi_sfc_v3xx_exec_op,
+};
+
+static int hisi_sfc_v3xx_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct hisi_sfc_v3xx_host *host;
+	struct spi_controller *ctlr;
+	u32 version;
+	int ret;
+
+	ctlr = spi_alloc_master(&pdev->dev, sizeof(*host));
+	if (!ctlr)
+		return -ENOMEM;
+
+	ctlr->mode_bits = SPI_RX_DUAL | SPI_RX_QUAD |
+			  SPI_TX_DUAL | SPI_TX_QUAD;
+
+	host = spi_controller_get_devdata(ctlr);
+	host->dev = dev;
+
+	platform_set_drvdata(pdev, host);
+
+	host->regbase = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(host->regbase)) {
+		ret = PTR_ERR(host->regbase);
+		goto err_put_master;
+	}
+
+	ctlr->bus_num = -1;
+	ctlr->num_chipselect = 1;
+	ctlr->mem_ops = &hisi_sfc_v3xx_mem_ops;
+
+	version = readl(host->regbase + HISI_SFC_V3XX_VERSION);
+
+	switch (version) {
+	case 0x351:
+		host->max_cmd_dword = 64;
+		break;
+	default:
+		host->max_cmd_dword = 16;
+		break;
+	}
+
+	ret = devm_spi_register_controller(dev, ctlr);
+	if (ret)
+		goto err_put_master;
+
+	dev_info(&pdev->dev, "hw version 0x%x\n", version);
+
+	return 0;
+
+err_put_master:
+	spi_master_put(ctlr);
+	return ret;
+}
+
+#if IS_ENABLED(CONFIG_ACPI)
+static const struct acpi_device_id hisi_sfc_v3xx_acpi_ids[] = {
+	{"HISI0341", 0},
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, hisi_sfc_v3xx_acpi_ids);
+#endif
+
+static struct platform_driver hisi_sfc_v3xx_spi_driver = {
+	.driver = {
+		.name	= "hisi-sfc-v3xx",
+		.acpi_match_table = ACPI_PTR(hisi_sfc_v3xx_acpi_ids),
+	},
+	.probe	= hisi_sfc_v3xx_probe,
+};
+
+module_platform_driver(hisi_sfc_v3xx_spi_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("John Garry <john.garry@huawei.com>");
+MODULE_DESCRIPTION("HiSilicon SPI NOR V3XX Flash Controller Driver for hi16xx chipsets");
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v2 3/3] MAINTAINERS: Add a maintainer for the HiSilicon v3xx SFC driver

[John Garry]

Set John Garry @ Huawei as the maintainer.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index bd5847e802de..8c119bd4f687 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7492,6 +7492,12 @@ S:	Supported
 F:	drivers/scsi/hisi_sas/
 F:	Documentation/devicetree/bindings/scsi/hisilicon-sas.txt
 
+HISILICON V3XX SPI NOR FLASH Controller Driver
+M:	John Garry <john.garry@huawei.com>
+W:	http://www.hisilicon.com
+S:	Maintained
+F:	drivers/spi/spi-hisi-sfc-v3xx.c
+
 HISILICON QM AND ZIP Controller DRIVER
 M:	Zhou Wang <wangzhou1@hisilicon.com>
 L:	linux-crypto@vger.kernel.org
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Applied "MAINTAINERS: Add a maintainer for the HiSilicon v3xx SFC driver" to the spi tree

[Mark Brown]

The patch

   MAINTAINERS: Add a maintainer for the HiSilicon v3xx SFC driver

has been applied to the spi tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.6

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 99eb0a1221ac1083b8c87932c438ef016abdaa05 Mon Sep 17 00:00:00 2001
From: John Garry <john.garry@huawei.com>
Date: Mon, 9 Dec 2019 22:08:10 +0800
Subject: [PATCH] MAINTAINERS: Add a maintainer for the HiSilicon v3xx SFC
 driver

Set John Garry @ Huawei as the maintainer.

Signed-off-by: John Garry <john.garry@huawei.com>
Link: https://lore.kernel.org/r/1575900490-74467-4-git-send-email-john.garry@huawei.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index bd5847e802de..8c119bd4f687 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7492,6 +7492,12 @@ S:	Supported
 F:	drivers/scsi/hisi_sas/
 F:	Documentation/devicetree/bindings/scsi/hisilicon-sas.txt
 
+HISILICON V3XX SPI NOR FLASH Controller Driver
+M:	John Garry <john.garry@huawei.com>
+W:	http://www.hisilicon.com
+S:	Maintained
+F:	drivers/spi/spi-hisi-sfc-v3xx.c
+
 HISILICON QM AND ZIP Controller DRIVER
 M:	Zhou Wang <wangzhou1@hisilicon.com>
 L:	linux-crypto@vger.kernel.org
-- 
2.20.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 0/3] HiSilicon v3xx SFC driver

[John Garry]

On 09/12/2019 14:08, John Garry wrote:
> This patchset introduces support for the HiSilicon SFC V3XX driver.
> 

Hi guys,

Just a friendly reminder on this series.

Thanks,
John

> Whilst the kernel tree already includes support for a "HiSilicon SFC
> driver", that is for different HW. Indeed, as mentioned in patch #1, the
> naming for that driver could be better, as it should support more memory
> technologies than SPI NOR (as I have been told), and it is actually known
> internally as FMC. As such, maybe "hisi-fmc" would have been better, but
> we can't change that now.
> 
> I used V3XX in this driver name, as that is the unique versioning for
> this HW.
> 
> As for the driver itself, it is quite simple. Only ACPI firmware is
> supported, and we assume m25p80 compatible SPI NOR part will be used.
> 
> DMA is not supported, and we just use polling mode for operation
> completion notification. The driver uses the SPI MEM OPs.
> 
> Changes from v1:
> - Add ACPI kconfig dependency
> - Fix up header comment style
> - Change macros naming style
> - Try to enforce aligned accesses in hisi_sfc_v3xx_adjust_op_size()
> 
> John Garry (3):
>    mtd: spi-nor: hisi-sfc: Try to provide some clarity on which SFC we
>      are
>    spi: Add HiSilicon v3xx SPI NOR flash controller driver
>    MAINTAINERS: Add a maintainer for the HiSilicon v3xx SFC driver
> 
>   MAINTAINERS                     |   6 +
>   drivers/mtd/spi-nor/Kconfig     |   4 +-
>   drivers/mtd/spi-nor/hisi-sfc.c  |   2 +-
>   drivers/spi/Kconfig             |   9 +
>   drivers/spi/Makefile            |   1 +
>   drivers/spi/spi-hisi-sfc-v3xx.c | 284 ++++++++++++++++++++++++++++++++
>   6 files changed, 303 insertions(+), 3 deletions(-)
>   create mode 100644 drivers/spi/spi-hisi-sfc-v3xx.c
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2 0/3] HiSilicon v3xx SFC driver

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1011 bytes --]

On Mon, Dec 16, 2019 at 02:52:23PM +0000, John Garry wrote:
> On 09/12/2019 14:08, John Garry wrote:
> > This patchset introduces support for the HiSilicon SFC V3XX driver.
> > 
> 
> Hi guys,
> 
> Just a friendly reminder on this series.

Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.

Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, so sending again is generally a better approach though there are
some other maintainers who like them - if in doubt look at how patches
for the subsystem are normally handled.

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

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/