diff mbox

[v2,3/5,RFT] ARM: dts: wheat: Fix ADV7513 address usage

Message ID 1518459117-16733-4-git-send-email-kbingham@kernel.org (mailing list archive)
State New, archived
Headers show

Commit Message

Kieran Bingham Feb. 12, 2018, 6:11 p.m. UTC 
From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

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>
---
 arch/arm/boot/dts/r8a7792-wheat.dts | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Lars-Peter Clausen Feb. 12, 2018, 6:24 p.m. UTC | #1 
On 02/12/2018 07:11 PM, Kieran Bingham wrote:
[...]
> +	/*
> +	 * The adv75xx resets its addresses to defaults during low power 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>;

To have the correct semantics this should be:
		reg = <0x3d>, <0x2d>, <0x4d>, <0x5d>;

It is a set of 4 single cell addresses. The other thing is a single 4 cell
address. It will get compiled to the same bytes, but the DT tools should
complain about it, because it doesn't match #address-cells.

> +		reg-names = "main", "cec", "edid", "packet";
>  
>  		adi,input-depth = <8>;
>  		adi,input-colorspace = "rgb";
> @@ -272,7 +279,8 @@
>  
>  	hdmi@39 {
>  		compatible = "adi,adv7513";
> -		reg = <0x39>;
> +		reg = <0x39 0x29 0x49, 0x59>;

Same here.

> +		reg-names = "main", "cec", "edid", "packet";
>  
>  		adi,input-depth = <8>;
>  		adi,input-colorspace = "rgb";
>
Kieran Bingham Feb. 12, 2018, 6:30 p.m. UTC | #2 
Hi Lars,

Thanks for your review!

On 12/02/18 18:24, Lars-Peter Clausen wrote:
> On 02/12/2018 07:11 PM, Kieran Bingham wrote:
> [...]
>> +	/*
>> +	 * The adv75xx resets its addresses to defaults during low power 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>;
> 
> To have the correct semantics this should be:
> 		reg = <0x3d>, <0x2d>, <0x4d>, <0x5d>;>
> It is a set of 4 single cell addresses. The other thing is a single 4 cell
> address. It will get compiled to the same bytes, but the DT tools should
> complain about it, because it doesn't match #address-cells.

Not to mention the spurious comma's!!! (at least I marked the patch RFT :D)

I'll resend a v2.1 RFT here, and update my local changes (with the same fault,
sans comma) to my other DT files!

Thanks for the fast review.

--
Kieran


> 
>> +		reg-names = "main", "cec", "edid", "packet";
>>  
>>  		adi,input-depth = <8>;
>>  		adi,input-colorspace = "rgb";
>> @@ -272,7 +279,8 @@
>>  
>>  	hdmi@39 {
>>  		compatible = "adi,adv7513";
>> -		reg = <0x39>;
>> +		reg = <0x39 0x29 0x49, 0x59>;
> 
> Same here.
> 
>> +		reg-names = "main", "cec", "edid", "packet";
>>  
>>  		adi,input-depth = <8>;
>>  		adi,input-colorspace = "rgb";
>>
>
kernel test robot Feb. 14, 2018, 1:04 a.m. UTC | #3 
Hi Kieran,

I love your patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.16-rc1 next-20180213]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Kieran-Bingham/dt-bindings-media-adv7604-Add-support-for-i2c_new_secondary_device/20180214-032943
base:   git://linuxtv.org/media_tree.git master
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> Error: arch/arm/boot/dts/r8a7792-wheat.dts:251.24-25 syntax error
   FATAL ERROR: Unable to parse input tree

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/arch/arm/boot/dts/r8a7792-wheat.dts b/arch/arm/boot/dts/r8a7792-wheat.dts
index b9471b67b728..c94f330392ee 100644
--- a/arch/arm/boot/dts/r8a7792-wheat.dts
+++ b/arch/arm/boot/dts/r8a7792-wheat.dts
@@ -240,9 +240,16 @@ 
 	status = "okay";
 	clock-frequency = <400000>;
 
+	/*
+	 * The adv75xx resets its addresses to defaults during low power 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";
@@ -272,7 +279,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";