| 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: <20200511181431.442cdf0b@collabora.com>
Date: Mon, 11 May 2020 18:14:31 +0200
From: Boris Brezillon <boris.brezillon@...labora.com>
To: Parshuram Thombare <pthombar@...ence.com>
Cc: <bbrezillon@...nel.org>, <vitor.soares@...opsys.com>,
<pgaj@...ence.com>, <linux-i3c@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>, <mparab@...ence.com>,
<praneeth@...com>
Subject: Re: [PATCH v7 3/7] i3c: master: add i3c_secondary_master_register
On Mon, 11 May 2020 15:13:53 +0200
Parshuram Thombare <pthombar@...ence.com> wrote:
> add i3c_secondary_master_register which is used
> to register secondary masters.
>
> Signed-off-by: Parshuram Thombare <pthombar@...ence.com>
> ---
> drivers/i3c/master.c | 154 ++++++++++++++++++++++++++++++++++++-
> include/linux/i3c/master.h | 3 +
> 2 files changed, 156 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index ba07a7d49633..669bd7e45810 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -1768,6 +1768,90 @@ static int i3c_master_bus_init(struct i3c_master_controller *master)
> return ret;
> }
>
> +/**
> + * i3c_secondary_master_bus_init() - initialize an I3C bus for secondary
> + * master
> + * @master: secondary master initializing the bus
> + *
> + * This function does
> + *
> + * 1. Attach I2C devs to the master
> + *
> + * 2. Call &i3c_master_controller_ops->bus_init() method to initialize
> + * the master controller. That's usually where the bus mode is selected
> + * (pure bus or mixed fast/slow bus)
Can you really select the bus mode without knowing the I3C devices you
have on the bus? Or maybe that's a preliminary initialization which is
then updated when you receive DEFSLVS events.
> + *
> + * Once this is done, I2C devices should be usable.
I suspect you'll have to request bus ownership first, which means
they're not really usable, just registered to the I2C subsystem. This
might lead to a whole bunch of problems when drivers will try to send
messages to the I2C devices and receive ETIMEDOUT/EBUSY errors. We'll
probably need some updates to the I2C framework if we want I2C to play
nicely with bus handover, but I think we can keep that for later. I'd
suggest to forget about I2C for now and mark that as a limitation (no
I2C devs on secondary masters).
> + *
> + * Return: a 0 in case of success, an negative error code otherwise.
> + */
> +static int i3c_secondary_master_bus_init(struct i3c_master_controller *master)
> +{
> + enum i3c_addr_slot_status status;
> + struct i2c_dev_boardinfo *i2cboardinfo;
> + struct i2c_dev_desc *i2cdev;
> + int ret;
> +
> + /*
> + * First attach all devices with static definitions provided by the
> + * FW.
> + */
> + list_for_each_entry(i2cboardinfo, &master->boardinfo.i2c, node) {
> + status = i3c_bus_get_addr_slot_status(&master->bus,
> + i2cboardinfo->base.addr);
> + if (status != I3C_ADDR_SLOT_FREE) {
> + ret = -EBUSY;
> + goto err_detach_devs;
> + }
> +
> + i3c_bus_set_addr_slot_status(&master->bus,
> + i2cboardinfo->base.addr,
> + I3C_ADDR_SLOT_I2C_DEV);
> +
> + i2cdev = i3c_master_alloc_i2c_dev(master, i2cboardinfo);
> + if (IS_ERR(i2cdev)) {
> + ret = PTR_ERR(i2cdev);
> + goto err_detach_devs;
> + }
> +
> + ret = i3c_master_attach_i2c_dev(master, i2cdev);
> + if (ret) {
> + i3c_master_free_i2c_dev(i2cdev);
> + goto err_detach_devs;
> + }
> + }
> +
> + /*
> + * Now execute the controller specific ->bus_init() routine, which
> + * might configure its internal logic to match the bus limitations.
> + */
> + ret = master->ops->bus_init(master);
> + if (ret)
> + goto err_detach_devs;
> +
> + /*
> + * The master device should have been instantiated in ->bus_init(),
> + * complain if this was not the case.
> + */
> + if (!master->this) {
> + dev_err(&master->dev,
> + "master_set_info() was not called in ->bus_init()\n");
> + ret = -EINVAL;
> + goto err_bus_cleanup;
> + }
> +
> + return 0;
> +
> +err_bus_cleanup:
> + if (master->ops->bus_cleanup)
> + master->ops->bus_cleanup(master);
> +
> +err_detach_devs:
> + i3c_master_detach_free_devs(master);
> +
> + return ret;
> +}
> +
> static void i3c_master_bus_cleanup(struct i3c_master_controller *master)
> {
> if (master->ops->bus_cleanup)
> @@ -2457,7 +2541,10 @@ static int i3c_master_init(struct i3c_master_controller *master,
> goto err_put_dev;
> }
>
> - ret = i3c_master_bus_init(master);
> + if (secondary)
> + ret = i3c_secondary_master_bus_init(master);
> + else
> + ret = i3c_master_bus_init(master);
> if (ret)
> goto err_put_dev;
>
> @@ -2535,6 +2622,71 @@ int i3c_master_register(struct i3c_master_controller *master,
> }
> EXPORT_SYMBOL_GPL(i3c_master_register);
>
> +/**
> + * i3c_secondary_master_register() - register an I3C secondary master
> + * @master: master used to send frames on the bus
> + * @parent: the parent device (the one that provides this I3C master
> + * controller)
> + * @ops: the master controller operations
> + *
> + * This function does minimal required initialization for secondary
> + * master, rest functionality like creating and registering I2C
> + * and I3C devices is done in defslvs processing.
> + *
> + * i3c_secondary_master_register() does following things -
> + * - creates and initializes the I3C bus
> + * - populates the bus with static I2C devs if @parent->of_node is not
> + * NULL
> + * initialization
> + * - allocate memory for defslvs_data.devs, which is used to receive
> + * defslvs list
> + * - create I3C device representing this master
> + * - registers the I2C adapter and all I2C devices
> + *
> + * Return: 0 in case of success, a negative error code otherwise.
> + */
> +int i3c_secondary_master_register(struct i3c_master_controller *master,
> + struct device *parent,
> + const struct i3c_master_controller_ops *ops)
> +{
> + int ret;
> +
> + ret = i3c_master_init(master, parent, ops, true);
> + if (ret)
> + return ret;
> +
> + ret = device_add(&master->dev);
> + if (ret)
> + goto err_cleanup_bus;
> +
> + /*
> + * Expose our I3C bus as an I2C adapter so that I2C devices are exposed
> + * through the I2C subsystem.
> + */
> + ret = i3c_master_i2c_adapter_init(master);
> + if (ret)
> + goto err_del_dev;
> +
> + /*
> + * We're done initializing the bus and the controller, we can now
> + * register I3C devices from defslvs list.
> + */
> + master->init_done = true;
> +
> + return 0;
> +
> +err_del_dev:
> + device_del(&master->dev);
> +
> +err_cleanup_bus:
> + i3c_master_bus_cleanup(master);
> +
> + put_device(&master->dev);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(i3c_secondary_master_register);
> +
> /**
> * i3c_master_unregister() - unregister an I3C master
> * @master: master used to send frames on the bus
> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> index f5ba82c390bc..5124ff4831eb 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -534,6 +534,9 @@ int i3c_master_set_info(struct i3c_master_controller *master,
> int i3c_master_register(struct i3c_master_controller *master,
> struct device *parent,
> const struct i3c_master_controller_ops *ops);
> +int i3c_secondary_master_register(struct i3c_master_controller *master,
> + struct device *parent,
> + const struct i3c_master_controller_ops *ops);
> int i3c_master_unregister(struct i3c_master_controller *master);
>
> /**
Powered by blists - more mailing lists