From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mario Six <mario.six@gdsys.cc>
Date: Fri, 4 May 2018 09:37:00 +0200
Subject: [U-Boot] [PATCH v2 3/4] core: Add dev_{disable,enable}_by_path
In-Reply-To: <CAPnjgZ0CtKuKgLWHRKvMuFKv+tBXgGBHBSAAOdhPX9RGxkBW+Q@mail.gmail.com>
References: <20180427125149.794-1-mario.six@gdsys.cc>
 <20180427125149.794-3-mario.six@gdsys.cc>
 <CAPnjgZ0CtKuKgLWHRKvMuFKv+tBXgGBHBSAAOdhPX9RGxkBW+Q@mail.gmail.com>
Message-ID: <CAN1kZooEggKboLOt4mrN35=k4nU+d9D2MUKOdizhYRWAVb76zw@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Simon,

On Thu, May 3, 2018 at 4:33 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Mario,
>
> On 27 April 2018 at 06:51, Mario Six <mario.six@gdsys.cc> 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.
>>
>> Signed-off-by: Mario Six <mario.six@gdsys.cc>
>>
>> ---
>>
>> v1 -> v2:
>> * Simplified np_to_ofnode(of_find_node_by_path(path)) to ofnode_path(path)
>> * Switched to returning -ENOSYS if livetree is not enabled
>> * Removed device_find_by_ofnode usage
>>
>> ---
>>  drivers/core/device.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/dm/device.h   | 20 ++++++++++++++++++
>>  2 files changed, 78 insertions(+)
>>
>> diff --git a/drivers/core/device.c b/drivers/core/device.c
>> index 940a153c58..3bb2fa02af 100644
>> --- a/drivers/core/device.c
>> +++ b/drivers/core/device.c
>> @@ -724,3 +724,61 @@ bool of_machine_is_compatible(const char *compat)
>>
>>         return !fdt_node_check_compatible(fdt, 0, compat);
>>  }
>> +
>> +int dev_disable_by_path(const char *path)
>> +{
>> +       struct uclass *uc;
>> +       ofnode node = ofnode_path(path);
>> +       struct udevice *dev;
>> +       int res = 1;
>
> For consistency with current code can you please use ret instead of res?
>

Sure, will be fixed in v3.

>> +
>> +       if (!of_live_active())
>> +               return -ENOSYS;
>> +
>> +       list_for_each_entry(uc, &gd->uclass_root, sibling_node) {
>> +               res = uclass_find_device_by_ofnode(uc->uc_drv->id, node, &dev);
>
> How about putting this in global device_find_by_ofnode() function,
> which doesn't work on a per-uclass basis?
>
>> +               if (!res || dev)
>> +                       break;
>
> Just !res
>

Will be fixed in v3.

>> +       }
>> +
>> +       if (res || !dev)
>> +               return res;
>
> Just
>
> if (res)
>

Will be fixed in v3.

>> +
>> +       res = device_remove(dev, DM_REMOVE_NORMAL);
>> +       if (res)
>> +               return res;
>> +
>> +       res = device_unbind(dev);
>> +       if (res)
>> +               return res;
>> +
>> +       return ofnode_set_enabled(node, false);
>> +}
>> +
>> +int dev_enable_by_path(const char *path)
>> +{
>> +       struct uclass *uc;
>> +       ofnode node = ofnode_path(path);
>> +       ofnode pnode = ofnode_get_parent(node);
>> +       struct udevice *parent;
>> +       int res = 1;
>> +
>> +       if (!of_live_active())
>> +               return -ENOSYS;
>> +
>> +       list_for_each_entry(uc, &gd->uclass_root, sibling_node) {
>> +               res = uclass_find_device_by_ofnode(uc->uc_drv->id, pnode,
>> +                                                  &parent);
>> +               if (!res || parent)
>
> if (!res)
>

Will be fixed in v3.

>> +                       break;
>> +       }
>> +
>> +       if (res || !parent)
>
> if (res)
>

Will be fixed in v3.

>> +               return res;
>> +
>> +       res = ofnode_set_enabled(node, true);
>> +       if (res)
>> +               return res;
>> +
>> +       return lists_bind_fdt(parent, node, NULL);
>> +}
>> 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
>

Best regards,
Mario