Hi,
On 15/01/2025 16:23, Sakari Ailus wrote:
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@xxxxxxxxxxxxxxxx>
---
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?
You mean something like:
struct ub960_rxport_iter it = { 0 };
for_each_rxport(priv, &it) { }
Then we leak the iterator, and I really hate it. I've fixed numerous
bugs caused by such cases.
Tomi
+ 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))
