| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 20 Jun 2014 14:47:58 -0400
From: Murali Karicheri <m-karicheri2@...com>
To: Pratyush Anand <pratyush.anand@...com>
CC: "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"Shilimkar, Santosh" <santosh.shilimkar@...com>,
Russell King <linux@....linux.org.uk>,
Grant Likely <grant.likely@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Mohit KUMAR DCG <Mohit.KUMAR@...com>,
Jingoo Han <jg1.han@...sung.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Richard Zhu <r65037@...escale.com>,
"ABRAHAM, KISHON VIJAY" <kishon@...com>,
Marek Vasut <marex@...x.de>, Arnd Bergmann <arnd@...db.de>,
Pawel Moll <pawel.moll@....com>,
Mark Rutland <mark.rutland@....com>,
Ian Campbell <ijc+devicetree@...lion.org.uk>,
Kumar Gala <galak@...eaurora.org>,
Randy Dunlap <rdunlap@...radead.org>
Subject: Re: [PATCH v2 2/8] PCI: designware: refactor host init code to re-use
on v3.65 DW pci hw
On 6/18/2014 3:05 AM, Pratyush Anand wrote:
> Hi Murali,
>
> On Wed, Jun 11, 2014 at 02:51:21AM +0800, Murali Karicheri wrote:
>> Current DW PCI host init code has code specific to newer hw such as
>> ATU port specific resource parsing and map. v3.65 DW PCI host has
> OK, Older version did not had standard viewport implementation, so patch 1
> of this series will help you to take care for that.
>
>> MSI controller in application space and requires different controller
> Since MSI controller is implemented in application space, so this may
> not be same for different older version dw controller users.
>
> Therefore, I would suggest to implement all application specific code
> in your keystone driver only.
Pratyush,
Thanks for the comments.
This is IP specific code and another driver that has this version of the
IP will be able to
re-use the code. This is also being discussed in another thread from
Bjorn and Jingoo.
Please discuss this in that thread.
>> initialization code. So refactor the msi host controller code into a
>> separate function. Other common functions across both hw versions
>> are moved to dw_pcie_common_host_init() and called from the newer and
>> v3.65 hw host initialization code.
>>
>> Signed-off-by: Murali Karicheri <m-karicheri2@...com>
>>
>> CC: Santosh Shilimkar <santosh.shilimkar@...com>
>> CC: Russell King <linux@....linux.org.uk>
>> CC: Grant Likely <grant.likely@...aro.org>
>> CC: Rob Herring <robh+dt@...nel.org>
>> CC: Mohit Kumar <mohit.kumar@...com>
>> CC: Jingoo Han <jg1.han@...sung.com>
>> CC: Bjorn Helgaas <bhelgaas@...gle.com>
>> CC: Pratyush Anand <pratyush.anand@...com>
>> CC: Richard Zhu <r65037@...escale.com>
>> CC: Kishon Vijay Abraham I <kishon@...com>
>> CC: Marek Vasut <marex@...x.de>
>> CC: Arnd Bergmann <arnd@...db.de>
>> CC: Pawel Moll <pawel.moll@....com>
>> CC: Mark Rutland <mark.rutland@....com>
>> CC: Ian Campbell <ijc+devicetree@...lion.org.uk>
>> CC: Kumar Gala <galak@...eaurora.org>
>> CC: Randy Dunlap <rdunlap@...radead.org>
>> CC: Grant Likely <grant.likely@...aro.org>
>>
>> ---
>> drivers/pci/host/pcie-designware.c | 136 ++++++++++++++++++++++++++----------
>> 1 file changed, 101 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
>> index e8f5d8d..e4bd19a 100644
>> --- a/drivers/pci/host/pcie-designware.c
>> +++ b/drivers/pci/host/pcie-designware.c
>> @@ -389,13 +389,19 @@ static const struct irq_domain_ops msi_domain_ops = {
>> .map = dw_pcie_msi_map,
>> };
>>
>> -int __init dw_pcie_host_init(struct pcie_port *pp)
>> +/*
>> + * dw_pcie_parse_resource() - Function to parse common resources
>> + *
>> + * @pp: ptr to pcie port
>> + *
>> + * Parse the range property for MEM, IO and cfg resources, and map
>> + * the cfg register space.
>> + */
> Why do you need this function? If you have some extra resource, you
> can ioremap that before you call dw_pcie_host_init.
I assume you are referring to dw_pcie_parse_resource(). Keystone PCIe
driver needs
this to parse the range property for IORESOURCE_MEM and IORESOURCE_IO
resources.
So refactored this into a function and called from host_init()
code for v3.65 and dw_pcie_host_init . But for Keystone PCI driver, no
need to ioremap
cfg1 and cfg2 as it doesn't have ATU ports. So host_init() code has to
be little different.
Idea is to have IP specific host_init() and refactor and re-use the
common code on both.
>
>> +int __init dw_pcie_parse_resource(struct pcie_port *pp)
>> {
>> struct device_node *np = pp->dev->of_node;
>> struct of_pci_range range;
>> struct of_pci_range_parser parser;
>> - u32 val;
>> - int i;
>>
>> if (of_pci_range_parser_init(&parser, np)) {
>> dev_err(pp->dev, "missing ranges property\n");
>> @@ -431,41 +437,29 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>> pp->config.cfg1_size = resource_size(&pp->cfg)/2;
>> }
>> }
>> -
>> - if (!pp->dbi_base) {
>> - pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
>> - resource_size(&pp->cfg));
>> - if (!pp->dbi_base) {
>> - dev_err(pp->dev, "error with ioremap\n");
>> - return -ENOMEM;
>> - }
>> - }
>> -
>> - pp->cfg0_base = pp->cfg.start;
>> - pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size;
>> pp->mem_base = pp->mem.start;
>>
>> - pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
>> - pp->config.cfg0_size);
>> - if (!pp->va_cfg0_base) {
>> - dev_err(pp->dev, "error with ioremap in function\n");
>> - return -ENOMEM;
>> - }
>> - pp->va_cfg1_base = devm_ioremap(pp->dev, pp->cfg1_base,
>> - pp->config.cfg1_size);
>> - if (!pp->va_cfg1_base) {
>> - dev_err(pp->dev, "error with ioremap\n");
>> - return -ENOMEM;
>> - }
>> + return 0;
>> +}
>>
>> - if (of_property_read_u32(np, "num-lanes", &pp->lanes)) {
>> - dev_err(pp->dev, "Failed to parse the number of lanes\n");
>> - return -EINVAL;
>> - }
>> +/*
>> + * dw_pcie_msi_host_init() - Function to initialize msi host controller
>> + *
>> + * @pp: ptr to pcie port
>> + * @np: device node ptr to msi irq controller
>> + * @irq_msi_ops: ptr to MSI irq_domain_ops struct
>> + *
>> + * Function register irq domain for msi irq controller and create mappings
>> + * for MSI irqs.
>> + */
>
> May be you can only do following to support your MSI chip:
>
> Initialize pp->irq_domain into your add_pcie_port function before you
> call dw_pcie_host_init.
>
> In dw_pcie_host_init,
>
> if (IS_ENABLED(CONFIG_PCI_MSI) && !pp->irq_domain)
How do I pass the keystone specific msi irq_domain_ops? It looks clean
the way it is implemented
in this patch. Also since MSI host controller and legacy host controller
are different, it make
sense to add separate host_init for each. Let me know.
Regards,
Murali
>
>> +int dw_pcie_msi_host_init(struct pcie_port *pp, struct device_node *np,
>> + const struct irq_domain_ops *irq_msi_ops)
>> +{
>> + int i;
>>
>> if (IS_ENABLED(CONFIG_PCI_MSI)) {
>> pp->irq_domain = irq_domain_add_linear(pp->dev->of_node,
>> - MAX_MSI_IRQS, &msi_domain_ops,
>> + MAX_MSI_IRQS, irq_msi_ops,
>> &dw_pcie_msi_chip);
>> if (!pp->irq_domain) {
>> dev_err(pp->dev, "irq domain init failed\n");
>> @@ -476,6 +470,29 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>> irq_create_mapping(pp->irq_domain, i);
>> }
>>
>> + return 0;
>> +}
>> +
>> +/*
>> + * dw_pcie_common_host_init() - common host init function across different
>> + * versions of the designware PCI controller.
>> + * @pp: ptr to pcie port
>> + * @hw: ptr to hw_pci structure
>> + *
>> + * This functions parses common DT properties, call host_init() callback
>> + * of the PCI controller driver. Also initialize the common RC configurations
>> + * and call common pci core function to intialize the controller driver.
>> + */
>> +int __init dw_pcie_common_host_init(struct pcie_port *pp, struct hw_pci *hw)
>> +{
>> + struct device_node *np = pp->dev->of_node;
>> + u32 val;
>> +
>> + if (of_property_read_u32(np, "num-lanes", &pp->lanes)) {
>> + dev_err(pp->dev, "Failed to parse the number of lanes\n");
>> + return -EINVAL;
>> + }
>> +
>> if (pp->ops->host_init)
>> pp->ops->host_init(pp);
>>
>> @@ -488,10 +505,10 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>> val |= PORT_LOGIC_SPEED_CHANGE;
>> dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
>>
>> - dw_pci.nr_controllers = 1;
>> - dw_pci.private_data = (void **)&pp;
>> + hw->nr_controllers = 1;
>> + hw->private_data = (void **)&pp;
>>
>> - pci_common_init_dev(pp->dev, &dw_pci);
>> + pci_common_init_dev(pp->dev, hw);
>> pci_assign_unassigned_resources();
>> #ifdef CONFIG_PCI_DOMAINS
>> dw_pci.domain++;
>> @@ -500,6 +517,55 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>> return 0;
>> }
>>
>> +/*
>> + * dw_pcie_host_init() - Host init function for new designware h/w
>> + *
>> + * @pp: ptr to pcie port
>> + *
>> + * The function parse the PCI resurces for IO, Memory and map the config
>> + * space addresses. Also initliaze the MSI irq controller and call
>> + * dw_pcie_common_host_init() to initialize the PCI controller.
>> + */
>> +int __init dw_pcie_host_init(struct pcie_port *pp)
>> +{
>> + int ret;
>> +
>> + ret = dw_pcie_parse_resource(pp);
>> + if (ret)
>> + return ret;
>> +
>> + if (!pp->dbi_base) {
>> + pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
>> + resource_size(&pp->cfg));
>> + if (!pp->dbi_base) {
>> + dev_err(pp->dev, "error with ioremap\n");
>> + return -ENOMEM;
>> + }
>> + }
>> +
>> + pp->cfg0_base = pp->cfg.start;
>> + pp->cfg1_base = pp->cfg.start + pp->config.cfg0_size;
>> +
>> + pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
>> + pp->config.cfg0_size);
>> + if (!pp->va_cfg0_base) {
>> + dev_err(pp->dev, "error with ioremap in function\n");
>> + return -ENOMEM;
>> + }
>> + pp->va_cfg1_base = devm_ioremap(pp->dev, pp->cfg1_base,
>> + pp->config.cfg1_size);
>> + if (!pp->va_cfg1_base) {
>> + dev_err(pp->dev, "error with ioremap\n");
>> + return -ENOMEM;
>> + }
>> +
>> + ret = dw_pcie_msi_host_init(pp, pp->dev->of_node, &msi_domain_ops);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return dw_pcie_common_host_init(pp, &dw_pci);
>> +}
>> +
>> static void dw_pcie_prog_viewport_cfg0(struct pcie_port *pp, u32 busdev)
>> {
>> /* Program viewport 0 : OUTBOUND : CFG0 */
> Regards
> Pratyush
>
>> --
>> 1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists