[U-Boot] [PATCH] power: pmic/regulator: Add basic support for TPS65910

[Felix Brack]

Hello Simon,

On 26.11.2017 12:38, Simon Glass wrote:
> Hi Felix,
> 
> On 23 November 2017 at 08:36, Felix Brack <fb at ltec.ch> wrote:
>>
>>
>> On 22.11.2017 11:39, Felix Brack wrote:
>>> Hello Simon,
>>>
>>> Many thanks for taking the time to review my patch.
>>>
>>> On 20.11.2017 16:38, Simon Glass wrote:
>>>> Hi Felix,
>>>>
>>>>> +
>>>>> +/* platform data */
>>>>> +struct tps65910_pdata {
>>>>> +       u32 supply[TPS65910_NUM_SUPPLIES]; /* regulator supply voltage in uV */
>>>>> +};
>>>>
>>>> Is this used outside the driver? Do you need one driver to access
>>>> another's platform data? That seems dodgy.
>>>>
>>> No. You are right: no need to place it in the header file. I will move
>>> it to pmic_tps65910_dm.c file.
>>>
>> Sorry, this is not correct. The regulators of the PMIC need access to
>> this. Let me try to explain:
>>
>> The PMIC has 8 different supply inputs. These are therefore properties
>> of the PMIC. Each of the 12 regulators uses one of these 8 supply
>> voltages as its input. Hence the regulators of the TPS65910 need read
>> access to the PMIC's information about the supplies. In other words: a
>> regulator's supply voltage has to be queried from the corresponding PMIC.
>>
>> I think I followed the "rules" when implementing the PMIC and the
>> regulator code in separate files.
> 
> OK, so the problem is that the regulator has to look up the supply
> voltage in the PMIC?
> 
Yes

> Should this be platform data (accessible before the device is probed)
> or private data (which exists only when the device is active)?
> 
The regulator supply voltages are defined in the DT. Hence they should
be read by ofdata_to_platdata() and store as platform data, I think.

> Also why does the PMIC driver itself know about supply voltages? I
> think that info should be in the regulator driver. The PMIC is really
> just a parent driver providing access to the various subsystems of the
> PMIC (regulator, GPIO, battery, etc).
> 
I agree - the PMIC driver does not require the knowledge about the
supply voltage of a distinct regulator. It is just that the PMIC is
modeled like this in the DT, i.e. the supplies are proerties of the PMIC
and not the regulators (which are subnodes on the same level as the
supply nodes).
Having said that, I think the proper solution is as follows:
Use the regulator driver's ofdata_to_platdata() to query the supply
voltage of that specific regulator by device_get_supply_regulator()
using dev->parent (the PMIC) as device parameter.
The supply voltage will then be "part" of the regulator instead of the
PMIC. Do you agree to this solution?

regards, Felix