Hi Simon,
On Wed, 19 Nov 2014 09:35:54 +0000 Simon Glass <s...@chromium.org> wrote: > Hi Masahiro, > > On 19 November 2014 08:25, Masahiro Yamada <yamad...@jp.panasonic.com> wrote: > > Hi Simon, > > > > > > > > On Tue, 11 Nov 2014 10:46:18 -0700 > > Simon Glass <s...@chromium.org> wrote: > > > >> diff --git a/drivers/core/device.c b/drivers/core/device.c > >> index 49faa29..0d84776 100644 > >> --- a/drivers/core/device.c > >> +++ b/drivers/core/device.c > >> @@ -548,3 +548,8 @@ int device_find_next_child(struct udevice **devp) > >> > >> return 0; > >> } > >> + > >> +ulong dev_get_of_data(struct udevice *dev) > >> +{ > >> + return dev->of_id->data; > >> +} > > > > > > Since this function is short enough, perhaps you might want to > > define it as "static inline" in the header file include/dm/device.h > > > > > > Thanks for looking at this series. > > The background here is that I'm quite worried about all the pointers > in driver model. It might be quite easy to use the wrong one and get > confused. Me too. > So my plan is to add code to check that the pointers are > what we think they are, like: > > DM_CHECK_PTR(dev, "udevice"); > > or similar. Then that code would compile to nothing unless it is > enabled with a CONFIG_DM_CHECK_PTRS or whatever. That's the reason for > accessors. > Sounds good! > > > >> @@ -116,6 +120,7 @@ int lists_bind_fdt(struct udevice *parent, const void > >> *blob, int offset, > >> { > >> struct driver *driver = ll_entry_start(struct driver, driver); > >> const int n_ents = ll_entry_count(struct driver, driver); > >> + const struct udevice_id *id; > >> struct driver *entry; > >> struct udevice *dev; > >> bool found = false; > >> @@ -127,7 +132,8 @@ int lists_bind_fdt(struct udevice *parent, const void > >> *blob, int offset, > >> if (devp) > >> *devp = NULL; > >> for (entry = driver; entry != driver + n_ents; entry++) { > >> - ret = driver_check_compatible(blob, offset, entry->of_match); > >> + ret = driver_check_compatible(blob, offset, entry->of_match, > >> + &id); > >> name = fdt_get_name(blob, offset, NULL); > >> if (ret == -ENOENT) { > >> continue; > >> @@ -147,6 +153,7 @@ int lists_bind_fdt(struct udevice *parent, const void > >> *blob, int offset, > >> dm_warn("Error binding driver '%s'\n", entry->name); > >> return ret; > >> } else { > >> + dev->of_id = id; > > > > > > Instead of all the chages above, only one line change, > > > > dev->of_id = entry->of_match > > > > > > > > Does this work for you? > > > > > > entry->of_match is the first element in an array of records. I want to > know exactly which one matches, so I can't just use the first one. > Sorry, it was my misunderstanding. Thanks for explaining this. Could you update the comment block of driver_check_compatible? /** * driver_check_compatible() - Check if a driver is compatible with this node * * @param blob: Device tree pointer * @param offset: Offset of node in device tree * @param of_matchL List of compatible strings to match * @return 0 if there is a match, -ENOENT if no match, -ENODEV if the node * does not have a compatible string, other error <0 if there is a device * tree error */ - Add description of "@param of_idp" - Fix "of_matchL" -> "of_match" Best Regards Masahiro Yamada _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot