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@...

+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

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

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

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