passing two interrupts two an I2C driver
Subsystems (3)
Hi all,
I'd like to consult the implementation of DT binding for the I2C device that exposes two interrupt pins (INT1 and INT2). Both pins can be configured to generate either data ready interrupts or event interrupts. I want to implement DT binding that will handle also the situation when only one of the interrupt sources is routed from the device to the CPU.
Below is my implementation using interrupt-map:
+++ b/Documentation/devicetree/bindings/iio/pressure/lps331ap.txt @@ -0,0 +1,41 @@
+* STMicroelectronics LPS331AP barometer sensor
+
+Required properties:
+
+ - compatible : should be "st,lps331ap"
+ - reg : the I2C address of the barometer
+
+Optional properties:
+
+ - st,data-ready-interrupt-pin : redirect DRDY on pin INT1 (1) or pin INT2 (2) (u8) + - interrupt-parent : phandle to the interrupt map subnode + - interrupts : interrupt mapping for LPS331AP interrupt sources: + 2 sources: 0 - data ready, 1 - threshold event + - irq-map : irq sub-node defining interrupt map + (all properties listed below are required): + - #interrupt-cells : should be 1 + - #address-cells : should be 0 + - #size-cells : should be 0 + - interrupt-map : table of entries consisting of three child elements: + - unit_interrupt_specifier - 0 : data ready, 1 : threshold event + - interrupt parent phandle + - parent unit interrupt specifier consisiting of two elements:
+ - index of the interrupt within the controller
+ - flags : should be 0
+
+Example:
+
+lps331ap@5d {
+ compatible = "st,lps331ap";
+ reg = <0x5d>;
+ drdy-int-pin = /bits/ 8 <2>;
+ interrupt-parent = <&irq_map>;
+ interrupts = <0>, <1>;
+
+ irq_map: irq-map {
+ #interrupt-cells = <1>;
+ #address-cells = <0>;
+ #size-cells = <0>;
+ interrupt-map = <0 &gpf0 5 0>;+ };
+};
And here is how the driver uses this information:
- if interrupt-map is empty then the driver configures itself to work without interrupt support
- if only one interrupt source is available then the driver configures the device to generate data ready interrupts on the corresponding INTx pin (in this case the driver must know which of the device pins is routed to the cpu - st,data-ready-interrupt-pin property conveys this information)
- if both interrupt sources are available then the driver configures the device to generate data ready interrupts on the interrupt pin corresponding to the interrupt source with index 0 and event interrupts to the interrupt source with index 1.
This solution seems to be a little awkward so I'd like to ask if there is any neater way to handle presented requirements. The solution must facilitate passing information about two interrupt sources two the I2C driver. I have been unable to find similar solution in the kernel so far.
Thanks,
Jacek Anaszewski
Quoted 9 lines · Jacek Anaszewski
Hi all,
I'd like to consult the implementation of DT binding for the I2C device that exposes two interrupt pins (INT1 and INT2). Both pins can be configured to generate either data ready interrupts or event interrupts. I want to implement DT binding that will handle also the situation when only one of the interrupt sources is routed from the device to the CPU.
Quoted 3 lines
Below is my implementation using interrupt-map:
+ - interrupt-parent : phandle to the interrupt map subnode
When using interrupt-parent to point at an interrupt map, I believe you usually just point at the current node; there's no need to a child node.
Review
Quoted 5 lines
(quoted diff) +4
-0+ - interrupts : interrupt mapping for LPS331AP interrupt sources: + 2 sources: 0 - data ready, 1 - threshold event + - irq-map : irq sub-node defining interrupt map + (all properties listed below are required):
So, this node isn't required.
Review
Quoted 4 lines
(quoted diff) +3
-0+ - #interrupt-cells : should be 1 + - #address-cells : should be 0 + - #size-cells : should be 0
There are no addressed entities in this node, so I don't see why those two properties are needed.
Review
Quoted 6 lines
(quoted diff) +6
-0+ - interrupt-map : table of entries consisting of three child elements: + - unit_interrupt_specifier - 0 : data ready, 1 : threshold event + - interrupt parent phandle + - parent unit interrupt specifier consisiting of two elements: + - index of the interrupt within the controller + - flags : should be 0
It's up to the binding for the node referenced by the phandle to define how many cells need be present for "flags", and their meaning. This binding shouldn't attempt to describe those. Equally, the concept of interrupt-map should be defined elsewwere (e.g. Documentation/devicetree/bindings/interrupt-controller/interrupts.txt); it's a generic that shouldn't need duplication in each binding that uses interrupts.
Quoted 17 lines
(quoted diff) +16
-0+Example: + +lps331ap@5d { + compatible = "st,lps331ap"; + reg = <0x5d>; + drdy-int-pin = /bits/ 8 <2>; + interrupt-parent = <&irq_map>; + interrupts = <0>, <1>; + + irq_map: irq-map { + #interrupt-cells = <1>; + #address-cells = <0>; + #size-cells = <0>; + interrupt-map = <0 &gpf0 5 0>; + }; +};
Quoted 4 lines
And here is how the driver uses this information:
- if interrupt-map is empty then the driver configures itself to work without interrupt support
The presence or lack of interrupt support should be driven by the presence of the interrupts property. interrupt-map should only be used (if present) to assist in the parsing of the interrupts property.
Quoted 10 lines
- if only one interrupt source is available then the driver configures the device to generate data ready interrupts on the corresponding INTx pin (in this case the driver must know which of the device pins is routed to the cpu - st,data-ready-interrupt-pin property conveys this information)
- if both interrupt sources are available then the driver configures the device to generate data ready interrupts on the interrupt pin corresponding to the interrupt source with index 0 and event interrupts to the interrupt source with index 1.
Quoted 5 lines
This solution seems to be a little awkward so I'd like to ask if there is any neater way to handle presented requirements. The solution must facilitate passing information about two interrupt sources two the I2C driver. I have been unable to find similar solution in the kernel so far.
Indeed. I think it would be better to work as follows:
interrupts: contains one or two interrupt specifiers. The first entry always defines the data ready interrupt. The second entry, if present, defines the threshold event interrupt. This at least allows the following combinations to be very simple expressed:
* no interrrupts
* just data
* both data and threshold (assuming they're routed to the same parent)
(you could swap the order if it's likely to be more common to have just a threshold interrupt without any data interrupt).
In order to allow the presence of a threshold interrupt but no data interrupt, then I think you would need interrupt-map:
lps331ap: lps331ap@5d {
compatible = "st,lps331ap";
reg = <0x5d>;
interrupt-parent = <&lps331ap>;
interrupts = <0>, <1>;
interrupt-map = <0 0>, /* nowhere */
<1 &gpf0 6 0>;
};In either of those interrupt-map entries, you should be able to set the phandle to 0 to indicate no interrupt, hence allowing the first entry to have "no interrupt". Whether that works currently in the code, I have no idea.
If only the interrupts property were (phandle, args*)*, we could just set the phandle to 0 in the interrupts property directly. Oh well, roll on DT 2.0;-)
Quoted 107 lines · Stephen Warren
Quoted 9 lines · Jacek Anaszewski
Hi all,
I'd like to consult the implementation of DT binding for the I2C device that exposes two interrupt pins (INT1 and INT2). Both pins can be configured to generate either data ready interrupts or event interrupts. I want to implement DT binding that will handle also the situation when only one of the interrupt sources is routed from the device to the CPU.
Quoted 3 lines
Below is my implementation using interrupt-map:
+ - interrupt-parent : phandle to the interrupt map subnode
When using interrupt-parent to point at an interrupt map, I believe you usually just point at the current node; there's no need to a child node.
Review
Quoted 5 lines
(quoted diff) +4
-0+ - interrupts : interrupt mapping for LPS331AP interrupt sources: + 2 sources: 0 - data ready, 1 - threshold event + - irq-map : irq sub-node defining interrupt map + (all properties listed below are required):So, this node isn't required.
Review
Quoted 4 lines
(quoted diff) +3
-0+ - #interrupt-cells : should be 1 + - #address-cells : should be 0 + - #size-cells : should be 0There are no addressed entities in this node, so I don't see why those two properties are needed.
Review
Quoted 6 lines
(quoted diff) +6
-0+ - interrupt-map : table of entries consisting of three child elements: + - unit_interrupt_specifier - 0 : data ready, 1 : threshold event + - interrupt parent phandle + - parent unit interrupt specifier consisiting of two elements: + - index of the interrupt within the controller + - flags : should be 0It's up to the binding for the node referenced by the phandle to define how many cells need be present for "flags", and their meaning. This binding shouldn't attempt to describe those. Equally, the concept of interrupt-map should be defined elsewwere (e.g.
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt); it's a generic that shouldn't need duplication in each binding that uses interrupts.Quoted 17 lines
(quoted diff) +16
-0+Example: + +lps331ap@5d { + compatible = "st,lps331ap"; + reg = <0x5d>; + drdy-int-pin = /bits/ 8 <2>; + interrupt-parent = <&irq_map>; + interrupts = <0>, <1>; + + irq_map: irq-map { + #interrupt-cells = <1>; + #address-cells = <0>; + #size-cells = <0>; + interrupt-map = <0 &gpf0 5 0>; + }; +};Quoted 4 lines
And here is how the driver uses this information:
- if interrupt-map is empty then the driver configures itself to work without interrupt support
The presence or lack of interrupt support should be driven by the presence of the interrupts property. interrupt-map should only be used (if present) to assist in the parsing of the interrupts property.
Quoted 10 lines
- if only one interrupt source is available then the driver configures the device to generate data ready interrupts on the corresponding INTx pin (in this case the driver must know which of the device pins is routed to the cpu - st,data-ready-interrupt-pin property conveys this information)
- if both interrupt sources are available then the driver configures the device to generate data ready interrupts on the interrupt pin corresponding to the interrupt source with index 0 and event interrupts to the interrupt source with index 1.Quoted 5 lines
This solution seems to be a little awkward so I'd like to ask if there is any neater way to handle presented requirements. The solution must facilitate passing information about two interrupt sources two the I2C driver. I have been unable to find similar solution in the kernel so far.
Indeed. I think it would be better to work as follows:
interrupts: contains one or two interrupt specifiers. The first entry always defines the data ready interrupt. The second entry, if present, defines the threshold event interrupt. This at least allows the following combinations to be very simple expressed:
* no interrrupts
* just data
* both data and threshold (assuming they're routed to the same parent)(you could swap the order if it's likely to be more common to have just a threshold interrupt without any data interrupt).
In order to allow the presence of a threshold interrupt but no data interrupt, then I think you would need interrupt-map:
Quoted 8 lines
lps331ap: lps331ap@5d {
compatible = "st,lps331ap";
reg = <0x5d>;
interrupt-parent = <&lps331ap>;
interrupts = <0>, <1>;
interrupt-map = <0 0>, /* nowhere */
<1 &gpf0 6 0>;
};The interrupt-names property exists for this purpose (describing interrupts which may or may not be present). Describing a nonexistent interrupt and mapping it nowhere feels like a hack to me when we can describe exactly what's present.
Thanks,
Mark.
Quoted 117 lines · Mark Rutland
Quoted 107 lines · Stephen Warren
Quoted 9 lines · Jacek Anaszewski
Hi all,
I'd like to consult the implementation of DT binding for the I2C device that exposes two interrupt pins (INT1 and INT2). Both pins can be configured to generate either data ready interrupts or event interrupts. I want to implement DT binding that will handle also the situation when only one of the interrupt sources is routed from the device to the CPU.
Quoted 3 lines
Below is my implementation using interrupt-map:
+ - interrupt-parent : phandle to the interrupt map subnode
When using interrupt-parent to point at an interrupt map, I believe you usually just point at the current node; there's no need to a child node.
Review
Quoted 5 lines
(quoted diff) +4
-0+ - interrupts : interrupt mapping for LPS331AP interrupt sources: + 2 sources: 0 - data ready, 1 - threshold event + - irq-map : irq sub-node defining interrupt map + (all properties listed below are required):So, this node isn't required.
Review
Quoted 4 lines
(quoted diff) +3
-0+ - #interrupt-cells : should be 1 + - #address-cells : should be 0 + - #size-cells : should be 0There are no addressed entities in this node, so I don't see why those two properties are needed.
Review
Quoted 6 lines
(quoted diff) +6
-0+ - interrupt-map : table of entries consisting of three child elements: + - unit_interrupt_specifier - 0 : data ready, 1 : threshold event + - interrupt parent phandle + - parent unit interrupt specifier consisiting of two elements: + - index of the interrupt within the controller + - flags : should be 0It's up to the binding for the node referenced by the phandle to define how many cells need be present for "flags", and their meaning. This binding shouldn't attempt to describe those. Equally, the concept of interrupt-map should be defined elsewwere (e.g.
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt); it's a generic that shouldn't need duplication in each binding that uses interrupts.Quoted 17 lines
(quoted diff) +16
-0+Example: + +lps331ap@5d { + compatible = "st,lps331ap"; + reg = <0x5d>; + drdy-int-pin = /bits/ 8 <2>; + interrupt-parent = <&irq_map>; + interrupts = <0>, <1>; + + irq_map: irq-map { + #interrupt-cells = <1>; + #address-cells = <0>; + #size-cells = <0>; + interrupt-map = <0 &gpf0 5 0>; + }; +};Quoted 4 lines
And here is how the driver uses this information:
- if interrupt-map is empty then the driver configures itself to work without interrupt support
The presence or lack of interrupt support should be driven by the presence of the interrupts property. interrupt-map should only be used (if present) to assist in the parsing of the interrupts property.
Quoted 10 lines
- if only one interrupt source is available then the driver configures the device to generate data ready interrupts on the corresponding INTx pin (in this case the driver must know which of the device pins is routed to the cpu - st,data-ready-interrupt-pin property conveys this information)
- if both interrupt sources are available then the driver configures the device to generate data ready interrupts on the interrupt pin corresponding to the interrupt source with index 0 and event interrupts to the interrupt source with index 1.Quoted 5 lines
This solution seems to be a little awkward so I'd like to ask if there is any neater way to handle presented requirements. The solution must facilitate passing information about two interrupt sources two the I2C driver. I have been unable to find similar solution in the kernel so far.
Indeed. I think it would be better to work as follows:
interrupts: contains one or two interrupt specifiers. The first entry always defines the data ready interrupt. The second entry, if present, defines the threshold event interrupt. This at least allows the following combinations to be very simple expressed:
* no interrrupts
* just data
* both data and threshold (assuming they're routed to the same parent)(you could swap the order if it's likely to be more common to have just a threshold interrupt without any data interrupt).
In order to allow the presence of a threshold interrupt but no data interrupt, then I think you would need interrupt-map:
Quoted 8 lines
lps331ap: lps331ap@5d { compatible = "st,lps331ap"; reg = <0x5d>; interrupt-parent = <&lps331ap>; interrupts = <0>, <1>; interrupt-map = <0 0>, /* nowhere */ <1 &gpf0 6 0>; };
Quoted 4 lines
The interrupt-names property exists for this purpose (describing interrupts which may or may not be present). Describing a nonexistent interrupt and mapping it nowhere feels like a hack to me when we can describe exactly what's present.
But the rules for interrupts basically precludes the useful use of interrupt-names.
The interrupts property was introduced long before interrupt-names. As such, the rule was always that entries in interrupts had to appear at a specific index in the property, in other words, the property had to be in a specific order, and there's no way of missing entries out.
The interrupt-names property was added much later and more as a documentation for the order in *.dts than as the primary lookup key. Even with an interrupt-names property present, the order of entries in interrupts is still fixed.
So, using interrupt-names doesn't allow you to have optional entries in interrupts, nor re-order the property. We really should not have added interrupt-names, since it gives false impressions.
For newer bindings such as clocks/clock-names, clock-names is the primary lookup key, so things can be optional.
We should document which properties are purely looked up by index, and which properties have a useful *-names property associated with them as the primary lookup key.
Quoted 123 lines · Stephen Warren
Quoted 117 lines · Mark Rutland
Quoted 107 lines · Stephen Warren
Quoted 9 lines · Jacek Anaszewski
Hi all,
I'd like to consult the implementation of DT binding for the I2C device that exposes two interrupt pins (INT1 and INT2). Both pins can be configured to generate either data ready interrupts or event interrupts. I want to implement DT binding that will handle also the situation when only one of the interrupt sources is routed from the device to the CPU.
Quoted 3 lines
Below is my implementation using interrupt-map:
+ - interrupt-parent : phandle to the interrupt map subnode
When using interrupt-parent to point at an interrupt map, I believe you usually just point at the current node; there's no need to a child node.
Review
Quoted 5 lines
(quoted diff) +4
-0+ - interrupts : interrupt mapping for LPS331AP interrupt sources: + 2 sources: 0 - data ready, 1 - threshold event + - irq-map : irq sub-node defining interrupt map + (all properties listed below are required):So, this node isn't required.
Review
Quoted 4 lines
(quoted diff) +3
-0+ - #interrupt-cells : should be 1 + - #address-cells : should be 0 + - #size-cells : should be 0There are no addressed entities in this node, so I don't see why those two properties are needed.
Review
Quoted 6 lines
(quoted diff) +6
-0+ - interrupt-map : table of entries consisting of three child elements: + - unit_interrupt_specifier - 0 : data ready, 1 : threshold event + - interrupt parent phandle + - parent unit interrupt specifier consisiting of two elements: + - index of the interrupt within the controller + - flags : should be 0It's up to the binding for the node referenced by the phandle to define how many cells need be present for "flags", and their meaning. This binding shouldn't attempt to describe those. Equally, the concept of interrupt-map should be defined elsewwere (e.g.
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt); it's a generic that shouldn't need duplication in each binding that uses interrupts.Quoted 17 lines
(quoted diff) +16
-0+Example: + +lps331ap@5d { + compatible = "st,lps331ap"; + reg = <0x5d>; + drdy-int-pin = /bits/ 8 <2>; + interrupt-parent = <&irq_map>; + interrupts = <0>, <1>; + + irq_map: irq-map { + #interrupt-cells = <1>; + #address-cells = <0>; + #size-cells = <0>; + interrupt-map = <0 &gpf0 5 0>; + }; +};Quoted 4 lines
And here is how the driver uses this information:
- if interrupt-map is empty then the driver configures itself to work without interrupt support
The presence or lack of interrupt support should be driven by the presence of the interrupts property. interrupt-map should only be used (if present) to assist in the parsing of the interrupts property.
Quoted 10 lines
- if only one interrupt source is available then the driver configures the device to generate data ready interrupts on the corresponding INTx pin (in this case the driver must know which of the device pins is routed to the cpu - st,data-ready-interrupt-pin property conveys this information)
- if both interrupt sources are available then the driver configures the device to generate data ready interrupts on the interrupt pin corresponding to the interrupt source with index 0 and event interrupts to the interrupt source with index 1.Quoted 5 lines
This solution seems to be a little awkward so I'd like to ask if there is any neater way to handle presented requirements. The solution must facilitate passing information about two interrupt sources two the I2C driver. I have been unable to find similar solution in the kernel so far.
Indeed. I think it would be better to work as follows:
interrupts: contains one or two interrupt specifiers. The first entry always defines the data ready interrupt. The second entry, if present, defines the threshold event interrupt. This at least allows the following combinations to be very simple expressed:
* no interrrupts
* just data
* both data and threshold (assuming they're routed to the same parent)(you could swap the order if it's likely to be more common to have just a threshold interrupt without any data interrupt).
In order to allow the presence of a threshold interrupt but no data interrupt, then I think you would need interrupt-map:
Quoted 8 lines
lps331ap: lps331ap@5d { compatible = "st,lps331ap"; reg = <0x5d>; interrupt-parent = <&lps331ap>; interrupts = <0>, <1>; interrupt-map = <0 0>, /* nowhere */ <1 &gpf0 6 0>; };Quoted 4 lines
The interrupt-names property exists for this purpose (describing interrupts which may or may not be present). Describing a nonexistent interrupt and mapping it nowhere feels like a hack to me when we can describe exactly what's present.
Quoted 7 lines
But the rules for interrupts basically precludes the useful use of interrupt-names.
The interrupts property was introduced long before interrupt-names. As such, the rule was always that entries in interrupts had to appear at a specific index in the property, in other words, the property had to be in a specific order, and there's no way of missing entries out.
Ok, so for bindings that existed before reg-names, and never mentioned reg-names, indexed accesses to reg entries must always be used.
Quoted 9 lines
The interrupt-names property was added much later and more as a documentation for the order in *.dts than as the primary lookup key. Even with an interrupt-names property present, the order of entries in interrupts is still fixed.
So, using interrupt-names doesn't allow you to have optional entries in interrupts, nor re-order the property. We really should not have added interrupt-names, since it gives false impressions.
I'm not sure why that has to be true for new bindings. The new bindings were never used before the existence of reg-names, so it doesn't seem so bad for the new bindings to require the use of reg-names, or to define some optional usage of reg-names.
Quoted 3 lines
For newer bindings such as clocks/clock-names, clock-names is the primary lookup key, so things can be optional.
I don't see why a new class of bindings is that different from new instances of bindings in this regard.
Quoted 5 lines
We should document which properties are purely looked up by index, and which properties have a useful *-names property associated with them as the primary lookup key.
Certainly, but I think this can be more fine-grained than "all clock propeties may use clock-names semantically, all reg properties may not use reg-names semantically".
Thanks,
Mark.
Quoted 140 lines · Mark Rutland
Quoted 123 lines · Stephen Warren
Quoted 117 lines · Mark Rutland
Quoted 107 lines · Stephen Warren
Quoted 9 lines · Jacek Anaszewski
Hi all,
I'd like to consult the implementation of DT binding for the I2C device that exposes two interrupt pins (INT1 and INT2). Both pins can be configured to generate either data ready interrupts or event interrupts. I want to implement DT binding that will handle also the situation when only one of the interrupt sources is routed from the device to the CPU.
Quoted 3 lines
Below is my implementation using interrupt-map:
+ - interrupt-parent : phandle to the interrupt map subnode
When using interrupt-parent to point at an interrupt map, I believe you usually just point at the current node; there's no need to a child node.
Review
Quoted 5 lines
(quoted diff) +4
-0+ - interrupts : interrupt mapping for LPS331AP interrupt sources: + 2 sources: 0 - data ready, 1 - threshold event + - irq-map : irq sub-node defining interrupt map + (all properties listed below are required):So, this node isn't required.
Review
Quoted 4 lines
(quoted diff) +3
-0+ - #interrupt-cells : should be 1 + - #address-cells : should be 0 + - #size-cells : should be 0There are no addressed entities in this node, so I don't see why those two properties are needed.
Review
Quoted 6 lines
(quoted diff) +6
-0+ - interrupt-map : table of entries consisting of three child elements: + - unit_interrupt_specifier - 0 : data ready, 1 : threshold event + - interrupt parent phandle + - parent unit interrupt specifier consisiting of two elements: + - index of the interrupt within the controller + - flags : should be 0It's up to the binding for the node referenced by the phandle to define how many cells need be present for "flags", and their meaning. This binding shouldn't attempt to describe those. Equally, the concept of interrupt-map should be defined elsewwere (e.g.
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt); it's a generic that shouldn't need duplication in each binding that uses interrupts.Quoted 17 lines
(quoted diff) +16
-0+Example: + +lps331ap@5d { + compatible = "st,lps331ap"; + reg = <0x5d>; + drdy-int-pin = /bits/ 8 <2>; + interrupt-parent = <&irq_map>; + interrupts = <0>, <1>; + + irq_map: irq-map { + #interrupt-cells = <1>; + #address-cells = <0>; + #size-cells = <0>; + interrupt-map = <0 &gpf0 5 0>; + }; +};Quoted 4 lines
And here is how the driver uses this information:
- if interrupt-map is empty then the driver configures itself to work without interrupt support
The presence or lack of interrupt support should be driven by the presence of the interrupts property. interrupt-map should only be used (if present) to assist in the parsing of the interrupts property.
Quoted 10 lines
- if only one interrupt source is available then the driver configures the device to generate data ready interrupts on the corresponding INTx pin (in this case the driver must know which of the device pins is routed to the cpu - st,data-ready-interrupt-pin property conveys this information)
- if both interrupt sources are available then the driver configures the device to generate data ready interrupts on the interrupt pin corresponding to the interrupt source with index 0 and event interrupts to the interrupt source with index 1.Quoted 5 lines
This solution seems to be a little awkward so I'd like to ask if there is any neater way to handle presented requirements. The solution must facilitate passing information about two interrupt sources two the I2C driver. I have been unable to find similar solution in the kernel so far.
Indeed. I think it would be better to work as follows:
interrupts: contains one or two interrupt specifiers. The first entry always defines the data ready interrupt. The second entry, if present, defines the threshold event interrupt. This at least allows the following combinations to be very simple expressed:
* no interrrupts
* just data
* both data and threshold (assuming they're routed to the same parent)(you could swap the order if it's likely to be more common to have just a threshold interrupt without any data interrupt).
In order to allow the presence of a threshold interrupt but no data interrupt, then I think you would need interrupt-map:
Quoted 8 lines
lps331ap: lps331ap@5d { compatible = "st,lps331ap"; reg = <0x5d>; interrupt-parent = <&lps331ap>; interrupts = <0>, <1>; interrupt-map = <0 0>, /* nowhere */ <1 &gpf0 6 0>; };Quoted 4 lines
The interrupt-names property exists for this purpose (describing interrupts which may or may not be present). Describing a nonexistent interrupt and mapping it nowhere feels like a hack to me when we can describe exactly what's present.
Quoted 7 lines
But the rules for interrupts basically precludes the useful use of interrupt-names.
The interrupts property was introduced long before interrupt-names. As such, the rule was always that entries in interrupts had to appear at a specific index in the property, in other words, the property had to be in a specific order, and there's no way of missing entries out.
Ok, so for bindings that existed before reg-names, and never mentioned reg-names, indexed accesses to reg entries must always be used.
Quoted 5 lines
The interrupt-names property was added much later and more as a documentation for the order in *.dts than as the primary lookup key. Even with an interrupt-names property present, the order of entries in interrupts is still fixed.
Quoted 8 lines
Quoted 3 lines
So, using interrupt-names doesn't allow you to have optional entries in interrupts, nor re-order the property. We really should not have added interrupt-names, since it gives false impressions.
I'm not sure why that has to be true for new bindings. The new bindings were never used before the existence of reg-names, so it doesn't seem so bad for the new bindings to require the use of reg-names, or to define some optional usage of reg-names.
IIRC, the argument put forth was that OSs may assume a complete lack of the reg-names property and only allow looking up reg values by index, and hence making use of reg-names would prohibit those OSs from supporting new bindings. Although that said, quite why reg-names was then allowed as a property I don't know. IIRC, another argument was that we didn't want to force people into the overhead of parsing via reg-names when a simple index could be used instead, so reg had to always have a defined order even if reg-names was allowed.
On the assumption that reg-names wasn't retro-fitted to any "old" bindings that originally just documents reg and not reg-names, then I suppose we could say:
If a binding defines reg-names, either reg-names or hard-coded indices may be used to find entries in reg. Otherwise, only hard-coded indices may be allowed.
But I'm worried that people will see reg-names and assume they can put entries in arbitrary order...
Quoted 163 lines · Stephen Warren
Quoted 140 lines · Mark Rutland
Quoted 123 lines · Stephen Warren
Quoted 117 lines · Mark Rutland
Quoted 107 lines · Stephen Warren
Quoted 9 lines · Jacek Anaszewski
Hi all,
I'd like to consult the implementation of DT binding for the I2C device that exposes two interrupt pins (INT1 and INT2). Both pins can be configured to generate either data ready interrupts or event interrupts. I want to implement DT binding that will handle also the situation when only one of the interrupt sources is routed from the device to the CPU.
Quoted 3 lines
Below is my implementation using interrupt-map:
+ - interrupt-parent : phandle to the interrupt map subnode
When using interrupt-parent to point at an interrupt map, I believe you usually just point at the current node; there's no need to a child node.
Review
Quoted 5 lines
(quoted diff) +4
-0+ - interrupts : interrupt mapping for LPS331AP interrupt sources: + 2 sources: 0 - data ready, 1 - threshold event + - irq-map : irq sub-node defining interrupt map + (all properties listed below are required):So, this node isn't required.
Review
Quoted 4 lines
(quoted diff) +3
-0+ - #interrupt-cells : should be 1 + - #address-cells : should be 0 + - #size-cells : should be 0There are no addressed entities in this node, so I don't see why those two properties are needed.
Review
Quoted 6 lines
(quoted diff) +6
-0+ - interrupt-map : table of entries consisting of three child elements: + - unit_interrupt_specifier - 0 : data ready, 1 : threshold event + - interrupt parent phandle + - parent unit interrupt specifier consisiting of two elements: + - index of the interrupt within the controller + - flags : should be 0It's up to the binding for the node referenced by the phandle to define how many cells need be present for "flags", and their meaning. This binding shouldn't attempt to describe those. Equally, the concept of interrupt-map should be defined elsewwere (e.g.
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt); it's a generic that shouldn't need duplication in each binding that uses interrupts.Quoted 17 lines
(quoted diff) +16
-0+Example: + +lps331ap@5d { + compatible = "st,lps331ap"; + reg = <0x5d>; + drdy-int-pin = /bits/ 8 <2>; + interrupt-parent = <&irq_map>; + interrupts = <0>, <1>; + + irq_map: irq-map { + #interrupt-cells = <1>; + #address-cells = <0>; + #size-cells = <0>; + interrupt-map = <0 &gpf0 5 0>; + }; +};Quoted 4 lines
And here is how the driver uses this information:
- if interrupt-map is empty then the driver configures itself to work without interrupt support
The presence or lack of interrupt support should be driven by the presence of the interrupts property. interrupt-map should only be used (if present) to assist in the parsing of the interrupts property.
Quoted 10 lines
- if only one interrupt source is available then the driver configures the device to generate data ready interrupts on the corresponding INTx pin (in this case the driver must know which of the device pins is routed to the cpu - st,data-ready-interrupt-pin property conveys this information)
- if both interrupt sources are available then the driver configures the device to generate data ready interrupts on the interrupt pin corresponding to the interrupt source with index 0 and event interrupts to the interrupt source with index 1.Quoted 5 lines
This solution seems to be a little awkward so I'd like to ask if there is any neater way to handle presented requirements. The solution must facilitate passing information about two interrupt sources two the I2C driver. I have been unable to find similar solution in the kernel so far.
Indeed. I think it would be better to work as follows:
interrupts: contains one or two interrupt specifiers. The first entry always defines the data ready interrupt. The second entry, if present, defines the threshold event interrupt. This at least allows the following combinations to be very simple expressed:
* no interrrupts
* just data
* both data and threshold (assuming they're routed to the same parent)(you could swap the order if it's likely to be more common to have just a threshold interrupt without any data interrupt).
In order to allow the presence of a threshold interrupt but no data interrupt, then I think you would need interrupt-map:
Quoted 8 lines
lps331ap: lps331ap@5d { compatible = "st,lps331ap"; reg = <0x5d>; interrupt-parent = <&lps331ap>; interrupts = <0>, <1>; interrupt-map = <0 0>, /* nowhere */ <1 &gpf0 6 0>; };Quoted 4 lines
The interrupt-names property exists for this purpose (describing interrupts which may or may not be present). Describing a nonexistent interrupt and mapping it nowhere feels like a hack to me when we can describe exactly what's present.
Quoted 7 lines
But the rules for interrupts basically precludes the useful use of interrupt-names.
The interrupts property was introduced long before interrupt-names. As such, the rule was always that entries in interrupts had to appear at a specific index in the property, in other words, the property had to be in a specific order, and there's no way of missing entries out.
Ok, so for bindings that existed before reg-names, and never mentioned reg-names, indexed accesses to reg entries must always be used.
Quoted 5 lines
The interrupt-names property was added much later and more as a documentation for the order in *.dts than as the primary lookup key. Even with an interrupt-names property present, the order of entries in interrupts is still fixed.
Quoted 8 lines
Quoted 3 lines
So, using interrupt-names doesn't allow you to have optional entries in interrupts, nor re-order the property. We really should not have added interrupt-names, since it gives false impressions.
I'm not sure why that has to be true for new bindings. The new bindings were never used before the existence of reg-names, so it doesn't seem so bad for the new bindings to require the use of reg-names, or to define some optional usage of reg-names.
IIRC, the argument put forth was that OSs may assume a complete lack of the reg-names property and only allow looking up reg values by index, and hence making use of reg-names would prohibit those OSs from supporting new bindings. Although that said, quite why reg-names was then allowed as a property I don't know. IIRC, another argument was that we didn't want to force people into the overhead of parsing via reg-names when a simple index could be used instead, so reg had to always have a defined order even if reg-names was allowed.
On the assumption that reg-names wasn't retro-fitted to any "old" bindings that originally just documents reg and not reg-names, then I suppose we could say:
Quoted 6 lines
If a binding defines reg-names, either reg-names or hard-coded indices may be used to find entries in reg. Otherwise, only hard-coded indices may be allowed.
But I'm worried that people will see reg-names and assume they can put entries in arbitrary order...
We should fix the binding documents to make clear which bindings require their reg properties at fixed indexes, and those which allow for arbitrarily ordered named reg entries. Named reg entries are really useful for blocks with optional components, and given we have drivers using them, they're already mandatory for some bindings.
Thanks,
Mark.
Quoted 168 lines · Mark Rutland
Quoted 167 lines · Stephen Warren
Quoted 140 lines · Mark Rutland
Quoted 123 lines · Stephen Warren
Quoted 117 lines · Mark Rutland
Quoted 107 lines · Stephen Warren
Quoted 9 lines · Jacek Anaszewski
Hi all,
I'd like to consult the implementation of DT binding for the I2C device that exposes two interrupt pins (INT1 and INT2). Both pins can be configured to generate either data ready interrupts or event interrupts. I want to implement DT binding that will handle also the situation when only one of the interrupt sources is routed from the device to the CPU.
Quoted 3 lines
Below is my implementation using interrupt-map:
+ - interrupt-parent : phandle to the interrupt map subnode
When using interrupt-parent to point at an interrupt map, I believe you usually just point at the current node; there's no need to a child node.
Review
Quoted 5 lines
(quoted diff) +4
-0+ - interrupts : interrupt mapping for LPS331AP interrupt sources: + 2 sources: 0 - data ready, 1 - threshold event + - irq-map : irq sub-node defining interrupt map + (all properties listed below are required):So, this node isn't required.
Review
Quoted 4 lines
(quoted diff) +3
-0+ - #interrupt-cells : should be 1 + - #address-cells : should be 0 + - #size-cells : should be 0There are no addressed entities in this node, so I don't see why those two properties are needed.
Review
Quoted 6 lines
(quoted diff) +6
-0+ - interrupt-map : table of entries consisting of three child elements: + - unit_interrupt_specifier - 0 : data ready, 1 : threshold event + - interrupt parent phandle + - parent unit interrupt specifier consisiting of two elements: + - index of the interrupt within the controller + - flags : should be 0It's up to the binding for the node referenced by the phandle to define how many cells need be present for "flags", and their meaning. This binding shouldn't attempt to describe those. Equally, the concept of interrupt-map should be defined elsewwere (e.g.
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt); it's a generic that shouldn't need duplication in each binding that uses interrupts.Quoted 17 lines
(quoted diff) +16
-0+Example: + +lps331ap@5d { + compatible = "st,lps331ap"; + reg = <0x5d>; + drdy-int-pin = /bits/ 8 <2>; + interrupt-parent = <&irq_map>; + interrupts = <0>, <1>; + + irq_map: irq-map { + #interrupt-cells = <1>; + #address-cells = <0>; + #size-cells = <0>; + interrupt-map = <0 &gpf0 5 0>; + }; +};Quoted 4 lines
And here is how the driver uses this information:
- if interrupt-map is empty then the driver configures itself to work without interrupt support
The presence or lack of interrupt support should be driven by the presence of the interrupts property. interrupt-map should only be used (if present) to assist in the parsing of the interrupts property.
Quoted 10 lines
- if only one interrupt source is available then the driver configures the device to generate data ready interrupts on the corresponding INTx pin (in this case the driver must know which of the device pins is routed to the cpu - st,data-ready-interrupt-pin property conveys this information)
- if both interrupt sources are available then the driver configures the device to generate data ready interrupts on the interrupt pin corresponding to the interrupt source with index 0 and event interrupts to the interrupt source with index 1.Quoted 5 lines
This solution seems to be a little awkward so I'd like to ask if there is any neater way to handle presented requirements. The solution must facilitate passing information about two interrupt sources two the I2C driver. I have been unable to find similar solution in the kernel so far.
Indeed. I think it would be better to work as follows:
interrupts: contains one or two interrupt specifiers. The first entry always defines the data ready interrupt. The second entry, if present, defines the threshold event interrupt. This at least allows the following combinations to be very simple expressed:
* no interrrupts
* just data
* both data and threshold (assuming they're routed to the same parent)(you could swap the order if it's likely to be more common to have just a threshold interrupt without any data interrupt).
In order to allow the presence of a threshold interrupt but no data interrupt, then I think you would need interrupt-map:
Quoted 8 lines
lps331ap: lps331ap@5d { compatible = "st,lps331ap"; reg = <0x5d>; interrupt-parent = <&lps331ap>; interrupts = <0>, <1>; interrupt-map = <0 0>, /* nowhere */ <1 &gpf0 6 0>; };Quoted 4 lines
The interrupt-names property exists for this purpose (describing interrupts which may or may not be present). Describing a nonexistent interrupt and mapping it nowhere feels like a hack to me when we can describe exactly what's present.
Quoted 7 lines
But the rules for interrupts basically precludes the useful use of interrupt-names.
The interrupts property was introduced long before interrupt-names. As such, the rule was always that entries in interrupts had to appear at a specific index in the property, in other words, the property had to be in a specific order, and there's no way of missing entries out.
Ok, so for bindings that existed before reg-names, and never mentioned reg-names, indexed accesses to reg entries must always be used.
Quoted 5 lines
The interrupt-names property was added much later and more as a documentation for the order in *.dts than as the primary lookup key. Even with an interrupt-names property present, the order of entries in interrupts is still fixed.
Quoted 8 lines
Quoted 3 lines
So, using interrupt-names doesn't allow you to have optional entries in interrupts, nor re-order the property. We really should not have added interrupt-names, since it gives false impressions.
I'm not sure why that has to be true for new bindings. The new bindings were never used before the existence of reg-names, so it doesn't seem so bad for the new bindings to require the use of reg-names, or to define some optional usage of reg-names.
IIRC, the argument put forth was that OSs may assume a complete lack of the reg-names property and only allow looking up reg values by index, and hence making use of reg-names would prohibit those OSs from supporting new bindings. Although that said, quite why reg-names was then allowed as a property I don't know. IIRC, another argument was that we didn't want to force people into the overhead of parsing via reg-names when a simple index could be used instead, so reg had to always have a defined order even if reg-names was allowed.
On the assumption that reg-names wasn't retro-fitted to any "old" bindings that originally just documents reg and not reg-names, then I suppose we could say:
If a binding defines reg-names, either reg-names or hard-coded indices may be used to find entries in reg. Otherwise, only hard-coded indices may be allowed.
Quoted 8 lines
Quoted 2 lines
But I'm worried that people will see reg-names and assume they can put entries in arbitrary order...
We should fix the binding documents to make clear which bindings require their reg properties at fixed indexes, and those which allow for arbitrarily ordered named reg entries. Named reg entries are really useful for blocks with optional components, and given we have drivers using them, they're already mandatory for some bindings.
No, we should fix the bindings that have arbitrarily ordered reg properties. There is no obvious reg examples of this that I see from a quick scan of dts files.
There is this questionable use of "empty" for interrupt-names:
arch/arm/boot/dts/imx23.dtsi:
interrupts = <0 14 20 0 13 13 13 13>; interrupt-names = "empty", "ssp0", "ssp1", "empty", "gpmi0", "gpmi1", "gpmi2", "gpmi3";
Rob
Quoted 1 line · Rob Herring
On 08/21/2013 03:54 AM, Mark Rutland wrote:
...
Quoted 10 lines
Quoted 5 lines
We should fix the binding documents to make clear which bindings require their reg properties at fixed indexes, and those which allow for arbitrarily ordered named reg entries. Named reg entries are really useful for blocks with optional components, and given we have drivers using them, they're already mandatory for some bindings.
No, we should fix the bindings that have arbitrarily ordered reg properties. There is no obvious reg examples of this that I see from a quick scan of dts files.
Quoted 8 lines
There is this questionable use of "empty" for interrupt-names:
arch/arm/boot/dts/imx23.dtsi:interrupts = <0 14 20 0 13 13 13 13>; interrupt-names = "empty", "ssp0", "ssp1", "empty", "gpmi0", "gpmi1", "gpmi2", "gpmi3";
Well, there's been an assertion that bindings that define an ordered interrupts property shouldn't have a interrupt-names property, and bindings that define interrupt-names entries explicitly don't require a specific order of entries in the interrupts property.
I assume from your statement, you don't agree, and think that interrupts must always be in a specific order, even if interrupt-names exists?
If so, that'd make interrupt-names at least a bit useless and at worst horribly misleading, since people will assume it defines the order and that the order can be arbitrary.
Quoted 117 lines · Mark Rutland
Quoted 107 lines · Stephen Warren
Quoted 9 lines · Jacek Anaszewski
Hi all,
I'd like to consult the implementation of DT binding for the I2C device that exposes two interrupt pins (INT1 and INT2). Both pins can be configured to generate either data ready interrupts or event interrupts. I want to implement DT binding that will handle also the situation when only one of the interrupt sources is routed from the device to the CPU.
Quoted 3 lines
Below is my implementation using interrupt-map:
+ - interrupt-parent : phandle to the interrupt map subnode
When using interrupt-parent to point at an interrupt map, I believe you usually just point at the current node; there's no need to a child node.
Review
Quoted 5 lines
(quoted diff) +4
-0+ - interrupts : interrupt mapping for LPS331AP interrupt sources: + 2 sources: 0 - data ready, 1 - threshold event + - irq-map : irq sub-node defining interrupt map + (all properties listed below are required):So, this node isn't required.
Review
Quoted 4 lines
(quoted diff) +3
-0+ - #interrupt-cells : should be 1 + - #address-cells : should be 0 + - #size-cells : should be 0There are no addressed entities in this node, so I don't see why those two properties are needed.
Review
Quoted 6 lines
(quoted diff) +6
-0+ - interrupt-map : table of entries consisting of three child elements: + - unit_interrupt_specifier - 0 : data ready, 1 : threshold event + - interrupt parent phandle + - parent unit interrupt specifier consisiting of two elements: + - index of the interrupt within the controller + - flags : should be 0It's up to the binding for the node referenced by the phandle to define how many cells need be present for "flags", and their meaning. This binding shouldn't attempt to describe those. Equally, the concept of interrupt-map should be defined elsewwere (e.g.
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt); it's a generic that shouldn't need duplication in each binding that uses interrupts.Quoted 17 lines
(quoted diff) +16
-0+Example: + +lps331ap@5d { + compatible = "st,lps331ap"; + reg =<0x5d>; + drdy-int-pin = /bits/ 8<2>; + interrupt-parent =<&irq_map>; + interrupts =<0>,<1>; + + irq_map: irq-map { + #interrupt-cells =<1>; + #address-cells =<0>; + #size-cells =<0>; + interrupt-map =<0&gpf0 5 0>; + }; +};Quoted 4 lines
And here is how the driver uses this information:
- if interrupt-map is empty then the driver configures itself to work without interrupt support
The presence or lack of interrupt support should be driven by the presence of the interrupts property. interrupt-map should only be used (if present) to assist in the parsing of the interrupts property.
Quoted 10 lines
- if only one interrupt source is available then the driver configures the device to generate data ready interrupts on the corresponding INTx pin (in this case the driver must know which of the device pins is routed to the cpu - st,data-ready-interrupt-pin property conveys this information)
- if both interrupt sources are available then the driver configures the device to generate data ready interrupts on the interrupt pin corresponding to the interrupt source with index 0 and event interrupts to the interrupt source with index 1.Quoted 5 lines
This solution seems to be a little awkward so I'd like to ask if there is any neater way to handle presented requirements. The solution must facilitate passing information about two interrupt sources two the I2C driver. I have been unable to find similar solution in the kernel so far.
Indeed. I think it would be better to work as follows:
interrupts: contains one or two interrupt specifiers. The first entry always defines the data ready interrupt. The second entry, if present, defines the threshold event interrupt. This at least allows the following combinations to be very simple expressed:
* no interrrupts
* just data
* both data and threshold (assuming they're routed to the same parent)(you could swap the order if it's likely to be more common to have just a threshold interrupt without any data interrupt).
In order to allow the presence of a threshold interrupt but no data interrupt, then I think you would need interrupt-map:
Quoted 8 lines
lps331ap: lps331ap@5d { compatible = "st,lps331ap"; reg =<0x5d>; interrupt-parent =<&lps331ap>; interrupts =<0>,<1>; interrupt-map = <0 0>, /* nowhere */ <1&gpf0 6 0>; };
Quoted 7 lines
The interrupt-names property exists for this purpose (describing interrupts which may or may not be present). Describing a nonexistent interrupt and mapping it nowhere feels like a hack to me when we can describe exactly what's present.
Thanks,
Mark.
From what I've figured out in order to obtain the interrupt id by name the function
platform_get_irq_by_name(struct platform_device *dev, const char *name)
has to be exploited. Unfortunately required platform_device structure is not available in the struct i2c_client that is passed to the probe function of an i2c driver. Is there some different way to parse the interrupt-names property from the I2C driver?
Thanks,
Jacek
Quoted 7 lines · Jacek Anaszewski
From what I've figured out in order to obtain the interrupt id
by name the function
platform_get_irq_by_name(struct platform_device *dev, const char *name)has to be exploited. Unfortunately required
platform_devicestructure is not available in the structi2c_client
Well, platform devices belong to platform bus, nothing to do with I2C. Platform devices are associated with an array of named resources and the code creating such devices from the DT nodes (of_device_alloc()) do the necessary magic.
Quoted 3 lines
that is passed to the
probe function of an i2c driver. Is there some different way to parse the interrupt-names property from the I2C driver?
Interesting. Very interesting indeed. It seems (at least at the first sight) that I2C framework assumes that only one interrupt can be generated by a I2C device:
struct i2c_client {
[...]
int irq; /* irq issued by device */
[...]
};
So let me ask such question... If Device Tree didn't exist, how would you make drive such device? I guess it would require some custom code, as struct i2c_board_info has single int irq as well... If this is the case, you could parse the irq properties in a similar way as done in of_irq_to_resource() in drivers/of/irq.c. Doesn't like the optimal solution, though...
I've copied Wolfram and the I2C mailing list in hope of them shedding some light at the matter.
Pawel
[resending, this time really copying Wolfram and linux-i2c...]
Quoted 7 lines · Jacek Anaszewski
From what I've figured out in order to obtain the interrupt id
by name the function
platform_get_irq_by_name(struct platform_device *dev, const char *name)has to be exploited. Unfortunately required
platform_devicestructure is not available in the structi2c_client
Well, platform devices belong to platform bus, nothing to do with I2C. Platform devices are associated with an array of named resources and the code creating such devices from the DT nodes (of_device_alloc()) do the necessary magic.
Quoted 3 lines
that is passed to the
probe function of an i2c driver. Is there some different way to parse the interrupt-names property from the I2C driver?
Interesting. Very interesting indeed. It seems (at least at the first sight) that I2C framework assumes that only one interrupt can be generated by a I2C device:
struct i2c_client {
[...]
int irq; /* irq issued by device */
[...]
};
So let me ask such question... If Device Tree didn't exist, how would you make drive such device? I guess it would require some custom code, as struct i2c_board_info has single int irq as well... If this is the case, you could parse the irq properties in a similar way as done in of_irq_to_resource() in drivers/of/irq.c. Doesn't like the optimal solution, though...
I've copied Wolfram and the I2C mailing list in hope of them shedding some light at the matter.
Pawel
Quoted 2 lines · Pawel Moll
So let me ask such question... If Device Tree didn't exist, how would you make drive such device? I guess it would require some custom code,
It's always done using platform data, same for SPI - if we update one we should probably update both.
Quoted 7 lines · Mark Brown
Quoted 2 lines · Pawel Moll
So let me ask such question... If Device Tree didn't exist, how would you make drive such device? I guess it would require some custom code,
It's always done using platform data, same for SPI - if we update one we should probably update both.
If the platform data used to carry the (custom) irq data, the DT-powered driver could interrogate the DT on is own, couldn't it? Of course there should be some helper available, maybe something of that sort? (warning, untested)
8<---------------------
drivers/of/irq.c +9 -0
@@ -46,6 +46,15 @@ unsigned int irq_of_parse_and_map(struct device_node *dev, int index) }EXPORT_SYMBOL_GPL (irq_of_parse_and_map); +unsigned int irq_of_parse_and_map_by_name (struct device_node *dev, +const char *name) +{ +int index =of_property_match_string (dev,"interrupt-names" , name); + +return index <0 ?0 :irq_of_parse_and_map (dev, index); +} +EXPORT_SYMBOL_GPL (irq_of_parse_and_map_by_name); +/** * of_irq_find_parent - Given a device node, find its interrupt parent node * @child: pointer to device node
8<---------------------
Pawel
Quoted 4 lines · Pawel Moll
If the platform data used to carry the (custom) irq data, the DT-powered driver could interrogate the DT on is own, couldn't it? Of course there should be some helper available, maybe something of that sort? (warning, untested)
Yes, that's probably the most straightforward thing - we'd need to either have the bindings specify which interrupt must be first for reading i2c->irq or just have the drivers always do a name based lookup if there's more than one interrupt.
Quoted 11 lines · Mark Brown
Quoted 4 lines · Pawel Moll
If the platform data used to carry the (custom) irq data, the DT-powered driver could interrogate the DT on is own, couldn't it? Of course there should be some helper available, maybe something of that sort? (warning, untested)
Yes, that's probably the most straightforward thing - we'd need to either have the bindings specify which interrupt must be first for reading
i2c->irqor just have the drivers always do a name based lookup if there's more than one interrupt.
... or make sure that of_i2c_register_devices() does *not* set i2c->irq (or rather: set it to 0) when there is more than one interrupt in the tree...
Paweł
Quoted 10 lines · Pawel Moll
Quoted 4 lines · Mark Brown
Yes, that's probably the most straightforward thing - we'd need to either have the bindings specify which interrupt must be first for reading
i2c->irqor just have the drivers always do a name based lookup if there's more than one interrupt.... or make sure that
of_i2c_register_devices()does *not* seti2c->irq(or rather: set it to 0) when there is more than one interrupt in the tree...
Yes, that'd be a really good idea if the drivers are always getting the interrupts by name, provides a backup.