All of lore.kernel.org
 help / color / mirror / Atom feed
From: Angus Clark <angus.clark@st.com>
To: Huang Shijie <b32955@freescale.com>
Cc: Lee Jones <lee.jones@linaro.org>,
	linus.walleij@linaro.org, linux-kernel@vger.kernel.org,
	linux-spi@vger.kernel.org, Mark Brown <broonie@kernel.org>,
	linux-mtd@lists.infradead.org,
	Brian Norris <computersforpeace@gmail.com>,
	dwmw2@infradead.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 00/23] mtd: st_spi_fsm: Add new device
Date: Thu, 28 Nov 2013 09:07:15 +0000	[thread overview]
Message-ID: <529707C3.40208@st.com> (raw)
In-Reply-To: <5296B9C5.3060704@freescale.com>

Hi Huang Shijie,

On 11/28/2013 03:34 AM, Huang Shijie wrote:
> 于 2013年11月27日 19:52, Lee Jones 写道:
>> However, as we send entire 'message sequences' to the FSM Controller
>> as opposed to merely OPCODEs we would have to extract the OPCODE from
>> flash->command[0] and call our own functions to craft the correct
>> 'message sequence' for the task. For this reason we rejected the idea
>> and went with a stand-alone driver.
>>
> could you send me the datasheet of your spi nor controller?
> I can change my code if it really not good enough.

I will reply to the "mtd: spi-nor" thread regarding the proposed framework, but
a couple of answers to your specific questions below.

> 
> we can store the opcode to a field, such as spi_nor_write_op.
>> The framework which Huang is proposing suffers from the same issues.
>> Only providing read(), write(), read_reg() and write_reg() doesn't
>> work for our use-case, as we'd have to decode the flash->command[0] and
>> invoke our own internal routines as before.
>>
>> The only framework with would work for us would consist almost all
>> of the important functions such as; read(), write(), erase(),
>> wait_busy(), read_jedec(), read_status_reg(), write_status_reg(),
>> read_control_reg(), write_control_reg(), etc. However, this approach
>>   
> read_jedec() can be replaced by read_reg(0x9f);
> 
> read_status() can be replaced by read_reg(0x5);
> 
> ....
> 
> write_control_reg() can be replaced by write_reg(xx).

Unfortunately the H/W Controller in question comes with a few restrictions.  One
restriction is that data must be read in multiples of 4 bytes.  As such, it
would not be able to honour a call of "flash->read_reg(flash, OPCODE_RDID, id, 5);"

Of course, if the H/W driver knows that we are issuing a read_jedec() operation,
then it can make the assumption that over-reading is benign, and we can instead
read 8 bytes of data from the Flash device, and return 5 to the caller.
However, without knowing what operation is being requested, no such assumption
can be made.

> Please correct me if i am wrong.
> 
> IMHO, the current four hooks for spi-nor{} can do all the things.
> 
>      read/write/read_reg/write_reg.

As it stands, the spi-nor framework cannot support the requirements of the
st_spi_fsm controller.  I will go into further details on the "mtd: spi-nor" thread.

Cheers,

Angus


-- 
-------------------------------------
Angus Clark
ST Microelectronics (R&D) Ltd.
1000 Aztec West, Bristol, BS32 4SQ
email: angus.clark@st.com
tel: +44 (0) 1454 462389
st-tina: 065 2389
-------------------------------------

WARNING: multiple messages have this Message-ID (diff)
From: Angus Clark <angus.clark@st.com>
To: Huang Shijie <b32955@freescale.com>
Cc: Lee Jones <lee.jones@linaro.org>,
	linus.walleij@linaro.org, linux-kernel@vger.kernel.org,
	linux-spi@vger.kernel.org, Mark Brown <broonie@kernel.org>,
	linux-mtd@lists.infradead.org,
	Brian Norris <computersforpeace@gmail.com>,
	dwmw2@infradead.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 00/23] mtd: st_spi_fsm: Add new device
Date: Thu, 28 Nov 2013 09:07:15 +0000	[thread overview]
Message-ID: <529707C3.40208@st.com> (raw)
In-Reply-To: <5296B9C5.3060704@freescale.com>

Hi Huang Shijie,

On 11/28/2013 03:34 AM, Huang Shijie wrote:
> 于 2013年11月27日 19:52, Lee Jones 写道:
>> However, as we send entire 'message sequences' to the FSM Controller
>> as opposed to merely OPCODEs we would have to extract the OPCODE from
>> flash->command[0] and call our own functions to craft the correct
>> 'message sequence' for the task. For this reason we rejected the idea
>> and went with a stand-alone driver.
>>
> could you send me the datasheet of your spi nor controller?
> I can change my code if it really not good enough.

I will reply to the "mtd: spi-nor" thread regarding the proposed framework, but
a couple of answers to your specific questions below.

> 
> we can store the opcode to a field, such as spi_nor_write_op.
>> The framework which Huang is proposing suffers from the same issues.
>> Only providing read(), write(), read_reg() and write_reg() doesn't
>> work for our use-case, as we'd have to decode the flash->command[0] and
>> invoke our own internal routines as before.
>>
>> The only framework with would work for us would consist almost all
>> of the important functions such as; read(), write(), erase(),
>> wait_busy(), read_jedec(), read_status_reg(), write_status_reg(),
>> read_control_reg(), write_control_reg(), etc. However, this approach
>>   
> read_jedec() can be replaced by read_reg(0x9f);
> 
> read_status() can be replaced by read_reg(0x5);
> 
> ....
> 
> write_control_reg() can be replaced by write_reg(xx).

Unfortunately the H/W Controller in question comes with a few restrictions.  One
restriction is that data must be read in multiples of 4 bytes.  As such, it
would not be able to honour a call of "flash->read_reg(flash, OPCODE_RDID, id, 5);"

Of course, if the H/W driver knows that we are issuing a read_jedec() operation,
then it can make the assumption that over-reading is benign, and we can instead
read 8 bytes of data from the Flash device, and return 5 to the caller.
However, without knowing what operation is being requested, no such assumption
can be made.

> Please correct me if i am wrong.
> 
> IMHO, the current four hooks for spi-nor{} can do all the things.
> 
>      read/write/read_reg/write_reg.

As it stands, the spi-nor framework cannot support the requirements of the
st_spi_fsm controller.  I will go into further details on the "mtd: spi-nor" thread.

Cheers,

Angus


-- 
-------------------------------------
Angus Clark
ST Microelectronics (R&D) Ltd.
1000 Aztec West, Bristol, BS32 4SQ
email: angus.clark@st.com
tel: +44 (0) 1454 462389
st-tina: 065 2389
-------------------------------------

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

WARNING: multiple messages have this Message-ID (diff)
From: angus.clark@st.com (Angus Clark)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 00/23] mtd: st_spi_fsm: Add new device
Date: Thu, 28 Nov 2013 09:07:15 +0000	[thread overview]
Message-ID: <529707C3.40208@st.com> (raw)
In-Reply-To: <5296B9C5.3060704@freescale.com>

Hi Huang Shijie,

On 11/28/2013 03:34 AM, Huang Shijie wrote:
> ? 2013?11?27? 19:52, Lee Jones ??:
>> However, as we send entire 'message sequences' to the FSM Controller
>> as opposed to merely OPCODEs we would have to extract the OPCODE from
>> flash->command[0] and call our own functions to craft the correct
>> 'message sequence' for the task. For this reason we rejected the idea
>> and went with a stand-alone driver.
>>
> could you send me the datasheet of your spi nor controller?
> I can change my code if it really not good enough.

I will reply to the "mtd: spi-nor" thread regarding the proposed framework, but
a couple of answers to your specific questions below.

> 
> we can store the opcode to a field, such as spi_nor_write_op.
>> The framework which Huang is proposing suffers from the same issues.
>> Only providing read(), write(), read_reg() and write_reg() doesn't
>> work for our use-case, as we'd have to decode the flash->command[0] and
>> invoke our own internal routines as before.
>>
>> The only framework with would work for us would consist almost all
>> of the important functions such as; read(), write(), erase(),
>> wait_busy(), read_jedec(), read_status_reg(), write_status_reg(),
>> read_control_reg(), write_control_reg(), etc. However, this approach
>>   
> read_jedec() can be replaced by read_reg(0x9f);
> 
> read_status() can be replaced by read_reg(0x5);
> 
> ....
> 
> write_control_reg() can be replaced by write_reg(xx).

