Re: [PATCH 1/2] [v3][POWERPC] refactor dcr code
From: Josh Boyer <hidden>
Date: 2008-04-19 14:39:07
On Fri, 18 Apr 2008 14:55:03 -0700 Stephen Neuendorffer [off-list ref] wrote:
Previously, dcr support was configured at compile time to either using MMIO or native dcr instructions. Although this works for most platforms, it fails on FPGA platforms: 1) Systems may include more than one dcr bus. 2) Systems may be native dcr capable and still use memory mapped dcr interface. This patch provides runtime support based on the device trees for the case where CONFIG_PPC_DCR_MMIO and CONFIG_PPC_DCR_NATIVE are both selected. Previously, this was a poorly defined configuration, which happened to provide NATIVE support. The runtime selection is made based on the dcr controller having a 'dcr-access-method' attribute in the device tree. If only one of the above options is selected, then the code uses #defines to select only the used code in order to avoid introducing overhead in existing usage. Signed-off-by: Stephen Neuendorffer <redacted>
Hi Stephen, Sorry for the late review. See some comments below. Mostly minor stuff and I think the general direction here is good.
quoted hunk ↗ jump to hunk
diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c index 437e48d..d3de0ff 100644 --- a/arch/powerpc/sysdev/dcr.c +++ b/arch/powerpc/sysdev/dcr.c@@ -23,6 +23,68 @@ #include <asm/prom.h> #include <asm/dcr.h> +#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO) + +bool dcr_map_ok_generic(dcr_host_t host) +{ + if (host.type == INVALID) + return 0; + else if (host.type == NATIVE) + return dcr_map_ok_native(host.host.native); + else + return dcr_map_ok_mmio(host.host.mmio); +} +EXPORT_SYMBOL_GPL(dcr_map_ok_generic); + +dcr_host_t dcr_map_generic(struct device_node *dev, + unsigned int dcr_n, + unsigned int dcr_c) +{ + dcr_host_t host; + const char *prop = of_get_property(dev, "dcr-access-method", NULL); + + if (!strcmp(prop, "native")) { + host.type = NATIVE; + host.host.native = dcr_map_native(dev, dcr_n, dcr_c); + } else if (!strcmp(prop, "mmio")) { + host.type = MMIO; + host.host.mmio = dcr_map_mmio(dev, dcr_n, dcr_c); + } else + host.type = INVALID; + + return host; +} +EXPORT_SYMBOL_GPL(dcr_map_generic); + +void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c) +{ + if (host.type == NATIVE) + dcr_unmap_native(host.host.native, dcr_c); + else + dcr_unmap_mmio(host.host.mmio, dcr_c);
What happens if host.type == INVALID? Same question for the other accessors in dcr_*_generic. <snip>
quoted hunk ↗ jump to hunk
diff --git a/include/asm-powerpc/dcr-generic.h b/include/asm-powerpc/dcr-generic.h new file mode 100644 index 0000000..0ee74fb --- /dev/null +++ b/include/asm-powerpc/dcr-generic.h@@ -0,0 +1,49 @@
<snip>
+enum host_type_t {MMIO, NATIVE, INVALID};Should these be DCR_HOST_MMIO, DCR_HOST_NATIVE, DCR_HOST_INVALID? I worry about the generic nature of the names. josh