On Sat, Oct 5, 2019 at 1:03 PM Bin Meng <bmeng...@gmail.com> wrote:
>
> On Wed, Sep 25, 2019 at 10:58 PM Simon Glass <s...@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 <s...@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(s...@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...@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...@gmail.com>
> Tested-by: Bin Meng <bmeng...@gmail.com>
applied to u-boot-x86/next, thanks!
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
