| Message ID | 20211102093618.114928-2-angelogioacchino.delregno@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/3] drm/bridge: parade-ps8640: Don't try to enable VDO if poweron fails | expand |
On 02.11.21 11:36, AngeloGioacchino Del Regno wrote: > In preparation for varying the poweron error handling in function > ps8640_bridge_poweron(), move function ps8640_bridge_poweroff() up > and also move the actual logic to power off the chip to a new > __ps8640_bridge_poweroff() function. > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- > drivers/gpu/drm/bridge/parade-ps8640.c | 37 ++++++++++++++------------ > 1 file changed, 20 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c > index 8c5402947b3c..41f5d511d516 100644 > --- a/drivers/gpu/drm/bridge/parade-ps8640.c > +++ b/drivers/gpu/drm/bridge/parade-ps8640.c > @@ -293,6 +293,26 @@ static int ps8640_bridge_vdo_control(struct ps8640 *ps_bridge, > return 0; > } > > +static void __ps8640_bridge_poweroff(struct ps8640 *ps_bridge) > +{ > + gpiod_set_value(ps_bridge->gpio_reset, 1); > + gpiod_set_value(ps_bridge->gpio_powerdown, 1); > + if (regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies), > + ps_bridge->supplies)) { > + DRM_ERROR("cannot disable regulators\n"); > + } That '{' is redundant Thanks, Danfa > +} > + > +static void ps8640_bridge_poweroff(struct ps8640 *ps_bridge) > +{ > + if (!ps_bridge->powered) > + return; > + > + __ps8640_bridge_poweroff(ps_bridge); > + > + ps_bridge->powered = false; > +} > + > static int ps8640_bridge_poweron(struct ps8640 *ps_bridge) > { > struct regmap *map = ps_bridge->regmap[PAGE2_TOP_CNTL]; > @@ -361,23 +381,6 @@ static int ps8640_bridge_poweron(struct ps8640 *ps_bridge) > return ret; > } > > -static void ps8640_bridge_poweroff(struct ps8640 *ps_bridge) > -{ > - int ret; > - > - if (!ps_bridge->powered) > - return; > - > - gpiod_set_value(ps_bridge->gpio_reset, 1); > - gpiod_set_value(ps_bridge->gpio_powerdown, 1); > - ret = regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies), > - ps_bridge->supplies); > - if (ret < 0) > - DRM_ERROR("cannot disable regulators %d\n", ret); > - > - ps_bridge->powered = false; > -} > - > static void ps8640_pre_enable(struct drm_bridge *bridge) > { > struct ps8640 *ps_bridge = bridge_to_ps8640(bridge); >
Il 10/11/21 13:44, Dafna Hirschfeld ha scritto: > > > On 02.11.21 11:36, AngeloGioacchino Del Regno wrote: >> In preparation for varying the poweron error handling in function >> ps8640_bridge_poweron(), move function ps8640_bridge_poweroff() up >> and also move the actual logic to power off the chip to a new >> __ps8640_bridge_poweroff() function. >> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >> --- >> drivers/gpu/drm/bridge/parade-ps8640.c | 37 ++++++++++++++------------ >> 1 file changed, 20 insertions(+), 17 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c >> b/drivers/gpu/drm/bridge/parade-ps8640.c >> index 8c5402947b3c..41f5d511d516 100644 >> --- a/drivers/gpu/drm/bridge/parade-ps8640.c >> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c >> @@ -293,6 +293,26 @@ static int ps8640_bridge_vdo_control(struct ps8640 *ps_bridge, >> return 0; >> } >> +static void __ps8640_bridge_poweroff(struct ps8640 *ps_bridge) >> +{ >> + gpiod_set_value(ps_bridge->gpio_reset, 1); >> + gpiod_set_value(ps_bridge->gpio_powerdown, 1); >> + if (regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies), >> + ps_bridge->supplies)) { >> + DRM_ERROR("cannot disable regulators\n"); >> + } > > That '{' is redundant > > Thanks, > Danfa > Hi Dafna, the braces were added as a way to increase human readability. Cheers, - Angelo
Hey Angelo, On Wed, 10 Nov 2021 at 13:46, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Il 10/11/21 13:44, Dafna Hirschfeld ha scritto: > > > > > > On 02.11.21 11:36, AngeloGioacchino Del Regno wrote: > >> In preparation for varying the poweron error handling in function > >> ps8640_bridge_poweron(), move function ps8640_bridge_poweroff() up > >> and also move the actual logic to power off the chip to a new > >> __ps8640_bridge_poweroff() function. > >> > >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > >> --- > >> drivers/gpu/drm/bridge/parade-ps8640.c | 37 ++++++++++++++------------ > >> 1 file changed, 20 insertions(+), 17 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c > >> b/drivers/gpu/drm/bridge/parade-ps8640.c > >> index 8c5402947b3c..41f5d511d516 100644 > >> --- a/drivers/gpu/drm/bridge/parade-ps8640.c > >> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c > >> @@ -293,6 +293,26 @@ static int ps8640_bridge_vdo_control(struct ps8640 *ps_bridge, > >> return 0; > >> } > >> +static void __ps8640_bridge_poweroff(struct ps8640 *ps_bridge) > >> +{ > >> + gpiod_set_value(ps_bridge->gpio_reset, 1); > >> + gpiod_set_value(ps_bridge->gpio_powerdown, 1); > >> + if (regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies), > >> + ps_bridge->supplies)) { > >> + DRM_ERROR("cannot disable regulators\n"); > >> + } > > > > That '{' is redundant > > > > Thanks, > > Danfa > > > > Hi Dafna, > the braces were added as a way to increase human readability. Not to bikeshed this, but the kernel style guide is clear about this. No unneeded braces should be used where a single statement will do.
Il 19/11/21 14:19, Robert Foss ha scritto: > Hey Angelo, > > On Wed, 10 Nov 2021 at 13:46, AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> wrote: >> >> Il 10/11/21 13:44, Dafna Hirschfeld ha scritto: >>> >>> >>> On 02.11.21 11:36, AngeloGioacchino Del Regno wrote: >>>> In preparation for varying the poweron error handling in function >>>> ps8640_bridge_poweron(), move function ps8640_bridge_poweroff() up >>>> and also move the actual logic to power off the chip to a new >>>> __ps8640_bridge_poweroff() function. >>>> >>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >>>> --- >>>> drivers/gpu/drm/bridge/parade-ps8640.c | 37 ++++++++++++++------------ >>>> 1 file changed, 20 insertions(+), 17 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c >>>> b/drivers/gpu/drm/bridge/parade-ps8640.c >>>> index 8c5402947b3c..41f5d511d516 100644 >>>> --- a/drivers/gpu/drm/bridge/parade-ps8640.c >>>> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c >>>> @@ -293,6 +293,26 @@ static int ps8640_bridge_vdo_control(struct ps8640 *ps_bridge, >>>> return 0; >>>> } >>>> +static void __ps8640_bridge_poweroff(struct ps8640 *ps_bridge) >>>> +{ >>>> + gpiod_set_value(ps_bridge->gpio_reset, 1); >>>> + gpiod_set_value(ps_bridge->gpio_powerdown, 1); >>>> + if (regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies), >>>> + ps_bridge->supplies)) { >>>> + DRM_ERROR("cannot disable regulators\n"); >>>> + } >>> >>> That '{' is redundant >>> >>> Thanks, >>> Danfa >>> >> >> Hi Dafna, >> the braces were added as a way to increase human readability. > > Not to bikeshed this, but the kernel style guide is clear about this. > No unneeded braces should be used where a single statement will do. > Hey Robert! That's right, style rules come before personal preferences. I'll send a v3 without the braces as soon as possible! Cheers, - Angelo
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c index 8c5402947b3c..41f5d511d516 100644 --- a/drivers/gpu/drm/bridge/parade-ps8640.c +++ b/drivers/gpu/drm/bridge/parade-ps8640.c @@ -293,6 +293,26 @@ static int ps8640_bridge_vdo_control(struct ps8640 *ps_bridge, return 0; } +static void __ps8640_bridge_poweroff(struct ps8640 *ps_bridge) +{ + gpiod_set_value(ps_bridge->gpio_reset, 1); + gpiod_set_value(ps_bridge->gpio_powerdown, 1); + if (regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies), + ps_bridge->supplies)) { + DRM_ERROR("cannot disable regulators\n"); + } +} + +static void ps8640_bridge_poweroff(struct ps8640 *ps_bridge) +{ + if (!ps_bridge->powered) + return; + + __ps8640_bridge_poweroff(ps_bridge); + + ps_bridge->powered = false; +} + static int ps8640_bridge_poweron(struct ps8640 *ps_bridge) { struct regmap *map = ps_bridge->regmap[PAGE2_TOP_CNTL]; @@ -361,23 +381,6 @@ static int ps8640_bridge_poweron(struct ps8640 *ps_bridge) return ret; } -static void ps8640_bridge_poweroff(struct ps8640 *ps_bridge) -{ - int ret; - - if (!ps_bridge->powered) - return; - - gpiod_set_value(ps_bridge->gpio_reset, 1); - gpiod_set_value(ps_bridge->gpio_powerdown, 1); - ret = regulator_bulk_disable(ARRAY_SIZE(ps_bridge->supplies), - ps_bridge->supplies); - if (ret < 0) - DRM_ERROR("cannot disable regulators %d\n", ret); - - ps_bridge->powered = false; -} - static void ps8640_pre_enable(struct drm_bridge *bridge) { struct ps8640 *ps_bridge = bridge_to_ps8640(bridge);
In preparation for varying the poweron error handling in function ps8640_bridge_poweron(), move function ps8640_bridge_poweroff() up and also move the actual logic to power off the chip to a new __ps8640_bridge_poweroff() function. Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> --- drivers/gpu/drm/bridge/parade-ps8640.c | 37 ++++++++++++++------------ 1 file changed, 20 insertions(+), 17 deletions(-)