[PATCH v3 0/2] ARM: dts: sun6i: hummingbird-a31: Enable display output through VGA bridge

[PATCH v3 0/2] ARM: dts: sun6i: hummingbird-a31: Enable display output through VGA bridge

[Chen-Yu Tsai]

Hi,

This is v3 of my sun6i DRM/KMS display enablement series. Most of the
patches in v2 were merged, leaving only the dumb-vga-dac patch and the
hummingbird-a31 VGA output patch.

A few things were changed, mainly the DAC enable method from a GPIO
to a regulator supply.

Changes since v2:

  - Changed the enable-gpio of dumb-vga-dac to vdd-supply regulator.
    This better matches the hardware that I have: the DAC has a fixed
    regulator dropping the voltage from the board-wide 5V to 3.3V the
    DAC uses. The regulator is controlled through a GPIO pin.

  - Renamed the node of the VGA connector from "vga" to "vga-connector".

  - Renamed the node of the VGA DAC from "bridge" to "vga-dac".


Regards
ChenYu


Chen-Yu Tsai (2):
  drm/bridge: dumb-vga-dac: Support a VDD regulator supply
  ARM: dts: sun6i: hummingbird-a31: Enable display output through VGA
    bridge

 .../bindings/display/bridge/dumb-vga-dac.txt       |  2 +
 arch/arm/boot/dts/sun6i-a31-hummingbird.dts        | 76 ++++++++++++++++++++++
 drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++
 3 files changed, 113 insertions(+)

-- 
2.9.3

[PATCH v3 0/2] ARM: dts: sun6i: hummingbird-a31: Enable display output through VGA bridge

[Chen-Yu Tsai]

Hi,

This is v3 of my sun6i DRM/KMS display enablement series. Most of the
patches in v2 were merged, leaving only the dumb-vga-dac patch and the
hummingbird-a31 VGA output patch.

A few things were changed, mainly the DAC enable method from a GPIO
to a regulator supply.

Changes since v2:

  - Changed the enable-gpio of dumb-vga-dac to vdd-supply regulator.
    This better matches the hardware that I have: the DAC has a fixed
    regulator dropping the voltage from the board-wide 5V to 3.3V the
    DAC uses. The regulator is controlled through a GPIO pin.

  - Renamed the node of the VGA connector from "vga" to "vga-connector".

  - Renamed the node of the VGA DAC from "bridge" to "vga-dac".


Regards
ChenYu


Chen-Yu Tsai (2):
  drm/bridge: dumb-vga-dac: Support a VDD regulator supply
  ARM: dts: sun6i: hummingbird-a31: Enable display output through VGA
    bridge

 .../bindings/display/bridge/dumb-vga-dac.txt       |  2 +
 arch/arm/boot/dts/sun6i-a31-hummingbird.dts        | 76 ++++++++++++++++++++++
 drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++
 3 files changed, 113 insertions(+)

-- 
2.9.3

[PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply

[Chen-Yu Tsai]

Some dumb VGA DACs are active components which require external power.
Add support for specifying a regulator as its power supply.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 .../bindings/display/bridge/dumb-vga-dac.txt       |  2 ++
 drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
index 003bc246a270..164cbb15f04c 100644
--- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
+++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
@@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
 - Video port 0 for RGB input
 - Video port 1 for VGA output
 
+Optional properties:
+- vdd-supply: Power supply for DAC
 
 Example
 -------
diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index afec232185a7..59781e031220 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -12,6 +12,7 @@
 
 #include <linux/module.h>
 #include <linux/of_graph.h>
+#include <linux/regulator/consumer.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_atomic_helper.h>
@@ -23,6 +24,7 @@ struct dumb_vga {
 	struct drm_connector	connector;
 
 	struct i2c_adapter	*ddc;
+	struct regulator	*vdd;
 };
 
 static inline struct dumb_vga *
@@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
 	return 0;
 }
 
+static void dumb_vga_enable(struct drm_bridge *bridge)
+{
+	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
+	int ret;
+
+	if (!IS_ERR(vga->vdd)) {
+		ret = regulator_enable(vga->vdd);
+
+		if (ret) {
+			DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
+			return;
+		}
+	}
+}
+
+static void dumb_vga_disable(struct drm_bridge *bridge)
+{
+	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
+
+	if (!IS_ERR(vga->vdd))
+		regulator_disable(vga->vdd);
+}
+
 static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
 	.attach		= dumb_vga_attach,
+	.enable		= dumb_vga_enable,
+	.disable	= dumb_vga_disable,
 };
 
 static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
@@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	platform_set_drvdata(pdev, vga);
 
+	vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
+	if (IS_ERR(vga->vdd)) {
+		ret = PTR_ERR(vga->vdd);
+		if (ret == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
+	}
+
 	vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
 	if (IS_ERR(vga->ddc)) {
 		if (PTR_ERR(vga->ddc) == -ENODEV) {
-- 
2.9.3

[PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply

[Chen-Yu Tsai]

Some dumb VGA DACs are active components which require external power.
Add support for specifying a regulator as its power supply.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 .../bindings/display/bridge/dumb-vga-dac.txt       |  2 ++
 drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
index 003bc246a270..164cbb15f04c 100644
--- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
+++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
@@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
 - Video port 0 for RGB input
 - Video port 1 for VGA output
 
+Optional properties:
+- vdd-supply: Power supply for DAC
 
 Example
 -------
diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
index afec232185a7..59781e031220 100644
--- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
+++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
@@ -12,6 +12,7 @@
 
 #include <linux/module.h>
 #include <linux/of_graph.h>
+#include <linux/regulator/consumer.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_atomic_helper.h>
@@ -23,6 +24,7 @@ struct dumb_vga {
 	struct drm_connector	connector;
 
 	struct i2c_adapter	*ddc;
+	struct regulator	*vdd;
 };
 
 static inline struct dumb_vga *
@@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
 	return 0;
 }
 
+static void dumb_vga_enable(struct drm_bridge *bridge)
+{
+	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
+	int ret;
+
+	if (!IS_ERR(vga->vdd)) {
+		ret = regulator_enable(vga->vdd);
+
+		if (ret) {
+			DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
+			return;
+		}
+	}
+}
+
+static void dumb_vga_disable(struct drm_bridge *bridge)
+{
+	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
+
+	if (!IS_ERR(vga->vdd))
+		regulator_disable(vga->vdd);
+}
+
 static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
 	.attach		= dumb_vga_attach,
+	.enable		= dumb_vga_enable,
+	.disable	= dumb_vga_disable,
 };
 
 static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
@@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	platform_set_drvdata(pdev, vga);
 
+	vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
+	if (IS_ERR(vga->vdd)) {
+		ret = PTR_ERR(vga->vdd);
+		if (ret == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+		dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
+	}
+
 	vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
 	if (IS_ERR(vga->ddc)) {
 		if (PTR_ERR(vga->ddc) == -ENODEV) {
-- 
2.9.3

[PATCH v3 2/2] ARM: dts: sun6i: hummingbird-a31: Enable display output through VGA bridge

[Chen-Yu Tsai]

The Hummingbird A31 board has a VGA DAC which converts RGB output
from the LCD interface to VGA analog signals.

Add nodes for the VGA DAC, its power supply, and enable this part
of the display pipeline.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 76 +++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
index 9a74637f677f..1ab28b2108fe 100644
--- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
+++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
@@ -63,6 +63,62 @@
 		stdout-path = "serial0:115200n8";
 	};
 
+	vga-connector {
+		compatible = "vga-connector";
+
+		port {
+			vga_con_in: endpoint {
+				remote-endpoint = <&vga_dac_out>;
+			};
+		};
+	};
+
+	vga-dac {
+		compatible = "dumb-vga-dac";
+		vdd-supply = <&reg_vga_3v3>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0>;
+
+				vga_dac_in: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&tcon0_out_vga>;
+				};
+			};
+
+			port@1 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <1>;
+
+				vga_dac_out: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&vga_con_in>;
+				};
+			};
+		};
+	};
+
+	reg_vga_3v3: vga_3v3_regulator {
+		compatible = "regulator-fixed";
+		pinctrl-names = "default";
+		pinctrl-0 = <&vga_3v3_enable_pin_hummingbird>;
+		regulator-name = "vga-3v3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		enable-active-high;
+		gpio = <&pio 7 25 GPIO_ACTIVE_HIGH>; /* PH25 */
+	};
+
 	wifi_pwrseq: wifi_pwrseq {
 		compatible = "mmc-pwrseq-simple";
 		reset-gpios = <&pio 6 10 GPIO_ACTIVE_LOW>; /* PG10 */
@@ -166,6 +222,13 @@
 		allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
 	};
 
