* Re: [PATCH 4/4] sc16is7xx: Use threaded IRQ
From: Daniel Mack @ 2020-05-17 20:44 UTC (permalink / raw)
To: Maarten Brock
Cc: devicetree, linux-serial, gregkh, robh+dt, jslaby, pascal.huerst,
linux-serial-owner
In-Reply-To: <61fdcf12976c924fd86c5203aba673a7@vanmierlo.com>
Hi Maarten,
Thanks for your review!
On 5/9/20 2:55 PM, Maarten Brock wrote:
> On 2020-05-08 16:37, Daniel Mack wrote:
>> Use a threaded IRQ handler to get rid of the irq_work kthread.
>> This also allows for the driver to use interrupts generated by
>> a threaded controller.
>>
>> Signed-off-by: Daniel Mack <daniel@zonque.org>
>> ---
>> drivers/tty/serial/sc16is7xx.c | 18 +++++-------------
>> 1 file changed, 5 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/tty/serial/sc16is7xx.c
>> b/drivers/tty/serial/sc16is7xx.c
>> @@ -1317,8 +1308,9 @@ static int sc16is7xx_probe(struct device *dev,
>> }
>>
>> /* Setup interrupt */
>> - ret = devm_request_irq(dev, irq, sc16is7xx_irq,
>> - IRQF_TRIGGER_FALLING, dev_name(dev), s);
>> + ret = devm_request_threaded_irq(dev, irq, NULL, sc16is7xx_irq,
>> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>> + dev_name(dev), s);
>> if (!ret)
>> return 0;
>
> Since UART0 is first handled completely in the for loop before UART1 is
> handled, a new interrupt may arise on UART0 while UART1 is being handled.
The code in the interrupt handling function loops forever until there is
no more interrupt bits pending. So if there is a new IRQ happening for
UART0 while UART1 is being served, it will be handled in the same loop.
And just to be sure I understand correctly: this is unrelated to the
switch to threaded IRQs, right? Falling edge triggers were always used
for pdata probed devices.
> The result is a missed interrupt since the IRQ line might not *FALL* again.
It doesn't have to. We only exit the interrupt handler when there is
nothing left to do, at which point the IRQ line ist back high. So it
will fall again in case of new events.
> Therefor I suggest to change IRQF_TRIGGER_FALLING to IRQF_TRIGGER_LOW. This
> way the thread will be retriggered after IRQ_HANDLED is returned.
This doesn't work in my setup unfortunately, as the interrupt controller
is incapable of handling level IRQs.
Thanks,
Daniel
^ permalink raw reply
* [PATCH v7 2/7] IIO: Ingenic JZ47xx: Error check clk_enable calls.
From: Artur Rojek @ 2020-05-17 19:48 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Mark Rutland, Jonathan Cameron,
Paul Cercueil, Andy Shevchenko
Cc: Heiko Stuebner, Ezequiel Garcia, linux-input, devicetree,
linux-iio, linux-kernel, Artur Rojek
In-Reply-To: <20200517194904.34758-1-contact@artur-rojek.eu>
Introduce error checks for the clk_enable calls used in this driver.
As part of the changes, move clk_enable/clk_disable calls out of
ingenic_adc_set_config and into respective logic of its callers.
Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
Tested-by: Paul Cercueil <paul@crapouillou.net>
---
Changes:
v6: new patch
v7: no change
drivers/iio/adc/ingenic-adc.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-adc.c
index 39c0a609fc94..6c3bbba7c44b 100644
--- a/drivers/iio/adc/ingenic-adc.c
+++ b/drivers/iio/adc/ingenic-adc.c
@@ -73,7 +73,6 @@ static void ingenic_adc_set_config(struct ingenic_adc *adc,
{
uint32_t cfg;
- clk_enable(adc->clk);
mutex_lock(&adc->lock);
cfg = readl(adc->base + JZ_ADC_REG_CFG) & ~mask;
@@ -81,7 +80,6 @@ static void ingenic_adc_set_config(struct ingenic_adc *adc,
writel(cfg, adc->base + JZ_ADC_REG_CFG);
mutex_unlock(&adc->lock);
- clk_disable(adc->clk);
}
static void ingenic_adc_enable(struct ingenic_adc *adc,
@@ -124,6 +122,8 @@ static int ingenic_adc_write_raw(struct iio_dev *iio_dev,
long m)
{
struct ingenic_adc *adc = iio_priv(iio_dev);
+ struct device *dev = iio_dev->dev.parent;
+ int ret;
switch (m) {
case IIO_CHAN_INFO_SCALE:
@@ -131,6 +131,14 @@ static int ingenic_adc_write_raw(struct iio_dev *iio_dev,
case INGENIC_ADC_BATTERY:
if (!adc->soc_data->battery_vref_mode)
return -EINVAL;
+
+ ret = clk_enable(adc->clk);
+ if (ret) {
+ dev_err(dev, "Failed to enable clock: %d\n",
+ ret);
+ return ret;
+ }
+
if (val > JZ_ADC_BATTERY_LOW_VREF) {
ingenic_adc_set_config(adc,
JZ_ADC_REG_CFG_BAT_MD,
@@ -142,6 +150,9 @@ static int ingenic_adc_write_raw(struct iio_dev *iio_dev,
JZ_ADC_REG_CFG_BAT_MD);
adc->low_vref_mode = true;
}
+
+ clk_disable(adc->clk);
+
return 0;
default:
return -EINVAL;
@@ -317,6 +328,13 @@ static int ingenic_adc_read_chan_info_raw(struct ingenic_adc *adc,
int *val)
{
int bit, ret, engine = (chan->channel == INGENIC_ADC_BATTERY);
+ struct device *dev = iio_priv_to_dev(adc)->dev.parent;
+
+ ret = clk_enable(adc->clk);
+ if (ret) {
+ dev_err(dev, "Failed to enable clock: %d\n", ret);
+ return ret;
+ }
/* We cannot sample AUX/AUX2 in parallel. */
mutex_lock(&adc->aux_lock);
@@ -325,7 +343,6 @@ static int ingenic_adc_read_chan_info_raw(struct ingenic_adc *adc,
ingenic_adc_set_config(adc, JZ_ADC_REG_CFG_AUX_MD, bit);
}
- clk_enable(adc->clk);
ret = ingenic_adc_capture(adc, engine);
if (ret)
goto out;
--
2.26.2
^ permalink raw reply related
* [PATCH v7 4/7] dt-bindings: iio/adc: Add touchscreen idx for JZ47xx SoC ADC
From: Artur Rojek @ 2020-05-17 19:49 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Mark Rutland, Jonathan Cameron,
Paul Cercueil, Andy Shevchenko
Cc: Heiko Stuebner, Ezequiel Garcia, linux-input, devicetree,
linux-iio, linux-kernel, Artur Rojek, Rob Herring
In-Reply-To: <20200517194904.34758-1-contact@artur-rojek.eu>
Introduce support for touchscreen channels found in JZ47xx SoCs.
Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
Tested-by: Paul Cercueil <paul@crapouillou.net>
Acked-by: Rob Herring <robh@kernel.org>
---
Changes:
v2-v7: no change
include/dt-bindings/iio/adc/ingenic,adc.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/dt-bindings/iio/adc/ingenic,adc.h b/include/dt-bindings/iio/adc/ingenic,adc.h
index 42f871ab3272..95e20a8d6dc8 100644
--- a/include/dt-bindings/iio/adc/ingenic,adc.h
+++ b/include/dt-bindings/iio/adc/ingenic,adc.h
@@ -7,5 +7,7 @@
#define INGENIC_ADC_AUX 0
#define INGENIC_ADC_BATTERY 1
#define INGENIC_ADC_AUX2 2
+#define INGENIC_ADC_TOUCH_XP 3
+#define INGENIC_ADC_TOUCH_YP 4
#endif
--
2.26.2
^ permalink raw reply related
* [PATCH v7 5/7] IIO: Ingenic JZ47xx: Add touchscreen mode.
From: Artur Rojek @ 2020-05-17 19:49 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Mark Rutland, Jonathan Cameron,
Paul Cercueil, Andy Shevchenko
Cc: Heiko Stuebner, Ezequiel Garcia, linux-input, devicetree,
linux-iio, linux-kernel, Artur Rojek
In-Reply-To: <20200517194904.34758-1-contact@artur-rojek.eu>
The SADC component in JZ47xx SoCs provides support for touchscreen
operations (pen position and pen down pressure) in single-ended and
differential modes.
Of the known hardware to use this controller, GCW Zero and Anbernic RG-350
utilize the touchscreen mode by having their joystick(s) attached to the
X/Y positive/negative input pins.
GCW Zero comes with a single joystick and is sufficiently handled with the
currently implemented single-ended mode. Support for boards with two
joysticks, where one is hooked up to Xn/Yn and the other to Xp/Yp channels
will need to be provided in the future.
The touchscreen component of SADC takes a significant time to stabilize
after first receiving the clock and a delay of 50ms has been empirically
proven to be a safe value before data sampling can begin.
All the boards which probe this driver have the interrupt provided from
Device Tree, with no need to handle a case where the IRQ was not provided.
Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
Tested-by: Paul Cercueil <paul@crapouillou.net>
---
Changes:
v2: - improve description of the touchscreen mode,
- get rid of the unneeded kfifo,
- drop IIO_BUFFER_CB from Kconfig,
- remove extended names from the touchscreen channels
v3: remove unneeded `linux/iio/kfifo_buf.h` include
v4: clarify irq provider source in the patch description
v5: no change
v6: - correct the spelling of Device Tree and IRQ in commit message
- don't omit trailing commas from initializer lists
- error check `clk_enable`
- remove redundant `dev_err` from `platform_get_irq` error check
v7: no change
drivers/iio/adc/Kconfig | 1 +
drivers/iio/adc/ingenic-adc.c | 115 +++++++++++++++++++++++++++++++++-
2 files changed, 114 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 12bb8b7ca1ff..c3314135fa2a 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -465,6 +465,7 @@ config INA2XX_ADC
config INGENIC_ADC
tristate "Ingenic JZ47xx SoCs ADC driver"
depends on MIPS || COMPILE_TEST
+ select IIO_BUFFER
help
Say yes here to build support for the Ingenic JZ47xx SoCs ADC unit.
diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-adc.c
index 0eee4b4fb96c..49226d8fd8f6 100644
--- a/drivers/iio/adc/ingenic-adc.c
+++ b/drivers/iio/adc/ingenic-adc.c
@@ -8,7 +8,9 @@
#include <dt-bindings/iio/adc/ingenic,adc.h>
#include <linux/clk.h>
+#include <linux/iio/buffer.h>
#include <linux/iio/iio.h>
+#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/iopoll.h>
#include <linux/kernel.h>
@@ -20,6 +22,8 @@
#define JZ_ADC_REG_CFG 0x04
#define JZ_ADC_REG_CTRL 0x08
#define JZ_ADC_REG_STATUS 0x0c
+#define JZ_ADC_REG_ADSAME 0x10
+#define JZ_ADC_REG_ADWAIT 0x14
#define JZ_ADC_REG_ADTCH 0x18
#define JZ_ADC_REG_ADBDAT 0x1c
#define JZ_ADC_REG_ADSDAT 0x20
@@ -28,6 +32,9 @@
#define JZ_ADC_REG_ENABLE_PD BIT(7)
#define JZ_ADC_REG_CFG_AUX_MD (BIT(0) | BIT(1))
#define JZ_ADC_REG_CFG_BAT_MD BIT(4)
+#define JZ_ADC_REG_CFG_PULL_UP(n) ((n) << 16)
+#define JZ_ADC_REG_CFG_SAMPLE_NUM(n) ((n) << 10)
+#define JZ_ADC_REG_CFG_TOUCH_OPS_MASK (BIT(31) | GENMASK(23, 10))
#define JZ_ADC_REG_ADCLK_CLKDIV_LSB 0
#define JZ4725B_ADC_REG_ADCLK_CLKDIV10US_LSB 16
#define JZ4770_ADC_REG_ADCLK_CLKDIV10US_LSB 8
@@ -44,6 +51,14 @@
#define JZ4770_ADC_BATTERY_VREF 6600
#define JZ4770_ADC_BATTERY_VREF_BITS 12
+#define JZ_ADC_IRQ_AUX BIT(0)
+#define JZ_ADC_IRQ_BATTERY BIT(1)
+#define JZ_ADC_IRQ_TOUCH BIT(2)
+#define JZ_ADC_IRQ_PEN_DOWN BIT(3)
+#define JZ_ADC_IRQ_PEN_UP BIT(4)
+#define JZ_ADC_IRQ_PEN_DOWN_SLEEP BIT(5)
+#define JZ_ADC_IRQ_SLEEP BIT(7)
+
struct ingenic_adc;
struct ingenic_adc_soc_data {
@@ -428,6 +443,28 @@ static const struct iio_info ingenic_adc_info = {
};
static const struct iio_chan_spec ingenic_channels[] = {
+ {
+ .type = IIO_POSITIONRELATIVE,
+ .indexed = 1,
+ .channel = INGENIC_ADC_TOUCH_XP,
+ .scan_index = 0,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 12,
+ .storagebits = 16,
+ },
+ },
+ {
+ .type = IIO_POSITIONRELATIVE,
+ .indexed = 1,
+ .channel = INGENIC_ADC_TOUCH_YP,
+ .scan_index = 1,
+ .scan_type = {
+ .sign = 'u',
+ .realbits = 12,
+ .storagebits = 16,
+ },
+ },
{
.extend_name = "aux",
.type = IIO_VOLTAGE,
@@ -435,6 +472,7 @@ static const struct iio_chan_spec ingenic_channels[] = {
BIT(IIO_CHAN_INFO_SCALE),
.indexed = 1,
.channel = INGENIC_ADC_AUX,
+ .scan_index = -1,
},
{
.extend_name = "battery",
@@ -445,6 +483,7 @@ static const struct iio_chan_spec ingenic_channels[] = {
BIT(IIO_CHAN_INFO_SCALE),
.indexed = 1,
.channel = INGENIC_ADC_BATTERY,
+ .scan_index = -1,
},
{ /* Must always be last in the array. */
.extend_name = "aux2",
@@ -453,16 +492,76 @@ static const struct iio_chan_spec ingenic_channels[] = {
BIT(IIO_CHAN_INFO_SCALE),
.indexed = 1,
.channel = INGENIC_ADC_AUX2,
+ .scan_index = -1,
},
};
+static int ingenic_adc_buffer_enable(struct iio_dev *iio_dev)
+{
+ struct ingenic_adc *adc = iio_priv(iio_dev);
+ int ret;
+
+ ret = clk_enable(adc->clk);
+ if (ret) {
+ dev_err(iio_dev->dev.parent, "Failed to enable clock: %d\n",
+ ret);
+ return ret;
+ }
+
+ /* It takes significant time for the touchscreen hw to stabilize. */
+ msleep(50);
+ ingenic_adc_set_config(adc, JZ_ADC_REG_CFG_TOUCH_OPS_MASK,
+ JZ_ADC_REG_CFG_SAMPLE_NUM(4) |
+ JZ_ADC_REG_CFG_PULL_UP(4));
+ writew(80, adc->base + JZ_ADC_REG_ADWAIT);
+ writew(2, adc->base + JZ_ADC_REG_ADSAME);
+ writeb((u8)~JZ_ADC_IRQ_TOUCH, adc->base + JZ_ADC_REG_CTRL);
+ writel(0, adc->base + JZ_ADC_REG_ADTCH);
+ ingenic_adc_enable(adc, 2, true);
+
+ return 0;
+}
+
+static int ingenic_adc_buffer_disable(struct iio_dev *iio_dev)
+{
+ struct ingenic_adc *adc = iio_priv(iio_dev);
+
+ ingenic_adc_enable(adc, 2, false);
+ writeb(0xff, adc->base + JZ_ADC_REG_CTRL);
+ writeb(0xff, adc->base + JZ_ADC_REG_STATUS);
+ ingenic_adc_set_config(adc, JZ_ADC_REG_CFG_TOUCH_OPS_MASK, 0);
+ writew(0, adc->base + JZ_ADC_REG_ADSAME);
+ writew(0, adc->base + JZ_ADC_REG_ADWAIT);
+ clk_disable(adc->clk);
+
+ return 0;
+}
+
+static const struct iio_buffer_setup_ops ingenic_buffer_setup_ops = {
+ .postenable = &ingenic_adc_buffer_enable,
+ .predisable = &ingenic_adc_buffer_disable
+};
+
+static irqreturn_t ingenic_adc_irq(int irq, void *data)
+{
+ struct iio_dev *iio_dev = data;
+ struct ingenic_adc *adc = iio_priv(iio_dev);
+ u32 tdat;
+
+ tdat = readl(adc->base + JZ_ADC_REG_ADTCH);
+ iio_push_to_buffers(iio_dev, &tdat);
+ writeb(JZ_ADC_IRQ_TOUCH, adc->base + JZ_ADC_REG_STATUS);
+
+ return IRQ_HANDLED;
+}
+
static int ingenic_adc_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct iio_dev *iio_dev;
struct ingenic_adc *adc;
const struct ingenic_adc_soc_data *soc_data;
- int ret;
+ int irq, ret;
soc_data = device_get_match_data(dev);
if (!soc_data)
@@ -477,6 +576,17 @@ static int ingenic_adc_probe(struct platform_device *pdev)
mutex_init(&adc->aux_lock);
adc->soc_data = soc_data;
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ ret = devm_request_irq(dev, irq, ingenic_adc_irq, 0,
+ dev_name(dev), iio_dev);
+ if (ret < 0) {
+ dev_err(dev, "Failed to request irq: %d\n", ret);
+ return ret;
+ }
+
adc->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(adc->base))
return PTR_ERR(adc->base);
@@ -516,7 +626,8 @@ static int ingenic_adc_probe(struct platform_device *pdev)
iio_dev->dev.parent = dev;
iio_dev->name = "jz-adc";
- iio_dev->modes = INDIO_DIRECT_MODE;
+ iio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
+ iio_dev->setup_ops = &ingenic_buffer_setup_ops;
iio_dev->channels = ingenic_channels;
iio_dev->num_channels = ARRAY_SIZE(ingenic_channels);
/* Remove AUX2 from the list of supported channels. */
--
2.26.2
^ permalink raw reply related
* [PATCH v7 6/7] dt-bindings: input: Add docs for ADC driven joystick.
From: Artur Rojek @ 2020-05-17 19:49 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Mark Rutland, Jonathan Cameron,
Paul Cercueil, Andy Shevchenko
Cc: Heiko Stuebner, Ezequiel Garcia, linux-input, devicetree,
linux-iio, linux-kernel, Artur Rojek, Rob Herring
In-Reply-To: <20200517194904.34758-1-contact@artur-rojek.eu>
Add documentation for the adc-joystick driver, used to provide support
for joysticks connected over ADC.
Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
Tested-by: Paul Cercueil <paul@crapouillou.net>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changes:
v2: - Add `reg` property to axis subnode in order to enumerate the axes,
- rename `linux,abs-code` property to `linux,code`,
- drop `linux,` prefix from the remaining properties of axis subnode
v3: no change
v4: - remove "bindings" from the unique identifier string,
- replace `|` with `>` for all description properties,
- specify the number of items for `io-channels`,
- correct the regex pattern of `axis` property,
- specify the value range of `reg` property for each axis,
- put `abs-range` properties under `allOf`
v5: add `a-f` to the regex pattern of `axis` property
v6-v7: no change
.../bindings/input/adc-joystick.yaml | 121 ++++++++++++++++++
1 file changed, 121 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/adc-joystick.yaml
diff --git a/Documentation/devicetree/bindings/input/adc-joystick.yaml b/Documentation/devicetree/bindings/input/adc-joystick.yaml
new file mode 100644
index 000000000000..054406bbd22b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/adc-joystick.yaml
@@ -0,0 +1,121 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019-2020 Artur Rojek
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/input/adc-joystick.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: ADC attached joystick
+
+maintainers:
+ - Artur Rojek <contact@artur-rojek.eu>
+
+description: >
+ Bindings for joystick devices connected to ADC controllers supporting
+ the Industrial I/O subsystem.
+
+properties:
+ compatible:
+ const: adc-joystick
+
+ io-channels:
+ minItems: 1
+ maxItems: 1024
+ description: >
+ List of phandle and IIO specifier pairs.
+ Each pair defines one ADC channel to which a joystick axis is connected.
+ See Documentation/devicetree/bindings/iio/iio-bindings.txt for details.
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+required:
+ - compatible
+ - io-channels
+ - '#address-cells'
+ - '#size-cells'
+
+additionalProperties: false
+
+patternProperties:
+ "^axis@[0-9a-f]+$":
+ type: object
+ description: >
+ Represents a joystick axis bound to the given ADC channel.
+ For each entry in the io-channels list, one axis subnode with a matching
+ reg property must be specified.
+
+ properties:
+ reg:
+ minimum: 0
+ maximum: 1023
+ description: Index of an io-channels list entry bound to this axis.
+
+ linux,code:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: EV_ABS specific event code generated by the axis.
+
+ abs-range:
+ allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32-array
+ - items:
+ - description: minimum value
+ - description: maximum value
+ description: >
+ Minimum and maximum values produced by the axis.
+ For an ABS_X axis this will be the left-most and right-most
+ inclination of the joystick. If min > max, it is left to userspace to
+ treat the axis as inverted.
+ This property is interpreted as two signed 32 bit values.
+
+ abs-fuzz:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: >
+ Amount of noise in the input value.
+ Omitting this property indicates the axis is precise.
+
+ abs-flat:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: >
+ Axial "deadzone", or area around the center position, where the axis
+ is considered to be at rest.
+ Omitting this property indicates the axis always returns to exactly
+ the center position.
+
+ required:
+ - reg
+ - linux,code
+ - abs-range
+
+ additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/iio/adc/ingenic,adc.h>
+ #include <dt-bindings/input/input.h>
+
+ joystick: adc-joystick {
+ compatible = "adc-joystick";
+ io-channels = <&adc INGENIC_ADC_TOUCH_XP>,
+ <&adc INGENIC_ADC_TOUCH_YP>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ axis@0 {
+ reg = <0>;
+ linux,code = <ABS_X>;
+ abs-range = <3300 0>;
+ abs-fuzz = <4>;
+ abs-flat = <200>;
+ };
+ axis@1 {
+ reg = <1>;
+ linux,code = <ABS_Y>;
+ abs-range = <0 3300>;
+ abs-fuzz = <4>;
+ abs-flat = <200>;
+ };
+ };
--
2.26.2
^ permalink raw reply related
* [PATCH v7 7/7] input: joystick: Add ADC attached joystick driver.
From: Artur Rojek @ 2020-05-17 19:49 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Mark Rutland, Jonathan Cameron,
Paul Cercueil, Andy Shevchenko
Cc: Heiko Stuebner, Ezequiel Garcia, linux-input, devicetree,
linux-iio, linux-kernel, Artur Rojek
In-Reply-To: <20200517194904.34758-1-contact@artur-rojek.eu>
Add a driver for joystick devices connected to ADC controllers
supporting the Industrial I/O subsystem.
Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
Tested-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
Changes:
v2: - sanity check supported channel format on probe,
- rename adc_joystick_disable to a more sensible adc_joystick_cleanup,
- enforce correct axis order by checking the `reg` property of
child nodes
v3-v5: no change
v6: - remove redundant `<linux/of.h>`
- set `val` for each endianness case in their respective branches
- pass received error codes to return value of `adc_joystick_set_axes`
- change `(bits >> 3) > 2` to `bits > 16` for readability
- drop `of_match_ptr`
v7: no change
drivers/input/joystick/Kconfig | 10 +
drivers/input/joystick/Makefile | 1 +
drivers/input/joystick/adc-joystick.c | 253 ++++++++++++++++++++++++++
3 files changed, 264 insertions(+)
create mode 100644 drivers/input/joystick/adc-joystick.c
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index 940b744639c7..efbc20ec5099 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -42,6 +42,16 @@ config JOYSTICK_A3D
To compile this driver as a module, choose M here: the
module will be called a3d.
+config JOYSTICK_ADC
+ tristate "Simple joystick connected over ADC"
+ depends on IIO
+ select IIO_BUFFER_CB
+ help
+ Say Y here if you have a simple joystick connected over ADC.
+
+ To compile this driver as a module, choose M here: the
+ module will be called adc-joystick.
+
config JOYSTICK_ADI
tristate "Logitech ADI digital joysticks and gamepads"
select GAMEPORT
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 8656023f6ef5..58232b3057d3 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -6,6 +6,7 @@
# Each configuration option enables a list of files.
obj-$(CONFIG_JOYSTICK_A3D) += a3d.o
+obj-$(CONFIG_JOYSTICK_ADC) += adc-joystick.o
obj-$(CONFIG_JOYSTICK_ADI) += adi.o
obj-$(CONFIG_JOYSTICK_AMIGA) += amijoy.o
obj-$(CONFIG_JOYSTICK_AS5011) += as5011.o
diff --git a/drivers/input/joystick/adc-joystick.c b/drivers/input/joystick/adc-joystick.c
new file mode 100644
index 000000000000..a4ba8eac5a12
--- /dev/null
+++ b/drivers/input/joystick/adc-joystick.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Input driver for joysticks connected over ADC.
+ * Copyright (c) 2019-2020 Artur Rojek <contact@artur-rojek.eu>
+ */
+#include <linux/ctype.h>
+#include <linux/input.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/consumer.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+
+struct adc_joystick_axis {
+ u32 code;
+ s32 range[2];
+ s32 fuzz;
+ s32 flat;
+};
+
+struct adc_joystick {
+ struct input_dev *input;
+ struct iio_cb_buffer *buffer;
+ struct adc_joystick_axis *axes;
+ struct iio_channel *chans;
+ int num_chans;
+};
+
+static int adc_joystick_handle(const void *data, void *private)
+{
+ struct adc_joystick *joy = private;
+ enum iio_endian endianness;
+ int bytes, msb, val, i;
+ bool sign;
+
+ bytes = joy->chans[0].channel->scan_type.storagebits >> 3;
+
+ for (i = 0; i < joy->num_chans; ++i) {
+ endianness = joy->chans[i].channel->scan_type.endianness;
+ msb = joy->chans[i].channel->scan_type.realbits - 1;
+ sign = (tolower(joy->chans[i].channel->scan_type.sign) == 's');
+
+ switch (bytes) {
+ case 1:
+ val = ((const u8 *)data)[i];
+ break;
+ case 2:
+ if (endianness == IIO_BE)
+ val = be16_to_cpu(((const u16 *)data)[i]);
+ else if (endianness == IIO_LE)
+ val = le16_to_cpu(((const u16 *)data)[i]);
+ else /* IIO_CPU */
+ val = ((const u16 *)data)[i];
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ val >>= joy->chans[i].channel->scan_type.shift;
+ if (sign)
+ val = sign_extend32(val, msb);
+ else
+ val &= GENMASK(msb, 0);
+ input_report_abs(joy->input, joy->axes[i].code, val);
+ }
+
+ input_sync(joy->input);
+
+ return 0;
+}
+
+static int adc_joystick_open(struct input_dev *dev)
+{
+ struct adc_joystick *joy = input_get_drvdata(dev);
+ int ret;
+
+ ret = iio_channel_start_all_cb(joy->buffer);
+ if (ret)
+ dev_err(dev->dev.parent, "Unable to start callback buffer");
+
+ return ret;
+}
+
+static void adc_joystick_close(struct input_dev *dev)
+{
+ struct adc_joystick *joy = input_get_drvdata(dev);
+
+ iio_channel_stop_all_cb(joy->buffer);
+}
+
+static void adc_joystick_cleanup(void *data)
+{
+ iio_channel_release_all_cb(data);
+}
+
+static int adc_joystick_set_axes(struct device *dev, struct adc_joystick *joy)
+{
+ struct adc_joystick_axis *axes;
+ struct fwnode_handle *child;
+ int num_axes, ret, i;
+
+ num_axes = device_get_child_node_count(dev);
+ if (!num_axes) {
+ dev_err(dev, "Unable to find child nodes");
+ return -EINVAL;
+ }
+
+ if (num_axes != joy->num_chans) {
+ dev_err(dev, "Got %d child nodes for %d channels",
+ num_axes, joy->num_chans);
+ return -EINVAL;
+ }
+
+ axes = devm_kmalloc_array(dev, num_axes, sizeof(*axes), GFP_KERNEL);
+ if (!axes)
+ return -ENOMEM;
+
+ device_for_each_child_node(dev, child) {
+ ret = fwnode_property_read_u32(child, "reg", &i);
+ if (ret) {
+ dev_err(dev, "reg invalid or missing");
+ goto err;
+ }
+
+ if (i >= num_axes) {
+ ret = -EINVAL;
+ dev_err(dev, "No matching axis for reg %d", i);
+ goto err;
+ }
+
+ ret = fwnode_property_read_u32(child, "linux,code",
+ &axes[i].code);
+ if (ret) {
+ dev_err(dev, "linux,code invalid or missing");
+ goto err;
+ }
+
+ ret = fwnode_property_read_u32_array(child, "abs-range",
+ axes[i].range, 2);
+ if (ret) {
+ dev_err(dev, "abs-range invalid or missing");
+ goto err;
+ }
+
+ fwnode_property_read_u32(child, "abs-fuzz",
+ &axes[i].fuzz);
+ fwnode_property_read_u32(child, "abs-flat",
+ &axes[i].flat);
+
+ input_set_abs_params(joy->input, axes[i].code,
+ axes[i].range[0], axes[i].range[1],
+ axes[i].fuzz,
+ axes[i].flat);
+ input_set_capability(joy->input, EV_ABS, axes[i].code);
+ }
+
+ joy->axes = axes;
+
+ return 0;
+
+err:
+ fwnode_handle_put(child);
+ return ret;
+}
+
+static int adc_joystick_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct adc_joystick *joy;
+ struct input_dev *input;
+ int bits, ret, i;
+
+ joy = devm_kzalloc(dev, sizeof(*joy), GFP_KERNEL);
+ if (!joy)
+ return -ENOMEM;
+
+ joy->chans = devm_iio_channel_get_all(dev);
+ if (IS_ERR(joy->chans)) {
+ ret = PTR_ERR(joy->chans);
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Unable to get IIO channels");
+ return ret;
+ }
+
+ /* Count how many channels we got. NULL terminated. */
+ while (joy->chans[joy->num_chans].indio_dev)
+ joy->num_chans++;
+
+ bits = joy->chans[0].channel->scan_type.storagebits;
+ if (!bits || (bits > 16)) {
+ dev_err(dev, "Unsupported channel storage size");
+ return -EINVAL;
+ }
+ for (i = 1; i < joy->num_chans; ++i)
+ if (joy->chans[i].channel->scan_type.storagebits != bits) {
+ dev_err(dev, "Channels must have equal storage size");
+ return -EINVAL;
+ }
+
+ input = devm_input_allocate_device(dev);
+ if (!input) {
+ dev_err(dev, "Unable to allocate input device");
+ return -ENOMEM;
+ }
+
+ joy->input = input;
+ input->name = pdev->name;
+ input->id.bustype = BUS_HOST;
+ input->open = adc_joystick_open;
+ input->close = adc_joystick_close;
+
+ ret = adc_joystick_set_axes(dev, joy);
+ if (ret)
+ return ret;
+
+ input_set_drvdata(input, joy);
+ ret = input_register_device(input);
+ if (ret) {
+ dev_err(dev, "Unable to register input device: %d", ret);
+ return ret;
+ }
+
+ joy->buffer = iio_channel_get_all_cb(dev, adc_joystick_handle, joy);
+ if (IS_ERR(joy->buffer)) {
+ dev_err(dev, "Unable to allocate callback buffer");
+ return PTR_ERR(joy->buffer);
+ }
+
+ ret = devm_add_action_or_reset(dev, adc_joystick_cleanup, joy->buffer);
+ if (ret)
+ dev_err(dev, "Unable to add action");
+
+ return ret;
+}
+
+static const struct of_device_id adc_joystick_of_match[] = {
+ { .compatible = "adc-joystick", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, adc_joystick_of_match);
+
+static struct platform_driver adc_joystick_driver = {
+ .driver = {
+ .name = "adc-joystick",
+ .of_match_table = adc_joystick_of_match,
+ },
+ .probe = adc_joystick_probe,
+};
+module_platform_driver(adc_joystick_driver);
+
+MODULE_DESCRIPTION("Input driver for joysticks connected over ADC");
+MODULE_AUTHOR("Artur Rojek <contact@artur-rojek.eu>");
+MODULE_LICENSE("GPL");
--
2.26.2
^ permalink raw reply related
* [PATCH v7 3/7] IIO: Ingenic JZ47xx: Add xlate cb to retrieve correct channel idx
From: Artur Rojek @ 2020-05-17 19:49 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Mark Rutland, Jonathan Cameron,
Paul Cercueil, Andy Shevchenko
Cc: Heiko Stuebner, Ezequiel Garcia, linux-input, devicetree,
linux-iio, linux-kernel, Artur Rojek
In-Reply-To: <20200517194904.34758-1-contact@artur-rojek.eu>
Provide an of_xlate callback in order to retrieve the correct channel
specifier index from the IIO channels array.
Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
Tested-by: Paul Cercueil <paul@crapouillou.net>
---
Changes:
v2-v7: no change
drivers/iio/adc/ingenic-adc.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-adc.c
index 6c3bbba7c44b..0eee4b4fb96c 100644
--- a/drivers/iio/adc/ingenic-adc.c
+++ b/drivers/iio/adc/ingenic-adc.c
@@ -400,6 +400,21 @@ static int ingenic_adc_read_raw(struct iio_dev *iio_dev,
}
}
+static int ingenic_adc_of_xlate(struct iio_dev *iio_dev,
+ const struct of_phandle_args *iiospec)
+{
+ int i;
+
+ if (!iiospec->args_count)
+ return -EINVAL;
+
+ for (i = 0; i < iio_dev->num_channels; ++i)
+ if (iio_dev->channels[i].channel == iiospec->args[0])
+ return i;
+
+ return -EINVAL;
+}
+
static void ingenic_adc_clk_cleanup(void *data)
{
clk_unprepare(data);
@@ -409,6 +424,7 @@ static const struct iio_info ingenic_adc_info = {
.write_raw = ingenic_adc_write_raw,
.read_raw = ingenic_adc_read_raw,
.read_avail = ingenic_adc_read_avail,
+ .of_xlate = ingenic_adc_of_xlate,
};
static const struct iio_chan_spec ingenic_channels[] = {
--
2.26.2
^ permalink raw reply related
* [PATCH v7 1/7] dt-bindings: iio/adc: Convert ingenic-adc docs to YAML.
From: Artur Rojek @ 2020-05-17 19:48 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Mark Rutland, Jonathan Cameron,
Paul Cercueil, Andy Shevchenko
Cc: Heiko Stuebner, Ezequiel Garcia, linux-input, devicetree,
linux-iio, linux-kernel, Artur Rojek
Convert the textual documentation of Device Tree bindings for the
Ingenic JZ47xx SoCs ADC controller to YAML.
The `interrupts` property is now explicitly listed and marked as
required. While missing from the previous textual documentation, this
property has been used with all the boards which probe this driver.
Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
Tested-by: Paul Cercueil <paul@crapouillou.net>
---
Changes:
v6: new patch
v7: - specify `maxItems: 1` for single entry properties
- get rid of redundant descriptions of said properties
.../bindings/iio/adc/ingenic,adc.txt | 49 -------------
.../bindings/iio/adc/ingenic,adc.yaml | 71 +++++++++++++++++++
2 files changed, 71 insertions(+), 49 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/iio/adc/ingenic,adc.txt
create mode 100644 Documentation/devicetree/bindings/iio/adc/ingenic,adc.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/ingenic,adc.txt b/Documentation/devicetree/bindings/iio/adc/ingenic,adc.txt
deleted file mode 100644
index cd9048cf9dcf..000000000000
--- a/Documentation/devicetree/bindings/iio/adc/ingenic,adc.txt
+++ /dev/null
@@ -1,49 +0,0 @@
-* Ingenic JZ47xx ADC controller IIO bindings
-
-Required properties:
-
-- compatible: Should be one of:
- * ingenic,jz4725b-adc
- * ingenic,jz4740-adc
- * ingenic,jz4770-adc
-- reg: ADC controller registers location and length.
-- clocks: phandle to the SoC's ADC clock.
-- clock-names: Must be set to "adc".
-- #io-channel-cells: Must be set to <1> to indicate channels are selected
- by index.
-
-ADC clients must use the format described in iio-bindings.txt, giving
-a phandle and IIO specifier pair ("io-channels") to the ADC controller.
-
-Example:
-
-#include <dt-bindings/iio/adc/ingenic,adc.h>
-
-adc: adc@10070000 {
- compatible = "ingenic,jz4740-adc";
- #io-channel-cells = <1>;
-
- reg = <0x10070000 0x30>;
-
- clocks = <&cgu JZ4740_CLK_ADC>;
- clock-names = "adc";
-
- interrupt-parent = <&intc>;
- interrupts = <18>;
-};
-
-adc-keys {
- ...
- compatible = "adc-keys";
- io-channels = <&adc INGENIC_ADC_AUX>;
- io-channel-names = "buttons";
- ...
-};
-
-battery {
- ...
- compatible = "ingenic,jz4740-battery";
- io-channels = <&adc INGENIC_ADC_BATTERY>;
- io-channel-names = "battery";
- ...
-};
diff --git a/Documentation/devicetree/bindings/iio/adc/ingenic,adc.yaml b/Documentation/devicetree/bindings/iio/adc/ingenic,adc.yaml
new file mode 100644
index 000000000000..9f414dbdae86
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ingenic,adc.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019-2020 Artur Rojek
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/iio/adc/ingenic,adc.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Ingenic JZ47xx ADC controller IIO bindings
+
+maintainers:
+ - Artur Rojek <contact@artur-rojek.eu>
+
+description: >
+ Industrial I/O subsystem bindings for ADC controller found in
+ Ingenic JZ47xx SoCs.
+
+ ADC clients must use the format described in iio-bindings.txt, giving
+ a phandle and IIO specifier pair ("io-channels") to the ADC controller.
+
+properties:
+ compatible:
+ enum:
+ - ingenic,jz4725b-adc
+ - ingenic,jz4740-adc
+ - ingenic,jz4770-adc
+
+ '#io-channel-cells':
+ const: 1
+ description:
+ Must be set to <1> to indicate channels are selected by index.
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: adc
+
+ interrupts:
+ maxItems: 1
+
+required:
+ - compatible
+ - '#io-channel-cells'
+ - reg
+ - clocks
+ - clock-names
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/jz4740-cgu.h>
+ #include <dt-bindings/iio/adc/ingenic,adc.h>
+
+ adc@10070000 {
+ compatible = "ingenic,jz4740-adc";
+ #io-channel-cells = <1>;
+
+ reg = <0x10070000 0x30>;
+
+ clocks = <&cgu JZ4740_CLK_ADC>;
+ clock-names = "adc";
+
+ interrupt-parent = <&intc>;
+ interrupts = <18>;
+ };
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v2 5/6] dmaengine: dw: Introduce max burst length hw config
From: Serge Semin @ 2020-05-17 19:38 UTC (permalink / raw)
To: Vinod Koul
Cc: Serge Semin, Andy Shevchenko, Viresh Kumar, Dan Williams,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Rob Herring, linux-mips, devicetree, dmaengine,
linux-kernel
In-Reply-To: <20200515063950.GI333670@vkoul-mobl>
On Fri, May 15, 2020 at 12:09:50PM +0530, Vinod Koul wrote:
> On 12-05-20, 22:12, Andy Shevchenko wrote:
> > On Tue, May 12, 2020 at 05:08:20PM +0300, Serge Semin wrote:
> > > On Fri, May 08, 2020 at 02:41:53PM +0300, Andy Shevchenko wrote:
> > > > On Fri, May 08, 2020 at 01:53:03PM +0300, Serge Semin wrote:
> > > > > IP core of the DW DMA controller may be synthesized with different
> > > > > max burst length of the transfers per each channel. According to Synopsis
> > > > > having the fixed maximum burst transactions length may provide some
> > > > > performance gain. At the same time setting up the source and destination
> > > > > multi size exceeding the max burst length limitation may cause a serious
> > > > > problems. In our case the system just hangs up. In order to fix this
> > > > > lets introduce the max burst length platform config of the DW DMA
> > > > > controller device and don't let the DMA channels configuration code
> > > > > exceed the burst length hardware limitation. Depending on the IP core
> > > > > configuration the maximum value can vary from channel to channel.
> > > > > It can be detected either in runtime from the DWC parameter registers
> > > > > or from the dedicated dts property.
> > > >
> > > > I'm wondering what can be the scenario when your peripheral will ask something
> > > > which is not supported by DMA controller?
> > >
> > > I may misunderstood your statement, because seeing your activity around my
> > > patchsets including the SPI patchset and sometimes very helpful comments,
> > > this question answer seems too obvious to see you asking it.
> > >
> > > No need to go far for an example. See the DW APB SSI driver. Its DMA module
> > > specifies the burst length to be 16, while not all of ours channels supports it.
> > > Yes, originally it has been developed for the Intel Midfield SPI, but since I
> > > converted the driver into a generic code we can't use a fixed value. For instance
> > > in our hardware only two DMA channels of total 16 are capable of bursting up to
> > > 16 bytes (data items) at a time, the rest of them are limited with up to 4 bytes
> > > burst length. While there are two SPI interfaces, each of which need to have two
> > > DMA channels for communications. So I need four channels in total to allocate to
> > > provide the DMA capability for all interfaces. In order to set the SPI controller
> > > up with valid optimized parameters the max-burst-length is required. Otherwise we
> > > can end up with buffers overrun/underrun.
> >
> > Right, and we come to the question which channel better to be used by SPI and
> > the rest devices. Without specific filter function you can easily get into a
> > case of inverted optimizations, when SPI got channels with burst = 4, while
> > it's needed 16, and other hardware otherwise. Performance wise it's worse
> > scenario which we may avoid in the first place, right?
>
> If one has channels which are different and described as such in DT,
> then I think it does make sense to specify in your board-dt about the
> specific channels you would require...
Well, we do have such hardware. Our DW DMA controller has got different max
burst lengths assigned to first two and the rest of the channels. But creating
a functionality of the individual channels assignment is a matter of different
patchset. Sorry. It's not one of my task at the moment.
My primary task is to integrate the Baikal-T1 SoC support into the kernel. I've
refactored a lot of code found in the Baikal-T1 SDK and currently under a pressure
of a lot of review. Alas there is no time to create new functionality as you
suggest. In future I may provide such, but not in the framework of this patchset.
> >
> > > > Peripheral needs to supply a lot of configuration parameters specific to the
> > > > DMA controller in use (that's why we have struct dw_dma_slave).
> > > > So, seems to me the feasible approach is supply correct data in the first place.
> > >
> > > How to supply a valid data if clients don't know the DMA controller limitations
> > > in general?
> >
> > This is a good question. DMA controllers are quite different and having unified
> > capabilities structure for all is almost impossible task to fulfil. That's why
> > custom filter function(s) can help here. Based on compatible string you can
> > implement whatever customized quirks like two functions, for example, to try 16
> > burst size first and fallback to 4 if none was previously found.
> >
> > > > If you have specific channels to acquire then you probably need to provide a
> > > > custom xlate / filter functions. Because above seems a bit hackish workaround
> > > > of dynamic channel allocation mechanism.
> > >
> > > No, I don't have a specific channel to acquire and in general you may use any
> > > returned from the DMA subsystem (though some platforms may need a dedicated
> > > channels to use, in this case xlate / filter is required). In our SoC any DW DMAC
> > > channel can be used for any DMA-capable peripherals like SPI, I2C, UART. But the
> > > their DMA settings must properly and optimally configured. It can be only done
> > > if you know the DMA controller parameters like max burst length, max block-size,
> > > etc.
> > >
> > > So no. The change proposed by this patch isn't workaround, but a useful feature,
> > > moreover expected to be supported by the generic DMA subsystem.
> >
> > See above.
> >
> > > > But let's see what we can do better. Since maximum is defined on the slave side
> > > > device, it probably needs to define minimum as well, otherwise it's possible
> > > > that some hardware can't cope underrun bursts.
> > >
> > > There is no need to define minimum if such limit doesn't exists except a
> > > natural 1. Moreover it doesn't exist for all DMA controllers seeing noone has
> > > added such capability into the generic DMA subsystem so far.
> >
> > There is a contract between provider and consumer about DMA resource. That's
> > why both sides should participate in fulfilling it. Theoretically it may be a
> > hardware that doesn't support minimum burst available in DMA by a reason. For
> > such we would need minimum to be provided as well.
>
> Agreed and if required caps should be extended to tell consumer the
> minimum values supported.
Sorry, it's not required by our hardware. Is there any, which actually has such
limitation? (minimum burst length)
-Sergey
>
> --
> ~Vinod
^ permalink raw reply
* Re: [PATCH 28/28] dt-bindings: usb: Convert ehci-mv to json-schema
From: Lubomir Rintel @ 2020-05-17 19:34 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Bartosz Golaszewski, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Mauro Carvalho Chehab, Ulf Hansson,
Kishon Vijay Abraham I, Alessandro Zummo, Alexandre Belloni,
Greg Kroah-Hartman, Mark Brown, Daniel Lezcano, Andrew Lunn,
Gregory Clement, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
devicetree, linux-kernel, linux-gpio, linux-i2c, linux-media,
linux-mmc, linux-rtc, linux-serial, linux-spi, linux-usb
In-Reply-To: <20200327195520.GA2235@bogus>
On Fri, Mar 27, 2020 at 01:55:20PM -0600, Rob Herring wrote:
> On Tue, Mar 17, 2020 at 10:39:22AM +0100, Lubomir Rintel wrote:
> > A straightforward conversion of the ehci-mv binding to DT schema format
> > using json-schema.
> >
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > ---
> > .../devicetree/bindings/usb/ehci-mv.txt | 23 -------
> > .../bindings/usb/marvell,pxau2o-ehci.yaml | 60 +++++++++++++++++++
> > 2 files changed, 60 insertions(+), 23 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/usb/ehci-mv.txt
> > create mode 100644 Documentation/devicetree/bindings/usb/marvell,pxau2o-ehci.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/usb/ehci-mv.txt b/Documentation/devicetree/bindings/usb/ehci-mv.txt
> > deleted file mode 100644
> > index 335589895763e..0000000000000
> > --- a/Documentation/devicetree/bindings/usb/ehci-mv.txt
> > +++ /dev/null
> > @@ -1,23 +0,0 @@
> > -* Marvell PXA/MMP EHCI controller.
> > -
> > -Required properties:
> > -
> > -- compatible: must be "marvell,pxau2o-ehci"
> > -- reg: physical base addresses of the controller and length of memory mapped region
> > -- interrupts: one EHCI controller interrupt should be described here
> > -- clocks: phandle list of usb clocks
> > -- clock-names: should be "USBCLK"
> > -- phys: phandle for the PHY device
> > -- phy-names: should be "usb"
> > -
> > -Example:
> > -
> > - ehci0: usb-ehci@d4208000 {
> > - compatible = "marvell,pxau2o-ehci";
> > - reg = <0xd4208000 0x200>;
> > - interrupts = <44>;
> > - clocks = <&soc_clocks MMP2_CLK_USB>;
> > - clock-names = "USBCLK";
> > - phys = <&usb_otg_phy>;
> > - phy-names = "usb";
> > - };
> > diff --git a/Documentation/devicetree/bindings/usb/marvell,pxau2o-ehci.yaml b/Documentation/devicetree/bindings/usb/marvell,pxau2o-ehci.yaml
> > new file mode 100644
> > index 0000000000000..189025ef1e92e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/marvell,pxau2o-ehci.yaml
> > @@ -0,0 +1,60 @@
> > +# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
>
> Same license comment.
I wrote that binding document and chose that license.
> > +# Copyright 2019,2020 Lubomir Rintel <lkundrak@v3.sk>
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/usb/marvell,pxau2o-ehci.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Marvell PXA/MMP EHCI bindings
> > +
> > +maintainers:
> > + - Lubomir Rintel <lkundrak@v3.sk>
> > +
> > +allOf:
> > + - $ref: usb-hcd.yaml#
> > +
> > +properties:
> > + compatible:
> > + const: marvell,pxau2o-ehci
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + clock-names:
> > + const: USBCLK
> > +
> > + phys:
> > + maxItems: 1
> > +
> > + phy-names:
> > + const: usb
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - clocks
> > + - clock-names
> > + - phys
> > + - phy-names
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/marvell,mmp2.h>
> > + usb@d4208000 {
> > + compatible = "marvell,pxau2o-ehci";
> > + reg = <0xd4208000 0x200>;
> > + interrupts = <44>;
> > + clocks = <&soc_clocks MMP2_CLK_USB>;
> > + clock-names = "USBCLK";
> > + phys = <&usb_otg_phy>;
> > + phy-names = "usb";
> > + };
> > +
> > +...
> > --
> > 2.25.1
> >
^ permalink raw reply
* Re: [PATCH 26/28] dt-bindings: spi: Convert spi-pxa2xx to json-schema
From: Lubomir Rintel @ 2020-05-17 19:30 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Bartosz Golaszewski, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Mauro Carvalho Chehab, Ulf Hansson,
Kishon Vijay Abraham I, Alessandro Zummo, Alexandre Belloni,
Greg Kroah-Hartman, Mark Brown, Daniel Lezcano, Andrew Lunn,
Gregory Clement, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
devicetree, linux-kernel, linux-gpio, linux-i2c, linux-media,
linux-mmc, linux-rtc, linux-serial, linux-spi, linux-usb
In-Reply-To: <20200327195239.GA28087@bogus>
On Fri, Mar 27, 2020 at 01:52:39PM -0600, Rob Herring wrote:
> On Tue, Mar 17, 2020 at 10:39:20AM +0100, Lubomir Rintel wrote:
> > A straightforward conversion of the the spi-pxa2xx binding to DT schema
> > format using json-schema.
> >
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > ---
> > .../bindings/spi/marvell,mmp2-ssp.yaml | 58 +++++++++++++++++++
> > .../devicetree/bindings/spi/spi-pxa2xx.txt | 27 ---------
> > 2 files changed, 58 insertions(+), 27 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/spi/marvell,mmp2-ssp.yaml
> > delete mode 100644 Documentation/devicetree/bindings/spi/spi-pxa2xx.txt
> >
> > diff --git a/Documentation/devicetree/bindings/spi/marvell,mmp2-ssp.yaml b/Documentation/devicetree/bindings/spi/marvell,mmp2-ssp.yaml
> > new file mode 100644
> > index 0000000000000..de6b6a53b70d8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/spi/marvell,mmp2-ssp.yaml
> > @@ -0,0 +1,58 @@
> > +# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
>
> Same issue on license...
I've written the original document and have chosen this license (for no
good reason, but I don't think that matters).
> > +# Copyright 2019,2020 Lubomir Rintel <lkundrak@v3.sk>
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/spi/marvell,mmp2-ssp.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: PXA2xx SSP SPI Controller bindings
> > +
> > +maintainers:
> > + - Lubomir Rintel <lkundrak@v3.sk>
> > +
> > +allOf:
> > + - $ref: spi-controller.yaml#
> > +
> > +properties:
> > + compatible:
> > + const: marvell,mmp2-ssp
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + ready-gpios:
> > + description: |
> > + GPIO used to signal a SPI master that the FIFO is filled and we're
> > + ready to service a transfer. Only useful in slave mode.
> > + maxItems: 1
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - clocks
>
> > + - '#address-cells'
> > + - '#size-cells'
>
> spi-controller.yaml handles these being required and also that they
> aren't for slave mode.
>
> > +
> > +dependencies:
> > + ready-gpios: [ spi-slave ]
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/marvell,mmp2.h>
> > + spi@d4035000 {
> > + compatible = "marvell,mmp2-ssp";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + reg = <0xd4035000 0x1000>;
> > + clocks = <&soc_clocks MMP2_CLK_SSP0>;
> > + interrupts = <0>;
> > + };
> > +
> > +...
> > diff --git a/Documentation/devicetree/bindings/spi/spi-pxa2xx.txt b/Documentation/devicetree/bindings/spi/spi-pxa2xx.txt
> > deleted file mode 100644
> > index e30e0c2a4bce1..0000000000000
> > --- a/Documentation/devicetree/bindings/spi/spi-pxa2xx.txt
> > +++ /dev/null
> > @@ -1,27 +0,0 @@
> > -PXA2xx SSP SPI Controller
> > -
> > -Required properties:
> > -- compatible: Must be "marvell,mmp2-ssp".
> > -- reg: Offset and length of the device's register set.
> > -- interrupts: Should be the interrupt number.
> > -- clocks: Should contain a single entry describing the clock input.
> > -- #address-cells: Number of cells required to define a chip select address.
> > -- #size-cells: Should be zero.
> > -
> > -Optional properties:
> > -- cs-gpios: list of GPIO chip selects. See the SPI bus bindings,
> > - Documentation/devicetree/bindings/spi/spi-bus.txt
> > -- spi-slave: Empty property indicating the SPI controller is used in slave mode.
> > -- ready-gpios: GPIO used to signal a SPI master that the FIFO is filled
> > - and we're ready to service a transfer. Only useful in slave mode.
> > -
> > -Child nodes represent devices on the SPI bus
> > - See ../spi/spi-bus.txt
> > -
> > -Example:
> > - ssp1: spi@d4035000 {
> > - compatible = "marvell,mmp2-ssp";
> > - reg = <0xd4035000 0x1000>;
> > - clocks = <&soc_clocks MMP2_CLK_SSP0>;
> > - interrupts = <0>;
> > - };
> > --
> > 2.25.1
> >
^ permalink raw reply
* Re: [PATCH 25/28] dt-bindings: rtc: Convert sa1100-rtc to json-schema
From: Lubomir Rintel @ 2020-05-17 19:25 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Bartosz Golaszewski, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Mauro Carvalho Chehab, Ulf Hansson,
Kishon Vijay Abraham I, Alessandro Zummo, Alexandre Belloni,
Greg Kroah-Hartman, Mark Brown, Daniel Lezcano, Andrew Lunn,
Gregory Clement, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
devicetree, linux-kernel, linux-gpio, linux-i2c, linux-media,
linux-mmc, linux-rtc, linux-serial, linux-spi, linux-usb
In-Reply-To: <20200327194902.GA24914@bogus>
On Fri, Mar 27, 2020 at 01:49:02PM -0600, Rob Herring wrote:
> On Tue, Mar 17, 2020 at 10:39:19AM +0100, Lubomir Rintel wrote:
> > Convert the sa1100-rtc binding to DT schema format using json-schema.
> >
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > ---
> > .../devicetree/bindings/rtc/sa1100-rtc.txt | 17 ------
> > .../devicetree/bindings/rtc/sa1100-rtc.yaml | 55 +++++++++++++++++++
> > 2 files changed, 55 insertions(+), 17 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/rtc/sa1100-rtc.txt
> > create mode 100644 Documentation/devicetree/bindings/rtc/sa1100-rtc.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/rtc/sa1100-rtc.txt b/Documentation/devicetree/bindings/rtc/sa1100-rtc.txt
> > deleted file mode 100644
> > index 968ac820254bb..0000000000000
> > --- a/Documentation/devicetree/bindings/rtc/sa1100-rtc.txt
> > +++ /dev/null
> > @@ -1,17 +0,0 @@
> > -* Marvell Real Time Clock controller
> > -
> > -Required properties:
> > -- compatible: should be "mrvl,sa1100-rtc"
> > -- reg: physical base address of the controller and length of memory mapped
> > - region.
> > -- interrupts: Should be two. The first interrupt number is the rtc alarm
> > - interrupt and the second interrupt number is the rtc hz interrupt.
> > -- interrupt-names: Assign name of irq resource.
> > -
> > -Example:
> > - rtc: rtc@d4010000 {
> > - compatible = "mrvl,mmp-rtc";
> > - reg = <0xd4010000 0x1000>;
> > - interrupts = <5>, <6>;
> > - interrupt-names = "rtc 1Hz", "rtc alarm";
> > - };
> > diff --git a/Documentation/devicetree/bindings/rtc/sa1100-rtc.yaml b/Documentation/devicetree/bindings/rtc/sa1100-rtc.yaml
> > new file mode 100644
> > index 0000000000000..53a8b72df9f34
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/sa1100-rtc.yaml
> > @@ -0,0 +1,55 @@
>
> License
The original file lacked one. Should I just go with GPL-2.0?
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/rtc/sa1100-rtc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Marvell Real Time Clock controller bindings
> > +
> > +allOf:
> > + - $ref: rtc.yaml#
> > +
> > +maintainers:
> > + - devicetree@vger.kernel.org
>
> Real person
It's not clear who would that be.
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - mrvl,sa1100-rtc
> > + - mrvl,mmp-rtc
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + resets:
> > + maxItems: 1
>
> clocks and resets weren't documented before. Fine to add here, but add
> that to the commit msg.
>
> > +
> > + interrupts:
> > + minItems: 2
> > + maxItems: 2
>
> Drop minItems.
It needs to be exactly 2. I suppose for that I should drop maxItems
instead?
> > +
> > + interrupt-names:
> > + items:
> > + - const: 'rtc 1Hz'
> > + - const: 'rtc alarm'
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - interrupt-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + rtc: rtc@d4010000 {
> > + compatible = "mrvl,mmp-rtc";
> > + reg = <0xd4010000 0x1000>;
> > + interrupts = <5>, <6>;
> > + interrupt-names = "rtc 1Hz", "rtc alarm";
> > + };
> > +
> > +...
> > --
> > 2.25.1
> >
^ permalink raw reply
* Re: [PATCH v2 4/6] dmaengine: dw: Print warning if multi-block is unsupported
From: Serge Semin @ 2020-05-17 19:23 UTC (permalink / raw)
To: Vinod Koul
Cc: Serge Semin, Andy Shevchenko, Mark Brown, Viresh Kumar,
Dan Williams, Alexey Malahov, Thomas Bogendoerfer, Paul Burton,
Ralf Baechle, Arnd Bergmann, Rob Herring, linux-mips, devicetree,
dmaengine, Linux Kernel Mailing List
In-Reply-To: <20200515063039.GH333670@vkoul-mobl>
On Fri, May 15, 2020 at 12:00:39PM +0530, Vinod Koul wrote:
> Hi Serge,
>
> On 12-05-20, 15:42, Serge Semin wrote:
> > Vinod,
> >
> > Could you join the discussion for a little bit?
> >
> > In order to properly fix the problem discussed in this topic, we need to
> > introduce an additional capability exported by DMA channel handlers on per-channel
> > basis. It must be a number, which would indicate an upper limitation of the SG list
> > entries amount.
> > Something like this would do it:
> > struct dma_slave_caps {
> > ...
> > unsigned int max_sg_nents;
> > ...
>
> Looking at the discussion, I agree we should can this up in the
> interface. The max_dma_len suggests the length of a descriptor allowed,
> it does not convey the sg_nents supported which in the case of nollp is
> one.
>
> Btw is this is a real hardware issue, I have found that value of such
> hardware is very less and people did fix it up in subsequent revs to add
> llp support.
Yes, it is. My DW DMAC doesn't support LLP and there isn't going to be new SoC
version produced.(
>
> Also, another question is why this cannot be handled in driver, I agree
> your hardware does not support llp but that does not stop you from
> breaking a multi_sg list into N hardware descriptors and keep submitting
> them (for this to work submission should be done in isr and not in bh,
> unfortunately very few driver take that route).
Current DW DMA driver does that, but this isn't enough. The problem is that
in order to fix the issue in the DMA hardware driver we need to introduce
an inter-dependent channels abstraction and synchronously feed both Tx and
Rx DMA channels with hardware descriptors (LLP entries) one-by-one. This hardly
needed by any slave device driver rather than SPI, which Tx and Rx buffers are
inter-dependent. So Andy's idea was to move the fix to the SPI driver (feed
the DMA engine channels with Tx and Rx data buffers synchronously), but DMA
engine would provide an info whether such fix is required. This can be
determined by the maximum SG entries capability.
(Note max_sg_ents isn't a limitation on the number of SG entries supported by
the DMA driver, but the number of SG entries handled by the DMA engine in a
single DMA transaction.)
> TBH the max_sg_nents or
> max_dma_len are HW restrictions and SW *can* deal with then :-)
Yes, it can, but it only works for the cases when individual DMA channels are
utilized. DMA hardware driver doesn't know that the target and source slave
device buffers (SPI Tx and Rx FIFOs) are inter-dependent, that writing to one
you will implicitly push data to another. So due to the interrupts handling
latency Tx DMA channel is restarted faster than Rx DMA channel is reinitialized.
This causes the SPI Rx FIFO overflow and data loss.
>
> In an idea world, you should break the sw descriptor submitted into N hw
> descriptors and submit to hardware and let user know when the sw
> descriptor is completed. Of course we do not do that :(
Well, the current Dw DMA driver does that. But due to the two slave device
buffers inter-dependency this isn't enough to perform safe DMA transactions.
Due to the interrupts handling latency Tx DMA channel pushes data to the slave
device buffer faster than Rx DMA channel starts to handle incoming data. This
causes the SPI Rx FIFO overflow.
>
> > };
> > As Andy suggested it's value should be interpreted as:
> > 0 - unlimited number of entries,
> > 1:MAX_UINT - actual limit to the number of entries.
>
> Hmm why 0, why not MAX_UINT for unlimited?
0 is much better for many reasons. First of all MAX_UINT is a lot, but it's
still a number. On x64 platform this might be actual limit if for instance
the block-size register is 32-bits wide. Secondly interpreting 0 as unlimited
number of entries would be more suitable since most of the drivers support
LLP functionality and we wouldn't need to update their code to set MAX_UINT.
Thirdly DMA engines, which don't support LLPs would need to set this parameter
as 1. So if we do as you say and interpret unlimited number of LLPs as MAX_UINT,
then 0 would left unused.
To sum up I also think that using 0 as unlimited number SG entries supported is
much better.
>
> > In addition to that seeing the dma_get_slave_caps() method provide the caps only
> > by getting them from the DMA device descriptor, while we need to have an info on
> > per-channel basis, it would be good to introduce a new DMA-device callback like:
> > struct dma_device {
> > ...
> > int (*device_caps)(struct dma_chan *chan,
> > struct dma_slave_caps *caps);
>
> Do you have a controller where channel caps are on per-channel basis?
Yes, I do. Our DW DMA controller has got the maximum burst length non-uniformly
distributed per DMA channels. There are eight channels our controller supports,
among which first two channels can burst up to 32 transfer words, but the rest
of the channels support bursting up to 4 transfer words.
So having such device_caps() callback to customize the device capabilities on
per-DMA-channel basis would be very useful! What do you think?
-Sergey
>
> > ...
> > };
> > So the DMA driver could override the generic DMA device capabilities with the
> > values specific to the DMA channels. Such functionality will be also helpful for
> > the max-burst-len parameter introduced by this patchset, since depending on the
> > IP-core synthesis parameters it may be channel-specific.
> >
> > Alternatively we could just introduce a new fields to the dma_chan structure and
> > retrieve the new caps values from them in the dma_get_slave_caps() method.
> > Though the solution with callback I like better.
> >
> > What is your opinion about this? What solution you'd prefer?
> >
> > On Tue, May 12, 2020 at 12:08:00AM +0300, Andy Shevchenko wrote:
> > > On Tue, May 12, 2020 at 12:07:14AM +0300, Andy Shevchenko wrote:
> > > > On Mon, May 11, 2020 at 10:32:55PM +0300, Serge Semin wrote:
> > > > > On Mon, May 11, 2020 at 04:58:53PM +0300, Andy Shevchenko wrote:
> > > > > > On Mon, May 11, 2020 at 4:48 PM Serge Semin
> > > > > > <Sergey.Semin@baikalelectronics.ru> wrote:
> > > > > > >
> > > > > > > On Mon, May 11, 2020 at 12:58:13PM +0100, Mark Brown wrote:
> > > > > > > > On Mon, May 11, 2020 at 05:10:16AM +0300, Serge Semin wrote:
> > > > > > > >
> > > > > > > > > Alas linearizing the SPI messages won't help in this case because the DW DMA
> > > > > > > > > driver will split it into the max transaction chunks anyway.
> > > > > > > >
> > > > > > > > That sounds like you need to also impose a limit on the maximum message
> > > > > > > > size as well then, with that you should be able to handle messages up
> > > > > > > > to whatever that limit is. There's code for that bit already, so long
> > > > > > > > as the limit is not too low it should be fine for most devices and
> > > > > > > > client drivers can see the limit so they can be updated to work with it
> > > > > > > > if needed.
> > > > > > >
> > > > > > > Hmm, this might work. The problem will be with imposing such limitation through
> > > > > > > the DW APB SSI driver. In order to do this I need to know:
> > > > > > > 1) Whether multi-block LLP is supported by the DW DMA controller.
> > > > > > > 2) Maximum DW DMA transfer block size.
> > > > > > > Then I'll be able to use this information in the can_dma() callback to enable
> > > > > > > the DMA xfers only for the safe transfers. Did you mean something like this when
> > > > > > > you said "There's code for that bit already" ? If you meant the max_dma_len
> > > > > > > parameter, then setting it won't work, because it just limits the SG items size
> > > > > > > not the total length of a single transfer.
> > > > > > >
> > > > > > > So the question is of how to export the multi-block LLP flag from DW DMAc
> > > > > > > driver. Andy?
> > > > > >
> > > > > > I'm not sure I understand why do you need this being exported. Just
> > > > > > always supply SG list out of single entry and define the length
> > > > > > according to the maximum segment size (it's done IIRC in SPI core).
> > > > >
> > > > > Finally I see your point. So you suggest to feed the DMA engine with SG list
> > > > > entries one-by-one instead of sending all of them at once in a single
> > > > > dmaengine_prep_slave_sg() -> dmaengine_submit() -> dma_async_issue_pending()
> > > > > session. Hm, this solution will work, but there is an issue. There is no
> > > > > guarantee, that Tx and Rx SG lists are symmetric, consisting of the same
> > > > > number of items with the same sizes. It depends on the Tx/Rx buffers physical
> > > > > address alignment and their offsets within the memory pages. Though this
> > > > > problem can be solved by making the Tx and Rx SG lists symmetric. I'll have
> > > > > to implement a clever DMA IO loop, which would extract the DMA
> > > > > addresses/lengths from the SG entries and perform the single-buffer DMA
> > > > > transactions with the DMA buffers of the same length.
> > > > >
> > > > > Regarding noLLP being exported. Obviously I intended to solve the problem in a
> > > > > generic way since the problem is common for noLLP DW APB SSI/DW DMAC combination.
> > > > > In order to do this we need to know whether the multi-block LLP feature is
> > > > > unsupported by the DW DMA controller. We either make such info somehow exported
> > > > > from the DW DMA driver, so the DMA clients (like Dw APB SSI controller driver)
> > > > > could be ready to work around the problem; or just implement a flag-based quirk
> > > > > in the DMA client driver, which would be enabled in the platform-specific basis
> > > > > depending on the platform device actually detected (for instance, a specific
> > > > > version of the DW APB SSI IP). AFAICS You'd prefer the later option.
> > > >
> > > > So, we may extend the struct of DMA parameters to tell the consumer amount of entries (each of which is no longer than maximum segment size) it can afford:
> > > > - 0: Auto (DMA driver handles any cases itself)
> > > > - 1: Only single entry
> > > > - 2: Up to two...
> > >
> > > It will left implementation details (or i.o.w. obstacles or limitation) why DMA
> > > can't do otherwise.
> >
> > Sounds good. Thanks for assistance.
> >
> > -Sergey
> >
> > >
> > > --
> > > With Best Regards,
> > > Andy Shevchenko
> > >
> > >
>
> --
> ~Vinod
^ permalink raw reply
* Re: [PATCH 24/28] dt-bindings: media: Convert marvell,mmp2-ccic to json-schema
From: Lubomir Rintel @ 2020-05-17 19:21 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Bartosz Golaszewski, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Mauro Carvalho Chehab, Ulf Hansson,
Kishon Vijay Abraham I, Alessandro Zummo, Alexandre Belloni,
Greg Kroah-Hartman, Mark Brown, Daniel Lezcano, Andrew Lunn,
Gregory Clement, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
devicetree, linux-kernel, linux-gpio, linux-i2c, linux-media,
linux-mmc, linux-rtc, linux-serial, linux-spi, linux-usb
In-Reply-To: <20200327194637.GA18803@bogus>
On Fri, Mar 27, 2020 at 01:46:37PM -0600, Rob Herring wrote:
> On Tue, Mar 17, 2020 at 10:39:18AM +0100, Lubomir Rintel wrote:
> > Convert the marvell,mmp2-ccic binding to DT schema format using
> > json-schema.
> >
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > ---
> > .../bindings/media/marvell,mmp2-ccic.txt | 50 ---------
> > .../bindings/media/marvell,mmp2-ccic.yaml | 102 ++++++++++++++++++
> > 2 files changed, 102 insertions(+), 50 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
> > create mode 100644 Documentation/devicetree/bindings/media/marvell,mmp2-ccic.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt b/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
> > deleted file mode 100644
> > index 7ec2c8c8a3b98..0000000000000
> > --- a/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.txt
> > +++ /dev/null
> > @@ -1,50 +0,0 @@
> > -Marvell MMP2 camera host interface
> > -
> > -Required properties:
> > - - compatible: Should be "marvell,mmp2-ccic".
> > - - reg: Register base and size.
> > - - interrupts: The interrupt number.
> > - - #clock-cells: Must be 0.
> > -
> > -Optional properties:
> > - - clocks: Reference to the input clock as specified by
> > - Documentation/devicetree/bindings/clock/clock-bindings.txt.
> > - - clock-names: Names of the clocks used; "axi" for the AXI bus interface,
> > - "func" for the peripheral clock and "phy" for the parallel
> > - video bus interface.
> > - - clock-output-names: Optional clock source for sensors. Shall be "mclk".
> > -
> > -Required subnodes:
> > - - port: The parallel bus interface port with a single endpoint linked to
> > - the sensor's endpoint as described in
> > - Documentation/devicetree/bindings/media/video-interfaces.txt.
> > -
> > -Required endpoint properties:
> > - - bus-type: data bus type, <5> or <6> for Parallel or Bt.656 respectively
> > - - pclk-sample: pixel clock polarity
> > - - hsync-active: horizontal synchronization polarity (only required for
> > - parallel bus)
> > - - vsync-active: vertical synchronization polarity (only required for
> > - parallel bus)
> > -
> > -Example:
> > -
> > - camera0: camera@d420a000 {
> > - compatible = "marvell,mmp2-ccic";
> > - reg = <0xd420a000 0x800>;
> > - interrupts = <42>;
> > - clocks = <&soc_clocks MMP2_CLK_CCIC0>;
> > - clock-names = "axi";
> > - #clock-cells = <0>;
> > - clock-output-names = "mclk";
> > -
> > - port {
> > - camera0_0: endpoint {
> > - remote-endpoint = <&ov7670_0>;
> > - bus-type = <5>; /* Parallel */
> > - hsync-active = <1>; /* Active high */
> > - vsync-active = <1>; /* Active high */
> > - pclk-sample = <0>; /* Falling */
> > - };
> > - };
> > - };
> > diff --git a/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.yaml b/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.yaml
> > new file mode 100644
> > index 0000000000000..890a3f9d0302f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/marvell,mmp2-ccic.yaml
> > @@ -0,0 +1,102 @@
> > +# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
>
> GPL-2.0-only please. Dual license only if you have rights on the old
> file to do so.
I do. I wrote the original binding file.
> > +# Copyright 2019,2020 Lubomir Rintel <lkundrak@v3.sk>
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/marvell,mmp2-ccic.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Marvell MMP2 camera host interface bindings
> > +
> > +maintainers:
> > + - Lubomir Rintel <lkundrak@v3.sk>
> > +
> > +properties:
> > + $nodename:
> > + pattern: '^camera@[a-f0-9]+$'
> > +
> > + compatible:
> > + const: marvell,mmp2-ccic
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + port:
> > + type: object
> > + additionalProperties: false
> > +
> > + properties:
> > + endpoint:
>
> Wrong indentaion.
>
> > + type: object
> > + additionalProperties: false
> > +
> > + # Properties described in
> > + # Documentation/devicetree/bindings/media/video-interfaces.txt
> > + properties:
> > + remote-endpoint: true
> > + hsync-active: true
> > + vsync-active: true
> > + pclk-sample: true
> > + bus-type: true
> > +
> > + required:
> > + - remote-endpoint
> > +
> > + required:
> > + - endpoint
> > +
> > + clocks:
> > + minItems: 1
> > + maxItems: 3
>
> Shouldn't really be variable for a single compatible.
>
> > + items:
> > + - description: AXI bus interface clock
> > + - description: Peripheral clock
> > + - description: Parallel video bus interface clock
> > +
> > + clock-names:
> > + minItems: 1
> > + maxItems: 3
> > + items:
> > + - const: axi
> > + - const: func
> > + - const: phy
> > +
> > + '#clock-cells':
> > + const: 0
> > +
> > + clock-output-names:
> > + const: mclk
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - port
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/marvell,mmp2.h>
> > +
> > + camera@d420a000 {
> > + compatible = "marvell,mmp2-ccic";
> > + reg = <0xd420a000 0x800>;
> > + interrupts = <42>;
> > + clocks = <&soc_clocks MMP2_CLK_CCIC0>;
> > + clock-names = "axi";
> > + #clock-cells = <0>;
> > + clock-output-names = "mclk";
> > +
> > + port {
> > + camera0_0: endpoint {
> > + remote-endpoint = <&ov7670_0>;
> > + bus-type = <5>; /* Parallel */
> > + hsync-active = <1>; /* Active high */
> > + vsync-active = <1>; /* Active high */
> > + pclk-sample = <0>; /* Falling */
> > + };
> > + };
> > + };
> > +
> > +...
> > --
> > 2.25.1
> >
^ permalink raw reply
* Re: [PATCH 23/28] dt-bindings: interrupt-controller: Convert mrvl,intc to json-schema
From: Lubomir Rintel @ 2020-05-17 19:11 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Bartosz Golaszewski, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Mauro Carvalho Chehab, Ulf Hansson,
Kishon Vijay Abraham I, Alessandro Zummo, Alexandre Belloni,
Greg Kroah-Hartman, Mark Brown, Daniel Lezcano, Andrew Lunn,
Gregory Clement, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
devicetree, linux-kernel, linux-gpio, linux-i2c, linux-media,
linux-mmc, linux-rtc, linux-serial, linux-spi, linux-usb
In-Reply-To: <20200327194207.GA1996@bogus>
On Fri, Mar 27, 2020 at 01:42:07PM -0600, Rob Herring wrote:
> On Tue, Mar 17, 2020 at 10:39:17AM +0100, Lubomir Rintel wrote:
> > Convert the mrvl,intc binding to DT schema format using json-schema.
> >
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > ---
> > .../interrupt-controller/mrvl,intc.txt | 64 --------
> > .../interrupt-controller/mrvl,intc.yaml | 144 ++++++++++++++++++
> > 2 files changed, 144 insertions(+), 64 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/mrvl,intc.txt
> > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/mrvl,intc.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/mrvl,intc.txt b/Documentation/devicetree/bindings/interrupt-controller/mrvl,intc.txt
> > deleted file mode 100644
> > index a0ed02725a9d7..0000000000000
> > --- a/Documentation/devicetree/bindings/interrupt-controller/mrvl,intc.txt
> > +++ /dev/null
> > @@ -1,64 +0,0 @@
> > -* Marvell MMP Interrupt controller
> > -
> > -Required properties:
> > -- compatible : Should be
> > - "mrvl,mmp-intc" on Marvel MMP,
> > - "mrvl,mmp2-intc" along with "mrvl,mmp2-mux-intc" on MMP2 or
> > - "marvell,mmp3-intc" with "mrvl,mmp2-mux-intc" on MMP3
> > -- reg : Address and length of the register set of the interrupt controller.
> > - If the interrupt controller is intc, address and length means the range
> > - of the whole interrupt controller. The "marvell,mmp3-intc" controller
> > - also has a secondary range for the second CPU core. If the interrupt
> > - controller is mux-intc, address and length means one register. Since
> > - address of mux-intc is in the range of intc. mux-intc is secondary
> > - interrupt controller.
> > -- reg-names : Name of the register set of the interrupt controller. It's
> > - only required in mux-intc interrupt controller.
> > -- interrupts : Should be the port interrupt shared by mux interrupts. It's
> > - only required in mux-intc interrupt controller.
> > -- interrupt-controller : Identifies the node as an interrupt controller.
> > -- #interrupt-cells : Specifies the number of cells needed to encode an
> > - interrupt source.
> > -- mrvl,intc-nr-irqs : Specifies the number of interrupts in the interrupt
> > - controller.
> > -- mrvl,clr-mfp-irq : Specifies the interrupt that needs to clear MFP edge
> > - detection first.
> > -
> > -Example:
> > - intc: interrupt-controller@d4282000 {
> > - compatible = "mrvl,mmp2-intc";
> > - interrupt-controller;
> > - #interrupt-cells = <1>;
> > - reg = <0xd4282000 0x1000>;
> > - mrvl,intc-nr-irqs = <64>;
> > - };
> > -
> > - intcmux4@d4282150 {
> > - compatible = "mrvl,mmp2-mux-intc";
> > - interrupts = <4>;
> > - interrupt-controller;
> > - #interrupt-cells = <1>;
> > - reg = <0x150 0x4>, <0x168 0x4>;
> > - reg-names = "mux status", "mux mask";
> > - mrvl,intc-nr-irqs = <2>;
> > - };
> > -
> > -* Marvell Orion Interrupt controller
> > -
> > -Required properties
> > -- compatible : Should be "marvell,orion-intc".
> > -- #interrupt-cells: Specifies the number of cells needed to encode an
> > - interrupt source. Supported value is <1>.
> > -- interrupt-controller : Declare this node to be an interrupt controller.
> > -- reg : Interrupt mask address. A list of 4 byte ranges, one per controller.
> > - One entry in the list represents 32 interrupts.
> > -
> > -Example:
> > -
> > - intc: interrupt-controller {
> > - compatible = "marvell,orion-intc", "marvell,intc";
> > - interrupt-controller;
> > - #interrupt-cells = <1>;
> > - reg = <0xfed20204 0x04>,
> > - <0xfed20214 0x04>;
> > - };
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/mrvl,intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/mrvl,intc.yaml
> > new file mode 100644
> > index 0000000000000..f0644f7d7e1d2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/mrvl,intc.yaml
> > @@ -0,0 +1,144 @@
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/interrupt-controller/mrvl,intc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Marvell MMP/Orion Interrupt controller bindings
> > +
> > +maintainers:
> > + - devicetree@vger.kernel.org
> > +
> > +allOf:
> > + - $ref: /schemas/interrupt-controller.yaml#
>
> Drop this. It is already applied based on matching on the node name.
>
> > + - if:
> > + properties:
> > + compatible:
> > + not:
> > + contains:
> > + const: marvell,orion-intc
> > + then:
> > + required:
> > + - mrvl,intc-nr-irqs
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - mrvl,mmp-intc
> > + - mrvl,mmp2-intc
> > + then:
> > + properties:
> > + reg:
> > + minItems: 1
> > + maxItems: 1
>
> Drop minItems, as just 'maxItems: 1' is enough.
>
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - marvell,mmp3-intc
> > + - mrvl,mmp2-mux-intc
> > + then:
> > + properties:
> > + reg:
> > + minItems: 2
> > + maxItems: 2
>
> Just 'minItems: 2'.
>
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: marvell,orion-intc
> > + then:
> > + properties:
> > + reg:
> > + minItems: 1
> > + maxItems: 2
>
> Normally, for a compatible this would not vary...
>
> In any case, move this to the main section and drop this if.
>
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: mrvl,mmp2-mux-intc
> > + then:
> > + properties:
> > + interrupts:
> > + minItems: 1
> > + maxItems: 1
>
> Just 'maxItems'
>
> > + reg-names:
> > + minItems: 2
> > + maxItems: 2
>
> These are redundant as 'items' size implies this.
>
> > + items:
> > + - const: 'mux status'
> > + - const: 'mux mask'
>
> Move this to the main section.
>
> > + required:
> > + - interrupts
> > + else:
> > + properties:
> > + interrupts: false
> > +
> > +properties:
> > + '#interrupt-cells':
> > + const: 1
> > +
> > + compatible:
> > + enum:
> > + - mrvl,mmp-intc
> > + - mrvl,mmp2-intc
> > + - marvell,mmp3-intc
> > + - marvell,orion-intc
> > + - mrvl,mmp2-mux-intc
> > +
> > + reg: true
> > +
> > + reg-names: true
> > +
> > + interrupts: true
> > +
> > + interrupt-controller: true
> > +
> > + mrvl,intc-nr-irqs:
> > + description: |
> > + Specifies the number of interrupts in the interrupt controller.
> > + $ref: /schemas/types.yaml#/definitions/uint32
>
> Is there a max number?
There's none in the original bindings document or enforced by the
driver.
> > +
> > + mrvl,clr-mfp-irq:
> > + description: |
> > + Specifies the interrupt that needs to clear MFP edge detection first.
> > + $ref: /schemas/types.yaml#/definitions/uint32
>
> Constraints?
I don't know how should this be constrained and the original bindings
document is not helpful.
> > +
> > +required:
> > + - '#interrupt-cells'
> > + - compatible
> > + - reg
> > + - interrupt-controller
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + interrupt-controller@d4282000 {
> > + compatible = "mrvl,mmp2-intc";
> > + interrupt-controller;
> > + #interrupt-cells = <1>;
> > + reg = <0xd4282000 0x1000>;
> > + mrvl,intc-nr-irqs = <64>;
> > + };
> > +
> > + interrupt-controller@d4282150 {
> > + compatible = "mrvl,mmp2-mux-intc";
> > + interrupts = <4>;
> > + interrupt-controller;
> > + #interrupt-cells = <1>;
> > + reg = <0x150 0x4>, <0x168 0x4>;
> > + reg-names = "mux status", "mux mask";
> > + mrvl,intc-nr-irqs = <2>;
> > + };
> > + - |
> > + interrupt-controller@fed20204 {
> > + compatible = "marvell,orion-intc";
> > + interrupt-controller;
> > + #interrupt-cells = <1>;
> > + reg = <0xfed20204 0x04>,
> > + <0xfed20214 0x04>;
> > + };
> > +
> > +...
> > --
> > 2.25.1
> >
^ permalink raw reply
* Re: [PATCH v5 5/6] iio: imu: Add support for adis16475
From: Lars-Peter Clausen @ 2020-05-17 19:07 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Nuno Sá, Nuno Sá, linux-iio, devicetree, Hartmut Knaack,
Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
Alexandru Ardelean, Michael Hennerich
In-Reply-To: <20200517171523.54fa6deb@archlinux>
On 5/17/20 6:15 PM, Jonathan Cameron wrote:
> On Sun, 3 May 2020 11:12:34 +0200
> Lars-Peter Clausen <lars@metafoo.de> wrote:
>
>> On 5/3/20 11:07 AM, Lars-Peter Clausen wrote:
>>> On 5/3/20 10:47 AM, Jonathan Cameron wrote:
>>>> On Sat, 02 May 2020 21:52:18 +0200
>>>> Nuno Sá <noname.nuno@gmail.com> wrote:
>>>>
>>>>> On Sat, 2020-05-02 at 20:01 +0200, Lars-Peter Clausen wrote:
>>>>>> On 5/2/20 7:40 PM, Jonathan Cameron wrote:
>>>>>>> On Mon, 27 Apr 2020 20:06:07 +0200
>>>>>>> Lars-Peter Clausen <lars@metafoo.de> wrote:
>>>>>>>> On 4/13/20 10:24 AM, Nuno Sá wrote:
>>>>>>>>> [...]
>>>>>>>>> +static irqreturn_t adis16475_trigger_handler(int irq, void *p)
>>>>>>>>> +{
>>>>>>>>> [...]
>>>>>>>>> + __be16 data[ADIS16475_MAX_SCAN_DATA], *buffer;
>>>>>>>>> [...]
>>>>>>>>> +
>>>>>>>>> + iio_push_to_buffers_with_timestamp(indio_dev, data, pf-
>>>>>>>>>> timestamp);
>>>>>>>> If the timestamp is enabled the IIO core might insert padding
>>>>>>>> between
>>>>>>>> the data channels and the timestamp. If that happens this will
>>>>>>>> disclose
>>>>>>>> kernel stack memory to userspace.
>>>>>>>>
>>>>>>>> This needs either a memset(data, 0x00, sizeof(data)) or maybe put
>>>>>>>> data
>>>>>>>> into the state struct and kzalloc it.
>>>>>>> Good spot. Could simply do __be16 data[ADI..] = {0}; rather than
>>>>>>> explicit
>>>>>>> memset, but some form of zeroization is needed.
>>>>>>>
>>>>>>> I've fixed up the applied patch with the above approach.
>>>>>> There is actually another issue. The stack data is not necessarily
>>>>>> aligned to 64 bit, which causes issues if we try to put the 64-bit
>>>>> Oh, this is actually more problematic. Yes, since we have an array of
>>>>> u16, that is not guaranteed to be 64bit aligned. Doing a quick search
>>>>> of `iio_push_to_buffers_with_timestamp()` users and I could quickly
>>>>> find 4/5 drivers with the same problem. I guess the API should clearly
>>>>> state that `data` needs to be __at least__ 64 bits aligned (maybe a
>>>>> future patch). Or we could even check the address and guarantee that it
>>>>> is properly aligned before continuing (though Im guessing this will
>>>>> break a lot of users...)
>>>>>> timestamp in it. I think data should really be in the state struct.
>>>>> Yes, with a proper __aligned(8) attribute... Or couldn't we just use
>>>>> __aligned(8) on the stack variable?
>>>> Forcing alignment on the stack isn't terribly reliable, which is why
>>>> we never do that for dma safe buffers.
>>>>
>>>> Probably better to just move it to the state structure.
>>>> I'll fix it up to do that. Please sanity check what will shortly
>>>> be in the testing branch.
>>>>
>>>> The moment Lars mentioned this I groaned. As you've noted a few other
>>>> drivers have the same problem + the ABI doesn't clearly state
>>>> or check this.
>>>>
>>>> We should certainly fix all the drivers that suffer this problem
>>>> first then we can think about adding a runtime check.
>>> It looks like it is actually quite a few drivers, maybe we should
>>> switch to put_unaligned(). We probably got lucky in most cases and the
>>> buffer is naturally aligned to 64 bit.
> Just a quick update on this. I've been taking a deeper look and there
> are some 'interesting' cases in here so the put_unaligned is attractive
> unfortunately I don't think we can go that way because it would be
> reasonable for consumers of the buffer to expect it to be appropriately
> aligned. We need to rework many of these anyway to fix the related
> data leak.
>
> I've done some of below and will post shortly - a few will take more
> effort and probably need testing rather than just relying on review.
>
> So far the 'interesting ones' are mpu3050 and isl29501.
isl29501 looks OK to me. mpu3050 is clearly broken, buffer is both
unaligned and too small!
^ permalink raw reply
* Re: [PATCH 21/28] dt-bindings: gpio: Convert mrvl-gpio to json-schema
From: Lubomir Rintel @ 2020-05-17 19:00 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Bartosz Golaszewski, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Mauro Carvalho Chehab, Ulf Hansson,
Kishon Vijay Abraham I, Alessandro Zummo, Alexandre Belloni,
Greg Kroah-Hartman, Mark Brown, Daniel Lezcano, Andrew Lunn,
Gregory Clement, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
devicetree, linux-kernel, linux-gpio, linux-i2c, linux-media,
linux-mmc, linux-rtc, linux-serial, linux-spi, linux-usb
In-Reply-To: <20200327192820.GA8577@bogus>
On Fri, Mar 27, 2020 at 01:28:20PM -0600, Rob Herring wrote:
> On Tue, Mar 17, 2020 at 10:39:15AM +0100, Lubomir Rintel wrote:
> > This converts the mrvl-gpio binding to DT schema format using json-schema.
> >
> > Various fixes were done during the conversion, such as adding more
> > properties that are in fact mandatory or extending the examples to
> > include child nodes with extra GPIO blocks.
>
> Ugg, not how I would have done this. Differences in register layout
> should be implied by the compatible strings. But I guess we have to live
> with it now.
>
> > The compatible strings are a mess. It is not clear why so many of them
> > are needed; the driver doesn't really seem to differentiate between the
> > models. Some of them, like marvell,pxa93x-gpio and marvell,pxa1928-gpio
> > are not used at all, so it's not known how many interrupts they utilize.
> > On the other hand, mrvl,pxa-gpio has been seen in the tree, but it
> > doesn't end up in any actual DTB file.
>
> I added pxa1928 and then work on it ended. I think it is safe to remove,
> but I dug up dts file and it is 1 irq.
>
> > In any case -- the schema merely copies whatever was in the original
> > binding document, so it's hopefully no more wrong that the original.
> >
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > ---
> > .../devicetree/bindings/gpio/mrvl-gpio.txt | 48 -----
> > .../devicetree/bindings/gpio/mrvl-gpio.yaml | 173 ++++++++++++++++++
> > 2 files changed, 173 insertions(+), 48 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/gpio/mrvl-gpio.txt
> > create mode 100644 Documentation/devicetree/bindings/gpio/mrvl-gpio.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/mrvl-gpio.txt b/Documentation/devicetree/bindings/gpio/mrvl-gpio.txt
> > deleted file mode 100644
> > index 30fd2201b3d4c..0000000000000
> > --- a/Documentation/devicetree/bindings/gpio/mrvl-gpio.txt
> > +++ /dev/null
> > @@ -1,48 +0,0 @@
> > -* Marvell PXA GPIO controller
> > -
> > -Required properties:
> > -- compatible : Should be "intel,pxa25x-gpio", "intel,pxa26x-gpio",
> > - "intel,pxa27x-gpio", "intel,pxa3xx-gpio",
> > - "marvell,pxa93x-gpio", "marvell,mmp-gpio",
> > - "marvell,mmp2-gpio" or marvell,pxa1928-gpio.
> > -- reg : Address and length of the register set for the device
> > -- interrupts : Should be the port interrupt shared by all gpio pins.
> > - There're three gpio interrupts in arch-pxa, and they're gpio0,
> > - gpio1 and gpio_mux. There're only one gpio interrupt in arch-mmp,
> > - gpio_mux.
> > -- interrupt-names : Should be the names of irq resources. Each interrupt
> > - uses its own interrupt name, so there should be as many interrupt names
> > - as referenced interrupts.
> > -- interrupt-controller : Identifies the node as an interrupt controller.
> > -- #interrupt-cells: Specifies the number of cells needed to encode an
> > - interrupt source.
> > -- gpio-controller : Marks the device node as a gpio controller.
> > -- #gpio-cells : Should be two. The first cell is the pin number and
> > - the second cell is used to specify flags. See gpio.txt for possible
> > - values.
> > -
> > -Example for a MMP platform:
> > -
> > - gpio: gpio@d4019000 {
> > - compatible = "marvell,mmp-gpio";
> > - reg = <0xd4019000 0x1000>;
> > - interrupts = <49>;
> > - interrupt-names = "gpio_mux";
> > - gpio-controller;
> > - #gpio-cells = <2>;
> > - interrupt-controller;
> > - #interrupt-cells = <1>;
> > - };
> > -
> > -Example for a PXA3xx platform:
> > -
> > - gpio: gpio@40e00000 {
> > - compatible = "intel,pxa3xx-gpio";
> > - reg = <0x40e00000 0x10000>;
> > - interrupt-names = "gpio0", "gpio1", "gpio_mux";
> > - interrupts = <8 9 10>;
> > - gpio-controller;
> > - #gpio-cells = <0x2>;
> > - interrupt-controller;
> > - #interrupt-cells = <0x2>;
> > - };
> > diff --git a/Documentation/devicetree/bindings/gpio/mrvl-gpio.yaml b/Documentation/devicetree/bindings/gpio/mrvl-gpio.yaml
> > new file mode 100644
> > index 0000000000000..5c713bf59b06d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/mrvl-gpio.yaml
> > @@ -0,0 +1,173 @@
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/gpio/mrvl-gpio.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Marvell PXA GPIO controller
> > +
> > +maintainers:
> > + - devicetree@vger.kernel.org
>
> Needs to be a real person that cares about this h/w.
As I've said in the cover letter -- it's not clear who would that be.
> > +
> > +allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - intel,pxa25x-gpio
> > + - intel,pxa26x-gpio
> > + - intel,pxa27x-gpio
> > + - intel,pxa3xx-gpio
> > + then:
> > + properties:
> > + interrupts:
> > + minItems: 3
> > + maxItems: 3
> > + interrupt-names:
> > + items:
> > + - const: gpio0
> > + - const: gpio1
> > + - const: gpio_mux
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - marvell,mmp-gpio
> > + - marvell,mmp2-gpio
>
> I'd make this an else clause.
This would imply that marvell,pxa93x-gpio has one interrupt and it is
not know whether that is actually the case.
> > + then:
> > + properties:
> > + interrupts:
> > + maxItems: 1
> > + interrupt-names:
> > + items:
> > + - const: gpio_mux
> > +
> > +properties:
> > + $nodename:
> > + pattern: '^gpio@[0-9a-f]+$'
> > +
> > + compatible:
> > + enum:
> > + - intel,pxa25x-gpio
> > + - intel,pxa26x-gpio
> > + - intel,pxa27x-gpio
> > + - intel,pxa3xx-gpio
> > + - marvell,mmp-gpio
> > + - marvell,mmp2-gpio
> > + - marvell,pxa93x-gpio
> > + - marvell,pxa1928-gpio
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + resets:
> > + maxItems: 1
> > +
> > + ranges: true
> > +
> > + '#address-cells':
> > + const: 1
> > +
> > + '#size-cells':
> > + const: 1
> > +
> > + gpio-controller: true
> > +
> > + '#gpio-cells':
> > + const: 2
> > +
> > + gpio-ranges:
> > + maxItems: 1
>
> My pxa1928 dts has this in the child nodes. Sure this is right?
I actually don't know anything about pxa1928. pxa27x.dtsi and
pxa3xx.dtsi specify gpio-ranges on main gpio nodes and have no gcb
child nodes.
> > +
> > + interrupts: true
> > +
> > + interrupt-names: true
> > +
> > + interrupt-controller: true
> > +
> > + '#interrupt-cells':
> > + const: 2
> > +
> > +patternProperties:
> > + '^gpio@[0-9a-f]*$':
> > + type: object
> > + properties:
> > + reg:
> > + maxItems: 1
> > +
> > + required:
> > + - reg
> > +
> > + additionalProperties: false
> > +
> > +required:
> > + - compatible
> > + - '#address-cells'
> > + - '#size-cells'
> > + - reg
> > + - gpio-controller
> > + - '#gpio-cells'
> > + - interrupts
> > + - interrupt-names
> > + - interrupt-controller
> > + - '#interrupt-cells'
> > + - ranges
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/pxa-clock.h>
> > + gpio@40e00000 {
> > + compatible = "intel,pxa3xx-gpio";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + reg = <0x40e00000 0x10000>;
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + interrupts = <8>, <9>, <10>;
> > + interrupt-names = "gpio0", "gpio1", "gpio_mux";
> > + clocks = <&clks CLK_GPIO>;
> > + interrupt-controller;
> > + #interrupt-cells = <2>;
> > + ranges;
>
> No child, so there should be no ranges here.
Will fix in next version.
> > + };
> > + - |
> > + #include <dt-bindings/clock/marvell,pxa910.h>
> > + gpio@d4019000 {
> > + compatible = "marvell,mmp-gpio";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + reg = <0xd4019000 0x1000>;
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + interrupts = <49>;
> > + interrupt-names = "gpio_mux";
> > + clocks = <&soc_clocks PXA910_CLK_GPIO>;
> > + resets = <&soc_clocks PXA910_CLK_GPIO>;
> > + interrupt-controller;
> > + #interrupt-cells = <2>;
> > + ranges;
> > +
> > + gpio@d4019000 {
> > + reg = <0xd4019000 0x4>;
> > + };
> > +
> > + gpio@d4019004 {
> > + reg = <0xd4019004 0x4>;
> > + };
> > +
> > + gpio@d4019008 {
> > + reg = <0xd4019008 0x4>;
> > + };
> > +
> > + gpio@d4019100 {
> > + reg = <0xd4019100 0x4>;
> > + };
> > + };
> > +
> > +...
> > --
> > 2.25.1
> >
^ permalink raw reply
* Re: [PATCH 19/28] dt-bindings: mmc: Convert sdhci-pxa to json-schema
From: Lubomir Rintel @ 2020-05-17 18:35 UTC (permalink / raw)
To: Rob Herring
Cc: Linus Walleij, Bartosz Golaszewski, Thomas Gleixner, Jason Cooper,
Marc Zyngier, Mauro Carvalho Chehab, Ulf Hansson,
Kishon Vijay Abraham I, Alessandro Zummo, Alexandre Belloni,
Greg Kroah-Hartman, Mark Brown, Daniel Lezcano, Andrew Lunn,
Gregory Clement, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
devicetree, linux-kernel@vger.kernel.org,
open list:GPIO SUBSYSTEM, Linux I2C, Linux Media Mailing List,
linux-mmc, open list:REAL TIME CLOCK (RTC) SUBSYSTEM,
open list:SERIAL DRIVERS, linux-spi, Linux USB List
In-Reply-To: <CAL_JsqK-z+yx6vMv_vUCc-QCigDnN8K3zPkbWM_CgXj02FGY2w@mail.gmail.com>
On Wed, Mar 18, 2020 at 04:37:55PM -0600, Rob Herring wrote:
> On Tue, Mar 17, 2020 at 3:40 AM Lubomir Rintel <lkundrak@v3.sk> wrote:
> >
> > Convert the sdhci-pxa binding to DT schema format using json-schema.
>
> Ignore what my bot said, I see you addressed that earlier in the series.
>
> > At the same time, fix a couple of issues with the examples discovered by
> > the validation tool -- a semicolon instead of a comma and wrong node names.
> >
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > ---
> > .../devicetree/bindings/mmc/sdhci-pxa.txt | 50 ---------
> > .../devicetree/bindings/mmc/sdhci-pxa.yaml | 101 ++++++++++++++++++
> > 2 files changed, 101 insertions(+), 50 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/mmc/sdhci-pxa.txt
> > create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-pxa.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/sdhci-pxa.txt b/Documentation/devicetree/bindings/mmc/sdhci-pxa.txt
> > deleted file mode 100644
> > index 3d1b449d6097d..0000000000000
> > --- a/Documentation/devicetree/bindings/mmc/sdhci-pxa.txt
> > +++ /dev/null
> > @@ -1,50 +0,0 @@
> > -* Marvell sdhci-pxa v2/v3 controller
> > -
> > -This file documents differences between the core properties in mmc.txt
> > -and the properties used by the sdhci-pxav2 and sdhci-pxav3 drivers.
> > -
> > -Required properties:
> > -- compatible: Should be "mrvl,pxav2-mmc", "mrvl,pxav3-mmc" or
> > - "marvell,armada-380-sdhci".
> > -- reg:
> > - * for "mrvl,pxav2-mmc" and "mrvl,pxav3-mmc", one register area for
> > - the SDHCI registers.
> > -
> > - * for "marvell,armada-380-sdhci", three register areas. The first
> > - one for the SDHCI registers themselves, the second one for the
> > - AXI/Mbus bridge registers of the SDHCI unit, the third one for the
> > - SDIO3 Configuration register
> > -- reg names: should be "sdhci", "mbus", "conf-sdio3". only mandatory
> > - for "marvell,armada-380-sdhci"
> > -- clocks: Array of clocks required for SDHCI; requires at least one for
> > - I/O clock.
> > -- clock-names: Array of names corresponding to clocks property; shall be
> > - "io" for I/O clock and "core" for optional core clock.
> > -
> > -Optional properties:
> > -- mrvl,clk-delay-cycles: Specify a number of cycles to delay for tuning.
> > -
> > -Example:
> > -
> > -sdhci@d4280800 {
> > - compatible = "mrvl,pxav3-mmc";
> > - reg = <0xd4280800 0x800>;
> > - bus-width = <8>;
> > - interrupts = <27>;
> > - clocks = <&chip CLKID_SDIO1XIN>, <&chip CLKID_SDIO1>;
> > - clock-names = "io", "core";
> > - non-removable;
> > - mrvl,clk-delay-cycles = <31>;
> > -};
> > -
> > -sdhci@d8000 {
> > - compatible = "marvell,armada-380-sdhci";
> > - reg-names = "sdhci", "mbus", "conf-sdio3";
> > - reg = <0xd8000 0x1000>,
> > - <0xdc000 0x100>;
> > - <0x18454 0x4>;
> > - interrupts = <0 25 0x4>;
> > - clocks = <&gateclk 17>;
> > - clock-names = "io";
> > - mrvl,clk-delay-cycles = <0x1F>;
> > -};
> > diff --git a/Documentation/devicetree/bindings/mmc/sdhci-pxa.yaml b/Documentation/devicetree/bindings/mmc/sdhci-pxa.yaml
> > new file mode 100644
> > index 0000000000000..4ae0926ac294f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mmc/sdhci-pxa.yaml
> > @@ -0,0 +1,101 @@
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mmc/sdhci-pxa.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Marvell PXA SDHCI v2/v3 bindings
> > +
> > +maintainers:
> > + - devicetree@vger.kernel.org
> > +
> > +allOf:
> > + - $ref: mmc-controller.yaml#
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: marvell,armada-380-sdhci
> > + then:
> > + properties:
> > + regs:
> > + minItems: 3
> > + maxItems: 3
>
> Here, you just need minItems.
>
> > + reg-names:
> > + items:
> > + - const: sdhci
> > + - const: mbus
> > + - const: conf-sdio3
>
> This should be under the main definition of 'reg-names' and then just
> 'minItems: 3' here.
>
> > + required:
> > + - reg-names
> > + else:
> > + properties:
> > + regs:
> > + minItems: 1
> > + maxItems: 1
>
> Just 'maxItems' is sufficient.
>
> > + reg-names:
> > + minItems: 1
> > + maxItems: 1
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - mrvl,pxav2-mmc
> > + - mrvl,pxav3-mmc
> > + - marvell,armada-380-sdhci
> > +
> > + reg: true
>
> Here you should have:
>
> minItems: 1
> maxItems: 3
>
> > +
> > + reg-names: true
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + minItems: 1
> > + maxItems: 2
> > +
> > + clock-names:
> > + minItems: 1
> > + maxItems: 2
> > + items:
> > + - const: io
> > + - const: core
> > +
> > + mrvl,clk-delay-cycles:
> > + description: Specify a number of cycles to delay for tuning.
> > + $ref: /schemas/types.yaml#/definitions/uint32
>
> No range of valid values?
No. The document I'm converting didn't specify a range. For some of the
hardware supported Marvell doesn't provide documentation, and the
drivers that use this seem to accept any u32 number.
If this needs a range, then I need to make something up. I can do that,
but it will be difficult to defend that number if anyone asks.
Lubo
^ permalink raw reply
* Re: [PATCH v2 3/6] dmaengine: dw: Set DMA device max segment size parameter
From: Serge Semin @ 2020-05-17 18:22 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Vinod Koul, Vineet Gupta, Viresh Kumar, Dan Williams,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Rob Herring, linux-mips, devicetree, dmaengine,
linux-kernel
In-Reply-To: <20200515105313.GL185537@smile.fi.intel.com>
On Fri, May 15, 2020 at 01:53:13PM +0300, Andy Shevchenko wrote:
> On Fri, May 15, 2020 at 11:46:01AM +0530, Vinod Koul wrote:
> > On 12-05-20, 15:35, Andy Shevchenko wrote:
> > > On Tue, May 12, 2020 at 12:16:22AM +0300, Serge Semin wrote:
> > > > On Fri, May 08, 2020 at 02:21:52PM +0300, Andy Shevchenko wrote:
> > > > > On Fri, May 08, 2020 at 01:53:01PM +0300, Serge Semin wrote:
>
> ...
>
> > > My point here that we probably can avoid complications till we have real
> > > hardware where it's different. As I said I don't remember a such, except
> > > *maybe* Intel Medfield, which is quite outdated and not supported for wider
> > > audience anyway.
> >
> > IIRC Intel Medfield has couple of dma controller instances each one with
> > different parameters *but* each instance has same channel configuration.
>
> That's my memory too.
>
> > I do not recall seeing that we have synthesis parameters per channel
> > basis... But I maybe wrong, it's been a while.
>
> Exactly, that's why I think we better simplify things till we will have real
> issue with it. I.o.w. no need to solve the problem which doesn't exist.
Ok then. My hardware is also synthesized with uniform max block size
parameter. I'll remove that maximum of maximum search pattern and use the block
size found for the very first channel to set the maximum segment size parameter.
-Sergey
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH v2 2/6] dt-bindings: dma: dw: Add max burst transaction length property
From: Serge Semin @ 2020-05-17 17:47 UTC (permalink / raw)
To: Vinod Koul
Cc: Serge Semin, Andy Shevchenko, Viresh Kumar, Rob Herring,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Dan Williams, linux-mips, dmaengine, devicetree,
linux-kernel
In-Reply-To: <20200515111112.4umynrpgzjnca223@mobilestation>
On Fri, May 15, 2020 at 02:11:13PM +0300, Serge Semin wrote:
> On Fri, May 15, 2020 at 04:26:58PM +0530, Vinod Koul wrote:
> > On 15-05-20, 13:51, Andy Shevchenko wrote:
> > > On Fri, May 15, 2020 at 11:39:11AM +0530, Vinod Koul wrote:
> > > > On 12-05-20, 15:38, Andy Shevchenko wrote:
> > > > > On Tue, May 12, 2020 at 02:49:46PM +0300, Serge Semin wrote:
> > > > > > On Tue, May 12, 2020 at 12:08:04PM +0300, Andy Shevchenko wrote:
> > > > > > > On Tue, May 12, 2020 at 12:35:31AM +0300, Serge Semin wrote:
> > > > > > > > On Tue, May 12, 2020 at 12:01:38AM +0300, Andy Shevchenko wrote:
> > > > > > > > > On Mon, May 11, 2020 at 11:05:28PM +0300, Serge Semin wrote:
> > > > > > > > > > On Fri, May 08, 2020 at 02:12:42PM +0300, Andy Shevchenko wrote:
> > > > > > > > > > > On Fri, May 08, 2020 at 01:53:00PM +0300, Serge Semin wrote:
> > >
> > > ...
> > >
> > > > > I leave it to Rob and Vinod.
> > > > > It won't break our case, so, feel free with your approach.
> > > >
> > > > I agree the DT is about describing the hardware and looks like value of
> > > > 1 is not allowed. If allowed it should be added..
> > >
> > > It's allowed at *run time*, it's illegal in *pre-silicon stage* when
> > > synthesizing the IP.
> >
> > Then it should be added ..
>
> Vinod, max-burst-len is "MAXimum" burst length not "run-time or current or any
> other" burst length. It's a constant defined at the IP-core synthesis stage and
> according to the Data Book, MAX burst length can't be 1. The allowed values are
> exactly as I described in the binding [4, 8, 16, 32, ...]. MAX burst length
> defines the upper limit of the run-time burst length. So setting it to 1 isn't
> about describing a hardware, but using DT for the software convenience.
>
> -Sergey
Vinod, to make this completely clear. According to the DW DMAC data book:
- In general, run-time parameter of the DMA transaction burst length (set in
the SRC_MSIZE/DST_MSIZE fields of the channel control register) may belong
to the set [1, 4, 8, 16, 32, 64, 128, 256].
- Actual upper limit of the burst length run-time parameter is limited by a
constant defined at the IP-synthesize stage (it's called DMAH_CHx_MAX_MULT_SIZE)
and this constant belongs to the set [4, 8, 16, 32, 64, 128, 256]. (See, no 1
in this set).
So the run-time burst length in a case of particular DW DMA controller belongs
to the range:
1 <= SRC_MSIZE <= DMAH_CHx_MAX_MULT_SIZE
and
1 <= DST_MSIZE <= DMAH_CHx_MAX_MULT_SIZE
See. No mater which DW DMA controller we get each of them will at least support
the burst length of 1 and 4 transfer words. This is determined by design of the
DW DMA controller IP since DMAH_CHx_MAX_MULT_SIZE constant set starts with 4.
In this patch I suggest to add the max-burst-len property, which specifies
the upper limit for the run-time burst length. Since the maximum burst length
capable to be set to the SRC_MSIZE/DST_MSIZE fields of the DMA channel control
register is determined by the DMAH_CHx_MAX_MULT_SIZE constant (which can't be 1
by the DW DMA IP design), max-burst-len property as being also responsible for
the maximum burst length setting should be associated with DMAH_CHx_MAX_MULT_SIZE
thus should belong to the same set [4, 8, 16, 32, 64, 128, 256].
So 1 shouldn't be in the enum of the max-burst-len property constraint, because
hardware doesn't support such limitation by design, while setting 1 as
max-burst-len would mean incorrect description of the DMA controller.
Vinod, could you take a look at the info I provided above and say your final word
whether 1 should be really allowed to be in the max-burst-len enum constraints?
I'll do as you say in the next version of the patchset.
Regards,
-Sergey
>
> >
> > --
> > ~Vinod
^ permalink raw reply
* Re: [PATCH v1 8/9] arm64: dts: actions: Add MMC controller support for S700
From: Amit Tomer @ 2020-05-17 17:12 UTC (permalink / raw)
To: André Przywara
Cc: Andreas Färber, Manivannan Sadhasivam, Rob Herring,
cristian.ciocaltea, linux-arm-kernel, linux-actions, devicetree
In-Reply-To: <2cd3cdaf-826e-9d12-9fd4-9f7e2a517ecd@arm.com>
> I don't understand what this has to do with the driver, but I asked
> above to also change the binding, allowing this compatible string
> combination.
if we add these two strings "actions,s700-mmc", "actions,owl-mmc" to dts file
and leave the driver as it. Wouldn't this be mismatch(as driver only
has "actions,owl-mmc"
and DTS has two strings).
Shouldn't that be concerned about ?
Thanks
-Amit
^ permalink raw reply
* Re: [PATCH v1 8/9] arm64: dts: actions: Add MMC controller support for S700
From: André Przywara @ 2020-05-17 16:42 UTC (permalink / raw)
To: Amit Tomer
Cc: Andreas Färber, Manivannan Sadhasivam, Rob Herring,
cristian.ciocaltea, linux-arm-kernel, linux-actions, devicetree
In-Reply-To: <CABHD4K9yjUGuo0w-RfhdZQJm3Wtj6bU2H4DXcp4Jjp=e0fFeyA@mail.gmail.com>
On 17/05/2020 12:56, Amit Tomer wrote:
> Hi,
>
>> So, using "actions,s700-mmc", "actions,owl-mmc" here, adding this combo
>> to the binding, but leaving the driver alone for now.
>
> But if we leave this new string from driver , there would be DT
> validation issue.
I don't understand what this has to do with the driver, but I asked
above to also change the binding, allowing this compatible string
combination.
Cheers,
Andre
> Are we okay with it ?
>
> Thanks
> -Amit
>
^ permalink raw reply
* Re: [PATCH v3] ARM: dts: kirkwood: Add Check Point L-50 board
From: Gregory CLEMENT @ 2020-05-17 16:27 UTC (permalink / raw)
To: Pawel Dembicki
Cc: Pawel Dembicki, Andrew Lunn, Rob Herring, Jason Cooper,
Sebastian Hesselbarth, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20200422150007.29119-1-paweldembicki@gmail.com>
Hello Pawel,
> This patch adds dts for the Check Point L-50 from 600/1100 series
> routers.
>
> Specification:
> -CPU: Marvell Kirkwood 88F6821 1200MHz
> -RAM: 512MB
> -Flash: NAND 512MB
> -WiFi: mPCIe card based on Atheros AR9287 b/g/n
> -WAN: 1 Gigabit Port (Marvell 88E1116R PHY)
> -LAN: 9 Gigabit Ports (2x Marvell 88E6171(5+4))
> -USB: 2x USB2.0
> -Express card slot
> -SD card slot
> -Serial console: RJ-45 115200 8n1
> -Unsupported DSL
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
I've missed there was a v3, so I replaced the v2 patch by this patch.
Gregory
> ---
> Changes in v3:
> - fix typo and code style issues pointed by OpenWrt guys
> Changes in v2:
> - none
>
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/kirkwood-l-50.dts | 438 ++++++++++++++++++++++++++++
> 2 files changed, 439 insertions(+)
> create mode 100644 arch/arm/boot/dts/kirkwood-l-50.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index e8dd99201397..eba030b3ba69 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -279,6 +279,7 @@ dtb-$(CONFIG_MACH_KIRKWOOD) += \
> kirkwood-iomega_ix2_200.dtb \
> kirkwood-is2.dtb \
> kirkwood-km_kirkwood.dtb \
> + kirkwood-l-50.dtb \
> kirkwood-laplug.dtb \
> kirkwood-linkstation-lsqvl.dtb \
> kirkwood-linkstation-lsvl.dtb \
> diff --git a/arch/arm/boot/dts/kirkwood-l-50.dts b/arch/arm/boot/dts/kirkwood-l-50.dts
> new file mode 100644
> index 000000000000..0d81c43a6a73
> --- /dev/null
> +++ b/arch/arm/boot/dts/kirkwood-l-50.dts
> @@ -0,0 +1,438 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Check Point L-50 Board Description
> + * Copyright 2020 Pawel Dembicki <paweldembicki@gmail.com>
> + */
> +
> +/dts-v1/;
> +
> +#include "kirkwood.dtsi"
> +#include "kirkwood-6281.dtsi"
> +
> +/ {
> + model = "Check Point L-50";
> + compatible = "checkpoint,l-50", "marvell,kirkwood-88f6281", "marvell,kirkwood";
> +
> + memory {
> + device_type = "memory";
> + reg = <0x00000000 0x20000000>;
> + };
> +
> + chosen {
> + bootargs = "console=ttyS0,115200n8";
> + stdout-path = &uart0;
> + };
> +
> + ocp@f1000000 {
> + pinctrl: pin-controller@10000 {
> + pinctrl-0 = <&pmx_led38 &pmx_sysrst &pmx_button29>;
> + pinctrl-names = "default";
> +
> + pmx_sysrst: pmx-sysrst {
> + marvell,pins = "mpp6";
> + marvell,function = "sysrst";
> + };
> +
> + pmx_button29: pmx_button29 {
> + marvell,pins = "mpp29";
> + marvell,function = "gpio";
> + };
> +
> + pmx_led38: pmx_led38 {
> + marvell,pins = "mpp38";
> + marvell,function = "gpio";
> + };
> +
> + pmx_sdio_cd: pmx-sdio-cd {
> + marvell,pins = "mpp46";
> + marvell,function = "gpio";
> + };
> + };
> +
> + serial@12000 {
> + status = "okay";
> + };
> +
> + mvsdio@90000 {
> + status = "okay";
> + cd-gpios = <&gpio1 14 9>;
> + };
> +
> + i2c@11000 {
> + status = "okay";
> + clock-frequency = <400000>;
> +
> + gpio2: gpio-expander@20{
> + #gpio-cells = <2>;
> + #interrupt-cells = <2>;
> + compatible = "semtech,sx1505q";
> + reg = <0x20>;
> +
> + gpio-controller;
> + };
> +
> + /* Three GPIOs from 0x21 exp. are undescribed in dts:
> + * 1: DSL module reset (active low)
> + * 5: mPCIE reset (active low)
> + * 6: Express card reset (active low)
> + */
> + gpio3: gpio-expander@21{
> + #gpio-cells = <2>;
> + #interrupt-cells = <2>;
> + compatible = "semtech,sx1505q";
> + reg = <0x21>;
> +
> + gpio-controller;
> + };
> +
> + rtc@30 {
> + compatible = "s35390a";
> + reg = <0x30>;
> + };
> + };
> + };
> +
> + leds {
> + compatible = "gpio-leds";
> +
> + status_green {
> + label = "l-50:green:status";
> + gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
> + };
> +
> + status_red {
> + label = "l-50:red:status";
> + gpios = <&gpio3 2 GPIO_ACTIVE_LOW>;
> + };
> +
> + wifi {
> + label = "l-50:green:wifi";
> + gpios = <&gpio2 7 GPIO_ACTIVE_LOW>;
> + linux,default-trigger = "phy0tpt";
> + };
> +
> + internet_green {
> + label = "l-50:green:internet";
> + gpios = <&gpio2 3 GPIO_ACTIVE_LOW>;
> + };
> +
> + internet_red {
> + label = "l-50:red:internet";
> + gpios = <&gpio2 1 GPIO_ACTIVE_LOW>;
> + };
> +
> + usb1_green {
> + label = "l-50:green:usb1";
> + gpios = <&gpio2 0 GPIO_ACTIVE_LOW>;
> + linux,default-trigger = "usbport";
> + trigger-sources = <&hub_port3>;
> + };
> +
> + usb1_red {
> + label = "l-50:red:usb1";
> + gpios = <&gpio2 4 GPIO_ACTIVE_LOW>;
> + };
> +
> + usb2_green {
> + label = "l-50:green:usb2";
> + gpios = <&gpio2 2 GPIO_ACTIVE_LOW>;
> + linux,default-trigger = "usbport";
> + trigger-sources = <&hub_port1>;
> + };
> +
> + usb2_red {
> + label = "l-50:red:usb2";
> + gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
> + };
> + };
> +
> + usb2_pwr {
> + compatible = "regulator-fixed";
> + regulator-name = "usb2_pwr";
> +
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + gpio = <&gpio3 3 GPIO_ACTIVE_LOW>;
> + regulator-always-on;
> + };
> +
> + usb1_pwr {
> + compatible = "regulator-fixed";
> + regulator-name = "usb1_pwr";
> +
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + gpio = <&gpio3 4 GPIO_ACTIVE_LOW>;
> + regulator-always-on;
> + };
> +
> + mpcie_pwr {
> + compatible = "regulator-fixed";
> + regulator-name = "mpcie_pwr";
> +
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + gpio = <&gpio3 5 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + regulator-always-on;
> + };
> +
> + express_card_pwr {
> + compatible = "regulator-fixed";
> + regulator-name = "express_card_pwr";
> +
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + gpio = <&gpio3 7 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + regulator-always-on;
> + };
> +
> + keys {
> + compatible = "gpio-keys";
> +
> + factory_defaults {
> + label = "factory_defaults";
> + gpios = <&gpio0 29 GPIO_ACTIVE_LOW>;
> + linux,code = <KEY_RESTART>;
> + };
> + };
> +};
> +
> +&mdio {
> + status = "okay";
> +
> + ethphy8: ethernet-phy@8 {
> + reg = <0x08>;
> + };
> +
> + switch0: switch@10 {
> + compatible = "marvell,mv88e6085";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x10>;
> + dsa,member = <0 0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + label = "lan5";
> + };
> +
> + port@1 {
> + reg = <1>;
> + label = "lan1";
> + };
> +
> + port@2 {
> + reg = <2>;
> + label = "lan6";
> + };
> +
> + port@3 {
> + reg = <3>;
> + label = "lan2";
> + };
> +
> + port@4 {
> + reg = <4>;
> + label = "lan7";
> + };
> +
> + switch0port5: port@5 {
> + reg = <5>;
> + phy-mode = "rgmii-txid";
> + link = <&switch1port5>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> +
> + port@6 {
> + reg = <6>;
> + label = "cpu";
> + phy-mode = "rgmii-id";
> + ethernet = <ð1port>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + };
> + };
> +
> + switch@11 {
> + compatible = "marvell,mv88e6085";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x11>;
> + dsa,member = <0 1>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + label = "lan3";
> + };
> +
> + port@1 {
> + reg = <1>;
> + label = "lan8";
> + };
> +
> + port@2 {
> + reg = <2>;
> + label = "lan4";
> + };
> +
> + port@3 {
> + reg = <3>;
> + label = "dmz";
> + };
> +
> + switch1port5: port@5 {
> + reg = <5>;
> + phy-mode = "rgmii-txid";
> + link = <&switch0port5>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> +
> + port@6 {
> + reg = <6>;
> + label = "dsl";
> + fixed-link {
> + speed = <100>;
> + full-duplex;
> + };
> + };
> + };
> + };
> +};
> +
> +ð0 {
> + status = "okay";
> + ethernet0-port@0 {
> + phy-handle = <ðphy8>;
> + };
> +};
> +
> +ð1 {
> + status = "okay";
> + ethernet1-port@0 {
> + speed = <1000>;
> + duplex = <1>;
> + };
> +};
> +
> +&nand {
> + status = "okay";
> + pinctrl-0 = <&pmx_nand>;
> + pinctrl-names = "default";
> +
> + partition@0 {
> + label = "u-boot";
> + reg = <0x00000000 0x000c0000>;
> + };
> +
> + partition@a0000 {
> + label = "bootldr-env";
> + reg = <0x000c0000 0x00040000>;
> + };
> +
> + partition@100000 {
> + label = "kernel-1";
> + reg = <0x00100000 0x00800000>;
> + };
> +
> + partition@900000 {
> + label = "rootfs-1";
> + reg = <0x00900000 0x07100000>;
> + };
> +
> + partition@7a00000 {
> + label = "kernel-2";
> + reg = <0x07a00000 0x00800000>;
> + };
> +
> + partition@8200000 {
> + label = "rootfs-2";
> + reg = <0x08200000 0x07100000>;
> + };
> +
> + partition@f300000 {
> + label = "default_sw";
> + reg = <0x0f300000 0x07900000>;
> + };
> +
> + partition@16c00000 {
> + label = "logs";
> + reg = <0x16c00000 0x01800000>;
> + };
> +
> + partition@18400000 {
> + label = "preset_cfg";
> + reg = <0x18400000 0x00100000>;
> + };
> +
> + partition@18500000 {
> + label = "adsl";
> + reg = <0x18500000 0x00100000>;
> + };
> +
> + partition@18600000 {
> + label = "storage";
> + reg = <0x18600000 0x07a00000>;
> + };
> +};
> +
> +&rtc {
> + status = "disabled";
> +};
> +
> +&pciec {
> + status = "okay";
> +};
> +
> +&pcie0 {
> + status = "okay";
> +};
> +
> +&sata_phy0 {
> + status = "disabled";
> +};
> +
> +&sata_phy1 {
> + status = "disabled";
> +};
> +
> +&usb0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "okay";
> +
> + port@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> + #trigger-source-cells = <0>;
> +
> + hub_port1: port@1 {
> + reg = <1>;
> + #trigger-source-cells = <0>;
> + };
> +
> + hub_port3: port@3 {
> + reg = <3>;
> + #trigger-source-cells = <0>;
> + };
> + };
> +};
> --
> 2.20.1
>
--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com
^ permalink raw reply
* Re: [PATCH v5 5/6] iio: imu: Add support for adis16475
From: Jonathan Cameron @ 2020-05-17 16:15 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Nuno Sá, Nuno Sá, linux-iio, devicetree, Hartmut Knaack,
Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
Alexandru Ardelean, Michael Hennerich
In-Reply-To: <b7289aca-f393-faca-f512-a952a77c1e68@metafoo.de>
On Sun, 3 May 2020 11:12:34 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 5/3/20 11:07 AM, Lars-Peter Clausen wrote:
> > On 5/3/20 10:47 AM, Jonathan Cameron wrote:
> >> On Sat, 02 May 2020 21:52:18 +0200
> >> Nuno Sá <noname.nuno@gmail.com> wrote:
> >>
> >>> On Sat, 2020-05-02 at 20:01 +0200, Lars-Peter Clausen wrote:
> >>>> On 5/2/20 7:40 PM, Jonathan Cameron wrote:
> >>>>> On Mon, 27 Apr 2020 20:06:07 +0200
> >>>>> Lars-Peter Clausen <lars@metafoo.de> wrote:
> >>>>>> On 4/13/20 10:24 AM, Nuno Sá wrote:
> >>>>>>> [...]
> >>>>>>> +static irqreturn_t adis16475_trigger_handler(int irq, void *p)
> >>>>>>> +{
> >>>>>>> [...]
> >>>>>>> + __be16 data[ADIS16475_MAX_SCAN_DATA], *buffer;
> >>>>>>> [...]
> >>>>>>> +
> >>>>>>> + iio_push_to_buffers_with_timestamp(indio_dev, data, pf-
> >>>>>>>> timestamp);
> >>>>>> If the timestamp is enabled the IIO core might insert padding
> >>>>>> between
> >>>>>> the data channels and the timestamp. If that happens this will
> >>>>>> disclose
> >>>>>> kernel stack memory to userspace.
> >>>>>>
> >>>>>> This needs either a memset(data, 0x00, sizeof(data)) or maybe put
> >>>>>> data
> >>>>>> into the state struct and kzalloc it.
> >>>>> Good spot. Could simply do __be16 data[ADI..] = {0}; rather than
> >>>>> explicit
> >>>>> memset, but some form of zeroization is needed.
> >>>>>
> >>>>> I've fixed up the applied patch with the above approach.
> >>>> There is actually another issue. The stack data is not necessarily
> >>>> aligned to 64 bit, which causes issues if we try to put the 64-bit
> >>> Oh, this is actually more problematic. Yes, since we have an array of
> >>> u16, that is not guaranteed to be 64bit aligned. Doing a quick search
> >>> of `iio_push_to_buffers_with_timestamp()` users and I could quickly
> >>> find 4/5 drivers with the same problem. I guess the API should clearly
> >>> state that `data` needs to be __at least__ 64 bits aligned (maybe a
> >>> future patch). Or we could even check the address and guarantee that it
> >>> is properly aligned before continuing (though Im guessing this will
> >>> break a lot of users...)
> >>>> timestamp in it. I think data should really be in the state struct.
> >>> Yes, with a proper __aligned(8) attribute... Or couldn't we just use
> >>> __aligned(8) on the stack variable?
> >> Forcing alignment on the stack isn't terribly reliable, which is why
> >> we never do that for dma safe buffers.
> >>
> >> Probably better to just move it to the state structure.
> >> I'll fix it up to do that. Please sanity check what will shortly
> >> be in the testing branch.
> >>
> >> The moment Lars mentioned this I groaned. As you've noted a few other
> >> drivers have the same problem + the ABI doesn't clearly state
> >> or check this.
> >>
> >> We should certainly fix all the drivers that suffer this problem
> >> first then we can think about adding a runtime check.
> >
> > It looks like it is actually quite a few drivers, maybe we should
> > switch to put_unaligned(). We probably got lucky in most cases and the
> > buffer is naturally aligned to 64 bit.
Just a quick update on this. I've been taking a deeper look and there
are some 'interesting' cases in here so the put_unaligned is attractive
unfortunately I don't think we can go that way because it would be
reasonable for consumers of the buffer to expect it to be appropriately
aligned. We need to rework many of these anyway to fix the related
data leak.
I've done some of below and will post shortly - a few will take more
effort and probably need testing rather than just relying on review.
So far the 'interesting ones' are mpu3050 and isl29501.
J
> >
> > But the reason I noticed this is because I ran into the issue in the
> > wild where the timestamp ended up at the wrong offset in the buffer,
> > so it does happen.
> >
> > The following semantic patch finds affected drivers.
> >
> > @@
> > type T;
> > identifier buf;
> > expression N;
> > expression ts;
> > expression indio_dev;
> > @@
> > *T buf[N];
> > ...
> > *iio_push_to_buffers_with_timestamp(indio_dev, buf, ts)
> >
> > Matched files:
> >
> > --- drivers/iio/health/afe4403.c
> > --- drivers/iio/health/afe4404.c
> > --- drivers/iio/gyro/mpu3050-core.c
> > --- drivers/iio/gyro/itg3200_buffer.c
> > --- drivers/iio/chemical/ccs811.c
> > --- drivers/iio/chemical/sps30.c
> > --- drivers/iio/chemical/pms7003.c
> > --- drivers/iio/proximity/isl29501.c
> > --- drivers/iio/proximity/mb1232.c
> > --- drivers/iio/accel/kxsd9.c
> > --- drivers/iio/accel/mma8452.c
> > --- drivers/iio/accel/bmc150-accel-core.c
> > --- drivers/iio/accel/mma7455_core.c
> > --- drivers/iio/adc/ti-adc081c.c
> > --- drivers/iio/adc/ti-adc084s021.c
> > --- drivers/iio/adc/ti-ads1015.c
> > --- drivers/iio/adc/ti-ads124s08.c
> > --- drivers/iio/adc/ina2xx-adc.c
> > --- drivers/iio/adc/ti-ads8688.c
> > --- drivers/iio/adc/ti-adc0832.c
> > --- drivers/iio/adc/ti-adc12138.c
> > --- drivers/iio/adc/max1118.c
> > --- drivers/iio/adc/ad_sigma_delta.c
> > --- drivers/iio/light/si1145.c
> > --- drivers/iio/light/vcnl4035.c
> > --- drivers/iio/light/max44000.c
> > --- drivers/iio/light/rpr0521.c
> > --- drivers/iio/light/st_uvis25_core.c
> > --- drivers/iio/light/ltr501.c
> > --- drivers/iio/magnetometer/ak8974.c
> > --- drivers/iio/magnetometer/mag3110.c
> > --- drivers/iio/magnetometer/ak8975.c
> > --- drivers/iio/humidity/hdc100x.c
> > --- drivers/iio/humidity/hts221_buffer.c
> > --- drivers/iio/imu/bmi160/bmi160_core.c
> > --- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > --- drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> > --- drivers/iio/pressure/ms5611_core.c
> > --- drivers/iio/pressure/mpl3115.c
> >
> Most of these drivers also seem to have the issue that they leak stack
> memory. Only 11 of them clear the buffer.
>
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: add uDPU i2c bus recovery
From: Gregory CLEMENT @ 2020-05-17 16:10 UTC (permalink / raw)
To: Russell King, linux-i2c
Cc: Andrew Lunn, devicetree, Jason Cooper, linux-arm-kernel,
Rob Herring, Sebastian Hesselbarth, Vladimir Vid
In-Reply-To: <E1jWGXd-0000Z7-1n@rmk-PC.armlinux.org.uk>
Hi Russell,
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Applied on mvebu/dt64
Thanks,
Gregory
> ---
> .../boot/dts/marvell/armada-3720-uDPU.dts | 22 +++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts b/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
> index 7eb6c1796cef..95d46e8d081c 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-uDPU.dts
> @@ -117,18 +117,36 @@
> };
> };
>
> +&pinctrl_nb {
> + i2c1_recovery_pins: i2c1-recovery-pins {
> + groups = "i2c1";
> + function = "gpio";
> + };
> +
> + i2c2_recovery_pins: i2c2-recovery-pins {
> + groups = "i2c2";
> + function = "gpio";
> + };
> +};
> +
> &i2c0 {
> status = "okay";
> - pinctrl-names = "default";
> + pinctrl-names = "default", "recovery";
> pinctrl-0 = <&i2c1_pins>;
> + pinctrl-1 = <&i2c1_recovery_pins>;
> /delete-property/mrvl,i2c-fast-mode;
> + scl-gpios = <&gpionb 0 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> + sda-gpios = <&gpionb 1 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> };
>
> &i2c1 {
> status = "okay";
> - pinctrl-names = "default";
> + pinctrl-names = "default", "recovery";
> pinctrl-0 = <&i2c2_pins>;
> + pinctrl-1 = <&i2c2_recovery_pins>;
> /delete-property/mrvl,i2c-fast-mode;
> + scl-gpios = <&gpionb 2 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> + sda-gpios = <&gpionb 3 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>
> lm75@48 {
> status = "okay";
> --
> 2.20.1
>
--
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com
^ 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