Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Manne, Nava kishore" <nava.kishore.manne@amd.com>
To:     Xu Yilun <yilun.xu@intel.com>
CC:     Nava kishore Manne <nava.manne@xilinx.com>,
        "mdf@kernel.org" <mdf@kernel.org>,
        "hao.wu@intel.com" <hao.wu@intel.com>,
        "trix@redhat.com" <trix@redhat.com>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "krzysztof.kozlowski+dt@linaro.org" 
        <krzysztof.kozlowski+dt@linaro.org>,
        "michal.simek@xilinx.com" <michal.simek@xilinx.com>,
        "linux-fpga@vger.kernel.org" <linux-fpga@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "git@xilinx.com" <git@xilinx.com>, Rob Herring <robh@kernel.org>
Subject: RE: [PATCH v2 2/3] fpga: region: Add fpga-region 'power-domains'
  property
Thread-Topic: [PATCH v2 2/3] fpga: region: Add fpga-region 'power-domains'
  property
Thread-Index: AQHYnNAAqZyFU+ID7UG/jL7udw2/r62xbBfA
Date:   Tue, 16 Aug 2022 10:22:28 +0000
Message-ID: <DM6PR12MB39935950EC1E2998075E0A1DCD6B9@DM6PR12MB3993.namprd12.prod.outlook.com>
References: <20220523134517.4056873-1-nava.manne@xilinx.com>
 <20220523134517.4056873-3-nava.manne@xilinx.com>
 <20220624162815.GA2142910@yilunxu-OptiPlex-7050>
 <DM6PR12MB39937A19AE147CD9105C8D52CD8C9@DM6PR12MB3993.namprd12.prod.outlook.com>
 <20220721065454.GA1689481@yilunxu-OptiPlex-7050>
