All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sourav Poddar <sourav.poddar@ti.com>
To: Lee Jones <lee.jones@linaro.org>
Cc: angus.clark@st.com, Linus Walleij <linus.walleij@linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Mark Brown <broonie@kernel.org>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	David Woodhouse <dwmw2@infradead.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 02/10] mtd: st_spi_fsm: Supply all register address and bit logic defines
Date: Mon, 18 Nov 2013 23:29:21 +0530	[thread overview]
Message-ID: <528A5579.60705@ti.com> (raw)
In-Reply-To: <20131118173201.GP13640@lee--X1>

On Monday 18 November 2013 11:02 PM, Lee Jones wrote:
> On Mon, 18 Nov 2013, Mark Brown wrote:
>
>> On Mon, Nov 18, 2013 at 04:02:26PM +0000, Lee Jones wrote:
>>> On Mon, 18 Nov 2013, Mark Brown wrote:
>>>> Like I say I'm suggesting that the bit of the code that understands the
>>>> flash chip is separate to the bit of code that knows the mechanics of
>>>> sending commands and data to the chip.
>>> The issue is that almost the entire driver is controller side. The
>>> only bits that are the same (and not in all cases) are the OPCODEs,
>>> but they are one liners (21 lines out of 1153). Most of the
>>> controllers which use this stuff could reuse quite a bit of the m25p80
>>> driver as they just write the message containing the OPCODE as the
>>> m25p80 driver sets it up, but that's simply not the case with our
>>> controller. We would have to pull the OPCODE out and based on which
>>> one it is, we'd have to build our own message.
>> OK, so then perhaps the abstraction here is simply to export the table
>> with the opcodes from the m25p80 driver so that when someone comes along
>> and adds a new chip they can just add it there and other drivers will
>> get the update too.
> We could do that, although I'd have to insist on extending the current
> framework to add a configuration call-back, as it's the neatest way to
> configure chip specific attributes.
>
This looks like the problem which some other controllers(from ti,
freescale) are facing.
I will just summarise the problem with the ti qspi flash controller 
which I am
working on. There is a set of registers which need to be filled with flash
specific commands. One way to deal it with to provide a device tree bindings
for all the requirements(which is really cumbersome.).

Similarly, for freescale there is a LUT registers which has such flash 
requirements.

So, surely we need a way out from m25p80 driver to handle such cases.

drivers/mtd/devices/m25p80.c


int pass_flash_info () {

    u8 info[6];

      info[0] = DUMMY WR;
      info[1] = RD_OPCODE;
      info[2] - DUMMY BITS;
      ......
     .......
    spi_write(flash, info, 6)
}
Then, somehow parse this information to set up the required info.
  This is just a rough idea, and can be implemented in a better way.
>   I can get a patch out tomorrow if the MTD guys agree. Where are they
> by the way? I haven't seen hide nor hair of them since sending out the
> patch set.
>
>>> Put it this way, if we tried to use the m25p80 our controller driver
>>> would most likely be twice as large and twice as complex as it is
>>> currently, which is exactly the inverse of what we're trying to
>>> achieve here.
>> If we're having to add new flashes to multiple drivers I'd not say we're
>> winning.
> I agree.
>

WARNING: multiple messages have this Message-ID (diff)
From: sourav.poddar@ti.com (Sourav Poddar)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 02/10] mtd: st_spi_fsm: Supply all register address and bit logic defines
Date: Mon, 18 Nov 2013 23:29:21 +0530	[thread overview]
Message-ID: <528A5579.60705@ti.com> (raw)
In-Reply-To: <20131118173201.GP13640@lee--X1>

On Monday 18 November 2013 11:02 PM, Lee Jones wrote:
> On Mon, 18 Nov 2013, Mark Brown wrote:
>
>> On Mon, Nov 18, 2013 at 04:02:26PM +0000, Lee Jones wrote:
>>> On Mon, 18 Nov 2013, Mark Brown wrote:
>>>> Like I say I'm suggesting that the bit of the code that understands the
>>>> flash chip is separate to the bit of code that knows the mechanics of
>>>> sending commands and data to the chip.
>>> The issue is that almost the entire driver is controller side. The
>>> only bits that are the same (and not in all cases) are the OPCODEs,
>>> but they are one liners (21 lines out of 1153). Most of the
>>> controllers which use this stuff could reuse quite a bit of the m25p80
>>> driver as they just write the message containing the OPCODE as the
>>> m25p80 driver sets it up, but that's simply not the case with our
>>> controller. We would have to pull the OPCODE out and based on which
>>> one it is, we'd have to build our own message.
>> OK, so then perhaps the abstraction here is simply to export the table
>> with the opcodes from the m25p80 driver so that when someone comes along
>> and adds a new chip they can just add it there and other drivers will
>> get the update too.
> We could do that, although I'd have to insist on extending the current
> framework to add a configuration call-back, as it's the neatest way to
> configure chip specific attributes.
>
This looks like the problem which some other controllers(from ti,
freescale) are facing.
I will just summarise the problem with the ti qspi flash controller 
which I am
working on. There is a set of registers which need to be filled with flash
specific commands. One way to deal it with to provide a device tree bindings
for all the requirements(which is really cumbersome.).

