[PATCH v3] mmc: Poll CD in case cyclic framework is enabled
Simon Glass
sjg at chromium.org
Tue Dec 12 15:05:59 CET 2023
Hi Marek,
On Mon, 11 Dec 2023 at 12:24, Marek Vasut <marek.vasut at mailbox.org> wrote:
>
> On 12/11/23 18:52, Simon Glass wrote:
> > Hi Marek,
> >
> > On Sun, 10 Dec 2023 at 08:03, Marek Vasut
> > <marek.vasut+renesas at mailbox.org> wrote:
> >>
> >> In case the cyclic framework is enabled, poll the card detect of already
> >> initialized cards and deinitialize them in case they are removed. Since
> >> the card initialization is a longer process and card initialization is
> >> done on first access to an uninitialized card anyway, avoid initializing
> >> newly detected uninitialized cards in the cyclic callback.
> >>
> >> Signed-off-by: Marek Vasut <marek.vasut+renesas at mailbox.org>
> >> ---
> >> Cc: Jaehoon Chung <jh80.chung at samsung.com>
> >> Cc: Peng Fan <peng.fan at nxp.com>
> >> Cc: Simon Glass <sjg at chromium.org>
> >> ---
> >> V2: Move the cyclic registration/unregistration into mmc init/deinit
> >> V3: Replace if (CONFIG_IS_ENABLED(CYCLIC)...) with #if as the former
> >> does not work with structure members
> >> ---
> >> drivers/mmc/mmc.c | 36 ++++++++++++++++++++++++++++++++++++
> >> include/mmc.h | 5 +++++
> >> 2 files changed, 41 insertions(+)
> >>
> >> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> >> index eb5010c1465..a5686dbc12e 100644
> >> --- a/drivers/mmc/mmc.c
> >> +++ b/drivers/mmc/mmc.c
> >> @@ -2985,6 +2985,22 @@ static int mmc_complete_init(struct mmc *mmc)
> >> return err;
> >> }
> >>
> >> +#if CONFIG_IS_ENABLED(CYCLIC)
> >> +static void mmc_cyclic_cd_poll(void *ctx)
> >> +{
> >> + struct mmc *m = ctx;
> >> +
> >> + if (!m->has_init)
> >> + return;
> >> +
> >> + if (mmc_getcd(m))
> >> + return;
> >> +
> >> + mmc_deinit(m);
> >> + m->has_init = 0;
> >> +}
> >> +#endif
> >> +
> >> int mmc_init(struct mmc *mmc)
> >> {
> >> int err = 0;
> >> @@ -3007,6 +3023,19 @@ int mmc_init(struct mmc *mmc)
> >> if (err)
> >> pr_info("%s: %d, time %lu\n", __func__, err, get_timer(start));
> >>
> >> +#if CONFIG_IS_ENABLED(CYCLIC)
> >
> > We really shouldn't be adding new #ifdefs to the code.
> >
> > If you really want to make put ->cyclic behind an #ifdef then how
> > about creating an accessor as is done in global_data.h ?
>
> That's really just working around the underlying issue, which is that
> this does not compile:
>
> struct foo {
> #if CONFIG_IS_ENABLED(STUFF)
> type member;
> #endif
> ...
> };
>
> type fn() {
> ...
> if (CONFIG_IS_ENABLED(STUFF))
> access struct->member;
> ...
> }
>
> Accessor won't make it any better, would it ? It would only attempt to
> hide the error and make the code more fragile, i.e. this:
>
> accessor(struct)
> {
> type fn() {
> if (CONFIG_IS_ENABLED(STUFF))
> return access struct->member;
> else
> return 0;
> }
>
> }
>
> type fn() {
> ...
> accessor(struct);
> ...
> }
>
> I suspect it also won't compile for one thing, and for the other, it
> really just hides the issue.
No, I mean like this:
#ifdef CONFIG_ACPI
#define gd_acpi_ctx() gd->acpi_ctx
#define gd_acpi_start() gd->acpi_start
#define gd_set_acpi_start(addr) gd->acpi_start = addr
#else
#define gd_acpi_ctx() NULL
#define gd_acpi_start() 0UL
#define gd_set_acpi_start(addr)
#endif
The header file has #ifdefs but not the C files.
Regards,
Simon
More information about the U-Boot
mailing list