Unfortunately the H/W Controller in question comes with a few restrictions.  One
restriction is that data must be read in multiples of 4 bytes.  As such, it
would not be able to honour a call of "flash->read_reg(flash, OPCODE_RDID, id, 5);"

Of course, if the H/W driver knows that we are issuing a read_jedec() operation,
then it can make the assumption that over-reading is benign, and we can instead
read 8 bytes of data from the Flash device, and return 5 to the caller.
However, without knowing what operation is being requested, no such assumption
can be made.

> Please correct me if i am wrong.
> 
> IMHO, the current four hooks for spi-nor{} can do all the things.
> 
>      read/write/read_reg/write_reg.

As it stands, the spi-nor framework cannot support the requirements of the
st_spi_fsm controller.  I will go into further details on the "mtd: spi-nor" thread.

Cheers,

Angus


-- 
-------------------------------------
Angus Clark
ST Microelectronics (R&D) Ltd.
1000 Aztec West, Bristol, BS32 4SQ
email: angus.clark at st.com
tel: +44 (0) 1454 462389
st-tina: 065 2389
-------------------------------------

WARNING: multiple messages have this Message-ID (diff)
From: Angus Clark <angus.clark@st.com>
To: Huang Shijie <b32955@freescale.com>
Cc: Lee Jones <lee.jones@linaro.org>,
	Brian Norris <computersforpeace@gmail.com>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>, <dwmw2@infradead.org>,
	<linux-mtd@lists.infradead.org>, <linus.walleij@linaro.org>,
	Mark Brown <broonie@kernel.org>, <linux-spi@vger.kernel.org>
Subject: Re: [PATCH 00/23] mtd: st_spi_fsm: Add new device
Date: Thu, 28 Nov 2013 09:07:15 +0000	[thread overview]
Message-ID: <529707C3.40208@st.com> (raw)
In-Reply-To: <5296B9C5.3060704@freescale.com>

Hi Huang Shijie,

On 11/28/2013 03:34 AM, Huang Shijie wrote:
> 于 2013年11月27日 19:52, Lee Jones 写道:
>> However, as we send entire 'message sequences' to the FSM Controller
>> as opposed to merely OPCODEs we would have to extract the OPCODE from
>> flash->command[0] and call our own functions to craft the correct
>> 'message sequence' for the task. For this reason we rejected the idea
>> and went with a stand-alone driver.
>>
> could you send me the datasheet of your spi nor controller?
> I can change my code if it really not good enough.

I will reply to the "mtd: spi-nor" thread regarding the proposed framework, but
a couple of answers to your specific questions below.

> 
> we can store the opcode to a field, such as spi_nor_write_op.
>> The framework which Huang is proposing suffers from the same issues.
>> Only providing read(), write(), read_reg() and write_reg() doesn't
>> work for our use-case, as we'd have to decode the flash->command[0] and
>> invoke our own internal routines as before.
>>
>> The only framework with would work for us would consist almost all
>> of the important functions such as; read(), write(), erase(),
>> wait_busy(), read_jedec(), read_status_reg(), write_status_reg(),
>> read_control_reg(), write_control_reg(), etc. However, this approach
>>   
> read_jedec() can be replaced by read_reg(0x9f);
> 
> read_status() can be replaced by read_reg(0x5);
> 
> ....
> 
> write_control_reg() can be replaced by write_reg(xx).

Unfortunately the H/W Controller in question comes with a few restrictions.  One
restriction is that data must be read in multiples of 4 bytes.  As such, it
would not be able to honour a call of "flash->read_reg(flash, OPCODE_RDID, id, 5);"

Of course, if the H/W driver knows that we are issuing a read_jedec() operation,
then it can make the assumption that over-reading is benign, and we can instead
read 8 bytes of data from the Flash device, and return 5 to the caller.
However, without knowing what operation is being requested, no such assumption
can be made.

> Please correct me if i am wrong.
> 
> IMHO, the current four hooks for spi-nor{} can do all the things.
> 
>      read/write/read_reg/write_reg.

As it stands, the spi-nor framework cannot support the requirements of the
st_spi_fsm controller.  I will go into further details on the "mtd: spi-nor" thread.

Cheers,

Angus


-- 
-------------------------------------
Angus Clark
ST Microelectronics (R&D) Ltd.
1000 Aztec West, Bristol, BS32 4SQ
email: angus.clark@st.com
tel: +44 (0) 1454 462389
st-tina: 065 2389
-------------------------------------

  reply	other threads:[~2013-11-28  9:07 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-22 16:22 [PATCH 00/23] mtd: st_spi_fsm: Add new device Lee Jones
2013-11-22 16:22 ` Lee Jones
2013-11-22 16:22 ` [PATCH 01/23] mtd: st_spi_fsm: Allocate resources and register with MTD framework Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 02/23] mtd: st_spi_fsm: Supply all register address and bit logic defines Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 03/23] mtd: st_spi_fsm: Initialise and configure the FSM for normal working conditions Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 04/23] mtd: st_spi_fsm: Supply framework for device requests Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 05/23] mtd: st_spi_fsm: Supply a method to read from the FSM's FIFO Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 06/23] mtd: st_spi_fsm: Supply defines for the possible flash command opcodes Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 07/23] mtd: st_spi_fsm: Add support for JEDEC ID extraction Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 08/23] mtd: devices: Provide header for shared OPCODEs and SFDP commands Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 09/23] mtd: st_spi_fsm: Provide device look-up table Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 10/23] mtd: st_spi_fsm: Dynamically setup flash device based on JEDEC ID Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 11/23] ARM: STi: Add support for the FSM Serial Flash Controller Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 12/23] mtd: st_spi_fsm: Search for preferred FSM message sequence configurations Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 13/23] mtd: st_spi_fsm: Fetch platform specific configurations Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 14/23] mtd: st_spi_fsm: Prepare the read/write FSM message sequence(s) Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 15/23] mtd: st_spi_fsm: Fetch boot-device from mode pins Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 16/23] mtd: st_spi_fsm: Provide the erase one sector sequence Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 17/23] mtd: st_spi_fsm: Provide the sequence for enabling 32bit addressing mode Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 18/23] mtd: st_spi_fsm: Prepare read/write sequences according to configuration Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 19/23] mtd: st_spi_fsm: Add a check to if the chip can handle an SoC reset Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 20/23] mtd: st_spi_fsm: Provide a method to put the chip into 32bit addressing mode Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 21/23] mtd: st_spi_fsm: Update the flash Volatile Configuration Register Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:22 ` [PATCH 22/23] mtd: st_spi_fsm: Provide the default read/write configurations Lee Jones
2013-11-22 16:22   ` Lee Jones
2013-11-22 16:23 ` [PATCH 23/23] mtd: st_spi_fsm: Supply the N25Qxxx specific read configurations Lee Jones
2013-11-22 16:23   ` Lee Jones
2013-11-27  4:07 ` [PATCH 00/23] mtd: st_spi_fsm: Add new device Brian Norris
2013-11-27  4:07   ` Brian Norris
2013-11-27  4:07   ` Brian Norris
2013-11-27  4:07   ` Brian Norris
2013-11-27 11:52   ` Lee Jones
2013-11-27 11:52     ` Lee Jones
2013-11-27 11:52     ` Lee Jones
2013-11-27 11:52     ` Lee Jones
2013-11-28  3:34     ` Huang Shijie
2013-11-28  3:34       ` Huang Shijie
2013-11-28  3:34       ` Huang Shijie
2013-11-28  3:34       ` Huang Shijie
2013-11-28  9:07       ` Angus Clark [this message]
2013-11-28  9:07         ` Angus Clark
2013-11-28  9:07         ` Angus Clark
2013-11-28  9:07         ` Angus Clark
2013-11-28  9:29       ` Lee Jones
2013-11-28  9:29         ` Lee Jones
2013-11-28  9:29         ` Lee Jones
2013-11-28  9:29         ` Lee Jones
2013-11-29 11:05         ` Huang Shijie
2013-11-29 11:05           ` Huang Shijie
2013-11-29 11:05           ` Huang Shijie
2013-11-29 11:05           ` Huang Shijie
2013-11-29 11:53           ` Lee Jones
2013-11-29 11:53             ` Lee Jones
2013-11-29 11:53             ` Lee Jones

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=529707C3.40208@st.com \
    --to=angus.clark@st.com \
    --cc=b32955@freescale.com \
    --cc=broonie@kernel.org \
    --cc=computersforpeace@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=lee.jones@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-spi@vger.kernel.org \
    /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.