Re: [PATCH v3 12/17] xen/arm: Add support for Xilinx ZynqMP PCI host controller

From: Rahul Singh <Rahul.Singh@arm.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Bertrand Marquis <Bertrand.Marquis@arm.com>,
	Andre Przywara <Andre.Przywara@arm.com>,
	Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>,
	Julien Grall <julien@xen.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [PATCH v3 12/17] xen/arm: Add support for Xilinx ZynqMP PCI host controller
Date: Thu, 30 Sep 2021 10:42:41 +0000	[thread overview]
Message-ID: <ACBA6FD3-DC58-4531-91BD-DB39E262325E@arm.com> (raw)
In-Reply-To: <3478d1b9-7043-3505-a125-700eb3761ed4@suse.com>

Hi Jan

> On 30 Sep 2021, at 8:48 am, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 29.09.2021 18:41, Stefano Stabellini wrote:
>> On Tue, 28 Sep 2021, Rahul Singh wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/arm/pci/pci-host-zynqmp.c
>>> @@ -0,0 +1,63 @@
>>> +/*
>>> + * Based on Linux drivers/pci/controller/pci-host-common.c
>>> + * Based on Linux drivers/pci/controller/pci-host-generic.c
>>> + * Based on xen/arch/arm/pci/pci-host-generic.c
>>> + *
>>> + * 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://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include <asm/device.h>
>>> +#include <xen/pci.h>
>>> +#include <asm/pci.h>
>>> +
>>> +static int nwl_cfg_reg_index(struct dt_device_node *np)
>>> +{
>>> +    return dt_property_match_string(np, "reg-names", "cfg");
>>> +}
>> 
>> Can this be __init?
>> 
>> 
>>> +/* ECAM ops */
>>> +const struct pci_ecam_ops nwl_pcie_ops = {
>>> +    .bus_shift  = 20,
>>> +    .cfg_reg_index = nwl_cfg_reg_index,
>>> +    .pci_ops    = {
>>> +        .map_bus                = pci_ecam_map_bus,
>>> +        .read                   = pci_generic_config_read,
>>> +        .write                  = pci_generic_config_write,
>>> +    }
>>> +};
>>> +
>>> +static const struct dt_device_match nwl_pcie_dt_match[] = {
>> 
>> This should probably be __initconst
> 
> __initconstrel again as it seems.

Ack. I will use __initconstrel in next version.

Regards,
Rahul
> 
> Jan
> 