| Message ID | 20190925145750.200592-27-sjg@chromium.org |
|---|---|
| State | Accepted |
| Delegated to: | Bin Meng |
| Headers | show |
| Series | x86: Add initial support for apollolake | expand |
On Wed, Sep 25, 2019 at 10:58 PM Simon Glass <sjg@chromium.org> wrote: > > Sandbox i2c works using emulation drivers which are currently children of pci > the i2c device: pci > > pci-controller { > pci@1f,0 { > compatible = "pci-generic"; > reg = <0xf800 0 0 0 0>; > emul@1f,0 { > compatible = "sandbox,swap-case"; > }; > }; > }; > > In this case the emulation device is attached to pci device on address > f800 (device 1f, function 0) and provides the swap-case functionality. > > However this is not ideal, since every device on a PCI bus has a child > device. This is only really the case for sandbox, but we want to avoid > special-case code for sandbox. > > Worse, child devices cannot be probed before their parents. This forces > us to use 'find' rather than 'get' to obtain the emulator device. In fact > the emulator devices are never probed. There is code in > sandbox_pci_emul_post_probe() which tries to track when emulators are > active, but at present this does not work. > > A better approach seems to be to add a separate node elsewhere in the > device tree, an 'emulation parent'. This could be given a bogus address > (such as -1) to hide the emulators away from the 'pci' command, but it > seems better to keep it at the root node to avoid such hacks. > > Then we can use a phandle to point from the evice to the correct typo: device > emulator, and only on sandbox. The code to find an emulator does not > interfere with normal pci operation. > > Add a new UCLASS_PCI_EMUL_PARENT uclass which allows finding an emulator > given a bus, and finding a bus given an emulator. Update the existing > device trees and the code for finding an emulator. > > This brings PCI emulators more into line with I2C. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > arch/sandbox/dts/sandbox.dtsi | 11 +++++++--- > arch/sandbox/dts/test.dts | 38 +++++++++++++++++++++++------------ > doc/driver-model/pci-info.rst | 23 +++++++++++---------- > drivers/pci/pci-emul-uclass.c | 37 ++++++++++++++++++++++++++++------ > include/dm/uclass-id.h | 1 + > 5 files changed, 77 insertions(+), 33 deletions(-) > > diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi > index c6d5650c20b..f09bc70b0da 100644 > --- a/arch/sandbox/dts/sandbox.dtsi > +++ b/arch/sandbox/dts/sandbox.dtsi > @@ -103,9 +103,14 @@ > pci@1f,0 { > compatible = "pci-generic"; > reg = <0xf800 0 0 0 0>; > - emul@1f,0 { > - compatible = "sandbox,swap-case"; > - }; > + sandbox,emul = <&swap_case_emul>; > + }; > + }; > + > + emul { > + compatible = "sandbox,pci-emul-parent"; > + swap_case_emul: emul@1f,0 { > + compatible = "sandbox,swap-case"; > }; > }; > > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts > index 5669ede7051..a2e75981f0b 100644 > --- a/arch/sandbox/dts/test.dts > +++ b/arch/sandbox/dts/test.dts > @@ -452,24 +452,31 @@ > pci@0,0 { > compatible = "pci-generic"; > reg = <0x0000 0 0 0 0>; > - emul@0,0 { > - compatible = "sandbox,swap-case"; > - }; > + sandbox,emul = <&swap_case_emul0>; &swap_case_emul0_0 > }; > pci@1,0 { > compatible = "pci-generic"; > reg = <0x0800 0 0 0 0>; > - emul@0,0 { > - compatible = "sandbox,swap-case"; > - use-ea; > - }; > + sandbox,emul = <&swap_case_emul1>; &swap_case_emul0_1 > }; > pci@1f,0 { > compatible = "pci-generic"; > reg = <0xf800 0 0 0 0>; > - emul@1f,0 { > - compatible = "sandbox,swap-case"; > - }; > + sandbox,emul = <&swap_case_emul1f>; &swap_case_emul0_1f > + }; > + }; > + > + pci-emul0 { > + compatible = "sandbox,pci-emul-parent"; > + swap_case_emul0: emul@0,0 { > + compatible = "sandbox,swap-case"; > + }; > + swap_case_emul1: emul@1,0 { > + compatible = "sandbox,swap-case"; > + use-ea; > + }; > + swap_case_emul1f: emul@1f,0 { > + compatible = "sandbox,swap-case"; > }; > }; > > @@ -499,9 +506,14 @@ > pci@1f,0 { > compatible = "pci-generic"; > reg = <0xf800 0 0 0 0>; > - emul@1f,0 { > - compatible = "sandbox,swap-case"; > - }; > + sandbox,emul = <&swap_case_emul2_1f>; > + }; > + }; > + > + pci-emul2 { > + compatible = "sandbox,pci-emul-parent"; > + swap_case_emul2_1f: emul2@1f,0 { > + compatible = "sandbox,swap-case"; > }; > }; > > diff --git a/doc/driver-model/pci-info.rst b/doc/driver-model/pci-info.rst > index d93ab8b61d5..f39ff990a67 100644 > --- a/doc/driver-model/pci-info.rst > +++ b/doc/driver-model/pci-info.rst > @@ -113,14 +113,17 @@ Sandbox > ------- > > With sandbox we need a device emulator for each device on the bus since there > -is no real PCI bus. This works by looking in the device tree node for a > -driver. For example:: > - > +is no real PCI bus. This works by looking in the device tree node for an > +emulator driver. For example:: > > pci@1f,0 { > compatible = "pci-generic"; > reg = <0xf800 0 0 0 0>; > - emul@1f,0 { > + sandbox,emul = <&emul_1f>; > + }; > + pci-emul { > + compatible = "sandbox,pci-emul-parent"; > + emul_1f: emul@1f,0 { > compatible = "sandbox,swap-case"; > }; > }; > @@ -130,14 +133,16 @@ Note that the first cell in the 'reg' value is the bus/device/function. See > PCI_BDF() for the encoding (it is also specified in the IEEE Std 1275-1994 > PCI bus binding document, v2.1) > > +The pci-emul node should go outside the pci bus node, since otherwise it will > +be scanned as a PCI device, causing confusion. > + > When this bus is scanned we will end up with something like this:: > > `- * pci-controller @ 05c660c8, 0 > `- pci@1f,0 @ 05c661c8, 63488 > - `- emul@1f,0 @ 05c662c8 > + `- emul@1f,0 @ 05c662c8 > > -When accesses go to the pci@1f,0 device they are forwarded to its child, the > -emulator. > +When accesses go to the pci@1f,0 device they are forwarded to its emulator. > > The sandbox PCI drivers also support dynamic driver binding, allowing device > driver to declare the driver binding information via U_BOOT_PCI_DEVICE(), > @@ -164,7 +169,3 @@ When this bus is scanned we will end up with something like this:: > pci [ + ] pci_sandbo |-- pci-controller1 > pci_emul [ ] sandbox_sw | |-- sandbox_swap_case_emul > pci_emul [ ] sandbox_sw | `-- sandbox_swap_case_emul > - > -Note the difference from the statically declared device nodes is that the > -device is directly attached to the host controller, instead of via a container > -device like pci@1f,0. > diff --git a/drivers/pci/pci-emul-uclass.c b/drivers/pci/pci-emul-uclass.c > index 38227583547..f918e9a52fd 100644 > --- a/drivers/pci/pci-emul-uclass.c > +++ b/drivers/pci/pci-emul-uclass.c > @@ -10,6 +10,7 @@ > #include <linux/libfdt.h> > #include <pci.h> > #include <dm/lists.h> > +#include <dm/uclass-internal.h> > > struct sandbox_pci_emul_priv { > int dev_count; > @@ -30,13 +31,15 @@ int sandbox_pci_get_emul(struct udevice *bus, pci_dev_t find_devfn, > } > *containerp = dev; > > - if (device_get_uclass_id(dev) == UCLASS_PCI_GENERIC) { > - ret = device_find_first_child(dev, emulp); > - if (ret) > - return ret; > - } else { > + /* > + * TODO(sjg@chromium.org): This code needs a comment as I'm not sure > + * why UCLASS_PCI_GENERIC devices end up being their own emulators. I > + * left this code as is. > + */ This code comes from: commit 4345998ae9dfad7ba0beb54ad4322134557504a9 Author: Bin Meng <bmeng.cn@gmail.com> Date: Fri Aug 3 01:14:45 2018 -0700 pci: sandbox: Support dynamically binding device driver I think the comments can be removed :) > + ret = uclass_find_device_by_phandle(UCLASS_PCI_EMUL, dev, > + "sandbox,emul", emulp); > + if (ret && device_get_uclass_id(dev) != UCLASS_PCI_GENERIC) > *emulp = dev; > - } > > return *emulp ? 0 : -ENODEV; > } > @@ -68,3 +71,25 @@ UCLASS_DRIVER(pci_emul) = { > .pre_remove = sandbox_pci_emul_pre_remove, > .priv_auto_alloc_size = sizeof(struct sandbox_pci_emul_priv), > }; > + > +/* > + * This uclass is a child of the pci bus. Its platdata is not defined here so > + * is defined by its parent, UCLASS_PCI, which uses struct pci_child_platdata. > + * See per_child_platdata_auto_alloc_size in UCLASS_DRIVER(pci). > + */ > +UCLASS_DRIVER(pci_emul_parent) = { > + .id = UCLASS_PCI_EMUL_PARENT, > + .name = "pci_emul_parent", > + .post_bind = dm_scan_fdt_dev, > +}; > + > +static const struct udevice_id pci_emul_parent_ids[] = { > + { .compatible = "sandbox,pci-emul-parent" }, > + { } > +}; > + > +U_BOOT_DRIVER(pci_emul_parent_drv) = { > + .name = "pci_emul_parent_drv", > + .id = UCLASS_PCI_EMUL_PARENT, > + .of_match = pci_emul_parent_ids, > +}; > diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h > index d4d96106b37..f431f3bf294 100644 > --- a/include/dm/uclass-id.h > +++ b/include/dm/uclass-id.h > @@ -23,6 +23,7 @@ enum uclass_id { > UCLASS_I2C_EMUL, /* sandbox I2C device emulator */ > UCLASS_I2C_EMUL_PARENT, /* parent for I2C device emulators */ > UCLASS_PCI_EMUL, /* sandbox PCI device emulator */ > + UCLASS_PCI_EMUL_PARENT, /* parent for PCI device emulators */ > UCLASS_USB_EMUL, /* sandbox USB bus device emulator */ > UCLASS_AXI_EMUL, /* sandbox AXI bus device emulator */ > > -- Other than the issues above, Reviewed-by: Bin Meng <bmeng.cn@gmail.com> Tested-by: Bin Meng <bmeng.cn@gmail.com>
On Sat, Oct 5, 2019 at 1:03 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > On Wed, Sep 25, 2019 at 10:58 PM Simon Glass <sjg@chromium.org> wrote: > > > > Sandbox i2c works using emulation drivers which are currently children of > > pci > > > the i2c device: > > pci Fixed these, and > > > > > pci-controller { > > pci@1f,0 { > > compatible = "pci-generic"; > > reg = <0xf800 0 0 0 0>; > > emul@1f,0 { > > compatible = "sandbox,swap-case"; > > }; > > }; > > }; > > > > In this case the emulation device is attached to pci device on address > > f800 (device 1f, function 0) and provides the swap-case functionality. > > > > However this is not ideal, since every device on a PCI bus has a child > > device. This is only really the case for sandbox, but we want to avoid > > special-case code for sandbox. > > > > Worse, child devices cannot be probed before their parents. This forces > > us to use 'find' rather than 'get' to obtain the emulator device. In fact > > the emulator devices are never probed. There is code in > > sandbox_pci_emul_post_probe() which tries to track when emulators are > > active, but at present this does not work. > > > > A better approach seems to be to add a separate node elsewhere in the > > device tree, an 'emulation parent'. This could be given a bogus address > > (such as -1) to hide the emulators away from the 'pci' command, but it > > seems better to keep it at the root node to avoid such hacks. > > > > Then we can use a phandle to point from the evice to the correct > > typo: device Fixed this, and > > > > emulator, and only on sandbox. The code to find an emulator does not > > interfere with normal pci operation. > > > > Add a new UCLASS_PCI_EMUL_PARENT uclass which allows finding an emulator > > given a bus, and finding a bus given an emulator. Update the existing > > device trees and the code for finding an emulator. > > > > This brings PCI emulators more into line with I2C. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > arch/sandbox/dts/sandbox.dtsi | 11 +++++++--- > > arch/sandbox/dts/test.dts | 38 +++++++++++++++++++++++------------ > > doc/driver-model/pci-info.rst | 23 +++++++++++---------- > > drivers/pci/pci-emul-uclass.c | 37 ++++++++++++++++++++++++++++------ > > include/dm/uclass-id.h | 1 + > > 5 files changed, 77 insertions(+), 33 deletions(-) > > > > diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi > > index c6d5650c20b..f09bc70b0da 100644 > > --- a/arch/sandbox/dts/sandbox.dtsi > > +++ b/arch/sandbox/dts/sandbox.dtsi > > @@ -103,9 +103,14 @@ > > pci@1f,0 { > > compatible = "pci-generic"; > > reg = <0xf800 0 0 0 0>; > > - emul@1f,0 { > > - compatible = "sandbox,swap-case"; > > - }; > > + sandbox,emul = <&swap_case_emul>; > > + }; > > + }; > > + > > + emul { > > + compatible = "sandbox,pci-emul-parent"; > > + swap_case_emul: emul@1f,0 { > > + compatible = "sandbox,swap-case"; > > }; > > }; > > > > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts > > index 5669ede7051..a2e75981f0b 100644 > > --- a/arch/sandbox/dts/test.dts > > +++ b/arch/sandbox/dts/test.dts > > @@ -452,24 +452,31 @@ > > pci@0,0 { > > compatible = "pci-generic"; > > reg = <0x0000 0 0 0 0>; > > - emul@0,0 { > > - compatible = "sandbox,swap-case"; > > - }; > > + sandbox,emul = <&swap_case_emul0>; > > &swap_case_emul0_0 > > > }; > > pci@1,0 { > > compatible = "pci-generic"; > > reg = <0x0800 0 0 0 0>; > > - emul@0,0 { > > - compatible = "sandbox,swap-case"; > > - use-ea; > > - }; > > + sandbox,emul = <&swap_case_emul1>; > > &swap_case_emul0_1 > > > }; > > pci@1f,0 { > > compatible = "pci-generic"; > > reg = <0xf800 0 0 0 0>; > > - emul@1f,0 { > > - compatible = "sandbox,swap-case"; > > - }; > > + sandbox,emul = <&swap_case_emul1f>; > > &swap_case_emul0_1f Fixed the label names, and > > > + }; > > + }; > > + > > + pci-emul0 { > > + compatible = "sandbox,pci-emul-parent"; > > + swap_case_emul0: emul@0,0 { > > + compatible = "sandbox,swap-case"; > > + }; > > + swap_case_emul1: emul@1,0 { > > + compatible = "sandbox,swap-case"; > > + use-ea; > > + }; > > + swap_case_emul1f: emul@1f,0 { > > + compatible = "sandbox,swap-case"; > > }; > > }; > > > > @@ -499,9 +506,14 @@ > > pci@1f,0 { > > compatible = "pci-generic"; > > reg = <0xf800 0 0 0 0>; > > - emul@1f,0 { > > - compatible = "sandbox,swap-case"; > > - }; > > + sandbox,emul = <&swap_case_emul2_1f>; > > + }; > > + }; > > + > > + pci-emul2 { > > + compatible = "sandbox,pci-emul-parent"; > > + swap_case_emul2_1f: emul2@1f,0 { > > + compatible = "sandbox,swap-case"; > > }; > > }; > > > > diff --git a/doc/driver-model/pci-info.rst b/doc/driver-model/pci-info.rst > > index d93ab8b61d5..f39ff990a67 100644 > > --- a/doc/driver-model/pci-info.rst > > +++ b/doc/driver-model/pci-info.rst > > @@ -113,14 +113,17 @@ Sandbox > > ------- > > > > With sandbox we need a device emulator for each device on the bus since there > > -is no real PCI bus. This works by looking in the device tree node for a > > -driver. For example:: > > - > > +is no real PCI bus. This works by looking in the device tree node for an > > +emulator driver. For example:: > > > > pci@1f,0 { > > compatible = "pci-generic"; > > reg = <0xf800 0 0 0 0>; > > - emul@1f,0 { > > + sandbox,emul = <&emul_1f>; > > + }; > > + pci-emul { > > + compatible = "sandbox,pci-emul-parent"; > > + emul_1f: emul@1f,0 { > > compatible = "sandbox,swap-case"; > > }; > > }; > > @@ -130,14 +133,16 @@ Note that the first cell in the 'reg' value is the bus/device/function. See > > PCI_BDF() for the encoding (it is also specified in the IEEE Std 1275-1994 > > PCI bus binding document, v2.1) > > > > +The pci-emul node should go outside the pci bus node, since otherwise it will > > +be scanned as a PCI device, causing confusion. > > + > > When this bus is scanned we will end up with something like this:: > > > > `- * pci-controller @ 05c660c8, 0 > > `- pci@1f,0 @ 05c661c8, 63488 > > - `- emul@1f,0 @ 05c662c8 > > + `- emul@1f,0 @ 05c662c8 > > > > -When accesses go to the pci@1f,0 device they are forwarded to its child, the > > -emulator. > > +When accesses go to the pci@1f,0 device they are forwarded to its emulator. > > > > The sandbox PCI drivers also support dynamic driver binding, allowing device > > driver to declare the driver binding information via U_BOOT_PCI_DEVICE(), > > @@ -164,7 +169,3 @@ When this bus is scanned we will end up with something like this:: > > pci [ + ] pci_sandbo |-- pci-controller1 > > pci_emul [ ] sandbox_sw | |-- sandbox_swap_case_emul > > pci_emul [ ] sandbox_sw | `-- sandbox_swap_case_emul > > - > > -Note the difference from the statically declared device nodes is that the > > -device is directly attached to the host controller, instead of via a container > > -device like pci@1f,0. > > diff --git a/drivers/pci/pci-emul-uclass.c b/drivers/pci/pci-emul-uclass.c > > index 38227583547..f918e9a52fd 100644 > > --- a/drivers/pci/pci-emul-uclass.c > > +++ b/drivers/pci/pci-emul-uclass.c > > @@ -10,6 +10,7 @@ > > #include <linux/libfdt.h> > > #include <pci.h> > > #include <dm/lists.h> > > +#include <dm/uclass-internal.h> > > > > struct sandbox_pci_emul_priv { > > int dev_count; > > @@ -30,13 +31,15 @@ int sandbox_pci_get_emul(struct udevice *bus, pci_dev_t find_devfn, > > } > > *containerp = dev; > > > > - if (device_get_uclass_id(dev) == UCLASS_PCI_GENERIC) { > > - ret = device_find_first_child(dev, emulp); > > - if (ret) > > - return ret; > > - } else { > > + /* > > + * TODO(sjg@chromium.org): This code needs a comment as I'm not sure > > + * why UCLASS_PCI_GENERIC devices end up being their own emulators. I > > + * left this code as is. > > + */ > > This code comes from: > > commit 4345998ae9dfad7ba0beb54ad4322134557504a9 > Author: Bin Meng <bmeng.cn@gmail.com> > Date: Fri Aug 3 01:14:45 2018 -0700 > > pci: sandbox: Support dynamically binding device driver > > I think the comments can be removed :) Updated the comment to mention commit 4345998ae9dfad7ba0beb54ad4322134557504a9, and > > > + ret = uclass_find_device_by_phandle(UCLASS_PCI_EMUL, dev, > > + "sandbox,emul", emulp); > > + if (ret && device_get_uclass_id(dev) != UCLASS_PCI_GENERIC) > > *emulp = dev; > > - } > > > > return *emulp ? 0 : -ENODEV; > > } > > @@ -68,3 +71,25 @@ UCLASS_DRIVER(pci_emul) = { > > .pre_remove = sandbox_pci_emul_pre_remove, > > .priv_auto_alloc_size = sizeof(struct sandbox_pci_emul_priv), > > }; > > + > > +/* > > + * This uclass is a child of the pci bus. Its platdata is not defined here so > > + * is defined by its parent, UCLASS_PCI, which uses struct pci_child_platdata. > > + * See per_child_platdata_auto_alloc_size in UCLASS_DRIVER(pci). > > + */ > > +UCLASS_DRIVER(pci_emul_parent) = { > > + .id = UCLASS_PCI_EMUL_PARENT, > > + .name = "pci_emul_parent", > > + .post_bind = dm_scan_fdt_dev, > > +}; > > + > > +static const struct udevice_id pci_emul_parent_ids[] = { > > + { .compatible = "sandbox,pci-emul-parent" }, > > + { } > > +}; > > + > > +U_BOOT_DRIVER(pci_emul_parent_drv) = { > > + .name = "pci_emul_parent_drv", > > + .id = UCLASS_PCI_EMUL_PARENT, > > + .of_match = pci_emul_parent_ids, > > +}; > > diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h > > index d4d96106b37..f431f3bf294 100644 > > --- a/include/dm/uclass-id.h > > +++ b/include/dm/uclass-id.h > > @@ -23,6 +23,7 @@ enum uclass_id { > > UCLASS_I2C_EMUL, /* sandbox I2C device emulator */ > > UCLASS_I2C_EMUL_PARENT, /* parent for I2C device emulators */ > > UCLASS_PCI_EMUL, /* sandbox PCI device emulator */ > > + UCLASS_PCI_EMUL_PARENT, /* parent for PCI device emulators */ > > UCLASS_USB_EMUL, /* sandbox USB bus device emulator */ > > UCLASS_AXI_EMUL, /* sandbox AXI bus device emulator */ > > > > -- > > Other than the issues above, > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > Tested-by: Bin Meng <bmeng.cn@gmail.com> applied to u-boot-x86/next, thanks!
diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi index c6d5650c20b..f09bc70b0da 100644 --- a/arch/sandbox/dts/sandbox.dtsi +++ b/arch/sandbox/dts/sandbox.dtsi @@ -103,9 +103,14 @@ pci@1f,0 { compatible = "pci-generic"; reg = <0xf800 0 0 0 0>; - emul@1f,0 { - compatible = "sandbox,swap-case"; - }; + sandbox,emul = <&swap_case_emul>; + }; + }; + + emul { + compatible = "sandbox,pci-emul-parent"; + swap_case_emul: emul@1f,0 { + compatible = "sandbox,swap-case"; }; }; diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 5669ede7051..a2e75981f0b 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -452,24 +452,31 @@ pci@0,0 { compatible = "pci-generic"; reg = <0x0000 0 0 0 0>; - emul@0,0 { - compatible = "sandbox,swap-case"; - }; + sandbox,emul = <&swap_case_emul0>; }; pci@1,0 { compatible = "pci-generic"; reg = <0x0800 0 0 0 0>; - emul@0,0 { - compatible = "sandbox,swap-case"; - use-ea; - }; + sandbox,emul = <&swap_case_emul1>; }; pci@1f,0 { compatible = "pci-generic"; reg = <0xf800 0 0 0 0>; - emul@1f,0 { - compatible = "sandbox,swap-case"; - }; + sandbox,emul = <&swap_case_emul1f>; + }; + }; + + pci-emul0 { + compatible = "sandbox,pci-emul-parent"; + swap_case_emul0: emul@0,0 { + compatible = "sandbox,swap-case"; + }; + swap_case_emul1: emul@1,0 { + compatible = "sandbox,swap-case"; + use-ea; + }; + swap_case_emul1f: emul@1f,0 { + compatible = "sandbox,swap-case"; }; }; @@ -499,9 +506,14 @@ pci@1f,0 { compatible = "pci-generic"; reg = <0xf800 0 0 0 0>; - emul@1f,0 { - compatible = "sandbox,swap-case"; - }; + sandbox,emul = <&swap_case_emul2_1f>; + }; + }; + + pci-emul2 { + compatible = "sandbox,pci-emul-parent"; + swap_case_emul2_1f: emul2@1f,0 { + compatible = "sandbox,swap-case"; }; }; diff --git a/doc/driver-model/pci-info.rst b/doc/driver-model/pci-info.rst index d93ab8b61d5..f39ff990a67 100644 --- a/doc/driver-model/pci-info.rst +++ b/doc/driver-model/pci-info.rst @@ -113,14 +113,17 @@ Sandbox ------- With sandbox we need a device emulator for each device on the bus since there -is no real PCI bus. This works by looking in the device tree node for a -driver. For example:: - +is no real PCI bus. This works by looking in the device tree node for an +emulator driver. For example:: pci@1f,0 { compatible = "pci-generic"; reg = <0xf800 0 0 0 0>; - emul@1f,0 { + sandbox,emul = <&emul_1f>; + }; + pci-emul { + compatible = "sandbox,pci-emul-parent"; + emul_1f: emul@1f,0 { compatible = "sandbox,swap-case"; }; }; @@ -130,14 +133,16 @@ Note that the first cell in the 'reg' value is the bus/device/function. See PCI_BDF() for the encoding (it is also specified in the IEEE Std 1275-1994 PCI bus binding document, v2.1) +The pci-emul node should go outside the pci bus node, since otherwise it will +be scanned as a PCI device, causing confusion. + When this bus is scanned we will end up with something like this:: `- * pci-controller @ 05c660c8, 0 `- pci@1f,0 @ 05c661c8, 63488 - `- emul@1f,0 @ 05c662c8 + `- emul@1f,0 @ 05c662c8 -When accesses go to the pci@1f,0 device they are forwarded to its child, the -emulator. +When accesses go to the pci@1f,0 device they are forwarded to its emulator. The sandbox PCI drivers also support dynamic driver binding, allowing device driver to declare the driver binding information via U_BOOT_PCI_DEVICE(), @@ -164,7 +169,3 @@ When this bus is scanned we will end up with something like this:: pci [ + ] pci_sandbo |-- pci-controller1 pci_emul [ ] sandbox_sw | |-- sandbox_swap_case_emul pci_emul [ ] sandbox_sw | `-- sandbox_swap_case_emul - -Note the difference from the statically declared device nodes is that the -device is directly attached to the host controller, instead of via a container -device like pci@1f,0. diff --git a/drivers/pci/pci-emul-uclass.c b/drivers/pci/pci-emul-uclass.c index 38227583547..f918e9a52fd 100644 --- a/drivers/pci/pci-emul-uclass.c +++ b/drivers/pci/pci-emul-uclass.c @@ -10,6 +10,7 @@ #include <linux/libfdt.h> #include <pci.h> #include <dm/lists.h> +#include <dm/uclass-internal.h> struct sandbox_pci_emul_priv { int dev_count; @@ -30,13 +31,15 @@ int sandbox_pci_get_emul(struct udevice *bus, pci_dev_t find_devfn, } *containerp = dev; - if (device_get_uclass_id(dev) == UCLASS_PCI_GENERIC) { - ret = device_find_first_child(dev, emulp); - if (ret) - return ret; - } else { + /* + * TODO(sjg@chromium.org): This code needs a comment as I'm not sure + * why UCLASS_PCI_GENERIC devices end up being their own emulators. I + * left this code as is. + */ + ret = uclass_find_device_by_phandle(UCLASS_PCI_EMUL, dev, + "sandbox,emul", emulp); + if (ret && device_get_uclass_id(dev) != UCLASS_PCI_GENERIC) *emulp = dev; - } return *emulp ? 0 : -ENODEV; } @@ -68,3 +71,25 @@ UCLASS_DRIVER(pci_emul) = { .pre_remove = sandbox_pci_emul_pre_remove, .priv_auto_alloc_size = sizeof(struct sandbox_pci_emul_priv), }; + +/* + * This uclass is a child of the pci bus. Its platdata is not defined here so + * is defined by its parent, UCLASS_PCI, which uses struct pci_child_platdata. + * See per_child_platdata_auto_alloc_size in UCLASS_DRIVER(pci). + */ +UCLASS_DRIVER(pci_emul_parent) = { + .id = UCLASS_PCI_EMUL_PARENT, + .name = "pci_emul_parent", + .post_bind = dm_scan_fdt_dev, +}; + +static const struct udevice_id pci_emul_parent_ids[] = { + { .compatible = "sandbox,pci-emul-parent" }, + { } +}; + +U_BOOT_DRIVER(pci_emul_parent_drv) = { + .name = "pci_emul_parent_drv", + .id = UCLASS_PCI_EMUL_PARENT, + .of_match = pci_emul_parent_ids, +}; diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index d4d96106b37..f431f3bf294 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -23,6 +23,7 @@ enum uclass_id { UCLASS_I2C_EMUL, /* sandbox I2C device emulator */ UCLASS_I2C_EMUL_PARENT, /* parent for I2C device emulators */ UCLASS_PCI_EMUL, /* sandbox PCI device emulator */ + UCLASS_PCI_EMUL_PARENT, /* parent for PCI device emulators */ UCLASS_USB_EMUL, /* sandbox USB bus device emulator */ UCLASS_AXI_EMUL, /* sandbox AXI bus device emulator */
Sandbox i2c works using emulation drivers which are currently children of the i2c device: pci-controller { pci@1f,0 { compatible = "pci-generic"; reg = <0xf800 0 0 0 0>; emul@1f,0 { compatible = "sandbox,swap-case"; }; }; }; In this case the emulation device is attached to pci device on address f800 (device 1f, function 0) and provides the swap-case functionality. However this is not ideal, since every device on a PCI bus has a child device. This is only really the case for sandbox, but we want to avoid special-case code for sandbox. Worse, child devices cannot be probed before their parents. This forces us to use 'find' rather than 'get' to obtain the emulator device. In fact the emulator devices are never probed. There is code in sandbox_pci_emul_post_probe() which tries to track when emulators are active, but at present this does not work. A better approach seems to be to add a separate node elsewhere in the device tree, an 'emulation parent'. This could be given a bogus address (such as -1) to hide the emulators away from the 'pci' command, but it seems better to keep it at the root node to avoid such hacks. Then we can use a phandle to point from the evice to the correct emulator, and only on sandbox. The code to find an emulator does not interfere with normal pci operation. Add a new UCLASS_PCI_EMUL_PARENT uclass which allows finding an emulator given a bus, and finding a bus given an emulator. Update the existing device trees and the code for finding an emulator. This brings PCI emulators more into line with I2C. Signed-off-by: Simon Glass <sjg@chromium.org> --- arch/sandbox/dts/sandbox.dtsi | 11 +++++++--- arch/sandbox/dts/test.dts | 38 +++++++++++++++++++++++------------ doc/driver-model/pci-info.rst | 23 +++++++++++---------- drivers/pci/pci-emul-uclass.c | 37 ++++++++++++++++++++++++++++------ include/dm/uclass-id.h | 1 + 5 files changed, 77 insertions(+), 33 deletions(-)