Received: by 2002:ac0:a582:0:0:0:0:0 with SMTP id m2-v6csp468270imm; Fri, 12 Oct 2018 01:03:38 -0700 (PDT) X-Google-Smtp-Source: ACcGV63x6GjcgiFNEFdBB7yvjAp0q58BTuEFf8Dk9Ra7yKPA6l2aFyLxyaKsQX3vMcxx8EJ3nojm X-Received: by 2002:a17:902:101:: with SMTP id 1-v6mr4895940plb.15.1539331418128; Fri, 12 Oct 2018 01:03:38 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1539331418; cv=none; d=google.com; s=arc-20160816; b=R1X9uhWlKIODNoCQjsOV0M00wnpSMChqj0BlJL7AfiRTEz/f5AW2IglQs4+UksN48R P9wm1pUZrbK9ydANUxrnfZpdM3fNRBmpzdsdTNSFVTDo7n20iqe4LjnCdzJhkf77IkSF 01MPCFqs3ASIfcuyTCh1dwBUS8DX6Xda46bfTGj+bYAEvs5WbTEEsl/099hU88suE7DR X5X9kAH8t75e44eu7aGHDHWHxki2avFv3OOInRqsqpuBwBZk9oyFXVYdfanwn6HjmZ5w 9Et2lHEaFWpR7EjsvbYETl2ERHLJhPxe5N5X141aDc3DLBNRr0+nmpO6G2nvI4kemFTs IWTQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:mime-version:content-transfer-encoding :references:in-reply-to:date:cc:to:from:subject:message-id; bh=2Ec4s/vhPn5AiHl4Oahh0rSJRPtdcml0z5wROn3hmNc=; b=RRvG4+F+S/o2HItFRIhNGOFCqTEwb/cFt9K4OegxvBC5x7oS7wL8rz1qm9F4k9gULN 7NmSvkO5+IOmmgqIluMhZoKxXvlvjIzrUAfYv2SdLPrtMgRQgqAjPgpAu6eJHA6m9x6i ymCEq31E2xj0ya7miuj1hQMrwJ613nTcKELSp/JQFfPIwfNvmdCCZ/Gqj/Osc5S/X0nL unZwrU4qeoTcBa97BH4OV1t1SDFNHGW7QbH3VHy5tqr2OKlwL9qiZxJWBkz131Vl5muG dMBaLcwRoyFnvIlYZOTIcOpVKlB8+/vXlBHosGfMFyBnlmDHMX8KcyuGhGW9nBDIivv6 /rRQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id h125-v6si512529pfg.138.2018.10.12.01.03.23; Fri, 12 Oct 2018 01:03:38 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727554AbeJLPc6 (ORCPT + 99 others); Fri, 12 Oct 2018 11:32:58 -0400 Received: from mailgw02.mediatek.com ([1.203.163.81]:36665 "EHLO mailgw02.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1727315AbeJLPc6 (ORCPT ); Fri, 12 Oct 2018 11:32:58 -0400 X-UUID: 450a75e0774e4f0c81ee2c7e32e65889-20181012 X-UUID: 450a75e0774e4f0c81ee2c7e32e65889-20181012 Received: from mtkcas36.mediatek.inc [(172.27.4.250)] by mailgw02.mediatek.com (envelope-from ) (mailgw01.mediatek.com ESMTP with TLS) with ESMTP id 1223359043; Fri, 12 Oct 2018 16:01:31 +0800 Received: from MTKCAS32.mediatek.inc (172.27.4.184) by MTKMBS31DR.mediatek.inc (172.27.6.102) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Fri, 12 Oct 2018 16:01:29 +0800 Received: from [10.17.3.153] (10.17.3.153) by MTKCAS32.mediatek.inc (172.27.4.170) with Microsoft SMTP Server id 15.0.1395.4 via Frontend Transport; Fri, 12 Oct 2018 16:01:29 +0800 Message-ID: <1539331289.6246.61.camel@mhfsdcap03> Subject: Re: [PATCH v6 2/9] PCI: mediatek: Fixup class ID for MT7622 as PCI_CLASS_BRIDGE_PCI From: Honghui Zhang To: Lorenzo Pieralisi CC: , , , , , , , , , , , , , , Date: Fri, 12 Oct 2018 16:01:29 +0800 In-Reply-To: <20181011113748.GA3574@e107981-ln.cambridge.arm.com> References: <1538969088-7136-1-git-send-email-honghui.zhang@mediatek.com> <1538969088-7136-3-git-send-email-honghui.zhang@mediatek.com> <20181008172337.GA13538@e107981-ln.cambridge.arm.com> <1539054495.6246.31.camel@mhfsdcap03> <20181011113748.GA3574@e107981-ln.cambridge.arm.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.3-0ubuntu6 Content-Transfer-Encoding: 7bit MIME-Version: 1.0 X-MTK: N Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2018-10-11 at 12:38 +0100, Lorenzo Pieralisi wrote: > On Tue, Oct 09, 2018 at 11:08:15AM +0800, Honghui Zhang wrote: > > On Mon, 2018-10-08 at 18:23 +0100, Lorenzo Pieralisi wrote: > > > On Mon, Oct 08, 2018 at 11:24:41AM +0800, honghui.zhang@mediatek.com wrote: > > > > From: Honghui Zhang > > > > > > > > The PCIe controller of MT7622 has TYPE 1 configuration space type, but > > > > the HW default class type values is invalid. > > > > > > > > The commit 101c92dc80c8 ("PCI: mediatek: Set up vendor ID and class > > > > type for MT7622") have set the class ID for MT7622 as > > > > PCI_CLASS_BRIDGE_HOSTe, but it's not workable for MT7622: > > > > > > > > In __pci_bus_assign_resources, the framework only setup bridge's > > > > resource window only if class type is PCI_CLASS_BRIDGE_PCI. Or it > > > > will leave the subordinary PCIe device's MMIO window un-touched. > > Can you please provide me with a full log of the issue ? > > What is the bug this patch is actually fixing ? > > > > > Fixup the class type to PCI_CLASS_BRIDGE_PCI as most of the controller > > > > driver do. > > > > > > I think that this patch is correct but the commit log fails to pin point > > > the problem. The IP you are programming is a root port, that's why you > > > have to have the proper class code, the patch looks fine but I would > > > like to peek Bjorn's brain on this since it is a fundamental concept. > > > > > > > I'm a bit confused with the concepts of PCI_CLASS_BRIDGE_HOST and > > PCI_CLASS_BRIDGE_PCI, from PCI express spec, > > 4.0r1.0(PCI_Express_Base_4.0r1.0_September-27-2017-c), Host Bridge is > > "part of a Root Complex that connects a host CPU or CPUs to a > > Hierarchy". While Root Complex defines as "A defined System Element that > > includes at least one Host Bridge, Root port, or Root complex Integrated > > Endpoint". > > > > But according to my understanding, most of the root port IPs integrated > > with a "PCI_CLASS_BRIDGE_PCI", which has type 1 configuration space and > > could be saw as a pci device when using lspci. > > > > And for MT7622, it integrated with block of internal control registers, > > type 1 configuration space, and is considered as a root complex. > > I assume you mean a type 1 config header here. I do not think it > is mandatory for a host bridge to have a type 1 config header > (and related bridge windows + primary/secondary/subordinate bus > numbers) but I do not know how the IP you are programming is > designed. Yes, MT7622 has the type 1 config header and related bridge windows and primary/secondary/subordinate bus numbers. > > If the host bridge needs its memory window to be configured you can > easily do that in the driver (with driver specific code) without > requiring the generic PCI resource core to do that for you, the host > bridge is the root of the bus I do not see any reason why it should > be up to core PCI code to assign it, it is preprogrammed. > Thanks for explain this. Yes, the MT7622 bridge needs its memory window to be configured but I prefer the PCI resource core to do this for the following reasons: 1. MTK have MT7622 and MT2712, they pretty much using the same IP, but have different memory window base. Currently we using device tree to pass the memory window base. Take using of PCI resource core code to parse and assign those resource will release the burden of driver. 2. I do not want to re-implement the resource parse code to get the memory base from device node. And hard code the memory base in driver is not quite elegant. 3. Most of the host driver which I referenced are using the PCI resource core to help with it's memory window configure, I guess following the majority way maybe better even they may have slit different of the IP design from MTK. 4. Passing the memory base in device node make it more flexible to change the memory window base (in the HW acceptable range) in case of some special EP device required some special PCI address range. 5. MT7622 and MT2712 have the pretty much same IP only MT7622's HW has set the class type to an un-proper class type. Set it to the same values as MT2712 is an easy way to fix. thanks. > > I'm not sure which CLASS type it should have: > > From PCI_Code-ID_r_1_10__v8-Nov_2017, class type of > > 0x0604(PCI_CLASS_BRIDGE_PCI) is defined as a PCI-to-PCI bridge, not > > literally suitable for MT7622(which is a root complex)(In my personal > > opinion). But it is the only workable CLASS type for MT7622 in current > > kernel. > > > > > If the kernel does not assign resources unless it detects a > > > PCI_CLASS_BRIDGE_PCI this means that for components that are > > > actually PCI_CLASS_BRIDGE_HOST their register set must come > > > preprogrammed unless I am missing something. > > > > > > > In the function pci_request_resource_alignment, it will by pass the > > resource assignment for PCI_CLASS_BRIDGE_HOST, though I'm not figured > > out why. > > > > > I would like to get to the bottom of this since it is a fundamental > > > enumeration concept. > > > > > > > Do you like me to re-write the commit message for this patch and put the > > above information in? Or just not mention the PCI_CLASS_BRIDGE_HOST > > assign resource routine? > > I want to understand where the problem is and whether it is right to > define the component as a PCI bridge rather than a host bridge. > I will update the commit message (put some of the above reasons in the commit message) and send a new version of this patch if it's acceptable for you. Thanks > Lorenzo