[PATCH] altix: Add volatile qualifiers in places where it seems necessary

[PATCH] altix: Add volatile qualifiers in places where it seems necessary

[Mark Maule]

Add missing volatile qualifier to hardware MMR pointers in various SN files.
Cosmetic code rearranging in pcibr_reg.c to avoid line-wraps.

Signed-off-by: Mark Maule <maule@sgi.com>

Index: volatile/arch/ia64/sn/pci/pcibr/pcibr_reg.c
=================================--- volatile.orig/arch/ia64/sn/pci/pcibr/pcibr_reg.c	2005-09-02 09:13:09.993588678 -0500
+++ volatile/arch/ia64/sn/pci/pcibr/pcibr_reg.c	2005-09-02 09:29:01.898113823 -0500
@@ -24,8 +24,9 @@
  */
 void pcireg_control_bit_clr(struct pcibus_info *pcibus_info, uint64_t bits)
 {
-	union br_ptr *ptr = (union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
+	volatile union br_ptr *ptr;
 
+	ptr = (volatile union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
@@ -44,8 +45,9 @@
 
 void pcireg_control_bit_set(struct pcibus_info *pcibus_info, uint64_t bits)
 {
-	union br_ptr *ptr = (union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
+	volatile union br_ptr *ptr;
 
+	ptr = (volatile union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
@@ -67,9 +69,10 @@
  */
 uint64_t pcireg_tflush_get(struct pcibus_info *pcibus_info)
 {
-	union br_ptr *ptr = (union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
+	volatile union br_ptr *ptr;
 	uint64_t ret = 0;
 
+	ptr = (volatile union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
@@ -97,9 +100,10 @@
  */
 uint64_t pcireg_intr_status_get(struct pcibus_info * pcibus_info)
 {
-	union br_ptr *ptr = (union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
+	volatile union br_ptr *ptr;
 	uint64_t ret = 0;
 
+	ptr = (volatile union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
@@ -122,8 +126,9 @@
  */
 void pcireg_intr_enable_bit_clr(struct pcibus_info *pcibus_info, uint64_t bits)
 {
-	union br_ptr *ptr = (union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
+	volatile union br_ptr *ptr;
 
+	ptr = (volatile union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
@@ -142,8 +147,9 @@
 
 void pcireg_intr_enable_bit_set(struct pcibus_info *pcibus_info, uint64_t bits)
 {
-	union br_ptr *ptr = (union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
+	volatile union br_ptr *ptr;
 
+	ptr = (volatile union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
@@ -166,8 +172,9 @@
 void pcireg_intr_addr_addr_set(struct pcibus_info *pcibus_info, int int_n,
 			       uint64_t addr)
 {
-	union br_ptr *ptr = (union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
+	volatile union br_ptr *ptr;
 
+	ptr = (volatile union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
@@ -193,8 +200,9 @@
  */
 void pcireg_force_intr_set(struct pcibus_info *pcibus_info, int int_n)
 {
-	union br_ptr *ptr = (union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
+	volatile union br_ptr *ptr;
 
+	ptr = (volatile union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
@@ -216,9 +224,10 @@
  */
 uint64_t pcireg_wrb_flush_get(struct pcibus_info *pcibus_info, int device)
 {
-	union br_ptr *ptr = (union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
+	volatile union br_ptr *ptr;
 	uint64_t ret = 0;
 
+	ptr = (volatile union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
@@ -239,8 +248,9 @@
 void pcireg_int_ate_set(struct pcibus_info *pcibus_info, int ate_index,
 			uint64_t val)
 {
-	union br_ptr *ptr = (union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
+	volatile union br_ptr *ptr;
 
+	ptr = (volatile union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
@@ -259,9 +269,10 @@
 
 uint64_t *pcireg_int_ate_addr(struct pcibus_info *pcibus_info, int ate_index)
 {
-	union br_ptr *ptr = (union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
+	volatile union br_ptr *ptr;
 	uint64_t *ret = (uint64_t *) 0;
 
+	ptr = (volatile union br_ptr *)pcibus_info->pbi_buscommon.bs_base;
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
Index: volatile/arch/ia64/sn/pci/tioca_provider.c
=================================--- volatile.orig/arch/ia64/sn/pci/tioca_provider.c	2005-09-02 09:13:09.993588678 -0500
+++ volatile/arch/ia64/sn/pci/tioca_provider.c	2005-09-02 09:19:32.419128175 -0500
@@ -40,7 +40,7 @@
 	volatile struct tioca *ca_base;
 
 	tioca_common = tioca_kern->ca_common;
-	ca_base = (struct tioca *)tioca_common->ca_common.bs_base;
+	ca_base = (volatile struct tioca *)tioca_common->ca_common.bs_base;
 
 	if (list_empty(tioca_kern->ca_devices))
 		return 0;
@@ -213,7 +213,7 @@
 	int cap_ptr;
 	uint64_t ca_control1;
 	uint32_t reg;
-	struct tioca *tioca_base;
+	volatile struct tioca *tioca_base;
 	struct pci_dev *pdev;
 	struct tioca_common *common;
 
@@ -255,7 +255,7 @@
 	 * Set ca's fw to match
 	 */
 
-	tioca_base = (struct tioca *)common->ca_common.bs_base;
+	tioca_base = (volatile struct tioca *)common->ca_common.bs_base;
 	ca_control1 = tioca_base->ca_control1;
 	ca_control1 |= CA_AGP_FW_ENABLE;
 	tioca_base->ca_control1 = ca_control1;
@@ -322,7 +322,7 @@
 tioca_dma_d48(struct pci_dev *pdev, uint64_t paddr)
 {
 	struct tioca_common *tioca_common;
-	struct tioca *ca_base;
+	volatile struct tioca *ca_base;
 	uint64_t ct_addr;
 	dma_addr_t bus_addr;
 	uint32_t node_upper;
@@ -330,7 +330,7 @@
 	struct pcidev_info *pcidev_info = SN_PCIDEV_INFO(pdev);
 
 	tioca_common = (struct tioca_common *)pcidev_info->pdi_pcibus_info;
-	ca_base = (struct tioca *)tioca_common->ca_common.bs_base;
+	ca_base = (volatile struct tioca *)tioca_common->ca_common.bs_base;
 
 	ct_addr = PHYS_TO_TIODMA(paddr);
 	if (!ct_addr)
Index: volatile/arch/ia64/sn/pci/tioce_provider.c
=================================--- volatile.orig/arch/ia64/sn/pci/tioce_provider.c	2005-09-02 09:13:09.994565143 -0500
+++ volatile/arch/ia64/sn/pci/tioce_provider.c	2005-09-02 09:20:18.725033662 -0500
@@ -154,11 +154,11 @@
 	uint64_t *ate_shadow;
 	uint64_t *ate_reg;
 	uint64_t addr;
-	struct tioce *ce_mmr;
+	volatile struct tioce *ce_mmr;
 	uint64_t bus_base;
 	struct tioce_dmamap *map;
 
-	ce_mmr = (struct tioce *)ce_kern->ce_common->ce_pcibus.bs_base;
+	ce_mmr = (volatile struct tioce *)ce_kern->ce_common->ce_pcibus.bs_base;
 
 	switch (type) {
 	case TIOCE_ATE_M32:
@@ -256,7 +256,7 @@
 {
 	int dma_ok;
 	int port;
-	struct tioce *ce_mmr;
+	volatile struct tioce *ce_mmr;
 	struct tioce_kernel *ce_kern;
 	uint64_t ct_upper;
 	uint64_t ct_lower;
@@ -327,7 +327,7 @@
 	int i;
 	int port;
 	struct tioce_kernel *ce_kern;
-	struct tioce *ce_mmr;
+	volatile struct tioce *ce_mmr;
 	unsigned long flags;
 
 	bus_addr = tioce_dma_barrier(bus_addr, 0);
@@ -551,7 +551,7 @@
 {
 	int i;
 	uint32_t tmp;
-	struct tioce *tioce_mmr;
+	volatile struct tioce *tioce_mmr;
 	struct tioce_kernel *tioce_kern;
 
 	tioce_kern = kcalloc(1, sizeof(struct tioce_kernel), GFP_KERNEL);
@@ -581,7 +581,7 @@
 	 * the ate's.
 	 */
 
-	tioce_mmr = (struct tioce *)tioce_common->ce_pcibus.bs_base;
+	tioce_mmr = (volatile struct tioce *)tioce_common->ce_pcibus.bs_base;
 	tioce_mmr->ce_ure_page_map &= ~CE_URE_PAGESIZE_MASK;
 	tioce_mmr->ce_ure_page_map |= CE_URE_256K_PAGESIZE;
 	tioce_kern->ce_ate3240_pagesize = KB(256);
@@ -613,7 +613,7 @@
 {
 	struct pcidev_info *pcidev_info;
 	struct tioce_common *ce_common;
-	struct tioce *ce_mmr;
+	volatile struct tioce *ce_mmr;
 	uint64_t force_int_val;
 
 	if (!sn_irq_info->irq_bridge)
@@ -627,7 +627,7 @@
 		return;
 
 	ce_common = (struct tioce_common *)pcidev_info->pdi_pcibus_info;
-	ce_mmr = (struct tioce *)ce_common->ce_pcibus.bs_base;
+	ce_mmr = (volatile struct tioce *)ce_common->ce_pcibus.bs_base;
 
 	/*
 	 * irq_int_bit is originally set up by prom, and holds the interrupt
@@ -684,7 +684,7 @@
 {
 	struct pcidev_info *pcidev_info;
 	struct tioce_common *ce_common;
-	struct tioce *ce_mmr;
+	volatile struct tioce *ce_mmr;
 	int bit;
 
 	pcidev_info = (struct pcidev_info *)sn_irq_info->irq_pciioinfo;
@@ -692,7 +692,7 @@
 		return;
 
 	ce_common = (struct tioce_common *)pcidev_info->pdi_pcibus_info;
-	ce_mmr = (struct tioce *)ce_common->ce_pcibus.bs_base;
+	ce_mmr = (volatile struct tioce *)ce_common->ce_pcibus.bs_base;
 
 	bit = sn_irq_info->irq_int_bit;
 

Re: [PATCH] altix: Add volatile qualifiers in places where it seems necessary

[Christoph Hellwig]

On Fri, Sep 02, 2005 at 10:36:12AM -0500, Mark Maule wrote:
> Add missing volatile qualifier to hardware MMR pointers in various SN files.
> Cosmetic code rearranging in pcibr_reg.c to avoid line-wraps.

Volatile usage is wrong most of the time, and your use is certainly.
It seems you're accessing mmio-space or something similar and should
use proper acessors for it.

Re: [PATCH] altix: Add volatile qualifiers in places where it seems necessary

[Mark Maule]

On Fri, Sep 02, 2005 at 04:38:09PM +0100, Christoph Hellwig wrote:
> On Fri, Sep 02, 2005 at 10:36:12AM -0500, Mark Maule wrote:
> > Add missing volatile qualifier to hardware MMR pointers in various SN files.
> > Cosmetic code rearranging in pcibr_reg.c to avoid line-wraps.
> 
> Volatile usage is wrong most of the time, and your use is certainly.
> It seems you're accessing mmio-space or something similar and should
> use proper acessors for it.
> 

Okay.  In these cases, I am indeed accessing mapped reisters.  I don't know
what you mean by "proper acessors", so a reference would be appreciated.

thanks
Mark

Re: [PATCH] altix: Add volatile qualifiers in places where it seems necessary

[Christoph Hellwig]

On Fri, Sep 02, 2005 at 10:41:52AM -0500, Mark Maule wrote:
> On Fri, Sep 02, 2005 at 04:38:09PM +0100, Christoph Hellwig wrote:
> > On Fri, Sep 02, 2005 at 10:36:12AM -0500, Mark Maule wrote:
> > > Add missing volatile qualifier to hardware MMR pointers in various SN files.
> > > Cosmetic code rearranging in pcibr_reg.c to avoid line-wraps.
> > 
> > Volatile usage is wrong most of the time, and your use is certainly.
> > It seems you're accessing mmio-space or something similar and should
> > use proper acessors for it.
> > 
> 
> Okay.  In these cases, I am indeed accessing mapped reisters.  I don't know
> what you mean by "proper acessors", so a reference would be appreciated.

ioread<N> / iowrite<N>

Re: [PATCH] altix: Add volatile qualifiers in places where it seems necessary

[Russ Anderson]

Mark Maule wrote:
> On Fri, Sep 02, 2005 at 04:38:09PM +0100, Christoph Hellwig wrote:
> > On Fri, Sep 02, 2005 at 10:36:12AM -0500, Mark Maule wrote:
> > > Add missing volatile qualifier to hardware MMR pointers in various SN files.
> > > Cosmetic code rearranging in pcibr_reg.c to avoid line-wraps.
> > 
> > Volatile usage is wrong most of the time, and your use is certainly.
> > It seems you're accessing mmio-space or something similar and should
> > use proper acessors for it.
> > 
> 
> Okay.  In these cases, I am indeed accessing mapped reisters.  I don't know
> what you mean by "proper acessors", so a reference would be appreciated.

The BTE code used HUB_L() and HUB_S() for comperable type useage.

In include/asm-ia64/sn/bte.h:

#define BTE_LNSTAT_LOAD(_bte)                                           \
                        HUB_L(_bte->bte_base_addr)


In include/asm-ia64/sn/addrs.h:

#define HUB_L(x)                        (*((volatile typeof(*x) *)x))
#define HUB_S(x,d)                      (*((volatile typeof(*x) *)x) = (d))


-- 
Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          rja@sgi.com

Re: [PATCH] altix: Add volatile qualifiers in places where it seems necessary

[Mark Maule]

On Fri, Sep 02, 2005 at 04:49:53PM +0100, Christoph Hellwig wrote:
> On Fri, Sep 02, 2005 at 10:41:52AM -0500, Mark Maule wrote:
> > On Fri, Sep 02, 2005 at 04:38:09PM +0100, Christoph Hellwig wrote:
> > > On Fri, Sep 02, 2005 at 10:36:12AM -0500, Mark Maule wrote:
> > > > Add missing volatile qualifier to hardware MMR pointers in various SN files.
> > > > Cosmetic code rearranging in pcibr_reg.c to avoid line-wraps.
> > > 
> > > Volatile usage is wrong most of the time, and your use is certainly.
> > > It seems you're accessing mmio-space or something similar and should
> > > use proper acessors for it.
> > > 
> > 
> > Okay.  In these cases, I am indeed accessing mapped reisters.  I don't know
> > what you mean by "proper acessors", so a reference would be appreciated.
> 
> ioread<N> / iowrite<N>

Would readq_relaxed()/writeq() be adequate?

thanks
Mark

Re: [PATCH] altix: Add volatile qualifiers in places where it seems necessary

[Christoph Hellwig]

On Fri, Sep 02, 2005 at 11:08:58AM -0500, Mark Maule wrote:
> On Fri, Sep 02, 2005 at 04:49:53PM +0100, Christoph Hellwig wrote:
> > On Fri, Sep 02, 2005 at 10:41:52AM -0500, Mark Maule wrote:
> > > On Fri, Sep 02, 2005 at 04:38:09PM +0100, Christoph Hellwig wrote:
> > > > On Fri, Sep 02, 2005 at 10:36:12AM -0500, Mark Maule wrote:
> > > > > Add missing volatile qualifier to hardware MMR pointers in various SN files.
> > > > > Cosmetic code rearranging in pcibr_reg.c to avoid line-wraps.
> > > > 
> > > > Volatile usage is wrong most of the time, and your use is certainly.
> > > > It seems you're accessing mmio-space or something similar and should
> > > > use proper acessors for it.
> > > > 
> > > 
> > > Okay.  In these cases, I am indeed accessing mapped reisters.  I don't know
> > > what you mean by "proper acessors", so a reference would be appreciated.
> > 
> > ioread<N> / iowrite<N>
> 
> Would readq_relaxed()/writeq() be adequate?

Yes, I think so.  In fact you might even want to __sn_read*/__sn_write*
directly as this code runs only on SN2, and you can avoid the unessecary
indirection for generic kernels.

[PATCH] altix: Add volatile qualifiers in places where it seems necessary

[Mark Maule]

Resend using accessors instead of volatile qualifiers per hch comments.  Added
a __sn_rwq_relaxed() convenience macro for doing read/modify/write ops
(e.g.  &=, |=).

Patch to apply volatile semantics when accessing MMR's in various SN files.

Signed-off-by: Mark Maule <maule@sgi.com>

Index: volatile/arch/ia64/sn/pci/pcibr/pcibr_reg.c
=================================--- volatile.orig/arch/ia64/sn/pci/pcibr/pcibr_reg.c	2005-09-02 09:13:09.993588678 -0500
+++ volatile/arch/ia64/sn/pci/pcibr/pcibr_reg.c	2005-09-02 13:31:17.970957143 -0500
@@ -29,10 +29,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ptr->tio.cp_control &= ~bits;
+			__sn_rwq_relaxed(&ptr->tio.cp_control, &=, ~bits);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ptr->pic.p_wid_control &= ~bits;
+			__sn_rwq_relaxed(&ptr->pic.p_wid_control, &=, ~bits);
 			break;
 		default:
 			panic
@@ -49,10 +49,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ptr->tio.cp_control |= bits;
+			__sn_rwq_relaxed(&ptr->tio.cp_control, |=, bits);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ptr->pic.p_wid_control |= bits;
+			__sn_rwq_relaxed(&ptr->pic.p_wid_control, |=, bits);
 			break;
 		default:
 			panic
@@ -73,10 +73,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ret = ptr->tio.cp_tflush;
+			ret = __sn_readq_relaxed(&ptr->tio.cp_tflush);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ret = ptr->pic.p_wid_tflush;
+			ret = __sn_readq_relaxed(&ptr->pic.p_wid_tflush);
 			break;
 		default:
 			panic
@@ -103,10 +103,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ret = ptr->tio.cp_int_status;
+			ret = __sn_readq_relaxed(&ptr->tio.cp_int_status);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ret = ptr->pic.p_int_status;
+			ret = __sn_readq_relaxed(&ptr->pic.p_int_status);
 			break;
 		default:
 			panic
@@ -127,10 +127,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ptr->tio.cp_int_enable &= ~bits;
+			__sn_rwq_relaxed(&ptr->tio.cp_int_enable, &=, ~bits);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ptr->pic.p_int_enable &= ~bits;
+			__sn_rwq_relaxed(&ptr->pic.p_int_enable, &=, ~bits);
 			break;
 		default:
 			panic
@@ -147,10 +147,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ptr->tio.cp_int_enable |= bits;
+			__sn_rwq_relaxed(&ptr->tio.cp_int_enable, |=, bits);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ptr->pic.p_int_enable |= bits;
+			__sn_rwq_relaxed(&ptr->pic.p_int_enable, |=, bits);
 			break;
 		default:
 			panic
@@ -171,14 +171,16 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ptr->tio.cp_int_addr[int_n] &= ~TIOCP_HOST_INTR_ADDR;
-			ptr->tio.cp_int_addr[int_n] |-			    (addr & TIOCP_HOST_INTR_ADDR);
+			__sn_rwq_relaxed(&ptr->tio.cp_int_addr[int_n], &=,
+			    ~TIOCP_HOST_INTR_ADDR);
+			__sn_rwq_relaxed(&ptr->tio.cp_int_addr[int_n], |=,
+			    (addr & TIOCP_HOST_INTR_ADDR));
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ptr->pic.p_int_addr[int_n] &= ~PIC_HOST_INTR_ADDR;
-			ptr->pic.p_int_addr[int_n] |-			    (addr & PIC_HOST_INTR_ADDR);
+			__sn_rwq_relaxed(&ptr->pic.p_int_addr[int_n], &=,
+			    ~PIC_HOST_INTR_ADDR);
+			__sn_rwq_relaxed(&ptr->pic.p_int_addr[int_n], |=,
+			    (addr & PIC_HOST_INTR_ADDR));
 			break;
 		default:
 			panic
@@ -198,10 +200,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ptr->tio.cp_force_pin[int_n] = 1;
+			writeq(1, &ptr->tio.cp_force_pin[int_n]);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ptr->pic.p_force_pin[int_n] = 1;
+			writeq(1, &ptr->pic.p_force_pin[int_n]);
 			break;
 		default:
 			panic
@@ -222,10 +224,12 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ret = ptr->tio.cp_wr_req_buf[device];
+			ret +			    __sn_readq_relaxed(&ptr->tio.cp_wr_req_buf[device]);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ret = ptr->pic.p_wr_req_buf[device];
+			ret +			    __sn_readq_relaxed(&ptr->pic.p_wr_req_buf[device]);
 			break;
 		default:
 		      panic("pcireg_wrb_flush_get: unknown bridgetype bridge 0x%p", (void *)ptr);
@@ -244,10 +248,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ptr->tio.cp_int_ate_ram[ate_index] = (uint64_t) val;
+			writeq(val, &ptr->tio.cp_int_ate_ram[ate_index]);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ptr->pic.p_int_ate_ram[ate_index] = (uint64_t) val;
+			writeq(val, &ptr->pic.p_int_ate_ram[ate_index]);
 			break;
 		default:
 			panic
@@ -265,12 +269,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ret -			    (uint64_t *) & (ptr->tio.cp_int_ate_ram[ate_index]);
+			ret = &ptr->tio.cp_int_ate_ram[ate_index];
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ret -			    (uint64_t *) & (ptr->pic.p_int_ate_ram[ate_index]);
+			ret = &ptr->pic.p_int_ate_ram[ate_index];
 			break;
 		default:
 			panic
Index: volatile/arch/ia64/sn/pci/tioca_provider.c
=================================--- volatile.orig/arch/ia64/sn/pci/tioca_provider.c	2005-09-02 09:13:09.993588678 -0500
+++ volatile/arch/ia64/sn/pci/tioca_provider.c	2005-09-02 13:32:41.618729313 -0500
@@ -37,7 +37,7 @@
 	uint64_t offset;
 	struct page *tmp;
 	struct tioca_common *tioca_common;
-	volatile struct tioca *ca_base;
+	struct tioca *ca_base;
 
 	tioca_common = tioca_kern->ca_common;
 	ca_base = (struct tioca *)tioca_common->ca_common.bs_base;
@@ -174,27 +174,30 @@
 	 * 	DISABLE GART PREFETCHING due to hw bug tracked in SGI PV930029
 	 */
 
-	ca_base->ca_control1 |= CA_AGPDMA_OP_ENB_COMBDELAY;	/* PV895469 ? */
-	ca_base->ca_control2 &= ~(CA_GART_MEM_PARAM);
-	ca_base->ca_control2 |= (0x2ull << CA_GART_MEM_PARAM_SHFT);
+	__sn_rwq_relaxed(&ca_base->ca_control1, |=,
+			CA_AGPDMA_OP_ENB_COMBDELAY);	/* PV895469 ? */
+	__sn_rwq_relaxed(&ca_base->ca_control2, &=,
+			~(CA_GART_MEM_PARAM));
+	__sn_rwq_relaxed(&ca_base->ca_control2, |=,
+			(0x2ull << CA_GART_MEM_PARAM_SHFT));
 	tioca_kern->ca_gart_iscoherent = 1;
-	ca_base->ca_control2 &-	    ~(CA_GART_WR_PREFETCH_ENB | CA_GART_RD_PREFETCH_ENB);
+	__sn_rwq_relaxed(&ca_base->ca_control2, &=,
+	    		~(CA_GART_WR_PREFETCH_ENB | CA_GART_RD_PREFETCH_ENB));
 
 	/*
 	 * Unmask GART fetch error interrupts.  Clear residual errors first.
 	 */
 
-	ca_base->ca_int_status_alias = CA_GART_FETCH_ERR;
-	ca_base->ca_mult_error_alias = CA_GART_FETCH_ERR;
-	ca_base->ca_int_mask &= ~CA_GART_FETCH_ERR;
+	writeq(CA_GART_FETCH_ERR, &ca_base->ca_int_status_alias);
+	writeq(CA_GART_FETCH_ERR, &ca_base->ca_mult_error_alias);
+	__sn_rwq_relaxed(&ca_base->ca_int_mask, &=, ~CA_GART_FETCH_ERR);
 
 	/*
 	 * Program the aperature and gart registers in TIOCA
 	 */
 
-	ca_base->ca_gart_aperature = ap_reg;
-	ca_base->ca_gart_ptr_table = tioca_kern->ca_gart_coretalk_addr | 1;
+	writeq(ap_reg, &ca_base->ca_gart_aperature);
+	writeq(tioca_kern->ca_gart_coretalk_addr|1, &ca_base->ca_gart_ptr_table);
 
 	return 0;
 }
@@ -211,7 +214,6 @@
 tioca_fastwrite_enable(struct tioca_kernel *tioca_kern)
 {
 	int cap_ptr;
-	uint64_t ca_control1;
 	uint32_t reg;
 	struct tioca *tioca_base;
 	struct pci_dev *pdev;
@@ -256,9 +258,7 @@
 	 */
 
 	tioca_base = (struct tioca *)common->ca_common.bs_base;
-	ca_control1 = tioca_base->ca_control1;
-	ca_control1 |= CA_AGP_FW_ENABLE;
-	tioca_base->ca_control1 = ca_control1;
+	__sn_rwq_relaxed(&tioca_base->ca_control1, |=, CA_AGP_FW_ENABLE);
 }
 
 EXPORT_SYMBOL(tioca_fastwrite_enable);	/* used by agp-sgi */
@@ -345,7 +345,7 @@
 		return 0;
 	}
 
-	agp_dma_extn = ca_base->ca_agp_dma_addr_extn;
+	agp_dma_extn = __sn_readq_relaxed(&ca_base->ca_agp_dma_addr_extn);
 	if (node_upper != (agp_dma_extn >> CA_AGP_DMA_NODE_ID_SHFT)) {
 		printk(KERN_ERR "%s:  coretalk upper node (%u) "
 		       "mismatch with ca_agp_dma_addr_extn (%lu)\n",
Index: volatile/arch/ia64/sn/pci/tioce_provider.c
=================================--- volatile.orig/arch/ia64/sn/pci/tioce_provider.c	2005-09-02 09:13:09.994565143 -0500
+++ volatile/arch/ia64/sn/pci/tioce_provider.c	2005-09-02 13:34:48.219167948 -0500
@@ -227,7 +227,7 @@
 
 		ate = ATE_MAKE(addr, pagesize);
 		ate_shadow[i + j] = ate;
-		ate_reg[i + j] = ate;
+		writeq(ate, &ate_reg[i + j]);
 		addr += pagesize;
 	}
 
@@ -268,10 +268,10 @@
 	pcidev_to_tioce(pdev, &ce_mmr, &ce_kern, &port);
 
 	if (ce_kern->ce_port[port].dirmap_refcnt = 0) {
-		volatile uint64_t tmp;
+		uint64_t tmp;
 
 		ce_kern->ce_port[port].dirmap_shadow = ct_upper;
-		ce_mmr->ce_ure_dir_map[port] = ct_upper;
+		writeq(ct_upper, &ce_mmr->ce_ure_dir_map[port]);
 		tmp = ce_mmr->ce_ure_dir_map[port];
 		dma_ok = 1;
 	} else
@@ -343,7 +343,7 @@
 	if (TIOCE_D32_ADDR(bus_addr)) {
 		if (--ce_kern->ce_port[port].dirmap_refcnt = 0) {
 			ce_kern->ce_port[port].dirmap_shadow = 0;
-			ce_mmr->ce_ure_dir_map[port] = 0;
+			writeq(0, &ce_mmr->ce_ure_dir_map[port]);
 		}
 	} else {
 		struct tioce_dmamap *map;
@@ -582,18 +582,18 @@
 	 */
 
 	tioce_mmr = (struct tioce *)tioce_common->ce_pcibus.bs_base;
-	tioce_mmr->ce_ure_page_map &= ~CE_URE_PAGESIZE_MASK;
-	tioce_mmr->ce_ure_page_map |= CE_URE_256K_PAGESIZE;
+	__sn_rwq_relaxed(&tioce_mmr->ce_ure_page_map, &=, ~CE_URE_PAGESIZE_MASK);
+	__sn_rwq_relaxed(&tioce_mmr->ce_ure_page_map, |=, CE_URE_256K_PAGESIZE);
 	tioce_kern->ce_ate3240_pagesize = KB(256);
 
 	for (i = 0; i < TIOCE_NUM_M40_ATES; i++) {
 		tioce_kern->ce_ate40_shadow[i] = 0;
-		tioce_mmr->ce_ure_ate40[i] = 0;
+		writeq(0, &tioce_mmr->ce_ure_ate40[i]);
 	}
 
 	for (i = 0; i < TIOCE_NUM_M3240_ATES; i++) {
 		tioce_kern->ce_ate3240_shadow[i] = 0;
-		tioce_mmr->ce_ure_ate3240[i] = 0;
+		writeq(0, &tioce_mmr->ce_ure_ate3240[i]);
 	}
 
 	return tioce_kern;
@@ -665,7 +665,7 @@
 	default:
 		return;
 	}
-	ce_mmr->ce_adm_force_int = force_int_val;
+	writeq(force_int_val, &ce_mmr->ce_adm_force_int);
 }
 
 /**
@@ -686,6 +686,7 @@
 	struct tioce_common *ce_common;
 	struct tioce *ce_mmr;
 	int bit;
+	uint64_t vector;
 
 	pcidev_info = (struct pcidev_info *)sn_irq_info->irq_pciioinfo;
 	if (!pcidev_info)
@@ -696,11 +697,11 @@
 
 	bit = sn_irq_info->irq_int_bit;
 
-	ce_mmr->ce_adm_int_mask |= (1UL << bit);
-	ce_mmr->ce_adm_int_dest[bit] -		((uint64_t)sn_irq_info->irq_irq << INTR_VECTOR_SHFT) |
-			   sn_irq_info->irq_xtalkaddr;
-	ce_mmr->ce_adm_int_mask &= ~(1UL << bit);
+	__sn_rwq_relaxed(&ce_mmr->ce_adm_int_mask, |=, (1UL << bit));
+	vector = (uint64_t)sn_irq_info->irq_irq << INTR_VECTOR_SHFT;
+	vector |= sn_irq_info->irq_xtalkaddr;
+	writeq(vector, &ce_mmr->ce_adm_int_dest[bit]);
+	__sn_rwq_relaxed(&ce_mmr->ce_adm_int_mask, &=, ~(1UL << bit));
 
 	tioce_force_interrupt(sn_irq_info);
 }
Index: volatile/include/asm-ia64/sn/io.h
=================================--- volatile.orig/include/asm-ia64/sn/io.h	2005-09-02 09:13:20.730793919 -0500
+++ volatile/include/asm-ia64/sn/io.h	2005-09-02 12:35:33.897375370 -0500
@@ -36,6 +36,13 @@
 #define __sn_readq_relaxed ___sn_readq_relaxed
 
 /*
+ * Convenience macros for read/modify/write operations using above accessors
+ */
+
+#define __sn_rwq_relaxed(addr, op, val) \
+	{uint64_t r = __sn_readq_relaxed(addr); r op (val); writeq(r, addr);}
+
+/*
  * The following routines are SN Platform specific, called when
  * a reference is made to inX/outX set macros.  SN Platform
  * inX set of macros ensures that Posted DMA writes on the
Index: volatile/include/asm-ia64/sn/tioca_provider.h
=================================--- volatile.orig/include/asm-ia64/sn/tioca_provider.h	2005-09-02 09:13:20.823558060 -0500
+++ volatile/include/asm-ia64/sn/tioca_provider.h	2005-09-02 12:44:15.174868982 -0500
@@ -182,11 +182,11 @@
 			 * touch every CL aligned GART entry.
 			 */
 
-			ca_base->ca_control2 &= ~(CA_GART_MEM_PARAM);
-			ca_base->ca_control2 |= CA_GART_FLUSH_TLB;
-			ca_base->ca_control2 |-			    (0x2ull << CA_GART_MEM_PARAM_SHFT);
-			tmp = ca_base->ca_control2;
+			__sn_rwq_relaxed(&ca_base->ca_control2, &=, ~(CA_GART_MEM_PARAM));
+			__sn_rwq_relaxed(&ca_base->ca_control2, |=, CA_GART_FLUSH_TLB);
+			__sn_rwq_relaxed(&ca_base->ca_control2, |=,
+			    (0x2ull << CA_GART_MEM_PARAM_SHFT));
+			tmp = __sn_readq(&ca_base->ca_control2);
 		}
 
 		return;
@@ -196,8 +196,8 @@
 	 * Gart in uncached mode ... need an explicit flush.
 	 */
 
-	ca_base->ca_control2 |= CA_GART_FLUSH_TLB;
-	tmp = ca_base->ca_control2;
+	__sn_rwq_relaxed(&ca_base->ca_control2, |=, CA_GART_FLUSH_TLB);
+	tmp = __sn_readq(&ca_base->ca_control2);
 }
 
 extern uint32_t	tioca_gart_found;

Re: [PATCH] altix: Add volatile qualifiers in places where it seems necessary

[Andreas Schwab]

Mark Maule <maule@sgi.com> writes:

> Index: volatile/include/asm-ia64/sn/io.h
> =================================> --- volatile.orig/include/asm-ia64/sn/io.h	2005-09-02 09:13:20.730793919 -0500
> +++ volatile/include/asm-ia64/sn/io.h	2005-09-02 12:35:33.897375370 -0500
> @@ -36,6 +36,13 @@
>  #define __sn_readq_relaxed ___sn_readq_relaxed
>  
>  /*
> + * Convenience macros for read/modify/write operations using above accessors
> + */
> +
> +#define __sn_rwq_relaxed(addr, op, val) \
> +	{uint64_t r = __sn_readq_relaxed(addr); r op (val); writeq(r, addr);}

The local variable should be renamed to something less likely to clash
with names used in the arguments.  The expansion should be surrounded by
do ... while(0) to make it a proper statement immune from the dangling
else problem.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] altix: Add volatile qualifiers in places where it seems necessary

[Russ Anderson]

Mark Maule wrote:
> 
> Resend using accessors instead of volatile qualifiers per hch comments.  Added
> a __sn_rwq_relaxed() convenience macro for doing read/modify/write ops
> (e.g.  &=, |=).

Instead of passing in op codes, why not have two macros:  __sn_set_rwq_relaxed()
and __sn_clear_rwq_relaxed(), since the operation is either setting or clearing bits.


>  	if (pcibus_info) {
>  		switch (pcibus_info->pbi_bridge_type) {
>  		case PCIBR_BRIDGETYPE_TIOCP:
> -			ptr->tio.cp_control &= ~bits;
> +			__sn_rwq_relaxed(&ptr->tio.cp_control, &=, ~bits);

			__sn_clear_rwq_relaxed(&ptr->tio.cp_control, bits);

>  	tioca_base = (struct tioca *)common->ca_common.bs_base;
> -	ca_control1 = tioca_base->ca_control1;
> -	ca_control1 |= CA_AGP_FW_ENABLE;
> -	tioca_base->ca_control1 = ca_control1;
> +	__sn_rwq_relaxed(&tioca_base->ca_control1, |=, CA_AGP_FW_ENABLE);

	__sn_set_rwq_relaxed(&tioca_base->ca_control1, CA_AGP_FW_ENABLE);

>  /*
> + * Convenience macros for read/modify/write operations using above accessors
> + */
> +
> +#define __sn_rwq_relaxed(addr, op, val) \
> +	{uint64_t r = __sn_readq_relaxed(addr); r op (val); writeq(r, addr);}


#define __sn_set_rwq_relaxed(addr, val) \
	writeq((__sn_readq_relaxed(addr) | (val)), (addr))

#define __sn_clear_rwq_relaxed(addr, val) \
	writeq((__sn_readq_relaxed(addr) & ~(val)), (addr))



-- 
Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          rja@sgi.com

[PATCH] altix: Add volatile qualifiers in places where it seems necessary

[Mark Maule]

Resend using accessors instead of volatile qualifiers per hch comments, and
easier to understand convenience macros per rja comments.

Patch to apply volatile semantics when accessing MMR's in various SN files.

Signed-off-by: Mark Maule <maule@sgi.com>

Index: volatile/arch/ia64/sn/pci/pcibr/pcibr_reg.c
=================================--- volatile.orig/arch/ia64/sn/pci/pcibr/pcibr_reg.c	2005-09-02 09:13:09.993588678 -0500
+++ volatile/arch/ia64/sn/pci/pcibr/pcibr_reg.c	2005-09-06 09:56:23.032131661 -0500
@@ -29,10 +29,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ptr->tio.cp_control &= ~bits;
+			__sn_clrq_relaxed(&ptr->tio.cp_control, bits);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ptr->pic.p_wid_control &= ~bits;
+			__sn_clrq_relaxed(&ptr->pic.p_wid_control, bits);
 			break;
 		default:
 			panic
@@ -49,10 +49,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ptr->tio.cp_control |= bits;
+			__sn_setq_relaxed(&ptr->tio.cp_control, bits);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ptr->pic.p_wid_control |= bits;
+			__sn_setq_relaxed(&ptr->pic.p_wid_control, bits);
 			break;
 		default:
 			panic
@@ -73,10 +73,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ret = ptr->tio.cp_tflush;
+			ret = __sn_readq_relaxed(&ptr->tio.cp_tflush);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ret = ptr->pic.p_wid_tflush;
+			ret = __sn_readq_relaxed(&ptr->pic.p_wid_tflush);
 			break;
 		default:
 			panic
@@ -103,10 +103,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ret = ptr->tio.cp_int_status;
+			ret = __sn_readq_relaxed(&ptr->tio.cp_int_status);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ret = ptr->pic.p_int_status;
+			ret = __sn_readq_relaxed(&ptr->pic.p_int_status);
 			break;
 		default:
 			panic
@@ -127,10 +127,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ptr->tio.cp_int_enable &= ~bits;
+			__sn_clrq_relaxed(&ptr->tio.cp_int_enable, bits);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ptr->pic.p_int_enable &= ~bits;
+			__sn_clrq_relaxed(&ptr->pic.p_int_enable, ~bits);
 			break;
 		default:
 			panic
@@ -147,10 +147,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ptr->tio.cp_int_enable |= bits;
+			__sn_setq_relaxed(&ptr->tio.cp_int_enable, bits);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ptr->pic.p_int_enable |= bits;
+			__sn_setq_relaxed(&ptr->pic.p_int_enable, bits);
 			break;
 		default:
 			panic
@@ -171,14 +171,16 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ptr->tio.cp_int_addr[int_n] &= ~TIOCP_HOST_INTR_ADDR;
-			ptr->tio.cp_int_addr[int_n] |-			    (addr & TIOCP_HOST_INTR_ADDR);
+			__sn_clrq_relaxed(&ptr->tio.cp_int_addr[int_n],
+			    TIOCP_HOST_INTR_ADDR);
+			__sn_setq_relaxed(&ptr->tio.cp_int_addr[int_n],
+			    (addr & TIOCP_HOST_INTR_ADDR));
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ptr->pic.p_int_addr[int_n] &= ~PIC_HOST_INTR_ADDR;
-			ptr->pic.p_int_addr[int_n] |-			    (addr & PIC_HOST_INTR_ADDR);
+			__sn_clrq_relaxed(&ptr->pic.p_int_addr[int_n],
+			    PIC_HOST_INTR_ADDR);
+			__sn_setq_relaxed(&ptr->pic.p_int_addr[int_n],
+			    (addr & PIC_HOST_INTR_ADDR));
 			break;
 		default:
 			panic
@@ -198,10 +200,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ptr->tio.cp_force_pin[int_n] = 1;
+			writeq(1, &ptr->tio.cp_force_pin[int_n]);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ptr->pic.p_force_pin[int_n] = 1;
+			writeq(1, &ptr->pic.p_force_pin[int_n]);
 			break;
 		default:
 			panic
@@ -222,10 +224,12 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ret = ptr->tio.cp_wr_req_buf[device];
+			ret +			    __sn_readq_relaxed(&ptr->tio.cp_wr_req_buf[device]);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ret = ptr->pic.p_wr_req_buf[device];
+			ret +			    __sn_readq_relaxed(&ptr->pic.p_wr_req_buf[device]);
 			break;
 		default:
 		      panic("pcireg_wrb_flush_get: unknown bridgetype bridge 0x%p", (void *)ptr);
@@ -244,10 +248,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ptr->tio.cp_int_ate_ram[ate_index] = (uint64_t) val;
+			writeq(val, &ptr->tio.cp_int_ate_ram[ate_index]);
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ptr->pic.p_int_ate_ram[ate_index] = (uint64_t) val;
+			writeq(val, &ptr->pic.p_int_ate_ram[ate_index]);
 			break;
 		default:
 			panic
@@ -265,12 +269,10 @@
 	if (pcibus_info) {
 		switch (pcibus_info->pbi_bridge_type) {
 		case PCIBR_BRIDGETYPE_TIOCP:
-			ret -			    (uint64_t *) & (ptr->tio.cp_int_ate_ram[ate_index]);
+			ret = &ptr->tio.cp_int_ate_ram[ate_index];
 			break;
 		case PCIBR_BRIDGETYPE_PIC:
-			ret -			    (uint64_t *) & (ptr->pic.p_int_ate_ram[ate_index]);
+			ret = &ptr->pic.p_int_ate_ram[ate_index];
 			break;
 		default:
 			panic
Index: volatile/arch/ia64/sn/pci/tioca_provider.c
=================================--- volatile.orig/arch/ia64/sn/pci/tioca_provider.c	2005-09-02 09:13:09.993588678 -0500
+++ volatile/arch/ia64/sn/pci/tioca_provider.c	2005-09-06 09:56:23.032131661 -0500
@@ -37,7 +37,7 @@
 	uint64_t offset;
 	struct page *tmp;
 	struct tioca_common *tioca_common;
-	volatile struct tioca *ca_base;
+	struct tioca *ca_base;
 
 	tioca_common = tioca_kern->ca_common;
 	ca_base = (struct tioca *)tioca_common->ca_common.bs_base;
@@ -174,27 +174,29 @@
 	 * 	DISABLE GART PREFETCHING due to hw bug tracked in SGI PV930029
 	 */
 
-	ca_base->ca_control1 |= CA_AGPDMA_OP_ENB_COMBDELAY;	/* PV895469 ? */
-	ca_base->ca_control2 &= ~(CA_GART_MEM_PARAM);
-	ca_base->ca_control2 |= (0x2ull << CA_GART_MEM_PARAM_SHFT);
+	__sn_setq_relaxed(&ca_base->ca_control1,
+			CA_AGPDMA_OP_ENB_COMBDELAY);	/* PV895469 ? */
+	__sn_clrq_relaxed(&ca_base->ca_control2, CA_GART_MEM_PARAM);
+	__sn_setq_relaxed(&ca_base->ca_control2,
+			(0x2ull << CA_GART_MEM_PARAM_SHFT));
 	tioca_kern->ca_gart_iscoherent = 1;
-	ca_base->ca_control2 &-	    ~(CA_GART_WR_PREFETCH_ENB | CA_GART_RD_PREFETCH_ENB);
+	__sn_clrq_relaxed(&ca_base->ca_control2,
+	    		(CA_GART_WR_PREFETCH_ENB | CA_GART_RD_PREFETCH_ENB));
 
 	/*
 	 * Unmask GART fetch error interrupts.  Clear residual errors first.
 	 */
 
-	ca_base->ca_int_status_alias = CA_GART_FETCH_ERR;
-	ca_base->ca_mult_error_alias = CA_GART_FETCH_ERR;
-	ca_base->ca_int_mask &= ~CA_GART_FETCH_ERR;
+	writeq(CA_GART_FETCH_ERR, &ca_base->ca_int_status_alias);
+	writeq(CA_GART_FETCH_ERR, &ca_base->ca_mult_error_alias);
+	__sn_clrq_relaxed(&ca_base->ca_int_mask, CA_GART_FETCH_ERR);
 
 	/*
 	 * Program the aperature and gart registers in TIOCA
 	 */
 
-	ca_base->ca_gart_aperature = ap_reg;
-	ca_base->ca_gart_ptr_table = tioca_kern->ca_gart_coretalk_addr | 1;
+	writeq(ap_reg, &ca_base->ca_gart_aperature);
+	writeq(tioca_kern->ca_gart_coretalk_addr|1, &ca_base->ca_gart_ptr_table);
 
 	return 0;
 }
@@ -211,7 +213,6 @@
 tioca_fastwrite_enable(struct tioca_kernel *tioca_kern)
 {
 	int cap_ptr;
-	uint64_t ca_control1;
 	uint32_t reg;
 	struct tioca *tioca_base;
 	struct pci_dev *pdev;
@@ -256,9 +257,7 @@
 	 */
 
 	tioca_base = (struct tioca *)common->ca_common.bs_base;
-	ca_control1 = tioca_base->ca_control1;
-	ca_control1 |= CA_AGP_FW_ENABLE;
-	tioca_base->ca_control1 = ca_control1;
+	__sn_setq_relaxed(&tioca_base->ca_control1, CA_AGP_FW_ENABLE);
 }
 
 EXPORT_SYMBOL(tioca_fastwrite_enable);	/* used by agp-sgi */
@@ -345,7 +344,7 @@
 		return 0;
 	}
 
-	agp_dma_extn = ca_base->ca_agp_dma_addr_extn;
+	agp_dma_extn = __sn_readq_relaxed(&ca_base->ca_agp_dma_addr_extn);
 	if (node_upper != (agp_dma_extn >> CA_AGP_DMA_NODE_ID_SHFT)) {
 		printk(KERN_ERR "%s:  coretalk upper node (%u) "
 		       "mismatch with ca_agp_dma_addr_extn (%lu)\n",
Index: volatile/arch/ia64/sn/pci/tioce_provider.c
=================================--- volatile.orig/arch/ia64/sn/pci/tioce_provider.c	2005-09-02 09:13:09.994565143 -0500
+++ volatile/arch/ia64/sn/pci/tioce_provider.c	2005-09-06 09:56:23.032131661 -0500
@@ -227,7 +227,7 @@
 
 		ate = ATE_MAKE(addr, pagesize);
 		ate_shadow[i + j] = ate;
-		ate_reg[i + j] = ate;
+		writeq(ate, &ate_reg[i + j]);
 		addr += pagesize;
 	}
 
@@ -268,10 +268,10 @@
 	pcidev_to_tioce(pdev, &ce_mmr, &ce_kern, &port);
 
 	if (ce_kern->ce_port[port].dirmap_refcnt = 0) {
-		volatile uint64_t tmp;
+		uint64_t tmp;
 
 		ce_kern->ce_port[port].dirmap_shadow = ct_upper;
-		ce_mmr->ce_ure_dir_map[port] = ct_upper;
+		writeq(ct_upper, &ce_mmr->ce_ure_dir_map[port]);
 		tmp = ce_mmr->ce_ure_dir_map[port];
 		dma_ok = 1;
 	} else
@@ -343,7 +343,7 @@
 	if (TIOCE_D32_ADDR(bus_addr)) {
 		if (--ce_kern->ce_port[port].dirmap_refcnt = 0) {
 			ce_kern->ce_port[port].dirmap_shadow = 0;
-			ce_mmr->ce_ure_dir_map[port] = 0;
+			writeq(0, &ce_mmr->ce_ure_dir_map[port]);
 		}
 	} else {
 		struct tioce_dmamap *map;
@@ -582,18 +582,18 @@
 	 */
 
 	tioce_mmr = (struct tioce *)tioce_common->ce_pcibus.bs_base;
-	tioce_mmr->ce_ure_page_map &= ~CE_URE_PAGESIZE_MASK;
-	tioce_mmr->ce_ure_page_map |= CE_URE_256K_PAGESIZE;
+	__sn_clrq_relaxed(&tioce_mmr->ce_ure_page_map, CE_URE_PAGESIZE_MASK);
+	__sn_setq_relaxed(&tioce_mmr->ce_ure_page_map, CE_URE_256K_PAGESIZE);
 	tioce_kern->ce_ate3240_pagesize = KB(256);
 
 	for (i = 0; i < TIOCE_NUM_M40_ATES; i++) {
 		tioce_kern->ce_ate40_shadow[i] = 0;
-		tioce_mmr->ce_ure_ate40[i] = 0;
+		writeq(0, &tioce_mmr->ce_ure_ate40[i]);
 	}
 
 	for (i = 0; i < TIOCE_NUM_M3240_ATES; i++) {
 		tioce_kern->ce_ate3240_shadow[i] = 0;
-		tioce_mmr->ce_ure_ate3240[i] = 0;
+		writeq(0, &tioce_mmr->ce_ure_ate3240[i]);
 	}
 
 	return tioce_kern;
@@ -665,7 +665,7 @@
 	default:
 		return;
 	}
-	ce_mmr->ce_adm_force_int = force_int_val;
+	writeq(force_int_val, &ce_mmr->ce_adm_force_int);
 }
 
 /**
@@ -686,6 +686,7 @@
 	struct tioce_common *ce_common;
 	struct tioce *ce_mmr;
 	int bit;
+	uint64_t vector;
 
 	pcidev_info = (struct pcidev_info *)sn_irq_info->irq_pciioinfo;
 	if (!pcidev_info)
@@ -696,11 +697,11 @@
 
 	bit = sn_irq_info->irq_int_bit;
 
-	ce_mmr->ce_adm_int_mask |= (1UL << bit);
-	ce_mmr->ce_adm_int_dest[bit] -		((uint64_t)sn_irq_info->irq_irq << INTR_VECTOR_SHFT) |
-			   sn_irq_info->irq_xtalkaddr;
-	ce_mmr->ce_adm_int_mask &= ~(1UL << bit);
+	__sn_setq_relaxed(&ce_mmr->ce_adm_int_mask, (1UL << bit));
+	vector = (uint64_t)sn_irq_info->irq_irq << INTR_VECTOR_SHFT;
+	vector |= sn_irq_info->irq_xtalkaddr;
+	writeq(vector, &ce_mmr->ce_adm_int_dest[bit]);
+	__sn_clrq_relaxed(&ce_mmr->ce_adm_int_mask, (1UL << bit));
 
 	tioce_force_interrupt(sn_irq_info);
 }
Index: volatile/include/asm-ia64/sn/io.h
=================================--- volatile.orig/include/asm-ia64/sn/io.h	2005-09-02 09:13:20.730793919 -0500
+++ volatile/include/asm-ia64/sn/io.h	2005-09-06 09:56:23.032131661 -0500
@@ -36,6 +36,15 @@
 #define __sn_readq_relaxed ___sn_readq_relaxed
 
 /*
+ * Convenience macros for setting/clearing bits using the above accessors
+ */
+
+#define __sn_setq_relaxed(addr, val) \
+	writeq((__sn_readq_relaxed(addr) | (val)), (addr))
+#define __sn_clrq_relaxed(addr, val) \
+	writeq((__sn_readq_relaxed(addr) & ~(val)), (addr))
+
+/*
  * The following routines are SN Platform specific, called when
  * a reference is made to inX/outX set macros.  SN Platform
  * inX set of macros ensures that Posted DMA writes on the
Index: volatile/include/asm-ia64/sn/tioca_provider.h
=================================--- volatile.orig/include/asm-ia64/sn/tioca_provider.h	2005-09-02 09:13:20.823558060 -0500
+++ volatile/include/asm-ia64/sn/tioca_provider.h	2005-09-06 10:27:41.025752340 -0500
@@ -182,11 +182,11 @@
 			 * touch every CL aligned GART entry.
 			 */
 
-			ca_base->ca_control2 &= ~(CA_GART_MEM_PARAM);
-			ca_base->ca_control2 |= CA_GART_FLUSH_TLB;
-			ca_base->ca_control2 |-			    (0x2ull << CA_GART_MEM_PARAM_SHFT);
-			tmp = ca_base->ca_control2;
+			__sn_clrq_relaxed(&ca_base->ca_control2, CA_GART_MEM_PARAM);
+			__sn_setq_relaxed(&ca_base->ca_control2, CA_GART_FLUSH_TLB);
+			__sn_setq_relaxed(&ca_base->ca_control2,
+			    (0x2ull << CA_GART_MEM_PARAM_SHFT));
+			tmp = __sn_readq_relaxed(&ca_base->ca_control2);
 		}
 
 		return;
@@ -196,8 +196,8 @@
 	 * Gart in uncached mode ... need an explicit flush.
 	 */
 
-	ca_base->ca_control2 |= CA_GART_FLUSH_TLB;
-	tmp = ca_base->ca_control2;
+	__sn_setq_relaxed(&ca_base->ca_control2, CA_GART_FLUSH_TLB);
+	tmp = __sn_readq_relaxed(&ca_base->ca_control2);
 }
 
 extern uint32_t	tioca_gart_found;

RE: [PATCH] altix: Add volatile qualifiers in places where it seems necessary

[Luck, Tony]

>+			__sn_clrq_relaxed(&ptr->tio.cp_control, bits);
>+			__sn_setq_relaxed(&ptr->tio.cp_control, bits);

Breaks the build for bigsur, generic and hpsim configs with several
"undefined reference to" link errors for these two symbols.

Probably means that the bigsur/hpsim defconfigs are dragging in
things that they shouldn't.  But the errors on the generic build
are more worrying.

-Tony

RE: [PATCH] altix: Add volatile qualifiers in places where it seems necessary

[Luck, Tony]

>>+			__sn_clrq_relaxed(&ptr->tio.cp_control, bits);
>>+			__sn_setq_relaxed(&ptr->tio.cp_control, bits);
>
>Breaks the build for bigsur, generic and hpsim configs with several
>"undefined reference to" link errors for these two symbols.
>
>Probably means that the bigsur/hpsim defconfigs are dragging in
>things that they shouldn't.  But the errors on the generic build
>are more worrying.

Broken generic build was real ... caused by a few missing includes
of asm/sn/io.h to provide the #defines for these macros.

The bigsur/hpsim problems turned out to be a couple of typos in my
auto-build scripts (which turn out to have been building generic
kernels instead of bigsur/hpsim for the past few weeks).

All fixed now.  This patch, and a few others are now in my test tree.

-Tony