Hello Simon,
On 03/06/2015 03:10 PM, Simon Glass wrote:
Hi Przemyslaw,
On 3 March 2015 at 09:24, Przemyslaw Marczak p.marczak@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.
Thank you, and also thank you for the review. I hope to finish this all successfully.
Here are some high-level comments on this series:
- 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
- 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)
- 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.
- Should use Kconfig now.
Regards, Simon
I quickly read your all comments, and all are very helpful for me, I will reply to each in the next week.
I see that this piece of code should be done as e.g. the gpio uclass is.
My previous assumption was, to use the regulator numbers rather than names - as it is easy and faster - cause we have one driver instance for few regulators - but I agree, need change.
Usually there are numbers in the pmic documentation, and the names are in the board schematics which uses both names and numbers. So I added getting the names from dts just as some useful info for the regulator command.
I agree with you, that using the names instead of the numbers allow to make some things more easy, e.g. getting the 'udevice' by name.
I made some rework of the soft i2c with dm support, it looks that it is working fine. I will need this for work on next pmic devices in my Trats2 (charger, muic and fuelgauge).
Probably, I will send the dm soft i2c as independent patch set, and then get back to this framework code.
Thank you and best regards,