linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Smartmedia/xd card support - request for comments
@ 2008-04-17  8:50 Alex Dubov
  2008-04-17  9:01 ` Ben Dooks
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Alex Dubov @ 2008-04-17  8:50 UTC (permalink / raw)
  To: Linux kernel mailing list

I've implemented, with generous help from JMicron, a native support for Smartmedia/xD picture card
media. Currently, only JMicron backend is available, but TI expressed some interest in this too,
so TI Flashmedia backend may soon follow.

Smartmedia cards are quite akin to the dumb flash chips, but they have their quirks that put them
aside as a separate media type.

I thought of the following merge structure:

include/linux/
    flash_bd.h
    xd_card.h

drivers/xd_card/
    xd_card_blk.c - protocol implementation
    xd_card_ecc.c - software version of Smartmedia ecc algorithm
    jmb38x_xd.c   - JMicron backend

lib/
    flash_bd.c    - generic FTL implementation

A few more words about flash_bd component. Legacy MemoryStick and SmartMedia cards require direct
management of flash memory. I studied the mtd layer, but found it to be not very helpful, so I
wrote my own FTL module.

Its requirements were:
1. Subsystem independent. For a given byte range it will produce a sequence of generic opcodes
needed to read or write such a range to/off the flash memory, assuming page grained access. This
is used not only for block device operations, but also for metadata retrieval.
2. No FTL related metadata is stored to the flash (as nothing can be assumed about other
readers/writers in interchange media formats).

My FTL implements random block replacement policy, as there's no spec-mandated way to maintain
wear counts on Smartmedia or Memorystick. It also supports block allocation zoning, as required by
these formats.

Current version will always issue whole block writes (no individual page programming). I,in fact,
implemented a version which tracks partially filled blocks, but experiments with some Olympus xd
cards triggered a lot of funny hardware problems with this, and then, it appeared that latest
version of xd card spec specifically disallows partial block writes.

MemoryStick spec does allow individual page writes, but so did an older version of xd card spec.
Given this, I decided to start with simpler version of FTL for now and replace it with a more
complex one if need arises.



      ____________________________________________________________________________________
Be a better friend, newshound, and 
know-it-all with Yahoo! Mobile.  Try it now.  http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-17  8:50 Smartmedia/xd card support - request for comments Alex Dubov
@ 2008-04-17  9:01 ` Ben Dooks
  2008-04-17  9:11   ` Alex Dubov
  2008-04-17 13:35 ` Thomas Gleixner
  2008-04-17 14:19 ` Jörn Engel
  2 siblings, 1 reply; 23+ messages in thread
From: Ben Dooks @ 2008-04-17  9:01 UTC (permalink / raw)
  To: Alex Dubov; +Cc: Linux kernel mailing list

On Thu, Apr 17, 2008 at 01:50:33AM -0700, Alex Dubov wrote:
> I've implemented, with generous help from JMicron, a native support for Smartmedia/xD picture card
> media. Currently, only JMicron backend is available, but TI expressed some interest in this too,
> so TI Flashmedia backend may soon follow.
> 
> Smartmedia cards are quite akin to the dumb flash chips, but they have their quirks that put them
> aside as a separate media type.

They're NAND chips, just with a standard ECC/block replacement
stratergy... why isn't this under drivers/mtd ?

-- 
Ben (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-17  9:01 ` Ben Dooks
@ 2008-04-17  9:11   ` Alex Dubov
  2008-04-17 13:10     ` Andy Lutomirski
  2008-04-17 13:38     ` Thomas Gleixner
  0 siblings, 2 replies; 23+ messages in thread
From: Alex Dubov @ 2008-04-17  9:11 UTC (permalink / raw)
  To: Ben Dooks; +Cc: Linux kernel mailing list


--- Ben Dooks <ben@fluff.org> wrote:

> On Thu, Apr 17, 2008 at 01:50:33AM -0700, Alex Dubov wrote:
> > I've implemented, with generous help from JMicron, a native support for Smartmedia/xD picture
> card
> > media. Currently, only JMicron backend is available, but TI expressed some interest in this
> too,
> > so TI Flashmedia backend may soon follow.
> > 
> > Smartmedia cards are quite akin to the dumb flash chips, but they have their quirks that put
> them
> > aside as a separate media type.
> 
> They're NAND chips, just with a standard ECC/block replacement
> stratergy... why isn't this under drivers/mtd ?
> 

They have nothing to do with JFFS or UBI (it's an interchange format).
They require FTL.
On the other hand, "SmartMedia reader" is a rather definite kind of animal that exists on its own.




      ____________________________________________________________________________________
Be a better friend, newshound, and 
know-it-all with Yahoo! Mobile.  Try it now.  http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-17  9:11   ` Alex Dubov
@ 2008-04-17 13:10     ` Andy Lutomirski
  2008-04-17 13:36       ` Arnd Bergmann
  2008-04-17 13:38     ` Thomas Gleixner
  1 sibling, 1 reply; 23+ messages in thread
From: Andy Lutomirski @ 2008-04-17 13:10 UTC (permalink / raw)
  To: Alex Dubov; +Cc: Ben Dooks, Linux kernel mailing list

Alex Dubov wrote:
> --- Ben Dooks <ben@fluff.org> wrote:
> 
>> On Thu, Apr 17, 2008 at 01:50:33AM -0700, Alex Dubov wrote:
>>> I've implemented, with generous help from JMicron, a native support for Smartmedia/xD picture
>> card
>>> media. Currently, only JMicron backend is available, but TI expressed some interest in this
>> too,
>>> so TI Flashmedia backend may soon follow.
>>>
>>> Smartmedia cards are quite akin to the dumb flash chips, but they have their quirks that put
>> them
>>> aside as a separate media type.
>> They're NAND chips, just with a standard ECC/block replacement
>> stratergy... why isn't this under drivers/mtd ?
>>
> 
> They have nothing to do with JFFS or UBI (it's an interchange format).
> They require FTL.

Or does using them according to the interchange spec require FTL?

(That is, it would be really cool to finally have a supported MTD device 
that doesn't need to be soldered on to the motherboard, so I could plug 
the thing in and run UBIFS / JFFS2 / whatever.  I wouldn't be able to 
use it for a camera, then, but it would be great for little machines 
that don't need full hard drives, or for developing MTD filesystems on 
readily-available hardware.)

--Andy

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-17  8:50 Smartmedia/xd card support - request for comments Alex Dubov
  2008-04-17  9:01 ` Ben Dooks
@ 2008-04-17 13:35 ` Thomas Gleixner
  2008-04-17 14:19 ` Jörn Engel
  2 siblings, 0 replies; 23+ messages in thread
From: Thomas Gleixner @ 2008-04-17 13:35 UTC (permalink / raw)
  To: Alex Dubov; +Cc: Linux kernel mailing list

On Thu, 17 Apr 2008, Alex Dubov wrote:

> I've implemented, with generous help from JMicron, a native support
> for Smartmedia/xD picture card media. Currently, only JMicron
> backend is available, but TI expressed some interest in this too, so
> TI Flashmedia backend may soon follow.

> Smartmedia cards are quite akin to the dumb flash chips, but they
> have their quirks that put them aside as a separate media type.

Smartmedia cards _ARE_ dumb NAND flash chips in a thin plastic cover
with gold contacts. The access to those cards is supported in the mtd
subsystem since ages. Why are you trying to reinvent the wheel ?

The only missing piece is the FTL layer, but there was a halfways
working version in the old mtd CVS which should be still available.
 
> management of flash memory. I studied the mtd layer, but found it to
> be not very helpful, so I wrote my own FTL module.

Err, why is MTD not helpful ? It covers all the low level access to
the NAND Flash and has a flexible interface to adapt to hardware.

We really do not need a second incarnation of NAND flash software in
the kernel.

Thanks,
	tglx

 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-17 13:10     ` Andy Lutomirski
@ 2008-04-17 13:36       ` Arnd Bergmann
  0 siblings, 0 replies; 23+ messages in thread
From: Arnd Bergmann @ 2008-04-17 13:36 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Alex Dubov, Ben Dooks, Linux kernel mailing list, Jörn Engel

