[U-Boot] [PATCH 4/6] mmc: fsl_esdhc: Add support to force VSELECT set

[Marek Vasut]

On Monday, June 16, 2014 at 01:48:11 PM, Otavio Salvador wrote:
> On Mon, Jun 16, 2014 at 4:03 AM, Igor Grinberg <grinberg at compulab.co.il> 
wrote:
> > Hi Otavio,
> > 
> > On 06/16/14 05:24, Otavio Salvador wrote:
> >> On Sun, Jun 15, 2014 at 11:03 PM, Marek Vasut <marex at denx.de> wrote:
> >>> On Monday, June 16, 2014 at 03:39:08 AM, Otavio Salvador wrote:
> >>>> On Sun, Jun 15, 2014 at 10:27 PM, Marek Vasut <marex at denx.de> wrote:
> >>>>> On Monday, June 16, 2014 at 03:22:22 AM, Otavio Salvador wrote:
> >>>>> 
> >>>>> [...]
> >>>>> 
> >>>>>>>> +#ifdef CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT
> >>>>>>>> +     esdhc_setbits32(&regs->vendorspec,
> >>>>>>>> ESDHC_VENDORSPEC_VSELECT); +#endif
> >>>>>>> 
> >>>>>>> Documentation is missing.
> >>>>>> 
> >>>>>> There is no FSL ESDHC README file so that's why I didn't include it
> >>>>>> anywhere.
> >>>>> 
> >>>>> I'm at loss for words here, really...
> >>>>> 
> >>>>> I think you know what needs to be done (hint: write the
> >>>>> documentation), right ?
> >>>> 
> >>>> I won't write the full documentation for it. I am sorry.
> >>> 
> >>> Undocumented configuration option is not acceptable, period.
> >> 
> >> Who accepted the driver in the first version, without Doc?
> >> 
> >> I am not in the position to write the full doc.
> > 
> > I think there is a misunderstanding here...
> > I think Marek does not want to say that you need to write the full
> > documentation for the driver, but only document the
> > CONFIG_SYS_FSL_ESDHC_FORCE_VSELECT configuration option (what does it do
> > when you define it and why should one define it).
> 
> Great but where if it does not exist?
> 
> should I make a README.fsl-esdhc and include just it?

I briefly looked over the options in the driver, prefixed with hash are the ones 
which are not driver specific:

CONFIG_ESDHC_DETECT_8_BIT_QUIRK
CONFIG_ESDHC_DETECT_QUIRK
CONFIG_FSL_ESDHC_PIN_MUX
CONFIG_FSL_USDHC
#CONFIG_MX53
#CONFIG_OF_LIBFDT
CONFIG_SYS_FSL_ERRATUM_ESDHC111
CONFIG_SYS_FSL_ERRATUM_ESDHC135
CONFIG_SYS_FSL_ERRATUM_ESDHC_A001
CONFIG_SYS_FSL_ESDHC_ADDR
CONFIG_SYS_FSL_ESDHC_USE_PIO
CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33
#CONFIG_SYS_MMC_MAX_BLK_COUNT
#CONFIG_SYS_SD_VOLTAGE
#CONFIG_T4240QDS

It looks like completely ad-hoc addition of new and new config options as 
whoever seen fit to me, both from their names and git log of the driver. This 
makes the driver completely unmaintainable. I would like to see them documented 
before any new config options are added, so please instead of fighting the 
mailing list, spend 10 minutes of your time and document them.

Best regards,
Marek Vasut