From patchwork Mon Feb 17 23:24:20 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jaehoon Chung X-Patchwork-Id: 236444 List-Id: U-Boot discussion From: jh80.chung at samsung.com (Jaehoon Chung) Date: Tue, 18 Feb 2020 08:24:20 +0900 Subject: [PATCH v2 04/10] mmc: sdhci: Expose sdhci_init() as non-static In-Reply-To: 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> <08c1e8e7-40b4-fb6a-9108-6a596eb66912@ti.com> Message-ID: <1be44121-2a6e-e97b-c47b-332735f5c952@samsung.com> Hi Faiz, On 2/18/20 7:35 AM, Jaehoon Chung wrote: > 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 >>>>>>>>> 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. How about below codes? Then you can just add am654_sdhci_deferred_probe { ... return sdhci_probe() .. } > > Best Regards, > Jaehoon Chung > >> >> Thanks, >> Faiz >> >> > > > diff --git a/drivers/mmc/mmc-uclass.c b/drivers/mmc/mmc-uclass.c index 0b90a97650..c75892a72c 100644 --- a/drivers/mmc/mmc-uclass.c +++ b/drivers/mmc/mmc-uclass.c @@ -138,6 +138,21 @@ int mmc_host_power_cycle(struct mmc *mmc) return dm_mmc_host_power_cycle(mmc->dev); } +int dm_mmc_deferred_probe(struct udevice *dev) +{ + struct dm_mmc_ops *ops = mmc_get_ops(dev); + + if (ops->deferred_probe) + return ops->deferred_probe(dev); + + return 0; +} + +int mmc_deferred_probe(struct mmc *mmc) +{ + return dm_mmc_deferred_probe(mmc->dev); +} + int mmc_of_parse(struct udevice *dev, struct mmc_config *cfg) { int val; diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index d43983d4a6..9eae538af4 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -2790,6 +2790,7 @@ int mmc_get_op_cond(struct mmc *mmc) #if CONFIG_IS_ENABLED(DM_MMC) /* The device has already been probed ready for use */ + mmc_deferred_probe(mmc); #else /* made sure it's not NULL earlier */ err = mmc->cfg->ops->init(mmc); diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 01fa5a9d4d..9ff37b888b 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -661,6 +661,20 @@ int sdhci_probe(struct udevice *dev) return sdhci_init(mmc); } +static int sdhci_deferred_probe(struct udevice *dev) +{ + int err; + struct mmc *mmc = mmc_get_mmc_dev(dev); + struct sdhci_host *host = mmc->priv; + + if (host->ops && host->ops->deferred_probe) { + err = host->ops->deferred_probe(host); + if (err) + return err; + } + return 0; +} + static int sdhci_get_cd(struct udevice *dev) { struct mmc *mmc = mmc_get_mmc_dev(dev); @@ -695,6 +709,7 @@ const struct dm_mmc_ops sdhci_ops = { .send_cmd = sdhci_send_command, .set_ios = sdhci_set_ios, .get_cd = sdhci_get_cd, + .deferred_probe = sdhci_deferred_probe, #ifdef MMC_SUPPORTS_TUNING .execute_tuning = sdhci_execute_tuning, #endif diff --git a/include/mmc.h b/include/mmc.h index b5cb514f57..b362b7f4c7 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -477,6 +477,8 @@ struct dm_mmc_ops { * @return 0 if not present, 1 if present, -ve on error */ int (*host_power_cycle)(struct udevice *dev); + + int (*deferred_probe)(struct udevice *dev); }; #define mmc_get_ops(dev) ((struct dm_mmc_ops *)(dev)->driver->ops) @@ -489,6 +491,7 @@ int dm_mmc_get_wp(struct udevice *dev); int dm_mmc_execute_tuning(struct udevice *dev, uint opcode); int dm_mmc_wait_dat0(struct udevice *dev, int state, int timeout_us); int dm_mmc_host_power_cycle(struct udevice *dev); +int dm_mmc_deferred_probe(struct udevice *dev); /* Transition functions for compatibility */ int mmc_set_ios(struct mmc *mmc); @@ -498,6 +501,7 @@ int mmc_execute_tuning(struct mmc *mmc, uint opcode); int mmc_wait_dat0(struct mmc *mmc, int state, int timeout_us); int mmc_set_enhanced_strobe(struct mmc *mmc); int mmc_host_power_cycle(struct mmc *mmc); +int mmc_deferred_probe(struct mmc *mmc); #else struct mmc_ops { diff --git a/include/sdhci.h b/include/sdhci.h index 01addb7a60..1276f43935 100644 --- a/include/sdhci.h +++ b/include/sdhci.h @@ -267,6 +267,7 @@ struct sdhci_ops { void (*set_clock)(struct sdhci_host *host, u32 div); int (*platform_execute_tuning)(struct mmc *host, u8 opcode); void (*set_delay)(struct sdhci_host *host); + int (*deferred_probe)(struct sdhci_host *host); }; #if CONFIG_IS_ENABLED(MMC_SDHCI_ADMA)