Received: by 2002:a05:6a10:22f:0:0:0:0 with SMTP id 15csp1915649pxk; Tue, 1 Sep 2020 10:44:19 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz5mXFhxtn/z/2belN1yjeTwM9jfgq36byEly7S9UvoebowAyi7M1Ta1zmv7K0wUb6Rvw08 X-Received: by 2002:a05:6402:16:: with SMTP id d22mr2899135edu.175.1598982259306; Tue, 01 Sep 2020 10:44:19 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1598982259; cv=pass; d=google.com; s=arc-20160816; b=y69WtKyK8R0cFOp+/BsjknydwZB/zw4PH4S8DGlyOxad2VWH1mTqmwUhMVlTg/eIY1 HrB5CHfGMdInUyWeg21N/nWk9pZddazMHFfaKwVjs1EFNqZK8FPwHxsJqVJCEPk/LcQ+ 2Dt7sxD2RyR90ZZsdQesdW5R5espTIAujv4zgiE3+j8CHh63S4DIdEsArZHYx4eFXdB8 HzcBeAC/dL2mGVXPwQfv52JhlTE7BmmCw1fKKh/nY0OpXREfDYSUHW/4CkMFPNABCs6S Zehf6W0AXOg1gI6I3y2AinlRseVhZ9kg8mRCY+I4MABjba3HTl3sqhWcZV/IFyhAXObd BpLQ== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:mime-version:content-transfer-encoding :content-language:accept-language:in-reply-to:references:message-id :date:thread-index:thread-topic:subject:cc:to:from:dkim-signature; bh=E+NchDK4uk+eEi86hGe5cQf5v8qYAQFx6QPhieK9hRI=; b=qVCw+jwZLsJwv79A3CzMpcpnnN0o3DjNNak5FbyFI2WdRvhOOnUpfhbCUvEU24jI4g ATo5ret1RPO2i2aX8QLyoBGn5I5slnSySZJ2FzvhcSRL1eCnzjpL2X9PD6UiPX/DQLSG mlvWeIWrjZ2lNTuE6bD1zLrYRP1BDm8ESTSeOaLsogU7AYF8Vyb3cGjGiYvLn0XL3ORO t0ZggLbaRPpzbbQPGi7yx3e0KdoLFH6gSssEvcHIT2rVXUqjlzFz37oDFomatb/RgHNi br7T2QWdu8xAXO1rfBZjpob/k2Qce/I7k8V/vPooLolAVAFlwCL55uKhXD0SGkKbV83j /0SA== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@xilinx.onmicrosoft.com header.s=selector2-xilinx-onmicrosoft-com header.b=l23rL9FD; arc=pass (i=1 spf=pass spfdomain=xilinx.com dkim=pass dkdomain=xilinx.com dmarc=pass fromdomain=xilinx.com); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id dt2si1187888ejc.67.2020.09.01.10.43.55; Tue, 01 Sep 2020 10:44:19 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@xilinx.onmicrosoft.com header.s=selector2-xilinx-onmicrosoft-com header.b=l23rL9FD; arc=pass (i=1 spf=pass spfdomain=xilinx.com dkim=pass dkdomain=xilinx.com dmarc=pass fromdomain=xilinx.com); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728788AbgIARnQ (ORCPT + 99 others); Tue, 1 Sep 2020 13:43:16 -0400 Received: from mail-co1nam11on2065.outbound.protection.outlook.com ([40.107.220.65]:54496 "EHLO NAM11-CO1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726301AbgIARnO (ORCPT ); Tue, 1 Sep 2020 13:43:14 -0400 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=EgxF2TC4F292PLISKzO6M+xF+HKHX7ckXU1HxUuLk0cF5ad1vmWn4YC/I6nLsAGAToKkNE8Fj8P4XRR+2nj1nkBcEP8L63+3HQU5S7GdkMtZEm7S0e8huckxW0I5e5ocWvB73XgztzSPycealosMg4A00x0a7K4RKSp6mDX4hxHkYDKFVYG9c3XKfUF2OHKOY36wBXoZovCJCw1BjcdLp5SHf9NIrxw0ByHlU0XEFxalRbjS73zFWJlVmf/ySfxKqmPWsr7vKan+12+DtH/VNXksXMU55rF5te1bmb8vFBj1Ty2evLcbRuNjAdenFUnldtuSceFGBc7SRA/4Oa3DsQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=E+NchDK4uk+eEi86hGe5cQf5v8qYAQFx6QPhieK9hRI=; b=juX+yLNP+WK96H/XUCZP5va8XvJbA2lOETOUyQoR/eAKQ7Lqp+16xE/ir/YliAccvMfkYmTdeXFIok4n9YSLJIPPoRZ2ufuLsD3/SAmvTmJJRT/jqQapa0IO51wGfwdeldlfo5A7J0KqnxuKizUeJFVHNcPIHGYJ9wRZORMNspjSGthHvQr47SW/z2+S2ia2ROBDJCwYNCh+D0uXlbMeRAkRqJS+qI85IV+t50WQ/DSELmAOTWV7OKufmVlFOYZoR+TKaELIOBcD/JUY1nc0XEFgi0y2NgG3sYd+IghbYOaVW1RsBIPHChZn9NNexU4IGeqzenkPFuzkN5S6xmncWw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=xilinx.com; dmarc=pass action=none header.from=xilinx.com; dkim=pass header.d=xilinx.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=xilinx.onmicrosoft.com; s=selector2-xilinx-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=E+NchDK4uk+eEi86hGe5cQf5v8qYAQFx6QPhieK9hRI=; b=l23rL9FDwWNH8E3zhb56rl977RWCnkCKDcPh+vHlNsxZjCeQPUsACASSQsj8ZtXrmxos+cFRtAuBdc13D4B78MX8bNCSDEQRVsNew7a6JcEqL0hCeLfBZeacFVLqNPl6Vj/Rm6OUneGfBpEJBOtEVCNTQ5im47VvBWXKFFmEajM= Received: from BYAPR02MB4407.namprd02.prod.outlook.com (2603:10b6:a03:55::31) by BY5PR02MB6178.namprd02.prod.outlook.com (2603:10b6:a03:1fc::31) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3348.15; Tue, 1 Sep 2020 17:43:05 +0000 Received: from BYAPR02MB4407.namprd02.prod.outlook.com ([fe80::b0f6:b3a:6543:26f5]) by BYAPR02MB4407.namprd02.prod.outlook.com ([fe80::b0f6:b3a:6543:26f5%5]) with mapi id 15.20.3326.025; Tue, 1 Sep 2020 17:43:05 +0000 From: Ben Levinsky To: Michael Auchter CC: "devicetree@vger.kernel.org" , "linux-remoteproc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" Subject: RE: [PATCH v9 5/5] remoteproc: Add initial zynqmp R5 remoteproc driver Thread-Topic: [PATCH v9 5/5] remoteproc: Add initial zynqmp R5 remoteproc driver Thread-Index: AQHWe8QVOFVHYsWxu0aNDK2y8QHEVKlK6TOAgAkBH3CAACl58A== Date: Tue, 1 Sep 2020 17:43:04 +0000 Message-ID: References: <20200826161307.1064-1-ben.levinsky@xilinx.com> <20200826161307.1064-6-ben.levinsky@xilinx.com> <20200826213215.GA404249@xaphan> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply X-MS-TNEF-Correlator: authentication-results: ni.com; dkim=none (message not signed) header.d=none;ni.com; dmarc=none action=none header.from=xilinx.com; x-originating-ip: [149.199.62.133] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: fe6bc0b8-727f-4198-e037-08d84e9e7b19 x-ms-traffictypediagnostic: BY5PR02MB6178: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:3513; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: FRsSnyZlWnBNvmnJp/sgVyGG3ksD2Gpq1Vwe6675lWMLDe1LFuVVC2PcSria954nTZn6O8Uja64lSGJPnxG6xSCUwdzHTOZ3NkZVNS4xrB23FqFsRtvgLCQU45YA7cvF4PVtlhsag08UMZ6UUt+FnAJkXxW0l+93MD1IUuleBlZlay0/xd6OPGSmWYH+fVo+y6zwKRRZcSBuLiqHZi4ifu66ldTq7Elhk6y/30y799BAOLbzc8beJwr7OEbXfxnG8knQIRolJDfLpoz7Wgs10F0bXcpEb73rCuETsV/9mT+goskiptqvjcjkMb8P81NsXlZqg2Tu6xiG0m5igvT+DQ== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BYAPR02MB4407.namprd02.prod.outlook.com;PTR:;CAT:NONE;SFS:(376002)(136003)(366004)(396003)(39860400002)(346002)(7696005)(86362001)(64756008)(66556008)(76116006)(66476007)(2940100002)(66446008)(2906002)(4326008)(66946007)(30864003)(5660300002)(478600001)(71200400001)(33656002)(52536014)(6506007)(53546011)(26005)(9686003)(186003)(6916009)(55016002)(83380400001)(8936002)(54906003)(966005)(316002)(8676002)(559001)(579004);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata: P7136bnmT2w6gsDmbpDhcE5qa7c1J4CGr/vDdtu+msaezmCDLB5xE6+4oZylcK3U0K2tHL4LuJHH8Ev8Nu1CkeVlYnLPGb+VUxR/ZHsMT8Ua/NmDk7NrfeqVWh0B+PVgyO8BGkl3ZwIgSp+mt7AxCZSURm6Q/nNKIfr0Kmr7Q97C7krXAu6oV6KhfYSd6kHazdKJQCfaKc4aKRUhlya4A9QAIYxJKuoZhqK5+HDeXT7dZT22wkG0PTBiL6kJav79sVy0Ayme4A4638J8JLSyuN2eoWZFQowZsepIifOFi7djUM08u+i4o50+d3m9w5O98hkhnl0gXPYPcneDq8y0oni6NgzMJJPmmdJw/ZQgJfrM3fFAKi8PqgvfZQszxkB4BXyKJ1trulF+V/eEFMMR+pG/F5LmfbbuX/v3Bn8rgTshznGzZSmLjo0rn41SPiA8gAFt2Vdt6iaMRq5KE2Rkh/tbiJ2v6IwI0R+J80aS16j7rV4wSGIV1DUvT7Bol+L9bwytdYZFRUbx/3wWQMky82cGY1ABgfpgpYK6oW0itAorM9zp95Ocw3e22sW0+aX5Z3r+/dUBieEN/QePRvXJ6vQRaELIPblXV6m3ZFaecQwng3Po4M/ZVgYorhA8NpO2Ge227VRFvecVHPwvzzQZWQ== x-ms-exchange-transport-forked: True Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: xilinx.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: BYAPR02MB4407.namprd02.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: fe6bc0b8-727f-4198-e037-08d84e9e7b19 X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Sep 2020 17:43:04.7699 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 657af505-d5df-48d0-8300-c31994686c5c X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: T0CbLGhi6/R4u+LMnDRoU+namjrG33yLAA2m/A9A9ddTbztqgbtE2NTWnhdpYYWEiyRYIVANJfgbn/IuIeRWjA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR02MB6178 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Michael, I put some comments inline on reworking the zynqmp_pm* functions Best, Ben > -----Original Message----- > From: Ben Levinsky > Sent: Tuesday, September 1, 2020 8:10 AM > To: Michael Auchter > Cc: devicetree@vger.kernel.org; linux-remoteproc@vger.kernel.org; linux- > kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org > Subject: RE: [PATCH v9 5/5] remoteproc: Add initial zynqmp R5 remoteproc > driver >=20 > Hi Michael, >=20 >=20 > > -----Original Message----- > > From: Michael Auchter > > Sent: Wednesday, August 26, 2020 2:32 PM > > To: Ben Levinsky > > Cc: Stefano Stabellini ; Michal Simek > > ; devicetree@vger.kernel.org; > > mathieu.poirier@linaro.org; Ed T. Mooring ; linux- > > remoteproc@vger.kernel.org; linux-kernel@vger.kernel.org; Jiaying Liang > > ; robh+dt@kernel.org; linux-arm- > > kernel@lists.infradead.org > > Subject: Re: [PATCH v9 5/5] remoteproc: Add initial zynqmp R5 remotepro= c > > driver > > > > Hello Ben, > > > > On Wed, Aug 26, 2020 at 09:13:07AM -0700, Ben Levinsky wrote: > > > R5 is included in Xilinx Zynq UltraScale MPSoC so by adding this > > > remotproc driver, we can boot the R5 sub-system in different 2 > > > configurations: split or lock-step. > > > > > > The Xilinx R5 Remoteproc Driver boots the R5's via calls to the Xilin= x > > > Platform Management Unit that handles the R5 configuration, memory > > access > > > and R5 lifecycle management. The interface to this manager is done in= this > > > driver via zynqmp_pm_* function calls. > > > > > > Signed-off-by: Ben Levinsky > > > Signed-off-by: Wendy Liang > > > Signed-off-by: Michal Simek > > > Signed-off-by: Ed Mooring > > > Signed-off-by: Jason Wu > > > --- > > > v2: > > > - remove domain struct as per review from Mathieu > > > v3: > > > - add xilinx-related platform mgmt fn's instead of wrapping around > > > function pointer in xilinx eemi ops struct > > > v4: > > > - add default values for enums > > > - fix formatting as per checkpatch.pl --strict. Note that 1 warning = and 1 > > check > > > are still raised as each is due to fixing the warning results in t= hat > > > particular line going over 80 characters. > > > v5: > > > - parse_fw change from use of rproc_of_resm_mem_entry_init to > > > rproc_mem_entry_init and use of alloc/release > > > - var's of type zynqmp_r5_pdata all have same local variable name > > > - use dev_dbg instead of dev_info > > > v6: > > > - adding memory carveouts is handled much more similarly. All mem > > > carveouts are > > > now described in reserved memory as needed. That is, TCM nodes are > not > > > coupled to remoteproc anymore. This is reflected in the remoteproc= R5 > > > driver > > > and the device tree binding. > > > - remove mailbox from device tree binding as it is not necessary for= elf > > > loading > > > - use lockstep-mode property for configuring RPU > > > v7: > > > - remove unused headers > > > - change u32 *lockstep_mode -> u32 lockstep_mode; > > > - change device-tree binding "lockstep-mode" to xlnx,cluster-mode > > > - remove zynqmp_r5_mem_probe and loop to Probe R5 memory devices > at > > > remoteproc-probe time > > > - remove is_r5_mode_set from zynqmp rpu remote processor private > data > > > - do not error out if no mailbox is provided > > > - remove zynqmp_r5_remoteproc_probe call of platform_set_drvdata as > > > pdata is > > > handled in zynqmp_r5_remoteproc_remove > > > v8: > > > - remove old acks, reviewed-by's in commit message > > > v9: > > > - as mboxes are now optional, if pdata->tx_mc_skbs not initialized th= en > > > do not call skb_queue_empty > > > - update usage for zynqmp_pm_set_rpu_mode, > > zynqmp_pm_set_tcm_config and > > > zynqmp_pm_get_rpu_mode > > > - update 5/5 patch commit message to document supported > configurations > > > and how they are booted by the driver. > > > - remove copyrights other than SPDX from zynqmp_r5_remoteproc.c > > > - compilation warnings no longer raised > > > - remove unused includes from zynqmp_r5_remoteproc.c > > > - remove unused var autoboot from zynqmp_r5_remoteproc.c > > > - reorder zynqmp_r5_pdata fpr small mem savings due to alignment > > > - use of zynqmp_pm_set_tcm_config now does not have > > > output arg > > > - in tcm handling, unconditionally use &=3D 0x000fffff mask since all= nodes > > > in this fn are for tcm > > > - update comments for translating dma field in tcm handling to device > > > address > > > - update calls to rproc_mem_entry_init in parse_mem_regions so that > there > > > are only 2 cases for types of carveouts instead of 3 > > > - in parse_mem_regions, check if device tree node is null before usin= g it > > > - add example device tree nodes used in parse_mem_regions and tcm > > parsing > > > - add comment for vring id node length > > > - add check for string length so that vring id is at least min length > > > - move tcm nodes from reserved mem to instead own device tree nodes > > > and only use them if enabled in device tree > > > - add comment for explaining handling of rproc_elf_load_rsc_table > > > - remove obsolete check for "if (vqid < 0)" in zynqmp_r5_rproc_kick > > > - remove unused field mems in struct zynqmp_r5_pdata > > > - remove call to zynqmp_r5_mem_probe and the fn itself as tcm handlin= g > > > is done by zyqmp_r5_pm_request_tcm > > > - remove obsolete setting of dma_ops and parent device dma_mask > > > - remove obsolete use of of_dma_configure > > > - add comment for call to r5_set_mode fn > > > - make mbox usage optional and gracefully inform user via dev_dbg if = not > > > present > > > - change var lockstep_mode from u32* to u32 > > > --- > > > drivers/remoteproc/Kconfig | 10 + > > > drivers/remoteproc/Makefile | 1 + > > > drivers/remoteproc/zynqmp_r5_remoteproc.c | 898 > > ++++++++++++++++++++++ > > > 3 files changed, 909 insertions(+) > > > create mode 100644 drivers/remoteproc/zynqmp_r5_remoteproc.c > > > > > > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig > > > index c4d1731295eb..dd9ed45654e0 100644 > > > --- a/drivers/remoteproc/Kconfig > > > +++ b/drivers/remoteproc/Kconfig > > > @@ -249,6 +249,16 @@ config STM32_RPROC > > > > > > This can be either built-in or a loadable module. > > > > > > +config ZYNQMP_R5_REMOTEPROC > > > + tristate "ZynqMP_R5 remoteproc support" > > > + depends on ARM64 && PM && ARCH_ZYNQMP > > > + select RPMSG_VIRTIO > > > + select MAILBOX > > > + select ZYNQMP_IPI_MBOX > > > + help > > > + Say y or m here to support ZynqMP R5 remote processors via the > > remote > > > + processor framework. > > > + > > > endif # REMOTEPROC > > > > > > endmenu > > > diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefil= e > > > index e8b886e511f0..04d1c95d06d7 100644 > > > --- a/drivers/remoteproc/Makefile > > > +++ b/drivers/remoteproc/Makefile > > > @@ -28,5 +28,6 @@ obj-$(CONFIG_QCOM_WCNSS_PIL) +=3D > > qcom_wcnss_pil.o > > > qcom_wcnss_pil-y +=3D qcom_wcnss.o > > > qcom_wcnss_pil-y +=3D qcom_wcnss_iris.o > > > obj-$(CONFIG_ST_REMOTEPROC) +=3D st_remoteproc.o > > > +obj-$(CONFIG_ZYNQMP_R5_REMOTEPROC) +=3D zynqmp_r5_remoteproc.o > > > obj-$(CONFIG_ST_SLIM_REMOTEPROC) +=3D st_slim_rproc.o > > > obj-$(CONFIG_STM32_RPROC) +=3D stm32_rproc.o > > > diff --git a/drivers/remoteproc/zynqmp_r5_remoteproc.c > > b/drivers/remoteproc/zynqmp_r5_remoteproc.c > > > new file mode 100644 > > > index 000000000000..2fbcccfe538c > > > --- /dev/null > > > +++ b/drivers/remoteproc/zynqmp_r5_remoteproc.c > > > @@ -0,0 +1,898 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Zynq R5 Remote Processor driver > > > + * > > > + * Based on origin OMAP and Zynq Remote Processor driver > > > + * > > > + */ > > > + > > > +#include > > > > Compilation fails for me here, since there are typedefs used (e.g. u32) > > in xlnx-zynqmp.h that are missing. Likely was masked before due to > > include order. > > > > Seems like xlnx-zynqmp.h should include linux/types.h > > > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#include "remoteproc_internal.h" > > > + > > > +#define MAX_RPROCS 2 /* Support up to 2 RPU */ > > > +#define MAX_MEM_PNODES 4 /* Max power nodes for one RPU > memory > > instance */ > > > + > > > +#define DEFAULT_FIRMWARE_NAME "rproc-rpu-fw" > > > + > > > +/* PM proc states */ > > > +#define PM_PROC_STATE_ACTIVE 1U > > > + > > > +/* IPI buffer MAX length */ > > > +#define IPI_BUF_LEN_MAX 32U > > > +/* RX mailbox client buffer max length */ > > > +#define RX_MBOX_CLIENT_BUF_MAX (IPI_BUF_LEN_MAX + \ > > > + sizeof(struct zynqmp_ipi_message)) > > > + > > > +/** > > > + * struct zynqmp_r5_mem - zynqmp rpu memory data > > > + * @pnode_id: TCM power domain ids > > > + * @res: memory resource > > > + * @node: list node > > > + */ > > > +struct zynqmp_r5_mem { > > > + u32 pnode_id[MAX_MEM_PNODES]; > > > + struct resource res; > > > + struct list_head node; > > > +}; > > > + > > > +/** > > > + * struct zynqmp_r5_pdata - zynqmp rpu remote processor private data > > > + * @dev: device of RPU instance > > > + * @rproc: rproc handle > > > + * @pnode_id: RPU CPU power domain id > > > + * @is_r5_mode_set: indicate if r5 operation mode is set > > > + * @tx_mc: tx mailbox client > > > + * @rx_mc: rx mailbox client > > > + * @tx_chan: tx mailbox channel > > > + * @rx_chan: rx mailbox channel > > > + * @mbox_work: mbox_work for the RPU remoteproc > > > + * @tx_mc_skbs: socket buffers for tx mailbox client > > > + * @rx_mc_buf: rx mailbox client buffer to save the rx message > > > + */ > > > +struct zynqmp_r5_pdata { > > > + unsigned char rx_mc_buf[RX_MBOX_CLIENT_BUF_MAX]; > > > + struct mbox_client tx_mc; > > > + struct mbox_client rx_mc; > > > + struct work_struct mbox_work; > > > + struct sk_buff_head tx_mc_skbs; > > > + struct device dev; > > > + struct rproc *rproc; > > > + struct mbox_chan *tx_chan; > > > + struct mbox_chan *rx_chan; > > > + u32 pnode_id; > > > + bool is_r5_mode_set; > > > +}; > > > + > > > +/** > > > + * table of RPUs > > > + */ > > > +struct zynqmp_r5_pdata rpus[MAX_RPROCS]; > > > +/** > > > + * RPU core configuration > > > + */ > > > +enum rpu_oper_mode rpu_mode; > > > + > > > +/* > > > + * r5_set_mode - set RPU operation mode > > > + * @pdata: Remote processor private data > > > + * > > > + * set RPU oepration mode > > > + * > > > + * Return: 0 for success, negative value for failure > > > + */ > > > +static int r5_set_mode(struct zynqmp_r5_pdata *pdata) > > > +{ > > > + u32 val[PAYLOAD_ARG_CNT] =3D {0}, expect, tcm_mode; > > > + struct device *dev =3D &pdata->dev; > > > + int ret; > > > + > > > + expect =3D (u32)rpu_mode; > > > + ret =3D zynqmp_pm_get_rpu_mode(pdata->pnode_id, val); > > > + if (ret < 0) { > > > + dev_err(dev, "failed to get RPU oper mode.\n"); > > > + return ret; > > > + } > > > + if (val[0] =3D=3D expect) { > > > + dev_dbg(dev, "RPU mode matches: %x\n", val[0]); > > > > Can the zynqmp_pm_* functions be reworked to take the correct types fro= m > > the caller's perspective? All of the casting in this function is rather > > distracting. > > > > enum rpu_oper_mode cur_rpu_mode; > > > > ret =3D zynqmp_pm_get_rpu_mode(pdata->pnode_id, &cur_rpu_mode); > > // ... > > if (cur_rpu_mode =3D=3D rpu_mode) { > > // ... > > > > would be easier on the eyes. > > > [Ben Levinsky] yes will rework in next series. [Ben Levinsky] for reworking the zynqmp_pm* functions there are few things: 1. it still makes sense to have a return-by-reference variable here as the = zynqmp_pm* functions expect to have a payload u32* struct to set return val= ues in. In this case, keeping some ptr other than a simple enum is still re= quired 2. instead of calling zynqmp_pm* functions we could directly call the IOTCL= 's that are being wrapped in drivers/firmware/Xilinx/zynqmp.c that are call= ed here if you prefer. doing the payload ptr's allocation within drivers/firmware/Xilinx/zynqmp.c = might be a bit much because then we have to pass in the remoteproc driver's= device pointer. With the above in mind, would keeping the original impl. But casting payloa= d return values to enum's before doing comparisons in this function with th= em be OK? > > > + } else { > > > + ret =3D zynqmp_pm_set_rpu_mode(pdata->pnode_id, > > > + expect, 0); > > > > Is the last argument ever used? Can it perhaps be removed? > > > [Ben Levinsky] in this use case no. it was ported from another API elsewh= ere. > In Linux kernel case, can rework as you suggest in next series. > > > + if (ret < 0) { > > > + dev_err(dev, > > > + "failed to set RPU oper mode.\n"); > > > + return ret; > > > + } > > > + } > > > + > > > + tcm_mode =3D (expect =3D=3D (u32)PM_RPU_MODE_LOCKSTEP) ? > > > + PM_RPU_TCM_COMB : PM_RPU_TCM_SPLIT; > > > + ret =3D zynqmp_pm_set_tcm_config(pdata->pnode_id, tcm_mode, 0); > > > > Same question about the last argument. > > > [Ben Levinsky] same, will do. > > > + if (ret < 0) { > > > + dev_err(dev, "failed to config TCM to %x.\n", > > > + expect); > > > + return ret; > > > + } > > > + pdata->is_r5_mode_set =3D true; > > > + return 0; > > > +} > > > + > > > +/* > > > + * ZynqMP R5 remoteproc memory release function > > > + */ > > > +static int zynqmp_r5_mem_release(struct rproc *rproc, > > > + struct rproc_mem_entry *mem) > > > +{ > > > + struct zynqmp_r5_mem *priv; > > > + int i, ret; > > > + struct device *dev =3D &rproc->dev; > > > + > > > + priv =3D mem->priv; > > > + if (!priv) > > > + return 0; > > > + for (i =3D 0; i < MAX_MEM_PNODES; i++) { > > > + if (priv->pnode_id[i]) { > > > + dev_dbg(dev, "%s, pnode %d\n", > > > + __func__, priv->pnode_id[i]); > > > + ret =3D zynqmp_pm_release_node(priv->pnode_id[i]); > > > + if (ret < 0) { > > > + dev_err(dev, > > > + "failed to release power node: %u\n", > > > + priv->pnode_id[i]); > > > + return ret; > > > + } > > > + } else { > > > + break; > > > + } > > > + } > > > + return 0; > > > +} > > > + > > > +/* > > > + * ZynqMP R5 remoteproc operations > > > + */ > > > +static int zynqmp_r5_rproc_start(struct rproc *rproc) > > > +{ > > > + struct device *dev =3D rproc->dev.parent; > > > + struct zynqmp_r5_pdata *pdata =3D rproc->priv; > > > + enum rpu_boot_mem bootmem; > > > + int ret; > > > + > > > + if ((rproc->bootaddr & 0xF0000000) =3D=3D 0xF0000000) > > > + bootmem =3D PM_RPU_BOOTMEM_HIVEC; > > > + else > > > + bootmem =3D PM_RPU_BOOTMEM_LOVEC; > > > + dev_dbg(dev, "RPU boot from %s.", > > > + bootmem =3D=3D PM_RPU_BOOTMEM_HIVEC ? "OCM" : "TCM"); > > > + ret =3D zynqmp_pm_request_wake(pdata->pnode_id, 1, > > > + bootmem, > > ZYNQMP_PM_REQUEST_ACK_NO); > > > + if (ret < 0) { > > > + dev_err(dev, "failed to boot R5.\n"); > > > + return ret; > > > + } > > > + return 0; > > > +} > > > + > > > +static int zynqmp_r5_rproc_stop(struct rproc *rproc) > > > +{ > > > + struct zynqmp_r5_pdata *pdata =3D rproc->priv; > > > + int ret; > > > + > > > + ret =3D zynqmp_pm_force_pwrdwn(pdata->pnode_id, > > > + ZYNQMP_PM_REQUEST_ACK_BLOCKING); > > > + if (ret < 0) { > > > + dev_err(&pdata->dev, "failed to shutdown R5.\n"); > > > + return ret; > > > + } > > > + return 0; > > > +} > > > + > > > +static int zynqmp_r5_rproc_mem_alloc(struct rproc *rproc, > > > + struct rproc_mem_entry *mem) > > > +{ > > > + struct device *dev =3D rproc->dev.parent; > > > + void *va; > > > + > > > + dev_dbg(rproc->dev.parent, "map memory: %pa\n", &mem->dma); > > > + va =3D ioremap_wc(mem->dma, mem->len); > > > + if (IS_ERR_OR_NULL(va)) { > > > + dev_err(dev, "Unable to map memory region: %pa+%lx\n", > > > + &mem->dma, mem->len); > > > + return -ENOMEM; > > > + } > > > + > > > + /* Update memory entry va */ > > > + mem->va =3D va; > > > + > > > + return 0; > > > +} > > > + > > > +static int zynqmp_r5_rproc_mem_release(struct rproc *rproc, > > > + struct rproc_mem_entry *mem) > > > +{ > > > + dev_dbg(rproc->dev.parent, "unmap memory: %pa\n", &mem->dma); > > > + iounmap(mem->va); > > > + > > > + return 0; > > > +} > > > + > > > +static int parse_mem_regions(struct rproc *rproc) > > > +{ > > > + int num_mems, i; > > > + struct zynqmp_r5_pdata *pdata =3D rproc->priv; > > > + struct device *dev =3D &pdata->dev; > > > + struct device_node *np =3D dev->of_node; > > > + struct rproc_mem_entry *mem; > > > + > > > + num_mems =3D of_count_phandle_with_args(np, "memory-region", > > NULL); > > > + if (num_mems <=3D 0) > > > + return 0; > > > + for (i =3D 0; i < num_mems; i++) { > > > + struct device_node *node; > > > + struct reserved_mem *rmem; > > > + > > > + node =3D of_parse_phandle(np, "memory-region", i); > > > + if (!node) { > > > + dev_err(dev, "no memory-region specified\n"); > > > + return -EINVAL; > > > + } > > > + > > > + rmem =3D of_reserved_mem_lookup(node); > > > + if (!rmem) { > > > + dev_err(dev, "unable to acquire memory-region\n"); > > > + return -EINVAL; > > > + } > > > + > > > + if (strstr(node->name, "vdev0vring")) { > > > + int vring_id; > > > + char name[16]; > > > + > > > + /* > > > + * expecting form of "rpuXvdev0vringX as documented > > > + * in xilinx remoteproc device tree binding > > > + */ > > > + if (strlen(node->name) < 14) { > > > + dev_err(dev, "%s has to be at least 14 chars", > > > + node->name); > > > + return -EINVAL; > > > + } > > > + > > > + /* > > > + * can be 1 of multiple vring IDs per IPC channel > > > + * e.g. 'vdev0vring0' and 'vdev0vring1' > > > + */ > > > + vring_id =3D node->name[14] - '0'; > > > + snprintf(name, sizeof(name), "vdev0vring%d", > > vring_id); > > > + /* Register vring */ > > > + mem =3D rproc_mem_entry_init(dev, NULL, > > > + (dma_addr_t)rmem->base, > > > + rmem->size, rmem->base, > > > + > > zynqmp_r5_rproc_mem_alloc, > > > + > > zynqmp_r5_rproc_mem_release, > > > + name); > > > + dev_dbg(dev, "parsed %s at %llx\r\n", mem->name, > > > + mem->dma); > > > + } else { > > > + /* Register DMA region */ > > > + int (*alloc)(struct rproc *r, > > > + struct rproc_mem_entry *rme); > > > + int (*release)(struct rproc *r, > > > + struct rproc_mem_entry *rme); > > > + char name[20]; > > > + > > > + if (strstr(node->name, "vdev0buffer")) { > > > + alloc =3D NULL; > > > + release =3D NULL; > > > + strcpy(name, "vdev0buffer"); > > > + } else { > > > + alloc =3D zynqmp_r5_rproc_mem_alloc; > > > + release =3D zynqmp_r5_rproc_mem_release; > > > + strcpy(name, node->name); > > > + } > > > + > > > + mem =3D rproc_mem_entry_init(dev, NULL, > > > + (dma_addr_t)rmem->base, > > > + rmem->size, rmem->base, > > > + alloc, release, name); > > > + if (!mem) { > > > + dev_err(dev, "unable to initialize memory- > > region %s\n", > > > + node->name); > > > + return -ENOMEM; > > > + } > > > + dev_dbg(dev, "parsed %s at %llx\r\n", mem->name, > > > + mem->dma); > > > + } > > > + rproc_add_carveout(rproc, mem); > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +/* call Xilix Platform manager to request access to TCM bank */ > > > +static int zyqmp_r5_pm_request_tcm(struct device_node *tcm_node, > > > + struct device *dev) > > > +{ > > > + u32 pnode_id; > > > + int ret; > > > + > > > + ret =3D of_property_read_u32(tcm_node, "pnode-id", &pnode_id); > > > + if (ret) { > > > + dev_err(dev, "Can't parse pnode-id property\n"); > > > + return ret; > > > + } > > > + > > > + ret =3D zynqmp_pm_request_node(pnode_id, > > ZYNQMP_PM_CAPABILITY_ACCESS, 0, > > > + ZYNQMP_PM_REQUEST_ACK_BLOCKING); > > > + if (ret < 0) { > > > + dev_err(dev, "failed to request power node: %u\n", > > pnode_id); > > > + return ret; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +/* Given tcm bank entry, > > > + * this callback will set device address for R5 running on TCM > > > + * and also setup virtual address for tcm bank remoteproc carveout > > > + */ > > > +static int tcm_mem_alloc(struct rproc *rproc, > > > + struct rproc_mem_entry *mem) > > > +{ > > > + void *va; > > > + struct device *dev =3D rproc->dev.parent; > > > + > > > + dev_dbg(rproc->dev.parent, "map memory: %pa\n", &mem->dma); > > > + va =3D ioremap_wc(mem->dma, mem->len); > > > + if (IS_ERR_OR_NULL(va)) { > > > + dev_err(dev, "Unable to map memory region: %pa+%lx\n", > > > + &mem->dma, mem->len); > > > + return -ENOMEM; > > > + } > > > + > > > + /* Update memory entry va */ > > > + mem->va =3D va; > > > + > > > + va =3D devm_ioremap_wc(dev, mem->da, mem->len); > > > + if (!va) > > > + return -ENOMEM; > > > + /* As R5 is 32 bit, wipe out extra high bits */ > > > + mem->da &=3D 0x000fffff; > > > + /* > > > + * handle tcm banks 1 a and b (0xffe90000 and oxffeb0000) > > > + * As both of these the only common bit found not in tcm bank0 a or > > b > > > + * is at 0x80000 use this mask to suss it out > > > + */ > > > + if (mem->da & 0x80000) > > > + /* > > > + * similar to wiping tcm banks 0a and 0b with just the > > > + * mask of 0x000fffff will translate to device addr's > > > + * at 0x0 and 0x20000, need to do more to further translate > > > + * tcm banks 1a and 1b at 0xffe90000 and oxffeb0000 > > > + * respectively to 0x0 and 0x20000 > > > + */ > > > + mem->da -=3D 0x90000; > > > + > > > + return 0; > > > +} > > > + > > > +/* > > > + * Given R5 node in remoteproc instance, > > > + * allocate remoteproc carveout for TCM memory > > > + * needed for firmware to be loaded > > > + */ > > > +static int parse_tcm_banks(struct rproc *rproc) > > > +{ > > > + int ret; > > > + struct device_node *dt_node; > > > + int tcm_bank_idx, tcm_bank_done; > > > + char tcm_str[10]; > > > + struct resource rsc; > > > + resource_size_t size; > > > + struct rproc_mem_entry *mem; > > > + > > > + struct zynqmp_r5_pdata *pdata =3D rproc->priv; > > > + struct device *dev =3D &pdata->dev; > > > + > > > + /* set start and stop for loop through tcm banks. */ > > > + if (rpu_mode =3D=3D PM_RPU_MODE_SPLIT) { > > > + /* > > > + * In split mode, R50 can use TCM banks > > > + * 0a, 0b and R51 can use 1a, 1b > > > + * > > > + * indices relate to the power node id that will > > > + * then be translated by Xilinx Platform Manager > > > + */ > > > + tcm_bank_idx =3D (pdata->pnode_id =3D=3D 0x7) ? 0 : 2; > > > + tcm_bank_done =3D tcm_bank_idx + 2; > > > + } else { > > > + /* if RPU in lockstep, all banks are available */ > > > + tcm_bank_idx =3D 0; > > > + tcm_bank_done =3D tcm_bank_idx + 4; > > > + } > > > + > > > + /* process TCM bank nodes in device tree */ > > > + for (; tcm_bank_idx < tcm_bank_done ; tcm_bank_idx++) { > > > + /* construct tcm bank name */ > > > + strcpy(tcm_str, "tcm_"); > > > + strcat(tcm_str, (tcm_bank_idx <=3D 0x10) ? "0" : "1"); > > > > tcm_bank_idx <=3D 0x10? Seems like this should be tcm_bank_idx <=3D 1 > instead... > > > > > + strcat(tcm_str, (tcm_bank_idx & 0x1) ? "b" : "a"); > > > > Instead of all this string manipulation at runtime, wouldn't a static > > table be more straightforward? e.g. > > > > static const char tcm_banks[] =3D { > > "tcm_0a", "tcm_0b", "tcm_1a", "tcm_1b" > > }; > > > [Ben Levinsky] makes sense. Will do so. > > > + > > > + dt_node =3D of_find_node_by_name(NULL, tcm_str); > > > > It seems strange to require specific node names, and perhaps > > contradicts: https://devicetree- > > specification.readthedocs.io/en/latest/devicetree-basics.html#generic- > names- > > recommendation > > > > Should the names instead be specified at the reference using > > memory-region-names, as suggested in the reserved-memory binding? > > > [Ben Levinsky] Good catch. This was actually how it was done in a previou= s > implementation. Issue with that is > TCM is not regular RAM so does not belong in reserved-mem as a carveout. >=20 > > > + if (dt_node && of_device_is_available(dt_node)) { > > > + dev_dbg(dev, "node %s is available\n", tcm_str); > > > + > > > + /* get address */ > > > + ret =3D of_address_to_resource(dt_node, 0, &rsc); > > > + if (ret < 0) { > > > + dev_err(dev, "fail to get rsc mem %s", > > > + of_node_full_name(dt_node)); > > > > %pOF can be used instead of of_node_full_name() > > > [Ben Levinsky] will do for this and subsequent usages as you pointed out. > > > + return -EINVAL; > > > + } > > > + > > > + /* request access */ > > > + zyqmp_r5_pm_request_tcm(dt_node, dev); > > > + > > > + /* add carveout */ > > > + size =3D resource_size(&rsc); > > > + mem =3D rproc_mem_entry_init(dev, NULL, rsc.start, > > > + (int)size, rsc.start, > > > + tcm_mem_alloc, > > > + zynqmp_r5_mem_release, > > > + rsc.name); > > > + if (!mem) > > > + return -ENOMEM; > > > + > > > + rproc_add_carveout(rproc, mem); > > > + } else { > > > + dev_info(dev, "node %s is not available\n", tcm_str); > > > + } > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +static int zynqmp_r5_parse_fw(struct rproc *rproc, const struct firm= ware > > *fw) > > > +{ > > > + int ret; > > > + struct zynqmp_r5_pdata *pdata =3D rproc->priv; > > > + struct device *dev =3D &pdata->dev; > > > + > > > + ret =3D parse_tcm_banks(rproc); > > > + if (ret) { > > > + dev_err(dev, "parse_tcm_banks failed %x\n", ret); > > > + return ret; > > > + } > > > + > > > + ret =3D parse_mem_regions(rproc); > > > + if (ret) { > > > + dev_err(dev, "parse_mem_regions failed %x\n", ret); > > > + return ret; > > > + } > > > + > > > + ret =3D rproc_elf_load_rsc_table(rproc, fw); > > > + if (ret =3D=3D -EINVAL) { > > > + /* > > > + * resource table only required for IPC. > > > + * if not present, this is not necessarily an error; > > > + * for example, loading r5 hello world application > > > + * so simply inform user and keep going. > > > + */ > > > + dev_info(dev, "no resource table found.\n"); > > > + ret =3D 0; > > > + } > > > + return ret; > > > +} > > > + > > > +/* kick a firmware */ > > > +static void zynqmp_r5_rproc_kick(struct rproc *rproc, int vqid) > > > +{ > > > + struct sk_buff *skb; > > > + unsigned int skb_len; > > > + struct zynqmp_ipi_message *mb_msg; > > > + int ret; > > > + > > > + struct device *dev =3D rproc->dev.parent; > > > + struct zynqmp_r5_pdata *pdata =3D rproc->priv; > > > + > > > + dev_dbg(dev, "KICK Firmware to start send messages vqid %d\n", > > vqid); > > > + > > > + skb_len =3D (unsigned int)(sizeof(vqid) + sizeof(mb_msg)); > > > + skb =3D alloc_skb(skb_len, GFP_ATOMIC); > > > + if (!skb) { > > > + dev_err(dev, > > > + "Failed to allocate skb to kick remote.\n"); > > > + return; > > > + } > > > + mb_msg =3D (struct zynqmp_ipi_message *)skb_put(skb, skb_len); > > > + mb_msg->len =3D sizeof(vqid); > > > + memcpy(mb_msg->data, &vqid, sizeof(vqid)); > > > + skb_queue_tail(&pdata->tx_mc_skbs, skb); > > > + ret =3D mbox_send_message(pdata->tx_chan, mb_msg); > > > + if (ret < 0) { > > > + dev_warn(dev, "Failed to kick remote.\n"); > > > + skb_dequeue_tail(&pdata->tx_mc_skbs); > > > + kfree_skb(skb); > > > + } > > > +} > > > + > > > +static struct rproc_ops zynqmp_r5_rproc_ops =3D { > > > + .start =3D zynqmp_r5_rproc_start, > > > + .stop =3D zynqmp_r5_rproc_stop, > > > + .load =3D rproc_elf_load_segments, > > > + .parse_fw =3D zynqmp_r5_parse_fw, > > > + .find_loaded_rsc_table =3D rproc_elf_find_loaded_rsc_table, > > > + .sanity_check =3D rproc_elf_sanity_check, > > > + .get_boot_addr =3D rproc_elf_get_boot_addr, > > > + .kick =3D zynqmp_r5_rproc_kick, > > > +}; > > > + > > > +/** > > > + * zynqmp_r5_release() - ZynqMP R5 device release function > > > + * @dev: pointer to the device struct of ZynqMP R5 > > > + * > > > + * Function to release ZynqMP R5 device. > > > + */ > > > +static void zynqmp_r5_release(struct device *dev) > > > +{ > > > + struct zynqmp_r5_pdata *pdata; > > > + struct rproc *rproc; > > > + struct sk_buff *skb; > > > + > > > + pdata =3D dev_get_drvdata(dev); > > > + rproc =3D pdata->rproc; > > > + if (rproc) { > > > + rproc_del(rproc); > > > + rproc_free(rproc); > > > + } > > > + if (pdata->tx_chan) > > > + mbox_free_channel(pdata->tx_chan); > > > + if (pdata->rx_chan) > > > + mbox_free_channel(pdata->rx_chan); > > > + > > > + /* Discard all SKBs if tx_mc_skbs is initialized */ > > > + if (&pdata->tx_mc_skbs.prev) { > > > + while (!skb_queue_empty(&pdata->tx_mc_skbs)) { > > > + skb =3D skb_dequeue(&pdata->tx_mc_skbs); > > > + kfree_skb(skb); > > > + } > > > + } > > > + > > > + put_device(dev->parent); > > > +} > > > + > > > +/** > > > + * event_notified_idr_cb() - event notified idr callback > > > + * @id: idr id > > > + * @ptr: pointer to idr private data > > > + * @data: data passed to idr_for_each callback > > > + * > > > + * Pass notification to remoteproc virtio > > > + * > > > + * Return: 0. having return is to satisfy the idr_for_each() functio= n > > > + * pointer input argument requirement. > > > + **/ > > > +static int event_notified_idr_cb(int id, void *ptr, void *data) > > > +{ > > > + struct rproc *rproc =3D data; > > > + > > > + (void)rproc_vq_interrupt(rproc, id); > > > + return 0; > > > +} > > > + > > > +/** > > > + * handle_event_notified() - remoteproc notification work funciton > > > + * @work: pointer to the work structure > > > + * > > > + * It checks each registered remoteproc notify IDs. > > > + */ > > > +static void handle_event_notified(struct work_struct *work) > > > +{ > > > + struct rproc *rproc; > > > + struct zynqmp_r5_pdata *pdata; > > > + > > > + pdata =3D container_of(work, struct zynqmp_r5_pdata, mbox_work); > > > + > > > + (void)mbox_send_message(pdata->rx_chan, NULL); > > > + rproc =3D pdata->rproc; > > > + /* > > > + * We only use IPI for interrupt. The firmware side may or may > > > + * not write the notifyid when it trigger IPI. > > > + * And thus, we scan through all the registered notifyids. > > > + */ > > > + idr_for_each(&rproc->notifyids, event_notified_idr_cb, rproc); > > > +} > > > + > > > +/** > > > + * zynqmp_r5_mb_rx_cb() - Receive channel mailbox callback > > > + * @cl: mailbox client > > > + * @mssg: message pointer > > > + * > > > + * It will schedule the R5 notification work. > > > + */ > > > +static void zynqmp_r5_mb_rx_cb(struct mbox_client *cl, void *mssg) > > > +{ > > > + struct zynqmp_r5_pdata *pdata; > > > + > > > + pdata =3D container_of(cl, struct zynqmp_r5_pdata, rx_mc); > > > + if (mssg) { > > > + struct zynqmp_ipi_message *ipi_msg, *buf_msg; > > > + size_t len; > > > + > > > + ipi_msg =3D (struct zynqmp_ipi_message *)mssg; > > > + buf_msg =3D (struct zynqmp_ipi_message *)pdata->rx_mc_buf; > > > + len =3D (ipi_msg->len >=3D IPI_BUF_LEN_MAX) ? > > > + IPI_BUF_LEN_MAX : ipi_msg->len; > > > + buf_msg->len =3D len; > > > + memcpy(buf_msg->data, ipi_msg->data, len); > > > + } > > > + schedule_work(&pdata->mbox_work); > > > +} > > > + > > > +/** > > > + * zynqmp_r5_mb_tx_done() - Request has been sent to the remote > > > + * @cl: mailbox client > > > + * @mssg: pointer to the message which has been sent > > > + * @r: status of last TX - OK or error > > > + * > > > + * It will be called by the mailbox framework when the last TX has d= one. > > > + */ > > > +static void zynqmp_r5_mb_tx_done(struct mbox_client *cl, void *mssg, > int > > r) > > > +{ > > > + struct zynqmp_r5_pdata *pdata; > > > + struct sk_buff *skb; > > > + > > > + if (!mssg) > > > + return; > > > + pdata =3D container_of(cl, struct zynqmp_r5_pdata, tx_mc); > > > + skb =3D skb_dequeue(&pdata->tx_mc_skbs); > > > + kfree_skb(skb); > > > +} > > > + > > > +/** > > > + * zynqmp_r5_setup_mbox() - Setup mailboxes > > > + * > > > + * @pdata: pointer to the ZynqMP R5 processor platform data > > > + * @node: pointer of the device node > > > + * > > > + * Function to setup mailboxes to talk to RPU. > > > + * > > > + * Return: 0 for success, negative value for failure. > > > + */ > > > +static int zynqmp_r5_setup_mbox(struct zynqmp_r5_pdata *pdata, > > > + struct device_node *node) > > > +{ > > > + struct device *dev =3D &pdata->dev; > > > + struct mbox_client *mclient; > > > + > > > + /* Setup TX mailbox channel client */ > > > + mclient =3D &pdata->tx_mc; > > > + mclient->dev =3D dev; > > > + mclient->rx_callback =3D NULL; > > > + mclient->tx_block =3D false; > > > + mclient->knows_txdone =3D false; > > > + mclient->tx_done =3D zynqmp_r5_mb_tx_done; > > > + > > > + /* Setup TX mailbox channel client */ > > > + mclient =3D &pdata->rx_mc; > > > + mclient->dev =3D dev; > > > + mclient->rx_callback =3D zynqmp_r5_mb_rx_cb; > > > + mclient->tx_block =3D false; > > > + mclient->knows_txdone =3D false; > > > + > > > + INIT_WORK(&pdata->mbox_work, handle_event_notified); > > > + > > > + /* Request TX and RX channels */ > > > + pdata->tx_chan =3D mbox_request_channel_byname(&pdata->tx_mc, > > "tx"); > > > + if (IS_ERR(pdata->tx_chan)) { > > > + dev_err(dev, "failed to request mbox tx channel.\n"); > > > + pdata->tx_chan =3D NULL; > > > + return -EINVAL; > > > + } > > > + pdata->rx_chan =3D mbox_request_channel_byname(&pdata->rx_mc, > > "rx"); > > > + if (IS_ERR(pdata->rx_chan)) { > > > + dev_err(dev, "failed to request mbox rx channel.\n"); > > > + pdata->rx_chan =3D NULL; > > > + return -EINVAL; > > > + } > > > + skb_queue_head_init(&pdata->tx_mc_skbs); > > > + > > > + return 0; > > > +} > > > + > > > +/** > > > + * zynqmp_r5_probe() - Probes ZynqMP R5 processor device node > > > + * @pdata: pointer to the ZynqMP R5 processor platform data > > > + * @pdev: parent RPU domain platform device > > > + * @node: pointer of the device node > > > + * > > > + * Function to retrieve the information of the ZynqMP R5 device node= . > > > + * > > > + * Return: 0 for success, negative value for failure. > > > + */ > > > +static int zynqmp_r5_probe(struct zynqmp_r5_pdata *pdata, > > > + struct platform_device *pdev, > > > + struct device_node *node) > > > +{ > > > + struct device *dev =3D &pdata->dev; > > > + struct rproc *rproc; > > > + int ret; > > > + > > > + /* Create device for ZynqMP R5 device */ > > > + dev->parent =3D &pdev->dev; > > > + dev->release =3D zynqmp_r5_release; > > > + dev->of_node =3D node; > > > + dev_set_name(dev, "%s", of_node_full_name(node)); > > > > %pOF can be used instead of of_node_full_name() > > > > > + dev_set_drvdata(dev, pdata); > > > + ret =3D device_register(dev); > > > + if (ret) { > > > + dev_err(dev, "failed to register device.\n"); > > > + return ret; > > > + } > > > + get_device(&pdev->dev); > > > + > > > + /* Allocate remoteproc instance */ > > > + rproc =3D rproc_alloc(dev, dev_name(dev), &zynqmp_r5_rproc_ops, > > NULL, 0); > > > + if (!rproc) { > > > + dev_err(dev, "rproc allocation failed.\n"); > > > + ret =3D -ENOMEM; > > > + goto error; > > > + } > > > + pdata->rproc =3D rproc; > > > + rproc->priv =3D pdata; > > > + > > > + /* Set up DMA mask */ > > > + ret =3D dma_set_coherent_mask(dev, DMA_BIT_MASK(32)); > > > + if (ret) { > > > + dev_err(&pdev->dev, "dma_set_coherent_mask: %d\n", ret); > > > + return ret; > > > + } > > > + > > > + /* Get R5 power domain node */ > > > + ret =3D of_property_read_u32(node, "pnode-id", &pdata->pnode_id); > > > + if (ret) { > > > + dev_err(dev, "failed to get power node id.\n"); > > > + goto error; > > > + } > > > + > > > + /* TODO Check if R5 is running */ > > > + > > > + /* > > > + * Set up R5 if not already setup > > > + * This check is needed as there are cases where > > > + * a user might repeatedly do modprobe > > > + * and modprobe -r. In this case, upon a > > > + * subsequent load, this check is needed. > > > + */ > > > + ret =3D pdata->is_r5_mode_set ? 0 : r5_set_mode(pdata); > > > + if (ret) { > > > + dev_err(dev, "failed to set R5 operation mode.\n"); > > > + return ret; > > > + } > > > + > > > + if (!of_get_property(dev->of_node, "mboxes", NULL)) { > > > + dev_dbg(dev, "no mailboxes.\n"); > > > + } else { > > > + ret =3D zynqmp_r5_setup_mbox(pdata, node); > > > + if (ret < 0) > > > + goto error; > > > + } > > > + > > > + /* Add R5 remoteproc */ > > > + ret =3D rproc_add(rproc); > > > + if (ret) { > > > + dev_err(dev, "rproc registration failed\n"); > > > + goto error; > > > + } > > > + return 0; > > > +error: > > > + if (pdata->rproc) > > > + rproc_free(pdata->rproc); > > > + pdata->rproc =3D NULL; > > > + device_unregister(dev); > > > + put_device(&pdev->dev); > > > + return ret; > > > +} > > > + > > > +static int zynqmp_r5_remoteproc_probe(struct platform_device *pdev) > > > +{ > > > + int ret, i =3D 0; > > > + u32 lockstep_mode; > > > + struct device *dev =3D &pdev->dev; > > > + struct device_node *nc; > > > + struct zynqmp_r5_pdata *pdata; > > > + > > > + pdata =3D devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > > > + if (!pdata) > > > + return -ENOMEM; > > > + > > > + platform_set_drvdata(pdev, pdata); > > > + > > > + of_property_read_u32(dev->of_node, "lockstep-mode", > > &lockstep_mode); > > > + > > > + if (!lockstep_mode) { > > > + rpu_mode =3D PM_RPU_MODE_SPLIT; > > > + } else if (lockstep_mode =3D=3D 1) { > > > + rpu_mode =3D PM_RPU_MODE_LOCKSTEP; > > > + } else { > > > + dev_err(dev, > > > + "Invalid lockstep-mode mode provided - %x %d\n", > > > + lockstep_mode, rpu_mode); > > > + return -EINVAL; > > > + } > > > + dev_dbg(dev, "RPU configuration: %s\r\n", > > > > \n should be sufficient > > > [Ben Levinsky] will do > > > + lockstep_mode ? "lockstep" : "split"); > > > + > > > + for_each_available_child_of_node(dev->of_node, nc) { > > > + ret =3D zynqmp_r5_probe(&rpus[i], pdev, nc); > > > + if (ret) { > > > + dev_err(dev, "failed to probe rpu %s.\n", > > > + of_node_full_name(nc)); > > > > %pOF can be used instead of of_node_full_name() > > > > > + return ret; > > > + } > > > + i++; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +static int zynqmp_r5_remoteproc_remove(struct platform_device *pdev) > > > +{ > > > + int i; > > > + > > > + for (i =3D 0; i < MAX_RPROCS; i++) { > > > + struct zynqmp_r5_pdata *pdata =3D &rpus[i]; > > > + struct rproc *rproc; > > > + > > > + rproc =3D pdata->rproc; > > > + if (rproc) { > > > + rproc_del(rproc); > > > + rproc_free(rproc); > > > + pdata->rproc =3D NULL; > > > + } > > > + if (pdata->tx_chan) { > > > + mbox_free_channel(pdata->tx_chan); > > > + pdata->tx_chan =3D NULL; > > > + } > > > + if (pdata->rx_chan) { > > > + mbox_free_channel(pdata->rx_chan); > > > + pdata->rx_chan =3D NULL; > > > + } > > > + > > > + device_unregister(&pdata->dev); > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +/* Match table for OF platform binding */ > > > +static const struct of_device_id zynqmp_r5_remoteproc_match[] =3D { > > > + { .compatible =3D "xlnx,zynqmp-r5-remoteproc-1.0", }, > > > + { /* end of list */ }, > > > +}; > > > +MODULE_DEVICE_TABLE(of, zynqmp_r5_remoteproc_match); > > > + > > > +static struct platform_driver zynqmp_r5_remoteproc_driver =3D { > > > + .probe =3D zynqmp_r5_remoteproc_probe, > > > + .remove =3D zynqmp_r5_remoteproc_remove, > > > + .driver =3D { > > > + .name =3D "zynqmp_r5_remoteproc", > > > + .of_match_table =3D zynqmp_r5_remoteproc_match, > > > + }, > > > +}; > > > +module_platform_driver(zynqmp_r5_remoteproc_driver); > > > + > > > +MODULE_AUTHOR("Ben Levinsky "); > > > +MODULE_LICENSE("GPL v2"); > > > -- > > > 2.17.1 > > > > > > Cheers, > > Michael > [Ben Levinsky] Thanks for the review BTW I have a question on %pOF... is = this > method depenedent on kernel configuration? For example printing ptr vals = in > kernel there are few ways to do this that depend on certain kernel config > options enabled/disabled or earlyprintk.