Hi On Tue, Jun 17, 2014 at 5:49 PM, Marek Vasut <ma...@denx.de> wrote: > On Tuesday, June 17, 2014 at 08:06:38 AM, Michael Trimarchi wrote: >> Hi Marek >> >> Il 17/giu/2014 08:03 "Marek Vasut" <ma...@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 >> > > <grinb...@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 <ma...@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 <ma...@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(®s->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? > > Eventually, when we transition to DT, this will be indeed needed anyway. But > before that, we need to understand what those flags do (which we don't because > previous patches neglected adding proper documentation).
I suggest as Stefano said to add this option/flags now in preparation of DT transition and start to document flags as you said. Michael _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot