SSB bus driver documentation?

SSB bus driver documentation?

[Rafał Miłecki]

I spent few hours trying to understand/track ssb code and I'm giving
up now. I can not understand basic code flow.

We have some ssb_modinit, but I don't see it performing scanning bus
at all. For some reason this function is aware of b43 bridge and gige
driver. GigE driver doesn't seem to be module, while b43 seems to be.
gige uses pci_device_id, while b43 uses ssb_device_id. I have no idea
what b43 PCI bridge is. Flow seems to be ugly, at ssb_modinit we call
ssb_gige_init which calls out ssb_driver_register. There is some magic
"ssb_attach_queued_buses", which sounds like SSB (which is bus)
attaching... buses.

I wanted to at least understand when scanning of cores is done. I
found "ssb_bus_scan", it is called from "ssb_bus_register". This
function is called from code for every host, for example:
ssb_bus_pcibus_register. This one seems to be called from
ssb_pcihost_probe, however pcihost_wrapper is not module itself, it
does not contains any devices table. This function seems to be
registered with pci_register_driver from ssb_pcihost_register. That
one gets called b43_pci_ssb_bridge_init. But damn, I don't remember
anymore where I was 5 step ago. Not to mention whole code path.


OK, I wrote all that to show you my confusion. I don't think asking
any single questions will help understanding that. Is there any
documentation I could read to get any basic understanding of ssb?


P.S.
Even one more thing: try to guess by function's name what can it be
responsible for: "ssb_bus_ssbbus_register". It is used by some
/arch/mips/bcm47xx/setup.c, but I can not image digging into next
driver to understand whole architecture :|

-- 
Rafa?

SSB bus driver documentation?

[Michael Buesch]

On Mon, 2011-03-07 at 23:37 +0100, Rafa? Mi?ecki wrote:
> We have some ssb_modinit, but I don't see it performing scanning bus
> at all.

It just registers the bus type to the kernel.

> For some reason this function is aware of b43 bridge and gige
> driver. GigE driver doesn't seem to be module, while b43 seems to be.

Well, b43-pci-bridge and gige should be standalone
modules. However b43-pci-bridge is so small that it's
not worth it and so it is integrated into ssb module.
gige has some weird dependencies to the mips platform
code, so it can't easily be standalone.

> while b43 uses ssb_device_id.

Well, b43 is the driver for the "wireless" core of an SSB chip (bus).
Therefore it is an ssb_device.

> I have no idea
> what b43 PCI bridge is.

b43-pci-bridge is the driver for the host-pci device
for SSB based PCI cards. Looks like this:

PCI bus -> b43-pci-bridge -> SSB bus -> b43 (ssb_device)


> Flow seems to be ugly, at ssb_modinit we call
> ssb_gige_init which calls out ssb_driver_register.

Well, that's just because we cannot have two module_init
in one module.

> There is some magic
> "ssb_attach_queued_buses", which sounds like SSB (which is bus)
> attaching... buses.

This is just used for embedded devices.
As I already said, this mechanism should be rewritten to use the
standard platform_device mechanism.
However, I doubt it would make it easier to understand. But it
would be more ideomatic.

> I wanted to at least understand when scanning of cores is done. I
> found "ssb_bus_scan", it is called from "ssb_bus_register". This
> function is called from code for every host, for example:
> ssb_bus_pcibus_register. This one seems to be called from
> ssb_pcihost_probe, however pcihost_wrapper is not module itself, it
> does not contains any devices table. This function seems to be
> registered with pci_register_driver from ssb_pcihost_register. That
> one gets called b43_pci_ssb_bridge_init. But damn, I don't remember
> anymore where I was 5 step ago. Not to mention whole code path.

I think you're probably trying to understand the wrong thing here.
If you want to understand kernel booting, you don't walk the code
from physical CPU entry point to forking of the init process.

There's a lot of standard kernel subsystem code involved here,
so it certainly is very hard to "walk" the codepath by hand.

What happens on a PCI card is pretty easy:

SSB (ssb_modinit) registers the "SSB" bustype to the kernel. Now the kernel
knows about the new bustype. No more no less.
B43 registers the driver for the SSB wireless core.
The b43-pci-bridge registers the toplevel PCI driver to the kernel.
If the kernel detects such a device, it probes the driver (pcihost_wrapper).
Which in turn registers the SSB bus. The kernel notices that the SSB bustype
was registered and calls its initialization. Which in turn triggers scanning
of the bus. The scanning discovers the devices and registers the SSB devices
to the kernel. As the b43 driver already registered the SSB wireless core
driver, the kernel matches that against the registered devices and calls
b43 probe if it was found.

> OK, I wrote all that to show you my confusion. I don't think asking
> any single questions will help understanding that. Is there any
> documentation I could read to get any basic understanding of ssb?

No. But you should probably read something about the kernel device
framework. Some book like Linux Device Drivers, perhaps.

> 
> P.S.
> Even one more thing: try to guess by function's name what can it be
> responsible for: "ssb_bus_ssbbus_register". It is used by some
> /arch/mips/bcm47xx/setup.c, but I can not image digging into next
> driver to understand whole architecture :|

That function registers an SSB bus on an embedded system.
It's basically the same thing what b43-pci-bridge does, except
that the bustype is set to a different value and some minor things
are done differently in the init process.

As I said, this should use platform_device mechanism. But it would
make it even harder for you to understand. :D

-- 
Greetings, Michael.

SSB bus driver documentation?

[Rafał Miłecki]

OK, today that ssb code makes more sense to me, thanks for help.


I'd like to ask few more questions:
1) In case of PCI host we have b43_pci_bridge that loads ssb. What
about other hosts? I don't see any IDs table in sdio.c or pcmcia.c.
2) Could you say a word more about pcihost_wrapper? What is this for?
I can see we use that in pci bridge. What we can't call
ssb_bus_pcibus_register directly?
3) What is embedded.c?


I've also few comments to analyzed code. Could you check them, say if
it's OK/worth fixing that?

1) main.c::ssb_modinit
Hacks for bridge and gige (both being not modules) are not documented.
New developer have no idea why we call b43_pci_ssb_bridge_init and
ssb_gige_init. Tracking that calls lead to even more confusion.

2) ssb.h::ssb_bustype
SSB_BUSTYPE_SSB,
SSB_BUSTYPE_PCI,
SSB_BUSTYPE_PCMCIA,
SSB_BUSTYPE_SDIO,
I think first define is confusing. Is sounds like SSB being host for
SSB, or whatever... I think it should be sth like SSB_BUSTYPE_SYSTEM,
SSB_BUSTYPE_NONE, SSB_BUSTYPE_NATIVE, SSB_BUSTYPE_EMBEDDED, or sth.

3) main.c::ssb_ssb_ops
We keep all host-specific ops in separated files, but not in this
case. OK, it makes some sense as this one is not for host, but I think
it makes main.c more complicated and we can not compile SSB without
it. I think we could build every PC kernel without this, right?

4) scan.c::scan_switchcore
Why we don't have ops->switchcore? We could get rid of that switch
with simple pointer.

5) scan.c::scan_read32
I'm not sure about this yet... but do we need that here? Shouldn't
scan.c focus on just scanning? It's just one another "offset +=
current_coreidx * SSB_CORE_SIZE;" calculation.


I criticized scan.c::scan_read32, but could you say why do we need
that specific read at all? Why can't we use some ops->read32? I can
see sdio.c have some additional masking and is claims host, but didn't
analyze that yet.


-- 
Rafa?

SSB bus driver documentation?

[Michael Büsch]

On Tue, 2011-03-08 at 12:01 +0100, Rafa? Mi?ecki wrote: 
> OK, today that ssb code makes more sense to me, thanks for help.
> 
> 
> I'd like to ask few more questions:
> 1) In case of PCI host we have b43_pci_bridge that loads ssb. What
> about other hosts? I don't see any IDs table in sdio.c or pcmcia.c.

That part is in b43. b43_pci_bridge is only in SSB, because it is
shared between b43 and b43legacy.

> 2) Could you say a word more about pcihost_wrapper? What is this for?
> I can see we use that in pci bridge. What we can't call
> ssb_bus_pcibus_register directly?

It's just a library of common PCI initialization functions. It's
used from b43_pci_bridge and from b44's PCI init code.

> 3) What is embedded.c?

A collection of functions for embedded devices (SSB without host bus),
only.

> 1) main.c::ssb_modinit
> Hacks for bridge and gige (both being not modules) are not documented.
> New developer have no idea why we call b43_pci_ssb_bridge_init and
> ssb_gige_init. Tracking that calls lead to even more confusion.

Well, it's pretty obvious what those functions do, if you know how
the kernel device driver model works. They register device drivers.
Which is done in the modinit function in almost all drivers.

And I don't think those calls are "hacks". It's pretty standard stuff.

> 2) ssb.h::ssb_bustype
> SSB_BUSTYPE_SSB,
> SSB_BUSTYPE_PCI,
> SSB_BUSTYPE_PCMCIA,
> SSB_BUSTYPE_SDIO,
> I think first define is confusing. Is sounds like SSB being host for
> SSB, or whatever... I think it should be sth like SSB_BUSTYPE_SYSTEM,
> SSB_BUSTYPE_NONE, SSB_BUSTYPE_NATIVE, SSB_BUSTYPE_EMBEDDED, or sth.

Well, SSB_BUSTYPE_NONE probably is the best choice of all of those.

It should actually be SSB_HOSTBUSTYPE_...

But it's probably not worth changing.

> 3) main.c::ssb_ssb_ops
> We keep all host-specific ops in separated files, but not in this
> case. OK, it makes some sense as this one is not for host, but I think
> it makes main.c more complicated and we can not compile SSB without
> it. I think we could build every PC kernel without this, right?

I think your goal was to reduce complexity? Now you want to increase it
just to save some 20 bytes? Could put it into embedded.c, if you
care.

> 4) scan.c::scan_switchcore
> Why we don't have ops->switchcore? We could get rid of that switch
> with simple pointer.

No. scan.c is special in that almost nothing is initialized, yet.
That is why we have custom read and switch core helpers here.

We do _not_ need core switch as an ops pointer, because after scanning
the core switch is _completely_ hidden in the read/write ops
implementations.

> 5) scan.c::scan_read32
> I'm not sure about this yet... but do we need that here? Shouldn't
> scan.c focus on just scanning? It's just one another "offset +=
> current_coreidx * SSB_CORE_SIZE;" calculation.
> 
> 
> I criticized scan.c::scan_read32, but could you say why do we need
> that specific read at all?

Because ops is not usable, yet. The task of scan is to create the
ssb_devices. The ops expect an ssb_device as argument. We can't
pass an ssb_device that we're about to detect.

-- 
Greetings Michael.