linux-kernel - Re: [PATCH] mfd: Add Simple PCI MFD driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL_JsqLHi_1QJ4s28uM1b_Z2f+Nsu_CusAsqsx6vVOCCAPLMgw@mail.gmail.com>
Date:   Mon, 23 Jan 2023 10:36:06 -0600
From:   Rob Herring <robh@...nel.org>
To:     Vincent Whitchurch <vincent.whitchurch@...s.com>,
        Lizhi Hou <lizhi.hou@....com>
Cc:     Lee Jones <lee@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        kernel <kernel@...s.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH] mfd: Add Simple PCI MFD driver

On Mon, Jan 23, 2023 at 10:02 AM Vincent Whitchurch
<vincent.whitchurch@...s.com> wrote:
>
> On Mon, Jan 23, 2023 at 04:32:55PM +0100, Lee Jones wrote:
> > On Mon, 23 Jan 2023, Vincent Whitchurch wrote:
> > > Add a PCI driver which registers all child nodes specified in the
> > > devicetree.  It will allow platform devices to be used on virtual
> > > systems which already support PCI and devicetree, such as UML with
> > > virt-pci.
> > >
> > > The driver has no id_table by default; user space needs to provide one
> > > using the new_id mechanism in sysfs.
> >
> > This feels wrong for several reasons.
> >
> > Firstly, I think Greg (Cc:ed) will have something to say about this.
> >
> > Secondly, this driver does literally nothing.
>
> Well, it does do what the commit message says.  If there's another way
> of accomplishing that, I'm all ears.
>
> > Why can't you use of of the other, pre-existing "also register my
> > children" compatibles?
> >
> > See: drivers/bus/simple-pm-bus.c
> >      drivers/of/platform.c
>
> simple-pm-bus registers a platform driver, and drivers/of/platform.c
> works on the platform bus.  The driver added by this patch is a PCI
> driver.  So I don't understand how the files you mention could be used
> here?
>
> In case it helps, the relevant nodes in my UML devicetree look something
> like this:
>
>     virtio@2 {

dtc should complain about this...

>         compatible = "virtio,uml";

Binding?

>         virtio-device-id = <1234>;
>         ranges;
>
>         pci {
>                 #address-cells = <3>;
>                 #size-cells = <2>;
>                 ranges = <0x0000000 0 0 0 0xf0000000 0 0x20000>;
>                 compatible = "virtio,device4d2", "pci";

"pci" is not a valid compatible string.

>                 device_type = "pci";
>                 bus-range = <0 0>;
>
>                 platform_parent: device@0,0 {
>                         compatible = "pci494f,dc8";
>                         reg = <0x00000 0 0 0x0 0x10000>;
>                         ranges;
>
>                         uart@...00 {
>                                 compatible = "google,goldfish-tty";
>                                 reg = <0x00000 0 0x10000 0 0x10000>;

This is not a PCI device, so it shouldn't be using PCI addressing.
'ranges' needs an entry (for each BAR) to translate to just a normal
MMIO bus with 1 or 2 address/size cells. Maybe we want a 'simple-bus'
node for each BAR. The FPGA series needs the same things, but that
aspect hasn't really been addressed as the first issue is populating
the PCI devices dynamically.

The DT address translation code should support all this
(MMIO->PCI->MMIO), but I don't think there's any existing examples. An
example (that I can test) would be great. If the unittest had that
example, I'd be thrilled.

Rob