On 2023/2/27 22:53, Ulf Hansson wrote: > On Mon, 27 Feb 2023 at 08:47, William Qiu <william.qiu@xxxxxxxxxxxxxxxx> wrote: >> >> >> >> On 2023/2/15 20:37, Ulf Hansson wrote: >> > On Wed, 15 Feb 2023 at 12:32, William Qiu <william.qiu@xxxxxxxxxxxxxxxx> wrote: >> >> >> >> Hi, >> >> >> >> This patchset adds initial rudimentary support for the StarFive >> >> designware mobile storage host controller driver. And this driver will >> >> be used in StarFive's VisionFive 2 board. The main purpose of adding >> >> this driver is to accommodate the ultra-high speed mode of eMMC. >> >> >> >> The last patch should be applied after the patchset [1]: >> >> [1] https://lore.kernel.org/all/20221220011247.35560-1-hal.feng@xxxxxxxxxxxxxxxx/ >> >> >> >> Changes v3->v4: >> >> - Added documentation to describe StarFive System Controller Registers. >> >> - Added aon_syscon and stg_syscon node. >> >> - Fixed some checkpatch errors/warnings. >> >> >> >> Changes v2->v3: >> >> - Wraped commit message according to Linux coding style. >> >> - Rephrased the description of the patches. >> >> - Changed the description of syscon regsiter. >> >> - Dropped redundant properties. >> >> >> >> Changes v1->v2: >> >> - Renamed the dt-binding 'starfive,jh7110-sdio.yaml' to 'starfive,jh7110-mmc.yaml'. >> >> - Changed the type of 'starfive,syscon' and modify its description. >> >> - Deleted unused head files like '#include <linux/gpio.h>'. >> >> - Added comment for the 'rise_point' and 'fall_point'. >> >> - Changed the API 'num_caps' to 'common_caps'. >> >> - Changed the node name 'sys_syscon' to 'syscon'. >> >> - Changed the node name 'sdio' to 'mmc'. >> >> >> >> The patch series is based on v6.1. >> >> >> >> William Qiu (4): >> >> dt-bindings: mmc: Add StarFive MMC module >> >> mmc: starfive: Add sdio/emmc driver support >> >> riscv: dts: starfive: Add mmc node >> >> dt-bindings: syscon: Add StarFive syscon doc >> >> >> >> .../bindings/mmc/starfive,jh7110-mmc.yaml | 77 ++++++++ >> >> .../bindings/soc/starfive/jh7110-syscon.yaml | 51 +++++ >> >> MAINTAINERS | 11 ++ >> >> .../jh7110-starfive-visionfive-2.dtsi | 23 +++ >> >> arch/riscv/boot/dts/starfive/jh7110.dtsi | 47 +++++ >> >> drivers/mmc/host/Kconfig | 10 + >> >> drivers/mmc/host/Makefile | 1 + >> >> drivers/mmc/host/dw_mmc-starfive.c | 186 ++++++++++++++++++ >> >> 8 files changed, 406 insertions(+) >> >> create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml >> >> create mode 100644 Documentation/devicetree/bindings/soc/starfive/jh7110-syscon.yaml >> >> create mode 100644 drivers/mmc/host/dw_mmc-starfive.c >> >> >> > >> > I have dropped the v3 patches and applied patch1 and patch2 from the >> > v4 series instead, for my next branch, thanks! >> > >> > Kind regards >> > Uffe >> >> Hi Uffe, >> >> Sorry to bother you.But I found a bug that in drivers/mmc/host/dw_mmc-starfive.c: >> >> 47 static int dw_mci_starfive_execute_tuning(struct dw_mci_slot *slot, >> 48 u32 opcode) >> 49 { >> 50 static const int grade = MAX_DELAY_CHAIN; >> 51 struct dw_mci *host = slot->host; >> 52 struct starfive_priv *priv = host->priv; >> 53 int rise_point = -1, fall_point = -1; >> 54 int err, prev_err; >> 55 int i; >> 56 bool found = 0; >> 57 u32 regval; >> 58 >> 59 /* >> 60 * Use grade as the max delay chain, and use the rise_point and >> 61 * fall_point to ensure the best sampling point of a data input >> 62 * signals. >> 63 */ >> 64 for (i = 0; i < grade; i++) { >> 65 regval = i << priv->syscon_shift; >> 66 err = regmap_update_bits(priv->reg_syscon, priv->syscon_offset, >> 67 priv->syscon_mask, regval); >> 68 if (err) >> 69 return err; >> 70 mci_writel(host, RINTSTS, ALL_INT_CLR); >> 71 >> 72 err = mmc_send_tuning(slot->mmc, opcode, NULL); >> 73 if (!err) >> 74 found = 1; >> 75 >> 76 if (i > 0) { >> --> 77 if (err && !prev_err) >> >> prev_err was never initialized to zero. >> >> So I'm here to ask for your suggestion, should I send a new version >> to fix it or send you a patch with a fixes tag? > > Please send a new incremental patch on top. I will queue it up as a > fix for v6.3-rc[n]. > > Kind regards > Uffe Fine, I'll do it in my next version. Thanks for your apply. Best regards William