Hi Przemyslaw,
On 7 April 2015 at 09:31, Przemyslaw Marczak p.marczak@samsung.com wrote:
Hello Simon,
On 04/05/2015 08:30 PM, Simon Glass wrote:
Hi Przemyslaw,
On 3 April 2015 at 10:09, Przemyslaw Marczak p.marczak@samsung.com wrote:
Hello Simon,
On 03/29/2015 03:07 PM, Simon Glass wrote:
Hi Przemyslaw,
On 24 March 2015 at 14:30, Przemyslaw Marczak p.marczak@samsung.com wrote:
[snip]
info->min_uV = fdtdec_get_int(gd->fdt_blob, offset,"regulator-min-microvolt", -1);if (info->min_uV < 0)return -ENXIO;info->max_uV = fdtdec_get_int(gd->fdt_blob, offset,"regulator-max-microvolt", -1);if (info->max_uV < 0)return -ENXIO;/* Optional constraints */info->min_uA = fdtdec_get_int(gd->fdt_blob, offset,"regulator-min-microamp", -1);info->max_uA = fdtdec_get_int(gd->fdt_blob, offset,"regulator-max-microamp", -1);info->always_on = fdtdec_get_bool(gd->fdt_blob, offset,"regulator-always-on");info->boot_on = fdtdec_get_bool(gd->fdt_blob, offset,"regulator-boot-on");return 0;+}
+int regulator_get(char *name, struct udevice **devp) +{
struct dm_regulator_info *info;struct udevice *dev;int ret;*devp = NULL;for (ret = uclass_first_device(UCLASS_REGULATOR, &dev);dev;ret = uclass_next_device(&dev)) {This will probe all devices. See my suggestion about creating uclass_find_first_device()/uclass_find_next_device() in the next patch.
As before, I think this could use a function like uclass_get_device_by_name().
Yes, this is the same. But in this case, there is one more issue, which is the regulator device name. Usually after bind -> the dev->name is the same as node name. This is good, since it's natural that regulator IC, provides e.g "ldo1", or some other device-output name. But we would like check the "regulator-name" property. For this patch-set, the name is fixed at device probe stage, when dev->ofdata_to_platdata() is called, and the regulator constraints, the same as it's name goes to struct dm_regulator_info.
I put the dm_regulator_info into uclass priv, because it it uclass-specific, the same as struct dm_i2c_bus is specific for i2c buses.
But, the ucalss priv is allocated at device probe stage.
I can't use the .per_child_platdata_auto_alloc_size, since the parent is a pmic, and its uclass type is different.
Actually I could, move the dm_regulator_info only to device platdata, but then, the drivers should take care of this uclass-specific structure allocation. Is it acceptable?
But then, also ambiguous seem to be filling platdata (struct dm_regulator_info) in uclass post_bind() method.
So then, maybe reasonable is:
- move dm_regulator_info from dev->uclass_priv to dev->platdata - is
allocated after device bind
- add .post_bind() method to regulator uclass, and get the
"regulator-name" in it - only
- fill all platdata constraints on call to dev->ofdata_to_platdata()
Then, I could avoid probing every device, when checking the regulator name. But, still I can't use the uclass.c functions, since I'm checking the regulator info at dev->platdata.
So I update the function as below:
- uclass_foreach_dev(dev, uc) {
if (!dev->platdata)continue;info = dev->platdata;if (!strcmp(name, info->name)) {ret = device_probe(dev);if (ret)....*regulator = dev;return ret;}}The problem here is similar to I2C which uses per-child platdata (specified by the uclass) for the bus address. This is different from device platdata. I think you are suggesting that we should support uclass platdata. In this case we would have for each device:
- device platform data
- parent platform data
- uclass platform data
Yes, but note, that the uclass type is the same for I2C bus and i2c chip. This is a different than for the PMIC, for which childs uclass type are usually different than for the parent. In this case I can't use the field per-child-platdata.
The I2C bus uses UCLASS_I2C. The chips use a different UCLASS. If there is no specific driver for the chip then we will use UCLASS_I2C_GENERIC, but in general it could be anything. So perhaps there is no difference here?
Per-child platdata works for I2C buses because its children are all I2C chips, whatever their uclass.
The last one is not supported. I have through several times about adding it. The alternative is to require each device to provide a platform data struct at the start of its own platform data, which the uclass can find and use. This is not really in the spirit of driver model. But for now this is what I have done with USB (see dm/usb-working). struct usb_platdata appears at the start of struct exynos_ehci_platdata but is owned by the uclass.
If PMIC has use for uclass platform data, then perhaps that would be enough users to warrant it. You could add a patch to do this (don't forget to update tests) or you could do what I have done with USB and we can tidy it up later.
The example of usb is good enough. I could move the "dm_regulator_info" into the dm_regulator_platdata, and add "void *dev_platdata" at the end of it. From the other side, the regulator constraints and modes, are all what we need for this uclass. We have also the fixed-regulators which some platform data are the same as for the generic regulators - then the dev->uclass_platdata is reasonable for this purpose.
I will add the uclass platdata to struct udevice and also some test to test/dm drivers. I will send it as a separate patch.
OK thanks. As a reminded, please continue to rebase on dm/next.
Re your naming problem, apart from case the device name and regulator-compatible name are the same. So perhaps just use case-insensitive search for regulators? But if not then I take back my comment about using a common function for searching for regulator names. You are really searching the platform data for the regulator-compatible string, not the device name, and so the common function cannot be used.
Then we could provide two functions:
- regulator_by_devname() - search for device -> name
- regulator_by_platname() - search for platdata -> name
OK.
Regards, Simon