All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-09-24 20:05 ` Alexandru Gagniuc
  0 siblings, 0 replies; 43+ messages in thread
From: Alexandru Gagniuc @ 2020-09-24 20:05 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: Alexandru Gagniuc, David Airlie, Daniel Vetter, Rob Herring,
	Andrzej Hajda, Neil Armstrong, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, Mauro Carvalho Chehab, Mark Brown,
	Thomas Bogendoerfer, linux-kernel

On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
voltage before the reset sequence is initiated. On most boards, this
assumption is true at boot-up, so initialization succeeds.

However, when we try to initialize the chip with incorrect supply
voltages, it will not respond to I2C requests. sii902x_probe() fails
with -ENXIO.

To resolve this, look for the "iovcc" and "cvcc12" regulators, and
make sure they are enabled before starting the reset sequence. If
these supplies are not available in devicetree, then they will default
to dummy-regulator. In that case everything will work like before.

This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
On this board, the supplies would be set by the second stage
bootloader, which does not run in falcon mode.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 drivers/gpu/drm/bridge/sii902x.c | 54 ++++++++++++++++++++++++++++----
 1 file changed, 48 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 33fd33f953ec..a5558d83e4c5 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -17,6 +17,7 @@
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/clk.h>
 
 #include <drm/drm_atomic_helper.h>
@@ -168,6 +169,8 @@ struct sii902x {
 	struct drm_connector connector;
 	struct gpio_desc *reset_gpio;
 	struct i2c_mux_core *i2cmux;
+	struct regulator *iovcc;
+	struct regulator *cvcc12;
 	/*
 	 * Mutex protects audio and video functions from interfering
 	 * each other, by keeping their i2c command sequences atomic.
@@ -954,13 +957,13 @@ static const struct drm_bridge_timings default_sii902x_timings = {
 		 | DRM_BUS_FLAG_DE_HIGH,
 };
 
+static int sii902x_init(struct sii902x *sii902x);
+
 static int sii902x_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct device *dev = &client->dev;
-	unsigned int status = 0;
 	struct sii902x *sii902x;
-	u8 chipid[4];
 	int ret;
 
 	ret = i2c_check_functionality(client->adapter,
@@ -989,6 +992,43 @@ static int sii902x_probe(struct i2c_client *client,
 
 	mutex_init(&sii902x->mutex);
 
+	sii902x->iovcc = devm_regulator_get(dev, "iovcc");
+	if (IS_ERR(sii902x->iovcc))
+		return PTR_ERR(sii902x->iovcc);
+
+	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
+	if (IS_ERR(sii902x->cvcc12))
+		return PTR_ERR(sii902x->cvcc12);
+
+	ret = regulator_enable(sii902x->iovcc);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enable iovcc supply: %d\n", ret);
+		return ret;
+	}
+
+	ret = regulator_enable(sii902x->cvcc12);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enable cvcc12 supply: %d\n", ret);
+		regulator_disable(sii902x->iovcc);
+		return PTR_ERR(sii902x->cvcc12);
+	}
+
+	ret = sii902x_init(sii902x);
+	if (ret < 0) {
+		regulator_disable(sii902x->cvcc12);
+		regulator_disable(sii902x->iovcc);
+	}
+
+	return ret;
+}
+
+static int sii902x_init(struct sii902x *sii902x)
+{
+	struct device *dev = &sii902x->i2c->dev;
+	unsigned int status = 0;
+	u8 chipid[4];
+	int ret;
+
 	sii902x_reset(sii902x);
 
 	ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
@@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
 	regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
 	regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
 
-	if (client->irq > 0) {
+	if (sii902x->i2c->irq > 0) {
 		regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
 			     SII902X_HOTPLUG_EVENT);
 
-		ret = devm_request_threaded_irq(dev, client->irq, NULL,
+		ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,
 						sii902x_interrupt,
 						IRQF_ONESHOT, dev_name(dev),
 						sii902x);
@@ -1031,9 +1071,9 @@ static int sii902x_probe(struct i2c_client *client,
 
 	sii902x_audio_codec_init(sii902x, dev);
 
-	i2c_set_clientdata(client, sii902x);
+	i2c_set_clientdata(sii902x->i2c, sii902x);
 
-	sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
+	sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,
 					1, 0, I2C_MUX_GATE,
 					sii902x_i2c_bypass_select,
 					sii902x_i2c_bypass_deselect);
@@ -1051,6 +1091,8 @@ static int sii902x_remove(struct i2c_client *client)
 
 	i2c_mux_del_adapters(sii902x->i2cmux);
 	drm_bridge_remove(&sii902x->bridge);
+	regulator_disable(sii902x->cvcc12);
+	regulator_disable(sii902x->iovcc);
 
 	return 0;
 }
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-09-24 20:05 ` Alexandru Gagniuc
  0 siblings, 0 replies; 43+ messages in thread
From: Alexandru Gagniuc @ 2020-09-24 20:05 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: Jernej Skrabec, Mauro Carvalho Chehab, Neil Armstrong,
	David Airlie, Mark Brown, Jonas Karlman, linux-kernel,
	Andrzej Hajda, Rob Herring, Alexandru Gagniuc,
	Thomas Bogendoerfer, Laurent Pinchart

On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
voltage before the reset sequence is initiated. On most boards, this
assumption is true at boot-up, so initialization succeeds.

However, when we try to initialize the chip with incorrect supply
voltages, it will not respond to I2C requests. sii902x_probe() fails
with -ENXIO.

To resolve this, look for the "iovcc" and "cvcc12" regulators, and
make sure they are enabled before starting the reset sequence. If
these supplies are not available in devicetree, then they will default
to dummy-regulator. In that case everything will work like before.

This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
On this board, the supplies would be set by the second stage
bootloader, which does not run in falcon mode.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 drivers/gpu/drm/bridge/sii902x.c | 54 ++++++++++++++++++++++++++++----
 1 file changed, 48 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 33fd33f953ec..a5558d83e4c5 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -17,6 +17,7 @@
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/clk.h>
 
 #include <drm/drm_atomic_helper.h>
@@ -168,6 +169,8 @@ struct sii902x {
 	struct drm_connector connector;
 	struct gpio_desc *reset_gpio;
 	struct i2c_mux_core *i2cmux;
+	struct regulator *iovcc;
+	struct regulator *cvcc12;
 	/*
 	 * Mutex protects audio and video functions from interfering
 	 * each other, by keeping their i2c command sequences atomic.
@@ -954,13 +957,13 @@ static const struct drm_bridge_timings default_sii902x_timings = {
 		 | DRM_BUS_FLAG_DE_HIGH,
 };
 
+static int sii902x_init(struct sii902x *sii902x);
+
 static int sii902x_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct device *dev = &client->dev;
-	unsigned int status = 0;
 	struct sii902x *sii902x;
-	u8 chipid[4];
 	int ret;
 
 	ret = i2c_check_functionality(client->adapter,
@@ -989,6 +992,43 @@ static int sii902x_probe(struct i2c_client *client,
 
 	mutex_init(&sii902x->mutex);
 
+	sii902x->iovcc = devm_regulator_get(dev, "iovcc");
+	if (IS_ERR(sii902x->iovcc))
+		return PTR_ERR(sii902x->iovcc);
+
+	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
+	if (IS_ERR(sii902x->cvcc12))
+		return PTR_ERR(sii902x->cvcc12);
+
+	ret = regulator_enable(sii902x->iovcc);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enable iovcc supply: %d\n", ret);
+		return ret;
+	}
+
+	ret = regulator_enable(sii902x->cvcc12);
+	if (ret < 0) {
+		dev_err(dev, "Failed to enable cvcc12 supply: %d\n", ret);
+		regulator_disable(sii902x->iovcc);
+		return PTR_ERR(sii902x->cvcc12);
+	}
+
+	ret = sii902x_init(sii902x);
+	if (ret < 0) {
+		regulator_disable(sii902x->cvcc12);
+		regulator_disable(sii902x->iovcc);
+	}
+
+	return ret;
+}
+
+static int sii902x_init(struct sii902x *sii902x)
+{
+	struct device *dev = &sii902x->i2c->dev;
+	unsigned int status = 0;
+	u8 chipid[4];
+	int ret;
+
 	sii902x_reset(sii902x);
 
 	ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
@@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
 	regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
 	regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
 
-	if (client->irq > 0) {
+	if (sii902x->i2c->irq > 0) {
 		regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
 			     SII902X_HOTPLUG_EVENT);
 
-		ret = devm_request_threaded_irq(dev, client->irq, NULL,
+		ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,
 						sii902x_interrupt,
 						IRQF_ONESHOT, dev_name(dev),
 						sii902x);
@@ -1031,9 +1071,9 @@ static int sii902x_probe(struct i2c_client *client,
 
 	sii902x_audio_codec_init(sii902x, dev);
 
-	i2c_set_clientdata(client, sii902x);
+	i2c_set_clientdata(sii902x->i2c, sii902x);
 
-	sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
+	sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,
 					1, 0, I2C_MUX_GATE,
 					sii902x_i2c_bypass_select,
 					sii902x_i2c_bypass_deselect);
@@ -1051,6 +1091,8 @@ static int sii902x_remove(struct i2c_client *client)
 
 	i2c_mux_del_adapters(sii902x->i2cmux);
 	drm_bridge_remove(&sii902x->bridge);
+	regulator_disable(sii902x->cvcc12);
+	regulator_disable(sii902x->iovcc);
 
 	return 0;
 }
-- 
2.25.4

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

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH 2/2] dt-bindings: display: sii902x: Add supply bindings
  2020-09-24 20:05 ` Alexandru Gagniuc
@ 2020-09-24 20:05   ` Alexandru Gagniuc
  -1 siblings, 0 replies; 43+ messages in thread
From: Alexandru Gagniuc @ 2020-09-24 20:05 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: Alexandru Gagniuc, David Airlie, Daniel Vetter, Rob Herring,
	Andrzej Hajda, Neil Armstrong, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, Thomas Bogendoerfer, Mauro Carvalho Chehab,
	Mark Brown, linux-kernel

The sii902x chip family requires IO and core voltages to reach the
correct voltage before chip initialization. Add binding for describing
the two supplies.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 Documentation/devicetree/bindings/display/bridge/sii902x.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
index 0d1db3f9da84..02c21b584741 100644
--- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
+++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
@@ -8,6 +8,8 @@ Optional properties:
 	- interrupts: describe the interrupt line used to inform the host
 	  about hotplug events.
 	- reset-gpios: OF device-tree gpio specification for RST_N pin.
+	- iovcc-supply: I/O Supply Voltage (1.8V or 3.3V)
+	- cvcc12-supply: Digital Core Supply Voltage (1.2V)
 
 	HDMI audio properties:
 	- #sound-dai-cells: <0> or <1>. <0> if only i2s or spdif pin
@@ -54,6 +56,8 @@ Example:
 		compatible = "sil,sii9022";
 		reg = <0x39>;
 		reset-gpios = <&pioA 1 0>;
+		iovcc-supply = <&v3v3_hdmi>;
+		cvcc12-supply = <&v1v2_hdmi>;
 
 		#sound-dai-cells = <0>;
 		sil,i2s-data-lanes = < 0 1 2 >;
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH 2/2] dt-bindings: display: sii902x: Add supply bindings
@ 2020-09-24 20:05   ` Alexandru Gagniuc
  0 siblings, 0 replies; 43+ messages in thread
From: Alexandru Gagniuc @ 2020-09-24 20:05 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: Jernej Skrabec, Thomas Bogendoerfer, Neil Armstrong,
	David Airlie, Mark Brown, Jonas Karlman, linux-kernel,
	Andrzej Hajda, Rob Herring, Alexandru Gagniuc,
	Mauro Carvalho Chehab, Laurent Pinchart

The sii902x chip family requires IO and core voltages to reach the
correct voltage before chip initialization. Add binding for describing
the two supplies.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 Documentation/devicetree/bindings/display/bridge/sii902x.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
index 0d1db3f9da84..02c21b584741 100644
--- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
+++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
@@ -8,6 +8,8 @@ Optional properties:
 	- interrupts: describe the interrupt line used to inform the host
 	  about hotplug events.
 	- reset-gpios: OF device-tree gpio specification for RST_N pin.
+	- iovcc-supply: I/O Supply Voltage (1.8V or 3.3V)
+	- cvcc12-supply: Digital Core Supply Voltage (1.2V)
 
 	HDMI audio properties:
 	- #sound-dai-cells: <0> or <1>. <0> if only i2s or spdif pin
@@ -54,6 +56,8 @@ Example:
 		compatible = "sil,sii9022";
 		reg = <0x39>;
 		reset-gpios = <&pioA 1 0>;
+		iovcc-supply = <&v3v3_hdmi>;
+		cvcc12-supply = <&v1v2_hdmi>;
 
 		#sound-dai-cells = <0>;
 		sil,i2s-data-lanes = < 0 1 2 >;
-- 
2.25.4

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

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* Re: [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
  2020-09-24 20:05 ` Alexandru Gagniuc
@ 2020-09-24 20:22   ` Fabio Estevam
  -1 siblings, 0 replies; 43+ messages in thread
From: Fabio Estevam @ 2020-09-24 20:22 UTC (permalink / raw)
  To: Alexandru Gagniuc
  Cc: DRI mailing list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	David Airlie, Daniel Vetter, Rob Herring, Andrzej Hajda,
	Neil Armstrong, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	Mauro Carvalho Chehab, Mark Brown, Thomas Bogendoerfer,
	linux-kernel

Hi Alexandre,

On Thu, Sep 24, 2020 at 5:16 PM Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:

> +       ret = regulator_enable(sii902x->cvcc12);
> +       if (ret < 0) {
> +               dev_err(dev, "Failed to enable cvcc12 supply: %d\n", ret);
> +               regulator_disable(sii902x->iovcc);
> +               return PTR_ERR(sii902x->cvcc12);

return ret;

>
>         ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
> @@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
>         regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
>         regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
>
> -       if (client->irq > 0) {
> +       if (sii902x->i2c->irq > 0) {

Unrelated change.

>                 regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
>                              SII902X_HOTPLUG_EVENT);
>
> -               ret = devm_request_threaded_irq(dev, client->irq, NULL,
> +               ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,

 Unrelated change.
                                                 sii902x_interrupt,
>                                                 IRQF_ONESHOT, dev_name(dev),
>                                                 sii902x);
> @@ -1031,9 +1071,9 @@ static int sii902x_probe(struct i2c_client *client,
>
>         sii902x_audio_codec_init(sii902x, dev);
>
> -       i2c_set_clientdata(client, sii902x);
> +       i2c_set_clientdata(sii902x->i2c, sii902x);

Unrelated change.

> -       sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
> +       sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,

Unrelated change.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-09-24 20:22   ` Fabio Estevam
  0 siblings, 0 replies; 43+ messages in thread
From: Fabio Estevam @ 2020-09-24 20:22 UTC (permalink / raw)
  To: Alexandru Gagniuc
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Jernej Skrabec, Mauro Carvalho Chehab, Neil Armstrong,
	David Airlie, Mark Brown, Jonas Karlman, linux-kernel,
	DRI mailing list, Andrzej Hajda, Rob Herring, Laurent Pinchart,
	Thomas Bogendoerfer

Hi Alexandre,

On Thu, Sep 24, 2020 at 5:16 PM Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:

> +       ret = regulator_enable(sii902x->cvcc12);
> +       if (ret < 0) {
> +               dev_err(dev, "Failed to enable cvcc12 supply: %d\n", ret);
> +               regulator_disable(sii902x->iovcc);
> +               return PTR_ERR(sii902x->cvcc12);

return ret;

>
>         ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
> @@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
>         regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
>         regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
>
> -       if (client->irq > 0) {
> +       if (sii902x->i2c->irq > 0) {

Unrelated change.

>                 regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
>                              SII902X_HOTPLUG_EVENT);
>
> -               ret = devm_request_threaded_irq(dev, client->irq, NULL,
> +               ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,

 Unrelated change.
                                                 sii902x_interrupt,
>                                                 IRQF_ONESHOT, dev_name(dev),
>                                                 sii902x);
> @@ -1031,9 +1071,9 @@ static int sii902x_probe(struct i2c_client *client,
>
>         sii902x_audio_codec_init(sii902x, dev);
>
> -       i2c_set_clientdata(client, sii902x);
> +       i2c_set_clientdata(sii902x->i2c, sii902x);

Unrelated change.

> -       sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
> +       sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,

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

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
  2020-09-24 20:22   ` Fabio Estevam
@ 2020-09-24 20:34     ` Alex G.
  -1 siblings, 0 replies; 43+ messages in thread
From: Alex G. @ 2020-09-24 20:34 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: DRI mailing list,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	David Airlie, Daniel Vetter, Rob Herring, Andrzej Hajda,
	Neil Armstrong, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	Mauro Carvalho Chehab, Mark Brown, Thomas Bogendoerfer,
	linux-kernel

On 9/24/20 3:22 PM, Fabio Estevam wrote:
Hi Fabio,

> On Thu, Sep 24, 2020 at 5:16 PM Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:
> 
>> +       ret = regulator_enable(sii902x->cvcc12);
>> +       if (ret < 0) {
>> +               dev_err(dev, "Failed to enable cvcc12 supply: %d\n", ret);
>> +               regulator_disable(sii902x->iovcc);
>> +               return PTR_ERR(sii902x->cvcc12);
> 
> return ret;

Thank you for catching that. I will fix it in v2.

>>
>>          ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
>> @@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
>>          regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
>>          regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
>>
>> -       if (client->irq > 0) {
>> +       if (sii902x->i2c->irq > 0) {
> 
> Unrelated change.
[snip]
>   Unrelated change.
[snip]
> Unrelated change.
[snip]
> Unrelated change.

The i2c initialization is moved into a separate function. Hence 'client' 
is no longer available. Instead, it can be accessed via 'sii902x->i2c'.

Alex

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-09-24 20:34     ` Alex G.
  0 siblings, 0 replies; 43+ messages in thread
From: Alex G. @ 2020-09-24 20:34 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Jernej Skrabec, Mauro Carvalho Chehab, Neil Armstrong,
	David Airlie, Mark Brown, Jonas Karlman, linux-kernel,
	DRI mailing list, Andrzej Hajda, Rob Herring, Laurent Pinchart,
	Thomas Bogendoerfer

On 9/24/20 3:22 PM, Fabio Estevam wrote:
Hi Fabio,

> On Thu, Sep 24, 2020 at 5:16 PM Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:
> 
>> +       ret = regulator_enable(sii902x->cvcc12);
>> +       if (ret < 0) {
>> +               dev_err(dev, "Failed to enable cvcc12 supply: %d\n", ret);
>> +               regulator_disable(sii902x->iovcc);
>> +               return PTR_ERR(sii902x->cvcc12);
> 
> return ret;

Thank you for catching that. I will fix it in v2.

>>
>>          ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
>> @@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
>>          regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
>>          regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
>>
>> -       if (client->irq > 0) {
>> +       if (sii902x->i2c->irq > 0) {
> 
> Unrelated change.
[snip]
>   Unrelated change.
[snip]
> Unrelated change.
[snip]
> Unrelated change.

The i2c initialization is moved into a separate function. Hence 'client' 
is no longer available. Instead, it can be accessed via 'sii902x->i2c'.

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

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
  2020-09-24 20:05 ` Alexandru Gagniuc
  (?)
  (?)
@ 2020-09-26 12:50   ` Dan Carpenter
  -1 siblings, 0 replies; 43+ messages in thread
From: Dan Carpenter @ 2020-09-26 12:50 UTC (permalink / raw)
  To: kbuild, Alexandru Gagniuc, dri-devel, devicetree
  Cc: lkp, kbuild-all, Alexandru Gagniuc, David Airlie, Daniel Vetter,
	Rob Herring, Andrzej Hajda, Neil Armstrong, Laurent Pinchart,
	Jonas Karlman

[-- Attachment #1: Type: text/plain, Size: 2667 bytes --]

Hi Alexandru,

url:    https://github.com/0day-ci/linux/commits/Alexandru-Gagniuc/drm-bridge-sii902x-Enable-I-O-and-core-VCC-supplies-if-present/20200925-041504
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-m001-20200925 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/gpu/drm/bridge/sii902x.c:1013 sii902x_probe() warn: passing zero to 'PTR_ERR'

vim +/PTR_ERR +1013 drivers/gpu/drm/bridge/sii902x.c

6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24   999  	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1000  	if (IS_ERR(sii902x->cvcc12))
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1001  		return PTR_ERR(sii902x->cvcc12);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1002  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1003  	ret = regulator_enable(sii902x->iovcc);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1004  	if (ret < 0) {
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1005  		dev_err(dev, "Failed to enable iovcc supply: %d\n", ret);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1006  		return ret;
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1007  	}
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1008  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1009  	ret = regulator_enable(sii902x->cvcc12);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1010  	if (ret < 0) {
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1011  		dev_err(dev, "Failed to enable cvcc12 supply: %d\n", ret);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1012  		regulator_disable(sii902x->iovcc);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24 @1013  		return PTR_ERR(sii902x->cvcc12);

return ret;

6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1014  	}
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1015  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1016  	ret = sii902x_init(sii902x);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1017  	if (ret < 0) {
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1018  		regulator_disable(sii902x->cvcc12);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1019  		regulator_disable(sii902x->iovcc);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1020  	}
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1021  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1022  	return ret;
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1023  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30891 bytes --]

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-09-26 12:50   ` Dan Carpenter
  0 siblings, 0 replies; 43+ messages in thread
From: Dan Carpenter @ 2020-09-26 12:50 UTC (permalink / raw)
  To: kbuild, Alexandru Gagniuc, dri-devel, devicetree
  Cc: kbuild-all, lkp, Neil Armstrong, David Airlie, Jonas Karlman,
	Andrzej Hajda, Rob Herring, Alexandru Gagniuc, Laurent Pinchart

[-- Attachment #1: Type: text/plain, Size: 2667 bytes --]

Hi Alexandru,

url:    https://github.com/0day-ci/linux/commits/Alexandru-Gagniuc/drm-bridge-sii902x-Enable-I-O-and-core-VCC-supplies-if-present/20200925-041504
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-m001-20200925 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/gpu/drm/bridge/sii902x.c:1013 sii902x_probe() warn: passing zero to 'PTR_ERR'

vim +/PTR_ERR +1013 drivers/gpu/drm/bridge/sii902x.c

6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24   999  	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1000  	if (IS_ERR(sii902x->cvcc12))
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1001  		return PTR_ERR(sii902x->cvcc12);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1002  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1003  	ret = regulator_enable(sii902x->iovcc);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1004  	if (ret < 0) {
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1005  		dev_err(dev, "Failed to enable iovcc supply: %d\n", ret);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1006  		return ret;
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1007  	}
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1008  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1009  	ret = regulator_enable(sii902x->cvcc12);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1010  	if (ret < 0) {
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1011  		dev_err(dev, "Failed to enable cvcc12 supply: %d\n", ret);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1012  		regulator_disable(sii902x->iovcc);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24 @1013  		return PTR_ERR(sii902x->cvcc12);

return ret;

6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1014  	}
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1015  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1016  	ret = sii902x_init(sii902x);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1017  	if (ret < 0) {
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1018  		regulator_disable(sii902x->cvcc12);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1019  		regulator_disable(sii902x->iovcc);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1020  	}
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1021  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1022  	return ret;
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1023  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30891 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-09-26 12:50   ` Dan Carpenter
  0 siblings, 0 replies; 43+ messages in thread
From: Dan Carpenter @ 2020-09-26 12:50 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 2717 bytes --]

Hi Alexandru,

url:    https://github.com/0day-ci/linux/commits/Alexandru-Gagniuc/drm-bridge-sii902x-Enable-I-O-and-core-VCC-supplies-if-present/20200925-041504
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-m001-20200925 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/gpu/drm/bridge/sii902x.c:1013 sii902x_probe() warn: passing zero to 'PTR_ERR'

vim +/PTR_ERR +1013 drivers/gpu/drm/bridge/sii902x.c

6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24   999  	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1000  	if (IS_ERR(sii902x->cvcc12))
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1001  		return PTR_ERR(sii902x->cvcc12);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1002  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1003  	ret = regulator_enable(sii902x->iovcc);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1004  	if (ret < 0) {
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1005  		dev_err(dev, "Failed to enable iovcc supply: %d\n", ret);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1006  		return ret;
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1007  	}
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1008  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1009  	ret = regulator_enable(sii902x->cvcc12);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1010  	if (ret < 0) {
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1011  		dev_err(dev, "Failed to enable cvcc12 supply: %d\n", ret);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1012  		regulator_disable(sii902x->iovcc);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24 @1013  		return PTR_ERR(sii902x->cvcc12);

return ret;

6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1014  	}
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1015  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1016  	ret = sii902x_init(sii902x);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1017  	if (ret < 0) {
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1018  		regulator_disable(sii902x->cvcc12);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1019  		regulator_disable(sii902x->iovcc);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1020  	}
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1021  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1022  	return ret;
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1023  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 30891 bytes --]

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-09-26 12:50   ` Dan Carpenter
  0 siblings, 0 replies; 43+ messages in thread
From: Dan Carpenter @ 2020-09-26 12:50 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2717 bytes --]

Hi Alexandru,

url:    https://github.com/0day-ci/linux/commits/Alexandru-Gagniuc/drm-bridge-sii902x-Enable-I-O-and-core-VCC-supplies-if-present/20200925-041504
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-m001-20200925 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/gpu/drm/bridge/sii902x.c:1013 sii902x_probe() warn: passing zero to 'PTR_ERR'

vim +/PTR_ERR +1013 drivers/gpu/drm/bridge/sii902x.c

6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24   999  	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1000  	if (IS_ERR(sii902x->cvcc12))
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1001  		return PTR_ERR(sii902x->cvcc12);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1002  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1003  	ret = regulator_enable(sii902x->iovcc);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1004  	if (ret < 0) {
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1005  		dev_err(dev, "Failed to enable iovcc supply: %d\n", ret);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1006  		return ret;
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1007  	}
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1008  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1009  	ret = regulator_enable(sii902x->cvcc12);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1010  	if (ret < 0) {
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1011  		dev_err(dev, "Failed to enable cvcc12 supply: %d\n", ret);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1012  		regulator_disable(sii902x->iovcc);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24 @1013  		return PTR_ERR(sii902x->cvcc12);

return ret;

6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1014  	}
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1015  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1016  	ret = sii902x_init(sii902x);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1017  	if (ret < 0) {
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1018  		regulator_disable(sii902x->cvcc12);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1019  		regulator_disable(sii902x->iovcc);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1020  	}
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1021  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1022  	return ret;
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1023  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 30891 bytes --]

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH 2/2] dt-bindings: display: sii902x: Add supply bindings
  2020-09-24 20:05   ` Alexandru Gagniuc
@ 2020-09-26 18:42     ` Sam Ravnborg
  -1 siblings, 0 replies; 43+ messages in thread
From: Sam Ravnborg @ 2020-09-26 18:42 UTC (permalink / raw)
  To: Alexandru Gagniuc
  Cc: dri-devel, devicetree, Jernej Skrabec, Thomas Bogendoerfer,
	Neil Armstrong, David Airlie, Mark Brown, Jonas Karlman,
	linux-kernel, Andrzej Hajda, Rob Herring, Mauro Carvalho Chehab,
	Laurent Pinchart

Hi Alexandru

On Thu, Sep 24, 2020 at 03:05:06PM -0500, Alexandru Gagniuc wrote:
> The sii902x chip family requires IO and core voltages to reach the
> correct voltage before chip initialization. Add binding for describing
> the two supplies.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

It is not mandatory but encouraged to convert a binding to DT schema
format before updating it. This is in order to reach a point
in time where all bindings are in DT Schema format thus
allowing much better verification of the DT files.

	Sam

> ---
>  Documentation/devicetree/bindings/display/bridge/sii902x.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> index 0d1db3f9da84..02c21b584741 100644
> --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> @@ -8,6 +8,8 @@ Optional properties:
>  	- interrupts: describe the interrupt line used to inform the host
>  	  about hotplug events.
>  	- reset-gpios: OF device-tree gpio specification for RST_N pin.
> +	- iovcc-supply: I/O Supply Voltage (1.8V or 3.3V)
> +	- cvcc12-supply: Digital Core Supply Voltage (1.2V)
>  
>  	HDMI audio properties:
>  	- #sound-dai-cells: <0> or <1>. <0> if only i2s or spdif pin
> @@ -54,6 +56,8 @@ Example:
>  		compatible = "sil,sii9022";
>  		reg = <0x39>;
>  		reset-gpios = <&pioA 1 0>;
> +		iovcc-supply = <&v3v3_hdmi>;
> +		cvcc12-supply = <&v1v2_hdmi>;
>  
>  		#sound-dai-cells = <0>;
>  		sil,i2s-data-lanes = < 0 1 2 >;
> -- 
> 2.25.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH 2/2] dt-bindings: display: sii902x: Add supply bindings
@ 2020-09-26 18:42     ` Sam Ravnborg
  0 siblings, 0 replies; 43+ messages in thread
From: Sam Ravnborg @ 2020-09-26 18:42 UTC (permalink / raw)
  To: Alexandru Gagniuc
  Cc: devicetree, Jernej Skrabec, Thomas Bogendoerfer, Rob Herring,
	Neil Armstrong, David Airlie, Jonas Karlman, linux-kernel,
	dri-devel, Andrzej Hajda, Mark Brown, Laurent Pinchart,
	Mauro Carvalho Chehab

Hi Alexandru

On Thu, Sep 24, 2020 at 03:05:06PM -0500, Alexandru Gagniuc wrote:
> The sii902x chip family requires IO and core voltages to reach the
> correct voltage before chip initialization. Add binding for describing
> the two supplies.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

It is not mandatory but encouraged to convert a binding to DT schema
format before updating it. This is in order to reach a point
in time where all bindings are in DT Schema format thus
allowing much better verification of the DT files.

	Sam

> ---
>  Documentation/devicetree/bindings/display/bridge/sii902x.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> index 0d1db3f9da84..02c21b584741 100644
> --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
> @@ -8,6 +8,8 @@ Optional properties:
>  	- interrupts: describe the interrupt line used to inform the host
>  	  about hotplug events.
>  	- reset-gpios: OF device-tree gpio specification for RST_N pin.
> +	- iovcc-supply: I/O Supply Voltage (1.8V or 3.3V)
> +	- cvcc12-supply: Digital Core Supply Voltage (1.2V)
>  
>  	HDMI audio properties:
>  	- #sound-dai-cells: <0> or <1>. <0> if only i2s or spdif pin
> @@ -54,6 +56,8 @@ Example:
>  		compatible = "sil,sii9022";
>  		reg = <0x39>;
>  		reset-gpios = <&pioA 1 0>;
> +		iovcc-supply = <&v3v3_hdmi>;
> +		cvcc12-supply = <&v1v2_hdmi>;
>  
>  		#sound-dai-cells = <0>;
>  		sil,i2s-data-lanes = < 0 1 2 >;
> -- 
> 2.25.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
  2020-09-24 20:05 ` Alexandru Gagniuc
@ 2020-09-26 18:49   ` Sam Ravnborg
  -1 siblings, 0 replies; 43+ messages in thread
From: Sam Ravnborg @ 2020-09-26 18:49 UTC (permalink / raw)
  To: Alexandru Gagniuc
  Cc: dri-devel, devicetree, Jernej Skrabec, Mauro Carvalho Chehab,
	Neil Armstrong, David Airlie, Mark Brown, Jonas Karlman,
	linux-kernel, Andrzej Hajda, Rob Herring, Thomas Bogendoerfer,
	Laurent Pinchart

Hi Alexandru

On Thu, Sep 24, 2020 at 03:05:05PM -0500, Alexandru Gagniuc wrote:
> On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
> voltage before the reset sequence is initiated. On most boards, this
> assumption is true at boot-up, so initialization succeeds.
> 
> However, when we try to initialize the chip with incorrect supply
> voltages, it will not respond to I2C requests. sii902x_probe() fails
> with -ENXIO.
> 
> To resolve this, look for the "iovcc" and "cvcc12" regulators, and
> make sure they are enabled before starting the reset sequence. If
> these supplies are not available in devicetree, then they will default
> to dummy-regulator. In that case everything will work like before.
> 
> This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
> On this board, the supplies would be set by the second stage
> bootloader, which does not run in falcon mode.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

One nitpick here. The binding should be present in the tree before
you start using it. So this patch should be applied after the binding.

One detail below - I think others have already commented on the rest.

	Sam
> ---
>  drivers/gpu/drm/bridge/sii902x.c | 54 ++++++++++++++++++++++++++++----
>  1 file changed, 48 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index 33fd33f953ec..a5558d83e4c5 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -17,6 +17,7 @@
>  #include <linux/i2c.h>
>  #include <linux/module.h>
>  #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/clk.h>
>  
>  #include <drm/drm_atomic_helper.h>
> @@ -168,6 +169,8 @@ struct sii902x {
>  	struct drm_connector connector;
>  	struct gpio_desc *reset_gpio;
>  	struct i2c_mux_core *i2cmux;
> +	struct regulator *iovcc;
> +	struct regulator *cvcc12;
>  	/*
>  	 * Mutex protects audio and video functions from interfering
>  	 * each other, by keeping their i2c command sequences atomic.
> @@ -954,13 +957,13 @@ static const struct drm_bridge_timings default_sii902x_timings = {
>  		 | DRM_BUS_FLAG_DE_HIGH,
>  };
>  
> +static int sii902x_init(struct sii902x *sii902x);
> +
>  static int sii902x_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
>  	struct device *dev = &client->dev;
> -	unsigned int status = 0;
>  	struct sii902x *sii902x;
> -	u8 chipid[4];
>  	int ret;
>  
>  	ret = i2c_check_functionality(client->adapter,
> @@ -989,6 +992,43 @@ static int sii902x_probe(struct i2c_client *client,
>  
>  	mutex_init(&sii902x->mutex);
>  
> +	sii902x->iovcc = devm_regulator_get(dev, "iovcc");
> +	if (IS_ERR(sii902x->iovcc))
> +		return PTR_ERR(sii902x->iovcc);
Consider using dev_err_probe() here.

> +
> +	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
> +	if (IS_ERR(sii902x->cvcc12))
> +		return PTR_ERR(sii902x->cvcc12);
Consider using dev_err_probe() here.
> +
> +	ret = regulator_enable(sii902x->iovcc);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to enable iovcc supply: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regulator_enable(sii902x->cvcc12);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to enable cvcc12 supply: %d\n", ret);
> +		regulator_disable(sii902x->iovcc);
> +		return PTR_ERR(sii902x->cvcc12);
> +	}
> +
> +	ret = sii902x_init(sii902x);
> +	if (ret < 0) {
> +		regulator_disable(sii902x->cvcc12);
> +		regulator_disable(sii902x->iovcc);
> +	}
> +
> +	return ret;
> +}
> +
> +static int sii902x_init(struct sii902x *sii902x)
> +{
> +	struct device *dev = &sii902x->i2c->dev;
> +	unsigned int status = 0;
> +	u8 chipid[4];
> +	int ret;
> +
>  	sii902x_reset(sii902x);
>  
>  	ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
> @@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
>  	regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
>  	regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
>  
> -	if (client->irq > 0) {
> +	if (sii902x->i2c->irq > 0) {
>  		regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
>  			     SII902X_HOTPLUG_EVENT);
>  
> -		ret = devm_request_threaded_irq(dev, client->irq, NULL,
> +		ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,
>  						sii902x_interrupt,
>  						IRQF_ONESHOT, dev_name(dev),
>  						sii902x);
> @@ -1031,9 +1071,9 @@ static int sii902x_probe(struct i2c_client *client,
>  
>  	sii902x_audio_codec_init(sii902x, dev);
>  
> -	i2c_set_clientdata(client, sii902x);
> +	i2c_set_clientdata(sii902x->i2c, sii902x);
>  
> -	sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
> +	sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,
>  					1, 0, I2C_MUX_GATE,
>  					sii902x_i2c_bypass_select,
>  					sii902x_i2c_bypass_deselect);
> @@ -1051,6 +1091,8 @@ static int sii902x_remove(struct i2c_client *client)
>  
>  	i2c_mux_del_adapters(sii902x->i2cmux);
>  	drm_bridge_remove(&sii902x->bridge);
> +	regulator_disable(sii902x->cvcc12);
> +	regulator_disable(sii902x->iovcc);
>  
>  	return 0;
>  }
> -- 
> 2.25.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-09-26 18:49   ` Sam Ravnborg
  0 siblings, 0 replies; 43+ messages in thread
From: Sam Ravnborg @ 2020-09-26 18:49 UTC (permalink / raw)
  To: Alexandru Gagniuc
  Cc: devicetree, Jernej Skrabec, Thomas Bogendoerfer, Rob Herring,
	Neil Armstrong, Mauro Carvalho Chehab, Jonas Karlman,
	linux-kernel, dri-devel, Andrzej Hajda, David Airlie, Mark Brown,
	Laurent Pinchart

Hi Alexandru

On Thu, Sep 24, 2020 at 03:05:05PM -0500, Alexandru Gagniuc wrote:
> On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
> voltage before the reset sequence is initiated. On most boards, this
> assumption is true at boot-up, so initialization succeeds.
> 
> However, when we try to initialize the chip with incorrect supply
> voltages, it will not respond to I2C requests. sii902x_probe() fails
> with -ENXIO.
> 
> To resolve this, look for the "iovcc" and "cvcc12" regulators, and
> make sure they are enabled before starting the reset sequence. If
> these supplies are not available in devicetree, then they will default
> to dummy-regulator. In that case everything will work like before.
> 
> This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
> On this board, the supplies would be set by the second stage
> bootloader, which does not run in falcon mode.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

One nitpick here. The binding should be present in the tree before
you start using it. So this patch should be applied after the binding.

One detail below - I think others have already commented on the rest.

	Sam
> ---
>  drivers/gpu/drm/bridge/sii902x.c | 54 ++++++++++++++++++++++++++++----
>  1 file changed, 48 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index 33fd33f953ec..a5558d83e4c5 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -17,6 +17,7 @@
>  #include <linux/i2c.h>
>  #include <linux/module.h>
>  #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/clk.h>
>  
>  #include <drm/drm_atomic_helper.h>
> @@ -168,6 +169,8 @@ struct sii902x {
>  	struct drm_connector connector;
>  	struct gpio_desc *reset_gpio;
>  	struct i2c_mux_core *i2cmux;
> +	struct regulator *iovcc;
> +	struct regulator *cvcc12;
>  	/*
>  	 * Mutex protects audio and video functions from interfering
>  	 * each other, by keeping their i2c command sequences atomic.
> @@ -954,13 +957,13 @@ static const struct drm_bridge_timings default_sii902x_timings = {
>  		 | DRM_BUS_FLAG_DE_HIGH,
>  };
>  
> +static int sii902x_init(struct sii902x *sii902x);
> +
>  static int sii902x_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
>  	struct device *dev = &client->dev;
> -	unsigned int status = 0;
>  	struct sii902x *sii902x;
> -	u8 chipid[4];
>  	int ret;
>  
>  	ret = i2c_check_functionality(client->adapter,
> @@ -989,6 +992,43 @@ static int sii902x_probe(struct i2c_client *client,
>  
>  	mutex_init(&sii902x->mutex);
>  
> +	sii902x->iovcc = devm_regulator_get(dev, "iovcc");
> +	if (IS_ERR(sii902x->iovcc))
> +		return PTR_ERR(sii902x->iovcc);
Consider using dev_err_probe() here.

> +
> +	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
> +	if (IS_ERR(sii902x->cvcc12))
> +		return PTR_ERR(sii902x->cvcc12);
Consider using dev_err_probe() here.
> +
> +	ret = regulator_enable(sii902x->iovcc);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to enable iovcc supply: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regulator_enable(sii902x->cvcc12);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to enable cvcc12 supply: %d\n", ret);
> +		regulator_disable(sii902x->iovcc);
> +		return PTR_ERR(sii902x->cvcc12);
> +	}
> +
> +	ret = sii902x_init(sii902x);
> +	if (ret < 0) {
> +		regulator_disable(sii902x->cvcc12);
> +		regulator_disable(sii902x->iovcc);
> +	}
> +
> +	return ret;
> +}
> +
> +static int sii902x_init(struct sii902x *sii902x)
> +{
> +	struct device *dev = &sii902x->i2c->dev;
> +	unsigned int status = 0;
> +	u8 chipid[4];
> +	int ret;
> +
>  	sii902x_reset(sii902x);
>  
>  	ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
> @@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
>  	regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
>  	regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
>  
> -	if (client->irq > 0) {
> +	if (sii902x->i2c->irq > 0) {
>  		regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
>  			     SII902X_HOTPLUG_EVENT);
>  
> -		ret = devm_request_threaded_irq(dev, client->irq, NULL,
> +		ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,
>  						sii902x_interrupt,
>  						IRQF_ONESHOT, dev_name(dev),
>  						sii902x);
> @@ -1031,9 +1071,9 @@ static int sii902x_probe(struct i2c_client *client,
>  
>  	sii902x_audio_codec_init(sii902x, dev);
>  
> -	i2c_set_clientdata(client, sii902x);
> +	i2c_set_clientdata(sii902x->i2c, sii902x);
>  
> -	sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
> +	sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,
>  					1, 0, I2C_MUX_GATE,
>  					sii902x_i2c_bypass_select,
>  					sii902x_i2c_bypass_deselect);
> @@ -1051,6 +1091,8 @@ static int sii902x_remove(struct i2c_client *client)
>  
>  	i2c_mux_del_adapters(sii902x->i2cmux);
>  	drm_bridge_remove(&sii902x->bridge);
> +	regulator_disable(sii902x->cvcc12);
> +	regulator_disable(sii902x->iovcc);
>  
>  	return 0;
>  }
> -- 
> 2.25.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* [PATCH v2 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
  2020-09-24 20:05 ` Alexandru Gagniuc
@ 2020-09-28 17:30   ` Alexandru Gagniuc
  -1 siblings, 0 replies; 43+ messages in thread
From: Alexandru Gagniuc @ 2020-09-28 17:30 UTC (permalink / raw)
  To: devicetree, dri-devel
  Cc: Alexandru Gagniuc, David Airlie, Daniel Vetter, Rob Herring,
	Andrzej Hajda, Neil Armstrong, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, Thomas Bogendoerfer, Mark Brown,
	Mauro Carvalho Chehab, open list

On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
voltage before the reset sequence is initiated. On most boards, this
assumption is true at boot-up, so initialization succeeds.

However, when we try to initialize the chip with incorrect supply
voltages, it will not respond to I2C requests. sii902x_probe() fails
with -ENXIO.

To resolve this, look for the "iovcc" and "cvcc12" regulators, and
make sure they are enabled before starting the reset sequence. If
these supplies are not available in devicetree, then they will default
to dummy-regulator. In that case everything will work like before.

This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
On this board, the supplies would be set by the second stage
bootloader, which does not run in falcon mode.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
Changes since v1:
  * Fix return code after regulator_enable(sii902x->iovcc) fails (Fabio Estevam)
  * Use dev_err_probe() instead of dev_err() where appropriate (Sam Ravnborg)

 drivers/gpu/drm/bridge/sii902x.c | 54 ++++++++++++++++++++++++++++----
 1 file changed, 48 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 33fd33f953ec..d15e9f2c0d8a 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -17,6 +17,7 @@
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/clk.h>
 
 #include <drm/drm_atomic_helper.h>
@@ -168,6 +169,8 @@ struct sii902x {
 	struct drm_connector connector;
 	struct gpio_desc *reset_gpio;
 	struct i2c_mux_core *i2cmux;
+	struct regulator *iovcc;
+	struct regulator *cvcc12;
 	/*
 	 * Mutex protects audio and video functions from interfering
 	 * each other, by keeping their i2c command sequences atomic.
@@ -954,13 +957,13 @@ static const struct drm_bridge_timings default_sii902x_timings = {
 		 | DRM_BUS_FLAG_DE_HIGH,
 };
 
+static int sii902x_init(struct sii902x *sii902x);
+
 static int sii902x_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct device *dev = &client->dev;
-	unsigned int status = 0;
 	struct sii902x *sii902x;
-	u8 chipid[4];
 	int ret;
 
 	ret = i2c_check_functionality(client->adapter,
@@ -989,6 +992,43 @@ static int sii902x_probe(struct i2c_client *client,
 
 	mutex_init(&sii902x->mutex);
 
+	sii902x->iovcc = devm_regulator_get(dev, "iovcc");
+	if (IS_ERR(sii902x->iovcc))
+		return PTR_ERR(sii902x->iovcc);
+
+	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
+	if (IS_ERR(sii902x->cvcc12))
+		return PTR_ERR(sii902x->cvcc12);
+
+	ret = regulator_enable(sii902x->iovcc);
+	if (ret < 0) {
+		dev_err_probe(dev, ret, "Failed to enable iovcc supply");
+		return ret;
+	}
+
+	ret = regulator_enable(sii902x->cvcc12);
+	if (ret < 0) {
+		dev_err_probe(dev, ret, "Failed to enable cvcc12 supply");
+		regulator_disable(sii902x->iovcc);
+		return ret;
+	}
+
+	ret = sii902x_init(sii902x);
+	if (ret < 0) {
+		regulator_disable(sii902x->cvcc12);
+		regulator_disable(sii902x->iovcc);
+	}
+
+	return ret;
+}
+
+static int sii902x_init(struct sii902x *sii902x)
+{
+	struct device *dev = &sii902x->i2c->dev;
+	unsigned int status = 0;
+	u8 chipid[4];
+	int ret;
+
 	sii902x_reset(sii902x);
 
 	ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
@@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
 	regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
 	regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
 
-	if (client->irq > 0) {
+	if (sii902x->i2c->irq > 0) {
 		regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
 			     SII902X_HOTPLUG_EVENT);
 
-		ret = devm_request_threaded_irq(dev, client->irq, NULL,
+		ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,
 						sii902x_interrupt,
 						IRQF_ONESHOT, dev_name(dev),
 						sii902x);
@@ -1031,9 +1071,9 @@ static int sii902x_probe(struct i2c_client *client,
 
 	sii902x_audio_codec_init(sii902x, dev);
 
-	i2c_set_clientdata(client, sii902x);
+	i2c_set_clientdata(sii902x->i2c, sii902x);
 
-	sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
+	sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,
 					1, 0, I2C_MUX_GATE,
 					sii902x_i2c_bypass_select,
 					sii902x_i2c_bypass_deselect);
@@ -1051,6 +1091,8 @@ static int sii902x_remove(struct i2c_client *client)
 
 	i2c_mux_del_adapters(sii902x->i2cmux);
 	drm_bridge_remove(&sii902x->bridge);
+	regulator_disable(sii902x->cvcc12);
+	regulator_disable(sii902x->iovcc);
 
 	return 0;
 }
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-09-28 17:30   ` Alexandru Gagniuc
  0 siblings, 0 replies; 43+ messages in thread
From: Alexandru Gagniuc @ 2020-09-28 17:30 UTC (permalink / raw)
  To: devicetree, dri-devel
  Cc: Jernej Skrabec, Thomas Bogendoerfer, Neil Armstrong,
	David Airlie, Mark Brown, Jonas Karlman, open list,
	Andrzej Hajda, Rob Herring, Alexandru Gagniuc,
	Mauro Carvalho Chehab, Laurent Pinchart

On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
voltage before the reset sequence is initiated. On most boards, this
assumption is true at boot-up, so initialization succeeds.

However, when we try to initialize the chip with incorrect supply
voltages, it will not respond to I2C requests. sii902x_probe() fails
with -ENXIO.

To resolve this, look for the "iovcc" and "cvcc12" regulators, and
make sure they are enabled before starting the reset sequence. If
these supplies are not available in devicetree, then they will default
to dummy-regulator. In that case everything will work like before.

This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
On this board, the supplies would be set by the second stage
bootloader, which does not run in falcon mode.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
Changes since v1:
  * Fix return code after regulator_enable(sii902x->iovcc) fails (Fabio Estevam)
  * Use dev_err_probe() instead of dev_err() where appropriate (Sam Ravnborg)

 drivers/gpu/drm/bridge/sii902x.c | 54 ++++++++++++++++++++++++++++----
 1 file changed, 48 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 33fd33f953ec..d15e9f2c0d8a 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -17,6 +17,7 @@
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/clk.h>
 
 #include <drm/drm_atomic_helper.h>
@@ -168,6 +169,8 @@ struct sii902x {
 	struct drm_connector connector;
 	struct gpio_desc *reset_gpio;
 	struct i2c_mux_core *i2cmux;
+	struct regulator *iovcc;
+	struct regulator *cvcc12;
 	/*
 	 * Mutex protects audio and video functions from interfering
 	 * each other, by keeping their i2c command sequences atomic.
@@ -954,13 +957,13 @@ static const struct drm_bridge_timings default_sii902x_timings = {
 		 | DRM_BUS_FLAG_DE_HIGH,
 };
 
+static int sii902x_init(struct sii902x *sii902x);
+
 static int sii902x_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
 	struct device *dev = &client->dev;
-	unsigned int status = 0;
 	struct sii902x *sii902x;
-	u8 chipid[4];
 	int ret;
 
 	ret = i2c_check_functionality(client->adapter,
@@ -989,6 +992,43 @@ static int sii902x_probe(struct i2c_client *client,
 
 	mutex_init(&sii902x->mutex);
 
+	sii902x->iovcc = devm_regulator_get(dev, "iovcc");
+	if (IS_ERR(sii902x->iovcc))
+		return PTR_ERR(sii902x->iovcc);
+
+	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
+	if (IS_ERR(sii902x->cvcc12))
+		return PTR_ERR(sii902x->cvcc12);
+
+	ret = regulator_enable(sii902x->iovcc);
+	if (ret < 0) {
+		dev_err_probe(dev, ret, "Failed to enable iovcc supply");
+		return ret;
+	}
+
+	ret = regulator_enable(sii902x->cvcc12);
+	if (ret < 0) {
+		dev_err_probe(dev, ret, "Failed to enable cvcc12 supply");
+		regulator_disable(sii902x->iovcc);
+		return ret;
+	}
+
+	ret = sii902x_init(sii902x);
+	if (ret < 0) {
+		regulator_disable(sii902x->cvcc12);
+		regulator_disable(sii902x->iovcc);
+	}
+
+	return ret;
+}
+
+static int sii902x_init(struct sii902x *sii902x)
+{
+	struct device *dev = &sii902x->i2c->dev;
+	unsigned int status = 0;
+	u8 chipid[4];
+	int ret;
+
 	sii902x_reset(sii902x);
 
 	ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
@@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
 	regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
 	regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
 
-	if (client->irq > 0) {
+	if (sii902x->i2c->irq > 0) {
 		regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
 			     SII902X_HOTPLUG_EVENT);
 
-		ret = devm_request_threaded_irq(dev, client->irq, NULL,
+		ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,
 						sii902x_interrupt,
 						IRQF_ONESHOT, dev_name(dev),
 						sii902x);
@@ -1031,9 +1071,9 @@ static int sii902x_probe(struct i2c_client *client,
 
 	sii902x_audio_codec_init(sii902x, dev);
 
-	i2c_set_clientdata(client, sii902x);
+	i2c_set_clientdata(sii902x->i2c, sii902x);
 
-	sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
+	sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,
 					1, 0, I2C_MUX_GATE,
 					sii902x_i2c_bypass_select,
 					sii902x_i2c_bypass_deselect);
@@ -1051,6 +1091,8 @@ static int sii902x_remove(struct i2c_client *client)
 
 	i2c_mux_del_adapters(sii902x->i2cmux);
 	drm_bridge_remove(&sii902x->bridge);
+	regulator_disable(sii902x->cvcc12);
+	regulator_disable(sii902x->iovcc);
 
 	return 0;
 }
-- 
2.25.4

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

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 2/2] dt-bindings: display: sii902x: Add supply bindings
  2020-09-28 17:30   ` Alexandru Gagniuc
@ 2020-09-28 17:30     ` Alexandru Gagniuc
  -1 siblings, 0 replies; 43+ messages in thread
From: Alexandru Gagniuc @ 2020-09-28 17:30 UTC (permalink / raw)
  To: devicetree, dri-devel
  Cc: Alexandru Gagniuc, David Airlie, Daniel Vetter, Rob Herring,
	Andrzej Hajda, Neil Armstrong, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, Mauro Carvalho Chehab, Thomas Bogendoerfer,
	Mark Brown, open list

The sii902x chip family requires IO and core voltages to reach the
correct voltage before chip initialization. Add binding for describing
the two supplies.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
Changes since v1:
  * Nothing. version incremented to stay in sync with sii902x regulator patch
  
 Documentation/devicetree/bindings/display/bridge/sii902x.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
index 0d1db3f9da84..02c21b584741 100644
--- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
+++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
@@ -8,6 +8,8 @@ Optional properties:
 	- interrupts: describe the interrupt line used to inform the host
 	  about hotplug events.
 	- reset-gpios: OF device-tree gpio specification for RST_N pin.
+	- iovcc-supply: I/O Supply Voltage (1.8V or 3.3V)
+	- cvcc12-supply: Digital Core Supply Voltage (1.2V)
 
 	HDMI audio properties:
 	- #sound-dai-cells: <0> or <1>. <0> if only i2s or spdif pin
@@ -54,6 +56,8 @@ Example:
 		compatible = "sil,sii9022";
 		reg = <0x39>;
 		reset-gpios = <&pioA 1 0>;
+		iovcc-supply = <&v3v3_hdmi>;
+		cvcc12-supply = <&v1v2_hdmi>;
 
 		#sound-dai-cells = <0>;
 		sil,i2s-data-lanes = < 0 1 2 >;
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 2/2] dt-bindings: display: sii902x: Add supply bindings
@ 2020-09-28 17:30     ` Alexandru Gagniuc
  0 siblings, 0 replies; 43+ messages in thread
From: Alexandru Gagniuc @ 2020-09-28 17:30 UTC (permalink / raw)
  To: devicetree, dri-devel
  Cc: Jernej Skrabec, Mauro Carvalho Chehab, Neil Armstrong,
	David Airlie, Mark Brown, Jonas Karlman, open list,
	Andrzej Hajda, Rob Herring, Alexandru Gagniuc,
	Thomas Bogendoerfer, Laurent Pinchart

The sii902x chip family requires IO and core voltages to reach the
correct voltage before chip initialization. Add binding for describing
the two supplies.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
Changes since v1:
  * Nothing. version incremented to stay in sync with sii902x regulator patch
  
 Documentation/devicetree/bindings/display/bridge/sii902x.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
index 0d1db3f9da84..02c21b584741 100644
--- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
+++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
@@ -8,6 +8,8 @@ Optional properties:
 	- interrupts: describe the interrupt line used to inform the host
 	  about hotplug events.
 	- reset-gpios: OF device-tree gpio specification for RST_N pin.
+	- iovcc-supply: I/O Supply Voltage (1.8V or 3.3V)
+	- cvcc12-supply: Digital Core Supply Voltage (1.2V)
 
 	HDMI audio properties:
 	- #sound-dai-cells: <0> or <1>. <0> if only i2s or spdif pin
@@ -54,6 +56,8 @@ Example:
 		compatible = "sil,sii9022";
 		reg = <0x39>;
 		reset-gpios = <&pioA 1 0>;
+		iovcc-supply = <&v3v3_hdmi>;
+		cvcc12-supply = <&v1v2_hdmi>;
 
 		#sound-dai-cells = <0>;
 		sil,i2s-data-lanes = < 0 1 2 >;
-- 
2.25.4

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

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* Re: [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
  2020-09-26 18:49   ` Sam Ravnborg
@ 2020-09-28 17:35     ` Alex G.
  -1 siblings, 0 replies; 43+ messages in thread
From: Alex G. @ 2020-09-28 17:35 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: dri-devel, devicetree, Jernej Skrabec, Mauro Carvalho Chehab,
	Neil Armstrong, David Airlie, Mark Brown, Jonas Karlman,
	linux-kernel, Andrzej Hajda, Rob Herring, Thomas Bogendoerfer,
	Laurent Pinchart

On 9/26/20 1:49 PM, Sam Ravnborg wrote:
> Hi Alexandru
> 
> On Thu, Sep 24, 2020 at 03:05:05PM -0500, Alexandru Gagniuc wrote:
>> On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
>> voltage before the reset sequence is initiated. On most boards, this
>> assumption is true at boot-up, so initialization succeeds.
>>
>> However, when we try to initialize the chip with incorrect supply
>> voltages, it will not respond to I2C requests. sii902x_probe() fails
>> with -ENXIO.
>>
>> To resolve this, look for the "iovcc" and "cvcc12" regulators, and
>> make sure they are enabled before starting the reset sequence. If
>> these supplies are not available in devicetree, then they will default
>> to dummy-regulator. In that case everything will work like before.
>>
>> This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
>> On this board, the supplies would be set by the second stage
>> bootloader, which does not run in falcon mode.
>>
>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> 
> One nitpick here. The binding should be present in the tree before
> you start using it. So this patch should be applied after the binding.

It is :)
   * arch/arm/boot/dts/stm32mp15xx-dkx.dtsi

Alex

> One detail below - I think others have already commented on the rest.
[snip]

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-09-28 17:35     ` Alex G.
  0 siblings, 0 replies; 43+ messages in thread
From: Alex G. @ 2020-09-28 17:35 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: devicetree, Jernej Skrabec, Thomas Bogendoerfer, Rob Herring,
	Neil Armstrong, Mauro Carvalho Chehab, Jonas Karlman,
	linux-kernel, dri-devel, Andrzej Hajda, David Airlie, Mark Brown,
	Laurent Pinchart

On 9/26/20 1:49 PM, Sam Ravnborg wrote:
> Hi Alexandru
> 
> On Thu, Sep 24, 2020 at 03:05:05PM -0500, Alexandru Gagniuc wrote:
>> On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
>> voltage before the reset sequence is initiated. On most boards, this
>> assumption is true at boot-up, so initialization succeeds.
>>
>> However, when we try to initialize the chip with incorrect supply
>> voltages, it will not respond to I2C requests. sii902x_probe() fails
>> with -ENXIO.
>>
>> To resolve this, look for the "iovcc" and "cvcc12" regulators, and
>> make sure they are enabled before starting the reset sequence. If
>> these supplies are not available in devicetree, then they will default
>> to dummy-regulator. In that case everything will work like before.
>>
>> This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
>> On this board, the supplies would be set by the second stage
>> bootloader, which does not run in falcon mode.
>>
>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> 
> One nitpick here. The binding should be present in the tree before
> you start using it. So this patch should be applied after the binding.

It is :)
   * arch/arm/boot/dts/stm32mp15xx-dkx.dtsi

Alex

> One detail below - I think others have already commented on the rest.
[snip]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
  2020-09-28 17:35     ` Alex G.
@ 2020-09-28 19:03       ` Sam Ravnborg
  -1 siblings, 0 replies; 43+ messages in thread
From: Sam Ravnborg @ 2020-09-28 19:03 UTC (permalink / raw)
  To: Alex G.
  Cc: dri-devel, devicetree, Jernej Skrabec, Mauro Carvalho Chehab,
	Neil Armstrong, David Airlie, Mark Brown, Jonas Karlman,
	linux-kernel, Andrzej Hajda, Rob Herring, Thomas Bogendoerfer,
	Laurent Pinchart

Hi Alex.

On Mon, Sep 28, 2020 at 12:35:01PM -0500, Alex G. wrote:
> On 9/26/20 1:49 PM, Sam Ravnborg wrote:
> > Hi Alexandru
> > 
> > On Thu, Sep 24, 2020 at 03:05:05PM -0500, Alexandru Gagniuc wrote:
> > > On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
> > > voltage before the reset sequence is initiated. On most boards, this
> > > assumption is true at boot-up, so initialization succeeds.
> > > 
> > > However, when we try to initialize the chip with incorrect supply
> > > voltages, it will not respond to I2C requests. sii902x_probe() fails
> > > with -ENXIO.
> > > 
> > > To resolve this, look for the "iovcc" and "cvcc12" regulators, and
> > > make sure they are enabled before starting the reset sequence. If
> > > these supplies are not available in devicetree, then they will default
> > > to dummy-regulator. In that case everything will work like before.
> > > 
> > > This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
> > > On this board, the supplies would be set by the second stage
> > > bootloader, which does not run in falcon mode.
> > > 
> > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > 
> > One nitpick here. The binding should be present in the tree before
> > you start using it. So this patch should be applied after the binding.
> 
> It is :)
>   * arch/arm/boot/dts/stm32mp15xx-dkx.dtsi

This is the device tree. So essentially there is part of the device
tree that is not yet documented - so in a perfect world all parts of the
device tree is documented in bindings
(Documentation/devicetree/bindings/* ) before the device tree is
committed.

	Sam

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-09-28 19:03       ` Sam Ravnborg
  0 siblings, 0 replies; 43+ messages in thread
From: Sam Ravnborg @ 2020-09-28 19:03 UTC (permalink / raw)
  To: Alex G.
  Cc: devicetree, Jernej Skrabec, Thomas Bogendoerfer, Rob Herring,
	Neil Armstrong, Mauro Carvalho Chehab, Jonas Karlman,
	linux-kernel, dri-devel, Andrzej Hajda, David Airlie, Mark Brown,
	Laurent Pinchart

Hi Alex.

On Mon, Sep 28, 2020 at 12:35:01PM -0500, Alex G. wrote:
> On 9/26/20 1:49 PM, Sam Ravnborg wrote:
> > Hi Alexandru
> > 
> > On Thu, Sep 24, 2020 at 03:05:05PM -0500, Alexandru Gagniuc wrote:
> > > On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
> > > voltage before the reset sequence is initiated. On most boards, this
> > > assumption is true at boot-up, so initialization succeeds.
> > > 
> > > However, when we try to initialize the chip with incorrect supply
> > > voltages, it will not respond to I2C requests. sii902x_probe() fails
> > > with -ENXIO.
> > > 
> > > To resolve this, look for the "iovcc" and "cvcc12" regulators, and
> > > make sure they are enabled before starting the reset sequence. If
> > > these supplies are not available in devicetree, then they will default
> > > to dummy-regulator. In that case everything will work like before.
> > > 
> > > This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
> > > On this board, the supplies would be set by the second stage
> > > bootloader, which does not run in falcon mode.
> > > 
> > > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > 
> > One nitpick here. The binding should be present in the tree before
> > you start using it. So this patch should be applied after the binding.
> 
> It is :)
>   * arch/arm/boot/dts/stm32mp15xx-dkx.dtsi

This is the device tree. So essentially there is part of the device
tree that is not yet documented - so in a perfect world all parts of the
device tree is documented in bindings
(Documentation/devicetree/bindings/* ) before the device tree is
committed.

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

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: display: sii902x: Add supply bindings
  2020-09-28 17:30     ` Alexandru Gagniuc
@ 2020-09-29 20:17       ` Rob Herring
  -1 siblings, 0 replies; 43+ messages in thread
From: Rob Herring @ 2020-09-29 20:17 UTC (permalink / raw)
  To: Alexandru Gagniuc
  Cc: Jernej Skrabec, Rob Herring, dri-devel, devicetree,
	Mauro Carvalho Chehab, open list, Thomas Bogendoerfer,
	Andrzej Hajda, Mark Brown, Neil Armstrong, Laurent Pinchart,
	David Airlie, Jonas Karlman

On Mon, 28 Sep 2020 12:30:54 -0500, Alexandru Gagniuc wrote:
> The sii902x chip family requires IO and core voltages to reach the
> correct voltage before chip initialization. Add binding for describing
> the two supplies.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> Changes since v1:
>   * Nothing. version incremented to stay in sync with sii902x regulator patch
> 
>  Documentation/devicetree/bindings/display/bridge/sii902x.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 

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

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 2/2] dt-bindings: display: sii902x: Add supply bindings
@ 2020-09-29 20:17       ` Rob Herring
  0 siblings, 0 replies; 43+ messages in thread
From: Rob Herring @ 2020-09-29 20:17 UTC (permalink / raw)
  To: Alexandru Gagniuc
  Cc: devicetree, Jernej Skrabec, Thomas Bogendoerfer, Neil Armstrong,
	Mauro Carvalho Chehab, Mark Brown, Jonas Karlman, open list,
	dri-devel, Andrzej Hajda, Rob Herring, Laurent Pinchart,
	David Airlie

On Mon, 28 Sep 2020 12:30:54 -0500, Alexandru Gagniuc wrote:
> The sii902x chip family requires IO and core voltages to reach the
> correct voltage before chip initialization. Add binding for describing
> the two supplies.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> Changes since v1:
>   * Nothing. version incremented to stay in sync with sii902x regulator patch
> 
>  Documentation/devicetree/bindings/display/bridge/sii902x.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
  2020-09-28 17:30   ` Alexandru Gagniuc
@ 2020-10-20  1:24     ` Alex G.
  -1 siblings, 0 replies; 43+ messages in thread
From: Alex G. @ 2020-10-20  1:24 UTC (permalink / raw)
  To: devicetree, dri-devel
  Cc: David Airlie, Daniel Vetter, Rob Herring, Andrzej Hajda,
	Neil Armstrong, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	Thomas Bogendoerfer, Mark Brown, Mauro Carvalho Chehab,
	open list

On 9/28/20 12:30 PM, Alexandru Gagniuc wrote:
> On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
> voltage before the reset sequence is initiated. On most boards, this
> assumption is true at boot-up, so initialization succeeds.
> 
> However, when we try to initialize the chip with incorrect supply
> voltages, it will not respond to I2C requests. sii902x_probe() fails
> with -ENXIO.
> 
> To resolve this, look for the "iovcc" and "cvcc12" regulators, and
> make sure they are enabled before starting the reset sequence. If
> these supplies are not available in devicetree, then they will default
> to dummy-regulator. In that case everything will work like before.
> 
> This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
> On this board, the supplies would be set by the second stage
> bootloader, which does not run in falcon mode.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> Changes since v1:
>    * Fix return code after regulator_enable(sii902x->iovcc) fails (Fabio Estevam)
>    * Use dev_err_probe() instead of dev_err() where appropriate (Sam Ravnborg)
> 
>   drivers/gpu/drm/bridge/sii902x.c | 54 ++++++++++++++++++++++++++++----
>   1 file changed, 48 insertions(+), 6 deletions(-)

This patch seems to have entered fall dormancy. Did I miss somebody in 
the CC field?

Alex


> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index 33fd33f953ec..d15e9f2c0d8a 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -17,6 +17,7 @@
>   #include <linux/i2c.h>
>   #include <linux/module.h>
>   #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>   #include <linux/clk.h>
>   
>   #include <drm/drm_atomic_helper.h>
> @@ -168,6 +169,8 @@ struct sii902x {
>   	struct drm_connector connector;
>   	struct gpio_desc *reset_gpio;
>   	struct i2c_mux_core *i2cmux;
> +	struct regulator *iovcc;
> +	struct regulator *cvcc12;
>   	/*
>   	 * Mutex protects audio and video functions from interfering
>   	 * each other, by keeping their i2c command sequences atomic.
> @@ -954,13 +957,13 @@ static const struct drm_bridge_timings default_sii902x_timings = {
>   		 | DRM_BUS_FLAG_DE_HIGH,
>   };
>   
> +static int sii902x_init(struct sii902x *sii902x);
> +
>   static int sii902x_probe(struct i2c_client *client,
>   			 const struct i2c_device_id *id)
>   {
>   	struct device *dev = &client->dev;
> -	unsigned int status = 0;
>   	struct sii902x *sii902x;
> -	u8 chipid[4];
>   	int ret;
>   
>   	ret = i2c_check_functionality(client->adapter,
> @@ -989,6 +992,43 @@ static int sii902x_probe(struct i2c_client *client,
>   
>   	mutex_init(&sii902x->mutex);
>   
> +	sii902x->iovcc = devm_regulator_get(dev, "iovcc");
> +	if (IS_ERR(sii902x->iovcc))
> +		return PTR_ERR(sii902x->iovcc);
> +
> +	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
> +	if (IS_ERR(sii902x->cvcc12))
> +		return PTR_ERR(sii902x->cvcc12);
> +
> +	ret = regulator_enable(sii902x->iovcc);
> +	if (ret < 0) {
> +		dev_err_probe(dev, ret, "Failed to enable iovcc supply");
> +		return ret;
> +	}
> +
> +	ret = regulator_enable(sii902x->cvcc12);
> +	if (ret < 0) {
> +		dev_err_probe(dev, ret, "Failed to enable cvcc12 supply");
> +		regulator_disable(sii902x->iovcc);
> +		return ret;
> +	}
> +
> +	ret = sii902x_init(sii902x);
> +	if (ret < 0) {
> +		regulator_disable(sii902x->cvcc12);
> +		regulator_disable(sii902x->iovcc);
> +	}
> +
> +	return ret;
> +}
> +
> +static int sii902x_init(struct sii902x *sii902x)
> +{
> +	struct device *dev = &sii902x->i2c->dev;
> +	unsigned int status = 0;
> +	u8 chipid[4];
> +	int ret;
> +
>   	sii902x_reset(sii902x);
>   
>   	ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
> @@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
>   	regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
>   	regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
>   
> -	if (client->irq > 0) {
> +	if (sii902x->i2c->irq > 0) {
>   		regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
>   			     SII902X_HOTPLUG_EVENT);
>   
> -		ret = devm_request_threaded_irq(dev, client->irq, NULL,
> +		ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,
>   						sii902x_interrupt,
>   						IRQF_ONESHOT, dev_name(dev),
>   						sii902x);
> @@ -1031,9 +1071,9 @@ static int sii902x_probe(struct i2c_client *client,
>   
>   	sii902x_audio_codec_init(sii902x, dev);
>   
> -	i2c_set_clientdata(client, sii902x);
> +	i2c_set_clientdata(sii902x->i2c, sii902x);
>   
> -	sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
> +	sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,
>   					1, 0, I2C_MUX_GATE,
>   					sii902x_i2c_bypass_select,
>   					sii902x_i2c_bypass_deselect);
> @@ -1051,6 +1091,8 @@ static int sii902x_remove(struct i2c_client *client)
>   
>   	i2c_mux_del_adapters(sii902x->i2cmux);
>   	drm_bridge_remove(&sii902x->bridge);
> +	regulator_disable(sii902x->cvcc12);
> +	regulator_disable(sii902x->iovcc);
>   
>   	return 0;
>   }
> 

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-10-20  1:24     ` Alex G.
  0 siblings, 0 replies; 43+ messages in thread
From: Alex G. @ 2020-10-20  1:24 UTC (permalink / raw)
  To: devicetree, dri-devel
  Cc: Jernej Skrabec, Thomas Bogendoerfer, Neil Armstrong,
	David Airlie, Mark Brown, Jonas Karlman, open list,
	Andrzej Hajda, Rob Herring, Laurent Pinchart,
	Mauro Carvalho Chehab

On 9/28/20 12:30 PM, Alexandru Gagniuc wrote:
> On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
> voltage before the reset sequence is initiated. On most boards, this
> assumption is true at boot-up, so initialization succeeds.
> 
> However, when we try to initialize the chip with incorrect supply
> voltages, it will not respond to I2C requests. sii902x_probe() fails
> with -ENXIO.
> 
> To resolve this, look for the "iovcc" and "cvcc12" regulators, and
> make sure they are enabled before starting the reset sequence. If
> these supplies are not available in devicetree, then they will default
> to dummy-regulator. In that case everything will work like before.
> 
> This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
> On this board, the supplies would be set by the second stage
> bootloader, which does not run in falcon mode.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
> Changes since v1:
>    * Fix return code after regulator_enable(sii902x->iovcc) fails (Fabio Estevam)
>    * Use dev_err_probe() instead of dev_err() where appropriate (Sam Ravnborg)
> 
>   drivers/gpu/drm/bridge/sii902x.c | 54 ++++++++++++++++++++++++++++----
>   1 file changed, 48 insertions(+), 6 deletions(-)

This patch seems to have entered fall dormancy. Did I miss somebody in 
the CC field?

Alex


> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index 33fd33f953ec..d15e9f2c0d8a 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -17,6 +17,7 @@
>   #include <linux/i2c.h>
>   #include <linux/module.h>
>   #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>   #include <linux/clk.h>
>   
>   #include <drm/drm_atomic_helper.h>
> @@ -168,6 +169,8 @@ struct sii902x {
>   	struct drm_connector connector;
>   	struct gpio_desc *reset_gpio;
>   	struct i2c_mux_core *i2cmux;
> +	struct regulator *iovcc;
> +	struct regulator *cvcc12;
>   	/*
>   	 * Mutex protects audio and video functions from interfering
>   	 * each other, by keeping their i2c command sequences atomic.
> @@ -954,13 +957,13 @@ static const struct drm_bridge_timings default_sii902x_timings = {
>   		 | DRM_BUS_FLAG_DE_HIGH,
>   };
>   
> +static int sii902x_init(struct sii902x *sii902x);
> +
>   static int sii902x_probe(struct i2c_client *client,
>   			 const struct i2c_device_id *id)
>   {
>   	struct device *dev = &client->dev;
> -	unsigned int status = 0;
>   	struct sii902x *sii902x;
> -	u8 chipid[4];
>   	int ret;
>   
>   	ret = i2c_check_functionality(client->adapter,
> @@ -989,6 +992,43 @@ static int sii902x_probe(struct i2c_client *client,
>   
>   	mutex_init(&sii902x->mutex);
>   
> +	sii902x->iovcc = devm_regulator_get(dev, "iovcc");
> +	if (IS_ERR(sii902x->iovcc))
> +		return PTR_ERR(sii902x->iovcc);
> +
> +	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
> +	if (IS_ERR(sii902x->cvcc12))
> +		return PTR_ERR(sii902x->cvcc12);
> +
> +	ret = regulator_enable(sii902x->iovcc);
> +	if (ret < 0) {
> +		dev_err_probe(dev, ret, "Failed to enable iovcc supply");
> +		return ret;
> +	}
> +
> +	ret = regulator_enable(sii902x->cvcc12);
> +	if (ret < 0) {
> +		dev_err_probe(dev, ret, "Failed to enable cvcc12 supply");
> +		regulator_disable(sii902x->iovcc);
> +		return ret;
> +	}
> +
> +	ret = sii902x_init(sii902x);
> +	if (ret < 0) {
> +		regulator_disable(sii902x->cvcc12);
> +		regulator_disable(sii902x->iovcc);
> +	}
> +
> +	return ret;
> +}
> +
> +static int sii902x_init(struct sii902x *sii902x)
> +{
> +	struct device *dev = &sii902x->i2c->dev;
> +	unsigned int status = 0;
> +	u8 chipid[4];
> +	int ret;
> +
>   	sii902x_reset(sii902x);
>   
>   	ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
> @@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
>   	regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
>   	regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
>   
> -	if (client->irq > 0) {
> +	if (sii902x->i2c->irq > 0) {
>   		regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
>   			     SII902X_HOTPLUG_EVENT);
>   
> -		ret = devm_request_threaded_irq(dev, client->irq, NULL,
> +		ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,
>   						sii902x_interrupt,
>   						IRQF_ONESHOT, dev_name(dev),
>   						sii902x);
> @@ -1031,9 +1071,9 @@ static int sii902x_probe(struct i2c_client *client,
>   
>   	sii902x_audio_codec_init(sii902x, dev);
>   
> -	i2c_set_clientdata(client, sii902x);
> +	i2c_set_clientdata(sii902x->i2c, sii902x);
>   
> -	sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
> +	sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,
>   					1, 0, I2C_MUX_GATE,
>   					sii902x_i2c_bypass_select,
>   					sii902x_i2c_bypass_deselect);
> @@ -1051,6 +1091,8 @@ static int sii902x_remove(struct i2c_client *client)
>   
>   	i2c_mux_del_adapters(sii902x->i2cmux);
>   	drm_bridge_remove(&sii902x->bridge);
> +	regulator_disable(sii902x->cvcc12);
> +	regulator_disable(sii902x->iovcc);
>   
>   	return 0;
>   }
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
  2020-10-20  1:24     ` Alex G.
@ 2020-10-20  7:16       ` Sam Ravnborg
  -1 siblings, 0 replies; 43+ messages in thread
From: Sam Ravnborg @ 2020-10-20  7:16 UTC (permalink / raw)
  To: Alex G.
  Cc: devicetree, dri-devel, Jernej Skrabec, Thomas Bogendoerfer,
	Neil Armstrong, David Airlie, Mark Brown, Jonas Karlman,
	open list, Andrzej Hajda, Rob Herring, Laurent Pinchart,
	Mauro Carvalho Chehab

Hi Alex.

On Mon, Oct 19, 2020 at 08:24:40PM -0500, Alex G. wrote:
> On 9/28/20 12:30 PM, Alexandru Gagniuc wrote:
> > On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
> > voltage before the reset sequence is initiated. On most boards, this
> > assumption is true at boot-up, so initialization succeeds.
> > 
> > However, when we try to initialize the chip with incorrect supply
> > voltages, it will not respond to I2C requests. sii902x_probe() fails
> > with -ENXIO.
> > 
> > To resolve this, look for the "iovcc" and "cvcc12" regulators, and
> > make sure they are enabled before starting the reset sequence. If
> > these supplies are not available in devicetree, then they will default
> > to dummy-regulator. In that case everything will work like before.
> > 
> > This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
> > On this board, the supplies would be set by the second stage
> > bootloader, which does not run in falcon mode.
> > 
> > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > ---
> > Changes since v1:
> >    * Fix return code after regulator_enable(sii902x->iovcc) fails (Fabio Estevam)
> >    * Use dev_err_probe() instead of dev_err() where appropriate (Sam Ravnborg)
> > 
> >   drivers/gpu/drm/bridge/sii902x.c | 54 ++++++++++++++++++++++++++++----
> >   1 file changed, 48 insertions(+), 6 deletions(-)
> 
> This patch seems to have entered fall dormancy. Did I miss somebody in the
> CC field?

I have lost the original mail/patch.
Can you resend with one fix - see below.

	Sam

> 
> Alex
> 
> 
> > diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> > index 33fd33f953ec..d15e9f2c0d8a 100644
> > --- a/drivers/gpu/drm/bridge/sii902x.c
> > +++ b/drivers/gpu/drm/bridge/sii902x.c
> > @@ -17,6 +17,7 @@
> >   #include <linux/i2c.h>
> >   #include <linux/module.h>
> >   #include <linux/regmap.h>
> > +#include <linux/regulator/consumer.h>
> >   #include <linux/clk.h>
> >   #include <drm/drm_atomic_helper.h>
> > @@ -168,6 +169,8 @@ struct sii902x {
> >   	struct drm_connector connector;
> >   	struct gpio_desc *reset_gpio;
> >   	struct i2c_mux_core *i2cmux;
> > +	struct regulator *iovcc;
> > +	struct regulator *cvcc12;
> >   	/*
> >   	 * Mutex protects audio and video functions from interfering
> >   	 * each other, by keeping their i2c command sequences atomic.
> > @@ -954,13 +957,13 @@ static const struct drm_bridge_timings default_sii902x_timings = {
> >   		 | DRM_BUS_FLAG_DE_HIGH,
> >   };
> > +static int sii902x_init(struct sii902x *sii902x);
Please re-arrange the code so this prototype is not needed.

> > +
> >   static int sii902x_probe(struct i2c_client *client,
> >   			 const struct i2c_device_id *id)
> >   {
> >   	struct device *dev = &client->dev;
> > -	unsigned int status = 0;
> >   	struct sii902x *sii902x;
> > -	u8 chipid[4];
> >   	int ret;
> >   	ret = i2c_check_functionality(client->adapter,
> > @@ -989,6 +992,43 @@ static int sii902x_probe(struct i2c_client *client,
> >   	mutex_init(&sii902x->mutex);
> > +	sii902x->iovcc = devm_regulator_get(dev, "iovcc");
> > +	if (IS_ERR(sii902x->iovcc))
> > +		return PTR_ERR(sii902x->iovcc);
> > +
> > +	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
> > +	if (IS_ERR(sii902x->cvcc12))
> > +		return PTR_ERR(sii902x->cvcc12);
> > +
> > +	ret = regulator_enable(sii902x->iovcc);
> > +	if (ret < 0) {
> > +		dev_err_probe(dev, ret, "Failed to enable iovcc supply");
> > +		return ret;
> > +	}
> > +
> > +	ret = regulator_enable(sii902x->cvcc12);
> > +	if (ret < 0) {
> > +		dev_err_probe(dev, ret, "Failed to enable cvcc12 supply");
> > +		regulator_disable(sii902x->iovcc);
> > +		return ret;
> > +	}
> > +
> > +	ret = sii902x_init(sii902x);
> > +	if (ret < 0) {
> > +		regulator_disable(sii902x->cvcc12);
> > +		regulator_disable(sii902x->iovcc);
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static int sii902x_init(struct sii902x *sii902x)
> > +{
> > +	struct device *dev = &sii902x->i2c->dev;
> > +	unsigned int status = 0;
> > +	u8 chipid[4];
> > +	int ret;
> > +
> >   	sii902x_reset(sii902x);
> >   	ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
> > @@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
> >   	regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
> >   	regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
> > -	if (client->irq > 0) {
> > +	if (sii902x->i2c->irq > 0) {
> >   		regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
> >   			     SII902X_HOTPLUG_EVENT);
> > -		ret = devm_request_threaded_irq(dev, client->irq, NULL,
> > +		ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,
> >   						sii902x_interrupt,
> >   						IRQF_ONESHOT, dev_name(dev),
> >   						sii902x);
> > @@ -1031,9 +1071,9 @@ static int sii902x_probe(struct i2c_client *client,
> >   	sii902x_audio_codec_init(sii902x, dev);
> > -	i2c_set_clientdata(client, sii902x);
> > +	i2c_set_clientdata(sii902x->i2c, sii902x);
> > -	sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
> > +	sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,
> >   					1, 0, I2C_MUX_GATE,
> >   					sii902x_i2c_bypass_select,
> >   					sii902x_i2c_bypass_deselect);
> > @@ -1051,6 +1091,8 @@ static int sii902x_remove(struct i2c_client *client)
> >   	i2c_mux_del_adapters(sii902x->i2cmux);
> >   	drm_bridge_remove(&sii902x->bridge);
> > +	regulator_disable(sii902x->cvcc12);
> > +	regulator_disable(sii902x->iovcc);
> >   	return 0;
> >   }
> > 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-10-20  7:16       ` Sam Ravnborg
  0 siblings, 0 replies; 43+ messages in thread
From: Sam Ravnborg @ 2020-10-20  7:16 UTC (permalink / raw)
  To: Alex G.
  Cc: devicetree, Jernej Skrabec, Thomas Bogendoerfer, Rob Herring,
	Neil Armstrong, David Airlie, Jonas Karlman, open list,
	dri-devel, Andrzej Hajda, Mark Brown, Laurent Pinchart,
	Mauro Carvalho Chehab

Hi Alex.

On Mon, Oct 19, 2020 at 08:24:40PM -0500, Alex G. wrote:
> On 9/28/20 12:30 PM, Alexandru Gagniuc wrote:
> > On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
> > voltage before the reset sequence is initiated. On most boards, this
> > assumption is true at boot-up, so initialization succeeds.
> > 
> > However, when we try to initialize the chip with incorrect supply
> > voltages, it will not respond to I2C requests. sii902x_probe() fails
> > with -ENXIO.
> > 
> > To resolve this, look for the "iovcc" and "cvcc12" regulators, and
> > make sure they are enabled before starting the reset sequence. If
> > these supplies are not available in devicetree, then they will default
> > to dummy-regulator. In that case everything will work like before.
> > 
> > This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
> > On this board, the supplies would be set by the second stage
> > bootloader, which does not run in falcon mode.
> > 
> > Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> > ---
> > Changes since v1:
> >    * Fix return code after regulator_enable(sii902x->iovcc) fails (Fabio Estevam)
> >    * Use dev_err_probe() instead of dev_err() where appropriate (Sam Ravnborg)
> > 
> >   drivers/gpu/drm/bridge/sii902x.c | 54 ++++++++++++++++++++++++++++----
> >   1 file changed, 48 insertions(+), 6 deletions(-)
> 
> This patch seems to have entered fall dormancy. Did I miss somebody in the
> CC field?

I have lost the original mail/patch.
Can you resend with one fix - see below.

	Sam

> 
> Alex
> 
> 
> > diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> > index 33fd33f953ec..d15e9f2c0d8a 100644
> > --- a/drivers/gpu/drm/bridge/sii902x.c
> > +++ b/drivers/gpu/drm/bridge/sii902x.c
> > @@ -17,6 +17,7 @@
> >   #include <linux/i2c.h>
> >   #include <linux/module.h>
> >   #include <linux/regmap.h>
> > +#include <linux/regulator/consumer.h>
> >   #include <linux/clk.h>
> >   #include <drm/drm_atomic_helper.h>
> > @@ -168,6 +169,8 @@ struct sii902x {
> >   	struct drm_connector connector;
> >   	struct gpio_desc *reset_gpio;
> >   	struct i2c_mux_core *i2cmux;
> > +	struct regulator *iovcc;
> > +	struct regulator *cvcc12;
> >   	/*
> >   	 * Mutex protects audio and video functions from interfering
> >   	 * each other, by keeping their i2c command sequences atomic.
> > @@ -954,13 +957,13 @@ static const struct drm_bridge_timings default_sii902x_timings = {
> >   		 | DRM_BUS_FLAG_DE_HIGH,
> >   };
> > +static int sii902x_init(struct sii902x *sii902x);
Please re-arrange the code so this prototype is not needed.

> > +
> >   static int sii902x_probe(struct i2c_client *client,
> >   			 const struct i2c_device_id *id)
> >   {
> >   	struct device *dev = &client->dev;
> > -	unsigned int status = 0;
> >   	struct sii902x *sii902x;
> > -	u8 chipid[4];
> >   	int ret;
> >   	ret = i2c_check_functionality(client->adapter,
> > @@ -989,6 +992,43 @@ static int sii902x_probe(struct i2c_client *client,
> >   	mutex_init(&sii902x->mutex);
> > +	sii902x->iovcc = devm_regulator_get(dev, "iovcc");
> > +	if (IS_ERR(sii902x->iovcc))
> > +		return PTR_ERR(sii902x->iovcc);
> > +
> > +	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
> > +	if (IS_ERR(sii902x->cvcc12))
> > +		return PTR_ERR(sii902x->cvcc12);
> > +
> > +	ret = regulator_enable(sii902x->iovcc);
> > +	if (ret < 0) {
> > +		dev_err_probe(dev, ret, "Failed to enable iovcc supply");
> > +		return ret;
> > +	}
> > +
> > +	ret = regulator_enable(sii902x->cvcc12);
> > +	if (ret < 0) {
> > +		dev_err_probe(dev, ret, "Failed to enable cvcc12 supply");
> > +		regulator_disable(sii902x->iovcc);
> > +		return ret;
> > +	}
> > +
> > +	ret = sii902x_init(sii902x);
> > +	if (ret < 0) {
> > +		regulator_disable(sii902x->cvcc12);
> > +		regulator_disable(sii902x->iovcc);
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static int sii902x_init(struct sii902x *sii902x)
> > +{
> > +	struct device *dev = &sii902x->i2c->dev;
> > +	unsigned int status = 0;
> > +	u8 chipid[4];
> > +	int ret;
> > +
> >   	sii902x_reset(sii902x);
> >   	ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
> > @@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
> >   	regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
> >   	regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
> > -	if (client->irq > 0) {
> > +	if (sii902x->i2c->irq > 0) {
> >   		regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
> >   			     SII902X_HOTPLUG_EVENT);
> > -		ret = devm_request_threaded_irq(dev, client->irq, NULL,
> > +		ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,
> >   						sii902x_interrupt,
> >   						IRQF_ONESHOT, dev_name(dev),
> >   						sii902x);
> > @@ -1031,9 +1071,9 @@ static int sii902x_probe(struct i2c_client *client,
> >   	sii902x_audio_codec_init(sii902x, dev);
> > -	i2c_set_clientdata(client, sii902x);
> > +	i2c_set_clientdata(sii902x->i2c, sii902x);
> > -	sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
> > +	sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,
> >   					1, 0, I2C_MUX_GATE,
> >   					sii902x_i2c_bypass_select,
> >   					sii902x_i2c_bypass_deselect);
> > @@ -1051,6 +1091,8 @@ static int sii902x_remove(struct i2c_client *client)
> >   	i2c_mux_del_adapters(sii902x->i2cmux);
> >   	drm_bridge_remove(&sii902x->bridge);
> > +	regulator_disable(sii902x->cvcc12);
> > +	regulator_disable(sii902x->iovcc);
> >   	return 0;
> >   }
> > 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
  2020-10-20  7:16       ` Sam Ravnborg
@ 2020-10-20 14:01         ` Alex G.
  -1 siblings, 0 replies; 43+ messages in thread
From: Alex G. @ 2020-10-20 14:01 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: devicetree, dri-devel, Jernej Skrabec, Thomas Bogendoerfer,
	Neil Armstrong, David Airlie, Mark Brown, Jonas Karlman,
	open list, Andrzej Hajda, Rob Herring, Laurent Pinchart,
	Mauro Carvalho Chehab



On 10/20/20 2:16 AM, Sam Ravnborg wrote:
> Hi Alex.

[snip]

>>
>>
>>> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
>>> index 33fd33f953ec..d15e9f2c0d8a 100644
>>> --- a/drivers/gpu/drm/bridge/sii902x.c
>>> +++ b/drivers/gpu/drm/bridge/sii902x.c
>>> @@ -17,6 +17,7 @@
>>>    #include <linux/i2c.h>
>>>    #include <linux/module.h>
>>>    #include <linux/regmap.h>
>>> +#include <linux/regulator/consumer.h>
>>>    #include <linux/clk.h>
>>>    #include <drm/drm_atomic_helper.h>
>>> @@ -168,6 +169,8 @@ struct sii902x {
>>>    	struct drm_connector connector;
>>>    	struct gpio_desc *reset_gpio;
>>>    	struct i2c_mux_core *i2cmux;
>>> +	struct regulator *iovcc;
>>> +	struct regulator *cvcc12;
>>>    	/*
>>>    	 * Mutex protects audio and video functions from interfering
>>>    	 * each other, by keeping their i2c command sequences atomic.
>>> @@ -954,13 +957,13 @@ static const struct drm_bridge_timings default_sii902x_timings = {
>>>    		 | DRM_BUS_FLAG_DE_HIGH,
>>>    };
>>> +static int sii902x_init(struct sii902x *sii902x);
> Please re-arrange the code so this prototype is not needed.

I'd be happy to re-arrange things. It will make the diff look a lot 
bigger than what it is. Is that okay?

Alex

>>> +
>>>    static int sii902x_probe(struct i2c_client *client,
>>>    			 const struct i2c_device_id *id)
>>>    {
>>>    	struct device *dev = &client->dev;
>>> -	unsigned int status = 0;
>>>    	struct sii902x *sii902x;
>>> -	u8 chipid[4];
>>>    	int ret;
>>>    	ret = i2c_check_functionality(client->adapter,
>>> @@ -989,6 +992,43 @@ static int sii902x_probe(struct i2c_client *client,
>>>    	mutex_init(&sii902x->mutex);
>>> +	sii902x->iovcc = devm_regulator_get(dev, "iovcc");
>>> +	if (IS_ERR(sii902x->iovcc))
>>> +		return PTR_ERR(sii902x->iovcc);
>>> +
>>> +	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
>>> +	if (IS_ERR(sii902x->cvcc12))
>>> +		return PTR_ERR(sii902x->cvcc12);
>>> +
>>> +	ret = regulator_enable(sii902x->iovcc);
>>> +	if (ret < 0) {
>>> +		dev_err_probe(dev, ret, "Failed to enable iovcc supply");
>>> +		return ret;
>>> +	}
>>> +
>>> +	ret = regulator_enable(sii902x->cvcc12);
>>> +	if (ret < 0) {
>>> +		dev_err_probe(dev, ret, "Failed to enable cvcc12 supply");
>>> +		regulator_disable(sii902x->iovcc);
>>> +		return ret;
>>> +	}
>>> +
>>> +	ret = sii902x_init(sii902x);
>>> +	if (ret < 0) {
>>> +		regulator_disable(sii902x->cvcc12);
>>> +		regulator_disable(sii902x->iovcc);
>>> +	}
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static int sii902x_init(struct sii902x *sii902x)
>>> +{
>>> +	struct device *dev = &sii902x->i2c->dev;
>>> +	unsigned int status = 0;
>>> +	u8 chipid[4];
>>> +	int ret;
>>> +
>>>    	sii902x_reset(sii902x);
>>>    	ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
>>> @@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
>>>    	regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
>>>    	regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
>>> -	if (client->irq > 0) {
>>> +	if (sii902x->i2c->irq > 0) {
>>>    		regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
>>>    			     SII902X_HOTPLUG_EVENT);
>>> -		ret = devm_request_threaded_irq(dev, client->irq, NULL,
>>> +		ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,
>>>    						sii902x_interrupt,
>>>    						IRQF_ONESHOT, dev_name(dev),
>>>    						sii902x);
>>> @@ -1031,9 +1071,9 @@ static int sii902x_probe(struct i2c_client *client,
>>>    	sii902x_audio_codec_init(sii902x, dev);
>>> -	i2c_set_clientdata(client, sii902x);
>>> +	i2c_set_clientdata(sii902x->i2c, sii902x);
>>> -	sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
>>> +	sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,
>>>    					1, 0, I2C_MUX_GATE,
>>>    					sii902x_i2c_bypass_select,
>>>    					sii902x_i2c_bypass_deselect);
>>> @@ -1051,6 +1091,8 @@ static int sii902x_remove(struct i2c_client *client)
>>>    	i2c_mux_del_adapters(sii902x->i2cmux);
>>>    	drm_bridge_remove(&sii902x->bridge);
>>> +	regulator_disable(sii902x->cvcc12);
>>> +	regulator_disable(sii902x->iovcc);
>>>    	return 0;
>>>    }
>>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-10-20 14:01         ` Alex G.
  0 siblings, 0 replies; 43+ messages in thread
From: Alex G. @ 2020-10-20 14:01 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: devicetree, Jernej Skrabec, Thomas Bogendoerfer, Rob Herring,
	Neil Armstrong, David Airlie, Jonas Karlman, open list,
	dri-devel, Andrzej Hajda, Mark Brown, Laurent Pinchart,
	Mauro Carvalho Chehab



On 10/20/20 2:16 AM, Sam Ravnborg wrote:
> Hi Alex.

[snip]

>>
>>
>>> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
>>> index 33fd33f953ec..d15e9f2c0d8a 100644
>>> --- a/drivers/gpu/drm/bridge/sii902x.c
>>> +++ b/drivers/gpu/drm/bridge/sii902x.c
>>> @@ -17,6 +17,7 @@
>>>    #include <linux/i2c.h>
>>>    #include <linux/module.h>
>>>    #include <linux/regmap.h>
>>> +#include <linux/regulator/consumer.h>
>>>    #include <linux/clk.h>
>>>    #include <drm/drm_atomic_helper.h>
>>> @@ -168,6 +169,8 @@ struct sii902x {
>>>    	struct drm_connector connector;
>>>    	struct gpio_desc *reset_gpio;
>>>    	struct i2c_mux_core *i2cmux;
>>> +	struct regulator *iovcc;
>>> +	struct regulator *cvcc12;
>>>    	/*
>>>    	 * Mutex protects audio and video functions from interfering
>>>    	 * each other, by keeping their i2c command sequences atomic.
>>> @@ -954,13 +957,13 @@ static const struct drm_bridge_timings default_sii902x_timings = {
>>>    		 | DRM_BUS_FLAG_DE_HIGH,
>>>    };
>>> +static int sii902x_init(struct sii902x *sii902x);
> Please re-arrange the code so this prototype is not needed.

I'd be happy to re-arrange things. It will make the diff look a lot 
bigger than what it is. Is that okay?

Alex

>>> +
>>>    static int sii902x_probe(struct i2c_client *client,
>>>    			 const struct i2c_device_id *id)
>>>    {
>>>    	struct device *dev = &client->dev;
>>> -	unsigned int status = 0;
>>>    	struct sii902x *sii902x;
>>> -	u8 chipid[4];
>>>    	int ret;
>>>    	ret = i2c_check_functionality(client->adapter,
>>> @@ -989,6 +992,43 @@ static int sii902x_probe(struct i2c_client *client,
>>>    	mutex_init(&sii902x->mutex);
>>> +	sii902x->iovcc = devm_regulator_get(dev, "iovcc");
>>> +	if (IS_ERR(sii902x->iovcc))
>>> +		return PTR_ERR(sii902x->iovcc);
>>> +
>>> +	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
>>> +	if (IS_ERR(sii902x->cvcc12))
>>> +		return PTR_ERR(sii902x->cvcc12);
>>> +
>>> +	ret = regulator_enable(sii902x->iovcc);
>>> +	if (ret < 0) {
>>> +		dev_err_probe(dev, ret, "Failed to enable iovcc supply");
>>> +		return ret;
>>> +	}
>>> +
>>> +	ret = regulator_enable(sii902x->cvcc12);
>>> +	if (ret < 0) {
>>> +		dev_err_probe(dev, ret, "Failed to enable cvcc12 supply");
>>> +		regulator_disable(sii902x->iovcc);
>>> +		return ret;
>>> +	}
>>> +
>>> +	ret = sii902x_init(sii902x);
>>> +	if (ret < 0) {
>>> +		regulator_disable(sii902x->cvcc12);
>>> +		regulator_disable(sii902x->iovcc);
>>> +	}
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static int sii902x_init(struct sii902x *sii902x)
>>> +{
>>> +	struct device *dev = &sii902x->i2c->dev;
>>> +	unsigned int status = 0;
>>> +	u8 chipid[4];
>>> +	int ret;
>>> +
>>>    	sii902x_reset(sii902x);
>>>    	ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
>>> @@ -1012,11 +1052,11 @@ static int sii902x_probe(struct i2c_client *client,
>>>    	regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
>>>    	regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
>>> -	if (client->irq > 0) {
>>> +	if (sii902x->i2c->irq > 0) {
>>>    		regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
>>>    			     SII902X_HOTPLUG_EVENT);
>>> -		ret = devm_request_threaded_irq(dev, client->irq, NULL,
>>> +		ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,
>>>    						sii902x_interrupt,
>>>    						IRQF_ONESHOT, dev_name(dev),
>>>    						sii902x);
>>> @@ -1031,9 +1071,9 @@ static int sii902x_probe(struct i2c_client *client,
>>>    	sii902x_audio_codec_init(sii902x, dev);
>>> -	i2c_set_clientdata(client, sii902x);
>>> +	i2c_set_clientdata(sii902x->i2c, sii902x);
>>> -	sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
>>> +	sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,
>>>    					1, 0, I2C_MUX_GATE,
>>>    					sii902x_i2c_bypass_select,
>>>    					sii902x_i2c_bypass_deselect);
>>> @@ -1051,6 +1091,8 @@ static int sii902x_remove(struct i2c_client *client)
>>>    	i2c_mux_del_adapters(sii902x->i2cmux);
>>>    	drm_bridge_remove(&sii902x->bridge);
>>> +	regulator_disable(sii902x->cvcc12);
>>> +	regulator_disable(sii902x->iovcc);
>>>    	return 0;
>>>    }
>>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
  2020-10-20 14:01         ` Alex G.
@ 2020-10-20 15:08           ` Sam Ravnborg
  -1 siblings, 0 replies; 43+ messages in thread
From: Sam Ravnborg @ 2020-10-20 15:08 UTC (permalink / raw)
  To: Alex G.
  Cc: devicetree, dri-devel, Jernej Skrabec, Thomas Bogendoerfer,
	Neil Armstrong, David Airlie, Mark Brown, Jonas Karlman,
	open list, Andrzej Hajda, Rob Herring, Laurent Pinchart,
	Mauro Carvalho Chehab

Hi Alex.

On Tue, Oct 20, 2020 at 09:01:27AM -0500, Alex G. wrote:
> 
> 
> On 10/20/20 2:16 AM, Sam Ravnborg wrote:
> > Hi Alex.
> 
> [snip]
> 
> > > 
> > > 
> > > > diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> > > > index 33fd33f953ec..d15e9f2c0d8a 100644
> > > > --- a/drivers/gpu/drm/bridge/sii902x.c
> > > > +++ b/drivers/gpu/drm/bridge/sii902x.c
> > > > @@ -17,6 +17,7 @@
> > > >    #include <linux/i2c.h>
> > > >    #include <linux/module.h>
> > > >    #include <linux/regmap.h>
> > > > +#include <linux/regulator/consumer.h>
> > > >    #include <linux/clk.h>
> > > >    #include <drm/drm_atomic_helper.h>
> > > > @@ -168,6 +169,8 @@ struct sii902x {
> > > >    	struct drm_connector connector;
> > > >    	struct gpio_desc *reset_gpio;
> > > >    	struct i2c_mux_core *i2cmux;
> > > > +	struct regulator *iovcc;
> > > > +	struct regulator *cvcc12;
> > > >    	/*
> > > >    	 * Mutex protects audio and video functions from interfering
> > > >    	 * each other, by keeping their i2c command sequences atomic.
> > > > @@ -954,13 +957,13 @@ static const struct drm_bridge_timings default_sii902x_timings = {
> > > >    		 | DRM_BUS_FLAG_DE_HIGH,
> > > >    };
> > > > +static int sii902x_init(struct sii902x *sii902x);
> > Please re-arrange the code so this prototype is not needed.
> 
> I'd be happy to re-arrange things. It will make the diff look a lot bigger
> than what it is. Is that okay?

The best way would be to split it in two patches.
One that is pure code movement and one that does the actula changes.

	Sam

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-10-20 15:08           ` Sam Ravnborg
  0 siblings, 0 replies; 43+ messages in thread
From: Sam Ravnborg @ 2020-10-20 15:08 UTC (permalink / raw)
  To: Alex G.
  Cc: devicetree, Jernej Skrabec, Thomas Bogendoerfer, Rob Herring,
	Neil Armstrong, David Airlie, Jonas Karlman, open list,
	dri-devel, Andrzej Hajda, Mark Brown, Laurent Pinchart,
	Mauro Carvalho Chehab

Hi Alex.

On Tue, Oct 20, 2020 at 09:01:27AM -0500, Alex G. wrote:
> 
> 
> On 10/20/20 2:16 AM, Sam Ravnborg wrote:
> > Hi Alex.
> 
> [snip]
> 
> > > 
> > > 
> > > > diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> > > > index 33fd33f953ec..d15e9f2c0d8a 100644
> > > > --- a/drivers/gpu/drm/bridge/sii902x.c
> > > > +++ b/drivers/gpu/drm/bridge/sii902x.c
> > > > @@ -17,6 +17,7 @@
> > > >    #include <linux/i2c.h>
> > > >    #include <linux/module.h>
> > > >    #include <linux/regmap.h>
> > > > +#include <linux/regulator/consumer.h>
> > > >    #include <linux/clk.h>
> > > >    #include <drm/drm_atomic_helper.h>
> > > > @@ -168,6 +169,8 @@ struct sii902x {
> > > >    	struct drm_connector connector;
> > > >    	struct gpio_desc *reset_gpio;
> > > >    	struct i2c_mux_core *i2cmux;
> > > > +	struct regulator *iovcc;
> > > > +	struct regulator *cvcc12;
> > > >    	/*
> > > >    	 * Mutex protects audio and video functions from interfering
> > > >    	 * each other, by keeping their i2c command sequences atomic.
> > > > @@ -954,13 +957,13 @@ static const struct drm_bridge_timings default_sii902x_timings = {
> > > >    		 | DRM_BUS_FLAG_DE_HIGH,
> > > >    };
> > > > +static int sii902x_init(struct sii902x *sii902x);
> > Please re-arrange the code so this prototype is not needed.
> 
> I'd be happy to re-arrange things. It will make the diff look a lot bigger
> than what it is. Is that okay?

The best way would be to split it in two patches.
One that is pure code movement and one that does the actula changes.

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

^ permalink raw reply	[flat|nested] 43+ messages in thread

* [PATCH v3 1/3] drm/bridge: sii902x: Refactor init code into separate function
  2020-09-28 17:30   ` Alexandru Gagniuc
@ 2020-10-20 22:14     ` Alexandru Gagniuc
  -1 siblings, 0 replies; 43+ messages in thread
From: Alexandru Gagniuc @ 2020-10-20 22:14 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: sam, Alexandru Gagniuc, David Airlie, Daniel Vetter, Rob Herring,
	Andrzej Hajda, Neil Armstrong, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, Thomas Bogendoerfer, Mark Brown,
	Mauro Carvalho Chehab, open list

Separate the hardware initialization code from setting up the data
structures and parsing the device tree. The purpose of this change is
to provide a single exit point and avoid a waterfall of 'goto's in
the subsequent patch.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
Changes since v1/and v2:
  * Separated this from main patch to better show diff

 drivers/gpu/drm/bridge/sii902x.c | 77 ++++++++++++++++++--------------
 1 file changed, 43 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 33fd33f953ec..f78c17f49887 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -954,41 +954,13 @@ static const struct drm_bridge_timings default_sii902x_timings = {
 		 | DRM_BUS_FLAG_DE_HIGH,
 };
 
-static int sii902x_probe(struct i2c_client *client,
-			 const struct i2c_device_id *id)
+static int sii902x_init(struct sii902x *sii902x)
 {
-	struct device *dev = &client->dev;
+	struct device *dev = &sii902x->i2c->dev;
 	unsigned int status = 0;
-	struct sii902x *sii902x;
 	u8 chipid[4];
 	int ret;
 
-	ret = i2c_check_functionality(client->adapter,
-				      I2C_FUNC_SMBUS_BYTE_DATA);
-	if (!ret) {
-		dev_err(dev, "I2C adapter not suitable\n");
-		return -EIO;
-	}
-
-	sii902x = devm_kzalloc(dev, sizeof(*sii902x), GFP_KERNEL);
-	if (!sii902x)
-		return -ENOMEM;
-
-	sii902x->i2c = client;
-	sii902x->regmap = devm_regmap_init_i2c(client, &sii902x_regmap_config);
-	if (IS_ERR(sii902x->regmap))
-		return PTR_ERR(sii902x->regmap);
-
-	sii902x->reset_gpio = devm_gpiod_get_optional(dev, "reset",
-						      GPIOD_OUT_LOW);
-	if (IS_ERR(sii902x->reset_gpio)) {
-		dev_err(dev, "Failed to retrieve/request reset gpio: %ld\n",
-			PTR_ERR(sii902x->reset_gpio));
-		return PTR_ERR(sii902x->reset_gpio);
-	}
-
-	mutex_init(&sii902x->mutex);
-
 	sii902x_reset(sii902x);
 
 	ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
@@ -1012,11 +984,11 @@ static int sii902x_probe(struct i2c_client *client,
 	regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
 	regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
 
-	if (client->irq > 0) {
+	if (sii902x->i2c->irq > 0) {
 		regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
 			     SII902X_HOTPLUG_EVENT);
 
-		ret = devm_request_threaded_irq(dev, client->irq, NULL,
+		ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,
 						sii902x_interrupt,
 						IRQF_ONESHOT, dev_name(dev),
 						sii902x);
@@ -1031,9 +1003,9 @@ static int sii902x_probe(struct i2c_client *client,
 
 	sii902x_audio_codec_init(sii902x, dev);
 
-	i2c_set_clientdata(client, sii902x);
+	i2c_set_clientdata(sii902x->i2c, sii902x);
 
-	sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
+	sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,
 					1, 0, I2C_MUX_GATE,
 					sii902x_i2c_bypass_select,
 					sii902x_i2c_bypass_deselect);
@@ -1044,6 +1016,43 @@ static int sii902x_probe(struct i2c_client *client,
 	return i2c_mux_add_adapter(sii902x->i2cmux, 0, 0, 0);
 }
 
+static int sii902x_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	struct sii902x *sii902x;
+	int ret;
+
+	ret = i2c_check_functionality(client->adapter,
+				      I2C_FUNC_SMBUS_BYTE_DATA);
+	if (!ret) {
+		dev_err(dev, "I2C adapter not suitable\n");
+		return -EIO;
+	}
+
+	sii902x = devm_kzalloc(dev, sizeof(*sii902x), GFP_KERNEL);
+	if (!sii902x)
+		return -ENOMEM;
+
+	sii902x->i2c = client;
+	sii902x->regmap = devm_regmap_init_i2c(client, &sii902x_regmap_config);
+	if (IS_ERR(sii902x->regmap))
+		return PTR_ERR(sii902x->regmap);
+
+	sii902x->reset_gpio = devm_gpiod_get_optional(dev, "reset",
+						      GPIOD_OUT_LOW);
+	if (IS_ERR(sii902x->reset_gpio)) {
+		dev_err(dev, "Failed to retrieve/request reset gpio: %ld\n",
+			PTR_ERR(sii902x->reset_gpio));
+		return PTR_ERR(sii902x->reset_gpio);
+	}
+
+	mutex_init(&sii902x->mutex);
+
+	ret = sii902x_init(sii902x);
+	return ret;
+}
+
 static int sii902x_remove(struct i2c_client *client)
 
 {
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 1/3] drm/bridge: sii902x: Refactor init code into separate function
@ 2020-10-20 22:14     ` Alexandru Gagniuc
  0 siblings, 0 replies; 43+ messages in thread
From: Alexandru Gagniuc @ 2020-10-20 22:14 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: Jernej Skrabec, Thomas Bogendoerfer, Neil Armstrong,
	David Airlie, Mark Brown, Jonas Karlman, open list,
	Andrzej Hajda, Rob Herring, Alexandru Gagniuc, sam,
	Mauro Carvalho Chehab, Laurent Pinchart

Separate the hardware initialization code from setting up the data
structures and parsing the device tree. The purpose of this change is
to provide a single exit point and avoid a waterfall of 'goto's in
the subsequent patch.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
Changes since v1/and v2:
  * Separated this from main patch to better show diff

 drivers/gpu/drm/bridge/sii902x.c | 77 ++++++++++++++++++--------------
 1 file changed, 43 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 33fd33f953ec..f78c17f49887 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -954,41 +954,13 @@ static const struct drm_bridge_timings default_sii902x_timings = {
 		 | DRM_BUS_FLAG_DE_HIGH,
 };
 
-static int sii902x_probe(struct i2c_client *client,
-			 const struct i2c_device_id *id)
+static int sii902x_init(struct sii902x *sii902x)
 {
-	struct device *dev = &client->dev;
+	struct device *dev = &sii902x->i2c->dev;
 	unsigned int status = 0;
-	struct sii902x *sii902x;
 	u8 chipid[4];
 	int ret;
 
-	ret = i2c_check_functionality(client->adapter,
-				      I2C_FUNC_SMBUS_BYTE_DATA);
-	if (!ret) {
-		dev_err(dev, "I2C adapter not suitable\n");
-		return -EIO;
-	}
-
-	sii902x = devm_kzalloc(dev, sizeof(*sii902x), GFP_KERNEL);
-	if (!sii902x)
-		return -ENOMEM;
-
-	sii902x->i2c = client;
-	sii902x->regmap = devm_regmap_init_i2c(client, &sii902x_regmap_config);
-	if (IS_ERR(sii902x->regmap))
-		return PTR_ERR(sii902x->regmap);
-
-	sii902x->reset_gpio = devm_gpiod_get_optional(dev, "reset",
-						      GPIOD_OUT_LOW);
-	if (IS_ERR(sii902x->reset_gpio)) {
-		dev_err(dev, "Failed to retrieve/request reset gpio: %ld\n",
-			PTR_ERR(sii902x->reset_gpio));
-		return PTR_ERR(sii902x->reset_gpio);
-	}
-
-	mutex_init(&sii902x->mutex);
-
 	sii902x_reset(sii902x);
 
 	ret = regmap_write(sii902x->regmap, SII902X_REG_TPI_RQB, 0x0);
@@ -1012,11 +984,11 @@ static int sii902x_probe(struct i2c_client *client,
 	regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
 	regmap_write(sii902x->regmap, SII902X_INT_STATUS, status);
 
-	if (client->irq > 0) {
+	if (sii902x->i2c->irq > 0) {
 		regmap_write(sii902x->regmap, SII902X_INT_ENABLE,
 			     SII902X_HOTPLUG_EVENT);
 
-		ret = devm_request_threaded_irq(dev, client->irq, NULL,
+		ret = devm_request_threaded_irq(dev, sii902x->i2c->irq, NULL,
 						sii902x_interrupt,
 						IRQF_ONESHOT, dev_name(dev),
 						sii902x);
@@ -1031,9 +1003,9 @@ static int sii902x_probe(struct i2c_client *client,
 
 	sii902x_audio_codec_init(sii902x, dev);
 
-	i2c_set_clientdata(client, sii902x);
+	i2c_set_clientdata(sii902x->i2c, sii902x);
 
-	sii902x->i2cmux = i2c_mux_alloc(client->adapter, dev,
+	sii902x->i2cmux = i2c_mux_alloc(sii902x->i2c->adapter, dev,
 					1, 0, I2C_MUX_GATE,
 					sii902x_i2c_bypass_select,
 					sii902x_i2c_bypass_deselect);
@@ -1044,6 +1016,43 @@ static int sii902x_probe(struct i2c_client *client,
 	return i2c_mux_add_adapter(sii902x->i2cmux, 0, 0, 0);
 }
 
+static int sii902x_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	struct sii902x *sii902x;
+	int ret;
+
+	ret = i2c_check_functionality(client->adapter,
+				      I2C_FUNC_SMBUS_BYTE_DATA);
+	if (!ret) {
+		dev_err(dev, "I2C adapter not suitable\n");
+		return -EIO;
+	}
+
+	sii902x = devm_kzalloc(dev, sizeof(*sii902x), GFP_KERNEL);
+	if (!sii902x)
+		return -ENOMEM;
+
+	sii902x->i2c = client;
+	sii902x->regmap = devm_regmap_init_i2c(client, &sii902x_regmap_config);
+	if (IS_ERR(sii902x->regmap))
+		return PTR_ERR(sii902x->regmap);
+
+	sii902x->reset_gpio = devm_gpiod_get_optional(dev, "reset",
+						      GPIOD_OUT_LOW);
+	if (IS_ERR(sii902x->reset_gpio)) {
+		dev_err(dev, "Failed to retrieve/request reset gpio: %ld\n",
+			PTR_ERR(sii902x->reset_gpio));
+		return PTR_ERR(sii902x->reset_gpio);
+	}
+
+	mutex_init(&sii902x->mutex);
+
+	ret = sii902x_init(sii902x);
+	return ret;
+}
+
 static int sii902x_remove(struct i2c_client *client)
 
 {
-- 
2.26.2

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

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 2/3] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
  2020-09-28 17:30   ` Alexandru Gagniuc
@ 2020-10-20 22:14     ` Alexandru Gagniuc
  -1 siblings, 0 replies; 43+ messages in thread
From: Alexandru Gagniuc @ 2020-10-20 22:14 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: sam, Alexandru Gagniuc, David Airlie, Daniel Vetter, Rob Herring,
	Andrzej Hajda, Neil Armstrong, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, Mauro Carvalho Chehab, Mark Brown,
	Thomas Bogendoerfer, open list

On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
voltage before the reset sequence is initiated. On most boards, this
assumption is true at boot-up, so initialization succeeds.

However, when we try to initialize the chip with incorrect supply
voltages, it will not respond to I2C requests. sii902x_probe() fails
with -ENXIO.

To resolve this, look for the "iovcc" and "cvcc12" regulators, and
make sure they are enabled before starting the reset sequence. If
these supplies are not available in devicetree, then they will default
to dummy-regulator. In that case everything will work like before.

This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
On this board, the supplies would be set by the second stage
bootloader, which does not run in falcon mode.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
Changes since v1:
  * Fix return code after regulator_enable(sii902x->iovcc) fails (Fabio Estevam)
  * Use dev_err_probe() instead of dev_err() where appropriate (Sam Ravnborg)

Changes since v2:
  * Eliminate prototype for static functionn sii902x_init (Sam Ravnborg)
  * Bundled supplies under regulator_bulk_get/enable/disable()
  
 drivers/gpu/drm/bridge/sii902x.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index f78c17f49887..5bab51a6b55c 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -17,6 +17,7 @@
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/clk.h>
 
 #include <drm/drm_atomic_helper.h>
@@ -168,6 +169,7 @@ struct sii902x {
 	struct drm_connector connector;
 	struct gpio_desc *reset_gpio;
 	struct i2c_mux_core *i2cmux;
+	struct regulator_bulk_data supplies[2];
 	/*
 	 * Mutex protects audio and video functions from interfering
 	 * each other, by keeping their i2c command sequences atomic.
@@ -1049,7 +1051,26 @@ static int sii902x_probe(struct i2c_client *client,
 
 	mutex_init(&sii902x->mutex);
 
+	sii902x->supplies[0].supply = "iovcc";
+	sii902x->supplies[1].supply = "cvcc12";
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(sii902x->supplies),
+				      sii902x->supplies);
+	if (ret < 0)
+		return ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(sii902x->supplies),
+				    sii902x->supplies);
+	if (ret < 0) {
+		dev_err_probe(dev, ret, "Failed to enable supplies");
+		return ret;
+	}
+
 	ret = sii902x_init(sii902x);
+	if (ret < 0) {
+		regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
+					sii902x->supplies);
+	}
+
 	return ret;
 }
 
@@ -1060,6 +1081,8 @@ static int sii902x_remove(struct i2c_client *client)
 
 	i2c_mux_del_adapters(sii902x->i2cmux);
 	drm_bridge_remove(&sii902x->bridge);
+	regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
+				sii902x->supplies);
 
 	return 0;
 }
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 2/3] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-10-20 22:14     ` Alexandru Gagniuc
  0 siblings, 0 replies; 43+ messages in thread
From: Alexandru Gagniuc @ 2020-10-20 22:14 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: Jernej Skrabec, Mauro Carvalho Chehab, Neil Armstrong,
	David Airlie, Mark Brown, Jonas Karlman, open list,
	Andrzej Hajda, Rob Herring, Alexandru Gagniuc,
	Thomas Bogendoerfer, sam, Laurent Pinchart

On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
voltage before the reset sequence is initiated. On most boards, this
assumption is true at boot-up, so initialization succeeds.

However, when we try to initialize the chip with incorrect supply
voltages, it will not respond to I2C requests. sii902x_probe() fails
with -ENXIO.

To resolve this, look for the "iovcc" and "cvcc12" regulators, and
make sure they are enabled before starting the reset sequence. If
these supplies are not available in devicetree, then they will default
to dummy-regulator. In that case everything will work like before.

This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
On this board, the supplies would be set by the second stage
bootloader, which does not run in falcon mode.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
Changes since v1:
  * Fix return code after regulator_enable(sii902x->iovcc) fails (Fabio Estevam)
  * Use dev_err_probe() instead of dev_err() where appropriate (Sam Ravnborg)

Changes since v2:
  * Eliminate prototype for static functionn sii902x_init (Sam Ravnborg)
  * Bundled supplies under regulator_bulk_get/enable/disable()
  
 drivers/gpu/drm/bridge/sii902x.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index f78c17f49887..5bab51a6b55c 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -17,6 +17,7 @@
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/clk.h>
 
 #include <drm/drm_atomic_helper.h>
@@ -168,6 +169,7 @@ struct sii902x {
 	struct drm_connector connector;
 	struct gpio_desc *reset_gpio;
 	struct i2c_mux_core *i2cmux;
+	struct regulator_bulk_data supplies[2];
 	/*
 	 * Mutex protects audio and video functions from interfering
 	 * each other, by keeping their i2c command sequences atomic.
@@ -1049,7 +1051,26 @@ static int sii902x_probe(struct i2c_client *client,
 
 	mutex_init(&sii902x->mutex);
 
+	sii902x->supplies[0].supply = "iovcc";
+	sii902x->supplies[1].supply = "cvcc12";
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(sii902x->supplies),
+				      sii902x->supplies);
+	if (ret < 0)
+		return ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(sii902x->supplies),
+				    sii902x->supplies);
+	if (ret < 0) {
+		dev_err_probe(dev, ret, "Failed to enable supplies");
+		return ret;
+	}
+
 	ret = sii902x_init(sii902x);
+	if (ret < 0) {
+		regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
+					sii902x->supplies);
+	}
+
 	return ret;
 }
 
@@ -1060,6 +1081,8 @@ static int sii902x_remove(struct i2c_client *client)
 
 	i2c_mux_del_adapters(sii902x->i2cmux);
 	drm_bridge_remove(&sii902x->bridge);
+	regulator_bulk_disable(ARRAY_SIZE(sii902x->supplies),
+				sii902x->supplies);
 
 	return 0;
 }
-- 
2.26.2

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

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 3/3] dt-bindings: display: sii902x: Add supply bindings
  2020-09-28 17:30   ` Alexandru Gagniuc
@ 2020-10-20 22:14     ` Alexandru Gagniuc
  -1 siblings, 0 replies; 43+ messages in thread
From: Alexandru Gagniuc @ 2020-10-20 22:14 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: sam, Alexandru Gagniuc, Rob Herring, David Airlie, Daniel Vetter,
	Rob Herring, Andrzej Hajda, Neil Armstrong, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Thomas Bogendoerfer, Mark Brown,
	Mauro Carvalho Chehab, open list

The sii902x chip family requires IO and core voltages to reach the
correct voltage before chip initialization. Add binding for describing
the two supplies.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
Changes since v1, v2:
  * Nothing. version incremented to stay in sync with sii902x regulator patch
  * Added Rob's acked-by line

 Documentation/devicetree/bindings/display/bridge/sii902x.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
index 0d1db3f9da84..02c21b584741 100644
--- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
+++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
@@ -8,6 +8,8 @@ Optional properties:
 	- interrupts: describe the interrupt line used to inform the host
 	  about hotplug events.
 	- reset-gpios: OF device-tree gpio specification for RST_N pin.
+	- iovcc-supply: I/O Supply Voltage (1.8V or 3.3V)
+	- cvcc12-supply: Digital Core Supply Voltage (1.2V)
 
 	HDMI audio properties:
 	- #sound-dai-cells: <0> or <1>. <0> if only i2s or spdif pin
@@ -54,6 +56,8 @@ Example:
 		compatible = "sil,sii9022";
 		reg = <0x39>;
 		reset-gpios = <&pioA 1 0>;
+		iovcc-supply = <&v3v3_hdmi>;
+		cvcc12-supply = <&v1v2_hdmi>;
 
 		#sound-dai-cells = <0>;
 		sil,i2s-data-lanes = < 0 1 2 >;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v3 3/3] dt-bindings: display: sii902x: Add supply bindings
@ 2020-10-20 22:14     ` Alexandru Gagniuc
  0 siblings, 0 replies; 43+ messages in thread
From: Alexandru Gagniuc @ 2020-10-20 22:14 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: Jernej Skrabec, Thomas Bogendoerfer, Neil Armstrong,
	David Airlie, Mark Brown, Jonas Karlman, open list,
	Andrzej Hajda, Rob Herring, Alexandru Gagniuc, sam,
	Mauro Carvalho Chehab, Laurent Pinchart

The sii902x chip family requires IO and core voltages to reach the
correct voltage before chip initialization. Add binding for describing
the two supplies.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
Changes since v1, v2:
  * Nothing. version incremented to stay in sync with sii902x regulator patch
  * Added Rob's acked-by line

 Documentation/devicetree/bindings/display/bridge/sii902x.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
index 0d1db3f9da84..02c21b584741 100644
--- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
+++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
@@ -8,6 +8,8 @@ Optional properties:
 	- interrupts: describe the interrupt line used to inform the host
 	  about hotplug events.
 	- reset-gpios: OF device-tree gpio specification for RST_N pin.
+	- iovcc-supply: I/O Supply Voltage (1.8V or 3.3V)
+	- cvcc12-supply: Digital Core Supply Voltage (1.2V)
 
 	HDMI audio properties:
 	- #sound-dai-cells: <0> or <1>. <0> if only i2s or spdif pin
@@ -54,6 +56,8 @@ Example:
 		compatible = "sil,sii9022";
 		reg = <0x39>;
 		reset-gpios = <&pioA 1 0>;
+		iovcc-supply = <&v3v3_hdmi>;
+		cvcc12-supply = <&v1v2_hdmi>;
 
 		#sound-dai-cells = <0>;
 		sil,i2s-data-lanes = < 0 1 2 >;
-- 
2.26.2

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

^ permalink raw reply related	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 2/3] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
  2020-10-20 22:14     ` Alexandru Gagniuc
@ 2020-11-08 10:55       ` Sam Ravnborg
  -1 siblings, 0 replies; 43+ messages in thread
From: Sam Ravnborg @ 2020-11-08 10:55 UTC (permalink / raw)
  To: Alexandru Gagniuc
  Cc: dri-devel, devicetree, Jernej Skrabec, Mauro Carvalho Chehab,
	Neil Armstrong, David Airlie, Mark Brown, Jonas Karlman,
	open list, Andrzej Hajda, Rob Herring, Thomas Bogendoerfer,
	Laurent Pinchart

Hi Alexandru

On Tue, Oct 20, 2020 at 05:14:58PM -0500, Alexandru Gagniuc wrote:
> On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
> voltage before the reset sequence is initiated. On most boards, this
> assumption is true at boot-up, so initialization succeeds.
> 
> However, when we try to initialize the chip with incorrect supply
> voltages, it will not respond to I2C requests. sii902x_probe() fails
> with -ENXIO.
> 
> To resolve this, look for the "iovcc" and "cvcc12" regulators, and
> make sure they are enabled before starting the reset sequence. If
> these supplies are not available in devicetree, then they will default
> to dummy-regulator. In that case everything will work like before.
> 
> This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
> On this board, the supplies would be set by the second stage
> bootloader, which does not run in falcon mode.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

Thanks, applied series to drm-misc-next. I fixed two
checkpatch --strict warnings while applying.

	Sam

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v3 2/3] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-11-08 10:55       ` Sam Ravnborg
  0 siblings, 0 replies; 43+ messages in thread
From: Sam Ravnborg @ 2020-11-08 10:55 UTC (permalink / raw)
  To: Alexandru Gagniuc
  Cc: devicetree, Jernej Skrabec, Thomas Bogendoerfer, Rob Herring,
	Neil Armstrong, Mauro Carvalho Chehab, Jonas Karlman, open list,
	dri-devel, Andrzej Hajda, David Airlie, Mark Brown,
	Laurent Pinchart

Hi Alexandru

On Tue, Oct 20, 2020 at 05:14:58PM -0500, Alexandru Gagniuc wrote:
> On the SII9022, the IOVCC and CVCC12 supplies must reach the correct
> voltage before the reset sequence is initiated. On most boards, this
> assumption is true at boot-up, so initialization succeeds.
> 
> However, when we try to initialize the chip with incorrect supply
> voltages, it will not respond to I2C requests. sii902x_probe() fails
> with -ENXIO.
> 
> To resolve this, look for the "iovcc" and "cvcc12" regulators, and
> make sure they are enabled before starting the reset sequence. If
> these supplies are not available in devicetree, then they will default
> to dummy-regulator. In that case everything will work like before.
> 
> This was observed on a STM32MP157C-DK2 booting in u-boot falcon mode.
> On this board, the supplies would be set by the second stage
> bootloader, which does not run in falcon mode.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

Thanks, applied series to drm-misc-next. I fixed two
checkpatch --strict warnings while applying.

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

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present
@ 2020-09-26  0:13 kernel test robot
  0 siblings, 0 replies; 43+ messages in thread
From: kernel test robot @ 2020-09-26  0:13 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 6908 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <20200924200507.1175888-1-mr.nuke.me@gmail.com>
References: <20200924200507.1175888-1-mr.nuke.me@gmail.com>
TO: Alexandru Gagniuc <mr.nuke.me@gmail.com>
TO: dri-devel(a)lists.freedesktop.org
TO: devicetree(a)vger.kernel.org
CC: Alexandru Gagniuc <mr.nuke.me@gmail.com>
CC: David Airlie <airlied@linux.ie>
CC: Daniel Vetter <daniel@ffwll.ch>
CC: Rob Herring <robh+dt@kernel.org>
CC: Andrzej Hajda <a.hajda@samsung.com>
CC: Neil Armstrong <narmstrong@baylibre.com>
CC: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
CC: Jonas Karlman <jonas@kwiboo.se>

Hi Alexandru,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next v5.9-rc6 next-20200925]
[cannot apply to drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Alexandru-Gagniuc/drm-bridge-sii902x-Enable-I-O-and-core-VCC-supplies-if-present/20200925-041504
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
:::::: branch date: 28 hours ago
:::::: commit date: 28 hours ago
config: x86_64-randconfig-m001-20200925 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/gpu/drm/bridge/sii902x.c:1013 sii902x_probe() warn: passing zero to 'PTR_ERR'

vim +/PTR_ERR +1013 drivers/gpu/drm/bridge/sii902x.c

6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24   961  
675605c1c8e6748 Boris Brezillon   2015-12-31   962  static int sii902x_probe(struct i2c_client *client,
675605c1c8e6748 Boris Brezillon   2015-12-31   963  			 const struct i2c_device_id *id)
675605c1c8e6748 Boris Brezillon   2015-12-31   964  {
675605c1c8e6748 Boris Brezillon   2015-12-31   965  	struct device *dev = &client->dev;
675605c1c8e6748 Boris Brezillon   2015-12-31   966  	struct sii902x *sii902x;
675605c1c8e6748 Boris Brezillon   2015-12-31   967  	int ret;
675605c1c8e6748 Boris Brezillon   2015-12-31   968  
21d808405fe4902 Fabrizio Castro   2018-11-06   969  	ret = i2c_check_functionality(client->adapter,
21d808405fe4902 Fabrizio Castro   2018-11-06   970  				      I2C_FUNC_SMBUS_BYTE_DATA);
21d808405fe4902 Fabrizio Castro   2018-11-06   971  	if (!ret) {
21d808405fe4902 Fabrizio Castro   2018-11-06   972  		dev_err(dev, "I2C adapter not suitable\n");
21d808405fe4902 Fabrizio Castro   2018-11-06   973  		return -EIO;
21d808405fe4902 Fabrizio Castro   2018-11-06   974  	}
21d808405fe4902 Fabrizio Castro   2018-11-06   975  
675605c1c8e6748 Boris Brezillon   2015-12-31   976  	sii902x = devm_kzalloc(dev, sizeof(*sii902x), GFP_KERNEL);
675605c1c8e6748 Boris Brezillon   2015-12-31   977  	if (!sii902x)
675605c1c8e6748 Boris Brezillon   2015-12-31   978  		return -ENOMEM;
675605c1c8e6748 Boris Brezillon   2015-12-31   979  
675605c1c8e6748 Boris Brezillon   2015-12-31   980  	sii902x->i2c = client;
675605c1c8e6748 Boris Brezillon   2015-12-31   981  	sii902x->regmap = devm_regmap_init_i2c(client, &sii902x_regmap_config);
675605c1c8e6748 Boris Brezillon   2015-12-31   982  	if (IS_ERR(sii902x->regmap))
675605c1c8e6748 Boris Brezillon   2015-12-31   983  		return PTR_ERR(sii902x->regmap);
675605c1c8e6748 Boris Brezillon   2015-12-31   984  
675605c1c8e6748 Boris Brezillon   2015-12-31   985  	sii902x->reset_gpio = devm_gpiod_get_optional(dev, "reset",
675605c1c8e6748 Boris Brezillon   2015-12-31   986  						      GPIOD_OUT_LOW);
675605c1c8e6748 Boris Brezillon   2015-12-31   987  	if (IS_ERR(sii902x->reset_gpio)) {
675605c1c8e6748 Boris Brezillon   2015-12-31   988  		dev_err(dev, "Failed to retrieve/request reset gpio: %ld\n",
675605c1c8e6748 Boris Brezillon   2015-12-31   989  			PTR_ERR(sii902x->reset_gpio));
675605c1c8e6748 Boris Brezillon   2015-12-31   990  		return PTR_ERR(sii902x->reset_gpio);
675605c1c8e6748 Boris Brezillon   2015-12-31   991  	}
675605c1c8e6748 Boris Brezillon   2015-12-31   992  
ff5781634c41167 Jyri Sarha        2019-05-27   993  	mutex_init(&sii902x->mutex);
ff5781634c41167 Jyri Sarha        2019-05-27   994  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24   995  	sii902x->iovcc = devm_regulator_get(dev, "iovcc");
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24   996  	if (IS_ERR(sii902x->iovcc))
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24   997  		return PTR_ERR(sii902x->iovcc);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24   998  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24   999  	sii902x->cvcc12 = devm_regulator_get(dev, "cvcc12");
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1000  	if (IS_ERR(sii902x->cvcc12))
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1001  		return PTR_ERR(sii902x->cvcc12);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1002  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1003  	ret = regulator_enable(sii902x->iovcc);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1004  	if (ret < 0) {
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1005  		dev_err(dev, "Failed to enable iovcc supply: %d\n", ret);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1006  		return ret;
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1007  	}
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1008  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1009  	ret = regulator_enable(sii902x->cvcc12);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1010  	if (ret < 0) {
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1011  		dev_err(dev, "Failed to enable cvcc12 supply: %d\n", ret);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1012  		regulator_disable(sii902x->iovcc);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24 @1013  		return PTR_ERR(sii902x->cvcc12);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1014  	}
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1015  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1016  	ret = sii902x_init(sii902x);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1017  	if (ret < 0) {
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1018  		regulator_disable(sii902x->cvcc12);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1019  		regulator_disable(sii902x->iovcc);
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1020  	}
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1021  
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1022  	return ret;
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1023  }
6f6c7f7bbe3f177 Alexandru Gagniuc 2020-09-24  1024  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 30891 bytes --]

^ permalink raw reply	[flat|nested] 43+ messages in thread

end of thread, other threads:[~2020-11-08 10:55 UTC | newest]

Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-24 20:05 [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present Alexandru Gagniuc
2020-09-24 20:05 ` Alexandru Gagniuc
2020-09-24 20:05 ` [PATCH 2/2] dt-bindings: display: sii902x: Add supply bindings Alexandru Gagniuc
2020-09-24 20:05   ` Alexandru Gagniuc
2020-09-26 18:42   ` Sam Ravnborg
2020-09-26 18:42     ` Sam Ravnborg
2020-09-24 20:22 ` [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present Fabio Estevam
2020-09-24 20:22   ` Fabio Estevam
2020-09-24 20:34   ` Alex G.
2020-09-24 20:34     ` Alex G.
2020-09-26 12:50 ` Dan Carpenter
2020-09-26 12:50   ` Dan Carpenter
2020-09-26 12:50   ` Dan Carpenter
2020-09-26 12:50   ` Dan Carpenter
2020-09-26 18:49 ` Sam Ravnborg
2020-09-26 18:49   ` Sam Ravnborg
2020-09-28 17:35   ` Alex G.
2020-09-28 17:35     ` Alex G.
2020-09-28 19:03     ` Sam Ravnborg
2020-09-28 19:03       ` Sam Ravnborg
2020-09-28 17:30 ` [PATCH v2 " Alexandru Gagniuc
2020-09-28 17:30   ` Alexandru Gagniuc
2020-09-28 17:30   ` [PATCH v2 2/2] dt-bindings: display: sii902x: Add supply bindings Alexandru Gagniuc
2020-09-28 17:30     ` Alexandru Gagniuc
2020-09-29 20:17     ` Rob Herring
2020-09-29 20:17       ` Rob Herring
2020-10-20  1:24   ` [PATCH v2 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present Alex G.
2020-10-20  1:24     ` Alex G.
2020-10-20  7:16     ` Sam Ravnborg
2020-10-20  7:16       ` Sam Ravnborg
2020-10-20 14:01       ` Alex G.
2020-10-20 14:01         ` Alex G.
2020-10-20 15:08         ` Sam Ravnborg
2020-10-20 15:08           ` Sam Ravnborg
2020-10-20 22:14   ` [PATCH v3 1/3] drm/bridge: sii902x: Refactor init code into separate function Alexandru Gagniuc
2020-10-20 22:14     ` Alexandru Gagniuc
2020-10-20 22:14   ` [PATCH v3 2/3] drm/bridge: sii902x: Enable I/O and core VCC supplies if present Alexandru Gagniuc
2020-10-20 22:14     ` Alexandru Gagniuc
2020-11-08 10:55     ` Sam Ravnborg
2020-11-08 10:55       ` Sam Ravnborg
2020-10-20 22:14   ` [PATCH v3 3/3] dt-bindings: display: sii902x: Add supply bindings Alexandru Gagniuc
2020-10-20 22:14     ` Alexandru Gagniuc
2020-09-26  0:13 [PATCH 1/2] drm/bridge: sii902x: Enable I/O and core VCC supplies if present kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.