linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Vitaly Bordug <vitb@kernel.crashing.org>
Cc: linuxppc-dev@ozlabs.org
Subject: Re: [RFC] [PATCH] PowerPC: Add 64-bit phys addr support to 32-bit pci.
Date: Wed, 19 Sep 2007 07:38:20 +1000	[thread overview]
Message-ID: <1190151501.6403.113.camel@localhost.localdomain> (raw)
In-Reply-To: <20070918221811.57b92c18@localhost.localdomain>


> > @@ -879,11 +891,18 @@
> >  	prev = NULL;
> >  	while ((rlen -= np * sizeof(unsigned int)) >= 0) {
> >  		if (prev) {
> > -			if (prev[0] == ranges[0] && prev[1] == ranges[1] &&
> > -				(prev[2] + prev[na+4]) == ranges[2] &&
> > -				(prev[na+2] + prev[na+4]) == ranges[na+2]) {
> > -				prev[na+4] += ranges[na+4];
> > +			prev_pci_space = prev[0];
> > +			prev_pci_addr = pci_get_range64(&prev[1]);
> > +			prev_size = pci_get_range64(&prev[na+3]);
> > +			pci_space = ranges[0];
> > +			pci_addr = pci_get_range64(&ranges[1]);
> > +			if ((prev_pci_space == pci_space) && 
> > +			    ((prev_pci_addr + prev_size) == pci_addr)) {
> > +				size = pci_get_range64(&ranges[na+3]);
> > +				prev_size += size;
> >  				ranges[0] = 0;
> > +				prev[na+3] = (u32)((prev_size >> 32) & 0xffffffff);
> > +				prev[na+4] = (u32)(prev_size & 0xffffffff);
> >  				ranges += np;
> >  				continue;
> 
> I do think that ranges hacking (even on a copy) to cope with contiguous ranges is not a good deed. And nobody would object the upper looks horrible from the maintenance POV.
> >  			

Yes, hacking of ranges shouldn't be done there. PowerMac does range
coalescing separately, we can move this to some generic place maybe and
use it where needed.

Just make everything use resource_size_t in the PCI bits and u64 in the
parsing bits.

> This is not correct - it should depend on #ac of the parent node.
> But I'll stop right here - there is no deep mutual difference between 32 & 64 bit so that to keep 2 similar implementations, none of which (esp 32bit) really comply the spec. It should work the same way, and, if there are differences, they should be handled
> explicitly, and, of course, reconsidered if they make sense (like isa_io_base, absense of io_size in ppc32 and so on)
> 
> I am *not* telling here that my implementation is the only true way around. But we need to improve and make code cleaner rather then just extend existing error-prone approach.

We need to merge 32 and 64 bits here for sure.

Ben.

      reply	other threads:[~2007-09-18 21:38 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-18 14:44 [RFC] [PATCH] PowerPC: Add 64-bit phys addr support to 32-bit pci Valentine Barshak
2007-09-18 14:52 ` Kumar Gala
2007-09-18 15:01   ` Valentine Barshak
2007-09-18 15:13     ` Kumar Gala
2007-09-18 16:07       ` Valentine Barshak
2007-09-18 18:18         ` Vitaly Bordug
2007-09-18 21:38           ` Benjamin Herrenschmidt [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1190151501.6403.113.camel@localhost.localdomain \
    --to=benh@kernel.crashing.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=vitb@kernel.crashing.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).