[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <eb73fb40c2d102f24cf4d9a4ba26ffeea278b715.camel@buserror.net>
Date: Fri, 17 Apr 2020 17:50:04 -0500
From: Scott Wood <oss@...error.net>
To: 王文虎 <wenhu.wang@...o.com>
Cc: Greg KH <gregkh@...uxfoundation.org>,
Rob Herring <robh@...nel.org>, linux-kernel@...r.kernel.org,
christophe.leroy@....fr, linuxppc-dev@...ts.ozlabs.org,
kernel@...o.com
Subject: Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
On Fri, 2020-04-17 at 22:16 +0800, 王文虎 wrote:
> > On Fri, 2020-04-17 at 09:42 +0200, Greg KH wrote:>> On Thu, Apr 16, 2020
> > at 11:58:29PM -0500, Scott Wood wrote:
> > > > On Fri, 2020-04-17 at 10:31 +0800, 王文虎 wrote:
> > > > > Sounds it is. And does the modification below fit well?
> > > > > ---
> > > > > -static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> > > > > - { .compatible = "uio,mpc85xx-cache-sram", },
> > > > > - {},
> > > > > +#ifdef CONFIG_OF
> > > > > +static struct of_device_id uio_fsl_85xx_cache_sram_of_match[] = {
> > > > > + { /* This is filled with module_parm */ },
> > > > > + { /* Sentinel */ },
> > > > > };
> > > > > +MODULE_DEVICE_TABLE(of, uio_fsl_85xx_cache_sram_of_match);
> > > > > +module_param_string(of_id,
> > > > > uio_fsl_85xx_cache_sram_of_match[0].compatible,
> > > > > + sizeof(uio_fsl_85xx_cache_sram_of_match[
> > > > > 0].c
> > > > > ompa
> > > > > tible), 0);
> > > > > +MODULE_PARM_DESC(of_id, "platform device id to be handled by cache-
> > > > > sram-
> > > > > uio");
> > > > > +#endif
> > > >
> > > > No. The point is that you wouldn't be configuring this with the
> > > > device
> > > > tree
> > > > at all.
> > >
> > > Wait, why not? Don't force people to use module parameters, that is
> > > crazy. DT describes the hardware involved, if someone wants to bind to
> > > a specific range of memory, as described by DT, why can't they do so?
> >
> > Yes, DT describes the hardware, and as I've said a couple times already,
> > this
> > isn't hardware description.
> >
> > I'm not forcing people to use module parameters. That was a least-effort
> > suggestion to avoid abusing the DT. I later said I'd try to come up with
> > a
> > patch that allocates regions dynamically (and most likely doesn't use UIO
> > at
> > all).
> >
> > > I can understand not liking the name "uio" in a dt tree, but there's no
> > > reason that DT can not describe what a driver binds to here.
> >
> > The DT already describes this hardware, and there is already code that
> > binds
> > to it. This patch is trying to add a second node for it with
> > configuration.
> >
>
> Hi, Scott, Greg,
> Seems like no balance here. How about I implement a driver of uio including
> the l2ctrl and cache_sram related implementations?
> And this way, the driver would be a hardware level driver and targeted for
> uio.
No, duplicating the code makes no sense whatsoever. Please just wait a bit
and I'll send a patch to have the existing driver expose a dynamic allocation
interface to userspace.
-Scott
Powered by blists - more mailing lists