* [PATCH v2] rtc: add support for EPSON TOYOCOM RTC-7301SF/DG
From: Akinobu Mita @ 2016-12-03 16:13 UTC (permalink / raw)
To: rtc-linux-/JYPxA39Uh5TLH3MbocFFw,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Akinobu Mita, Alessandro Zummo, Alexandre Belloni
This adds support for EPSON TOYOCOM RTC-7301SF/DG which has parallel
interface compatible with SRAM.
This driver supports basic clock, calendar and alarm functionality.
Tested with Microblaze linux running on Artix7 FPGA board with my own
custom IP for RTC-7301.
Signed-off-by: Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Alessandro Zummo <a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org>
Cc: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
* Changes from v1
- Add interrupt-parent property in the DT example
- Fix typo for RTC7301_TIMER_CONTROL_* register bit field names
- Don't forcibly disable alarm in the initialization
.../devicetree/bindings/rtc/epson,rtc7301.txt | 16 +
drivers/rtc/Kconfig | 11 +
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-r7301.c | 455 +++++++++++++++++++++
4 files changed, 483 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/epson,rtc7301.txt
create mode 100644 drivers/rtc/rtc-r7301.c
diff --git a/Documentation/devicetree/bindings/rtc/epson,rtc7301.txt b/Documentation/devicetree/bindings/rtc/epson,rtc7301.txt
new file mode 100644
index 0000000..5f9df3f
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/epson,rtc7301.txt
@@ -0,0 +1,16 @@
+EPSON TOYOCOM RTC-7301SF/DG
+
+Required properties:
+
+- compatible: Should be "epson,rtc7301sf" or "epson,rtc7301dg"
+- reg: Specifies base physical address and size of the registers.
+- interrupts: A single interrupt specifier.
+
+Example:
+
+rtc: rtc@44a00000 {
+ compatible = "epson,rtc7301dg";
+ reg = <0x44a00000 0x10000>;
+ interrupt-parent = <&axi_intc_0>;
+ interrupts = <3 2>;
+};
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 0a5e058..b8f1f9c 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1705,6 +1705,17 @@ config RTC_DRV_PIC32
This driver can also be built as a module. If so, the module
will be called rtc-pic32
+config RTC_DRV_R7301
+ tristate "EPSON TOYOCOM RTC-7301SF/DG"
+ select REGMAP_MMIO
+ depends on OF && HAS_IOMEM
+ help
+ If you say yes here you get support for the EPSON TOYOCOM
+ RTC-7301SF/DG chips.
+
+ This driver can also be built as a module. If so, the module
+ will be called rtc-r7301.
+
comment "HID Sensor RTC drivers"
config RTC_DRV_HID_SENSOR_TIME
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 1ac694a..f13ab1c 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -120,6 +120,7 @@ obj-$(CONFIG_RTC_DRV_PM8XXX) += rtc-pm8xxx.o
obj-$(CONFIG_RTC_DRV_PS3) += rtc-ps3.o
obj-$(CONFIG_RTC_DRV_PUV3) += rtc-puv3.o
obj-$(CONFIG_RTC_DRV_PXA) += rtc-pxa.o
+obj-$(CONFIG_RTC_DRV_R7301) += rtc-r7301.o
obj-$(CONFIG_RTC_DRV_R9701) += rtc-r9701.o
obj-$(CONFIG_RTC_DRV_RC5T583) += rtc-rc5t583.o
obj-$(CONFIG_RTC_DRV_RK808) += rtc-rk808.o
diff --git a/drivers/rtc/rtc-r7301.c b/drivers/rtc/rtc-r7301.c
new file mode 100644
index 0000000..0929f10
--- /dev/null
+++ b/drivers/rtc/rtc-r7301.c
@@ -0,0 +1,455 @@
+/*
+ * EPSON TOYOCOM RTC-7301SF/DG Driver
+ *
+ * Copyright (c) 2016 Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+ *
+ * Based on rtc-rp5c01.c
+ *
+ * Datasheet: http://www5.epsondevice.com/en/products/parallel/rtc7301sf.html
+ */
+
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/regmap.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+
+#define DRV_NAME "rtc-r7301"
+
+enum {
+ RTC7301_1_SEC = 0x0, /* Bank 0 and Band 1 */
+ RTC7301_10_SEC = 0x1, /* Bank 0 and Band 1 */
+ RTC7301_AE = BIT(3),
+ RTC7301_1_MIN = 0x2, /* Bank 0 and Band 1 */
+ RTC7301_10_MIN = 0x3, /* Bank 0 and Band 1 */
+ RTC7301_1_HOUR = 0x4, /* Bank 0 and Band 1 */
+ RTC7301_10_HOUR = 0x5, /* Bank 0 and Band 1 */
+ RTC7301_DAY_OF_WEEK = 0x6, /* Bank 0 and Band 1 */
+ RTC7301_1_DAY = 0x7, /* Bank 0 and Band 1 */
+ RTC7301_10_DAY = 0x8, /* Bank 0 and Band 1 */
+ RTC7301_1_MONTH = 0x9, /* Bank 0 */
+ RTC7301_10_MONTH = 0xa, /* Bank 0 */
+ RTC7301_1_YEAR = 0xb, /* Bank 0 */
+ RTC7301_10_YEAR = 0xc, /* Bank 0 */
+ RTC7301_100_YEAR = 0xd, /* Bank 0 */
+ RTC7301_1000_YEAR = 0xe, /* Bank 0 */
+ RTC7301_ALARM_CONTROL = 0xe, /* Bank 1 */
+ RTC7301_ALARM_CONTROL_AIE = BIT(0),
+ RTC7301_ALARM_CONTROL_AF = BIT(1),
+ RTC7301_TIMER_CONTROL = 0xe, /* Bank 2 */
+ RTC7301_TIMER_CONTROL_TIE = BIT(0),
+ RTC7301_TIMER_CONTROL_TF = BIT(1),
+ RTC7301_CONTROL = 0xf, /* All banks */
+ RTC7301_CONTROL_BUSY = BIT(0),
+ RTC7301_CONTROL_STOP = BIT(1),
+ RTC7301_CONTROL_BANK_SEL_0 = BIT(2),
+ RTC7301_CONTROL_BANK_SEL_1 = BIT(3),
+};
+
+struct rtc7301_priv {
+ struct regmap *regmap;
+ int irq;
+ spinlock_t lock;
+ u8 bank;
+};
+
+static const struct regmap_config rtc7301_regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 8,
+ .reg_stride = 4,
+};
+
+static u8 rtc7301_read(struct rtc7301_priv *priv, unsigned int reg)
+{
+ int reg_stride = regmap_get_reg_stride(priv->regmap);
+ unsigned int val;
+
+ regmap_read(priv->regmap, reg_stride * reg, &val);
+
+ return val & 0xf;
+}
+
+static void rtc7301_write(struct rtc7301_priv *priv, u8 val, unsigned int reg)
+{
+ int reg_stride = regmap_get_reg_stride(priv->regmap);
+
+ regmap_write(priv->regmap, reg_stride * reg, val);
+}
+
+static void rtc7301_update_bits(struct rtc7301_priv *priv, unsigned int reg,
+ u8 mask, u8 val)
+{
+ int reg_stride = regmap_get_reg_stride(priv->regmap);
+
+ regmap_update_bits(priv->regmap, reg_stride * reg, mask, val);
+}
+
+static int rtc7301_wait_while_busy(struct rtc7301_priv *priv)
+{
+ int retries = 100;
+
+ while (retries-- > 0) {
+ u8 val;
+
+ val = rtc7301_read(priv, RTC7301_CONTROL);
+ if (!(val & RTC7301_CONTROL_BUSY))
+ return 0;
+
+ usleep_range(200, 300);
+ }
+
+ return -ETIMEDOUT;
+}
+
+static void rtc7301_stop(struct rtc7301_priv *priv)
+{
+ rtc7301_update_bits(priv, RTC7301_CONTROL, RTC7301_CONTROL_STOP,
+ RTC7301_CONTROL_STOP);
+}
+
+static void rtc7301_start(struct rtc7301_priv *priv)
+{
+ rtc7301_update_bits(priv, RTC7301_CONTROL, RTC7301_CONTROL_STOP, 0);
+}
+
+static void rtc7301_select_bank(struct rtc7301_priv *priv, u8 bank)
+{
+ u8 val = 0;
+
+ if (bank == priv->bank)
+ return;
+
+ if (bank & BIT(0))
+ val |= RTC7301_CONTROL_BANK_SEL_0;
+ if (bank & BIT(1))
+ val |= RTC7301_CONTROL_BANK_SEL_1;
+
+ rtc7301_update_bits(priv, RTC7301_CONTROL,
+ RTC7301_CONTROL_BANK_SEL_0 |
+ RTC7301_CONTROL_BANK_SEL_1, val);
+
+ priv->bank = bank;
+}
+
+static void rtc7301_get_time(struct rtc7301_priv *priv, struct rtc_time *tm,
+ bool alarm)
+{
+ int year;
+
+ tm->tm_sec = rtc7301_read(priv, RTC7301_1_SEC);
+ tm->tm_sec += (rtc7301_read(priv, RTC7301_10_SEC) & ~RTC7301_AE) * 10;
+ tm->tm_min = rtc7301_read(priv, RTC7301_1_MIN);
+ tm->tm_min += (rtc7301_read(priv, RTC7301_10_MIN) & ~RTC7301_AE) * 10;
+ tm->tm_hour = rtc7301_read(priv, RTC7301_1_HOUR);
+ tm->tm_hour += (rtc7301_read(priv, RTC7301_10_HOUR) & ~RTC7301_AE) * 10;
+ tm->tm_mday = rtc7301_read(priv, RTC7301_1_DAY);
+ tm->tm_mday += (rtc7301_read(priv, RTC7301_10_DAY) & ~RTC7301_AE) * 10;
+
+ if (alarm) {
+ tm->tm_wday = -1;
+ tm->tm_mon = -1;
+ tm->tm_year = -1;
+ tm->tm_yday = -1;
+ tm->tm_isdst = -1;
+ return;
+ }
+
+ tm->tm_wday = (rtc7301_read(priv, RTC7301_DAY_OF_WEEK) & ~RTC7301_AE);
+ tm->tm_mon = rtc7301_read(priv, RTC7301_10_MONTH) * 10 +
+ rtc7301_read(priv, RTC7301_1_MONTH) - 1;
+ year = rtc7301_read(priv, RTC7301_1000_YEAR) * 1000 +
+ rtc7301_read(priv, RTC7301_100_YEAR) * 100 +
+ rtc7301_read(priv, RTC7301_10_YEAR) * 10 +
+ rtc7301_read(priv, RTC7301_1_YEAR);
+
+ tm->tm_year = year - 1900;
+}
+
+static void rtc7301_write_time(struct rtc7301_priv *priv, struct rtc_time *tm,
+ bool alarm)
+{
+ int year;
+
+ rtc7301_write(priv, tm->tm_sec % 10, RTC7301_1_SEC);
+ rtc7301_write(priv, tm->tm_sec / 10, RTC7301_10_SEC);
+
+ rtc7301_write(priv, tm->tm_min % 10, RTC7301_1_MIN);
+ rtc7301_write(priv, tm->tm_min / 10, RTC7301_10_MIN);
+
+ rtc7301_write(priv, tm->tm_hour % 10, RTC7301_1_HOUR);
+ rtc7301_write(priv, tm->tm_hour / 10, RTC7301_10_HOUR);
+
+ rtc7301_write(priv, tm->tm_mday % 10, RTC7301_1_DAY);
+ rtc7301_write(priv, tm->tm_mday / 10, RTC7301_10_DAY);
+
+ /* Don't care for alarm register */
+ rtc7301_write(priv, alarm ? RTC7301_AE : tm->tm_wday,
+ RTC7301_DAY_OF_WEEK);
+
+ if (alarm)
+ return;
+
+ rtc7301_write(priv, (tm->tm_mon + 1) % 10, RTC7301_1_MONTH);
+ rtc7301_write(priv, (tm->tm_mon + 1) / 10, RTC7301_10_MONTH);
+
+ year = tm->tm_year + 1900;
+
+ rtc7301_write(priv, year % 10, RTC7301_1_YEAR);
+ rtc7301_write(priv, (year / 10) % 10, RTC7301_10_YEAR);
+ rtc7301_write(priv, (year / 100) % 10, RTC7301_100_YEAR);
+ rtc7301_write(priv, year / 1000, RTC7301_1000_YEAR);
+}
+
+static void rtc7301_alarm_irq(struct rtc7301_priv *priv, unsigned int enabled)
+{
+ rtc7301_update_bits(priv, RTC7301_ALARM_CONTROL,
+ RTC7301_ALARM_CONTROL_AF |
+ RTC7301_ALARM_CONTROL_AIE,
+ enabled ? RTC7301_ALARM_CONTROL_AIE : 0);
+}
+
+static int rtc7301_read_time(struct device *dev, struct rtc_time *tm)
+{
+ struct rtc7301_priv *priv = dev_get_drvdata(dev);
+ unsigned long flags;
+ int err;
+
+ spin_lock_irqsave(&priv->lock, flags);
+
+ rtc7301_select_bank(priv, 0);
+
+ err = rtc7301_wait_while_busy(priv);
+ if (!err)
+ rtc7301_get_time(priv, tm, false);
+
+ spin_unlock_irqrestore(&priv->lock, flags);
+
+ return err ? err : rtc_valid_tm(tm);
+}
+
+static int rtc7301_set_time(struct device *dev, struct rtc_time *tm)
+{
+ struct rtc7301_priv *priv = dev_get_drvdata(dev);
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->lock, flags);
+
+ rtc7301_stop(priv);
+ usleep_range(200, 300);
+ rtc7301_select_bank(priv, 0);
+ rtc7301_write_time(priv, tm, false);
+ rtc7301_start(priv);
+
+ spin_unlock_irqrestore(&priv->lock, flags);
+
+ return 0;
+}
+
+static int rtc7301_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+ struct rtc7301_priv *priv = dev_get_drvdata(dev);
+ unsigned long flags;
+ u8 alrm_ctrl;
+
+ if (priv->irq <= 0)
+ return -EINVAL;
+
+ spin_lock_irqsave(&priv->lock, flags);
+
+ rtc7301_select_bank(priv, 1);
+ rtc7301_get_time(priv, &alarm->time, true);
+
+ alrm_ctrl = rtc7301_read(priv, RTC7301_ALARM_CONTROL);
+
+ alarm->enabled = !!(alrm_ctrl & RTC7301_ALARM_CONTROL_AIE);
+ alarm->pending = !!(alrm_ctrl & RTC7301_ALARM_CONTROL_AF);
+
+ spin_unlock_irqrestore(&priv->lock, flags);
+
+ return 0;
+}
+
+static int rtc7301_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
+{
+ struct rtc7301_priv *priv = dev_get_drvdata(dev);
+ unsigned long flags;
+
+ if (priv->irq <= 0)
+ return -EINVAL;
+
+ spin_lock_irqsave(&priv->lock, flags);
+
+ rtc7301_select_bank(priv, 1);
+ rtc7301_write_time(priv, &alarm->time, true);
+ rtc7301_alarm_irq(priv, alarm->enabled);
+
+ spin_unlock_irqrestore(&priv->lock, flags);
+
+ return 0;
+}
+
+static int rtc7301_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+ struct rtc7301_priv *priv = dev_get_drvdata(dev);
+ unsigned long flags;
+
+ if (priv->irq <= 0)
+ return -EINVAL;
+
+ spin_lock_irqsave(&priv->lock, flags);
+
+ rtc7301_select_bank(priv, 1);
+ rtc7301_alarm_irq(priv, enabled);
+
+ spin_unlock_irqrestore(&priv->lock, flags);
+
+ return 0;
+}
+
+static const struct rtc_class_ops rtc7301_rtc_ops = {
+ .read_time = rtc7301_read_time,
+ .set_time = rtc7301_set_time,
+ .read_alarm = rtc7301_read_alarm,
+ .set_alarm = rtc7301_set_alarm,
+ .alarm_irq_enable = rtc7301_alarm_irq_enable,
+};
+
+static irqreturn_t rtc7301_irq_handler(int irq, void *dev_id)
+{
+ struct rtc_device *rtc = dev_id;
+ struct rtc7301_priv *priv = dev_get_drvdata(rtc->dev.parent);
+ unsigned long flags;
+ irqreturn_t ret = IRQ_NONE;
+ u8 alrm_ctrl;
+
+ spin_lock_irqsave(&priv->lock, flags);
+
+ rtc7301_select_bank(priv, 1);
+
+ alrm_ctrl = rtc7301_read(priv, RTC7301_ALARM_CONTROL);
+ if (alrm_ctrl & RTC7301_ALARM_CONTROL_AF) {
+ ret = IRQ_HANDLED;
+ rtc7301_alarm_irq(priv, false);
+ rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
+ }
+
+ spin_unlock_irqrestore(&priv->lock, flags);
+
+ return ret;
+}
+
+static void rtc7301_init(struct rtc7301_priv *priv)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->lock, flags);
+
+ rtc7301_select_bank(priv, 2);
+ rtc7301_write(priv, 0, RTC7301_TIMER_CONTROL);
+
+ spin_unlock_irqrestore(&priv->lock, flags);
+}
+
+static int __init rtc7301_rtc_probe(struct platform_device *dev)
+{
+ struct resource *res;
+ void __iomem *regs;
+ struct rtc7301_priv *priv;
+ struct rtc_device *rtc;
+ int ret;
+
+ res = platform_get_resource(dev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENODEV;
+
+ priv = devm_kzalloc(&dev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ regs = devm_ioremap_resource(&dev->dev, res);
+ if (IS_ERR(regs))
+ return PTR_ERR(regs);
+
+ priv->regmap = devm_regmap_init_mmio(&dev->dev, regs,
+ &rtc7301_regmap_config);
+ if (IS_ERR(priv->regmap))
+ return PTR_ERR(priv->regmap);
+
+ priv->irq = platform_get_irq(dev, 0);
+
+ spin_lock_init(&priv->lock);
+ priv->bank = -1;
+
+ rtc7301_init(priv);
+
+ platform_set_drvdata(dev, priv);
+
+ rtc = devm_rtc_device_register(&dev->dev, DRV_NAME, &rtc7301_rtc_ops,
+ THIS_MODULE);
+ if (IS_ERR(rtc))
+ return PTR_ERR(rtc);
+
+ if (priv->irq > 0) {
+ ret = devm_request_irq(&dev->dev, priv->irq,
+ rtc7301_irq_handler, IRQF_SHARED,
+ dev_name(&dev->dev), rtc);
+ if (ret) {
+ priv->irq = 0;
+ dev_err(&dev->dev, "unable to request IRQ\n");
+ } else {
+ device_set_wakeup_capable(&dev->dev, true);
+ }
+ }
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+
+static int rtc7301_suspend(struct device *dev)
+{
+ struct rtc7301_priv *priv = dev_get_drvdata(dev);
+
+ if (device_may_wakeup(dev))
+ enable_irq_wake(priv->irq);
+
+ return 0;
+}
+
+static int rtc7301_resume(struct device *dev)
+{
+ struct rtc7301_priv *priv = dev_get_drvdata(dev);
+
+ if (device_may_wakeup(dev))
+ disable_irq_wake(priv->irq);
+
+ return 0;
+}
+
+#endif
+
+static SIMPLE_DEV_PM_OPS(rtc7301_pm_ops, rtc7301_suspend, rtc7301_resume);
+
+static const struct of_device_id rtc7301_dt_match[] = {
+ { .compatible = "epson,rtc7301sf" },
+ { .compatible = "epson,rtc7301dg" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, rtc7301_dt_match);
+
+static struct platform_driver rtc7301_rtc_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ .of_match_table = rtc7301_dt_match,
+ .pm = &rtc7301_pm_ops,
+ },
+};
+
+module_platform_driver_probe(rtc7301_rtc_driver, rtc7301_rtc_probe);
+
+MODULE_AUTHOR("Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("EPSON TOYOCOM RTC-7301SF/DG Driver");
+MODULE_ALIAS("platform:rtc-r7301");
--
2.7.4
--
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
* Re: [PATCH] rtc: add support for EPSON TOYOCOM RTC-7301SF/DG
From: Akinobu Mita @ 2016-12-03 15:40 UTC (permalink / raw)
To: Alexandre Belloni
Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw, open list:OPEN FIRMWARE AND...,
Alessandro Zummo
In-Reply-To: <20161130210152.hiyfa3srismgvtj5-m++hUPXGwpdeoWH0uzbU5w@public.gmane.org>
2016-12-01 6:01 GMT+09:00 Alexandre Belloni
<alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
> Hi,
>
> Sorry for the very late review!
>
> It seems mostly fine for me, two small comments:
>
> On 28/08/2016 at 23:55:18 +0900, Akinobu Mita wrote :
>> diff --git a/drivers/rtc/rtc-r7301.c b/drivers/rtc/rtc-r7301.c
>> new file mode 100644
>> index 0000000..b1be281
>> --- /dev/null
>> +++ b/drivers/rtc/rtc-r7301.c
>> @@ -0,0 +1,458 @@
>> +/*
>> + * EPSON TOYOCOM RTC-7301SF/DG Driver
>> + *
>> + * Copyright (c) 2016 Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> + *
>> + * Based on rtc-rp5c01.c
>> + *
>> + * Datasheet: http://www5.epsondevice.com/en/products/parallel/rtc7301sf.html
>> + */
>> +
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/delay.h>
>> +#include <linux/regmap.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/rtc.h>
>> +
>> +#define DRV_NAME "rtc-r7301"
>> +
>> +enum {
>> + RTC7301_1_SEC = 0x0, /* Bank 0 and Band 1 */
>> + RTC7301_10_SEC = 0x1, /* Bank 0 and Band 1 */
>> + RTC7301_AE = BIT(3),
>> + RTC7301_1_MIN = 0x2, /* Bank 0 and Band 1 */
>> + RTC7301_10_MIN = 0x3, /* Bank 0 and Band 1 */
>> + RTC7301_1_HOUR = 0x4, /* Bank 0 and Band 1 */
>> + RTC7301_10_HOUR = 0x5, /* Bank 0 and Band 1 */
>> + RTC7301_DAY_OF_WEEK = 0x6, /* Bank 0 and Band 1 */
>> + RTC7301_1_DAY = 0x7, /* Bank 0 and Band 1 */
>> + RTC7301_10_DAY = 0x8, /* Bank 0 and Band 1 */
>> + RTC7301_1_MONTH = 0x9, /* Bank 0 */
>> + RTC7301_10_MONTH = 0xa, /* Bank 0 */
>> + RTC7301_1_YEAR = 0xb, /* Bank 0 */
>> + RTC7301_10_YEAR = 0xc, /* Bank 0 */
>> + RTC7301_100_YEAR = 0xd, /* Bank 0 */
>> + RTC7301_1000_YEAR = 0xe, /* Bank 0 */
>> + RTC7301_ALARM_CONTROL = 0xe, /* Bank 1 */
>> + RTC7301_ALARM_CONTROL_AIE = BIT(0),
>> + RTC7301_ALARM_CONTROL_AF = BIT(1),
>> + RTC7301_TIMER_CONTROL = 0xe, /* Bank 2 */
>> + RTC7301_ALARM_CONTROL_TIE = BIT(0),
>> + RTC7301_ALARM_CONTROL_TF = BIT(1),
>> + RTC7301_CONTROL = 0xf, /* All banks */
>> + RTC7301_CONTROL_BUSY = BIT(0),
>> + RTC7301_CONTROL_STOP = BIT(1),
>> + RTC7301_CONTROL_BANK_SEL_0 = BIT(2),
>> + RTC7301_CONTROL_BANK_SEL_1 = BIT(3),
>> +};
>> +
>
> Any particular reason why you use an enum instead of the usual #define?
No particular reason. A part of this driver is based on rtc-rp5c01.c
which declares the register addresses like this.
> [...]
>
>> +static void rtc7301_init(struct rtc7301_priv *priv)
>> +{
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&priv->lock, flags);
>> +
>> + rtc7301_select_bank(priv, 1);
>> + rtc7301_alarm_irq(priv, false);
>> +
>
> If the RTC is battery backed, it may still run with the core power off
> and maybe someone will actually expect the alarm to trigger at a later
> time.
You're right. I'll remove these lines.
--
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
* Re: [PATCH v3 -next 2/2] ARM: dts: sunxi: add support for Orange Pi Zero board
From: Icenowy Zheng @ 2016-12-03 13:24 UTC (permalink / raw)
To: Jernej Skrabec
Cc: linux-sunxi, arnd-r2nGTMty4D4,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
vishnupatekar0510-Re5JQEeQqe8AvxtiuMwx3w,
andre.przywara-5wv7dgnIgG8,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, hdegoede-H+wXaHxf7aLQT0dZR+AlfA,
wens-jdAy2FN1RRM, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
[-- Attachment #1: Type: text/html, Size: 15303 bytes --]
^ permalink raw reply
* [PATCH] arm64: dts: zx: add pcu_domain node for zx296718
From: Baoyou Xie @ 2016-12-03 11:59 UTC (permalink / raw)
To: jun.nie-QSEj5FYQhm4dnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, shawnguo-DgEjT+Ai2ygdnm+yROfE0A,
viresh.kumar-QSEj5FYQhm4dnm+yROfE0A
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
baoyou.xie-QSEj5FYQhm4dnm+yROfE0A,
xie.baoyou-Th6q7B73Y6EnDS1+zs4M5A,
chen.chaokai-Th6q7B73Y6EnDS1+zs4M5A,
wang.qiang01-Th6q7B73Y6EnDS1+zs4M5A
This patch adds the pcu_domain node, so it can be used
by zte-soc's power domain driver.
Furthermore, it adds the document of the node.
Signed-off-by: Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
Documentation/devicetree/bindings/arm/zte.txt | 11 +++++++++++
arch/arm64/boot/dts/zte/zx296718.dtsi | 7 +++++++
2 files changed, 18 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/zte.txt b/Documentation/devicetree/bindings/arm/zte.txt
index 83369785..19a7e1b 100644
--- a/Documentation/devicetree/bindings/arm/zte.txt
+++ b/Documentation/devicetree/bindings/arm/zte.txt
@@ -27,6 +27,9 @@ System management required properties:
- compatible = "zte,zx296718-aon-sysctrl"
- compatible = "zte,zx296718-sysctrl"
+Low power management required properties:
+ - compatible = "zte,zx296718-pcu"
+
Example:
aon_sysctrl: aon-sysctrl@116000 {
compatible = "zte,zx296718-aon-sysctrl", "syscon";
@@ -37,3 +40,11 @@ sysctrl: sysctrl@1463000 {
compatible = "zte,zx296718-sysctrl", "syscon";
reg = <0x1463000 0x1000>;
};
+
+pcu_domain: pcu@0x00117000 {
+ compatible = "zte,zx296718-pcu";
+ reg = <0x00117000 0x1000>;
+ #power-domain-cells = <1>;
+ status = "ok";
+};
+
diff --git a/arch/arm64/boot/dts/zte/zx296718.dtsi b/arch/arm64/boot/dts/zte/zx296718.dtsi
index b44d1d1..39e70c7 100644
--- a/arch/arm64/boot/dts/zte/zx296718.dtsi
+++ b/arch/arm64/boot/dts/zte/zx296718.dtsi
@@ -351,5 +351,12 @@
reg = <0x01480000 0x1000>;
#clock-cells = <1>;
};
+
+ pcu_domain: pcu@0x00117000 {
+ compatible = "zte,zx296718-pcu";
+ reg = <0x00117000 0x1000>;
+ #power-domain-cells = <1>;
+ status = "ok";
+ };
};
};
--
2.7.4
--
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
* Re: [PATCH v2 1/2] iio: imu: add support to lsm6dsx driver
From: Jonathan Cameron @ 2016-12-03 11:25 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
In-Reply-To: <20161130200559.29910-2-lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
On 30/11/16 20:05, Lorenzo Bianconi wrote:
> Add support to STM LSM6DS3-LSM6DSM 6-axis (acc + gyro) Mems sensor
>
> http://www.st.com/resource/en/datasheet/lsm6ds3.pdf
> http://www.st.com/resource/en/datasheet/lsm6dsm.pdf
>
> - continuous mode support
> - i2c support
> - spi support
> - sw fifo mode support
> - supported devices: lsm6ds3, lsm6dsm
>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
Hi Lorenzo,
I'm actually pretty happy with this. Most of the comments inline
are requests for more documentation. There are some non obvious corners
so please make reviewers and anyone reading this in the future's life
easier by explaining them.
We are also just at the start of the new cycle, so have plenty of time.
I'd like this one to sit on the list for a few weeks after we are happy
with it to see if anyone else would like to comment on it. It's a bit
complex for me to feel totally confortable on taking it based only on
my own review!
Jonathan
> ---
> drivers/iio/imu/Kconfig | 1 +
> drivers/iio/imu/Makefile | 2 +
> drivers/iio/imu/st_lsm6dsx/Kconfig | 23 +
> drivers/iio/imu/st_lsm6dsx/Makefile | 6 +
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 107 ++++
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 696 +++++++++++++++++++++++++++
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c | 111 +++++
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_ring.c | 401 +++++++++++++++
> drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c | 129 +++++
> 9 files changed, 1476 insertions(+)
> create mode 100644 drivers/iio/imu/st_lsm6dsx/Kconfig
> create mode 100644 drivers/iio/imu/st_lsm6dsx/Makefile
> create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
> create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_ring.c
> create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c
>
> diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
> index 1f1ad41..156630a 100644
> --- a/drivers/iio/imu/Kconfig
> +++ b/drivers/iio/imu/Kconfig
> @@ -39,6 +39,7 @@ config KMX61
> be called kmx61.
>
> source "drivers/iio/imu/inv_mpu6050/Kconfig"
> +source "drivers/iio/imu/st_lsm6dsx/Kconfig"
>
> endmenu
>
> diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile
> index c71bcd3..8b563c3 100644
> --- a/drivers/iio/imu/Makefile
> +++ b/drivers/iio/imu/Makefile
> @@ -17,3 +17,5 @@ obj-y += bmi160/
> obj-y += inv_mpu6050/
>
> obj-$(CONFIG_KMX61) += kmx61.o
> +
> +obj-y += st_lsm6dsx/
> diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig
> new file mode 100644
> index 0000000..9a0781b
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
> @@ -0,0 +1,23 @@
> +
> +config IIO_ST_LSM6DSX
> + tristate "ST_LSM6DSx driver for STM 6-axis imu Mems sensors"
> + depends on (I2C || SPI)
> + select IIO_BUFFER
> + select IIO_KFIFO_BUF
> + select IIO_ST_LSM6DSX_I2C if (I2C)
> + select IIO_ST_LSM6DSX_SPI if (SPI_MASTER)
> + help
> + Say yes here to build support for STMicroelectronics LSM6DSx imu
> + sensor. Supported devices: lsm6ds3, lsm6dsm
> +
> + To compile this driver as a module, choose M here: the module
> + will be called st_lsm6dsx.
> +
> +config IIO_ST_LSM6DSX_I2C
> + tristate
> + depends on IIO_ST_LSM6DSX
> +
> +config IIO_ST_LSM6DSX_SPI
> + tristate
> + depends on IIO_ST_LSM6DSX
> +
> diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile
> new file mode 100644
> index 0000000..812d655
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/Makefile
> @@ -0,0 +1,6 @@
> +st_lsm6dsx-y := st_lsm6dsx_core.o \
> + st_lsm6dsx_ring.o
> +
> +obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o
> +obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o
> +obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> new file mode 100644
> index 0000000..a43beab
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -0,0 +1,107 @@
> +/*
> + * STMicroelectronics st_lsm6dsx sensor driver
> + *
> + * Copyright 2016 STMicroelectronics Inc.
> + *
> + * Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> + * Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#ifndef ST_LSM6DSX_H
> +#define ST_LSM6DSX_H
> +
> +#include <linux/device.h>
> +
> +#define ST_LSM6DS3_DEV_NAME "lsm6ds3"
> +#define ST_LSM6DSM_DEV_NAME "lsm6dsm"
> +
> +#define ST_LSM6DSX_CHAN_SIZE 2
> +#define ST_LSM6DSX_SAMPLE_SIZE 6
> +#define ST_LSM6DSX_SAMPLE_DEPTH (ST_LSM6DSX_SAMPLE_SIZE / \
> + ST_LSM6DSX_CHAN_SIZE)
> +
> +#if defined(CONFIG_SPI_MASTER)
> +#define ST_LSM6DSX_RX_MAX_LENGTH 256
> +#define ST_LSM6DSX_TX_MAX_LENGTH 8
> +
> +struct st_lsm6dsx_transfer_buffer {
> + u8 rx_buf[ST_LSM6DSX_RX_MAX_LENGTH];
> + u8 tx_buf[ST_LSM6DSX_TX_MAX_LENGTH] ____cacheline_aligned;
> +};
> +#endif /* CONFIG_SPI_MASTER */
> +
> +struct st_lsm6dsx_transfer_function {
> + int (*read)(struct device *dev, u8 addr, int len, u8 *data);
> + int (*write)(struct device *dev, u8 addr, int len, u8 *data);
> +};
> +
> +struct st_lsm6dsx_reg {
> + u8 addr;
> + u8 mask;
> +};
> +
> +struct st_lsm6dsx_settings {
> + u8 wai;
> + u16 max_fifo_size;
> +};
> +
> +enum st_lsm6dsx_sensor_id {
> + ST_LSM6DSX_ID_ACC,
> + ST_LSM6DSX_ID_GYRO,
> + ST_LSM6DSX_ID_MAX,
> +};
> +
> +enum st_lsm6dsx_fifo_mode {
> + ST_LSM6DSX_FIFO_BYPASS = 0x0,
> + ST_LSM6DSX_FIFO_CONT = 0x6,
> +};
> +
> +struct st_lsm6dsx_sensor {
> + enum st_lsm6dsx_sensor_id id;
> + struct st_lsm6dsx_hw *hw;
> +
> + u32 gain;
> + u16 odr;
> +
> + u16 watermark;
Please document this structure. I've no immediate idea of what sip is.
> + u8 sip;
> + u8 decimator;
> + u8 decimator_mask;
> +
> + s64 delta_ts;
> + s64 ts;
> +};
> +
Document this one as well please. Kernel doc ideally.
> +struct st_lsm6dsx_hw {
> + const char *name;
> + struct device *dev;
> + int irq;
> + struct mutex lock;
> +
> + enum st_lsm6dsx_fifo_mode fifo_mode;
> + u8 enable_mask;
> + u8 sip;
> +
> + struct iio_dev *iio_devs[ST_LSM6DSX_ID_MAX];
> +
> + const struct st_lsm6dsx_settings *settings;
> +
> + const struct st_lsm6dsx_transfer_function *tf;
> +#if defined(CONFIG_SPI_MASTER)
> + struct st_lsm6dsx_transfer_buffer tb;
> +#endif /* CONFIG_SPI_MASTER */
> +};
> +
> +int st_lsm6dsx_probe(struct st_lsm6dsx_hw *hw);
> +int st_lsm6dsx_sensor_enable(struct st_lsm6dsx_sensor *sensor);
> +int st_lsm6dsx_sensor_disable(struct st_lsm6dsx_sensor *sensor);
> +int st_lsm6dsx_allocate_rings(struct st_lsm6dsx_hw *hw);
> +int st_lsm6dsx_write_with_mask(struct st_lsm6dsx_hw *hw, u8 addr, u8 mask,
> + u8 val);
> +int st_lsm6dsx_update_watermark(struct st_lsm6dsx_sensor *sensor,
> + u16 watermark);
> +
> +#endif /* ST_LSM6DSX_H */
> +
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> new file mode 100644
> index 0000000..ae4cf30
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -0,0 +1,696 @@
> +/*
> + * STMicroelectronics st_lsm6dsx sensor driver
Perhaps a little more detail on the device in here?
> + *
> + * Copyright 2016 STMicroelectronics Inc.
> + *
> + * Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> + * Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <asm/unaligned.h>
> +
> +#include "st_lsm6dsx.h"
> +
> +#define ST_LSM6DSX_REG_ACC_DEC_MASK 0x07
> +#define ST_LSM6DSX_REG_GYRO_DEC_MASK 0x38
> +#define ST_LSM6DSX_REG_INT1_ADDR 0x0d
> +#define ST_LSM6DSX_REG_FIFO_FTH_IRQ_MASK 0x08
> +#define ST_LSM6DSX_REG_WHOAMI_ADDR 0x0f
> +#define ST_LSM6DSX_REG_RESET_ADDR 0x12
> +#define ST_LSM6DSX_REG_RESET_MASK 0x01
> +#define ST_LSM6DSX_REG_BDU_ADDR 0x12
> +#define ST_LSM6DSX_REG_BDU_MASK 0x40
> +#define ST_LSM6DSX_REG_INT2_ON_INT1_ADDR 0x13
> +#define ST_LSM6DSX_REG_INT2_ON_INT1_MASK 0x20
> +#define ST_LSM6DSX_REG_ROUNDING_ADDR 0x16
> +#define ST_LSM6DSX_REG_ROUNDING_MASK 0x04
> +#define ST_LSM6DSX_REG_LIR_ADDR 0x58
> +#define ST_LSM6DSX_REG_LIR_MASK 0x01
> +
> +#define ST_LSM6DSX_REG_ACC_ODR_ADDR 0x10
> +#define ST_LSM6DSX_REG_ACC_ODR_MASK 0xf0
> +#define ST_LSM6DSX_REG_ACC_FS_ADDR 0x10
> +#define ST_LSM6DSX_REG_ACC_FS_MASK 0x0c
> +#define ST_LSM6DSX_REG_ACC_OUT_X_L_ADDR 0x28
> +#define ST_LSM6DSX_REG_ACC_OUT_Y_L_ADDR 0x2a
> +#define ST_LSM6DSX_REG_ACC_OUT_Z_L_ADDR 0x2c
> +
> +#define ST_LSM6DSX_REG_GYRO_ODR_ADDR 0x11
> +#define ST_LSM6DSX_REG_GYRO_ODR_MASK 0xf0
> +#define ST_LSM6DSX_REG_GYRO_FS_ADDR 0x11
> +#define ST_LSM6DSX_REG_GYRO_FS_MASK 0x0c
> +#define ST_LSM6DSX_REG_GYRO_OUT_X_L_ADDR 0x22
> +#define ST_LSM6DSX_REG_GYRO_OUT_Y_L_ADDR 0x24
> +#define ST_LSM6DSX_REG_GYRO_OUT_Z_L_ADDR 0x26
> +
> +#define ST_LSM6DS3_WHOAMI 0x69
> +#define ST_LSM6DSM_WHOAMI 0x6a
> +
> +#define ST_LSM6DS3_MAX_FIFO_SIZE 8192
> +#define ST_LSM6DSM_MAX_FIFO_SIZE 4096
> +
> +#define ST_LSM6DSX_ACC_FS_2G_GAIN IIO_G_TO_M_S_2(61)
> +#define ST_LSM6DSX_ACC_FS_4G_GAIN IIO_G_TO_M_S_2(122)
> +#define ST_LSM6DSX_ACC_FS_8G_GAIN IIO_G_TO_M_S_2(244)
> +#define ST_LSM6DSX_ACC_FS_16G_GAIN IIO_G_TO_M_S_2(488)
> +
> +#define ST_LSM6DSX_GYRO_FS_245_GAIN IIO_DEGREE_TO_RAD(4375)
> +#define ST_LSM6DSX_GYRO_FS_500_GAIN IIO_DEGREE_TO_RAD(8750)
> +#define ST_LSM6DSX_GYRO_FS_1000_GAIN IIO_DEGREE_TO_RAD(17500)
> +#define ST_LSM6DSX_GYRO_FS_2000_GAIN IIO_DEGREE_TO_RAD(70000)
> +
> +struct st_lsm6dsx_odr {
> + u16 hz;
> + u8 val;
> +};
> +
> +#define ST_LSM6DSX_ODR_LIST_SIZE 6
> +struct st_lsm6dsx_odr_table_entry {
> + struct st_lsm6dsx_reg reg;
> + struct st_lsm6dsx_odr odr_avl[ST_LSM6DSX_ODR_LIST_SIZE];
> +};
> +
> +static const struct st_lsm6dsx_odr_table_entry st_lsm6dsx_odr_table[] = {
> + [ST_LSM6DSX_ID_ACC] = {
> + .reg = {
> + .addr = ST_LSM6DSX_REG_ACC_ODR_ADDR,
> + .mask = ST_LSM6DSX_REG_ACC_ODR_MASK,
> + },
> + .odr_avl[0] = { 13, 0x01 },
> + .odr_avl[1] = { 26, 0x02 },
> + .odr_avl[2] = { 52, 0x03 },
> + .odr_avl[3] = { 104, 0x04 },
> + .odr_avl[4] = { 208, 0x05 },
> + .odr_avl[5] = { 416, 0x06 },
> + },
> + [ST_LSM6DSX_ID_GYRO] = {
> + .reg = {
> + .addr = ST_LSM6DSX_REG_GYRO_ODR_ADDR,
> + .mask = ST_LSM6DSX_REG_GYRO_ODR_MASK,
> + },
> + .odr_avl[0] = { 13, 0x01 },
> + .odr_avl[1] = { 26, 0x02 },
> + .odr_avl[2] = { 52, 0x03 },
> + .odr_avl[3] = { 104, 0x04 },
> + .odr_avl[4] = { 208, 0x05 },
> + .odr_avl[5] = { 416, 0x06 },
> + }
> +};
> +
> +struct st_lsm6dsx_fs {
> + u32 gain;
> + u8 val;
> +};
> +
> +#define ST_LSM6DSX_FS_LIST_SIZE 4
> +struct st_lsm6dsx_fs_table_entry {
> + struct st_lsm6dsx_reg reg;
> + struct st_lsm6dsx_fs fs_avl[ST_LSM6DSX_FS_LIST_SIZE];
> +};
> +
> +static const struct st_lsm6dsx_fs_table_entry st_lsm6dsx_fs_table[] = {
> + [ST_LSM6DSX_ID_ACC] = {
> + .reg = {
> + .addr = ST_LSM6DSX_REG_ACC_FS_ADDR,
> + .mask = ST_LSM6DSX_REG_ACC_FS_MASK,
> + },
> + .fs_avl[0] = { ST_LSM6DSX_ACC_FS_2G_GAIN, 0x0 },
> + .fs_avl[1] = { ST_LSM6DSX_ACC_FS_4G_GAIN, 0x2 },
> + .fs_avl[2] = { ST_LSM6DSX_ACC_FS_8G_GAIN, 0x3 },
> + .fs_avl[3] = { ST_LSM6DSX_ACC_FS_16G_GAIN, 0x1 },
> + },
> + [ST_LSM6DSX_ID_GYRO] = {
> + .reg = {
> + .addr = ST_LSM6DSX_REG_GYRO_FS_ADDR,
> + .mask = ST_LSM6DSX_REG_GYRO_FS_MASK,
> + },
> + .fs_avl[0] = { ST_LSM6DSX_GYRO_FS_245_GAIN, 0x0 },
> + .fs_avl[1] = { ST_LSM6DSX_GYRO_FS_500_GAIN, 0x1 },
> + .fs_avl[2] = { ST_LSM6DSX_GYRO_FS_1000_GAIN, 0x2 },
> + .fs_avl[3] = { ST_LSM6DSX_GYRO_FS_2000_GAIN, 0x3 },
> + }
> +};
> +
> +static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
> + {
> + .wai = ST_LSM6DS3_WHOAMI,
> + .max_fifo_size = ST_LSM6DS3_MAX_FIFO_SIZE,
> + },
> + {
> + .wai = ST_LSM6DSM_WHOAMI,
> + .max_fifo_size = ST_LSM6DSM_MAX_FIFO_SIZE,
> + },
> +};
> +
> +static const struct iio_chan_spec st_lsm6dsx_acc_channels[] = {
> + {
> + .type = IIO_ACCEL,
> + .address = ST_LSM6DSX_REG_ACC_OUT_X_L_ADDR,
> + .modified = 1,
> + .channel2 = IIO_MOD_X,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> + .scan_index = 0,
> + .scan_type = {
> + .sign = 's',
> + .realbits = 16,
> + .storagebits = 16,
> + .endianness = IIO_LE,
> + },
> + },
> + {
> + .type = IIO_ACCEL,
> + .address = ST_LSM6DSX_REG_ACC_OUT_Y_L_ADDR,
> + .modified = 1,
> + .channel2 = IIO_MOD_Y,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> + .scan_index = 1,
> + .scan_type = {
> + .sign = 's',
> + .realbits = 16,
> + .storagebits = 16,
> + .endianness = IIO_LE,
> + },
> + },
> + {
> + .type = IIO_ACCEL,
> + .address = ST_LSM6DSX_REG_ACC_OUT_Z_L_ADDR,
> + .modified = 1,
> + .channel2 = IIO_MOD_Z,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> + .scan_index = 2,
> + .scan_type = {
> + .sign = 's',
> + .realbits = 16,
> + .storagebits = 16,
> + .endianness = IIO_LE,
> + },
> + },
> + IIO_CHAN_SOFT_TIMESTAMP(3),
> +};
> +
> +static const struct iio_chan_spec st_lsm6dsx_gyro_channels[] = {
> + {
> + .type = IIO_ANGL_VEL,
> + .address = ST_LSM6DSX_REG_GYRO_OUT_X_L_ADDR,
> + .modified = 1,
> + .channel2 = IIO_MOD_X,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> + .scan_index = 0,
> + .scan_type = {
> + .sign = 's',
> + .realbits = 16,
> + .storagebits = 16,
> + .endianness = IIO_LE,
> + },
> + },
> + {
> + .type = IIO_ANGL_VEL,
> + .address = ST_LSM6DSX_REG_GYRO_OUT_Y_L_ADDR,
> + .modified = 1,
> + .channel2 = IIO_MOD_Y,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> + .scan_index = 1,
> + .scan_type = {
> + .sign = 's',
> + .realbits = 16,
> + .storagebits = 16,
> + .endianness = IIO_LE,
> + },
> + },
> + {
> + .type = IIO_ANGL_VEL,
> + .address = ST_LSM6DSX_REG_GYRO_OUT_Z_L_ADDR,
> + .modified = 1,
> + .channel2 = IIO_MOD_Z,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> + .scan_index = 2,
> + .scan_type = {
> + .sign = 's',
> + .realbits = 16,
> + .storagebits = 16,
> + .endianness = IIO_LE,
> + },
> + },
> + IIO_CHAN_SOFT_TIMESTAMP(3),
> +};
> +
> +int st_lsm6dsx_write_with_mask(struct st_lsm6dsx_hw *hw, u8 addr, u8 mask,
> + u8 val)
> +{
> + u8 data;
> + int err;
> +
> + mutex_lock(&hw->lock);
> +
> + err = hw->tf->read(hw->dev, addr, sizeof(data), &data);
> + if (err < 0) {
> + dev_err(hw->dev, "failed to read %02x register\n", addr);
> + goto out;
> + }
> +
> + data = (data & ~mask) | ((val << __ffs(mask)) & mask);
> +
> + err = hw->tf->write(hw->dev, addr, sizeof(data), &data);
> + if (err < 0)
> + dev_err(hw->dev, "failed to write %02x register\n", addr);
> +
> +out:
> + mutex_unlock(&hw->lock);
> +
> + return err;
> +}
> +
> +static int st_lsm6dsx_check_whoami(struct st_lsm6dsx_hw *hw)
> +{
> + int err, i;
> + u8 data;
> +
> + err = hw->tf->read(hw->dev, ST_LSM6DSX_REG_WHOAMI_ADDR, sizeof(data),
> + &data);
> + if (err < 0) {
> + dev_err(hw->dev, "failed to read whoami register\n");
> + return err;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(st_lsm6dsx_sensor_settings); i++) {
> + if (data == st_lsm6dsx_sensor_settings[i].wai) {
> + hw->settings = &st_lsm6dsx_sensor_settings[i];
> + break;
> + }
> + }
> +
> + if (i == ARRAY_SIZE(st_lsm6dsx_sensor_settings)) {
> + dev_err(hw->dev, "unsupported whoami [%02x]\n", data);
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> +static int st_lsm6dsx_set_fs(struct st_lsm6dsx_sensor *sensor, u32 gain)
full_scale? Better to let the naming make it obvious rather than making
reviewers think about it ;) Same for other abreviations - particularly
in function names.
> +{
> + enum st_lsm6dsx_sensor_id id = sensor->id;
> + int i, err;
> + u8 val;
> +
> + for (i = 0; i < ST_LSM6DSX_FS_LIST_SIZE; i++)
> + if (st_lsm6dsx_fs_table[id].fs_avl[i].gain == gain)
> + break;
> +
> + if (i == ST_LSM6DSX_FS_LIST_SIZE)
> + return -EINVAL;
> +
> + val = st_lsm6dsx_fs_table[id].fs_avl[i].val;
> + err = st_lsm6dsx_write_with_mask(sensor->hw,
> + st_lsm6dsx_fs_table[id].reg.addr,
> + st_lsm6dsx_fs_table[id].reg.mask,
> + val);
> + if (err < 0)
> + return err;
> +
> + sensor->gain = gain;
> +
> + return 0;
> +}
> +
> +static int st_lsm6dsx_set_odr(struct st_lsm6dsx_sensor *sensor, u16 odr)
> +{
> + enum st_lsm6dsx_sensor_id id = sensor->id;
> + int i, err, val;
> +
> + for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++)
> + if (st_lsm6dsx_odr_table[id].odr_avl[i].hz == odr)
> + break;
> +
> + if (i == ST_LSM6DSX_ODR_LIST_SIZE)
> + return -EINVAL;
> +
> + val = st_lsm6dsx_odr_table[id].odr_avl[i].val;
> + err = st_lsm6dsx_write_with_mask(sensor->hw,
> + st_lsm6dsx_odr_table[id].reg.addr,
> + st_lsm6dsx_odr_table[id].reg.mask,
> + val);
> + if (err < 0)
> + return err;
> +
> + sensor->odr = odr;
> +
> + return 0;
> +}
> +
> +int st_lsm6dsx_sensor_enable(struct st_lsm6dsx_sensor *sensor)
> +{
> + int err;
> +
> + err = st_lsm6dsx_set_odr(sensor, sensor->odr);
> + if (err < 0)
> + return err;
> +
> + sensor->hw->enable_mask |= BIT(sensor->id);
> +
> + return 0;
> +}
> +
> +int st_lsm6dsx_sensor_disable(struct st_lsm6dsx_sensor *sensor)
> +{
> + enum st_lsm6dsx_sensor_id id = sensor->id;
> + int err;
> +
> + err = st_lsm6dsx_write_with_mask(sensor->hw,
> + st_lsm6dsx_odr_table[id].reg.addr,
> + st_lsm6dsx_odr_table[id].reg.mask, 0);
> + if (err < 0)
> + return err;
> +
> + sensor->hw->enable_mask &= ~BIT(id);
> +
> + return 0;
> +}
> +
> +static int st_lsm6dsx_read_oneshot(struct st_lsm6dsx_sensor *sensor,
> + u8 addr, int *val)
> +{
> + int err, delay;
> + u8 data[2];
> +
> + err = st_lsm6dsx_sensor_enable(sensor);
> + if (err < 0)
> + return err;
> +
> + delay = 1000000 / sensor->odr;
> + usleep_range(delay, 2 * delay);
> +
> + err = sensor->hw->tf->read(sensor->hw->dev, addr, sizeof(data), data);
> + if (err < 0)
> + return err;
> +
> + st_lsm6dsx_sensor_disable(sensor);
> +
> + *val = (s16)get_unaligned_le16(data);
> +
> + return IIO_VAL_INT;
> +}
> +
> +static int st_lsm6dsx_read_raw(struct iio_dev *iio_dev,
> + struct iio_chan_spec const *ch,
> + int *val, int *val2, long mask)
> +{
> + struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> + int ret;
> +
> + ret = iio_device_claim_direct_mode(iio_dev);
> + if (ret)
> + return ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = st_lsm6dsx_read_oneshot(sensor, ch->address, val);
> + break;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *val = sensor->odr;
> + ret = IIO_VAL_INT;
> + break;
> + case IIO_CHAN_INFO_SCALE:
> + *val = 0;
> + *val2 = sensor->gain;
> + ret = IIO_VAL_INT_PLUS_MICRO;
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> + iio_device_release_direct_mode(iio_dev);
> +
> + return ret;
> +}
> +
> +static int st_lsm6dsx_write_raw(struct iio_dev *iio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> + int err;
> +
> + err = iio_device_claim_direct_mode(iio_dev);
> + if (err)
> + return err;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + err = st_lsm6dsx_set_fs(sensor, val2);
> + break;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + err = st_lsm6dsx_set_odr(sensor, val);
> + break;
> + default:
> + err = -EINVAL;
> + break;
> + }
> +
> + iio_device_release_direct_mode(iio_dev);
> +
> + return err;
> +}
> +
> +static int st_lsm6dsx_set_watermark(struct iio_dev *iio_dev, unsigned int val)
> +{
> + struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> + struct st_lsm6dsx_hw *hw = sensor->hw;
> + int err, max_fifo_len;
> +
> + max_fifo_len = hw->settings->max_fifo_size / ST_LSM6DSX_SAMPLE_SIZE;
> + if (val < 1 || val > max_fifo_len)
> + return -EINVAL;
> +
> + err = st_lsm6dsx_update_watermark(sensor, val);
> + if (err < 0)
> + return err;
> +
> + sensor->watermark = val;
> +
> + return 0;
> +}
> +
You could port these over to the new available infrastructure
(see new callbacks in iio_info) but fine if you'd prefer not to for
now as that is very new.
> +static ssize_t
> +st_lsm6dsx_sysfs_sampling_frequency_avl(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct st_lsm6dsx_sensor *sensor = iio_priv(dev_get_drvdata(dev));
> + enum st_lsm6dsx_sensor_id id = sensor->id;
> + int i, len = 0;
> +
> + for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++)
> + len += scnprintf(buf + len, PAGE_SIZE - len, "%d ",
> + st_lsm6dsx_odr_table[id].odr_avl[i].hz);
> + buf[len - 1] = '\n';
> +
> + return len;
> +}
> +
> +static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct st_lsm6dsx_sensor *sensor = iio_priv(dev_get_drvdata(dev));
> + enum st_lsm6dsx_sensor_id id = sensor->id;
> + int i, len = 0;
> +
> + for (i = 0; i < ST_LSM6DSX_FS_LIST_SIZE; i++)
> + len += scnprintf(buf + len, PAGE_SIZE - len, "0.%06u ",
> + st_lsm6dsx_fs_table[id].fs_avl[i].gain);
> + buf[len - 1] = '\n';
> +
> + return len;
> +}
> +
> +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(st_lsm6dsx_sysfs_sampling_frequency_avl);
> +static IIO_DEVICE_ATTR(in_accel_scale_available, 0444,
> + st_lsm6dsx_sysfs_scale_avail, NULL, 0);
> +static IIO_DEVICE_ATTR(in_anglvel_scale_available, 0444,
> + st_lsm6dsx_sysfs_scale_avail, NULL, 0);
> +
> +static struct attribute *st_lsm6dsx_acc_attributes[] = {
> + &iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> + &iio_dev_attr_in_accel_scale_available.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group st_lsm6dsx_acc_attribute_group = {
> + .attrs = st_lsm6dsx_acc_attributes,
> +};
> +
> +static const struct iio_info st_lsm6dsx_acc_info = {
> + .driver_module = THIS_MODULE,
> + .attrs = &st_lsm6dsx_acc_attribute_group,
> + .read_raw = st_lsm6dsx_read_raw,
> + .write_raw = st_lsm6dsx_write_raw,
> + .hwfifo_set_watermark = st_lsm6dsx_set_watermark,
> +};
> +
> +static struct attribute *st_lsm6dsx_gyro_attributes[] = {
> + &iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> + &iio_dev_attr_in_anglvel_scale_available.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group st_lsm6dsx_gyro_attribute_group = {
> + .attrs = st_lsm6dsx_gyro_attributes,
> +};
> +
> +static const struct iio_info st_lsm6dsx_gyro_info = {
> + .driver_module = THIS_MODULE,
> + .attrs = &st_lsm6dsx_gyro_attribute_group,
> + .read_raw = st_lsm6dsx_read_raw,
> + .write_raw = st_lsm6dsx_write_raw,
> + .hwfifo_set_watermark = st_lsm6dsx_set_watermark,
> +};
> +
> +static const unsigned long st_lsm6dsx_available_scan_masks[] = {0x7, 0x0};
> +
> +static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
> +{
> + int err;
> + u8 data;
> +
> + data = ST_LSM6DSX_REG_RESET_MASK;
> + err = hw->tf->write(hw->dev, ST_LSM6DSX_REG_RESET_ADDR, sizeof(data),
> + &data);
> + if (err < 0)
> + return err;
> +
> + msleep(200);
> +
> + /* latch interrupts */
> + err = st_lsm6dsx_write_with_mask(hw, ST_LSM6DSX_REG_LIR_ADDR,
> + ST_LSM6DSX_REG_LIR_MASK, 1);
> + if (err < 0)
> + return err;
> +
> + /* enable BDU */
Expand BDU rather than using the acronym.
> + err = st_lsm6dsx_write_with_mask(hw, ST_LSM6DSX_REG_BDU_ADDR,
> + ST_LSM6DSX_REG_BDU_MASK, 1);
> + if (err < 0)
> + return err;
> +
> + err = st_lsm6dsx_write_with_mask(hw, ST_LSM6DSX_REG_ROUNDING_ADDR,
> + ST_LSM6DSX_REG_ROUNDING_MASK, 1);
> + if (err < 0)
> + 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);
> + 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);
> +}
> +
> +static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
> + enum st_lsm6dsx_sensor_id id)
> +{
> + struct st_lsm6dsx_sensor *sensor;
> + struct iio_dev *iio_dev;
> +
> + iio_dev = devm_iio_device_alloc(hw->dev, sizeof(*sensor));
> + if (!iio_dev)
> + return NULL;
> +
> + iio_dev->modes = INDIO_DIRECT_MODE;
> + iio_dev->dev.parent = hw->dev;
> + iio_dev->available_scan_masks = st_lsm6dsx_available_scan_masks;
> +
> + sensor = iio_priv(iio_dev);
> + sensor->id = id;
> + sensor->hw = hw;
> + sensor->odr = st_lsm6dsx_odr_table[id].odr_avl[0].hz;
> + sensor->gain = st_lsm6dsx_fs_table[id].fs_avl[0].gain;
> + sensor->watermark = 1;
> +
> + switch (id) {
> + case ST_LSM6DSX_ID_ACC:
> + iio_dev->channels = st_lsm6dsx_acc_channels;
> + iio_dev->num_channels = ARRAY_SIZE(st_lsm6dsx_acc_channels);
> + iio_dev->name = "lsm6dsx_accel";
> + iio_dev->info = &st_lsm6dsx_acc_info;
> +
> + sensor->decimator_mask = ST_LSM6DSX_REG_ACC_DEC_MASK;
> + break;
> + case ST_LSM6DSX_ID_GYRO:
> + iio_dev->channels = st_lsm6dsx_gyro_channels;
> + iio_dev->num_channels = ARRAY_SIZE(st_lsm6dsx_gyro_channels);
> + iio_dev->name = "lsm6dsx_gyro";
> + iio_dev->info = &st_lsm6dsx_gyro_info;
> +
> + sensor->decimator_mask = ST_LSM6DSX_REG_GYRO_DEC_MASK;
> + break;
> + default:
> + return NULL;
> + }
> +
> + return iio_dev;
> +}
> +
> +int st_lsm6dsx_probe(struct st_lsm6dsx_hw *hw)
> +{
> + int i, err;
> +
> + mutex_init(&hw->lock);
> +
> + err = st_lsm6dsx_check_whoami(hw);
> + if (err < 0)
> + return err;
> +
> + for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> + hw->iio_devs[i] = st_lsm6dsx_alloc_iiodev(hw, i);
> + if (!hw->iio_devs[i])
> + return -ENOMEM;
> + }
> +
> + err = st_lsm6dsx_init_device(hw);
> + if (err < 0)
> + return err;
> +
> + if (hw->irq > 0) {
> + err = st_lsm6dsx_allocate_rings(hw);
> + if (err < 0)
> + return err;
> + }
> +
> + for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> + err = devm_iio_device_register(hw->dev, hw->iio_devs[i]);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(st_lsm6dsx_probe);
> +
> +MODULE_AUTHOR("Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>");
> +MODULE_AUTHOR("Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
> new file mode 100644
> index 0000000..c80e624
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
> @@ -0,0 +1,111 @@
> +/*
> + * STMicroelectronics st_lsm6dsx i2c driver
> + *
> + * Copyright 2016 STMicroelectronics Inc.
> + *
> + * Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> + * Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +
> +#include "st_lsm6dsx.h"
> +
> +static int st_lsm6dsx_i2c_read(struct device *dev, u8 addr, int len, u8 *data)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct i2c_msg msg[2];
> +
> + msg[0].addr = client->addr;
> + msg[0].flags = client->flags;
> + msg[0].len = 1;
> + msg[0].buf = &addr;
> +
> + msg[1].addr = client->addr;
> + msg[1].flags = client->flags | I2C_M_RD;
> + msg[1].len = len;
> + msg[1].buf = data;
> +
> + return i2c_transfer(client->adapter, msg, 2);
> +}
> +
> +static int st_lsm6dsx_i2c_write(struct device *dev, u8 addr, int len, u8 *data)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct i2c_msg msg;
> + u8 send[len + 1];
> +
> + send[0] = addr;
> + memcpy(&send[1], data, len * sizeof(u8));
> +
> + msg.addr = client->addr;
> + msg.flags = client->flags;
> + msg.len = len + 1;
> + msg.buf = send;
> +
> + return i2c_transfer(client->adapter, &msg, 1);
Could (I think) do this with the smbus_write_block_data command as long as
we never use more than 32 bytes. However, probably not worth it as the
reads break those rules anyway so we'll need a fully fledged i2c controller
whatever.
> +}
> +
> +static const struct st_lsm6dsx_transfer_function st_lsm6dsx_transfer_fn = {
> + .read = st_lsm6dsx_i2c_read,
> + .write = st_lsm6dsx_i2c_write,
> +};
> +
> +static int st_lsm6dsx_i2c_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct st_lsm6dsx_hw *hw;
> +
> + hw = devm_kzalloc(&client->dev, sizeof(*hw), GFP_KERNEL);
> + if (!hw)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(client, hw);
> + hw->name = client->name;
> + hw->dev = &client->dev;
> + hw->irq = client->irq;
> + hw->tf = &st_lsm6dsx_transfer_fn;
> +
> + return st_lsm6dsx_probe(hw);
> +}
> +
> +static const struct of_device_id st_lsm6dsx_i2c_of_match[] = {
> + {
> + .compatible = "st,lsm6ds3",
> + .data = ST_LSM6DS3_DEV_NAME,
> + },
> + {
> + .compatible = "st,lsm6dsm",
> + .data = ST_LSM6DSM_DEV_NAME,
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, st_lsm6dsx_i2c_of_match);
> +
> +static const struct i2c_device_id st_lsm6dsx_i2c_id_table[] = {
> + { ST_LSM6DS3_DEV_NAME },
> + { ST_LSM6DSM_DEV_NAME },
> + {},
> +};
> +MODULE_DEVICE_TABLE(i2c, st_lsm6dsx_i2c_id_table);
> +
> +static struct i2c_driver st_lsm6dsx_driver = {
> + .driver = {
> + .name = "st_lsm6dsx_i2c",
> + .of_match_table = of_match_ptr(st_lsm6dsx_i2c_of_match),
> + },
> + .probe = st_lsm6dsx_i2c_probe,
> + .id_table = st_lsm6dsx_i2c_id_table,
> +};
> +module_i2c_driver(st_lsm6dsx_driver);
> +
> +MODULE_AUTHOR("Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>");
> +MODULE_AUTHOR("Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i2c driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_ring.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_ring.c
> new file mode 100644
> index 0000000..9a8c503
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_ring.c
> @@ -0,0 +1,401 @@
> +/*
> + * STMicroelectronics st_lsm6dsx sensor driver
Whilst it's part of the driver, a quick description of what this bit does
would be good.
> + *
> + * Copyright 2016 STMicroelectronics Inc.
> + *
> + * Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> + * Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>
> + *
> + * Licensed under the GPL-2.
> + */
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/iio/kfifo_buf.h>
> +#include <asm/unaligned.h>
> +
> +#include "st_lsm6dsx.h"
> +
> +#define ST_LSM6DSX_REG_FIFO_THL_ADDR 0x06
> +#define ST_LSM6DSX_REG_FIFO_THH_ADDR 0x07
> +#define ST_LSM6DSX_FIFO_TH_MASK 0x0fff
> +#define ST_LSM6DSX_REG_FIFO_DEC_GXL_ADDR 0x08
> +#define ST_LSM6DSX_REG_FIFO_MODE_ADDR 0x0a
> +#define ST_LSM6DSX_FIFO_MODE_MASK 0x07
> +#define ST_LSM6DSX_FIFO_ODR_MASK 0x78
> +#define ST_LSM6DSX_REG_FIFO_DIFFL_ADDR 0x3a
> +#define ST_LSM6DSX_FIFO_DIFF_MASK 0x0f
> +#define ST_LSM6DSX_FIFO_EMPTY_MASK 0x10
> +#define ST_LSM6DSX_REG_FIFO_OUTL_ADDR 0x3e
> +
> +struct st_lsm6dsx_dec_entry {
> + u8 decimator;
> + u8 val;
> +};
> +
> +static const struct st_lsm6dsx_dec_entry st_lsm6dsx_dec_table[] = {
> + { 0, 0x0 },
> + { 1, 0x1 },
> + { 2, 0x2 },
> + { 3, 0x3 },
> + { 4, 0x4 },
> + { 8, 0x5 },
> + { 16, 0x6 },
> + { 32, 0x7 },
> +};
> +
> +static int st_lsm6dsx_get_decimator_val(u8 val)
> +{
> + int i, max_size = ARRAY_SIZE(st_lsm6dsx_dec_table);
> +
> + for (i = 0; i < max_size; i++)
> + if (st_lsm6dsx_dec_table[i].decimator == val)
> + break;
> +
> + return i == max_size ? 0 : st_lsm6dsx_dec_table[i].val;
> +}
> +
> +static void st_lsm6dsx_get_max_min_odr(struct st_lsm6dsx_hw *hw,
> + u16 *max_odr, u16 *min_odr)
> +{
> + struct st_lsm6dsx_sensor *sensor;
> + int i;
> +
> + *max_odr = 0, *min_odr = ~0;
> + for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> + sensor = iio_priv(hw->iio_devs[i]);
> +
> + if (!(hw->enable_mask & BIT(sensor->id)))
> + continue;
> +
> + *max_odr = max_t(u16, *max_odr, sensor->odr);
> + *min_odr = min_t(u16, *min_odr, sensor->odr);
> + }
> +}
> +
> +static int st_lsm6dsx_update_decimators(struct st_lsm6dsx_hw *hw)
> +{
> + struct st_lsm6dsx_sensor *sensor;
> + u16 max_odr, min_odr, sip = 0;
> + int err, i;
> + u8 data;
> +
> + st_lsm6dsx_get_max_min_odr(hw, &max_odr, &min_odr);
> +
> + for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> + sensor = iio_priv(hw->iio_devs[i]);
> +
> + /* update fifo decimators and sample in pattern */
> + if (hw->enable_mask & BIT(sensor->id)) {
> + sensor->sip = sensor->odr / min_odr;
> + sensor->decimator = max_odr / sensor->odr;
> + data = st_lsm6dsx_get_decimator_val(sensor->decimator);
> + } else {
> + sensor->sip = 0;
> + sensor->decimator = 0;
> + data = 0;
> + }
> +
> + err = st_lsm6dsx_write_with_mask(hw,
> + ST_LSM6DSX_REG_FIFO_DEC_GXL_ADDR,
> + sensor->decimator_mask, data);
> + if (err < 0)
> + return err;
> +
> + sip += sensor->sip;
> + }
> + hw->sip = sip;
> +
> + return 0;
> +}
> +
> +static int st_lsm6dsx_set_fifo_mode(struct st_lsm6dsx_hw *hw,
> + enum st_lsm6dsx_fifo_mode fifo_mode)
> +{
> + u8 data;
> + int err;
> +
> + switch (fifo_mode) {
> + case ST_LSM6DSX_FIFO_BYPASS:
> + data = fifo_mode;
> + break;
> + case ST_LSM6DSX_FIFO_CONT:
use a define for that magic number.
> + data = fifo_mode | 0x40;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + err = hw->tf->write(hw->dev, ST_LSM6DSX_REG_FIFO_MODE_ADDR,
> + sizeof(data), &data);
> + if (err < 0)
> + return err;
> +
> + hw->fifo_mode = fifo_mode;
> +
> + return 0;
> +}
> +
> +int st_lsm6dsx_update_watermark(struct st_lsm6dsx_sensor *sensor, u16 watermark)
> +{
> + u16 fifo_watermark = ~0, cur_watermark, sip = 0;
> + struct st_lsm6dsx_hw *hw = sensor->hw;
> + struct st_lsm6dsx_sensor *cur_sensor;
> + int i, err;
> + u8 data;
> +
> + for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> + cur_sensor = iio_priv(hw->iio_devs[i]);
> +
> + if (!(hw->enable_mask & BIT(cur_sensor->id)))
> + continue;
> +
> + cur_watermark = (cur_sensor == sensor) ? watermark
> + : cur_sensor->watermark;
> +
> + fifo_watermark = min_t(u16, fifo_watermark, cur_watermark);
> + sip += cur_sensor->sip;
> + }
> +
> + if (!sip)
> + return 0;
> +
> + fifo_watermark = max_t(u16, fifo_watermark, sip);
> + fifo_watermark = (fifo_watermark / sip) * sip;
> + fifo_watermark = fifo_watermark * ST_LSM6DSX_SAMPLE_DEPTH;
> +
> + mutex_lock(&hw->lock);
> +
> + err = hw->tf->read(hw->dev, ST_LSM6DSX_REG_FIFO_THH_ADDR,
> + sizeof(data), &data);
> + if (err < 0)
> + goto out;
> +
> + fifo_watermark = ((data & ~ST_LSM6DSX_FIFO_TH_MASK) << 8) |
> + (fifo_watermark & ST_LSM6DSX_FIFO_TH_MASK);
> +
> + err = hw->tf->write(hw->dev, ST_LSM6DSX_REG_FIFO_THL_ADDR,
> + sizeof(fifo_watermark), (u8 *)&fifo_watermark);
> +out:
> + mutex_unlock(&hw->lock);
> +
> + return err < 0 ? err : 0;
> +}
> +
I'd like a bit of documentation on this. Wasn't immediately obvious to
me what it returns which lead me to confusion around the return in the
ring_handler_thread.
> +static int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
> +{
> + u16 fifo_len, pattern_len = hw->sip * ST_LSM6DSX_SAMPLE_SIZE;
> + int err, acc_sip, gyro_sip, read_len, offset, samples;
> + struct st_lsm6dsx_sensor *acc_sensor, *gyro_sensor;
> + s64 acc_ts, acc_delta_ts, gyro_ts, gyro_delta_ts;
> + u8 iio_buf[ALIGN(ST_LSM6DSX_SAMPLE_SIZE, sizeof(s64)) + sizeof(s64)];
> + u8 fifo_status[2], buf[pattern_len];
> +
> + err = hw->tf->read(hw->dev, ST_LSM6DSX_REG_FIFO_DIFFL_ADDR,
> + sizeof(fifo_status), fifo_status);
> + if (err < 0)
> + return err;
> +
> + if (fifo_status[1] & ST_LSM6DSX_FIFO_EMPTY_MASK)
> + return 0;
> +
> + fifo_status[1] &= ST_LSM6DSX_FIFO_DIFF_MASK;
> + fifo_len = (u16)get_unaligned_le16(fifo_status) * ST_LSM6DSX_CHAN_SIZE;
> + samples = fifo_len / ST_LSM6DSX_SAMPLE_SIZE;
> + fifo_len = (fifo_len / pattern_len) * pattern_len;
> + /*
> + * leave one complete pattern in FIFO to guarantee
> + * proper alignment
That needs more info! I have no idea what you mean by a pattern for starters.
> + */
> + fifo_len -= pattern_len;
> +
> + /* compute delta timestamp */
I want some more info in here on why we deal with delta timestamps.
> + acc_sensor = iio_priv(hw->iio_devs[ST_LSM6DSX_ID_ACC]);
> + acc_ts = acc_sensor->ts - acc_sensor->delta_ts;
> + acc_delta_ts = div_s64(acc_sensor->delta_ts * acc_sensor->decimator,
> + samples);
> +
> + gyro_sensor = iio_priv(hw->iio_devs[ST_LSM6DSX_ID_GYRO]);
> + gyro_ts = gyro_sensor->ts - gyro_sensor->delta_ts;
> + gyro_delta_ts = div_s64(gyro_sensor->delta_ts * gyro_sensor->decimator,
> + samples);
> +
> + for (read_len = 0; read_len < fifo_len; read_len += pattern_len) {
> + err = hw->tf->read(hw->dev, ST_LSM6DSX_REG_FIFO_OUTL_ADDR,
> + sizeof(buf), buf);
> + if (err < 0)
> + return err;
> +
> + gyro_sip = gyro_sensor->sip;
> + acc_sip = acc_sensor->sip;
> + offset = 0;
> +
Could you add a brief description here of the data layout as it would make
it easier to verify this code makes sense.
> + while (acc_sip > 0 || gyro_sip > 0) {
> + if (gyro_sip-- > 0) {
> + memcpy(iio_buf, &buf[offset],
> + ST_LSM6DSX_SAMPLE_SIZE);
> + iio_push_to_buffers_with_timestamp(
> + hw->iio_devs[ST_LSM6DSX_ID_GYRO],
> + iio_buf, gyro_ts);
> + offset += ST_LSM6DSX_SAMPLE_SIZE;
> + gyro_ts += gyro_delta_ts;
> + }
> +
> + if (acc_sip-- > 0) {
> + memcpy(iio_buf, &buf[offset],
> + ST_LSM6DSX_SAMPLE_SIZE);
> + iio_push_to_buffers_with_timestamp(
> + hw->iio_devs[ST_LSM6DSX_ID_ACC],
> + iio_buf, acc_ts);
> + offset += ST_LSM6DSX_SAMPLE_SIZE;
> + acc_ts += acc_delta_ts;
> + }
> + }
> + }
> +
> + return read_len;
> +}
> +
> +static int st_lsm6dsx_flush_fifo(struct st_lsm6dsx_hw *hw)
> +{
> + int err;
> +
> + disable_irq(hw->irq);
Comment on why this is needed rather than simply locking to prevent
concurrent access.
> +
> + st_lsm6dsx_read_fifo(hw);
> + err = st_lsm6dsx_set_fifo_mode(hw, ST_LSM6DSX_FIFO_BYPASS);
> +
> + enable_irq(hw->irq);
> +
> + return err;
> +}
> +
> +static int st_lsm6dsx_update_fifo(struct iio_dev *iio_dev, bool enable)
> +{
> + struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> + struct st_lsm6dsx_hw *hw = sensor->hw;
> + int err;
> +
> + if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) {
> + err = st_lsm6dsx_flush_fifo(hw);
> + if (err < 0)
> + return err;
> + }
> +
> + err = enable ? st_lsm6dsx_sensor_enable(sensor)
> + : st_lsm6dsx_sensor_disable(sensor);
I'd just do this with an if statement as it's marginally confusing to read.
Trinary operators have a lot to answer for in readability!
> + if (err < 0)
> + return err;
> +
> + err = st_lsm6dsx_update_decimators(hw);
> + if (err < 0)
> + return err;
> +
> + err = st_lsm6dsx_update_watermark(sensor, sensor->watermark);
> + if (err < 0)
> + return err;
> +
> + if (hw->enable_mask) {
> + err = st_lsm6dsx_set_fifo_mode(hw, ST_LSM6DSX_FIFO_CONT);
> + if (err < 0)
> + return err;
> +
Comment here on why we should be grabbing a timestmap whilst enabling the
buffer.
> + sensor->ts = iio_get_time_ns(iio_dev);
> + }
> +
> + return 0;
> +}
> +
> +static irqreturn_t st_lsm6dsx_ring_handler_irq(int irq, void *private)
> +{
> + struct st_lsm6dsx_hw *hw = (struct st_lsm6dsx_hw *)private;
> + struct st_lsm6dsx_sensor *sensor;
> + int i;
> +
> + if (!hw->sip)
> + return IRQ_NONE;
> +
> + for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> + sensor = iio_priv(hw->iio_devs[i]);
> +
> + if (sensor->sip > 0) {
> + s64 timestamp;
> +
> + timestamp = iio_get_time_ns(hw->iio_devs[i]);
> + sensor->delta_ts = timestamp - sensor->ts;
> + sensor->ts = timestamp;
> + }
> + }
> +
> + return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t st_lsm6dsx_ring_handler_thread(int irq, void *private)
> +{
> + struct st_lsm6dsx_hw *hw = (struct st_lsm6dsx_hw *)private;
> + int count;
> +
> + count = st_lsm6dsx_read_fifo(hw);
> +
> + return !count ? IRQ_NONE : IRQ_HANDLED;
> +}
> +
> +static int st_lsm6dsx_buffer_preenable(struct iio_dev *iio_dev)
> +{
> + return st_lsm6dsx_update_fifo(iio_dev, true);
> +}
> +
> +static int st_lsm6dsx_buffer_postdisable(struct iio_dev *iio_dev)
> +{
> + return st_lsm6dsx_update_fifo(iio_dev, false);
> +}
> +
> +static const struct iio_buffer_setup_ops st_lsm6dsx_buffer_ops = {
> + .preenable = st_lsm6dsx_buffer_preenable,
> + .postdisable = st_lsm6dsx_buffer_postdisable,
> +};
> +
> +int st_lsm6dsx_allocate_rings(struct st_lsm6dsx_hw *hw)
> +{
> + struct iio_buffer *buffer;
> + unsigned long irq_type;
> + int i, err;
> +
> + irq_type = irqd_get_trigger_type(irq_get_irq_data(hw->irq));
> +
> + switch (irq_type) {
> + case IRQF_TRIGGER_HIGH:
> + case IRQF_TRIGGER_RISING:
> + break;
> + default:
> + dev_info(hw->dev,
> + "mode %lx unsupported, using IRQF_TRIGGER_HIGH\n",
> + irq_type);
I'd argue that if the type is 'miss specified' we should be dumping out.
Obviously good to have a default for if it isn't specified at all though.
Is it easy to make this distinction?
> + irq_type = IRQF_TRIGGER_HIGH;
> + break;
> + }
> +
> + err = devm_request_threaded_irq(hw->dev, hw->irq,
> + st_lsm6dsx_ring_handler_irq,
> + st_lsm6dsx_ring_handler_thread,
> + irq_type | IRQF_ONESHOT,
> + hw->name, hw);
> + if (err) {
> + dev_err(hw->dev, "failed to request trigger irq %d\n",
> + hw->irq);
> + return err;
> + }
> +
> + for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> + buffer = devm_iio_kfifo_allocate(hw->dev);
Kfifo isn't a ring ;) Probably want to rename this appropriately to reflect
this. The ring naming in various IIO drivers predates the presence of kfifo
in the kernel when we had a hideous hand rolled ring buffer.
> + if (!buffer)
> + return -ENOMEM;
> +
> + iio_device_attach_buffer(hw->iio_devs[i], buffer);
> + hw->iio_devs[i]->modes |= INDIO_BUFFER_SOFTWARE;
> + hw->iio_devs[i]->setup_ops = &st_lsm6dsx_buffer_ops;
> + }
> +
> + return 0;
> +}
> +
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c
> new file mode 100644
> index 0000000..262eae6
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c
> @@ -0,0 +1,129 @@
> +/*
> + * STMicroelectronics st_lsm6dsx spi driver
> + *
> + * Copyright 2016 STMicroelectronics Inc.
> + *
> + * Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> + * Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +
> +#include "st_lsm6dsx.h"
> +
> +#define SENSORS_SPI_READ 0x80
> +
> +static int st_lsm6dsx_spi_read(struct device *dev, u8 addr, int len,
> + u8 *data)
> +{
> + struct spi_device *spi = to_spi_device(dev);
> + struct st_lsm6dsx_hw *hw = spi_get_drvdata(spi);
> + int err;
> +
> + struct spi_transfer xfers[] = {
> + {
> + .tx_buf = hw->tb.tx_buf,
> + .bits_per_word = 8,
> + .len = 1,
> + },
> + {
> + .rx_buf = hw->tb.rx_buf,
> + .bits_per_word = 8,
> + .len = len,
> + }
> + };
> +
> + hw->tb.tx_buf[0] = addr | SENSORS_SPI_READ;
> +
> + err = spi_sync_transfer(spi, xfers, ARRAY_SIZE(xfers));
> + if (err < 0)
> + return err;
> +
> + memcpy(data, hw->tb.rx_buf, len * sizeof(u8));
> +
> + return len;
> +}
> +
> +static int st_lsm6dsx_spi_write(struct device *dev, u8 addr, int len,
> + u8 *data)
> +{
> + struct spi_device *spi = to_spi_device(dev);
> + struct st_lsm6dsx_hw *hw = spi_get_drvdata(spi);
> +
> + struct spi_transfer xfers = {
> + .tx_buf = hw->tb.tx_buf,
> + .bits_per_word = 8,
> + .len = len + 1,
> + };
> +
> + if (len >= ST_LSM6DSX_TX_MAX_LENGTH)
> + return -ENOMEM;
> +
> + hw->tb.tx_buf[0] = addr;
> + memcpy(&hw->tb.tx_buf[1], data, len);
> +
> + return spi_sync_transfer(spi, &xfers, 1);
Why not spi_write? Would be a little simpler.. Maybe not worth it, just to
keep things similar between the write and read.
> +}
> +
> +static const struct st_lsm6dsx_transfer_function st_lsm6dsx_transfer_fn = {
> + .read = st_lsm6dsx_spi_read,
> + .write = st_lsm6dsx_spi_write,
> +};
> +
> +static int st_lsm6dsx_spi_probe(struct spi_device *spi)
> +{
> + struct st_lsm6dsx_hw *hw;
> +
> + hw = devm_kzalloc(&spi->dev, sizeof(*hw), GFP_KERNEL);
> + if (!hw)
> + return -ENOMEM;
> +
> + spi_set_drvdata(spi, hw);
> + hw->name = spi->modalias;
> + hw->dev = &spi->dev;
> + hw->irq = spi->irq;
> + hw->tf = &st_lsm6dsx_transfer_fn;
> +
> + return st_lsm6dsx_probe(hw);
> +}
> +
> +static const struct of_device_id st_lsm6dsx_spi_of_match[] = {
> + {
> + .compatible = "st,lsm6ds3",
> + .data = ST_LSM6DS3_DEV_NAME,
> + },
> + {
> + .compatible = "st,lsm6dsm",
> + .data = ST_LSM6DSM_DEV_NAME,
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, st_lsm6dsx_spi_of_match);
> +
> +static const struct spi_device_id st_lsm6dsx_spi_id_table[] = {
> + { ST_LSM6DS3_DEV_NAME },
> + { ST_LSM6DSM_DEV_NAME },
> + {},
> +};
> +MODULE_DEVICE_TABLE(spi, st_lsm6dsx_spi_id_table);
> +
> +static struct spi_driver st_lsm6dsx_driver = {
> + .driver = {
> + .name = "st_lsm6dsx_spi",
> + .of_match_table = of_match_ptr(st_lsm6dsx_spi_of_match),
> + },
> + .probe = st_lsm6dsx_spi_probe,
> + .id_table = st_lsm6dsx_spi_id_table,
> +};
> +module_spi_driver(st_lsm6dsx_driver);
> +
> +MODULE_AUTHOR("Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>");
> +MODULE_AUTHOR("Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx spi driver");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply
* Re: [PATCH v2 3/3 pci/next] PCI: rcar: Add gen3 fallback compatibility string for pcie-rcar
From: Geert Uytterhoeven @ 2016-12-03 11:19 UTC (permalink / raw)
To: Simon Horman
Cc: Bjorn Helgaas, Phil Edworthy, Magnus Damm, linux-pci,
Linux-Renesas, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20161202215016.GA21497-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
On Fri, Dec 2, 2016 at 10:50 PM, Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org> wrote:
> On Fri, Dec 02, 2016 at 11:37:00AM +0100, Simon Horman wrote:
>> --- a/drivers/pci/host/pcie-rcar.c
>> +++ b/drivers/pci/host/pcie-rcar.c
>> @@ -1078,6 +1078,8 @@ static const struct of_device_id rcar_pcie_of_match[] = {
>> { .compatible = "renesas,pcie-rcar-gen2",
>> .data = rcar_pcie_hw_init_gen2 },
>> { .compatible = "renesas,pcie-r8a7795", .data = rcar_pcie_hw_init },
>> + { .compatible = "renesas,pcie-rcar-gen3",
>> + .data = rcar_pcie_hw_init_hw_init },
>
> It looks like I failed to compile-test this.
>
> s/rcar_pcie_hw_init_hw_init/rcar_pcie_hw_init/
Bummer, and my tired eyes didn't notice...
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: [PATCH v3 0/7] mux controller abstraction and iio/i2c muxes
From: Jonathan Cameron @ 2016-12-03 10:11 UTC (permalink / raw)
To: Peter Rosin, Lars-Peter Clausen, linux-kernel
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
Peter Meerwald-Stadler, Jonathan Corbet, Arnd Bergmann,
Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio, linux-doc
In-Reply-To: <f649f482-5c1c-dd07-161a-556a9c2b31ff@axentia.se>
On 29/11/16 16:02, Peter Rosin wrote:
> On 2016-11-27 13:00, Jonathan Cameron wrote:
>> On 23/11/16 11:47, Peter Rosin wrote:
>>> On 2016-11-22 21:58, Lars-Peter Clausen wrote:
>>>> On 11/21/2016 02:17 PM, Peter Rosin wrote:
>>>> [...]
>>>>> I have a piece of hardware that is using the same 3 GPIO pins
>>>>> to control four 8-way muxes. Three of them control ADC lines
>>>>> to an ADS1015 chip with an iio driver, and the last one
>>>>> controls the SDA line of an i2c bus. We have some deployed
>>>>> code to handle this, but you do not want to see it or ever
>>>>> hear about it. I'm not sure why I even mention it. Anyway,
>>>>> the situation has nagged me to no end for quite some time.
>>>>>
>>>>> So, after first getting more intimate with the i2c muxing code
>>>>> and later discovering the drivers/iio/inkern.c file and
>>>>> writing a couple of drivers making use of it, I came up with
>>>>> what I think is an acceptable solution; add a generic mux
>>>>> controller driver (and subsystem) that is shared between all
>>>>> instances, and combine that with an iio mux driver and a new
>>>>> generic i2c mux driver. The new i2c mux I called "simple"
>>>>> since it is only hooking the i2c muxing and the new mux
>>>>> controller (much like the alsa simple card driver does for ASoC).
>>>>
>>>> While abstracting this properly is all nice and good and the way it should
>>>> be done, but it also adds a lot of complexity and the devicetree adds a lot
>>>> of restrictions on what can actually be represented.
>>>
>>> This is a characterization without any specifics. But is the
>>> characterization true? You have two complaints, complexity
>>> and restrictions with bindings.
>>>
>>>> There is a certain point where the fabric on a PCB becomes so complex that
>>>> it deserves to be a device on its own (like the audio fabric drivers).
>>>> Especially when the hardware is built with a certain application in mind and
>>>> the driver is supposed to impose policy which reflects this application. The
>>>> latter can often not properly be described with the primitives the
>>>> devicetree can offer.
>>>>
>>>> And I think your setup is very borderline what can be done in a declarative
>>>> way only and it adds a lot of complexity over a more imperative solution in
>>>> form of a driver. I think it is worth investigating about having a driver
>>>> that is specific to your fabric and handles the interdependencies of the
>>>> discrete components.
>>>
>>> So, there are three "new" concepts:
>>>
>>> 1. Sticking a mux in front of an AD-converter. That's not all that
>>> novel, nor complex. Quite the opposite, I'd say. In fact, I find it
>>> a bit amazing that there is no in-kernel support for it.
>> As ever first person who needs it and has the skills to write it gets to do it ;)
>> Congratulations Peter ;)
>>>
>>> 2. Reusing the same GPIO-pins to drive different muxes. There are
>>> obviously chips that work this way (as Jonathan pointed out) and
>>> these will at some point get used in Linux devices. I guess they
>>> already are used, but that people handle them in userspace. Or
>>> something? If this is complex, which I question, it will still need
>>> support at some point. At least that's what I believe.
>>>
>>> 3. Using the same GPIO pins to mux things handled by different
>>> subsystems. Right, this is a bit crazy, and I'd rather not have this
>>> requirement, but this HW is what it is so I'll need to handle it in
>>> some way. It is also what stops me from falling back to a userspace
>>> solution, which is probably connected to why #1 and #2 is not supported
>>> by the kernel; everybody probably does muxing in userspace. Which is
>>> not necessarily a good idea, nor how it's supposed to be done...
>>>
>>> So, the only thing that's out of the ordinary (as I see it), is #3.
>>> The question that then needs an answer is how the in-kernel solution
>>> for #1 and #2 would look if we do not consider #3.
>>>
>>> And I claim that the desired solution to #1 and #2 is pretty close
>>> to my proposal.
>>>
>>> A. You do not want mux-controller drivers in every subsystem that
>>> needs them.
>> Agreed.
>>>
>>> B. You do not want every mux-consumer to know the specifics of how to
>>> operate every kind of mux; there are muxes that are not controlled
>>> with GPIO pins...
>>>
>>> C. When you implement muxing in a subsystem, there will in some cases
>>> be a need to handle parallel muxing, where there is a need to share
>>> mux-controllers.
>>>
>>> It just feels right to separate out the mux-controller and refer to
>>> it from where a mux is needed. It solves #1 and #2. And, of course,
>>> as a bonus #3 is also solved. But my bias is obvious.
>>>
>>> And that leads us to the restrictions with the bindings. And the same
>>> thing happens; the solution for #2 also solves #3.
>>>
>>> So how do you refer to a mux-controller from where it's needed? My
>>> first proposal used a dt phandle, for the second round I put them in
>>> the parent node. It would be super if it was possible for the mux-
>>> consumer to create the mux-controller device from the same dt
>>> node that is already bound to the mux-consumer. The problem is that
>>> the mux-consumer should not hard-code which mux-controller device it
>>> should create. The best I can think of is some kind of 'mux-compatible'
>>> attribute, that works like the standard 'compatible' attribute. That
>>> would simplify the bindings for the normal case (#1) where the mux-
>>> controller isn't shared (#2 and #3). Maybe it's possible to fix this
>>> issue somehow? I simply don't know?
>> As Lars stated, it's marginal. The question is not at what point do we
>> 'have to' bother with a fabric driver, but rather at what point does it
>> make a our lives easier.
>>
>> Take you nastiest mux case described earlier.
>> The ideal would be to represent the ADC and 3 muxes as (approximately) a
>> single ADC to userspace that just happens to have somewhere near 23 inputs.
>>
>> To do that in device tree we need to describe
>>
>> 1 The adc
>> 2 The three muxes
>> 3 The software representation to pull all of these back into a single device.
>>
>> That last part to my mind trips the balance to the point where a fabric driver
>> would make sense. It's not complex. Just a few lines of code tying all the
>> elements together without ending up with a fairly fiendish setup to describe in
>> device tree.
>>
>> Also just wait until we have muxes stacked on muxes, with cross overs occuring.
>> Some of the ASoC parts can actually have effective loops if you try all the mux
>> combinations.
>>
>> So question is do we have a 'simple case description' in device tree or force
>> fabric drivers everywhere? I think I'm in favour of the simple case - which handles
>> one of your two uses nicely. The second one to do the the recombining of channels after
>> the muxes, ends up looking to me like it needs a fabric driver.
>>
>> Note we are only talking about bindings vs code based description here. I agree
>> entirely with the concept of a generic mux subsystem.
>
> Ok, take the simple case - an adc with a mux in front of it.
>
> We do not want to build a whole new iio-mux subsystem like the one under
> i2c, so from iio we want to refer to the actual mux controller driver
> which lives under the mux subsystem. Getting this far is what solves my
> "second need" [2] from the v2 thread.
>
> Agreed, doing this w/o a fabric driver spills the guts and it might be
> cleaner to build a fabric driver that takes a reference to the dpot and
> the mux controller and just knows the rest. In the end this fabric driver
> requires two things to actually make a difference; some way to instantiate
> drivers without the help of device-tree and some way make those drivers
> only provide in-kernel access (and preferably it should hide the dpot from
> userspace too, while at it).
>
> But doing all that in a fabric driver is not going to change the fact that
> the iio-mux driver is useful on its own. I bet someone else is going to
> reuse it somewhere down the line. Which means that a fabric driver would
> perhaps be nice for my "second need", but not critical, it works pretty
> well to just piggyback on the general solution .
Absolutely. No denying the usefulness of having a nice little mux subsystem
with the resulting iio-mux driver.
>
> Over to my "first need" [1] from the v2 thread.
>
> The above iio-mux driver handles the three muxed adc lines beautifully,
> just refer all three iio-muxes to the same mux controller. Agreed, with
> a fabric driver I could get one device with 25 channels instead of three
> devices with 8 channels each plus one unmuxed line directly from the adc
> device. However, that doesn't bother me at all, I may even think it is
> preferable because otherwise I'd have to come up with some way to
> identify which channel is which in that big 25-channel jungle. Another
> drawback with having a fabric driver here is that it would need to be an
> i2c-mux driver, because one of the key points of doing a fabric driver
> for my first need was to hide the shared mux, right? Instead, I have the
> new i2c-mux-simple driver that builds an i2c-mux using any mux controller
> from the mux subsystem (something that may prove useful to others in the
> future), which in my case is the same mux controller that also muxes the
> three adc lines.
>
> In short, doing fabric drivers buys me almost nothing besides having a
> slightly more distinct device tree. All the components used to describe
> this are either already accepted drivers or usable by others (at least
> the way I see it).
Fair enough. Perhaps it's not worthwhile in this case, but lets keep
the concept in mind as we move forward and see if anyone else has
more complex hardware than you do ;)
(there is always something out there!)
>
> Cheers,
> Peter
>
> [1] three adc lines and an i2c bus muxed with the same gpio pins
> [2] mcp4561 dpot -> dpot-dac -> envelope-detector-adc -> iio-mux
>
^ permalink raw reply
* Re: [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer driver in DT
From: Jonathan Cameron @ 2016-12-03 9:45 UTC (permalink / raw)
To: Benjamin Gaignard, lee.jones-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
alexandre.torgue-qxv4g6HH51o, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
linux-pwm-u79uwXL29TY76Z2rM5mHXA, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: fabrice.gasnier-qxv4g6HH51o, gerald.baeza-qxv4g6HH51o,
arnaud.pouliquen-qxv4g6HH51o,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
linaro-kernel-cunTk1MwBs8s++Sfvej+rw, Benjamin Gaignard
In-Reply-To: <1480673842-20804-8-git-send-email-benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
On 02/12/16 10:17, Benjamin Gaignard wrote:
> Add general purpose timers and it sub-nodes into DT for stm32f4.
> Define and enable pwm1 and pwm3 for stm32f469 discovery board
>
> version 3:
> - use "st,stm32-timer-trigger" in DT
>
> version 2:
> - use parameters to describe hardware capabilities
> - do not use references for pwm and iio timer subnodes
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
This didn't come out nearly as badly as I thought it would.
Seems we don't need the multiple compatibles which is good!
> ---
> arch/arm/boot/dts/stm32f429.dtsi | 333 +++++++++++++++++++++++++++++++++-
> arch/arm/boot/dts/stm32f469-disco.dts | 28 +++
> 2 files changed, 360 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> index bca491d..8c50d03 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -48,7 +48,7 @@
> #include "skeleton.dtsi"
> #include "armv7-m.dtsi"
> #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
> -
> +#include <dt-bindings/iio/timer/st,stm32-timer-triggers.h>
> / {
> clocks {
> clk_hse: clk-hse {
> @@ -355,6 +355,21 @@
> slew-rate = <2>;
> };
> };
> +
> + pwm1_pins: pwm@1 {
> + pins {
> + pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
> + <STM32F429_PB13_FUNC_TIM1_CH1N>,
> + <STM32F429_PB12_FUNC_TIM1_BKIN>;
> + };
> + };
> +
> + pwm3_pins: pwm@3 {
> + pins {
> + pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
> + <STM32F429_PB5_FUNC_TIM3_CH2>;
> + };
> + };
> };
>
> rcc: rcc@40023810 {
> @@ -426,6 +441,322 @@
> interrupts = <80>;
> clocks = <&rcc 0 38>;
> };
> +
> + gptimer1: gptimer1@40010000 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40010000 0x400>;
> + clocks = <&rcc 0 160>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm1@0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <4>;
> + st,breakinput;
> + st,complementary;
> + status = "disabled";
> + };
> +
> + timer1@0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <27>;
> + st,input-triggers-names = TIM5_TRGO,
> + TIM2_TRGO,
> + TIM4_TRGO,
> + TIM3_TRGO;
> + st,output-triggers-names = TIM1_TRGO,
> + TIM1_CH1,
> + TIM1_CH2,
> + TIM1_CH3,
> + TIM1_CH4;
> + status = "disabled";
> + };
> + };
> +
> + gptimer2: gptimer2@40000000 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40000000 0x400>;
> + clocks = <&rcc 0 128>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm2@0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <4>;
> + st,32bits-counter;
> + status = "disabled";
> + };
> +
> + timer2@0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <28>;
> + st,input-triggers-names = TIM1_TRGO,
> + TIM8_TRGO,
> + TIM3_TRGO,
> + TIM4_TRGO;
> + st,output-triggers-names = TIM2_TRGO,
> + TIM2_CH1,
> + TIM2_CH2,
> + TIM2_CH3,
> + TIM2_CH4;
> + status = "disabled";
> + };
> + };
> +
> + gptimer3: gptimer3@40000400 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40000400 0x400>;
> + clocks = <&rcc 0 129>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm3@0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <4>;
> + status = "disabled";
> + };
> +
> + timer3@0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <29>;
> + st,input-triggers-names = TIM1_TRGO,
> + TIM8_TRGO,
> + TIM5_TRGO,
> + TIM4_TRGO;
> + st,output-triggers-names = TIM3_TRGO,
> + TIM3_CH1,
> + TIM3_CH2,
> + TIM3_CH3,
> + TIM3_CH4;
> + status = "disabled";
> + };
> + };
> +
> + gptimer4: gptimer4@40000800 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40000800 0x400>;
> + clocks = <&rcc 0 130>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm4@0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <4>;
> + status = "disabled";
> + };
> +
> + timer4@0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <30>;
> + st,input-triggers-names = TIM1_TRGO,
> + TIM2_TRGO,
> + TIM3_TRGO,
> + TIM8_TRGO;
> + st,output-triggers-names = TIM4_TRGO,
> + TIM4_CH1,
> + TIM4_CH2,
> + TIM4_CH3,
> + TIM4_CH4;
> + status = "disabled";
> + };
> + };
> +
> + gptimer5: gptimer5@40000C00 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40000C00 0x400>;
> + clocks = <&rcc 0 131>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm5@0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <4>;
> + st,32bits-counter;
> + status = "disabled";
> + };
> +
> + timer5@0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <50>;
> + st,input-triggers-names = TIM2_TRGO,
> + TIM3_TRGO,
> + TIM4_TRGO,
> + TIM8_TRGO;
> + st,output-triggers-names = TIM5_TRGO,
> + TIM5_CH1,
> + TIM5_CH2,
> + TIM5_CH3,
> + TIM5_CH4;
> + status = "disabled";
> + };
> + };
> +
> + gptimer6: gptimer6@40001000 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40001000 0x400>;
> + clocks = <&rcc 0 132>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + timer6@0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <54>;
> + st,output-triggers-names = TIM6_TRGO;
> + status = "disabled";
> + };
> + };
> +
> + gptimer7: gptimer7@40001400 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40001400 0x400>;
> + clocks = <&rcc 0 133>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + timer7@0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <55>;
> + st,output-triggers-names = TIM7_TRGO;
> + status = "disabled";
> + };
> + };
> +
> + gptimer8: gptimer8@40010400 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40010400 0x400>;
> + clocks = <&rcc 0 161>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm8@0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <4>;
> + st,complementary;
> + st,breakinput;
> + status = "disabled";
> + };
> +
> + timer8@0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <46>;
> + st,input-triggers-names = TIM1_TRGO,
> + TIM2_TRGO,
> + TIM4_TRGO,
> + TIM5_TRGO;
> + st,output-triggers-names = TIM8_TRGO,
> + TIM8_CH1,
> + TIM8_CH2,
> + TIM8_CH3,
> + TIM8_CH4;
> + status = "disabled";
> + };
> + };
> +
> + gptimer9: gptimer9@40014000 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40014000 0x400>;
> + clocks = <&rcc 0 176>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm9@0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <2>;
> + status = "disabled";
> + };
> +
> + timer9@0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <24>;
> + st,input-triggers-names = TIM2_TRGO,
> + TIM3_TRGO;
> + st,output-triggers-names = TIM9_TRGO,
> + TIM9_CH1,
> + TIM9_CH2;
> + status = "disabled";
> + };
> + };
> +
> + gptimer10: gptimer10@40014400 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40014400 0x400>;
> + clocks = <&rcc 0 177>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm10@0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <1>;
> + status = "disabled";
> + };
> + };
> +
> + gptimer11: gptimer11@40014800 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40014800 0x400>;
> + clocks = <&rcc 0 178>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm11@0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <1>;
> + status = "disabled";
> + };
> + };
> +
> + gptimer12: gptimer12@40001800 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40001800 0x400>;
> + clocks = <&rcc 0 134>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm12@0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <2>;
> + status = "disabled";
> + };
> +
> + timer12@0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <43>;
> + st,input-triggers-names = TIM4_TRGO,
> + TIM5_TRGO;
> + st,output-triggers-names = TIM12_TRGO,
> + TIM12_CH1,
> + TIM12_CH2;
> + status = "disabled";
> + };
> + };
> +
> + gptimer13: gptimer13@40001C00 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40001C00 0x400>;
> + clocks = <&rcc 0 135>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm13@0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <1>;
> + status = "disabled";
> + };
> + };
> +
> + gptimer14: gptimer14@40002000 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40002000 0x400>;
> + clocks = <&rcc 0 136>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm14@0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <1>;
> + status = "disabled";
> + };
> + };
> };
> };
>
> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
> index 8a163d7..df4ca7e 100644
> --- a/arch/arm/boot/dts/stm32f469-disco.dts
> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
> @@ -81,3 +81,31 @@
> &usart3 {
> status = "okay";
> };
> +
> +&gptimer1 {
> + status = "okay";
> +
> + pwm1@0 {
> + pinctrl-0 = <&pwm1_pins>;
> + pinctrl-names = "default";
> + status = "okay";
> + };
> +
> + timer1@0 {
> + status = "okay";
> + };
> +};
> +
> +&gptimer3 {
> + status = "okay";
> +
> + pwm3@0 {
> + pinctrl-0 = <&pwm3_pins>;
> + pinctrl-names = "default";
> + status = "okay";
> + };
> +
> + timer3@0 {
> + status = "okay";
> + };
> +};
>
^ permalink raw reply
* Re: [PATCH v3 -next 2/2] ARM: dts: sunxi: add support for Orange Pi Zero board
From: Jernej Skrabec @ 2016-12-03 9:43 UTC (permalink / raw)
To: linux-sunxi
Cc: icenowy-ymACFijhrKM,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
andre.przywara-5wv7dgnIgG8, hdegoede-H+wXaHxf7aLQT0dZR+AlfA,
arnd-r2nGTMty4D4, vishnupatekar0510-Re5JQEeQqe8AvxtiuMwx3w,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAGb2v661hBsd7tBcYByeRKEZh+8YMB3ewbnkrT3rUvzP2f90yQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 9524 bytes --]
Hi,
Dne petek, 02. december 2016 17.42.04 UTC+1 je oseba Chen-Yu Tsai napisala:
>
> Hi,
>
> On Fri, Dec 2, 2016 at 11:05 PM, Icenowy Zheng <ice...-ymACFijhrKM@public.gmane.org
> <javascript:>> wrote:
> > Orange Pi Zero is a board that came with the new Allwinner H2+ SoC and a
> > SDIO Wi-Fi chip by Allwinner (XR819).
> >
> > Add a device tree file for it.
> >
> > Signed-off-by: Icenowy Zheng <ice...-ymACFijhrKM@public.gmane.org <javascript:>>
> > ---
> > Changes since v2:
> > - Merged SDIO Wi-Fi patch into it.
> > - SDIO Wi-Fi: add a ethernet1 alias to it, as it has no internal NVRAM.
> > - SDIO Wi-Fi: changed pinctrl binding to generic pinconf
> > - removed all gpio pinctrl nodes
> > - changed h2plus to h2-plus
> > Changes since v1:
> > - Convert to generic pinconf bindings.
> > - SDIO Wi-Fi: add patch.
> >
> > Some notes:
> > - The uart1 and uart2 is available on the unsoldered gpio header.
> > - The onboard USB connector has its Vbus directly connected to DCIN-5V
> (the
> > power jack)
> >
> > arch/arm/boot/dts/Makefile | 1 +
> > arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts | 159
> ++++++++++++++++++++++
> > 2 files changed, 160 insertions(+)
> > create mode 100644 arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
> >
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index 6447abc..59f6e86 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -844,6 +844,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
> > sun8i-a33-sinlinx-sina33.dtb \
> > sun8i-a83t-allwinner-h8homlet-v2.dtb \
> > sun8i-a83t-cubietruck-plus.dtb \
> > + sun8i-h2-plus-orangepi-zero.dtb \
> > sun8i-h3-bananapi-m2-plus.dtb \
> > sun8i-h3-nanopi-neo.dtb \
> > sun8i-h3-orangepi-2.dtb \
> > diff --git a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
> b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
> > new file mode 100644
> > index 0000000..d18807f
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
> > @@ -0,0 +1,159 @@
> > +/*
> > + * Copyright (C) 2016 Icenowy Zheng <ice...-ymACFijhrKM@public.gmane.org <javascript:>>
> > + *
> > + * Based on sun8i-h3-orangepi-one.dts, which is:
> > + * Copyright (C) 2016 Hans de Goede <hdeg...-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org <javascript:>>
>
> > + *
> > + * This file is dual-licensed: you can use it either under the terms
> > + * of the GPL or the X11 license, at your option. Note that this dual
> > + * licensing only applies to this file, and not this project as a
> > + * whole.
> > + *
> > + * a) This file 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 file 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.
> > + *
> > + * Or, alternatively,
> > + *
> > + * b) Permission is hereby granted, free of charge, to any person
> > + * obtaining a copy of this software and associated documentation
> > + * files (the "Software"), to deal in the Software without
> > + * restriction, including without limitation the rights to use,
> > + * copy, modify, merge, publish, distribute, sublicense, and/or
> > + * sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following
> > + * conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be
> > + * included in all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> > + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> > + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> > + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > + * OTHER DEALINGS IN THE SOFTWARE.
> > + */
> > +
> > +/dts-v1/;
> > +#include "sun8i-h3.dtsi"
> > +#include "sunxi-common-regulators.dtsi"
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/input/input.h>
> > +#include <dt-bindings/pinctrl/sun4i-a10.h>
> > +
> > +/ {
> > + model = "Xunlong Orange Pi Zero";
> > + compatible = "xunlong,orangepi-zero", "allwinner,sun8i-h2-plus";
> > +
> > + aliases {
> > + serial0 = &uart0;
> > + /* ethernet0 is the H3 emac, defined in sun8i-h3.dtsi */
> > + ethernet1 = &xr819;
> > + };
> > +
> > + chosen {
> > + stdout-path = "serial0:115200n8";
> > + };
> > +
> > + leds {
> > + compatible = "gpio-leds";
> > +
> > + pwr_led {
> > + label = "orangepi:green:pwr";
> > + gpios = <&r_pio 0 10 GPIO_ACTIVE_HIGH>;
> > + default-state = "on";
> > + };
> > +
> > + status_led {
> > + label = "orangepi:red:status";
> > + gpios = <&pio 0 17 GPIO_ACTIVE_HIGH>;
> > + };
> > + };
> > +
> > + reg_vcc_wifi: reg_vcc_wifi {
> > + compatible = "regulator-fixed";
> > + regulator-min-microvolt = <3300000>;
> > + regulator-max-microvolt = <3300000>;
> > + regulator-name = "vcc-wifi";
> > + enable-active-high;
> > + gpio = <&pio 0 20 GPIO_ACTIVE_HIGH>;
> > + };
> > +
> > + wifi_pwrseq: wifi_pwrseq {
> > + compatible = "mmc-pwrseq-simple";
> > + reset-gpios = <&r_pio 0 7 GPIO_ACTIVE_LOW>;
> > + };
> > +};
> > +
> > +&ehci1 {
> > + status = "okay";
> > +};
> > +
> > +&mmc0 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&mmc0_pins_a>;
> > + vmmc-supply = <®_vcc3v3>;
> > + bus-width = <4>;
> > + cd-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
> > + cd-inverted;
> > + status = "okay";
> > +};
> > +
> > +&mmc1 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&mmc1_pins_a>;
> > + vmmc-supply = <®_vcc_wifi>;
> > + mmc-pwrseq = <&wifi_pwrseq>;
> > + bus-width = <4>;
> > + non-removable;
> > + status = "okay";
> > +
> > + /*
> > + * Explicitly define the sdio device, so that we can add an
> ethernet
> > + * alias for it (which e.g. makes u-boot set a mac-address).
> > + */
> > + xr819: sdio_wifi@1 {
> > + reg = <1>;
> > + };
> > +};
> > +
> > +&mmc1_pins_a {
> > + bias-pull-up;
>
> This is already set in h3.dtsi
>
> > +};
> > +
> > +&ohci1 {
> > + status = "okay";
> > +};
> > +
> > +&uart0 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&uart0_pins_a>;
> > + status = "okay";
> > +};
> > +
> > +&uart1 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&uart1_pins>;
> > + status = "disabled";
> > +};
> > +
> > +&uart2 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&uart2_pins>;
> > + status = "disabled";
> > +};
> > +
> > +&usbphy {
> > + /* USB VBUS is always on */
>
> I think this comment could use a little work.
>
> AFAIK this board doesn't have an actual USB port.
> It's just the D+/D- pins on the pin header, along
> with the board-wide 5V, also on the pin header.
>
How is addon board handled then? It has two USBs,
IR, audio, mic and probably also TV out. I can't see
any obvious way how to detect that the addon board
is plugged in from the image.
https://www.aliexpress.com/item/New-Orange-Pi-Zreo-Expansion-board-Interface-board-Development-board-beyond-Raspberry-Pi/32770665186.html
Best regards,
Jernej Škrabec
>
> ChenYu
>
> > + status = "okay";
> > +};
> > --
> > 2.10.2
> >
> > --
> > 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...-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org <javascript:>.
> > For more options, visit https://groups.google.com/d/optout.
>
--
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.
[-- Attachment #1.2: Type: text/html, Size: 13038 bytes --]
^ permalink raw reply
* Re: [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer driver in DT
From: Jonathan Cameron @ 2016-12-03 9:42 UTC (permalink / raw)
To: Lee Jones, Benjamin Gaignard
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
alexandre.torgue-qxv4g6HH51o, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
linux-pwm-u79uwXL29TY76Z2rM5mHXA, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
fabrice.gasnier-qxv4g6HH51o, gerald.baeza-qxv4g6HH51o,
arnaud.pouliquen-qxv4g6HH51o,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
linaro-kernel-cunTk1MwBs8s++Sfvej+rw, Benjamin Gaignard
In-Reply-To: <20161202132251.GL2683@dell>
On 02/12/16 13:22, Lee Jones wrote:
> On Fri, 02 Dec 2016, Benjamin Gaignard wrote:
>
>> Add general purpose timers and it sub-nodes into DT for stm32f4.
>> Define and enable pwm1 and pwm3 for stm32f469 discovery board
>>
>> version 3:
>> - use "st,stm32-timer-trigger" in DT
>>
>> version 2:
>> - use parameters to describe hardware capabilities
>> - do not use references for pwm and iio timer subnodes
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
>> ---
>> arch/arm/boot/dts/stm32f429.dtsi | 333 +++++++++++++++++++++++++++++++++-
>> arch/arm/boot/dts/stm32f469-disco.dts | 28 +++
>> 2 files changed, 360 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
>> index bca491d..8c50d03 100644
>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>> @@ -48,7 +48,7 @@
>> #include "skeleton.dtsi"
>> #include "armv7-m.dtsi"
>> #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
>> -
>> +#include <dt-bindings/iio/timer/st,stm32-timer-triggers.h>
>> / {
>> clocks {
>> clk_hse: clk-hse {
>> @@ -355,6 +355,21 @@
>> slew-rate = <2>;
>> };
>> };
>> +
>> + pwm1_pins: pwm@1 {
>> + pins {
>> + pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
>> + <STM32F429_PB13_FUNC_TIM1_CH1N>,
>> + <STM32F429_PB12_FUNC_TIM1_BKIN>;
>> + };
>> + };
>> +
>> + pwm3_pins: pwm@3 {
>> + pins {
>> + pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
>> + <STM32F429_PB5_FUNC_TIM3_CH2>;
>> + };
>> + };
>> };
>>
>> rcc: rcc@40023810 {
>> @@ -426,6 +441,322 @@
>> interrupts = <80>;
>> clocks = <&rcc 0 38>;
>> };
>> +
>> + gptimer1: gptimer1@40010000 {
>
> timer@xxxxxxx
>
> Node names should be generic and not numbered.
>
> I suggest that this isn't actually a timer either. Is contains a
> timer (and a PWM), but in it's completeness it is not a timer per
> say.
That's just mean ;) At least suggest an alternative?
stm32-gptimerish-magic?
>
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40010000 0x400>;
>> + clocks = <&rcc 0 160>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm1@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + st,breakinput;
>> + st,complementary;
>> + status = "disabled";
>> + };
>> +
>> + timer1@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <27>;
>> + st,input-triggers-names = TIM5_TRGO,
>> + TIM2_TRGO,
>> + TIM4_TRGO,
>> + TIM3_TRGO;
>
> I'm still dubious with matching by strings.
>
> I'll take a look at the C code to see what the alternatives could be.
>
>> + st,output-triggers-names = TIM1_TRGO,
>> + TIM1_CH1,
>> + TIM1_CH2,
>> + TIM1_CH3,
>> + TIM1_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer2: gptimer2@40000000 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40000000 0x400>;
>> + clocks = <&rcc 0 128>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm2@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + st,32bits-counter;
>> + status = "disabled";
>> + };
>> +
>> + timer2@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <28>;
>> + st,input-triggers-names = TIM1_TRGO,
>> + TIM8_TRGO,
>> + TIM3_TRGO,
>> + TIM4_TRGO;
>> + st,output-triggers-names = TIM2_TRGO,
>> + TIM2_CH1,
>> + TIM2_CH2,
>> + TIM2_CH3,
>> + TIM2_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer3: gptimer3@40000400 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40000400 0x400>;
>> + clocks = <&rcc 0 129>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm3@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + status = "disabled";
>> + };
>> +
>> + timer3@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <29>;
>> + st,input-triggers-names = TIM1_TRGO,
>> + TIM8_TRGO,
>> + TIM5_TRGO,
>> + TIM4_TRGO;
>> + st,output-triggers-names = TIM3_TRGO,
>> + TIM3_CH1,
>> + TIM3_CH2,
>> + TIM3_CH3,
>> + TIM3_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer4: gptimer4@40000800 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40000800 0x400>;
>> + clocks = <&rcc 0 130>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm4@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + status = "disabled";
>> + };
>> +
>> + timer4@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <30>;
>> + st,input-triggers-names = TIM1_TRGO,
>> + TIM2_TRGO,
>> + TIM3_TRGO,
>> + TIM8_TRGO;
>> + st,output-triggers-names = TIM4_TRGO,
>> + TIM4_CH1,
>> + TIM4_CH2,
>> + TIM4_CH3,
>> + TIM4_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer5: gptimer5@40000C00 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40000C00 0x400>;
>> + clocks = <&rcc 0 131>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm5@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + st,32bits-counter;
>> + status = "disabled";
>> + };
>> +
>> + timer5@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <50>;
>> + st,input-triggers-names = TIM2_TRGO,
>> + TIM3_TRGO,
>> + TIM4_TRGO,
>> + TIM8_TRGO;
>> + st,output-triggers-names = TIM5_TRGO,
>> + TIM5_CH1,
>> + TIM5_CH2,
>> + TIM5_CH3,
>> + TIM5_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer6: gptimer6@40001000 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40001000 0x400>;
>> + clocks = <&rcc 0 132>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + timer6@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <54>;
>> + st,output-triggers-names = TIM6_TRGO;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer7: gptimer7@40001400 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40001400 0x400>;
>> + clocks = <&rcc 0 133>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + timer7@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <55>;
>> + st,output-triggers-names = TIM7_TRGO;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer8: gptimer8@40010400 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40010400 0x400>;
>> + clocks = <&rcc 0 161>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm8@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + st,complementary;
>> + st,breakinput;
>> + status = "disabled";
>> + };
>> +
>> + timer8@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <46>;
>> + st,input-triggers-names = TIM1_TRGO,
>> + TIM2_TRGO,
>> + TIM4_TRGO,
>> + TIM5_TRGO;
>> + st,output-triggers-names = TIM8_TRGO,
>> + TIM8_CH1,
>> + TIM8_CH2,
>> + TIM8_CH3,
>> + TIM8_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer9: gptimer9@40014000 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40014000 0x400>;
>> + clocks = <&rcc 0 176>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm9@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <2>;
>> + status = "disabled";
>> + };
>> +
>> + timer9@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <24>;
>> + st,input-triggers-names = TIM2_TRGO,
>> + TIM3_TRGO;
>> + st,output-triggers-names = TIM9_TRGO,
>> + TIM9_CH1,
>> + TIM9_CH2;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer10: gptimer10@40014400 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40014400 0x400>;
>> + clocks = <&rcc 0 177>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm10@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <1>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer11: gptimer11@40014800 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40014800 0x400>;
>> + clocks = <&rcc 0 178>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm11@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <1>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer12: gptimer12@40001800 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40001800 0x400>;
>> + clocks = <&rcc 0 134>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm12@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <2>;
>> + status = "disabled";
>> + };
>> +
>> + timer12@0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <43>;
>> + st,input-triggers-names = TIM4_TRGO,
>> + TIM5_TRGO;
>> + st,output-triggers-names = TIM12_TRGO,
>> + TIM12_CH1,
>> + TIM12_CH2;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer13: gptimer13@40001C00 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40001C00 0x400>;
>> + clocks = <&rcc 0 135>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm13@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <1>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer14: gptimer14@40002000 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40002000 0x400>;
>> + clocks = <&rcc 0 136>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm14@0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <1>;
>> + status = "disabled";
>> + };
>> + };
>> };
>> };
>>
>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
>> index 8a163d7..df4ca7e 100644
>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>> @@ -81,3 +81,31 @@
>> &usart3 {
>> status = "okay";
>> };
>> +
>> +&gptimer1 {
>> + status = "okay";
>> +
>> + pwm1@0 {
>> + pinctrl-0 = <&pwm1_pins>;
>> + pinctrl-names = "default";
>> + status = "okay";
>> + };
>> +
>> + timer1@0 {
>> + status = "okay";
>> + };
>> +};
>
> This is a much *better* format than before.
>
> I still don't like the '&' syntax though.
>
>> +&gptimer3 {
>> + status = "okay";
>> +
>> + pwm3@0 {
>> + pinctrl-0 = <&pwm3_pins>;
>> + pinctrl-names = "default";
>> + status = "okay";
>> + };
>> +
>> + timer3@0 {
>> + status = "okay";
>> + };
>> +};
>
^ permalink raw reply
* Re: [PATCH v3 5/7] IIO: add bindings for stm32 timer trigger driver
From: Jonathan Cameron @ 2016-12-03 9:35 UTC (permalink / raw)
To: Benjamin Gaignard, Lee Jones
Cc: Mark Rutland, devicetree, Lars-Peter Clausen, alexandre.torgue,
linux-pwm, linux-iio, Linus Walleij, Arnaud Pouliquen,
Linux Kernel Mailing List, robh+dt, Thierry Reding,
Peter Meerwald-Stadler, knaack.h, Gerald Baeza, Fabrice Gasnier,
Linaro Kernel Mailman List, linux-arm-kernel, Benjamin Gaignard
In-Reply-To: <CA+M3ks7GhvApJwZVFK7KAbbvbAhp40sdOVvhkyZWBHZXpLfZDQ@mail.gmail.com>
On 02/12/16 14:23, Benjamin Gaignard wrote:
> 2016-12-02 14:59 GMT+01:00 Lee Jones <lee.jones@linaro.org>:
>> On Fri, 02 Dec 2016, Benjamin Gaignard wrote:
>>
>>> Define bindings for stm32 timer trigger
>>>
>>> version 3:
>>> - change file name
>>> - add cross reference with mfd bindings
>>>
>>> version 2:
>>> - only keep one compatible
>>> - add DT parameters to set lists of the triggers:
>>> one list describe the triggers created by the device
>>> another one give the triggers accepted by the device
>>>
>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>> ---
>>> .../bindings/iio/timer/stm32-timer-trigger.txt | 39 ++++++++++++++++++++++
>>> 1 file changed, 39 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/iio/timer/stm32-timer-trigger.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/timer/stm32-timer-trigger.txt b/Documentation/devicetree/bindings/iio/timer/stm32-timer-trigger.txt
>>> new file mode 100644
>>> index 0000000..858816d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/timer/stm32-timer-trigger.txt
>>> @@ -0,0 +1,39 @@
>>> +timer trigger bindings for STM32
>>> +
>>> +Must be a sub-node of STM32 general purpose timer driver
>>> +Parent node properties are describe in ../mfd/stm32-general-purpose-timer.txt
>>> +
>>> +Required parameters:
>>> +- compatible: must be "st,stm32-iio-timer"
>>> +- interrupts: Interrupt for this device
>>> + See ../interrupt-controller/st,stm32-exti.txt
>>> +
>>> +Optional parameters:
>>> +- st,input-triggers-names: List of the possible input triggers for
>>> + the device
>>> +- st,output-triggers-names: List of the possible output triggers for
>>> + the device
>>> +
>>> +Possible triggers are defined in include/dt-bindings/iio/timer/st,stm32-timer-trigger.h
>>> +
>>> +Example:
>>> + gptimer1: gptimer1@40010000 {
>>> + compatible = "st,stm32-gptimer";
>>> + reg = <0x40010000 0x400>;
>>> + clocks = <&rcc 0 160>;
>>> + clock-names = "clk_int";
>>> +
>>> + timer1@0 {
>>> + compatible = "st,stm32-timer-trigger";
>>> + interrupts = <27>;
>>> + st,input-triggers-names = TIM5_TRGO,
>>> + TIM2_TRGO,
>>> + TIM4_TRGO,
>>> + TIM3_TRGO;
>>> + st,output-triggers-names = TIM1_TRGO,
>>> + TIM1_CH1,
>>> + TIM1_CH2,
>>> + TIM1_CH3,
>>> + TIM1_CH4;
>>
>> I see why you've done it like this now ... because it makes things
>> easier for you in the driver, since the IIO subsystem matches on names
>> such as these.
>>
>> BUT, this is a Linux-implementation-ism. Just use pairs of integers
>> and create the Linux-ism strings in the driver.
>
> The goal is not to make things easier in driver but to be able to share
> the triggers names with other drivers like DAC or ADC.
> If each driver have to create it own triggers names it will more difficult
> to keep them coherent than it they share the same definitions
Do it by documentation. This will be effectively ABI going forward
so fixed once it is defined. Should be fairly easy to tell during testing
if someone has messed it up ;)
Jonathan
>
>>
>>> + };
>>> + };
>>
>> --
>> Lee Jones
>> Linaro STMicroelectronics Landing Team Lead
>> Linaro.org │ Open source software for ARM SoCs
>> Follow Linaro: Facebook | Twitter | Blog
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 0/7] Add pwm and IIO timer drivers for stm32
From: Jonathan Cameron @ 2016-12-03 9:32 UTC (permalink / raw)
To: Benjamin Gaignard, lee.jones-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
alexandre.torgue-qxv4g6HH51o, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
linux-pwm-u79uwXL29TY76Z2rM5mHXA, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: fabrice.gasnier-qxv4g6HH51o, gerald.baeza-qxv4g6HH51o,
arnaud.pouliquen-qxv4g6HH51o,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
linaro-kernel-cunTk1MwBs8s++Sfvej+rw, Benjamin Gaignard
In-Reply-To: <1480673842-20804-1-git-send-email-benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
On 02/12/16 10:17, Benjamin Gaignard wrote:
> version 3:
> - no change on mfd and pwm divers patches
> - add cross reference between bindings
> - change compatible to "st,stm32-timer-trigger"
> - fix attributes access rights
> - use string instead of int for master_mode and slave_mode
> - document device attributes in sysfs-bus-iio-timer-stm32
> - udpate DT with the new compatible
>
> version 2:
> - keep only one compatible per driver
> - use DT parameters to describe hardware block configuration:
> - pwm channels, complementary output, counter size, break input
> - triggers accepted and create by IIO timers
> - change DT to limite use of reference to the node
> - interrupt is now in IIO timer driver
> - rename stm32-mfd-timer to stm32-gptimer (for general purpose timer)
>
> The following patches enable pwm and IIO Timer features for stm32 platforms.
>
> Those two features are mixed into the registers of the same hardware block
> (named general purpose timer) which lead to introduce a multifunctions driver
> on the top of them to be able to share the registers.
>
> In stm32 14 instances of timer hardware block exist, even if they all have
> the same register mapping they could have a different number of pwm channels
> and/or different triggers capabilities. We use various parameters in DT to
> describe the differences between hardware blocks
>
> The MFD (stm32-gptimer.c) takes care of clock and register mapping
> by using regmap. stm32_gptimer_dev structure is provided to its sub-node to
> share those information.
>
> PWM driver is implemented into pwm-stm32.c. Depending of the instance we may
> have up to 4 channels, sometime with complementary outputs or 32 bits counter
> instead of 16 bits. Some hardware blocks may also have a break input function
> which allows to stop pwm depending of a level, defined in devicetree, on an
> external pin.
>
> IIO timer driver (stm32-iio-timer.c and stm32-iio-timers.h) define a list of
> hardware triggers usable by hardware blocks like ADC, DAC or other timers.
>
> The matrix of possible connections between blocks is quite complex so we use
> trigger names and is_stm32_iio_timer_trigger() function to be sure that
> triggers are valid and configure the IPs.
> Possible triggers ar listed in include/dt-bindings/iio/timer/st,stm32-iio-timer.h
>
> At run time IIO timer hardware blocks can configure (through "master_mode"
> IIO device attribute) which internal signal (counter enable, reset,
> comparison block, etc...) is used to generate the trigger.
>
> By using "slave_mode" IIO device attribute timer can also configure on which
> event (level, rising edge) of the block is enabled.
>
> Since we can use trigger from one hardware to control an other block, we can
> use a pwm to control an other one. The following example shows how to configure
> pwm1 and pwm3 to make pwm3 generate pulse only when pwm1 pulse level is high.
>
> /sys/bus/iio/devices # ls
> iio:device0 iio:device1 trigger0 trigger1
>
> configure timer1 to use pwm1 channel 0 as output trigger
> /sys/bus/iio/devices # echo 'OC1REF' > iio\:device0/master_mode
> configure timer3 to enable only when input is high
> /sys/bus/iio/devices # echo 'gated' > iio\:device1/slave_mode
> /sys/bus/iio/devices # cat trigger0/name
> tim1_trgo
> configure timer2 to use timer1 trigger is input
> /sys/bus/iio/devices # echo "tim1_trgo" > iio\:device1/trigger/current_trigger
>
> configure pwm3 channel 0 to generate a signal with a period of 100ms and a
> duty cycle of 50%
> /sys/devices/platform/soc/40000400.gptimer3/40000400.gptimer3:pwm3@0/pwm/pwmchip4 # echo 0 > export
> /sys/devices/platform/soc/40000400.gptimer3/40000400.gptimer3:pwm3@0/pwm/pwmchip4 # echo 100000000 > pwm0/period
> /sys/devices/platform/soc/40000400.gptimer3/40000400.gptimer3:pwm3@0/pwm/pwmchip4 # echo 50000000 > pwm0/duty_cycle
> /sys/devices/platform/soc/40000400.gptimer3/40000400.gptimer3:pwm3@0/pwm/pwmchip4# echo 1 > pwm0/enable
> here pwm3 channel 0, as expected, doesn't start because has to be triggered by
> pwm1 channel 0
>
> configure pwm1 channel 0 to generate a signal with a period of 1s and a
> duty cycle of 50%
> /sys/devices/platform/soc/40010000.gptimer1/40010000.gptimer1:pwm1@0/pwm/pwmchip0 # echo 0 > export
> /sys/devices/platform/soc/40010000.gptimer1/40010000.gptimer1:pwm1@0/pwm/pwmchip0 # echo 1000000000 > pwm0/period
> /sys/devices/platform/soc/40010000.gptimer1/40010000.gptimer1:pwm1@0/pwm/pwmchip0 # echo 500000000 > pwm0/duty_cycle
> /sys/devices/platform/soc/40010000.gptimer1/40010000.gptimer1:pwm1@0/pwm/pwmchip0 # echo 1 > pwm0/enable
> finally pwm1 starts and pwm3 only generates pulse when pwm1 signal is high
>
> An other example to use a timer as source of clock for another device.
> Here timer1 is used a source clock for pwm3:
>
> /sys/bus/iio/devices # echo 100000 > trigger0/sampling_frequency
> /sys/bus/iio/devices # echo tim1_trgo > iio\:device1/trigger/current_trigger
> /sys/bus/iio/devices # echo 'external_clock' > iio\:device1/slave_mode
> /sys/devices/platform/soc/40000400.gptimer3/40000400.gptimer3:pwm3@0/pwm/pwmchip4 # echo 0 > export
> /sys/devices/platform/soc/40000400.gptimer3/40000400.gptimer3:pwm3@0/pwm/pwmchip4 # echo 1000000 > pwm0/period
> /sys/devices/platform/soc/40000400.gptimer3/40000400.gptimer3:pwm3@0/pwm/pwmchip4 # echo 500000 > pwm0/duty_cycle
> /sys/devices/platform/soc/40000400.gptimer3/40000400.gptimer3:pwm3@0/pwm/pwmchip4 # echo 1 > pwm0/enable
This is good thorough documentation. Could we have an additional
patch adding just this documentation to the tree (probably under
Documentation/mfd?). Documentation in general is a bit in flux at the
moment so we'll may want to sphixify it afterwards but plain text
is fine for now.
Jonathan
>
> Benjamin Gaignard (7):
> MFD: add bindings for stm32 general purpose timer driver
> MFD: add stm32 general purpose timer driver
> PWM: add pwm-stm32 DT bindings
> PWM: add pwm driver for stm32 plaftorm
> IIO: add bindings for stm32 timer trigger driver
> IIO: add STM32 timer trigger driver
> ARM: dts: stm32: add stm32 general purpose timer driver in DT
>
> .../ABI/testing/sysfs-bus-iio-timer-stm32 | 47 ++
> .../bindings/iio/timer/stm32-timer-trigger.txt | 39 ++
> .../bindings/mfd/stm32-general-purpose-timer.txt | 47 ++
> .../devicetree/bindings/pwm/pwm-stm32.txt | 38 ++
> arch/arm/boot/dts/stm32f429.dtsi | 333 +++++++++++++-
> arch/arm/boot/dts/stm32f469-disco.dts | 28 ++
> drivers/iio/Kconfig | 2 +-
> drivers/iio/Makefile | 1 +
> drivers/iio/timer/Kconfig | 15 +
> drivers/iio/timer/Makefile | 1 +
> drivers/iio/timer/stm32-timer-trigger.c | 477 +++++++++++++++++++++
> drivers/iio/trigger/Kconfig | 1 -
> drivers/mfd/Kconfig | 10 +
> drivers/mfd/Makefile | 2 +
> drivers/mfd/stm32-gptimer.c | 73 ++++
> drivers/pwm/Kconfig | 8 +
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-stm32.c | 285 ++++++++++++
> .../iio/timer/st,stm32-timer-triggers.h | 60 +++
> include/linux/iio/timer/stm32-timer-trigger.h | 16 +
> include/linux/mfd/stm32-gptimer.h | 62 +++
> 21 files changed, 1543 insertions(+), 3 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
> create mode 100644 Documentation/devicetree/bindings/iio/timer/stm32-timer-trigger.txt
> create mode 100644 Documentation/devicetree/bindings/mfd/stm32-general-purpose-timer.txt
> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> create mode 100644 drivers/iio/timer/Kconfig
> create mode 100644 drivers/iio/timer/Makefile
> create mode 100644 drivers/iio/timer/stm32-timer-trigger.c
> create mode 100644 drivers/mfd/stm32-gptimer.c
> create mode 100644 drivers/pwm/pwm-stm32.c
> create mode 100644 include/dt-bindings/iio/timer/st,stm32-timer-triggers.h
> create mode 100644 include/linux/iio/timer/stm32-timer-trigger.h
> create mode 100644 include/linux/mfd/stm32-gptimer.h
>
--
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 v2 6/7] IIO: add STM32 IIO timer driver
From: Jonathan Cameron @ 2016-12-03 9:28 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: Lee Jones, robh+dt, Mark Rutland, alexandre.torgue, devicetree,
Linux Kernel Mailing List, Thierry Reding, linux-pwm, knaack.h,
Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio,
linux-arm-kernel, Fabrice Gasnier, Gerald Baeza, Arnaud Pouliquen,
Linus Walleij, Linaro Kernel Mailman List, Benjamin Gaignard
In-Reply-To: <CA+M3ks7ks=YsToEyLcPhPf+JfdJtekUYcihpcYLLt0h6APbcfA@mail.gmail.com>
On 29/11/16 09:46, Benjamin Gaignard wrote:
> 2016-11-27 16:42 GMT+01:00 Jonathan Cameron <jic23@kernel.org>:
>> I delved into the datasheet after trying to figure this out, so I think
>> I now sort of understand your intent, but please do answer the questions
>> inline.
>>
>> On 24/11/16 15:14, Benjamin Gaignard wrote:
>>> Timers IPs can be used to generate triggers for other IPs like
>>> DAC, ADC or other timers.
>>> Each trigger may result of timer internals signals like counter enable,
>>> reset or edge, this configuration could be done through "master_mode"
>>> device attribute.
>>>
>>> A timer device could be triggered by other timers, we use the trigger
>>> name and is_stm32_iio_timer_trigger() function to distinguish them
>>> and configure IP input switch.
>> The presence of an IIO device in here was a suprise.. What is it actually for?
>
> IIO device is needed to be able to valid the input triggers, which
> aren't the same than
> those generated by the device.
> Since I use triggers name to distinguish them I have introduced
> is_stm32_iio_timer_trigger()
> function to be sure that triggers are coming for a valid hardware and
> not from a fake one
> using the same name.
>
>>
>> I think this needs some examples of usage to make it clear what the aim is.
>
> In the hardware block there is switch in input to select which trigger
> will drive the IP.
> For example that allow to start multiple pwm exactly that the same
> time or to start/stop
> it on master edges.
Hmm. OK. We need to think about how to represent this concept of a tree
of triggers - independent of having an IIO device as that is down right
misleading.
In the first instance the tree is full supported by this one driver I think?
If so lets use it as a testbed and try and put together a simple tree between
the triggers.
So the child triggers (started on the parent firing) can perhaps have a
'parent' attribute? (might be better naming possible!)
>
>>
>> I was basically expecting to see a driver instantiating one iio trigger
>> per timer that can act as a trigger. Those would each have sampling frequency
>> controls and basica enable / disable.
>
> An hardware device could have up to 5 triggers: timX_trgo, timX_ch1, timX_ch2,
> timX_ch3, timX_ch4.
On a train so I don't have the datasheet. Which of these would actually make
sense when driving an adc scan?
> Until now I have try to simplify the problem and just use timX_trgo trigger.
> I have added a "sampling_frequency" attribute on the trigger to
> control the frequence
> and I use trigger set_state function to enable disable it.
Wise move! Makes sense to build this up in baby steps if possible.
>
>>
>> I'm seeing something much more complex here so additional explanation is
>> needed.
>>>
>>> Timer may also decide on which event (edge, level) they could
>>> be activated by a trigger, this configuration is done by writing in
>>> "slave_mode" device attribute.
>> Really? Sounds like magic numbers in sysfs which is never a good idea.
>> Please document those attributes / or break them up into elements that
>> don't require magic numbers.
>
> I would like to use strings here, it is possible to use IIO_CONST_ATTR
> to describe them ?
If it's on the iio device use the iio_ext_info stuff that has support
for enums. If you need this for the trigger feel free to add equivalent
support to the core as needed.
Note that we are still evolving IIO so if we need new stuff to support
your usecase, never be afraid of proposing it! The only element
I am keen on is keeping anything that is opaque to drivers opaque
unless there is a VERY good reason to do otherwise. Mostly this
just means using access functions etc. That makes messing around
with the core internals (as still happens from time to time) a lot
less painful!
>
>>>
>>> Since triggers could also be used by DAC or ADC their names are defined
>>> in include/dt-bindings/iio/timer/st,stm32-iio-timer.h so those IPs will be able
>>> to configure themselves in valid_trigger function
>>>
>>> Trigger have a "sampling_frequency" attribute which allow to configure
>>> timer sampling frequency without using pwm interface
>>>
>>> version 2:
>>> - keep only one compatible
>> Hmm. I'm not sure I like this as such. We are actually dealing with lots
>> of instances of a hardware block with only a small amount of shared
>> infrastrcuture (which is classic mfd teritory). So to my mind we
>> should have a separate device for each.
>
> Registers mapping and offset are the same, from triggers point of view
> only the configuration of the input switch is different.
>
>>
>>> - use st,input-triggers-names and st,output-triggers-names
>>> to know which triggers are accepted and/or create by the device
>> I'm not following why we have this cascade setup?
>>
>> These are triggers, not devices in the IIO context. We need some detailed
>> description of why you have it setup like this. This would include the
>> ABI with examples of how you are using it.
>
> I had put example of usage on the cover letter, I will duplicate them
> in this commit
> message.
Ooops. I didn't ready that ;) Sorry.
>
>>
>> Basically I don't currently understand what you are doing :(
>>
>>
>> Thanks,
>>
>> Jonathan
>>>
>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>> ---
>>> drivers/iio/Kconfig | 2 +-
>>> drivers/iio/Makefile | 1 +
>>> drivers/iio/timer/Kconfig | 15 +
>>> drivers/iio/timer/Makefile | 1 +
>>> drivers/iio/timer/stm32-iio-timer.c | 448 +++++++++++++++++++++
>>> drivers/iio/trigger/Kconfig | 1 -
>>> include/dt-bindings/iio/timer/st,stm32-iio-timer.h | 23 ++
>>> include/linux/iio/timer/stm32-iio-timers.h | 16 +
>>> 8 files changed, 505 insertions(+), 2 deletions(-)
>>> create mode 100644 drivers/iio/timer/Kconfig
>>> create mode 100644 drivers/iio/timer/Makefile
>>> create mode 100644 drivers/iio/timer/stm32-iio-timer.c
>>> create mode 100644 include/dt-bindings/iio/timer/st,stm32-iio-timer.h
>>> create mode 100644 include/linux/iio/timer/stm32-iio-timers.h
>>>
>>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
>>> index 6743b18..2de2a80 100644
>>> --- a/drivers/iio/Kconfig
>>> +++ b/drivers/iio/Kconfig
>>> @@ -90,5 +90,5 @@ source "drivers/iio/potentiometer/Kconfig"
>>> source "drivers/iio/pressure/Kconfig"
>>> source "drivers/iio/proximity/Kconfig"
>>> source "drivers/iio/temperature/Kconfig"
>>> -
>>> +source "drivers/iio/timer/Kconfig"
>>> endif # IIO
>>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
>>> index 87e4c43..b797c08 100644
>>> --- a/drivers/iio/Makefile
>>> +++ b/drivers/iio/Makefile
>>> @@ -32,4 +32,5 @@ obj-y += potentiometer/
>>> obj-y += pressure/
>>> obj-y += proximity/
>>> obj-y += temperature/
>>> +obj-y += timer/
>>> obj-y += trigger/
>>> diff --git a/drivers/iio/timer/Kconfig b/drivers/iio/timer/Kconfig
>>> new file mode 100644
>>> index 0000000..7a73bc6
>>> --- /dev/null
>>> +++ b/drivers/iio/timer/Kconfig
>>> @@ -0,0 +1,15 @@
>>> +#
>>> +# Timers drivers
>>> +
>>> +menu "Timers"
>>> +
>>> +config IIO_STM32_TIMER
>>> + tristate "stm32 iio timer"
>>> + depends on ARCH_STM32
>>> + depends on OF
>>> + select IIO_TRIGGERED_EVENT
>>> + select MFD_STM32_GP_TIMER
>>> + help
>>> + Select this option to enable stm32 timers hardware IPs
>>> +
>>> +endmenu
>>> diff --git a/drivers/iio/timer/Makefile b/drivers/iio/timer/Makefile
>>> new file mode 100644
>>> index 0000000..a360c9f
>>> --- /dev/null
>>> +++ b/drivers/iio/timer/Makefile
>>> @@ -0,0 +1 @@
>>> +obj-$(CONFIG_IIO_STM32_TIMER) += stm32-iio-timer.o
>>> diff --git a/drivers/iio/timer/stm32-iio-timer.c b/drivers/iio/timer/stm32-iio-timer.c
>>> new file mode 100644
>>> index 0000000..35f2687
>>> --- /dev/null
>>> +++ b/drivers/iio/timer/stm32-iio-timer.c
>>> @@ -0,0 +1,448 @@
>>> +/*
>>> + * stm32-iio-timer.c
>>> + *
>>> + * Copyright (C) STMicroelectronics 2016
>>> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
>>> + * License terms: GNU General Public License (GPL), version 2
>>> + */
>>> +
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/sysfs.h>
>>> +#include <linux/iio/timer/stm32-iio-timers.h>
>>> +#include <linux/iio/trigger.h>
>>> +#include <linux/iio/triggered_event.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/mfd/stm32-gptimer.h>
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#define DRIVER_NAME "stm32-iio-timer"
>>> +
>>> +struct stm32_iio_timer_dev {
>>> + struct device *dev;
>>> + struct regmap *regmap;
>>> + struct clk *clk;
>>> + int irq;
>>> + bool own_timer;
>>> + unsigned int sampling_frequency;
>>> + struct iio_trigger *active_trigger;
>>> +};
>>> +
>>> +static ssize_t _store_frequency(struct device *dev,
>>> + struct device_attribute *attr,
>>> + const char *buf, size_t len)
>>> +{
>>> + struct iio_trigger *trig = to_iio_trigger(dev);
>>> + struct stm32_iio_timer_dev *stm32 = iio_trigger_get_drvdata(trig);
>>> + unsigned int freq;
>>> + int ret;
>>> +
>>> + ret = kstrtouint(buf, 10, &freq);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + stm32->sampling_frequency = freq;
>>> +
>>> + return len;
>>> +}
>>> +
>>> +static ssize_t _read_frequency(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + struct iio_trigger *trig = to_iio_trigger(dev);
>>> + struct stm32_iio_timer_dev *stm32 = iio_trigger_get_drvdata(trig);
>>> + unsigned long long freq = stm32->sampling_frequency;
>>> + u32 psc, arr, cr1;
>>> +
>>> + regmap_read(stm32->regmap, TIM_CR1, &cr1);
>>> + regmap_read(stm32->regmap, TIM_PSC, &psc);
>>> + regmap_read(stm32->regmap, TIM_ARR, &arr);
>>> +
>>> + if (psc && arr && (cr1 & TIM_CR1_CEN)) {
>>> + freq = (unsigned long long)clk_get_rate(stm32->clk);
>>> + do_div(freq, psc);
>>> + do_div(freq, arr);
>>> + }
>>> +
>>> + return sprintf(buf, "%d\n", (unsigned int)freq);
>>> +}
>>> +
>>> +static IIO_DEV_ATTR_SAMP_FREQ(S_IWUSR | S_IRUGO,
>>> + _read_frequency,
>>> + _store_frequency);
>>> +
>>> +static struct attribute *stm32_trigger_attrs[] = {
>>> + &iio_dev_attr_sampling_frequency.dev_attr.attr,
>>> + NULL,
>>> +};
>>> +
>>> +static const struct attribute_group stm32_trigger_attr_group = {
>>> + .attrs = stm32_trigger_attrs,
>>> +};
>>> +
>>> +static const struct attribute_group *stm32_trigger_attr_groups[] = {
>>> + &stm32_trigger_attr_group,
>>> + NULL,
>>> +};
>>> +
>>> +static
>>> +ssize_t _show_master_mode(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>> + struct stm32_iio_timer_dev *stm32 = iio_priv(indio_dev);
>>> + u32 cr2;
>>> +
>>> + regmap_read(stm32->regmap, TIM_CR2, &cr2);
>>> +
>>> + return snprintf(buf, PAGE_SIZE, "%d\n", (cr2 >> 4) & 0x7);
>>> +}
>>> +
>>> +static
>>> +ssize_t _store_master_mode(struct device *dev,
>>> + struct device_attribute *attr,
>>> + const char *buf, size_t len)
>>> +{
>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>> + struct stm32_iio_timer_dev *stm32 = iio_priv(indio_dev);
>>> + u8 mode;
>>> + int ret;
>>> +
>>> + ret = kstrtou8(buf, 10, &mode);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (mode > 0x7)
>>> + return -EINVAL;
>>> +
>>> + regmap_update_bits(stm32->regmap, TIM_CR2, TIM_CR2_MMS, mode << 4);
>>> +
>>> + return len;
>>> +}
>>> +
>>> +static IIO_DEVICE_ATTR(master_mode, S_IRUGO | S_IWUSR,
>>> + _show_master_mode,
>>> + _store_master_mode,
>>> + 0);
>>> +
>>> +static
>>> +ssize_t _show_slave_mode(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>> + struct stm32_iio_timer_dev *stm32 = iio_priv(indio_dev);
>>> + u32 smcr;
>>> +
>>> + regmap_read(stm32->regmap, TIM_SMCR, &smcr);
>>> +
>>> + return snprintf(buf, PAGE_SIZE, "%d\n", smcr & 0x3);
>>> +}
>>> +
>>> +static
>>> +ssize_t _store_slave_mode(struct device *dev,
>>> + struct device_attribute *attr,
>>> + const char *buf, size_t len)
>>> +{
>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>> + struct stm32_iio_timer_dev *stm32 = iio_priv(indio_dev);
>>> + u8 mode;
>>> + int ret;
>>> +
>>> + ret = kstrtou8(buf, 10, &mode);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (mode > 0x7)
>>> + return -EINVAL;
>> How is something called slave mode going to be fed a number between 0 and 7?
>> Rule of thumb is no magic numbers in sysfs and right now this is looking
>> rather cryptic to say the least!
>
> I would like to use strings here, it is possible to use IIO_CONST_ATTR
> to describe them ?
> In documentation slave modes are describe that this:
> 000: Slave mode disabled - if CEN = ‘1’ then the prescaler is clocked
> directly by the internal clock.
> 001: Encoder mode 1 - Counter counts up/down on TI2FP1 edge depending
> on TI1FP2 level.
> 010: Encoder mode 2 - Counter counts up/down on TI1FP2 edge depending
> on TI2FP1 level.
> 011: Encoder mode 3 - Counter counts up/down on both TI1FP1 and TI2FP2
> edges depending on the level of the other input.
> 100: Reset Mode - Rising edge of the selected trigger input (TRGI)
> reinitializes the counter and generates an update of the registers.
> 101: Gated Mode - The counter clock is enabled when the trigger input
> (TRGI) is high.
> The counter stops (but is not reset) as soon as the trigger becomes low.
> Both start and stop of the counter are controlled.
> 110: Trigger Mode - The counter starts at a rising edge of the trigger
> TRGI (but it is notreset).
> Only the start of the counter is controlled.
> 111: External Clock Mode 1 - Rising edges of the selected trigger
> (TRGI) clock the counter.
>
>>> +
>>> + regmap_update_bits(stm32->regmap, TIM_SMCR, TIM_SMCR_SMS, mode);
>>> +
>>> + return len;
>>> +}
>>> +
>>> +static IIO_DEVICE_ATTR(slave_mode, S_IRUGO | S_IWUSR,
>> There is an iritating move (in terms of noise it's generating) to use values
>> directly instead fo these defines. Still if you don't fix it here I'll only
>> get a patch 'fixing' it soon after...
>
> I will fix at in version 3
>
>>
>>> + _show_slave_mode,
>>> + _store_slave_mode,
>>> + 0);
>>> +
>>> +static struct attribute *stm32_timer_attrs[] = {
>>> + &iio_dev_attr_master_mode.dev_attr.attr,
>>> + &iio_dev_attr_slave_mode.dev_attr.attr,
>> New ABI so must be documented under Documentation/ABI/testing/sysfs-bus-iio-*
>
> OK
>
>>> + NULL,
>>> +};
>>> +
>>> +static const struct attribute_group stm32_timer_attr_group = {
>>> + .attrs = stm32_timer_attrs,
>>> +};
>>> +
>>> +static int stm32_timer_start(struct stm32_iio_timer_dev *stm32)
>>> +{
>>> + unsigned long long prd, div;
>>> + int prescaler = 0;
>>> + u32 max_arr = 0xFFFF, cr1;
>>> +
>>> + if (stm32->sampling_frequency == 0)
>>> + return 0;
>>> +
>>> + /* Period and prescaler values depends of clock rate */
>>> + div = (unsigned long long)clk_get_rate(stm32->clk);
>>> +
>>> + do_div(div, stm32->sampling_frequency);
>>> +
>>> + prd = div;
>>> +
>>> + while (div > max_arr) {
>>> + prescaler++;
>>> + div = prd;
>>> + do_div(div, (prescaler + 1));
>>> + }
>>> + prd = div;
>>> +
>>> + if (prescaler > MAX_TIM_PSC) {
>>> + dev_err(stm32->dev, "prescaler exceeds the maximum value\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + /* Check that we own the timer */
>>> + regmap_read(stm32->regmap, TIM_CR1, &cr1);
>>> + if ((cr1 & TIM_CR1_CEN) && !stm32->own_timer)
>>> + return -EBUSY;
>>> +
>>> + if (!stm32->own_timer) {
>>> + stm32->own_timer = true;
>>> + clk_enable(stm32->clk);
>>> + }
>>> +
>>> + regmap_write(stm32->regmap, TIM_PSC, prescaler);
>>> + regmap_write(stm32->regmap, TIM_ARR, prd - 1);
>>> + regmap_update_bits(stm32->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
>>> +
>>> + /* Force master mode to update mode */
>>> + regmap_update_bits(stm32->regmap, TIM_CR2, TIM_CR2_MMS, 0x20);
>>> +
>>> + /* Make sure that registers are updated */
>>> + regmap_update_bits(stm32->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
>>> +
>>> + /* Enable interrupt */
>>> + regmap_write(stm32->regmap, TIM_SR, 0);
>>> + regmap_update_bits(stm32->regmap, TIM_DIER, TIM_DIER_UIE, TIM_DIER_UIE);
>>> +
>>> + /* Enable controller */
>>> + regmap_update_bits(stm32->regmap, TIM_CR1, TIM_CR1_CEN, TIM_CR1_CEN);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int stm32_timer_stop(struct stm32_iio_timer_dev *stm32)
>>> +{
>>> + if (!stm32->own_timer)
>>> + return 0;
>>> +
>>> + /* Stop timer */
>>> + regmap_update_bits(stm32->regmap, TIM_DIER, TIM_DIER_UIE, 0);
>>> + regmap_update_bits(stm32->regmap, TIM_CR1, TIM_CR1_CEN, 0);
>>> + regmap_write(stm32->regmap, TIM_PSC, 0);
>>> + regmap_write(stm32->regmap, TIM_ARR, 0);
>>> +
>>> + clk_disable(stm32->clk);
>>> +
>>> + stm32->own_timer = false;
>>> + stm32->active_trigger = NULL;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int stm32_set_trigger_state(struct iio_trigger *trig, bool state)
>>> +{
>>> + struct stm32_iio_timer_dev *stm32 = iio_trigger_get_drvdata(trig);
>>> +
>>> + stm32->active_trigger = trig;
>>> +
>>> + if (state)
>>> + return stm32_timer_start(stm32);
>>> + else
>>> + return stm32_timer_stop(stm32);
>>> +}
>>> +
>>> +static irqreturn_t stm32_timer_irq_handler(int irq, void *private)
>>> +{
>>> + struct stm32_iio_timer_dev *stm32 = private;
>>> + u32 sr;
>>> +
>>> + regmap_read(stm32->regmap, TIM_SR, &sr);
>>> + regmap_write(stm32->regmap, TIM_SR, 0);
>>> +
>>> + if ((sr & TIM_SR_UIF) && stm32->active_trigger)
>>> + iio_trigger_poll(stm32->active_trigger);
>> This is acting like a trigger cascading off another trigger?
>
> Not only a trigger but ADC or DAC too.
>
>>
>> Normally this interrupt handler would be directly associated with the
>> trigger hardware - in this case the timer.
>
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static const struct iio_trigger_ops timer_trigger_ops = {
>>> + .owner = THIS_MODULE,
>>> + .set_trigger_state = stm32_set_trigger_state,
>>> +};
>>> +
>>> +static int stm32_setup_iio_triggers(struct stm32_iio_timer_dev *stm32)
>>> +{
>>> + int ret;
>>> + struct property *p;
>>> + const char *cur = NULL;
>>> +
>>> + p = of_find_property(stm32->dev->of_node,
>>> + "st,output-triggers-names", NULL);
>>> +
>>> + while ((cur = of_prop_next_string(p, cur)) != NULL) {
>>> + struct iio_trigger *trig;
>>> +
>>> + trig = devm_iio_trigger_alloc(stm32->dev, "%s", cur);
>>> + if (!trig)
>>> + return -ENOMEM;
>>> +
>>> + trig->dev.parent = stm32->dev->parent;
>>> + trig->ops = &timer_trigger_ops;
>>> + trig->dev.groups = stm32_trigger_attr_groups;
>>> + iio_trigger_set_drvdata(trig, stm32);
>>> +
>>> + ret = devm_iio_trigger_register(stm32->dev, trig);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * is_stm32_iio_timer_trigger
>>> + * @trig: trigger to be checked
>>> + *
>>> + * return true if the trigger is a valid stm32 iio timer trigger
>>> + * either return false
>>> + */
>>> +bool is_stm32_iio_timer_trigger(struct iio_trigger *trig)
>>> +{
>>> + return (trig->ops == &timer_trigger_ops);
>>> +}
>>> +EXPORT_SYMBOL(is_stm32_iio_timer_trigger);
>>> +
>>> +static int stm32_validate_trigger(struct iio_dev *indio_dev,
>>> + struct iio_trigger *trig)
>>> +{
>>> + struct stm32_iio_timer_dev *dev = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + if (!is_stm32_iio_timer_trigger(trig))
>>> + return -EINVAL;
>>> +
>>> + ret = of_property_match_string(dev->dev->of_node,
>>> + "st,input-triggers-names",
>>> + trig->name);
>>> +
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + regmap_update_bits(dev->regmap, TIM_SMCR, TIM_SMCR_TS, ret << 4);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct iio_info stm32_trigger_info = {
>>> + .driver_module = THIS_MODULE,
>>> + .validate_trigger = stm32_validate_trigger,
>>> + .attrs = &stm32_timer_attr_group,
>>> +};
>>> +
>>> +static struct stm32_iio_timer_dev *stm32_setup_iio_device(struct device *dev)
>>> +{
>>> + struct iio_dev *indio_dev;
>>> + int ret;
>>> +
>>> + indio_dev = devm_iio_device_alloc(dev, sizeof(struct stm32_iio_timer_dev));
>>> + if (!indio_dev)
>>> + return NULL;
>> This is 'unusual'. Why does a trigger driver need an iio_dev at all?
>
> Trigger doesn't need it but for configuring the input switch when
> validating the triggers I need a device
As suggested above, lets pull this trigger cascade clear of involving devices
at all. It's nice functionality to have anyway. Once we've figured it
out for this driver, I'd like to generalize it as I think the same stuff could
be used to do clean setup of oscilloscope sampling approaches for more
complex sensor setups...
>
>>> +
>>> + indio_dev->name = dev_name(dev);
>>> + indio_dev->dev.parent = dev;
>>> + indio_dev->info = &stm32_trigger_info;
>>> + indio_dev->modes = INDIO_EVENT_TRIGGERED;
>>> + indio_dev->num_channels = 0;
>>> + indio_dev->dev.of_node = dev->of_node;
>>> +
>>> + ret = iio_triggered_event_setup(indio_dev,
>>> + NULL,
>>> + stm32_timer_irq_handler);
>> So the iio_dev exists to provide the ability to fire this interrupt from
>> another trigger? Why do you want to do this?
>
> I need interrupt because I use set_trigger_state() to enable/disable
> the sampling frequency.
> As far I understand and test set_trigger_state() is only called when
> indio_dev->modes = INDIO_EVENT_TRIGGERED
> and iio_triggered_event_setup have been called to create register an
> irq handler.
> I just need irq declaration for this last condition, I don't need the
> irq to fire in kernel to drive other hardware block.
>
> If I could use set_trigger_state() without calling using
> iio_triggered_event_setup() I should remove all
> irq code from the driver.
>
> One possible solution would be to add calls to set_trigger_state() in
> iio_trigger_write_current() function
> at the same level than iio_trigger_detach_poll_func() or
> iio_trigger_attach_poll_func() calls:
I fear this may introduce race conditions in drivers that assume this stuff
can't change whilst the trigger is enabled.
Bit too risky a change to my mind.
If you need to add functions to explicitly do such a trigger enable, then
feel free to propose them. I never have a problem with adding core
functionality if that is the best way to solve a particular issue.
(subject to standard questions of maintainability and insisting they have
good documentation - do as I say, not as I did years ago ;)
>
> if (indio_dev->modes = INDIO_DIRECT_MODE && oldtrig->ops->set_trigger_state)
> oldtrig->ops->set_trigger_state(oldtrig, false);
>
> if (indio_dev->modes = INDIO_DIRECT_MODE &&
> indio_dev->trig->ops->set_trigger_state)
> indio_dev->trig->ops->set_trigger_state(indio_dev->trig, true);
>
> I'm to new in IIO framework to understand if that it possible or not
>
>>> + if (ret)
>>> + return NULL;
>>> +
>>> + ret = devm_iio_device_register(dev, indio_dev);
>>> + if (ret) {
>>> + iio_triggered_event_cleanup(indio_dev);
>>> + return NULL;
>>> + }
>>> +
>>> + return iio_priv(indio_dev);
>>> +}
>>> +
>>> +static int stm32_iio_timer_probe(struct platform_device *pdev)
>>> +{
>>> + struct device *dev = &pdev->dev;
>>> + struct stm32_iio_timer_dev *stm32;
>>> + struct stm32_gptimer_dev *mfd = dev_get_drvdata(pdev->dev.parent);
>>> + int ret;
>>> +
>>> + stm32 = stm32_setup_iio_device(dev);
>>> + if (!stm32)
>>> + return -ENOMEM;
>>> +
>>> + stm32->dev = dev;
>>> + stm32->regmap = mfd->regmap;
>>> + stm32->clk = mfd->clk;
>>> +
>>> + stm32->irq = platform_get_irq(pdev, 0);
>>> + if (stm32->irq < 0)
>>> + return -EINVAL;
>>> +
>>> + ret = devm_request_irq(stm32->dev, stm32->irq,
>>> + stm32_timer_irq_handler, IRQF_SHARED,
>>> + "iiotimer_event", stm32);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = stm32_setup_iio_triggers(stm32);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + platform_set_drvdata(pdev, stm32);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int stm32_iio_timer_remove(struct platform_device *pdev)
>>> +{
>>> + struct stm32_iio_timer_dev *stm32 = platform_get_drvdata(pdev);
>>> +
>>> + iio_triggered_event_cleanup((struct iio_dev *)stm32);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct of_device_id stm32_trig_of_match[] = {
>>> + {
>>> + .compatible = "st,stm32-iio-timer",
>>> + },
>>> +};
>>> +MODULE_DEVICE_TABLE(of, stm32_trig_of_match);
>>> +
>>> +static struct platform_driver stm32_iio_timer_driver = {
>>> + .probe = stm32_iio_timer_probe,
>>> + .remove = stm32_iio_timer_remove,
>>> + .driver = {
>>> + .name = DRIVER_NAME,
>>> + .of_match_table = stm32_trig_of_match,
>>> + },
>>> +};
>>> +module_platform_driver(stm32_iio_timer_driver);
>>> +
>>> +MODULE_ALIAS("platform:" DRIVER_NAME);
>>> +MODULE_DESCRIPTION("STMicroelectronics STM32 iio timer driver");
>>> +MODULE_LICENSE("GPL");
>>> diff --git a/drivers/iio/trigger/Kconfig b/drivers/iio/trigger/Kconfig
>>> index 809b2e7..f2af4fe 100644
>>> --- a/drivers/iio/trigger/Kconfig
>>> +++ b/drivers/iio/trigger/Kconfig
>>> @@ -46,5 +46,4 @@ config IIO_SYSFS_TRIGGER
>>>
>>> To compile this driver as a module, choose M here: the
>>> module will be called iio-trig-sysfs.
>>> -
>> Clear this out...
>>> endmenu
>>> diff --git a/include/dt-bindings/iio/timer/st,stm32-iio-timer.h b/include/dt-bindings/iio/timer/st,stm32-iio-timer.h
>>> new file mode 100644
>>> index 0000000..d39bf16
>>> --- /dev/null
>>> +++ b/include/dt-bindings/iio/timer/st,stm32-iio-timer.h
>>> @@ -0,0 +1,23 @@
>>> +/*
>>> + * st,stm32-iio-timer.h
>>> + *
>>> + * Copyright (C) STMicroelectronics 2016
>>> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
>>> + * License terms: GNU General Public License (GPL), version 2
>>> + */
>>> +
>>> +#ifndef _DT_BINDINGS_IIO_TIMER_H_
>>> +#define _DT_BINDINGS_IIO_TIMER_H_
>>> +
>>> +#define TIM1_TRGO "tim1_trgo"
>>> +#define TIM2_TRGO "tim2_trgo"
>>> +#define TIM3_TRGO "tim3_trgo"
>>> +#define TIM4_TRGO "tim4_trgo"
>>> +#define TIM5_TRGO "tim5_trgo"
>>> +#define TIM6_TRGO "tim6_trgo"
>>> +#define TIM7_TRGO "tim7_trgo"
>>> +#define TIM8_TRGO "tim8_trgo"
>>> +#define TIM9_TRGO "tim9_trgo"
>>> +#define TIM12_TRGO "tim12_trgo"
>>> +
>>> +#endif
>>> diff --git a/include/linux/iio/timer/stm32-iio-timers.h b/include/linux/iio/timer/stm32-iio-timers.h
>>> new file mode 100644
>>> index 0000000..5d1b86c
>>> --- /dev/null
>>> +++ b/include/linux/iio/timer/stm32-iio-timers.h
>>> @@ -0,0 +1,16 @@
>>> +/*
>>> + * stm32-iio-timers.h
>>> + *
>>> + * Copyright (C) STMicroelectronics 2016
>>> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
>>> + * License terms: GNU General Public License (GPL), version 2
>>> + */
>>> +
>>> +#ifndef _STM32_IIO_TIMERS_H_
>>> +#define _STM32_IIO_TIMERS_H_
>>> +
>>> +#include <dt-bindings/iio/timer/st,stm32-iio-timer.h>
>>> +
>>> +bool is_stm32_iio_timer_trigger(struct iio_trigger *trig);
>>> +
>>> +#endif
>>>
>>
^ permalink raw reply
* Re: [PATCH v3 00/13] net: ethernet: ti: cpts: update and fixes
From: Richard Cochran @ 2016-12-03 9:22 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev-u79uwXL29TY76Z2rM5mHXA, Mugunthan V N,
Sekhar Nori, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA, Murali Karicheri, Wingman Kwok,
Thomas Gleixner
In-Reply-To: <20161202203023.25526-1-grygorii.strashko-l0cyMroinI0@public.gmane.org>
On Fri, Dec 02, 2016 at 02:30:10PM -0600, Grygorii Strashko wrote:
> It is preparation series intended to clean up and optimize TI CPTS driver to
> facilitate further integration with other TI's SoCs like Keystone 2.
>
> Changes in v3:
> - patches reordered: fixes and small updates moved first
> - added comments in code about cpts->cc_mult
> - conversation range (maxsec) limited to 10sec
On net-next:
$ git am ~/grygorii.strashko
Applying: net: ethernet: ti: cpts: switch to readl/writel_relaxed()
Applying: net: ethernet: ti: allow cpts to be built separately
error: patch failed: drivers/net/ethernet/ti/cpsw.c:1963
error: drivers/net/ethernet/ti/cpsw.c: patch does not apply
Patch failed at 0002 net: ethernet: ti: allow cpts to be built separately
Also, you have the order of the SOB tags wrong. The author's SOB goes
first.
Thanks,
Richard
--
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 v2 1/7] MFD: add bindings for stm32 general purpose timer driver
From: Jonathan Cameron @ 2016-12-03 9:14 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: Lee Jones, robh+dt, Mark Rutland, alexandre.torgue, devicetree,
Linux Kernel Mailing List, Thierry Reding, linux-pwm, knaack.h,
Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio,
linux-arm-kernel, Fabrice Gasnier, Gerald Baeza, Arnaud Pouliquen,
Linus Walleij, Linaro Kernel Mailman List, Benjamin Gaignard
In-Reply-To: <CA+M3ks4Y4FnJGqwyH0oitrxvzRnKNHA261wqfEOSfc1aA4am4g@mail.gmail.com>
On 29/11/16 08:48, Benjamin Gaignard wrote:
> 2016-11-27 16:41 GMT+01:00 Jonathan Cameron <jic23@kernel.org>:
>> On 27/11/16 14:10, Jonathan Cameron wrote:
>>> On 24/11/16 15:14, Benjamin Gaignard wrote:
>>>> Add bindings information for stm32 general purpose timer
>>>>
>>>> version 2:
>>>> - rename stm32-mfd-timer to stm32-gptimer
>>>> - only keep one compatible string
>>>>
>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>>> ---
>>>> .../bindings/mfd/stm32-general-purpose-timer.txt | 43 ++++++++++++++++++++++
>>>> 1 file changed, 43 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/mfd/stm32-general-purpose-timer.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/stm32-general-purpose-timer.txt b/Documentation/devicetree/bindings/mfd/stm32-general-purpose-timer.txt
>>>> new file mode 100644
>>>> index 0000000..2f10e67
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mfd/stm32-general-purpose-timer.txt
>>>> @@ -0,0 +1,43 @@
>>>> +STM32 general purpose timer driver
>>>> +
>>>> +Required parameters:
>>>> +- compatible: must be "st,stm32-gptimer"
>>>> +
>>>> +- reg: Physical base address and length of the controller's
>>>> + registers.
>>>> +- clock-names: Set to "clk_int".
>>>> +- clocks: Phandle to the clock used by the timer module.
>>>> + For Clk properties, please refer to ../clock/clock-bindings.txt
>>>> +
>>>> +Optional parameters:
>>>> +- resets: Phandle to the parent reset controller.
>>>> + See ..reset/st,stm32-rcc.txt
>>>> +
>>>> +Optional subnodes:
>>>> +- pwm: See ../pwm/pwm-stm32.txt
>>>> +- iiotimer: See ../iio/timer/stm32-iio-timer.txt
>>> Naming issue here. Can't mention IIO as that's a linux subsystem and all
>>> bindings must be independent of OS.
>>>
>>> Perhaps adc-trigger-timer?
>>>> +
>>>> +Example:
>>>> + gptimer1: gptimer1@40010000 {
>>>> + compatible = "st,stm32-gptimer";
>>>> + reg = <0x40010000 0x400>;
>>>> + clocks = <&rcc 0 160>;
>>>> + clock-names = "clk_int";
>>>> +
>>>> + pwm1@0 {
>>>> + compatible = "st,stm32-pwm";
>>>> + st,pwm-num-chan = <4>;
>>>> + st,breakinput;
>>>> + st,complementary;
>>>> + };
>>>> +
>>>> + iiotimer1@0 {
>>>> + compatible = "st,stm32-iio-timer";
>>> Again, avoid the use of iio in here (same issue you had with mfd in the previous
>>> version).
>>>> + interrupts = <27>;
>>>> + st,input-triggers-names = TIM5_TRGO,
>>> Docs for these should be introduced before they are used in an example.
>>> Same for the PWM ones above. Expand the detail fo the example as you add
>>> the other elements.
>>
>> I've just dived into the datasheet for these timers.
>> http://www.st.com/content/ccc/resource/technical/document/reference_manual/3d/6d/5a/66/b4/99/40/d4/DM00031020.pdf/files/DM00031020.pdf/jcr:content/translations/en.DM00031020.pdf
>>
>
> I really appreciate that you do this effort, thanks.
>
>>
>> I think you need a binding that describes the capabilities of each of the timers
>> explicitly. Down to the level of whether there is a repetition counter or not.
>> Each should exists as a separate entity in device tree.
>>
>> They then have an existence as timers separate to the description of what they
>> are used for.
>>
>> Here the only way we are saying they exist is by their use which doesn't feel
>> right to me.
>>
>> So I think you need to move back to what you had in the first place. The key
>> thing is that ever timer needs describing fully. They are different enough
>> that for example the datasheet doesn't even try to describe them in one section.
>> (it has 4 separate chapters covering different sets of these hardware blocks).
>> The naming isn't really based on index, we are talking different hardware
>> that the datasheet authors have decided not to give different names to!
>
> Even if the hardware are named differently in the documentation they
> all share the
> same registers definitions and mapping but configurations are
> different for each device.
>
>>
>> If they'd called them
>> advanced timers
>> generic timers
>> basic timers
>> really basic timers meant for driving the DAC (6 and 7)
>>
>> We'd all have been quite happy with different compatible strings giving away
>> what they can do.
>
> 4 compatible strings will not be enough to describe devices
> configuration, for example
> in the documentation general purpose timers could have a 16 or 32 bit
> counter, for 1 to 4
> pwm channels and triggers (accepted or generated by the device) are
> different for each device.
>
> DAC could be drive by timers 2, 4, 5, 6, 7 and 8.
> ADC could be driver by 32 triggers
My point was more about the fact that though the naming appears to (and kind
of does describe an index) these devices are really as different as for example
different part numbers would imply on a range of ADCs (say the multitude supported
by the max1363 driver - all of which are very nearly register compatible)
Hence I'd be less quick to dismiss the option of a number of compatible strings
rather than the wealth of description you'll otherwise have to put in the device
tree.
>
>> What you have here is far too specific to what you are trying to do with them
>> right now.
>>
>> These things are separately capable of timing capture (which is I guess where
>> the IIO device later comes in).
>>
>> So my expectation is that we end up potentially instantiating:
>>
>> 1) An MFD to handle the shared elements of the timers.
>> 2) Up to 12ish timers each with separate existence as a device in the driver model
>> and in device tree.
>> (nasty corner cases here are using timers as perscalers for other timers - I'd be
>> tempted to leave that for now)
>> Note that each of these devices has a different register set I think? Any shared
>> bits are handled via the mfd above (if we even need that MFD which I'm starting
>> to doubt looking at the datasheet).
>>
>
> pwm and trigger share the same registers but not the same bits.
> With regmap write functions I don't have sharing problems.
>
>> 3) Up to N pwms again with there own existence in the device model. These don't
>> do much other than wrap the timer and stick it in output mode.
>> 4) Up to N iio triggers - this is basically using the timer as a periodic interrupt
>> (though without the interrupt having visibility to the kernel) which fires off
>> sampling on associated ADCs.
>> 5) Up to N iio capture devices for all channels that support timing capture.
>> Note there is also hardware encoder capture support in these which should be
>> correctly handled as well. This comes back to an ancient discussion on the
>> TI ecap units which have similar capabilities (driver never went anywhere but
>> I think that was because the author left TI).
>>
>> Certainly for the IIO devices these should no be bound up into one instance
>> as you have done here.
>>
>> Anyhow, I fear that right now this discussion is missing the key ingredient
>> that the hardware is horrendously variable in it's capabilities and really
>> is 4-5 different types of hardware that just happen to share a few bits of
>> their offsets in their register maps.
>
> Hardware really share the same registers mapping that why I have wrote
> one only driver
> per framework. Only the configurations are different
One driver is fine, but the difference to my mind are sufficient that
we may need to use compatible strings for the various options. Worth
a go at trying to fully describe them first though!
>
>>
>> So after all that I'm almost more confused than I was at the start!
>>
>> Jonathan
>>
>>
>>>> + TIM2_TRGO,
>>>> + TIM4_TRGO,
>>>> + TIM3_TRGO;
>>>> + st,output-triggers-names = TIM1_TRGO;
>>>> + };
>>>> + };
>>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>
>
>
^ permalink raw reply
* Re: [PATCH] iio: misc: add a generic regulator driver
From: Jonathan Cameron @ 2016-12-03 9:11 UTC (permalink / raw)
To: Lars-Peter Clausen, Bartosz Golaszewski
Cc: Hartmut Knaack, Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-devicetree, LKML,
Kevin Hilman, Patrick Titiano, Neil Armstrong, Liam Girdwood,
Mark Brown
In-Reply-To: <d4c7f1ca-49d4-7abe-bfc9-e1728f62a9fb-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
On 30/11/16 10:10, Lars-Peter Clausen wrote:
> On 11/29/2016 04:35 PM, Bartosz Golaszewski wrote:
>> 2016-11-29 16:30 GMT+01:00 Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>:
>>> On 11/29/2016 04:22 PM, Bartosz Golaszewski wrote:
>>> [...]
>>>> diff --git a/Documentation/devicetree/bindings/iio/misc/iio-regulator.txt b/Documentation/devicetree/bindings/iio/misc/iio-regulator.txt
>>>> new file mode 100644
>>>> index 0000000..147458f
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/iio/misc/iio-regulator.txt
>>>> @@ -0,0 +1,18 @@
>>>> +Industrial IO regulator device driver
>>>> +-------------------------------------
>>>> +
>>>> +This document describes the bindings for the iio-regulator - a dummy device
>>>> +driver representing a physical regulator within the iio framework.
>>>
>>> No bindings for drivers, only for hardware. So this wont work.
>>>
>>
>> What about exporting regulator attributes analogous to the one in this
>> patch from the iio-core when a *-supply property is specified for a
>> node?
>
> The problem with exposing direct control to the regulator is that it allows
> to modify the hardware state without the drivers knowledge. If you
> power-cycle a device all previous configuration that has been written to the
> device is reset. The device driver needs to be aware of this otherwise its
> assumed state and the actual device state can divert which will result in
> undefined behavior. Also access to the device will fail unexpectedly when
> the regulator is turned off. So I think generally the driver should
> explicitly control the regulator, power-up when needed, power-down when not.
I agree with what Lars has said.
There 'may' be some argument to ultimately have a bridge driver from
regulators to IIO. That would be for cases where the divide between a regulator
and a DAC is blurred. However it would still have to play nicely with the
regulator framework and any other devices registered on that regulator.
Ultimately the ideal in that case would then be to describe what the DAC is
actually being used to do but that's a more complex issue!
That doesn't seem to be what you are targeting here.
What it sounds like you need is to have the hardware well enough described that
the standard runtime power management can disable the regulator just fine when
it is not in use. This may mean improving the power management in the relevant
drivers.
Jonathan
p.s. If ever proposing to do something 'unusual' with a regulator you should
bring in the regulator framework maintainers in the cc list.
>
> - Lars
>
--
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 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants
From: Marek Vasut @ 2016-12-03 2:49 UTC (permalink / raw)
To: Masahiro Yamada, Rob Herring
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux Kernel Mailing List, Boris Brezillon, Brian Norris,
Richard Weinberger, David Woodhouse, Cyrille Pitchen,
Mark Rutland, Dinh Nguyen, Alan Tull, Chin Liang See
In-Reply-To: <CAK7LNAQHnH=On=+7fzenu_v6rB71y9hYuAZi5oinZFu-tfAdjw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 12/03/2016 03:41 AM, Masahiro Yamada wrote:
> Hi Rob,
Hi!
> 2016-12-03 1:26 GMT+09:00 Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
>
>>>
>>>
>>> (Plan A)
>>> "denali,socfpga-nand" (for Altera SOCFPGA variant)
>>> "denali,uniphier-nand-v1" (for old Socionext UniPhier family variant)
>>> "denali,uniphier-nand-v2" (for new Socionext UniPhier family variant)
>>>
>>> (Plan B)
>>> "altera,denali-nand" (for Altera SOCFPGA variant)
>>> "socionext,denali-nand-v5a" (for old Socionext UniPhier family variant)
>>> "socionext,denali-nand-v5b" (for new Socionext UniPhier family variant)
>
>> Let the Altera folks worry about their stuff. At least for soft IP in
>> FPGA, it's a bit of a special case. The old string can remain as bad
>> as it is.
>
>
> Hmm, I am not sure if this IP would fit in FPGA
> (to use it along with NIOS-II?)
>
> (even if it happened, nothing of this IP would be customizable on users' side.
> When buying the IP, SoC vendors submit a list of desired features.
> Denali (now Cadence) generates the RTL according to the configuration sheet.
> The function is fixed at this point. So, generic compatible would be
> useless anyway.)
>
>
> If we are talking about SOCFPGA,
> SOCFPGA is not only FPGA. Rather "SOC" + "FPGA".
> It consists of two parts:
> [1] SOC part (Cortex-A9 + various hard-wired peripherals such UART,
> USB, SD, NAND, ...)
> [2] FPGA part (User design logic)
>
> The Denali NAND controller is included in [1].
> So, as far as we talk about the Denali on SOCFPGA,
> it is as hard-wired as Intel, Socionext's ones.
That's correct, the Denali NAND IP in altera socfpga is a hardware
block. You can make it available to the fabric too, but by default
it's used by the ARM part of the chip, so for this discussion, you
can forget that the FPGA part exists altogether.
I would be in favor of plan B, since it seems to be the more often
taken approach. A nice example is ci-hdrc:
$ git grep compatible drivers/usb/chipidea/
>> I simply would do "socionext,uniphier-v5b-nand" (and v5a).
>> The fact that it is denali is part of the documentation.
>>
>
> Let me think about this.
>
> Socionext bought two version of Denali IP,
> and we are now re-using the newer one (v5b) for several SoCs.
> Socionext has some more product lines other than Uniphier SoC family,
> perhaps wider re-use might happen in the future.
>
> At first, I included "uniphier" in compatible, but I am still wondering
> if such a specific string is good or not.
>
> Also, comments from Altera engineers are appreciated.
Adding a few more on Cc
--
Best regards,
Marek Vasut
--
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 39/39] mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants
From: Masahiro Yamada @ 2016-12-03 2:41 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree@vger.kernel.org, Boris Brezillon,
Richard Weinberger, Linux Kernel Mailing List, Marek Vasut,
linux-mtd@lists.infradead.org, Cyrille Pitchen, Brian Norris,
David Woodhouse, Dinh Nguyen
In-Reply-To: <CAL_Jsq+H-JcAtDV3NCfptCWrmTgkQkmZFTj=Do8HhX4b=At9RA@mail.gmail.com>
Hi Rob,
2016-12-03 1:26 GMT+09:00 Rob Herring <robh@kernel.org>:
>>
>>
>> (Plan A)
>> "denali,socfpga-nand" (for Altera SOCFPGA variant)
>> "denali,uniphier-nand-v1" (for old Socionext UniPhier family variant)
>> "denali,uniphier-nand-v2" (for new Socionext UniPhier family variant)
>>
>> (Plan B)
>> "altera,denali-nand" (for Altera SOCFPGA variant)
>> "socionext,denali-nand-v5a" (for old Socionext UniPhier family variant)
>> "socionext,denali-nand-v5b" (for new Socionext UniPhier family variant)
> Let the Altera folks worry about their stuff. At least for soft IP in
> FPGA, it's a bit of a special case. The old string can remain as bad
> as it is.
Hmm, I am not sure if this IP would fit in FPGA
(to use it along with NIOS-II?)
(even if it happened, nothing of this IP would be customizable on users' side.
When buying the IP, SoC vendors submit a list of desired features.
Denali (now Cadence) generates the RTL according to the configuration sheet.
The function is fixed at this point. So, generic compatible would be
useless anyway.)
If we are talking about SOCFPGA,
SOCFPGA is not only FPGA. Rather "SOC" + "FPGA".
It consists of two parts:
[1] SOC part (Cortex-A9 + various hard-wired peripherals such UART,
USB, SD, NAND, ...)
[2] FPGA part (User design logic)
The Denali NAND controller is included in [1].
So, as far as we talk about the Denali on SOCFPGA,
it is as hard-wired as Intel, Socionext's ones.
> I simply would do "socionext,uniphier-v5b-nand" (and v5a).
> The fact that it is denali is part of the documentation.
>
Let me think about this.
Socionext bought two version of Denali IP,
and we are now re-using the newer one (v5b) for several SoCs.
Socionext has some more product lines other than Uniphier SoC family,
perhaps wider re-use might happen in the future.
At first, I included "uniphier" in compatible, but I am still wondering
if such a specific string is good or not.
Also, comments from Altera engineers are appreciated.
--
Best Regards
Masahiro Yamada
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply
* Re: [PATCHv4 11/15] clk: ti: clockdomain: add clock provider support to clockdomains
From: Tony Lindgren @ 2016-12-03 0:18 UTC (permalink / raw)
To: Michael Turquette
Cc: Stephen Boyd, Tero Kristo, linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring
In-Reply-To: <148072274164.32158.6012452044533845688@resonance>
* Michael Turquette <mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> [161202 15:52]:
> Quoting Tony Lindgren (2016-12-02 15:12:40)
> > * Michael Turquette <mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> [161202 14:34]:
> > > Quoting Tony Lindgren (2016-10-28 16:54:48)
> > > > * Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> [161028 16:37]:
> > > > > On 10/28, Tony Lindgren wrote:
> > > > > > * Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org> [161028 00:43]:
> > > > > > > On 28/10/16 03:50, Stephen Boyd wrote:
> > > > > > > > I suppose a PRCM is
> > > > > > > > like an MFD that has clocks and resets under it? On other
> > > > > > > > platforms we've combined that all into one node and just had
> > > > > > > > #clock-cells and #reset-cells in that node. Is there any reason
> > > > > > > > we can't do that here?
> > > > > > >
> > > > > > > For OMAPs, there are typically multiple instances of the PRCM around; OMAP4
> > > > > > > for example has:
> > > > > > >
> > > > > > > cm1 @ 0x4a004000 (clocks + clockdomains)
> > > > > > > cm2 @ 0x4a008000 (clocks + clockdomains)
> > > > > > > prm @ 0x4a306000 (few clocks + resets + power state handling)
> > > > > > > scrm @ 0x4a30a000 (few external clocks + plenty of misc stuff)
> > > > > > >
> > > > > > > These instances are also under different power/voltage domains which means
> > > > > > > their PM behavior is different.
> > > > > > >
> > > > > > > The idea behind having a clockdomain as a provider was mostly to have the
> > > > > > > topology visible : prcm-instance -> clockdomain -> clocks
> > > > > >
> > > > > > Yeah that's needed to get the interconnect hierarchy right for
> > > > > > genpd :)
> > > > > >
> > > > > > > ... but basically I think it would be possible to drop the clockdomain
> > > > > > > representation and just mark the prcm-instance as a clock provider. Tony,
> > > > > > > any thoughts on that?
> > > > > >
> > > > > > No let's not drop the clockdomains as those will be needed when we
> > > > > > move things into proper hierarchy within the interconnect instances.
> > > > > > This will then help with getting things right with genpd.
> > > > > >
> > > > > > In the long run we just want to specify clockdomain and the offset of
> > > > > > the clock instance within the clockdomain in the dts files.
> > > > > >
> > > > >
> > > > > Sorry, I have very little idea how OMAP hardware works. Do you
> > > > > mean that you will have different nodes for each clockdomain so
> > > > > that genpd can map 1:1 to the node in dts? But in hardware
> > > > > there's a prcm that allows us to control many clock domains
> > > > > through register read/writes? How is the interconnect involved?
> > > >
> > > > There are multiple clockdomains, at least one for each interconnect
> > > > instance. Once a clockdomain is idle, the related interconnect can
> > > > idle too. So yeah genpd pretty much maps 1:1 with the clockdomains.
> > > >
> > > > There's more info in for example omap4 TRM section "3.4.1 Device
> > > > Power-Management Layout" that shows the voltage/power/clock domains.
> > > > The interconnect instances are mostly named there too looking at
> > > > the L4/L3 naming.
> > >
> > > I'm confused on two points:
> > >
> > > 1) why are the clkdm's acting as clock providers? I've always hated the
> > > name "clock domain" since those bits are for managing module state, not
> > > clock state. The PRM, CM1 and CM2 provide the clocks, not the
> > > clockdomains.
> >
> > The clock domains have multiple clock inputs that are routed to multiple
> > child clocks. So it is a clock :)
> >
> > See for example omap4430 TRM "3.6.4 CD_WKUP Clock Domain" on page
> > 393 in my revision here.
> >
> > On that page "Figure 3-48" shows CD_WKUP with the four input clocks.
> > And then "Table 3-84. CD_WKUP Control and Status Parameters" shows
> > the CD_WKUP clock domain specific registers. These registers show
> > the status, I think they are all read-only registers. Then CD_WKUP
> > has multiple child clocks with configurable registers.
> >
> > From hardware register point of view, each clock domain has:
> >
> > - Read-only clockdomain status registers in the beginning of
> > the address space
> >
> > - Multiple similar clock instances register instances each
> > mapping to a specific interconnect target module
> >
> > These are documented in "3.11.16.1 WKUP_CM Register Summary".
>
> Oh, this is because you are treating the MODULEMODE bits like gate
> clocks. I never really figured out if this was the best way to model
> those bits since they do more than control a line toggling at a rate.
> For instance this bit will affect the master/slave IDLE protocol between
> the module and the PRCM.
Yes seems like there is some negotiation going on there with the
target module. But from practical point of view the CLKCTRL
register is the gate for a module functional clock.
> > From hardware point of view, we ideally want to map interconnect
> > target modules to the clock instance offset from the clock domain
> > for that interconnect segment. For example gptimer1 clocks would
> > be just:
> >
> > clocks = <&cd_wkup 0x40>;
> >
> > > 2) why aren't the clock domains modeled as genpds with their associated
> > > devices attached to them? Note that it is possible to "nest" genpd
> > > objects. This would also allow for the "Clockdomain Dependency"
> > > relationships to be properly modeled (see section 3.1.1.1.7 Clock Domain
> > > Dependency in the OMAP4 TRM).
> >
> > Clock domains only route clocks to child clocks. Power domains
> > are different registers. The power domains map roughly to
> > interconnect instances, there we have registers to disable the
> > whole interconnect when idle.
>
> I'm not talking about power islands at all, but the genpd object in
> Linux. For instance, if we treat each clock domain like a clock
> provider, how could the functional dependency between clkdm_A and
> clkdm_B be asserted?
To me it seems that some output of a clockdomain is just a input
of another clockdomain? So it's just the usual parent child
relationship once we treat a clockdomain just as a clock. Tero
probably has some input here.
> There is certainly no API for that in the clock framework, but for genpd
> your runtime_pm_get() callback for clkdm_A could call runtime_pm_get
> against clkdm_B, which would satisfy the requirement. See section
> 3.1.1.1.7 Clock Domain Dependency in the OMAP4 TRM, version AB.
To me it seems the API is just clk_get() :) Do you have some
specific example we can use to check? My guess is that the
TRM "Clock Domain Dependency" is just the usual parent child
relationship between clocks that are the clockdomains..
If there is something more magical there certainly that should
be considered though.
Regards,
Tony
--
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: [RESEND PATCH v2 0/7] drm/vc4: VEC (SDTV) output support
From: Eric Anholt @ 2016-12-02 23:59 UTC (permalink / raw)
To: Boris Brezillon, David Airlie, Daniel Vetter, dri-devel
Cc: Mark Rutland, devicetree, Ian Campbell, Florian Fainelli,
Pawel Moll, Scott Branden, Stephen Warren, Ray Jui, Lee Jones,
Rob Herring, bcm-kernel-feedback-list, linux-rpi-kernel,
Kumar Gala, linux-arm-kernel
In-Reply-To: <1480686493-4813-1-git-send-email-boris.brezillon@free-electrons.com>
[-- Attachment #1.1: Type: text/plain, Size: 396 bytes --]
Boris Brezillon <boris.brezillon@free-electrons.com> writes:
> Sorry for the noise, but I forgot to Cc the DT maintainers.
>
> Here is the 2nd version of the VC4/VEC series.
>
> We still miss the two clock patches mentioned by Eric in the first
> version to make the encoder work no matter the setting applied by the
> bootloader.
I'm happy with the series and I'm just waiting for the DT ack.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCHv4 11/15] clk: ti: clockdomain: add clock provider support to clockdomains
From: Michael Turquette @ 2016-12-02 23:52 UTC (permalink / raw)
To: Tony Lindgren
Cc: devicetree, Stephen Boyd, Tero Kristo, Rob Herring, linux-omap,
linux-clk, linux-arm-kernel
In-Reply-To: <20161202231240.GH4705@atomide.com>
Quoting Tony Lindgren (2016-12-02 15:12:40)
> * Michael Turquette <mturquette@baylibre.com> [161202 14:34]:
> > Quoting Tony Lindgren (2016-10-28 16:54:48)
> > > * Stephen Boyd <sboyd@codeaurora.org> [161028 16:37]:
> > > > On 10/28, Tony Lindgren wrote:
> > > > > * Tero Kristo <t-kristo@ti.com> [161028 00:43]:
> > > > > > On 28/10/16 03:50, Stephen Boyd wrote:
> > > > > > > I suppose a PRCM is
> > > > > > > like an MFD that has clocks and resets under it? On other
> > > > > > > platforms we've combined that all into one node and just had
> > > > > > > #clock-cells and #reset-cells in that node. Is there any reason
> > > > > > > we can't do that here?
> > > > > >
> > > > > > For OMAPs, there are typically multiple instances of the PRCM around; OMAP4
> > > > > > for example has:
> > > > > >
> > > > > > cm1 @ 0x4a004000 (clocks + clockdomains)
> > > > > > cm2 @ 0x4a008000 (clocks + clockdomains)
> > > > > > prm @ 0x4a306000 (few clocks + resets + power state handling)
> > > > > > scrm @ 0x4a30a000 (few external clocks + plenty of misc stuff)
> > > > > >
> > > > > > These instances are also under different power/voltage domains which means
> > > > > > their PM behavior is different.
> > > > > >
> > > > > > The idea behind having a clockdomain as a provider was mostly to have the
> > > > > > topology visible : prcm-instance -> clockdomain -> clocks
> > > > >
> > > > > Yeah that's needed to get the interconnect hierarchy right for
> > > > > genpd :)
> > > > >
> > > > > > ... but basically I think it would be possible to drop the clockdomain
> > > > > > representation and just mark the prcm-instance as a clock provider. Tony,
> > > > > > any thoughts on that?
> > > > >
> > > > > No let's not drop the clockdomains as those will be needed when we
> > > > > move things into proper hierarchy within the interconnect instances.
> > > > > This will then help with getting things right with genpd.
> > > > >
> > > > > In the long run we just want to specify clockdomain and the offset of
> > > > > the clock instance within the clockdomain in the dts files.
> > > > >
> > > >
> > > > Sorry, I have very little idea how OMAP hardware works. Do you
> > > > mean that you will have different nodes for each clockdomain so
> > > > that genpd can map 1:1 to the node in dts? But in hardware
> > > > there's a prcm that allows us to control many clock domains
> > > > through register read/writes? How is the interconnect involved?
> > >
> > > There are multiple clockdomains, at least one for each interconnect
> > > instance. Once a clockdomain is idle, the related interconnect can
> > > idle too. So yeah genpd pretty much maps 1:1 with the clockdomains.
> > >
> > > There's more info in for example omap4 TRM section "3.4.1 Device
> > > Power-Management Layout" that shows the voltage/power/clock domains.
> > > The interconnect instances are mostly named there too looking at
> > > the L4/L3 naming.
> >
> > I'm confused on two points:
> >
> > 1) why are the clkdm's acting as clock providers? I've always hated the
> > name "clock domain" since those bits are for managing module state, not
> > clock state. The PRM, CM1 and CM2 provide the clocks, not the
> > clockdomains.
>
> The clock domains have multiple clock inputs that are routed to multiple
> child clocks. So it is a clock :)
>
> See for example omap4430 TRM "3.6.4 CD_WKUP Clock Domain" on page
> 393 in my revision here.
>
> On that page "Figure 3-48" shows CD_WKUP with the four input clocks.
> And then "Table 3-84. CD_WKUP Control and Status Parameters" shows
> the CD_WKUP clock domain specific registers. These registers show
> the status, I think they are all read-only registers. Then CD_WKUP
> has multiple child clocks with configurable registers.
>
> From hardware register point of view, each clock domain has:
>
> - Read-only clockdomain status registers in the beginning of
> the address space
>
> - Multiple similar clock instances register instances each
> mapping to a specific interconnect target module
>
> These are documented in "3.11.16.1 WKUP_CM Register Summary".
Oh, this is because you are treating the MODULEMODE bits like gate
clocks. I never really figured out if this was the best way to model
those bits since they do more than control a line toggling at a rate.
For instance this bit will affect the master/slave IDLE protocol between
the module and the PRCM.
>
> From hardware point of view, we ideally want to map interconnect
> target modules to the clock instance offset from the clock domain
> for that interconnect segment. For example gptimer1 clocks would
> be just:
>
> clocks = <&cd_wkup 0x40>;
>
> > 2) why aren't the clock domains modeled as genpds with their associated
> > devices attached to them? Note that it is possible to "nest" genpd
> > objects. This would also allow for the "Clockdomain Dependency"
> > relationships to be properly modeled (see section 3.1.1.1.7 Clock Domain
> > Dependency in the OMAP4 TRM).
>
> Clock domains only route clocks to child clocks. Power domains
> are different registers. The power domains map roughly to
> interconnect instances, there we have registers to disable the
> whole interconnect when idle.
I'm not talking about power islands at all, but the genpd object in
Linux. For instance, if we treat each clock domain like a clock
provider, how could the functional dependency between clkdm_A and
clkdm_B be asserted?
There is certainly no API for that in the clock framework, but for genpd
your runtime_pm_get() callback for clkdm_A could call runtime_pm_get
against clkdm_B, which would satisfy the requirement. See section
3.1.1.1.7 Clock Domain Dependency in the OMAP4 TRM, version AB.
Regards,
Mike
>
> Regards,
>
> Tony
^ permalink raw reply
* Re: [PATCH v2 0/7] stmmac: dwmac-meson8b: configurable RGMII TX delay
From: Martin Blumenstingl @ 2016-12-02 23:52 UTC (permalink / raw)
To: David Miller
Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
khilman-rdvid1DuHRBWk0Htik3J/w, mark.rutland-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
alexandre.torgue-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o,
will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
carlo-KA+7E9HrN00dnm+yROfE0A, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20161127.203324.1634866862730391239.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Mon, Nov 28, 2016 at 2:33 AM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> From: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> Date: Fri, 25 Nov 2016 14:01:49 +0100
>
>> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
>> cycle TX clock delay. This seems to work fine for many boards (for
>> example Odroid-C2 or Amlogic's reference boards) but there are some
>> others where TX traffic is simply broken.
>> There are probably multiple reasons why it's working on some boards
>> while it's broken on others:
>> - some of Amlogic's reference boards are using a Micrel PHY
>> - hardware circuit design
>> - maybe more...
>
> The ARM arch file changes do not apply cleanly to net-next, you probably
> want to merge them via the ARM tree instead of mine, and respin this series
> to be without the .dts file changes.
done, v3 contains only the net-next changes while the dts changes can
be found here: [0]
Regards,
Martin
[0] http://lists.infradead.org/pipermail/linux-amlogic/2016-December/001836.html
--
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 5/5] ARM64: dts: amlogic: add the ethernet TX delay configuration
From: Martin Blumenstingl @ 2016-12-02 23:47 UTC (permalink / raw)
To: linux-amlogic, khilman, carlo
Cc: mark.rutland, devicetree, Martin Blumenstingl, catalin.marinas,
will.deacon, robh+dt, linux-arm-kernel
In-Reply-To: <20161202234739.22929-1-martin.blumenstingl@googlemail.com>
This adds the amlogic,tx-delay-ns with the old (hardcoded) default value
of 2ns to all boards which are using an RGMII ethernet PHY.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Tested-by: Neil Armstrong <narmstrong@baylibre.com>
---
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 2 ++
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 2 ++
arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi | 2 ++
arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts | 2 ++
arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts | 2 ++
arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts | 2 ++
6 files changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index cbaf024..fdade07 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -161,6 +161,8 @@
snps,reset-delays-us = <0 10000 1000000>;
snps,reset-active-low;
+ amlogic,tx-delay-ns = <2>;
+
phy-mode = "rgmii";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
index 2abc553..8172e12 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
@@ -152,6 +152,8 @@
snps,reset-delays-us = <0 10000 1000000>;
snps,reset-active-low;
+ amlogic,tx-delay-ns = <2>;
+
phy-mode = "rgmii";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
index a0e92e3..ab49712 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
@@ -131,6 +131,8 @@
snps,reset-delays-us = <0 10000 1000000>;
snps,reset-active-low;
+ amlogic,tx-delay-ns = <2>;
+
phy-mode = "rgmii";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
index f66939c..7fd11c6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
@@ -64,6 +64,8 @@
snps,reset-delays-us = <0 10000 1000000>;
snps,reset-active-low;
+ amlogic,tx-delay-ns = <2>;
+
/* External PHY is in RGMII */
phy-mode = "rgmii";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
index f859d75..cf9b960 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
@@ -156,6 +156,8 @@
snps,reset-delays-us = <0 10000 1000000>;
snps,reset-active-low;
+ amlogic,tx-delay-ns = <2>;
+
/* External PHY is in RGMII */
phy-mode = "rgmii";
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts
index 5dbc660..e428e29 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-s912-q200.dts
@@ -64,6 +64,8 @@
snps,reset-delays-us = <0 10000 1000000>;
snps,reset-active-low;
+ amlogic,tx-delay-ns = <2>;
+
/* External PHY is in RGMII */
phy-mode = "rgmii";
};
--
2.10.2
^ permalink raw reply related
* [PATCH 4/5] ARM64: dts: meson-gxbb-vega-s95: add reset for the ethernet PHY
From: Martin Blumenstingl @ 2016-12-02 23:47 UTC (permalink / raw)
To: linux-amlogic, khilman, carlo
Cc: mark.rutland, devicetree, Martin Blumenstingl, catalin.marinas,
will.deacon, robh+dt, linux-arm-kernel
In-Reply-To: <20161202234739.22929-1-martin.blumenstingl@googlemail.com>
This resets the ethernet PHY during boot to get the PHY into a "clean"
state. While here also specify the phy-handle of the ethmac node to
make the PHY configuration similar to the one we have on GXL devices.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Tested-by: Neil Armstrong <narmstrong@baylibre.com>
---
arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
index e59ad30..a0e92e3 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
@@ -113,10 +113,25 @@
pinctrl-names = "default";
};
+&mdio0 {
+ ethernet_phy0: ethernet-phy@0 {
+ compatible = "ethernet-phy-id001c.c916", "ethernet-phy-ieee802.3-c22";
+ reg = <0>;
+ };
+};
+
ðmac {
status = "okay";
pinctrl-0 = <ð_rgmii_pins>;
pinctrl-names = "default";
+
+ phy-handle = <ðernet_phy0>;
+
+ snps,reset-gpio = <&gpio GPIOZ_14 0>;
+ snps,reset-delays-us = <0 10000 1000000>;
+ snps,reset-active-low;
+
+ phy-mode = "rgmii";
};
&usb0_phy {
--
2.10.2
^ permalink raw reply related
* [PATCH 3/5] ARM64: dts: meson-gxbb-p20x: add reset for the ethernet PHY
From: Martin Blumenstingl @ 2016-12-02 23:47 UTC (permalink / raw)
To: linux-amlogic, khilman, carlo
Cc: mark.rutland, devicetree, Martin Blumenstingl, catalin.marinas,
will.deacon, robh+dt, linux-arm-kernel
In-Reply-To: <20161202234739.22929-1-martin.blumenstingl@googlemail.com>
This resets the ethernet PHY during boot to get the PHY into a "clean"
state. While here also specify the phy-handle of the ethmac node to
make the PHY configuration similar to the one we have on GXL devices.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Tested-by: Neil Armstrong <narmstrong@baylibre.com>
---
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
index 203be28..2abc553 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
@@ -134,10 +134,25 @@
pinctrl-names = "default";
};
+&mdio0 {
+ ethernet_phy0: ethernet-phy@0 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <0>;
+ };
+};
+
ðmac {
status = "okay";
pinctrl-0 = <ð_rgmii_pins>;
pinctrl-names = "default";
+
+ phy-handle = <ðernet_phy0>;
+
+ snps,reset-gpio = <&gpio GPIOZ_14 0>;
+ snps,reset-delays-us = <0 10000 1000000>;
+ snps,reset-active-low;
+
+ phy-mode = "rgmii";
};
&ir {
--
2.10.2
^ permalink raw reply related
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