From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LBsM5-0003bC-E8 for qemu-devel@nongnu.org; Sun, 14 Dec 2008 09:54:01 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LBsM3-0003YY-Rh for qemu-devel@nongnu.org; Sun, 14 Dec 2008 09:54:01 -0500 Received: from [199.232.76.173] (port=34083 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LBsM3-0003YR-PZ for qemu-devel@nongnu.org; Sun, 14 Dec 2008 09:53:59 -0500 Received: from 42.mail-out.ovh.net ([213.251.189.42]:47088) by monty-python.gnu.org with smtp (Exim 4.60) (envelope-from ) id 1LBsM3-0004JG-9n for qemu-devel@nongnu.org; Sun, 14 Dec 2008 09:53:59 -0500 Date: Sun, 14 Dec 2008 15:51:45 +0100 From: Jean-Christophe PLAGNIOL-VILLARD Subject: Re: [Qemu-devel] [PATCH] SH7750/51: add register BCR3, NCR4, PCR, RTCOR, RTCNT, RTCSR, SDMR2, SDMR3 and fix BCR2 support Message-ID: <20081214145145.GC18560@game.jcrosoft.org> References: <1228477782-19842-1-git-send-email-plagnioj@jcrosoft.com> <4944EFFE.1050604@juno.dti.ne.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4944EFFE.1050604@juno.dti.ne.jp> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Takashi Yoshii , Nobuhiro Iwamatsu > > > > - > > +static int inline is_sh7751r_cpu(SH7750State * s) > > +{ > > + return (s->cpu->id & (SH_CPU_SH7751R)); > > +} > > /********************************************************************** > > I/O ports > > **********************************************************************/ > > @@ -212,8 +219,17 @@ static uint32_t sh7750_mem_readw(void *opaque, target_phys_addr_t addr) > > switch (addr) { > > case SH7750_BCR2_A7: > > return s->bcr2; > > + case SH7750_BCR3_A7: > > + if(is_sh7751r_cpu(s)) { > > + return s->bcr3; > > + } else { > > + error_access("word read", addr); > > + assert(0); > > + } > > BCR3 exists not only for SH7751R, but also SH7750. > I think is_shh751r_cpu() check and error handling > should be removed to simplify the differcence. as write in the SH7751r datasheet Bus Control Register 3 (BCR3) (SH7751R Only) Bus Control Register 4 (BCR4) (SH7751R Only) That's why I've add the check > > > case SH7750_FRQCR_A7: > > return 0; > > + case SH7750_PCR_A7: > > + return s->pcr; > > case SH7750_RFCR_A7: > > fprintf(stderr, > > "Read access to refresh count register, incrementing\n"); > > @@ -222,6 +238,11 @@ static uint32_t sh7750_mem_readw(void *opaque, target_phys_addr_t addr) > > return porta_lines(s); > > case SH7750_PDTRB_A7: > > return portb_lines(s); > > + case SH7750_RTCOR_A7: > > + case SH7750_RTCNT_A7: > > + case SH7750_RTCSR_A7: > > + ignore_access("word read", addr); > > + return 0; > > case 0x1fd00000: > > return s->icr; > > default: > > @@ -238,6 +259,12 @@ static uint32_t sh7750_mem_readl(void *opaque, target_phys_addr_t addr) > > case SH7750_BCR1_A7: > > return s->bcr1; > > case SH7750_BCR4_A7: > > + if(is_sh7751r_cpu(s)) { > > + return s->bcr4; > > + } else { > > + error_access("long read", addr); > > + assert(0); > > + } > > For BCR4, same as above. > > > case SH7750_WCR1_A7: > > case SH7750_WCR2_A7: > > case SH7750_WCR3_A7: > > @@ -274,9 +301,17 @@ static uint32_t sh7750_mem_readl(void *opaque, target_phys_addr_t addr) > > } > > } > > > > +#define is_in_sdrmx(a, x) (a >= SH7750_SDMR ## x ## _A7 \ > > + && a <= (SH7750_SDMR ## x ## _A7 + SH7750_SDMR ## x ## _REGNB)) > > static void sh7750_mem_writeb(void *opaque, target_phys_addr_t addr, > > uint32_t mem_value) > > { > > + > > + if (is_in_sdrmx(addr, 2) || is_in_sdrmx(addr, 3)) { > > + ignore_access("word write", addr); > > + return; > > + } > > + > > SRMR2 and SDMR3 region overlaps with the PRECHARGE0/1 register. > If you introduce them, PRECHARGE0/1 register should be removed. > # U-Boot does not seem to touch these regions. Does it? IIRC no I'll check it > > Compared to 'if' statement, 'switch-case' might be more easy to > understand, like as follows. > case SH7750_SDMR2 ... SH7750_SDMR2 + SDMR2_REGNB ok evenif it's a gcc only extension Best Regards, J.