linux-kernel - RE: [PATCH v7 1/2] dt-bindings: dma: Add bindings for intel LGM SOC

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: <DM6PR19MB397705C898DE2FBA755B2D80BBE90@DM6PR19MB3977.namprd19.prod.outlook.com>
Date:   Tue, 10 Nov 2020 17:39:15 +0000
From:   Thomas Langer <tlanger@...linear.com>
To:     "Reddy, MallikarjunaX" <mallikarjunax.reddy@...ux.intel.com>,
        "dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
        "vkoul@...nel.org" <vkoul@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Shevchenko, Andriy" <andriy.shevchenko@...el.com>,
        "chuanhua.lei@...ux.intel.com" <chuanhua.lei@...ux.intel.com>,
        "Kim, Cheol Yong" <Cheol.Yong.Kim@...el.com>,
        "Wu, Qiming" <qi-ming.wu@...el.com>,
        "malliamireddy009@...il.com" <malliamireddy009@...il.com>,
        "peter.ujfalusi@...com" <peter.ujfalusi@...com>,
        "Langer, Thomas" <thomas.langer@...el.com>
Subject: RE: [PATCH v7 1/2] dt-bindings: dma: Add bindings for intel LGM SOC



> -----Original Message-----
> From: Reddy, MallikarjunaX <mallikarjunax.reddy@...ux.intel.com>
> Sent: Montag, 2. November 2020 15:42
> To: Thomas Langer <tlanger@...linear.com>; dmaengine@...r.kernel.org;
> vkoul@...nel.org; devicetree@...r.kernel.org; robh+dt@...nel.org
> Cc: linux-kernel@...r.kernel.org; Shevchenko, Andriy
> <andriy.shevchenko@...el.com>; chuanhua.lei@...ux.intel.com; Kim,
> Cheol Yong <Cheol.Yong.Kim@...el.com>; Wu, Qiming <qi-
> ming.wu@...el.com>; malliamireddy009@...il.com; peter.ujfalusi@...com;
> Langer, Thomas <thomas.langer@...el.com>
> Subject: Re: [PATCH v7 1/2] dt-bindings: dma: Add bindings for intel
> LGM SOC
> 
> This email was sent from outside of MaxLinear.
> 
> 
> Hi Thomas,
> Thanks for the review, my comments inline.
> 
> On 10/28/2020 3:24 AM, Thomas Langer wrote:
> > Hello Reddy,
> >
> > I think "Intel" should always be written with a capital "I" (like in
> the Subject, but except in the binding below)
> OK.
> >
> >> + compatible:
> >> +  oneOf:
> >> +   - const: intel,lgm-cdma
> >> +   - const: intel,lgm-dma2tx
> >> +   - const: intel,lgm-dma1rx
> >> +   - const: intel,lgm-dma1tx
> >> +   - const: intel,lgm-dma0tx
> >> +   - const: intel,lgm-dma3
> >> +   - const: intel,lgm-toe-dma30
> >> +   - const: intel,lgm-toe-dma31
> > Bindings are normally not per instance.
> > What if next generation chip gets more DMA modules but has no other
> changes in the HW block?
> > What is wrong with
> >    - const: intel,lgm-cdma
> >    - const: intel,lgm-hdma
> > and extra attributes to define the rx/tx restriction (or what do it
> mean?)?
> >  From the driver code I saw that "toe" is also just of type "hdma"
> and no further differences in code are done.
> We had a discussion on the same in the previous patches and Rob
> Herring
> said Okay using Different compatibles.
> below the snippet.
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +
>  >>> + compatible:
>  >>> +  anyOf:
>  >>> +   - const: intel,lgm-cdma
>  >>> +   - const: intel,lgm-dma2tx
>  >>> +   - const: intel,lgm-dma1rx
>  >>> +   - const: intel,lgm-dma1tx
>  >>> +   - const: intel,lgm-dma0tx
>  >>> +   - const: intel,lgm-dma3
>  >>> +   - const: intel,lgm-toe-dma30
>  >>> +   - const: intel,lgm-toe-dma31
>  >> Please explain why you need so many different compatible strings.
>  > This hw dma has 7 DMA instances.
>  > Some for datapath, some for memcpy  and some for TOE.
>  > Some for TX only, some for RX only, and some for TX/RX(memcpy and
> ToE).
>  >
>  > dma TX/RX type we considered as driver specific data of each
> instance and
>  > used different compatible strings for each instance.
>  > And also idea is in future if any driver specific data of any
> particular
>  > instance we can handle.
>  >
>  > Here if dma name and type(tx or rx) will be accepted as devicetree
>  > attributes then we can move .name = "toe_dma31", & .type =
> DMA_TYPE_MCPY
>  > to devicetree. So that the compatible strings can be limited to
> two.
>  > intel,lgm-cdma & intel,lgm-hdma .
> 
> [Rob]
> Different compatibles are okay if the instances are different and we
> don't have properties to describe the differences.

Okay, but then explain what the differences are, that cannot be described
by other properties/attributes. In the driver code I cannot see anything,
except the "name". But for printouts in driver, "drv_dbg" or similar will
just use the node path for the instance.

> 
> For some of what you have in this binding, I think it should be part
> of
> the consumer cells.
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++
> >
> > Best regards,
> > Thomas
> >