Re: [PATCH] mfd: Add Simple PCI MFD driver
From: Lizhi Hou
Date: Mon Jan 23 2023 - 21:31:11 EST
On 1/23/23 08:36, Rob Herring wrote:
On Mon, Jan 23, 2023 at 10:02 AM Vincent Whitchurch
<vincent.whitchurch@xxxxxxxx> 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@10000 {
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.
(I tried to reply with my comment this morning, but it did not post to
kernel mail alias. I am re-sending it. Please ignore if you already
received my reply)
I have proposed the address format for hardware apertures on PCI BAR.
The address consists of BAR index and offset. It uses the following
encoding:
0xIooooooo 0xoooooooo
Where:
I = BAR index
oooooo oooooooo = BAR offset
The endpoint is compatible with 'simple-bus' and contains 'ranges'
property for translating aperture address to CPU address.
Ref:
https://lore.kernel.org/lkml/20220305052304.726050-3-lizhi.hou@xxxxxxxxxx/
The 64-bit address can use the default translator defined of/address.c
I implemented an example of top of my latest patch
https://lore.kernel.org/lkml/1674183732-5157-1-git-send-email-lizhi.hou@xxxxxxx/
to verify the address translation. The test device tree nodes are defined
to present two hardware apertures on our alveo PCI device:
pr-isolation: PCI BAR 0, offset 0x41000, len 0x1000
xdma: PCI BAR 2, offset 0x0, len 0x40000
/ {
fragment@0 {
target-path="";
__overlay__ {
pci-ep-bus@0 {
compatible = "simple-bus";
#address-cells = <2>;
#size-cells = <2>;
ranges = <0x0 0x0 0x0 0x0 0x0 0x2000000>;
pr_isolate_ulp@41000 {
compatible = "xlnx,alveo-pr-isolation";
reg = <0x0 0x41000 0x0 0x1000>;
};
};
pci-ep-bus@2 {
compatible = "simple-bus";
#address-cells = <2>;
#size-cells = <2>;
ranges = <0x0 0x0 0x20000000 0x0 0x0 0x40000>;
alveo-xdma@0 {
compatible = "xlnx,alveo-xdma";
reg = <0x0 0x0 0x0 0x40000>;
};
};
};
};
};
Overall, the test is as below
1) added code to generate 'ranges' for PCI endpoint dt node
00000000 00000000 C30B0000 00000080 10000000 00000000 02000000 (BAR 0)
20000000 00000000 C30B0000 00000080 14000000 00000000 00040000 (BAR 2)
^ bar index ^^^^^^^^^^^^^BAR IOMEM address
code link:
https://github.com/houlz0507/linux-xoclv2/compare/29031e597fd6272f825dd04ba41a38defb77a99a...pci-dt-v6?diff=unified#diff-bf1b86155c18e04c439b74f5a02bad99c91a8c04f3c21243afce996c2174be56
2) overlay the test device tree fragment under PCI endpoint.
3) Alveo pci device driver probe function calls
of_platform_default_populate().
I can see the BAR index+offset is translated to IO address correctly.
# ls /sys/bus/platform/devices/
0.flash
3f000000.pcie
3f000000.pcie:pci@2,0:pci@0,0:pci@0,0:dev@0,0:pci-ep-bus@0
3f000000.pcie:pci@2,0:pci@0,0:pci@0,0:dev@0,0:pci-ep-bus@2
8010041000.pr_isolate_ulp
8014000000.alveo-xdma
# lspci -vd 10ee:5020 | grep Memory
Memory at 8010000000 (64-bit, prefetchable) [disabled] [size=32M]
Memory at 8014000000 (64-bit, prefetchable) [disabled] [size=256K]
This test needs our Alveo PCI device. It might be a reference to create
unittest.
Thanks,
Lizhi
Rob