[PATCH 0/9] docs: i2c: rework I2C documentation, part II

[PATCH 0/9] docs: i2c: rework I2C documentation, part II

[luca.ceresoli]

From: Luca Ceresoli <luca.ceresoli@bootlin.com>

Back in January 2020 I sent a first batch of patches to improve I2C
documentation, titled "docs: i2c: rework I2C documentation, part I" [0] and
I wrote "I will continue to cover the rest of the sections later".

After about 2.5y, here I am with part II!

This series contains assorted improvements to the "Introduction"
section. The plan is to cover the remaining sections "later", but hopefully
sooner than in 2.5 more years.

[0] https://lore.kernel.org/linux-i2c/20200123135103.20540-1-luca@lucaceresoli.net/

Luca

Luca Ceresoli (9):
  docs: i2c: i2c-protocol: update introductory paragraph
  docs: i2c: i2c-protocol,smbus-protocol: remove nonsense words
  docs: i2c: i2c-protocol: remove unused legend items
  docs: i2c: smbus-protocol: improve DataLow/DataHigh definition
  docs: i2c: instantiating-devices: add syntax coloring to dts and C
    blocks
  docs: i2c: i2c-topology: fix incorrect heading
  docs: i2c: i2c-topology: reorder sections more logically
  docs: i2c: i2c-sysfs: improve wording
  docs: i2c: i2c-sysfs: fix hyperlinks

 Documentation/i2c/i2c-protocol.rst          |  11 +-
 Documentation/i2c/i2c-sysfs.rst             |  24 +--
 Documentation/i2c/i2c-topology.rst          | 209 ++++++++++----------
 Documentation/i2c/instantiating-devices.rst |  16 +-
 Documentation/i2c/smbus-protocol.rst        |   6 +-
 5 files changed, 137 insertions(+), 129 deletions(-)

-- 
2.34.1

[PATCH 1/9] docs: i2c: i2c-protocol: update introductory paragraph

[luca.ceresoli]

From: Luca Ceresoli <luca.ceresoli@bootlin.com>

This sentence dates back to the pre-git era and it does not look very
prefessional... As there is no clear definition of "finished", and given
this page is already a pretty good overview, not to mention it is not the
kernel responsibility to document the protocol in detail, let's update the
text accordingly.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 Documentation/i2c/i2c-protocol.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/i2c/i2c-protocol.rst b/Documentation/i2c/i2c-protocol.rst
index b2092f8f815d..9ecf03f5de33 100644
--- a/Documentation/i2c/i2c-protocol.rst
+++ b/Documentation/i2c/i2c-protocol.rst
@@ -2,7 +2,8 @@
 The I2C Protocol
 ================
 
-This document describes the I2C protocol. Or will, when it is finished :-)
+This document is an overview of the basic I2C transactions and the kernel
+APIs to perform them.
 
 Key to symbols
 ==============
-- 
2.34.1

Re: [PATCH 1/9] docs: i2c: i2c-protocol: update introductory paragraph

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 557 bytes --]

On Mon, Aug 08, 2022 at 04:17:00PM +0200, luca.ceresoli@bootlin.com wrote:
> From: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> This sentence dates back to the pre-git era and it does not look very
> prefessional... As there is no clear definition of "finished", and given
> this page is already a pretty good overview, not to mention it is not the
> kernel responsibility to document the protocol in detail, let's update the
> text accordingly.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Applied to for-current, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/9] docs: i2c: i2c-protocol,smbus-protocol: remove nonsense words

[luca.ceresoli]

From: Luca Ceresoli <luca.ceresoli@bootlin.com>

"as usual" does not mean much here, especially as these are introductory
sections and 10-bit addressing hasn't been introduced yet.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 Documentation/i2c/i2c-protocol.rst   | 2 +-
 Documentation/i2c/smbus-protocol.rst | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/i2c/i2c-protocol.rst b/Documentation/i2c/i2c-protocol.rst
index 9ecf03f5de33..7ffada1f3335 100644
--- a/Documentation/i2c/i2c-protocol.rst
+++ b/Documentation/i2c/i2c-protocol.rst
@@ -13,7 +13,7 @@ S               Start condition
 P               Stop condition
 Rd/Wr (1 bit)   Read/Write bit. Rd equals 1, Wr equals 0.
 A, NA (1 bit)   Acknowledge (ACK) and Not Acknowledge (NACK) bit
-Addr  (7 bits)  I2C 7 bit address. Note that this can be expanded as usual to
+Addr  (7 bits)  I2C 7 bit address. Note that this can be expanded to
                 get a 10 bit I2C address.
 Comm  (8 bits)  Command byte, a data byte which often selects a register on
                 the device.
diff --git a/Documentation/i2c/smbus-protocol.rst b/Documentation/i2c/smbus-protocol.rst
index 00d8e17d0aca..55e209c7e520 100644
--- a/Documentation/i2c/smbus-protocol.rst
+++ b/Documentation/i2c/smbus-protocol.rst
@@ -41,7 +41,7 @@ Sr              Repeated start condition, used to switch from write to
 P               Stop condition
 Rd/Wr (1 bit)   Read/Write bit. Rd equals 1, Wr equals 0.
 A, NA (1 bit)   Acknowledge (ACK) and Not Acknowledge (NACK) bit
-Addr  (7 bits)  I2C 7 bit address. Note that this can be expanded as usual to
+Addr  (7 bits)  I2C 7 bit address. Note that this can be expanded to
                 get a 10 bit I2C address.
 Comm  (8 bits)  Command byte, a data byte which often selects a register on
                 the device.
-- 
2.34.1

Re: [PATCH 2/9] docs: i2c: i2c-protocol,smbus-protocol: remove nonsense words

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 370 bytes --]

On Mon, Aug 08, 2022 at 04:17:01PM +0200, luca.ceresoli@bootlin.com wrote:
> From: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> "as usual" does not mean much here, especially as these are introductory
> sections and 10-bit addressing hasn't been introduced yet.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Applied to for-current, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 3/9] docs: i2c: i2c-protocol: remove unused legend items

[luca.ceresoli]

From: Luca Ceresoli <luca.ceresoli@bootlin.com>

"Comm", "Count", "DataLow", "DataHigh" are not used in this section.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 Documentation/i2c/i2c-protocol.rst | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/Documentation/i2c/i2c-protocol.rst b/Documentation/i2c/i2c-protocol.rst
index 7ffada1f3335..df0febfe6410 100644
--- a/Documentation/i2c/i2c-protocol.rst
+++ b/Documentation/i2c/i2c-protocol.rst
@@ -15,11 +15,7 @@ Rd/Wr (1 bit)   Read/Write bit. Rd equals 1, Wr equals 0.
 A, NA (1 bit)   Acknowledge (ACK) and Not Acknowledge (NACK) bit
 Addr  (7 bits)  I2C 7 bit address. Note that this can be expanded to
                 get a 10 bit I2C address.
-Comm  (8 bits)  Command byte, a data byte which often selects a register on
-                the device.
-Data  (8 bits)  A plain data byte. Sometimes, I write DataLow, DataHigh
-                for 16 bit data.
-Count (8 bits)  A data byte containing the length of a block operation.
+Data  (8 bits)  A plain data byte.
 
 [..]            Data sent by I2C device, as opposed to data sent by the
                 host adapter.
-- 
2.34.1

Re: [PATCH 3/9] docs: i2c: i2c-protocol: remove unused legend items

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 304 bytes --]

On Mon, Aug 08, 2022 at 04:17:02PM +0200, luca.ceresoli@bootlin.com wrote:
> From: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> "Comm", "Count", "DataLow", "DataHigh" are not used in this section.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Applied to for-current, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 4/9] docs: i2c: smbus-protocol: improve DataLow/DataHigh definition

[luca.ceresoli]

From: Luca Ceresoli <luca.ceresoli@bootlin.com>

Use a more professional wording.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 Documentation/i2c/smbus-protocol.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/i2c/smbus-protocol.rst b/Documentation/i2c/smbus-protocol.rst
index 55e209c7e520..4942c4cad4ad 100644
--- a/Documentation/i2c/smbus-protocol.rst
+++ b/Documentation/i2c/smbus-protocol.rst
@@ -45,8 +45,8 @@ Addr  (7 bits)  I2C 7 bit address. Note that this can be expanded to
                 get a 10 bit I2C address.
 Comm  (8 bits)  Command byte, a data byte which often selects a register on
                 the device.
-Data  (8 bits)  A plain data byte. Sometimes, I write DataLow, DataHigh
-                for 16 bit data.
+Data  (8 bits)  A plain data byte. DataLow and DataHigh represent the low and
+                high byte of a 16 bit word.
 Count (8 bits)  A data byte containing the length of a block operation.
 
 [..]            Data sent by I2C device, as opposed to data sent by the host
-- 
2.34.1

Re: [PATCH 4/9] docs: i2c: smbus-protocol: improve DataLow/DataHigh definition

[Bagas Sanjaya]

On 8/8/22 21:17, luca.ceresoli@bootlin.com wrote:
> From: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> Use a more professional wording.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  Documentation/i2c/smbus-protocol.rst | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/i2c/smbus-protocol.rst b/Documentation/i2c/smbus-protocol.rst
> index 55e209c7e520..4942c4cad4ad 100644
> --- a/Documentation/i2c/smbus-protocol.rst
> +++ b/Documentation/i2c/smbus-protocol.rst
> @@ -45,8 +45,8 @@ Addr  (7 bits)  I2C 7 bit address. Note that this can be expanded to
>                  get a 10 bit I2C address.
>  Comm  (8 bits)  Command byte, a data byte which often selects a register on
>                  the device.
> -Data  (8 bits)  A plain data byte. Sometimes, I write DataLow, DataHigh
> -                for 16 bit data.
> +Data  (8 bits)  A plain data byte. DataLow and DataHigh represent the low and
> +                high byte of a 16 bit word.
>  Count (8 bits)  A data byte containing the length of a block operation.
>  
>  [..]            Data sent by I2C device, as opposed to data sent by the host

Looks better, thanks.

Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH 4/9] docs: i2c: smbus-protocol: improve DataLow/DataHigh definition

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 268 bytes --]

On Mon, Aug 08, 2022 at 04:17:03PM +0200, luca.ceresoli@bootlin.com wrote:
> From: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> Use a more professional wording.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Applied to for-current, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 5/9] docs: i2c: instantiating-devices: add syntax coloring to dts and C blocks

[luca.ceresoli]

From: Luca Ceresoli <luca.ceresoli@bootlin.com>

These blocks can be nicely coloured via Sphinx.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 Documentation/i2c/instantiating-devices.rst | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/Documentation/i2c/instantiating-devices.rst b/Documentation/i2c/instantiating-devices.rst
index 890c9360ce19..3ea056a95812 100644
--- a/Documentation/i2c/instantiating-devices.rst
+++ b/Documentation/i2c/instantiating-devices.rst
@@ -31,7 +31,9 @@ Declare the I2C devices via devicetree
 On platforms using devicetree, the declaration of I2C devices is done in
 subnodes of the master controller.
 
-Example::
+Example:
+
+.. code-block:: dts
 
 	i2c1: i2c@400a0000 {
 		/* ... master properties skipped ... */
@@ -71,7 +73,9 @@ code. Instantiating I2C devices via board files is done with an array of
 struct i2c_board_info which is registered by calling
 i2c_register_board_info().
 
-Example (from omap2 h4)::
+Example (from omap2 h4):
+
+.. code-block:: c
 
   static struct i2c_board_info h4_i2c_board_info[] __initdata = {
 	{
@@ -111,7 +115,9 @@ bus in advance, so the method 1 described above can't be used. Instead,
 you can instantiate your I2C devices explicitly. This is done by filling
 a struct i2c_board_info and calling i2c_new_client_device().
 
-Example (from the sfe4001 network driver)::
+Example (from the sfe4001 network driver):
+
+.. code-block:: c
 
   static struct i2c_board_info sfe4001_hwmon_info = {
 	I2C_BOARD_INFO("max6647", 0x4e),
@@ -136,7 +142,9 @@ it may have different addresses from one board to the next (manufacturer
 changing its design without notice). In this case, you can call
 i2c_new_scanned_device() instead of i2c_new_client_device().
 
-Example (from the nxp OHCI driver)::
+Example (from the nxp OHCI driver):
+
+.. code-block:: c
 
   static const unsigned short normal_i2c[] = { 0x2c, 0x2d, I2C_CLIENT_END };
 
-- 
2.34.1

Re: [PATCH 5/9] docs: i2c: instantiating-devices: add syntax coloring to dts and C blocks

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 283 bytes --]

On Mon, Aug 08, 2022 at 04:17:04PM +0200, luca.ceresoli@bootlin.com wrote:
> From: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> These blocks can be nicely coloured via Sphinx.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Applied to for-current, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 6/9] docs: i2c: i2c-topology: fix incorrect heading

[luca.ceresoli]

From: Luca Ceresoli <luca.ceresoli@bootlin.com>

"Etc" here was never meant to be a heading, it became one while converting
to ReST.

It would be easy to just convert it to plain text, but rather remove it and
add an introductory text before the list that conveys the same meaning but
with a better reading flow.

Fixes: ccf988b66d69 ("docs: i2c: convert to ReST and add to driver-api bookset")
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 Documentation/i2c/i2c-topology.rst | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/i2c/i2c-topology.rst b/Documentation/i2c/i2c-topology.rst
index 7cb53819778e..1b11535c8946 100644
--- a/Documentation/i2c/i2c-topology.rst
+++ b/Documentation/i2c/i2c-topology.rst
@@ -5,6 +5,8 @@ I2C muxes and complex topologies
 There are a couple of reasons for building more complex I2C topologies
 than a straight-forward I2C bus with one adapter and one or more devices.
 
+Some example use cases are:
+
 1. A mux may be needed on the bus to prevent address collisions.
 
 2. The bus may be accessible from some external bus master, and arbitration
@@ -14,9 +16,6 @@ than a straight-forward I2C bus with one adapter and one or more devices.
    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.
 
-Etc
-===
-
 These constructs 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
-- 
2.34.1

Re: [PATCH 6/9] docs: i2c: i2c-topology: fix incorrect heading

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

On Mon, Aug 08, 2022 at 04:17:05PM +0200, luca.ceresoli@bootlin.com wrote:
> From: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> "Etc" here was never meant to be a heading, it became one while converting
> to ReST.
> 
> It would be easy to just convert it to plain text, but rather remove it and
> add an introductory text before the list that conveys the same meaning but
> with a better reading flow.
> 
> Fixes: ccf988b66d69 ("docs: i2c: convert to ReST and add to driver-api bookset")
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Peter, are you fine with this change?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 7/9] docs: i2c: i2c-topology: reorder sections more logically

[luca.ceresoli]

From: Luca Ceresoli <luca.ceresoli@bootlin.com>

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.
 * 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@bootlin.com>
---
 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.
+
+
 
 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
+=======================   =============================================
-- 
2.34.1

Re: [PATCH 7/9] docs: i2c: i2c-topology: reorder sections more logically

[Bagas Sanjaya]

On 8/8/22 21:17, luca.ceresoli@bootlin.com wrote:
> +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.
> +

These two sentences in n. 2) can be combined into a single paragraph.
Also, did you mean s/intension/intention/?

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH 7/9] docs: i2c: i2c-topology: reorder sections more logically

[Luca Ceresoli]

Hello Bagas,

thanks for the prompt review!

On Tue, 9 Aug 2022 09:08:03 +0700
Bagas Sanjaya <bagasdotme@gmail.com> wrote:

> On 8/8/22 21:17, luca.ceresoli@bootlin.com wrote:
> > +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.
> > +  
> 
> These two sentences in n. 2) can be combined into a single paragraph.
> Also, did you mean s/intension/intention/?

This patch does nothing but reformatting the current text.

Definitely "intension" is a mistake that I didn't spot, I'm adding a
patch to fix that.

About the paragraph split, I have no strong opinion but I'm feeling OK
with the current layout. It splits the generic statement from the
example and IMHO helps readability. Feel free to send a patch to change
that though, if you think it is useful.

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 7/9] docs: i2c: i2c-topology: reorder sections more logically

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]

On Mon, Aug 08, 2022 at 04:17:06PM +0200, luca.ceresoli@bootlin.com wrote:
> From: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> 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.
>  * 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@bootlin.com>

Peter, are you fine with this change?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 8/9] docs: i2c: i2c-sysfs: improve wording

[luca.ceresoli]

From: Luca Ceresoli <luca.ceresoli@bootlin.com>

Improve wording in a couple sentences.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 Documentation/i2c/i2c-sysfs.rst | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/Documentation/i2c/i2c-sysfs.rst b/Documentation/i2c/i2c-sysfs.rst
index 6b68b95cd427..d816a23b80f2 100644
--- a/Documentation/i2c/i2c-sysfs.rst
+++ b/Documentation/i2c/i2c-sysfs.rst
@@ -51,11 +51,10 @@ Google Pixel 3 phone for example::
 ``i2c-2`` is an I2C bus whose number is 2, and ``2-0049`` is an I2C device
 on bus 2 address 0x49 bound with a kernel driver.
 
-Terminologies
-=============
+Terminology
+===========
 
-First, let us define a couple of terminologies to avoid confusions in the later
-sections.
+First, let us define some terms to avoid confusions in the later sections.
 
 (Physical) I2C Bus Controller
 -----------------------------
@@ -117,7 +116,7 @@ Walk through Logical I2C Bus
 
 For the following content, we will use a more complex I2C topology as an
 example. Here is a brief graph for the I2C topology. If you do not understand
-this graph at the first glance, do not be afraid to continue reading this doc
+this graph at first glance, do not be afraid to continue reading this doc
 and review it when you finish reading.
 
 ::
-- 
2.34.1

Re: [PATCH 8/9] docs: i2c: i2c-sysfs: improve wording

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 260 bytes --]


> -First, let us define a couple of terminologies to avoid confusions in the later
> -sections.
> +First, let us define some terms to avoid confusions in the later sections.

Does 'confusion' have a plural? I think singular is better here.

Rest looks good.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 8/9] docs: i2c: i2c-sysfs: improve wording

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 177 bytes --]


> Does 'confusion' have a plural? I think singular is better here.

I took the liberty to fix it myself so we can have the patch in rc1
already. I hope this is fine with you.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 8/9] docs: i2c: i2c-sysfs: improve wording

[Luca Ceresoli]

Hi Wolfram,

On Thu, 11 Aug 2022 23:23:12 +0200
Wolfram Sang <wsa@kernel.org> wrote:

> > Does 'confusion' have a plural? I think singular is better here.  
> 
> I took the liberty to fix it myself so we can have the patch in rc1
> already. I hope this is fine with you.

Sure, thank you!

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 9/9] docs: i2c: i2c-sysfs: fix hyperlinks

[luca.ceresoli]

From: Luca Ceresoli <luca.ceresoli@bootlin.com>

dts files cannot be linked conveniently, thus replace them with literal
formatting.

The links to other rst pages are broken, fix them using the proper syntax.

Fixes: 31df7195b100 ("Documentation: i2c: Add doc for I2C sysfs")
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 Documentation/i2c/i2c-sysfs.rst | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/Documentation/i2c/i2c-sysfs.rst b/Documentation/i2c/i2c-sysfs.rst
index d816a23b80f2..b6d31290fc4b 100644
--- a/Documentation/i2c/i2c-sysfs.rst
+++ b/Documentation/i2c/i2c-sysfs.rst
@@ -99,9 +99,7 @@ Caveat
 This may be a confusing part for people who only know about the physical I2C
 design of a board. It is actually possible to rename the I2C bus physical number
 to a different number in logical I2C bus level in Device Tree Source (DTS) under
-section ``aliases``. See
-`arch/arm/boot/dts/nuvoton-npcm730-gsj.dts
-<../../arch/arm/boot/dts/nuvoton-npcm730-gsj.dts>`_
+section ``aliases``. See ``arch/arm/boot/dts/nuvoton-npcm730-gsj.dts``
 for an example of DTS file.
 
 Best Practice: **(To kernel software developers)** It is better to keep the I2C
@@ -289,8 +287,7 @@ MUX channel 0, and all the way to ``i2c-19`` for the MUX channel 3.
 The kernel software developer is able to pin the fanout MUX channels to a static
 logical I2C bus number in the DTS. This doc will not go through the details on
 how to implement this in DTS, but we can see an example in:
-`arch/arm/boot/dts/aspeed-bmc-facebook-wedge400.dts
-<../../arch/arm/boot/dts/aspeed-bmc-facebook-wedge400.dts>`_
+``arch/arm/boot/dts/aspeed-bmc-facebook-wedge400.dts``
 
 In the above example, there is an 8-channel I2C MUX at address 0x70 on physical
 I2C bus 2. The channel 2 of the MUX is defined as ``imux18`` in DTS,
@@ -382,13 +379,9 @@ Sysfs for the I2C sensor device::
 
 For more info on the Hwmon Sysfs, refer to the doc:
 
-`Naming and data format standards for sysfs files
-<../hwmon/sysfs-interface.rst>`_
+../hwmon/sysfs-interface.rst
 
 Instantiate I2C Devices in I2C Sysfs
 ------------------------------------
 
-Refer to the doc:
-
-`How to instantiate I2C devices, Method 4: Instantiate from user-space
-<instantiating-devices.rst#method-4-instantiate-from-user-space>`_
+Refer to section "Method 4: Instantiate from user-space" of instantiating-devices.rst
-- 
2.34.1

Re: [PATCH 9/9] docs: i2c: i2c-sysfs: fix hyperlinks

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]

On Mon, Aug 08, 2022 at 04:17:08PM +0200, luca.ceresoli@bootlin.com wrote:
> From: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> dts files cannot be linked conveniently, thus replace them with literal
> formatting.
> 
> The links to other rst pages are broken, fix them using the proper syntax.
> 
> Fixes: 31df7195b100 ("Documentation: i2c: Add doc for I2C sysfs")
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Applied to for-current, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]