[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220212183200.GB9933@thinkpad>
Date: Sun, 13 Feb 2022 00:02:00 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: mhi@...ts.linux.dev
Cc: quic_hemantk@...cinc.com, quic_bbhatt@...cinc.com,
quic_jhugo@...cinc.com, vinod.koul@...aro.org,
bjorn.andersson@...aro.org, dmitry.baryshkov@...aro.org,
quic_vbadigan@...cinc.com, quic_cang@...cinc.com,
quic_skananth@...cinc.com, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org, elder@...aro.org
Subject: Re: [PATCH v3 09/25] bus: mhi: ep: Add support for registering MHI
endpoint client drivers
On Sat, Feb 12, 2022 at 11:51:01PM +0530, Manivannan Sadhasivam wrote:
> This commit adds support for registering MHI endpoint client drivers
> with the MHI endpoint stack. MHI endpoint client drivers binds to one
> or more MHI endpoint devices inorder to send and receive the upper-layer
> protocol packets like IP packets, modem control messages, and diagnostics
> messages over MHI bus.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> ---
> drivers/bus/mhi/ep/main.c | 86 +++++++++++++++++++++++++++++++++++++++
> include/linux/mhi_ep.h | 53 ++++++++++++++++++++++++
> 2 files changed, 139 insertions(+)
>
> diff --git a/drivers/bus/mhi/ep/main.c b/drivers/bus/mhi/ep/main.c
> index b006011d025d..f66404181972 100644
> --- a/drivers/bus/mhi/ep/main.c
> +++ b/drivers/bus/mhi/ep/main.c
> @@ -196,9 +196,89 @@ void mhi_ep_unregister_controller(struct mhi_ep_cntrl *mhi_cntrl)
> }
> EXPORT_SYMBOL_GPL(mhi_ep_unregister_controller);
>
> +static int mhi_ep_driver_probe(struct device *dev)
> +{
> + struct mhi_ep_device *mhi_dev = to_mhi_ep_device(dev);
> + struct mhi_ep_driver *mhi_drv = to_mhi_ep_driver(dev->driver);
> + struct mhi_ep_chan *ul_chan = mhi_dev->ul_chan;
> + struct mhi_ep_chan *dl_chan = mhi_dev->dl_chan;
> +
> + /* Client drivers should have callbacks for both channels */
> + if (!mhi_drv->ul_xfer_cb || !mhi_drv->dl_xfer_cb)
> + return -EINVAL;
> +
Hmm, I had a change that moved this check to __mhi_ep_driver_register() but I
missed to apply it. Will do it in next iteration.
Thanks,
Mani
> + ul_chan->xfer_cb = mhi_drv->ul_xfer_cb;
> + dl_chan->xfer_cb = mhi_drv->dl_xfer_cb;
> +
> + return mhi_drv->probe(mhi_dev, mhi_dev->id);
> +}
> +
> +static int mhi_ep_driver_remove(struct device *dev)
> +{
> + struct mhi_ep_device *mhi_dev = to_mhi_ep_device(dev);
> + struct mhi_ep_driver *mhi_drv = to_mhi_ep_driver(dev->driver);
> + struct mhi_result result = {};
> + struct mhi_ep_chan *mhi_chan;
> + int dir;
> +
> + /* Skip if it is a controller device */
> + if (mhi_dev->dev_type == MHI_DEVICE_CONTROLLER)
> + return 0;
> +
> + /* Disconnect the channels associated with the driver */
> + for (dir = 0; dir < 2; dir++) {
> + mhi_chan = dir ? mhi_dev->ul_chan : mhi_dev->dl_chan;
> +
> + if (!mhi_chan)
> + continue;
> +
> + mutex_lock(&mhi_chan->lock);
> + /* Send channel disconnect status to the client driver */
> + if (mhi_chan->xfer_cb) {
> + result.transaction_status = -ENOTCONN;
> + result.bytes_xferd = 0;
> + mhi_chan->xfer_cb(mhi_chan->mhi_dev, &result);
> + }
> +
> + /* Set channel state to DISABLED */
> + mhi_chan->state = MHI_CH_STATE_DISABLED;
> + mhi_chan->xfer_cb = NULL;
> + mutex_unlock(&mhi_chan->lock);
> + }
> +
> + /* Remove the client driver now */
> + mhi_drv->remove(mhi_dev);
> +
> + return 0;
> +}
> +
> +int __mhi_ep_driver_register(struct mhi_ep_driver *mhi_drv, struct module *owner)
> +{
> + struct device_driver *driver = &mhi_drv->driver;
> +
> + if (!mhi_drv->probe || !mhi_drv->remove)
> + return -EINVAL;
> +
> + driver->bus = &mhi_ep_bus_type;
> + driver->owner = owner;
> + driver->probe = mhi_ep_driver_probe;
> + driver->remove = mhi_ep_driver_remove;
> +
> + return driver_register(driver);
> +}
> +EXPORT_SYMBOL_GPL(__mhi_ep_driver_register);
> +
> +void mhi_ep_driver_unregister(struct mhi_ep_driver *mhi_drv)
> +{
> + driver_unregister(&mhi_drv->driver);
> +}
> +EXPORT_SYMBOL_GPL(mhi_ep_driver_unregister);
> +
> static int mhi_ep_match(struct device *dev, struct device_driver *drv)
> {
> struct mhi_ep_device *mhi_dev = to_mhi_ep_device(dev);
> + struct mhi_ep_driver *mhi_drv = to_mhi_ep_driver(drv);
> + const struct mhi_device_id *id;
>
> /*
> * If the device is a controller type then there is no client driver
> @@ -207,6 +287,12 @@ static int mhi_ep_match(struct device *dev, struct device_driver *drv)
> if (mhi_dev->dev_type == MHI_DEVICE_CONTROLLER)
> return 0;
>
> + for (id = mhi_drv->id_table; id->chan[0]; id++)
> + if (!strcmp(mhi_dev->name, id->chan)) {
> + mhi_dev->id = id;
> + return 1;
> + }
> +
> return 0;
> };
>
> diff --git a/include/linux/mhi_ep.h b/include/linux/mhi_ep.h
> index 20238e9df1b3..da865f9d3646 100644
> --- a/include/linux/mhi_ep.h
> +++ b/include/linux/mhi_ep.h
> @@ -122,7 +122,60 @@ struct mhi_ep_device {
> enum mhi_device_type dev_type;
> };
>
> +/**
> + * struct mhi_ep_driver - Structure representing a MHI Endpoint client driver
> + * @id_table: Pointer to MHI Endpoint device ID table
> + * @driver: Device driver model driver
> + * @probe: CB function for client driver probe function
> + * @remove: CB function for client driver remove function
> + * @ul_xfer_cb: CB function for UL data transfer
> + * @dl_xfer_cb: CB function for DL data transfer
> + */
> +struct mhi_ep_driver {
> + const struct mhi_device_id *id_table;
> + struct device_driver driver;
> + int (*probe)(struct mhi_ep_device *mhi_ep,
> + const struct mhi_device_id *id);
> + void (*remove)(struct mhi_ep_device *mhi_ep);
> + void (*ul_xfer_cb)(struct mhi_ep_device *mhi_dev,
> + struct mhi_result *result);
> + void (*dl_xfer_cb)(struct mhi_ep_device *mhi_dev,
> + struct mhi_result *result);
> +};
> +
> #define to_mhi_ep_device(dev) container_of(dev, struct mhi_ep_device, dev)
> +#define to_mhi_ep_driver(drv) container_of(drv, struct mhi_ep_driver, driver)
> +
> +/*
> + * module_mhi_ep_driver() - Helper macro for drivers that don't do
> + * anything special other than using default mhi_ep_driver_register() and
> + * mhi_ep_driver_unregister(). This eliminates a lot of boilerplate.
> + * Each module may only use this macro once.
> + */
> +#define module_mhi_ep_driver(mhi_drv) \
> + module_driver(mhi_drv, mhi_ep_driver_register, \
> + mhi_ep_driver_unregister)
> +
> +/*
> + * Macro to avoid include chaining to get THIS_MODULE
> + */
> +#define mhi_ep_driver_register(mhi_drv) \
> + __mhi_ep_driver_register(mhi_drv, THIS_MODULE)
> +
> +/**
> + * __mhi_ep_driver_register - Register a driver with MHI Endpoint bus
> + * @mhi_drv: Driver to be associated with the device
> + * @owner: The module owner
> + *
> + * Return: 0 if driver registrations succeeds, a negative error code otherwise.
> + */
> +int __mhi_ep_driver_register(struct mhi_ep_driver *mhi_drv, struct module *owner);
> +
> +/**
> + * mhi_ep_driver_unregister - Unregister a driver from MHI Endpoint bus
> + * @mhi_drv: Driver associated with the device
> + */
> +void mhi_ep_driver_unregister(struct mhi_ep_driver *mhi_drv);
>
> /**
> * mhi_ep_register_controller - Register MHI Endpoint controller
> --
> 2.25.1
>
Powered by blists - more mailing lists