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