[PATCH] drm/imx: imx-tve: Make the 'dac' regulator optional

[PATCH] drm/imx: imx-tve: Make the 'dac' regulator optional

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@nxp.com>

Commit deb65870b5d9d ("drm/imx: imx-tve: check the value returned by
regulator_set_voltage()") exposes the following probe issue:

63ff0000.tve supply dac not found, using dummy regulator
imx-drm display-subsystem: failed to bind 63ff0000.tve (ops imx_tve_ops): -22

When the 'dac' regulator is not passed in the device tree,
devm_regulator_get() will return NULL and when regulator_set_voltage()
is called it returns an error.

Fix the issue by making the 'dac' regulator optional.

Fixes: deb65870b5d9d ("drm/imx: imx-tve: check the value returned by regulator_set_voltage()")
Cc: <stable@vger.kernel.org> # 4.8+
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 drivers/gpu/drm/imx/imx-tve.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
index 3b602ee..cec0e12 100644
--- a/drivers/gpu/drm/imx/imx-tve.c
+++ b/drivers/gpu/drm/imx/imx-tve.c
@@ -619,7 +619,7 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data)
 		return ret;
 	}
 
-	tve->dac_reg = devm_regulator_get(dev, "dac");
+	tve->dac_reg = devm_regulator_get_optional(dev, "dac");
 	if (!IS_ERR(tve->dac_reg)) {
 		ret = regulator_set_voltage(tve->dac_reg, 2750000, 2750000);
 		if (ret)
-- 
2.7.4

[PATCH] drm/imx: imx-tve: Make the 'dac' regulator optional

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@nxp.com>

Commit deb65870b5d9d ("drm/imx: imx-tve: check the value returned by
regulator_set_voltage()") exposes the following probe issue:

63ff0000.tve supply dac not found, using dummy regulator
imx-drm display-subsystem: failed to bind 63ff0000.tve (ops imx_tve_ops): -22

When the 'dac' regulator is not passed in the device tree,
devm_regulator_get() will return NULL and when regulator_set_voltage()
is called it returns an error.

Fix the issue by making the 'dac' regulator optional.

Fixes: deb65870b5d9d ("drm/imx: imx-tve: check the value returned by regulator_set_voltage()")
Cc: <stable@vger.kernel.org> # 4.8+
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 drivers/gpu/drm/imx/imx-tve.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
index 3b602ee..cec0e12 100644
--- a/drivers/gpu/drm/imx/imx-tve.c
+++ b/drivers/gpu/drm/imx/imx-tve.c
@@ -619,7 +619,7 @@ static int imx_tve_bind(struct device *dev, struct device *master, void *data)
 		return ret;
 	}
 
-	tve->dac_reg = devm_regulator_get(dev, "dac");
+	tve->dac_reg = devm_regulator_get_optional(dev, "dac");
 	if (!IS_ERR(tve->dac_reg)) {
 		ret = regulator_set_voltage(tve->dac_reg, 2750000, 2750000);
 		if (ret)
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/imx: imx-tve: Make the 'dac' regulator optional

[Fabio Estevam]

Hi Philipp,

On Tue, Jan 3, 2017 at 5:11 PM, Fabio Estevam <festevam@gmail.com> wrote:
> From: Fabio Estevam <fabio.estevam@nxp.com>
>
> Commit deb65870b5d9d ("drm/imx: imx-tve: check the value returned by
> regulator_set_voltage()") exposes the following probe issue:
>
> 63ff0000.tve supply dac not found, using dummy regulator
> imx-drm display-subsystem: failed to bind 63ff0000.tve (ops imx_tve_ops): -22
>
> When the 'dac' regulator is not passed in the device tree,
> devm_regulator_get() will return NULL and when regulator_set_voltage()
> is called it returns an error.
>
> Fix the issue by making the 'dac' regulator optional.
>
> Fixes: deb65870b5d9d ("drm/imx: imx-tve: check the value returned by regulator_set_voltage()")
> Cc: <stable@vger.kernel.org> # 4.8+
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>

Any comments, please?

Re: [PATCH] drm/imx: imx-tve: Make the 'dac' regulator optional

[Philipp Zabel]

On Fri, 2017-02-03 at 10:52 -0200, Fabio Estevam wrote:
> Hi Philipp,
> 
> On Tue, Jan 3, 2017 at 5:11 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > From: Fabio Estevam <fabio.estevam@nxp.com>
> >
> > Commit deb65870b5d9d ("drm/imx: imx-tve: check the value returned by
> > regulator_set_voltage()") exposes the following probe issue:
> >
> > 63ff0000.tve supply dac not found, using dummy regulator
> > imx-drm display-subsystem: failed to bind 63ff0000.tve (ops imx_tve_ops): -22
> >
> > When the 'dac' regulator is not passed in the device tree,
> > devm_regulator_get() will return NULL and when regulator_set_voltage()
> > is called it returns an error.
> >
> > Fix the issue by making the 'dac' regulator optional.
> >
> > Fixes: deb65870b5d9d ("drm/imx: imx-tve: check the value returned by regulator_set_voltage()")
> > Cc: <stable@vger.kernel.org> # 4.8+
> > Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> 
> Any comments, please?

I've applied this to the fixes branch, since the current device trees
don't have the regulator set.

Is this really optional, though? It would be better to add the correct
dac-supply to the device trees.

regards
Philipp

Re: [PATCH] drm/imx: imx-tve: Make the 'dac' regulator optional

[Philipp Zabel]

On Fri, 2017-02-03 at 10:52 -0200, Fabio Estevam wrote:
> Hi Philipp,
> 
> On Tue, Jan 3, 2017 at 5:11 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > From: Fabio Estevam <fabio.estevam@nxp.com>
> >
> > Commit deb65870b5d9d ("drm/imx: imx-tve: check the value returned by
> > regulator_set_voltage()") exposes the following probe issue:
> >
> > 63ff0000.tve supply dac not found, using dummy regulator
> > imx-drm display-subsystem: failed to bind 63ff0000.tve (ops imx_tve_ops): -22
> >
> > When the 'dac' regulator is not passed in the device tree,
> > devm_regulator_get() will return NULL and when regulator_set_voltage()
> > is called it returns an error.
> >
> > Fix the issue by making the 'dac' regulator optional.
> >
> > Fixes: deb65870b5d9d ("drm/imx: imx-tve: check the value returned by regulator_set_voltage()")
> > Cc: <stable@vger.kernel.org> # 4.8+
> > Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> 
> Any comments, please?

I've applied this to the fixes branch, since the current device trees
don't have the regulator set.

Is this really optional, though? It would be better to add the correct
dac-supply to the device trees.

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/imx: imx-tve: Make the 'dac' regulator optional

[Fabio Estevam]

Hi Philipp,

On Tue, Feb 7, 2017 at 2:45 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:

> I've applied this to the fixes branch, since the current device trees
> don't have the regulator set.

I have sent a patch providing the dac-supply regulator for imx53-qsb:
https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=for-next&id=042f6d98261d8edc225237acaeb627aeadbf54ad

> Is this really optional, though? It would be better to add the correct
> dac-supply to the device trees.

The other TVE user is imx53-mba53.dts, but as I don't have its
schematics I am not able to provide the regulator for it.

Thanks

Re: [PATCH] drm/imx: imx-tve: Make the 'dac' regulator optional

[Lucas Stach]

Am Dienstag, den 07.02.2017, 17:45 +0100 schrieb Philipp Zabel:
> On Fri, 2017-02-03 at 10:52 -0200, Fabio Estevam wrote:
> > Hi Philipp,
> > 
> > On Tue, Jan 3, 2017 at 5:11 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > > From: Fabio Estevam <fabio.estevam@nxp.com>
> > >
> > > Commit deb65870b5d9d ("drm/imx: imx-tve: check the value returned by
> > > regulator_set_voltage()") exposes the following probe issue:
> > >
> > > 63ff0000.tve supply dac not found, using dummy regulator
> > > imx-drm display-subsystem: failed to bind 63ff0000.tve (ops imx_tve_ops): -22
> > >
> > > When the 'dac' regulator is not passed in the device tree,
> > > devm_regulator_get() will return NULL and when regulator_set_voltage()
> > > is called it returns an error.
> > >
> > > Fix the issue by making the 'dac' regulator optional.
> > >
> > > Fixes: deb65870b5d9d ("drm/imx: imx-tve: check the value returned by regulator_set_voltage()")
> > > Cc: <stable@vger.kernel.org> # 4.8+
> > > Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> > 
> > Any comments, please?
> 
> I've applied this to the fixes branch, since the current device trees
> don't have the regulator set.
> 
> Is this really optional, though? It would be better to add the correct
> dac-supply to the device trees.
> 
Why does the driver attempt to set the voltage? I guess the voltage is
fixed, even if it is hooked up to a configurable regulator.

Shouldn't we just remove the set_voltage call from the driver and make
sure the correct voltage is supplied by board level DT constraints?

Regards,
Lucas

Re: [PATCH] drm/imx: imx-tve: Make the 'dac' regulator optional

[Philipp Zabel]

On Tue, 2017-02-07 at 18:09 +0100, Lucas Stach wrote:
> Am Dienstag, den 07.02.2017, 17:45 +0100 schrieb Philipp Zabel:
> > On Fri, 2017-02-03 at 10:52 -0200, Fabio Estevam wrote:
> > > Hi Philipp,
> > > 
> > > On Tue, Jan 3, 2017 at 5:11 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > > > From: Fabio Estevam <fabio.estevam@nxp.com>
> > > >
> > > > Commit deb65870b5d9d ("drm/imx: imx-tve: check the value returned by
> > > > regulator_set_voltage()") exposes the following probe issue:
> > > >
> > > > 63ff0000.tve supply dac not found, using dummy regulator
> > > > imx-drm display-subsystem: failed to bind 63ff0000.tve (ops imx_tve_ops): -22
> > > >
> > > > When the 'dac' regulator is not passed in the device tree,
> > > > devm_regulator_get() will return NULL and when regulator_set_voltage()
> > > > is called it returns an error.
> > > >
> > > > Fix the issue by making the 'dac' regulator optional.
> > > >
> > > > Fixes: deb65870b5d9d ("drm/imx: imx-tve: check the value returned by regulator_set_voltage()")
> > > > Cc: <stable@vger.kernel.org> # 4.8+
> > > > Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> > > 
> > > Any comments, please?
> > 
> > I've applied this to the fixes branch, since the current device trees
> > don't have the regulator set.
> > 
> > Is this really optional, though? It would be better to add the correct
> > dac-supply to the device trees.
> > 
> Why does the driver attempt to set the voltage? I guess the voltage is
> fixed, even if it is hooked up to a configurable regulator.

Good point, I suppose what the driver should really do is warn if the
voltage not set correctly?

> Shouldn't we just remove the set_voltage call from the driver and make
> sure the correct voltage is supplied by board level DT constraints?

I think so, yes.

regards
Philipp

Re: [PATCH] drm/imx: imx-tve: Make the 'dac' regulator optional

[Philipp Zabel]

On Tue, 2017-02-07 at 18:09 +0100, Lucas Stach wrote:
> Am Dienstag, den 07.02.2017, 17:45 +0100 schrieb Philipp Zabel:
> > On Fri, 2017-02-03 at 10:52 -0200, Fabio Estevam wrote:
> > > Hi Philipp,
> > > 
> > > On Tue, Jan 3, 2017 at 5:11 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > > > From: Fabio Estevam <fabio.estevam@nxp.com>
> > > >
> > > > Commit deb65870b5d9d ("drm/imx: imx-tve: check the value returned by
> > > > regulator_set_voltage()") exposes the following probe issue:
> > > >
> > > > 63ff0000.tve supply dac not found, using dummy regulator
> > > > imx-drm display-subsystem: failed to bind 63ff0000.tve (ops imx_tve_ops): -22
> > > >
> > > > When the 'dac' regulator is not passed in the device tree,
> > > > devm_regulator_get() will return NULL and when regulator_set_voltage()
> > > > is called it returns an error.
> > > >
> > > > Fix the issue by making the 'dac' regulator optional.
> > > >
> > > > Fixes: deb65870b5d9d ("drm/imx: imx-tve: check the value returned by regulator_set_voltage()")
> > > > Cc: <stable@vger.kernel.org> # 4.8+
> > > > Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> > > 
> > > Any comments, please?
> > 
> > I've applied this to the fixes branch, since the current device trees
> > don't have the regulator set.
> > 
> > Is this really optional, though? It would be better to add the correct
> > dac-supply to the device trees.
> > 
> Why does the driver attempt to set the voltage? I guess the voltage is
> fixed, even if it is hooked up to a configurable regulator.

Good point, I suppose what the driver should really do is warn if the
voltage not set correctly?

> Shouldn't we just remove the set_voltage call from the driver and make
> sure the correct voltage is supplied by board level DT constraints?

I think so, yes.

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/imx: imx-tve: Make the 'dac' regulator optional

[Philipp Zabel]

On Tue, 2017-02-07 at 14:52 -0200, Fabio Estevam wrote:
> Hi Philipp,
> 
> On Tue, Feb 7, 2017 at 2:45 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> 
> > I've applied this to the fixes branch, since the current device trees
> > don't have the regulator set.
> 
> I have sent a patch providing the dac-supply regulator for imx53-qsb:
> https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=for-next&id=042f6d98261d8edc225237acaeb627aeadbf54ad

Thanks, I've commented there.

regards
Philipp

Re: [PATCH] drm/imx: imx-tve: Make the 'dac' regulator optional

[Philipp Zabel]

On Tue, 2017-02-07 at 14:52 -0200, Fabio Estevam wrote:
> Hi Philipp,
> 
> On Tue, Feb 7, 2017 at 2:45 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> 
> > I've applied this to the fixes branch, since the current device trees
> > don't have the regulator set.
> 
> I have sent a patch providing the dac-supply regulator for imx53-qsb:
> https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=for-next&id=042f6d98261d8edc225237acaeb627aeadbf54ad

Thanks, I've commented there.

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/imx: imx-tve: Make the 'dac' regulator optional

[Fabio Estevam]

On Wed, Feb 8, 2017 at 7:52 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:

> Good point, I suppose what the driver should really do is warn if the
> voltage not set correctly?

Yes, I can do as suggested. Will prepare a patch. Thanks

Re: [PATCH] drm/imx: imx-tve: Make the 'dac' regulator optional

[Fabio Estevam]

On Wed, Feb 8, 2017 at 7:52 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:

> Good point, I suppose what the driver should really do is warn if the
> voltage not set correctly?

Yes, I can do as suggested. Will prepare a patch. Thanks
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel