All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v8 2/2] mmc: support packed write command for eMMC4.5    device
@ 2012-11-14  8:14 merez
  2012-11-15 10:05 ` Seungwon Jeon
  0 siblings, 1 reply; 11+ messages in thread
From: merez @ 2012-11-14  8:14 UTC (permalink / raw)
  To: Chris Ball
  Cc: merez, Seungwon Jeon, linux-mmc, 'Subhash Jadavani',
	'S, Venkatraman', 'Saugata Das',
	'Namjae Jeon'

Hi Chris,

The amount of improvement from the packed commands, as from any other
eMMC4.5 feature, depends on several parameters:
1. The card support of this feature. If the card supports only the feature
interface, then you'll see no improvement when using the feature.
2. The benchmark tool used. Since the packed command preparation is
stopped due to a FLUSH request, a benchmark that issues many FLUSH
requests can result in a small amount of packing and you will see no
improvement.

You can use the following patch to get the packed commands statistics:
http://marc.info/?l=linux-mmc&m=134374508625826&w=2
With this patch you will be able to see the amount of packing and what
caused the packed preparation to stop.

We tested the packed commands feature with SanDisk cards and got
improvement of 30% when using lmdd and tiotest. We don't use iozone for
sequential tests but if you'll send me the exact command that you use we
can try it as well.

It is true that packed commands can cause degradation of read in
read-write collisions. However, it is only nature that when having longer
write request a read request has to wait for a longer time and its latency
will increase. I believe that it is not our duty to decide if this is a
reason to exclude this feature. Everyone should take its own decision if
he wants to benefit from the write improvement, while risking the
read-write collisions scenarios.
eMMC4.5 introduces the HPI and stop transmission to overcome the
degradation of read latency due to write (regardless of the packed
commands).
The packing control is our own enhancement that we believe can also be
used to overcome this degradation. It is tunable and requires a specific
enabling, so it can be the developer’s decision whether to use it or not.
Since it is not a standard feature we can discuss separately if it should
be accepted or not and what is the best way to use it.

Packed commands is not the only eMMC4.5 feature that can cause degradation
in specific scenarios. If we will look at the cache feature, it causes
degradation by almost a half in random operations when FLUSH is being
used.
When using the following iozone command when cache is enabled, you will
see degradation in the iozone results:
./data/iozone -i0 -i2 -r4k -s50m -O -o -I -f /data/mmc0/file3
However, cache support was accepted regardless of this degradation and it
is the developer’s responsibility to decide if to use this feature or not.

To summarize, all eMMC4.5 features that were added are tunable and
disabled by default.
I believe that when someone would enable a certain feature he will do all
the required testing for determining if he can benefit from this feature
or not in his own environment.

Thanks,
Maya

On Tue, November 13, 2012 6:54 pm, Chris Ball wrote:
> Hi Maya,
>
> On Sun, Nov 04 2012, merez@codeaurora.org wrote:
>> Packed commands is a mandatory eMMC4.5 feature and is supported by all
the card vendors.
>
> We're still only talking about using packed writes, though, right?
>
>> It wa proven to be beneficial for eMMC4.5 cards and harmless for non
eMMC4.5 cards.
>
> My understanding is that write packing causes a regression in read
performance that can be tuned/fixed by your num_wr_reqs_to_start_packing
tunable (and read packing causes a read regression with current eMMC 4.5
cards).  Is that wrong?
>
>> I don't see a point to hold it back while it can be enabled or
>> disabled by a flag and most of the code it adds is guarded in specific
functions and is not active when packed commands is disabled.
>
> Earlier in the thread I wrote:
>
>>> * I still don't have a good set of representative benchmarks showing
>>>   what kind of performance changes come with this patchset. It seems
like we've had a small amount of testing on one controller/eMMC part
combo from Seungwon, and an entirely different test from Maya, and
the results aren't documented fully anywhere to the level of
describing what the hardware was, what the test was, and what the
results were before and after the patchset.
>
> I still feel this way.  I'm worried that we might be merging code that
works well on your controller/card but causes large regressions for
everyone else.  I don't want to handle this by making a tunable that
everyone has to tune for their system, because I don't think anyone will
tune it.  I don't think that shipping a capability that will probably
lead to performance regressions if you turn it on is a good idea.
>
> I'm in a better position to help now, though -- I have some motherboards
with Marvell SoCs and a socketed eMMC slot, and I have eMMC 4.5 parts
from Sandisk and Toshiba.  So I can try to help work out how
> generalizable your results are across other controllers and cards.
>
> So far I've only tried the Sandisk part, but it didn't show any write
improvement with write packing.  I've verified that the switch command
to turn on packed_event_en happens and succeeds, and that the caps are
set correctly, so I'm not sure what's wrong yet.  With iozone I get:
>
>                        KB  reclen   write rewrite
> Unpacked writes:    10240    8192   17250   16794
> Packed writes:      10240    8192   16930   17353
>
> I'll try the Toshiba part next, and I'll start using lmdd as well as
iozone.  Any ideas on why I might not be seeing improvements with
Sandisk?
>
> I'm not opposed to merging packed write support in principle, I just
want to be convinced that we're not causing regressions for most users
who turn it on.  (And more than that, I want to see that it leads to
improvements that make it worth adding the code complexity for.)
>
> Thanks,
>
> - Chris.
> --
> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> One Laptop Per Child
>


-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation




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

* RE: [PATCH v8 2/2] mmc: support packed write command for eMMC4.5 device
  2012-11-14  8:14 [PATCH v8 2/2] mmc: support packed write command for eMMC4.5 device merez
@ 2012-11-15 10:05 ` Seungwon Jeon
  0 siblings, 0 replies; 11+ messages in thread
From: Seungwon Jeon @ 2012-11-15 10:05 UTC (permalink / raw)
  To: merez, 'Chris Ball'
  Cc: linux-mmc, 'Subhash Jadavani', 'S, Venkatraman',
	'Saugata Das', 'Namjae Jeon'

Hi Maya,

Thanks to remind it.
I'm struggling with something. I didn't check more since v8.

Hi Chris,

As we know, nand flash write is slower than read, because write is costly operation.
Write operation need block erase operation. Of course, if there is free block to write, erase will not happen.
In worst case, write operation may include copy exiting data as well as block erase block.
So, in the block management of nand, several data of small size is disadvantageous for write.
That is when writing large amounts of data, eMMC give it a significant speed advantage.
Packed write can help it by gathering the data. Actually we got the performance gain.

There are several bus speed mode. Such as HS200 mode, clock speed is up to 200MHz.
However, with little or no effect for write performance. ASYNC transfer patch is the same.
I think packed wrtie is a candidate which can improve write performance just now.

If you want to find the write improvement, you need to check the following two parameters.
	/sys/block/mmcblk0/queue/max_hw_sectors_kb
	/sys/block/mmcblk0/queue/max_sectors_kb
