Hi Tomi and Sebastian,
Thank you for the patch.
On Thu, Nov 05, 2020 at 02:03:00PM +0200, Tomi Valkeinen wrote:
> From: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
>
> Use bulk regulator API to simplify the code. This also switches
> from _optional variant to normal variant, which will provide a
> dummy regulator (i.e. if some always-enabled regulator is not
> described in DT).
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
> ---
> .../gpu/drm/omapdrm/displays/panel-dsi-cm.c | 65 ++++++-------------
> 1 file changed, 21 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> index 2e9de33fc8d4..1e742cf798b6 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c
> @@ -32,6 +32,8 @@
> #define DCS_GET_ID2 0xdb
> #define DCS_GET_ID3 0xdc
>
> +#define DCS_REGULATOR_SUPPLY_NUM 2
> +
> struct panel_drv_data {
> struct mipi_dsi_device *dsi;
>
> @@ -54,8 +56,7 @@ struct panel_drv_data {
> struct gpio_desc *reset_gpio;
> struct gpio_desc *ext_te_gpio;
>
> - struct regulator *vpnl;
> - struct regulator *vddi;
> + struct regulator_bulk_data supplies[DCS_REGULATOR_SUPPLY_NUM];
>
> bool use_dsi_backlight;
>
> @@ -557,28 +558,16 @@ static int dsicm_power_on(struct panel_drv_data *ddata)
> .lp_clk_max = 10000000,
> };
>
> - if (ddata->vpnl) {
> - r = regulator_enable(ddata->vpnl);
> - if (r) {
> - dev_err(&ddata->dsi->dev,
> - "failed to enable VPNL: %d\n", r);
> - return r;
> - }
> - }
> -
> - if (ddata->vddi) {
> - r = regulator_enable(ddata->vddi);
> - if (r) {
> - dev_err(&ddata->dsi->dev,
> - "failed to enable VDDI: %d\n", r);
> - goto err_vpnl;
> - }
> + r = regulator_bulk_enable(DCS_REGULATOR_SUPPLY_NUM, ddata->supplies);
I'd use ARRAY_SIZE(data->supplies) to replace DCS_REGULATOR_SUPPLY_NUM,
and drop the DCS_REGULATOR_SUPPLY_NUM macro.
> + if (r) {
> + dev_err(&ddata->dsi->dev, "failed to enable supplies: %d\n", r);
> + return r;
> }
>
> r = src->ops->dsi.set_config(src, &dsi_config);
> if (r) {
> dev_err(&ddata->dsi->dev, "failed to configure DSI\n");
> - goto err_vddi;
> + goto err_regulators;
> }
>
> src->ops->enable(src);
> @@ -637,12 +626,10 @@ static int dsicm_power_on(struct panel_drv_data *ddata)
> dsicm_hw_reset(ddata);
>
> src->ops->dsi.disable(src, true, false);
> -err_vddi:
> - if (ddata->vddi)
> - regulator_disable(ddata->vddi);
> -err_vpnl:
> - if (ddata->vpnl)
> - regulator_disable(ddata->vpnl);
> +err_regulators:
> + r = regulator_bulk_disable(DCS_REGULATOR_SUPPLY_NUM, ddata->supplies);
> + if (r)
> + dev_err(&ddata->dsi->dev, "failed to disable supplies: %d\n", r);
Not sure if the error message is needed, but I suppose it doesn't hurt.
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> return r;
> }
> @@ -666,10 +653,9 @@ static void dsicm_power_off(struct panel_drv_data *ddata)
>
> src->ops->dsi.disable(src, true, false);
>
> - if (ddata->vddi)
> - regulator_disable(ddata->vddi);
> - if (ddata->vpnl)
> - regulator_disable(ddata->vpnl);
> + r = regulator_bulk_disable(DCS_REGULATOR_SUPPLY_NUM, ddata->supplies);
> + if (r)
> + dev_err(&ddata->dsi->dev, "failed to disable supplies: %d\n", r);
>
> ddata->enabled = false;
> }
> @@ -973,21 +959,12 @@ static int dsicm_probe_of(struct mipi_dsi_device *dsi)
> ddata->height_mm = 0;
> of_property_read_u32(node, "height-mm", &ddata->height_mm);
>
> - ddata->vpnl = devm_regulator_get_optional(&dsi->dev, "vpnl");
> - if (IS_ERR(ddata->vpnl)) {
> - err = PTR_ERR(ddata->vpnl);
> - if (err == -EPROBE_DEFER)
> - return err;
> - ddata->vpnl = NULL;
> - }
> -
> - ddata->vddi = devm_regulator_get_optional(&dsi->dev, "vddi");
> - if (IS_ERR(ddata->vddi)) {
> - err = PTR_ERR(ddata->vddi);
> - if (err == -EPROBE_DEFER)
> - return err;
> - ddata->vddi = NULL;
> - }
> + ddata->supplies[0].supply = "vpnl";
> + ddata->supplies[1].supply = "vddi";
> + err = devm_regulator_bulk_get(&dsi->dev, DCS_REGULATOR_SUPPLY_NUM,
> + ddata->supplies);
> + if (err)
> + return err;
>
> backlight = devm_of_find_backlight(&dsi->dev);
> if (IS_ERR(backlight))
--
Regards,
Laurent Pinchart
