Re: [PATCH] omapfb: Fix regulator API abuse in dss.c and hdmi4.c

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2195 bytes --]

On 31/03/16 19:49, Mark Brown wrote:
> On Thu, Mar 31, 2016 at 09:30:11AM +0300, Tomi Valkeinen wrote:
> 
>> This code did fix an issue, see 02b7a32083b9930543663720758de249b4f6a2a3.
> 
> That change isn't sensible, especially the _can_change_voltage() like I
> said in the commit log.

I may remember wrong, but I think regulator_set_voltage() failed if
regulator_can_change_voltage() returned false. So I ended up having the
'if' there. But I may remember wrong, or maybe it's been changed since.

>> Now, even at the time when I wrote that fix, it did feel a bit odd to
>> me. I have to say I don't remember the discussion that led to the patch,
>> perhaps it was something along "yes, the driver should not need to do
>> that, but for the time being do it".
> 
> That wasn't a discusion I was involved in, a quick google suggests it
> might've been off-list.

That's possible. With quick googling, this may have longer history than
the patch I sent.

>> So where are these "machine constraints" defined?
> 
> They're the DT.  Your machine constraints just seem broken and need to
> be fixed, most likely whoever wrote the constraints for the platform
> completely failed to understand the purpose of constraints and filled in
> the maximum range of voltages that the regulator in use on the board can
> support.

Ok.

Tero, Tony, with a quick look, for example omap5-board-common.dtsi
defines ldo4_reg having range from 1.5 to 1.8. That should be changed to
1.8 only, right? ldo1_reg has a range too, I'm not familiar with that
LDO's use, but it's for camera so my guess is that it should be 1.8 too.

I can have a more thorough look tomorrow, and do a test run on omap5
uevm (with Mark's patch).

I wonder why we have the same code in hdmi4. Again with a quick look,
omap4 boards seem to use vdac for hdmi, and vdac doesn't have any
constraints in twl6030.dtsi, so I presume it's a fixed-voltage.

Anyway, I'm happy to apply this patch (and we need similar for hdmi5,
and also for omapdrm), we just need to do any necessary fixes to the
.dts first.

Although strictly speaking, I guess that's breaking backward
compatibility...

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]