[U-Boot] [RFC PATCH 03/11] mmc: Add support for Qualcomm SDHCI controller

[Mateusz Kulikowski <mateusz.kulikowski@gmail.com>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi Simon, 

On 15.12.2015 19:58, Simon Glass wrote:
> Hi Mateusz,
> 
> On 10 December 2015 at 14:41, Mateusz Kulikowski
> <mateusz.kulikowski@gmail.com> wrote:
[...]
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +static int msm_sdc_clk_init(struct udevice *dev)
>> +{
>> +       uint clk_rate = fdtdec_get_uint(gd->fdt_blob, dev->of_offset,
>> +                                       "clock-frequency", 400000);
>> +       uint clkd[2]; /* clk_id and clk_no */
>> +       fdtdec_get_int_array(gd->fdt_blob, dev->of_offset, "clock", clkd, 2);
>> +       clkd[0] = fdt_node_offset_by_phandle(gd->fdt_blob, clkd[0]);
>> +
>> +       struct udevice *clk = NULL;
>> +       uclass_get_device_by_of_offset(UCLASS_CLK, clkd[0], &clk);
>> +       if (clk)
>> +               clk_set_periph_rate(clk, clkd[1], clk_rate);
>> +
> 
> See comments on the previous patch. Also you could move the DT decode
> code all into your ofdata_to_platdata function (if you like).

But requesting clock must be handled in probe (i.e. when clock is 
already bound) right?

> 
>> +       return 0;
>> +}
>> +
>> +static int msm_sdc_probe(struct udevice *dev)
>> +{
>> +       struct msm_sdhc *prv = dev_get_priv(dev);
>> +       struct sdhci_host *host = &prv->host;
>> +       u32 core_version, core_minor, core_major;
>> +
>> +       host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD | SDHCI_QUIRK_BROKEN_R1B;
>> +
>> +       /* Init clocks */
>> +       if (msm_sdc_clk_init(dev))
>> +               return -EIO;
>> +
>> +       /* Reset the core and Enable SDHC mode */
>> +       writel(readl(prv->base + SDCC_MCI_POWER) | SDCC_MCI_POWER_SW_RST,
>> +              prv->base + SDCC_MCI_POWER);
>> +
>> +       /* SW reset can take upto 10HCLK + 15MCLK cycles. (min 40us) */
>> +       mdelay(2);
> 
> So why such a long delay? Perhaps you should put the code immediately
> below int a timeout loop?
> 
> start = get_timer(0);
> while (readl...) {
>    if (get_timer(start) > 2)
>       return -ETIMEDOUT;
> }
> 
I'm not sure if I can do that - I can test it, perhaps it will even work on 
my board, but it was put explicitly in Linux driver (1-5ms delay).

Documentation says:
"SW should wait until bit 0 (MCLK_REG_WR_ACTIVE) of MCI_STATUS2 register 
is 0, after writing to MCI_POWER register and before accessing this 
register again."

But this applies to all writes, not to reset request - I'm not sure 
how MCI_STATUS2 will behave during reset (it may get zeroed earlier).

Do you really think I should try to decrease this delay?

[...]
>> +}
>> +
>> +static int msm_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +       struct msm_sdhc *priv = dev_get_priv(dev);
>> +       struct sdhci_host *host = &priv->host;
>> +
>> +       host->name = strdup(dev->name);
>> +       host->ioaddr = (void *)dev_get_addr(dev);
>> +       host->bus_width = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
>> +                                        "bus-width", 4);
>> +       host->index = fdtdec_get_uint(gd->fdt_blob, dev->of_offset, "index", 0);
>> +       priv->base = fdtdec_get_addr_size_auto_parent(gd->fdt_blob,
>> +                                                     dev->parent->of_offset,
>> +                                                     dev->of_offset, "reg",
>> +                                                     1, NULL);
> 
> Odd that you are reading the second cell. How come?

I tried to use as much parts of original qcom Linux dts as possible.
For mmc controller, there are 2 addresses needed:
- - SDCC base (i.e. base of IP block with vendor-specific regs etc.)
- - SDHCI base (i.e. place where SDHCI "standard" registers start)

My assumption is that SDHCI offset may be different on different 
SoCs, and wanted to have generic driver.

[...]
Regards,
Mateusz

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJWcenTAAoJELvtohmVtQzBddYH+waGw+OePJ8MdeMgI8BYJ295
3l+Op/2FGquWNIIoJRlO59JOeZqVkJJWOph19DQWpmybIra1HoRRmVTVqY2l2Fon
vTdzZ+cZ3jY2bzLGzYARXCEJ4jNxUILkPS/r/SrRfhDVC4vxSROSA7Jh6EmnssDm
kktzR98/o834uDQc0niGWYi9K1hpUnwQ3m8b1e8LmwdH3LcuHH7+UWntTnEesBai
Ox3ES6y5S9VtCBnyZ/LSilqe30AVB4ccKlk69G41AQYPI5u5CZEfm1OP8AGjpkfV
0D4WrErh/CGmYpG0b4G14CYe39GekCmD9IAnMC4zxjRwwZCmp1kGoeunNwpiLjk=
=scwU
-----END PGP SIGNATURE-----