| Message ID | 20220405121627.1560949-2-vincent.whitchurch@axis.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | i2c: Allow disabling auto detection via devicetree | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success |
On 05/04/2022 14:16, 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.) > Thank you for your patch. There is something to discuss/improve. > These kinds of transactions are unnecessary on systems where the > devicetree specifies all the devices on the I2C bus, so add a property > to allow them to be disabled. > > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> > --- > Documentation/devicetree/bindings/i2c/i2c.txt | 4 ++++ Use the subject matching the subsystem (git log --oneline -- .......). > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt > index fc3dd7ec0445..5f7bdf4851e8 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 the system should not attempt to automatically detect > + devices which are not explicitly specified as child nodes. Focus on the hardware, not on system and behavior. The hardware property is that bus is completely described in Devicetree or detection does not work correctly. I guess the property name could stay like this, but the description could be rephrased. Best regards, Krzysztof
On Tue, Apr 05, 2022 at 03:57:16PM +0200, Krzysztof Kozlowski wrote: > On 05/04/2022 14:16, Vincent Whitchurch wrote: > > Documentation/devicetree/bindings/i2c/i2c.txt | 4 ++++ > > Use the subject matching the subsystem (git log --oneline -- .......). I did look at the git log when writing the subject, but there's a bunch of variation there so I went for the subject prefix which Wolfram himself used since I assumed he would be the one applying these patches. $ git log -n5 --oneline origin/master -- Documentation/devicetree/bindings/i2c/i2c.txt 6881e493b08f dt-bindings: net: New binding mctp-i2c-controller 168290fb2675 i2c: add binding to mark a bus as supporting SMBus-Alert e6277308ac21 i2c: add binding to mark a bus as SMBus db36e827d876 dt-bindings: i2c: add generic properties for GPIO bus recovery fad5972a1eca i2c: add 'single-master' property to generic bindings > > > 1 file changed, 4 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt > > index fc3dd7ec0445..5f7bdf4851e8 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 the system should not attempt to automatically detect > > + devices which are not explicitly specified as child nodes. > > Focus on the hardware, not on system and behavior. The hardware property > is that bus is completely described in Devicetree or detection does not > work correctly. I guess the property name could stay like this, but the > description could be rephrased. I'm having a hard time drawing the line between hardware and the "system" here. The bus being completely described in devicetree doesn't sound like a hardware property to me, but, yes, I can change the description to say that.
On 05/04/2022 16:12, Vincent Whitchurch wrote: > On Tue, Apr 05, 2022 at 03:57:16PM +0200, Krzysztof Kozlowski wrote: >> On 05/04/2022 14:16, Vincent Whitchurch wrote: >>> Documentation/devicetree/bindings/i2c/i2c.txt | 4 ++++ >> >> Use the subject matching the subsystem (git log --oneline -- .......). > > I did look at the git log when writing the subject, but there's a bunch > of variation there so I went for the subject prefix which Wolfram > himself used since I assumed he would be the one applying these patches. > > $ git log -n5 --oneline origin/master -- Documentation/devicetree/bindings/i2c/i2c.txt > 6881e493b08f dt-bindings: net: New binding mctp-i2c-controller > 168290fb2675 i2c: add binding to mark a bus as supporting SMBus-Alert > e6277308ac21 i2c: add binding to mark a bus as SMBus > db36e827d876 dt-bindings: i2c: add generic properties for GPIO bus recovery > fad5972a1eca i2c: add 'single-master' property to generic bindings The proper command for a subsystem is: git log --oneline -- Documentation/devicetree/bindings/i2c/ and then you see mostly different pattern. Other way, like for example Mark Brown prefers, is: i2c: dt-bindings: It would be fine as well, but the point is that you did not include the dt-bindings prefix here at all and almost always I ignore all such emails. >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt >>> index fc3dd7ec0445..5f7bdf4851e8 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 the system should not attempt to automatically detect >>> + devices which are not explicitly specified as child nodes. >> >> Focus on the hardware, not on system and behavior. The hardware property >> is that bus is completely described in Devicetree or detection does not >> work correctly. I guess the property name could stay like this, but the >> description could be rephrased. > > I'm having a hard time drawing the line between hardware and the > "system" here. The bus being completely described in devicetree doesn't > sound like a hardware property to me, but, yes, I can change the > description to say that. Then you need to come up with better description. :) By system I understood here Operating System, therefore a Software piece or pieces. Bindings should not rather focus on hardware instead of referencing software. Software actually not matter for the bindings... There is quite clear distinction between hardware and operating system... Can you explain why do you think "bus being completely described" is not a hardware description? Bindings and DTS describe hardware, so one of parts of such description is whether nodes and other properties are present. If all child nodes are present in DT - no-detect - and this is a purely hardware point of view. No software here. Best regards, Krzysztof
On 06/04/2022 09:26, Krzysztof Kozlowski wrote: >> >> I'm having a hard time drawing the line between hardware and the >> "system" here. The bus being completely described in devicetree doesn't >> sound like a hardware property to me, but, yes, I can change the >> description to say that. > > Then you need to come up with better description. :) By system I > understood here Operating System, therefore a Software piece or pieces. > Bindings should not rather focus on hardware instead of referencing > software. Ah... I meant: Bindings should rather focus on hardware instead of referencing software. > Software actually not matter for the bindings... There is > quite clear distinction between hardware and operating system... > > Can you explain why do you think "bus being completely described" is not > a hardware description? Bindings and DTS describe hardware, so one of > parts of such description is whether nodes and other properties are > present. If all child nodes are present in DT - no-detect - and this is > a purely hardware point of view. No software here. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt index fc3dd7ec0445..5f7bdf4851e8 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 the system should not attempt to automatically detect + devices which are not explicitly specified as child nodes. + - 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 allow them to be disabled. Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> --- Documentation/devicetree/bindings/i2c/i2c.txt | 4 ++++ 1 file changed, 4 insertions(+)