* [PATCH v4 0/4] ti-sn65dsi83 patches
@ 2021-11-18 9:19 Alexander Stein
2021-11-18 9:19 ` [PATCH v4 1/4] dt-bindings: display: bridge: sn65dsi83: Make enable GPIO optional Alexander Stein
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Alexander Stein @ 2021-11-18 9:19 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Rob Herring, Andrzej Hajda,
Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec
Cc: Alexander Stein, dri-devel, devicetree
Changes in V4 of this set:
* Rebased to next-20211118 (due to merge-conflict in linux-next)
* Added Rob Herring's Ack on Patch 1 & 3
* Reworked patch 4 due to other changes in linux-next
* Removed Sam Ravnborg's Reviewed-by for patch4 due to rework
Changes in V3 of this set:
* Do not require vcc-supply in bindings, making it purely optional
* Move regulator enable/disable to sn65dsi83_atomic_pre_enable and
sn65dsi83_atomic_disable
Changes in V2 of this set:
* Add patch from Laurent for fixing the binding regarding optional GPIO
* Reorder patches so bindings are changed beforehand
* Add small fixes from Sam's review
Alexander Stein (3):
drm/bridge: ti-sn65dsi83: Make enable GPIO optional
dt-bindings: drm/bridge: ti-sn65dsi83: Add vcc supply bindings
drm/bridge: ti-sn65dsi83: Add vcc supply regulator support
Laurent Pinchart (1):
dt-bindings: display: bridge: sn65dsi83: Make enable GPIO optional
.../bindings/display/bridge/ti,sn65dsi83.yaml | 5 ++++-
drivers/gpu/drm/bridge/ti-sn65dsi83.c | 22 ++++++++++++++++++-
2 files changed, 25 insertions(+), 2 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 1/4] dt-bindings: display: bridge: sn65dsi83: Make enable GPIO optional
2021-11-18 9:19 [PATCH v4 0/4] ti-sn65dsi83 patches Alexander Stein
@ 2021-11-18 9:19 ` Alexander Stein
2021-12-09 7:07 ` Jagan Teki
2021-11-18 9:19 ` [PATCH v4 2/4] drm/bridge: ti-sn65dsi83: " Alexander Stein
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Alexander Stein @ 2021-11-18 9:19 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Rob Herring, Andrzej Hajda,
Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec
Cc: Laurent Pinchart, dri-devel, devicetree, Rob Herring, Alexander Stein
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The SN65DSI8x EN signal may be tied to VCC, or otherwise controlled by
means not available to the kernel. Make the GPIO optional.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
.../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 1 -
1 file changed, 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
index b446d0f0f1b4..c3f3e73f740a 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
@@ -91,7 +91,6 @@ properties:
required:
- compatible
- reg
- - enable-gpios
- ports
allOf:
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 2/4] drm/bridge: ti-sn65dsi83: Make enable GPIO optional
2021-11-18 9:19 [PATCH v4 0/4] ti-sn65dsi83 patches Alexander Stein
2021-11-18 9:19 ` [PATCH v4 1/4] dt-bindings: display: bridge: sn65dsi83: Make enable GPIO optional Alexander Stein
@ 2021-11-18 9:19 ` Alexander Stein
2021-11-18 9:19 ` [PATCH v4 3/4] dt-bindings: drm/bridge: ti-sn65dsi83: Add vcc supply bindings Alexander Stein
2021-11-18 9:19 ` [PATCH v4 4/4] drm/bridge: ti-sn65dsi83: Add vcc supply regulator support Alexander Stein
3 siblings, 0 replies; 12+ messages in thread
From: Alexander Stein @ 2021-11-18 9:19 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Rob Herring, Andrzej Hajda,
Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec
Cc: Alexander Stein, dri-devel, devicetree, Laurent Pinchart, Sam Ravnborg
The enable signal may not be controllable by the kernel. Make it
optional.
This is a similar to commit bbda1704fc15 ("drm/bridge: ti-sn65dsi86: Make
enable GPIO optional")
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
drivers/gpu/drm/bridge/ti-sn65dsi83.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 945f08de45f1..065610edc37a 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -662,7 +662,8 @@ static int sn65dsi83_probe(struct i2c_client *client,
}
/* Put the chip in reset, pull EN line low, and assure 10ms reset low timing. */
- ctx->enable_gpio = devm_gpiod_get(ctx->dev, "enable", GPIOD_OUT_LOW);
+ ctx->enable_gpio = devm_gpiod_get_optional(ctx->dev, "enable",
+ GPIOD_OUT_LOW);
if (IS_ERR(ctx->enable_gpio))
return PTR_ERR(ctx->enable_gpio);
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 3/4] dt-bindings: drm/bridge: ti-sn65dsi83: Add vcc supply bindings
2021-11-18 9:19 [PATCH v4 0/4] ti-sn65dsi83 patches Alexander Stein
2021-11-18 9:19 ` [PATCH v4 1/4] dt-bindings: display: bridge: sn65dsi83: Make enable GPIO optional Alexander Stein
2021-11-18 9:19 ` [PATCH v4 2/4] drm/bridge: ti-sn65dsi83: " Alexander Stein
@ 2021-11-18 9:19 ` Alexander Stein
2021-12-09 7:08 ` Jagan Teki
2021-11-18 9:19 ` [PATCH v4 4/4] drm/bridge: ti-sn65dsi83: Add vcc supply regulator support Alexander Stein
3 siblings, 1 reply; 12+ messages in thread
From: Alexander Stein @ 2021-11-18 9:19 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Rob Herring, Andrzej Hajda,
Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec
Cc: Alexander Stein, dri-devel, devicetree, Sam Ravnborg, Rob Herring
Add a VCC regulator which needs to be enabled before the EN pin is
released.
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
.../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
index c3f3e73f740a..48a97bb3e2e0 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
@@ -32,6 +32,9 @@ properties:
maxItems: 1
description: GPIO specifier for bridge_en pin (active high).
+ vcc-supply:
+ description: A 1.8V power supply (see regulator/regulator.yaml).
+
ports:
$ref: /schemas/graph.yaml#/properties/ports
@@ -132,6 +135,7 @@ examples:
reg = <0x2d>;
enable-gpios = <&gpio2 1 GPIO_ACTIVE_HIGH>;
+ vcc-supply = <®_sn65dsi83_1v8>;
ports {
#address-cells = <1>;
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 4/4] drm/bridge: ti-sn65dsi83: Add vcc supply regulator support
2021-11-18 9:19 [PATCH v4 0/4] ti-sn65dsi83 patches Alexander Stein
` (2 preceding siblings ...)
2021-11-18 9:19 ` [PATCH v4 3/4] dt-bindings: drm/bridge: ti-sn65dsi83: Add vcc supply bindings Alexander Stein
@ 2021-11-18 9:19 ` Alexander Stein
2021-12-09 7:04 ` Jagan Teki
2021-12-09 16:37 ` Laurent Pinchart
3 siblings, 2 replies; 12+ messages in thread
From: Alexander Stein @ 2021-11-18 9:19 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Rob Herring, Andrzej Hajda,
Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec
Cc: Alexander Stein, dri-devel, devicetree
VCC needs to be enabled before releasing the enable GPIO.
Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
drivers/gpu/drm/bridge/ti-sn65dsi83.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
index 065610edc37a..54d18e82ed74 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
@@ -33,6 +33,7 @@
#include <linux/of_device.h>
#include <linux/of_graph.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_bridge.h>
@@ -143,6 +144,7 @@ struct sn65dsi83 {
struct mipi_dsi_device *dsi;
struct drm_bridge *panel_bridge;
struct gpio_desc *enable_gpio;
+ struct regulator *vcc;
int dsi_lanes;
bool lvds_dual_link;
bool lvds_dual_link_even_odd_swap;
@@ -337,6 +339,12 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
u16 val;
int ret;
+ ret = regulator_enable(ctx->vcc);
+ if (ret) {
+ dev_err(ctx->dev, "Failed to enable vcc\n");
+ return;
+ }
+
/* Deassert reset */
gpiod_set_value(ctx->enable_gpio, 1);
usleep_range(1000, 1100);
@@ -486,11 +494,16 @@ static void sn65dsi83_atomic_disable(struct drm_bridge *bridge,
struct drm_bridge_state *old_bridge_state)
{
struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
+ int ret;
/* Put the chip in reset, pull EN line low, and assure 10ms reset low timing. */
gpiod_set_value(ctx->enable_gpio, 0);
usleep_range(10000, 11000);
+ ret = regulator_disable(ctx->vcc);
+ if (ret)
+ dev_err(ctx->dev, "Failed to disable vcc: %i\n", ret);
+
regcache_mark_dirty(ctx->regmap);
}
@@ -599,6 +612,12 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
ctx->panel_bridge = panel_bridge;
+ ctx->vcc = devm_regulator_get(dev, "vcc");
+ if (IS_ERR(ctx->vcc))
+ return dev_err_probe(dev, PTR_ERR(ctx->vcc),
+ "Failed to get supply 'vcc': %pe\n",
+ ERR_PTR(ret));
+
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 4/4] drm/bridge: ti-sn65dsi83: Add vcc supply regulator support
2021-11-18 9:19 ` [PATCH v4 4/4] drm/bridge: ti-sn65dsi83: Add vcc supply regulator support Alexander Stein
@ 2021-12-09 7:04 ` Jagan Teki
2021-12-09 16:38 ` Laurent Pinchart
2021-12-09 16:37 ` Laurent Pinchart
1 sibling, 1 reply; 12+ messages in thread
From: Jagan Teki @ 2021-12-09 7:04 UTC (permalink / raw)
To: Alexander Stein
Cc: David Airlie, Daniel Vetter, Rob Herring, Andrzej Hajda,
Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, devicetree, dri-devel
On Thu, Nov 18, 2021 at 2:50 PM Alexander Stein
<alexander.stein@ew.tq-group.com> wrote:
>
> VCC needs to be enabled before releasing the enable GPIO.
>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> drivers/gpu/drm/bridge/ti-sn65dsi83.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index 065610edc37a..54d18e82ed74 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -33,6 +33,7 @@
> #include <linux/of_device.h>
> #include <linux/of_graph.h>
> #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_bridge.h>
> @@ -143,6 +144,7 @@ struct sn65dsi83 {
> struct mipi_dsi_device *dsi;
> struct drm_bridge *panel_bridge;
> struct gpio_desc *enable_gpio;
> + struct regulator *vcc;
> int dsi_lanes;
> bool lvds_dual_link;
> bool lvds_dual_link_even_odd_swap;
> @@ -337,6 +339,12 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
> u16 val;
> int ret;
>
> + ret = regulator_enable(ctx->vcc);
> + if (ret) {
> + dev_err(ctx->dev, "Failed to enable vcc\n");
> + return;
> + }
Better check the vcc and enable it since it is an optional one.
Jagan.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/4] dt-bindings: display: bridge: sn65dsi83: Make enable GPIO optional
2021-11-18 9:19 ` [PATCH v4 1/4] dt-bindings: display: bridge: sn65dsi83: Make enable GPIO optional Alexander Stein
@ 2021-12-09 7:07 ` Jagan Teki
2021-12-09 12:23 ` (EXT) " Alexander Stein
0 siblings, 1 reply; 12+ messages in thread
From: Jagan Teki @ 2021-12-09 7:07 UTC (permalink / raw)
To: Alexander Stein
Cc: David Airlie, Daniel Vetter, Rob Herring, Andrzej Hajda,
Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, devicetree, dri-devel
On Thu, Nov 18, 2021 at 2:50 PM Alexander Stein
<alexander.stein@ew.tq-group.com> wrote:
>
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> The SN65DSI8x EN signal may be tied to VCC, or otherwise controlled by
> means not available to the kernel. Make the GPIO optional.
Sorry, I couldn't understand what it means. Does it mean VCC enabled
designs no need to enable GPIO? I've a design that do support both EN
and VCC.
Jagan.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 3/4] dt-bindings: drm/bridge: ti-sn65dsi83: Add vcc supply bindings
2021-11-18 9:19 ` [PATCH v4 3/4] dt-bindings: drm/bridge: ti-sn65dsi83: Add vcc supply bindings Alexander Stein
@ 2021-12-09 7:08 ` Jagan Teki
0 siblings, 0 replies; 12+ messages in thread
From: Jagan Teki @ 2021-12-09 7:08 UTC (permalink / raw)
To: Alexander Stein
Cc: David Airlie, Daniel Vetter, Rob Herring, Andrzej Hajda,
Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, devicetree, Sam Ravnborg, dri-devel
On Thu, Nov 18, 2021 at 2:50 PM Alexander Stein
<alexander.stein@ew.tq-group.com> wrote:
>
> Add a VCC regulator which needs to be enabled before the EN pin is
> released.
>
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> .../devicetree/bindings/display/bridge/ti,sn65dsi83.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> index c3f3e73f740a..48a97bb3e2e0 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi83.yaml
> @@ -32,6 +32,9 @@ properties:
> maxItems: 1
> description: GPIO specifier for bridge_en pin (active high).
>
> + vcc-supply:
> + description: A 1.8V power supply (see regulator/regulator.yaml).
Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: (EXT) Re: [PATCH v4 1/4] dt-bindings: display: bridge: sn65dsi83: Make enable GPIO optional
2021-12-09 7:07 ` Jagan Teki
@ 2021-12-09 12:23 ` Alexander Stein
0 siblings, 0 replies; 12+ messages in thread
From: Alexander Stein @ 2021-12-09 12:23 UTC (permalink / raw)
To: Jagan Teki
Cc: David Airlie, Daniel Vetter, Rob Herring, Andrzej Hajda,
Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, devicetree, dri-devel
Am Donnerstag, dem 09.12.2021 um 12:37 +0530 schrieb Jagan Teki:
> On Thu, Nov 18, 2021 at 2:50 PM Alexander Stein
> <
> alexander.stein@ew.tq-group.com
> > wrote:
> > From: Laurent Pinchart <
> > laurent.pinchart@ideasonboard.com
> > >
> >
> > The SN65DSI8x EN signal may be tied to VCC, or otherwise controlled
> > by
> > means not available to the kernel. Make the GPIO optional.
>
> Sorry, I couldn't understand what it means. Does it mean VCC enabled
> designs no need to enable GPIO? I've a design that do support both EN
> and VCC.
The patches 1 & 2 are only about the "enable" gpio for the bridge, it's
unrelated to the VCC regulator introduced in patch 3 & 4.
Maybe the commit message should say:
> The SN65DSI8x EN signal may be hard-wired to VCC, or otherwise
controlled[...]
But I copied the message from bbda1704fc15 ("drm/bridge: ti-sn65dsi86:
Make enable GPIO optional").
This is for use-cases where there is no GPIO the kernel can use, to
control the EN pad of the bridge. Thus make this gpio optional in
bindings and driver.
HTH
Alexander
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 4/4] drm/bridge: ti-sn65dsi83: Add vcc supply regulator support
2021-11-18 9:19 ` [PATCH v4 4/4] drm/bridge: ti-sn65dsi83: Add vcc supply regulator support Alexander Stein
2021-12-09 7:04 ` Jagan Teki
@ 2021-12-09 16:37 ` Laurent Pinchart
1 sibling, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2021-12-09 16:37 UTC (permalink / raw)
To: Alexander Stein
Cc: David Airlie, Daniel Vetter, Rob Herring, Andrzej Hajda,
Neil Armstrong, Robert Foss, Jonas Karlman, Jernej Skrabec,
dri-devel, devicetree
Hi Alexander,
Thank you for the patch.
On Thu, Nov 18, 2021 at 10:19:55AM +0100, Alexander Stein wrote:
> VCC needs to be enabled before releasing the enable GPIO.
>
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> drivers/gpu/drm/bridge/ti-sn65dsi83.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index 065610edc37a..54d18e82ed74 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -33,6 +33,7 @@
> #include <linux/of_device.h>
> #include <linux/of_graph.h>
> #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_bridge.h>
> @@ -143,6 +144,7 @@ struct sn65dsi83 {
> struct mipi_dsi_device *dsi;
> struct drm_bridge *panel_bridge;
> struct gpio_desc *enable_gpio;
> + struct regulator *vcc;
> int dsi_lanes;
> bool lvds_dual_link;
> bool lvds_dual_link_even_odd_swap;
> @@ -337,6 +339,12 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
> u16 val;
> int ret;
>
> + ret = regulator_enable(ctx->vcc);
> + if (ret) {
> + dev_err(ctx->dev, "Failed to enable vcc\n");
I'd print the error code here as you do so in
sn65dsi83_atomic_disable().
> + return;
> + }
> +
> /* Deassert reset */
> gpiod_set_value(ctx->enable_gpio, 1);
> usleep_range(1000, 1100);
> @@ -486,11 +494,16 @@ static void sn65dsi83_atomic_disable(struct drm_bridge *bridge,
> struct drm_bridge_state *old_bridge_state)
> {
> struct sn65dsi83 *ctx = bridge_to_sn65dsi83(bridge);
> + int ret;
>
> /* Put the chip in reset, pull EN line low, and assure 10ms reset low timing. */
> gpiod_set_value(ctx->enable_gpio, 0);
> usleep_range(10000, 11000);
>
> + ret = regulator_disable(ctx->vcc);
> + if (ret)
> + dev_err(ctx->dev, "Failed to disable vcc: %i\n", ret);
I wish printf didn't have identical %i and %d specifiers :-)
> +
> regcache_mark_dirty(ctx->regmap);
> }
>
> @@ -599,6 +612,12 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
>
> ctx->panel_bridge = panel_bridge;
>
> + ctx->vcc = devm_regulator_get(dev, "vcc");
> + if (IS_ERR(ctx->vcc))
> + return dev_err_probe(dev, PTR_ERR(ctx->vcc),
> + "Failed to get supply 'vcc': %pe\n",
> + ERR_PTR(ret));
This doesn't seem right, ret doesn't contain any useful error code at
this point.
return dev_err_probe(dev, PTR_ERR(ctx->vcc),
"Failed to get supply 'vcc'\n");
should be enough, as dev_err_probe() adds the error to the message
internally.
With those small fixes,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +
> return 0;
> }
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 4/4] drm/bridge: ti-sn65dsi83: Add vcc supply regulator support
2021-12-09 7:04 ` Jagan Teki
@ 2021-12-09 16:38 ` Laurent Pinchart
2021-12-09 16:44 ` Jagan Teki
0 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2021-12-09 16:38 UTC (permalink / raw)
To: Jagan Teki
Cc: Alexander Stein, David Airlie, Daniel Vetter, Rob Herring,
Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman,
Jernej Skrabec, devicetree, dri-devel
Hi Jagan,
On Thu, Dec 09, 2021 at 12:34:49PM +0530, Jagan Teki wrote:
> On Thu, Nov 18, 2021 at 2:50 PM Alexander Stein wrote:
> >
> > VCC needs to be enabled before releasing the enable GPIO.
> >
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > drivers/gpu/drm/bridge/ti-sn65dsi83.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > index 065610edc37a..54d18e82ed74 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > @@ -33,6 +33,7 @@
> > #include <linux/of_device.h>
> > #include <linux/of_graph.h>
> > #include <linux/regmap.h>
> > +#include <linux/regulator/consumer.h>
> >
> > #include <drm/drm_atomic_helper.h>
> > #include <drm/drm_bridge.h>
> > @@ -143,6 +144,7 @@ struct sn65dsi83 {
> > struct mipi_dsi_device *dsi;
> > struct drm_bridge *panel_bridge;
> > struct gpio_desc *enable_gpio;
> > + struct regulator *vcc;
> > int dsi_lanes;
> > bool lvds_dual_link;
> > bool lvds_dual_link_even_odd_swap;
> > @@ -337,6 +339,12 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
> > u16 val;
> > int ret;
> >
> > + ret = regulator_enable(ctx->vcc);
> > + if (ret) {
> > + dev_err(ctx->dev, "Failed to enable vcc\n");
> > + return;
> > + }
>
> Better check the vcc and enable it since it is an optional one.
Won't the regulator core create a dummy regulator if none is specified
in DT ?
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 4/4] drm/bridge: ti-sn65dsi83: Add vcc supply regulator support
2021-12-09 16:38 ` Laurent Pinchart
@ 2021-12-09 16:44 ` Jagan Teki
0 siblings, 0 replies; 12+ messages in thread
From: Jagan Teki @ 2021-12-09 16:44 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Alexander Stein, David Airlie, Daniel Vetter, Rob Herring,
Andrzej Hajda, Neil Armstrong, Robert Foss, Jonas Karlman,
Jernej Skrabec, devicetree, dri-devel
Hi Laurent,
On Thu, Dec 9, 2021 at 10:09 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Jagan,
>
> On Thu, Dec 09, 2021 at 12:34:49PM +0530, Jagan Teki wrote:
> > On Thu, Nov 18, 2021 at 2:50 PM Alexander Stein wrote:
> > >
> > > VCC needs to be enabled before releasing the enable GPIO.
> > >
> > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > > ---
> > > drivers/gpu/drm/bridge/ti-sn65dsi83.c | 19 +++++++++++++++++++
> > > 1 file changed, 19 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > index 065610edc37a..54d18e82ed74 100644
> > > --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> > > @@ -33,6 +33,7 @@
> > > #include <linux/of_device.h>
> > > #include <linux/of_graph.h>
> > > #include <linux/regmap.h>
> > > +#include <linux/regulator/consumer.h>
> > >
> > > #include <drm/drm_atomic_helper.h>
> > > #include <drm/drm_bridge.h>
> > > @@ -143,6 +144,7 @@ struct sn65dsi83 {
> > > struct mipi_dsi_device *dsi;
> > > struct drm_bridge *panel_bridge;
> > > struct gpio_desc *enable_gpio;
> > > + struct regulator *vcc;
> > > int dsi_lanes;
> > > bool lvds_dual_link;
> > > bool lvds_dual_link_even_odd_swap;
> > > @@ -337,6 +339,12 @@ static void sn65dsi83_atomic_enable(struct drm_bridge *bridge,
> > > u16 val;
> > > int ret;
> > >
> > > + ret = regulator_enable(ctx->vcc);
> > > + if (ret) {
> > > + dev_err(ctx->dev, "Failed to enable vcc\n");
> > > + return;
> > > + }
> >
> > Better check the vcc and enable it since it is an optional one.
>
> Won't the regulator core create a dummy regulator if none is specified
> in DT ?
Agreed, thanks (Usually I do check to avoid NULL pointer if any).
Jagan.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-12-09 16:47 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-18 9:19 [PATCH v4 0/4] ti-sn65dsi83 patches Alexander Stein
2021-11-18 9:19 ` [PATCH v4 1/4] dt-bindings: display: bridge: sn65dsi83: Make enable GPIO optional Alexander Stein
2021-12-09 7:07 ` Jagan Teki
2021-12-09 12:23 ` (EXT) " Alexander Stein
2021-11-18 9:19 ` [PATCH v4 2/4] drm/bridge: ti-sn65dsi83: " Alexander Stein
2021-11-18 9:19 ` [PATCH v4 3/4] dt-bindings: drm/bridge: ti-sn65dsi83: Add vcc supply bindings Alexander Stein
2021-12-09 7:08 ` Jagan Teki
2021-11-18 9:19 ` [PATCH v4 4/4] drm/bridge: ti-sn65dsi83: Add vcc supply regulator support Alexander Stein
2021-12-09 7:04 ` Jagan Teki
2021-12-09 16:38 ` Laurent Pinchart
2021-12-09 16:44 ` Jagan Teki
2021-12-09 16:37 ` Laurent Pinchart
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).