'max_hw_sectors_kb' is maximum size which host controller can gather in device driver.
If this size is large enough, performance effect may not be shown noticeably.

Thanks,
Seungwon Jeon
On Wednesday, November 14, 2012, merez@codeaurora.org wrote:
> Hi Chris,
> 
> The amount of improvement from the packed commands, as from any other
> eMMC4.5 feature, depends on several parameters:
> 1. The card support of this feature. If the card supports only the feature
> interface, then you'll see no improvement when using the feature.
> 2. The benchmark tool used. Since the packed command preparation is
> stopped due to a FLUSH request, a benchmark that issues many FLUSH
> requests can result in a small amount of packing and you will see no
> improvement.
> 
> You can use the following patch to get the packed commands statistics:
> http://marc.info/?l=linux-mmc&m=134374508625826&w=2
> With this patch you will be able to see the amount of packing and what
> caused the packed preparation to stop.
> 
> We tested the packed commands feature with SanDisk cards and got
> improvement of 30% when using lmdd and tiotest. We don't use iozone for
> sequential tests but if you'll send me the exact command that you use we
> can try it as well.
> 
> It is true that packed commands can cause degradation of read in
> read-write collisions. However, it is only nature that when having longer
> write request a read request has to wait for a longer time and its latency
> will increase. I believe that it is not our duty to decide if this is a
> reason to exclude this feature. Everyone should take its own decision if
> he wants to benefit from the write improvement, while risking the
> read-write collisions scenarios.
> eMMC4.5 introduces the HPI and stop transmission to overcome the
> degradation of read latency due to write (regardless of the packed
> commands).
> The packing control is our own enhancement that we believe can also be
> used to overcome this degradation. It is tunable and requires a specific
> enabling, so it can be the developer’s decision whether to use it or not.
> Since it is not a standard feature we can discuss separately if it should
> be accepted or not and what is the best way to use it.
> 
> Packed commands is not the only eMMC4.5 feature that can cause degradation
> in specific scenarios. If we will look at the cache feature, it causes
> degradation by almost a half in random operations when FLUSH is being
> used.
> When using the following iozone command when cache is enabled, you will
> see degradation in the iozone results:
> ./data/iozone -i0 -i2 -r4k -s50m -O -o -I -f /data/mmc0/file3
> However, cache support was accepted regardless of this degradation and it
> is the developer’s responsibility to decide if to use this feature or not.
> 
> To summarize, all eMMC4.5 features that were added are tunable and
> disabled by default.
> I believe that when someone would enable a certain feature he will do all
> the required testing for determining if he can benefit from this feature
> or not in his own environment.
> 
> Thanks,
> Maya
> 
> On Tue, November 13, 2012 6:54 pm, Chris Ball wrote:
> > Hi Maya,
> >
> > On Sun, Nov 04 2012, merez@codeaurora.org wrote:
> >> Packed commands is a mandatory eMMC4.5 feature and is supported by all
> the card vendors.
> >
> > We're still only talking about using packed writes, though, right?
> >
> >> It wa proven to be beneficial for eMMC4.5 cards and harmless for non
> eMMC4.5 cards.
> >
> > My understanding is that write packing causes a regression in read
> performance that can be tuned/fixed by your num_wr_reqs_to_start_packing
> tunable (and read packing causes a read regression with current eMMC 4.5
> cards).  Is that wrong?
> >
> >> I don't see a point to hold it back while it can be enabled or
> >> disabled by a flag and most of the code it adds is guarded in specific
> functions and is not active when packed commands is disabled.
> >
> > Earlier in the thread I wrote:
> >
> >>> * I still don't have a good set of representative benchmarks showing
> >>>   what kind of performance changes come with this patchset. It seems
> like we've had a small amount of testing on one controller/eMMC part
> combo from Seungwon, and an entirely different test from Maya, and
> the results aren't documented fully anywhere to the level of
> describing what the hardware was, what the test was, and what the
> results were before and after the patchset.
> >
> > I still feel this way.  I'm worried that we might be merging code that
> works well on your controller/card but causes large regressions for
> everyone else.  I don't want to handle this by making a tunable that
> everyone has to tune for their system, because I don't think anyone will
> tune it.  I don't think that shipping a capability that will probably
> lead to performance regressions if you turn it on is a good idea.
> >
> > I'm in a better position to help now, though -- I have some motherboards
> with Marvell SoCs and a socketed eMMC slot, and I have eMMC 4.5 parts
> from Sandisk and Toshiba.  So I can try to help work out how
> > generalizable your results are across other controllers and cards.
> >
> > So far I've only tried the Sandisk part, but it didn't show any write
> improvement with write packing.  I've verified that the switch command
> to turn on packed_event_en happens and succeeds, and that the caps are
> set correctly, so I'm not sure what's wrong yet.  With iozone I get:
> >
> >                        KB  reclen   write rewrite
> > Unpacked writes:    10240    8192   17250   16794
> > Packed writes:      10240    8192   16930   17353
> >
> > I'll try the Toshiba part next, and I'll start using lmdd as well as
> iozone.  Any ideas on why I might not be seeing improvements with
> Sandisk?
> >
> > I'm not opposed to merging packed write support in principle, I just
> want to be convinced that we're not causing regressions for most users
> who turn it on.  (And more than that, I want to see that it leads to
> improvements that make it worth adding the code complexity for.)
> >
> > Thanks,
> >
> > - Chris.
> > --
> > Chris Ball   <cjb@laptop.org>   <http://printf.net/>
> > One Laptop Per Child
> >
> 
> 
> --
> QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

* Re: [PATCH v8 2/2] mmc: support packed write command for eMMC4.5  device
  2012-11-04  5:05       ` merez
@ 2012-11-14  2:54         ` Chris Ball
  0 siblings, 0 replies; 11+ messages in thread
From: Chris Ball @ 2012-11-14  2:54 UTC (permalink / raw)
  To: merez
  Cc: Seungwon Jeon, linux-mmc, 'Subhash Jadavani',
	'S, Venkatraman', 'Saugata Das',
	'Namjae Jeon'

Hi Maya,

On Sun, Nov 04 2012, merez@codeaurora.org wrote:
> Packed commands is a mandatory eMMC4.5 feature and is supported by all
> the card vendors.

We're still only talking about using packed writes, though, right?

> It wa proven to be beneficial for eMMC4.5 cards and harmless for non
> eMMC4.5 cards.

My understanding is that write packing causes a regression in read
performance that can be tuned/fixed by your num_wr_reqs_to_start_packing
tunable (and read packing causes a read regression with current eMMC 4.5
cards).  Is that wrong?

> I don't see a point to hold it back while it can be enabled or
> disabled by a flag and most of the code it adds is guarded in specific
> functions and is not active when packed commands is disabled.

Earlier in the thread I wrote:

>> * I still don't have a good set of representative benchmarks showing
>>   what kind of performance changes come with this patchset. It seems
>>   like we've had a small amount of testing on one controller/eMMC
>>   part combo from Seungwon, and an entirely different test from Maya,
>>   and the results aren't documented fully anywhere to the level of
>>   describing what the hardware was, what the test was, and what the
>>   results were before and after the patchset.

I still feel this way.  I'm worried that we might be merging code that
works well on your controller/card but causes large regressions for
everyone else.  I don't want to handle this by making a tunable that
everyone has to tune for their system, because I don't think anyone will
tune it.  I don't think that shipping a capability that will probably
lead to performance regressions if you turn it on is a good idea.

I'm in a better position to help now, though -- I have some motherboards
with Marvell SoCs and a socketed eMMC slot, and I have eMMC 4.5 parts
from Sandisk and Toshiba.  So I can try to help work out how
generalizable your results are across other controllers and cards.

So far I've only tried the Sandisk part, but it didn't show any write
improvement with write packing.  I've verified that the switch command
to turn on packed_event_en happens and succeeds, and that the caps are
set correctly, so I'm not sure what's wrong yet.  With iozone I get:

                       KB  reclen   write rewrite
Unpacked writes:    10240    8192   17250   16794 
Packed writes:      10240    8192   16930   17353

I'll try the Toshiba part next, and I'll start using lmdd as well as
iozone.  Any ideas on why I might not be seeing improvements with
Sandisk?

I'm not opposed to merging packed write support in principle, I just
want to be convinced that we're not causing regressions for most users
who turn it on.  (And more than that, I want to see that it leads to
improvements that make it worth adding the code complexity for.)

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

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

* RE: [PATCH v8 2/2] mmc: support packed write command for eMMC4.5 device
  2012-07-09 10:13     ` Seungwon Jeon
  2012-07-10  5:40       ` merez
@ 2012-11-04  5:05       ` merez
  2012-11-14  2:54         ` Chris Ball
  1 sibling, 1 reply; 11+ messages in thread
From: merez @ 2012-11-04  5:05 UTC (permalink / raw)
  To: Seungwon Jeon
  Cc: merez, linux-mmc, 'Chris Ball',
	'Subhash Jadavani', 'S, Venkatraman',
	'Saugata Das', 'Namjae Jeon'

Hi Chris,

Packed commands is a mandatory eMMC4.5 feature and is supported by all the
card vendors.
It wa proven to be beneficial for eMMC4.5 cards and harmless for non
eMMC4.5 cards.
I don't see a point to hold it back while it can be enabled or disabled by
a flag and most of the code it adds is guarded in specific functions and
is not active when packed commands is disabled.

Thanks,
Maya
On Mon, July 9, 2012 3:13 am, Seungwon Jeon wrote:
> Monday, July 09, 2012, Maya <merez@codeaurora.org> wrote:
>> On Sun, July 8, 2012 4:52 pm, Seungwon Jeon wrote:
>> > Sunday, July 08, 2012, Maya <merez@codeaurora.org> wrote:
>> >> One minor comment below:
>> >>
>> >> > @@ -1278,9 +1609,15 @@ static int mmc_blk_issue_rw_rq(struct
>> mmc_queue
>> >> *mq, struct request *rqc)
>> >> >  			    (card->ext_csd.data_sector_size == 4096)) {
>> >> You can use mmc_large_sec here (instead of
>> >> card->ext_csd.data_sector_size
>> >> == 4096)
>> > Yes, but this line is kept due to another issue related with 4KB
>> sector.
>> >
>> > Thanks,
>> > Seungwon Jeon
>> Hi,
>>
>> I'm not sure I understand your reply. mmc_large_sec does exactly what
>> the
>> if here does.
>> My suggestion is to replace the code with the new function you added.
>> I agree it's an old code but this way the code will be consistent and
>> mmc_large_sec will be used in every place we would like to check if the
>> sector size is 4K.
> I just remained. The code you pointed can be replaced.
> Regardless this review, I'll rework.
> eMMC 4.51 spec was updated in June and there is some changes.
> Spec mentions packed header is 8 blocks in 4KB native device.
> I'll apply this change.
>
> Thanks,
> Seungwon Jeon
>
>>
>> Thanks,
>> Maya
>>
>> --
>> Sent by consultant of Qualcomm Innovation Center, Inc.
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


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

* RE: [PATCH v8 2/2] mmc: support packed write command for eMMC4.5    device
  2012-07-10  5:40       ` merez
@ 2012-07-10  5:48         ` merez
  0 siblings, 0 replies; 11+ messages in thread
From: merez @ 2012-07-10  5:48 UTC (permalink / raw)
  Cc: Seungwon Jeon, merez, linux-mmc, 'Chris Ball',
	'Subhash Jadavani', 'S, Venkatraman',
	'Saugata Das', 'Namjae Jeon'

Hi Chris,

Can we push this change to mmc-next?

Thanks,
Maya
On Mon, July 9, 2012 10:40 pm, merez@codeaurora.org wrote:
>
> On Mon, July 9, 2012 3:13 am, Seungwon Jeon wrote:
>> Monday, July 09, 2012, Maya <merez@codeaurora.org> wrote:
>>> On Sun, July 8, 2012 4:52 pm, Seungwon Jeon wrote:
>>> > Sunday, July 08, 2012, Maya <merez@codeaurora.org> wrote:
>>> >> One minor comment below:
>>> >>
>>> >> > @@ -1278,9 +1609,15 @@ static int mmc_blk_issue_rw_rq(struct
>>> mmc_queue
>>> >> *mq, struct request *rqc)
>>> >> >  			    (card->ext_csd.data_sector_size == 4096)) {
>>> >> You can use mmc_large_sec here (instead of
>>> >> card->ext_csd.data_sector_size
>>> >> == 4096)
>>> > Yes, but this line is kept due to another issue related with 4KB
>>> sector.
>>> >
>>> > Thanks,
>>> > Seungwon Jeon
>>> Hi,
>>>
>>> I'm not sure I understand your reply. mmc_large_sec does exactly what
>>> the
>>> if here does.
>>> My suggestion is to replace the code with the new function you added.
>>> I agree it's an old code but this way the code will be consistent and
>>> mmc_large_sec will be used in every place we would like to check if the
>>> sector size is 4K.
>> I just remained. The code you pointed can be replaced.
>> Regardless this review, I'll rework.
>> eMMC 4.51 spec was updated in June and there is some changes.
>> Spec mentions packed header is 8 blocks in 4KB native device.
>> I'll apply this change.
> No need to apply the change.
> You are probably right and since it is not related to your change it can
> be done later on.
>>
>> Thanks,
>> Seungwon Jeon
>>
>>>
>>> Thanks,
>>> Maya
>>>
>>> --
>>> Sent by consultant of Qualcomm Innovation Center, Inc.
>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>
>
> --
> Sent by consultant of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>
>


-- 
Sent by consultant of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum


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

* RE: [PATCH v8 2/2] mmc: support packed write command for eMMC4.5 device
  2012-07-09 10:13     ` Seungwon Jeon
@ 2012-07-10  5:40       ` merez
  2012-07-10  5:48         ` merez
  2012-11-04  5:05       ` merez
  1 sibling, 1 reply; 11+ messages in thread
From: merez @ 2012-07-10  5:40 UTC (permalink / raw)
  To: Seungwon Jeon
  Cc: merez, linux-mmc, 'Chris Ball',
	'Subhash Jadavani', 'S, Venkatraman',
	'Saugata Das', 'Namjae Jeon'


On Mon, July 9, 2012 3:13 am, Seungwon Jeon wrote:
> Monday, July 09, 2012, Maya <merez@codeaurora.org> wrote:
>> On Sun, July 8, 2012 4:52 pm, Seungwon Jeon wrote:
>> > Sunday, July 08, 2012, Maya <merez@codeaurora.org> wrote:
>> >> One minor comment below:
>> >>
>> >> > @@ -1278,9 +1609,15 @@ static int mmc_blk_issue_rw_rq(struct
>> mmc_queue
>> >> *mq, struct request *rqc)
>> >> >  			    (card->ext_csd.data_sector_size == 4096)) {
>> >> You can use mmc_large_sec here (instead of
>> >> card->ext_csd.data_sector_size
>> >> == 4096)
>> > Yes, but this line is kept due to another issue related with 4KB
>> sector.
>> >
>> > Thanks,
>> > Seungwon Jeon
>> Hi,
>>
>> I'm not sure I understand your reply. mmc_large_sec does exactly what
>> the
>> if here does.
>> My suggestion is to replace the code with the new function you added.
>> I agree it's an old code but this way the code will be consistent and
>> mmc_large_sec will be used in every place we would like to check if the
>> sector size is 4K.
> I just remained. The code you pointed can be replaced.
> Regardless this review, I'll rework.
> eMMC 4.51 spec was updated in June and there is some changes.
> Spec mentions packed header is 8 blocks in 4KB native device.
> I'll apply this change.
No need to apply the change.
You are probably right and since it is not related to your change it can
be done later on.
>
> Thanks,
> Seungwon Jeon
>
>>
>> Thanks,
>> Maya
>>
>> --
>> Sent by consultant of Qualcomm Innovation Center, Inc.
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>


-- 
Sent by consultant of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum


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

* RE: [PATCH v8 2/2] mmc: support packed write command for eMMC4.5 device
  2012-07-09  7:01   ` merez
@ 2012-07-09 10:13     ` Seungwon Jeon
  2012-07-10  5:40       ` merez
  2012-11-04  5:05       ` merez
  0 siblings, 2 replies; 11+ messages in thread
From: Seungwon Jeon @ 2012-07-09 10:13 UTC (permalink / raw)
  To: merez
  Cc: linux-mmc, 'Chris Ball', 'Subhash Jadavani',
	'S, Venkatraman', 'Saugata Das',
	'Namjae Jeon'

Monday, July 09, 2012, Maya <merez@codeaurora.org> wrote:
> On Sun, July 8, 2012 4:52 pm, Seungwon Jeon wrote:
> > Sunday, July 08, 2012, Maya <merez@codeaurora.org> wrote:
> >> One minor comment below:
> >>
> >> > @@ -1278,9 +1609,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
> >> *mq, struct request *rqc)
> >> >  			    (card->ext_csd.data_sector_size == 4096)) {
> >> You can use mmc_large_sec here (instead of
> >> card->ext_csd.data_sector_size
> >> == 4096)
> > Yes, but this line is kept due to another issue related with 4KB sector.
> >
> > Thanks,
> > Seungwon Jeon
> Hi,
> 
> I'm not sure I understand your reply. mmc_large_sec does exactly what the
> if here does.
> My suggestion is to replace the code with the new function you added.
> I agree it's an old code but this way the code will be consistent and
> mmc_large_sec will be used in every place we would like to check if the
> sector size is 4K.
I just remained. The code you pointed can be replaced.
Regardless this review, I'll rework.
eMMC 4.51 spec was updated in June and there is some changes.
Spec mentions packed header is 8 blocks in 4KB native device.
I'll apply this change.

Thanks,
Seungwon Jeon

> 
> Thanks,
> Maya
> 
> --
> Sent by consultant of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

* RE: [PATCH v8 2/2] mmc: support packed write command for eMMC4.5 device
  2012-07-08 23:52 ` Seungwon Jeon
@ 2012-07-09  7:01   ` merez
  2012-07-09 10:13     ` Seungwon Jeon
  0 siblings, 1 reply; 11+ messages in thread
From: merez @ 2012-07-09  7:01 UTC (permalink / raw)
  To: Seungwon Jeon
  Cc: merez, linux-mmc, 'Chris Ball',
	'Subhash Jadavani', 'S, Venkatraman',
	'Saugata Das', 'Namjae Jeon'


On Sun, July 8, 2012 4:52 pm, Seungwon Jeon wrote:
> Sunday, July 08, 2012, Maya <merez@codeaurora.org> wrote:
>> One minor comment below:
>>
>> > @@ -1278,9 +1609,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
>> *mq, struct request *rqc)
>> >  			    (card->ext_csd.data_sector_size == 4096)) {
>> You can use mmc_large_sec here (instead of
>> card->ext_csd.data_sector_size
>> == 4096)
> Yes, but this line is kept due to another issue related with 4KB sector.
>
> Thanks,
> Seungwon Jeon
Hi,

I'm not sure I understand your reply. mmc_large_sec does exactly what the
if here does.
My suggestion is to replace the code with the new function you added.
I agree it's an old code but this way the code will be consistent and
mmc_large_sec will be used in every place we would like to check if the
sector size is 4K.

Thanks,
Maya

-- 
Sent by consultant of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum


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

* RE: [PATCH v8 2/2] mmc: support packed write command for eMMC4.5 device
  2012-07-08 12:12 merez
@ 2012-07-08 23:52 ` Seungwon Jeon
  2012-07-09  7:01   ` merez
  0 siblings, 1 reply; 11+ messages in thread
From: Seungwon Jeon @ 2012-07-08 23:52 UTC (permalink / raw)
  To: merez
  Cc: linux-mmc, 'Chris Ball', 'Subhash Jadavani',
	'S, Venkatraman', 'Saugata Das',
	'Namjae Jeon'

Sunday, July 08, 2012, Maya <merez@codeaurora.org> wrote:
> One minor comment below:
> 
> > @@ -1278,9 +1609,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
> *mq, struct request *rqc)
> >  			    (card->ext_csd.data_sector_size == 4096)) {
> You can use mmc_large_sec here (instead of card->ext_csd.data_sector_size
> == 4096)
Yes, but this line is kept due to another issue related with 4KB sector. 

Thanks,
Seungwon Jeon

> 
> Thanks,
> Maya
> 
> --
> Sent by consultant of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
> 
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

* Re: [PATCH v8 2/2] mmc: support packed write command for eMMC4.5    device
@ 2012-07-08 12:12 merez
  2012-07-08 23:52 ` Seungwon Jeon
  0 siblings, 1 reply; 11+ messages in thread
From: merez @ 2012-07-08 12:12 UTC (permalink / raw)
  To: Seungwon Jeon
  Cc: linux-mmc, 'Chris Ball', 'Maya Erez',
	'Subhash Jadavani', 'S, Venkatraman',
	'Saugata Das', 'Namjae Jeon'

One minor comment below:

> @@ -1278,9 +1609,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue
*mq, struct request *rqc)
>  			    (card->ext_csd.data_sector_size == 4096)) {
You can use mmc_large_sec here (instead of card->ext_csd.data_sector_size
== 4096)

Thanks,
Maya

-- 
Sent by consultant of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum





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

* [PATCH v8 2/2] mmc: support packed write command for eMMC4.5 device
@ 2012-06-29  4:33 Seungwon Jeon
  0 siblings, 0 replies; 11+ messages in thread
From: Seungwon Jeon @ 2012-06-29  4:33 UTC (permalink / raw)
  To: linux-mmc
  Cc: 'Chris Ball', 'Maya Erez',
	'Subhash Jadavani', 'S, Venkatraman',
	'Saugata Das', 'Namjae Jeon'

This patch supports packed write command of eMMC4.5 device.
Several writes can be grouped in packed command and all data
of the individual commands can be sent in a single transfer
on the bus.

Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
Reviewed-by: Maya Erez <merez@codeaurora.org>
Reviewed-by: Namjae Jeon <linkinjeon@gmail.com>
---
 drivers/mmc/card/block.c   |  394 ++++++++++++++++++++++++++++++++++++++++++--
 drivers/mmc/card/queue.c   |   45 +++++-
 drivers/mmc/card/queue.h   |   12 ++
 drivers/mmc/core/mmc_ops.c |    1 +
 include/linux/mmc/card.h   |    5 +
 include/linux/mmc/core.h   |    4 +
 6 files changed, 444 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index f1c84de..c965f2b 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -58,6 +58,12 @@ MODULE_ALIAS("mmc:block");
 #define INAND_CMD38_ARG_SECTRIM1 0x81
 #define INAND_CMD38_ARG_SECTRIM2 0x88
 
+#define mmc_req_rel_wr(req)	(((req->cmd_flags & REQ_FUA) || \
+			(req->cmd_flags & REQ_META)) && \
+			(rq_data_dir(req) == WRITE))
+#define PACKED_CMD_VER		0x01
+#define PACKED_CMD_WR		0x02
+
 static DEFINE_MUTEX(block_mutex);
 
 /*
@@ -123,9 +129,21 @@ enum mmc_blk_status {
 	MMC_BLK_NOMEDIUM,
 };
 
+enum {
+	MMC_PACKED_N_IDX = -1,
+	MMC_PACKED_N_ZERO,
+	MMC_PACKED_N_SINGLE,
+};
+
 module_param(perdev_minors, int, 0444);
 MODULE_PARM_DESC(perdev_minors, "Minors numbers to allocate per device");
 
+static inline void mmc_blk_clear_packed(struct mmc_queue_req *mqrq)
+{
+	mqrq->packed_cmd = MMC_PACKED_NONE;
+	mqrq->packed_num = MMC_PACKED_N_ZERO;
+}
+
 static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk)
 {
 	struct mmc_blk_data *md;
@@ -1069,12 +1087,61 @@ static int mmc_blk_err_check(struct mmc_card *card,
 	if (!brq->data.bytes_xfered)
 		return MMC_BLK_RETRY;
 
+	if (mq_mrq->packed_cmd != MMC_PACKED_NONE) {
+		if (unlikely(brq->data.blocks << 9 != brq->data.bytes_xfered))
+			return MMC_BLK_PARTIAL;
+		else
+			return MMC_BLK_SUCCESS;
+	}
+
 	if (blk_rq_bytes(req) != brq->data.bytes_xfered)
 		return MMC_BLK_PARTIAL;
 
 	return MMC_BLK_SUCCESS;
 }
 
+static int mmc_blk_packed_err_check(struct mmc_card *card,
+				    struct mmc_async_req *areq)
+{
+	struct mmc_queue_req *mq_rq = container_of(areq, struct mmc_queue_req,
+			mmc_active);
+	struct request *req = mq_rq->req;
+	int err, check, status;
+	u8 ext_csd[512];
+
+	mq_rq->packed_retries--;
+	check = mmc_blk_err_check(card, areq);
+	err = get_card_status(card, &status, 0);
+	if (err) {
+		pr_err("%s: error %d sending status command\n",
+				req->rq_disk->disk_name, err);
+		return MMC_BLK_ABORT;
+	}
+
+	if (status & R1_EXP_EVENT) {
+		err = mmc_send_ext_csd(card, ext_csd);
+		if (err) {
+			pr_err("%s: error %d sending ext_csd\n",
+					req->rq_disk->disk_name, err);
+			return MMC_BLK_ABORT;
+		}
+
+		if ((ext_csd[EXT_CSD_EXP_EVENTS_STATUS] &
+					EXT_CSD_PACKED_FAILURE) &&
+				(ext_csd[EXT_CSD_PACKED_CMD_STATUS] &
+				 EXT_CSD_PACKED_GENERIC_ERROR)) {
+			if (ext_csd[EXT_CSD_PACKED_CMD_STATUS] &
+					EXT_CSD_PACKED_INDEXED_ERROR) {
+				mq_rq->packed_fail_idx =
+				  ext_csd[EXT_CSD_PACKED_FAILURE_INDEX] - 1;
+				return MMC_BLK_PARTIAL;
+			}
+		}
+	}
+
+	return check;
+}
+
 static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
 			       struct mmc_card *card,
 			       int disable_multi,
@@ -1229,10 +1296,206 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
 	mmc_queue_bounce_pre(mqrq);
 }
 
+static u8 mmc_blk_prep_packed_list(struct mmc_queue *mq, struct request *req)
+{
+	struct request_queue *q = mq->queue;
+	struct mmc_card *card = mq->card;
+	struct request *cur = req, *next = NULL;
+	struct mmc_blk_data *md = mq->data;
+	bool en_rel_wr = card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN;
+	unsigned int req_sectors = 0, phys_segments = 0;
+	unsigned int max_blk_count, max_phys_segs;
+	u8 put_back = 0;
+	u8 max_packed_rw = 0;
+	u8 reqs = 0;
+
+	mmc_blk_clear_packed(mq->mqrq_cur);
+
+	if (!(md->flags & MMC_BLK_CMD23) ||
+			!card->ext_csd.packed_event_en)
+		goto no_packed;
+
+	if ((rq_data_dir(cur) == WRITE) &&
+			(card->host->caps2 & MMC_CAP2_PACKED_WR))
+		max_packed_rw = card->ext_csd.max_packed_writes;
+
+	if (max_packed_rw == 0)
+		goto no_packed;
+
+	if (mmc_req_rel_wr(cur) &&
+			(md->flags & MMC_BLK_REL_WR) &&
+			!en_rel_wr)
+		goto no_packed;
+
+	if (mmc_large_sec(card) &&
+			!IS_ALIGNED(blk_rq_sectors(cur), 8))
+		goto no_packed;
+
+	max_blk_count = min(card->host->max_blk_count,
+			card->host->max_req_size >> 9);
+	if (unlikely(max_blk_count > 0xffff))
+		max_blk_count = 0xffff;
+
+	max_phys_segs = queue_max_segments(q);
+	req_sectors += blk_rq_sectors(cur);
+	phys_segments += cur->nr_phys_segments;
+
+	if (rq_data_dir(cur) == WRITE) {
+		req_sectors++;
+		phys_segments++;
+	}
+
+	while (reqs < max_packed_rw - 1) {
+		spin_lock_irq(q->queue_lock);
+		next = blk_fetch_request(q);
+		spin_unlock_irq(q->queue_lock);
+		if (!next)
+			break;
+
+		if (mmc_large_sec(card) &&
+				!IS_ALIGNED(blk_rq_sectors(next), 8)) {
+			put_back = 1;
+			break;
+		}
+
+		if (next->cmd_flags & REQ_DISCARD ||
+				next->cmd_flags & REQ_FLUSH) {
+			put_back = 1;
+			break;
+		}
+
+		if (rq_data_dir(cur) != rq_data_dir(next)) {
+			put_back = 1;
+			break;
+		}
+
+		if (mmc_req_rel_wr(next) &&
+				(md->flags & MMC_BLK_REL_WR) &&
+				!en_rel_wr) {
+			put_back = 1;
+			break;
+		}
+
+		req_sectors += blk_rq_sectors(next);
+		if (req_sectors > max_blk_count) {
+			put_back = 1;
+			break;
+		}
+
+		phys_segments +=  next->nr_phys_segments;
+		if (phys_segments > max_phys_segs) {
+			put_back = 1;
+			break;
+		}
+
+		list_add_tail(&next->queuelist, &mq->mqrq_cur->packed_list);
+		cur = next;
+		reqs++;
+	}
+
+	if (put_back) {
+		spin_lock_irq(q->queue_lock);
+		blk_requeue_request(q, next);
+		spin_unlock_irq(q->queue_lock);
+	}
+
+	if (reqs > 0) {
+		list_add(&req->queuelist, &mq->mqrq_cur->packed_list);
+		mq->mqrq_cur->packed_num = ++reqs;
+		mq->mqrq_cur->packed_retries = reqs;
+		return reqs;
+	}
+
+no_packed:
+	mmc_blk_clear_packed(mq->mqrq_cur);
+	return 0;
+}
+
+static void mmc_blk_packed_hdr_wrq_prep(struct mmc_queue_req *mqrq,
+					struct mmc_card *card,
+					struct mmc_queue *mq)
+{
+	struct mmc_blk_request *brq = &mqrq->brq;
+	struct request *req = mqrq->req;
+	struct request *prq;
+	struct mmc_blk_data *md = mq->data;
+	bool do_rel_wr, do_data_tag;
+	u32 *packed_cmd_hdr = mqrq->packed_cmd_hdr;
+	u8 i = 1;
+
+	mqrq->packed_cmd = MMC_PACKED_WRITE;
+	mqrq->packed_blocks = 0;
+	mqrq->packed_fail_idx = MMC_PACKED_N_IDX;
+
+	memset(packed_cmd_hdr, 0, sizeof(mqrq->packed_cmd_hdr));
+	packed_cmd_hdr[0] = (mqrq->packed_num << 16) |
+		(PACKED_CMD_WR << 8) | PACKED_CMD_VER;
+
+	/*
+	 * Argument for each entry of packed group
+	 */
+	list_for_each_entry(prq, &mqrq->packed_list, queuelist) {
+		do_rel_wr = mmc_req_rel_wr(prq) && (md->flags & MMC_BLK_REL_WR);
+		do_data_tag = (card->ext_csd.data_tag_unit_size) &&
+			(prq->cmd_flags & REQ_META) &&
+			(rq_data_dir(prq) == WRITE) &&
+			((brq->data.blocks * brq->data.blksz) >=
+			 card->ext_csd.data_tag_unit_size);
+		/* Argument of CMD23 */
+		packed_cmd_hdr[(i * 2)] =
+			(do_rel_wr ? MMC_CMD23_ARG_REL_WR : 0) |
+			(do_data_tag ? MMC_CMD23_ARG_TAG_REQ : 0) |
+			blk_rq_sectors(prq);
+		/* Argument of CMD18 or CMD25 */
+		packed_cmd_hdr[((i * 2)) + 1] =
+			mmc_card_blockaddr(card) ?
+			blk_rq_pos(prq) : blk_rq_pos(prq) << 9;
+		mqrq->packed_blocks += blk_rq_sectors(prq);
+		i++;
+	}
+
+	memset(brq, 0, sizeof(struct mmc_blk_request));
+	brq->mrq.cmd = &brq->cmd;
+	brq->mrq.data = &brq->data;
+	brq->mrq.sbc = &brq->sbc;
+	brq->mrq.stop = &brq->stop;
+
+	brq->sbc.opcode = MMC_SET_BLOCK_COUNT;
+	brq->sbc.arg = MMC_CMD23_ARG_PACKED | (mqrq->packed_blocks + 1);
+	brq->sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
+
+	brq->cmd.opcode = MMC_WRITE_MULTIPLE_BLOCK;
+	brq->cmd.arg = blk_rq_pos(req);
+	if (!mmc_card_blockaddr(card))
+		brq->cmd.arg <<= 9;
+	brq->cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
+
+	brq->data.blksz = 512;
+	brq->data.blocks = mqrq->packed_blocks + 1;
+	brq->data.flags |= MMC_DATA_WRITE;
+
+	brq->stop.opcode = MMC_STOP_TRANSMISSION;
+	brq->stop.arg = 0;
+	brq->stop.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC;
+
+	mmc_set_data_timeout(&brq->data, card);
+
+	brq->data.sg = mqrq->sg;
+	brq->data.sg_len = mmc_queue_map_sg(mq, mqrq);
+
+	mqrq->mmc_active.mrq = &brq->mrq;
+	mqrq->mmc_active.err_check = mmc_blk_packed_err_check;
+
+	mmc_queue_bounce_pre(mqrq);
+}
+
 static int mmc_blk_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,
 			   struct mmc_blk_request *brq, struct request *req,
 			   int ret)
 {
+	struct mmc_queue_req *mq_rq;
+	mq_rq = container_of(brq, struct mmc_queue_req, brq);
+
 	/*
 	 * If this is an SD card and we're writing, we can first
 	 * mark the known good sectors as ok.
@@ -1249,11 +1512,74 @@ static int mmc_blk_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,
 			ret = blk_end_request(req, 0, blocks << 9);
 		}
 	} else {
-		ret = blk_end_request(req, 0, brq->data.bytes_xfered);
+		if (mq_rq->packed_cmd == MMC_PACKED_NONE)
+			ret = blk_end_request(req, 0, brq->data.bytes_xfered);
 	}
 	return ret;
 }
 
+static int mmc_blk_end_packed_req(struct mmc_queue_req *mq_rq)
+{
+	struct request *prq;
+	int idx = mq_rq->packed_fail_idx, i = 0;
+	int ret = 0;
+
+	while (!list_empty(&mq_rq->packed_list)) {
+		prq = list_entry_rq(mq_rq->packed_list.next);
+		if (idx == i) {
+			/* retry from error index */
+			mq_rq->packed_num -= idx;
+			mq_rq->req = prq;
+			ret = 1;
+
+			if (mq_rq->packed_num == MMC_PACKED_N_SINGLE) {
+				list_del_init(&prq->queuelist);
+				mmc_blk_clear_packed(mq_rq);
+			}
+			return ret;
+		}
+		list_del_init(&prq->queuelist);
+		blk_end_request(prq, 0, blk_rq_bytes(prq));
+		i++;
+	}
+
+	mmc_blk_clear_packed(mq_rq);
+	return ret;
+}
+static void mmc_blk_abort_packed_req(struct mmc_queue_req *mq_rq)
+{
+	struct request *prq;
+
+	while (!list_empty(&mq_rq->packed_list)) {
+		prq = list_entry_rq(mq_rq->packed_list.next);
+		list_del_init(&prq->queuelist);
+		blk_end_request(prq, -EIO, blk_rq_bytes(prq));
+	}
+
+	mmc_blk_clear_packed(mq_rq);
+}
+
+static void mmc_blk_revert_packed_req(struct mmc_queue *mq,
+				      struct mmc_queue_req *mq_rq)
+{
+	struct request *prq;
+	struct request_queue *q = mq->queue;
+
+	while (!list_empty(&mq_rq->packed_list)) {
+		prq = list_entry_rq(mq_rq->packed_list.prev);
+		if (prq->queuelist.prev != &mq_rq->packed_list) {
+			list_del_init(&prq->queuelist);
+			spin_lock_irq(q->queue_lock);
+			blk_requeue_request(mq->queue, prq);
+			spin_unlock_irq(q->queue_lock);
+		} else {
+			list_del_init(&prq->queuelist);
+		}
+	}
+
+	mmc_blk_clear_packed(mq_rq);
+}
+
 static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 {
 	struct mmc_blk_data *md = mq->data;
@@ -1264,10 +1590,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 	struct mmc_queue_req *mq_rq;
 	struct request *req = rqc;
 	struct mmc_async_req *areq;
+	const u8 packed_num = 2;
+	u8 reqs = 0;
 
 	if (!rqc && !mq->mqrq_prev->req)
 		return 0;
 
+	if (rqc)
+		reqs = mmc_blk_prep_packed_list(mq, rqc);
+
 	do {
 		if (rqc) {
 			/*
@@ -1278,9 +1609,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 			    (card->ext_csd.data_sector_size == 4096)) {
 				pr_err("%s: Transfer size is not 4KB sector size aligned\n",
 					req->rq_disk->disk_name);
+				mq_rq = mq->mqrq_cur;
 				goto cmd_abort;
 			}
-			mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
+
+			if (reqs >= packed_num)
+				mmc_blk_packed_hdr_wrq_prep(mq->mqrq_cur,
+						card, mq);
+			else
+				mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
 			areq = &mq->mqrq_cur->mmc_active;
 		} else
 			areq = NULL;
@@ -1301,8 +1638,15 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 			 * A block was successfully transferred.
 			 */
 			mmc_blk_reset_success(md, type);
-			ret = blk_end_request(req, 0,
+
+			if (mq_rq->packed_cmd != MMC_PACKED_NONE) {
+				ret = mmc_blk_end_packed_req(mq_rq);
+				break;
+			} else {
+				ret = blk_end_request(req, 0,
 						brq->data.bytes_xfered);
+			}
+
 			/*
 			 * If the blk_end_request function returns non-zero even
 			 * though all data has been transferred and no errors
@@ -1335,7 +1679,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 			err = mmc_blk_reset(md, card->host, type);
 			if (!err)
 				break;
-			if (err == -ENODEV)
+			if (err == -ENODEV ||
+				mq_rq->packed_cmd != MMC_PACKED_NONE)
 				goto cmd_abort;
 			/* Fall through */
 		}
@@ -1362,25 +1707,46 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 		}
 
 		if (ret) {
-			/*
-			 * In case of a incomplete request
-			 * prepare it again and resend.
-			 */
-			mmc_blk_rw_rq_prep(mq_rq, card, disable_multi, mq);
-			mmc_start_req(card->host, &mq_rq->mmc_active, NULL);
+			if (mq_rq->packed_cmd == MMC_PACKED_NONE) {
+				/*
+				 * In case of a incomplete request
+				 * prepare it again and resend.
+				 */
+				mmc_blk_rw_rq_prep(mq_rq, card,
+						disable_multi, mq);
+				mmc_start_req(card->host,
+						&mq_rq->mmc_active, NULL);
+			} else {
+				if (!mq_rq->packed_retries)
+					goto cmd_abort;
+				mmc_blk_packed_hdr_wrq_prep(mq_rq, card, mq);
+				mmc_start_req(card->host,
+						&mq_rq->mmc_active, NULL);
+			}
 		}
 	} while (ret);
 
 	return 1;
 
  cmd_abort:
-	if (mmc_card_removed(card))
-		req->cmd_flags |= REQ_QUIET;
-	while (ret)
-		ret = blk_end_request(req, -EIO, blk_rq_cur_bytes(req));
+	if (mq_rq->packed_cmd == MMC_PACKED_NONE) {
+		if (mmc_card_removed(card))
+			req->cmd_flags |= REQ_QUIET;
+		while (ret)
+			ret = blk_end_request(req, -EIO,
+					blk_rq_cur_bytes(req));
+	} else {
+		mmc_blk_abort_packed_req(mq_rq);
+	}
 
  start_new_req:
 	if (rqc) {
+		/*
+		 * If current request is packed, it needs to put back.
+		 */
+		if (mq->mqrq_cur->packed_cmd != MMC_PACKED_NONE)
+			mmc_blk_revert_packed_req(mq, mq->mqrq_cur);
+
 		mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
 		mmc_start_req(card->host, &mq->mqrq_cur->mmc_active, NULL);
 	}
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index e360a97..165d85a 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -175,6 +175,9 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
 	if (!mq->queue)
 		return -ENOMEM;
 
+	INIT_LIST_HEAD(&mqrq_cur->packed_list);
+	INIT_LIST_HEAD(&mqrq_prev->packed_list);
+
 	mq->mqrq_cur = mqrq_cur;
 	mq->mqrq_prev = mqrq_prev;
 	mq->queue->queuedata = mq;
@@ -375,6 +378,35 @@ void mmc_queue_resume(struct mmc_queue *mq)
 	}
 }
 
