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

[Michael Trimarchi]

Hi Marek

Il 17/giu/2014 08:03 "Marek Vasut" <marex at denx.de> ha scritto:
>
> 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

Do you think that could be a better option to use flags and runtime detect
instead having hundred of define?

Michael

> 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
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot