Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Tue, Jun 26, 2018 at 4:47 PM, Boris Brezillon wrote: > Hi Andy, > > On Tue, 26 Jun 2018 16:18:44 +0300 > Andy Shevchenko wrote: > >> >> > What is wrong? Some newlines are missing here between the MODULE_ macros, >> > but in my original patch it seems correct. >> >> It should be like >> >> MODULE_FOO(...); >> MODULE_BAR(...); >> MODULE_BAZ(...); >> >> One macro — one line. > > It's not Frieder's fault, it's Yogesh mailer which messed-up the > formatting. Just look at the original patch and you'll see. One problem less, then. Good! -- With Best Regards, Andy Shevchenko
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Tue, Jun 26, 2018 at 4:47 PM, Boris Brezillon wrote: > Hi Andy, > > On Tue, 26 Jun 2018 16:18:44 +0300 > Andy Shevchenko wrote: > >> >> > What is wrong? Some newlines are missing here between the MODULE_ macros, >> > but in my original patch it seems correct. >> >> It should be like >> >> MODULE_FOO(...); >> MODULE_BAR(...); >> MODULE_BAZ(...); >> >> One macro — one line. > > It's not Frieder's fault, it's Yogesh mailer which messed-up the > formatting. Just look at the original patch and you'll see. One problem less, then. Good! -- With Best Regards, Andy Shevchenko
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Andy, On Tue, 26 Jun 2018 16:18:44 +0300 Andy Shevchenko wrote: > > > What is wrong? Some newlines are missing here between the MODULE_ macros, > > but in my original patch it seems correct. > > It should be like > > MODULE_FOO(...); > MODULE_BAR(...); > MODULE_BAZ(...); > > One macro — one line. It's not Frieder's fault, it's Yogesh mailer which messed-up the formatting. Just look at the original patch and you'll see.
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Andy, On Tue, 26 Jun 2018 16:18:44 +0300 Andy Shevchenko wrote: > > > What is wrong? Some newlines are missing here between the MODULE_ macros, > > but in my original patch it seems correct. > > It should be like > > MODULE_FOO(...); > MODULE_BAR(...); > MODULE_BAZ(...); > > One macro — one line. It's not Frieder's fault, it's Yogesh mailer which messed-up the formatting. Just look at the original patch and you'll see.
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Tue, Jun 26, 2018 at 3:26 PM, Frieder Schrempf
wrote:
> On 08.06.2018 22:27, Andy Shevchenko wrote:
>> On Fri, Jun 8, 2018 at 2:54 PM, Yogesh Narayan Gaur
>> wrote:
>>> +static int fsl_qspi_check_buswidth(struct fsl_qspi *q, u8 width) {
>>> + switch (width) {
>>> + case 1:
>>> + case 2:
>>> + case 4:
>>> + return 0;
>>> + }
>> if (!is_power_of_2(width) || width >= 8)
>> return -E...;
>>
>> return 0;
>>
>> ?
> Your proposition is a bit shorter, but I think it's slightly harder to read.
OK.
>>> +
>>> + return -ENOTSUPP;
>>> +}
>>> + for (i = 0; i < op->data.nbytes; i += 4) {
>>> + u32 val = 0;
>>> +
>>> + memcpy(, op->data.buf.out + i,
>>> + min_t(unsigned int, op->data.nbytes - i, 4));
>> You may easily avoid this conditional on each iteration.
> Do you mean something like this, or are there better ways?
>
> u32 val = 0;
>
> for (i = 0; i < ALIGN_DOWN(op->data.nbytes, 4); i += 4)
> {
> memcpy(, op->data.buf.out + i, 4);
> val = fsl_qspi_endian_xchg(q, val);
> qspi_writel(q, val, base + QUADSPI_TBDR);
> }
>
> memcpy(, op->data.buf.out + i, op->data.nbytes);
> val = fsl_qspi_endian_xchg(q, val);
> qspi_writel(q, val, base + QUADSPI_TBDR);
Something like this, though last part should go under
if (IS_ALIGNED(...))
(My comment was about moving out an invariant conditional)
>>> + val = fsl_qspi_endian_xchg(q, val);
>>> + qspi_writel(q, val, base + QUADSPI_TBDR);
>>> + }
>>> +MODULE_AUTHOR("Freescale Semiconductor Inc."); MODULE_AUTHOR("Boris
>>> +Brezillion "); MODULE_AUTHOR("Frieder
>>> +Schrempf "); MODULE_LICENSE("GPL v2");
>> Wrong indentation.
> What is wrong? Some newlines are missing here between the MODULE_ macros,
> but in my original patch it seems correct.
It should be like
MODULE_FOO(...);
MODULE_BAR(...);
MODULE_BAZ(...);
One macro — one line.
--
With Best Regards,
Andy Shevchenko
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Tue, Jun 26, 2018 at 3:26 PM, Frieder Schrempf
wrote:
> On 08.06.2018 22:27, Andy Shevchenko wrote:
>> On Fri, Jun 8, 2018 at 2:54 PM, Yogesh Narayan Gaur
>> wrote:
>>> +static int fsl_qspi_check_buswidth(struct fsl_qspi *q, u8 width) {
>>> + switch (width) {
>>> + case 1:
>>> + case 2:
>>> + case 4:
>>> + return 0;
>>> + }
>> if (!is_power_of_2(width) || width >= 8)
>> return -E...;
>>
>> return 0;
>>
>> ?
> Your proposition is a bit shorter, but I think it's slightly harder to read.
OK.
>>> +
>>> + return -ENOTSUPP;
>>> +}
>>> + for (i = 0; i < op->data.nbytes; i += 4) {
>>> + u32 val = 0;
>>> +
>>> + memcpy(, op->data.buf.out + i,
>>> + min_t(unsigned int, op->data.nbytes - i, 4));
>> You may easily avoid this conditional on each iteration.
> Do you mean something like this, or are there better ways?
>
> u32 val = 0;
>
> for (i = 0; i < ALIGN_DOWN(op->data.nbytes, 4); i += 4)
> {
> memcpy(, op->data.buf.out + i, 4);
> val = fsl_qspi_endian_xchg(q, val);
> qspi_writel(q, val, base + QUADSPI_TBDR);
> }
>
> memcpy(, op->data.buf.out + i, op->data.nbytes);
> val = fsl_qspi_endian_xchg(q, val);
> qspi_writel(q, val, base + QUADSPI_TBDR);
Something like this, though last part should go under
if (IS_ALIGNED(...))
(My comment was about moving out an invariant conditional)
>>> + val = fsl_qspi_endian_xchg(q, val);
>>> + qspi_writel(q, val, base + QUADSPI_TBDR);
>>> + }
>>> +MODULE_AUTHOR("Freescale Semiconductor Inc."); MODULE_AUTHOR("Boris
>>> +Brezillion "); MODULE_AUTHOR("Frieder
>>> +Schrempf "); MODULE_LICENSE("GPL v2");
>> Wrong indentation.
> What is wrong? Some newlines are missing here between the MODULE_ macros,
> but in my original patch it seems correct.
It should be like
MODULE_FOO(...);
MODULE_BAR(...);
MODULE_BAZ(...);
One macro — one line.
--
With Best Regards,
Andy Shevchenko
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Andy,
On 08.06.2018 22:27, Andy Shevchenko wrote:
On Fri, Jun 8, 2018 at 2:54 PM, Yogesh Narayan Gaur
wrote:
Hi Frieder,
+#define QUADSPI_MCR_RESERVED_MASK (0xF << 16)
GENMASK()
Ok.
+#define QUADSPI_MCR_END_CFG_MASK (0x3 << 2)
+#define QUADSPI_BUF3CR_ADATSZ_MASK (0xFF << QUADSPI_BUF3CR_ADATSZ_SHIFT)
+#define QUADSPI_SMPR_DDRSMP_MASK (7 << 16)
+#define QUADSPI_RBCT_WMRK_MASK 0x1F
Ditto.
Ok.
+#define QUADSPI_LUT_OFFSET (SEQID_LUT * 4 * 4)
+#define QUADSPI_LUT_REG(idx) (QUADSPI_LUT_BASE + \
+ QUADSPI_LUT_OFFSET + (idx) * 4)
It looks slightly better when
#define FOO \
(BAR1 + BAR2 ...)
Ok.
+/*
+ * An IC bug makes it necessary to rearrange the 32-bit data.
+ * Later chips, such as IMX6SLX, have fixed this bug.
+ */
+static inline u32 fsl_qspi_endian_xchg(struct fsl_qspi *q, u32 a) {
+ return needs_swap_endian(q) ? __swab32(a) : a; }
func()
{
...
}
Fix this everywhere.
I will fix this.
+static void qspi_writel(struct fsl_qspi *q, u32 val, void __iomem
+*addr) {
+ if (q->big_endian)
+ iowrite32be(val, addr);
+ else
+ iowrite32(val, addr);
+}
+
+static u32 qspi_readl(struct fsl_qspi *q, void __iomem *addr) {
+ if (q->big_endian)
+ return ioread32be(addr);
+ else
+ return ioread32(addr);
+}
Better to define ->read() and ->write() callbacks and call them unconditionally.
Ok.
+static int fsl_qspi_check_buswidth(struct fsl_qspi *q, u8 width) {
+ switch (width) {
+ case 1:
+ case 2:
+ case 4:
+ return 0;
+ }
if (!is_power_of_2(width) || width >= 8)
return -E...;
return 0;
?
Your proposition is a bit shorter, but I think it's slightly harder to read.
+
+ return -ENOTSUPP;
+}
+static int fsl_qspi_clk_prep_enable(struct fsl_qspi *q) {
+ int ret;
+
+ ret = clk_prepare_enable(q->clk_en);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(q->clk);
+ if (ret) {
+ clk_disable_unprepare(q->clk_en);
Is it needed here?
No, this is probably superfluous. I will remove it.
+ return ret;
+ }
+
+ if (needs_wakeup_wait_mode(q))
+ pm_qos_add_request(>pm_qos_req, PM_QOS_CPU_DMA_LATENCY, 0);
+
+ return 0;
+}
+ qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4));
Redundant parens.
Ok, I thought this is better for readability.
+ seq = seq ? 0 : 1;
seq = (seq + 1) % 2;
?
Ok.
+}
+ for (i = 0; i < op->data.nbytes; i += 4) {
+ u32 val = 0;
+
+ memcpy(, op->data.buf.out + i,
+ min_t(unsigned int, op->data.nbytes - i, 4));
You may easily avoid this conditional on each iteration.
Do you mean something like this, or are there better ways?
u32 val = 0;
for (i = 0; i < ALIGN_DOWN(op->data.nbytes, 4); i += 4)
{
memcpy(, op->data.buf.out + i, 4);
val = fsl_qspi_endian_xchg(q, val);
qspi_writel(q, val, base + QUADSPI_TBDR);
}
memcpy(, op->data.buf.out + i, op->data.nbytes);
val = fsl_qspi_endian_xchg(q, val);
qspi_writel(q, val, base + QUADSPI_TBDR);
+
+ val = fsl_qspi_endian_xchg(q, val);
+ qspi_writel(q, val, base + QUADSPI_TBDR);
+ }
+ /* wait for the controller being ready */
FOREVER! See below.
+ do {
+ u32 status;
+
+ status = qspi_readl(q, base + QUADSPI_SR);
+ if (status &
+ (QUADSPI_SR_IP_ACC_MASK | QUADSPI_SR_AHB_ACC_MASK)) {
+ udelay(1);
+ dev_dbg(q->dev, "The controller is busy, 0x%x\n",
+ status);
+ continue;
+ }
+ break;
+ } while (1);
Please, avoid infinite loops.
unsigned int count = 100;
...
do {
...
} while (--count);
Ok, will change that.
+ if (of_get_available_child_count(q->dev->of_node) == 1)
+ name = dev_name(q->dev);
+ else
+ name = devm_kasprintf(dev, GFP_KERNEL,
+ "%s-%d", dev_name(q->dev),
+ mem->spi->chip_select);
+
+ if (!name) {
+ dev_err(dev, "failed to get memory for custom flash name\n");
+ return dev_name(q->dev);
Might it be racy if in between device gets a name assigned?
Ok, I will change that to make sure that dev_name() is only called once.
+ }
+ if (q->ahb_addr)
+ iounmap(q->ahb_addr);
Double unmap?
Right, this is redundant. I will remove it.
+static struct platform_driver fsl_qspi_driver = {
+ .driver = {
+ .name = "fsl-quadspi",
+ .of_match_table = fsl_qspi_dt_ids,
+
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Andy,
On 08.06.2018 22:27, Andy Shevchenko wrote:
On Fri, Jun 8, 2018 at 2:54 PM, Yogesh Narayan Gaur
wrote:
Hi Frieder,
+#define QUADSPI_MCR_RESERVED_MASK (0xF << 16)
GENMASK()
Ok.
+#define QUADSPI_MCR_END_CFG_MASK (0x3 << 2)
+#define QUADSPI_BUF3CR_ADATSZ_MASK (0xFF << QUADSPI_BUF3CR_ADATSZ_SHIFT)
+#define QUADSPI_SMPR_DDRSMP_MASK (7 << 16)
+#define QUADSPI_RBCT_WMRK_MASK 0x1F
Ditto.
Ok.
+#define QUADSPI_LUT_OFFSET (SEQID_LUT * 4 * 4)
+#define QUADSPI_LUT_REG(idx) (QUADSPI_LUT_BASE + \
+ QUADSPI_LUT_OFFSET + (idx) * 4)
It looks slightly better when
#define FOO \
(BAR1 + BAR2 ...)
Ok.
+/*
+ * An IC bug makes it necessary to rearrange the 32-bit data.
+ * Later chips, such as IMX6SLX, have fixed this bug.
+ */
+static inline u32 fsl_qspi_endian_xchg(struct fsl_qspi *q, u32 a) {
+ return needs_swap_endian(q) ? __swab32(a) : a; }
func()
{
...
}
Fix this everywhere.
I will fix this.
+static void qspi_writel(struct fsl_qspi *q, u32 val, void __iomem
+*addr) {
+ if (q->big_endian)
+ iowrite32be(val, addr);
+ else
+ iowrite32(val, addr);
+}
+
+static u32 qspi_readl(struct fsl_qspi *q, void __iomem *addr) {
+ if (q->big_endian)
+ return ioread32be(addr);
+ else
+ return ioread32(addr);
+}
Better to define ->read() and ->write() callbacks and call them unconditionally.
Ok.
+static int fsl_qspi_check_buswidth(struct fsl_qspi *q, u8 width) {
+ switch (width) {
+ case 1:
+ case 2:
+ case 4:
+ return 0;
+ }
if (!is_power_of_2(width) || width >= 8)
return -E...;
return 0;
?
Your proposition is a bit shorter, but I think it's slightly harder to read.
+
+ return -ENOTSUPP;
+}
+static int fsl_qspi_clk_prep_enable(struct fsl_qspi *q) {
+ int ret;
+
+ ret = clk_prepare_enable(q->clk_en);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(q->clk);
+ if (ret) {
+ clk_disable_unprepare(q->clk_en);
Is it needed here?
No, this is probably superfluous. I will remove it.
+ return ret;
+ }
+
+ if (needs_wakeup_wait_mode(q))
+ pm_qos_add_request(>pm_qos_req, PM_QOS_CPU_DMA_LATENCY, 0);
+
+ return 0;
+}
+ qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4));
Redundant parens.
Ok, I thought this is better for readability.
+ seq = seq ? 0 : 1;
seq = (seq + 1) % 2;
?
Ok.
+}
+ for (i = 0; i < op->data.nbytes; i += 4) {
+ u32 val = 0;
+
+ memcpy(, op->data.buf.out + i,
+ min_t(unsigned int, op->data.nbytes - i, 4));
You may easily avoid this conditional on each iteration.
Do you mean something like this, or are there better ways?
u32 val = 0;
for (i = 0; i < ALIGN_DOWN(op->data.nbytes, 4); i += 4)
{
memcpy(, op->data.buf.out + i, 4);
val = fsl_qspi_endian_xchg(q, val);
qspi_writel(q, val, base + QUADSPI_TBDR);
}
memcpy(, op->data.buf.out + i, op->data.nbytes);
val = fsl_qspi_endian_xchg(q, val);
qspi_writel(q, val, base + QUADSPI_TBDR);
+
+ val = fsl_qspi_endian_xchg(q, val);
+ qspi_writel(q, val, base + QUADSPI_TBDR);
+ }
+ /* wait for the controller being ready */
FOREVER! See below.
+ do {
+ u32 status;
+
+ status = qspi_readl(q, base + QUADSPI_SR);
+ if (status &
+ (QUADSPI_SR_IP_ACC_MASK | QUADSPI_SR_AHB_ACC_MASK)) {
+ udelay(1);
+ dev_dbg(q->dev, "The controller is busy, 0x%x\n",
+ status);
+ continue;
+ }
+ break;
+ } while (1);
Please, avoid infinite loops.
unsigned int count = 100;
...
do {
...
} while (--count);
Ok, will change that.
+ if (of_get_available_child_count(q->dev->of_node) == 1)
+ name = dev_name(q->dev);
+ else
+ name = devm_kasprintf(dev, GFP_KERNEL,
+ "%s-%d", dev_name(q->dev),
+ mem->spi->chip_select);
+
+ if (!name) {
+ dev_err(dev, "failed to get memory for custom flash name\n");
+ return dev_name(q->dev);
Might it be racy if in between device gets a name assigned?
Ok, I will change that to make sure that dev_name() is only called once.
+ }
+ if (q->ahb_addr)
+ iounmap(q->ahb_addr);
Double unmap?
Right, this is redundant. I will remove it.
+static struct platform_driver fsl_qspi_driver = {
+ .driver = {
+ .name = "fsl-quadspi",
+ .of_match_table = fsl_qspi_dt_ids,
+
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, hi Yogesh, first, thank you for testing and debugging the new driver. On 19.06.2018 10:46, Boris Brezillon wrote: On Tue, 19 Jun 2018 08:31:25 + Yogesh Narayan Gaur wrote: [...] On Tue, 19 Jun 2018 07:10:37 + Yogesh Narayan Gaur wrote: Let us take below layout of memory address space map. QuadSPI Controller can access range from 0x2000_ - 0x2FFF_ i.e. 256 MB address space reserved and it is having 4 slave devices connected. These slave devices[of size 64MB, 64MB, 32MB and 64MB ] are connected at below address 0x2000_, 0x2400_, 0x2A00_, 0x2C00_ i.e. there is gap of 32MB from 0x2800_ to 0x29FF_. Okay, I'm fine with pre-reserving 32MB per chip select. As per my understanding of the controller, flash XX top address, register should have below values: QUADSPI_SFA1AD - 0x0 QUADSPI_SFA2AD - 0x400_ QUADSPI_SFB1AD - 0xA00_ QUADSPI_SFB2AD - 0xC00_ And Register QUADSPI_SFAR should point to the range for the flash in which operation is happening. My mistake values of these register would be for said case are: QUADSPI_SFA1AD - 0x400_ QUADSPI_SFA2AD - 0x800_ QUADSPI_SFB1AD - 0xC00_ QUADSPI_SFB2AD - 0x1000_ i.e. as per controller each register is having the Top address for serial flash connected at A1/A2/B1/B2 respectively. This is still wrong ;-). I guess you mean: QUADSPI_SFA1AD - 0x2400_ QUADSPI_SFA2AD - 0x2800_ QUADSPI_SFB1AD - 0x2C00_ QUADSPI_SFB2AD - 0x3000_ Wait, I thought it was supposed to be an absolute address, not one relative to the 0x2000 offset. Please check Table10-32, page 1657, in [1] for more details on flash address assignment. Yes, I still don't see where it says that having one of the range with a zero size is forbidden, or anything mentioning a required alignment. But say if I assign address to register QUADSPI_SFA2AD as "0 + 2 * - ahb_buf_size" then this address value is not correct as per the value range explained in above mentioned table. Why? If the SFA1AD is set to zero, that should not, right? What this table says that for TOP_ADDR_MEMA1 defines the top address for flash connected at A1 and any address space between TOP_ADDR_MEMA1 and QSPI_AMBA_BASE will be routed to Serial Flash A1. In my example case TOP_ADDR_MEMA1 is 0x400_ If assign value to SFAR register is "0 + 2 * ->ahb_buf_size", then this would lie in access range of Serial Flash A1 and access happens to A1 flash whereas we want access to A2 flash. No, not if SFA1AD is 0x2000, because then the address range for CS0 would be 0x2000 -> 0x2000. If you look at the code, you'll see that I adjust the CS mapping dynamically, making the one being access use the range 0x2000 -> (0x2000 + 2 * ->ahb_buf_size) and assigning a 0-size range for the other ones (either 0x2000 -> 0x2000 or (0x2000 + 2 * ->ahb_buf_size) -> (0x2000 + 2 * ->ahb_buf_size)) For access of serial flash A2, any address space access between TOP_ADDR_MEMA2 and TOP_ADDR_MEMA1 would be routed to serial flash A2. Thus to access A2 flash, SFAR would be in range from 0x400_ and 0x800_ I understand what you're explaining, what I don't get is why the QSPI IP doesn't cope with a 0-size range. If you have SFA1AD set to 0x2000 and SFA2AD set so 0x2800, I would except any access to the 0x2000 -> 0x2800 range to be routed to CS1 not CS0. But apparently it's not working like that. But it should definitely be possible to have 0-size ranges in the mapping. At least the RM for the i.MX6UL says: "In case single die flash devices, TOP_ADDR_MEMA2 and TOP_ADDR_MEMB2 should be initialized/programmed to TOP_ADDR_MEMA1 and TOP_ADDR_MEMB1 respectively- in effect, setting the size of these devices to 0. This would ensure that the complete memory map is assigned to only one flash device." Yogesh, can you test if it works with the fixed mapping proposed above, reserving a fixed range of 2 * ahb_buf_size for each chip? Thanks, Frieder
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, hi Yogesh, first, thank you for testing and debugging the new driver. On 19.06.2018 10:46, Boris Brezillon wrote: On Tue, 19 Jun 2018 08:31:25 + Yogesh Narayan Gaur wrote: [...] On Tue, 19 Jun 2018 07:10:37 + Yogesh Narayan Gaur wrote: Let us take below layout of memory address space map. QuadSPI Controller can access range from 0x2000_ - 0x2FFF_ i.e. 256 MB address space reserved and it is having 4 slave devices connected. These slave devices[of size 64MB, 64MB, 32MB and 64MB ] are connected at below address 0x2000_, 0x2400_, 0x2A00_, 0x2C00_ i.e. there is gap of 32MB from 0x2800_ to 0x29FF_. Okay, I'm fine with pre-reserving 32MB per chip select. As per my understanding of the controller, flash XX top address, register should have below values: QUADSPI_SFA1AD - 0x0 QUADSPI_SFA2AD - 0x400_ QUADSPI_SFB1AD - 0xA00_ QUADSPI_SFB2AD - 0xC00_ And Register QUADSPI_SFAR should point to the range for the flash in which operation is happening. My mistake values of these register would be for said case are: QUADSPI_SFA1AD - 0x400_ QUADSPI_SFA2AD - 0x800_ QUADSPI_SFB1AD - 0xC00_ QUADSPI_SFB2AD - 0x1000_ i.e. as per controller each register is having the Top address for serial flash connected at A1/A2/B1/B2 respectively. This is still wrong ;-). I guess you mean: QUADSPI_SFA1AD - 0x2400_ QUADSPI_SFA2AD - 0x2800_ QUADSPI_SFB1AD - 0x2C00_ QUADSPI_SFB2AD - 0x3000_ Wait, I thought it was supposed to be an absolute address, not one relative to the 0x2000 offset. Please check Table10-32, page 1657, in [1] for more details on flash address assignment. Yes, I still don't see where it says that having one of the range with a zero size is forbidden, or anything mentioning a required alignment. But say if I assign address to register QUADSPI_SFA2AD as "0 + 2 * - ahb_buf_size" then this address value is not correct as per the value range explained in above mentioned table. Why? If the SFA1AD is set to zero, that should not, right? What this table says that for TOP_ADDR_MEMA1 defines the top address for flash connected at A1 and any address space between TOP_ADDR_MEMA1 and QSPI_AMBA_BASE will be routed to Serial Flash A1. In my example case TOP_ADDR_MEMA1 is 0x400_ If assign value to SFAR register is "0 + 2 * ->ahb_buf_size", then this would lie in access range of Serial Flash A1 and access happens to A1 flash whereas we want access to A2 flash. No, not if SFA1AD is 0x2000, because then the address range for CS0 would be 0x2000 -> 0x2000. If you look at the code, you'll see that I adjust the CS mapping dynamically, making the one being access use the range 0x2000 -> (0x2000 + 2 * ->ahb_buf_size) and assigning a 0-size range for the other ones (either 0x2000 -> 0x2000 or (0x2000 + 2 * ->ahb_buf_size) -> (0x2000 + 2 * ->ahb_buf_size)) For access of serial flash A2, any address space access between TOP_ADDR_MEMA2 and TOP_ADDR_MEMA1 would be routed to serial flash A2. Thus to access A2 flash, SFAR would be in range from 0x400_ and 0x800_ I understand what you're explaining, what I don't get is why the QSPI IP doesn't cope with a 0-size range. If you have SFA1AD set to 0x2000 and SFA2AD set so 0x2800, I would except any access to the 0x2000 -> 0x2800 range to be routed to CS1 not CS0. But apparently it's not working like that. But it should definitely be possible to have 0-size ranges in the mapping. At least the RM for the i.MX6UL says: "In case single die flash devices, TOP_ADDR_MEMA2 and TOP_ADDR_MEMB2 should be initialized/programmed to TOP_ADDR_MEMA1 and TOP_ADDR_MEMB1 respectively- in effect, setting the size of these devices to 0. This would ensure that the complete memory map is assigned to only one flash device." Yogesh, can you test if it works with the fixed mapping proposed above, reserving a fixed range of 2 * ahb_buf_size for each chip? Thanks, Frieder
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Tue, 19 Jun 2018 08:31:25 + Yogesh Narayan Gaur wrote: > > > > Could you please use a mailer that is quoting things correctly. I > > have a hard time differentiating your replies from mine. > > Sorry for this, have changed my mailer settings. Thanks for doing. It's still not perfect, but it's definitely better. > > > > > On Tue, 19 Jun 2018 07:10:37 + > > Yogesh Narayan Gaur wrote: > > > > > Let us take below layout of memory address space map. > > > QuadSPI Controller can access range from 0x2000_ - > > > 0x2FFF_ i.e. 256 > > MB address space reserved and it is having 4 slave devices > > connected. > > > These slave devices[of size 64MB, 64MB, 32MB and 64MB ] are > > > connected at below address 0x2000_, 0x2400_, 0x2A00_, > > > 0x2C00_ i.e. there is gap of 32MB from 0x2800_ to > > > 0x29FF_. > > > > Okay, I'm fine with pre-reserving 32MB per chip select. > > > > > > > > As per my understanding of the controller, flash XX top address, > > > register should > > have below values: > > > QUADSPI_SFA1AD - 0x0 > > > QUADSPI_SFA2AD - 0x400_ > > > QUADSPI_SFB1AD - 0xA00_ > > > QUADSPI_SFB2AD - 0xC00_ > > > And Register QUADSPI_SFAR should point to the range for the flash > > > in which > > operation is happening. > > My mistake values of these register would be for said case are: > QUADSPI_SFA1AD - 0x400_ > QUADSPI_SFA2AD - 0x800_ > QUADSPI_SFB1AD - 0xC00_ > QUADSPI_SFB2AD - 0x1000_ > > i.e. as per controller each register is having the Top address for > serial flash connected at A1/A2/B1/B2 respectively. This is still wrong ;-). I guess you mean: QUADSPI_SFA1AD - 0x2400_ QUADSPI_SFA2AD - 0x2800_ QUADSPI_SFB1AD - 0x2C00_ QUADSPI_SFB2AD - 0x3000_ > > > > > Wait, I thought it was supposed to be an absolute address, not one > > relative to the 0x2000 offset. > > > > > > > > Please check Table10-32, page 1657, in [1] for more details on > > > flash address > > assignment. > > > > Yes, I still don't see where it says that having one of the range > > with a zero size is forbidden, or anything mentioning a required > > alignment. > > > > > > But say if I assign address to register QUADSPI_SFA2AD as "0 + 2 > > > * - > > >ahb_buf_size" then this address value is not correct as per the > > >value range > > explained in above mentioned table. > > > > Why? If the SFA1AD is set to zero, that should not, right? > What this table says that for TOP_ADDR_MEMA1 defines the top address > for flash connected at A1 and any address space between > TOP_ADDR_MEMA1 and QSPI_AMBA_BASE will be routed to Serial Flash A1. > In my example case TOP_ADDR_MEMA1 is 0x400_ If assign value to > SFAR register is "0 + 2 * ->ahb_buf_size", then this would lie in > access range of Serial Flash A1 and access happens to A1 flash > whereas we want access to A2 flash. No, not if SFA1AD is 0x2000, because then the address range for CS0 would be 0x2000 -> 0x2000. If you look at the code, you'll see that I adjust the CS mapping dynamically, making the one being access use the range 0x2000 -> (0x2000 + 2 * ->ahb_buf_size) and assigning a 0-size range for the other ones (either 0x2000 -> 0x2000 or (0x2000 + 2 * ->ahb_buf_size) -> (0x2000 + 2 * ->ahb_buf_size)) > > For access of serial flash A2, any address space access between > TOP_ADDR_MEMA2 and TOP_ADDR_MEMA1 would be routed to serial flash A2. > Thus to access A2 flash, SFAR would be in range from 0x400_ and > 0x800_ I understand what you're explaining, what I don't get is why the QSPI IP doesn't cope with a 0-size range. If you have SFA1AD set to 0x2000 and SFA2AD set so 0x2800, I would except any access to the 0x2000 -> 0x2800 range to be routed to CS1 not CS0. But apparently it's not working like that.
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Tue, 19 Jun 2018 08:31:25 + Yogesh Narayan Gaur wrote: > > > > Could you please use a mailer that is quoting things correctly. I > > have a hard time differentiating your replies from mine. > > Sorry for this, have changed my mailer settings. Thanks for doing. It's still not perfect, but it's definitely better. > > > > > On Tue, 19 Jun 2018 07:10:37 + > > Yogesh Narayan Gaur wrote: > > > > > Let us take below layout of memory address space map. > > > QuadSPI Controller can access range from 0x2000_ - > > > 0x2FFF_ i.e. 256 > > MB address space reserved and it is having 4 slave devices > > connected. > > > These slave devices[of size 64MB, 64MB, 32MB and 64MB ] are > > > connected at below address 0x2000_, 0x2400_, 0x2A00_, > > > 0x2C00_ i.e. there is gap of 32MB from 0x2800_ to > > > 0x29FF_. > > > > Okay, I'm fine with pre-reserving 32MB per chip select. > > > > > > > > As per my understanding of the controller, flash XX top address, > > > register should > > have below values: > > > QUADSPI_SFA1AD - 0x0 > > > QUADSPI_SFA2AD - 0x400_ > > > QUADSPI_SFB1AD - 0xA00_ > > > QUADSPI_SFB2AD - 0xC00_ > > > And Register QUADSPI_SFAR should point to the range for the flash > > > in which > > operation is happening. > > My mistake values of these register would be for said case are: > QUADSPI_SFA1AD - 0x400_ > QUADSPI_SFA2AD - 0x800_ > QUADSPI_SFB1AD - 0xC00_ > QUADSPI_SFB2AD - 0x1000_ > > i.e. as per controller each register is having the Top address for > serial flash connected at A1/A2/B1/B2 respectively. This is still wrong ;-). I guess you mean: QUADSPI_SFA1AD - 0x2400_ QUADSPI_SFA2AD - 0x2800_ QUADSPI_SFB1AD - 0x2C00_ QUADSPI_SFB2AD - 0x3000_ > > > > > Wait, I thought it was supposed to be an absolute address, not one > > relative to the 0x2000 offset. > > > > > > > > Please check Table10-32, page 1657, in [1] for more details on > > > flash address > > assignment. > > > > Yes, I still don't see where it says that having one of the range > > with a zero size is forbidden, or anything mentioning a required > > alignment. > > > > > > But say if I assign address to register QUADSPI_SFA2AD as "0 + 2 > > > * - > > >ahb_buf_size" then this address value is not correct as per the > > >value range > > explained in above mentioned table. > > > > Why? If the SFA1AD is set to zero, that should not, right? > What this table says that for TOP_ADDR_MEMA1 defines the top address > for flash connected at A1 and any address space between > TOP_ADDR_MEMA1 and QSPI_AMBA_BASE will be routed to Serial Flash A1. > In my example case TOP_ADDR_MEMA1 is 0x400_ If assign value to > SFAR register is "0 + 2 * ->ahb_buf_size", then this would lie in > access range of Serial Flash A1 and access happens to A1 flash > whereas we want access to A2 flash. No, not if SFA1AD is 0x2000, because then the address range for CS0 would be 0x2000 -> 0x2000. If you look at the code, you'll see that I adjust the CS mapping dynamically, making the one being access use the range 0x2000 -> (0x2000 + 2 * ->ahb_buf_size) and assigning a 0-size range for the other ones (either 0x2000 -> 0x2000 or (0x2000 + 2 * ->ahb_buf_size) -> (0x2000 + 2 * ->ahb_buf_size)) > > For access of serial flash A2, any address space access between > TOP_ADDR_MEMA2 and TOP_ADDR_MEMA1 would be routed to serial flash A2. > Thus to access A2 flash, SFAR would be in range from 0x400_ and > 0x800_ I understand what you're explaining, what I don't get is why the QSPI IP doesn't cope with a 0-size range. If you have SFA1AD set to 0x2000 and SFA2AD set so 0x2800, I would except any access to the 0x2000 -> 0x2800 range to be routed to CS1 not CS0. But apparently it's not working like that.
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Tuesday, June 19, 2018 12:59 PM > To: Yogesh Narayan Gaur ; > marek.va...@gmail.com; Frieder Schrempf ; > broo...@kernel.org > Cc: Fabio Estevam ; David Wolfe > ; dw...@infradead.org; rich...@nod.at; Prabhakar > Kushwaha ; Han Xu ; linux- > ker...@vger.kernel.org; linux-...@vger.kernel.org; linux- > m...@lists.infradead.org; miquel.ray...@bootlin.com; > computersforpe...@gmail.com > Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI > controller > > Hi Yogesh, > > Could you please use a mailer that is quoting things correctly. I have a hard > time > differentiating your replies from mine. Sorry for this, have changed my mailer settings. > > On Tue, 19 Jun 2018 07:10:37 + > Yogesh Narayan Gaur wrote: > > > Let us take below layout of memory address space map. > > QuadSPI Controller can access range from 0x2000_ - 0x2FFF_ i.e. 256 > MB address space reserved and it is having 4 slave devices connected. > > These slave devices[of size 64MB, 64MB, 32MB and 64MB ] are connected > > at below address 0x2000_, 0x2400_, 0x2A00_, 0x2C00_ > > i.e. there is gap of 32MB from 0x2800_ to 0x29FF_. > > Okay, I'm fine with pre-reserving 32MB per chip select. > > > > > As per my understanding of the controller, flash XX top address, register > > should > have below values: > > QUADSPI_SFA1AD - 0x0 > > QUADSPI_SFA2AD - 0x400_ > > QUADSPI_SFB1AD - 0xA00_ > > QUADSPI_SFB2AD - 0xC00_ > > And Register QUADSPI_SFAR should point to the range for the flash in which > operation is happening. My mistake values of these register would be for said case are: QUADSPI_SFA1AD - 0x400_ QUADSPI_SFA2AD - 0x800_ QUADSPI_SFB1AD - 0xC00_ QUADSPI_SFB2AD - 0x1000_ i.e. as per controller each register is having the Top address for serial flash connected at A1/A2/B1/B2 respectively. > > Wait, I thought it was supposed to be an absolute address, not one relative to > the 0x2000 offset. > > > > > Please check Table10-32, page 1657, in [1] for more details on flash address > assignment. > > Yes, I still don't see where it says that having one of the range with a zero > size is > forbidden, or anything mentioning a required alignment. > > > > > But say if I assign address to register QUADSPI_SFA2AD as "0 + 2 * - > >ahb_buf_size" then this address value is not correct as per the value range > explained in above mentioned table. > > Why? If the SFA1AD is set to zero, that should not, right? What this table says that for TOP_ADDR_MEMA1 defines the top address for flash connected at A1 and any address space between TOP_ADDR_MEMA1 and QSPI_AMBA_BASE will be routed to Serial Flash A1. In my example case TOP_ADDR_MEMA1 is 0x400_ If assign value to SFAR register is "0 + 2 * ->ahb_buf_size", then this would lie in access range of Serial Flash A1 and access happens to A1 flash whereas we want access to A2 flash. For access of serial flash A2, any address space access between TOP_ADDR_MEMA2 and TOP_ADDR_MEMA1 would be routed to serial flash A2. Thus to access A2 flash, SFAR would be in range from 0x400_ and 0x800_ -- Regards Yogesh Gaur
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Tuesday, June 19, 2018 12:59 PM > To: Yogesh Narayan Gaur ; > marek.va...@gmail.com; Frieder Schrempf ; > broo...@kernel.org > Cc: Fabio Estevam ; David Wolfe > ; dw...@infradead.org; rich...@nod.at; Prabhakar > Kushwaha ; Han Xu ; linux- > ker...@vger.kernel.org; linux-...@vger.kernel.org; linux- > m...@lists.infradead.org; miquel.ray...@bootlin.com; > computersforpe...@gmail.com > Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI > controller > > Hi Yogesh, > > Could you please use a mailer that is quoting things correctly. I have a hard > time > differentiating your replies from mine. Sorry for this, have changed my mailer settings. > > On Tue, 19 Jun 2018 07:10:37 + > Yogesh Narayan Gaur wrote: > > > Let us take below layout of memory address space map. > > QuadSPI Controller can access range from 0x2000_ - 0x2FFF_ i.e. 256 > MB address space reserved and it is having 4 slave devices connected. > > These slave devices[of size 64MB, 64MB, 32MB and 64MB ] are connected > > at below address 0x2000_, 0x2400_, 0x2A00_, 0x2C00_ > > i.e. there is gap of 32MB from 0x2800_ to 0x29FF_. > > Okay, I'm fine with pre-reserving 32MB per chip select. > > > > > As per my understanding of the controller, flash XX top address, register > > should > have below values: > > QUADSPI_SFA1AD - 0x0 > > QUADSPI_SFA2AD - 0x400_ > > QUADSPI_SFB1AD - 0xA00_ > > QUADSPI_SFB2AD - 0xC00_ > > And Register QUADSPI_SFAR should point to the range for the flash in which > operation is happening. My mistake values of these register would be for said case are: QUADSPI_SFA1AD - 0x400_ QUADSPI_SFA2AD - 0x800_ QUADSPI_SFB1AD - 0xC00_ QUADSPI_SFB2AD - 0x1000_ i.e. as per controller each register is having the Top address for serial flash connected at A1/A2/B1/B2 respectively. > > Wait, I thought it was supposed to be an absolute address, not one relative to > the 0x2000 offset. > > > > > Please check Table10-32, page 1657, in [1] for more details on flash address > assignment. > > Yes, I still don't see where it says that having one of the range with a zero > size is > forbidden, or anything mentioning a required alignment. > > > > > But say if I assign address to register QUADSPI_SFA2AD as "0 + 2 * - > >ahb_buf_size" then this address value is not correct as per the value range > explained in above mentioned table. > > Why? If the SFA1AD is set to zero, that should not, right? What this table says that for TOP_ADDR_MEMA1 defines the top address for flash connected at A1 and any address space between TOP_ADDR_MEMA1 and QSPI_AMBA_BASE will be routed to Serial Flash A1. In my example case TOP_ADDR_MEMA1 is 0x400_ If assign value to SFAR register is "0 + 2 * ->ahb_buf_size", then this would lie in access range of Serial Flash A1 and access happens to A1 flash whereas we want access to A2 flash. For access of serial flash A2, any address space access between TOP_ADDR_MEMA2 and TOP_ADDR_MEMA1 would be routed to serial flash A2. Thus to access A2 flash, SFAR would be in range from 0x400_ and 0x800_ -- Regards Yogesh Gaur
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Yogesh, Could you please use a mailer that is quoting things correctly. I have a hard time differentiating your replies from mine. On Tue, 19 Jun 2018 07:10:37 + Yogesh Narayan Gaur wrote: > Let us take below layout of memory address space map. > QuadSPI Controller can access range from 0x2000_ - 0x2FFF_ i.e. 256 > MB address space reserved and it is having 4 slave devices connected. > These slave devices[of size 64MB, 64MB, 32MB and 64MB ] are connected at > below address > 0x2000_, 0x2400_, 0x2A00_, 0x2C00_ > i.e. there is gap of 32MB from 0x2800_ to 0x29FF_. Okay, I'm fine with pre-reserving 32MB per chip select. > > As per my understanding of the controller, flash XX top address, register > should have below values: > QUADSPI_SFA1AD - 0x0 > QUADSPI_SFA2AD - 0x400_ > QUADSPI_SFB1AD - 0xA00_ > QUADSPI_SFB2AD - 0xC00_ > And Register QUADSPI_SFAR should point to the range for the flash in which > operation is happening. Wait, I thought it was supposed to be an absolute address, not one relative to the 0x2000 offset. > > Please check Table10-32, page 1657, in [1] for more details on flash address > assignment. Yes, I still don't see where it says that having one of the range with a zero size is forbidden, or anything mentioning a required alignment. > > But say if I assign address to register QUADSPI_SFA2AD as "0 + 2 * > ->ahb_buf_size" then this address value is not correct as per the value range > explained in above mentioned table. Why? If the SFA1AD is set to zero, that should not, right?
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Yogesh, Could you please use a mailer that is quoting things correctly. I have a hard time differentiating your replies from mine. On Tue, 19 Jun 2018 07:10:37 + Yogesh Narayan Gaur wrote: > Let us take below layout of memory address space map. > QuadSPI Controller can access range from 0x2000_ - 0x2FFF_ i.e. 256 > MB address space reserved and it is having 4 slave devices connected. > These slave devices[of size 64MB, 64MB, 32MB and 64MB ] are connected at > below address > 0x2000_, 0x2400_, 0x2A00_, 0x2C00_ > i.e. there is gap of 32MB from 0x2800_ to 0x29FF_. Okay, I'm fine with pre-reserving 32MB per chip select. > > As per my understanding of the controller, flash XX top address, register > should have below values: > QUADSPI_SFA1AD - 0x0 > QUADSPI_SFA2AD - 0x400_ > QUADSPI_SFB1AD - 0xA00_ > QUADSPI_SFB2AD - 0xC00_ > And Register QUADSPI_SFAR should point to the range for the flash in which > operation is happening. Wait, I thought it was supposed to be an absolute address, not one relative to the 0x2000 offset. > > Please check Table10-32, page 1657, in [1] for more details on flash address > assignment. Yes, I still don't see where it says that having one of the range with a zero size is forbidden, or anything mentioning a required alignment. > > But say if I assign address to register QUADSPI_SFA2AD as "0 + 2 * > ->ahb_buf_size" then this address value is not correct as per the value range > explained in above mentioned table. Why? If the SFA1AD is set to zero, that should not, right?
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Tuesday, June 19, 2018 12:46 AM To: Yogesh Narayan Gaur Cc: Fabio Estevam ; David Wolfe ; dw...@infradead.org; rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; computersforpe...@gmail.com Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Hi Yogesh, On Mon, 18 Jun 2018 13:32:27 + Yogesh Narayan Gaur wrote: > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Friday, June 15, 2018 7:26 PM > To: Yogesh Narayan Gaur ; Fabio Estevam > ; David Wolfe ; > dw...@infradead.org > Cc: rich...@nod.at; Prabhakar Kushwaha ; > Han Xu ; linux-kernel@vger.kernel.org; > linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf > ; broo...@kernel.org; > linux-...@lists.infradead.org; miquel.ray...@bootlin.com; > computersforpe...@gmail.com > Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP > QuadSPI controller > > On Fri, 15 Jun 2018 13:42:12 + > Yogesh Narayan Gaur wrote: > > > Hi Boris, > > > > I am still debugging the issue. > > With some analysis, able to check that proper values are not being written > > for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. > > > > In current code, value of map_addr are being assigned to these register. > > map_addr = q->memmap_phy + > > 2 * q->devtype_data->ahb_buf_size; > > > > qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); > > > > But instead of "q->devtype_data->ahb_buf_size" it should be flash size. > > No, because we're only using 2 * ->ahb_buf_size in the direct mapping for > each device, and we're modifying the mapping dynamically based on the > selected device. Maybe we got the logic wrong though. > > Yes, for register QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD, we need to > save starting actual address from where this flash is getting started. > Thus, if my first flash size is 64MB, then register QUADSPI_SFA2AD > would have value of q->memmap_phy + 0x400 i.e. (QUADSPI_SFA1AD + sizeof > First Flash) If second flash is of size 32MB, then register QUADSPI_SFB1AD > would have value of value of QUADSPI_SFA2AD + sizeof second flash. Again, no, that's not what I'm trying to do, and the fact that it worked fine with CS0 makes me think you don't need to map the whole device to get it to work, just 2 * ->ahb_buf_size per device. > > > For my case flash size is 0x400 and with this hard coded value I am > > able to perform Write and Erase operation. > > One more change, I have to do is adding the flash_size when writing the > > base_address in SFAR register for case when "mem->spi->chip_select == 1" > > qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); > > I don't want to expose the full device in the direct mapping yet (that's part > of the direct-mapping API I posted here [1]). What this version of the driver > does is, map only 2 time the ahb_size so that we can bypass the internal > cache of the QSPI engine. > > To perform any operation on second flash, we need to provide it's base > address should be saved in SFAR register for this particular operation. That's what we tried to do, we tried to make all CS start at 0 when they are used and declare unused CS at having a size of 0. So, say you're trying to access CS1, you should have the following ranges: CS0: 0 -> 0 (size = 0) CS1: 0 -> 2 * ->ahb_buf_size (size = 2 * ->ahb_buf_size) CS2: 2 * ->ahb_buf_size -> 2 * ->ahb_buf_size (size = 0) CS3: 2 * ->ahb_buf_size -> 2 * ->ahb_buf_size (size = 0) now, if you're trying to access CS3: CS0: 0 -> 0 (size = 0) CS1: 0 -> 0 (size = 0) CS2: 0 -> 0 (size = 0) CS3: 0 -> 2 * ->ahb_buf_size (size = 2 * ->ahb_buf_size) maybe this approach does not work, but that's not clearly stated as 'not supported' in the datasheet. > Exposing only 2 time of ahb_size is design decision but value in SFAR > register should be correct. > > > > > Thus, there should be mechanism or the entry in structure where we can have > > the information of the size of the connected slave device. > > Because that's exactly the kind of thing I'd like to avoid. What if the > device is bigger than the reserved memory region? What if the sum of all > devices does not fit in there? Here I tried to support all cases by j
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Tuesday, June 19, 2018 12:46 AM To: Yogesh Narayan Gaur Cc: Fabio Estevam ; David Wolfe ; dw...@infradead.org; rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; computersforpe...@gmail.com Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Hi Yogesh, On Mon, 18 Jun 2018 13:32:27 + Yogesh Narayan Gaur wrote: > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Friday, June 15, 2018 7:26 PM > To: Yogesh Narayan Gaur ; Fabio Estevam > ; David Wolfe ; > dw...@infradead.org > Cc: rich...@nod.at; Prabhakar Kushwaha ; > Han Xu ; linux-kernel@vger.kernel.org; > linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf > ; broo...@kernel.org; > linux-...@lists.infradead.org; miquel.ray...@bootlin.com; > computersforpe...@gmail.com > Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP > QuadSPI controller > > On Fri, 15 Jun 2018 13:42:12 + > Yogesh Narayan Gaur wrote: > > > Hi Boris, > > > > I am still debugging the issue. > > With some analysis, able to check that proper values are not being written > > for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. > > > > In current code, value of map_addr are being assigned to these register. > > map_addr = q->memmap_phy + > > 2 * q->devtype_data->ahb_buf_size; > > > > qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); > > > > But instead of "q->devtype_data->ahb_buf_size" it should be flash size. > > No, because we're only using 2 * ->ahb_buf_size in the direct mapping for > each device, and we're modifying the mapping dynamically based on the > selected device. Maybe we got the logic wrong though. > > Yes, for register QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD, we need to > save starting actual address from where this flash is getting started. > Thus, if my first flash size is 64MB, then register QUADSPI_SFA2AD > would have value of q->memmap_phy + 0x400 i.e. (QUADSPI_SFA1AD + sizeof > First Flash) If second flash is of size 32MB, then register QUADSPI_SFB1AD > would have value of value of QUADSPI_SFA2AD + sizeof second flash. Again, no, that's not what I'm trying to do, and the fact that it worked fine with CS0 makes me think you don't need to map the whole device to get it to work, just 2 * ->ahb_buf_size per device. > > > For my case flash size is 0x400 and with this hard coded value I am > > able to perform Write and Erase operation. > > One more change, I have to do is adding the flash_size when writing the > > base_address in SFAR register for case when "mem->spi->chip_select == 1" > > qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); > > I don't want to expose the full device in the direct mapping yet (that's part > of the direct-mapping API I posted here [1]). What this version of the driver > does is, map only 2 time the ahb_size so that we can bypass the internal > cache of the QSPI engine. > > To perform any operation on second flash, we need to provide it's base > address should be saved in SFAR register for this particular operation. That's what we tried to do, we tried to make all CS start at 0 when they are used and declare unused CS at having a size of 0. So, say you're trying to access CS1, you should have the following ranges: CS0: 0 -> 0 (size = 0) CS1: 0 -> 2 * ->ahb_buf_size (size = 2 * ->ahb_buf_size) CS2: 2 * ->ahb_buf_size -> 2 * ->ahb_buf_size (size = 0) CS3: 2 * ->ahb_buf_size -> 2 * ->ahb_buf_size (size = 0) now, if you're trying to access CS3: CS0: 0 -> 0 (size = 0) CS1: 0 -> 0 (size = 0) CS2: 0 -> 0 (size = 0) CS3: 0 -> 2 * ->ahb_buf_size (size = 2 * ->ahb_buf_size) maybe this approach does not work, but that's not clearly stated as 'not supported' in the datasheet. > Exposing only 2 time of ahb_size is design decision but value in SFAR > register should be correct. > > > > > Thus, there should be mechanism or the entry in structure where we can have > > the information of the size of the connected slave device. > > Because that's exactly the kind of thing I'd like to avoid. What if the > device is bigger than the reserved memory region? What if the sum of all > devices does not fit in there? Here I tried to support all cases by j
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Yogesh,
On Wed, 30 May 2018 15:14:32 +0200
Frieder Schrempf wrote:
> +static void fsl_qspi_select_mem(struct fsl_qspi *q, struct spi_device *spi)
> +{
> + unsigned long rate = spi->max_speed_hz;
> + int ret, i;
> + u32 map_addr;
> +
> + if (q->selected == spi->chip_select)
> + return;
> +
> + /*
> + * In HW there can be a maximum of four chips on two buses with
> + * two chip selects on each bus. We use four chip selects in SW
> + * to differentiate between the four chips.
> + * We use the SFA1AD, SFA2AD, SFB1AD, SFB2AD registers to select
> + * the chip we want to access.
> + */
> + for (i = 0; i < 4; i++) {
> + if (i < spi->chip_select)
Can you try with:
if (i <= spi->chip_select)
and let me know if it fixes the problem you have when CS != 0?
> + map_addr = q->memmap_phy;
> + else
> + map_addr = q->memmap_phy +
> +2 * q->devtype_data->ahb_buf_size;
> +
> + qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4));
> + }
> +
> + if (needs_4x_clock(q))
> + rate *= 4;
> +
> + fsl_qspi_clk_disable_unprep(q);
> +
> + ret = clk_set_rate(q->clk, rate);
> + if (ret)
> + return;
> +
> + ret = fsl_qspi_clk_prep_enable(q);
> + if (ret)
> + return;
> +
> + q->selected = spi->chip_select;
> +}
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Yogesh,
On Wed, 30 May 2018 15:14:32 +0200
Frieder Schrempf wrote:
> +static void fsl_qspi_select_mem(struct fsl_qspi *q, struct spi_device *spi)
> +{
> + unsigned long rate = spi->max_speed_hz;
> + int ret, i;
> + u32 map_addr;
> +
> + if (q->selected == spi->chip_select)
> + return;
> +
> + /*
> + * In HW there can be a maximum of four chips on two buses with
> + * two chip selects on each bus. We use four chip selects in SW
> + * to differentiate between the four chips.
> + * We use the SFA1AD, SFA2AD, SFB1AD, SFB2AD registers to select
> + * the chip we want to access.
> + */
> + for (i = 0; i < 4; i++) {
> + if (i < spi->chip_select)
Can you try with:
if (i <= spi->chip_select)
and let me know if it fixes the problem you have when CS != 0?
> + map_addr = q->memmap_phy;
> + else
> + map_addr = q->memmap_phy +
> +2 * q->devtype_data->ahb_buf_size;
> +
> + qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4));
> + }
> +
> + if (needs_4x_clock(q))
> + rate *= 4;
> +
> + fsl_qspi_clk_disable_unprep(q);
> +
> + ret = clk_set_rate(q->clk, rate);
> + if (ret)
> + return;
> +
> + ret = fsl_qspi_clk_prep_enable(q);
> + if (ret)
> + return;
> +
> + q->selected = spi->chip_select;
> +}
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Yogesh, On Mon, 18 Jun 2018 13:32:27 + Yogesh Narayan Gaur wrote: > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Friday, June 15, 2018 7:26 PM > To: Yogesh Narayan Gaur ; Fabio Estevam > ; David Wolfe ; > dw...@infradead.org > Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu > ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; > marek.va...@gmail.com; Frieder Schrempf ; > broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; > computersforpe...@gmail.com > Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI > controller > > On Fri, 15 Jun 2018 13:42:12 + > Yogesh Narayan Gaur wrote: > > > Hi Boris, > > > > I am still debugging the issue. > > With some analysis, able to check that proper values are not being written > > for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. > > > > In current code, value of map_addr are being assigned to these register. > > map_addr = q->memmap_phy + > > 2 * q->devtype_data->ahb_buf_size; > > > > qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); > > > > But instead of "q->devtype_data->ahb_buf_size" it should be flash size. > > No, because we're only using 2 * ->ahb_buf_size in the direct mapping for > each device, and we're modifying the mapping dynamically based on the > selected device. Maybe we got the logic wrong though. > > Yes, for register QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD, we need to > save starting actual address from where this flash is getting started. > Thus, if my first flash size is 64MB, then register QUADSPI_SFA2AD would have > value of q->memmap_phy + 0x400 i.e. (QUADSPI_SFA1AD + sizeof First Flash) > If second flash is of size 32MB, then register QUADSPI_SFB1AD would have > value of value of QUADSPI_SFA2AD + sizeof second flash. Again, no, that's not what I'm trying to do, and the fact that it worked fine with CS0 makes me think you don't need to map the whole device to get it to work, just 2 * ->ahb_buf_size per device. > > > For my case flash size is 0x400 and with this hard coded value I am > > able to perform Write and Erase operation. > > One more change, I have to do is adding the flash_size when writing the > > base_address in SFAR register for case when "mem->spi->chip_select == 1" > > qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); > > I don't want to expose the full device in the direct mapping yet (that's part > of the direct-mapping API I posted here [1]). What this version of the driver > does is, map only 2 time the ahb_size so that we can bypass the internal > cache of the QSPI engine. > > To perform any operation on second flash, we need to provide it's base > address should be saved in SFAR register for this particular operation. That's what we tried to do, we tried to make all CS start at 0 when they are used and declare unused CS at having a size of 0. So, say you're trying to access CS1, you should have the following ranges: CS0: 0 -> 0 (size = 0) CS1: 0 -> 2 * ->ahb_buf_size (size = 2 * ->ahb_buf_size) CS2: 2 * ->ahb_buf_size -> 2 * ->ahb_buf_size (size = 0) CS3: 2 * ->ahb_buf_size -> 2 * ->ahb_buf_size (size = 0) now, if you're trying to access CS3: CS0: 0 -> 0 (size = 0) CS1: 0 -> 0 (size = 0) CS2: 0 -> 0 (size = 0) CS3: 0 -> 2 * ->ahb_buf_size (size = 2 * ->ahb_buf_size) maybe this approach does not work, but that's not clearly stated as 'not supported' in the datasheet. > Exposing only 2 time of ahb_size is design decision but value in SFAR > register should be correct. > > > > > Thus, there should be mechanism or the entry in structure where we can have > > the information of the size of the connected slave device. > > Because that's exactly the kind of thing I'd like to avoid. What if the > device is bigger than the reserved memory region? What if the sum of all > devices does not fit in there? Here I tried to support all cases by just > mapping the portion of memory we need. > > So IMO, there should be mechanism to have value of start address of each > slave device. This might can be done from DTS entry of each slave device > connected to the controller. Let's not put that in the DT. If we really can't re-use 0 as the start address and make some ranges 0 in size, then let's reserve 2 * ->ahb_buf_size per chip, and be done with it. This should leave us enough space in the AHB mem range to then support temporary direct mappings through the direct mapping API. Regards, Boris
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Yogesh, On Mon, 18 Jun 2018 13:32:27 + Yogesh Narayan Gaur wrote: > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Friday, June 15, 2018 7:26 PM > To: Yogesh Narayan Gaur ; Fabio Estevam > ; David Wolfe ; > dw...@infradead.org > Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu > ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; > marek.va...@gmail.com; Frieder Schrempf ; > broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; > computersforpe...@gmail.com > Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI > controller > > On Fri, 15 Jun 2018 13:42:12 + > Yogesh Narayan Gaur wrote: > > > Hi Boris, > > > > I am still debugging the issue. > > With some analysis, able to check that proper values are not being written > > for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. > > > > In current code, value of map_addr are being assigned to these register. > > map_addr = q->memmap_phy + > > 2 * q->devtype_data->ahb_buf_size; > > > > qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); > > > > But instead of "q->devtype_data->ahb_buf_size" it should be flash size. > > No, because we're only using 2 * ->ahb_buf_size in the direct mapping for > each device, and we're modifying the mapping dynamically based on the > selected device. Maybe we got the logic wrong though. > > Yes, for register QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD, we need to > save starting actual address from where this flash is getting started. > Thus, if my first flash size is 64MB, then register QUADSPI_SFA2AD would have > value of q->memmap_phy + 0x400 i.e. (QUADSPI_SFA1AD + sizeof First Flash) > If second flash is of size 32MB, then register QUADSPI_SFB1AD would have > value of value of QUADSPI_SFA2AD + sizeof second flash. Again, no, that's not what I'm trying to do, and the fact that it worked fine with CS0 makes me think you don't need to map the whole device to get it to work, just 2 * ->ahb_buf_size per device. > > > For my case flash size is 0x400 and with this hard coded value I am > > able to perform Write and Erase operation. > > One more change, I have to do is adding the flash_size when writing the > > base_address in SFAR register for case when "mem->spi->chip_select == 1" > > qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); > > I don't want to expose the full device in the direct mapping yet (that's part > of the direct-mapping API I posted here [1]). What this version of the driver > does is, map only 2 time the ahb_size so that we can bypass the internal > cache of the QSPI engine. > > To perform any operation on second flash, we need to provide it's base > address should be saved in SFAR register for this particular operation. That's what we tried to do, we tried to make all CS start at 0 when they are used and declare unused CS at having a size of 0. So, say you're trying to access CS1, you should have the following ranges: CS0: 0 -> 0 (size = 0) CS1: 0 -> 2 * ->ahb_buf_size (size = 2 * ->ahb_buf_size) CS2: 2 * ->ahb_buf_size -> 2 * ->ahb_buf_size (size = 0) CS3: 2 * ->ahb_buf_size -> 2 * ->ahb_buf_size (size = 0) now, if you're trying to access CS3: CS0: 0 -> 0 (size = 0) CS1: 0 -> 0 (size = 0) CS2: 0 -> 0 (size = 0) CS3: 0 -> 2 * ->ahb_buf_size (size = 2 * ->ahb_buf_size) maybe this approach does not work, but that's not clearly stated as 'not supported' in the datasheet. > Exposing only 2 time of ahb_size is design decision but value in SFAR > register should be correct. > > > > > Thus, there should be mechanism or the entry in structure where we can have > > the information of the size of the connected slave device. > > Because that's exactly the kind of thing I'd like to avoid. What if the > device is bigger than the reserved memory region? What if the sum of all > devices does not fit in there? Here I tried to support all cases by just > mapping the portion of memory we need. > > So IMO, there should be mechanism to have value of start address of each > slave device. This might can be done from DTS entry of each slave device > connected to the controller. Let's not put that in the DT. If we really can't re-use 0 as the start address and make some ranges 0 in size, then let's reserve 2 * ->ahb_buf_size per chip, and be done with it. This should leave us enough space in the AHB mem range to then support temporary direct mappings through the direct mapping API. Regards, Boris
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
-Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Friday, June 15, 2018 7:26 PM To: Yogesh Narayan Gaur ; Fabio Estevam ; David Wolfe ; dw...@infradead.org Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; computersforpe...@gmail.com Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Fri, 15 Jun 2018 13:42:12 + Yogesh Narayan Gaur wrote: > Hi Boris, > > I am still debugging the issue. > With some analysis, able to check that proper values are not being written > for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. > > In current code, value of map_addr are being assigned to these register. > map_addr = q->memmap_phy + > 2 * q->devtype_data->ahb_buf_size; > > qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); > > But instead of "q->devtype_data->ahb_buf_size" it should be flash size. No, because we're only using 2 * ->ahb_buf_size in the direct mapping for each device, and we're modifying the mapping dynamically based on the selected device. Maybe we got the logic wrong though. Yes, for register QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD, we need to save starting actual address from where this flash is getting started. Thus, if my first flash size is 64MB, then register QUADSPI_SFA2AD would have value of q->memmap_phy + 0x400 i.e. (QUADSPI_SFA1AD + sizeof First Flash) If second flash is of size 32MB, then register QUADSPI_SFB1AD would have value of value of QUADSPI_SFA2AD + sizeof second flash. > For my case flash size is 0x400 and with this hard coded value I am able > to perform Write and Erase operation. > One more change, I have to do is adding the flash_size when writing the > base_address in SFAR register for case when "mem->spi->chip_select == 1" > qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); I don't want to expose the full device in the direct mapping yet (that's part of the direct-mapping API I posted here [1]). What this version of the driver does is, map only 2 time the ahb_size so that we can bypass the internal cache of the QSPI engine. To perform any operation on second flash, we need to provide it's base address should be saved in SFAR register for this particular operation. Exposing only 2 time of ahb_size is design decision but value in SFAR register should be correct. > > Thus, there should be mechanism or the entry in structure where we can have > the information of the size of the connected slave device. Because that's exactly the kind of thing I'd like to avoid. What if the device is bigger than the reserved memory region? What if the sum of all devices does not fit in there? Here I tried to support all cases by just mapping the portion of memory we need. So IMO, there should be mechanism to have value of start address of each slave device. This might can be done from DTS entry of each slave device connected to the controller.
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
-Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Friday, June 15, 2018 7:26 PM To: Yogesh Narayan Gaur ; Fabio Estevam ; David Wolfe ; dw...@infradead.org Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; computersforpe...@gmail.com Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Fri, 15 Jun 2018 13:42:12 + Yogesh Narayan Gaur wrote: > Hi Boris, > > I am still debugging the issue. > With some analysis, able to check that proper values are not being written > for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. > > In current code, value of map_addr are being assigned to these register. > map_addr = q->memmap_phy + > 2 * q->devtype_data->ahb_buf_size; > > qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); > > But instead of "q->devtype_data->ahb_buf_size" it should be flash size. No, because we're only using 2 * ->ahb_buf_size in the direct mapping for each device, and we're modifying the mapping dynamically based on the selected device. Maybe we got the logic wrong though. Yes, for register QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD, we need to save starting actual address from where this flash is getting started. Thus, if my first flash size is 64MB, then register QUADSPI_SFA2AD would have value of q->memmap_phy + 0x400 i.e. (QUADSPI_SFA1AD + sizeof First Flash) If second flash is of size 32MB, then register QUADSPI_SFB1AD would have value of value of QUADSPI_SFA2AD + sizeof second flash. > For my case flash size is 0x400 and with this hard coded value I am able > to perform Write and Erase operation. > One more change, I have to do is adding the flash_size when writing the > base_address in SFAR register for case when "mem->spi->chip_select == 1" > qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); I don't want to expose the full device in the direct mapping yet (that's part of the direct-mapping API I posted here [1]). What this version of the driver does is, map only 2 time the ahb_size so that we can bypass the internal cache of the QSPI engine. To perform any operation on second flash, we need to provide it's base address should be saved in SFAR register for this particular operation. Exposing only 2 time of ahb_size is design decision but value in SFAR register should be correct. > > Thus, there should be mechanism or the entry in structure where we can have > the information of the size of the connected slave device. Because that's exactly the kind of thing I'd like to avoid. What if the device is bigger than the reserved memory region? What if the sum of all devices does not fit in there? Here I tried to support all cases by just mapping the portion of memory we need. So IMO, there should be mechanism to have value of start address of each slave device. This might can be done from DTS entry of each slave device connected to the controller.
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Fri, 15 Jun 2018 15:55:41 +0200 Boris Brezillon wrote: > On Fri, 15 Jun 2018 13:42:12 + > Yogesh Narayan Gaur wrote: > > > Hi Boris, > > > > I am still debugging the issue. > > With some analysis, able to check that proper values are not being written > > for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. > > > > In current code, value of map_addr are being assigned to these register. > > map_addr = q->memmap_phy + > > 2 * q->devtype_data->ahb_buf_size; > > > > qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); > > > > But instead of "q->devtype_data->ahb_buf_size" it should be flash size. > > No, because we're only using 2 * ->ahb_buf_size in the direct mapping > for each device, and we're modifying the mapping dynamically based on > the selected device. Maybe we got the logic wrong though. > > > For my case flash size is 0x400 and with this hard coded value I am > > able to perform Write and Erase operation. > > One more change, I have to do is adding the flash_size when writing the > > base_address in SFAR register for case when "mem->spi->chip_select == 1" > > qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); > > I don't want to expose the full device in the direct mapping yet > (that's part of the direct-mapping API I posted here [1]). What this > version of the driver does is, map only 2 time the ahb_size so that we > can bypass the internal cache of the QSPI engine. Oops, forgot to add the link. [1]http://lists.infradead.org/pipermail/linux-mtd/2018-June/081460.html
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Fri, 15 Jun 2018 15:55:41 +0200 Boris Brezillon wrote: > On Fri, 15 Jun 2018 13:42:12 + > Yogesh Narayan Gaur wrote: > > > Hi Boris, > > > > I am still debugging the issue. > > With some analysis, able to check that proper values are not being written > > for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. > > > > In current code, value of map_addr are being assigned to these register. > > map_addr = q->memmap_phy + > > 2 * q->devtype_data->ahb_buf_size; > > > > qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); > > > > But instead of "q->devtype_data->ahb_buf_size" it should be flash size. > > No, because we're only using 2 * ->ahb_buf_size in the direct mapping > for each device, and we're modifying the mapping dynamically based on > the selected device. Maybe we got the logic wrong though. > > > For my case flash size is 0x400 and with this hard coded value I am > > able to perform Write and Erase operation. > > One more change, I have to do is adding the flash_size when writing the > > base_address in SFAR register for case when "mem->spi->chip_select == 1" > > qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); > > I don't want to expose the full device in the direct mapping yet > (that's part of the direct-mapping API I posted here [1]). What this > version of the driver does is, map only 2 time the ahb_size so that we > can bypass the internal cache of the QSPI engine. Oops, forgot to add the link. [1]http://lists.infradead.org/pipermail/linux-mtd/2018-June/081460.html
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Fri, 15 Jun 2018 13:42:12 + Yogesh Narayan Gaur wrote: > Hi Boris, > > I am still debugging the issue. > With some analysis, able to check that proper values are not being written > for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. > > In current code, value of map_addr are being assigned to these register. > map_addr = q->memmap_phy + > 2 * q->devtype_data->ahb_buf_size; > > qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); > > But instead of "q->devtype_data->ahb_buf_size" it should be flash size. No, because we're only using 2 * ->ahb_buf_size in the direct mapping for each device, and we're modifying the mapping dynamically based on the selected device. Maybe we got the logic wrong though. > For my case flash size is 0x400 and with this hard coded value I am able > to perform Write and Erase operation. > One more change, I have to do is adding the flash_size when writing the > base_address in SFAR register for case when "mem->spi->chip_select == 1" > qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); I don't want to expose the full device in the direct mapping yet (that's part of the direct-mapping API I posted here [1]). What this version of the driver does is, map only 2 time the ahb_size so that we can bypass the internal cache of the QSPI engine. > > Thus, there should be mechanism or the entry in structure where we can have > the information of the size of the connected slave device. Because that's exactly the kind of thing I'd like to avoid. What if the device is bigger than the reserved memory region? What if the sum of all devices does not fit in there? Here I tried to support all cases by just mapping the portion of memory we need.
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Fri, 15 Jun 2018 13:42:12 + Yogesh Narayan Gaur wrote: > Hi Boris, > > I am still debugging the issue. > With some analysis, able to check that proper values are not being written > for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. > > In current code, value of map_addr are being assigned to these register. > map_addr = q->memmap_phy + > 2 * q->devtype_data->ahb_buf_size; > > qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); > > But instead of "q->devtype_data->ahb_buf_size" it should be flash size. No, because we're only using 2 * ->ahb_buf_size in the direct mapping for each device, and we're modifying the mapping dynamically based on the selected device. Maybe we got the logic wrong though. > For my case flash size is 0x400 and with this hard coded value I am able > to perform Write and Erase operation. > One more change, I have to do is adding the flash_size when writing the > base_address in SFAR register for case when "mem->spi->chip_select == 1" > qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); I don't want to expose the full device in the direct mapping yet (that's part of the direct-mapping API I posted here [1]). What this version of the driver does is, map only 2 time the ahb_size so that we can bypass the internal cache of the QSPI engine. > > Thus, there should be mechanism or the entry in structure where we can have > the information of the size of the connected slave device. Because that's exactly the kind of thing I'd like to avoid. What if the device is bigger than the reserved memory region? What if the sum of all devices does not fit in there? Here I tried to support all cases by just mapping the portion of memory we need.
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, I am still debugging the issue. With some analysis, able to check that proper values are not being written for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. In current code, value of map_addr are being assigned to these register. map_addr = q->memmap_phy + 2 * q->devtype_data->ahb_buf_size; qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); But instead of "q->devtype_data->ahb_buf_size" it should be flash size. For my case flash size is 0x400 and with this hard coded value I am able to perform Write and Erase operation. One more change, I have to do is adding the flash_size when writing the base_address in SFAR register for case when "mem->spi->chip_select == 1" qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); Thus, there should be mechanism or the entry in structure where we can have the information of the size of the connected slave device. With both of above hardcoded changes, I am able to perform Write and Erase operation on my second flash device but still facing issue in Read operation, debugging in progress for that. -- Regards Yogesh Gaur -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Friday, June 15, 2018 6:20 PM To: Yogesh Narayan Gaur Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; Fabio Estevam ; David Wolfe ; computersforpe...@gmail.com; dw...@infradead.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Tue, 12 Jun 2018 08:51:25 + Yogesh Narayan Gaur wrote: > > I am working on lsxxx platform. With further debugging, I found that my erase > operation for second flash device is not working properly. > Need to have debugging for this in Frieder Patch. Did you find the problem? Could it be a wrong "reg = <>" definition in your DT (Frieder changed the CS numbering scheme in the new driver)?
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, I am still debugging the issue. With some analysis, able to check that proper values are not being written for QUADSPI_SFA2AD/ QUADSPI_SFB1AD/ QUADSPI_SFB2AD register. In current code, value of map_addr are being assigned to these register. map_addr = q->memmap_phy + 2 * q->devtype_data->ahb_buf_size; qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4)); But instead of "q->devtype_data->ahb_buf_size" it should be flash size. For my case flash size is 0x400 and with this hard coded value I am able to perform Write and Erase operation. One more change, I have to do is adding the flash_size when writing the base_address in SFAR register for case when "mem->spi->chip_select == 1" qspi_writel(q, q->memmap_phy + 0x400, base + QUADSPI_SFAR); Thus, there should be mechanism or the entry in structure where we can have the information of the size of the connected slave device. With both of above hardcoded changes, I am able to perform Write and Erase operation on my second flash device but still facing issue in Read operation, debugging in progress for that. -- Regards Yogesh Gaur -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Friday, June 15, 2018 6:20 PM To: Yogesh Narayan Gaur Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; Fabio Estevam ; David Wolfe ; computersforpe...@gmail.com; dw...@infradead.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Tue, 12 Jun 2018 08:51:25 + Yogesh Narayan Gaur wrote: > > I am working on lsxxx platform. With further debugging, I found that my erase > operation for second flash device is not working properly. > Need to have debugging for this in Frieder Patch. Did you find the problem? Could it be a wrong "reg = <>" definition in your DT (Frieder changed the CS numbering scheme in the new driver)?
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Tue, 12 Jun 2018 08:51:25 + Yogesh Narayan Gaur wrote: > > I am working on lsxxx platform. With further debugging, I found that my erase > operation for second flash device is not working properly. > Need to have debugging for this in Frieder Patch. Did you find the problem? Could it be a wrong "reg = <>" definition in your DT (Frieder changed the CS numbering scheme in the new driver)?
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Tue, 12 Jun 2018 08:51:25 + Yogesh Narayan Gaur wrote: > > I am working on lsxxx platform. With further debugging, I found that my erase > operation for second flash device is not working properly. > Need to have debugging for this in Frieder Patch. Did you find the problem? Could it be a wrong "reg = <>" definition in your DT (Frieder changed the CS numbering scheme in the new driver)?
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Tuesday, June 12, 2018 12:43 PM To: Yogesh Narayan Gaur Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; Fabio Estevam ; David Wolfe ; computersforpe...@gmail.com; dw...@infradead.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Tue, 12 Jun 2018 06:42:42 + Yogesh Narayan Gaur wrote: > I have tried JFFS2 mounting with smaller partition size but still getting > failure. > For partition size equal or less than 1MB, getting errors as > [ 25.044930] jffs2: Too few erase blocks (4) > Thus, need to have size more than 1MB. > > For 2MB partition size getting error message from jffs2_scan_eraseblock(). > root@ls1012ardb:~# cat /proc/mtd > dev:size erasesize name > mtd0: 0400 0004 "20c.quadspi-0" > mtd1: 0050 0004 "rcw" > mtd2: 00a0 0004 "test" > mtd3: 0020 0004 "rootfs" > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > flash_eraseall has been replaced by `flash_erase 0 0`; please > use it > Erasing 256 Kibyte @ 1c -- 100 % complete > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > [ 26.380989] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x: 0x0dd0 instead > [ 26.390509] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x004c: 0x7366 instead > [ 26.39] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x0050: 0x736c instead That's weird. Can you tell me on which platform you're testing? lsxxx or vf610? Can you dump the NOR after the erase to make sure the memory is actually erased (filled with 0xff)? I am working on lsxxx platform. With further debugging, I found that my erase operation for second flash device is not working properly. Need to have debugging for this in Frieder Patch. When I have created multiple partition for First flash device, then JFFS2 mounting and booting of Linux kernel from rootfstype=jffs2 is successful. root@ls1012ardb:~# cat /proc/mtd dev:size erasesize name mtd0: 0050 0004 "rcw" mtd1: 00a0 0004 "test" mtd2: 02e0 0004 "rootfs" mtd3: 0400 0004 "20c.quadspi-1" In above list, for MTD2 partition, able to perform JFFS2 mounting. Below is logs of erase for both flashes: root@ls1012ardb:~# cat /proc/mtd dev:size erasesize name mtd0: 0400 0004 "20c.quadspi-0" mtd1: 0400 0004 "20c.quadspi-1" root@ls1012ardb:~# mtd_debug erase /dev/mtd0 0x100 0x200 Erased 33554432 bytes from address 0x0100 in flash root@ls1012ardb:~# root@ls1012ardb:~# mtd_debug read /dev/mtd0 0x100 0xa0 rp Copied 10485760 bytes from address 0x0100 in flash to rp root@ls1012ardb:~# hexdump rp 000 * 0a0 root@ls1012ardb:~# root@ls1012ardb:~# mtd_debug erase /dev/mtd1 0x100 0x200 [ 25.023027] random: crng init done Erased 33554432 bytes from address 0x0100 in flash root@ls1012ardb:~# mtd_debug read /dev/mtd1 0x100 0xa0 rp Copied 10485760 bytes from address 0x0100 in flash to rp root@ls1012ardb:~# root@ls1012ardb:~# hexdump rp 000 1985 2003 000c b0b1 e41e 010 * 004 1985 2003 000c b0b1 e41e 0040010 -- Yogesh Gaur
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Tuesday, June 12, 2018 12:43 PM To: Yogesh Narayan Gaur Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; Fabio Estevam ; David Wolfe ; computersforpe...@gmail.com; dw...@infradead.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Tue, 12 Jun 2018 06:42:42 + Yogesh Narayan Gaur wrote: > I have tried JFFS2 mounting with smaller partition size but still getting > failure. > For partition size equal or less than 1MB, getting errors as > [ 25.044930] jffs2: Too few erase blocks (4) > Thus, need to have size more than 1MB. > > For 2MB partition size getting error message from jffs2_scan_eraseblock(). > root@ls1012ardb:~# cat /proc/mtd > dev:size erasesize name > mtd0: 0400 0004 "20c.quadspi-0" > mtd1: 0050 0004 "rcw" > mtd2: 00a0 0004 "test" > mtd3: 0020 0004 "rootfs" > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > flash_eraseall has been replaced by `flash_erase 0 0`; please > use it > Erasing 256 Kibyte @ 1c -- 100 % complete > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > [ 26.380989] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x: 0x0dd0 instead > [ 26.390509] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x004c: 0x7366 instead > [ 26.39] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x0050: 0x736c instead That's weird. Can you tell me on which platform you're testing? lsxxx or vf610? Can you dump the NOR after the erase to make sure the memory is actually erased (filled with 0xff)? I am working on lsxxx platform. With further debugging, I found that my erase operation for second flash device is not working properly. Need to have debugging for this in Frieder Patch. When I have created multiple partition for First flash device, then JFFS2 mounting and booting of Linux kernel from rootfstype=jffs2 is successful. root@ls1012ardb:~# cat /proc/mtd dev:size erasesize name mtd0: 0050 0004 "rcw" mtd1: 00a0 0004 "test" mtd2: 02e0 0004 "rootfs" mtd3: 0400 0004 "20c.quadspi-1" In above list, for MTD2 partition, able to perform JFFS2 mounting. Below is logs of erase for both flashes: root@ls1012ardb:~# cat /proc/mtd dev:size erasesize name mtd0: 0400 0004 "20c.quadspi-0" mtd1: 0400 0004 "20c.quadspi-1" root@ls1012ardb:~# mtd_debug erase /dev/mtd0 0x100 0x200 Erased 33554432 bytes from address 0x0100 in flash root@ls1012ardb:~# root@ls1012ardb:~# mtd_debug read /dev/mtd0 0x100 0xa0 rp Copied 10485760 bytes from address 0x0100 in flash to rp root@ls1012ardb:~# hexdump rp 000 * 0a0 root@ls1012ardb:~# root@ls1012ardb:~# mtd_debug erase /dev/mtd1 0x100 0x200 [ 25.023027] random: crng init done Erased 33554432 bytes from address 0x0100 in flash root@ls1012ardb:~# mtd_debug read /dev/mtd1 0x100 0xa0 rp Copied 10485760 bytes from address 0x0100 in flash to rp root@ls1012ardb:~# root@ls1012ardb:~# hexdump rp 000 1985 2003 000c b0b1 e41e 010 * 004 1985 2003 000c b0b1 e41e 0040010 -- Yogesh Gaur
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Tue, 12 Jun 2018 06:42:42 + Yogesh Narayan Gaur wrote: > I have tried JFFS2 mounting with smaller partition size but still getting > failure. > For partition size equal or less than 1MB, getting errors as > [ 25.044930] jffs2: Too few erase blocks (4) > Thus, need to have size more than 1MB. > > For 2MB partition size getting error message from jffs2_scan_eraseblock(). > root@ls1012ardb:~# cat /proc/mtd > dev:size erasesize name > mtd0: 0400 0004 "20c.quadspi-0" > mtd1: 0050 0004 "rcw" > mtd2: 00a0 0004 "test" > mtd3: 0020 0004 "rootfs" > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > flash_eraseall has been replaced by `flash_erase 0 0`; please > use it > Erasing 256 Kibyte @ 1c -- 100 % complete > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > [ 26.380989] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x: 0x0dd0 instead > [ 26.390509] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x004c: 0x7366 instead > [ 26.39] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x0050: 0x736c instead That's weird. Can you tell me on which platform you're testing? lsxxx or vf610? Can you dump the NOR after the erase to make sure the memory is actually erased (filled with 0xff)?
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Tue, 12 Jun 2018 06:42:42 + Yogesh Narayan Gaur wrote: > I have tried JFFS2 mounting with smaller partition size but still getting > failure. > For partition size equal or less than 1MB, getting errors as > [ 25.044930] jffs2: Too few erase blocks (4) > Thus, need to have size more than 1MB. > > For 2MB partition size getting error message from jffs2_scan_eraseblock(). > root@ls1012ardb:~# cat /proc/mtd > dev:size erasesize name > mtd0: 0400 0004 "20c.quadspi-0" > mtd1: 0050 0004 "rcw" > mtd2: 00a0 0004 "test" > mtd3: 0020 0004 "rootfs" > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > flash_eraseall has been replaced by `flash_erase 0 0`; please > use it > Erasing 256 Kibyte @ 1c -- 100 % complete > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > [ 26.380989] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x: 0x0dd0 instead > [ 26.390509] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x004c: 0x7366 instead > [ 26.39] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x0050: 0x736c instead That's weird. Can you tell me on which platform you're testing? lsxxx or vf610? Can you dump the NOR after the erase to make sure the memory is actually erased (filled with 0xff)?
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: linux-mtd [mailto:linux-mtd-boun...@lists.infradead.org] On Behalf Of Yogesh Narayan Gaur Sent: Monday, June 11, 2018 3:51 PM To: Boris Brezillon Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; Fabio Estevam ; David Wolfe ; computersforpe...@gmail.com; dw...@infradead.org Subject: RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Monday, June 11, 2018 3:46 PM To: Yogesh Narayan Gaur Cc: marek.va...@gmail.com; Frieder Schrempf ; linux-...@lists.infradead.org; linux-...@vger.kernel.org; dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Mon, 11 Jun 2018 09:38:14 + Yogesh Narayan Gaur wrote: > > > Observation 3: > > > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > > > commands should work fine on NOR flash. > > > But with this driver change my mount command is not working. > > > > > > In my target there are 2 flash slave devices connected, and I have given > > > argument to create MTD partition like > > > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > > > Below is output for /proc/mtd commands > > > root@ls1012ardb:~# cat /proc/mtd > > > dev:size erasesize name > > > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > > > mtd1: 0050 0004 "rcw" --> > > > Second 64 MB flash device, 3 MTD partition are created for it. > > > mtd2: 00a0 0004 "test" > > > mtd3: 02e0 0004 "rootfs" When I do mtd1 + mtd2 + mtd3, I end up with 0x3d0 instead of 0x400. Is that normal? Do you reserve a bit of space at the end or is it that rcw is not starting at 0? I have given partition size n bootargs as mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) 5 + 10 + 46 ==> 61M i.e. 0x3d0. I have just reserve the bit at the end, we can modify these settings also. > > > > > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > > > flash_eraseall has been replaced by `flash_erase 0 0`; > > > please use it > > > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng > > > init done > > > Erasing 256 Kibyte @ 2dc -- 100 % complete > > > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > > > > > This command didn't finish successfully and there are lot of messages > > > coming on console mentioning failure in jffs2_scan_eraseblock() > > > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask > > > 0x1985 not found at 0x013c: 0x2886 instead >> Did you try to create a smaller partition? Maybe we have a problem when >> accessing addresses higher than X with the new driver (X to be determined). > Would try and update you. I have tried JFFS2 mounting with smaller partition size but still getting failure. For partition size equal or less than 1MB, getting errors as [ 25.044930] jffs2: Too few erase blocks (4) Thus, need to have size more than 1MB. For 2MB partition size getting error message from jffs2_scan_eraseblock(). root@ls1012ardb:~# cat /proc/mtd dev:size erasesize name mtd0: 0400 0004 "20c.quadspi-0" mtd1: 0050 0004 "rcw" mtd2: 00a0 0004 "test" mtd3: 0020 0004 "rootfs" root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 flash_eraseall has been replaced by `flash_erase 0 0`; please use it Erasing 256 Kibyte @ 1c -- 100 % complete root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ [ 26.380989] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x: 0x0dd0 instead [ 26.390509] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x004c: 0x7366 instead [ 26.39] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x0050: 0x736c instead -- Regards Yogesh Gaur > > > [ 187.128159] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 > > > not found at 0x013c0004: 0x7a3
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: linux-mtd [mailto:linux-mtd-boun...@lists.infradead.org] On Behalf Of Yogesh Narayan Gaur Sent: Monday, June 11, 2018 3:51 PM To: Boris Brezillon Cc: rich...@nod.at; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; marek.va...@gmail.com; Frieder Schrempf ; broo...@kernel.org; linux-...@lists.infradead.org; miquel.ray...@bootlin.com; Fabio Estevam ; David Wolfe ; computersforpe...@gmail.com; dw...@infradead.org Subject: RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Monday, June 11, 2018 3:46 PM To: Yogesh Narayan Gaur Cc: marek.va...@gmail.com; Frieder Schrempf ; linux-...@lists.infradead.org; linux-...@vger.kernel.org; dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Mon, 11 Jun 2018 09:38:14 + Yogesh Narayan Gaur wrote: > > > Observation 3: > > > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > > > commands should work fine on NOR flash. > > > But with this driver change my mount command is not working. > > > > > > In my target there are 2 flash slave devices connected, and I have given > > > argument to create MTD partition like > > > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > > > Below is output for /proc/mtd commands > > > root@ls1012ardb:~# cat /proc/mtd > > > dev:size erasesize name > > > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > > > mtd1: 0050 0004 "rcw" --> > > > Second 64 MB flash device, 3 MTD partition are created for it. > > > mtd2: 00a0 0004 "test" > > > mtd3: 02e0 0004 "rootfs" When I do mtd1 + mtd2 + mtd3, I end up with 0x3d0 instead of 0x400. Is that normal? Do you reserve a bit of space at the end or is it that rcw is not starting at 0? I have given partition size n bootargs as mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) 5 + 10 + 46 ==> 61M i.e. 0x3d0. I have just reserve the bit at the end, we can modify these settings also. > > > > > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > > > flash_eraseall has been replaced by `flash_erase 0 0`; > > > please use it > > > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng > > > init done > > > Erasing 256 Kibyte @ 2dc -- 100 % complete > > > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > > > > > This command didn't finish successfully and there are lot of messages > > > coming on console mentioning failure in jffs2_scan_eraseblock() > > > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask > > > 0x1985 not found at 0x013c: 0x2886 instead >> Did you try to create a smaller partition? Maybe we have a problem when >> accessing addresses higher than X with the new driver (X to be determined). > Would try and update you. I have tried JFFS2 mounting with smaller partition size but still getting failure. For partition size equal or less than 1MB, getting errors as [ 25.044930] jffs2: Too few erase blocks (4) Thus, need to have size more than 1MB. For 2MB partition size getting error message from jffs2_scan_eraseblock(). root@ls1012ardb:~# cat /proc/mtd dev:size erasesize name mtd0: 0400 0004 "20c.quadspi-0" mtd1: 0050 0004 "rcw" mtd2: 00a0 0004 "test" mtd3: 0020 0004 "rootfs" root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 flash_eraseall has been replaced by `flash_erase 0 0`; please use it Erasing 256 Kibyte @ 1c -- 100 % complete root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ [ 26.380989] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x: 0x0dd0 instead [ 26.390509] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x004c: 0x7366 instead [ 26.39] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x0050: 0x736c instead -- Regards Yogesh Gaur > > > [ 187.128159] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 > > > not found at 0x013c0004: 0x7a3
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Monday, June 11, 2018 3:46 PM To: Yogesh Narayan Gaur Cc: marek.va...@gmail.com; Frieder Schrempf ; linux-...@lists.infradead.org; linux-...@vger.kernel.org; dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Mon, 11 Jun 2018 09:38:14 + Yogesh Narayan Gaur wrote: > > > Observation 3: > > > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > > > commands should work fine on NOR flash. > > > But with this driver change my mount command is not working. > > > > > > In my target there are 2 flash slave devices connected, and I have given > > > argument to create MTD partition like > > > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > > > Below is output for /proc/mtd commands > > > root@ls1012ardb:~# cat /proc/mtd > > > dev:size erasesize name > > > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > > > mtd1: 0050 0004 "rcw" --> > > > Second 64 MB flash device, 3 MTD partition are created for it. > > > mtd2: 00a0 0004 "test" > > > mtd3: 02e0 0004 "rootfs" When I do mtd1 + mtd2 + mtd3, I end up with 0x3d0 instead of 0x400. Is that normal? Do you reserve a bit of space at the end or is it that rcw is not starting at 0? I have given partition size n bootargs as mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) 5 + 10 + 46 ==> 61M i.e. 0x3d0. I have just reserve the bit at the end, we can modify these settings also. > > > > > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > > > flash_eraseall has been replaced by `flash_erase 0 0`; > > > please use it > > > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng > > > init done > > > Erasing 256 Kibyte @ 2dc -- 100 % complete > > > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > > > > > This command didn't finish successfully and there are lot of messages > > > coming on console mentioning failure in jffs2_scan_eraseblock() > > > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask > > > 0x1985 not found at 0x013c: 0x2886 instead Did you try to create a smaller partition? Maybe we have a problem when accessing addresses higher than X with the new driver (X to be determined). Would try and update you. -- Regards Yogesh Gaur > > > [ 187.128159] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 > > > not found at 0x013c0004: 0x7a3b instead > > > [ 187.137641] jffs2: jffs2_scan_eraseblock(): Magic bitmask > > > 0x1985 not found at 0x013c0008: 0xb10f instead > > >
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Monday, June 11, 2018 3:46 PM To: Yogesh Narayan Gaur Cc: marek.va...@gmail.com; Frieder Schrempf ; linux-...@lists.infradead.org; linux-...@vger.kernel.org; dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller On Mon, 11 Jun 2018 09:38:14 + Yogesh Narayan Gaur wrote: > > > Observation 3: > > > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > > > commands should work fine on NOR flash. > > > But with this driver change my mount command is not working. > > > > > > In my target there are 2 flash slave devices connected, and I have given > > > argument to create MTD partition like > > > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > > > Below is output for /proc/mtd commands > > > root@ls1012ardb:~# cat /proc/mtd > > > dev:size erasesize name > > > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > > > mtd1: 0050 0004 "rcw" --> > > > Second 64 MB flash device, 3 MTD partition are created for it. > > > mtd2: 00a0 0004 "test" > > > mtd3: 02e0 0004 "rootfs" When I do mtd1 + mtd2 + mtd3, I end up with 0x3d0 instead of 0x400. Is that normal? Do you reserve a bit of space at the end or is it that rcw is not starting at 0? I have given partition size n bootargs as mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) 5 + 10 + 46 ==> 61M i.e. 0x3d0. I have just reserve the bit at the end, we can modify these settings also. > > > > > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > > > flash_eraseall has been replaced by `flash_erase 0 0`; > > > please use it > > > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng > > > init done > > > Erasing 256 Kibyte @ 2dc -- 100 % complete > > > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > > > > > This command didn't finish successfully and there are lot of messages > > > coming on console mentioning failure in jffs2_scan_eraseblock() > > > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask > > > 0x1985 not found at 0x013c: 0x2886 instead Did you try to create a smaller partition? Maybe we have a problem when accessing addresses higher than X with the new driver (X to be determined). Would try and update you. -- Regards Yogesh Gaur > > > [ 187.128159] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 > > > not found at 0x013c0004: 0x7a3b instead > > > [ 187.137641] jffs2: jffs2_scan_eraseblock(): Magic bitmask > > > 0x1985 not found at 0x013c0008: 0xb10f instead > > >
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Mon, 11 Jun 2018 09:38:14 + Yogesh Narayan Gaur wrote: > > > Observation 3: > > > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > > > commands should work fine on NOR flash. > > > But with this driver change my mount command is not working. > > > > > > In my target there are 2 flash slave devices connected, and I have given > > > argument to create MTD partition like > > > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > > > Below is output for /proc/mtd commands > > > root@ls1012ardb:~# cat /proc/mtd > > > dev:size erasesize name > > > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > > > mtd1: 0050 0004 "rcw" --> > > > Second 64 MB flash device, 3 MTD partition are created for it. > > > mtd2: 00a0 0004 "test" > > > mtd3: 02e0 0004 "rootfs" When I do mtd1 + mtd2 + mtd3, I end up with 0x3d0 instead of 0x400. Is that normal? Do you reserve a bit of space at the end or is it that rcw is not starting at 0? > > > > > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > > > flash_eraseall has been replaced by `flash_erase 0 0`; > > > please use it > > > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng > > > init done > > > Erasing 256 Kibyte @ 2dc -- 100 % complete > > > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > > > > > This command didn't finish successfully and there are lot of messages > > > coming on console mentioning failure in jffs2_scan_eraseblock() > > > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 > > > not found at 0x013c: 0x2886 instead Did you try to create a smaller partition? Maybe we have a problem when accessing addresses higher than X with the new driver (X to be determined). > > > [ 187.128159] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 > > > not found at 0x013c0004: 0x7a3b instead > > > [ 187.137641] jffs2: jffs2_scan_eraseblock(): Magic bitmask > > > 0x1985 not found at 0x013c0008: 0xb10f instead > > >
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Mon, 11 Jun 2018 09:38:14 + Yogesh Narayan Gaur wrote: > > > Observation 3: > > > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > > > commands should work fine on NOR flash. > > > But with this driver change my mount command is not working. > > > > > > In my target there are 2 flash slave devices connected, and I have given > > > argument to create MTD partition like > > > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > > > Below is output for /proc/mtd commands > > > root@ls1012ardb:~# cat /proc/mtd > > > dev:size erasesize name > > > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > > > mtd1: 0050 0004 "rcw" --> > > > Second 64 MB flash device, 3 MTD partition are created for it. > > > mtd2: 00a0 0004 "test" > > > mtd3: 02e0 0004 "rootfs" When I do mtd1 + mtd2 + mtd3, I end up with 0x3d0 instead of 0x400. Is that normal? Do you reserve a bit of space at the end or is it that rcw is not starting at 0? > > > > > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > > > flash_eraseall has been replaced by `flash_erase 0 0`; > > > please use it > > > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng > > > init done > > > Erasing 256 Kibyte @ 2dc -- 100 % complete > > > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > > > > > This command didn't finish successfully and there are lot of messages > > > coming on console mentioning failure in jffs2_scan_eraseblock() > > > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 > > > not found at 0x013c: 0x2886 instead Did you try to create a smaller partition? Maybe we have a problem when accessing addresses higher than X with the new driver (X to be determined). > > > [ 187.128159] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 > > > not found at 0x013c0004: 0x7a3b instead > > > [ 187.137641] jffs2: jffs2_scan_eraseblock(): Magic bitmask > > > 0x1985 not found at 0x013c0008: 0xb10f instead > > >
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Monday, June 11, 2018 1:16 PM To: Yogesh Narayan Gaur ; marek.va...@gmail.com Cc: Frieder Schrempf ; linux-...@lists.infradead.org; linux-...@vger.kernel.org; dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Hi Yogesh, On Mon, 11 Jun 2018 06:31:00 + Yogesh Narayan Gaur wrote: > > > > > Observation 2: > > I have observed data sanity issue after performing read/write > > operations using MTD interface. Explained below > > > > root:~# mtd_debug erase /dev/mtd0 0x100 0x4 > > Erased 262144 bytes from address 0x0100 in flash > > --> Erase at address 0x100 of erase size 0x4 > > root:~# mtd_debug read /dev/mtd0 0x0 0x100 rp > > Copied 256 bytes from address 0x in flash to rp > > --> Read 0x100 bytes from flash from address 0x0 in file rp > > root:~# mtd_debug write /dev/mtd0 0x100 0x100 rp > > Copied 256 bytes from rp to address 0x0100 in flash > > --> Write 0x100 bytes to flash address 0x100 from file rp > > root:~# mtd_debug read /dev/mtd0 0x100 0x100 wp > > Copied 256 bytes from address 0x0100 in flash to wp > > --> Read 0x100 bytes from flash from address 0x100 in file wp > > root:~# diff rp wp > > --> compare both rp and wp files, if they > > are different output comes on console stating file are different > > Files rp and wp differ > > root:~# hexdump wp > > 000 aa55 aa55 8010 541c 4000 0040 > > 010 000a > > 020 0030 11a0 00a0 2580 > > 030 0040 005b > > 040 > > * > > 100 > > root:~# hexdump rp > > 000 aa55 aa55 8010 541c 4000 0040 > > 010 000a > > 020 0030 11a0 00a0 2580 > > 030 0040 005b > > 040 2403 > > 050 > > * > > 070 0011 09e7 4411 9555 0050 > > 080 f9bc afa1 0404 31e0 > > 090 0400 31e0 2010 08dc 31eb > > 0a0 2880 0050 1300 31eb 4e20 8010 80ff > > 0b0 beef dead beef dead beef dead > > 0c0 beef dead beef dead beef dead beef dead > > * > > 100 > > root:~# > > > > In hexdump output of the file which being read from address 0x100,wp, > > it can be observed that only first 64 bytes (0x40) are written on the flash. > > > > Observation 3: > > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > > commands should work fine on NOR flash. > > But with this driver change my mount command is not working. > > > > In my target there are 2 flash slave devices connected, and I have given > > argument to create MTD partition like > > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > > Below is output for /proc/mtd commands > > root@ls1012ardb:~# cat /proc/mtd > > dev:size erasesize name > > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > > mtd1: 0050 0004 "rcw" --> Second > > 64 MB flash device, 3 MTD partition are created for it. > > mtd2: 00a0 0004 "test" > > mtd3: 02e0 0004 "rootfs" > > > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > > flash_eraseall has been replaced by `flash_erase 0 0`; please > > use it > > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng > > init done > > Erasing 256 Kibyte @ 2dc -- 100 % complete > > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > > > This command didn't finish successfully and there are lot of messages > > coming on console mentioning failure in jffs2_scan_eraseblock() > > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > > found at 0x013c: 0x2886
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Monday, June 11, 2018 1:16 PM To: Yogesh Narayan Gaur ; marek.va...@gmail.com Cc: Frieder Schrempf ; linux-...@lists.infradead.org; linux-...@vger.kernel.org; dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Hi Yogesh, On Mon, 11 Jun 2018 06:31:00 + Yogesh Narayan Gaur wrote: > > > > > Observation 2: > > I have observed data sanity issue after performing read/write > > operations using MTD interface. Explained below > > > > root:~# mtd_debug erase /dev/mtd0 0x100 0x4 > > Erased 262144 bytes from address 0x0100 in flash > > --> Erase at address 0x100 of erase size 0x4 > > root:~# mtd_debug read /dev/mtd0 0x0 0x100 rp > > Copied 256 bytes from address 0x in flash to rp > > --> Read 0x100 bytes from flash from address 0x0 in file rp > > root:~# mtd_debug write /dev/mtd0 0x100 0x100 rp > > Copied 256 bytes from rp to address 0x0100 in flash > > --> Write 0x100 bytes to flash address 0x100 from file rp > > root:~# mtd_debug read /dev/mtd0 0x100 0x100 wp > > Copied 256 bytes from address 0x0100 in flash to wp > > --> Read 0x100 bytes from flash from address 0x100 in file wp > > root:~# diff rp wp > > --> compare both rp and wp files, if they > > are different output comes on console stating file are different > > Files rp and wp differ > > root:~# hexdump wp > > 000 aa55 aa55 8010 541c 4000 0040 > > 010 000a > > 020 0030 11a0 00a0 2580 > > 030 0040 005b > > 040 > > * > > 100 > > root:~# hexdump rp > > 000 aa55 aa55 8010 541c 4000 0040 > > 010 000a > > 020 0030 11a0 00a0 2580 > > 030 0040 005b > > 040 2403 > > 050 > > * > > 070 0011 09e7 4411 9555 0050 > > 080 f9bc afa1 0404 31e0 > > 090 0400 31e0 2010 08dc 31eb > > 0a0 2880 0050 1300 31eb 4e20 8010 80ff > > 0b0 beef dead beef dead beef dead > > 0c0 beef dead beef dead beef dead beef dead > > * > > 100 > > root:~# > > > > In hexdump output of the file which being read from address 0x100,wp, > > it can be observed that only first 64 bytes (0x40) are written on the flash. > > > > Observation 3: > > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > > commands should work fine on NOR flash. > > But with this driver change my mount command is not working. > > > > In my target there are 2 flash slave devices connected, and I have given > > argument to create MTD partition like > > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > > Below is output for /proc/mtd commands > > root@ls1012ardb:~# cat /proc/mtd > > dev:size erasesize name > > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > > mtd1: 0050 0004 "rcw" --> Second > > 64 MB flash device, 3 MTD partition are created for it. > > mtd2: 00a0 0004 "test" > > mtd3: 02e0 0004 "rootfs" > > > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > > flash_eraseall has been replaced by `flash_erase 0 0`; please > > use it > > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng > > init done > > Erasing 256 Kibyte @ 2dc -- 100 % complete > > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > > > This command didn't finish successfully and there are lot of messages > > coming on console mentioning failure in jffs2_scan_eraseblock() > > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > > found at 0x013c: 0x2886
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Yogesh, On Mon, 11 Jun 2018 06:31:00 + Yogesh Narayan Gaur wrote: > > > > > Observation 2: > > I have observed data sanity issue after performing read/write > > operations using MTD interface. Explained below > > > > root:~# mtd_debug erase /dev/mtd0 0x100 0x4 > > Erased 262144 bytes from address 0x0100 in flash > > --> Erase at address 0x100 of erase size 0x4 > > root:~# mtd_debug read /dev/mtd0 0x0 0x100 rp > > Copied 256 bytes from address 0x in flash to rp > > --> Read 0x100 bytes from flash from address 0x0 in file rp > > root:~# mtd_debug write /dev/mtd0 0x100 0x100 rp > > Copied 256 bytes from rp to address 0x0100 in flash > > --> Write 0x100 bytes to flash address 0x100 from file rp > > root:~# mtd_debug read /dev/mtd0 0x100 0x100 wp > > Copied 256 bytes from address 0x0100 in flash to wp > > --> Read 0x100 bytes from flash from address 0x100 in file wp > > root:~# diff rp wp > > --> compare both rp and wp files, if they > > are different output comes on console stating file are different > > Files rp and wp differ > > root:~# hexdump wp > > 000 aa55 aa55 8010 541c 4000 0040 > > 010 000a > > 020 0030 11a0 00a0 2580 > > 030 0040 005b > > 040 > > * > > 100 > > root:~# hexdump rp > > 000 aa55 aa55 8010 541c 4000 0040 > > 010 000a > > 020 0030 11a0 00a0 2580 > > 030 0040 005b > > 040 2403 > > 050 > > * > > 070 0011 09e7 4411 9555 0050 > > 080 f9bc afa1 0404 31e0 > > 090 0400 31e0 2010 08dc 31eb > > 0a0 2880 0050 1300 31eb 4e20 8010 80ff > > 0b0 beef dead beef dead beef dead > > 0c0 beef dead beef dead beef dead beef dead > > * > > 100 > > root:~# > > > > In hexdump output of the file which being read from address 0x100,wp, > > it can be observed that only first 64 bytes (0x40) are written on the flash. > > > > Observation 3: > > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > > commands should work fine on NOR flash. > > But with this driver change my mount command is not working. > > > > In my target there are 2 flash slave devices connected, and I have given > > argument to create MTD partition like > > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > > Below is output for /proc/mtd commands > > root@ls1012ardb:~# cat /proc/mtd > > dev:size erasesize name > > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > > mtd1: 0050 0004 "rcw" --> Second > > 64 MB flash device, 3 MTD partition are created for it. > > mtd2: 00a0 0004 "test" > > mtd3: 02e0 0004 "rootfs" > > > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > > flash_eraseall has been replaced by `flash_erase 0 0`; please > > use it > > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng > > init done > > Erasing 256 Kibyte @ 2dc -- 100 % complete > > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > > > This command didn't finish successfully and there are lot of messages > > coming on console mentioning failure in jffs2_scan_eraseblock() > > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > > found at 0x013c: 0x2886 instead > > [ 187.128159] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > > found at 0x013c0004: 0x7a3b instead > > [ 187.137641] jffs2: jffs2_scan_eraseblock(): Magic bitmask > > 0x1985 not found at 0x013c0008: 0xb10f instead > > > > If I remove this patch series and check with older implementation, JFFS2 > > mounting is working fine. > > Problems 2 and 3 should definitely be fixed. That's weird because I remember > that Frieder tested the new driver with a NOR chip, maybe not with JFFS2 > though. > > For write issue, it would be happening due to the changes pushed in spi-mem > framework. Now I understand why Frieder didn't face this issue: he was testing on an imx6 which has a 512 bytes TX FIFO, while you're probably testing on a vhybrid or layerscape platform which only has a 64 bytes TX FIFO. I think it's time to accept having partial page writes. This has come up several times (last time was [1]) and it looks like the fsl quadspi driver was already doing this sort of things (well hidden in the probe path [2] :-)). Marek, any comment on that?
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Yogesh, On Mon, 11 Jun 2018 06:31:00 + Yogesh Narayan Gaur wrote: > > > > > Observation 2: > > I have observed data sanity issue after performing read/write > > operations using MTD interface. Explained below > > > > root:~# mtd_debug erase /dev/mtd0 0x100 0x4 > > Erased 262144 bytes from address 0x0100 in flash > > --> Erase at address 0x100 of erase size 0x4 > > root:~# mtd_debug read /dev/mtd0 0x0 0x100 rp > > Copied 256 bytes from address 0x in flash to rp > > --> Read 0x100 bytes from flash from address 0x0 in file rp > > root:~# mtd_debug write /dev/mtd0 0x100 0x100 rp > > Copied 256 bytes from rp to address 0x0100 in flash > > --> Write 0x100 bytes to flash address 0x100 from file rp > > root:~# mtd_debug read /dev/mtd0 0x100 0x100 wp > > Copied 256 bytes from address 0x0100 in flash to wp > > --> Read 0x100 bytes from flash from address 0x100 in file wp > > root:~# diff rp wp > > --> compare both rp and wp files, if they > > are different output comes on console stating file are different > > Files rp and wp differ > > root:~# hexdump wp > > 000 aa55 aa55 8010 541c 4000 0040 > > 010 000a > > 020 0030 11a0 00a0 2580 > > 030 0040 005b > > 040 > > * > > 100 > > root:~# hexdump rp > > 000 aa55 aa55 8010 541c 4000 0040 > > 010 000a > > 020 0030 11a0 00a0 2580 > > 030 0040 005b > > 040 2403 > > 050 > > * > > 070 0011 09e7 4411 9555 0050 > > 080 f9bc afa1 0404 31e0 > > 090 0400 31e0 2010 08dc 31eb > > 0a0 2880 0050 1300 31eb 4e20 8010 80ff > > 0b0 beef dead beef dead beef dead > > 0c0 beef dead beef dead beef dead beef dead > > * > > 100 > > root:~# > > > > In hexdump output of the file which being read from address 0x100,wp, > > it can be observed that only first 64 bytes (0x40) are written on the flash. > > > > Observation 3: > > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > > commands should work fine on NOR flash. > > But with this driver change my mount command is not working. > > > > In my target there are 2 flash slave devices connected, and I have given > > argument to create MTD partition like > > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > > Below is output for /proc/mtd commands > > root@ls1012ardb:~# cat /proc/mtd > > dev:size erasesize name > > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > > mtd1: 0050 0004 "rcw" --> Second > > 64 MB flash device, 3 MTD partition are created for it. > > mtd2: 00a0 0004 "test" > > mtd3: 02e0 0004 "rootfs" > > > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > > flash_eraseall has been replaced by `flash_erase 0 0`; please > > use it > > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng > > init done > > Erasing 256 Kibyte @ 2dc -- 100 % complete > > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > > > This command didn't finish successfully and there are lot of messages > > coming on console mentioning failure in jffs2_scan_eraseblock() > > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > > found at 0x013c: 0x2886 instead > > [ 187.128159] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > > found at 0x013c0004: 0x7a3b instead > > [ 187.137641] jffs2: jffs2_scan_eraseblock(): Magic bitmask > > 0x1985 not found at 0x013c0008: 0xb10f instead > > > > If I remove this patch series and check with older implementation, JFFS2 > > mounting is working fine. > > Problems 2 and 3 should definitely be fixed. That's weird because I remember > that Frieder tested the new driver with a NOR chip, maybe not with JFFS2 > though. > > For write issue, it would be happening due to the changes pushed in spi-mem > framework. Now I understand why Frieder didn't face this issue: he was testing on an imx6 which has a 512 bytes TX FIFO, while you're probably testing on a vhybrid or layerscape platform which only has a 64 bytes TX FIFO. I think it's time to accept having partial page writes. This has come up several times (last time was [1]) and it looks like the fsl quadspi driver was already doing this sort of things (well hidden in the probe path [2] :-)). Marek, any comment on that?
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Friday, June 8, 2018 6:22 PM To: Yogesh Narayan Gaur Cc: Frieder Schrempf ; linux-...@lists.infradead.org; linux-...@vger.kernel.org; dw...@infradead.org; computersforpe...@gmail.com; marek.va...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Hi Yogesh, On Fri, 8 Jun 2018 11:54:12 + Yogesh Narayan Gaur wrote: > Hi Frieder, > > I have tried to validate your patch on fsl,ls2080a target having 2 Spansion > NOR flash, S25FS512S, as slave device. > Below are my observations: > > Observation 1: > In Linux boot logs after driver probing is successful, getting below log > messages > [1.435986] m25p80 spi0.0: found s25fl512s, expected m25p80 > [1.441564] m25p80 spi0.0: s25fl512s (65536 Kbytes) > [1.446972] m25p80 spi0.1: found s25fl512s, expected m25p80 > [1.452548] m25p80 spi0.1: s25fl512s (65536 Kbytes) > > IMHO, we need to correct message as 'found s25fl512s, expected m25p80' as > final underlying connected flash device is s25fl512s. Not sure what you mean here. What would you like us to fix exactly? > > Observation 2: > I have observed data sanity issue after performing read/write > operations using MTD interface. Explained below > > root:~# mtd_debug erase /dev/mtd0 0x100 0x4 > Erased 262144 bytes from address 0x0100 in flash --> > Erase at address 0x100 of erase size 0x4 > root:~# mtd_debug read /dev/mtd0 0x0 0x100 rp > Copied 256 bytes from address 0x in flash to rp --> > Read 0x100 bytes from flash from address 0x0 in file rp > root:~# mtd_debug write /dev/mtd0 0x100 0x100 rp > Copied 256 bytes from rp to address 0x0100 in flash --> > Write 0x100 bytes to flash address 0x100 from file rp > root:~# mtd_debug read /dev/mtd0 0x100 0x100 wp > Copied 256 bytes from address 0x0100 in flash to wp --> > Read 0x100 bytes from flash from address 0x100 in file wp > root:~# diff rp wp >--> compare both rp and wp files, if they are > different output comes on console stating file are different > Files rp and wp differ > root:~# hexdump wp > 000 aa55 aa55 8010 541c 4000 0040 > 010 000a > 020 0030 11a0 00a0 2580 > 030 0040 005b > 040 > * > 100 > root:~# hexdump rp > 000 aa55 aa55 8010 541c 4000 0040 > 010 000a > 020 0030 11a0 00a0 2580 > 030 0040 005b > 040 2403 > 050 > * > 070 0011 09e7 4411 9555 0050 > 080 f9bc afa1 0404 31e0 > 090 0400 31e0 2010 08dc 31eb > 0a0 2880 0050 1300 31eb 4e20 8010 80ff > 0b0 beef dead beef dead beef dead > 0c0 beef dead beef dead beef dead beef dead > * > 100 > root:~# > > In hexdump output of the file which being read from address 0x100,wp, it > can be observed that only first 64 bytes (0x40) are written on the flash. > > Observation 3: > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > commands should work fine on NOR flash. > But with this driver change my mount command is not working. > > In my target there are 2 flash slave devices connected, and I have given > argument to create MTD partition like > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > Below is output for /proc/mtd commands > root@ls1012ardb:~# cat /proc/mtd > dev:size erasesize name > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > mtd1: 0050 0004 "rcw" --> Second 64 > MB flash device, 3 MTD partition are created for it. > mtd2: 00a0 0004 "test" > mtd3: 02e0 0004 "rootfs" > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > flash_eraseall has been replaced by `flash_erase 0 0`; please > use it > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng init > done > Eras
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris, -Original Message- From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] Sent: Friday, June 8, 2018 6:22 PM To: Yogesh Narayan Gaur Cc: Frieder Schrempf ; linux-...@lists.infradead.org; linux-...@vger.kernel.org; dw...@infradead.org; computersforpe...@gmail.com; marek.va...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Han Xu ; linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller Hi Yogesh, On Fri, 8 Jun 2018 11:54:12 + Yogesh Narayan Gaur wrote: > Hi Frieder, > > I have tried to validate your patch on fsl,ls2080a target having 2 Spansion > NOR flash, S25FS512S, as slave device. > Below are my observations: > > Observation 1: > In Linux boot logs after driver probing is successful, getting below log > messages > [1.435986] m25p80 spi0.0: found s25fl512s, expected m25p80 > [1.441564] m25p80 spi0.0: s25fl512s (65536 Kbytes) > [1.446972] m25p80 spi0.1: found s25fl512s, expected m25p80 > [1.452548] m25p80 spi0.1: s25fl512s (65536 Kbytes) > > IMHO, we need to correct message as 'found s25fl512s, expected m25p80' as > final underlying connected flash device is s25fl512s. Not sure what you mean here. What would you like us to fix exactly? > > Observation 2: > I have observed data sanity issue after performing read/write > operations using MTD interface. Explained below > > root:~# mtd_debug erase /dev/mtd0 0x100 0x4 > Erased 262144 bytes from address 0x0100 in flash --> > Erase at address 0x100 of erase size 0x4 > root:~# mtd_debug read /dev/mtd0 0x0 0x100 rp > Copied 256 bytes from address 0x in flash to rp --> > Read 0x100 bytes from flash from address 0x0 in file rp > root:~# mtd_debug write /dev/mtd0 0x100 0x100 rp > Copied 256 bytes from rp to address 0x0100 in flash --> > Write 0x100 bytes to flash address 0x100 from file rp > root:~# mtd_debug read /dev/mtd0 0x100 0x100 wp > Copied 256 bytes from address 0x0100 in flash to wp --> > Read 0x100 bytes from flash from address 0x100 in file wp > root:~# diff rp wp >--> compare both rp and wp files, if they are > different output comes on console stating file are different > Files rp and wp differ > root:~# hexdump wp > 000 aa55 aa55 8010 541c 4000 0040 > 010 000a > 020 0030 11a0 00a0 2580 > 030 0040 005b > 040 > * > 100 > root:~# hexdump rp > 000 aa55 aa55 8010 541c 4000 0040 > 010 000a > 020 0030 11a0 00a0 2580 > 030 0040 005b > 040 2403 > 050 > * > 070 0011 09e7 4411 9555 0050 > 080 f9bc afa1 0404 31e0 > 090 0400 31e0 2010 08dc 31eb > 0a0 2880 0050 1300 31eb 4e20 8010 80ff > 0b0 beef dead beef dead beef dead > 0c0 beef dead beef dead beef dead beef dead > * > 100 > root:~# > > In hexdump output of the file which being read from address 0x100,wp, it > can be observed that only first 64 bytes (0x40) are written on the flash. > > Observation 3: > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > commands should work fine on NOR flash. > But with this driver change my mount command is not working. > > In my target there are 2 flash slave devices connected, and I have given > argument to create MTD partition like > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > Below is output for /proc/mtd commands > root@ls1012ardb:~# cat /proc/mtd > dev:size erasesize name > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > mtd1: 0050 0004 "rcw" --> Second 64 > MB flash device, 3 MTD partition are created for it. > mtd2: 00a0 0004 "test" > mtd3: 02e0 0004 "rootfs" > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > flash_eraseall has been replaced by `flash_erase 0 0`; please > use it > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng init > done > Eras
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Fri, Jun 8, 2018 at 2:54 PM, Yogesh Narayan Gaur
wrote:
Hi Frieder,
> +#define QUADSPI_MCR_RESERVED_MASK (0xF << 16)
GENMASK()
> +#define QUADSPI_MCR_END_CFG_MASK (0x3 << 2)
> +#define QUADSPI_BUF3CR_ADATSZ_MASK (0xFF << QUADSPI_BUF3CR_ADATSZ_SHIFT)
> +#define QUADSPI_SMPR_DDRSMP_MASK (7 << 16)
> +#define QUADSPI_RBCT_WMRK_MASK 0x1F
Ditto.
> +#define QUADSPI_LUT_OFFSET (SEQID_LUT * 4 * 4)
> +#define QUADSPI_LUT_REG(idx) (QUADSPI_LUT_BASE + \
> + QUADSPI_LUT_OFFSET + (idx) * 4)
It looks slightly better when
#define FOO \
(BAR1 + BAR2 ...)
> +/*
> + * An IC bug makes it necessary to rearrange the 32-bit data.
> + * Later chips, such as IMX6SLX, have fixed this bug.
> + */
> +static inline u32 fsl_qspi_endian_xchg(struct fsl_qspi *q, u32 a) {
> + return needs_swap_endian(q) ? __swab32(a) : a; }
func()
{
...
}
Fix this everywhere.
> +static void qspi_writel(struct fsl_qspi *q, u32 val, void __iomem
> +*addr) {
> + if (q->big_endian)
> + iowrite32be(val, addr);
> + else
> + iowrite32(val, addr);
> +}
> +
> +static u32 qspi_readl(struct fsl_qspi *q, void __iomem *addr) {
> + if (q->big_endian)
> + return ioread32be(addr);
> + else
> + return ioread32(addr);
> +}
Better to define ->read() and ->write() callbacks and call them unconditionally.
> +static int fsl_qspi_check_buswidth(struct fsl_qspi *q, u8 width) {
> + switch (width) {
> + case 1:
> + case 2:
> + case 4:
> + return 0;
> + }
if (!is_power_of_2(width) || width >= 8)
return -E...;
return 0;
?
> +
> + return -ENOTSUPP;
> +}
> +static int fsl_qspi_clk_prep_enable(struct fsl_qspi *q) {
> + int ret;
> +
> + ret = clk_prepare_enable(q->clk_en);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(q->clk);
> + if (ret) {
> + clk_disable_unprepare(q->clk_en);
Is it needed here?
> + return ret;
> + }
> +
> + if (needs_wakeup_wait_mode(q))
> + pm_qos_add_request(>pm_qos_req, PM_QOS_CPU_DMA_LATENCY, 0);
> +
> + return 0;
> +}
> + qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i *
> 4));
Redundant parens.
> + seq = seq ? 0 : 1;
seq = (seq + 1) % 2;
?
> +}
> + for (i = 0; i < op->data.nbytes; i += 4) {
> + u32 val = 0;
> +
> + memcpy(, op->data.buf.out + i,
> + min_t(unsigned int, op->data.nbytes - i, 4));
You may easily avoid this conditional on each iteration.
> +
> + val = fsl_qspi_endian_xchg(q, val);
> + qspi_writel(q, val, base + QUADSPI_TBDR);
> + }
> + /* wait for the controller being ready */
FOREVER! See below.
> + do {
> + u32 status;
> +
> + status = qspi_readl(q, base + QUADSPI_SR);
> + if (status &
> + (QUADSPI_SR_IP_ACC_MASK | QUADSPI_SR_AHB_ACC_MASK)) {
> + udelay(1);
> + dev_dbg(q->dev, "The controller is busy, 0x%x\n",
> + status);
> + continue;
> + }
> + break;
> + } while (1);
Please, avoid infinite loops.
unsigned int count = 100;
...
do {
...
} while (--count);
> + if (of_get_available_child_count(q->dev->of_node) == 1)
> + name = dev_name(q->dev);
> + else
> + name = devm_kasprintf(dev, GFP_KERNEL,
> + "%s-%d", dev_name(q->dev),
> + mem->spi->chip_select);
> +
> + if (!name) {
> + dev_err(dev, "failed to get memory for custom flash name\n");
> + return dev_name(q->dev);
Might it be racy if in between device gets a name assigned?
> + }
> + if (q->ahb_addr)
> + iounmap(q->ahb_addr);
Double unmap?
> +static struct platform_driver fsl_qspi_driver = {
> + .driver = {
> + .name = "fsl-quadspi",
> + .of_match_table = fsl_qspi_dt_ids,
> + },
> + .probe = fsl_qspi_probe,
> + .remove = fsl_qspi_remove,
> + .suspend= fsl_qspi_suspend,
> + .resume = fsl_qspi_resume,
Why not in struct dev_pm_ops?
> +};
> +MODULE_AUTHOR("Freescale Semiconductor Inc."); MODULE_AUTHOR("Boris
> +Brezillion "); MODULE_AUTHOR("Frieder
> +Schrempf "); MODULE_LICENSE("GPL v2");
Wrong indentation.
--
With Best Regards,
Andy Shevchenko
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Fri, Jun 8, 2018 at 2:54 PM, Yogesh Narayan Gaur
wrote:
Hi Frieder,
> +#define QUADSPI_MCR_RESERVED_MASK (0xF << 16)
GENMASK()
> +#define QUADSPI_MCR_END_CFG_MASK (0x3 << 2)
> +#define QUADSPI_BUF3CR_ADATSZ_MASK (0xFF << QUADSPI_BUF3CR_ADATSZ_SHIFT)
> +#define QUADSPI_SMPR_DDRSMP_MASK (7 << 16)
> +#define QUADSPI_RBCT_WMRK_MASK 0x1F
Ditto.
> +#define QUADSPI_LUT_OFFSET (SEQID_LUT * 4 * 4)
> +#define QUADSPI_LUT_REG(idx) (QUADSPI_LUT_BASE + \
> + QUADSPI_LUT_OFFSET + (idx) * 4)
It looks slightly better when
#define FOO \
(BAR1 + BAR2 ...)
> +/*
> + * An IC bug makes it necessary to rearrange the 32-bit data.
> + * Later chips, such as IMX6SLX, have fixed this bug.
> + */
> +static inline u32 fsl_qspi_endian_xchg(struct fsl_qspi *q, u32 a) {
> + return needs_swap_endian(q) ? __swab32(a) : a; }
func()
{
...
}
Fix this everywhere.
> +static void qspi_writel(struct fsl_qspi *q, u32 val, void __iomem
> +*addr) {
> + if (q->big_endian)
> + iowrite32be(val, addr);
> + else
> + iowrite32(val, addr);
> +}
> +
> +static u32 qspi_readl(struct fsl_qspi *q, void __iomem *addr) {
> + if (q->big_endian)
> + return ioread32be(addr);
> + else
> + return ioread32(addr);
> +}
Better to define ->read() and ->write() callbacks and call them unconditionally.
> +static int fsl_qspi_check_buswidth(struct fsl_qspi *q, u8 width) {
> + switch (width) {
> + case 1:
> + case 2:
> + case 4:
> + return 0;
> + }
if (!is_power_of_2(width) || width >= 8)
return -E...;
return 0;
?
> +
> + return -ENOTSUPP;
> +}
> +static int fsl_qspi_clk_prep_enable(struct fsl_qspi *q) {
> + int ret;
> +
> + ret = clk_prepare_enable(q->clk_en);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(q->clk);
> + if (ret) {
> + clk_disable_unprepare(q->clk_en);
Is it needed here?
> + return ret;
> + }
> +
> + if (needs_wakeup_wait_mode(q))
> + pm_qos_add_request(>pm_qos_req, PM_QOS_CPU_DMA_LATENCY, 0);
> +
> + return 0;
> +}
> + qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i *
> 4));
Redundant parens.
> + seq = seq ? 0 : 1;
seq = (seq + 1) % 2;
?
> +}
> + for (i = 0; i < op->data.nbytes; i += 4) {
> + u32 val = 0;
> +
> + memcpy(, op->data.buf.out + i,
> + min_t(unsigned int, op->data.nbytes - i, 4));
You may easily avoid this conditional on each iteration.
> +
> + val = fsl_qspi_endian_xchg(q, val);
> + qspi_writel(q, val, base + QUADSPI_TBDR);
> + }
> + /* wait for the controller being ready */
FOREVER! See below.
> + do {
> + u32 status;
> +
> + status = qspi_readl(q, base + QUADSPI_SR);
> + if (status &
> + (QUADSPI_SR_IP_ACC_MASK | QUADSPI_SR_AHB_ACC_MASK)) {
> + udelay(1);
> + dev_dbg(q->dev, "The controller is busy, 0x%x\n",
> + status);
> + continue;
> + }
> + break;
> + } while (1);
Please, avoid infinite loops.
unsigned int count = 100;
...
do {
...
} while (--count);
> + if (of_get_available_child_count(q->dev->of_node) == 1)
> + name = dev_name(q->dev);
> + else
> + name = devm_kasprintf(dev, GFP_KERNEL,
> + "%s-%d", dev_name(q->dev),
> + mem->spi->chip_select);
> +
> + if (!name) {
> + dev_err(dev, "failed to get memory for custom flash name\n");
> + return dev_name(q->dev);
Might it be racy if in between device gets a name assigned?
> + }
> + if (q->ahb_addr)
> + iounmap(q->ahb_addr);
Double unmap?
> +static struct platform_driver fsl_qspi_driver = {
> + .driver = {
> + .name = "fsl-quadspi",
> + .of_match_table = fsl_qspi_dt_ids,
> + },
> + .probe = fsl_qspi_probe,
> + .remove = fsl_qspi_remove,
> + .suspend= fsl_qspi_suspend,
> + .resume = fsl_qspi_resume,
Why not in struct dev_pm_ops?
> +};
> +MODULE_AUTHOR("Freescale Semiconductor Inc."); MODULE_AUTHOR("Boris
> +Brezillion "); MODULE_AUTHOR("Frieder
> +Schrempf "); MODULE_LICENSE("GPL v2");
Wrong indentation.
--
With Best Regards,
Andy Shevchenko
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Yogesh, On Fri, 8 Jun 2018 11:54:12 + Yogesh Narayan Gaur wrote: > Hi Frieder, > > I have tried to validate your patch on fsl,ls2080a target having 2 Spansion > NOR flash, S25FS512S, as slave device. > Below are my observations: > > Observation 1: > In Linux boot logs after driver probing is successful, getting below log > messages > [1.435986] m25p80 spi0.0: found s25fl512s, expected m25p80 > [1.441564] m25p80 spi0.0: s25fl512s (65536 Kbytes) > [1.446972] m25p80 spi0.1: found s25fl512s, expected m25p80 > [1.452548] m25p80 spi0.1: s25fl512s (65536 Kbytes) > > IMHO, we need to correct message as 'found s25fl512s, expected m25p80' as > final underlying connected flash device is s25fl512s. Not sure what you mean here. What would you like us to fix exactly? > > Observation 2: > I have observed data sanity issue after performing read/write operations > using MTD interface. Explained below > > root:~# mtd_debug erase /dev/mtd0 0x100 0x4 > Erased 262144 bytes from address 0x0100 in flash --> > Erase at address 0x100 of erase size 0x4 > root:~# mtd_debug read /dev/mtd0 0x0 0x100 rp > Copied 256 bytes from address 0x in flash to rp --> > Read 0x100 bytes from flash from address 0x0 in file rp > root:~# mtd_debug write /dev/mtd0 0x100 0x100 rp > Copied 256 bytes from rp to address 0x0100 in flash --> > Write 0x100 bytes to flash address 0x100 from file rp > root:~# mtd_debug read /dev/mtd0 0x100 0x100 wp > Copied 256 bytes from address 0x0100 in flash to wp --> > Read 0x100 bytes from flash from address 0x100 in file wp > root:~# diff rp wp >--> compare both rp and wp files, if they are > different output comes on console stating file are different > Files rp and wp differ > root:~# hexdump wp > 000 aa55 aa55 8010 541c 4000 0040 > 010 000a > 020 0030 11a0 00a0 2580 > 030 0040 005b > 040 > * > 100 > root:~# hexdump rp > 000 aa55 aa55 8010 541c 4000 0040 > 010 000a > 020 0030 11a0 00a0 2580 > 030 0040 005b > 040 2403 > 050 > * > 070 0011 09e7 4411 9555 0050 > 080 f9bc afa1 0404 31e0 > 090 0400 31e0 2010 08dc 31eb > 0a0 2880 0050 1300 31eb 4e20 8010 80ff > 0b0 beef dead beef dead beef dead > 0c0 beef dead beef dead beef dead beef dead > * > 100 > root:~# > > In hexdump output of the file which being read from address 0x100,wp, it > can be observed that only first 64 bytes (0x40) are written on the flash. > > Observation 3: > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > commands should work fine on NOR flash. > But with this driver change my mount command is not working. > > In my target there are 2 flash slave devices connected, and I have given > argument to create MTD partition like > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > Below is output for /proc/mtd commands > root@ls1012ardb:~# cat /proc/mtd > dev:size erasesize name > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > mtd1: 0050 0004 "rcw" --> Second 64 > MB flash device, 3 MTD partition are created for it. > mtd2: 00a0 0004 "test" > mtd3: 02e0 0004 "rootfs" > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > flash_eraseall has been replaced by `flash_erase 0 0`; please > use it > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng init > done > Erasing 256 Kibyte @ 2dc -- 100 % complete > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > This command didn't finish successfully and there are lot of messages coming > on console mentioning failure in jffs2_scan_eraseblock() > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x013c: 0x2886 instead > [ 187.128159] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x013c0004: 0x7a3b instead > [ 187.137641] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x013c0008: 0xb10f instead > > If I remove this patch series and check with older implementation, JFFS2 > mounting is working fine. Problems 2 and 3 should definitely be fixed. That's weird because I remember that Frieder tested the new driver with a NOR chip, maybe not with JFFS2 though. > > Observation 4: > With previous
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Yogesh, On Fri, 8 Jun 2018 11:54:12 + Yogesh Narayan Gaur wrote: > Hi Frieder, > > I have tried to validate your patch on fsl,ls2080a target having 2 Spansion > NOR flash, S25FS512S, as slave device. > Below are my observations: > > Observation 1: > In Linux boot logs after driver probing is successful, getting below log > messages > [1.435986] m25p80 spi0.0: found s25fl512s, expected m25p80 > [1.441564] m25p80 spi0.0: s25fl512s (65536 Kbytes) > [1.446972] m25p80 spi0.1: found s25fl512s, expected m25p80 > [1.452548] m25p80 spi0.1: s25fl512s (65536 Kbytes) > > IMHO, we need to correct message as 'found s25fl512s, expected m25p80' as > final underlying connected flash device is s25fl512s. Not sure what you mean here. What would you like us to fix exactly? > > Observation 2: > I have observed data sanity issue after performing read/write operations > using MTD interface. Explained below > > root:~# mtd_debug erase /dev/mtd0 0x100 0x4 > Erased 262144 bytes from address 0x0100 in flash --> > Erase at address 0x100 of erase size 0x4 > root:~# mtd_debug read /dev/mtd0 0x0 0x100 rp > Copied 256 bytes from address 0x in flash to rp --> > Read 0x100 bytes from flash from address 0x0 in file rp > root:~# mtd_debug write /dev/mtd0 0x100 0x100 rp > Copied 256 bytes from rp to address 0x0100 in flash --> > Write 0x100 bytes to flash address 0x100 from file rp > root:~# mtd_debug read /dev/mtd0 0x100 0x100 wp > Copied 256 bytes from address 0x0100 in flash to wp --> > Read 0x100 bytes from flash from address 0x100 in file wp > root:~# diff rp wp >--> compare both rp and wp files, if they are > different output comes on console stating file are different > Files rp and wp differ > root:~# hexdump wp > 000 aa55 aa55 8010 541c 4000 0040 > 010 000a > 020 0030 11a0 00a0 2580 > 030 0040 005b > 040 > * > 100 > root:~# hexdump rp > 000 aa55 aa55 8010 541c 4000 0040 > 010 000a > 020 0030 11a0 00a0 2580 > 030 0040 005b > 040 2403 > 050 > * > 070 0011 09e7 4411 9555 0050 > 080 f9bc afa1 0404 31e0 > 090 0400 31e0 2010 08dc 31eb > 0a0 2880 0050 1300 31eb 4e20 8010 80ff > 0b0 beef dead beef dead beef dead > 0c0 beef dead beef dead beef dead beef dead > * > 100 > root:~# > > In hexdump output of the file which being read from address 0x100,wp, it > can be observed that only first 64 bytes (0x40) are written on the flash. > > Observation 3: > As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 > commands should work fine on NOR flash. > But with this driver change my mount command is not working. > > In my target there are 2 flash slave devices connected, and I have given > argument to create MTD partition like > "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. > Below is output for /proc/mtd commands > root@ls1012ardb:~# cat /proc/mtd > dev:size erasesize name > mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash > mtd1: 0050 0004 "rcw" --> Second 64 > MB flash device, 3 MTD partition are created for it. > mtd2: 00a0 0004 "test" > mtd3: 02e0 0004 "rootfs" > > root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 > flash_eraseall has been replaced by `flash_erase 0 0`; please > use it > Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng init > done > Erasing 256 Kibyte @ 2dc -- 100 % complete > root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ > > This command didn't finish successfully and there are lot of messages coming > on console mentioning failure in jffs2_scan_eraseblock() > [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x013c: 0x2886 instead > [ 187.128159] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x013c0004: 0x7a3b instead > [ 187.137641] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not > found at 0x013c0008: 0xb10f instead > > If I remove this patch series and check with older implementation, JFFS2 > mounting is working fine. Problems 2 and 3 should definitely be fixed. That's weird because I remember that Frieder tested the new driver with a NOR chip, maybe not with JFFS2 though. > > Observation 4: > With previous
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Frieder, I have tried to validate your patch on fsl,ls2080a target having 2 Spansion NOR flash, S25FS512S, as slave device. Below are my observations: Observation 1: In Linux boot logs after driver probing is successful, getting below log messages [1.435986] m25p80 spi0.0: found s25fl512s, expected m25p80 [1.441564] m25p80 spi0.0: s25fl512s (65536 Kbytes) [1.446972] m25p80 spi0.1: found s25fl512s, expected m25p80 [1.452548] m25p80 spi0.1: s25fl512s (65536 Kbytes) IMHO, we need to correct message as 'found s25fl512s, expected m25p80' as final underlying connected flash device is s25fl512s. Observation 2: I have observed data sanity issue after performing read/write operations using MTD interface. Explained below root:~# mtd_debug erase /dev/mtd0 0x100 0x4 Erased 262144 bytes from address 0x0100 in flash --> Erase at address 0x100 of erase size 0x4 root:~# mtd_debug read /dev/mtd0 0x0 0x100 rp Copied 256 bytes from address 0x in flash to rp --> Read 0x100 bytes from flash from address 0x0 in file rp root:~# mtd_debug write /dev/mtd0 0x100 0x100 rp Copied 256 bytes from rp to address 0x0100 in flash --> Write 0x100 bytes to flash address 0x100 from file rp root:~# mtd_debug read /dev/mtd0 0x100 0x100 wp Copied 256 bytes from address 0x0100 in flash to wp --> Read 0x100 bytes from flash from address 0x100 in file wp root:~# diff rp wp --> compare both rp and wp files, if they are different output comes on console stating file are different Files rp and wp differ root:~# hexdump wp 000 aa55 aa55 8010 541c 4000 0040 010 000a 020 0030 11a0 00a0 2580 030 0040 005b 040 * 100 root:~# hexdump rp 000 aa55 aa55 8010 541c 4000 0040 010 000a 020 0030 11a0 00a0 2580 030 0040 005b 040 2403 050 * 070 0011 09e7 4411 9555 0050 080 f9bc afa1 0404 31e0 090 0400 31e0 2010 08dc 31eb 0a0 2880 0050 1300 31eb 4e20 8010 80ff 0b0 beef dead beef dead beef dead 0c0 beef dead beef dead beef dead beef dead * 100 root:~# In hexdump output of the file which being read from address 0x100,wp, it can be observed that only first 64 bytes (0x40) are written on the flash. Observation 3: As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 commands should work fine on NOR flash. But with this driver change my mount command is not working. In my target there are 2 flash slave devices connected, and I have given argument to create MTD partition like "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. Below is output for /proc/mtd commands root@ls1012ardb:~# cat /proc/mtd dev:size erasesize name mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash mtd1: 0050 0004 "rcw" --> Second 64 MB flash device, 3 MTD partition are created for it. mtd2: 00a0 0004 "test" mtd3: 02e0 0004 "rootfs" root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 flash_eraseall has been replaced by `flash_erase 0 0`; please use it Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng init done Erasing 256 Kibyte @ 2dc -- 100 % complete root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ This command didn't finish successfully and there are lot of messages coming on console mentioning failure in jffs2_scan_eraseblock() [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x013c: 0x2886 instead [ 187.128159] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x013c0004: 0x7a3b instead [ 187.137641] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x013c0008: 0xb10f instead If I remove this patch series and check with older implementation, JFFS2 mounting is working fine. Observation 4: With previous driver, we can read content of flash directly using devmem command Like devmem 0x2000 "Flash is connected at this Quad-SPI address" But with new driver devmem interface reporting in-correct value. Few other comments inline. -- Regards, Yogesh Gaur -Original Message- From: Frieder Schrempf [mailto:frieder.schre...@exceet.de] Sent: Wednesday, May 30, 2018 6:45 PM To: linux-...@lists.infradead.org; boris.brezil...@bootlin.com; linux-...@vger.kernel.org Cc: dw...@infradead.org;
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Frieder, I have tried to validate your patch on fsl,ls2080a target having 2 Spansion NOR flash, S25FS512S, as slave device. Below are my observations: Observation 1: In Linux boot logs after driver probing is successful, getting below log messages [1.435986] m25p80 spi0.0: found s25fl512s, expected m25p80 [1.441564] m25p80 spi0.0: s25fl512s (65536 Kbytes) [1.446972] m25p80 spi0.1: found s25fl512s, expected m25p80 [1.452548] m25p80 spi0.1: s25fl512s (65536 Kbytes) IMHO, we need to correct message as 'found s25fl512s, expected m25p80' as final underlying connected flash device is s25fl512s. Observation 2: I have observed data sanity issue after performing read/write operations using MTD interface. Explained below root:~# mtd_debug erase /dev/mtd0 0x100 0x4 Erased 262144 bytes from address 0x0100 in flash --> Erase at address 0x100 of erase size 0x4 root:~# mtd_debug read /dev/mtd0 0x0 0x100 rp Copied 256 bytes from address 0x in flash to rp --> Read 0x100 bytes from flash from address 0x0 in file rp root:~# mtd_debug write /dev/mtd0 0x100 0x100 rp Copied 256 bytes from rp to address 0x0100 in flash --> Write 0x100 bytes to flash address 0x100 from file rp root:~# mtd_debug read /dev/mtd0 0x100 0x100 wp Copied 256 bytes from address 0x0100 in flash to wp --> Read 0x100 bytes from flash from address 0x100 in file wp root:~# diff rp wp --> compare both rp and wp files, if they are different output comes on console stating file are different Files rp and wp differ root:~# hexdump wp 000 aa55 aa55 8010 541c 4000 0040 010 000a 020 0030 11a0 00a0 2580 030 0040 005b 040 * 100 root:~# hexdump rp 000 aa55 aa55 8010 541c 4000 0040 010 000a 020 0030 11a0 00a0 2580 030 0040 005b 040 2403 050 * 070 0011 09e7 4411 9555 0050 080 f9bc afa1 0404 31e0 090 0400 31e0 2010 08dc 31eb 0a0 2880 0050 1300 31eb 4e20 8010 80ff 0b0 beef dead beef dead beef dead 0c0 beef dead beef dead beef dead beef dead * 100 root:~# In hexdump output of the file which being read from address 0x100,wp, it can be observed that only first 64 bytes (0x40) are written on the flash. Observation 3: As we can support JFFS2 filesystem on NOR flash, so we can expect JFFS2 commands should work fine on NOR flash. But with this driver change my mount command is not working. In my target there are 2 flash slave devices connected, and I have given argument to create MTD partition like "mtdparts=20c.quadspi-1:5M(rcw),10M(test),46M(rootfs) " for 2nd flash. Below is output for /proc/mtd commands root@ls1012ardb:~# cat /proc/mtd dev:size erasesize name mtd0: 0400 0004 "20c.quadspi-0" --> First 64MB flash mtd1: 0050 0004 "rcw" --> Second 64 MB flash device, 3 MTD partition are created for it. mtd2: 00a0 0004 "test" mtd3: 02e0 0004 "rootfs" root@ls1012ardb:~# mkdir /media/ram ; flash_eraseall /dev/mtd3 flash_eraseall has been replaced by `flash_erase 0 0`; please use it Erasing 256 Kibyte @ 0 -- 0 % complete [ 18.299929] random: crng init done Erasing 256 Kibyte @ 2dc -- 100 % complete root@ls1012ardb:~# mount -t jffs2 /dev/mtdblock3 /media/ram/ This command didn't finish successfully and there are lot of messages coming on console mentioning failure in jffs2_scan_eraseblock() [ 187.118677] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x013c: 0x2886 instead [ 187.128159] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x013c0004: 0x7a3b instead [ 187.137641] jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found at 0x013c0008: 0xb10f instead If I remove this patch series and check with older implementation, JFFS2 mounting is working fine. Observation 4: With previous driver, we can read content of flash directly using devmem command Like devmem 0x2000 "Flash is connected at this Quad-SPI address" But with new driver devmem interface reporting in-correct value. Few other comments inline. -- Regards, Yogesh Gaur -Original Message- From: Frieder Schrempf [mailto:frieder.schre...@exceet.de] Sent: Wednesday, May 30, 2018 6:45 PM To: linux-...@lists.infradead.org; boris.brezil...@bootlin.com; linux-...@vger.kernel.org Cc: dw...@infradead.org;
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Wed, 30 May 2018 15:14:32 +0200
Frieder Schrempf wrote:
> +
> +static void fsl_qspi_read_ahb(struct fsl_qspi *q, const struct spi_mem_op
> *op)
> +{
> + static int seq;
> +
> + /*
> + * We want to avoid needing to invalidate the cache by issueing
> + * a reset to the AHB and Serial Flash domain, as this needs
> + * time. So we change the address on each read to trigger an
> + * actual read operation on the flash. The actual address for
> + * the flash memory is set by programming the LUT.
> + */
> + memcpy_fromio(op->data.buf.in,
> + q->ahb_addr +
> + (seq * q->devtype_data->ahb_buf_size),
> + op->data.nbytes);
> +
> + seq = seq ? 0 : 1;
We should get rid of this hack. Yogesh, Han, do you know if there's an
easy way to invalidate the AHB buffer without resetting the IP?
> +}
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
On Wed, 30 May 2018 15:14:32 +0200
Frieder Schrempf wrote:
> +
> +static void fsl_qspi_read_ahb(struct fsl_qspi *q, const struct spi_mem_op
> *op)
> +{
> + static int seq;
> +
> + /*
> + * We want to avoid needing to invalidate the cache by issueing
> + * a reset to the AHB and Serial Flash domain, as this needs
> + * time. So we change the address on each read to trigger an
> + * actual read operation on the flash. The actual address for
> + * the flash memory is set by programming the LUT.
> + */
> + memcpy_fromio(op->data.buf.in,
> + q->ahb_addr +
> + (seq * q->devtype_data->ahb_buf_size),
> + op->data.nbytes);
> +
> + seq = seq ? 0 : 1;
We should get rid of this hack. Yogesh, Han, do you know if there's an
easy way to invalidate the AHB buffer without resetting the IP?
> +}
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Yogesh,
On 30.05.2018 16:24, Boris Brezillon wrote:
Hi Yogesh,
On Wed, 30 May 2018 13:50:51 +
Yogesh Narayan Gaur wrote:
Hi Frieder,
Thanks for migrating the fsl-quadspi.c driver on the new SPI
framework. This patch is using dynamic LUT approach to create the LUT
at run time instead of fixed static LUT as being used in current
driver present at mtd/spi-nor/fsl-quadspi.c. I have pushed the
changes for dynamic LUT on mtd/spi-nor/fsl-quadspi.c and v10 has been
in review stage.
Request you to please add 'signed-off' mentioned in those patches in
this patch, patchwork link is
https://patchwork.ozlabs.org/patch/896534/
So for reasons already given by Boris, I won't add your S-o-b tags. But
I can add your name (and that of Suresh Gupta?) to the file header and
as MODULE_AUTHOR in the next version.
Regards,
Frieder
First, I'd like to state that this work has not been based on your
dynamic LUT code, and I actually asked you to adapt your code to match
the way we were handling it in the new driver (which at that time was
still under development). Then, even if you want to be cited as one of
the author of the new code, SoB tag is not the right way to do it (see
[1] for an explanation on when SoB should be added). Instead, you
should add your name in the copyright header and maybe be add a
MODULE_AUTHOR():
/*
* Copyright ...
* ...
* Authors:
* ...
* Yogesh Narayan Gaur
*/
...
MODULE_AUTHOR("Yogesh Narayan Gaur ");
Regards,
Boris
[1]https://elixir.bootlin.com/linux/latest/source/Documentation/process/submitting-patches.rst#L429
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Yogesh,
On 30.05.2018 16:24, Boris Brezillon wrote:
Hi Yogesh,
On Wed, 30 May 2018 13:50:51 +
Yogesh Narayan Gaur wrote:
Hi Frieder,
Thanks for migrating the fsl-quadspi.c driver on the new SPI
framework. This patch is using dynamic LUT approach to create the LUT
at run time instead of fixed static LUT as being used in current
driver present at mtd/spi-nor/fsl-quadspi.c. I have pushed the
changes for dynamic LUT on mtd/spi-nor/fsl-quadspi.c and v10 has been
in review stage.
Request you to please add 'signed-off' mentioned in those patches in
this patch, patchwork link is
https://patchwork.ozlabs.org/patch/896534/
So for reasons already given by Boris, I won't add your S-o-b tags. But
I can add your name (and that of Suresh Gupta?) to the file header and
as MODULE_AUTHOR in the next version.
Regards,
Frieder
First, I'd like to state that this work has not been based on your
dynamic LUT code, and I actually asked you to adapt your code to match
the way we were handling it in the new driver (which at that time was
still under development). Then, even if you want to be cited as one of
the author of the new code, SoB tag is not the right way to do it (see
[1] for an explanation on when SoB should be added). Instead, you
should add your name in the copyright header and maybe be add a
MODULE_AUTHOR():
/*
* Copyright ...
* ...
* Authors:
* ...
* Yogesh Narayan Gaur
*/
...
MODULE_AUTHOR("Yogesh Narayan Gaur ");
Regards,
Boris
[1]https://elixir.bootlin.com/linux/latest/source/Documentation/process/submitting-patches.rst#L429
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris,
On 30.05.2018 16:58, Boris Brezillon wrote:
Hi Frieder,
On Wed, 30 May 2018 15:14:32 +0200
Frieder Schrempf wrote:
+
+static const char *fsl_qspi_get_name(struct spi_mem *mem)
+{
+ struct fsl_qspi *q = spi_controller_get_devdata(mem->spi->master);
+ struct device *dev = >spi->dev;
+ const char *name;
+
+ /*
+* In order to keep mtdparts compatible with the old MTD driver at
+* mtd/spi-nor/fsl-quadspi.c, we set a custom name derived from the
+* platform_device of the controller.
+*/
+ if (of_get_available_child_count(q->dev->of_node) == 1)
+ name = dev_name(q->dev);
+ else
+ name = devm_kasprintf(dev, GFP_KERNEL,
+ "%s-%d", dev_name(q->dev),
+ mem->spi->chip_select);
+
+ if (!name) {
+ dev_err(dev, "failed to get memory for custom flash name\n");
+ return dev_name(q->dev);
Hm, not sure that's what we want. We should probably fail when the
allocation fails.
Right, we should definitely fail when the allocation fails.
How about letting ->get_name() return an error pointer or NULL in case
of error. With the other I made suggestion in my review of patch 1
(calling ->get_name() at probe time) you could refuse to probe the
device when ->get_name() fails.
Ok, I will change that.
Thanks,
Frieder
+ }
+
+ return name;
+}
+
Regards,
Boris
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Boris,
On 30.05.2018 16:58, Boris Brezillon wrote:
Hi Frieder,
On Wed, 30 May 2018 15:14:32 +0200
Frieder Schrempf wrote:
+
+static const char *fsl_qspi_get_name(struct spi_mem *mem)
+{
+ struct fsl_qspi *q = spi_controller_get_devdata(mem->spi->master);
+ struct device *dev = >spi->dev;
+ const char *name;
+
+ /*
+* In order to keep mtdparts compatible with the old MTD driver at
+* mtd/spi-nor/fsl-quadspi.c, we set a custom name derived from the
+* platform_device of the controller.
+*/
+ if (of_get_available_child_count(q->dev->of_node) == 1)
+ name = dev_name(q->dev);
+ else
+ name = devm_kasprintf(dev, GFP_KERNEL,
+ "%s-%d", dev_name(q->dev),
+ mem->spi->chip_select);
+
+ if (!name) {
+ dev_err(dev, "failed to get memory for custom flash name\n");
+ return dev_name(q->dev);
Hm, not sure that's what we want. We should probably fail when the
allocation fails.
Right, we should definitely fail when the allocation fails.
How about letting ->get_name() return an error pointer or NULL in case
of error. With the other I made suggestion in my review of patch 1
(calling ->get_name() at probe time) you could refuse to probe the
device when ->get_name() fails.
Ok, I will change that.
Thanks,
Frieder
+ }
+
+ return name;
+}
+
Regards,
Boris
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Frieder,
On Wed, 30 May 2018 15:14:32 +0200
Frieder Schrempf wrote:
> +
> +static const char *fsl_qspi_get_name(struct spi_mem *mem)
> +{
> + struct fsl_qspi *q = spi_controller_get_devdata(mem->spi->master);
> + struct device *dev = >spi->dev;
> + const char *name;
> +
> + /*
> + * In order to keep mtdparts compatible with the old MTD driver at
> + * mtd/spi-nor/fsl-quadspi.c, we set a custom name derived from the
> + * platform_device of the controller.
> + */
> + if (of_get_available_child_count(q->dev->of_node) == 1)
> + name = dev_name(q->dev);
> + else
> + name = devm_kasprintf(dev, GFP_KERNEL,
> + "%s-%d", dev_name(q->dev),
> + mem->spi->chip_select);
> +
> + if (!name) {
> + dev_err(dev, "failed to get memory for custom flash name\n");
> + return dev_name(q->dev);
Hm, not sure that's what we want. We should probably fail when the
allocation fails.
How about letting ->get_name() return an error pointer or NULL in case
of error. With the other I made suggestion in my review of patch 1
(calling ->get_name() at probe time) you could refuse to probe the
device when ->get_name() fails.
> + }
> +
> + return name;
> +}
> +
Regards,
Boris
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Frieder,
On Wed, 30 May 2018 15:14:32 +0200
Frieder Schrempf wrote:
> +
> +static const char *fsl_qspi_get_name(struct spi_mem *mem)
> +{
> + struct fsl_qspi *q = spi_controller_get_devdata(mem->spi->master);
> + struct device *dev = >spi->dev;
> + const char *name;
> +
> + /*
> + * In order to keep mtdparts compatible with the old MTD driver at
> + * mtd/spi-nor/fsl-quadspi.c, we set a custom name derived from the
> + * platform_device of the controller.
> + */
> + if (of_get_available_child_count(q->dev->of_node) == 1)
> + name = dev_name(q->dev);
> + else
> + name = devm_kasprintf(dev, GFP_KERNEL,
> + "%s-%d", dev_name(q->dev),
> + mem->spi->chip_select);
> +
> + if (!name) {
> + dev_err(dev, "failed to get memory for custom flash name\n");
> + return dev_name(q->dev);
Hm, not sure that's what we want. We should probably fail when the
allocation fails.
How about letting ->get_name() return an error pointer or NULL in case
of error. With the other I made suggestion in my review of patch 1
(calling ->get_name() at probe time) you could refuse to probe the
device when ->get_name() fails.
> + }
> +
> + return name;
> +}
> +
Regards,
Boris
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Yogesh,
On Wed, 30 May 2018 13:50:51 +
Yogesh Narayan Gaur wrote:
> Hi Frieder,
>
> Thanks for migrating the fsl-quadspi.c driver on the new SPI
> framework. This patch is using dynamic LUT approach to create the LUT
> at run time instead of fixed static LUT as being used in current
> driver present at mtd/spi-nor/fsl-quadspi.c. I have pushed the
> changes for dynamic LUT on mtd/spi-nor/fsl-quadspi.c and v10 has been
> in review stage.
>
> Request you to please add 'signed-off' mentioned in those patches in
> this patch, patchwork link is
> https://patchwork.ozlabs.org/patch/896534/
First, I'd like to state that this work has not been based on your
dynamic LUT code, and I actually asked you to adapt your code to match
the way we were handling it in the new driver (which at that time was
still under development). Then, even if you want to be cited as one of
the author of the new code, SoB tag is not the right way to do it (see
[1] for an explanation on when SoB should be added). Instead, you
should add your name in the copyright header and maybe be add a
MODULE_AUTHOR():
/*
* Copyright ...
* ...
* Authors:
* ...
* Yogesh Narayan Gaur
*/
...
MODULE_AUTHOR("Yogesh Narayan Gaur ");
Regards,
Boris
[1]https://elixir.bootlin.com/linux/latest/source/Documentation/process/submitting-patches.rst#L429
Re: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Yogesh,
On Wed, 30 May 2018 13:50:51 +
Yogesh Narayan Gaur wrote:
> Hi Frieder,
>
> Thanks for migrating the fsl-quadspi.c driver on the new SPI
> framework. This patch is using dynamic LUT approach to create the LUT
> at run time instead of fixed static LUT as being used in current
> driver present at mtd/spi-nor/fsl-quadspi.c. I have pushed the
> changes for dynamic LUT on mtd/spi-nor/fsl-quadspi.c and v10 has been
> in review stage.
>
> Request you to please add 'signed-off' mentioned in those patches in
> this patch, patchwork link is
> https://patchwork.ozlabs.org/patch/896534/
First, I'd like to state that this work has not been based on your
dynamic LUT code, and I actually asked you to adapt your code to match
the way we were handling it in the new driver (which at that time was
still under development). Then, even if you want to be cited as one of
the author of the new code, SoB tag is not the right way to do it (see
[1] for an explanation on when SoB should be added). Instead, you
should add your name in the copyright header and maybe be add a
MODULE_AUTHOR():
/*
* Copyright ...
* ...
* Authors:
* ...
* Yogesh Narayan Gaur
*/
...
MODULE_AUTHOR("Yogesh Narayan Gaur ");
Regards,
Boris
[1]https://elixir.bootlin.com/linux/latest/source/Documentation/process/submitting-patches.rst#L429
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Frieder, Thanks for migrating the fsl-quadspi.c driver on the new SPI framework. This patch is using dynamic LUT approach to create the LUT at run time instead of fixed static LUT as being used in current driver present at mtd/spi-nor/fsl-quadspi.c. I have pushed the changes for dynamic LUT on mtd/spi-nor/fsl-quadspi.c and v10 has been in review stage. Request you to please add 'signed-off' mentioned in those patches in this patch, patchwork link is https://patchwork.ozlabs.org/patch/896534/ Thanks Yogesh Gaur -Original Message- From: Frieder Schrempf [mailto:frieder.schre...@exceet.de] Sent: Wednesday, May 30, 2018 6:45 PM To: linux-...@lists.infradead.org; boris.brezil...@bootlin.com; linux-...@vger.kernel.org Cc: dw...@infradead.org; computersforpe...@gmail.com; marek.va...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Yogesh Narayan Gaur ; Han Xu ; Frieder Schrempf ; linux-kernel@vger.kernel.org Subject: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller This driver is derived from the SPI NOR driver at mtd/spi-nor/fsl-quadspi.c. It uses the new SPI memory interface of the SPI framework to issue flash memory operations to up to four connected flash chips (2 buses with 2 CS each). The controller does not support generic SPI messages. Signed-off-by: Frieder Schrempf --- drivers/spi/Kconfig| 11 + drivers/spi/Makefile | 1 + drivers/spi/spi-fsl-qspi.c | 929 3 files changed, 941 insertions(+) diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index e62ac32..6de0df5 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -251,6 +251,17 @@ config SPI_FSL_LPSPI help This enables Freescale i.MX LPSPI controllers in master mode. +config SPI_FSL_QSPI + tristate "Freescale QSPI controller" + depends on ARCH_MXC || SOC_LS1021A || ARCH_LAYERSCAPE || COMPILE_TEST + depends on HAS_IOMEM + help + This enables support for the Quad SPI controller in master mode. + Up to four flash chips can be connected on two buses with two + chipselects each. + This controller does not support generic SPI messages. It only + supports the high-level SPI memory interface. + config SPI_GPIO tristate "GPIO-based bitbanging SPI Master" depends on GPIOLIB || COMPILE_TEST diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index cb1f437..a8f7fda 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -43,6 +43,7 @@ obj-$(CONFIG_SPI_FSL_DSPI)+= spi-fsl-dspi.o obj-$(CONFIG_SPI_FSL_LIB) += spi-fsl-lib.o obj-$(CONFIG_SPI_FSL_ESPI) += spi-fsl-espi.o obj-$(CONFIG_SPI_FSL_LPSPI)+= spi-fsl-lpspi.o +obj-$(CONFIG_SPI_FSL_QSPI) += spi-fsl-qspi.o obj-$(CONFIG_SPI_FSL_SPI) += spi-fsl-spi.o obj-$(CONFIG_SPI_GPIO) += spi-gpio.o obj-$(CONFIG_SPI_IMG_SPFI) += spi-img-spfi.o diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c new file mode 100644 index 000..c16d070 --- /dev/null +++ b/drivers/spi/spi-fsl-qspi.c @@ -0,0 +1,929 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/* + * Freescale QuadSPI driver. + * + * Copyright (C) 2013 Freescale Semiconductor, Inc. + * Copyright (C) 2018 Bootlin + * Copyright (C) 2018 Exceet Electronics GmbH + * + * Transition to SPI MEM interface: + * Author: + * Boris Brezillion + * Frieder Schrempf + * + * Based on the original fsl-quadspi.c spi-nor driver: + * Author: Freescale Semiconductor, Inc. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +/* + * The driver only uses one single LUT entry, that is updated on + * each call of exec_op(). Index 0 is preset at boot with a basic + * read operation, so let's use the last entry (15). + */ +#defineSEQID_LUT 15 + +/* Registers used by the driver */ +#define QUADSPI_MCR0x00 +#define QUADSPI_MCR_RESERVED_MASK (0xF << 16) +#define QUADSPI_MCR_MDIS_MASK BIT(14) +#define QUADSPI_MCR_CLR_TXF_MASK BIT(11) +#define QUADSPI_MCR_CLR_RXF_MASK BIT(10) +#define QUADSPI_MCR_DDR_EN_MASKBIT(7) +#define QUADSPI_MCR_END_CFG_MASK (0x3 << 2) +#define QUADSPI_MCR_SWRSTHD_MASK BIT(1) +#define QUADSPI_MCR_SWRSTSD_MASK BIT(0) + +#define QUADSPI_IPCR 0x08 +#define QUADSPI_IPCR_SEQID_SHIFT 24 + +#define QUADSPI_BUF3CR 0x1c +#define QUADSPI_BUF3CR_ALLMST_MASK BIT(31) +#define QUADSPI_BUF3CR_ADATSZ_SHIFT8 +#define QUADSPI_BUF3CR_ADATSZ_MASK (0xFF << QUADSPI_BUF3CR_ADATSZ_SHIFT) + +#define QUADSPI_BFGENCR
RE: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller
Hi Frieder, Thanks for migrating the fsl-quadspi.c driver on the new SPI framework. This patch is using dynamic LUT approach to create the LUT at run time instead of fixed static LUT as being used in current driver present at mtd/spi-nor/fsl-quadspi.c. I have pushed the changes for dynamic LUT on mtd/spi-nor/fsl-quadspi.c and v10 has been in review stage. Request you to please add 'signed-off' mentioned in those patches in this patch, patchwork link is https://patchwork.ozlabs.org/patch/896534/ Thanks Yogesh Gaur -Original Message- From: Frieder Schrempf [mailto:frieder.schre...@exceet.de] Sent: Wednesday, May 30, 2018 6:45 PM To: linux-...@lists.infradead.org; boris.brezil...@bootlin.com; linux-...@vger.kernel.org Cc: dw...@infradead.org; computersforpe...@gmail.com; marek.va...@gmail.com; rich...@nod.at; miquel.ray...@bootlin.com; broo...@kernel.org; David Wolfe ; Fabio Estevam ; Prabhakar Kushwaha ; Yogesh Narayan Gaur ; Han Xu ; Frieder Schrempf ; linux-kernel@vger.kernel.org Subject: [PATCH 03/11] spi: Add a driver for the Freescale/NXP QuadSPI controller This driver is derived from the SPI NOR driver at mtd/spi-nor/fsl-quadspi.c. It uses the new SPI memory interface of the SPI framework to issue flash memory operations to up to four connected flash chips (2 buses with 2 CS each). The controller does not support generic SPI messages. Signed-off-by: Frieder Schrempf --- drivers/spi/Kconfig| 11 + drivers/spi/Makefile | 1 + drivers/spi/spi-fsl-qspi.c | 929 3 files changed, 941 insertions(+) diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index e62ac32..6de0df5 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -251,6 +251,17 @@ config SPI_FSL_LPSPI help This enables Freescale i.MX LPSPI controllers in master mode. +config SPI_FSL_QSPI + tristate "Freescale QSPI controller" + depends on ARCH_MXC || SOC_LS1021A || ARCH_LAYERSCAPE || COMPILE_TEST + depends on HAS_IOMEM + help + This enables support for the Quad SPI controller in master mode. + Up to four flash chips can be connected on two buses with two + chipselects each. + This controller does not support generic SPI messages. It only + supports the high-level SPI memory interface. + config SPI_GPIO tristate "GPIO-based bitbanging SPI Master" depends on GPIOLIB || COMPILE_TEST diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index cb1f437..a8f7fda 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -43,6 +43,7 @@ obj-$(CONFIG_SPI_FSL_DSPI)+= spi-fsl-dspi.o obj-$(CONFIG_SPI_FSL_LIB) += spi-fsl-lib.o obj-$(CONFIG_SPI_FSL_ESPI) += spi-fsl-espi.o obj-$(CONFIG_SPI_FSL_LPSPI)+= spi-fsl-lpspi.o +obj-$(CONFIG_SPI_FSL_QSPI) += spi-fsl-qspi.o obj-$(CONFIG_SPI_FSL_SPI) += spi-fsl-spi.o obj-$(CONFIG_SPI_GPIO) += spi-gpio.o obj-$(CONFIG_SPI_IMG_SPFI) += spi-img-spfi.o diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c new file mode 100644 index 000..c16d070 --- /dev/null +++ b/drivers/spi/spi-fsl-qspi.c @@ -0,0 +1,929 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/* + * Freescale QuadSPI driver. + * + * Copyright (C) 2013 Freescale Semiconductor, Inc. + * Copyright (C) 2018 Bootlin + * Copyright (C) 2018 Exceet Electronics GmbH + * + * Transition to SPI MEM interface: + * Author: + * Boris Brezillion + * Frieder Schrempf + * + * Based on the original fsl-quadspi.c spi-nor driver: + * Author: Freescale Semiconductor, Inc. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +/* + * The driver only uses one single LUT entry, that is updated on + * each call of exec_op(). Index 0 is preset at boot with a basic + * read operation, so let's use the last entry (15). + */ +#defineSEQID_LUT 15 + +/* Registers used by the driver */ +#define QUADSPI_MCR0x00 +#define QUADSPI_MCR_RESERVED_MASK (0xF << 16) +#define QUADSPI_MCR_MDIS_MASK BIT(14) +#define QUADSPI_MCR_CLR_TXF_MASK BIT(11) +#define QUADSPI_MCR_CLR_RXF_MASK BIT(10) +#define QUADSPI_MCR_DDR_EN_MASKBIT(7) +#define QUADSPI_MCR_END_CFG_MASK (0x3 << 2) +#define QUADSPI_MCR_SWRSTHD_MASK BIT(1) +#define QUADSPI_MCR_SWRSTSD_MASK BIT(0) + +#define QUADSPI_IPCR 0x08 +#define QUADSPI_IPCR_SEQID_SHIFT 24 + +#define QUADSPI_BUF3CR 0x1c +#define QUADSPI_BUF3CR_ALLMST_MASK BIT(31) +#define QUADSPI_BUF3CR_ADATSZ_SHIFT8 +#define QUADSPI_BUF3CR_ADATSZ_MASK (0xFF << QUADSPI_BUF3CR_ADATSZ_SHIFT) + +#define QUADSPI_BFGENCR
