ADV7533 requires supply to the AVDD, V1P2 and V3P3 pins for proper functionality.
Initialize and enable the regulators during probe itself. Controlling these dynamically is left for later.
Cc: dri-devel@lists.freedesktop.org Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Archit Taneja architt@codeaurora.org --- drivers/gpu/drm/bridge/adv7511/adv7511.h | 16 ++++++++++ drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 34 +++++++++++++++------ drivers/gpu/drm/bridge/adv7511/adv7533.c | 45 ++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 161c923..3fcb202 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h @@ -12,6 +12,7 @@ #include <linux/hdmi.h> #include <linux/i2c.h> #include <linux/regmap.h> +#include <linux/regulator/consumer.h>
#include <drm/drm_crtc_helper.h> #include <drm/drm_mipi_dsi.h> @@ -327,6 +328,10 @@ struct adv7511 {
struct gpio_desc *gpio_pd;
+ struct regulator *avdd; + struct regulator *v1p2; + struct regulator *v3p3; + /* ADV7533 DSI RX related params */ struct device_node *host_node; struct mipi_dsi_device *dsi; @@ -343,6 +348,8 @@ void adv7533_mode_set(struct adv7511 *adv, struct drm_display_mode *mode); int adv7533_patch_registers(struct adv7511 *adv); void adv7533_uninit_cec(struct adv7511 *adv); int adv7533_init_cec(struct adv7511 *adv); +int adv7533_init_regulators(struct adv7511 *adv); +void adv7533_uninit_regulators(struct adv7511 *adv); int adv7533_attach_dsi(struct adv7511 *adv); void adv7533_detach_dsi(struct adv7511 *adv); int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv); @@ -374,6 +381,15 @@ static inline int adv7533_init_cec(struct adv7511 *adv) return -ENODEV; }
+static inline int adv7533_init_regulators(struct adv7511 *adv) +{ + return -ENODEV; +} + +static inline void adv7533_uninit_regulators(struct adv7511 *adv) +{ +} + static inline int adv7533_attach_dsi(struct adv7511 *adv) { return -ENODEV; diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index ec8fb2e..221bc75 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -941,6 +941,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) if (!adv7511) return -ENOMEM;
+ adv7511->i2c_main = i2c; adv7511->powered = false; adv7511->status = connector_status_disconnected;
@@ -958,13 +959,21 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) if (ret) return ret;
+ if (adv7511->type == ADV7533) { + ret = adv7533_init_regulators(adv7511); + if (ret) + return ret; + } + /* * The power down GPIO is optional. If present, toggle it from active to * inactive to wake up the encoder. */ adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH); - if (IS_ERR(adv7511->gpio_pd)) - return PTR_ERR(adv7511->gpio_pd); + if (IS_ERR(adv7511->gpio_pd)) { + ret = PTR_ERR(adv7511->gpio_pd); + goto uninit_regulators; + }
if (adv7511->gpio_pd) { mdelay(5); @@ -972,12 +981,14 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) }
adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config); - if (IS_ERR(adv7511->regmap)) - return PTR_ERR(adv7511->regmap); + if (IS_ERR(adv7511->regmap)) { + ret = PTR_ERR(adv7511->regmap); + goto uninit_regulators; + }
ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); if (ret) - return ret; + goto uninit_regulators; dev_dbg(dev, "Rev. %d\n", val);
if (adv7511->type == ADV7511) @@ -987,7 +998,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) else ret = adv7533_patch_registers(adv7511); if (ret) - return ret; + goto uninit_regulators;
regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, edid_i2c_addr); regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, @@ -995,10 +1006,11 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr); adv7511_packet_disable(adv7511, 0xffff);
- adv7511->i2c_main = i2c; adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); - if (!adv7511->i2c_edid) - return -ENOMEM; + if (!adv7511->i2c_edid) { + ret = -ENOMEM; + goto uninit_regulators; + }
if (adv7511->type == ADV7533) { ret = adv7533_init_cec(adv7511); @@ -1043,6 +1055,9 @@ err_unregister_cec: adv7533_uninit_cec(adv7511); err_i2c_unregister_edid: i2c_unregister_device(adv7511->i2c_edid); +uninit_regulators: + if (adv7511->type == ADV7533) + adv7533_uninit_regulators(adv7511);
return ret; } @@ -1054,6 +1069,7 @@ static int adv7511_remove(struct i2c_client *i2c) if (adv7511->type == ADV7533) { adv7533_detach_dsi(adv7511); adv7533_uninit_cec(adv7511); + adv7533_uninit_regulators(adv7511); }
drm_bridge_remove(&adv7511->bridge); diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c index 5eebd15..03a59fd 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c @@ -178,6 +178,51 @@ err: return ret; }
+int adv7533_init_regulators(struct adv7511 *adv) +{ + struct device *dev = &adv->i2c_main->dev; + int ret; + + adv->avdd = devm_regulator_get(dev, "avdd"); + if (IS_ERR(adv->avdd)) + return PTR_ERR(adv->avdd); + + adv->v1p2 = devm_regulator_get(dev, "v1p2"); + if (IS_ERR(adv->v1p2)) + return PTR_ERR(adv->v1p2); + + adv->v3p3 = devm_regulator_get(dev, "v3p3"); + if (IS_ERR(adv->v3p3)) + return PTR_ERR(adv->v3p3); + + ret = regulator_enable(adv->avdd); + if (ret) + return ret; + + ret = regulator_enable(adv->v1p2); + if (ret) + goto err_v1p2; + + ret = regulator_enable(adv->v3p3); + if (ret) + goto err_v3p3; + + return 0; +err_v3p3: + regulator_disable(adv->v1p2); +err_v1p2: + regulator_disable(adv->avdd); + + return ret; +} + +void adv7533_uninit_regulators(struct adv7511 *adv) +{ + regulator_disable(adv->v3p3); + regulator_disable(adv->v1p2); + regulator_disable(adv->avdd); +} + int adv7533_attach_dsi(struct adv7511 *adv) { struct device *dev = &adv->i2c_main->dev;
Hi Archit,
Thank you for the patch.
On Wednesday 31 Aug 2016 16:22:09 Archit Taneja wrote:
ADV7533 requires supply to the AVDD, V1P2 and V3P3 pins for proper functionality.
Initialize and enable the regulators during probe itself. Controlling these dynamically is left for later.
You should document the power supplies in the DT bindings.
Cc: dri-devel@lists.freedesktop.org Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Archit Taneja architt@codeaurora.org
drivers/gpu/drm/bridge/adv7511/adv7511.h | 16 ++++++++++ drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 34 +++++++++++++++------ drivers/gpu/drm/bridge/adv7511/adv7533.c | 45 +++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 161c923..3fcb202 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h @@ -12,6 +12,7 @@ #include <linux/hdmi.h> #include <linux/i2c.h> #include <linux/regmap.h> +#include <linux/regulator/consumer.h>
#include <drm/drm_crtc_helper.h> #include <drm/drm_mipi_dsi.h> @@ -327,6 +328,10 @@ struct adv7511 {
struct gpio_desc *gpio_pd;
- struct regulator *avdd;
- struct regulator *v1p2;
- struct regulator *v3p3;
- /* ADV7533 DSI RX related params */ struct device_node *host_node; struct mipi_dsi_device *dsi;
@@ -343,6 +348,8 @@ void adv7533_mode_set(struct adv7511 *adv, struct drm_display_mode *mode); int adv7533_patch_registers(struct adv7511 *adv); void adv7533_uninit_cec(struct adv7511 *adv); int adv7533_init_cec(struct adv7511 *adv); +int adv7533_init_regulators(struct adv7511 *adv); +void adv7533_uninit_regulators(struct adv7511 *adv); int adv7533_attach_dsi(struct adv7511 *adv); void adv7533_detach_dsi(struct adv7511 *adv); int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv); @@ -374,6 +381,15 @@ static inline int adv7533_init_cec(struct adv7511 *adv) return -ENODEV; }
+static inline int adv7533_init_regulators(struct adv7511 *adv) +{
- return -ENODEV;
+}
+static inline void adv7533_uninit_regulators(struct adv7511 *adv) +{ +}
static inline int adv7533_attach_dsi(struct adv7511 *adv) { return -ENODEV; diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index ec8fb2e..221bc75 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -941,6 +941,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) if (!adv7511) return -ENOMEM;
- adv7511->i2c_main = i2c; adv7511->powered = false; adv7511->status = connector_status_disconnected;
@@ -958,13 +959,21 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) if (ret) return ret;
- if (adv7511->type == ADV7533) {
ret = adv7533_init_regulators(adv7511);if (ret)return ret;- }
- /*
- The power down GPIO is optional. If present, toggle it from active
to
* inactive to wake up the encoder. */adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH);
- if (IS_ERR(adv7511->gpio_pd))
return PTR_ERR(adv7511->gpio_pd);
if (IS_ERR(adv7511->gpio_pd)) {
ret = PTR_ERR(adv7511->gpio_pd);goto uninit_regulators;}
if (adv7511->gpio_pd) { mdelay(5);
@@ -972,12 +981,14 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) }
adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config);
- if (IS_ERR(adv7511->regmap))
return PTR_ERR(adv7511->regmap);
if (IS_ERR(adv7511->regmap)) {
ret = PTR_ERR(adv7511->regmap);goto uninit_regulators;}
ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); if (ret)
return ret;
goto uninit_regulators;dev_dbg(dev, "Rev. %d\n", val);
if (adv7511->type == ADV7511)
@@ -987,7 +998,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) else ret = adv7533_patch_registers(adv7511); if (ret)
return ret;
goto uninit_regulators;regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR,
edid_i2c_addr);
regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, @@ -995,10 +1006,11 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr); adv7511_packet_disable(adv7511, 0xffff);
- adv7511->i2c_main = i2c; adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1);
- if (!adv7511->i2c_edid)
return -ENOMEM;
if (!adv7511->i2c_edid) {
ret = -ENOMEM;goto uninit_regulators;}
if (adv7511->type == ADV7533) { ret = adv7533_init_cec(adv7511);
@@ -1043,6 +1055,9 @@ err_unregister_cec: adv7533_uninit_cec(adv7511); err_i2c_unregister_edid: i2c_unregister_device(adv7511->i2c_edid); +uninit_regulators:
if (adv7511->type == ADV7533)
adv7533_uninit_regulators(adv7511);return ret;
} @@ -1054,6 +1069,7 @@ static int adv7511_remove(struct i2c_client *i2c) if (adv7511->type == ADV7533) { adv7533_detach_dsi(adv7511); adv7533_uninit_cec(adv7511);
adv7533_uninit_regulators(adv7511);}
drm_bridge_remove(&adv7511->bridge);
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c index 5eebd15..03a59fd 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c @@ -178,6 +178,51 @@ err: return ret; }
+int adv7533_init_regulators(struct adv7511 *adv) +{
- struct device *dev = &adv->i2c_main->dev;
- int ret;
- adv->avdd = devm_regulator_get(dev, "avdd");
- if (IS_ERR(adv->avdd))
return PTR_ERR(adv->avdd);
Doesn't this cause backward compatibility issues with existing device trees that don't declare regulators ?
- adv->v1p2 = devm_regulator_get(dev, "v1p2");
- if (IS_ERR(adv->v1p2))
return PTR_ERR(adv->v1p2);- adv->v3p3 = devm_regulator_get(dev, "v3p3");
- if (IS_ERR(adv->v3p3))
return PTR_ERR(adv->v3p3);- ret = regulator_enable(adv->avdd);
- if (ret)
return ret;- ret = regulator_enable(adv->v1p2);
- if (ret)
goto err_v1p2;- ret = regulator_enable(adv->v3p3);
- if (ret)
goto err_v3p3;
How about using the devm_regulator_bulk_*() API ?
- return 0;
+err_v3p3:
- regulator_disable(adv->v1p2);
+err_v1p2:
- regulator_disable(adv->avdd);
- return ret;
+}
+void adv7533_uninit_regulators(struct adv7511 *adv) +{
- regulator_disable(adv->v3p3);
- regulator_disable(adv->v1p2);
- regulator_disable(adv->avdd);
+}
int adv7533_attach_dsi(struct adv7511 *adv) { struct device *dev = &adv->i2c_main->dev;
Hi Laurent,
On 8/31/2016 9:23 PM, Laurent Pinchart wrote:
Hi Archit,
Thank you for the patch.
On Wednesday 31 Aug 2016 16:22:09 Archit Taneja wrote:
ADV7533 requires supply to the AVDD, V1P2 and V3P3 pins for proper functionality.
Initialize and enable the regulators during probe itself. Controlling these dynamically is left for later.
You should document the power supplies in the DT bindings.
The DT bindings doc update was a part of the same series. I accidentally Cc'd you only for this patch.
Cc: dri-devel@lists.freedesktop.org Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Archit Taneja architt@codeaurora.org
drivers/gpu/drm/bridge/adv7511/adv7511.h | 16 ++++++++++ drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 34 +++++++++++++++------ drivers/gpu/drm/bridge/adv7511/adv7533.c | 45 +++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h index 161c923..3fcb202 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h @@ -12,6 +12,7 @@ #include <linux/hdmi.h> #include <linux/i2c.h> #include <linux/regmap.h> +#include <linux/regulator/consumer.h>
#include <drm/drm_crtc_helper.h> #include <drm/drm_mipi_dsi.h> @@ -327,6 +328,10 @@ struct adv7511 {
struct gpio_desc *gpio_pd;
- struct regulator *avdd;
- struct regulator *v1p2;
- struct regulator *v3p3;
- /* ADV7533 DSI RX related params */ struct device_node *host_node; struct mipi_dsi_device *dsi;
@@ -343,6 +348,8 @@ void adv7533_mode_set(struct adv7511 *adv, struct drm_display_mode *mode); int adv7533_patch_registers(struct adv7511 *adv); void adv7533_uninit_cec(struct adv7511 *adv); int adv7533_init_cec(struct adv7511 *adv); +int adv7533_init_regulators(struct adv7511 *adv); +void adv7533_uninit_regulators(struct adv7511 *adv); int adv7533_attach_dsi(struct adv7511 *adv); void adv7533_detach_dsi(struct adv7511 *adv); int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv); @@ -374,6 +381,15 @@ static inline int adv7533_init_cec(struct adv7511 *adv) return -ENODEV; }
+static inline int adv7533_init_regulators(struct adv7511 *adv) +{
- return -ENODEV;
+}
+static inline void adv7533_uninit_regulators(struct adv7511 *adv) +{ +}
- static inline int adv7533_attach_dsi(struct adv7511 *adv) { return -ENODEV;
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index ec8fb2e..221bc75 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -941,6 +941,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) if (!adv7511) return -ENOMEM;
- adv7511->i2c_main = i2c; adv7511->powered = false; adv7511->status = connector_status_disconnected;
@@ -958,13 +959,21 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) if (ret) return ret;
- if (adv7511->type == ADV7533) {
ret = adv7533_init_regulators(adv7511);if (ret)return ret;- }
- /*
- The power down GPIO is optional. If present, toggle it from active
to
* inactive to wake up the encoder. */adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH);
- if (IS_ERR(adv7511->gpio_pd))
return PTR_ERR(adv7511->gpio_pd);
if (IS_ERR(adv7511->gpio_pd)) {
ret = PTR_ERR(adv7511->gpio_pd);goto uninit_regulators;}
if (adv7511->gpio_pd) { mdelay(5);
@@ -972,12 +981,14 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) }
adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config);
- if (IS_ERR(adv7511->regmap))
return PTR_ERR(adv7511->regmap);
if (IS_ERR(adv7511->regmap)) {
ret = PTR_ERR(adv7511->regmap);goto uninit_regulators;}
ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); if (ret)
return ret;
goto uninit_regulators;dev_dbg(dev, "Rev. %d\n", val);
if (adv7511->type == ADV7511)
@@ -987,7 +998,7 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) else ret = adv7533_patch_registers(adv7511); if (ret)
return ret;
goto uninit_regulators;regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR,
edid_i2c_addr);
regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, @@ -995,10 +1006,11 @@ static int adv7511_probe(struct i2c_client *i2c, const struct i2c_device_id *id) regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr); adv7511_packet_disable(adv7511, 0xffff);
- adv7511->i2c_main = i2c; adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1);
- if (!adv7511->i2c_edid)
return -ENOMEM;
if (!adv7511->i2c_edid) {
ret = -ENOMEM;goto uninit_regulators;}
if (adv7511->type == ADV7533) { ret = adv7533_init_cec(adv7511);
@@ -1043,6 +1055,9 @@ err_unregister_cec: adv7533_uninit_cec(adv7511); err_i2c_unregister_edid: i2c_unregister_device(adv7511->i2c_edid); +uninit_regulators:
if (adv7511->type == ADV7533)
adv7533_uninit_regulators(adv7511);return ret; }
@@ -1054,6 +1069,7 @@ static int adv7511_remove(struct i2c_client *i2c) if (adv7511->type == ADV7533) { adv7533_detach_dsi(adv7511); adv7533_uninit_cec(adv7511);
adv7533_uninit_regulators(adv7511);}
drm_bridge_remove(&adv7511->bridge);
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c index 5eebd15..03a59fd 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c @@ -178,6 +178,51 @@ err: return ret; }
+int adv7533_init_regulators(struct adv7511 *adv) +{
- struct device *dev = &adv->i2c_main->dev;
- int ret;
- adv->avdd = devm_regulator_get(dev, "avdd");
- if (IS_ERR(adv->avdd))
return PTR_ERR(adv->avdd);Doesn't this cause backward compatibility issues with existing device trees that don't declare regulators ?
Well, there isn't any DT upstream that doesn't declare these regulators. The first DT file using ADV7533 would be merged in 4.9, and same for this patch.
Also, if a DT file doesn't declare a regulator here (maybe because the board has a constant supply to this pin since it's powered up), a dummy regulator would be used here.
- adv->v1p2 = devm_regulator_get(dev, "v1p2");
- if (IS_ERR(adv->v1p2))
return PTR_ERR(adv->v1p2);- adv->v3p3 = devm_regulator_get(dev, "v3p3");
- if (IS_ERR(adv->v3p3))
return PTR_ERR(adv->v3p3);- ret = regulator_enable(adv->avdd);
- if (ret)
return ret;- ret = regulator_enable(adv->v1p2);
- if (ret)
goto err_v1p2;- ret = regulator_enable(adv->v3p3);
- if (ret)
goto err_v3p3;How about using the devm_regulator_bulk_*() API ?
Yes, that makes more sense, should make the error handling simpler too.
Thanks for the review, Archit
- return 0;
+err_v3p3:
- regulator_disable(adv->v1p2);
+err_v1p2:
- regulator_disable(adv->avdd);
- return ret;
+}
+void adv7533_uninit_regulators(struct adv7511 *adv) +{
- regulator_disable(adv->v3p3);
- regulator_disable(adv->v1p2);
- regulator_disable(adv->avdd);
+}
- int adv7533_attach_dsi(struct adv7511 *adv) { struct device *dev = &adv->i2c_main->dev;
Hi Archit,
On Wednesday 31 Aug 2016 22:24:30 Archit Taneja wrote:
On 8/31/2016 9:23 PM, Laurent Pinchart wrote:
On Wednesday 31 Aug 2016 16:22:09 Archit Taneja wrote:
ADV7533 requires supply to the AVDD, V1P2 and V3P3 pins for proper functionality.
Initialize and enable the regulators during probe itself. Controlling these dynamically is left for later.
You should document the power supplies in the DT bindings.
The DT bindings doc update was a part of the same series. I accidentally Cc'd you only for this patch.
No worries. What's the patch's subject ?
Cc: dri-devel@lists.freedesktop.org Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Archit Taneja architt@codeaurora.org
drivers/gpu/drm/bridge/adv7511/adv7511.h | 16 ++++++++++ drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 34 +++++++++++++++------ drivers/gpu/drm/bridge/adv7511/adv7533.c | 45 +++++++++++++++++++++ 3 files changed, 86 insertions(+), 9 deletions(-)
On 09/01/2016 02:00 AM, Laurent Pinchart wrote:
Hi Archit,
On Wednesday 31 Aug 2016 22:24:30 Archit Taneja wrote:
On 8/31/2016 9:23 PM, Laurent Pinchart wrote:
On Wednesday 31 Aug 2016 16:22:09 Archit Taneja wrote:
ADV7533 requires supply to the AVDD, V1P2 and V3P3 pins for proper functionality.
Initialize and enable the regulators during probe itself. Controlling these dynamically is left for later.
You should document the power supplies in the DT bindings.
The DT bindings doc update was a part of the same series. I accidentally Cc'd you only for this patch.
No worries. What's the patch's subject ?
It is: dt-bindings: drm/bridge: adv7511: Add regulators for ADV7533
https://patchwork.kernel.org/patch/9306945/
Thanks, Archit
Cc: dri-devel@lists.freedesktop.org Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Signed-off-by: Archit Taneja architt@codeaurora.org
drivers/gpu/drm/bridge/adv7511/adv7511.h | 16 ++++++++++ drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 34 +++++++++++++++------ drivers/gpu/drm/bridge/adv7511/adv7533.c | 45 +++++++++++++++++++++ 3 files changed, 86 insertions(+), 9 deletions(-)
dri-devel@lists.freedesktop.org
-
Archit Taneja -
Laurent Pinchart