* [PATCH v4 4/8] dt-bindings: add axentia to vendor-prefixes
From: Peter Rosin @ 2016-11-08 11:58 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Daniel Baluta,
Slawomir Stepien, Thomas Gleixner,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478606339-31253-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index f0a48ea78659..a437120a7eee 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -39,6 +39,7 @@ auo AU Optronics Corporation
auvidea Auvidea GmbH
avago Avago Technologies
avic Shanghai AVIC Optoelectronics Co., Ltd.
+axentia Axentia Technologies AB
axis Axis Communications AB
boe BOE Technology Group Co., Ltd.
bosch Bosch Sensortec GmbH
--
2.1.4
^ permalink raw reply related
* [PATCH v4 5/8] dt-bindings: iio: document dpot-dac bindings
From: Peter Rosin @ 2016-11-08 11:58 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Daniel Baluta,
Slawomir Stepien, Thomas Gleixner, linux-iio, devicetree
In-Reply-To: <1478606339-31253-1-git-send-email-peda@axentia.se>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
.../devicetree/bindings/iio/dac/dpot-dac.txt | 41 ++++++++++++++++++++++
MAINTAINERS | 6 ++++
2 files changed, 47 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
diff --git a/Documentation/devicetree/bindings/iio/dac/dpot-dac.txt b/Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
new file mode 100644
index 000000000000..fdf47a01bfef
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
@@ -0,0 +1,41 @@
+Bindings for DAC emulation using a digital potentiometer
+
+It is assumed that the dpot is used as a voltage divider between the
+current dpot wiper setting and the maximum resistance of the dpot. The
+divided voltage is provided by a vref regulator.
+
+ .------.
+ .-----------. | |
+ | vref |--' .---.
+ | regulator |--. | |
+ '-----------' | | d |
+ | | p |
+ | | o | wiper
+ | | t |<---------+
+ | | |
+ | '---' dac output voltage
+ | |
+ '------+------------+
+
+Required properties:
+- compatible: Should be "dpot-dac"
+- vref-supply: The regulator supplying the voltage divider.
+- io-channels: Channel node of the dpot to be used for the voltage division.
+- io-channel-names: Should be "dpot".
+
+Example:
+
+ &i2c {
+ dpot: mcp4651-503@28 {
+ compatible = "microchip,mcp4651-503";
+ reg = <0x28>;
+ #io-channel-cells = <1>;
+ };
+ };
+
+ dac {
+ compatible = "dpot-dac";
+ vref-supply = <®_3v3>;
+ io-channels = <&dpot 0>;
+ io-channel-names = "dpot";
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 2409e1d79cb6..4ff9c12b1313 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6124,6 +6124,12 @@ L: linux-media@vger.kernel.org
S: Maintained
F: drivers/media/rc/iguanair.c
+IIO DIGITAL POTENTIOMETER DAC
+M: Peter Rosin <peda@axentia.se>
+L: linux-iio@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
+
IIO SUBSYSTEM AND DRIVERS
M: Jonathan Cameron <jic23@kernel.org>
R: Hartmut Knaack <knaack.h@gmx.de>
--
2.1.4
^ permalink raw reply related
* [PATCH v4 6/8] iio: dpot-dac: DAC driver based on a digital potentiometer
From: Peter Rosin @ 2016-11-08 11:58 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Daniel Baluta,
Slawomir Stepien, Thomas Gleixner, linux-iio, devicetree
In-Reply-To: <1478606339-31253-1-git-send-email-peda@axentia.se>
It is assumed that the dpot is used as a voltage divider between the
current dpot wiper setting and the maximum resistance of the dpot. The
divided voltage is provided by a vref regulator.
.------.
.-----------. | |
| vref |--' .---.
| regulator |--. | |
'-----------' | | d |
| | p |
| | o | wiper
| | t |<---------+
| | |
| '---' dac output voltage
| |
'------+------------+
Signed-off-by: Peter Rosin <peda@axentia.se>
---
.../ABI/testing/sysfs-bus-iio-dac-dpot-dac | 8 +
MAINTAINERS | 2 +
drivers/iio/dac/Kconfig | 10 +
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/dpot-dac.c | 266 +++++++++++++++++++++
5 files changed, 287 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
create mode 100644 drivers/iio/dac/dpot-dac.c
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac b/Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
new file mode 100644
index 000000000000..580e93f373f6
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
@@ -0,0 +1,8 @@
+What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_raw_available
+Date: October 2016
+KernelVersion: 4.9
+Contact: Peter Rosin <peda@axentia.se>
+Description:
+ The range of available values represented as the minimum value,
+ the step and the maximum value, all enclosed in square brackets.
+ Example: [0 1 256]
diff --git a/MAINTAINERS b/MAINTAINERS
index 4ff9c12b1313..583c6c93b6f3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6128,7 +6128,9 @@ IIO DIGITAL POTENTIOMETER DAC
M: Peter Rosin <peda@axentia.se>
L: linux-iio@vger.kernel.org
S: Maintained
+F: Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
F: Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
+F: drivers/iio/dac/dpot-dac.c
IIO SUBSYSTEM AND DRIVERS
M: Jonathan Cameron <jic23@kernel.org>
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 120b24478469..d3084028905b 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -200,6 +200,16 @@ config AD8801
To compile this driver as a module choose M here: the module will be called
ad8801.
+config DPOT_DAC
+ tristate "DAC emulation using a DPOT"
+ depends on OF
+ help
+ Say yes here to build support for DAC emulation using a digital
+ potentiometer.
+
+ To compile this driver as a module, choose M here: the module will be
+ called dpot-dac.
+
config LPC18XX_DAC
tristate "NXP LPC18xx DAC driver"
depends on ARCH_LPC18XX || COMPILE_TEST
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 27642bbf75f2..f01bf4a99867 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_AD5686) += ad5686.o
obj-$(CONFIG_AD7303) += ad7303.o
obj-$(CONFIG_AD8801) += ad8801.o
obj-$(CONFIG_CIO_DAC) += cio-dac.o
+obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
obj-$(CONFIG_M62332) += m62332.o
obj-$(CONFIG_MAX517) += max517.o
diff --git a/drivers/iio/dac/dpot-dac.c b/drivers/iio/dac/dpot-dac.c
new file mode 100644
index 000000000000..eb1f0509a3ec
--- /dev/null
+++ b/drivers/iio/dac/dpot-dac.c
@@ -0,0 +1,266 @@
+/*
+ * IIO DAC emulation driver using a digital potentiometer
+ *
+ * Copyright (C) 2016 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/*
+ * It is assumed that the dpot is used as a voltage divider between the
+ * current dpot wiper setting and the maximum resistance of the dpot. The
+ * divided voltage is provided by a vref regulator.
+ *
+ * .------.
+ * .-----------. | |
+ * | vref |--' .---.
+ * | regulator |--. | |
+ * '-----------' | | d |
+ * | | p |
+ * | | o | wiper
+ * | | t |<---------+
+ * | | |
+ * | '---' dac output voltage
+ * | |
+ * '------+------------+
+ */
+
+#include <linux/err.h>
+#include <linux/iio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+
+struct dpot_dac {
+ struct regulator *vref;
+ struct iio_channel *dpot;
+ u32 max_ohms;
+};
+
+static const struct iio_chan_spec dpot_dac_iio_channel = {
+ .type = IIO_VOLTAGE,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
+ | BIT(IIO_CHAN_INFO_SCALE),
+ .info_mask_separate_available = BIT(IIO_CHAN_INFO_RAW),
+ .output = 1,
+ .indexed = 1,
+};
+
+static int dpot_dac_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct dpot_dac *dac = iio_priv(indio_dev);
+ int ret;
+ unsigned long long tmp;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ return iio_read_channel_raw(dac->dpot, val);
+
+ case IIO_CHAN_INFO_SCALE:
+ ret = iio_read_channel_scale(dac->dpot, val, val2);
+ switch (ret) {
+ case IIO_VAL_FRACTIONAL_LOG2:
+ tmp = *val * 1000000000LL;
+ do_div(tmp, dac->max_ohms);
+ tmp *= regulator_get_voltage(dac->vref) / 1000;
+ do_div(tmp, 1000000000LL);
+ *val = tmp;
+ return ret;
+ case IIO_VAL_INT:
+ /*
+ * Convert integer scale to fractional scale by
+ * setting the denominator (val2) to one...
+ */
+ *val2 = 1;
+ ret = IIO_VAL_FRACTIONAL;
+ /* ...and fall through. */
+ case IIO_VAL_FRACTIONAL:
+ *val *= regulator_get_voltage(dac->vref) / 1000;
+ *val2 *= dac->max_ohms;
+ break;
+ }
+
+ return ret;
+ }
+
+ return -EINVAL;
+}
+
+static int dpot_dac_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length,
+ long mask)
+{
+ struct dpot_dac *dac = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ *type = IIO_VAL_INT;
+ return iio_read_avail_channel_raw(dac->dpot, vals, length);
+ }
+
+ return -EINVAL;
+}
+
+static int dpot_dac_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct dpot_dac *dac = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ return iio_write_channel_raw(dac->dpot, val);
+ }
+
+ return -EINVAL;
+}
+
+static const struct iio_info dpot_dac_info = {
+ .read_raw = dpot_dac_read_raw,
+ .read_avail = dpot_dac_read_avail,
+ .write_raw = dpot_dac_write_raw,
+ .driver_module = THIS_MODULE,
+};
+
+static int dpot_dac_channel_max_ohms(struct iio_dev *indio_dev)
+{
+ struct device *dev = &indio_dev->dev;
+ struct dpot_dac *dac = iio_priv(indio_dev);
+ unsigned long long tmp;
+ int ret;
+ int val;
+ int val2;
+ int max;
+
+ ret = iio_read_max_channel_raw(dac->dpot, &max);
+ if (ret < 0) {
+ dev_err(dev, "dpot does not indicate its raw maximum value\n");
+ return ret;
+ }
+
+ switch (iio_read_channel_scale(dac->dpot, &val, &val2)) {
+ case IIO_VAL_INT:
+ return max * val;
+ case IIO_VAL_FRACTIONAL:
+ tmp = (unsigned long long)max * val;
+ do_div(tmp, val2);
+ return tmp;
+ case IIO_VAL_FRACTIONAL_LOG2:
+ tmp = val * 1000000000LL * max >> val2;
+ do_div(tmp, 1000000000LL);
+ return tmp;
+ default:
+ dev_err(dev, "dpot has a scale that is too weird\n");
+ }
+
+ return -EINVAL;
+}
+
+static int dpot_dac_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct iio_dev *indio_dev;
+ struct dpot_dac *dac;
+ enum iio_chan_type type;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*dac));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, indio_dev);
+ dac = iio_priv(indio_dev);
+
+ indio_dev->name = dev_name(dev);
+ indio_dev->dev.parent = dev;
+ indio_dev->info = &dpot_dac_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = &dpot_dac_iio_channel;
+ indio_dev->num_channels = 1;
+
+ dac->vref = devm_regulator_get(dev, "vref");
+ if (IS_ERR(dac->vref)) {
+ if (PTR_ERR(dac->dpot) != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "failed to get vref regulator\n");
+ return PTR_ERR(dac->vref);
+ }
+
+ dac->dpot = devm_iio_channel_get(dev, "dpot");
+ if (IS_ERR(dac->dpot)) {
+ if (PTR_ERR(dac->dpot) != -EPROBE_DEFER)
+ dev_err(dev, "failed to get dpot input channel\n");
+ return PTR_ERR(dac->dpot);
+ }
+
+ ret = iio_get_channel_type(dac->dpot, &type);
+ if (ret < 0)
+ return ret;
+
+ if (type != IIO_RESISTANCE) {
+ dev_err(dev, "dpot is of the wrong type\n");
+ return -EINVAL;
+ }
+
+ ret = dpot_dac_channel_max_ohms(indio_dev);
+ if (ret < 0)
+ return ret;
+ dac->max_ohms = ret;
+
+ ret = regulator_enable(dac->vref);
+ if (ret) {
+ dev_err(dev, "failed to enable the vref regulator\n");
+ return ret;
+ }
+
+ ret = iio_device_register(indio_dev);
+ if (ret) {
+ dev_err(dev, "failed to register iio device\n");
+ goto disable_reg;
+ }
+
+ return 0;
+
+disable_reg:
+ regulator_disable(dac->vref);
+ return ret;
+}
+
+static int dpot_dac_remove(struct platform_device *pdev)
+{
+ struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+ struct dpot_dac *dac = iio_priv(indio_dev);
+
+ iio_device_unregister(indio_dev);
+ regulator_disable(dac->vref);
+
+ return 0;
+}
+
+static const struct of_device_id dpot_dac_match[] = {
+ { .compatible = "dpot-dac" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, dpot_dac_match);
+
+static struct platform_driver dpot_dac_driver = {
+ .probe = dpot_dac_probe,
+ .remove = dpot_dac_remove,
+ .driver = {
+ .name = "iio-dpot-dac",
+ .of_match_table = dpot_dac_match,
+ },
+};
+module_platform_driver(dpot_dac_driver);
+
+MODULE_DESCRIPTION("DAC emulation driver using a digital potentiometer");
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
+MODULE_LICENSE("GPL v2");
--
2.1.4
^ permalink raw reply related
* [PATCH v4 7/8] dt-bindings: iio: document envelope-detector bindings
From: Peter Rosin @ 2016-11-08 11:58 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Daniel Baluta,
Slawomir Stepien, Thomas Gleixner, linux-iio, devicetree
In-Reply-To: <1478606339-31253-1-git-send-email-peda@axentia.se>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
.../bindings/iio/adc/envelope-detector.txt | 54 ++++++++++++++++++++++
MAINTAINERS | 6 +++
2 files changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt b/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
new file mode 100644
index 000000000000..27544bdd4478
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
@@ -0,0 +1,54 @@
+Bindings for ADC envelope detector using a DAC and a comparator
+
+The DAC is used to find the peak level of an alternating voltage input
+signal by a binary search using the output of a comparator wired to
+an interrupt pin. Like so:
+ _
+ | \
+ input +------>-------|+ \
+ | \
+ .-------. | }---.
+ | | | / |
+ | dac|-->--|- / |
+ | | |_/ |
+ | | |
+ | | |
+ | irq|------<-------'
+ | |
+ '-------'
+
+Required properties:
+- compatible: Should be "axentia,tse850-envelope-detector"
+- io-channels: Channel node of the dac to be used for comparator input.
+- io-channel-names: Should be "dac".
+- interrupt specification for one client interrupt,
+ see ../../interrupt-controller/interrupts.txt for details.
+- interrupt-names: Should be "comp".
+
+Example:
+
+ &i2c {
+ dpot: mcp4651-104@28 {
+ compatible = "microchip,mcp4651-104";
+ reg = <0x28>;
+ #io-channel-cells = <1>;
+ };
+ };
+
+ dac: dac {
+ compatible = "dpot-dac";
+ vref-supply = <®_3v3>;
+ io-channels = <&dpot 0>;
+ io-channel-names = "dpot";
+ #io-channel-cells = <1>;
+ };
+
+ envelope-detector {
+ compatible = "axentia,tse850-envelope-detector";
+ io-channels = <&dac 0>;
+ io-channel-names = "dac";
+
+ interrupt-parent = <&gpio>;
+ interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-names = "comp";
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 583c6c93b6f3..0e13066ca3a2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6132,6 +6132,12 @@ F: Documentation/ABI/testing/sysfs-bus-iio-dac-dpot-dac
F: Documentation/devicetree/bindings/iio/dac/dpot-dac.txt
F: drivers/iio/dac/dpot-dac.c
+IIO ENVELOPE DETECTOR
+M: Peter Rosin <peda@axentia.se>
+L: linux-iio@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
+
IIO SUBSYSTEM AND DRIVERS
M: Jonathan Cameron <jic23@kernel.org>
R: Hartmut Knaack <knaack.h@gmx.de>
--
2.1.4
^ permalink raw reply related
* [PATCH v4 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator
From: Peter Rosin @ 2016-11-08 11:58 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Rob Herring, Mark Rutland, Daniel Baluta,
Slawomir Stepien, Thomas Gleixner, linux-iio, devicetree
In-Reply-To: <1478606339-31253-1-git-send-email-peda@axentia.se>
The DAC is used to find the peak level of an alternating voltage input
signal by a binary search using the output of a comparator wired to
an interrupt pin. Like so:
_
| \
input +------>-------|+ \
| \
.-------. | }---.
| | | / |
| dac|-->--|- / |
| | |_/ |
| | |
| | |
| irq|------<-------'
| |
'-------'
Signed-off-by: Peter Rosin <peda@axentia.se>
---
.../testing/sysfs-bus-iio-adc-envelope-detector | 36 ++
MAINTAINERS | 2 +
drivers/iio/adc/Kconfig | 10 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/envelope-detector.c | 422 +++++++++++++++++++++
5 files changed, 471 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
create mode 100644 drivers/iio/adc/envelope-detector.c
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector b/Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
new file mode 100644
index 000000000000..2071f9bcfaa5
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
@@ -0,0 +1,36 @@
+What: /sys/bus/iio/devices/iio:deviceX/in_altvoltageY_invert
+Date: October 2016
+KernelVersion: 4.9
+Contact: Peter Rosin <peda@axentia.se>
+Description:
+ The DAC is used to find the peak level of an alternating
+ voltage input signal by a binary search using the output
+ of a comparator wired to an interrupt pin. Like so:
+ _
+ | \
+ input +------>-------|+ \
+ | \
+ .-------. | }---.
+ | | | / |
+ | dac|-->--|- / |
+ | | |_/ |
+ | | |
+ | | |
+ | irq|------<-------'
+ | |
+ '-------'
+ The boolean invert attribute (0/1) should be set when the
+ input signal is centered around the maximum value of the
+ dac instead of zero. The envelope detector will search
+ from below in this case and will also invert the result.
+ The edge/level of the interrupt is also switched to its
+ opposite value.
+
+What: /sys/bus/iio/devices/iio:deviceX/in_altvoltageY_compare_interval
+Date: October 2016
+KernelVersion: 4.9
+Contact: Peter Rosin <peda@axentia.se>
+Description:
+ Number of milliseconds to wait for the comparator in each
+ step of the binary search for the input peak level. Needs
+ to relate to the frequency of the input signal.
diff --git a/MAINTAINERS b/MAINTAINERS
index 0e13066ca3a2..539b20baf791 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6136,7 +6136,9 @@ IIO ENVELOPE DETECTOR
M: Peter Rosin <peda@axentia.se>
L: linux-iio@vger.kernel.org
S: Maintained
+F: Documentation/ABI/testing/sysfs-bus-iio-adc-envelope-detector
F: Documentation/devicetree/bindings/iio/adc/envelope-detector.txt
+F: drivers/iio/adc/envelope-detector.c
IIO SUBSYSTEM AND DRIVERS
M: Jonathan Cameron <jic23@kernel.org>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 99c051490eff..de4030894f23 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -195,6 +195,16 @@ config DA9150_GPADC
To compile this driver as a module, choose M here: the module will be
called berlin2-adc.
+config ENVELOPE_DETECTOR
+ tristate "Envelope detector using a DAC and a comparator"
+ depends on OF
+ help
+ Say yes here to build support for an envelope detector using a DAC
+ and a comparator.
+
+ To compile this driver as a module, choose M here: the module will be
+ called envelope-detector.
+
config EXYNOS_ADC
tristate "Exynos ADC driver support"
depends on ARCH_EXYNOS || ARCH_S3C24XX || ARCH_S3C64XX || (OF && COMPILE_TEST)
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 7a40c04c311f..0d773c6a0578 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_BCM_IPROC_ADC) += bcm_iproc_adc.o
obj-$(CONFIG_BERLIN2_ADC) += berlin2-adc.o
obj-$(CONFIG_CC10001_ADC) += cc10001_adc.o
obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o
+obj-$(CONFIG_ENVELOPE_DETECTOR) += envelope-detector.o
obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
obj-$(CONFIG_FSL_MX25_ADC) += fsl-imx25-gcq.o
obj-$(CONFIG_HI8435) += hi8435.o
diff --git a/drivers/iio/adc/envelope-detector.c b/drivers/iio/adc/envelope-detector.c
new file mode 100644
index 000000000000..fef15c0d7c9c
--- /dev/null
+++ b/drivers/iio/adc/envelope-detector.c
@@ -0,0 +1,422 @@
+/*
+ * Driver for an envelope detector using a DAC and a comparator
+ *
+ * Copyright (C) 2016 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/*
+ * The DAC is used to find the peak level of an alternating voltage input
+ * signal by a binary search using the output of a comparator wired to
+ * an interrupt pin. Like so:
+ * _
+ * | \
+ * input +------>-------|+ \
+ * | \
+ * .-------. | }---.
+ * | | | / |
+ * | dac|-->--|- / |
+ * | | |_/ |
+ * | | |
+ * | | |
+ * | irq|------<-------'
+ * | |
+ * '-------'
+ */
+
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/iio/consumer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/workqueue.h>
+
+struct envelope {
+ spinlock_t comp_lock; /* protects comp */
+ int comp;
+
+ struct mutex read_lock; /* protects everything else */
+
+ int comp_irq;
+ u32 comp_irq_trigger;
+ u32 comp_irq_trigger_inv;
+
+ struct iio_channel *dac;
+ struct delayed_work comp_timeout;
+
+ unsigned int comp_interval;
+ bool invert;
+ u32 dac_max;
+
+ int high;
+ int level;
+ int low;
+
+ struct completion done;
+};
+
+/*
+ * The envelope_detector_comp_latch function works together with the compare
+ * interrupt service routine below (envelope_detector_comp_isr) as a latch
+ * (one-bit memory) for if the interrupt has triggered since last calling
+ * this function.
+ * The ..._comp_isr function disables the interrupt so that the cpu does not
+ * need to service a possible interrupt flood from the comparator when no-one
+ * cares anyway, and this ..._comp_latch function reenables them again if
+ * needed.
+ */
+static int envelope_detector_comp_latch(struct envelope *env)
+{
+ int comp;
+
+ spin_lock_irq(&env->comp_lock);
+ comp = env->comp;
+ env->comp = 0;
+ spin_unlock_irq(&env->comp_lock);
+
+ if (!comp)
+ return 0;
+
+ /*
+ * The irq was disabled, and is reenabled just now.
+ * But there might have been a pending irq that
+ * happened while the irq was disabled that fires
+ * just as the irq is reenabled. That is not what
+ * is desired.
+ */
+ enable_irq(env->comp_irq);
+
+ /* So, synchronize this possibly pending irq... */
+ synchronize_irq(env->comp_irq);
+
+ /* ...and redo the whole dance. */
+ spin_lock_irq(&env->comp_lock);
+ comp = env->comp;
+ env->comp = 0;
+ spin_unlock_irq(&env->comp_lock);
+
+ if (comp)
+ enable_irq(env->comp_irq);
+
+ return 1;
+}
+
+static irqreturn_t envelope_detector_comp_isr(int irq, void *ctx)
+{
+ struct envelope *env = ctx;
+
+ spin_lock(&env->comp_lock);
+ env->comp = 1;
+ disable_irq_nosync(env->comp_irq);
+ spin_unlock(&env->comp_lock);
+
+ return IRQ_HANDLED;
+}
+
+static void envelope_detector_setup_compare(struct envelope *env)
+{
+ int ret;
+
+ /*
+ * Do a binary search for the peak input level, and stop
+ * when that level is "trapped" between two adjacent DAC
+ * values.
+ * When invert is active, use the midpoint floor so that
+ * env->level ends up as env->low when the termination
+ * criteria below is fulfilled, and use the midpoint
+ * ceiling when invert is not active so that env->level
+ * ends up as env->high in that case.
+ */
+ env->level = (env->high + env->low + !env->invert) / 2;
+
+ if (env->high == env->low + 1) {
+ complete(&env->done);
+ return;
+ }
+
+ /* Set a "safe" DAC level (if there is such a thing)... */
+ ret = iio_write_channel_raw(env->dac, env->invert ? 0 : env->dac_max);
+ if (ret < 0)
+ goto err;
+
+ /* ...clear the comparison result... */
+ envelope_detector_comp_latch(env);
+
+ /* ...set the real DAC level... */
+ ret = iio_write_channel_raw(env->dac, env->level);
+ if (ret < 0)
+ goto err;
+
+ /* ...and wait for a bit to see if the latch catches anything. */
+ schedule_delayed_work(&env->comp_timeout,
+ msecs_to_jiffies(env->comp_interval));
+ return;
+
+err:
+ env->level = ret;
+ complete(&env->done);
+}
+
+static void envelope_detector_timeout(struct work_struct *work)
+{
+ struct envelope *env = container_of(work, struct envelope,
+ comp_timeout.work);
+
+ /* Adjust low/high depending on the latch content... */
+ if (!envelope_detector_comp_latch(env) ^ !env->invert)
+ env->low = env->level;
+ else
+ env->high = env->level;
+
+ /* ...and continue the search. */
+ envelope_detector_setup_compare(env);
+}
+
+static int envelope_detector_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct envelope *env = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ /*
+ * When invert is active, start with high=max+1 and low=0
+ * since we will end up with the low value when the
+ * termination criteria is fulfilled (rounding down). And
+ * start with high=max and low=-1 when invert is not active
+ * since we will end up with the high value in that case.
+ * This ensures that the returned value in both cases are
+ * in the same range as the DAC and is a value that has not
+ * triggered the comparator.
+ */
+ mutex_lock(&env->read_lock);
+ env->high = env->dac_max + env->invert;
+ env->low = -1 + env->invert;
+ envelope_detector_setup_compare(env);
+ wait_for_completion(&env->done);
+ if (env->level < 0) {
+ ret = env->level;
+ goto err_unlock;
+ }
+ *val = env->invert ? env->dac_max - env->level : env->level;
+ mutex_unlock(&env->read_lock);
+
+ return IIO_VAL_INT;
+
+ case IIO_CHAN_INFO_SCALE:
+ return iio_read_channel_scale(env->dac, val, val2);
+ }
+
+ return -EINVAL;
+
+err_unlock:
+ mutex_unlock(&env->read_lock);
+ return ret;
+}
+
+static ssize_t envelope_show_invert(struct iio_dev *indio_dev,
+ uintptr_t private,
+ struct iio_chan_spec const *ch, char *buf)
+{
+ struct envelope *env = iio_priv(indio_dev);
+
+ return sprintf(buf, "%u\n", env->invert);
+}
+
+static ssize_t envelope_store_invert(struct iio_dev *indio_dev,
+ uintptr_t private,
+ struct iio_chan_spec const *ch,
+ const char *buf, size_t len)
+{
+ struct envelope *env = iio_priv(indio_dev);
+ unsigned long invert;
+ int ret;
+ u32 trigger;
+
+ ret = kstrtoul(buf, 0, &invert);
+ if (ret < 0)
+ return ret;
+ if (invert > 1)
+ return -EINVAL;
+
+ trigger = invert ? env->comp_irq_trigger_inv : env->comp_irq_trigger;
+
+ mutex_lock(&env->read_lock);
+ if (invert != env->invert)
+ ret = irq_set_irq_type(env->comp_irq, trigger);
+ if (!ret) {
+ env->invert = invert;
+ ret = len;
+ }
+ mutex_unlock(&env->read_lock);
+
+ return ret;
+}
+
+static ssize_t envelope_show_comp_interval(struct iio_dev *indio_dev,
+ uintptr_t private,
+ struct iio_chan_spec const *ch,
+ char *buf)
+{
+ struct envelope *env = iio_priv(indio_dev);
+
+ return sprintf(buf, "%u\n", env->comp_interval);
+}
+
+static ssize_t envelope_store_comp_interval(struct iio_dev *indio_dev,
+ uintptr_t private,
+ struct iio_chan_spec const *ch,
+ const char *buf, size_t len)
+{
+ struct envelope *env = iio_priv(indio_dev);
+ unsigned long interval;
+ int ret;
+
+ ret = kstrtoul(buf, 0, &interval);
+ if (ret < 0)
+ return ret;
+ if (interval > 1000)
+ return -EINVAL;
+
+ mutex_lock(&env->read_lock);
+ env->comp_interval = interval;
+ mutex_unlock(&env->read_lock);
+
+ return len;
+}
+
+static const struct iio_chan_spec_ext_info envelope_detector_ext_info[] = {
+ { .name = "invert",
+ .read = envelope_show_invert,
+ .write = envelope_store_invert, },
+ { .name = "compare_interval",
+ .read = envelope_show_comp_interval,
+ .write = envelope_store_comp_interval, },
+ { /* sentinel */ }
+};
+
+static const struct iio_chan_spec envelope_detector_iio_channel = {
+ .type = IIO_ALTVOLTAGE,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
+ | BIT(IIO_CHAN_INFO_SCALE),
+ .ext_info = envelope_detector_ext_info,
+ .indexed = 1,
+};
+
+static const struct iio_info envelope_detector_info = {
+ .read_raw = &envelope_detector_read_raw,
+ .driver_module = THIS_MODULE,
+};
+
+static int envelope_detector_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct iio_dev *indio_dev;
+ struct envelope *env;
+ enum iio_chan_type type;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*env));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, indio_dev);
+ env = iio_priv(indio_dev);
+ env->comp_interval = 50; /* some sensible default? */
+
+ spin_lock_init(&env->comp_lock);
+ mutex_init(&env->read_lock);
+ init_completion(&env->done);
+ INIT_DELAYED_WORK(&env->comp_timeout, envelope_detector_timeout);
+
+ indio_dev->name = dev_name(dev);
+ indio_dev->dev.parent = dev;
+ indio_dev->dev.of_node = dev->of_node;
+ indio_dev->info = &envelope_detector_info;
+ indio_dev->channels = &envelope_detector_iio_channel;
+ indio_dev->num_channels = 1;
+
+ env->dac = devm_iio_channel_get(dev, "dac");
+ if (IS_ERR(env->dac)) {
+ if (PTR_ERR(env->dac) != -EPROBE_DEFER)
+ dev_err(dev, "failed to get dac input channel\n");
+ return PTR_ERR(env->dac);
+ }
+
+ env->comp_irq = platform_get_irq_byname(pdev, "comp");
+ if (env->comp_irq < 0) {
+ if (env->comp_irq != -EPROBE_DEFER)
+ dev_err(dev, "failed to get compare interrupt\n");
+ return env->comp_irq;
+ }
+
+ ret = devm_request_irq(dev, env->comp_irq, envelope_detector_comp_isr,
+ 0, "envelope-detector", env);
+ if (ret) {
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "failed to request interrupt\n");
+ return ret;
+ }
+ env->comp_irq_trigger = irq_get_trigger_type(env->comp_irq);
+ if (env->comp_irq_trigger & IRQF_TRIGGER_RISING)
+ env->comp_irq_trigger_inv |= IRQF_TRIGGER_FALLING;
+ if (env->comp_irq_trigger & IRQF_TRIGGER_FALLING)
+ env->comp_irq_trigger_inv |= IRQF_TRIGGER_RISING;
+ if (env->comp_irq_trigger & IRQF_TRIGGER_HIGH)
+ env->comp_irq_trigger_inv |= IRQF_TRIGGER_LOW;
+ if (env->comp_irq_trigger & IRQF_TRIGGER_LOW)
+ env->comp_irq_trigger_inv |= IRQF_TRIGGER_HIGH;
+
+ ret = iio_get_channel_type(env->dac, &type);
+ if (ret < 0)
+ return ret;
+
+ if (type != IIO_VOLTAGE) {
+ dev_err(dev, "dac is of the wrong type\n");
+ return -EINVAL;
+ }
+
+ ret = iio_read_max_channel_raw(env->dac, &env->dac_max);
+ if (ret < 0) {
+ dev_err(dev, "dac does not indicate its raw maximum value\n");
+ return ret;
+ }
+
+ return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id envelope_detector_match[] = {
+ { .compatible = "axentia,tse850-envelope-detector", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, envelope_detector_match);
+
+static struct platform_driver envelope_detector_driver = {
+ .probe = envelope_detector_probe,
+ .driver = {
+ .name = "iio-envelope-detector",
+ .of_match_table = envelope_detector_match,
+ },
+};
+module_platform_driver(envelope_detector_driver);
+
+MODULE_DESCRIPTION("Envelope detector using a DAC and a comparator");
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
+MODULE_LICENSE("GPL v2");
--
2.1.4
^ permalink raw reply related
* Re: [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: Mark Rutland @ 2016-11-08 12:03 UTC (permalink / raw)
To: zhichang.yuan
Cc: catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, bhelgaas-hpIqsD4AKlfQT0dZR+AlfA,
olof-nZhT3qVonbNeoWH0uzbU5w, arnd-r2nGTMty4D4,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
lorenzo.pieralisi-5wv7dgnIgG8,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linuxarm-hv44wF8Li93QT0dZR+AlfA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA, minyard-HInyCGIudOg,
benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r, liviu.dudau-5wv7dgnIgG8,
zourongrong-Re5JQEeQqe8AvxtiuMwx3w,
john.garry-hv44wF8Li93QT0dZR+AlfA,
gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA,
zhichang.yuan02-Re5JQEeQqe8AvxtiuMwx3w, kantyzc-9Onoh4P/yGk,
xuwei5-C8/M+/jPZTeaMJb+Lgu22Q, marc.zyngier-5wv7dgnIgG8
In-Reply-To: <1478576829-112707-2-git-send-email-yuanzhichang-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
On Tue, Nov 08, 2016 at 11:47:07AM +0800, zhichang.yuan wrote:
> For arm64, there is no I/O space as other architectural platforms, such as
> X86. Most I/O accesses are achieved based on MMIO. But for some arm64 SoCs,
> such as Hip06, when accessing some legacy ISA devices connected to LPC, those
> known port addresses are used to control the corresponding target devices, for
> example, 0x2f8 is for UART, 0xe4 is for ipmi-bt. It is different from the
> normal MMIO mode in using.
This has nothing to do with arm64. Hardware with this kind of indirect
bus access could be integrated with a variety of CPU architectures. It
simply hasn't been, yet.
> To drive these devices, this patch introduces a method named indirect-IO.
> In this method the in/out pair in arch/arm64/include/asm/io.h will be
> redefined. When upper layer drivers call in/out with those known legacy port
> addresses to access the peripherals, the hooking functions corrresponding to
> those target peripherals will be called. Through this way, those upper layer
> drivers which depend on in/out can run on Hip06 without any changes.
As above, this has nothing to do with arm64, and as such, should live in
generic code, exactly as we would do if we had higher-level ISA
accessor ops.
Regardless, given the multi-instance case, I don't think this is
sufficient in general (and I think we need higher-level ISA accessors
to handle the indirection).
[...]
> diff --git a/arch/arm64/include/asm/extio.h b/arch/arm64/include/asm/extio.h
> new file mode 100644
> index 0000000..6ae0787
> --- /dev/null
> +++ b/arch/arm64/include/asm/extio.h
> +#ifndef __LINUX_EXTIO_H
> +#define __LINUX_EXTIO_H
This doesn't match the file naming, __ASM_EXTIO_H would be consistent
with other arm64 headers.
> +
> +struct extio_ops {
> + unsigned long start;/* inclusive, sys io addr */
> + unsigned long end;/* inclusive, sys io addr */
Please put whitespace before inline comments.
[...]
> +type in##bw(unsigned long addr) \
> +{ \
> + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> + arm64_extio_ops->end < addr) \
> + return read##bw(PCI_IOBASE + addr); \
> + return arm64_extio_ops->pfin ? \
> + arm64_extio_ops->pfin(arm64_extio_ops->devpara, \
> + addr, sizeof(type)) : -1; \
> +} \
> + \
> +void out##bw(type value, unsigned long addr) \
> +{ \
> + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> + arm64_extio_ops->end < addr) \
> + write##bw(value, PCI_IOBASE + addr); \
> + else \
> + if (arm64_extio_ops->pfout) \
> + arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> + addr, value, sizeof(type)); \
> +} \
> + \
> +void ins##bw(unsigned long addr, void *buffer, unsigned int count) \
> +{ \
> + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> + arm64_extio_ops->end < addr) \
> + reads##bw(PCI_IOBASE + addr, buffer, count); \
> + else \
> + if (arm64_extio_ops->pfins) \
> + arm64_extio_ops->pfins(arm64_extio_ops->devpara,\
> + addr, buffer, sizeof(type), count); \
> +} \
> + \
> +void outs##bw(unsigned long addr, const void *buffer, unsigned int count) \
> +{ \
> + if (!arm64_extio_ops || arm64_extio_ops->start > addr || \
> + arm64_extio_ops->end < addr) \
> + writes##bw(PCI_IOBASE + addr, buffer, count); \
> + else \
> + if (arm64_extio_ops->pfouts) \
> + arm64_extio_ops->pfouts(arm64_extio_ops->devpara,\
> + addr, buffer, sizeof(type), count); \
> +}
> +
So all PCI I/O will be slowed down by irrelevant checks when this is
enabled?
[...]
> +static inline void arm64_set_extops(struct extio_ops *ops)
> +{
> + if (ops)
> + WRITE_ONCE(arm64_extio_ops, ops);
> +}
Why WRITE_ONCE()?
Is this not protected/propagated by some synchronisation mechanism?
WRITE_ONCE() is not sufficient to ensure that this is consistently
observed by readers, and regardless, I don't see READ_ONCE() anywhere in
this patch.
This looks very suspicious.
Thanks,
Mark.
--
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 v6 06/14] mmc: sdhci-msm: Add get_min_clock() and get_max_clock() callback
From: Adrian Hunter @ 2016-11-08 12:15 UTC (permalink / raw)
To: Ritesh Harjani, ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
shawn.lin-TNX95d0MmH7DzftRWevZcw, sboyd-sgV2jX0FEOL9JmXXK+q4OQ
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
david.brown-QSEj5FYQhm4dnm+yROfE0A,
andy.gross-QSEj5FYQhm4dnm+yROfE0A,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
georgi.djakov-QSEj5FYQhm4dnm+yROfE0A,
alex.lemberg-XdAiOPVOjttBDgjK7y7TUQ,
mateusz.nowak-ral2JQCrhuEAvxtiuMwx3w,
Yuliy.Izrailov-XdAiOPVOjttBDgjK7y7TUQ,
asutoshd-sgV2jX0FEOL9JmXXK+q4OQ, kdorfman-sgV2jX0FEOL9JmXXK+q4OQ,
david.griego-QSEj5FYQhm4dnm+yROfE0A,
stummala-sgV2jX0FEOL9JmXXK+q4OQ, venkatg-sgV2jX0FEOL9JmXXK+q4OQ,
rnayak-sgV2jX0FEOL9JmXXK+q4OQ,
pramod.gurav-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1478517877-23733-7-git-send-email-riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 07/11/16 13:24, Ritesh Harjani wrote:
> This add get_min_clock() and get_max_clock() callback
> for sdhci-msm. sdhci-msm min/max clocks may be different
> hence implement these callbacks.
>
> Signed-off-by: Sahitya Tummala <stummala-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Acked-by: Adrian Hunter <adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> drivers/mmc/host/sdhci-msm.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 32b0b79..66ca444 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -68,6 +68,7 @@
> #define CORE_VENDOR_SPEC_CAPABILITIES0 0x11c
>
> #define TCXO_FREQ 19200000
> +#define SDHCI_MSM_MIN_CLOCK 400000
>
> #define CDR_SELEXT_SHIFT 20
> #define CDR_SELEXT_MASK (0xf << CDR_SELEXT_SHIFT)
> @@ -559,6 +560,19 @@ static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> +static unsigned int sdhci_msm_get_max_clock(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> +
> + return clk_round_rate(msm_host->clk, ULONG_MAX);
> +}
> +
> +static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
> +{
> + return SDHCI_MSM_MIN_CLOCK;
> +}
> +
> static const struct of_device_id sdhci_msm_dt_match[] = {
> { .compatible = "qcom,sdhci-msm-v4" },
> {},
> @@ -570,6 +584,8 @@ static irqreturn_t sdhci_msm_pwr_irq(int irq, void *data)
> .platform_execute_tuning = sdhci_msm_execute_tuning,
> .reset = sdhci_reset,
> .set_clock = sdhci_set_clock,
> + .get_min_clock = sdhci_msm_get_min_clock,
> + .get_max_clock = sdhci_msm_get_max_clock,
> .set_bus_width = sdhci_set_bus_width,
> .set_uhs_signaling = sdhci_msm_set_uhs_signaling,
> .voltage_switch = sdhci_msm_voltage_switch,
>
--
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 v6 08/14] mmc: sdhci-msm: Implement set_clock callback for sdhci-msm
From: Adrian Hunter @ 2016-11-08 12:16 UTC (permalink / raw)
To: Ritesh Harjani, ulf.hansson, linux-mmc, shawn.lin, sboyd
Cc: devicetree, linux-clk, david.brown, andy.gross, linux-arm-msm,
georgi.djakov, alex.lemberg, mateusz.nowak, Yuliy.Izrailov,
asutoshd, kdorfman, david.griego, stummala, venkatg, rnayak,
pramod.gurav
In-Reply-To: <1478517877-23733-9-git-send-email-riteshh@codeaurora.org>
On 07/11/16 13:24, Ritesh Harjani wrote:
> sdhci-msm controller may have different clk-rates for each
> bus speed mode. Thus implement set_clock callback for
> sdhci-msm driver.
>
> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
^ permalink raw reply
* Re: [PATCH v6 09/14] mmc: sdhci-msm: Add clock changes for DDR mode.
From: Adrian Hunter @ 2016-11-08 12:20 UTC (permalink / raw)
To: Ritesh Harjani, ulf.hansson, linux-mmc, shawn.lin, sboyd
Cc: devicetree, linux-clk, david.brown, andy.gross, linux-arm-msm,
georgi.djakov, alex.lemberg, mateusz.nowak, Yuliy.Izrailov,
asutoshd, kdorfman, david.griego, stummala, venkatg, rnayak,
pramod.gurav
In-Reply-To: <1478517877-23733-10-git-send-email-riteshh@codeaurora.org>
On 07/11/16 13:24, Ritesh Harjani wrote:
> SDHC MSM controller need 2x clock for MCLK at GCC.
> Hence make required changes to have 2x clock for
> DDR timing modes.
>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/sdhci-msm.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index ff0915b..220567c 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -633,6 +633,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> {
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> + struct mmc_ios curr_ios = host->mmc->ios;
> int rc;
>
> if (!clock) {
> @@ -641,11 +642,23 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> }
>
> spin_unlock_irq(&host->lock);
> + /*
> + * The SDHC requires internal clock frequency to be double the
> + * actual clock that will be set for DDR mode. The controller
> + * uses the faster clock(100/400MHz) for some of its parts and
> + * send the actual required clock (50/200MHz) to the card.
> + */
> + if ((curr_ios.timing == MMC_TIMING_UHS_DDR50) ||
> + (curr_ios.timing == MMC_TIMING_MMC_DDR52) ||
> + (curr_ios.timing == MMC_TIMING_MMC_HS400))
> + clock *= 2;
> +
> if (clock != msm_host->clk_rate) {
> rc = clk_set_rate(msm_host->clk, clock);
> if (rc) {
> - pr_err("%s: Failed to set clock at rate %u\n",
> - mmc_hostname(host->mmc), clock);
> + pr_err("%s: Failed to set clock at rate %u at timing %d\n",
> + mmc_hostname(host->mmc), clock,
> + curr_ios.timing);
> spin_lock_irq(&host->lock);
> goto out;
> }
>
^ permalink raw reply
* Re: [PATCH 4/6] ARM: dts: add basic support for Rockchip RK1108 SOC
From: Andy Yan @ 2016-11-08 12:31 UTC (permalink / raw)
To: Heiko Stuebner
Cc: elaine.zhang, mturquette, linux-rockchip, devicetree, robh+dt,
mark.rutland, linux, linux-clk, linux-arm-kernel, linux-kernel
In-Reply-To: <1788707.rfxhNfegGu@phil>
Hi Heiko:
On 2016年11月04日 16:00, Heiko Stuebner wrote:
> Am Donnerstag, 3. November 2016, 20:40:48 CET schrieb Andy Yan:
>> RK1108 is embedded with an ARM Cortex-A7 single core and a DSP core.
>> It is designed for varies application scenario such as car DVR, sports
>> DV, secure camera and UAV camera.
>>
>> This patch add basic support for it with DMAC / UART / CRU / pinctrl
>> enabled.
>>
>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>> ---
>>
>> arch/arm/boot/dts/rk1108.dtsi | 420
>> ++++++++++++++++++++++++++++++++++++++ arch/arm/mach-rockchip/rockchip.c |
>> 1 +
>> 2 files changed, 421 insertions(+)
>> create mode 100644 arch/arm/boot/dts/rk1108.dtsi
>>
>> diff --git a/arch/arm/boot/dts/rk1108.dtsi b/arch/arm/boot/dts/rk1108.dtsi
>> new file mode 100644
>> index 0000000..9dccfea
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/rk1108.dtsi
>> @@ -0,0 +1,420 @@
>> +/*
>> + * 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.
>> + */
>> +
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include <dt-bindings/clock/rk1108-cru.h>
>> +#include <dt-bindings/pinctrl/rockchip.h>
>> +/ {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> +
>> + compatible = "rockchip,rk1108";
>> +
>> + interrupt-parent = <&gic>;
>> +
>> + aliases {
>> + serial0 = &uart0;
>> + serial1 = &uart1;
>> + serial2 = &uart2;
>> + };
>> +
>> + cpus {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + cpu0: cpu@f00 {
>> + device_type = "cpu";
>> + compatible = "arm,cortex-a7";
>> + reg = <0xf00>;
>> + };
>> +
> unnecessary empty line
Okay, I will remove it.
>> + };
>> +
>> + amba {
>> + compatible = "simple-bus";
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges;
>> +
>> + pdma: pdma@102a0000 {
>> + compatible = "arm,pl330", "arm,primecell";
>> + reg = <0x102a0000 0x4000>;
>> + interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
>> + #dma-cells = <1>;
>> + arm,pl330-broken-no-flushp;
>> + clocks = <&cru ACLK_DMAC>;
>> + clock-names = "apb_pclk";
>> + };
>> + };
>> +
>> + arm-pmu {
>> + compatible = "arm,cortex-a7-pmu";
>> + interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
>> + };
>> +
>> +
>> + timer {
>> + compatible = "arm,armv7-timer";
>> + interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_HIGH)>,
>> + <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> CPU_MASK_SIMPLE(4)? You only have one core, not 4.
>
>
>> + clock-frequency = <24000000>;
>> + };
>> +
>> + xin24m: oscillator {
>> + compatible = "fixed-clock";
>> + clock-frequency = <24000000>;
>> + clock-output-names = "xin24m";
>> + #clock-cells = <0>;
>> + };
>> +
>> + bus_intmem@10080000 {
>> + compatible = "mmio-sram";
>> + reg = <0x10080000 0x2000>;
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges = <0 0x10080000 0x2000>;
>> + };
>> +
>> + grf: syscon@10300000 {
>> + compatible = "rockchip,rk1108-grf", "syscon";
>> + reg = <0x10300000 0x1000>;
>> + };
>> +
>> + emmc: dwmmc@30110000 {
>> + compatible = "rockchip,rk1108-dw-mshc", "rockchip,rk3288-dw-mshc";
>> + clock-freq-min-max = <400000 150000000>;
>> + clocks = <&cru HCLK_EMMC>, <&cru SCLK_EMMC>,
>> + <&cru SCLK_EMMC_DRV>, <&cru SCLK_EMMC_SAMPLE>;
>> + clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
>> + fifo-depth = <0x100>;
>> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
>> + reg = <0x30110000 0x4000>;
>> + status = "disabled";
>> + };
>> +
>> + sdio: dwmmc@30120000 {
>> + compatible = "rockchip,rk1108-dw-mshc", "rockchip,rk3288-dw-mshc";
>> + clock-freq-min-max = <400000 150000000>;
>> + clocks = <&cru HCLK_SDIO>, <&cru SCLK_SDIO>,
>> + <&cru SCLK_SDIO_DRV>, <&cru SCLK_SDIO_SAMPLE>;
>> + clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
>> + fifo-depth = <0x100>;
>> + interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
>> + reg = <0x30120000 0x4000>;
>> + status = "disabled";
>> + };
>> +
>> + sdmmc: dwmmc@30130000 {
> ordering by register address please (uart2 before sdmmc etc; same for
> everything else)
>
>
>> + compatible = "rockchip,rk1108-dw-mshc", "rockchip,rk3288-dw-mshc";
>> + clock-freq-min-max = <400000 100000000>;
>> + clocks = <&cru HCLK_SDMMC>, <&cru SCLK_SDMMC>,
>> + <&cru SCLK_SDMMC_DRV>, <&cru SCLK_SDMMC_SAMPLE>;
>> + clock-names = "biu", "ciu", "ciu-drive", "ciu-sample";
>> + fifo-depth = <0x100>;
>> + interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
>> + reg = <0x30130000 0x4000>;
>> + status = "disabled";
>> + };
>> +
>> + uart2: serial@10210000 {
>> + compatible = "rockchip,rk1108-uart", "snps,dw-apb-uart";
>> + reg = <0x10210000 0x100>;
>> + interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
>> + reg-shift = <2>;
>> + reg-io-width = <4>;
>> + clock-frequency = <24000000>;
>> + clocks = <&cru SCLK_UART2>, <&cru PCLK_UART2>;
>> + clock-names = "baudclk", "apb_pclk";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&uart2m0_xfer>;
>> + status = "disabled";
>> + };
>> +
>> + uart1: serial@10220000 {
>> + compatible = "rockchip,rk1108-uart", "snps,dw-apb-uart";
>> + reg = <0x10220000 0x100>;
>> + interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
>> + reg-shift = <2>;
>> + reg-io-width = <4>;
>> + clock-frequency = <24000000>;
>> + clocks = <&cru SCLK_UART1>, <&cru PCLK_UART1>;
>> + clock-names = "baudclk", "apb_pclk";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&uart1_xfer>;
>> + status = "disabled";
>> + };
>> +
>> + uart0: serial@10230000 {
>> + compatible = "rockchip,rk1108-uart", "snps,dw-apb-uart";
>> + reg = <0x10230000 0x100>;
>> + interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
>> + reg-shift = <2>;
>> + reg-io-width = <4>;
>> + clock-frequency = <24000000>;
>> + clocks = <&cru SCLK_UART0>, <&cru PCLK_UART0>;
>> + clock-names = "baudclk", "apb_pclk";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&uart0_xfer &uart0_cts &uart0_rts>;
>> + status = "disabled";
>> + };
>> +
>> + cru: clock-controller@20200000 {
>> + compatible = "rockchip,rk1108-cru";
>> + reg = <0x20200000 0x1000>;
>> + rockchip,grf = <&grf>;
>> + #clock-cells = <1>;
>> + #reset-cells = <1>;
>> + };
>> +
>> + gic: interrupt-controller@32010000 {
>> + compatible = "arm,cortex-a15-gic";
> compatible = "arm,gic-400"; ?
>
>> + interrupt-controller;
>> + #interrupt-cells = <3>;
>> + #address-cells = <0>;
>> +
>> + reg = <0x32011000 0x1000>,
>> + <0x32012000 0x1000>;
> please provide all 4 register areas and also the interrupt (
I only found 2 register areas in our rockchip linux 3.10 source
code. And haven't found the interrupt. From the arm,gic bindings, the
interrupt property is optional. So am not sure if we
really need it here.
>
>> + };
>> +
> [...]
>
>> diff --git a/arch/arm/mach-rockchip/rockchip.c
>> b/arch/arm/mach-rockchip/rockchip.c index a7ab9ec..e7fdf06 100644
>> --- a/arch/arm/mach-rockchip/rockchip.c
>> +++ b/arch/arm/mach-rockchip/rockchip.c
>> @@ -76,6 +76,7 @@ static void __init rockchip_dt_init(void)
>> }
>>
>> static const char * const rockchip_board_dt_compat[] = {
>> + "rockchip,rk1108",
>> "rockchip,rk2928",
>> "rockchip,rk3066a",
>> "rockchip,rk3066b",
> please split this into a separate patch, as code and dts changes need to go
> through different branches.
>
>
> Thanks
> Heiko
>
>
>
^ permalink raw reply
* Re: [PATCH v6 11/14] mmc: sdhci-msm: Add HS400 platform support
From: Adrian Hunter @ 2016-11-08 12:37 UTC (permalink / raw)
To: Ritesh Harjani, ulf.hansson, linux-mmc, shawn.lin, sboyd
Cc: devicetree, linux-clk, david.brown, andy.gross, linux-arm-msm,
georgi.djakov, alex.lemberg, mateusz.nowak, Yuliy.Izrailov,
asutoshd, kdorfman, david.griego, stummala, venkatg, rnayak,
pramod.gurav
In-Reply-To: <1478517877-23733-12-git-send-email-riteshh@codeaurora.org>
On 07/11/16 13:24, Ritesh Harjani wrote:
> From: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>
> The following msm platform specific changes are added to support HS400.
> - Allow tuning for HS400 mode.
> - Configure HS400 timing mode using the VENDOR_SPECIFIC_FUNC register.
>
> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/sdhci-msm.c | 127 +++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 117 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 220567c..2561c41 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -31,6 +31,7 @@
> #define HC_MODE_EN 0x1
> #define CORE_POWER 0x0
> #define CORE_SW_RST BIT(7)
> +#define FF_CLK_SW_RST_DIS BIT(13)
>
> #define CORE_PWRCTL_STATUS 0xdc
> #define CORE_PWRCTL_MASK 0xe0
> @@ -64,11 +65,18 @@
>
> #define CORE_VENDOR_SPEC 0x10c
> #define CORE_CLK_PWRSAVE BIT(1)
> +#define CORE_HC_MCLK_SEL_DFLT (2 << 8)
> +#define CORE_HC_MCLK_SEL_HS400 (3 << 8)
> +#define CORE_HC_MCLK_SEL_MASK (3 << 8)
> +#define CORE_HC_SELECT_IN_EN BIT(18)
> +#define CORE_HC_SELECT_IN_HS400 (6 << 19)
> +#define CORE_HC_SELECT_IN_MASK (7 << 19)
>
> #define CORE_VENDOR_SPEC_CAPABILITIES0 0x11c
>
> #define TCXO_FREQ 19200000
> #define SDHCI_MSM_MIN_CLOCK 400000
> +#define CORE_FREQ_100MHZ (100 * 1000 * 1000)
>
> #define CDR_SELEXT_SHIFT 20
> #define CDR_SELEXT_MASK (0xf << CDR_SELEXT_SHIFT)
> @@ -85,6 +93,8 @@ struct sdhci_msm_host {
> unsigned long clk_rate;
> struct mmc_host *mmc;
> bool use_14lpp_dll_reset;
> + bool tuning_done;
> + bool calibration_done;
> };
>
> /* Platform specific tuning */
> @@ -173,8 +183,8 @@ static int msm_config_cm_dll_phase(struct sdhci_host *host, u8 phase)
> * Find out the greatest range of consecuitive selected
> * DLL clock output phases that can be used as sampling
> * setting for SD3.0 UHS-I card read operation (in SDR104
> - * timing mode) or for eMMC4.5 card read operation (in HS200
> - * timing mode).
> + * timing mode) or for eMMC4.5 card read operation (in
> + * HS400/HS200 timing mode).
> * Select the 3/4 of the range and configure the DLL with the
> * selected DLL clock output phase.
> */
> @@ -426,9 +436,10 @@ static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
> * Tuning is required for SDR104, HS200 and HS400 cards and
> * if clock frequency is greater than 100MHz in these modes.
> */
> - if (host->clock <= 100 * 1000 * 1000 ||
> - !((ios.timing == MMC_TIMING_MMC_HS200) ||
> - (ios.timing == MMC_TIMING_UHS_SDR104)))
> + if (host->clock <= CORE_FREQ_100MHZ ||
> + !(ios.timing == MMC_TIMING_MMC_HS400 ||
> + ios.timing == MMC_TIMING_MMC_HS200 ||
> + ios.timing == MMC_TIMING_UHS_SDR104))
> return 0;
>
> retry:
> @@ -479,6 +490,8 @@ static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
> rc = -EIO;
> }
>
> + if (!rc)
> + msm_host->tuning_done = true;
> return rc;
> }
>
> @@ -486,7 +499,10 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
> unsigned int uhs)
> {
> struct mmc_host *mmc = host->mmc;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> u16 ctrl_2;
> + u32 config;
>
> ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> /* Select Bus Speed Mode for host */
> @@ -501,6 +517,7 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
> case MMC_TIMING_UHS_SDR50:
> ctrl_2 |= SDHCI_CTRL_UHS_SDR50;
> break;
> + case MMC_TIMING_MMC_HS400:
> case MMC_TIMING_MMC_HS200:
> case MMC_TIMING_UHS_SDR104:
> ctrl_2 |= SDHCI_CTRL_UHS_SDR104;
> @@ -517,11 +534,31 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
> * provide feedback clock, the mode selection can be any value less
> * than 3'b011 in bits [2:0] of HOST CONTROL2 register.
> */
> - if (host->clock <= 100000000 &&
> - (uhs == MMC_TIMING_MMC_HS400 ||
> - uhs == MMC_TIMING_MMC_HS200 ||
> - uhs == MMC_TIMING_UHS_SDR104))
> - ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
> + if (host->clock <= CORE_FREQ_100MHZ) {
> + if ((uhs == MMC_TIMING_MMC_HS400) ||
> + (uhs == MMC_TIMING_MMC_HS200) ||
> + (uhs == MMC_TIMING_UHS_SDR104))
> + ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
> + /*
> + * Make sure DLL is disabled when not required
> + *
> + * Write 1 to DLL_RST bit of DLL_CONFIG register
> + */
> + config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> + config |= CORE_DLL_RST;
> + writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +
> + /* Write 1 to DLL_PDN bit of DLL_CONFIG register */
> + config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> + config |= CORE_DLL_PDN;
> + writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +
> + /*
> + * The DLL needs to be restored and CDCLP533 recalibrated
> + * when the clock frequency is set back to 400MHz.
> + */
> + msm_host->calibration_done = false;
> + }
>
> dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
> mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
> @@ -634,6 +671,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> struct mmc_ios curr_ios = host->mmc->ios;
> + u32 config;
> int rc;
>
> if (!clock) {
> @@ -652,6 +690,70 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> (curr_ios.timing == MMC_TIMING_MMC_DDR52) ||
> (curr_ios.timing == MMC_TIMING_MMC_HS400))
> clock *= 2;
> + /*
> + * In general all timing modes are controlled via UHS mode select in
> + * Host Control2 register. eMMC specific HS200/HS400 doesn't have
> + * their respective modes defined here, hence we use these values.
> + *
> + * HS200 - SDR104 (Since they both are equivalent in functionality)
> + * HS400 - This involves multiple configurations
> + * Initially SDR104 - when tuning is required as HS200
> + * Then when switching to DDR @ 400MHz (HS400) we use
> + * the vendor specific HC_SELECT_IN to control the mode.
> + *
> + * In addition to controlling the modes we also need to select the
> + * correct input clock for DLL depending on the mode.
> + *
> + * HS400 - divided clock (free running MCLK/2)
> + * All other modes - default (free running MCLK)
> + */
> + if (curr_ios.timing == MMC_TIMING_MMC_HS400) {
> + /* Select the divided clock (free running MCLK/2) */
> + config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> + config &= ~CORE_HC_MCLK_SEL_MASK;
> + config |= CORE_HC_MCLK_SEL_HS400;
> +
> + writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
> + /*
> + * Select HS400 mode using the HC_SELECT_IN from VENDOR SPEC
> + * register
> + */
> + if (msm_host->tuning_done && !msm_host->calibration_done) {
> + /*
> + * Write 0x6 to HC_SELECT_IN and 1 to HC_SELECT_IN_EN
> + * field in VENDOR_SPEC_FUNC
> + */
> + config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> + config |= CORE_HC_SELECT_IN_HS400;
> + config |= CORE_HC_SELECT_IN_EN;
> + writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
> + }
> + } else {
> + /* Select the default clock (free running MCLK) */
> + config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> + config &= ~CORE_HC_MCLK_SEL_MASK;
> + config |= CORE_HC_MCLK_SEL_DFLT;
> + writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
> +
> + /*
> + * Disable HC_SELECT_IN to be able to use the UHS mode select
> + * configuration from Host Control2 register for all other
> + * modes.
> + *
> + * Write 0 to HC_SELECT_IN and HC_SELECT_IN_EN field
> + * in VENDOR_SPEC_FUNC
> + */
> + config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> + config &= ~CORE_HC_SELECT_IN_EN;
> + config &= ~CORE_HC_SELECT_IN_MASK;
> + writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
> + }
> +
> + /*
> + * Make sure above writes impacting free running MCLK are completed
> + * before changing the clk_rate at GCC.
> + */
> + wmb();
>
> if (clock != msm_host->clk_rate) {
> rc = clk_set_rate(msm_host->clk, clock);
> @@ -790,6 +892,11 @@ static int sdhci_msm_probe(struct platform_device *pdev)
> /* Set HC_MODE_EN bit in HC_MODE register */
> writel_relaxed(HC_MODE_EN, (msm_host->core_mem + CORE_HC_MODE));
>
> + /* Set FF_CLK_SW_RST_DIS bit in HC_MODE register */
> + config = readl_relaxed(msm_host->core_mem + CORE_HC_MODE);
> + config |= FF_CLK_SW_RST_DIS;
> + writel_relaxed(config, msm_host->core_mem + CORE_HC_MODE);
> +
> host_version = readw_relaxed((host->ioaddr + SDHCI_HOST_VERSION));
> dev_dbg(&pdev->dev, "Host Version: 0x%x Vendor Version 0x%x\n",
> host_version, ((host_version & SDHCI_VENDOR_VER_MASK) >>
>
^ permalink raw reply
* Re: [PATCH v6 12/14] mmc: sdhci-msm: Save the calculated tuning phase
From: Adrian Hunter @ 2016-11-08 12:41 UTC (permalink / raw)
To: Ritesh Harjani, ulf.hansson, linux-mmc, shawn.lin, sboyd
Cc: devicetree, linux-clk, david.brown, andy.gross, linux-arm-msm,
georgi.djakov, alex.lemberg, mateusz.nowak, Yuliy.Izrailov,
asutoshd, kdorfman, david.griego, stummala, venkatg, rnayak,
pramod.gurav
In-Reply-To: <1478517877-23733-13-git-send-email-riteshh@codeaurora.org>
On 07/11/16 13:24, Ritesh Harjani wrote:
> Save the tuning phase once the tuning is performed.
> This phase value will be used while calibrating DLL
> for HS400 mode.
>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/sdhci-msm.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 2561c41..6431bb8 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -74,6 +74,7 @@
>
> #define CORE_VENDOR_SPEC_CAPABILITIES0 0x11c
>
> +#define INVALID_TUNING_PHASE -1
> #define TCXO_FREQ 19200000
> #define SDHCI_MSM_MIN_CLOCK 400000
> #define CORE_FREQ_100MHZ (100 * 1000 * 1000)
> @@ -95,6 +96,7 @@ struct sdhci_msm_host {
> bool use_14lpp_dll_reset;
> bool tuning_done;
> bool calibration_done;
> + u8 saved_tuning_phase;
> };
>
> /* Platform specific tuning */
> @@ -134,6 +136,9 @@ static int msm_config_cm_dll_phase(struct sdhci_host *host, u8 phase)
> u32 config;
> struct mmc_host *mmc = host->mmc;
>
> + if (phase > 0xf)
> + return -EINVAL;
> +
> spin_lock_irqsave(&host->lock, flags);
>
> config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> @@ -431,6 +436,8 @@ static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
> int rc;
> struct mmc_host *mmc = host->mmc;
> struct mmc_ios ios = host->mmc->ios;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>
> /*
> * Tuning is required for SDR104, HS200 and HS400 cards and
> @@ -455,6 +462,7 @@ static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
> if (rc)
> return rc;
>
> + msm_host->saved_tuning_phase = phase;
> rc = mmc_send_tuning(mmc, opcode, NULL);
> if (!rc) {
> /* Tuning is successful at this tuning point */
> @@ -826,6 +834,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>
> sdhci_get_of_property(pdev);
>
> + msm_host->saved_tuning_phase = INVALID_TUNING_PHASE;
> +
> /* Setup SDCC bus voter clock. */
> msm_host->bus_clk = devm_clk_get(&pdev->dev, "bus");
> if (!IS_ERR(msm_host->bus_clk)) {
>
^ permalink raw reply
* Re: [PATCH v6 13/14] mmc: sdhci-msm: Add calibration tuning for CDCLP533 circuit
From: Adrian Hunter @ 2016-11-08 12:50 UTC (permalink / raw)
To: Ritesh Harjani, ulf.hansson, linux-mmc, shawn.lin, sboyd
Cc: devicetree, linux-clk, david.brown, andy.gross, linux-arm-msm,
georgi.djakov, alex.lemberg, mateusz.nowak, Yuliy.Izrailov,
asutoshd, kdorfman, david.griego, stummala, venkatg, rnayak,
pramod.gurav
In-Reply-To: <1478517877-23733-14-git-send-email-riteshh@codeaurora.org>
On 07/11/16 13:24, Ritesh Harjani wrote:
> In HS400 mode a new RCLK is introduced on the interface for read data
> transfers. The eMMC5.0 device transmits the read data to the host with
> respect to rising and falling edges of RCLK. In order to ensure correct
> operation of read data transfers in HS400 mode, the incoming RX data
> needs to be sampled by delayed version of RCLK.
>
> The CDCLP533 delay circuit shifts the RCLK by T/4. It needs to be
> initialized, configured and enabled once during HS400 mode switch and
> when operational voltage/clock is changed.
>
> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/sdhci-msm.c | 163 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 163 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 6431bb8..ae19d1a 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -19,6 +19,7 @@
> #include <linux/delay.h>
> #include <linux/mmc/mmc.h>
> #include <linux/slab.h>
> +#include <linux/iopoll.h>
>
> #include "sdhci-pltfm.h"
>
> @@ -57,6 +58,7 @@
> #define CORE_DLL_PDN BIT(29)
> #define CORE_DLL_RST BIT(30)
> #define CORE_DLL_CONFIG 0x100
> +#define CORE_CMD_DAT_TRACK_SEL BIT(0)
> #define CORE_DLL_STATUS 0x108
>
> #define CORE_DLL_CONFIG_2 0x1b4
> @@ -72,6 +74,33 @@
> #define CORE_HC_SELECT_IN_HS400 (6 << 19)
> #define CORE_HC_SELECT_IN_MASK (7 << 19)
>
> +#define CORE_CSR_CDC_CTLR_CFG0 0x130
> +#define CORE_SW_TRIG_FULL_CALIB BIT(16)
> +#define CORE_HW_AUTOCAL_ENA BIT(17)
> +
> +#define CORE_CSR_CDC_CTLR_CFG1 0x134
> +#define CORE_CSR_CDC_CAL_TIMER_CFG0 0x138
> +#define CORE_TIMER_ENA BIT(16)
> +
> +#define CORE_CSR_CDC_CAL_TIMER_CFG1 0x13C
> +#define CORE_CSR_CDC_REFCOUNT_CFG 0x140
> +#define CORE_CSR_CDC_COARSE_CAL_CFG 0x144
> +#define CORE_CDC_OFFSET_CFG 0x14C
> +#define CORE_CSR_CDC_DELAY_CFG 0x150
> +#define CORE_CDC_SLAVE_DDA_CFG 0x160
> +#define CORE_CSR_CDC_STATUS0 0x164
> +#define CORE_CALIBRATION_DONE BIT(0)
> +
> +#define CORE_CDC_ERROR_CODE_MASK 0x7000000
> +
> +#define CORE_CSR_CDC_GEN_CFG 0x178
> +#define CORE_CDC_SWITCH_BYPASS_OFF BIT(0)
> +#define CORE_CDC_SWITCH_RC_EN BIT(1)
> +
> +#define CORE_DDR_200_CFG 0x184
> +#define CORE_CDC_T4_DLY_SEL BIT(0)
> +#define CORE_START_CDC_TRAFFIC BIT(6)
> +
> #define CORE_VENDOR_SPEC_CAPABILITIES0 0x11c
>
> #define INVALID_TUNING_PHASE -1
> @@ -429,6 +458,131 @@ static int msm_init_cm_dll(struct sdhci_host *host)
> return 0;
> }
>
> +static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> + u32 config, calib_done;
> + int ret;
> +
> + pr_debug("%s: %s: Enter\n", mmc_hostname(host->mmc), __func__);
> +
> + /*
> + * Retuning in HS400 (DDR mode) will fail, just reset the
> + * tuning block and restore the saved tuning phase.
> + */
> + ret = msm_init_cm_dll(host);
> + if (ret)
> + goto out;
> +
> + /* Set the selected phase in delay line hw block */
> + ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
> + if (ret)
> + goto out;
> +
> + /* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
> + config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> + config |= CORE_CMD_DAT_TRACK_SEL;
> + writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> +
> + /* Write 0 to CDC_T4_DLY_SEL field in VENDOR_SPEC_DDR200_CFG */
> + config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
> + config &= ~CORE_CDC_T4_DLY_SEL;
> + writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
> +
> + /* Write 0 to CDC_SWITCH_BYPASS_OFF field in CORE_CSR_CDC_GEN_CFG */
> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
> + config &= ~CORE_CDC_SWITCH_BYPASS_OFF;
> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
> +
> + /* Write 1 to CDC_SWITCH_RC_EN field in CORE_CSR_CDC_GEN_CFG */
> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_GEN_CFG);
> + config |= CORE_CDC_SWITCH_RC_EN;
> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_GEN_CFG);
> +
> + /* Write 0 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
> + config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
> + config &= ~CORE_START_CDC_TRAFFIC;
> + writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
> +
> + /*
> + * Perform CDC Register Initialization Sequence
> + *
> + * CORE_CSR_CDC_CTLR_CFG0 0x11800EC
> + * CORE_CSR_CDC_CTLR_CFG1 0x3011111
> + * CORE_CSR_CDC_CAL_TIMER_CFG0 0x1201000
> + * CORE_CSR_CDC_CAL_TIMER_CFG1 0x4
> + * CORE_CSR_CDC_REFCOUNT_CFG 0xCB732020
> + * CORE_CSR_CDC_COARSE_CAL_CFG 0xB19
> + * CORE_CSR_CDC_DELAY_CFG 0x3AC
> + * CORE_CDC_OFFSET_CFG 0x0
> + * CORE_CDC_SLAVE_DDA_CFG 0x16334
> + */
> +
> + writel_relaxed(0x11800EC, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> + writel_relaxed(0x3011111, host->ioaddr + CORE_CSR_CDC_CTLR_CFG1);
> + writel_relaxed(0x1201000, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
> + writel_relaxed(0x4, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG1);
> + writel_relaxed(0xCB732020, host->ioaddr + CORE_CSR_CDC_REFCOUNT_CFG);
> + writel_relaxed(0xB19, host->ioaddr + CORE_CSR_CDC_COARSE_CAL_CFG);
> + writel_relaxed(0x3AC, host->ioaddr + CORE_CSR_CDC_DELAY_CFG);
> + writel_relaxed(0x0, host->ioaddr + CORE_CDC_OFFSET_CFG);
> + writel_relaxed(0x16334, host->ioaddr + CORE_CDC_SLAVE_DDA_CFG);
> +
> + /* CDC HW Calibration */
> +
> + /* Write 1 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> + config |= CORE_SW_TRIG_FULL_CALIB;
> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +
> + /* Write 0 to SW_TRIG_FULL_CALIB field in CORE_CSR_CDC_CTLR_CFG0 */
> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> + config &= ~CORE_SW_TRIG_FULL_CALIB;
> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +
> + /* Write 1 to HW_AUTOCAL_ENA field in CORE_CSR_CDC_CTLR_CFG0 */
> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> + config |= CORE_HW_AUTOCAL_ENA;
> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CTLR_CFG0);
> +
> + /* Write 1 to TIMER_ENA field in CORE_CSR_CDC_CAL_TIMER_CFG0 */
> + config = readl_relaxed(host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
> + config |= CORE_TIMER_ENA;
> + writel_relaxed(config, host->ioaddr + CORE_CSR_CDC_CAL_TIMER_CFG0);
> +
> + /* Poll on CALIBRATION_DONE field in CORE_CSR_CDC_STATUS0 to be 1 */
> + ret = readl_relaxed_poll_timeout(host->ioaddr + CORE_CSR_CDC_STATUS0,
> + calib_done,
> + (calib_done & CORE_CALIBRATION_DONE),
> + 1, 50);
> +
> + if (ret == -ETIMEDOUT) {
> + pr_err("%s: %s: CDC calibration was not completed\n",
> + mmc_hostname(host->mmc), __func__);
> + goto out;
> + }
> +
> + /* Verify CDC_ERROR_CODE field in CORE_CSR_CDC_STATUS0 is 0 */
> + ret = readl_relaxed(host->ioaddr + CORE_CSR_CDC_STATUS0)
> + & CORE_CDC_ERROR_CODE_MASK;
> + if (ret) {
> + pr_err("%s: %s: CDC error code %d\n",
> + mmc_hostname(host->mmc), __func__, ret);
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + /* Write 1 to START_CDC_TRAFFIC field in CORE_DDR200_CFG */
> + config = readl_relaxed(host->ioaddr + CORE_DDR_200_CFG);
> + config |= CORE_START_CDC_TRAFFIC;
> + writel_relaxed(config, host->ioaddr + CORE_DDR_200_CFG);
> +out:
> + pr_debug("%s: %s: Exit, ret %d\n", mmc_hostname(host->mmc),
> + __func__, ret);
> + return ret;
> +}
> +
> static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
> {
> int tuning_seq_cnt = 3;
> @@ -571,6 +725,15 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
> dev_dbg(mmc_dev(mmc), "%s: clock=%u uhs=%u ctrl_2=0x%x\n",
> mmc_hostname(host->mmc), host->clock, uhs, ctrl_2);
> sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> +
> + spin_unlock_irq(&host->lock);
> + /* CDCLP533 HW calibration is only required for HS400 mode*/
> + if (host->clock > CORE_FREQ_100MHZ &&
> + msm_host->tuning_done && !msm_host->calibration_done &&
> + (mmc->ios.timing == MMC_TIMING_MMC_HS400))
> + if (!sdhci_msm_cdclp533_calibration(host))
> + msm_host->calibration_done = true;
> + spin_lock_irq(&host->lock);
> }
>
> static void sdhci_msm_voltage_switch(struct sdhci_host *host)
>
^ permalink raw reply
* Re: [PATCH v6 14/14] sdhci: sdhci-msm: update dll configuration
From: Adrian Hunter @ 2016-11-08 12:57 UTC (permalink / raw)
To: Ritesh Harjani, ulf.hansson-QSEj5FYQhm4dnm+yROfE0A,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
shawn.lin-TNX95d0MmH7DzftRWevZcw, sboyd-sgV2jX0FEOL9JmXXK+q4OQ
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
david.brown-QSEj5FYQhm4dnm+yROfE0A,
andy.gross-QSEj5FYQhm4dnm+yROfE0A,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
georgi.djakov-QSEj5FYQhm4dnm+yROfE0A,
alex.lemberg-XdAiOPVOjttBDgjK7y7TUQ,
mateusz.nowak-ral2JQCrhuEAvxtiuMwx3w,
Yuliy.Izrailov-XdAiOPVOjttBDgjK7y7TUQ,
asutoshd-sgV2jX0FEOL9JmXXK+q4OQ, kdorfman-sgV2jX0FEOL9JmXXK+q4OQ,
david.griego-QSEj5FYQhm4dnm+yROfE0A,
stummala-sgV2jX0FEOL9JmXXK+q4OQ, venkatg-sgV2jX0FEOL9JmXXK+q4OQ,
rnayak-sgV2jX0FEOL9JmXXK+q4OQ,
pramod.gurav-QSEj5FYQhm4dnm+yROfE0A, Krishna Konda
In-Reply-To: <1478517877-23733-15-git-send-email-riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 07/11/16 13:24, Ritesh Harjani wrote:
> The newer msm sdhci's cores use a different DLL hardware for HS400.
> Update the configuration and calibration of the newer DLL block.
>
> The HS400 DLL block used previously is CDC LP 533 and requires
> programming multiple registers and waiting for configuration to
> complete and then enable it. It has about 18 register writes and
> two register reads.
>
> The newer HS400 DLL block is SDC4 DLL and requires two register
> writes for configuration and one register read to confirm that it
> is initialized. There is an additional register write to enable
> the power save mode for SDC4 DLL block.
>
> Signed-off-by: Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Signed-off-by: Krishna Konda <kkonda-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Acked-by: Adrian Hunter <adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> drivers/mmc/host/sdhci-msm.c | 132 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 130 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index ae19d1a..84812e6 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -51,6 +51,7 @@
> #define INT_MASK 0xf
> #define MAX_PHASES 16
> #define CORE_DLL_LOCK BIT(7)
> +#define CORE_DDR_DLL_LOCK BIT(11)
> #define CORE_DLL_EN BIT(16)
> #define CORE_CDR_EN BIT(17)
> #define CORE_CK_OUT_EN BIT(18)
> @@ -62,6 +63,7 @@
> #define CORE_DLL_STATUS 0x108
>
> #define CORE_DLL_CONFIG_2 0x1b4
> +#define CORE_DDR_CAL_EN BIT(0)
> #define CORE_FLL_CYCLE_CNT BIT(18)
> #define CORE_DLL_CLOCK_DISABLE BIT(21)
>
> @@ -100,6 +102,11 @@
> #define CORE_DDR_200_CFG 0x184
> #define CORE_CDC_T4_DLY_SEL BIT(0)
> #define CORE_START_CDC_TRAFFIC BIT(6)
> +#define CORE_VENDOR_SPEC3 0x1b0
> +#define CORE_PWRSAVE_DLL BIT(3)
> +
> +#define CORE_DDR_CONFIG 0x1b8
> +#define DDR_CONFIG_POR_VAL 0x80040853
>
> #define CORE_VENDOR_SPEC_CAPABILITIES0 0x11c
>
> @@ -126,6 +133,7 @@ struct sdhci_msm_host {
> bool tuning_done;
> bool calibration_done;
> u8 saved_tuning_phase;
> + bool use_cdclp533;
> };
>
> /* Platform specific tuning */
> @@ -583,6 +591,93 @@ static int sdhci_msm_cdclp533_calibration(struct sdhci_host *host)
> return ret;
> }
>
> +static int sdhci_msm_cm_dll_sdc4_calibration(struct sdhci_host *host)
> +{
> + u32 dll_status, config;
> + int ret;
> +
> + pr_debug("%s: %s: Enter\n", mmc_hostname(host->mmc), __func__);
> +
> + /*
> + * Currently the CORE_DDR_CONFIG register defaults to desired
> + * configuration on reset. Currently reprogramming the power on
> + * reset (POR) value in case it might have been modified by
> + * bootloaders. In the future, if this changes, then the desired
> + * values will need to be programmed appropriately.
> + */
> + writel_relaxed(DDR_CONFIG_POR_VAL, host->ioaddr + CORE_DDR_CONFIG);
> +
> + /* Write 1 to DDR_CAL_EN field in CORE_DLL_CONFIG_2 */
> + config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG_2);
> + config |= CORE_DDR_CAL_EN;
> + writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG_2);
> +
> + /* Poll on DDR_DLL_LOCK bit in CORE_DLL_STATUS to be set */
> + ret = readl_relaxed_poll_timeout(host->ioaddr + CORE_DLL_STATUS,
> + dll_status,
> + (dll_status & CORE_DDR_DLL_LOCK),
> + 10, 1000);
> +
> + if (ret == -ETIMEDOUT) {
> + pr_err("%s: %s: CM_DLL_SDC4 calibration was not completed\n",
> + mmc_hostname(host->mmc), __func__);
> + goto out;
> + }
> +
> + /* set CORE_PWRSAVE_DLL bit in CORE_VENDOR_SPEC3 */
> + config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC3);
> + config |= CORE_PWRSAVE_DLL;
> + writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC3);
> +
> + /*
> + * Drain writebuffer to ensure above DLL calibration
> + * and PWRSAVE DLL is enabled.
> + */
> + wmb();
> +out:
> + pr_debug("%s: %s: Exit, ret %d\n", mmc_hostname(host->mmc),
> + __func__, ret);
> + return ret;
> +}
> +
> +static int sdhci_msm_hs400_dll_calibration(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> + int ret;
> + u32 config;
> +
> + pr_debug("%s: %s: Enter\n", mmc_hostname(host->mmc), __func__);
> +
> + /*
> + * Retuning in HS400 (DDR mode) will fail, just reset the
> + * tuning block and restore the saved tuning phase.
> + */
> + ret = msm_init_cm_dll(host);
> + if (ret)
> + goto out;
> +
> + /* Set the selected phase in delay line hw block */
> + ret = msm_config_cm_dll_phase(host, msm_host->saved_tuning_phase);
> + if (ret)
> + goto out;
> +
> + /* Write 1 to CMD_DAT_TRACK_SEL field in DLL_CONFIG */
> + config = readl_relaxed(host->ioaddr + CORE_DLL_CONFIG);
> + config |= CORE_CMD_DAT_TRACK_SEL;
> + writel_relaxed(config, host->ioaddr + CORE_DLL_CONFIG);
> + if (msm_host->use_cdclp533)
> + /* Calibrate CDCLP533 DLL HW */
> + ret = sdhci_msm_cdclp533_calibration(host);
> + else
> + /* Calibrate CM_DLL_SDC4 HW */
> + ret = sdhci_msm_cm_dll_sdc4_calibration(host);
> +out:
> + pr_debug("%s: %s: Exit, ret %d\n", mmc_hostname(host->mmc),
> + __func__, ret);
> + return ret;
> +}
> +
> static int sdhci_msm_execute_tuning(struct sdhci_host *host, u32 opcode)
> {
> int tuning_seq_cnt = 3;
> @@ -731,7 +826,7 @@ static void sdhci_msm_set_uhs_signaling(struct sdhci_host *host,
> if (host->clock > CORE_FREQ_100MHZ &&
> msm_host->tuning_done && !msm_host->calibration_done &&
> (mmc->ios.timing == MMC_TIMING_MMC_HS400))
> - if (!sdhci_msm_cdclp533_calibration(host))
> + if (!sdhci_msm_hs400_dll_calibration(host))
> msm_host->calibration_done = true;
> spin_lock_irq(&host->lock);
> }
> @@ -842,7 +937,7 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> struct mmc_ios curr_ios = host->mmc->ios;
> - u32 config;
> + u32 config, dll_lock;
> int rc;
>
> if (!clock) {
> @@ -899,7 +994,33 @@ static void sdhci_msm_set_clock(struct sdhci_host *host, unsigned int clock)
> config |= CORE_HC_SELECT_IN_EN;
> writel_relaxed(config, host->ioaddr + CORE_VENDOR_SPEC);
> }
> + if (!msm_host->clk_rate && !msm_host->use_cdclp533) {
> + /*
> + * Poll on DLL_LOCK or DDR_DLL_LOCK bits in
> + * CORE_DLL_STATUS to be set. This should get set
> + * within 15 us at 200 MHz.
> + */
> + rc = readl_relaxed_poll_timeout(host->ioaddr +
> + CORE_DLL_STATUS,
> + dll_lock,
> + (dll_lock &
> + (CORE_DLL_LOCK |
> + CORE_DDR_DLL_LOCK)), 10,
> + 1000);
> + if (rc == -ETIMEDOUT)
> + pr_err("%s: Unable to get DLL_LOCK/DDR_DLL_LOCK, dll_status: 0x%08x\n",
> + mmc_hostname(host->mmc), dll_lock);
> + }
> } else {
> + if (!msm_host->use_cdclp533) {
> + /* set CORE_PWRSAVE_DLL bit in CORE_VENDOR_SPEC3 */
> + config = readl_relaxed(host->ioaddr +
> + CORE_VENDOR_SPEC3);
> + config &= ~CORE_PWRSAVE_DLL;
> + writel_relaxed(config, host->ioaddr +
> + CORE_VENDOR_SPEC3);
> + }
> +
> /* Select the default clock (free running MCLK) */
> config = readl_relaxed(host->ioaddr + CORE_VENDOR_SPEC);
> config &= ~CORE_HC_MCLK_SEL_MASK;
> @@ -1086,6 +1207,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
> msm_host->use_14lpp_dll_reset = true;
>
> /*
> + * SDCC 5 controller with major version 1, minor version 0x34 and later
> + * with HS 400 mode support will use CM DLL instead of CDC LP 533 DLL.
> + */
> + if ((core_major == 1) && (core_minor < 0x34))
> + msm_host->use_cdclp533 = true;
> +
> + /*
> * Support for some capabilities is not advertised by newer
> * controller versions and must be explicitly enabled.
> */
>
--
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] ARM: tegra: nyan: Mark all USB ports as host
From: Paul Kocialkowski @ 2016-11-08 13:02 UTC (permalink / raw)
To: Jon Hunter, Thierry Reding
Cc: Peter De Schrijver, devicetree, Stephen Warren, linux-kernel,
linux-tegra, Alexandre Courbot, linux-arm-kernel
In-Reply-To: <3fbd3fb9-1dc5-a46d-355b-f7c94b3c43ef@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 3014 bytes --]
Le mardi 08 novembre 2016 à 11:09 +0000, Jon Hunter a écrit :
> On 08/11/16 11:07, Thierry Reding wrote:
> >
> > * PGP Signed by an unknown key
> >
> > On Tue, Nov 08, 2016 at 09:47:42AM +0000, Jon Hunter wrote:
> > >
> > >
> > > On 08/11/16 08:54, Peter De Schrijver wrote:
> > > >
> > > > On Mon, Nov 07, 2016 at 02:09:31PM +0000, Jon Hunter wrote:
> > > > >
> > > > >
> > > > > On 07/11/16 13:28, Thierry Reding wrote:
> > > > > >
> > > > > > >
> > > > > > > Old Signed by an unknown key
> > > > > >
> > > > > > On Sun, Sep 18, 2016 at 12:28:52PM +0200, Paul Kocialkowski wrote:
> > > > > > >
> > > > > > > Nyan boards only have host USB ports (2 external, 1 internal),
> > > > > > > there is
> > > > > > > no OTG-enabled connector.
> > > > > > >
> > > > > > > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > > > > > > ---
> > > > > > > arch/arm/boot/dts/tegra124-nyan.dtsi | 2 +-
> > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > Where is this information coming from? I don't have one of the Nyans
> > > > > > myself, but one of the Tegra132 devices I have, which I think was
> > > > > > derived from one of the Nyans uses one of the external host ports as
> > > > > > forced recovery port, for which it would need OTG.
> > > > > >
> > > > > > I suspect that the way to get U-Boot onto the Nyans is via tegrarcm?
> > > > > > In that case I think one of the ports must be OTG.
> > > > >
> > > > > It is true that the port on the back on the nyan-big can be used with
> > > > > recovery mode. I was thinking that this is not a true OTG port as it
> > > > > is
> > > > > just a 4-pin type A socket and does not have an ID pin. Thinking some
> > > > > more about this the USB spec does include a "Host Negotiation Protocol
> > > > > (HNP)" that allows a host and device to swap roles and so keeping it
> > > > > as
> > > > > OTG seems valid afterall.
> > > >
> > > > I don't think the bootrom implements that though. I expect recovery mode
> > > > to just program the controller in device mode, without performing any
> > > > negotiation.
> > >
> > > I am not talking about the bootrom and I would not expect the bootrom to
> > > do that. However, the kernel could.
> >
> > Either way, configuring the controller in device mode is enough to make
> > the host detect it, otherwise tegrarcm wouldn't work.
> >
> > From the point of view of the binding I think "otg" is the most accurate
> > option because we know that the controller can operate in both modes. If
> > it currently doesn't or how exactly switching modes is done is outside
> > the scope of this property.
> >
> > Is everyone okay with just dropping this patch?
>
> Fine with me.
Same here.
--
Paul Kocialkowski, developer of low-level free software for embedded devices
Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH 4/6] ARM: dts: add basic support for Rockchip RK1108 SOC
From: Heiko Stübner @ 2016-11-08 13:20 UTC (permalink / raw)
To: Andy Yan
Cc: elaine.zhang-TNX95d0MmH7DzftRWevZcw,
mturquette-rdvid1DuHRBWk0Htik3J/w,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <0516ad0b-bfbe-ec80-fdb6-e118dab3e758-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Am Dienstag, 8. November 2016, 20:31:55 schrieb Andy Yan:
> Hi Heiko:
>
> On 2016年11月04日 16:00, Heiko Stuebner wrote:
> > Am Donnerstag, 3. November 2016, 20:40:48 CET schrieb Andy Yan:
> >> + gic: interrupt-controller@32010000 {
> >> + compatible = "arm,cortex-a15-gic";
> >
> > compatible = "arm,gic-400"; ?
> >
> >> + interrupt-controller;
> >> + #interrupt-cells = <3>;
> >> + #address-cells = <0>;
> >> +
> >> + reg = <0x32011000 0x1000>,
> >> + <0x32012000 0x1000>;
> >
> > please provide all 4 register areas and also the interrupt (
>
> I only found 2 register areas in our rockchip linux 3.10 source
> code. And haven't found the interrupt. From the arm,gic bindings, the
> interrupt property is optional. So am not sure if we
> really need it here.
Devicetree is a hardware description, so it's not a factor if we "need" it but
only if it is present in the hardware. And we really want this information to
be complete, as these additional areas are necessary if someone wants to use
the virtualization extensions the cortext-A7 does contain.
The gic is a very standard component and the gic400 used here should definitly
have those two additional areas as well as the interrupt.
I think the memory areas are pretty standard and should be for the rk1108:
reg = <0x32011000 0x1000>,
<0x32012000 0x1000>,
<0x32014000 0x2000>,
<0x32016000 0x2000>;
The TRM talks about 128 SPI and 3 PPI interrupts but the irq-list does not
contain them, so this seems to be an error in the TRM, as the gic interrupt
should be one of those PPI interrupts.
Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads
From: Linus Walleij @ 2016-11-08 13:29 UTC (permalink / raw)
To: Laxman Dewangan
Cc: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
Stephen Warren, Rob Herring, Mark Rutland, Jon Hunter,
Masahiro Yamada,
linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <5821A6D3.7010000-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On Tue, Nov 8, 2016 at 11:20 AM, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> On Tuesday 08 November 2016 03:45 PM, Linus Walleij wrote:
>> If you can *actually* change the volatage, it needs to be modeled
>> as a (fixed voltage?) regulator, not as a custom property for the pin
>> control attributes. I guess you definiately need the regulator framework
>> to accumulate and infer the different consumer requirements anyway
>> in that case.
>
> The PMIC voltage output is changed via regulator calls.
> Here, we need to have two configruations for given voltage level of
> interface:
> * One at IO voltage from PMIC via regulator call to change votlage of IO
> rail.
> * Second, configure the IO pad register to tell the IO voltage level so that
> it can configured internally for that level.
I understand! (I think.)
But then the two things (A) changing the regulator voltage and (B) changing
the pin setting need to happen at the same time do they
not?
Now you're just hardcoding something into these device tree properties
and hoping that the regulators will somehow be set up in accordance to
what you set up for the pads in the device tree, correct?
To me it seems like the pins/pads should all have an <&phandle> to
the regulator controlling its voltage output, in the device tree.
In the Linux kernel, the driver has to regulator_[bulk_]get() this for
each pin, check the voltage with regulator_get_voltage() and set up
this according to the supplied voltage.
The driver then ideally should subscribe to regulator voltage notifier
events to change the setting if the voltage changes. I guess. But
atleast the first step seems inevitable: get the voltage from a regulator.
Else there is no dependency between the regulator and its consumer.
So what your pins need is a regulator phandle, not a magic value to
be poked into a register, hoping things will match up.
I understand that this is a simple quick-and-dirty solution but it is
not the right solution.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] pinctrl: tegra: Add driver to configure voltage and power of io pads
From: Laxman Dewangan @ 2016-11-08 13:35 UTC (permalink / raw)
To: Linus Walleij
Cc: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
Stephen Warren, Rob Herring, Mark Rutland, Jon Hunter,
Masahiro Yamada,
linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CACRpkdYO9ZCTGVdnyhVZ4DNLsHq+d-1_xuws2be8yab+MsyyVw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tuesday 08 November 2016 06:59 PM, Linus Walleij wrote:
> On Tue, Nov 8, 2016 at 11:20 AM, Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> On Tuesday 08 November 2016 03:45 PM, Linus Walleij wrote:
>>> If you can *actually* change the volatage, it needs to be modeled
>>> as a (fixed voltage?) regulator, not as a custom property for the pin
>>> control attributes. I guess you definiately need the regulator framework
>>> to accumulate and infer the different consumer requirements anyway
>>> in that case.
>> The PMIC voltage output is changed via regulator calls.
>> Here, we need to have two configruations for given voltage level of
>> interface:
>> * One at IO voltage from PMIC via regulator call to change votlage of IO
>> rail.
>> * Second, configure the IO pad register to tell the IO voltage level so that
>> it can configured internally for that level.
> I understand! (I think.)
Thanks,
>
> But then the two things (A) changing the regulator voltage and (B) changing
> the pin setting need to happen at the same time do they
> not?
>
> Now you're just hardcoding something into these device tree properties
> and hoping that the regulators will somehow be set up in accordance to
> what you set up for the pads in the device tree, correct?
There is two types of configuration in given platform, the IO voltage
does not get change (fixed in given platform) and in some of cases, get
change dynamically like SDIO3.0 where the voltage switches to 3.3V and 1.8V.
Yes, it can be integrated with the regulator handle and then it can call
the required configurations through notifier and regulator_get_voltage().
But I think it is too much complex for the static configurations. This
mandate also to populate the regulator handle and all power tree.
The simple way for static configuration (case where voltage does not get
change), just take the power tree IO voltage from DT and configure the
IO pad control register.
For dynamic case, there is some sequence need to be followed based on
voltage direction change (towards lower or towards higher) for the
voltage change and the IO pad voltage configuration and it is simple to
do it from client driver.
>
> To me it seems like the pins/pads should all have an <&phandle> to
> the regulator controlling its voltage output, in the device tree.
>
> In the Linux kernel, the driver has to regulator_[bulk_]get() this for
> each pin, check the voltage with regulator_get_voltage() and set up
> this according to the supplied voltage.
>
> The driver then ideally should subscribe to regulator voltage notifier
> events to change the setting if the voltage changes. I guess. But
> atleast the first step seems inevitable: get the voltage from a regulator.
>
> Else there is no dependency between the regulator and its consumer.
>
> So what your pins need is a regulator phandle, not a magic value to
> be poked into a register, hoping things will match up.
>
> I understand that this is a simple quick-and-dirty solution but it is
> not the right solution.
Yaah, the static power tree configuration is much simple in this
approach without having regulator drivers and support.
Integrating with regulator driver can be done here also.
I like to have both approach, through pinmux DT and also from regulator.
So based on the platform, if regulator supported then populate required
properties in DT for regulator else go on standard pinmux DT way (for
non-regulator cases).
Need your opinion?
^ permalink raw reply
* [PATCH 6/9 v2] arm64: dts: m3ulcb: enable SDHI0
From: Vladimir Barinov @ 2016-11-08 14:14 UTC (permalink / raw)
To: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland
Cc: devicetree, linux-renesas-soc, Vladimir Barinov
In-Reply-To: <1478196375-1131-1-git-send-email-vladimir.barinov@cogentembedded.com>
This supports SDHI0 on M3ULCB board SD card slot
Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
Reviewed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
Changes in version 2:
- renamed sdhi0_pins_3v3 to sdhi0_pins
- renamed sd0_3v3 to sd0
- renamed sdhi0_pins_1v8 to sdhi0_pins_uhs
- renamed sd0_1v8 to sd0_uhs
arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts | 48 ++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts b/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
index 3329f78..5be0cf6 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
@@ -44,6 +44,30 @@
gpios = <&gpio6 11 GPIO_ACTIVE_LOW>;
};
};
+
+ vcc_sdhi0: regulator-vcc-sdhi0 {
+ compatible = "regulator-fixed";
+
+ regulator-name = "SDHI0 Vcc";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+
+ gpio = <&gpio5 2 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+
+ vccq_sdhi0: regulator-vccq-sdhi0 {
+ compatible = "regulator-gpio";
+
+ regulator-name = "SDHI0 VccQ";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+
+ gpios = <&gpio5 1 GPIO_ACTIVE_HIGH>;
+ gpios-states = <1>;
+ states = <3300000 1
+ 1800000 0>;
+ };
};
&extal_clk {
@@ -68,8 +92,33 @@
groups = "scif_clk_a";
function = "scif_clk";
};
+
+ sdhi0_pins: sd0 {
+ groups = "sdhi0_data4", "sdhi0_ctrl";
+ function = "sdhi0";
+ power-source = <3300>;
+ };
+
+ sdhi0_pins_uhs: sd0_uhs {
+ groups = "sdhi0_data4", "sdhi0_ctrl";
+ function = "sdhi0";
+ power-source = <1800>;
+ };
};
+&sdhi0 {
+ pinctrl-0 = <&sdhi0_pins>;
+ pinctrl-1 = <&sdhi0_pins_uhs>;
+ pinctrl-names = "default", "state_uhs";
+
+ vmmc-supply = <&vcc_sdhi0>;
+ vqmmc-supply = <&vccq_sdhi0>;
+ cd-gpios = <&gpio3 12 GPIO_ACTIVE_LOW>;
+ bus-width = <4>;
+ sd-uhs-sdr50;
+ status = "okay";
+};
+
&scif2 {
pinctrl-0 = <&scif2_pins>;
pinctrl-names = "default";
--
1.9.1
^ permalink raw reply related
* [PATCH 9/9 v2] arm64: dts: m3ulcb: enable SDHI2
From: Vladimir Barinov @ 2016-11-08 14:14 UTC (permalink / raw)
To: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Vladimir Barinov
In-Reply-To: <1478196375-1131-1-git-send-email-vladimir.barinov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
This supports SDHI2 for M3ULCB onboard eMMC
Signed-off-by: Vladimir Barinov <vladimir.barinov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
Reviewed-off-by: Simon Horman <horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
---
Changes in version 2:
- renamed sdhi2_pins_3v3 to sdhi2_pins
- renamed sd2_3v3 to sd2
- renamed sdhi2_pins_1v8 to sdhi2_pins_uhs
- renamed sd2_1v8 to sd2_uhs
arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts | 43 ++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts b/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
index a244edb..e46687e 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-m3ulcb.dts
@@ -55,6 +55,24 @@
};
};
+ reg_1p8v: regulator0 {
+ compatible = "regulator-fixed";
+ regulator-name = "fixed-1.8V";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ reg_3p3v: regulator1 {
+ compatible = "regulator-fixed";
+ regulator-name = "fixed-3.3V";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
vcc_sdhi0: regulator-vcc-sdhi0 {
compatible = "regulator-fixed";
@@ -113,6 +131,18 @@
function = "sdhi0";
power-source = <1800>;
};
+
+ sdhi2_pins: sd2 {
+ groups = "sdhi2_data8", "sdhi2_ctrl";
+ function = "sdhi2";
+ power-source = <3300>;
+ };
+
+ sdhi2_pins_uhs: sd2_uhs {
+ groups = "sdhi2_data8", "sdhi2_ctrl";
+ function = "sdhi2";
+ power-source = <1800>;
+ };
};
&sdhi0 {
@@ -128,6 +158,19 @@
status = "okay";
};
+&sdhi2 {
+ /* used for on-board 8bit eMMC */
+ pinctrl-0 = <&sdhi2_pins>;
+ pinctrl-1 = <&sdhi2_pins_uhs>;
+ pinctrl-names = "default", "state_uhs";
+
+ vmmc-supply = <®_3p3v>;
+ vqmmc-supply = <®_1p8v>;
+ bus-width = <8>;
+ non-removable;
+ status = "okay";
+};
+
&scif2 {
pinctrl-0 = <&scif2_pins>;
pinctrl-names = "default";
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 3/4 v2] arm64: dts: h3ulcb: enable SDHI2
From: Vladimir Barinov @ 2016-11-08 14:16 UTC (permalink / raw)
To: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA, Vladimir Barinov
In-Reply-To: <1478196242-939-1-git-send-email-vladimir.barinov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
This supports SDHI2 for H3ULCB onboard eMMC
Signed-off-by: Vladimir Barinov <vladimir.barinov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
Reviewed-off-by: Simon Horman <horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
---
Changes in version 2:
- renamed sdhi2_pins_3v3 to sdhi2_pins
- renamed sd2_3v3 to sd2
- renamed sdhi2_pins_1v8 to sdhi2_pins_uhs
- renamed sd2_1v8 to sd2_uhs
arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts | 43 ++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts b/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
index a244edb..e46687e 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
@@ -55,6 +55,24 @@
clock-frequency = <24576000>;
};
+ reg_1p8v: regulator0 {
+ compatible = "regulator-fixed";
+ regulator-name = "fixed-1.8V";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
+ reg_3p3v: regulator1 {
+ compatible = "regulator-fixed";
+ regulator-name = "fixed-3.3V";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
vcc_sdhi0: regulator-vcc-sdhi0 {
compatible = "regulator-fixed";
@@ -113,6 +131,18 @@
function = "sdhi0";
power-source = <1800>;
};
+
+ sdhi2_pins: sd2 {
+ groups = "sdhi2_data8", "sdhi2_ctrl";
+ function = "sdhi2";
+ power-source = <3300>;
+ };
+
+ sdhi2_pins_uhs: sd2_uhs {
+ groups = "sdhi2_data8", "sdhi2_ctrl";
+ function = "sdhi2";
+ power-source = <1800>;
+ };
sound_pins: sound {
groups = "ssi01239_ctrl", "ssi0_data", "ssi1_data_a";
@@ -128,6 +158,19 @@
status = "okay";
};
+&sdhi2 {
+ /* used for on-board 8bit eMMC */
+ pinctrl-0 = <&sdhi2_pins>;
+ pinctrl-1 = <&sdhi2_pins_uhs>;
+ pinctrl-names = "default", "state_uhs";
+
+ vmmc-supply = <®_3p3v>;
+ vqmmc-supply = <®_1p8v>;
+ bus-width = <8>;
+ non-removable;
+ status = "okay";
+};
+
&ssi1 {
shared-pin;
};
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 4/4 v2] arm64: dts: h3ulcb: rename SDHI0 pins
From: Vladimir Barinov @ 2016-11-08 14:16 UTC (permalink / raw)
To: Simon Horman, Magnus Damm, Rob Herring, Mark Rutland
Cc: devicetree, linux-renesas-soc, Vladimir Barinov
In-Reply-To: <1478196242-939-1-git-send-email-vladimir.barinov@cogentembedded.com>
This changes SDHI0 pin names for H3ULCB board
Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
---
Changes in version 2:
- Initially added
diff --git a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts b/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
index 8d0ac07..6ffb051 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dts
@@ -163,13 +163,13 @@
function = "avb";
};
- sdhi0_pins_3v3: sd0_3v3 {
+ sdhi0_pins: sd0 {
groups = "sdhi0_data4", "sdhi0_ctrl";
function = "sdhi0";
power-source = <3300>;
};
- sdhi0_pins_1v8: sd0_1v8 {
+ sdhi0_pins_uhs: sd0 {
groups = "sdhi0_data4", "sdhi0_ctrl";
function = "sdhi0";
power-source = <1800>;
@@ -291,8 +291,8 @@
};
&sdhi0 {
- pinctrl-0 = <&sdhi0_pins_3v3>;
- pinctrl-1 = <&sdhi0_pins_1v8>;
+ pinctrl-0 = <&sdhi0_pins>;
+ pinctrl-1 = <&sdhi0_pins_uhs>;
pinctrl-names = "default", "state_uhs";
vmmc-supply = <&vcc_sdhi0>;
^ permalink raw reply related
* Re: [PATCH v2 3/4] Device bindings documentation updated ACPI-enabled platforms not currently supported
From: Luis Oliveira @ 2016-11-08 14:18 UTC (permalink / raw)
To: Andy Shevchenko, Luis Oliveira, Wolfram Sang, Ramiro Oliveira
Cc: Mark Rutland, jarkko.nikula, mika.westerberg, linux-i2c,
linux-kernel, robh+dt, devicetree, CARLOS.PALMINHA
In-Reply-To: <1477047272.6423.6.camel@linux.intel.com>
Hi,
As you suggested I will split the drivers. I am thinking of doing 5 patches:
- factor out master() parts
- separate Master part to i2c-designware-master.c (changes in i2c-designware-core.c)
- enable Slave part to i2c-designware-slave (changes in i2c-designware-core.c)
- glue drivers and device bindings
- cleaning
Regards,
Luis
On 21-Oct-16 11:54, Andy Shevchenko wrote:
> On Fri, 2016-10-21 at 10:56 +0100, Luis Oliveira wrote:
>> Since practically 90% of the code is shared between master and slave,
>> I was
>> thinking if it will be acceptable to use the same driver for both but
>> differentiate the master/slave mode by the compatible strings.
> It might be possible to split like other drivers do:
>
> 1. Core part (i2c-designware-core.c)
> 2. Master part (i2c-designware-master.c)
> 3. Slave part (i2c-designware-slave.c)
> 4. Glue drivers (like: i2c-designware-platdrv.c)
>
>> Thanks,
>> Luis
>>
>> On 10/18/2016 16:17, Wolfram Sang wrote:
>>>> This is needed because the configuration is different and the i2c-
>>>> designware
>>>> cannot be master/slave without a reset. To resolve that I added
>>>> this property
>>>> to bind it as a slave when needed.
>>> Aww, pity that the HW can't do that. Do you have details why?
>>>
>>> If that is really a HW limitation, then I'd suggest having a
>>> seperate
>>> driver for slave-only mode so we can differentiate by compatible
>>> strings.
>>>
>>
^ permalink raw reply
* Re: [PATCH v5 6/8] Documentation: bindings: add compatible specific to legacy SCPI protocol
From: Sudeep Holla @ 2016-11-08 14:32 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, Neil Armstrong, linux-kernel, Sudeep Holla,
Olof Johansson, linux-amlogic, linux-arm-kernel
In-Reply-To: <1478148731-11712-7-git-send-email-sudeep.holla@arm.com>
Hi Rob,
On 03/11/16 04:52, Sudeep Holla wrote:
> This patch adds specific compatible to support legacy SCPI protocol.
>
Sorry for messing it up before, I think this version is much better.
Only this patch introduces new compatible, while 5,7,8/8 are just
reorganization to move the platform specific stuff out of the generic
SCPI bindings. It would be good if you can have a quick look and
provide ack if you are happy with these patches.
--
Regards,
Sudeep
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> Documentation/devicetree/bindings/arm/arm,scpi.txt | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/arm,scpi.txt b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> index d1882c4540d0..ebd03fc93135 100644
> --- a/Documentation/devicetree/bindings/arm/arm,scpi.txt
> +++ b/Documentation/devicetree/bindings/arm/arm,scpi.txt
> @@ -7,7 +7,9 @@ by Linux to initiate various system control and power operations.
>
> Required properties:
>
> -- compatible : should be "arm,scpi"
> +- compatible : should be
> + * "arm,scpi" : For implementations complying to SCPI v1.0 or above
> + * "arm,legacy-scpi" : For implementations complying pre SCPI v1.0
> - mboxes: List of phandle and mailbox channel specifiers
> All the channels reserved by remote SCP firmware for use by
> SCPI message protocol should be specified in any order
>
^ permalink raw reply
* Re: [PATCH v5 1/7] drm: sunxi: Add a basic DRM driver for Allwinner DE2
From: Jean-Francois Moine @ 2016-11-08 14:37 UTC (permalink / raw)
To: Maxime Ripard
Cc: Dave Airlie, Liam Girdwood, Mark Brown, Rob Herring,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
devicetree-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20161107223741.gjxj4tqwuxud2iqc@lukather>
On Mon, 7 Nov 2016 23:37:41 +0100
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Hi,
>
> On Fri, Oct 28, 2016 at 07:34:20PM +0200, Jean-Francois Moine wrote:
> > On Fri, 28 Oct 2016 00:03:16 +0200
> > Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
[snip]
> > > > > We've been calling them bus and mod.
> > > >
> > > > I can understand "bus" (which is better than "apb"), but why "mod"?
> > >
> > > Allwinner has been calling the clocks that are supposed to generate
> > > the external signals (depending on where you were looking) module or
> > > mod clocks (which is also why we have mod in the clock
> > > compatibles). The module 1 clocks being used for the audio and the
> > > module 0 for the rest (SPI, MMC, NAND, display, etc.)
> >
> > I did not find any 'module' in the H3 documentation.
> > So, is it really a good name?
>
> It's true that they use it less nowadays, but they still do,
> ie. https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/clk/sunxi/clk-sun8iw7.c#L513
There is a 'mod' suffix, but it is used for the bus gates only, not for
the main clocks.
> And we have to remain consistent anyway.
I don't see any consistency in the H3 DT:
- the bus gates are named "ahb" and apb"
- the (main) clocks are named "mmc", "usb0_phy" and "ir"
There is no "bus" nor "mod".
> > > > > > +
> > > > > > +- resets: phandle to the reset of the device
> > > > > > +
> > > > > > +- ports: phandle's to the LCD ports
> > > > >
> > > > > Please use the OF graph.
> > > >
> > > > These ports are references to the graph of nodes. See
> > > > http://www.kernelhub.org/?msg=911825&p=2
> > >
> > > In an OF-graph, your phandle to the LCD controller would be replaced
> > > by an output endpoint.
> >
> > This is the DE controller. There is no endpoint link at this level.
>
> The display engine definitely has an endpoint: the TCON.
Not at all. The video chain is simply
CRTC (TCON) -> connector (HDMI/LCD/DAC/..)
The DE is an ancillary device which handles the planes.
> > The Device Engine just handles the planes of the LCDs, but, indeed,
> > the LCDs must know about the DE and the DE must know about the LCDs.
> > There are 2 ways to realize this knowledge in the DT:
> > 1) either the DE has one or two phandle's to the LCDs,
> > 2) or the LCDs have a phandle to the DE.
> >
> > I chose the 1st way, the DE ports pointing to the endpoint of the LCDs
> > which is part of the video link (OF-graph LCD <-> connector).
> > It would be possible to have phandles to the LCDs themselves, but this
> > asks for more code.
> >
> > The second way is also possible, but it also complexifies a bit the
> > exchanges DE <-> LCD.
>
> I'm still not sure how it would complexify anything, and why you can't
> use the display graph to model the relation between the display engine
> and the TCON (and why you want to use a generic property that refers
> to the of-graph while it really isn't).
Complexification:
1- my solution:
At startup time, the DE device is the DRM device. It has to know the
devices entering in the video chains.
The CRTCs (LCD/TCON) are found by
ports[i] -> parent
The connectors are found by
ports[i] -> endpoint -> remote_endpoint -> parent
2- with ports pointing to the LCDs:
The CRTCs (LCD/TCON) are simply
ports[i]
The connectors are found by
loop on all ports of ports[i]
ports[i][j] -> endpoint -> remote_endpoint -> parent
3- with a phandle to the DE in the LCDs:
The DE cannot be the DRM device because there is no information about
the video devices in the DT. Then, the DRM devices are the LCDs.
These LCDs must give their indices to the DE. So, the DE must implement
some callback function to accept a LCD definition, and there must be
a list of DEs in the driver to make the association DE <-> LCD[i]
Some more problem may be raised if a user wants to have the same frame
buffer on the 2 LCDs of a DE.
Anyway, my solution is already used in the IMX Soc.
See 'display-subsystem' in arch/arm/boot/dts/imx6q.dtsi for an example.
> > > > > > +void de2_disable_vblank(struct drm_device *drm, unsigned crtc)
> > > > > > +{
> > > > > > + struct priv *priv = drm->dev_private;
> > > > > > + struct lcd *lcd = priv->lcds[crtc];
> > > > > > +
> > > > > > + tcon_write(lcd->mmio, gint0,
> > > > > > + tcon_read(lcd->mmio, gint0) &
> > > > > > + ~TCON_GINT0_TCON1_Vb_Int_En);
> > > > > > +}
> > > > > > +
> > > > > > +/* panel functions */
> > > > >
> > > > > Panel functions? In the CRTC driver?
> > > >
> > > > Yes, dumb panel.
> > >
> > > What do you mean by that? Using a Parallel/RGB interface?
> >
> > Sorry, I though this was a well-known name. The 'dump panel' was used
> > in the documentation of my previous ARM machine as the video frame sent
> > to the HDMI controller. 'video_frame' is OK for you?
>
> If it's the frame sent to the encoder, then it would be the CRTC by
> DRM's nomenclature.
The CRTC is a software entity. The frame buffer is a hardware entity.
> > > > > > +static const struct {
> > > > > > + char chan;
> > > > > > + char layer;
> > > > > > + char pipe;
> > > > > > +} plane2layer[DE2_N_PLANES] = {
> > > > > > + [DE2_PRIMARY_PLANE] = {0, 0, 0},
> > > > > > + [DE2_CURSOR_PLANE] = {1, 0, 1},
> > > > > > + [DE2_VI_PLANE] = {0, 1, 0},
> > > > > > +};
[snip]
> > > > >
> > > > > Comments?
> > > >
> > > > This
> > > > primary plane is channel 0 (VI), layer 0, pipe 0
> > > > cursor plane is channel 1 (UI), layer 0, pipe 1
> > > > overlay plane is channel 0 (VI), layer 1, pipe 0
> > > > or the full explanation:
> > > > Constraints:
> > > > The VI channels can do RGB or YUV, while UI channels can do RGB
> > > > only.
> > > > The LCD 0 has 1 VI channel and 4 UI channels, while
> > > > LCD 1 has only 1 VI channel and 1 UI channel.
> > > > The cursor must go to a channel bigger than the primary channel,
> > > > otherwise it is not transparent.
> > > > First try:
> > > > Letting the primary plane (usually RGB) in the 2nd channel (UI),
> > > > as this is done in the legacy driver, asks for the cursor to go
> > > > to the next channel (UI), but this one does not exist in LCD1.
> > > > Retained layout:
> > > > So, we must use only 2 channels for the same behaviour on LCD0
> > > > (H3) and LCD1 (A83T)
> > > > The retained combination is:
> > > > - primary plane in the first channel (VI),
> > > > - cursor plane inthe 2nd channel (UI), and
> > > > - overlay plane in the 1st channel (VI).
> > > >
> > > > Note that there could be 3 overlay planes (a channel has 4
> > > > layers), but I am not sure that the A83T or the H3 could
> > > > support 3 simultaneous video streams...
> > >
> > > Do you know if the pipe works in the old display engine?
> > >
> > > Especially about the two-steps composition that wouldn't allow you to
> > > have alpha on all the planes?
> > >
> > > If it is similar, I think hardcoding the pipe number is pretty bad,
> > > because that would restrict the combination of planes and formats,
> > > while some other might have worked.
> >
> > From what I understood about the DE2, the pipes just define the priority
> > of the overlay channels (one pipe for one channel).
> > With the cursor constraint, there must be at least 2 channels in
> > order (primary, cursor). Then, with these 2 channels/pipes, there can be
> > 6 so-called overlay planes (3 RGB/YUV and 3 RGB only).
> > Enabling the pipes 2 and 3 (LCD 0 only) would offer 8 more planes, but
> > RGB only. Then, it might be useful to have dynamic pipes.
>
> That's very valuable (and definitely should go into a comment),
> thanks!
>
> I still believe that's it should be into a (simple at first)
> atomic_check. That would be easier to extend and quite easy to
> document and get simply by looking at the code.
Sorry for I don't understand what you mean.
> > > > > > +static int __init de2_drm_init(void)
> > > > > > +{
> > > > > > + int ret;
> > > > > > +
> > > > > > +/* uncomment to activate the drm traces at startup time */
> > > > > > +/* drm_debug = DRM_UT_CORE | DRM_UT_DRIVER | DRM_UT_KMS |
> > > > > > + DRM_UT_PRIME | DRM_UT_ATOMIC; */
> > > > >
> > > > > That's useless.
> > > >
> > > > Right, but it seems that some people don't know how to debug a DRM
> > > > driver. This is only a reminder.
> > > >
> > > > > > + DRM_DEBUG_DRIVER("\n");
> > > > > > +
> > > > > > + ret = platform_driver_register(&de2_lcd_platform_driver);
> > > > > > + if (ret < 0)
> > > > > > + return ret;
> > > > > > +
> > > > > > + ret = platform_driver_register(&de2_drm_platform_driver);
> > > > > > + if (ret < 0)
> > > > > > + platform_driver_unregister(&de2_lcd_platform_driver);
> > > > > > +
> > > > > > + return ret;
> > > > > > +}
> > > > >
> > > > > And that really shouldn't be done that way.
> > > >
> > > > May you explain?
> > >
> > > This goes against the whole idea of the device and driver
> > > model. Drivers should only register themselves, device should be
> > > created by buses (or by using some external components if the bus
> > > can't: DT, ACPI, etc.). If there's a match, you get probed.
> > >
> > > A driver that creates its own device just to probe itself violates
> > > that.
> >
> > In this function (module init), there is no driver yet.
> > The module contains 2 drivers: the DE (planes) and the LCD (CRTC),
> > and there is no macro to handle such modules.
>
> Ah, yes, my bad. I thought you were registering a device and a
> driver. Still this is a very unusual pattern. Why do you need to split
> the two? Can't you just merge them?
The DE and the LCDs are different devices on different drivers.
A DE must be only one device because it has to handle concurent
accesses from its 2 LCDs. Then 2 drivers.
But only one module. Why? Because there cannot be double direction
calls from one module to an other one, and, in our case, for example,
- the DRM (DE) device must call vblank functions which are handled in
the CRTC (TCON) device, and
- the CRTC device must call DE initialization functions at startup time.
> > > > > > +int de2_plane_init(struct drm_device *drm, struct lcd *lcd)
> > > > > > +{
> > > > > > + int ret, possible_crtcs = 1 << lcd->crtc_idx;
> > > > > > +
> > > > > > + ret = de2_one_plane_init(drm, &lcd->planes[DE2_PRIMARY_PLANE],
> > > > > > + DRM_PLANE_TYPE_PRIMARY, possible_crtcs,
> > > > > > + ui_formats, ARRAY_SIZE(ui_formats));
> > > > > > + if (ret >= 0)
> > > > > > + ret = de2_one_plane_init(drm, &lcd->planes[DE2_CURSOR_PLANE],
> > > > > > + DRM_PLANE_TYPE_CURSOR, possible_crtcs,
> > > > > > + ui_formats, ARRAY_SIZE(ui_formats));
> > > > >
> > > > > Nothing looks really special about that cursor plane. Any reasion not
> > > > > to make it an overlay?
> > > >
> > > > As explained above (channel/layer/pipe plane definitions), the cursor
> > > > cannot go in a channel lower or equal to the one of the primary plane.
> > > > Then, it must be known and, so, have an explicit plane.
> > >
> > > If you were to make it a plane, you could use atomic_check to check
> > > this and make sure this doesn't happen. And you would gain a generic
> > > plane that can be used for other purposes if needed.
> >
> > The function drm_crtc_init_with_planes() offers a cursor plane for free.
> > On the other side, having 6 overlay planes is more than the SoCs can
> > support.
>
> It's not really for free, it costs you a generic plane that could
> definitely be used for something else and cannot anymore because
> they've been hardcoded to a cursor.
>
> And having a camera, the VPU or even an application directly output
> directly into one of these planes seems a much better use of a generic
> plane than a cursor.
Looking at the harder case (A83T), there may be 8 planes on 2 channels.
Using a primary plane and the cursor,
8 planes - primary plane - cursor plane = 6 planes
6 planes are available.
If I count correctly, in your example:
one camera + one VPU + one application = 3 planes
3 planes are used.
So, 3 planes are still available.
On the other side, removing the cursor would just let one more plane.
Do we really need this one? In other words, I'd be pleased to know how
you run 7 applications doing video overlay.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
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