Re: [PATCH v6 2/2] spi: cadence-quadpsi: Add support for the Cadence QSPI controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 8/1/2020 1:14 PM, Vignesh Raghavendra wrote:

On 06/01/20 4:49 pm, Ramuthevar, Vadivel MuruganX wrote:
Hi,

Thank you for the review comments.

On 6/1/2020 6:40 PM, Vignesh Raghavendra wrote:
Hi,

On 30/12/19 1:11 pm, Ramuthevar,Vadivel MuruganX wrote:
[...]
+static u32 cqspi_cmd2addr(const unsigned char *addr_buf, u32
addr_width)
+{
+    unsigned int addr = 0;
+    int i;
+
+    /* Invalid address return zero. */
+    if (addr_width > 4)
+        return 0;
+
+    for (i = 0; i < addr_width; i++) {
+        addr = addr << 8;
+        addr |= addr_buf[i];
+    }
+
+    return addr;
+}
+
[...]
+static int cqspi_apb_read_setup(struct struct_cqspi *cqspi,
+                const struct spi_mem_op *op,
+                const u8 *addrbuf)
+{
+    void __iomem *reg_base = cqspi->iobase;
+    size_t addrlen = op->addr.nbytes;
+    size_t dummy_bytes = op->dummy.nbytes;
+    unsigned int addr_value, dummy_clk, reg;
+
+    if (addrlen) {
+        addr_value = cqspi_cmd2addr(&addrbuf[0], addrlen);
+        writel(addr_value, reg_base + CQSPI_REG_INDIRECTRDSTARTADDR);
+    }
+
Why do you need to swap the address bytes to SPI bus order?
Yes , you are right to align with spi bus order swap is done .
   You are
writing to a controller register that accepts 24 bit or 32 bit address.
32bit address.
There is no need to swap the address bytes. The current driver
(drivers/mtd/spi-nor/cadence-quadspi.c) does not swap the address to SPI
bus order, why does the new driver required to do so?
Thanks! for clarification, actually we are not swapping , just Converting address buffer into word format (MSB first).
+    reg = op->cmd.opcode << CQSPI_REG_RD_INSTR_OPCODE_LSB;
+    reg |= (op->data.buswidth & CQSPI_REG_RD_INSTR_TYPE_DATA_MASK) <<
+        CQSPI_REG_RD_INSTR_TYPE_DATA_LSB;
+
This is wrong... op->data.buswidth's range is 1 to 8 whereas
CQSPI_REG_RD_INSTR_TYPE range is 0 to 3. I wonder whether you tested
dual/quad mode with this driver?
Yes I have tested with Quad mode since Cadence-IP supports dual/quad on
my platform, used to validate
before sending the patch that's  standard procedure here.
please let me know if you have any further queries.

Then I have no idea how it works on your platform..
while testing on my platform I have hardcoded to 4 instead of 8, for me it is working fine.
should be handled properly for OCTAL mode once your changes are ready

  What you are
programming above overflows the assigned bit fields for bus width, right?
yes, overflow should be handled
once started working on your platform with your changes, I will squash and send it back.
Thanks for your time.

Regards
Vadivel
---
Best Regards
Vadivel
I am still unable to get this series to work on my platform. Will
continue to debug...




[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux