All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Pratyush Yadav <p.yadav@ti.com>
Cc: Tudor Ambarus <Tudor.Ambarus@microchip.com>,
	Michael Walle <michael@walle.cc>,
	Rob Herring <robh+dt@kernel.org>, Mark Brown <broonie@kernel.org>,
	<linux-mtd@lists.infradead.org>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Michal Simek <monstr@monstr.eu>
Subject: Re: [RFC PATCH 0/3] Dual stacked/parallel memories bindings
Date: Fri, 26 Nov 2021 16:05:02 +0100	[thread overview]
Message-ID: <20211126160502.514b837a@xps13> (raw)
In-Reply-To: <20211119190023.rzcvxonvtl7pwjl4@ti.com>

Hi Pratyush,

p.yadav@ti.com wrote on Sat, 20 Nov 2021 00:30:25 +0530:

> On 16/11/21 09:19AM, Miquel Raynal wrote:
> > Hi Pratyush,
> > 
> > p.yadav@ti.com wrote on Mon, 15 Nov 2021 15:53:10 +0530:
> >   
> > > On 12/11/21 04:24PM, Miquel Raynal wrote:  
> > > > Hello Rob, Mark, Tudor & Pratyush,
> > > > 
> > > > Here is an RFC to open the discussion about the sensitive task of
> > > > supporting specific SPI controller modes like Xilinx's where the
> > > > controller can highly abstract the hardware and provide access to a
> > > > single bigger device instead. I'll let you go through the series and
> > > > tell me what you think.
> > > > 
> > > > I think there are two possible approaches:
> > > > 1- Describe the two devices as being a single one which is what we will
> > > >    get from the controller anyway (implies supporting two CS per SPI
> > > >    device)
> > > > or
> > > > 2- Describe the two devices in the device tree and then by software hack
> > > >    into the MTD core to simulate a single device to talk to.    
> > > 
> > > Approach 1 makes more sense to me since once we implement it you can 
> > > also use such multi-CS flashes with "dumber" controllers as well like 
> > > spi-cadence-quadspi. There, the driver would have to manually set the 
> > > chip select instead of it being done automatically by looking at the top 
> > > bit. This would at least work for the dual-stacked memories.  
> > 
> > I believe it would. But in that case we should think about a more
> > generic binding for the stacked mode. So far I've proposed:
> > - xlnx,dual-stacked-memories
> > - xlnx,dual-parallel-memories
> > 
> > It actually looks like the former might be a generic binding. What do
> > you think is best between:
> > - 'dual-stacked-memories'
> > - 'stacked-memories' ('dual' is encoded in the reg property)  
> 
> I think this works best. This would also allow "triple" and "quad" 
> stacked flashes.

Agreed.

> > - no specific property, it's just a memory with two CS, again 'reg'
> >   gives us the information.
> > 
> > Then we could keep only the latter property, which looks more specific
> > to Xilinx and use it as a flash node property instead (as advised
> > by Mark).  
> 
> Even if the parallel mode is only implemented by the Xilinx controller, 
> we would need to support it in the core, right? So we need to figure out 
> how that case would work as well.


> 
> >   
> > > How I envision this being implemented is that SPI NOR would be aware of 
> > > the number of Chip Selects and when to use which one, and it would 
> > > specify the CS value in the SPI MEM op.  
> > 
> > Yes, this is the approach I had in mind to. This fits both the purpose
> > of SPI-NOR and SPI-NAND which will both need to be updated as well tu
> > support multi-CS.
> >   
> > > The controller driver can then 
> > > execute this op as needed. One point to note here is that the entire 
> > > memory won't be read in a single transaction. There would be 2 
> > > transactions: one with CS=0 and one with CS=1. Is this fine for you? Do 
> > > you have something else in mind?  
> > 
> > I believe this should be let to the controller's discretion and appear
> > like a single op in the upper layers.  
> 
> But then how do you tell the controller when to change the CS if all it 
> sees is a single large transaction that spans across multiple flashes?

Actually after changing my mind a couple of times I think we agree on
the fact that the core should be aware of all of that, and:
- in parallel mode let the controller handle a single big op,
- in stacked mode split the request into two ops.

> You mention in patch 1 that your controller automatically switches CS 
> based on most significant address bit, but that would only work if you 
> have two 2 GiB flashes wired in. In case someone uses two 1 GiB flashes, 
> the MSB always remains 0. And what about controllers that can't switch 
> the CS automatically?
> 
> >   
> > > I am not sure how this model would work for a dual-parallel memory 
> > > though.  
> > 
> > If the controller is aware of the two CS and knows about the full
> > request we can hope that integration won't be difficult (last famous
> > words).  
> 
> For your specific controller this might work but if we want this feature 
> implemented generically I think it would need some more thought.
> 


Thanks,
Miquèl

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

      reply	other threads:[~2021-11-26 15:06 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-12 15:24 [RFC PATCH 0/3] Dual stacked/parallel memories bindings Miquel Raynal
2021-11-12 15:24 ` [RFC PATCH 1/3] spi: dt-bindings: Allow describing flashes with two CS Miquel Raynal
2021-11-12 16:52   ` Rob Herring
2021-11-16  8:21     ` Miquel Raynal
2021-11-12 15:24 ` [RFC PATCH 2/3] dt-binding: mtd: spi-nor: Allow two CS per device Miquel Raynal
2021-11-12 16:53   ` Rob Herring
2021-11-12 15:24 ` [RFC PATCH 3/3] spi: dt-bindings: zynqmp: Describe dual stacked/parallel memories modes Miquel Raynal
2021-11-12 15:42   ` Mark Brown
2021-11-16  8:23     ` Miquel Raynal
2021-11-15 10:23 ` [RFC PATCH 0/3] Dual stacked/parallel memories bindings Pratyush Yadav
2021-11-16  8:19   ` Miquel Raynal
2021-11-19 19:00     ` Pratyush Yadav
2021-11-26 15:05       ` Miquel Raynal [this message]

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=20211126160502.514b837a@xps13 \
    --to=miquel.raynal@bootlin.com \
    --cc=Tudor.Ambarus@microchip.com \
    --cc=broonie@kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=michael@walle.cc \
    --cc=monstr@monstr.eu \
    --cc=p.yadav@ti.com \
    --cc=richard@nod.at \
    --cc=robh+dt@kernel.org \
    --cc=thomas.petazzoni@bootlin.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.