[U-Boot] [PATCH v2 2/4] net: add MSCC Ocelot switch support

[Daniel Schwierzeck]

Am 16.01.19 um 15:21 schrieb Gregory CLEMENT:
> Hi Daniel,
>  
>  On mer., janv. 16 2019, Daniel Schwierzeck <daniel.schwierzeck at 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.

maybe a problem with asm/io.h? Could have been the same issue as with
enabling DM live-tree. This was fixed with
a0abb52c5a5b1de522380984bb9429ee973b1f8e

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

-- 
- Daniel