Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200131135009.31477-1-manivannan.sadhasivam@linaro.org> <20200131135009.31477-12-manivannan.sadhasivam@linaro.org>
In-Reply-To: <20200131135009.31477-12-manivannan.sadhasivam@linaro.org>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Mon, 17 Feb 2020 17:13:37 +0100
Message-ID: <CAK8P3a1DOta5Uj3dNFWVqwgyPKs0cQsoymXE7svcOZoPiY+YGw@mail.gmail.com>
Subject: Re: [PATCH v2 11/16] bus: mhi: core: Add support for data transfer
To:     Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc:     gregkh <gregkh@linuxfoundation.org>, smohanad@codeaurora.org,
        Jeffrey Hugo <jhugo@codeaurora.org>,
        Kalle Valo <kvalo@codeaurora.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        hemantk@codeaurora.org,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jan 31, 2020 at 2:51 PM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:

> @@ -648,6 +715,31 @@ static int parse_ch_cfg(struct mhi_controller *mhi_cntrl,
>                 mhi_chan->db_cfg.pollcfg = ch_cfg->pollcfg;
>                 mhi_chan->xfer_type = ch_cfg->data_type;
>
> +               switch (mhi_chan->xfer_type) {
> +               case MHI_BUF_RAW:
> +                       mhi_chan->gen_tre = mhi_gen_tre;
> +                       mhi_chan->queue_xfer = mhi_queue_buf;
> +                       break;
> +               case MHI_BUF_SKB:
> +                       mhi_chan->queue_xfer = mhi_queue_skb;
> +                       break;
> +               case MHI_BUF_SCLIST:
> +                       mhi_chan->gen_tre = mhi_gen_tre;
> +                       mhi_chan->queue_xfer = mhi_queue_sclist;
> +                       break;
> +               case MHI_BUF_NOP:
> +                       mhi_chan->queue_xfer = mhi_queue_nop;
> +                       break;
> +               case MHI_BUF_DMA:
> +               case MHI_BUF_RSC_DMA:
> +                       mhi_chan->queue_xfer = mhi_queue_dma;
> +                       break;
> +               default:
> +                       dev_err(mhi_cntrl->dev,
> +                               "Channel datatype not supported\n");
> +                       goto error_chan_cfg;
> +               }
> +

While looking through the driver to see how the DMA gets handled, I came
across the multitude of mhi_queue_* functions, which seems like a
layering violation to me, given that they are all implemented by the
core code as well, and the client driver needs to be aware of
which one to call. Are you able to lift these out of the common interface
and make the client driver call these directly, or maybe provide a direct
interface based on mhi_buf_info to replace these?

      Arnd