Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram

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:

quoted

On Fri, 2020-04-17 at 10:31 +0800, 王文虎 wrote:

quoted

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.

-Scott