From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michal Simek <michal.simek@xilinx.com>
Date: Tue, 1 Dec 2015 08:08:01 +0100
Subject: [U-Boot] [PATCH 12/14] net: gem: Move driver to DM
In-Reply-To: <CAPnjgZ301ChAMYQoN5DReXfe7N8tMi4Ckz+Nos40RthHN6PMJQ@mail.gmail.com>
References: <0338b73426d58c60170a52813ee6a113c50bb6be.1448895925.git.michal.simek@xilinx.com>
 <619d64346109f2dbe81ec177c2febc6786a6e058.1448895925.git.michal.simek@xilinx.com>
 <CAPnjgZ301ChAMYQoN5DReXfe7N8tMi4Ckz+Nos40RthHN6PMJQ@mail.gmail.com>
Message-ID: <565D4751.6000901@xilinx.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Simon,

On 1.12.2015 00:17, Simon Glass wrote:
> Hi Michal,
> 

...

>> diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
>> index e169b774932a..5ea992a3ce65 100644
>> --- a/drivers/mmc/zynq_sdhci.c
>> +++ b/drivers/mmc/zynq_sdhci.c
>> @@ -33,6 +33,23 @@ int zynq_sdhci_init(phys_addr_t regbase)
>>         return 0;
>>  }
>>
>> +
>> +
>> +static const struct udevice_id arasan_sdhci_ids[] = {
>> +       { .compatible = "arasan,sdhci-8.9a" },
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(arasan_sdhci_drv) = {
>> +       .name           = "rockchip_dwmmc",
>> +       .id             = UCLASS_MMC,
>> +       .of_match       = rockchip_dwmmc_ids,
>> +       .ofdata_to_platdata = rockchip_dwmmc_ofdata_to_platdata,
>> +       .probe          = rockchip_dwmmc_probe,
>> +       .priv_auto_alloc_size = sizeof(struct rockchip_dwmmc_priv),
>> +};
>> +
>> +
> 
> This seems unrelated / also rockchip stuff.

I reported it in my reply that this was added by accident. As you know I
am playing with SD DM and rockchip was that guy I use for inspiration.


> 
>>  #if CONFIG_IS_ENABLED(OF_CONTROL)
>>  int zynq_sdhci_of_init(const void *blob)
>>  {
>> diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c
>> index 4e93707c7ab1..f2a14938036f 100644
>> --- a/drivers/net/zynq_gem.c
>> +++ b/drivers/net/zynq_gem.c
>> @@ -13,6 +13,7 @@
>>  #include <net.h>
>>  #include <netdev.h>
>>  #include <config.h>
>> +#include <dm.h>
> 
> Can you put <dm.h> up higher so that these are in order?

sure

...

>>
>> -       miiphy_register(dev->name, zynq_gem_miiphy_read, zynq_gem_miiphy_write);
>> -       priv->bus = miiphy_get_dev_by_name(dev->name);
>> +static int zynq_gem_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +       struct eth_pdata *pdata = dev_get_platdata(dev);
>> +       struct zynq_gem_priv *priv = dev_get_priv(dev);
>> +       int offset = 0;
>>
>> -       ret = zynq_phy_init(dev);
>> -       if (ret)
>> -               return ret;
>> +       pdata->iobase = (phys_addr_t)dev_get_addr(dev);
>> +       priv->iobase = (struct zynq_gem_regs *)dev_get_addr(dev);
> 
> Better to use:
> 
> priv->iobase = (struct zynq_gem_regs *)pdata->iobase
> 
> I think. But is pdata->iobase ever used?

That was one think I wanted to check. There is eth_pdata structure which
has iobase, enetaddr and phy_interface.

I do fill them here but driver is using iobase saved in private
structure. I do need more information from private structure that's why
I don't need to load it from pdata structure.

I probably also miss to allocate pdata. Is this required?
.platdata_auto_alloc_size = sizeof(struct eth_pdata)

Can you please check logic around pdata if I use it right?


>> +       /* Hardcode for now */
>> +       priv->emio = 0;
>> +
>> +       offset = fdtdec_lookup_phandle(gd->fdt_blob, dev->of_offset,
>> +                                      "phy-handle");
>> +       if (offset != -1)
> 
> I think this should be:
> 
> offset > 0

ok. Will fix it.

Thanks,
Michal