From mboxrd@z Thu Jan 1 00:00:00 1970 From: Faiz Abbas Date: Mon, 17 Feb 2020 18:12:57 +0530 Subject: [PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static In-Reply-To: <77dac8dd-5735-0da1-061d-2cce3719ebac@samsung.com> References: <20200124115252.15712-1-faiz_abbas@ti.com> <20200124115252.15712-5-faiz_abbas@ti.com> <4a860cbf-2712-c1e2-91ed-9f878724a3a3@samsung.com> <5da4edf9-8992-5414-5511-2d78449b070b@samsung.com> <1cae7c7c-4a9c-5c9e-2e2f-8a91b2b85a93@ti.com> <68139659-9c2c-89ee-55b6-17df6a157005@ti.com> <77dac8dd-5735-0da1-061d-2cce3719ebac@samsung.com> Message-ID: <08c1e8e7-40b4-fb6a-9108-6a596eb66912@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >>>>>>> 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