[PATCH] ARM: dts: Add ddc i2c reference to veyron

[PATCH] ARM: dts: Add ddc i2c reference to veyron

[Douglas Anderson]

The ddc-i2c-bus property was missing from the veyron dtsi file since
downstream the ddc-i2c-bus was still being specified in rk3288.dtsi and
nobody noticed when the veyron dtsi was sent upstream.  Add it.

Signed-off-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Note: I noticed that this was wrong but I don't currently have
graphics up and running on upstream on veyron.  Posting this anyway
since it's pretty clear that it's needed.  If someone else wants to
try it out that'd be nice, otherwise I'll put it on my list to figure
out how to get myself setup for graphics upstream.  ;)

 arch/arm/boot/dts/rk3288-veyron.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi b/arch/arm/boot/dts/rk3288-veyron.dtsi
index 2fa7a0d..275c78c 100644
--- a/arch/arm/boot/dts/rk3288-veyron.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
@@ -158,6 +158,7 @@
 };
 
 &hdmi {
+	ddc-i2c-bus = <&i2c5>;
 	status = "okay";
 };
 
-- 
2.5.0.457.gab17608

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: Add ddc i2c reference to veyron

[Rob Herring]

On Wed, Sep 2, 2015 at 4:25 PM, Douglas Anderson <dianders@chromium.org> wrote:
> The ddc-i2c-bus property was missing from the veyron dtsi file since
> downstream the ddc-i2c-bus was still being specified in rk3288.dtsi and
> nobody noticed when the veyron dtsi was sent upstream.  Add it.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> Note: I noticed that this was wrong but I don't currently have
> graphics up and running on upstream on veyron.  Posting this anyway
> since it's pretty clear that it's needed.  If someone else wants to
> try it out that'd be nice, otherwise I'll put it on my list to figure
> out how to get myself setup for graphics upstream.  ;)

Based on your other patch, this is temporary, right?

I've been looking at DRM a lot lately. I think specifying the i2c bus
in the hdmi chip or IP block node is wrong. If the I2C host is
separate from the HDMI block, then it's only connection is to the HDMI
connector. So the I2C host to the connector relationship is what the
DT should describe. HPD gpio is similar. Now if the HDMI bridge
controls DDC and HPD directly, then we don't need to describe those
connections.

Rob

>
>  arch/arm/boot/dts/rk3288-veyron.dtsi | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi b/arch/arm/boot/dts/rk3288-veyron.dtsi
> index 2fa7a0d..275c78c 100644
> --- a/arch/arm/boot/dts/rk3288-veyron.dtsi
> +++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
> @@ -158,6 +158,7 @@
>  };
>
>  &hdmi {
> +       ddc-i2c-bus = <&i2c5>;
>         status = "okay";
>  };
>
> --
> 2.5.0.457.gab17608
>

Re: [PATCH] ARM: dts: Add ddc i2c reference to veyron

[Doug Anderson]

Rob,

On Wed, Sep 2, 2015 at 5:13 PM, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Sep 2, 2015 at 4:25 PM, Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>> The ddc-i2c-bus property was missing from the veyron dtsi file since
>> downstream the ddc-i2c-bus was still being specified in rk3288.dtsi and
>> nobody noticed when the veyron dtsi was sent upstream.  Add it.
>>
>> Signed-off-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> ---
>> Note: I noticed that this was wrong but I don't currently have
>> graphics up and running on upstream on veyron.  Posting this anyway
>> since it's pretty clear that it's needed.  If someone else wants to
>> try it out that'd be nice, otherwise I'll put it on my list to figure
>> out how to get myself setup for graphics upstream.  ;)
>
> Based on your other patch, this is temporary, right?

Yes, though since I'm not personally working on the other patch series
upstream I can't say for how long the "temporary" is..  I mostly
posted the 2nd patch because it was clearly correct to add some
pinmuxing states and could land any time, so I thought I'd be helpful.

You're right that in the Chrome OS tree I turned right around and
effectively removed the "ddc-i2c-bus", but having it land first adds a
much better logical progression (make it the same as everyone else and
_then_ change it).  It also provides a revert path if something goes
wrong.  :)


> I've been looking at DRM a lot lately. I think specifying the i2c bus
> in the hdmi chip or IP block node is wrong. If the I2C host is
> separate from the HDMI block, then it's only connection is to the HDMI
> connector. So the I2C host to the connector relationship is what the
> DT should describe. HPD gpio is similar. Now if the HDMI bridge
> controls DDC and HPD directly, then we don't need to describe those
> connections.

I will say that I know very very little about DRM.  Mostly I just
visit it when there's some bug I'm running into that I can't find a
better suited owner for.  ;)

I'm not sure I followed your whole paragraph.  Could you give a
fragment of DTS for how you'd imagine this ought to work?  Also: the
patch I submitted does match the current bindings if I understand it
right.  ...as is typical with device tree, if we want to change the
bindings we've got to have a really good reason because we'd either
need to figure out how to deal with existing DTBs in the field that
need to run with newer kernels (if those exist).

-Doug
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: Add ddc i2c reference to veyron

[Rob Herring]

On Wed, Sep 2, 2015 at 7:22 PM, Doug Anderson <dianders@chromium.org> wrote:
> Rob,
>
> On Wed, Sep 2, 2015 at 5:13 PM, Rob Herring <robherring2@gmail.com> wrote:
>> On Wed, Sep 2, 2015 at 4:25 PM, Douglas Anderson <dianders@chromium.org> wrote:
>>> The ddc-i2c-bus property was missing from the veyron dtsi file since
>>> downstream the ddc-i2c-bus was still being specified in rk3288.dtsi and
>>> nobody noticed when the veyron dtsi was sent upstream.  Add it.
>>>
>>> Signed-off-by: Douglas Anderson <dianders@chromium.org>
>>> ---
>>> Note: I noticed that this was wrong but I don't currently have
>>> graphics up and running on upstream on veyron.  Posting this anyway
>>> since it's pretty clear that it's needed.  If someone else wants to
>>> try it out that'd be nice, otherwise I'll put it on my list to figure
>>> out how to get myself setup for graphics upstream.  ;)
>>
>> Based on your other patch, this is temporary, right?
>
> Yes, though since I'm not personally working on the other patch series
> upstream I can't say for how long the "temporary" is..  I mostly
> posted the 2nd patch because it was clearly correct to add some
> pinmuxing states and could land any time, so I thought I'd be helpful.
>
> You're right that in the Chrome OS tree I turned right around and
> effectively removed the "ddc-i2c-bus", but having it land first adds a
> much better logical progression (make it the same as everyone else and
> _then_ change it).  It also provides a revert path if something goes
> wrong.  :)
>
>
>> I've been looking at DRM a lot lately. I think specifying the i2c bus
>> in the hdmi chip or IP block node is wrong. If the I2C host is
>> separate from the HDMI block, then it's only connection is to the HDMI
>> connector. So the I2C host to the connector relationship is what the
>> DT should describe. HPD gpio is similar. Now if the HDMI bridge
>> controls DDC and HPD directly, then we don't need to describe those
>> connections.
>
> I will say that I know very very little about DRM.  Mostly I just
> visit it when there's some bug I'm running into that I can't find a
> better suited owner for.  ;)
>
> I'm not sure I followed your whole paragraph.  Could you give a
> fragment of DTS for how you'd imagine this ought to work?  Also: the
> patch I submitted does match the current bindings if I understand it
> right.  ...as is typical with device tree, if we want to change the
> bindings we've got to have a really good reason because we'd either
> need to figure out how to deal with existing DTBs in the field that
> need to run with newer kernels (if those exist).
>
> -Doug

Re: [PATCH] ARM: dts: Add ddc i2c reference to veyron

[Russell King - ARM Linux]

On Wed, Sep 02, 2015 at 07:13:24PM -0500, Rob Herring wrote:
> On Wed, Sep 2, 2015 at 4:25 PM, Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > The ddc-i2c-bus property was missing from the veyron dtsi file since
> > downstream the ddc-i2c-bus was still being specified in rk3288.dtsi and
> > nobody noticed when the veyron dtsi was sent upstream.  Add it.
> >
> > Signed-off-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > ---
> > Note: I noticed that this was wrong but I don't currently have
> > graphics up and running on upstream on veyron.  Posting this anyway
> > since it's pretty clear that it's needed.  If someone else wants to
> > try it out that'd be nice, otherwise I'll put it on my list to figure
> > out how to get myself setup for graphics upstream.  ;)
> 
> Based on your other patch, this is temporary, right?
> 
> I've been looking at DRM a lot lately. I think specifying the i2c bus
> in the hdmi chip or IP block node is wrong. If the I2C host is
> separate from the HDMI block, then it's only connection is to the HDMI
> connector. So the I2C host to the connector relationship is what the
> DT should describe. HPD gpio is similar. Now if the HDMI bridge
> controls DDC and HPD directly, then we don't need to describe those
> connections.

Except... we don't generally model connectors under DRM as a general
rule.  (The fbdev video connector stuff happened without very much
publicity afaics.)

It's not always appropriate to split it out from the bridge in any
case.  Consider something like a TDA998x where the TDA998x itself
takes care of reading the DDC bus, and doesn't provide an I2C-like
interface.  If you try and split that into "bridge" or "encoder" and
"connector" you end up having to invent a new kind of I2C thing which
isn't an I2C adapter, or somehow squeezing an I2C adapter which isn't
into the I2C layer.

The TDA998x provides an interface to read a block of EDID at a time.
It always does the page register access.  You don't get to read it
byte wise.  It doesn't fit into I2C as an adapter at all.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: Add ddc i2c reference to veyron

[Rob Herring]

On Thu, Sep 3, 2015 at 3:22 AM, Russell King - ARM Linux
<linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> wrote:
> On Wed, Sep 02, 2015 at 07:13:24PM -0500, Rob Herring wrote:
>> On Wed, Sep 2, 2015 at 4:25 PM, Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>> > The ddc-i2c-bus property was missing from the veyron dtsi file since
>> > downstream the ddc-i2c-bus was still being specified in rk3288.dtsi and
>> > nobody noticed when the veyron dtsi was sent upstream.  Add it.
>> >
>> > Signed-off-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> > ---
>> > Note: I noticed that this was wrong but I don't currently have
>> > graphics up and running on upstream on veyron.  Posting this anyway
>> > since it's pretty clear that it's needed.  If someone else wants to
>> > try it out that'd be nice, otherwise I'll put it on my list to figure
>> > out how to get myself setup for graphics upstream.  ;)
>>
>> Based on your other patch, this is temporary, right?
>>
>> I've been looking at DRM a lot lately. I think specifying the i2c bus
>> in the hdmi chip or IP block node is wrong. If the I2C host is
>> separate from the HDMI block, then it's only connection is to the HDMI
>> connector. So the I2C host to the connector relationship is what the
>> DT should describe. HPD gpio is similar. Now if the HDMI bridge
>> controls DDC and HPD directly, then we don't need to describe those
>> connections.
>
> Except... we don't generally model connectors under DRM as a general
> rule.  (The fbdev video connector stuff happened without very much
> publicity afaics.)

True. We have a binding with no users in the kernel. I think we should
move in the direction of using it though. After all, it should not be
a question of fbdev vs. DRM support for the binding. Connectors are
certainly a concept within DRM, but they are very closely tied to
encoder/bridge drivers ATM. We should be able to separate them to
handle the common cases, but we have to have some consistency across
DT bindings to do that. I don't yet have more concrete proposals
though.

