From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26562 invoked by alias); 25 Mar 2011 15:15:38 -0000 Received: (qmail 26543 invoked by uid 22791); 25 Mar 2011 15:15:37 -0000 X-SWARE-Spam-Status: No, hits=-6.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SARE_SUB_ENC_UTF8,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 25 Mar 2011 15:15:27 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p2PFFAWN013122 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 25 Mar 2011 11:15:10 -0400 Received: from fche.csb (vpn-8-169.rdu.redhat.com [10.11.8.169]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p2PFF9oE011125; Fri, 25 Mar 2011 11:15:10 -0400 Received: by fche.csb (Postfix, from userid 2569) id 624A058158; Fri, 25 Mar 2011 11:15:09 -0400 (EDT) To: Mike Frysinger Cc: gdb-patches@sourceware.org, toolchain-devel@blackfin.uclinux.org Subject: Re: =?us-ascii?Q?=3D=3FUTF-8=3Fq=3F=3D5BPATCH=3D20v3=3D5D=3D20sim?= =?us-ascii?Q?=3D3A=3D20cfi=3D3A=3D20new=3D20flash=3D20device=3D20simulati?= =?us-ascii?Q?on=3F=3D?= References: <1293750414-14626-1-git-send-email-vapier@gentoo.org> <1301012212-2372-1-git-send-email-vapier__17299.0819243298$1301012210$gmane$org@gentoo.org> From: fche@redhat.com (Frank Ch. Eigler) Date: Fri, 25 Mar 2011 15:54:00 -0000 In-Reply-To: <1301012212-2372-1-git-send-email-vapier__17299.0819243298$1301012210$gmane$org@gentoo.org> (Mike Frysinger's message of "Thu, 24 Mar 2011 20:16:52 -0400") Message-ID: User-Agent: Gnus/5.1008 (Gnus v5.10.8) Emacs/21.4 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-03/txt/msg01094.txt.bz2 vapier wrote: > This simulates a CFI flash. [...] Nice work, good looking code. Only some tiny nits: > +static unsigned > +cfi_unshift_addr (struct cfi *cfi, unsigned addr) > +{ > + switch (cfi->width) > + { > + case 4: addr >>= 1; > + case 2: addr >>= 1; > + } A note about the deliberate case fallthrough might be nice. > +/* Clean up any state when this device is removed (e.g. when shutting down, > + or when reloading via gdb). */ > +static void > +cfi_delete_callback (struct hw *me) > +{ > +#ifdef HAVE_MMAP > + struct cfi *cfi = hw_data (me); > + > + if (cfi->mmap) > + munmap (cfi->mmap, cfi->dev_size); > +#endif > +} Woudln't you want to write(2) out the contents of the flash backing store file, if !HAVE_MMAP? > +static void > +attach_cfi_regs (struct hw *me, struct cfi *cfi) > +[...] > + cfi->data = HW_NALLOC (me, unsigned char, cfi->dev_size); > + if (fd != -1) > + read_len = read (fd, cfi->data, cfi->dev_size); It's more typical to loop in read(2), in case of an EINTR or somesuch temporary-failure return code. Or use fread(3)? - FChE