[U-Boot] [RFC] POWER framework v3 - wish list

[U-Boot] [RFC] POWER framework v3 - wish list

[Przemyslaw Marczak]

Hello,

Some time ago I had a pleasure to work on some kind of a simple
charger manager for u-boot. For checking battery charge level, cable
state, and some more info I made a common calls which were implemented 
in board files, so actually it didn't base directly on PMIC framework. 
This allowed making charger abstraction but it was a second power 
related framework next to the PMIC.
 From my side I would like to introduce its next version according to 
the doc/README.power-framework documents "TO DO" list and some more...

In this RFC any comments about next version of power management 
framework are welcome.
What new PMIC framework should provide?
How to extend it?
What can be fixed?

So, what should be done (in my opinion):

1. Framework architecture:

I think that current PMIC hierarchy is not so clean as it could be.
It is just based on Trats board architecture. I would like to add
there some more abstraction to make it common and simply to use.

The main problem is that current PMIC framework is based on names of 
devices. 				
So to check some specific device options I need to know the name of such 
device.

Examples of useful options:
  - battery state,
  - battery level
  - charger state,
  - charger type,
  - battery charge,
    and some else...

On Trats2 which I mostly use, not every option refers to a single PMIC
device. Now this problem refers only to few Samsung boards - no more
supports battery charger at present - but this maybe change...

Looking at pmic header we can find some specific ops:
    [struct structure_name: fields (ops)]
  - struct power_fg: fg_battery_check; fg_battery_update;
  - struct power_chrg: chrg_type; chrg_bat_present; chrg_state;
  - struct power_battery: struct bat; battery_init; battery_charge;

To simplify this - in my opinion those all operations could be stored in 
a single structure,
because actually it refers to one abstract device - CHARGER.
Battery should not be treated as a device because it is only passive
element and actually in a simple version it has no logical interface
besides power contacts - so actually charger is responsible for 
maintaining battery condition and checking its state.

Next architectural change which I'm thinking of about is to add some 
abstraction of real IC features.
Nowadays boards come with highly integrated power management ICs which 
provide various subsystems like:
  - ldo / buck
  - battery charger
  - micro USB interface controller
  - gpio / keys
  - rtc
  - led flash driver
  - motor driver
  - system temperature control
  - and more, and more...(e.g. device power off feature)

  And this all is often controlled by more then one device/interface.

  Example in trats2: there are two power ICs registered as many devices, 
some of them share the same interface and bus.

This is shown on diagram below:
(taken from: doc/README.power-framework)
                           -----------------
                   --------| BAT           |------------
                   |       |               |           |
                   |       -----------------           |
                   |               |                   |
                  \|/             \|/                 \|/
           -----------     -----------------       ---------
           |FG       |     |MUIC           |       |CHRG   |
           |         |     |               |       |       |
           -----------     -----------------       ---------
  	MAX77693        MAX77693                MAX77693
           addr: 0x6C      addr: 0x4A              addr: 0xCC

  There is also another ic: MAX77686 - which acts as:
  - LDO/BUCK regulator, pwr key event (i2c addr: 0x12)
  - rtc (i2c addr: 0x0C)

  So this is the main POWER device - but framework doesn't provide any
  common ops for it besides standard register's read/write.

