Re: [linux-keystone] [PATCH] spi: davinci: add OF support for the spi controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/15/2012 11:20 AM, Grant Likely wrote:
On Mon, 12 Nov 2012 16:28:22 -0500, Murali Karicheri <m-karicheri2@xxxxxx> wrote:
This adds OF support to DaVinci SPI controller to configure platform
data through device bindings.

Signed-off-by: Murali Karicheri <m-karicheri2@xxxxxx>
Hi Murali,

Comments below...

---
  .../devicetree/bindings/spi/spi-davinci.txt        |   50 ++++++++++++
  drivers/spi/spi-davinci.c                          |   80 +++++++++++++++++++-
  2 files changed, 126 insertions(+), 4 deletions(-)
  create mode 100644 Documentation/devicetree/bindings/spi/spi-davinci.txt

diff --git a/Documentation/devicetree/bindings/spi/spi-davinci.txt b/Documentation/devicetree/bindings/spi/spi-davinci.txt
new file mode 100644
index 0000000..0369369
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-davinci.txt
@@ -0,0 +1,50 @@
+Davinci SPI controller device bindings
+
+Required properties:
+- #address-cells: number of cells required to define a chip select
+	address on the SPI bus.
Will this *ever* be something other than '1'?

+- #size-cells: should be zero.
+- compatible:
+	- "ti,davinci-spi"
Please use the actual model of the chip here. Don't try to be generic
with the compatible string. A driver can bind against more than one
compatible value and new devices can claim compatiblity with old by
including the old compatible string in this list.

+- reg: Offset and length of SPI controller register space
+- ti,davinci-spi-version: SPI version :- "1.0" or "2.0"
Usually this is determined from the compatible value directly (which is
why compatible strings shouldn't be generic). Don't use a separate
property for it.

+- ti,davinci-spi-num-cs: Number of chip selects
+- ti,davinci-spi-intr-line: interrupt line used to connect the SPI
+	IP to the ARM interrupt controller withn the SoC. Possible values
+	are 0 and 1.
? Isn't that what the interrupts property is for? I don't understand why
this is needed from the description.

+- interrupts: interrupt number offset at the irq parent
+- clocks: spi clk phandle
+
+Example of a NOR flash slave device (n25q032) connected to DaVinci
+SPI controller device over the SPI bus.
+
+spi:spi0@20BF0000 {
spi@20BF0000  (use 'spi' not 'spi0')

+	#address-cells			= <1>;
+	#size-cells			= <0>;
+	compatible			= "ti,davinci-spi";
+	reg				= <0x20BF0000 0x1000>;
+	ti,davinci-spi-version		= "1.0";
+	ti,davinci-spi-num-cs		= <4>;
+	ti,davinci-spi-intr-line	= <0>;
+	interrupts			= <338>;
+	clocks				= <&clkspi>;
+
+	flash: n25q032@0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "st,m25p32";
+		spi-max-frequency = <25000000>;
+		reg = <0>;
+
+		partition@0 {
+			label = "u-boot-spl";
+			reg = <0x0 0x80000>;
+			read-only;
+		};
+
+		partition@1 {
+			label = "test";
+			reg = <0x80000 0x380000>;
+		};
+	};
+};
diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index 71a6423..0f50319 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -26,6 +26,9 @@
  #include <linux/err.h>
  #include <linux/clk.h>
  #include <linux/dma-mapping.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
  #include <linux/spi/spi.h>
  #include <linux/spi/spi_bitbang.h>
  #include <linux/slab.h>
@@ -788,6 +791,69 @@ rx_dma_failed:
  	return r;
  }
+#if defined(CONFIG_OF)
+static const struct of_device_id davinci_spi_of_match[] = {
+	{
+		.compatible = "ti,davinci-spi",
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, davini_spi_of_match);
+
+/**
+ * spi_davinci_get_pdata - Get platform_data from DTS binding
+ * @pdev: ptr to platform data
+ *
+ * Parses and populate platform_data from device tree bindings.
+ *
+ * NOTE: Not all platform_data params are supported currently.
+ */
+static struct davinci_spi_platform_data
+	*spi_davinci_get_pdata(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct davinci_spi_platform_data *pdata;
+	unsigned int num_cs, intr_line = 0;
+	const char *version;
+
+	if (pdev->dev.platform_data)
+		return pdev->dev.platform_data;
+
+	if (!pdev->dev.of_node)
+		return NULL;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	pdev->dev.platform_data = pdata;
+	if (!pdata)
+		return NULL;
Since pdata must always be present, roll it directly into struct
spi_davinci and get rid of the kzmalloc here. It is less expensive and
is simpler code.
Grant,

Could you elaborate a bit? What you mean by rolling pdata into spi_davinci? I believe you are referring
to davinci_spi. Are you saying change following:-

struct davinci_spi {

...
struct davinci_spi_platform_data *pdata; <= to struct davinci_spi_platform_data pdata

};

+
+	/* default version is 1.0 */
+	pdata->version = SPI_VERSION_1;
+	of_property_read_string(node, "ti,davinci-spi-version", &version);
+	if (!strcmp(version, "2.0"))
+		pdata->version = SPI_VERSION_2;
+
+	/*
+	 * default num_cs is 1 and all chipsel are internal to the chip
+	 * indicated by chip_sel being NULL. GPIO based CS is not
+	 * supported yet in DT bindings.
+	 */
+	num_cs = 1;
+	of_property_read_u32(node, "ti,davinci-spi-num-cs", &num_cs);
+	pdata->num_chipselect = num_cs;
+	of_property_read_u32(node, "ti,davinci-spi-intr-line", &intr_line);
+	pdata->intr_line = intr_line;
+	return pdev->dev.platform_data;
+}
+#else
+#define davinci_spi_of_match NULL
+static struct davinci_spi_platform_data
+	*spi_davinci_get_pdata(struct platform_device *pdev)
+{
+	return pdev->dev.platform_data;
+}
+#endif
+
  /**
   * davinci_spi_probe - probe function for SPI Master Controller
   * @pdev: platform_device structure which contains plateform specific data
@@ -801,16 +867,16 @@ rx_dma_failed:
   */
  static int __devinit davinci_spi_probe(struct platform_device *pdev)
  {
+	resource_size_t dma_rx_chan = SPI_NO_RESOURCE;
+	resource_size_t	dma_tx_chan = SPI_NO_RESOURCE;
+	struct davinci_spi_platform_data *pdata;
  	struct spi_master *master;
  	struct davinci_spi *dspi;
-	struct davinci_spi_platform_data *pdata;
  	struct resource *r, *mem;
-	resource_size_t dma_rx_chan = SPI_NO_RESOURCE;
-	resource_size_t	dma_tx_chan = SPI_NO_RESOURCE;
  	int i = 0, ret = 0;
  	u32 spipc0;
- pdata = pdev->dev.platform_data;
+	pdata = spi_davinci_get_pdata(pdev);
  	if (pdata == NULL) {
  		ret = -ENODEV;
  		goto err;
@@ -851,7 +917,11 @@ static int __devinit davinci_spi_probe(struct platform_device *pdev)
  		goto release_region;
  	}
+ /* first get irq through resource table, else try of irq method */
  	dspi->irq = platform_get_irq(pdev, 0);
+	if (dspi->irq <= 0)
+		dspi->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+
This should never be needed. The irq should already be populated in the
platform_device.

  	if (dspi->irq <= 0) {
  		ret = -EINVAL;
  		goto unmap_io;
@@ -875,6 +945,7 @@ static int __devinit davinci_spi_probe(struct platform_device *pdev)
  	}
  	clk_prepare_enable(dspi->clk);
+ master->dev.of_node = pdev->dev.of_node;
  	master->bus_num = pdev->id;
  	master->num_chipselect = pdata->num_chipselect;
  	master->setup = davinci_spi_setup;
@@ -1010,6 +1081,7 @@ static struct platform_driver davinci_spi_driver = {
  	.driver = {
  		.name = "spi_davinci",
  		.owner = THIS_MODULE,
+		.of_match_table = davinci_spi_of_match,
  	},
  	.probe = davinci_spi_probe,
  	.remove = __devexit_p(davinci_spi_remove),
--
1.7.9.5


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux