> On Mon, Apr 29, 2024 at 02:47:23PM +0000, Witold Sadowski wrote: > > > -------------------------------------------------------------------- > > > -- On Wed, Apr 17, 2024 at 06:13:49PM -0700, Witold Sadowski wrote: > > > > Add new bindings for v2 Marvell xSPI overlay: > > > > mrvl,xspi-nor compatible string > > > > New compatible string to distinguish between orginal and modified > > > > xSPI block > > > > > > > > PHY configuration registers > > > > Allow to change orginal xSPI PHY configuration values. If not set, > > > > and Marvell overlay is enabled, safe defaults will be written into > > > > xSPI PHY > > > > > > > > Optional base for xfer register set Additional reg field to > > > > allocate xSPI Marvell overlay XFER block > > > > > > > > Signed-off-by: Witold Sadowski <wsadowski@xxxxxxxxxxx> > > > > --- > > > > .../devicetree/bindings/spi/cdns,xspi.yaml | 92 > ++++++++++++++++++- > > > > 1 file changed, 91 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml > > > > b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml > > > > index eb0f92468185..0e608245b136 100644 > > > > --- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml > > > > +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml > > > > @@ -20,23 +20,82 @@ allOf: > > > > > > > > properties: > > > > compatible: > > > > - const: cdns,xspi-nor > > > > + oneOf: > > > > + - description: Vanilla Cadence xSPI controller > > > > + items: > > > > + - const: cdns,xspi-nor > > > > > > The "items: isn't required here is it? Can't you just have > > > oneOf: > > > - description: Vanilla Cadence xSPI controller > > > const: cdns,xspi-nor > > > - description: Cadence xSPI controller with v2 Marvell overlay > > > const: mrvl,xspi-nor > > > if you don't want to use an enum? > > > > It works without items, but I will try also with enums. > > > > > > > > > + - description: Cadence xSPI controller with v2 Marvell > overlay > > > > + items: > > > > + - const: mrvl,xspi-nor > > > > > > > > > "mrvl" is deprecated, please use "marvell". You're also missing a > > > soc- specific compatible here, I doubt there's only going to be one > > > device from marvell with an xspi controller ever. > > > > The intention is to add overlay on top of existing IP block to gain > > some More features from it. So if there will be different SoC with > > same xSPI IP, we can simply use that property, as internal SoC structure > will be the same. > > On the other hand, if there will be used different IP to handle SPI > > operations It should use different driver. Also, I do not expect that > > new version of the Overlay will be developed to handle different IP. > > I'm struggling to understand what you mean here by "overlay". Ordinarily > I'd expect someone to meant a dt-overlay, but you're talking about IP > blocks, so this sounds like hardware modifications. > I am also a bit confused by the claim that the "internal SoC structure > will be the same". Usually different SoCs have different internal > structures, even when they re-use IP cores. If they have the same internal > structure then they're not really different SoCs, just different packages! > I think what you're saying here is that you intend using the "mrvl,xspi- > nor" compatible for multiple SoCs that all contain the same modified > versions of the Cadence IP, not different packages for the same SoC? Yes, by HW overlay I meant actual HW modification. I called it overlay, as it is not modifying xSPI block itself, but it is rather build around it. As I don't have better word for it now, I will continue to use it. With that approach we can still have full functionality of xSPI block (like memory transfers), and additional features(like SPI full-duplex operations). Regarding "internal SoC structure" - I meant physical parameters of silicon, Signal propagation times inside block etc. That won't be changed if we have Two different SoC, bud made in same technology. > > Confusing wording aside, using the same generic compatible for different > SoCs is what I trying to avoid. I don't mind there being a fallback > compatible that's generic, but I want to see specific compatibles here for > the individual SoCs. > > If you did actually mean that only the packaging is different between the > devices, then I don't think you need specific compatibles for each > different package, but you should have one for the SoC itself IMO. We can have SoC A, B with common xSPI block, and both of them can share Same dtb node with compatible property "marvell,cn10k,xspi-nor" for example. I don't think it will be beneficial to have different compatible name for each different SoC, for example "marvell,t98,xspi-nor", if all other parts will be the same. Or am I not correct? > > Cheers, > Conor. Regards Witek
