[PATCH] clk: zynq: Fix EMIO clock use detection for gem0

[Michal Simek]

On 4/25/24 17:18, Ondřej Jirman wrote:
> On Thu, Apr 25, 2024 at 12:59:29PM GMT, Michal Simek wrote:
>>> Well, it doesn't help that the code is almost all refering to CLK_CTRL while
>>> actually accessing gem1_rclk_ctrl in the struct.
>>>
>>> In any case it can't detect the case when sourcing the clock from EMIO and
>>> not one of the PLLs, apparently.
>>>
>>> The failure I'm talking about is here in zynq_gem.c:
>>>
>>>           ret = clk_get_rate(&priv->tx_clk);
>>>           if (ret != clk_rate) {
>>>                   ret = clk_set_rate(&priv->tx_clk, clk_rate);
>>>                   if (IS_ERR_VALUE(ret)) {
>>>                           dev_err(dev, "failed to set tx clock rate %ld\n", clk_rate);
>>>                           return ret;
>>>                   }
>>>           }
>>>
>>> And all I get is "failed to set tx clock rate" from U-Boot and no ethernet.
>>
>> Zynq is quite a old device but I expect you have external clock out of chip
>> or you generate it in PL. For that you need to describe it.
>>
>> How does your DT looks like for external clocks?
> 
> The situation is that RX/TX clocks just come from outside
> https://megous.com/dl/tmp/af9e7c9e8d51781b.png via PL. 25MHz clock is
> generated by the PHY.
> 
> I solved it eventually.
> 
> I described the PHY generated clock as a fixed-clock node, and added a
> reference to it under &clkc as a source for "gem0_emio_clk".
> 
>    https://megous.com/git/u-boot/tree/arch/arm/dts/zynq-ebaz-megi.dts?h=v2024.07#n26
> 
> Zynq clk code seems to fetch gem0_emio_clk from DT for the purpose of
> determining the clk rate for gem0 in emio mode:
> 
>    https://elixir.bootlin.com/u-boot/latest/source/drivers/clk/clk_zynq.c#L524
>    https://elixir.bootlin.com/u-boot/latest/source/drivers/clk/clk_zynq.c#L280
> 
> This got rid of the "failed to set tx clock rate" error. Thank you for the hint.
> 
> 
> BTW, I managed to get the .bit file loaded via SPL only with a SPL patch:
> 
>    https://megous.com/git/u-boot/commit/?h=v2024.07&id=c1403bd080bc517c9dc6f507ee12b62fa85153bc
> 
> It's similar to how it's done in:
> 
>    https://elixir.bootlin.com/u-boot/latest/source/boot/image-board.c#L677
> 
> fpga_load() function doesn't work with .bit file I get from Vivado but
> fpga_loadbitstream() one does. SPL doesn't call it, though. Is that by design?
> Should I be converting .bit file to something else usable by fpga_load()
> somehow?

Yes there was limitation or missing detection which fpga format is supported.

> 
> I'd like to load bitstream in SPL from FIT because U-Boot proper may already
> need some PL logic activated for ethernet to work there. Also it's one less
> file on the /boot filesystem to worry about. :)

I think you have two options.
1. convert your bitstream in bit format to bin format
2. Extend SPL code to support both formats.

I would go with option 1 but patches for option 2 are definitely welcome.

Thanks,
Michal