Re: [PATCH v2 3/3] ARM: orion5x: Add D-Link DNS-323 based on Device Tree

[Andrew Lunn <andrew@lunn.ch>]

> +static void __init dns323_dt_eth_fixup(void)
> +{
> +	struct device_node *np;
> +	u8 addr[ETH_ALEN];
> +	int ret;
> +
> +	/*
> +	 * The ethernet interfaces forget the MAC address assigned by u-boot
> +	 * if the clocks are turned off. Usually, u-boot on orion boards
> +	 * has no DT support to properly set local-mac-address property.
> +	 * As a workaround, we get the MAC address that is stored in flash
> +	 * and update the port device node if no valid MAC address is set.
> +	 */

This is true for Kirkwood, but orion5x does not have any clocks to
gate. So i'm pretty sure this is not true. You copied this code for a
different reason. Please document here the real reason for this code.

> +	ret = dns323_read_mac_addr(addr);
> +
> +	if (ret) {
> +		pr_warn("Unable to find MAC address in flash memory\n");
> +		return;
> +	}
> +
> +	np = of_find_compatible_node(NULL, NULL, "marvell,orion-eth-port");
> +
> +	if (!IS_ERR(np)) {
> +		struct device_node *pnp = of_get_parent(np);
> +		struct clk *clk;
> +		struct property *pmac;
> +		u8 tmpmac[ETH_ALEN];
> +		u8 *macaddr;
> +		int i;
> +
> +		if (!pnp)
> +			return;
> +
> +		/* skip disabled nodes or nodes with valid MAC address*/
> +		if (!of_device_is_available(pnp) ||
> +		    !of_get_mac_address(np, tmpmac))
> +			goto eth_fixup_skip;
> +
> +		clk = of_clk_get(pnp, 0);
> +		if (IS_ERR(clk))
> +			goto eth_fixup_skip;
> +
> +		/* ensure port clock is not gated to not hang CPU */
> +		clk_prepare_enable(clk);

I'm pretty sure this clock stuff is not needed. Please comment it out
and see if the machine locks up. Kirkwood just stops dead if you
access registers when there clocks are disabled. For Orion5x, the
ethernet should always have a clock.

> +
> +		/* store MAC address register contents in local-mac-address */
> +		pmac = kzalloc(sizeof(*pmac) + 6, GFP_KERNEL);
> +		if (!pmac)
> +			goto eth_fixup_no_mem;
> +
> +		pmac->value = pmac + 1;
> +		pmac->length = ETH_ALEN;
> +		pmac->name = kstrdup("local-mac-address", GFP_KERNEL);
> +		if (!pmac->name) {
> +			kfree(pmac);
> +			goto eth_fixup_no_mem;
> +		}
> +
> +		macaddr = pmac->value;
> +		for (i = 0; i < ETH_ALEN; i++)
> +			macaddr[i] = addr[i];
> +
> +		of_update_property(np, pmac);
> +
> +eth_fixup_no_mem:
> +		clk_disable_unprepare(clk);
> +		clk_put(clk);
> +eth_fixup_skip:
> +		of_node_put(pnp);
> +	}
> +}
> +
> +void __init dns323_init_dt(void)
> +{
> +	if (of_machine_is_compatible("dlink,dns323a1")) {
> +		writel(0, MPP_DEV_CTRL);		/* DEV_D[31:16] */

I spotted this in dns323-setup.c as well. Do you have any idea what it
does?

	Andrew