Hi Mateusz,
On 16 December 2015 at 15:46, Mateusz Kulikowski
<mateusz.kulikow...@gmail.com> wrote:
> -----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.kulikow...@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?
Yes. Although bear in mind that the ofdata_to_platdata() call is made
just before probe() so it is OK to put 'get' calls in there. In
general all devices are bound before any are probed.
>
>>
>>> + 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?
Can you change the code to wait until bit 0 is 0, instead of having a 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.
OK I see, thanks.
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
