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

[kbuild test robot]

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 infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

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

[Kieran Bingham]

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";
>>
> 

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

[Lars-Peter Clausen]

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";
> 

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

[Kieran Bingham]

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

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 = <40>;
 
+   /*
+* 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";
-- 
2.7.4