| Message ID | 20220412085046.1110127-2-vincent.whitchurch@axis.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/2] dt-bindings: i2c: add property to avoid device detection | expand |
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 <vincent.whitchurch@axis.com> > --- > > 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 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?
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.
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/20220206115939.3091265-1-luca@lucaceresoli.net/ 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/20220206115939.3091265-1-luca@lucaceresoli.net/ > > 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/20220206115939.3091265-1-luca@lucaceresoli.net/ >> >> 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.
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
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 <vincent.whitchurch@axis.com> --- Notes: v2: - Change subject prefix - Reword description of property Documentation/devicetree/bindings/i2c/i2c.txt | 4 ++++ 1 file changed, 4 insertions(+)