> It's not always appropriate to split it out from the bridge in any
> case.  Consider something like a TDA998x where the TDA998x itself
> takes care of reading the DDC bus, and doesn't provide an I2C-like
> interface.  If you try and split that into "bridge" or "encoder" and
> "connector" you end up having to invent a new kind of I2C thing which
> isn't an I2C adapter, or somehow squeezing an I2C adapter which isn't
> into the I2C layer.

Yes, that is fairly common (ADV75xx is same), and we would not
describe an I2C bus in DT in that case. Same with HPD directly handled
vs. a GPIO line. That is no different than what Doug has said:
ddc-i2c-bus is present if using the SOC's I2C host and absent if using
the HDMI block's DDC functionality. I'm only questioning the location
of the property.

Even if the bridge handles DDC and HPD, there is also regulator
control for the connector power supply. That's most likely never going
to be part of the bridge.

> The TDA998x provides an interface to read a block of EDID at a time.
> It always does the page register access.  You don't get to read it
> byte wise.  It doesn't fit into I2C as an adapter at all.

Agreed.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: Add ddc i2c reference to veyron

[Russell King - ARM Linux]

On Thu, Sep 03, 2015 at 09:46:38AM -0500, Rob Herring wrote:
> Yes, that is fairly common (ADV75xx is same), and we would not
> describe an I2C bus in DT in that case. Same with HPD directly handled
> vs. a GPIO line. That is no different than what Doug has said:
> ddc-i2c-bus is present if using the SOC's I2C host and absent if using
> the HDMI block's DDC functionality. I'm only questioning the location
> of the property.

No, I don't think that's what Doug wants.  Doug wants the bridge's
internal I2C host to be exposed, so he can number it through a DT
alias.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH] ARM: dts: Add ddc i2c reference to veyron

[Rob Herring]

On Thu, Sep 3, 2015 at 10:18 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Sep 03, 2015 at 09:46:38AM -0500, Rob Herring wrote:
>> Yes, that is fairly common (ADV75xx is same), and we would not
>> describe an I2C bus in DT in that case. Same with HPD directly handled
>> vs. a GPIO line. That is no different than what Doug has said:
>> ddc-i2c-bus is present if using the SOC's I2C host and absent if using
>> the HDMI block's DDC functionality. I'm only questioning the location
>> of the property.
>
> No, I don't think that's what Doug wants.  Doug wants the bridge's
> internal I2C host to be exposed, so he can number it through a DT
> alias.

See his earlier reply and other patch[1] which states once the dw_hdmi
built-in I2C controller support is added in mainline, then this
property is not needed. For now, the SOC's general purpose I2C
controller is used.

Rob

[1] https://lkml.org/lkml/2015/9/2/571

Re: [PATCH] ARM: dts: Add ddc i2c reference to veyron

[Doug Anderson]

Hi,

On Thu, Sep 3, 2015 at 8:46 AM, Rob Herring <robherring2@gmail.com> wrote:
> On Thu, Sep 3, 2015 at 10:18 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Thu, Sep 03, 2015 at 09:46:38AM -0500, Rob Herring wrote:
>>> Yes, that is fairly common (ADV75xx is same), and we would not
>>> describe an I2C bus in DT in that case. Same with HPD directly handled
>>> vs. a GPIO line. That is no different than what Doug has said:
>>> ddc-i2c-bus is present if using the SOC's I2C host and absent if using
>>> the HDMI block's DDC functionality. I'm only questioning the location
>>> of the property.
>>
>> No, I don't think that's what Doug wants.  Doug wants the bridge's
>> internal I2C host to be exposed, so he can number it through a DT
>> alias.
>
> See his earlier reply and other patch[1] which states once the dw_hdmi
> built-in I2C controller support is added in mainline, then this
> property is not needed. For now, the SOC's general purpose I2C
> controller is used.
>
> Rob
>
> [1] https://lkml.org/lkml/2015/9/2/571

Hmmm, I think we're getting all mixed up here.  To summarize:

1. On rk3288 you can mux the same pins on the SoC to _either_ be
controlled by a generic rk3288 i2c controller (i2c5) or controlled by
the dw_hdmi's i2c block.

