Re: [PATCH v3 15/17] tests/qtest: Disable npcm7xx_sdhci tests using hardcoded RCA

[Philippe Mathieu-Daudé]

On 27/6/24 18:47, Thomas Huth wrote:
> On 27/06/2024 18.22, Philippe Mathieu-Daudé wrote:
> > Disable tests using 0x4567 hardcoded RCA otherwise when
> > using random RCA we get:
> >
> >    ERROR:../../tests/qtest/npcm7xx_sdhci-test.c:69:write_sdread: 
> > assertion failed: (ret == len)
> >    not ok /arm/npcm7xx_sdhci/read_sd - 
> > ERROR:../../tests/qtest/npcm7xx_sdhci-test.c:69:write_sdread: 
> > assertion failed: (ret == len)
> >    Bail out!
> >
> > See 
> > 37f83be9-deb5-42a1-b704-14984351d803@linaro.org/">https://lore.kernel.org/qemu-devel/37f83be9-deb5-42a1-b704-14984351d803@linaro.org/
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> > Cc: Hao Wu <wuhaotsh@google.com>
> > Cc: Shengtan Mao <stmao@google.com>
> > Cc: Tyrone Ting <kfting@nuvoton.com>
> > ---
> >   tests/qtest/npcm7xx_sdhci-test.c | 7 +++++++
> >   1 file changed, 7 insertions(+)
> >
> > diff --git a/tests/qtest/npcm7xx_sdhci-test.c 
> > b/tests/qtest/npcm7xx_sdhci-test.c
> > index 5d68540e52..6a42b142ad 100644
> > --- a/tests/qtest/npcm7xx_sdhci-test.c
> > +++ b/tests/qtest/npcm7xx_sdhci-test.c
> > @@ -44,6 +44,7 @@ static QTestState *setup_sd_card(void)
> >       sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x41200000, 0, (41 << 
> > 8));
> >       sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_ALL_SEND_CID);
> >       sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, 
> > SDHC_SEND_RELATIVE_ADDR);
> > +    g_test_skip("hardcoded 0x4567 card address");
>
> This g_test_skip here does not make too much sense (since you're doing 
> it in the caller site, too) ... could you please replace it with a 
> proper comment why this code needs to be reworked? Thanks!

Sorry I forgot to tag "NOTFORMERGE". Ideally I'd wait Google
maintainers to fix the test so we don't need this patch. If
they don't I'll update as you suggested.
(An alternative I haven't investigated is to run the test
using the -seed argument to force deterministic mode).

Thanks!

>   Thomas
>
>
> >       sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x45670000, 0,
> >                      SDHC_SELECT_DESELECT_CARD);
> > @@ -76,6 +77,9 @@ static void test_read_sd(void)
> >   {
> >       QTestState *qts = setup_sd_card();
> > +    g_test_skip("hardcoded 0x4567 card address used in 
> > setup_sd_card()");
> > +    return;