[PATCH v6] ARM: dts: wheat: Fix ADV7513 address usage

[PATCH v6] ARM: dts: wheat: Fix ADV7513 address usage

[Kieran Bingham]

The r8a7792 Wheat board has two ADV7513 devices sharing a single I2C
bus, however in low power mode the ADV7513 will reset it's slave maps to
use the hardware defined default addresses.

The ADV7511 driver was adapted to allow the two devices to be registered
correctly - but it did not take into account the fault whereby the
devices reset the addresses.

This results in an address conflict between the device using the default
addresses, and the other device if it is in low-power-mode.

Repair this issue by moving both devices away from the default address
definitions.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
v2:
 - Addition to series

v3:
 - Split map register addresses into individual declarations.

v4:
 - Normalise I2C usage

v5:
 - Repost without [RFT] now that it has been tested

v6:
 - s/low power power/low power/ correction from Laurent.

Testing on a wheat board shows the addresses correctly assigned, and the
default addresses (0x38, 0x3e, 0x3f which would otherwise conflict) are
shown as actively returning data in low power mode during the scan.
(they return 0)

     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
00:          -- -- -- -- -- -- -- -- -- -- -- -- --
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
20: -- -- -- -- -- -- -- -- -- UU -- -- -- UU -- --
30: -- -- -- -- -- -- -- -- 38 UU -- -- -- UU 3e 3f
40: -- -- -- -- -- -- -- -- -- UU -- -- -- UU -- --
50: -- -- -- -- -- -- -- -- -- UU -- -- -- UU -- --
60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
70: -- -- -- -- -- -- -- --

 arch/arm/boot/dts/r8a7792-wheat.dts | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7792-wheat.dts b/arch/arm/boot/dts/r8a7792-wheat.dts
index 293b9e3b3e70..db01de7a3811 100644
--- a/arch/arm/boot/dts/r8a7792-wheat.dts
+++ b/arch/arm/boot/dts/r8a7792-wheat.dts
@@ -245,9 +245,15 @@
 	status = "okay";
 	clock-frequency = <400000>;
 
+	/*
+	 * The adv75xx resets its addresses to defaults during low power mode.
+	 * Because we have two ADV7513 devices on the same bus, we must change
+	 * both of them away from the defaults so that they do not conflict.
+	 */
 	hdmi@3d {
 		compatible = "adi,adv7513";
-		reg = <0x3d>;
+		reg = <0x3d>, <0x2d>, <0x4d>, <0x5d>;
+		reg-names = "main", "cec", "edid", "packet";
 
 		adi,input-depth = <8>;
 		adi,input-colorspace = "rgb";
@@ -277,7 +283,8 @@
 
 	hdmi@39 {
 		compatible = "adi,adv7513";
-		reg = <0x39>;
+		reg = <0x39>, <0x29>, <0x49>, <0x59>;
+		reg-names = "main", "cec", "edid", "packet";
 
 		adi,input-depth = <8>;
 		adi,input-colorspace = "rgb";
-- 
2.7.4

Re: [PATCH v6] ARM: dts: wheat: Fix ADV7513 address usage

[Simon Horman]

On Thu, Mar 22, 2018 at 09:30:40PM +0000, Kieran Bingham wrote:
> The r8a7792 Wheat board has two ADV7513 devices sharing a single I2C
> bus, however in low power mode the ADV7513 will reset it's slave maps to
> use the hardware defined default addresses.
> 
> The ADV7511 driver was adapted to allow the two devices to be registered
> correctly - but it did not take into account the fault whereby the
> devices reset the addresses.
> 
> This results in an address conflict between the device using the default
> addresses, and the other device if it is in low-power-mode.
> 
> Repair this issue by moving both devices away from the default address
> definitions.

Hi Kierean,

as this is a fix
a) Does it warrant a fixes tag?
   Fixes: f6eea82a87db ("ARM: dts: wheat: add DU support")
b) Does it warrant being posted as a fix for v4.16;
c) or v4.17?

Re: [PATCH v6] ARM: dts: wheat: Fix ADV7513 address usage

[Kieran Bingham]

Hi Simon,

On 23/03/18 08:51, Simon Horman wrote:
> On Thu, Mar 22, 2018 at 09:30:40PM +0000, Kieran Bingham wrote:
>> The r8a7792 Wheat board has two ADV7513 devices sharing a single I2C
>> bus, however in low power mode the ADV7513 will reset it's slave maps to
>> use the hardware defined default addresses.
>>
>> The ADV7511 driver was adapted to allow the two devices to be registered
>> correctly - but it did not take into account the fault whereby the
>> devices reset the addresses.
>>
>> This results in an address conflict between the device using the default
>> addresses, and the other device if it is in low-power-mode.
>>
>> Repair this issue by moving both devices away from the default address
>> definitions.
> 
> Hi Kierean,
> 
> as this is a fix
> a) Does it warrant a fixes tag?
>    Fixes: f6eea82a87db ("ARM: dts: wheat: add DU support")
> b) Does it warrant being posted as a fix for v4.16;
> c) or v4.17?

