From: Luca Ceresoli <[email protected]>
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/[email protected]/
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
From: Luca Ceresoli <[email protected]>
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 <[email protected]>
---
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
From: Luca Ceresoli <[email protected]>
"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 <[email protected]>
---
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
From: Luca Ceresoli <[email protected]>
These blocks can be nicely coloured via Sphinx.
Signed-off-by: Luca Ceresoli <[email protected]>
---
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
From: Luca Ceresoli <[email protected]>
Improve wording in a couple sentences.
Signed-off-by: Luca Ceresoli <[email protected]>
---
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
From: Luca Ceresoli <[email protected]>
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 <[email protected]>
---
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
From: Luca Ceresoli <[email protected]>
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 <[email protected]>
---
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
From: Luca Ceresoli <[email protected]>
"Comm", "Count", "DataLow", "DataHigh" are not used in this section.
Signed-off-by: Luca Ceresoli <[email protected]>
---
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
From: Luca Ceresoli <[email protected]>
Use a more professional wording.
Signed-off-by: Luca Ceresoli <[email protected]>
---
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
From: Luca Ceresoli <[email protected]>
"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 <[email protected]>
---
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
On 8/8/22 21:17, [email protected] wrote:
> From: Luca Ceresoli <[email protected]>
>
> Use a more professional wording.
>
> Signed-off-by: Luca Ceresoli <[email protected]>
> ---
> 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 <[email protected]>
--
An old man doll... just what I always wanted! - Clara
On 8/8/22 21:17, [email protected] 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
Hello Bagas,
thanks for the prompt review!
On Tue, 9 Aug 2022 09:08:03 +0700
Bagas Sanjaya <[email protected]> wrote:
> On 8/8/22 21:17, [email protected] 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
On Mon, Aug 08, 2022 at 04:17:00PM +0200, [email protected] wrote:
> From: Luca Ceresoli <[email protected]>
>
> 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 <[email protected]>
Applied to for-current, thanks!
On Mon, Aug 08, 2022 at 04:17:02PM +0200, [email protected] wrote:
> From: Luca Ceresoli <[email protected]>
>
> "Comm", "Count", "DataLow", "DataHigh" are not used in this section.
>
> Signed-off-by: Luca Ceresoli <[email protected]>
Applied to for-current, thanks!
On Mon, Aug 08, 2022 at 04:17:03PM +0200, [email protected] wrote:
> From: Luca Ceresoli <[email protected]>
>
> Use a more professional wording.
>
> Signed-off-by: Luca Ceresoli <[email protected]>
Applied to for-current, thanks!
On Mon, Aug 08, 2022 at 04:17:06PM +0200, [email protected] wrote:
> From: Luca Ceresoli <[email protected]>
>
> 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 <[email protected]>
Peter, are you fine with this change?
On Mon, Aug 08, 2022 at 04:17:01PM +0200, [email protected] wrote:
> From: Luca Ceresoli <[email protected]>
>
> "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 <[email protected]>
Applied to for-current, thanks!
On Mon, Aug 08, 2022 at 04:17:08PM +0200, [email protected] wrote:
> From: Luca Ceresoli <[email protected]>
>
> 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 <[email protected]>
Applied to for-current, thanks!
On Mon, Aug 08, 2022 at 04:17:05PM +0200, [email protected] wrote:
> From: Luca Ceresoli <[email protected]>
>
> "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 <[email protected]>
Peter, are you fine with this change?
On Mon, Aug 08, 2022 at 04:17:04PM +0200, [email protected] wrote:
> From: Luca Ceresoli <[email protected]>
>
> These blocks can be nicely coloured via Sphinx.
>
> Signed-off-by: Luca Ceresoli <[email protected]>
Applied to for-current, thanks!
> -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.
> 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.
Hi Wolfram,
On Thu, 11 Aug 2022 23:23:12 +0200
Wolfram Sang <[email protected]> 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