| Message ID | 20200421140840.25729-3-patrice.chotard@st.com |
|---|---|
| State | New |
| Headers | show |
| Series | cmd: bind allow to bind driver with driver_data | expand |
Hi Patrice, On Tue, 21 Apr 2020 at 08:09, Patrice Chotard <patrice.chotard at st.com> wrote: > > Initial implementation invokes device_bind_with_driver_data() > with driver_data parameter equal to 0. > For driver with driver data, the bind command can't bind > correctly this driver or even worse causes data abort. > > Add find_udevice_id() to parse the driver's of_match list > and return the entry corresponding to the driver compatible string. > This allows to get access to driver_data and to use it as > parameters of device_bind_with_driver_data(). > > Signed-off-by: Patrice Chotard <patrice.chotard at st.com> > Cc: Jean-Jacques Hiblot <jjhiblot at ti.com> > > --- > > cmd/bind.c | 29 ++++++++++++++++++++++++++++- > 1 file changed, 28 insertions(+), 1 deletion(-) > The thing I don't quite get here is why the driver name needs to be specified. If the device tree node is present, and it has a compatible string, can't DM find the driver and bind a device automatically? Also, is there any docs for this command? It would be good to add to doc/driver-model and also add a simple test. Regards, Simon
On 4/21/20 7:36 PM, Simon Glass wrote: > Hi Patrice, > > On Tue, 21 Apr 2020 at 08:09, Patrice Chotard <patrice.chotard at st.com> wrote: >> Initial implementation invokes device_bind_with_driver_data() >> with driver_data parameter equal to 0. >> For driver with driver data, the bind command can't bind >> correctly this driver or even worse causes data abort. >> >> Add find_udevice_id() to parse the driver's of_match list >> and return the entry corresponding to the driver compatible string. >> This allows to get access to driver_data and to use it as >> parameters of device_bind_with_driver_data(). >> >> Signed-off-by: Patrice Chotard <patrice.chotard at st.com> >> Cc: Jean-Jacques Hiblot <jjhiblot at ti.com> >> >> --- >> >> cmd/bind.c | 29 ++++++++++++++++++++++++++++- >> 1 file changed, 28 insertions(+), 1 deletion(-) >> > The thing I don't quite get here is why the driver name needs to be > specified. If the device tree node is present, and it has a compatible Sorry, i didn't get your point when you said "why the driver name needs to be specified" Which part of this patch do you made reference to ? > string, can't DM find the driver and bind a device automatically? > > Also, is there any docs for this command? It would be good to add to Is what in cmd/bind.c not enough ? U_BOOT_CMD( ??? bind,??? 4,??? 0,??? do_bind_unbind, ??? "Bind a device to a driver", ??? "<node path> <driver>\n" ??? "bind <class> <index> <driver>\n" ); U_BOOT_CMD( ??? unbind,??? 4,??? 0,??? do_bind_unbind, ??? "Unbind a device from a driver", ??? "<node path>\n" ??? "unbind <class> <index>\n" ??? "unbind <class> <index> <driver>\n" ); > doc/driver-model and also add a simple test. Ok i will add an additionnal test to test/py/tests/test_bind.py Thanks Patrice > > Regards, > Simon
Hi Patrice, On Wed, 22 Apr 2020 at 02:13, Patrice CHOTARD <patrice.chotard at st.com> wrote: > > > On 4/21/20 7:36 PM, Simon Glass wrote: > > Hi Patrice, > > > > On Tue, 21 Apr 2020 at 08:09, Patrice Chotard <patrice.chotard at st.com> wrote: > >> Initial implementation invokes device_bind_with_driver_data() > >> with driver_data parameter equal to 0. > >> For driver with driver data, the bind command can't bind > >> correctly this driver or even worse causes data abort. > >> > >> Add find_udevice_id() to parse the driver's of_match list > >> and return the entry corresponding to the driver compatible string. > >> This allows to get access to driver_data and to use it as > >> parameters of device_bind_with_driver_data(). > >> > >> Signed-off-by: Patrice Chotard <patrice.chotard at st.com> > >> Cc: Jean-Jacques Hiblot <jjhiblot at ti.com> > >> > >> --- > >> > >> cmd/bind.c | 29 ++++++++++++++++++++++++++++- > >> 1 file changed, 28 insertions(+), 1 deletion(-) > >> > > The thing I don't quite get here is why the driver name needs to be > > specified. If the device tree node is present, and it has a compatible > > Sorry, i didn't get your point when you said "why the driver name needs to be specified" It's just that I don't understand it at all. If the compatible string is available, why not use lists_bind_fdt()? > > Which part of this patch do you made reference to ? The whole thing, because I just don't understand the bind command. > > > string, can't DM find the driver and bind a device automatically? > > > > Also, is there any docs for this command? It would be good to add to > > Is what in cmd/bind.c not enough ? I am just confused here. You obviously have a use case in mind, but the help below is not sufficient to understand what is going on. As I said, if you have a device-tree node you can find the driver. I am just not sure what this is for. It could really use a short document as I said, to explain the uses of this command and what it does in a bit more detail. > > > U_BOOT_CMD( > bind, 4, 0, do_bind_unbind, > "Bind a device to a driver", > "<node path> <driver>\n" > "bind <class> <index> <driver>\n" > ); > > U_BOOT_CMD( > unbind, 4, 0, do_bind_unbind, > "Unbind a device from a driver", > "<node path>\n" > "unbind <class> <index>\n" > "unbind <class> <index> <driver>\n" > ); > > > > doc/driver-model and also add a simple test. > > Ok i will add an additionnal test to test/py/tests/test_bind.py OK thanks. Regards, SImon
diff --git a/cmd/bind.c b/cmd/bind.c index 44a5f17f0d..ab1844c855 100644 --- a/cmd/bind.c +++ b/cmd/bind.c @@ -118,6 +118,29 @@ static int unbind_child_by_class_index(const char *uclass, int index, return ret; } +static const struct udevice_id *find_udevice_id(struct driver *drv, + ofnode ofnode) +{ + const char *compat_list, *compat; + const struct udevice_id *id; + int ret, compat_length, i; + + compat_list = ofnode_get_property(ofnode, "compatible", &compat_length); + /* + * Walk through the compatible string list and find the corresponding + * udevice_id entry + */ + for (i = 0; i < compat_length; i += strlen(compat) + 1) { + compat = compat_list + i; + + ret = driver_check_compatible(drv->of_match, &id, compat); + if (!ret) + break; + } + + return id; +} + static int bind_by_node_path(const char *path, const char *drv_name) { struct udevice *dev; @@ -125,6 +148,7 @@ static int bind_by_node_path(const char *path, const char *drv_name) int ret; ofnode ofnode; struct driver *drv; + const struct udevice_id *id; drv = lists_driver_lookup_name(drv_name); if (!drv) { @@ -150,8 +174,11 @@ static int bind_by_node_path(const char *path, const char *drv_name) } ofnode = ofnode_path(path); + id = find_udevice_id(drv, ofnode); + ret = device_bind_with_driver_data(parent, drv, ofnode_get_name(ofnode), - 0, ofnode, &dev); + id->data, ofnode, &dev); + if (!dev || ret) { printf("Unable to bind. err:%d\n", ret); return ret;
Initial implementation invokes device_bind_with_driver_data() with driver_data parameter equal to 0. For driver with driver data, the bind command can't bind correctly this driver or even worse causes data abort. Add find_udevice_id() to parse the driver's of_match list and return the entry corresponding to the driver compatible string. This allows to get access to driver_data and to use it as parameters of device_bind_with_driver_data(). Signed-off-by: Patrice Chotard <patrice.chotard at st.com> Cc: Jean-Jacques Hiblot <jjhiblot at ti.com> --- cmd/bind.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-)