+static unsigned int mmc_queue_packed_map_sg(struct mmc_queue *mq,
+					    struct mmc_queue_req *mqrq,
+					    struct scatterlist *sg)
+{
+	struct scatterlist *__sg;
+	unsigned int sg_len = 0;
+	struct request *req;
+	enum mmc_packed_cmd cmd;
+
+	cmd = mqrq->packed_cmd;
+
+	if (cmd == MMC_PACKED_WRITE) {
+		__sg = sg;
+		sg_set_buf(__sg, mqrq->packed_cmd_hdr,
+				sizeof(mqrq->packed_cmd_hdr));
+		sg_len++;
+		__sg->page_link &= ~0x02;
+	}
+
+	__sg = sg + sg_len;
+	list_for_each_entry(req, &mqrq->packed_list, queuelist) {
+		sg_len += blk_rq_map_sg(mq->queue, req, __sg);
+		__sg = sg + (sg_len - 1);
+		(__sg++)->page_link &= ~0x02;
+	}
+	sg_mark_end(sg + (sg_len - 1));
+	return sg_len;
+}
+
 /*
  * Prepare the sg list(s) to be handed of to the host driver
  */
@@ -385,12 +417,19 @@ unsigned int mmc_queue_map_sg(struct mmc_queue *mq, struct mmc_queue_req *mqrq)
 	struct scatterlist *sg;
 	int i;
 
