public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: Josh Boyer <jwboyer@linux•vnet.ibm.com>
To: Stephen Neuendorffer <stephen.neuendorffer@xilinx•com>
Cc: linuxppc-dev@ozlabs•org
Subject: Re: [PATCH 1/2] [v3][POWERPC] refactor dcr code
Date: Sat, 19 Apr 2008 09:35:00 -0500	[thread overview]
Message-ID: <20080419093500.752c1930@zod.rchland.ibm.com> (raw)
In-Reply-To: <20080418215513.ACA0C17F007A@mail131-dub.bigfish.com>

On Fri, 18 Apr 2008 14:55:03 -0700
Stephen Neuendorffer <stephen.neuendorffer@xilinx•com> 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 <stephen.neuendorffer@xilinx•com>

Hi Stephen,

Sorry for the late review.  See some comments below.  Mostly minor
stuff and I think the general direction here is good.

> 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>

> 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

  parent reply	other threads:[~2008-04-19 14:39 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1206401313-1625-1-git-send-email-stephen.neuendorffer@xilinx.com>
2008-03-28 16:23 ` [PATCH 1/5] [v2][POWERPC] refactor dcr code Stephen Neuendorffer
2008-03-30 22:02   ` Benjamin Herrenschmidt
2008-04-01 20:16     ` Stephen Neuendorffer
2008-04-04 22:02     ` Stephen Neuendorffer
2008-04-04 22:10       ` Benjamin Herrenschmidt
2008-04-18 21:49     ` Stephen Neuendorffer
2008-04-18 21:55     ` [PATCH 1/2] [v3][POWERPC] " Stephen Neuendorffer
2008-04-19  2:30       ` Grant Likely
2008-04-19  3:04         ` Benjamin Herrenschmidt
2008-04-19 14:35       ` Josh Boyer [this message]
2008-04-21  3:56         ` Stephen Neuendorffer
2008-04-21 13:03           ` Josh Boyer
2008-05-05 17:56             ` [PATCH 0/4] [v4][POWERPC] " Stephen Neuendorffer
     [not found]             ` <1210010201-28436-1-git-send-email-stephen.neuendorffer@xilinx.com>
2008-05-05 17:56               ` [PATCH 1/4] " Stephen Neuendorffer
2008-05-06  0:12                 ` Benjamin Herrenschmidt
2008-05-06  3:40                 ` Stephen Rothwell
2008-05-06  4:02                   ` Benjamin Herrenschmidt
2008-05-06 17:34                     ` Stephen Neuendorffer
2008-05-06 17:40                       ` Josh Boyer
2008-05-06 18:29                   ` [PATCH 1/4] [v5][POWERPC] " Stephen Neuendorffer
     [not found]               ` <1210010201-28436-2-git-send-email-stephen.neuendorffer@xilinx.com>
2008-05-05 17:56                 ` [PATCH 2/4] [POWERPC] Xilinx: Virtex: Enable dcr for MMIO and NATIVE Stephen Neuendorffer
2008-05-06  0:11                   ` Benjamin Herrenschmidt
2008-05-06 17:33                     ` [PATCH 2/4] [POWERPC] Xilinx: Virtex: Enable dcr for MMIO andNATIVE Stephen Neuendorffer
     [not found]                 ` <1210010201-28436-3-git-send-email-stephen.neuendorffer@xilinx.com>
2008-05-05 17:56                   ` [PATCH 3/4] [POWERPC] Xilinx: Framebuffer: remove platform device support Stephen Neuendorffer
     [not found]                   ` <1210010201-28436-4-git-send-email-stephen.neuendorffer@xilinx.com>
2008-05-05 17:56                     ` [PATCH 4/4] [POWERPC] Xilinx: Framebuffer: Use dcr infrastructure Stephen Neuendorffer
2008-05-06  4:55                       ` Grant Likely
2008-05-06  6:14                         ` David Gibson
2008-05-06 10:56                           ` Segher Boessenkool
2008-05-08  1:57                             ` David Gibson
2008-05-06 17:43                           ` [PATCH 4/4] [POWERPC] Xilinx: Framebuffer: Use dcrinfrastructure Stephen Neuendorffer
2008-05-08  1:59                             ` David Gibson
2008-05-08  4:46                               ` [PATCH 4/4] [POWERPC] Xilinx: Framebuffer: Usedcrinfrastructure Stephen Neuendorffer
2008-05-08  7:01                                 ` David Gibson
     [not found]     ` <1208555704-8978-1-git-send-email-stephen.neuendorffer@xilinx.com>
2008-04-18 21:55       ` [PATCH 2/2] [POWERPC] Xilinx: Virtex: Enable dcr for MMIO and NATIVE Stephen Neuendorffer
2008-04-19  2:31         ` Grant Likely
     [not found] ` <1206721429-18276-1-git-send-email-stephen.neuendorffer@xilinx.com>
2008-03-28 16:23   ` [PATCH 2/5] " Stephen Neuendorffer
     [not found]   ` <1206721429-18276-2-git-send-email-stephen.neuendorffer@xilinx.com>
2008-03-28 16:23     ` [PATCH 3/5] [POWERPC] explicit dcr support Stephen Neuendorffer
2008-03-30 22:04       ` Benjamin Herrenschmidt
     [not found]     ` <1206721429-18276-3-git-send-email-stephen.neuendorffer@xilinx.com>
2008-03-28 16:23       ` [PATCH 4/5] [POWERPC] Xilinx: Framebuffer: Use dcr infrastructure Stephen Neuendorffer
     [not found]       ` <1206721429-18276-4-git-send-email-stephen.neuendorffer@xilinx.com>
2008-03-28 16:23         ` [PATCH 5/5] [RFC][PPC] Use DCR for arch ppc, and enable MMIO and NATIVE for virtex Stephen Neuendorffer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080419093500.752c1930@zod.rchland.ibm.com \
    --to=jwboyer@linux$(echo .)vnet.ibm.com \
    --cc=linuxppc-dev@ozlabs$(echo .)org \
    --cc=stephen.neuendorffer@xilinx$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox