From: Olof Johansson <olof@lixom•net>
To: Benjamin Herrenschmidt <benh@kernel•crashing.org>
Cc: linuxppc-dev@ozlabs•org, paulus@samba•org, anton@samba•org
Subject: Re: [4/5] powerpc: PA Semi PWRficient platform support
Date: Tue, 5 Sep 2006 17:10:13 -0500 [thread overview]
Message-ID: <20060905171013.734fa390@localhost.localdomain> (raw)
In-Reply-To: <1157491909.22705.143.camel@localhost.localdomain>
On Wed, 06 Sep 2006 07:31:49 +1000 Benjamin Herrenschmidt <benh@kernel•crashing.org> wrote:
> Overall, looks good, some bits tho:
Thanks! See below.
>
> > +static int pa_pxp_read_config(struct pci_bus *bus, unsigned int devfn,
> > + int offset, int len, u32 *val)
> > +{
> > + struct pci_controller *hose;
> > + unsigned long addr;
>
> .../...
>
> You don't seem to encode support for extended config space (offsets >
> 256). If it's PCIe, you should have it (flush the 4 top bits of the
> offste in the 4 top bits of the cfg address in general). In any case,
> you should check the passed-in offset and error out if it is above your
> max offset (256 or 4096 depending if you implement extended config space
> support) or the kernel might get confused.
I'll take a look at this, thanks for pointing it out.
> > + of_prop = alloc_bootmem(sizeof(struct property) +
> > + sizeof(hose->global_number));
> > + if (of_prop) {
> > + memset(of_prop, 0, sizeof(struct property));
> > + of_prop->name = "linux,pci-domain";
> > + of_prop->length = sizeof(hose->global_number);
> > + of_prop->value = (unsigned char *)&of_prop[1];
> > + memcpy(of_prop->value, &hose->global_number, sizeof(hose->global_number));
> > + prom_add_property(dev, of_prop);
> > + }
>
> Just kill that... nobody cares about that property.
Done
> > + if (device_is_compatible(dev, "pa-pxp"))
> > + setup_pa_pxp(hose);
>
> And if not ? You have a non-initialized hose without config space access
> methods ? Hrm...
True. Just a leftover from forking off the maple setup code.
> > + printk(KERN_INFO "Found PA-PXP PCI host bridge. Firmware bus number: %d->%d\n",
> > + hose->first_busno, hose->last_busno);
> > +
> > + /* Interpret the "ranges" property */
> > + /* This also maps the I/O region and sets isa_io/mem_base */
> > + pci_process_bridge_OF_ranges(hose, dev, 1);
> > + pci_setup_phb_io(hose, 1);
> > +
> > + return 0;
> > +}
> > +
> > +
> > +void __init pas_pcibios_fixup(void)
> > +{
> > + struct pci_dev *dev = NULL;
> > +
> > + for_each_pci_dev(dev)
> > + pci_read_irq_line(dev);
> > +}
> > +
> > +static void __init pas_fixup_phb_resources(void)
> > +{
> > + struct pci_controller *hose, *tmp;
> > +
> > + list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
> > + unsigned long offset = (unsigned long)hose->io_base_virt - pci_io_base;
> > + hose->io_resource.start += offset;
> > + hose->io_resource.end += offset;
> > + printk(KERN_INFO "PCI Host %d, io start: %lx; io end: %lx\n",
> > + hose->global_number,
> > + hose->io_resource.start, hose->io_resource.end);
> > + }
> > +}
>
> There is no generic code for the above ? (I have to double check). If
> not, we shall fix that ;)
Didn't see anything, but then I didn't look that long. I'll follow up with a
separate patch if it doesn't exist, since other platforms can use it too.
>
> > +#ifdef CONFIG_SMP
> > +struct smp_ops_t pas_smp_ops = {
> > + .probe = smp_mpic_probe,
> > + .message_pass = smp_mpic_message_pass,
> > + .kick_cpu = smp_generic_kick_cpu,
> > + .setup_cpu = smp_mpic_setup_cpu,
> > + .give_timebase = smp_generic_give_timebase,
> > + .take_timebase = smp_generic_take_timebase,
> > +};
>
> You don't have a HW timebase sync facility ? Is this the board or the
> processor doesn't have a pin to freeze the timebase ? Either way, that
> is bad ! Time to use the cluebat !
See comment to Arnd in another email: We have hardware support for it, just
not including it at this time.
>
> > +#endif /* CONFIG_SMP */
> > +
> > +void __init pas_setup_arch(void)
> > +{
> > + /* init to some ~sane value until calibrate_delay() runs */
> > + loops_per_jiffy = 50000000;
>
> That value still makes any sense with the new tb based delays ? I know
> we still have it here or there, I'm wondering tho...
I haven't tried doing without it, I'll take a look.
-Olof
next prev parent reply other threads:[~2006-09-05 22:10 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20060904175742.5472a6fa@localhost.localdomain>
2006-09-05 17:26 ` [1/5] powerpc: Reduce default cacheline size to 64 bytes Olof Johansson
2006-09-05 17:27 ` [2/5] powerpc: Divorce CPU_FTR_CTRL from CPU_FTR_PPCAS_ARCH_V2_BASE Olof Johansson
2006-09-05 17:28 ` [3/5] powerpc: PA6T cputable entry, PVR value Olof Johansson
2006-09-05 18:43 ` Michael Neuling
2006-09-05 17:28 ` [4/5] powerpc: PA Semi PWRficient platform support Olof Johansson
2006-09-05 21:31 ` Benjamin Herrenschmidt
2006-09-05 22:10 ` Olof Johansson [this message]
2006-09-05 17:29 ` Olof Johansson
2006-09-05 19:49 ` Roland Dreier
2006-09-05 20:15 ` Olof Johansson
2006-09-05 21:37 ` Arnd Bergmann
2006-09-05 21:48 ` Olof Johansson
2006-09-06 13:38 ` Segher Boessenkool
2006-09-06 15:10 ` Olof Johansson
2006-09-06 15:26 ` Segher Boessenkool
2006-09-07 0:58 ` Benjamin Herrenschmidt
2006-09-07 11:30 ` Segher Boessenkool
2006-09-07 0:58 ` Benjamin Herrenschmidt
2006-09-07 11:28 ` Segher Boessenkool
2006-09-07 12:47 ` Olof Johansson
2006-09-07 22:33 ` Benjamin Herrenschmidt
2006-09-05 17:30 ` [5/5] powerpc: PA Semi PWRficient MAINTAINER entry Olof Johansson
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=20060905171013.734fa390@localhost.localdomain \
--to=olof@lixom$(echo .)net \
--cc=anton@samba$(echo .)org \
--cc=benh@kernel$(echo .)crashing.org \
--cc=linuxppc-dev@ozlabs$(echo .)org \
--cc=paulus@samba$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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