Hi Krzysztof,
On 6/23/21 3:53 PM, Krzysztof Wilczyński wrote:
> [+cc Sasha for visibility]
>
> Hi Michal,
>
> Thank you for sending v2 so promptly! And for all the extra changes and
> fixes. Much appreciated!
>
>> Enable PCIE reference clock. There is no remove function that's why
>> this should be enough for simple operation.
>> Normally this clock is enabled by default by firmware but there are
>> usecases where this clock should be enabled by driver itself.
>> It is also good that clock user is recorded in clock framework.
>
> Small nitpicks: it would be PCIe here in the above and in the error
> message (this is as per [1]), and "use cases" also in the above.
>
> This can be corrected when the patch will be merged by either Bjorn or
> Lorenzo, to avoid sending v3 unnecessarily, provided that they would
> have a moment to do it, of course.
Ok. Will wait for them.
>
>> Fixes: ab597d35ef11 ("PCI: xilinx-nwl: Add support for Xilinx NWL PCIe Host Controller")
>
> Thank you!
>
> Does it make sense for this change to be back-ported to stable and
> long-term kernels?
>
> I am asking to make sure we do the right thing here, as I can imagine
> that older kernels (primarily because some folks could use, for example,
> Ubuntu LTS releases for development) might often be used by people who
> work with the Xilinx FPGAs and such.
I think that make sense to do so. I haven't had a time to take look at
it closely but I think on Xilinx ZynqMP zcu102 board this missing patch
is causing hang when standard debian 5.10 is used.
>
> [...]
>> + err = clk_prepare_enable(pcie->clk);
>> + if (err) {
>> + dev_err(dev, "can't enable pcie ref clock\n");
>> + return err;
>> + }
>> +
>
> As per the nitpick above, it would be "PCIe", but probably no need to
> send v3 to correct this.
I will keep my eyes on it and will update it if v3 is required.
Thanks,
Michal
