Skip to contentSkip to conversation

mips: Let cache.h be included from assembly source

PATCH New Marek Vasut 5 participants 41 messages Nov 25, 2016 – Dec 1, 2016

Subsystems (6)
lore
Share & tools
lore
#0 lore raw

PATCH 1/14 mips: Let cache.h be included from assembly source

Add ifdef __ASSEMBLY__ around the function prototype to let cache.h be included from assembly code.

Cc Daniel Schwierzeck <[email protected]>
Cc Paul Burton <[email protected]>
---
 arch/mips/include/asm/cache.h | 2 ++
 1 file changed, 2 insertions(+)
arch/mips/include/asm/cache.h +2 -0
index 669c362..83165d5 100644
@@ -19,6 +19,7 @@
  */
 #define CONFIG_SYS_CACHELINE_SIZE ARCH_DMA_MINALIGN
 
+#ifndef __ASSEMBLY__
 /**
  * mips_cache_probe() - Probe the properties of the caches
  *
@@ -27,5 +28,6 @@
  * functions such as flush_cache may be called.
  */
 void mips_cache_probe(void);
+#endif
 
 #endif /* __MIPS_CACHE_H__ */
Signature · 1 line
2.10.2
#1 lore raw

PATCH 2/14 spl: mmc: Fix build without LIBCOMMON_SUPPORT

If CONFIG_SPL_LIBCOMMON_SUPPORT is undefined, the following error will happen, so fix it.

In file included from common/spl/spl_mmc.c:11:0:
common/spl/spl_mmc.c: In function ?spl_mmc_load_image?:
include/spl.h:18:30: error: label at end of compound statement

 #define MMCSD_MODE_UNDEFINED 0
                              ^
common/spl/spl_mmc.c:335:7: note: in expansion of macro ?MMCSD_MODE_UNDEFINED?
  case MMCSD_MODE_UNDEFINED:
       ^
Cc Pantelis Antoniou <[email protected]>
Cc Tom Rini <[email protected]>
---
 common/spl/spl_mmc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
common/spl/spl_mmc.c +3 -1
index 0b681c2..43c1030 100644
@@ -343,10 +343,12 @@ static int spl_mmc_load_image(struct spl_image_info *spl_image,
 
 		break;
 	case MMCSD_MODE_UNDEFINED:
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 	default:
+		{
+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 		puts("spl: mmc: wrong boot mode\n");
 #endif
+		}
 	}
 
 	return err;
Signature · 1 line
2.10.2
#2 lore raw

PATCH 2/14 spl: mmc: Fix build without LIBCOMMON_SUPPORT

Quoted 26 lines · Marek Vasut

If CONFIG_SPL_LIBCOMMON_SUPPORT is undefined, the following error will happen, so fix it.

In file included from common/spl/spl_mmc.c:11:0:
common/spl/spl_mmc.c: In function ?spl_mmc_load_image?:
include/spl.h:18:30: error: label at end of compound statement

 #define MMCSD_MODE_UNDEFINED 0
                              ^
common/spl/spl_mmc.c:335:7: note: in expansion of macro ?MMCSD_MODE_UNDEFINED?
  case MMCSD_MODE_UNDEFINED:
       ^
Cc Pantelis Antoniou <[email protected]>
Cc Tom Rini <[email protected]>
---
 common/spl/spl_mmc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
common/spl/spl_mmc.c +0 -0
index 0b681c2..43c1030 100644
@@ -343,10 +343,12 @@ static int spl_mmc_load_image(struct spl_image_info *spl_image,
 
 		break;
 	case MMCSD_MODE_UNDEFINED:

common/spl/spl_mmc.c static int spl_mmc_load_image(struct spl_image_info *spl_image, · line 354

Quoted 10 lines
common/spl/spl_mmc.c +3 -1
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 	default:
+		{
+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 		puts("spl: mmc: wrong boot mode\n");
 #endif
+		}
 	}
 
 	return err;

I think we have something slightly off here. Is CONFIG_SPL_SERIAL_SUPPORT also enabled? That would turn this into a no-op as puts becomes a do { } while(0), if disabled. So I think we should just remove the ifdef here.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161126/51959093/attachment.sig>
#3 lore raw

PATCH 2/14 spl: mmc: Fix build without LIBCOMMON_SUPPORT

Quoted 36 lines · Tom Rini
Quoted 35 lines · Marek Vasut

If CONFIG_SPL_LIBCOMMON_SUPPORT is undefined, the following error will happen, so fix it.

In file included from common/spl/spl_mmc.c:11:0:
common/spl/spl_mmc.c: In function ?spl_mmc_load_image?:
include/spl.h:18:30: error: label at end of compound statement

 #define MMCSD_MODE_UNDEFINED 0
                              ^
common/spl/spl_mmc.c:335:7: note: in expansion of macro ?MMCSD_MODE_UNDEFINED?
  case MMCSD_MODE_UNDEFINED:
       ^
Cc Pantelis Antoniou <[email protected]>
Cc Tom Rini <[email protected]>
---
 common/spl/spl_mmc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
common/spl/spl_mmc.c +3 -1
index 0b681c2..43c1030 100644
@@ -343,10 +343,12 @@ static int spl_mmc_load_image(struct spl_image_info *spl_image,
 
 		break;
 	case MMCSD_MODE_UNDEFINED:
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 	default:
+		{
+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 		puts("spl: mmc: wrong boot mode\n");
 #endif
+		}
 	}
 
Quoted 6 lines

common/spl/spl_mmc.c static int spl_mmc_load_image(struct spl_image_info *spl_image, · line 354

Quoted 1 line
common/spl/spl_mmc.c +0 -0
 	return err;

I think we have something slightly off here. Is CONFIG_SPL_SERIAL_SUPPORT also enabled? That would turn this into a no-op as puts becomes a do { } while(0), if disabled. So I think we should just remove the ifdef here.

No, series does not fit into SPL on this platform.

Signature · 2 lines
Best regards,
Marek Vasut
#4 lore raw

PATCH 2/14 spl: mmc: Fix build without LIBCOMMON_SUPPORT

Quoted 37 lines · Marek Vasut
Quoted 36 lines · Tom Rini
Quoted 35 lines · Marek Vasut

If CONFIG_SPL_LIBCOMMON_SUPPORT is undefined, the following error will happen, so fix it.

In file included from common/spl/spl_mmc.c:11:0:
common/spl/spl_mmc.c: In function ?spl_mmc_load_image?:
include/spl.h:18:30: error: label at end of compound statement

 #define MMCSD_MODE_UNDEFINED 0
                              ^
common/spl/spl_mmc.c:335:7: note: in expansion of macro ?MMCSD_MODE_UNDEFINED?
  case MMCSD_MODE_UNDEFINED:
       ^
Cc Pantelis Antoniou <[email protected]>
Cc Tom Rini <[email protected]>
---
 common/spl/spl_mmc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
common/spl/spl_mmc.c +3 -1
index 0b681c2..43c1030 100644
@@ -343,10 +343,12 @@ static int spl_mmc_load_image(struct spl_image_info *spl_image,
 
 		break;
 	case MMCSD_MODE_UNDEFINED:
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 	default:
+		{
+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 		puts("spl: mmc: wrong boot mode\n");
 #endif
+		}
 	}
 
Quoted 8 lines
Quoted 6 lines

common/spl/spl_mmc.c static int spl_mmc_load_image(struct spl_image_info *spl_image, · line 354

Quoted 1 line
common/spl/spl_mmc.c +0 -0
 	return err;

I think we have something slightly off here. Is CONFIG_SPL_SERIAL_SUPPORT also enabled? That would turn this into a no-op as puts becomes a do { } while(0), if disabled. So I think we should just remove the ifdef here.

No, series does not fit into SPL on this platform.

I don't know..but i remembered that i suggested about putting just "break;" end of bracket. how about?

Best Regards,
Jaehoon Chung

Quoted 1 line
#5 lore raw

PATCH 2/14 spl: mmc: Fix build without LIBCOMMON_SUPPORT

Quoted 45 lines · Jaehoon Chung
Quoted 44 lines · Marek Vasut
Quoted 42 lines · Tom Rini
Quoted 26 lines · Marek Vasut

If CONFIG_SPL_LIBCOMMON_SUPPORT is undefined, the following error will happen, so fix it.

In file included from common/spl/spl_mmc.c:11:0:
common/spl/spl_mmc.c: In function ?spl_mmc_load_image?:
include/spl.h:18:30: error: label at end of compound statement

 #define MMCSD_MODE_UNDEFINED 0
                              ^
common/spl/spl_mmc.c:335:7: note: in expansion of macro ?MMCSD_MODE_UNDEFINED?
  case MMCSD_MODE_UNDEFINED:
       ^
Cc Pantelis Antoniou <[email protected]>
Cc Tom Rini <[email protected]>
---
 common/spl/spl_mmc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
common/spl/spl_mmc.c +0 -0
index 0b681c2..43c1030 100644
@@ -343,10 +343,12 @@ static int spl_mmc_load_image(struct spl_image_info *spl_image,
 
 		break;
 	case MMCSD_MODE_UNDEFINED:

common/spl/spl_mmc.c static int spl_mmc_load_image(struct spl_image_info *spl_image, · line 354

Quoted 10 lines
common/spl/spl_mmc.c +3 -1
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 	default:
+		{
+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 		puts("spl: mmc: wrong boot mode\n");
 #endif
+		}
 	}
 
 	return err;

I think we have something slightly off here. Is CONFIG_SPL_SERIAL_SUPPORT also enabled? That would turn this into a no-op as puts becomes a do { } while(0), if disabled. So I think we should just remove the ifdef here.

Quoted 4 lines
Quoted 1 line

No, series does not fit into SPL on this platform.

I don't know..but i remembered that i suggested about putting just "break;" end of bracket. how about?

Sure, that works too.

Signature · 2 lines
Best regards,
Marek Vasut
#6 lore raw

PATCH 2/14 spl: mmc: Fix build without LIBCOMMON_SUPPORT

Hi Marek,

Quoted 46 lines · Marek Vasut
Quoted 45 lines · Jaehoon Chung
Quoted 44 lines · Marek Vasut
Quoted 42 lines · Tom Rini
Quoted 26 lines · Marek Vasut

If CONFIG_SPL_LIBCOMMON_SUPPORT is undefined, the following error will happen, so fix it.

In file included from common/spl/spl_mmc.c:11:0:
common/spl/spl_mmc.c: In function ?spl_mmc_load_image?:
include/spl.h:18:30: error: label at end of compound statement

 #define MMCSD_MODE_UNDEFINED 0
                              ^
common/spl/spl_mmc.c:335:7: note: in expansion of macro ?MMCSD_MODE_UNDEFINED?
  case MMCSD_MODE_UNDEFINED:
       ^
Cc Pantelis Antoniou <[email protected]>
Cc Tom Rini <[email protected]>
---
 common/spl/spl_mmc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
common/spl/spl_mmc.c +0 -0
index 0b681c2..43c1030 100644
@@ -343,10 +343,12 @@ static int spl_mmc_load_image(struct spl_image_info *spl_image,
 
 		break;
 	case MMCSD_MODE_UNDEFINED:

common/spl/spl_mmc.c static int spl_mmc_load_image(struct spl_image_info *spl_image, · line 354

Quoted 10 lines
common/spl/spl_mmc.c +3 -1
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 	default:
+		{
+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 		puts("spl: mmc: wrong boot mode\n");
 #endif
+		}
 	}
 
 	return err;

I think we have something slightly off here. Is CONFIG_SPL_SERIAL_SUPPORT also enabled? That would turn this into a no-op as puts becomes a do { } while(0), if disabled. So I think we should just remove the ifdef here.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

No, series does not fit into SPL on this platform.

I don't know..but i remembered that i suggested about putting just "break;" end of bracket. how about?

Sure, that works too.

I didn't see this patch in your next version, right?

Best Regards,
Jaehoon Chung

Quoted 1 line
#7 lore raw

PATCH 2/14 spl: mmc: Fix build without LIBCOMMON_SUPPORT

Quoted 49 lines · Jaehoon Chung

Hi Marek,

Quoted 46 lines · Marek Vasut
Quoted 45 lines · Jaehoon Chung
Quoted 44 lines · Marek Vasut
Quoted 42 lines · Tom Rini
Quoted 26 lines · Marek Vasut

If CONFIG_SPL_LIBCOMMON_SUPPORT is undefined, the following error will happen, so fix it.

In file included from common/spl/spl_mmc.c:11:0:
common/spl/spl_mmc.c: In function ?spl_mmc_load_image?:
include/spl.h:18:30: error: label at end of compound statement

 #define MMCSD_MODE_UNDEFINED 0
                              ^
common/spl/spl_mmc.c:335:7: note: in expansion of macro ?MMCSD_MODE_UNDEFINED?
  case MMCSD_MODE_UNDEFINED:
       ^
Cc Pantelis Antoniou <[email protected]>
Cc Tom Rini <[email protected]>
---
 common/spl/spl_mmc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
common/spl/spl_mmc.c +0 -0
index 0b681c2..43c1030 100644
@@ -343,10 +343,12 @@ static int spl_mmc_load_image(struct spl_image_info *spl_image,
 
 		break;
 	case MMCSD_MODE_UNDEFINED:

common/spl/spl_mmc.c static int spl_mmc_load_image(struct spl_image_info *spl_image, · line 354

Quoted 10 lines
common/spl/spl_mmc.c +3 -1
-#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 	default:
+		{
+#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 		puts("spl: mmc: wrong boot mode\n");
 #endif
+		}
 	}
 
 	return err;

I think we have something slightly off here. Is CONFIG_SPL_SERIAL_SUPPORT also enabled? That would turn this into a no-op as puts becomes a do { } while(0), if disabled. So I think we should just remove the ifdef here.

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

No, series does not fit into SPL on this platform.

I don't know..but i remembered that i suggested about putting just "break;" end of bracket. how about?

Sure, that works too.

I didn't see this patch in your next version, right?

It doesn't seem to be necessary anymore.

Signature · 2 lines
Best regards,
Marek Vasut
#8 lore raw

PATCH 3/14 mmc: Fix warning if debug() is not used

In reply to Marek Vasut

If debug() is not used, then the whole content of debug(...) will be removed by the preprocessor, which will result in the following warning. This patch adds __maybe_unused annotation to fix this.

drivers/mmc/mmc.c: In function ?mmc_init?:
drivers/mmc/mmc.c:1685:11: warning: variable ?start? set but not used [-Wunused-but-set-variable]
unsigned start;

Cc Pantelis Antoniou <[email protected]>
Cc Tom Rini <[email protected]>
---
 drivers/mmc/mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mmc/mmc.c +1 -1
index d6b7e4f..6e25b67 100644
@@ -1708,7 +1708,7 @@ static int mmc_complete_init(struct mmc *mmc)
 int mmc_init(struct mmc *mmc)
 {
 	int err = 0;
-	unsigned start;
+	__maybe_unused unsigned start;
 #ifdef CONFIG_DM_MMC
 	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(mmc->dev);
 
Signature · 1 line
2.10.2
#9 lore raw

PATCH 3/14 mmc: Fix warning if debug() is not used

Quoted 11 lines · Marek Vasut

If debug() is not used, then the whole content of debug(...) will be removed by the preprocessor, which will result in the following warning. This patch adds __maybe_unused annotation to fix this.

drivers/mmc/mmc.c: In function ?mmc_init?:
drivers/mmc/mmc.c:1685:11: warning: variable ?start? set but not used [-Wunused-but-set-variable]
unsigned start;

Cc Pantelis Antoniou <[email protected]>
Cc Tom Rini <[email protected]>
Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161126/286c903f/attachment.sig>
#10 lore raw

PATCH 4/14 serial: 16550: Add getfcr accessor

In reply to Marek Vasut

Add function which allows fetching the default FCR register setting from platform data for DM , while retaining old behavior for non-DM by returning UART_FCRVAL.

---
V2: If CONFIG_DM_SERIAL and DEBUG_UART are enabled, the ns16550_getfcr()
can be invoked with NULL plat data . Check for this case and return the default UART_FCRVAL then.
V3: It turns out that if DEBUG_UART is defined, $port points directly to
hardware registers. Add additional ifdef to handle the case where debug uart is enabled with DM_SERIAL correctly.
V4: Use UART_FCRVAL in _debug_uart_init() directly

---
 drivers/serial/ns16550.c | 18 ++++++++++++++++--
 include/ns16550.h        |  1 +
 2 files changed, 17 insertions(+), 2 deletions(-)
drivers/serial/ns16550.c +16 -2
index 6e9b946..3c9f3b0 100644
@@ -121,6 +121,13 @@ static int ns16550_readb(NS16550_t port, int offset)
 	return serial_in_shift(addr + plat->reg_offset, plat->reg_shift);
 }
 
+static u32 ns16550_getfcr(NS16550_t port)
+{
+	struct ns16550_platdata *plat = port->plat;
+
+	return plat->fcr;
+}
+
 /* We can clean these up once everything is moved to driver model */
 #define serial_out(value, addr)	\
 	ns16550_writeb(com_port, \
@@ -128,6 +135,11 @@ static int ns16550_readb(NS16550_t port, int offset)
 #define serial_in(addr) \
 	ns16550_readb(com_port, \
 		(unsigned char *)addr - (unsigned char *)com_port)
+#else
+static u32 ns16550_getfcr(NS16550_t port)
+{
+	return UART_FCRVAL;
+}
 #endif
 
 int ns16550_calc_divisor(NS16550_t port, int clock, int baudrate)
@@ -171,7 +183,7 @@ void NS16550_init(NS16550_t com_port, int baud_divisor)
 	serial_out(0x7, &com_port->mdr1);	/* mode select reset TL16C750*/
 #endif
 	serial_out(UART_MCRVAL, &com_port->mcr);
-	serial_out(UART_FCRVAL, &com_port->fcr);
+	serial_out(ns16550_getfcr(com_port), &com_port->fcr);
 	if (baud_divisor != -1)
 		NS16550_setbrg(com_port, baud_divisor);
 #if defined(CONFIG_OMAP) || \
@@ -192,7 +204,7 @@ void NS16550_reinit(NS16550_t com_port, int baud_divisor)
 	serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier);
 	NS16550_setbrg(com_port, 0);
 	serial_out(UART_MCRVAL, &com_port->mcr);
-	serial_out(UART_FCRVAL, &com_port->fcr);
+	serial_out(ns16550_getfcr(com_port), &com_port->fcr);
 	NS16550_setbrg(com_port, baud_divisor);
 }
 #endif /* CONFIG_NS16550_MIN_FUNCTIONS */
@@ -420,6 +432,8 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 		return -EINVAL;
 	}
 
+	plat->fcr = UART_FCRVAL;
+
 	return 0;
 }
 #endif
include/ns16550.h +1 -0
index 1311f4c..45fd68b 100644
@@ -57,6 +57,7 @@ struct ns16550_platdata {
 	int reg_shift;
 	int clock;
 	int reg_offset;
+	u32 fcr;
 };
 
 struct udevice;
Signature · 1 line
2.10.2
#11 lore raw

PATCH 4/14 serial: 16550: Add getfcr accessor

Quoted 7 lines · Marek Vasut

Add function which allows fetching the default FCR register setting from platform data for DM , while retaining old behavior for non-DM by returning UART_FCRVAL.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161126/029ea24f/attachment.sig>
#12 lore raw

PATCH 4/14 serial: 16550: Add getfcr accessor

In reply to Marek Vasut
Quoted 15 lines · Marek Vasut

Add function which allows fetching the default FCR register setting from platform data for DM , while retaining old behavior for non-DM by returning UART_FCRVAL.

---
V2: If CONFIG_DM_SERIAL and DEBUG_UART are enabled, the ns16550_getfcr()
can be invoked with NULL plat data . Check for this case and return the default UART_FCRVAL then.
V3: It turns out that if DEBUG_UART is defined, $port points directly to
hardware registers. Add additional ifdef to handle the case where debug uart is enabled with DM_SERIAL correctly.
V4: Use UART_FCRVAL in _debug_uart_init() directly

Quoted 4 lines
---
 drivers/serial/ns16550.c | 18 ++++++++++++++++--
 include/ns16550.h        |  1 +
 2 files changed, 17 insertions(+), 2 deletions(-)
#13 lore raw

PATCH 5/14 serial: 16550: Add port type as driver data

In reply to Marek Vasut

Add driver data to each compatible string to identify the type of the port. Since all the ports in the driver are entirely compatible with 16550 for now, all are marked with PORT_NS16550. But, there are ports which have specific quirks, like the JZ4780 UART, which do not have any DT property to denote the quirks. Instead, Linux uses the compatible string to discern such ports and enable the necessary quirks.

---
 drivers/serial/ns16550.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
drivers/serial/ns16550.c +16 -10
index 3c9f3b0..3130a1d 100644
@@ -360,6 +360,12 @@ int ns16550_serial_probe(struct udevice *dev)
 	return 0;
 }
 
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+enum {
+	PORT_NS16550 = 0,
+};
+#endif
+
 #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 {
@@ -453,16 +459,16 @@ const struct dm_serial_ops ns16550_serial_ops = {
  * compatible string to your dts.
  */
 static const struct udevice_id ns16550_serial_ids[] = {
-	{ .compatible = "ns16550" },
-	{ .compatible = "ns16550a" },
-	{ .compatible = "nvidia,tegra20-uart" },
-	{ .compatible = "snps,dw-apb-uart" },
-	{ .compatible = "ti,omap2-uart" },
-	{ .compatible = "ti,omap3-uart" },
-	{ .compatible = "ti,omap4-uart" },
-	{ .compatible = "ti,am3352-uart" },
-	{ .compatible = "ti,am4372-uart" },
-	{ .compatible = "ti,dra742-uart" },
+	{ .compatible = "ns16550",		.data = PORT_NS16550 },
+	{ .compatible = "ns16550a",		.data = PORT_NS16550 },
+	{ .compatible = "nvidia,tegra20-uart",	.data = PORT_NS16550 },
+	{ .compatible = "snps,dw-apb-uart",	.data = PORT_NS16550 },
+	{ .compatible = "ti,omap2-uart",	.data = PORT_NS16550 },
+	{ .compatible = "ti,omap3-uart",	.data = PORT_NS16550 },
+	{ .compatible = "ti,omap4-uart",	.data = PORT_NS16550 },
+	{ .compatible = "ti,am3352-uart",	.data = PORT_NS16550 },
+	{ .compatible = "ti,am4372-uart",	.data = PORT_NS16550 },
+	{ .compatible = "ti,dra742-uart",	.data = PORT_NS16550 },
 	{}
 };
 #endif
Signature · 1 line
2.10.2
#14 lore raw

PATCH 5/14 serial: 16550: Add port type as driver data

Hi Marex,

Quoted 46 lines · Marek Vasut

Add driver data to each compatible string to identify the type of the port. Since all the ports in the driver are entirely compatible with 16550 for now, all are marked with PORT_NS16550. But, there are ports which have specific quirks, like the JZ4780 UART, which do not have any DT property to denote the quirks. Instead, Linux uses the compatible string to discern such ports and enable the necessary quirks.

---
 drivers/serial/ns16550.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
drivers/serial/ns16550.c +6 -10
index 3c9f3b0..3130a1d 100644
@@ -360,6 +360,12 @@ int ns16550_serial_probe(struct udevice *dev)
        return 0;
 }
 
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+enum {
+       PORT_NS16550 = 0,
+};
+#endif
+
 #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 {
@@ -453,16 +459,16 @@ const struct dm_serial_ops ns16550_serial_ops = {
  * compatible string to your dts.
  */
 static const struct udevice_id ns16550_serial_ids[] = {
-       { .compatible = "ns16550" },
-       { .compatible = "ns16550a" },
-       { .compatible = "nvidia,tegra20-uart" },
-       { .compatible = "snps,dw-apb-uart" },
-       { .compatible = "ti,omap2-uart" },
-       { .compatible = "ti,omap3-uart" },
-       { .compatible = "ti,omap4-uart" },
-       { .compatible = "ti,am3352-uart" },
-       { .compatible = "ti,am4372-uart" },
-       { .compatible = "ti,dra742-uart" },

drivers/serial/ns16550.c const struct dm_serial_ops ns16550_serial_ops = { · line 471

Quoted 10 lines
drivers/serial/ns16550.c +10 -0
+       { .compatible = "ns16550",              .data = PORT_NS16550 },
+       { .compatible = "ns16550a",             .data = PORT_NS16550 },
+       { .compatible = "nvidia,tegra20-uart",  .data = PORT_NS16550 },
+       { .compatible = "snps,dw-apb-uart",     .data = PORT_NS16550 },
+       { .compatible = "ti,omap2-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,omap3-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,omap4-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,am3352-uart",       .data = PORT_NS16550 },
+       { .compatible = "ti,am4372-uart",       .data = PORT_NS16550 },
+       { .compatible = "ti,dra742-uart",       .data = PORT_NS16550 },

But can we have 0 as the default so we don't need these values?

Regards,
Simon

#15 lore raw

PATCH 5/14 serial: 16550: Add port type as driver data

Quoted 53 lines · Simon Glass

Hi Marex,

Quoted 50 lines · Marek Vasut

Add driver data to each compatible string to identify the type of the port. Since all the ports in the driver are entirely compatible with 16550 for now, all are marked with PORT_NS16550. But, there are ports which have specific quirks, like the JZ4780 UART, which do not have any DT property to denote the quirks. Instead, Linux uses the compatible string to discern such ports and enable the necessary quirks.

---
 drivers/serial/ns16550.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
drivers/serial/ns16550.c +10 -10
index 3c9f3b0..3130a1d 100644
@@ -360,6 +360,12 @@ int ns16550_serial_probe(struct udevice *dev)
        return 0;
 }
 
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+enum {
+       PORT_NS16550 = 0,
+};
+#endif
+
 #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 {
@@ -453,16 +459,16 @@ const struct dm_serial_ops ns16550_serial_ops = {
  * compatible string to your dts.
  */
 static const struct udevice_id ns16550_serial_ids[] = {
-       { .compatible = "ns16550" },
-       { .compatible = "ns16550a" },
-       { .compatible = "nvidia,tegra20-uart" },
-       { .compatible = "snps,dw-apb-uart" },
-       { .compatible = "ti,omap2-uart" },
-       { .compatible = "ti,omap3-uart" },
-       { .compatible = "ti,omap4-uart" },
-       { .compatible = "ti,am3352-uart" },
-       { .compatible = "ti,am4372-uart" },
-       { .compatible = "ti,dra742-uart" },
+       { .compatible = "ns16550",              .data = PORT_NS16550 },
+       { .compatible = "ns16550a",             .data = PORT_NS16550 },
+       { .compatible = "nvidia,tegra20-uart",  .data = PORT_NS16550 },
+       { .compatible = "snps,dw-apb-uart",     .data = PORT_NS16550 },
Quoted 8 lines

drivers/serial/ns16550.c const struct dm_serial_ops ns16550_serial_ops = { · line 471

Quoted 6 lines
drivers/serial/ns16550.c +6 -0
+       { .compatible = "ti,omap2-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,omap3-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,omap4-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,am3352-uart",       .data = PORT_NS16550 },
+       { .compatible = "ti,am4372-uart",       .data = PORT_NS16550 },
+       { .compatible = "ti,dra742-uart",       .data = PORT_NS16550 },

But can we have 0 as the default so we don't need these values?

PORT_NS16550 is zero anyway, it's just explicitly spelled here.

Signature · 2 lines
Best regards,
Marek Vasut
#16 lore raw

PATCH 5/14 serial: 16550: Add port type as driver data

Hi Marek,

Quoted 63 lines · Marek Vasut
Quoted 53 lines · Simon Glass

Hi Marex,

Quoted 50 lines · Marek Vasut

Add driver data to each compatible string to identify the type of the port. Since all the ports in the driver are entirely compatible with 16550 for now, all are marked with PORT_NS16550. But, there are ports which have specific quirks, like the JZ4780 UART, which do not have any DT property to denote the quirks. Instead, Linux uses the compatible string to discern such ports and enable the necessary quirks.

---
 drivers/serial/ns16550.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
drivers/serial/ns16550.c +10 -10
index 3c9f3b0..3130a1d 100644
@@ -360,6 +360,12 @@ int ns16550_serial_probe(struct udevice *dev)
        return 0;
 }
 
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+enum {
+       PORT_NS16550 = 0,
+};
+#endif
+
 #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 {
@@ -453,16 +459,16 @@ const struct dm_serial_ops ns16550_serial_ops = {
  * compatible string to your dts.
  */
 static const struct udevice_id ns16550_serial_ids[] = {
-       { .compatible = "ns16550" },
-       { .compatible = "ns16550a" },
-       { .compatible = "nvidia,tegra20-uart" },
-       { .compatible = "snps,dw-apb-uart" },
-       { .compatible = "ti,omap2-uart" },
-       { .compatible = "ti,omap3-uart" },
-       { .compatible = "ti,omap4-uart" },
-       { .compatible = "ti,am3352-uart" },
-       { .compatible = "ti,am4372-uart" },
-       { .compatible = "ti,dra742-uart" },
+       { .compatible = "ns16550",              .data = PORT_NS16550 },
+       { .compatible = "ns16550a",             .data = PORT_NS16550 },
+       { .compatible = "nvidia,tegra20-uart",  .data = PORT_NS16550 },
+       { .compatible = "snps,dw-apb-uart",     .data = PORT_NS16550 },
Quoted 8 lines

drivers/serial/ns16550.c const struct dm_serial_ops ns16550_serial_ops = { · line 471

Quoted 6 lines
drivers/serial/ns16550.c +6 -0
+       { .compatible = "ti,omap2-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,omap3-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,omap4-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,am3352-uart",       .data = PORT_NS16550 },
+       { .compatible = "ti,am4372-uart",       .data = PORT_NS16550 },
+       { .compatible = "ti,dra742-uart",       .data = PORT_NS16550 },

But can we have 0 as the default so we don't need these values?

Quoted 1 line

PORT_NS16550 is zero anyway, it's just explicitly spelled here.

One way might be to use PORT_DEFAULT, set it to 0 and omit it here. It does seem odd to have PORT_NS16550 in the ns16550 driver.

Regards,
Simon

#17 lore raw

PATCH 5/14 serial: 16550: Add port type as driver data

Quoted 66 lines · Simon Glass

Hi Marek,

Quoted 63 lines · Marek Vasut
Quoted 61 lines · Simon Glass

Hi Marex,

Quoted 46 lines · Marek Vasut

Add driver data to each compatible string to identify the type of the port. Since all the ports in the driver are entirely compatible with 16550 for now, all are marked with PORT_NS16550. But, there are ports which have specific quirks, like the JZ4780 UART, which do not have any DT property to denote the quirks. Instead, Linux uses the compatible string to discern such ports and enable the necessary quirks.

---
 drivers/serial/ns16550.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
drivers/serial/ns16550.c +6 -10
index 3c9f3b0..3130a1d 100644
@@ -360,6 +360,12 @@ int ns16550_serial_probe(struct udevice *dev)
        return 0;
 }
 
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+enum {
+       PORT_NS16550 = 0,
+};
+#endif
+
 #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 {
@@ -453,16 +459,16 @@ const struct dm_serial_ops ns16550_serial_ops = {
  * compatible string to your dts.
  */
 static const struct udevice_id ns16550_serial_ids[] = {
-       { .compatible = "ns16550" },
-       { .compatible = "ns16550a" },
-       { .compatible = "nvidia,tegra20-uart" },
-       { .compatible = "snps,dw-apb-uart" },
-       { .compatible = "ti,omap2-uart" },
-       { .compatible = "ti,omap3-uart" },
-       { .compatible = "ti,omap4-uart" },
-       { .compatible = "ti,am3352-uart" },
-       { .compatible = "ti,am4372-uart" },
-       { .compatible = "ti,dra742-uart" },

drivers/serial/ns16550.c const struct dm_serial_ops ns16550_serial_ops = { · line 471

Quoted 10 lines
drivers/serial/ns16550.c +10 -0
+       { .compatible = "ns16550",              .data = PORT_NS16550 },
+       { .compatible = "ns16550a",             .data = PORT_NS16550 },
+       { .compatible = "nvidia,tegra20-uart",  .data = PORT_NS16550 },
+       { .compatible = "snps,dw-apb-uart",     .data = PORT_NS16550 },
+       { .compatible = "ti,omap2-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,omap3-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,omap4-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,am3352-uart",       .data = PORT_NS16550 },
+       { .compatible = "ti,am4372-uart",       .data = PORT_NS16550 },
+       { .compatible = "ti,dra742-uart",       .data = PORT_NS16550 },

But can we have 0 as the default so we don't need these values?

Quoted 4 lines
Quoted 1 line

PORT_NS16550 is zero anyway, it's just explicitly spelled here.

One way might be to use PORT_DEFAULT, set it to 0 and omit it here. It does seem odd to have PORT_NS16550 in the ns16550 driver.

I'd rather be explicit in case we grow this list, it also doesn't hurt anything since it is implicitly set to zero. Or is this something more than a matter of preference here ?

Signature · 2 lines
Best regards,
Marek Vasut
#18 lore raw

PATCH 5/14 serial: 16550: Add port type as driver data

Hi Marek,

Quoted 67 lines · Marek Vasut
Quoted 66 lines · Simon Glass

Hi Marek,

Quoted 63 lines · Marek Vasut
Quoted 61 lines · Simon Glass

Hi Marex,

Quoted 46 lines · Marek Vasut

Add driver data to each compatible string to identify the type of the port. Since all the ports in the driver are entirely compatible with 16550 for now, all are marked with PORT_NS16550. But, there are ports which have specific quirks, like the JZ4780 UART, which do not have any DT property to denote the quirks. Instead, Linux uses the compatible string to discern such ports and enable the necessary quirks.

---
 drivers/serial/ns16550.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
drivers/serial/ns16550.c +6 -10
index 3c9f3b0..3130a1d 100644
@@ -360,6 +360,12 @@ int ns16550_serial_probe(struct udevice *dev)
        return 0;
 }
 
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+enum {
+       PORT_NS16550 = 0,
+};
+#endif
+
 #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 {
@@ -453,16 +459,16 @@ const struct dm_serial_ops ns16550_serial_ops = {
  * compatible string to your dts.
  */
 static const struct udevice_id ns16550_serial_ids[] = {
-       { .compatible = "ns16550" },
-       { .compatible = "ns16550a" },
-       { .compatible = "nvidia,tegra20-uart" },
-       { .compatible = "snps,dw-apb-uart" },
-       { .compatible = "ti,omap2-uart" },
-       { .compatible = "ti,omap3-uart" },
-       { .compatible = "ti,omap4-uart" },
-       { .compatible = "ti,am3352-uart" },
-       { .compatible = "ti,am4372-uart" },
-       { .compatible = "ti,dra742-uart" },

drivers/serial/ns16550.c const struct dm_serial_ops ns16550_serial_ops = { · line 471

Quoted 10 lines
drivers/serial/ns16550.c +10 -0
+       { .compatible = "ns16550",              .data = PORT_NS16550 },
+       { .compatible = "ns16550a",             .data = PORT_NS16550 },
+       { .compatible = "nvidia,tegra20-uart",  .data = PORT_NS16550 },
+       { .compatible = "snps,dw-apb-uart",     .data = PORT_NS16550 },
+       { .compatible = "ti,omap2-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,omap3-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,omap4-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,am3352-uart",       .data = PORT_NS16550 },
+       { .compatible = "ti,am4372-uart",       .data = PORT_NS16550 },
+       { .compatible = "ti,dra742-uart",       .data = PORT_NS16550 },

But can we have 0 as the default so we don't need these values?

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

PORT_NS16550 is zero anyway, it's just explicitly spelled here.

One way might be to use PORT_DEFAULT, set it to 0 and omit it here. It does seem odd to have PORT_NS16550 in the ns16550 driver.

I'd rather be explicit in case we grow this list, it also doesn't hurt anything since it is implicitly set to zero. Or is this something more than a matter of preference here ?

Well, at least rename the PORT value to DEFAULT or something like that if you want to be explicit.

Regards,
Simon

#19 lore raw

PATCH 5/14 serial: 16550: Add port type as driver data

Quoted 75 lines · Simon Glass

Hi Marek,

Quoted 72 lines · Marek Vasut
Quoted 70 lines · Simon Glass

Hi Marek,

Quoted 63 lines · Marek Vasut
Quoted 53 lines · Simon Glass

Hi Marex,

Quoted 50 lines · Marek Vasut

Add driver data to each compatible string to identify the type of the port. Since all the ports in the driver are entirely compatible with 16550 for now, all are marked with PORT_NS16550. But, there are ports which have specific quirks, like the JZ4780 UART, which do not have any DT property to denote the quirks. Instead, Linux uses the compatible string to discern such ports and enable the necessary quirks.

---
 drivers/serial/ns16550.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
drivers/serial/ns16550.c +10 -10
index 3c9f3b0..3130a1d 100644
@@ -360,6 +360,12 @@ int ns16550_serial_probe(struct udevice *dev)
        return 0;
 }
 
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+enum {
+       PORT_NS16550 = 0,
+};
+#endif
+
 #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 {
@@ -453,16 +459,16 @@ const struct dm_serial_ops ns16550_serial_ops = {
  * compatible string to your dts.
  */
 static const struct udevice_id ns16550_serial_ids[] = {
-       { .compatible = "ns16550" },
-       { .compatible = "ns16550a" },
-       { .compatible = "nvidia,tegra20-uart" },
-       { .compatible = "snps,dw-apb-uart" },
-       { .compatible = "ti,omap2-uart" },
-       { .compatible = "ti,omap3-uart" },
-       { .compatible = "ti,omap4-uart" },
-       { .compatible = "ti,am3352-uart" },
-       { .compatible = "ti,am4372-uart" },
-       { .compatible = "ti,dra742-uart" },
+       { .compatible = "ns16550",              .data = PORT_NS16550 },
+       { .compatible = "ns16550a",             .data = PORT_NS16550 },
+       { .compatible = "nvidia,tegra20-uart",  .data = PORT_NS16550 },
+       { .compatible = "snps,dw-apb-uart",     .data = PORT_NS16550 },
Quoted 8 lines

drivers/serial/ns16550.c const struct dm_serial_ops ns16550_serial_ops = { · line 471

Quoted 6 lines
drivers/serial/ns16550.c +6 -0
+       { .compatible = "ti,omap2-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,omap3-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,omap4-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,am3352-uart",       .data = PORT_NS16550 },
+       { .compatible = "ti,am4372-uart",       .data = PORT_NS16550 },
+       { .compatible = "ti,dra742-uart",       .data = PORT_NS16550 },

But can we have 0 as the default so we don't need these values?

Quoted 1 line

PORT_NS16550 is zero anyway, it's just explicitly spelled here.

One way might be to use PORT_DEFAULT, set it to 0 and omit it here. It does seem odd to have PORT_NS16550 in the ns16550 driver.

Quoted 6 lines
Quoted 3 lines

I'd rather be explicit in case we grow this list, it also doesn't hurt anything since it is implicitly set to zero. Or is this something more than a matter of preference here ?

Well, at least rename the PORT value to DEFAULT or something like that if you want to be explicit.

Why DEFAULT ? It's NS16550 compatible port and the others are not entirely NS16550 compatible, so I think NS16550 is exactly how it should be named. There never was any DEFAULT label on these chips, but the original 16550 UART chip had 16550 written on it.

Signature · 2 lines
Best regards,
Marek Vasut
#20 lore raw

PATCH 5/14 serial: 16550: Add port type as driver data

Hi Marek,

Quoted 80 lines · Marek Vasut
Quoted 79 lines · Simon Glass

Hi Marek,

Quoted 75 lines · Marek Vasut
Quoted 66 lines · Simon Glass

Hi Marek,

Quoted 63 lines · Marek Vasut
Quoted 61 lines · Simon Glass

Hi Marex,

Quoted 46 lines · Marek Vasut

Add driver data to each compatible string to identify the type of the port. Since all the ports in the driver are entirely compatible with 16550 for now, all are marked with PORT_NS16550. But, there are ports which have specific quirks, like the JZ4780 UART, which do not have any DT property to denote the quirks. Instead, Linux uses the compatible string to discern such ports and enable the necessary quirks.

---
 drivers/serial/ns16550.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
drivers/serial/ns16550.c +6 -10
index 3c9f3b0..3130a1d 100644
@@ -360,6 +360,12 @@ int ns16550_serial_probe(struct udevice *dev)
        return 0;
 }
 
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+enum {
+       PORT_NS16550 = 0,
+};
+#endif
+
 #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 {
@@ -453,16 +459,16 @@ const struct dm_serial_ops ns16550_serial_ops = {
  * compatible string to your dts.
  */
 static const struct udevice_id ns16550_serial_ids[] = {
-       { .compatible = "ns16550" },
-       { .compatible = "ns16550a" },
-       { .compatible = "nvidia,tegra20-uart" },
-       { .compatible = "snps,dw-apb-uart" },
-       { .compatible = "ti,omap2-uart" },
-       { .compatible = "ti,omap3-uart" },
-       { .compatible = "ti,omap4-uart" },
-       { .compatible = "ti,am3352-uart" },
-       { .compatible = "ti,am4372-uart" },
-       { .compatible = "ti,dra742-uart" },

drivers/serial/ns16550.c const struct dm_serial_ops ns16550_serial_ops = { · line 471

Quoted 10 lines
drivers/serial/ns16550.c +10 -0
+       { .compatible = "ns16550",              .data = PORT_NS16550 },
+       { .compatible = "ns16550a",             .data = PORT_NS16550 },
+       { .compatible = "nvidia,tegra20-uart",  .data = PORT_NS16550 },
+       { .compatible = "snps,dw-apb-uart",     .data = PORT_NS16550 },
+       { .compatible = "ti,omap2-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,omap3-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,omap4-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,am3352-uart",       .data = PORT_NS16550 },
+       { .compatible = "ti,am4372-uart",       .data = PORT_NS16550 },
+       { .compatible = "ti,dra742-uart",       .data = PORT_NS16550 },

But can we have 0 as the default so we don't need these values?

Quoted 4 lines
Quoted 1 line

PORT_NS16550 is zero anyway, it's just explicitly spelled here.

One way might be to use PORT_DEFAULT, set it to 0 and omit it here. It does seem odd to have PORT_NS16550 in the ns16550 driver.

I'd rather be explicit in case we grow this list, it also doesn't hurt anything since it is implicitly set to zero. Or is this something more than a matter of preference here ?

Quoted 7 lines
Quoted 2 lines

Well, at least rename the PORT value to DEFAULT or something like that if you want to be explicit.

Why DEFAULT ? It's NS16550 compatible port and the others are not entirely NS16550 compatible, so I think NS16550 is exactly how it should be named. There never was any DEFAULT label on these chips, but the original 16550 UART chip had 16550 written on it.

Because the driver is called ns16550. So one can assume that this is the base case...

Regards,
Simon

#21 lore raw

PATCH 5/14 serial: 16550: Add port type as driver data

Quoted 86 lines · Simon Glass

Hi Marek,

Quoted 83 lines · Marek Vasut
Quoted 81 lines · Simon Glass

Hi Marek,

Quoted 67 lines · Marek Vasut
Quoted 66 lines · Simon Glass

Hi Marek,

Quoted 63 lines · Marek Vasut
Quoted 61 lines · Simon Glass

Hi Marex,

Quoted 46 lines · Marek Vasut

Add driver data to each compatible string to identify the type of the port. Since all the ports in the driver are entirely compatible with 16550 for now, all are marked with PORT_NS16550. But, there are ports which have specific quirks, like the JZ4780 UART, which do not have any DT property to denote the quirks. Instead, Linux uses the compatible string to discern such ports and enable the necessary quirks.

---
 drivers/serial/ns16550.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
drivers/serial/ns16550.c +6 -10
index 3c9f3b0..3130a1d 100644
@@ -360,6 +360,12 @@ int ns16550_serial_probe(struct udevice *dev)
        return 0;
 }
 
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+enum {
+       PORT_NS16550 = 0,
+};
+#endif
+
 #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 {
@@ -453,16 +459,16 @@ const struct dm_serial_ops ns16550_serial_ops = {
  * compatible string to your dts.
  */
 static const struct udevice_id ns16550_serial_ids[] = {
-       { .compatible = "ns16550" },
-       { .compatible = "ns16550a" },
-       { .compatible = "nvidia,tegra20-uart" },
-       { .compatible = "snps,dw-apb-uart" },
-       { .compatible = "ti,omap2-uart" },
-       { .compatible = "ti,omap3-uart" },
-       { .compatible = "ti,omap4-uart" },
-       { .compatible = "ti,am3352-uart" },
-       { .compatible = "ti,am4372-uart" },
-       { .compatible = "ti,dra742-uart" },

drivers/serial/ns16550.c const struct dm_serial_ops ns16550_serial_ops = { · line 471

Quoted 10 lines
drivers/serial/ns16550.c +10 -0
+       { .compatible = "ns16550",              .data = PORT_NS16550 },
+       { .compatible = "ns16550a",             .data = PORT_NS16550 },
+       { .compatible = "nvidia,tegra20-uart",  .data = PORT_NS16550 },
+       { .compatible = "snps,dw-apb-uart",     .data = PORT_NS16550 },
+       { .compatible = "ti,omap2-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,omap3-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,omap4-uart",        .data = PORT_NS16550 },
+       { .compatible = "ti,am3352-uart",       .data = PORT_NS16550 },
+       { .compatible = "ti,am4372-uart",       .data = PORT_NS16550 },
+       { .compatible = "ti,dra742-uart",       .data = PORT_NS16550 },

But can we have 0 as the default so we don't need these values?

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

PORT_NS16550 is zero anyway, it's just explicitly spelled here.

One way might be to use PORT_DEFAULT, set it to 0 and omit it here. It does seem odd to have PORT_NS16550 in the ns16550 driver.

I'd rather be explicit in case we grow this list, it also doesn't hurt anything since it is implicitly set to zero. Or is this something more than a matter of preference here ?

Well, at least rename the PORT value to DEFAULT or something like that if you want to be explicit.

Quoted 7 lines
Quoted 4 lines

Why DEFAULT ? It's NS16550 compatible port and the others are not entirely NS16550 compatible, so I think NS16550 is exactly how it should be named. There never was any DEFAULT label on these chips, but the original 16550 UART chip had 16550 written on it.

Because the driver is called ns16550. So one can assume that this is the base case...

Until someone refactors it or splits it. I wouldn't go with that assumption. I really dislike having some default here.

Signature · 2 lines
Best regards,
Marek Vasut
#22 lore raw

PATCH 6/14 serial: 16550: Add Ingenic JZ4780 support

In reply to Marek Vasut

Add compatibility string for the Ingenic JZ4780 SoC, the necessary UART enable bit into FCR and register shift. Neither are encoded in the DTS coming from Linux, so we need to support it this way.

Cc Tom Rini <[email protected]>
Cc Simon Glass <[email protected]>
Cc Daniel Schwierzeck <[email protected]>
Cc Paul Burton <[email protected]>
---
 drivers/serial/ns16550.c | 7 +++++++
 include/ns16550.h        | 3 +++
 2 files changed, 10 insertions(+)
drivers/serial/ns16550.c +7 -0
index 3130a1d..d00161c 100644
@@ -363,6 +363,7 @@ int ns16550_serial_probe(struct udevice *dev)
 #if CONFIG_IS_ENABLED(OF_CONTROL)
 enum {
 	PORT_NS16550 = 0,
+	PORT_JZ4780,
 };
 #endif
 
@@ -370,6 +371,7 @@ enum {
 int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 {
 	struct ns16550_platdata *plat = dev->platdata;
+	const u32 port_type = dev_get_driver_data(dev);
 	fdt_addr_t addr;
 	struct clk clk;
 	int err;
@@ -439,6 +441,10 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 	}
 
 	plat->fcr = UART_FCRVAL;
+	if (port_type == PORT_JZ4780) {
+		plat->fcr |= UART_FCR_UME;
+		plat->reg_shift = 2;
+	}
 
 	return 0;
 }
@@ -461,6 +467,7 @@ const struct dm_serial_ops ns16550_serial_ops = {
 static const struct udevice_id ns16550_serial_ids[] = {
 	{ .compatible = "ns16550",		.data = PORT_NS16550 },
 	{ .compatible = "ns16550a",		.data = PORT_NS16550 },
+	{ .compatible = "ingenic,jz4780-uart",	.data = PORT_JZ4780  },
 	{ .compatible = "nvidia,tegra20-uart",	.data = PORT_NS16550 },
 	{ .compatible = "snps,dw-apb-uart",	.data = PORT_NS16550 },
 	{ .compatible = "ti,omap2-uart",	.data = PORT_NS16550 },
include/ns16550.h +3 -0
index 45fd68b..7c97036 100644
@@ -118,6 +118,9 @@ typedef struct NS16550 *NS16550_t;
 #define UART_FCR_RXSR		0x02 /* Receiver soft reset */
 #define UART_FCR_TXSR		0x04 /* Transmitter soft reset */
 
+/* Ingenic JZ47xx specific UART-enable bit. */
+#define UART_FCR_UME		0x10
+
 /*
  * These are the definitions for the Modem Control Register
  */
Signature · 1 line
2.10.2
#23 lore raw

PATCH 6/14 serial: 16550: Add Ingenic JZ4780 support

Hi Marex,

Quoted 35 lines · Marek Vasut

Add compatibility string for the Ingenic JZ4780 SoC, the necessary UART enable bit into FCR and register shift. Neither are encoded in the DTS coming from Linux, so we need to support it this way.

Cc Tom Rini <[email protected]>
Cc Simon Glass <[email protected]>
Cc Daniel Schwierzeck <[email protected]>
Cc Paul Burton <[email protected]>
---
 drivers/serial/ns16550.c | 7 +++++++
 include/ns16550.h        | 3 +++
 2 files changed, 10 insertions(+)
drivers/serial/ns16550.c +2 -0
index 3130a1d..d00161c 100644
@@ -363,6 +363,7 @@ int ns16550_serial_probe(struct udevice *dev)
 #if CONFIG_IS_ENABLED(OF_CONTROL)
 enum {
        PORT_NS16550 = 0,
+       PORT_JZ4780,
 };
 #endif
 
@@ -370,6 +371,7 @@ enum {
 int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 {
        struct ns16550_platdata *plat = dev->platdata;
+       const u32 port_type = dev_get_driver_data(dev);
        fdt_addr_t addr;
        struct clk clk;
        int err;

drivers/serial/ns16550.c int ns16550_serial_ofdata_to_platdata(struct udevice *dev) · line 446

Quoted 6 lines
drivers/serial/ns16550.c +3 -0
@@ -439,6 +441,10 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
        }
 
        plat->fcr = UART_FCRVAL;
+       if (port_type == PORT_JZ4780) {
+               plat->fcr |= UART_FCR_UME;
+               plat->reg_shift = 2;

This should be in the device tree, shouldn't it?

Quoted 27 lines
drivers/serial/ns16550.c +2 -0
+       }
 
        return 0;
 }
@@ -461,6 +467,7 @@ const struct dm_serial_ops ns16550_serial_ops = {
 static const struct udevice_id ns16550_serial_ids[] = {
        { .compatible = "ns16550",              .data = PORT_NS16550 },
        { .compatible = "ns16550a",             .data = PORT_NS16550 },
+       { .compatible = "ingenic,jz4780-uart",  .data = PORT_JZ4780  },
        { .compatible = "nvidia,tegra20-uart",  .data = PORT_NS16550 },
        { .compatible = "snps,dw-apb-uart",     .data = PORT_NS16550 },
        { .compatible = "ti,omap2-uart",        .data = PORT_NS16550 },
include/ns16550.h +3 -1
index 45fd68b..7c97036 100644
@@ -118,6 +118,9 @@ typedef struct NS16550 *NS16550_t;
 #define UART_FCR_RXSR          0x02 /* Receiver soft reset */
 #define UART_FCR_TXSR          0x04 /* Transmitter soft reset */
 
+/* Ingenic JZ47xx specific UART-enable bit. */
+#define UART_FCR_UME           0x10
+
 /*
  * These are the definitions for the Modem Control Register
  */
--
Quoted 2 lines

2.10.2

Regards,
Simon

#24 lore raw

PATCH 6/14 serial: 16550: Add Ingenic JZ4780 support

Quoted 40 lines · Simon Glass

Hi Marex,

Quoted 37 lines · Marek Vasut

Add compatibility string for the Ingenic JZ4780 SoC, the necessary UART enable bit into FCR and register shift. Neither are encoded in the DTS coming from Linux, so we need to support it this way.

Cc Tom Rini <[email protected]>
Cc Simon Glass <[email protected]>
Cc Daniel Schwierzeck <[email protected]>
Cc Paul Burton <[email protected]>
---
 drivers/serial/ns16550.c | 7 +++++++
 include/ns16550.h        | 3 +++
 2 files changed, 10 insertions(+)
drivers/serial/ns16550.c +2 -0
index 3130a1d..d00161c 100644
@@ -363,6 +363,7 @@ int ns16550_serial_probe(struct udevice *dev)
 #if CONFIG_IS_ENABLED(OF_CONTROL)
 enum {
        PORT_NS16550 = 0,
+       PORT_JZ4780,
 };
 #endif
 
@@ -370,6 +371,7 @@ enum {
 int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 {
        struct ns16550_platdata *plat = dev->platdata;
+       const u32 port_type = dev_get_driver_data(dev);
        fdt_addr_t addr;
        struct clk clk;
        int err;
@@ -439,6 +441,10 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
        }
 
Quoted 6 lines

drivers/serial/ns16550.c int ns16550_serial_ofdata_to_platdata(struct udevice *dev) · line 446

Quoted 4 lines
drivers/serial/ns16550.c +3 -0
        plat->fcr = UART_FCRVAL;
+       if (port_type == PORT_JZ4780) {
+               plat->fcr |= UART_FCR_UME;
+               plat->reg_shift = 2;

This should be in the device tree, shouldn't it?

Yeah, thanks.

Quoted 33 lines
Quoted 27 lines
drivers/serial/ns16550.c +2 -0
+       }
 
        return 0;
 }
@@ -461,6 +467,7 @@ const struct dm_serial_ops ns16550_serial_ops = {
 static const struct udevice_id ns16550_serial_ids[] = {
        { .compatible = "ns16550",              .data = PORT_NS16550 },
        { .compatible = "ns16550a",             .data = PORT_NS16550 },
+       { .compatible = "ingenic,jz4780-uart",  .data = PORT_JZ4780  },
        { .compatible = "nvidia,tegra20-uart",  .data = PORT_NS16550 },
        { .compatible = "snps,dw-apb-uart",     .data = PORT_NS16550 },
        { .compatible = "ti,omap2-uart",        .data = PORT_NS16550 },
include/ns16550.h +3 -1
index 45fd68b..7c97036 100644
@@ -118,6 +118,9 @@ typedef struct NS16550 *NS16550_t;
 #define UART_FCR_RXSR          0x02 /* Receiver soft reset */
 #define UART_FCR_TXSR          0x04 /* Transmitter soft reset */
 
+/* Ingenic JZ47xx specific UART-enable bit. */
+#define UART_FCR_UME           0x10
+
 /*
  * These are the definitions for the Modem Control Register
  */
--
Quoted 2 lines

2.10.2

Regards,
Simon

Signature · 2 lines
Best regards,
Marek Vasut
#25 lore raw

PATCH 7/14 mmc: Tinification of the mmc code

In reply to Marek Vasut

Add new configuration option CONFIG_MMC_TINY which strips away all memory allocation within the MMC code and code for handling multiple cards. This allows extremely space-constrained SPL code use the MMC framework.

---
V2: Switch the MMC_TINY option to Kconfig

---
 common/spl/spl_mmc.c     |  6 +++++-
 drivers/mmc/Kconfig      | 16 ++++++++++++++++
 drivers/mmc/mmc.c        | 31 ++++++++++++++++++++++++++++++-
 drivers/mmc/mmc_legacy.c | 32 ++++++++++++++++++++++++++++++++
 include/mmc.h            |  1 +
 5 files changed, 84 insertions(+), 2 deletions(-)
common/spl/spl_mmc.c +5 -1
index 43c1030..3cad998 100644
@@ -306,7 +306,11 @@ static int spl_mmc_load_image(struct spl_image_info *spl_image,
 			if (part == 7)
 				part = 0;
 
-			err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part);
+			if (CONFIG_IS_ENABLED(MMC_TINY))
+				err = mmc_switch_part(mmc, part);
+			else
+				err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part);
+
 			if (err) {
 #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
 				puts("spl: mmc partition switch failed\n");
drivers/mmc/Kconfig +16 -0
index 24f4b28..aca438b8 100644
@@ -26,6 +26,22 @@ config DM_MMC_OPS
 	  option will be removed as soon as all DM_MMC drivers use it, as it
 	  will the only supported behaviour.
 
+config SPL_MMC_TINY
+	bool "Tiny MMC framework in SPL"
+	default n
+	help
+	  Enable MMC framework tinification support. This option is useful if
+	  if your SPL is extremely size constrained. Heed the warning, enable
+	  this option if and only if you know exactly what you are doing, if
+	  you are reading this help text, you most likely have no idea :-)
+
+	  The MMC framework is reduced to bare minimum to be useful. No malloc
+	  support is needed for the MMC framework operation with this option
+	  enabled. The framework supports exactly one MMC device and exactly
+	  one MMC driver. The MMC driver can be adjusted to avoid any malloc
+	  operations too, which can remove the need for malloc support in SPL
+	  and thus further reduce footprint.
+
 config MSM_SDHCI
 	bool "Qualcomm SDHCI controller"
 	depends on DM_MMC && BLK && DM_MMC_OPS
drivers/mmc/mmc.c +30 -1
index 6e25b67..19c4225 100644
@@ -30,6 +30,29 @@ static const unsigned int sd_au_size[] = {
 	SZ_16M / 512,	(SZ_16M + SZ_8M) / 512,	SZ_32M / 512,	SZ_64M / 512,
 };
 
+#if CONFIG_IS_ENABLED(MMC_TINY)
+static struct mmc mmc_static;
+struct mmc *find_mmc_device(int dev_num)
+{
+	return &mmc_static;
+}
+
+void mmc_do_preinit(void)
+{
+	struct mmc *m = &mmc_static;
+#ifdef CONFIG_FSL_ESDHC_ADAPTER_IDENT
+	mmc_set_preinit(m, 1);
+#endif
+	if (m->preinit)
+		mmc_start_init(m);
+}
+
+struct blk_desc *mmc_get_blk_desc(struct mmc *mmc)
+{
+	return &mmc->block_dev;
+}
+#endif
+
 #ifndef CONFIG_DM_MMC_OPS
 __weak int board_mmc_getwp(struct mmc *mmc)
 {
@@ -259,7 +282,11 @@ ulong mmc_bread(struct blk_desc *block_dev, lbaint_t start, lbaint_t blkcnt,
 	if (!mmc)
 		return 0;
 
-	err = blk_dselect_hwpart(block_dev, block_dev->hwpart);
+	if (CONFIG_IS_ENABLED(MMC_TINY))
+		err = mmc_switch_part(mmc, block_dev->hwpart);
+	else
+		err = blk_dselect_hwpart(block_dev, block_dev->hwpart);
+
 	if (err < 0)
 		return 0;
 
@@ -1804,8 +1831,10 @@ int mmc_initialize(bd_t *bis)
 	initialized = 1;
 
 #ifndef CONFIG_BLK
+#if !CONFIG_IS_ENABLED(MMC_TINY)
 	mmc_list_init();
 #endif
+#endif
 	ret = mmc_probe(bis);
 	if (ret)
 		return ret;
drivers/mmc/mmc_legacy.c +32 -0
index 25361d1..bdf9d98 100644
@@ -13,6 +13,7 @@
 static struct list_head mmc_devices;
 static int cur_dev_num = -1;
 
+#if !CONFIG_IS_ENABLED(MMC_TINY)
 struct mmc *find_mmc_device(int dev_num)
 {
 	struct mmc *m;
@@ -62,6 +63,7 @@ void mmc_do_preinit(void)
 			mmc_start_init(m);
 	}
 }
+#endif
 
 void mmc_list_init(void)
 {
@@ -109,6 +111,35 @@ void print_mmc_devices(char separator)
 void print_mmc_devices(char separator) { }
 #endif
 
+#if CONFIG_IS_ENABLED(MMC_TINY)
+static struct mmc mmc_static = {
+	.dsr_imp		= 0,
+	.dsr			= 0xffffffff,
+	.block_dev = {
+		.if_type	= IF_TYPE_MMC,
+		.removable	= 1,
+		.devnum		= 0,
+		.block_read	= mmc_bread,
+		.block_write	= mmc_bwrite,
+		.block_erase	= mmc_berase,
+		.part_type	= 0,
+	},
+};
+
+struct mmc *mmc_create(const struct mmc_config *cfg, void *priv)
+{
+	struct mmc *mmc = &mmc_static;
+
+	mmc->cfg = cfg;
+	mmc->priv = priv;
+
+	return mmc;
+}
+
+void mmc_destroy(struct mmc *mmc)
+{
+}
+#else
 struct mmc *mmc_create(const struct mmc_config *cfg, void *priv)
 {
 	struct blk_desc *bdesc;
@@ -157,6 +188,7 @@ void mmc_destroy(struct mmc *mmc)
 	/* only freeing memory for now */
 	free(mmc);
 }
+#endif
 
 static int mmc_select_hwpartp(struct blk_desc *desc, int hwpart)
 {
include/mmc.h +1 -0
index e815eb3..f39be3c 100644
@@ -513,6 +513,7 @@ void print_mmc_devices(char separator);
  * @return 0 if there is no MMC device, else the number of devices
  */
 int get_mmc_num(void);
+int mmc_switch_part(struct mmc *mmc, unsigned int part_num);
 int mmc_hwpart_config(struct mmc *mmc, const struct mmc_hwpart_conf *conf,
 		      enum mmc_hwpart_conf_mode mode);
 
Signature · 1 line
2.10.2
#26 lore raw

PATCH 7/14 mmc: Tinification of the mmc code

Hi Marek,

Quoted 10 lines · Marek Vasut

Add new configuration option CONFIG_MMC_TINY which strips away all memory allocation within the MMC code and code for handling multiple cards. This allows extremely space-constrained SPL code use the MMC framework.

---
V2: Switch the MMC_TINY option to Kconfig

Quoted 7 lines
---
 common/spl/spl_mmc.c     |  6 +++++-
 drivers/mmc/Kconfig      | 16 ++++++++++++++++
 drivers/mmc/mmc.c        | 31 ++++++++++++++++++++++++++++++-
 drivers/mmc/mmc_legacy.c | 32 ++++++++++++++++++++++++++++++++
 include/mmc.h            |  1 +
 5 files changed, 84 insertions(+), 2 deletions(-)

nit below.

I assume this does not use DM?

Quoted 23 lines
common/spl/spl_mmc.c +5 -1
index 43c1030..3cad998 100644
@@ -306,7 +306,11 @@ static int spl_mmc_load_image(struct spl_image_info *spl_image,
                        if (part == 7)
                                part = 0;
 
-                       err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part);
+                       if (CONFIG_IS_ENABLED(MMC_TINY))
+                               err = mmc_switch_part(mmc, part);
+                       else
+                               err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part);
+
                        if (err) {
 #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
                                puts("spl: mmc partition switch failed\n");
drivers/mmc/Kconfig +0 -0
index 24f4b28..aca438b8 100644

drivers/mmc/Kconfig config DM_MMC_OPS · line 31

Quoted 6 lines
drivers/mmc/Kconfig +3 -0
@@ -26,6 +26,22 @@ config DM_MMC_OPS
          option will be removed as soon as all DM_MMC drivers use it, as it
          will the only supported behaviour.
 
+config SPL_MMC_TINY
+       bool "Tiny MMC framework in SPL"
+       default n

Not needed?

Regards,
Simon

#27 lore raw

PATCH 7/14 mmc: Tinification of the mmc code

Quoted 21 lines · Simon Glass

Hi Marek,

Quoted 10 lines · Marek Vasut

Add new configuration option CONFIG_MMC_TINY which strips away all memory allocation within the MMC code and code for handling multiple cards. This allows extremely space-constrained SPL code use the MMC framework.

---
V2: Switch the MMC_TINY option to Kconfig

Quoted 7 lines
---
 common/spl/spl_mmc.c     |  6 +++++-
 drivers/mmc/Kconfig      | 16 ++++++++++++++++
 drivers/mmc/mmc.c        | 31 ++++++++++++++++++++++++++++++-
 drivers/mmc/mmc_legacy.c | 32 ++++++++++++++++++++++++++++++++
 include/mmc.h            |  1 +
 5 files changed, 84 insertions(+), 2 deletions(-)
Quoted 5 lines

nit below.

I assume this does not use DM?

Nope, DM does not fit into those 14 kiB I have for SPL, no way.

Quoted 25 lines
Quoted 25 lines
common/spl/spl_mmc.c +5 -1
index 43c1030..3cad998 100644
@@ -306,7 +306,11 @@ static int spl_mmc_load_image(struct spl_image_info *spl_image,
                        if (part == 7)
                                part = 0;
 
-                       err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part);
+                       if (CONFIG_IS_ENABLED(MMC_TINY))
+                               err = mmc_switch_part(mmc, part);
+                       else
+                               err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part);
+
                        if (err) {
 #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
                                puts("spl: mmc partition switch failed\n");
drivers/mmc/Kconfig +0 -0
index 24f4b28..aca438b8 100644
@@ -26,6 +26,22 @@ config DM_MMC_OPS
          option will be removed as soon as all DM_MMC drivers use it, as it
          will the only supported behaviour.
 
Quoted 5 lines

drivers/mmc/Kconfig config DM_MMC_OPS · line 31

Quoted 3 lines
drivers/mmc/Kconfig +3 -0
+config SPL_MMC_TINY
+       bool "Tiny MMC framework in SPL"
+       default n

Not needed?

Why not ? This makes the tinification configurable and is used on the CI20 (JZ4780 horrorchip).

Signature · 2 lines
Best regards,
Marek Vasut
#28 lore raw

PATCH 7/14 mmc: Tinification of the mmc code

Hi Marek,

Quoted 55 lines · Marek Vasut
Quoted 21 lines · Simon Glass

Hi Marek,

Quoted 10 lines · Marek Vasut

Add new configuration option CONFIG_MMC_TINY which strips away all memory allocation within the MMC code and code for handling multiple cards. This allows extremely space-constrained SPL code use the MMC framework.

---
V2: Switch the MMC_TINY option to Kconfig

Quoted 7 lines
---
 common/spl/spl_mmc.c     |  6 +++++-
 drivers/mmc/Kconfig      | 16 ++++++++++++++++
 drivers/mmc/mmc.c        | 31 ++++++++++++++++++++++++++++++-
 drivers/mmc/mmc_legacy.c | 32 ++++++++++++++++++++++++++++++++
 include/mmc.h            |  1 +
 5 files changed, 84 insertions(+), 2 deletions(-)
Quoted 5 lines

nit below.

I assume this does not use DM?

Nope, DM does not fit into those 14 kiB I have for SPL, no way.

Quoted 25 lines
Quoted 25 lines
common/spl/spl_mmc.c +5 -1
index 43c1030..3cad998 100644
@@ -306,7 +306,11 @@ static int spl_mmc_load_image(struct spl_image_info *spl_image,
                        if (part == 7)
                                part = 0;
 
-                       err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part);
+                       if (CONFIG_IS_ENABLED(MMC_TINY))
+                               err = mmc_switch_part(mmc, part);
+                       else
+                               err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part);
+
                        if (err) {
 #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
                                puts("spl: mmc partition switch failed\n");
drivers/mmc/Kconfig +0 -0
index 24f4b28..aca438b8 100644
@@ -26,6 +26,22 @@ config DM_MMC_OPS
          option will be removed as soon as all DM_MMC drivers use it, as it
          will the only supported behaviour.
 
Quoted 8 lines
Quoted 5 lines

drivers/mmc/Kconfig config DM_MMC_OPS · line 31

Quoted 3 lines
drivers/mmc/Kconfig +3 -0
+config SPL_MMC_TINY
+       bool "Tiny MMC framework in SPL"
+       default n

Not needed?

Why not ? This makes the tinification configurable and is used on the CI20 (JZ4780 horrorchip).

Well it should default to n anyway so I don't think you need that line.

Regards,
Simon

#29 lore raw

PATCH 7/14 mmc: Tinification of the mmc code

Quoted 64 lines · Simon Glass

Hi Marek,

Quoted 61 lines · Marek Vasut
Quoted 21 lines · Simon Glass

Hi Marek,

Quoted 10 lines · Marek Vasut

Add new configuration option CONFIG_MMC_TINY which strips away all memory allocation within the MMC code and code for handling multiple cards. This allows extremely space-constrained SPL code use the MMC framework.

---
V2: Switch the MMC_TINY option to Kconfig

Quoted 7 lines
---
 common/spl/spl_mmc.c     |  6 +++++-
 drivers/mmc/Kconfig      | 16 ++++++++++++++++
 drivers/mmc/mmc.c        | 31 ++++++++++++++++++++++++++++++-
 drivers/mmc/mmc_legacy.c | 32 ++++++++++++++++++++++++++++++++
 include/mmc.h            |  1 +
 5 files changed, 84 insertions(+), 2 deletions(-)
Quoted 5 lines

nit below.

I assume this does not use DM?

Nope, DM does not fit into those 14 kiB I have for SPL, no way.

Quoted 30 lines
Quoted 22 lines
common/spl/spl_mmc.c +5 -1
index 43c1030..3cad998 100644
@@ -306,7 +306,11 @@ static int spl_mmc_load_image(struct spl_image_info *spl_image,
                        if (part == 7)
                                part = 0;
 
-                       err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part);
+                       if (CONFIG_IS_ENABLED(MMC_TINY))
+                               err = mmc_switch_part(mmc, part);
+                       else
+                               err = blk_dselect_hwpart(mmc_get_blk_desc(mmc), part);
+
                        if (err) {
 #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
                                puts("spl: mmc partition switch failed\n");
drivers/mmc/Kconfig +0 -0
index 24f4b28..aca438b8 100644

drivers/mmc/Kconfig config DM_MMC_OPS · line 31

Quoted 6 lines
drivers/mmc/Kconfig +3 -0
@@ -26,6 +26,22 @@ config DM_MMC_OPS
          option will be removed as soon as all DM_MMC drivers use it, as it
          will the only supported behaviour.
 
+config SPL_MMC_TINY
+       bool "Tiny MMC framework in SPL"
+       default n

Not needed?

Quoted 4 lines
Quoted 2 lines

Why not ? This makes the tinification configurable and is used on the CI20 (JZ4780 horrorchip).

Well it should default to n anyway so I don't think you need that line.

To be clear, there's a lot of "default n" which isn't needed, but I hadn't publicly said anything about, but yes, we should stop adding that line as it is the normal default.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161129/fb940bd4/attachment.sig>
#30 lore raw

PATCH 8/14 mmc: Add JZ47xx SD/MMC controller driver

In reply to Marek Vasut

From: Paul Burton <[email protected]>

Add driver for the JZ47xx MSC controller.

Cc Daniel Schwierzeck <[email protected]>
Cc Paul Burton <[email protected]>
---
 drivers/mmc/Kconfig  |   6 +
 drivers/mmc/Makefile |   1 +
 drivers/mmc/jz_mmc.c | 443 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 450 insertions(+)
 create mode 100644 drivers/mmc/jz_mmc.c
drivers/mmc/Kconfig +6 -0
index aca438b8..761b886 100644
@@ -102,6 +102,12 @@ config MMC_UNIPHIER
 	help
 	  This selects support for the SD/MMC Host Controller on UniPhier SoCs.
 
+config JZ47XX_MMC
+	bool "Ingenic JZ47xx SD/MMC Host Controller support"
+	depends on ARCH_JZ47XX
+	help
+	  This selects support for the SD Card Controller on Ingenic JZ47xx SoCs.
+
 config SANDBOX_MMC
 	bool "Sandbox MMC support"
 	depends on MMC && SANDBOX
drivers/mmc/Makefile +1 -0
index d850758..5f7cca3 100644
@@ -57,6 +57,7 @@ obj-$(CONFIG_TEGRA_MMC) += tegra_mmc.o
 obj-$(CONFIG_MMC_UNIPHIER) += uniphier-sd.o
 obj-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o
 obj-$(CONFIG_ROCKCHIP_SDHCI) += rockchip_sdhci.o
+obj-$(CONFIG_JZ47XX_MMC) += jz_mmc.o
 
 ifdef CONFIG_SPL_BUILD
 obj-$(CONFIG_SPL_MMC_BOOT) += fsl_esdhc_spl.o
drivers/mmc/jz_mmc.c new file +443 -0

443 lines not on this page · raw message

Signature · 1 line
2.10.2
#31 lore raw

PATCH 8/14 mmc: Add JZ47xx SD/MMC controller driver

Hi Marek,

Quoted 3 lines · Marek Vasut

From: Paul Burton <[email protected]>

Add driver for the JZ47xx MSC controller.

There are some checkpatch error and warings. Could you fix them?

And i don't know what means MSC?

Quoted 133 lines
Cc Daniel Schwierzeck <[email protected]>
Cc Paul Burton <[email protected]>
---
 drivers/mmc/Kconfig  |   6 +
 drivers/mmc/Makefile |   1 +
 drivers/mmc/jz_mmc.c | 443 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 450 insertions(+)
 create mode 100644 drivers/mmc/jz_mmc.c
drivers/mmc/Kconfig +6 -0
index aca438b8..761b886 100644
@@ -102,6 +102,12 @@ config MMC_UNIPHIER
 	help
 	  This selects support for the SD/MMC Host Controller on UniPhier SoCs.
 
+config JZ47XX_MMC
+	bool "Ingenic JZ47xx SD/MMC Host Controller support"
+	depends on ARCH_JZ47XX
+	help
+	  This selects support for the SD Card Controller on Ingenic JZ47xx SoCs.
+
 config SANDBOX_MMC
 	bool "Sandbox MMC support"
 	depends on MMC && SANDBOX
drivers/mmc/Makefile +1 -0
index d850758..5f7cca3 100644
@@ -57,6 +57,7 @@ obj-$(CONFIG_TEGRA_MMC) += tegra_mmc.o
 obj-$(CONFIG_MMC_UNIPHIER) += uniphier-sd.o
 obj-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o
 obj-$(CONFIG_ROCKCHIP_SDHCI) += rockchip_sdhci.o
+obj-$(CONFIG_JZ47XX_MMC) += jz_mmc.o
 
 ifdef CONFIG_SPL_BUILD
 obj-$(CONFIG_SPL_MMC_BOOT) += fsl_esdhc_spl.o
drivers/mmc/jz_mmc.c new file +87 -0
new file mode 100644
index 0000000..213fe63
@@ -0,0 +1,443 @@
+/*
+ * Ingenic JZ MMC driver
+ *
+ * Copyright (c) 2013 Imagination Technologies
+ * Author: Paul Burton <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <malloc.h>
+#include <mmc.h>
+#include <errno.h>
+#include <asm/io.h>
+#include <asm/unaligned.h>
+#include <mach/jz4780.h>
+
+/* Registers */
+#define MSC_STRPCL			0x000
+#define MSC_STAT			0x004
+#define MSC_CLKRT			0x008
+#define MSC_CMDAT			0x00c
+#define MSC_RESTO			0x010
+#define MSC_RDTO			0x014
+#define MSC_BLKLEN			0x018
+#define MSC_NOB				0x01c
+#define MSC_SNOB			0x020
+#define MSC_IMASK			0x024
+#define MSC_IREG			0x028
+#define MSC_CMD				0x02c
+#define MSC_ARG				0x030
+#define MSC_RES				0x034
+#define MSC_RXFIFO			0x038
+#define MSC_TXFIFO			0x03c
+#define MSC_LPM 			0x040
+#define MSC_DMAC			0x044
+#define MSC_DMANDA			0x048
+#define MSC_DMADA			0x04c
+#define MSC_DMALEN			0x050
+#define MSC_DMACMD			0x054
+#define MSC_CTRL2			0x058
+#define MSC_RTCNT			0x05c
+#define MSC_DBG				0x0fc
+
+/* MSC Clock and Control Register (MSC_STRPCL) */
+
+#define MSC_STRPCL_EXIT_MULTIPLE	BIT(7)
+#define MSC_STRPCL_EXIT_TRANSFER	BIT(6)
+#define MSC_STRPCL_START_READWAIT	BIT(5)
+#define MSC_STRPCL_STOP_READWAIT	BIT(4)
+#define MSC_STRPCL_RESET		BIT(3)
+#define MSC_STRPCL_START_OP		BIT(2)
+#define MSC_STRPCL_CLOCK_CONTROL_STOP	BIT(0)
+#define MSC_STRPCL_CLOCK_CONTROL_START	BIT(1)
+
+/* MSC Status Register (MSC_STAT) */
+
+#define MSC_STAT_AUTO_CMD_DONE		BIT(31)
+#define MSC_STAT_IS_RESETTING		BIT(15)
+#define MSC_STAT_SDIO_INT_ACTIVE	BIT(14)
+#define MSC_STAT_PRG_DONE		BIT(13)
+#define MSC_STAT_DATA_TRAN_DONE		BIT(12)
+#define MSC_STAT_END_CMD_RES		BIT(11)
+#define MSC_STAT_DATA_FIFO_AFULL	BIT(10)
+#define MSC_STAT_IS_READWAIT		BIT(9)
+#define MSC_STAT_CLK_EN			BIT(8)
+#define MSC_STAT_DATA_FIFO_FULL		BIT(7)
+#define MSC_STAT_DATA_FIFO_EMPTY	BIT(6)
+#define MSC_STAT_CRC_RES_ERR		BIT(5)
+#define MSC_STAT_CRC_READ_ERROR		BIT(4)
+#define MSC_STAT_CRC_WRITE_ERROR	BIT(2)
+#define MSC_STAT_CRC_WRITE_ERROR_NOSTS	BIT(4)
+#define MSC_STAT_TIME_OUT_RES		BIT(1)
+#define MSC_STAT_TIME_OUT_READ		BIT(0)
+
+/* MSC Bus Clock Control Register (MSC_CLKRT) */
+#define MSC_CLKRT_CLK_RATE_MASK		0x7
+
+/* MSC Command Sequence Control Register (MSC_CMDAT) */
+
+#define MSC_CMDAT_IO_ABORT		BIT(11)
+#define MSC_CMDAT_BUS_WIDTH_1BIT	(0x0 << 9)
+#define MSC_CMDAT_BUS_WIDTH_4BIT	(0x2 << 9)
+#define MSC_CMDAT_DMA_EN		BIT(8)
+#define MSC_CMDAT_INIT			BIT(7)
+#define MSC_CMDAT_BUSY			BIT(6)
+#define MSC_CMDAT_STREAM_BLOCK		BIT(5)

drivers/mmc/jz_mmc.c line 97

Quoted 10 lines
drivers/mmc/jz_mmc.c new file +10 -0
+#define MSC_CMDAT_WRITE			BIT(4)
+#define MSC_CMDAT_DATA_EN		BIT(3)
+#define MSC_CMDAT_RESPONSE_MASK		(0x7 << 0)
+#define MSC_CMDAT_RESPONSE_NONE		(0x0 << 0) /* No response */
+#define MSC_CMDAT_RESPONSE_R1		(0x1 << 0) /* Format R1 and R1b */
+#define MSC_CMDAT_RESPONSE_R2		(0x2 << 0) /* Format R2 */
+#define MSC_CMDAT_RESPONSE_R3		(0x3 << 0) /* Format R3 */
+#define MSC_CMDAT_RESPONSE_R4		(0x4 << 0) /* Format R4 */
+#define MSC_CMDAT_RESPONSE_R5		(0x5 << 0) /* Format R5 */
+#define MSC_CMDAT_RESPONSE_R6		(0x6 << 0) /* Format R6 */

It doesn't need to shift 0.

Quoted 18 lines
drivers/mmc/jz_mmc.c new file +18 -0
+
+/* MSC Interrupts Mask Register (MSC_IMASK) */
+
+#define MSC_IMASK_TIME_OUT_RES		BIT(9)
+#define MSC_IMASK_TIME_OUT_READ		BIT(8)
+#define MSC_IMASK_SDIO			BIT(7)
+#define MSC_IMASK_TXFIFO_WR_REQ		BIT(6)
+#define MSC_IMASK_RXFIFO_RD_REQ		BIT(5)
+#define MSC_IMASK_END_CMD_RES		BIT(2)
+#define MSC_IMASK_PRG_DONE		BIT(1)
+#define MSC_IMASK_DATA_TRAN_DONE	BIT(0)
+
+
+/* MSC Interrupts Status Register (MSC_IREG) */
+
+#define MSC_IREG_TIME_OUT_RES		BIT(9)
+#define MSC_IREG_TIME_OUT_READ		BIT(8)
+#define MSC_IREG_SDIO			BIT(7)

drivers/mmc/jz_mmc.c line 125

Quoted 10 lines
drivers/mmc/jz_mmc.c new file +10 -0
+#define MSC_IREG_TXFIFO_WR_REQ		BIT(6)
+#define MSC_IREG_RXFIFO_RD_REQ		BIT(5)
+#define MSC_IREG_END_CMD_RES		BIT(2)
+#define MSC_IREG_PRG_DONE		BIT(1)
+#define MSC_IREG_DATA_TRAN_DONE		BIT(0)
+
+struct jz_mmc_priv {
+	struct mmc_config	cfg;
+	void __iomem		*regs;
+	u32			flags;	/* FIXME */

What FIXME?

drivers/mmc/jz_mmc.c line 137

Quoted 12 lines
drivers/mmc/jz_mmc.c new file +12 -0
+/* priv flags */
+#define JZ_MMC_BUS_WIDTH_MASK	0x3
+#define JZ_MMC_BUS_WIDTH_1	0x0
+#define JZ_MMC_BUS_WIDTH_4	0x2
+#define JZ_MMC_BUS_WIDTH_8	0x3
+#define JZ_MMC_SENT_INIT	BIT(2)
+};
+
+static int jz_mmc_clock_rate(void)
+{
+	return 24000000;
+}

I don't know this function is really needs..

drivers/mmc/jz_mmc.c line 147

Quoted 10 lines
drivers/mmc/jz_mmc.c new file +10 -0
+
+static int jz_mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
+						struct mmc_data *data)
+{
+	struct jz_mmc_priv *priv = mmc->priv;
+	u32 stat, mask, cmdat = 0;
+
+	/* stop the clock */
+	writel(MSC_STRPCL_CLOCK_CONTROL_STOP, priv->regs + MSC_STRPCL);
+	/* FIXME -- wait_bit() */

Fixme -- wait bit()?

Quoted 109 lines
drivers/mmc/jz_mmc.c new file +109 -0
+	while (readl(priv->regs + MSC_STAT) & MSC_STAT_CLK_EN);
+
+	writel(0, priv->regs + MSC_DMAC);
+
+	/* setup command */
+	writel(cmd->cmdidx, priv->regs + MSC_CMD);
+	writel(cmd->cmdarg, priv->regs + MSC_ARG);
+
+	if (data) {
+		/* setup data */
+		cmdat |= MSC_CMDAT_DATA_EN;
+		if (data->flags & MMC_DATA_WRITE)
+			cmdat |= MSC_CMDAT_WRITE;
+
+		writel(data->blocks, priv->regs + MSC_NOB);
+		writel(data->blocksize, priv->regs + MSC_BLKLEN);
+	} else {
+		writel(0, priv->regs + MSC_NOB);
+		writel(0, priv->regs + MSC_BLKLEN);
+	}
+
+	/* setup response */
+	switch (cmd->resp_type) {
+	case MMC_RSP_NONE:
+		break;
+	case MMC_RSP_R1:
+	case MMC_RSP_R1b:
+		cmdat |= MSC_CMDAT_RESPONSE_R1;
+		break;
+	case MMC_RSP_R2:
+		cmdat |= MSC_CMDAT_RESPONSE_R2;
+		break;
+	case MMC_RSP_R3:
+		cmdat |= MSC_CMDAT_RESPONSE_R3;
+		break;
+	default:
+		break;
+	}
+
+	if (cmd->resp_type & MMC_RSP_BUSY)
+		cmdat |= MSC_CMDAT_BUSY;
+
+	/* set init for the first command only */
+	if (!(priv->flags & JZ_MMC_SENT_INIT)) {
+		cmdat |= MSC_CMDAT_INIT;
+		priv->flags |= JZ_MMC_SENT_INIT;
+	}
+
+	cmdat |= (priv->flags & JZ_MMC_BUS_WIDTH_MASK) << 9;
+
+	/* write the data setup */
+	writel(cmdat, priv->regs + MSC_CMDAT);
+
+	/* unmask interrupts */
+	mask = 0xffffffff & ~(MSC_IMASK_END_CMD_RES | MSC_IMASK_TIME_OUT_RES);
+	if (data) {
+		mask &= ~MSC_IMASK_DATA_TRAN_DONE;
+		if (data->flags & MMC_DATA_WRITE)
+			mask &= ~MSC_IMASK_TXFIFO_WR_REQ;
+		else
+			mask &= ~(MSC_IMASK_RXFIFO_RD_REQ | MSC_IMASK_TIME_OUT_READ);
+	}
+	writel(mask, priv->regs + MSC_IMASK);
+
+	/* clear interrupts */
+	writel(0xffffffff, priv->regs + MSC_IREG);
+
+	/* start the command (& the clock) */
+	writel(MSC_STRPCL_START_OP | MSC_STRPCL_CLOCK_CONTROL_START, priv->regs + MSC_STRPCL);
+
+	/* wait for completion */
+	/* FIXME -- wait_bit() */
+	while (!(stat = (readl(priv->regs + MSC_IREG) & (MSC_IREG_END_CMD_RES | MSC_IREG_TIME_OUT_RES))))
+		mdelay(1);
+	writel(stat, priv->regs + MSC_IREG);
+	if (stat & MSC_IREG_TIME_OUT_RES)
+		return -ETIMEDOUT;
+
+	if (cmd->resp_type & MMC_RSP_PRESENT) {
+		/* read the response */
+		if (cmd->resp_type & MMC_RSP_136) {
+			u16 a, b, c, i;
+			a = readw(priv->regs + MSC_RES);
+			for (i = 0; i < 4; i++) {
+				b = readw(priv->regs + MSC_RES);
+				c = readw(priv->regs + MSC_RES);
+				cmd->response[i] = (a << 24) | (b << 8) | (c >> 8);
+				a = c;
+			}
+		} else {
+			cmd->response[0] = readw(priv->regs + MSC_RES) << 24;
+			cmd->response[0] |= readw(priv->regs + MSC_RES) << 8;
+			cmd->response[0] |= readw(priv->regs + MSC_RES) & 0xff;
+		}
+	}
+
+	if (data && (data->flags & MMC_DATA_WRITE)) {
+		/* write the data */
+		int sz = DIV_ROUND_UP(data->blocks * data->blocksize, 4);
+		const void *buf = data->src;
+
+		while (sz--) {
+			u32 val = get_unaligned_le32(buf);
+			while (!(readl(priv->regs + MSC_IREG) & MSC_IREG_TXFIFO_WR_REQ));
+			writel(val, priv->regs + MSC_TXFIFO);
+			buf += 4;
+		}
+	} else if (data && (data->flags & MMC_DATA_READ)) {
+		/* read the data */

drivers/mmc/jz_mmc.c line 266

Quoted 10 lines
drivers/mmc/jz_mmc.c new file +10 -0
+		int sz = data->blocks * data->blocksize;
+		void *buf = data->dest;
+
+		do {
+			stat = readl(priv->regs + MSC_STAT);
+
+			if (stat & MSC_STAT_TIME_OUT_READ)
+				return -ETIMEDOUT;
+			if (stat & MSC_STAT_CRC_READ_ERROR)
+				return -ECOMM;

Not ECOMM..It's CRC error..then use the "-EILSEQ 84"

drivers/mmc/jz_mmc.c line 268

Quoted 2 lines
drivers/mmc/jz_mmc.c new file +2 -0
+			if (stat & MSC_STAT_DATA_FIFO_EMPTY) {
+				udelay(10);

Why put udelay at here?

Quoted 105 lines
drivers/mmc/jz_mmc.c new file +105 -0
+				continue;
+			}
+			do {
+				u32 val = readl(priv->regs + MSC_RXFIFO);
+
+				if (sz == 1)
+					*(u8 *)buf = (u8)val;
+				else if (sz == 2)
+					put_unaligned_le16(val, buf);
+				else if (sz >= 4)
+					put_unaligned_le32(val, buf);
+				buf += 4;
+				sz -= 4;
+				stat = readl(priv->regs + MSC_STAT);
+			} while (!(stat & MSC_STAT_DATA_FIFO_EMPTY));
+		} while (!(stat & MSC_STAT_DATA_TRAN_DONE));
+	}
+
+	return 0;
+}
+
+static void jz_mmc_set_ios(struct mmc *mmc)
+{
+	struct jz_mmc_priv *priv = mmc->priv;
+	u32 real_rate = jz_mmc_clock_rate();
+	u8 clk_div = 0;
+
+	/* calculate clock divide */
+	while ((real_rate > mmc->clock) && (clk_div < 7)) {
+		real_rate >>= 1;
+		clk_div++;
+	}
+	writel(clk_div & MSC_CLKRT_CLK_RATE_MASK, priv->regs + MSC_CLKRT);
+
+	/* set the bus width for the next command */
+	priv->flags &= ~JZ_MMC_BUS_WIDTH_MASK;
+	if (mmc->bus_width == 8)
+		priv->flags |= JZ_MMC_BUS_WIDTH_8;
+	else if (mmc->bus_width == 4)
+		priv->flags |= JZ_MMC_BUS_WIDTH_4;
+	else
+		priv->flags |= JZ_MMC_BUS_WIDTH_1;
+}
+
+static int jz_mmc_core_init(struct mmc *mmc)
+{
+	struct jz_mmc_priv *priv = mmc->priv;
+
+	/* Reset */
+	writel(MSC_STRPCL_RESET, priv->regs + MSC_STRPCL);
+	/* FIXME -- wait_bit() */
+	while (readl(priv->regs + MSC_STAT) & MSC_STAT_IS_RESETTING);
+
+	/* Maximum timeouts */
+	writel(0xffff, priv->regs + MSC_RESTO);
+	writel(0xffffffff, priv->regs + MSC_RDTO);
+
+	/* Enable low power mode */
+	writel(0x1, priv->regs + MSC_LPM);
+
+	return 0;
+}
+
+static const struct mmc_ops jz_msc_ops = {
+	.send_cmd	= jz_mmc_send_cmd,
+	.set_ios	= jz_mmc_set_ios,
+	.init		= jz_mmc_core_init,
+};
+
+#ifdef CONFIG_MMC_TINY
+static struct jz_mmc_priv jz_mmc_priv_static = {
+	.cfg = {
+		.name = "MSC",
+		.ops = &jz_msc_ops,
+
+		.voltages = MMC_VDD_27_28 | MMC_VDD_28_29 | MMC_VDD_29_30 |
+				MMC_VDD_30_31 | MMC_VDD_31_32 | MMC_VDD_32_33 |
+				MMC_VDD_33_34 | MMC_VDD_34_35 | MMC_VDD_35_36,
+		.host_caps = MMC_MODE_4BIT | MMC_MODE_HS_52MHz | MMC_MODE_HS,
+
+		.f_min = 375000,
+		.f_max = 48000000,
+		.b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT,
+	},
+};
+
+int jz_mmc_init(void __iomem *base)
+{
+	struct mmc *mmc;
+
+	jz_mmc_priv_static.regs = base;
+
+	mmc = mmc_create(&jz_mmc_priv_static.cfg, &jz_mmc_priv_static);
+
+	return mmc ? 0 : -ENODEV;
+}
+#endif
+
+#ifdef CONFIG_DM_MMC
+#include <dm.h>
+DECLARE_GLOBAL_DATA_PTR;
+
+static int jz_mmc_ofdata_to_platdata(struct udevice *dev)
+{
+	struct jz_mmc_priv *priv = dev_get_priv(dev);

drivers/mmc/jz_mmc.c line 383

Quoted 10 lines
drivers/mmc/jz_mmc.c new file +10 -0
+	const void *fdt = gd->fdt_blob;
+	int node = dev->of_offset;
+	struct mmc_config *cfg;
+	int val;
+
+	priv->regs = map_physmem(dev_get_addr(dev), 0x100, MAP_NOCACHE);
+	cfg = &priv->cfg;
+
+	cfg->host_caps = MMC_MODE_HS_52MHz | MMC_MODE_HS;
+	val = fdtdec_get_int(fdt, node, "bus-width", -1);

I think that almost all mmc can support 1bit buswidth..

drivers/mmc/jz_mmc.c line 396

Quoted 13 lines
drivers/mmc/jz_mmc.c new file +13 -0
+	if (val < 0) {
+		printf("error: bus-width property missing\n");
+		return -ENOENT;
+	}
+
+	switch (val) {
+	case 0x8:
+		cfg->host_caps |= MMC_MODE_8BIT;
+	case 0x4:
+		cfg->host_caps |= MMC_MODE_4BIT;
+		break;
+	default:
+		printf("error: invalid bus-width property\n");

Dosen't consider 1bit-buswidth?

drivers/mmc/jz_mmc.c line 404

Quoted 8 lines
drivers/mmc/jz_mmc.c new file +8 -0
+		return -ENOENT;
+	}
+
+	cfg->f_min = 400000;
+	cfg->f_max = fdtdec_get_int(fdt, node, "max-frequency", 52000000);
+
+
+

Remove the unnecessary wihte space..

Quoted 40 lines
drivers/mmc/jz_mmc.c new file +39 -0
+	cfg->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
+	cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;
+
+	return 0;
+}
+
+static int jz_mmc_probe(struct udevice *dev)
+{
+	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
+	struct jz_mmc_priv *priv = dev_get_priv(dev);
+	struct mmc_config *cfg = &priv->cfg;
+	struct mmc *mmc;
+
+	cfg->name = "MSC";
+	cfg->ops = &jz_msc_ops;
+
+	mmc = mmc_create(cfg, priv);
+	if (!mmc)
+		return -ENODEV;
+
+	mmc->dev = dev;
+	upriv->mmc = mmc;
+
+	return 0;
+}
+static const struct udevice_id jz_mmc_ids[] = {
+	{ .compatible = "ingenic,jz4780-mmc" },
+	{ }
+};
+
+U_BOOT_DRIVER(jz_mmc_drv) = {
+	.name			= "jz_mmc",
+	.id			= UCLASS_MMC,
+	.of_match		= jz_mmc_ids,
+	.ofdata_to_platdata	= jz_mmc_ofdata_to_platdata,
+	.probe			= jz_mmc_probe,
+	.priv_auto_alloc_size	= sizeof(struct jz_mmc_priv),
+};
+#endif
#32 lore raw

PATCH 8/14 mmc: Add JZ47xx SD/MMC controller driver

Quoted 8 lines · Jaehoon Chung

Hi Marek,

Quoted 3 lines · Marek Vasut

From: Paul Burton <[email protected]>

Add driver for the JZ47xx MSC controller.

There are some checkpatch error and warings. Could you fix them?

Yeah

Quoted 1 line

And i don't know what means MSC?

Me neither, probably MMC SD Controller .

Quoted 167 lines
Quoted 132 lines
Cc Daniel Schwierzeck <[email protected]>
Cc Paul Burton <[email protected]>
---
 drivers/mmc/Kconfig  |   6 +
 drivers/mmc/Makefile |   1 +
 drivers/mmc/jz_mmc.c | 443 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 450 insertions(+)
 create mode 100644 drivers/mmc/jz_mmc.c
drivers/mmc/Kconfig +6 -0
index aca438b8..761b886 100644
@@ -102,6 +102,12 @@ config MMC_UNIPHIER
 	help
 	  This selects support for the SD/MMC Host Controller on UniPhier SoCs.
 
+config JZ47XX_MMC
+	bool "Ingenic JZ47xx SD/MMC Host Controller support"
+	depends on ARCH_JZ47XX
+	help
+	  This selects support for the SD Card Controller on Ingenic JZ47xx SoCs.
+
 config SANDBOX_MMC
 	bool "Sandbox MMC support"
 	depends on MMC && SANDBOX
drivers/mmc/Makefile +1 -0
index d850758..5f7cca3 100644
@@ -57,6 +57,7 @@ obj-$(CONFIG_TEGRA_MMC) += tegra_mmc.o
 obj-$(CONFIG_MMC_UNIPHIER) += uniphier-sd.o
 obj-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o
 obj-$(CONFIG_ROCKCHIP_SDHCI) += rockchip_sdhci.o
+obj-$(CONFIG_JZ47XX_MMC) += jz_mmc.o
 
 ifdef CONFIG_SPL_BUILD
 obj-$(CONFIG_SPL_MMC_BOOT) += fsl_esdhc_spl.o
drivers/mmc/jz_mmc.c new file +87 -0
new file mode 100644
index 0000000..213fe63
@@ -0,0 +1,443 @@
+/*
+ * Ingenic JZ MMC driver
+ *
+ * Copyright (c) 2013 Imagination Technologies
+ * Author: Paul Burton <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <malloc.h>
+#include <mmc.h>
+#include <errno.h>
+#include <asm/io.h>
+#include <asm/unaligned.h>
+#include <mach/jz4780.h>
+
+/* Registers */
+#define MSC_STRPCL			0x000
+#define MSC_STAT			0x004
+#define MSC_CLKRT			0x008
+#define MSC_CMDAT			0x00c
+#define MSC_RESTO			0x010
+#define MSC_RDTO			0x014
+#define MSC_BLKLEN			0x018
+#define MSC_NOB				0x01c
+#define MSC_SNOB			0x020
+#define MSC_IMASK			0x024
+#define MSC_IREG			0x028
+#define MSC_CMD				0x02c
+#define MSC_ARG				0x030
+#define MSC_RES				0x034
+#define MSC_RXFIFO			0x038
+#define MSC_TXFIFO			0x03c
+#define MSC_LPM 			0x040
+#define MSC_DMAC			0x044
+#define MSC_DMANDA			0x048
+#define MSC_DMADA			0x04c
+#define MSC_DMALEN			0x050
+#define MSC_DMACMD			0x054
+#define MSC_CTRL2			0x058
+#define MSC_RTCNT			0x05c
+#define MSC_DBG				0x0fc
+
+/* MSC Clock and Control Register (MSC_STRPCL) */
+
+#define MSC_STRPCL_EXIT_MULTIPLE	BIT(7)
+#define MSC_STRPCL_EXIT_TRANSFER	BIT(6)
+#define MSC_STRPCL_START_READWAIT	BIT(5)
+#define MSC_STRPCL_STOP_READWAIT	BIT(4)
+#define MSC_STRPCL_RESET		BIT(3)
+#define MSC_STRPCL_START_OP		BIT(2)
+#define MSC_STRPCL_CLOCK_CONTROL_STOP	BIT(0)
+#define MSC_STRPCL_CLOCK_CONTROL_START	BIT(1)
+
+/* MSC Status Register (MSC_STAT) */
+
+#define MSC_STAT_AUTO_CMD_DONE		BIT(31)
+#define MSC_STAT_IS_RESETTING		BIT(15)
+#define MSC_STAT_SDIO_INT_ACTIVE	BIT(14)
+#define MSC_STAT_PRG_DONE		BIT(13)
+#define MSC_STAT_DATA_TRAN_DONE		BIT(12)
+#define MSC_STAT_END_CMD_RES		BIT(11)
+#define MSC_STAT_DATA_FIFO_AFULL	BIT(10)
+#define MSC_STAT_IS_READWAIT		BIT(9)
+#define MSC_STAT_CLK_EN			BIT(8)
+#define MSC_STAT_DATA_FIFO_FULL		BIT(7)
+#define MSC_STAT_DATA_FIFO_EMPTY	BIT(6)
+#define MSC_STAT_CRC_RES_ERR		BIT(5)
+#define MSC_STAT_CRC_READ_ERROR		BIT(4)
+#define MSC_STAT_CRC_WRITE_ERROR	BIT(2)
+#define MSC_STAT_CRC_WRITE_ERROR_NOSTS	BIT(4)
+#define MSC_STAT_TIME_OUT_RES		BIT(1)
+#define MSC_STAT_TIME_OUT_READ		BIT(0)
+
+/* MSC Bus Clock Control Register (MSC_CLKRT) */
+#define MSC_CLKRT_CLK_RATE_MASK		0x7
+
+/* MSC Command Sequence Control Register (MSC_CMDAT) */
+
+#define MSC_CMDAT_IO_ABORT		BIT(11)
+#define MSC_CMDAT_BUS_WIDTH_1BIT	(0x0 << 9)
+#define MSC_CMDAT_BUS_WIDTH_4BIT	(0x2 << 9)
+#define MSC_CMDAT_DMA_EN		BIT(8)
+#define MSC_CMDAT_INIT			BIT(7)
+#define MSC_CMDAT_BUSY			BIT(6)
+#define MSC_CMDAT_STREAM_BLOCK		BIT(5)

drivers/mmc/jz_mmc.c line 97

Quoted 10 lines
drivers/mmc/jz_mmc.c new file +10 -0
+#define MSC_CMDAT_WRITE			BIT(4)
+#define MSC_CMDAT_DATA_EN		BIT(3)
+#define MSC_CMDAT_RESPONSE_MASK		(0x7 << 0)
+#define MSC_CMDAT_RESPONSE_NONE		(0x0 << 0) /* No response */
+#define MSC_CMDAT_RESPONSE_R1		(0x1 << 0) /* Format R1 and R1b */
+#define MSC_CMDAT_RESPONSE_R2		(0x2 << 0) /* Format R2 */
+#define MSC_CMDAT_RESPONSE_R3		(0x3 << 0) /* Format R3 */
+#define MSC_CMDAT_RESPONSE_R4		(0x4 << 0) /* Format R4 */
+#define MSC_CMDAT_RESPONSE_R5		(0x5 << 0) /* Format R5 */
+#define MSC_CMDAT_RESPONSE_R6		(0x6 << 0) /* Format R6 */

It doesn't need to shift 0.

Quoted 22 lines
drivers/mmc/jz_mmc.c new file +22 -0
+
+/* MSC Interrupts Mask Register (MSC_IMASK) */
+
+#define MSC_IMASK_TIME_OUT_RES		BIT(9)
+#define MSC_IMASK_TIME_OUT_READ		BIT(8)
+#define MSC_IMASK_SDIO			BIT(7)
+#define MSC_IMASK_TXFIFO_WR_REQ		BIT(6)
+#define MSC_IMASK_RXFIFO_RD_REQ		BIT(5)
+#define MSC_IMASK_END_CMD_RES		BIT(2)
+#define MSC_IMASK_PRG_DONE		BIT(1)
+#define MSC_IMASK_DATA_TRAN_DONE	BIT(0)
+
+
+/* MSC Interrupts Status Register (MSC_IREG) */
+
+#define MSC_IREG_TIME_OUT_RES		BIT(9)
+#define MSC_IREG_TIME_OUT_READ		BIT(8)
+#define MSC_IREG_SDIO			BIT(7)
+#define MSC_IREG_TXFIFO_WR_REQ		BIT(6)
+#define MSC_IREG_RXFIFO_RD_REQ		BIT(5)
+#define MSC_IREG_END_CMD_RES		BIT(2)
+#define MSC_IREG_PRG_DONE		BIT(1)
Quoted 8 lines

drivers/mmc/jz_mmc.c line 125

Quoted 6 lines
drivers/mmc/jz_mmc.c new file +6 -0
+#define MSC_IREG_DATA_TRAN_DONE		BIT(0)
+
+struct jz_mmc_priv {
+	struct mmc_config	cfg;
+	void __iomem		*regs;
+	u32			flags;	/* FIXME */

What FIXME?

Dunno, removed.

Quoted 6 lines
Quoted 6 lines
drivers/mmc/jz_mmc.c new file +6 -0
+/* priv flags */
+#define JZ_MMC_BUS_WIDTH_MASK	0x3
+#define JZ_MMC_BUS_WIDTH_1	0x0
+#define JZ_MMC_BUS_WIDTH_4	0x2
+#define JZ_MMC_BUS_WIDTH_8	0x3
+#define JZ_MMC_SENT_INIT	BIT(2)
Quoted 8 lines

drivers/mmc/jz_mmc.c line 137

Quoted 6 lines
drivers/mmc/jz_mmc.c new file +6 -0
+};
+
+static int jz_mmc_clock_rate(void)
+{
+	return 24000000;
+}

I don't know this function is really needs..

Ev. this clock rate should come from DT, so this function should be here. But, we don't support DT in SPL due to size concerns (the SPL has to be below 14 kiB, see previous patch), so there will eventually be come more code here I think.

Quoted 4 lines
Quoted 4 lines
drivers/mmc/jz_mmc.c new file +4 -0
+
+static int jz_mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
+						struct mmc_data *data)
+{
Quoted 8 lines

drivers/mmc/jz_mmc.c line 147

Quoted 6 lines
drivers/mmc/jz_mmc.c new file +6 -0
+	struct jz_mmc_priv *priv = mmc->priv;
+	u32 stat, mask, cmdat = 0;
+
+	/* stop the clock */
+	writel(MSC_STRPCL_CLOCK_CONTROL_STOP, priv->regs + MSC_STRPCL);
+	/* FIXME -- wait_bit() */

Fixme -- wait bit()?

Seems like wait_for_bit now fits, so fixed.

Quoted 3 lines
Quoted 3 lines
drivers/mmc/jz_mmc.c new file +3 -0
+	while (readl(priv->regs + MSC_STAT) & MSC_STAT_CLK_EN);
+
+	writel(0, priv->regs + MSC_DMAC);

[...]

Quoted 15 lines

drivers/mmc/jz_mmc.c line 266

Quoted 12 lines
drivers/mmc/jz_mmc.c new file +12 -0
+	} else if (data && (data->flags & MMC_DATA_READ)) {
+		/* read the data */
+		int sz = data->blocks * data->blocksize;
+		void *buf = data->dest;
+
+		do {
+			stat = readl(priv->regs + MSC_STAT);
+
+			if (stat & MSC_STAT_TIME_OUT_READ)
+				return -ETIMEDOUT;
+			if (stat & MSC_STAT_CRC_READ_ERROR)
+				return -ECOMM;

Not ECOMM..It's CRC error..then use the "-EILSEQ 84"

Quoted 4 lines

drivers/mmc/jz_mmc.c line 268

Quoted 2 lines
drivers/mmc/jz_mmc.c new file +2 -0
+			if (stat & MSC_STAT_DATA_FIFO_EMPTY) {
+				udelay(10);

Why put udelay at here?

Wait until more data get loaded, the hardware is crap and sensitive to me polling this register too often (sigh).

Quoted 20 lines
Quoted 20 lines
drivers/mmc/jz_mmc.c new file +20 -0
+				continue;
+			}
+			do {
+				u32 val = readl(priv->regs + MSC_RXFIFO);
+
+				if (sz == 1)
+					*(u8 *)buf = (u8)val;
+				else if (sz == 2)
+					put_unaligned_le16(val, buf);
+				else if (sz >= 4)
+					put_unaligned_le32(val, buf);
+				buf += 4;
+				sz -= 4;
+				stat = readl(priv->regs + MSC_STAT);
+			} while (!(stat & MSC_STAT_DATA_FIFO_EMPTY));
+		} while (!(stat & MSC_STAT_DATA_TRAN_DONE));
+	}
+
+	return 0;
+}

[...]

Quoted 7 lines
Quoted 7 lines
drivers/mmc/jz_mmc.c new file +7 -0
+static int jz_mmc_ofdata_to_platdata(struct udevice *dev)
+{
+	struct jz_mmc_priv *priv = dev_get_priv(dev);
+	const void *fdt = gd->fdt_blob;
+	int node = dev->of_offset;
+	struct mmc_config *cfg;
+	int val;
Quoted 8 lines

drivers/mmc/jz_mmc.c line 383

Quoted 6 lines
drivers/mmc/jz_mmc.c new file +6 -0
+
+	priv->regs = map_physmem(dev_get_addr(dev), 0x100, MAP_NOCACHE);
+	cfg = &priv->cfg;
+
+	cfg->host_caps = MMC_MODE_HS_52MHz | MMC_MODE_HS;
+	val = fdtdec_get_int(fdt, node, "bus-width", -1);

I think that almost all mmc can support 1bit buswidth..

True

Quoted 7 lines
Quoted 7 lines
drivers/mmc/jz_mmc.c new file +7 -0
+	if (val < 0) {
+		printf("error: bus-width property missing\n");
+		return -ENOENT;
+	}
+
+	switch (val) {
+	case 0x8:
Quoted 8 lines

drivers/mmc/jz_mmc.c line 396

Quoted 6 lines
drivers/mmc/jz_mmc.c new file +6 -0
+		cfg->host_caps |= MMC_MODE_8BIT;
+	case 0x4:
+		cfg->host_caps |= MMC_MODE_4BIT;
+		break;
+	default:
+		printf("error: invalid bus-width property\n");

Dosen't consider 1bit-buswidth?

Oh, good catch.

Quoted 8 lines
Quoted 8 lines
drivers/mmc/jz_mmc.c new file +8 -0
+		return -ENOENT;
+	}
+
+	cfg->f_min = 400000;
+	cfg->f_max = fdtdec_get_int(fdt, node, "max-frequency", 52000000);
+
+
+

[...]

Signature · 2 lines
Best regards,
Marek Vasut
#33 lore raw

PATCH 9/14 gpio: Add JZ47xx GPIO driver

In reply to Marek Vasut

From: Paul Burton <[email protected]>

Add primitive GPIO controller driver for the JZ47xx SoC.

Cc Daniel Schwierzeck <[email protected]>
Cc Paul Burton <[email protected]>
---
 drivers/gpio/Kconfig       |  8 +++++
 drivers/gpio/Makefile      |  1 +
 drivers/gpio/gpio-jz47xx.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 88 insertions(+)
 create mode 100644 drivers/gpio/gpio-jz47xx.c
drivers/gpio/Kconfig +8 -0
index 8d9ab52..4515883 100644
@@ -221,4 +221,12 @@ config MPC85XX_GPIO
 
 	  The driver has been tested on MPC85XX, but it is likely that other
 	  PowerQUICC III devices will work as well.
+
+config JZ47XX_GPIO
+	bool "Ingenic JZ47xx GPIO driver"
+	depends on ARCH_JZ47XX
+	default y
+	help
+	  Supports GPIO access on Ingenic JZ47xx SoCs.
+
 endmenu
drivers/gpio/Makefile +1 -0
index 8939226..e562022 100644
@@ -58,3 +58,4 @@ obj-$(CONFIG_MVEBU_GPIO)	+= mvebu_gpio.o
 obj-$(CONFIG_MSM_GPIO)		+= msm_gpio.o
 obj-$(CONFIG_$(SPL_)PCF8575_GPIO)	+= pcf8575_gpio.o
 obj-$(CONFIG_PM8916_GPIO)	+= pm8916_gpio.o
+obj-$(CONFIG_JZ47XX_GPIO)	+= gpio-jz47xx.o
drivers/gpio/gpio-jz47xx.c new file +79 -0

79 lines not on this page · raw message

Signature · 1 line
2.10.2
#34 lore raw

PATCH 10/14 misc: Add JZ47xx efuse driver

In reply to Marek Vasut

From: Paul Burton <[email protected]>

Add driver for the efuse block in the JZ47xx SOC.

Cc Daniel Schwierzeck <[email protected]>
Cc Paul Burton <[email protected]>
---
 drivers/misc/Kconfig        |   6 +++
 drivers/misc/Makefile       |   1 +
 drivers/misc/jz4780_efuse.c | 100 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 107 insertions(+)
 create mode 100644 drivers/misc/jz4780_efuse.c
drivers/misc/Kconfig +6 -0
index 1aae4bc..ed0659b 100644
@@ -83,6 +83,12 @@ config FSL_SEC_MON
 	  Security Monitor can be transitioned on any security failures,
 	  like software violations or hardware security violations.
 
+config JZ4780_EFUSE
+	bool "Ingenic JZ4780 eFUSE support"
+	depends on ARCH_JZ47XX
+	help
+	  This selects support for the eFUSE on Ingenic JZ4780 SoCs.
+
 config MXC_OCOTP
 	bool "Enable MXC OCOTP Driver"
 	help
drivers/misc/Makefile +1 -0
index 9fbb5a7..6904a76 100644
@@ -51,3 +51,4 @@ obj-$(CONFIG_PCA9551_LED) += pca9551_led.o
 obj-$(CONFIG_FSL_DEVICE_DISABLE) += fsl_devdis.o
 obj-$(CONFIG_WINBOND_W83627) += winbond_w83627.o
 obj-$(CONFIG_QFW) += qfw.o
+obj-$(CONFIG_JZ4780_EFUSE) += jz4780_efuse.o
drivers/misc/jz4780_efuse.c new file +100 -0

100 lines not on this page · raw message

Signature · 1 line
2.10.2
#35 lore raw

PATCH 11/14 mips: Add SPL header

In reply to Marek Vasut

From: Paul Burton <[email protected]>

Add header with SPL boot mode and type definitions.

Cc Daniel Schwierzeck <[email protected]>
Cc Paul Burton <[email protected]>
---
 arch/mips/include/asm/spl.h | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 arch/mips/include/asm/spl.h
arch/mips/include/asm/spl.h new file +35 -0
new file mode 100644
index 0000000..01baab6
@@ -0,0 +1,35 @@
+/*
+ * (C) Copyright 2012
+ * Texas Instruments, <www.ti.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+#ifndef	_ASM_SPL_H_
+#define	_ASM_SPL_H_
+
+enum {
+	BOOT_DEVICE_RAM,
+	BOOT_DEVICE_MMC1,
+	BOOT_DEVICE_MMC2,
+	BOOT_DEVICE_MMC2_2,
+	BOOT_DEVICE_NAND,
+	BOOT_DEVICE_ONENAND,
+	BOOT_DEVICE_NOR,
+	BOOT_DEVICE_UART,
+	BOOT_DEVICE_SPI,
+	BOOT_DEVICE_USB,
+	BOOT_DEVICE_SATA,
+	BOOT_DEVICE_I2C,
+	BOOT_DEVICE_BOARD,
+	BOOT_DEVICE_NONE
+};
+
+/* Linker symbols. */
+extern char __bss_start[];
+extern ulong __bss_end;
+
+#ifndef CONFIG_DM
+extern gd_t gdata;
+#endif
+
+#endif
Signature · 1 line
2.10.2
#36 lore raw

PATCH 12/14 mips: jz47xx: Add JZ4780 SoC support

In reply to Marek Vasut

From: Paul Burton <[email protected]>

Add initial support for the Ingenic JZ47xx MIPS SoC. The DTSI file comes from Linux 4.6 as of revision 78800558d104e003f9ae92e0107f1de39cf9de9f

Cc Daniel Schwierzeck <[email protected]>
Cc Paul Burton <[email protected]>
---
 arch/mips/Kconfig                                |   7 +
 arch/mips/Makefile                               |   1 +
 arch/mips/dts/jz4780.dtsi                        | 137 ++++++
 arch/mips/mach-jz47xx/Kconfig                    |  15 +
 arch/mips/mach-jz47xx/Makefile                   |   7 +
 arch/mips/mach-jz47xx/include/mach/jz4780.h      |  99 +++++
 arch/mips/mach-jz47xx/include/mach/jz4780_dram.h | 457 ++++++++++++++++++++
 arch/mips/mach-jz47xx/jz4780/Makefile            |   5 +
 arch/mips/mach-jz47xx/jz4780/jz4780.c            | 135 ++++++
 arch/mips/mach-jz47xx/jz4780/pll.c               | 527 +++++++++++++++++++++++
 arch/mips/mach-jz47xx/jz4780/sdram.c             | 271 ++++++++++++
 arch/mips/mach-jz47xx/jz4780/timer.c             | 238 ++++++++++
 arch/mips/mach-jz47xx/jz4780/u-boot-spl.lds      |  52 +++
 arch/mips/mach-jz47xx/start.S                    |  99 +++++
 include/dt-bindings/clock/jz4780-cgu.h           |  88 ++++
 15 files changed, 2138 insertions(+)
 create mode 100644 arch/mips/dts/jz4780.dtsi
 create mode 100644 arch/mips/mach-jz47xx/Kconfig
 create mode 100644 arch/mips/mach-jz47xx/Makefile
 create mode 100644 arch/mips/mach-jz47xx/include/mach/jz4780.h
 create mode 100644 arch/mips/mach-jz47xx/include/mach/jz4780_dram.h
 create mode 100644 arch/mips/mach-jz47xx/jz4780/Makefile
 create mode 100644 arch/mips/mach-jz47xx/jz4780/jz4780.c
 create mode 100644 arch/mips/mach-jz47xx/jz4780/pll.c
 create mode 100644 arch/mips/mach-jz47xx/jz4780/sdram.c
 create mode 100644 arch/mips/mach-jz47xx/jz4780/timer.c
 create mode 100644 arch/mips/mach-jz47xx/jz4780/u-boot-spl.lds
 create mode 100644 arch/mips/mach-jz47xx/start.S
 create mode 100644 include/dt-bindings/clock/jz4780-cgu.h
arch/mips/Kconfig +7 -0
index 097ad58..ec381b0 100644
@@ -70,6 +70,12 @@ config ARCH_ATH79
 	select OF_CONTROL
 	select DM
 
+config ARCH_JZ47XX
+	bool "Support Ingenic JZ47xx"
+	select SUPPORT_SPL
+	select OF_CONTROL
+	select DM
+
 config MACH_PIC32
 	bool "Support Microchip PIC32"
 	select OF_CONTROL
@@ -116,6 +122,7 @@ source "board/micronas/vct/Kconfig"
 source "board/pb1x00/Kconfig"
 source "board/qemu-mips/Kconfig"
 source "arch/mips/mach-ath79/Kconfig"
+source "arch/mips/mach-jz47xx/Kconfig"
 source "arch/mips/mach-pic32/Kconfig"
 
 if MIPS
arch/mips/Makefile +1 -0
index efe7e44..4f0f797 100644
@@ -15,6 +15,7 @@ libs-y += arch/mips/lib/
 
 machine-$(CONFIG_SOC_AU1X00) += au1x00
 machine-$(CONFIG_ARCH_ATH79) += ath79
+machine-$(CONFIG_ARCH_JZ47XX) += jz47xx
 machine-$(CONFIG_MACH_PIC32) += pic32
 
 machdirs := $(patsubst %,arch/mips/mach-%/,$(machine-y))
arch/mips/dts/jz4780.dtsi new file +137 -0

137 lines not on this page · raw message

arch/mips/mach-jz47xx/Kconfig new file +15 -0
new file mode 100644
index 0000000..cd6944c
@@ -0,0 +1,15 @@
+menu "Ingenic JZ47xx platforms"
+	depends on ARCH_JZ47XX
+
+config SYS_SOC
+	default "jz47xx"
+
+config SOC_JZ4780
+	bool
+	select SUPPORTS_LITTLE_ENDIAN
+	select SUPPORTS_CPU_MIPS32_R1
+	select SUPPORTS_CPU_MIPS32_R2
+	help
+	  Support for Ingenic JZ4780 family SoCs.
+
+endmenu
arch/mips/mach-jz47xx/Makefile new file +7 -0
new file mode 100644
index 0000000..5eee9ac
@@ -0,0 +1,7 @@
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+
+extra-$(CONFIG_SPL_BUILD)	:= start.o
+
+obj-$(CONFIG_SOC_JZ4780)	+= jz4780/
arch/mips/mach-jz47xx/include/mach/jz4780.h new file +99 -0

99 lines not on this page · raw message

arch/mips/mach-jz47xx/include/mach/jz4780_dram.h new file +457 -0

457 lines not on this page · raw message

arch/mips/mach-jz47xx/jz4780/Makefile new file +5 -0
new file mode 100644
index 0000000..683eea6
@@ -0,0 +1,5 @@
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+
+obj-y := jz4780.o pll.o sdram.o timer.o
arch/mips/mach-jz47xx/jz4780/jz4780.c new file +135 -0

135 lines not on this page · raw message

arch/mips/mach-jz47xx/jz4780/pll.c new file +527 -0

527 lines not on this page · raw message

arch/mips/mach-jz47xx/jz4780/sdram.c new file +271 -0

271 lines not on this page · raw message

arch/mips/mach-jz47xx/jz4780/timer.c new file +238 -0

238 lines not on this page · raw message

arch/mips/mach-jz47xx/jz4780/u-boot-spl.lds new file +52 -0

52 lines not on this page · raw message

arch/mips/mach-jz47xx/start.S new file +99 -0

99 lines not on this page · raw message

include/dt-bindings/clock/jz4780-cgu.h new file +88 -0

88 lines not on this page · raw message

Signature · 1 line
2.10.2
#37 lore raw

PATCH 13/14 mips: jz47xx: Add minimal JZ MMC node

In reply to Marek Vasut

Add minimal JZ MMC node into the JZ4780 device tree. This piece is picked from the CI20 Linux repository.

Cc Daniel Schwierzeck <[email protected]>
Cc Paul Burton <[email protected]>
---
 arch/mips/dts/jz4780.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
arch/mips/dts/jz4780.dtsi +20 -0
index b868b42..2b022c3 100644
@@ -44,6 +44,26 @@
 		#clock-cells = <1>;
 	};
 
+	msc0: msc at 13450000 {
+		compatible = "ingenic,jz4780-mmc";
+		reg = <0x13450000 0x1000>;
+
+		status = "disabled";
+
+		clocks = <&cgu JZ4780_CLK_MSC0>;
+		clock-names = "mmc";
+	};
+
+	msc1: msc at 13460000 {
+		compatible = "ingenic,jz4780-mmc";
+		reg = <0x13460000 0x1000>;
+
+		clocks = <&cgu JZ4780_CLK_MSC1>;
+		clock-names = "mmc";
+
+		status = "disabled";
+	};
+
 	uart0: serial at 10030000 {
 		compatible = "ingenic,jz4780-uart";
 		reg = <0x10030000 0x100>;
Signature · 1 line
2.10.2
#38 lore raw

PATCH 14/14 mips: jz47xx: Add Creator CI20 platform

In reply to Marek Vasut

From: Paul Burton <[email protected]>

Add support for the Creator CI20 platform based on the JZ4780 SoC. The DTS file comes from Linux 4.6 as of revision 78800558d104e003f9ae92e0107f1de39cf9de9f

So far, there are still a few details which will have to be fixed once they are fleshed out in Linux:
- pinmux: Thus far, this board just pokes the pinmux registers to set the pinmux. For MMC in SPL, this will have to stay. But for full u-boot a proper pinmux driver will have to be added once the pinmux semantics in DT are in mainline Linux.
- ethernet,efuse: DT bindings are missing from mainline Linux.

Cc Daniel Schwierzeck <[email protected]>
Cc Paul Burton <[email protected]>
---
 arch/mips/dts/Makefile        |   1 +
 arch/mips/dts/ci20.dts        | 114 ++++++++++++++
 arch/mips/mach-jz47xx/Kconfig |  11 ++
 board/imgtec/ci20/Kconfig     |  35 +++++
 board/imgtec/ci20/Makefile    |   5 +
 board/imgtec/ci20/README      |  10 ++
 board/imgtec/ci20/ci20.c      | 354 ++++++++++++++++++++++++++++++++++++++++++
 configs/ci20_defconfig        |  28 ++++
 include/configs/ci20.h        | 105 +++++++++++++
 9 files changed, 663 insertions(+)
 create mode 100644 arch/mips/dts/ci20.dts
 create mode 100644 board/imgtec/ci20/Kconfig
 create mode 100644 board/imgtec/ci20/Makefile
 create mode 100644 board/imgtec/ci20/README
 create mode 100644 board/imgtec/ci20/ci20.c
 create mode 100644 configs/ci20_defconfig
 create mode 100644 include/configs/ci20.h
arch/mips/dts/Makefile +1 -0
index 30fcc2b..e3473d9 100644
@@ -9,6 +9,7 @@ dtb-$(CONFIG_TARGET_MALTA) += mti,malta.dtb
 dtb-$(CONFIG_TARGET_PIC32MZDASK) += pic32mzda_sk.dtb
 dtb-$(CONFIG_TARGET_XILFPGA) += nexys4ddr.dtb
 dtb-$(CONFIG_BOARD_TPLINK_WDR4300) += tplink_wdr4300.dtb
+dtb-$(CONFIG_TARGET_JZ4780_CI20) += ci20.dtb
 
 targets += $(dtb-y)
 
arch/mips/dts/ci20.dts new file +114 -0

114 lines not on this page · raw message

arch/mips/mach-jz47xx/Kconfig +11 -0
index cd6944c..dcaac01 100644
@@ -12,4 +12,15 @@ config SOC_JZ4780
 	help
 	  Support for Ingenic JZ4780 family SoCs.
 
+choice
+	prompt "Board select"
+
+config TARGET_JZ4780_CI20
+	bool "Creator CI20 Reference Board"
+	select SOC_JZ4780
+
+endchoice
+
+source "board/imgtec/ci20/Kconfig"
+
 endmenu
board/imgtec/ci20/Kconfig new file +35 -0
new file mode 100644
index 0000000..5613df8
@@ -0,0 +1,35 @@
+if TARGET_JZ4780_CI20
+
+config SYS_BOARD
+	default "ci20"
+
+config SYS_VENDOR
+	default "imgtec"
+
+config SYS_CONFIG_NAME
+	default "ci20"
+
+config SYS_TEXT_BASE
+	default 0x80000000
+
+config JTAG
+	bool
+	default y
+
+config SYS_AUDIO_SPEED
+	int
+	default 768000000
+
+config SYS_EXTAL
+	int
+	default 48000000
+
+config SYS_MEM_DIV
+	int
+	default 3
+
+config SYS_MEM_SPEED
+	int
+	default 1200000000
+
+endif
board/imgtec/ci20/Makefile new file +5 -0
new file mode 100644
index 0000000..8c00081
@@ -0,0 +1,5 @@
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+
+obj-y := ci20.o
board/imgtec/ci20/README new file +10 -0
new file mode 100644
index 0000000..c757d41
@@ -0,0 +1,10 @@
+CI20 U-Boot
+
+Installation to an SD card:
+  Repartition your card with an MBR such that the first partition starts at an
+  offset of no less than 270KB. Then install U-Boot SPL & the full U-Boot image
+  to the card like so:
+
+    dd if=spl/u-boot-spl.bin of=/dev/sdX obs=512 seek=1
+    dd if=u-boot.img of=/dev/sdX obs=1K seek=14
+    sync
board/imgtec/ci20/ci20.c new file +354 -0

354 lines not on this page · raw message

configs/ci20_defconfig new file +28 -0
new file mode 100644
index 0000000..65bb2a0
@@ -0,0 +1,28 @@
+CONFIG_MIPS=y
+CONFIG_ARCH_JZ47XX=y
+CONFIG_TARGET_JZ4780_CI20=y
+CONFIG_DEFAULT_DEVICE_TREE="ci20"
+CONFIG_SPL=y
+CONFIG_SPL_MMC_TINY=y
+CONFIG_FIT=y
+CONFIG_HUSH_PARSER=y
+CONFIG_DISPLAY_BOARDINFO=y
+# CONFIG_CMD_IMLS is not set
+CONFIG_CMD_MMC=y
+CONFIG_DM_MMC=y
+# CONFIG_DM_MMC_OPS is not set
+# CONFIG_BLK is not set
+# CONFIG_SPL_DM_MMC is not set
+CONFIG_CMD_DHCP=y
+CONFIG_CMD_EXT4=y
+CONFIG_CMD_FAT=y
+CONFIG_JZ4780_EFUSE=y
+CONFIG_JZ47XX_GPIO=y
+CONFIG_JZ47XX_MMC=y
+CONFIG_USE_PRIVATE_LIBGCC=y
+CONFIG_SYS_NS16550=y
+CONFIG_DM_SERIAL=y
+CONFIG_SYS_DCACHE_SIZE=32768
+CONFIG_SYS_ICACHE_SIZE=32768
+CONFIG_SYS_DCACHE_LINE_SIZE=32
+CONFIG_SYS_ICACHE_LINE_SIZE=32
include/configs/ci20.h new file +105 -0

105 lines not on this page · raw message

Signature · 1 line
2.10.2
#39 lore raw

PATCH 14/14 mips: jz47xx: Add Creator CI20 platform

Quoted 15 lines · Marek Vasut

From: Paul Burton <[email protected]>

Add support for the Creator CI20 platform based on the JZ4780 SoC. The DTS file comes from Linux 4.6 as of revision 78800558d104e003f9ae92e0107f1de39cf9de9f

So far, there are still a few details which will have to be fixed once they are fleshed out in Linux:
- pinmux: Thus far, this board just pokes the pinmux registers to set the pinmux. For MMC in SPL, this will have to stay. But for full u-boot a proper pinmux driver will have to be added once the pinmux semantics in DT are in mainline Linux.
- ethernet,efuse: DT bindings are missing from mainline Linux.

Quoted 3 lines
Cc Daniel Schwierzeck <[email protected]>
Cc Paul Burton <[email protected]>

This needs to be rebased on top of master as there's a lot of config.h things that should be in the defconfig.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161126/65e7e3f0/attachment.sig>
#40 lore raw

PATCH 1/14 mips: Let cache.h be included from assembly source

In reply to Marek Vasut

Am 25.11.2016 um 23:32 schrieb Marek Vasut:

Quoted 10 lines

Add ifdef __ASSEMBLY__ around the function prototype to let cache.h be included from assembly code.

Cc Daniel Schwierzeck <[email protected]>
Cc Paul Burton <[email protected]>
---
 arch/mips/include/asm/cache.h | 2 ++
 1 file changed, 2 insertions(+)

applied to u-boot-mips, thanks!

Signature · 9 lines
- Daniel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161130/6eed4dac/attachment.sig>