Received: by 2002:a05:6a10:206:0:0:0:0 with SMTP id 6csp907815pxj; Wed, 16 Jun 2021 16:56:45 -0700 (PDT) X-Google-Smtp-Source: ABdhPJychPBTFA/nC7qy4SGmfflCmOyAnRsnPFXvxQ36gNuh+flxCKptUAhL79ssQtreN2dLOn9z X-Received: by 2002:a17:907:7b8b:: with SMTP id ne11mr2007963ejc.177.1623887805483; Wed, 16 Jun 2021 16:56:45 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1623887805; cv=none; d=google.com; s=arc-20160816; b=qATOKFOyNZ3TJTnqcg0qCuQ9iu4QkmaU6KnxAK8TmQNnPgEqirpImdwrKFsPsI1DJe URdpzIoXYcK/8Nnen2smeB9/9MhpczWoazwva4OKfXmhPhb/086OzQT8Cxn4WbVMOcgM tzKYhc/nDFEDc/7Q5aUiY3JBTRpUg6FjLVfrTK5Z5SHY4H5XIyYvbtrYO6gU7lPWEgNS YsoHEnEEVrSxgy2y9EAyy0SSmkEj5NP15Xfp/GBjYEdNZtMLaBa3kP/6czCriOy5SQlz w1JJiW+i4dKYYxCedKtAU/IoF+R63j7DyL55cVXOk5/Aq05lNsGmcaYMjabaflEHvDye aZew== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=xD7LbR/9JIs10ujeSy8yRxfvFpSuzxskN3Z3PFU43eM=; b=jLOgZ6kx6vHcNW96ZXP6KHGkzSjsm9vqAaEWHowhPtZSOeLU4QAWducna3Bv55KSa/ zO13Aio3u4Jw8PMguqe+leuB7PaV98WADeWIDXb132S/4TKSpvmvMakGBWeh5yUL+Pzq indR2UDRg4Ff3SVJY/71HEUZrqdGrp8lwrMEfkWh8AUt1JKOiXDv748mBSalNNPqHO4K UJhwwU5KdoCjdxk2/9LKiJ+t6hJMt7dnNN97+dw4NSLcsjjo0YkKWXuadRgta2YsVjPd TPt1OeDCYXZm78IPN7fKps4XomYCEZMLHPDiA7Ku02jsKDbhE6owP55OOOdy1UAiVm92 IB3A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b="WO/FaVs2"; 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; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id x25si3755041eds.477.2021.06.16.16.56.22; Wed, 16 Jun 2021 16:56:45 -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=@kernel.org header.s=k20201202 header.b="WO/FaVs2"; 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; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230136AbhFPQeY (ORCPT + 99 others); Wed, 16 Jun 2021 12:34:24 -0400 Received: from mail.kernel.org ([198.145.29.99]:39804 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229989AbhFPQeY (ORCPT ); Wed, 16 Jun 2021 12:34:24 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 3E64461375; Wed, 16 Jun 2021 16:32:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1623861138; bh=HnQ8O75YA6igOLb0Of/jo3UVORF2qdfDgqTXkgouYkM=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=WO/FaVs2gYQb9kwpCc++rVs0vsOyI0YQAsbAnsaVLlBTv2mDPwaSi5Msurlo2hZra rSwRuj/cXTqxBiKcMP6gsRYOrGmbk8TfRauEKsB9OnG+sEbpK6PtgfAtSyk14VDWje qshqKehekV94Lxmzxu8r9YxJAsP/3Z44hQvvTQWqf3CDD4XrNwzTr3m0YphnkuuIGm cAfg4wpG5iC8eeLA6Qpe+79beTOkQgUVfKqa17VRbNQAQQCFcOlO6I3LZL0UjUC85q +65YZB8U2udbdIXsU6kd8XW+hBZsaEQh9ehHtxUZ9OYqB1xumR+A3L84tXYgg2rIJk gxwkEciYWh9EQ== Received: by mail-ed1-f47.google.com with SMTP id r7so3506600edv.12; Wed, 16 Jun 2021 09:32:18 -0700 (PDT) X-Gm-Message-State: AOAM530BdrDXzEbG2BPkaPAdXx0chJHx86PyqYettzSTbMv2mA+ow0It YExWpGsPaqed8XwbBXI5Xj99JKucryGTeGsnjg== X-Received: by 2002:aa7:cb19:: with SMTP id s25mr667990edt.194.1623861136799; Wed, 16 Jun 2021 09:32:16 -0700 (PDT) MIME-Version: 1.0 References: <20210524063004.132043-3-nobuhiro1.iwamatsu@toshiba.co.jp> <20210524185839.GA1102116@bjorn-Precision-5520> In-Reply-To: <20210524185839.GA1102116@bjorn-Precision-5520> From: Rob Herring Date: Wed, 16 Jun 2021 10:32:05 -0600 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v3 2/3] PCI: Visconti: Add Toshiba Visconti PCIe host controller driver To: Bjorn Helgaas Cc: Nobuhiro Iwamatsu , Bjorn Helgaas , Lorenzo Pieralisi , PCI , Punit Agrawal , yuji2.ishikawa@toshiba.co.jp, linux-arm-kernel , "linux-kernel@vger.kernel.org" , Kishon Vijay Abraham I Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, May 24, 2021 at 12:58 PM Bjorn Helgaas wrote: > > [+cc Kishon for cpu_addr_fixup() question] > > Please make the subject "PCI: visconti: Add ..." since the driver > names are usually lower-case. When referring to the hardware itself, > use "Visconti", of course. > > On Mon, May 24, 2021 at 03:30:03PM +0900, Nobuhiro Iwamatsu wrote: > > Add support to PCIe RC controller on Toshiba Visconti ARM SoCs. PCIe > > controller is based of Synopsys DesignWare PCIe core. > > > > This patch does not yet use the clock framework to control the clock. > > This will be replaced in the future. > > > > v2 -> v3: > > - Update subject. > > - Wrap description in 75 columns. > > - Change config name to PCIE_VISCONTI_HOST. > > - Update Kconfig text. > > - Drop blank lines. > > - Adjusted to 80 columns. > > - Drop inline from functions for register access. > > - Changed function name from visconti_pcie_check_link_status to > > visconti_pcie_link_up. > > - Update to using dw_pcie_host_init(). > > - Reorder these in the order of use in visconti_pcie_establish_link. > > - Rewrite visconti_pcie_host_init() without dw_pcie_setup_rc(). > > - Change function name from visconti_device_turnon() to > > visconti_pcie_power_on(). > > - Unify formats such as dev_err(). > > - Drop error label in visconti_add_pcie_port(). > > > > v1 -> v2: > > - Fix typo in commit message. > > - Drop "depends on OF && HAS_IOMEM" from Kconfig. > > - Stop using the pointer of struct dw_pcie. > > - Use _relaxed variant. > > - Drop dw_pcie_wait_for_link. > > - Drop dbi resource processing. > > - Drop MSI IRQ initialization processing. > > Thanks for the changelog. Please move it after the "---" line for > future versions. That way it won't appear in the commit log when this > is merged. The notes about v1->v2, v2->v3, etc are useful during > review, but not after this is merged. > > > Signed-off-by: Yuji Ishikawa > > Signed-off-by: Nobuhiro Iwamatsu > > --- > > drivers/pci/controller/dwc/Kconfig | 9 + > > drivers/pci/controller/dwc/Makefile | 1 + > > drivers/pci/controller/dwc/pcie-visconti.c | 369 +++++++++++++++++++++ > > 3 files changed, 379 insertions(+) > > create mode 100644 drivers/pci/controller/dwc/pcie-visconti.c > > +static u64 visconti_pcie_cpu_addr_fixup(struct dw_pcie *pci, u64 pci_addr) > > +{ > > + return pci_addr - PCIE_BUS_OFFSET; > > This is called from __dw_pcie_prog_outbound_atu() as: > > cpu_addr = pci->ops->cpu_addr_fixup(pci, cpu_addr); > > so I think the parameter here should be *cpu_addr*, not pci_addr. > > dra7xx and artpec6 also call it "pci_addr", which is at best > confusing. > > I'm also confused about exactly what .cpu_addr_fixup() does. Is it > applying an offset that cannot be deduced from the DT description? If > so, *should* this offset be described in DT? It could be perhaps, but it would be a custom property, not something we can handle in 'ranges'. I'd rather it be implicit from the compatible than a custom property. AIUI, the issue is the cpu address gets masked (high bits discarded). This can happen when the internal bus address decoding throws away upper address bits. For example: 0xa0000000 -> 0x20000000 -> 0x00000000 cpu addr -> DW local addr -> PCI bus addr DT has the first and last addresses, but iATU needs the middle and last address. This could be just a data value rather than an ops function. While a subtract works here, that's fragile (the DT needs to match the #define) and I think a mask would be more appropriate. Rob