Re: [PATCH 4/4] mci: dwcmshc-sdhci: add support for Kalray Coolidge v2 SoC eMMC controller

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hello Yann,

On 29.02.24 17:31, Yann Sionneau wrote:
> On 2/29/24 17:19, Ahmad Fatoum wrote:
>> On 29.02.24 16:57, Yann Sionneau wrote:
>>> Kalray Coolidge v2 SoC eMMC controller needs static tx delay tuning even
>>> for basic standard or high speed modes.
>>> +struct dwcmshc_callbacks {
>>> +    void (*init)(struct mci_host *mci, struct device *dev);
>> Why is dev needed? There's already mci->hw_dev and mci->mci->dev
> 
> dev is not needed per se, but I thought that since the callback is used to extend dwcmshc_mci_init() called from generic code:
> 
> https://elixir.bootlin.com/barebox/latest/source/drivers/mci/mci-core.c#L2015
> 
> Then it would be a good idea to provide the same arguments.
> 
> But I can remove it.

No, I agree with your reasoning. No need to change it.

>>> +    void (*set_ios)(struct mci_host *mci, struct mci_ios *ios);
>> You don't actually use set_ios. But I assume you intend to add in future?
>> Maybe add it when it's actually needed?
> 
> Ok indeed it's for the future, in the future to support HS200/HS400 high speed modes
> 
> vendor specific tuning (tx/rx delay lines tuning) would take place in such callback.

I'd prefer you add it when it's needed.

> 
>>
>>> +static void dwcmshc_coolidgev2_init(struct mci_host *mci, struct device *dev)
>>> +{
>>> +    struct dwcmshc_host *host = priv_from_mci_host(mci);
>>> +
>>> +    // configure TX delay to set correct setup/hold for Coolidge V2
>> Nite: If you are going to send v2, please change into /* */ comments
>> for uniformity.
> Ok!

Thanks,
Ahmad

>>
>>
>> Cheers,
>> Ahmad
>>
>>> +    sdhci_write32(&host->sdhci,
>>> +              host->vendor_specific_area + DWCMSHC_GPIO_OUT,
>>> +              tx_delay_static_cfg(0xf) | tx_tuning_clk_sel(4));
>>> +}
>>> +
>>> +struct dwcmshc_callbacks kalray_coolidgev2_callbacks = {
>>> +    .init = dwcmshc_coolidgev2_init,
>>> +};
>>> +
>>>   static struct of_device_id dwcmshc_dt_ids[] = {
>>>       { .compatible = "snps,dwcmshc-sdhci", },
>>> +    { .compatible = "kalray,coolidge-v2-dwcmshc-sdhci", .data = &kalray_coolidgev2_callbacks },
>>>       { }
>>>   };
>>>   
> 
> 
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |