Hi Andrew, Quoting Andrew Lunn (2020-06-20 17:10:01) > On Fri, Jun 19, 2020 at 02:22:57PM +0200, Antoine Tenart wrote: > > From: Quentin Schulz <quentin.sch...@bootlin.com> > > > > This patch adds the first parts of the 1588 support in the MSCC PHY, > > with registers definition and the 1588 block initialization. > > > > Those PHYs are distributed in hardware packages containing multiple > > times the PHY. The VSC8584 for example is composed of 4 PHYs. With > > hardware packages, parts of the logic is usually common and one of the > > PHY has to be used for some parts of the initialization. Following this > > logic, the 1588 blocks of those PHYs are shared between two PHYs and > > accessing the registers has to be done using the "base" PHY of the > > group. This is handled thanks to helpers in the PTP code (and locks). > > We also need the MDIO bus lock while performing a single read or write > > to the 1588 registers as the read/write are composed of multiple MDIO > > transactions (and we don't want other threads updating the page). > > Locking sounds complex. I assume LOCKDEP was your friend in getting > this correct and deadlock free.
I agree, locking is not straight forward. But it's actually not that complex: - The MDIO bus lock is used for all TS/PHC register access. - There is one lock for PHC operations and one for timestamping operations. The two are never used in the same function. We could use the same lock; introducing more waiting. - There is one shared lock for GPIO operations. It is only used in PHC functions, in two places. And I realized I can remove the locks from vsc8584_ptp_init, as PHC/TS helpers are not registered until the PHY is initialized. Thanks, Antoine -- Antoine Ténart, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
