[U-Boot] [PATCH 6/8] net: designware: Add support to PCI designware devices

[Simon Glass]

Hi Bin,

On 31 August 2015 at 19:19, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Tue, Sep 1, 2015 at 8:33 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Bin,
>>
>> On 31 August 2015 at 03:52, Bin Meng <bmeng.cn at 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.cn at gmail.com>
>>> ---
>>>
>>>  drivers/net/designware.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 39 insertions(+)
>>>
>>> diff --git a/drivers/net/designware.c b/drivers/net/designware.c
>>> index ae78d21..06d6f6a 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_get_uclass_id(dev->parent) == UCLASS_PCI) {
>>> +               sprintf(name, "eth_designware#%u", num_cards++);
>>> +               device_set_name(dev, name);
>>
>> Hmm this is the second time it has come up/ This is OK, but I wonder
>> if we should add a generic function that names devices based on the
>> number seen for each driver?
>
> Maybe not based on the number seen for each driver, as some drivers
> can support both PCI and non-PCI devices. What we need is just for PCI
> devices. For non-PCI devices, device tree already provides the name,
> although they might not be unique (depending on how device tree is
> written)
>
>> Could be tricky though. Maybe when we get to 3?
>>
>
> Yes, it is tricky. It is a per-driver setting, but depending on where
> the device resides.

OK, well what you have is OK for now.

>
>>> +       }
>>> +
>>> +       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_get_uclass_id(dev->parent) == UCLASS_PCI) {
>>
>> To make it easier to adjust the logic here, can you please create
>> something like device_is_on_pci_bus() to hold this logic? I still
>> think we might change to using UCLASS_BRIDGE for bridges.
>>
>
> Yes, I can add device_is_on_pci_bus(). Do you think we should put it
> into pci-uclass.c, or just local in this designware driver?

If it is static inline then pci.h would be OK. I'm not sure it would
work if you put in pci-uclass.c since some drivers might be built
without PCI support enabled?

>
>>> +               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;
>>> +       }
>>> +
>>>         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,
> Bin

Regards,
Simon