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
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.
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.
[snip]
Regards, Simon