2. At the moment, there's no code in mainline to handle the dw_hdmi's i2c block.

3. Right now there _is_ code in mainline to handle specifying
"ddc-i2c-bus" and have it point to the generic rk3288 i2c controller.

4. So in mainline if you want to read an EDID, you've got to configure
the pinmux as "i2c5" and add a "ddc-i2c-bus" reference to the HDMI
section of the device tree.  That's what most rk3288 boards do and (as
far as I understand) matches existing bindings.  The only reason
veyron didn't have this reference was due to a small oversight when
sending the DTS file upstream.

5. There are apparently benefits to using the builtin i2c controller
in dw_hdmi.  There's an outstanding patch add code to support the
dw_hdmi's i2c block.

6. Once you start using the dw_hdmi's i2c block with the currently
posted patch against mainline (to do this you not only need the patch
but you need to remove the ddc-i2c-bus property, set the pinmux, and
disable i2c5) then you'll see a bonafide i2c bus exposed to Linux.  In
my case this stole i2c0 away from the builtin SoC I2C bus and caused
the SoC I2C bus to fail to probe.  Doh.

7. I was trying to solve #6 by adding an "of_node" to the i2c bus
which allowed me to give it a (non-conflicting) bus ID.


-Doug

Re: [PATCH] ARM: dts: Add ddc i2c reference to veyron

[Lucas Stach]

Am Donnerstag, den 03.09.2015, 09:04 -0700 schrieb Doug Anderson:
> Hi,
> 
> On Thu, Sep 3, 2015 at 8:46 AM, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Thu, Sep 3, 2015 at 10:18 AM, Russell King - ARM Linux
> > <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> wrote:
> >> On Thu, Sep 03, 2015 at 09:46:38AM -0500, Rob Herring wrote:
> >>> Yes, that is fairly common (ADV75xx is same), and we would not
> >>> describe an I2C bus in DT in that case. Same with HPD directly handled
> >>> vs. a GPIO line. That is no different than what Doug has said:
> >>> ddc-i2c-bus is present if using the SOC's I2C host and absent if using
> >>> the HDMI block's DDC functionality. I'm only questioning the location
> >>> of the property.
> >>
> >> No, I don't think that's what Doug wants.  Doug wants the bridge's
> >> internal I2C host to be exposed, so he can number it through a DT
> >> alias.
> >
> > See his earlier reply and other patch[1] which states once the dw_hdmi
> > built-in I2C controller support is added in mainline, then this
> > property is not needed. For now, the SOC's general purpose I2C
> > controller is used.
> >
> > Rob
> >
> > [1] https://lkml.org/lkml/2015/9/2/571
> 
> Hmmm, I think we're getting all mixed up here.  To summarize:
> 
> 1. On rk3288 you can mux the same pins on the SoC to _either_ be
> controlled by a generic rk3288 i2c controller (i2c5) or controlled by
> the dw_hdmi's i2c block.
> 
> 2. At the moment, there's no code in mainline to handle the dw_hdmi's i2c block.
> 
> 3. Right now there _is_ code in mainline to handle specifying
> "ddc-i2c-bus" and have it point to the generic rk3288 i2c controller.
> 
> 4. So in mainline if you want to read an EDID, you've got to configure
> the pinmux as "i2c5" and add a "ddc-i2c-bus" reference to the HDMI
> section of the device tree.  That's what most rk3288 boards do and (as
> far as I understand) matches existing bindings.  The only reason
> veyron didn't have this reference was due to a small oversight when
> sending the DTS file upstream.
> 
> 5. There are apparently benefits to using the builtin i2c controller
> in dw_hdmi.  There's an outstanding patch add code to support the
> dw_hdmi's i2c block.
> 
> 6. Once you start using the dw_hdmi's i2c block with the currently
> posted patch against mainline (to do this you not only need the patch
> but you need to remove the ddc-i2c-bus property, set the pinmux, and
> disable i2c5) then you'll see a bonafide i2c bus exposed to Linux.  In
> my case this stole i2c0 away from the builtin SoC I2C bus and caused
> the SoC I2C bus to fail to probe.  Doh.
> 
This shouldn't happen. I don't know if the patches landed yet, but I
know Wolfram (i2c maintainer) had patches to reserve the range of bus
numbers that are fixed by alias nodes and don't hand out those numbers
to adapters with a dynamic bus number allocation.

