Skip to contentSkip to conversation

ARM: pxa: ssp: prepare for devicetree usage

PATCH New Daniel Mack 3 participants 15 messages Aug 12, 2013 – Aug 14, 2013

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

cover ARM: pxa: ssp: prepare for devicetree usage

Hi Haojian,

this is an excerpt of my pxa-dma patches that can be merged independantly.

In contrast to what I already posted, this one contains a patch to make ssp ports lookups from device nodes possible. As described in the Documentation, users of ssp ports just reference the upstream by phandle.

The rest of the dma conversion will take more time anyway, so let's better merge it step by step.

Thanks,
Daniel

Daniel Mack (5):
ARM: pxa: ssp: remove unnecessary warning on kzalloc() failure
ARM: pxa: ssp: add shortcut for &pdev->dev
ARM: pxa: ssp: add DT bindings
ARM: pxa: ssp: use devm_ functions
ARM: pxa: ssp: add pxa_ssp_request_of()

 .../devicetree/bindings/serial/mrvl,pxa-ssp.txt    |  65 ++++++++
 arch/arm/plat-pxa/ssp.c                            | 174 ++++++++++++++-------
 include/linux/pxa2xx_ssp.h                         |  11 ++
 3 files changed, 193 insertions(+), 57 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/serial/mrvl,pxa-ssp.txt
Signature · 1 line
1.8.3.1
#1 lore raw

PATCH 1/5 ARM: pxa: ssp: remove unnecessary warning on kzalloc() failure

The memory subsystem will already complain loudly enough in such cases.

---
 arch/arm/plat-pxa/ssp.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
arch/arm/plat-pxa/ssp.c +2 -3
index 8e11e96..f746b6a 100644
@@ -80,10 +80,9 @@ static int pxa_ssp_probe(struct platform_device *pdev)
 	int ret = 0;
 
 	ssp = kzalloc(sizeof(struct ssp_device), GFP_KERNEL);
-	if (ssp == NULL) {
-		dev_err(&pdev->dev, "failed to allocate memory");
+	if (ssp == NULL)
 		return -ENOMEM;
-	}
+
 	ssp->pdev = pdev;
 
 	ssp->clk = clk_get(&pdev->dev, NULL);
Signature · 1 line
1.8.3.1
#2 lore raw

PATCH 1/5 ARM: pxa: ssp: remove unnecessary warning on kzalloc() failure

Quoted 1 line · Daniel Mack

The memory subsystem will already complain loudly enough in such cases.

Applied, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130812/b025b3ca/attachment.sig>

#3 lore raw

PATCH 2/5 ARM: pxa: ssp: add shortcut for &pdev->dev

In reply to Daniel Mack

No functional change, just a cosmetic cleanup.

---
 arch/arm/plat-pxa/ssp.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
arch/arm/plat-pxa/ssp.c +8 -7
index f746b6a..65ba28a 100644
@@ -77,6 +77,7 @@ static int pxa_ssp_probe(struct platform_device *pdev)
 	const struct platform_device_id *id = platform_get_device_id(pdev);
 	struct resource *res;
 	struct ssp_device *ssp;
+	struct device *dev = &pdev->dev;
 	int ret = 0;
 
 	ssp = kzalloc(sizeof(struct ssp_device), GFP_KERNEL);
@@ -85,7 +86,7 @@ static int pxa_ssp_probe(struct platform_device *pdev)
 
 	ssp->pdev = pdev;
 
-	ssp->clk = clk_get(&pdev->dev, NULL);
+	ssp->clk = clk_get(dev, NULL);
 	if (IS_ERR(ssp->clk)) {
 		ret = PTR_ERR(ssp->clk);
 		goto err_free;
@@ -93,7 +94,7 @@ static int pxa_ssp_probe(struct platform_device *pdev)
 
 	res = platform_get_resource(pdev, IORESOURCE_DMA, 0);
 	if (res == NULL) {
-		dev_err(&pdev->dev, "no SSP RX DRCMR defined\n");
+		dev_err(dev, "no SSP RX DRCMR defined\n");
 		ret = -ENODEV;
 		goto err_free_clk;
 	}
@@ -101,7 +102,7 @@ static int pxa_ssp_probe(struct platform_device *pdev)
 
 	res = platform_get_resource(pdev, IORESOURCE_DMA, 1);
 	if (res == NULL) {
-		dev_err(&pdev->dev, "no SSP TX DRCMR defined\n");
+		dev_err(dev, "no SSP TX DRCMR defined\n");
 		ret = -ENODEV;
 		goto err_free_clk;
 	}
@@ -109,7 +110,7 @@ static int pxa_ssp_probe(struct platform_device *pdev)
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res == NULL) {
-		dev_err(&pdev->dev, "no memory resource defined\n");
+		dev_err(dev, "no memory resource defined\n");
 		ret = -ENODEV;
 		goto err_free_clk;
 	}
@@ -117,7 +118,7 @@ static int pxa_ssp_probe(struct platform_device *pdev)
 	res = request_mem_region(res->start, resource_size(res),
 			pdev->name);
 	if (res == NULL) {
-		dev_err(&pdev->dev, "failed to request memory resource\n");
+		dev_err(dev, "failed to request memory resource\n");
 		ret = -EBUSY;
 		goto err_free_clk;
 	}
@@ -126,14 +127,14 @@ static int pxa_ssp_probe(struct platform_device *pdev)
 
 	ssp->mmio_base = ioremap(res->start, resource_size(res));
 	if (ssp->mmio_base == NULL) {
-		dev_err(&pdev->dev, "failed to ioremap() registers\n");
+		dev_err(dev, "failed to ioremap() registers\n");
 		ret = -ENODEV;
 		goto err_free_mem;
 	}
 
 	ssp->irq = platform_get_irq(pdev, 0);
 	if (ssp->irq < 0) {
-		dev_err(&pdev->dev, "no IRQ resource defined\n");
+		dev_err(dev, "no IRQ resource defined\n");
 		ret = -ENODEV;
 		goto err_free_io;
 	}
Signature · 1 line
1.8.3.1
#5 lore raw

PATCH 2/5 ARM: pxa: ssp: add shortcut for &pdev->dev

Quoted 5 lines · Mark Brown
Quoted 1 line · Daniel Mack

No functional change, just a cosmetic cleanup.

Applied, thanks.

Which tree will you merge them through? ASoC?

I actually thought Haojian would pick them, but I see there's a build dependency. So it might make sense if you take them instead.

Daniel

#6 lore raw

PATCH 2/5 ARM: pxa: ssp: add shortcut for &pdev->dev

Quoted 1 line · Daniel Mack

Which tree will you merge them through? ASoC?

Yes.

Quoted 2 lines

I actually thought Haojian would pick them, but I see there's a build dependency. So it might make sense if you take them instead.

They didn't obviously touch arch/arm so I'd expect them to go through
the relevant driver tree.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130812/12b9a500/attachment.sig>

#7 lore raw

PATCH 2/5 ARM: pxa: ssp: add shortcut for &pdev->dev

Quoted 5 lines · Mark Brown
Quoted 2 lines · Daniel Mack

I actually thought Haojian would pick them, but I see there's a build dependency. So it might make sense if you take them instead.

They didn't obviously touch arch/arm

Well yes, *this* series does:

 .../devicetree/bindings/serial/mrvl,pxa-ssp.txt    |  65 ++++++++
 arch/arm/plat-pxa/ssp.c                            | 174
++++++++++++++-------
 include/linux/pxa2xx_ssp.h                         |  11 ++

I posted two series, one is for arch/arm and one is for sound/soc/pxa. But as they have a dependency via pxa_ssp_request_of(), I'd actually prefer if you took all of them.

Thanks,
Daniel

#8 lore raw

