On Thu, 16 Sept 2021 at 11:47, Wenbin Mei <wenbin.mei@xxxxxxxxxxxx> wrote: > > On Tue, 2021-09-14 at 10:46 +0200, Ulf Hansson wrote: > > On Wed, 8 Sept 2021 at 03:32, Wenbin Mei <wenbin.mei@xxxxxxxxxxxx> > > wrote: > > > > > > Due to the influence of the corner IC and vcore voltage, for the > > > stability > > > of HS400 mode, we Add HS400 mode online tuning support for mediatek > > > mmc > > > host. > > > > My apologies, but I am not familiar with what 'HS400 online tuning' > > is? Can you please elaborate on this? > > > > Is it specific for a Mediatek eMMC controller - or is a common eMMC > > feature that is described in the eMMC spec? > > > According to JEDEC Spec, there is no need to do tuning under HS400 mode > since the Rx signal is aligned with the DS signal. However, MediaTek's > IC need set its "DS delay" internally to ensure it can latch Rx signal > correctly. > In previous version, We provide an "hs400-ds-delay" in device tree to > cover different chipset/PCB design, and it works fine in most cases. > But, with the development of process technology and the big VCore > voltage scale range(may have 0.7V/0.6V/0.55V), it is difficult to find > a suitable "hs400-ds-delay" to cover all of IC corner > cases(SSSS/TTTT/FFFF). > So that We must have the ability to do hs400 online tuning. > It is specific for the Mediatek eMMC controller which support HS400 > mode. I see, thanks for clarifying. Please put some of this information in the commit message for the next version, it certainly helps to understand. [...] > > > +static int msdc_send_cxd_data(struct mmc_card *card, struct > > > mmc_host *host) > > > +{ > > > + struct mmc_request mrq = {}; > > > + struct mmc_command cmd = {}; > > > + struct mmc_data data = {}; > > > + unsigned int len = 512; > > > + struct scatterlist sg; > > > + u8 *ext_csd; > > > + > > > + ext_csd = kzalloc(len, GFP_KERNEL); > > > + if (!ext_csd) > > > + return -ENOMEM; > > > + > > > + mrq.cmd = &cmd; > > > + mrq.data = &data; > > > + > > > + cmd.opcode = MMC_SEND_EXT_CSD; > > > + cmd.arg = 0; > > > + cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC; > > > + > > > + data.blksz = len; > > > + data.blocks = 1; > > > + data.flags = MMC_DATA_READ; > > > + data.sg = &sg; > > > + data.sg_len = 1; > > > + > > > + sg_init_one(&sg, ext_csd, len); > > > + mmc_set_data_timeout(&data, card); > > > + mmc_wait_for_req(host, &mrq); > > > + > > > + kfree(ext_csd); > > > + > > > + if (cmd.error) > > > + return cmd.error; > > > + if (data.error) > > > + return data.error; > > > + > > > + return 0; > > > > Why do we need to send a MMC_SEND_EXT_CSD command, exactly? > > > > Why can't mmc_send_tuning() work here too? What does the eMMC spec > > state about this? > > > The CMD21 is illegal under hs400 mode so that cannot use the > mmc_send_tuning(). The CMD8 is suitable because it will receive 1 block > of non-zero data. I see. In that case it seems better to use mmc_get_ext_csd(), from the core, rather than open coding the above. To do that, you also need to move the declaration of mmc_get_ext_csd() to include/linux/mmc/host.h. [...] Kind regards Uffe