| 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
| ||
|
Message-ID: <10646964.nUPlyArG6x@fw-rgant>
Date: Thu, 27 Feb 2025 14:29:47 +0100
From: Romain Gantois <romain.gantois@...tlin.com>
To: Cosmin Tanislav <demonsingur@...il.com>
Cc: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>,
Luca Ceresoli <luca.ceresoli@...tlin.com>,
Wolfram Sang <wsa+renesas@...g-engineering.com>, linux-i2c@...r.kernel.org,
linux-kernel@...r.kernel.org, Cosmin Tanislav <demonsingur@...il.com>
Subject: Re: [PATCH v2 4/9] i2c: atr: split up i2c_atr_get_mapping_by_addr()
On mardi 25 février 2025 12:39:32 heure normale d’Europe centrale Cosmin
Tanislav wrote:
> The i2c_atr_get_mapping_by_addr() function handles three separate
> usecases: finding an existing mapping, creating a new mapping, or
> replacing an existing mapping if a new mapping cannot be created
> because there aren't enough aliases available.
>
> Split up the function into three different functions handling its
> individual usecases to prepare for better usage of each one.
>
> Signed-off-by: Cosmin Tanislav <demonsingur@...il.com>
> ---
> drivers/i2c/i2c-atr.c | 108 ++++++++++++++++++++++++++++++------------
> 1 file changed, 79 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/i2c/i2c-atr.c b/drivers/i2c/i2c-atr.c
> index f2485d1670a2..9c4e9e8ec802 100644
> --- a/drivers/i2c/i2c-atr.c
> +++ b/drivers/i2c/i2c-atr.c
> @@ -239,9 +239,26 @@ static void i2c_atr_release_alias(struct
> i2c_atr_alias_pool *alias_pool, u16 ali spin_unlock(&alias_pool->lock);
> }
>
> -/* Must be called with alias_pairs_lock held */
> static struct i2c_atr_alias_pair *
> -i2c_atr_get_mapping_by_addr(struct i2c_atr_chan *chan, u16 addr)
> +i2c_atr_find_mapping_by_addr(struct i2c_atr_chan *chan, u16 addr)
> +{
> + struct i2c_atr_alias_pair *c2a;
> + struct list_head *alias_pairs;
> +
> + lockdep_assert_held(&chan->alias_pairs_lock);
> +
> + alias_pairs = &chan->alias_pairs;
This variable isn't really needed since it's only being used once.
> +
> + list_for_each_entry(c2a, alias_pairs, node) {
> + if (c2a->addr == addr)
> + return c2a;
> + }
> +
> + return NULL;
> +}
> +
> +static struct i2c_atr_alias_pair *
> +i2c_atr_replace_mapping_by_addr(struct i2c_atr_chan *chan, u16 addr)
> {
> struct i2c_atr *atr = chan->atr;
> struct i2c_atr_alias_pair *c2a;
> @@ -253,38 +270,55 @@ i2c_atr_get_mapping_by_addr(struct i2c_atr_chan *chan,
> u16 addr)
>
> alias_pairs = &chan->alias_pairs;
>
> - list_for_each_entry(c2a, alias_pairs, node) {
> - if (c2a->addr == addr)
> - return c2a;
> + if (unlikely(list_empty(alias_pairs)))
> + return NULL;
> +
> + list_for_each_entry_reverse(c2a, alias_pairs, node)
> + if (!c2a->fixed)
> + break;
> +
> + if (c2a->fixed)
> + return NULL;
> +
> + atr->ops->detach_addr(atr, chan->chan_id, c2a->addr);
> + c2a->addr = addr;
> +
> + list_move(&c2a->node, alias_pairs);
> +
> + alias = c2a->alias;
> +
> + ret = atr->ops->attach_addr(atr, chan->chan_id, c2a->addr, c2a-
>alias);
> + if (ret) {
> + dev_err(atr->dev, "failed to attach 0x%02x on channel %d: err
%d\n",
> + addr, chan->chan_id, ret);
> + goto err_del_c2a;
I think this goto should be removed, the corresponding label code is only used
here anyway.
> }
>
> + return c2a;
> +
> +err_del_c2a:
> + i2c_atr_destroy_c2a(&c2a);
> + i2c_atr_release_alias(chan->alias_pool, alias);
> + return NULL;
> +}
> +
> +static struct i2c_atr_alias_pair *
> +i2c_atr_create_mapping_by_addr(struct i2c_atr_chan *chan, u16 addr)
> +{
> + struct i2c_atr *atr = chan->atr;
> + struct i2c_atr_alias_pair *c2a;
> + u16 alias;
> + int ret;
> +
> ret = i2c_atr_reserve_alias(chan->alias_pool);
> - if (ret < 0) {
> - // If no free aliases are left, replace an existing one
> - if (unlikely(list_empty(alias_pairs)))
> - return NULL;
> + if (ret < 0)
> + return NULL;
>
> - list_for_each_entry_reverse(c2a, alias_pairs, node)
> - if (!c2a->fixed)
> - break;
> + alias = ret;
>
> - if (c2a->fixed)
> - return NULL;
> -
> - atr->ops->detach_addr(atr, chan->chan_id, c2a->addr);
> - c2a->addr = addr;
> -
> - // Move updated entry to beginning of list
> - list_move(&c2a->node, alias_pairs);
> -
> - alias = c2a->alias;
> - } else {
> - alias = ret;
> -
> - c2a = i2c_atr_create_c2a(chan, alias, addr);
> - if (!c2a)
> - goto err_release_alias;
> - }
> + c2a = i2c_atr_create_c2a(chan, alias, addr);
> + if (!c2a)
> + goto err_release_alias;
>
> ret = atr->ops->attach_addr(atr, chan->chan_id, c2a->addr, c2a-
>alias);
> if (ret) {
> @@ -302,6 +336,22 @@ i2c_atr_get_mapping_by_addr(struct i2c_atr_chan *chan,
> u16 addr) return NULL;
> }
>
> +static struct i2c_atr_alias_pair *
> +i2c_atr_get_mapping_by_addr(struct i2c_atr_chan *chan, u16 addr)
> +{
> + struct i2c_atr_alias_pair *c2a;
> +
> + c2a = i2c_atr_find_mapping_by_addr(chan, addr);
> + if (c2a)
> + return c2a;
> +
> + c2a = i2c_atr_create_mapping_by_addr(chan, addr);
> + if (c2a)
> + return c2a;
> +
> + return i2c_atr_replace_mapping_by_addr(chan, addr);
> +}
> +
> /*
> * Replace all message addresses with their aliases, saving the original
> * addresses.
Best Regards,
--
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists