Re: [PATCH] ARM: dts: armada-{370,xp}: drop "marvell,orion-spi" from SPI controllers

[Andrew Lunn <andrew@lunn.ch>]

On Thu, Sep 09, 2021 at 09:50:05AM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> On Wed, Dec 07, 2016 at 04:41:45PM +0100, Uwe Kleine-König wrote:
> > Hello Gregory,
> > 
> > On Wed, Dec 07, 2016 at 04:30:02PM +0100, Gregory CLEMENT wrote:
> > >  On mer., déc. 07 2016, Uwe Kleine-König <uwe@kleine-koenig.org> wrote:
> > > 
> > > > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > >
> > > > The SPI controllers on Armada 370 and XP differ from the original Orion
> > > > SPI controllers (at least) in the configuration of the baud rate. So
> > > > it's wrong to claim compatibility which results in bogus baud rates.
> > > 
> > > Until two years ago with the commits
> > > df59fa7f4bca9658b75f0f5fee225b3a057475c5 and
> > > 4dacccfac69494ba70248b134352f299171c41b7
> > > we used "marvell,orion-spi" compatible on Armada XP and Armada 370
> > > without any problem.
> > > 
> > > The new compatible "marvell,armada-xp-spi" and "marvell,armada-xp-spi"
> > > allows to have more choice for the baudrate for a given clock but it is
> > > not true that Armada 370 and Armada XP are not compatible with
> > > "marvell,orion-spi".
> > 
> > The issue I was faced with that made me create this patch is that in
> > barebox no special case for 370/XP was active. The result was that a
> > device that could be operated at 60 MHz only got a clock of 11 MHz and
> > the driver assumed it configured 41.666 MHz. I didn't check if the same
> > can happen in the opposite direction (or if there are other important
> > incompatibilities) but still I'd say this is a bug with my patch being
> > the obvious fix.
> 
> I just found this patch in an old branch and wonder what do to with it.
> It still applies fine at least.
> 
> (If the original patch already disappeared from your inbox, it can be
> found at https://lore.kernel.org/r/20161207152109.17545-1-uwe@kleine-koenig.org/ )

If you remove "marvell,orion-spi" you are going to break running a new
DT blob on an old kernel.

The compatible list is supposed to be most specific to most generic in
order. So "marvell,orion-spi" is the fall back option for Armada, you
don't get all the features, but it should work at a basic level. And
in this case, barebox did its best, it gave you a working but
unexpectedly slow bus. If you take away "marvell,orion-spi", and there
is no support for "marvell,armada-xp-spi" and "marvell,armada-xp-spi"
in barebox, does that not then mean there is no SPI support at all?

   Andrew