[PATCH] drivers: pcie_xilinx: Fix "reg" not found error

[PATCH] drivers: pcie_xilinx: Fix "reg" not found error

[Mayuresh Chitale]

Fix the driver to use the dev_read_addr_size API to fetch the reg
property from the DT.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
 drivers/pci/pcie_xilinx.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/pcie_xilinx.c b/drivers/pci/pcie_xilinx.c
index 53fd121e90..20b630027f 100644
--- a/drivers/pci/pcie_xilinx.c
+++ b/drivers/pci/pcie_xilinx.c
@@ -140,20 +140,14 @@ static int pcie_xilinx_write_config(struct udevice *bus, pci_dev_t bdf,
 static int pcie_xilinx_of_to_plat(struct udevice *dev)
 {
 	struct xilinx_pcie *pcie = dev_get_priv(dev);
-	struct fdt_resource reg_res;
-	DECLARE_GLOBAL_DATA_PTR;
-	int err;
-
-	err = fdt_get_resource(gd->fdt_blob, dev_of_offset(dev), "reg",
-			       0, &reg_res);
-	if (err < 0) {
-		pr_err("\"reg\" resource not found\n");
-		return err;
-	}
-
-	pcie->cfg_base = map_physmem(reg_res.start,
-				     fdt_resource_size(&reg_res),
-				     MAP_NOCACHE);
+	fdt_addr_t addr;
+	fdt_size_t size;
+
+	addr = dev_read_addr_size(dev, &size);
+	if (addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	pcie->cfg_base = map_physmem(addr, size, MAP_NOCACHE);
 
 	return 0;
 }
-- 
2.34.1

[PATCH] drivers: xilinx_spi: Fixes for MMC_SPI

[Mayuresh Chitale]

Add the xfer callback which is used by the MMC_SPI driver and generally by
the dm_spi_xfer callback. Also probe the fifo_depth during init as is
done in the linux spi-xilinx driver and fix a compiler warning while at it.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
 drivers/spi/xilinx_spi.c | 70 ++++++++++++++++++++++++++++++----------
 1 file changed, 53 insertions(+), 17 deletions(-)

diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
index b58a3f632a..0d4ff25600 100644
--- a/drivers/spi/xilinx_spi.c
+++ b/drivers/spi/xilinx_spi.c
@@ -67,7 +67,7 @@
 /* SPI Slave Select Register (spissr), [1] p13, [2] p13 */
 #define SPISSR_MASK(cs)		(1 << (cs))
 #define SPISSR_ACT(cs)		~SPISSR_MASK(cs)
-#define SPISSR_OFF		~0UL
+#define SPISSR_OFF		(~0U)
 
 /* SPI Software Reset Register (ssr) */
 #define SPISSR_RESET_VALUE	0x0a
@@ -109,6 +109,27 @@ struct xilinx_spi_priv {
 	u8 startup;
 };
 
+/* Detect fifo depth. Ported from linux sha: 4c9a761402d78 */
+static int xilinx_spi_find_buffer_size(struct xilinx_spi_regs *regs)
+{
+	u8 sr;
+	int n_words = 0;
+
+	/*
+	 * Before the buffer_size detection we reset the core
+	 * to make sure we start with a clean state.
+	 */
+	writel(SPISSR_RESET_VALUE, &regs->srr);
+
+	/* Fill the Tx FIFO with as many words as possible */
+	do {
+		writel(0, &regs->spidtr);
+		sr = readl(&regs->spisr);
+		n_words++;
+	} while (!(sr & SPISR_TX_FULL));
+
+	return n_words;
+}
 static int xilinx_spi_probe(struct udevice *bus)
 {
 	struct xilinx_spi_priv *priv = dev_get_priv(bus);
@@ -116,6 +137,8 @@ static int xilinx_spi_probe(struct udevice *bus)
 
 	regs = priv->regs = dev_read_addr_ptr(bus);
 	priv->fifo_depth = dev_read_u32_default(bus, "fifo-size", 0);
+	if (!priv->fifo_depth)
+		priv->fifo_depth = xilinx_spi_find_buffer_size(regs);
 
 	writel(SPISSR_RESET_VALUE, &regs->srr);
 
@@ -197,7 +220,7 @@ static u32 xilinx_spi_fill_txfifo(struct udevice *bus, const u8 *txp,
 	return i;
 }
 
-static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp, u32 rxbytes)
+static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp, int rxbytes)
 {
 	struct xilinx_spi_priv *priv = dev_get_priv(bus);
 	struct xilinx_spi_regs *regs = priv->regs;
@@ -217,9 +240,9 @@ static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp, u32 rxbytes)
 	return i;
 }
 
-static int start_transfer(struct spi_slave *spi, const void *dout, void *din, u32 len)
+static int start_transfer(struct udevice *dev, const void *dout, void *din, u32 len)
 {
-	struct udevice *bus = spi->dev->parent;
+	struct udevice *bus = dev->parent;
 	struct xilinx_spi_priv *priv = dev_get_priv(bus);
 	struct xilinx_spi_regs *regs = priv->regs;
 	u32 count, txbytes, rxbytes;
@@ -259,10 +282,9 @@ static int start_transfer(struct spi_slave *spi, const void *dout, void *din, u3
 	return 0;
 }
 
-static void xilinx_spi_startup_block(struct spi_slave *spi)
+static void xilinx_spi_startup_block(struct udevice *dev)
 {
-	struct dm_spi_slave_plat *slave_plat =
-				dev_get_parent_plat(spi->dev);
+	struct dm_spi_slave_plat *slave_plat = dev_get_parent_plat(dev);
 	unsigned char txp;
 	unsigned char rxp[8];
 
@@ -270,13 +292,25 @@ static void xilinx_spi_startup_block(struct spi_slave *spi)
 	 * Perform a dummy read as a work around for
 	 * the startup block issue.
 	 */
-	spi_cs_activate(spi->dev, slave_plat->cs);
+	spi_cs_activate(dev, slave_plat->cs);
 	txp = 0x9f;
-	start_transfer(spi, (void *)&txp, NULL, 1);
+	start_transfer(dev, (void *)&txp, NULL, 1);
 
-	start_transfer(spi, NULL, (void *)rxp, 6);
+	start_transfer(dev, NULL, (void *)rxp, 6);
 
-	spi_cs_deactivate(spi->dev);
+	spi_cs_deactivate(dev);
+}
+
+static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen,
+			   const void *dout, void *din, unsigned long flags)
+{
+	struct dm_spi_slave_plat *slave_plat = dev_get_parent_plat(dev);
+	int ret;
+
+	spi_cs_activate(dev, slave_plat->cs);
+	ret = start_transfer(dev, dout, din, (bitlen / 8));
+	spi_cs_deactivate(dev);
+	return ret;
 }
 
 static int xilinx_spi_mem_exec_op(struct spi_slave *spi,
@@ -294,14 +328,15 @@ static int xilinx_spi_mem_exec_op(struct spi_slave *spi,
 	 * as QSPI provides command. So first command fails.
 	 */
 	if (!startup) {
-		xilinx_spi_startup_block(spi);
+		xilinx_spi_startup_block(spi->dev);
 		startup++;
 	}
 
 	spi_cs_activate(spi->dev, slave_plat->cs);
 
 	if (op->cmd.opcode) {
-		ret = start_transfer(spi, (void *)&op->cmd.opcode, NULL, 1);
+		ret = start_transfer(spi->dev, (void *)&op->cmd.opcode,
+				     NULL, 1);
 		if (ret)
 			goto done;
 	}
@@ -313,7 +348,7 @@ static int xilinx_spi_mem_exec_op(struct spi_slave *spi,
 			addr_buf[i] = op->addr.val >>
 			(8 * (op->addr.nbytes - i - 1));
 
-		ret = start_transfer(spi, (void *)addr_buf, NULL,
+		ret = start_transfer(spi->dev, (void *)addr_buf, NULL,
 				     op->addr.nbytes);
 		if (ret)
 			goto done;
@@ -322,16 +357,16 @@ static int xilinx_spi_mem_exec_op(struct spi_slave *spi,
 		dummy_len = (op->dummy.nbytes * op->data.buswidth) /
 			     op->dummy.buswidth;
 
-		ret = start_transfer(spi, NULL, NULL, dummy_len);
+		ret = start_transfer(spi->dev, NULL, NULL, dummy_len);
 		if (ret)
 			goto done;
 	}
 	if (op->data.nbytes) {
 		if (op->data.dir == SPI_MEM_DATA_IN) {
-			ret = start_transfer(spi, NULL,
+			ret = start_transfer(spi->dev, NULL,
 					     op->data.buf.in, op->data.nbytes);
 		} else {
-			ret = start_transfer(spi, op->data.buf.out,
+			ret = start_transfer(spi->dev, op->data.buf.out,
 					     NULL, op->data.nbytes);
 		}
 		if (ret)
@@ -427,6 +462,7 @@ static const struct spi_controller_mem_ops xilinx_spi_mem_ops = {
 static const struct dm_spi_ops xilinx_spi_ops = {
 	.claim_bus	= xilinx_spi_claim_bus,
 	.release_bus	= xilinx_spi_release_bus,
+	.xfer           = xilinx_spi_xfer,
 	.set_speed	= xilinx_spi_set_speed,
 	.set_mode	= xilinx_spi_set_mode,
 	.mem_ops	= &xilinx_spi_mem_ops,
-- 
2.34.1

[PATCH] net: axi_emac: Use reg property for DMA registers

[Mayuresh Chitale]

As per the xlnx,axi-ethernet-1.00.a DT documentation in linux, the AXI
DMA registers can be obtained via the reg property or via a separate
node for the axistream DMA controller. Currently only the latter is
supported, so add support to fetch the DMA controller registers from the
"reg" property.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
 drivers/net/xilinx_axi_emac.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/xilinx_axi_emac.c b/drivers/net/xilinx_axi_emac.c
index 54f2232768..ef151ee51b 100644
--- a/drivers/net/xilinx_axi_emac.c
+++ b/drivers/net/xilinx_axi_emac.c
@@ -903,12 +903,11 @@ static int axi_emac_of_to_plat(struct udevice *dev)
 
 	ret = dev_read_phandle_with_args(dev, "axistream-connected", NULL, 0, 0,
 					 &axistream_node);
-	if (ret) {
-		printf("%s: axistream is not found\n", __func__);
-		return -EINVAL;
-	}
+	if (!ret)
+		plat->dmatx = (struct axidma_reg *)ofnode_get_addr(axistream_node.node);
+	else
+		plat->dmatx = (struct axidma_reg *)dev_read_addr_index(dev, 1);
 
-	plat->dmatx = (struct axidma_reg *)ofnode_get_addr(axistream_node.node);
 	if (!plat->dmatx) {
 		printf("%s: axi_dma register space not found\n", __func__);
 		return -EINVAL;
-- 
2.34.1

[PATCH] pci: xilinx: Enable MMIO region

[Mayuresh Chitale]

The host bridge MMIO region is disabled by default due to which MMIO
accesses cause an exception. Fix it by setting the bridge enable bit.
This change is ported from the linux pcie-xilinx driver.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
 drivers/pci/pcie_xilinx.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pci/pcie_xilinx.c b/drivers/pci/pcie_xilinx.c
index 20b630027f..d1fbd40175 100644
--- a/drivers/pci/pcie_xilinx.c
+++ b/drivers/pci/pcie_xilinx.c
@@ -25,6 +25,8 @@ struct xilinx_pcie {
 /* Register definitions */
 #define XILINX_PCIE_REG_PSCR		0x144
 #define XILINX_PCIE_REG_PSCR_LNKUP	BIT(11)
+#define XILINX_PCIE_REG_RPSC		0x148
+#define XILINX_PCIE_REG_RPSC_BEN	BIT(0)
 
 /**
  * pcie_xilinx_link_up() - Check whether the PCIe link is up
@@ -142,6 +144,7 @@ static int pcie_xilinx_of_to_plat(struct udevice *dev)
 	struct xilinx_pcie *pcie = dev_get_priv(dev);
 	fdt_addr_t addr;
 	fdt_size_t size;
+	u32 rpsc;
 
 	addr = dev_read_addr_size(dev, &size);
 	if (addr == FDT_ADDR_T_NONE)
@@ -149,6 +152,11 @@ static int pcie_xilinx_of_to_plat(struct udevice *dev)
 
 	pcie->cfg_base = map_physmem(addr, size, MAP_NOCACHE);
 
+	/* Enable the Bridge enable bit */
+	rpsc = __raw_readl(pcie->cfg_base + XILINX_PCIE_REG_RPSC);
+	rpsc |= XILINX_PCIE_REG_RPSC_BEN;
+	__raw_writel(rpsc, pcie->cfg_base + XILINX_PCIE_REG_RPSC);
+
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH] drivers: xilinx_spi: Fixes for MMC_SPI

[Michal Simek]

On 11/2/23 09:23, Mayuresh Chitale wrote:
> Add the xfer callback which is used by the MMC_SPI driver and generally by
> the dm_spi_xfer callback. Also probe the fifo_depth during init as is
> done in the linux spi-xilinx driver and fix a compiler warning while at it.

They are independent things that's why please separate them by that topics.

Also update subject to describe issue there.

> 
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> ---
>   drivers/spi/xilinx_spi.c | 70 ++++++++++++++++++++++++++++++----------
>   1 file changed, 53 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
> index b58a3f632a..0d4ff25600 100644
> --- a/drivers/spi/xilinx_spi.c
> +++ b/drivers/spi/xilinx_spi.c
> @@ -67,7 +67,7 @@
>   /* SPI Slave Select Register (spissr), [1] p13, [2] p13 */
>   #define SPISSR_MASK(cs)		(1 << (cs))
>   #define SPISSR_ACT(cs)		~SPISSR_MASK(cs)
> -#define SPISSR_OFF		~0UL
> +#define SPISSR_OFF		(~0U)
>   
>   /* SPI Software Reset Register (ssr) */
>   #define SPISSR_RESET_VALUE	0x0a
> @@ -109,6 +109,27 @@ struct xilinx_spi_priv {
>   	u8 startup;
>   };
>   
> +/* Detect fifo depth. Ported from linux sha: 4c9a761402d78 */

don't add linux sha1 here.
Add it to commit message or I normally just provide a link to that patch.
https://lore.kernel.org/r/1422029330-10971-5-git-send-email-ricardo.ribalda@gmail.com



> +static int xilinx_spi_find_buffer_size(struct xilinx_spi_regs *regs)
> +{
> +	u8 sr;
> +	int n_words = 0;
> +
> +	/*
> +	 * Before the buffer_size detection we reset the core
> +	 * to make sure we start with a clean state.
> +	 */
> +	writel(SPISSR_RESET_VALUE, &regs->srr);
> +
> +	/* Fill the Tx FIFO with as many words as possible */
> +	do {
> +		writel(0, &regs->spidtr);
> +		sr = readl(&regs->spisr);
> +		n_words++;
> +	} while (!(sr & SPISR_TX_FULL));
> +
> +	return n_words;
> +}

newline here.

>   static int xilinx_spi_probe(struct udevice *bus)
>   {
>   	struct xilinx_spi_priv *priv = dev_get_priv(bus);
> @@ -116,6 +137,8 @@ static int xilinx_spi_probe(struct udevice *bus)
>   
>   	regs = priv->regs = dev_read_addr_ptr(bus);
>   	priv->fifo_depth = dev_read_u32_default(bus, "fifo-size", 0);
> +	if (!priv->fifo_depth)
> +		priv->fifo_depth = xilinx_spi_find_buffer_size(regs);
>   
>   	writel(SPISSR_RESET_VALUE, &regs->srr);
>   
> @@ -197,7 +220,7 @@ static u32 xilinx_spi_fill_txfifo(struct udevice *bus, const u8 *txp,
>   	return i;
>   }
>   
> -static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp, u32 rxbytes)
> +static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp, int rxbytes)

Unrelated. Or do we have a case where we ask for reading negative number of bytes?

>   {
>   	struct xilinx_spi_priv *priv = dev_get_priv(bus);
>   	struct xilinx_spi_regs *regs = priv->regs;
> @@ -217,9 +240,9 @@ static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp, u32 rxbytes)
>   	return i;
>   }
>   
> -static int start_transfer(struct spi_slave *spi, const void *dout, void *din, u32 len)
> +static int start_transfer(struct udevice *dev, const void *dout, void *din, u32 len)

All these spi -> dev changes should be in separate patch. I expect they are 
preparation for xfer but still better to do it separately.

>   {
> -	struct udevice *bus = spi->dev->parent;
> +	struct udevice *bus = dev->parent;
>   	struct xilinx_spi_priv *priv = dev_get_priv(bus);
>   	struct xilinx_spi_regs *regs = priv->regs;
>   	u32 count, txbytes, rxbytes;
> @@ -259,10 +282,9 @@ static int start_transfer(struct spi_slave *spi, const void *dout, void *din, u3
>   	return 0;
>   }
>   
> -static void xilinx_spi_startup_block(struct spi_slave *spi)
> +static void xilinx_spi_startup_block(struct udevice *dev)
>   {
> -	struct dm_spi_slave_plat *slave_plat =
> -				dev_get_parent_plat(spi->dev);
> +	struct dm_spi_slave_plat *slave_plat = dev_get_parent_plat(dev);
>   	unsigned char txp;
>   	unsigned char rxp[8];
>   
> @@ -270,13 +292,25 @@ static void xilinx_spi_startup_block(struct spi_slave *spi)
>   	 * Perform a dummy read as a work around for
>   	 * the startup block issue.
>   	 */
> -	spi_cs_activate(spi->dev, slave_plat->cs);
> +	spi_cs_activate(dev, slave_plat->cs);
>   	txp = 0x9f;
> -	start_transfer(spi, (void *)&txp, NULL, 1);
> +	start_transfer(dev, (void *)&txp, NULL, 1);
>   
> -	start_transfer(spi, NULL, (void *)rxp, 6);
> +	start_transfer(dev, NULL, (void *)rxp, 6);
>   
> -	spi_cs_deactivate(spi->dev);
> +	spi_cs_deactivate(dev);
> +}
> +
> +static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen,
> +			   const void *dout, void *din, unsigned long flags)
> +{
> +	struct dm_spi_slave_plat *slave_plat = dev_get_parent_plat(dev);
> +	int ret;
> +
> +	spi_cs_activate(dev, slave_plat->cs);
> +	ret = start_transfer(dev, dout, din, (bitlen / 8));

no need for () around

> +	spi_cs_deactivate(dev);
> +	return ret;
>   }
>   
>   static int xilinx_spi_mem_exec_op(struct spi_slave *spi,
> @@ -294,14 +328,15 @@ static int xilinx_spi_mem_exec_op(struct spi_slave *spi,
>   	 * as QSPI provides command. So first command fails.
>   	 */
>   	if (!startup) {
> -		xilinx_spi_startup_block(spi);
> +		xilinx_spi_startup_block(spi->dev);
>   		startup++;
>   	}
>   
>   	spi_cs_activate(spi->dev, slave_plat->cs);
>   
>   	if (op->cmd.opcode) {
> -		ret = start_transfer(spi, (void *)&op->cmd.opcode, NULL, 1);
> +		ret = start_transfer(spi->dev, (void *)&op->cmd.opcode,
> +				     NULL, 1);
>   		if (ret)
>   			goto done;
>   	}
> @@ -313,7 +348,7 @@ static int xilinx_spi_mem_exec_op(struct spi_slave *spi,
>   			addr_buf[i] = op->addr.val >>
>   			(8 * (op->addr.nbytes - i - 1));
>   
> -		ret = start_transfer(spi, (void *)addr_buf, NULL,
> +		ret = start_transfer(spi->dev, (void *)addr_buf, NULL,
>   				     op->addr.nbytes);
>   		if (ret)
>   			goto done;
> @@ -322,16 +357,16 @@ static int xilinx_spi_mem_exec_op(struct spi_slave *spi,
>   		dummy_len = (op->dummy.nbytes * op->data.buswidth) /
>   			     op->dummy.buswidth;
>   
> -		ret = start_transfer(spi, NULL, NULL, dummy_len);
> +		ret = start_transfer(spi->dev, NULL, NULL, dummy_len);
>   		if (ret)
>   			goto done;
>   	}
>   	if (op->data.nbytes) {
>   		if (op->data.dir == SPI_MEM_DATA_IN) {
> -			ret = start_transfer(spi, NULL,
> +			ret = start_transfer(spi->dev, NULL,
>   					     op->data.buf.in, op->data.nbytes);
>   		} else {
> -			ret = start_transfer(spi, op->data.buf.out,
> +			ret = start_transfer(spi->dev, op->data.buf.out,
>   					     NULL, op->data.nbytes);
>   		}
>   		if (ret)
> @@ -427,6 +462,7 @@ static const struct spi_controller_mem_ops xilinx_spi_mem_ops = {
>   static const struct dm_spi_ops xilinx_spi_ops = {
>   	.claim_bus	= xilinx_spi_claim_bus,
>   	.release_bus	= xilinx_spi_release_bus,
> +	.xfer           = xilinx_spi_xfer,
>   	.set_speed	= xilinx_spi_set_speed,
>   	.set_mode	= xilinx_spi_set_mode,
>   	.mem_ops	= &xilinx_spi_mem_ops,

M

Re: [PATCH] drivers: pcie_xilinx: Fix "reg" not found error

[Michal Simek]

On 11/2/23 09:23, Mayuresh Chitale wrote:
> Fix the driver to use the dev_read_addr_size API to fetch the reg
> property from the DT.
> 
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> ---
>   drivers/pci/pcie_xilinx.c | 22 ++++++++--------------
>   1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/pci/pcie_xilinx.c b/drivers/pci/pcie_xilinx.c
> index 53fd121e90..20b630027f 100644
> --- a/drivers/pci/pcie_xilinx.c
> +++ b/drivers/pci/pcie_xilinx.c
> @@ -140,20 +140,14 @@ static int pcie_xilinx_write_config(struct udevice *bus, pci_dev_t bdf,
>   static int pcie_xilinx_of_to_plat(struct udevice *dev)
>   {
>   	struct xilinx_pcie *pcie = dev_get_priv(dev);
> -	struct fdt_resource reg_res;
> -	DECLARE_GLOBAL_DATA_PTR;
> -	int err;
> -
> -	err = fdt_get_resource(gd->fdt_blob, dev_of_offset(dev), "reg",

I don't have HW to test this but here you are removing reference to GD that's 
why I think you should also remove

#include <asm/global_data.h>




> -			       0, &reg_res);
> -	if (err < 0) {
> -		pr_err("\"reg\" resource not found\n");
> -		return err;
> -	}
> -
> -	pcie->cfg_base = map_physmem(reg_res.start,
> -				     fdt_resource_size(&reg_res),
> -				     MAP_NOCACHE);
> +	fdt_addr_t addr;
> +	fdt_size_t size;
> +
> +	addr = dev_read_addr_size(dev, &size);
> +	if (addr == FDT_ADDR_T_NONE)
> +		return -EINVAL;
> +
> +	pcie->cfg_base = map_physmem(addr, size, MAP_NOCACHE);

Definitely it is better then what was there before.

But can't use use devm_ioremap() instead?

Thanks,
Michal

Re: [PATCH] pci: xilinx: Enable MMIO region

[Michal Simek]

On 11/2/23 09:23, Mayuresh Chitale wrote:
> 
> The host bridge MMIO region is disabled by default due to which MMIO
> accesses cause an exception. Fix it by setting the bridge enable bit.
> This change is ported from the linux pcie-xilinx driver.
> 
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> ---
>   drivers/pci/pcie_xilinx.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/pci/pcie_xilinx.c b/drivers/pci/pcie_xilinx.c
> index 20b630027f..d1fbd40175 100644
> --- a/drivers/pci/pcie_xilinx.c
> +++ b/drivers/pci/pcie_xilinx.c
> @@ -25,6 +25,8 @@ struct xilinx_pcie {
>   /* Register definitions */
>   #define XILINX_PCIE_REG_PSCR		0x144
>   #define XILINX_PCIE_REG_PSCR_LNKUP	BIT(11)
> +#define XILINX_PCIE_REG_RPSC		0x148
> +#define XILINX_PCIE_REG_RPSC_BEN	BIT(0)
>   
>   /**
>    * pcie_xilinx_link_up() - Check whether the PCIe link is up
> @@ -142,6 +144,7 @@ static int pcie_xilinx_of_to_plat(struct udevice *dev)
>   	struct xilinx_pcie *pcie = dev_get_priv(dev);
>   	fdt_addr_t addr;
>   	fdt_size_t size;
> +	u32 rpsc;
>   
>   	addr = dev_read_addr_size(dev, &size);
>   	if (addr == FDT_ADDR_T_NONE)
> @@ -149,6 +152,11 @@ static int pcie_xilinx_of_to_plat(struct udevice *dev)
>   
>   	pcie->cfg_base = map_physmem(addr, size, MAP_NOCACHE);
>   
> +	/* Enable the Bridge enable bit */
> +	rpsc = __raw_readl(pcie->cfg_base + XILINX_PCIE_REG_RPSC);
> +	rpsc |= XILINX_PCIE_REG_RPSC_BEN;
> +	__raw_writel(rpsc, pcie->cfg_base + XILINX_PCIE_REG_RPSC);
> +
>   	return 0;
>   }
>   

This looks good but depends on previous patch in this series that's why
Reviewed-by: Michal Simek <michal.simek@amd.com>

Thanks,
Michal

Re: [PATCH] net: axi_emac: Use reg property for DMA registers

[Michal Simek]

On 11/2/23 09:23, Mayuresh Chitale wrote:
> As per the xlnx,axi-ethernet-1.00.a DT documentation in linux, the AXI
> DMA registers can be obtained via the reg property or via a separate
> node for the axistream DMA controller. Currently only the latter is
> supported, so add support to fetch the DMA controller registers from the
> "reg" property.
> 
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> ---
>   drivers/net/xilinx_axi_emac.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/xilinx_axi_emac.c b/drivers/net/xilinx_axi_emac.c
> index 54f2232768..ef151ee51b 100644
> --- a/drivers/net/xilinx_axi_emac.c
> +++ b/drivers/net/xilinx_axi_emac.c
> @@ -903,12 +903,11 @@ static int axi_emac_of_to_plat(struct udevice *dev)
>   
>   	ret = dev_read_phandle_with_args(dev, "axistream-connected", NULL, 0, 0,
>   					 &axistream_node);
> -	if (ret) {
> -		printf("%s: axistream is not found\n", __func__);
> -		return -EINVAL;
> -	}
> +	if (!ret)
> +		plat->dmatx = (struct axidma_reg *)ofnode_get_addr(axistream_node.node);
> +	else
> +		plat->dmatx = (struct axidma_reg *)dev_read_addr_index(dev, 1);
>   
> -	plat->dmatx = (struct axidma_reg *)ofnode_get_addr(axistream_node.node);
>   	if (!plat->dmatx) {
>   		printf("%s: axi_dma register space not found\n", __func__);
>   		return -EINVAL;

This looks good to me. Can you please send it separately?
It is really independent of change from other patches.

Reviewed-by: Michal Simek <michal.simek@amd.com>

Thanks,
Michal

Re: [PATCH] drivers: xilinx_spi: Fixes for MMC_SPI

[mchitale]

Hi Michal,

On Thu, 2023-11-02 at 09:37 +0100, Michal Simek wrote:
> On 11/2/23 09:23, Mayuresh Chitale wrote:
> > Add the xfer callback which is used by the MMC_SPI driver and
> > generally by
> > the dm_spi_xfer callback. Also probe the fifo_depth during init as
> > is
> > done in the linux spi-xilinx driver and fix a compiler warning
> > while at it.
> 
> They are independent things that's why please separate them by that
> topics.
> 
> Also update subject to describe issue there.
Ok.
> 
> > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> > ---
> >   drivers/spi/xilinx_spi.c | 70 ++++++++++++++++++++++++++++++-----
> > -----
> >   1 file changed, 53 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
> > index b58a3f632a..0d4ff25600 100644
> > --- a/drivers/spi/xilinx_spi.c
> > +++ b/drivers/spi/xilinx_spi.c
> > @@ -67,7 +67,7 @@
> >   /* SPI Slave Select Register (spissr), [1] p13, [2] p13 */
> >   #define SPISSR_MASK(cs)		(1 << (cs))
> >   #define SPISSR_ACT(cs)		~SPISSR_MASK(cs)
> > -#define SPISSR_OFF		~0UL
> > +#define SPISSR_OFF		(~0U)
> >   
> >   /* SPI Software Reset Register (ssr) */
> >   #define SPISSR_RESET_VALUE	0x0a
> > @@ -109,6 +109,27 @@ struct xilinx_spi_priv {
> >   	u8 startup;
> >   };
> >   
> > +/* Detect fifo depth. Ported from linux sha: 4c9a761402d78 */
> 
> don't add linux sha1 here.
> Add it to commit message or I normally just provide a link to that
> patch.
> https://lore.kernel.org/r/1422029330-10971-5-git-send-email-ricardo.ribalda@gmail.com
Ok.
> 
> 
> 
> > +static int xilinx_spi_find_buffer_size(struct xilinx_spi_regs
> > *regs)
> > +{
> > +	u8 sr;
> > +	int n_words = 0;
> > +
> > +	/*
> > +	 * Before the buffer_size detection we reset the core
> > +	 * to make sure we start with a clean state.
> > +	 */
> > +	writel(SPISSR_RESET_VALUE, &regs->srr);
> > +
> > +	/* Fill the Tx FIFO with as many words as possible */
> > +	do {
> > +		writel(0, &regs->spidtr);
> > +		sr = readl(&regs->spisr);
> > +		n_words++;
> > +	} while (!(sr & SPISR_TX_FULL));
> > +
> > +	return n_words;
> > +}
> 
> newline here.
Ok.
> 
> >   static int xilinx_spi_probe(struct udevice *bus)
> >   {
> >   	struct xilinx_spi_priv *priv = dev_get_priv(bus);
> > @@ -116,6 +137,8 @@ static int xilinx_spi_probe(struct udevice
> > *bus)
> >   
> >   	regs = priv->regs = dev_read_addr_ptr(bus);
> >   	priv->fifo_depth = dev_read_u32_default(bus, "fifo-size", 0);
> > +	if (!priv->fifo_depth)
> > +		priv->fifo_depth = xilinx_spi_find_buffer_size(regs);
> >   
> >   	writel(SPISSR_RESET_VALUE, &regs->srr);
> >   
> > @@ -197,7 +220,7 @@ static u32 xilinx_spi_fill_txfifo(struct
> > udevice *bus, const u8 *txp,
> >   	return i;
> >   }
> >   
> > -static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp,
> > u32 rxbytes)
> > +static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp,
> > int rxbytes)
> 
> Unrelated. Or do we have a case where we ask for reading negative
> number of bytes?
I will revert the change.
> 
> >   {
> >   	struct xilinx_spi_priv *priv = dev_get_priv(bus);
> >   	struct xilinx_spi_regs *regs = priv->regs;
> > @@ -217,9 +240,9 @@ static u32 xilinx_spi_read_rxfifo(struct
> > udevice *bus, u8 *rxp, u32 rxbytes)
> >   	return i;
> >   }
> >   
> > -static int start_transfer(struct spi_slave *spi, const void *dout,
> > void *din, u32 len)
> > +static int start_transfer(struct udevice *dev, const void *dout,
> > void *din, u32 len)
> 
> All these spi -> dev changes should be in separate patch. I expect
> they are 
> preparation for xfer but still better to do it separately.
Ok.
> 
> >   {
> > -	struct udevice *bus = spi->dev->parent;
> > +	struct udevice *bus = dev->parent;
> >   	struct xilinx_spi_priv *priv = dev_get_priv(bus);
> >   	struct xilinx_spi_regs *regs = priv->regs;
> >   	u32 count, txbytes, rxbytes;
> > @@ -259,10 +282,9 @@ static int start_transfer(struct spi_slave
> > *spi, const void *dout, void *din, u3
> >   	return 0;
> >   }
> >   
> > -static void xilinx_spi_startup_block(struct spi_slave *spi)
> > +static void xilinx_spi_startup_block(struct udevice *dev)
> >   {
> > -	struct dm_spi_slave_plat *slave_plat =
> > -				dev_get_parent_plat(spi->dev);
> > +	struct dm_spi_slave_plat *slave_plat =
> > dev_get_parent_plat(dev);
> >   	unsigned char txp;
> >   	unsigned char rxp[8];
> >   
> > @@ -270,13 +292,25 @@ static void xilinx_spi_startup_block(struct
> > spi_slave *spi)
> >   	 * Perform a dummy read as a work around for
> >   	 * the startup block issue.
> >   	 */
> > -	spi_cs_activate(spi->dev, slave_plat->cs);
> > +	spi_cs_activate(dev, slave_plat->cs);
> >   	txp = 0x9f;
> > -	start_transfer(spi, (void *)&txp, NULL, 1);
> > +	start_transfer(dev, (void *)&txp, NULL, 1);
> >   
> > -	start_transfer(spi, NULL, (void *)rxp, 6);
> > +	start_transfer(dev, NULL, (void *)rxp, 6);
> >   
> > -	spi_cs_deactivate(spi->dev);
> > +	spi_cs_deactivate(dev);
> > +}
> > +
> > +static int xilinx_spi_xfer(struct udevice *dev, unsigned int
> > bitlen,
> > +			   const void *dout, void *din, unsigned long
> > flags)
> > +{
> > +	struct dm_spi_slave_plat *slave_plat =
> > dev_get_parent_plat(dev);
> > +	int ret;
> > +
> > +	spi_cs_activate(dev, slave_plat->cs);
> > +	ret = start_transfer(dev, dout, din, (bitlen / 8));
> 
> no need for () around
Ok.
> 
> > +	spi_cs_deactivate(dev);
> > +	return ret;
> >   }
> >   
> >   static int xilinx_spi_mem_exec_op(struct spi_slave *spi,
> > @@ -294,14 +328,15 @@ static int xilinx_spi_mem_exec_op(struct
> > spi_slave *spi,
> >   	 * as QSPI provides command. So first command fails.
> >   	 */
> >   	if (!startup) {
> > -		xilinx_spi_startup_block(spi);
> > +		xilinx_spi_startup_block(spi->dev);
> >   		startup++;
> >   	}
> >   
> >   	spi_cs_activate(spi->dev, slave_plat->cs);
> >   
> >   	if (op->cmd.opcode) {
> > -		ret = start_transfer(spi, (void *)&op->cmd.opcode,
> > NULL, 1);
> > +		ret = start_transfer(spi->dev, (void *)&op->cmd.opcode,
> > +				     NULL, 1);
> >   		if (ret)
> >   			goto done;
> >   	}
> > @@ -313,7 +348,7 @@ static int xilinx_spi_mem_exec_op(struct
> > spi_slave *spi,
> >   			addr_buf[i] = op->addr.val >>
> >   			(8 * (op->addr.nbytes - i - 1));
> >   
> > -		ret = start_transfer(spi, (void *)addr_buf, NULL,
> > +		ret = start_transfer(spi->dev, (void *)addr_buf, NULL,
> >   				     op->addr.nbytes);
> >   		if (ret)
> >   			goto done;
> > @@ -322,16 +357,16 @@ static int xilinx_spi_mem_exec_op(struct
> > spi_slave *spi,
> >   		dummy_len = (op->dummy.nbytes * op->data.buswidth) /
> >   			     op->dummy.buswidth;
> >   
> > -		ret = start_transfer(spi, NULL, NULL, dummy_len);
> > +		ret = start_transfer(spi->dev, NULL, NULL, dummy_len);
> >   		if (ret)
> >   			goto done;
> >   	}
> >   	if (op->data.nbytes) {
> >   		if (op->data.dir == SPI_MEM_DATA_IN) {
> > -			ret = start_transfer(spi, NULL,
> > +			ret = start_transfer(spi->dev, NULL,
> >   					     op->data.buf.in, op-
> > >data.nbytes);
> >   		} else {
> > -			ret = start_transfer(spi, op->data.buf.out,
> > +			ret = start_transfer(spi->dev, op-
> > >data.buf.out,
> >   					     NULL, op->data.nbytes);
> >   		}
> >   		if (ret)
> > @@ -427,6 +462,7 @@ static const struct spi_controller_mem_ops
> > xilinx_spi_mem_ops = {
> >   static const struct dm_spi_ops xilinx_spi_ops = {
> >   	.claim_bus	= xilinx_spi_claim_bus,
> >   	.release_bus	= xilinx_spi_release_bus,
> > +	.xfer           = xilinx_spi_xfer,
> >   	.set_speed	= xilinx_spi_set_speed,
> >   	.set_mode	= xilinx_spi_set_mode,
> >   	.mem_ops	= &xilinx_spi_mem_ops,
> 
> M

Re: [PATCH] net: axi_emac: Use reg property for DMA registers

[mchitale]

Hi Michal,

On Thu, 2023-11-02 at 10:15 +0100, Michal Simek wrote:
> 
> On 11/2/23 09:23, Mayuresh Chitale wrote:
> > As per the xlnx,axi-ethernet-1.00.a DT documentation in linux, the
> > AXI
> > DMA registers can be obtained via the reg property or via a
> > separate
> > node for the axistream DMA controller. Currently only the latter is
> > supported, so add support to fetch the DMA controller registers
> > from the
> > "reg" property.
> > 
> > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> > ---
> >   drivers/net/xilinx_axi_emac.c | 9 ++++-----
> >   1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/net/xilinx_axi_emac.c
> > b/drivers/net/xilinx_axi_emac.c
> > index 54f2232768..ef151ee51b 100644
> > --- a/drivers/net/xilinx_axi_emac.c
> > +++ b/drivers/net/xilinx_axi_emac.c
> > @@ -903,12 +903,11 @@ static int axi_emac_of_to_plat(struct udevice
> > *dev)
> >   
> >   	ret = dev_read_phandle_with_args(dev, "axistream-connected",
> > NULL, 0, 0,
> >   					 &axistream_node);
> > -	if (ret) {
> > -		printf("%s: axistream is not found\n", __func__);
> > -		return -EINVAL;
> > -	}
> > +	if (!ret)
> > +		plat->dmatx = (struct axidma_reg
> > *)ofnode_get_addr(axistream_node.node);
> > +	else
> > +		plat->dmatx = (struct axidma_reg
> > *)dev_read_addr_index(dev, 1);
> >   
> > -	plat->dmatx = (struct axidma_reg
> > *)ofnode_get_addr(axistream_node.node);
> >   	if (!plat->dmatx) {
> >   		printf("%s: axi_dma register space not found\n",
> > __func__);
> >   		return -EINVAL;
> 
> This looks good to me. Can you please send it separately?
> It is really independent of change from other patches.
Actually it is a separate patch.
> 
> Reviewed-by: Michal Simek <michal.simek@amd.com>
> 
> Thanks,
> Michal

Re: [PATCH] drivers: pcie_xilinx: Fix "reg" not found error

[mchitale]

Hi Michal,

On Thu, 2023-11-02 at 10:05 +0100, Michal Simek wrote:
> 
> On 11/2/23 09:23, Mayuresh Chitale wrote:
> > Fix the driver to use the dev_read_addr_size API to fetch the reg
> > property from the DT.
> > 
> > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> > ---
> >   drivers/pci/pcie_xilinx.c | 22 ++++++++--------------
> >   1 file changed, 8 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/pci/pcie_xilinx.c b/drivers/pci/pcie_xilinx.c
> > index 53fd121e90..20b630027f 100644
> > --- a/drivers/pci/pcie_xilinx.c
> > +++ b/drivers/pci/pcie_xilinx.c
> > @@ -140,20 +140,14 @@ static int pcie_xilinx_write_config(struct
> > udevice *bus, pci_dev_t bdf,
> >   static int pcie_xilinx_of_to_plat(struct udevice *dev)
> >   {
> >   	struct xilinx_pcie *pcie = dev_get_priv(dev);
> > -	struct fdt_resource reg_res;
> > -	DECLARE_GLOBAL_DATA_PTR;
> > -	int err;
> > -
> > -	err = fdt_get_resource(gd->fdt_blob, dev_of_offset(dev), "reg",
> 
> I don't have HW to test this but here you are removing reference to
> GD that's 
> why I think you should also remove
> 
> #include <asm/global_data.h>
Ok.
> 
> 
> 
> 
> > -			       0, &reg_res);
> > -	if (err < 0) {
> > -		pr_err("\"reg\" resource not found\n");
> > -		return err;
> > -	}
> > -
> > -	pcie->cfg_base = map_physmem(reg_res.start,
> > -				     fdt_resource_size(&reg_res),
> > -				     MAP_NOCACHE);
> > +	fdt_addr_t addr;
> > +	fdt_size_t size;
> > +
> > +	addr = dev_read_addr_size(dev, &size);
> > +	if (addr == FDT_ADDR_T_NONE)
> > +		return -EINVAL;
> > +
> > +	pcie->cfg_base = map_physmem(addr, size, MAP_NOCACHE);
> 
> Definitely it is better then what was there before.
> 
> But can't use use devm_ioremap() instead?
Ok.
> 
> Thanks,
> Michal