* [PATCH v4 1/3] dt: bindings: add documentation for zx2967 family watchdog controller
From: Baoyou Xie @ 2017-01-22 8:16 UTC (permalink / raw)
To: jun.nie, wim, linux, robh+dt, mark.rutland
Cc: linux-arm-kernel, linux-watchdog, devicetree, linux-kernel,
shawnguo, baoyou.xie, xie.baoyou, chen.chaokai, wang.qiang01
This patch adds dt-binding documentation for zx2967 family
watchdog controller.
Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
.../bindings/watchdog/zte,zx2967-wdt.txt | 31 ++++++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/watchdog/zte,zx2967-wdt.txt
diff --git a/Documentation/devicetree/bindings/watchdog/zte,zx2967-wdt.txt b/Documentation/devicetree/bindings/watchdog/zte,zx2967-wdt.txt
new file mode 100644
index 0000000..772403a
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/zte,zx2967-wdt.txt
@@ -0,0 +1,31 @@
+ZTE zx2967 Watchdog timer
+
+Required properties:
+
+- compatible : should be one of the following.
+ * zte,zx296718-wdt
+- reg : Specifies base physical address and size of the registers.
+- clocks : Pairs of phandle and specifier referencing the controller's clocks.
+- resets : Reference to the reset controller controlling the watchdog
+ controller.
+
+Optional properties:
+
+- zte,wdt-reset-sysctrl : Directs how to reset system by the watchdog.
+ if we don't want to restart system when watchdog been triggered,
+ it's not required, vice versa.
+ It should include following fields.
+ * phandle of aon-sysctrl.
+ * offset of register that be written, should be 0xb0.
+ * configure value that be written to aon-sysctrl.
+ * bit mask, corresponding bits will be affected.
+
+Example:
+
+wdt: watchdog@1465000 {
+ compatible = "zte,zx296718-wdt";
+ reg = <0x1465000 0x1000>;
+ clocks = <&topcrm WDT_WCLK>;
+ resets = <&toprst 35>;
+ zte,wdt-reset-sysctrl = <&aon_sysctrl 0xb0 1 0x115>;
+};
--
2.7.4
^ permalink raw reply related
* [PATCH v4 2/3] MAINTAINERS: add zx2967 watchdog controller driver to ARM ZTE architecture
From: Baoyou Xie @ 2017-01-22 8:16 UTC (permalink / raw)
To: jun.nie-QSEj5FYQhm4dnm+yROfE0A, wim-IQzOog9fTRqzQB+pC5nmwQ,
linux-0h96xk9xTtrk1uMJSBkQmQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A,
baoyou.xie-QSEj5FYQhm4dnm+yROfE0A,
xie.baoyou-Th6q7B73Y6EnDS1+zs4M5A,
chen.chaokai-Th6q7B73Y6EnDS1+zs4M5A,
wang.qiang01-Th6q7B73Y6EnDS1+zs4M5A
In-Reply-To: <1485073008-26699-1-git-send-email-baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Add the zx2967 watchdog controller driver as maintained by ARM ZTE
architecture maintainers, as they're parts of the core IP.
Signed-off-by: Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index edfdea3..275c434 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1990,11 +1990,13 @@ F: drivers/clk/zte/
F: drivers/reset/reset-zx2967.c
F: drivers/soc/zte/
F: drivers/thermal/zx*
+F: drivers/watchdog/zx2967_wdt.c
F: Documentation/devicetree/bindings/arm/zte.txt
F: Documentation/devicetree/bindings/clock/zx296702-clk.txt
F: Documentation/devicetree/bindings/reset/zte,zx2967-reset.txt
F: Documentation/devicetree/bindings/soc/zte/
F: Documentation/devicetree/bindings/thermal/zx*
+F: Documentation/devicetree/bindings/watchdog/zte,zx2967-wdt.txt
F: include/dt-bindings/soc/zx*.h
ARM/ZYNQ ARCHITECTURE
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v4 3/3] watchdog: zx2967: add watchdog controller driver for ZTE's zx2967 family
From: Baoyou Xie @ 2017-01-22 8:16 UTC (permalink / raw)
To: jun.nie, wim, linux, robh+dt, mark.rutland
Cc: linux-arm-kernel, linux-watchdog, devicetree, linux-kernel,
shawnguo, baoyou.xie, xie.baoyou, chen.chaokai, wang.qiang01
In-Reply-To: <1485073008-26699-1-git-send-email-baoyou.xie@linaro.org>
This patch adds watchdog controller driver for ZTE's zx2967 family.
Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
drivers/watchdog/Kconfig | 10 ++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/zx2967_wdt.c | 282 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 293 insertions(+)
create mode 100644 drivers/watchdog/zx2967_wdt.c
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index acb00b5..05093a2 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -714,6 +714,16 @@ config ASPEED_WATCHDOG
To compile this driver as a module, choose M here: the
module will be called aspeed_wdt.
+config ZX2967_WATCHDOG
+ tristate "ZTE zx2967 SoCs watchdog support"
+ depends on ARCH_ZX
+ select WATCHDOG_CORE
+ help
+ Say Y here to include support for the watchdog timer
+ in ZTE zx2967 SoCs.
+ To compile this driver as a module, choose M here: the
+ module will be called zx2967_wdt.
+
# AVR32 Architecture
config AT32AP700X_WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 0c3d35e..bf2d296 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_BCM7038_WDT) += bcm7038_wdt.o
obj-$(CONFIG_ATLAS7_WATCHDOG) += atlas7_wdt.o
obj-$(CONFIG_RENESAS_WDT) += renesas_wdt.o
obj-$(CONFIG_ASPEED_WATCHDOG) += aspeed_wdt.o
+obj-$(CONFIG_ZX2967_WATCHDOG) += zx2967_wdt.o
# AVR32 Architecture
obj-$(CONFIG_AT32AP700X_WDT) += at32ap700x_wdt.o
diff --git a/drivers/watchdog/zx2967_wdt.c b/drivers/watchdog/zx2967_wdt.c
new file mode 100644
index 0000000..7340605
--- /dev/null
+++ b/drivers/watchdog/zx2967_wdt.c
@@ -0,0 +1,282 @@
+/*
+ * watchdog driver for ZTE's zx2967 family
+ *
+ * Copyright (C) 2017 ZTE Ltd.
+ *
+ * Author: Baoyou Xie <baoyou.xie@linaro.org>
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/watchdog.h>
+
+#define ZX2967_WDT_CFG_REG 0x4
+#define ZX2967_WDT_LOAD_REG 0x8
+#define ZX2967_WDT_REFRESH_REG 0x18
+#define ZX2967_WDT_START_REG 0x1c
+
+#define ZX2967_WDT_REFRESH_MASK 0x3f
+
+#define ZX2967_WDT_CFG_DIV(n) ((((n) & 0xff) - 1) << 8)
+#define ZX2967_WDT_START_EN 0x1
+
+#define ZX2967_WDT_WRITEKEY 0x12340000
+
+#define ZX2967_WDT_DIV_DEFAULT 16
+#define ZX2967_WDT_DEFAULT_TIMEOUT 32
+#define ZX2967_WDT_MIN_TIMEOUT 1
+#define ZX2967_WDT_MAX_TIMEOUT 500
+#define ZX2967_WDT_MAX_COUNT 0xffff
+
+#define ZX2967_WDT_CLK_FREQ 0x8000
+
+#define ZX2967_WDT_FLAG_REBOOT_MON BIT(0)
+
+struct zx2967_wdt {
+ struct clk *clock;
+ void __iomem *reg_base;
+ unsigned int conf;
+ unsigned int load;
+ unsigned int flags;
+ struct watchdog_device wdt_device;
+ struct notifier_block restart_handler;
+};
+
+static inline u32 zx2967_wdt_readl(struct zx2967_wdt *wdt, u16 reg)
+{
+ return readl_relaxed(wdt->reg_base + reg);
+}
+
+static inline void zx2967_wdt_writel(struct zx2967_wdt *wdt, u16 reg, u32 val)
+{
+ writel_relaxed(val | ZX2967_WDT_WRITEKEY, wdt->reg_base + reg);
+}
+
+static void zx2967_wdt_refresh(struct zx2967_wdt *wdt)
+{
+ u32 val;
+
+ val = zx2967_wdt_readl(wdt, ZX2967_WDT_REFRESH_REG);
+ val ^= ZX2967_WDT_REFRESH_MASK;
+ zx2967_wdt_writel(wdt, ZX2967_WDT_REFRESH_REG, val);
+}
+
+static int
+zx2967_wdt_set_timeout(struct watchdog_device *wdd, unsigned int timeout)
+{
+ struct zx2967_wdt *wdt = watchdog_get_drvdata(wdd);
+ unsigned int divisor = ZX2967_WDT_DIV_DEFAULT;
+ unsigned int count;
+
+ count = timeout * ZX2967_WDT_CLK_FREQ;
+ if (count > divisor * ZX2967_WDT_MAX_COUNT)
+ divisor = DIV_ROUND_UP(count, ZX2967_WDT_MAX_COUNT);
+ count = DIV_ROUND_UP(count, divisor);
+ zx2967_wdt_writel(wdt, ZX2967_WDT_CFG_REG, ZX2967_WDT_CFG_DIV(divisor));
+ zx2967_wdt_writel(wdt, ZX2967_WDT_LOAD_REG, count);
+ zx2967_wdt_refresh(wdt);
+ wdt->load = count;
+ wdd->timeout = (count * divisor) / ZX2967_WDT_CLK_FREQ;
+
+ return 0;
+}
+
+static void __zx2967_wdt_start(struct zx2967_wdt *wdt)
+{
+ u32 val;
+
+ val = zx2967_wdt_readl(wdt, ZX2967_WDT_START_REG);
+ val |= ZX2967_WDT_START_EN;
+ zx2967_wdt_writel(wdt, ZX2967_WDT_START_REG, val);
+}
+
+static void __zx2967_wdt_stop(struct zx2967_wdt *wdt)
+{
+ u32 val;
+
+ val = zx2967_wdt_readl(wdt, ZX2967_WDT_START_REG);
+ val &= ~ZX2967_WDT_START_EN;
+ zx2967_wdt_writel(wdt, ZX2967_WDT_START_REG, val);
+}
+
+static int zx2967_wdt_start(struct watchdog_device *wdd)
+{
+ struct zx2967_wdt *wdt = watchdog_get_drvdata(wdd);
+
+ zx2967_wdt_set_timeout(wdd, wdd->timeout);
+ __zx2967_wdt_start(wdt);
+
+ return 0;
+}
+
+static int zx2967_wdt_stop(struct watchdog_device *wdd)
+{
+ struct zx2967_wdt *wdt = watchdog_get_drvdata(wdd);
+
+ __zx2967_wdt_stop(wdt);
+
+ return 0;
+}
+
+static int zx2967_wdt_keepalive(struct watchdog_device *wdd)
+{
+ struct zx2967_wdt *wdt = watchdog_get_drvdata(wdd);
+
+ zx2967_wdt_refresh(wdt);
+
+ return 0;
+}
+
+#define ZX2967_WDT_OPTIONS \
+ (WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE)
+static const struct watchdog_info zx2967_wdt_ident = {
+ .options = ZX2967_WDT_OPTIONS,
+ .firmware_version = 0,
+ .identity = "zx2967 watchdog",
+};
+
+static struct watchdog_ops zx2967_wdt_ops = {
+ .owner = THIS_MODULE,
+ .start = zx2967_wdt_start,
+ .stop = zx2967_wdt_stop,
+ .ping = zx2967_wdt_keepalive,
+ .set_timeout = zx2967_wdt_set_timeout,
+};
+
+static void zx2967_wdt_reset_sysctrl(struct device *dev)
+{
+ int ret;
+ struct device_node *np = NULL;
+ void __iomem *regmap;
+ unsigned int offset, mask, config;
+ struct of_phandle_args out_args;
+
+ ret = of_parse_phandle_with_fixed_args(dev->of_node,
+ "zte,wdt-reset-sysctrl", 3, 0, &out_args);
+ if (ret)
+ return;
+
+ offset = out_args.args[0];
+ config = out_args.args[1];
+ mask = out_args.args[2];
+
+ regmap = syscon_node_to_regmap(out_args.np);
+ if (IS_ERR(regmap))
+ goto out;
+
+ regmap_update_bits(regmap, offset, mask, config);
+out:
+ of_node_put(np);
+}
+
+static int zx2967_wdt_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct zx2967_wdt *wdt;
+ struct resource *base;
+ int ret;
+ struct reset_control *rstc;
+
+ wdt = devm_kzalloc(dev, sizeof(*wdt), GFP_KERNEL);
+ if (!wdt)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, wdt);
+
+ wdt->wdt_device.info = &zx2967_wdt_ident;
+ wdt->wdt_device.ops = &zx2967_wdt_ops;
+ wdt->wdt_device.timeout = ZX2967_WDT_DEFAULT_TIMEOUT;
+ wdt->wdt_device.max_timeout = ZX2967_WDT_MAX_TIMEOUT;
+ wdt->wdt_device.min_timeout = ZX2967_WDT_MIN_TIMEOUT;
+ wdt->wdt_device.parent = &pdev->dev;
+
+ base = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ wdt->reg_base = devm_ioremap_resource(dev, base);
+ if (IS_ERR(wdt->reg_base)) {
+ dev_err(dev, "ioremap failed\n");
+ return PTR_ERR(wdt->reg_base);
+ }
+
+ zx2967_wdt_reset_sysctrl(dev);
+
+ wdt->clock = devm_clk_get(dev, NULL);
+ if (IS_ERR(wdt->clock)) {
+ dev_err(dev, "failed to find watchdog clock source\n");
+ return PTR_ERR(wdt->clock);
+ }
+
+ ret = clk_prepare_enable(wdt->clock);
+ if (ret < 0) {
+ dev_err(dev, "failed to enable clock\n");
+ return ret;
+ }
+ clk_set_rate(wdt->clock, ZX2967_WDT_CLK_FREQ);
+
+ rstc = devm_reset_control_get(dev, NULL);
+ if (IS_ERR(rstc)) {
+ dev_err(dev, "failed to get rstc");
+ ret = PTR_ERR(rstc);
+ goto fail_get_reset_control;
+ }
+
+ reset_control_assert(rstc);
+ mdelay(10);
+ reset_control_deassert(rstc);
+
+ watchdog_set_drvdata(&wdt->wdt_device, wdt);
+ watchdog_set_nowayout(&wdt->wdt_device, WATCHDOG_NOWAYOUT);
+
+ ret = watchdog_register_device(&wdt->wdt_device);
+ if (ret)
+ goto fail_register;
+
+ dev_info(dev, "watchdog enabled (timeout=%d sec, nowayout=%d)",
+ wdt->wdt_device.timeout, WATCHDOG_NOWAYOUT);
+
+ return 0;
+
+fail_get_reset_control:
+ watchdog_unregister_device(&wdt->wdt_device);
+fail_register:
+ clk_disable_unprepare(wdt->clock);
+ return ret;
+}
+
+static int zx2967_wdt_remove(struct platform_device *pdev)
+{
+ struct zx2967_wdt *wdt = platform_get_drvdata(pdev);
+
+ watchdog_unregister_device(&wdt->wdt_device);
+ clk_disable_unprepare(wdt->clock);
+
+ return 0;
+}
+
+static const struct of_device_id zx2967_wdt_match[] = {
+ { .compatible = "zte,zx296718-wdt", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, zx2967_wdt_match);
+
+static struct platform_driver zx2967_wdt_driver = {
+ .probe = zx2967_wdt_probe,
+ .remove = zx2967_wdt_remove,
+ .driver = {
+ .name = "zx2967-wdt",
+ .of_match_table = of_match_ptr(zx2967_wdt_match),
+ },
+};
+module_platform_driver(zx2967_wdt_driver);
+
+MODULE_AUTHOR("Baoyou Xie <baoyou.xie@linaro.org>");
+MODULE_DESCRIPTION("ZTE zx2967 Watchdog Device Driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] iio:temperature:Add irq and threshold events support for TI TMP007 sensor
From: Peter Meerwald-Stadler @ 2017-01-22 9:19 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: jic23-DgEjT+Ai2ygdnm+yROfE0A, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170121150147.GA7781-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
I'd prefer subject "iio:temperature:tmp007: Add irq and threshold events support"
> This patch adds support for ALERT irq and limit threshold events for TI TMP007 - 16 bit IR thermopile sensor
> with integrated math engine.
>
> Following threshold events are supported:
> 1. TObj high limit
> 2. TObj low limit
> 3. TDie high limit
> 4. TDie low limit
comments below
I suggest to separate clearup from actually adding events support, so two
patches
it would be nice if the driver can operate without irq also (irq is
optional in DT...)
> Signed-off-by: Manivannan Sadhasivam <manivannanece23-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
note: this applies on top of patch...
> .../devicetree/bindings/iio/temperature/tmp007.txt | 8 +
> drivers/iio/temperature/tmp007.c | 334 ++++++++++++++++++---
> 2 files changed, 304 insertions(+), 38 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/temperature/tmp007.txt b/Documentation/devicetree/bindings/iio/temperature/tmp007.txt
> index 3b8f41f..07c1658 100644
> --- a/Documentation/devicetree/bindings/iio/temperature/tmp007.txt
> +++ b/Documentation/devicetree/bindings/iio/temperature/tmp007.txt
> @@ -18,10 +18,18 @@ Required properties:
> 1 SDA 0x46
> 1 SCL 0x47
>
> +Optional properties:
> +
> + - interrupt-parent: should be the phandle for the interrupt controller
> +
> + - interrupts: interrupt mapping for GPIO IRQ
> +
> Example:
>
> tmp007@40 {
> compatible = "ti,tmp007";
> reg = <0x40>;
> + interrupt-parent = <&gpio0>;
> + interrupts = <5 8>;
why are there two GPIOs?
> };
>
> diff --git a/drivers/iio/temperature/tmp007.c b/drivers/iio/temperature/tmp007.c
> index 24c6c16..73b5b8d 100644
> --- a/drivers/iio/temperature/tmp007.c
> +++ b/drivers/iio/temperature/tmp007.c
> @@ -11,9 +11,10 @@
> *
> * (7-bit I2C slave address (0x40 - 0x47), changeable via ADR pins)
> *
> - * Note: This driver assumes that the sensor has been calibrated beforehand
> - *
> - * TODO: ALERT irq, limit threshold events
> + * Note:
> + * 1. This driver assumes that the sensor has been calibrated beforehand
> + * 2. Limit threshold events are enabled at the start
> + * 3. Operating mode: INT
> *
> */
>
> @@ -24,35 +25,49 @@
> #include <linux/pm.h>
> #include <linux/bitops.h>
> #include <linux/of.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
>
> #include <linux/iio/iio.h>
> #include <linux/iio/sysfs.h>
> -
> -#define TMP007_TDIE 0x01
> -#define TMP007_CONFIG 0x02
> -#define TMP007_TOBJECT 0x03
> -#define TMP007_STATUS 0x04
> -#define TMP007_STATUS_MASK 0x05
> -#define TMP007_MANUFACTURER_ID 0x1e
> -#define TMP007_DEVICE_ID 0x1f
> -
> -#define TMP007_CONFIG_CONV_EN BIT(12)
> -#define TMP007_CONFIG_COMP_EN BIT(5)
> -#define TMP007_CONFIG_TC_EN BIT(6)
> -#define TMP007_CONFIG_CR_MASK GENMASK(11, 9)
> -#define TMP007_CONFIG_CR_SHIFT 9
> -
> -#define TMP007_STATUS_CONV_READY BIT(14)
> -#define TMP007_STATUS_DATA_VALID BIT(9)
> -
> -#define TMP007_MANUFACTURER_MAGIC 0x5449
> -#define TMP007_DEVICE_MAGIC 0x0078
> -
> -#define TMP007_TEMP_SHIFT 2
> +#include <linux/iio/events.h>
> +
> +#define TMP007_TDIE 0x01
this has a lot of clutter; the patch also reformats the #defines by adding
whitespace; don't do this in the same patch
> +#define TMP007_CONFIG 0x02
> +#define TMP007_TOBJECT 0x03
> +#define TMP007_STATUS 0x04
> +#define TMP007_STATUS_MASK 0x05
> +#define TMP007_TOBJ_HIGH_LIMIT 0x06
> +#define TMP007_TOBJ_LOW_LIMIT 0x07
> +#define TMP007_TDIE_HIGH_LIMIT 0x08
> +#define TMP007_TDIE_LOW_LIMIT 0x09
> +#define TMP007_MANUFACTURER_ID 0x1e
> +#define TMP007_DEVICE_ID 0x1f
> +
> +#define TMP007_CONFIG_CONV_EN BIT(12)
> +#define TMP007_CONFIG_TC_EN BIT(6)
> +#define TMP007_CONFIG_CR_MASK GENMASK(11, 9)
> +#define TMP007_CONFIG_ALERT_EN BIT(8)
> +#define TMP007_CONFIG_CR_SHIFT 9
> +
> +/* Status register flags */
> +#define TMP007_STATUS_ALERT BIT(15)
> +#define TMP007_STATUS_CONV_READY BIT(14)
> +#define TMP007_STATUS_OHF BIT(13)
> +#define TMP007_STATUS_OLF BIT(12)
> +#define TMP007_STATUS_LHF BIT(11)
> +#define TMP007_STATUS_LLF BIT(10)
> +#define TMP007_STATUS_DATA_VALID BIT(9)
> +
> +#define TMP007_MANUFACTURER_MAGIC 0x5449
> +#define TMP007_DEVICE_MAGIC 0x0078
> +
> +#define TMP007_TEMP_SHIFT 2
>
> struct tmp007_data {
> struct i2c_client *client;
> u16 config;
> + u16 status_mask;
> };
>
> static const int tmp007_avgs[5][2] = { {4, 0}, {2, 0}, {1, 0},
> @@ -70,7 +85,7 @@ static int tmp007_read_temperature(struct tmp007_data *data, u8 reg)
> return ret;
> if ((ret & TMP007_STATUS_CONV_READY) &&
> !(ret & TMP007_STATUS_DATA_VALID))
> - break;
another unrelated cleanup
> + break;
> msleep(100);
> }
>
> @@ -156,6 +171,185 @@ static int tmp007_write_raw(struct iio_dev *indio_dev,
> return -EINVAL;
> }
>
> +static irqreturn_t tmp007_interrupt_handler(int irq, void *private)
> +{
> + struct iio_dev *indio_dev = private;
> + struct tmp007_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + ret = i2c_smbus_read_word_swapped(data->client, TMP007_STATUS);
> + if (ret < 0)
|| !(ret & (_STATUSS_OHF | STATUS_OLF | ..))
> + return IRQ_NONE;
> +
> + if (ret & TMP007_STATUS_OHF)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_TEMP, 0,
> + IIO_MOD_TEMP_OBJECT,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_RISING),
> + iio_get_time_ns(indio_dev));
> +
> + if (ret & TMP007_STATUS_OLF)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_TEMP, 0,
> + IIO_MOD_TEMP_OBJECT,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_FALLING),
> + iio_get_time_ns(indio_dev));
> +
> + if (ret & TMP007_STATUS_LHF)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_TEMP, 0,
> + IIO_MOD_TEMP_AMBIENT,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_RISING),
> + iio_get_time_ns(indio_dev));
> +
> + if (ret & TMP007_STATUS_LLF)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_TEMP, 0,
> + IIO_MOD_TEMP_AMBIENT,
> + IIO_EV_TYPE_THRESH,
> + IIO_EV_DIR_FALLING),
> + iio_get_time_ns(indio_dev));
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int tmp007_write_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan, enum iio_event_type type,
> + enum iio_event_direction dir, int state)
> +{
> + struct tmp007_data *data = iio_priv(indio_dev);
> + unsigned int status_mask;
> + int ret;
> +
> + switch (chan->channel2) {
> + case IIO_MOD_TEMP_AMBIENT:
> + if (dir == IIO_EV_DIR_RISING)
> + status_mask = TMP007_STATUS_LHF;
> + else
> + status_mask = TMP007_STATUS_LLF;
> + break;
> + case IIO_MOD_TEMP_OBJECT:
> + if (dir == IIO_EV_DIR_RISING)
> + status_mask = TMP007_STATUS_OHF;
> + else
> + status_mask = TMP007_STATUS_OLF;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
mutex around this?
> + ret = i2c_smbus_read_word_swapped(data->client, TMP007_STATUS_MASK);
> + if (ret < 0)
> + return ret;
> +
> + if (state)
> + ret |= status_mask;
> + else
> + ret &= ~status_mask;
> +
> + return i2c_smbus_write_word_swapped(data->client, TMP007_STATUS_MASK,
> + data->status_mask = ret);
> +}
> +
> +static int tmp007_read_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan, enum iio_event_type type,
> + enum iio_event_direction dir)
> +{
> + struct tmp007_data *data = iio_priv(indio_dev);
> + unsigned int mask;
> +
> + switch (chan->channel2) {
> + case IIO_MOD_TEMP_AMBIENT:
> + if (dir == IIO_EV_DIR_RISING)
> + mask = TMP007_STATUS_LHF;
> + else
> + mask = TMP007_STATUS_LLF;
> + break;
> + case IIO_MOD_TEMP_OBJECT:
> + if (dir == IIO_EV_DIR_RISING)
> + mask = TMP007_STATUS_OHF;
> + else
> + mask = TMP007_STATUS_OLF;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return (bool)(data->status_mask & mask);
return type is int, maybe use !! to force to 0/1
return !!(data->status_mask & mask);
> +}
> +
> +static int tmp007_read_thresh(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan, enum iio_event_type type,
> + enum iio_event_direction dir, enum iio_event_info info,
> + int *val, int *val2)
> +{
> + struct tmp007_data *data = iio_priv(indio_dev);
> + int ret;
> + u8 reg;
> +
> + switch (chan->channel2) {
> + case IIO_MOD_TEMP_AMBIENT: /* LSB: 0.5 degree Celsius */
> + if (dir == IIO_EV_DIR_RISING)
> + reg = TMP007_TDIE_HIGH_LIMIT;
> + else
> + reg = TMP007_TDIE_LOW_LIMIT;
> + break;
> + case IIO_MOD_TEMP_OBJECT:
> + if (dir == IIO_EV_DIR_RISING)
> + reg = TMP007_TOBJ_HIGH_LIMIT;
> + else
> + reg = TMP007_TOBJ_LOW_LIMIT;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ret = i2c_smbus_read_word_swapped(data->client, reg);
> + if (ret < 0)
> + return ret;
> +
> + /* Shift length 7 bits = 6(15:6) + 1(0.5 LSB) */
> + *val = sign_extend32(ret, 15) >> 7;
> +
> + return IIO_VAL_INT;
> +}
> +
> +static int tmp007_write_thresh(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan, enum iio_event_type type,
> + enum iio_event_direction dir, enum iio_event_info info,
> + int val, int val2)
> +{
> + struct tmp007_data *data = iio_priv(indio_dev);
> + u8 reg;
> +
> + switch (chan->channel2) {
> + case IIO_MOD_TEMP_AMBIENT:
> + if (dir == IIO_EV_DIR_RISING)
> + reg = TMP007_TDIE_HIGH_LIMIT;
> + else
> + reg = TMP007_TDIE_LOW_LIMIT;
> + break;
> + case IIO_MOD_TEMP_OBJECT:
> + if (dir == IIO_EV_DIR_RISING)
> + reg = TMP007_TOBJ_HIGH_LIMIT;
> + else
> + reg = TMP007_TOBJ_LOW_LIMIT;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + /* Full scale value is +/- 256 degree Celsius */
I guess we want to specify milli degree Celsius according to sysfs-bus-iio
for in_temp_raw
> + if (val < -256 || val > 255)
> + return -EINVAL;
> +
> + /* Shift length 7 bits = 6(15:6) + 1(0.5 LSB) */
> + return i2c_smbus_write_word_swapped(data->client, reg, (val << 7));
> +}
> +
> static IIO_CONST_ATTR(sampling_frequency_available, "4 2 1 0.5 0.25");
>
> static struct attribute *tmp007_attributes[] = {
> @@ -167,6 +361,36 @@ static const struct attribute_group tmp007_attribute_group = {
> .attrs = tmp007_attributes,
> };
>
> +static const struct iio_event_spec tmp007_obj_event[] = {
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_RISING,
> + .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> + BIT(IIO_EV_INFO_ENABLE),
> + },
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_FALLING,
> + .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> + BIT(IIO_EV_INFO_ENABLE),
> + },
> +};
> +
> +static const struct iio_event_spec tmp007_die_event[] = {
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_RISING,
> + .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> + BIT(IIO_EV_INFO_ENABLE),
> + },
> + {
> + .type = IIO_EV_TYPE_THRESH,
> + .dir = IIO_EV_DIR_FALLING,
> + .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> + BIT(IIO_EV_INFO_ENABLE),
> + },
> +};
> +
> static const struct iio_chan_spec tmp007_channels[] = {
> {
> .type = IIO_TEMP,
> @@ -175,6 +399,8 @@ static const struct iio_chan_spec tmp007_channels[] = {
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> BIT(IIO_CHAN_INFO_SCALE),
> .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> + .event_spec = tmp007_die_event,
> + .num_event_specs = ARRAY_SIZE(tmp007_die_event),
> },
> {
> .type = IIO_TEMP,
> @@ -183,12 +409,18 @@ static const struct iio_chan_spec tmp007_channels[] = {
> .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> BIT(IIO_CHAN_INFO_SCALE),
> .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> - }
> + .event_spec = tmp007_obj_event,
> + .num_event_specs = ARRAY_SIZE(tmp007_obj_event),
> + },
> };
>
> static const struct iio_info tmp007_info = {
> - .read_raw = tmp007_read_raw,
> - .write_raw = tmp007_write_raw,
> + .read_raw = &tmp007_read_raw,
> + .write_raw = &tmp007_write_raw,
unrelated change, no need for & for a function
> + .read_event_config = &tmp007_read_event_config,
> + .write_event_config = &tmp007_write_event_config,
> + .read_event_value = &tmp007_read_thresh,
> + .write_event_value = &tmp007_write_thresh,
> .attrs = &tmp007_attribute_group,
> .driver_module = THIS_MODULE,
> };
> @@ -214,7 +446,6 @@ static int tmp007_probe(struct i2c_client *client,
> struct tmp007_data *data;
> struct iio_dev *indio_dev;
> int ret;
> - u16 status;
>
> if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
> return -EOPNOTSUPP;
> @@ -243,8 +474,8 @@ static int tmp007_probe(struct i2c_client *client,
> /*
> * Set Configuration register:
> * 1. Conversion ON
> - * 2. Comparator mode
> - * 3. Transient correction enable
> + * 2. Transient correction enable
> + * 3. ALERT enable
> */
>
> ret = i2c_smbus_read_word_swapped(data->client, TMP007_CONFIG);
> @@ -252,7 +483,7 @@ static int tmp007_probe(struct i2c_client *client,
> return ret;
>
> data->config = ret;
> - data->config |= (TMP007_CONFIG_CONV_EN | TMP007_CONFIG_COMP_EN | TMP007_CONFIG_TC_EN);
> + data->config |= (TMP007_CONFIG_CONV_EN | TMP007_CONFIG_TC_EN | TMP007_CONFIG_ALERT_EN);
>
> ret = i2c_smbus_write_word_swapped(data->client, TMP007_CONFIG,
> data->config);
> @@ -260,22 +491,39 @@ static int tmp007_probe(struct i2c_client *client,
> return ret;
>
> /*
> + * Only the following flags can activate ALERT pin. Data conversion/validity flags
> + * flags can still be polled for getting temperature data
> + *
> * Set Status Mask register:
> - * 1. Conversion ready enable
> - * 2. Data valid enable
> + * 1. Object temperature high limit enable
> + * 2. Object temperature low limit enable
> + * 3. TDIE temperature high limit enable
> + * 4. TDIE temperature low limit enable
> */
>
> ret = i2c_smbus_read_word_swapped(data->client, TMP007_STATUS_MASK);
> if (ret < 0)
> goto error_powerdown;
>
> - status = ret;
> - status |= (TMP007_STATUS_CONV_READY | TMP007_STATUS_DATA_VALID);
> + data->status_mask = ret;
> + data->status_mask |= (TMP007_STATUS_OHF | TMP007_STATUS_OLF
> + | TMP007_STATUS_LHF | TMP007_STATUS_LLF);
>
> - ret = i2c_smbus_write_word_swapped(data->client, TMP007_STATUS_MASK, status);
> + ret = i2c_smbus_write_word_swapped(data->client, TMP007_STATUS_MASK, data->status_mask);
> if (ret < 0)
> goto error_powerdown;
>
> + if (client->irq) {
> + ret = devm_request_threaded_irq(&client->dev, client->irq,
> + NULL, tmp007_interrupt_handler,
> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> + tmp007_id->name, indio_dev);
> + if (ret) {
> + dev_err(&client->dev, "irq request error %d\n", -ret);
> + goto error_powerdown;
> + }
> + }
what happens when there is no IRQ? it would be nice to still use the
driver (without event capability)
> +
> return iio_device_register(indio_dev);
>
> error_powerdown:
> @@ -312,6 +560,16 @@ static int tmp007_resume(struct device *dev)
> return i2c_smbus_write_word_swapped(data->client, TMP007_CONFIG,
> data->config | TMP007_CONFIG_CONV_EN);
> }
> +#else
this seems to be unrelated to the patch
> +static int tmp007_suspend(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static int tmp007_resume(struct device *dev)
> +{
> + return 0;
> +}
> #endif
>
> static SIMPLE_DEV_PM_OPS(tmp007_pm_ops, tmp007_suspend, tmp007_resume);
>
--
Peter Meerwald-Stadler
+43-664-2444418 (mobile)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/3] dts: hisi: add dts files for Hi3516CV300 demo board
From: Kefeng Wang @ 2017-01-22 9:33 UTC (permalink / raw)
To: Pan Wen, mturquette-rdvid1DuHRBWk0Htik3J/w,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
xuwei5-C8/M+/jPZTeaMJb+Lgu22Q
Cc: linux-clk-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
howell.yang-C8/M+/jPZTeaMJb+Lgu22Q,
xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q,
jalen.hsu-C8/M+/jPZTeaMJb+Lgu22Q,
lvkuanliang-C8/M+/jPZTeaMJb+Lgu22Q,
suwenping-C8/M+/jPZTeaMJb+Lgu22Q, raojun-C8/M+/jPZTeaMJb+Lgu22Q,
kevin.lixu-C8/M+/jPZTeaMJb+Lgu22Q
In-Reply-To: <20161017120705.3726-4-wenpan-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
On 2016/10/17 20:07, Pan Wen wrote:
> Add dts files for Hi3516CV300 demo board.
>
> Signed-off-by: Pan Wen <wenpan-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> ---
> arch/arm/boot/dts/Makefile | 1 +
[...]
> diff --git a/arch/arm/boot/dts/hi3516cv300.dtsi b/arch/arm/boot/dts/hi3516cv300.dtsi
> new file mode 100644
> index 0000000..1da41ab
> --- /dev/null
> +++ b/arch/arm/boot/dts/hi3516cv300.dtsi
> @@ -0,0 +1,397 @@
> +/*
> + * Copyright (c) 2016 HiSilicon Technologies Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "skeleton.dtsi"
please drop this include due to commit 9c0da3cc61f1233c ("ARM: dts: explicitly mark skeleton.dtsi as deprecated")
Thanks
Kefeng
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Re: [PATCH 1/4] phy: sun4i-usb: support PHY0 on H3 in MUSB mode
From: Icenowy Zheng @ 2017-01-22 9:39 UTC (permalink / raw)
To: Hans de Goede, Karsten Merker
Cc: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Kishon Vijay Abraham I,
Greg Kroah-Hartman, Bin Liu,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
In-Reply-To: <93a01892-47b5-5445-d802-f56bdac8371f-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
20.01.2017, 16:04, "Hans de Goede" <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> HI,
>
> On 19-01-17 21:27, Karsten Merker wrote:
>> On Thu, Jan 19, 2017 at 11:10:08PM +0800, Icenowy Zheng wrote:
>>> 19.01.2017, 22:34, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
>>>> On Wed, Jan 18, 2017 at 04:09:32AM +0800, Chen-Yu Tsai wrote:
>>>>> On Wed, Jan 18, 2017 at 4:06 AM, Maxime Ripard
>>>>> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>>>> > On Wed, Jan 18, 2017 at 12:57:08AM +0800, Icenowy Zheng wrote:
>>>>> >> 17.01.2017, 16:06, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
>>>>> >> > On Tue, Jan 17, 2017 at 03:14:46AM +0800, Icenowy Zheng wrote:
>>>>> >> >> The PHY0 on H3 can be wired either to MUSB controller or OHCI/EHCI
>>>>> >> >> controller.
>>>>> >> >>
>>>>> >> >> The original driver wired it to OHCI/EHCI controller; however, as the
>>>>> >> >> code to use PHY0 as OHCI/EHCI is missing, it makes the PHY fully
>>>>> >> >> unusable.
>>>>> >> >>
>>>>> >> >> Rename the register (according to its function and the name in BSP
>>>>> >> >> driver), and remove the code which wires the PHY0 to OHCI/EHCI, as MUSB
>>>>> >> >> can support both peripheral and host mode (although the host mode of
>>>>> >> >> MUSB is buggy).
>>>>> >> >
>>>>> >> > Can you elaborate on that? What's wrong with it?
>>>>> >>
>>>>> >> The configuration is at bit 0 of register 0x20 in PHY.
>>>>> >>
>>>>> >> When the PHY is reseted, it defaults as MUSB mode.
>>>>> >>
>>>>> >> However, the original author of the H3 PHY code seems to be lack of
>>>>> >> this knowledge (He named it PHY_UNK_H3), and changed the PHY to HCI
>>>>> >> mode.
>>>>> >>
>>>>> >> I just removed the code that wires it to HCI mode, thus it will work
>>>>> >> in MUSB mode, with my sun8i-h3-musb patch.
>>>>> >
>>>>> > I have no idea what you mean by MUSB mode.
>>>>> >
>>>>> > Do you mean that the previous code was only working in host mode, and
>>>>> > now it only works in peripheral?
>>>>>
>>>>> From what I understand, with the H3, Allwinner has put a mux
>>>>> in front of the MUSB controller. The mux can send the USB data
>>>>> to/from the MUSB controller, or a standard EHCI/OHCI pair.
>>>>> This register controls said mux.
>>>>>
>>>>> This means we can use a proper USB host for host mode,
>>>>> instead of the limited support in MUSB.
>>>>
>>>> But musb can still operate as a host, right?
>>>
>>> Yes!
>>
>> Hello,
>>
>> I don't know how the MUSB implementation in the H3 behaves as I
>> don't have any H3-based systems, but if it should happen to be
>> similar to the one in the A31s, it probably isn't a full-fledged
>> alternative to using an OHCI/EHCI controller.
>
> You right it isn't which is why I suggested that the phy-sun4i-usb
> code should set the mux to the OCHI/EHCI pair when the id pin
> is pulled low (host-mode).
>
>> From my practical experiments with the MUSB in the A31s in host
>> mode I can report that I hadn't been able to get multiple HIDs
>> (in my case keyboard and mouse) working at the same time. The
>> keyboard alone worked without problems, the mouse alone worked
>> without problems, but when both were connected, only one of them
>> worked.
>>
>> I had at that time talked to Hans de Goede about the problem and
>> if I remenber correctly, he had mentioned that the MUSB has
>> problems servicing more than one device that does interrupt
>> transfers (as HIDs do).
>>
>> Hans, can you perhaps shed some light on this?
>
> Everything you've said is correct, the MUSB can emulate a
> host-controller, but it is not really one and when possible
> should not be used as such.
But implement proper EHCI/OHCI pair and MUSB coexistence needs
a lot of code, and MUSB can still work.
We can now just enable MUSB, then after the coexistence is done,
switch to use MUSB for peripheral and {E,O}HCI for host.
Seems that SoCs after sun8iw7 (H3) all have this feature. (At least
I verified it on H3, V3s, A64, H5)
P.S. The implementation on FreeBSD of A64 PHY uses a dedicated
PHY number for the HCI pair, which seems to break the DT compatibility
between Linux and FreeBSD.
>
> Hans
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH v2 2/3] ARM: dts: r7s72100: add ostm to device tree
From: Geert Uytterhoeven @ 2017-01-22 9:40 UTC (permalink / raw)
To: Chris Brandt
Cc: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Russell King, Thomas Gleixner, Geert Uytterhoeven,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux-Renesas
In-Reply-To: <20170120212055.4157-3-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
On Fri, Jan 20, 2017 at 10:20 PM, Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> ---
> v2:
> * wrap clock lines to avoid 80 char max
> * split into 2 separate channel nodes
> ---
> arch/arm/boot/dts/r7s72100.dtsi | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
> index d5946df..47b2165 100644
> --- a/arch/arm/boot/dts/r7s72100.dtsi
> +++ b/arch/arm/boot/dts/r7s72100.dtsi
> @@ -505,4 +505,20 @@
> cap-sdio-irq;
> status = "disabled";
> };
> +
> + ostm0: ostm@fcfec000 {
> + compatible = "renesas,r7s72100-ostm", "renesas,ostm";
> + reg = <0xfcfec000 0x30>;
> + interrupts = <GIC_SPI 102 IRQ_TYPE_EDGE_RISING>;
> + clocks = <&mstp5_clks R7S72100_CLK_OSTM0>;
Missing power-domains property:
power-domains = <&cpg_clocks>;
> + status = "disabled";
> + };
> +
> + ostm1: ostm@fcfec400 {
> + compatible = "renesas,r7s72100-ostm", "renesas,ostm";
> + reg = <0xfcfec400 0x30>;
> + interrupts = <GIC_SPI 103 IRQ_TYPE_EDGE_RISING>;
> + clocks = <&mstp5_clks R7S72100_CLK_OSTM1>;
Likewise.
BTW, they're also missing for the last added device nodes (MMC and SDHI).
> + status = "disabled";
> + };
> };
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Re: [PATCH 1/4] phy: sun4i-usb: support PHY0 on H3 in MUSB mode
From: Hans de Goede @ 2017-01-22 9:58 UTC (permalink / raw)
To: Icenowy Zheng, Karsten Merker
Cc: Maxime Ripard, Chen-Yu Tsai, Rob Herring, Kishon Vijay Abraham I,
Greg Kroah-Hartman, Bin Liu,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
In-Reply-To: <9112691485077945-NPKjhoV82L1xpj1cXAZ9Bg@public.gmane.org>
HI,
On 22-01-17 10:39, Icenowy Zheng wrote:
>
>
> 20.01.2017, 16:04, "Hans de Goede" <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
>> HI,
>>
>> On 19-01-17 21:27, Karsten Merker wrote:
>>> On Thu, Jan 19, 2017 at 11:10:08PM +0800, Icenowy Zheng wrote:
>>>> 19.01.2017, 22:34, "Maxime Ripard" <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
>>>>> On Wed, Jan 18, 2017 at 04:09:32AM +0800, Chen-Yu Tsai wrote:
>>>>>> On Wed, Jan 18, 2017 at 4:06 AM, Maxime Ripard
>>>>>> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>>>>>> > On Wed, Jan 18, 2017 at 12:57:08AM +0800, Icenowy Zheng wrote:
>>>>>> >> 17.01.2017, 16:06, "Maxime Ripard" <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
>>>>>> >> > On Tue, Jan 17, 2017 at 03:14:46AM +0800, Icenowy Zheng wrote:
>>>>>> >> >> The PHY0 on H3 can be wired either to MUSB controller or OHCI/EHCI
>>>>>> >> >> controller.
>>>>>> >> >>
>>>>>> >> >> The original driver wired it to OHCI/EHCI controller; however, as the
>>>>>> >> >> code to use PHY0 as OHCI/EHCI is missing, it makes the PHY fully
>>>>>> >> >> unusable.
>>>>>> >> >>
>>>>>> >> >> Rename the register (according to its function and the name in BSP
>>>>>> >> >> driver), and remove the code which wires the PHY0 to OHCI/EHCI, as MUSB
>>>>>> >> >> can support both peripheral and host mode (although the host mode of
>>>>>> >> >> MUSB is buggy).
>>>>>> >> >
>>>>>> >> > Can you elaborate on that? What's wrong with it?
>>>>>> >>
>>>>>> >> The configuration is at bit 0 of register 0x20 in PHY.
>>>>>> >>
>>>>>> >> When the PHY is reseted, it defaults as MUSB mode.
>>>>>> >>
>>>>>> >> However, the original author of the H3 PHY code seems to be lack of
>>>>>> >> this knowledge (He named it PHY_UNK_H3), and changed the PHY to HCI
>>>>>> >> mode.
>>>>>> >>
>>>>>> >> I just removed the code that wires it to HCI mode, thus it will work
>>>>>> >> in MUSB mode, with my sun8i-h3-musb patch.
>>>>>> >
>>>>>> > I have no idea what you mean by MUSB mode.
>>>>>> >
>>>>>> > Do you mean that the previous code was only working in host mode, and
>>>>>> > now it only works in peripheral?
>>>>>>
>>>>>> From what I understand, with the H3, Allwinner has put a mux
>>>>>> in front of the MUSB controller. The mux can send the USB data
>>>>>> to/from the MUSB controller, or a standard EHCI/OHCI pair.
>>>>>> This register controls said mux.
>>>>>>
>>>>>> This means we can use a proper USB host for host mode,
>>>>>> instead of the limited support in MUSB.
>>>>>
>>>>> But musb can still operate as a host, right?
>>>>
>>>> Yes!
>>>
>>> Hello,
>>>
>>> I don't know how the MUSB implementation in the H3 behaves as I
>>> don't have any H3-based systems, but if it should happen to be
>>> similar to the one in the A31s, it probably isn't a full-fledged
>>> alternative to using an OHCI/EHCI controller.
>>
>> You right it isn't which is why I suggested that the phy-sun4i-usb
>> code should set the mux to the OCHI/EHCI pair when the id pin
>> is pulled low (host-mode).
>>
>>> From my practical experiments with the MUSB in the A31s in host
>>> mode I can report that I hadn't been able to get multiple HIDs
>>> (in my case keyboard and mouse) working at the same time. The
>>> keyboard alone worked without problems, the mouse alone worked
>>> without problems, but when both were connected, only one of them
>>> worked.
>>>
>>> I had at that time talked to Hans de Goede about the problem and
>>> if I remenber correctly, he had mentioned that the MUSB has
>>> problems servicing more than one device that does interrupt
>>> transfers (as HIDs do).
>>>
>>> Hans, can you perhaps shed some light on this?
>>
>> Everything you've said is correct, the MUSB can emulate a
>> host-controller, but it is not really one and when possible
>> should not be used as such.
>
> But implement proper EHCI/OHCI pair and MUSB coexistence needs
> a lot of code
No it doesn't see the previous thread on this I give an example
of how this can be implemented there, and it is not a lot of
work, it just requires someone to do it.
> and MUSB can still work.
>
> We can now just enable MUSB, then after the coexistence is done,
> switch to use MUSB for peripheral and {E,O}HCI for host.
I'm not in favor of these kinda hacks, we are creating expectations
of how things work then only to change them later with a possible
risk of regression things for some users / use-cases. Lets do
this the right way in one go please.
> Seems that SoCs after sun8iw7 (H3) all have this feature. (At least
> I verified it on H3, V3s, A64, H5)
>
> P.S. The implementation on FreeBSD of A64 PHY uses a dedicated
> PHY number for the HCI pair, which seems to break the DT compatibility
> between Linux and FreeBSD.
That is unfortunate and also weird since AFAICT there is only
one phy and it is the phy data lines which get muxed not the
physical phy. Anyways can you contact them and ask them to
please not do that ?
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v1 3/3] i2c: zx2967: add i2c controller driver for ZTE's zx2967 family
From: Baoyou Xie @ 2017-01-22 10:12 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jun Nie, Wolfram Sang, Rob Herring, Mark Rutland,
linux-arm Mailing List, linux-i2c, devicetree,
linux-kernel@vger.kernel.org, Shawn Guo, xie.baoyou, chen.chaokai,
wang.qiang01
In-Reply-To: <CAHp75VfjS-Wdun65Tx8xEniTVfiMASu022K9ipVRwxJLE8SXkg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 10037 bytes --]
On 21 January 2017 at 05:45, Andy Shevchenko <andy.shevchenko@gmail.com>
wrote:
> On Fri, Jan 20, 2017 at 10:43 AM, Baoyou Xie <baoyou.xie@linaro.org>
> wrote:
> > This patch adds i2c controller driver for ZTE's zx2967 family.
> >
>
> > +config I2C_ZX2967
> > + bool "ZTE zx2967 I2C support"
>
> No module?
>
> > + depends on ARCH_ZX
> > + default y
>
> Would architecture selects it instead?
>
> of cause, It's ok that architecture selects it. but for subject to other
i2c drivers, I suggest to remain it.
> > + help
> > + Selecting this option will add ZX I2C driver.
> > + If unsure, say y.
>
> Two sentences about the same in spite 'default y'.
>
> > +#define BW1 (u32)1
>
> Do you really need casting?
>
> > +#define I2C_STOP 0
> > +#define I2C_MASTER (1 << 0)
>
> BIT(0) ?
> And so further.
>
> > +#define I2C_INT_MASK 0x3f
>
> GENMASK(6, 0) ?
>
>
> > +#define I2C_ERROR_MASK (I2C_ERROR_DATA |
> I2C_ERROR_DEVICE)
>
> Ditto.
> GENMASK() with explicit numbers.
>
> > +static int zx2967_i2c_flush_fifos(struct zx2967_i2c_info *zx_i2c)
> > +{
> > + u32 val = 0;
> > + u32 offset = 0;
>
> Useless assignments.
>
> > +
> > + if (zx_i2c->msg_rd) {
> > + offset = REG_RDCONF;
> > + val = I2C_RFIFO_RESET;
> > + } else {
> > + offset = REG_WRCONF;
> > + val = I2C_WFIFO_RESET;
> > + }
>
> > +static int zx2967_i2c_empty_rx_fifo(struct zx2967_i2c_info *zx_i2c,
> u32 size)
> > +{
>
> > + u8 val[I2C_FIFO_MAX] = {0};
> > + int i = 0;
>
> Ditto.
>
> > +
> > + if (size == 0) {
>
> Would it be the case?
>
> > + dev_err(zx_i2c->dev, "size is invalid\n");
> > + return -EINVAL;
> > + }
>
> And what happens even in that case if you just remove this check?
>
> en, thanks for you find this big bug:)
> > +
> > + zx2967_i2c_readsb(zx_i2c, val, REG_DATA, size);
> > + for (i = 0; i < size; i++) {
> > + *(zx_i2c->buf++) = val[i];
> > + zx_i2c->residue--;
> > + if (zx_i2c->residue <= 0)
> > + break;
>
> > +static int zx2967_i2c_fill_tx_fifo(struct zx2967_i2c_info *zx_i2c)
> > +{
> > + u8 *buf = zx_i2c->buf;
> > + size_t residue = zx_i2c->residue;
> > +
> > + if (residue == 0) {
> > + dev_err(zx_i2c->dev, "residue is %d\n", (int)residue);
> > + return -EINVAL;
> > + }
>
> Ditto.
>
> here, we need not worry the value of residue over I2C_FIFO_MAX.
>
> > +static void zx2967_enable_tenbit(struct zx2967_i2c_info *zx_i2c, __u16
> addr)
> > +{
> > + u16 val = ((addr >> 7) & 0x7);
>
> Redundant parens.
>
> > + if (val > 0) {
>
> if (!val)
> return;
>
> > +static int zx2967_i2c_xfer_msg(struct zx2967_i2c_info *zx_i2c,
> > + struct i2c_msg *msg
> > +{
> > + unsigned long time_left = 0;
>
> > + unsigned int i = 0;
>
> Redundant assignment.
>
> > +
> > + if (msg->len == 0)
>
> if (!msg...)
>
> > + return -EINVAL;
>
> > +static int zx2967_i2c_xfer(struct i2c_adapter *adap,
> > + struct i2c_msg *msgs, int num)
> > +{
> > + int i;
>
> > + int ret = 0;
>
> Redundant. See below.
>
> > + struct zx2967_i2c_info *zx_i2c = i2c_get_adapdata(adap);
>
> Reverse tree, please.
>
> > +
> > + if (zx_i2c->is_suspended)
> > + return -EBUSY;
> > +
> > + zx2967_i2c_writel(zx_i2c, (msgs->addr & 0x7f), REG_DEVADDR_L);
> > + zx2967_i2c_writel(zx_i2c, (msgs->addr >> 7) & 0x7,
> REG_DEVADDR_H);
> > + if (zx2967_i2c_readl(zx_i2c, REG_DEVADDR_H) > 0)
> > + zx2967_enable_tenbit(zx_i2c, msgs->addr);
> > +
> > + for (i = 0; i < num; i++) {
>
> int ret;
>
> > + ret = zx2967_i2c_xfer_msg(zx_i2c, &msgs[i]);
> > + if (num > 1)
> > + usleep_range(1000, 2000);
> > + if (ret)
>
> > + break;
>
> return ret;
>
> > + }
> > +
>
> > + return ret ?: i;
>
> return i;
>
> And looking now, how i can't be num here?
>
> > +}
>
> > +{
> > + struct zx2967_i2c_info *zx_i2c = i2c_get_adapdata(adap);
>
> > + u32 val;
> > + u8 buf[2];
> > + unsigned long time_left;
>
> Reverse tree.
>
> > + usleep_range(1000, 2000);
> > + switch (size) {
> > + case I2C_SMBUS_BYTE:
> > + case I2C_SMBUS_BYTE_DATA:
> > + val = zx2967_i2c_readl(zx_i2c, REG_DATA);
> > + data->byte = val;
> > + break;
> > + case I2C_SMBUS_WORD_DATA:
> > + case I2C_SMBUS_PROC_CALL:
> > + buf[0] = zx2967_i2c_readl(zx_i2c, REG_DATA);
> > + buf[1] = zx2967_i2c_readl(zx_i2c, REG_DATA);
> > + data->word = (buf[0] << 8) | buf[1];
> > + break;
> > + default:
> > + dev_warn(&adap->dev, "Unsupported transaction %d\n",
> size);
> > + return -EOPNOTSUPP;
> > + }
> > +
> > + return 0;
> > +}
>
> I would revisit those several loooong functions for splitting/refactoring.
>
> > +
> > +static u32 zx2967_i2c_func(struct i2c_adapter *adap)
> > +{
> > + return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
> > + I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
> > + I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_PROC_CALL |
> > + I2C_FUNC_I2C | I2C_FUNC_SMBUS_I2C_BLOCK;
> > +}
>
> It's just
> #define XXX YY1 | YY2 | ...
>
> > +#ifdef CONFIG_PM
> > +static int zx2967_i2c_suspend(struct device *dev)
>
> Switch to __maybe_unuse.
>
> > +static int zx2967_i2c_probe(struct platform_device *pdev)
> > +{
>
> > + struct resource *res;
> > + struct zx2967_i2c_info *zx_i2c = NULL;
>
> Redundant assignment.
>
> > + struct clk *clk;
> > + void __iomem *reg_base;
>
> Reverse tree for all above until top of this function.
>
> > + int ret = 0;
>
> Redundant assignment.
>
> > +
> > + zx_i2c = devm_kzalloc(&pdev->dev, sizeof(*zx_i2c), GFP_KERNEL);
> > + if (!zx_i2c)
> > + return -ENOMEM;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
> > + if (!res) {
> > + dev_err(&pdev->dev, "missing io resource\n");
> > + return -EINVAL;
> > + }
> > +
>
> Completely redundant lines of code.
>
> > + reg_base = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(reg_base))
> > + return PTR_ERR(reg_base);
> > +
> > + clk = devm_clk_get(&pdev->dev, NULL);
> > + if (IS_ERR(clk)) {
> > + dev_err(&pdev->dev, "missing controller clock");
> > + return PTR_ERR(clk);
> > + }
> > +
> > + ret = clk_prepare_enable(clk);
> > + if (ret) {
> > + dev_err(&pdev->dev, "failed to enable i2c_clk\n");
> > + return ret;
> > + }
> > +
>
> > + of_property_read_u32(pdev->dev.of_node,
> > + "clock-frequency", &zx_i2c->clk_freq);
>
> devce_property_read_uXX();
>
> > + zx_i2c->reg_base = reg_base;
> > + zx_i2c->clk = clk;
>
> > + zx_i2c->irq = platform_get_irq(pdev, 0);
>
> Is this optional?
> Otherwise has to be checked.
>
> > + zx_i2c->id = pdev->id;
> > + zx_i2c->dev = &pdev->dev;
> > +
> > + zx_i2c->pin_scl = devm_pinctrl_get_select(&pdev->dev, "scl");
> > + if (IS_ERR(zx_i2c->pin_scl))
> > + dev_info(&pdev->dev, "scl pin is not specified in
> dts\n");
> > +
> > + zx_i2c->pin_sda = devm_pinctrl_get_select(&pdev->dev, "sda");
> > + if (IS_ERR(zx_i2c->pin_sda))
> > + dev_info(&pdev->dev, "sda pin is not specified in
> dts\n");
> > +
> > + spin_lock_init(&zx_i2c->lock);
> > + init_completion(&zx_i2c->complete);
> > + platform_set_drvdata(pdev, zx_i2c);
> > +
> > + ret = zx2967_i2c_reset_hardware(zx_i2c);
> > + if (ret) {
> > + dev_err(&pdev->dev, "failed to initialize i2c
> controller\n");
> > + goto unprepare_clk;
> > + }
> > +
> > + ret = devm_request_irq(&pdev->dev, zx_i2c->irq,
> > + zx2967_i2c_isr, 0, dev_name(&pdev->dev), zx_i2c);
> > + if (ret) {
> > + dev_err(&pdev->dev, "failed to request irq %i\n",
> zx_i2c->irq);
> > + goto unprepare_clk;
> > + }
> > +
> > + i2c_set_adapdata(&zx_i2c->adap, zx_i2c);
>
> > + zx_i2c->adap.owner = THIS_MODULE;
>
> Do you need this?
>
> > + zx_i2c->adap.class = I2C_CLASS_DEPRECATED;
> > + strlcpy(zx_i2c->adap.name, "zx2967 i2c adapter",
> > + sizeof(zx_i2c->adap.name));
> > + zx_i2c->adap.algo = &zx2967_i2c_algo;
> > + zx_i2c->adap.dev.parent = &pdev->dev;
> > + zx_i2c->adap.nr = pdev->id;
> > + zx_i2c->adap.dev.of_node = pdev->dev.of_node;
> > +
> > + ret = i2c_add_numbered_adapter(&zx_i2c->adap);
> > + if (ret) {
> > + dev_err(&pdev->dev, "failed to add zx2967 i2c
> adapter\n");
> > + goto unprepare_clk;
> > + }
> > +
> > + return 0;
> > +
> > +unprepare_clk:
>
> err_clk_unprepare:
>
> > + clk_unprepare(zx_i2c->clk);
>
> Perhaps clk_disable_unprepare();
>
> > + return ret;
> > +}
> > +
> > +static int zx2967_i2c_remove(struct platform_device *pdev)
> > +{
> > + struct zx2967_i2c_info *zx_i2c = platform_get_drvdata(pdev);
> > +
> > + i2c_del_adapter(&zx_i2c->adap);
>
> > + clk_unprepare(zx_i2c->clk);
>
> Ditto.
>
> > +
> > + return 0;
> > +}
>
> > +MODULE_AUTHOR("Baoyou Xie <baoyou.xie@linaro.org>");
> > +MODULE_DESCRIPTION("ZTE zx2967 I2C Bus Controller driver");
> > +MODULE_LICENSE("GPL v2");
>
> When bool in Kconfig this all redundant.
>
> --
> With Best Regards,
> Andy Shevchenko
>
[-- Attachment #2: Type: text/html, Size: 15631 bytes --]
^ permalink raw reply
* Re: Re: [PATCH 1/4] phy: sun4i-usb: support PHY0 on H3 in MUSB mode
From: Icenowy Zheng @ 2017-01-22 10:24 UTC (permalink / raw)
To: Hans de Goede
Cc: Greg Kroah-Hartman, Rob Herring,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Bin Liu,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Karsten Merker,
Kishon Vijay Abraham I, Maxime Ripard,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Chen-Yu Tsai
2017年1月22日 17:58于 Hans de Goede <hdegoede@redhat.com>写道：
>
> HI,
>
> On 22-01-17 10:39, Icenowy Zheng wrote:
> >
> >
> > 20.01.2017, 16:04, "Hans de Goede" <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> >> HI,
> >>
> >> On 19-01-17 21:27, Karsten Merker wrote:
> >>> On Thu, Jan 19, 2017 at 11:10:08PM +0800, Icenowy Zheng wrote:
> >>>> 19.01.2017, 22:34, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
> >>>>> On Wed, Jan 18, 2017 at 04:09:32AM +0800, Chen-Yu Tsai wrote:
> >>>>>> On Wed, Jan 18, 2017 at 4:06 AM, Maxime Ripard
> >>>>>> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> >>>>>> > On Wed, Jan 18, 2017 at 12:57:08AM +0800, Icenowy Zheng wrote:
> >>>>>> >> 17.01.2017, 16:06, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
> >>>>>> >> > On Tue, Jan 17, 2017 at 03:14:46AM +0800, Icenowy Zheng wrote:
> >>>>>> >> >> The PHY0 on H3 can be wired either to MUSB controller or OHCI/EHCI
> >>>>>> >> >> controller.
> >>>>>> >> >>
> >>>>>> >> >> The original driver wired it to OHCI/EHCI controller; however, as the
> >>>>>> >> >> code to use PHY0 as OHCI/EHCI is missing, it makes the PHY fully
> >>>>>> >> >> unusable.
> >>>>>> >> >>
> >>>>>> >> >> Rename the register (according to its function and the name in BSP
> >>>>>> >> >> driver), and remove the code which wires the PHY0 to OHCI/EHCI, as MUSB
> >>>>>> >> >> can support both peripheral and host mode (although the host mode of
> >>>>>> >> >> MUSB is buggy).
> >>>>>> >> >
> >>>>>> >> > Can you elaborate on that? What's wrong with it?
> >>>>>> >>
> >>>>>> >> The configuration is at bit 0 of register 0x20 in PHY.
> >>>>>> >>
> >>>>>> >> When the PHY is reseted, it defaults as MUSB mode.
> >>>>>> >>
> >>>>>> >> However, the original author of the H3 PHY code seems to be lack of
> >>>>>> >> this knowledge (He named it PHY_UNK_H3), and changed the PHY to HCI
> >>>>>> >> mode.
> >>>>>> >>
> >>>>>> >> I just removed the code that wires it to HCI mode, thus it will work
> >>>>>> >> in MUSB mode, with my sun8i-h3-musb patch.
> >>>>>> >
> >>>>>> > I have no idea what you mean by MUSB mode.
> >>>>>> >
> >>>>>> > Do you mean that the previous code was only working in host mode, and
> >>>>>> > now it only works in peripheral?
> >>>>>>
> >>>>>> From what I understand, with the H3, Allwinner has put a mux
> >>>>>> in front of the MUSB controller. The mux can send the USB data
> >>>>>> to/from the MUSB controller, or a standard EHCI/OHCI pair.
> >>>>>> This register controls said mux.
> >>>>>>
> >>>>>> This means we can use a proper USB host for host mode,
> >>>>>> instead of the limited support in MUSB.
> >>>>>
> >>>>> But musb can still operate as a host, right?
> >>>>
> >>>> Yes!
> >>>
> >>> Hello,
> >>>
> >>> I don't know how the MUSB implementation in the H3 behaves as I
> >>> don't have any H3-based systems, but if it should happen to be
> >>> similar to the one in the A31s, it probably isn't a full-fledged
> >>> alternative to using an OHCI/EHCI controller.
> >>
> >> You right it isn't which is why I suggested that the phy-sun4i-usb
> >> code should set the mux to the OCHI/EHCI pair when the id pin
> >> is pulled low (host-mode).
> >>
> >>> From my practical experiments with the MUSB in the A31s in host
> >>> mode I can report that I hadn't been able to get multiple HIDs
> >>> (in my case keyboard and mouse) working at the same time. The
> >>> keyboard alone worked without problems, the mouse alone worked
> >>> without problems, but when both were connected, only one of them
> >>> worked.
> >>>
> >>> I had at that time talked to Hans de Goede about the problem and
> >>> if I remenber correctly, he had mentioned that the MUSB has
> >>> problems servicing more than one device that does interrupt
> >>> transfers (as HIDs do).
> >>>
> >>> Hans, can you perhaps shed some light on this?
> >>
> >> Everything you've said is correct, the MUSB can emulate a
> >> host-controller, but it is not really one and when possible
> >> should not be used as such.
> >
> > But implement proper EHCI/OHCI pair and MUSB coexistence needs
> > a lot of code
>
> No it doesn't see the previous thread on this I give an example
> of how this can be implemented there, and it is not a lot of
> work, it just requires someone to do it.
>
However, currently dr_mode is specified in musb, and it's musb that is responsible for changing the mode (PHY driver only detects ID pin).
> > and MUSB can still work.
> >
> > We can now just enable MUSB, then after the coexistence is done,
> > switch to use MUSB for peripheral and {E,O}HCI for host.
>
> I'm not in favor of these kinda hacks, we are creating expectations
> of how things work then only to change them later with a possible
> risk of regression things for some users / use-cases. Lets do
> this the right way in one go please.
As we should move dr_mode to phy node, we can
just let phy behave like a musb-only phy if such a dr_mode is not specified.
And this patch just makes the phy behave in a way
like the phy in the old SoCs, not a hack.
>
> > Seems that SoCs after sun8iw7 (H3) all have this feature. (At least
> > I verified it on H3, V3s, A64, H5)
> >
> > P.S. The implementation on FreeBSD of A64 PHY uses a dedicated
> > PHY number for the HCI pair, which seems to break the DT compatibility
> > between Linux and FreeBSD.
>
> That is unfortunate and also weird since AFAICT there is only
> one phy and it is the phy data lines which get muxed not the
> physical phy. Anyways can you contact them and ask them to
> please not do that ?
>
> Regards,
>
> Hans
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [PATCH 0/2] iio: imu: st_lsm6dsx: add capability to select interrupt pin
From: Lorenzo Bianconi @ 2017-01-22 10:30 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
Lorenzo Bianconi (2):
iio: imu: st_lsm6dsx: add possibility to select drdy pin
Documentation: dt: iio: imu: st_lsm6dsx: add st,drdy-int-pin property
.../devicetree/bindings/iio/imu/st_lsm6dsx.txt | 2 +
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 66 ++++++++++++++++++++--
2 files changed, 62 insertions(+), 6 deletions(-)
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/2] iio: imu: st_lsm6dsx: add possibility to select drdy pin
From: Lorenzo Bianconi @ 2017-01-22 10:30 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
In-Reply-To: <20170122103052.8769-1-lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
Add capability to route data ready signal on pin 1 or pin 2 of the package
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
---
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 66 +++++++++++++++++++++++++---
1 file changed, 60 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index f869dfa..bc50164 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -37,11 +37,14 @@
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
+#include <linux/platform_data/st_sensors_pdata.h>
+
#include "st_lsm6dsx.h"
#define ST_LSM6DSX_REG_ACC_DEC_MASK GENMASK(2, 0)
#define ST_LSM6DSX_REG_GYRO_DEC_MASK GENMASK(5, 3)
#define ST_LSM6DSX_REG_INT1_ADDR 0x0d
+#define ST_LSM6DSX_REG_INT2_ADDR 0x0e
#define ST_LSM6DSX_REG_FIFO_FTH_IRQ_MASK BIT(3)
#define ST_LSM6DSX_REG_WHOAMI_ADDR 0x0f
#define ST_LSM6DSX_REG_RESET_ADDR 0x12
@@ -532,10 +535,63 @@ static const struct iio_info st_lsm6dsx_gyro_info = {
static const unsigned long st_lsm6dsx_available_scan_masks[] = {0x7, 0x0};
+#ifdef CONFIG_OF
+static int st_lsm6dsx_of_get_drdy_pin(struct st_lsm6dsx_hw *hw, int *drdy_pin)
+{
+ struct device_node *np = hw->dev->of_node;
+ int err;
+
+ if (!np)
+ return -EINVAL;
+
+ err = of_property_read_u32(np, "st,drdy-int-pin", drdy_pin);
+ if (err == -ENODATA) {
+ /* if the property has not been specified use default value */
+ *drdy_pin = 1;
+ err = 0;
+ }
+
+ return err;
+}
+#else
+static int st_lsm6dsx_of_get_drdy_pin(struct st_lsm6dsx_hw *hw, int *drdy_pin)
+{
+ return -EINVAL;
+}
+#endif /* CONFIG_OF */
+
+static int st_lsm6dsx_get_drdy_reg(struct st_lsm6dsx_hw *hw, u8 *drdy_reg)
+{
+ int err = 0, drdy_pin;
+
+ if (st_lsm6dsx_of_get_drdy_pin(hw, &drdy_pin) < 0) {
+ struct st_sensors_platform_data *pdata;
+ struct device *dev = hw->dev;
+
+ pdata = (struct st_sensors_platform_data *)dev->platform_data;
+ drdy_pin = pdata ? pdata->drdy_int_pin : 1;
+ }
+
+ switch (drdy_pin) {
+ case 1:
+ *drdy_reg = ST_LSM6DSX_REG_INT1_ADDR;
+ break;
+ case 2:
+ *drdy_reg = ST_LSM6DSX_REG_INT2_ADDR;
+ break;
+ default:
+ dev_err(hw->dev, "unsupported data ready pin\n");
+ err = -EINVAL;
+ break;
+ }
+
+ return err;
+}
+
static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
{
+ u8 data, drdy_int_reg;
int err;
- u8 data;
data = ST_LSM6DSX_REG_RESET_MASK;
err = hw->tf->write(hw->dev, ST_LSM6DSX_REG_RESET_ADDR, sizeof(data),
@@ -563,14 +619,12 @@ static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
return err;
/* enable FIFO watermak interrupt */
- err = st_lsm6dsx_write_with_mask(hw, ST_LSM6DSX_REG_INT1_ADDR,
- ST_LSM6DSX_REG_FIFO_FTH_IRQ_MASK, 1);
+ err = st_lsm6dsx_get_drdy_reg(hw, &drdy_int_reg);
if (err < 0)
return err;
- /* redirect INT2 on INT1 */
- return st_lsm6dsx_write_with_mask(hw, ST_LSM6DSX_REG_INT2_ON_INT1_ADDR,
- ST_LSM6DSX_REG_INT2_ON_INT1_MASK, 1);
+ return st_lsm6dsx_write_with_mask(hw, drdy_int_reg,
+ ST_LSM6DSX_REG_FIFO_FTH_IRQ_MASK, 1);
}
static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 2/2] Documentation: dt: iio: imu: st_lsm6dsx: add st,drdy-int-pin property
From: Lorenzo Bianconi @ 2017-01-22 10:30 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
In-Reply-To: <20170122103052.8769-1-lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
Add st,drdy-int-pin property to select interrupt pin of the package
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
---
Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt b/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
index ed3cdac..cf81afd 100644
--- a/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
+++ b/Documentation/devicetree/bindings/iio/imu/st_lsm6dsx.txt
@@ -7,6 +7,8 @@ Required properties:
- reg: i2c address of the sensor / spi cs line
Optional properties:
+- st,drdy-int-pin: the pin on the package that will be used to signal
+ "data ready" (valid values: 1 or 2).
- interrupt-parent: should be the phandle for the interrupt controller
- interrupts: interrupt mapping for IRQ. It should be configured with
flags IRQ_TYPE_LEVEL_HIGH or IRQ_TYPE_EDGE_RISING.
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 02/22] irqchip: DT bindings for Cortina Gemini irqchip
From: Linus Walleij @ 2017-01-22 12:16 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Hans Ulli Kroll, Florian Fainelli, Thomas Gleixner, Marc Zyngier
Cc: Janos Laube, Paulius Zaleckas,
openwrt-devel-p3rKhJxN3npAfugRpC6u6w, Arnd Bergmann,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Linus Walleij,
devicetree-u79uwXL29TY76Z2rM5mHXA
This adds device tree bindings for the Cortina Gemini interrupt
controller. They are pretty standard.
Cc: Janos Laube <janos.dev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Paulius Zaleckas <paulius.zaleckas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Hans Ulli Kroll <ulli.kroll-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Cc: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
irqchip maintainers: please just apply this when you feel pleased
with it. These portions are functionally orthogonal to the series,
it is just in a series for context.
---
.../cortina,gemini-interrupt-controller.txt | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/cortina,gemini-interrupt-controller.txt
diff --git a/Documentation/devicetree/bindings/interrupt-controller/cortina,gemini-interrupt-controller.txt b/Documentation/devicetree/bindings/interrupt-controller/cortina,gemini-interrupt-controller.txt
new file mode 100644
index 000000000000..97c1167fa533
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/cortina,gemini-interrupt-controller.txt
@@ -0,0 +1,22 @@
+* Cortina Systems Gemini interrupt controller
+
+This interrupt controller is found on the Gemini SoCs.
+
+Required properties:
+- compatible: must be "cortina,gemini-interrupt-controller"
+- reg: The register bank for the interrupt controller.
+- interrupt-controller: Identifies the node as an interrupt controller
+- #interrupt-cells: The number of cells to define the interrupts.
+ Must be 2 as the controller can specify level or rising edge
+ IRQs. The bindings follows the standard binding for controllers
+ with two cells specified in
+ interrupt-controller/interrupts.txt
+
+Example:
+
+interrupt-controller@48000000 {
+ compatible = "cortina,gemini-interrupt-controller";
+ reg = <0x48000000 0x1000>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+};
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 04/22] clocksource: add DT bindings for Cortina Gemini
From: Linus Walleij @ 2017-01-22 12:17 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Hans Ulli Kroll, Florian Fainelli, Daniel Lezcano,
Thomas Gleixner
Cc: Janos Laube, Paulius Zaleckas,
openwrt-devel-p3rKhJxN3npAfugRpC6u6w, Arnd Bergmann,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Linus Walleij,
devicetree-u79uwXL29TY76Z2rM5mHXA
This adds device tree bindings for the Cortina Systems Gemini
timer block used in these SoCs.
Cc: Janos Laube <janos.dev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Paulius Zaleckas <paulius.zaleckas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Hans Ulli Kroll <ulli.kroll-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Cc: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
clocksource maintainers: please just apply this when you feel
pleased with it. These portions are functionally orthogonal to
the series, it is just in a series for context.
---
.../bindings/timer/cortina,gemini-timer.txt | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 Documentation/devicetree/bindings/timer/cortina,gemini-timer.txt
diff --git a/Documentation/devicetree/bindings/timer/cortina,gemini-timer.txt b/Documentation/devicetree/bindings/timer/cortina,gemini-timer.txt
new file mode 100644
index 000000000000..16ea1d3b2e9e
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/cortina,gemini-timer.txt
@@ -0,0 +1,22 @@
+Cortina Systems Gemini timer
+
+This timer is embedded in the Cortina Systems Gemini SoCs.
+
+Required properties:
+
+- compatible : Must be "cortina,gemini-timer"
+- reg : Should contain registers location and length
+- interrupts : Should contain the three timer interrupts with
+ flags for rising edge
+- syscon : a phandle to the global Gemini system controller
+
+Example:
+
+timer@43000000 {
+ compatible = "cortina,gemini-timer";
+ reg = <0x43000000 0x1000>;
+ interrupts = <14 IRQ_TYPE_EDGE_RISING>, /* Timer 1 */
+ <15 IRQ_TYPE_EDGE_RISING>, /* Timer 2 */
+ <16 IRQ_TYPE_EDGE_RISING>; /* Timer 3 */
+ syscon = <&syscon>;
+};
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 06/22] gpio: add DT bindings for Cortina Gemini GPIO
From: Linus Walleij @ 2017-01-22 12:18 UTC (permalink / raw)
To: linux-arm-kernel, Hans Ulli Kroll, Florian Fainelli,
Linus Walleij, Alexandre Courbot
Cc: openwrt-devel, devicetree, Paulius Zaleckas, Arnd Bergmann,
linux-kernel, linux-gpio, Janos Laube
This adds fairly standard DT bindings for the Cortina Systems
Gemini GPIO controller.
Cc: Janos Laube <janos.dev@gmail.com>
Cc: Paulius Zaleckas <paulius.zaleckas@gmail.com>
Cc: Hans Ulli Kroll <ulli.kroll@googlemail.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Note to self: just apply this to the GPIO tree when discussion is over.
---
.../bindings/gpio/cortina,gemini-gpio.txt | 24 ++++++++++++++++++++++
1 file changed, 24 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/cortina,gemini-gpio.txt
diff --git a/Documentation/devicetree/bindings/gpio/cortina,gemini-gpio.txt b/Documentation/devicetree/bindings/gpio/cortina,gemini-gpio.txt
new file mode 100644
index 000000000000..5c9246c054e5
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/cortina,gemini-gpio.txt
@@ -0,0 +1,24 @@
+Cortina Systems Gemini GPIO Controller
+
+Required properties:
+
+- compatible : Must be "cortina,gemini-gpio"
+- reg : Should contain registers location and length
+- interrupts : Should contain the interrupt line for the GPIO block
+- gpio-controller : marks this as a GPIO controller
+- #gpio-cells : Should be 2, see gpio/gpio.txt
+- interrupt-controller : marks this as an interrupt controller
+- #interrupt-cells : a standard two-cell interrupt flag, see
+ interrupt-controller/interrupts.txt
+
+Example:
+
+gpio@4d000000 {
+ compatible = "cortina,gemini-gpio";
+ reg = <0x4d000000 0x100>;
+ interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+};
--
2.9.3
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
^ permalink raw reply related
* [PATCH 08/22] rtc: Add device tree probing to the Gemini driver
From: Linus Walleij @ 2017-01-22 12:19 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Hans Ulli Kroll, Florian Fainelli, Alessandro Zummo,
Alexandre Belloni
Cc: Janos Laube, Paulius Zaleckas,
openwrt-devel-p3rKhJxN3npAfugRpC6u6w, Arnd Bergmann,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
rtc-linux-/JYPxA39Uh5TLH3MbocFFw, Linus Walleij,
devicetree-u79uwXL29TY76Z2rM5mHXA
This adds bindings and simple probing for the Cortina Systems Gemini
SoC RTC.
Cc: Janos Laube <janos.dev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Paulius Zaleckas <paulius.zaleckas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Hans Ulli Kroll <ulli.kroll-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Cc: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
RTC maintainers: please just apply this to the RTC tree when you
are pleased with it. It is functionally orthogonal to the rest of
the series and just in a series for context.
---
Documentation/devicetree/bindings/rtc/cortina,gemini.txt | 14 ++++++++++++++
drivers/rtc/rtc-gemini.c | 7 +++++++
2 files changed, 21 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/cortina,gemini.txt
diff --git a/Documentation/devicetree/bindings/rtc/cortina,gemini.txt b/Documentation/devicetree/bindings/rtc/cortina,gemini.txt
new file mode 100644
index 000000000000..4ce4e794ddbb
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/cortina,gemini.txt
@@ -0,0 +1,14 @@
+* Cortina Systems Gemini RTC
+
+Gemini SoC real-time clock.
+
+Required properties:
+- compatible : Should be "cortina,gemini-rtc"
+
+Examples:
+
+rtc@45000000 {
+ compatible = "cortina,gemini-rtc";
+ reg = <0x45000000 0x100>;
+ interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
+};
diff --git a/drivers/rtc/rtc-gemini.c b/drivers/rtc/rtc-gemini.c
index 688debc14348..ccf0dbadb62d 100644
--- a/drivers/rtc/rtc-gemini.c
+++ b/drivers/rtc/rtc-gemini.c
@@ -159,9 +159,16 @@ static int gemini_rtc_remove(struct platform_device *pdev)
return 0;
}
+static const struct of_device_id gemini_rtc_dt_match[] = {
+ { .compatible = "cortina,gemini-rtc" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, gemini_rtc_dt_match);
+
static struct platform_driver gemini_rtc_driver = {
.driver = {
.name = DRV_NAME,
+ .of_match_table = gemini_rtc_dt_match,
},
.probe = gemini_rtc_probe,
.remove = gemini_rtc_remove,
--
2.9.3
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply related
* [PATCH 09/22] mtd: add DT bindings for the Cortina Systems Gemini Flash
From: Linus Walleij @ 2017-01-22 12:21 UTC (permalink / raw)
To: linux-arm-kernel, Hans Ulli Kroll, Florian Fainelli,
David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger, Cyrille Pitchen
Cc: openwrt-devel, devicetree, Paulius Zaleckas, Arnd Bergmann,
linux-kernel, linux-mtd, Janos Laube
This adds device tree bindings for the Cortina systems Gemini
flash controller, a simple physmap which however need a few
syscon bits to be poked to operate properly.
Cc: Janos Laube <janos.dev@gmail.com>
Cc: Paulius Zaleckas <paulius.zaleckas@gmail.com>
Cc: Hans Ulli Kroll <ulli.kroll@googlemail.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
MTD maintainers: please just apply this to the MTD tree when
you are happy with it. It is functionally orthogonal to the rest
of the series and is just in the series for context.
---
.../bindings/mtd/cortina,gemini-flash.txt | 24 ++++++++++++++++++++++
1 file changed, 24 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mtd/cortina,gemini-flash.txt
diff --git a/Documentation/devicetree/bindings/mtd/cortina,gemini-flash.txt b/Documentation/devicetree/bindings/mtd/cortina,gemini-flash.txt
new file mode 100644
index 000000000000..3fa1b34d69ad
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/cortina,gemini-flash.txt
@@ -0,0 +1,24 @@
+Flash device on Cortina Systems Gemini SoC
+
+This flash is regular CFI compatible (Intel or AMD extended) flash chips with
+some special bits that can be controlled by the machine's system controller.
+
+Required properties:
+- compatible : must be "cortina,gemini-flash", "cfi-flash";
+- reg : memory address for the flash chip
+- syscon : must be a phandle to the system controller
+- bank-width : width in bytes of flash interface, should be <2>
+
+For the rest of the properties, see mtd-physmap.txt.
+
+The device tree may optionally contain sub-nodes describing partitions of the
+address space. See partition.txt for more detail.
+
+Example:
+
+flash@30000000 {
+ compatible = "cortina,gemini-flash", "cfi-flash";
+ reg = <0x30000000 0x01000000>;
+ syscon = <&syscon>;
+ bank-width = <2>;
+};
--
2.9.3
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
^ permalink raw reply related
* [PATCH 11/22] ARM: dts: add top-level DT bindings for Cortina Gemini
From: Linus Walleij @ 2017-01-22 12:22 UTC (permalink / raw)
To: linux-arm-kernel, Hans Ulli Kroll, Florian Fainelli
Cc: openwrt-devel, devicetree, Paulius Zaleckas, Arnd Bergmann,
linux-kernel, Janos Laube
This adds the top level SoC bindings for Cortina systems Gemini
platforms.
Cc: Janos Laube <janos.dev@gmail.com>
Cc: Paulius Zaleckas <paulius.zaleckas@gmail.com>
Cc: Hans Ulli Kroll <ulli.kroll@googlemail.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Documentation/devicetree/bindings/arm/gemini.txt | 58 ++++++++++++++++++++++++
1 file changed, 58 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/gemini.txt
diff --git a/Documentation/devicetree/bindings/arm/gemini.txt b/Documentation/devicetree/bindings/arm/gemini.txt
new file mode 100644
index 000000000000..28ce7db0cfd3
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/gemini.txt
@@ -0,0 +1,58 @@
+Cortina systems Gemini platforms
+
+The gemini SoC is an ARMv4 SoC from Cortina systems used for NAS
+and similar usecases.
+
+Required properties (in root node):
+ compatible = "cortina,gemini";
+
+Required nodes:
+
+- syscon: the root node must have a system controller node pointing to the
+ global control registers, with the compatible string
+ "cortina,gemini-syscon", "syscon";
+
+- timer: the root node must have a timer node pointing to the SoC timer
+ block, with the compatible string "cortina,gemini-timer"
+ See: clocksource/cortina,gemini-timer.txt
+
+- intcon: the root node must have an interrupt controller node pointing to
+ the SoC interrupt controller block, with the compatible string
+ "cortina,gemini-interrupt-controller"
+ See interrupt-controller/cortina,gemini-interrupt-controller.txt
+
+Example:
+
+/ {
+ interrupt-parent = <&intcon>;
+
+ syscon: syscon@40000000 {
+ compatible = "cortina,gemini-syscon", "syscon";
+ reg = <0x40000000 0x1000>;
+ };
+
+ timer@43000000 {
+ compatible = "cortina,gemini-timer";
+ reg = <0x43000000 0x1000>;
+ interrupt-parent = <&intcon>;
+ interrupts = <14 IRQ_TYPE_EDGE_RISING>, /* Timer 1 */
+ <15 IRQ_TYPE_EDGE_RISING>, /* Timer 2 */
+ <16 IRQ_TYPE_EDGE_RISING>; /* Timer 3 */
+ syscon = <&syscon>;
+ };
+
+ uart0: serial@42000000 {
+ compatible = "ns16550a";
+ reg = <0x42000000 0x100>;
+ clock-frequency = <48000000>;
+ interrupts = <18 IRQ_TYPE_LEVEL_HIGH>;
+ reg-shift = <2>;
+ };
+
+ intcon: interrupt-controller@48000000 {
+ compatible = "cortina,gemini-interrupt-controller";
+ reg = <0x48000000 0x1000>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+};
--
2.9.3
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
^ permalink raw reply related
* Re: [PATCH 1/2] devicetree: add documentation for MAX30102 oximeter
From: Jonathan Cameron @ 2017-01-22 12:22 UTC (permalink / raw)
To: Matt Ranostay
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170120054100.902-2-matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
On 20/01/17 05:40, Matt Ranostay wrote:
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
> ---
> .../devicetree/bindings/iio/health/max30102.txt | 30 ++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/health/max30102.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/health/max30102.txt b/Documentation/devicetree/bindings/iio/health/max30102.txt
> new file mode 100644
> index 000000000000..c93d1bb25597
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/health/max30102.txt
> @@ -0,0 +1,30 @@
> +Maxim MAX30102 heart rate and pulse oximeter sensor
> +
> +* https://datasheets.maximintegrated.com/en/ds/MAX30102.pdf
> +
> +Required properties:
> + - compatible: must be "maxim,max30102"
> + - reg: the I2C address of the sensor
> + - interrupt-parent: should be the phandle for the interrupt controller
> + - interrupts: the sole interrupt generated by the device
> +
> + Refer to interrupt-controller/interrupts.txt for generic
> + interrupt client node bindings.
> +
> +Optional properties:
> + - maxim,red-led-current-microamp: configuration for RED LED current
> + - maxim,ir-led-current-microamp: configuration for IR LED current
> +
> + Note that each step is approximately 200 microamps, ranging from 0 uA to
> + 50800 uA.
Are these due to the hardware present, or are we looking at something that should
be controllable from userspace?
> +
> +Example:
> +
> +max30100@57 {
> + compatible = "maxim,max30102";
> + reg = <57>;
> + maxim,red-led-current-microamp = <7000>;
> + maxim,ir-led-current-microamp = <7000>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <16 2>;
> +};
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 1/4] Documentation: dt-bindings: add the Amlogic Meson SAR ADC documentation
From: Jonathan Cameron @ 2017-01-22 12:30 UTC (permalink / raw)
To: Martin Blumenstingl, Rob Herring
Cc: knaack.h, lars, pmeerw, mark.rutland, khilman, linux-iio,
devicetree, linux-amlogic, linux-clk, carlo, catalin.marinas,
will.deacon, mturquette, sboyd, narmstrong, linux-arm-kernel
In-Reply-To: <CAFBinCDghzRXnzdBd0qh=LYDTpkQg7r2zdj1QE7cRKAU0fAFPQ@mail.gmail.com>
On 21/01/17 23:10, Martin Blumenstingl wrote:
> Hi Rob,
>
> thanks for reviewing this!
>
> On Sat, Jan 21, 2017 at 9:56 PM, Rob Herring <robh@kernel.org> wrote:
>> On Thu, Jan 19, 2017 at 03:58:19PM +0100, Martin Blumenstingl wrote:
>>> This adds the devicetree binding documentation for the SAR ADC found in
>>> Amlogic Meson SoCs.
>>> Currently only the GXBB, GXL and GXM SoCs are supported.
>>>
>>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>>> Tested-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>> .../bindings/iio/adc/amlogic,meson-saradc.txt | 31 ++++++++++++++++++++++
>>> 1 file changed, 31 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt b/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt
>>> new file mode 100644
>>> index 000000000000..9a0bec7afc63
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt
>>> @@ -0,0 +1,31 @@
>>> +* Amlogic Meson SAR (Successive Approximation Register) A/D converter
>>> +
>>> +Required properties:
>>> +- compatible: depending on the SoC this should be one of:
>>> + - "amlogic,meson-gxbb-saradc" for GXBB
>>> + - "amlogic,meson-gxl-saradc" for GXL and GXM
>>
>> GXL and GXM should probably be 2 compatibles.
> Amlogic's ADC driver does not differentiate between GXL and GXM
> (because both SoCs are identical except the CPU and GPU cores).
> I can still introduce a separate binding for GXM if you want (better
> safe than sorry and takes less than 5 minutes) - just let me know
Probably best to do so as it's just conceivable there will be a hardware
bug in the the silicon or similar. Also makes things a little more
'obviously correct'.
Jonathan
>
>>> + along with the generic "amlogic,meson-saradc"
>>> +- reg: the physical base address and length of the registers
>>> +- clocks: phandle and clock identifier (see clock-names)
>>> +- clock-names: mandatory clocks:
>>> + - "clkin" for the reference clock (typically XTAL)
>>> + - "core" for the SAR ADC core clock
>>> + optional clocks:
>>> + - "sana" for the analog clock
>>> + - "adc_clk" for the ADC (sampling) clock
>>> + - "adc_sel" for the ADC (sampling) clock mux
>>> +- vref-supply: the regulator supply for the ADC reference voltage
>>> +- #io-channel-cells: must be 1, see ../iio-bindings.txt
>>> +
>>> +Example:
>>> + saradc: adc@8680 {
>>> + compatible = "amlogic,meson-gxl-saradc", "amlogic,meson-saradc";
>>> + #io-channel-cells = <1>;
>>> + reg = <0x0 0x8680 0x0 0x34>;
>>> + clocks = <&xtal>,
>>> + <&clkc CLKID_SAR_ADC>,
>>> + <&clkc CLKID_SANA>,
>>> + <&clkc CLKID_SAR_ADC_CLK>,
>>> + <&clkc CLKID_SAR_ADC_SEL>;
>>> + clock-names = "clkin", "core", "sana", "adc_clk", "adc_sel";
>>> + };
>>> --
>>> 2.11.0
>>>
^ permalink raw reply
* Re: [PATCH] ARM: dts: imx6q-utilite-pro: enable 2nd display pipeline
From: Christopher Spinrath @ 2017-01-22 12:51 UTC (permalink / raw)
To: Philipp Zabel
Cc: mark.rutland, devicetree, christopher.spinrath, linux, robh+dt,
grinberg, kernel, fabio.estevam, shawnguo, linux-arm-kernel
In-Reply-To: <7b5acaa225ee4e068ae058e8aaf49a44@rwthex-w2-b.rwth-ad.de>
Hi Philipp,
sorry for the late answer; I'm quite busy with other stuff these days.
On 01/19/2017 03:34 PM, Philipp Zabel wrote:
> Hi Christopher,
>
> On Wed, 2017-01-18 at 23:20 +0100, Christopher Spinrath wrote:
>> Hi Philipp,
>>
>> turns out I have a question on your comment after all:
>>
>> On 01/17/2017 07:35 PM, Christopher Spinrath wrote:
>>> Hi Philipp,
>>>
>>> thanks for the review!
>>>
>>> On 01/17/2017 09:57 AM, Philipp Zabel wrote:
>>>> [...]
>>>>> +
>>>>> + parallel-display {
>>>>> + compatible = "fsl,imx-parallel-display";
>>>>> + #address-cells = <1>;
>>>>> + #size-cells = <0>;
>>>>> + pinctrl-names = "default";
>>>>> + pinctrl-0 = <&pinctrl_ipu1>;
>>>>> +
>>>>> + interface-pix-fmt = "rgb24";
>>>>
>>>> This is not necessary if the connector created by the tpf410 has the
>>>> correct media bus format set in its display_info structure. This can be
>>>> done in tfp410_attach, before calling drm_mode_connector_attach_encoder:
>>>>
>>>> u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
>>>>
>>>> drm_display_info_set_bus_formats(&dvi->connector.display_info,
>>>> &bus_format, 1);
>>>>
>>>> After this is done, the above line should be removed in a follow-up
>>>> patch.
>>
>> On closer inspection the tfp410 can handle rgb12, rgb24, and DVI
>> formats. Considering this it feels wrong to hardcode the bus format to
>> rgb24 (isn't it?).
>
> That is a good point, I agree. I have some thoughts on this:
>
>> So a solution might be to add a property specifying the bus format to
>> the tfp410 binding. But then we would effectively just move this
>> property from one node to another. I wonder if this is still desireable...?
>
> If this is configurable on both ends, the necessary setting is neither a
> hardware property of the bridge, nor of the display interface, but
> rather one of the board specific wiring between the two (if at all).
>
> Ideally all possible settings should be known to the drivers and the
> best format should be negotiated. Note that display_info.bus_formats
> already is an array of possible bus formats, even though the parallel
> display driver currently only looks at the first element.
>
> If there is no limitation imposed by the wiring, presenting best format
> first in that array seems reasonable for now.
The tfp410 can be operated either in i2c mode or in dump mode (i2c is
not wired up). The Linux driver does only support dump mode so far. (In
essence the driver does nothing but the neccesary boilerplate stuff.)
While the format can be read and changed neither is possible in the dumb
mode. The format is configured by connecting some pins of the chip to
ground. So we have to deal with the "otherwise" case below.
> Otherwise I think the links should describe the parallel bus layout,
> which is specified for the (input) media video interfaces:
>
> Documentation/devicetree/bindings/media/video-interfaces.txt
>
> Using the
> bus-width = <24>;
> or
> bus-width = <12>;
> property the tfp510 driver could choose which bus_formats to add to the
> display_info.
I see your point that the format is a property of the wiring and, thus,
it is better to utilizie the bus-width property in the of graph remote
nodes from a modelling point of view.
On the other hand, the parallel-display already has some data on the
wiring: the pinctrl settings. This weakens the above reasoning because
the information is splitted up. Moreover, a quick grep session revealed
that on all arm platforms the format is obtained from the "SoC-side"
(read parallel-display) node, if it is dynamic. Is there some effort to
change that? (essentially, I'm trying to understand here the
justification for the change we are discussing).
Cheers,
Christopher
> regards
> Philipp
>
^ permalink raw reply
* Re: [PATCH 1/7] iio: adc: stm32: add support for triggered buffer mode
From: Jonathan Cameron @ 2017-01-22 12:53 UTC (permalink / raw)
To: Fabrice Gasnier, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
alexandre.torgue-qxv4g6HH51o, lars-Qo5EllUWu/uELgA04lAiVw,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A,
benjamin.gaignard-qxv4g6HH51o
In-Reply-To: <1484832854-6314-2-git-send-email-fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
On 19/01/17 13:34, Fabrice Gasnier wrote:
> STM32 ADC conversions can be launched using hardware triggers.
> It can be used to start conversion sequences (group of channels).
> Selected channels are select via sequence registers.
> Trigger source is selected via 'extsel' (external trigger mux).
> Trigger polarity is set to rising edge by default.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
A few small bits and pieces inline. The only significant element is about
making the data flow conform a little better to our poorly documented :(
typical data flow. All about what the trigger is responsible for rather than
the device.
Looks pretty good to me!
Jonathan
> ---
> drivers/iio/adc/Kconfig | 2 +
> drivers/iio/adc/stm32-adc.c | 349 +++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 348 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 9c8b558..33341f4 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -446,6 +446,8 @@ config STM32_ADC_CORE
> depends on ARCH_STM32 || COMPILE_TEST
> depends on OF
> depends on REGULATOR
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> help
> Select this option to enable the core driver for STMicroelectronics
> STM32 analog-to-digital converter (ADC).
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 5715e79..8d0b74b 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -22,11 +22,16 @@
> #include <linux/clk.h>
> #include <linux/delay.h>
> #include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> #include <linux/interrupt.h>
> #include <linux/io.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> #include <linux/of.h>
> +#include <linux/slab.h>
>
> #include "stm32-adc-core.h"
>
> @@ -58,21 +63,66 @@
>
> /* STM32F4_ADC_CR2 - bit fields */
> #define STM32F4_SWSTART BIT(30)
> +#define STM32F4_EXTEN_SHIFT 28
> #define STM32F4_EXTEN_MASK GENMASK(29, 28)
> +#define STM32F4_EXTSEL_SHIFT 24
> +#define STM32F4_EXTSEL_MASK GENMASK(27, 24)
> #define STM32F4_EOCS BIT(10)
> #define STM32F4_ADON BIT(0)
>
> /* STM32F4_ADC_SQR1 - bit fields */
> #define STM32F4_L_SHIFT 20
> #define STM32F4_L_MASK GENMASK(23, 20)
> +#define STM32F4_SQ16_SHIFT 15
> +#define STM32F4_SQ16_MASK GENMASK(19, 15)
> +#define STM32F4_SQ15_SHIFT 10
> +#define STM32F4_SQ15_MASK GENMASK(14, 10)
> +#define STM32F4_SQ14_SHIFT 5
> +#define STM32F4_SQ14_MASK GENMASK(9, 5)
> +#define STM32F4_SQ13_SHIFT 0
> +#define STM32F4_SQ13_MASK GENMASK(4, 0)
> +
> +/* STM32F4_ADC_SQR2 - bit fields */
> +#define STM32F4_SQ12_SHIFT 25
> +#define STM32F4_SQ12_MASK GENMASK(29, 25)
> +#define STM32F4_SQ11_SHIFT 20
> +#define STM32F4_SQ11_MASK GENMASK(24, 20)
> +#define STM32F4_SQ10_SHIFT 15
> +#define STM32F4_SQ10_MASK GENMASK(19, 15)
> +#define STM32F4_SQ9_SHIFT 10
> +#define STM32F4_SQ9_MASK GENMASK(14, 10)
> +#define STM32F4_SQ8_SHIFT 5
> +#define STM32F4_SQ8_MASK GENMASK(9, 5)
> +#define STM32F4_SQ7_SHIFT 0
> +#define STM32F4_SQ7_MASK GENMASK(4, 0)
>
> /* STM32F4_ADC_SQR3 - bit fields */
> +#define STM32F4_SQ6_SHIFT 25
> +#define STM32F4_SQ6_MASK GENMASK(29, 25)
> +#define STM32F4_SQ5_SHIFT 20
> +#define STM32F4_SQ5_MASK GENMASK(24, 20)
> +#define STM32F4_SQ4_SHIFT 15
> +#define STM32F4_SQ4_MASK GENMASK(19, 15)
> +#define STM32F4_SQ3_SHIFT 10
> +#define STM32F4_SQ3_MASK GENMASK(14, 10)
> +#define STM32F4_SQ2_SHIFT 5
> +#define STM32F4_SQ2_MASK GENMASK(9, 5)
> #define STM32F4_SQ1_SHIFT 0
> #define STM32F4_SQ1_MASK GENMASK(4, 0)
>
Given all the above are only used in a big table which basically says what
the are, I wonder in this particular case if there is any real benefit to
using the defines? Might actually be clearer just to have the numbers
inline. What do you think? I'm not fussed either way.
> +#define STM32_ADC_MAX_SQ 16 /* SQ1..SQ16 */
> #define STM32_ADC_TIMEOUT_US 100000
> #define STM32_ADC_TIMEOUT (msecs_to_jiffies(STM32_ADC_TIMEOUT_US / 1000))
>
> +/* External trigger enable */
> +enum stm32_adc_exten {
> + STM32_EXTEN_SWTRIG,
> + STM32_EXTEN_HWTRIG_RISING_EDGE,
> + STM32_EXTEN_HWTRIG_FALLING_EDGE,
> + STM32_EXTEN_HWTRIG_BOTH_EDGES,
> +};
> +
> +
> /**
> * struct stm32_adc - private data of each ADC IIO instance
> * @common: reference to ADC block common data
> @@ -82,6 +132,8 @@
> * @clk: clock for this adc instance
> * @irq: interrupt for this adc instance
> * @lock: spinlock
> + * @bufi: data buffer index
> + * @num_conv: expected number of scan conversions
> */
> struct stm32_adc {
> struct stm32_adc_common *common;
> @@ -91,6 +143,8 @@ struct stm32_adc {
> struct clk *clk;
> int irq;
> spinlock_t lock; /* interrupt lock */
> + int bufi;
> + int num_conv;
> };
>
> /**
> @@ -105,6 +159,18 @@ struct stm32_adc_chan_spec {
> const char *name;
> };
>
> +/**
> + * stm32_adc_regs - stm32 ADC misc registers & bitfield desc
> + * @reg: register offset
> + * @mask: bitfield mask
> + * @shift: left shift
> + */
> +struct stm32_adc_regs {
> + int reg;
> + int mask;
> + int shift;
You could use the FIELD_PREP macro to avoid having to store both the mask
and shift. Up to you though as definitely a matter for personal taste!
> +};
> +
> /* Input definitions common for all STM32F4 instances */
> static const struct stm32_adc_chan_spec stm32f4_adc123_channels[] = {
> { IIO_VOLTAGE, 0, "in0" },
> @@ -126,6 +192,33 @@ struct stm32_adc_chan_spec {
> };
>
> /**
> + * stm32f4_sqr_regs - describe regular sequence registers
> + * - L: sequence len (register & bit field)
> + * - SQ1..SQ16: sequence entries (register & bit field)
> + */
> +static const struct stm32_adc_regs stm32f4_sqr_regs[STM32_ADC_MAX_SQ + 1] = {
> + /* L: len bit field description to be kept as first element */
> + { STM32F4_ADC_SQR1, STM32F4_L_MASK, STM32F4_L_SHIFT },
> + /* SQ1..SQ16 registers & bit fields */
> + { STM32F4_ADC_SQR3, STM32F4_SQ1_MASK, STM32F4_SQ1_SHIFT },
> + { STM32F4_ADC_SQR3, STM32F4_SQ2_MASK, STM32F4_SQ2_SHIFT },
> + { STM32F4_ADC_SQR3, STM32F4_SQ3_MASK, STM32F4_SQ3_SHIFT },
> + { STM32F4_ADC_SQR3, STM32F4_SQ4_MASK, STM32F4_SQ4_SHIFT },
> + { STM32F4_ADC_SQR3, STM32F4_SQ5_MASK, STM32F4_SQ5_SHIFT },
> + { STM32F4_ADC_SQR3, STM32F4_SQ6_MASK, STM32F4_SQ6_SHIFT },
> + { STM32F4_ADC_SQR2, STM32F4_SQ7_MASK, STM32F4_SQ7_SHIFT },
> + { STM32F4_ADC_SQR2, STM32F4_SQ8_MASK, STM32F4_SQ8_SHIFT },
> + { STM32F4_ADC_SQR2, STM32F4_SQ9_MASK, STM32F4_SQ9_SHIFT },
> + { STM32F4_ADC_SQR2, STM32F4_SQ10_MASK, STM32F4_SQ10_SHIFT },
> + { STM32F4_ADC_SQR2, STM32F4_SQ11_MASK, STM32F4_SQ11_SHIFT },
> + { STM32F4_ADC_SQR2, STM32F4_SQ12_MASK, STM32F4_SQ12_SHIFT },
> + { STM32F4_ADC_SQR1, STM32F4_SQ13_MASK, STM32F4_SQ13_SHIFT },
> + { STM32F4_ADC_SQR1, STM32F4_SQ14_MASK, STM32F4_SQ14_SHIFT },
> + { STM32F4_ADC_SQR1, STM32F4_SQ15_MASK, STM32F4_SQ15_SHIFT },
> + { STM32F4_ADC_SQR1, STM32F4_SQ16_MASK, STM32F4_SQ16_SHIFT },
> +};
> +
> +/**
> * STM32 ADC registers access routines
> * @adc: stm32 adc instance
> * @reg: reg offset in adc instance
> @@ -211,6 +304,106 @@ static void stm32_adc_stop_conv(struct stm32_adc *adc)
> }
>
> /**
> + * stm32_adc_conf_scan_seq() - Build regular channels scan sequence
> + * @indio_dev: IIO device
> + * @scan_mask: channels to be converted
> + *
> + * Conversion sequence :
> + * Configure ADC scan sequence based on selected channels in scan_mask.
> + * Add channels to SQR registers, from scan_mask LSB to MSB, then
> + * program sequence len.
> + */
> +static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
> + const unsigned long *scan_mask)
> +{
> + struct stm32_adc *adc = iio_priv(indio_dev);
> + const struct iio_chan_spec *chan;
> + u32 val, bit;
> + int i = 0;
> +
> + for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
> + chan = indio_dev->channels + bit;
> + /*
> + * Assign one channel per SQ entry in regular
> + * sequence, starting with SQ1.
> + */
> + i++;
> + if (i > STM32_ADC_MAX_SQ)
> + return -EINVAL;
> +
> + dev_dbg(&indio_dev->dev, "%s chan %d to SQ%d\n",
> + __func__, chan->channel, i);
> +
> + val = stm32_adc_readl(adc, stm32f4_sqr_regs[i].reg);
> + val &= ~stm32f4_sqr_regs[i].mask;
> + val |= chan->channel << stm32f4_sqr_regs[i].shift;
> + stm32_adc_writel(adc, stm32f4_sqr_regs[i].reg, val);
> + }
> +
> + if (!i)
> + return -EINVAL;
> +
> + /* Sequence len */
> + val = stm32_adc_readl(adc, stm32f4_sqr_regs[0].reg);
> + val &= ~stm32f4_sqr_regs[0].mask;
> + val |= ((i - 1) << stm32f4_sqr_regs[0].shift);
> + stm32_adc_writel(adc, stm32f4_sqr_regs[0].reg, val);
> +
> + return 0;
> +}
> +
> +/**
> + * stm32_adc_get_trig_extsel() - Get external trigger selection
> + * @indio_dev: IIO device
> + * @trig: trigger
> + *
> + * Returns trigger extsel value, if trig matches, -EINVAL otherwise.
> + */
> +static int stm32_adc_get_trig_extsel(struct iio_dev *indio_dev,
> + struct iio_trigger *trig)
> +{
> + return -EINVAL;
?????
> +}
> +
> +/**
> + * stm32_adc_set_trig() - Set a regular trigger
> + * @indio_dev: IIO device
> + * @trig: IIO trigger
> + *
> + * Set trigger source/polarity (e.g. SW, or HW with polarity) :
> + * - if HW trigger disabled (e.g. trig == NULL, conversion launched by sw)
> + * - if HW trigger enabled, set source & polarity
> + */
> +static int stm32_adc_set_trig(struct iio_dev *indio_dev,
> + struct iio_trigger *trig)
> +{
> + struct stm32_adc *adc = iio_priv(indio_dev);
> + u32 val, extsel = 0, exten = STM32_EXTEN_SWTRIG;
> + unsigned long flags;
> + int ret;
> +
> + if (trig) {
> + ret = stm32_adc_get_trig_extsel(indio_dev, trig);
> + if (ret < 0)
> + return ret;
> +
> + /* set trigger source, default to rising edge */
Interesting. The complexity of this device kicks in again.
If we are talking true exposed hardware pin (which I think it can be?) then
this ought to come from DT. For the case of trigging on either edge of the
pwm style signals, we probably want to figure some way of allowing userspace
to control this. For now in either case this is fine though! Got to start
somewhere.
> + extsel = ret;
> + exten = STM32_EXTEN_HWTRIG_RISING_EDGE;
> + }
> +
> + spin_lock_irqsave(&adc->lock, flags);
> + val = stm32_adc_readl(adc, STM32F4_ADC_CR2);
> + val &= ~(STM32F4_EXTEN_MASK | STM32F4_EXTSEL_MASK);
> + val |= exten << STM32F4_EXTEN_SHIFT;
> + val |= extsel << STM32F4_EXTSEL_SHIFT;
> + stm32_adc_writel(adc, STM32F4_ADC_CR2, val);
> + spin_unlock_irqrestore(&adc->lock, flags);
> +
> + return 0;
> +}
> +
> +/**
> * stm32_adc_single_conv() - Performs a single conversion
> * @indio_dev: IIO device
> * @chan: IIO channel
> @@ -234,6 +427,7 @@ static int stm32_adc_single_conv(struct iio_dev *indio_dev,
> reinit_completion(&adc->completion);
>
> adc->buffer = &result;
> + adc->bufi = 0;
>
> /* Program chan number in regular sequence */
> val = stm32_adc_readl(adc, STM32F4_ADC_SQR3);
> @@ -301,17 +495,58 @@ static int stm32_adc_read_raw(struct iio_dev *indio_dev,
> static irqreturn_t stm32_adc_isr(int irq, void *data)
> {
> struct stm32_adc *adc = data;
> + struct iio_dev *indio_dev = iio_priv_to_dev(adc);
> u32 status = stm32_adc_readl(adc, STM32F4_ADC_SR);
>
> if (status & STM32F4_EOC) {
> - *adc->buffer = stm32_adc_readw(adc, STM32F4_ADC_DR);
> - complete(&adc->completion);
Normally I'd have put the separation between trigger and device a bit
earlier and had the actual reads in the function called by iio_trigger_poll
(and here after the wait for completion for the sysfs reads).
I think it will make little practical difference, but it will conform better
to the data flow model (that kicks around in my head and has been in various
presentations others have done) of the trigger indicating either:
1. Data ready
2. Data should be ready - i.e. grab it now if on demand.
The device side is responsible for then actually doing the read.
Code wise I think this means moving a few lines but I may have missed some
details.
> + adc->buffer[adc->bufi] = stm32_adc_readw(adc, STM32F4_ADC_DR);
> + if (iio_buffer_enabled(indio_dev)) {
> + adc->bufi++;
> + if (adc->bufi >= adc->num_conv) {
> + stm32_adc_conv_irq_disable(adc);
> + iio_trigger_poll(indio_dev->trig);
> + }
> + } else {
> + complete(&adc->completion);
> + }
> return IRQ_HANDLED;
> }
>
> return IRQ_NONE;
> }
>
> +/**
> + * stm32_adc_validate_trigger() - validate trigger for stm32 adc
> + * @indio_dev: IIO device
> + * @trig: new trigger
> + *
> + * Returns: 0 if trig matches one of the triggers registered by stm32 adc
> + * driver, -EINVAL otherwise.
> + */
> +static int stm32_adc_validate_trigger(struct iio_dev *indio_dev,
> + struct iio_trigger *trig)
> +{
> + return stm32_adc_get_trig_extsel(indio_dev, trig) < 0 ? -EINVAL : 0;
> +}
> +
> +static int stm32_adc_update_scan_mode(struct iio_dev *indio_dev,
> + const unsigned long *scan_mask)
> +{
> + struct stm32_adc *adc = iio_priv(indio_dev);
> + int ret;
> + u32 bit;
> +
> + adc->num_conv = 0;
> + for_each_set_bit(bit, scan_mask, indio_dev->masklength)
> + adc->num_conv++;
Isn't there a count bits function? bitmap_weight I think which using the
hamming weight.
> +
> + ret = stm32_adc_conf_scan_seq(indio_dev, scan_mask);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> static int stm32_adc_of_xlate(struct iio_dev *indio_dev,
> const struct of_phandle_args *iiospec)
> {
> @@ -350,11 +585,106 @@ static int stm32_adc_debugfs_reg_access(struct iio_dev *indio_dev,
>
> static const struct iio_info stm32_adc_iio_info = {
> .read_raw = stm32_adc_read_raw,
> + .validate_trigger = stm32_adc_validate_trigger,
> + .update_scan_mode = stm32_adc_update_scan_mode,
> .debugfs_reg_access = stm32_adc_debugfs_reg_access,
> .of_xlate = stm32_adc_of_xlate,
> .driver_module = THIS_MODULE,
> };
>
> +static int stm32_adc_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + struct stm32_adc *adc = iio_priv(indio_dev);
> +
> + /* Reset adc buffer index */
> + adc->bufi = 0;
> +
> + /* Allocate adc buffer */
> + adc->buffer = kzalloc(indio_dev->scan_bytes, GFP_KERNEL);
Is this big enough to justify a separate allocation? I'd be tempted to
just allocate the maximum possible size as part of the stm32_adc structure..
> + if (!adc->buffer)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> +static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
> +{
> + struct stm32_adc *adc = iio_priv(indio_dev);
> + int ret;
> +
> + ret = stm32_adc_set_trig(indio_dev, indio_dev->trig);
> + if (ret) {
> + dev_err(&indio_dev->dev, "Can't set trigger\n");
> + return ret;
> + }
> +
> + ret = iio_triggered_buffer_postenable(indio_dev);
> + if (ret < 0)
Handle unwinding of previous calls if this fails.
> + return ret;
> +
> + stm32_adc_conv_irq_enable(adc);
> + stm32_adc_start_conv(adc);
> +
> + return 0;
> +}
> +
> +static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
> +{
> + struct stm32_adc *adc = iio_priv(indio_dev);
> + int ret;
> +
> + stm32_adc_stop_conv(adc);
> + stm32_adc_conv_irq_disable(adc);
> +
> + ret = iio_triggered_buffer_predisable(indio_dev);
> + if (ret < 0)
> + return ret;
> +
> + ret = stm32_adc_set_trig(indio_dev, NULL);
> + if (ret)
> + dev_err(&indio_dev->dev, "Can't clear trigger\n");
> +
> + return ret;
> +}
> +
> +static int stm32_adc_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> + struct stm32_adc *adc = iio_priv(indio_dev);
> +
> + kfree(adc->buffer);
> + adc->buffer = NULL;
> +
> + return 0;
> +}
> +
> +static const struct iio_buffer_setup_ops stm32_adc_buffer_setup_ops = {
> + .preenable = &stm32_adc_buffer_preenable,
> + .postenable = &stm32_adc_buffer_postenable,
> + .predisable = &stm32_adc_buffer_predisable,
> + .postdisable = &stm32_adc_buffer_postdisable,
> +};
> +
> +static irqreturn_t stm32_adc_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct stm32_adc *adc = iio_priv(indio_dev);
> +
> + dev_dbg(&indio_dev->dev, "%s bufi=%d\n", __func__, adc->bufi);
> +
> + /* reset buffer index */
> + adc->bufi = 0;
> + iio_push_to_buffers_with_timestamp(indio_dev, adc->buffer,
> + pf->timestamp);
> +
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + /* re-enable eoc irq */
> + stm32_adc_conv_irq_enable(adc);
> +
> + return IRQ_HANDLED;
> +}
> +
> static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
> struct iio_chan_spec *chan,
> const struct stm32_adc_chan_spec *channel,
> @@ -471,14 +801,26 @@ static int stm32_adc_probe(struct platform_device *pdev)
> if (ret < 0)
> goto err_clk_disable;
>
> + ret = iio_triggered_buffer_setup(indio_dev,
> + &iio_pollfunc_store_time,
> + &stm32_adc_trigger_handler,
> + &stm32_adc_buffer_setup_ops);
> + if (ret) {
> + dev_err(&pdev->dev, "buffer setup failed\n");
> + goto err_clk_disable;
> + }
> +
> ret = iio_device_register(indio_dev);
> if (ret) {
> dev_err(&pdev->dev, "iio dev register failed\n");
> - goto err_clk_disable;
> + goto err_buffer_cleanup;
> }
>
> return 0;
>
> +err_buffer_cleanup:
> + iio_triggered_buffer_cleanup(indio_dev);
> +
> err_clk_disable:
> clk_disable_unprepare(adc->clk);
>
> @@ -491,6 +833,7 @@ static int stm32_adc_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = iio_priv_to_dev(adc);
>
> iio_device_unregister(indio_dev);
> + iio_triggered_buffer_cleanup(indio_dev);
> clk_disable_unprepare(adc->clk);
>
> return 0;
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/7] iio: adc: stm32: Enable use of stm32 timer triggers
From: Jonathan Cameron @ 2017-01-22 12:55 UTC (permalink / raw)
To: Fabrice Gasnier, linux, robh+dt, linux-arm-kernel, devicetree,
linux-kernel
Cc: mark.rutland, benjamin.gaignard, lars, alexandre.torgue,
linux-iio, pmeerw, mcoquelin.stm32, knaack.h, benjamin.gaignard
In-Reply-To: <1484832854-6314-3-git-send-email-fabrice.gasnier@st.com>
On 19/01/17 13:34, Fabrice Gasnier wrote:
> STM32 ADC has external timer trigger sources. Use stm32 timer triggers
> API (e.g. is_stm32_timer_trigger()) with local ADC lookup table to
> validate a trigger can be used.
> This also provides correct trigger selection value (e.g. extsel).
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Looks good. Observations inline.
> ---
> drivers/iio/adc/Kconfig | 2 ++
> drivers/iio/adc/stm32-adc.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 62 insertions(+)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 33341f4..9a7b090 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -447,6 +447,8 @@ config STM32_ADC_CORE
> depends on OF
> depends on REGULATOR
> select IIO_BUFFER
> + select MFD_STM32_TIMERS
> + select IIO_STM32_TIMER_TRIGGER
> select IIO_TRIGGERED_BUFFER
> help
> Select this option to enable the core driver for STMicroelectronics
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 8d0b74b..30708bc 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -23,6 +23,7 @@
> #include <linux/delay.h>
> #include <linux/iio/iio.h>
> #include <linux/iio/buffer.h>
> +#include <linux/iio/timer/stm32-timer-trigger.h>
> #include <linux/iio/trigger.h>
> #include <linux/iio/trigger_consumer.h>
> #include <linux/iio/triggered_buffer.h>
> @@ -122,6 +123,35 @@ enum stm32_adc_exten {
> STM32_EXTEN_HWTRIG_BOTH_EDGES,
> };
>
> +/* extsel - trigger mux selection value */
> +enum stm32_adc_extsel {
> + STM32_EXT0,
> + STM32_EXT1,
> + STM32_EXT2,
> + STM32_EXT3,
> + STM32_EXT4,
> + STM32_EXT5,
> + STM32_EXT6,
> + STM32_EXT7,
> + STM32_EXT8,
> + STM32_EXT9,
> + STM32_EXT10,
> + STM32_EXT11,
> + STM32_EXT12,
> + STM32_EXT13,
> + STM32_EXT14,
> + STM32_EXT15,
> +};
> +
> +/**
> + * struct stm32_adc_trig_info - ADC trigger info
> + * @name: name of the trigger, corresponding to its source
> + * @extsel: trigger selection
> + */
> +struct stm32_adc_trig_info {
> + const char *name;
> + enum stm32_adc_extsel extsel;
> +};
>
> /**
> * struct stm32_adc - private data of each ADC IIO instance
> @@ -218,6 +248,26 @@ struct stm32_adc_regs {
> { STM32F4_ADC_SQR1, STM32F4_SQ16_MASK, STM32F4_SQ16_SHIFT },
> };
>
> +/* STM32F4 external trigger sources for all instances */
> +static struct stm32_adc_trig_info stm32f4_adc_timer_trigs[] = {
> + { TIM1_CH1, STM32_EXT0 },
> + { TIM1_CH2, STM32_EXT1 },
> + { TIM1_CH3, STM32_EXT2 },
> + { TIM2_CH2, STM32_EXT3 },
> + { TIM2_CH3, STM32_EXT4 },
> + { TIM2_CH4, STM32_EXT5 },
> + { TIM2_TRGO, STM32_EXT6 },
> + { TIM3_CH1, STM32_EXT7 },
> + { TIM3_TRGO, STM32_EXT8 },
> + { TIM4_CH4, STM32_EXT9 },
> + { TIM5_CH1, STM32_EXT10 },
> + { TIM5_CH2, STM32_EXT11 },
> + { TIM5_CH3, STM32_EXT12 },
> + { TIM8_CH1, STM32_EXT13 },
> + { TIM8_TRGO, STM32_EXT14 },
> + {}, /* sentinel */
> +};
> +
> /**
> * STM32 ADC registers access routines
> * @adc: stm32 adc instance
> @@ -362,6 +412,16 @@ static int stm32_adc_conf_scan_seq(struct iio_dev *indio_dev,
> static int stm32_adc_get_trig_extsel(struct iio_dev *indio_dev,
> struct iio_trigger *trig)
> {
> + int i;
Ah. This makes more sense than patch 1 on it's own did.
> +
> + /* lookup triggers registered by stm32 timer trigger driver */
> + for (i = 0; stm32f4_adc_timer_trigs[i].name; i++) {
> + if (is_stm32_timer_trigger(trig) &&
> + !strcmp(stm32f4_adc_timer_trigs[i].name, trig->name)) {
> + return stm32f4_adc_timer_trigs[i].extsel;
Good. The combination of the first check and the name match should make this safe
against those triggers that can be assigned arbitrary names.
> + }
> + }
> +
> return -EINVAL;
> }
>
>
^ permalink raw reply
* Re: [PATCH 3/7] iio: adc: stm32: add trigger polarity extended attribute
From: Jonathan Cameron @ 2017-01-22 12:58 UTC (permalink / raw)
To: Fabrice Gasnier, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, mark.rutland-5wv7dgnIgG8,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
alexandre.torgue-qxv4g6HH51o, lars-Qo5EllUWu/uELgA04lAiVw,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A,
benjamin.gaignard-qxv4g6HH51o
In-Reply-To: <1484832854-6314-4-git-send-email-fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
On 19/01/17 13:34, Fabrice Gasnier wrote:
> Define extended attribute so that user may choose rising, falling or both
> edges for external trigger sources.
> Default to rising edge in case it isn't set.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier-qxv4g6HH51o@public.gmane.org>
Creates a custom attibute. Documentation please!
/Documentation/ABI/testing/sysfs-bus-iio-stm32 or similar.
The approach seems reasonable, but easier to discuss when it's
formally described.
I'd also not care about characters saved and go with
trigger_polarity rather than the short form (unless this you have
a good reason for it!)
Jonathan
> ---
> drivers/iio/adc/stm32-adc.c | 51 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 30708bc..9753c39 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -164,6 +164,7 @@ struct stm32_adc_trig_info {
> * @lock: spinlock
> * @bufi: data buffer index
> * @num_conv: expected number of scan conversions
> + * @exten: external trigger config (enable/polarity)
> */
> struct stm32_adc {
> struct stm32_adc_common *common;
> @@ -175,6 +176,7 @@ struct stm32_adc {
> spinlock_t lock; /* interrupt lock */
> int bufi;
> int num_conv;
> + enum stm32_adc_exten exten;
> };
>
> /**
> @@ -449,7 +451,9 @@ static int stm32_adc_set_trig(struct iio_dev *indio_dev,
>
> /* set trigger source, default to rising edge */
> extsel = ret;
> - exten = STM32_EXTEN_HWTRIG_RISING_EDGE;
> + if (adc->exten == STM32_EXTEN_SWTRIG)
> + adc->exten = STM32_EXTEN_HWTRIG_RISING_EDGE;
> + exten = adc->exten;
> }
>
> spin_lock_irqsave(&adc->lock, flags);
> @@ -463,6 +467,39 @@ static int stm32_adc_set_trig(struct iio_dev *indio_dev,
> return 0;
> }
>
> +static int stm32_adc_set_trig_pol(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + unsigned int type)
> +{
> + struct stm32_adc *adc = iio_priv(indio_dev);
> +
> + adc->exten = type;
> +
> + return 0;
> +}
> +
> +static int stm32_adc_get_trig_pol(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct stm32_adc *adc = iio_priv(indio_dev);
> +
> + return adc->exten;
> +}
> +
> +static const char * const stm32_trig_pol_items[] = {
> + [STM32_EXTEN_SWTRIG] = "swtrig",
> + [STM32_EXTEN_HWTRIG_RISING_EDGE] = "rising-edge",
> + [STM32_EXTEN_HWTRIG_FALLING_EDGE] = "falling-edge",
> + [STM32_EXTEN_HWTRIG_BOTH_EDGES] = "both-edges",
> +};
> +
> +const struct iio_enum stm32_adc_trig_pol = {
> + .items = stm32_trig_pol_items,
> + .num_items = ARRAY_SIZE(stm32_trig_pol_items),
> + .get = stm32_adc_get_trig_pol,
> + .set = stm32_adc_set_trig_pol,
> +};
> +
> /**
> * stm32_adc_single_conv() - Performs a single conversion
> * @indio_dev: IIO device
> @@ -745,6 +782,17 @@ static irqreturn_t stm32_adc_trigger_handler(int irq, void *p)
> return IRQ_HANDLED;
> }
>
> +static const struct iio_chan_spec_ext_info stm32_adc_ext_info[] = {
> + IIO_ENUM("trigger_pol", IIO_SHARED_BY_ALL, &stm32_adc_trig_pol),
> + {
> + .name = "trigger_pol_available",
> + .shared = IIO_SHARED_BY_ALL,
> + .read = iio_enum_available_read,
> + .private = (uintptr_t)&stm32_adc_trig_pol,
> + },
> + {},
> +};
> +
> static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
> struct iio_chan_spec *chan,
> const struct stm32_adc_chan_spec *channel,
> @@ -760,6 +808,7 @@ static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
> chan->scan_type.sign = 'u';
> chan->scan_type.realbits = 12;
> chan->scan_type.storagebits = 16;
> + chan->ext_info = stm32_adc_ext_info;
> }
>
> static int stm32_adc_chan_of_init(struct iio_dev *indio_dev)
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox