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;
Date:   Mon, 17 Feb 2020 22:17:51 +0530
From:   Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To:     Arnd Bergmann <arnd@arndb.de>
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>
Subject: Re: [PATCH v2 11/16] bus: mhi: core: Add support for data transfer
Message-ID: <20200217164751.GA7305@Mani-XPS-13-9360>
References: <20200131135009.31477-1-manivannan.sadhasivam@linaro.org>
 <20200131135009.31477-12-manivannan.sadhasivam@linaro.org>
 <CAK8P3a1DOta5Uj3dNFWVqwgyPKs0cQsoymXE7svcOZoPiY+YGw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAK8P3a1DOta5Uj3dNFWVqwgyPKs0cQsoymXE7svcOZoPiY+YGw@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Arnd,

On Mon, Feb 17, 2020 at 05:13:37PM +0100, Arnd Bergmann wrote:
> 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?
> 

It sounds reasonable to me. Let me discuss this internally with Qcom guys to
see if they have any objections.

Thanks,
Mani

>       Arnd