[PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static

[Faiz Abbas]

Jaehoon,

On 17/02/20 5:47 pm, Jaehoon Chung wrote:
> Hi,
> 
> On 2/5/20 4:33 PM, Faiz Abbas wrote:
>> Hi Peng,
>>
>> On 05/02/20 12:58 pm, Peng Fan wrote:
>>>> Subject: Re: [PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static
>>>>
>>>> Hi,
>>>>
>>>> On 31/01/20 3:55 am, Simon Goldschmidt wrote:
>>>>> Am 30.01.2020 um 23:21 schrieb Jaehoon Chung:
>>>>>> Hi Simon,
>>>>>>
>>>>>> On 1/29/20 11:16 PM, Simon Goldschmidt wrote:
>>>>>>> On Wed, Jan 29, 2020 at 12:00 AM Jaehoon Chung
>>>>>>> <jh80.chung at samsung.com> wrote:
>>>>>>>>
>>>>>>>> On 1/24/20 8:52 PM, Faiz Abbas wrote:
>>>>>>>>> Expose sdhci_init() as non-static.
>>>>>>>>
>>>>>>>> Does it need to change to non-static?
>>>>>>>
>>>>>>> And even if it needs to, can we have a reason *why* in the commit
>>>>>>> message?
>>>>>>
>>>>>> When i read entire your series, it seems that doesn't need to change
>>>>>> to non-static.
>>>>>> All of change that touched MMC code are only for your board.
>>>>>> I don't know Peng's opinion, but it's not my prefer.
>>>>>
>>>>> +1!
>>>>>
>>>>> We need to keep the core code clean of such hacks in order to keep the
>>>>> size small for constrained targets!
>>>>>
>>>>
>>>> Peng can you comment on this?
>>>
>>> Just one more question, does kernel has same issue, how resolved?
>>> Could U-Boot follow similar approach?
>>
>> Kernel has interrupts enabled. So software just has to wait for the card
>> detect interrupt to arrive rather than poll on anything.
>>
>>>
>>> Actually I am fine with platform specific approach , considering
>>> your platform issue.
>>>
>>> But since Simon and Jaehoon has concerns, not sure whether
>>> Simon and Jaehoon have good ideas.
>>>
>>
>> Ok. Lets see if they have some better ideas.
> 
> Well, Your code needs to call am654_sdhci_init() before sdhci_init(), right?
> 
> ops->init() -> am654_sdhci_init -> sdhci_init().
> If am654_sdhci_init is called for preset, how about adding pre_init() or other ops callback.
> (pre_init is just example.)
> 
> ops->pre_init = am654_sdhci_init; or ops->preset = am654_sdhci_preset;
> 
> In mmc.
> 
> ops->preset or ops->pre_init  -> ops->init 
> 
> How about adding plotform specific init callback? Then someone can also use it for platform specific things in future.
> 

So we basically want an init() callback even in sdhci.c.

I have to create one more wrapper sdhci_pltaform_init() API in the sdhci
driver that just calls a platform init function inside it. So its

include/sdhci.h:

struct sdhci_ops {
..
+       int (*platform_init)()

and then drivers/mmc/sdhci.c:


+sdhci_platform_init()
+{
+...
+       host->ops->platform_init();
+}

const struct dm_mmc_ops sdhci_ops  = {
...
+       .init           = sdhci_platform_init,
};

Right?

Thanks,
Faiz