[PATCH v3 0/5] Add support for i2c_new_secondary_device

[PATCH v3 0/5] Add support for i2c_new_secondary_device

[Kieran Bingham]

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Back in 2014, Jean-Michel provided patches [0] to implement a means of
describing software defined I2C addresses for devices through the DT nodes.

The patch to implement the function "i2c_new_secondary_device()" was integrated,
but the corresponding driver update didn't get applied.

This short series re-bases Jean-Michel's patch to mainline for the ADV7604 driver
in linux-media, and also provides a patch for the ADV7511 DRM Bridge driver taking
the same approach.

This series allows us to define the I2C address allocations of these devices in
the device tree for the Renesas D3 platform where these two devices reside on
the same bus and conflict with each other presently..

[0] https://lkml.org/lkml/2014/10/22/468
[1] https://lkml.org/lkml/2014/10/22/469

v2:
 - dt bindings split from driver changes
 - fixed up dt binding property descriptions
 - Update missing edid-i2c address setting (adv7511)
 - Provide update for r8a7792 DTB to account for address conflict

v3:
 - Split map register addresses into individual declarations across all uses


Jean-Michel Hautbois (2):
  dt-bindings: media: adv7604: Add support for i2c_new_secondary_device
  media: adv7604: Add support for i2c_new_secondary_device

Kieran Bingham (3):
  dt-bindings: adv7511: Add support for i2c_new_secondary_device
  [RFT] ARM: dts: wheat: Fix ADV7513 address usage
  drm: adv7511: Add support for i2c_new_secondary_device

 .../bindings/display/bridge/adi,adv7511.txt        | 18 ++++++-
 .../devicetree/bindings/media/i2c/adv7604.txt      | 18 ++++++-
 arch/arm/boot/dts/r8a7792-wheat.dts                | 12 ++++-
 drivers/gpu/drm/bridge/adv7511/adv7511.h           |  6 +++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c       | 42 +++++++++------
 drivers/media/i2c/adv7604.c                        | 62 ++++++++++++++--------
 6 files changed, 115 insertions(+), 43 deletions(-)

-- 
2.7.4

[PATCH v3 1/5] dt-bindings: media: adv7604: Add support for i2c_new_secondary_device

[Kieran Bingham]

From: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>

The ADV7604 has thirteen 256-byte maps that can be accessed via the main
I²C ports. Each map has it own I²C address and acts as a standard slave
device on the I²C bus.

Extend the device tree node bindings to be able to override the default
addresses so that address conflicts with other devices on the same bus
may be resolved at the board description level.

Signed-off-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
[Kieran: Re-adapted for mainline]
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Rob Herring <robh@kernel.org>

---
Based upon the original posting :
  https://lkml.org/lkml/2014/10/22/469

v2:
 - DT Binding update separated from code change
 - Minor reword to commit message to account for DT only change.
 - Collected Rob's RB tag.

v3:
 - Split map register addresses into individual declarations.

 .../devicetree/bindings/media/i2c/adv7604.txt          | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
index 9cbd92eb5d05..ebb5f070c05b 100644
--- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
+++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
@@ -13,7 +13,11 @@ Required Properties:
     - "adi,adv7611" for the ADV7611
     - "adi,adv7612" for the ADV7612
 
-  - reg: I2C slave address
+  - reg: I2C slave addresses
+    The ADV76xx has up to thirteen 256-byte maps that can be accessed via the
+    main I²C ports. Each map has it own I²C address and acts as a standard
+    slave device on the I²C bus. The main address is mandatory, others are
+    optional and revert to defaults if not specified.
 
   - hpd-gpios: References to the GPIOs that control the HDMI hot-plug
     detection pins, one per HDMI input. The active flag indicates the GPIO
@@ -35,6 +39,11 @@ Optional Properties:
 
   - reset-gpios: Reference to the GPIO connected to the device's reset pin.
   - default-input: Select which input is selected after reset.
+  - reg-names : Names of maps with programmable addresses.
+		It can contain any map needing a non-default address.
+		Possible maps names are :
+		  "main", "avlink", "cec", "infoframe", "esdp", "dpp", "afe",
+		  "rep", "edid", "hdmi", "test", "cp", "vdp"
 
 Optional Endpoint Properties:
 
@@ -52,7 +61,12 @@ Example:
 
 	hdmi_receiver@4c {
 		compatible = "adi,adv7611";
-		reg = <0x4c>;
+		/*
+		 * The edid page will be accessible @ 0x66 on the i2c bus. All
+		 * other maps will retain their default addresses.
+		 */
+		reg = <0x4c>, <0x66>;
+		reg-names "main", "edid";
 
 		reset-gpios = <&ioexp 0 GPIO_ACTIVE_LOW>;
 		hpd-gpios = <&ioexp 2 GPIO_ACTIVE_HIGH>;
-- 
2.7.4

[PATCH v3 1/5] dt-bindings: media: adv7604: Add support for i2c_new_secondary_device

[Kieran Bingham]

From: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>

The ADV7604 has thirteen 256-byte maps that can be accessed via the main
I²C ports. Each map has it own I²C address and acts as a standard slave
device on the I²C bus.

Extend the device tree node bindings to be able to override the default
addresses so that address conflicts with other devices on the same bus
may be resolved at the board description level.

Signed-off-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
[Kieran: Re-adapted for mainline]
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Rob Herring <robh@kernel.org>

---
Based upon the original posting :
  https://lkml.org/lkml/2014/10/22/469

v2:
 - DT Binding update separated from code change
 - Minor reword to commit message to account for DT only change.
 - Collected Rob's RB tag.

v3:
 - Split map register addresses into individual declarations.

 .../devicetree/bindings/media/i2c/adv7604.txt          | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
index 9cbd92eb5d05..ebb5f070c05b 100644
--- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
+++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
@@ -13,7 +13,11 @@ Required Properties:
     - "adi,adv7611" for the ADV7611
     - "adi,adv7612" for the ADV7612
 
-  - reg: I2C slave address
+  - reg: I2C slave addresses
+    The ADV76xx has up to thirteen 256-byte maps that can be accessed via the
+    main I²C ports. Each map has it own I²C address and acts as a standard
+    slave device on the I²C bus. The main address is mandatory, others are
+    optional and revert to defaults if not specified.
 
   - hpd-gpios: References to the GPIOs that control the HDMI hot-plug
     detection pins, one per HDMI input. The active flag indicates the GPIO
@@ -35,6 +39,11 @@ Optional Properties:
 
   - reset-gpios: Reference to the GPIO connected to the device's reset pin.
   - default-input: Select which input is selected after reset.
+  - reg-names : Names of maps with programmable addresses.
+		It can contain any map needing a non-default address.
+		Possible maps names are :
+		  "main", "avlink", "cec", "infoframe", "esdp", "dpp", "afe",
+		  "rep", "edid", "hdmi", "test", "cp", "vdp"
 
 Optional Endpoint Properties:
 
@@ -52,7 +61,12 @@ Example:
 
 	hdmi_receiver@4c {
 		compatible = "adi,adv7611";
-		reg = <0x4c>;
+		/*
+		 * The edid page will be accessible @ 0x66 on the i2c bus. All
+		 * other maps will retain their default addresses.
+		 */
+		reg = <0x4c>, <0x66>;
+		reg-names "main", "edid";
 
 		reset-gpios = <&ioexp 0 GPIO_ACTIVE_LOW>;
 		hpd-gpios = <&ioexp 2 GPIO_ACTIVE_HIGH>;
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/5] dt-bindings: media: adv7604: Add support for i2c_new_secondary_device

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tuesday, 13 February 2018 00:07:49 EET Kieran Bingham wrote:
> From: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
> 
> The ADV7604 has thirteen 256-byte maps that can be accessed via the main
> I²C ports. Each map has it own I²C address and acts as a standard slave
> device on the I²C bus.
> 
> Extend the device tree node bindings to be able to override the default
> addresses so that address conflicts with other devices on the same bus
> may be resolved at the board description level.
> 
> Signed-off-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
> [Kieran: Re-adapted for mainline]
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Nitpicking, I might not mention i2c_new_secondary_device in the subject, as 
this is a DT bindings change. I don't mind too much though, as long as the 
bindings themselves don't contain Linux-specific information, and they don't, 
so

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> Based upon the original posting :
>   https://lkml.org/lkml/2014/10/22/469
> 
> v2:
>  - DT Binding update separated from code change
>  - Minor reword to commit message to account for DT only change.
>  - Collected Rob's RB tag.
> 
> v3:
>  - Split map register addresses into individual declarations.
> 
>  .../devicetree/bindings/media/i2c/adv7604.txt          | 18
> ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> b/Documentation/devicetree/bindings/media/i2c/adv7604.txt index
> 9cbd92eb5d05..ebb5f070c05b 100644
> --- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> @@ -13,7 +13,11 @@ Required Properties:
>      - "adi,adv7611" for the ADV7611
>      - "adi,adv7612" for the ADV7612
> 
> -  - reg: I2C slave address
> +  - reg: I2C slave addresses
> +    The ADV76xx has up to thirteen 256-byte maps that can be accessed via
> the +    main I²C ports. Each map has it own I²C address and acts as a
> standard +    slave device on the I²C bus. The main address is mandatory,
> others are +    optional and revert to defaults if not specified.
> 
>    - hpd-gpios: References to the GPIOs that control the HDMI hot-plug
>      detection pins, one per HDMI input. The active flag indicates the GPIO
> @@ -35,6 +39,11 @@ Optional Properties:
> 
>    - reset-gpios: Reference to the GPIO connected to the device's reset pin.
> - default-input: Select which input is selected after reset.
> +  - reg-names : Names of maps with programmable addresses.
> +		It can contain any map needing a non-default address.
> +		Possible maps names are :
> +		  "main", "avlink", "cec", "infoframe", "esdp", "dpp", "afe",
> +		  "rep", "edid", "hdmi", "test", "cp", "vdp"
> 
>  Optional Endpoint Properties:
> 
> @@ -52,7 +61,12 @@ Example:
> 
>  	hdmi_receiver@4c {
>  		compatible = "adi,adv7611";
> -		reg = <0x4c>;
> +		/*
> +		 * The edid page will be accessible @ 0x66 on the i2c bus. All
> +		 * other maps will retain their default addresses.
> +		 */
> +		reg = <0x4c>, <0x66>;
> +		reg-names "main", "edid";
> 
>  		reset-gpios = <&ioexp 0 GPIO_ACTIVE_LOW>;
>  		hpd-gpios = <&ioexp 2 GPIO_ACTIVE_HIGH>;


-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 1/5] dt-bindings: media: adv7604: Add support for i2c_new_secondary_device

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tuesday, 13 February 2018 00:07:49 EET Kieran Bingham wrote:
> From: Jean-Michel Hautbois <jean-michel.hautbois-B+Q8N6RmIDZBDgjK7y7TUQ@public.gmane.org>
> 
> The ADV7604 has thirteen 256-byte maps that can be accessed via the main
> I²C ports. Each map has it own I²C address and acts as a standard slave
> device on the I²C bus.
> 
> Extend the device tree node bindings to be able to override the default
> addresses so that address conflicts with other devices on the same bus
> may be resolved at the board description level.
> 
> Signed-off-by: Jean-Michel Hautbois <jean-michel.hautbois-B+Q8N6RmIDZBDgjK7y7TUQ@public.gmane.org>
> [Kieran: Re-adapted for mainline]
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Nitpicking, I might not mention i2c_new_secondary_device in the subject, as 
this is a DT bindings change. I don't mind too much though, as long as the 
bindings themselves don't contain Linux-specific information, and they don't, 
so

Reviewed-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

> ---
> Based upon the original posting :
>   https://lkml.org/lkml/2014/10/22/469
> 
> v2:
>  - DT Binding update separated from code change
>  - Minor reword to commit message to account for DT only change.
>  - Collected Rob's RB tag.
> 
> v3:
>  - Split map register addresses into individual declarations.
> 
>  .../devicetree/bindings/media/i2c/adv7604.txt          | 18
> ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> b/Documentation/devicetree/bindings/media/i2c/adv7604.txt index
> 9cbd92eb5d05..ebb5f070c05b 100644
> --- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> @@ -13,7 +13,11 @@ Required Properties:
>      - "adi,adv7611" for the ADV7611
>      - "adi,adv7612" for the ADV7612
> 
> -  - reg: I2C slave address
> +  - reg: I2C slave addresses
> +    The ADV76xx has up to thirteen 256-byte maps that can be accessed via
> the +    main I²C ports. Each map has it own I²C address and acts as a
> standard +    slave device on the I²C bus. The main address is mandatory,
> others are +    optional and revert to defaults if not specified.
> 
>    - hpd-gpios: References to the GPIOs that control the HDMI hot-plug
>      detection pins, one per HDMI input. The active flag indicates the GPIO
> @@ -35,6 +39,11 @@ Optional Properties:
> 
>    - reset-gpios: Reference to the GPIO connected to the device's reset pin.
> - default-input: Select which input is selected after reset.
> +  - reg-names : Names of maps with programmable addresses.
> +		It can contain any map needing a non-default address.
> +		Possible maps names are :
> +		  "main", "avlink", "cec", "infoframe", "esdp", "dpp", "afe",
> +		  "rep", "edid", "hdmi", "test", "cp", "vdp"
> 
>  Optional Endpoint Properties:
> 
> @@ -52,7 +61,12 @@ Example:
> 
>  	hdmi_receiver@4c {
>  		compatible = "adi,adv7611";
> -		reg = <0x4c>;
> +		/*
> +		 * The edid page will be accessible @ 0x66 on the i2c bus. All
> +		 * other maps will retain their default addresses.
> +		 */
> +		reg = <0x4c>, <0x66>;
> +		reg-names "main", "edid";
> 
>  		reset-gpios = <&ioexp 0 GPIO_ACTIVE_LOW>;
>  		hpd-gpios = <&ioexp 2 GPIO_ACTIVE_HIGH>;


-- 
Regards,

Laurent Pinchart

--
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 v3 1/5] dt-bindings: media: adv7604: Add support for i2c_new_secondary_device

[Kieran Bingham]

Hi Laurent,

On 13/02/18 12:06, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.

Thank you for your review,

> On Tuesday, 13 February 2018 00:07:49 EET Kieran Bingham wrote:
>> From: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
>>
>> The ADV7604 has thirteen 256-byte maps that can be accessed via the main
>> I²C ports. Each map has it own I²C address and acts as a standard slave
>> device on the I²C bus.
>>
>> Extend the device tree node bindings to be able to override the default
>> addresses so that address conflicts with other devices on the same bus
>> may be resolved at the board description level.
>>
>> Signed-off-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
>> [Kieran: Re-adapted for mainline]
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> Reviewed-by: Rob Herring <robh@kernel.org>
> 
> Nitpicking, I might not mention i2c_new_secondary_device in the subject, as 
> this is a DT bindings change. I don't mind too much though, as long as the 
> bindings themselves don't contain Linux-specific information, and they don't, 
> so
How about: ... adv7604: Extend bindings to allow specifying slave map addresses



> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Collected, thanks.

--
Kieran


> 
>> ---
>> Based upon the original posting :
>>   https://lkml.org/lkml/2014/10/22/469
>>
>> v2:
>>  - DT Binding update separated from code change
>>  - Minor reword to commit message to account for DT only change.
>>  - Collected Rob's RB tag.
>>
>> v3:
>>  - Split map register addresses into individual declarations.
>>
>>  .../devicetree/bindings/media/i2c/adv7604.txt          | 18
>> ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
>> b/Documentation/devicetree/bindings/media/i2c/adv7604.txt index
>> 9cbd92eb5d05..ebb5f070c05b 100644
>> --- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
>> +++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
>> @@ -13,7 +13,11 @@ Required Properties:
>>      - "adi,adv7611" for the ADV7611
>>      - "adi,adv7612" for the ADV7612
>>
>> -  - reg: I2C slave address
>> +  - reg: I2C slave addresses
>> +    The ADV76xx has up to thirteen 256-byte maps that can be accessed via
>> the +    main I²C ports. Each map has it own I²C address and acts as a
>> standard +    slave device on the I²C bus. The main address is mandatory,
>> others are +    optional and revert to defaults if not specified.
>>
>>    - hpd-gpios: References to the GPIOs that control the HDMI hot-plug
>>      detection pins, one per HDMI input. The active flag indicates the GPIO
>> @@ -35,6 +39,11 @@ Optional Properties:
>>
>>    - reset-gpios: Reference to the GPIO connected to the device's reset pin.
>> - default-input: Select which input is selected after reset.
>> +  - reg-names : Names of maps with programmable addresses.
>> +		It can contain any map needing a non-default address.
>> +		Possible maps names are :
>> +		  "main", "avlink", "cec", "infoframe", "esdp", "dpp", "afe",
>> +		  "rep", "edid", "hdmi", "test", "cp", "vdp"
>>
>>  Optional Endpoint Properties:
>>
>> @@ -52,7 +61,12 @@ Example:
>>
>>  	hdmi_receiver@4c {
>>  		compatible = "adi,adv7611";
>> -		reg = <0x4c>;
>> +		/*
>> +		 * The edid page will be accessible @ 0x66 on the i2c bus. All
>> +		 * other maps will retain their default addresses.
>> +		 */
>> +		reg = <0x4c>, <0x66>;
>> +		reg-names "main", "edid";
>>
>>  		reset-gpios = <&ioexp 0 GPIO_ACTIVE_LOW>;
>>  		hpd-gpios = <&ioexp 2 GPIO_ACTIVE_HIGH>;
> 
> 

Re: [PATCH v3 1/5] dt-bindings: media: adv7604: Add support for i2c_new_secondary_device

[Laurent Pinchart]

Hi Kieran,

On Tuesday, 13 February 2018 15:14:43 EET Kieran Bingham wrote:
> On 13/02/18 12:06, Laurent Pinchart wrote:
> > On Tuesday, 13 February 2018 00:07:49 EET Kieran Bingham wrote:
> >> From: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
> >> 
> >> The ADV7604 has thirteen 256-byte maps that can be accessed via the main
> >> I²C ports. Each map has it own I²C address and acts as a standard slave
> >> device on the I²C bus.
> >> 
> >> Extend the device tree node bindings to be able to override the default
> >> addresses so that address conflicts with other devices on the same bus
> >> may be resolved at the board description level.
> >> 
> >> Signed-off-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
> >> [Kieran: Re-adapted for mainline]
> >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >> Reviewed-by: Rob Herring <robh@kernel.org>
> > 
> > Nitpicking, I might not mention i2c_new_secondary_device in the subject,
> > as this is a DT bindings change. I don't mind too much though, as long as
> > the bindings themselves don't contain Linux-specific information, and they
> > don't, so
> 
> How about: ... adv7604: Extend bindings to allow specifying slave map
> addresses

Sounds good to me.

> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Collected, thanks.
> 
> --
> Kieran
> 
> >> ---
> >> 
> >> Based upon the original posting :
> >>   https://lkml.org/lkml/2014/10/22/469
> >> 
> >> v2:
> >>  - DT Binding update separated from code change
> >>  - Minor reword to commit message to account for DT only change.
> >>  - Collected Rob's RB tag.
> >> 
> >> v3:
> >>  - Split map register addresses into individual declarations.
> >>  
> >>  .../devicetree/bindings/media/i2c/adv7604.txt          | 18
> >> 
> >> ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> >> b/Documentation/devicetree/bindings/media/i2c/adv7604.txt index
> >> 9cbd92eb5d05..ebb5f070c05b 100644
> >> --- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> >> +++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> >> 
> >> @@ -13,7 +13,11 @@ Required Properties:
> >>      - "adi,adv7611" for the ADV7611
> >>      - "adi,adv7612" for the ADV7612
> >> 
> >> -  - reg: I2C slave address
> >> +  - reg: I2C slave addresses
> >> +    The ADV76xx has up to thirteen 256-byte maps that can be accessed
> >> via
> >> the +    main I²C ports. Each map has it own I²C address and acts as a
> >> standard +    slave device on the I²C bus. The main address is mandatory,
> >> others are +    optional and revert to defaults if not specified.
> >> 
> >>    - hpd-gpios: References to the GPIOs that control the HDMI hot-plug
> >>    
> >>      detection pins, one per HDMI input. The active flag indicates the
> >>      GPIO
> >> 
> >> @@ -35,6 +39,11 @@ Optional Properties:
> >>    - reset-gpios: Reference to the GPIO connected to the device's reset
> >>    pin.
> >> 
> >> - default-input: Select which input is selected after reset.
> >> +  - reg-names : Names of maps with programmable addresses.
> >> +		It can contain any map needing a non-default address.
> >> +		Possible maps names are :
> >> +		  "main", "avlink", "cec", "infoframe", "esdp", "dpp", "afe",
> >> +		  "rep", "edid", "hdmi", "test", "cp", "vdp"
> >> 
> >>  Optional Endpoint Properties:
> >> @@ -52,7 +61,12 @@ Example:
> >>  	hdmi_receiver@4c {
> >>  	
> >>  		compatible = "adi,adv7611";
> >> 
> >> -		reg = <0x4c>;
> >> +		/*
> >> +		 * The edid page will be accessible @ 0x66 on the i2c bus. All
> >> +		 * other maps will retain their default addresses.
> >> +		 */
> >> +		reg = <0x4c>, <0x66>;
> >> +		reg-names "main", "edid";
> >> 
> >>  		reset-gpios = <&ioexp 0 GPIO_ACTIVE_LOW>;
> >>  		hpd-gpios = <&ioexp 2 GPIO_ACTIVE_HIGH>;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 1/5] dt-bindings: media: adv7604: Add support for i2c_new_secondary_device

[Laurent Pinchart]

Hi Kieran,

On Tuesday, 13 February 2018 15:14:43 EET Kieran Bingham wrote:
> On 13/02/18 12:06, Laurent Pinchart wrote:
> > On Tuesday, 13 February 2018 00:07:49 EET Kieran Bingham wrote:
> >> From: Jean-Michel Hautbois <jean-michel.hautbois-B+Q8N6RmIDZBDgjK7y7TUQ@public.gmane.org>
> >> 
> >> The ADV7604 has thirteen 256-byte maps that can be accessed via the main
> >> I²C ports. Each map has it own I²C address and acts as a standard slave
> >> device on the I²C bus.
> >> 
> >> Extend the device tree node bindings to be able to override the default
> >> addresses so that address conflicts with other devices on the same bus
> >> may be resolved at the board description level.
> >> 
> >> Signed-off-by: Jean-Michel Hautbois <jean-michel.hautbois-B+Q8N6RmIDZBDgjK7y7TUQ@public.gmane.org>
> >> [Kieran: Re-adapted for mainline]
> >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> >> Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > 
> > Nitpicking, I might not mention i2c_new_secondary_device in the subject,
> > as this is a DT bindings change. I don't mind too much though, as long as
> > the bindings themselves don't contain Linux-specific information, and they
> > don't, so
> 
> How about: ... adv7604: Extend bindings to allow specifying slave map
> addresses

Sounds good to me.

> > Reviewed-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> 
> Collected, thanks.
> 
> --
> Kieran
> 
> >> ---
> >> 
> >> Based upon the original posting :
> >>   https://lkml.org/lkml/2014/10/22/469
> >> 
> >> v2:
> >>  - DT Binding update separated from code change
> >>  - Minor reword to commit message to account for DT only change.
> >>  - Collected Rob's RB tag.
> >> 
> >> v3:
> >>  - Split map register addresses into individual declarations.
> >>  
> >>  .../devicetree/bindings/media/i2c/adv7604.txt          | 18
> >> 
> >> ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> >> b/Documentation/devicetree/bindings/media/i2c/adv7604.txt index
> >> 9cbd92eb5d05..ebb5f070c05b 100644
> >> --- a/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> >> +++ b/Documentation/devicetree/bindings/media/i2c/adv7604.txt
> >> 
> >> @@ -13,7 +13,11 @@ Required Properties:
> >>      - "adi,adv7611" for the ADV7611
> >>      - "adi,adv7612" for the ADV7612
> >> 
> >> -  - reg: I2C slave address
> >> +  - reg: I2C slave addresses
> >> +    The ADV76xx has up to thirteen 256-byte maps that can be accessed
> >> via
> >> the +    main I²C ports. Each map has it own I²C address and acts as a
> >> standard +    slave device on the I²C bus. The main address is mandatory,
> >> others are +    optional and revert to defaults if not specified.
> >> 
> >>    - hpd-gpios: References to the GPIOs that control the HDMI hot-plug
> >>    
> >>      detection pins, one per HDMI input. The active flag indicates the
> >>      GPIO
> >> 
> >> @@ -35,6 +39,11 @@ Optional Properties:
> >>    - reset-gpios: Reference to the GPIO connected to the device's reset
> >>    pin.
> >> 
> >> - default-input: Select which input is selected after reset.
> >> +  - reg-names : Names of maps with programmable addresses.
> >> +		It can contain any map needing a non-default address.
> >> +		Possible maps names are :
> >> +		  "main", "avlink", "cec", "infoframe", "esdp", "dpp", "afe",
> >> +		  "rep", "edid", "hdmi", "test", "cp", "vdp"
> >> 
> >>  Optional Endpoint Properties:
> >> @@ -52,7 +61,12 @@ Example:
> >>  	hdmi_receiver@4c {
> >>  	
> >>  		compatible = "adi,adv7611";
> >> 
> >> -		reg = <0x4c>;
> >> +		/*
> >> +		 * The edid page will be accessible @ 0x66 on the i2c bus. All
> >> +		 * other maps will retain their default addresses.
> >> +		 */
> >> +		reg = <0x4c>, <0x66>;
> >> +		reg-names "main", "edid";
> >> 
> >>  		reset-gpios = <&ioexp 0 GPIO_ACTIVE_LOW>;
> >>  		hpd-gpios = <&ioexp 2 GPIO_ACTIVE_HIGH>;

-- 
Regards,

Laurent Pinchart

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

[PATCH v3 2/5] dt-bindings: adv7511: Add support for i2c_new_secondary_device

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

The ADV7511 has four 256-byte maps that can be accessed via the main I²C
ports. Each map has it own I²C address and acts as a standard slave
device on the I²C bus.

Extend the device tree node bindings to be able to override the default
addresses so that address conflicts with other devices on the same bus
may be resolved at the board description level.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
v2:
 - Fixed up reg: property description to account for multiple optional
   addresses.
 - Minor reword to commit message to account for DT only change
 - Collected Robs RB tag

v3:
 - Split map register addresses into individual declarations.

 .../devicetree/bindings/display/bridge/adi,adv7511.txt | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 0047b1394c70..3f85c351dd39 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -14,7 +14,13 @@ Required properties:
 		"adi,adv7513"
 		"adi,adv7533"
 
-- reg: I2C slave address
+- reg: I2C slave addresses
+  The ADV7511 internal registers are split into four pages exposed through
+  different I2C addresses, creating four register maps. Each map has it own
+  I2C address and acts as a standard slave device on the I²C bus. The main
+  address is mandatory, others are optional and revert to defaults if not
+  specified.
+
 
 The ADV7511 supports a large number of input data formats that differ by their
 color depth, color format, clock mode, bit justification and random
@@ -70,6 +76,9 @@ Optional properties:
   rather than generate its own timings for HDMI output.
 - clocks: from common clock binding: reference to the CEC clock.
 - clock-names: from common clock binding: must be "cec".
+- reg-names : Names of maps with programmable addresses.
+	It can contain any map needing a non-default address.
+	Possible maps names are : "main", "edid", "cec", "packet"
 
 Required nodes:
 
@@ -88,7 +97,12 @@ Example
 
 	adv7511w: hdmi@39 {
 		compatible = "adi,adv7511w";
-		reg = <39>;
+		/*
+		 * The EDID page will be accessible on address 0x66 on the i2c
+		 * bus. All other maps continue to use their default addresses.
+		 */
+		reg = <0x39>, <0x66>;
+		reg-names = "main", "edid";
 		interrupt-parent = <&gpio3>;
 		interrupts = <29 IRQ_TYPE_EDGE_FALLING>;
 		clocks = <&cec_clock>;
-- 
2.7.4

[PATCH v3 2/5] dt-bindings: adv7511: Add support for i2c_new_secondary_device

[Kieran Bingham]

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

The ADV7511 has four 256-byte maps that can be accessed via the main I²C
ports. Each map has it own I²C address and acts as a standard slave
device on the I²C bus.

Extend the device tree node bindings to be able to override the default
addresses so that address conflicts with other devices on the same bus
may be resolved at the board description level.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
v2:
 - Fixed up reg: property description to account for multiple optional
   addresses.
 - Minor reword to commit message to account for DT only change
 - Collected Robs RB tag

v3:
 - Split map register addresses into individual declarations.

 .../devicetree/bindings/display/bridge/adi,adv7511.txt | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 0047b1394c70..3f85c351dd39 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -14,7 +14,13 @@ Required properties:
 		"adi,adv7513"
 		"adi,adv7533"
 
-- reg: I2C slave address
+- reg: I2C slave addresses
+  The ADV7511 internal registers are split into four pages exposed through
+  different I2C addresses, creating four register maps. Each map has it own
+  I2C address and acts as a standard slave device on the I²C bus. The main
+  address is mandatory, others are optional and revert to defaults if not
+  specified.
+
 
 The ADV7511 supports a large number of input data formats that differ by their
 color depth, color format, clock mode, bit justification and random
@@ -70,6 +76,9 @@ Optional properties:
   rather than generate its own timings for HDMI output.
 - clocks: from common clock binding: reference to the CEC clock.
 - clock-names: from common clock binding: must be "cec".
+- reg-names : Names of maps with programmable addresses.
+	It can contain any map needing a non-default address.
+	Possible maps names are : "main", "edid", "cec", "packet"
 
 Required nodes:
 
@@ -88,7 +97,12 @@ Example
 
 	adv7511w: hdmi@39 {
 		compatible = "adi,adv7511w";
-		reg = <39>;
+		/*
+		 * The EDID page will be accessible on address 0x66 on the i2c
+		 * bus. All other maps continue to use their default addresses.
+		 */
+		reg = <0x39>, <0x66>;
+		reg-names = "main", "edid";
 		interrupt-parent = <&gpio3>;
 		interrupts = <29 IRQ_TYPE_EDGE_FALLING>;
 		clocks = <&cec_clock>;
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/5] dt-bindings: adv7511: Add support for i2c_new_secondary_device

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tuesday, 13 February 2018 00:07:50 EET Kieran Bingham wrote:
> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> The ADV7511 has four 256-byte maps that can be accessed via the main I²C
> ports. Each map has it own I²C address and acts as a standard slave
> device on the I²C bus.
> 
> Extend the device tree node bindings to be able to override the default
> addresses so that address conflicts with other devices on the same bus
> may be resolved at the board description level.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Same comment as for 1/5 about the subject line.

> ---
> v2:
>  - Fixed up reg: property description to account for multiple optional
>    addresses.
>  - Minor reword to commit message to account for DT only change
>  - Collected Robs RB tag
> 
> v3:
>  - Split map register addresses into individual declarations.
> 
>  .../devicetree/bindings/display/bridge/adi,adv7511.txt | 18 +++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt index
> 0047b1394c70..3f85c351dd39 100644
> --- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
> @@ -14,7 +14,13 @@ Required properties:
>  		"adi,adv7513"
>  		"adi,adv7533"
> 
> -- reg: I2C slave address
> +- reg: I2C slave addresses
> +  The ADV7511 internal registers are split into four pages exposed through
> +  different I2C addresses, creating four register maps. Each map has it own
> +  I2C address and acts as a standard slave device on the I²C bus. The main
> +  address is mandatory, others are optional and revert to defaults if not
> +  specified.

Nitpicking again, you're mixing I2C and I²C.

> +
> 
>  The ADV7511 supports a large number of input data formats that differ by
> their color depth, color format, clock mode, bit justification and random
> @@ -70,6 +76,9 @@ Optional properties:
>    rather than generate its own timings for HDMI output.
>  - clocks: from common clock binding: reference to the CEC clock.
>  - clock-names: from common clock binding: must be "cec".
> +- reg-names : Names of maps with programmable addresses.
> +	It can contain any map needing a non-default address.
> +	Possible maps names are : "main", "edid", "cec", "packet"
> 
>  Required nodes:
> 
> @@ -88,7 +97,12 @@ Example
> 
>  	adv7511w: hdmi@39 {
>  		compatible = "adi,adv7511w";
> -		reg = <39>;
> +		/*
> +		 * The EDID page will be accessible on address 0x66 on the i2c

And now you're using lowercase :-)

> +		 * bus. All other maps continue to use their default addresses.
> +		 */
> +		reg = <0x39>, <0x66>;
> +		reg-names = "main", "edid";
>  		interrupt-parent = <&gpio3>;
>  		interrupts = <29 IRQ_TYPE_EDGE_FALLING>;
>  		clocks = <&cec_clock>;

With these fixed (or not, up to you),

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart

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

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>

---
v2:
 - Addition to series

v3:
 - Split map register addresses into individual declarations.

 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..42fff8837eab 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";
-- 
2.7.4

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

[Kieran Bingham]

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>

---
v2:
 - Addition to series

v3:
 - Split map register addresses into individual declarations.

 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..42fff8837eab 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 at 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 at 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

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

[Kieran Bingham]

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>

---
v2:
 - Addition to series

v3:
 - Split map register addresses into individual declarations.

 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..42fff8837eab 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";
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tuesday, 13 February 2018 00:07:51 EET Kieran Bingham wrote:
> 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>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> v2:
>  - Addition to series
> 
> v3:
>  - Split map register addresses into individual declarations.
> 
>  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..42fff8837eab
> 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";

-- 
Regards,

Laurent Pinchart

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

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tuesday, 13 February 2018 00:07:51 EET Kieran Bingham wrote:
> 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>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> v2:
>  - Addition to series
> 
> v3:
>  - Split map register addresses into individual declarations.
> 
>  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..42fff8837eab
> 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 at 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 at 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";

-- 
Regards,

Laurent Pinchart

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

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tuesday, 13 February 2018 00:07:51 EET Kieran Bingham wrote:
> From: Kieran Bingham <kieran.bingham+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> 
> 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-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

> ---
> v2:
>  - Addition to series
> 
> v3:
>  - Split map register addresses into individual declarations.
> 
>  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..42fff8837eab
> 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";

-- 
Regards,

Laurent Pinchart

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

[PATCH v3 4/5] media: adv7604: Add support for i2c_new_secondary_device

[Kieran Bingham]

From: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>

The ADV7604 has thirteen 256-byte maps that can be accessed via the main
I²C ports. Each map has it own I²C address and acts as a standard slave
device on the I²C bus.

Allow a device tree node to override the default addresses so that
address conflicts with other devices on the same bus may be resolved at
the board description level.

Signed-off-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
[Kieran: Re-adapted for mainline]
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
Based upon the original posting :
  https://lkml.org/lkml/2014/10/22/469

v2:
 - Split out DT bindings from driver updates
 - Return -EINVAL on error paths from adv76xx_dummy_client()

 drivers/media/i2c/adv7604.c | 62 +++++++++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 22 deletions(-)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 1544920ec52d..872e124793f8 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -2734,6 +2734,27 @@ static const struct v4l2_ctrl_config adv76xx_ctrl_free_run_color = {
 
 /* ----------------------------------------------------------------------- */
 
+struct adv76xx_register {
+	const char *name;
+	u8 default_addr;
+};
+
+static const struct adv76xx_register adv76xx_secondary_names[] = {
+	[ADV76XX_PAGE_IO] = { "main", 0x4c },
+	[ADV7604_PAGE_AVLINK] = { "avlink", 0x42 },
+	[ADV76XX_PAGE_CEC] = { "cec", 0x40 },
+	[ADV76XX_PAGE_INFOFRAME] = { "infoframe", 0x3e },
+	[ADV7604_PAGE_ESDP] = { "esdp", 0x38 },
+	[ADV7604_PAGE_DPP] = { "dpp", 0x3c },
+	[ADV76XX_PAGE_AFE] = { "afe", 0x26 },
+	[ADV76XX_PAGE_REP] = { "rep", 0x32 },
+	[ADV76XX_PAGE_EDID] = { "edid", 0x36 },
+	[ADV76XX_PAGE_HDMI] = { "hdmi", 0x34 },
+	[ADV76XX_PAGE_TEST] = { "test", 0x30 },
+	[ADV76XX_PAGE_CP] = { "cp", 0x22 },
+	[ADV7604_PAGE_VDP] = { "vdp", 0x24 },
+};
+
 static int adv76xx_core_init(struct v4l2_subdev *sd)
 {
 	struct adv76xx_state *state = to_state(sd);
@@ -2834,13 +2855,26 @@ static void adv76xx_unregister_clients(struct adv76xx_state *state)
 }
 
 static struct i2c_client *adv76xx_dummy_client(struct v4l2_subdev *sd,
-							u8 addr, u8 io_reg)
+					       unsigned int i)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct adv76xx_state *state = to_state(sd);
+	struct adv76xx_platform_data *pdata = &state->pdata;
+	unsigned int io_reg = 0xf2 + i;
+	struct i2c_client *new_client;
+
+	if (pdata && pdata->i2c_addresses[i])
+		new_client = i2c_new_dummy(client->adapter,
+					   pdata->i2c_addresses[i]);
+	else
+		new_client = i2c_new_secondary_device(client,
+				adv76xx_secondary_names[i].name,
+				adv76xx_secondary_names[i].default_addr);
 
-	if (addr)
-		io_write(sd, io_reg, addr << 1);
-	return i2c_new_dummy(client->adapter, io_read(sd, io_reg) >> 1);
+	if (new_client)
+		io_write(sd, io_reg, new_client->addr << 1);
+
+	return new_client;
 }
 
 static const struct adv76xx_reg_seq adv7604_recommended_settings_afe[] = {
@@ -3115,20 +3149,6 @@ static int adv76xx_parse_dt(struct adv76xx_state *state)
 	/* Disable the interrupt for now as no DT-based board uses it. */
 	state->pdata.int1_config = ADV76XX_INT1_CONFIG_DISABLED;
 
-	/* Use the default I2C addresses. */
-	state->pdata.i2c_addresses[ADV7604_PAGE_AVLINK] = 0x42;
-	state->pdata.i2c_addresses[ADV76XX_PAGE_CEC] = 0x40;
-	state->pdata.i2c_addresses[ADV76XX_PAGE_INFOFRAME] = 0x3e;
-	state->pdata.i2c_addresses[ADV7604_PAGE_ESDP] = 0x38;
-	state->pdata.i2c_addresses[ADV7604_PAGE_DPP] = 0x3c;
-	state->pdata.i2c_addresses[ADV76XX_PAGE_AFE] = 0x26;
-	state->pdata.i2c_addresses[ADV76XX_PAGE_REP] = 0x32;
-	state->pdata.i2c_addresses[ADV76XX_PAGE_EDID] = 0x36;
-	state->pdata.i2c_addresses[ADV76XX_PAGE_HDMI] = 0x34;
-	state->pdata.i2c_addresses[ADV76XX_PAGE_TEST] = 0x30;
-	state->pdata.i2c_addresses[ADV76XX_PAGE_CP] = 0x22;
-	state->pdata.i2c_addresses[ADV7604_PAGE_VDP] = 0x24;
-
 	/* Hardcode the remaining platform data fields. */
 	state->pdata.disable_pwrdnb = 0;
 	state->pdata.disable_cable_det_rst = 0;
@@ -3478,11 +3498,9 @@ static int adv76xx_probe(struct i2c_client *client,
 		if (!(BIT(i) & state->info->page_mask))
 			continue;
 
-		state->i2c_clients[i] =
-			adv76xx_dummy_client(sd, state->pdata.i2c_addresses[i],
-					     0xf2 + i);
+		state->i2c_clients[i] = adv76xx_dummy_client(sd, i);
 		if (!state->i2c_clients[i]) {
-			err = -ENOMEM;
+			err = -EINVAL;
 			v4l2_err(sd, "failed to create i2c client %u\n", i);
 			goto err_i2c;
 		}
-- 
2.7.4

[PATCH v3 4/5] media: adv7604: Add support for i2c_new_secondary_device

[Kieran Bingham]

From: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>

The ADV7604 has thirteen 256-byte maps that can be accessed via the main
I²C ports. Each map has it own I²C address and acts as a standard slave
device on the I²C bus.

Allow a device tree node to override the default addresses so that
address conflicts with other devices on the same bus may be resolved at
the board description level.

Signed-off-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
[Kieran: Re-adapted for mainline]
Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
Based upon the original posting :
  https://lkml.org/lkml/2014/10/22/469

v2:
 - Split out DT bindings from driver updates
 - Return -EINVAL on error paths from adv76xx_dummy_client()

 drivers/media/i2c/adv7604.c | 62 +++++++++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 22 deletions(-)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 1544920ec52d..872e124793f8 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -2734,6 +2734,27 @@ static const struct v4l2_ctrl_config adv76xx_ctrl_free_run_color = {
 
 /* ----------------------------------------------------------------------- */
 
+struct adv76xx_register {
+	const char *name;
+	u8 default_addr;
+};
+
+static const struct adv76xx_register adv76xx_secondary_names[] = {
+	[ADV76XX_PAGE_IO] = { "main", 0x4c },
+	[ADV7604_PAGE_AVLINK] = { "avlink", 0x42 },
+	[ADV76XX_PAGE_CEC] = { "cec", 0x40 },
+	[ADV76XX_PAGE_INFOFRAME] = { "infoframe", 0x3e },
+	[ADV7604_PAGE_ESDP] = { "esdp", 0x38 },
+	[ADV7604_PAGE_DPP] = { "dpp", 0x3c },
+	[ADV76XX_PAGE_AFE] = { "afe", 0x26 },
+	[ADV76XX_PAGE_REP] = { "rep", 0x32 },
+	[ADV76XX_PAGE_EDID] = { "edid", 0x36 },
+	[ADV76XX_PAGE_HDMI] = { "hdmi", 0x34 },
+	[ADV76XX_PAGE_TEST] = { "test", 0x30 },
+	[ADV76XX_PAGE_CP] = { "cp", 0x22 },
+	[ADV7604_PAGE_VDP] = { "vdp", 0x24 },
+};
+
 static int adv76xx_core_init(struct v4l2_subdev *sd)
 {
 	struct adv76xx_state *state = to_state(sd);
@@ -2834,13 +2855,26 @@ static void adv76xx_unregister_clients(struct adv76xx_state *state)
 }
 
 static struct i2c_client *adv76xx_dummy_client(struct v4l2_subdev *sd,
-							u8 addr, u8 io_reg)
+					       unsigned int i)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct adv76xx_state *state = to_state(sd);
+	struct adv76xx_platform_data *pdata = &state->pdata;
+	unsigned int io_reg = 0xf2 + i;
+	struct i2c_client *new_client;
+
+	if (pdata && pdata->i2c_addresses[i])
+		new_client = i2c_new_dummy(client->adapter,
+					   pdata->i2c_addresses[i]);
+	else
+		new_client = i2c_new_secondary_device(client,
+				adv76xx_secondary_names[i].name,
+				adv76xx_secondary_names[i].default_addr);
 
-	if (addr)
-		io_write(sd, io_reg, addr << 1);
-	return i2c_new_dummy(client->adapter, io_read(sd, io_reg) >> 1);
+	if (new_client)
+		io_write(sd, io_reg, new_client->addr << 1);
+
+	return new_client;
 }
 
 static const struct adv76xx_reg_seq adv7604_recommended_settings_afe[] = {
@@ -3115,20 +3149,6 @@ static int adv76xx_parse_dt(struct adv76xx_state *state)
 	/* Disable the interrupt for now as no DT-based board uses it. */
 	state->pdata.int1_config = ADV76XX_INT1_CONFIG_DISABLED;
 
-	/* Use the default I2C addresses. */
-	state->pdata.i2c_addresses[ADV7604_PAGE_AVLINK] = 0x42;
-	state->pdata.i2c_addresses[ADV76XX_PAGE_CEC] = 0x40;
-	state->pdata.i2c_addresses[ADV76XX_PAGE_INFOFRAME] = 0x3e;
-	state->pdata.i2c_addresses[ADV7604_PAGE_ESDP] = 0x38;
-	state->pdata.i2c_addresses[ADV7604_PAGE_DPP] = 0x3c;
-	state->pdata.i2c_addresses[ADV76XX_PAGE_AFE] = 0x26;
-	state->pdata.i2c_addresses[ADV76XX_PAGE_REP] = 0x32;
-	state->pdata.i2c_addresses[ADV76XX_PAGE_EDID] = 0x36;
-	state->pdata.i2c_addresses[ADV76XX_PAGE_HDMI] = 0x34;
-	state->pdata.i2c_addresses[ADV76XX_PAGE_TEST] = 0x30;
-	state->pdata.i2c_addresses[ADV76XX_PAGE_CP] = 0x22;
-	state->pdata.i2c_addresses[ADV7604_PAGE_VDP] = 0x24;
-
 	/* Hardcode the remaining platform data fields. */
 	state->pdata.disable_pwrdnb = 0;
 	state->pdata.disable_cable_det_rst = 0;
@@ -3478,11 +3498,9 @@ static int adv76xx_probe(struct i2c_client *client,
 		if (!(BIT(i) & state->info->page_mask))
 			continue;
 
-		state->i2c_clients[i] =
-			adv76xx_dummy_client(sd, state->pdata.i2c_addresses[i],
-					     0xf2 + i);
+		state->i2c_clients[i] = adv76xx_dummy_client(sd, i);
 		if (!state->i2c_clients[i]) {
-			err = -ENOMEM;
+			err = -EINVAL;
 			v4l2_err(sd, "failed to create i2c client %u\n", i);
 			goto err_i2c;
 		}
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 4/5] media: adv7604: Add support for i2c_new_secondary_device

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tuesday, 13 February 2018 00:07:52 EET Kieran Bingham wrote:
> From: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
> 
> The ADV7604 has thirteen 256-byte maps that can be accessed via the main
> I²C ports. Each map has it own I²C address and acts as a standard slave
> device on the I²C bus.
> 
> Allow a device tree node to override the default addresses so that
> address conflicts with other devices on the same bus may be resolved at
> the board description level.
> 
> Signed-off-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
> [Kieran: Re-adapted for mainline]
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> ---
> Based upon the original posting :
>   https://lkml.org/lkml/2014/10/22/469
> 
> v2:
>  - Split out DT bindings from driver updates
>  - Return -EINVAL on error paths from adv76xx_dummy_client()
> 
>  drivers/media/i2c/adv7604.c | 62 ++++++++++++++++++++++++++----------------
>  1 file changed, 40 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 1544920ec52d..872e124793f8 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -2734,6 +2734,27 @@ static const struct v4l2_ctrl_config
> adv76xx_ctrl_free_run_color = {
> 
>  /* --------------------------------------------------------------------- */
> 
> +struct adv76xx_register {

adv76xx_register seems to imply that this describes a particular register, 
while the structure describes a registers map. How about adv76xx_register_map, 
adv76xx_register_bank or adv76xx_register_page ?

> +	const char *name;
> +	u8 default_addr;
> +};
> +
> +static const struct adv76xx_register adv76xx_secondary_names[] = {

The table doesn't contain secondary names only as there's an entry for the 
main map. How about calling it adv76xx_default_addresses or something along 
the same line ?

> +	[ADV76XX_PAGE_IO] = { "main", 0x4c },
> +	[ADV7604_PAGE_AVLINK] = { "avlink", 0x42 },
> +	[ADV76XX_PAGE_CEC] = { "cec", 0x40 },
> +	[ADV76XX_PAGE_INFOFRAME] = { "infoframe", 0x3e },
> +	[ADV7604_PAGE_ESDP] = { "esdp", 0x38 },
> +	[ADV7604_PAGE_DPP] = { "dpp", 0x3c },
> +	[ADV76XX_PAGE_AFE] = { "afe", 0x26 },
> +	[ADV76XX_PAGE_REP] = { "rep", 0x32 },
> +	[ADV76XX_PAGE_EDID] = { "edid", 0x36 },
> +	[ADV76XX_PAGE_HDMI] = { "hdmi", 0x34 },
> +	[ADV76XX_PAGE_TEST] = { "test", 0x30 },
> +	[ADV76XX_PAGE_CP] = { "cp", 0x22 },
> +	[ADV7604_PAGE_VDP] = { "vdp", 0x24 },
> +};
> +
>  static int adv76xx_core_init(struct v4l2_subdev *sd)
>  {
>  	struct adv76xx_state *state = to_state(sd);
> @@ -2834,13 +2855,26 @@ static void adv76xx_unregister_clients(struct
> adv76xx_state *state) }
> 
>  static struct i2c_client *adv76xx_dummy_client(struct v4l2_subdev *sd,
> -							u8 addr, u8 io_reg)
> +					       unsigned int i)

Maybe unsigned int page ?

With these fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  {
>  	struct i2c_client *client = v4l2_get_subdevdata(sd);
> +	struct adv76xx_state *state = to_state(sd);
> +	struct adv76xx_platform_data *pdata = &state->pdata;
> +	unsigned int io_reg = 0xf2 + i;
> +	struct i2c_client *new_client;
> +
> +	if (pdata && pdata->i2c_addresses[i])
> +		new_client = i2c_new_dummy(client->adapter,
> +					   pdata->i2c_addresses[i]);
> +	else
> +		new_client = i2c_new_secondary_device(client,
> +				adv76xx_secondary_names[i].name,
> +				adv76xx_secondary_names[i].default_addr);
> 
> -	if (addr)
> -		io_write(sd, io_reg, addr << 1);
> -	return i2c_new_dummy(client->adapter, io_read(sd, io_reg) >> 1);
> +	if (new_client)
> +		io_write(sd, io_reg, new_client->addr << 1);
> +
> +	return new_client;
>  }
> 
>  static const struct adv76xx_reg_seq adv7604_recommended_settings_afe[] = {
> @@ -3115,20 +3149,6 @@ static int adv76xx_parse_dt(struct adv76xx_state
> *state) /* Disable the interrupt for now as no DT-based board uses it. */
> state->pdata.int1_config = ADV76XX_INT1_CONFIG_DISABLED;
> 
> -	/* Use the default I2C addresses. */
> -	state->pdata.i2c_addresses[ADV7604_PAGE_AVLINK] = 0x42;
> -	state->pdata.i2c_addresses[ADV76XX_PAGE_CEC] = 0x40;
> -	state->pdata.i2c_addresses[ADV76XX_PAGE_INFOFRAME] = 0x3e;
> -	state->pdata.i2c_addresses[ADV7604_PAGE_ESDP] = 0x38;
> -	state->pdata.i2c_addresses[ADV7604_PAGE_DPP] = 0x3c;
> -	state->pdata.i2c_addresses[ADV76XX_PAGE_AFE] = 0x26;
> -	state->pdata.i2c_addresses[ADV76XX_PAGE_REP] = 0x32;
> -	state->pdata.i2c_addresses[ADV76XX_PAGE_EDID] = 0x36;
> -	state->pdata.i2c_addresses[ADV76XX_PAGE_HDMI] = 0x34;
> -	state->pdata.i2c_addresses[ADV76XX_PAGE_TEST] = 0x30;
> -	state->pdata.i2c_addresses[ADV76XX_PAGE_CP] = 0x22;
> -	state->pdata.i2c_addresses[ADV7604_PAGE_VDP] = 0x24;
> -
>  	/* Hardcode the remaining platform data fields. */
>  	state->pdata.disable_pwrdnb = 0;
>  	state->pdata.disable_cable_det_rst = 0;
> @@ -3478,11 +3498,9 @@ static int adv76xx_probe(struct i2c_client *client,
>  		if (!(BIT(i) & state->info->page_mask))
>  			continue;
> 
> -		state->i2c_clients[i] =
> -			adv76xx_dummy_client(sd, state->pdata.i2c_addresses[i],
> -					     0xf2 + i);
> +		state->i2c_clients[i] = adv76xx_dummy_client(sd, i);
>  		if (!state->i2c_clients[i]) {
> -			err = -ENOMEM;
> +			err = -EINVAL;
>  			v4l2_err(sd, "failed to create i2c client %u\n", i);
>  			goto err_i2c;
>  		}

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 4/5] media: adv7604: Add support for i2c_new_secondary_device

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tuesday, 13 February 2018 00:07:52 EET Kieran Bingham wrote:
> From: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
> 
> The ADV7604 has thirteen 256-byte maps that can be accessed via the main
> I²C ports. Each map has it own I²C address and acts as a standard slave
> device on the I²C bus.
> 
> Allow a device tree node to override the default addresses so that
> address conflicts with other devices on the same bus may be resolved at
> the board description level.
> 
> Signed-off-by: Jean-Michel Hautbois <jean-michel.hautbois@vodalys.com>
> [Kieran: Re-adapted for mainline]
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> ---
> Based upon the original posting :
>   https://lkml.org/lkml/2014/10/22/469
> 
> v2:
>  - Split out DT bindings from driver updates
>  - Return -EINVAL on error paths from adv76xx_dummy_client()
> 
>  drivers/media/i2c/adv7604.c | 62 ++++++++++++++++++++++++++----------------
>  1 file changed, 40 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 1544920ec52d..872e124793f8 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -2734,6 +2734,27 @@ static const struct v4l2_ctrl_config
> adv76xx_ctrl_free_run_color = {
> 
>  /* --------------------------------------------------------------------- */
> 
> +struct adv76xx_register {

adv76xx_register seems to imply that this describes a particular register, 
while the structure describes a registers map. How about adv76xx_register_map, 
adv76xx_register_bank or adv76xx_register_page ?

> +	const char *name;
> +	u8 default_addr;
> +};
> +
> +static const struct adv76xx_register adv76xx_secondary_names[] = {

The table doesn't contain secondary names only as there's an entry for the 
main map. How about calling it adv76xx_default_addresses or something along 
the same line ?

> +	[ADV76XX_PAGE_IO] = { "main", 0x4c },
> +	[ADV7604_PAGE_AVLINK] = { "avlink", 0x42 },
> +	[ADV76XX_PAGE_CEC] = { "cec", 0x40 },
> +	[ADV76XX_PAGE_INFOFRAME] = { "infoframe", 0x3e },
> +	[ADV7604_PAGE_ESDP] = { "esdp", 0x38 },
> +	[ADV7604_PAGE_DPP] = { "dpp", 0x3c },
> +	[ADV76XX_PAGE_AFE] = { "afe", 0x26 },
> +	[ADV76XX_PAGE_REP] = { "rep", 0x32 },
> +	[ADV76XX_PAGE_EDID] = { "edid", 0x36 },
> +	[ADV76XX_PAGE_HDMI] = { "hdmi", 0x34 },
> +	[ADV76XX_PAGE_TEST] = { "test", 0x30 },
> +	[ADV76XX_PAGE_CP] = { "cp", 0x22 },
> +	[ADV7604_PAGE_VDP] = { "vdp", 0x24 },
> +};
> +
>  static int adv76xx_core_init(struct v4l2_subdev *sd)
>  {
>  	struct adv76xx_state *state = to_state(sd);
> @@ -2834,13 +2855,26 @@ static void adv76xx_unregister_clients(struct
> adv76xx_state *state) }
> 
>  static struct i2c_client *adv76xx_dummy_client(struct v4l2_subdev *sd,
> -							u8 addr, u8 io_reg)
> +					       unsigned int i)

Maybe unsigned int page ?

With these fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  {
>  	struct i2c_client *client = v4l2_get_subdevdata(sd);
> +	struct adv76xx_state *state = to_state(sd);
> +	struct adv76xx_platform_data *pdata = &state->pdata;
> +	unsigned int io_reg = 0xf2 + i;
> +	struct i2c_client *new_client;
> +
> +	if (pdata && pdata->i2c_addresses[i])
> +		new_client = i2c_new_dummy(client->adapter,
> +					   pdata->i2c_addresses[i]);
> +	else
> +		new_client = i2c_new_secondary_device(client,
> +				adv76xx_secondary_names[i].name,
> +				adv76xx_secondary_names[i].default_addr);
> 
> -	if (addr)
> -		io_write(sd, io_reg, addr << 1);
> -	return i2c_new_dummy(client->adapter, io_read(sd, io_reg) >> 1);
> +	if (new_client)
> +		io_write(sd, io_reg, new_client->addr << 1);
> +
> +	return new_client;
>  }
> 
>  static const struct adv76xx_reg_seq adv7604_recommended_settings_afe[] = {
> @@ -3115,20 +3149,6 @@ static int adv76xx_parse_dt(struct adv76xx_state
> *state) /* Disable the interrupt for now as no DT-based board uses it. */
> state->pdata.int1_config = ADV76XX_INT1_CONFIG_DISABLED;
> 
> -	/* Use the default I2C addresses. */
> -	state->pdata.i2c_addresses[ADV7604_PAGE_AVLINK] = 0x42;
> -	state->pdata.i2c_addresses[ADV76XX_PAGE_CEC] = 0x40;
> -	state->pdata.i2c_addresses[ADV76XX_PAGE_INFOFRAME] = 0x3e;
> -	state->pdata.i2c_addresses[ADV7604_PAGE_ESDP] = 0x38;
> -	state->pdata.i2c_addresses[ADV7604_PAGE_DPP] = 0x3c;
> -	state->pdata.i2c_addresses[ADV76XX_PAGE_AFE] = 0x26;
> -	state->pdata.i2c_addresses[ADV76XX_PAGE_REP] = 0x32;
> -	state->pdata.i2c_addresses[ADV76XX_PAGE_EDID] = 0x36;
> -	state->pdata.i2c_addresses[ADV76XX_PAGE_HDMI] = 0x34;
> -	state->pdata.i2c_addresses[ADV76XX_PAGE_TEST] = 0x30;
> -	state->pdata.i2c_addresses[ADV76XX_PAGE_CP] = 0x22;
> -	state->pdata.i2c_addresses[ADV7604_PAGE_VDP] = 0x24;
> -
>  	/* Hardcode the remaining platform data fields. */
>  	state->pdata.disable_pwrdnb = 0;
>  	state->pdata.disable_cable_det_rst = 0;
> @@ -3478,11 +3498,9 @@ static int adv76xx_probe(struct i2c_client *client,
>  		if (!(BIT(i) & state->info->page_mask))
>  			continue;
> 
> -		state->i2c_clients[i] =
> -			adv76xx_dummy_client(sd, state->pdata.i2c_addresses[i],
> -					     0xf2 + i);
> +		state->i2c_clients[i] = adv76xx_dummy_client(sd, i);
>  		if (!state->i2c_clients[i]) {
> -			err = -ENOMEM;
> +			err = -EINVAL;
>  			v4l2_err(sd, "failed to create i2c client %u\n", i);
>  			goto err_i2c;
>  		}

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 5/5] drm: adv7511: Add support for i2c_new_secondary_device

[Kieran Bingham]

From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

The ADV7511 has four 256-byte maps that can be accessed via the main I²C
ports. Each map has it own I²C address and acts as a standard slave
device on the I²C bus.

Allow a device tree node to override the default addresses so that
address conflicts with other devices on the same bus may be resolved at
the board description level.

Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
v2:
 - Update missing edid-i2c address setting
 - Split out DT bindings
 - Rename and move the I2C default addresses to their own section

 drivers/gpu/drm/bridge/adv7511/adv7511.h     |  6 ++++
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 42 ++++++++++++++++++----------
 2 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index d034b2cb5eee..04e6759ee45b 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -93,6 +93,11 @@
 #define ADV7511_REG_CHIP_ID_HIGH		0xf5
 #define ADV7511_REG_CHIP_ID_LOW			0xf6
 
+/* Hardware defined default addresses for i2c register maps */
+#define ADV7511_CEC_I2C_ADDR_DEFAULT		0x3c
+#define ADV7511_EDID_I2C_ADDR_DEFAULT		0x3f
+#define ADV7511_PACKET_I2C_ADDR_DEFAULT		0x38
+
 #define ADV7511_CSC_ENABLE			BIT(7)
 #define ADV7511_CSC_UPDATE_MODE			BIT(5)
 
@@ -322,6 +327,7 @@ struct adv7511 {
 	struct i2c_client *i2c_main;
 	struct i2c_client *i2c_edid;
 	struct i2c_client *i2c_cec;
+	struct i2c_client *i2c_packet;
 
 	struct regmap *regmap;
 	struct regmap *regmap_cec;
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index efa29db5fc2b..5e61b928c9c0 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -586,7 +586,7 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
 	/* Reading the EDID only works if the device is powered */
 	if (!adv7511->powered) {
 		unsigned int edid_i2c_addr =
-					(adv7511->i2c_main->addr << 1) + 4;
+					(adv7511->i2c_edid->addr << 1);
 
 		__adv7511_power_on(adv7511);
 
@@ -969,10 +969,10 @@ static int adv7511_init_cec_regmap(struct adv7511 *adv)
 {
 	int ret;
 
-	adv->i2c_cec = i2c_new_dummy(adv->i2c_main->adapter,
-				     adv->i2c_main->addr - 1);
+	adv->i2c_cec = i2c_new_secondary_device(adv->i2c_main, "cec",
+					ADV7511_CEC_I2C_ADDR_DEFAULT);
 	if (!adv->i2c_cec)
-		return -ENOMEM;
+		return -EINVAL;
 	i2c_set_clientdata(adv->i2c_cec, adv);
 
 	adv->regmap_cec = devm_regmap_init_i2c(adv->i2c_cec,
@@ -1082,8 +1082,6 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 	struct adv7511_link_config link_config;
 	struct adv7511 *adv7511;
 	struct device *dev = &i2c->dev;
-	unsigned int main_i2c_addr = i2c->addr << 1;
-	unsigned int edid_i2c_addr = main_i2c_addr + 4;
 	unsigned int val;
 	int ret;
 
@@ -1153,24 +1151,35 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 	if (ret)
 		goto uninit_regulators;
 
-	regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr);
-	regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
-		     main_i2c_addr - 0xa);
-	regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR,
-		     main_i2c_addr - 2);
-
 	adv7511_packet_disable(adv7511, 0xffff);
 
-	adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1);
+	adv7511->i2c_edid = i2c_new_secondary_device(i2c, "edid",
+					ADV7511_EDID_I2C_ADDR_DEFAULT);
 	if (!adv7511->i2c_edid) {
-		ret = -ENOMEM;
+		ret = -EINVAL;
 		goto uninit_regulators;
 	}
 
+	regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR,
+		     adv7511->i2c_edid->addr << 1);
+
 	ret = adv7511_init_cec_regmap(adv7511);
 	if (ret)
 		goto err_i2c_unregister_edid;
 
+	regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR,
+		     adv7511->i2c_cec->addr << 1);
+
+	adv7511->i2c_packet = i2c_new_secondary_device(i2c, "packet",
+					ADV7511_PACKET_I2C_ADDR_DEFAULT);
+	if (!adv7511->i2c_packet) {
+		ret = -EINVAL;
+		goto err_unregister_cec;
+	}
+
+	regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
+		     adv7511->i2c_packet->addr << 1);
+
 	INIT_WORK(&adv7511->hpd_work, adv7511_hpd_work);
 
 	if (i2c->irq) {
@@ -1181,7 +1190,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 						IRQF_ONESHOT, dev_name(dev),
 						adv7511);
 		if (ret)
-			goto err_unregister_cec;
+			goto err_unregister_packet;
 	}
 
 	adv7511_power_off(adv7511);
@@ -1203,6 +1212,8 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
 	adv7511_audio_init(dev, adv7511);
 	return 0;
 
+err_unregister_packet:
+	i2c_unregister_device(adv7511->i2c_packet);
 err_unregister_cec:
 	i2c_unregister_device(adv7511->i2c_cec);
 	if (adv7511->cec_clk)
@@ -1234,6 +1245,7 @@ static int adv7511_remove(struct i2c_client *i2c)
 	cec_unregister_adapter(adv7511->cec_adap);
 
 	i2c_unregister_device(adv7511->i2c_edid);
+	i2c_unregister_device(adv7511->i2c_packet);
 
 	return 0;
 }
-- 
2.7.4

Re: [PATCH v3 5/5] drm: adv7511: Add support for i2c_new_secondary_device

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tuesday, 13 February 2018 00:07:53 EET Kieran Bingham wrote:
> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> The ADV7511 has four 256-byte maps that can be accessed via the main I²C
> ports. Each map has it own I²C address and acts as a standard slave
> device on the I²C bus.
> 
> Allow a device tree node to override the default addresses so that
> address conflicts with other devices on the same bus may be resolved at
> the board description level.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
> v2:
>  - Update missing edid-i2c address setting
>  - Split out DT bindings
>  - Rename and move the I2C default addresses to their own section
> 
>  drivers/gpu/drm/bridge/adv7511/adv7511.h     |  6 ++++
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 42 ++++++++++++++++--------
>  2 files changed, 33 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index d034b2cb5eee..04e6759ee45b
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -93,6 +93,11 @@
>  #define ADV7511_REG_CHIP_ID_HIGH		0xf5
>  #define ADV7511_REG_CHIP_ID_LOW			0xf6
> 
> +/* Hardware defined default addresses for i2c register maps */

s/i2c/I2C/ ? That's really because I had to find something :-)

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +#define ADV7511_CEC_I2C_ADDR_DEFAULT		0x3c
> +#define ADV7511_EDID_I2C_ADDR_DEFAULT		0x3f
> +#define ADV7511_PACKET_I2C_ADDR_DEFAULT		0x38
> +
>  #define ADV7511_CSC_ENABLE			BIT(7)
>  #define ADV7511_CSC_UPDATE_MODE			BIT(5)
> 
> @@ -322,6 +327,7 @@ struct adv7511 {
>  	struct i2c_client *i2c_main;
>  	struct i2c_client *i2c_edid;
>  	struct i2c_client *i2c_cec;
> +	struct i2c_client *i2c_packet;
> 
>  	struct regmap *regmap;
>  	struct regmap *regmap_cec;
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index
> efa29db5fc2b..5e61b928c9c0 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -586,7 +586,7 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
>  	/* Reading the EDID only works if the device is powered */
>  	if (!adv7511->powered) {
>  		unsigned int edid_i2c_addr =
> -					(adv7511->i2c_main->addr << 1) + 4;
> +					(adv7511->i2c_edid->addr << 1);
> 
>  		__adv7511_power_on(adv7511);
> 
> @@ -969,10 +969,10 @@ static int adv7511_init_cec_regmap(struct adv7511
> *adv) {
>  	int ret;
> 
> -	adv->i2c_cec = i2c_new_dummy(adv->i2c_main->adapter,
> -				     adv->i2c_main->addr - 1);
> +	adv->i2c_cec = i2c_new_secondary_device(adv->i2c_main, "cec",
> +					ADV7511_CEC_I2C_ADDR_DEFAULT);
>  	if (!adv->i2c_cec)
> -		return -ENOMEM;
> +		return -EINVAL;
>  	i2c_set_clientdata(adv->i2c_cec, adv);
> 
>  	adv->regmap_cec = devm_regmap_init_i2c(adv->i2c_cec,
> @@ -1082,8 +1082,6 @@ static int adv7511_probe(struct i2c_client *i2c, const
> struct i2c_device_id *id) struct adv7511_link_config link_config;
>  	struct adv7511 *adv7511;
>  	struct device *dev = &i2c->dev;
> -	unsigned int main_i2c_addr = i2c->addr << 1;
> -	unsigned int edid_i2c_addr = main_i2c_addr + 4;
>  	unsigned int val;
>  	int ret;
> 
> @@ -1153,24 +1151,35 @@ static int adv7511_probe(struct i2c_client *i2c,
> const struct i2c_device_id *id) if (ret)
>  		goto uninit_regulators;
> 
> -	regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr);
> -	regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
> -		     main_i2c_addr - 0xa);
> -	regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR,
> -		     main_i2c_addr - 2);
> -
>  	adv7511_packet_disable(adv7511, 0xffff);
> 
> -	adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1);
> +	adv7511->i2c_edid = i2c_new_secondary_device(i2c, "edid",
> +					ADV7511_EDID_I2C_ADDR_DEFAULT);
>  	if (!adv7511->i2c_edid) {
> -		ret = -ENOMEM;
> +		ret = -EINVAL;
>  		goto uninit_regulators;
>  	}
> 
> +	regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR,
> +		     adv7511->i2c_edid->addr << 1);
> +
>  	ret = adv7511_init_cec_regmap(adv7511);
>  	if (ret)
>  		goto err_i2c_unregister_edid;
> 
> +	regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR,
> +		     adv7511->i2c_cec->addr << 1);
> +
> +	adv7511->i2c_packet = i2c_new_secondary_device(i2c, "packet",
> +					ADV7511_PACKET_I2C_ADDR_DEFAULT);
> +	if (!adv7511->i2c_packet) {
> +		ret = -EINVAL;
> +		goto err_unregister_cec;
> +	}
> +
> +	regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
> +		     adv7511->i2c_packet->addr << 1);
> +
>  	INIT_WORK(&adv7511->hpd_work, adv7511_hpd_work);
> 
>  	if (i2c->irq) {
> @@ -1181,7 +1190,7 @@ static int adv7511_probe(struct i2c_client *i2c, const
> struct i2c_device_id *id) IRQF_ONESHOT, dev_name(dev),
>  						adv7511);
>  		if (ret)
> -			goto err_unregister_cec;
> +			goto err_unregister_packet;
>  	}
> 
>  	adv7511_power_off(adv7511);
> @@ -1203,6 +1212,8 @@ static int adv7511_probe(struct i2c_client *i2c, const
> struct i2c_device_id *id) adv7511_audio_init(dev, adv7511);
>  	return 0;
> 
> +err_unregister_packet:
> +	i2c_unregister_device(adv7511->i2c_packet);
>  err_unregister_cec:
>  	i2c_unregister_device(adv7511->i2c_cec);
>  	if (adv7511->cec_clk)
> @@ -1234,6 +1245,7 @@ static int adv7511_remove(struct i2c_client *i2c)
>  	cec_unregister_adapter(adv7511->cec_adap);
> 
>  	i2c_unregister_device(adv7511->i2c_edid);
> +	i2c_unregister_device(adv7511->i2c_packet);
> 
>  	return 0;
>  }

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 5/5] drm: adv7511: Add support for i2c_new_secondary_device

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tuesday, 13 February 2018 00:07:53 EET Kieran Bingham wrote:
> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> The ADV7511 has four 256-byte maps that can be accessed via the main I²C
> ports. Each map has it own I²C address and acts as a standard slave
> device on the I²C bus.
> 
> Allow a device tree node to override the default addresses so that
> address conflicts with other devices on the same bus may be resolved at
> the board description level.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
> v2:
>  - Update missing edid-i2c address setting
>  - Split out DT bindings
>  - Rename and move the I2C default addresses to their own section
> 
>  drivers/gpu/drm/bridge/adv7511/adv7511.h     |  6 ++++
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 42 ++++++++++++++++--------
>  2 files changed, 33 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index d034b2cb5eee..04e6759ee45b
> 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
> @@ -93,6 +93,11 @@
>  #define ADV7511_REG_CHIP_ID_HIGH		0xf5
>  #define ADV7511_REG_CHIP_ID_LOW			0xf6
> 
> +/* Hardware defined default addresses for i2c register maps */

