Hi Loic,
On 18 September 2020 3:46:59 PM IST, Loic Poulain <loic.poulain@xxxxxxxxxx> wrote:
>There is really no point having an auto-start for channels.
>This is confusing for the device drivers, some have to enable the
>channels, others don't have... and waste resources (e.g. pre allocated
>buffers) that may never be used.
>
>This is really up to the MHI device(channel) driver to manage the state
>of its channels.
>
>Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxx>
The patch content looks good but you should split the qrtr change to a separate patch. I can't queue the networking change through mhi tree.
Thanks,
Mani
>---
> drivers/bus/mhi/core/init.c | 9 ---------
> drivers/bus/mhi/core/internal.h | 1 -
> include/linux/mhi.h | 2 --
> net/qrtr/mhi.c | 5 +++++
> 4 files changed, 5 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
>index ac19067..7aef6b7 100644
>--- a/drivers/bus/mhi/core/init.c
>+++ b/drivers/bus/mhi/core/init.c
>@@ -727,7 +727,6 @@ static int parse_ch_cfg(struct mhi_controller
>*mhi_cntrl,
> mhi_chan->offload_ch = ch_cfg->offload_channel;
> mhi_chan->db_cfg.reset_req = ch_cfg->doorbell_mode_switch;
> mhi_chan->pre_alloc = ch_cfg->auto_queue;
>- mhi_chan->auto_start = ch_cfg->auto_start;
>
> /*
> * If MHI host allocates buffers, then the channel direction
>@@ -1125,11 +1124,6 @@ static int mhi_driver_probe(struct device *dev)
> goto exit_probe;
>
> ul_chan->xfer_cb = mhi_drv->ul_xfer_cb;
>- if (ul_chan->auto_start) {
>- ret = mhi_prepare_channel(mhi_cntrl, ul_chan);
>- if (ret)
>- goto exit_probe;
>- }
> }
>
> ret = -EINVAL;
>@@ -1163,9 +1157,6 @@ static int mhi_driver_probe(struct device *dev)
> if (ret)
> goto exit_probe;
>
>- if (dl_chan && dl_chan->auto_start)
>- mhi_prepare_channel(mhi_cntrl, dl_chan);
>-
> mhi_device_put(mhi_dev);
>
> return ret;
>diff --git a/drivers/bus/mhi/core/internal.h
>b/drivers/bus/mhi/core/internal.h
>index 5a81a42..73b52a0 100644
>--- a/drivers/bus/mhi/core/internal.h
>+++ b/drivers/bus/mhi/core/internal.h
>@@ -563,7 +563,6 @@ struct mhi_chan {
> bool configured;
> bool offload_ch;
> bool pre_alloc;
>- bool auto_start;
> bool wake_capable;
> };
>
>diff --git a/include/linux/mhi.h b/include/linux/mhi.h
>index 008b8f6..742dabe 100644
>--- a/include/linux/mhi.h
>+++ b/include/linux/mhi.h
>@@ -214,7 +214,6 @@ enum mhi_db_brst_mode {
> * @offload_channel: The client manages the channel completely
>* @doorbell_mode_switch: Channel switches to doorbell mode on M0
>transition
>* @auto_queue: Framework will automatically queue buffers for DL
>traffic
>- * @auto_start: Automatically start (open) this channel
> * @wake-capable: Channel capable of waking up the system
> */
> struct mhi_channel_config {
>@@ -232,7 +231,6 @@ struct mhi_channel_config {
> bool offload_channel;
> bool doorbell_mode_switch;
> bool auto_queue;
>- bool auto_start;
> bool wake_capable;
> };
>
>diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
>index ff0c414..7100f0b 100644
>--- a/net/qrtr/mhi.c
>+++ b/net/qrtr/mhi.c
>@@ -76,6 +76,11 @@ static int qcom_mhi_qrtr_probe(struct mhi_device
>*mhi_dev,
> struct qrtr_mhi_dev *qdev;
> int rc;
>
>+ /* start channels */
>+ rc = mhi_prepare_for_transfer(mhi_dev);
>+ if (rc)
>+ return rc;
>+
> qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
> if (!qdev)
> return -ENOMEM;
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
