Re: [RFC PATCH v1 1/4] arm/pci: PCI setup and PCI host bridge discovery within XEN on ARM.

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Rahul Singh <Rahul.Singh@arm.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>,
	Bertrand Marquis <Bertrand.Marquis@arm.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	nd <nd@arm.com>, Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [RFC PATCH v1 1/4] arm/pci: PCI setup and PCI host bridge discovery within XEN on ARM.
Date: Tue, 28 Jul 2020 10:21:00 +0200	[thread overview]
Message-ID: <20200728082100.GV7191@Air-de-Roger> (raw)
In-Reply-To: <567A8D4D-9D7F-4D53-B740-6095F5512026@arm.com>

On Tue, Jul 28, 2020 at 08:06:17AM +0000, Rahul Singh wrote:
> 
> 
> > On 24 Jul 2020, at 3:44 pm, Roger Pau Monné <roger.pau@citrix.com> wrote:
> > 
> > On Thu, Jul 23, 2020 at 04:40:21PM +0100, Rahul Singh wrote:
> >> +
> >> +    struct pci_host_bridge *bridge = pci_find_host_bridge(sbdf.seg, sbdf.bus);
> >> +
> >> +    if ( unlikely(!bridge) )
> >> +    {
> >> +        printk(XENLOG_ERR "Unable to find bridge for "PRI_pci"\n",
> >> +                sbdf.seg, sbdf.bus, sbdf.dev, sbdf.fn);
> > 
> > I had a patch to add a custom modifier to out printf format in
> > order to handle pci_sbdf_t natively:
> > 
> > https://patchew.org/Xen/20190822065132.48200-1-roger.pau@citrix.com/
> > 
> > It missed maintainers Acks and was never committed. Since you are
> > doing a bunch of work here, and likely adding a lot of SBDF related
> > prints, feel free to import the modifier (%pp) and use in your code
> > (do not attempt to switch existing users, or it's likely to get
> > stuck again).
> 
> Ok Will integrate that patch once submitted.

I've posted an updated version to the list yesterday:

https://lore.kernel.org/xen-devel/20200727103136.53343-1-roger.pau@citrix.com/

> >> diff --git a/xen/include/asm-arm/pci.h b/xen/include/asm-arm/pci.h
> >> index de13359f65..94fd00360a 100644
> >> --- a/xen/include/asm-arm/pci.h
> >> +++ b/xen/include/asm-arm/pci.h
> >> @@ -1,7 +1,98 @@
> >> -#ifndef __X86_PCI_H__
> >> -#define __X86_PCI_H__
> >> +/*
> >> + * Copyright (C) 2020 Arm Ltd.
> >> + *
> >> + * Based on Linux drivers/pci/ecam.c
> >> + * Copyright 2016 Broadcom.
> >> + *
> >> + * Based on Linux drivers/pci/controller/pci-host-common.c
> >> + * Based on Linux drivers/pci/controller/pci-host-generic.c
> >> + * Copyright (C) 2014 ARM Limited Will Deacon <will.deacon@arm.com>
> >> + *
> >> + * This program is free software; you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License version 2 as
> >> + * published by the Free Software Foundation.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> + * GNU General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU General Public License
> >> + * along with this program.  If not, see <http://secure-web.cisco.com/1YIShdE6aU0WIMsRjhIYYGdRo_H_wyOVeLqk0ITHI7i_FMIXuMEmV9Y6lR76qCBB4XHss0ba_WMfQa0mLGD37bM1S3AFMBVC1WUEF0LmbURPWld2kU2zuyOamUxqnYwy4ZAv4N2EUJy7HCgDYn2s183YCQ-5YBc98vH8TO-tfBgEShkKVzAm23AgzIqikuKN_BIhcQhAbsjypr9ffeNZDsSX2jC8ClqegEPXFsCWJldrASGgWoR16rMjI-INfPNy55m9nGP5UmHIYWBmTpnHLjXtqCkruENixh20vPWIUROhcjjtzVBrR8d-Q5HnJy0hSR97WdlUSAryfxeH-8VxcPw/http%3A%2F%2Fwww.gnu.org%2Flicenses%2F>.
> >> + */
> >> 
> >> +#ifndef __ARM_PCI_H__
> >> +#define __ARM_PCI_H__
> >> +
> >> +#include <xen/pci.h>
> >> +#include <xen/device_tree.h>
> >> +#include <asm/device.h>
> >> +
> >> +#ifdef CONFIG_ARM_PCI
> >> +
> >> +/* Arch pci dev struct */
> >> struct arch_pci_dev {
> >> +    struct device dev;
> >> +};
> > 
> > This seems to be completely unused?
> 
> Will be using going forward.

Please introduce it when it's required then.

Thanks, Roger.