Re: [PATCH 1/1] [PATCH v4 1/1] mmc: Support of DUAL BUFFER DESC[ring] mode for dw_mmc

[James Hogan <james@albanarts.com>]

Hi Shashidhar,

On Mon, Nov 14, 2011 at 09:32:25PM +0530, Shashidhar Hiremath wrote:
> Hi Will,
> 
> Just following up since i did not see any response from you on my
> earlier mail.. In previous mail, im not sure whether i gave you enough
> context/background.. So here it goes (apologies in advance if this
> turns out to be a  lengthy mail...):
> 
> 1>At Vayavya Labs, our work is to make sure that drivers for SNPS
> designware IP is available in kernel.org. One of the
> mandates/requirements from SNPS is that the driver submitted to
> kernel.org should support all the features/configurations of the IP.
> The bigger picture being that once this is done, SNPS customers dont
> have to write code for any specific feature/configuration since it is
> already available in the driver and they can start using it
> immediately...
> 2>As such, it is required that code for dual descriptors be also
> present in the driver submitted to kernel.org.. In future there, there
> could be some person wanting to use this feature of the IP..
> 3>Also note that in kconfig, there does not have to be a choice
> between this mode and chained mode since the default will always be
> chained mode. STM Ethernet MAC driver already has such a kind of
> support.. If this is not acceptable, do you feel the code should be
> wrapped in some totally different preprocessor directive with comments
> in the driver explaining the same?
> 4>With regards to your points on performance, I am open to look at it
> and see where we can make improvements in the driver (if any... if its
> a hardware thing, there is not much we can do)

Can I suggest you try running mmc_test with and without the dual
descriptor mode. It has a bunch of performance tests in it.

Cheers
James

> 
> Do let me know your views.
> 
> best regards
> --Shashidhar Hiremath
> 
> On Fri, Nov 4, 2011 at 12:36 PM, Shashidhar Hiremath
> <shashidharh@vayavyalabs.com> wrote:
> > Hi Will,
> >   I agree with you that it should not be merged  unless it improves
> > the performance. But I still feel that there might be some reason for
> > which the IP Vendor has provided an additional feature. So will this
> > not be a good feature to have as it will help in IP Validation for
> > different modes.
> >  As far as the Kconfig option is concerned, the user need not always
> > modify it since the default case will still be Chained Mode. Also Such
> > Kconfig options for selecting mode is already  used for stmmac
> > Ethernet Drivers.
> >
> > On Thu, Nov 3, 2011 at 8:48 PM, Will Newton <will.newton@gmail.com> wrote:
> >> On Thu, Nov 3, 2011 at 12:35 PM, Chris Ball <cjb@laptop.org> wrote:
> >>> Hi,
> >>>
> >>> On Thu, Nov 03 2011, Shashidhar Hiremath wrote:
> >>>> Hi Chris,
> >>>>   Can this patch be accepted by criteria that its an additional
> >>>> feature supported by the hardware and hence good to have the support
> >>>> in the driver.Also note the patch has been tested.
> >>>
> >>> I think Will and James should make the call on that.
> >>>
> >>> My own opinion is that it's not usually a good idea to merge code that
> >>> increases complexity for no performance gain; if the feature is actually
> >>> important, someone should find a way to finish it and measure a
> >>> performance gain (the gain can be in any of bandwidth, memory, or
> >>> lower CPU utilization) with it, to prove that the change is worthwhile.
> >>
> >> I'm inclined to agree. I don't want to feel like I am blocking
> >> inclusion of anyone's hard work, but unless there is a clear advantage
> >> for one option over the other I can't see a good reason for merging
> >> it. At present it adds a question to the Kconfig that is pretty much
> >> impossible for the user to answer (do I turn this feature on or off?
> >> what is the advantage of choosing each option?).
> >>
> >
> >
> >
> > --
> > regards,
> > Shashidhar Hiremath
> >
> 
> 
> 
> -- 
> regards,
> Shashidhar Hiremath
> --
> 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