On Tue, Jan 23, 2024 at 7:14 AM Nuno Sa via B4 Relay
<devnull+nuno.sa.analog.com@xxxxxxxxxx> wrote:
>
> From: Nuno Sa <nuno.sa@xxxxxxxxxx>
>
> If a device_link is previously created (eg: via
> fw_devlink_create_devlink()) before the supplier + consumer are both
> present and bound to their respective drivers, there's no way to set
> DL_FLAG_AUTOREMOVE_CONSUMER anymore while one can still set
> DL_FLAG_AUTOREMOVE_SUPPLIER. Hence, rework the flags checks to allow
> for DL_FLAG_AUTOREMOVE_CONSUMER in the same way
> DL_FLAG_AUTOREMOVE_SUPPLIER is done.
Curious, why do you want to set DL_FLAG_AUTOREMOVE_CONSUMER?
Especially if fw_devlink already created the link? You are effectively
trying to delete the link fw_devlink created if any of your devices
unbind.
> While at it, make sure that we are never left with
> DL_FLAG_AUTOPROBE_CONSUMER set together with one of
> DL_FLAG_AUTOREMOVE_CONSUMER or DL_FLAG_AUTOREMOVE_SUPPLIER.
fw_devlink sets AUTOPROBE_CONSUMER. So, are you trying to clear it? Why?
I almost want to NACK this, but I'll hear more before I do.
-Saravana
>
> Signed-off-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
> ---
> drivers/base/core.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 14d46af40f9a..ee8a46df28e1 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -807,11 +807,15 @@ struct device_link *device_link_add(struct device *consumer,
> * update the existing link to stay around longer.
> */
> if (flags & DL_FLAG_AUTOREMOVE_SUPPLIER) {
> - if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER) {
> - link->flags &= ~DL_FLAG_AUTOREMOVE_CONSUMER;
> - link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER;
> - }
> - } else if (!(flags & DL_FLAG_AUTOREMOVE_CONSUMER)) {
> + link->flags &= ~DL_FLAG_AUTOREMOVE_CONSUMER;
> + link->flags &= ~DL_FLAG_AUTOPROBE_CONSUMER;
> + link->flags |= DL_FLAG_AUTOREMOVE_SUPPLIER;
> +
> + } else if (flags & DL_FLAG_AUTOREMOVE_CONSUMER) {
> + link->flags &= ~DL_FLAG_AUTOREMOVE_SUPPLIER;
> + link->flags &= ~DL_FLAG_AUTOPROBE_CONSUMER;
> + link->flags |= DL_FLAG_AUTOREMOVE_CONSUMER;
> + } else {
> link->flags &= ~(DL_FLAG_AUTOREMOVE_CONSUMER |
> DL_FLAG_AUTOREMOVE_SUPPLIER);
> }
>
> --
> 2.43.0
>
