From: Christoph Hellwig <hch@lst.de>
To: Ishizaki Kou <kou.ishizaki@toshiba.co.jp>
Cc: linuxppc-dev@ozlabs.org
Subject: Re: [PATCH 9/16] Supporting of PCI bus for Celleb
Date: Wed, 15 Nov 2006 19:43:28 +0100 [thread overview]
Message-ID: <20061115184328.GD21633@lst.de> (raw)
In-Reply-To: <200611150945.kAF9jXRV007053@toshiba.co.jp>
> +static inline void ioif_setup(struct ioif *ioif, struct device_node *dn)
> +{
> + ioif->iommu_table = NULL;
> +}
> +
> +struct ioif *ioif_alloc(struct device_node *dn)
> +{
> + struct ioif *ioif;
> +
> + if (mem_init_done)
> + ioif = kmalloc(sizeof(struct ioif), GFP_KERNEL);
> + else
> + ioif = alloc_bootmem(sizeof(struct ioif));
> +
> + if (ioif)
> + ioif_setup(ioif, dn);
> +
> + return ioif;
Please switch the above from kmalloc to kzalloc. As alloc_bootmem also
zeroes it's return value you now have ioif pre-cleared and don't need
ioif_setup. Also when can this be called from non-initialization code?
> +struct ioif {
Struct ioif is a bit too generic, can you give it a better name?
> +extern int
> +__init celleb_setup_pci_bridge(struct device_node *, struct pci_controller *);
Please move this extern declaration to a header.
> +
> +static int
> +celleb_dummy_pci_read_config(struct pci_bus *bus,
> + unsigned int devfn,
> + int where, int size, u32 *val)
> +{
> +
> +
> + return PCIBIOS_DEVICE_NOT_FOUND;
> +}
> +
> +
> +static int
> +celleb_dummy_pci_write_config(struct pci_bus *bus,
> + unsigned int devfn,
> + int where, int size, u32 val)
> +{
> +
> + return PCIBIOS_DEVICE_NOT_FOUND;
> +}
> +
> +struct pci_ops celleb_dummy_pci_ops = {
> +/* for generic PCI buses/devices */
> + celleb_dummy_pci_read_config,
> + celleb_dummy_pci_write_config
> +};
This look broken to me. Busses that do not support config space
cycles shouldn't be reported to the PCI layer at all.
> +struct pci_ops celleb_fake_pci_ops = {
> + celleb_fake_pci_read_config,
> + celleb_fake_pci_write_config
> +};
What are these fake ops for? If you don't have a real PCI
bus you shouldn't emulate it but use a vio-style bus insted.
next prev parent reply other threads:[~2006-11-15 18:43 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-11-15 9:45 [PATCH 9/16] Supporting of PCI bus for Celleb Ishizaki Kou
2006-11-15 18:43 ` Christoph Hellwig [this message]
2006-11-15 23:40 ` Benjamin Herrenschmidt
2006-11-17 10:40 ` Ishizaki Kou
2006-11-17 22:08 ` Arnd Bergmann
2006-11-17 22:20 ` Benjamin Herrenschmidt
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=20061115184328.GD21633@lst.de \
--to=hch@lst.de \
--cc=kou.ishizaki@toshiba.co.jp \
--cc=linuxppc-dev@ozlabs.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.