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

[Simon Glass]

Hi Joe,

On 8 September 2015 at 09:09, Joe Hershberger <joe.hershberger at gmail.com> wrote:
> Hi Simon,
>
> On Tue, Sep 8, 2015 at 9:22 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Joe,
>>
>> On 4 September 2015 at 23:57, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> 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.
>>>
>>
>> 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