PATCH 2/5 ARM: pxa: ssp: add shortcut for &pdev->dev

Quoted 9 lines · Daniel Mack
Quoted 5 lines · Mark Brown
Quoted 2 lines · Daniel Mack

I actually thought Haojian would pick them, but I see there's a build dependency. So it might make sense if you take them instead.

They didn't obviously touch arch/arm

Well yes, *this* series does:

Quoted 8 lines
 .../devicetree/bindings/serial/mrvl,pxa-ssp.txt    |  65 ++++++++
 arch/arm/plat-pxa/ssp.c                            | 174
++++++++++++++-------
 include/linux/pxa2xx_ssp.h                         |  11 ++

I posted two series, one is for arch/arm and one is for sound/soc/pxa. But as they have a dependency via pxa_ssp_request_of(), I'd actually prefer if you took all of them.

Haojian, could you give your Ack on this procedure?

Thanks,
Daniel

#9 lore raw

PATCH 2/5 ARM: pxa: ssp: add shortcut for &pdev->dev

Quoted 19 lines · Daniel Mack
Quoted 9 lines · Daniel Mack
Quoted 5 lines · Mark Brown
Quoted 2 lines · Daniel Mack

I actually thought Haojian would pick them, but I see there's a build dependency. So it might make sense if you take them instead.

They didn't obviously touch arch/arm

Well yes, *this* series does:

Quoted 8 lines
 .../devicetree/bindings/serial/mrvl,pxa-ssp.txt    |  65 ++++++++
 arch/arm/plat-pxa/ssp.c                            | 174
++++++++++++++-------
 include/linux/pxa2xx_ssp.h                         |  11 ++

I posted two series, one is for arch/arm and one is for sound/soc/pxa. But as they have a dependency via pxa_ssp_request_of(), I'd actually prefer if you took all of them.

Quoted 7 lines

Haojian, could you give your Ack on this procedure?

Thanks,
Daniel

#11 lore raw

PATCH 2/5 ARM: pxa: ssp: add shortcut for &pdev->dev

Quoted 6 lines · Mark Brown
Quoted 1 line · Haojian Zhuang

Applied, thanks.

Great, thanks. You'll take the pxa/ssp series for ASoC as well then, right?

Daniel

#12 lore raw

PATCH 3/5 ARM: pxa: ssp: add DT bindings

In reply to Daniel Mack

This patch contains an ugly hack for looking up the the DMA request number. The problem here is that the implementation as it stands will allocate the DMA channel from the user of the ssp port, and hence we cannot allocate a real channel here.

---
 .../devicetree/bindings/serial/mrvl,pxa-ssp.txt    | 65 ++++++++++++++++
 arch/arm/plat-pxa/ssp.c                            | 90 +++++++++++++++++-----
 2 files changed, 134 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/serial/mrvl,pxa-ssp.txt
