On 12/11/23 18:52, Simon Glass wrote:
Hi Marek,
On Sun, 10 Dec 2023 at 08:03, Marek Vasut
<marek.vasut+rene...@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+rene...@mailbox.org>
---
Cc: Jaehoon Chung <jh80.ch...@samsung.com>
Cc: Peng Fan <peng....@nxp.com>
Cc: Simon Glass <s...@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.