I think that this architecture could look as shown on the diagram below.

    -------------------------
  | COMMON POWER FRAMEWORK  |
  | with supported ops:     |
  | - A			   |
  | - B                     |
  | - C                     |
  | - and N - others        |
    -------------------------
         | | | | | |
    (N-number of registered power devices)

         |                       |       . . .       |
         |                       |                   |
    ------------- 	  -------------          ----
  | POWER DEV   |	 | POWER DEV   |        |
  | name "AAA"  |	 | name "BBB"  |        |
  | dev id 0    |         | dev id 1    | . . .  |
  |-------------|         |-------------|        | . . .
  | ops type A  |         | ops type B  |        |
  | iface I2C1  |         | iface SPI1  |        |
  |-------------|          -------------         |
  | ops type B  |                                |
  | iface I2C2  |                                .
  |-------------|                                .
  | ops type C  |                                .
  | iface XXXX  |
    -------------

  So the main change is to add multiple ops on a multiple interfaces.

  And those changes actually resolve PMIC IC instances problem.
  Please look at some piece of code below.

  (some part of include/power/pmic.h):
  enum { PMIC_I2C, PMIC_SPI, PMIC_NONE};
  enum { I2C_PMIC, I2C_NUM, };
  enum { PMIC_READ, PMIC_WRITE, };
  enum { PMIC_SENSOR_BYTE_ORDER_LITTLE, PMIC_SENSOR_BYTE_ORDER_BIG, };

  struct p_i2c { no changes };

  struct p_spi { no changes };

  (new framework):
  enum {
  	POWER_OPS_REGISTER_RW,
  	POWER_OPS_REGULATOR,
  	POWER_OPS_CHARGER,
  	POWER_OPS_MUIC,
  	POWER_OPS_KEY_POWER,
  	POWER_OPS_RTC,
  	POWER_OPS_MOTOR,
  	POWER_OPS_LED_FLASH,
  	POWER_OPS_TEMP_CONTROL,
  };

  struct power_ops_register_rw {
  	int (*register_read) (int reg, int *val);
  	int (*register_write) (int reg, int *val);
  };

  struct power_ops_regulator {
  	int (*ldo_set_value) (int regulator, int val, int mode, ...);
  	int (*ldo_get_value) (int regulator, int *val, int mode, ...);
  	int (*buck_set_value) (int buck, int val, int mode, ...);
  	int (*buck_get_value) (int buck, int *val, int mode, ...);
  };

  struct power_ops_charger {
  	int (*chrg_present) (void);
  	int (*chrg_type) (void);
  	int (*bat_present) (void);
  	int (*battery_state) (struct battery *bat);
  	int (*battery_charge) (struct battery *bat);
  };

  struct power_ops_muic {
  	int (*cable_attached) (void);
  	int (*cable_accesory_type) (void);
  };

  struct power_ops_key_power {
  	int (*key_state) (int *state);
  };

  struct power_ops_rtc {
  	int (*sec) (int set_get, int *val);
  	int (*min) (int set_get, int *val);
  	int (*hour) (int set_get, int *val);
  	int (*day) (int set_get, int *val);
  	int (*month) (int set_get, int *val);
  	int (*year) (int set_get, int *val);
  };

  struct power_ops_motor {
  	int (*configure) (void);
  	int (*enable) (int time, int gain);
  };

  struct power_ops_led_flash {
  	int (*configure) (void);
  	int (*enable) (void);
  	int (*disable) (void);
  };

  struct power_ops_temp_control {
  	int (*configure) (void);
  	int (*get_temp) (int *val);
  };

  struct power_dev_interface {
  	int id;   /* One device can use few interfaces */
  	int type; /* enum { PMIC_I2C, PMIC_SPI, ...}; */
  	int bus;
  	int sensor_byte_order;
  	int number_of_regs;
  	union hw {
  		struct p_i2c i2c;
  		struct p_spi spi;
  	} hw;
};

struct power_dev_ops {
  	/* type: enum { POWER_DEV_REGULATOR, ... } */
	int type;

  	/* id: for multiple ops of one type */
  	int id;

  	/* corresponding interface */
  	struct power_dev_interface *io;

  	/* Pointer to proper type ops structure */
  	void *ops;

  	/* Recognized by sequential OPS */
  	struct list_head list;
};

struct power_dev {
  	int id;
  	const char *name;

  	/* device operations list */
  	struct power_dev_ops *ops;

  	/* device instances list */
  	struct list_head list;
};


Such organization of structures allows to:
- initialize few instances of one driver,	
- initialize only supported operations,
- provide driver's operations on a few interfaces,
- initialize driver with the same ops on a different interfaces,

Specified ops types allow implementation of optional framework command line
tool especially we can search devices by: name, ops type, interface -
every with optional multiple instance.

e.g. command use
# power list ops
# power list devices
# power list interfaces

the rest of commands could be used as in current pmic framework.

Of course this is just a proposition - It's not implemented yet.

The next change is the integration with device tree which is obligatory
here and device model could be added in the future.

Drivers ops and interfaces could be initialized into linker lists.
Device parameters like an interface, bus and instances could be
parsed from dtb at runtime.

