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:
signature.asc
Description: OpenPGP digital signature
