v2:
- Change subject prefix of bindings patch
- Reword description of property in bindings patch
When a driver with a ->detect callback (such as lm75) is loaded, the i2c core
performs I2C transactions on the bus to all the addresses listed in that
driver's address_list. This kind of probing wastes time and as
Documentation/i2c/instantiating-devices.rst says, this method is not
recommended and it is instead advised to list all devices in the devicetree.
However, even if all the devices are listed in the devicetree, there is
currently no way to prevent the core from attempting auto detection short of
patching controller drivers to not pass the I2C_CLASS* bits in adap->class.
The latter is not always possible since generic drivers like i2c-gpio set these
bits.
To avoid this unnecessary probing and reduce boot time, this series adds a
property to the devicetree and support in the I2C core to allow this feature to
be disabled.
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Vincent Whitchurch (2):
dt-bindings: i2c: add property to avoid device detection
i2c: core: support no-detect property
Documentation/devicetree/bindings/i2c/i2c.txt | 4 ++++
drivers/i2c/i2c-core-base.c | 8 +++++++-
include/linux/i2c.h | 1 +
3 files changed, 12 insertions(+), 1 deletion(-)
--
2.34.1
When drivers with ->detect callbacks are loaded, the I2C core does a
bunch of transactions to try to probe for these devices, regardless of
whether they are specified in the devicetree or not. (This only happens
on I2C controllers whose drivers enable the I2C_CLASS* flags, but this
is the case for generic drivers like i2c-gpio.)
These kinds of transactions are unnecessary on systems where the
devicetree specifies all the devices on the I2C bus, so add a property
to indicate that the devicetree description of the hardware is complete
and thus allow this discovery to be disabled.
Signed-off-by: Vincent Whitchurch <[email protected]>
---
Notes:
v2:
- Change subject prefix
- Reword description of property
Documentation/devicetree/bindings/i2c/i2c.txt | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index fc3dd7ec0445..960d1d5c9362 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -72,6 +72,10 @@ wants to support one of the below features, it should adapt these bindings.
this information to adapt power management to keep the arbitration awake
all the time, for example. Can not be combined with 'single-master'.
+- no-detect
+ states that no other devices are present on this bus other than the
+ ones listed in the devicetree.
+
- pinctrl
add extra pinctrl to configure SCL/SDA pins to GPIO function for bus
recovery, call it "gpio" or "recovery" (deprecated) state
--
2.34.1
On 12/04/2022 10:50, Vincent Whitchurch wrote:
> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
> index fc3dd7ec0445..960d1d5c9362 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> @@ -72,6 +72,10 @@ wants to support one of the below features, it should adapt these bindings.
> this information to adapt power management to keep the arbitration awake
> all the time, for example. Can not be combined with 'single-master'.
>
> +- no-detect
> + states that no other devices are present on this bus other than the
> + ones listed in the devicetree.
Looks good to me.
Acked-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
If the devicetree specifies the no-detect property, we know that there
are no other devices on the bus other than the ones listed in the
devicetree, so avoid calling drivers' detect callback and wasting time
probing for devices which do not exist.
Signed-off-by: Vincent Whitchurch <[email protected]>
---
drivers/i2c/i2c-core-base.c | 8 +++++++-
include/linux/i2c.h | 1 +
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index d43db2c3876e..d43025b84546 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1341,7 +1341,8 @@ static int i2c_do_add_adapter(struct i2c_driver *driver,
struct i2c_adapter *adap)
{
/* Detect supported devices on that bus, and instantiate them */
- i2c_detect(adap, driver);
+ if (adap->detect)
+ i2c_detect(adap, driver);
return 0;
}
@@ -1432,6 +1433,7 @@ EXPORT_SYMBOL_GPL(i2c_handle_smbus_host_notify);
static int i2c_register_adapter(struct i2c_adapter *adap)
{
+ struct device_node *np = adap->dev.of_node;
int res = -EINVAL;
/* Can't register until after driver model init */
@@ -1502,6 +1504,10 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
"Failed to create compatibility class link\n");
#endif
+ adap->detect = true;
+ if (np && of_property_read_bool(np, "no-detect"))
+ adap->detect = false;
+
/* create pre-declared device nodes */
of_i2c_register_devices(adap);
i2c_acpi_install_space_handler(adap);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index fbda5ada2afc..8fad5fe85685 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -728,6 +728,7 @@ struct i2c_adapter {
struct rt_mutex bus_lock;
struct rt_mutex mux_lock;
+ bool detect;
int timeout; /* in jiffies */
int retries;
struct device dev; /* the adapter device */
--
2.34.1
On Tue, Apr 12, 2022 at 10:50:45AM +0200, Vincent Whitchurch wrote:
> When drivers with ->detect callbacks are loaded, the I2C core does a
> bunch of transactions to try to probe for these devices, regardless of
> whether they are specified in the devicetree or not. (This only happens
> on I2C controllers whose drivers enable the I2C_CLASS* flags, but this
> is the case for generic drivers like i2c-gpio.)
>
> These kinds of transactions are unnecessary on systems where the
> devicetree specifies all the devices on the I2C bus, so add a property
> to indicate that the devicetree description of the hardware is complete
> and thus allow this discovery to be disabled.
>
> Signed-off-by: Vincent Whitchurch <[email protected]>
> ---
>
> Notes:
> v2:
> - Change subject prefix
> - Reword description of property
>
> Documentation/devicetree/bindings/i2c/i2c.txt | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
> index fc3dd7ec0445..960d1d5c9362 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> @@ -72,6 +72,10 @@ wants to support one of the below features, it should adapt these bindings.
> this information to adapt power management to keep the arbitration awake
> all the time, for example. Can not be combined with 'single-master'.
>
> +- no-detect
> + states that no other devices are present on this bus other than the
> + ones listed in the devicetree.
This belongs in the schema instead:
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/i2c/i2c-controller.yaml
Rob
On Thu, Apr 14, 2022 at 10:55:40AM +0200, Vincent Whitchurch wrote:
> On Tue, Apr 12, 2022 at 11:22:41PM +0200, Rob Herring wrote:
> > On Tue, Apr 12, 2022 at 10:50:45AM +0200, Vincent Whitchurch wrote:
> > > diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
> > > index fc3dd7ec0445..960d1d5c9362 100644
> > > --- a/Documentation/devicetree/bindings/i2c/i2c.txt
> > > +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> > > @@ -72,6 +72,10 @@ wants to support one of the below features, it should adapt these bindings.
> > > this information to adapt power management to keep the arbitration awake
> > > all the time, for example. Can not be combined with 'single-master'.
> > >
> > > +- no-detect
> > > + states that no other devices are present on this bus other than the
> > > + ones listed in the devicetree.
> >
> > This belongs in the schema instead:
> >
> > https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/i2c/i2c-controller.yaml
>
> OK, thank you, I've sent a PR[0] now, but I must admit I don't quite
> understand how this property differs from the other ones in this file
> which aren't documented there.
Thanks.
The issue in general is we need permission to relicense anything in the
kernel tree to move it. In some cases, the schema is written, but the
descriptions have not been moved (as that's the part needing to be
copied. If we missed properties, I'm not sure what happened but they
should be in the schema too. Maybe they were added around the same time
the schema got written.
Rob
On Tue, Apr 12, 2022 at 11:22:41PM +0200, Rob Herring wrote:
> On Tue, Apr 12, 2022 at 10:50:45AM +0200, Vincent Whitchurch wrote:
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
> > index fc3dd7ec0445..960d1d5c9362 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> > @@ -72,6 +72,10 @@ wants to support one of the below features, it should adapt these bindings.
> > this information to adapt power management to keep the arbitration awake
> > all the time, for example. Can not be combined with 'single-master'.
> >
> > +- no-detect
> > + states that no other devices are present on this bus other than the
> > + ones listed in the devicetree.
>
> This belongs in the schema instead:
>
> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/i2c/i2c-controller.yaml
OK, thank you, I've sent a PR[0] now, but I must admit I don't quite
understand how this property differs from the other ones in this file
which aren't documented there.
[0] https://github.com/devicetree-org/dt-schema/pull/72
On Sat, May 14, 2022 at 04:26:16PM +0200, Wolfram Sang wrote:
> That aside, I am not sure we should handle this at DT level. Maybe we
> should better change the GPIO driver to not populate a class if we have
> a firmware node?
Is it always safe to not do this detection if we have a firmware node?
Then maybe the core could just always skip it in that case without
looking for a special property or requiring individual drivers to choose
what to do?
> > That aside, I am not sure we should handle this at DT level. Maybe we
> > should better change the GPIO driver to not populate a class if we have
> > a firmware node?
>
> Is it always safe to not do this detection if we have a firmware node?
> Then maybe the core could just always skip it in that case without
> looking for a special property or requiring individual drivers to choose
> what to do?
Need to think about it. Could be argued. So far, setting .class
correctly was the job of the driver.
On Tue, Apr 12, 2022 at 10:50:45AM +0200, Vincent Whitchurch wrote:
> When drivers with ->detect callbacks are loaded, the I2C core does a
> bunch of transactions to try to probe for these devices, regardless of
> whether they are specified in the devicetree or not. (This only happens
> on I2C controllers whose drivers enable the I2C_CLASS* flags, but this
> is the case for generic drivers like i2c-gpio.)
>
> These kinds of transactions are unnecessary on systems where the
> devicetree specifies all the devices on the I2C bus, so add a property
> to indicate that the devicetree description of the hardware is complete
> and thus allow this discovery to be disabled.
Hmm, I don't think the name is fitting. "no-detect" is the desired
behaviour but a proper description is more like "bus-complete" or
something?
That aside, I am not sure we should handle this at DT level. Maybe we
should better change the GPIO driver to not populate a class if we have
a firmware node?
2022-05-14 at 16:26, Wolfram Sang wrote:
> On Tue, Apr 12, 2022 at 10:50:45AM +0200, Vincent Whitchurch wrote:
>> When drivers with ->detect callbacks are loaded, the I2C core does a
>> bunch of transactions to try to probe for these devices, regardless of
>> whether they are specified in the devicetree or not. (This only happens
>> on I2C controllers whose drivers enable the I2C_CLASS* flags, but this
>> is the case for generic drivers like i2c-gpio.)
>>
>> These kinds of transactions are unnecessary on systems where the
>> devicetree specifies all the devices on the I2C bus, so add a property
>> to indicate that the devicetree description of the hardware is complete
>> and thus allow this discovery to be disabled.
>
> Hmm, I don't think the name is fitting. "no-detect" is the desired
> behaviour but a proper description is more like "bus-complete" or
> something?
>
> That aside, I am not sure we should handle this at DT level. Maybe we
> should better change the GPIO driver to not populate a class if we have
> a firmware node?
We also have the somewhat related address translation case (which I
still need to look at). [Adding Luca to Cc]
https://lore.kernel.org/lkml/[email protected]/
If a bus is "bus-complete", then address translation could use
any unused address instead of from an explicit list of addresses.
I.e. the "i2c-alias-pool" in the binding in patch 4/6 of that
series could be made optional if the bus is "bus-complete".
Not sure how much value there is in that?
Cheers,
Peter
[Now with the proper email to Luca, sorry about that...]
2022-05-16 at 09:57, Peter Rosin wrote:
> 2022-05-14 at 16:26, Wolfram Sang wrote:
>> On Tue, Apr 12, 2022 at 10:50:45AM +0200, Vincent Whitchurch wrote:
>>> When drivers with ->detect callbacks are loaded, the I2C core does a
>>> bunch of transactions to try to probe for these devices, regardless of
>>> whether they are specified in the devicetree or not. (This only happens
>>> on I2C controllers whose drivers enable the I2C_CLASS* flags, but this
>>> is the case for generic drivers like i2c-gpio.)
>>>
>>> These kinds of transactions are unnecessary on systems where the
>>> devicetree specifies all the devices on the I2C bus, so add a property
>>> to indicate that the devicetree description of the hardware is complete
>>> and thus allow this discovery to be disabled.
>> Hmm, I don't think the name is fitting. "no-detect" is the desired
>> behaviour but a proper description is more like "bus-complete" or
>> something?
>>
>> That aside, I am not sure we should handle this at DT level. Maybe we
>> should better change the GPIO driver to not populate a class if we have
>> a firmware node?
> We also have the somewhat related address translation case (which I
> still need to look at). [Adding Luca to Cc]
>
> https://lore.kernel.org/lkml/[email protected]/
>
> If a bus is "bus-complete", then address translation could use
> any unused address instead of from an explicit list of addresses.
> I.e. the "i2c-alias-pool" in the binding in patch 4/6 of that
> series could be made optional if the bus is "bus-complete".
>
> Not sure how much value there is in that?
>
> Cheers,
> Peter
Hi Peter, all,
On 16/05/22 10:07, Peter Rosin wrote:
> [Now with the proper email to Luca, sorry about that...]
>
> 2022-05-16 at 09:57, Peter Rosin wrote:
>> 2022-05-14 at 16:26, Wolfram Sang wrote:
>>> On Tue, Apr 12, 2022 at 10:50:45AM +0200, Vincent Whitchurch wrote:
>>>> When drivers with ->detect callbacks are loaded, the I2C core does a
>>>> bunch of transactions to try to probe for these devices, regardless of
>>>> whether they are specified in the devicetree or not. (This only happens
>>>> on I2C controllers whose drivers enable the I2C_CLASS* flags, but this
>>>> is the case for generic drivers like i2c-gpio.)
>>>>
>>>> These kinds of transactions are unnecessary on systems where the
>>>> devicetree specifies all the devices on the I2C bus, so add a property
>>>> to indicate that the devicetree description of the hardware is complete
>>>> and thus allow this discovery to be disabled.
>>> Hmm, I don't think the name is fitting. "no-detect" is the desired
>>> behaviour but a proper description is more like "bus-complete" or
>>> something?
>>>
>>> That aside, I am not sure we should handle this at DT level. Maybe we
>>> should better change the GPIO driver to not populate a class if we have
>>> a firmware node?
>> We also have the somewhat related address translation case (which I
>> still need to look at). [Adding Luca to Cc]
>>
>> https://lore.kernel.org/lkml/[email protected]/
>>
>> If a bus is "bus-complete", then address translation could use
>> any unused address instead of from an explicit list of addresses.
>> I.e. the "i2c-alias-pool" in the binding in patch 4/6 of that
>> series could be made optional if the bus is "bus-complete".
Indeed the alias pool is meant to completely disappear from the ATR
implementation. The i2c core should evolve to know which addresses
correspond to a device (no matter if it has a driver or not) and use any
other addresses as aliases. This was the outcome of discussion on this
topic with Wolfram, even though I AFAIK any implementation effort is
idle since a long time.
--
Luca