[U-Boot] [PATCH v2 09/11] net: designware: Add support to PCI designware devices

[Bin Meng]

Hi Joe,

On Fri, Sep 4, 2015 at 11:29 PM, Joe Hershberger
<joe.hershberger at gmail.com> wrote:
> Hi Bin,
>
> On Wed, Sep 2, 2015 at 4:17 AM, 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>
>
> 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.

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