2014-04-21 05:31:32

by Andy Gross

[permalink] [raw]
Subject: [PATCH 0/4] Introduce drivers/soc and add QCOM GSBI driver

The first patch in this set adds the drivers/soc directory and all the necessary
plumbing. These changes were discussed at the kernel summit and also were
introduced in an earlier patch set from Santosh Shilimkar.

Reference the following set of patches:
https://lkml.org/lkml/2014/2/28/567

The remaining patches add the QCOM GSBI (General Serial Bus Interface) driver,
device tree binding information for both the GSBI and child node interaction,
and lastly a patch to fix the current MSM serial driver to work correctly with
the GSBI changes.

Before this patch series, serial drivers (UART, I2C, and SPI) were all directly
accessing the overarching mux control settings for the parent GSBI device. This
leads to unfortunate interactions when you want a UART and I2C device which
share the same GSBI interface. By moving the serial devices to child nodes of
the GSBI, we can get the right mode setting for the ports and keep the children
from accessing the GSBI directly.

Andy Gross (4):
soc: Placeholder files for drivers/soc
soc: qcom: Add GSBI driver
soc: qcom: Add device tree binding for GSBI
tty: serial: msm: Remove direct access to GSBI

.../devicetree/bindings/soc/qcom/qcom,gsbi.txt | 78 +++++++++++++++
drivers/Kconfig | 2 +
drivers/Makefile | 4 +
drivers/soc/Kconfig | 5 +
drivers/soc/Makefile | 5 +
drivers/soc/qcom/Kconfig | 11 +++
drivers/soc/qcom/Makefile | 1 +
drivers/soc/qcom/qcom_gsbi.c | 101 ++++++++++++++++++++
drivers/tty/serial/msm_serial.c | 48 +---------
drivers/tty/serial/msm_serial.h | 5 -
10 files changed, 209 insertions(+), 51 deletions(-)
create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt
create mode 100644 drivers/soc/Kconfig
create mode 100644 drivers/soc/Makefile
create mode 100644 drivers/soc/qcom/Kconfig
create mode 100644 drivers/soc/qcom/Makefile
create mode 100644 drivers/soc/qcom/qcom_gsbi.c

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


2014-04-21 05:32:13

by Andy Gross

[permalink] [raw]
Subject: [PATCH 2/4] soc: qcom: Add GSBI driver

The GSBI (General Serial Bus Interface) driver controls the overarching
configuration of the shared serial bus infrastructure on APQ8064, IPQ8064, and
earlier QCOM processors. The GSBI supports UART, I2C, SPI, and UIM
functionality in various combinations.

Signed-off-by: Andy Gross <[email protected]>
---
drivers/soc/Kconfig | 3 +-
drivers/soc/Makefile | 5 +++
drivers/soc/qcom/Kconfig | 11 +++++
drivers/soc/qcom/Makefile | 1 +
drivers/soc/qcom/qcom_gsbi.c | 101 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 120 insertions(+), 1 deletion(-)
create mode 100644 drivers/soc/Makefile
create mode 100644 drivers/soc/qcom/Kconfig
create mode 100644 drivers/soc/qcom/Makefile
create mode 100644 drivers/soc/qcom/qcom_gsbi.c

diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index 58bd962..07a11be 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -1,4 +1,5 @@
menu "SOC specific Drivers"