Tricky one, yes it could but this DTS fix, will only actually 'fix' the issue if
the corresponding driver updates to allow secondary addresses to be parsed are
also backported.

It should be safe to back port the dts fix without the driver updates, but the
addresses specified by this patch will simply be ignored.

Thus if this is marked with the fixes tag the corresponding patch "drm: adv7511:
Add support for i2c_new_secondary_device" should also be marked.

It looks like that patch has yet to be picked up by the DRM subsystem, so how
about I bundle both of these two patches together in a repost along with the
fixes tag.

In fact, I don't think the ADV7511 dt-bindings update has made any progress
either. (dt-bindings: adv7511: Extend bindings to allow specifying slave map
addresses). The media tree variants for the adv7604 have already been picked up
by Mauro I believe though.

I presume it would be acceptable for this dts patch (or rather all three patches
mentioned) to get integrated through the DRM tree ?

--
Kieran

Re: [PATCH v6] ARM: dts: wheat: Fix ADV7513 address usage

[Simon Horman]

On Fri, Mar 23, 2018 at 09:16:13PM +0000, Kieran Bingham wrote:
> Hi Simon,
> 
> On 23/03/18 08:51, Simon Horman wrote:
> > On Thu, Mar 22, 2018 at 09:30:40PM +0000, Kieran Bingham wrote:
> >> The r8a7792 Wheat board has two ADV7513 devices sharing a single I2C
> >> bus, however in low power mode the ADV7513 will reset it's slave maps to
> >> use the hardware defined default addresses.
> >>
> >> The ADV7511 driver was adapted to allow the two devices to be registered
> >> correctly - but it did not take into account the fault whereby the
> >> devices reset the addresses.
> >>
> >> This results in an address conflict between the device using the default
> >> addresses, and the other device if it is in low-power-mode.
> >>
> >> Repair this issue by moving both devices away from the default address
> >> definitions.
> > 
> > Hi Kierean,
> > 
> > as this is a fix
> > a) Does it warrant a fixes tag?
> >    Fixes: f6eea82a87db ("ARM: dts: wheat: add DU support")
> > b) Does it warrant being posted as a fix for v4.16;
> > c) or v4.17?
> 
> Tricky one, yes it could but this DTS fix, will only actually 'fix' the issue if
> the corresponding driver updates to allow secondary addresses to be parsed are
> also backported.
> 
> It should be safe to back port the dts fix without the driver updates, but the
> addresses specified by this patch will simply be ignored.

In that case I think its safe to add the fixes tag and take the DTS patch
via the renesas tree. Perhaps applying it for v4.18 and allowing automatic
backporting to take its course is the cleanest option.

> Thus if this is marked with the fixes tag the corresponding patch "drm: adv7511:
> Add support for i2c_new_secondary_device" should also be marked.
> 
> It looks like that patch has yet to be picked up by the DRM subsystem, so how
> about I bundle both of these two patches together in a repost along with the
> fixes tag.
> 
> In fact, I don't think the ADV7511 dt-bindings update has made any progress
> either. (dt-bindings: adv7511: Extend bindings to allow specifying slave map
> addresses). The media tree variants for the adv7604 have already been picked up
> by Mauro I believe though.
> 
> I presume it would be acceptable for this dts patch (or rather all three patches
> mentioned) to get integrated through the DRM tree ?

Unless there is a strong reason I would prefer the dts patch to go via
my tree. The reason is to avoid merge conflicts bubbling up to Linus,
which really is something best avoided.

Re: [PATCH v6] ARM: dts: wheat: Fix ADV7513 address usage

[Kieran Bingham]

Hi Simon,

