Re: [PATCH v6 3/8] driver: core: allow modifying device_links flags

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 19 Jan 2024 17:00:49 +0100
Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:

> From: Nuno Sa <nuno.sa@analog.com>
> 
> 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.
> 
> 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.
> 
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>

Looking for an Ack or RB from someone more familiar with device links
than I am.

Greg / Rafael?  

thanks

Jonathan

> ---
>  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 67ba592afc77..fdbb5abc75d5 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);
>  		}
>