+source "drivers/soc/qcom/Kconfig"
+
endmenu
-`
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
new file mode 100644
index 0000000..0f7c447
--- /dev/null
+++ b/drivers/soc/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for the Linux Kernel SOC specific device drivers.
+#
+
+obj-$(CONFIG_ARCH_QCOM) += qcom/
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
new file mode 100644
index 0000000..7bd2c94
--- /dev/null
+++ b/drivers/soc/qcom/Kconfig
@@ -0,0 +1,11 @@
+#
+# QCOM Soc drivers
+#
+config QCOM_GSBI
+ tristate "QCOM General Serial Bus Interface"
+ depends on ARCH_QCOM
+ help
+ Say y here to enable GSBI support. The GSBI provides control
+ functions for connecting the underlying serial UART, SPI, and I2C
+ devices to the output pins.
+
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
new file mode 100644
index 0000000..4389012
--- /dev/null
+++ b/drivers/soc/qcom/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o
diff --git a/drivers/soc/qcom/qcom_gsbi.c b/drivers/soc/qcom/qcom_gsbi.c
new file mode 100644
index 0000000..e8451a3
--- /dev/null
+++ b/drivers/soc/qcom/qcom_gsbi.c
@@ -0,0 +1,101 @@
+/*
+ * Copyright (c) 2014, The Linux foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License rev 2 and
+ * only rev 2 as published by the free Software foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or fITNESS fOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#define GSBI_CTRL_REG 0x0000
+#define GSBI_PROTOCOL_SHIFT 4
+
+struct gsbi_dev {
+ struct device *dev;
+ void __iomem *base;
+
+ struct clk *hclk;
+};
+
+static int gsbi_probe(struct platform_device *pdev)
+{
+ struct device_node *node = pdev->dev.of_node;
+ struct gsbi_dev *gsbi;
+ struct resource *res;
+ u32 mode;
+
+ gsbi = devm_kzalloc(&pdev->dev, sizeof(*gsbi), GFP_KERNEL);
+ if (!gsbi)
+ return -ENOMEM;
+
+ gsbi->dev = &pdev->dev;
+ platform_set_drvdata(pdev, gsbi);
+
+ if (of_property_read_u32(node, "qcom,mode", &mode)) {
+ dev_err(gsbi->dev, "missing mode configuration\n");
+ return -EINVAL;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ gsbi->base = devm_ioremap_resource(gsbi->dev, res);
+ if (IS_ERR(gsbi->base))
+ return PTR_ERR(gsbi->base);
+
+ gsbi->hclk = devm_clk_get(gsbi->dev, "iface");
+ if (IS_ERR(gsbi->hclk)) {
+ dev_err(gsbi->dev, "Could not get core clock\n");
+ return PTR_ERR(gsbi->hclk);
+ }
+ clk_prepare_enable(gsbi->hclk);
+
+ writel_relaxed((mode << GSBI_PROTOCOL_SHIFT), gsbi + GSBI_CTRL_REG);
+
+ /* make sure the gsbi control write is not reordered */
+ wmb();
+
+ return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+}
+
+static int gsbi_remove(struct platform_device *pdev)
+{
+ struct gsbi_dev *gsbi = platform_get_drvdata(pdev);
+
+ clk_disable_unprepare(gsbi->hclk);
+
+ return 0;
+}
+
+static 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",
+ .owner = THIS_MODULE,
+ .of_match_table = gsbi_dt_match,
+ },
+ .probe = gsbi_probe,
+ .remove = gsbi_remove,
+};
+
+module_platform_driver(gsbi_driver);
+
+MODULE_AUTHOR("Andy Gross <[email protected]>");
+MODULE_DESCRIPTION("QCOM GSBI driver");
+MODULE_LICENSE("GPL v2");
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2014-04-21 05:32:21

by Andy Gross

[permalink] [raw]
Subject: [PATCH 3/4] soc: qcom: Add device tree binding for GSBI

Add device tree binding support for the QCOM GSBI driver.

Signed-off-by: Andy Gross <[email protected]>
---
.../devicetree/bindings/soc/qcom/qcom,gsbi.txt | 78 ++++++++++++++++++++
1 file changed, 78 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt
new file mode 100644
index 0000000..6462e61
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt
@@ -0,0 +1,78 @@
+QCOM GSBI (General Serial Bus Interface) Driver
+
+The GSBI controller is modeled as a node with zero or more child nodes, each
+representing a serial sub-node device that is mux'd as part of the GSBI
+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
+- 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.
+ mode 0: idle, null values applied to all four GSBI IOs
+ mode 1: I2C on 2 ports, SIM/R-UIM on other 2.
+ mode 2: I2C
+ mode 3: SPI
+ mode 4: UART w/ flow control
+ mode 5: SIM/R-UIM
+ mode 6: I2C on 2 ports, UART (without flow control) on other 2
+ mode 7: undefined
+
+Required properties if child node exists:
+- #address-cells: Must be 1
+- #size-cells: Must be 1
+- ranges: Must be present
+
+Properties for children:
+
+A GSBI controller node can contain 0 or more child nodes representing serial
+devices. These serial devices can be a QCOM UART, I2C controller, spi
+controller, or some combination of aforementioned devices.
+
+See the following for child node definitions:
+Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
+Documentation/devicetree/bindings/spi/qcom,spi-qup.txt
+Documentation/devicetree/bindings/serial/qcom,msm-uartdm.txt
+
+Example for APQ8064:
+
+ gsbi4@16300000 {
+ compatible = "qcom,gsbi-v1.0.0";
+ reg = <0x16300000 0x100>;
+ clocks = <&gcc GSBI4_H_CLK>;
+ clock-names = "iface";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+ qcom,mode = <6>;
+
+ /* child nodes go under here */
+
+ i2c_qup4: i2c@16380000 {
+ compatible = "qcom,i2c-qup-v1.1.1";
+ reg = <0x16380000 0x1000>;
+ interrupts = <0 153 0>;
+
+ clocks = <&gcc GSBI4_QUP_CLK>, <&gcc GSBI4_H_CLK>;
+ clock-names = "core", "iface";
+
+ clock-frequency = <200000>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ };
+
+ uart4: serial@16340000 {
+ compatible = "qcom,msm-uartdm-v1.3", "qcom,msm-uartdm";
+ reg = <0x16340000 0x1000>,
+ <0x16300000 0x1000>;
+ interrupts = <0 152 0x0>;
+ clocks = <&gcc GSBI4_UART_CLK>, <&gcc GSBI4_H_CLK>;
+ clock-names = "core", "iface";
+ status = "ok";
+ };
+ };
+
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2014-04-21 05:32:31

by Andy Gross

[permalink] [raw]
Subject: [PATCH 1/4] soc: Placeholder files for drivers/soc

Add placeholder Kconfig and linkage for driver/soc.

The first patch set that implemented this was authored by Santosh Shilimkar:
https://lkml.org/lkml/2014/2/28/567

Signed-off-by: Andy Gross <[email protected]>
---
drivers/Kconfig | 2 ++
drivers/Makefile | 4 ++++
drivers/soc/Kconfig | 4 ++++
3 files changed, 10 insertions(+)
create mode 100644 drivers/soc/Kconfig

diff --git a/drivers/Kconfig b/drivers/Kconfig
index b3138fb..37f955f 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -130,6 +130,8 @@ source "drivers/staging/Kconfig"

source "drivers/platform/Kconfig"

+source "drivers/soc/Kconfig"
+
source "drivers/clk/Kconfig"

source "drivers/hwspinlock/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index 8e3b8b0..ba77e7b 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -132,6 +132,10 @@ obj-$(CONFIG_VHOST_RING) += vhost/
obj-$(CONFIG_VLYNQ) += vlynq/
obj-$(CONFIG_STAGING) += staging/
obj-y += platform/
+
+# soc specific drivers
+obj-y += soc/
+
#common clk code
obj-y += clk/

diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
new file mode 100644
index 0000000..58bd962
--- /dev/null
+++ b/drivers/soc/Kconfig
@@ -0,0 +1,4 @@
+menu "SOC specific Drivers"
+
+endmenu
+`
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2014-04-21 05:32:27

by Andy Gross

[permalink] [raw]
Subject: [PATCH 4/4] tty: serial: msm: Remove direct access to GSBI

This patch removes direct access of the GSBI registers. GSBI configuration
should be done through the GSBI driver directly.

Signed-off-by: Andy Gross <[email protected]>
---
drivers/tty/serial/msm_serial.c | 48 ++-------------------------------------
drivers/tty/serial/msm_serial.h | 5 ----
2 files changed, 2 insertions(+), 51 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index b5d779c..8901114 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -45,7 +45,6 @@ struct msm_port {
struct clk *clk;
struct clk *pclk;
unsigned int imr;
- void __iomem *gsbi_base;
int is_uartdm;
unsigned int old_snap_state;
};
@@ -586,9 +585,7 @@ static const char *msm_type(struct uart_port *port)
static void msm_release_port(struct uart_port *port)
{
struct platform_device *pdev = to_platform_device(port->dev);
- struct msm_port *msm_port = UART_TO_MSM(port);
struct resource *uart_resource;
- struct resource *gsbi_resource;
resource_size_t size;

uart_resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -599,28 +596,12 @@ static void msm_release_port(struct uart_port *port)
release_mem_region(port->mapbase, size);
iounmap(port->membase);
port->membase = NULL;
-
- if (msm_port->gsbi_base) {
- writel_relaxed(GSBI_PROTOCOL_IDLE,
- msm_port->gsbi_base + GSBI_CONTROL);
-
- gsbi_resource = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- if (unlikely(!gsbi_resource))
- return;
-
- size = resource_size(gsbi_resource);
- release_mem_region(gsbi_resource->start, size);
- iounmap(msm_port->gsbi_base);
- msm_port->gsbi_base = NULL;
- }
}