On 26/03/18 09:31, Simon Horman wrote:
> On Fri, Mar 23, 2018 at 09:16:13PM +0000, Kieran Bingham wrote:
>> Hi Simon,
>>
>> On 23/03/18 08:51, Simon Horman wrote:
>>> On Thu, Mar 22, 2018 at 09:30:40PM +0000, Kieran Bingham wrote:
>>>> The r8a7792 Wheat board has two ADV7513 devices sharing a single I2C
>>>> bus, however in low power mode the ADV7513 will reset it's slave maps to
>>>> use the hardware defined default addresses.
>>>>
>>>> The ADV7511 driver was adapted to allow the two devices to be registered
>>>> correctly - but it did not take into account the fault whereby the
>>>> devices reset the addresses.
>>>>
>>>> This results in an address conflict between the device using the default
>>>> addresses, and the other device if it is in low-power-mode.
>>>>
>>>> Repair this issue by moving both devices away from the default address
>>>> definitions.
>>>
>>> Hi Kierean,
>>>
>>> as this is a fix
>>> a) Does it warrant a fixes tag?
>>>    Fixes: f6eea82a87db ("ARM: dts: wheat: add DU support")
>>> b) Does it warrant being posted as a fix for v4.16;
>>> c) or v4.17?
>>
>> Tricky one, yes it could but this DTS fix, will only actually 'fix' the issue if
>> the corresponding driver updates to allow secondary addresses to be parsed are
>> also backported.
>>
>> It should be safe to back port the dts fix without the driver updates, but the
>> addresses specified by this patch will simply be ignored.
> 
> In that case I think its safe to add the fixes tag and take the DTS patch
> via the renesas tree. Perhaps applying it for v4.18 and allowing automatic
> backporting to take its course is the cleanest option.
> 
>> Thus if this is marked with the fixes tag the corresponding patch "drm: adv7511:
>> Add support for i2c_new_secondary_device" should also be marked.
>>
>> It looks like that patch has yet to be picked up by the DRM subsystem, so how
>> about I bundle both of these two patches together in a repost along with the
>> fixes tag.
>>
>> In fact, I don't think the ADV7511 dt-bindings update has made any progress
>> either. (dt-bindings: adv7511: Extend bindings to allow specifying slave map
>> addresses). The media tree variants for the adv7604 have already been picked up
>> by Mauro I believe though.
>>
>> I presume it would be acceptable for this dts patch (or rather all three patches
>> mentioned) to get integrated through the DRM tree ?
> 
> Unless there is a strong reason I would prefer the dts patch to go via
> my tree. The reason is to avoid merge conflicts bubbling up to Linus,
> which really is something best avoided.

That's perfectly fine with me.

Feel free to add:

Fixes: f6eea82a87db ("ARM: dts: wheat: add DU support")

as you suggested when you apply, or alternatively let me know if you need a repost.

Regards
--
Kieran

Re: [PATCH v6] ARM: dts: wheat: Fix ADV7513 address usage

[Simon Horman]

On Mon, Mar 26, 2018 at 10:04:24AM +0100, Kieran Bingham wrote:
> Hi Simon,
> 
> On 26/03/18 09:31, Simon Horman wrote:
> > On Fri, Mar 23, 2018 at 09:16:13PM +0000, Kieran Bingham wrote:
> >> Hi Simon,
> >>
> >> On 23/03/18 08:51, Simon Horman wrote:
> >>> On Thu, Mar 22, 2018 at 09:30:40PM +0000, Kieran Bingham wrote:
> >>>> The r8a7792 Wheat board has two ADV7513 devices sharing a single I2C
> >>>> bus, however in low power mode the ADV7513 will reset it's slave maps to
> >>>> use the hardware defined default addresses.
> >>>>
> >>>> The ADV7511 driver was adapted to allow the two devices to be registered
> >>>> correctly - but it did not take into account the fault whereby the
> >>>> devices reset the addresses.
> >>>>
> >>>> This results in an address conflict between the device using the default
> >>>> addresses, and the other device if it is in low-power-mode.
> >>>>
> >>>> Repair this issue by moving both devices away from the default address
> >>>> definitions.
> >>>
> >>> Hi Kierean,
> >>>
> >>> as this is a fix
> >>> a) Does it warrant a fixes tag?
> >>>    Fixes: f6eea82a87db ("ARM: dts: wheat: add DU support")
> >>> b) Does it warrant being posted as a fix for v4.16;
> >>> c) or v4.17?
> >>
> >> Tricky one, yes it could but this DTS fix, will only actually 'fix' the issue if
> >> the corresponding driver updates to allow secondary addresses to be parsed are
> >> also backported.
> >>
> >> It should be safe to back port the dts fix without the driver updates, but the
> >> addresses specified by this patch will simply be ignored.
> > 
> > In that case I think its safe to add the fixes tag and take the DTS patch
> > via the renesas tree. Perhaps applying it for v4.18 and allowing automatic
> > backporting to take its course is the cleanest option.
> > 
> >> Thus if this is marked with the fixes tag the corresponding patch "drm: adv7511:
> >> Add support for i2c_new_secondary_device" should also be marked.
> >>
> >> It looks like that patch has yet to be picked up by the DRM subsystem, so how
> >> about I bundle both of these two patches together in a repost along with the
> >> fixes tag.
> >>
> >> In fact, I don't think the ADV7511 dt-bindings update has made any progress
> >> either. (dt-bindings: adv7511: Extend bindings to allow specifying slave map
> >> addresses). The media tree variants for the adv7604 have already been picked up
> >> by Mauro I believe though.
> >>
> >> I presume it would be acceptable for this dts patch (or rather all three patches
> >> mentioned) to get integrated through the DRM tree ?
> > 
> > Unless there is a strong reason I would prefer the dts patch to go via
> > my tree. The reason is to avoid merge conflicts bubbling up to Linus,
> > which really is something best avoided.
> 
> That's perfectly fine with me.
> 
> Feel free to add:
> 
> Fixes: f6eea82a87db ("ARM: dts: wheat: add DU support")
> 
> as you suggested when you apply, or alternatively let me know if you need a repost.

Thanks, applied for v4.18 with the fixes tag.