[PATCH v2 0/6] add regmap & indirect access support

[PATCH v2 0/6] add regmap & indirect access support

[Xu Yilun]

Updated the regmap & indirect access support for spi-altera.

Patch #1, #2, #3 is already applied.
Patch #4 is an 1:1 replacement of of readl/writel with regmap_read/write
Patch #5 introduced a new platform_device_id to support indirect access as
         a sub device.
Patch #6 is a minor fix.

Main changes from v1: 
 - Split the v1 Patch #4 to v2 Patch #4 & #5. 
 - Add a new platform_device_id to support indirect access.
 - Removed the v1 Patch #5. Now we use driver name string directly.
 - Add Yilun's Signed-off-by for Patch #6


Matthew Gerlach (1):
  spi: altera: fix size mismatch on 64 bit processors

Xu Yilun (5):
  spi: altera: add 32bit data width transfer support.
  spi: altera: add SPI core parameters support via platform data.
  spi: altera: add platform data for slave information.
  spi: altera: use regmap-mmio instead of direct mmio register access
  spi: altera: support indirect access to the registers

 drivers/spi/Kconfig        |   1 +
 drivers/spi/spi-altera.c   | 177 +++++++++++++++++++++++++++++++++++++++------
 include/linux/spi/altera.h |  29 ++++++++
 3 files changed, 183 insertions(+), 24 deletions(-)
 create mode 100644 include/linux/spi/altera.h

-- 
2.7.4

[PATCH v2 4/6] spi: altera: use regmap-mmio instead of direct mmio register access

[Xu Yilun]

This patch adds support for regmap. It makes preparation for supporting
different ways to access the registers.

Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
 drivers/spi/Kconfig      |  1 +
 drivers/spi/spi-altera.c | 91 ++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 73 insertions(+), 19 deletions(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 8f1f8fc..6d79fc7 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -59,6 +59,7 @@ comment "SPI Master Controller Drivers"
 
 config SPI_ALTERA
 	tristate "Altera SPI Controller"
+	select REGMAP_MMIO
 	help
 	  This is the driver for the Altera SPI Controller.
 
diff --git a/drivers/spi/spi-altera.c b/drivers/spi/spi-altera.c
index aa9d1a2..b215bdf 100644
--- a/drivers/spi/spi-altera.c
+++ b/drivers/spi/spi-altera.c
@@ -44,7 +44,6 @@
 #define ALTERA_SPI_MAX_CS		32
 
 struct altera_spi {
-	void __iomem *base;
 	int irq;
 	int len;
 	int count;
@@ -54,8 +53,43 @@ struct altera_spi {
 	/* data buffers */
 	const unsigned char *tx;
 	unsigned char *rx;
+
+	struct regmap *regmap;
+	struct device *dev;
+};
+
+static const struct regmap_config spi_altera_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.fast_io = true,
 };
 
+static int altr_spi_writel(struct altera_spi *hw, unsigned int reg,
+			   unsigned int val)
+{
+	int ret;
+
+	ret = regmap_write(hw->regmap, reg, val);
+	if (ret)
+		dev_err(hw->dev, "fail to write reg 0x%x val 0x%x: %d\n",
+			reg, val, ret);
+
+	return ret;
+}
+
+static int altr_spi_readl(struct altera_spi *hw, unsigned int reg,
+			  unsigned int *val)
+{
+	int ret;
+
+	ret = regmap_read(hw->regmap, reg, val);
+	if (ret)
+		dev_err(hw->dev, "fail to read reg 0x%x: %d\n", reg, ret);
+
+	return ret;
+}
+
 static inline struct altera_spi *altera_spi_to_hw(struct spi_device *sdev)
 {
 	return spi_master_get_devdata(sdev->master);
@@ -67,12 +101,13 @@ static void altera_spi_set_cs(struct spi_device *spi, bool is_high)
 
 	if (is_high) {
 		hw->imr &= ~ALTERA_SPI_CONTROL_SSO_MSK;
-		writel(hw->imr, hw->base + ALTERA_SPI_CONTROL);
-		writel(0, hw->base + ALTERA_SPI_SLAVE_SEL);
+		altr_spi_writel(hw, ALTERA_SPI_CONTROL, hw->imr);
+		altr_spi_writel(hw, ALTERA_SPI_SLAVE_SEL, 0);
 	} else {
-		writel(BIT(spi->chip_select), hw->base + ALTERA_SPI_SLAVE_SEL);
+		altr_spi_writel(hw, ALTERA_SPI_SLAVE_SEL,
+				BIT(spi->chip_select));
 		hw->imr |= ALTERA_SPI_CONTROL_SSO_MSK;
-		writel(hw->imr, hw->base + ALTERA_SPI_CONTROL);
+		altr_spi_writel(hw, ALTERA_SPI_CONTROL, hw->imr);
 	}
 }
 
@@ -99,14 +134,14 @@ static void altera_spi_tx_word(struct altera_spi *hw)
 		}
 	}
 
-	writel(txd, hw->base + ALTERA_SPI_TXDATA);
+	altr_spi_writel(hw, ALTERA_SPI_TXDATA, txd);
 }
 
 static void altera_spi_rx_word(struct altera_spi *hw)
 {
 	unsigned int rxd;
 
-	rxd = readl(hw->base + ALTERA_SPI_RXDATA);
+	altr_spi_readl(hw, ALTERA_SPI_RXDATA, &rxd);
 	if (hw->rx) {
 		switch (hw->bytes_per_word) {
 		case 1:
@@ -133,6 +168,7 @@ static int altera_spi_txrx(struct spi_master *master,
 	struct spi_device *spi, struct spi_transfer *t)
 {
 	struct altera_spi *hw = spi_master_get_devdata(master);
+	u32 val;
 
 	hw->tx = t->tx_buf;
 	hw->rx = t->rx_buf;
@@ -143,7 +179,7 @@ static int altera_spi_txrx(struct spi_master *master,
 	if (hw->irq >= 0) {
 		/* enable receive interrupt */
 		hw->imr |= ALTERA_SPI_CONTROL_IRRDY_MSK;
-		writel(hw->imr, hw->base + ALTERA_SPI_CONTROL);
+		altr_spi_writel(hw, ALTERA_SPI_CONTROL, hw->imr);
 
 		/* send the first byte */
 		altera_spi_tx_word(hw);
@@ -151,9 +187,13 @@ static int altera_spi_txrx(struct spi_master *master,
 		while (hw->count < hw->len) {
 			altera_spi_tx_word(hw);
 
-			while (!(readl(hw->base + ALTERA_SPI_STATUS) &
-				 ALTERA_SPI_STATUS_RRDY_MSK))
+			for (;;) {
+				altr_spi_readl(hw, ALTERA_SPI_STATUS, &val);
+				if (val & ALTERA_SPI_STATUS_RRDY_MSK)
+					break;
+
 				cpu_relax();
+			}
 
 			altera_spi_rx_word(hw);
 		}
@@ -175,7 +215,7 @@ static irqreturn_t altera_spi_irq(int irq, void *dev)
 	} else {
 		/* disable receive interrupt */
 		hw->imr &= ~ALTERA_SPI_CONTROL_IRRDY_MSK;
-		writel(hw->imr, hw->base + ALTERA_SPI_CONTROL);
+		altr_spi_writel(hw, ALTERA_SPI_CONTROL, hw->imr);
 
 		spi_finalize_current_transfer(master);
 	}
@@ -188,7 +228,9 @@ static int altera_spi_probe(struct platform_device *pdev)
 	struct altera_spi_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	struct altera_spi *hw;
 	struct spi_master *master;
+	void __iomem *res;
 	int err = -ENODEV;
+	u32 val;
 	u16 i;
 
 	master = spi_alloc_master(&pdev->dev, sizeof(struct altera_spi));
@@ -220,19 +262,30 @@ static int altera_spi_probe(struct platform_device *pdev)
 	master->set_cs = altera_spi_set_cs;
 
 	hw = spi_master_get_devdata(master);
+	hw->dev = &pdev->dev;
 
 	/* find and map our resources */
-	hw->base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(hw->base)) {
-		err = PTR_ERR(hw->base);
+	res = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(res)) {
+		err = PTR_ERR(res);
 		goto exit;
 	}
+
+	hw->regmap = devm_regmap_init_mmio(&pdev->dev, res,
+					   &spi_altera_config);
+	if (IS_ERR(hw->regmap)) {
+		dev_err(&pdev->dev, "regmap mmio init failed\n");
+		err = PTR_ERR(hw->regmap);
+		goto exit;
+	}
+
 	/* program defaults into the registers */
 	hw->imr = 0;		/* disable spi interrupts */
-	writel(hw->imr, hw->base + ALTERA_SPI_CONTROL);
-	writel(0, hw->base + ALTERA_SPI_STATUS);	/* clear status reg */
-	if (readl(hw->base + ALTERA_SPI_STATUS) & ALTERA_SPI_STATUS_RRDY_MSK)
-		readl(hw->base + ALTERA_SPI_RXDATA);	/* flush rxdata */
+	altr_spi_writel(hw, ALTERA_SPI_CONTROL, hw->imr);
+	altr_spi_writel(hw, ALTERA_SPI_STATUS, 0);	/* clear status reg */
+	altr_spi_readl(hw, ALTERA_SPI_STATUS, &val);
+	if (val & ALTERA_SPI_STATUS_RRDY_MSK)
+		altr_spi_readl(hw, ALTERA_SPI_RXDATA, &val); /* flush rxdata */
 	/* irq is optional */
 	hw->irq = platform_get_irq(pdev, 0);
 	if (hw->irq >= 0) {
@@ -255,7 +308,7 @@ static int altera_spi_probe(struct platform_device *pdev)
 		}
 	}
 
-	dev_info(&pdev->dev, "base %p, irq %d\n", hw->base, hw->irq);
+	dev_info(&pdev->dev, "base %p, irq %d\n", res, hw->irq);
 
 	return 0;
 exit:
-- 
2.7.4

[PATCH v2 5/6] spi: altera: support indirect access to the registers

[Xu Yilun]

This patch adds support for indirect access to the registers via parent
regmap.

The use case is, the spi master is a sub device of a Multifunction
device, which is connected to host by some indirect bus. To support this
device type, a new platform_device_id is introduced, and the driver tries
to get parent regmap for register accessing like many MFD sub device
drivers do.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Signed-off-by: Russ Weight <russell.h.weight@intel.com>
---
 drivers/spi/spi-altera.c | 62 ++++++++++++++++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 15 deletions(-)

diff --git a/drivers/spi/spi-altera.c b/drivers/spi/spi-altera.c
index b215bdf..4f7717f 100644
--- a/drivers/spi/spi-altera.c
+++ b/drivers/spi/spi-altera.c
@@ -43,6 +43,11 @@
 
 #define ALTERA_SPI_MAX_CS		32
 
+enum altera_spi_type {
+	ALTERA_SPI_TYPE_UNKNOWN,
+	ALTERA_SPI_TYPE_SUBDEV,
+};
+
 struct altera_spi {
 	int irq;
 	int len;
@@ -55,6 +60,7 @@ struct altera_spi {
 	unsigned char *rx;
 
 	struct regmap *regmap;
+	u32 regoff;
 	struct device *dev;
 };
 
@@ -70,7 +76,7 @@ static int altr_spi_writel(struct altera_spi *hw, unsigned int reg,
 {
 	int ret;
 
-	ret = regmap_write(hw->regmap, reg, val);
+	ret = regmap_write(hw->regmap, hw->regoff + reg, val);
 	if (ret)
 		dev_err(hw->dev, "fail to write reg 0x%x val 0x%x: %d\n",
 			reg, val, ret);
@@ -83,7 +89,7 @@ static int altr_spi_readl(struct altera_spi *hw, unsigned int reg,
 {
 	int ret;
 
-	ret = regmap_read(hw->regmap, reg, val);
+	ret = regmap_read(hw->regmap, hw->regoff + reg, val);
 	if (ret)
 		dev_err(hw->dev, "fail to read reg 0x%x: %d\n", reg, ret);
 
@@ -225,10 +231,11 @@ static irqreturn_t altera_spi_irq(int irq, void *dev)
 
 static int altera_spi_probe(struct platform_device *pdev)
 {
+	const struct platform_device_id *platid = platform_get_device_id(pdev);
 	struct altera_spi_platform_data *pdata = dev_get_platdata(&pdev->dev);
+	enum altera_spi_type type = ALTERA_SPI_TYPE_UNKNOWN;
 	struct altera_spi *hw;
 	struct spi_master *master;
-	void __iomem *res;
 	int err = -ENODEV;
 	u32 val;
 	u16 i;
@@ -264,19 +271,38 @@ static int altera_spi_probe(struct platform_device *pdev)
 	hw = spi_master_get_devdata(master);
 	hw->dev = &pdev->dev;
 
+	if (platid)
+		type = platid->driver_data;
+
 	/* find and map our resources */
-	res = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(res)) {
-		err = PTR_ERR(res);
-		goto exit;
-	}
+	if (type == ALTERA_SPI_TYPE_SUBDEV) {
+		struct resource *regoff;
 
-	hw->regmap = devm_regmap_init_mmio(&pdev->dev, res,
-					   &spi_altera_config);
-	if (IS_ERR(hw->regmap)) {
-		dev_err(&pdev->dev, "regmap mmio init failed\n");
-		err = PTR_ERR(hw->regmap);
-		goto exit;
+		hw->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+		if (!hw->regmap) {
+			dev_err(&pdev->dev, "get regmap failed\n");
+			goto exit;
+		}
+
+		regoff = platform_get_resource(pdev, IORESOURCE_REG, 0);
+		if (regoff)
+			hw->regoff = regoff->start;
+	} else {
+		void __iomem *res;
+
+		res = devm_platform_ioremap_resource(pdev, 0);
+		if (IS_ERR(res)) {
+			err = PTR_ERR(res);
+			goto exit;
+		}
+
+		hw->regmap = devm_regmap_init_mmio(&pdev->dev, res,
+						   &spi_altera_config);
+		if (IS_ERR(hw->regmap)) {
+			dev_err(&pdev->dev, "regmap mmio init failed\n");
+			err = PTR_ERR(hw->regmap);
+			goto exit;
+		}
 	}
 
 	/* program defaults into the registers */
@@ -308,7 +334,7 @@ static int altera_spi_probe(struct platform_device *pdev)
 		}
 	}
 
-	dev_info(&pdev->dev, "base %p, irq %d\n", res, hw->irq);
+	dev_info(&pdev->dev, "regoff %u, irq %d\n", hw->regoff, hw->irq);
 
 	return 0;
 exit:
@@ -325,6 +351,11 @@ static const struct of_device_id altera_spi_match[] = {
 MODULE_DEVICE_TABLE(of, altera_spi_match);
 #endif /* CONFIG_OF */
 
+static const struct platform_device_id altera_spi_ids[] = {
+	{ "subdev_spi_altera", ALTERA_SPI_TYPE_SUBDEV },
+	{ }
+};
+
 static struct platform_driver altera_spi_driver = {
 	.probe = altera_spi_probe,
 	.driver = {
@@ -332,6 +363,7 @@ static struct platform_driver altera_spi_driver = {
 		.pm = NULL,
 		.of_match_table = of_match_ptr(altera_spi_match),
 	},
+	.id_table	= altera_spi_ids,
 };
 module_platform_driver(altera_spi_driver);
 
-- 
2.7.4

[PATCH v2 6/6] spi: altera: fix size mismatch on 64 bit processors

[Xu Yilun]

From: Matthew Gerlach <matthew.gerlach@linux.intel.com>

The spi-altera driver was originally written with a 32
bit processor, where sizeof(unsigned long) is 4.  On a
64 bit processor sizeof(unsigned long) is 8.  Change the structure
member to u32 to match the actual size of the control
register.

Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
---
 drivers/spi/spi-altera.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-altera.c b/drivers/spi/spi-altera.c
index 4f7717f..d91c093 100644
--- a/drivers/spi/spi-altera.c
+++ b/drivers/spi/spi-altera.c
@@ -53,7 +53,7 @@ struct altera_spi {
 	int len;
 	int count;
 	int bytes_per_word;
-	unsigned long imr;
+	u32 imr;
 
 	/* data buffers */
 	const unsigned char *tx;
-- 
2.7.4

Re: [PATCH v2 0/6] add regmap & indirect access support

[Mark Brown]

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

On Thu, Jun 18, 2020 at 11:25:07PM +0800, Xu Yilun wrote:
> Updated the regmap & indirect access support for spi-altera.
> 
> Patch #1, #2, #3 is already applied.

The numbering for patches within a series is there *only* to say what
order things need to go in in that posting, they don't have any
relevance from one posting to another.  Sending a series with some
numbers missing causes a lot of confusion since it's not clear which
patches even exist.

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

Re: [PATCH v2 0/6] add regmap & indirect access support

[Tom Rix]

This set looks good to me.

Reviewed-by : Tom Rix <trix@redhat.com>

Tom

On 6/18/20 8:25 AM, Xu Yilun wrote:
> Updated the regmap & indirect access support for spi-altera.
>
> Patch #1, #2, #3 is already applied.
> Patch #4 is an 1:1 replacement of of readl/writel with regmap_read/write
> Patch #5 introduced a new platform_device_id to support indirect access as
>          a sub device.
> Patch #6 is a minor fix.
>
> Main changes from v1: 
>  - Split the v1 Patch #4 to v2 Patch #4 & #5. 
>  - Add a new platform_device_id to support indirect access.
>  - Removed the v1 Patch #5. Now we use driver name string directly.
>  - Add Yilun's Signed-off-by for Patch #6
>
>
> Matthew Gerlach (1):
>   spi: altera: fix size mismatch on 64 bit processors
>
> Xu Yilun (5):
>   spi: altera: add 32bit data width transfer support.
>   spi: altera: add SPI core parameters support via platform data.
>   spi: altera: add platform data for slave information.
>   spi: altera: use regmap-mmio instead of direct mmio register access
>   spi: altera: support indirect access to the registers
>
>  drivers/spi/Kconfig        |   1 +
>  drivers/spi/spi-altera.c   | 177 +++++++++++++++++++++++++++++++++++++++------
>  include/linux/spi/altera.h |  29 ++++++++
>  3 files changed, 183 insertions(+), 24 deletions(-)
>  create mode 100644 include/linux/spi/altera.h
>

Re: [PATCH v2 0/6] add regmap & indirect access support

[Xu Yilun]

On Thu, Jun 18, 2020 at 04:35:39PM +0100, Mark Brown wrote:
> On Thu, Jun 18, 2020 at 11:25:07PM +0800, Xu Yilun wrote:
> > Updated the regmap & indirect access support for spi-altera.
> > 
> > Patch #1, #2, #3 is already applied.
> 
> The numbering for patches within a series is there *only* to say what
> order things need to go in in that posting, they don't have any
> relevance from one posting to another.  Sending a series with some
> numbers missing causes a lot of confusion since it's not clear which
> patches even exist.

Thanks for your guidance. I'll take care of it.

Think I should re-send this patch to make it clear.