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