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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200217164751.GA7305@Mani-XPS-13-9360>
Date:   Mon, 17 Feb 2020 22:17:51 +0530
From:   Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     gregkh <gregkh@...uxfoundation.org>, smohanad@...eaurora.org,
        Jeffrey Hugo <jhugo@...eaurora.org>,
        Kalle Valo <kvalo@...eaurora.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        hemantk@...eaurora.org,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 11/16] bus: mhi: core: Add support for data transfer

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@...aro.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