From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47824) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SsE3p-0008U8-37 for qemu-devel@nongnu.org; Fri, 20 Jul 2012 10:20:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SsE3j-00028H-Tj for qemu-devel@nongnu.org; Fri, 20 Jul 2012 10:20:04 -0400 Received: from mailout4.w1.samsung.com ([210.118.77.14]:14425) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SsE3j-000289-KZ for qemu-devel@nongnu.org; Fri, 20 Jul 2012 10:19:59 -0400 Received: from eusync2.samsung.com (mailout4.w1.samsung.com [210.118.77.14]) by mailout4.w1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0M7G00GSLQIBSD20@mailout4.w1.samsung.com> for qemu-devel@nongnu.org; Fri, 20 Jul 2012 15:20:35 +0100 (BST) Received: from [106.109.9.94] by eusync2.samsung.com (Oracle Communications Messaging Server 7u4-23.01(7.0.4.23.0) 64bit (built Aug 10 2011)) with ESMTPA id <0M7G00ML9QH77480@eusync2.samsung.com> for qemu-devel@nongnu.org; Fri, 20 Jul 2012 15:19:56 +0100 (BST) Message-id: <5009690B.7080301@samsung.com> Date: Fri, 20 Jul 2012 18:19:55 +0400 From: Igor Mitsyanko MIME-version: 1.0 References: <1342638220-3600-1-git-send-email-402jagan@gmail.com> <1342638220-3600-3-git-send-email-402jagan@gmail.com> In-reply-to: Content-type: multipart/alternative; boundary=------------030603040709060701030100 Subject: Re: [Qemu-devel] [PATCH 2/2] vexpress: Add NOR1 Flash support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: jagan <402jagan@gmail.com> Cc: Peter Crosthwaite , peter.maydell@linaro.org, qemu-devel@nongnu.org This is a multi-part message in MIME format. --------------030603040709060701030100 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit On 07/20/2012 05:30 PM, jagan wrote: > I think I understand the situation, like when I called > pflash_cfi01_register, it verifies BlockDriverState is < 0. > Was my understanding correct? > > If ie so. I think I need to change drive_get(IF_PFLASH, 0, 0) to > drive_get_next(IF_PFLASH) on second flash access by > keeping drive_get(IF_PFLASH, 0, 0 > on first flash, correct? > > But I am wondering why it's still detecting 2 Flashes with 128MB when > I tested through u-boot. > Even I was written Linux and Ramdisk on to flashes again read back and > able to boot it, Fine. > > Please find the attachment for u-boot log. > > I haven't tested -pflash argument through ./qemu-system-arm > because it again asking about kernel argument, do I need to test this > also? Now I understand why QEMU didn't abort during your test, If you do not specify a -pflash argument then there is no backing drive for any of flash banks and they just behave as read-only RAM regions, and in that case it doesn't matter whether you used drive_get(IF_PFLASH, 0, 0) or drive_get_next(IF_PFLASH) or nothing at all. U-boot should detect both banks without any problem, but he wouldn't be able to write anything to them. As Peter already said, you need to use drive_get_next(IF_PFLASH) for both flash banks initialization, or use drive_get(IF_PFLASH, 0, 0) for first and drive_get(IF_PFLASH, 0, 1) for second bank. > > Regards, > Jagan. > > On Fri, Jul 20, 2012 at 6:58 AM, Peter Crosthwaite > > wrote: > > On Thu, Jul 19, 2012 at 7:16 PM, jagan <402jagan@gmail.com > > wrote: > > Yes, I have used same drive_get(IF_PFLASH, 0, 0) with two flashes. > > As these flashes are two different banks with individual bases > address, I > > used the same. > > > > Was there any block allocation problem with this..will you > please elaborate. > > I couldn't understand about drive_get_next(), > > s/drive_get(IF_PFLASH, 0, 0) /drive_get_next(IF_PFLASH)/ > > This will mean you have two -fplash arguments on qemu cmd line > your two flashes. > > qemu-system-arm ... -pflash nor0.bin -pflash nor1.bin > > Currently you back (or attempt to back) both flashes to the same bdrv > which means they share storage. The two flashes will corrupt each > others data. > > Regards, > Peter > > I think function can be useful > > single drive devices SD/MTD. > > > > Please suggest your comments. > > > > Regards, > > Jagan. > > > > On Thu, Jul 19, 2012 at 5:27 AM, Peter Crosthwaite > > > wrote: > >> > >> On Thu, Jul 19, 2012 at 5:03 AM, <402jagan@gmail.com > > wrote: > >> > From: Jagan <402jagan@gmail.com > > >> > > >> > This patch adds support for NOR1 flash (Bank #2) on > >> > vexpress-a9 platform. It is 64MB CFI01 compliant flash. > >> > > >> > Tested on stable u-boot version through Linux. > >> > > >> > Signed-off-by: Jagan <402jagan@gmail.com > > > >> > --- > >> > hw/vexpress.c | 10 +++++++++- > >> > 1 files changed, 9 insertions(+), 1 deletions(-) > >> > > >> > diff --git a/hw/vexpress.c b/hw/vexpress.c > >> > index 2e889a8..b4262ed 100644 > >> > --- a/hw/vexpress.c > >> > +++ b/hw/vexpress.c > >> > @@ -422,7 +422,15 @@ static void vexpress_common_init(const > VEDBoardInfo > >> > *daughterboard, > >> > } > >> > > >> > /* VE_NORFLASH0ALIAS: not modelled */ > >> > - /* VE_NORFLASH1: not modelled */ > >> > + /* VE_NORFLASH1: */ > >> > + dinfo = drive_get(IF_PFLASH, 0, 0); > >> > >> Both flashes use drive_get(IF_PFLASH, 0, 0). Doesnt this means they > >> are both going to back to the same file (one -pflash argument) and > >> share storage? Should this use drive_get_next() and you specify two > >> -pflash args, one for each device? > >> > >> Regards > >> Peter > >> > >> > + if (!pflash_cfi01_register(map[VE_NORFLASH1], NULL, > >> > "vexpress.flash1", > >> > + VEXPRESS_FLASH_SIZE, dinfo ? dinfo->bdrv : NULL, > >> > + VEXPRESS_FLASH_SECT_SIZE, > >> > + VEXPRESS_FLASH_SIZE / VEXPRESS_FLASH_SECT_SIZE, > >> > + 4, 0x0089, 0x0018, 0x0000, 0x1, 0)) { > >> > + fprintf(stderr, "qemu: Error registering flash1 > memory.\n"); > >> > + } > >> > > >> > sram_size = 0x2000000; > >> > memory_region_init_ram(sram, "vexpress.sram", sram_size); > >> > -- > >> > 1.7.0.4 > >> > > >> > > > > > > > --------------030603040709060701030100 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit
On 07/20/2012 05:30 PM, jagan wrote:
I think I understand the situation, like when I called pflash_cfi01_register, it verifies BlockDriverState is < 0.
Was my understanding correct?

If ie so. I think I need to change  drive_get(IF_PFLASH, 0, 0)  to drive_get_next(IF_PFLASH) on second flash access by keeping  drive_get(IF_PFLASH, 0, 0 
on first flash, correct?

But I am wondering why it's still detecting 2 Flashes with 128MB when I tested through u-boot.
Even I was written Linux and Ramdisk on to flashes again read back and able to boot it, Fine.

Please find the attachment for u-boot log.

I haven't tested -pflash argument through ./qemu-system-arm because it again asking about kernel argument, do I need to test this also?

Now I understand why QEMU didn't abort during your test, If you do not specify a -pflash argument then there is no backing drive for any of flash banks and they just behave as read-only RAM regions, and in that case it doesn't matter whether you used drive_get(IF_PFLASH, 0, 0) or drive_get_next(IF_PFLASH) or nothing at all. U-boot should detect both banks without any problem, but he wouldn't be able to write anything to them.
As Peter already said, you need to use drive_get_next(IF_PFLASH) for both flash banks initialization, or use drive_get(IF_PFLASH, 0, 0) for first and drive_get(IF_PFLASH, 0, 1) for second bank.


Regards,
Jagan.

On Fri, Jul 20, 2012 at 6:58 AM, Peter Crosthwaite <peter.crosthwaite@petalogix.com> wrote:
On Thu, Jul 19, 2012 at 7:16 PM, jagan <402jagan@gmail.com> wrote:
> Yes, I have used same  drive_get(IF_PFLASH, 0, 0) with two flashes.
> As these flashes are two different banks with individual bases address, I
> used the same.
>
> Was there any block allocation problem with this..will you please elaborate.
> I couldn't understand about drive_get_next(),

s/drive_get(IF_PFLASH, 0, 0) /drive_get_next(IF_PFLASH)/

This will mean you have two -fplash arguments on qemu cmd line your two flashes.

qemu-system-arm ... -pflash nor0.bin -pflash nor1.bin

Currently you back (or attempt to back) both flashes to the same bdrv
which means they share storage. The two flashes will corrupt each
others data.

Regards,
Peter

 I think function can be useful
> single drive devices SD/MTD.
>
> Please suggest your comments.
>
> Regards,
> Jagan.
>
> On Thu, Jul 19, 2012 at 5:27 AM, Peter Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>>
>> On Thu, Jul 19, 2012 at 5:03 AM,  <402jagan@gmail.com> wrote:
>> > From: Jagan <402jagan@gmail.com>
>> >
>> > This patch adds support for NOR1 flash (Bank #2) on
>> > vexpress-a9 platform. It is 64MB CFI01 compliant flash.
>> >
>> > Tested on stable u-boot version through Linux.
>> >
>> > Signed-off-by: Jagan <402jagan@gmail.com>
>> > ---
>> >  hw/vexpress.c |   10 +++++++++-
>> >  1 files changed, 9 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/hw/vexpress.c b/hw/vexpress.c
>> > index 2e889a8..b4262ed 100644
>> > --- a/hw/vexpress.c
>> > +++ b/hw/vexpress.c
>> > @@ -422,7 +422,15 @@ static void vexpress_common_init(const VEDBoardInfo
>> > *daughterboard,
>> >      }
>> >
>> >      /* VE_NORFLASH0ALIAS: not modelled */
>> > -    /* VE_NORFLASH1: not modelled */
>> > +    /* VE_NORFLASH1: */
>> > +    dinfo = drive_get(IF_PFLASH, 0, 0);
>>
>> Both flashes use drive_get(IF_PFLASH, 0, 0). Doesnt this means they
>> are both going to back to the same file (one -pflash argument) and
>> share storage? Should this use drive_get_next() and you specify two
>> -pflash args, one for each device?
>>
>> Regards
>> Peter
>>
>> > +    if (!pflash_cfi01_register(map[VE_NORFLASH1], NULL,
>> > "vexpress.flash1",
>> > +        VEXPRESS_FLASH_SIZE, dinfo ? dinfo->bdrv : NULL,
>> > +        VEXPRESS_FLASH_SECT_SIZE,
>> > +        VEXPRESS_FLASH_SIZE / VEXPRESS_FLASH_SECT_SIZE,
>> > +        4, 0x0089, 0x0018, 0x0000, 0x1, 0)) {
>> > +        fprintf(stderr, "qemu: Error registering flash1 memory.\n");
>> > +    }
>> >
>> >      sram_size = 0x2000000;
>> >      memory_region_init_ram(sram, "vexpress.sram", sram_size);
>> > --
>> > 1.7.0.4
>> >
>> >
>
>


--------------030603040709060701030100--