Hi Joe, On 8 September 2015 at 09:09, Joe Hershberger <joe.hershber...@gmail.com> wrote: > Hi Simon, > > On Tue, Sep 8, 2015 at 9:22 AM, Simon Glass <s...@chromium.org> wrote: >> Hi Joe, >> >> On 4 September 2015 at 23:57, Bin Meng <bmeng...@gmail.com> wrote: >>> Hi Joe, >>> >>> On Fri, Sep 4, 2015 at 11:29 PM, Joe Hershberger >>> <joe.hershber...@gmail.com> wrote: >>>> Hi Bin, >>>> >>>> On Wed, Sep 2, 2015 at 4:17 AM, Bin Meng <bmeng...@gmail.com> wrote: >>>>> The Designware ethernet controller is also seen on PCI bus, e.g. >>>>> on Intel Quark SoC. Add this support in the DM version driver. >>>>> >>>>> Signed-off-by: Bin Meng <bmeng...@gmail.com> >>>> >>>> Looks good. One question below. >>>> >>>>> --- >>>>> >>>>> Changes in v2: >>>>> - Change to use device_is_on_pci_bus() >>>>> >>>>> drivers/net/designware.c | 39 +++++++++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 39 insertions(+) >>>>> >>>>> diff --git a/drivers/net/designware.c b/drivers/net/designware.c >>>>> index ae78d21..39a9e6b 100644 >>>>> --- a/drivers/net/designware.c >>>>> +++ b/drivers/net/designware.c >>>>> @@ -14,6 +14,7 @@ >>>>> #include <errno.h> >>>>> #include <miiphy.h> >>>>> #include <malloc.h> >>>>> +#include <pci.h> >>>>> #include <linux/compiler.h> >>>>> #include <linux/err.h> >>>>> #include <asm/io.h> >>>>> @@ -558,6 +559,20 @@ static int designware_eth_write_hwaddr(struct >>>>> udevice *dev) >>>>> return _dw_write_hwaddr(priv, pdata->enetaddr); >>>>> } >>>>> >>>>> +static int designware_eth_bind(struct udevice *dev) >>>>> +{ >>>>> + static int num_cards; >>>>> + char name[20]; >>>>> + >>>>> + /* Create a unique device name for PCI type devices */ >>>>> + if (device_is_on_pci_bus(dev)) { >>>>> + sprintf(name, "eth_designware#%u", num_cards++); >>>>> + device_set_name(dev, name); >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> static int designware_eth_probe(struct udevice *dev) >>>>> { >>>>> struct eth_pdata *pdata = dev_get_platdata(dev); >>>>> @@ -565,6 +580,22 @@ static int designware_eth_probe(struct udevice *dev) >>>>> u32 iobase = pdata->iobase; >>>>> int ret; >>>>> >>>>> + /* >>>>> + * If we are on PCI bus, either directly attached to a PCI root >>>>> port, >>>>> + * or via a PCI bridge, fill in platdata before we probe the >>>>> hardware. >>>>> + */ >>>>> + if (device_is_on_pci_bus(dev)) { >>>>> + pci_dev_t bdf; >>>>> + >>>>> + bdf = pci_get_bdf(dev); >>>>> + pci_read_config_dword(bdf, PCI_BASE_ADDRESS_0, &iobase); >>>>> + iobase &= PCI_BASE_ADDRESS_MEM_MASK; >>>>> + iobase = pci_mem_to_phys(bdf, iobase); >>>>> + >>>>> + pdata->iobase = iobase; >>>>> + pdata->phy_interface = PHY_INTERFACE_MODE_RMII; >>>> >>>> Is this just assumed for all PCI devices? Seems like it could vary, >>>> but I don't know the specifics of all the implementations this is >>>> expected to support. Should it be part of a quirk based on PID or >>>> something? >>> >>> I think it could vary, but given so far the driver only supports Intel >>> Quark PCI variant, so I did not put a test logic against PCI >>> vendor/device ID here. When in the future we support another PCI >>> variant, we can add such logic. >>> >> >> Does this sound OK to you? I'd like to apply this series to u-boot-x86 >> but want to make sure you are happy with it. > > This is fine. It will be up to the next developer to design the way to > select this attribute. I'm guessing this is the first driver of its > kind (PCI - different PHY connections) so there isn't anything to > model after yet.
OK, I'll go ahead and apply the rest of this series then. > >>>> >>>>> + } >>>>> + >>>>> debug("%s, iobase=%x, priv=%p\n", __func__, iobase, priv); >>>>> priv->mac_regs_p = (struct eth_mac_regs *)iobase; >>>>> priv->dma_regs_p = (struct eth_dma_regs *)(iobase + >>>>> DW_DMA_BASE_OFFSET); >>>>> @@ -617,10 +648,18 @@ U_BOOT_DRIVER(eth_designware) = { >>>>> .id = UCLASS_ETH, >>>>> .of_match = designware_eth_ids, >>>>> .ofdata_to_platdata = designware_eth_ofdata_to_platdata, >>>>> + .bind = designware_eth_bind, >>>>> .probe = designware_eth_probe, >>>>> .ops = &designware_eth_ops, >>>>> .priv_auto_alloc_size = sizeof(struct dw_eth_dev), >>>>> .platdata_auto_alloc_size = sizeof(struct eth_pdata), >>>>> .flags = DM_FLAG_ALLOC_PRIV_DMA, >>>>> }; >>>>> + >>>>> +static struct pci_device_id supported[] = { >>>>> + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_QRK_EMAC) }, >>>>> + { } >>>>> +}; >>>>> + >>>>> +U_BOOT_PCI_DEVICE(eth_designware, supported); >>>>> #endif >>>>> -- Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot