On Fri, Nov 27, 2020 at 05:05:21PM +0100, Loic Poulain wrote:
> Today the MHI controller name is simply cloned from the underlying
> bus device (its parent), that gives the following device structure
> for e.g. a MHI/PCI controller:
> devices/pci0000:00/0000:00:01.2/0000:02:00.0/0000:02:00.0
> devices/pci0000:00/0000:00:01.2/0000:02:00.0/0000:02:00.0/0000:02:00.0_IPCR
> ...
>
> That's quite misleading/confusing and can cause device registering
> issues because of duplicate dev name (e.g. if a PCI device register
> two different MHI instances).
>
> This patch changes MHI core to create indexed mhi controller names
> (mhi0, mhi1...) in the same way as other busses (i2c0, usb0...).
>
> The previous example becomes:
> devices/pci0000:00/0000:00:01.2/0000:02:00.0/mhi0
> devices/pci0000:00/0000:00:01.2/0000:02:00.0/mhi0/mhi0_IPCR
> ...
>
> Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxx>
> ---
> v2: move index field at the end of mhi_controller struct (before bool)
> to avoid breaking well packed alignment.
> v3: Align debugfs directory name
>
> drivers/bus/mhi/core/debugfs.c | 2 +-
> drivers/bus/mhi/core/init.c | 18 ++++++++++++++++--
> drivers/bus/mhi/core/main.c | 2 +-
> include/linux/mhi.h | 2 ++
> 4 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bus/mhi/core/debugfs.c b/drivers/bus/mhi/core/debugfs.c
> index 7d43138..858d751 100644
> --- a/drivers/bus/mhi/core/debugfs.c
> +++ b/drivers/bus/mhi/core/debugfs.c
> @@ -377,7 +377,7 @@ static struct dentry *mhi_debugfs_root;
> void mhi_create_debugfs(struct mhi_controller *mhi_cntrl)
> {
> mhi_cntrl->debugfs_dentry =
> - debugfs_create_dir(dev_name(mhi_cntrl->cntrl_dev),
> + debugfs_create_dir(dev_name(&mhi_cntrl->mhi_dev->dev),
> mhi_debugfs_root);
This change doesn't belong to this patch, right? This should be part of
"bus: mhi: core: Fix device hierarchy".
Thanks,
Mani
>
> debugfs_create_file("states", 0444, mhi_cntrl->debugfs_dentry,
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index c7a7354..4acad28 100644
> --- a/drivers/bus/mhi/core/init.c
> +++ b/drivers/bus/mhi/core/init.c
> @@ -8,6 +8,7 @@
> #include <linux/device.h>
> #include <linux/dma-direction.h>
> #include <linux/dma-mapping.h>
> +#include <linux/idr.h>
> #include <linux/interrupt.h>
> #include <linux/list.h>
> #include <linux/mhi.h>
> @@ -18,6 +19,8 @@
> #include <linux/wait.h>
> #include "internal.h"
>
> +static DEFINE_IDA(mhi_controller_ida);
> +
> const char * const mhi_ee_str[MHI_EE_MAX] = {
> [MHI_EE_PBL] = "PBL",
> [MHI_EE_SBL] = "SBL",
> @@ -940,6 +943,12 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl,
> mhi_cntrl->minor_version = (soc_info & SOC_HW_VERSION_MINOR_VER_BMSK) >>
> SOC_HW_VERSION_MINOR_VER_SHFT;
>
> + mhi_cntrl->index = ida_alloc(&mhi_controller_ida, GFP_KERNEL);
> + if (mhi_cntrl->index < 0) {
> + ret = mhi_cntrl->index;
> + goto error_ida_alloc;
> + }
> +
> /* Register controller with MHI bus */
> mhi_dev = mhi_alloc_device(mhi_cntrl);
> if (IS_ERR(mhi_dev)) {
> @@ -950,8 +959,8 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl,
>
> mhi_dev->dev_type = MHI_DEVICE_CONTROLLER;
> mhi_dev->mhi_cntrl = mhi_cntrl;
> - dev_set_name(&mhi_dev->dev, "%s", dev_name(mhi_cntrl->cntrl_dev));
> - mhi_dev->name = dev_name(mhi_cntrl->cntrl_dev);
> + dev_set_name(&mhi_dev->dev, "mhi%d", mhi_cntrl->index);
> + mhi_dev->name = dev_name(&mhi_dev->dev);
>
> /* Init wakeup source */
> device_init_wakeup(&mhi_dev->dev, true);
> @@ -970,6 +979,9 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl,
> put_device(&mhi_dev->dev);
>
> error_alloc_dev:
> + ida_free(&mhi_controller_ida, mhi_cntrl->index);
> +
> +error_ida_alloc:
> kfree(mhi_cntrl->mhi_cmd);
>
> error_alloc_cmd:
> @@ -1004,6 +1016,8 @@ void mhi_unregister_controller(struct mhi_controller *mhi_cntrl)
>
> device_del(&mhi_dev->dev);
> put_device(&mhi_dev->dev);
> +
> + ida_free(&mhi_controller_ida, mhi_cntrl->index);
> }
> EXPORT_SYMBOL_GPL(mhi_unregister_controller);
>
> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
> index 7f3c990..ea39df0 100644
> --- a/drivers/bus/mhi/core/main.c
> +++ b/drivers/bus/mhi/core/main.c
> @@ -350,7 +350,7 @@ void mhi_create_devices(struct mhi_controller *mhi_cntrl)
> /* Channel name is same for both UL and DL */
> mhi_dev->name = mhi_chan->name;
> dev_set_name(&mhi_dev->dev, "%s_%s",
> - dev_name(mhi_cntrl->cntrl_dev),
> + dev_name(&mhi_cntrl->mhi_dev->dev),
> mhi_dev->name);
>
> /* Init wakeup source if available */
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index 27078db..30c676d 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -356,6 +356,7 @@ struct mhi_controller_config {
> * @read_reg: Read a MHI register via the physical link (required)
> * @write_reg: Write a MHI register via the physical link (required)
> * @buffer_len: Bounce buffer length
> + * @index: Index of the MHI controller instance
> * @bounce_buf: Use of bounce buffer
> * @fbc_download: MHI host needs to do complete image transfer (optional)
> * @pre_init: MHI host needs to do pre-initialization before power up
> @@ -446,6 +447,7 @@ struct mhi_controller {
> u32 val);
>
> size_t buffer_len;
> + int index;
> bool bounce_buf;
> bool fbc_download;
> bool pre_init;
> --
> 2.7.4
>
