[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f39edf3d-aa9e-43a0-8997-762d76c9c248@kernel.org>
Date: Thu, 5 Sep 2024 18:52:10 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Andrea della Porta <andrea.porta@...e.com>
Cc: Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Florian Fainelli <florian.fainelli@...adcom.com>,
Broadcom internal kernel review list
<bcm-kernel-feedback-list@...adcom.com>,
Linus Walleij <linus.walleij@...aro.org>,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
Derek Kiernan <derek.kiernan@....com>, Dragan Cvetic
<dragan.cvetic@....com>, Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Nicolas Ferre <nicolas.ferre@...rochip.com>,
Claudiu Beznea <claudiu.beznea@...on.dev>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Saravana Kannan <saravanak@...gle.com>, Bjorn Helgaas <bhelgaas@...gle.com>,
linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
linux-rpi-kernel@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
netdev@...r.kernel.org, linux-pci@...r.kernel.org,
linux-arch@...r.kernel.org, Lee Jones <lee@...nel.org>,
Andrew Lunn <andrew@...n.ch>, Stefan Wahren <wahrenst@....net>
Subject: Re: [PATCH 08/11] misc: rp1: RaspberryPi RP1 misc driver
On 05/09/2024 18:33, Andrea della Porta wrote:
> Hi Krzysztof,
>
> On 20:27 Tue 03 Sep , Krzysztof Kozlowski wrote:
>> On 03/09/2024 17:15, Andrea della Porta wrote:
>>>>>>> +
>>>>>>> + rp1_clocks: clocks@...0018000 {
>>>>>>
>>>>>> Why do you mix MMIO with non-MMIO nodes? This really does not look
>>>>>> correct.
>>>>>>
>>>>>
>>>>> Right. This is already under discussion here:
>>>>> https://lore.kernel.org/all/ZtBzis5CzQMm8loh@apocalypse/
>>>>>
>>>>> IIUC you proposed to instantiate the non-MMIO nodes (the three clocks) by
>>>>> using CLK_OF_DECLARE.
>>>>
>>>> Depends. Where are these clocks? Naming suggests they might not be even
>>>> part of this device. But if these are part of the device, then why this
>>>> is not a clock controller (if they are controllable) or even removed
>>>> (because we do not represent internal clock tree in DTS).
>>>
>>> xosc is a crystal connected to the oscillator input of the RP1, so I would
>>> consider it an external fixed-clock. If we were in the entire dts, I would have
>>> put it in root under /clocks node, but here we're in the dtbo so I'm not sure
>>> where else should I put it.
>>
>> But physically, on which PCB, where is this clock located?
>
> xosc is a crystal, feeding the reference clock oscillator input pins of the RP1,
> please see page 12 of the following document:
> https://datasheets.raspberrypi.com/rp1/rp1-peripherals.pdf
That's not the answer. Where is it physically located?
> On Rpi5, the PCB is the very same as the one on which the BCM2712 (SoC) and RP1
> are soldered. Would you consider it external (since the crystal is outside the RP1)
> or internal (since the oscillator feeded by the crystal is inside the RP1)?
So it is on RPi 5 board? Just like every other SoC and every other
vendor? Then just like every other SoC and every other vendor it is in
board DTS file.
>
>>
>>>
>>> Regarding pclk and hclk, I'm still trying to understand where they come from.
>>> If they are external clocks (since they are fixed-clock too), they should be
>>> in the same node as xosc. CLK_OF_DECLARE does not seem to fit here because
>>
>> There is no such node as "/clocks" so do not focus on that. That's just
>> placeholder but useless and it is inconsistent with other cases (e.g.
>> regulators).
>
> Fine, I beleve that the root node would be okay then, or some other carefully named
> node in root, if the clock is not internal to any chip.
>
>>
>> If this is external oscillator then it is not part of RP1 and you cannot
>> put it inside just to satisfy your drivers.
>
> Ack.
>
>>
>>> there's no special management of these clocks, so no new clock definition is
>>> needed.
>>
>>> If they are internal tree, I cannot simply get rid of them because rp1_eth node
>>> references these two clocks (see clocks property), so they must be decalred
>>> somewhere. Any hint about this?.
>>>
>>
>> Describe the hardware. Show the diagram or schematics where is which device.
>
> Unfortunately I don't have the documentation (schematics or other info) about
> how these two clocks (pclk and hclk) are arranged, but I'm trying to get
> some insight about that from various sources. While we're waiting for some
> (hopefully) more certain info, I'd like to speculate a bit. I would say that
> they both probably be either external (just like xosc), or generated internally
> to the RP1:
>
> If externals, I would place them in the same position as xosc, so root node
> or some other node under root (eg.: /rp1-clocks)
Just like /clocks, /rp1-clocks is not better. Neither /rp1-foo-clocks.
I think there is some sort of big misunderstanding here. Is this RP1
co-processor on the RP board, connected over PCI to Broadcom SoC?
>
> If internals, I would leave them just where they are, i.e. inside the rp1 node
>
> Does it make sense?
No, because you do not have xosc there, according to my knowledge.
Best regards,
Krzysztof
Powered by blists - more mailing lists