Hello Simon,
On 04/08/2015 03:47 AM, Simon Glass wrote:
Hi Przemyslaw,
On 7 April 2015 at 09:31, Przemyslaw Marczak <p.marc...@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.marc...@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.marc...@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.
Sorry, I wrote nonsense. I meant something different.
We could use per-child-platdata field for pmic uclass driver with the
assumption, that each pmic's child's uclass is the same type (e.g.
regulator). But this assumption will be broken by fixed-regulator. And
also, pmic's childs are not only regulators - e.g. one can be RTC, other
charger, etc...
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.
Ok, I will rebase it.
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
Thanks,
--
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