From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 36529C61DA3 for ; Tue, 21 Feb 2023 10:56:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=l2bLRgIjPk3QTnVazLydbbjivxZeEKYqclvzKqspK7s=; b=UL+6JBxaUd+OIV jkAEWb7kTNz5OpZN9buqM/bDaNSbc8Y0VSWA28+vQuGc4fiocZo2CleapKfE/MJTBnybhhTzb0xaE HiQI2EhfkdoDiPeL/DLyDYu5nueCBxwSbKChtthV2mjD1r2/T6Y1UHjyTLNyaH0NG3K4Y7EC/eGYo REfStv+rDa9uMTWZ0xKFG8ShY7b20RbWnuZwjDeqaVjVOecXKlYKb3/4fPpzQbaWAKtUZbCLRcePT TOyU/fID+YPIllgJC/ONbAHZG5Y0L1dmNqYchiC8p7EyQ6yRCIE/SpSbJ3kxGKOv2iKr+Al3NOe5C 5S+N91p9VR89bs3sQqFw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pUQJF-007TOU-N5; Tue, 21 Feb 2023 10:55:45 +0000 Received: from esa2.hgst.iphmx.com ([68.232.143.124]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pUQJC-007TKp-BE for linux-arm-kernel@lists.infradead.org; Tue, 21 Feb 2023 10:55:43 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=wdc.com; i=@wdc.com; q=dns/txt; s=dkim.wdc.com; t=1676976942; x=1708512942; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=dottw58DX6NpLcrIxoBfsO6WcV5uJQI3WvU4f4U5ZFg=; b=J0fJh7CwUc/UcnjCaQtrkdoJfTxyuaCpTh1akSsEOhz434Z3uBm1lfPq 5PZ+ZDszd3VLYHovsGctzFv4wZKNCFGQWd9RTPacG6pKvwB5fCn9dlXLj 43HOJyfLKxHQAVhhkLZKGdiZ730CClX897v2Orj929wtmn75I2WZ1OxHb mIra2OvuTeRigDbPR2XJWA6xIbECiAN6wj6fAlo8WgXF8IRTNmZ42JpaN 5nlnAKzFTjJ3io6gDkE2GzR6CQ5uvt7l7v3c3jNBQmlqjwZbcweMtX+a1 JkPM4rKSl70CvnymEy+K1tHQp/7fF39A45K6ninfnKSiSQZ+OZiI3gJge w==; X-IronPort-AV: E=Sophos;i="5.97,315,1669046400"; d="scan'208";a="328106628" Received: from h199-255-45-15.hgst.com (HELO uls-op-cesaep02.wdc.com) ([199.255.45.15]) by ob1.hgst.iphmx.com with ESMTP; 21 Feb 2023 18:55:33 +0800 IronPort-SDR: QQnAs71R6a99ftsgva2RRj+qkhIM6vN03npY3ysoHPW3NBlJXEqwnDFRXDtlM98R0xoWO/xwML 2VA9dwEMP29nEdUdTU8PdasxGwhiroYB+fuL55nL+SR9fIoxLaVD3mXf5aJZF6pnaRD8YKyQfE qcYkW8eQgqucMI3N+CK8YV6lkaWPi9IFpRb6vjWKkArO+IFSFvFaPEN+3sEZH3xwl/UVz+092L ROB4up/9HFkY9vIezBGYswBTg5wtKrFv+ErcD23+smbAmIPBBsucDniEd0ZDo0xjKO1yCD1Uzt /mk= Received: from uls-op-cesaip01.wdc.com ([10.248.3.36]) by uls-op-cesaep02.wdc.com with ESMTP/TLS/ECDHE-RSA-AES128-GCM-SHA256; 21 Feb 2023 02:06:48 -0800 IronPort-SDR: zoGvYhDW3o2udDuhrmZXli8uGoNP8S+GLKClnPLInMWh0SUZT1AlEwh19o2g5Ez71C/dj9reP8 xolZtuiuvH4Vng4lIMlQxMiW/D3eW+L9504XuzQLNMxNtev5c5cjJeJvawyTbxcA+XoFYcKU5f eEkH4bdmNZ1Xj1k/HG03vAXkn+vy+97ggB0Jy/9sKeAUAIvkU4EnvcNrlJypc90FDfW6I4XKqR G/UCfUYpJPJ6SRZLpYwzWdQPVDxPgWA7Ck1oXxu2FrdECYfacePpVJIg8vxkf74ugt3wJOMXlv 7Cg= WDCIronportException: Internal Received: from usg-ed-osssrv.wdc.com ([10.3.10.180]) by uls-op-cesaip01.wdc.com with ESMTP/TLS/ECDHE-RSA-AES128-GCM-SHA256; 21 Feb 2023 02:55:35 -0800 Received: from usg-ed-osssrv.wdc.com (usg-ed-osssrv.wdc.com [127.0.0.1]) by usg-ed-osssrv.wdc.com (Postfix) with ESMTP id 4PLbmP48pkz1RwvT for ; Tue, 21 Feb 2023 02:55:33 -0800 (PST) Authentication-Results: usg-ed-osssrv.wdc.com (amavisd-new); dkim=pass reason="pass (just generated, assumed good)" header.d=opensource.wdc.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d= opensource.wdc.com; h=content-transfer-encoding:content-type :in-reply-to:organization:from:references:to:content-language :subject:user-agent:mime-version:date:message-id; s=dkim; t= 1676976931; x=1679568932; bh=dottw58DX6NpLcrIxoBfsO6WcV5uJQI3WvU 4f4U5ZFg=; b=RqStWAGmwTKsaOFcjGMNiMQAycpsAdG0h/DujjZpLQIKf7RN8AR hlck4UC7t3C4HPOTq6dLYjS83znf9raxBpss1R2Imt+mIRvlo37r/LQaFFafom5m fo1dC3WtrmO8LIVKFXaK8WrSvCKPTEsrWfq30tZ2bZVawdtnF2tH6XJ5zzWCDyX3 FyGgmFhMi+FcqWn0qU+cpNQ5JO6xC5eKzPQvEmLDA5i/SsuzQ71rt1xFWu4jTwU6 n5xK/cgsCi+LGWrEf+BFxlmMWE3QUntug+ym4lBV+Wg0DoLwkxOTdwavBF3ruDie jvTKml0nhnsb97bbB1ki8MpDlF0E85A0Gpw== X-Virus-Scanned: amavisd-new at usg-ed-osssrv.wdc.com Received: from usg-ed-osssrv.wdc.com ([127.0.0.1]) by usg-ed-osssrv.wdc.com (usg-ed-osssrv.wdc.com [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id exmTCfrmAiba for ; Tue, 21 Feb 2023 02:55:31 -0800 (PST) Received: from [10.225.163.9] (unknown [10.225.163.9]) by usg-ed-osssrv.wdc.com (Postfix) with ESMTPSA id 4PLbmJ0q4rz1RvLy; Tue, 21 Feb 2023 02:55:27 -0800 (PST) Message-ID: <38ae72c9-0f0b-1a94-d2e0-f4ea80e94705@opensource.wdc.com> Date: Tue, 21 Feb 2023 19:55:26 +0900 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.2 Subject: Re: [PATCH v2 9/9] PCI: rockchip: Add parameter check for RK3399 PCIe endpoint core set_msi() Content-Language: en-US To: Rick Wertenbroek Cc: alberto.dassatti@heig-vd.ch, xxm@rock-chips.com, rick.wertenbroek@heig-vd.ch, Rob Herring , Krzysztof Kozlowski , Heiko Stuebner , Shawn Lin , Lorenzo Pieralisi , =?UTF-8?Q?Krzysztof_Wilczy=c5=84ski?= , Bjorn Helgaas , Jani Nikula , Greg Kroah-Hartman , Rodrigo Vivi , Mikko Kovanen , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org References: <20230214140858.1133292-1-rick.wertenbroek@gmail.com> <20230214140858.1133292-10-rick.wertenbroek@gmail.com> From: Damien Le Moal Organization: Western Digital Research In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230221_025542_468922_A9BFC3D6 X-CRM114-Status: GOOD ( 42.46 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2/21/23 19:47, Rick Wertenbroek wrote: > On Wed, Feb 15, 2023 at 2:39 AM Damien Le Moal > wrote: >> >> On 2/14/23 23:08, Rick Wertenbroek wrote: >>> The RK3399 PCIe endpoint core supports only a single PCIe physcial >>> function (function number 0), therefore return -EINVAL if set_msi() is >>> called with a function number greater than 0. >>> The PCIe standard only allows the multi message capability (MMC) value >>> to be up to 0x5 (32 messages), therefore return -EINVAL if set_msi() is >>> called with a MMC value of over 0x5. >>> >>> Signed-off-by: Rick Wertenbroek >>> --- >>> drivers/pci/controller/pcie-rockchip-ep.c | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c >>> index b7865a94e..80634b690 100644 >>> --- a/drivers/pci/controller/pcie-rockchip-ep.c >>> +++ b/drivers/pci/controller/pcie-rockchip-ep.c >>> @@ -294,6 +294,16 @@ static int rockchip_pcie_ep_set_msi(struct pci_epc *epc, u8 fn, u8 vfn, >>> struct rockchip_pcie *rockchip = &ep->rockchip; >>> u32 flags; >>> >>> + if (fn) { >>> + dev_err(&epc->dev, "This endpoint controller only supports a single physical function\n"); >>> + return -EINVAL; >>> + } >> >> Checking this here is late... Given that at most only one physical >> function is supported, the check should be in rockchip_pcie_parse_ep_dt(). >> Something like: >> >> err = of_property_read_u8(dev->of_node, "max-functions", >> &ep->epc->max_functions); >> >> if (err < 0 || ep->epc->max_functions > 1) >> >> ep->epc->max_functions = 1; >> > > Yes, this could be moved to the probe, thanks. > >> And all the macros with the (fn) argument could also be simplified >> (argument fn removed) since fn will always be 0. > > These functions cannot be simplified because they have to follow the signature > given by "pci_epc_ops" (include/linux/pci-epc.h). And this signature has the > function number as a parameter. If we change the function signature we won't > be able to assign these functions to the pc_epc_ops structure I was not suggesting to change the functions signature. I was suggesting dropping the fn argument for the *macros*, e.g. ROCKCHIP_PCIE_EP_FUNC_BASE(fn) -> ROCKCHIP_PCIE_EP_FUNC_BASE since fn is always 0. That said, I am not entirely sure if the limit really is 1 function at most. The TRM seems to be suggesting that up to 4 functions can be supported... [...] >> Another nice cleanup: define ROCKCHIP_PCIE_EP_MSI_CTRL_REG to include the >> ROCKCHIP_PCIE_EP_FUNC_BASE(fn) addition so that we do not have to do it >> here all the time. > > Yes, this could be an improvement but this is the way it is written > everywhere in this > driver, I chose to keep it so as to remain coherent with the rest of the driver. > Cleaning this is not so important since this code will not be > rewritten / changed so > often. But I agree that it might be nicer. But, on the other side if > at some point > support for virtual functions would be added, the offsets would need > to be computed > based on the virtual function number and the code would be written > like it is now, > so I suggest keeping this the way it is for now. Yes, sure, this can be cleaned later. A more pressing problem is the lack of support for MSIX despite the fact that the controller supports that *and* advertize it as a capability. That is what was causing my problem with the Linux nvme driver and my prototype nvme epf function driver: the host driver was seeing MSIX support (1 vector supported by default), and so was allocating one MSIX for the device probe. But on the EP end, it is MSI or INTX only... Working on adding that to solve this issue. -- Damien Le Moal Western Digital Research _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel