Memstick patches for 2.6.39
Subsystems (3)
Maxim Levitsky
maintainercover Memstick patches for 2.6.39
Hi,
This is a repost of my patches for 2.6.39 inclusion, which I hope not to miss this time.
I addressed the comments on the scatterlist issues.
Andrew, please note that my richoh memstick driver is standalone, unchanged from previos versions has many users which use the version I posted at ubuntu's Launchpad and happy with it. Please include it regardless of other patches.
The other half of my work is support for legacy memorysticks which consists of 2 patches, first that adds few functions to scatterlist.c, and the other patch that adds the driver. Driver is also stable and tested.
Best regards,
Maxim Levitsky
Maxim Levitsky
maintainerPATCH 1/4 scatterlist: new helper functions
While developing memstick driver for legacy memsticks I found the need in few helpers that I think should be in common scatterlist library
The functions that were added:
* sg_nents/sg_total_len - iterate over scatterlist to figure out total length of memory it covers / number of entries.
Usefull for small sg lists where there is no prefomance advantage of storing this info in a special variable.
* sg_copy/sg_truncate - Allow to break scatterlists apart into smaller chunks. sg_copy creates smaller scatterlist, spanning first 'len' bytes, while sg_truncate edits the scatterlist in such way that it skips over 'len' bytes.
* sg_compare_to_buffer - another function to hide gory details of access to sg list by CPU.
Allows to transparently compare contents of the sg list to given linear buffer.
If needed later, a function that compares two sgs can be added.
All of this code is used by my ms_block.c driver.
--- include/linux/scatterlist.h | 8 ++ lib/scatterlist.c | 152 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 0 deletions(-)
include/linux/scatterlist.h +8 -0
@@ -199,6 +199,12 @@ static inline void *sg_virt(struct scatterlist *sg)return page_address (sg_page (sg)) + sg->offset; } +struct scatterlist *sg_truncate (struct scatterlist *sg,int consumed); +int sg_nents (struct scatterlist *sg); +int sg_total_len (struct scatterlist *sg); +int sg_copy (struct scatterlist *sg_from,struct scatterlist *sg_to, +int to_nents,int len); +struct scatterlist *sg_next (struct scatterlist *);struct scatterlist *sg_last (struct scatterlist *s,unsigned int );void sg_init_table (struct scatterlist *,unsigned int );
@@ -217,6 +223,8 @@ size_t sg_copy_from_buffer(struct scatterlist *sgl, unsigned int nents, void *buf, size_t buflen);size_t sg_copy_to_buffer (struct scatterlist *sgl,unsigned int nents,void *buf,size_t buflen); +bool sg_compare_to_buffer (struct scatterlist *sg,unsigned int nents, +u8 *buffer,size_t len);/* * Maximum number of entries that will be allocated in one piece, if
lib/scatterlist.c +152 -0
167 lines not on this page · raw message
Signature · 1 line
1.7.1
Re: [PATCH 1/4] scatterlist: new helper functions
Quoted 8 lines · On Fri, 4 Mar 2011 06:16:50 +0200 Maxim Levitsky <[email protected]>…
While developing memstick driver for legacy memsticks I found the need in few helpers that I think should be in common scatterlist library
The functions that were added:
* sg_nents/sg_total_len - iterate over scatterlist to figure out total length of memory it covers / number of entries.
You should invent a data structure per I/O request, something like msb_request structure. Then you can store nents and total_len in that.
That's what block subsystems and drivers do. I took a look at your driver but I can't see why your driver can't do the same.
Maxim Levitsky
maintainerRe: [PATCH 1/4] scatterlist: new helper functions
Quoted 12 lines · FUJITA Tomonori
Quoted 8 lines · On Fri, 4 Mar 2011 06:16:50 +0200 Maxim Levitsky <[email protected]>…
While developing memstick driver for legacy memsticks I found the need in few helpers that I think should be in common scatterlist library
The functions that were added:
* sg_nents/sg_total_len - iterate over scatterlist to figure out total length of memory it covers / number of entries.
Quoted 6 lines
You should invent a data structure per I/O request, something like
msb_requeststructure. Then you can store nents andtotal_lenin that.That's what block subsystems and drivers do. I took a look at your driver but I can't see why your driver can't do the same.
I also need to break the request into small grained chunks. If I invent such structure, I will end up writing these helpers for it.
The I have this lifetime of a request:
I get arbitary sized request from block layer (I can of course control maximum size/number of segments in it, etc).
I break it into eraseblock sized chunks, and for each I translate the the LBA, into flash address.
Then I break it into flash page sized requests (512 bytes), and yet its better not to assume that such requests always contained in one sg entry.
Worse than that, I have to pass an sg list that spans always one 512 page to lowlevel driver, because thats how Alex defined the interface.
Thats why I coded it this way.
Folks, really what the status of this, when to expect it to be merged?
If you think some of helper functions don't belong to scatterlist.c, just tell me to move them back to ms_block.c.
Andrew, please note again that richoh lowlevel driver doesn't need any helper functions, its patch is standalone and thus should be merged regardless.
Best regards,
Maxim Levitsky
Re: [PATCH 1/4] scatterlist: new helper functions
Quoted 30 lines · On Sun, 06 Mar 2011 17:14:30 +0200 Maxim Levitsky <[email protected]>…
Quoted 12 lines · FUJITA Tomonori
Quoted 8 lines · On Fri, 4 Mar 2011 06:16:50 +0200 Maxim Levitsky <[email protected]>…
While developing memstick driver for legacy memsticks I found the need in few helpers that I think should be in common scatterlist library
The functions that were added:
* sg_nents/sg_total_len - iterate over scatterlist to figure out total length of memory it covers / number of entries.
Quoted 6 lines
You should invent a data structure per I/O request, something like
msb_requeststructure. Then you can store nents andtotal_lenin that.That's what block subsystems and drivers do. I took a look at your driver but I can't see why your driver can't do the same.
I also need to break the request into small grained chunks. If I invent such structure, I will end up writing these helpers for it.
The I have this lifetime of a request:
I get arbitary sized request from block layer (I can of course control maximum size/number of segments in it, etc).
I break it into eraseblock sized chunks, and for each I translate the the LBA, into flash address.
Quoted 6 lines
Then I break it into flash page sized requests (512 bytes), and yet its better not to assume that such requests always contained in one sg entry.
Worse than that, I have to pass an sg list that spans always one 512 page to lowlevel driver, because thats how Alex defined the interface.
This restriction is due to hardware specification or the software design (e.g. memstick layer)? If it is due to the latter, why can't you fix that?
Why can't the block layer split requests for you? It's better to let the block layer handle that.
Quoted 8 lines
Folks, really what the status of this, when to expect it to be merged?
If you think some of helper functions don't belong to
scatterlist.c, just tell me to move them back toms_block.c.Andrew, please note again that richoh lowlevel driver doesn't need any helper functions, its patch is standalone and thus should be merged regardless.
I think that we need to make the design of the driver easily understandable to kernel developers and maintainable by them. I don't think that this is 'standalone or not' issue.
Adding a doc about why the driver is designed in such odd way would be helpful. But I still think that we could design the driver in a better way.
Maxim Levitsky
maintainerRe: [PATCH 1/4] scatterlist: new helper functions
Quoted 37 lines · FUJITA Tomonori
Quoted 34 lines · On Sun, 06 Mar 2011 17:14:30 +0200 Maxim Levitsky <[email protected]>…
Quoted 12 lines · FUJITA Tomonori
Quoted 8 lines · On Fri, 4 Mar 2011 06:16:50 +0200 Maxim Levitsky <[email protected]>…
While developing memstick driver for legacy memsticks I found the need in few helpers that I think should be in common scatterlist library
The functions that were added:
* sg_nents/sg_total_len - iterate over scatterlist to figure out total length of memory it covers / number of entries.
Quoted 6 lines
You should invent a data structure per I/O request, something like
msb_requeststructure. Then you can store nents andtotal_lenin that.That's what block subsystems and drivers do. I took a look at your driver but I can't see why your driver can't do the same.
I also need to break the request into small grained chunks. If I invent such structure, I will end up writing these helpers for it.
The I have this lifetime of a request:
I get arbitary sized request from block layer (I can of course control maximum size/number of segments in it, etc).
I break it into eraseblock sized chunks, and for each I translate the the LBA, into flash address.
Then I break it into flash page sized requests (512 bytes), and yet its better not to assume that such requests always contained in one sg entry.
Quoted 6 lines
Quoted 2 lines
Worse than that, I have to pass an sg list that spans always one 512 page to lowlevel driver, because thats how Alex defined the interface.
This restriction is due to hardware specification or the software design (e.g. memstick layer)? If it is due to the latter, why can't you fix that?
Yes.
I already tried addressing some shortcomings of memstick layer, no no, I don't want to deal with its author, Alex Dubov again.
I think this code tries to be too clever/complex for the range of devices/speeds it supports, but I rather leave it as is.
To be honest, the code in question is for >5 year old memstick standard cards, thats hardly anybody uses.
It works, it is more or less simple, its not performance bound, its testd, and thus I want to keep it as is _for_ now.
Why I break sg lists into chunks?
Because unlike vast majority of block devices, I need to do FTL in the driver, thus its easier to work on eraseblock boundary.
Also unlike anything else, you can't just read/write a sector from a memorystick (especially the legacy one), you have to perform full dance of commands.
Not to mention error handling (like if you failed to write to block, you must try to choose another one, etc...)
(Of course writes follow same rules as raw nand flash, thats is writes only clear bits, and you can erase a eraseblock only).
Quoted 3 lines
Why can't the block layer split requests for you? It's better to let the block layer handle that.
You mean tell it not to give me more that one eraseblock to handle? Could you explain that a bit more?
Anyway, could we merge the code?
I would happy to improve it later, but currently merge window is very close, and the code is more or less agreed upon everyone, and written more that 1/2 of year ago.
Andrew Morton, could you help me with this, Please?
Best regards,
Maxim Levitsky
Quoted 18 lines
Quoted 10 lines
Folks, really what the status of this, when to expect it to be merged?
If you think some of helper functions don't belong to
scatterlist.c, just tell me to move them back toms_block.c.Andrew, please note again that richoh lowlevel driver doesn't need any helper functions, its patch is standalone and thus should be merged regardless.
I think that we need to make the design of the driver easily understandable to kernel developers and maintainable by them. I don't think that this is 'standalone or not' issue.
Adding a doc about why the driver is designed in such odd way would be helpful. But I still think that we could design the driver in a better way.
Maxim Levitsky
maintainerRe: [PATCH 1/4] scatterlist: new helper functions
Quoted 80 lines · Maxim Levitsky
Quoted 37 lines · FUJITA Tomonori
Quoted 34 lines · On Sun, 06 Mar 2011 17:14:30 +0200 Maxim Levitsky <[email protected]>…
Quoted 12 lines · FUJITA Tomonori
Quoted 8 lines · On Fri, 4 Mar 2011 06:16:50 +0200 Maxim Levitsky <[email protected]>…
While developing memstick driver for legacy memsticks I found the need in few helpers that I think should be in common scatterlist library
The functions that were added:
* sg_nents/sg_total_len - iterate over scatterlist to figure out total length of memory it covers / number of entries.
Quoted 6 lines
You should invent a data structure per I/O request, something like
msb_requeststructure. Then you can store nents andtotal_lenin that.That's what block subsystems and drivers do. I took a look at your driver but I can't see why your driver can't do the same.
I also need to break the request into small grained chunks. If I invent such structure, I will end up writing these helpers for it.
The I have this lifetime of a request:
I get arbitary sized request from block layer (I can of course control maximum size/number of segments in it, etc).
I break it into eraseblock sized chunks, and for each I translate the the LBA, into flash address.
Then I break it into flash page sized requests (512 bytes), and yet its better not to assume that such requests always contained in one sg entry.
Quoted 6 lines
Quoted 2 lines
Worse than that, I have to pass an sg list that spans always one 512 page to lowlevel driver, because thats how Alex defined the interface.
This restriction is due to hardware specification or the software design (e.g. memstick layer)? If it is due to the latter, why can't you fix that?
Yes.
I already tried addressing some shortcomings of memstick layer, no no, I don't want to deal with its author, Alex Dubov again.
I think this code tries to be too clever/complex for the range of devices/speeds it supports, but I rather leave it as is.To be honest, the code in question is for >5 year old memstick standard cards, thats hardly anybody uses.
It works, it is more or less simple, its not performance bound, its testd, and thus I want to keep it as is _for_ now.Why I break sg lists into chunks?
Because unlike vast majority of block devices, I need to do FTL in the driver, thus its easier to work on eraseblock boundary.
Also unlike anything else, you can't just read/write a sector from a memorystick (especially the legacy one), you have to perform full dance of commands.Not to mention error handling (like if you failed to write to block, you must try to choose another one, etc...)
(Of course writes follow same rules as raw nand flash, thats is writes only clear bits, and you can erase a eraseblock only).
Quoted 3 lines
Why can't the block layer split requests for you? It's better to let the block layer handle that.
You mean tell it not to give me more that one eraseblock to handle? Could you explain that a bit more?
Quoted 6 lines
Anyway, could we merge the code?
I would happy to improve it later, but currently merge window is very close, and the code is more or less agreed upon everyone, and written more that 1/2 of year ago.Andrew Morton, could you help me with this, Please?
Ping (I really worry about this).
Quoted 25 lines
Best regards,
Maxim LevitskyQuoted 18 lines
Quoted 10 lines
Folks, really what the status of this, when to expect it to be merged?
If you think some of helper functions don't belong to
scatterlist.c, just tell me to move them back toms_block.c.Andrew, please note again that richoh lowlevel driver doesn't need any helper functions, its patch is standalone and thus should be merged regardless.
I think that we need to make the design of the driver easily understandable to kernel developers and maintainable by them. I don't think that this is 'standalone or not' issue.
Adding a doc about why the driver is designed in such odd way would be helpful. But I still think that we could design the driver in a better way.
Andrew Morton
maintainerRe: [PATCH 1/4] scatterlist: new helper functions
In reply to Maxim LevitskyQuoted 45 lines · On Mon, 07 Mar 2011 04:20:37 +0200 Maxim Levitsky <[email protected]>…
Quoted 37 lines · FUJITA Tomonori
Quoted 34 lines · On Sun, 06 Mar 2011 17:14:30 +0200 Maxim Levitsky <[email protected]>…
Quoted 12 lines · FUJITA Tomonori
Quoted 8 lines · On Fri, 4 Mar 2011 06:16:50 +0200 Maxim Levitsky <[email protected]>…
While developing memstick driver for legacy memsticks I found the need in few helpers that I think should be in common scatterlist library
The functions that were added:
* sg_nents/sg_total_len - iterate over scatterlist to figure out total length of memory it covers / number of entries.
Quoted 6 lines
You should invent a data structure per I/O request, something like
msb_requeststructure. Then you can store nents andtotal_lenin that.That's what block subsystems and drivers do. I took a look at your driver but I can't see why your driver can't do the same.
I also need to break the request into small grained chunks. If I invent such structure, I will end up writing these helpers for it.
The I have this lifetime of a request:
I get arbitary sized request from block layer (I can of course control maximum size/number of segments in it, etc).
I break it into eraseblock sized chunks, and for each I translate the the LBA, into flash address.
Then I break it into flash page sized requests (512 bytes), and yet its better not to assume that such requests always contained in one sg entry.
Quoted 6 lines
Quoted 2 lines
Worse than that, I have to pass an sg list that spans always one 512 page to lowlevel driver, because thats how Alex defined the interface.
This restriction is due to hardware specification or the software design (e.g. memstick layer)? If it is due to the latter, why can't you fix that?
Quoted 6 lines
Yes.
I already tried addressing some shortcomings of memstick layer, no no, I don't want to deal with its author, Alex Dubov again.
I think this code tries to be too clever/complex for the range of devices/speeds it supports, but I rather leave it as is.
I have to say, these aren't very good reasons for a particular implementation!
Quoted 13 lines
To be honest, the code in question is for >5 year old memstick standard cards, thats hardly anybody uses.
It works, it is more or less simple, its not performance bound, its testd, and thus I want to keep it as is _for_ now.Why I break sg lists into chunks?
Because unlike vast majority of block devices, I need to do FTL in the driver, thus its easier to work on eraseblock boundary.
Also unlike anything else, you can't just read/write a sector from a memorystick (especially the legacy one), you have to perform full dance of commands.
Quoted 5 lines
Not to mention error handling (like if you failed to write to block, you must try to choose another one, etc...)
(Of course writes follow same rules as raw nand flash, thats is writes only clear bits, and you can erase a eraseblock only).
hm. If you think there's little likelihood that other drivers will need the new sg functions in the future then perhaps they should be made private to the memstick driver, rather than bloating everyone's kernels. Which is, I think, the exact opposite of what I suggested last year :(
Fujita-san, you've gone all quiet. Do you believe that these functions should be added to the sg API?
Thanks.
Re: [PATCH 1/4] scatterlist: new helper functions
Quoted 11 lines · On Tue, 15 Mar 2011 17:44:59 -0700 Andrew Morton <[email protected]>…
Quoted 10 lines
Quoted 3 lines
This restriction is due to hardware specification or the software design (e.g. memstick layer)? If it is due to the latter, why can't you fix that?
Yes.
I already tried addressing some shortcomings of memstick layer, no no, I don't want to deal with its author, Alex Dubov again.
I think this code tries to be too clever/complex for the range of devices/speeds it supports, but I rather leave it as is.
Quoted 2 lines
I have to say, these aren't very good reasons for a particular implementation!
Agreed. We need to fix it.
Quoted 19 lines
Quoted 13 lines
To be honest, the code in question is for >5 year old memstick standard cards, thats hardly anybody uses.
It works, it is more or less simple, its not performance bound, its testd, and thus I want to keep it as is _for_ now.Why I break sg lists into chunks?
Because unlike vast majority of block devices, I need to do FTL in the driver, thus its easier to work on eraseblock boundary.
Also unlike anything else, you can't just read/write a sector from a memorystick (especially the legacy one), you have to perform full dance of commands.Quoted 5 lines
Not to mention error handling (like if you failed to write to block, you must try to choose another one, etc...)
(Of course writes follow same rules as raw nand flash, thats is writes only clear bits, and you can erase a eraseblock only).
Quoted 8 lines
hm. If you think there's little likelihood that other drivers will need the new sg functions in the future then perhaps they should be made private to the memstick driver, rather than bloating everyone's kernels. Which is, I think, the exact opposite of what I suggested last year :(
Fujita-san, you've gone all quiet. Do you believe that these functions should be added to the sg API?
I don't think so.
Splitting (or merging) a request and playing with sg lists inside a driver is a bad idea. Such should be done in the block layer.
I still don't see why the block layer can't do that for the driver.
I believe that the helper functions for such should not be added.
Alex Dubov
maintainerRe: [PATCH 1/4] scatterlist: new helper functions
Quoted 9 lines
I don't think so.
Splitting (or merging) a request and playing with sg lists inside a
driver is a bad idea. Such should be done in the block layer.
Quoted 6 lines
I still don't see why the block layer can't do that for the driver.
I believe that the helper functions for such should not be added.
In a particular case of flash-like devices, letting the block layer split requests into memory sequential blocks too often results in unnecessary fragmentation of writes/erases.
If only one sg entry is requested from the block layer, it will be (more often than not) only 1 or 2 pages in length, even if total size of prospective write request spans multiple erase blocks.
So there are really only two options for legacy memorystick driver:
1. Play with scatterlists explicitly.
2. Make it an MTD backend, rather then stand-alone block device.
The second option makes more sense, but it is not necessarily the optimal approach for implementation of this particular media format.
Re: [PATCH 1/4] scatterlist: new helper functions
Quoted 12 lines · On Tue, 15 Mar 2011 21:18:14 -0700 (PDT) Alex Dubov <[email protected]> wrote:
Quoted 12 lines
I don't think so.
Splitting (or merging) a request and playing with sg lists inside a
driver is a bad idea. Such should be done in the block layer.I still don't see why the block layer can't do that for the driver.
Quoted 8 lines
Quoted 3 lines
I believe that the helper functions for such should not be added.
In a particular case of flash-like devices, letting the block layer split requests into memory sequential blocks too often results in unnecessary fragmentation of writes/erases.
Why?
Why can't the block layer split a request in the way the driver wants to do?
That is, why can't the driver tell the block layer how to split a request?
Quoted 3 lines
If only one sg entry is requested from the block layer, it will be (more often than not) only 1 or 2 pages in length, even if total size of prospective write request spans multiple erase blocks.
In this case, what does the driver do? Why can't the block layer do the same?
Quoted 15 lines
So there are really only two options for legacy memorystick driver:
1. Play with scatterlists explicitly.
2. Make it an MTD backend, rather then stand-alone block device.The second option makes more sense, but it is not necessarily the optimal approach for implementation of this particular media format.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Alex Dubov
maintainerRe: [PATCH 1/4] scatterlist: new helper functions
Quoted 10 lines
Why?
Why can't the block layer split a request in the way the driver wants
to do?That is, why can't the driver tell the block layer how to split a
request?
What is needed is the ability to get fixed sized (in bytes) blocks from the block layer.
Last time I checked (it was a long time ago, admittedly) one could only ask for a fixed number of sg entries, without any control on how many bytes each sg entry references.
Is there a way to get data from the block layer in a fashion of:
"Give me 16k/32k/whatever in one sg entry if request is equal or larger than this"?
If my knowledge is correct, MTD currently addresses this issue by maintaining its own cache, which it uses to aggregate write requests until it can write a whole erase block. While this is ok with old media (legacy memory stick being an example of such), new flash chips can have multi-megabyte sized erase blocks and can benefit from operations (like copy and compare) directly on scatter list .
Re: [PATCH 1/4] scatterlist: new helper functions
Quoted 14 lines · Alex Dubov
Quoted 10 lines
Why?
Why can't the block layer split a request in the way the driver wants
to do?That is, why can't the driver tell the block layer how to split a
request?What is needed is the ability to get fixed sized (in bytes) blocks from the block layer.
Quoted 7 lines
Last time I checked (it was a long time ago, admittedly) one could only ask for a fixed number of sg entries, without any control on how many bytes each sg entry references.
Is there a way to get data from the block layer in a fashion of:
"Give me 16k/32k/whatever in one sg entry if request is equal or larger than this"?
Yes; we've always had it: it's blk_max_queue_hw_sectors(). No request will go over that number times the sector size (of course, they may go under).
Quoted 6 lines
If my knowledge is correct, MTD currently addresses this issue by maintaining its own cache, which it uses to aggregate write requests until it can write a whole erase block. While this is ok with old media (legacy memory stick being an example of such), new flash chips can have multi-megabyte sized erase blocks and can benefit from operations (like copy and compare) directly on scatter list .
So this is where you want a minimum too. What you likely want is to set the logical block size to your erase block (blk_queue_logical_block_size) and the physical block size to the actual block size. Then we'll try as hard as we can to send down blocks on an erase boundary. Of course, there are some that just won't fit (like fs metadata) and you'll have to do a RMW for them.
James
Andrew Morton
maintainerRe: [PATCH 1/4] scatterlist: new helper functions
In reply to Alex DubovQuoted 25 lines · On Tue, 15 Mar 2011 21:18:14 -0700 (PDT) Alex Dubov <[email protected]> wrote:
Quoted 9 lines
I don't think so.
Splitting (or merging) a request and playing with sg lists inside a
driver is a bad idea. Such should be done in the block layer.Quoted 6 lines
I still don't see why the block layer can't do that for the driver.
I believe that the helper functions for such should not be added.
In a particular case of flash-like devices, letting the block layer split requests into memory sequential blocks too often results in unnecessary fragmentation of writes/erases.
If only one sg entry is requested from the block layer, it will be (more often than not) only 1 or 2 pages in length, even if total size of prospective write request spans multiple erase blocks.
Quoted 7 lines
So there are really only two options for legacy memorystick driver:
1. Play with scatterlists explicitly.
2. Make it an MTD backend, rather then stand-alone block device.The second option makes more sense, but it is not necessarily the optimal approach for implementation of this particular media format.
Thanks.
Do you believe that any other driver is likely to ue the sg infrastructure which this patch adds? Should those additions be internal to the memstick code, at least for now?
Alex Dubov
maintainerRe: [PATCH 1/4] scatterlist: new helper functions
Quoted 7 lines
Do you believe that any other driver is likely to ue the sg
infrastructure which this patch adds? Should those additions be
internal to the memstick code, at least for now?
Considering the comments from Fujita and James, I suppose it will be best for now to fold the new sg functions into the memstick patch and investigate how the block layer functionality can be used to a greater utility at some later time.
Maxim Levitsky
maintainerRe: [PATCH 1/4] scatterlist: new helper functions
Quoted 8 lines · Alex Dubov
Quoted 7 lines
Do you believe that any other driver is likely to ue the sg
infrastructure which this patch adds? Should those additions be
internal to the memstick code, at least for now?
Quoted 4 lines
Considering the comments from Fujita and James, I suppose it will be best for now to fold the new sg functions into the memstick patch and investigate how the block layer functionality can be used to a greater utility at some later time.
I am thinking the same thing.
If the unusual sg code is a problem, I'll think of something later to improve it, heck I can even completely rip it off and just use kernel pointers, relying on block core to bounce for high-mem situations. Since its a driver for a legacy hardware, mostly provided for completeness, the performance difference shouldn't be much.
I think its ok to merge the code with folded sg code _for_ now, and then I will send patches to address this point you didn't like.
OK?
Best regards,
Maxim Levitsky
Re: [PATCH 1/4] scatterlist: new helper functions
In reply to Alex DubovQuoted 4 lines · On Wed, 16 Mar 2011 23:41:13 -0700 (PDT) Alex Dubov <[email protected]> wrote:
Considering the comments from Fujita and James, I suppose it will be best for now to fold the new sg functions into the memstick patch and investigate how the block layer functionality can be used to a greater utility at some later time.
I suspect that such 'some later time' never come (especially for old hardware). So I would prefer to merge the properly designed driver rather than the current hacky driver.
Maxim Levitsky
maintainerPATCH 2/4 memstick: add support for legacy memorysticks
In reply to Maxim LevitskyBased partially on MS standard spec quotes from Alex Dubov.
As any code that works with user data this driver isn't recommended to use to write cards that contain valuable data.
It tries its best though to avoid data corruption
and possible damage to the card.
Tested on MS DUO 64 MB card on Ricoh R592 card reader.
--- MAINTAINERS | 5 + drivers/memstick/core/Kconfig | 12 + drivers/memstick/core/Makefile | 2 +- drivers/memstick/core/ms_block.c | 2288 ++++++++++++++++++++++++++++++++++++++ drivers/memstick/core/ms_block.h | 245 ++++ 5 files changed, 2551 insertions(+), 1 deletions(-) create mode 100644 drivers/memstick/core/ms_block.c create mode 100644 drivers/memstick/core/ms_block.h
MAINTAINERS +5 -0
@@ -5799,6 +5799,11 @@ W: http://tifmxx.berlios.de/ S: Maintained F: drivers/memstick/host/tifm_ms.c +SONY MEMORYSTICK STANDARD SUPPORT +M: Maxim Levitsky <[email protected]> +S: Maintained +F: drivers/memstick/core/ms_block.* + SOUND M: Jaroslav Kysela <[email protected]> M: Takashi Iwai <[email protected]>
drivers/memstick/core/Kconfig +12 -0
@@ -24,3 +24,15 @@ config MSPRO_BLOCK support. This provides a block device driver, which you can use to mount the filesystem. Almost everyone wishing MemoryStick support should say Y or M here. + +config MS_BLOCK + tristate "MemoryStick Standard device driver" + depends on BLOCK && EXPERIMENTAL + help + Say Y here to enable the MemoryStick Standard device driver + support. This provides a block device driver, which you can use + to mount the filesystem. + This driver works with old (bulky) MemoryStick and MemoryStick Duo + but not PRO. Say Y if you have such card. + Driver is new and not yet well tested, thus it can damage your card + (even permanently)
drivers/memstick/core/Makefile +1 -1
@@ -7,5 +7,5 @@ ifeq ($(CONFIG_MEMSTICK_DEBUG),y) endif obj-$(CONFIG_MEMSTICK) += memstick.o - +obj-$(CONFIG_MS_BLOCK) += ms_block.o obj-$(CONFIG_MSPRO_BLOCK) += mspro_block.o
drivers/memstick/core/ms_block.c new file +2288 -0
2,288 lines not on this page · raw message
drivers/memstick/core/ms_block.h new file +245 -0
245 lines not on this page · raw message
Signature · 1 line
1.7.1
Maxim Levitsky
maintainerPATCH 3/4 memstick: Add driver for Ricoh R5C592 card reader.
In reply to Maxim Levitsky--- MAINTAINERS | 5 + drivers/memstick/host/Kconfig | 12 + drivers/memstick/host/Makefile | 1 + drivers/memstick/host/r592.c | 908 ++++++++++++++++++++++++++++++++++++++++ drivers/memstick/host/r592.h | 175 ++++++++ 5 files changed, 1101 insertions(+), 0 deletions(-) create mode 100644 drivers/memstick/host/r592.c create mode 100644 drivers/memstick/host/r592.h
MAINTAINERS +5 -0
@@ -5243,6 +5243,11 @@ S: Maintained F: drivers/mtd/nand/r852.c F: drivers/mtd/nand/r852.h +RICOH R5C592 MEMORYSTICK DRIVER +M: Maxim Levitsky <[email protected]> +S: Maintained +F: drivers/memstick/host/r592.* + RISCOM8 DRIVER S: Orphan F: Documentation/serial/riscom8.txt
drivers/memstick/host/Kconfig +12 -0
@@ -30,3 +30,15 @@ config MEMSTICK_JMICRON_38X To compile this driver as a module, choose M here: the module will be called jmb38x_ms. + +config MEMSTICK_R592 + tristate "Ricoh R5C592 MemoryStick interface support (EXPERIMENTAL)" + depends on EXPERIMENTAL && PCI + + help + Say Y here if you want to be able to access MemoryStick cards with + the Ricoh R5C592 MemoryStick card reader (which is part of 5 in one + multifunction reader) + + To compile this driver as a module, choose M here: the module will + be called r592.
drivers/memstick/host/Makefile +1 -0
@@ -8,3 +8,4 @@ endif obj-$(CONFIG_MEMSTICK_TIFM_MS) += tifm_ms.o obj-$(CONFIG_MEMSTICK_JMICRON_38X) += jmb38x_ms.o +obj-$(CONFIG_MEMSTICK_R592) += r592.o
drivers/memstick/host/r592.c new file +908 -0
908 lines not on this page · raw message
drivers/memstick/host/r592.h new file +175 -0
175 lines not on this page · raw message
Signature · 1 line
1.7.1
Re: [PATCH 3/4] memstick: Add driver for Ricoh R5C592 card reader.
Quoted 6 lines
From: Maxim Levitsky <[email protected]>
Subject: [PATCH 3/4] memstick: Add driver for Ricoh R5C592 card reader.
To: "Andrew Morton" <[email protected]>
Cc: "James Bottomley" <[email protected]>, "FUJITA Tomonori" <[email protected]>, [email protected], [email protected], "Maxim Levitsky" <[email protected]>
Received: Friday, 4 March, 2011, 3:16 PMSigned-off-by Maxim Levitsky <[email protected]>
---
I understand, this patch can be applied as it is, and has no dependency on the discussed scatter list functionality.
Maxim Levitsky
maintainerPATCH 4/4 memstick: add Alex Dubov to MAINTAINERS of the memstick core
In reply to Maxim LevitskyThis code is supposed to be maintained by him and unless he doesn't want to do so, MAINTAINERS should reflect the state of things, and bugreporters should know whom to blame for bugs.
--- MAINTAINERS | 13 ++++++++++++- 1 files changed, 12 insertions(+), 1 deletions(-)
MAINTAINERS +12 -1
@@ -5798,7 +5798,18 @@ F: drivers/char/sonypi.c F: drivers/platform/x86/sony-laptop.c F: include/linux/sony-laptop.h -SONY MEMORYSTICK CARD SUPPORT +SONY MEMORYSTICK SUBSYSTEM +M: Alex Dubov <[email protected]> +S: Maintained +F: drivers/memstick/memstick.c +F: drivers/memstick/mspro_block.c + +SONY MEMORYSTICK DRIVER FOR JMICRON CONTROLLERS +M: Alex Dubov <[email protected]> +S: Maintained +F: drivers/memstick/host/jmb38x_ms.c + +SONY MEMORYSTICK DRIVER FOR TI CONTROLLERS M: Alex Dubov <[email protected]> W: http://tifmxx.berlios.de/ S: Maintained
Signature · 1 line
1.7.1
Maxim Levitsky
maintainerQuoted 11 lines · Maxim Levitsky
Hi,
This is a repost of my patches for 2.6.39 inclusion, which I hope not to miss this time.
I addressed the comments on the scatterlist issues.
Andrew, please note that my richoh memstick driver is standalone, unchanged from previos versions has many users which use the version I posted at ubuntu's Launchpad and happy with it. Please include it regardless of other patches.
Quoted 6 lines
The other half of my work is support for legacy memorysticks which consists of 2 patches, first that adds few functions to
scatterlist.c, and the other patch that adds the driver. Driver is also stable and tested.Best regards,
Maxim Levitsky
Any update?
Best regards,
Maxim Levitsky
Maxim Levitsky
maintainerQuoted 16 lines · Maxim Levitsky
Quoted 15 lines · Maxim Levitsky
Hi,
This is a repost of my patches for 2.6.39 inclusion, which I hope not to miss this time.
I addressed the comments on the scatterlist issues.
Andrew, please note that my richoh memstick driver is standalone, unchanged from previos versions has many users which use the version I posted at ubuntu's Launchpad and happy with it. Please include it regardless of other patches.
The other half of my work is support for legacy memorysticks which consists of 2 patches, first that adds few functions to
scatterlist.c, and the other patch that adds the driver. Driver is also stable and tested.
Quoted 5 lines
Quoted 2 lines
Best regards,
Maxim LevitskyAny update?
Any update?
Best regards,
Maxim Levitsky
Andrew Morton
maintainerQuoted 17 lines · On Tue, 15 Mar 2011 22:00:10 +0200 Maxim Levitsky <[email protected]>…
Quoted 16 lines · Maxim Levitsky
Quoted 15 lines · Maxim Levitsky
Hi,
This is a repost of my patches for 2.6.39 inclusion, which I hope not to miss this time.
I addressed the comments on the scatterlist issues.
Andrew, please note that my richoh memstick driver is standalone, unchanged from previos versions has many users which use the version I posted at ubuntu's Launchpad and happy with it. Please include it regardless of other patches.
The other half of my work is support for legacy memorysticks which consists of 2 patches, first that adds few functions to
scatterlist.c, and the other patch that adds the driver. Driver is also stable and tested.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Best regards,
Maxim LevitskyAny update?
Any update?
I'm hoping that Alex will soon have time to (re)review these patches.
Maxim Levitsky
maintainerQuoted 24 lines · Andrew Morton
Quoted 21 lines · On Tue, 15 Mar 2011 22:00:10 +0200 Maxim Levitsky <[email protected]>…
Quoted 20 lines · Maxim Levitsky
Quoted 17 lines · Maxim Levitsky
Hi,
This is a repost of my patches for 2.6.39 inclusion, which I hope not to miss this time.
I addressed the comments on the scatterlist issues.
Andrew, please note that my richoh memstick driver is standalone, unchanged from previos versions has many users which use the version I posted at ubuntu's Launchpad and happy with it. Please include it regardless of other patches.
The other half of my work is support for legacy memorysticks which consists of 2 patches, first that adds few functions to
scatterlist.c, and the other patch that adds the driver. Driver is also stable and tested.Best regards,
Maxim Levitsky
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Any update?
Any update?
I'm hoping that Alex will soon have time to (re)review these patches.
Andrew Morton, I am so glad to hear something, really was frustrated to see no responses, really thanks.
Alex already did gave green light for these patches, and they are pretty much unchanged from that point, except trivial adoptions of changes in scatterlist code.
Both drivers I send are completely standalone, patch just adds the .c and .h file.
And I add few functions to scatterlist.c, which is more or less agreed. (If some of these functions don't fit there, I can just put in in the driver locally).
It is very important to me to merge that code now. There is absolutely to risk of regressions, I don't touch any code there.
Thanks in advance,
Best regards,
Maxim Levitsky
Maxim Levitsky
maintainerQuoted 24 lines · Andrew Morton
Quoted 21 lines · On Tue, 15 Mar 2011 22:00:10 +0200 Maxim Levitsky <[email protected]>…
Quoted 20 lines · Maxim Levitsky
Quoted 17 lines · Maxim Levitsky
Hi,
This is a repost of my patches for 2.6.39 inclusion, which I hope not to miss this time.
I addressed the comments on the scatterlist issues.
Andrew, please note that my richoh memstick driver is standalone, unchanged from previos versions has many users which use the version I posted at ubuntu's Launchpad and happy with it. Please include it regardless of other patches.
The other half of my work is support for legacy memorysticks which consists of 2 patches, first that adds few functions to
scatterlist.c, and the other patch that adds the driver. Driver is also stable and tested.Best regards,
Maxim Levitsky
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Any update?
Any update?
I'm hoping that Alex will soon have time to (re)review these patches.
Andrew Morton, what the current state now?
Signature · 5 lines
Best regards,
Maxim Levitsky
Visit my blog: http://maximlevitsky.wordpress.com
Warning: Above blog contains rants.Andrew Morton
maintainerQuoted 25 lines · On Sun, 20 Mar 2011 05:09:05 +0200 Maxim Levitsky <[email protected]>…
Quoted 24 lines · Andrew Morton
Quoted 21 lines · On Tue, 15 Mar 2011 22:00:10 +0200 Maxim Levitsky <[email protected]>…
Quoted 20 lines · Maxim Levitsky
Quoted 17 lines · Maxim Levitsky
Hi,
This is a repost of my patches for 2.6.39 inclusion, which I hope not to miss this time.
I addressed the comments on the scatterlist issues.
Andrew, please note that my richoh memstick driver is standalone, unchanged from previos versions has many users which use the version I posted at ubuntu's Launchpad and happy with it. Please include it regardless of other patches.
The other half of my work is support for legacy memorysticks which consists of 2 patches, first that adds few functions to
scatterlist.c, and the other patch that adds the driver. Driver is also stable and tested.Best regards,
Maxim Levitsky
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line
Any update?
Any update?
I'm hoping that Alex will soon have time to (re)review these patches.
Andrew Morton, what the current state now?
Technical discussion is ongoing. James has described what appears to be the architecturally preferred way of implementing this and there is as yet no followup to his suggestion.
Maxim Levitsky
maintainerQuoted 32 lines · Andrew Morton
Quoted 30 lines · On Sun, 20 Mar 2011 05:09:05 +0200 Maxim Levitsky <[email protected]>…
Quoted 29 lines · Andrew Morton
Quoted 25 lines · On Tue, 15 Mar 2011 22:00:10 +0200 Maxim Levitsky <[email protected]>…
Quoted 16 lines · Maxim Levitsky
Quoted 15 lines · Maxim Levitsky
Hi,
This is a repost of my patches for 2.6.39 inclusion, which I hope not to miss this time.
I addressed the comments on the scatterlist issues.
Andrew, please note that my richoh memstick driver is standalone, unchanged from previos versions has many users which use the version I posted at ubuntu's Launchpad and happy with it. Please include it regardless of other patches.
The other half of my work is support for legacy memorysticks which consists of 2 patches, first that adds few functions to
scatterlist.c, and the other patch that adds the driver. Driver is also stable and tested.Quoted 5 lines
Quoted 2 lines
Best regards,
Maxim LevitskyAny update?
Any update?
Quoted 9 lines
Quoted 4 lines
Quoted 1 line
I'm hoping that Alex will soon have time to (re)review these patches.
Andrew Morton, what the current state now?
Technical discussion is ongoing. James has described what appears to be the architecturally preferred way of implementing this and there is as yet no followup to his suggestion.
But I will at least see my r592 driver in kernel? It doesn't depend on ether ms legacy driver nor on changes in scatterlist.c
Also, I don't have much time now to improve the ms_block driver till this summer (studying).
The driver works. Yes it has a flaw in regard to scatterlist processing, because I didn't find a better way to deal with this monster, but I will fix that later. I am not the kind of guy that runs away after a merge. It would be nice to just see my code in kernel, code I wrote more that a year ago.
This flaw is purely theoretical. Driver does work.
One of the ways to fix this is just use plain good kernel pointers. Yes that means bouncing of high mem, but like they say "to hell with that". The driver deals with legacy, and quite slow devices, so there won't be any performance difference.
Besides, my other driver for that card reader, for xD portion, does precisely that (more correctly common code in FTL frontend, the mtd_blkdev.c does that) and the end result is quite good.
So, I am waiting for a word from you,
Signature · 6 lines
Thanks in advance,
Best regards,
Maxim Levitsky
Visit my blog: http://maximlevitsky.wordpress.com
Warning: Above blog contains rants.Maxim Levitsky
maintainerQuoted 66 lines · Maxim Levitsky
Quoted 32 lines · Andrew Morton
Quoted 30 lines · On Sun, 20 Mar 2011 05:09:05 +0200 Maxim Levitsky <[email protected]>…
Quoted 29 lines · Andrew Morton
Quoted 25 lines · On Tue, 15 Mar 2011 22:00:10 +0200 Maxim Levitsky <[email protected]>…
Quoted 16 lines · Maxim Levitsky
Quoted 15 lines · Maxim Levitsky
Hi,
This is a repost of my patches for 2.6.39 inclusion, which I hope not to miss this time.
I addressed the comments on the scatterlist issues.
Andrew, please note that my richoh memstick driver is standalone, unchanged from previos versions has many users which use the version I posted at ubuntu's Launchpad and happy with it. Please include it regardless of other patches.
The other half of my work is support for legacy memorysticks which consists of 2 patches, first that adds few functions to
scatterlist.c, and the other patch that adds the driver. Driver is also stable and tested.Quoted 5 lines
Quoted 2 lines
Best regards,
Maxim LevitskyAny update?
Any update?
Quoted 9 lines
Quoted 4 lines
Quoted 1 line
I'm hoping that Alex will soon have time to (re)review these patches.
Andrew Morton, what the current state now?
Technical discussion is ongoing. James has described what appears to be the architecturally preferred way of implementing this and there is as yet no followup to his suggestion.
But I will at least see my r592 driver in kernel? It doesn't depend on ether ms legacy driver nor on changes in
scatterlist.cAlso, I don't have much time now to improve the
ms_blockdriver till this summer (studying).
The driver works. Yes it has a flaw in regard to scatterlist processing, because I didn't find a better way to deal with this monster, but I will fix that later. I am not the kind of guy that runs away after a merge. It would be nice to just see my code in kernel, code I wrote more that a year ago.This flaw is purely theoretical. Driver does work.
One of the ways to fix this is just use plain good kernel pointers. Yes that means bouncing of high mem, but like they say "to hell with that". The driver deals with legacy, and quite slow devices, so there won't be any performance difference.
Besides, my other driver for that card reader, for xD portion, does precisely that (more correctly common code in FTL frontend, themtd_blkdev.cdoes that) and the end result is quite good.
Quoted 1 line
So, I am waiting for a word from you,
Any update?
Signature · 5 lines
Best regards,
Maxim Levitsky
Visit my blog: http://maximlevitsky.wordpress.com
Warning: Above blog contains rants.Andrew Morton
maintainerQuoted 14 lines · On Sun, 20 Mar 2011 13:42:51 +0200 Maxim Levitsky <[email protected]>…
Quoted 2 lines · Andrew Morton
On Sun, 20 Mar 2011 05:09:05 +0200 Maxim Levitsky <[email protected]> wrote:
...
Also, I don't have much time now to improve the
ms_blockdriver till this summer (studying).
The driver works. Yes it has a flaw in regard to scatterlist processing, because I didn't find a better way to deal with this monster, but I will fix that later. I am not the kind of guy that runs away after a merge. It would be nice to just see my code in kernel, code I wrote more that a year ago.
Quoted 1 line
This flaw is purely theoretical. Driver does work.
Lots of code is "flawed but works". The place for such code is drivers/staging/ - it gets put in there so the code is available for those who need the driver and the code is later moved over into drivers/ once the flaws have been addressed.
So a path forward here would be for us to put the driver and the sglist extensions into a directory under drivers/staging/.
Maxim Levitsky
maintainerQuoted 19 lines · Andrew Morton
Quoted 14 lines · On Sun, 20 Mar 2011 13:42:51 +0200 Maxim Levitsky <[email protected]>…
Quoted 2 lines · Andrew Morton
On Sun, 20 Mar 2011 05:09:05 +0200 Maxim Levitsky <[email protected]> wrote:
...
Also, I don't have much time now to improve the
ms_blockdriver till this summer (studying).
The driver works. Yes it has a flaw in regard to scatterlist processing, because I didn't find a better way to deal with this monster, but I will fix that later. I am not the kind of guy that runs away after a merge. It would be nice to just see my code in kernel, code I wrote more that a year ago.Quoted 1 line
This flaw is purely theoretical. Driver does work.
Quoted 7 lines
Lots of code is "flawed but works". The place for such code is
drivers/staging/- it gets put in there so the code is available for those who need the driver and the code is later moved over intodrivers/once the flaws have been addressed.So a path forward here would be for us to put the driver and the sglist extensions into a directory under
drivers/staging/.
Ok, I won't be fighting with you over this one.
However, I ask for this:
1. Please merge r592.c, it doesn't depend on anything.
2. Please review ms_block.c for other problems that might prevent merge. For example when I published the sg list helpers, nobody told me that I am not allowed to add them. Actually the opposite, I was told to put them in scatterlist.c.
When I did so, again I was told that I didn't do the kerneldoc comments right, and also was told to improve few of the functions. Now I did all that, and I am told that scatterlist usage in my driver is no-go. OK. But what else is there that you don't like?
Best regards,
Maxim Levitsky
Quoted 25 lines · Maxim Levitsky
Quoted 24 lines · Andrew Morton
Quoted 14 lines · On Sun, 20 Mar 2011 13:42:51 +0200 Maxim Levitsky <[email protected]>…
Quoted 2 lines · Andrew Morton
On Sun, 20 Mar 2011 05:09:05 +0200 Maxim Levitsky <[email protected]> wrote:
...
Also, I don't have much time now to improve the
ms_blockdriver till this summer (studying).
The driver works. Yes it has a flaw in regard to scatterlist processing, because I didn't find a better way to deal with this monster, but I will fix that later. I am not the kind of guy that runs away after a merge. It would be nice to just see my code in kernel, code I wrote more that a year ago.Quoted 1 line
This flaw is purely theoretical. Driver does work.
Lots of code is "flawed but works". The place for such code is
drivers/staging/- it gets put in there so the code is available for those who need the driver and the code is later moved over intodrivers/once the flaws have been addressed.
Quoted 8 lines
Quoted 2 lines
So a path forward here would be for us to put the driver and the sglist extensions into a directory under
drivers/staging/.Ok, I won't be fighting with you over this one.
However, I ask for this:
1. Please merge
r592.c, it doesn't depend on anything.
What type of driver is that?
Quoted 8 lines
2. Please review
ms_block.cfor other problems that might prevent merge. For example when I published the sg list helpers, nobody told me that I am not allowed to add them. Actually the opposite, I was told to put them inscatterlist.c.
When I did so, again I was told that I didn't do the kerneldoc comments right, and also was told to improve few of the functions. Now I did all that, and I am told that scatterlist usage in my driver is no-go. OK. But what else is there that you don't like?
Ick, that's not nice, but formatting issues are usually the easiest to pick on, sorry for that happening.
So the result is to use the block layer functions instead, right? How much work do you imagine that would take to do?
If it's a bunch, care to put the code in drivers/staging/ now? I'll gladly take the patches to queue them up for .40.
thanks,
greg k-h
Maxim Levitsky
maintainerQuoted 28 lines · Greg KH
Quoted 27 lines · Maxim Levitsky
Quoted 26 lines · Andrew Morton
Quoted 14 lines · On Sun, 20 Mar 2011 13:42:51 +0200 Maxim Levitsky <[email protected]>…
Quoted 2 lines · Andrew Morton
On Sun, 20 Mar 2011 05:09:05 +0200 Maxim Levitsky <[email protected]> wrote:
...
Also, I don't have much time now to improve the
ms_blockdriver till this summer (studying).
The driver works. Yes it has a flaw in regard to scatterlist processing, because I didn't find a better way to deal with this monster, but I will fix that later. I am not the kind of guy that runs away after a merge. It would be nice to just see my code in kernel, code I wrote more that a year ago.Quoted 1 line
This flaw is purely theoretical. Driver does work.
Lots of code is "flawed but works". The place for such code is
drivers/staging/- it gets put in there so the code is available for those who need the driver and the code is later moved over intodrivers/once the flaws have been addressed.So a path forward here would be for us to put the driver and the sglist extensions into a directory under
drivers/staging/.
Quoted 8 lines
Quoted 6 lines
Ok, I won't be fighting with you over this one.
However, I ask for this:
1. Please merge
r592.c, it doesn't depend on anything.What type of driver is that?
A driver for card reader that reads Memstick cards. Two such drivers from Alex Dubov were already merged.
Quoted 10 lines
Quoted 9 lines
2. Please review
ms_block.cfor other problems that might prevent merge. For example when I published the sg list helpers, nobody told me that I am not allowed to add them. Actually the opposite, I was told to put them inscatterlist.c.
When I did so, again I was told that I didn't do the kerneldoc comments right, and also was told to improve few of the functions. Now I did all that, and I am told that scatterlist usage in my driver is no-go. OK. But what else is there that you don't like?
Quoted 2 lines
Ick, that's not nice, but formatting issues are usually the easiest to pick on, sorry for that happening.
Folk, could you actually review the driver once again, and really tell me what is wrong with it?
I did another review of it now, and it looks nice and clean.
Let me explain exactly the controversial point:
I get a request from block core to read or write a specific amount of 512 blocks.
I turn that request into scatterlist.
For reads (look at msb_do_read_request), I read one page into first 512 bytes of pages that are pointed by that scatterlist, and then modify it such as it now shorter and points to next 512 bytes.
For writes, I choose for 2 cases.
If I was told to write whole erase block or more, I create new scatter list that covers the 'erase block size' bytes of original one and just write it.
For smaller writes, I have a eraseblock sized cache buffer that I write to until I ased to write to different erase block. Then I flush current eraseblock doing copyonwrite.
Basicly the controversy is around my usage of scatterlist. I just added ability to create a copy of a scatterlistby which will cover n bytes, and ability to truncate a scatterlist by removing n bytes from its head.
Quoted 3 lines
So the result is to use the block layer functions instead, right? How much work do you imagine that would take to do?
I could just stop using scatterlists at all, and just do all writes/reads 512 bytes a time (using the copy on write cache for writes of course).
I did that successfully in my sm_ftl.c
Quoted 7 lines
If it's a bunch, care to put the code in
drivers/staging/now? I'll gladly take the patches to queue them up for .40.thanks,
greg k-h
Signature · 5 lines
Best regards,
Maxim Levitsky
Visit my blog: http://maximlevitsky.wordpress.com
Warning: Above blog contains rants.Alex Dubov
maintainerQuoted 7 lines · Maxim Levitsky
Quoted 2 lines
Quoted 2 lines
1. Please merge
r592.c, it doesn't depend onanything.
Quoted 2 lines
What type of driver is that?
A driver for card reader that reads Memstick cards. Two such drivers from Alex Dubov were already merged.
I would like to point out that the discussion is not about r592. We all agreed, quite some time ago, that it can be merged and were you, Maxim, submitted it separately 4 month ago as per my advice it would be in the tree already.