On 12/09/2022 02:38, Icenowy Zheng wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > 在 2022-09-08星期四的 18:14 +0000，Conor.Dooley@xxxxxxxxxxxxx写道： >> On 07/09/2022 06:40, Icenowy Zheng wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you >>> know the content is safe >>> >>> The DT binding of FU740 PCIe does not enforce a clock-names property, >>> and there exist some device tree that has a clock name that does not >>> stick to the one used by Linux DT (e.g. the one shipped with current >>> U-Boot mainline). >> >> I recently added the missing enforcement: >> https://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git/commit/?h=pci/dt&id=b408fad61d34c765c3e01895286332af2d50402a > > Unfortunately binding w/o clock-names enforcement has already entered a > stable release (5.19), and the real clock name "pcie_aux" is never > enforced before (there's a DT in U-Boot that uses "pcieaux" instead), > should this be considered as breakage to stable DT binding? Does anything in U-Boot actually use that clock name? The clock name is currently being relied on by both Linux and BSD (although BSD does have a fallback to the U-Boot provided name. There's only one clock so it seems fine to me to stop using the name, but the DT in U-Boot should be fixed so that PCI works IMO. fwiw: > > Anyway, I had sent out a patch that synchorizes all FU740-related DT > files to U-Boot, see [1]. > > [1] > https://lore.kernel.org/all/20220825081119.1694007-2-uwu@xxxxxxxxxx/ From that patch, should this be changed too? - [PRCI_CLK_PCIEAUX] { + [FU740_PRCI_CLK_PCIE_AUX] { .name = "pcieaux", .parent_name = "", .ops = &sifive_fu740_prci_pcieaux_clk_ops, > >> >> Since there's only one clock though, I'd imagine it makes little to no >> real difference if the check here is relaxed. >> >> Reviewed-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> >> >>> >>> Drop the name in the clock request, instead just pass NULL (because >>> this device should have only a single clock). >>> >>> Signed-off-by: Icenowy Zheng <uwu@xxxxxxxxxx> >>> --- >>> drivers/pci/controller/dwc/pcie-fu740.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/pci/controller/dwc/pcie-fu740.c >>> b/drivers/pci/controller/dwc/pcie-fu740.c >>> index 0c90583c078b..edb218a37a4f 100644 >>> --- a/drivers/pci/controller/dwc/pcie-fu740.c >>> +++ b/drivers/pci/controller/dwc/pcie-fu740.c >>> @@ -315,7 +315,7 @@ static int fu740_pcie_probe(struct >>> platform_device *pdev) >>> return dev_err_probe(dev, PTR_ERR(afp->pwren), >>> "unable to get pwren-gpios\n"); >>> >>> /* Fetch clocks */ >>> - afp->pcie_aux = devm_clk_get(dev, "pcie_aux"); >>> + afp->pcie_aux = devm_clk_get(dev, NULL); >>> if (IS_ERR(afp->pcie_aux)) >>> return dev_err_probe(dev, PTR_ERR(afp->pcie_aux), >>> "pcie_aux clock source >>> missing or invalid\n"); >>> -- >>> 2.37.1 >>> >> > >
