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