lkml.org 
[lkml]   [2022]   [Sep]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [PATCH] PCI: fu740: do not use clock name when requesting clock
Date
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@microchip.com写道:
>> 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@icenowy.me/

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@microchip.com>
>>
>>>
>>> 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@icenowy.me>
>>> ---
>>> 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
>>>
>>
>
>

\
 
 \ /
  Last update: 2022-09-12 12:14    [W:0.055 / U:0.732 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site