Hi,
On vendredi 29 novembre 2024 14:46:38 heure normale d’Europe centrale Tomi
Valkeinen wrote:
> Hi,
>
> On 25/11/2024 10:45, Romain Gantois wrote:
> > The ds90ub960 driver currently uses a list of i2c_client structs to keep
> > track of used I2C address translator (ATR) alias slots for each RX port.
...
> > dev_err(dev, "rx%u: alias pool exhausted\n", rxport->nport);
> > return -EADDRNOTAVAIL;
> >
> > }
> >
> > - rxport->aliased_clients[reg_idx] = client;
> > + set_bit(reg_idx, rxport->alias_use_mask);
> >
> > ub960_rxport_write(priv, chan_id, UB960_RR_SLAVE_ID(reg_idx),
> >
> > client->addr << 1);
> >
> > @@ -1063,18 +1059,15 @@ static void ub960_atr_detach_client(struct i2c_atr
> > *atr, u32 chan_id,>
> > struct device *dev = &priv->client->dev;
> > unsigned int reg_idx;
> >
> > - for (reg_idx = 0; reg_idx < ARRAY_SIZE(rxport->aliased_clients);
> > reg_idx++) { - if (rxport->aliased_clients[reg_idx] == client)
> > - break;
> > - }
> > + reg_idx = find_first_zero_bit(rxport->alias_use_mask,
> > UB960_MAX_PORT_ALIASES);
> The old code went through the alias table to find the matching client,
> so that it can be removed. The new code... Tries to find the first
> unused entry in the mask, to... free it?
>
> I'm not sure how this is supposed to work, or how the driver even could
> manage with just a bit mask. The driver needs to remove the one that was
> assigned in ub960_atr_attach_addr(), so it somehow has to find the same
> entry using the address or the alias.
Indeed, there is an issue here. Tracking client addresses is still required,
so the correct change would be to convert aliased_clients to aliased_addrs.
Thanks,
--
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
