[U-Boot] [PATCH v2 00/12] Power(full) framework based on Driver Model

[Simon Glass]

Hi Przemyslaw,

On 3 March 2015 at 09:24, Przemyslaw Marczak <p.marczak at samsung.com> wrote:
> Hello,
> Here is the second RFC version of the new PMIC framework.
> The changes made in this version are described below each commit.
>
> So again, a quick summary of:
> Framework:
> - Add new uclass types:
>  -- UCLASS_PMIC(for device I/O)
>  -- UCLASS_PMIC_REGULATOR (for common regulator ops)
> - Two uclass drivers for the above types
> - A common regulator operations - will easy cover the real devices design
> - V2: pmic: add read/write ops
> - V2: regulator: use regulator type as an argument - not as function name
>
>
> Drivers:
> - Introduce new PMIC API for drivers - now everything base on "struct udevice"
> - Introduce Regulator Voltage descriptors and Operation Mode descriptors
>   which are usually taken from the device tree (board dependent data)
> - Two uclass device drivers for MAX77686(PMIC+REGULATOR)
> - V2: don't use the 'hw union' from old pmic
> - V2: remove the files: pmic_i2c.c/pmic_spi.c - now using bus drivers
> - V2: cleanup the pmic_get() functions
> - V2: add pmic_io_dev() function for getting the proper I/O dev for devices
> - V2: add function calls for getting pmic devices platdata
> - V2: remove regulator type from regulator operations function calls,
>       use type as an argument
>
> User Interface:
> - command pmic, unchanged functionality and ported to the driver model
> - command regulator(NEW) for safe regulator setup from commandline,
>   - now can check output Voltage and operation mode of the regulators,
>   - also can check the board Voltage limits and driver available modes
> - V2: simplify the code after remove the regulator type from function naming
> - V2: add on/off command
>
> Supported boards:
> - Odroid U3
> - V2: drop the commits for Trats2 - wait for charger and muic uclass types
>
> The assumptions of this work is:
> - Add new code to independent files
> - Keep two Frameworks as independent and without conflicts
> - Don't mix OLD/NEW Framework code - for the readability
>
> The future plans:
> - Add additional uclass types: MUIC, CHARGER, BATTERY, MFD and maybe more.
> - Port all U-Boot drivers to the new Framework
> - Remove the old drivers and the old PMIC Framework code
>
> Need help:
> - After merge this, it is welcome to help with driver porting
> - Every new driver should be tested on real hardware
>
> Best regards
>
> Przemyslaw Marczak (12):
>   exynos5: fix build break by adding CONFIG_POWER
>   dm: device: add function device_get_first_child_by_uclass_id()
>   dm: pmic: add implementation of driver model pmic uclass
>   dm: pmic: add implementation of driver model regulator uclass
>   dm: pmic: new commands: pmic and regulator
>   dm: pmic: add max77686 pmic driver
>   dm: regulator: add max77686 regulator driver
>   doc: driver-model: pmic and regulator uclass documentation
>   dm: board:samsung: power_init_board: add requirement of CONFIG_DM_PMIC
>   odroid: board: add support to dm pmic api
>   odroid: dts: add 'voltage-regulators' description to max77686 node
>   odroid: config: enable dm pmic, dm regulator and max77686 driver
>
>  Makefile                               |   1 +
>  arch/arm/dts/exynos4412-odroid.dts     | 249 ++++++++-
>  board/samsung/common/board.c           |   4 +-
>  board/samsung/common/misc.c            |   1 +
>  board/samsung/odroid/odroid.c          |  52 +-
>  configs/odroid_defconfig               |   1 -
>  doc/driver-model/dm-pmic-framework.txt | 367 +++++++++++++
>  drivers/core/device.c                  |  15 +
>  drivers/power/Makefile                 |   5 +-
>  drivers/power/cmd_pmic.c               | 820 +++++++++++++++++++++++++++++
>  drivers/power/pmic-uclass.c            | 191 +++++++
>  drivers/power/pmic/Makefile            |   1 +
>  drivers/power/pmic/max77686.c          | 102 ++++
>  drivers/power/pmic/pmic_max77686.c     |   2 +-
>  drivers/power/regulator-uclass.c       | 227 ++++++++
>  drivers/power/regulator/Makefile       |   8 +
>  drivers/power/regulator/max77686.c     | 926 +++++++++++++++++++++++++++++++++
>  include/configs/exynos5-common.h       |   4 +
>  include/configs/odroid.h               |   9 +-
>  include/dm/device.h                    |  16 +
>  include/dm/uclass-id.h                 |   4 +
>  include/power/max77686_pmic.h          |  26 +-
>  include/power/pmic.h                   | 265 ++++++++++
>  include/power/regulator.h              | 310 +++++++++++
>  24 files changed, 3573 insertions(+), 33 deletions(-)
>  create mode 100644 doc/driver-model/dm-pmic-framework.txt
>  create mode 100644 drivers/power/cmd_pmic.c
>  create mode 100644 drivers/power/pmic-uclass.c
>  create mode 100644 drivers/power/pmic/max77686.c
>  create mode 100644 drivers/power/regulator-uclass.c
>  create mode 100644 drivers/power/regulator/Makefile
>  create mode 100644 drivers/power/regulator/max77686.c
>  create mode 100644 include/power/regulator.h

This is an impressive pieces of work! It will be great to get these in.

Here are some high-level comments on this series:

1. I think regulator LDOs and bucks should be proper devices (struct
udevice), bound by the pmic when it is probed. The advantages are

a. You can see them in the device list - the pmic will end up with a
lot more children
b. You can use the same regulator uclass for each, but have different
operations for each driver (e.g. max77686 might provide two different
drivers, one for LDO, one for buck).
c. Things like your 'switch (type)' in max7786_get_state() etc. will go away
d. You can perform operations on them without having to specify their
parent and number - e.g. regulator_set_mode(struct udevice *ldo, int
mode) which is much more natural for users
e. You avoid needing your own list of regulators and bucks - struct
max7786_regulator_info. After all, keeping track of child devices is
something that driver model can do

2. I see device tree support, but the Linux device tree bindings are
not fully supported, e.g. the regulators sub-node uses
regulator-compatible instead of regulator-name. I think it should be
exactly the same (and we should copy the device tree files, only
leaving out what we don't support)

3. The I2C/SPI difference is a bit clunky. We should try to hide this
away. The most obvious problem is in getting the device. Instead of
pmic_i2c_get() we should use the "power-supply" property in the device
tree, so we need a function which can find the regulator given the
device node (a bit like gpio_request_by_name() but for PMICs). The
pmic_get() function is OK and will be needed also, as I am sure we
will use names in some places. We should remove any mention of the bus
type from the API I think. Also regulator number seems and odd concept
- better to use the name/device tree link to find the right device.
One way to avoid I2C/SPI problems is to create a helper file which
allows you to read and write registers given a struct udevice. It
could look at whether the device is I2C or SPI and do the right thing.
This could be generally useful, not just for PMICs.

4. Should use Kconfig now.

Regards,
Simon