diff mbox

spi: spi-omap2-mcspi.c: Add dts for slave device configuration.

Message ID 1364252415-3613-1-git-send-email-matthias.bgg@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Matthias Brugger March 25, 2013, 11 p.m. UTC
TI omap2 mcspi allows the slave devices to configure the behavior of
the SPI master. This patch adds device tree support to the existing
options.

Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
---
 Documentation/devicetree/bindings/spi/omap-spi.txt | 23 ++++++++++++
 drivers/spi/spi-omap2-mcspi.c                      | 41 ++++++++++++++++++++++
 2 files changed, 64 insertions(+)

Comments

Mark Brown April 8, 2013, 12:40 p.m. UTC | #1
On Mon, Apr 08, 2013 at 02:18:00PM +0200, Matthias Brugger wrote:
> 2013/3/26 Matthias Brugger <matthias.bgg@gmail.com>

> > TI omap2 mcspi allows the slave devices to configure the behavior of
> > the SPI master. This patch adds device tree support to the existing
> > options.

> Any comments on this patch?

Don't send contentless pings, and it appears to have only been a couple
of weeks since it was first posted...
Mark Brown April 17, 2013, 3:11 p.m. UTC | #2
On Tue, Mar 26, 2013 at 12:00:15AM +0100, Matthias Brugger wrote:

> +- The spi slave nodes can provide the following information which is used
> +  by the spi controller.

> +- ti,spi-cs-per-word: Set chipselect to be toggled on every word send.

Why is this part of the DT binding for the SPI controller?  Surely if
the devices attached to the SPI bus want to configure this they should
be setting things up appropriately in the transfers?  There might be
devices that both want and do not want this on the same bus for example,
and this isn't something that should depend on controller support.

> +- ti,spi-turbo-mode: Set turbo mode for this device.

> +		controller-data {
> +			ti,spi-cs-per-word = <1>;
> +			ti,spi-turbo-mode = <0>;

Better to use a boolean property here - check with of_find_property() to
see if the property is defined at all rather than using a value.
Illia Smyrnov May 14, 2013, 9:53 a.m. UTC | #3
> @@ -745,6 +781,11 @@ static int omap2_mcspi_setup_transfer(struct
> spi_device *spi,
>          mcspi = spi_master_get_devdata(spi->master);
>          spi_cntrl = mcspi->master;
>
> +       if (!cd && spi->dev.of_node) {
> +               cd = omap2_mcspi_get_slave_ctrldata(spi);
 > +               spi->controller_data = cd;

Here you call omap2_mcspi_get_slave_ctrldata function that allocate 
memory for cd structure, but this memory never freed.

Also, why do you read DT data in omap2_mcspi_setup_transfer function?
Under certain conditions the omap2_mcspi_setup_transfer function may be 
calling for each transfer and each call it will check (!cd && 
spi->dev.of_node) condition.

Consider to move DT data read code from omap2_mcspi_setup_transfer to
omap2_mcspi_setup function and free spi->controller_data pointer in 
omap2_mcspi_cleanup function.

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

Patch

diff --git a/Documentation/devicetree/bindings/spi/omap-spi.txt b/Documentation/devicetree/bindings/spi/omap-spi.txt
index 938809c..fef16bf 100644
--- a/Documentation/devicetree/bindings/spi/omap-spi.txt
+++ b/Documentation/devicetree/bindings/spi/omap-spi.txt
@@ -10,8 +10,20 @@  Required properties:
 			  input. The default is D0 as input and
 			  D1 as output.
 
+SPI Controller specific data in SPI slave nodes:
+
+- The spi slave nodes can provide the following information which is used
+  by the spi controller.
+
+- ti,spi-cs-per-word: Set chipselect to be toggled on every word send.
+
+- ti,spi-turbo-mode: Set turbo mode for this device.
+
+
 Example:
 
+- SoC Specific Portion:
+
 mcspi1: mcspi@1 {
     #address-cells = <1>;
     #size-cells = <0>;
@@ -20,3 +32,14 @@  mcspi1: mcspi@1 {
     ti,spi-num-cs = <4>;
 };
 
+- Board Specific Portion:
+
+	spi-device@0 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		controller-data {
+			ti,spi-cs-per-word = <1>;
+			ti,spi-turbo-mode = <0>;
+		};
+	};
diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c
index 893c3d7..1ae5009 100644
--- a/drivers/spi/spi-omap2-mcspi.c
+++ b/drivers/spi/spi-omap2-mcspi.c
@@ -731,11 +731,47 @@  static u32 omap2_mcspi_calc_divisor(u32 speed_hz)
 	return 15;
 }
 
+static struct omap2_mcspi_device_config *omap2_mcspi_get_slave_ctrldata(
+			struct spi_device *spi)
+{
+	struct omap2_mcspi_device_config *cd;
+	struct device_node *slave_np, *data_np = NULL;
+
+	slave_np = spi->dev.of_node;
+	if (!slave_np) {
+		dev_err(&spi->dev, "device node not found\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	data_np = of_get_child_by_name(slave_np, "controller-data");
+	if (!data_np) {
+		dev_err(&spi->dev, "child node 'controller-data' not found\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	cd = kzalloc(sizeof(*cd), GFP_KERNEL);
+	if (!cd) {
+		dev_err(&spi->dev, "could not allocate memory for controller data\n");
+		of_node_put(data_np);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	if (of_get_property(data_np, "ti,spi-cs-per-word", NULL))
+		cd->cs_per_word = 1;
+
+	if (of_get_property(data_np, "ti,spi-turbo-mode", NULL))
+		cd->turbo_mode = 1;
+
+	of_node_put(data_np);
+	return cd;
+}
+
 /* called only when no transfer is active to this device */
 static int omap2_mcspi_setup_transfer(struct spi_device *spi,
 		struct spi_transfer *t)
 {
 	struct omap2_mcspi_cs *cs = spi->controller_state;
+	struct omap2_mcspi_device_config *cd = spi->controller_data;
 	struct omap2_mcspi *mcspi;
 	struct spi_master *spi_cntrl;
 	u32 l = 0, div = 0;
@@ -745,6 +781,11 @@  static int omap2_mcspi_setup_transfer(struct spi_device *spi,
 	mcspi = spi_master_get_devdata(spi->master);
 	spi_cntrl = mcspi->master;
 
+	if (!cd && spi->dev.of_node) {
+		cd = omap2_mcspi_get_slave_ctrldata(spi);
+		spi->controller_data = cd;
+	}
+
 	if (t != NULL && t->bits_per_word)
 		word_len = t->bits_per_word;