* Re: [RESEND PATCH v2 1/3] dt-bindings: phy: Drop reset-gpios from marvell,mmp3-hsic-phy
From: Rob Herring @ 2020-05-18 18:17 UTC (permalink / raw)
To: Lubomir Rintel; +Cc: linux-kernel, Kishon Vijay Abraham I, devicetree
In-Reply-To: <20200509081754.474787-2-lkundrak@v3.sk>
On Sat, 9 May 2020 10:17:52 +0200, Lubomir Rintel wrote:
> This has been added in error -- the PHY block doesn't have a reset pin.
>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
> .../devicetree/bindings/phy/marvell,mmp3-hsic-phy.yaml | 7 -------
> 1 file changed, 7 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 1/3] dt-bindings: mailbox: Add devicetree binding for Qcom IPCC
From: Rob Herring @ 2020-05-18 18:16 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: jassisinghbrar, bjorn.andersson, linux-arm-msm, linux-kernel,
devicetree
In-Reply-To: <20200508182703.29021-2-manivannan.sadhasivam@linaro.org>
On Fri, May 08, 2020 at 11:57:01PM +0530, Manivannan Sadhasivam wrote:
> Add devicetree YAML binding for Qualcomm Inter-Processor Communication
> Controller (IPCC) block.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
> .../bindings/mailbox/qcom-ipcc.yaml | 77 +++++++++++++++++++
> include/dt-bindings/mailbox/qcom-ipcc.h | 33 ++++++++
> 2 files changed, 110 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/qcom-ipcc.yaml
> create mode 100644 include/dt-bindings/mailbox/qcom-ipcc.h
>
> diff --git a/Documentation/devicetree/bindings/mailbox/qcom-ipcc.yaml b/Documentation/devicetree/bindings/mailbox/qcom-ipcc.yaml
> new file mode 100644
> index 000000000000..62e7bc8ceb0b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/qcom-ipcc.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mailbox/qcom-ipcc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Technologies, Inc. Inter-Processor Communication Controller
> +
> +maintainers:
> + - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> +
> +description:
> + The Inter-Processor Communication Controller (IPCC) is a centralized hardware
> + to route interrupts across various subsystems. It involves a three-level
> + addressing scheme called protocol, client and signal. For example, consider an
> + entity on the Application Processor Subsystem (APSS) that wants to listen to
> + Modem's interrupts via Shared Memory Point to Point (SMP2P) interface. In such
> + a case, the client would be Modem (client-id is 2) and the signal would be
> + SMP2P (signal-id is 2). The SMP2P itself falls under the Multiprocessor (MPROC)
> + protocol (protocol-id is 0). Refer include/dt-bindings/mailbox/qcom-ipcc.h
> + for the list of such IDs.
> +
> +properties:
> + compatible:
> + const: "qcom,ipcc"
Need SoC specific compatible(s).
No need for quotes.
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + interrupt-controller: true
> +
> + "#interrupt-cells":
> + const: 3
> + description:
> + The first cell is the client-id, the second cell is the signal-id and the
> + third cell is the interrupt type.
> +
> + "#mbox-cells":
> + const: 2
> + description:
> + The first cell is the client-id, and the second cell is the signal-id.
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - interrupt-controller
> + - "#interrupt-cells"
> + - "#mbox-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/mailbox/qcom-ipcc.h>
> +
> + mailbox: qcom,ipcc@408000 {
mailbox@408000
> + compatible = "qcom,ipcc";
> + reg = <0x408000 0x1000>;
> + interrupts = <GIC_SPI 229 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-controller;
> + #interrupt-cells = <3>;
> + #mbox-cells = <2>;
> + };
> +
> + smp2p-modem {
> + compatible = "qcom,smp2p";
> + interrupts-extended = <&ipcc_mproc IPCC_CLIENT_MPSS
> + IPCC_MPROC_SIGNAL_SMP2P IRQ_TYPE_EDGE_RISING>;
> + mboxes = <&ipcc_mproc IPCC_CLIENT_MPSS IPCC_MPROC_SIGNAL_SMP2P>;
> +
> + /* Other SMP2P fields */
> + };
> diff --git a/include/dt-bindings/mailbox/qcom-ipcc.h b/include/dt-bindings/mailbox/qcom-ipcc.h
> new file mode 100644
> index 000000000000..b8c04f6df57c
> --- /dev/null
> +++ b/include/dt-bindings/mailbox/qcom-ipcc.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
Dual license please. I'm pretty sure your employer has an opinion on
this.
> +/*
> + * Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef __DT_BINDINGS_MAILBOX_IPCC_H
> +#define __DT_BINDINGS_MAILBOX_IPCC_H
> +
> +/* Signal IDs for MPROC protocol */
> +#define IPCC_MPROC_SIGNAL_GLINK_QMP 0
> +#define IPCC_MPROC_SIGNAL_SMP2P 2
> +#define IPCC_MPROC_SIGNAL_PING 3
> +
> +/* Client IDs */
> +#define IPCC_CLIENT_AOP 0
> +#define IPCC_CLIENT_TZ 1
> +#define IPCC_CLIENT_MPSS 2
> +#define IPCC_CLIENT_LPASS 3
> +#define IPCC_CLIENT_SLPI 4
> +#define IPCC_CLIENT_SDC 5
> +#define IPCC_CLIENT_CDSP 6
> +#define IPCC_CLIENT_NPU 7
> +#define IPCC_CLIENT_APSS 8
> +#define IPCC_CLIENT_GPU 9
> +#define IPCC_CLIENT_CVP 10
> +#define IPCC_CLIENT_CAM 11
> +#define IPCC_CLIENT_VPU 12
> +#define IPCC_CLIENT_PCIE0 13
> +#define IPCC_CLIENT_PCIE1 14
> +#define IPCC_CLIENT_PCIE2 15
> +#define IPCC_CLIENT_SPSS 16
> +
> +#endif
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v5 0/6] drm: Prepare to use a GPIO on ti-sn65dsi86 for Hot Plug Detect
From: Sam Ravnborg @ 2020-05-18 17:59 UTC (permalink / raw)
To: Doug Anderson
Cc: LinusW, Stephen Boyd, Laurent Pinchart, Neil Armstrong,
David Airlie, Sandeep Panda, dri-devel, Bjorn Andersson,
Andrzej Hajda, Thierry Reding, Rob Clark, Jeffrey Hugo,
Krzysztof Kozlowski, Bartosz Golaszewski, Andy Gross,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jonas Karlman, linux-arm-msm, open list:GPIO SUBSYSTEM,
Rob Herring, Paul Walmsley, Jernej Skrabec, Stephen Boyd, LKML
In-Reply-To: <CAD=FV=UNuwb+YYJKw9+HNMKUNfuNFxj+Gr+yB9tXANbXAvDgCg@mail.gmail.com>
Hi Douglas.
> > 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?
Applied, including the small fix pointed out by Linus.
Sam
^ permalink raw reply
* Re: [PATCH v4 6/8] dt-bindings: mtd: Document ARASAN NAND bindings
From: Rob Herring @ 2020-05-18 18:12 UTC (permalink / raw)
To: Miquel Raynal
Cc: Naga Sureshkumar Relli, Michal Simek, Vignesh Raghavendra,
Tudor Ambarus, Rob Herring, Richard Weinberger, Thomas Petazzoni,
linux-mtd, Boris Brezillon, Mark Rutland, devicetree
In-Reply-To: <20200508171339.8052-7-miquel.raynal@bootlin.com>
On Fri, 8 May 2020 19:13:37 +0200, Miquel Raynal wrote:
> Document the Arasan NAND controller bindings.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> .../bindings/mtd/arasan,nand-controller.yaml | 63 +++++++++++++++++++
> 1 file changed, 63 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mtd/arasan,nand-controller.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH V4 8/8] arm64: dts: ipq6018: Add a53 pll and apcs clock
From: Bjorn Andersson @ 2020-05-18 18:03 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: agross, mturquette, sboyd, robh+dt, jassisinghbrar, linux-arm-msm,
linux-clk, devicetree, linux-kernel
In-Reply-To: <80039c21-41cf-2ace-939a-ef48fd773aa4@codeaurora.org>
On Tue 12 May 20:55 PDT 2020, Sivaprakash Murugesan wrote:
> Hi Bjorn,
>
> On 5/13/2020 1:54 AM, Bjorn Andersson wrote:
> > On Sun 03 May 23:20 PDT 2020, Sivaprakash Murugesan wrote:
> >
> > > add support for apps pll and apcs clock.
> > >
> > > Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> > > ---
> > > arch/arm64/boot/dts/qcom/ipq6018.dtsi | 16 +++++++++++++---
> > > 1 file changed, 13 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> > > index 1aa8d85..af2ceeb 100644
> > > --- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
> > > @@ -294,12 +294,22 @@
> > > };
> > > apcs_glb: mailbox@b111000 {
> > > - compatible = "qcom,ipq8074-apcs-apps-global";
> > > - reg = <0x0b111000 0xc>;
> > > -
> > > + compatible = "qcom,ipq6018-apcs-apps-global";
> > > + reg = <0x0b111000 0x1000>;
> > My documentation states that IPQ8074 indeed has this block at
> > 0x0b111000, but IPQ6018 it's at 0x6b111000. Can you confirm this is
> > correct? Same with the pll below.
> The address 0x6b111000 is how the RPM sees this block. For A53 it is still
> 0xb111000
Okay, thanks for confirming.
Regards,
Bjorn
^ permalink raw reply
* [PATCH 3/3] ARM: dts: qcom: Add MikroTik RB3011
From: Jonathan McDowell @ 2020-05-18 18:10 UTC (permalink / raw)
To: linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <cover.1589824955.git.noodles@earth.li>
This patch adds a DTS file for the MikroTik RouterBoard 3011, which is a
1U rackmount router based on the IPQ8064, supporting the serial UART,
dual QCA8337 Gigabit ethernet switches, boot loader NOR and user LED
device.
Signed-off-by: Jonathan McDowell <noodles@earth.li>
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/qcom-ipq8064-rb3011.dts | 308 ++++++++++++++++++++++
2 files changed, 309 insertions(+)
create mode 100644 arch/arm/boot/dts/qcom-ipq8064-rb3011.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index e8dd99201397..e697a4bd7426 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -884,6 +884,7 @@ dtb-$(CONFIG_ARCH_QCOM) += \
qcom-ipq4019-ap.dk07.1-c1.dtb \
qcom-ipq4019-ap.dk07.1-c2.dtb \
qcom-ipq8064-ap148.dtb \
+ qcom-ipq8064-rb3011.dtb \
qcom-msm8660-surf.dtb \
qcom-msm8960-cdp.dtb \
qcom-msm8974-fairphone-fp2.dtb \
diff --git a/arch/arm/boot/dts/qcom-ipq8064-rb3011.dts b/arch/arm/boot/dts/qcom-ipq8064-rb3011.dts
new file mode 100644
index 000000000000..282b89ce3d45
--- /dev/null
+++ b/arch/arm/boot/dts/qcom-ipq8064-rb3011.dts
@@ -0,0 +1,308 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "qcom-ipq8064.dtsi"
+#include <dt-bindings/input/input.h>
+
+/ {
+ model = "MikroTik RB3011UiAS-RM";
+ compatible = "mikrotik,rb3011";
+
+ aliases {
+ serial0 = &gsbi7_serial;
+ ethernet0 = &gmac0;
+ ethernet1 = &gmac3;
+ mdio-gpio0 = &mdio0;
+ mdio-gpio1 = &mdio1;
+ };
+
+ chosen {
+ bootargs = "loglevel=8 console=ttyMSM0,115200";
+ stdout-path = "serial0:115200n8";
+ };
+
+ memory@0 {
+ reg = <0x42000000 0x3e000000>;
+ device_type = "memory";
+ };
+
+ mdio0: mdio@0 {
+ status = "okay";
+ compatible = "virtual,mdio-gpio";
+ gpios = <&qcom_pinmux 1 GPIO_ACTIVE_HIGH>,
+ <&qcom_pinmux 0 GPIO_ACTIVE_HIGH>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pinctrl-0 = <&mdio0_pins>;
+ pinctrl-names = "default";
+
+ switch0: switch@10 {
+ compatible = "qca,qca8337";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dsa,member = <0 0>;
+
+ pinctrl-0 = <&sw0_reset_pin>;
+ pinctrl-names = "default";
+
+ reset-gpios = <&qcom_pinmux 16 GPIO_ACTIVE_LOW>;
+ reg = <0x10>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ switch0cpu: port@0 {
+ reg = <0>;
+ label = "cpu";
+ ethernet = <&gmac0>;
+ phy-mode = "rgmii-id";
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ label = "sw1";
+ };
+
+ port@2 {
+ reg = <2>;
+ label = "sw2";
+ };
+
+ port@3 {
+ reg = <3>;
+ label = "sw3";
+ };
+
+ port@4 {
+ reg = <4>;
+ label = "sw4";
+ };
+
+ port@5 {
+ reg = <5>;
+ label = "sw5";
+ };
+ };
+ };
+ };
+
+ mdio1: mdio@1 {
+ status = "okay";
+ compatible = "virtual,mdio-gpio";
+ gpios = <&qcom_pinmux 11 GPIO_ACTIVE_HIGH>,
+ <&qcom_pinmux 10 GPIO_ACTIVE_HIGH>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pinctrl-0 = <&mdio1_pins>;
+ pinctrl-names = "default";
+
+ switch1: switch@14 {
+ compatible = "qca,qca8337";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dsa,member = <1 0>;
+
+ pinctrl-0 = <&sw1_reset_pin>;
+ pinctrl-names = "default";
+
+ reset-gpios = <&qcom_pinmux 17 GPIO_ACTIVE_LOW>;
+ reg = <0x10>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ switch1cpu: port@0 {
+ reg = <0>;
+ label = "cpu";
+ ethernet = <&gmac3>;
+ phy-mode = "sgmii";
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ label = "sw6";
+ };
+
+ port@2 {
+ reg = <2>;
+ label = "sw7";
+ };
+
+ port@3 {
+ reg = <3>;
+ label = "sw8";
+ };
+
+ port@4 {
+ reg = <4>;
+ label = "sw9";
+ };
+
+ port@5 {
+ reg = <5>;
+ label = "sw10";
+ };
+ };
+ };
+ };
+
+ soc {
+ gsbi5: gsbi@1a200000 {
+ qcom,mode = <GSBI_PROT_SPI>;
+ status = "okay";
+
+ spi4: spi@1a280000 {
+ status = "okay";
+ spi-max-frequency = <50000000>;
+
+ pinctrl-0 = <&spi_pins>;
+ pinctrl-names = "default";
+
+ cs-gpios = <&qcom_pinmux 20 GPIO_ACTIVE_HIGH>;
+
+ norflash: s25fl016k@0 {
+ compatible = "jedec,spi-nor";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ spi-max-frequency = <50000000>;
+ reg = <0>;
+
+ partition@0 {
+ label = "RouterBoot";
+ reg = <0x0 0x40000>;
+ };
+ };
+ };
+ };
+
+ gpio_keys {
+ compatible = "gpio-keys";
+ pinctrl-0 = <&buttons_pins>;
+ pinctrl-names = "default";
+
+ button@1 {
+ label = "reset";
+ linux,code = <KEY_RESTART>;
+ gpios = <&qcom_pinmux 66 GPIO_ACTIVE_LOW>;
+ linux,input-type = <1>;
+ debounce-interval = <60>;
+ };
+ };
+
+ leds {
+ compatible = "gpio-leds";
+ pinctrl-0 = <&leds_pins>;
+ pinctrl-names = "default";
+
+ led@7 {
+ label = "rb3011:green:user";
+ gpios = <&qcom_pinmux 33 GPIO_ACTIVE_HIGH>;
+ default-state = "off";
+ };
+ };
+
+ };
+};
+
+&gmac0 {
+ status = "okay";
+
+ phy-mode = "rgmii";
+ qcom,id = <0>;
+ phy-handle = <&switch0cpu>;
+
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+};
+
+&gmac3 {
+ status = "okay";
+
+ phy-mode = "sgmii";
+ qcom,id = <3>;
+ phy-handle = <&switch1cpu>;
+
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+};
+
+&gsbi7 {
+ status = "okay";
+ qcom,mode = <GSBI_PROT_I2C_UART>;
+};
+
+&gsbi7_serial {
+ status = "okay";
+};
+
+&qcom_pinmux {
+ buttons_pins: buttons_pins {
+ mux {
+ pins = "gpio66";
+ drive-strength = <16>;
+ bias-disable;
+ };
+ };
+
+ leds_pins: leds_pins {
+ mux {
+ pins = "gpio33";
+ drive-strength = <16>;
+ bias-disable;
+ };
+ };
+
+ mdio0_pins: mdio0_pins {
+ mux {
+ pins = "gpio0", "gpio1";
+ function = "gpio";
+ drive-strength = <8>;
+ bias-disable;
+ };
+ };
+
+ mdio1_pins: mdio1_pins {
+ mux {
+ pins = "gpio10", "gpio11";
+ function = "gpio";
+ drive-strength = <8>;
+ bias-disable;
+ };
+ };
+
+ sw0_reset_pin: sw0_reset_pin {
+ mux {
+ pins = "gpio16";
+ drive-strength = <16>;
+ function = "gpio";
+ bias-disable;
+ input-disable;
+ };
+ };
+
+ sw1_reset_pin: sw1_reset_pin {
+ mux {
+ pins = "gpio17";
+ drive-strength = <16>;
+ function = "gpio";
+ bias-disable;
+ input-disable;
+ };
+ };
+};
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v5 0/6] drm: Prepare to use a GPIO on ti-sn65dsi86 for Hot Plug Detect
From: Doug Anderson @ 2020-05-18 18:05 UTC (permalink / raw)
To: Sam Ravnborg
Cc: LinusW, Stephen Boyd, Laurent Pinchart, Neil Armstrong,
David Airlie, Sandeep Panda, dri-devel, Bjorn Andersson,
Andrzej Hajda, Thierry Reding, Rob Clark, Jeffrey Hugo,
Krzysztof Kozlowski, Bartosz Golaszewski, Andy Gross,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jonas Karlman, linux-arm-msm, open list:GPIO SUBSYSTEM,
Rob Herring, Paul Walmsley, Jernej Skrabec, Stephen Boyd, LKML
In-Reply-To: <20200518175939.GA770425@ravnborg.org>
Sam,
On Mon, May 18, 2020 at 10:59 AM Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Douglas.
>
> > > 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?
> Applied, including the small fix pointed out by Linus.
Thanks! Ugh, I just realized what the problem was. I posted a v6
with this fix but insanely somehow didn't CC you (!!!). It was here:
https://lore.kernel.org/r/20200513215902.261547-1-dianders@chromium.org
I'm super sorry about that and thanks for fixing the nit. That was
the only difference between v5 and v6. I just checked what you pushed
and it looks great, thank you.
-Doug
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: sc16is7xx: Add flag to activate IrDA mode
From: Rob Herring @ 2020-05-18 18:08 UTC (permalink / raw)
To: Daniel Mack; +Cc: devicetree, linux-serial, gregkh, jslaby, pascal.huerst
In-Reply-To: <20200508143757.2609740-2-daniel@zonque.org>
On Fri, May 08, 2020 at 04:37:54PM +0200, Daniel Mack wrote:
> From: Pascal Huerst <pascal.huerst@gmail.com>
>
> This series of uart controllers is able to work in IrDA mode.
> This adds a flag to the device tree to enable that feature if needed.
>
> Signed-off-by: Pascal Huerst <pascal.huerst@gmail.com>
> ---
> Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> index c1091a923a89..9317c320c82c 100644
> --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> @@ -21,6 +21,8 @@ Optional properties:
> the second cell is used to specify the GPIO polarity:
> 0 = active high,
> 1 = active low.
> +- linux,irda-mode-port-0: Use IrDA mode on port #0
> +- linux,irda-mode-port-1: Use IrDA mode on port #1 (for dual-port devices only)
>
> Example:
> sc16is750: sc16is750@51 {
> @@ -55,6 +57,8 @@ Optional properties:
> the second cell is used to specify the GPIO polarity:
> 0 = active high,
> 1 = active low.
> +- linux,irda-mode-port-0: Use IrDA mode on port #0
> +- linux,irda-mode-port-1: Use IrDA mode on port #1 (for dual-port devices only)
How about an array of 1 or 2 entries instead?
>
> Example:
> sc16is750: sc16is750@0 {
> --
> 2.26.2
>
^ permalink raw reply
* Re: [PATCH 3/3] iio: magnetometer: ak8975: Add gpio reset support
From: Andy Shevchenko @ 2020-05-18 17:51 UTC (permalink / raw)
To: Jonathan Albrieux
Cc: Linux Kernel Mailing List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Greg Kroah-Hartman, Hartmut Knaack, Jonathan Cameron,
Lars-Peter Clausen, Linus Walleij,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Steve Winslow, Thomas Gleixner, Jonathan Cameron
In-Reply-To: <20200518174335.GA6392@jonathan-N53SV>
On Mon, May 18, 2020 at 8:43 PM Jonathan Albrieux
<jonathan.albrieux@gmail.com> wrote:
>
> On Mon, May 18, 2020 at 07:43:17PM +0300, Andy Shevchenko wrote:
> > On Mon, May 18, 2020 at 06:01:20PM +0200, Jonathan Albrieux wrote:
> > > On Mon, May 18, 2020 at 05:55:51PM +0300, Andy Shevchenko wrote:
> > > > On Mon, May 18, 2020 at 4:38 PM Jonathan Albrieux
> > > > <jonathan.albrieux@gmail.com> wrote:
> > > >
> > > > > + gpiod_set_value_cansleep(data->reset_gpiod, 1);
> > > >
> > > > (1)
> > > >
> > > > ...
> > > >
> > > > > + /*
> > > > > + * If reset pin is provided then will be set to high on power on
> > > > > + * and to low on power off according to AK09911 datasheet
> > > > > + */
> > > >
> > > > Wording is confusing, perhaps you have to use 'asserted / deasserted'.
> > >
> > > Thank you for the suggestion, I'll be working on rewording as soon as
> > > possible.
> > >
> > > > Btw, in (1) it's also "high" (asserted). I barely understand how it's
> > > > supposed to work in all cases?
> > > >
> > > > > + reset_gpiod = devm_gpiod_get_optional(&client->dev,
> > > > > + "reset", GPIOD_OUT_HIGH);
> > > > > + if (IS_ERR(reset_gpiod))
> > > > > + return PTR_ERR(reset_gpiod);
> > > >
> > >
> > > I'm sorry but I'm not sure about what you mean by saying all cases.
> > > Currently I'm testing this driver on a msm8916 device having AK09911
> > > magnetometer. At the current stage the driver is failing on probe
> > > because reset pin is not connected to VID (as datasheet requires in case
> > > of pin not being used). In case of reset pin not asserted, register's
> > > reset is triggered resulting in empty registers, leading to probe fail.
> > > For this reason pin is asserted during power on in order to have
> > > informations in registers and deasserted before power off triggering
> > > a reset.
> > >
> > > A workaround that gets AK09911 working on device is by setting the
> > > reset pin always high on device tree. This way registers gets reset by
> > > a Power On Reset circuit autonomously and reset pin never triggers the
> > > reset.
> >
> > You need to distinguish electrical level from logical (GPIO flag defines
> > logical). So, I'm talking about active-high vs. active-low case.
> >
> > Now I re-read above, and see that here you assert the reset signal. But where
> > is desertion?
>
> Oh I see, I'll try explaining by points the proposed approach:
> - reset pin is active low
> - during power on gpio is set to 0 so the reset pin is high, thus no reset
deasserted
> - during power off gpio is set to 1 so the reset pin becomes low, thus resetting
asserted
> this is a possible solution but maybe there are other ways to achieve that,
> do you have suggestions on how to get a better approach for solving this issue?
I see now, that at requesting reset you wanted to chip be in reset
state (asserted) till driver calls power_on().
Seems everything you done is correct. Just correct terminology, please.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 3/3] iio: magnetometer: ak8975: Add gpio reset support
From: Jonathan Albrieux @ 2020-05-18 17:43 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linux Kernel Mailing List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Greg Kroah-Hartman, Hartmut Knaack, Jonathan Cameron,
Lars-Peter Clausen, Linus Walleij,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Steve Winslow, Thomas Gleixner, Jonathan Cameron
In-Reply-To: <20200518164317.GL1634618@smile.fi.intel.com>
On Mon, May 18, 2020 at 07:43:17PM +0300, Andy Shevchenko wrote:
> On Mon, May 18, 2020 at 06:01:20PM +0200, Jonathan Albrieux wrote:
> > On Mon, May 18, 2020 at 05:55:51PM +0300, Andy Shevchenko wrote:
> > > On Mon, May 18, 2020 at 4:38 PM Jonathan Albrieux
> > > <jonathan.albrieux@gmail.com> wrote:
> > >
> > > > + gpiod_set_value_cansleep(data->reset_gpiod, 1);
> > >
> > > (1)
> > >
> > > ...
> > >
> > > > + /*
> > > > + * If reset pin is provided then will be set to high on power on
> > > > + * and to low on power off according to AK09911 datasheet
> > > > + */
> > >
> > > Wording is confusing, perhaps you have to use 'asserted / deasserted'.
> >
> > Thank you for the suggestion, I'll be working on rewording as soon as
> > possible.
> >
> > > Btw, in (1) it's also "high" (asserted). I barely understand how it's
> > > supposed to work in all cases?
> > >
> > > > + reset_gpiod = devm_gpiod_get_optional(&client->dev,
> > > > + "reset", GPIOD_OUT_HIGH);
> > > > + if (IS_ERR(reset_gpiod))
> > > > + return PTR_ERR(reset_gpiod);
> > >
> >
> > I'm sorry but I'm not sure about what you mean by saying all cases.
> > Currently I'm testing this driver on a msm8916 device having AK09911
> > magnetometer. At the current stage the driver is failing on probe
> > because reset pin is not connected to VID (as datasheet requires in case
> > of pin not being used). In case of reset pin not asserted, register's
> > reset is triggered resulting in empty registers, leading to probe fail.
> > For this reason pin is asserted during power on in order to have
> > informations in registers and deasserted before power off triggering
> > a reset.
> >
> > A workaround that gets AK09911 working on device is by setting the
> > reset pin always high on device tree. This way registers gets reset by
> > a Power On Reset circuit autonomously and reset pin never triggers the
> > reset.
>
> You need to distinguish electrical level from logical (GPIO flag defines
> logical). So, I'm talking about active-high vs. active-low case.
>
> Now I re-read above, and see that here you assert the reset signal. But where
> is desertion?
Oh I see, I'll try explaining by points the proposed approach:
- reset pin is active low
- during power on gpio is set to 0 so the reset pin is high, thus no reset
- during power off gpio is set to 1 so the reset pin becomes low, thus resetting
this is a possible solution but maybe there are other ways to achieve that,
do you have suggestions on how to get a better approach for solving this issue?
> --
> With Best Regards,
> Andy Shevchenko
>
>
Best regards,
Jonathan Albrieux
^ permalink raw reply
* Re: [PATCH v2 2/6] dt-bindings: dma: dw: Add max burst transaction length property
From: Rob Herring @ 2020-05-18 17:30 UTC (permalink / raw)
To: Serge Semin
Cc: Vinod Koul, Serge Semin, Andy Shevchenko, Viresh Kumar,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Dan Williams, linux-mips, dmaengine, devicetree,
linux-kernel
In-Reply-To: <20200517174739.uis3wfievdcmtsxj@mobilestation>
On Sun, May 17, 2020 at 08:47:39PM +0300, Serge Semin wrote:
> On Fri, May 15, 2020 at 02:11:13PM +0300, Serge Semin wrote:
> > On Fri, May 15, 2020 at 04:26:58PM +0530, Vinod Koul wrote:
> > > On 15-05-20, 13:51, Andy Shevchenko wrote:
> > > > On Fri, May 15, 2020 at 11:39:11AM +0530, Vinod Koul wrote:
> > > > > On 12-05-20, 15:38, Andy Shevchenko wrote:
> > > > > > On Tue, May 12, 2020 at 02:49:46PM +0300, Serge Semin wrote:
> > > > > > > On Tue, May 12, 2020 at 12:08:04PM +0300, Andy Shevchenko wrote:
> > > > > > > > On Tue, May 12, 2020 at 12:35:31AM +0300, Serge Semin wrote:
> > > > > > > > > On Tue, May 12, 2020 at 12:01:38AM +0300, Andy Shevchenko wrote:
> > > > > > > > > > On Mon, May 11, 2020 at 11:05:28PM +0300, Serge Semin wrote:
> > > > > > > > > > > On Fri, May 08, 2020 at 02:12:42PM +0300, Andy Shevchenko wrote:
> > > > > > > > > > > > On Fri, May 08, 2020 at 01:53:00PM +0300, Serge Semin wrote:
> > > >
> > > > ...
> > > >
> > > > > > I leave it to Rob and Vinod.
> > > > > > It won't break our case, so, feel free with your approach.
> > > > >
> > > > > I agree the DT is about describing the hardware and looks like value of
> > > > > 1 is not allowed. If allowed it should be added..
> > > >
> > > > It's allowed at *run time*, it's illegal in *pre-silicon stage* when
> > > > synthesizing the IP.
> > >
> > > Then it should be added ..
> >
> > Vinod, max-burst-len is "MAXimum" burst length not "run-time or current or any
> > other" burst length. It's a constant defined at the IP-core synthesis stage and
> > according to the Data Book, MAX burst length can't be 1. The allowed values are
> > exactly as I described in the binding [4, 8, 16, 32, ...]. MAX burst length
> > defines the upper limit of the run-time burst length. So setting it to 1 isn't
> > about describing a hardware, but using DT for the software convenience.
> >
> > -Sergey
>
> Vinod, to make this completely clear. According to the DW DMAC data book:
> - In general, run-time parameter of the DMA transaction burst length (set in
> the SRC_MSIZE/DST_MSIZE fields of the channel control register) may belong
> to the set [1, 4, 8, 16, 32, 64, 128, 256].
> - Actual upper limit of the burst length run-time parameter is limited by a
> constant defined at the IP-synthesize stage (it's called DMAH_CHx_MAX_MULT_SIZE)
> and this constant belongs to the set [4, 8, 16, 32, 64, 128, 256]. (See, no 1
> in this set).
>
> So the run-time burst length in a case of particular DW DMA controller belongs
> to the range:
> 1 <= SRC_MSIZE <= DMAH_CHx_MAX_MULT_SIZE
> and
> 1 <= DST_MSIZE <= DMAH_CHx_MAX_MULT_SIZE
>
> See. No mater which DW DMA controller we get each of them will at least support
> the burst length of 1 and 4 transfer words. This is determined by design of the
> DW DMA controller IP since DMAH_CHx_MAX_MULT_SIZE constant set starts with 4.
>
> In this patch I suggest to add the max-burst-len property, which specifies
> the upper limit for the run-time burst length. Since the maximum burst length
> capable to be set to the SRC_MSIZE/DST_MSIZE fields of the DMA channel control
> register is determined by the DMAH_CHx_MAX_MULT_SIZE constant (which can't be 1
> by the DW DMA IP design), max-burst-len property as being also responsible for
> the maximum burst length setting should be associated with DMAH_CHx_MAX_MULT_SIZE
> thus should belong to the same set [4, 8, 16, 32, 64, 128, 256].
>
> So 1 shouldn't be in the enum of the max-burst-len property constraint, because
> hardware doesn't support such limitation by design, while setting 1 as
> max-burst-len would mean incorrect description of the DMA controller.
>
> Vinod, could you take a look at the info I provided above and say your final word
> whether 1 should be really allowed to be in the max-burst-len enum constraints?
> I'll do as you say in the next version of the patchset.
I generally think the synthesis time IP configuration should be implied
by the compatible string which is why we have SoC specific compatible
strings (Of course I dream for IP vendors to make all that discoverable
which is only occasionally the case). There are exceptions to this. If
one SoC has the same IP configured in different ways, then we'd probably
have properties for the differences.
As to whether h/w configuration is okay in DT, the answer is yes. The
question is whether it is determined by SoC, board, OS and also who
would set it and how often. Something tuned per board and independent of
the OS/user is the ideal example.
Rob
^ permalink raw reply
* Re: [PATCHv3 00/14] Update omaps to use drivers/clocksource timers
From: Daniel Lezcano @ 2020-05-18 17:01 UTC (permalink / raw)
To: Tony Lindgren
Cc: Thomas Gleixner, linux-kernel, linux-omap, linux-arm-kernel,
H. Nikolaus Schaller, Aaro Koskinen, Adam Ford, Andreas Kemnade,
Brian Hutchinson, Graeme Smecher, Grygorii Strashko, Keerthy,
Lokesh Vutla, Michael Turquette, Rob Herring, Stephen Boyd,
Tero Kristo, devicetree, linux-clk
In-Reply-To: <20200518170059.GR37466@atomide.com>
On 18/05/2020 19:00, Tony Lindgren wrote:
> * Daniel Lezcano <daniel.lezcano@linaro.org> [200518 16:59]:
>>
>> Hi Tony,
>>
>> On 07/05/2020 19:23, Tony Lindgren wrote:
>>> Hi all,
>>>
>>> Here's v3 series to udpate omaps to use drivers/clocksource timers for
>>> the 32k counter and dmtimer, and to remove the old legacy platform code.
>>> Please review and test.
>>>
>>> I've updated the timer-ti-dm-systimer.c patch based on the comments from
>>> Daniel and Rob, and added support for selecting the preferred timers to
>>> use.
>>>
>>> Then for merging when folks are happy with this series, Daniel if you
>>> can please apply the first three patches into an immutable branch it
>>> would be great.
>>
>> so the clk patch also ?
>
> Oops sorry, just first two are needed as we no longer need the
> device tree binding changes :)
Ok
--
<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: [PATCHv3 00/14] Update omaps to use drivers/clocksource timers
From: Tony Lindgren @ 2020-05-18 17:00 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Thomas Gleixner, linux-kernel, linux-omap, linux-arm-kernel,
H. Nikolaus Schaller, Aaro Koskinen, Adam Ford, Andreas Kemnade,
Brian Hutchinson, Graeme Smecher, Grygorii Strashko, Keerthy,
Lokesh Vutla, Michael Turquette, Rob Herring, Stephen Boyd,
Tero Kristo, devicetree, linux-clk
In-Reply-To: <340cdd4a-d996-073f-127d-89ce6c05f221@linaro.org>
* Daniel Lezcano <daniel.lezcano@linaro.org> [200518 16:59]:
>
> Hi Tony,
>
> On 07/05/2020 19:23, Tony Lindgren wrote:
> > Hi all,
> >
> > Here's v3 series to udpate omaps to use drivers/clocksource timers for
> > the 32k counter and dmtimer, and to remove the old legacy platform code.
> > Please review and test.
> >
> > I've updated the timer-ti-dm-systimer.c patch based on the comments from
> > Daniel and Rob, and added support for selecting the preferred timers to
> > use.
> >
> > Then for merging when folks are happy with this series, Daniel if you
> > can please apply the first three patches into an immutable branch it
> > would be great.
>
> so the clk patch also ?
Oops sorry, just first two are needed as we no longer need the
device tree binding changes :)
Regards,
Tony
^ permalink raw reply
* Re: [PATCHv3 00/14] Update omaps to use drivers/clocksource timers
From: Daniel Lezcano @ 2020-05-18 16:58 UTC (permalink / raw)
To: Tony Lindgren, Thomas Gleixner
Cc: linux-kernel, linux-omap, linux-arm-kernel, H. Nikolaus Schaller,
Aaro Koskinen, Adam Ford, Andreas Kemnade, Brian Hutchinson,
Graeme Smecher, Grygorii Strashko, Keerthy, Lokesh Vutla,
Michael Turquette, Rob Herring, Stephen Boyd, Tero Kristo,
devicetree, linux-clk
In-Reply-To: <20200507172330.18679-1-tony@atomide.com>
Hi Tony,
On 07/05/2020 19:23, Tony Lindgren wrote:
> Hi all,
>
> Here's v3 series to udpate omaps to use drivers/clocksource timers for
> the 32k counter and dmtimer, and to remove the old legacy platform code.
> Please review and test.
>
> I've updated the timer-ti-dm-systimer.c patch based on the comments from
> Daniel and Rob, and added support for selecting the preferred timers to
> use.
>
> Then for merging when folks are happy with this series, Daniel if you
> can please apply the first three patches into an immutable branch it
> would be great.
so the clk patch also ?
[ ... ]
--
<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 4/4] sc16is7xx: Use threaded IRQ
From: Daniel Mack @ 2020-05-18 16:57 UTC (permalink / raw)
To: Maarten Brock
Cc: devicetree, linux-serial, gregkh, robh+dt, jslaby, pascal.huerst,
linux-serial-owner
In-Reply-To: <dfafc770e7e308cb6a2db5a1003cd759@vanmierlo.com>
Hi Maarten,
On 5/18/20 1:14 PM, Maarten Brock wrote:
> On 2020-05-17 22:44, Daniel Mack wrote:
>>> Therefor I suggest to change IRQF_TRIGGER_FALLING to
>>> IRQF_TRIGGER_LOW. This
>>> way the thread will be retriggered after IRQ_HANDLED is returned.
>>
>> This doesn't work in my setup unfortunately, as the interrupt controller
>> is incapable of handling level IRQs.
>
> That sounds like a lousy interrupt controller to me.
While that is true, there are many such controllers around.
> Summerizing:
> - After switching to a threaded IRQ, the trigger could be switched to
> IRQF_TRIGGER_LOW and with that interrupt sharing can be enabled for
> this device with IRQF_SHARED.
Yes, but we don't need that. As discussed, the UART driver can cope with
edge IRQs just fine.
> - Some (your) interrupt controllers do not support IRQF_TRIGGER_LOW.
> For those only IRQF_TRIGGER_FALLING can be used for this device and
> thus IRQF_SHARED cannot be used.
True. Interrupts cannot be shared for this device then. That's a fair
limitation, and it has always been like that.
> - The driver for your interrupt controller should be improved to support
> level IRQs.
It's a controller that sits behind another hardware bus itself, so
polling is expensive. If the controller would need to check for level
IRQs it would need to poll, and then we could as well just poll the UART
directly, that's just as good :)
But again - the UART driver works perfectly fine with edge IRQs as long
as the interrupt is not shared.
> This makes me wonder if it would be better to let the device tree specify
> the interrupt configuration.
There can be flags in the 2nd cell of the node, but their meaning is
specific to the controller. Hence the SPI/I2C layers don't pass that
information up.
What many drivers do is try with one setting, and if that fails because
the interrupt controller returns an error, they fall back to something
else. We could do the same here of course, but it'd be another patch on
top, as it's unrelated to the concrete change the patch we're commenting
on is bringing in.
So what I can add is logic that first tries with IRQF_LOW|IRQF_SHARED,
and if that fails, we fall back to IRQF_FALLING and retry. WDYT?
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH 3/3] iio: magnetometer: ak8975: Add gpio reset support
From: Andy Shevchenko @ 2020-05-18 16:43 UTC (permalink / raw)
To: Jonathan Albrieux
Cc: Linux Kernel Mailing List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Greg Kroah-Hartman, Hartmut Knaack, Jonathan Cameron,
Lars-Peter Clausen, Linus Walleij,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Steve Winslow, Thomas Gleixner, Jonathan Cameron
In-Reply-To: <20200518160120.GA21361@ict14-OptiPlex-980>
On Mon, May 18, 2020 at 06:01:20PM +0200, Jonathan Albrieux wrote:
> On Mon, May 18, 2020 at 05:55:51PM +0300, Andy Shevchenko wrote:
> > On Mon, May 18, 2020 at 4:38 PM Jonathan Albrieux
> > <jonathan.albrieux@gmail.com> wrote:
> >
> > > + gpiod_set_value_cansleep(data->reset_gpiod, 1);
> >
> > (1)
> >
> > ...
> >
> > > + /*
> > > + * If reset pin is provided then will be set to high on power on
> > > + * and to low on power off according to AK09911 datasheet
> > > + */
> >
> > Wording is confusing, perhaps you have to use 'asserted / deasserted'.
>
> Thank you for the suggestion, I'll be working on rewording as soon as
> possible.
>
> > Btw, in (1) it's also "high" (asserted). I barely understand how it's
> > supposed to work in all cases?
> >
> > > + reset_gpiod = devm_gpiod_get_optional(&client->dev,
> > > + "reset", GPIOD_OUT_HIGH);
> > > + if (IS_ERR(reset_gpiod))
> > > + return PTR_ERR(reset_gpiod);
> >
>
> I'm sorry but I'm not sure about what you mean by saying all cases.
> Currently I'm testing this driver on a msm8916 device having AK09911
> magnetometer. At the current stage the driver is failing on probe
> because reset pin is not connected to VID (as datasheet requires in case
> of pin not being used). In case of reset pin not asserted, register's
> reset is triggered resulting in empty registers, leading to probe fail.
> For this reason pin is asserted during power on in order to have
> informations in registers and deasserted before power off triggering
> a reset.
>
> A workaround that gets AK09911 working on device is by setting the
> reset pin always high on device tree. This way registers gets reset by
> a Power On Reset circuit autonomously and reset pin never triggers the
> reset.
You need to distinguish electrical level from logical (GPIO flag defines
logical). So, I'm talking about active-high vs. active-low case.
Now I re-read above, and see that here you assert the reset signal. But where
is desertion?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled
From: Thomas Bogendoerfer @ 2020-05-18 16:32 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
Greg Kroah-Hartman, Arnd Bergmann, Rob Herring, linux-pm,
devicetree, Vincenzo Frascino, Thomas Gleixner, linux-mips,
linux-kernel
In-Reply-To: <20200518134820.wedoumgbsllvhem6@mobilestation>
On Mon, May 18, 2020 at 04:48:20PM +0300, Serge Semin wrote:
> On Fri, May 15, 2020 at 11:06:47PM +0200, Thomas Bogendoerfer wrote:
> > On Fri, May 15, 2020 at 10:48:27AM +0300, Serge Semin wrote:
> > > Thomas,
> > > Could you take a look at my comment below so I could proceed with the
> > > patchset v3 development?
> >
> > I can't help, but using r4k clocksource with changing frequency is
> > probaly only usefull as a random generator. So IMHO the only two
> > options are disabling it or implement what arch/x86/kernel/tsc.c does.
> >
> > Thomas.
>
> Thomas, could you proceed with the rest of the patches review?
> ├─>[PATCH v2 16/20] bus: cdmm: Add MIPS R5 arch support
> ├─>[PATCH v2 15/20] mips: cdmm: Add mti,mips-cdmm dtb node support
both are not my call, but look ok to me.
> ├─>[PATCH v2 13/20] mips: early_printk_8250: Use offset-sized IO-mem accessors
that's broken. A reg shift of 2 doesn't mean we could use 32bit access
to the registers on other platforms. As I don't think adding some ifdefery
makes things nicer, just implement the your prom_putchar in board code.
> ├─>[PATCH v2 12/20] mips: MAAR: Add XPA mode support
looks ok so far.
> ├─>[PATCH v2 10/20] mips: Add CONFIG/CONFIG6/Cause reg fields macro
that is fine
> └─>[PATCH v2 09/20] mips: Add CP0 Write Merge config support
this is IMHO a dangerous change. Enabling write merging for any
CPU supporting it might triggers bugs. Do it in your board bringup
code and at the moment I don't see a reason for the rest of that
patch.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v5 5/6] iio: imu: Add support for adis16475
From: Jonathan Cameron @ 2020-05-18 16:29 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Jonathan Cameron, Nuno Sá, Nuno Sá, linux-iio,
devicetree, Hartmut Knaack, Peter Meerwald-Stadler, Rob Herring,
Mark Rutland, Alexandru Ardelean, Michael Hennerich
In-Reply-To: <3136c4c7-b964-dce4-9e43-4cbbcc9187a1@metafoo.de>
On Sun, 17 May 2020 21:07:45 +0200
Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 5/17/20 6:15 PM, Jonathan Cameron wrote:
> > On Sun, 3 May 2020 11:12:34 +0200
> > Lars-Peter Clausen <lars@metafoo.de> wrote:
> >
> >> On 5/3/20 11:07 AM, Lars-Peter Clausen wrote:
> >>> On 5/3/20 10:47 AM, Jonathan Cameron wrote:
> >>>> On Sat, 02 May 2020 21:52:18 +0200
> >>>> Nuno Sá <noname.nuno@gmail.com> wrote:
> >>>>
> >>>>> On Sat, 2020-05-02 at 20:01 +0200, Lars-Peter Clausen wrote:
> >>>>>> On 5/2/20 7:40 PM, Jonathan Cameron wrote:
> >>>>>>> On Mon, 27 Apr 2020 20:06:07 +0200
> >>>>>>> Lars-Peter Clausen <lars@metafoo.de> wrote:
> >>>>>>>> On 4/13/20 10:24 AM, Nuno Sá wrote:
> >>>>>>>>> [...]
> >>>>>>>>> +static irqreturn_t adis16475_trigger_handler(int irq, void *p)
> >>>>>>>>> +{
> >>>>>>>>> [...]
> >>>>>>>>> + __be16 data[ADIS16475_MAX_SCAN_DATA], *buffer;
> >>>>>>>>> [...]
> >>>>>>>>> +
> >>>>>>>>> + iio_push_to_buffers_with_timestamp(indio_dev, data, pf-
> >>>>>>>>>> timestamp);
> >>>>>>>> If the timestamp is enabled the IIO core might insert padding
> >>>>>>>> between
> >>>>>>>> the data channels and the timestamp. If that happens this will
> >>>>>>>> disclose
> >>>>>>>> kernel stack memory to userspace.
> >>>>>>>>
> >>>>>>>> This needs either a memset(data, 0x00, sizeof(data)) or maybe put
> >>>>>>>> data
> >>>>>>>> into the state struct and kzalloc it.
> >>>>>>> Good spot. Could simply do __be16 data[ADI..] = {0}; rather than
> >>>>>>> explicit
> >>>>>>> memset, but some form of zeroization is needed.
> >>>>>>>
> >>>>>>> I've fixed up the applied patch with the above approach.
> >>>>>> There is actually another issue. The stack data is not necessarily
> >>>>>> aligned to 64 bit, which causes issues if we try to put the 64-bit
> >>>>> Oh, this is actually more problematic. Yes, since we have an array of
> >>>>> u16, that is not guaranteed to be 64bit aligned. Doing a quick search
> >>>>> of `iio_push_to_buffers_with_timestamp()` users and I could quickly
> >>>>> find 4/5 drivers with the same problem. I guess the API should clearly
> >>>>> state that `data` needs to be __at least__ 64 bits aligned (maybe a
> >>>>> future patch). Or we could even check the address and guarantee that it
> >>>>> is properly aligned before continuing (though Im guessing this will
> >>>>> break a lot of users...)
> >>>>>> timestamp in it. I think data should really be in the state struct.
> >>>>> Yes, with a proper __aligned(8) attribute... Or couldn't we just use
> >>>>> __aligned(8) on the stack variable?
> >>>> Forcing alignment on the stack isn't terribly reliable, which is why
> >>>> we never do that for dma safe buffers.
> >>>>
> >>>> Probably better to just move it to the state structure.
> >>>> I'll fix it up to do that. Please sanity check what will shortly
> >>>> be in the testing branch.
> >>>>
> >>>> The moment Lars mentioned this I groaned. As you've noted a few other
> >>>> drivers have the same problem + the ABI doesn't clearly state
> >>>> or check this.
> >>>>
> >>>> We should certainly fix all the drivers that suffer this problem
> >>>> first then we can think about adding a runtime check.
> >>> It looks like it is actually quite a few drivers, maybe we should
> >>> switch to put_unaligned(). We probably got lucky in most cases and the
> >>> buffer is naturally aligned to 64 bit.
> > Just a quick update on this. I've been taking a deeper look and there
> > are some 'interesting' cases in here so the put_unaligned is attractive
> > unfortunately I don't think we can go that way because it would be
> > reasonable for consumers of the buffer to expect it to be appropriately
> > aligned. We need to rework many of these anyway to fix the related
> > data leak.
> >
> > I've done some of below and will post shortly - a few will take more
> > effort and probably need testing rather than just relying on review.
> >
> > So far the 'interesting ones' are mpu3050 and isl29501.
>
> isl29501 looks OK to me. mpu3050 is clearly broken, buffer is both
> unaligned and too small!
isl29501 reads a pair of 8 bit registers, then writes them to a 32 bit value
for no particular reason and puts that in the first 32 bits of the buffer
having declared the channel to have a storage size of 16 bits.
So going to have some fun on one or other of the endian types...
J
>
^ permalink raw reply
* Re: [Freedreno] [PATCH 5/6] drm: msm: a6xx: use dev_pm_opp_set_bw to set DDR bandwidth
From: Rob Clark @ 2020-05-18 16:25 UTC (permalink / raw)
To: Sharat Masetty, freedreno,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
dri-devel, linux-arm-msm, Linux Kernel Mailing List,
Georgi Djakov, Matthias Kaehlcke
In-Reply-To: <20200518142333.GA10796@jcrouse1-lnx.qualcomm.com>
On Mon, May 18, 2020 at 7:23 AM Jordan Crouse <jcrouse@codeaurora.org> wrote:
>
> 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.
>
it looks like we could maybe do something like:
ret = dev_pm_opp_set_bw(...);
if (ret) {
dev_warn_once(dev, "no bandwidth settings");
icc_set_bw(...);
}
?
BR,
-R
> 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
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply
* Re: [PATCH] dt-bindings: clock: renesas: mstp: Convert to json-schema
From: Rob Herring @ 2020-05-18 16:18 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Michael Turquette, Stephen Boyd, devicetree, linux-clk,
linux-renesas-soc
In-Reply-To: <20200508100321.6720-1-geert+renesas@glider.be>
On Fri, May 08, 2020 at 12:03:21PM +0200, Geert Uytterhoeven wrote:
> Convert the Renesas Clock Pulse Generator (CPG) Module Stop (MSTP)
> Clocks Device Tree binding documentation to json-schema.
>
> Drop R-Car Gen2 compatible values, which were obsoleted by the unified
> "Renesas Clock Pulse Generator / Module Standby and Software Reset" DT
> bindings.
> Replace the obsolete example for R-Car H2 by an example that is still
> valid.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> To be queued in clk-renesas-for-v5.8.
> ---
> .../clock/renesas,cpg-mstp-clocks.txt | 60 --------------
> .../clock/renesas,cpg-mstp-clocks.yaml | 82 +++++++++++++++++++
> 2 files changed, 82 insertions(+), 60 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
> create mode 100644 Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.yaml
>
> diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
> deleted file mode 100644
> index da578ebdda2889fc..0000000000000000
> --- a/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
> +++ /dev/null
> @@ -1,60 +0,0 @@
> -* Renesas CPG Module Stop (MSTP) Clocks
> -
> -The CPG can gate SoC device clocks. The gates are organized in groups of up to
> -32 gates.
> -
> -This device tree binding describes a single 32 gate clocks group per node.
> -Clocks are referenced by user nodes by the MSTP node phandle and the clock
> -index in the group, from 0 to 31.
> -
> -Required Properties:
> -
> - - compatible: Must be one of the following
> - - "renesas,r7s72100-mstp-clocks" for R7S72100 (RZ) MSTP gate clocks
> - - "renesas,r8a73a4-mstp-clocks" for R8A73A4 (R-Mobile APE6) MSTP gate clocks
> - - "renesas,r8a7740-mstp-clocks" for R8A7740 (R-Mobile A1) MSTP gate clocks
> - - "renesas,r8a7778-mstp-clocks" for R8A7778 (R-Car M1) MSTP gate clocks
> - - "renesas,r8a7779-mstp-clocks" for R8A7779 (R-Car H1) MSTP gate clocks
> - - "renesas,r8a7790-mstp-clocks" for R8A7790 (R-Car H2) MSTP gate clocks
> - - "renesas,r8a7791-mstp-clocks" for R8A7791 (R-Car M2-W) MSTP gate clocks
> - - "renesas,r8a7792-mstp-clocks" for R8A7792 (R-Car V2H) MSTP gate clocks
> - - "renesas,r8a7793-mstp-clocks" for R8A7793 (R-Car M2-N) MSTP gate clocks
> - - "renesas,r8a7794-mstp-clocks" for R8A7794 (R-Car E2) MSTP gate clocks
> - - "renesas,sh73a0-mstp-clocks" for SH73A0 (SH-MobileAG5) MSTP gate clocks
> - and "renesas,cpg-mstp-clocks" as a fallback.
> - - reg: Base address and length of the I/O mapped registers used by the MSTP
> - clocks. The first register is the clock control register and is mandatory.
> - The second register is the clock status register and is optional when not
> - implemented in hardware.
> - - clocks: Reference to the parent clocks, one per output clock. The parents
> - must appear in the same order as the output clocks.
> - - #clock-cells: Must be 1
> - - clock-output-names: The name of the clocks as free-form strings
> - - clock-indices: Indices of the gate clocks into the group (0 to 31)
> -
> -The clocks, clock-output-names and clock-indices properties contain one entry
> -per gate clock. The MSTP groups are sparsely populated. Unimplemented gate
> -clocks must not be declared.
> -
> -
> -Example
> --------
> -
> - #include <dt-bindings/clock/r8a7790-clock.h>
> -
> - mstp3_clks: mstp3_clks@e615013c {
> - compatible = "renesas,r8a7790-mstp-clocks", "renesas,cpg-mstp-clocks";
> - reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
> - clocks = <&cp_clk>, <&mmc1_clk>, <&sd3_clk>, <&sd2_clk>,
> - <&cpg_clocks R8A7790_CLK_SD1>, <&cpg_clocks R8A7790_CLK_SD0>,
> - <&mmc0_clk>;
> - #clock-cells = <1>;
> - clock-output-names =
> - "tpu0", "mmcif1", "sdhi3", "sdhi2",
> - "sdhi1", "sdhi0", "mmcif0";
> - clock-indices = <
> - R8A7790_CLK_TPU0 R8A7790_CLK_MMCIF1 R8A7790_CLK_SDHI3
> - R8A7790_CLK_SDHI2 R8A7790_CLK_SDHI1 R8A7790_CLK_SDHI0
> - R8A7790_CLK_MMCIF0
> - >;
> - };
> diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.yaml b/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.yaml
> new file mode 100644
> index 0000000000000000..9b79e70ae5504bd7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.yaml
> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/renesas,cpg-mstp-clocks.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas Clock Pulse Generator (CPG) Module Stop (MSTP) Clocks
> +
> +maintainers:
> + - Geert Uytterhoeven <geert+renesas@glider.be>
> +
> +description:
> + The Clock Pulse Generator (CPG) can gate SoC device clocks. The gates are
> + organized in groups of up to 32 gates.
> +
> + This device tree binding describes a single 32 gate clocks group per node.
> + Clocks are referenced by user nodes by the Module Stop (MSTP) node phandle
> + and the clock index in the group, from 0 to 31.
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - renesas,r7s72100-mstp-clocks # RZ/A1
> + - renesas,r8a73a4-mstp-clocks # R-Mobile APE6
> + - renesas,r8a7740-mstp-clocks # R-Mobile A1
> + - renesas,r8a7778-mstp-clocks # R-Car M1
> + - renesas,r8a7779-mstp-clocks # R-Car H1
> + - renesas,sh73a0-mstp-clocks # SH-Mobile AG5
> + - const: renesas,cpg-mstp-clocks
> +
> + reg:
> + minItems: 1
> + items:
> + - description: Module Stop Control Register (MSTPCR)
> + - description: Module Stop Status Register (MSTPSR)
> +
> + clocks:
> + minItems: 1
> + maxItems: 32
> +
> + '#clock-cells':
> + const: 1
> +
> + clock-indices:
> + minItems: 1
> + maxItems: 32
> +
> + clock-output-names:
> + minItems: 1
> + maxItems: 32
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - '#clock-cells'
> + - clock-indices
> + - clock-output-names
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/r8a73a4-clock.h>
> + mstp2_clks: mstp2_clks@e6150138 {
> + compatible = "renesas,r8a73a4-mstp-clocks",
> + "renesas,cpg-mstp-clocks";
> + reg = <0xe6150138 0 4>, <0xe6150040 0 4>;
Size is 2 cells?
With that fixed,
Reviewed-by: Rob Herring <robh@kernel.org>
> + clocks = <&mp_clk>, <&mp_clk>, <&mp_clk>, <&mp_clk>, <&mp_clk>,
> + <&mp_clk>, <&cpg_clocks R8A73A4_CLK_HP>;
> + #clock-cells = <1>;
> + clock-indices = <
> + R8A73A4_CLK_SCIFA0 R8A73A4_CLK_SCIFA1
> + R8A73A4_CLK_SCIFB0 R8A73A4_CLK_SCIFB1
> + R8A73A4_CLK_SCIFB2 R8A73A4_CLK_SCIFB3
> + R8A73A4_CLK_DMAC
> + >;
> + clock-output-names =
> + "scifa0", "scifa1", "scifb0", "scifb1", "scifb2", "scifb3",
> + "dmac";
> + };
> --
> 2.17.1
>
^ permalink raw reply
* Re: u-boot DT configuration node
From: Michal Simek @ 2020-05-18 16:05 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree@vger.kernel.org, u-boot@lists.denx.de, Tom Rini,
Loic Poulain
In-Reply-To: <CAL_JsqLLVweHu3LE=FkZNFPFss4zqh4ByXRjVaGiXYY+q4gJjA@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 8400 bytes --]
On 18. 05. 20 17:55, Rob Herring wrote:
> On Thu, May 14, 2020 at 12:47 PM Michal Simek <monstr@monstr.eu> wrote:
>>
>> čt 14. 5. 2020 v 20:07 odesílatel Rob Herring <robh@kernel.org> napsal:
>>>
>>> On Thu, Apr 30, 2020 at 6:13 AM Michal Simek <michal.simek@xilinx.com> wrote:
>>>>
>>>> On 29. 04. 20 16:55, Rob Herring wrote:
>>>>> On Tue, Apr 28, 2020 at 8:51 AM Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>>
>>>>>> On 28. 04. 20 15:23, Rob Herring wrote:
>>>>>>> On Wed, Apr 1, 2020 at 4:23 AM Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>>>>
>>>>>>>> Hi Rob and others,
>>>>>>>>
>>>>>>>> for couple of years already u-boot is using config node in root DT for
>>>>>>>> u-boot configuration.
>>>>>>>>
>>>>>>>> Here is one example in u-boot source code.
>>>>>>>> https://gitlab.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/exynos5250-spring.dts#L47
>>>>>>>>
>>>>>>>> And here is dt binding description
>>>>>>>> https://gitlab.denx.de/u-boot/u-boot/-/blob/master/doc/device-tree-bindings/config.txt
>>>>>>>>
>>>>>>>> I was checking dt binding specification and there no such a thing
>>>>>>>> described there. It means I expect this is more adhoc u-boot solution.
>>>>>>>> We have reached the point where could be beneficial to put some u-boot
>>>>>>>> specific configurations to DT.
>>>>>>>>
>>>>>>>> Actually I have done similar thing some time ago too by using chosen
>>>>>>>> node and add xilinx specific property there to point to eeprom.
>>>>>>>> https://gitlab.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/zynqmp-zcu102-revA.dts#L39
>>>>>>>
>>>>>>> In this case, I think an alias should be used as it's more of just a
>>>>>>> shortcut to finding a specific node.
>>>>>>
>>>>>> What alias name do you suggest to use?
>>>>>> We have systems where one i2c eeprom described based board and another
>>>>>> i2c eeprom describe bootable module. And I need to have shotcuts to both
>>>>>> of them.
>>>>>>
>>>>>> dt specification doesn't list any keywords for aliases but there is
>>>>>> generic name recommendation.
>>>>>
>>>>> I do want make aliases a registered list of names.
>>>>>
>>>>>> Based on keywords it should look like this.
>>>>>> eeprom0 = ...;
>>>>>> eeprom1 = ...;
>>>>>
>>>>> That was my initial thought, but maybe "nvmemX" to be a bit more generic.
>>>>
>>>> I am fine with that. It means that multiple eeproms and order will be
>>>> direct by alias number.
>>>> In past I wanted to use list but aliases number is also fine.
>>>>
>>>>>
>>>>>
>>>>>>>> I think it is a time to discuss it and do it properly.
>>>>>>>>
>>>>>>>> First of all my question is where we could list SW prefixes to make sure
>>>>>>>> that they are listed and everybody is aware about it. We have
>>>>>>>> vendor-prefixes and we should have a way to record also prefixes for sw
>>>>>>>> projects. U-Boot is using u-boot. Xen has file in the kernel with using
>>>>>>>> xen prefix. At least these two should be listed.
>>>>>>>
>>>>>>> Documentation/devicetree/bindings/vendor-prefixes.yaml.
>>>>>>
>>>>>> thx
>>>>
>>>> Sent a patch for it. Please review.
>>>> https://lore.kernel.org/linux-devicetree/85b8dc9e6288270bbfdf55f1c156dba160293f01.1588239081.git.michal.simek@xilinx.com/
>>>>
>>>>
>>>>>>>> Next my question is what is the recommended way to pass sw specific
>>>>>>>> parameters via DT? I think using chosen node is more appropriate then
>>>>>>>> adhoc config node. Or is there a better way how this should be done?
>>>>>>>
>>>>>>> /chosen
>>>>>>>
>>>>>>> For vendor specific things though I would be cautious. If they are
>>>>>>> settings for a specific device, then they probably belong in the
>>>>>>> device's node. Second, are they really vendor specific? What we don't
>>>>>>> want is each vendor doing the same thing in slightly different ways.
>>>>>>
>>>>>> For u-boot specific setting like - offsets it should be generic for
>>>>>> everybody. I was already talking to Loic that for saving u-boot
>>>>>> variables to QSPI we should be using MTD partition map and put there
>>>>>> maybe a flag to say that this is the location for storing them.
>>>>>
>>>>> I'd standardize on the partition name.
>>>>
>>>> ok. Documentation/devicetree/bindings/mtd/partition.txt?
>>>>
>>>> I have grep u-boot repo and I see these label names
>>>>
>>>> "NAND.u-boot";
>>>> "NAND.u-boot-env";
>>>> "NAND.u-boot-env.backup1";
>>>> "NAND.u-boot-spl-os";
>>>> "QSPI.u-boot";
>>>> "QSPI.u-boot-env";
>>>> "QSPI.u-boot-env.backup1";
>>>> "qspi-u-boot-img";
>>>> "qspi-u-boot-spl";
>>>> "QSPI.u-boot-spl-os";
>>>> "u-boot
>>>> "u-boot";
>>>> "u-boot-2";
>>>> "u-boot-2.backup1";
>>>> "u-boot.backup1";
>>>> "u-boot-env";
>>>> "u-boot-env.backup1";
>>>> "u-boot-spl";
>>>>
>>>> kernel is kind of similar
>>>> "alt-u-boot";
>>>> "alt-u-boot-env";
>>>> "NAND.u-boot";
>>>> "NAND.u-boot-env";
>>>> "NAND.u-boot-env.backup1";
>>>> "NAND.u-boot-spl-os";
>>>> "QSPI.u-boot";
>>>> "QSPI.u-boot-env";
>>>> "QSPI.u-boot-env.backup1";
>>>> "QSPI.u-boot-spl-os";
>>>> "u-boot
>>>> "u-boot";
>>>> "u-boot.backup1";
>>>> "u-boot-env";
>>>> "u-boot-env2";
>>>> "u-boot-env.backup1";
>>>> "u-boot-environment";
>>>> "u-boot-factory";
>>>> "u-boot-nand";
>>>> "u-boot-nor";
>>>> "u-boot-spi";
>>>> "u-boot-spl";
>>>>
>>>> It means it is mix of names. I think SPI cases are the most complicated
>>>> one because you can have multiple spi devices in the system and you
>>>> can't use the same name for registration.
>>>>
>>>> That's why I think that make sense to use an optional prefix as people
>>>> are using QSPI/NAND already. But not quite sure that using QSPI is
>>>> generic enough because you can have multiple QSPIs. Using alias name is
>>>> also not ideal because one simple change in aliases would require
>>>> changes in partition name/label.
>>>> Any better suggestion?
>>>
>>> Okay, that's a mess of names. I guess perhaps properties in /chosen
>>> pointing to data would work. Then you just have to update that
>>> property if you're switching partitions (using SPI vs. MMC or for A/B
>>> style partition switching). We should point to partitions rather than
>>> raw offsets though.
>>
>> That means that when you deploy images this property doesn't need to be there
>> and then your firmware (in our case u-boot) can fill this property
>> based on your logic.
>> I definitely want to avoid cases where we would need to maintain
>> different DTs for
>> different mode which would bring more overhead.
>
> Not sure I follow. How would u-boot fill in properties it needs for
> itself? The properties could be populated whenever the partitions are.
> Just as stdout-path is populated when the uart node is.
If you look at all xilinx evalution boards then you can find out that at
least 3 boot modes are supported on all of them. And we use the same
boot images irrespective of bootmode selection. For example you have
zcu102 and boot image (called boot.bin). If you copy boot.bin to fat32
partition on sd, or write it to qspi, etc. It is the same image all the
time.
But doesn't make sense to save u-boot variables or look for boot script
in qspi if you boot from SD boot mode. I mean if you explicitly say that
via DT property then fine but if this property is not there u-boot
should be looking for it and bootmode should have priority.
Exactly how for example in u-boot distro boot works. You boot from sd,
than sd is your first location to look at. If you boot from qspi then
look at qspi, etc.
>
>> We should document these u-boot properties in the u-boot project for sure.
>> But there could also be the reason to do it in Linux because likely
>> these properties
>> will get to Linux dtses. Would be good to get some feedback on this.
>
> No problem with that as long as the properties are documented.
>
>> And if we should
>> document it in Linux, path and name suggestions would be welcome.
>
> /chosen already has a schema in dt-schema[1]. Add it there or add a
> chosen-u-boot.yaml.
good.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] power: supply: bq25150 introduce the bq25150
From: Andrew F. Davis @ 2020-05-18 16:05 UTC (permalink / raw)
To: Dan Murphy, sre
Cc: sspatil, linux-pm, robh, linux-kernel, devicetree,
Ricardo Rivera-Matos
In-Reply-To: <20200515193148.8502-4-dmurphy@ti.com>
On 5/15/20 3:31 PM, Dan Murphy wrote:
> From: Ricardo Rivera-Matos <r-rivera-matos@ti.com>
>
> Introduce the bq2515x family of chargers.
>
> The BQ2515X family of devices are highly integrated battery management
> ICs that integrate the most common functions for wearbale devices
s/wearbale/wearable
> namely a charger, an output voltage rail, ADC for battery and system
> monitoring, and a push-button controller.
>
> Datasheets:
> bq25150 - http://www.ti.com/lit/ds/symlink/bq25150.pdf
> bq25155 - http://www.ti.com/lit/ds/symlink/bq25155.pdf
>
> Signed-off-by: Ricardo Rivera-Matos <r-rivera-matos@ti.com>
> ---
> drivers/power/supply/Kconfig | 13 +
> drivers/power/supply/Makefile | 1 +
> drivers/power/supply/bq2515x_charger.c | 1176 ++++++++++++++++++++++++
> 3 files changed, 1190 insertions(+)
> create mode 100644 drivers/power/supply/bq2515x_charger.c
>
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index f3424fdce341..266193301e2d 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -589,6 +589,19 @@ config CHARGER_BQ24735
> help
> Say Y to enable support for the TI BQ24735 battery charger.
>
> +config CHARGER_BQ2515X
> + tristate "TI BQ2515X battery charger family"
> + depends on I2C
> + depends on GPIOLIB || COMPILE_TEST
> + select REGMAP_I2C
> + help
> + Say Y to enable support for the TI BQ2515X family of battery
> + charging integrated circuits. The BQ2515X are highly integrated
> + battery charge management ICs that integrate the most common
> + functions for wearable devices, namely a charger, an output voltage
> + rail, ADC for battery and system monitoring, and push-button
> + controller.
> +
> config CHARGER_BQ25890
> tristate "TI BQ25890 battery charger driver"
> depends on I2C
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index 6c7da920ea83..8fcc175a7e22 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -80,6 +80,7 @@ obj-$(CONFIG_CHARGER_BQ2415X) += bq2415x_charger.o
> obj-$(CONFIG_CHARGER_BQ24190) += bq24190_charger.o
> obj-$(CONFIG_CHARGER_BQ24257) += bq24257_charger.o
> obj-$(CONFIG_CHARGER_BQ24735) += bq24735-charger.o
> +obj-$(CONFIG_CHARGER_BQ2515X) += bq2515x_charger.o
> obj-$(CONFIG_CHARGER_BQ25890) += bq25890_charger.o
> obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o
> obj-$(CONFIG_CHARGER_TPS65090) += tps65090-charger.o
> diff --git a/drivers/power/supply/bq2515x_charger.c b/drivers/power/supply/bq2515x_charger.c
> new file mode 100644
> index 000000000000..78311168b171
> --- /dev/null
> +++ b/drivers/power/supply/bq2515x_charger.c
> @@ -0,0 +1,1176 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// BQ2515X Battery Charger Driver
> +// Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/
> +
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
Is this header used? Looks like the chips have an INT pin for faults but
this driver doesn't handle that yet.
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
This one can also be dropped after addressing a comment below.
> +#include <linux/gpio/consumer.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +
> +#define BQ2515X_MANUFACTURER "Texas Instruments"
> +
> +#define BQ2515X_STAT0 0x00
> +#define BQ2515X_STAT1 0x01
> +#define BQ2515X_STAT2 0x02
> +#define BQ2515X_FLAG0 0x03
> +#define BQ2515X_FLAG1 0x04
> +#define BQ2515X_FLAG2 0x05
> +#define BQ2515X_FLAG3 0x06
> +#define BQ2515X_MASK0 0x07
> +#define BQ2515X_MASK1 0x08
> +#define BQ2515X_MASK2 0x09
> +#define BQ2515X_MASK3 0x0a
> +#define BQ2515X_VBAT_CTRL 0x12
> +#define BQ2515X_ICHG_CTRL 0x13
> +#define BQ2515X_PCHRGCTRL 0x14
> +#define BQ2515X_TERMCTRL 0x15
> +#define BQ2515X_BUVLO 0x16
> +#define BQ2515X_CHARGERCTRL0 0x17
> +#define BQ2515X_CHARGERCTRL1 0x18
> +#define BQ2515X_ILIMCTRL 0x19
> +#define BQ2515X_LDOCTRL 0x1d
> +#define BQ2515X_MRCTRL 0x30
> +#define BQ2515X_ICCTRL0 0x35
> +#define BQ2515X_ICCTRL1 0x36
> +#define BQ2515X_ICCTRL2 0x37
> +#define BQ2515X_ADCCTRL0 0x40
> +#define BQ2515X_ADCCTRL1 0x41
> +#define BQ2515X_ADC_VBAT_M 0x42
> +#define BQ2515X_ADC_VBAT_L 0x43
> +#define BQ2515X_ADC_TS_M 0x44
> +#define BQ2515X_ADC_TS_L 0x45
> +#define BQ2515X_ADC_ICHG_M 0x46
> +#define BQ2515X_ADC_ICHG_L 0x47
> +#define BQ2515X_ADC_ADCIN_M 0x48
> +#define BQ2515X_ADC_ADCIN_L 0x49
> +#define BQ2515X_ADC_VIN_M 0x4a
> +#define BQ2515X_ADC_VIN_L 0x4b
> +#define BQ2515X_ADC_PMID_M 0x4c
> +#define BQ2515X_ADC_PMID_L 0x4d
> +#define BQ2515X_ADC_IIN_M 0x4e
> +#define BQ2515X_ADC_IIN_L 0x4f
> +#define BQ2515X_ADC_COMP1_M 0x52
> +#define BQ2515X_ADC_COMP1_L 0X53
> +#define BQ2515X_ADC_COMP2_M 0X54
> +#define BQ2515X_ADC_COMP2_L 0x55
> +#define BQ2515X_ADC_COMP3_M 0x56
> +#define BQ2515X_ADC_COMP3_L 0x57
> +#define BQ2515X_ADC_READ_EN 0x58
> +#define BQ2515X_TS_FASTCHGCTRL 0x61
> +#define BQ2515X_TS_COLD 0x62
> +#define BQ2515X_TS_COOL 0x63
> +#define BQ2515X_TS_WARM 0x64
> +#define BQ2515X_TS_HOT 0x65
> +#define BQ2515X_DEVICE_ID 0x6f
> +
> +#define BQ2515X_DEFAULT_ICHG_UA 10000
> +#define BQ25150_DEFAULT_ILIM_UA 100000
> +#define BQ25155_DEFAULT_ILIM_UA 500000
> +#define BQ2515X_DEFAULT_VBAT_REG_UV 4200000
> +#define BQ2515X_DEFAULT_IPRECHARGE_UA 2500
> +
> +#define BQ2515X_DIVISOR 65536
> +#define BQ2515X_VBAT_BASE_VOLT 3600000
> +#define BQ2515X_VBAT_REG_MAX 4600000
> +#define BQ2515X_VBAT_REG_MIN 3600000
> +#define BQ2515X_VBAT_STEP_UV 10000
> +#define BQ2515X_UV_FACTOR 1000000
> +#define BQ2515X_VBAT_MULTIPLIER 6
> +#define BQ2515X_ICHG_DIVISOR 52429
> +#define BQ2515X_ICHG_CURR_STEP_THRESH_UA 318750
> +#define BQ2515X_ICHG_MIN_UA 0
> +#define BQ2515X_ICHG_MAX_UA 500000
> +#define BQ2515X_ICHG_RNG_1B0_UA 1250
> +#define BQ2515X_ICHG_RNG_1B1_UA 2500
> +#define BQ2515X_VLOWV_SEL_1B0_UV 3000000
> +#define BQ2515X_VLOWV_SEL_1B1_UV 2800000
> +#define BQ2515X_PRECHRG_ICHRG_RNGE_1875_UA 18750
> +#define BQ2515X_PRECHRG_ICHRG_RNGE_3750_UA 37500
> +#define BQ2515X_TWAKE2_MIN_US 1700000
> +#define BQ2515X_TWAKE2_MAX_US 2300000
> +
> +#define BQ2515X_ILIM_150MA 0x2
> +#define BQ2515X_ILIM_MASK 0x7
> +#define BQ2515X_HEALTH_MASK 0xf
> +#define BQ2515X_ICHGRNG_MASK 0x80
> +#define BQ2515X_STAT0_MASK 0x0f
> +#define BQ2515X_STAT1_MASK 0x1f
> +#define BQ2515X_PRECHARGE_MASK 0x1f
> +
> +#define BQ2515X_TS_HOT_STAT BIT(0)
> +#define BQ2515X_TS_WARM_STAT BIT(1)
> +#define BQ2515X_TS_COOL_STAT BIT(2)
> +#define BQ2515X_TS_COLD_STAT BIT(3)
> +#define BQ2515X_SAFETY_TIMER_EXP BIT(5)
> +
> +#define BQ2515X_EN_VBAT_READ BIT(3)
> +#define BQ2515X_EN_ICHG_READ BIT(5)
> +
> +#define BQ2515X_VIN_GOOD BIT(0)
> +#define BQ2515X_CHRG_DONE BIT(5)
> +#define BQ2515X_CV_CHRG_MODE BIT(6)
> +
> +#define BQ2515X_VIN_OVP_FAULT_STAT BIT(7)
> +
> +#define BQ2515X_WATCHDOG_DISABLE BIT(4)
> +
> +#define BQ2515X_ICHARGE_RANGE BIT(7)
> +
> +#define BQ2515X_VLOWV_SEL BIT(5)
> +
> +#define BQ2515X_CHARGER_DISABLE BIT(0)
> +
> +#define BQ2515X_HWRESET_14S_WD BIT(1)
> +
> +static const int bq2515x_ilim_lvl_values[] = {
> + 50000, 100000, 150000, 200000, 300000, 400000, 500000, 600000
> +};
> +
> +/**
> + * struct bq2515x_init_data -
> + * @ilim: input current limit
> + */
> +struct bq2515x_init_data {
> + int ilim;
> +};
> +
> +enum bq2515x_id {
> + BQ25150,
> + BQ25155,
> +};
> +
> +/**
> + * struct bq2515x_device -
> + * @mains: mains properties
> + * @battery: battery properties
> + * @client: i2c client structure
> + * @regmap: register map structure
> + * @dev: device structure
> + * @id: enumeration of device ids
> + *
> + * @reset_gpio: manual reset (MR) pin
> + * @lp_gpio: low power mode pin
> + * @ac_detect_gpio: power good (PG) pin
> + * @ce_gpio: charge enable (CE) pin
> + *
> + * @model_name: string value describing device model
> + * @device_id: value of device_id
> + * @mains_online: boolean value indicating power supply online
> + *
> + * @bq2515x_init_data init_data: charger initialization data structure
> + */
> +struct bq2515x_device {
> + struct power_supply *mains;
> + struct power_supply *battery;
> + struct i2c_client *client;
'client' doesn't seem used, it usually has use in the remove path, but
this driver has everything devm managed, so not needed.
> + struct regmap *regmap;
> + struct device *dev;
> + enum bq2515x_id id;
> +
> + struct gpio_desc *reset_gpio;
> + struct gpio_desc *lp_gpio;
> + struct gpio_desc *ac_detect_gpio;
> + struct gpio_desc *ce_gpio;
> +
> + char model_name[I2C_NAME_SIZE];
> + int device_id;
This looks to be the same 'id' above, drop this one and use 'id'
everywhere (or rename this ones type as enum bq2515x_id then remove the
otherone).
> + bool mains_online;
> +
> + struct bq2515x_init_data init_data;
> +};
> +
> +static struct reg_default bq25150_reg_defs[] = {
> + {BQ2515X_FLAG0, 0x0},
> + {BQ2515X_FLAG1, 0x0},
> + {BQ2515X_FLAG2, 0x0},
> + {BQ2515X_FLAG3, 0x0},
> + {BQ2515X_MASK0, 0x0},
> + {BQ2515X_MASK1, 0x0},
> + {BQ2515X_MASK2, 0x71},
> + {BQ2515X_MASK3, 0x0},
> + {BQ2515X_VBAT_CTRL, 0x3C},
> + {BQ2515X_ICHG_CTRL, 0x8},
> + {BQ2515X_PCHRGCTRL, 0x2},
> + {BQ2515X_TERMCTRL, 0x14},
> + {BQ2515X_BUVLO, 0x0},
> + {BQ2515X_CHARGERCTRL0, 0x82},
> + {BQ2515X_CHARGERCTRL1, 0x42},
> + {BQ2515X_ILIMCTRL, 0x1},
> + {BQ2515X_LDOCTRL, 0xB0},
> + {BQ2515X_MRCTRL, 0x2A},
> + {BQ2515X_ICCTRL0, 0x10},
> + {BQ2515X_ICCTRL1, 0x0},
> + {BQ2515X_ICCTRL2, 0x0},
> + {BQ2515X_ADCCTRL0, 0x2},
> + {BQ2515X_ADCCTRL1, 0x40},
> + {BQ2515X_ADC_COMP1_M, 0x23},
> + {BQ2515X_ADC_COMP1_L, 0x20},
> + {BQ2515X_ADC_COMP2_M, 0x38},
> + {BQ2515X_ADC_COMP2_L, 0x90},
> + {BQ2515X_ADC_COMP3_M, 0x0},
> + {BQ2515X_ADC_COMP3_L, 0x0},
> + {BQ2515X_ADC_READ_EN, 0x0},
> + {BQ2515X_TS_FASTCHGCTRL, 0x34},
> + {BQ2515X_TS_COLD, 0x7C},
> + {BQ2515X_TS_COOL, 0x6D},
> + {BQ2515X_TS_WARM, 0x38},
> + {BQ2515X_TS_HOT, 0x27},
> + {BQ2515X_DEVICE_ID, 0x20},
> +};
> +
> +static struct reg_default bq25155_reg_defs[] = {
> + {BQ2515X_FLAG0, 0x0},
> + {BQ2515X_FLAG1, 0x0},
> + {BQ2515X_FLAG2, 0x0},
> + {BQ2515X_FLAG3, 0x0},
> + {BQ2515X_MASK0, 0x0},
> + {BQ2515X_MASK1, 0x0},
> + {BQ2515X_MASK2, 0x71},
> + {BQ2515X_MASK3, 0x0},
> + {BQ2515X_VBAT_CTRL, 0x3C},
> + {BQ2515X_ICHG_CTRL, 0x8},
> + {BQ2515X_PCHRGCTRL, 0x2},
> + {BQ2515X_TERMCTRL, 0x14},
> + {BQ2515X_BUVLO, 0x0},
> + {BQ2515X_CHARGERCTRL0, 0x82},
> + {BQ2515X_CHARGERCTRL1, 0xC2},
> + {BQ2515X_ILIMCTRL, 0x6},
> + {BQ2515X_LDOCTRL, 0xB0},
> + {BQ2515X_MRCTRL, 0x2A},
> + {BQ2515X_ICCTRL0, 0x10},
> + {BQ2515X_ICCTRL1, 0x0},
> + {BQ2515X_ICCTRL2, 0x40},
> + {BQ2515X_ADCCTRL0, 0x2},
> + {BQ2515X_ADCCTRL1, 0x40},
> + {BQ2515X_ADC_COMP1_M, 0x23},
> + {BQ2515X_ADC_COMP1_L, 0x20},
> + {BQ2515X_ADC_COMP2_M, 0x38},
> + {BQ2515X_ADC_COMP2_L, 0x90},
> + {BQ2515X_ADC_COMP3_M, 0x0},
> + {BQ2515X_ADC_COMP3_L, 0x0},
> + {BQ2515X_ADC_READ_EN, 0x0},
> + {BQ2515X_TS_FASTCHGCTRL, 0x34},
> + {BQ2515X_TS_COLD, 0x7C},
> + {BQ2515X_TS_COOL, 0x6D},
> + {BQ2515X_TS_WARM, 0x38},
> + {BQ2515X_TS_HOT, 0x27},
> + {BQ2515X_DEVICE_ID, 0x35},
> +};
> +
> +static bool bq2515x_is_ps_online(struct bq2515x_device *bq2515x)
> +{
> + return bq2515x->mains_online;
Do we need a function for this? Just use 'bq2515x->mains_online' directly.
> +}
> +
> +static int bq2515x_wake_up(struct bq2515x_device *bq2515x)
> +{
> + int ret;
> + int val;
> +
> + /* Read the STAT register if we can read it then the device is out
> + * of ship mode. If the register cannot be read then attempt to wake
> + * it up and enable the ADC.
> + */
> + ret = regmap_read(bq2515x->regmap, BQ2515X_STAT0, &val);
> + if (ret)
> + return ret;
> +
> + /* Need to toggle LP and bring device out of ship mode. The device
> + * will exit the ship mode when the MR pin is held low for at least
> + * t_WAKE2 as shown in section 8.3.7.1 of the datasheet.
> + */
> + gpiod_set_value_cansleep(bq2515x->lp_gpio, 0);
> +
> + gpiod_set_value_cansleep(bq2515x->reset_gpio, 0);
> + usleep_range(BQ2515X_TWAKE2_MIN_US, BQ2515X_TWAKE2_MAX_US);
> + gpiod_set_value_cansleep(bq2515x->reset_gpio, 1);
> +
> + return regmap_write(bq2515x->regmap, BQ2515X_ADC_READ_EN,
> + (BQ2515X_EN_VBAT_READ | BQ2515X_EN_ICHG_READ));
> +}
> +
> +static int bq2515x_update_ps_status(struct bq2515x_device *bq2515x)
> +{
> + bool dc = false;
> + unsigned int val;
> + int ret;
> +
> + if (bq2515x->ac_detect_gpio)
> + val = gpiod_get_value_cansleep(bq2515x->ac_detect_gpio);
> + else {
> + ret = regmap_read(bq2515x->regmap, BQ2515X_STAT0, &val);
> + if (ret)
> + return ret;
> + }
> +
> + dc = val & BQ2515X_VIN_GOOD;
> +
> + ret = bq2515x->mains_online != dc;
> +
> + bq2515x->mains_online = dc;
> +
> + return ret;
> +}
> +
> +static int bq2515x_disable_watchdog_timers(struct bq2515x_device *bq2515x)
> +{
> + int ret;
> +
> + ret = regmap_update_bits(bq2515x->regmap, BQ2515X_CHARGERCTRL0,
> + BQ2515X_WATCHDOG_DISABLE, BQ2515X_WATCHDOG_DISABLE);
> + if (ret)
> + return ret;
> +
> + return regmap_update_bits(bq2515x->regmap, BQ2515X_ICCTRL2,
> + BQ2515X_HWRESET_14S_WD, 0);
> +}
> +
> +static int bq2515x_get_battery_voltage_now(struct bq2515x_device *bq2515x)
> +{
> + int ret;
> + int vbat_msb;
> + int vbat_lsb;
> + uint32_t vbat_measurement;
> +
> + if (!bq2515x_is_ps_online(bq2515x))
> + bq2515x_wake_up(bq2515x);
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_VBAT_M, &vbat_msb);
> + if (ret)
> + return ret;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_VBAT_L, &vbat_lsb);
> + if (ret)
> + return ret;
> +
> + vbat_measurement = (vbat_msb << 8) | vbat_lsb;
> +
> + return vbat_measurement * (BQ2515X_UV_FACTOR / BQ2515X_DIVISOR) *
> + BQ2515X_VBAT_MULTIPLIER;
> +}
> +
> +static int bq2515x_get_battery_current_now(struct bq2515x_device *bq2515x)
> +{
> + int ret;
> + int ichg_msb;
> + int ichg_lsb;
> + uint32_t ichg_measurement;
> + u16 ichg_multiplier = BQ2515X_ICHG_RNG_1B0_UA;
> + unsigned int ichg_reg_code, reg_code;
> + unsigned int icharge_range = 0, pchrgctrl;
> + unsigned int buvlo, vlowv_sel, vlowv = BQ2515X_VLOWV_SEL_1B0_UV;
> +
> + if (!bq2515x_is_ps_online(bq2515x))
> + return -ENODATA;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_ICHG_M, &ichg_msb);
> + if (ret)
> + return ret;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_ADC_ICHG_L, &ichg_lsb);
> + if (ret)
> + return ret;
> +
> + ichg_measurement = (ichg_msb << 8) | ichg_lsb;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_BUVLO, &buvlo);
> + if (ret)
> + return ret;
> +
> + vlowv_sel = buvlo & BQ2515X_VLOWV_SEL;
> +
> + if (vlowv_sel)
> + vlowv = BQ2515X_VLOWV_SEL_1B1_UV;
> +
> + if (bq2515x_get_battery_voltage_now(bq2515x) < vlowv) {
> + ret = regmap_read(bq2515x->regmap, BQ2515X_PCHRGCTRL,
> + &pchrgctrl);
> + if (ret)
> + return ret;
> +
> + reg_code = pchrgctrl & BQ2515X_PRECHARGE_MASK;
> + } else {
> + ret = regmap_read(bq2515x->regmap, BQ2515X_ICHG_CTRL,
> + &ichg_reg_code);
> + if (ret)
> + return ret;
> +
> + reg_code = ichg_reg_code;
> + }
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_PCHRGCTRL, &pchrgctrl);
> + if (ret)
> + return ret;
> +
> + icharge_range = pchrgctrl & BQ2515X_ICHARGE_RANGE;
> +
> + if (icharge_range)
> + ichg_multiplier = BQ2515X_ICHG_RNG_1B1_UA;
> +
> + return reg_code * (ichg_multiplier * ichg_measurement /
> + BQ2515X_ICHG_DIVISOR);
> +}
> +
> +static bool bq2515x_get_charge_disable(struct bq2515x_device *bq2515x)
> +{
> + int ret;
> + int ce_pin;
> + int icctrl2;
> + int charger_disable;
> +
> + ce_pin = gpiod_get_value_cansleep(bq2515x->ce_gpio);
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_ICCTRL2, &icctrl2);
> + if (ret)
> + return ret;
> +
> + charger_disable = icctrl2 & BQ2515X_CHARGER_DISABLE;
> +
> + if (charger_disable || ce_pin)
> + return true;
> +
> + return false;
> +}
> +
> +static int bq2515x_set_charge_disable(struct bq2515x_device *bq2515x, int val)
> +{
> + gpiod_set_value_cansleep(bq2515x->ce_gpio, val);
> +
> + return regmap_update_bits(bq2515x->regmap, BQ2515X_ICCTRL2,
> + BQ2515X_CHARGER_DISABLE, val);
> +}
> +
> +static int bq2515x_get_const_charge_current(struct bq2515x_device *bq2515x)
> +{
> + int ret;
> + u16 ichg_multiplier = BQ2515X_ICHG_RNG_1B0_UA;
> + unsigned int ichg_reg_code;
> + unsigned int pchrgctrl;
> + unsigned int icharge_range;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_ICHG_CTRL, &ichg_reg_code);
> + if (ret)
> + return ret;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_PCHRGCTRL, &pchrgctrl);
> + if (ret)
> + return ret;
> +
> + icharge_range = pchrgctrl & BQ2515X_ICHARGE_RANGE;
> +
> + if (icharge_range)
> + ichg_multiplier = BQ2515X_ICHG_RNG_1B1_UA;
> +
> + return ichg_reg_code * ichg_multiplier;
> +}
> +
> +static int bq2515x_set_const_charge_current(struct bq2515x_device *bq2515x,
> + int val)
> +{
> + int ret;
> + unsigned int ichg_reg_code;
> + u16 ichg_multiplier = BQ2515X_ICHG_RNG_1B0_UA;
> + unsigned int icharge_range = 0;
> +
> + if (val > BQ2515X_ICHG_MAX_UA || val < BQ2515X_ICHG_MIN_UA)
> + return -EINVAL;
> +
> + if (val > BQ2515X_ICHG_CURR_STEP_THRESH_UA) {
> + ichg_multiplier = BQ2515X_ICHG_RNG_1B1_UA;
> + icharge_range = BQ2515X_ICHARGE_RANGE;
> + }
> +
> + bq2515x_set_charge_disable(bq2515x, 1);
> +
> + ret = regmap_update_bits(bq2515x->regmap, BQ2515X_PCHRGCTRL,
> + BQ2515X_ICHARGE_RANGE, icharge_range);
> + if (ret)
> + return ret;
> +
> + ichg_reg_code = val / ichg_multiplier;
> +
> + ret = regmap_write(bq2515x->regmap, BQ2515X_ICHG_CTRL, ichg_reg_code);
> + if (ret)
> + return ret;
> +
> + return bq2515x_set_charge_disable(bq2515x, 0);
> +}
> +
> +static int bq2515x_get_precharge_current(struct bq2515x_device *bq2515x)
> +{
> + int ret;
> + unsigned int pchrgctrl;
> + unsigned int icharge_range;
> + u16 precharge_multiplier = BQ2515X_ICHG_RNG_1B0_UA;
> + unsigned int precharge_reg_code;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_PCHRGCTRL, &pchrgctrl);
> + if (ret)
> + return ret;
> +
> + icharge_range = pchrgctrl & BQ2515X_ICHARGE_RANGE;
> +
> + if (icharge_range)
> + precharge_multiplier = BQ2515X_ICHG_RNG_1B1_UA;
> +
> + precharge_reg_code = pchrgctrl & BQ2515X_PRECHARGE_MASK;
> +
> + return precharge_reg_code * precharge_multiplier;
> +}
> +
> +static int bq2515x_set_precharge_current(struct bq2515x_device *bq2515x,
> + int val)
> +{
> + int ret;
> + unsigned int pchrgctrl;
> + unsigned int icharge_range;
> + unsigned int precharge_reg_code;
> + u16 precharge_multiplier = BQ2515X_ICHG_RNG_1B0_UA;
> + u16 precharge_max_ua = BQ2515X_PRECHRG_ICHRG_RNGE_1875_UA;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_PCHRGCTRL, &pchrgctrl);
> + if (ret)
> + return ret;
> +
> + icharge_range = pchrgctrl & BQ2515X_ICHARGE_RANGE;
> +
> + if (icharge_range) {
> + precharge_max_ua = BQ2515X_PRECHRG_ICHRG_RNGE_3750_UA;
> + precharge_multiplier = BQ2515X_ICHG_RNG_1B1_UA;
> + }
> + if (val > precharge_max_ua || val < BQ2515X_ICHG_MIN_UA)
> + return -EINVAL;
> +
> + precharge_reg_code = val / precharge_multiplier;
> +
> + ret = bq2515x_set_charge_disable(bq2515x, 1);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(bq2515x->regmap, BQ2515X_PCHRGCTRL,
> + BQ2515X_PRECHARGE_MASK, precharge_reg_code);
> + if (ret)
> + return ret;
> +
> + return bq2515x_set_charge_disable(bq2515x, 0);
> +}
> +
> +static int bq2515x_charging_status(struct bq2515x_device *bq2515x,
> + union power_supply_propval *val)
> +{
> + bool status0_no_fault;
> + bool status1_no_fault;
> + bool ce_status;
> + bool charge_done;
> + unsigned int status;
> + int ret;
> +
> + if (!bq2515x_is_ps_online(bq2515x)) {
> + val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
> + return 0;
> + }
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_STAT0, &status);
> + if (ret)
> + return ret;
> +
> + /*
> + * The code block below is used to determine if any faults from the
> + * STAT0 register are disbaling charging or if the charge has completed
> + * according to the CHARGE_DONE_STAT bit.
> + */
> + if (((status & BQ2515X_STAT0_MASK) == true) &
> + ((status & BQ2515X_CHRG_DONE) == false)) {
> + status0_no_fault = true;
> + charge_done = false;
> + } else if (status & BQ2515X_CHRG_DONE) {
> + charge_done = true;
> + status0_no_fault = false;
> + } else {
> + status0_no_fault = false;
> + charge_done = false;
> + }
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_STAT1, &status);
> + if (ret)
> + return ret;
> + /*
> + * The code block below is used to determine if any faults from the
> + * STAT1 register are disbaling charging
> + */
> + if ((status & BQ2515X_STAT1_MASK) == false)
> + status1_no_fault = true;
> + else
> + status1_no_fault = false;
> +
> + ce_status = (!bq2515x_get_charge_disable(bq2515x));
> +
> + /*
> + * If there are no faults and charging is enabled, then status is
> + * charging. Otherwise, if charging is complete, then status is full.
> + * Otherwise, if a fault exists or charging is disabled, then status is
> + * not charging
> + */
> + if (status0_no_fault & status1_no_fault & ce_status)
> + val->intval = POWER_SUPPLY_STATUS_CHARGING;
> + else if (charge_done)
> + val->intval = POWER_SUPPLY_STATUS_FULL;
> + else if (!(status0_no_fault & status1_no_fault & ce_status))
> + val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
> +
> + return 0;
> +}
> +
> +static int bq2515x_get_batt_reg(struct bq2515x_device *bq2515x)
> +{
> + int vbat_reg_code;
> + int ret;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_VBAT_CTRL, &vbat_reg_code);
> + if (ret)
> + return ret;
> +
> + return BQ2515X_VBAT_BASE_VOLT + vbat_reg_code * BQ2515X_VBAT_STEP_UV;
> +}
> +
> +static int bq2515x_set_batt_reg(struct bq2515x_device *bq2515x, int val)
> +{
> + int vbat_reg_code;
> +
> + if (val > BQ2515X_VBAT_REG_MAX || val < BQ2515X_VBAT_REG_MIN)
> + return -EINVAL;
> +
> + vbat_reg_code = (val - BQ2515X_VBAT_BASE_VOLT) / BQ2515X_VBAT_STEP_UV;
> +
> + return regmap_write(bq2515x->regmap, BQ2515X_VBAT_CTRL, vbat_reg_code);
> +}
> +
> +static int bq2515x_get_ilim_lvl(struct bq2515x_device *bq2515x)
> +{
> + int ret;
> + int ilimctrl;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_ILIMCTRL, &ilimctrl);
> + if (ret)
> + return ret;
> +
> + return bq2515x_ilim_lvl_values[ilimctrl & BQ2515X_ILIM_MASK];
> +}
> +
> +static int bq2515x_set_ilim_lvl(struct bq2515x_device *bq2515x, int val)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(bq2515x_ilim_lvl_values); i++) {
> + if (val == bq2515x_ilim_lvl_values[i])
> + break;
> +
> + if (val > bq2515x_ilim_lvl_values[i - 1] &&
> + val < bq2515x_ilim_lvl_values[i]) {
> + if (val - bq2515x_ilim_lvl_values[i - 1] <
> + bq2515x_ilim_lvl_values[i] - val) {
> + i = i - 1;
> + break;
This is a little messy, and it's not immediately obvious it will always
work, in fact thinking about this, what happends in the else case.. the
value is closer to the [i] value, then we just never find it (I don't
think that is what we want, to only find the closest value below the next.)
Lucky for us, there are helper macros already in kernel to do most of
these tasks (I belive there is one for finding the closest value in an
array, but I'll leave finding it for you :) (if it exists..)).
> + }
> + }
> + }
> +
> + return regmap_write(bq2515x->regmap, BQ2515X_ILIMCTRL, i);
> +}
> +
> +static int bq2515x_power_supply_property_is_writeable(struct power_supply *psy,
> + enum power_supply_property prop)
> +{
> + switch (prop) {
> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
> + case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static int bq2515x_charger_get_health(struct bq2515x_device *bq2515x,
> + union power_supply_propval *val)
> +{
> + int health = POWER_SUPPLY_HEALTH_GOOD;
> + int ret;
> + unsigned int stat1;
> + unsigned int flag3;
> +
> + if (!bq2515x_is_ps_online(bq2515x))
> + bq2515x_wake_up(bq2515x);
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_FLAG3, &flag3);
> + if (ret)
> + return ret;
> +
> + ret = regmap_read(bq2515x->regmap, BQ2515X_STAT1, &stat1);
> + if (ret)
> + return ret;
> +
> + if (stat1 & BQ2515X_HEALTH_MASK) {
> + switch (stat1 & BQ2515X_HEALTH_MASK) {
> + case BQ2515X_TS_HOT_STAT:
> + health = POWER_SUPPLY_HEALTH_HOT;
> + break;
> + case BQ2515X_TS_WARM_STAT:
> + health = POWER_SUPPLY_HEALTH_WARM;
> + break;
> + case BQ2515X_TS_COOL_STAT:
> + health = POWER_SUPPLY_HEALTH_COOL;
> + break;
> + case BQ2515X_TS_COLD_STAT:
> + health = POWER_SUPPLY_HEALTH_COLD;
> + break;
> + default:
> + health = POWER_SUPPLY_HEALTH_UNKNOWN;
> + break;
> + }
> + }
> +
> + if (stat1 & BQ2515X_VIN_OVP_FAULT_STAT)
> + health = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> +
> + if (flag3 & BQ2515X_SAFETY_TIMER_EXP)
> + health = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
> +
> + val->intval = health;
> + return 0;
> +}
> +
> +static int bq2515x_mains_set_property(struct power_supply *psy,
> + enum power_supply_property prop,
> + const union power_supply_propval *val)
> +{
> + struct bq2515x_device *bq2515x = power_supply_get_drvdata(psy);
> + int ret;
> +
> + switch (prop) {
> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> + ret = bq2515x_set_ilim_lvl(bq2515x, val->intval);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static int bq2515x_battery_set_property(struct power_supply *psy,
> + enum power_supply_property prop,
> + const union power_supply_propval *val)
> +{
> + struct bq2515x_device *bq2515x = power_supply_get_drvdata(psy);
> + int ret;
> +
> + switch (prop) {
> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
> + ret = bq2515x_set_batt_reg(bq2515x, val->intval);
> + break;
> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
> + ret = bq2515x_set_const_charge_current(bq2515x, val->intval);
> + break;
> + case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
> + ret = bq2515x_set_precharge_current(bq2515x, val->intval);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static int bq2515x_mains_get_property(struct power_supply *psy,
> + enum power_supply_property prop,
> + union power_supply_propval *val)
> +{
> + struct bq2515x_device *bq2515x = power_supply_get_drvdata(psy);
> + int ret = 0;
> +
> + switch (prop) {
> + case POWER_SUPPLY_PROP_ONLINE:
> + val->intval = bq2515x->mains_online;
> + break;
> + case POWER_SUPPLY_PROP_HEALTH:
> + ret = bq2515x_charger_get_health(bq2515x, val);
> + if (ret)
> + val->intval = POWER_SUPPLY_HEALTH_UNKNOWN;
> + break;
> + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> + ret = bq2515x_get_ilim_lvl(bq2515x);
> + if (ret)
> + return ret;
> +
> + val->intval = ret;
> + break;
> + case POWER_SUPPLY_PROP_MODEL_NAME:
> + val->strval = bq2515x->model_name;
> + break;
> + case POWER_SUPPLY_PROP_MANUFACTURER:
> + val->strval = BQ2515X_MANUFACTURER;
> + break;
> + case POWER_SUPPLY_PROP_STATUS:
> + ret = bq2515x_charging_status(bq2515x, val);
> + if (ret)
> + return ret;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static int bq2515x_battery_get_property(struct power_supply *psy,
> + enum power_supply_property prop,
> + union power_supply_propval *val)
> +{
> + struct bq2515x_device *bq2515x = power_supply_get_drvdata(psy);
> + int ret;
> +
> + ret = bq2515x_update_ps_status(bq2515x);
> + if (ret)
> + return ret;
> +
> + switch (prop) {
> +
> + case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> + ret = bq2515x_get_battery_voltage_now(bq2515x);
> + if (ret < 0)
> + return ret;
> +
> + val->intval = ret;
> + break;
> + case POWER_SUPPLY_PROP_CURRENT_NOW:
> + ret = bq2515x_get_battery_current_now(bq2515x);
> + if (ret < 0)
> + return ret;
> +
> + val->intval = ret;
> + break;
> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT:
> + ret = bq2515x_get_const_charge_current(bq2515x);
> + if (ret < 0)
> + return ret;
> +
> + val->intval = ret;
> + break;
> + case POWER_SUPPLY_PROP_PRECHARGE_CURRENT:
> + ret = bq2515x_get_precharge_current(bq2515x);
> + if (ret < 0)
> + return ret;
> +
> + val->intval = ret;
> + break;
> + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
> + ret = bq2515x_get_batt_reg(bq2515x);
> + if (ret < 0)
> + return ret;
> +
> + val->intval = ret;
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static enum power_supply_property bq2515x_battery_properties[] = {
> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
> + POWER_SUPPLY_PROP_CURRENT_NOW,
> + POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT,
> + POWER_SUPPLY_PROP_PRECHARGE_CURRENT,
> + POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
> +};
> +
> +static enum power_supply_property bq2515x_charger_properties[] = {
> + POWER_SUPPLY_PROP_ONLINE,
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_HEALTH,
> + POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
> + POWER_SUPPLY_PROP_MODEL_NAME,
> + POWER_SUPPLY_PROP_MANUFACTURER,
> +};
> +
> +static struct power_supply_desc bq2515x_mains_desc = {
> + .name = "bq2515x-mains",
> + .type = POWER_SUPPLY_TYPE_MAINS,
> + .get_property = bq2515x_mains_get_property,
> + .set_property = bq2515x_mains_set_property,
> + .properties = bq2515x_charger_properties,
> + .num_properties = ARRAY_SIZE(bq2515x_charger_properties),
> + .property_is_writeable = bq2515x_power_supply_property_is_writeable,
> +};
> +
> +static struct power_supply_desc bq2515x_battery_desc = {
> + .name = "bq2515x-battery",
> + .type = POWER_SUPPLY_TYPE_BATTERY,
> + .get_property = bq2515x_battery_get_property,
> + .set_property = bq2515x_battery_set_property,
> + .properties = bq2515x_battery_properties,
> + .num_properties = ARRAY_SIZE(bq2515x_battery_properties),
> + .property_is_writeable = bq2515x_power_supply_property_is_writeable,
> +};
> +
> +static int bq2515x_power_supply_register(struct bq2515x_device *bq2515x,
> + struct device *dev)
> +{
> + struct power_supply_config psy_cfg = { .drv_data = bq2515x,
> + .of_node = dev->of_node, };
> +
> + bq2515x->mains = devm_power_supply_register(bq2515x->dev,
> + &bq2515x_mains_desc,
> + &psy_cfg);
> + if (IS_ERR(bq2515x->mains))
> + return -EINVAL;
> +
> + bq2515x->battery = devm_power_supply_register(bq2515x->dev,
> + &bq2515x_battery_desc,
> + &psy_cfg);
> + if (IS_ERR(bq2515x->battery))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int bq2515x_hw_init(struct bq2515x_device *bq2515x)
> +{
> + int ret;
> + struct power_supply_battery_info bat_info = { };
> +
> + ret = bq2515x_disable_watchdog_timers(bq2515x);
> + if (ret)
> + return ret;
> +
> + if (bq2515x->init_data.ilim) {
> + ret = bq2515x_set_ilim_lvl(bq2515x, bq2515x->init_data.ilim);
> + if (ret)
> + return ret;
> + }
> +
> + ret = power_supply_get_battery_info(bq2515x->mains, &bat_info);
> + if (ret) {
> + dev_warn(bq2515x->dev, "battery info missing, default values will be applied\n");
> +
> + bat_info.constant_charge_current_max_ua =
> + BQ2515X_DEFAULT_ICHG_UA;
> +
> + bat_info.constant_charge_voltage_max_uv =
> + BQ2515X_DEFAULT_VBAT_REG_UV;
> +
> + bat_info.precharge_current_ua =
> + BQ2515X_DEFAULT_IPRECHARGE_UA;
> + }
> +
> + ret = bq2515x_set_const_charge_current(bq2515x,
> + bat_info.constant_charge_current_max_ua);
> + if (ret)
> + return ret;
> +
> + ret = bq2515x_set_batt_reg(bq2515x,
> + bat_info.constant_charge_voltage_max_uv);
> + if (ret)
> + return ret;
> +
> + return bq2515x_set_precharge_current(bq2515x,
> + bat_info.precharge_current_ua);
> +}
> +
> +static int bq2515x_read_properties(struct bq2515x_device *bq2515x)
> +{
> + int ret;
> +
> + ret = device_property_read_u32(bq2515x->dev,
> + "input-current-limit-microamp",
> + &bq2515x->init_data.ilim);
> + if (ret)
> + switch (bq2515x->device_id) {
> + case BQ25150:
> + bq2515x->init_data.ilim = BQ25150_DEFAULT_ILIM_UA;
> + break;
> + case BQ25155:
> + bq2515x->init_data.ilim = BQ25155_DEFAULT_ILIM_UA;
> + break;
> + }
> +
> + bq2515x->ac_detect_gpio = devm_gpiod_get_optional(bq2515x->dev,
> + "ac-detect", GPIOD_IN);
> + if (IS_ERR(bq2515x->ac_detect_gpio)) {
> + ret = PTR_ERR(bq2515x->ac_detect_gpio);
> + if (ret != -ENODEV) {
> + dev_err(bq2515x->dev, "Failed to get ac detect: %d\n",
> + ret);
Do we need to print 'ret' here? It is print out in the caller on
returning, plus the alignment wrap is really not so pretty here..
> + return ret;
> + }
> + bq2515x->ac_detect_gpio = NULL;
> + }
> +
> + bq2515x->reset_gpio = devm_gpiod_get_optional(bq2515x->dev,
> + "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(bq2515x->reset_gpio)) {
> + ret = PTR_ERR(bq2515x->reset_gpio);
> + if (ret != -ENODEV) {
> + dev_err(bq2515x->dev, "Failed to get reset: %d\n",
> + ret);
> + return ret;
> + }
> + bq2515x->reset_gpio = NULL;
> + }
> +
> + bq2515x->lp_gpio = devm_gpiod_get_optional(bq2515x->dev, "low-power",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(bq2515x->lp_gpio)) {
> + ret = PTR_ERR(bq2515x->lp_gpio);
> + if (ret != -ENODEV) {
> + dev_err(bq2515x->dev, "Failed to get low power: %d\n",
> + ret);
> + return ret;
> + }
> + bq2515x->lp_gpio = NULL;
> + }
> +
> + bq2515x->ce_gpio = devm_gpiod_get_optional(bq2515x->dev,
> + "charge-enable",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(bq2515x->ce_gpio)) {
> + ret = PTR_ERR(bq2515x->ce_gpio);
> + if (ret != -ENODEV) {
> + dev_err(bq2515x->dev, "Failed to get ce %d\n", ret);
> + return ret;
> + }
> + bq2515x->ce_gpio = NULL;
> + }
> +
> + return 0;
> +}
> +
> +static bool bq2515x_volatile_register(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case BQ2515X_STAT0 ... BQ2515X_FLAG3:
> + case BQ2515X_ADC_VBAT_M ... BQ2515X_ADC_IIN_L:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static const struct regmap_config bq25150_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +
> + .max_register = BQ2515X_DEVICE_ID,
> + .reg_defaults = bq25150_reg_defs,
NITPICK: 'defs' reads to me like definitions, maybe just use _defautls.
> + .num_reg_defaults = ARRAY_SIZE(bq25150_reg_defs),
> + .cache_type = REGCACHE_RBTREE,
> + .volatile_reg = bq2515x_volatile_register,
> +};
> +
> +static const struct regmap_config bq25155_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +
> + .max_register = BQ2515X_DEVICE_ID,
> + .reg_defaults = bq25155_reg_defs,
> + .num_reg_defaults = ARRAY_SIZE(bq25155_reg_defs),
> + .cache_type = REGCACHE_RBTREE,
> + .volatile_reg = bq2515x_volatile_register,
> +};
> +
> +static int bq2515x_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct device *dev = &client->dev;
> + struct bq2515x_device *bq;
> + int ret;
> +
> + bq = devm_kzalloc(dev, sizeof(*bq), GFP_KERNEL);
> + if (!bq)
> + return -ENOMEM;
> +
> + bq->client = client;
> + bq->dev = dev;
> +
> + strncpy(bq->model_name, id->name, I2C_NAME_SIZE);
> +
> + bq->device_id = (enum bq2515x_id)of_device_get_match_data(&client->dev);
This grabs the 'data' field of of_device_id table below which will
always be zero as you do not define the IDs there. You are actually
wanting to grab from 'data' in i2c_device_id table. So replace this line
with:
bq->device_id = id->driver_data;
After that since you don't use any of_* functions, drop the
'of_device.h' header include. (Idea here is that most drivers should not
use of_ functions directly anyway).
> +
> + switch (bq->device_id) {
> + case BQ25150:
> + bq->regmap = devm_regmap_init_i2c(client,
> + &bq25150_regmap_config);
> + break;
> + case BQ25155:
> + bq->regmap = devm_regmap_init_i2c(client,
> + &bq25155_regmap_config);
> + break;
> + }
> +
> + if (IS_ERR(bq->regmap)) {
> + dev_err(dev, "failed to allocate register map\n");
> + return PTR_ERR(bq->regmap);
> + }
> +
> + i2c_set_clientdata(client, bq);
> +
> + ret = bq2515x_read_properties(bq);
> + if (ret) {
> + dev_err(dev, "Failed to read device tree properties %d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = bq2515x_power_supply_register(bq, dev);
> + if (ret) {
> + dev_err(dev, "failed to register power supply\n");
> + return ret;
> + }
> +
Is this order right here? Usually the last thing you do is register with
the framework, doing it before all the device init is done could lead to
some accessor functions in this driver getting called before the below
fully inits the chip.
Andrew
> + ret = bq2515x_hw_init(bq);
> + if (ret) {
> + dev_err(dev, "Cannot initialize the chip\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id bq2515x_i2c_ids[] = {
> + { "bq25150", BQ25150, },
> + { "bq25155", BQ25155, },
> + {},
> +};
> +MODULE_DEVICE_TABLE(i2c, bq2515x_i2c_ids);
> +
> +static const struct of_device_id bq2515x_of_match[] = {
> + { .compatible = "ti,bq25150", },
> + { .compatible = "ti,bq25155", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, bq2515x_of_match);
> +
> +static struct i2c_driver bq2515x_driver = {
> + .driver = {
> + .name = "bq2515x-charger",
> + .of_match_table = bq2515x_of_match,
> + },
> + .probe = bq2515x_probe,
> + .id_table = bq2515x_i2c_ids,
> +};
> +module_i2c_driver(bq2515x_driver);
> +
> +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
> +MODULE_AUTHOR("Ricardo Rivera-Matos <r-rivera-matos@ti.com>");
> +MODULE_DESCRIPTION("BQ2515X charger driver");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply
* Re: [PATCH v3 0/9] clocksource/drivers/timer-atmel-tcb: add sama5d2 support
From: Daniel Lezcano @ 2020-05-18 16:03 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Thomas Gleixner, Nicolas Ferre, Sebastian Andrzej Siewior,
kamel.bouhara, linux-arm-kernel, linux-kernel, devicetree
In-Reply-To: <20200506080554.283177-1-alexandre.belloni@bootlin.com>
Hi Alexandre,
I would like to take the whole series but I need Rob's blessing for the
DT parts.
On 06/05/2020 10:05, Alexandre Belloni wrote:
> This series mainly adds sama5d2 support where we need to avoid using
> clock index 0 because that clock is never enabled by the driver.
>
> There is also a rework of the 32khz clock handling so it is not used for
> clockevents on 32 bit counter because the increased rate improves the
> resolution and doesn't have any drawback with that counter width. This
> replaces a patch that has been carried in the linux-rt tree for a while.
>
> Changes in v3:
> - Moved the child node documentation to the parent documentation
>
> Changes in v2:
> - Rebased on v5.7-rc1
> - Moved the binding documentation to its proper place
> - Added back the atmel,tcb-timer child node documentation
>
> Alexandre Belloni (8):
> dt-bindings: atmel-tcb: convert bindings to json-schema
> dt-bindings: microchip: atmel,at91rm9200-tcb: add sama5d2 compatible
> ARM: dts: at91: sama5d2: add TCB GCLK
> clocksource/drivers/timer-atmel-tcb: rework 32khz clock selection
> clocksource/drivers/timer-atmel-tcb: fill tcb_config
> clocksource/drivers/timer-atmel-tcb: stop using the 32kHz for
> clockevents
> clocksource/drivers/timer-atmel-tcb: allow selecting first divider
> clocksource/drivers/timer-atmel-tcb: add sama5d2 support
>
> Kamel Bouhara (1):
> ARM: at91: add atmel tcb capabilities
>
> .../devicetree/bindings/mfd/atmel-tcb.txt | 56 -------
> .../soc/microchip/atmel,at91rm9200-tcb.yaml | 150 ++++++++++++++++++
> arch/arm/boot/dts/sama5d2.dtsi | 12 +-
> drivers/clocksource/timer-atmel-tcb.c | 101 +++++++-----
> include/soc/at91/atmel_tcb.h | 5 +
> 5 files changed, 219 insertions(+), 105 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-tcb.txt
> create mode 100644 Documentation/devicetree/bindings/soc/microchip/atmel,at91rm9200-tcb.yaml
>
--
<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 3/3] iio: magnetometer: ak8975: Add gpio reset support
From: Jonathan Albrieux @ 2020-05-18 16:01 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linux Kernel Mailing List, Andy Shevchenko,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Greg Kroah-Hartman, Hartmut Knaack, Jonathan Cameron,
Lars-Peter Clausen, Linus Walleij,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Steve Winslow, Thomas Gleixner, Jonathan Cameron
In-Reply-To: <CAHp75VdFJUNOtRyCNEGnvoOCZYoPvyhjC15_iC72JD-1sOavwA@mail.gmail.com>
On Mon, May 18, 2020 at 05:55:51PM +0300, Andy Shevchenko wrote:
> On Mon, May 18, 2020 at 4:38 PM Jonathan Albrieux
> <jonathan.albrieux@gmail.com> wrote:
>
> > + gpiod_set_value_cansleep(data->reset_gpiod, 1);
>
> (1)
>
> ...
>
> > + /*
> > + * If reset pin is provided then will be set to high on power on
> > + * and to low on power off according to AK09911 datasheet
> > + */
>
> Wording is confusing, perhaps you have to use 'asserted / deasserted'.
Thank you for the suggestion, I'll be working on rewording as soon as
possible.
> Btw, in (1) it's also "high" (asserted). I barely understand how it's
> supposed to work in all cases?
>
> > + reset_gpiod = devm_gpiod_get_optional(&client->dev,
> > + "reset", GPIOD_OUT_HIGH);
> > + if (IS_ERR(reset_gpiod))
> > + return PTR_ERR(reset_gpiod);
>
I'm sorry but I'm not sure about what you mean by saying all cases.
Currently I'm testing this driver on a msm8916 device having AK09911
magnetometer. At the current stage the driver is failing on probe
because reset pin is not connected to VID (as datasheet requires in case
of pin not being used). In case of reset pin not asserted, register's
reset is triggered resulting in empty registers, leading to probe fail.
For this reason pin is asserted during power on in order to have
informations in registers and deasserted before power off triggering
a reset.
A workaround that gets AK09911 working on device is by setting the
reset pin always high on device tree. This way registers gets reset by
a Power On Reset circuit autonomously and reset pin never triggers the
reset.
> --
> With Best Regards,
> Andy Shevchenko
Hoping to having answered to your question,
Best regards,
Jonathan Albrieux
^ permalink raw reply
* Re: [PATCH 1/4] clk/soc: mediatek: mt8183: Bind clock driver from platform device
From: Matthias Brugger @ 2020-05-18 15:57 UTC (permalink / raw)
To: Chun-Kuang Hu, matthias.bgg
Cc: Rob Herring, Michael Turquette, Stephen Boyd, Kate Stewart,
devicetree, Greg Kroah-Hartman, linux-kernel,
moderated list:ARM/Mediatek SoC support, Linux ARM, mtk01761,
Thomas Gleixner, linux-clk, Allison Randal,
Enric Balletbo i Serra
In-Reply-To: <CAAOTY_9-_fy4oGTBLPsXkWJ2ihL7k2hzfwiTorit+YkNi_SeMw@mail.gmail.com>
On 18/05/2020 17:30, Chun-Kuang Hu wrote:
> Hi, Matthias:
>
> <matthias.bgg@kernel.org> 於 2020年5月18日 週一 下午7:32寫道：
>>
>> From: Matthias Brugger <matthias.bgg@gmail.com>
>>
>> The mmsys driver is now the top level entry point for the multimedia
>> system (mmsys), we bind the clock driver by creating a platform device.
>> We also bind the MediaTek DRM driver which is not yet implement and
>> therefor will errror out for now.
>
> You may need to let CONFIG_MTK_MMSYS depends on
> CONFIG_COMMON_CLK_MT8173_MMSYS || CONFIG_COMMON_CLK_MT8183_MMSYS
>
That will be fixed by:
https://patchwork.kernel.org/patch/11469521/
Otherwise this won't scale as we would need to add the SoCs using the mtk-mmsys
driver and this will be a lot if not all.
Regards,
Matthias
> Regards,
> Chun-Kuang.
>
>>
>> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
>> ---
>>
>> drivers/clk/mediatek/clk-mt8183-mm.c | 9 ++-------
>> drivers/soc/mediatek/mtk-mmsys.c | 8 ++++++++
>> 2 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/clk/mediatek/clk-mt8183-mm.c b/drivers/clk/mediatek/clk-mt8183-mm.c
>> index 720c696b506d..9d60e09619c1 100644
>> --- a/drivers/clk/mediatek/clk-mt8183-mm.c
>> +++ b/drivers/clk/mediatek/clk-mt8183-mm.c
>> @@ -84,8 +84,9 @@ static const struct mtk_gate mm_clks[] = {
>>
>> static int clk_mt8183_mm_probe(struct platform_device *pdev)
>> {
>> + struct device *dev = &pdev->dev;
>> + struct device_node *node = dev->parent->of_node;
>> struct clk_onecell_data *clk_data;
>> - struct device_node *node = pdev->dev.of_node;
>>
>> clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
>>
>> @@ -95,16 +96,10 @@ static int clk_mt8183_mm_probe(struct platform_device *pdev)
>> return of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
>> }
>>
>> -static const struct of_device_id of_match_clk_mt8183_mm[] = {
>> - { .compatible = "mediatek,mt8183-mmsys", },
>> - {}
>> -};
>> -
>> static struct platform_driver clk_mt8183_mm_drv = {
>> .probe = clk_mt8183_mm_probe,
>> .driver = {
>> .name = "clk-mt8183-mm",
>> - .of_match_table = of_match_clk_mt8183_mm,
>> },
>> };
>>
>> diff --git a/drivers/soc/mediatek/mtk-mmsys.c b/drivers/soc/mediatek/mtk-mmsys.c
>> index cacafe23c823..783c3dd008b2 100644
>> --- a/drivers/soc/mediatek/mtk-mmsys.c
>> +++ b/drivers/soc/mediatek/mtk-mmsys.c
>> @@ -92,6 +92,10 @@ static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
>> .clk_driver = "clk-mt8173-mm",
>> };
>>
>> +static const struct mtk_mmsys_driver_data mt8183_mmsys_driver_data = {
>> + .clk_driver = "clk-mt8183-mm",
>> +};
>> +
>> static unsigned int mtk_mmsys_ddp_mout_en(enum mtk_ddp_comp_id cur,
>> enum mtk_ddp_comp_id next,
>> unsigned int *addr)
>> @@ -339,6 +343,10 @@ static const struct of_device_id of_match_mtk_mmsys[] = {
>> .compatible = "mediatek,mt8173-mmsys",
>> .data = &mt8173_mmsys_driver_data,
>> },
>> + {
>> + .compatible = "mediatek,mt8183-mmsys",
>> + .data = &mt8183_mmsys_driver_data,
>> + },
>> { }
>> };
>>
>> --
>> 2.26.2
>>
>>
>> _______________________________________________
>> Linux-mediatek mailing list
>> Linux-mediatek@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ 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