[PATCH net-next 2/2] net: macb: Add zynqmp SGMII dynamic configuration support

[Pandey, Radhey Shyam]

> -----Original Message-----
> From: Andrew Lunn <andrew at lunn.ch>
> Sent: Monday, July 25, 2022 10:41 PM
> To: Pandey, Radhey Shyam <radhey.shyam.pandey at amd.com>
> Cc: michal.simek at xilinx.com; nicolas.ferre at microchip.com;
> claudiu.beznea at microchip.com; davem at davemloft.net;
> edumazet at google.com; kuba at kernel.org; pabeni at redhat.com;
> gregkh at linuxfoundation.org; ronak.jain at xilinx.com; linux-arm-
> kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> netdev at vger.kernel.org; git at xilinx.com; git (AMD-Xilinx) <git at amd.com>
> Subject: Re: [PATCH net-next 2/2] net: macb: Add zynqmp SGMII dynamic
> configuration support
> 
> On Mon, Jul 25, 2022 at 02:34:51PM +0000, Pandey, Radhey Shyam wrote:
> > > -----Original Message-----
> > > From: Andrew Lunn <andrew at lunn.ch>
> > > Sent: Sunday, July 24, 2022 10:24 PM
> > > To: Pandey, Radhey Shyam <radhey.shyam.pandey at amd.com>
> > > Cc: michal.simek at xilinx.com; nicolas.ferre at microchip.com;
> > > claudiu.beznea at microchip.com; davem at davemloft.net;
> > > edumazet at google.com; kuba at kernel.org; pabeni at redhat.com;
> > > gregkh at linuxfoundation.org; ronak.jain at xilinx.com; linux-arm-
> > > kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> > > netdev at vger.kernel.org; git at xilinx.com; git (AMD-Xilinx)
> > > <git at amd.com>
> > > Subject: Re: [PATCH net-next 2/2] net: macb: Add zynqmp SGMII
> > > dynamic configuration support
> > >
> > > > +		ret = of_property_read_u32_array(pdev->dev.of_node,
> > > "power-domains",
> > > > +						 pm_info,
> > > ARRAY_SIZE(pm_info));
> > > > +		if (ret < 0) {
> > > > +			dev_err(&pdev->dev, "Failed to read power
> > > management information\n");
> > > > +			return ret;
> > > > +		}
> > > > +		ret = zynqmp_pm_set_gem_config(pm_info[1],
> > > GEM_CONFIG_FIXED, 0);
> > > > +		if (ret < 0)
> > > > +			return ret;
> > > > +
> > >
> > > Documentation/devicetree/bindings/net/cdns,macb.yaml says:
> > >
> > >   power-domains:
> > >     maxItems: 1
> > >
> > > Yet you are using pm_info[1]?
> >
> > >From power-domain description - It's a phandle and PM domain
> > specifier as defined by bindings of the power controller specified by
> > phandle.
> >
> > I assume the numbers of cells is specified by "#power-domain-cells":
> > Power-domain-cell is set to 1 in this case.
> >
> > arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> > #power-domain-cells = <1>;
> > power-domains = <&zynqmp_firmware PD_ETH_0>;
> >
> > Please let me know your thoughts.
> 
> Ah, so you ignore the phandle value, and just use the PD_ETH_0?
> 
> How robust is this? What if somebody specified a different power domain?

Some background - init_reset_optional() fn is implemented
for three platforms i.e., zynqmp, versal, MPFS.

zynqmp_pm_set_gem_config API expect first argument as GEM node id
so, power-domain DT property is passed to get node ID.

However, power-domain property is read only if underlying firmware 
supports configuration of GEM secure space. It's only true for 
zynqmp SGMII case and for zynqmp power domain is fixed.
In addition to it there is an error handling in power-domain 
property parsing. Hope this answers the question.

> 
> 	Andrew