Hi Uffe, On Thu, Mar 24, 2022 at 12:23 AM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote: > > On Tue, 22 Feb 2022 at 04:40, Jason Lai <jasonlai.genesyslogic@xxxxxxxxx> wrote: > > > > From: Jason Lai <jason.lai@xxxxxxxxxxxxxxxxxxx> > > > > Embed UHS-II access functionality into the MMC request processing flow. > > > > Signed-off-by: Jason Lai <jason.lai@xxxxxxxxxxxxxxxxxxx> > > --- > > drivers/mmc/core/core.c | 26 ++++++++++++++++++++++++-- > > 1 file changed, 24 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > > index fc3d8d61a97c..d2dcaa64bf27 100644 > > --- a/drivers/mmc/core/core.c > > +++ b/drivers/mmc/core/core.c > > @@ -31,6 +31,7 @@ > > #include <linux/mmc/mmc.h> > > #include <linux/mmc/sd.h> > > #include <linux/mmc/slot-gpio.h> > > +#include <linux/mmc/sd_uhs2.h> > > > > #define CREATE_TRACE_POINTS > > #include <trace/events/mmc.h> > > @@ -42,6 +43,7 @@ > > #include "host.h" > > #include "sdio_bus.h" > > #include "pwrseq.h" > > +#include "sd_uhs2.h" > > > > #include "mmc_ops.h" > > #include "sd_ops.h" > > @@ -335,6 +337,8 @@ static int mmc_mrq_prep(struct mmc_host *host, struct mmc_request *mrq) > > > > int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq) > > { > > + struct uhs2_command uhs2_cmd; > > + u32 payload[4]; /* for maximum size */ > > int err; > > > > init_completion(&mrq->cmd_completion); > > @@ -352,6 +356,13 @@ int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq) > > if (err) > > return err; > > > > + if (host->uhs2_caps.flags & MMC_UHS2_SUPPORT && > > + host->uhs2_caps.flags & MMC_UHS2_INITIALIZED) { > > + uhs2_cmd.payload = payload; > > + mrq->cmd->uhs2_cmd = &uhs2_cmd; > > + sd_uhs2_prepare_cmd(host, mrq); > > + } > > + > > led_trigger_event(host->led, LED_FULL); > > __mmc_start_request(host, mrq); > > > > @@ -431,6 +442,8 @@ EXPORT_SYMBOL(mmc_wait_for_req_done); > > */ > > int mmc_cqe_start_req(struct mmc_host *host, struct mmc_request *mrq) > > { > > + struct uhs2_command uhs2_cmd; > > + u32 payload[4]; /* for maximum size */ > > int err; > > > > /* > > @@ -451,6 +464,13 @@ int mmc_cqe_start_req(struct mmc_host *host, struct mmc_request *mrq) > > if (err) > > goto out_err; > > > > + if (host->uhs2_caps.flags & MMC_UHS2_SUPPORT && > > + host->uhs2_caps.flags & MMC_UHS2_INITIALIZED) { > > + uhs2_cmd.payload = payload; > > + mrq->cmd->uhs2_cmd = &uhs2_cmd; > > + sd_uhs2_prepare_cmd(host, mrq); > > + } > > + > > err = host->cqe_ops->cqe_request(host, mrq); > > if (err) > > goto out_err; > > @@ -899,8 +919,10 @@ static inline void mmc_set_ios(struct mmc_host *host) > > */ > > void mmc_set_chip_select(struct mmc_host *host, int mode) > > { > > - host->ios.chip_select = mode; > > - mmc_set_ios(host); > > + if (!(host->uhs2_caps.flags & MMC_UHS2_INITIALIZED)) { > > + host->ios.chip_select = mode; > > + mmc_set_ios(host); > > + } > > As I stated for patch6, rather than having these hacks spread out into > the legacy code, we must not allow the ->set_ios() ops to be invoked > when UHS-II is being used/initialized. > After checking sd_uhs2.c, mmc_set_chip_select() is called in mmc_go_idle() and mmc_go_idle() is called in sd_uhs2_legacy_init(). Hence, I issue CMD0 directly in sd_uhs2_legacy_init(). > > } > > > > Moreover, I think $subject patch should be squashed into patch6. There > is really no reason to have them split up like this. Or is there? > OK. Kind regards, Jason > I have now walked through the series - and it seems like there are a > few more pieces that we need to sort out, but it's certainly getting > better and better. > > Kind regards > Uffe