Re: [Qemu-devel] [PATCH 06/10] r2d: Flash memory creation is confused about size, mark FIXME - Philippe Mathieu-Daudé

From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>, Cleber Rosa <crosa@redhat.com>
Cc: "Markus Armbruster" <armbru@redhat.com>,
	"Magnus Damm" <magnus.damm@gmail.com>,
	"Kevin Wolf" <kwolf@redhat.com>,
	Qemu-block <qemu-block@nongnu.org>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Max Reitz" <mreitz@redhat.com>, qemu-ppc <qemu-ppc@nongnu.org>,
	"Laszlo Ersek" <lersek@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 06/10] r2d: Flash memory creation is confused about size, mark FIXME
Date: Tue, 5 Mar 2019 22:50:45 +0100	[thread overview]
Message-ID: <90123914-f0c1-ea2e-f8e0-44fee62bb870@redhat.com> (raw)
In-Reply-To: <CAFEAcA-9U_D=UWTUH69mBBuC+a=FgXBC5kSawoCxDM4nB_FPZw@mail.gmail.com>

On 3/5/19 6:25 PM, Peter Maydell wrote:
> On Tue, 5 Mar 2019 at 17:21, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>> But I'd recommend changing/fixing the sector size during the next dev
>> cycle, so we have more time for testing.
> 
> Nobody in the upstream dev community is using or testing this board.

Well I submitted a Avocado test last year:
https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02749.html

And I rebase/run it from time to time.

> The only way we'll find out if there's a problem with changing the
> sector size is to put the change in and get it into a release.
> I would vote for just making the change now.

I'm happy with this vote, and am sure Markus will be too :)

Markus: the last field I wasn't sure about without double checking the
code is the @width one. I find it misleading, is that the size of the
data bus or the size of the flash words? Answer: this is the size of the
words in byte. NOR flash devices can not write less data than their word
boundary.

The S29PL127J60TFI130 only support 16bit words, so using @width=2 is
correct.

And the winner is.... ta-da!

    pflash_cfi02_register(0x0, NULL, "r2d.flash", FLASH_SIZE,
                          dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-                         16 * KiB, FLASH_SIZE >> 16,
-                         1, 4, 0x0000, 0x0000, 0x0000, 0x0000,
+                         64 * KiB, FLASH_SIZE >> 16 /* will get removed
later */,
+                         1, 2, 0x0001, 0x227e, 0x2220, 0x2200
                          0x555, 0x2aa, 0);

> 
> thanks
> -- PMM

Thanks for your support!

Phil.