All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Pratyush Yadav <p.yadav@ti.com>
Cc: Tudor Ambarus <tudor.ambarus@microchip.com>,
	Mark Brown <broonie@kernel.org>, Lee Jones <lee.jones@linaro.org>,
	Michael Walle <michael@walle.cc>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Mauro Lima <mauro.lima@eclypsium.com>,
	Alexander Sverdlin <alexander.sverdlin@nokia.com>,
	linux-mtd@lists.infradead.org, linux-spi@vger.kernel.org
Subject: Re: [PATCH 2/3] mtd: spi-nor: intel-spi: Convert to SPI MEM
Date: Thu, 7 Oct 2021 19:46:12 +0300	[thread overview]
Message-ID: <YV8kVKiMShWp4g7a@lahna> (raw)
In-Reply-To: <20211007123621.ld4aqasr3hlwq2c7@ti.com>

Hi,

On Thu, Oct 07, 2021 at 06:06:23PM +0530, Pratyush Yadav wrote:
> > Unfortunately there is no way to tell the controller any of these. It
> > simply does "read" or "write" (as we command it) and internally then
> > uses whatever it got from the SFDP tables of the flash chip. That's why
> > we just claim to support all these operations and let the controller do
> > its thing (whatever it is).
> 
> That is not ideal. SPI NOR uses this to negotiate the best available 
> protocol with the controller. Say you have a flash that is capable of 
> octal mode but the controller can only support quad mode. Your driver 
> will happily tell SPI NOR that it can support octal mode. I think you 
> should check the SPI mode bits to make sure the protocol bus width is 
> supported at least (see spi_check_buswidth_req() in spi-mem.c).

Okay, I'll see if I can add that check somewhere.

> As for opcodes, is there no way to find out what opcodes the controller 
> discovered via SFDP? Maybe we can't change them, but can we at least 
> take a peek at them?

AFAICT no. The controller only allows "higher" level commands like read,
write, erase but does not expose any of that to software. You can see
yourself if you want, the spec is here:
 
  https://cdrdv2.intel.com/v1/dl/getContent/636174

Page 403 has the control register.

> I think this has problems similar to the Cadence xSPI controller [0].

Probably but I would not call these "problems" - it is how the
controller is designed. This one is meant only for SPI-NOR flash access,
typically used by the BIOS. It is by no means general purpose SPI
controller (as you can see from the datasheet). The BIOS does need the
full SPI stack, it just issues these simple commands and let's the
controller figure out what actually needs to be done.

> Sorry I only replied to this after you posted a new version. It got lost 
> in the heap of emails in my inbox :-(

No worries :)

WARNING: multiple messages have this Message-ID (diff)
From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Pratyush Yadav <p.yadav@ti.com>
Cc: Tudor Ambarus <tudor.ambarus@microchip.com>,
	Mark Brown <broonie@kernel.org>, Lee Jones <lee.jones@linaro.org>,
	Michael Walle <michael@walle.cc>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Mauro Lima <mauro.lima@eclypsium.com>,
	Alexander Sverdlin <alexander.sverdlin@nokia.com>,
	linux-mtd@lists.infradead.org, linux-spi@vger.kernel.org
Subject: Re: [PATCH 2/3] mtd: spi-nor: intel-spi: Convert to SPI MEM
Date: Thu, 7 Oct 2021 19:46:12 +0300	[thread overview]
Message-ID: <YV8kVKiMShWp4g7a@lahna> (raw)
In-Reply-To: <20211007123621.ld4aqasr3hlwq2c7@ti.com>

Hi,

On Thu, Oct 07, 2021 at 06:06:23PM +0530, Pratyush Yadav wrote:
> > Unfortunately there is no way to tell the controller any of these. It
> > simply does "read" or "write" (as we command it) and internally then
> > uses whatever it got from the SFDP tables of the flash chip. That's why
> > we just claim to support all these operations and let the controller do
> > its thing (whatever it is).
> 
> That is not ideal. SPI NOR uses this to negotiate the best available 
> protocol with the controller. Say you have a flash that is capable of 
> octal mode but the controller can only support quad mode. Your driver 
> will happily tell SPI NOR that it can support octal mode. I think you 
> should check the SPI mode bits to make sure the protocol bus width is 
> supported at least (see spi_check_buswidth_req() in spi-mem.c).

Okay, I'll see if I can add that check somewhere.

> As for opcodes, is there no way to find out what opcodes the controller 
> discovered via SFDP? Maybe we can't change them, but can we at least 
> take a peek at them?

AFAICT no. The controller only allows "higher" level commands like read,
write, erase but does not expose any of that to software. You can see
yourself if you want, the spec is here:
 
  https://cdrdv2.intel.com/v1/dl/getContent/636174

Page 403 has the control register.

> I think this has problems similar to the Cadence xSPI controller [0].

Probably but I would not call these "problems" - it is how the
controller is designed. This one is meant only for SPI-NOR flash access,
typically used by the BIOS. It is by no means general purpose SPI
controller (as you can see from the datasheet). The BIOS does need the
full SPI stack, it just issues these simple commands and let's the
controller figure out what actually needs to be done.

> Sorry I only replied to this after you posted a new version. It got lost 
> in the heap of emails in my inbox :-(

No worries :)

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply	other threads:[~2021-10-07 16:47 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-30 10:07 [PATCH 0/3] mtd: spi-nor / spi / MFD: Convert intel-spi to SPI MEM Mika Westerberg
2021-09-30 10:07 ` Mika Westerberg
2021-09-30 10:07 ` [PATCH 1/3] mtd: spi-nor: intel-spi: Disable write protection only if asked Mika Westerberg
2021-09-30 10:07   ` Mika Westerberg
2021-10-01 20:23   ` Mauro Lima
2021-10-01 20:23     ` Mauro Lima
2021-10-04  5:18     ` Mika Westerberg
2021-10-04  5:18       ` Mika Westerberg
2021-10-12 18:49       ` Mauro Lima
2021-10-12 18:49         ` Mauro Lima
2021-10-13  9:03         ` Mika Westerberg
2021-10-13  9:03           ` Mika Westerberg
2021-10-13 18:22           ` Mauro Lima
2021-10-13 18:22             ` Mauro Lima
2021-09-30 10:07 ` [PATCH 2/3] mtd: spi-nor: intel-spi: Convert to SPI MEM Mika Westerberg
2021-09-30 10:07   ` Mika Westerberg
2021-10-04  9:52   ` Pratyush Yadav
2021-10-04  9:52     ` Pratyush Yadav
2021-10-04 10:07     ` Mika Westerberg
2021-10-04 10:07       ` Mika Westerberg
2021-10-07 12:36       ` Pratyush Yadav
2021-10-07 12:36         ` Pratyush Yadav
2021-10-07 16:46         ` Mika Westerberg [this message]
2021-10-07 16:46           ` Mika Westerberg
2021-10-07 18:00           ` Pratyush Yadav
2021-10-07 18:00             ` Pratyush Yadav
2021-10-08  9:02             ` Mika Westerberg
2021-10-08  9:02               ` Mika Westerberg
2021-10-08 10:56               ` Pratyush Yadav
2021-10-08 10:56                 ` Pratyush Yadav
2021-10-04 14:29   ` Andy Shevchenko
2021-10-04 14:29     ` Andy Shevchenko
2021-10-05  9:41     ` Mika Westerberg
2021-10-05  9:41       ` Mika Westerberg
2021-09-30 10:07 ` [PATCH 3/3] Documentation / MTD: Rename the intel-spi driver Mika Westerberg
2021-09-30 10:07   ` Mika Westerberg
2021-09-30 15:03   ` Alexander Sverdlin
2021-09-30 15:03     ` Alexander Sverdlin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YV8kVKiMShWp4g7a@lahna \
    --to=mika.westerberg@linux.intel.com \
    --cc=alexander.sverdlin@nokia.com \
    --cc=broonie@kernel.org \
    --cc=corbet@lwn.net \
    --cc=lee.jones@linaro.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=mauro.lima@eclypsium.com \
    --cc=michael@walle.cc \
    --cc=miquel.raynal@bootlin.com \
    --cc=p.yadav@ti.com \
    --cc=richard@nod.at \
    --cc=tudor.ambarus@microchip.com \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.