>> Hmm. Looking a bit deeper into this, we have some additional related >> code to fixup. :-) >> >> In ->probe(), we do clk_set_rate(100MHz), if the mclk > 100MHz. That's >> due to the current variants don't support higher frequency than this. >> It seems like the Qcom variant may support up to 208MHz? Now, if >> that's the case, we need to add "f_max" to the struct variant_data to >> store this information, so we can respect different values while doing >> clk_set_rate() at ->probe(). >> > Yes, qcom SOCs support more than 100Hhz clock. > > Probe and clk_set_rate/set_ios should respect this. > > On the other thought, Should probe actually care about clocks which are > explicitly controlled? It should not even attempt to set any frequency to > start with. The 100 MHz is related to constraints set by the specification of the IP block, not the MMC/SD/SDIO spec. Thus at ->probe() we must perform the clk_set_rate(). > mmc-core would set the right frequency depending on the mmc > state-machine respecting f_min and f_max. > I think for qcom, probe should just check the if f_max and f_min are valid > and set them to defaults if any in the same lines as existing code. > > >> While updating mmc->f_max for host->variant->explicit_mclk_control, we >> shouldn't care about using the host->mclk as a limiter, instead, use >> min(mmc->f_max, host->variant->f_max) and fallback to fmax. >> > Yes, that's correct, mclk should not be used as limiter. Adding f_max to the > variant looks useful. > > >> Not sure how that will affect the logic. :-) >> >>> >>> >>>> Additionally, this makes me wonder about f_min. I haven't seen >>>> anywhere in this patch were that value is being set to proper value, >>>> right? >>>> >>> >>> f_min should be 400000 for qcom, I think with the default mclk frequency >>> and >>> a divider of 512 used for calculating the f_min is bringing down the >>> f_min >>> to lessthan 400Kz. Which is why its working fine. >>> I think the possibility of mclk default frequency being greater than >>> 208Mhz >>> is very less. so I could either leave it as it is Or force this to 400000 >>> all the time for qcom chips. >> >> >> No, this seems like a wrong approach. >> >> I think you would like to do use the clk_round_rate() find out the >> lowest possible rate. Or just use a fixed fallback value somehow. > > > clk_round_rate(mclk, 0) might be more generic, instead of fixed fallback. > Let the mmc-core figure out what frequency would be best from its table > starting from f_min. Agree! clk_round_rate(mclk, 100KHz), might be better though - since that is actually the lowest request frequency whatsoever. Kind regards Ulf Hansson -- 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
