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 <[email protected]>
---
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
Hi Alexandre,
On Thu, Sep 24, 2020 at 5:16 PM Alexandru Gagniuc <[email protected]> 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.
On 9/24/20 3:22 PM, Fabio Estevam wrote:
Hi Fabio,
> On Thu, Sep 24, 2020 at 5:16 PM Alexandru Gagniuc <[email protected]> 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
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 <[email protected]>
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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
>
> 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]
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 <[email protected]>
> >
> > 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
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 <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]>
> ---
> 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;
> }
>
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 <[email protected]>
> > ---
> > 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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
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
>> [email protected]
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
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
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 <[email protected]>
Thanks, applied series to drm-misc-next. I fixed two
checkpatch --strict warnings while applying.
Sam