Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Mickey Rachamim <mickeyr@marvell.com>
To:     Vadym Kochan <vadym.kochan@plvision.eu>,
        Andrew Lunn <andrew@lunn.ch>
CC:     Jonathan McDowell <noodles@earth.li>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Jiri Pirko <jiri@mellanox.com>,
        Ido Schimmel <idosch@mellanox.com>,
        Oleksandr Mazur <oleksandr.mazur@plvision.eu>,
        Serhiy Boiko <serhiy.boiko@plvision.eu>,
        Serhiy Pshyk <serhiy.pshyk@plvision.eu>,
        Volodymyr Mytnyk <volodymyr.mytnyk@plvision.eu>,
        Taras Chornyi <taras.chornyi@plvision.eu>,
        Andrii Savka <andrii.savka@plvision.eu>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Andy Shevchenko <andy.shevchenko@gmail.com>
Subject: RE: [EXT] Re: [net-next v4 1/6] net: marvell: prestera: Add driver
 for Prestera family ASIC devices
Thread-Topic: [EXT] Re: [net-next v4 1/6] net: marvell: prestera: Add driver
 for Prestera family ASIC devices
Thread-Index: AQHWcj2Z7ghqmC8NyEeNZq3OYcL+5alAtCGAgAAHRVA=
Date:   Thu, 20 Aug 2020 10:00:21 +0000
Message-ID: <BN6PR18MB1587EB225C6B80BF35A44EBFBA5A0@BN6PR18MB1587.namprd18.prod.outlook.com>
References: <20200727122242.32337-1-vadym.kochan@plvision.eu>
 <20200727122242.32337-2-vadym.kochan@plvision.eu>
 <20200813080322.GH21409@earth.li> <20200814082054.GD17795@plvision.eu>
 <20200814120536.GA26106@earth.li> <20200814122744.GF17795@plvision.eu>
 <20200814131815.GA2238071@lunn.ch> <20200820083131.GA28129@plvision.eu>
In-Reply-To: <20200820083131.GA28129@plvision.eu>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 42e979da-23ae-4444-7647-08d844efd9c1
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Aug 2020 10:00:21.1963
 (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: EYTFGwK/ZIsHaMrQWsS8wmq7LBFGC3QK9UJ69Em+fODvp+xLUdgk4GQirWcC/u09SbUgMnuqReNZ9Xw8mqRiZQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR18MB2721
X-OriginatorOrg: marvell.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235,18.0.687
 definitions=2020-08-20_02:2020-08-19,2020-08-20 signatures=0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

External Email

Hi Andrew,=20
My name is Mickey and I'm the SW group manager in Marvell working on the Sw=
itchdev drivers for the Prestera Switching family.
In addition to Vadym's reply, please have my comments inline;

On Fri, Aug 14, 2020 at 03:18:15PM +0200, Andrew Lunn wrote:
> > > > > Currently
> > > > >=20
> > > > >     compatible =3D "marvell,prestera"
> > > > >=20
> > > > > is used as default, so may be
> > > > >=20
> > > > > you mean to support few matching including particular silicon too=
, like ?
> > > > >=20
> > > > >=20
> > > > >     compatible =3D "marvell,prestera"
> > > > >     compatible =3D "marvell,prestera-ac3x"
> > > > >=20
> > > > > Would you please give an example ?
> > > >=20
> > > > AFAICT "Prestera" is the general name for the Marvell=20
> > > > enterprise/data-centre silicon, comparable to the "LinkStreet"
> > > > designation for their lower end switching. The mv88e* drivers do=20
> > > > not mention LinkStreet in their compatible strings at all,=20
> > > > choosing instead to refer to chip IDs (I see mv88e6085, mv88e6190 +=
 mv88e6250).
> > > >=20
> > > > I do not have enough familiarity with the Prestera range to be=20
> > > > able to tell what commonality there is between the different=20
> > > > versions (it appears you need an NDA to get hold of the=20
> > > > programming references), but even just looking at your driver and=20
> > > > the vendor code for the BobCat it seems that AlleyCat3 uses an=20
> > > > extended DSA header format, and requires a firmware with message=20
> > > > based access, in comparison to the BobCat which uses register pokin=
g.
> > > >=20
> > > > Based on that I'd recommend not using the bare "marvell,prestera"
> > > > compatible string, but instead something more specific.
> > > > "marvell,prestera-ac3x" seems like a suitable choice, assuming=20
> > > > that's how these chips are named/generally referred to.
> > > >=20
> > > > Also I'd expand your Kconfig information to actually include=20
> > > > "Marvell Prestera 98DX326x" as that's the only supported chip range=
 at present.
> > > >=20
> > >=20
> > > Yes, Prestera covers more range of devices. But it is planning to=20
> > > cover other devices too, and currently there is no device-specific=20
> > > DTS properties which are used in this version, but only the generic=20
> > > one - since only the MAC address node.
> > >=20
> > > I mean that if there will be other Prestera devices supported then=20
> > > it will require to extend the DTS matching string in the driver just=
=20
> > > to support the same generic DTS properties for new device.
> > >=20
> > > Anyway I will rise and discuss this question.
> >=20
> > Hi Vadym
> >=20
> > Lets start with how mv88e6xxx does this. The switches have ID=20
> > registers. Once you have read the ID registers, you know what device=20
> > you have, and you can select device specific code as needed. However,=20
> > these ID registers are in three different locations, depending on the=20
> > chip family. So the compatible string is all about where to read the=20
> > ID from, not about what specific chip is. So most device tree bindings=
=20
> > say "marvell,mv88e6085", but the 6390 family use "marvell,mv88e6190"
> > for example.
> >=20
> > This naming scheme is actually odd compared to others. And that=20
> > oddness causes confusion. But it avoids a few problems. If you have=20
> > per chip compatible strings, what do you do when it conflicts with the=
=20
> > ID registers. If from day 1 you validate the compatible string against=
=20
> > the ID register and fail the probe if it is incorrect, you are O.K.=20
> > But if you decide to add this validation later, you are going to find=20
> > a number of device tree blobs which have the wrong compatible string.=20
> > Do you fail the probe on boards which have worked?
> >=20
> > So what to do with this driver?
> >=20
> > Does the prestera have ID registers? Are you using them in the driver?
> >=20
>=20
> ASIC device specific handling is serviced by the firmware, current driver=
's logic does not have PP specific code and relies on the FW ABI which is P=
P-generic, and it looks like this how it should work for boards with other =
ASICs, of course these boards should follow the Marvell's Switchdev board d=
esign.
>=20

All Marvell Prestera (DX) devices are all based on CPSS SDK. This is one SD=
K=20
and one build procedure that enables the Prestera driver to support all dev=
ices.=20
This unified support enables us (and our customers) to have one SW=20
implementation that will support variety of Prestera devices in same build/=
real-time=20
execution.=20
This approach also lead us with the implementation of the Prestera Switchde=
v drivers.
As having detailed familiarity (20Y) with Marvell Prestera old/current/futu=
re devices -=20
this approach will be kept strictly also on the future.

> > Marvell is not particularly good at backwards compatibility. Does your=
=20
> > compatible string give you enough wiggle room you can easily introduce=
=20
> > another compatible string in order to find the ID registers when they=20
> > move?
> >=20
> > 	Andrew