On Mon, Jun 11, 2018 at 1:26 AM, Y.b. Lu <yangbo.lu@xxxxxxx> wrote: > > Hi Leo, > > > -----Original Message----- > > From: Li Yang [mailto:leoyang.li@xxxxxxx] > > Sent: Thursday, June 7, 2018 4:04 AM > > To: Ulf Hansson <ulf.hansson@xxxxxxxxxx> > > Cc: Yinbo Zhu <yinbo.zhu@xxxxxxx>; Y.b. Lu <yangbo.lu@xxxxxxx>; Adrian > > Hunter <adrian.hunter@xxxxxxxxx>; linux-mmc@xxxxxxxxxxxxxxx; Xiaobo Xie > > <xiaobo.xie@xxxxxxx> > > Subject: Re: [PATCH v2] mmc: sdhci-of-esdhc: modify the sd clock in > > soc_device_match way > > > > On Wed, May 23, 2018 at 4:21 AM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> > > wrote: > > > On 23 May 2018 at 10:55, Yinbo Zhu <yinbo.zhu@xxxxxxx> wrote: > > >> > > >> -----Original Message----- > > >> From: Ulf Hansson [mailto:ulf.hansson@xxxxxxxxxx] > > >> Sent: 2018年5月23日 14:41 > > >> To: Yinbo Zhu <yinbo.zhu@xxxxxxx> > > >> Cc: Y.b. Lu <yangbo.lu@xxxxxxx>; Adrian Hunter > > >> <adrian.hunter@xxxxxxxxx>; linux-mmc@xxxxxxxxxxxxxxx; Xiaobo Xie > > >> <xiaobo.xie@xxxxxxx> > > >> Subject: Re: [PATCH v2] mmc: sdhci-of-esdhc: modify the sd clock in > > >> soc_device_match way > > >> > > >> On 23 May 2018 at 05:50, Yinbo Zhu <yinbo.zhu@xxxxxxx> wrote: > > >>> > > >>> > > >>> -----Original Message----- > > >>> From: Ulf Hansson [mailto:ulf.hansson@xxxxxxxxxx] > > >>> Sent: 2018年5月22日 20:16 > > >>> To: Yinbo Zhu <yinbo.zhu@xxxxxxx> > > >>> Cc: Y.b. Lu <yangbo.lu@xxxxxxx>; Adrian Hunter > > >>> <adrian.hunter@xxxxxxxxx>; linux-mmc@xxxxxxxxxxxxxxx; Xiaobo Xie > > >>> <xiaobo.xie@xxxxxxx> > > >>> Subject: Re: [PATCH v2] mmc: sdhci-of-esdhc: modify the sd clock in > > >>> soc_device_match way > > >>> > > >>> On 22 May 2018 at 14:06, Yinbo Zhu <yinbo.zhu@xxxxxxx> wrote: > > >>>> From: yinbo.zhu <yinbo.zhu@xxxxxxx> > > >>>> > > >>>> Convert to use soc_device_match method to fix up eSDHC clock for > > >>>> ls1046a/ls1012a/p1010. Also add eSDHC clock fixup for ls1021a > > >>>> according to its datasheet. The maxmum speed for ls1021a eSDHC high > > >>>> speed mode is 46.5MHz. > > >>> > > >>>>Why not use mmc DT property "max-frequency" instead? > > >>> > > >>>>Why exactly do you need a different max frequency depending on the > > selected speed mode? > > >>> > > >>>>Kind regards > > >>>>Uffe > > >>> Accroding to datasheet that the different max frequency depending on > > >>> the slected speed mode and SoC, > > >> > > >>>That it depends on the SoC, that I am or course fine with. > > >> > > >>>My point is rather that I think it depends on the actual slot connector (or > > board), rather that on the speed mode. > > >>>For example, one port may have an eMMC and for some reason it can be > > >>>run in a speed mode using full clock rate. Another one may be an > > >>>SD-card slot, and perhaps because of some external logic, the signal quality > > doesn't survive a too high clock- rate. Etc. > > >> > > >>>Other SoCs and platforms works like this, I am sure yours do as well. No? > > >> > > >>> In the current state that which only shows one of the max-frequency, > > >>> so it can't use mmc DT property "max-frequency" instead > > >> > > >>>According to my comments above, could you please investigate once > > >>>more, whether really "max-frequency" can be used as the DT binding? > > >> > > >>>If it can't then I think we should consider introducing new mmc DT > > bindings instead. > > >> > > >>>[...] > > >> > > >>>Kind regards > > >>>Uffe > > >> Hi Uffe, > > >> > > >> Attachment is some pictures which in data sheet, I think it can explain that > > max frequency depending on the slected speed mode and SoC, You can have a > > look. > > >> > > > > > > Thanks, yes, this seems to confirm it. Kind of weird that none until > > > now have raised issues about similar constraints to be needed. > > > > > > So, we can either use your suggested solution, via soc_device_match() > > > method, if you prefer, else we need to invent a new DT property for > > > each speed mode constraint. > > > > Yinbo, > > > > According to the comment of the soc_device_match() API, it is for cases when > > device tree doesn't provide enough information to identify the variant. But I > > don't think it is the case for the SoCs you added in this patch, they all have > > unique compatible strings in the dts files. Why don't we use of_match_node() > > instead? > > > > [Y.b. Lu] By now the of_match_node() could handle this case. What if this kind erratum happens on specific soc revision in the future? If that happens in the future, we should define new compatible string for that revision. "For new devices, the DT binding should always provide unique compatible strings that allow the use of of_match_node() instead." quoted from soc_device_match(). If there is new problem found on old SoCs without unique compatible string, we could then use this soc_device_match() mechanism. Regards, Leo -- 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