All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sourav Poddar <sourav.poddar@ti.com>
To: Huang Shijie <b32955@freescale.com>
Cc: computersforpeace@gmail.com, b44548@freescale.com,
	dedekind1@gmail.com, b18965@freescale.com,
	linux-spi@vger.kernel.org, Mark Brown <broonie@kernel.org>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	kernel@pengutronix.de, shawn.guo@linaro.org, dwmw2@infradead.org,
	yuhang wang <wangyuhang2014@gmail.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH V1 3/5] mtd: m25p80: add the quad-read support
Date: Fri, 23 Aug 2013 15:27:43 +0530	[thread overview]
Message-ID: <52173217.70505@ti.com> (raw)
In-Reply-To: <52172A81.5090501@freescale.com>

On Friday 23 August 2013 02:55 PM, Huang Shijie wrote:
> 于 2013年08月23日 17:05, yuhang wang 写道:
>>> +       u16 sr_cr;
>>> >  +       int ret;
>>> >
>>> >    #ifdef CONFIG_MTD_OF_PARTS
>>> >           if (!of_device_is_available(np))
>>> >  @@ -1014,6 +1050,21 @@ static int m25p_probe(struct spi_device *spi)
>>> >           else
>>> >                   flash->read_opcode = OPCODE_NORM_READ;
>>> >
>>> >  +       /* Try to enable the Quad Read */
>>> >  +       if (np&&  of_property_read_bool(np, "m25p,quad-read")) {
>>> >  +               /* The configuration register is set by the 
>>> second byte. */
>>> >  +               sr_cr = CR_QUAD<<  8;
>>> >  +
>>> >  +               /* Write the QUAD bit to the Configuration 
>>> Register. */
>>> >  +               write_enable(flash);
>>> >  +               if (write_sr_cr(flash, sr_cr) == 0) {
>>> >  +                       /* read back and check it */
>>> >  +                       ret = read_cr(flash);
>>> >  +                       if (ret>  0&&  (ret&  CR_QUAD))
>>> >  +                               flash->read_opcode = OPCODE_QIOR;
>>> >  +               }
>>> >  +       }
>>> >  +
>> Well, M25p80.c support lots of flash devices, so driver should be as
>> general as possible. Firstly not all the devices m25p80 supports set
>> quad mode as your sequence, perhaps write_sr_cr can not match all the
> It does not matter the NOR flash supports the write_sr_cr() or not,
> If the NOR flash does not support the write_sr_cr(), it may fails, and 
> you will not set the OPCODE_QIOR for the
> m25p80_read.
>
>> m25p80 flash. Secondly, why you only support QIOR(high performance)
>> not QOR or DOR. Maybe QIOR seems too special, so what if user want to
>> use QOR if he set quad mode in DTS.
>>
> Frankly speaking, i am reluctant to support the QIOR, it is a little 
> slow. :)
>
You should add QOR opcodes also in your patch, so we have the complete set.
> So the the QIOR is lowest speed for QUADSPI controller, and i do not 
> want to support the DOR.
>
> In my new version, i add the support for DDR QIOR read which is the 
> double rate of the QIOR.
>
> The user should knows if the NOR flash supports the quad-read or not, 
> and set the proper DT.
>
>> Another point, if command changed to OPCODE_QIOR, there should also
>> should be some correct in m25p_read. such as the number of dummy data.
> I only need to change the read opcode.
>> QIOR can support read without read command if set the certain bit in
>> transfer, these aspects did not reflect in your patch.
>>
> For the Quadspi, it will handle the dummy by the LUT sequence, such as 
> DDR QUAD read, the LUT sequence will
> set proper dummy (6 cycles for S25FL128S). I do not need the m25p_read 
> to set the dummy.
>
>
> thanks
> Huang Shijie
>
>
>
>
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

WARNING: multiple messages have this Message-ID (diff)
From: sourav.poddar@ti.com (Sourav Poddar)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V1 3/5] mtd: m25p80: add the quad-read support
Date: Fri, 23 Aug 2013 15:27:43 +0530	[thread overview]
Message-ID: <52173217.70505@ti.com> (raw)
In-Reply-To: <52172A81.5090501@freescale.com>

On Friday 23 August 2013 02:55 PM, Huang Shijie wrote:
> ? 2013?08?23? 17:05, yuhang wang ??:
>>> +       u16 sr_cr;
>>> >  +       int ret;
>>> >
>>> >    #ifdef CONFIG_MTD_OF_PARTS
>>> >           if (!of_device_is_available(np))
>>> >  @@ -1014,6 +1050,21 @@ static int m25p_probe(struct spi_device *spi)
>>> >           else
>>> >                   flash->read_opcode = OPCODE_NORM_READ;
>>> >
>>> >  +       /* Try to enable the Quad Read */
>>> >  +       if (np&&  of_property_read_bool(np, "m25p,quad-read")) {
>>> >  +               /* The configuration register is set by the 
>>> second byte. */
>>> >  +               sr_cr = CR_QUAD<<  8;
>>> >  +
>>> >  +               /* Write the QUAD bit to the Configuration 
>>> Register. */
>>> >  +               write_enable(flash);
>>> >  +               if (write_sr_cr(flash, sr_cr) == 0) {
>>> >  +                       /* read back and check it */
>>> >  +                       ret = read_cr(flash);
>>> >  +                       if (ret>  0&&  (ret&  CR_QUAD))
>>> >  +                               flash->read_opcode = OPCODE_QIOR;
>>> >  +               }
>>> >  +       }
>>> >  +
>> Well, M25p80.c support lots of flash devices, so driver should be as
>> general as possible. Firstly not all the devices m25p80 supports set
>> quad mode as your sequence, perhaps write_sr_cr can not match all the
> It does not matter the NOR flash supports the write_sr_cr() or not,
> If the NOR flash does not support the write_sr_cr(), it may fails, and 
> you will not set the OPCODE_QIOR for the
> m25p80_read.
>
>> m25p80 flash. Secondly, why you only support QIOR(high performance)
>> not QOR or DOR. Maybe QIOR seems too special, so what if user want to
>> use QOR if he set quad mode in DTS.
>>
> Frankly speaking, i am reluctant to support the QIOR, it is a little 
> slow. :)
>
You should add QOR opcodes also in your patch, so we have the complete set.
> So the the QIOR is lowest speed for QUADSPI controller, and i do not 
> want to support the DOR.
>
> In my new version, i add the support for DDR QIOR read which is the 
> double rate of the QIOR.
>
> The user should knows if the NOR flash supports the quad-read or not, 
> and set the proper DT.
>
>> Another point, if command changed to OPCODE_QIOR, there should also
>> should be some correct in m25p_read. such as the number of dummy data.
> I only need to change the read opcode.
>> QIOR can support read without read command if set the certain bit in
>> transfer, these aspects did not reflect in your patch.
>>
> For the Quadspi, it will handle the dummy by the LUT sequence, such as 
> DDR QUAD read, the LUT sequence will
> set proper dummy (6 cycles for S25FL128S). I do not need the m25p_read 
> to set the dummy.
>
>
> thanks
> Huang Shijie
>
>
>
>
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply	other threads:[~2013-08-23  9:57 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-19  4:09 [PATCH V1 0/5] Add the Quadspi driver for vf610-twr Huang Shijie
2013-08-19  4:09 ` Huang Shijie
2013-08-19  4:09 ` [PATCH V1 1/5] mtd: m25p80: move the spi-nor commands to a header Huang Shijie
2013-08-19  4:09   ` Huang Shijie
2013-08-19  4:10 ` [PATCH V1 2/5] mtd: m25p80: add support for Spansion s25fl128s chip Huang Shijie
2013-08-19  4:10   ` Huang Shijie
2013-08-19  4:10 ` [PATCH V1 3/5] mtd: m25p80: add the quad-read support Huang Shijie
2013-08-19  4:10   ` Huang Shijie
2013-08-22 19:34   ` Brian Norris
2013-08-22 19:34     ` Brian Norris
2013-08-22 19:55     ` Mark Brown
2013-08-22 19:55       ` Mark Brown
2013-08-22 20:29       ` Marek Vasut
2013-08-22 20:29         ` Marek Vasut
2013-08-22 23:36         ` Mark Brown
2013-08-22 23:36           ` Mark Brown
2013-08-22 23:58           ` Marek Vasut
2013-08-22 23:58             ` Marek Vasut
2013-08-23  9:41             ` Mark Brown
2013-08-23  9:41               ` Mark Brown
2013-08-23 10:42               ` Marek Vasut
2013-08-23 10:42                 ` Marek Vasut
2013-08-23 11:46               ` Brian Norris
2013-08-23 11:46                 ` Brian Norris
2013-08-23 11:53                 ` Brian Norris
2013-08-23 11:53                   ` Brian Norris
2013-08-23 12:01                   ` Mark Brown
2013-08-23 12:01                     ` Mark Brown
2013-08-23 13:20                   ` yuhang wang
2013-08-23 13:20                     ` yuhang wang
2013-08-23  6:26       ` Huang Shijie
2013-08-23  6:26         ` Huang Shijie
2013-08-23 11:23       ` Brian Norris
2013-08-23 11:23         ` Brian Norris
2013-08-23 11:27         ` Sourav Poddar
2013-08-23 11:27           ` Sourav Poddar
2013-08-23 11:30         ` Mark Brown
2013-08-23 11:30           ` Mark Brown
2013-08-23  9:05   ` yuhang wang
2013-08-23  9:05     ` yuhang wang
2013-08-23  9:25     ` Huang Shijie
2013-08-23  9:25       ` Huang Shijie
2013-08-23  9:57       ` Sourav Poddar [this message]
2013-08-23  9:57         ` Sourav Poddar
2013-08-24  2:45         ` Huang Shijie
2013-08-24  2:45           ` Huang Shijie
2013-08-23 15:59           ` Sourav Poddar
2013-08-23 15:59             ` Sourav Poddar
2013-08-23 13:59       ` yuhang wang
2013-08-23 13:59         ` yuhang wang
2013-08-24  3:01         ` Huang Shijie
2013-08-24  3:01           ` Huang Shijie
2013-08-19  4:10 ` [PATCH V1 4/5] spi: Add Freescale QuadSpi driver Huang Shijie
2013-08-19  4:10   ` Huang Shijie
2013-08-22 19:21   ` Brian Norris
2013-08-22 19:21     ` Brian Norris
2013-08-23  2:14     ` Huang Shijie
2013-08-23  2:14       ` Huang Shijie
2013-08-23  6:59     ` Huang Shijie
2013-08-23  6:59       ` Huang Shijie
2013-08-23 16:44   ` Mark Brown
2013-08-23 16:44     ` Mark Brown
2013-08-24  7:11     ` Brian Norris
2013-08-24  7:11       ` Brian Norris
2013-08-24 13:42     ` Huang Shijie
2013-08-24 13:42       ` Huang Shijie
2013-08-19  4:10 ` [PATCH V1 5/5] ARM: dts: vf610-twr: Add SPI NOR support Huang Shijie
2013-08-19  4:10   ` Huang Shijie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52173217.70505@ti.com \
    --to=sourav.poddar@ti.com \
    --cc=b18965@freescale.com \
    --cc=b32955@freescale.com \
    --cc=b44548@freescale.com \
    --cc=broonie@kernel.org \
    --cc=computersforpeace@gmail.com \
    --cc=dedekind1@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=shawn.guo@linaro.org \
    --cc=wangyuhang2014@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.