static int msm_request_port(struct uart_port *port)
{
- struct msm_port *msm_port = UART_TO_MSM(port);
struct platform_device *pdev = to_platform_device(port->dev);
struct resource *uart_resource;
- struct resource *gsbi_resource;
resource_size_t size;
int ret;

@@ -639,30 +620,8 @@ static int msm_request_port(struct uart_port *port)
goto fail_release_port;
}

- gsbi_resource = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- /* Is this a GSBI-based port? */
- if (gsbi_resource) {
- size = resource_size(gsbi_resource);
-
- if (!request_mem_region(gsbi_resource->start, size,
- "msm_serial")) {
- ret = -EBUSY;
- goto fail_release_port_membase;
- }
-
- msm_port->gsbi_base = ioremap(gsbi_resource->start, size);
- if (!msm_port->gsbi_base) {
- ret = -EBUSY;
- goto fail_release_gsbi;
- }
- }
-
return 0;

-fail_release_gsbi:
- release_mem_region(gsbi_resource->start, size);
-fail_release_port_membase:
- iounmap(port->membase);
fail_release_port:
release_mem_region(port->mapbase, size);
return ret;
@@ -670,7 +629,6 @@ fail_release_port:

static void msm_config_port(struct uart_port *port, int flags)
{
- struct msm_port *msm_port = UART_TO_MSM(port);
int ret;
if (flags & UART_CONFIG_TYPE) {
port->type = PORT_MSM;
@@ -678,9 +636,6 @@ static void msm_config_port(struct uart_port *port, int flags)
if (ret)
return;
}
- if (msm_port->gsbi_base)
- writel_relaxed(GSBI_PROTOCOL_UART,
- msm_port->gsbi_base + GSBI_CONTROL);
}

