On 8/27/2019 3:38 AM, Voon, Weifeng wrote: >>>> +#include <linux/clk-provider.h> >>>> #include <linux/pci.h> >>>> #include <linux/dmi.h> >>>> >>>> @@ -174,6 +175,19 @@ static int intel_mgbe_common_data(struct >> pci_dev *pdev, >>>> plat->axi->axi_blen[1] = 8; >>>> plat->axi->axi_blen[2] = 16; >>>> >>>> + plat->ptp_max_adj = plat->clk_ptp_rate; >>>> + >>>> + /* Set system clock */ >>>> + plat->stmmac_clk = clk_register_fixed_rate(&pdev->dev, >>>> + "stmmac-clk", NULL, 0, >>>> + plat->clk_ptp_rate); >>>> + >>>> + if (IS_ERR(plat->stmmac_clk)) { >>>> + dev_warn(&pdev->dev, "Fail to register stmmac-clk\n"); >>>> + plat->stmmac_clk = NULL; >>> >>> Don't you need to propagate at least EPROBE_DEFER here? >> >> Hi Florian >> >> Isn't a fixed rate clock a complete fake. There is no hardware behind it. >> So can it return EPROBE_DEFER? >> >> Andrew > > Yes, there is no hardware behind it. So, I don't think we need to deferred > probe > and a warning message should be sufficient. Anyhow, please point it out if I > miss > out anything. Looks good to me, thanks both for clarifying. -- Florian