2022-08-22 at 11:10, luca.ceresoli@xxxxxxxxxxx wrote:
> From: Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx>
>
> The sequence of sections is a bit confusing here:
>
> * we list the mux locking scheme for existing drivers before introducing
> what mux locking schemes are
> * we list the caveats for each locking scheme (which are tricky) before
> the example of the simple use case
>
> Restructure it entirely with the following logic:
>
> * Intro ("I2C muxes and complex topologies")
> * Locking
> - mux-locked
> - example
> - caveats
> - parent-locked
> - example
> - caveats
> * Complex examples
> * Mux type of existing device drivers
>
> While there, also apply some other improvements:
>
> * convert the caveat list from a table (with only one column carrying
> content) to a bullet list.
I want to be able to refer to a specific caveat if/when someone has
questions, so I prefer to have the caveats "named". Not that this is
very frequent, but if we do remove the tags now I'm sure I'm going
to need them a few minutes later...
> * add a small introductory text to bridge the gap from listing the use
> cases to telling about the hardware components to handle them and then
> the device drivers that implement those.
> * make empty lines usage more uniform
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx>
>
> ---
>
> Changed in v2: none
> ---
> Documentation/i2c/i2c-topology.rst | 206 +++++++++++++++--------------
> 1 file changed, 109 insertions(+), 97 deletions(-)
>
> diff --git a/Documentation/i2c/i2c-topology.rst b/Documentation/i2c/i2c-topology.rst
> index 1b11535c8946..6f2da7f386fd 100644
> --- a/Documentation/i2c/i2c-topology.rst
> +++ b/Documentation/i2c/i2c-topology.rst
> @@ -16,7 +16,10 @@ Some example use cases are:
> from the I2C bus, at least most of the time, and sits behind a gate
> that has to be operated before the device can be accessed.
>
> -These constructs are represented as I2C adapter trees by Linux, where
> +Several types of hardware components such as I2C muxes, I2C gates and I2C
> +arbitrators allow to handle such needs.
> +
> +These components are represented as I2C adapter trees by Linux, where
> each adapter has a parent adapter (except the root adapter) and zero or
> more child adapters. The root adapter is the actual adapter that issues
> I2C transfers, and all adapters with a parent are part of an "i2c-mux"
> @@ -34,46 +37,7 @@ Locking
> =======
>
> There are two variants of locking available to I2C muxes, they can be
> -mux-locked or parent-locked muxes. As is evident from below, it can be
> -useful to know if a mux is mux-locked or if it is parent-locked. The
> -following list was correct at the time of writing:
> -
> -In drivers/i2c/muxes/:
> -
> -====================== =============================================
> -i2c-arb-gpio-challenge Parent-locked
> -i2c-mux-gpio Normally parent-locked, mux-locked iff
> - all involved gpio pins are controlled by the
> - same I2C root adapter that they mux.
> -i2c-mux-gpmux Normally parent-locked, mux-locked iff
> - specified in device-tree.
> -i2c-mux-ltc4306 Mux-locked
> -i2c-mux-mlxcpld Parent-locked
> -i2c-mux-pca9541 Parent-locked
> -i2c-mux-pca954x Parent-locked
> -i2c-mux-pinctrl Normally parent-locked, mux-locked iff
> - all involved pinctrl devices are controlled
> - by the same I2C root adapter that they mux.
> -i2c-mux-reg Parent-locked
> -====================== =============================================
> -
> -In drivers/iio/:
> -
> -====================== =============================================
> -gyro/mpu3050 Mux-locked
> -imu/inv_mpu6050/ Mux-locked
> -====================== =============================================
> -
> -In drivers/media/:
> -
> -======================= =============================================
> -dvb-frontends/lgdt3306a Mux-locked
> -dvb-frontends/m88ds3103 Parent-locked
> -dvb-frontends/rtl2830 Parent-locked
> -dvb-frontends/rtl2832 Mux-locked
> -dvb-frontends/si2168 Mux-locked
> -usb/cx231xx/ Parent-locked
> -======================= =============================================
> +mux-locked or parent-locked muxes.
>
>
> Mux-locked muxes
> @@ -88,40 +52,8 @@ full transaction, unrelated I2C transfers may interleave the different
> stages of the transaction. This has the benefit that the mux driver
> may be easier and cleaner to implement, but it has some caveats.
>
> -==== =====================================================================
> -ML1. If you build a topology with a mux-locked mux being the parent
> - of a parent-locked mux, this might break the expectation from the
> - parent-locked mux that the root adapter is locked during the
> - transaction.
> -
> -ML2. It is not safe to build arbitrary topologies with two (or more)
> - mux-locked muxes that are not siblings, when there are address
> - collisions between the devices on the child adapters of these
> - non-sibling muxes.
> -
> - I.e. the select-transfer-deselect transaction targeting e.g. device
> - address 0x42 behind mux-one may be interleaved with a similar
> - operation targeting device address 0x42 behind mux-two. The
> - intension with such a topology would in this hypothetical example
> - be that mux-one and mux-two should not be selected simultaneously,
> - but mux-locked muxes do not guarantee that in all topologies.
> -
> -ML3. A mux-locked mux cannot be used by a driver for auto-closing
> - gates/muxes, i.e. something that closes automatically after a given
> - number (one, in most cases) of I2C transfers. Unrelated I2C transfers
> - may creep in and close prematurely.
> -
> -ML4. If any non-I2C operation in the mux driver changes the I2C mux state,
> - the driver has to lock the root adapter during that operation.
> - Otherwise garbage may appear on the bus as seen from devices
> - behind the mux, when an unrelated I2C transfer is in flight during
> - the non-I2C mux-changing operation.
> -==== =====================================================================
> -
> -
> Mux-locked Example
> -------------------
> -
> +~~~~~~~~~~~~~~~~~~
>
> ::
>
> @@ -152,6 +84,39 @@ This means that accesses to D2 are lockout out for the full duration
> of the entire operation. But accesses to D3 are possibly interleaved
> at any point.
>
> +Mux-locked caveats
> +~~~~~~~~~~~~~~~~~~
> +
> +When using a mux-locked mux, be aware of the following restrictions:
> +
> +* If you build a topology with a mux-locked mux being the parent
> + of a parent-locked mux, this might break the expectation from the
> + parent-locked mux that the root adapter is locked during the
> + transaction.
> +
> +* It is not safe to build arbitrary topologies with two (or more)
> + mux-locked muxes that are not siblings, when there are address
> + collisions between the devices on the child adapters of these
> + non-sibling muxes.
> +
> + I.e. the select-transfer-deselect transaction targeting e.g. device
> + address 0x42 behind mux-one may be interleaved with a similar
> + operation targeting device address 0x42 behind mux-two. The
> + intension with such a topology would in this hypothetical example
> + be that mux-one and mux-two should not be selected simultaneously,
> + but mux-locked muxes do not guarantee that in all topologies.
> +
> +* A mux-locked mux cannot be used by a driver for auto-closing
> + gates/muxes, i.e. something that closes automatically after a given
> + number (one, in most cases) of I2C transfers. Unrelated I2C transfers
> + may creep in and close prematurely.
> +
> +* If any non-I2C operation in the mux driver changes the I2C mux state,
> + the driver has to lock the root adapter during that operation.
> + Otherwise garbage may appear on the bus as seen from devices
> + behind the mux, when an unrelated I2C transfer is in flight during
> + the non-I2C mux-changing operation.
> +
>
> Parent-locked muxes
> -------------------
> @@ -160,28 +125,10 @@ Parent-locked muxes lock the parent adapter during the full select-
> transfer-deselect transaction. The implication is that the mux driver
> has to ensure that any and all I2C transfers through that parent
> adapter during the transaction are unlocked I2C transfers (using e.g.
> -__i2c_transfer), or a deadlock will follow. There are a couple of
> -caveats.
> -
> -==== ====================================================================
> -PL1. If you build a topology with a parent-locked mux being the child
> - of another mux, this might break a possible assumption from the
> - child mux that the root adapter is unused between its select op
> - and the actual transfer (e.g. if the child mux is auto-closing
> - and the parent mux issues I2C transfers as part of its select).
> - This is especially the case if the parent mux is mux-locked, but
> - it may also happen if the parent mux is parent-locked.
> -
> -PL2. If select/deselect calls out to other subsystems such as gpio,
> - pinctrl, regmap or iio, it is essential that any I2C transfers
> - caused by these subsystems are unlocked. This can be convoluted to
> - accomplish, maybe even impossible if an acceptably clean solution
> - is sought.
> -==== ====================================================================
> -
> +__i2c_transfer), or a deadlock will follow.
>
> Parent-locked Example
> ----------------------
> +~~~~~~~~~~~~~~~~~~~~~
>
> ::
>
> @@ -211,10 +158,29 @@ When there is an access to D1, this happens:
> 9. M1 unlocks its parent adapter.
> 10. M1 unlocks muxes on its parent.
>
> -
> This means that accesses to both D2 and D3 are locked out for the full
> duration of the entire operation.
>
> +Parent-locked Caveats
> +~~~~~~~~~~~~~~~~~~~~~
> +
> +When using a parent-locked mux, be aware of the following restrictions:
> +
> +* If you build a topology with a parent-locked mux being the child
> + of another mux, this might break a possible assumption from the
> + child mux that the root adapter is unused between its select op
> + and the actual transfer (e.g. if the child mux is auto-closing
> + and the parent mux issues I2C transfers as part of its select).
> + This is especially the case if the parent mux is mux-locked, but
> + it may also happen if the parent mux is parent-locked.
> +
> +* If select/deselect calls out to other subsystems such as gpio,
> + pinctrl, regmap or iio, it is essential that any I2C transfers
> + caused by these subsystems are unlocked. This can be convoluted to
> + accomplish, maybe even impossible if an acceptably clean solution
> + is sought.
> +
> +
>
Three empty lines is excessive and inconsistent with the other two
===-headers.
Cheers,
Peter
> Complex Examples
> ================
> @@ -260,8 +226,10 @@ This is a good topology::
> When device D1 is accessed, accesses to D2 are locked out for the
> full duration of the operation (muxes on the top child adapter of M1
> are locked). But accesses to D3 and D4 are possibly interleaved at
> -any point. Accesses to D3 locks out D1 and D2, but accesses to D4
> -are still possibly interleaved.
> +any point.
> +
> +Accesses to D3 locks out D1 and D2, but accesses to D4 are still possibly
> +interleaved.
>
>
> Mux-locked mux as parent of parent-locked mux
> @@ -393,3 +361,47 @@ This is a good topology::
> When D1 or D2 are accessed, accesses to D3 and D4 are locked out while
> accesses to D5 may interleave. When D3 or D4 are accessed, accesses to
> all other devices are locked out.
> +
> +
> +Mux type of existing device drivers
> +===================================
> +
> +Whether a device is mux-locked or parent-locked depends on its
> +implementation. The following list was correct at the time of writing:
> +
> +In drivers/i2c/muxes/:
> +
> +====================== =============================================
> +i2c-arb-gpio-challenge Parent-locked
> +i2c-mux-gpio Normally parent-locked, mux-locked iff
> + all involved gpio pins are controlled by the
> + same I2C root adapter that they mux.
> +i2c-mux-gpmux Normally parent-locked, mux-locked iff
> + specified in device-tree.
> +i2c-mux-ltc4306 Mux-locked
> +i2c-mux-mlxcpld Parent-locked
> +i2c-mux-pca9541 Parent-locked
> +i2c-mux-pca954x Parent-locked
> +i2c-mux-pinctrl Normally parent-locked, mux-locked iff
> + all involved pinctrl devices are controlled
> + by the same I2C root adapter that they mux.
> +i2c-mux-reg Parent-locked
> +====================== =============================================
> +
> +In drivers/iio/:
> +
> +====================== =============================================
> +gyro/mpu3050 Mux-locked
> +imu/inv_mpu6050/ Mux-locked
> +====================== =============================================
> +
> +In drivers/media/:
> +
> +======================= =============================================
> +dvb-frontends/lgdt3306a Mux-locked
> +dvb-frontends/m88ds3103 Parent-locked
> +dvb-frontends/rtl2830 Parent-locked
> +dvb-frontends/rtl2832 Mux-locked
> +dvb-frontends/si2168 Mux-locked
> +usb/cx231xx/ Parent-locked
> +======================= =============================================