static int msm_verify_port(struct uart_port *port, struct serial_struct *ser)
@@ -976,6 +931,7 @@ static struct of_device_id msm_match_table[] = {

static struct platform_driver msm_platform_driver = {
.remove = msm_serial_remove,
+ .probe = msm_serial_probe,
.driver = {
.name = "msm_serial",
.owner = THIS_MODULE,
@@ -991,7 +947,7 @@ static int __init msm_serial_init(void)
if (unlikely(ret))
return ret;

- ret = platform_driver_probe(&msm_platform_driver, msm_serial_probe);
+ ret = platform_driver_register(&msm_platform_driver);
if (unlikely(ret))
uart_unregister_driver(&msm_uart_driver);

diff --git a/drivers/tty/serial/msm_serial.h b/drivers/tty/serial/msm_serial.h
index 469fda5..a77cc76 100644
--- a/drivers/tty/serial/msm_serial.h
+++ b/drivers/tty/serial/msm_serial.h
@@ -108,11 +108,6 @@
#define UART_ISR 0x0014
#define UART_ISR_TX_READY (1 << 7)

-#define GSBI_CONTROL 0x0
-#define GSBI_PROTOCOL_CODE 0x30
-#define GSBI_PROTOCOL_UART 0x40
-#define GSBI_PROTOCOL_IDLE 0x0
-
#define UARTDM_DMRX 0x34
#define UARTDM_NCF_TX 0x40
#define UARTDM_RX_TOTAL_SNAP 0x38
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

2014-04-21 13:48:22

by Christopher Covington

[permalink] [raw]
Subject: Re: [PATCH 0/4] Introduce drivers/soc and add QCOM GSBI driver

Hi Andy,

On 04/21/2014 01:30 AM, Andy Gross wrote:
> The first patch in this set adds the drivers/soc directory and all the necessary
> plumbing. These changes were discussed at the kernel summit and also were
> introduced in an earlier patch set from Santosh Shilimkar.
>
> Reference the following set of patches:
> https://lkml.org/lkml/2014/2/28/567

In that thread, Olof wrote, "The code [going into drivers/soc] isn't the pure
drivers. Those we find homes for."

https://lkml.org/lkml/2014/3/2/123

> The remaining patches add the QCOM GSBI (General Serial Bus Interface) driver,
> device tree binding information for both the GSBI and child node interaction,
> and lastly a patch to fix the current MSM serial driver to work correctly with
> the GSBI changes.

It's not obvious to me what makes the GSBI driver "impure" and unfit for say
drivers/bus. Could you perhaps include a brief explanation?

Thanks,
Christopher

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

2014-04-21 16:21:58

by Andy Gross

[permalink] [raw]
Subject: Re: [PATCH 0/4] Introduce drivers/soc and add QCOM GSBI driver

On Mon, Apr 21, 2014 at 09:48:16AM -0400, Christopher Covington wrote:

<snip>

> In that thread, Olof wrote, "The code [going into drivers/soc] isn't the pure
> drivers. Those we find homes for."

Right. I see this as glue for the most part. You could argue it's a small
pinctrl, but this doesn't route external pins, it's more to connect pins
internally to the IP block and it's sub-device parts (qup, uart, etc).

>
> https://lkml.org/lkml/2014/3/2/123
>
> > The remaining patches add the QCOM GSBI (General Serial Bus Interface) driver,
> > device tree binding information for both the GSBI and child node interaction,
> > and lastly a patch to fix the current MSM serial driver to work correctly with
> > the GSBI changes.
>
> It's not obvious to me what makes the GSBI driver "impure" and unfit for say
> drivers/bus. Could you perhaps include a brief explanation?

The intent of the gsbi driver is merely to provide/configure the single mux
setting for the devices that share those 4 IO lines. I don't see this as a full
blown bus.

Regards,

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

2014-04-21 16:55:51

by Kumar Gala

[permalink] [raw]
Subject: Re: [PATCH 3/4] soc: qcom: Add device tree binding for GSBI


On Apr 21, 2014, at 12:30 AM, Andy Gross <[email protected]> wrote:

> Add device tree binding support for the QCOM GSBI driver.
>
> Signed-off-by: Andy Gross <[email protected]>
> ---
> .../devicetree/bindings/soc/qcom/qcom,gsbi.txt | 78 ++++++++++++++++++++
> 1 file changed, 78 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt
>
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt
> new file mode 100644
> index 0000000..6462e61
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,gsbi.txt
> @@ -0,0 +1,78 @@
> +QCOM GSBI (General Serial Bus Interface) Driver
> +
> +The GSBI controller is modeled as a node with zero or more child nodes, each
> +representing a serial sub-node device that is mux'd as part of the GSBI
> +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
> +- 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.
> + mode 0: idle, null values applied to all four GSBI IOs
> + mode 1: I2C on 2 ports, SIM/R-UIM on other 2.
> + mode 2: I2C
> + mode 3: SPI
> + mode 4: UART w/ flow control
> + mode 5: SIM/R-UIM
> + mode 6: I2C on 2 ports, UART (without flow control) on other 2
> + mode 7: undefined

Can we add defines in a include/dt-bindings for these modes.

> +
> +Required properties if child node exists:
> +- #address-cells: Must be 1
> +- #size-cells: Must be 1
> +- ranges: Must be present
> +
> +Properties for children:
> +
> +A GSBI controller node can contain 0 or more child nodes representing serial
> +devices. These serial devices can be a QCOM UART, I2C controller, spi
> +controller, or some combination of aforementioned devices.
> +
> +See the following for child node definitions:
> +Documentation/devicetree/bindings/i2c/qcom,i2c-qup.txt
> +Documentation/devicetree/bindings/spi/qcom,spi-qup.txt
> +Documentation/devicetree/bindings/serial/qcom,msm-uartdm.txt
> +
> +Example for APQ8064:
> +
> + gsbi4@16300000 {
> + compatible = "qcom,gsbi-v1.0.0";
> + reg = <0x16300000 0x100>;
> + clocks = <&gcc GSBI4_H_CLK>;
> + clock-names = "iface";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> + qcom,mode = <6>;
> +
> + /* child nodes go under here */
> +
> + i2c_qup4: i2c@16380000 {
> + compatible = "qcom,i2c-qup-v1.1.1";
> + reg = <0x16380000 0x1000>;
> + interrupts = <0 153 0>;
> +
> + clocks = <&gcc GSBI4_QUP_CLK>, <&gcc GSBI4_H_CLK>;
> + clock-names = "core", "iface";
> +
> + clock-frequency = <200000>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + };
> +
> + uart4: serial@16340000 {
> + compatible = "qcom,msm-uartdm-v1.3", "qcom,msm-uartdm";
> + reg = <0x16340000 0x1000>,
> + <0x16300000 0x1000>;
> + interrupts = <0 152 0x0>;
> + clocks = <&gcc GSBI4_UART_CLK>, <&gcc GSBI4_H_CLK>;
> + clock-names = "core", "iface";
> + status = "ok";
> + };
> + };
> +
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

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

2014-04-21 16:57:22

by Josh Cartwright

[permalink] [raw]
Subject: Re: [PATCH 2/4] soc: qcom: Add GSBI driver

On Mon, Apr 21, 2014 at 12:30:42AM -0500, Andy Gross wrote:
> The GSBI (General Serial Bus Interface) driver controls the overarching
> configuration of the shared serial bus infrastructure on APQ8064, IPQ8064, and
> earlier QCOM processors. The GSBI supports UART, I2C, SPI, and UIM
> functionality in various combinations.
>
> Signed-off-by: Andy Gross <[email protected]>
[..]
> +++ b/drivers/soc/qcom/qcom_gsbi.c
[..]
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +
> +#define GSBI_CTRL_REG 0x0000
> +#define GSBI_PROTOCOL_SHIFT 4
> +
> +struct gsbi_dev {
> + struct device *dev;
> + void __iomem *base;

You don't really need these.

> +
> + struct clk *hclk;
> +};
> +
> +static int gsbi_probe(struct platform_device *pdev)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct gsbi_dev *gsbi;
> + struct resource *res;
> + u32 mode;
> +
> + gsbi = devm_kzalloc(&pdev->dev, sizeof(*gsbi), GFP_KERNEL);
> + if (!gsbi)
> + return -ENOMEM;
> +
> + gsbi->dev = &pdev->dev;
> + platform_set_drvdata(pdev, gsbi);
> +
> + if (of_property_read_u32(node, "qcom,mode", &mode)) {
> + dev_err(gsbi->dev, "missing mode configuration\n");
> + return -EINVAL;
> + }

I'm wondering if you should really be a (very simple) pinctrl driver
proper.

> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + gsbi->base = devm_ioremap_resource(gsbi->dev, res);
> + if (IS_ERR(gsbi->base))
> + return PTR_ERR(gsbi->base);
> +
> + gsbi->hclk = devm_clk_get(gsbi->dev, "iface");
> + if (IS_ERR(gsbi->hclk)) {
> + dev_err(gsbi->dev, "Could not get core clock\n");
> + return PTR_ERR(gsbi->hclk);
> + }
> + clk_prepare_enable(gsbi->hclk);
> +
> + writel_relaxed((mode << GSBI_PROTOCOL_SHIFT), gsbi + GSBI_CTRL_REG);

Did you mean: gsbi->base + GSBI_CTRL_REG ?

> +
> + /* make sure the gsbi control write is not reordered */
> + wmb();
> +
> + return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> +}
> +
> +static int gsbi_remove(struct platform_device *pdev)
> +{
> + struct gsbi_dev *gsbi = platform_get_drvdata(pdev);
> +
> + clk_disable_unprepare(gsbi->hclk);
> +
> + return 0;
> +}
> +
> +static struct of_device_id gsbi_dt_match[] = {

const

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

2014-04-21 17:11:25

by Andy Gross

[permalink] [raw]
Subject: Re: [PATCH 2/4] soc: qcom: Add GSBI driver

On Mon, Apr 21, 2014 at 11:54:00AM -0500, Josh Cartwright wrote:

<snip>

> > +
> > +struct gsbi_dev {
> > + struct device *dev;
> > + void __iomem *base;
>
> You don't really need these.

Old habits die hard. I'll remove.

<snip>

> > + if (of_property_read_u32(node, "qcom,mode", &mode)) {
> > + dev_err(gsbi->dev, "missing mode configuration\n");
> > + return -EINVAL;
> > + }
>
> I'm wondering if you should really be a (very simple) pinctrl driver
> proper.

Perhaps. But how would i reconcile more than one device node that uses the same
GSBI? One could still trounce the other unless I only allow one setting of the
GSBI.

<snip>

> > + clk_prepare_enable(gsbi->hclk);
> > +
> > + writel_relaxed((mode << GSBI_PROTOCOL_SHIFT), gsbi + GSBI_CTRL_REG);
>
> Did you mean: gsbi->base + GSBI_CTRL_REG ?

Ouch, how did this get munged. I'll fix this on resend.

<snip>

> > +
> > +static struct of_device_id gsbi_dt_match[] = {
>
> const

Will fix.

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

2014-04-21 17:29:28

by Josh Cartwright

[permalink] [raw]
Subject: Re: [PATCH 2/4] soc: qcom: Add GSBI driver

On Mon, Apr 21, 2014 at 12:11:18PM -0500, Andy Gross wrote:
> On Mon, Apr 21, 2014 at 11:54:00AM -0500, Josh Cartwright wrote:
> > > + if (of_property_read_u32(node, "qcom,mode", &mode)) {
> > > + dev_err(gsbi->dev, "missing mode configuration\n");
> > > + return -EINVAL;
> > > + }
> >
> > I'm wondering if you should really be a (very simple) pinctrl driver
> > proper.
>
> Perhaps. But how would i reconcile more than one device node that uses the same
> GSBI? One could still trounce the other unless I only allow one setting of the
> GSBI.
>

I don't understand, as long as the pins/functions have been specified
properly to the pinctrl core, I would expect a conflicting configuration
to be rejected.

Anyway, I wouldn't expect the subnodes to be consuming the GSBI pin
configuration anyway (although that could probably be done), instead, I
would expect the GSBI node to consume it's own pin configuration.

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