| Message ID | 20230215113249.47727-1-william.qiu@starfivetech.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | StarFive's SDIO/eMMC driver support | expand |
On Wed, 15 Feb 2023 at 12:32, William Qiu <william.qiu@starfivetech.com> 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@starfivetech.com/ > > 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
On 2023/2/15 20:37, Ulf Hansson wrote: > On Wed, 15 Feb 2023 at 12:32, William Qiu <william.qiu@starfivetech.com> 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@starfivetech.com/ >> >> 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? Best regards William
On Mon, 27 Feb 2023 at 08:47, William Qiu <william.qiu@starfivetech.com> wrote: > > > > On 2023/2/15 20:37, Ulf Hansson wrote: > > On Wed, 15 Feb 2023 at 12:32, William Qiu <william.qiu@starfivetech.com> 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@starfivetech.com/ > >> > >> 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
On 2023/2/27 22:53, Ulf Hansson wrote: > On Mon, 27 Feb 2023 at 08:47, William Qiu <william.qiu@starfivetech.com> wrote: >> >> >> >> On 2023/2/15 20:37, Ulf Hansson wrote: >> > On Wed, 15 Feb 2023 at 12:32, William Qiu <william.qiu@starfivetech.com> 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@starfivetech.com/ >> >> >> >> 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