Signed-off-by: Rafał Miłecki <[email protected]>
---
To keep ssb clean and avoid bigger code duplications, my idea is to:
1) Rename ssb directory to bcmb (like Broadcom's bus)
2) Have two *separated* drivers in it: ssb and ai
3) Share common function between ssb and ai
Example of such a common functions can be translating enum to *char and
(re)mapping MMIO.
---
drivers/ssb/Makefile | 2 +-
drivers/ssb/bcmb_scan.c | 182 +++++++++++++++++++++++++++++++++++++++++++++
drivers/ssb/bcmb_scan.h | 13 +++
drivers/ssb/scan.c | 163 ----------------------------------------
drivers/ssb/ssb_private.h | 3 +-
5 files changed, 197 insertions(+), 166 deletions(-)
create mode 100644 drivers/ssb/bcmb_scan.c
create mode 100644 drivers/ssb/bcmb_scan.h
diff --git a/drivers/ssb/Makefile b/drivers/ssb/Makefile
index 656e58b..a91045e 100644
--- a/drivers/ssb/Makefile
+++ b/drivers/ssb/Makefile
@@ -1,5 +1,5 @@
# core
-ssb-y += main.o scan.o
+ssb-y += main.o bcmb_scan.o scan.o
ssb-$(CONFIG_SSB_EMBEDDED) += embedded.o
ssb-$(CONFIG_SSB_SPROM) += sprom.o
diff --git a/drivers/ssb/bcmb_scan.c b/drivers/ssb/bcmb_scan.c
new file mode 100644
index 0000000..17b93f7
--- /dev/null
+++ b/drivers/ssb/bcmb_scan.c
@@ -0,0 +1,182 @@
+/*
+ * Sonics Silicon Backplane
+ * Bus scanning
+ *
+ * Copyright (C) 2005-2007 Michael Buesch <[email protected]>
+ * Copyright (C) 2005 Martin Langer <[email protected]>
+ * Copyright (C) 2005 Stefano Brivio <[email protected]>
+ * Copyright (C) 2005 Danny van Dyk <[email protected]>
+ * Copyright (C) 2005 Andreas Jaggi <[email protected]>
+ * Copyright (C) 2006 Broadcom Corporation.
+ *
+ * Licensed under the GNU/GPL. See COPYING for details.
+ */
+
+#include <linux/ssb/ssb.h>
+#include <linux/pci.h>
+#include <linux/io.h>
+
+#include "bcmb_scan.h"
+
+#include "ssb_private.h"
+
+const char *ssb_core_name(u16 coreid)
+{
+ switch (coreid) {
+ case SSB_DEV_CHIPCOMMON:
+ return "ChipCommon";
+ case SSB_DEV_ILINE20:
+ return "ILine 20";
+ case SSB_DEV_SDRAM:
+ return "SDRAM";
+ case SSB_DEV_PCI:
+ return "PCI";
+ case SSB_DEV_MIPS:
+ return "MIPS";
+ case SSB_DEV_ETHERNET:
+ return "Fast Ethernet";
+ case SSB_DEV_V90:
+ return "V90";
+ case SSB_DEV_USB11_HOSTDEV:
+ return "USB 1.1 Hostdev";
+ case SSB_DEV_ADSL:
+ return "ADSL";
+ case SSB_DEV_ILINE100:
+ return "ILine 100";
+ case SSB_DEV_IPSEC:
+ return "IPSEC";
+ case SSB_DEV_PCMCIA:
+ return "PCMCIA";
+ case SSB_DEV_INTERNAL_MEM:
+ return "Internal Memory";
+ case SSB_DEV_MEMC_SDRAM:
+ return "MEMC SDRAM";
+ case SSB_DEV_EXTIF:
+ return "EXTIF";
+ case SSB_DEV_80211:
+ return "IEEE 802.11";
+ case SSB_DEV_MIPS_3302:
+ return "MIPS 3302";
+ case SSB_DEV_USB11_HOST:
+ return "USB 1.1 Host";
+ case SSB_DEV_USB11_DEV:
+ return "USB 1.1 Device";
+ case SSB_DEV_USB20_HOST:
+ return "USB 2.0 Host";
+ case SSB_DEV_USB20_DEV:
+ return "USB 2.0 Device";
+ case SSB_DEV_SDIO_HOST:
+ return "SDIO Host";
+ case SSB_DEV_ROBOSWITCH:
+ return "Roboswitch";
+ case SSB_DEV_PARA_ATA:
+ return "PATA";
+ case SSB_DEV_SATA_XORDMA:
+ return "SATA XOR-DMA";
+ case SSB_DEV_ETHERNET_GBIT:
+ return "GBit Ethernet";
+ case SSB_DEV_PCIE:
+ return "PCI-E";
+ case SSB_DEV_MIMO_PHY:
+ return "MIMO PHY";
+ case SSB_DEV_SRAM_CTRLR:
+ return "SRAM Controller";
+ case SSB_DEV_MINI_MACPHY:
+ return "Mini MACPHY";
+ case SSB_DEV_ARM_1176:
+ return "ARM 1176";
+ case SSB_DEV_ARM_7TDMI:
+ return "ARM 7TDMI";
+ }
+ return "UNKNOWN";
+}
+
+u32 scan_read32(struct ssb_bus *bus, u8 current_coreidx,
+ u16 offset)
+{
+ u32 lo, hi;
+
+ switch (bus->bustype) {
+ case SSB_BUSTYPE_SSB:
+ offset += current_coreidx * SSB_CORE_SIZE;
+ break;
+ case SSB_BUSTYPE_PCI:
+ break;
+ case SSB_BUSTYPE_PCMCIA:
+ if (offset >= 0x800) {
+ ssb_pcmcia_switch_segment(bus, 1);
+ offset -= 0x800;
+ } else
+ ssb_pcmcia_switch_segment(bus, 0);
+ lo = readw(bus->mmio + offset);
+ hi = readw(bus->mmio + offset + 2);
+ return lo | (hi << 16);
+ case SSB_BUSTYPE_SDIO:
+ offset += current_coreidx * SSB_CORE_SIZE;
+ return ssb_sdio_scan_read32(bus, offset);
+ }
+ return readl(bus->mmio + offset);
+}
+
+int scan_switchcore(struct ssb_bus *bus, u8 coreidx)
+{
+ switch (bus->bustype) {
+ case SSB_BUSTYPE_SSB:
+ break;
+ case SSB_BUSTYPE_PCI:
+ return ssb_pci_switch_coreidx(bus, coreidx);
+ case SSB_BUSTYPE_PCMCIA:
+ return ssb_pcmcia_switch_coreidx(bus, coreidx);
+ case SSB_BUSTYPE_SDIO:
+ return ssb_sdio_scan_switch_coreidx(bus, coreidx);
+ }
+ return 0;
+}
+
+void ssb_iounmap(struct ssb_bus *bus)
+{
+ switch (bus->bustype) {
+ case SSB_BUSTYPE_SSB:
+ case SSB_BUSTYPE_PCMCIA:
+ iounmap(bus->mmio);
+ break;
+ case SSB_BUSTYPE_PCI:
+#ifdef CONFIG_SSB_PCIHOST
+ pci_iounmap(bus->host_pci, bus->mmio);
+#else
+ SSB_BUG_ON(1); /* Can't reach this code. */
+#endif
+ break;
+ case SSB_BUSTYPE_SDIO:
+ break;
+ }
+ bus->mmio = NULL;
+ bus->mapped_device = NULL;
+}
+
+void __iomem *ssb_ioremap(struct ssb_bus *bus, unsigned long baseaddr)
+{
+ void __iomem *mmio = NULL;
+
+ switch (bus->bustype) {
+ case SSB_BUSTYPE_SSB:
+ /* Only map the first core for now. */
+ /* fallthrough... */
+ case SSB_BUSTYPE_PCMCIA:
+ mmio = ioremap(baseaddr, SSB_CORE_SIZE);
+ break;
+ case SSB_BUSTYPE_PCI:
+#ifdef CONFIG_SSB_PCIHOST
+ mmio = pci_iomap(bus->host_pci, 0, ~0UL);
+#else
+ SSB_BUG_ON(1); /* Can't reach this code. */
+#endif
+ break;
+ case SSB_BUSTYPE_SDIO:
+ /* Nothing to ioremap in the SDIO case, just fake it */
+ mmio = (void __iomem *)baseaddr;
+ break;
+ }
+
+ return mmio;
+}
diff --git a/drivers/ssb/bcmb_scan.h b/drivers/ssb/bcmb_scan.h
new file mode 100644
index 0000000..f1fd16c
--- /dev/null
+++ b/drivers/ssb/bcmb_scan.h
@@ -0,0 +1,13 @@
+#ifndef BCMB_SCAN_H_
+#define BCMB_SCAN_H_
+
+extern const char *ssb_core_name(u16 coreid);
+
+extern u32 scan_read32(struct ssb_bus *bus, u8 current_coreidx,
+ u16 offset);
+extern int scan_switchcore(struct ssb_bus *bus, u8 coreidx);
+
+extern void ssb_iounmap(struct ssb_bus *bus);
+extern void __iomem *ssb_ioremap(struct ssb_bus *bus, unsigned long baseaddr);
+
+#endif
diff --git a/drivers/ssb/scan.c b/drivers/ssb/scan.c
index 29884c0..0087795 100644
--- a/drivers/ssb/scan.c
+++ b/drivers/ssb/scan.c
@@ -22,78 +22,6 @@
#include "ssb_private.h"
-
-const char *ssb_core_name(u16 coreid)
-{
- switch (coreid) {
- case SSB_DEV_CHIPCOMMON:
- return "ChipCommon";
- case SSB_DEV_ILINE20:
- return "ILine 20";
- case SSB_DEV_SDRAM:
- return "SDRAM";
- case SSB_DEV_PCI:
- return "PCI";
- case SSB_DEV_MIPS:
- return "MIPS";
- case SSB_DEV_ETHERNET:
- return "Fast Ethernet";
- case SSB_DEV_V90:
- return "V90";
- case SSB_DEV_USB11_HOSTDEV:
- return "USB 1.1 Hostdev";
- case SSB_DEV_ADSL:
- return "ADSL";
- case SSB_DEV_ILINE100:
- return "ILine 100";
- case SSB_DEV_IPSEC:
- return "IPSEC";
- case SSB_DEV_PCMCIA:
- return "PCMCIA";
- case SSB_DEV_INTERNAL_MEM:
- return "Internal Memory";
- case SSB_DEV_MEMC_SDRAM:
- return "MEMC SDRAM";
- case SSB_DEV_EXTIF:
- return "EXTIF";
- case SSB_DEV_80211:
- return "IEEE 802.11";
- case SSB_DEV_MIPS_3302:
- return "MIPS 3302";
- case SSB_DEV_USB11_HOST:
- return "USB 1.1 Host";
- case SSB_DEV_USB11_DEV:
- return "USB 1.1 Device";
- case SSB_DEV_USB20_HOST:
- return "USB 2.0 Host";
- case SSB_DEV_USB20_DEV:
- return "USB 2.0 Device";
- case SSB_DEV_SDIO_HOST:
- return "SDIO Host";
- case SSB_DEV_ROBOSWITCH:
- return "Roboswitch";
- case SSB_DEV_PARA_ATA:
- return "PATA";
- case SSB_DEV_SATA_XORDMA:
- return "SATA XOR-DMA";
- case SSB_DEV_ETHERNET_GBIT:
- return "GBit Ethernet";
- case SSB_DEV_PCIE:
- return "PCI-E";
- case SSB_DEV_MIMO_PHY:
- return "MIMO PHY";
- case SSB_DEV_SRAM_CTRLR:
- return "SRAM Controller";
- case SSB_DEV_MINI_MACPHY:
- return "Mini MACPHY";
- case SSB_DEV_ARM_1176:
- return "ARM 1176";
- case SSB_DEV_ARM_7TDMI:
- return "ARM 7TDMI";
- }
- return "UNKNOWN";
-}
-
static u16 pcidev_to_chipid(struct pci_dev *pci_dev)
{
u16 chipid_fallback = 0;
@@ -157,97 +85,6 @@ static u8 chipid_to_nrcores(u16 chipid)
return 1;
}
-static u32 scan_read32(struct ssb_bus *bus, u8 current_coreidx,
- u16 offset)
-{
- u32 lo, hi;
-
- switch (bus->bustype) {
- case SSB_BUSTYPE_SSB:
- offset += current_coreidx * SSB_CORE_SIZE;
- break;
- case SSB_BUSTYPE_PCI:
- break;
- case SSB_BUSTYPE_PCMCIA:
- if (offset >= 0x800) {
- ssb_pcmcia_switch_segment(bus, 1);
- offset -= 0x800;
- } else
- ssb_pcmcia_switch_segment(bus, 0);
- lo = readw(bus->mmio + offset);
- hi = readw(bus->mmio + offset + 2);
- return lo | (hi << 16);
- case SSB_BUSTYPE_SDIO:
- offset += current_coreidx * SSB_CORE_SIZE;
- return ssb_sdio_scan_read32(bus, offset);
- }
- return readl(bus->mmio + offset);
-}
-
-static int scan_switchcore(struct ssb_bus *bus, u8 coreidx)
-{
- switch (bus->bustype) {
- case SSB_BUSTYPE_SSB:
- break;
- case SSB_BUSTYPE_PCI:
- return ssb_pci_switch_coreidx(bus, coreidx);
- case SSB_BUSTYPE_PCMCIA:
- return ssb_pcmcia_switch_coreidx(bus, coreidx);
- case SSB_BUSTYPE_SDIO:
- return ssb_sdio_scan_switch_coreidx(bus, coreidx);
- }
- return 0;
-}
-
-void ssb_iounmap(struct ssb_bus *bus)
-{
- switch (bus->bustype) {
- case SSB_BUSTYPE_SSB:
- case SSB_BUSTYPE_PCMCIA:
- iounmap(bus->mmio);
- break;
- case SSB_BUSTYPE_PCI:
-#ifdef CONFIG_SSB_PCIHOST
- pci_iounmap(bus->host_pci, bus->mmio);
-#else
- SSB_BUG_ON(1); /* Can't reach this code. */
-#endif
- break;
- case SSB_BUSTYPE_SDIO:
- break;
- }
- bus->mmio = NULL;
- bus->mapped_device = NULL;
-}
-
-static void __iomem *ssb_ioremap(struct ssb_bus *bus,
- unsigned long baseaddr)
-{
- void __iomem *mmio = NULL;
-
- switch (bus->bustype) {
- case SSB_BUSTYPE_SSB:
- /* Only map the first core for now. */
- /* fallthrough... */
- case SSB_BUSTYPE_PCMCIA:
- mmio = ioremap(baseaddr, SSB_CORE_SIZE);
- break;
- case SSB_BUSTYPE_PCI:
-#ifdef CONFIG_SSB_PCIHOST
- mmio = pci_iomap(bus->host_pci, 0, ~0UL);
-#else
- SSB_BUG_ON(1); /* Can't reach this code. */
-#endif
- break;
- case SSB_BUSTYPE_SDIO:
- /* Nothing to ioremap in the SDIO case, just fake it */
- mmio = (void __iomem *)baseaddr;
- break;
- }
-
- return mmio;
-}
-
static int we_support_multiple_80211_cores(struct ssb_bus *bus)
{
/* More than one 802.11 core is only supported by special chips.
diff --git a/drivers/ssb/ssb_private.h b/drivers/ssb/ssb_private.h
index 0331139..929a722 100644
--- a/drivers/ssb/ssb_private.h
+++ b/drivers/ssb/ssb_private.h
@@ -4,6 +4,7 @@
#include <linux/ssb/ssb.h>
#include <linux/types.h>
+#include "bcmb_scan.h"
#define PFX "ssb: "
@@ -156,10 +157,8 @@ static inline int ssb_sdio_init(struct ssb_bus *bus)
/* scan.c */
-extern const char *ssb_core_name(u16 coreid);
extern int ssb_bus_scan(struct ssb_bus *bus,
unsigned long baseaddr);
-extern void ssb_iounmap(struct ssb_bus *ssb);
/* sprom.c */
--
1.7.3.4
> 2011/3/18 George Kashperko <[email protected]>:
> > The clean solution I see here is to have host code apart from
> > backplane-specific processing. The way to accomplish this is obvious as
> > actual host device responsibility is not to switch cores but rather
> > provide mappings into physical address space of the backplane regardless
> > of the target request being made to core, sprom/otp, agent or whatever.
> > This makes much cleaner model than that provided by ssb.
>
> How do you think, what interface would fit for that?
>
> Something like:
> struct host_ops {
> bool (*init)(struct device *dev);
> bool (*exit)(struct device *dev);
> bool (*set_agent_addr)(struct device *dev, u16 addr);
> bool (*set_core_addr)(struct device *dev, u16 addr);
> };
> ?
Well, I see this as following. In generic host life time there are
several states. These states are following:
1. Host just started up, underlying backplane is powered up, we issued
backplane detect/scan. At this point at least some minimal windowed
access is up (if such required), not yet cores/devices are known.
2. Backplane got identified, scanned, individual cores/devices
recognised, buscommon and buscore are registered with kernel to get them
matched with drivers, and then probed and set up.
3. Buscommon and buscore are driven, host can finish with host specific
workarounds, both buscommon and buscore can get their _init entry points
called, we can setup host device irq routine, finally we can expose the
rest cores/devices to kernel.
With that in mind here is my general host ops design pseudo code:
struct host_ops {
/* Init call we should get once both buscommon and buscore drivers are bound (state #3) */
int (*init)(struct bcmb_bus *bus);
/* Regular backplane access ops */
u8 (*read(8|16|32))(struct bcmb_bus *bus, bcmb_addr_t addr);
void (*write(8|16|32))(struct bcmb_bus *bus, bcmb_addr_t addr, u(8|16|32) val);
/* For some theoretically hard-to-set-up before scan hosts we could keep scan_read32 */
u32 scan_read32(struct bcmb_bus *bus, bcmb_addr_t addr);
};
Finally (I'm not yet decided on that - still planning on sertaing
things) I see here a place for one more op - something like
int (*device_fixup)(struct bcmb_bus *bus, u16 ven, u16 id, u8 rev, struct bcmb_device **dev);
This one is supposed to allocate and setup struct bcmb_device at scan
time with optionally extending device list with more devices the host is
confident with (in my code model each core exposes at least 2 devices -
the core agent and the core itself, first one considered as
backplane-private and thus never registered with kernel). Such a fixup
could help to get a clean decision on how to handle host-specific
resourses such as sprom/otp/flash/uarts/etc.
Have nice day,
George
2011/3/18 George Kashperko <[email protected]>:
> The clean solution I see here is to have host code apart from
> backplane-specific processing. The way to accomplish this is obvious as
> actual host device responsibility is not to switch cores but rather
> provide mappings into physical address space of the backplane regardless
> of the target request being made to core, sprom/otp, agent or whatever.
> This makes much cleaner model than that provided by ssb.
How do you think, what interface would fit for that?
Something like:
struct host_ops {
bool (*init)(struct device *dev);
bool (*exit)(struct device *dev);
bool (*set_agent_addr)(struct device *dev, u16 addr);
bool (*set_core_addr)(struct device *dev, u16 addr);
};
?
--
Rafał
> 2011/3/18 George Kashperko <[email protected]>:
> > Well, here you are:
> > 1. PCI hosts with PCI buscores prior rev. 13 (8Kb window)
> > offset size type description
> > 0x0000 0x1000 sliding controlled with BAR0_WIN1 register
> > 0x1000 0x0800 fixed SPROM
> > 0x1800 0x0400 fixed PCI core (core registers 0x0000 to 0x03FF range)
> > 0x1C00 0x0400 fixed PCI core (agent registers, 0x0C00 to 0x0FFF range)
> >
> > 2. PCI hosts with PCI buscores rev. 13 and above and PCIE with
> > Chipcommon rev. up to 31 (16Kb window)
> > offset size type description
> > 0x0000 0x1000 sliding controlled with BAR0_WIN1 register
> > 0x1000 0x1000 fixed SPROM
> > 0x2000 0x1000 fixed PCI core
> > 0x3000 0x1000 fixed Chipcommon core
> >
> > 3. PCIE hosts with Chipcommon rev. 32 and above
> > offset size type description
> > 0x0000 0x1000 sliding controlled with BAR0_WIN1 register
> > 0x1000 0x1000 sliding controlled with BAR0_WIN2 register
> > 0x2000 0x1000 fixed PCI core
> > 0x3000 0x1000 fixed Chipcommon core
> >
> > As you can see even if PCI_REVISION can't feed us with guaranteed choice
> > of either of these 3 layouts we can distingiush between them easily like
> > following:
> >
> > if (pci_is_pcie(dev)) {
> > u32 chipid = ioread32(bar0_base + 0x3000);
> > if (chipid & 0x10000000)
> > /* AXI, Chipcommon rev. is 32+ */
> > goto win_3_setup;
> > else
> > /* SB, Chipcommon rev. is <= 31 */
> > goto win_2_setup;
> > } else {
> > u32 idhi = ioread32(bar0_base + 0x1C00 + 0x03FC);
> > if (((idhi & 0x00008FF0) >> 4) == 0x804)
> > /* PCI core id */
> > goto win_1_setup;
> > else
> > /* Some crap from SPROM area */
> > goto win_2_setup;
> > }
> >
> > Therefore we can setup PCI(e) host windows for backplane access prior to
> > scanning.
> >
> > As for BAR0_WIN[12] registers, they point to physical base address on
> > backplane that will be mapped into corresponding 0x1000-size window. And
> > here again registers don't care what exactly you want to see there. Both
> > the windows can be controlled independently.
> >
> > Finally, you might noticed we don't have SPROM in last 3rd layout. Thats
> > because it is in Chipcommon registers' space. For layouts #1 & #2 BAR0
> > range 0x1000 to 0x2000 is either mapped to actual SPROM or to SPROM
> > shadow in PCI core.
>
> Wohoo, and this is second part of info I really needed, thanks a lot!
> I think there are mistakes in it, but I just wanted to get the idea of
> windows. So thanks a lot.
>
> As for mistakes:
> 1) The split is not 1-31 vs. 32-... I believe it is 1-30 vs. 31-...
> 2) On chipco >= 31 SSB SPROM seems to be 0x800
> Maybe sth more...
>
Watch bcmsrom.c for #define SROM_OFFSET
For Chipcommons rev. >31 (the same as >=32) SPROM is in Chipcommon at
0x0800 offset - 0x3800 offset of BAR0 (if CAP_SPROM capability bit is
set), otherwise SPROM is unavailable.
For Chipcommons rev. <=31 (the same as <32) SPROM is mapped to 0x1000
offset of BAR0.
Have nice day,
George
> 2011/3/18 Rafał Miłecki <[email protected]>:
> > What for example about pci.c? Which functions from that file won't be
> > duplicated in totally separated?
>
> At first I though we will need to duplicate all pci routines like
> ssb_pci_switch_coreidx and ssb_pci_xtal. But now I've checked
> brcm80211 and it seems it reads SPROM even from AI bus-cards.
Btw ssb_pci_xtal is for PCI. PCIE hosts don't need this.
>
> Do you really want to duplicate all the SPROM code in *totally
> separated* driver for AI?!
>
Every sb/ai backplane known to me at the moment are featuring the
following hardware design:
System backplane with individual devices on it (called cores)
communicating with the means of agents. The agents for sb are in the
main core registers space, and apart from the core registers for axi.
The backplane itself is "mastered" by buscore device. This is mips core
for embeddables, pci(e) core for pci(e) hosted backplanes, pcmcia core
for backplanes on pcmcia/sdio. Might OCP core is some sort of buscore as
well used to bridge two sb backplanes.
Buscore is responsible for interrupts management, backplane-to-host-bus
operations, agent-to-agent transfers.
Apart from the buscore, there is another "special" device on the
backplane - buscommon. Unlike buscore this one seems to be optional, and
not present on some old pcmcia-hosted backplanes. The buscommon is
responsible for managing bus clocks, can serve uarts, also is a source
of misc. configuration information for the backplane. These buscommons
are chipcommon and extif cores.
Here it would be great to have more technical background on the subject
but unfortunately apart from the staging brcm80211 and GPL packages for
respective embeddables the only open doc on the subject available to me
is http://www.broadcom.com/collateral/pg/440X-PG02-R.pdf
So software model I see here looks like following:
* Backplane-type handler responsible for
~ initial scanning;
~ agent-specific operations (core enable/disable, irq flags management,
etc.);
* The bus driver itself responsible for initial detection and assignment
of backplane handler and also managig driver registration/binding/etc
for
~ buscommon;
~ buscore;
~ regular cores;
* Host driver managing:
~ requests to the physical address space of backplane;
~ host interrupt management;
~ host-specific workarounds (ssb_pci_xtal is one of them);
This requires generic interfaces for:
* host (like those ssb_bus_ops which are actually not bus but host ops -
handling not core accesses but physical backplane addresses requests;
iterrupt management ops);
* backplane (scan, enable, disable, irq_flag etc.);
* buscore (backplane irq/errors/etc. management);
* buscommon (backplane clocks/etc. management, capabilities queries);
Buscommon and buscore unlike current ssb model could be separate
drivers. This will help to break apart all that mess of versions
checking and revision-specific processing. This will provide clean way
of obsoleting and removing the support for old hardware and introducing
newer one.
Regular bus core devices thus are to be registered with linux once
buscommon, buscore and host drivers are bound and set up making the bus
operational.
Buscore and buscommon as separate drivers will require some code to be
replicated over close versions but overall I've already tested this
approach with chipcommons with pmu r0, r1 and r5 on pcie and mips hosts
and final drivers' code is clean and manageable unlike all that mess in
hndpmu.c
Same stands for mips/pci host cores.
Well, keep in mind its my own view on the things how they are to be done
right :)
Have nice day,
George
2011/3/18 George Kashperko <[email protected]>:
> My english is awful therefore seems we missunderstood each other. I'm
> sure I got your point right - you plan to start up the new (bcmb)
> project for both sb and ai support.
No, no, no. My bcmb is not expected to be project for both buses. It
is expected to provide helper functions for *two separated* drivers:
ai and ssb.
I don't want to duplicate ssb_core_name (already implemented in ssb)
in my ai. That way I put it in bcmb_scan.c. I can now use
ssb_core_name in both: ssb and ai without double, duplicated
implementation.
I'm still reading your earlier e-mail...
--
Rafał
2011/3/18 George Kashperko <[email protected]>:
>> 2011/3/18 Rafał Miłecki <[email protected]>:
>> > What for example about pci.c? Which functions from that file won't be
>> > duplicated in totally separated?
>>
>> At first I though we will need to duplicate all pci routines like
>> ssb_pci_switch_coreidx and ssb_pci_xtal. But now I've checked
>> brcm80211 and it seems it reads SPROM even from AI bus-cards.
> Btw ssb_pci_xtal is for PCI. PCIE hosts don't need this.
Not even all PCI devices. Just 4306 I believe:
/* kludge to enable the clock on the 4306 which lacks a slowclock */
if (bustype == PCI_BUS && !si_ispcie(sii))
si_clkctl_xtal(&sii->pub, XTAL | PLL, ON);
>> Do you really want to duplicate all the SPROM code in *totally
>> separated* driver for AI?!
>>
> Every sb/ai backplane known to me at the moment are featuring the
> following hardware design:
> System backplane with individual devices on it (called cores)
> communicating with the means of agents. The agents for sb are in the
> main core registers space, and apart from the core registers for axi.
> The backplane itself is "mastered" by buscore device. This is mips core
> for embeddables, pci(e) core for pci(e) hosted backplanes, pcmcia core
> for backplanes on pcmcia/sdio. Might OCP core is some sort of buscore as
> well used to bridge two sb backplanes.
> Buscore is responsible for interrupts management, backplane-to-host-bus
> operations, agent-to-agent transfers.
>
> Apart from the buscore, there is another "special" device on the
> backplane - buscommon. Unlike buscore this one seems to be optional, and
> not present on some old pcmcia-hosted backplanes. The buscommon is
> responsible for managing bus clocks, can serve uarts, also is a source
> of misc. configuration information for the backplane. These buscommons
> are chipcommon and extif cores.
>
> Here it would be great to have more technical background on the subject
> but unfortunately apart from the staging brcm80211 and GPL packages for
> respective embeddables the only open doc on the subject available to me
> is http://www.broadcom.com/collateral/pg/440X-PG02-R.pdf
Well, OK, it's generally well known already. Maybe I just prefer to
call "buscore" a "hostcore" and "buscommon" a "chipcommon". I believe
that are the names we hot used to.
> So software model I see here looks like following:
> * Backplane-type handler responsible for
> ~ initial scanning;
> ~ agent-specific operations (core enable/disable, irq flags management,
> etc.);
>
> * The bus driver itself responsible for initial detection and assignment
> of backplane handler and also managig driver registration/binding/etc
> for
> ~ buscommon;
> ~ buscore;
> ~ regular cores;
>
> * Host driver managing:
> ~ requests to the physical address space of backplane;
> ~ host interrupt management;
> ~ host-specific workarounds (ssb_pci_xtal is one of them);
>
> This requires generic interfaces for:
> * host (like those ssb_bus_ops which are actually not bus but host ops -
> handling not core accesses but physical backplane addresses requests;
> iterrupt management ops);
> * backplane (scan, enable, disable, irq_flag etc.);
> * buscore (backplane irq/errors/etc. management);
> * buscommon (backplane clocks/etc. management, capabilities queries);
>
> Buscommon and buscore unlike current ssb model could be separate
> drivers. This will help to break apart all that mess of versions
> checking and revision-specific processing. This will provide clean way
> of obsoleting and removing the support for old hardware and introducing
> newer one.
> Regular bus core devices thus are to be registered with linux once
> buscommon, buscore and host drivers are bound and set up making the bus
> operational.
> Buscore and buscommon as separate drivers will require some code to be
> replicated over close versions but overall I've already tested this
> approach with chipcommons with pmu r0, r1 and r5 on pcie and mips hosts
> and final drivers' code is clean and manageable unlike all that mess in
> hndpmu.c
> Same stands for mips/pci host cores.
OK, I generally agree. We can try moving to such a layout. The only
thing I hate in your view is "obsoleting and removing the support for
old hardware".
Answer me this question, please:
Why do you think my proposed patch conflicts with layout proposed by you?
You said you want to have "Backplane-type handler". So according to
this we will need two drivers/handlers: 1 for scanning SSB and 1 for
scanning AI. That's what I try to implement. I just want to share some
functions between the one for SSB and the one for AI.
I don't see the point where my patch is in conflict with your idea.
--
Rafał
On 03/18/2011 11:25 AM, Rafał Miłecki wrote:
> 2011/3/18 George Kashperko<[email protected]>:
>>> 2011/3/18 Rafał Miłecki<[email protected]>:
>>>> What for example about pci.c? Which functions from that file won't be
>>>> duplicated in totally separated?
>>>
>>> At first I though we will need to duplicate all pci routines like
>>> ssb_pci_switch_coreidx and ssb_pci_xtal. But now I've checked
>>> brcm80211 and it seems it reads SPROM even from AI bus-cards.
>> Btw ssb_pci_xtal is for PCI. PCIE hosts don't need this.
>
> Not even all PCI devices. Just 4306 I believe:
> /* kludge to enable the clock on the 4306 which lacks a slowclock */
> if (bustype == PCI_BUS&& !si_ispcie(sii))
> si_clkctl_xtal(&sii->pub, XTAL | PLL, ON);
>
>
>>> Do you really want to duplicate all the SPROM code in *totally
>>> separated* driver for AI?!
>>>
>> Every sb/ai backplane known to me at the moment are featuring the
>> following hardware design:
>> System backplane with individual devices on it (called cores)
>> communicating with the means of agents. The agents for sb are in the
>> main core registers space, and apart from the core registers for axi.
>> The backplane itself is "mastered" by buscore device. This is mips core
>> for embeddables, pci(e) core for pci(e) hosted backplanes, pcmcia core
>> for backplanes on pcmcia/sdio. Might OCP core is some sort of buscore as
>> well used to bridge two sb backplanes.
>> Buscore is responsible for interrupts management, backplane-to-host-bus
>> operations, agent-to-agent transfers.
>>
>> Apart from the buscore, there is another "special" device on the
>> backplane - buscommon. Unlike buscore this one seems to be optional, and
>> not present on some old pcmcia-hosted backplanes. The buscommon is
>> responsible for managing bus clocks, can serve uarts, also is a source
>> of misc. configuration information for the backplane. These buscommons
>> are chipcommon and extif cores.
>>
>> Here it would be great to have more technical background on the subject
>> but unfortunately apart from the staging brcm80211 and GPL packages for
>> respective embeddables the only open doc on the subject available to me
>> is http://www.broadcom.com/collateral/pg/440X-PG02-R.pdf
>
> Well, OK, it's generally well known already. Maybe I just prefer to
> call "buscore" a "hostcore" and "buscommon" a "chipcommon". I believe
> that are the names we hot used to.
>
>
>> So software model I see here looks like following:
>> * Backplane-type handler responsible for
>> ~ initial scanning;
>> ~ agent-specific operations (core enable/disable, irq flags management,
>> etc.);
>>
>> * The bus driver itself responsible for initial detection and assignment
>> of backplane handler and also managig driver registration/binding/etc
>> for
>> ~ buscommon;
>> ~ buscore;
>> ~ regular cores;
>>
>> * Host driver managing:
>> ~ requests to the physical address space of backplane;
>> ~ host interrupt management;
>> ~ host-specific workarounds (ssb_pci_xtal is one of them);
>>
>> This requires generic interfaces for:
>> * host (like those ssb_bus_ops which are actually not bus but host ops -
>> handling not core accesses but physical backplane addresses requests;
>> iterrupt management ops);
>> * backplane (scan, enable, disable, irq_flag etc.);
>> * buscore (backplane irq/errors/etc. management);
>> * buscommon (backplane clocks/etc. management, capabilities queries);
>>
>> Buscommon and buscore unlike current ssb model could be separate
>> drivers. This will help to break apart all that mess of versions
>> checking and revision-specific processing. This will provide clean way
>> of obsoleting and removing the support for old hardware and introducing
>> newer one.
>> Regular bus core devices thus are to be registered with linux once
>> buscommon, buscore and host drivers are bound and set up making the bus
>> operational.
>> Buscore and buscommon as separate drivers will require some code to be
>> replicated over close versions but overall I've already tested this
>> approach with chipcommons with pmu r0, r1 and r5 on pcie and mips hosts
>> and final drivers' code is clean and manageable unlike all that mess in
>> hndpmu.c
>> Same stands for mips/pci host cores.
>
> OK, I generally agree. We can try moving to such a layout. The only
> thing I hate in your view is "obsoleting and removing the support for
> old hardware".
I also do not like the thought of removing support of old hardware. Given the
lack of support from some vendors, Linux may be somewhat slow in providing
drivers for new devices; however, we can and should keep the support for legacy
devices. I built a sandbox computer just for testing a BCM4306/3 that uses b43
and a BCM4303 using b43legacy.
Larry
> 2011/3/18 George Kashperko <[email protected]>:
> > - read pci revision id/vendor id (pci_read_config_word), if required
> > to decide if second window is sliding or sprom (actually rev_id should
> > be sufficient for that) read offset 0x0000 from bar0 offset to get
> > chipid register value
>
> Could you say something about WIN1 and WIN2? What are they for, can we
> use them somehow separately? What do you mean by second window being
> SPROM? We read sprom without any special window magic:
> sprom[i] = ioread16(bus->mmio + bus->sprom_offset + (i * 2));
>
> My understanding is limited to that:
> If we want to use device with CORE_ADDR and WRAP_ADDR we have to write
> CORE_ADDR to BAR0_WIN and WRAP_ADDR to BAR0_WIN2. Then we can use
> readl/writel.
>
Historically layout of BAR0 for Broadcom pci(e) hosts differs depending
on pci(e) and chipcommon cores revisions. Unfortunately there are no
info from Broadcom on the host dev id, pci/chipco core revisions
correspondance but unlikely that we could have different hosts with same
PCI_REVISION but different sets of pci/chipco cores. Here I with my poor
english mean that pci host identification obtainable with
pci_read_config_dword PCI_REVISION should be sufficient to distinguish
between these BAR0 layouts.
Well, here you are:
1. PCI hosts with PCI buscores prior rev. 13 (8Kb window)
offset size type description
0x0000 0x1000 sliding controlled with BAR0_WIN1 register
0x1000 0x0800 fixed SPROM
0x1800 0x0400 fixed PCI core (core registers 0x0000 to 0x03FF range)
0x1C00 0x0400 fixed PCI core (agent registers, 0x0C00 to 0x0FFF range)
2. PCI hosts with PCI buscores rev. 13 and above and PCIE with
Chipcommon rev. up to 31 (16Kb window)
offset size type description
0x0000 0x1000 sliding controlled with BAR0_WIN1 register
0x1000 0x1000 fixed SPROM
0x2000 0x1000 fixed PCI core
0x3000 0x1000 fixed Chipcommon core
3. PCIE hosts with Chipcommon rev. 32 and above
offset size type description
0x0000 0x1000 sliding controlled with BAR0_WIN1 register
0x1000 0x1000 sliding controlled with BAR0_WIN2 register
0x2000 0x1000 fixed PCI core
0x3000 0x1000 fixed Chipcommon core
As you can see even if PCI_REVISION can't feed us with guaranteed choice
of either of these 3 layouts we can distingiush between them easily like
following:
if (pci_is_pcie(dev)) {
u32 chipid = ioread32(bar0_base + 0x3000);
if (chipid & 0x10000000)
/* AXI, Chipcommon rev. is 32+ */
goto win_3_setup;
else
/* SB, Chipcommon rev. is <= 31 */
goto win_2_setup;
} else {
u32 idhi = ioread32(bar0_base + 0x1C00 + 0x03FC);
if (((idhi & 0x00008FF0) >> 4) == 0x804)
/* PCI core id */
goto win_1_setup;
else
/* Some crap from SPROM area */
goto win_2_setup;
}
Therefore we can setup PCI(e) host windows for backplane access prior to
scanning.
As for BAR0_WIN[12] registers, they point to physical base address on
backplane that will be mapped into corresponding 0x1000-size window. And
here again registers don't care what exactly you want to see there. Both
the windows can be controlled independently.
Finally, you might noticed we don't have SPROM in last 3rd layout. Thats
because it is in Chipcommon registers' space. For layouts #1 & #2 BAR0
range 0x1000 to 0x2000 is either mapped to actual SPROM or to SPROM
shadow in PCI core.
Have nice day,
George
> 2011/3/18 George Kashperko <[email protected]>:
> > Current ssb code ideology isn't really good place to start with support
> > for ai backplanes. Its really great to see you willing to get ai
> > supported in mainline but I'm sure this should be done apart from the
> > ssb code and not even with one as the design decisions origin. Several
> > concepts the ssb is based on are not designed to support anything else
> > than ocp/sb and will require workarounds to suport ai.
> > Thanks to Michael I had a time to think over the possibly code
> > abstraction for shared sb and ai support. And while I'm still sure the
> > patchwork for ai over ssb support is of good use as some intermediate
> > decision to support ai-based hardware in sertain distributions but now I
> > support Michael in that such (hopefully) a temporary buildups should not
> > be in mainline.
>
> Please, give some concrete arguments, which part of design does not match AI.
SSB design is core-centric, where all the bus activities are made in
regard to the cores. This mostly is correct as most of the time
bus/drivers code work with cores. At the same time ssb model completely
ignores the fact that each sb backplane core is a composite device
consisting of the core itself and the core agent. In sb the agent
registers space is referenced by manuals available and the Broadcom SDK
code as one starting at 0x0F00 offset from the main core registers space
beginning (actually I beleive this is true for sb revisions prior to 2.3
and this range is actually 0x0C00-0x0FFF at least for sb rev. 2.3 and
above). This is neglible for embedded backplanes as the whole backplane
physical address space is accessed simultaneously, completely ignored by
the ssb code for pci-hosted sb backplanes as only sliding bar0 window #1
is utilised for backplane access, workarounded with
select_core_and_segment in pcmcia host code.
With axi introduced we have agents registers' spaces in completely
different physical backplane pages, apart from the main core registers
space. This is still have no impact for embeddables, will require bar0
window #2 adjustment with each core switch (and here you will be
required to have backplane specific processing in pci host code) and I
don't really know how this could be workarounded for other hosts without
them to know which exactly backplane they run and how should it be
treaten.
The clean solution I see here is to have host code apart from
backplane-specific processing. The way to accomplish this is obvious as
actual host device responsibility is not to switch cores but rather
provide mappings into physical address space of the backplane regardless
of the target request being made to core, sprom/otp, agent or whatever.
This makes much cleaner model than that provided by ssb.
>
> My patch has shown we need to duplicate 40% of SSB's scanning code.
> What for example about pci.c? Which functions from that file won't be
> duplicated in totally separated?
>
Going further in extending ssb to support ai will either require the
host to be confident of the backplane type and layout (see
drivers/staging/brcm80211/utils for the good example of messup it will
lead to) or will reguire the backplane-specific code to break into the
host code (brcm80211/utils again).
Current ssb code is two-layer - in two words its about cores on host.
This makes sertain hosts to know everyting about cores, but at the same
time it let the cores to be abstract from the hosts. Building ai support
over this model will make hosts to know even more about cores, but if
done right it will still keep the cores autonomous.
At the same time host-backplane-core model provide better code
separation, little-to-nothing code duplication and let host, backplane
and core code writers avoid workarounds required for current ssb model.
Have nice day,
George
2011/3/18 George Kashperko <[email protected]>:
> Well, here you are:
> 1. PCI hosts with PCI buscores prior rev. 13 (8Kb window)
> offset size type description
> 0x0000 0x1000 sliding controlled with BAR0_WIN1 register
> 0x1000 0x0800 fixed SPROM
> 0x1800 0x0400 fixed PCI core (core registers 0x0000 to 0x03FF range)
> 0x1C00 0x0400 fixed PCI core (agent registers, 0x0C00 to 0x0FFF range)
>
> 2. PCI hosts with PCI buscores rev. 13 and above and PCIE with
> Chipcommon rev. up to 31 (16Kb window)
> offset size type description
> 0x0000 0x1000 sliding controlled with BAR0_WIN1 register
> 0x1000 0x1000 fixed SPROM
> 0x2000 0x1000 fixed PCI core
> 0x3000 0x1000 fixed Chipcommon core
>
> 3. PCIE hosts with Chipcommon rev. 32 and above
> offset size type description
> 0x0000 0x1000 sliding controlled with BAR0_WIN1 register
> 0x1000 0x1000 sliding controlled with BAR0_WIN2 register
> 0x2000 0x1000 fixed PCI core
> 0x3000 0x1000 fixed Chipcommon core
>
> As you can see even if PCI_REVISION can't feed us with guaranteed choice
> of either of these 3 layouts we can distingiush between them easily like
> following:
>
> if (pci_is_pcie(dev)) {
> u32 chipid = ioread32(bar0_base + 0x3000);
> if (chipid & 0x10000000)
> /* AXI, Chipcommon rev. is 32+ */
> goto win_3_setup;
> else
> /* SB, Chipcommon rev. is <= 31 */
> goto win_2_setup;
> } else {
> u32 idhi = ioread32(bar0_base + 0x1C00 + 0x03FC);
> if (((idhi & 0x00008FF0) >> 4) == 0x804)
> /* PCI core id */
> goto win_1_setup;
> else
> /* Some crap from SPROM area */
> goto win_2_setup;
> }
>
> Therefore we can setup PCI(e) host windows for backplane access prior to
> scanning.
>
> As for BAR0_WIN[12] registers, they point to physical base address on
> backplane that will be mapped into corresponding 0x1000-size window. And
> here again registers don't care what exactly you want to see there. Both
> the windows can be controlled independently.
>
> Finally, you might noticed we don't have SPROM in last 3rd layout. Thats
> because it is in Chipcommon registers' space. For layouts #1 & #2 BAR0
> range 0x1000 to 0x2000 is either mapped to actual SPROM or to SPROM
> shadow in PCI core.
Wohoo, and this is second part of info I really needed, thanks a lot!
I think there are mistakes in it, but I just wanted to get the idea of
windows. So thanks a lot.
As for mistakes:
1) The split is not 1-31 vs. 32-... I believe it is 1-30 vs. 31-...
2) On chipco >= 31 SSB SPROM seems to be 0x800
Maybe sth more...
--
Rafał
2011/3/18 George Kashperko <[email protected]>:
> Current ssb code ideology isn't really good place to start with support
> for ai backplanes. Its really great to see you willing to get ai
> supported in mainline but I'm sure this should be done apart from the
> ssb code and not even with one as the design decisions origin. Several
> concepts the ssb is based on are not designed to support anything else
> than ocp/sb and will require workarounds to suport ai.
> Thanks to Michael I had a time to think over the possibly code
> abstraction for shared sb and ai support. And while I'm still sure the
> patchwork for ai over ssb support is of good use as some intermediate
> decision to support ai-based hardware in sertain distributions but now I
> support Michael in that such (hopefully) a temporary buildups should not
> be in mainline.
Please, give some concrete arguments, which part of design does not match AI.
My patch has shown we need to duplicate 40% of SSB's scanning code.
What for example about pci.c? Which functions from that file won't be
duplicated in totally separated?
--
Rafał
2011/3/18 George Kashperko <[email protected]>:
>> I don't see the point where my patch is in conflict with your idea.
> Not your patch, just scan_read32, scan_switchcore, ssb_iounmap,
> ssb_ioremap. So far ssb_core_name is fine.
> Yet again, ssb code model is core-sentric which is not actually true
> from the hardware pint of view. With few workarounds it works fine for
> sb. For axi we will have more of these.
>
> scan_read32, scan_switchcore, ssb_iounmap, ssb_ioremap are all
> host-driven services. In ssb model host is operating cores and therefore
> requires all that switch, scan_read, map, unmap etc. While with model
> where host is providing not core but backplane access the only routine
> among those above you need is scan_read32 and nothing more (actually you
> don't need it too as host ops with read(8|16|32) are completely
> sufficient for scanning). The purpose of scan_read32 for ssb model is
> that ssb_bus_ops are operating cores which you don't have at the moment
> when scanning. With host ops designed as u32 (*read32)(struct host
> *host, phys_addr_t addr) host will do what it supposed to - manage its
> means of backplane physical access and will have absolutely no need to
> break into understanding what the core/sprom/otp/etc. is.
That's what I wanted to hear from the beginning, thanks! :)
The reason I implemented scan_switchcore is early stage, when I
prepare bus for reading EROM:
erombase = ai_scan_read32(bus, 0, SSB_CHIPCO_EROM);
pci_write_config_dword(bus->host_pci, PCI_BAR0_WIN, erombase);
In above you can see my hack which works for PCI host only. This hack
I wanted to replace with scan_switchcore and that functions seems to
perform exactly this operation.
AFAIU we need to adjust PCI_BAR0_WIN and PCI_BAR0_WIN2 later, when we
already have info about core from EROM. In early stage we can not use
that.
Anyway with what you said now, I'll re-read your mails again, to fully
understand your POV.
--
Rafał
2011/3/18 George Kashperko <[email protected]>:
> Well, I see this as following. In generic host life time there are
> several states. These states are following:
> 1. Host just started up, underlying backplane is powered up, we issued
> backplane detect/scan. At this point at least some minimal windowed
> access is up (if such required), not yet cores/devices are known.
> 2. Backplane got identified, scanned, individual cores/devices
> recognised, buscommon and buscore are registered with kernel to get them
> matched with drivers, and then probed and set up.
> 3. Buscommon and buscore are driven, host can finish with host specific
> workarounds, both buscommon and buscore can get their _init entry points
> called, we can setup host device irq routine, finally we can expose the
> rest cores/devices to kernel.
>
> With that in mind here is my general host ops design pseudo code:
> struct host_ops {
> /* Init call we should get once both buscommon and buscore drivers are bound (state #3) */
> int (*init)(struct bcmb_bus *bus);
>
> /* Regular backplane access ops */
> u8 (*read(8|16|32))(struct bcmb_bus *bus, bcmb_addr_t addr);
> void (*write(8|16|32))(struct bcmb_bus *bus, bcmb_addr_t addr, u(8|16|32) val);
>
> /* For some theoretically hard-to-set-up before scan hosts we could keep scan_read32 */
> u32 scan_read32(struct bcmb_bus *bus, bcmb_addr_t addr);
> };
I don't think it makes much sense. If init is going to be called in
state #3, what about some pre-init? set_master, request_region, xtal,
iomap?
--
Rafał
2011/3/19 George Kashperko <[email protected]>:
>
>> 2011/3/18 George Kashperko <[email protected]>:
>> > Well, here you are:
>> > 1. PCI hosts with PCI buscores prior rev. 13 (8Kb window)
>> > offset size type description
>> > 0x0000 0x1000 sliding controlled with BAR0_WIN1 register
>> > 0x1000 0x0800 fixed SPROM
>> > 0x1800 0x0400 fixed PCI core (core registers 0x0000 to 0x03FF range)
>> > 0x1C00 0x0400 fixed PCI core (agent registers, 0x0C00 to 0x0FFF range)
>> >
>> > 2. PCI hosts with PCI buscores rev. 13 and above and PCIE with
>> > Chipcommon rev. up to 31 (16Kb window)
>> > offset size type description
>> > 0x0000 0x1000 sliding controlled with BAR0_WIN1 register
>> > 0x1000 0x1000 fixed SPROM
>> > 0x2000 0x1000 fixed PCI core
>> > 0x3000 0x1000 fixed Chipcommon core
>> >
>> > 3. PCIE hosts with Chipcommon rev. 32 and above
>> > offset size type description
>> > 0x0000 0x1000 sliding controlled with BAR0_WIN1 register
>> > 0x1000 0x1000 sliding controlled with BAR0_WIN2 register
>> > 0x2000 0x1000 fixed PCI core
>> > 0x3000 0x1000 fixed Chipcommon core
>> >
>> > As you can see even if PCI_REVISION can't feed us with guaranteed choice
>> > of either of these 3 layouts we can distingiush between them easily like
>> > following:
>> >
>> > if (pci_is_pcie(dev)) {
>> > u32 chipid = ioread32(bar0_base + 0x3000);
>> > if (chipid & 0x10000000)
>> > /* AXI, Chipcommon rev. is 32+ */
>> > goto win_3_setup;
>> > else
>> > /* SB, Chipcommon rev. is <= 31 */
>> > goto win_2_setup;
>> > } else {
>> > u32 idhi = ioread32(bar0_base + 0x1C00 + 0x03FC);
>> > if (((idhi & 0x00008FF0) >> 4) == 0x804)
>> > /* PCI core id */
>> > goto win_1_setup;
>> > else
>> > /* Some crap from SPROM area */
>> > goto win_2_setup;
>> > }
>> >
>> > Therefore we can setup PCI(e) host windows for backplane access prior to
>> > scanning.
>> >
>> > As for BAR0_WIN[12] registers, they point to physical base address on
>> > backplane that will be mapped into corresponding 0x1000-size window. And
>> > here again registers don't care what exactly you want to see there. Both
>> > the windows can be controlled independently.
>> >
>> > Finally, you might noticed we don't have SPROM in last 3rd layout. Thats
>> > because it is in Chipcommon registers' space. For layouts #1 & #2 BAR0
>> > range 0x1000 to 0x2000 is either mapped to actual SPROM or to SPROM
>> > shadow in PCI core.
>>
>> Wohoo, and this is second part of info I really needed, thanks a lot!
>> I think there are mistakes in it, but I just wanted to get the idea of
>> windows. So thanks a lot.
>>
>> As for mistakes:
>> 1) The split is not 1-31 vs. 32-... I believe it is 1-30 vs. 31-...
>> 2) On chipco >= 31 SSB SPROM seems to be 0x800
>> Maybe sth more...
>>
> Watch bcmsrom.c for #define SROM_OFFSET
>
> For Chipcommons rev. >31 (the same as >=32) SPROM is in Chipcommon at
> 0x0800 offset - 0x3800 offset of BAR0 (if CAP_SPROM capability bit is
> set), otherwise SPROM is unavailable.
> For Chipcommons rev. <=31 (the same as <32) SPROM is mapped to 0x1000
> offset of BAR0.
I guess we may have errors in:
http://bcm-v4.sipsolutions.net/SPROM
http://bcm-v4.sipsolutions.net/802.11/IsSpromAvailable
then.
--
Rafał
> 2011/3/18 George Kashperko <[email protected]>:
> >> I don't see the point where my patch is in conflict with your idea.
> > Not your patch, just scan_read32, scan_switchcore, ssb_iounmap,
> > ssb_ioremap. So far ssb_core_name is fine.
> > Yet again, ssb code model is core-sentric which is not actually true
> > from the hardware pint of view. With few workarounds it works fine for
> > sb. For axi we will have more of these.
> >
> > scan_read32, scan_switchcore, ssb_iounmap, ssb_ioremap are all
> > host-driven services. In ssb model host is operating cores and therefore
> > requires all that switch, scan_read, map, unmap etc. While with model
> > where host is providing not core but backplane access the only routine
> > among those above you need is scan_read32 and nothing more (actually you
> > don't need it too as host ops with read(8|16|32) are completely
> > sufficient for scanning). The purpose of scan_read32 for ssb model is
> > that ssb_bus_ops are operating cores which you don't have at the moment
> > when scanning. With host ops designed as u32 (*read32)(struct host
> > *host, phys_addr_t addr) host will do what it supposed to - manage its
> > means of backplane physical access and will have absolutely no need to
> > break into understanding what the core/sprom/otp/etc. is.
>
> That's what I wanted to hear from the beginning, thanks! :)
>
> The reason I implemented scan_switchcore is early stage, when I
> prepare bus for reading EROM:
> erombase = ai_scan_read32(bus, 0, SSB_CHIPCO_EROM);
> pci_write_config_dword(bus->host_pci, PCI_BAR0_WIN, erombase);
> In above you can see my hack which works for PCI host only. This hack
> I wanted to replace with scan_switchcore and that functions seems to
> perform exactly this operation.
>
> AFAIU we need to adjust PCI_BAR0_WIN and PCI_BAR0_WIN2 later, when we
> already have info about core from EROM. In early stage we can not use
> that.
>
> Anyway with what you said now, I'll re-read your mails again, to fully
> understand your POV.
You still look through the prism of the ssb model. There is absolutely
nothing which prevents PCI(e) host driver to setup windowed access right
away at the _probe before even trying to feed the bcmb code with any
data for detection/scanning/etc. For more generic implementation
scan_read32 still seems to be useful one, but again you can build the
code to use regular host-provided read(8|16|32) ops for the same purpose
as initial scan_read32 as soon as they provide access in regard to
backplane physicall addresses, not cores. As for switch_core - yet again
its host service, and only windowed accesses' hosts. And in any case
host have absolutely _no_ reason to operate cores. The only thigns the
host should care of are:
~ provide access to backplane physicall address range;
~ manage buscore (mostly that part which is on the host bus side);
~ manage host bus interrupt(s);
Sample generic host startup:
* probe
~ initial host device power up and setup (that part of buscore core that
interconnected to host bus the host is at). For e. g. pci(e) its:
- pci_enable_device
- pci_request_regions
- pci_set_master
- pci_iomap
- powerup (like xtal for pci)
- host-specific workarounds (like ldo_war)
~ setup windowed access
- read pci revision id/vendor id (pci_read_config_word), if required
to decide if second window is sliding or sprom (actually rev_id should
be sufficient for that) read offset 0x0000 from bar0 offset to get
chipid register value
- at this point host have all the required information to decide on
amount on host-to-backplane windows available and their configuration;
- call some bcmb_add_bus to get bus detected/scanned/whatever.
At this point scanning code is good to go with any of
bcmb_scan_read32(struct hnd_host *host, phys_addr_t addr) or
bcmb_read32(struct hnd_host *host, phys_addr_t addr) as both of them do
exactly the same.
Either bcmb_scan_read32/bcmb_read32 need window switching or not depends
on the host implementation and is part of the host code. Generic
scanning/detection/etc. code have absolutely nothing to do with window
switching. They are there in ssb because of the design decisions made
towards core-sentric implementation. Desicions which I'm sure should not
blind you while planning for new sb/ai support code.
e. g. for embeddable read implementation will be like following pseudo
code
u32 bcmb_read32(struct hnd_host *host, phys_addr_t addr)
{
return ioread32(phys_addr_to_embedded(host, addr));
}
while for e. g. pci it could be something like next (again not a real
code, just a general idea):
u32 bcmb_read32(struct hnd_host *host, phys_addr_t addr)
{
struct pci_win *win;
u32;
win = phys_addr_to_pci_win(host, addr);
claim_win(win);
tmp = ioread(phys_addr_to_win(win, addr);
release_win(win);
return tmp;
}
Have nice day,
George
2011/3/18 George Kashperko <[email protected]>:
>> 2011/3/18 George Kashperko <[email protected]>:
>> > Current ssb code ideology isn't really good place to start with support
>> > for ai backplanes. Its really great to see you willing to get ai
>> > supported in mainline but I'm sure this should be done apart from the
>> > ssb code and not even with one as the design decisions origin. Several
>> > concepts the ssb is based on are not designed to support anything else
>> > than ocp/sb and will require workarounds to suport ai.
>> > Thanks to Michael I had a time to think over the possibly code
>> > abstraction for shared sb and ai support. And while I'm still sure the
>> > patchwork for ai over ssb support is of good use as some intermediate
>> > decision to support ai-based hardware in sertain distributions but now I
>> > support Michael in that such (hopefully) a temporary buildups should not
>> > be in mainline.
>>
>> Please, give some concrete arguments, which part of design does not match AI.
> SSB design is core-centric, where all the bus activities are made in
> regard to the cores. This mostly is correct as most of the time
> bus/drivers code work with cores.
So finally, is there anything wrong about that?
> At the same time ssb model completely
> ignores the fact that each sb backplane core is a composite device
> consisting of the core itself and the core agent. In sb the agent
> registers space is referenced by manuals available and the Broadcom SDK
> code as one starting at 0x0F00 offset from the main core registers space
> beginning (actually I beleive this is true for sb revisions prior to 2.3
> and this range is actually 0x0C00-0x0FFF at least for sb rev. 2.3 and
> above). This is neglible for embedded backplanes as the whole backplane
> physical address space is accessed simultaneously, completely ignored by
> the ssb code for pci-hosted sb backplanes as only sliding bar0 window #1
> is utilised for backplane access, workarounded with
> select_core_and_segment in pcmcia host code.
> With axi introduced we have agents registers' spaces in completely
> different physical backplane pages, apart from the main core registers
> space. This is still have no impact for embeddables, will require bar0
> window #2 adjustment with each core switch (and here you will be
> required to have backplane specific processing in pci host code) and I
> don't really know how this could be workarounded for other hosts without
> them to know which exactly backplane they run and how should it be
> treaten.
> The clean solution I see here is to have host code apart from
> backplane-specific processing. The way to accomplish this is obvious as
> actual host device responsibility is not to switch cores but rather
> provide mappings into physical address space of the backplane regardless
> of the target request being made to core, sprom/otp, agent or whatever.
> This makes much cleaner model than that provided by ssb.
Yes, I noticed this already. Sill, after analyzing pci.c it seems we
will duplicate ssb_pci_xtal, ssb_pci_switch_coreidx and all SPROM
functions. We will just have different ssb_pci_switch_core and we will
drop old SPROMs layouts support.
My idea: separate functions we can share (ssb_pci_xtal,
ssb_pci_switch_coreidx, most of the SPROM functions) and create
separated PCI host drivers.
>> My patch has shown we need to duplicate 40% of SSB's scanning code.
>> What for example about pci.c? Which functions from that file won't be
>> duplicated in totally separated?
>>
> Going further in extending ssb to support ai will either require the
> host to be confident of the backplane type and layout (see
> drivers/staging/brcm80211/utils for the good example of messup it will
> lead to) or will reguire the backplane-specific code to break into the
> host code (brcm80211/utils again).
I don't want to extend ssb, I said that with my patch message. I
wanted separated drivers sharing some functions, please take a look at
my comment included in patch. I think we could also create *separated*
host drivers sharing most of the code.
> Current ssb code is two-layer - in two words its about cores on host.
> This makes sertain hosts to know everyting about cores, but at the same
> time it let the cores to be abstract from the hosts. Building ai support
> over this model will make hosts to know even more about cores, but if
> done right it will still keep the cores autonomous.
> At the same time host-backplane-core model provide better code
> separation, little-to-nothing code duplication and let host, backplane
> and core code writers avoid workarounds required for current ssb model.
--
Rafał
В Птн, 18/03/2011 в 23:42 +0100, Rafał Miłecki пишет:
> 2011/3/18 George Kashperko <[email protected]>:
> > Well, I see this as following. In generic host life time there are
> > several states. These states are following:
> > 1. Host just started up, underlying backplane is powered up, we issued
> > backplane detect/scan. At this point at least some minimal windowed
> > access is up (if such required), not yet cores/devices are known.
> > 2. Backplane got identified, scanned, individual cores/devices
> > recognised, buscommon and buscore are registered with kernel to get them
> > matched with drivers, and then probed and set up.
> > 3. Buscommon and buscore are driven, host can finish with host specific
> > workarounds, both buscommon and buscore can get their _init entry points
> > called, we can setup host device irq routine, finally we can expose the
> > rest cores/devices to kernel.
> >
> > With that in mind here is my general host ops design pseudo code:
> > struct host_ops {
> > /* Init call we should get once both buscommon and buscore drivers are bound (state #3) */
> > int (*init)(struct bcmb_bus *bus);
> >
> > /* Regular backplane access ops */
> > u8 (*read(8|16|32))(struct bcmb_bus *bus, bcmb_addr_t addr);
> > void (*write(8|16|32))(struct bcmb_bus *bus, bcmb_addr_t addr, u(8|16|32) val);
> >
> > /* For some theoretically hard-to-set-up before scan hosts we could keep scan_read32 */
> > u32 scan_read32(struct bcmb_bus *bus, bcmb_addr_t addr);
> > };
>
> I don't think it makes much sense. If init is going to be called in
> state #3, what about some pre-init? set_master, request_region, xtal,
> iomap?
>
These are already done by host-device driver in its _probe.
After you done with that you alloc your host-specific bcmb_pci_host
struct and call something like bcmb_add_bus(host, &my_host_ops);
Have nice day,
George
> 2011/3/18 George Kashperko <[email protected]>:
> >> 2011/3/18 Rafał Miłecki <[email protected]>:
> >> > What for example about pci.c? Which functions from that file won't be
> >> > duplicated in totally separated?
> >>
> >> At first I though we will need to duplicate all pci routines like
> >> ssb_pci_switch_coreidx and ssb_pci_xtal. But now I've checked
> >> brcm80211 and it seems it reads SPROM even from AI bus-cards.
> > Btw ssb_pci_xtal is for PCI. PCIE hosts don't need this.
>
> Not even all PCI devices. Just 4306 I believe:
> /* kludge to enable the clock on the 4306 which lacks a slowclock */
> if (bustype == PCI_BUS && !si_ispcie(sii))
> si_clkctl_xtal(&sii->pub, XTAL | PLL, ON);
More likely it was introduced with 4306 and was required for all
successor up until pcie.
> Well, OK, it's generally well known already. Maybe I just prefer to
> call "buscore" a "hostcore" and "buscommon" a "chipcommon". I believe
> that are the names we hot used to.
There are number of backplanes with Extif (bcm44xx and bcm4710 e. g.),
while even more with chipcommon. Well, its a matter of taste ofcourse,
just think that naming both chipcommon and ext. interface cores
"chipcommon" could be the source for missunderstandings.
> > So software model I see here looks like following:
> > * Backplane-type handler responsible for
> > ~ initial scanning;
> > ~ agent-specific operations (core enable/disable, irq flags management,
> > etc.);
> >
> > * The bus driver itself responsible for initial detection and assignment
> > of backplane handler and also managig driver registration/binding/etc
> > for
> > ~ buscommon;
> > ~ buscore;
> > ~ regular cores;
> >
> > * Host driver managing:
> > ~ requests to the physical address space of backplane;
> > ~ host interrupt management;
> > ~ host-specific workarounds (ssb_pci_xtal is one of them);
> >
> > This requires generic interfaces for:
> > * host (like those ssb_bus_ops which are actually not bus but host ops -
> > handling not core accesses but physical backplane addresses requests;
> > iterrupt management ops);
> > * backplane (scan, enable, disable, irq_flag etc.);
> > * buscore (backplane irq/errors/etc. management);
> > * buscommon (backplane clocks/etc. management, capabilities queries);
> >
> > Buscommon and buscore unlike current ssb model could be separate
> > drivers. This will help to break apart all that mess of versions
> > checking and revision-specific processing. This will provide clean way
> > of obsoleting and removing the support for old hardware and introducing
> > newer one.
> > Regular bus core devices thus are to be registered with linux once
> > buscommon, buscore and host drivers are bound and set up making the bus
> > operational.
> > Buscore and buscommon as separate drivers will require some code to be
> > replicated over close versions but overall I've already tested this
> > approach with chipcommons with pmu r0, r1 and r5 on pcie and mips hosts
> > and final drivers' code is clean and manageable unlike all that mess in
> > hndpmu.c
> > Same stands for mips/pci host cores.
>
> OK, I generally agree. We can try moving to such a layout. The only
> thing I hate in your view is "obsoleting and removing the support for
> old hardware".
Whats wrong with this one ? :)
The point is - each system part can be obsoleted and removed at some
point. Can and must or should are kinda different ones.
At the moment I see it rather interesting to support as much hardware as
possible with new model. Might I change my mind in lets say 5-6 years.
In either case separate buscommon/buscore drivers each ~10Kb size looks
much more manageable than 40-70Kb monolithick one - take a look at
hndpmu.c or driver_chipcommon.c + driver_chipcommon_pmu.c
The whole purpose of the model I drawn is to fit most (in the best case
all) the existing sb/ai hardware into single well designed abstraction
not requiring model quirks and workarounds unlike current ssb code is.
>
> Answer me this question, please:
> Why do you think my proposed patch conflicts with layout proposed by you?
>
> You said you want to have "Backplane-type handler". So according to
> this we will need two drivers/handlers: 1 for scanning SSB and 1 for
> scanning AI. That's what I try to implement. I just want to share some
> functions between the one for SSB and the one for AI.
I dont see here much difference between backplane type handler and
separate sb and ai backplane drivers. Actually its almost the same as in
either case final purpose of backplane (either driver or handler) will
be agent management abstraction with some ops or dedicated bcmb_sb_... /
bcmb_ai_... routines.
>
> I don't see the point where my patch is in conflict with your idea.
Not your patch, just scan_read32, scan_switchcore, ssb_iounmap,
ssb_ioremap. So far ssb_core_name is fine.
Yet again, ssb code model is core-sentric which is not actually true
from the hardware pint of view. With few workarounds it works fine for
sb. For axi we will have more of these.
scan_read32, scan_switchcore, ssb_iounmap, ssb_ioremap are all
host-driven services. In ssb model host is operating cores and therefore
requires all that switch, scan_read, map, unmap etc. While with model
where host is providing not core but backplane access the only routine
among those above you need is scan_read32 and nothing more (actually you
don't need it too as host ops with read(8|16|32) are completely
sufficient for scanning). The purpose of scan_read32 for ssb model is
that ssb_bus_ops are operating cores which you don't have at the moment
when scanning. With host ops designed as u32 (*read32)(struct host
*host, phys_addr_t addr) host will do what it supposed to - manage its
means of backplane physical access and will have absolutely no need to
break into understanding what the core/sprom/otp/etc. is.
SSB is the great project from all the sides I've looked at it. Just a
perfect solution considering no sources of knowledge for sb other than
reverse engineering. But planning new code with the model based on ssb
will get us back to the point we are - in order to support new hardware
we must introduce new workarounds.
I'm pretty much sure it should be completely separate solution, well
planned and thought of.
Have nice day,
George
> 2011/3/18 George Kashperko <[email protected]>:
> >> 2011/3/18 George Kashperko <[email protected]>:
> >> > Current ssb code ideology isn't really good place to start with support
> >> > for ai backplanes. Its really great to see you willing to get ai
> >> > supported in mainline but I'm sure this should be done apart from the
> >> > ssb code and not even with one as the design decisions origin. Several
> >> > concepts the ssb is based on are not designed to support anything else
> >> > than ocp/sb and will require workarounds to suport ai.
> >> > Thanks to Michael I had a time to think over the possibly code
> >> > abstraction for shared sb and ai support. And while I'm still sure the
> >> > patchwork for ai over ssb support is of good use as some intermediate
> >> > decision to support ai-based hardware in sertain distributions but now I
> >> > support Michael in that such (hopefully) a temporary buildups should not
> >> > be in mainline.
> >>
> >> Please, give some concrete arguments, which part of design does not match AI.
> > SSB design is core-centric, where all the bus activities are made in
> > regard to the cores. This mostly is correct as most of the time
> > bus/drivers code work with cores.
>
> So finally, is there anything wrong about that?
Nothing wrong as soon as technically imperfect solution is not intended
to be in mainline :) Otherwise my latest AI RFC already ready to merge.
> >> My patch has shown we need to duplicate 40% of SSB's scanning code.
> >> What for example about pci.c? Which functions from that file won't be
> >> duplicated in totally separated?
> >>
> > Going further in extending ssb to support ai will either require the
> > host to be confident of the backplane type and layout (see
> > drivers/staging/brcm80211/utils for the good example of messup it will
> > lead to) or will reguire the backplane-specific code to break into the
> > host code (brcm80211/utils again).
>
> I don't want to extend ssb, I said that with my patch message. I
> wanted separated drivers sharing some functions, please take a look at
> my comment included in patch. I think we could also create *separated*
> host drivers sharing most of the code.
My english is awful therefore seems we missunderstood each other. I'm
sure I got your point right - you plan to start up the new (bcmb)
project for both sb and ai support.
My point here is - this is great but making design decisions on ssb code
model is wrong.
Have nice day,
George
2011/3/18 George Kashperko <[email protected]>:
> - read pci revision id/vendor id (pci_read_config_word), if required
> to decide if second window is sliding or sprom (actually rev_id should
> be sufficient for that) read offset 0x0000 from bar0 offset to get
> chipid register value
Could you say something about WIN1 and WIN2? What are they for, can we
use them somehow separately? What do you mean by second window being
SPROM? We read sprom without any special window magic:
sprom[i] = ioread16(bus->mmio + bus->sprom_offset + (i * 2));
My understanding is limited to that:
If we want to use device with CORE_ADDR and WRAP_ADDR we have to write
CORE_ADDR to BAR0_WIN and WRAP_ADDR to BAR0_WIN2. Then we can use
readl/writel.
--
Rafał
> Signed-off-by: Rafał Miłecki <[email protected]>
> ---
> To keep ssb clean and avoid bigger code duplications, my idea is to:
> 1) Rename ssb directory to bcmb (like Broadcom's bus)
> 2) Have two *separated* drivers in it: ssb and ai
> 3) Share common function between ssb and ai
> Example of such a common functions can be translating enum to *char and
> (re)mapping MMIO.
Current ssb code ideology isn't really good place to start with support
for ai backplanes. Its really great to see you willing to get ai
supported in mainline but I'm sure this should be done apart from the
ssb code and not even with one as the design decisions origin. Several
concepts the ssb is based on are not designed to support anything else
than ocp/sb and will require workarounds to suport ai.
Thanks to Michael I had a time to think over the possibly code
abstraction for shared sb and ai support. And while I'm still sure the
patchwork for ai over ssb support is of good use as some intermediate
decision to support ai-based hardware in sertain distributions but now I
support Michael in that such (hopefully) a temporary buildups should not
be in mainline.
Have nice day,
George
2011/3/18 Rafał Miłecki <[email protected]>:
> What for example about pci.c? Which functions from that file won't be
> duplicated in totally separated?
At first I though we will need to duplicate all pci routines like
ssb_pci_switch_coreidx and ssb_pci_xtal. But now I've checked
brcm80211 and it seems it reads SPROM even from AI bus-cards.
Do you really want to duplicate all the SPROM code in *totally
separated* driver for AI?!
--
Rafał