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

[Simon Glass]

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?

Should this be platform data (accessible before the device is probed)
or private data (which exists only when the device is active)?

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).

Regards,
Simon