On Thursday 17 April 2008, Andy Lutomirski wrote:
> > 
> > They have nothing to do with JFFS or UBI (it's an interchange format).
> > They require FTL.
> 
> Or does using them according to the interchange spec require FTL?
> 
> (That is, it would be really cool to finally have a supported MTD device 
> that doesn't need to be soldered on to the motherboard, so I could plug 
> the thing in and run UBIFS / JFFS2 / whatever.  I wouldn't be able to 
> use it for a camera, then, but it would be great for little machines 
> that don't need full hard drives, or for developing MTD filesystems on 
> readily-available hardware.)

There is one MTD driver for Smartmedia/xD in the kernel, see
drivers/mtd/nand/alauda.c. It is derived from the USB mass storage
driver drivers/usb/storage/alauda.c, which contains another copy of
the SM FTL.

	Arnd <><

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-17  9:11   ` Alex Dubov
  2008-04-17 13:10     ` Andy Lutomirski
@ 2008-04-17 13:38     ` Thomas Gleixner
  2008-04-17 14:07       ` Jörn Engel
  1 sibling, 1 reply; 23+ messages in thread
From: Thomas Gleixner @ 2008-04-17 13:38 UTC (permalink / raw)
  To: Alex Dubov; +Cc: Ben Dooks, Linux kernel mailing list

On Thu, 17 Apr 2008, Alex Dubov wrote:
> --- Ben Dooks <ben@fluff.org> wrote:
> 
> > On Thu, Apr 17, 2008 at 01:50:33AM -0700, Alex Dubov wrote:
> > > I've implemented, with generous help from JMicron, a native support for Smartmedia/xD picture
> > card
> > > media. Currently, only JMicron backend is available, but TI expressed some interest in this
> > too,
> > > so TI Flashmedia backend may soon follow.
> > > 
> > > Smartmedia cards are quite akin to the dumb flash chips, but they have their quirks that put
> > them
> > > aside as a separate media type.
> > 
> > They're NAND chips, just with a standard ECC/block replacement
> > stratergy... why isn't this under drivers/mtd ?
> > 
> 
> They have nothing to do with JFFS or UBI (it's an interchange
> format).  They require FTL. 

MTD is independent of JFFS2 and UBI. It's the hardware abtraction
layer on which you build an FTL.

> On the other hand, "SmartMedia reader"
> is a rather definite kind of animal that exists on its own.

No it is not. SmardMedia is a bare NAND chip, which is already covered
by MTD. There are already other FTLs on top of MTD and the SmartMedia
format just can be added to those.

Thanks,
	tglx


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-17 13:38     ` Thomas Gleixner
@ 2008-04-17 14:07       ` Jörn Engel
  2008-04-17 19:11         ` Thomas Gleixner
  0 siblings, 1 reply; 23+ messages in thread
From: Jörn Engel @ 2008-04-17 14:07 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Alex Dubov, Ben Dooks, Linux kernel mailing list

On Thu, 17 April 2008 15:38:17 +0200, Thomas Gleixner wrote:
> 
> No it is not. SmardMedia is a bare NAND chip, which is already covered
> by MTD. There are already other FTLs on top of MTD and the SmartMedia
> format just can be added to those.

Isn't drivers/mtd/ssfdc.c - despite its name - an implementation of the
SmartMedia FTL?  When I last looked at it, I failed to see any
functional differences.

Jörn

-- 
Doubt is not a pleasant condition, but certainty is an absurd one.
-- Voltaire

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-17  8:50 Smartmedia/xd card support - request for comments Alex Dubov
  2008-04-17  9:01 ` Ben Dooks
  2008-04-17 13:35 ` Thomas Gleixner
@ 2008-04-17 14:19 ` Jörn Engel
  2 siblings, 0 replies; 23+ messages in thread
From: Jörn Engel @ 2008-04-17 14:19 UTC (permalink / raw)
  To: Alex Dubov; +Cc: Linux kernel mailing list

On Thu, 17 April 2008 01:50:33 -0700, Alex Dubov wrote:
> 
> Current version will always issue whole block writes (no individual page programming). I,in fact,
> implemented a version which tracks partially filled blocks, but experiments with some Olympus xd
> cards triggered a lot of funny hardware problems with this, and then, it appeared that latest
> version of xd card spec specifically disallows partial block writes.

drivers/mtd/nand/alauda.c already does page writes.  You didn't know the
existing wheel and invented new one.  That happens and there is nothing
wrong with it.

Your chances of merging your new code as-is are roughly zero.  But I
would still like to see the code and see where yours is better and ours
should be improved.  Or you could take a look yourself and send patches.

> MemoryStick spec does allow individual page writes, but so did an older version of xd card spec.
> Given this, I decided to start with simpler version of FTL for now and replace it with a more
> complex one if need arises.

Are Memory Sticks covered by your code?  That would be a worthwhile
improvement.

Jörn

-- 
Joern's library part 3:
http://inst.eecs.berkeley.edu/~cs152/fa05/handouts/clark-test.pdf

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-17 14:07       ` Jörn Engel
@ 2008-04-17 19:11         ` Thomas Gleixner
  2008-04-17 21:21           ` Jörn Engel
  0 siblings, 1 reply; 23+ messages in thread
From: Thomas Gleixner @ 2008-04-17 19:11 UTC (permalink / raw)
  To: Jörn Engel; +Cc: Alex Dubov, Ben Dooks, Linux kernel mailing list

[-- Attachment #1: Type: TEXT/PLAIN, Size: 751 bytes --]

On Thu, 17 Apr 2008, Jörn Engel wrote:
> On Thu, 17 April 2008 15:38:17 +0200, Thomas Gleixner wrote:
> > 
> > No it is not. SmardMedia is a bare NAND chip, which is already covered
> > by MTD. There are already other FTLs on top of MTD and the SmartMedia
> > format just can be added to those.
> 
> Isn't drivers/mtd/ssfdc.c - despite its name - an implementation of the
> SmartMedia FTL?  When I last looked at it, I failed to see any
> functional differences.

Oops. I did not bother to look into drivers/mtd and for some reason it
slipped out of my memory that it got mainlined. It's read only, but
the missing bits to make it r/w are not that hard. At least less code
than duplicating code all over the place for no good reason.

Thanks,
	tglx


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-17 19:11         ` Thomas Gleixner
@ 2008-04-17 21:21           ` Jörn Engel
  2008-04-18  8:47             ` Alex Dubov
  0 siblings, 1 reply; 23+ messages in thread
From: Jörn Engel @ 2008-04-17 21:21 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Alex Dubov, Ben Dooks, Linux kernel mailing list

On Thu, 17 April 2008 21:11:30 +0200, Thomas Gleixner wrote:
> 
> Oops. I did not bother to look into drivers/mtd and for some reason it
> slipped out of my memory that it got mainlined. It's read only, but
> the missing bits to make it r/w are not that hard. At least less code
> than duplicating code all over the place for no good reason.

Indeed.  If some janitor or kernel-newbie is looking for a fun project,
most of the code in drivers/usb/storage/*.c can be abstracted out to a
library implementation and possibly combined with drivers/mtd/ssfdc.c,
drivers/mtd/nand/alauda.c and some others.

Jörn

-- 
The only good bug is a dead bug.
-- Starship Troopers

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-17 21:21           ` Jörn Engel
@ 2008-04-18  8:47             ` Alex Dubov
  2008-04-18  9:35               ` Thomas Gleixner
  2008-04-18 14:00               ` Jörn Engel
  0 siblings, 2 replies; 23+ messages in thread
From: Alex Dubov @ 2008-04-18  8:47 UTC (permalink / raw)
  To: linux-kernel; +Cc: joern, tglx, ben

Unfortunately, I must say, Smartmedia is not supported on linux for any reasonable definition of
the word.

First, with all the respect to Jorn, alauda driver can only be considered "proof of concept". It
does not try to abstract any smartmedia functionality.

Second, ssfdc is hopelessly obsolete and requires rewrite.

Third, there's no attempt made by mtd to support advanced functionality present in many smartmedia
and all memorystick readers: adapter side copy and multi page programming (this mostly relates to
memorystick, of course). I also failed to see any support for device writing policy, needed to
discern, for example, sequential page programmable devices vs. block programmable devices. I also
failed to see an unified approach to page-accessible devices, meaning duplication of "bouncing"
code in the backends.

In general, block device emulation layer of mtd is incomplete at best.

No infrastructure exists for exporting and manipulation of metadata in userspace.

And, in general, architecture of mtd is suboptimal, relying on blocking calls (as opposed to
callback based architecture).

Considering all this, amount of effort needed for satisfactory support of smartmedia through mtd
was found by me to be far greater than coming with stand-alone implementation. I do agree, that
eventually my work can be merged into mtd. I don't see what prevents merging of my implementation
as is on an interim basis, considering there are no real alternatives.

In case somebody is actually interested in looking at the code:

http://svn.berlios.de/wsvn/tifmxx/trunk/driver/#_trunk_driver_



      ____________________________________________________________________________________
Be a better friend, newshound, and 
know-it-all with Yahoo! Mobile.  Try it now.  http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-18  8:47             ` Alex Dubov
@ 2008-04-18  9:35               ` Thomas Gleixner
  2008-04-19  2:49                 ` Alex Dubov
  2008-04-18 14:00               ` Jörn Engel
  1 sibling, 1 reply; 23+ messages in thread
From: Thomas Gleixner @ 2008-04-18  9:35 UTC (permalink / raw)
  To: Alex Dubov; +Cc: linux-kernel, joern, ben

On Fri, 18 Apr 2008, Alex Dubov wrote:
>
> First, with all the respect to Jorn, alauda driver can only be
> considered "proof of concept". It does not try to abstract any
> smartmedia functionality.
> 
> Second, ssfdc is hopelessly obsolete and requires rewrite.

Nobody claimed that it is perfect.

> Third, there's no attempt made by mtd to support advanced
> functionality present in many smartmedia and all memorystick
> readers: adapter side copy and multi page programming (this mostly
> relates to memorystick, of course). I also failed to see any support
> for device writing policy, needed to discern, for example,
> sequential page programmable devices vs. block programmable
> devices. I also failed to see an unified approach to page-accessible
> devices, meaning duplication of "bouncing" code in the backends.

And instead of addressing the missing features and shortcomings you
add a new duplicated code layer which is only useful for a restricted
set of hardware.

> Considering all this, amount of effort needed for satisfactory
> support of smartmedia through mtd was found by me to be far greater
> than coming with stand-alone implementation.

Yeah and you made this decision on your own w/o even discussing the
issues at hand with the mtd developers before implementing a separate
code stack. And now we should be impressed and merge it. That's not
the way it works.

> I do agree, that eventually my work can be merged into mtd. I don't
> see what prevents merging of my implementation as is on an interim
> basis, considering there are no real alternatives.

Merging your code as is is a bad idea as it contains userspace visible
changes which can not be undone easily. Interim solutions burden more
problems on us than they solve.

Thanks,
	tglx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-18  8:47             ` Alex Dubov
  2008-04-18  9:35               ` Thomas Gleixner
@ 2008-04-18 14:00               ` Jörn Engel
  2008-04-19  3:05                 ` Alex Dubov
  1 sibling, 1 reply; 23+ messages in thread
From: Jörn Engel @ 2008-04-18 14:00 UTC (permalink / raw)
  To: Alex Dubov; +Cc: linux-kernel, tglx, ben

On Fri, 18 April 2008 01:47:12 -0700, Alex Dubov wrote:
> 
> First, with all the respect to Jorn, alauda driver can only be considered "proof of concept". It
> does not try to abstract any smartmedia functionality.

"Proof of concept" is the wrong term.  The goal of
drivers/mtd/nand/alauda.c is to get raw access to the flash.  It
explicitly does not abstract anything because I don't want the
abstraction.  I want a raw piece of flash, represented via mtd.  And I
believe it does match those goals - modulo some bugs.

You seem to want something else.  I have a rough idea what that might
be, nothing more.  Can you explain your goals to give us a better idea?

> Second, ssfdc is hopelessly obsolete and requires rewrite.

Not unusual.

> No infrastructure exists for exporting and manipulation of metadata in userspace.

When using the raw mtd, it does.  After block device translation, it
doesn't.

> And, in general, architecture of mtd is suboptimal, relying on blocking calls (as opposed to
> callback based architecture).

Correct.  Erase() is the only callback-based function and every single
driver implements it synchronous anyway.  Improving this situation is
needed for mtd's own sake anyway.  Either that or replacing mtd with
something else, likely the block device code.

> Considering all this, amount of effort needed for satisfactory support of smartmedia through mtd
> was found by me to be far greater than coming with stand-alone implementation. I do agree, that
> eventually my work can be merged into mtd. I don't see what prevents merging of my implementation
> as is on an interim basis, considering there are no real alternatives.

As Thomas already stated, the kernel crowd has little sympathy for
duplication.  In general, if someone comes along and sais "A if not good
enough, so I wrote B", the answer is "go fix A instead".  Not always in
a polite way, I might add.

Sometimes there are good reasons not to fix A.  Usually the reason is to
remove A after B has been merged.  Which does not apply here.  So you
should prepare to explain yourself rather well, if you ever want to see
your code merged.

That said, I personally like duplication and believe it is the way to go
in many causes.  If your code can ignore all the issues with A, you can
focus on the problem at hand and write the best code possible.  But that
is just the first step.  In a second step, any common code of the two
pieces gets generalized and used by both.

The reason why I prefer generalizing late rather than early is that
almost noone gets the abstractions right before seeing all users.
Certainly not me.  The end result is better by being specialized first
and generalize later.  But you have to do the generalization step.

And your chances of getting the code merged before the generalization
are low.  Particularly when adding new userspace interfaces, because we
cannot remove those in a later stage.

> In case somebody is actually interested in looking at the code:
> 
> http://svn.berlios.de/wsvn/tifmxx/trunk/driver/#_trunk_driver_

Sure am.  You should use explicitly sized types in most of your data
structures, like here:
struct xd_card_extra {
        unsigned int   reserved;
        unsigned char  data_status;
        unsigned char  block_status;
        unsigned short addr1;
        unsigned char  ecc_hi[3];
        unsigned short addr2; /* this should be identical to addr1 */
        unsigned char  ecc_lo[3];
} __attribute__((packed));

