[PATCH 1/5] Update ibm_newemac to use dcr_host_t.base

[PATCH 1/5] Update ibm_newemac to use dcr_host_t.base

[Michael Ellerman]

Now that dcr_host_t contains the base address, we can use that in the
ibm_newemac code, rather than storing it separately.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 drivers/net/ibm_newemac/mal.c |    9 +++++----
 drivers/net/ibm_newemac/mal.h |    5 ++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ibm_newemac/mal.c b/drivers/net/ibm_newemac/mal.c
index 5885411..748a869 100644
--- a/drivers/net/ibm_newemac/mal.c
+++ b/drivers/net/ibm_newemac/mal.c
@@ -461,6 +461,7 @@ static int __devinit mal_probe(struct of_device *ofdev,
 	struct mal_instance *mal;
 	int err = 0, i, bd_size;
 	int index = mal_count++;
+	unsigned int dcr_base;
 	const u32 *prop;
 	u32 cfg;
 
@@ -497,14 +498,14 @@ static int __devinit mal_probe(struct of_device *ofdev,
 	}
 	mal->num_rx_chans = prop[0];
 
-	mal->dcr_base = dcr_resource_start(ofdev->node, 0);
-	if (mal->dcr_base == 0) {
+	dcr_base = dcr_resource_start(ofdev->node, 0);
+	if (dcr_base == 0) {
 		printk(KERN_ERR
 		       "mal%d: can't find DCR resource!\n", index);
 		err = -ENODEV;
 		goto fail;
 	}
-        mal->dcr_host = dcr_map(ofdev->node, mal->dcr_base, 0x100);
+	mal->dcr_host = dcr_map(ofdev->node, dcr_base, 0x100);
 	if (!DCR_MAP_OK(mal->dcr_host)) {
 		printk(KERN_ERR
 		       "mal%d: failed to map DCRs !\n", index);
@@ -626,7 +627,7 @@ static int __devinit mal_probe(struct of_device *ofdev,
  fail2:
 	dma_free_coherent(&ofdev->dev, bd_size, mal->bd_virt, mal->bd_dma);
  fail_unmap:
-	dcr_unmap(mal->dcr_host, mal->dcr_base, 0x100);
+	dcr_unmap(mal->dcr_host, dcr_base, 0x100);
  fail:
 	kfree(mal);
 
diff --git a/drivers/net/ibm_newemac/mal.h b/drivers/net/ibm_newemac/mal.h
index cb1a16d..6daa98e 100644
--- a/drivers/net/ibm_newemac/mal.h
+++ b/drivers/net/ibm_newemac/mal.h
@@ -185,7 +185,6 @@ struct mal_commac {
 
 struct mal_instance {
 	int			version;
-	int			dcr_base;
 	dcr_host_t		dcr_host;
 
 	int			num_tx_chans;	/* Number of TX channels */
@@ -213,12 +212,12 @@ struct mal_instance {
 
 static inline u32 get_mal_dcrn(struct mal_instance *mal, int reg)
 {
-	return dcr_read(mal->dcr_host, mal->dcr_base + reg);
+	return dcr_read(mal->dcr_host, mal->dcr_host.base + reg);
 }
 
 static inline void set_mal_dcrn(struct mal_instance *mal, int reg, u32 val)
 {
-	dcr_write(mal->dcr_host, mal->dcr_base + reg, val);
+	dcr_write(mal->dcr_host, mal->dcr_host.base + reg, val);
 }
 
 /* Register MAL devices */
-- 
1.5.1.3.g7a33b

[PATCH 2/5] Use dcr_host_t.base in ibm_emac_mal

[Michael Ellerman]

This requires us to do a sort-of fake dcr_map(), so that base is set
properly. This will be fixed/removed when the device-tree-aware emac driver
is merged.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 drivers/net/ibm_emac/ibm_emac_mal.c |    5 ++++-
 drivers/net/ibm_emac/ibm_emac_mal.h |    5 ++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ibm_emac/ibm_emac_mal.c b/drivers/net/ibm_emac/ibm_emac_mal.c
index 4e49e8c..dcd8826 100644
--- a/drivers/net/ibm_emac/ibm_emac_mal.c
+++ b/drivers/net/ibm_emac/ibm_emac_mal.c
@@ -413,7 +413,10 @@ static int __init mal_probe(struct ocp_device *ocpdev)
 		       ocpdev->def->index);
 		return -ENOMEM;
 	}
-	mal->dcrbase = maldata->dcr_base;
+
+	/* XXX This only works for native dcr for now */
+	mal->dcrhost = dcr_map(NULL, maldata->dcr_base, 0);
+
 	mal->def = ocpdev->def;
 
 	INIT_LIST_HEAD(&mal->poll_list);
diff --git a/drivers/net/ibm_emac/ibm_emac_mal.h b/drivers/net/ibm_emac/ibm_emac_mal.h
index 8f54d62..aa76d3f 100644
--- a/drivers/net/ibm_emac/ibm_emac_mal.h
+++ b/drivers/net/ibm_emac/ibm_emac_mal.h
@@ -191,7 +191,6 @@ struct mal_commac {
 };
 
 struct ibm_ocp_mal {
-	int			dcrbase;
 	dcr_host_t		dcrhost;
 
 	struct list_head	poll_list;
@@ -209,12 +208,12 @@ struct ibm_ocp_mal {
 
 static inline u32 get_mal_dcrn(struct ibm_ocp_mal *mal, int reg)
 {
-	return dcr_read(mal->dcrhost, mal->dcrbase + reg);
+	return dcr_read(mal->dcrhost, mal->dcrhost.base + reg);
 }
 
 static inline void set_mal_dcrn(struct ibm_ocp_mal *mal, int reg, u32 val)
 {
-	dcr_write(mal->dcrhost, mal->dcrbase + reg, val);
+	dcr_write(mal->dcrhost, mal->dcrhost.base + reg, val);
 }
 
 /* Register MAL devices */
-- 
1.5.1.3.g7a33b

[PATCH 3/5] Add dcr_host_t.base in dcr_read()/dcr_write()

[Michael Ellerman]

Now that all users of dcr_read()/dcr_write() add the dcr_host_t.base, we
can save them the trouble and do it in dcr_read()/dcr_write().

As some background to why we just went through all this jiggery-pokery,
benh sayeth:

 Initially the goal of the dcr_read/dcr_write routines was to operate like
 mfdcr/mtdcr which take absolute DCR numbers. The reason is that on 4xx
 hardware, indirect DCR access is a pain (goes through a table of
 instructions) and it's useful to have the compiler resolve an absolute DCR
 inline.

 We decided that wasn't worth the API bastardisation since most places
 where absolute DCR values are used are low level 4xx-only code which may
 as well continue using mfdcr/mtdcr, while the new API is designed for
 device "instances" that can exist on 4xx and Axon type platforms and may
 be located at variable DCR offsets.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/cell/axon_msi.c |    4 ++--
 arch/powerpc/sysdev/mpic.c             |    4 ++--
 drivers/net/ibm_emac/ibm_emac_mal.h    |    4 ++--
 drivers/net/ibm_newemac/mal.h          |    4 ++--
 include/asm-powerpc/dcr-mmio.h         |    4 ++--
 include/asm-powerpc/dcr-native.h       |    4 ++--
 6 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c
index 1245b2f..aca1500 100644
--- a/arch/powerpc/platforms/cell/axon_msi.c
+++ b/arch/powerpc/platforms/cell/axon_msi.c
@@ -77,12 +77,12 @@ static void msic_dcr_write(struct axon_msic *msic, unsigned int dcr_n, u32 val)
 {
 	pr_debug("axon_msi: dcr_write(0x%x, 0x%x)\n", val, dcr_n);
 
-	dcr_write(msic->dcr_host, msic->dcr_host.base + dcr_n, val);
+	dcr_write(msic->dcr_host, dcr_n, val);
 }
 
 static u32 msic_dcr_read(struct axon_msic *msic, unsigned int dcr_n)
 {
-	return dcr_read(msic->dcr_host, msic->dcr_host.base + dcr_n);
+	return dcr_read(msic->dcr_host, dcr_n);
 }
 
 static void axon_msi_cascade(unsigned int irq, struct irq_desc *desc)
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 893e654..e479388 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -156,7 +156,7 @@ static inline u32 _mpic_read(enum mpic_reg_type type,
 	switch(type) {
 #ifdef CONFIG_PPC_DCR
 	case mpic_access_dcr:
-		return dcr_read(rb->dhost, rb->dhost.base + reg);
+		return dcr_read(rb->dhost, reg);
 #endif
 	case mpic_access_mmio_be:
 		return in_be32(rb->base + (reg >> 2));
@@ -173,7 +173,7 @@ static inline void _mpic_write(enum mpic_reg_type type,
 	switch(type) {
 #ifdef CONFIG_PPC_DCR
 	case mpic_access_dcr:
-		return dcr_write(rb->dhost, rb->dhost.base + reg, value);
+		return dcr_write(rb->dhost, reg, value);
 #endif
 	case mpic_access_mmio_be:
 		return out_be32(rb->base + (reg >> 2), value);
diff --git a/drivers/net/ibm_emac/ibm_emac_mal.h b/drivers/net/ibm_emac/ibm_emac_mal.h
index aa76d3f..b8adbe6 100644
--- a/drivers/net/ibm_emac/ibm_emac_mal.h
+++ b/drivers/net/ibm_emac/ibm_emac_mal.h
@@ -208,12 +208,12 @@ struct ibm_ocp_mal {
 
 static inline u32 get_mal_dcrn(struct ibm_ocp_mal *mal, int reg)
 {
-	return dcr_read(mal->dcrhost, mal->dcrhost.base + reg);
+	return dcr_read(mal->dcrhost, reg);
 }
 
 static inline void set_mal_dcrn(struct ibm_ocp_mal *mal, int reg, u32 val)
 {
-	dcr_write(mal->dcrhost, mal->dcrhost.base + reg, val);
+	dcr_write(mal->dcrhost, reg, val);
 }
 
 /* Register MAL devices */
diff --git a/drivers/net/ibm_newemac/mal.h b/drivers/net/ibm_newemac/mal.h
index 6daa98e..784edb8 100644
--- a/drivers/net/ibm_newemac/mal.h
+++ b/drivers/net/ibm_newemac/mal.h
@@ -212,12 +212,12 @@ struct mal_instance {
 
 static inline u32 get_mal_dcrn(struct mal_instance *mal, int reg)
 {
-	return dcr_read(mal->dcr_host, mal->dcr_host.base + reg);
+	return dcr_read(mal->dcr_host, reg);
 }
 
 static inline void set_mal_dcrn(struct mal_instance *mal, int reg, u32 val)
 {
-	dcr_write(mal->dcr_host, mal->dcr_host.base + reg, val);
+	dcr_write(mal->dcr_host, reg, val);
 }
 
 /* Register MAL devices */
diff --git a/include/asm-powerpc/dcr-mmio.h b/include/asm-powerpc/dcr-mmio.h
index 6b82c3b..a7d9eaf 100644
--- a/include/asm-powerpc/dcr-mmio.h
+++ b/include/asm-powerpc/dcr-mmio.h
@@ -37,12 +37,12 @@ extern void dcr_unmap(dcr_host_t host, unsigned int dcr_n, unsigned int dcr_c);
 
 static inline u32 dcr_read(dcr_host_t host, unsigned int dcr_n)
 {
-	return in_be32(host.token + dcr_n * host.stride);
+	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)
 {
-	out_be32(host.token + dcr_n * host.stride, value);
+	out_be32(host.token + ((host.base + dcr_n) * host.stride), value);
 }
 
 extern u64 of_translate_dcr_address(struct device_node *dev,
diff --git a/include/asm-powerpc/dcr-native.h b/include/asm-powerpc/dcr-native.h
index f41058c..3bc780f 100644
--- a/include/asm-powerpc/dcr-native.h
+++ b/include/asm-powerpc/dcr-native.h
@@ -30,8 +30,8 @@ typedef struct {
 
 #define dcr_map(dev, dcr_n, dcr_c)	((dcr_host_t){ .base = (dcr_n) })
 #define dcr_unmap(host, dcr_n, dcr_c)	do {} while (0)
-#define dcr_read(host, dcr_n)		mfdcr(dcr_n)
-#define dcr_write(host, dcr_n, value)	mtdcr(dcr_n, value)
+#define dcr_read(host, dcr_n)		mfdcr(dcr_n + host.base)
+#define dcr_write(host, dcr_n, value)	mtdcr(dcr_n + host.base, value)
 
 /* Device Control Registers */
 void __mtdcr(int reg, unsigned int val);
-- 
1.5.1.3.g7a33b

[PATCH 4/5] Use dcr_host_t.base in dcr_unmap()

[Michael Ellerman]

With the base stored in dcr_host_t, there's no need for callers to pass
the dcr_n into dcr_unmap(). In fact this removes the possibility of them
passing the incorrect value, which would then be iounmap()'ed.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/sysdev/dcr.c        |    4 ++--
 drivers/net/ibm_newemac/mal.c    |    2 +-
 include/asm-powerpc/dcr-mmio.h   |    2 +-
 include/asm-powerpc/dcr-native.h |    2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/sysdev/dcr.c b/arch/powerpc/sysdev/dcr.c
index ab11c0b..427027c 100644
--- a/arch/powerpc/sysdev/dcr.c
+++ b/arch/powerpc/sysdev/dcr.c
@@ -126,13 +126,13 @@ dcr_host_t dcr_map(struct device_node *dev, unsigned int dcr_n,
 }
 EXPORT_SYMBOL_GPL(dcr_map);
 
-void dcr_unmap(dcr_host_t host, unsigned int dcr_n, unsigned int dcr_c)
+void dcr_unmap(dcr_host_t host, unsigned int dcr_c)
 {
 	dcr_host_t h = host;
 
 	if (h.token == NULL)
 		return;
-	h.token += dcr_n * h.stride;
+	h.token += host.base * h.stride;
 	iounmap(h.token);
 	h.token = NULL;
 }
diff --git a/drivers/net/ibm_newemac/mal.c b/drivers/net/ibm_newemac/mal.c
index 748a869..39f4cb6 100644
--- a/drivers/net/ibm_newemac/mal.c
+++ b/drivers/net/ibm_newemac/mal.c
@@ -627,7 +627,7 @@ static int __devinit mal_probe(struct of_device *ofdev,
  fail2:
 	dma_free_coherent(&ofdev->dev, bd_size, mal->bd_virt, mal->bd_dma);
  fail_unmap:
-	dcr_unmap(mal->dcr_host, dcr_base, 0x100);
+	dcr_unmap(mal->dcr_host, 0x100);
  fail:
 	kfree(mal);
 
diff --git a/include/asm-powerpc/dcr-mmio.h b/include/asm-powerpc/dcr-mmio.h
index a7d9eaf..08532ff 100644
--- a/include/asm-powerpc/dcr-mmio.h
+++ b/include/asm-powerpc/dcr-mmio.h
@@ -33,7 +33,7 @@ typedef struct {
 
 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_n, unsigned int dcr_c);
+extern void dcr_unmap(dcr_host_t host, unsigned int dcr_c);
 
 static inline u32 dcr_read(dcr_host_t host, unsigned int dcr_n)
 {
diff --git a/include/asm-powerpc/dcr-native.h b/include/asm-powerpc/dcr-native.h
index 3bc780f..8dbb1ab 100644
--- a/include/asm-powerpc/dcr-native.h
+++ b/include/asm-powerpc/dcr-native.h
@@ -29,7 +29,7 @@ typedef struct {
 #define DCR_MAP_OK(host)	(1)
 
 #define dcr_map(dev, dcr_n, dcr_c)	((dcr_host_t){ .base = (dcr_n) })
-#define dcr_unmap(host, dcr_n, dcr_c)	do {} while (0)
+#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)
 
-- 
1.5.1.3.g7a33b

[PATCH 5/5] Remove msic_dcr_read() in axon_msi.c

[Michael Ellerman]

msic_dcr_read() doesn't really do anything useful, just replace it with
direct calls to dcr_read().

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/platforms/cell/axon_msi.c |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c
index aca1500..095988f 100644
--- a/arch/powerpc/platforms/cell/axon_msi.c
+++ b/arch/powerpc/platforms/cell/axon_msi.c
@@ -80,18 +80,13 @@ static void msic_dcr_write(struct axon_msic *msic, unsigned int dcr_n, u32 val)
 	dcr_write(msic->dcr_host, dcr_n, val);
 }
 
-static u32 msic_dcr_read(struct axon_msic *msic, unsigned int dcr_n)
-{
-	return dcr_read(msic->dcr_host, dcr_n);
-}
-
 static void axon_msi_cascade(unsigned int irq, struct irq_desc *desc)
 {
 	struct axon_msic *msic = get_irq_data(irq);
 	u32 write_offset, msi;
 	int idx;
 
-	write_offset = msic_dcr_read(msic, MSIC_WRITE_OFFSET_REG);
+	write_offset = dcr_read(msic->dcr_host, MSIC_WRITE_OFFSET_REG);
 	pr_debug("axon_msi: original write_offset 0x%x\n", write_offset);
 
 	/* write_offset doesn't wrap properly, so we have to mask it */
@@ -306,7 +301,7 @@ static int axon_msi_notify_reboot(struct notifier_block *nb,
 	list_for_each_entry(msic, &axon_msic_list, list) {
 		pr_debug("axon_msi: disabling %s\n",
 			  msic->irq_host->of_node->full_name);
-		tmp  = msic_dcr_read(msic, MSIC_CTRL_REG);
+		tmp  = dcr_read(msic->dcr_host, MSIC_CTRL_REG);
 		tmp &= ~MSIC_CTRL_ENABLE & ~MSIC_CTRL_IRQ_ENABLE;
 		msic_dcr_write(msic, MSIC_CTRL_REG, tmp);
 	}
-- 
1.5.1.3.g7a33b

Re: [PATCH 1/5] Update ibm_newemac to use dcr_host_t.base

[Jeff Garzik]

Michael Ellerman wrote:
> Now that dcr_host_t contains the base address, we can use that in the
> ibm_newemac code, rather than storing it separately.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
>  drivers/net/ibm_newemac/mal.c |    9 +++++----
>  drivers/net/ibm_newemac/mal.h |    5 ++---
>  2 files changed, 7 insertions(+), 7 deletions(-)

applied 1-5

Re: [PATCH 1/5] Update ibm_newemac to use dcr_host_t.base

[Benjamin Herrenschmidt]

On Mon, 2007-10-15 at 14:34 -0400, Jeff Garzik wrote:
> Michael Ellerman wrote:
> > Now that dcr_host_t contains the base address, we can use that in the
> > ibm_newemac code, rather than storing it separately.
> > 
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> > ---
> >  drivers/net/ibm_newemac/mal.c |    9 +++++----
> >  drivers/net/ibm_newemac/mal.h |    5 ++---
> >  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> applied 1-5

Those patches have been around for some time now, they didn't make
paulus initial merge for reasons I'm not sure about but I think they
should go into 2.6.24. Now the question is via Jeff or via Paulus ? :-)
 
Ben.

Re: [PATCH 1/5] Update ibm_newemac to use dcr_host_t.base

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1251 bytes --]

On Tue, 2007-10-16 at 06:54 +1000, Benjamin Herrenschmidt wrote:
> On Mon, 2007-10-15 at 14:34 -0400, Jeff Garzik wrote:
> > Michael Ellerman wrote:
> > > Now that dcr_host_t contains the base address, we can use that in the
> > > ibm_newemac code, rather than storing it separately.
> > > 
> > > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> > > ---
> > >  drivers/net/ibm_newemac/mal.c |    9 +++++----
> > >  drivers/net/ibm_newemac/mal.h |    5 ++---
> > >  2 files changed, 7 insertions(+), 7 deletions(-)
> > 
> > applied 1-5
> 
> Those patches have been around for some time now, they didn't make
> paulus initial merge for reasons I'm not sure about but I think they
> should go into 2.6.24. Now the question is via Jeff or via Paulus ? :-)

The first three went in already, these are the following cleanups. I
wanted to wait for the newemac driver to go into Linus' tree via Jeff. I
have no opinion on who's tree they should go through from here.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]