Hello Simon,

On 03/10/2015 03:12 AM, Simon Glass wrote:
Hi Przemyslaw,

On 6 March 2015 at 07:10, Simon Glass <s...@chromium.org> wrote:
Hi Przemyslaw,

On 3 March 2015 at 09:24, Przemyslaw Marczak <p.marc...@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:


So, just as a summary of V3 rework.

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
This is done.

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).
This is done.

c. Things like your 'switch (type)' in max7786_get_state() etc. will go away
This is done.

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
This is done.

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
This is done. Now, each regulator device keeps the common regulator constraints as uclass private data (dev->uclass_priv) and the additional constraints for the fixed-regulator (gpio constraints) are kept as the device private data (dev->priv).


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)
This is done. Since the compatible is stored in a different property than just 'compatible', I added the function to bind devices by custom compatible property name. So regulators are bind by 'regulator-compatible' property compatibles


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.
This is done partially. My time was limited. The bus type and address is now removed from the api. I think that the 'pmic_get()' by the name should be ok, when we have different device-tree names for each pmic intarface. From the other side this is not so needed, since we are going to provide the device features by driver-model uclass drivers. This will take some time, so I add some extended function for getting the pmic in the next version. The 'power-supply' seem to be right for all devices, I will work on it in the next version.

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.
Yes, this would be useful, and actually will make something like the relationship for the pmic device and it's regulator child.


4. Should use Kconfig now.
This is done.


Sorry I don't know how I forgot to mention this:

5. A test PMIC device for sandbox so that the tests can run (test/dm/pmic.c).
This one, will be task for the future.


Regards,
Simon


Thank you again for the helpful suggestions!

Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marc...@samsung.com
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to