linux-kernel - Re: [PATCH v2 04/18] PCI: designware: use untranslated address while programming ATU

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 20 Jun 2014 18:18:23 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	linux-arm-kernel@...ts.infradead.org
Cc:	Kishon Vijay Abraham I <kishon@...com>, devicetree@...r.kernel.org,
	linux-doc@...r.kernel.org, linux-omap@...r.kernel.org,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
	Marek Vasut <marex@...x.de>, tony@...mide.com,
	Mohit Kumar <mohit.kumar@...com>, jg1.han@...sung.com,
	Jason Gunthorpe <jgunthorpe@...idianresearch.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>, rob.herring@...aro.org,
	Andrew Murray <Andrew.Murray@....com>
Subject: Re: [PATCH v2 04/18] PCI: designware: use untranslated address while programming ATU

On Wednesday 18 June 2014 14:38:43 Kishon Vijay Abraham I wrote:
> On Thursday 29 May 2014 12:08 PM, Kishon Vijay Abraham I wrote:
> > 
> > Here the CPU address for configuration space is 0x20013000 and the controller
> > address for configuration space is 0x13000. The controller address should be
> > used while programming the ATU (in order for translation to happen properly in
> > DRA7xx).
> 
> I've fixed this up with what you suggested in the v1 of the series. Do you
> think this is fine?

Hi Kishon,

Yes, I think this will work and do the right thing in all cases. I'm not
completely sure if the change should be generalized into the range
parser or better kept in the pcie-designware driver as you do it here:

> > @@ -406,6 +413,12 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> >               pp->config.cfg1_size = resource_size(cfg_res)/2;
> >               pp->cfg0_base = cfg_res->start;
> >               pp->cfg1_base = cfg_res->start + pp->config.cfg0_size;
> > +
> > +             /* Find the untranslated configuration space address */
> > +             index = of_property_match_string(np, "reg-names", "config");
> > +             addrp = of_get_address(np, index, false, false);
> > +             pp->cfg0_mod_addr = of_read_number(addrp, ns);
> > +             pp->cfg1_mod_addr = pp->cfg0_mod_addr + pp->config.cfg0_size;

I think I've recently seen some objections to hand-parsing standard
properties in driver code.

> >       } else {
> >               dev_err(pp->dev, "missing *config* reg space\n");
> >       }
> > @@ -431,12 +444,20 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> >                       pp->config.io_size = resource_size(&pp->io);
> >                       pp->config.io_bus_addr = range.pci_addr;
> >                       pp->io_base = range.cpu_addr;
> > +
> > +                     /* Find the untranslated IO space address */
> > +                     pp->io_mod_addr = of_read_number(parser.range -
> > +                                                      parser.np + na, ns);
> >               }

So the of_read_number() call could be moved into of_pci_range_to_resource(),
with another field added to struct of_pci_range to carry the address on
the parent bus. Any other thoughts on this? Maybe Rob Herring or Andrew
Murray have a strong opinion on this.


	Arnd
--
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/