From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S939823AbdDSVf0 (ORCPT ); Wed, 19 Apr 2017 17:35:26 -0400 Received: from mail-wr0-f178.google.com ([209.85.128.178]:36066 "EHLO mail-wr0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S939808AbdDSVfY (ORCPT ); Wed, 19 Apr 2017 17:35:24 -0400 Subject: Re: [PATCH v7 3/4] mtd: spi-nor: introduce Double Transfer Rate (DTR) SPI protocols To: Cyrille Pitchen , Cyrille Pitchen , linux-mtd@lists.infradead.org References: <73d701a3-cd56-70b2-cf15-ec16e6734481@gmail.com> <69db61ee-5de3-6518-6d44-fd4988d18bb6@wedev4u.fr> Cc: boris.brezillon@free-electrons.com, computersforpeace@gmail.com, dwmw2@infradead.org, linux-kernel@vger.kernel.org, richard@nod.at From: Marek Vasut Message-ID: Date: Wed, 19 Apr 2017 23:35:21 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.4.0 MIME-Version: 1.0 In-Reply-To: <69db61ee-5de3-6518-6d44-fd4988d18bb6@wedev4u.fr> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/19/2017 10:20 PM, Cyrille Pitchen wrote: > Hi Marek, > > Le 19/04/2017 à 01:05, Marek Vasut a écrit : >> On 04/19/2017 12:51 AM, Cyrille Pitchen wrote: >>> This patch introduces support to Double Transfer Rate (DTR) SPI protocols. >>> DTR is used only for Fast Read operations. >>> >>> According to manufacturer datasheets, whatever the number of I/O lines >>> used during instruction (x) and address/mode/dummy (y) clock cycles, DTR >>> is used only during data (z) clock cycles of SPI x-y-z protocols. >>> >>> Signed-off-by: Cyrille Pitchen >> >> [...] >> >>> @@ -282,19 +305,22 @@ struct spi_nor_hwcaps { >>> * As a matter of performances, it is relevant to use Quad SPI protocols first, >>> * then Dual SPI protocols before Fast Read and lastly (Slow) Read. >>> */ >>> -#define SNOR_HWCAPS_READ_MASK GENMASK(7, 0) >>> +#define SNOR_HWCAPS_READ_MASK GENMASK(10, 0) >>> #define SNOR_HWCAPS_READ BIT(0) >>> #define SNOR_HWCAPS_READ_FAST BIT(1) >>> - >>> -#define SNOR_HWCAPS_READ_DUAL GENMASK(4, 2) >>> -#define SNOR_HWCAPS_READ_1_1_2 BIT(2) >>> -#define SNOR_HWCAPS_READ_1_2_2 BIT(3) >>> -#define SNOR_HWCAPS_READ_2_2_2 BIT(4) >>> - >>> -#define SNOR_HWCAPS_READ_QUAD GENMASK(7, 5) >>> -#define SNOR_HWCAPS_READ_1_1_4 BIT(5) >>> -#define SNOR_HWCAPS_READ_1_4_4 BIT(6) >>> -#define SNOR_HWCAPS_READ_4_4_4 BIT(7) >>> +#define SNOR_HWCAPS_READ_1_1_1_DTR BIT(2) >>> + >>> +#define SNOR_HWCAPS_READ_DUAL GENMASK(6, 3) >>> +#define SNOR_HWCAPS_READ_1_1_2 BIT(3) >>> +#define SNOR_HWCAPS_READ_1_2_2 BIT(4) >>> +#define SNOR_HWCAPS_READ_2_2_2 BIT(5) >>> +#define SNOR_HWCAPS_READ_1_2_2_DTR BIT(6) >>> + >>> +#define SNOR_HWCAPS_READ_QUAD GENMASK(10, 7) >>> +#define SNOR_HWCAPS_READ_1_1_4 BIT(7) >>> +#define SNOR_HWCAPS_READ_1_4_4 BIT(8) >>> +#define SNOR_HWCAPS_READ_4_4_4 BIT(9) >>> +#define SNOR_HWCAPS_READ_1_4_4_DTR BIT(10) >> >> I can't say I'm a big fan on this re-numeration when you add a new >> entry. But unless you have a better idea, we'll have to live with this ... >> > > Well, the other solution would be to reserve unused bit position in > patch 1 but would look odd too, wouldn't it? Yeah, that's not super either ... I was pondering if there might be some less error-prone way to autogenerate this maybe. > As explained in the comments just above those definitions, the order of > the bits *does* matter. So maybe in the future, those bits would have to > be reordered again depending on the new features we would add then. > > Thanks for your review! > > Best regards, > > Cyrille > -- Best regards, Marek Vasut