+	vga_3v3_enable_pin_hummingbird: vga_3v3_enable_pin {
+		allwinner,pins = "PH25";
+		allwinner,function = "gpio_out";
+		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+		allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+	};
+
 	wifi_reset_pin_hummingbird: wifi_reset_pin@0 {
 		allwinner,pins = "PG10";
 		allwinner,function = "gpio_out";
@@ -245,6 +308,19 @@
 	status = "okay";
 };
 
+&tcon0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&lcd0_rgb888_pins>;
+	status = "okay";
+};
+
+&tcon0_out {
+	tcon0_out_vga: endpoint@0 {
+		reg = <0>;
+		remote-endpoint = <&vga_dac_in>;
+	};
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pins_a>;
-- 
2.9.3

[PATCH v3 2/2] ARM: dts: sun6i: hummingbird-a31: Enable display output through VGA bridge

[Chen-Yu Tsai]

The Hummingbird A31 board has a VGA DAC which converts RGB output
from the LCD interface to VGA analog signals.

Add nodes for the VGA DAC, its power supply, and enable this part
of the display pipeline.

Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
 arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 76 +++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
index 9a74637f677f..1ab28b2108fe 100644
--- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
+++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
@@ -63,6 +63,62 @@
 		stdout-path = "serial0:115200n8";
 	};
 
+	vga-connector {
+		compatible = "vga-connector";
+
+		port {
+			vga_con_in: endpoint {
+				remote-endpoint = <&vga_dac_out>;
+			};
+		};
+	};
+
+	vga-dac {
+		compatible = "dumb-vga-dac";
+		vdd-supply = <&reg_vga_3v3>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0>;
+
+				vga_dac_in: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&tcon0_out_vga>;
+				};
+			};
+
+			port@1 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <1>;
+
+				vga_dac_out: endpoint@0 {
+					reg = <0>;
+					remote-endpoint = <&vga_con_in>;
+				};
+			};
+		};
+	};
+
+	reg_vga_3v3: vga_3v3_regulator {
+		compatible = "regulator-fixed";
+		pinctrl-names = "default";
+		pinctrl-0 = <&vga_3v3_enable_pin_hummingbird>;
+		regulator-name = "vga-3v3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		enable-active-high;
+		gpio = <&pio 7 25 GPIO_ACTIVE_HIGH>; /* PH25 */
+	};
+
 	wifi_pwrseq: wifi_pwrseq {
 		compatible = "mmc-pwrseq-simple";
 		reset-gpios = <&pio 6 10 GPIO_ACTIVE_LOW>; /* PG10 */
@@ -166,6 +222,13 @@
 		allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
 	};
 
+	vga_3v3_enable_pin_hummingbird: vga_3v3_enable_pin {
+		allwinner,pins = "PH25";
+		allwinner,function = "gpio_out";
+		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+		allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+	};
+
 	wifi_reset_pin_hummingbird: wifi_reset_pin@0 {
 		allwinner,pins = "PG10";
 		allwinner,function = "gpio_out";
@@ -245,6 +308,19 @@
 	status = "okay";
 };
 
+&tcon0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&lcd0_rgb888_pins>;
+	status = "okay";
+};
+
+&tcon0_out {
+	tcon0_out_vga: endpoint@0 {
+		reg = <0>;
+		remote-endpoint = <&vga_dac_in>;
+	};
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pins_a>;
-- 
2.9.3

[PATCH v3 2/2] ARM: dts: sun6i: hummingbird-a31: Enable display output through VGA bridge

[Chen-Yu Tsai]

The Hummingbird A31 board has a VGA DAC which converts RGB output
from the LCD interface to VGA analog signals.

Add nodes for the VGA DAC, its power supply, and enable this part
of the display pipeline.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun6i-a31-hummingbird.dts | 76 +++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
index 9a74637f677f..1ab28b2108fe 100644
--- a/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
+++ b/arch/arm/boot/dts/sun6i-a31-hummingbird.dts
@@ -63,6 +63,62 @@
 		stdout-path = "serial0:115200n8";
 	};
 
+	vga-connector {
+		compatible = "vga-connector";
+
+		port {
+			vga_con_in: endpoint {
+				remote-endpoint = <&vga_dac_out>;
+			};
+		};
+	};
+
+	vga-dac {
+		compatible = "dumb-vga-dac";
+		vdd-supply = <&reg_vga_3v3>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port at 0 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0>;
+
+				vga_dac_in: endpoint at 0 {
+					reg = <0>;
+					remote-endpoint = <&tcon0_out_vga>;
+				};
+			};
+
+			port at 1 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <1>;
+
+				vga_dac_out: endpoint at 0 {
+					reg = <0>;
+					remote-endpoint = <&vga_con_in>;
+				};
+			};
+		};
+	};
+
+	reg_vga_3v3: vga_3v3_regulator {
+		compatible = "regulator-fixed";
+		pinctrl-names = "default";
+		pinctrl-0 = <&vga_3v3_enable_pin_hummingbird>;
+		regulator-name = "vga-3v3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		enable-active-high;
+		gpio = <&pio 7 25 GPIO_ACTIVE_HIGH>; /* PH25 */
+	};
+
 	wifi_pwrseq: wifi_pwrseq {
 		compatible = "mmc-pwrseq-simple";
 		reset-gpios = <&pio 6 10 GPIO_ACTIVE_LOW>; /* PG10 */
@@ -166,6 +222,13 @@
 		allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
 	};
 