> 7. I was trying to solve #6 by adding an "of_node" to the i2c bus
> which allowed me to give it a (non-conflicting) bus ID.
> 
This should not be needed.

Regards,
Lucas

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: Add ddc i2c reference to veyron

[Doug Anderson]

Lucas,

On Thu, Sep 3, 2015 at 9:13 AM, Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
>> 6. Once you start using the dw_hdmi's i2c block with the currently
>> posted patch against mainline (to do this you not only need the patch
>> but you need to remove the ddc-i2c-bus property, set the pinmux, and
>> disable i2c5) then you'll see a bonafide i2c bus exposed to Linux.  In
>> my case this stole i2c0 away from the builtin SoC I2C bus and caused
>> the SoC I2C bus to fail to probe.  Doh.
>>
> This shouldn't happen. I don't know if the patches landed yet, but I
> know Wolfram (i2c maintainer) had patches to reserve the range of bus
> numbers that are fixed by alias nodes and don't hand out those numbers
> to adapters with a dynamic bus number allocation.
>
>> 7. I was trying to solve #6 by adding an "of_node" to the i2c bus
>> which allowed me to give it a (non-conflicting) bus ID.
>>
> This should not be needed.

Ah.  It's very possible that the issue is due to the fact that I was
testing this on an old kernel (3.14) that's missing that feature.  OK,
if that's fixed then at least the actual bug (making i2c0 fail to
load) is not a problem.  It'd still be nice to be able to assign a bus
number to this bus just to make reading of the logs easier, but it's
not a functionality requirement.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: Add ddc i2c reference to veyron

[Doug Anderson]

Hi,

On Thu, Sep 3, 2015 at 8:18 AM, Russell King - ARM Linux
<linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> wrote:
> On Thu, Sep 03, 2015 at 09:46:38AM -0500, Rob Herring wrote:
>> Yes, that is fairly common (ADV75xx is same), and we would not
>> describe an I2C bus in DT in that case. Same with HPD directly handled
>> vs. a GPIO line. That is no different than what Doug has said:
>> ddc-i2c-bus is present if using the SOC's I2C host and absent if using
>> the HDMI block's DDC functionality. I'm only questioning the location
>> of the property.
>
> No, I don't think that's what Doug wants.  Doug wants the bridge's
> internal I2C host to be exposed, so he can number it through a DT
> alias.

Actually, my primary concern is that it _doesn't_ conflict with the DT
aliases for busses I care about.  If I didn't provide it with a number
the i2c subsystem was assigning 0 which was conflicting with the real
i2c bus 0.  If the bus was simply not exposed that would have solved
my problem just fine.  ...but if it is being exposed, we need a way to
give it a number.

That being said, my (uninformed) opinion is that if the hardware does
provide enough functionality to expose an i2c bus it's convenient to
expose it, even if it's under a DEBUG config option.  That allows you
to use standard i2c tools during bringup or to debug strange problems.

Obviously since there is hardware that doesn't expose a full i2c bus
then the abstraction should handle that and let a driver just provide
the data directly.

-Doug

Re: [PATCH] ARM: dts: Add ddc i2c reference to veyron

[Doug Anderson]

Hi,

On Wed, Sep 2, 2015 at 2:25 PM, Douglas Anderson <dianders@chromium.org> wrote:
> The ddc-i2c-bus property was missing from the veyron dtsi file since
> downstream the ddc-i2c-bus was still being specified in rk3288.dtsi and
> nobody noticed when the veyron dtsi was sent upstream.  Add it.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> Note: I noticed that this was wrong but I don't currently have
> graphics up and running on upstream on veyron.  Posting this anyway
> since it's pretty clear that it's needed.  If someone else wants to
> try it out that'd be nice, otherwise I'll put it on my list to figure
> out how to get myself setup for graphics upstream.  ;)

