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

Jaehoon Chung jh80.chung at samsung.com
Mon Feb 17 23:35:31 CET 2020


Hi Faiz,

On 2/17/20 9:42 PM, Faiz Abbas wrote:
> 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

Yes, I checked wrong sequence. Sorry.

When i have checked, some functions are needs.

> 
> 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?

Right. but not init. Just platform_init?

Well, if you want, i will make patch about callback function.

Best Regards,
Jaehoon Chung

> 
> Thanks,
> Faiz
> 
> 



More information about the U-Boot mailing list