[PATCH linux dev-4.7] arm: aspped: romulus: Set PNOR SPI address mapping

[Andrew Jeffery]

On Wed, 2017-03-01 at 08:26 +0100, Cédric Le Goater wrote:
> On 03/01/2017 04:05 AM, Andrew Jeffery wrote:
> > Replying to myself again... I think I've learnt the lesson not to email
> > whilst in bed.
> > 
> > I've done some further investigation after jumping on a Romulus
> > machine:
> > 
> > On Tue, 2017-02-28 at 23:10 +1030, Andrew Jeffery wrote:
> > > On Tue, 2017-02-28 at 20:27 +0800, Lei YU wrote:
> > > > Hi Andrew, Joel,
> > > > 
> > > > This patch is for issue https://github.com/openbmc/openbmc/issues/1214
> > > > 
> > > > Here's what I have tested:
> > > > 1. rom1-19 with updated OpenBMC (c85f23e1), which has mboxd enabled;
> > > > 2. Do a power cycle;
> > > > 3. Once BMC is ready, check the registers by devmem:
> > > >    ```
> > > > >    root at romulus:/tmp# devmem 0x1e630034
> > > > 
> > > >    0x70640000        <== Wrong, expect 0x70680000
> > > > >    root at romulus:/tmp# devmem 0x1e630030
> > > > 
> > > >    0x64600000        <== Wrong, expect 0x68600000
> > > >    ```
> > > > I can see the two registers have the unexpected value.
> 
> These are the default values : 
> 
> SPIR30: SPI1 CE0 Address Decoding Range Register Init = 0x6460 0000
> SPIR34: SPI1 CE1 Address Decoding Range Register Init = 0x7064 0000
> 
> We should let the driver configure these values I think.
> see below.
> 

Agreed.

> > 
> > Looking at the driver[1] the only time we touch the segment register is
> > to read the window start, we don't ever write the segment mapping
> > configuration.
> 
> yes, not in 4.7 yet. It is a little "complex" to configure the 
> segments up to 5 chips handling the possible overlaps, the 
> different chip sizes, which might be bigger than the maximum 
> window size, and the HW bugs. AST2500 SPI controller has a bug 
> when the CE0 chip size exceeds 120MB.

Yeah, I appreciate it's a finicky problem.

> 
> Also the controller only really makes use of them in command 
> mode. In user mode, the start address is mostly used to 
> identify the chip when writing commands on the AHB Bus.    
> 
> > Cédric: Have you looked into supporting configuration of the segment
> > mapping?
> 
> yes. QEMU (openbmc and mainline) supports them already. 
> 
> As for the kernel, here is an initial commit :
> 
>    https://github.com/legoater/linux/commit/c08765882b4091cb1b9998d872415c072ecfe8af
> 
> which I will send with a bunch of improvements when we switch 
> to linux 4.10. I can eventually rebase on 4.7 but I would have
> preferred not maintaining too much branches.

That's fair enough. If we apply Lei's patch to 4.7 as a hack it means
that we can ignore the issue from then on. Any new systems will be
based on at least 4.10, which should have your improvements.

> 
> > We could configure the mapping if we know the chip sizes, however
> > looking at the device's bindings documentation[2] we find:
> > 
> >     ...
> >     Required properties:
> >     ...
> >      - #size-cells : must be 0 corresponding to chip select child binding
> > 
> > and
> > 
> >     ...
> >     Child node required properties:
> >       - reg : must contain chip select number in first cell of address, must
> >     be 1 tuple long
> >     ...
> > 
> > I think we need #size-cells to be 1 in the parent so we can define the
> > mapping ranges. I don't know that its going to be an easy change to
> > make though given the bindings are already upstream. Thoughts?
> 
> In the aspeed-smc bindings, the mapping range is for the overall 
> AHB window. This is a HW constant you can not change. 
> 
> As for the settings of the each chip window, I would let the 
> driver discover and configure them as you can ruin the controller 
> behavior in command mode if they are wrong. 

I wasn't concerned about the overall AHB window so much as the chip
windows. Letting the driver discover and configure them was what I was
hoping for, and was investigating providing the information via the DT.
If we have an alternative approach (querying the chip?) that would be
great.

>  
> What is the specific need ? Is it to have a window covering all 
> the SPI CEx chips ? 

In this case it was to have CE0 window cover 64MB rather than 32MB.
Supporting more than one chip would be a nice-to-have rather than
something Lei needs as far as I understand.

> If so, my tree should have that support 
> already. But, please note that the HW is broken for chips > 128MB, 
> on witherspoon for instance.
> 

I guess the question is about what support is going to be in which
branch of openbmc/linux. As it stands it looks like dev-4.7 will not
have chip window configuration support in the driver, and dev-4.10
will.

Andrew
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20170302/fb1ddbbd/attachment-0001.sig>