Messages in this thread | | | Date | Fri, 22 Jan 2021 12:20:27 +0100 | From | Michael Walle <> | Subject | Re: [PATCH] net: macb: ignore tx_clk if MII is used |
| |
Am 2021-01-22 10:10, schrieb Claudiu.Beznea@microchip.com: > On 21.01.2021 11:41, Michael Walle wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know >> the >> content is safe >> >> Hi Claudiu, >> >> Am 2021-01-21 10:19, schrieb Claudiu.Beznea@microchip.com: >>> On 20.01.2021 21:43, Michael Walle wrote: >>>> EXTERNAL EMAIL: Do not click links or open attachments unless you >>>> know >>>> the content is safe >>>> >>>> If the MII interface is used, the PHY is the clock master, thus >>>> don't >>>> set the clock rate. On Zynq-7000, this will prevent the following >>>> warning: >>>> macb e000b000.ethernet eth0: unable to generate target frequency: >>>> 25000000 Hz >>>> >>> >>> Since in this case the PHY provides the TX clock and it provides the >>> proper >>> rate based on link speed, the MACB driver should not handle the >>> bp->tx_clk >>> at all (MACB driver uses this clock only for setting the proper rate >>> on >>> it >>> based on link speed). So, I believe the proper fix would be to not >>> pass >>> the >>> tx_clk at all in device tree. This clock is optional for MACB driver. >> >> Thanks for looking into this. >> >> I had the same thought. But shouldn't the driver handle this case >> gracefully? >> I mean it does know that the clock isn't needed at all. > > Currently it may knows that by checking the bp->tx_clk. Moreover the > clock > could be provided by PHY not only for MII interface.
That doesn't make this patch wrong, does it? It just doesn't cover all use cases (which also wasn't covered before).
> Moreover the IP has the bit "refclk" of register at offset 0xc (userio) > that tells it to use the clock provided by PHY or to use one internal > to > the SoC. If a SoC generated clock would be used the IP logic may have > the > option to do the proper division based on link speed (if IP has this > option > enabled then this should be selected in driver with capability > MACB_CAPS_CLK_HW_CHG). > > If the clock provided by the PHY is the one to be used then this is > selected with capability MACB_CAPS_USRIO_HAS_CLKEN. So, if the change > you > proposed in this patch is still imperative then checking for this > capability would be the best as the clock could be provided by PHY not > only > for MII interface.
Fair enough, but this register doesn't seem to be implemented on Zynq-7000. Albeit MACB_CAPS_USRIO_DISABLED isn't defined for the Zynq MACB. It isn't defined in the Zynq-7000 reference manual and you cannot set any bits:
=> mw 0xE000B00C 0xFFFFFFFF => md 0xE000B00C 1 e000b00c: 00000000
Also please note, that tx_clk may be an arbitrary clock which doesn't necessarily need to be the clock which is controlled by CLK_EN. Or am I missing something here?
-michael
>> Ususually that >> clock >> is defined in a device tree include. So you'd have to redefine that >> node >> in >> an actual board file which means duplicating the other clocks. >> >> -michael
| |