Received-SPF: pass (google.com: domain of linux-kernel+bounces-163118-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: Witold Sadowski <wsadowski@marvell.com>
To: Conor Dooley <conor@kernel.org>
CC: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-spi@vger.kernel.org" <linux-spi@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "broonie@kernel.org" <broonie@kernel.org>,
        "robh@kernel.org"
	<robh@kernel.org>,
        "krzysztof.kozlowski+dt@linaro.org"
	<krzysztof.kozlowski+dt@linaro.org>,
        "conor+dt@kernel.org"
	<conor+dt@kernel.org>,
        "pthombar@cadence.com" <pthombar@cadence.com>
Subject: RE: [EXTERNAL] Re: [PATCH v3 2/5] spi: cadence: Add MRVL overlay
 bindings documentation for Cadence XSPI
Thread-Topic: [EXTERNAL] Re: [PATCH v3 2/5] spi: cadence: Add MRVL overlay
 bindings documentation for Cadence XSPI
Thread-Index: AQHakS28W/KzCqcstE+2q5E6FRh8o7FuNt8AgBEr6pCAAHSOAIAAFNPg
Date: Mon, 29 Apr 2024 22:59:37 +0000
Message-ID: 
 <CO6PR18MB40985FCD4A28467DA36E07C0B01B2@CO6PR18MB4098.namprd18.prod.outlook.com>
References: <20240329194849.25554-1-wsadowski@marvell.com>
 <20240418011353.1764672-1-wsadowski@marvell.com>
 <20240418011353.1764672-3-wsadowski@marvell.com>
 <20240418-sacrament-cornea-fd6fd569827e@spud>
 <CO6PR18MB4098C815325699975B1BD794B01B2@CO6PR18MB4098.namprd18.prod.outlook.com>
 <20240429-quickstep-hypnotic-5b8d1fbeb920@spud>
In-Reply-To: <20240429-quickstep-hypnotic-5b8d1fbeb920@spud>
Accept-Language: en-US, pl-PL
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?us-ascii?Q?uX3MDs/V83NRtXTIy0uRpp50o+vMVkDzjXh/EDzIqzjm/qonfo5oNnW+wjZ2?=
 =?us-ascii?Q?nA6inl+wzJS3VTPFHwBPxHR7G8Fb+F9iyxrAVw3LQXmrGz+Vac2XLKuHZLRD?=
 =?us-ascii?Q?zudwvp+MNQKHMV71/5Rk8ibVzGGNtxh20HAJ7q/LaKN30VfLXZtARjAHKOGX?=
 =?us-ascii?Q?mrJ0eJ/3sYSJJ2kqMlX1lmkepT1+XRSRSYbWBFeQNiNutyYRV5CSxQgeNw2e?=
 =?us-ascii?Q?CJ6yzqTbuxxZDjAFxbwA9GZujuclNl0vBx2nkuTOTMZDeg0VaNSEGtvUKmr3?=
 =?us-ascii?Q?0TamZWiGCr639ZcalUXYBuljRAfcMP17tT9TDYckoJsaIXNPJWBew2Pd2sQ3?=
 =?us-ascii?Q?s6HItV62Hf4O+kHgvcL5wQn8173CuG8g9V4zOgKDlsaUdQw94XsYPgCKaZzJ?=
 =?us-ascii?Q?JLa7+OzmuEDHC4dE05Zvd9a2Dh3q6b7TEFqz6GCIi5ouNtaAqihm/AmNGT7A?=
 =?us-ascii?Q?b/OwL2QfsI1+vJV1a/jiO6oGhaKqAVFG07yQfJIUy8a0jnckPbQMrQa/w72Y?=
 =?us-ascii?Q?twBDhMan3LcqjFxnIf+ism+RZYfjtjsiZRUStycPW45f/fpt4UlpMpBzWBNz?=
 =?us-ascii?Q?EzOcEllEfINj94vCdDGQkKFqwbhkPhpWI5fpRdlQn4caVJxqV8jJ/ChzVI8I?=
 =?us-ascii?Q?NrA+4AgGfDEjU4FeSc22j/zQHIjsg3LkdNp85COdrIw/0G92r2xQt5MCo9TT?=
 =?us-ascii?Q?m+bTB0OW2m3tXQyIvOvr0BHK35DLjunaW7GDGQMwjUZMzN8IjiuvRWdAlFjs?=
 =?us-ascii?Q?kBDKzgU2FduFzzrP4w40y3g0sIROzbRsc2ClxQerfUBqSruB3XnVKJkQ9Wb5?=
 =?us-ascii?Q?2fL8SEOgEHNLf/1mU2RkYq12mvd+pwOqigmkeoZ8+5N40Fs2m7CwLQw0W5IQ?=
 =?us-ascii?Q?731bG0lISq+Qn8UKZ7bNeN/hWDY7Rfy7WEYE8AzAdbOeeByaOTJNOeMmXMXq?=
 =?us-ascii?Q?xcnWE7JUFp7ebrCo9qy0V76QdlTrM80LwegYiwxDlkhoMdtk0bR9AK9UYZFr?=
 =?us-ascii?Q?36WeChOSaepUyJ5qfLel+BYgBwvKjbm0YTriESYhPD946byPWyQgX0S8D635?=
 =?us-ascii?Q?/wH1LmxKnxJD1D4jYgYhZQpHbAV2vaqMQcr7sOfpk9InDhRBJHIW0JvcOlnI?=
 =?us-ascii?Q?FOguMsolPX3hxULf/EkR0gcSPxprFaoEFED/PcdeAzSQFlDpiH1wX/q6MNot?=
 =?us-ascii?Q?cZ+pVGsevkQ+msn89crJ29ntxT9RqqCMj+aPOpmadJ1RxxAwFd3oFH9JotnI?=
 =?us-ascii?Q?SRxs9zFTw1F6b823Q8NRUQw1v8su+zWabV+GmM6XjRbWeQA6LuadUcIIr1HY?=
 =?us-ascii?Q?5Psy4GoDsj2qSXsqFLarWSBlKrb6UatQmA+s5Tl8dzTDoDWoR1Xvx+1hkhTa?=
 =?us-ascii?Q?nYFnst25yCGvaL1nZSvGkweQI/U7tc/Q1SNVxDbkl4ENwSBplk2vZkphqtyJ?=
 =?us-ascii?Q?HeEqqsPSGo6JGgvf0LmicYyIlDtwkxJ7sNrWvhZpV3OEpqVmY3qE6d4zmSlw?=
 =?us-ascii?Q?XdBHNzgsB90PVYyan469/4NN1WmFOljc/hokR6dlAwpI2b12/8hpg2nl5i3v?=
 =?us-ascii?Q?8+rFoAH6JCQcTn5KGpVx1bzvXiqxNWlQwh4WT6Ac?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CO6PR18MB4098.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 739bcf36-3185-46b9-bea6-08dc68a00b43
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Apr 2024 22:59:37.2250
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: nAuyEF8Mqmi6CGYQWxhz+nkHohgDcDidM4kk0BiQ1LD7sEkDU1MkzwwdNJSkU/t/JER7Jr6n61gTK3yUFnq0RQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV8PR18MB5677
X-Proofpoint-GUID: 0TJ6Gpnw6A9Y5nC6qwg7TX2kFhm5K65A
X-Proofpoint-ORIG-GUID: 0TJ6Gpnw6A9Y5nC6qwg7TX2kFhm5K65A
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1011,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-04-29_20,2024-04-29_01,2023-05-22_02

> 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@marvell.com>
> > > > ---
> > > >  .../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 structur=
e
> 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.
>=20
> 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 interna=
l
> 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.=20

>=20
> 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 fo=
r
> the individual SoCs.
>=20
> 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?

>=20
> Cheers,
> Conor.

Regards
Witek