dma: imx-sdma: Add ROM script addresses to driver

This adds the ROM script addresses to the SDMA driver needed for the driver to work without additional firmware.

The ROM script addresses are SoC specific and in some cases even tapeout specific. This makes it necessary to introduce SoC specific and tapeout specific bindings for the driver. While extending the binding also make the <chip_type> explicit in the binding documentation.

The non tapeout specific variants of the i.MX31/35 bindings are kept for compatibility.

Signed-off-by Sascha Hauer <[email protected]>

---
 .../devicetree/bindings/dma/fsl-imx-sdma.txt       |   7 +-
 drivers/dma/imx-sdma.c                             | 180 ++++++++++++++++++++-
 2 files changed, 185 insertions(+), 2 deletions(-)

Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt +6 -1

index 68cee4f5..4fa814d 100644
@@ -1,7 +1,12 @@
 * Freescale Smart Direct Memory Access (SDMA) Controller for i.MX
 
 Required properties:
-- compatible : Should be "fsl,<chip>-sdma"
+- compatible : Should be "fsl,imx31-sdma", "fsl,imx31-to1-sdma",
+  "fsl,imx31-to2-sdma", "fsl,imx35-sdma", "fsl,imx35-to1-sdma",
+  "fsl,imx35-to2-sdma", "fsl,imx51-sdma", "fsl,imx53-sdma" or
+  "fsl,imx6q-sdma". The -to variants should be preferred since they
+  allow to determnine the correct ROM script addresses needed for
+  the driver to work without additional firmware.
 - reg : Should contain SDMA registers location and length
 - interrupts : Should contain SDMA interrupt
 - #dma-cells : Must be <3>.

drivers/dma/imx-sdma.c +169 -0

index e3e7e3f..42142b7 100644
@@ -310,6 +310,7 @@ struct sdma_firmware_header {
 struct sdma_driver_data {
 	int chnenbl0;
 	int num_events;
+	struct sdma_script_start_addrs	*script_addrs;
 };
 
 struct sdma_engine {

@@ -328,23 +329,191 @@ struct sdma_engine {
 	const struct sdma_driver_data	*drvdata;
 };
 
+static struct sdma_script_start_addrs sdma_script_imx31_to1 = {
+	.per_2_per_addr = 1677,
+};
+
+static struct sdma_script_start_addrs sdma_script_imx31_to2 = {
+	.ap_2_ap_addr = 423,
+	.ap_2_bp_addr = 829,
+	.bp_2_ap_addr = 1029,
+};
+
 struct sdma_driver_data sdma_imx31 = {
 	.chnenbl0 = SDMA_CHNENBL0_IMX31,
 	.num_events = 32,
 };
 
+struct sdma_driver_data sdma_imx31_to1 = {
+	.chnenbl0 = SDMA_CHNENBL0_IMX31,
+	.num_events = 32,
+	.script_addrs = &sdma_script_imx31_to1,
+};
+
+struct sdma_driver_data sdma_imx31_to2 = {
+	.chnenbl0 = SDMA_CHNENBL0_IMX31,
+	.num_events = 32,
+	.script_addrs = &sdma_script_imx31_to2,
+};
+
+static struct sdma_script_start_addrs sdma_script_imx25 = {
+	.ap_2_ap_addr = 729,
+	.uart_2_mcu_addr = 904,
+	.per_2_app_addr = 1255,
+	.mcu_2_app_addr = 834,
+	.uartsh_2_mcu_addr = 1120,
+	.per_2_shp_addr = 1329,
+	.mcu_2_shp_addr = 1048,
+	.ata_2_mcu_addr = 1560,
+	.mcu_2_ata_addr = 1479,
+	.app_2_per_addr = 1189,
+	.app_2_mcu_addr = 770,
+	.shp_2_per_addr = 1407,
+	.shp_2_mcu_addr = 979,
+};
+
+struct sdma_driver_data sdma_imx25 = {
+	.chnenbl0 = SDMA_CHNENBL0_IMX35,
+	.num_events = 48,
+	.script_addrs = &sdma_script_imx25,
+};
+
+static struct sdma_script_start_addrs sdma_script_imx35_to1 = {
+	.ap_2_ap_addr = 642,
+	.uart_2_mcu_addr = 817,
+	.mcu_2_app_addr = 747,
+	.uartsh_2_mcu_addr = 1183,
+	.per_2_shp_addr = 1033,
+	.mcu_2_shp_addr = 961,
+	.ata_2_mcu_addr = 1333,
+	.mcu_2_ata_addr = 1252,
+	.app_2_mcu_addr = 683,
+	.shp_2_per_addr = 1111,
+	.shp_2_mcu_addr = 892,
+};
+
+static struct sdma_script_start_addrs sdma_script_imx35_to2 = {
+	.ap_2_ap_addr = 729,
+	.uart_2_mcu_addr = 904,
+	.per_2_app_addr = 1597,
+	.mcu_2_app_addr = 834,
+	.uartsh_2_mcu_addr = 1270,
+	.per_2_shp_addr = 1120,
+	.mcu_2_shp_addr = 1048,
+	.ata_2_mcu_addr = 1429,
+	.mcu_2_ata_addr = 1339,
+	.app_2_per_addr = 1531,
+	.app_2_mcu_addr = 770,
+	.shp_2_per_addr = 1198,
+	.shp_2_mcu_addr = 979,
+};
+
 struct sdma_driver_data sdma_imx35 = {
 	.chnenbl0 = SDMA_CHNENBL0_IMX35,
 	.num_events = 48,
 };
 
+struct sdma_driver_data sdma_imx35_to1 = {
+	.chnenbl0 = SDMA_CHNENBL0_IMX35,
+	.num_events = 48,
+	.script_addrs = &sdma_script_imx35_to1,
+};
+
+struct sdma_driver_data sdma_imx35_to2 = {
+	.chnenbl0 = SDMA_CHNENBL0_IMX35,
+	.num_events = 48,
+	.script_addrs = &sdma_script_imx35_to2,
+};
+
+static struct sdma_script_start_addrs sdma_script_imx51 = {
+	.ap_2_ap_addr = 642,
+	.uart_2_mcu_addr = 817,
+	.mcu_2_app_addr = 747,
+	.mcu_2_shp_addr = 961,
+	.ata_2_mcu_addr = 1473,
+	.mcu_2_ata_addr = 1392,
+	.app_2_per_addr = 1033,
+	.app_2_mcu_addr = 683,
+	.shp_2_per_addr = 1251,
+	.shp_2_mcu_addr = 892,
+};
+
+struct sdma_driver_data sdma_imx51 = {
+	.chnenbl0 = SDMA_CHNENBL0_IMX35,
+	.num_events = 48,
+	.script_addrs = &sdma_script_imx51,
+};
+
+static struct sdma_script_start_addrs sdma_script_imx53 = {
+	.ap_2_ap_addr = 642,
+	.app_2_mcu_addr = 683,
+	.mcu_2_app_addr = 747,
+	.uart_2_mcu_addr = 817,
+	.shp_2_mcu_addr = 891,
+	.mcu_2_shp_addr = 960,
+	.uartsh_2_mcu_addr = 1032,
+	.spdif_2_mcu_addr = 1100,
+	.mcu_2_spdif_addr = 1134,
+	.firi_2_mcu_addr = 1193,
+	.mcu_2_firi_addr = 1290,
+};
+
+struct sdma_driver_data sdma_imx53 = {
+	.chnenbl0 = SDMA_CHNENBL0_IMX35,
+	.num_events = 48,
+	.script_addrs = &sdma_script_imx53,
+};
+
+static struct sdma_script_start_addrs sdma_script_imx6q = {
+	.ap_2_ap_addr = 642,
+	.uart_2_mcu_addr = 817,
+	.mcu_2_app_addr = 747,
+	.per_2_per_addr = 6331,
+	.uartsh_2_mcu_addr = 1032,
+	.mcu_2_shp_addr = 960,
+	.app_2_mcu_addr = 683,
+	.shp_2_mcu_addr = 891,
+	.spdif_2_mcu_addr = 1100,
+	.mcu_2_spdif_addr = 1134,
+};
+
+struct sdma_driver_data sdma_imx6q = {
+	.chnenbl0 = SDMA_CHNENBL0_IMX35,
+	.num_events = 48,
+	.script_addrs = &sdma_script_imx6q,
+};
+
 static struct platform_device_id sdma_devtypes[] = {
 	{
+		.name = "imx25-sdma",
+		.driver_data = (unsigned long)&sdma_imx25,
+	}, {
 		.name = "imx31-sdma",
 		.driver_data = (unsigned long)&sdma_imx31,
+	},  {
+		.name = "imx31-to1-sdma",
+		.driver_data = (unsigned long)&sdma_imx31_to1,
+	},  {
+		.name = "imx31-to2-sdma",
+		.driver_data = (unsigned long)&sdma_imx31_to2,
 	}, {
 		.name = "imx35-sdma",
 		.driver_data = (unsigned long)&sdma_imx35,
+	},  {
+		.name = "imx35-to1-sdma",
+		.driver_data = (unsigned long)&sdma_imx35_to1,
+	},  {
+		.name = "imx35-to2-sdma",
+		.driver_data = (unsigned long)&sdma_imx35_to2,
+	}, {
+		.name = "imx51-sdma",
+		.driver_data = (unsigned long)&sdma_imx51,
+	}, {
+		.name = "imx53-sdma",
+		.driver_data = (unsigned long)&sdma_imx53,
+	}, {
+		.name = "imx6q-sdma",
+		.driver_data = (unsigned long)&sdma_imx6q,
 	}, {
 		/* sentinel */
 	}

drivers/dma/imx-sdma.c +5 -0

+	{ .compatible = "fsl,imx31-to2-sdma", .data = &sdma_imx31_to2, },
+	{ .compatible = "fsl,imx31-to1-sdma", .data = &sdma_imx31_to1, },
+	{ .compatible = "fsl,imx31-sdma", .data = &sdma_imx31, },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, sdma_dt_ids);

@@ -1434,6 +1610,8 @@ static int __init sdma_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_init;
 
+	if (sdma->drvdata->script_addrs)
+		sdma_add_scripts(sdma, sdma->drvdata->script_addrs);
 	if (pdata && pdata->script_addrs)
 		sdma_add_scripts(sdma, pdata->script_addrs);
 

Signature · 1 line

1.8.4.rc3

Copy devicetree mailing list.

drivers/dma/imx-sdma.c static struct platform_device_id sdma_devtypes[] = { · line 529

Quoted 11 lines

drivers/dma/imx-sdma.c +5 -1

@@ -352,8 +521,15 @@ static struct platform_device_id sdma_devtypes[] = {
 MODULE_DEVICE_TABLE(platform, sdma_devtypes);
 
 static const struct of_device_id sdma_dt_ids[] = {
-	{ .compatible = "fsl,imx31-sdma", .data = &sdma_imx31, },
+	{ .compatible = "fsl,imx6q-sdma", .data = &sdma_imx6q, },
+	{ .compatible = "fsl,imx53-sdma", .data = &sdma_imx53, },
+	{ .compatible = "fsl,imx51-sdma", .data = &sdma_imx51, },
+	{ .compatible = "fsl,imx35-to2-sdma", .data = &sdma_imx35_to2, },
+	{ .compatible = "fsl,imx35-to1-sdma", .data = &sdma_imx35_to1, },
 	{ .compatible = "fsl,imx35-sdma", .data = &sdma_imx35, },

Do we need to bind the compatible with sdma_imx35_to2 like below?

{ .compatible = "fsl,imx35-sdma", .data = &sdma_imx35_to2, },

Otherwise, we will need to patch sdma_imx35 with proper .script_addrs assignment?

Also, I do not know how we will play these tapeout specific compatible in device tree. As you know, we currently do not have tapeout but only chip specific DTS, since we expect kernel will figure out the tapeout/revision and handle the differences accordingly.

Quoted 13 lines · Shawn Guo

Copy devicetree mailing list.

Quoted 11 lines

drivers/dma/imx-sdma.c +5 -1

@@ -352,8 +521,15 @@ static struct platform_device_id sdma_devtypes[] = {
 MODULE_DEVICE_TABLE(platform, sdma_devtypes);
 
 static const struct of_device_id sdma_dt_ids[] = {
-	{ .compatible = "fsl,imx31-sdma", .data = &sdma_imx31, },
+	{ .compatible = "fsl,imx6q-sdma", .data = &sdma_imx6q, },
+	{ .compatible = "fsl,imx53-sdma", .data = &sdma_imx53, },
+	{ .compatible = "fsl,imx51-sdma", .data = &sdma_imx51, },
+	{ .compatible = "fsl,imx35-to2-sdma", .data = &sdma_imx35_to2, },
+	{ .compatible = "fsl,imx35-to1-sdma", .data = &sdma_imx35_to1, },
 	{ .compatible = "fsl,imx35-sdma", .data = &sdma_imx35, },

Quoted 4 lines

Also, I do not know how we will play these tapeout specific compatible in device tree. As you know, we currently do not have tapeout but only chip specific DTS, since we expect kernel will figure out the tapeout/revision and handle the differences accordingly.

In fact the kernel does figure out the tapeout version, but I don't have access to it in the driver.

Currently we have a problem anyway. We have the firmware names in the dts, but the firmware is tapeout specific for i.MX31/35.

So I think our options are:

- remove the tapeout specifics from the devicetrees, this would mean to also remove the firmware names.
- keep the tapeout specifics in the devicetree, then we could even add some more like I did ;)

I don't know what's the best way to proceed. For now I could remove the tapeout specific bindings, this way we would get the ROM script addresses at least for i.MX25/5/6.

Quoted 14 lines · Sascha Hauer

Quoted 13 lines · Shawn Guo

Copy devicetree mailing list.

Quoted 11 lines

drivers/dma/imx-sdma.c +5 -1

@@ -352,8 +521,15 @@ static struct platform_device_id sdma_devtypes[] = {
 MODULE_DEVICE_TABLE(platform, sdma_devtypes);
 
 static const struct of_device_id sdma_dt_ids[] = {
-	{ .compatible = "fsl,imx31-sdma", .data = &sdma_imx31, },
+	{ .compatible = "fsl,imx6q-sdma", .data = &sdma_imx6q, },
+	{ .compatible = "fsl,imx53-sdma", .data = &sdma_imx53, },
+	{ .compatible = "fsl,imx51-sdma", .data = &sdma_imx51, },
+	{ .compatible = "fsl,imx35-to2-sdma", .data = &sdma_imx35_to2, },
+	{ .compatible = "fsl,imx35-to1-sdma", .data = &sdma_imx35_to1, },
 	{ .compatible = "fsl,imx35-sdma", .data = &sdma_imx35, },

<snip>

Quoted 7 lines

Quoted 4 lines

Also, I do not know how we will play these tapeout specific compatible in device tree. As you know, we currently do not have tapeout but only chip specific DTS, since we expect kernel will figure out the tapeout/revision and handle the differences accordingly.

In fact the kernel does figure out the tapeout version, but I don't have access to it in the driver.

Yeah. But since we're using SoC for IP versioning and the version could vary even for same SoC between different revisions, I think we will see such need of accessing SoC revision from device driver more and more. Thus, we will need a generic approach for that sooner or later, I guess.

One approach I can think of is that we define a property "revision" under node "soc", and ask platform code to fill this property by calling of_update_property(). Then drivers can simply get it via device tree /soc/revision.

Quoted 11 lines

Currently we have a problem anyway. We have the firmware names in the dts, but the firmware is tapeout specific for i.MX31/35.

So I think our options are:

- remove the tapeout specifics from the devicetrees, this would mean to also remove the firmware names.
- keep the tapeout specifics in the devicetree, then we could even add some more like I did ;)

If I understand this option correctly, we will need something like imx35-to1.dtsi and imx35-to2.dtsi, and board level dts authors need to include the correct one per the chip revision on their boards. What about some board that solder both revisions, and socket board that can change chips? I do not think it's going to work.

The following series adds the ROM script addresses to the driver. These have been provided by platform_data for platform based systems, but with devicetree the driver no longer supports the ROM script addresses. This means that the sdma driver currently needs a firmware to work. By adding the script addresses to the driver it can work without additional firmware.

Sascha

----------------------------------------------------------------
Sascha Hauer (3):
dma: imx-sdma: Use struct for driver data
dma: imx-sdma: Add ROM script addresses to driver
ARM: i.MX: remove sdma script address arrays from platform data

 .../devicetree/bindings/dma/fsl-imx-sdma.txt       |   7 +-
 arch/arm/mach-imx/mm-imx25.c                       |  17 --
 arch/arm/mach-imx/mm-imx3.c                        |  67 ++----
 arch/arm/mach-imx/mm-imx5.c                        |  14 --
 drivers/dma/imx-sdma.c                             | 241 ++++++++++++++++++---
 5 files changed, 230 insertions(+), 116 deletions(-)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Please could you describe how the compatible property allows matching and covers both the ROM version *and* the distributed firmware file that is also referenced in the device tree for each sdma node for each SoC supported?

As far as I recall, the offsets for the (potentially buggy) on-chip version and the one in the BSP (and hopefully in upstream kernels at some soon-approaching version) are not the same for a good swath of these chips.

How would the driver hope to match this up without causing some DMA explosions and/or invalid device trees if the BSP firmware is described, the BSP firmware file is not found, and the driver is left with the ROM version?

You know what, I am really confused by this in the first place. Since when did the SDMA driver actually support ROM addresses in device tree anyway (referring your comments in the patch series) and when did it get removed? Why is platform data somehow now required in a driver and not being put in it's correct place in the device tree?

Why require a firmware file for anything here, bugs notwithstanding? As far as I recall there were one or two in older tape-out i.MX6 (uart dma I think) and a few more in i.MX51 and i.MX53 early tapeouts. I was sure I saw some BSP commits that update the firmware to fix some bugs in production tape-outs (oddly they're seemingly not in the errata for any tape-out?). The ROM addresses should be in DT, and for those devices that now have platform_data glue hardcoding the vaues, they'll work. DT platforms won't.

I don't understand here how it's not possible to specify the ROM addresses in the DT for instance using the way regulators are handled (but I'd want to see reg used to specify the address and name instead of a custom properties in this case).

What I'd really like to see is the "waiting on a firmware that doesn't exist when there is a working ROM version of those scripts in the SoC"

bug fixed properly, by using ROM addresses in DT - specifying the "wrong" addresses is a device tree bug for that board with that tape-out or users using the wrong device-tree. Then fudging platform_data for non-DT platforms makes sense in that they have no DTs. And when they are converted they will follow suit anyway - there's a path to take to get there that is really clear.

In this case it is just hardcoding one or two SoC and then leaving the rest tha tare actually device-tree compliant to use that abominable, OS-specific firmware path name property.. keeping the hangs. I'm really trying to figure out the intent and design process here, because from a perspective of getting more device tree support this really is stepping backwards via a hack to get a couple boards to work.

Quoted 3 lines

What I'd really like to see is the "waiting on a firmware that doesn't exist when there is a working ROM version of those scripts in the SoC"

Then this patch is for you. This is exactly what the patch solves.

Quoted 5 lines · Sascha Hauer

Quoted 3 lines

What I'd really like to see is the "waiting on a firmware that doesn't exist when there is a working ROM version of those scripts in the SoC"

Then this patch is for you. This is exactly what the patch solves.

Only by changing the device tree and not specifying a firmware filename. The driver probes, knows about ROM script addresses as per your patch, tries to load a new file from disk; same hang. What you have now is a way to use a potentially buggy script with several functions missing (ROM script isn't as full-featured as the file-based one on some chips) or a hang on boot if you misplaced a firmware which is notoriously hard to get hold of..

How do we resolve this except to load the firmware pre-Linux (or put the firmware in the tree, argh) and describe the loaded firmware and offsets? Once it's in the internal SDMA code RAM all the versioning and headers go away..