On Wednesday, August 12, 2015 at 03:04:15 PM, Simon Glass wrote: > Hi Marek,
Hi! [...] > >> >> > Why are you passing the @freq into get_mmc_clk() ? Shouldn't you > >> >> > call some clock framework function to determine the input > >> >> > frequency of the DWMMC block from within the get_mmc_clk() > >> >> > implementation instead ? What do you think please ? > >> >> > >> >> Well, yes. If such a clock frame work existed I would call it :-) We > >> >> do have a uclass now so we are getting there. > >> > > >> > Excellent, so do you really need this kind of patch ? :) Why don't you > >> > make just some kind of function -- get_dwmmc_clock() -- and call it > >> > instead ? > >> > >> This is sort-of what is happening. It is calling a function in the > >> host controller - i.e. the SoC's MMC controller. It is one step closer > >> to knowing the input clock to the dwmmc input clock. Note that it is > >> not the clock of the MMC bus itself, but the input clock to the dwmmc > >> logic block. > > > > I don't think I quite understand what you mean here. We're talking about > > obtaining the frequency of the clock which go into the DWMMC IP block, > > right ? > > > > So, if you implement a function, say -- dwmmc_get_upstream_clock() -- and > > call it from within the implementation of the .get_mmc_clk(), which is > > specific for that particular chip of yours*, you don't need this patch. > > Or am I really missing something fundamental ? > > > > *the .get_mmc_clk() is specific to a chip, see for example > > exynos_dw_mmc.c > > The purpose of the existing code (before my change) is to find out the > input frequency of the IP block. By knowing this, the dw_mmc driver > can work out what divisor it needs to achieve a particular MMC bus > clock. > > The implementation of get_mmc_clk() (which will be in the SoC-specific > MMC driver) is indeed the place where the clock is figured out. My > only change is to add a parameter which is the desired bus clock. This > parameter can be ignored, but for implementations which can select the > source clock such that it matches this bus clock, then they can do > this and dw_mmc can just use bypass mode. I see now, this wasn't really clear from the patch description. Shouldn't you introduce another callback for this purpose then, like .set_mmc_clk() instead ? > Unless we pass the bus frequency to get_mmc_clk() it has no way of > knowing what bus clock is required and thus cannot implement this > feature. The feature implementation is entirely within the > implementation of get_mmc_clk() - it just needs one more piece of > information to do its job. I see, thanks for clearing this up! _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