Similarly, for freescale there is a LUT registers which has such flash 
requirements.

So, surely we need a way out from m25p80 driver to handle such cases.

drivers/mtd/devices/m25p80.c


int pass_flash_info () {

    u8 info[6];

      info[0] = DUMMY WR;
      info[1] = RD_OPCODE;
      info[2] - DUMMY BITS;
      ......
     .......
    spi_write(flash, info, 6)
}
Then, somehow parse this information to set up the required info.
  This is just a rough idea, and can be implemented in a better way.
>   I can get a patch out tomorrow if the MTD guys agree. Where are they
> by the way? I haven't seen hide nor hair of them since sending out the
> patch set.
>
>>> Put it this way, if we tried to use the m25p80 our controller driver
>>> would most likely be twice as large and twice as complex as it is
>>> currently, which is exactly the inverse of what we're trying to
>>> achieve here.
>> If we're having to add new flashes to multiple drivers I'd not say we're
>> winning.
> I agree.
>

WARNING: multiple messages have this Message-ID (diff)
From: Sourav Poddar <sourav.poddar@ti.com>
To: Lee Jones <lee.jones@linaro.org>
Cc: Mark Brown <broonie@kernel.org>, <angus.clark@st.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	David Woodhouse <dwmw2@infradead.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 02/10] mtd: st_spi_fsm: Supply all register address and bit logic defines
Date: Mon, 18 Nov 2013 23:29:21 +0530	[thread overview]
Message-ID: <528A5579.60705@ti.com> (raw)
In-Reply-To: <20131118173201.GP13640@lee--X1>

On Monday 18 November 2013 11:02 PM, Lee Jones wrote:
> On Mon, 18 Nov 2013, Mark Brown wrote:
>
>> On Mon, Nov 18, 2013 at 04:02:26PM +0000, Lee Jones wrote:
>>> On Mon, 18 Nov 2013, Mark Brown wrote:
>>>> Like I say I'm suggesting that the bit of the code that understands the
>>>> flash chip is separate to the bit of code that knows the mechanics of
>>>> sending commands and data to the chip.
>>> The issue is that almost the entire driver is controller side. The
>>> only bits that are the same (and not in all cases) are the OPCODEs,
>>> but they are one liners (21 lines out of 1153). Most of the
>>> controllers which use this stuff could reuse quite a bit of the m25p80
>>> driver as they just write the message containing the OPCODE as the
>>> m25p80 driver sets it up, but that's simply not the case with our
>>> controller. We would have to pull the OPCODE out and based on which
>>> one it is, we'd have to build our own message.
>> OK, so then perhaps the abstraction here is simply to export the table
>> with the opcodes from the m25p80 driver so that when someone comes along
>> and adds a new chip they can just add it there and other drivers will
>> get the update too.
> We could do that, although I'd have to insist on extending the current
> framework to add a configuration call-back, as it's the neatest way to
> configure chip specific attributes.
>
This looks like the problem which some other controllers(from ti,
freescale) are facing.
I will just summarise the problem with the ti qspi flash controller 
which I am
working on. There is a set of registers which need to be filled with flash
specific commands. One way to deal it with to provide a device tree bindings
for all the requirements(which is really cumbersome.).

Similarly, for freescale there is a LUT registers which has such flash 
requirements.

So, surely we need a way out from m25p80 driver to handle such cases.

drivers/mtd/devices/m25p80.c


int pass_flash_info () {

    u8 info[6];

      info[0] = DUMMY WR;
      info[1] = RD_OPCODE;
      info[2] - DUMMY BITS;
      ......
     .......
    spi_write(flash, info, 6)
}
Then, somehow parse this information to set up the required info.
  This is just a rough idea, and can be implemented in a better way.
