| Message ID | 20230927072544.764285-1-Rong.Chen@amlogic.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mmc: meson-gx: Remove setting of CMD_CFG_ERROR | expand |
s On Wed 27 Sep 2023 at 15:25, "Rong.Chen" <Rong.Chen@amlogic.com> wrote: > From: Rong Chen <rong.chen@amlogic.com> > > Setting bit CMD_CFG_ERROR has non effects on previous controllers. Which previous controllers ? v2, v3 ? Can you list the SoCs please ? > However, it will cause an IRQ and stop working on latest controllers, Isn't it desirable to stop on error ? seems like a sane thing to do. > This bit is used for selecting SG LINK of data address. > Don't set the bit CMD_CFG_ERROR anymore to resolve this issuse. I'm not sure I understand the actual issue being resolved. Could you provide more details please ? > > Therefore, remove this setting. > > Signed-off-by: Rong Chen <rong.chen@amlogic.com> > --- > drivers/mmc/host/meson-gx-mmc.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c > index 9837dab096e6..c7c067b9415a 100644 > --- a/drivers/mmc/host/meson-gx-mmc.c > +++ b/drivers/mmc/host/meson-gx-mmc.c > @@ -801,7 +801,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd) > > cmd_cfg |= FIELD_PREP(CMD_CFG_CMD_INDEX_MASK, cmd->opcode); > cmd_cfg |= CMD_CFG_OWNER; /* owned by CPU */ > - cmd_cfg |= CMD_CFG_ERROR; /* stop in case of error */ > > meson_mmc_set_response_bits(cmd, &cmd_cfg);
On Wed 27 Sep 2023 at 16:51, "Rong.Chen@amlogic.com" <Rong.Chen@amlogic.com> wrote: > Hi Jerome, > > Socs containing previous controllers are gxl, gxm, gxbb，g12b，g12a，sm1，axg, a1，s4 and t7. That is mostly all the SoC we know of in mainline ... This why the way you explained is confusing. And below, you say 'on latest controllers', okay but which ones ? > The bit CMD_CFG_ERROR is used for selecting SG LINK of data address on latest controllers，but > data address don't use SG LINK at present，so it will cause an IRQ and stop working on latest controllers. > Therefore, remove this setting. Not totally sure what SG LINK but okay. While the change itself is fine you really need reformulate the description. Based on your explanation, something like: --- For the t7 and older SoC families, the CMD_CFG_ERROR has no effect. Starting from SoC family XYZ, it will select SG LINK on error but this is causing a problem with the data address ... blablabla --- Please expand a bit on this, try to make it clearer. Also, please insert the proper fixes tag Last, make sure tp reply with plain text emails, otherwise lists will drop your reply and please do not top post. Thx > > Thanks. > ------------------------------------------------------------------------------------------------------------------------- > Rong.Chen@amlogic.com > > > From: Jerome Brunet > Date: 2023-09-27 15:43 > To: Rong.Chen; Ulf Hansson; Neil Armstrong; Kevin Hilman; Martin Blumenstingl > CC: linux-mmc; linux-arm-kernel; linux-amlogic; linux-kernel > Subject: Re: [PATCH] mmc: meson-gx: Remove setting of CMD_CFG_ERROR > [You don't often get email from jbrunet@baylibre.com. Learn why this is important at > https://aka.ms/LearnAboutSenderIdentification ] > > [ EXTERNAL EMAIL ] > > s > On Wed 27 Sep 2023 at 15:25, "Rong.Chen" <Rong.Chen@amlogic.com> wrote: > > > From: Rong Chen <rong.chen@amlogic.com> > > > > Setting bit CMD_CFG_ERROR has non effects on previous controllers. > > Which previous controllers ? v2, v3 ? Can you list the SoCs please ? > > > However, it will cause an IRQ and stop working on latest controllers, > > Isn't it desirable to stop on error ? seems like a sane thing to do. > > > This bit is used for selecting SG LINK of data address. > > Don't set the bit CMD_CFG_ERROR anymore to resolve this issuse. > > I'm not sure I understand the actual issue being resolved. > Could you provide more details please ? > > > > > Therefore, remove this setting. > > > > Signed-off-by: Rong Chen <rong.chen@amlogic.com> > > --- > > drivers/mmc/host/meson-gx-mmc.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c > > index 9837dab096e6..c7c067b9415a 100644 > > --- a/drivers/mmc/host/meson-gx-mmc.c > > +++ b/drivers/mmc/host/meson-gx-mmc.c > > @@ -801,7 +801,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd) > > > > cmd_cfg |= FIELD_PREP(CMD_CFG_CMD_INDEX_MASK, cmd->opcode); > > cmd_cfg |= CMD_CFG_OWNER; /* owned by CPU */ > > - cmd_cfg |= CMD_CFG_ERROR; /* stop in case of error */ > > > > meson_mmc_set_response_bits(cmd, &cmd_cfg);
Hi Jerome, For the t7 and older SoC families, the CMD_CFG_ERROR has no effect. Starting from SoC family C3, setting this bit without SG LINK data address will cause the controller to generate an IRQ and stop working. 在 2023/9/27 17:30, Jerome Brunet 写道: > [ EXTERNAL EMAIL ] > > On Wed 27 Sep 2023 at 16:51, "Rong.Chen@amlogic.com" <Rong.Chen@amlogic.com> wrote: > >> Hi Jerome, >> >> Socs containing previous controllers are gxl, gxm, gxbb，g12b，g12a，sm1，axg, a1，s4 and t7. > That is mostly all the SoC we know of in mainline ... This why the way > you explained is confusing. > > And below, you say 'on latest controllers', okay but which ones ? > >> The bit CMD_CFG_ERROR is used for selecting SG LINK of data address on latest controllers，but >> data address don't use SG LINK at present，so it will cause an IRQ and stop working on latest controllers. >> Therefore, remove this setting. > Not totally sure what SG LINK but okay. > > While the change itself is fine you really need reformulate the description. > Based on your explanation, something like: > --- > For the t7 and older SoC families, the CMD_CFG_ERROR has no effect. > Starting from SoC family XYZ, it will select SG LINK on error but this > is causing a problem with the data address ... blablabla > --- > > Please expand a bit on this, try to make it clearer. > Also, please insert the proper fixes tag > > Last, make sure tp reply with plain text emails, otherwise lists will > drop your reply and please do not top post. > > Thx > >> Thanks. >> ------------------------------------------------------------------------------------------------------------------------- >> Rong.Chen@amlogic.com >> >> >> From: Jerome Brunet >> Date: 2023-09-27 15:43 >> To: Rong.Chen; Ulf Hansson; Neil Armstrong; Kevin Hilman; Martin Blumenstingl >> CC: linux-mmc; linux-arm-kernel; linux-amlogic; linux-kernel >> Subject: Re: [PATCH] mmc: meson-gx: Remove setting of CMD_CFG_ERROR >> [You don't often get email from jbrunet@baylibre.com. Learn why this is important at >> https://aka.ms/LearnAboutSenderIdentification ] >> >> [ EXTERNAL EMAIL ] >> >> s >> On Wed 27 Sep 2023 at 15:25, "Rong.Chen" <Rong.Chen@amlogic.com> wrote: >> >> > From: Rong Chen <rong.chen@amlogic.com> >> > >> > Setting bit CMD_CFG_ERROR has non effects on previous controllers. >> >> Which previous controllers ? v2, v3 ? Can you list the SoCs please ? >> >> > However, it will cause an IRQ and stop working on latest controllers, >> >> Isn't it desirable to stop on error ? seems like a sane thing to do. >> >> > This bit is used for selecting SG LINK of data address. >> > Don't set the bit CMD_CFG_ERROR anymore to resolve this issuse. >> >> I'm not sure I understand the actual issue being resolved. >> Could you provide more details please ? >> >> > >> > Therefore, remove this setting. >> > >> > Signed-off-by: Rong Chen <rong.chen@amlogic.com> >> > --- >> > drivers/mmc/host/meson-gx-mmc.c | 1 - >> > 1 file changed, 1 deletion(-) >> > >> > diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c >> > index 9837dab096e6..c7c067b9415a 100644 >> > --- a/drivers/mmc/host/meson-gx-mmc.c >> > +++ b/drivers/mmc/host/meson-gx-mmc.c >> > @@ -801,7 +801,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd) >> > >> > cmd_cfg |= FIELD_PREP(CMD_CFG_CMD_INDEX_MASK, cmd->opcode); >> > cmd_cfg |= CMD_CFG_OWNER; /* owned by CPU */ >> > - cmd_cfg |= CMD_CFG_ERROR; /* stop in case of error */ >> > >> > meson_mmc_set_response_bits(cmd, &cmd_cfg);
Hi Jerome, I tested on meson-g12a-u200 and meson-sm1-sei610 board successfully. *meson-g12a-u200:* [ 1.308436] mmc1: new HS200 MMC card at address 0001 [ 1.309223] mmcblk1: mmc1:0001 AJTD4R 14.6 GiB [ 1.314724] mmcblk1: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10 p11 p12 p13 p14 p15 p16 [ 1.321724] mmcblk1boot0: mmc1:0001 AJTD4R 4.00 MiB [ 1.325727] mmcblk1boot1: mmc1:0001 AJTD4R 4.00 MiB [ 1.330427] mmcblk1rpmb: mmc1:0001 AJTD4R 4.00 MiB, chardev (511:0) [ 11.368251] platform cpufreq-dt: deferred probe pending [ 29.695649] mmc0: new high speed SDHC card at address 0007 [ 29.696402] mmcblk0: mmc0:0007 SD16G 14.5 GiB [ 29.708053] mmcblk0: p1 [ 48.132967] mmc0: card 0007 removed *meson-sm1-sei610:* [ 1.815416] mmc1: new HS200 MMC card at address 0001 iled:[ 1.819542] mmcblk1: mmc1:0001 016G30 14.7 GiB No such file or d[ 1.826207] mmcblk1boot0: mmc1:0001 016G30 4.00 MiB [ 1.831692] mmcblk1boot1: mmc1:0001 016G30 4.00 MiB [ 1.836250] mmcblk1rpmb: mmc1:0001 016G30 4.00 MiB, chardev (511:0) [ 5.857437] mmc0: new high speed SDHC card at address 0007 [ 5.858046] mmcblk0: mmc0:0007 SD16G 14.5 GiB [ 5.869727] mmcblk0: p1 [ 9.958449] mmc0: card 0007 removed 在 2023/9/27 17:30, Jerome Brunet 写道: > [ EXTERNAL EMAIL ] > > On Wed 27 Sep 2023 at 16:51, "Rong.Chen@amlogic.com" <Rong.Chen@amlogic.com> wrote: > >> Hi Jerome, >> >> Socs containing previous controllers are gxl, gxm, gxbb，g12b，g12a，sm1，axg, a1，s4 and t7. > That is mostly all the SoC we know of in mainline ... This why the way > you explained is confusing. > > And below, you say 'on latest controllers', okay but which ones ? > >> The bit CMD_CFG_ERROR is used for selecting SG LINK of data address on latest controllers，but >> data address don't use SG LINK at present，so it will cause an IRQ and stop working on latest controllers. >> Therefore, remove this setting. > Not totally sure what SG LINK but okay. > > While the change itself is fine you really need reformulate the description. > Based on your explanation, something like: > --- > For the t7 and older SoC families, the CMD_CFG_ERROR has no effect. > Starting from SoC family XYZ, it will select SG LINK on error but this > is causing a problem with the data address ... blablabla > --- > > Please expand a bit on this, try to make it clearer. > Also, please insert the proper fixes tag > > Last, make sure tp reply with plain text emails, otherwise lists will > drop your reply and please do not top post. > > Thx > >> Thanks. >> ------------------------------------------------------------------------------------------------------------------------- >> Rong.Chen@amlogic.com >> >> >> From: Jerome Brunet >> Date: 2023-09-27 15:43 >> To: Rong.Chen; Ulf Hansson; Neil Armstrong; Kevin Hilman; Martin Blumenstingl >> CC: linux-mmc; linux-arm-kernel; linux-amlogic; linux-kernel >> Subject: Re: [PATCH] mmc: meson-gx: Remove setting of CMD_CFG_ERROR >> [You don't often get email from jbrunet@baylibre.com. Learn why this is important at >> https://aka.ms/LearnAboutSenderIdentification ] >> >> [ EXTERNAL EMAIL ] >> >> s >> On Wed 27 Sep 2023 at 15:25, "Rong.Chen" <Rong.Chen@amlogic.com> wrote: >> >> > From: Rong Chen <rong.chen@amlogic.com> >> > >> > Setting bit CMD_CFG_ERROR has non effects on previous controllers. >> >> Which previous controllers ? v2, v3 ? Can you list the SoCs please ? >> >> > However, it will cause an IRQ and stop working on latest controllers, >> >> Isn't it desirable to stop on error ? seems like a sane thing to do. >> >> > This bit is used for selecting SG LINK of data address. >> > Don't set the bit CMD_CFG_ERROR anymore to resolve this issuse. >> >> I'm not sure I understand the actual issue being resolved. >> Could you provide more details please ? >> >> > >> > Therefore, remove this setting. >> > >> > Signed-off-by: Rong Chen <rong.chen@amlogic.com> >> > --- >> > drivers/mmc/host/meson-gx-mmc.c | 1 - >> > 1 file changed, 1 deletion(-) >> > >> > diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c >> > index 9837dab096e6..c7c067b9415a 100644 >> > --- a/drivers/mmc/host/meson-gx-mmc.c >> > +++ b/drivers/mmc/host/meson-gx-mmc.c >> > @@ -801,7 +801,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd) >> > >> > cmd_cfg |= FIELD_PREP(CMD_CFG_CMD_INDEX_MASK, cmd->opcode); >> > cmd_cfg |= CMD_CFG_OWNER; /* owned by CPU */ >> > - cmd_cfg |= CMD_CFG_ERROR; /* stop in case of error */ >> > >> > meson_mmc_set_response_bits(cmd, &cmd_cfg);
diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c index 9837dab096e6..c7c067b9415a 100644 --- a/drivers/mmc/host/meson-gx-mmc.c +++ b/drivers/mmc/host/meson-gx-mmc.c @@ -801,7 +801,6 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd) cmd_cfg |= FIELD_PREP(CMD_CFG_CMD_INDEX_MASK, cmd->opcode); cmd_cfg |= CMD_CFG_OWNER; /* owned by CPU */ - cmd_cfg |= CMD_CFG_ERROR; /* stop in case of error */ meson_mmc_set_response_bits(cmd, &cmd_cfg);