2023-02-15 11:33:14

by William Qiu

[permalink] [raw]
Subject: [PATCH v4 0/4] StarFive's SDIO/eMMC driver support

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/[email protected]/

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

--
2.34.1



2023-02-15 12:38:26

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] StarFive's SDIO/eMMC driver support

On Wed, 15 Feb 2023 at 12:32, William Qiu <[email protected]> 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/[email protected]/
>
> 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

2023-02-27 07:47:55

by William Qiu

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] StarFive's SDIO/eMMC driver support



On 2023/2/15 20:37, Ulf Hansson wrote:
> On Wed, 15 Feb 2023 at 12:32, William Qiu <[email protected]> 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/[email protected]/
>>
>> 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

2023-02-27 14:53:53

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] StarFive's SDIO/eMMC driver support

On Mon, 27 Feb 2023 at 08:47, William Qiu <[email protected]> wrote:
>
>
>
> On 2023/2/15 20:37, Ulf Hansson wrote:
> > On Wed, 15 Feb 2023 at 12:32, William Qiu <[email protected]> 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/[email protected]/
> >>
> >> 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

2023-02-28 05:59:41

by William Qiu

[permalink] [raw]
Subject: Re: [PATCH v4 0/4] StarFive's SDIO/eMMC driver support



On 2023/2/27 22:53, Ulf Hansson wrote:
> On Mon, 27 Feb 2023 at 08:47, William Qiu <[email protected]> wrote:
>>
>>
>>
>> On 2023/2/15 20:37, Ulf Hansson wrote:
>> > On Wed, 15 Feb 2023 at 12:32, William Qiu <[email protected]> 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/[email protected]/
>> >>
>> >> 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