| 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: <4439912.ejJDZkT8p0@fw-rgant>
Date: Thu, 27 Feb 2025 14:58:56 +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 8/9] i2c: atr: add static flag
On mardi 25 février 2025 12:39:36 heure normale d’Europe centrale 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@...il.com>
> ---
> drivers/i2c/i2c-atr.c | 16 ++++++++++++----
> include/linux/i2c-atr.h | 20 +++++++++++++++++---
> 2 files changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/i2c-atr.c b/drivers/i2c/i2c-atr.c
> index d8748d71ae15..f7b853f55630 100644
> --- a/drivers/i2c/i2c-atr.c
> +++ b/drivers/i2c/i2c-atr.c
> @@ -106,6 +106,7 @@ struct i2c_atr_chan {
> * @lock: Lock for the I2C bus segment (see &struct
> i2c_lock_operations) * @lock_key: Lock key for @lock
> * @max_adapters: Maximum number of adapters this I2C ATR can have
> + * @flags: Flags for ATR
> * @alias_pool: Optional common pool of available client aliases
> * @i2c_nb: Notifier for remote client add & del events
> * @adapter: Array of adapters
> @@ -122,6 +123,7 @@ struct i2c_atr {
> struct mutex lock;
> struct lock_class_key lock_key;
> int max_adapters;
> + u32 flags;
>
> struct i2c_atr_alias_pool *alias_pool;
>
> @@ -339,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_STATIC)
> + return NULL;
> +
> c2a = i2c_atr_create_mapping_by_addr(chan, addr);
> if (c2a)
> return c2a;
> @@ -543,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_STATIC))
> c2a = i2c_atr_replace_mapping_by_addr(chan, addr);
>
> if (!c2a) {
> @@ -702,8 +708,9 @@ static int i2c_atr_parse_alias_pool(struct i2c_atr *atr)
> return ret;
> }
>
> -struct i2c_atr *i2c_atr_new(struct i2c_adapter *parent, struct device *dev,
> - const struct i2c_atr_ops *ops, int max_adapters)
> +struct i2c_atr *i2c_atr_new_flags(struct i2c_adapter *parent, struct device
> *dev, + const struct i2c_atr_ops *ops, int
max_adapters,
> + u32 flags)
> {
> struct i2c_atr *atr;
> int ret;
> @@ -725,6 +732,7 @@ struct i2c_atr *i2c_atr_new(struct i2c_adapter *parent,
> struct device *dev, atr->dev = dev;
> atr->ops = ops;
> atr->max_adapters = max_adapters;
> + atr->flags = flags;
>
> if (parent->algo->master_xfer)
> atr->algo.master_xfer = i2c_atr_master_xfer;
> @@ -752,7 +760,7 @@ struct i2c_atr *i2c_atr_new(struct i2c_adapter *parent,
> struct device *dev,
>
> return ERR_PTR(ret);
> }
> -EXPORT_SYMBOL_NS_GPL(i2c_atr_new, "I2C_ATR");
> +EXPORT_SYMBOL_NS_GPL(i2c_atr_new_flags, "I2C_ATR");
>
> void i2c_atr_delete(struct i2c_atr *atr)
> {
> diff --git a/include/linux/i2c-atr.h b/include/linux/i2c-atr.h
> index 1c3a5bcd939f..2f79d0d9140f 100644
> --- a/include/linux/i2c-atr.h
> +++ b/include/linux/i2c-atr.h
> @@ -18,6 +18,15 @@ struct device;
> struct fwnode_handle;
> struct i2c_atr;
>
> +/**
> + * enum i2c_atr_flags - Flags for an I2C ATR driver
> + *
> + * @I2C_ATR_STATIC: ATR does not support dynamic mapping, use static
> mapping + */
> +enum i2c_atr_flags {
> + I2C_ATR_STATIC = BIT(0),
I'd personally prefer the name I2C_ATR_F_STATIC, which would make it explicit
that this is a flag.
> +};
> +
> /**
> * struct i2c_atr_ops - Callbacks from ATR to the device driver.
> * @attach_addr: Notify the driver of a new device connected on a child
> @@ -60,11 +69,12 @@ struct i2c_atr_adap_desc {
> };
>
> /**
> - * i2c_atr_new() - Allocate and initialize an I2C ATR helper.
> + * i2c_atr_new_flags() - Allocate and initialize an I2C ATR helper.
> * @parent: The parent (upstream) adapter
> * @dev: The device acting as an ATR
> * @ops: Driver-specific callbacks
> * @max_adapters: Maximum number of child adapters
> + * @flags: Flags for ATR
> *
> * The new ATR helper is connected to the parent adapter but has no child
> * adapters. Call i2c_atr_add_adapter() to add some.
> @@ -73,8 +83,12 @@ struct i2c_atr_adap_desc {
> *
> * Return: pointer to the new ATR helper object, or ERR_PTR
> */
> -struct i2c_atr *i2c_atr_new(struct i2c_adapter *parent, struct device *dev,
> - const struct i2c_atr_ops *ops, int max_adapters);
> +struct i2c_atr *i2c_atr_new_flags(struct i2c_adapter *parent, struct device
> *dev, + const struct i2c_atr_ops *ops, int
max_adapters,
> + u32 flags);
> +
> +#define i2c_atr_new(parent, dev, ops, max_adapters) \
> + i2c_atr_new_flags(parent, dev, ops, max_adapters, 0)
There aren't that many users of i2c_atr_new() yet, so I think it would be
preferable to just add the "0" parameter to existing callers (including
FPC202 which this series depends on) rather than adding a layer of
indirection.
Thanks,
--
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