Hi Przemyslaw, On 6 March 2015 at 08:08, Przemyslaw Marczak <p.marc...@samsung.com> wrote: > Hello Simon, > > > On 03/06/2015 03:10 PM, Simon Glass 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. > > > 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: >> >> 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 >> > > 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.
Sounds good! It's a lot of work but it will be nice to have all this done. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