So this gives some abstraction to power related devices.
On a top of those changes I will add a simple charger manager.

Before I start writing code I would like to receive some feedback,
about this idea.

Thank you,
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

[U-Boot] [RFC] POWER framework v3 - wish list

[Marek Vasut]

On Friday, May 09, 2014 at 08:58:02 AM, Przemyslaw Marczak wrote:
> Hello,

[...]

>   struct power_ops_key_power {
>   	int (*key_state) (int *state);
>   };

This could be a key input device.

>   struct power_ops_rtc {
>   	int (*sec) (int set_get, int *val);
>   	int (*min) (int set_get, int *val);
>   	int (*hour) (int set_get, int *val);
>   	int (*day) (int set_get, int *val);
>   	int (*month) (int set_get, int *val);
>   	int (*year) (int set_get, int *val);
>   };

RTC device.

>   struct power_ops_motor {
>   	int (*configure) (void);
>   	int (*enable) (int time, int gain);
>   };
> 
>   struct power_ops_led_flash {
>   	int (*configure) (void);
>   	int (*enable) (void);
>   	int (*disable) (void);
>   };

LED device.

It seems like you're trying to assemble a huge framework while avoiding the 
already-present frameworks. No?

Best regards,
Marek Vasut

[U-Boot] [RFC] POWER framework v3 - wish list

[Simon Glass]

Hi,

On 9 May 2014 03:25, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
> Hello,
>
> Some time ago I had a pleasure to work on some kind of a simple
> charger manager for u-boot. For checking battery charge level, cable
> state, and some more info I made a common calls which were implemented in
> board files, so actually it didn't base directly on PMIC framework. This
> allowed making charger abstraction but it was a second power related
> framework next to the PMIC.
> From my side I would like to introduce its next version according to the
> doc/README.power-framework documents "TO DO" list and some more...
>
> In this RFC any comments about next version of power management framework
> are welcome.
> What new PMIC framework should provide?
> How to extend it?
> What can be fixed?
>
> So, what should be done (in my opinion):

Can I suggest we try to build this with driver model if possible? It
seems like you will otherwise end up duplicating much of the plumbing
of that. For example, see 'struct device'.

Regards,
Simon

[U-Boot] [RFC] POWER framework v3 - wish list

[Przemyslaw Marczak]

Hello Marek,

On 05/15/2014 09:01 PM, Marek Vasut wrote:
> On Friday, May 09, 2014 at 08:58:02 AM, Przemyslaw Marczak wrote:
>> Hello,
>
> [...]
>
>>    struct power_ops_key_power {
>>    	int (*key_state) (int *state);
>>    };
>
> This could be a key input device.
>
>>    struct power_ops_rtc {
>>    	int (*sec) (int set_get, int *val);
>>    	int (*min) (int set_get, int *val);
>>    	int (*hour) (int set_get, int *val);
>>    	int (*day) (int set_get, int *val);
>>    	int (*month) (int set_get, int *val);
>>    	int (*year) (int set_get, int *val);
>>    };
>
> RTC device.
>
>>    struct power_ops_motor {
>>    	int (*configure) (void);
>>    	int (*enable) (int time, int gain);
>>    };
>>
>>    struct power_ops_led_flash {
>>    	int (*configure) (void);
>>    	int (*enable) (void);
>>    	int (*disable) (void);
>>    };
>
> LED device.

Yes, they are actually a various devices with separated options.

I looked into some Frescale and Maxim PMICs documentation. And those 
integrated devices usually provides various ops on one or two 
interfaces. So I think it would be nice to have one framework and e.g. 
register available devices options for each interface of each device.

>
> It seems like you're trying to assemble a huge framework while avoiding the
> already-present frameworks. No?
>

You're right - we have some frameworks at present and this framework 
could be an additional abstraction level between device and uboot commands.
Device could be presented as it was designed - is it bad idea?

> Best regards,
> Marek Vasut
>

Thanks
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

[U-Boot] [RFC] POWER framework v3 - wish list

[Przemyslaw Marczak]

Hello Simon,

