Re: [PATCH v2] mtd: spi-nor: Add support for SPI boot flash access for AMD Family 16h

[Boris Brezillon]

On Wed, 31 Oct 2018 03:18:28 +0000
"Grandbois, Brett" <brett.grandbois@xxxxxxxxxxxx> wrote:

> On 30/10/18 6:26 pm, Boris Brezillon wrote:
> > On Mon, 29 Oct 2018 23:15:42 +0000
> > "Grandbois, Brett"<brett.grandbois@xxxxxxxxxxxx>  wrote:
> >  
> >> On 28/10/18 1:39 am, Boris Brezillon wrote:  
> >>> Hi Brett,
> >>>
> >>> On Tue, 16 Oct 2018 00:57:41 +0000
> >>> "Grandbois, Brett"<brett.grandbois@xxxxxxxxxxxx>   wrote:
> >>>     
> >>>> Add support to expose the SPI boot flash on AMD Family 16h CPUs
> >>>> as a standard mtd device to give userspace BIOS updaters greater
> >>>> feature support.  The BIOS and Kernel Developer's Guide refers to
> >>>> this as the 'SPI ROM' controller and so the driver follows that
> >>>> naming convention for consistency.
> >>>>     
> >>> We're currently trying to convert spi-nor controller drivers to
> >>> the spi-mem interface [1]. Can you look at this new interface and
> >>> tell me if you'd be able to implement it? If that's not possible,
> >>> then I'd prefer to have this driver implement the mtd_info
> >>> interface directly.  
> >> So from going over the spi-mem interface it looks like the intent
> >> is for these sorts of devices to be a standard spi_controller with
> >> only mem_ops defined and the transfer/_one/_one_message left as
> >> NULL?  Is that correct?  
> > Yes
> >  
> >> That's a bit of a pivot from how it's currently done
> >> (it's conceptually similar to the intel-spi-pci driver so I was
> >> following that)  
> > Yes, and that's exactly what I'd like to avoid. intel-spi-pci will
> > probably be the trickiest conversion, so I'd like to avoid having
> > another one ;-).
> >  
> >> but I should be able to rework it to the new
> >> interface.  This then lives under drivers/spi and thus should be
> >> submitted to linux-spi?  
> > Actually, if the controller is only ever connected to the same SPI
> > NOR chip (no need for advanced detection scheme) and does not
> > support support Octo/Quad/Dual modes (or any other advanced
> > features), you'll be better off implementing
> > mtd->_read/_write/_erase() directly (the driver would then live in
> > drivers/mtd/devices/).  
> 
> Hmm, conceptually the device is better suited to the mtd layer than
> the spi layer.  The HW is designed to only ever access spi flash
> chips, not as a general spi master controller, so really the end
> result of it should always only ever be 1 mtd device.  Unfortunately
> as the device probing and command sequences for this are the same as
> implemented in spi-nor I'd either be duplicating a lot of existing
> code, or just wrapping around spi_nor_scan which sounds like we're
> back to the dedicated spi-nor controller you're trying to move away
> from.
> 
> The spi-mem interface ops do nicely match up with what the controller
> supports, including the new direct mapping mode which I'd be able to
> make use of, so as long as there aren't any issues with supporting
> only mem_ops and not the message transfers

That's allowed.

> then it's probably the way to go.

Sounds good then.