Hi Daniel,
 
 On mer., janv. 16 2019, Daniel Schwierzeck <daniel.schwierz...@gmail.com> 
wrote:
>> +static int mscc_miim_wait_ready(struct mscc_miim_dev *miim)
>> +{
>> +    unsigned long deadline;
>> +    u32 val;
>> +
>> +    deadline = timer_get_us() + 250000;
>> +
>> +    do {
>> +            val = readl(miim->regs + MIIM_STATUS);
>> +    } while (timer_get_us() <= deadline && (val & MIIM_STAT_BUSY));
>
> you use this multiple times, maybe it makes sense to add a generic
> wait_for_bit set with timer_get_us()

Sure I will do it.

>> +
>> +    priv->data = (struct ocelot_soc_data *)dev_get_platdata(dev);
>> +    if (!priv->data)
>> +            return -EINVAL;
>> +
>
> actually you could avoid the overhead of allocating platdata and merge
> ocelot_ofdata_to_platdata() into ocelot_probe(). platdata is only needed
> if a driver want to support configuration without a device tree.

OK as we don't plan to support this driver without device tree, I will
do it.

>
>> +    plat = priv->data;
>> +    for (i = 0; i < ARRAY_SIZE(reg); i++) {
>> +            ret = dev_read_resource_byname(dev, reg[i].name, &res);
>> +            if (ret) {
>> +                    debug
>> +                        ("Error %d: can't get regs base addresses for %s\n",
>> +                         ret, reg[i].name);
>> +                    return -ENOMEM;
>> +            }
>> +
>> +            faddr = cpu_to_fdt32(res.start);
>> +            plat->base[reg[i].id] = dev_translate_address(dev, &faddr);
>> +            if (plat->base[reg[i].id] == OF_BAD_ADDR)
>> +                    return -ENOMEM;
>> +            plat->size[reg[i].id] = res.end - res.start;
>> +    }
>
> have you tried with dev_read_addr_name()? If maybe address translation
> didn't work, a disabled CONFIG_OF_TRANSLATE could be the reason.

Indeed I have issue with adress translation, and if I remember well,
when I tried to activated CONFIG_OF_TRANSLATE I had build failure with
MIPS.

>
>> +
>> +    /* gathered only the first mdio bus */
>> +    eth_node = dev_read_first_subnode(dev);
>> +    node = ofnode_first_subnode(eth_node);
>> +    ofnode_parse_phandle_with_args(node, "phy-handle", NULL, 0, 0,
>> +                                   &phandle);
>> +    mdio_node = ofnode_get_parent(phandle.node);
>> +
>> +    for (i = 0; i < TARGET_MDIO_MAX; i++) {
>> +            if (ofnode_read_resource(mdio_node, i, &res))
>> +                    return -ENOMEM;
>> +            faddr = cpu_to_fdt32(res.start);
>> +            plat->phy_base[INTERNAL][i] =
>> +                    ofnode_translate_address(mdio_node, &faddr);
>> +            plat->phy_size[INTERNAL][i] = res.end - res.start;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int ocelot_probe(struct udevice *dev)
>> +{
>> +    struct ocelot_private *priv = dev_get_priv(dev);
>> +    int i;
>> +
>> +    if (!priv)
>> +            return -EINVAL;
>> +
>> +    for (i = 0; i < TARGET_MAX; i++)
>> +            priv->regs[i] = ioremap(priv->data->base[i],
>> +                                    priv->data->size[i]);
>> +
>
> if dev_read_addr_name() works, you could also use dev_remap_addr_name()
> to implicitely do the ioremap

if dev_read_addr_name() works yes I can use it.

Gregory

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to