Re: [PATCH v3 03/16] bus: mhi: core: Add support for registering MHI client drivers

[Greg KH]

On Thu, Feb 20, 2020 at 03:28:41PM +0530, Manivannan Sadhasivam wrote:
> This commit adds support for registering MHI client drivers with the
> MHI stack. MHI client drivers binds to one or more MHI devices inorder
> to sends and receive the upper-layer protocol packets like IP packets,
> modem control messages, and diagnostics messages over MHI bus.
> 
> This is based on the patch submitted by Sujeev Dias:
> https://lkml.org/lkml/2018/7/9/987
> 
> Signed-off-by: Sujeev Dias <sdias@xxxxxxxxxxxxxx>
> Signed-off-by: Siddartha Mohanadoss <smohanad@xxxxxxxxxxxxxx>
> [mani: splitted and cleaned up for upstream]
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> Reviewed-by: Jeffrey Hugo <jhugo@xxxxxxxxxxxxxx>
> Tested-by: Jeffrey Hugo <jhugo@xxxxxxxxxxxxxx>
> ---
>  drivers/bus/mhi/core/init.c | 149 ++++++++++++++++++++++++++++++++++++
>  include/linux/mhi.h         |  39 ++++++++++
>  2 files changed, 188 insertions(+)
> 
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index 6f24c21284ec..12e386862b3f 100644
> --- a/drivers/bus/mhi/core/init.c
> +++ b/drivers/bus/mhi/core/init.c
> @@ -374,8 +374,157 @@ struct mhi_device *mhi_alloc_device(struct mhi_controller *mhi_cntrl)
>  	return mhi_dev;
>  }
>  
> +static int mhi_driver_probe(struct device *dev)
> +{
> +	struct mhi_device *mhi_dev = to_mhi_device(dev);
> +	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
> +	struct device_driver *drv = dev->driver;
> +	struct mhi_driver *mhi_drv = to_mhi_driver(drv);
> +	struct mhi_event *mhi_event;
> +	struct mhi_chan *ul_chan = mhi_dev->ul_chan;
> +	struct mhi_chan *dl_chan = mhi_dev->dl_chan;
> +
> +	if (ul_chan) {
> +		/*
> +		 * If channel supports LPM notifications then status_cb should
> +		 * be provided
> +		 */
> +		if (ul_chan->lpm_notify && !mhi_drv->status_cb)
> +			return -EINVAL;
> +
> +		/* For non-offload channels then xfer_cb should be provided */
> +		if (!ul_chan->offload_ch && !mhi_drv->ul_xfer_cb)
> +			return -EINVAL;
> +
> +		ul_chan->xfer_cb = mhi_drv->ul_xfer_cb;
> +	}
> +
> +	if (dl_chan) {
> +		/*
> +		 * If channel supports LPM notifications then status_cb should
> +		 * be provided
> +		 */
> +		if (dl_chan->lpm_notify && !mhi_drv->status_cb)
> +			return -EINVAL;
> +
> +		/* For non-offload channels then xfer_cb should be provided */
> +		if (!dl_chan->offload_ch && !mhi_drv->dl_xfer_cb)
> +			return -EINVAL;
> +
> +		mhi_event = &mhi_cntrl->mhi_event[dl_chan->er_index];
> +
> +		/*
> +		 * If the channel event ring is managed by client, then
> +		 * status_cb must be provided so that the framework can
> +		 * notify pending data
> +		 */
> +		if (mhi_event->cl_manage && !mhi_drv->status_cb)
> +			return -EINVAL;
> +
> +		dl_chan->xfer_cb = mhi_drv->dl_xfer_cb;
> +	}
> +
> +	/* Call the user provided probe function */
> +	return mhi_drv->probe(mhi_dev, mhi_dev->id);
> +}
> +
> +static int mhi_driver_remove(struct device *dev)
> +{
> +	struct mhi_device *mhi_dev = to_mhi_device(dev);
> +	struct mhi_driver *mhi_drv = to_mhi_driver(dev->driver);
> +	struct mhi_chan *mhi_chan;
> +	enum mhi_ch_state ch_state[] = {
> +		MHI_CH_STATE_DISABLED,
> +		MHI_CH_STATE_DISABLED
> +	};
> +	int dir;
> +
> +	/* Skip if it is a controller device */
> +	if (mhi_dev->dev_type == MHI_DEVICE_CONTROLLER)
> +		return 0;
> +
> +	/* Reset both channels */
> +	for (dir = 0; dir < 2; dir++) {
> +		mhi_chan = dir ? mhi_dev->ul_chan : mhi_dev->dl_chan;
> +
> +		if (!mhi_chan)
> +			continue;
> +
> +		/* Wake all threads waiting for completion */
> +		write_lock_irq(&mhi_chan->lock);
> +		mhi_chan->ccs = MHI_EV_CC_INVALID;
> +		complete_all(&mhi_chan->completion);
> +		write_unlock_irq(&mhi_chan->lock);
> +
> +		/* Set the channel state to disabled */
> +		mutex_lock(&mhi_chan->mutex);
> +		write_lock_irq(&mhi_chan->lock);
> +		ch_state[dir] = mhi_chan->ch_state;
> +		mhi_chan->ch_state = MHI_CH_STATE_SUSPENDED;
> +		write_unlock_irq(&mhi_chan->lock);
> +
> +		mutex_unlock(&mhi_chan->mutex);
> +	}
> +
> +	mhi_drv->remove(mhi_dev);
> +
> +	/* De-init channel if it was enabled */
> +	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->mutex);
> +
> +		mhi_chan->ch_state = MHI_CH_STATE_DISABLED;
> +
> +		mutex_unlock(&mhi_chan->mutex);
> +	}
> +
> +	return 0;
> +}
> +
> +int mhi_driver_register(struct mhi_driver *mhi_drv)
> +{
> +	struct device_driver *driver = &mhi_drv->driver;
> +
> +	if (!mhi_drv->probe || !mhi_drv->remove)
> +		return -EINVAL;
> +
> +	driver->bus = &mhi_bus_type;
> +	driver->probe = mhi_driver_probe;
> +	driver->remove = mhi_driver_remove;
> +
> +	return driver_register(driver);
> +}
> +EXPORT_SYMBOL_GPL(mhi_driver_register);

You don't care about module owners of the driver?  Odd :(

(hint, you probably should...)

greg k-h