RE: [PATCH v5 2/2] mmc: core: Support packed command for eMMC4.5 device

[Seungwon Jeon]

Maya Erez <merez@xxxxxxxxxxxxxx> wrote:
> > Maya Erez <merez@xxxxxxxxxxxxxx> wrote:
> >> > Hi. Merez.
> >> >
> >> > Thanks a lot about your performance measurement.
> >> >
> >> > I think that your measurement is enough and correct and the firmware
> >> > of mmc vender should be optimized or change properly rather than
> >> > modifying the current patch.
> >> >
> >> > And currently we can use only write packed cmd by my suggestion.
> >> >
> >> > I would like to add my reviewd-by tag in updated patches also.
> >> >
> >> > Reviewed-by: Namjae Jeon <linkinjeon@xxxxxxxxx>
> >> >
> >> > Thanks.
> >>
> >> I tend to disagree. Adding a massive amount of code that would be
> >> disabled
> >> can be risky. In case this code will not be in use it will not be
> >> properly
> >> tested and its reliability will be uncertain.
> >>
> > If you found something to be correct, please let me know that.
> > It would be rightly appreciated.
> >
> > Best regards,
> > Seungwon Jeon.
> Hi Jeon,
> 
> The write packing code looks good to me.
> However, the separation of read and write packing to different patches is
> very important to us.
> As I specified before, we decided to enable only the write packing. We
> plan to thoroughly test the write packing (edge cases and error handling)
> and will not test the read packing. Therefore we would like to have the
> ability to get only the write packing code.
As Namjae Jeon mentioned, how about this?
I think only MMC_CAP2_PACKED_WR can be set for enabling the write packing easily.
In my case, tested eMMC device is not optimized for packed read.
So I couldn't confirm that this patch is effective in packed read.
I think packed read as well as packed write of this patch conformed with the eMMC4.5 spec though.
I wonder that your eMMC device has the good ability in both operations.
It is difficult to decide the performance with excluding the device.
Soon I will test it with the improved sample for packed read.

> In my previous comment I talked about the risk of mainlining  a “dead”
> code. Every feature that is integrated is considered to be fully tested
> and in the future it might be enabled, assuming that is was already
> tested.
Right! It is desirable and I hope that.
Do you think this patch have the potential problem?
As I also ask you, if you have tested and find something is incorrect, we can discuss that.
It was submitted for that purpose.

> Can you please specify how you tested the read and write packing? Did you
> perform edge cases and error handling tests? Do you have test code that
> can be shared?
Basically, It has been tested with several I/O benchmark tool.
Some misvalued I/O timing and wrong argument for packed command 
was used for triggering the error case.

Best regards,
Seungwon Jeon.
> 
> Thanks,
> Maya Erez
> Consultant for 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@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/