linux-kernel - Re: [PATCH 12/19] media: i2c: ds90ub960: Add RX port iteration support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z4fE-qD7QvNiwOeH@kekkonen.localdomain>
Date: Wed, 15 Jan 2025 14:23:54 +0000
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>, linux-media@...r.kernel.org,
	linux-kernel@...r.kernel.org, Devarsh Thakkar <devarsht@...com>,
	Jai Luthra <jai.luthra@...asonboard.com>
Subject: Re: [PATCH 12/19] media: i2c: ds90ub960: Add RX port iteration
 support

Moi,

On Fri, Jan 10, 2025 at 11:14:12AM +0200, Tomi Valkeinen wrote:
> The driver does a lot of iteration over the RX ports with for loops. In
> most cases the driver will skip unused RX ports. Also, in the future
> patches the FPD-Link IV support will be refreshed with TI's latest init
> sequences which involves a lot of additional iterations over the RX
> ports, often only for FPD-Link IV ports.
> 
> To make the iteration simpler and to make it clearer what we're
> iterating over (all or only-active, all or only-fpd4), add macros and
> support functions for iterating the RX ports. Use the macros in the
> driver, replacing the for loops.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
> ---
>  drivers/media/i2c/ds90ub960.c | 260 ++++++++++++++++++++++--------------------
>  1 file changed, 135 insertions(+), 125 deletions(-)
> 
> diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c
> index bca858172942..02e22ae813fa 100644
> --- a/drivers/media/i2c/ds90ub960.c
> +++ b/drivers/media/i2c/ds90ub960.c
> @@ -649,6 +649,63 @@ static const struct ub960_format_info *ub960_find_format(u32 code)
>  	return NULL;
>  }
>  
> +struct ub960_rxport_iter {
> +	unsigned int nport;
> +	struct ub960_rxport *rxport;
> +};
> +
> +enum ub960_iter_flags {
> +	UB960_ITER_ACTIVE_ONLY = BIT(0),
> +	UB960_ITER_FPD4_ONLY = BIT(1),
> +};
> +
> +static struct ub960_rxport_iter ub960_iter_rxport(struct ub960_data *priv,
> +						  struct ub960_rxport_iter it,
> +						  enum ub960_iter_flags flags)
> +{
> +	for (; it.nport < priv->hw_data->num_rxports; it.nport++) {
> +		it.rxport = priv->rxports[it.nport];
> +
> +		if ((flags & UB960_ITER_ACTIVE_ONLY) && !it.rxport)
> +			continue;
> +
> +		if ((flags & UB960_ITER_FPD4_ONLY) &&
> +		    it.rxport->cdr_mode != RXPORT_CDR_FPD4)
> +			continue;
> +
> +		return it;
> +	}
> +
> +	it.rxport = NULL;
> +
> +	return it;
> +}
> +
> +#define for_each_rxport(priv)                                                 \

it should be also an argument to the macro as it's visible outside it.

And wouldn't it be reasonable to use a pointer instead for the purpsoe?

> +	for (struct ub960_rxport_iter it =                                    \
> +		     ub960_iter_rxport(priv, (struct ub960_rxport_iter){ 0 }, \
> +				       0);                                    \
> +	     it.nport < (priv)->hw_data->num_rxports;                         \
> +	     it.nport++, it = ub960_iter_rxport(priv, it, 0))
> +
> +#define for_each_active_rxport(priv)                                          \
> +	for (struct ub960_rxport_iter it =                                    \
> +		     ub960_iter_rxport(priv, (struct ub960_rxport_iter){ 0 }, \
> +				       UB960_ITER_ACTIVE_ONLY);               \
> +	     it.nport < (priv)->hw_data->num_rxports;                         \
> +	     it.nport++, it = ub960_iter_rxport(priv, it,                     \
> +						UB960_ITER_ACTIVE_ONLY))
> +
> +#define for_each_active_rxport_fpd4(priv)                                     \
> +	for (struct ub960_rxport_iter it =                                    \
> +		     ub960_iter_rxport(priv, (struct ub960_rxport_iter){ 0 }, \
> +				       UB960_ITER_ACTIVE_ONLY |               \
> +					       UB960_ITER_FPD4_ONLY);         \
> +	     it.nport < (priv)->hw_data->num_rxports;                         \
> +	     it.nport++, it = ub960_iter_rxport(priv, it,                     \
> +						UB960_ITER_ACTIVE_ONLY |      \
> +							UB960_ITER_FPD4_ONLY))

-- 
Terveisin,

Sakari Ailus