Re: [PULL 22/23] hw/sd: Fix incorrect populated function switch status data structure

From: Bin Meng <bmeng.cn@gmail.com>
To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	qemu-block@nongnu.org,
	Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>,
	Bin Meng <bin.meng@windriver.com>,
	"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
	Andrew Baumann <Andrew.Baumann@microsoft.com>,
	Beniamino Galvani <b.galvani@gmail.com>,
	Niek Linnenbank <nieklinnenbank@gmail.com>,
	qemu-arm <qemu-arm@nongnu.org>, Michael Walle <michael@walle.cc>
Subject: Re: [PULL 22/23] hw/sd: Fix incorrect populated function switch status data structure
Date: Wed, 21 Oct 2020 17:57:33 +0800	[thread overview]
Message-ID: <CAEUhbmVFZ8aWdv48D8acdfFRp6WMYL6n=ENeo4hBZfmqE9TrDA@mail.gmail.com> (raw)
In-Reply-To: <e378932a-1de3-83c1-834f-ae6526604cb7@amsat.org>

Hi Philippe,

On Tue, Oct 20, 2020 at 11:18 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Hi Bin,
>
> On 8/21/20 7:29 PM, Philippe Mathieu-Daudé wrote:
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > At present the function switch status data structure bit [399:376]
> > are wrongly pupulated. These 3 bytes encode function switch status
> > for the 6 function groups, with 4 bits per group, starting from
> > function group 6 at bit 399, then followed by function group 5 at
> > bit 395, and so on.
> >
> > However the codes mistakenly fills in the function group 1 status
> > at bit 399. This fixes the code logic.
> >
> > Fixes: a1bb27b1e9 ("SD card emulation (initial implementation)")
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > Tested-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> > Message-Id: <1598021136-49525-1-git-send-email-bmeng.cn@gmail.com>
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> >   hw/sd/sd.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> > index 7c9d956f113..805e21fc883 100644
> > --- a/hw/sd/sd.c
> > +++ b/hw/sd/sd.c
> > @@ -807,11 +807,12 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
> >       sd->data[11] = 0x43;
> >       sd->data[12] = 0x80;    /* Supported group 1 functions */
> >       sd->data[13] = 0x03;
> > +
> >       for (i = 0; i < 6; i ++) {
> >           new_func = (arg >> (i * 4)) & 0x0f;
> >           if (mode && new_func != 0x0f)
> >               sd->function_group[i] = new_func;
> > -        sd->data[14 + (i >> 1)] = new_func << ((i * 4) & 4);
> > +        sd->data[16 - (i >> 1)] |= new_func << ((i % 2) * 4);
>
> This patch broke the orangepi machine, reproducible running
> test_arm_orangepi_bionic:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg739449.html
>
> Can you have a look?

Yes, I can take a look. Could you please send more details on how to
run "test_arm_orangepi_bionic"?

Regards,
Bin