Messages in this thread | | | Date | Fri, 23 Sep 2022 12:03:14 +0300 | From | maukka@ext ... | Subject | Re: [PATCH v2 3/3] ARM: orion5x: Add D-Link DNS-323 based on Device Tree |
| |
On 23.9.2022 00:10, Andrew Lunn wrote: >> +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. >
Yes, will do. To my understanding it looks like uboot does not pass anything to the kernel.
>> + 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. >
Will do.
>> + >> + /* 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? >
No idea. I have tried to replicate what was in dns323-setup.c as exactly as possible. I can try to leave it out and see if anything changes.
| |