-	if (!mqrq->bounce_buf)
-		return blk_rq_map_sg(mq->queue, mqrq->req, mqrq->sg);
+	if (!mqrq->bounce_buf) {
+		if (!list_empty(&mqrq->packed_list))
+			return mmc_queue_packed_map_sg(mq, mqrq, mqrq->sg);
+		else
+			return blk_rq_map_sg(mq->queue, mqrq->req, mqrq->sg);
+	}
 
 	BUG_ON(!mqrq->bounce_sg);
 
-	sg_len = blk_rq_map_sg(mq->queue, mqrq->req, mqrq->bounce_sg);
+	if (!list_empty(&mqrq->packed_list))
+		sg_len = mmc_queue_packed_map_sg(mq, mqrq, mqrq->bounce_sg);
+	else
+		sg_len = blk_rq_map_sg(mq->queue, mqrq->req, mqrq->bounce_sg);
 
 	mqrq->bounce_sg_len = sg_len;
 
diff --git a/drivers/mmc/card/queue.h b/drivers/mmc/card/queue.h
index d2a1eb4..5e04938 100644
--- a/drivers/mmc/card/queue.h
+++ b/drivers/mmc/card/queue.h
@@ -12,6 +12,11 @@ struct mmc_blk_request {
 	struct mmc_data		data;
 };
 
