Re: [U-Boot] [PATCH v2 02/17] dm: core: Allow access to the device's driver_id data
On 20 November 2014 at 19:39, Simon Glass s...@chromium.org wrote:
Hi Masahiro,
On 20 November 2014 06:06, Masahiro Yamada yamad...@jp.panasonic.com wrote:
Hi Simon,
On Wed, 19 Nov 2014 09:35:54 +
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?
OK
/**
* 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
Will fix.
Fixed these and:
Applied to u-boot-dm.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 02/17] dm: core: Allow access to the device's driver_id data
Hi Masahiro,
On 20 November 2014 06:06, Masahiro Yamada yamad...@jp.panasonic.com wrote:
Hi Simon,
On Wed, 19 Nov 2014 09:35:54 +
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?
OK
/**
* 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
Will fix.
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 02/17] dm: core: Allow access to the device's driver_id data
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
diff --git a/drivers/core/lists.c b/drivers/core/lists.c
index 3a1ea85..9f33dde 100644
--- a/drivers/core/lists.c
+++ b/drivers/core/lists.c
@@ -89,22 +89,26 @@ int lists_bind_drivers(struct udevice *parent, bool
pre_reloc_only)
* tree error
*/
static int driver_check_compatible(const void *blob, int offset,
-const struct udevice_id *of_match)
+const struct udevice_id *of_match,
+const struct udevice_id **of_idp)
{
int ret;
+ *of_idp = NULL;
if (!of_match)
return -ENOENT;
while (of_match-compatible) {
ret = fdt_node_check_compatible(blob, offset,
of_match-compatible);
- if (!ret)
+ if (!ret) {
+ *of_idp = of_match;
return 0;
- else if (ret == -FDT_ERR_NOTFOUND)
+ } else if (ret == -FDT_ERR_NOTFOUND) {
return -ENODEV;
- else if (ret 0)
+ } else if (ret 0) {
return -EINVAL;
+ }
of_match++;
}
I think you are making things more complicated than is needed.
I guess what you want to do in this patch is just to set dev-of_id.
Why do you need to touch this function? (Please see below)
@@ -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?
Best Regards
Masahiro Yamada
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 02/17] dm: core: Allow access to the device's driver_id data
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. 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.
diff --git a/drivers/core/lists.c b/drivers/core/lists.c
index 3a1ea85..9f33dde 100644
--- a/drivers/core/lists.c
+++ b/drivers/core/lists.c
@@ -89,22 +89,26 @@ int lists_bind_drivers(struct udevice *parent, bool
pre_reloc_only)
* tree error
*/
static int driver_check_compatible(const void *blob, int offset,
-const struct udevice_id *of_match)
+const struct udevice_id *of_match,
+const struct udevice_id **of_idp)
{
int ret;
+ *of_idp = NULL;
if (!of_match)
return -ENOENT;
while (of_match-compatible) {
ret = fdt_node_check_compatible(blob, offset,
of_match-compatible);
- if (!ret)
+ if (!ret) {
+ *of_idp = of_match;
return 0;
- else if (ret == -FDT_ERR_NOTFOUND)
+ } else if (ret == -FDT_ERR_NOTFOUND) {
return -ENODEV;
- else if (ret 0)
+ } else if (ret 0) {
return -EINVAL;
+ }
of_match++;
}
I think you are making things more complicated than is needed.
I guess what you want to do in this patch is just to set dev-of_id.
Why do you need to touch this function? (Please see below)
See below.
@@ -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.
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 02/17] dm: core: Allow access to the device's driver_id data
Hi Simon,
On Wed, 19 Nov 2014 09:35:54 +
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
Re: [U-Boot] [PATCH v2 02/17] dm: core: Allow access to the device's driver_id data
On Tue, Nov 11, 2014 at 10:46:18AM -0700, Simon Glass wrote: When the device is created from a device tree node, it matches a compatible string. Allow access to that string and the associated data. Signed-off-by: Simon Glass s...@chromium.org Reviewed-by: Tom Rini tr...@ti.com -- Tom signature.asc Description: Digital signature ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 02/17] dm: core: Allow access to the device's driver_id data
Hello Simon, Am 11.11.2014 18:46, schrieb Simon Glass: When the device is created from a device tree node, it matches a compatible string. Allow access to that string and the associated data. Signed-off-by: Simon Glass s...@chromium.org --- Changes in v2: None drivers/core/device.c | 5 + drivers/core/lists.c | 17 - include/dm/device.h | 11 +++ 3 files changed, 28 insertions(+), 5 deletions(-) Acked-by: Heiko Schocher h...@denx.de bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v2 02/17] dm: core: Allow access to the device's driver_id data
When the device is created from a device tree node, it matches a compatible
string. Allow access to that string and the associated data.
Signed-off-by: Simon Glass s...@chromium.org
---
Changes in v2: None
drivers/core/device.c | 5 +
drivers/core/lists.c | 17 -
include/dm/device.h | 11 +++
3 files changed, 28 insertions(+), 5 deletions(-)
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;
+}
diff --git a/drivers/core/lists.c b/drivers/core/lists.c
index 3a1ea85..9f33dde 100644
--- a/drivers/core/lists.c
+++ b/drivers/core/lists.c
@@ -89,22 +89,26 @@ int lists_bind_drivers(struct udevice *parent, bool
pre_reloc_only)
* tree error
*/
static int driver_check_compatible(const void *blob, int offset,
- const struct udevice_id *of_match)
+ const struct udevice_id *of_match,
+ const struct udevice_id **of_idp)
{
int ret;
+ *of_idp = NULL;
if (!of_match)
return -ENOENT;
while (of_match-compatible) {
ret = fdt_node_check_compatible(blob, offset,
of_match-compatible);
- if (!ret)
+ if (!ret) {
+ *of_idp = of_match;
return 0;
- else if (ret == -FDT_ERR_NOTFOUND)
+ } else if (ret == -FDT_ERR_NOTFOUND) {
return -ENODEV;
- else if (ret 0)
+ } else if (ret 0) {
return -EINVAL;
+ }
of_match++;
}
@@ -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;
found = true;
if (devp)
*devp = dev;
diff --git a/include/dm/device.h b/include/dm/device.h
index 9ce95a8..287504c 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -47,6 +47,7 @@ struct driver_info;
* @name: Name of device, typically the FDT node name
* @platdata: Configuration data for this device
* @of_offset: Device tree node offset for this device (- for none)
+ * @of_id: Pointer to the udevice_id structure which created the device
* @parent: Parent of this device, or NULL for the top level device
* @priv: Private data for this device
* @uclass: Pointer to uclass for this device
@@ -65,6 +66,7 @@ struct udevice {
const char *name;
void *platdata;
int of_offset;
+ const struct udevice_id *of_id;
struct udevice *parent;
void *priv;
struct uclass *uclass;
@@ -206,6 +208,15 @@ void *dev_get_parentdata(struct udevice *dev);
void *dev_get_priv(struct udevice *dev);
/**
+ * dev_get_of_data() - get the device tree data used to bind a device
+ *
+ * When a device is bound using a device tree node, it matches a
+ * particular compatible string as in struct udevice_id. This function
+ * returns the associated data value for that compatible string
+ */
+ulong dev_get_of_data(struct udevice *dev);
+
+/**
* device_get_child() - Get the child of a device by index
*
* Returns the numbered child, 0 being the first. This does not use
--
2.1.0.rc2.206.gedb03e5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