In-Reply-To: <20220721065454.GA1689481@yilunxu-OptiPlex-7050>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?+lNM5JZ1vOftHPsCxc9/w3E9MJIWLa/TiK9wkPEQ918LhaHRzsGc/eivhGmn?=
 =?us-ascii?Q?fnhJehGQXFIWl/I9ZjIqM3Sh33ELcqOtZKi6n1R7vlQ7aUI1fIJsu3lXgqQx?=
 =?us-ascii?Q?T9eCDNadSIdgbllqcmM/j0yStT4Q0lhtknLeiSyXLzKljdnIYmYc0gjDA2bc?=
 =?us-ascii?Q?clMng5187MKLCcpKGb719XiCb9qI+REFhBvYRT8N7OSH2erMCU3jUmB7o4TM?=
 =?us-ascii?Q?6Hnm0EOm4cvU0CJhg00Cl8QLNuCJtrz3D9SLM9O202JrbkVh4Dm5ezlBKowu?=
 =?us-ascii?Q?E+rVoPD+kPQwBuF3RCNXJf0wmvPeZF2/vgY3HcjDvbXmta/tD2EGU25SIy5P?=
 =?us-ascii?Q?Jt8912mafr+CLmv5gnqFONc+tGqx9w5wkNOfvzQTtJ95y7oSfTlKEQHyQ6yw?=
 =?us-ascii?Q?qnicl1OoLdJMBiL2MaUJCI9blrAOcrMQiX2ORVoz7kOXeg01i2kLtWami/3O?=
 =?us-ascii?Q?kqyUFBMsUvaiyczDr6Sa3XpFaoaRqvpO40wfxPj7gfnalXw9cs/tr5UcE7ak?=
 =?us-ascii?Q?PoqocdkOIrVaR+l3Xy8gLoY/+8Qq1HFJEyjHuhYnNDE5prP8hoiaO7a/rnDb?=
 =?us-ascii?Q?yM5B4h0l5leAAcwX/zX6XTkHdRIMhXAHDNR/XDzt/faMI7aVsN4k/O5lkBVQ?=
 =?us-ascii?Q?PA17FO1nOXRrf0Esf21sGfGyl6HMNYPe0n4BVa8Firsv+Ia0KZs6oJ/SOez1?=
 =?us-ascii?Q?bFMy4dqQb85OheVePo2wFy5DPR9ZTpOlHvS6KERYXV9Src/YnlgMiVrv4LX1?=
 =?us-ascii?Q?6do09/AxItrfclMSLa0nqW15A6nwsfNoPKHf5M5JDPA8Bs8R/Yq1ri2n9q/j?=
 =?us-ascii?Q?KTWKfYifRvu+2B8FFrCx0JOuuUKrucjJVyj75nCxx9mTT3vrjU73zsb9bxCa?=
 =?us-ascii?Q?JjXXKdlyEpbpphobydqmQ0DsjcIopNJko4C7MJp4zpBAbim6l5jIPrxOu6Qb?=
 =?us-ascii?Q?dZYwgjF/xtM/kiakdjOvuGtJ5BScyDvTfSecQ03rDexyAtesNexmkw/Yh0UD?=
 =?us-ascii?Q?JA+DbAoLDhmrR7XpiWqUCYZXM/sAlkGztSqq8N6X2tNLTHxMrcU9ViQnB/rG?=
 =?us-ascii?Q?mH24Np9Xb0vMcdexSyrkixVBCdUzNcgPA4mcGTXzdCDqJGjnhpd4lMU5LmP5?=
 =?us-ascii?Q?e826Yk3LPoBP+YfwP3/wyzUfWM4Ly4y21N7SOURZz1U9pHkmvXhSycsi/XK+?=
 =?us-ascii?Q?NWMn6mtR3jrbgT2qzkbZJfVVGuM1imR82Z6plvAAdpTBiviiwLYTUVwdLMVn?=
 =?us-ascii?Q?2MJazLAcHNNRZxKlf43/6FJI0FZSxxua50fFO5vks9yhwjyxP3opnUyXxiKn?=
 =?us-ascii?Q?EEtWXEkZCaPXYJfMJ//qWOXmqfG0QMmxWb1k4vw2vaPjzR9WKPif+Q8RDPUn?=
 =?us-ascii?Q?c2qC2YpwLa8M2mjfB5A5+ZLPWHeZsFNbsd0yX4s8HvRXk1qNQh6XVxy1e5UK?=
 =?us-ascii?Q?rIFeH2mCMK910jjzdlfFnzwPZFcfQh+uUsb1jAIGEqvY0m7IZyqYAS3FI+2o?=
 =?us-ascii?Q?FijlBRCDJU7Tipgq7H/JyKv9nwp9rqv6OGfeeBK8H+E8MOHJIwZeZSvb6Yh6?=
 =?us-ascii?Q?FCHUjnK4YeHRz2uSDVM=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3993.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: eb7f1b6f-fd74-4ea5-257a-08da7f7138bc
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Aug 2022 10:22:28.5764
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: B1tsjRILtoZFgV4VAiOSg6iBBf45R0/SU29bIWNiUFpD90r3vSlqcjXomJHI93bW
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4340
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Yilun,

	Please find my response inline.

