From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750815AbcBEA2k (ORCPT ); Thu, 4 Feb 2016 19:28:40 -0500 Received: from mail-by2on0089.outbound.protection.outlook.com ([207.46.100.89]:40014 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750731AbcBEA2g (ORCPT ); Thu, 4 Feb 2016 19:28:36 -0500 Authentication-Results: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=caviumnetworks.com; Message-ID: <56B3ECAD.4050605@caviumnetworks.com> Date: Thu, 4 Feb 2016 16:28:29 -0800 From: David Daney User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Bjorn Helgaas CC: David Daney , Bjorn Helgaas , , Will Deacon , , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , , , David Daney Subject: Re: [PATCH v4 2/2] pci, pci-thunder-pem: Add PCIe host driver for ThunderX processors. References: <1453844781-24380-1-git-send-email-ddaney.cavm@gmail.com> <1453844781-24380-3-git-send-email-ddaney.cavm@gmail.com> <20160205000452.GI7031@localhost> In-Reply-To: <20160205000452.GI7031@localhost> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [64.2.3.194] X-ClientProxiedBy: BLUPR07CA072.namprd07.prod.outlook.com (25.160.24.27) To DM3PR07MB2138.namprd07.prod.outlook.com (25.164.4.144) X-Microsoft-Exchange-Diagnostics: 1;DM3PR07MB2138;2:+An4QS9AwCbz93HCmPtIF9jqzb1sVN0oxVXF+XuSjjVlAk8RYZC58OJhIcIWwK1Qt8PEljNsLtQOJmKJN04ZbjlbQPIzEcFWsEjWOp4PI702bx8xvTOnOSSd2Xd4/irn6Z1jxPVLz4DAfZ1lU542rQ==;3:VhAPUZT1lrFnoR/y6o2FvGGGDtCQ7wB9cKNshMNjYmuQLeCmr7RdFtr1rvsFVC8qK7d+/36iZCtfoy0WpnG143QTOeBFN5YHC05eay3yBf6QHXEV9SxW3UjKNkUk58Rj;25:rU/p9DdqA2b25fptPMNAAQez29DmI1WiYXtmJDwdELb3VuXmYEUFqBBtjje1Wful+TB7HRCkqekdhPJiyqyytJRylqarg1/+MgFZo9yW6/hrDiJvlP2E3LFmDeE57mECaUv2GA1SBHIntkkJ57QQ7vmKErHwuQ/yqyOXRfaIZZO/Nm2wszs3uD5r3L5WeCrUxXw+nK+LVZavwExe4ZB4kGO6X/VOFF1sGiwgDRVzWVdiYEsvznM4SqLdA08fBYgbYGxWkqb9FOGcrarO4i9zVs13Vo1WQAtLeBZ4b3eAfPCODaq63M7OKCdsPYzy/qUe X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DM3PR07MB2138; X-MS-Office365-Filtering-Correlation-Id: 29b5a0d6-d1a6-4358-f6a5-08d32dc34830 X-Microsoft-Exchange-Diagnostics: 1;DM3PR07MB2138;20:y9cXo/nQwkBScM6e9Roo1+Q9JYenJgC7ckEX1gJfPbGyzWAI9hRrUM8PZeQwTv/rPZ9d78JjwAfuyLBngOVeJtXwdETAjA66vKzjSvtUK9V3bYfmLGFNSwm01ifibno+OneN2nM5OuyXIoLPLKRYI0+GQGoFG1y8LpIFEzb8Vjn4wuukbGqQYJUtLxhv7u2+7PgRf9Ll4cYmwNwUFwtsOAiNkxxu/vsJg9TThZBrWQEH1fPM7YURaLA6HF6Q0nXFlrdYnHSm11FUD3F01M6R2qsOip2MNrx3n6yrwp7PaZ1/iLr0AwanGsgcbgyZu0/vZMgABOuVtGDhBZgBYi3jiUsadjpBsQoJrJY3IxiWdzluPYuEEPCom9bmuwLhls6ESoDSY4iE8Olq0cy9BktR36Ob+lh/ckm7rzPqf1DejAj6HPm3UzVb4Jg9sy5XrnZPH+5fjiEuFR1HDGiqTv9MASS1kzOX195bjdLvA2kaNeE0wxQVA8XwEdUxZe/h4FCcjP0B3Th/jCC6RYM3jFXdJ8TZ+uzs1aXEkGVBZ2cN2L7Qn9MRUFAp5mvdR6aWV/IlURmXrVasssIEHI3x126EzFSL+bJcFC0MAwg7PlVnD/0= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046);SRVR:DM3PR07MB2138;BCL:0;PCL:0;RULEID:;SRVR:DM3PR07MB2138; X-Microsoft-Exchange-Diagnostics: 1;DM3PR07MB2138;4:/tMalqRs85ZGx/b2p7t8w2QRFlynbFUrNheSuKZS7zXSlYOckGjSIrLdJBZbOlHxqvnz9FN/nKh1tCKh3OBo6n0dBSyWN/gVZG1R26m8RxCHc//5ZNIxPGRiKMd8qPUlr3SUzDfzaHO7rvt2gwEN7dPk+qnUzjjDfVtMY2ADD8LPuGByBmYx5eybY8eNnSKo49g1Tpe3bScSYmm6K2dcqgndM6LiC07kBn9PG+dw0fOG74jRKn1TxjyLU86/bdmkTRS42nUMXip6fxIvEpRre1aU7zPYL6mL9FtOSntr53q/lcfjSNkLQHV1BygDJJjU+166/r/eAKVhTdWTfsJsZ8kVnAAvCx4/tQF0pxeurm3Wj9600UdWXi5brBSYCKc5 X-Forefront-PRVS: 0843C17679 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6009001)(24454002)(377454003)(164054003)(479174004)(85664002)(50986999)(6116002)(65816999)(5004730100002)(1096002)(77096005)(2950100001)(76176999)(53416004)(230700001)(5008740100001)(4326007)(47776003)(66066001)(83506001)(586003)(3846002)(54356999)(33656002)(40100003)(42186005)(4001350100001)(110136002)(23756003)(87976001)(64126003)(59896002)(92566002)(50466002)(2906002)(189998001)(122386002)(230783001)(19580405001)(19580395003)(5001960100002)(36756003)(80316001);DIR:OUT;SFP:1101;SCL:1;SRVR:DM3PR07MB2138;H:dl.caveonetworks.com;FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?iso-8859-1?Q?1;DM3PR07MB2138;23:o4CMFxpPbZTFA25vfiB8WRD+MraRhk6r/DF1ZBM?= =?iso-8859-1?Q?lAc3mpYZ98G37qmbJqcK9KWeiZ3OY8QgLk+RZtbKxfxT8FRE/KYTPrJwZq?= =?iso-8859-1?Q?ET5BK49/Z/d2UULA2C0YIyXwx/+4MY3GybBTo5BuyN96TOdIvemm5Gd92m?= =?iso-8859-1?Q?0O22BrSScPd8nzbLDsae54HcKke8qN5kFvTUltMgYR0DbkZk/ExtsP8hG6?= =?iso-8859-1?Q?00LI57oBWPs5qeE6I9AZTMd5XvOkTJwdapLbZTKUaZFzEYuWSxXKDDBPXO?= =?iso-8859-1?Q?oJQML/sCXm4+9fydegpQiD12pYazvxveIVHbLJpJiU9VyKvt/l/KhkYoLm?= =?iso-8859-1?Q?j9gHCuVsa0HLEkZGWF2iX4855TGmvsZBaSx/yHRRNlhCDbiaHbaJZGsgqn?= =?iso-8859-1?Q?FowFPXzyFsHGQrO9bk5Ti+TyVA8uOjmxx5th3F81oxUV5GmCW/ARPFezxp?= =?iso-8859-1?Q?8rqIl5yqyBlCF3vvjE3QzhJtZwaNTq3iEUZPTZDoEFbPhhugsYrrO7DEfj?= =?iso-8859-1?Q?ZuMygoHnKo10jnScf2vCfHAu5n7nRr9ysZEnVvUpOB4YZV8Vann5koYVh2?= =?iso-8859-1?Q?ckQxVLwXC2p8VmPICrx+LluMs1SU5EqOEpFgRJ4ykWfNcc3iAu4myPiI1y?= =?iso-8859-1?Q?GdCisfvQyxOnEog+h0Ae3Oq1QgWjWtJHE2qyhmhCeFGfs8S9fFUerjcCSC?= =?iso-8859-1?Q?05Q9GInZrKqRtmPtDrtiTrhtT6cO2k9pEM8+nPNzRJ9B9D0t1QUalhIT/h?= =?iso-8859-1?Q?7gL+LbmS+9NngH/Q+0ZUlC9vVruTwaA8dzzBayynkkwM8ZWO8m962JPQKm?= =?iso-8859-1?Q?XyOpFgL9mTZfhIhLiEHedvtzytBVN0kLQkDed4Tg89ea9wAoZNgtE145Nx?= =?iso-8859-1?Q?B3UZ3w24giDkxb9p/AsDGbI9KIXQ4LerBQbPyE1NZUiD5dHzNt9n3qrlX7?= =?iso-8859-1?Q?cdXsVEvgJuVgjQieGwxa68fCB1zqvzH3n2cVp+JJ5d47DlAPk02OWNsGHj?= =?iso-8859-1?Q?Aa85DH/9/yRTDPQ8C2GJMRijSI3lYfF/zs/uH5MCBHH/pWVdSR5n2qk/RZ?= =?iso-8859-1?Q?uyVyMH46ZmEBk/g4y72cHivqB9Z8f5EdMqMTcrvfMazDmBDYvkO36kKLY8?= =?iso-8859-1?Q?Ev0RtLK4xI+0qH2irlAXF+dV/bAjlk0l/qAGVgPwIlKZ2Fd7MYzgrWdQHV?= =?iso-8859-1?Q?qmN7tkPkkhrSxSqf1sTse8O+lSYK8K3MQ=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1;DM3PR07MB2138;5:l0aVJ1hje2P+iYGIw6G29qBjEhUQhY33Wz7UdpIkCudeCGMUv6JWPIZs3p7J7HAC3G5ulDYZdKQ4046HV8vZ8vEYJ3KbnDWQ1KJEz7aYw5UZCALS8YCOH9yEZza7XRMU+dUWTB8u/OeU7PHegefGZg==;24:LSi24F+wIaC2sc+iCUObE1mHaZaTnCcO3A/5vvheMvKc8bK0yXMS/e5cwGjB06lMsqeRldq/MfcVNor1et4IkEybqO6bOSg6RbBpLDxhHAc= SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Feb 2016 00:28:32.5225 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM3PR07MB2138 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/04/2016 04:04 PM, Bjorn Helgaas wrote: > Hi David, > > Looks good, a few trival questions below. > > On Tue, Jan 26, 2016 at 01:46:21PM -0800, David Daney wrote: >> From: David Daney >> >> Some Cavium ThunderX processors require quirky access methods for the >> config space of the PCIe bridge. Add a driver to provide these config >> space accessor functions. The pci-host-common code is used to >> configure the PCI machinery. >> >> Signed-off-by: David Daney >> Acked-by: Rob Herring >> Acked-by: Arnd Bergmann >> --- >> .../devicetree/bindings/pci/pci-thunder-pem.txt | 43 ++++ >> MAINTAINERS | 8 + >> drivers/pci/host/Kconfig | 7 + >> drivers/pci/host/Makefile | 1 + >> drivers/pci/host/pci-thunder-pem.c | 283 +++++++++++++++++++++ > > What's the significance of the "pem" part of the name? I'm wondering > if we can shorten the filenames and function names by dropping it and > referring to this simply as "thunder" or "thunderx". PEM == PCI Express MAC, Essentially the circuitry related to off-chip PCI devices. This differentiates it from the internal on-chip devices which are connected to internal buses we refer to as "ECAMs" See also the follow on patch, from me, that adds the pci-thunder-ecam.c driver. Since PEM and ECAM are terminology used in the hardware manuals that have specific meanings relative to the Thunder SoC family, I think it is not too inconvenient to carry them over into the file names. > >> 5 files changed, 342 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/pci/pci-thunder-pem.txt >> create mode 100644 drivers/pci/host/pci-thunder-pem.c >> >> + >> +static int thunder_pem_config_write(struct pci_bus *bus, unsigned int devfn, >> + int where, int size, u32 val) >> +{ >> + struct gen_pci *pci = bus->sysdata; >> + struct thunder_pem_pci *pem_pci; >> + >> + pem_pci = container_of(pci, struct thunder_pem_pci, gen_pci); >> + >> + /* >> + * The first device on the bus in the PEM PCIe bridge. >> + * Special case its config access. >> + */ >> + if (bus->number == pci->cfg.bus_range->start) { >> + u64 write_val, read_val; >> + >> + if (devfn != 0 || where >= 2048) >> + return PCIBIOS_DEVICE_NOT_FOUND; >> + >> + /* >> + * 32-bit accesses only. If the write is for a size >> + * smaller than 32-bits, we must first read the 32-bit >> + * value and merge in the desired bits and then write >> + * the whole 32-bits back out. >> + */ > > Ugh. Another device that only supports 32-bit writes. I guess this > only affects this single device, and maybe you "know" that it has no > registers where RW1C bits may be corrupted. Although I suppose this > device has the standard status registers (Status at 0x06, Secondary > Status at 0x1e, Device Status in PCIe capability, etc.), which do > contain RW1C bits. This device is exactly the specific PCIe host bridge implementation, present on these SoCs. The only sane way to access its config space is via 32-bit operations. We know that it presents the appropriate Class and other registers to be recognized as, and operate as, a standard PCIe bridge. > > We need to print a warning at probe-time so we know to consider the > possibility of corruption when debugging. If you really want it, I suppose I can add that, but I am somewhat hesitant. > [...] >> + >> +static struct gen_pci_cfg_bus_ops thunder_pem_bus_ops = { >> + .bus_shift = 24, >> + .ops = { >> + .map_bus = map_cfg_bus_thunder_pem, > > How about "thunder_pem_map_bus"? That would be better. Actually, I wonder how I came up with that crappy name in the first place... > >> + .read = thunder_pem_config_read, >> + .write = thunder_pem_config_write, >> + } >> +}; >> + I will wait a day to see if you have any response, and then send a new version of the patch set. Thanks, David Daney