[PATCH v4 8/9] i2c: atr: add static flag

Cosmin Tanislav posted 9 patches 7 months, 3 weeks ago
There is a newer version of this series
[PATCH v4 8/9] i2c: atr: add static flag
Posted by Cosmin Tanislav 7 months, 3 weeks ago
Some I2C ATRs do not support dynamic remapping, only static mapping
of direct children.

Add a new flag that prevents old mappings to be replaced or new mappings
to be created in the alias finding code paths.

Signed-off-by: Cosmin Tanislav <demonsingur@gmail.com>
---
 drivers/i2c/i2c-atr.c   | 6 +++++-
 include/linux/i2c-atr.h | 3 +++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-atr.c b/drivers/i2c/i2c-atr.c
index e2350fcf3d68..721dd680f2ac 100644
--- a/drivers/i2c/i2c-atr.c
+++ b/drivers/i2c/i2c-atr.c
@@ -341,12 +341,16 @@ i2c_atr_create_mapping_by_addr(struct i2c_atr_chan *chan, u16 addr)
 static struct i2c_atr_alias_pair *
 i2c_atr_get_mapping_by_addr(struct i2c_atr_chan *chan, u16 addr)
 {
+	struct i2c_atr *atr = chan->atr;
 	struct i2c_atr_alias_pair *c2a;
 
 	c2a = i2c_atr_find_mapping_by_addr(chan, addr);
 	if (c2a)
 		return c2a;
 
+	if (atr->flags & I2C_ATR_F_STATIC)
+		return NULL;
+
 	c2a = i2c_atr_create_mapping_by_addr(chan, addr);
 	if (c2a)
 		return c2a;
@@ -545,7 +549,7 @@ static int i2c_atr_attach_addr(struct i2c_adapter *adapter,
 	mutex_lock(&chan->alias_pairs_lock);
 
 	c2a = i2c_atr_create_mapping_by_addr(chan, addr);
-	if (!c2a)
+	if (!c2a && !(atr->flags & I2C_ATR_F_STATIC))
 		c2a = i2c_atr_replace_mapping_by_addr(chan, addr);
 
 	if (!c2a) {
diff --git a/include/linux/i2c-atr.h b/include/linux/i2c-atr.h
index 5082f4dd0e23..7c6a9627191d 100644
--- a/include/linux/i2c-atr.h
+++ b/include/linux/i2c-atr.h
@@ -20,8 +20,11 @@ struct i2c_atr;
 
 /**
  * enum i2c_atr_flags - Flags for an I2C ATR driver
+ *
+ * @I2C_ATR_F_STATIC: ATR does not support dynamic mapping, use static mapping
  */
 enum i2c_atr_flags {
+	I2C_ATR_F_STATIC = BIT(0),
 };
 
 /**
-- 
2.49.0
Re: [PATCH v4 8/9] i2c: atr: add static flag
Posted by Romain Gantois 7 months, 2 weeks ago
Hi Cosmin,

On Monday, 28 April 2025 12:25:13 CEST Cosmin Tanislav wrote:
> Some I2C ATRs do not support dynamic remapping, only static mapping
> of direct children.
> 
> Add a new flag that prevents old mappings to be replaced or new mappings
> to be created in the alias finding code paths.
> 
> Signed-off-by: Cosmin Tanislav <demonsingur@gmail.com>
> ---
>  drivers/i2c/i2c-atr.c   | 6 +++++-
>  include/linux/i2c-atr.h | 3 +++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-atr.c b/drivers/i2c/i2c-atr.c
> index e2350fcf3d68..721dd680f2ac 100644
> --- a/drivers/i2c/i2c-atr.c
> +++ b/drivers/i2c/i2c-atr.c
> @@ -341,12 +341,16 @@ i2c_atr_create_mapping_by_addr(struct i2c_atr_chan
> *chan, u16 addr) static struct i2c_atr_alias_pair *
>  i2c_atr_get_mapping_by_addr(struct i2c_atr_chan *chan, u16 addr)
>  {
> +	struct i2c_atr *atr = chan->atr;
>  	struct i2c_atr_alias_pair *c2a;
> 
>  	c2a = i2c_atr_find_mapping_by_addr(chan, addr);
>  	if (c2a)
>  		return c2a;
> 
> +	if (atr->flags & I2C_ATR_F_STATIC)
> +		return NULL;
> +
...
>  		c2a = i2c_atr_replace_mapping_by_addr(chan, addr);
> 
>  	if (!c2a) {
> diff --git a/include/linux/i2c-atr.h b/include/linux/i2c-atr.h
> index 5082f4dd0e23..7c6a9627191d 100644
> --- a/include/linux/i2c-atr.h
> +++ b/include/linux/i2c-atr.h
> @@ -20,8 +20,11 @@ struct i2c_atr;
> 
>  /**
>   * enum i2c_atr_flags - Flags for an I2C ATR driver
> + *
> + * @I2C_ATR_F_STATIC: ATR does not support dynamic mapping, use static
> mapping */

I would suggest clarifying a bit more what "dynamic mapping" means in this 
doc. Maybe something like "ATR does not support on-the-fly modifications of its 
translation table, use static mappings". IMO this will make it easier for 
people who don't have the full technical context of this series and want to 
understand what the flag is for.

Everything else looks good to me.

Thanks,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH v4 8/9] i2c: atr: add static flag
Posted by Luca Ceresoli 7 months, 3 weeks ago
On Mon, 28 Apr 2025 13:25:13 +0300
Cosmin Tanislav <demonsingur@gmail.com> wrote:

> Some I2C ATRs do not support dynamic remapping, only static mapping
> of direct children.
> 
> Add a new flag that prevents old mappings to be replaced or new mappings
> to be created in the alias finding code paths.
> 
> Signed-off-by: Cosmin Tanislav <demonsingur@gmail.com>
> ---
>  drivers/i2c/i2c-atr.c   | 6 +++++-
>  include/linux/i2c-atr.h | 3 +++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-atr.c b/drivers/i2c/i2c-atr.c
> index e2350fcf3d68..721dd680f2ac 100644
> --- a/drivers/i2c/i2c-atr.c
> +++ b/drivers/i2c/i2c-atr.c
> @@ -341,12 +341,16 @@ i2c_atr_create_mapping_by_addr(struct i2c_atr_chan *chan, u16 addr)
>  static struct i2c_atr_alias_pair *
>  i2c_atr_get_mapping_by_addr(struct i2c_atr_chan *chan, u16 addr)
>  {
> +	struct i2c_atr *atr = chan->atr;
>  	struct i2c_atr_alias_pair *c2a;
>  
>  	c2a = i2c_atr_find_mapping_by_addr(chan, addr);
>  	if (c2a)
>  		return c2a;
>  
> +	if (atr->flags & I2C_ATR_F_STATIC)
> +		return NULL;
> +
>  	c2a = i2c_atr_create_mapping_by_addr(chan, addr);
>  	if (c2a)
>  		return c2a;
> @@ -545,7 +549,7 @@ static int i2c_atr_attach_addr(struct i2c_adapter *adapter,
>  	mutex_lock(&chan->alias_pairs_lock);
>  
>  	c2a = i2c_atr_create_mapping_by_addr(chan, addr);
> -	if (!c2a)
> +	if (!c2a && !(atr->flags & I2C_ATR_F_STATIC))
>  		c2a = i2c_atr_replace_mapping_by_addr(chan, addr);
>  
>  	if (!c2a) {
> diff --git a/include/linux/i2c-atr.h b/include/linux/i2c-atr.h
> index 5082f4dd0e23..7c6a9627191d 100644
> --- a/include/linux/i2c-atr.h
> +++ b/include/linux/i2c-atr.h
> @@ -20,8 +20,11 @@ struct i2c_atr;
>  
>  /**
>   * enum i2c_atr_flags - Flags for an I2C ATR driver
> + *
> + * @I2C_ATR_F_STATIC: ATR does not support dynamic mapping, use static mapping

Maybe add something along the lines of: "mappings can only be
added/removed by the higher level driver via
i2c_atr_attach_addr/i2c_atr_detach_addr"

Other than that, looks good.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com