From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ot0-x242.google.com ([2607:f8b0:4003:c0f::242]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1cTT1Z-00060z-Ao for linux-mtd@lists.infradead.org; Tue, 17 Jan 2017 12:38:07 +0000 Received: by mail-ot0-x242.google.com with SMTP id f9so9061684otd.0 for ; Tue, 17 Jan 2017 04:37:44 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <20170116220916.2603-1-zajec5@gmail.com> <20170117105110.26328-1-zajec5@gmail.com> <31ca1f51-887d-fce4-400c-cc0f97353eb8@gmail.com> <4e268f6f-42fd-84a8-2b28-6ebb5129d945@gmail.com> From: =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= Date: Tue, 17 Jan 2017 13:37:43 +0100 Message-ID: Subject: Re: [PATCH V2] mtd: bcm47xxsflash: support reading flash out of mapping window To: Marek Vasut Cc: David Woodhouse , Brian Norris , Boris Brezillon , Richard Weinberger , Cyrille Pitchen , "linux-mtd@lists.infradead.org" , Hauke Mehrtens , =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 17 January 2017 at 13:04, Rafa=C5=82 Mi=C5=82ecki wro= te: > On 17 January 2017 at 12:49, Marek Vasut wrote: >> On 01/17/2017 12:08 PM, Rafa=C5=82 Mi=C5=82ecki wrote: >>> On 17 January 2017 at 12:00, Marek Vasut wrote: >>>> On 01/17/2017 11:51 AM, Rafa=C5=82 Mi=C5=82ecki wrote: >>>>> From: Rafa=C5=82 Mi=C5=82ecki >>>>> >>>>> For reading flash content we use MMIO but it's possible to read only >>>>> first 16 MiB this way. It's simply an arch design/limitation. >>>>> To support flash sizes bigger than 16 MiB implement indirect access >>>>> using ChipCommon registers. >>>>> This has been tested using MX25L25635F. >>>>> >>>>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki >>>>> --- >>>>> V2: Simplify line writing to buf >>>>> Add some trivial comment for OPCODE_ST_READ4B >>>>> Both requested by Marek >>>>> --- >>>>> drivers/mtd/devices/bcm47xxsflash.c | 12 +++++++++++- >>>>> drivers/mtd/devices/bcm47xxsflash.h | 3 +++ >>>>> 2 files changed, 14 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/mtd/devices/bcm47xxsflash.c b/drivers/mtd/device= s/bcm47xxsflash.c >>>>> index 4decd8c..8d4c1db 100644 >>>>> --- a/drivers/mtd/devices/bcm47xxsflash.c >>>>> +++ b/drivers/mtd/devices/bcm47xxsflash.c >>>>> @@ -110,7 +110,17 @@ static int bcm47xxsflash_read(struct mtd_info *m= td, loff_t from, size_t len, >>>>> if ((from + len) > mtd->size) >>>>> return -EINVAL; >>>>> >>>>> - memcpy_fromio(buf, b47s->window + from, len); >>>>> + if (from + len <=3D BCM47XXSFLASH_WINDOW_SIZE) { >>>>> + memcpy_fromio(buf, b47s->window + from, len); >>>> >>>> One last nit, what if the read starts < 16 MiB and ends > 16 MiB ? >>>> Shouldn't that use partly the windowed mode and partly the other mode? >>> >>> You'll lost 10ns*... or not as splitting it into 2 code paths could >>> take longer, who knows. Most access are block aligned anyway. I don't >>> think such corner case with doubtful gain is much worth considering & >>> optimizing. >> >> So you can also read the first 16 MiB using the new method , right ? > > I could, but this could be noticeable in performance. Reading 16 MiB > using slower method is different from reading what... a few KiB? Are > you actually sure mtd does unaligned reads at all? I did some quick test: if ((from & (b47s->blocksize - 1)) + len > b47s->blocksize) pr_warn("[%s] Block unaligned read from 0x%llx len:0x%zx\n", __func__, from, len); And it seems unaligned reads can happen: [ 147.338850] [bcm47xxsflash_read] Block unaligned read from 0x4fe1c len:0= x200 [ 147.663053] [bcm47xxsflash_read] Block unaligned read from 0x5fe1c len:0= x200 [ 147.983868] [bcm47xxsflash_read] Block unaligned read from 0x6fe1c len:0= x200 [ 148.304766] [bcm47xxsflash_read] Block unaligned read from 0x7fe1c len:0= x200 [ 148.625637] [bcm47xxsflash_read] Block unaligned read from 0x8fe1c len:0= x200 [ 148.955133] [bcm47xxsflash_read] Block unaligned read from 0x9fe1c len:0= x200 [ 149.275948] [bcm47xxsflash_read] Block unaligned read from 0xafe1c len:0= x200 [ 149.596790] [bcm47xxsflash_read] Block unaligned read from 0xbfe1c len:0= x200 [ 149.917604] [bcm47xxsflash_read] Block unaligned read from 0xcfe1c len:0= x200 [ 150.248641] [bcm47xxsflash_read] Block unaligned read from 0xdfe1c len:0= x200 [ 150.569484] [bcm47xxsflash_read] Block unaligned read from 0xefe1c len:0= x200 [ 150.890298] [bcm47xxsflash_read] Block unaligned read from 0xffe1c len:0= x200 [ 151.211140] [bcm47xxsflash_read] Block unaligned read from 0x10fe1c len:= 0x200 [ 151.541393] [bcm47xxsflash_read] Block unaligned read from 0x11fe1c len:= 0x200 [ 151.862292] [bcm47xxsflash_read] Block unaligned read from 0x12fe1c len:= 0x200 [ 152.183246] [bcm47xxsflash_read] Block unaligned read from 0x13fe1c len:= 0x200 [ 152.504200] [bcm47xxsflash_read] Block unaligned read from 0x14fe1c len:= 0x200 [ 152.834957] [bcm47xxsflash_read] Block unaligned read from 0x15fe1c len:= 0x200 [ 153.155856] [bcm47xxsflash_read] Block unaligned read from 0x16fe1c len:= 0x200 [ 153.476782] [bcm47xxsflash_read] Block unaligned read from 0x17fe1c len:= 0x200 [ 153.797681] [bcm47xxsflash_read] Block unaligned read from 0x18fe1c len:= 0x200 [ 154.126925] [bcm47xxsflash_read] Block unaligned read from 0x19fe1c len:= 0x200 [ 154.447823] [bcm47xxsflash_read] Block unaligned read from 0x1afe1c len:= 0x200 I got these reads of only 0x200 so that doesn't worry me much. Should I ever expect bigger reads in my driver callback? --=20 Rafa=C5=82