On 05/19/2014 08:37 PM, Simon Glass wrote:
> Hi,
>
> On 9 May 2014 03:25, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
>> Hello,
>>
>> Some time ago I had a pleasure to work on some kind of a simple
>> charger manager for u-boot. For checking battery charge level, cable
>> state, and some more info I made a common calls which were implemented in
>> board files, so actually it didn't base directly on PMIC framework. This
>> allowed making charger abstraction but it was a second power related
>> framework next to the PMIC.
>>  From my side I would like to introduce its next version according to the
>> doc/README.power-framework documents "TO DO" list and some more...
>>
>> In this RFC any comments about next version of power management framework
>> are welcome.
>> What new PMIC framework should provide?
>> How to extend it?
>> What can be fixed?
>>
>> So, what should be done (in my opinion):
>
> Can I suggest we try to build this with driver model if possible? It
> seems like you will otherwise end up duplicating much of the plumbing
> of that. For example, see 'struct device'.
>
> Regards,
> Simon
>

Yes, I also would like to take it into account - but I am not sure that 
driver model is not too redundant for it?
If I well understand the current driver model - each driver can have 
only one uclass, right? If yes, then for each previous listed ops we 
need to register separate driver - isn't it too much for read one 
register or turn on the led in the same physically device?

So maybe better is to register one "power" driver in driver model? And 
the power uclass driver could init more then one class specific ops.

I just would like to keep it simply.

Thanks
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

[U-Boot] [RFC] POWER framework v3 - wish list

[Simon Glass]

Hi Przemyslaw,

On 20 May 2014 03:16, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
> Hello Simon,
>
>
> On 05/19/2014 08:37 PM, Simon Glass wrote:
>>
>> Hi,
>>
>> On 9 May 2014 03:25, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
>>>
>>> Hello,
>>>
>>> Some time ago I had a pleasure to work on some kind of a simple
>>> charger manager for u-boot. For checking battery charge level, cable
>>> state, and some more info I made a common calls which were implemented in
>>> board files, so actually it didn't base directly on PMIC framework. This
>>> allowed making charger abstraction but it was a second power related
>>> framework next to the PMIC.
>>>  From my side I would like to introduce its next version according to the
>>> doc/README.power-framework documents "TO DO" list and some more...
>>>
>>> In this RFC any comments about next version of power management framework
>>> are welcome.
>>> What new PMIC framework should provide?
>>> How to extend it?
>>> What can be fixed?
>>>
>>> So, what should be done (in my opinion):
>>
>>
>> Can I suggest we try to build this with driver model if possible? It
>> seems like you will otherwise end up duplicating much of the plumbing
>> of that. For example, see 'struct device'.
>>
>> Regards,
>> Simon
>>
>
> Yes, I also would like to take it into account - but I am not sure that
> driver model is not too redundant for it?
> If I well understand the current driver model - each driver can have only
> one uclass, right? If yes, then for each previous listed ops we need to
> register separate driver - isn't it too much for read one register or turn
> on the led in the same physically device?
>
> So maybe better is to register one "power" driver in driver model? And the
> power uclass driver could init more then one class specific ops.
>
> I just would like to keep it simply.

Well the intent is for all drivers to fit within the driver model
framework somehow. To answer your questions:

Each device can only have one uclass, but it is possible to create
multiple child devices associated with a driver, and each of those can
be associated with a different uclass. For example a PMIC chip might
have a GPIO driver, a battery driver and a regulator driver. You can
have a top-level mfd device perhaps, which provides access to the
rest. Note there is no mfd uclass as yet, but it would be pretty easy
to add something that supports basic enumeration of its children. This
is how it works in linux and that model works well enough in practice.

On the other hand it doesn't make sense to conflate one function with
another. Clearly the interface you need to access GPIOs is quite
different from that used for setting up power supply voltages. It
actually makes more sense to split up these functions into separate
devices than to try to invent a new semi-merged API for this strange
new franken-device.

Also, I'm not sure what the top level 'power' device would actually
do? Wouldn't you need to grub around inside it and find out its
capabilities (regulators, GPIO, ADC, etc.)? If so, that
grubbing-around layer might as well just be driver model IMO.

A bigger challenge is that almost no drivers support driver model yet,
so definitely there will be a bit of trail-blazing. I'm in there
looking at GPIOs at the moment so can help if questions come up.

Regards,
Simon