[PATCH v2 0/3] Add optional regulator support for LVDS codec.

[PATCH v2 0/3] Add optional regulator support for LVDS codec.

[Biju Das]

This patch series add support for enabling optional regulator for LVDS
codec bridge driver that may be used as VCC source.

Ref:https://patchwork.kernel.org/patch/11705819/

Biju Das (3):
  dt-bindings: display: bridge: lvds-codec: Document vcc-supply property
  drm/bridge: lvds-codec: Add support for regulator
  ARM: dts: iwg20d-q7-common: Fix touch controller probe failure

 .../bindings/display/bridge/lvds-codec.yaml   |  3 ++
 arch/arm/boot/dts/iwg20d-q7-common.dtsi       | 14 +++++++++
 drivers/gpu/drm/bridge/lvds-codec.c           | 29 +++++++++++++++++++
 3 files changed, 46 insertions(+)

-- 
2.17.1

[PATCH v2 1/3] dt-bindings: display: bridge: lvds-codec: Document vcc-supply property

[Biju Das]

Document optional vcc-supply property that may be used as VCC source.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
New patch Ref: Ref:https://patchwork.kernel.org/patch/11705819/
---
 .../devicetree/bindings/display/bridge/lvds-codec.yaml         | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
index 68951d56ebba..3248be31eceb 100644
--- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
@@ -79,6 +79,9 @@ properties:
       The GPIO used to control the power down line of this device.
     maxItems: 1
 
+  vcc-supply:
+    maxItems: 1
+
 required:
   - compatible
   - ports
-- 
2.17.1

[PATCH v2 2/3] drm/bridge: lvds-codec: Add support for regulator

[Biju Das]

Add the support for enabling optional regulator that may be used as VCC
source.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
New Patch Ref: Ref:https://patchwork.kernel.org/patch/11705819/
---
 drivers/gpu/drm/bridge/lvds-codec.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/bridge/lvds-codec.c b/drivers/gpu/drm/bridge/lvds-codec.c
index 24fb1befdfa2..5858a29aafe6 100644
--- a/drivers/gpu/drm/bridge/lvds-codec.c
+++ b/drivers/gpu/drm/bridge/lvds-codec.c
@@ -10,13 +10,16 @@
 #include <linux/of_device.h>
 #include <linux/of_graph.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 
 #include <drm/drm_bridge.h>
 #include <drm/drm_panel.h>
 
 struct lvds_codec {
+	struct device *dev;
 	struct drm_bridge bridge;
 	struct drm_bridge *panel_bridge;
+	struct regulator *vcc;
 	struct gpio_desc *powerdown_gpio;
 	u32 connector_type;
 };
@@ -38,6 +41,14 @@ static int lvds_codec_attach(struct drm_bridge *bridge,
 static void lvds_codec_enable(struct drm_bridge *bridge)
 {
 	struct lvds_codec *lvds_codec = to_lvds_codec(bridge);
+	int ret;
+
+	ret = regulator_enable(lvds_codec->vcc);
+	if (ret) {
+		dev_err(lvds_codec->dev,
+			"Failed to enable regulator \"vcc\": %d\n", ret);
+		return;
+	}
 
 	if (lvds_codec->powerdown_gpio)
 		gpiod_set_value_cansleep(lvds_codec->powerdown_gpio, 0);
@@ -46,9 +57,15 @@ static void lvds_codec_enable(struct drm_bridge *bridge)
 static void lvds_codec_disable(struct drm_bridge *bridge)
 {
 	struct lvds_codec *lvds_codec = to_lvds_codec(bridge);
+	int ret;
 
 	if (lvds_codec->powerdown_gpio)
 		gpiod_set_value_cansleep(lvds_codec->powerdown_gpio, 1);
+
+	ret = regulator_disable(lvds_codec->vcc);
+	if (ret)
+		dev_err(lvds_codec->dev,
+			"Failed to disable regulator \"vcc\": %d\n", ret);
 }
 
 static const struct drm_bridge_funcs funcs = {
@@ -63,12 +80,24 @@ static int lvds_codec_probe(struct platform_device *pdev)
 	struct device_node *panel_node;
 	struct drm_panel *panel;
 	struct lvds_codec *lvds_codec;
+	int error;
 
 	lvds_codec = devm_kzalloc(dev, sizeof(*lvds_codec), GFP_KERNEL);
 	if (!lvds_codec)
 		return -ENOMEM;
 
+	lvds_codec->dev = &pdev->dev;
 	lvds_codec->connector_type = (uintptr_t)of_device_get_match_data(dev);
+
+	lvds_codec->vcc = devm_regulator_get(lvds_codec->dev, "vcc");
+	if (IS_ERR(lvds_codec->vcc)) {
+		error = PTR_ERR(lvds_codec->vcc);
+		if (error != -EPROBE_DEFER)
+			dev_err(lvds_codec->dev,
+				"Unable to get \"vcc\" supply: %d\n", error);
+		return error;
+	}
+
 	lvds_codec->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown",
 							     GPIOD_OUT_HIGH);
 	if (IS_ERR(lvds_codec->powerdown_gpio)) {
-- 
2.17.1

[PATCH v2 3/3] ARM: dts: iwg20d-q7-common: Fix touch controller probe failure

[Biju Das]

As per the iWave RZ/G1M schematic, the signal LVDS_PPEN controls supply
voltage for touch panel, LVDS receiver and RGB LCD panel. Add regulator
for these device nodes and remove powerdown-gpios property from
lvds-receiver node as it results in touch controller driver probe failure.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 v1->v2 : Add regulator in touch panel, LVDS receiver and RGB LCD panel device nodes
 v1 : https://patchwork.kernel.org/patch/11705819/
---
 arch/arm/boot/dts/iwg20d-q7-common.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/iwg20d-q7-common.dtsi b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
index 4c8b9a6b0125..056f93119d8a 100644
--- a/arch/arm/boot/dts/iwg20d-q7-common.dtsi
+++ b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
@@ -57,6 +57,7 @@
 
 	lvds-receiver {
 		compatible = "ti,ds90cf384a", "lvds-decoder";
+		vcc-supply = <&vcc_3v3_tft1>;
 
 		ports {
 			#address-cells = <1>;
@@ -80,6 +81,7 @@
 	panel {
 		compatible = "edt,etm0700g0dh6";
 		backlight = <&lcd_backlight>;
+		power-supply = <&vcc_3v3_tft1>;
 
 		port {
 			panel_in: endpoint {
@@ -112,6 +114,17 @@
 		};
 	};
 
+	vcc_3v3_tft1: regulator-panel {
+		compatible = "regulator-fixed";
+
+		regulator-name = "Panel Vcc";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		enable-active-high;
+		startup-delay-us = <70000>;
+		gpio = <&gpio7 25 GPIO_ACTIVE_HIGH>;
+	};
+
 	vcc_sdhi1: regulator-vcc-sdhi1 {
 		compatible = "regulator-fixed";
 
@@ -206,6 +219,7 @@
 		reg = <0x38>;
 		interrupt-parent = <&gpio2>;
 		interrupts = <12 IRQ_TYPE_EDGE_FALLING>;
+		vcc-supply = <&vcc_3v3_tft1>;
 	};
 };
 
-- 
2.17.1

RE: [PATCH v2 3/3] ARM: dts: iwg20d-q7-common: Fix touch controller probe failure

[Biju Das]

> Subject: [PATCH v2 3/3] ARM: dts: iwg20d-q7-common: Fix touch controller
> probe failure
>
> As per the iWave RZ/G1M schematic, the signal LVDS_PPEN controls supply
> voltage for touch panel, LVDS receiver and RGB LCD panel. Add regulator for
> these device nodes and remove powerdown-gpios property from lvds-
> receiver node as it results in touch controller driver probe failure.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  v1->v2 : Add regulator in touch panel, LVDS receiver and RGB LCD panel
> device nodes
>  v1 : https://patchwork.kernel.org/patch/11705819/
> ---
>  arch/arm/boot/dts/iwg20d-q7-common.dtsi | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm/boot/dts/iwg20d-q7-common.dtsi
> b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
> index 4c8b9a6b0125..056f93119d8a 100644
> --- a/arch/arm/boot/dts/iwg20d-q7-common.dtsi
> +++ b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
> @@ -57,6 +57,7 @@
>
>  lvds-receiver {
>  compatible = "ti,ds90cf384a", "lvds-decoder";
> +vcc-supply = <&vcc_3v3_tft1>;

Sorry, will sent V3  for  this patch, since it is  missing removal of powerdown-gpios property.

>  ports {
>  #address-cells = <1>;
> @@ -80,6 +81,7 @@
>  panel {
>  compatible = "edt,etm0700g0dh6";
>  backlight = <&lcd_backlight>;
> +power-supply = <&vcc_3v3_tft1>;
>
>  port {
>  panel_in: endpoint {
> @@ -112,6 +114,17 @@
>  };
>  };
>
> +vcc_3v3_tft1: regulator-panel {
> +compatible = "regulator-fixed";
> +
> +regulator-name = "Panel Vcc";
> +regulator-min-microvolt = <3300000>;
> +regulator-max-microvolt = <3300000>;
> +enable-active-high;
> +startup-delay-us = <70000>;
> +gpio = <&gpio7 25 GPIO_ACTIVE_HIGH>;
> +};
> +
>  vcc_sdhi1: regulator-vcc-sdhi1 {
>  compatible = "regulator-fixed";
>
> @@ -206,6 +219,7 @@
>  reg = <0x38>;
>  interrupt-parent = <&gpio2>;
>  interrupts = <12 IRQ_TYPE_EDGE_FALLING>;
> +vcc-supply = <&vcc_3v3_tft1>;
>  };
>  };
>
> --
> 2.17.1



Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647

Re: [PATCH v2 1/3] dt-bindings: display: bridge: lvds-codec: Document vcc-supply property

[Laurent Pinchart]

Hi Biju,

Thank you for the patch.

On Mon, Aug 10, 2020 at 04:22:17PM +0100, Biju Das wrote:
> Document optional vcc-supply property that may be used as VCC source.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

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

> ---
> New patch Ref: Ref:https://patchwork.kernel.org/patch/11705819/
> ---
>  .../devicetree/bindings/display/bridge/lvds-codec.yaml         | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> index 68951d56ebba..3248be31eceb 100644
> --- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> @@ -79,6 +79,9 @@ properties:
>        The GPIO used to control the power down line of this device.
>      maxItems: 1
>  
> +  vcc-supply:
> +    maxItems: 1
> +
>  required:
>    - compatible
>    - ports

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/3] drm/bridge: lvds-codec: Add support for regulator

[Laurent Pinchart]

Hi Biju,

Thank you for the patch.

On Mon, Aug 10, 2020 at 04:22:18PM +0100, Biju Das wrote:
> Add the support for enabling optional regulator that may be used as VCC
> source.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> New Patch Ref: Ref:https://patchwork.kernel.org/patch/11705819/
> ---
>  drivers/gpu/drm/bridge/lvds-codec.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/lvds-codec.c b/drivers/gpu/drm/bridge/lvds-codec.c
> index 24fb1befdfa2..5858a29aafe6 100644
> --- a/drivers/gpu/drm/bridge/lvds-codec.c
> +++ b/drivers/gpu/drm/bridge/lvds-codec.c
> @@ -10,13 +10,16 @@
>  #include <linux/of_device.h>
>  #include <linux/of_graph.h>
>  #include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
>  
>  #include <drm/drm_bridge.h>
>  #include <drm/drm_panel.h>
>  
>  struct lvds_codec {
> +	struct device *dev;
>  	struct drm_bridge bridge;
>  	struct drm_bridge *panel_bridge;
> +	struct regulator *vcc;
>  	struct gpio_desc *powerdown_gpio;
>  	u32 connector_type;
>  };
> @@ -38,6 +41,14 @@ static int lvds_codec_attach(struct drm_bridge *bridge,
>  static void lvds_codec_enable(struct drm_bridge *bridge)
>  {
>  	struct lvds_codec *lvds_codec = to_lvds_codec(bridge);
> +	int ret;
> +
> +	ret = regulator_enable(lvds_codec->vcc);
> +	if (ret) {
> +		dev_err(lvds_codec->dev,
> +			"Failed to enable regulator \"vcc\": %d\n", ret);
> +		return;
> +	}
>  
>  	if (lvds_codec->powerdown_gpio)
>  		gpiod_set_value_cansleep(lvds_codec->powerdown_gpio, 0);
> @@ -46,9 +57,15 @@ static void lvds_codec_enable(struct drm_bridge *bridge)
>  static void lvds_codec_disable(struct drm_bridge *bridge)
>  {
>  	struct lvds_codec *lvds_codec = to_lvds_codec(bridge);
> +	int ret;
>  
>  	if (lvds_codec->powerdown_gpio)
>  		gpiod_set_value_cansleep(lvds_codec->powerdown_gpio, 1);
> +
> +	ret = regulator_disable(lvds_codec->vcc);
> +	if (ret)
> +		dev_err(lvds_codec->dev,
> +			"Failed to disable regulator \"vcc\": %d\n", ret);
>  }
>  
>  static const struct drm_bridge_funcs funcs = {
> @@ -63,12 +80,24 @@ static int lvds_codec_probe(struct platform_device *pdev)
>  	struct device_node *panel_node;
>  	struct drm_panel *panel;
>  	struct lvds_codec *lvds_codec;
> +	int error;

The driver tends to use "ret" for error status variables. If you're fine
with this change there's no need to resubmit the patch, I can fix when
applying.

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

>  
>  	lvds_codec = devm_kzalloc(dev, sizeof(*lvds_codec), GFP_KERNEL);
>  	if (!lvds_codec)
>  		return -ENOMEM;
>  
> +	lvds_codec->dev = &pdev->dev;
>  	lvds_codec->connector_type = (uintptr_t)of_device_get_match_data(dev);
> +
> +	lvds_codec->vcc = devm_regulator_get(lvds_codec->dev, "vcc");
> +	if (IS_ERR(lvds_codec->vcc)) {
> +		error = PTR_ERR(lvds_codec->vcc);
> +		if (error != -EPROBE_DEFER)
> +			dev_err(lvds_codec->dev,
> +				"Unable to get \"vcc\" supply: %d\n", error);
> +		return error;
> +	}
> +
>  	lvds_codec->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown",
>  							     GPIOD_OUT_HIGH);
>  	if (IS_ERR(lvds_codec->powerdown_gpio)) {

-- 
Regards,

Laurent Pinchart

RE: [PATCH v2 2/3] drm/bridge: lvds-codec: Add support for regulator

[Biju Das]

Hi Laurent,

Thanks for the feedback.

> Subject: Re: [PATCH v2 2/3] drm/bridge: lvds-codec: Add support for
> regulator
>
> Hi Biju,
>
> Thank you for the patch.
>
> On Mon, Aug 10, 2020 at 04:22:18PM +0100, Biju Das wrote:
> > Add the support for enabling optional regulator that may be used as
> > VCC source.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > New Patch Ref: Ref:https://patchwork.kernel.org/patch/11705819/
> > ---
> >  drivers/gpu/drm/bridge/lvds-codec.c | 29
> > +++++++++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/lvds-codec.c
> > b/drivers/gpu/drm/bridge/lvds-codec.c
> > index 24fb1befdfa2..5858a29aafe6 100644
> > --- a/drivers/gpu/drm/bridge/lvds-codec.c
> > +++ b/drivers/gpu/drm/bridge/lvds-codec.c
> > @@ -10,13 +10,16 @@
> >  #include <linux/of_device.h>
> >  #include <linux/of_graph.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/regulator/consumer.h>
> >
> >  #include <drm/drm_bridge.h>
> >  #include <drm/drm_panel.h>
> >
> >  struct lvds_codec {
> > +struct device *dev;
> >  struct drm_bridge bridge;
> >  struct drm_bridge *panel_bridge;
> > +struct regulator *vcc;
> >  struct gpio_desc *powerdown_gpio;
> >  u32 connector_type;
> >  };
> > @@ -38,6 +41,14 @@ static int lvds_codec_attach(struct drm_bridge
> > *bridge,  static void lvds_codec_enable(struct drm_bridge *bridge)  {
> >  struct lvds_codec *lvds_codec = to_lvds_codec(bridge);
> > +int ret;
> > +
> > +ret = regulator_enable(lvds_codec->vcc);
> > +if (ret) {
> > +dev_err(lvds_codec->dev,
> > +"Failed to enable regulator \"vcc\": %d\n", ret);
> > +return;
> > +}
> >
> >  if (lvds_codec->powerdown_gpio)
> >  gpiod_set_value_cansleep(lvds_codec->powerdown_gpio,
> 0); @@ -46,9
> > +57,15 @@ static void lvds_codec_enable(struct drm_bridge *bridge)
> > static void lvds_codec_disable(struct drm_bridge *bridge)  {
> >  struct lvds_codec *lvds_codec = to_lvds_codec(bridge);
> > +int ret;
> >
> >  if (lvds_codec->powerdown_gpio)
> >  gpiod_set_value_cansleep(lvds_codec->powerdown_gpio,
> 1);
> > +
> > +ret = regulator_disable(lvds_codec->vcc);
> > +if (ret)
> > +dev_err(lvds_codec->dev,
> > +"Failed to disable regulator \"vcc\": %d\n", ret);
> >  }
> >
> >  static const struct drm_bridge_funcs funcs = { @@ -63,12 +80,24 @@
> > static int lvds_codec_probe(struct platform_device *pdev)
> >  struct device_node *panel_node;
> >  struct drm_panel *panel;
> >  struct lvds_codec *lvds_codec;
> > +int error;
>
> The driver tends to use "ret" for error status variables. If you're fine with this
> change there's no need to resubmit the patch, I can fix when applying.

Yes, I am ok with that.

Cheers,
Biju


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


Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647

Re: [PATCH v2 1/3] dt-bindings: display: bridge: lvds-codec: Document vcc-supply property

[Rob Herring]

On Mon, Aug 10, 2020 at 04:22:17PM +0100, Biju Das wrote:
> Document optional vcc-supply property that may be used as VCC source.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> New patch Ref: Ref:https://patchwork.kernel.org/patch/11705819/
> ---
>  .../devicetree/bindings/display/bridge/lvds-codec.yaml         | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> index 68951d56ebba..3248be31eceb 100644
> --- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> @@ -79,6 +79,9 @@ properties:
>        The GPIO used to control the power down line of this device.
>      maxItems: 1
>  
> +  vcc-supply:
> +    maxItems: 1

Probably should be 'power-supply' to align with the 'simple' panels. 
That's also to signify there's only 1 supply. Using 'vcc' would 
encourage adding 'vdd-supply', 'vddio-supply', etc. A second supply I'll 
NAK because at that point it's not a simple bridge with no configuration 
(it's arguably already there).

Rob

RE: [PATCH v2 1/3] dt-bindings: display: bridge: lvds-codec: Document vcc-supply property

[Biju Das]

Hi Rob,

Thanks for the feedback.

> Subject: Re: [PATCH v2 1/3] dt-bindings: display: bridge: lvds-codec:
> Document vcc-supply property
>
> On Mon, Aug 10, 2020 at 04:22:17PM +0100, Biju Das wrote:
> > Document optional vcc-supply property that may be used as VCC source.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > New patch Ref: Ref:https://patchwork.kernel.org/patch/11705819/
> > ---
> >  .../devicetree/bindings/display/bridge/lvds-codec.yaml         | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > index 68951d56ebba..3248be31eceb 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/lvds-
> codec.yaml
> > @@ -79,6 +79,9 @@ properties:
> >        The GPIO used to control the power down line of this device.
> >      maxItems: 1
> >
> > +  vcc-supply:
> > +    maxItems: 1
>
> Probably should be 'power-supply' to align with the 'simple' panels.
> That's also to signify there's only 1 supply. Using 'vcc' would encourage
> adding 'vdd-supply', 'vddio-supply', etc. A second supply I'll NAK because at
> that point it's not a simple bridge with no configuration (it's arguably already
> there).

Yes, I am ok with 'power-supply', since LVDS CODEC driver is  generic and also to align with terminology used in generic 'simple' panels.

In our case this Receiver converts LVDS signals to RGB signals and fed this signal to simple panel.
On the receiver part, We need to supply  power to TTL output,  PLL and LVDS input. It all derived from the single power source.

Laurent, Please share you opinion on this.

Cheers,
Biju




Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647

Re: [PATCH v2 1/3] dt-bindings: display: bridge: lvds-codec: Document vcc-supply property

[Laurent Pinchart]

Hello,

On Wed, Aug 26, 2020 at 06:58:50AM +0000, Biju Das wrote:
> > Subject: Re: [PATCH v2 1/3] dt-bindings: display: bridge: lvds-codec:
> > Document vcc-supply property
> >
> > On Mon, Aug 10, 2020 at 04:22:17PM +0100, Biju Das wrote:
> > > Document optional vcc-supply property that may be used as VCC source.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > ---
> > > New patch Ref: Ref:https://patchwork.kernel.org/patch/11705819/
> > > ---
> > >  .../devicetree/bindings/display/bridge/lvds-codec.yaml         | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > > b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > > index 68951d56ebba..3248be31eceb 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > > +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > > @@ -79,6 +79,9 @@ properties:
> > >        The GPIO used to control the power down line of this device.
> > >      maxItems: 1
> > >
> > > +  vcc-supply:
> > > +    maxItems: 1
> >
> > Probably should be 'power-supply' to align with the 'simple' panels.
> > That's also to signify there's only 1 supply. Using 'vcc' would encourage
> > adding 'vdd-supply', 'vddio-supply', etc. A second supply I'll NAK because at
> > that point it's not a simple bridge with no configuration (it's arguably already
> > there).
> 
> Yes, I am ok with 'power-supply', since LVDS CODEC driver is  generic
> and also to align with terminology used in generic 'simple' panels.
> 
> In our case this Receiver converts LVDS signals to RGB signals and fed
> this signal to simple panel.
> On the receiver part, We need to supply  power to TTL output,  PLL and
> LVDS input. It all derived from the single power source.
> 
> Laurent, Please share you opinion on this.

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

That is, I think it's a good idea to rename it, and I agree with Rob
about not adding a second supply.

I've applied the modified patch to my tree, and will send a pull request
this week.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/3] dt-bindings: display: bridge: lvds-codec: Document vcc-supply property

[Laurent Pinchart]

Hi Rob,

On Mon, Aug 24, 2020 at 05:04:58PM -0600, Rob Herring wrote:
> On Mon, Aug 10, 2020 at 04:22:17PM +0100, Biju Das wrote:
> > Document optional vcc-supply property that may be used as VCC source.
> > 
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > New patch Ref: Ref:https://patchwork.kernel.org/patch/11705819/
> > ---
> >  .../devicetree/bindings/display/bridge/lvds-codec.yaml         | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > index 68951d56ebba..3248be31eceb 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > @@ -79,6 +79,9 @@ properties:
> >        The GPIO used to control the power down line of this device.
> >      maxItems: 1
> >  
> > +  vcc-supply:
> > +    maxItems: 1
> 
> Probably should be 'power-supply' to align with the 'simple' panels. 
> That's also to signify there's only 1 supply. Using 'vcc' would 
> encourage adding 'vdd-supply', 'vddio-supply', etc. A second supply I'll 
> NAK because at that point it's not a simple bridge with no configuration 
> (it's arguably already there).

Fully agreed.

Do I get your Ab or Rb line with s/vcc/power/ and the commit message
updated to

    dt-bindings: display: bridge: lvds-codec: Document power-supply property

    Document optional power-supply property that may be used to specify the
    regulator powering up the device.

?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/3] dt-bindings: display: bridge: lvds-codec: Document vcc-supply property

[Rob Herring]

On Tue, Sep 1, 2020 at 4:27 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Rob,
>
> On Mon, Aug 24, 2020 at 05:04:58PM -0600, Rob Herring wrote:
> > On Mon, Aug 10, 2020 at 04:22:17PM +0100, Biju Das wrote:
> > > Document optional vcc-supply property that may be used as VCC source.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > ---
> > > New patch Ref: Ref:https://patchwork.kernel.org/patch/11705819/
> > > ---
> > >  .../devicetree/bindings/display/bridge/lvds-codec.yaml         | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > > index 68951d56ebba..3248be31eceb 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > > +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml
> > > @@ -79,6 +79,9 @@ properties:
> > >        The GPIO used to control the power down line of this device.
> > >      maxItems: 1
> > >
> > > +  vcc-supply:
> > > +    maxItems: 1
> >
> > Probably should be 'power-supply' to align with the 'simple' panels.
> > That's also to signify there's only 1 supply. Using 'vcc' would
> > encourage adding 'vdd-supply', 'vddio-supply', etc. A second supply I'll
> > NAK because at that point it's not a simple bridge with no configuration
> > (it's arguably already there).
>
> Fully agreed.
>
> Do I get your Ab or Rb line with s/vcc/power/ and the commit message
> updated to
>
>     dt-bindings: display: bridge: lvds-codec: Document power-supply property
>
>     Document optional power-supply property that may be used to specify the
>     regulator powering up the device.
>
> ?

Yes, if not too late.

Reviewed-by: Rob Herring <robh@kernel.org>