Hi Arnd,
---- On Mon, 11 Jul 2022 21:28:10 +0800 Arnd Bergmann <arnd@xxxxxxxx> wrote ---
> On Mon, Jul 11, 2022 at 2:24 PM Li Chen <me@linux.beauty> wrote:
> >
> > From: Li Chen <lchen@xxxxxxxxxxxxx>
> >
> > This sample driver shows how to build struct pages support to no-map rmem.
> >
> > Signed-off-by: Li Chen <lchen@xxxxxxxxxxxxx>
>
> 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
>
