> 2011/4/13 George Kashperko <george@xxxxxxxxxxx>:
> >
> >> 2011/4/13 George Kashperko <george@xxxxxxxxxxx>:
> >> >
> >> > Ð ÐÑÐ, 13/04/2011 Ð 21:39 +0200, RafaÅ MiÅecki ÐÐÑÐÑ:
> >> >> 2011/4/13 Greg KH <greg@xxxxxxxxx>:
> >> >> >> diff --git a/drivers/axi/axi_pci_bridge.c b/drivers/axi/axi_pci_bridge.c
> >> >> >> new file mode 100644
> >> >> >> index 0000000..17e882c
> >> >> >> --- /dev/null
> >> >> >> +++ b/drivers/axi/axi_pci_bridge.c
> >> >> >> @@ -0,0 +1,33 @@
> >> >> >> +/*
> >> >> >> + * AXI PCI bridge module
> >> >> >> + *
> >> >> >> + * Licensed under the GNU/GPL. See COPYING for details.
> >> >> >> + */
> >> >> >> +
> >> >> >> +#include "axi_private.h"
> >> >> >> +
> >> >> >> +#include <linux/axi/axi.h>
> >> >> >> +#include <linux/pci.h>
> >> >> >> +
> >> >> >> +static DEFINE_PCI_DEVICE_TABLE(axi_pci_bridge_tbl) = {
> >> >> >> + { PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, 0x4331) },
> >> >> >> + { PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, 0x4353) },
> >> >> >> + { PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, 0x4727) },
> >> >> >> + { 0, },
> >> >> >> +};
> >> >> >> +MODULE_DEVICE_TABLE(pci, axi_pci_bridge_tbl);
> >> >> >> +
> >> >> >> +static struct pci_driver axi_pci_bridge_driver = {
> >> >> >> + .name = "axi-pci-bridge",
> >> >> >> + .id_table = axi_pci_bridge_tbl,
> >> >> >> +};
> >> >> >> +
> >> >> >> +int __init axi_pci_bridge_init(void)
> >> >> >> +{
> >> >> >> + return axi_host_pci_register(&axi_pci_bridge_driver);
> >> >> >> +}
> >> >> >> +
> >> >> >> +void __exit axi_pci_bridge_exit(void)
> >> >> >> +{
> >> >> >> + axi_host_pci_unregister(&axi_pci_bridge_driver);
> >> >> >> +}
> >> >> >
> >> >> > You register a pci driver that does nothing? That's not right, you need
> >> >> > to then base your axi bus off of that pci device, so it is hooked up
> >> >> > correctly in the /sys/devices/ tree. Otherwise you are somewhere up in
> >> >> > the virtual location for your axi bus, right?
> >> >>
> >> >> Please take a look at:
> >> >> driver->probe = axi_host_pci_probe;
> >> >> driver->remove = axi_host_pci_remove;
> >> >> return pci_register_driver(driver);
> >> >>
> >> >>
> >> >> >> +bool axi_core_is_enabled(struct axi_device *core)
> >> >> >> +{
> >> >> >> + if ((axi_aread32(core, AXI_IOCTL) & (AXI_IOCTL_CLK | AXI_IOCTL_FGC))
> >> >> >> + != AXI_IOCTL_CLK)
> >> >> >> + return false;
> >> >> >> + if (axi_aread32(core, AXI_RESET_CTL) & AXI_RESET_CTL_RESET)
> >> >> >> + return false;
> >> >> >> + return true;
> >> >> >> +}
> >> >> >> +EXPORT_SYMBOL(axi_core_is_enabled);
> >> >> >
> >> >> > EXPORT_SYMBOL_GPL()?
> >> >> >
> >> >> > What module uses this? And why would it care?
> >> >> >
> >> >> >> +EXPORT_SYMBOL(axi_core_enable);
> >> >> >
> >> >> > EXPORT_SYMBOL_GPL()?
> >> >> >
> >> >> > Same goes for your other exports, just want you to be sure here.
> >> >>
> >> >> Hm, I'm not sure. Using EXPORT_SYMBOL_GPL will forbid closed source
> >> >> drivers from using our bus driver, right? I'm don't have preferences
> >> >> on this, if you prefer us to force GPL, I can.
> >> >>
> >> >>
> >> >> >> +u32 xaxi_chipco_gpio_control(struct axi_drv_cc *cc, u32 mask, u32 value)
> >> >> >> +{
> >> >> >> + return axi_cc_write32_masked(cc, AXI_CC_GPIOCTL, mask, value);
> >> >> >> +}
> >> >> >> +EXPORT_SYMBOL(xaxi_chipco_gpio_control);
> >> >> >
> >> >> > "xaxi"? Shouldn't that be consistant with the other exports and start
> >> >> > with "axi"?
> >> >>
> >> >> Left from old tests/rewrites/splitting. Thanks.
> >> >>
> >> >>
> >> >> >> +static u8 axi_host_pci_read8(struct axi_device *core, u16 offset)
> >> >> >> +{
> >> >> >> + if (unlikely(core->bus->mapped_core != core))
> >> >> >
> >> >> > Are you sure about the use of unlikely in this, and other functions?
> >> >> > The compiler almost always does a better job than we do for these types
> >> >> > of calls, just let it do it's job.
> >> >> >
> >> >> >> + axi_host_pci_switch_core(core);
> >> >> >> + return ioread8(core->bus->mmio + offset);
> >> >> >
> >> >> > I think because of that unlikely, you just slowed down all pci devices,
> >> >> > right? That's not very nice :)
> >> >>
> >> >> Hm, my logic suggests it is alright, but please consider this once
> >> >> more with me ;)
> >> >>
> >> >> For the most of the time mapped_core (active core) do not change. We
> >> >> perform few hundreds of operations on one core in a row. This way
> >> >> mapped_core points to passed core for most of the time. Condition
> >> >> (mapped_core != core) is unlikely to happen.
> >> >>
> >> >> Is there anything wrong in my logic?
> >> >>
> >> > Yes, there is. You don't need that "if" at all.
> >>
> >> Damn, WHY do you make me ask why, why, why, all the time?! Can't you
> >> just write word of explanation without being asked for?
> >>
> > Errm... Sorry, but I've already explained PCIE host behaviour _several_
> > times several days ago. Personally I like to ask questions. Have
> > absolutely nothing agains anyone else asking good questions. Never try
> > to make people ask me questions I know they would ask anyway. Really you
> > might missed some my messages earlier or maybe my english is too awful ?
> >
> > Yet again, for PCIE cores (not only for them, for some PCI cores as
> > well) buscommon, buscore and function core are all available
> > simultaneously. You dont need window sliding when access them.
>
> I had no idea what you were referring to. We do not dig into PCIe
> functions yet, so I believe for now we need this "if".
You are already deep inside. Your host code drives pci function.
>
> I'm getting totally frustrated with that whole situation :|
>
Have nice day,
George
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