s/i2c/I2C/ ? That's really because I had to find something :-)

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +#define ADV7511_CEC_I2C_ADDR_DEFAULT		0x3c
> +#define ADV7511_EDID_I2C_ADDR_DEFAULT		0x3f
> +#define ADV7511_PACKET_I2C_ADDR_DEFAULT		0x38
> +
>  #define ADV7511_CSC_ENABLE			BIT(7)
>  #define ADV7511_CSC_UPDATE_MODE			BIT(5)
> 
> @@ -322,6 +327,7 @@ struct adv7511 {
>  	struct i2c_client *i2c_main;
>  	struct i2c_client *i2c_edid;
>  	struct i2c_client *i2c_cec;
> +	struct i2c_client *i2c_packet;
> 
>  	struct regmap *regmap;
>  	struct regmap *regmap_cec;
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index
> efa29db5fc2b..5e61b928c9c0 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -586,7 +586,7 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
>  	/* Reading the EDID only works if the device is powered */
>  	if (!adv7511->powered) {
>  		unsigned int edid_i2c_addr =
> -					(adv7511->i2c_main->addr << 1) + 4;
> +					(adv7511->i2c_edid->addr << 1);
> 
>  		__adv7511_power_on(adv7511);
> 
> @@ -969,10 +969,10 @@ static int adv7511_init_cec_regmap(struct adv7511
> *adv) {
>  	int ret;
> 
> -	adv->i2c_cec = i2c_new_dummy(adv->i2c_main->adapter,
> -				     adv->i2c_main->addr - 1);
> +	adv->i2c_cec = i2c_new_secondary_device(adv->i2c_main, "cec",
> +					ADV7511_CEC_I2C_ADDR_DEFAULT);
>  	if (!adv->i2c_cec)
> -		return -ENOMEM;
> +		return -EINVAL;
>  	i2c_set_clientdata(adv->i2c_cec, adv);
> 
>  	adv->regmap_cec = devm_regmap_init_i2c(adv->i2c_cec,
> @@ -1082,8 +1082,6 @@ static int adv7511_probe(struct i2c_client *i2c, const
> struct i2c_device_id *id) struct adv7511_link_config link_config;
>  	struct adv7511 *adv7511;
>  	struct device *dev = &i2c->dev;
> -	unsigned int main_i2c_addr = i2c->addr << 1;
> -	unsigned int edid_i2c_addr = main_i2c_addr + 4;
>  	unsigned int val;
>  	int ret;
> 
> @@ -1153,24 +1151,35 @@ static int adv7511_probe(struct i2c_client *i2c,
> const struct i2c_device_id *id) if (ret)
>  		goto uninit_regulators;
> 
> -	regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr);
> -	regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
> -		     main_i2c_addr - 0xa);
> -	regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR,
> -		     main_i2c_addr - 2);
> -
>  	adv7511_packet_disable(adv7511, 0xffff);
> 
> -	adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1);
> +	adv7511->i2c_edid = i2c_new_secondary_device(i2c, "edid",
> +					ADV7511_EDID_I2C_ADDR_DEFAULT);
>  	if (!adv7511->i2c_edid) {
> -		ret = -ENOMEM;
> +		ret = -EINVAL;
>  		goto uninit_regulators;
>  	}
> 
> +	regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR,
> +		     adv7511->i2c_edid->addr << 1);
> +
>  	ret = adv7511_init_cec_regmap(adv7511);
>  	if (ret)
>  		goto err_i2c_unregister_edid;
> 
> +	regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR,
> +		     adv7511->i2c_cec->addr << 1);
> +
> +	adv7511->i2c_packet = i2c_new_secondary_device(i2c, "packet",
> +					ADV7511_PACKET_I2C_ADDR_DEFAULT);
> +	if (!adv7511->i2c_packet) {
> +		ret = -EINVAL;
> +		goto err_unregister_cec;
> +	}
> +
> +	regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
> +		     adv7511->i2c_packet->addr << 1);
> +
>  	INIT_WORK(&adv7511->hpd_work, adv7511_hpd_work);
> 
>  	if (i2c->irq) {
> @@ -1181,7 +1190,7 @@ static int adv7511_probe(struct i2c_client *i2c, const
> struct i2c_device_id *id) IRQF_ONESHOT, dev_name(dev),
>  						adv7511);
>  		if (ret)
> -			goto err_unregister_cec;
> +			goto err_unregister_packet;
>  	}
> 
>  	adv7511_power_off(adv7511);
> @@ -1203,6 +1212,8 @@ static int adv7511_probe(struct i2c_client *i2c, const
> struct i2c_device_id *id) adv7511_audio_init(dev, adv7511);
>  	return 0;
> 
> +err_unregister_packet:
> +	i2c_unregister_device(adv7511->i2c_packet);
>  err_unregister_cec:
>  	i2c_unregister_device(adv7511->i2c_cec);
>  	if (adv7511->cec_clk)
> @@ -1234,6 +1245,7 @@ static int adv7511_remove(struct i2c_client *i2c)
>  	cec_unregister_adapter(adv7511->cec_adap);
> 
>  	i2c_unregister_device(adv7511->i2c_edid);
> +	i2c_unregister_device(adv7511->i2c_packet);
> 
>  	return 0;
>  }

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 5/5] drm: adv7511: Add support for i2c_new_secondary_device

[Kieran Bingham]

Hi Laurent,

Thanks for the review,

On 13/02/18 12:23, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Tuesday, 13 February 2018 00:07:53 EET Kieran Bingham wrote:
>> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>
>> The ADV7511 has four 256-byte maps that can be accessed via the main I²C
>> ports. Each map has it own I²C address and acts as a standard slave
>> device on the I²C bus.
>>
>> Allow a device tree node to override the default addresses so that
>> address conflicts with other devices on the same bus may be resolved at
>> the board description level.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> ---
>> v2:
>>  - Update missing edid-i2c address setting
>>  - Split out DT bindings
>>  - Rename and move the I2C default addresses to their own section
>>
>>  drivers/gpu/drm/bridge/adv7511/adv7511.h     |  6 ++++
>>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 42 ++++++++++++++++--------
>>  2 files changed, 33 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index d034b2cb5eee..04e6759ee45b
>> 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> @@ -93,6 +93,11 @@
>>  #define ADV7511_REG_CHIP_ID_HIGH		0xf5
>>  #define ADV7511_REG_CHIP_ID_LOW			0xf6
>>
>> +/* Hardware defined default addresses for i2c register maps */
> 
> s/i2c/I2C/ ? That's really because I had to find something :-)

The I²C comes from JMH's original patch, but is much harder to grep for, so
normalising to I2C throughout.


> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com
Collected, Thanks,
--
Kieran

> 
>> +#define ADV7511_CEC_I2C_ADDR_DEFAULT		0x3c
>> +#define ADV7511_EDID_I2C_ADDR_DEFAULT		0x3f
>> +#define ADV7511_PACKET_I2C_ADDR_DEFAULT		0x38
>> +
>>  #define ADV7511_CSC_ENABLE			BIT(7)
>>  #define ADV7511_CSC_UPDATE_MODE			BIT(5)
>>
>> @@ -322,6 +327,7 @@ struct adv7511 {
>>  	struct i2c_client *i2c_main;
>>  	struct i2c_client *i2c_edid;
>>  	struct i2c_client *i2c_cec;
>> +	struct i2c_client *i2c_packet;
>>
>>  	struct regmap *regmap;
>>  	struct regmap *regmap_cec;
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index
>> efa29db5fc2b..5e61b928c9c0 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> @@ -586,7 +586,7 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
>>  	/* Reading the EDID only works if the device is powered */
>>  	if (!adv7511->powered) {
>>  		unsigned int edid_i2c_addr =
>> -					(adv7511->i2c_main->addr << 1) + 4;
>> +					(adv7511->i2c_edid->addr << 1);
>>
>>  		__adv7511_power_on(adv7511);
>>
>> @@ -969,10 +969,10 @@ static int adv7511_init_cec_regmap(struct adv7511
>> *adv) {
>>  	int ret;
>>
>> -	adv->i2c_cec = i2c_new_dummy(adv->i2c_main->adapter,
>> -				     adv->i2c_main->addr - 1);
>> +	adv->i2c_cec = i2c_new_secondary_device(adv->i2c_main, "cec",
>> +					ADV7511_CEC_I2C_ADDR_DEFAULT);
>>  	if (!adv->i2c_cec)
>> -		return -ENOMEM;
>> +		return -EINVAL;
>>  	i2c_set_clientdata(adv->i2c_cec, adv);
>>
>>  	adv->regmap_cec = devm_regmap_init_i2c(adv->i2c_cec,
>> @@ -1082,8 +1082,6 @@ static int adv7511_probe(struct i2c_client *i2c, const
>> struct i2c_device_id *id) struct adv7511_link_config link_config;
>>  	struct adv7511 *adv7511;
>>  	struct device *dev = &i2c->dev;
>> -	unsigned int main_i2c_addr = i2c->addr << 1;
>> -	unsigned int edid_i2c_addr = main_i2c_addr + 4;
>>  	unsigned int val;
>>  	int ret;
>>
>> @@ -1153,24 +1151,35 @@ static int adv7511_probe(struct i2c_client *i2c,
>> const struct i2c_device_id *id) if (ret)
>>  		goto uninit_regulators;
>>
>> -	regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr);
>> -	regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
>> -		     main_i2c_addr - 0xa);
>> -	regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR,
>> -		     main_i2c_addr - 2);
>> -
>>  	adv7511_packet_disable(adv7511, 0xffff);
>>
>> -	adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1);
>> +	adv7511->i2c_edid = i2c_new_secondary_device(i2c, "edid",
>> +					ADV7511_EDID_I2C_ADDR_DEFAULT);
>>  	if (!adv7511->i2c_edid) {
>> -		ret = -ENOMEM;
>> +		ret = -EINVAL;
>>  		goto uninit_regulators;
>>  	}
>>
>> +	regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR,
>> +		     adv7511->i2c_edid->addr << 1);
>> +
>>  	ret = adv7511_init_cec_regmap(adv7511);
>>  	if (ret)
>>  		goto err_i2c_unregister_edid;
>>
>> +	regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR,
>> +		     adv7511->i2c_cec->addr << 1);
>> +
>> +	adv7511->i2c_packet = i2c_new_secondary_device(i2c, "packet",
>> +					ADV7511_PACKET_I2C_ADDR_DEFAULT);
>> +	if (!adv7511->i2c_packet) {
>> +		ret = -EINVAL;
>> +		goto err_unregister_cec;
>> +	}
>> +
>> +	regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
>> +		     adv7511->i2c_packet->addr << 1);
>> +
>>  	INIT_WORK(&adv7511->hpd_work, adv7511_hpd_work);
>>
>>  	if (i2c->irq) {
>> @@ -1181,7 +1190,7 @@ static int adv7511_probe(struct i2c_client *i2c, const
>> struct i2c_device_id *id) IRQF_ONESHOT, dev_name(dev),
>>  						adv7511);
>>  		if (ret)
>> -			goto err_unregister_cec;
>> +			goto err_unregister_packet;
>>  	}
>>
>>  	adv7511_power_off(adv7511);
>> @@ -1203,6 +1212,8 @@ static int adv7511_probe(struct i2c_client *i2c, const
>> struct i2c_device_id *id) adv7511_audio_init(dev, adv7511);
>>  	return 0;
>>
>> +err_unregister_packet:
>> +	i2c_unregister_device(adv7511->i2c_packet);
>>  err_unregister_cec:
>>  	i2c_unregister_device(adv7511->i2c_cec);
>>  	if (adv7511->cec_clk)
>> @@ -1234,6 +1245,7 @@ static int adv7511_remove(struct i2c_client *i2c)
>>  	cec_unregister_adapter(adv7511->cec_adap);
>>
>>  	i2c_unregister_device(adv7511->i2c_edid);
>> +	i2c_unregister_device(adv7511->i2c_packet);
>>
>>  	return 0;
>>  }
> 

Re: [PATCH v3 5/5] drm: adv7511: Add support for i2c_new_secondary_device

[Kieran Bingham]

Hi Laurent,

Thanks for the review,

On 13/02/18 12:23, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Tuesday, 13 February 2018 00:07:53 EET Kieran Bingham wrote:
>> From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>
>> The ADV7511 has four 256-byte maps that can be accessed via the main I²C
>> ports. Each map has it own I²C address and acts as a standard slave
>> device on the I²C bus.
>>
>> Allow a device tree node to override the default addresses so that
>> address conflicts with other devices on the same bus may be resolved at
>> the board description level.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> ---
>> v2:
>>  - Update missing edid-i2c address setting
>>  - Split out DT bindings
>>  - Rename and move the I2C default addresses to their own section
>>
>>  drivers/gpu/drm/bridge/adv7511/adv7511.h     |  6 ++++
>>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 42 ++++++++++++++++--------
>>  2 files changed, 33 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> b/drivers/gpu/drm/bridge/adv7511/adv7511.h index d034b2cb5eee..04e6759ee45b
>> 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
>> @@ -93,6 +93,11 @@
>>  #define ADV7511_REG_CHIP_ID_HIGH		0xf5
>>  #define ADV7511_REG_CHIP_ID_LOW			0xf6
>>
>> +/* Hardware defined default addresses for i2c register maps */
> 
> s/i2c/I2C/ ? That's really because I had to find something :-)

The I²C comes from JMH's original patch, but is much harder to grep for, so
normalising to I2C throughout.


> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com
Collected, Thanks,
--
Kieran

> 
>> +#define ADV7511_CEC_I2C_ADDR_DEFAULT		0x3c
>> +#define ADV7511_EDID_I2C_ADDR_DEFAULT		0x3f
>> +#define ADV7511_PACKET_I2C_ADDR_DEFAULT		0x38
>> +
>>  #define ADV7511_CSC_ENABLE			BIT(7)
>>  #define ADV7511_CSC_UPDATE_MODE			BIT(5)
>>
>> @@ -322,6 +327,7 @@ struct adv7511 {
>>  	struct i2c_client *i2c_main;
>>  	struct i2c_client *i2c_edid;
>>  	struct i2c_client *i2c_cec;
>> +	struct i2c_client *i2c_packet;
>>
>>  	struct regmap *regmap;
>>  	struct regmap *regmap_cec;
>> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index
>> efa29db5fc2b..5e61b928c9c0 100644
>> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
>> @@ -586,7 +586,7 @@ static int adv7511_get_modes(struct adv7511 *adv7511,
>>  	/* Reading the EDID only works if the device is powered */
>>  	if (!adv7511->powered) {
>>  		unsigned int edid_i2c_addr =
>> -					(adv7511->i2c_main->addr << 1) + 4;
>> +					(adv7511->i2c_edid->addr << 1);
>>
>>  		__adv7511_power_on(adv7511);
>>
>> @@ -969,10 +969,10 @@ static int adv7511_init_cec_regmap(struct adv7511
>> *adv) {
>>  	int ret;
>>
>> -	adv->i2c_cec = i2c_new_dummy(adv->i2c_main->adapter,
>> -				     adv->i2c_main->addr - 1);
>> +	adv->i2c_cec = i2c_new_secondary_device(adv->i2c_main, "cec",
>> +					ADV7511_CEC_I2C_ADDR_DEFAULT);
>>  	if (!adv->i2c_cec)
>> -		return -ENOMEM;
>> +		return -EINVAL;
>>  	i2c_set_clientdata(adv->i2c_cec, adv);
>>
>>  	adv->regmap_cec = devm_regmap_init_i2c(adv->i2c_cec,
>> @@ -1082,8 +1082,6 @@ static int adv7511_probe(struct i2c_client *i2c, const
>> struct i2c_device_id *id) struct adv7511_link_config link_config;
>>  	struct adv7511 *adv7511;
>>  	struct device *dev = &i2c->dev;
>> -	unsigned int main_i2c_addr = i2c->addr << 1;
>> -	unsigned int edid_i2c_addr = main_i2c_addr + 4;
>>  	unsigned int val;
>>  	int ret;
>>
>> @@ -1153,24 +1151,35 @@ static int adv7511_probe(struct i2c_client *i2c,
>> const struct i2c_device_id *id) if (ret)
>>  		goto uninit_regulators;
>>
>> -	regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr);
>> -	regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
>> -		     main_i2c_addr - 0xa);
>> -	regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR,
>> -		     main_i2c_addr - 2);
>> -
>>  	adv7511_packet_disable(adv7511, 0xffff);
>>
>> -	adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1);
>> +	adv7511->i2c_edid = i2c_new_secondary_device(i2c, "edid",
>> +					ADV7511_EDID_I2C_ADDR_DEFAULT);
>>  	if (!adv7511->i2c_edid) {
>> -		ret = -ENOMEM;
>> +		ret = -EINVAL;
>>  		goto uninit_regulators;
>>  	}
>>
>> +	regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR,
>> +		     adv7511->i2c_edid->addr << 1);
>> +
>>  	ret = adv7511_init_cec_regmap(adv7511);
>>  	if (ret)
>>  		goto err_i2c_unregister_edid;
>>
>> +	regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR,
>> +		     adv7511->i2c_cec->addr << 1);
>> +
>> +	adv7511->i2c_packet = i2c_new_secondary_device(i2c, "packet",
>> +					ADV7511_PACKET_I2C_ADDR_DEFAULT);
>> +	if (!adv7511->i2c_packet) {
>> +		ret = -EINVAL;
>> +		goto err_unregister_cec;
>> +	}
>> +
>> +	regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR,
>> +		     adv7511->i2c_packet->addr << 1);
>> +
>>  	INIT_WORK(&adv7511->hpd_work, adv7511_hpd_work);
>>
>>  	if (i2c->irq) {
>> @@ -1181,7 +1190,7 @@ static int adv7511_probe(struct i2c_client *i2c, const
>> struct i2c_device_id *id) IRQF_ONESHOT, dev_name(dev),
>>  						adv7511);
>>  		if (ret)
>> -			goto err_unregister_cec;
>> +			goto err_unregister_packet;
>>  	}
>>
>>  	adv7511_power_off(adv7511);
>> @@ -1203,6 +1212,8 @@ static int adv7511_probe(struct i2c_client *i2c, const
>> struct i2c_device_id *id) adv7511_audio_init(dev, adv7511);
>>  	return 0;
>>
>> +err_unregister_packet:
>> +	i2c_unregister_device(adv7511->i2c_packet);
>>  err_unregister_cec:
>>  	i2c_unregister_device(adv7511->i2c_cec);
>>  	if (adv7511->cec_clk)
>> @@ -1234,6 +1245,7 @@ static int adv7511_remove(struct i2c_client *i2c)
>>  	cec_unregister_adapter(adv7511->cec_adap);
>>
>>  	i2c_unregister_device(adv7511->i2c_edid);
>> +	i2c_unregister_device(adv7511->i2c_packet);
>>
>>  	return 0;
>>  }
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel