Re: [PATCH 5/9] PCI: host: brcmstb: add dma-ranges for inbound traffic
From: Jim Quinlan
Date: Thu Oct 12 2017 - 17:43:23 EST
On Thu, Oct 12, 2017 at 2:04 PM, Robin Murphy <robin.murphy@xxxxxxx> wrote:
> [+DMA API maintainers]
>
> On 11/10/17 23:34, Jim Quinlan wrote:
>> The Broadcom STB PCIe host controller is intimately related to the
>> memory subsystem. This close relationship adds complexity to how cpu
>> system memory is mapped to PCIe memory. Ideally, this mapping is an
>> identity mapping, or an identity mapping off by a constant. Not so in
>> this case.
>>
>> Consider the Broadcom reference board BCM97445LCC_4X8 which has 6 GB
>> of system memory. Here is how the PCIe controller maps the
>> system memory to PCIe memory:
>>
>> memc0-a@[ 0....3fffffff] <=> pci@[ 0....3fffffff]
>> memc0-b@[100000000...13fffffff] <=> pci@[ 40000000....7fffffff]
>> memc1-a@[ 40000000....7fffffff] <=> pci@[ 80000000....bfffffff]
>> memc1-b@[300000000...33fffffff] <=> pci@[ c0000000....ffffffff]
>> memc2-a@[ 80000000....bfffffff] <=> pci@[100000000...13fffffff]
>> memc2-b@[c00000000...c3fffffff] <=> pci@[140000000...17fffffff]
>>
>> Although there are some "gaps" that can be added between the
>> individual mappings by software, the permutation of memory regions for
>> the most part is fixed by HW. The solution of having something close
>> to an identity mapping is not possible.
>>
>> The idea behind this HW design is that the same PCIe module can
>> act as an RC or EP, and if it acts as an EP it concatenates all
>> of system memory into a BAR so anything can be accessed. Unfortunately,
>> when the PCIe block is in the role of an RC it also presents this
>> "BAR" to downstream PCIe devices, rather than offering an identity map
>> between its system memory and PCIe space.
>>
>> Suppose that an endpoint driver allocs some DMA memory. Suppose this
>> memory is located at 0x6000_0000, which is in the middle of memc1-a.
>> The driver wants a dma_addr_t value that it can pass on to the EP to
>> use. Without doing any custom mapping, the EP will use this value for
>> DMA: the driver will get a dma_addr_t equal to 0x6000_0000. But this
>> won't work; the device needs a dma_addr_t that reflects the PCIe space
>> address, namely 0xa000_0000.
>>
>> So, essentially the solution to this problem must modify the
>> dma_addr_t returned by the DMA routines routines. There are two
>> ways (I know of) of doing this:
>>
>> (a) overriding/redefining the dma_to_phys() and phys_to_dma() calls
>> that are used by the dma_ops routines. This is the approach of
>>
>> arch/mips/cavium-octeon/dma-octeon.c
>>
>> In ARM and ARM64 these two routines are defiend in asm/dma-mapping.h
>> as static inline functions.
>>
>> (b) Subscribe to a notifier that notifies when a device is added to a
>> bus. When this happens, set_dma_ops() can be called for the device.
>> This method is mentioned in:
>>
>> http://lxr.free-electrons.com/source/drivers/of/platform.c?v=3.16#L152
>>
>> where it says as a comment
>>
>> "In case if platform code need to use own special DMA
>> configuration, it can use Platform bus notifier and
>> handle BUS_NOTIFY_ADD_DEVICE event to fix up DMA
>> configuration."
>>
>> Solution (b) is what this commit does. It uses the native dma_ops
>> as the base set of operations, and overrides some with custom
>> functions that translate the address and then call the base
>> function.
>>
>> Signed-off-by: Jim Quinlan <jim2101024@xxxxxxxxx>
>> ---
>> drivers/pci/host/Makefile | 3 +-
>> drivers/pci/host/pci-brcmstb-dma.c | 219 +++++++++++++++++++++++++++++++++++++
>> drivers/pci/host/pci-brcmstb.c | 150 +++++++++++++++++++++++--
>> drivers/pci/host/pci-brcmstb.h | 7 ++
>> 4 files changed, 368 insertions(+), 11 deletions(-)
>> create mode 100644 drivers/pci/host/pci-brcmstb-dma.c
>>
>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>> index 4398d2c..c283321 100644
>> --- a/drivers/pci/host/Makefile
>> +++ b/drivers/pci/host/Makefile
>> @@ -21,7 +21,8 @@ obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o
>> obj-$(CONFIG_PCIE_MEDIATEK) += pcie-mediatek.o
>> obj-$(CONFIG_PCIE_TANGO_SMP8759) += pcie-tango.o
>> obj-$(CONFIG_VMD) += vmd.o
>> -obj-$(CONFIG_PCI_BRCMSTB) += pci-brcmstb.o
>> +obj-$(CONFIG_PCI_BRCMSTB) += brcmstb-pci.o
>> +brcmstb-pci-objs := pci-brcmstb.o pci-brcmstb-dma.o
>>
>> # The following drivers are for devices that use the generic ACPI
>> # pci_root.c driver but don't support standard ECAM config access.
>> diff --git a/drivers/pci/host/pci-brcmstb-dma.c b/drivers/pci/host/pci-brcmstb-dma.c
>> new file mode 100644
>> index 0000000..81ce122
>> --- /dev/null
>> +++ b/drivers/pci/host/pci-brcmstb-dma.c
>> @@ -0,0 +1,219 @@
>> +/*
>> + * Copyright (C) 2015-2017 Broadcom
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +#include <linux/dma-mapping.h>
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/of_address.h>
>> +#include <linux/pci.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/smp.h>
>> +
>> +#include "pci-brcmstb.h"
>> +
>> +static const struct dma_map_ops *arch_dma_ops;
>> +static struct dma_map_ops brcm_dma_ops;
>> +
>> +static void *brcm_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
>> + gfp_t gfp, unsigned long attrs)
>> +{
>> + void *ret;
>> +
>> + ret = arch_dma_ops->alloc(dev, size, handle, gfp, attrs);
>> + if (ret)
>> + *handle = brcm_to_pci(*handle);
>> + return ret;
>> +}
>> +
>> +static void brcm_dma_free(struct device *dev, size_t size, void *cpu_addr,
>> + dma_addr_t handle, unsigned long attrs)
>> +{
>> + handle = brcm_to_cpu(handle);
>> + arch_dma_ops->free(dev, size, cpu_addr, handle, attrs);
>> +}
>> +
>> +static int brcm_dma_mmap(struct device *dev, struct vm_area_struct *vma,
>> + void *cpu_addr, dma_addr_t dma_addr, size_t size,
>> + unsigned long attrs)
>> +{
>> + dma_addr = brcm_to_cpu(dma_addr);
>> + return arch_dma_ops->mmap(dev, vma, cpu_addr, dma_addr, size, attrs);
>> +}
>> +
>> +static int brcm_dma_get_sgtable(struct device *dev, struct sg_table *sgt,
>> + void *cpu_addr, dma_addr_t handle, size_t size,
>> + unsigned long attrs)
>> +{
>> + handle = brcm_to_cpu(handle);
>> + return arch_dma_ops->get_sgtable(dev, sgt, cpu_addr, handle, size,
>> + attrs);
>> +}
>> +
>> +static dma_addr_t brcm_dma_map_page(struct device *dev, struct page *page,
>> + unsigned long offset, size_t size,
>> + enum dma_data_direction dir,
>> + unsigned long attrs)
>> +{
>> + return brcm_to_pci(arch_dma_ops->map_page(dev, page, offset, size,
>> + dir, attrs));
>> +}
>> +
>> +static void brcm_dma_unmap_page(struct device *dev, dma_addr_t handle,
>> + size_t size, enum dma_data_direction dir,
>> + unsigned long attrs)
>> +{
>> + handle = brcm_to_cpu(handle);
>> + arch_dma_ops->unmap_page(dev, handle, size, dir, attrs);
>> +}
>> +
>> +static int brcm_dma_map_sg(struct device *dev, struct scatterlist *sgl,
>> + int nents, enum dma_data_direction dir,
>> + unsigned long attrs)
>> +{
>> + int ret, i;
>> + struct scatterlist *sg;
>> +
>> + ret = arch_dma_ops->map_sg(dev, sgl, nents, dir, attrs);
>> + /* The ARM and MIPS implementations of map_sg and unmap_sg
>> + * make calls to ops->map_page(), which we already intercept.
>> + * The ARM64 does not, so we must iterate through the SG list
>> + * and convert each dma_address to one that is compatible
>> + * with our PCI RC implementation.
>> + */
>
> That's a pretty fragile assumption given that arch code is free to
> change, and anyone doing so is unlikely to be aware of your driver.
> You'd be better off implementing these in terms of {brcm_dma_}map_page
> directly.
>
Will fix.
>> + if (IS_ENABLED(CONFIG_ARM64))
>> + for_each_sg(sgl, sg, ret, i)
>> + sg->dma_address = brcm_to_pci(sg->dma_address);
>> + return ret;
>> +}
>> +
>> +static void brcm_dma_unmap_sg(struct device *dev,
>> + struct scatterlist *sgl, int nents,
>> + enum dma_data_direction dir,
>> + unsigned long attrs)
>> +{
>> + int i;
>> + struct scatterlist *sg;
>> +
>> + /* The ARM and MIPS implementations of map_sg and unmap_sg
>> + * make calls to ops->map_page(), which we already intercept.
>> + * The ARM64 does not, so we must iterate through the SG list
>> + * and convert each dma_address to one that is compatible
>> + * with our PCI RC implementation.
>> + */
>> + if (IS_ENABLED(CONFIG_ARM64))
>> + for_each_sg(sgl, sg, nents, i)
>> + sg->dma_address = brcm_to_cpu(sg->dma_address);
>> + arch_dma_ops->map_sg(dev, sgl, nents, dir, attrs);
>> +}
>> +
>> +static void brcm_dma_sync_single_for_cpu(struct device *dev,
>> + dma_addr_t handle, size_t size,
>> + enum dma_data_direction dir)
>> +{
>> + handle = brcm_to_cpu(handle);
>> + arch_dma_ops->sync_single_for_cpu(dev, handle, size, dir);
>> +}
>> +
>> +static void brcm_dma_sync_single_for_device(struct device *dev,
>> + dma_addr_t handle, size_t size,
>> + enum dma_data_direction dir)
>> +{
>> + handle = brcm_to_cpu(handle);
>> + arch_dma_ops->sync_single_for_device(dev, handle, size, dir);
>> +}
>
> And sync_sg_*()? They might not be that commonly used by in-tree
> drivers, but who knows what lurks beyond?
>
Will fix.
>> +
>> +static dma_addr_t brcm_dma_map_resource(struct device *dev, phys_addr_t phys,
>> + size_t size,
>> + enum dma_data_direction dir,
>> + unsigned long attrs)
>> +{
>> + return brcm_to_pci(arch_dma_ops->map_resource
>> + (dev, phys, size, dir, attrs));
>> +}
>> +
>> +static void brcm_dma_unmap_resource(struct device *dev, dma_addr_t handle,
>> + size_t size, enum dma_data_direction dir,
>> + unsigned long attrs)
>> +{
>> + handle = brcm_to_cpu(handle);
>> + arch_dma_ops->unmap_resource(dev, handle, size, dir, attrs);
>> +}
>> +
>> +static int brcm_mapping_error(struct device *dev, dma_addr_t dma_addr)
>> +{
>> + return dma_addr == BRCMSTB_ERROR_CODE;
>
> Huh? How do you know this will work correctly with every platform's
> ->map_page implementation (hint: it doesn't).
>
Will fix.
>> +}
>> +
>> +static const struct dma_map_ops *brcm_get_arch_dma_ops(struct device *dev)
>> +{
>> +#if defined(CONFIG_MIPS)
>> + return mips_dma_map_ops;
>> +#elif defined(CONFIG_ARM)
>> + return &arm_dma_ops;
>> +#elif defined(CONFIG_ARM64)
>> + /* swiotlb_dma_ops is a static var, so we get ahold
>> + * of it by calling arch_setup_dma_ops(...).
>> + */
>> + arch_setup_dma_ops(dev, 0, 0, NULL, false);
>> + return dev->dma_ops;
>> +#endif
>> + return 0;
>> +}
>
> As mentioned earlier, no. There are theoretical cases where it might not
> be true, but in practice you can assume all PCI devices are going to get
> the same DMA ops as their associated host controller (and that's still a
> better assumption that what you have here), so you can just grab those
> at the point you install the notifier.
>
Yes, for some reason I did not know of get_dma_ops() and I wrote my
own, poorly. Will fix.
>> +
>> +static void brcm_set_dma_ops(struct device *dev)
>> +{
>> + arch_dma_ops = brcm_get_arch_dma_ops(dev);
>> + if (!arch_dma_ops)
>> + return;
>> +
>> + /* Set all of the base operations; some will be overridden */
>> + brcm_dma_ops = *arch_dma_ops;
>> +
>> + /* Insert the Brcm-specific override operations */
>> + brcm_dma_ops.alloc = brcm_dma_alloc;
>> + brcm_dma_ops.free = brcm_dma_free;
>> + brcm_dma_ops.mmap = brcm_dma_mmap;
>> + brcm_dma_ops.get_sgtable = brcm_dma_get_sgtable;
>> + brcm_dma_ops.map_page = brcm_dma_map_page;
>> + brcm_dma_ops.unmap_page = brcm_dma_unmap_page;
>> + brcm_dma_ops.sync_single_for_cpu = brcm_dma_sync_single_for_cpu;
>> + brcm_dma_ops.sync_single_for_device = brcm_dma_sync_single_for_device;
>> + brcm_dma_ops.map_sg = brcm_dma_map_sg;
>> + brcm_dma_ops.unmap_sg = brcm_dma_unmap_sg;
>> + if (arch_dma_ops->map_resource)
>> + brcm_dma_ops.map_resource = brcm_dma_map_resource;
>> + if (arch_dma_ops->unmap_resource)
>> + brcm_dma_ops.unmap_resource = brcm_dma_unmap_resource;
>> + brcm_dma_ops.mapping_error = brcm_mapping_error;
>> +
>> + /* Use our brcm_dma_ops for this driver */
>> + set_dma_ops(dev, &brcm_dma_ops);
>> +}
>
> Just have a static const set of ops like everyone else - you can handle
> the conditionality of ->{map,unmap}_resource inside the brcm_* wrappers.
>
Will fix.
>> +
>> +static int brcmstb_platform_notifier(struct notifier_block *nb,
>> + unsigned long event, void *__dev)
>> +{
>> + struct device *dev = __dev;
>> +
>> + if (event != BUS_NOTIFY_ADD_DEVICE)
>> + return NOTIFY_DONE;
>> +
>> + brcm_set_dma_ops(dev);
>> + return NOTIFY_OK;
>> +}
>> +
>> +struct notifier_block brcmstb_platform_nb = {
>> + .notifier_call = brcmstb_platform_notifier,
>> +};
>> +EXPORT_SYMBOL(brcmstb_platform_nb);
>> diff --git a/drivers/pci/host/pci-brcmstb.c b/drivers/pci/host/pci-brcmstb.c
>> index f4cd6e7..03c0da9 100644
>> --- a/drivers/pci/host/pci-brcmstb.c
>> +++ b/drivers/pci/host/pci-brcmstb.c
>> @@ -343,6 +343,8 @@ struct brcm_pcie {
>>
>> static struct list_head brcm_pcie = LIST_HEAD_INIT(brcm_pcie);
>> static phys_addr_t scb_size[BRCM_MAX_SCB];
>> +static struct of_pci_range *dma_ranges;
>> +static int num_dma_ranges;
>> static int num_memc;
>> static DEFINE_MUTEX(brcm_pcie_lock);
>>
>> @@ -362,6 +364,8 @@ static int brcm_pcie_add_controller(struct brcm_pcie *pcie)
>> {
>> mutex_lock(&brcm_pcie_lock);
>> snprintf(pcie->name, sizeof(pcie->name) - 1, "PCIe%d", pcie->id);
>> + if (list_empty(&brcm_pcie))
>> + bus_register_notifier(&pci_bus_type, &brcmstb_platform_nb);
>> list_add_tail(&pcie->list, &brcm_pcie);
>> mutex_unlock(&brcm_pcie_lock);
>>
>> @@ -378,8 +382,14 @@ static void brcm_pcie_remove_controller(struct brcm_pcie *pcie)
>> tmp = list_entry(pos, struct brcm_pcie, list);
>> if (tmp == pcie) {
>> list_del(pos);
>> - if (list_empty(&brcm_pcie))
>> + if (list_empty(&brcm_pcie)) {
>> + bus_unregister_notifier(&pci_bus_type,
>> + &brcmstb_platform_nb);
>> + kfree(dma_ranges);
>> + dma_ranges = NULL;
>> + num_dma_ranges = 0;
>> num_memc = 0;
>> + }
>> break;
>> }
>> }
>> @@ -403,6 +413,35 @@ int encode_ibar_size(u64 size)
>> return 0;
>> }
>>
>> +dma_addr_t brcm_to_pci(dma_addr_t addr)
>> +{
>> + struct of_pci_range *p;
>> +
>> + if (!num_dma_ranges)
>> + return addr;
>> +
>> + for (p = dma_ranges; p < &dma_ranges[num_dma_ranges]; p++)
>> + if (addr >= p->cpu_addr && addr < (p->cpu_addr + p->size))
>> + return addr - p->cpu_addr + p->pci_addr;
>> +
>> + return BRCMSTB_ERROR_CODE;
>> +}
>> +EXPORT_SYMBOL(brcm_to_pci);
>
> AFAICS it doesn't make much sense for anyone outside this driver to ever
> be calling these.
>
Will fix.
>> +dma_addr_t brcm_to_cpu(dma_addr_t addr)
>> +{
>> + struct of_pci_range *p;
>> +
>> + if (!num_dma_ranges)
>> + return addr;
>> + for (p = dma_ranges; p < &dma_ranges[num_dma_ranges]; p++)
>> + if (addr >= p->pci_addr && addr < (p->pci_addr + p->size))
>> + return addr - p->pci_addr + p->cpu_addr;
>> +
>> + return addr;
>> +}
>> +EXPORT_SYMBOL(brcm_to_cpu);
>> +
>> static u32 mdio_form_pkt(int port, int regad, int cmd)
>> {
>> u32 pkt = 0;
>> @@ -652,6 +691,74 @@ static int brcm_parse_ranges(struct brcm_pcie *pcie)
>> return 0;
>> }
>>
>> +static int brcm_pci_dma_range_parser_init(struct of_pci_range_parser *parser,
>> + struct device_node *node)
>> +{
>> + const int na = 3, ns = 2;
>> + int rlen;
>> +
>> + parser->node = node;
>> + parser->pna = of_n_addr_cells(node);
>> + parser->np = parser->pna + na + ns;
>> +
>> + parser->range = of_get_property(node, "dma-ranges", &rlen);
>> + if (!parser->range)
>> + return -ENOENT;
>> +
>> + parser->end = parser->range + rlen / sizeof(__be32);
>> +
>> + return 0;
>> +}
>
> Note that we've got a factored-out helper for this queued in -next
> already - see here:
>
> https://patchwork.kernel.org/patch/9927541/
>
I will submit a change when it gets merged.
>> +
>> +static int brcm_parse_dma_ranges(struct brcm_pcie *pcie)
>> +{
>> + int i, ret = 0;
>> + struct of_pci_range_parser parser;
>> + struct device_node *dn = pcie->dn;
>> +
>> + mutex_lock(&brcm_pcie_lock);
>> + if (dma_ranges)
>> + goto done;
>> +
>> + /* Parse dma-ranges property if present. If there are multiple
>> + * PCI controllers, we only have to parse from one of them since
>> + * the others will have an identical mapping.
>> + */
>> + if (!brcm_pci_dma_range_parser_init(&parser, dn)) {
>> + unsigned int max_ranges
>> + = (parser.end - parser.range) / parser.np;
>> +
>> + dma_ranges = kcalloc(max_ranges, sizeof(struct of_pci_range),
>> + GFP_KERNEL);
>> + if (!dma_ranges) {
>> + ret = -ENOMEM;
>> + goto done;
>> + }
>> + for (i = 0; of_pci_range_parser_one(&parser, dma_ranges + i);
>> + i++)
>> + num_dma_ranges++;
>> + }
>> +
>> + for (i = 0, num_memc = 0; i < BRCM_MAX_SCB; i++) {
>> + u64 size = brcmstb_memory_memc_size(i);
>> +
>> + if (size == (u64)-1) {
>> + dev_err(pcie->dev, "cannot get memc%d size", i);
>> + ret = -EINVAL;
>> + goto done;
>> + } else if (size) {
>> + scb_size[i] = roundup_pow_of_two_64(size);
>> + num_memc++;
>> + } else {
>> + break;
>> + }
>> + }
>> +
>> +done:
>> + mutex_unlock(&brcm_pcie_lock);
>> + return ret;
>> +}
>> +
>> static void set_regulators(struct brcm_pcie *pcie, bool on)
>> {
>> struct list_head *pos;
>> @@ -728,10 +835,34 @@ static void brcm_pcie_setup_prep(struct brcm_pcie *pcie)
>> */
>> rc_bar2_size = roundup_pow_of_two_64(total_mem_size);
>>
>> - /* Set simple configuration based on memory sizes
>> - * only. We always start the viewport at address 0.
>> - */
>> - rc_bar2_offset = 0;
>> + if (dma_ranges) {
>> + /* The best-case scenario is to place the inbound
>> + * region in the first 4GB of pci-space, as some
>> + * legacy devices can only address 32bits.
>> + * We would also like to put the MSI under 4GB
>> + * as well, since some devices require a 32bit
>> + * MSI target address.
>> + */
>> + if (total_mem_size <= 0xc0000000ULL &&
>> + rc_bar2_size <= 0x100000000ULL) {
>> + rc_bar2_offset = 0;
>> + } else {
>> + /* The system memory is 4GB or larger so we
>> + * cannot start the inbound region at location
>> + * 0 (since we have to allow some space for
>> + * outbound memory @ 3GB). So instead we
>> + * start it at the 1x multiple of its size
>> + */
>> + rc_bar2_offset = rc_bar2_size;
>> + }
>> +
>> + } else {
>> + /* Set simple configuration based on memory sizes
>> + * only. We always start the viewport at address 0,
>> + * and set the MSI target address accordingly.
>> + */
>> + rc_bar2_offset = 0;
>> + }
>>
>> tmp = lower_32_bits(rc_bar2_offset);
>> tmp = INSERT_FIELD(tmp, PCIE_MISC_RC_BAR2_CONFIG_LO, SIZE,
>> @@ -1040,11 +1171,6 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>> return -EINVAL;
>> }
>>
>> - if (of_property_read_u32(dn, "dma-ranges", &tmp) == 0) {
>> - pr_err("cannot yet handle dma-ranges\n");
>> - return -EINVAL;
>> - }
>> -
>> data = of_id->data;
>> pcie->reg_offsets = data->offsets;
>> pcie->reg_field_info = data->reg_field_info;
>> @@ -1113,6 +1239,10 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>> if (ret)
>> return ret;
>>
>> + ret = brcm_parse_dma_ranges(pcie);
>> + if (ret)
>> + return ret;
>> +
>> ret = clk_prepare_enable(pcie->clk);
>> if (ret) {
>> dev_err(&pdev->dev, "could not enable clock\n");
>> diff --git a/drivers/pci/host/pci-brcmstb.h b/drivers/pci/host/pci-brcmstb.h
>> index 86f9cd1..4851be8 100644
>> --- a/drivers/pci/host/pci-brcmstb.h
>> +++ b/drivers/pci/host/pci-brcmstb.h
>> @@ -21,6 +21,13 @@
>> /* Broadcom PCIE Offsets */
>> #define PCIE_INTR2_CPU_BASE 0x4300
>>
>> +dma_addr_t brcm_to_pci(dma_addr_t addr);
>> +dma_addr_t brcm_to_cpu(dma_addr_t addr);
>> +
>> +extern struct notifier_block brcmstb_platform_nb;
>
> It seems a bit weird to have the notifier code split across two
> compilation units in the way which requires this - it seems more
> reasonable to have it all together on one side or the other, with the
> common interface being either the callback for setting the ops or a
> function for installing the notifier, depending on where things fall.
Okay, I'll try to rework this.
Thanks Robin,
Jim
>
> Robin.
>
>> +
>> +#define BRCMSTB_ERROR_CODE (~(dma_addr_t)0x0)
>> +
>> #if defined(CONFIG_MIPS)
>> /* Broadcom MIPs HW implicitly does the swapping if necessary */
>> #define bcm_readl(a) __raw_readl(a)
>>
>