And if you want to be pre-flamed, running things through checkpatch.pl
lets you take the pain early.

Jörn

-- 
Joern's library part 6:
http://www.gzip.org/zlib/feldspar.html

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-18  9:35               ` Thomas Gleixner
@ 2008-04-19  2:49                 ` Alex Dubov
  2008-04-19  5:56                   ` Thomas Gleixner
  0 siblings, 1 reply; 23+ messages in thread
From: Alex Dubov @ 2008-04-19  2:49 UTC (permalink / raw)
  To: linux-kernel; +Cc: tglx, joern, ben

> > Third, there's no attempt made by mtd to support advanced
> > functionality present in many smartmedia and all memorystick
> > readers: adapter side copy and multi page programming (this mostly
> > relates to memorystick, of course). I also failed to see any support
> > for device writing policy, needed to discern, for example,
> > sequential page programmable devices vs. block programmable
> > devices. I also failed to see an unified approach to page-accessible
> > devices, meaning duplication of "bouncing" code in the backends.
> 
> And instead of addressing the missing features and shortcomings you
> add a new duplicated code layer which is only useful for a restricted
> set of hardware.
> 
> > Considering all this, amount of effort needed for satisfactory
> > support of smartmedia through mtd was found by me to be far greater
> > than coming with stand-alone implementation.
> 
> Yeah and you made this decision on your own w/o even discussing the
> issues at hand with the mtd developers before implementing a separate
> code stack. And now we should be impressed and merge it. That's not
> the way it works.
> 

I do not expect you, or anybody, to be "impressed" with anything. As I said before, from my
analysis of mtd it will require a great deal of patching to acquire certain functionality. This
will invariably require a large time span, as many people depend on the mtd in it present state.

If your position on the proposed interim solution is definite "no", I'll make an effort to rewrite
the driver in the mtd-friendly way.

This brings me to the first technical question:

mtd_blkdevs breaks the block layer request into separate pages, effectively disabling lower layer
from making decision based on a real data block size, and using dma to any useful extent. What is
the proposed way to deal with this?

(By the way, first question already puts us into a "quirk" area of the statement "smartmedia are
flash chips with quirks". Both memorystick and smartmedia are always operated in tandem with their
controllers, as you could notice.)



      ____________________________________________________________________________________
Be a better friend, newshound, and 
know-it-all with Yahoo! Mobile.  Try it now.  http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-18 14:00               ` Jörn Engel
@ 2008-04-19  3:05                 ` Alex Dubov
  2008-04-19  6:37                   ` Thomas Gleixner
  0 siblings, 1 reply; 23+ messages in thread
From: Alex Dubov @ 2008-04-19  3:05 UTC (permalink / raw)
  To: joern; +Cc: linux-kernel, tglx, ben


--- Jörn Engel <joern@logfs.org> wrote:

> On Fri, 18 April 2008 01:47:12 -0700, Alex Dubov wrote:
> > 
> > First, with all the respect to Jorn, alauda driver can only be considered "proof of concept".
> It
> > does not try to abstract any smartmedia functionality.
> 
> "Proof of concept" is the wrong term.  The goal of
> drivers/mtd/nand/alauda.c is to get raw access to the flash.  It
> explicitly does not abstract anything because I don't want the
> abstraction.  I want a raw piece of flash, represented via mtd.  And I
> believe it does match those goals - modulo some bugs.
> 
> You seem to want something else.  I have a rough idea what that might
> be, nothing more.  Can you explain your goals to give us a better idea?

As you understand, both smartmedia media and hosts follow certain spec (publicly available here:
http://www.ssfdc.or.jp/spec/index_e.htm, basic registration required). xd card spec is almost
identical, adding some cosmetic features and restrictions (such as prohibition of single page
programming). This means, that smartmedia access protocol can be abstracted out in the
reader-independent way. You can seen in the xd_card_blk.c that it can operate both completely dumb
controllers, while taking protocol shortcuts (using host->caps) to accommodate smarter readers
(the only feature currently not there is adapter-side page copy - it can be easily added under
FBD_COPY clause in xd_card_trans_req). The backend itself (jmb38x_xd.c) should not concern itself
with details of smartmedia spec, as it will directly lead to code duplication among backends.
> 
> > No infrastructure exists for exporting and manipulation of metadata in userspace.
> 
> When using the raw mtd, it does.  After block device translation, it
> doesn't.

I meant here the ability of user to look and modify CIS page, as well as look at the current block
translation table.

> 
> Correct.  Erase() is the only callback-based function and every single
> driver implements it synchronous anyway.  Improving this situation is
> needed for mtd's own sake anyway.  Either that or replacing mtd with
> something else, likely the block device code.

Revamping the underlying architecture of mtd will break a lot of stuff in the process, doesn't it?

> > In case somebody is actually interested in looking at the code:
> > 
> > http://svn.berlios.de/wsvn/tifmxx/trunk/driver/#_trunk_driver_
> 
> Sure am.  You should use explicitly sized types in most of your data
> structures, like here:
> struct xd_card_extra {
>         unsigned int   reserved;
>         unsigned char  data_status;
>         unsigned char  block_status;
>         unsigned short addr1;
>         unsigned char  ecc_hi[3];
>         unsigned short addr2; /* this should be identical to addr1 */
>         unsigned char  ecc_lo[3];
> } __attribute__((packed));
> 
> And if you want to be pre-flamed, running things through checkpatch.pl
> lets you take the pain early.
> 

I was not moving to submit just yet. As it's not my first submission, I "prefetched" some time for
an usual flame war.



      ____________________________________________________________________________________
Be a better friend, newshound, and 
know-it-all with Yahoo! Mobile.  Try it now.  http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-19  2:49                 ` Alex Dubov
@ 2008-04-19  5:56                   ` Thomas Gleixner
  0 siblings, 0 replies; 23+ messages in thread
From: Thomas Gleixner @ 2008-04-19  5:56 UTC (permalink / raw)
  To: Alex Dubov; +Cc: linux-kernel, joern, ben

On Fri, 18 Apr 2008, Alex Dubov wrote:

> mtd_blkdevs breaks the block layer request into separate pages,
> effectively disabling lower layer from making decision based on a
> real data block size, and using dma to any useful extent. What is
> the proposed way to deal with this?

Wrong in two aspects. 

1) it does not break the request into separate pages. It breaks it
into mtd_blktrans_ops->blksize chunks, which is set by the blkdev
user.

2) Using DMA on raw NAND Flash is on per page basis except you have an
"intellegent" controller chip which does the low level handling of the
nand flash itself. Such controllers do not necessarily fit into MTD.

> (By the way, first question already puts us into a "quirk" area of
> the statement "smartmedia are flash chips with quirks". Both
> memorystick and smartmedia are always operated in tandem with their
> controllers, as you could notice.)

I have no idea what you want to tell me. there is no quirk in a
smartmedia card.

Thanks,
	tglx



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-19  3:05                 ` Alex Dubov
@ 2008-04-19  6:37                   ` Thomas Gleixner
  2008-04-19 16:35                     ` Jörn Engel
  2008-04-20  2:25                     ` Alex Dubov
  0 siblings, 2 replies; 23+ messages in thread
From: Thomas Gleixner @ 2008-04-19  6:37 UTC (permalink / raw)
  To: Alex Dubov; +Cc: joern, linux-kernel, ben

On Fri, 18 Apr 2008, Alex Dubov wrote:

First off, can you please use a mail client which does line breaks at
around 78 chars ?
 
> As you understand, both smartmedia media and hosts follow certain
> spec (publicly available here:
> http://www.ssfdc.or.jp/spec/index_e.htm, basic registration
> required). xd card spec is almost identical, adding some cosmetic
> features and restrictions (such as prohibition of single page
> programming). This means, that smartmedia access protocol can be
> abstracted out in the reader-independent way. You can seen in the
> xd_card_blk.c that it can operate both completely dumb controllers,
> while taking protocol shortcuts (using host->caps) to accommodate
> smarter readers (the only feature currently not there is
> adapter-side page copy - it can be easily added under FBD_COPY
> clause in xd_card_trans_req). The backend itself (jmb38x_xd.c)
> should not concern itself with details of smartmedia spec, as it
> will directly lead to code duplication among backends.

That's all fine. Nobody asked for the backend to know about smartmedia
on FLASH format details. The MTD NAND FLASH layer does not care about
on FLASH data format at all. It provides merily access on the hardware
level.

I agree that your approach of making the SM/XD layer capable of
handling clever and dumb hardware adapters is a good one.

The point where I stringly disagree is that your implementation is
forcing people with dumb hardware (see drivers/mtd/nand/*) to
reimplement the (maybe already) existing drivers in order to make use
of the ssfdc format, which will not work with the following real world
example:

------------------
| NAND controller |----------| On board NAND
| with HW ECC     |
|                 |----------| SmartMedia Card
------------------

Having a separate driver infrastructure for clever controllers, which
provide only semi raw access to the FLASH chip is fine simply because
such controllers do not fit into the MTD layer.

> I meant here the ability of user to look and modify CIS page, as
> well as look at the current block translation table.

Well, you probably do this with an iotctl anyway. So what's preventing
you to modify the mtd block layer to handle client private ioctls.

Also there is no real requirement to use the mtd block layer at all
for that. Putting your layer on top of the raw MTD device is fine IMO.

> > Correct.  Erase() is the only callback-based function and every single
> > driver implements it synchronous anyway.  Improving this situation is
> > needed for mtd's own sake anyway.  Either that or replacing mtd with
> > something else, likely the block device code.
>
> Revamping the underlying architecture of mtd will break a lot of
> stuff in the process, doesn't it?

That's the best excuse I ever heard for not improving code which has
shortcomings.

Thanks,
	tglx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-19  6:37                   ` Thomas Gleixner
@ 2008-04-19 16:35                     ` Jörn Engel
  2008-04-20  2:25                     ` Alex Dubov
  1 sibling, 0 replies; 23+ messages in thread
From: Jörn Engel @ 2008-04-19 16:35 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Alex Dubov, linux-kernel, ben

On Sat, 19 April 2008 08:37:59 +0200, Thomas Gleixner wrote:
> On Fri, 18 Apr 2008, Alex Dubov wrote:
> 
> First off, can you please use a mail client which does line breaks at
> around 78 chars ?

It would also be nice if it wouldn't drop the name "Jörn Engel" from my
mail (indication for spam, mail ends in a different folder), kept a
References: or In-Reply-To: header and could deal with utf8.

Jörn

-- 
Fancy algorithms are buggier than simple ones, and they're much harder
to implement. Use simple algorithms as well as simple data structures.
-- Rob Pike

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-19  6:37                   ` Thomas Gleixner
  2008-04-19 16:35                     ` Jörn Engel
@ 2008-04-20  2:25                     ` Alex Dubov
  1 sibling, 0 replies; 23+ messages in thread
From: Alex Dubov @ 2008-04-20  2:25 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: joern, linux-kernel, ben

> 
> That's all fine. Nobody asked for the backend to know about smartmedia
> on FLASH format details. The MTD NAND FLASH layer does not care about
> on FLASH data format at all. It provides merily access on the hardware
> level.
> 
> I agree that your approach of making the SM/XD layer capable of
> handling clever and dumb hardware adapters is a good one.
> 
> The point where I stringly disagree is that your implementation is
> forcing people with dumb hardware (see drivers/mtd/nand/*) to
> reimplement the (maybe already) existing drivers in order to make use
> of the ssfdc format, which will not work with the following real world
> example:
> 
> ------------------
> | NAND controller |----------| On board NAND
> | with HW ECC     |
> |                 |----------| SmartMedia Card
> ------------------
> 
> Having a separate driver infrastructure for clever controllers, which
> provide only semi raw access to the FLASH chip is fine simply because
> such controllers do not fit into the MTD layer.

Shouldn't both smartmedia and legacy memorystick controllers can be considered
"semi-raw"? In second case, media itself implements "semi-raw" flash interface.

> 
> > I meant here the ability of user to look and modify CIS page, as
> > well as look at the current block translation table.
> 
> Well, you probably do this with an iotctl anyway. So what's preventing
> you to modify the mtd block layer to handle client private ioctls.

I think ioctls are nowhere as convenient as sysfs file. I believe many people
share this point of view - after all, ioctls throw a lot of different
functionality into one basket, using cryptic codes.

> 
> Also there is no real requirement to use the mtd block layer at all
> for that. Putting your layer on top of the raw MTD device is fine IMO.

I see that raw mtd operates on buffer pointers, rather than scatterlists. From
my understanding, it demands "map" and possibly "bounce" operation elsewhere,
incurring sizable overhead on performance (for fast media, the waits in the
blocking read/write operations will also have considerable effect). Are there
any plans to address this issue?

> >
> > Revamping the underlying architecture of mtd will break a lot of
> > stuff in the process, doesn't it?
> 
> That's the best excuse I ever heard for not improving code which has
> shortcomings.
> 

Can't agree. It's one of the most commonly used, according to my experience.



      ____________________________________________________________________________________
Be a better friend, newshound, and 
know-it-all with Yahoo! Mobile.  Try it now.  http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-20 16:01 ` Jörn Engel
@ 2008-04-21  1:33   ` Alex Dubov
  0 siblings, 0 replies; 23+ messages in thread
From: Alex Dubov @ 2008-04-21  1:33 UTC (permalink / raw)
  To: Linux Kernel list

> >
> > I wonder if mtd shouldn't modified to handler more clever controller :
> > I saw some raw nand controller where you can program a batch of 
> > command/read/write/ecc checking (that work on a internal RAM of the 
> > controller (and may be use DMA)) and all you have to do is to wait the 
> > controller do the job.
> > These controllers are very hard to integrate in current nand layer 
> > because it is very low level controller oriented.
> 
> The obvious solution is to have seperate io scheduling from io
> completion.  Requires an asynchronous interface to read/write and some
> sort of request queue.
> 
> Not quite as obvious is the question of whether to add all this to mtd
> or to replace mtd with block devices, which already have it.
> 

I was thinking about it myself and it appears to be the only solution outright.

The interesting, but apparently hard way to do so is to define a couple of
REQ_TYPE_SPECIAL mtd-specific commands and use the existing block device queue
to manipulate them. This can even open the possibility of "static" request
translation: mtd_blkdev can pop the block device request, put it through media
specific translator and then push a necessary set of mtd operations back into
the same queue, delegating the processing to raw mtd device.

The simpler approach I'm following with my xd_card_blk is to define a
simplistic, block device like api. First, the translator is initialized with
the request range (offset + length) and backend is notified to start
processing. The backend then pulls flash requests (READ, WRITE, COPY, ERASE)
from the translator, possibly putting them through additional, protocol/host
specific translation layer.



      ____________________________________________________________________________________
Be a better friend, newshound, and 
know-it-all with Yahoo! Mobile.  Try it now.  http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
  2008-04-19  8:31 matthieu castet
@ 2008-04-20 16:01 ` Jörn Engel
  2008-04-21  1:33   ` Alex Dubov
  0 siblings, 1 reply; 23+ messages in thread
From: Jörn Engel @ 2008-04-20 16:01 UTC (permalink / raw)
  To: matthieu castet; +Cc: Linux Kernel list, Thomas Gleixner, Alex Dubov

On Sat, 19 April 2008 10:31:51 +0200, matthieu castet wrote:
>
> I wonder if mtd shouldn't modified to handler more clever controller :
> I saw some raw nand controller where you can program a batch of 
> command/read/write/ecc checking (that work on a internal RAM of the 
> controller (and may be use DMA)) and all you have to do is to wait the 
> controller do the job.
> These controllers are very hard to integrate in current nand layer 
> because it is very low level controller oriented.

The obvious solution is to have seperate io scheduling from io
completion.  Requires an asynchronous interface to read/write and some
sort of request queue.

Not quite as obvious is the question of whether to add all this to mtd
or to replace mtd with block devices, which already have it.

Jörn

-- 
Never argue with idiots - first they drag you down to their level,
then they beat you with experience.
-- unknown

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Smartmedia/xd card support - request for comments
@ 2008-04-19  8:31 matthieu castet
  2008-04-20 16:01 ` Jörn Engel
  0 siblings, 1 reply; 23+ messages in thread
From: matthieu castet @ 2008-04-19  8:31 UTC (permalink / raw)
  To: Linux Kernel list; +Cc: Thomas Gleixner, Alex Dubov

Hi,

> On Fri, 18 Apr 2008, Alex Dubov wrote:
> 
> First off, can you please use a mail client which does line breaks at
> around 78 chars ?
>  
>> As you understand, both smartmedia media and hosts follow certain
>> spec (publicly available here:
>> http://www.ssfdc.or.jp/spec/index_e.htm, basic registration
>> required). xd card spec is almost identical, adding some cosmetic
>> features and restrictions (such as prohibition of single page
>> programming). This means, that smartmedia access protocol can be
>> abstracted out in the reader-independent way. You can seen in the
>> xd_card_blk.c that it can operate both completely dumb controllers,
>> while taking protocol shortcuts (using host->caps) to accommodate
>> smarter readers (the only feature currently not there is
>> adapter-side page copy - it can be easily added under FBD_COPY
>> clause in xd_card_trans_req). The backend itself (jmb38x_xd.c)
BTW some bare nand flash have support to page copy (there is some issue 
because of no ecc checking during the copy). So the support of this 
could be usefull in mtd.


> Having a separate driver infrastructure for clever controllers, which
> provide only semi raw access to the FLASH chip is fine simply because
> such controllers do not fit into the MTD layer.
I wonder if mtd shouldn't modified to handler more clever controller :
I saw some raw nand controller where you can program a batch of 
command/read/write/ecc checking (that work on a internal RAM of the 
controller (and may be use DMA)) and all you have to do is to wait the 
controller do the job.
These controllers are very hard to integrate in current nand layer 
because it is very low level controller oriented.

>> > Correct.  Erase() is the only callback-based function and every single
>> > driver implements it synchronous anyway.  Improving this situation is
>> > needed for mtd's own sake anyway.  Either that or replacing mtd with
>> > something else, likely the block device code.
>>
Current mtd have other limitations (4GB limit fix, dma buffer, ...),
but in the end it should be fixed.


Matthieu

PS : by using mtd layer you :
- will make improvement that will be useful for everybody
- will use a stack that have been and will be well tested
- will have some controller drivers for free
- will have nand fs for free (for people that don't trust FTL).
- may be provide interesting FTL for raw nand controller

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2008-04-21  1:33 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-04-17  8:50 Smartmedia/xd card support - request for comments Alex Dubov
2008-04-17  9:01 ` Ben Dooks
2008-04-17  9:11   ` Alex Dubov
2008-04-17 13:10     ` Andy Lutomirski
2008-04-17 13:36       ` Arnd Bergmann
2008-04-17 13:38     ` Thomas Gleixner
2008-04-17 14:07       ` Jörn Engel
2008-04-17 19:11         ` Thomas Gleixner
2008-04-17 21:21           ` Jörn Engel
2008-04-18  8:47             ` Alex Dubov
2008-04-18  9:35               ` Thomas Gleixner
2008-04-19  2:49                 ` Alex Dubov
2008-04-19  5:56                   ` Thomas Gleixner
2008-04-18 14:00               ` Jörn Engel
2008-04-19  3:05                 ` Alex Dubov
2008-04-19  6:37                   ` Thomas Gleixner
2008-04-19 16:35                     ` Jörn Engel
2008-04-20  2:25                     ` Alex Dubov
2008-04-17 13:35 ` Thomas Gleixner
2008-04-17 14:19 ` Jörn Engel
2008-04-19  8:31 matthieu castet
2008-04-20 16:01 ` Jörn Engel
2008-04-21  1:33   ` Alex Dubov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).