Documentation/devicetree/bindings/serial/mrvl,pxa-ssp.txt new file +65 -0
new file mode 100644
index 0000000..669b814
@@ -0,0 +1,65 @@
+Device tree bindings for Marvell PXA SSP ports
+
+Required properties:
+
+	- compatible:	Must be one of
+				mrvl,pxa25x-ssp
+				mvrl,pxa25x-nssp
+				mrvl,pxa27x-ssp
+				mrvl,pxa3xx-ssp
+				mvrl,pxa168-ssp
+				mrvl,pxa910-ssp
+				mrvl,ce4100-ssp
+				mrvl,lpss-ssp
+
+	- reg:		The memory base
+	- dmas:		Two dma phandles, one for rx, one for tx
+	- dma-names:	Must be "rx", "tx"
+
+
+Example for PXA3xx:
+
+	ssp0: ssp at 41000000 {
+		compatible = "mrvl,pxa3xx-ssp";
+		reg = <0x41000000 0x40>;
+		ssp-id = <1>;
+		interrupts = <24>;
+		clock-names = "pxa27x-ssp.0";
+		dmas = <&dma 13
+			&dma 14>;
+		dma-names = "rx", "tx";
+	};
+
+	ssp1: ssp at 41700000 {
+		compatible = "mrvl,pxa3xx-ssp";
+		reg = <0x41700000 0x40>;
+		ssp-id = <2>;
+		interrupts = <16>;
+		clock-names = "pxa27x-ssp.1";
+		dmas = <&dma 15
+			&dma 16>;
+		dma-names = "rx", "tx";
+	};
+
+	ssp2: ssp at 41900000 {
+		compatibl3 = "mrvl,pxa3xx-ssp";
+		reg = <0x41900000 0x40>;
+		ssp-id = <3>;
+		interrupts = <0>;
+		clock-names = "pxa27x-ssp.2";
+		dmas = <&dma 66
+			&dma 67>;
+		dma-names = "rx", "tx";
+	};
+
+	ssp3: ssp at 41a00000 {
+		compatible = "mrvl,pxa3xx-ssp";
+		reg = <0x41a00000 0x40>;
+		ssp-id = <4>;
+		interrupts = <13>;
+		clock-names = "pxa27x-ssp.3";
+		dmas = <&dma 2
+			&dma 3>;
+		dma-names = "rx", "tx";
+	};
+
arch/arm/plat-pxa/ssp.c +69 -21

125 lines not on this page · raw message

Signature · 1 line
1.8.3.1
#13 lore raw

PATCH 4/5 ARM: pxa: ssp: use devm_ functions

In reply to Daniel Mack

Use devm_ functions to allocate memory, ioremap, clk_get etc to clean up the error unwind path.

---
 arch/arm/plat-pxa/ssp.c | 40 ++++++++++++----------------------------
 1 file changed, 12 insertions(+), 28 deletions(-)
arch/arm/plat-pxa/ssp.c +12 -28
index c3afcec..f266135 100644
@@ -94,19 +94,16 @@ static int pxa_ssp_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct ssp_device *ssp;
 	struct device *dev = &pdev->dev;
-	int ret = 0;
 
-	ssp = kzalloc(sizeof(struct ssp_device), GFP_KERNEL);
+	ssp = devm_kzalloc(dev, sizeof(struct ssp_device), GFP_KERNEL);
 	if (ssp == NULL)
 		return -ENOMEM;
 
 	ssp->pdev = pdev;
 
-	ssp->clk = clk_get(dev, NULL);
-	if (IS_ERR(ssp->clk)) {
-		ret = PTR_ERR(ssp->clk);
-		goto err_free;
-	}
+	ssp->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(ssp->clk))
+		return PTR_ERR(ssp->clk);
 
 	if (dev->of_node) {
 		struct of_phandle_args dma_spec;
@@ -148,32 +145,28 @@ static int pxa_ssp_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res == NULL) {
 		dev_err(dev, "no memory resource defined\n");
-		ret = -ENODEV;
-		goto err_free_clk;
+		return -ENODEV;
 	}
 
-	res = request_mem_region(res->start, resource_size(res),
-			pdev->name);
+	res = devm_request_mem_region(dev, res->start, resource_size(res),
+				      pdev->name);
 	if (res == NULL) {
 		dev_err(dev, "failed to request memory resource\n");
-		ret = -EBUSY;
-		goto err_free_clk;
+		return -EBUSY;
 	}
 
 	ssp->phys_base = res->start;
 
