[PATCH 0/6] GSBI CRCI Autoconfiguration Support

[PATCH 0/6] GSBI CRCI Autoconfiguration Support

[Andy Gross]

This patch set adds support for automatic configuration of GSBI DMA CRCI values.

DMA operations require that the ADM CRCI mux values be properly configured in
the TCSR (Top Control and Status Register) block.  During probing of a GSBI
device, the client mode must be declared and this can be used to lookup the
correct TCSR ADM CRCI MUX settings and then program them so that they are
correct before any clients are populated.

These patches add the TCSR as a syscon device and that allows the GSBI to
access and manipulate the ADM CRCI MUX registers to correctly configure the
values based on the GSBI port configuration.

Andy Gross (6):
  soc: qcom: gsbi: Add support for ADM CRCI muxing
  mfd: qcom,tcsr: Add device tree binding for TCSR
  ARM: DT: apq8064: Add TCSR support
  ARM: DT: ipq8064: Add TCSR support
  ARM: DT: msm8660: Add TCSR support
  ARM: DT: msm8960: Add TCSR support

 .../devicetree/bindings/mfd/qcom,tcsr.txt          |   22 +++
 .../devicetree/bindings/soc/qcom/qcom,gsbi.txt     |   17 ++-
 arch/arm/boot/dts/qcom-apq8064.dtsi                |   20 ++-
 arch/arm/boot/dts/qcom-ipq8064.dtsi                |   20 ++-
 arch/arm/boot/dts/qcom-msm8660.dtsi                |   10 +-
 arch/arm/boot/dts/qcom-msm8960.dtsi                |   10 +-
 drivers/soc/qcom/Kconfig                           |    1 +
 drivers/soc/qcom/qcom_gsbi.c                       |  148 +++++++++++++++++++-
 8 files changed, 232 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/qcom,tcsr.txt

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 1/6] soc: qcom: gsbi: Add support for ADM CRCI muxing

[Andy Gross]

This patch adds automatic configuration for the ADM CRCI muxing required to
support DMA operations for GSBI clients.  The GSBI mode and instance determine
the correct TCSR ADM CRCI MUX value that must be programmed so that the DMA
works properly.

Signed-off-by: Andy Gross <agross@codeaurora.org>
---
 .../devicetree/bindings/soc/qcom/qcom,gsbi.txt     |   17 ++-
 drivers/soc/qcom/Kconfig                           |    1 +
 drivers/soc/qcom/qcom_gsbi.c                       |  148 +++++++++++++++++++-
 3 files changed, 158 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt
index 4ce24d4..39eea8a 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt
@@ -6,12 +6,18 @@ configuration settings.  The mode setting will govern the input/output mode of
 the 4 GSBI IOs.
 
 Required properties:
-- compatible: must contain "qcom,gsbi-v1.0.0" for APQ8064/IPQ8064
+- compatible:	Should contain:
+		"qcom,gsbi-ipq8064" for IPQ8064
+		"qcom,gsbi-apq8064" for APQ8064
+		"qcom,gsbi-msm8960" for MSM8960
+		"qcom,gsbi-msm8660" for MSM8660
 - reg: Address range for GSBI registers
 - clocks: required clock
 - clock-names: must contain "iface" entry
 - qcom,mode : indicates MUX value for configuration of the serial interface.
   Please reference dt-bindings/soc/qcom,gsbi.h for valid mux values.
+- qcom,gsbi-num: indicates GSBI instance number
+- syscon-tcsr: indicates phandle of TCSR syscon node
 
 Optional properties:
 - qcom,crci : indicates CRCI MUX value for QUP CRCI ports.  Please reference
@@ -48,6 +54,9 @@ Example for APQ8064:
 		qcom,mode = <GSBI_PROT_I2C_UART>;
 		qcom,crci = <GSBI_CRCI_QUP>;
 
+		qcom,gsbi-num = <4>;
+		syscon-tcsr = <&tcsr>;
+
 		/* child nodes go under here */
 
 		i2c_qup4: i2c@16380000 {
@@ -76,3 +85,9 @@ Example for APQ8064:
 		};
 	};
 
+	tcsr: syscon@1a400000 {
+		compatible = "qcom,apq8064-tcsr", "syscon";
+		reg = <0x1a400000 0x100>;
+	};
+
+
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 7bd2c94..32f20be 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -4,6 +4,7 @@
 config QCOM_GSBI
         tristate "QCOM General Serial Bus Interface"
         depends on ARCH_QCOM
+	select MFD_SYSCON
         help
           Say y here to enable GSBI support.  The GSBI provides control
           functions for connecting the underlying serial UART, SPI, and I2C
diff --git a/drivers/soc/qcom/qcom_gsbi.c b/drivers/soc/qcom/qcom_gsbi.c
index 729425d..c7a22b5 100644
--- a/drivers/soc/qcom/qcom_gsbi.c
+++ b/drivers/soc/qcom/qcom_gsbi.c
@@ -18,22 +18,129 @@
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include <dt-bindings/soc/qcom,gsbi.h>
 
 #define GSBI_CTRL_REG		0x0000
 #define GSBI_PROTOCOL_SHIFT	4
+#define MAX_GSBI		12
+
+#define TCSR_ADM_CRCI_BASE	0x70
+
+struct crci_config {
+	u32 num_rows;
+	const u32 *array;
+};
+
+static const u32 crci_ipq8064[][MAX_GSBI] = {
+	{
+		0x000003, 0x00000c, 0x000030, 0x0000c0,
+		0x000300, 0x000c00, 0x003000, 0x00c000,
+		0x030000, 0x0c0000, 0x300000, 0xc00000
+	},
+	{
+		0x000003, 0x00000c, 0x000030, 0x0000c0,
+		0x000300, 0x000c00, 0x003000, 0x00c000,
+		0x030000, 0x0c0000, 0x300000, 0xc00000
+	},
+};
+
+static const struct crci_config config_ipq8064 = {
+	.num_rows = ARRAY_SIZE(crci_ipq8064),
+	.array = crci_ipq8064[0],
+};
+
+static const unsigned int crci_apq8064[][MAX_GSBI] = {
+	{
+		0x001800, 0x006000, 0x000030, 0x0000c0,
+		0x000300, 0x000400, 0x000000, 0x000000,
+		0x000000, 0x000000, 0x000000, 0x000000
+	},
+	{
+		0x000000, 0x000000, 0x000000, 0x000000,
+		0x000000, 0x000020, 0x0000c0, 0x000000,
+		0x000000, 0x000000, 0x000000, 0x000000
+	},
+};
+
+static const struct crci_config config_apq8064 = {
+	.num_rows = ARRAY_SIZE(crci_apq8064),
+	.array = crci_apq8064[0],
+};
+
+static const unsigned int crci_msm8960[][MAX_GSBI] = {
+	{
+		0x000003, 0x00000c, 0x000030, 0x0000c0,
+		0x000300, 0x000400, 0x000000, 0x000000,
+		0x000000, 0x000000, 0x000000, 0x000000
+	},
+	{
+		0x000000, 0x000000, 0x000000, 0x000000,
+		0x000000, 0x000020, 0x0000c0, 0x000300,
+		0x001800, 0x006000, 0x000000, 0x000000
+	},
+};
+
+static const struct crci_config config_msm8960 = {
+	.num_rows = ARRAY_SIZE(crci_msm8960),
+	.array = crci_msm8960[0],
+};
+
+static const unsigned int crci_msm8660[][MAX_GSBI] = {
+	{	/* ADM 0 - B */
+		0x000003, 0x00000c, 0x000030, 0x0000c0,
+		0x000300, 0x000c00, 0x003000, 0x00c000,
+		0x030000, 0x0c0000, 0x300000, 0xc00000
+	},
+	{	/* ADM 0 - B */
+		0x000003, 0x00000c, 0x000030, 0x0000c0,
+		0x000300, 0x000c00, 0x003000, 0x00c000,
+		0x030000, 0x0c0000, 0x300000, 0xc00000
+	},
+	{	/* ADM 1 - A */
+		0x000003, 0x00000c, 0x000030, 0x0000c0,
+		0x000300, 0x000c00, 0x003000, 0x00c000,
+		0x030000, 0x0c0000, 0x300000, 0xc00000
+	},
+	{	/* ADM 1 - B */
+		0x000003, 0x00000c, 0x000030, 0x0000c0,
+		0x000300, 0x000c00, 0x003000, 0x00c000,
+		0x030000, 0x0c0000, 0x300000, 0xc00000
+	},
+};
+
+static const struct crci_config config_msm8660 = {
+	.num_rows = ARRAY_SIZE(crci_msm8660),
+	.array = crci_msm8660[0],
+};
 
 struct gsbi_info {
 	struct clk *hclk;
 	u32 mode;
 	u32 crci;
+	struct regmap *tcsr;
+};
+
+static const struct of_device_id gsbi_dt_match[] = {
+	{ .compatible = "qcom,gsbi-v1.0.0", .data = NULL},
+	{ .compatible = "qcom,gsbi-ipq8064", .data = &config_ipq8064},
+	{ .compatible = "qcom,gsbi-apq8064", .data = &config_apq8064},
+	{ .compatible = "qcom,gsbi-msm8960", .data = &config_msm8960},
+	{ .compatible = "qcom,gsbi-msm8660", .data = &config_msm8660},
+	{ },
 };
+MODULE_DEVICE_TABLE(of, gsbi_dt_match);
 
 static int gsbi_probe(struct platform_device *pdev)
 {
 	struct device_node *node = pdev->dev.of_node;
+	const struct of_device_id *match;
 	struct resource *res;
 	void __iomem *base;
 	struct gsbi_info *gsbi;
+	u32 gsbi_num, i, val;
+	struct crci_config *config;
 
 	gsbi = devm_kzalloc(&pdev->dev, sizeof(*gsbi), GFP_KERNEL);
 
@@ -45,6 +152,20 @@ static int gsbi_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
+	gsbi->tcsr = syscon_regmap_lookup_by_phandle(node, "syscon-tcsr");
+	if (IS_ERR(gsbi->tcsr))
+		return -EINVAL;
+
+	if (of_property_read_u32(node, "qcom,gsbi-num", &gsbi_num)) {
+		dev_err(&pdev->dev, "missing gsbi instance number\n");
+		return -EINVAL;
+	}
+
+	if (!gsbi_num || gsbi_num > MAX_GSBI) {
+		dev_err(&pdev->dev, "invalid gsbi number\n");
+		return -EINVAL;
+	}
+
 	if (of_property_read_u32(node, "qcom,mode", &gsbi->mode)) {
 		dev_err(&pdev->dev, "missing mode configuration\n");
 		return -EINVAL;
@@ -64,6 +185,26 @@ static int gsbi_probe(struct platform_device *pdev)
 	writel_relaxed((gsbi->mode << GSBI_PROTOCOL_SHIFT) | gsbi->crci,
 				base + GSBI_CTRL_REG);
 
+	/*
+	 * modify tcsr to reflect mode and ADM CRCI mux
+	 * Each gsbi contains a pair of bits, one for RX and one for TX
+	 * SPI mode requires both bits cleared, otherwise they are set
+	 */
+	match = of_match_node(gsbi_dt_match, node);
+	config = (struct crci_config *)match->data;
+
+	if (config)
+		for (i = 0; i < config->num_rows; i++) {
+			if (gsbi->mode == GSBI_PROT_SPI)
+				val = config->array[i*MAX_GSBI + gsbi_num - 1];
+			else
+				val = 0;
+
+			regmap_update_bits(gsbi->tcsr,
+				TCSR_ADM_CRCI_BASE + 0x4*i,
+				config->array[i*MAX_GSBI + gsbi_num - 1], val);
+		}
+
 	/* make sure the gsbi control write is not reordered */
 	wmb();
 
@@ -81,13 +222,6 @@ static int gsbi_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id gsbi_dt_match[] = {
-	{ .compatible = "qcom,gsbi-v1.0.0", },
-	{ },
-};
-
-MODULE_DEVICE_TABLE(of, gsbi_dt_match);
-
 static struct platform_driver gsbi_driver = {
 	.driver = {
 		.name		= "gsbi",
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 1/6] soc: qcom: gsbi: Add support for ADM CRCI muxing

[Bjorn Andersson]

On Tue 27 Jan 14:10 PST 2015, Andy Gross wrote:

This solution looks good, just some style things.

> diff --git a/drivers/soc/qcom/qcom_gsbi.c b/drivers/soc/qcom/qcom_gsbi.c
[..]
> +#define MAX_GSBI		12
> +
> +#define TCSR_ADM_CRCI_BASE	0x70
> +
> +struct crci_config {
> +	u32 num_rows;
> +	const u32 *array;

Making this:
 const u32 (*array)[MAX_GSBI];

> +};
> +
> +static const u32 crci_ipq8064[][MAX_GSBI] = {
> +	{
> +		0x000003, 0x00000c, 0x000030, 0x0000c0,
> +		0x000300, 0x000c00, 0x003000, 0x00c000,
> +		0x030000, 0x0c0000, 0x300000, 0xc00000
> +	},
> +	{
> +		0x000003, 0x00000c, 0x000030, 0x0000c0,
> +		0x000300, 0x000c00, 0x003000, 0x00c000,
> +		0x030000, 0x0c0000, 0x300000, 0xc00000
> +	},
> +};
> +
> +static const struct crci_config config_ipq8064 = {
> +	.num_rows = ARRAY_SIZE(crci_ipq8064),
> +	.array = crci_ipq8064[0],

...so that you can make this:
 .array = crci_ipq8064,

> +};
[..]
> @@ -45,6 +152,20 @@ static int gsbi_probe(struct platform_device *pdev)
[..]
> +	if (config)
> +		for (i = 0; i < config->num_rows; i++) {
> +			if (gsbi->mode == GSBI_PROT_SPI)
> +				val = config->array[i*MAX_GSBI + gsbi_num - 1];

...will give you:
 config->array[i][gsbi_num - 1];

> +			else
> +				val = 0;
> +
> +			regmap_update_bits(gsbi->tcsr,
> +				TCSR_ADM_CRCI_BASE + 0x4*i,
> +				config->array[i*MAX_GSBI + gsbi_num - 1], val);

To me this would be cleaner:

mask = config->array[i][gsbi_num - 1];
if (gsbi->mode == GSBI_PRO_SPI)
	regmap_update_bits(gsbi->tcsr, TCSR_ADM_CRCI_BASE + i * 4, mask, mask);
else
	regmap_update_bits(gsbi->tcsr, TCSR_ADM_CRCI_BASE + i * 4, mask, 0);

> +		}
> +
There should be an extra set of {} around the if statment body.

Regards,
Bjorn

Re: [PATCH 1/6] soc: qcom: gsbi: Add support for ADM CRCI muxing

[Andy Gross]

On Tue, Jan 27, 2015 at 07:11:50PM -0800, Bjorn Andersson wrote:
> On Tue 27 Jan 14:10 PST 2015, Andy Gross wrote:
> 
> This solution looks good, just some style things.
> 
> > diff --git a/drivers/soc/qcom/qcom_gsbi.c b/drivers/soc/qcom/qcom_gsbi.c
> [..]
> > +#define MAX_GSBI		12
> > +
> > +#define TCSR_ADM_CRCI_BASE	0x70
> > +
> > +struct crci_config {
> > +	u32 num_rows;
> > +	const u32 *array;
> 
> Making this:
>  const u32 (*array)[MAX_GSBI];

ok, i'll switch to do this instead and change the usage down below.

<snip>

> > +			else
> > +				val = 0;
> > +
> > +			regmap_update_bits(gsbi->tcsr,
> > +				TCSR_ADM_CRCI_BASE + 0x4*i,
> > +				config->array[i*MAX_GSBI + gsbi_num - 1], val);
> 
> To me this would be cleaner:
> 
> mask = config->array[i][gsbi_num - 1];
> if (gsbi->mode == GSBI_PRO_SPI)
> 	regmap_update_bits(gsbi->tcsr, TCSR_ADM_CRCI_BASE + i * 4, mask, mask);
> else
> 	regmap_update_bits(gsbi->tcsr, TCSR_ADM_CRCI_BASE + i * 4, mask, 0);

i had considered doing something similar.  It does make it cleaner, so I'll do
this.

> > +		}
> > +
> There should be an extra set of {} around the if statment body.

done


-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/6] soc: qcom: gsbi: Add support for ADM CRCI muxing

[Stanimir Varbanov]

Hi Andy,

On 01/28/2015 12:10 AM, Andy Gross wrote:
> This patch adds automatic configuration for the ADM CRCI muxing required to
> support DMA operations for GSBI clients.  The GSBI mode and instance determine
> the correct TCSR ADM CRCI MUX value that must be programmed so that the DMA
> works properly.
> 
> Signed-off-by: Andy Gross <agross@codeaurora.org>
> ---
>  .../devicetree/bindings/soc/qcom/qcom,gsbi.txt     |   17 ++-
>  drivers/soc/qcom/Kconfig                           |    1 +
>  drivers/soc/qcom/qcom_gsbi.c                       |  148 +++++++++++++++++++-
>  3 files changed, 158 insertions(+), 8 deletions(-)
> 

<snip>

> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 7bd2c94..32f20be 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -4,6 +4,7 @@
>  config QCOM_GSBI
>          tristate "QCOM General Serial Bus Interface"
>          depends on ARCH_QCOM
> +	select MFD_SYSCON

Wrong indentation? In fact the original Kconfig entry has spaces instead
of tabs, could you prepare a cleanup patch for this.

>          help
>            Say y here to enable GSBI support.  The GSBI provides control
>            functions for connecting the underlying serial UART, SPI, and I2C

<snip>

-- 
regards,
Stan

Re: [PATCH 1/6] soc: qcom: gsbi: Add support for ADM CRCI muxing

[Andy Gross]

On Wed, Jan 28, 2015 at 11:05:50AM +0200, Stanimir Varbanov wrote:
> <snip>
> 
> > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> > index 7bd2c94..32f20be 100644
> > --- a/drivers/soc/qcom/Kconfig
> > +++ b/drivers/soc/qcom/Kconfig
> > @@ -4,6 +4,7 @@
> >  config QCOM_GSBI
> >          tristate "QCOM General Serial Bus Interface"
> >          depends on ARCH_QCOM
> > +	select MFD_SYSCON
> 
> Wrong indentation? In fact the original Kconfig entry has spaces instead
> of tabs, could you prepare a cleanup patch for this.

yeah I'll switch to spaces since the Kconfig is using spaces.

> >          help
> >            Say y here to enable GSBI support.  The GSBI provides control
> >            functions for connecting the underlying serial UART, SPI, and I2C

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/6] soc: qcom: gsbi: Add support for ADM CRCI muxing

[Stephen Boyd]

On 01/27, Andy Gross wrote:
> This patch adds automatic configuration for the ADM CRCI muxing required to
> support DMA operations for GSBI clients.  The GSBI mode and instance determine
> the correct TCSR ADM CRCI MUX value that must be programmed so that the DMA
> works properly.
> 
> Signed-off-by: Andy Gross <agross@codeaurora.org>
> ---
>  .../devicetree/bindings/soc/qcom/qcom,gsbi.txt     |   17 ++-
>  drivers/soc/qcom/Kconfig                           |    1 +
>  drivers/soc/qcom/qcom_gsbi.c                       |  148 +++++++++++++++++++-
>  3 files changed, 158 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt
> index 4ce24d4..39eea8a 100644
> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt
> @@ -6,12 +6,18 @@ configuration settings.  The mode setting will govern the input/output mode of
>  the 4 GSBI IOs.
>  
>  Required properties:
> -- compatible: must contain "qcom,gsbi-v1.0.0" for APQ8064/IPQ8064
> +- compatible:	Should contain:
> +		"qcom,gsbi-ipq8064" for IPQ8064
> +		"qcom,gsbi-apq8064" for APQ8064
> +		"qcom,gsbi-msm8960" for MSM8960
> +		"qcom,gsbi-msm8660" for MSM8660

Hopefully this is not necessary, but if it is we should leave the
old compatible here and say it's deprecated or something.

>  - reg: Address range for GSBI registers
>  - clocks: required clock
>  - clock-names: must contain "iface" entry
>  - qcom,mode : indicates MUX value for configuration of the serial interface.
>    Please reference dt-bindings/soc/qcom,gsbi.h for valid mux values.
> +- qcom,gsbi-num: indicates GSBI instance number

Why not use DT aliases for this? Then other drivers or more
generic code can search for a gsbiN alias for the particular gsbi
node. No qcom specific property.

> +- syscon-tcsr: indicates phandle of TCSR syscon node

Make this optional but required if any child nodes use dma?

>  
>  Optional properties:
>  - qcom,crci : indicates CRCI MUX value for QUP CRCI ports.  Please reference

> diff --git a/drivers/soc/qcom/qcom_gsbi.c b/drivers/soc/qcom/qcom_gsbi.c
> index 729425d..c7a22b5 100644
> --- a/drivers/soc/qcom/qcom_gsbi.c
> +++ b/drivers/soc/qcom/qcom_gsbi.c
>  
>  struct gsbi_info {
>  	struct clk *hclk;
>  	u32 mode;
>  	u32 crci;
> +	struct regmap *tcsr;
> +};
> +
> +static const struct of_device_id gsbi_dt_match[] = {
> +	{ .compatible = "qcom,gsbi-v1.0.0", .data = NULL},
> +	{ .compatible = "qcom,gsbi-ipq8064", .data = &config_ipq8064},
> +	{ .compatible = "qcom,gsbi-apq8064", .data = &config_apq8064},
> +	{ .compatible = "qcom,gsbi-msm8960", .data = &config_msm8960},
> +	{ .compatible = "qcom,gsbi-msm8660", .data = &config_msm8660},
> +	{ },
>  };
> +MODULE_DEVICE_TABLE(of, gsbi_dt_match);
>  
>  static int gsbi_probe(struct platform_device *pdev)
>  {
>  	struct device_node *node = pdev->dev.of_node;
> +	const struct of_device_id *match;
>  	struct resource *res;
>  	void __iomem *base;
>  	struct gsbi_info *gsbi;
> +	u32 gsbi_num, i, val;

i should be int

> +	struct crci_config *config;

const?

>  
>  	gsbi = devm_kzalloc(&pdev->dev, sizeof(*gsbi), GFP_KERNEL);
>  
> @@ -45,6 +152,20 @@ static int gsbi_probe(struct platform_device *pdev)
>  	if (IS_ERR(base))
>  		return PTR_ERR(base);
>  
> +	gsbi->tcsr = syscon_regmap_lookup_by_phandle(node, "syscon-tcsr");
> +	if (IS_ERR(gsbi->tcsr))
> +		return -EINVAL;
> +
> +	if (of_property_read_u32(node, "qcom,gsbi-num", &gsbi_num)) {
> +		dev_err(&pdev->dev, "missing gsbi instance number\n");
> +		return -EINVAL;
> +	}

As said before, aliases would do the job the same and not require
some qcom specific property.

> +
> +	if (!gsbi_num || gsbi_num > MAX_GSBI) {
> +		dev_err(&pdev->dev, "invalid gsbi number\n");
> +		return -EINVAL;
> +	}
> +
>  	if (of_property_read_u32(node, "qcom,mode", &gsbi->mode)) {
>  		dev_err(&pdev->dev, "missing mode configuration\n");
>  		return -EINVAL;
> @@ -64,6 +185,26 @@ static int gsbi_probe(struct platform_device *pdev)
>  	writel_relaxed((gsbi->mode << GSBI_PROTOCOL_SHIFT) | gsbi->crci,
>  				base + GSBI_CTRL_REG);
>  
> +	/*
> +	 * modify tcsr to reflect mode and ADM CRCI mux
> +	 * Each gsbi contains a pair of bits, one for RX and one for TX
> +	 * SPI mode requires both bits cleared, otherwise they are set
> +	 */
> +	match = of_match_node(gsbi_dt_match, node);

Why not match the config to the TCSR compatible string? Wouldn't
that more accurately reflect that we need to set different bits
depending on which type of TCSR we're using? The version of GSBI
hardware is not actually changing in every different SoC so I
don't see why we want to change the compatible there just because
the TCSR register layout changed.

> +	config = (struct crci_config *)match->data;

Cast shouldn't be necessary if config is const?

> +
> +	if (config)
> +		for (i = 0; i < config->num_rows; i++) {
> +			if (gsbi->mode == GSBI_PROT_SPI)

Doesn't I2C need the same treatment (anything in QUP really)?
Maybe the logic could be changed to check for gsbi->crci ==
GSBI_CRCI_QUP?

> +				val = config->array[i*MAX_GSBI + gsbi_num - 1];
> +			else
> +				val = 0;
> +
> +			regmap_update_bits(gsbi->tcsr,
> +				TCSR_ADM_CRCI_BASE + 0x4*i,
> +				config->array[i*MAX_GSBI + gsbi_num - 1], val);
> +		}
> +
>  	/* make sure the gsbi control write is not reordered */
>  	wmb();
>  

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/6] soc: qcom: gsbi: Add support for ADM CRCI muxing

[Andy Gross]

On Wed, Jan 28, 2015 at 06:11:50PM -0800, Stephen Boyd wrote:

<snip>

> >  Required properties:
> > -- compatible: must contain "qcom,gsbi-v1.0.0" for APQ8064/IPQ8064
> > +- compatible:	Should contain:
> > +		"qcom,gsbi-ipq8064" for IPQ8064
> > +		"qcom,gsbi-apq8064" for APQ8064
> > +		"qcom,gsbi-msm8960" for MSM8960
> > +		"qcom,gsbi-msm8660" for MSM8660
> 
> Hopefully this is not necessary, but if it is we should leave the
> old compatible here and say it's deprecated or something.

Right.  I went back and forth with the tcsr vs gsbi.  If change the compats I'll
put in a deprecated.

> >  - reg: Address range for GSBI registers
> >  - clocks: required clock
> >  - clock-names: must contain "iface" entry
> >  - qcom,mode : indicates MUX value for configuration of the serial interface.
> >    Please reference dt-bindings/soc/qcom,gsbi.h for valid mux values.
> > +- qcom,gsbi-num: indicates GSBI instance number
> 
> Why not use DT aliases for this? Then other drivers or more
> generic code can search for a gsbiN alias for the particular gsbi
> node. No qcom specific property.

Yeah thats cleaner.  I'll do that.

> 
> > +- syscon-tcsr: indicates phandle of TCSR syscon node
> 
> Make this optional but required if any child nodes use dma?

To enforce that I'd have to determine that a child has a dmas.  I guess that
isn't so bad.

<snip>

> >  static int gsbi_probe(struct platform_device *pdev)
> >  {
> >  	struct device_node *node = pdev->dev.of_node;
> > +	const struct of_device_id *match;
> >  	struct resource *res;
> >  	void __iomem *base;
> >  	struct gsbi_info *gsbi;
> > +	u32 gsbi_num, i, val;
> 
> i should be int
> 
> > +	struct crci_config *config;
> 
> const?

will fix both.

> >  
> >  	gsbi = devm_kzalloc(&pdev->dev, sizeof(*gsbi), GFP_KERNEL);
> >  
> > @@ -45,6 +152,20 @@ static int gsbi_probe(struct platform_device *pdev)
> >  	if (IS_ERR(base))
> >  		return PTR_ERR(base);
> >  
> > +	gsbi->tcsr = syscon_regmap_lookup_by_phandle(node, "syscon-tcsr");
> > +	if (IS_ERR(gsbi->tcsr))
> > +		return -EINVAL;
> > +
> > +	if (of_property_read_u32(node, "qcom,gsbi-num", &gsbi_num)) {
> > +		dev_err(&pdev->dev, "missing gsbi instance number\n");
> > +		return -EINVAL;
> > +	}
> 
> As said before, aliases would do the job the same and not require
> some qcom specific property.

Yup. will fix.

> > +
> > +	if (!gsbi_num || gsbi_num > MAX_GSBI) {
> > +		dev_err(&pdev->dev, "invalid gsbi number\n");
> > +		return -EINVAL;
> > +	}
> > +
> >  	if (of_property_read_u32(node, "qcom,mode", &gsbi->mode)) {
> >  		dev_err(&pdev->dev, "missing mode configuration\n");
> >  		return -EINVAL;
> > @@ -64,6 +185,26 @@ static int gsbi_probe(struct platform_device *pdev)
> >  	writel_relaxed((gsbi->mode << GSBI_PROTOCOL_SHIFT) | gsbi->crci,
> >  				base + GSBI_CTRL_REG);
> >  
> > +	/*
> > +	 * modify tcsr to reflect mode and ADM CRCI mux
> > +	 * Each gsbi contains a pair of bits, one for RX and one for TX
> > +	 * SPI mode requires both bits cleared, otherwise they are set
> > +	 */
> > +	match = of_match_node(gsbi_dt_match, node);
> 
> Why not match the config to the TCSR compatible string? Wouldn't
> that more accurately reflect that we need to set different bits
> depending on which type of TCSR we're using? The version of GSBI
> hardware is not actually changing in every different SoC so I
> don't see why we want to change the compatible there just because
> the TCSR register layout changed.

That is true.  However, with the gsbi compat, I avoid doing a match multiple
times and get the table I need immediately.  The alternative is N checks or
pulling the compat strings and comparing them to get the right table.

> > +	config = (struct crci_config *)match->data;
> 
> Cast shouldn't be necessary if config is const?

will check if that works

> > +
> > +	if (config)
> > +		for (i = 0; i < config->num_rows; i++) {
> > +			if (gsbi->mode == GSBI_PROT_SPI)
> 
> Doesn't I2C need the same treatment (anything in QUP really)?
> Maybe the logic could be changed to check for gsbi->crci ==
> GSBI_CRCI_QUP?

Nope.  I2C doesn't support DMA when ADM is the controller.  It's only SPI or
UART.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/6] soc: qcom: gsbi: Add support for ADM CRCI muxing

[Stephen Boyd]

On 01/28, Andy Gross wrote:
> On Wed, Jan 28, 2015 at 06:11:50PM -0800, Stephen Boyd wrote:
> 
> > 
> > > +- syscon-tcsr: indicates phandle of TCSR syscon node
> > 
> > Make this optional but required if any child nodes use dma?
> 
> To enforce that I'd have to determine that a child has a dmas.  I guess that
> isn't so bad.

I don't think we need to write any code in the driver to enforce
this right now. As long as nobody gets it wrong bad things won't
happen and we can just assume that the DT is correct. Once we get
some bad DT we can go ahead and implement some DT parsing stuff.

> 
> 
> > > +
> > > +	if (!gsbi_num || gsbi_num > MAX_GSBI) {
> > > +		dev_err(&pdev->dev, "invalid gsbi number\n");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > >  	if (of_property_read_u32(node, "qcom,mode", &gsbi->mode)) {
> > >  		dev_err(&pdev->dev, "missing mode configuration\n");
> > >  		return -EINVAL;
> > > @@ -64,6 +185,26 @@ static int gsbi_probe(struct platform_device *pdev)
> > >  	writel_relaxed((gsbi->mode << GSBI_PROTOCOL_SHIFT) | gsbi->crci,
> > >  				base + GSBI_CTRL_REG);
> > >  
> > > +	/*
> > > +	 * modify tcsr to reflect mode and ADM CRCI mux
> > > +	 * Each gsbi contains a pair of bits, one for RX and one for TX
> > > +	 * SPI mode requires both bits cleared, otherwise they are set
> > > +	 */
> > > +	match = of_match_node(gsbi_dt_match, node);
> > 
> > Why not match the config to the TCSR compatible string? Wouldn't
> > that more accurately reflect that we need to set different bits
> > depending on which type of TCSR we're using? The version of GSBI
> > hardware is not actually changing in every different SoC so I
> > don't see why we want to change the compatible there just because
> > the TCSR register layout changed.
> 
> That is true.  However, with the gsbi compat, I avoid doing a match multiple
> times and get the table I need immediately.  The alternative is N checks or
> pulling the compat strings and comparing them to get the right table.

Sorry I'm not following. We always have to call of_match_node()
here and I'm just suggesting we replace gsbi_dt_match with
tcsr_dt_match and leave gsbi_dt_match like it was before. I don't
see how anything is avoided.

> 
> 
> > > +
> > > +	if (config)
> > > +		for (i = 0; i < config->num_rows; i++) {
> > > +			if (gsbi->mode == GSBI_PROT_SPI)
> > 
> > Doesn't I2C need the same treatment (anything in QUP really)?
> > Maybe the logic could be changed to check for gsbi->crci ==
> > GSBI_CRCI_QUP?
> 
> Nope.  I2C doesn't support DMA when ADM is the controller.  It's only SPI or
> UART.

Ok, I don't get it but I guess it doesn't matter if I2C doesn't
use it.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/6] mfd: qcom,tcsr: Add device tree binding for TCSR

[Andy Gross]

This patch adds the device tree binding for the Qualcomm Top Control and
Status Register device.  The TCSR is comprised of a set of registers that
provide various control and status functions for attached peripherals.

Signed-off-by: Andy Gross <agross@codeaurora.org>
---
 .../devicetree/bindings/mfd/qcom,tcsr.txt          |   22 ++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/qcom,tcsr.txt

diff --git a/Documentation/devicetree/bindings/mfd/qcom,tcsr.txt b/Documentation/devicetree/bindings/mfd/qcom,tcsr.txt
new file mode 100644
index 0000000..e90519d
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/qcom,tcsr.txt
@@ -0,0 +1,22 @@
+QCOM Top Control and Status Register
+
+Qualcomm devices have a set of registers that provide various control and status
+functions for their peripherals.  This node is intended to allow access to these
+registers via syscon.
+
+Required properties:
+- compatible:	Should contain:
+		"qcom,tcsr-ipq8064", "syscon" for IPQ8064
+		"qcom,tcsr-apq8064", "syscon" for APQ8064
+		"qcom,tcsr-msm8660", "syscon" for MSM8660
+		"qcom,tcsr-msm8960", "syscon" for MSM8960
+		"qcom,tcsr-msm8974", "syscon" for MSM8974
+		"qcom,tcsr-apq8084", "syscon" for APQ8084
+		"qcom,tcsr-msm8916", "syscon" for MSM8916
+- reg: Address range for TCSR registers
+
+Example:
+	tcsr: syscon@1a400000 {
+		compatible = "qcom,tcsr-msm8960", "syscon";
+		reg = <0x1a400000 0x100>;
+	};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 3/6] ARM: DT: apq8064: Add TCSR support

[Andy Gross]

This patch adds TCSR support for use by the GSBI to automatically configure ADM
CRCI values based on the GSBI port configuration.

Signed-off-by: Andy Gross <agross@codeaurora.org>
---
 arch/arm/boot/dts/qcom-apq8064.dtsi |   20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index b3154c0..f9a947c 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -165,7 +165,7 @@
 
 		gsbi1: gsbi@12440000 {
 			status = "disabled";
-			compatible = "qcom,gsbi-v1.0.0";
+			compatible = "qcom,gsbi-apq8064";
 			reg = <0x12440000 0x100>;
 			clocks = <&gcc GSBI1_H_CLK>;
 			clock-names = "iface";
@@ -173,6 +173,9 @@
 			#size-cells = <1>;
 			ranges;
 
+			qcom,gsbi-num = <1>;
+			syscon-tcsr = <&tcsr>;
+
 			i2c1: i2c@12460000 {
 				compatible = "qcom,i2c-qup-v1.1.1";
 				reg = <0x12460000 0x1000>;
@@ -186,7 +189,7 @@
 
 		gsbi2: gsbi@12480000 {
 			status = "disabled";
-			compatible = "qcom,gsbi-v1.0.0";
+			compatible = "qcom,gsbi-apq8064";
 			reg = <0x12480000 0x100>;
 			clocks = <&gcc GSBI2_H_CLK>;
 			clock-names = "iface";
@@ -194,6 +197,9 @@
 			#size-cells = <1>;
 			ranges;
 
+			qcom,gsbi-num = <2>;
+			syscon-tcsr = <&tcsr>;
+
 			i2c2: i2c@124a0000 {
 				compatible = "qcom,i2c-qup-v1.1.1";
 				reg = <0x124a0000 0x1000>;
@@ -207,7 +213,7 @@
 
 		gsbi7: gsbi@16600000 {
 			status = "disabled";
-			compatible = "qcom,gsbi-v1.0.0";
+			compatible = "qcom,gsbi-apq8064";
 			reg = <0x16600000 0x100>;
 			clocks = <&gcc GSBI7_H_CLK>;
 			clock-names = "iface";
@@ -215,6 +221,9 @@
 			#size-cells = <1>;
 			ranges;
 
+			qcom,gsbi-num = <7>;
+			syscon-tcsr = <&tcsr>;
+
 			serial@16640000 {
 				compatible = "qcom,msm-uartdm-v1.3", "qcom,msm-uartdm";
 				reg = <0x16640000 0x1000>,
@@ -349,5 +358,10 @@
 				pinctrl-0 = <&sdc4_gpios>;
 			};
 		};
+
+		tcsr: syscon@1a400000 {
+			compatible = "qcom,tcsr-apq8064", "syscon";
+			reg = <0x1a400000 0x100>;
+		};
 	};
 };
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 4/6] ARM: DT: ipq8064: Add TCSR support

[Andy Gross]

This patch adds TCSR support for use by the GSBI to automatically configure ADM
CRCI values based on the GSBI port configuration.

Signed-off-by: Andy Gross <agross@codeaurora.org>
---
 arch/arm/boot/dts/qcom-ipq8064.dtsi |   20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
index 63b2146..e47358d 100644
--- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
@@ -119,7 +119,7 @@
 		};
 
 		gsbi2: gsbi@12480000 {
-			compatible = "qcom,gsbi-v1.0.0";
+			compatible = "qcom,gsbi-ipq8064";
 			reg = <0x12480000 0x100>;
 			clocks = <&gcc GSBI2_H_CLK>;
 			clock-names = "iface";
@@ -128,6 +128,9 @@
 			ranges;
 			status = "disabled";
 
+			qcom,gsbi-num = <2>;
+			syscon-tcsr = <&tcsr>;
+
 			serial@12490000 {
 				compatible = "qcom,msm-uartdm-v1.3", "qcom,msm-uartdm";
 				reg = <0x12490000 0x1000>,
@@ -154,7 +157,7 @@
 		};
 
 		gsbi4: gsbi@16300000 {
-			compatible = "qcom,gsbi-v1.0.0";
+			compatible = "qcom,gsbi-ipq8064";
 			reg = <0x16300000 0x100>;
 			clocks = <&gcc GSBI4_H_CLK>;
 			clock-names = "iface";
@@ -163,6 +166,9 @@
 			ranges;
 			status = "disabled";
 
+			qcom,gsbi-num = <4>;
+			syscon-tcsr = <&tcsr>;
+
 			serial@16340000 {
 				compatible = "qcom,msm-uartdm-v1.3", "qcom,msm-uartdm";
 				reg = <0x16340000 0x1000>,
@@ -188,7 +194,7 @@
 		};
 
 		gsbi5: gsbi@1a200000 {
-			compatible = "qcom,gsbi-v1.0.0";
+			compatible = "qcom,gsbi-ipq8064";
 			reg = <0x1a200000 0x100>;
 			clocks = <&gcc GSBI5_H_CLK>;
 			clock-names = "iface";
@@ -197,6 +203,9 @@
 			ranges;
 			status = "disabled";
 
+			qcom,gsbi-num = <5>;
+			syscon-tcsr = <&tcsr>;
+
 			serial@1a240000 {
 				compatible = "qcom,msm-uartdm-v1.3", "qcom,msm-uartdm";
 				reg = <0x1a240000 0x1000>,
@@ -279,5 +288,10 @@
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 		};
+
+		tcsr: syscon@1a400000 {
+			compatible = "qcom,tcsr-ipq8064", "syscon";
+			reg = <0x1a400000 0x100>;
+		};
 	};
 };
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 5/6] ARM: DT: msm8660: Add TCSR support

[Andy Gross]

This patch adds TCSR support for use by the GSBI to automatically configure ADM
CRCI values based on the GSBI port configuration.

Signed-off-by: Andy Gross <agross@codeaurora.org>
---
 arch/arm/boot/dts/qcom-msm8660.dtsi |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi
index 0affd61..c162736 100644
--- a/arch/arm/boot/dts/qcom-msm8660.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8660.dtsi
@@ -81,7 +81,7 @@
 		};
 
 		gsbi12: gsbi@19c00000 {
-			compatible = "qcom,gsbi-v1.0.0";
+			compatible = "qcom,gsbi-msm8660";
 			reg = <0x19c00000 0x100>;
 			clocks = <&gcc GSBI12_H_CLK>;
 			clock-names = "iface";
@@ -89,6 +89,9 @@
 			#size-cells = <1>;
 			ranges;
 
+			qcom,gsbi-num = <12>;
+			syscon-tcsr = <&tcsr>;
+
 			serial@19c40000 {
 				compatible = "qcom,msm-uartdm-v1.3", "qcom,msm-uartdm";
 				reg = <0x19c40000 0x1000>,
@@ -196,6 +199,11 @@
 				vmmc-supply = <&vsdcc_fixed>;
 			};
 		};
+
+		tcsr: syscon@1a400000 {
+			compatible = "qcom,tcsr-msm8660", "syscon";
+			reg = <0x1a400000 0x100>;
+		};
 	};
 
 };
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 6/6] ARM: DT: msm8960: Add TCSR support

[Andy Gross]

This patch adds TCSR support for use by the GSBI to automatically configure ADM
CRCI values based on the GSBI port configuration.

Signed-off-by: Andy Gross <agross@codeaurora.org>
---
 arch/arm/boot/dts/qcom-msm8960.dtsi |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/qcom-msm8960.dtsi b/arch/arm/boot/dts/qcom-msm8960.dtsi
index e1b0d5c..913a9f1 100644
--- a/arch/arm/boot/dts/qcom-msm8960.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8960.dtsi
@@ -121,7 +121,7 @@
 		};
 
 		gsbi5: gsbi@16400000 {
-			compatible = "qcom,gsbi-v1.0.0";
+			compatible = "qcom,gsbi-msm8960";
 			reg = <0x16400000 0x100>;
 			clocks = <&gcc GSBI5_H_CLK>;
 			clock-names = "iface";
@@ -129,6 +129,9 @@
 			#size-cells = <1>;
 			ranges;
 
+			qcom,gsbi-num = <5>;
+			syscon-tcsr = <&tcsr>;
+
 			serial@16440000 {
 				compatible = "qcom,msm-uartdm-v1.3", "qcom,msm-uartdm";
 				reg = <0x16440000 0x1000>,
@@ -238,5 +241,10 @@
 				vmmc-supply = <&vsdcc_fixed>;
 			};
 		};
+
+		tcsr: syscon@1a400000 {
+			compatible = "qcom,tcsr-msm8960", "syscon";
+			reg = <0x1a400000 0x100>;
+		};
 	};
 };
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation