On 01/06/15 20:24, Stephen Warren wrote:
> On 05/29/2015 09:50 AM, Jon Hunter wrote:
>>
>> On 22/05/15 15:37, Thierry Reding wrote:
>>> I'd still clearly prefer to have the pinctrl code live directly in the
>>> DPAUX driver, so I think we should at least give that a shot.
>>
>> I have been working on this more this week and the good news is that by
>> using some of the pinconf-generic handlers I can simplify the code and
>> avoid moving headers and structures around.
>>
>> However, I have ran into another issue. The current binding looks like
>> this ...
>>
>> dpaux: dpaux@0,545c0000 {
>> compatible = "nvidia,tegra124-dpaux";
>> reg = <0x0 0x545c0000 0x0 0x40000>;
>> interrupts = <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&tegra_car TEGRA124_CLK_DPAUX>,
>> <&tegra_car TEGRA124_CLK_PLL_DP>;
>> clock-names = "dpaux", "parent";
>> resets = <&tegra_car 181>;
>> reset-names = "dpaux";
>> status = "disabled";
>>
>> dpaux_state: dpaux_state0 {
>> dpaux {
>> groups = "dpaux_io";
>> function = "dpaux";
>> nvidia,dpaux-drvi;
>> nvidia,dpaux-drvz;
>> nvidia,dpaux-cmh;
>> };
>> };
>>
>> i2c_state: i2c_state0 {
>> i2c {
>> groups = "dpaux_io";
>> function = "i2c";
>> };
>> };
>>
>> This all works well, however, because the display-port binding now has
>> child devices which are not i2c clients I see the following messages
>> during kernel boot ...
>>
>> [ 1.607606] i2c i2c-6: of_i2c: modalias failure on
>> /host1x@0,50000000/dpaux@0,545c0000/dpaux_state0
>> [ 1.616658] i2c i2c-6: of_i2c: modalias failure on
>> /host1x@0,50000000/dpaux@0,545c0000/i2c_state0
>
> Hmm. The DT binding doc for dpaux doesn't say anything about the device
> being an I2C controller and hence inheriting/aggregating the core I2C
> schema. It should...
Yes that would definitely be clearer.
> Equally, being an I2C controller means the node should have
> #address-cells/#size-cells properties for the I2C address.
Agree.
>> In other words, i2c_add_adapter() (which is called by probing the dpaux)
>> then parses the child nodes looking for i2c client devices. However,
>> these child devices are not i2c client devices and hence the above error
>> messages. I can't find an easy way to avoid this. There is no
>> side-effect from these messages, but I would prefer not to see them.
>
> If this were a completely new binding, I think the best way would be to
> have the dpaux node contain a child node for each semantic service
> implemented by the device, e.g.:
>
> dpaux {
> compatible = "nvidia,tegra124-dpaux";
> ...
> pinctrl {
> dpaux_state: dpaux_state0 {
> ...
> i2c_state: i2c_state0 {
> ...
> };
> i2c-bus {
> // i2c devices go here
> // I2C core pointed at this sub-node, not the dpaux node
> };
> };
>
> I guess we can't change the binding this way since it already exists,
> unless we introduce a new compatible value to distinguish the old and
> new styles.
>
> Perhaps i2c_add_adapter should only attempt to instantiate devices for
> sub-nodes that contain a compatible and/or a reg property?
It does. However, because I tried to add the pinctrl mappings as
sub-nodes, this causes the i2c-core to dump error messages during
initialisation of the dpaux driver, because these child nodes are not
i2c devices. So what I have works, but I see these error messages from
the i2c-core, which in this case are benign.
Therefore, to avoid the i2c error messages, the i2c-bus and pinctrl
nodes need to be sub-nodes like you have above. I like your above
proposal, but like you said, it does mean adding a new compatibility
string not to break existing dtbs.
Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
