[U-Boot] [PATCH 3/3] core: Add dev_{disable,enable}_by_path

2018-03-28 Thread Mario Six
We cannot use device structures to disable devices, since getting
them with the API functions would bind and activate the device, which
would fail if the underlying device does not exist.

Hence, add a function to disable devices by path in a live device tree.

Signed-off-by: Mario Six 
---
 drivers/core/device.c | 36 
 include/dm/device.h   | 20 
 2 files changed, 56 insertions(+)

diff --git a/drivers/core/device.c b/drivers/core/device.c
index 940a153c58..c627453bb9 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -724,3 +724,39 @@ bool of_machine_is_compatible(const char *compat)
 
return !fdt_node_check_compatible(fdt, 0, compat);
 }
+
+#ifdef CONFIG_OF_LIVE
+int dev_disable_by_path(const char *path)
+{
+   ofnode node = np_to_ofnode(of_find_node_by_path(path));
+   struct udevice *dev = ofnode_dev(node);
+   int res;
+
+   res = device_remove(dev, DM_REMOVE_NORMAL);
+   if (res)
+   return res;
+
+   res = device_unbind(dev);
+   if (res)
+   return res;
+
+   return ofnode_disable(node);
+}
+
+int dev_enable_by_path(const char *path)
+{
+   ofnode node = np_to_ofnode(of_find_node_by_path(path));
+   ofnode pnode = ofnode_get_parent(node);
+   struct udevice *parent = ofnode_dev(pnode);
+   int res;
+
+   if (!parent)
+   return -EINVAL;
+
+   res = ofnode_enable(node);
+   if (res)
+   return res;
+
+   return lists_bind_fdt(parent, node, NULL);
+}
+#endif /* CONFIG_OF_LIVE */
diff --git a/include/dm/device.h b/include/dm/device.h
index 7786b1cf4e..f55907966a 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -586,6 +586,26 @@ bool device_is_compatible(struct udevice *dev, const char 
*compat);
  */
 bool of_machine_is_compatible(const char *compat);
 
+#ifdef CONFIG_OF_LIVE
+
+/**
+ * dev_disable_by_path() - Disable a device given its device tree path
+ *
+ * @path:  The device tree path identifying the device to be disabled
+ * @return 0 on success, -ve on error
+ */
+int dev_disable_by_path(const char *path);
+
+/**
+ * dev_enable_by_path() - Enable a device given its device tree path
+ *
+ * @path:  The device tree path identifying the device to be enabled
+ * @return 0 on success, -ve on error
+ */
+int dev_enable_by_path(const char *path);
+
+#endif /* CONFIG_OF_LIVE */
+
 /**
  * device_is_on_pci_bus - Test if a device is on a PCI bus
  *
-- 
2.16.1

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 3/3] core: Add dev_{disable,enable}_by_path

2018-03-29 Thread Simon Glass
Hi Mario,

On 28 March 2018 at 20:37, Mario Six  wrote:
> We cannot use device structures to disable devices, since getting
> them with the API functions would bind and activate the device, which
> would fail if the underlying device does not exist.
>
> Hence, add a function to disable devices by path in a live device tree.

What is the use case here? Is it to disable something after it has
already been bound / probed? Why can this not be done in the device
tree before U-Boot starts?

Also this needs a test.

>
> Signed-off-by: Mario Six 
> ---
>  drivers/core/device.c | 36 
>  include/dm/device.h   | 20 
>  2 files changed, 56 insertions(+)
>
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index 940a153c58..c627453bb9 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -724,3 +724,39 @@ bool of_machine_is_compatible(const char *compat)
>
> return !fdt_node_check_compatible(fdt, 0, compat);
>  }
> +
> +#ifdef CONFIG_OF_LIVE
> +int dev_disable_by_path(const char *path)
> +{
> +   ofnode node = np_to_ofnode(of_find_node_by_path(path));

Please see ofnode_path()

> +   struct udevice *dev = ofnode_dev(node);
> +   int res;
> +
> +   res = device_remove(dev, DM_REMOVE_NORMAL);
> +   if (res)
> +   return res;
> +
> +   res = device_unbind(dev);
> +   if (res)
> +   return res;
> +
> +   return ofnode_disable(node);
> +}
> +
> +int dev_enable_by_path(const char *path)
> +{
> +   ofnode node = np_to_ofnode(of_find_node_by_path(path));
> +   ofnode pnode = ofnode_get_parent(node);
> +   struct udevice *parent = ofnode_dev(pnode);
> +   int res;
> +
> +   if (!parent)
> +   return -EINVAL;
> +
> +   res = ofnode_enable(node);
> +   if (res)
> +   return res;
> +
> +   return lists_bind_fdt(parent, node, NULL);
> +}
> +#endif /* CONFIG_OF_LIVE */
> diff --git a/include/dm/device.h b/include/dm/device.h
> index 7786b1cf4e..f55907966a 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -586,6 +586,26 @@ bool device_is_compatible(struct udevice *dev, const 
> char *compat);
>   */
>  bool of_machine_is_compatible(const char *compat);
>
> +#ifdef CONFIG_OF_LIVE
> +
> +/**
> + * dev_disable_by_path() - Disable a device given its device tree path
> + *
> + * @path:  The device tree path identifying the device to be disabled
> + * @return 0 on success, -ve on error
> + */
> +int dev_disable_by_path(const char *path);
> +
> +/**
> + * dev_enable_by_path() - Enable a device given its device tree path
> + *
> + * @path:  The device tree path identifying the device to be enabled
> + * @return 0 on success, -ve on error
> + */
> +int dev_enable_by_path(const char *path);
> +
> +#endif /* CONFIG_OF_LIVE */
> +
>  /**
>   * device_is_on_pci_bus - Test if a device is on a PCI bus
>   *
> --
> 2.16.1
>

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 3/3] core: Add dev_{disable,enable}_by_path

2018-04-10 Thread Mario Six
Hi Simon,

On Fri, Mar 30, 2018 at 12:43 AM, Simon Glass  wrote:
> Hi Mario,
>
> On 28 March 2018 at 20:37, Mario Six  wrote:
>> We cannot use device structures to disable devices, since getting
>> them with the API functions would bind and activate the device, which
>> would fail if the underlying device does not exist.
>>
>> Hence, add a function to disable devices by path in a live device tree.
>
> What is the use case here? Is it to disable something after it has
> already been bound / probed? Why can this not be done in the device
> tree before U-Boot starts?
>

The devices that may not be in the tree are all disabled in the device tree.
Then, we detect which devices are actually there, and enable those that are.
That way we can use a single device tree for lots of actual boards, which are
all very minor variants of each other.

> Also this needs a test.
>
>>
>> Signed-off-by: Mario Six 
>> ---
>>  drivers/core/device.c | 36 
>>  include/dm/device.h   | 20 
>>  2 files changed, 56 insertions(+)
>>
>> diff --git a/drivers/core/device.c b/drivers/core/device.c
>> index 940a153c58..c627453bb9 100644
>> --- a/drivers/core/device.c
>> +++ b/drivers/core/device.c
>> @@ -724,3 +724,39 @@ bool of_machine_is_compatible(const char *compat)
>>
>> return !fdt_node_check_compatible(fdt, 0, compat);
>>  }
>> +
>> +#ifdef CONFIG_OF_LIVE
>> +int dev_disable_by_path(const char *path)
>> +{
>> +   ofnode node = np_to_ofnode(of_find_node_by_path(path));
>
> Please see ofnode_path()
>
>> +   struct udevice *dev = ofnode_dev(node);
>> +   int res;
>> +
>> +   res = device_remove(dev, DM_REMOVE_NORMAL);
>> +   if (res)
>> +   return res;
>> +
>> +   res = device_unbind(dev);
>> +   if (res)
>> +   return res;
>> +
>> +   return ofnode_disable(node);
>> +}
>> +
>> +int dev_enable_by_path(const char *path)
>> +{
>> +   ofnode node = np_to_ofnode(of_find_node_by_path(path));
>> +   ofnode pnode = ofnode_get_parent(node);
>> +   struct udevice *parent = ofnode_dev(pnode);
>> +   int res;
>> +
>> +   if (!parent)
>> +   return -EINVAL;
>> +
>> +   res = ofnode_enable(node);
>> +   if (res)
>> +   return res;
>> +
>> +   return lists_bind_fdt(parent, node, NULL);
>> +}
>> +#endif /* CONFIG_OF_LIVE */
>> diff --git a/include/dm/device.h b/include/dm/device.h
>> index 7786b1cf4e..f55907966a 100644
>> --- a/include/dm/device.h
>> +++ b/include/dm/device.h
>> @@ -586,6 +586,26 @@ bool device_is_compatible(struct udevice *dev, const 
>> char *compat);
>>   */
>>  bool of_machine_is_compatible(const char *compat);
>>
>> +#ifdef CONFIG_OF_LIVE
>> +
>> +/**
>> + * dev_disable_by_path() - Disable a device given its device tree path
>> + *
>> + * @path:  The device tree path identifying the device to be disabled
>> + * @return 0 on success, -ve on error
>> + */
>> +int dev_disable_by_path(const char *path);
>> +
>> +/**
>> + * dev_enable_by_path() - Enable a device given its device tree path
>> + *
>> + * @path:  The device tree path identifying the device to be enabled
>> + * @return 0 on success, -ve on error
>> + */
>> +int dev_enable_by_path(const char *path);
>> +
>> +#endif /* CONFIG_OF_LIVE */
>> +
>>  /**
>>   * device_is_on_pci_bus - Test if a device is on a PCI bus
>>   *
>> --
>> 2.16.1
>>
>
> Regards,
> Simon

Everything else will be addressed in v2. Thanks for reviewing!

Best regards,

Mario
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot