Messages in this thread | | | Date | Wed, 11 Feb 2015 09:21:32 -0800 | From | Tim Bird <> | Subject | Re: [PATCH] regulator: Support different config and dev of_nodes in regulator_register |
| |
On 02/06/2015 11:56 AM, Tim Bird wrote: > > > On 02/06/2015 03:49 AM, Mark Brown wrote: >> On Thu, Feb 05, 2015 at 04:52:40PM -0800, Bjorn Andersson wrote: >>> On Thu 05 Feb 16:32 PST 2015, Mark Brown wrote: >>>> On Thu, Feb 05, 2015 at 02:08:54PM -0800, Bjorn Andersson wrote: >> >>>>> However this only works for the non-supply regulator properties - and >>>>> this is where Tim's patch is trying to sort out. >> >>>> No, this works completely fine for supply properties - to repeat what I >>>> said in reply to the original patch the supply is a supply to the chip >>>> not to an individual IP on the chip. >> >>> It does make some sense to consider the vbus-supply being connected to >>> the block, rather than directly to the vbus-switch. So it would work for >>> Tim's use case. >> >> Like I say if we do that then we don't have consistency in how we map a >> schematic into a DT binding - you have to dig into the binding of each >> device and figure out if the supply is viewed as being for blocks or for >> the chip as a whole and we've got the potential for problems in the >> binding if we figure out that a supply is actually used by other blocks >> later on and don't want to break existing DTs. > > OK - the light bulb finally went on for me on this one. > So a chip can have multiple supplies (I saw examples of this > poking around in other source), and the details of > internal routing in the chip don't have to be expressed in > DT at all (in fact shouldn't, for the reason you mention). > > Thanks - I will implement along these lines.
Mark,
Just to follow up on this, I got things working with the current code to my satisfaction. I ended up just punting on having multiple regulators come from the charger block. Instead I expose a single regulator from the charger driver, and just put all regulator attributes, including the supply reference, directly in the charger DT block. And I gave the charger block a label I could reference by phandle in the USB code.
I wanted to describe the difficulties I had, just for your consideration. Maybe something can be done (either in doc or in code), to help others avoid my pain.
My problem was in assuming that I could have a regulator with dev.of_node different from config.of_node.
The definition of of_get_regulator_init_data() implies that this is OK, because it accepts both a struct device and a struct device_node.
Also, when registering a regulator, you can pass a different of_node in config (struct regulator_config) than the one in dev (struct device)
However, this has problems in the current code, as the test in regulator_dev_lookup requires that the device_node found by of_get_regulator() match the dev.of_node in the regulator in the regulator list.
See this code in regulator_dev_lookup():
node = of_get_regulator(dev, NULL, supply); if (node) { list_for_each_entry(r, ®ulator_list, list) if (r->dev.parent && node == r->dev.of_node) return r;
It took me a while to figure this out, because a regulator defined with dev.of_node != config.of_node worked fine, when accessed directly by name (not using a supply-name). I only had problems when I accessed the regulator using the "-supply" indirection technique in DT.
In other words, using my previous DT configuration, I could do this: reg = regulator_get(dev, "chg_otg"); and everything would work fine, but if I did this: (in dt) vbus-supply = <&chg_otg>; reg = regulator_get(dev, "vbus"); would not work. (And using the "-supply" indirection in DT worked for other regulators).
Anyway, this caused me some confusion. If it's required to have dev.of_node == config.of_node, than maybe this field should be removed from struct regulator_config, and a separate device_node arg not be passed to of_get_regulator_init_data().
Just my 2 cents.
Thanks, -- Tim
| |