+enum mmc_packed_cmd {
+	MMC_PACKED_NONE = 0,
+	MMC_PACKED_WRITE,
+};
+
 struct mmc_queue_req {
 	struct request		*req;
 	struct mmc_blk_request	brq;
@@ -20,6 +25,13 @@ struct mmc_queue_req {
 	struct scatterlist	*bounce_sg;
 	unsigned int		bounce_sg_len;
 	struct mmc_async_req	mmc_active;
+	struct list_head	packed_list;
+	u32			packed_cmd_hdr[128];
+	unsigned int		packed_blocks;
+	enum mmc_packed_cmd	packed_cmd;
+	int		packed_retries;
+	int		packed_fail_idx;
+	u8		packed_num;
 };
 
 struct mmc_queue {
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 0ed2cc5..4ad994a 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -335,6 +335,7 @@ int mmc_send_ext_csd(struct mmc_card *card, u8 *ext_csd)
 	return mmc_send_cxd_data(card, card->host, MMC_SEND_EXT_CSD,
 			ext_csd, 512);
 }
+EXPORT_SYMBOL_GPL(mmc_send_ext_csd);
 
 int mmc_spi_read_ocr(struct mmc_host *host, int highcap, u32 *ocrp)
 {
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 4aeb4e9..643cd3c 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -294,6 +294,11 @@ static inline void mmc_part_add(struct mmc_card *card, unsigned int size,
 	card->nr_parts++;
 }
 
+static inline bool mmc_large_sec(struct mmc_card *card)
+{
+	return card->ext_csd.data_sector_size == 4096;
+}
+
 /*
  *  The world is not perfect and supplies us with broken mmc/sdio devices.
  *  For at least some of these bugs we need a work-around.
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 1b431c7..d787037 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -18,6 +18,9 @@ struct mmc_request;
 struct mmc_command {
 	u32			opcode;
 	u32			arg;
+#define MMC_CMD23_ARG_REL_WR	(1 << 31)
+#define MMC_CMD23_ARG_PACKED	((0 << 31) | (1 << 30))
+#define MMC_CMD23_ARG_TAG_REQ	(1 << 29)
 	u32			resp[4];
 	unsigned int		flags;		/* expected response type */
 #define MMC_RSP_PRESENT	(1 << 0)
@@ -143,6 +146,7 @@ extern int mmc_app_cmd(struct mmc_host *, struct mmc_card *);
 extern int mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *,
 	struct mmc_command *, int);
 extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int);
+extern int mmc_send_ext_csd(struct mmc_card *card, u8 *ext_csd);
 
 #define MMC_ERASE_ARG		0x00000000
 #define MMC_SECURE_ERASE_ARG	0x80000000
-- 
1.7.0.4



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

end of thread, other threads:[~2012-11-15 10:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-14  8:14 [PATCH v8 2/2] mmc: support packed write command for eMMC4.5 device merez
2012-11-15 10:05 ` Seungwon Jeon
  -- strict thread matches above, loose matches on Subject: below --
2012-07-08 12:12 merez
2012-07-08 23:52 ` Seungwon Jeon
2012-07-09  7:01   ` merez
2012-07-09 10:13     ` Seungwon Jeon
2012-07-10  5:40       ` merez
2012-07-10  5:48         ` merez
2012-11-04  5:05       ` merez
2012-11-14  2:54         ` Chris Ball
2012-06-29  4:33 Seungwon Jeon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.