>   I can get a patch out tomorrow if the MTD guys agree. Where are they
> by the way? I haven't seen hide nor hair of them since sending out the
> patch set.
>
>>> Put it this way, if we tried to use the m25p80 our controller driver
>>> would most likely be twice as large and twice as complex as it is
>>> currently, which is exactly the inverse of what we're trying to
>>> achieve here.
>> If we're having to add new flashes to multiple drivers I'd not say we're
>> winning.
> I agree.
>


  reply	other threads:[~2013-11-18 17:59 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-14 14:22 [PATCH 00/10] mtd: st_spi_fsm: Add new device Lee Jones
2013-11-14 14:22 ` Lee Jones
2013-11-14 14:22 ` Lee Jones
2013-11-14 14:22 ` [PATCH 01/10] mtd: st_spi_fsm: Allocate resources and register with MTD framework Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22 ` [PATCH 02/10] mtd: st_spi_fsm: Supply all register address and bit logic defines Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-18  9:15   ` Linus Walleij
2013-11-18  9:15     ` Linus Walleij
2013-11-18  9:15     ` Linus Walleij
2013-11-18  9:32     ` Lee Jones
2013-11-18  9:32       ` Lee Jones
2013-11-18  9:32       ` Lee Jones
2013-11-18 13:39       ` Mark Brown
2013-11-18 13:39         ` Mark Brown
2013-11-18 13:39         ` Mark Brown
2013-11-18 14:24         ` Lee Jones
2013-11-18 14:24           ` Lee Jones
2013-11-18 14:24           ` Lee Jones
2013-11-18 14:45           ` Mark Brown
2013-11-18 14:45             ` Mark Brown
2013-11-18 14:45             ` Mark Brown
2013-11-18 14:56             ` Lee Jones
2013-11-18 14:56               ` Lee Jones
2013-11-18 14:56               ` Lee Jones
2013-11-18 15:16               ` Mark Brown
2013-11-18 15:16                 ` Mark Brown
2013-11-18 15:16                 ` Mark Brown
2013-11-18 15:31                 ` Lee Jones
2013-11-18 15:31                   ` Lee Jones
2013-11-18 15:31                   ` Lee Jones
2013-11-18 15:41                   ` Mark Brown
2013-11-18 15:41                     ` Mark Brown
2013-11-18 15:41                     ` Mark Brown
2013-11-18 16:02                     ` Lee Jones
2013-11-18 16:02                       ` Lee Jones
2013-11-18 16:02                       ` Lee Jones
2013-11-18 17:22                       ` Mark Brown
2013-11-18 17:22                         ` Mark Brown
2013-11-18 17:22                         ` Mark Brown
2013-11-18 17:32                         ` Lee Jones
2013-11-18 17:32                           ` Lee Jones
2013-11-18 17:32                           ` Lee Jones
2013-11-18 17:59                           ` Sourav Poddar [this message]
2013-11-18 17:59                             ` Sourav Poddar
2013-11-18 17:59                             ` Sourav Poddar
2013-11-18 18:35                           ` Mark Brown
2013-11-18 18:35                             ` Mark Brown
2013-11-18 18:35                             ` Mark Brown
2013-11-14 14:22 ` [PATCH 03/10] mtd: st_spi_fsm: Initialise and configure the FSM for normal working conditions Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22 ` [PATCH 04/10] mtd: st_spi_fsm: Supply framework for device requests Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22 ` [PATCH 05/10] mtd: st_spi_fsm: Supply a method to read from the FSM's FIFO Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22 ` [PATCH 06/10] mtd: st_spi_fsm: Supply defines for the possible flash command opcodes Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22 ` [PATCH 07/10] mtd: st_spi_fsm: Add support for JEDEC ID extraction Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22 ` [PATCH 08/10] mtd: st_spi_fsm: Provide device look-up table Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22 ` [PATCH 09/10] mtd: st_spi_fsm: Dynamically setup flash device based on JEDEC ID Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22 ` [PATCH 10/10] ARM: STi: Add support for the FSM Serial Flash Controller Lee Jones
2013-11-14 14:22   ` Lee Jones
2013-11-14 14:22   ` 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=528A5579.60705@ti.com \
    --to=sourav.poddar@ti.com \
    --cc=angus.clark@st.com \
    --cc=broonie@kernel.org \
    --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 \
    /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.