Re: [PATCH v2 11/16] bus: mhi: core: Add support for data transfer

[Manivannan Sadhasivam]

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@xxxxxxxxxx> 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