On 14 July 2017 at 11:40, Chen-Yu Tsai <wens@xxxxxxxx> wrote:
> On Fri, Jul 14, 2017 at 5:26 PM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>> On 14 July 2017 at 08:42, Chen-Yu Tsai <wens@xxxxxxxx> wrote:
>>> On the SoCs that introduced the new timing mode for MMC controllers,
>>> both the old (where the clock delays are set in the CCU) and new
>>> (where the clock delays are set in the MMC controller) timing modes
>>> are available, and we have to support them both. However there are
>>> two bits that control which mode is active. One is in the CCU, the
>>> other is in the MMC controller. The settings on both sides must be
>>> the same, or nothing will work.
>>>
>>> The CCU's get/set_phase callbacks return -ENOTSUPP when the new
>>> timing mode is active. This provides a way to know which mode is
>>> active on that side, and we can set the bit on the MMC controller
>>> side accordingly.
>
> Argh... I forgot to update the commit log... :(
>
>>>
>>> Signed-off-by: Chen-Yu Tsai <wens@xxxxxxxx>
>>> ---
>>> drivers/mmc/host/sunxi-mmc.c | 34 ++++++++++++++++++++++++++++++----
>>> 1 file changed, 30 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
>>> index 0fb4e4c119e1..56e45c65b52d 100644
>>> --- a/drivers/mmc/host/sunxi-mmc.c
>>> +++ b/drivers/mmc/host/sunxi-mmc.c
>>> @@ -22,6 +22,7 @@
>>> #include <linux/err.h>
>>>
>>> #include <linux/clk.h>
>>> +#include <linux/clk/sunxi-ng.h>
>>
>> I don't like this. This looks like an SoC specific hack.
>>
>>> #include <linux/gpio.h>
>>> #include <linux/platform_device.h>
>>> #include <linux/spinlock.h>
>>> @@ -259,7 +260,7 @@ struct sunxi_mmc_cfg {
>>> /* Does DATA0 needs to be masked while the clock is updated */
>>> bool mask_data0;
>>>
>>> - bool needs_new_timings;
>>> + bool has_new_timings;
>>> };
>>>
>>> struct sunxi_mmc_host {
>>> @@ -293,6 +294,9 @@ struct sunxi_mmc_host {
>>>
>>> /* vqmmc */
>>> bool vqmmc_enabled;
>>> +
>>> + /* timings */
>>> + bool use_new_timings;
>>> };
>>>
>>> static int sunxi_mmc_reset_host(struct sunxi_mmc_host *host)
>>> @@ -714,7 +718,7 @@ static int sunxi_mmc_clk_set_phase(struct sunxi_mmc_host *host,
>>> {
>>> int index;
>>>
>>> - if (!host->cfg->clk_delays)
>>> + if (host->use_new_timings)
>>> return 0;
>>>
>>> /* determine delays */
>>> @@ -765,6 +769,15 @@ static int sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *host,
>>> ios->bus_width == MMC_BUS_WIDTH_8)
>>> clock <<= 1;
>>>
>>> + if (host->use_new_timings) {
>>> + ret = sunxi_ccu_set_mmc_timing_mode(host->clk_mmc, true);
>>
>> Can't this be solved through some other generic API/interface?
>
> The old discussion is here: https://lkml.org/lkml/2017/5/5/77
>
> It is possible to piggy back on existing API, but as Maxime mentioned
> back in the discussion, it is confusing.
>
> IIRC Mike said (via Maxime) an SoC specific call was the easy way
> to handle this. I don't think there's anything generic about this.
> Even if you could have a _set_mode callback for the clks, the modes
> would be SoC specific anyway.
Right. But it would benefit that we can keep drivers generic, as they
are using generic APIs/interfaces. I prefer that.
Anyway, let me try to dig up the earlier discussion.
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