+	vga_3v3_enable_pin_hummingbird: vga_3v3_enable_pin {
+		allwinner,pins = "PH25";
+		allwinner,function = "gpio_out";
+		allwinner,drive = <SUN4I_PINCTRL_10_MA>;
+		allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
+	};
+
 	wifi_reset_pin_hummingbird: wifi_reset_pin at 0 {
 		allwinner,pins = "PG10";
 		allwinner,function = "gpio_out";
@@ -245,6 +308,19 @@
 	status = "okay";
 };
 
+&tcon0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&lcd0_rgb888_pins>;
+	status = "okay";
+};
+
+&tcon0_out {
+	tcon0_out_vga: endpoint at 0 {
+		reg = <0>;
+		remote-endpoint = <&vga_dac_in>;
+	};
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pins_a>;
-- 
2.9.3

Re: [PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply

[Rob Herring]

On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
> Some dumb VGA DACs are active components which require external power.
> Add support for specifying a regulator as its power supply.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  .../bindings/display/bridge/dumb-vga-dac.txt       |  2 ++

For the binding,

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

One code comment below...

>  drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++++++++++++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> index 003bc246a270..164cbb15f04c 100644
> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>  - Video port 0 for RGB input
>  - Video port 1 for VGA output
>  
> +Optional properties:
> +- vdd-supply: Power supply for DAC
>  
>  Example
>  -------
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index afec232185a7..59781e031220 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -12,6 +12,7 @@
>  
>  #include <linux/module.h>
>  #include <linux/of_graph.h>
> +#include <linux/regulator/consumer.h>
>  
>  #include <drm/drmP.h>
>  #include <drm/drm_atomic_helper.h>
> @@ -23,6 +24,7 @@ struct dumb_vga {
>  	struct drm_connector	connector;
>  
>  	struct i2c_adapter	*ddc;
> +	struct regulator	*vdd;
>  };
>  
>  static inline struct dumb_vga *
> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>  	return 0;
>  }
>  
> +static void dumb_vga_enable(struct drm_bridge *bridge)
> +{
> +	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +	int ret;
> +
> +	if (!IS_ERR(vga->vdd)) {

if (IS_ERR())
	return;

...will save some intentation.

> +		ret = regulator_enable(vga->vdd);
> +
> +		if (ret) {
> +			DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
> +			return;
> +		}
> +	}
> +}
> +
> +static void dumb_vga_disable(struct drm_bridge *bridge)
> +{
> +	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +
> +	if (!IS_ERR(vga->vdd))
> +		regulator_disable(vga->vdd);
> +}
> +
>  static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>  	.attach		= dumb_vga_attach,
> +	.enable		= dumb_vga_enable,
> +	.disable	= dumb_vga_disable,
>  };
>  
>  static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	platform_set_drvdata(pdev, vga);
>  
> +	vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
> +	if (IS_ERR(vga->vdd)) {
> +		ret = PTR_ERR(vga->vdd);
> +		if (ret == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +		dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
> +	}
> +
>  	vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>  	if (IS_ERR(vga->ddc)) {
>  		if (PTR_ERR(vga->ddc) == -ENODEV) {
> -- 
> 2.9.3
> 

Re: [PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply

[Rob Herring]

On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
> Some dumb VGA DACs are active components which require external power.
> Add support for specifying a regulator as its power supply.
> 
> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
> ---
>  .../bindings/display/bridge/dumb-vga-dac.txt       |  2 ++

For the binding,

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

One code comment below...

>  drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++++++++++++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> index 003bc246a270..164cbb15f04c 100644
> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>  - Video port 0 for RGB input
>  - Video port 1 for VGA output
>  
> +Optional properties:
> +- vdd-supply: Power supply for DAC
>  
>  Example
>  -------
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index afec232185a7..59781e031220 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -12,6 +12,7 @@
>  
>  #include <linux/module.h>
>  #include <linux/of_graph.h>
> +#include <linux/regulator/consumer.h>
>  
>  #include <drm/drmP.h>
>  #include <drm/drm_atomic_helper.h>
> @@ -23,6 +24,7 @@ struct dumb_vga {
>  	struct drm_connector	connector;
>  
>  	struct i2c_adapter	*ddc;
> +	struct regulator	*vdd;
>  };
>  
>  static inline struct dumb_vga *
> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>  	return 0;
>  }
>  
> +static void dumb_vga_enable(struct drm_bridge *bridge)
> +{
> +	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +	int ret;
> +
> +	if (!IS_ERR(vga->vdd)) {

if (IS_ERR())
	return;

...will save some intentation.

> +		ret = regulator_enable(vga->vdd);
> +
> +		if (ret) {
> +			DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
> +			return;
> +		}
> +	}
> +}
> +
> +static void dumb_vga_disable(struct drm_bridge *bridge)
> +{
> +	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +
> +	if (!IS_ERR(vga->vdd))
> +		regulator_disable(vga->vdd);
> +}
> +
>  static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>  	.attach		= dumb_vga_attach,
> +	.enable		= dumb_vga_enable,
> +	.disable	= dumb_vga_disable,
>  };
>  
>  static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	platform_set_drvdata(pdev, vga);
>  
> +	vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
> +	if (IS_ERR(vga->vdd)) {
> +		ret = PTR_ERR(vga->vdd);
> +		if (ret == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +		dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
> +	}
> +
>  	vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>  	if (IS_ERR(vga->ddc)) {
>  		if (PTR_ERR(vga->ddc) == -ENODEV) {
> -- 
> 2.9.3
> 

[PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply

[Rob Herring]

On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
> Some dumb VGA DACs are active components which require external power.
> Add support for specifying a regulator as its power supply.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  .../bindings/display/bridge/dumb-vga-dac.txt       |  2 ++

For the binding,

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

One code comment below...

>  drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++++++++++++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> index 003bc246a270..164cbb15f04c 100644
> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>  - Video port 0 for RGB input
>  - Video port 1 for VGA output
>  
> +Optional properties:
> +- vdd-supply: Power supply for DAC
>  
>  Example
>  -------
> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> index afec232185a7..59781e031220 100644
> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
> @@ -12,6 +12,7 @@
>  
>  #include <linux/module.h>
>  #include <linux/of_graph.h>
> +#include <linux/regulator/consumer.h>
>  
>  #include <drm/drmP.h>
>  #include <drm/drm_atomic_helper.h>
> @@ -23,6 +24,7 @@ struct dumb_vga {
>  	struct drm_connector	connector;
>  
>  	struct i2c_adapter	*ddc;
> +	struct regulator	*vdd;
>  };
>  
>  static inline struct dumb_vga *
> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>  	return 0;
>  }
>  
> +static void dumb_vga_enable(struct drm_bridge *bridge)
> +{
> +	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +	int ret;
> +
> +	if (!IS_ERR(vga->vdd)) {

if (IS_ERR())
	return;

...will save some intentation.

> +		ret = regulator_enable(vga->vdd);
> +
> +		if (ret) {
> +			DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
> +			return;
> +		}
> +	}
> +}
> +
> +static void dumb_vga_disable(struct drm_bridge *bridge)
> +{
> +	struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
> +
> +	if (!IS_ERR(vga->vdd))
> +		regulator_disable(vga->vdd);
> +}
> +
>  static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>  	.attach		= dumb_vga_attach,
> +	.enable		= dumb_vga_enable,
> +	.disable	= dumb_vga_disable,
>  };
>  
>  static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	platform_set_drvdata(pdev, vga);
>  
> +	vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
> +	if (IS_ERR(vga->vdd)) {
> +		ret = PTR_ERR(vga->vdd);
> +		if (ret == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +		dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
> +	}
> +
>  	vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>  	if (IS_ERR(vga->ddc)) {
>  		if (PTR_ERR(vga->ddc) == -ENODEV) {
> -- 
> 2.9.3
> 

Re: [linux-sunxi] Re: [PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply

[Chen-Yu Tsai]

On Mon, Oct 31, 2016 at 2:28 PM, Rob Herring <robh@kernel.org> wrote:
> On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
>> Some dumb VGA DACs are active components which require external power.
>> Add support for specifying a regulator as its power supply.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>>  .../bindings/display/bridge/dumb-vga-dac.txt       |  2 ++
>
> For the binding,
>
> Acked-by: Rob Herring <robh@kernel.org>
>
> One code comment below...
>
>>  drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++++++++++++++
>>  2 files changed, 37 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> index 003bc246a270..164cbb15f04c 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>>  - Video port 0 for RGB input
>>  - Video port 1 for VGA output
>>
>> +Optional properties:
>> +- vdd-supply: Power supply for DAC
>>
>>  Example
>>  -------
>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> index afec232185a7..59781e031220 100644
>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> @@ -12,6 +12,7 @@
>>
>>  #include <linux/module.h>
>>  #include <linux/of_graph.h>
>> +#include <linux/regulator/consumer.h>
>>
>>  #include <drm/drmP.h>
>>  #include <drm/drm_atomic_helper.h>
>> @@ -23,6 +24,7 @@ struct dumb_vga {
>>       struct drm_connector    connector;
>>
>>       struct i2c_adapter      *ddc;
>> +     struct regulator        *vdd;
>>  };
>>
>>  static inline struct dumb_vga *
>> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>>       return 0;
>>  }
>>
>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>> +{
>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>> +     int ret;
>> +
>> +     if (!IS_ERR(vga->vdd)) {
>
> if (IS_ERR())
>         return;
>
> ...will save some intentation.

I thought about that, though if someone were to add more stuff to
this, such as an enable GPIO, they might have to rework it. A standalone
block of code would be easier to work with. I'm OK either way though.

ChenYu

>
>> +             ret = regulator_enable(vga->vdd);
>> +
>> +             if (ret) {
>> +                     DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
>> +                     return;
>> +             }
>> +     }
>> +}
>> +
>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>> +{
>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>> +
>> +     if (!IS_ERR(vga->vdd))
>> +             regulator_disable(vga->vdd);
>> +}
>> +
>>  static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>>       .attach         = dumb_vga_attach,
>> +     .enable         = dumb_vga_enable,
>> +     .disable        = dumb_vga_disable,
>>  };
>>
>>  static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>>               return -ENOMEM;
>>       platform_set_drvdata(pdev, vga);
>>
>> +     vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>> +     if (IS_ERR(vga->vdd)) {
>> +             ret = PTR_ERR(vga->vdd);
>> +             if (ret == -EPROBE_DEFER)
>> +                     return -EPROBE_DEFER;
>> +             dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
>> +     }
>> +
>>       vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>>       if (IS_ERR(vga->ddc)) {
>>               if (PTR_ERR(vga->ddc) == -ENODEV) {
>> --
>> 2.9.3
>>
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Re: Re: [PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply

[Chen-Yu Tsai]

On Mon, Oct 31, 2016 at 2:28 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
>> Some dumb VGA DACs are active components which require external power.
>> Add support for specifying a regulator as its power supply.
>>
>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>> ---
>>  .../bindings/display/bridge/dumb-vga-dac.txt       |  2 ++
>
> For the binding,
>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> One code comment below...
>
>>  drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++++++++++++++
>>  2 files changed, 37 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> index 003bc246a270..164cbb15f04c 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>>  - Video port 0 for RGB input
>>  - Video port 1 for VGA output
>>
>> +Optional properties:
>> +- vdd-supply: Power supply for DAC
>>
>>  Example
>>  -------
>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> index afec232185a7..59781e031220 100644
>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> @@ -12,6 +12,7 @@
>>
>>  #include <linux/module.h>
>>  #include <linux/of_graph.h>
>> +#include <linux/regulator/consumer.h>
>>
>>  #include <drm/drmP.h>
>>  #include <drm/drm_atomic_helper.h>
>> @@ -23,6 +24,7 @@ struct dumb_vga {
>>       struct drm_connector    connector;
>>
>>       struct i2c_adapter      *ddc;
>> +     struct regulator        *vdd;
>>  };
>>
>>  static inline struct dumb_vga *
>> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>>       return 0;
>>  }
>>
>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>> +{
>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>> +     int ret;
>> +
>> +     if (!IS_ERR(vga->vdd)) {
>
> if (IS_ERR())
>         return;
>
> ...will save some intentation.

I thought about that, though if someone were to add more stuff to
this, such as an enable GPIO, they might have to rework it. A standalone
block of code would be easier to work with. I'm OK either way though.

ChenYu

>
>> +             ret = regulator_enable(vga->vdd);
>> +
>> +             if (ret) {
>> +                     DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
>> +                     return;
>> +             }
>> +     }
>> +}
>> +
>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>> +{
>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>> +
>> +     if (!IS_ERR(vga->vdd))
>> +             regulator_disable(vga->vdd);
>> +}
>> +
>>  static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>>       .attach         = dumb_vga_attach,
>> +     .enable         = dumb_vga_enable,
>> +     .disable        = dumb_vga_disable,
>>  };
>>
>>  static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>>               return -ENOMEM;
>>       platform_set_drvdata(pdev, vga);
>>
>> +     vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>> +     if (IS_ERR(vga->vdd)) {
>> +             ret = PTR_ERR(vga->vdd);
>> +             if (ret == -EPROBE_DEFER)
>> +                     return -EPROBE_DEFER;
>> +             dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
>> +     }
>> +
>>       vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>>       if (IS_ERR(vga->ddc)) {
>>               if (PTR_ERR(vga->ddc) == -ENODEV) {
>> --
>> 2.9.3
>>
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
> For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply

[Chen-Yu Tsai]

On Mon, Oct 31, 2016 at 2:28 PM, Rob Herring <robh@kernel.org> wrote:
> On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
>> Some dumb VGA DACs are active components which require external power.
>> Add support for specifying a regulator as its power supply.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>>  .../bindings/display/bridge/dumb-vga-dac.txt       |  2 ++
>
> For the binding,
>
> Acked-by: Rob Herring <robh@kernel.org>
>
> One code comment below...
>
>>  drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++++++++++++++
>>  2 files changed, 37 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> index 003bc246a270..164cbb15f04c 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>>  - Video port 0 for RGB input
>>  - Video port 1 for VGA output
>>
>> +Optional properties:
>> +- vdd-supply: Power supply for DAC
>>
>>  Example
>>  -------
>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> index afec232185a7..59781e031220 100644
>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>> @@ -12,6 +12,7 @@
>>
>>  #include <linux/module.h>
>>  #include <linux/of_graph.h>
>> +#include <linux/regulator/consumer.h>
>>
>>  #include <drm/drmP.h>
>>  #include <drm/drm_atomic_helper.h>
>> @@ -23,6 +24,7 @@ struct dumb_vga {
>>       struct drm_connector    connector;
>>
>>       struct i2c_adapter      *ddc;
>> +     struct regulator        *vdd;
>>  };
>>
>>  static inline struct dumb_vga *
>> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>>       return 0;
>>  }
>>
>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>> +{
>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>> +     int ret;
>> +
>> +     if (!IS_ERR(vga->vdd)) {
>
> if (IS_ERR())
>         return;
>
> ...will save some intentation.

I thought about that, though if someone were to add more stuff to
this, such as an enable GPIO, they might have to rework it. A standalone
block of code would be easier to work with. I'm OK either way though.

ChenYu

>
>> +             ret = regulator_enable(vga->vdd);
>> +
>> +             if (ret) {
>> +                     DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
>> +                     return;
>> +             }
>> +     }
>> +}
>> +
>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>> +{
>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>> +
>> +     if (!IS_ERR(vga->vdd))
>> +             regulator_disable(vga->vdd);
>> +}
>> +
>>  static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>>       .attach         = dumb_vga_attach,
>> +     .enable         = dumb_vga_enable,
>> +     .disable        = dumb_vga_disable,
>>  };
>>
>>  static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>>               return -ENOMEM;
>>       platform_set_drvdata(pdev, vga);
>>
>> +     vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>> +     if (IS_ERR(vga->vdd)) {
>> +             ret = PTR_ERR(vga->vdd);
>> +             if (ret == -EPROBE_DEFER)
>> +                     return -EPROBE_DEFER;
>> +             dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
>> +     }
>> +
>>       vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>>       if (IS_ERR(vga->ddc)) {
>>               if (PTR_ERR(vga->ddc) == -ENODEV) {
>> --
>> 2.9.3
>>
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply

[Chen-Yu Tsai]

Hi,

On Wed, Nov 2, 2016 at 9:33 AM, Chen-Yu Tsai <wens@csie.org> wrote:
> On Mon, Oct 31, 2016 at 2:28 PM, Rob Herring <robh@kernel.org> wrote:
>> On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
>>> Some dumb VGA DACs are active components which require external power.
>>> Add support for specifying a regulator as its power supply.
>>>
>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>> ---
>>>  .../bindings/display/bridge/dumb-vga-dac.txt       |  2 ++
>>
>> For the binding,
>>
>> Acked-by: Rob Herring <robh@kernel.org>

Any comments on this patch from the DRM people?

ChenYu

>>
>> One code comment below...
>>
>>>  drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++++++++++++++
>>>  2 files changed, 37 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> index 003bc246a270..164cbb15f04c 100644
>>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>>>  - Video port 0 for RGB input
>>>  - Video port 1 for VGA output
>>>
>>> +Optional properties:
>>> +- vdd-supply: Power supply for DAC
>>>
>>>  Example
>>>  -------
>>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> index afec232185a7..59781e031220 100644
>>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> @@ -12,6 +12,7 @@
>>>
>>>  #include <linux/module.h>
>>>  #include <linux/of_graph.h>
>>> +#include <linux/regulator/consumer.h>
>>>
>>>  #include <drm/drmP.h>
>>>  #include <drm/drm_atomic_helper.h>
>>> @@ -23,6 +24,7 @@ struct dumb_vga {
>>>       struct drm_connector    connector;
>>>
>>>       struct i2c_adapter      *ddc;
>>> +     struct regulator        *vdd;
>>>  };
>>>
>>>  static inline struct dumb_vga *
>>> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>>>       return 0;
>>>  }
>>>
>>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>>> +{
>>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>> +     int ret;
>>> +
>>> +     if (!IS_ERR(vga->vdd)) {
>>
>> if (IS_ERR())
>>         return;
>>
>> ...will save some intentation.
>
> I thought about that, though if someone were to add more stuff to
> this, such as an enable GPIO, they might have to rework it. A standalone
> block of code would be easier to work with. I'm OK either way though.
>
> ChenYu
>
>>
>>> +             ret = regulator_enable(vga->vdd);
>>> +
>>> +             if (ret) {
>>> +                     DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
>>> +                     return;
>>> +             }
>>> +     }
>>> +}
>>> +
>>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>>> +{
>>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>> +
>>> +     if (!IS_ERR(vga->vdd))
>>> +             regulator_disable(vga->vdd);
>>> +}
>>> +
>>>  static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>>>       .attach         = dumb_vga_attach,
>>> +     .enable         = dumb_vga_enable,
>>> +     .disable        = dumb_vga_disable,
>>>  };
>>>
>>>  static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>>> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>>>               return -ENOMEM;
>>>       platform_set_drvdata(pdev, vga);
>>>
>>> +     vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>>> +     if (IS_ERR(vga->vdd)) {
>>> +             ret = PTR_ERR(vga->vdd);
>>> +             if (ret == -EPROBE_DEFER)
>>> +                     return -EPROBE_DEFER;
>>> +             dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
>>> +     }
>>> +
>>>       vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>>>       if (IS_ERR(vga->ddc)) {
>>>               if (PTR_ERR(vga->ddc) == -ENODEV) {
>>> --
>>> 2.9.3
>>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.

Re: Re: [PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply

[Chen-Yu Tsai]

Hi,

On Wed, Nov 2, 2016 at 9:33 AM, Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> wrote:
> On Mon, Oct 31, 2016 at 2:28 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
>>> Some dumb VGA DACs are active components which require external power.
>>> Add support for specifying a regulator as its power supply.
>>>
>>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>>> ---
>>>  .../bindings/display/bridge/dumb-vga-dac.txt       |  2 ++
>>
>> For the binding,
>>
>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Any comments on this patch from the DRM people?

ChenYu

>>
>> One code comment below...
>>
>>>  drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++++++++++++++
>>>  2 files changed, 37 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> index 003bc246a270..164cbb15f04c 100644
>>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>>>  - Video port 0 for RGB input
>>>  - Video port 1 for VGA output
>>>
>>> +Optional properties:
>>> +- vdd-supply: Power supply for DAC
>>>
>>>  Example
>>>  -------
>>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> index afec232185a7..59781e031220 100644
>>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> @@ -12,6 +12,7 @@
>>>
>>>  #include <linux/module.h>
>>>  #include <linux/of_graph.h>
>>> +#include <linux/regulator/consumer.h>
>>>
>>>  #include <drm/drmP.h>
>>>  #include <drm/drm_atomic_helper.h>
>>> @@ -23,6 +24,7 @@ struct dumb_vga {
>>>       struct drm_connector    connector;
>>>
>>>       struct i2c_adapter      *ddc;
>>> +     struct regulator        *vdd;
>>>  };
>>>
>>>  static inline struct dumb_vga *
>>> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>>>       return 0;
>>>  }
>>>
>>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>>> +{
>>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>> +     int ret;
>>> +
>>> +     if (!IS_ERR(vga->vdd)) {
>>
>> if (IS_ERR())
>>         return;
>>
>> ...will save some intentation.
>
> I thought about that, though if someone were to add more stuff to
> this, such as an enable GPIO, they might have to rework it. A standalone
> block of code would be easier to work with. I'm OK either way though.
>
> ChenYu
>
>>
>>> +             ret = regulator_enable(vga->vdd);
>>> +
>>> +             if (ret) {
>>> +                     DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
>>> +                     return;
>>> +             }
>>> +     }
>>> +}
>>> +
>>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>>> +{
>>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>> +
>>> +     if (!IS_ERR(vga->vdd))
>>> +             regulator_disable(vga->vdd);
>>> +}
>>> +
>>>  static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>>>       .attach         = dumb_vga_attach,
>>> +     .enable         = dumb_vga_enable,
>>> +     .disable        = dumb_vga_disable,
>>>  };
>>>
>>>  static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>>> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>>>               return -ENOMEM;
>>>       platform_set_drvdata(pdev, vga);
>>>
>>> +     vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>>> +     if (IS_ERR(vga->vdd)) {
>>> +             ret = PTR_ERR(vga->vdd);
>>> +             if (ret == -EPROBE_DEFER)
>>> +                     return -EPROBE_DEFER;
>>> +             dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
>>> +     }
>>> +
>>>       vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>>>       if (IS_ERR(vga->ddc)) {
>>>               if (PTR_ERR(vga->ddc) == -ENODEV) {
>>> --
>>> 2.9.3
>>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
>> For more options, visit https://groups.google.com/d/optout.

[linux-sunxi] Re: [PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply

[Chen-Yu Tsai]

Hi,

On Wed, Nov 2, 2016 at 9:33 AM, Chen-Yu Tsai <wens@csie.org> wrote:
> On Mon, Oct 31, 2016 at 2:28 PM, Rob Herring <robh@kernel.org> wrote:
>> On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
>>> Some dumb VGA DACs are active components which require external power.
>>> Add support for specifying a regulator as its power supply.
>>>
>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>> ---
>>>  .../bindings/display/bridge/dumb-vga-dac.txt       |  2 ++
>>
>> For the binding,
>>
>> Acked-by: Rob Herring <robh@kernel.org>

Any comments on this patch from the DRM people?

ChenYu

>>
>> One code comment below...
>>
>>>  drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++++++++++++++
>>>  2 files changed, 37 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> index 003bc246a270..164cbb15f04c 100644
>>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>>>  - Video port 0 for RGB input
>>>  - Video port 1 for VGA output
>>>
>>> +Optional properties:
>>> +- vdd-supply: Power supply for DAC
>>>
>>>  Example
>>>  -------
>>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> index afec232185a7..59781e031220 100644
>>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>> @@ -12,6 +12,7 @@
>>>
>>>  #include <linux/module.h>
>>>  #include <linux/of_graph.h>
>>> +#include <linux/regulator/consumer.h>
>>>
>>>  #include <drm/drmP.h>
>>>  #include <drm/drm_atomic_helper.h>
>>> @@ -23,6 +24,7 @@ struct dumb_vga {
>>>       struct drm_connector    connector;
>>>
>>>       struct i2c_adapter      *ddc;
>>> +     struct regulator        *vdd;
>>>  };
>>>
>>>  static inline struct dumb_vga *
>>> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>>>       return 0;
>>>  }
>>>
>>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>>> +{
>>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>> +     int ret;
>>> +
>>> +     if (!IS_ERR(vga->vdd)) {
>>
>> if (IS_ERR())
>>         return;
>>
>> ...will save some intentation.
>
> I thought about that, though if someone were to add more stuff to
> this, such as an enable GPIO, they might have to rework it. A standalone
> block of code would be easier to work with. I'm OK either way though.
>
> ChenYu
>
>>
>>> +             ret = regulator_enable(vga->vdd);
>>> +
>>> +             if (ret) {
>>> +                     DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
>>> +                     return;
>>> +             }
>>> +     }
>>> +}
>>> +
>>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>>> +{
>>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>> +
>>> +     if (!IS_ERR(vga->vdd))
>>> +             regulator_disable(vga->vdd);
>>> +}
>>> +
>>>  static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>>>       .attach         = dumb_vga_attach,
>>> +     .enable         = dumb_vga_enable,
>>> +     .disable        = dumb_vga_disable,
>>>  };
>>>
>>>  static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>>> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>>>               return -ENOMEM;
>>>       platform_set_drvdata(pdev, vga);
>>>
>>> +     vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>>> +     if (IS_ERR(vga->vdd)) {
>>> +             ret = PTR_ERR(vga->vdd);
>>> +             if (ret == -EPROBE_DEFER)
>>> +                     return -EPROBE_DEFER;
>>> +             dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);
>>> +     }
>>> +
>>>       vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>>>       if (IS_ERR(vga->ddc)) {
>>>               if (PTR_ERR(vga->ddc) == -ENODEV) {
>>> --
>>> 2.9.3
>>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] Re: [PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply

[Archit Taneja]

Hi,

On 11/15/2016 08:29 AM, Chen-Yu Tsai wrote:
> Hi,
>
> On Wed, Nov 2, 2016 at 9:33 AM, Chen-Yu Tsai <wens@csie.org> wrote:
>> On Mon, Oct 31, 2016 at 2:28 PM, Rob Herring <robh@kernel.org> wrote:
>>> On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
>>>> Some dumb VGA DACs are active components which require external power.
>>>> Add support for specifying a regulator as its power supply.
>>>>
>>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>>> ---
>>>>  .../bindings/display/bridge/dumb-vga-dac.txt       |  2 ++
>>>
>>> For the binding,
>>>
>>> Acked-by: Rob Herring <robh@kernel.org>
>
> Any comments on this patch from the DRM people?

A comment below.

>
> ChenYu
>
>>>
>>> One code comment below...
>>>
>>>>  drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++++++++++++++
>>>>  2 files changed, 37 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>>> index 003bc246a270..164cbb15f04c 100644
>>>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>>> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>>>>  - Video port 0 for RGB input
>>>>  - Video port 1 for VGA output
>>>>
>>>> +Optional properties:
>>>> +- vdd-supply: Power supply for DAC
>>>>
>>>>  Example
>>>>  -------
>>>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>>> index afec232185a7..59781e031220 100644
>>>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>>> @@ -12,6 +12,7 @@
>>>>
>>>>  #include <linux/module.h>
>>>>  #include <linux/of_graph.h>
>>>> +#include <linux/regulator/consumer.h>
>>>>
>>>>  #include <drm/drmP.h>
>>>>  #include <drm/drm_atomic_helper.h>
>>>> @@ -23,6 +24,7 @@ struct dumb_vga {
>>>>       struct drm_connector    connector;
>>>>
>>>>       struct i2c_adapter      *ddc;
>>>> +     struct regulator        *vdd;
>>>>  };
>>>>
>>>>  static inline struct dumb_vga *
>>>> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>>>>       return 0;
>>>>  }
>>>>
>>>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>>>> +{
>>>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>>> +     int ret;
>>>> +
>>>> +     if (!IS_ERR(vga->vdd)) {
>>>
>>> if (IS_ERR())
>>>         return;
>>>
>>> ...will save some intentation.
>>
>> I thought about that, though if someone were to add more stuff to
>> this, such as an enable GPIO, they might have to rework it. A standalone
>> block of code would be easier to work with. I'm OK either way though.
>>
>> ChenYu
>>
>>>
>>>> +             ret = regulator_enable(vga->vdd);
>>>> +
>>>> +             if (ret) {
>>>> +                     DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
>>>> +                     return;
>>>> +             }
>>>> +     }
>>>> +}
>>>> +
>>>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>>>> +{
>>>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>>> +
>>>> +     if (!IS_ERR(vga->vdd))
>>>> +             regulator_disable(vga->vdd);
>>>> +}
>>>> +
>>>>  static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>>>>       .attach         = dumb_vga_attach,
>>>> +     .enable         = dumb_vga_enable,
>>>> +     .disable        = dumb_vga_disable,
>>>>  };
>>>>
>>>>  static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>>>> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>>>>               return -ENOMEM;
>>>>       platform_set_drvdata(pdev, vga);
>>>>
>>>> +     vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>>>> +     if (IS_ERR(vga->vdd)) {
>>>> +             ret = PTR_ERR(vga->vdd);
>>>> +             if (ret == -EPROBE_DEFER)
>>>> +                     return -EPROBE_DEFER;
>>>> +             dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);

A cleaner way would be to set vga->vdd to NULL here, and do:

	int ret = 0;

	if (vga->vdd)
		ret = regulator_enable(vga->vdd);

	if (ret) {
		...
		return;
	}

	/* if a gpio is added later */
	if (vga->gpio)
		ret = gpiod_set_value_cansleep(vga->gpio, 1);

	if (ret) {
		...
		return;
	}
	...

We don't really care what error code is contained in vga->vdd once we have
identified whether we got a regulator or not.

The same thing can be done for ddc too, but that can done left for later.

If you're okay with the suggestion, can you re-spin a patch?

Thanks,
Archit

>>>> +     }
>>>> +
>>>>       vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>>>>       if (IS_ERR(vga->ddc)) {
>>>>               if (PTR_ERR(vga->ddc) == -ENODEV) {
>>>> --
>>>> 2.9.3
>>>>
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
>>> For more options, visit https://groups.google.com/d/optout.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [linux-sunxi] Re: [PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply

[Archit Taneja]

Hi,

On 11/15/2016 08:29 AM, Chen-Yu Tsai wrote:
> Hi,
>
> On Wed, Nov 2, 2016 at 9:33 AM, Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> wrote:
>> On Mon, Oct 31, 2016 at 2:28 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>> On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
>>>> Some dumb VGA DACs are active components which require external power.
>>>> Add support for specifying a regulator as its power supply.
>>>>
>>>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>>>> ---
>>>>  .../bindings/display/bridge/dumb-vga-dac.txt       |  2 ++
>>>
>>> For the binding,
>>>
>>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> Any comments on this patch from the DRM people?

A comment below.

>
> ChenYu
>
>>>
>>> One code comment below...
>>>
>>>>  drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++++++++++++++
>>>>  2 files changed, 37 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>>> index 003bc246a270..164cbb15f04c 100644
>>>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>>> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>>>>  - Video port 0 for RGB input
>>>>  - Video port 1 for VGA output
>>>>
>>>> +Optional properties:
>>>> +- vdd-supply: Power supply for DAC
>>>>
>>>>  Example
>>>>  -------
>>>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>>> index afec232185a7..59781e031220 100644
>>>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>>> @@ -12,6 +12,7 @@
>>>>
>>>>  #include <linux/module.h>
>>>>  #include <linux/of_graph.h>
>>>> +#include <linux/regulator/consumer.h>
>>>>
>>>>  #include <drm/drmP.h>
>>>>  #include <drm/drm_atomic_helper.h>
>>>> @@ -23,6 +24,7 @@ struct dumb_vga {
>>>>       struct drm_connector    connector;
>>>>
>>>>       struct i2c_adapter      *ddc;
>>>> +     struct regulator        *vdd;
>>>>  };
>>>>
>>>>  static inline struct dumb_vga *
>>>> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>>>>       return 0;
>>>>  }
>>>>
>>>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>>>> +{
>>>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>>> +     int ret;
>>>> +
>>>> +     if (!IS_ERR(vga->vdd)) {
>>>
>>> if (IS_ERR())
>>>         return;
>>>
>>> ...will save some intentation.
>>
>> I thought about that, though if someone were to add more stuff to
>> this, such as an enable GPIO, they might have to rework it. A standalone
>> block of code would be easier to work with. I'm OK either way though.
>>
>> ChenYu
>>
>>>
>>>> +             ret = regulator_enable(vga->vdd);
>>>> +
>>>> +             if (ret) {
>>>> +                     DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
>>>> +                     return;
>>>> +             }
>>>> +     }
>>>> +}
>>>> +
>>>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>>>> +{
>>>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>>> +
>>>> +     if (!IS_ERR(vga->vdd))
>>>> +             regulator_disable(vga->vdd);
>>>> +}
>>>> +
>>>>  static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>>>>       .attach         = dumb_vga_attach,
>>>> +     .enable         = dumb_vga_enable,
>>>> +     .disable        = dumb_vga_disable,
>>>>  };
>>>>
>>>>  static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>>>> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>>>>               return -ENOMEM;
>>>>       platform_set_drvdata(pdev, vga);
>>>>
>>>> +     vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>>>> +     if (IS_ERR(vga->vdd)) {
>>>> +             ret = PTR_ERR(vga->vdd);
>>>> +             if (ret == -EPROBE_DEFER)
>>>> +                     return -EPROBE_DEFER;
>>>> +             dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);

A cleaner way would be to set vga->vdd to NULL here, and do:

	int ret = 0;

	if (vga->vdd)
		ret = regulator_enable(vga->vdd);

	if (ret) {
		...
		return;
	}

	/* if a gpio is added later */
	if (vga->gpio)
		ret = gpiod_set_value_cansleep(vga->gpio, 1);

	if (ret) {
		...
		return;
	}
	...

We don't really care what error code is contained in vga->vdd once we have
identified whether we got a regulator or not.

The same thing can be done for ddc too, but that can done left for later.

If you're okay with the suggestion, can you re-spin a patch?

Thanks,
Archit

>>>> +     }
>>>> +
>>>>       vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>>>>       if (IS_ERR(vga->ddc)) {
>>>>               if (PTR_ERR(vga->ddc) == -ENODEV) {
>>>> --
>>>> 2.9.3
>>>>
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
>>> For more options, visit https://groups.google.com/d/optout.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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

[linux-sunxi] Re: [PATCH v3 1/2] drm/bridge: dumb-vga-dac: Support a VDD regulator supply

[Archit Taneja]

Hi,

On 11/15/2016 08:29 AM, Chen-Yu Tsai wrote:
> Hi,
>
> On Wed, Nov 2, 2016 at 9:33 AM, Chen-Yu Tsai <wens@csie.org> wrote:
>> On Mon, Oct 31, 2016 at 2:28 PM, Rob Herring <robh@kernel.org> wrote:
>>> On Sat, Oct 29, 2016 at 07:06:10PM +0800, Chen-Yu Tsai wrote:
>>>> Some dumb VGA DACs are active components which require external power.
>>>> Add support for specifying a regulator as its power supply.
>>>>
>>>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>>>> ---
>>>>  .../bindings/display/bridge/dumb-vga-dac.txt       |  2 ++
>>>
>>> For the binding,
>>>
>>> Acked-by: Rob Herring <robh@kernel.org>
>
> Any comments on this patch from the DRM people?

A comment below.

>
> ChenYu
>
>>>
>>> One code comment below...
>>>
>>>>  drivers/gpu/drm/bridge/dumb-vga-dac.c              | 35 ++++++++++++++++++++++
>>>>  2 files changed, 37 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>>> index 003bc246a270..164cbb15f04c 100644
>>>> --- a/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>>> +++ b/Documentation/devicetree/bindings/display/bridge/dumb-vga-dac.txt
>>>> @@ -16,6 +16,8 @@ graph bindings specified in Documentation/devicetree/bindings/graph.txt.
>>>>  - Video port 0 for RGB input
>>>>  - Video port 1 for VGA output
>>>>
>>>> +Optional properties:
>>>> +- vdd-supply: Power supply for DAC
>>>>
>>>>  Example
>>>>  -------
>>>> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>>> index afec232185a7..59781e031220 100644
>>>> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>>> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c
>>>> @@ -12,6 +12,7 @@
>>>>
>>>>  #include <linux/module.h>
>>>>  #include <linux/of_graph.h>
>>>> +#include <linux/regulator/consumer.h>
>>>>
>>>>  #include <drm/drmP.h>
>>>>  #include <drm/drm_atomic_helper.h>
>>>> @@ -23,6 +24,7 @@ struct dumb_vga {
>>>>       struct drm_connector    connector;
>>>>
>>>>       struct i2c_adapter      *ddc;
>>>> +     struct regulator        *vdd;
>>>>  };
>>>>
>>>>  static inline struct dumb_vga *
>>>> @@ -124,8 +126,33 @@ static int dumb_vga_attach(struct drm_bridge *bridge)
>>>>       return 0;
>>>>  }
>>>>
>>>> +static void dumb_vga_enable(struct drm_bridge *bridge)
>>>> +{
>>>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>>> +     int ret;
>>>> +
>>>> +     if (!IS_ERR(vga->vdd)) {
>>>
>>> if (IS_ERR())
>>>         return;
>>>
>>> ...will save some intentation.
>>
>> I thought about that, though if someone were to add more stuff to
>> this, such as an enable GPIO, they might have to rework it. A standalone
>> block of code would be easier to work with. I'm OK either way though.
>>
>> ChenYu
>>
>>>
>>>> +             ret = regulator_enable(vga->vdd);
>>>> +
>>>> +             if (ret) {
>>>> +                     DRM_ERROR("Failed to enable vdd regulator: %d\n", ret);
>>>> +                     return;
>>>> +             }
>>>> +     }
>>>> +}
>>>> +
>>>> +static void dumb_vga_disable(struct drm_bridge *bridge)
>>>> +{
>>>> +     struct dumb_vga *vga = drm_bridge_to_dumb_vga(bridge);
>>>> +
>>>> +     if (!IS_ERR(vga->vdd))
>>>> +             regulator_disable(vga->vdd);
>>>> +}
>>>> +
>>>>  static const struct drm_bridge_funcs dumb_vga_bridge_funcs = {
>>>>       .attach         = dumb_vga_attach,
>>>> +     .enable         = dumb_vga_enable,
>>>> +     .disable        = dumb_vga_disable,
>>>>  };
>>>>
>>>>  static struct i2c_adapter *dumb_vga_retrieve_ddc(struct device *dev)
>>>> @@ -169,6 +196,14 @@ static int dumb_vga_probe(struct platform_device *pdev)
>>>>               return -ENOMEM;
>>>>       platform_set_drvdata(pdev, vga);
>>>>
>>>> +     vga->vdd = devm_regulator_get_optional(&pdev->dev, "vdd");
>>>> +     if (IS_ERR(vga->vdd)) {
>>>> +             ret = PTR_ERR(vga->vdd);
>>>> +             if (ret == -EPROBE_DEFER)
>>>> +                     return -EPROBE_DEFER;
>>>> +             dev_dbg(&pdev->dev, "No vdd regulator found: %d\n", ret);

A cleaner way would be to set vga->vdd to NULL here, and do:

	int ret = 0;

	if (vga->vdd)
		ret = regulator_enable(vga->vdd);

	if (ret) {
		...
		return;
	}

	/* if a gpio is added later */
	if (vga->gpio)
		ret = gpiod_set_value_cansleep(vga->gpio, 1);

	if (ret) {
		...
		return;
	}
	...

We don't really care what error code is contained in vga->vdd once we have
identified whether we got a regulator or not.

The same thing can be done for ddc too, but that can done left for later.

If you're okay with the suggestion, can you re-spin a patch?

Thanks,
Archit

>>>> +     }
>>>> +
>>>>       vga->ddc = dumb_vga_retrieve_ddc(&pdev->dev);
>>>>       if (IS_ERR(vga->ddc)) {
>>>>               if (PTR_ERR(vga->ddc) == -ENODEV) {
>>>> --
>>>> 2.9.3
>>>>
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com.
>>> For more options, visit https://groups.google.com/d/optout.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project