Re: [PATCH v4 1/6] i3c: master: add enable(disable) hot join in sys entry

Miquel Raynal
2023-11-30T10:13:32Z

Hi Frank,

Frank.Li@nxp.com wrote on Wed, 29 Nov 2023 17:12:20 -0500:

> Add hotjoin entry in sys file system allow user enable/disable hotjoin
> feature.
>
> Add (*enable(disable)_hotjoin)() to i3c_master_controller_ops.
> Add api i3c_master_enable(disable)_hotjoin();
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> drivers/i3c/master.c | 84 ++++++++++++++++++++++++++++++++++++++
> include/linux/i3c/master.h | 5 +++
> 2 files changed, 89 insertions(+)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 08aeb69a78003..ed5e27cd20811 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -526,6 +526,89 @@ static ssize_t i2c_scl_frequency_show(struct device *dev,
> }
> static DEVICE_ATTR_RO(i2c_scl_frequency);
>
> +static int i3c_set_hotjoin(struct i3c_master_controller *master, bool enable)
> +{
> + int ret;
> +
> + if (!master ||
> + !master->ops ||
> + !master->ops->enable_hotjoin ||
> + !master->ops->disable_hotjoin
> + )

Style is wrong here (max limit is 100 chars and the ) should not be on
a new line)

> + return -EINVAL;
> +
> + i3c_bus_normaluse_lock(&master->bus);
> +
> + if (enable)
> + ret = master->ops->enable_hotjoin(master);
> + else
> + ret = master->ops->disable_hotjoin(master);
> +
> + master->hotjoin = enable;
> +
> + i3c_bus_normaluse_unlock(&master->bus);
> +
> + return ret;
> +}
> +
> +static ssize_t hotjoin_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct i3c_bus *i3cbus = dev_to_i3cbus(dev);
> + int ret;
> + long res;
> +
> + if (!i3cbus->cur_master)
> + return -EINVAL;
> +
> + if (kstrtol(buf, 10, &res))
> + return -EINVAL;

Isn't there a better approach to get a y/n answer in sysfs?

> +
> + ret = i3c_set_hotjoin(i3cbus->cur_master->common.master, !!res);
> + if (ret)
> + return ret;
> +
> + return count;
> +}
> +

Thanks,
Miquèl