Hi Stephen, Just some trivial issues ...
On Mon, 24 Mar 2008 16:28:33 -0700 Stephen Neuendorffer <[EMAIL PROTECTED]>
wrote:
>
> +++ b/arch/powerpc/sysdev/dcr.c
> @@ -23,6 +23,67 @@
> #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) {
^
Space, please.
> + return 0;
> + } else if(host.type == NATIVE) {
And again. And more below.
> + return dcr_map_ok_native(host.host.native);
> + } else {
> + return dcr_map_ok_mmio(host.host.mmio);
> + }
You don't need the braces around each if/else clause. Again, more below.
> +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")) {
Indentation. We also usually separate declarations from code with a a
blank line.
> -dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
> +dcr_host_mmio_t dcr_map_mmio(struct device_node *dev, unsigned int dcr_n,
> unsigned int dcr_c)
> {
> - dcr_host_t ret = { .token = NULL, .stride = 0, .base = dcr_n };
> + dcr_host_mmio_t ret = { .token = NULL, .stride = 0, .base = dcr_n };
> u64 addr;
>
> pr_debug("dcr_map(%s, 0x%x, 0x%x)\n",
> dev->full_name, dcr_n, dcr_c);
>
> addr = of_translate_dcr_address(dev, dcr_n, &ret.stride);
> - pr_debug("translates to addr: 0x%lx, stride: 0x%x\n",
> + pr_debug("translates to addr: 0x%p, stride: 0x%x\n",
> addr, ret.stride);
This change should cause a warning because you are passing a u64 where
printk is expecting a pointer. We usually use %llx and cast the u64 to
unsigned long long.
> +++ b/include/asm-powerpc/dcr-mmio.h
> @@ -27,20 +27,20 @@ typedef struct {
> void __iomem *token;
> unsigned int stride;
> unsigned int base;
> -} dcr_host_t;
> +} dcr_host_mmio_t;
>
> -#define DCR_MAP_OK(host) ((host).token != NULL)
> +#define dcr_map_ok_mmio(host) ((host).token != NULL)
This could be a static inline function.
> +++ b/include/asm-powerpc/dcr-native.h
> @@ -24,14 +24,14 @@
>
> typedef struct {
> unsigned int base;
> -} dcr_host_t;
> +} dcr_host_native_t;
>
> -#define DCR_MAP_OK(host) (1)
> +#define dcr_map_ok_native(host) (1)
As could this.
--
Cheers,
Stephen Rothwell [EMAIL PROTECTED]
http://www.canb.auug.org.au/~sfr/
pgpkCCAjQLCEi.pgp
Description: PGP signature
_______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
