| Message ID | 20161130115005.11508-1-l.stach@pengutronix.de |
|---|---|
| State | Rejected |
| Headers | show |
On Wed, Nov 30, 2016 at 12:50:05PM +0100, Lucas Stach wrote: > DDC buses are manually managed by their consumers to communicate > with the display. There is no need to try to populate OF childs. s/childs/children/ (in sum 3 times) Best regards Uwe
Hello Lucas, On 11/30/2016 01:50 PM, Lucas Stach wrote: > DDC buses are manually managed by their consumers to communicate > with the display. There is no need to try to populate OF childs. > > This gets rid of the device create failed warning caused by the > core trying to populate a DDC bus below a OF device, which has > other childs nodes, that aren't i2c devices. what kind of devices on a DDC bus represended by children nodes do you reference here? -- With best wishes, Vladimir -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am Mittwoch, den 30.11.2016, 15:54 +0200 schrieb Vladimir Zapolskiy: > Hello Lucas, > > On 11/30/2016 01:50 PM, Lucas Stach wrote: > > DDC buses are manually managed by their consumers to communicate > > with the display. There is no need to try to populate OF childs. > > > > This gets rid of the device create failed warning caused by the > > core trying to populate a DDC bus below a OF device, which has > > other childs nodes, that aren't i2c devices. > > what kind of devices on a DDC bus represended by children nodes > do you reference here? None. The device registering the DDC i2c adapter might have an of_node. The children of this device are not i2c devices, but for example port nodes for the of_graph binding. The same issue can be worked around if we make it explicit by placing an "i2c-bus" subnode with no children into the parent device OF node. But as the OF probing of devices on a DDC bus just doesn't make sense I figured it would be good to just skip all this. Regards, Lucas -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/30/2016 04:06 PM, Lucas Stach wrote: > Am Mittwoch, den 30.11.2016, 15:54 +0200 schrieb Vladimir Zapolskiy: >> Hello Lucas, >> >> On 11/30/2016 01:50 PM, Lucas Stach wrote: >>> DDC buses are manually managed by their consumers to communicate >>> with the display. There is no need to try to populate OF childs. >>> >>> This gets rid of the device create failed warning caused by the >>> core trying to populate a DDC bus below a OF device, which has >>> other childs nodes, that aren't i2c devices. >> >> what kind of devices on a DDC bus represended by children nodes >> do you reference here? > > None. The device registering the DDC i2c adapter might have an of_node. > The children of this device are not i2c devices, but for example port > nodes for the of_graph binding. Port node shall not be a child of any DDC bus device node, otherwise it contradicts to the hierarchy of hardware blocks. It is a common practice to describe port nodes and DDC bus adapter as siblings (devices which share the same display controller / encoder device). Do I miss something? > The same issue can be worked around if we make it explicit by placing an > "i2c-bus" subnode with no children into the parent device OF node. But > as the OF probing of devices on a DDC bus just doesn't make sense I > figured it would be good to just skip all this. > Right, as a micro optimization the change makes sense (*), but the second paragraph in the commit message is questionable. In my opinion it makes sense to include the change, and the warning you mention in the commit message needs its own attention as an indicator of potentially wrongly chosen representation of the device hierarchy. (*) experimentally I successfully performed test communications with AT24 connected to DW HDMI DDC lines on i.MX6Q, so I can imagine exotic boards, where DDC bus operates as an extra I2C bus. I'm unaware of any such real cases, but if they exist your commit breaks them. -- With best wishes, Vladimir -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am Mittwoch, den 30.11.2016, 22:18 +0200 schrieb Vladimir Zapolskiy: > On 11/30/2016 04:06 PM, Lucas Stach wrote: > > Am Mittwoch, den 30.11.2016, 15:54 +0200 schrieb Vladimir Zapolskiy: > >> Hello Lucas, > >> > >> On 11/30/2016 01:50 PM, Lucas Stach wrote: > >>> DDC buses are manually managed by their consumers to communicate > >>> with the display. There is no need to try to populate OF childs. > >>> > >>> This gets rid of the device create failed warning caused by the > >>> core trying to populate a DDC bus below a OF device, which has > >>> other childs nodes, that aren't i2c devices. > >> > >> what kind of devices on a DDC bus represended by children nodes > >> do you reference here? > > > > None. The device registering the DDC i2c adapter might have an of_node. > > The children of this device are not i2c devices, but for example port > > nodes for the of_graph binding. > > Port node shall not be a child of any DDC bus device node, otherwise it > contradicts to the hierarchy of hardware blocks. It is a common practice > to describe port nodes and DDC bus adapter as siblings (devices which > share the same display controller / encoder device). Do I miss something? > > > The same issue can be worked around if we make it explicit by placing an > > "i2c-bus" subnode with no children into the parent device OF node. But > > as the OF probing of devices on a DDC bus just doesn't make sense I > > figured it would be good to just skip all this. > > > > Right, as a micro optimization the change makes sense (*), but the second > paragraph in the commit message is questionable. > > In my opinion it makes sense to include the change, and the warning > you mention in the commit message needs its own attention as an indicator > of potentially wrongly chosen representation of the device hierarchy. > Okay to get away from the purely theoretical speaking here: the issue this patch fixes is seen with the tc358767 parallel-to-eDP bridge. (Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.txt) The bridge itself is represented in the DT, so it has a OF node. It then goes on to register the i2c over DP AUX channel i2c adapter with its own device node, which is what all consumers of the i2c_add_adapter() API so AFAICS. The i2c adapter has no own representation in the DT, as it's a pure software construct, after all it's just an emulated i2c bus over DP, there are no physical i2c wires. The i2c adapter then tries to populate the bridges children as if they were i2c devices, which is clearly wrong. We could fix this by creating an own device for the i2c adapter, but that's not how things are handled today and would require changes to all consumers of this API. Regards, Lucas -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 5ab67219f71e..fbf7aade2ca7 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -1686,6 +1686,10 @@ static void of_i2c_register_devices(struct i2c_adapter *adap) if (!adap->dev.of_node) return; + /* DDC buses have no OF populated childs */ + if (adap->class == I2C_CLASS_DDC) + return; + dev_dbg(&adap->dev, "of_i2c: walking child nodes\n"); bus = of_get_child_by_name(adap->dev.of_node, "i2c-bus");
DDC buses are manually managed by their consumers to communicate with the display. There is no need to try to populate OF childs. This gets rid of the device create failed warning caused by the core trying to populate a DDC bus below a OF device, which has other childs nodes, that aren't i2c devices. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- drivers/i2c/i2c-core.c | 4 ++++ 1 file changed, 4 insertions(+)