netdev - RE: [PATCH net-next 1/3] dt-bindings: net: emaclite: Add clock support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <MN0PR12MB59539E54E8BD46575FEC01B2B7772@MN0PR12MB5953.namprd12.prod.outlook.com>
Date: Tue, 1 Oct 2024 19:06:12 +0000
From: "Pandey, Radhey Shyam" <radhey.shyam.pandey@....com>
To: Conor Dooley <conor@...nel.org>
CC: "davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "robh@...nel.org" <robh@...nel.org>,
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "conor+dt@...nel.org"
	<conor+dt@...nel.org>, "Simek, Michal" <michal.simek@....com>, "Joseph, Abin"
	<Abin.Joseph@....com>, "u.kleine-koenig@...gutronix.de"
	<u.kleine-koenig@...gutronix.de>, "elfring@...rs.sourceforge.net"
	<elfring@...rs.sourceforge.net>, "Katakam, Harini" <harini.katakam@....com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "git (AMD-Xilinx)" <git@....com>
Subject: RE: [PATCH net-next 1/3] dt-bindings: net: emaclite: Add clock
 support

> -----Original Message-----
> From: Conor Dooley <conor@...nel.org>
> Sent: Tuesday, October 1, 2024 10:22 PM
> To: Pandey, Radhey Shyam <radhey.shyam.pandey@....com>
> Cc: davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org;
> pabeni@...hat.com; robh@...nel.org; krzk+dt@...nel.org; conor+dt@...nel.org;
> Simek, Michal <michal.simek@....com>; Joseph, Abin <Abin.Joseph@....com>;
> u.kleine-koenig@...gutronix.de; elfring@...rs.sourceforge.net; Katakam, Harini
> <harini.katakam@....com>; netdev@...r.kernel.org; devicetree@...r.kernel.org;
> linux-kernel@...r.kernel.org; linux-arm-kernel@...ts.infradead.org; git (AMD-Xilinx)
> <git@....com>
> Subject: Re: [PATCH net-next 1/3] dt-bindings: net: emaclite: Add clock support
> 
> On Tue, Oct 01, 2024 at 01:25:36AM +0530, Radhey Shyam Pandey wrote:
> > From: Abin Joseph <abin.joseph@....com>
> >
> > Add s_axi_aclk AXI4 clock support and make clk optional to keep DTB
> > backward compatibility. Define max supported clock constraints.
> 
> Why was the clock not provided before, but is now?
> Was it automatically enabled by firmware and that is no longer done?
> I'm suspicious of the clock being made optional, but the driver doing nothing other
> than enable it. That reeks of actually being required to me.

Traditionally these IP were used on microblaze platforms which had fixed
clocks enabled all the time. Since AXI Ethernet Lite is a PL IP, it can also
be used on SoC platforms like Zynq UltraScale+ MPSoC which combines 
processing system (PS) and user-programmable logic (PL) into the same 
device. On these platforms instead of fixed enabled clocks it is mandatory
to explicitly enable IP clocks for proper functionality. 

It gets more interesting when the PL clock is shared between two IPs 
and one of the drivers is clock adopted and disable the clocks after use 
and clock framework does not know about other clock users (emaclite 
IP using clock) and it will turn off the clocks which would lead to 
hang on emaclite reg access. So, it is needed to correctly model the
clock consumers.

While browsing i found a similar usecase for GMII to RGMII PL IP.
Similar to dt-bindings: net: xilinx_gmii2rgmii: Add clock support[1]
[1]: https://lore.kernel.org/all/4ae4d926-73f0-4f30-9d83-908a92046829@kernel.org/

In this series - I noticed that Krzysztof suggested to:
Nope, just write the description as items in clocks, instead of
maxItems. And drop clock names, are not needed and are kind of obvious.

So something like the below would be fine?

+  clocks:
+    items:
+      - description: AXI4 clock.

> 
> >
> > Signed-off-by: Abin Joseph <abin.joseph@....com>
> > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@....com>
> > ---
> >  Documentation/devicetree/bindings/net/xlnx,emaclite.yaml | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml
> > b/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml
> > index 92d8ade988f6..8fcf0732d713 100644
> > --- a/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml
> > +++ b/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml
> > @@ -29,6 +29,9 @@ properties:
> >    interrupts:
> >      maxItems: 1
> >
> > +  clocks:
> > +    maxItems: 1
> > +
> >    phy-handle: true
> >
> >    local-mac-address: true
> > --
> > 2.34.1
> >