From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42235) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VrA91-0000JZ-0C for qemu-devel@nongnu.org; Thu, 12 Dec 2013 12:33:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VrA8w-0002hR-6U for qemu-devel@nongnu.org; Thu, 12 Dec 2013 12:33:50 -0500 Received: from mail-pb0-f46.google.com ([209.85.160.46]:39838) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VrA8w-0002hD-1J for qemu-devel@nongnu.org; Thu, 12 Dec 2013 12:33:46 -0500 Received: by mail-pb0-f46.google.com with SMTP id md12so903858pbc.33 for ; Thu, 12 Dec 2013 09:33:45 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <1386279359-32286-7-git-send-email-roy.franz@linaro.org> References: <1386279359-32286-1-git-send-email-roy.franz@linaro.org> <1386279359-32286-7-git-send-email-roy.franz@linaro.org> From: Peter Maydell Date: Thu, 12 Dec 2013 17:33:25 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH V5 6/7] Fix CFI query responses for NOR flash List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roy Franz Cc: Kevin Wolf , QEMU Developers , Stefan Hajnoczi , Patch Tracking On 5 December 2013 21:35, Roy Franz wrote: > This change fixes the CFI query responses to handle NOR device > widths that are different from the bank width. Support is also > added for multi-width devices in a x8 configuration. This is > typically x8/x16 devices, but the CFI specification mentions > x8/x32 devices so those should be supported as well if they > exist. > The query response data is now replicated per-device in the bank, > and is adjusted for x16 or x32 parts configured in x8 mode. > > The existing code is left in place for boards that have not > been updated to specify an explicit device_width. The VExpress > board has been updated in an earlier patch in this series so > this is the only board currently affected. > > Signed-off-by: Roy Franz > --- > hw/block/pflash_cfi01.c | 103 ++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 92 insertions(+), 11 deletions(-) > > diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c > index 8f81341..564e6ee 100644 > --- a/hw/block/pflash_cfi01.c > +++ b/hw/block/pflash_cfi01.c > @@ -119,6 +119,66 @@ static void pflash_timer (void *opaque) > pfl->cmd = 0; > } > > +/* Perform a CFI query based on the bank width of the flash. > + * If this code is called we know we have a device_width set for > + * this flash. > + */ > +static uint32_t pflash_cfi_query(pflash_t *pfl, hwaddr offset) > +{ > + int i; > + uint32_t resp = 0; > + hwaddr boff; > + > + /* Adjust incoming offset to match expected device-width > + * addressing. CFI query addresses are always specified in terms of > + * the maximum supported width of the device. This means that x8 > + * devices and x8/x16 devices in x8 mode behave differently. For > + * devices that are not used at their max width, we will be > + * provided with addresses that use higher address bits than > + * expected (based on the max width), so we will shift them lower > + * so that they will match the addresses used when > + * device_width==max_device_width. > + */ > + boff = offset >> (ctz32(pfl->bank_width) + > + ctz32(pfl->max_device_width) - ctz32(pfl->device_width)); > + > + if (boff > pfl->cfi_len) { > + return 0; > + } > + /* Now we will construct the CFI response generated by a single > + * device, then replicate that for all devices that make up the > + * bus. For wide parts used in x8 mode, CFI query responses > + * are different than native byte-wide parts. > + */ > + resp = pfl->cfi_table[boff]; > + if (pfl->device_width != pfl->max_device_width) { > + /* The only case currently supported is x8 mode for a > + * wider part. > + */ > + if (pfl->device_width != 1 || pfl->bank_width > 4) { > + DPRINTF("%s: Unsupported device configuration: device_width=%d, max_device_width=%d\n", This line is overlong and needs a linebreak. > + boff = offset & 0xFF; > + if (pfl->bank_width == 2) > + boff = boff >> 1; > + else if (pfl->bank_width == 4) > + boff = boff >> 2; > + Missing braces. Otherwise: Reviewed-by: Peter Maydell thanks -- PMM