* Re: [PATCH v5 0/6] drm: Prepare to use a GPIO on ti-sn65dsi86 for Hot Plug Detect
From: Doug Anderson @ 2020-05-18 14:50 UTC (permalink / raw)
To: Sam Ravnborg, LinusW, Stephen Boyd, Laurent Pinchart
Cc: Bartosz Golaszewski, David Airlie, Daniel Vetter, Rob Herring,
Neil Armstrong, Andrzej Hajda, Sandeep Panda, Rob Clark,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jernej Skrabec, Jeffrey Hugo, Stephen Boyd, linux-arm-msm,
Jonas Karlman, dri-devel, open list:GPIO SUBSYSTEM, Andy Gross,
Thierry Reding, Krzysztof Kozlowski, Paul Walmsley,
Bjorn Andersson, LKML
In-Reply-To: <CAD=FV=VBU7JmTdvgWjyj_ytrFmz6Gkx2OjVr1FxLh9DBG_jN6w@mail.gmail.com>
Sam,
On Sat, May 9, 2020 at 3:48 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Sat, May 9, 2020 at 1:15 PM Sam Ravnborg <sam@ravnborg.org> wrote:
> >
> > Hi Douglas.
> >
> > On Thu, May 07, 2020 at 02:34:54PM -0700, Douglas Anderson wrote:
> > >
> > > As talked about in commit c2bfc223882d ("drm/bridge: ti-sn65dsi86:
> > > Remove the mystery delay"), the normal HPD pin on ti-sn65dsi86 is
> > > kinda useless, at least for embedded DisplayPort (eDP). However,
> > > despite the fact that the actual HPD pin on the bridge is mostly
> > > useless for eDP, the concept of HPD for eDP still makes sense. It
> > > allows us to optimize out a hardcoded delay that many panels need if
> > > HPD isn't hooked up. Panel timing diagrams show HPD as one of the
> > > events to measure timing from and we have to assume the worst case if
> > > we can't actually read HPD.
> > >
> > > One way to use HPD for eDP without using the mostly useless HPD pin on
> > > ti-sn65dsi86 is to route the panel's HPD somewhere else in the system,
> > > like to a GPIO. This works great because eDP panels aren't physically
> > > hotplugged. That means the debouncing logic that caused us problems
> > > wasn't really needed and a raw GPIO works great.
> > >
> > > As per the above, a smart board designer would realize the value of
> > > HPD and choose to route it to a GPIO somewhere on the board to avoid
> > > the silly sn65dsi86 debouncing. While said "smart designer" could
> > > theoretically route HPD anywhere on the board, a really smart designer
> > > would realize that there are several GPIOs on the bridge itself that
> > > are nearly useless for anything but this purpose and route HPD to one
> > > of those.
> > >
> > > This series of patches is intended to allow the scenario described
> > > above.
> > >
> > > This patch has been tested on a board that is not yet mainline. On
> > > the hardware I have:
> > > - Panel spec says HPD could take up to 200 ms to come up, so without
> > > HPD hooked up we need to delay 200 ms.
> > > - On my board the panel is powered by the same rail as the
> > > touchscreen. By chance of probe order the touchscreen comes up
> > > first. This means by the time we check HPD in ti_sn_bridge_enable()
> > > it's already up. Thus we can use the panel on 200 ms earlier.
> > > - If I measure HPD on this pane it comes up ~56 ms after the panel is
> > > powered. This means I can save 144 ms of delay.
> > >
> > > Side effects (though not main goals) of this series are:
> > > - ti-sn65dsi86 GPIOs are now exported in Linux.
> > > - ti-sn65dsi86 bindings are converted to yaml.
> > > - Common panel bindings now have "hpd-gpios" listed.
> > > - The simple-panel driver in Linux can delay in prepare based on
> > > "hpd-gpios"
> > > - ti-sn65dsi86 bindings (and current user) now specifies "no-hpd"
> > > if HPD isn't hooked up.
> > >
> > > Changes in v5:
> > > - Use of_xlate so that numbers in dts start at 1, not 0.
> > > - Squash https://lore.kernel.org/r/20200506140208.v2.2.I0a2bca02b09c1fcb6b09479b489736d600b3e57f@changeid/
> > >
> > > Changes in v4:
> > > - Don't include gpio.h
> > > - Use gpiochip_get_data() instead of container_of() to get data.
> > > - GPIOF_DIR_XXX => GPIO_LINE_DIRECTION_XXX
> > > - Use Linus W's favorite syntax to read a bit from a bitfield.
> > > - Define and use SN_GPIO_MUX_MASK.
> > > - Add a comment about why we use a bitmap for gchip_output.
> > > - Tacked on "or is otherwise unusable." to description.
> > >
> > > Changes in v3:
> > > - Becaue => Because
> > > - Add a kernel-doc to our pdata to clarify double-duty of gchip_output.
> > > - More comments about how powering off affects us (get_dir, dir_input).
> > > - Cleanup tail of ti_sn_setup_gpio_controller() to avoid one "return".
> > > - Use a bitmap rather than rolling my own.
> > > - Remind how gpio_get_optional() works in the commit message.
> > > - useful implement => useful to implement
> > >
> > > Changes in v2:
> > > - ("Export...GPIOs") is 1/2 of replacement for ("Allow...bridge GPIOs")
> > > - ("dt-bindings: display: Add hpd-gpios to panel-common...") new for v2
> > > - ("simple...hpd-gpios") is 1/2 of replacement for ("Allow...bridge GPIOs")
> > > - specification => specifier.
> > > - power up => power.
> > > - Added back missing suspend-gpios.
> > > - data-lanes and lane-polarities are are the right place now.
> > > - endpoints don't need to be patternProperties.
> > > - Specified more details for data-lanes and lane-polarities.
> > > - Added old example back in, fixing bugs in it.
> > > - Example i2c bus is just called "i2c", not "i2c1" now.
> > > - ("dt-bindings: drm/bridge: ti-sn65dsi86: Document no-hpd") new for v2.
> > > - ("arm64: dts: sdm845: Add "no-hpd" to sn65dsi86 on cheza") new for v2.
> > >
> > > Douglas Anderson (6):
> > > drm/bridge: ti-sn65dsi86: Export bridge GPIOs to Linux
> > > dt-bindings: display: Add hpd-gpios to panel-common bindings
> > > drm/panel-simple: Support hpd-gpios for delaying prepare()
> > > dt-bindings: drm/bridge: ti-sn65dsi86: Convert to yaml
> > > dt-bindings: drm/bridge: ti-sn65dsi86: Document no-hpd
> > > arm64: dts: sdm845: Add "no-hpd" to sn65dsi86 on cheza
> >
> > Applied:
> > > dt-bindings: display: Add hpd-gpios to panel-common bindings
> > > drm/panel-simple: Support hpd-gpios for delaying prepare()
> > to drm-misc-next.
> >
> > The others was missing reviews so we need to wait for feedback.
>
> Thanks!
>
> Given the previous feedback from Linus W, Stephen, and Laurent I
> expect things are good enough to land now, but it'd be good to get
> confirmation (I removed some of the previous tags just to get
> confirmation). If we can get review tags early next week maybe it'll
> still be in time to land for 5.8?
I think all the others have reviews now. Is there anything blocking
them from getting applied?
Thanks!
-Doug
^ permalink raw reply
* [PATCH v3 1/2] dt-bindings: serial: Add binding for rs485 bus termination GPIO
From: Lukas Wunner @ 2020-05-18 14:45 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring
Cc: Matwey V. Kornilov, Giulio Benetti, Heiko Stuebner,
Andy Shevchenko, Christoph Muellner, Jan Kiszka, linux-serial,
devicetree
In-Reply-To: <cover.1589811297.git.lukas@wunner.de>
Commit e8759ad17d41 ("serial: uapi: Add support for bus termination")
introduced the ability to enable rs485 bus termination from user space.
So far the feature is only used by a single driver, 8250_exar.c, using a
hardcoded GPIO pin specific to Siemens IOT2040 products.
Provide for a more generic solution by allowing specification of an
rs485 bus termination GPIO pin in the device tree. An upcoming commit
implements support for this pin for any 8250 driver. The binding is
used in device trees of the "Revolution Pi" PLCs offered by KUNBUS.
[Heiko Stuebner converted the binding to YAML, hence his Signed-off-by.]
Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
---
Documentation/devicetree/bindings/serial/rs485.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/serial/rs485.yaml b/Documentation/devicetree/bindings/serial/rs485.yaml
index d4beaf11222d..a9ad17864889 100644
--- a/Documentation/devicetree/bindings/serial/rs485.yaml
+++ b/Documentation/devicetree/bindings/serial/rs485.yaml
@@ -43,3 +43,7 @@ properties:
rs485-rx-during-tx:
description: enables the receiving of data even while sending data.
$ref: /schemas/types.yaml#/definitions/flag
+
+ rs485-term-gpios:
+ description: GPIO pin to enable RS485 bus termination.
+ maxItems: 1
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v3 2/7] dt-bindings: mdf: ti,j721e-syscon.yaml: Add J721e system controller
From: Rob Herring @ 2020-05-18 14:51 UTC (permalink / raw)
To: Roger Quadros
Cc: t-kristo, kishon, nm, nsekhar, vigneshr, devicetree, linux-kernel
In-Reply-To: <20200508082937.14171-3-rogerq@ti.com>
On Fri, May 08, 2020 at 11:29:32AM +0300, Roger Quadros wrote:
> Add DT binding schema for J721e system controller.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> .../bindings/mfd/ti,j721e-syscon.yaml | 69 +++++++++++++++++++
> 1 file changed, 69 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/ti,j721e-syscon.yaml
Use the compatible string for filename.
>
> diff --git a/Documentation/devicetree/bindings/mfd/ti,j721e-syscon.yaml b/Documentation/devicetree/bindings/mfd/ti,j721e-syscon.yaml
> new file mode 100644
> index 000000000000..e832fb43f884
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/ti,j721e-syscon.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/ti,j721e-syscon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI J721e System Controller Registers R/W Device Tree Bindings
> +
> +description: |
> + This represents the Control Module registers (CTRL_MMR0) on the SoC.
> + System controller node represents a register region containing a set
> + of miscellaneous registers. The registers are not cohesive enough to
> + represent as any specific type of device. The typical use-case is
> + for some other node's driver, or platform-specific code, to acquire
> + a reference to the syscon node (e.g. by phandle, node path, or
> + search using a specific compatible value), interrogate the node (or
> + associated OS driver) to determine the location of the registers,
> + and access the registers directly.
> +
> +maintainers:
> + - Kishon Vijay Abraham I <kishon@ti.com>
> + - Roger Quadros <rogerq@ti.com
> +
> +allOf:
> + - $ref: "syscon.yaml#"
Drop this as it is not needed.
> +
> +properties:
> + compatible:
> + anyOf:
> + - items:
> + - enum:
> + - ti,j721e-system-controller
> +
> + - const: syscon
What about simple-mfd?
> +
> + - contains:
> + const: syscon
> + additionalItems: true
Should be dropped.
You are going to need a 'select' entry or this will match on any syscon
or simple-mfd node:
select:
properties:
compatible:
contains:
const: ti,j721e-system-controller
required:
- compatible
> +
> +# Optional children
> +
> + "^serdes-ln-ctrl@[0-9a-f]+$":
> + type: object
> + description: |
> + This is the SERDES lane control mux. It should follow the bindings
> + specified in
> + Documentation/devicetree/bindings/mux/reg-mux.txt
Really this should reference a 'reg-mux' schema, but fine for now. Bonus
points if you do convert reg-mux.txt though.
> +
> +required:
> + - compatible
> + - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + scm_conf: scm-conf@100000 {
> + compatible = "ti,j721e-system-controller", "syscon", "simple-mfd";
> + reg = <0x00100000 0x1c000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
These aren't documented and you need 'ranges' for the child address to
be translatable.
> +
> + serdes_ln_ctrl: serdes-ln-ctrl@4080 {
> + compatible = "mmio-mux";
> + reg = <0x00004080 0x50>;
> + };
> + };
> +...
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
^ permalink raw reply
* Re: [PATCH v3 3/7] dt-bindings: interrupt-controller: Convert mti,gic to DT schema
From: Serge Semin @ 2020-05-18 14:51 UTC (permalink / raw)
To: Thomas Gleixner, Jason Cooper, Marc Zyngier
Cc: Rob Herring, Ralf Baechle, Arnd Bergmann, linux-mips, devicetree,
Thomas Gleixner, Daniel Lezcano, linux-rtc, Marc Zyngier,
Alexey Malahov, Alexandre Belloni, linux-kernel, Jason Cooper,
Thomas Bogendoerfer, Alessandro Zummo, Paul Burton
In-Reply-To: <20200514190632.GA9943@bogus>
Thomas, Jason, Marc
Could you take a look at this patch and merge it in if you are ok with its
content. We've got Rob's Reviewed-by tag here, so it's only waiting for your
acceptance.
-Sergey
On Thu, May 14, 2020 at 02:06:32PM -0500, Rob Herring wrote:
> On Thu, 7 May 2020 00:41:03 +0300, Serge Semin wrote:
> > Modern device tree bindings are supposed to be created as YAML-files
> > in accordance with DT schema. This commit replaces MIPS GIC legacy bare
> > text binding with YAML file. As before the binding file states that the
> > corresponding dts node is supposed to be compatible with MIPS Global
> > Interrupt Controller indicated by the "mti,gic" compatible string and
> > to provide a mandatory interrupt-controller and '#interrupt-cells'
> > properties. There might be optional registers memory range,
> > "mti,reserved-cpu-vectors" and "mti,reserved-ipi-vectors" properties
> > specified.
> >
> > MIPS GIC also includes a free-running global timer, per-CPU count/compare
> > timers, and a watchdog. Since currently the GIC Timer is only supported the
> > DT schema expects an IRQ and clock-phandler charged timer sub-node with
> > "mti,mips-gic-timer" compatible string.
> >
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > Cc: Paul Burton <paulburton@kernel.org>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > Cc: Alessandro Zummo <a.zummo@towertech.it>
> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: linux-mips@vger.kernel.org
> > Cc: linux-rtc@vger.kernel.org
> >
> > ---
> >
> > I don't really know who is the corresponding driver maintainer, so I
> > added Paul to the maintainers property since he used to be looking for the
> > MIPS arch and Thomas looking after it now. Any idea what email should be
> > specified there instead?
> >
> > Changelog v3:
> > - Since timer sub-node has no unit-address, the node shouldn't be named
> > with one. So alter the MIPS GIC bindings to have a pure "timer"
> > sub-node.
> > - Discard allOf: [ $ref: /schemas/interrupt-controller.yaml# ].
> > - Since it's a conversion patch use GPL-2.0-only SPDX header.
> > ---
> > .../interrupt-controller/mips-gic.txt | 67 --------
> > .../interrupt-controller/mti,gic.yaml | 148 ++++++++++++++++++
> > 2 files changed, 148 insertions(+), 67 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/interrupt-controller/mips-gic.txt
> > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/mti,gic.yaml
> >
>
> Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 10/19] spi: dw: Use DMA max burst to set the request thresholds
From: Andy Shevchenko @ 2020-05-18 14:48 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Mark Brown, Alexey Malahov, Thomas Bogendoerfer,
Paul Burton, Ralf Baechle, Arnd Bergmann, Allison Randal,
Gareth Williams, Rob Herring, linux-mips, devicetree,
Georgy Vlasov, Ramil Zaripov, Jarkko Nikula, Thomas Gleixner,
Wan Ahmad Zainie, Linus Walleij, Clement Leger, linux-spi,
Linux Kernel Mailing List
In-Reply-To: <20200518134306.7rvydoasv7pmxwxl@mobilestation>
On Mon, May 18, 2020 at 04:43:06PM +0300, Serge Semin wrote:
> On Mon, May 18, 2020 at 04:25:20PM +0300, Andy Shevchenko wrote:
> > On Mon, May 18, 2020 at 3:53 PM Serge Semin
> > <Sergey.Semin@baikalelectronics.ru> wrote:
> > > On Mon, May 18, 2020 at 02:03:43PM +0300, Andy Shevchenko wrote:
> > > > On Sat, May 16, 2020 at 11:01:33PM +0300, Serge Semin wrote:
> > > > > On Fri, May 15, 2020 at 05:38:42PM +0300, Andy Shevchenko wrote:
> > > > > > On Fri, May 15, 2020 at 01:47:49PM +0300, Serge Semin wrote:
...
> > > > > It's not like anyone cared about padding in this structure in the first place)
> > > >
> > > > I think I have been caring (to some extend).
> > >
> > > Well, If you have then instead of asking to rearrange just two members (which
> > > by the way finely grouped by the Tx-Rx affiliation) why not sending a
> > > patch, which would refactor the whole structure so to be optimal for the x64
> > > platforms? I don't really see why this gets very important for you seeing
> > > Mark is Ok with this. My current commit follows the common driver design
> > > including the DW SSI data members grouping. On the second thought I'll leave
> > > it as is then.
> >
> > Again same issue here. What is really easy to do for you here, will
> > become a burden and additional churn to anybody else.
> > So, why not to minimize it in the first place? Same with comma in
> > another patch. Sorry, I really don't get it.
>
> If comma is more or less understandable (though adding it is absolutely
> redundant there and doesn't worth even a bit of time spending for the
> discussion), here you consider the patch from padding point of view.
> The driver developer didn't care about it, but did care about grouping the
> members in a corresponding way. The padding burden will be there anyway and
> should be fixed for the whole structure in an additional patch. Until then
> the way of grouping should be preserved.
Like you said, we spent already much more time than that simple change can be
satisfied. And like you said, "deleloper ... did care about groupping members
in a corresponding way". So, if we look at this in the original code, my
suggestion, besides padding benefit, is consistent with existing pattern in
that data structure.
Note, I agree on extern keyword change can be postponed (it was in the original
code), but here you introduce a new code...
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v3 2/2] serial: 8250: Support rs485 bus termination GPIO
From: Lukas Wunner @ 2020-05-18 14:45 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring
Cc: Matwey V. Kornilov, Giulio Benetti, Heiko Stuebner,
Andy Shevchenko, Christoph Muellner, Jan Kiszka, linux-serial,
devicetree
In-Reply-To: <cover.1589811297.git.lukas@wunner.de>
Commit e8759ad17d41 ("serial: uapi: Add support for bus termination")
introduced the ability to enable rs485 bus termination from user space.
So far the feature is only used by a single driver, 8250_exar.c, using a
hardcoded GPIO pin specific to Siemens IOT2040 products.
Provide for a more generic solution by allowing specification of an
rs485 bus termination GPIO pin in the device tree: Amend the serial
core to retrieve the GPIO from the device tree (or ACPI table) and amend
the default ->rs485_config() callback for 8250 drivers to change the
GPIO on request from user space.
Perhaps 8250_exar.c can be converted to the generic approach in a
follow-up patch.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
---
drivers/tty/serial/8250/8250_port.c | 3 +++
drivers/tty/serial/serial_core.c | 16 ++++++++++++++++
include/linux/serial_core.h | 2 ++
3 files changed, 21 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 4d83c85a7389..4f6ff54175ce 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -681,6 +681,9 @@ int serial8250_em485_config(struct uart_port *port, struct serial_rs485 *rs485)
memset(rs485->padding, 0, sizeof(rs485->padding));
port->rs485 = *rs485;
+ gpiod_set_value(port->rs485_term_gpio,
+ rs485->flags & SER_RS485_TERMINATE_BUS);
+
/*
* Both serial8250_em485_init() and serial8250_em485_destroy()
* are idempotent.
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 43b6682877d5..57840cf90388 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3317,6 +3317,7 @@ int uart_get_rs485_mode(struct uart_port *port)
* to get to a defined state with the following properties:
*/
rs485conf->flags &= ~(SER_RS485_RX_DURING_TX | SER_RS485_ENABLED |
+ SER_RS485_TERMINATE_BUS |
SER_RS485_RTS_AFTER_SEND);
rs485conf->flags |= SER_RS485_RTS_ON_SEND;
@@ -3331,6 +3332,21 @@ int uart_get_rs485_mode(struct uart_port *port)
rs485conf->flags |= SER_RS485_RTS_AFTER_SEND;
}
+ /*
+ * Disabling termination by default is the safe choice: Else if many
+ * bus participants enable it, no communication is possible at all.
+ * Works fine for short cables and users may enable for longer cables.
+ */
+ port->rs485_term_gpio = devm_gpiod_get_optional(dev, "rs485-term",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(port->rs485_term_gpio)) {
+ ret = PTR_ERR(port->rs485_term_gpio);
+ port->rs485_term_gpio = NULL;
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Cannot get rs485-term-gpios\n");
+ return ret;
+ }
+
return 0;
}
EXPORT_SYMBOL_GPL(uart_get_rs485_mode);
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index b649a2b894e7..9fd550e7946a 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -10,6 +10,7 @@
#include <linux/bitops.h>
#include <linux/compiler.h>
#include <linux/console.h>
+#include <linux/gpio/consumer.h>
#include <linux/interrupt.h>
#include <linux/circ_buf.h>
#include <linux/spinlock.h>
@@ -251,6 +252,7 @@ struct uart_port {
struct attribute_group *attr_group; /* port specific attributes */
const struct attribute_group **tty_groups; /* all attributes (serial core use only) */
struct serial_rs485 rs485;
+ struct gpio_desc *rs485_term_gpio; /* enable RS485 bus termination */
struct serial_iso7816 iso7816;
void *private_data; /* generic platform data pointer */
};
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v3 7/7] clocksource: mips-gic-timer: Set limitations on clocksource/sched-clocks usage
From: Serge Semin @ 2020-05-18 14:45 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Serge Semin, Thomas Bogendoerfer, Thomas Gleixner, Alexey Malahov,
Paul Burton, Ralf Baechle, Alessandro Zummo, Alexandre Belloni,
Arnd Bergmann, Rob Herring, linux-mips, linux-rtc, devicetree,
Vincenzo Frascino, linux-kernel
In-Reply-To: <20200518144045.v56fajrhbnnrzbpf@mobilestation>
On Mon, May 18, 2020 at 05:40:46PM +0300, Serge Semin wrote:
> Daniel,
>
> On Mon, May 18, 2020 at 03:59:16PM +0200, Daniel Lezcano wrote:
> > On 16/05/2020 14:16, Serge Semin wrote:
> > > Hello Daniel,
> > >
> > > Thanks for your comment. My response is below.
> > >
> > > On Fri, May 15, 2020 at 07:10:04PM +0200, Daniel Lezcano wrote:
> > >> On Thu, May 07, 2020 at 12:41:07AM +0300, Serge Semin wrote:
> > >>> Currently neither clocksource nor scheduler clock kernel framework
> > >>> support the clocks with variable frequency. Needless to say how many
> > >>> problems may cause the sudden base clocks frequency change. In a
> > >>> simplest case the system time will either slow down or speed up.
> > >>> Since on CM2.5 and earlier MIPS GIC timer is synchronously clocked
> > >>> with CPU we must set some limitations on using it for these frameworks
> > >>> if CPU frequency may change. First of all it's not safe to have the
> > >>> MIPS GIC used for scheduler timings. So we shouldn't proceed with
> > >>> the clocks registration in the sched-subsystem. Secondly we must
> > >>> significantly decrease the MIPS GIC clocksource rating. This will let
> > >>> the system to use it only as a last resort.
> > >>>
> > >>> Note CM3.x-based systems may also experience the problems with MIPS GIC
> > >>> if the CPU-frequency change is activated for the whole CPU cluster
> > >>> instead of using the individual CPC core clocks divider.
> > >>
> > >> May be there is no alternative but the code looks a bit hacksih. Isn't possible
> > >> to do something with the sched_mark_unstable?
> > >>
> > >> Or just not use the timer at all ?
> > >
> > > Not using the timer might be better, but not that good alternative either
> > > especially in our case due to very slow external timer. Me and Thomas
> > > Bogendoerfer discussed the similar commit I've provided to the csrc-r4k driver
> > > available on MIPS:
> > > https://lkml.org/lkml/2020/5/11/576
> > >
> > > To cut it short, you are right. The solution with using clocksource_mark_unstable()
> > > is better alternative spied up in x86 tsc implementation. I'll use a similar
> > > approach here and submit the updated patch in v3.
> > >
> > > Could you please proceed with the rest of the series review? I'd like to send
> > > the next version with as many comments taken into account as possible. The
> > > patchset has been submitted a while ago, but except Rob noone have had any
> > > comments.(
> >
> > For me other patches are ok.
> >
> > I can apply patches 1, 2, 4, 5, 6
> >
> > Will remain patches 3 et 7
>
> That's be great! Thanks. Is patch 3 supposed to be merged in by Rob or by you?
> I don't see one being in the Rob's repo. He might be waiting for you
> acknowledgment or something.
Ah, it's about the MIPS GIC IRQchip bindings conversion. No questions tabout
patch 3then. I'll ask Thomas about that patch status.
-Sergey
>
> I'll send the updated patch 3 shortly today.
>
> -Sergey
>
> >
> >
> > --
> > <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> >
> > Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> > <http://twitter.com/#!/linaroorg> Twitter |
> > <http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* [PATCH v3 0/2] rs485 bus termination GPIO
From: Lukas Wunner @ 2020-05-18 14:45 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring
Cc: Matwey V. Kornilov, Giulio Benetti, Heiko Stuebner,
Andy Shevchenko, Christoph Muellner, Jan Kiszka, linux-serial,
devicetree
Define a device tree binding for an rs485 bus termination GPIO
(patch [1/2]), amend the serial core to retrieve the GPIO from
the device tree and amend the default ->rs485_config() callback
for 8250 drivers to change the GPIO on request from user space
(patch [2/2]).
Changes v2 -> v3:
* Patch [2/2]:
* Drop unnecessary NULL pointer check preceding gpiod_set_value(). (Heiko)
* Amend commit message to suggest moving 8250_exar.c to new binding. (Andy)
Add Jan to Cc and Andy's R-b tag.
Changes v1 -> v2:
* Patch [2/2]:
* Drop unnecessary devm_gpiod_put(). (Andy)
* Use GPIOD_OUT_LOW macro for brevity. (Andy)
* Document the rationale for disabling termination by default.
* Drop nonsensical read of GPIO after setting its direction to out.
Link to v2:
https://lore.kernel.org/linux-serial/cover.1589285873.git.lukas@wunner.de
Lukas Wunner (2):
dt-bindings: serial: Add binding for rs485 bus termination GPIO
serial: 8250: Support rs485 bus termination GPIO
.../devicetree/bindings/serial/rs485.yaml | 4 ++++
drivers/tty/serial/8250/8250_port.c | 3 +++
drivers/tty/serial/serial_core.c | 16 ++++++++++++++++
include/linux/serial_core.h | 2 ++
4 files changed, 25 insertions(+)
--
2.26.2
^ permalink raw reply
* Re: [PATCH] dt-bindings: clock: Convert i.MX7D clock to json-schema
From: Fabio Estevam @ 2020-05-18 14:45 UTC (permalink / raw)
To: Anson Huang
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Shawn Guo,
Sascha Hauer, Sascha Hauer, Frank Li, linux-clk,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-kernel, NXP Linux Team
In-Reply-To: <1589810819-19851-1-git-send-email-Anson.Huang@nxp.com>
On Mon, May 18, 2020 at 11:16 AM Anson Huang <Anson.Huang@nxp.com> wrote:
> +maintainers:
> + - Frank Li <Frank.Li@freescale.com>
The freescale.com e-mail domain is no longer valid for quite some time :-)
^ permalink raw reply
* Re: [PATCH v3 7/7] clocksource: mips-gic-timer: Set limitations on clocksource/sched-clocks usage
From: Serge Semin @ 2020-05-18 14:40 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Serge Semin, Thomas Bogendoerfer, Thomas Gleixner, Alexey Malahov,
Paul Burton, Ralf Baechle, Alessandro Zummo, Alexandre Belloni,
Arnd Bergmann, Rob Herring, linux-mips, linux-rtc, devicetree,
Vincenzo Frascino, linux-kernel
In-Reply-To: <4c723219-62f8-be6a-47ea-a586859d832d@linaro.org>
Daniel,
On Mon, May 18, 2020 at 03:59:16PM +0200, Daniel Lezcano wrote:
> On 16/05/2020 14:16, Serge Semin wrote:
> > Hello Daniel,
> >
> > Thanks for your comment. My response is below.
> >
> > On Fri, May 15, 2020 at 07:10:04PM +0200, Daniel Lezcano wrote:
> >> On Thu, May 07, 2020 at 12:41:07AM +0300, Serge Semin wrote:
> >>> Currently neither clocksource nor scheduler clock kernel framework
> >>> support the clocks with variable frequency. Needless to say how many
> >>> problems may cause the sudden base clocks frequency change. In a
> >>> simplest case the system time will either slow down or speed up.
> >>> Since on CM2.5 and earlier MIPS GIC timer is synchronously clocked
> >>> with CPU we must set some limitations on using it for these frameworks
> >>> if CPU frequency may change. First of all it's not safe to have the
> >>> MIPS GIC used for scheduler timings. So we shouldn't proceed with
> >>> the clocks registration in the sched-subsystem. Secondly we must
> >>> significantly decrease the MIPS GIC clocksource rating. This will let
> >>> the system to use it only as a last resort.
> >>>
> >>> Note CM3.x-based systems may also experience the problems with MIPS GIC
> >>> if the CPU-frequency change is activated for the whole CPU cluster
> >>> instead of using the individual CPC core clocks divider.
> >>
> >> May be there is no alternative but the code looks a bit hacksih. Isn't possible
> >> to do something with the sched_mark_unstable?
> >>
> >> Or just not use the timer at all ?
> >
> > Not using the timer might be better, but not that good alternative either
> > especially in our case due to very slow external timer. Me and Thomas
> > Bogendoerfer discussed the similar commit I've provided to the csrc-r4k driver
> > available on MIPS:
> > https://lkml.org/lkml/2020/5/11/576
> >
> > To cut it short, you are right. The solution with using clocksource_mark_unstable()
> > is better alternative spied up in x86 tsc implementation. I'll use a similar
> > approach here and submit the updated patch in v3.
> >
> > Could you please proceed with the rest of the series review? I'd like to send
> > the next version with as many comments taken into account as possible. The
> > patchset has been submitted a while ago, but except Rob noone have had any
> > comments.(
>
> For me other patches are ok.
>
> I can apply patches 1, 2, 4, 5, 6
>
> Will remain patches 3 et 7
That's be great! Thanks. Is patch 3 supposed to be merged in by Rob or by you?
I don't see one being in the Rob's repo. He might be waiting for you
acknowledgment or something.
I'll send the updated patch 3 shortly today.
-Sergey
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH v3 10/15] net: ethernet: mtk-eth-mac: new driver
From: Arnd Bergmann @ 2020-05-18 14:38 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Jonathan Corbet, Rob Herring, David S . Miller, Matthias Brugger,
John Crispin, Sean Wang, Mark Lee, Jakub Kicinski, Fabien Parent,
Heiner Kallweit, Edwin Peer, DTML, linux-kernel@vger.kernel.org,
Networking, Linux ARM, moderated list:ARM/Mediatek SoC...,
Stephane Le Provost, Pedro Tsai, Andrew Perepech,
Bartosz Golaszewski
In-Reply-To: <CAMRc=MeVyNzTWw_hk=J9kX1NE9reCE_O4P3wrNpMMc9z4xA_DA@mail.gmail.com>
On Mon, May 18, 2020 at 4:07 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> pt., 15 maj 2020 o 15:32 Arnd Bergmann <arnd@arndb.de> napisał(a):
> > I would get rid of the 'count' here, as it duplicates the information
> > that is already known from the difference between head and tail, and you
> > can't update it atomically without holding a lock around the access to
> > the ring. The way I'd do this is to have the head and tail pointers
> > in separate cache lines, and then use READ_ONCE/WRITE_ONCE
> > and smp barriers to access them, with each one updated on one
> > thread but read by the other.
> >
>
> Your previous solution seems much more reliable though. For instance
> in the above: when we're doing the TX cleanup (we got the TX ready
> irq, we're iterating over descriptors until we know there are no more
> packets scheduled (count == 0) or we encounter one that's still owned
> by DMA), a parallel TX path can schedule new packets to be sent and I
> don't see how we can atomically check the count (understood as a
> difference between tail and head) and run a new iteration (where we'd
> modify the head or tail) without risking the other path getting in the
> way. We'd have to always check the descriptor.
It should be enough to read both pointers once at the start of each
side, then do whatever work you want to do (cleaning, sending,
receiving, refilling) and finally updating the one pointer that changed.
If both sides do that, you minimize the cache line bouncing and
always do a useful amount of work that guarantees forward progress
and does not interfere with the other side.
> I experimented a bit with this and couldn't come up with anything that
> would pass any stress test.
>
> On the other hand: spin_lock_bh() works fine and I like your approach
> from the previous e-mail - except for the work for updating stats as
> we could potentially lose some stats when we're updating in process
> context with RX/TX paths running in parallel in napi context but that
> would be rare enough to overlook it.
>
> I hope v4 will be good enough even with spinlocks. :)
Yes, it should be fine. Avoiding all the locks is mainly an optimization
for the number of CPU cycles spent per packet, the other points
are more important to get right, in particular the flow control.
Arnd
^ permalink raw reply
* Re: [V6, 1/2] media: dt-bindings: media: i2c: Document DW9768 bindings
From: Rob Herring @ 2020-05-18 14:31 UTC (permalink / raw)
To: Tomasz Figa
Cc: Dongchun Zhu, Linus Walleij, Bartosz Golaszewski,
Mauro Carvalho Chehab, Andy Shevchenko, Mark Rutland,
Sakari Ailus, Nicolas Boichat, Matthias Brugger, Cao Bing Bu,
srv_heupstream, moderated list:ARM/Mediatek SoC support,
list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>,,
Sj Huang, Linux Media Mailing List, linux-devicetree, Louis Kuo,
Shengnan Wang (王圣男)
In-Reply-To: <CAAFQd5Byvc8Fb0f3_81xSKsuyvsQJm-8g8y1Kx2aUcC=PwpS7w@mail.gmail.com>
On Mon, May 18, 2020 at 04:12:28PM +0200, Tomasz Figa wrote:
> Hi Dongchun,
>
> On Mon, May 18, 2020 at 3:29 PM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> >
> > Add DeviceTree binding documentation for Dongwoon Anatech DW9768 voice
> > coil actuator.
>
> Thanks for the patch. Please see my comments below.
>
> >
> > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
>
> This version includes significant changes, so the reviewed-by tag
> shouldn't have been carried out.
>
> > ---
> > .../bindings/media/i2c/dongwoon,dw9768.yaml | 105 +++++++++++++++++++++
> > MAINTAINERS | 7 ++
> > 2 files changed, 112 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
> > new file mode 100644
> > index 0000000..b909e83
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.yaml
> > @@ -0,0 +1,105 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright (c) 2020 MediaTek Inc.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/dongwoon,dw9768.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Dongwoon Anatech DW9768 Voice Coil Motor (VCM) Lens Device Tree Bindings
> > +
> > +maintainers:
> > + - Dongchun Zhu <dongchun.zhu@mediatek.com>
> > +
> > +description: |-
> > + The Dongwoon DW9768 is a single 10-bit digital-to-analog (DAC) converter
> > + with 100 mA output current sink capability. VCM current is controlled with
> > + a linear mode driver. The DAC is controlled via a 2-wire (I2C-compatible)
> > + serial interface that operates at clock rates up to 1MHz. This chip
> > + integrates Advanced Actuator Control (AAC) technology and is intended for
> > + driving voice coil lenses in camera modules.
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + # for DW9768 VCM
> > + - dongwoon,dw9768
> > + # for GT9769 VCM
> > + - giantec,gt9769
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + vin-supply:
> > + description:
> > + Definition of the regulator used as I2C I/O interface power supply.
> > +
> > + vdd-supply:
> > + description:
> > + Definition of the regulator used as VCM chip power supply.
> > +
> > + dongwoon,aac-mode:
> > + description:
> > + Indication of AAC mode select.
> > + allOf:
> > + - $ref: "/schemas/types.yaml#/definitions/uint32"
> > + - enum:
> > + - 0 # Direct (default)
Default can be expressed as 'default: 0'.
> > + - 1 # AAC2 (operation time# 0.48 x Tvib)
> > + - 2 # AAC3 (operation time# 0.70 x Tvib)
> > + - 3 # AAC4 (operation time# 0.75 x Tvib)
> > + - 4 # Reserved
> > + - 5 # AAC8 (operation time# 1.13 x Tvib)
> > + - 6 # Reserved
> > + - 7 # Reserved
>
> I'll ultimately leave it to DT maintainers, but is there any reason to
> define the reserved values?
No.
>
> > +
> > + dongwoon,aac-timing:
> > + description:
> > + Indication of AAC Timing count, unit of 0.1 milliseconds.
Why not just use standard units (-us)?
> > + Valid values vary from 0 to 63 (default 32).
Looks like constraints.
> > + allOf:
> > + - $ref: "/schemas/types.yaml#/definitions/uint32"
> > +
> > + dongwoon,clock-dividing-rate:
> > + description:
> > + Indication of VCM internal clock dividing rate select, as one multiple
> > + factor to calculate VCM ring periodic time Tvib.
> > + allOf:
> > + - $ref: "/schemas/types.yaml#/definitions/uint32"
> > + - enum:
> > + - 0 # Dividing Rate - 2
> > + - 1 # Dividing Rate - 1 (default)
> > + - 2 # Dividing Rate - 1/2
> > + - 3 # Dividing Rate - 1/4
> > + - 4 # Dividing Rate - 8
> > + - 5 # Dividing Rate - 4
> > + - 6 # Dividing Rate - Reserved
> > + - 7 # Dividing Rate - Reserved
>
> Ditto.
>
> Best regards,
> Tomasz
^ permalink raw reply
* Re: [PATCH v5 1/2] clk: qcom: Add DT bindings for MSM8939 GCC
From: Rob Herring @ 2020-05-18 14:25 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: robh+dt, linux-arm-msm, shawn.guo, konradybcio, linux-kernel,
p.zabel, sboyd, devicetree, agross, bjorn.andersson,
vincent.knecht, mturquette, linux-clk
In-Reply-To: <20200517131348.688405-2-bryan.odonoghue@linaro.org>
On Sun, 17 May 2020 14:13:47 +0100, Bryan O'Donoghue wrote:
> Add compatible strings and the include files for the MSM8939 GCC.
>
> Cc: Andy Gross <agross@kernel.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: linux-clk@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Tested-by: Vincent Knecht <vincent.knecht@mailoo.org>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> .../devicetree/bindings/clock/qcom,gcc.yaml | 3 +
> include/dt-bindings/clock/qcom,gcc-msm8939.h | 206 ++++++++++++++++++
> include/dt-bindings/reset/qcom,gcc-msm8939.h | 110 ++++++++++
> 3 files changed, 319 insertions(+)
> create mode 100644 include/dt-bindings/clock/qcom,gcc-msm8939.h
> create mode 100644 include/dt-bindings/reset/qcom,gcc-msm8939.h
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH] arm: dts: am33xx-bone-common: add gpio-line-names
From: Drew Fustini @ 2020-05-18 14:24 UTC (permalink / raw)
To: Felipe Balbi
Cc: Linus Walleij, Grygorii Strashko, Benoît Cousson,
Tony Lindgren, Rob Herring, Linux-OMAP,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org, Jason Kridner, Robert Nelson
In-Reply-To: <87zha5whf4.fsf@kernel.org>
On Mon, May 18, 2020 at 03:34:23PM +0300, Felipe Balbi wrote:
> Linus Walleij <linus.walleij@linaro.org> writes:
>
> > On Mon, May 18, 2020 at 10:18 AM Felipe Balbi <balbi@kernel.org> wrote:
> >> Linus Walleij <linus.walleij@linaro.org> writes:
> >> >> gpiochip0 - 32 lines:
> >> >> line 0: "ethernet" unused input active-high
> >> >> line 1: "ethernet" unused input active-high
> >> >
> >> > Why are the ethernet lines not tagged with respective signal name
> >> > when right below the SPI lines are explicitly tagged with
> >> > sclk, cs0 etc?
> >> >
> >> > Ethernet is usually RGMII and has signal names like
> >> > tx_clk, tx_d0, tx_en etc.
> >> >
> >> > Also some lines seem to be tagged with the pin number
> >> > like P9_22, P2_21 below, it seems a bit inconsistent
> >> > to have much information on some pins and very sketchy
> >> > information on some.
> >>
> >> the pin names match the beagle bone documentation and would help users
> >> figure out which pins on the expansion headers match to a gpio signal.
Thank you for pointing this out. That is my goal with the line names.
> >
> > OK if it is how it looks in the documentation I agree that is what
> > users need, maybe the documentation is confusing but there is not
> > much to do about that.
>
> the board has two expansion headers, P1 and P2:
>
> https://github.com/beagleboard/pocketbeagle/wiki/System-Reference-Manual#531_Expansion_Headers
>
> Pins are always the pin number on the header, hence P2_21 and P1_10 and
> so on.
Just to clarify, the patch was for the BeagleBone {White,Green,Black}
which have P8 and P9 headers.
The PocketBeagle has lower pin count headers labeled P1 and P2. I do
plan to submit a patch for am335x-pocketbeagle.dts with the respective
line names, but I wanted to first integrate feedback regarding P8/P9 on
the bone.
thanks,
drew
^ permalink raw reply
* Re: [PATCH 5/6] drm: msm: a6xx: use dev_pm_opp_set_bw to set DDR bandwidth
From: Jordan Crouse @ 2020-05-18 14:23 UTC (permalink / raw)
To: Sharat Masetty
Cc: freedreno, devicetree, dri-devel, linux-arm-msm, linux-kernel,
georgi.djakov, mka
In-Reply-To: <1589453659-27581-6-git-send-email-smasetty@codeaurora.org>
On Thu, May 14, 2020 at 04:24:18PM +0530, Sharat Masetty wrote:
> This patches replaces the previously used static DDR vote and uses
> dev_pm_opp_set_bw() to scale GPU->DDR bandwidth along with scaling
> GPU frequency.
>
> Signed-off-by: Sharat Masetty <smasetty@codeaurora.org>
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 2d8124b..79433d3 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> @@ -141,11 +141,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp)
>
> gmu->freq = gmu->gpu_freqs[perf_index];
>
> - /*
> - * Eventually we will want to scale the path vote with the frequency but
> - * for now leave it at max so that the performance is nominal.
> - */
> - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
> + dev_pm_opp_set_bw(&gpu->pdev->dev, opp);
> }
This adds an implicit requirement that all targets need bandwidth settings
defined in the OPP or they won't get a bus vote at all. I would prefer that
there be an default escape valve but if not you'll need to add
bandwidth values for the sdm845 OPP that target doesn't regress.
Jordan
> unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu)
> --
> 2.7.4
>
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v7 6/7] dt-bindings: input: Add docs for ADC driven joystick.
From: Rob Herring @ 2020-05-18 14:22 UTC (permalink / raw)
To: Artur Rojek
Cc: Andy Shevchenko, Rob Herring, Heiko Stuebner, linux-input,
linux-iio, Dmitry Torokhov, linux-kernel, Mark Rutland,
Paul Cercueil, Ezequiel Garcia, devicetree, Jonathan Cameron
In-Reply-To: <20200517194904.34758-6-contact@artur-rojek.eu>
On Sun, 17 May 2020 21:49:03 +0200, Artur Rojek wrote:
> 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
>
My bot found errors running 'make dt_binding_check' on your patch:
Error: Documentation/devicetree/bindings/input/adc-joystick.example.dts:24.31-32 syntax error
FATAL ERROR: Unable to parse input tree
scripts/Makefile.lib:312: recipe for target 'Documentation/devicetree/bindings/input/adc-joystick.example.dt.yaml' failed
make[1]: *** [Documentation/devicetree/bindings/input/adc-joystick.example.dt.yaml] Error 1
Makefile:1300: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1292230
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
^ permalink raw reply
* Re: [PATCH V2] arm64: tegra: Fix flag for 64-bit resources in 'ranges' property
From: Vidya Sagar @ 2020-05-18 14:22 UTC (permalink / raw)
To: robh+dt, thierry.reding, jonathanh
Cc: Lorenzo Pieralisi, amurray, bhelgaas, linux-kernel, kthota,
mmaddireddy, sagar.tv, devicetree, linux-tegra
In-Reply-To: <20200518135006.GB31554@e121166-lin.cambridge.arm.com>
Thanks Lorenzo.
I've moved linux-pci to BCC and included
devicetree and linux-tegra mailing lists to CC.
Rob, Could you please review this patch?
Thanks,
Vidya Sagar
On 18-May-20 7:20 PM, Lorenzo Pieralisi wrote:
> External email: Use caution opening links or attachments
>
>
> On Thu, May 14, 2020 at 07:24:37PM +0530, Vidya Sagar wrote:
>> Fix flag in PCIe controllers device-tree nodes 'ranges' property to correctly
>> represent 64-bit resources.
>>
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>> V2:
>> * Extended the change to cover other controllers as well
>>
>> arch/arm64/boot/dts/nvidia/tegra194.dtsi | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> We don't apply DTS patches - so no need to CC linux-pci from now
> onwards on these. Marked as not-applicable.
>
> Lorenzo
>
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> index e1ae01c2d039..4bc187a4eacd 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
>> @@ -1405,7 +1405,7 @@
>>
>> bus-range = <0x0 0xff>;
>> ranges = <0x81000000 0x0 0x30100000 0x0 0x30100000 0x0 0x00100000 /* downstream I/O (1MB) */
>> - 0xc2000000 0x12 0x00000000 0x12 0x00000000 0x0 0x30000000 /* prefetchable memory (768MB) */
>> + 0xc3000000 0x12 0x00000000 0x12 0x00000000 0x0 0x30000000 /* prefetchable memory (768MB) */
>> 0x82000000 0x0 0x40000000 0x12 0x30000000 0x0 0x10000000>; /* non-prefetchable memory (256MB) */
>> };
>>
>> @@ -1450,7 +1450,7 @@
>>
>> bus-range = <0x0 0xff>;
>> ranges = <0x81000000 0x0 0x32100000 0x0 0x32100000 0x0 0x00100000 /* downstream I/O (1MB) */
>> - 0xc2000000 0x12 0x40000000 0x12 0x40000000 0x0 0x30000000 /* prefetchable memory (768MB) */
>> + 0xc3000000 0x12 0x40000000 0x12 0x40000000 0x0 0x30000000 /* prefetchable memory (768MB) */
>> 0x82000000 0x0 0x40000000 0x12 0x70000000 0x0 0x10000000>; /* non-prefetchable memory (256MB) */
>> };
>>
>> @@ -1495,7 +1495,7 @@
>>
>> bus-range = <0x0 0xff>;
>> ranges = <0x81000000 0x0 0x34100000 0x0 0x34100000 0x0 0x00100000 /* downstream I/O (1MB) */
>> - 0xc2000000 0x12 0x80000000 0x12 0x80000000 0x0 0x30000000 /* prefetchable memory (768MB) */
>> + 0xc3000000 0x12 0x80000000 0x12 0x80000000 0x0 0x30000000 /* prefetchable memory (768MB) */
>> 0x82000000 0x0 0x40000000 0x12 0xb0000000 0x0 0x10000000>; /* non-prefetchable memory (256MB) */
>> };
>>
>> @@ -1540,7 +1540,7 @@
>>
>> bus-range = <0x0 0xff>;
>> ranges = <0x81000000 0x0 0x36100000 0x0 0x36100000 0x0 0x00100000 /* downstream I/O (1MB) */
>> - 0xc2000000 0x14 0x00000000 0x14 0x00000000 0x3 0x40000000 /* prefetchable memory (13GB) */
>> + 0xc3000000 0x14 0x00000000 0x14 0x00000000 0x3 0x40000000 /* prefetchable memory (13GB) */
>> 0x82000000 0x0 0x40000000 0x17 0x40000000 0x0 0xc0000000>; /* non-prefetchable memory (3GB) */
>> };
>>
>> @@ -1585,7 +1585,7 @@
>>
>> bus-range = <0x0 0xff>;
>> ranges = <0x81000000 0x0 0x38100000 0x0 0x38100000 0x0 0x00100000 /* downstream I/O (1MB) */
>> - 0xc2000000 0x18 0x00000000 0x18 0x00000000 0x3 0x40000000 /* prefetchable memory (13GB) */
>> + 0xc3000000 0x18 0x00000000 0x18 0x00000000 0x3 0x40000000 /* prefetchable memory (13GB) */
>> 0x82000000 0x0 0x40000000 0x1b 0x40000000 0x0 0xc0000000>; /* non-prefetchable memory (3GB) */
>> };
>>
>> @@ -1634,7 +1634,7 @@
>>
>> bus-range = <0x0 0xff>;
>> ranges = <0x81000000 0x0 0x3a100000 0x0 0x3a100000 0x0 0x00100000 /* downstream I/O (1MB) */
>> - 0xc2000000 0x1c 0x00000000 0x1c 0x00000000 0x3 0x40000000 /* prefetchable memory (13GB) */
>> + 0xc3000000 0x1c 0x00000000 0x1c 0x00000000 0x3 0x40000000 /* prefetchable memory (13GB) */
>> 0x82000000 0x0 0x40000000 0x1f 0x40000000 0x0 0xc0000000>; /* non-prefetchable memory (3GB) */
>> };
>>
>> --
>> 2.17.1
>>
^ permalink raw reply
* Re: [PATCH 3/3] dt-bindings: pwm: keembay: Add bindings for Intel Keem Bay PWM
From: Rob Herring @ 2020-05-18 14:21 UTC (permalink / raw)
To: vineetha.g.jaya.kumaran
Cc: thierry.reding, u.kleine-koenig, linux-pwm, devicetree,
wan.ahmad.zainie.wan.mohamad, andriy.shevchenko
In-Reply-To: <1589723560-5734-4-git-send-email-vineetha.g.jaya.kumaran@intel.com>
On Sun, May 17, 2020 at 09:52:40PM +0800, vineetha.g.jaya.kumaran@intel.com wrote:
> From: "Vineetha G. Jaya Kumaran" <vineetha.g.jaya.kumaran@intel.com>
>
> Add PWM Device Tree bindings documentation for the Intel Keem Bay SoC.
>
> Signed-off-by: Vineetha G. Jaya Kumaran <vineetha.g.jaya.kumaran@intel.com>
> ---
> .../devicetree/bindings/pwm/pwm-keembay.yaml | 39 ++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-keembay.yaml
Use compatible string for filename: intel,keembay-pwn.yaml
>
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-keembay.yaml b/Documentation/devicetree/bindings/pwm/pwm-keembay.yaml
> new file mode 100644
> index 0000000..00968d7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pwm-keembay.yaml
> @@ -0,0 +1,39 @@
> +# SPDX-License-Identifier: GPL-2.0
Dual license new bindings:
(GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2020 Intel Corporation
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pwm/pwm-keembay.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Intel Keem Bay PWM Device Tree Bindings
> +
> +maintainers:
> + - Vineetha G. Jaya Kumaran <vineetha.g.jaya.kumaran@intel.com>
> +
> +allOf:
> + - $ref: pwm.yaml#
> +
> +properties:
> + compatible:
> + enum:
> + - intel,keembay-pwm
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + description:
> + phandle to the reference clock.
How many clocks? (maxItems: 1?)
You can drop the description.
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> +
> +examples:
> + - |
> + pwm@203200a0 {
> + compatible = "intel,keembay-pwm";
> + reg = <0x0 0x203200a0 0x0 0xe8>;
> + clocks = <&scmi_clk KEEM_BAY_A53_GPIO>;
> + };
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH 02/12] iio: imu: inv_icm42600: add I2C driver for inv_icm42600 driver
From: Jean-Baptiste Maneyrol @ 2020-05-18 14:19 UTC (permalink / raw)
To: Jonathan Cameron
Cc: jic23@kernel.org, robh+dt@kernel.org, robh@kernel.org,
mchehab+huawei@kernel.org, davem@davemloft.net,
gregkh@linuxfoundation.org, linux-iio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20200508144406.00006b8c@Huawei.com>
Hi Jonathan,
I am using generic device_get_match_data because I was thinking it was now the way to go. But since only of is supported with the driver, I can switch to using of_device_get_match_data instead.
Tell me what do you think is better.
I could definitely use the new probe interface indeed, good idea.
Thanks,
JB
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Sent: Friday, May 8, 2020 15:44
To: Jean-Baptiste Maneyrol <JManeyrol@invensense.com>
Cc: jic23@kernel.org <jic23@kernel.org>; robh+dt@kernel.org <robh+dt@kernel.org>; robh@kernel.org <robh@kernel.org>; mchehab+huawei@kernel.org <mchehab+huawei@kernel.org>; davem@davemloft.net <davem@davemloft.net>; gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>;
linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; devicetree@vger.kernel.org <devicetree@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 02/12] iio: imu: inv_icm42600: add I2C driver for inv_icm42600 driver
CAUTION: This email originated from outside of the organization. Please make sure the sender is who they say they are and do not click links or open attachments unless you recognize the sender and know the content is safe.
On Thu, 7 May 2020 16:42:12 +0200
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
> Add I2C driver for InvenSense ICM-426xxx devices.
>
> Configure bus signal slew rates as indicated in the datasheet.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
Some incoherent rambling inline. + a few comments
Jonathan
> ---
> .../iio/imu/inv_icm42600/inv_icm42600_i2c.c | 117 ++++++++++++++++++
> 1 file changed, 117 insertions(+)
> create mode 100644 drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c
>
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c
> new file mode 100644
> index 000000000000..b61f993beacf
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_i2c.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2020 InvenSense, Inc.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/of_device.h>
Why? Looks like you need the table and the device property stuff neither
of which are in that file.
linux/mod_devicetable.h
linux/property.h
> +
> +#include "inv_icm42600.h"
> +
> +static int inv_icm42600_i2c_bus_setup(struct inv_icm42600_state *st)
> +{
> + unsigned int mask, val;
> + int ret;
> +
> + /* setup interface registers */
> + mask = INV_ICM42600_INTF_CONFIG6_MASK;
> + val = INV_ICM42600_INTF_CONFIG6_I3C_EN;
> + ret = regmap_update_bits(st->map, INV_ICM42600_REG_INTF_CONFIG6,
> + mask, val);
I'd put the values inline where they are simple like these rather than
using local variables.
> + if (ret)
> + return ret;
> +
> + mask = INV_ICM42600_INTF_CONFIG4_I3C_BUS_ONLY;
> + val = 0;
> + ret = regmap_update_bits(st->map, INV_ICM42600_REG_INTF_CONFIG4,
> + mask, val);
> + if (ret)
> + return ret;
> +
> + /* set slew rates for I2C and SPI */
> + mask = INV_ICM42600_DRIVE_CONFIG_I2C_MASK |
> + INV_ICM42600_DRIVE_CONFIG_SPI_MASK;
> + val = INV_ICM42600_DRIVE_CONFIG_I2C(INV_ICM42600_SLEW_RATE_12_36NS) |
> + INV_ICM42600_DRIVE_CONFIG_SPI(INV_ICM42600_SLEW_RATE_12_36NS);
> + ret = regmap_update_bits(st->map, INV_ICM42600_REG_DRIVE_CONFIG,
> + mask, val);
> + if (ret)
> + return ret;
> +
> + /* disable SPI bus */
> + mask = INV_ICM42600_INTF_CONFIG0_UI_SIFS_CFG_MASK;
> + val = INV_ICM42600_INTF_CONFIG0_UI_SIFS_CFG_SPI_DIS;
> + return regmap_update_bits(st->map, INV_ICM42600_REG_INTF_CONFIG0,
> + mask, val);
> +}
> +
> +static int inv_icm42600_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + const void *match;
> + enum inv_icm42600_chip chip;
> + struct regmap *regmap;
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
> + return -ENOTSUPP;
> +
> + match = device_get_match_data(&client->dev);
Hmm. Annoyingly if one were to call the of specific option
of i2c_of_match_device it would handle the old style i2c match just fine without
needing special handling. However, it would fail to handle PRP0001 ACPI.
Rather feels like there should be something similar for
device_get_match_data so we could use the probe_new version of i2c device
probing.
Oh well. I guess thats a separate question for another day ;)
Mind you can we actually probe this driver via the sysfs route?
If not why do we need to handle the non firmware case at all?
> + if (match)
> + chip = (enum inv_icm42600_chip)match;
> + else if (id)
> + chip = (enum inv_icm42600_chip)id->driver_data;
> + else
> + return -EINVAL;
> +
> + regmap = devm_regmap_init_i2c(client, &inv_icm42600_regmap_config);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + return inv_icm42600_core_probe(regmap, chip,
> + inv_icm42600_i2c_bus_setup);
> +}
> +
> +static const struct of_device_id inv_icm42600_of_matches[] = {
> + {
> + .compatible = "invensense,icm42600",
> + .data = (void *)INV_CHIP_ICM42600,
> + }, {
> + .compatible = "invensense,icm42602",
> + .data = (void *)INV_CHIP_ICM42602,
> + }, {
> + .compatible = "invensense,icm42605",
> + .data = (void *)INV_CHIP_ICM42605,
> + }, {
> + .compatible = "invensense,icm42622",
> + .data = (void *)INV_CHIP_ICM42622,
> + },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, inv_icm42600_of_matches);
> +
> +static const struct i2c_device_id inv_icm42600_ids[] = {
> + {"icm42600", INV_CHIP_ICM42600},
> + {"icm42602", INV_CHIP_ICM42602},
> + {"icm42605", INV_CHIP_ICM42605},
> + {"icm42622", INV_CHIP_ICM42622},
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, inv_icm42600_ids);
> +
> +static struct i2c_driver inv_icm42600_driver = {
> + .probe = inv_icm42600_probe,
> + .id_table = inv_icm42600_ids,
> + .driver = {
> + .of_match_table = inv_icm42600_of_matches,
> + .name = "inv-icm42600-i2c",
> + .pm = &inv_icm42600_pm_ops,
> + },
> +};
> +module_i2c_driver(inv_icm42600_driver);
> +
> +MODULE_AUTHOR("InvenSense, Inc.");
> +MODULE_DESCRIPTION("InvenSense ICM-426xx I2C driver");
> +MODULE_LICENSE("GPL");
^ permalink raw reply
* Re: [PATCH] arm: dts: am33xx-bone-common: add gpio-line-names
From: Drew Fustini @ 2020-05-18 14:18 UTC (permalink / raw)
To: Linus Walleij
Cc: Grygorii Strashko, Benoît Cousson, Tony Lindgren,
Rob Herring, Linux-OMAP,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org, Jason Kridner, Robert Nelson
In-Reply-To: <CACRpkdb+ZP6rfjGg6Ef9_wYvNf6qmSc7LZyYBVKA3XWCtxPfqQ@mail.gmail.com>
On Mon, May 18, 2020 at 09:11:07AM +0200, Linus Walleij wrote:
> On Fri, May 8, 2020 at 6:57 PM Drew Fustini <drew@beagleboard.org> wrote:
>
> > Add gpio-line-names properties to the gpio controller nodes.
> > BeagleBone boards have P8 and P9 headers [0] which expose many the
> > AM3358 SoC balls to stacking expansion boards called "capes", or to
> > other external connections like jumper wires to a breadboard.
> >
> > Many of the P8/P9 header pins can muxed to a gpio line. The
> > gpio-line-names describe which P8/P9 pin that line goes to and the
> > default mux for that P8/P9 pin. Some lines are not routed to the
> > P8/P9 headers, but instead are dedicated to some functionality such as
> > status LEDs. The line name will indicate this. Some line names are
> > left empty as the corresponding AM3358 balls are not connected.
> >
> > The goal is to make it easier for a user viewing the output of gpioinfo
> > to determine which P8/P9 pin is connected to a line. The output of
> > gpioinfo on a BeagleBone Black will now look like this:
> >
> > gpiochip0 - 32 lines:
> > line 0: "ethernet" unused input active-high
> > line 1: "ethernet" unused input active-high
>
> Why are the ethernet lines not tagged with respective signal name
> when right below the SPI lines are explicitly tagged with
> sclk, cs0 etc?
>
> Ethernet is usually RGMII and has signal names like
> tx_clk, tx_d0, tx_en etc.
Thank you for the feedback, Linus.
My desire is to communicate that the AM3358 balls corresponding to these
GPIO lines are being used for Ethernet and not exposed to the P8 and P9
expansion headers.
I am happy to switch these labels to the actual Ethernet signals such as
RGMII and MDIO signal names if you think that is better.
For example, AM3358 ZCZ ball M17 is both gpio0_0 and mdio_data [0]. On
BeagleBone, the ball is routed to the Ethernet PHY and used for MDIO [1]
Thus gpiochio 0 line 0 is not connected to the P8 or P9 expansion header.
Which of the following line name would be best?
1) "[MDIO_DATA]"
precise signal name, placed in brackets to denote is not possible to
use as GPIO on the P8 or P9 headers
2) "[ethernet]"
instead of the precise signal name, just indicate that it is used for
Ethernet and is not usable for GPIO on the P8 or P9 headers
3) ""
no label as this gpio line is not connected to the P8/P9 and is
hardwired in the PCB layout for Ethernet (MDIO).
> Also some lines seem to be tagged with the pin number
> like P9_22, P2_21 below, it seems a bit inconsistent
> to have much information on some pins and very sketchy
> information on some.
The goal for these line names is make it easier for a BeagleBone user to
identify which GPIO lines are connected to the P8 and P9 expansion
headers. Our users are most likely to refer to cape-headers.png [2] as
it is part of the bone101 out-of-the-box tutorial [3].
Some GPIO lines are free to be used for GPIO in the default
configuration. For example, gpiochip 1 line 12 is connected to P8_12
and it is not used by another peripheral by default. I used the label:
"P8_12 gpio"
However, gpiochip 1 line 0 is connected to P8_25 but is also used by the
on-board eMMC. The eMMc is enabled by default so this line can not be
used for GPIO unless the user modifies the pinmux in the device tree.
Thus, I used this label: "P8_25 emmc"
Maybe a better label would be "P8_25 [EMMC]"?
>
> > line 18: "usb" unused input active-high
> > line 19: "hdmi" unused input active-high
>
> Similar comments for these.
These are similar to the Ethernet MDIO example above. The balls
corresponding to these GPIO lines are not connected to the P8 or P9
headers and are hardwired on the PCB for other peripherals like USB
and HDMI.
For example, gpiochip 0 line 18 is USB0_DRVVBUS so I simplified it to
"usb" to indicate it can not be used for GPIO. Maybe "[USB]" is better?
gpiochip 0 line 19 is AM3358 ZCZ ball A15 and the BeagleBone Black
schematic shows that this is connected to the CEC clock for the HDMI
framer [4]. I though "hdmi" was a nice way to summarize that this is
used for HDMI and can not be changed, though maybe "[HDMI]" is better
or no label at all.
In conclusion, the motivation of these line names is to be a quick
reference for a user to find GPIO lines on the P8 and P9 headers.
Thanks,
Drew
[0] http://www.ti.com/lit/ds/symlink/am3358.pdf
[1] https://github.com/beagleboard/beaglebone-black/wiki/System-Reference-Manual#ethernet-processor-interface
[2] http://beagleboard.org/static/images/cape-headers.png
[3] https://beagleboard.org/Support/bone101
[4] https://github.com/beagleboard/beaglebone-black/blob/master/BBB_SCH.pdf
^ permalink raw reply
* Re: [PATCH 3/3] dt-bindings: pwm: keembay: Add bindings for Intel Keem Bay PWM
From: Rob Herring @ 2020-05-18 14:18 UTC (permalink / raw)
To: vineetha.g.jaya.kumaran
Cc: devicetree, u.kleine-koenig, wan.ahmad.zainie.wan.mohamad,
robh+dt, linux-pwm, andriy.shevchenko, thierry.reding
In-Reply-To: <1589723560-5734-4-git-send-email-vineetha.g.jaya.kumaran@intel.com>
On Sun, 17 May 2020 21:52:40 +0800, wrote:
> From: "Vineetha G. Jaya Kumaran" <vineetha.g.jaya.kumaran@intel.com>
>
> Add PWM Device Tree bindings documentation for the Intel Keem Bay SoC.
>
> Signed-off-by: Vineetha G. Jaya Kumaran <vineetha.g.jaya.kumaran@intel.com>
> ---
> .../devicetree/bindings/pwm/pwm-keembay.yaml | 39 ++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-keembay.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
Error: Documentation/devicetree/bindings/pwm/pwm-keembay.example.dts:22.31-32 syntax error
FATAL ERROR: Unable to parse input tree
scripts/Makefile.lib:312: recipe for target 'Documentation/devicetree/bindings/pwm/pwm-keembay.example.dt.yaml' failed
make[1]: *** [Documentation/devicetree/bindings/pwm/pwm-keembay.example.dt.yaml] Error 1
Makefile:1300: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1292157
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
^ permalink raw reply
* Re: [PATCH v6 1/2] dt-bindings: usb: qcom,dwc3: Convert USB DWC3 bindings
From: Rob Herring @ 2020-05-18 14:17 UTC (permalink / raw)
To: Sandeep Maheswaram
Cc: Felipe Balbi, linux-arm-msm, Rob Herring, linux-usb, Andy Gross,
Stephen Boyd, Mark Rutland, Bjorn Andersson, linux-kernel,
Matthias Kaehlcke, Doug Anderson, devicetree, Greg Kroah-Hartman,
Manu Gautam
In-Reply-To: <1589534960-6973-2-git-send-email-sanm@codeaurora.org>
On Fri, 15 May 2020 14:59:19 +0530, Sandeep Maheswaram wrote:
> Convert USB DWC3 bindings to DT schema format using json-schema.
>
> Signed-off-by: Sandeep Maheswaram <sanm@codeaurora.org>
> ---
> .../devicetree/bindings/usb/qcom,dwc3.txt | 104 -------------
> .../devicetree/bindings/usb/qcom,dwc3.yaml | 162 +++++++++++++++++++++
> 2 files changed, 162 insertions(+), 104 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.txt
> create mode 100644 Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
Documentation/devicetree/bindings/usb/qcom,dwc3.example.dts:28.13-20: Warning (ranges_format): /example-0/usb@a6f8800:ranges: empty "ranges" property but its #address-cells (2) differs from /example-0 (1)
Documentation/devicetree/bindings/usb/qcom,dwc3.example.dts:28.13-20: Warning (ranges_format): /example-0/usb@a6f8800:ranges: empty "ranges" property but its #size-cells (2) differs from /example-0 (1)
See https://patchwork.ozlabs.org/patch/1290971
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
^ permalink raw reply
* [PATCH] dt-bindings: clock: Convert i.MX7D clock to json-schema
From: Anson Huang @ 2020-05-18 14:06 UTC (permalink / raw)
To: mturquette, sboyd, robh+dt, shawnguo, s.hauer, kernel, festevam,
Frank.Li, linux-clk, devicetree, linux-arm-kernel, linux-kernel
Cc: Linux-imx
Convert the i.MX7D clock binding to DT schema format using json-schema.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
.../devicetree/bindings/clock/imx7d-clock.txt | 13 -----
.../devicetree/bindings/clock/imx7d-clock.yaml | 64 ++++++++++++++++++++++
2 files changed, 64 insertions(+), 13 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/clock/imx7d-clock.txt
create mode 100644 Documentation/devicetree/bindings/clock/imx7d-clock.yaml
diff --git a/Documentation/devicetree/bindings/clock/imx7d-clock.txt b/Documentation/devicetree/bindings/clock/imx7d-clock.txt
deleted file mode 100644
index 9d3026d..0000000
--- a/Documentation/devicetree/bindings/clock/imx7d-clock.txt
+++ /dev/null
@@ -1,13 +0,0 @@
-* Clock bindings for Freescale i.MX7 Dual
-
-Required properties:
-- compatible: Should be "fsl,imx7d-ccm"
-- reg: Address and length of the register set
-- #clock-cells: Should be <1>
-- clocks: list of clock specifiers, must contain an entry for each required
- entry in clock-names
-- clock-names: should include entries "ckil", "osc"
-
-The clock consumer should specify the desired clock by having the clock
-ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx7d-clock.h
-for the full list of i.MX7 Dual clock IDs.
diff --git a/Documentation/devicetree/bindings/clock/imx7d-clock.yaml b/Documentation/devicetree/bindings/clock/imx7d-clock.yaml
new file mode 100644
index 0000000..72eb13f
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/imx7d-clock.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/imx7d-clock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Clock bindings for Freescale i.MX7 Dual
+
+maintainers:
+ - Frank Li <Frank.Li@freescale.com>
+ - Anson Huang <Anson.Huang@nxp.com>
+
+description: |
+ The clock consumer should specify the desired clock by having the clock
+ ID in its "clocks" phandle cell. See include/dt-bindings/clock/imx7d-clock.h
+ for the full list of i.MX7 Dual clock IDs.
+
+properties:
+ compatible:
+ const: fsl,imx7d-ccm
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ items:
+ - description: CCM interrupt request 1
+ - description: CCM interrupt request 2
+ maxItems: 2
+
+ '#clock-cells':
+ const: 1
+
+ clocks:
+ items:
+ - description: 32k osc
+ - description: 24m osc
+
+ clock-names:
+ items:
+ - const: ckil
+ - const: osc
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - '#clock-cells'
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ clock-controller@30380000 {
+ compatible = "fsl,imx7d-ccm";
+ reg = <0x30380000 0x10000>;
+ interrupts = <GIC_SPI 85 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
+ #clock-cells = <1>;
+ clocks = <&ckil>, <&osc>;
+ clock-names = "ckil", "osc";
+ };
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2 1/2] dt-bindings: Add ch7322 media i2c device
From: Rob Herring @ 2020-05-18 14:15 UTC (permalink / raw)
To: Jeff Chase; +Cc: robh+dt, hverkuil-cisco, mchehab, devicetree, linux-media
In-Reply-To: <20200515025159.101081-2-jnchase@google.com>
On Thu, 14 May 2020 22:51:58 -0400, Jeff Chase wrote:
> The ch7322 is a Chrontel CEC controller.
>
> Signed-off-by: Jeff Chase <jnchase@google.com>
> ---
> .../bindings/media/i2c/chrontel,ch7322.yaml | 65 +++++++++++++++++++
> .../devicetree/bindings/vendor-prefixes.yaml | 2 +
> MAINTAINERS | 7 ++
> 3 files changed, 74 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/chrontel,ch7322.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
Documentation/devicetree/bindings/media/i2c/chrontel,ch7322.yaml: $id: relative path/filename doesn't match actual path or filename
expected: http://devicetree.org/schemas/media/i2c/chrontel,ch7322.yaml#
See https://patchwork.ozlabs.org/patch/1290799
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
^ permalink raw reply
* Re: [PATCH 01/12] iio: imu: inv_icm42600: add core of new inv_icm42600 driver
From: Jean-Baptiste Maneyrol @ 2020-05-18 14:14 UTC (permalink / raw)
To: Jonathan Cameron
Cc: jic23@kernel.org, robh+dt@kernel.org, robh@kernel.org,
mchehab+huawei@kernel.org, davem@davemloft.net,
gregkh@linuxfoundation.org, linux-iio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20200508142820.000057f8@Huawei.com>
Hi Jonathan,
thanks for the feedbacks, I'm sorry but I will not be able to have a correct email formatting to respond you inline.
No problem with all the comments. For iio_device_get_drvdata, it would make more sense to use a const struct iio_dev * as argument. I am obliged to do the pointer conversion since iio_get_mount_matrix requires the use of a const struct iio_dev *.
For resume/suspend, I will add commentaries to explain what it is really doing and for which purpose. Sensor states save and restore will remain in this patch, since it makes more sense to have it as a core functionnality, as much as gyro/accel turn on/off.
Thanks.
JB
From: linux-iio-owner@vger.kernel.org <linux-iio-owner@vger.kernel.org> on behalf of Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Sent: Friday, May 8, 2020 15:28
To: Jean-Baptiste Maneyrol <JManeyrol@invensense.com>
Cc: jic23@kernel.org <jic23@kernel.org>; robh+dt@kernel.org <robh+dt@kernel.org>; robh@kernel.org <robh@kernel.org>; mchehab+huawei@kernel.org <mchehab+huawei@kernel.org>; davem@davemloft.net <davem@davemloft.net>; gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>;
linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; devicetree@vger.kernel.org <devicetree@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 01/12] iio: imu: inv_icm42600: add core of new inv_icm42600 driver
CAUTION: This email originated from outside of the organization. Please make sure the sender is who they say they are and do not click links or open attachments unless you recognize the sender and know the content is safe.
On Thu, 7 May 2020 16:42:11 +0200
Jean-Baptiste Maneyrol <jmaneyrol@invensense.com> wrote:
> Core component of a new driver for InvenSense ICM-426xx devices.
> It includes registers definition, main probe/setup, and device
> utility functions.
>
> ICM-426xx devices are latest generation of 6-axis IMU,
> gyroscope+accelerometer and temperature sensor. This device
> includes a 2K FIFO, supports I2C/I3C/SPI, and provides
> intelligent motion features like pedometer, tilt detection,
> and tap detection.
>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@invensense.com>
Hi Jean-Baptiste,
A few minor things inline.
Thanks,
Jonathan
> ---
> drivers/iio/imu/inv_icm42600/inv_icm42600.h | 372 +++++++++++
> .../iio/imu/inv_icm42600/inv_icm42600_core.c | 618 ++++++++++++++++++
> 2 files changed, 990 insertions(+)
> create mode 100644 drivers/iio/imu/inv_icm42600/inv_icm42600.h
> create mode 100644 drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
>
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600.h b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
> new file mode 100644
> index 000000000000..8da4c8249aed
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
> @@ -0,0 +1,372 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020 Invensense, Inc.
> + */
> +
> +#ifndef INV_ICM42600_H_
> +#define INV_ICM42600_H_
> +
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
> +#include <linux/regmap.h>
> +#include <linux/mutex.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/pm.h>
> +#include <linux/iio/iio.h>
> +
> +enum inv_icm42600_chip {
> + INV_CHIP_ICM42600,
> + INV_CHIP_ICM42602,
> + INV_CHIP_ICM42605,
> + INV_CHIP_ICM42622,
> + INV_CHIP_NB,
> +};
> +
> +/* serial bus slew rates */
> +enum inv_icm42600_slew_rate {
> + INV_ICM42600_SLEW_RATE_20_60NS,
> + INV_ICM42600_SLEW_RATE_12_36NS,
> + INV_ICM42600_SLEW_RATE_6_18NS,
> + INV_ICM42600_SLEW_RATE_4_12NS,
> + INV_ICM42600_SLEW_RATE_2_6NS,
> + INV_ICM42600_SLEW_RATE_INF_2NS,
> +};
> +
> +enum inv_icm42600_sensor_mode {
> + INV_ICM42600_SENSOR_MODE_OFF,
> + INV_ICM42600_SENSOR_MODE_STANDBY,
> + INV_ICM42600_SENSOR_MODE_LOW_POWER,
> + INV_ICM42600_SENSOR_MODE_LOW_NOISE,
> + INV_ICM42600_SENSOR_MODE_NB,
> +};
> +
> +/* gyroscope fullscale values */
> +enum inv_icm42600_gyro_fs {
> + INV_ICM42600_GYRO_FS_2000DPS,
> + INV_ICM42600_GYRO_FS_1000DPS,
> + INV_ICM42600_GYRO_FS_500DPS,
> + INV_ICM42600_GYRO_FS_250DPS,
> + INV_ICM42600_GYRO_FS_125DPS,
> + INV_ICM42600_GYRO_FS_62_5DPS,
> + INV_ICM42600_GYRO_FS_31_25DPS,
> + INV_ICM42600_GYRO_FS_15_625DPS,
> + INV_ICM42600_GYRO_FS_NB,
> +};
> +
> +/* accelerometer fullscale values */
> +enum inv_icm42600_accel_fs {
> + INV_ICM42600_ACCEL_FS_16G,
> + INV_ICM42600_ACCEL_FS_8G,
> + INV_ICM42600_ACCEL_FS_4G,
> + INV_ICM42600_ACCEL_FS_2G,
> + INV_ICM42600_ACCEL_FS_NB,
> +};
> +
> +/* ODR suffixed by LN or LP are Low-Noise or Low-Power mode only */
> +enum inv_icm42600_odr {
> + INV_ICM42600_ODR_8KHZ_LN = 3,
> + INV_ICM42600_ODR_4KHZ_LN,
> + INV_ICM42600_ODR_2KHZ_LN,
> + INV_ICM42600_ODR_1KHZ_LN,
> + INV_ICM42600_ODR_200HZ,
> + INV_ICM42600_ODR_100HZ,
> + INV_ICM42600_ODR_50HZ,
> + INV_ICM42600_ODR_25HZ,
> + INV_ICM42600_ODR_12_5HZ,
> + INV_ICM42600_ODR_6_25HZ_LP,
> + INV_ICM42600_ODR_3_125HZ_LP,
> + INV_ICM42600_ODR_1_5625HZ_LP,
> + INV_ICM42600_ODR_500HZ,
> + INV_ICM42600_ODR_NB,
> +};
> +
> +enum inv_icm42600_filter {
> + /* Low-Noise mode sensor data filter (3rd order filter by default) */
> + INV_ICM42600_FILTER_BW_ODR_DIV_2,
> +
> + /* Low-Power mode sensor data filter (averaging) */
> + INV_ICM42600_FILTER_AVG_1X = 1,
> + INV_ICM42600_FILTER_AVG_16X = 6,
> +};
> +
> +struct inv_icm42600_sensor_conf {
> + int mode;
> + int fs;
> + int odr;
> + int filter;
> +};
> +#define INV_ICM42600_SENSOR_CONF_INIT {-1, -1, -1, -1}
> +
> +struct inv_icm42600_conf {
> + struct inv_icm42600_sensor_conf gyro;
> + struct inv_icm42600_sensor_conf accel;
> + bool temp_en;
> +};
> +
> +struct inv_icm42600_suspended {
> + enum inv_icm42600_sensor_mode gyro;
> + enum inv_icm42600_sensor_mode accel;
> + bool temp;
> +};
> +
> +/*
/**
It's valid kernel doc so lets mark it as such.
> + * struct inv_icm42600_state - driver state variables
> + * @lock: chip access lock.
Nice to be a bit more specific on that. What about the chip needs
a lock at this level as opposed to bus locks etc?
> + * @chip: chip identifier.
> + * @name: chip name.
> + * @map: regmap pointer.
> + * @vdd_supply: VDD voltage regulator for the chip.
> + * @vddio_supply: I/O voltage regulator for the chip.
> + * @orientation: sensor chip orientation relative to main hardware.
> + * @conf: chip sensors configurations.
> + * @suspended: suspended sensors configuration.
> + */
> +struct inv_icm42600_state {
> + struct mutex lock;
> + enum inv_icm42600_chip chip;
> + const char *name;
> + struct regmap *map;
> + struct regulator *vdd_supply;
> + struct regulator *vddio_supply;
> + struct iio_mount_matrix orientation;
> + struct inv_icm42600_conf conf;
> + struct inv_icm42600_suspended suspended;
> +};
> +
> +/* Virtual register addresses: @bank on MSB (4 upper bits), @address on LSB */
> +
> +/* Bank selection register, available in all banks */
> +#define INV_ICM42600_REG_BANK_SEL 0x76
> +#define INV_ICM42600_BANK_SEL_MASK GENMASK(2, 0)
> +
> +/* User bank 0 (MSB 0x00) */
> +#define INV_ICM42600_REG_DEVICE_CONFIG 0x0011
> +#define INV_ICM42600_DEVICE_CONFIG_SOFT_RESET BIT(0)
> +
> +#define INV_ICM42600_REG_DRIVE_CONFIG 0x0013
> +#define INV_ICM42600_DRIVE_CONFIG_I2C_MASK GENMASK(5, 3)
> +#define INV_ICM42600_DRIVE_CONFIG_I2C(_rate) \
> + FIELD_PREP(INV_ICM42600_DRIVE_CONFIG_I2C_MASK, (_rate))
> +#define INV_ICM42600_DRIVE_CONFIG_SPI_MASK GENMASK(2, 0)
> +#define INV_ICM42600_DRIVE_CONFIG_SPI(_rate) \
> + FIELD_PREP(INV_ICM42600_DRIVE_CONFIG_SPI_MASK, (_rate))
> +
> +#define INV_ICM42600_REG_INT_CONFIG 0x0014
> +#define INV_ICM42600_INT_CONFIG_INT2_LATCHED BIT(5)
> +#define INV_ICM42600_INT_CONFIG_INT2_PUSH_PULL BIT(4)
> +#define INV_ICM42600_INT_CONFIG_INT2_ACTIVE_HIGH BIT(3)
> +#define INV_ICM42600_INT_CONFIG_INT2_ACTIVE_LOW 0x00
> +#define INV_ICM42600_INT_CONFIG_INT1_LATCHED BIT(2)
> +#define INV_ICM42600_INT_CONFIG_INT1_PUSH_PULL BIT(1)
> +#define INV_ICM42600_INT_CONFIG_INT1_ACTIVE_HIGH BIT(0)
> +#define INV_ICM42600_INT_CONFIG_INT1_ACTIVE_LOW 0x00
> +
> +#define INV_ICM42600_REG_FIFO_CONFIG 0x0016
> +#define INV_ICM42600_FIFO_CONFIG_MASK GENMASK(7, 6)
> +#define INV_ICM42600_FIFO_CONFIG_BYPASS \
> + FIELD_PREP(INV_ICM42600_FIFO_CONFIG_MASK, 0)
> +#define INV_ICM42600_FIFO_CONFIG_STREAM \
> + FIELD_PREP(INV_ICM42600_FIFO_CONFIG_MASK, 1)
> +#define INV_ICM42600_FIFO_CONFIG_STOP_ON_FULL \
> + FIELD_PREP(INV_ICM42600_FIFO_CONFIG_MASK, 2)
> +
> +/* all sensor data are 16 bits (2 registers wide) in big-endian */
> +#define INV_ICM42600_REG_TEMP_DATA 0x001D
> +#define INV_ICM42600_REG_ACCEL_DATA_X 0x001F
> +#define INV_ICM42600_REG_ACCEL_DATA_Y 0x0021
> +#define INV_ICM42600_REG_ACCEL_DATA_Z 0x0023
> +#define INV_ICM42600_REG_GYRO_DATA_X 0x0025
> +#define INV_ICM42600_REG_GYRO_DATA_Y 0x0027
> +#define INV_ICM42600_REG_GYRO_DATA_Z 0x0029
> +#define INV_ICM42600_DATA_INVALID -32768
> +
> +#define INV_ICM42600_REG_INT_STATUS 0x002D
> +#define INV_ICM42600_INT_STATUS_UI_FSYNC BIT(6)
> +#define INV_ICM42600_INT_STATUS_PLL_RDY BIT(5)
> +#define INV_ICM42600_INT_STATUS_RESET_DONE BIT(4)
> +#define INV_ICM42600_INT_STATUS_DATA_RDY BIT(3)
> +#define INV_ICM42600_INT_STATUS_FIFO_THS BIT(2)
> +#define INV_ICM42600_INT_STATUS_FIFO_FULL BIT(1)
> +#define INV_ICM42600_INT_STATUS_AGC_RDY BIT(0)
> +
> +/*
> + * FIFO access registers
> + * FIFO count is 16 bits (2 registers) big-endian
> + * FIFO data is a continuous read register to read FIFO content
> + */
> +#define INV_ICM42600_REG_FIFO_COUNT 0x002E
> +#define INV_ICM42600_REG_FIFO_DATA 0x0030
> +
> +#define INV_ICM42600_REG_SIGNAL_PATH_RESET 0x004B
> +#define INV_ICM42600_SIGNAL_PATH_RESET_DMP_INIT_EN BIT(6)
> +#define INV_ICM42600_SIGNAL_PATH_RESET_DMP_MEM_RESET BIT(5)
> +#define INV_ICM42600_SIGNAL_PATH_RESET_RESET BIT(3)
> +#define INV_ICM42600_SIGNAL_PATH_RESET_TMST_STROBE BIT(2)
> +#define INV_ICM42600_SIGNAL_PATH_RESET_FIFO_FLUSH BIT(1)
> +
> +/* default configuration: all data big-endian and fifo count in bytes */
> +#define INV_ICM42600_REG_INTF_CONFIG0 0x004C
> +#define INV_ICM42600_INTF_CONFIG0_FIFO_HOLD_LAST_DATA BIT(7)
> +#define INV_ICM42600_INTF_CONFIG0_FIFO_COUNT_REC BIT(6)
> +#define INV_ICM42600_INTF_CONFIG0_FIFO_COUNT_ENDIAN BIT(5)
> +#define INV_ICM42600_INTF_CONFIG0_SENSOR_DATA_ENDIAN BIT(4)
> +#define INV_ICM42600_INTF_CONFIG0_UI_SIFS_CFG_MASK GENMASK(1, 0)
> +#define INV_ICM42600_INTF_CONFIG0_UI_SIFS_CFG_SPI_DIS \
> + FIELD_PREP(INV_ICM42600_INTF_CONFIG0_UI_SIFS_CFG_MASK, 2)
> +#define INV_ICM42600_INTF_CONFIG0_UI_SIFS_CFG_I2C_DIS \
> + FIELD_PREP(INV_ICM42600_INTF_CONFIG0_UI_SIFS_CFG_MASK, 3)
> +
> +#define INV_ICM42600_REG_INTF_CONFIG1 0x004D
> +#define INV_ICM42600_INTF_CONFIG1_ACCEL_LP_CLK_RC BIT(3)
> +
> +#define INV_ICM42600_REG_PWR_MGMT0 0x004E
> +#define INV_ICM42600_PWR_MGMT0_TEMP_DIS BIT(5)
> +#define INV_ICM42600_PWR_MGMT0_IDLE BIT(4)
> +#define INV_ICM42600_PWR_MGMT0_GYRO(_mode) \
> + FIELD_PREP(GENMASK(3, 2), (_mode))
> +#define INV_ICM42600_PWR_MGMT0_ACCEL(_mode) \
> + FIELD_PREP(GENMASK(1, 0), (_mode))
> +
> +#define INV_ICM42600_REG_GYRO_CONFIG0 0x004F
> +#define INV_ICM42600_GYRO_CONFIG0_FS(_fs) \
> + FIELD_PREP(GENMASK(7, 5), (_fs))
> +#define INV_ICM42600_GYRO_CONFIG0_ODR(_odr) \
> + FIELD_PREP(GENMASK(3, 0), (_odr))
> +
> +#define INV_ICM42600_REG_ACCEL_CONFIG0 0x0050
> +#define INV_ICM42600_ACCEL_CONFIG0_FS(_fs) \
> + FIELD_PREP(GENMASK(7, 5), (_fs))
> +#define INV_ICM42600_ACCEL_CONFIG0_ODR(_odr) \
> + FIELD_PREP(GENMASK(3, 0), (_odr))
> +
> +#define INV_ICM42600_REG_GYRO_ACCEL_CONFIG0 0x0052
> +#define INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(_f) \
> + FIELD_PREP(GENMASK(7, 4), (_f))
> +#define INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(_f) \
> + FIELD_PREP(GENMASK(3, 0), (_f))
> +
> +#define INV_ICM42600_REG_TMST_CONFIG 0x0054
> +#define INV_ICM42600_TMST_CONFIG_MASK GENMASK(4, 0)
> +#define INV_ICM42600_TMST_CONFIG_TMST_TO_REGS_EN BIT(4)
> +#define INV_ICM42600_TMST_CONFIG_TMST_RES_16US BIT(3)
> +#define INV_ICM42600_TMST_CONFIG_TMST_DELTA_EN BIT(2)
> +#define INV_ICM42600_TMST_CONFIG_TMST_FSYNC_EN BIT(1)
> +#define INV_ICM42600_TMST_CONFIG_TMST_EN BIT(0)
> +
> +#define INV_ICM42600_REG_FIFO_CONFIG1 0x005F
> +#define INV_ICM42600_FIFO_CONFIG1_RESUME_PARTIAL_RD BIT(6)
> +#define INV_ICM42600_FIFO_CONFIG1_WM_GT_TH BIT(5)
> +#define INV_ICM42600_FIFO_CONFIG1_TMST_FSYNC_EN BIT(3)
> +#define INV_ICM42600_FIFO_CONFIG1_TEMP_EN BIT(2)
> +#define INV_ICM42600_FIFO_CONFIG1_GYRO_EN BIT(1)
> +#define INV_ICM42600_FIFO_CONFIG1_ACCEL_EN BIT(0)
> +
> +/* FIFO watermark is 16 bits (2 registers wide) in little-endian */
> +#define INV_ICM42600_REG_FIFO_WATERMARK 0x0060
> +#define INV_ICM42600_FIFO_WATERMARK_VAL(_wm) \
> + cpu_to_le16((_wm) & GENMASK(11, 0))
> +/* FIFO is 2048 bytes, let 12 samples for reading latency */
> +#define INV_ICM42600_FIFO_WATERMARK_MAX (2048 - 12 * 16)
> +
> +#define INV_ICM42600_REG_INT_CONFIG1 0x0064
> +#define INV_ICM42600_INT_CONFIG1_TPULSE_DURATION BIT(6)
> +#define INV_ICM42600_INT_CONFIG1_TDEASSERT_DISABLE BIT(5)
> +#define INV_ICM42600_INT_CONFIG1_ASYNC_RESET BIT(4)
> +
> +#define INV_ICM42600_REG_INT_SOURCE0 0x0065
> +#define INV_ICM42600_INT_SOURCE0_UI_FSYNC_INT1_EN BIT(6)
> +#define INV_ICM42600_INT_SOURCE0_PLL_RDY_INT1_EN BIT(5)
> +#define INV_ICM42600_INT_SOURCE0_RESET_DONE_INT1_EN BIT(4)
> +#define INV_ICM42600_INT_SOURCE0_UI_DRDY_INT1_EN BIT(3)
> +#define INV_ICM42600_INT_SOURCE0_FIFO_THS_INT1_EN BIT(2)
> +#define INV_ICM42600_INT_SOURCE0_FIFO_FULL_INT1_EN BIT(1)
> +#define INV_ICM42600_INT_SOURCE0_UI_AGC_RDY_INT1_EN BIT(0)
> +
> +#define INV_ICM42600_REG_WHOAMI 0x0075
> +#define INV_ICM42600_WHOAMI_ICM42600 0x40
> +#define INV_ICM42600_WHOAMI_ICM42602 0x41
> +#define INV_ICM42600_WHOAMI_ICM42605 0x42
> +#define INV_ICM42600_WHOAMI_ICM42622 0x46
> +
> +/* User bank 1 (MSB 0x10) */
> +#define INV_ICM42600_REG_SENSOR_CONFIG0 0x1003
> +#define INV_ICM42600_SENSOR_CONFIG0_ZG_DISABLE BIT(5)
> +#define INV_ICM42600_SENSOR_CONFIG0_YG_DISABLE BIT(4)
> +#define INV_ICM42600_SENSOR_CONFIG0_XG_DISABLE BIT(3)
> +#define INV_ICM42600_SENSOR_CONFIG0_ZA_DISABLE BIT(2)
> +#define INV_ICM42600_SENSOR_CONFIG0_YA_DISABLE BIT(1)
> +#define INV_ICM42600_SENSOR_CONFIG0_XA_DISABLE BIT(0)
> +
> +/* Timestamp value is 20 bits (3 registers) in little-endian */
> +#define INV_ICM42600_REG_TMSTVAL 0x1062
> +#define INV_ICM42600_TMSTVAL_MASK GENMASK(19, 0)
> +
> +#define INV_ICM42600_REG_INTF_CONFIG4 0x107A
> +#define INV_ICM42600_INTF_CONFIG4_I3C_BUS_ONLY BIT(6)
> +#define INV_ICM42600_INTF_CONFIG4_SPI_AP_4WIRE BIT(1)
> +
> +#define INV_ICM42600_REG_INTF_CONFIG6 0x107C
> +#define INV_ICM42600_INTF_CONFIG6_MASK GENMASK(4, 0)
> +#define INV_ICM42600_INTF_CONFIG6_I3C_EN BIT(4)
> +#define INV_ICM42600_INTF_CONFIG6_I3C_IBI_BYTE_EN BIT(3)
> +#define INV_ICM42600_INTF_CONFIG6_I3C_IBI_EN BIT(2)
> +#define INV_ICM42600_INTF_CONFIG6_I3C_DDR_EN BIT(1)
> +#define INV_ICM42600_INTF_CONFIG6_I3C_SDR_EN BIT(0)
> +
> +/* User bank 4 (MSB 0x40) */
> +#define INV_ICM42600_REG_INT_SOURCE8 0x404F
> +#define INV_ICM42600_INT_SOURCE8_FSYNC_IBI_EN BIT(5)
> +#define INV_ICM42600_INT_SOURCE8_PLL_RDY_IBI_EN BIT(4)
> +#define INV_ICM42600_INT_SOURCE8_UI_DRDY_IBI_EN BIT(3)
> +#define INV_ICM42600_INT_SOURCE8_FIFO_THS_IBI_EN BIT(2)
> +#define INV_ICM42600_INT_SOURCE8_FIFO_FULL_IBI_EN BIT(1)
> +#define INV_ICM42600_INT_SOURCE8_AGC_RDY_IBI_EN BIT(0)
> +
> +#define INV_ICM42600_REG_OFFSET_USER0 0x4077
> +#define INV_ICM42600_REG_OFFSET_USER1 0x4078
> +#define INV_ICM42600_REG_OFFSET_USER2 0x4079
> +#define INV_ICM42600_REG_OFFSET_USER3 0x407A
> +#define INV_ICM42600_REG_OFFSET_USER4 0x407B
> +#define INV_ICM42600_REG_OFFSET_USER5 0x407C
> +#define INV_ICM42600_REG_OFFSET_USER6 0x407D
> +#define INV_ICM42600_REG_OFFSET_USER7 0x407E
> +#define INV_ICM42600_REG_OFFSET_USER8 0x407F
> +
> +/* Sleep times required by the driver */
> +#define INV_ICM42600_POWER_UP_TIME_MS 100
> +#define INV_ICM42600_RESET_TIME_MS 1
> +#define INV_ICM42600_ACCEL_STARTUP_TIME_MS 20
> +#define INV_ICM42600_GYRO_STARTUP_TIME_MS 60
> +#define INV_ICM42600_GYRO_STOP_TIME_MS 150
> +#define INV_ICM42600_TEMP_STARTUP_TIME_MS 14
> +#define INV_ICM42600_SUSPEND_DELAY_MS 2000
> +
> +typedef int (*inv_icm42600_bus_setup)(struct inv_icm42600_state *);
> +
> +extern const struct regmap_config inv_icm42600_regmap_config;
> +extern const struct dev_pm_ops inv_icm42600_pm_ops;
> +
> +const struct iio_mount_matrix *
> +inv_icm42600_get_mount_matrix(const struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan);
> +
> +uint32_t inv_icm42600_odr_to_period(enum inv_icm42600_odr odr);
> +
> +int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
> + struct inv_icm42600_sensor_conf *conf,
> + unsigned int *sleep);
> +
> +int inv_icm42600_set_gyro_conf(struct inv_icm42600_state *st,
> + struct inv_icm42600_sensor_conf *conf,
> + unsigned int *sleep);
> +
> +int inv_icm42600_set_temp_conf(struct inv_icm42600_state *st, bool enable,
> + unsigned int *sleep);
> +
> +int inv_icm42600_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
> + unsigned int writeval, unsigned int *readval);
> +
> +int inv_icm42600_core_probe(struct regmap *regmap, int chip,
> + inv_icm42600_bus_setup bus_setup);
> +
> +#endif
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> new file mode 100644
> index 000000000000..35bdf4f9d31e
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> @@ -0,0 +1,618 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2020 Invensense, Inc.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/iio/iio.h>
> +
> +#include "inv_icm42600.h"
> +
> +static const struct regmap_range_cfg inv_icm42600_regmap_ranges[] = {
> + {
> + .name = "user banks",
> + .range_min = 0x0000,
> + .range_max = 0x4FFF,
> + .selector_reg = INV_ICM42600_REG_BANK_SEL,
> + .selector_mask = INV_ICM42600_BANK_SEL_MASK,
> + .selector_shift = 0,
> + .window_start = 0,
> + .window_len = 0x1000,
> + },
> +};
> +
> +const struct regmap_config inv_icm42600_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = 0x4FFF,
> + .ranges = inv_icm42600_regmap_ranges,
> + .num_ranges = ARRAY_SIZE(inv_icm42600_regmap_ranges),
> +};
> +EXPORT_SYMBOL_GPL(inv_icm42600_regmap_config);
> +
> +struct inv_icm42600_hw {
> + uint8_t whoami;
> + const char *name;
> + const struct inv_icm42600_conf *conf;
> +};
> +
> +/* chip initial default configuration */
> +static const struct inv_icm42600_conf inv_icm42600_default_conf = {
> + .gyro = {
> + .mode = INV_ICM42600_SENSOR_MODE_OFF,
> + .fs = INV_ICM42600_GYRO_FS_2000DPS,
> + .odr = INV_ICM42600_ODR_50HZ,
> + .filter = INV_ICM42600_FILTER_BW_ODR_DIV_2,
> + },
> + .accel = {
> + .mode = INV_ICM42600_SENSOR_MODE_OFF,
> + .fs = INV_ICM42600_ACCEL_FS_16G,
> + .odr = INV_ICM42600_ODR_50HZ,
> + .filter = INV_ICM42600_FILTER_BW_ODR_DIV_2,
> + },
> + .temp_en = false,
> +};
> +
> +static const struct inv_icm42600_hw inv_icm42600_hw[] = {
> + [INV_CHIP_ICM42600] = {
> + .whoami = INV_ICM42600_WHOAMI_ICM42600,
> + .name = "icm42600",
> + .conf = &inv_icm42600_default_conf,
> + },
> + [INV_CHIP_ICM42602] = {
> + .whoami = INV_ICM42600_WHOAMI_ICM42602,
> + .name = "icm42602",
> + .conf = &inv_icm42600_default_conf,
> + },
> + [INV_CHIP_ICM42605] = {
> + .whoami = INV_ICM42600_WHOAMI_ICM42605,
> + .name = "icm42605",
> + .conf = &inv_icm42600_default_conf,
> + },
> + [INV_CHIP_ICM42622] = {
> + .whoami = INV_ICM42600_WHOAMI_ICM42622,
> + .name = "icm42622",
> + .conf = &inv_icm42600_default_conf,
> + },
> +};
> +
> +const struct iio_mount_matrix *
> +inv_icm42600_get_mount_matrix(const struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + const struct inv_icm42600_state *st =
> + iio_device_get_drvdata((struct iio_dev *)indio_dev);
Interesting... iio_device_get_drvdata is never going to modify
the struct iio_dev. Should we just change that to take a
const struct iio_dev * ?
> +
> + return &st->orientation;
> +}
> +
> +uint32_t inv_icm42600_odr_to_period(enum inv_icm42600_odr odr)
> +{
> + static uint32_t odr_periods[INV_ICM42600_ODR_NB] = {
> + /* reserved values */
> + 0, 0, 0,
> + /* 8kHz */
> + 125000,
> + /* 4kHz */
> + 250000,
> + /* 2kHz */
> + 500000,
> + /* 1kHz */
> + 1000000,
> + /* 200Hz */
> + 5000000,
> + /* 100Hz */
> + 10000000,
> + /* 50Hz */
> + 20000000,
> + /* 25Hz */
> + 40000000,
> + /* 12.5Hz */
> + 80000000,
> + /* 6.25Hz */
> + 160000000,
> + /* 3.125Hz */
> + 320000000,
> + /* 1.5625Hz */
> + 640000000,
> + /* 500Hz */
> + 2000000,
> + };
> +
> + return odr_periods[odr];
> +}
> +
> +static int inv_icm42600_set_pwr_mgmt0(struct inv_icm42600_state *st,
> + enum inv_icm42600_sensor_mode gyro,
> + enum inv_icm42600_sensor_mode accel,
> + bool temp, unsigned int *sleep)
msleep or similar that indicates the units of the sleep time.
> +{
> + enum inv_icm42600_sensor_mode oldgyro = st->conf.gyro.mode;
> + enum inv_icm42600_sensor_mode oldaccel = st->conf.accel.mode;
> + bool oldtemp = st->conf.temp_en;
> + unsigned int sleepval;
> + unsigned int val;
> + int ret;
> +
> + /* if nothing changed, exit */
> + if (gyro == oldgyro && accel == oldaccel && temp == oldtemp)
> + return 0;
> +
> + val = INV_ICM42600_PWR_MGMT0_GYRO(gyro) |
> + INV_ICM42600_PWR_MGMT0_ACCEL(accel);
> + if (!temp)
> + val |= INV_ICM42600_PWR_MGMT0_TEMP_DIS;
> + dev_dbg(regmap_get_device(st->map), "pwr_mgmt0: %#02x\n", val);
I wonder if you have a little too much in the way of debug prints.
These are internal to the code and so could only be wrong due to a local
bug. Once you've finished writing the driver I'd hope we won't need these!
> + ret = regmap_write(st->map, INV_ICM42600_REG_PWR_MGMT0, val);
> + if (ret)
> + return ret;
> +
> + st->conf.gyro.mode = gyro;
> + st->conf.accel.mode = accel;
> + st->conf.temp_en = temp;
> +
> + /* compute required wait time for sensors to stabilize */
> + sleepval = 0;
> + /* temperature stabilization time */
> + if (temp && !oldtemp) {
> + if (sleepval < INV_ICM42600_TEMP_STARTUP_TIME_MS)
> + sleepval = INV_ICM42600_TEMP_STARTUP_TIME_MS;
> + }
> + /* accel startup time */
> + if (accel != oldaccel && oldaccel == INV_ICM42600_SENSOR_MODE_OFF) {
> + /* block any register write for at least 200 µs */
> + usleep_range(200, 300);
> + if (sleepval < INV_ICM42600_ACCEL_STARTUP_TIME_MS)
> + sleepval = INV_ICM42600_ACCEL_STARTUP_TIME_MS;
> + }
> + if (gyro != oldgyro) {
> + /* gyro startup time */
> + if (oldgyro == INV_ICM42600_SENSOR_MODE_OFF) {
> + /* block any register write for at least 200 µs */
> + usleep_range(200, 300);
> + if (sleepval < INV_ICM42600_GYRO_STARTUP_TIME_MS)
> + sleepval = INV_ICM42600_GYRO_STARTUP_TIME_MS;
> + /* gyro stop time */
> + } else if (gyro == INV_ICM42600_SENSOR_MODE_OFF) {
> + if (sleepval < INV_ICM42600_GYRO_STOP_TIME_MS)
> + sleepval = INV_ICM42600_GYRO_STOP_TIME_MS;
> + }
> + }
> +
> + /* deferred sleep value if sleep pointer is provided or direct sleep */
> + if (sleep)
> + *sleep = sleepval;
> + else if (sleepval)
> + msleep(sleepval);
> +
> + return 0;
> +}
> +
> +int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
> + struct inv_icm42600_sensor_conf *conf,
> + unsigned int *sleep)
> +{
> + struct inv_icm42600_sensor_conf *oldconf = &st->conf.accel;
> + unsigned int val;
> + int ret;
> +
> + /* Sanitize missing values with current values */
> + if (conf->mode < 0)
> + conf->mode = oldconf->mode;
> + if (conf->fs < 0)
> + conf->fs = oldconf->fs;
> + if (conf->odr < 0)
> + conf->odr = oldconf->odr;
> + if (conf->filter < 0)
> + conf->filter = oldconf->filter;
> +
> + /* set ACCEL_CONFIG0 register (accel fullscale & odr) */
> + if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
> + val = INV_ICM42600_ACCEL_CONFIG0_FS(conf->fs) |
> + INV_ICM42600_ACCEL_CONFIG0_ODR(conf->odr);
> + dev_dbg(regmap_get_device(st->map), "accel_config0: %#02x\n", val);
> + ret = regmap_write(st->map, INV_ICM42600_REG_ACCEL_CONFIG0, val);
> + if (ret)
> + return ret;
> + oldconf->fs = conf->fs;
> + oldconf->odr = conf->odr;
> + }
> +
> + /* set GYRO_ACCEL_CONFIG0 register (accel filter) */
> + if (conf->filter != oldconf->filter) {
> + val = INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(conf->filter) |
> + INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(st->conf.gyro.filter);
> + dev_dbg(regmap_get_device(st->map), "gyro_accel_config0: %#02x\n", val);
> + ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_ACCEL_CONFIG0, val);
> + if (ret)
> + return ret;
> + oldconf->filter = conf->filter;
> + }
> +
> + /* set PWR_MGMT0 register (accel sensor mode) */
> + return inv_icm42600_set_pwr_mgmt0(st, st->conf.gyro.mode, conf->mode,
> + st->conf.temp_en, sleep);
> +}
> +
> +int inv_icm42600_set_gyro_conf(struct inv_icm42600_state *st,
> + struct inv_icm42600_sensor_conf *conf,
> + unsigned int *sleep)
> +{
> + struct inv_icm42600_sensor_conf *oldconf = &st->conf.gyro;
> + unsigned int val;
> + int ret;
> +
> + /* sanitize missing values with current values */
> + if (conf->mode < 0)
> + conf->mode = oldconf->mode;
> + if (conf->fs < 0)
> + conf->fs = oldconf->fs;
> + if (conf->odr < 0)
> + conf->odr = oldconf->odr;
> + if (conf->filter < 0)
> + conf->filter = oldconf->filter;
> +
> + /* set GYRO_CONFIG0 register (gyro fullscale & odr) */
> + if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
> + val = INV_ICM42600_GYRO_CONFIG0_FS(conf->fs) |
> + INV_ICM42600_GYRO_CONFIG0_ODR(conf->odr);
> + dev_dbg(regmap_get_device(st->map), "gyro_config0: %#02x\n", val);
> + ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_CONFIG0, val);
> + if (ret)
> + return ret;
> + oldconf->fs = conf->fs;
> + oldconf->odr = conf->odr;
> + }
> +
> + /* set GYRO_ACCEL_CONFIG0 register (gyro filter) */
> + if (conf->filter != oldconf->filter) {
> + val = INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(st->conf.accel.filter) |
> + INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(conf->filter);
> + dev_dbg(regmap_get_device(st->map), "gyro_accel_config0: %#02x\n", val);
> + ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_ACCEL_CONFIG0, val);
> + if (ret)
> + return ret;
> + oldconf->filter = conf->filter;
> + }
> +
> + /* set PWR_MGMT0 register (gyro sensor mode) */
> + return inv_icm42600_set_pwr_mgmt0(st, conf->mode, st->conf.accel.mode,
> + st->conf.temp_en, sleep);
> +
> + return 0;
> +}
> +
> +int inv_icm42600_set_temp_conf(struct inv_icm42600_state *st, bool enable,
> + unsigned int *sleep)
> +{
> + return inv_icm42600_set_pwr_mgmt0(st, st->conf.gyro.mode,
> + st->conf.accel.mode, enable,
> + sleep);
> +}
> +
> +int inv_icm42600_debugfs_reg(struct iio_dev *indio_dev, unsigned int reg,
> + unsigned int writeval, unsigned int *readval)
> +{
> + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + int ret;
> +
> + mutex_lock(&st->lock);
> +
> + if (readval)
> + ret = regmap_read(st->map, reg, readval);
> + else
> + ret = regmap_write(st->map, reg, writeval);
> +
> + mutex_unlock(&st->lock);
> +
> + return ret;
> +}
> +
> +static int inv_icm42600_set_conf(struct inv_icm42600_state *st,
> + const struct inv_icm42600_conf *conf)
> +{
> + unsigned int val;
> + int ret;
> +
> + /* set PWR_MGMT0 register (gyro & accel sensor mode, temp enabled) */
> + val = INV_ICM42600_PWR_MGMT0_GYRO(conf->gyro.mode) |
> + INV_ICM42600_PWR_MGMT0_ACCEL(conf->accel.mode);
> + if (!conf->temp_en)
> + val |= INV_ICM42600_PWR_MGMT0_TEMP_DIS;
> + ret = regmap_write(st->map, INV_ICM42600_REG_PWR_MGMT0, val);
> + if (ret)
> + return ret;
> +
> + /* set GYRO_CONFIG0 register (gyro fullscale & odr) */
> + val = INV_ICM42600_GYRO_CONFIG0_FS(conf->gyro.fs) |
> + INV_ICM42600_GYRO_CONFIG0_ODR(conf->gyro.odr);
> + ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_CONFIG0, val);
> + if (ret)
> + return ret;
> +
> + /* set ACCEL_CONFIG0 register (accel fullscale & odr) */
> + val = INV_ICM42600_ACCEL_CONFIG0_FS(conf->accel.fs) |
> + INV_ICM42600_ACCEL_CONFIG0_ODR(conf->accel.odr);
> + ret = regmap_write(st->map, INV_ICM42600_REG_ACCEL_CONFIG0, val);
> + if (ret)
> + return ret;
> +
> + /* set GYRO_ACCEL_CONFIG0 register (gyro & accel filters) */
> + val = INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(conf->accel.filter) |
> + INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(conf->gyro.filter);
> + ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_ACCEL_CONFIG0, val);
> + if (ret)
> + return ret;
> +
> + /* update internal conf */
> + st->conf = *conf;
> +
> + return 0;
> +}
> +
> +/**
> + * inv_icm42600_setup() - check and setup chip.
If doing kernel-doc (which is good) you should do it all.
So document the parameters as well.
It's worth running the kernel-doc script over any file where
you put some and fixing up any warnings / errors.
> + */
> +static int inv_icm42600_setup(struct inv_icm42600_state *st,
> + inv_icm42600_bus_setup bus_setup)
> +{
> + const struct inv_icm42600_hw *hw = &inv_icm42600_hw[st->chip];
> + const struct device *dev = regmap_get_device(st->map);
> + unsigned int mask, val;
> + int ret;
> +
> + /* check chip self-identification value */
> + ret = regmap_read(st->map, INV_ICM42600_REG_WHOAMI, &val);
> + if (ret)
> + return ret;
> + if (val != hw->whoami) {
> + dev_err(dev, "invalid whoami %#02x expected %#02x (%s)\n",
> + val, hw->whoami, hw->name);
> + return -ENODEV;
> + }
> + dev_info(dev, "found %s (%#02x)\n", hw->name, hw->whoami);
Hmm. I'm never that keen on this sort of log noise. Why do you need it
except for initial debug?
> + st->name = hw->name;
> +
> + /* reset to make sure previous state are not there */
> + ret = regmap_write(st->map, INV_ICM42600_REG_DEVICE_CONFIG,
> + INV_ICM42600_DEVICE_CONFIG_SOFT_RESET);
> + if (ret)
> + return ret;
> + msleep(INV_ICM42600_RESET_TIME_MS);
blank line here to separate two logical blocks of code.
Slightly helps readability.
> + ret = regmap_read(st->map, INV_ICM42600_REG_INT_STATUS, &val);
> + if (ret)
> + return ret;
> + if (!(val & INV_ICM42600_INT_STATUS_RESET_DONE)) {
> + dev_err(dev, "reset error, reset done bit not set\n");
> + return -ENODEV;
> + }
> +
> + /* set chip bus configuration */
> + ret = bus_setup(st);
> + if (ret)
> + return ret;
> +
> + /* sensor data in big-endian (default) */
> + mask = INV_ICM42600_INTF_CONFIG0_SENSOR_DATA_ENDIAN;
> + val = INV_ICM42600_INTF_CONFIG0_SENSOR_DATA_ENDIAN;
> + ret = regmap_update_bits(st->map, INV_ICM42600_REG_INTF_CONFIG0,
> + mask, val);
Long line, but I'd rather you just didn't bother will local variables
in cases like this where you just set them to a constant.
Take the 80 chars thing as guidance not a rule :)
> + if (ret)
> + return ret;
> +
> + return inv_icm42600_set_conf(st, hw->conf);
> +}
> +
> +static int inv_icm42600_enable_regulator_vddio(struct inv_icm42600_state *st)
> +{
> + int ret;
> +
> + ret = regulator_enable(st->vddio_supply);
> + if (ret)
> + return ret;
> +
> + /* wait a little for supply ramp */
> + usleep_range(3000, 4000);
> +
> + return 0;
> +}
> +
> +static void inv_icm42600_disable_regulators(void *_data)
> +{
> + struct inv_icm42600_state *st = _data;
> + const struct device *dev = regmap_get_device(st->map);
> + int ret;
> +
> + ret = regulator_disable(st->vddio_supply);
> + if (ret)
> + dev_err(dev, "failed to disable vddio error %d\n", ret);
> +
> + ret = regulator_disable(st->vdd_supply);
> + if (ret)
> + dev_err(dev, "failed to disable vdd error %d\n", ret);
> +}
> +
> +static void inv_icm42600_disable_pm(void *_data)
> +{
> + struct device *dev = _data;
> +
> + pm_runtime_put_sync(dev);
> + pm_runtime_disable(dev);
> +}
> +
> +int inv_icm42600_core_probe(struct regmap *regmap, int chip,
> + inv_icm42600_bus_setup bus_setup)
> +{
> + struct device *dev = regmap_get_device(regmap);
> + struct inv_icm42600_state *st;
> + int ret;
> +
> + BUILD_BUG_ON(ARRAY_SIZE(inv_icm42600_hw) != INV_CHIP_NB);
Why not just give the array an explicit size when you define it above?
I guess it would in theory be possible to not instantiate all of the array
but relying on different size of a variable length array seems less than
ideal.
> + if (chip < 0 || chip >= INV_CHIP_NB) {
> + dev_err(dev, "invalid chip = %d\n", chip);
> + return -ENODEV;
> + }
> +
> + st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
> + if (!st)
> + return -ENOMEM;
nitpick: blank line here.
> + dev_set_drvdata(dev, st);
> + mutex_init(&st->lock);
> + st->chip = chip;
> + st->map = regmap;
> +
> + ret = iio_read_mount_matrix(dev, "mount-matrix", &st->orientation);
> + if (ret) {
> + dev_err(dev, "failed to retrieve mounting matrix %d\n", ret);
> + return ret;
> + }
> +
> + st->vdd_supply = devm_regulator_get(dev, "vdd");
> + if (IS_ERR(st->vdd_supply))
> + return PTR_ERR(st->vdd_supply);
> +
> + st->vddio_supply = devm_regulator_get(dev, "vddio");
> + if (IS_ERR(st->vddio_supply))
> + return PTR_ERR(st->vddio_supply);
> +
> + ret = regulator_enable(st->vdd_supply);
> + if (ret)
> + return ret;
> + msleep(INV_ICM42600_POWER_UP_TIME_MS);
> +
> + ret = inv_icm42600_enable_regulator_vddio(st);
> + if (ret) {
> + regulator_disable(st->vdd_supply);
> + return ret;
> + }
> +
> + ret = devm_add_action_or_reset(dev, inv_icm42600_disable_regulators,
> + st);
I'd prefer to see two devm_add_action_or_reset calls. One for each regulator.
That means you don't have to do the extra disable logic above which is
a bit fragile in amongst a whole load of device managed calls.
> + if (ret)
> + return ret;
> +
> + /* setup chip registers */
> + ret = inv_icm42600_setup(st, bus_setup);
> + if (ret)
> + return ret;
> +
> + /* setup runtime power management */
> + ret = pm_runtime_set_active(dev);
> + if (ret)
> + return ret;
> + pm_runtime_get_noresume(dev);
> + pm_runtime_enable(dev);
> + pm_runtime_set_autosuspend_delay(dev, INV_ICM42600_SUSPEND_DELAY_MS);
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_put(dev);
> +
> + return devm_add_action_or_reset(dev, inv_icm42600_disable_pm, dev);
> +}
> +EXPORT_SYMBOL_GPL(inv_icm42600_core_probe);
> +
> +static int __maybe_unused inv_icm42600_suspend(struct device *dev)
> +{
> + struct inv_icm42600_state *st = dev_get_drvdata(dev);
> + int ret;
> +
> + mutex_lock(&st->lock);
> +
> + st->suspended.gyro = st->conf.gyro.mode;
> + st->suspended.accel = st->conf.accel.mode;
> + st->suspended.temp = st->conf.temp_en;
> + if (pm_runtime_suspended(dev)) {
> + ret = 0;
> + goto out_unlock;
> + }
> +
> + ret = inv_icm42600_set_pwr_mgmt0(st, INV_ICM42600_SENSOR_MODE_OFF,
> + INV_ICM42600_SENSOR_MODE_OFF, false,
> + NULL);
> + if (ret)
> + goto out_unlock;
> +
> + regulator_disable(st->vddio_supply);
> +
> +out_unlock:
> + mutex_unlock(&st->lock);
> + return ret;
> +}
> +
> +static int __maybe_unused inv_icm42600_resume(struct device *dev)
> +{
> + struct inv_icm42600_state *st = dev_get_drvdata(dev);
> + int ret;
> +
> + mutex_lock(&st->lock);
> +
> + ret = inv_icm42600_enable_regulator_vddio(st);
> + if (ret)
> + goto out_unlock;
> +
> + pm_runtime_disable(dev);
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> +
> + /* restore sensors state */
> + ret = inv_icm42600_set_pwr_mgmt0(st, st->suspended.gyro,
> + st->suspended.accel,
> + st->suspended.temp, NULL);
> + if (ret)
> + goto out_unlock;
You may need this later, but for now it's a bit comic so ideally introduce
it only when needed.
> +
> +out_unlock:
> + mutex_unlock(&st->lock);
> + return ret;
> +}
> +
> +static int __maybe_unused inv_icm42600_runtime_suspend(struct device *dev)
> +{
> + struct inv_icm42600_state *st = dev_get_drvdata(dev);
> + int ret;
> +
> + mutex_lock(&st->lock);
> +
> + /* disable all sensors */
> + ret = inv_icm42600_set_pwr_mgmt0(st, INV_ICM42600_SENSOR_MODE_OFF,
> + INV_ICM42600_SENSOR_MODE_OFF, false,
> + NULL);
> + if (ret)
> + goto error_unlock;
> +
> + regulator_disable(st->vddio_supply);
> +
> +error_unlock:
> + mutex_unlock(&st->lock);
> + return ret;
> +}
> +
> +static int __maybe_unused inv_icm42600_runtime_resume(struct device *dev)
> +{
> + struct inv_icm42600_state *st = dev_get_drvdata(dev);
> + int ret;
> +
> + mutex_lock(&st->lock);
> +
Why don't we need to reenable all the sensors we disabled in runtime suspend?
I can guess why we might not, but a comment here to explain would save on
possible confusion..
> + ret = inv_icm42600_enable_regulator_vddio(st);
> +
> + mutex_unlock(&st->lock);
> + return ret;
> +}
> +
> +const struct dev_pm_ops inv_icm42600_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(inv_icm42600_suspend, inv_icm42600_resume)
> + SET_RUNTIME_PM_OPS(inv_icm42600_runtime_suspend,
> + inv_icm42600_runtime_resume, NULL)
> +};
> +EXPORT_SYMBOL_GPL(inv_icm42600_pm_ops);
> +
> +MODULE_AUTHOR("InvenSense, Inc.");
> +MODULE_DESCRIPTION("InvenSense ICM-426xx device driver");
> +MODULE_LICENSE("GPL");
^ 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