[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <181efcca6ae.de84203d522625.7740936811073442334@linux.beauty>
Date: Tue, 12 Jul 2022 08:26:15 +0800
From: Li Chen <me@...ux.beauty>
To: "Arnd Bergmann" <arnd@...db.de>
Cc: "Catalin Marinas" <catalin.marinas@....com>,
"Will Deacon" <will@...nel.org>,
"Rob Herring" <robh+dt@...nel.org>,
"Frank Rowand" <frowand.list@...il.com>,
"Andrew Morton" <akpm@...ux-foundation.org>,
"Li Chen" <lchen@...arella.com>,
"Linux ARM" <linux-arm-kernel@...ts.infradead.org>,
"Linux Kernel Mailing List" <linux-kernel@...r.kernel.org>,
"DTML" <devicetree@...r.kernel.org>,
"Linux-MM" <linux-mm@...ck.org>
Subject: Re: [PATCH 4/4] sample/reserved_mem: Introduce a sample of struct
page and dio support to no-map rmem
Hi Arnd,
---- On Mon, 11 Jul 2022 21:28:10 +0800 Arnd Bergmann <arnd@...db.de> wrote ---
> On Mon, Jul 11, 2022 at 2:24 PM Li Chen <me@...ux.beauty> wrote:
> >
> > From: Li Chen <lchen@...arella.com>
> >
> > This sample driver shows how to build struct pages support to no-map rmem.
> >
> > Signed-off-by: Li Chen <lchen@...arella.com>
>
> Not sure what a sample driver helps here if there are no actual users in-tree.
>
> It would make more sense to merge the driver that wants to actually use this
> first, and then add the additional feature.
Totally agree, but we plan to start rewriting our video driver in a long time, it has many legacy codes and I need to rewrite a lot of codes to migrate to v4l2.
That's why I also submit a sample driver here: to make the review progress easier and don't need reviewers to read video driver codes.
> > +/*
> > + * dts example
> > + * rmem: rmem@1 {
> > + * compatible = "shared-dma-pool";
> > + * no-map;
> > + * size = <0x0 0x20000000>;
> > + * };
> > + * perf {
> > + * compatible = "example,rmem";
> > + * memory-region = <&rmem>;
> > + * };
>
> The problem here is that the DT is meant to describe the platform in an OS
> independent way, so having a binding that just corresponds to a user space
> interface is not a good abstraction.
Gotcha, but IMO dts + rmem is the only choice for our use case. In our real case, we use reg instead of size to specify the physical address, so memremap cannot be used.
>
> > + vaddr = reserved_mem_memremap_pages(dev, rmem);
> > + if (IS_ERR_OR_NULL(vaddr))
> > + return PTR_ERR(vaddr);
>
> Using IS_ERR_OR_NULL() is usually an indication of a bad interface.
>
> For the reserved_mem_memremap_pages(), you should decide whether to return
> NULL on error or an error pointer, but not both.
Thanks, will fix in v2.
>
> Arnd
>
Powered by blists - more mailing lists