OK, I've used Heiko's "somewhat stable" branch to test this against
something very close to mainline.

Without my patch I can't read the HDMI EDID.  With my patch I can.  :)

-Doug

Re: [PATCH] ARM: dts: Add ddc i2c reference to veyron

[Heiko Stuebner]

Am Donnerstag, 3. September 2015, 14:00:26 schrieb Doug Anderson:
> Hi,
> 
> On Wed, Sep 2, 2015 at 2:25 PM, Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 
wrote:
> > The ddc-i2c-bus property was missing from the veyron dtsi file since
> > downstream the ddc-i2c-bus was still being specified in rk3288.dtsi and
> > nobody noticed when the veyron dtsi was sent upstream.  Add it.
> > 
> > Signed-off-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > ---
> > Note: I noticed that this was wrong but I don't currently have
> > graphics up and running on upstream on veyron.  Posting this anyway
> > since it's pretty clear that it's needed.  If someone else wants to
> > try it out that'd be nice, otherwise I'll put it on my list to figure
> > out how to get myself setup for graphics upstream.  ;)
> 
> OK, I've used Heiko's "somewhat stable" branch to test this against
> something very close to mainline.

for people playing along, that is [0]. Essentially bleeding edge mainline + 
some backports from other maintainer trees +the last bits in the works, like 
the eDP support.

> Without my patch I can't read the HDMI EDID.  With my patch I can.  :)

The change follows the current dt-binding in the kernel for the (rockchip-
variant of the) dw_hdmi, so if nobody keeps protesting loudly, I'd like to 
pick this up to get the hdmi on Veyron devices going.

And I guess we'll remove this again (also in the other boards), once the 
dw_hdmi-internal i2c stuff is included - so this should be temporary as Rob 
hoped.


Heiko


[0] https://github.com/mmind/linux-rockchip/tree/v4.2-rockchip-somewhat-stable

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: Add ddc i2c reference to veyron

[Brian Norris]

On Thu, Sep 03, 2015 at 02:00:26PM -0700, Doug Anderson wrote:
> On Wed, Sep 2, 2015 at 2:25 PM, Douglas Anderson <dianders@chromium.org> wrote:
> > The ddc-i2c-bus property was missing from the veyron dtsi file since
> > downstream the ddc-i2c-bus was still being specified in rk3288.dtsi and
> > nobody noticed when the veyron dtsi was sent upstream.  Add it.
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> > Note: I noticed that this was wrong but I don't currently have
> > graphics up and running on upstream on veyron.  Posting this anyway
> > since it's pretty clear that it's needed.  If someone else wants to
> > try it out that'd be nice, otherwise I'll put it on my list to figure
> > out how to get myself setup for graphics upstream.  ;)
> 
> OK, I've used Heiko's "somewhat stable" branch to test this against
> something very close to mainline.
> 
> Without my patch I can't read the HDMI EDID.  With my patch I can.  :)

Same for veyron-jaq:

Tested-by: Brian Norris <briannorris@chromium.org>

Re: [PATCH] ARM: dts: Add ddc i2c reference to veyron

[Heiko Stübner]

Am Mittwoch, 2. September 2015, 14:25:48 schrieb Douglas Anderson:
> The ddc-i2c-bus property was missing from the veyron dtsi file since
> downstream the ddc-i2c-bus was still being specified in rk3288.dtsi and
> nobody noticed when the veyron dtsi was sent upstream.  Add it.
> 
> Signed-off-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

I've applied that to a dts-fixes branch for 4.3 with Brian's Tested-by tag.

As discussed this is temporary-only anyway till the internal i2c stuff of the 
dw_hdmi works.


Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html