From: "Stephen Neuendorffer" <stephen.neuendorffer@xilinx•com>
To: <benh@kernel•crashing.org>
Cc: linuxppc-dev@ozlabs•org
Subject: RE: [PATCH 1/5] [v2][POWERPC] refactor dcr code
Date: Tue, 1 Apr 2008 13:16:53 -0700 [thread overview]
Message-ID: <20080401201655.1F689D4804B@mail109-sin.bigfish.com> (raw)
In-Reply-To: <1206914576.10388.132.camel@pasglop>
I considered function pointers as well, but I think something like
dcr-generic.h is still required in order to get the specialization if
only one is selected.
Also note that I neglected to change the patch description: The
dcr-access-method attribute is applied to the dcr-controller node, not
the dcr slave.
I'd particularly appreciate it if someone with access to PPC64 would
test this...
Steve
> -----Original Message-----
> From: Benjamin Herrenschmidt [mailto:benh@kernel•crashing.org]
> Sent: Sunday, March 30, 2008 3:03 PM
> To: Stephen Neuendorffer
> Cc: linuxppc-dev@ozlabs•org
> Subject: Re: [PATCH 1/5] [v2][POWERPC] refactor dcr code
>=20
>=20
> On Fri, 2008-03-28 at 09:23 -0700, Stephen Neuendorffer 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 slave device 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 interoducing overhead in existing usage.
> >
> > Signed-off-by: Stephen Neuendorffer
<stephen.neuendorffer@xilinx•com>
>=20
> Looks good. Haven't had a chance to test it yet, but tentatively
>=20
> Acked-by: Benjamin Herrenschmidt <benh@kernel•crashing.org>
>=20
> Initially, I thought about using function pointers but the if/else
will
> probably end up being more efficient. The only thing maybe to ponder
> is whether we could avoid the dcr-generic.h file completely, and have
> the generic wrappers be inline, though considering how slow DCRs are,
> it may not be that useful in practice and less clean...
>=20
> Cheers,
> Ben.
>=20
>=20
> > ---
> > arch/powerpc/sysdev/dcr.c | 91
++++++++++++++++++++++++++++++++-----
> > include/asm-powerpc/dcr-generic.h | 49 ++++++++++++++++++++
> > include/asm-powerpc/dcr-mmio.h | 20 +++++---
> > include/asm-powerpc/dcr-native.h | 16 ++++---
> > include/asm-powerpc/dcr.h | 36 ++++++++++++++-
> > 5 files changed, 186 insertions(+), 26 deletions(-)
> > create mode 100644 include/asm-powerpc/dcr-generic.h
> >
> > 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 =3D=3D INVALID)
> > + return 0;
> > + else if (host.type =3D=3D 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 =3D of_get_property(dev, "dcr-access-method",
NULL);
> > +
> > + if (!strcmp(prop, "native")) {
> > + host.type =3D NATIVE;
> > + host.host.native =3D dcr_map_native(dev, dcr_n, dcr_c);
> > + } else if (!strcmp(prop, "mmio")) {
> > + host.type =3D MMIO;
> > + host.host.mmio =3D dcr_map_mmio(dev, dcr_n, dcr_c);
> > + } else
> > + host.type =3D INVALID;
> > +
> > + return host;
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_map_generic);
> > +
> > +void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
> > +{
> > + if (host.type =3D=3D NATIVE)
> > + dcr_unmap_native(host.host.native, dcr_c);
> > + else
> > + dcr_unmap_mmio(host.host.mmio, dcr_c);
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_unmap_generic);
> > +
> > +u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n)
> > +{
> > + if (host.type =3D=3D NATIVE)
> > + return dcr_read_native(host.host.native, dcr_n);
> > + else
> > + return dcr_read_mmio(host.host.mmio, dcr_n);
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_read_generic);
> > +
> > +void dcr_write_generic(dcr_host_t host, unsigned int dcr_n, u32
value)
> > +{
> > + if (host.type =3D=3D NATIVE)
> > + dcr_write_native(host.host.native, dcr_n, value);
> > + else
> > + dcr_write_mmio(host.host.mmio, dcr_n, value);
> > +}
> > +EXPORT_SYMBOL_GPL(dcr_write_generic);
> > +
> > +#endif /* defined(CONFIG_PPC_DCR_NATIVE) &&
defined(CONFIG_PPC_DCR_MMIO) */
> > +
> > unsigned int dcr_resource_start(struct device_node *np, unsigned
int index)
> > {
> > unsigned int ds;
> > @@ -47,7 +109,7 @@ unsigned int dcr_resource_len(struct device_node
*np, unsigned int index)
> > }
> > EXPORT_SYMBOL_GPL(dcr_resource_len);
> >
> > -#ifndef CONFIG_PPC_DCR_NATIVE
> > +#ifdef CONFIG_PPC_DCR_MMIO
> >
> > static struct device_node * find_dcr_parent(struct device_node *
node)
> > {
> > @@ -101,18 +163,19 @@ u64 of_translate_dcr_address(struct
device_node *dev,
> > return ret;
> > }
> >
> > -dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
> > - unsigned int dcr_c)
> > +dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
> > + unsigned int dcr_n,
> > + unsigned int dcr_c)
> > {
> > - dcr_host_t ret =3D { .token =3D NULL, .stride =3D 0, .base =3D =
dcr_n };
> > + dcr_host_mmio_t ret =3D { .token =3D NULL, .stride =3D 0, .base =
=3D
dcr_n };
> > u64 addr;
> >
> > pr_debug("dcr_map(%s, 0x%x, 0x%x)\n",
> > dev->full_name, dcr_n, dcr_c);
> >
> > addr =3D of_translate_dcr_address(dev, dcr_n, &ret.stride);
> > - pr_debug("translates to addr: 0x%lx, stride: 0x%x\n",
> > - addr, ret.stride);
> > + pr_debug("translates to addr: 0x%llx, stride: 0x%x\n",
> > + (unsigned long long) addr, ret.stride);
> > if (addr =3D=3D OF_BAD_ADDR)
> > return ret;
> > pr_debug("mapping 0x%x bytes\n", dcr_c * ret.stride);
> > @@ -124,11 +187,11 @@ dcr_host_t dcr_map(struct device_node *dev,
unsigned int dcr_n,
> > ret.token -=3D dcr_n * ret.stride;
> > return ret;
> > }
> > -EXPORT_SYMBOL_GPL(dcr_map);
> > +EXPORT_SYMBOL_GPL(dcr_map_mmio);
> >
> > -void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
> > +void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int dcr_c)
> > {
> > - dcr_host_t h =3D host;
> > + dcr_host_mmio_t h =3D host;
> >
> > if (h.token =3D=3D NULL)
> > return;
> > @@ -136,7 +199,11 @@ void dcr_unmap(dcr_host_t host, unsigned int
dcr_c)
> > iounmap(h.token);
> > h.token =3D NULL;
> > }
> > -EXPORT_SYMBOL_GPL(dcr_unmap);
> > -#else /* defined(CONFIG_PPC_DCR_NATIVE) */
> > +EXPORT_SYMBOL_GPL(dcr_unmap_mmio);
> > +
> > +#endif /* defined(CONFIG_PPC_DCR_MMIO) */
> > +
> > +#ifdef CONFIG_PPC_DCR_NATIVE
> > DEFINE_SPINLOCK(dcr_ind_lock);
> > -#endif /* !defined(CONFIG_PPC_DCR_NATIVE) */
> > +#endif /* defined(CONFIG_PPC_DCR_NATIVE) */
> > +
> > 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 @@
> > +/*
> > + * (c) Copyright 2006 Benjamin Herrenschmidt, IBM Corp.
> > + * <benh@kernel•crashing.org>
> > + *
> > + * This program is free software; you can redistribute it and/or
modify
> > + * it under the terms of the GNU General Public License as
published by
> > + * the Free Software Foundation; either version 2 of the License,
or
> > + * (at your option) any later version.
> > + *
> > + * 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.
> > + *
> > + * You should have received a copy of the GNU General Public
License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
02111-1307 USA
> > + */
> > +
> > +#ifndef _ASM_POWERPC_DCR_GENERIC_H
> > +#define _ASM_POWERPC_DCR_GENERIC_H
> > +#ifdef __KERNEL__
> > +#ifndef __ASSEMBLY__
> > +
> > +enum host_type_t {MMIO, NATIVE, INVALID};
> > +
> > +typedef struct {
> > + enum host_type_t type;
> > + union {
> > + dcr_host_mmio_t mmio;
> > + dcr_host_native_t native;
> > + } host;
> > +} dcr_host_t;
> > +
> > +extern bool dcr_map_ok_generic(dcr_host_t host);
> > +
> > +extern dcr_host_t dcr_map_generic(struct device_node *dev, unsigned
int dcr_n,
> > + unsigned int dcr_c);
> > +extern void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c);
> > +
> > +extern u32 dcr_read_generic(dcr_host_t host, unsigned int dcr_n);
> > +
> > +extern void dcr_write_generic(dcr_host_t host, unsigned int dcr_n,
u32 value);
> > +
> > +#endif /* __ASSEMBLY__ */
> > +#endif /* __KERNEL__ */
> > +#endif /* _ASM_POWERPC_DCR_GENERIC_H */
> > +
> > +
> > diff --git a/include/asm-powerpc/dcr-mmio.h
b/include/asm-powerpc/dcr-mmio.h
> > index 08532ff..acd491d 100644
> > --- a/include/asm-powerpc/dcr-mmio.h
> > +++ b/include/asm-powerpc/dcr-mmio.h
> > @@ -27,20 +27,26 @@ 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 !=3D NULL)
> > +static inline bool dcr_map_ok_mmio(dcr_host_mmio_t host)
> > +{
> > + return host.token !=3D NULL;
> > +}
> >
> > -extern dcr_host_t dcr_map(struct device_node *dev, unsigned int
dcr_n,
> > - unsigned int dcr_c);
> > -extern void dcr_unmap(dcr_host_t host, unsigned int dcr_c);
> > +extern dcr_host_mmio_t dcr_map_mmio(struct device_node *dev,
> > + unsigned int dcr_n,
> > + unsigned int dcr_c);
> > +extern void dcr_unmap_mmio(dcr_host_mmio_t host, unsigned int
dcr_c);
> >
> > -static inline u32 dcr_read(dcr_host_t host, unsigned int dcr_n)
> > +static inline u32 dcr_read_mmio(dcr_host_mmio_t host, unsigned int
dcr_n)
> > {
> > return in_be32(host.token + ((host.base + dcr_n) *
host.stride));
> > }
> >
> > -static inline void dcr_write(dcr_host_t host, unsigned int dcr_n,
u32 value)
> > +static inline void dcr_write_mmio(dcr_host_mmio_t host,
> > + unsigned int dcr_n,
> > + u32 value)
> > {
> > out_be32(host.token + ((host.base + dcr_n) * host.stride),
value);
> > }
> > diff --git a/include/asm-powerpc/dcr-native.h
b/include/asm-powerpc/dcr-native.h
> > index be6c879..67832e5 100644
> > --- a/include/asm-powerpc/dcr-native.h
> > +++ b/include/asm-powerpc/dcr-native.h
> > @@ -26,14 +26,18 @@
> >
> > typedef struct {
> > unsigned int base;
> > -} dcr_host_t;
> > +} dcr_host_native_t;
> >
> > -#define DCR_MAP_OK(host) (1)
> > +static inline bool dcr_map_ok_native(dcr_host_native_t host)
> > +{
> > + return 1;
> > +}
> >
> > -#define dcr_map(dev, dcr_n, dcr_c) ((dcr_host_t){ .base =3D (dcr_n)
})
> > -#define dcr_unmap(host, dcr_c) do {} while (0)
> > -#define dcr_read(host, dcr_n) mfdcr(dcr_n + host.base)
> > -#define dcr_write(host, dcr_n, value) mtdcr(dcr_n + host.base,
value)
> > +#define dcr_map_native(dev, dcr_n, dcr_c) \
> > + ((dcr_host_native_t){ .base =3D (dcr_n) })
> > +#define dcr_unmap_native(host, dcr_c) do {} while (0)
> > +#define dcr_read_native(host, dcr_n) mfdcr(dcr_n +
host.base)
> > +#define dcr_write_native(host, dcr_n, value) mtdcr(dcr_n +
host.base, value)
> >
> > /* Device Control Registers */
> > void __mtdcr(int reg, unsigned int val);
> > diff --git a/include/asm-powerpc/dcr.h b/include/asm-powerpc/dcr.h
> > index 9338d50..6b86322 100644
> > --- a/include/asm-powerpc/dcr.h
> > +++ b/include/asm-powerpc/dcr.h
> > @@ -20,14 +20,47 @@
> > #ifndef _ASM_POWERPC_DCR_H
> > #define _ASM_POWERPC_DCR_H
> > #ifdef __KERNEL__
> > +#ifndef __ASSEMBLY__
> > #ifdef CONFIG_PPC_DCR
> >
> > #ifdef CONFIG_PPC_DCR_NATIVE
> > #include <asm/dcr-native.h>
> > -#else
> > +#endif
> > +
> > +#ifdef CONFIG_PPC_DCR_MMIO
> > #include <asm/dcr-mmio.h>
> > #endif
> >
> > +#if defined(CONFIG_PPC_DCR_NATIVE) && defined(CONFIG_PPC_DCR_MMIO)
> > +
> > +#include <asm/dcr-generic.h>
> > +
> > +#define DCR_MAP_OK(host) dcr_map_ok_generic(host)
> > +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_generic(dev, dcr_n,
dcr_c)
> > +#define dcr_unmap(host, dcr_c) dcr_unmap_generic(host, dcr_c)
> > +#define dcr_read(host, dcr_n) dcr_read_generic(host, dcr_n)
> > +#define dcr_write(host, dcr_n, value) dcr_write_generic(host,
dcr_n, value)
> > +
> > +#else
> > +
> > +#ifdef CONFIG_PPC_DCR_NATIVE
> > +typedef dcr_host_native_t dcr_host_t;
> > +#define DCR_MAP_OK(host) dcr_map_ok_native(host)
> > +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_native(dev, dcr_n,
dcr_c)
> > +#define dcr_unmap(host, dcr_c) dcr_unmap_native(host, dcr_c)
> > +#define dcr_read(host, dcr_n) dcr_read_native(host, dcr_n)
> > +#define dcr_write(host, dcr_n, value) dcr_write_native(host, dcr_n,
value)
> > +#else
> > +typedef dcr_host_mmio_t dcr_host_t;
> > +#define DCR_MAP_OK(host) dcr_map_ok_mmio(host)
> > +#define dcr_map(dev, dcr_n, dcr_c) dcr_map_mmio(dev, dcr_n, dcr_c)
> > +#define dcr_unmap(host, dcr_c) dcr_unmap_mmio(host, dcr_c)
> > +#define dcr_read(host, dcr_n) dcr_read_mmio(host, dcr_n)
> > +#define dcr_write(host, dcr_n, value) dcr_write_mmio(host, dcr_n,
value)
> > +#endif
> > +
> > +#endif /* defined(CONFIG_PPC_DCR_NATIVE) &&
defined(CONFIG_PPC_DCR_MMIO) */
> > +
> > /*
> > * On CONFIG_PPC_MERGE, we have additional helpers to read the DCR
> > * base from the device-tree
> > @@ -41,5 +74,6 @@ extern unsigned int dcr_resource_len(struct
device_node *np,
> > #endif /* CONFIG_PPC_MERGE */
> >
> > #endif /* CONFIG_PPC_DCR */
> > +#endif /* __ASSEMBLY__ */
> > #endif /* __KERNEL__ */
> > #endif /* _ASM_POWERPC_DCR_H */
>=20
next prev parent reply other threads:[~2008-04-01 20:18 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 [this message]
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
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=20080401201655.1F689D4804B@mail109-sin.bigfish.com \
--to=stephen.neuendorffer@xilinx$(echo .)com \
--cc=benh@kernel$(echo .)crashing.org \
--cc=linuxppc-dev@ozlabs$(echo .)org \
/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