Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <ede379a75ec60ccba90cd393a3a2e70b410191fa.camel@buserror.net>
From:   Scott Wood <oss@buserror.net>
To:     Greg KH <gregkh@linuxfoundation.org>
Cc:     =?UTF-8?Q?=E7=8E=8B=E6=96=87=E8=99=8E?= <wenhu.wang@vivo.com>,
        Rob Herring <robh@kernel.org>, linux-kernel@vger.kernel.org,
        christophe.leroy@c-s.fr, linuxppc-dev@lists.ozlabs.org,
        kernel@vivo.com
Date:   Fri, 17 Apr 2020 04:17:14 -0500
In-Reply-To: <20200417074228.GA22586@kroah.com>
References: <ANcAoADRCKKtO5p9r33Ll4og.3.1587090694317.Hmail.wenhu.wang@vivo.com>
         <64bb1f056abd8bfab2befef5d1e6baec2056077f.camel@buserror.net>
         <20200417074228.GA22586@kroah.com>
Organization: Red Hat
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.

-Scott