> -----Original Message-----
> From: Xu Yilun <yilun.xu@intel.com>
> Sent: Thursday, July 21, 2022 12:25 PM
> To: Manne, Nava kishore <nava.kishore.manne@amd.com>
> Cc: Nava kishore Manne <nava.manne@xilinx.com>; mdf@kernel.org;
> hao.wu@intel.com; trix@redhat.com; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; michal.simek@xilinx.com; linux-
> fpga@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> git@xilinx.com; Rob Herring <robh@kernel.org>
> Subject: Re: [PATCH v2 2/3] fpga: region: Add fpga-region 'power-domains'
> property
>=20
> On Mon, Jul 18, 2022 at 06:46:32AM +0000, Manne, Nava kishore wrote:
> > Hi Yilun,
> >
> > 	Please find my response inline.
> >
> > > -----Original Message-----
> > > From: Xu Yilun <yilun.xu@intel.com>
> > > Sent: Friday, June 24, 2022 9:58 PM
> > > To: Nava kishore Manne <nava.manne@xilinx.com>
> > > Cc: mdf@kernel.org; hao.wu@intel.com; trix@redhat.com;
> > > robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> > > michal.simek@xilinx.com; linux-fpga@vger.kernel.org;
> > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> > > kernel@lists.infradead.org; git@xilinx.com; Rob Herring
> > > <robh@kernel.org>
> > > Subject: Re: [PATCH v2 2/3] fpga: region: Add fpga-region 'power-
> domains'
> > > property
> > >
> > > CAUTION: This message has originated from an External Source. Please
> > > use proper judgment and caution when opening attachments, clicking
> > > links, or responding to this email.
> > >
> > >
> > > On Mon, May 23, 2022 at 07:15:16PM +0530, Nava kishore Manne wrote:
> > > > Add fpga-region 'power-domains' property to allow to handle the
> > > > FPGA/PL power domains.
> > > >
> > > > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com>
> > > > Acked-by: Rob Herring <robh@kernel.org>
> > > > ---
> > > > Changes for v2:
> > > >               - Updated power-domains description.
> > > >
> > > >  .../devicetree/bindings/fpga/fpga-region.txt       | 14 ++++++++++=
++++
> > > >  1 file changed, 14 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > > b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > > index 7d3515264838..f299c3749505 100644
> > > > --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > > +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> > > > @@ -196,6 +196,20 @@ Optional properties:
> > > >  - config-complete-timeout-us : The maximum time in microseconds
> > > > time
> > > for the
> > > >       FPGA to go to operating mode after the region has been
> programmed.
> > > >  - child nodes : devices in the FPGA after programming.
> > > > +- power-domains : A phandle and power domain specifier pair to
> > > > +the
> > > power domain
> > > > +     which is responsible for turning on/off the power to the
> > > > + FPGA/PL
> > > region.
> > >
> > > Could you help explain what is PL?
> > >
> > > > +Example:
> > > > +     fpga_full: fpga-full {
> > > > +                compatible =3D "fpga-region";
> > > > +                fpga-mgr =3D <&zynqmp_pcap>;
> > > > +                #address-cells =3D <2>;
> > > > +                #size-cells =3D <2>;
> > > > +                ranges;
> > > > +                power-domains =3D <&zynqmp_firmware PL_PD>;
> > > > +        };
> > > > +
> > > > +     The PL_PD power domain will be turned on before loading the
> > > > +bitstream and turned off while removing/unloading the bitstream
> > > > +using
> > > overlays.
> > >
> > > I think the single power-domain may not cover some use cases that
> > > of-fpga- region driver supports.
> >
> > I am not sure which use case you are talking about. Can you please poin=
t
> me the exact use case here?
> >
> > > It is possible there are already devices in fpga-region for static
> > > OF tree, or an overlay with no 'firmware-name' but 'external-fpga-
> > > config'. In these cases power domains may still be needed, is it?
> > >
> >
> > It's an optional property user can decide whether he needs this
> > support or not for 'external-fpga-config Use case.
>=20
> If an external-fpga-config FPGA region needs to enable a power domain
> before sub devices population, how could it config the DT? I assume in th=
is
> patch "power-domains" property is only used before & after reconfiguratio=
n
> but external-fpga-config FPGA region may need no reconfiguration.
>=20
I agree, For 'external-fpga-config' use case this patch is not capable of s=
upporting the dynamic PM.
I Will explore and get back to you to handle this use case.

> >
> > > Another case is the fpga-region may need multiple power domains?
> > >
> >
> > In our use case full region and relevant partial regions have
> > different power domains and this patch is capable of handle different
> > power domain regions (full and partial regions)
>=20
> If a FPGA region needs 2 or more power domains for partial reconfiguratio=
n,
> how could we find out and enable them all?
>=20
> From the 2 cases, I see as a generic driver, there may be need to enable
> different power domains at different moments. And I'm afraid a simple
> implementation of pm_runtime_get before reconfiguration may limit the
> usage of "power-domains" property for of-fpga-region.
>=20

Can you please point me any references to handle the multiple power domains=
?

This implementation is region specific which means it can handle both full =
and partial power domains
Independently based on the power domains info exists in the DT.
For example:

Base DT: For full
fpga_full: fpga-full {
                compatible =3D "fpga-region";
                fpga-mgr =3D <&zynqmp_pcap>;
                #address-cells =3D <2>;
                #size-cells =3D <2>;
                ranges;
                power-domains =3D <&zynqmp_firmware PL_PD>;
};

Overlay DT: For Partial
fragment@0 {
	target =3D <&fpga_full>;
	#address-cells =3D <2>;
	#size-cells =3D <2>;
	 __overlay__ {
		#address-cells =3D <2>;
		#size-cells =3D <2>;
		firmware-name =3D "full.bin";
		fpga_PR0: fpga-PR0 {
			compatible =3D "fpga-region";
			#address-cells =3D <2>;
			#size-cells =3D <2>;
			ranges;
			power-domains =3D <&zynqmp_firmware PL_PR_PD>;
		};
	 };
};

Regards,
Navakishore.