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 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).