-	ssp->mmio_base = ioremap(res->start, resource_size(res));
+	ssp->mmio_base = devm_ioremap(dev, res->start, resource_size(res));
 	if (ssp->mmio_base == NULL) {
 		dev_err(dev, "failed to ioremap() registers\n");
-		ret = -ENODEV;
-		goto err_free_mem;
+		return -ENODEV;
 	}
 
 	ssp->irq = platform_get_irq(pdev, 0);
 	if (ssp->irq < 0) {
 		dev_err(dev, "no IRQ resource defined\n");
-		ret = -ENODEV;
-		goto err_free_io;
+		return -ENODEV;
 	}
 
 	if (dev->of_node) {
@@ -198,17 +191,8 @@ static int pxa_ssp_probe(struct platform_device *pdev)
 	mutex_unlock(&ssp_lock);
 
 	platform_set_drvdata(pdev, ssp);
-	return 0;
 
-err_free_io:
-	iounmap(ssp->mmio_base);
-err_free_mem:
-	release_mem_region(res->start, resource_size(res));
-err_free_clk:
-	clk_put(ssp->clk);
-err_free:
-	kfree(ssp);
-	return ret;
+	return 0;
 }
 
 static int pxa_ssp_remove(struct platform_device *pdev)
Signature · 1 line
1.8.3.1
#14 lore raw

PATCH 5/5 ARM: pxa: ssp: add pxa_ssp_request_of()

In reply to Daniel Mack

Add a function to lookup ssp devices from device tree. This way, users can reference the ssp devices in order to register to them.

---
 arch/arm/plat-pxa/ssp.c    | 25 +++++++++++++++++++++++++
 include/linux/pxa2xx_ssp.h | 11 +++++++++++
 2 files changed, 36 insertions(+)
arch/arm/plat-pxa/ssp.c +25 -0
index f266135..c83f27b 100644
@@ -62,6 +62,30 @@ struct ssp_device *pxa_ssp_request(int port, const char *label)
 }
 EXPORT_SYMBOL(pxa_ssp_request);
 
+struct ssp_device *pxa_ssp_request_of(const struct device_node *of_node,
+				      const char *label)
+{
+	struct ssp_device *ssp = NULL;
+
+	mutex_lock(&ssp_lock);
+
+	list_for_each_entry(ssp, &ssp_list, node) {
+		if (ssp->of_node == of_node && ssp->use_count == 0) {
+			ssp->use_count++;
+			ssp->label = label;
+			break;
+		}
+	}
+
+	mutex_unlock(&ssp_lock);
+
+	if (&ssp->node == &ssp_list)
+		return NULL;
+
+	return ssp;
+}
+EXPORT_SYMBOL(pxa_ssp_request_of);
+
 void pxa_ssp_free(struct ssp_device *ssp)
 {
 	mutex_lock(&ssp_lock);
@@ -185,6 +209,7 @@ static int pxa_ssp_probe(struct platform_device *pdev)
 	}
 
 	ssp->use_count = 0;
+	ssp->of_node = dev->of_node;
 
 	mutex_lock(&ssp_lock);
 	list_add(&ssp->node, &ssp_list);
include/linux/pxa2xx_ssp.h +11 -0
index 467cc63..4944420 100644
@@ -21,6 +21,8 @@
 
 #include <linux/list.h>
 #include <linux/io.h>
+#include <linux/of.h>
+
 
 /*
  * SSP Serial Port Registers
@@ -190,6 +192,8 @@ struct ssp_device {
 	int		irq;
 	int		drcmr_rx;
 	int		drcmr_tx;
+
+	struct device_node	*of_node;
 };
 
 /**
@@ -218,11 +222,18 @@ static inline u32 pxa_ssp_read_reg(struct ssp_device *dev, u32 reg)
 #ifdef CONFIG_ARCH_PXA
 struct ssp_device *pxa_ssp_request(int port, const char *label);
 void pxa_ssp_free(struct ssp_device *);
+struct ssp_device *pxa_ssp_request_of(const struct device_node *of_node,
+				      const char *label);
 #else
 static inline struct ssp_device *pxa_ssp_request(int port, const char *label)
 {
 	return NULL;
 }
+static inline struct ssp_device *pxa_ssp_request_of(const struct device_node *n,
+						    const char *name)
+{
+	return NULL;
+}
 static inline void pxa_ssp_free(struct ssp_device *ssp) {}
 #endif
 
Signature · 1 line
1.8.3.1