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

[Pandey, Radhey Shyam]

> -----Original Message-----
> From: Andrew Lunn <andrew@xxxxxxx>
> Sent: Monday, July 25, 2022 10:41 PM
> To: Pandey, Radhey Shyam <radhey.shyam.pandey@xxxxxxx>
> Cc: michal.simek@xxxxxxxxxx; nicolas.ferre@xxxxxxxxxxxxx;
> claudiu.beznea@xxxxxxxxxxxxx; davem@xxxxxxxxxxxxx;
> edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx;
> gregkh@xxxxxxxxxxxxxxxxxxx; ronak.jain@xxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> netdev@xxxxxxxxxxxxxxx; git@xxxxxxxxxx; git (AMD-Xilinx) <git@xxxxxxx>
> 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@xxxxxxx>
> > > Sent: Sunday, July 24, 2022 10:24 PM
> > > To: Pandey, Radhey Shyam <radhey.shyam.pandey@xxxxxxx>
> > > Cc: michal.simek@xxxxxxxxxx; nicolas.ferre@xxxxxxxxxxxxx;
> > > claudiu.beznea@xxxxxxxxxxxxx; davem@xxxxxxxxxxxxx;
> > > edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx;
> > > gregkh@xxxxxxxxxxxxxxxxxxx; ronak.jain@xxxxxxxxxx; linux-arm-
> > > kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > > netdev@xxxxxxxxxxxxxxx; git@xxxxxxxxxx; git (AMD-Xilinx)
> > > <git@xxxxxxx>
> > > 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