Skip to contentSkip to conversation

mtd: nand: pxa3xx: fix build on ARM64

PATCH Superseded Rob Herring 3 participants 5 messages Feb 3, 2015 – Feb 6, 2015

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mtd: nand: pxa3xx: fix build on ARM64

In preparation to enable ARCH_MMP on ARM64, a couple of fixes are needed to build the pxa3xx_nand driver:

Legacy DMA will only used on ARM, so also make it condtional on CONFIG_ARM.
__raw_{read,write}sl are not available on ARM64 or generically, so use the readsl/writesl variants instead.

Cc Ezequiel Garcia <[email protected]>
Cc David Woodhouse <[email protected]>
Cc Brian Norris <[email protected]>
---
 drivers/mtd/nand/pxa3xx_nand.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)
drivers/mtd/nand/pxa3xx_nand.c +14 -14
index 96b0b1d..404d390 100644
@@ -28,7 +28,7 @@
 #include <linux/of_device.h>
 #include <linux/of_mtd.h>
 
-#if defined(CONFIG_ARCH_PXA) || defined(CONFIG_ARCH_MMP)
+#if defined(CONFIG_ARM) && (defined(CONFIG_ARCH_PXA) || defined(CONFIG_ARCH_MMP))
 #define ARCH_HAS_DMA
 #endif
 
@@ -486,24 +486,24 @@ static void handle_data_pio(struct pxa3xx_nand_info *info)
 
 	switch (info->state) {
 	case STATE_PIO_WRITING:
-		__raw_writesl(info->mmio_base + NDDB,
-			      info->data_buff + info->data_buff_pos,
-			      DIV_ROUND_UP(do_bytes, 4));
+		writesl(info->mmio_base + NDDB,
+			info->data_buff + info->data_buff_pos,
+			DIV_ROUND_UP(do_bytes, 4));
 
 		if (info->oob_size > 0)
-			__raw_writesl(info->mmio_base + NDDB,
-				      info->oob_buff + info->oob_buff_pos,
-				      DIV_ROUND_UP(info->oob_size, 4));
+			writesl(info->mmio_base + NDDB,
+				info->oob_buff + info->oob_buff_pos,
+				DIV_ROUND_UP(info->oob_size, 4));
 		break;
 	case STATE_PIO_READING:
-		__raw_readsl(info->mmio_base + NDDB,
-			     info->data_buff + info->data_buff_pos,
-			     DIV_ROUND_UP(do_bytes, 4));
+		readsl(info->mmio_base + NDDB,
+		       info->data_buff + info->data_buff_pos,
+		       DIV_ROUND_UP(do_bytes, 4));
 
 		if (info->oob_size > 0)
-			__raw_readsl(info->mmio_base + NDDB,
-				     info->oob_buff + info->oob_buff_pos,
-				     DIV_ROUND_UP(info->oob_size, 4));
+			readsl(info->mmio_base + NDDB,
+			       info->oob_buff + info->oob_buff_pos,
+			       DIV_ROUND_UP(info->oob_size, 4));
 		break;
 	default:
 		dev_err(&info->pdev->dev, "%s: invalid state %d\n", __func__,
@@ -1580,7 +1580,7 @@ static int alloc_nand_resource(struct platform_device *pdev)
 	info->pdev = pdev;
 	info->variant = pxa3xx_nand_get_variant(pdev);
 	for (cs = 0; cs < pdata->num_cs; cs++) {
-		mtd = (struct mtd_info *)((unsigned int)&info[1] +
+		mtd = (struct mtd_info *)((void *)&info[1] +
 		      (sizeof(*mtd) + sizeof(*host)) * cs);
 		chip = (struct nand_chip *)(&mtd[1]);
 		host = (struct pxa3xx_nand_host *)chip;
Signature · 1 line
2.1.0
#1 lore raw
Quoted 7 lines · Rob Herring

In preparation to enable ARCH_MMP on ARM64, a couple of fixes are needed to build the pxa3xx_nand driver:

Legacy DMA will only used on ARM, so also make it condtional on CONFIG_ARM.
__raw_{read,write}sl are not available on ARM64 or generically, so use the readsl/writesl variants instead.

Hmm, why the non __raw naming? I seems a little misleading that all the other (non-raw) {read,write}{w,l}() helpers have endian swapping and barriers, but this helper has neither.

Anyway, I think the patch looks good. Thomas or Ezequiel, can I get an ack?

Quoted 60 lines
Cc Ezequiel Garcia <[email protected]>
Cc David Woodhouse <[email protected]>
Cc Brian Norris <[email protected]>
---
 drivers/mtd/nand/pxa3xx_nand.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)
drivers/mtd/nand/pxa3xx_nand.c +13 -13
index 96b0b1d..404d390 100644
@@ -28,7 +28,7 @@
 #include <linux/of_device.h>
 #include <linux/of_mtd.h>
 
-#if defined(CONFIG_ARCH_PXA) || defined(CONFIG_ARCH_MMP)
+#if defined(CONFIG_ARM) && (defined(CONFIG_ARCH_PXA) || defined(CONFIG_ARCH_MMP))
 #define ARCH_HAS_DMA
 #endif
 
@@ -486,24 +486,24 @@ static void handle_data_pio(struct pxa3xx_nand_info *info)
 
 	switch (info->state) {
 	case STATE_PIO_WRITING:
-		__raw_writesl(info->mmio_base + NDDB,
-			      info->data_buff + info->data_buff_pos,
-			      DIV_ROUND_UP(do_bytes, 4));
+		writesl(info->mmio_base + NDDB,
+			info->data_buff + info->data_buff_pos,
+			DIV_ROUND_UP(do_bytes, 4));
 
 		if (info->oob_size > 0)
-			__raw_writesl(info->mmio_base + NDDB,
-				      info->oob_buff + info->oob_buff_pos,
-				      DIV_ROUND_UP(info->oob_size, 4));
+			writesl(info->mmio_base + NDDB,
+				info->oob_buff + info->oob_buff_pos,
+				DIV_ROUND_UP(info->oob_size, 4));
 		break;
 	case STATE_PIO_READING:
-		__raw_readsl(info->mmio_base + NDDB,
-			     info->data_buff + info->data_buff_pos,
-			     DIV_ROUND_UP(do_bytes, 4));
+		readsl(info->mmio_base + NDDB,
+		       info->data_buff + info->data_buff_pos,
+		       DIV_ROUND_UP(do_bytes, 4));
 
 		if (info->oob_size > 0)
-			__raw_readsl(info->mmio_base + NDDB,
-				     info->oob_buff + info->oob_buff_pos,
-				     DIV_ROUND_UP(info->oob_size, 4));
+			readsl(info->mmio_base + NDDB,
+			       info->oob_buff + info->oob_buff_pos,
+			       DIV_ROUND_UP(info->oob_size, 4));
 		break;
 	default:
 		dev_err(&info->pdev->dev, "%s: invalid state %d\n", __func__,

drivers/mtd/nand/pxa3xx_nand.c static int alloc_nand_resource(struct platform_device *pdev) · line 1586

Quoted 8 lines
drivers/mtd/nand/pxa3xx_nand.c +1 -1
@@ -1580,7 +1580,7 @@ static int alloc_nand_resource(struct platform_device *pdev)
 	info->pdev = pdev;
 	info->variant = pxa3xx_nand_get_variant(pdev);
 	for (cs = 0; cs < pdata->num_cs; cs++) {
-		mtd = (struct mtd_info *)((unsigned int)&info[1] +
+		mtd = (struct mtd_info *)((void *)&info[1] +
 		      (sizeof(*mtd) + sizeof(*host)) * cs);
 		chip = (struct nand_chip *)(&mtd[1]);
 		host = (struct pxa3xx_nand_host *)chip;

Brian

#2 lore raw
In reply to Rob Herring

On 02/03/2015 08:06 PM, Rob Herring wrote:
[..]

drivers/mtd/nand/pxa3xx_nand.c static int alloc_nand_resource(struct platform_device *pdev) · line 1586

Quoted 10 lines
drivers/mtd/nand/pxa3xx_nand.c +1 -1
@@ -1580,7 +1580,7 @@ static int alloc_nand_resource(struct platform_device *pdev)
 	info->pdev = pdev;
 	info->variant = pxa3xx_nand_get_variant(pdev);
 	for (cs = 0; cs < pdata->num_cs; cs++) {
-		mtd = (struct mtd_info *)((unsigned int)&info[1] +
+		mtd = (struct mtd_info *)((void *)&info[1] +
 		      (sizeof(*mtd) + sizeof(*host)) * cs);
 		chip = (struct nand_chip *)(&mtd[1]);
 		host = (struct pxa3xx_nand_host *)chip;

Those casts are an eyesore to me. Is this change related to the patch?

Signature · 3 lines
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
#3 lore raw
Quoted 14 lines · Ezequiel Garcia

On 02/03/2015 08:06 PM, Rob Herring wrote:
[..]

drivers/mtd/nand/pxa3xx_nand.c static int alloc_nand_resource(struct platform_device *pdev) · line 1586

Quoted 10 lines
drivers/mtd/nand/pxa3xx_nand.c +1 -1
@@ -1580,7 +1580,7 @@ static int alloc_nand_resource(struct platform_device *pdev)
 	info->pdev = pdev;
 	info->variant = pxa3xx_nand_get_variant(pdev);
 	for (cs = 0; cs < pdata->num_cs; cs++) {
-		mtd = (struct mtd_info *)((unsigned int)&info[1] +
+		mtd = (struct mtd_info *)((void *)&info[1] +
 		      (sizeof(*mtd) + sizeof(*host)) * cs);
 		chip = (struct nand_chip *)(&mtd[1]);
 		host = (struct pxa3xx_nand_host *)chip;

Those casts are an eyesore to me.

I suppose the (struct mtd_info *) cast is unecessary now, since the (void *) will be implicitly casted just fine. But I'm not sure if it's worth dropping it.

Quoted 1 line

Is this change related to the patch?

I believe the (void *) cast is a necessary change because sizeof(pointer) != sizeof(unsigned int) on a 64-bit arch, so the (unsigned int) cast would (rightly) generate a warning about the unsafe cast. You're losing the top 32 bits.

Brian

#4 lore raw
Quoted 20 lines · Brian Norris
Quoted 14 lines · Ezequiel Garcia

On 02/03/2015 08:06 PM, Rob Herring wrote:
[..]

drivers/mtd/nand/pxa3xx_nand.c static int alloc_nand_resource(struct platform_device *pdev) · line 1586

Quoted 10 lines
drivers/mtd/nand/pxa3xx_nand.c +1 -1
@@ -1580,7 +1580,7 @@ static int alloc_nand_resource(struct platform_device *pdev)
 	info->pdev = pdev;
 	info->variant = pxa3xx_nand_get_variant(pdev);
 	for (cs = 0; cs < pdata->num_cs; cs++) {
-		mtd = (struct mtd_info *)((unsigned int)&info[1] +
+		mtd = (struct mtd_info *)((void *)&info[1] +
 		      (sizeof(*mtd) + sizeof(*host)) * cs);
 		chip = (struct nand_chip *)(&mtd[1]);
 		host = (struct pxa3xx_nand_host *)chip;

Those casts are an eyesore to me.

I suppose the (struct mtd_info *) cast is unecessary now, since the (void *) will be implicitly casted just fine. But I'm not sure if it's worth dropping it.

Quoted 7 lines
Quoted 1 line

Is this change related to the patch?

I believe the (void *) cast is a necessary change because sizeof(pointer) != sizeof(unsigned int) on a 64-bit arch, so the (unsigned int) cast would (rightly) generate a warning about the unsafe cast. You're losing the top 32 bits.

Right, that makes sense. Thanks for the clarification.

Signature · 3 lines
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com