On 15 January 2015 at 15:17, Arend van Spriel <arend@xxxxxxxxxxxx> wrote: > On 01/15/15 15:07, Arend van Spriel wrote: >> >> On 01/15/15 14:39, Ulf Hansson wrote: >>> >>> On 15 January 2015 at 11:17, Adrian Hunter<adrian.hunter@xxxxxxxxx> >>> wrote: >>>> >>>> On 14/01/15 14:59, Ulf Hansson wrote: >>>>> >>>>> [...] >>>>> >>>>>>> >>>>>>> The value from the register is also just randomly selected, only >>>>>>> difference is that it's the HW that has randomly set it. >>>>>> >>>>>> >>>>>> Presumably the value is chosen based on the maximum rate of >>>>>> temperature >>>>>> change and the corresponding effect that has on the signal. >>>>>> >>>>>>> >>>>>>> Even if the above commit was merged, I don't think it was the correct >>>>>>> way of dealing with re-tuning. >>>>>>> >>>>>>> First of all, re-tuning this is a mmc protocol specific thing should >>>>>>> be managed from the mmc core, like the approach you have taken in >>>>>>> your >>>>>>> $subject patchset. Second I question whether the timer is useful at >>>>>>> all. >>>>>> >>>>>> >>>>>> The SD Host Controller Specification does not document another way >>>>>> to do >>>>>> mode 1 re-tuning. The timer is it. Otherwise re-tuning is never done. >>>>>> >>>>>> In the patches I sent, the driver must call mmc_retune_needed() to set >>>>>> host->need_retune = 1 otherwise mmc_retune() does nothing. >>>>>> >>>>>> I would like to extend the model to include transparently re-tuning >>>>>> and >>>>>> re-trying when there is a CRC error, but that is a separate issue, not >>>>>> documented in the spec but recommended by others. >>>>>> >>>>> >>>>> That perfect and in line from what I heard as recommendations from >>>>> memory vendors as well. >>>> >>>> >>>> How would that work for SDIO? How do you know it is OK to retry SDIO >>>> operations? >>> >>> >>> Retries or error handling, needs to be handled from SDIO func drivers >>> or upper level code. They certainly also need it for other errors, >>> which are not caused by the lack of a re-tune. I believe they exist >>> already. >>> >>> For mmc core point of view, we need to act on SDIO data transfers >>> errors and perform re-tuning for cases when it makes sense. >>> >>> More importantly, using a timer won't make SDIO data transfers error >>> free, since we can still end up needing a re-tune at any point. >>> >>> Still, you do have point for SDIO. Minimizing the number of errors for >>> SDIO could be important, due to that an SDIO func driver may not be >>> able to recover from data errors as smoothly as the mmc block layer >>> can. Thus, a timer could help to improve the situation, but I think it >>> only makes sense in the SDIO case. >>> >>> BTW, what's your experience around SDIO cards supporting SDR104. I >>> have never used such, have you? >> >> >> My primary focus in all this discussing is about SDIO cards. The main >> reason being that our 11ac wifi SDIO cards do support SDR104. So the >> brcmfmac driver support SDIO and has retry mechanisms in place. However, >> it may also end-up doing an abort under certain conditions. >> >> You also mentioned using runtime-pm, but how do you deal with func >> drivers not supporting runtime-pm. That is already an issue aka. bug >> right now. Our driver does not support runtime-pm (yet) and we have >> reported issues that host controller does runtime-pm basically killing >> communication between device and func driver. > Runtime PM is implemented a bit differently between SDIO vs MMC/SD. Your are right. For MMC/SD the mmc block device handles pm_runtime_get|put() in principle per request basis and makes use of the pm_runtime_autosuspend feature. While in the SDIO case, it's entirely up the SDIO func driver to deal with pm_runtime_get|put(). So it seems like we can use runtime PM for MMC/SD but not for SDIO. At least not using the SDIO func device. > > Could leave it to the function driver to call mmc_retune_needed(). Hmm, the positive side from such approach would be that the SDIO func driver can decide when it's convenient to do a re-tune. The negative side is that all SDIO func driver would need to care about this. I am not sure we want that. Kind regards Uffe -- 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
