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

[Bin Meng]

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.

>> +       }
>> +
>> +       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?

>> +               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