From: Artur Jedrysek <jartur@cadence.com>
To: Marek Vasut <marek.vasut@gmail.com>,
        "linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>
CC: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Cyrille Pitchen <cyrille.pitchen@atmel.com>,
        David Woodhouse <dwmw2@infradead.org>,
        Brian Norris <computersforpeace@gmail.com>,
        Boris Brezillon <boris.brezillon@free-electrons.com>,
        Richard Weinberger <richard@nod.at>, Dinh Nguyen <dinguyen@kernel.org>
Subject: RE: [v2, 2/4] mtd: spi-nor: Add Octal SPI support to Cadence QSPI
 driver.
Thread-Topic: [v2, 2/4] mtd: spi-nor: Add Octal SPI support to Cadence QSPI
 driver.
Thread-Index: AQHSl+J0sYoB0K7MN0ue0U4PecUqKKGNb3QAgACJu8CAABCVAIAAE1aggAAEmICAAAGO0A==
Date: Fri, 10 Mar 2017 14:22:11 +0000
Message-ID: <DM5PR07MB27801B207247AB49B4E352F5D4200@DM5PR07MB2780.namprd07.prod.outlook.com>
References: <1488959932-6657-1-git-send-email-jartur@cadence.com>
 <1488960178-11079-1-git-send-email-jartur@cadence.com>
 <891f9177-e03c-85e0-ebab-2dc7df2ce9e0@gmail.com>
 <DM5PR07MB278038D86D5AD05FA2F5AAA1D4200@DM5PR07MB2780.namprd07.prod.outlook.com>
 <8315061c-716d-89c9-3f04-687b48fbac22@gmail.com>
 <DM5PR07MB2780DC9981927880E1ED1B37D4200@DM5PR07MB2780.namprd07.prod.outlook.com>
 <23371579-495a-1336-cce9-3860a2e2a4c1@gmail.com>
In-Reply-To: <23371579-495a-1336-cce9-3860a2e2a4c1@gmail.com>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Mar 2017 14:22:11.3150
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: d36035c5-6ce6-4662-a3dc-e762e61ae4c9
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR07MB2777
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id v2AEMR1H028164
Content-Length: 2585
Lines: 74


From: Marek Vasut [mailto:marek.vasut@gmail.com]
Sent: 10 marca 2017 15:15

> On 03/10/2017 03:09 PM, Artur Jedrysek wrote:
> 
> CCing Dinh.
> 
> [...]
> 
> >>>>> +	/* Determine, whether or not octal transfer MAY be supported */
> >>>>
> >>>> But you already know that from DT, no ?
> >>>>
> >>>
> >>> Cadence Octal (formerly Quad) SPI Controller is sold as an IP, and is
> >>> configurable. This includes max SPI mode. It is possible to detect, that
> >>> Octal SPI controller is configured (during hardware compilation) to support
> >>> up to Quad mode, using revision register.
> >>
> >> So the octal-spi controller always has this config register, but the
> >> quad-spi controller may or may not have this register ?
> >>
> >
> > This register was always present. In Quad-SPI, however, it didn't contain
> > information about maximum possible mode, as only quad was possible, and
> > meaning of bits checked here was different.
> 
> OK
> 
> >>>>> +	rev_reg = readl(cqspi->iobase + CQSPI_REG_MODULEID);
> >>>>> +	dev_info(dev, "CQSPI Module id %x\n", rev_reg);
> >>>>> +
> >>>>> +	switch (rev_reg & CQSPI_REG_MODULEID_CONF_ID_MASK) {
> >>>>> +	case CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY:
> >>>>> +	case CQSPI_REG_MODULEID_CONF_ID_OCTAL:
> >>>>> +		mode = SPI_NOR_OCTAL;
> >>>>> +		break;
> >>>>> +	case CQSPI_REG_MODULEID_CONF_ID_QUAD:
> >>>>> +	case CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY:
> >>>>
> >>>> Does this work on all revisions of CQSPI ?
> >>>>
> >>>
> >>> After having a more thorough look at specification of older IP version
> >>> (quad only) it seems, that revision register format has indeed changed.
> >>> This will be fixed in the next version of the patch.
> >>
> >> Can the quad-spi controller be configured only as dual or single ?
> >> What about the octal one ? These cases should probably be handled
> >> somehow too, right ?
> >>
> >
> > Quad-SPI controller can always support single, dual and quad. There was
> > no option to configure max mode. Octal-SPI controller can be configured
> > to support either octal or quad mode. No controller could be configured
> > (during hardware compilation/synthesis) to support only single/dual
> > SPI mode. To put it shortly: single, dual and quad is always supported.
> 
> So basically the whole check you need to perform here is
> 
> mode = quad;
> if (controller->flags & CAN_DO_OCTAL) {
>  if (readl(ID_REGISTER) & IS_CONFIGURED_AS_OCTAL)
>    mode = octal;
> }
> 

Yes. Something similar was already written and tested, and will be sent
in the next version of the patch instead.

> --
> Best regards,
> Marek Vasut