Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH] i2c: add 'single-master' property to generic bindings
From: Rob Herring @ 2020-05-29 22:02 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-i2c, devicetree, Laine Jaakko EXT
In-Reply-To: <20200527113039.5380-1-wsa+renesas@sang-engineering.com>

On Wed, May 27, 2020 at 01:30:39PM +0200, Wolfram Sang wrote:
> It is useful to know if we are the only master on a given bus. Because
> this is a HW description of the bus, add it to the generic bindings.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Cc: Laine Jaakko EXT <ext-jaakko.laine@vaisala.com>
> ---
> 
> We added 'multi-master' back then because most busses are single-master
> and 'multi-master' was the exception. In hindsight, however, this was a
> bad choice because 'multi-master' should be the default, i.e. if you
> know nothing, you should assume there could be another master.
> 
> So, we can't deduce that a missing 'multi-master' property automatically
> means 'single-master'. That's why we need this new property.
> 
> I am a bit tempted to mark 'multi-master' as deprecated because the
> default should be multi-master. However, it might also be a bit more
> descriptive to let "no property" still mean "we don't know". I'd be
> thankful for more opinions here.

Could you just have different timeouts for clearing stalled bus. You 
know quickly if 'single-master' is set, but have to wait longer if not?

Note that we need to add a bunch of these properties to dt-schema 
i2c-controller.yaml. I hadn't done that because I want to dual license 
in the process, but lots of folks have touched i2c.txt IIRC.

Reviewed-by: Rob Herring <robh@kernel.org>

> Thanks and happy hacking,
> 
>    Wolfram
> 
>  Documentation/devicetree/bindings/i2c/i2c.txt | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
> index 819436b48fae..438ae123107e 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> @@ -70,7 +70,12 @@ wants to support one of the below features, it should adapt these bindings.
>  - multi-master
>  	states that there is another master active on this bus. The OS can use
>  	this information to adapt power management to keep the arbitration awake
> -	all the time, for example.
> +	all the time, for example. Can not be combined with 'single-master'.
> +
> +- single-master
> +	states that there is no other master active on this bus. The OS can use
> +	this information to detect a stalled bus more reliably, for example.
> +	Can not be combined with 'multi-master'.
>  
>  Required properties (per child device)
>  --------------------------------------
> -- 
> 2.20.1
> 

^ permalink raw reply

* Re: [PATCH v12 2/4] dt-bindings: power: Convert battery.txt to battery.yaml
From: Rob Herring @ 2020-05-29 22:16 UTC (permalink / raw)
  To: Ricardo Rivera-Matos
  Cc: sre, pali, afd, dmurphy, linux-pm, linux-kernel, devicetree,
	sspatil
In-Reply-To: <20200528225350.661-3-r-rivera-matos@ti.com>

On Thu, May 28, 2020 at 05:53:48PM -0500, Ricardo Rivera-Matos wrote:
> From: Dan Murphy <dmurphy@ti.com>
> 
> Convert the battery.txt file to yaml and fix up the examples.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  .../bindings/power/supply/battery.txt         |  82 +---------
>  .../bindings/power/supply/battery.yaml        | 143 ++++++++++++++++++
>  2 files changed, 144 insertions(+), 81 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/battery.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/battery.txt b/Documentation/devicetree/bindings/power/supply/battery.txt
> index 3049cf88bdcf..b9a81621ce59 100644
> --- a/Documentation/devicetree/bindings/power/supply/battery.txt
> +++ b/Documentation/devicetree/bindings/power/supply/battery.txt
> @@ -1,81 +1 @@
> -Battery Characteristics
> -
> -The devicetree battery node provides static battery characteristics.
> -In smart batteries, these are typically stored in non-volatile memory
> -on a fuel gauge chip. The battery node should be used where there is
> -no appropriate non-volatile memory, or it is unprogrammed/incorrect.
> -
> -Upstream dts files should not include battery nodes, unless the battery
> -represented cannot easily be replaced in the system by one of a
> -different type. This prevents unpredictable, potentially harmful,
> -behavior should a replacement that changes the battery type occur
> -without a corresponding update to the dtb.
> -
> -Required Properties:
> - - compatible: Must be "simple-battery"
> -
> -Optional Properties:
> - - voltage-min-design-microvolt: drained battery voltage
> - - voltage-max-design-microvolt: fully charged battery voltage
> - - energy-full-design-microwatt-hours: battery design energy
> - - charge-full-design-microamp-hours: battery design capacity
> - - precharge-current-microamp: current for pre-charge phase
> - - charge-term-current-microamp: current for charge termination phase
> - - constant-charge-current-max-microamp: maximum constant input current
> - - constant-charge-voltage-max-microvolt: maximum constant input voltage
> - - factory-internal-resistance-micro-ohms: battery factory internal resistance
> - - ocv-capacity-table-0: An array providing the open circuit voltage (OCV)
> -   of the battery and corresponding battery capacity percent, which is used
> -   to look up battery capacity according to current OCV value. And the open
> -   circuit voltage unit is microvolt.
> - - ocv-capacity-table-1: Same as ocv-capacity-table-0
> - ......
> - - ocv-capacity-table-n: Same as ocv-capacity-table-0
> - - ocv-capacity-celsius: An array containing the temperature in degree Celsius,
> -   for each of the battery capacity lookup table. The first temperature value
> -   specifies the OCV table 0, and the second temperature value specifies the
> -   OCV table 1, and so on.
> - - resistance-temp-table: An array providing the temperature in degree Celsius
> -   and corresponding battery internal resistance percent, which is used to look
> -   up the resistance percent according to current temperature to get a accurate
> -   batterty internal resistance in different temperatures.
> -
> -Battery properties are named, where possible, for the corresponding
> -elements in enum power_supply_property, defined in
> -https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/power_supply.h
> -
> -Batteries must be referenced by chargers and/or fuel-gauges
> -using a phandle. The phandle's property should be named
> -"monitored-battery".
> -
> -Example:
> -
> -	bat: battery {
> -		compatible = "simple-battery";
> -		voltage-min-design-microvolt = <3200000>;
> -		voltage-max-design-microvolt = <4200000>;
> -		energy-full-design-microwatt-hours = <5290000>;
> -		charge-full-design-microamp-hours = <1430000>;
> -		precharge-current-microamp = <256000>;
> -		charge-term-current-microamp = <128000>;
> -		constant-charge-current-max-microamp = <900000>;
> -		constant-charge-voltage-max-microvolt = <4200000>;
> -		factory-internal-resistance-micro-ohms = <250000>;
> -		ocv-capacity-celsius = <(-10) 0 10>;
> -		ocv-capacity-table-0 = <4185000 100>, <4113000 95>, <4066000 90>, ...;
> -		ocv-capacity-table-1 = <4200000 100>, <4185000 95>, <4113000 90>, ...;
> -		ocv-capacity-table-2 = <4250000 100>, <4200000 95>, <4185000 90>, ...;
> -		resistance-temp-table = <20 100>, <10 90>, <0 80>, <(-10) 60>;
> -	};
> -
> -	charger: charger@11 {
> -		....
> -		monitored-battery = <&bat>;
> -		...
> -	};
> -
> -	fuel_gauge: fuel-gauge@22 {
> -		....
> -		monitored-battery = <&bat>;
> -		...
> -	};
> +The contents of this file has been moved to battery.yaml
> diff --git a/Documentation/devicetree/bindings/power/supply/battery.yaml b/Documentation/devicetree/bindings/power/supply/battery.yaml
> new file mode 100644
> index 000000000000..f0b544a22219
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/battery.yaml
> @@ -0,0 +1,143 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/supply/battery.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Battery Characteristics
> +
> +maintainers:
> +  - Sebastian Reichel <sre@kernel.org> 
> +
> +description: |
> +  The devicetree battery node provides static battery characteristics.
> +  In smart batteries, these are typically stored in non-volatile memory
> +  on a fuel gauge chip. The battery node should be used where there is
> +  no appropriate non-volatile memory, or it is unprogrammed/incorrect.
> +
> +  Upstream dts files should not include battery nodes, unless the battery
> +  represented cannot easily be replaced in the system by one of a
> +  different type. This prevents unpredictable, potentially harmful,
> +  behavior should a replacement that changes the battery type occur
> +  without a corresponding update to the dtb.
> +
> +  Battery properties are named, where possible, for the corresponding elements
> +  in enum power_supply_property, defined in include/linux/power_supply.h
> +
> +  Batteries must be referenced by chargers and/or fuel-gauges using a phandle.
> +  The phandle's property should be named "monitored-battery".
> +
> +properties:
> +  compatible:
> +    const: simple-battery

I suspect we'll need a battery.yaml and simple-battery.yaml schema if 
these properties are used for other batteries. Not sure really, so fine 
for now.

> +
> +  voltage-min-design-microvolt: 
> +    $ref: /schemas/types.yaml#/definitions/uint32

Don't need types on many of these as standard units have a type already.

> +    description: drained battery voltage
> +
> +  voltage-max-design-microvolt:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: fully charged battery voltage
> +
> +  energy-full-design-microwatt-hours:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: battery design energy
> +
> +  charge-full-design-microamp-hours:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: battery design capacity
> +
> +  precharge-current-microamp:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: current for pre-charge phase
> +
> +  charge-term-current-microamp:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: current for charge termination phase
> +
> +  constant-charge-current-max-microamp:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: maximum constant input current
> +
> +  constant-charge-voltage-max-microvolt:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: maximum constant input voltage
> +
> +  factory-internal-resistance-micro-ohms:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: battery factory internal resistance
> +
> +  ocv-capacity-table-0:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    description: |
> +      An array providing the open circuit voltage (OCV)
> +      of the battery and corresponding battery capacity percent, which is used
> +      to look up battery capacity according to current OCV value. And the open
> +      circuit voltage unit is microvolt.
> +
> +  ocv-capacity-table-1:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    description: Same as ocv-capacity-table-0
> +
> +  ocv-capacity-table-n:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    description: Same as ocv-capacity-table-0

Make this a pattern under patternProperties: '^ocv-capacity-table-[0-9]$' 

Is 10 enough or maybe you need more.

maxItems: 100 ?? I asssume 1% granularity would be enough for everyone?
items:
  items:
    - description: open circuit voltage (OCV) in microvolts
    - description: battery capacity percent
      maximum: 100

> +
> +  ocv-capacity-celsius:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description: |
> +      An array containing the temperature in degree Celsius,
> +      for each of the battery capacity lookup table. The first temperature value
> +      specifies the OCV table 0, and the second temperature value specifies the
> +      OCV table 1, and so on.
> +
> +  resistance-temp-table:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    description: |
> +      An array providing the temperature in degree Celsius
> +      and corresponding battery internal resistance percent, which is used to
> +      look up the resistance percent according to current temperature to get an
> +      accurate batterty internal resistance in different temperatures.

Similar definition needed here.

> +
> +  monitored-battery:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle to the battery node being monitored

As this is the battery node, this property doesn't belong here.

> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    power {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      battery:battery {
> +        compatible = "simple-battery";
> +        voltage-min-design-microvolt = <3200000>;
> +        voltage-max-design-microvolt = <4200000>;
> +        energy-full-design-microwatt-hours = <5290000>;
> +        charge-full-design-microamp-hours = <1430000>;
> +        precharge-current-microamp = <256000>;
> +        charge-term-current-microamp = <128000>;
> +        constant-charge-current-max-microamp = <900000>;
> +        constant-charge-voltage-max-microvolt = <4200000>;
> +        factory-internal-resistance-micro-ohms = <250000>;
> +        ocv-capacity-celsius = <(-10) 0 10>;
> +        ocv-capacity-table-0 = <4185000 100>, <4113000 95>, <4066000 90>;
> +        ocv-capacity-table-1 = <4200000 100>, <4185000 95>, <4113000 90>;
> +        resistance-temp-table = <20 100>, <10 90>, <0 80>, <(-10) 60>;
> +      };
> +
> +      charger:charger@11 {

Drop unused labels.

> +        reg = <0x11>;
> +        monitored-battery = <&battery>;
> +      };
> +
> +      fuel_gauge:fuel-gauge@22 {
> +        reg = <0x22>;
> +        monitored-battery = <&battery>;
> +      };
> +    };
> -- 
> 2.26.2
> 

^ permalink raw reply

* Re: [PATCH v12 3/4] dt-bindings: power: Add the bindings for the bq2515x family of chargers.
From: Rob Herring @ 2020-05-29 22:18 UTC (permalink / raw)
  To: Ricardo Rivera-Matos
  Cc: sre, pali, afd, dmurphy, linux-pm, linux-kernel, devicetree,
	sspatil
In-Reply-To: <20200528225350.661-4-r-rivera-matos@ti.com>

On Thu, May 28, 2020 at 05:53:49PM -0500, Ricardo Rivera-Matos wrote:
> The BQ2515X family of devices are highly integrated battery 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 a push-button controller.
> 
> Datasheets:
> http://www.ti.com/lit/ds/symlink/bq25150.pdf
> http://www.ti.com/lit/ds/symlink/bq25155.pdf
> 
> Signed-off-by: Ricardo Rivera-Matos <r-rivera-matos@ti.com>
> ---
>  .../bindings/power/supply/bq2515x.yaml        | 91 +++++++++++++++++++
>  1 file changed, 91 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/supply/bq2515x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/bq2515x.yaml b/Documentation/devicetree/bindings/power/supply/bq2515x.yaml
> new file mode 100644
> index 000000000000..19cb336d581e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/bq2515x.yaml
> @@ -0,0 +1,91 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright (C) 2020 Texas Instruments Incorporated
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/power/supply/bq2515x.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: TI bq2515x 500-mA Linear charger family
> +
> +maintainers:
> +  - Dan Murphy <dmurphy@ti.com>
> +  - Ricardo Rivera-Matos <r-rivera-matos@ti.com>
> +
> +description: |
> +  The BQ2515x family is a highly integrated battery charge management IC that
> +  integrates the most common functions for wearable devices, namely a charger,
> +  an output voltage rail, ADC for battery and system monitoring, and
> +  push-button controller.
> +
> +  Specifications about the charger can be found at:
> +    http://www.ti.com/lit/ds/symlink/bq25150.pdf
> +    http://www.ti.com/lit/ds/symlink/bq25155.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,bq25150
> +      - ti,bq25155
> +
> +  reg:
> +    maxItems: 1
> +    description: I2C address of the charger.
> +
> +  ac-detect-gpios:
> +    description: |
> +       GPIO used for connecting the bq2515x device PG (AC Detect)
> +       pin.
> +    maxItems: 1
> +
> +  reset-gpios:
> +    description: GPIO used for hardware reset.
> +    maxItems: 1
> +
> +  powerdown-gpios:
> +    description: GPIO used for low power mode of IC.
> +    maxItems: 1
> +
> +  charge-enable-gpios:
> +    description: GPIO used to turn on and off charging.
> +    maxItems: 1
> +
> +  input-current-limit-microamp:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Maximum input current in micro Amps.
> +    minimum: 50000
> +    maximum: 500000
> +
> +  monitored-battery:
> +    $ref: battery.yaml#

This doesn't work. It's saying monitored-battery is a node containing 
all the properties in battery.yaml.

> +
> +required:
> +  - compatible
> +  - reg

How is the battery optional?

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    bat: battery {
> +      compatible = "simple-battery";
> +      constant-charge-current-max-microamp = <50000>;
> +      precharge-current-microamp = <2500>;
> +      constant-charge-voltage-max-microvolt = <4000000>;
> +    };
> +    #include <dt-bindings/gpio/gpio.h>
> +    i2c0 {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      bq25150: charger@6b {
> +        compatible = "ti,bq25150";
> +        reg = <0x6b>;
> +        monitored-battery = <&bat>;
> +        input-current-limit-microamp = <100000>;
> +
> +        ac-detect-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
> +        reset-gpios = <&gpio0 14 GPIO_ACTIVE_HIGH>;
> +        powerdown-gpios = <&gpio0 15 GPIO_ACTIVE_HIGH>;
> +        charge-enable-gpios = <&gpio0 13 GPIO_ACTIVE_LOW>;
> +      };
> +    };
> -- 
> 2.26.2
> 

^ permalink raw reply

* Re: [PATCH v7 01/20] dt-bindings: mtd: Document nand-ecc-placement
From: Rob Herring @ 2020-05-29 22:21 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd,
	Mark Rutland, devicetree, Boris Brezillon, Thomas Petazzoni,
	Paul Cercueil, Chuanhong Guo, Weijie Gao, linux-arm-kernel,
	Mason Yang, Julien Su
In-Reply-To: <20200529002517.3546-2-miquel.raynal@bootlin.com>

On Fri, May 29, 2020 at 02:24:58AM +0200, Miquel Raynal wrote:
> This optional property defines where the ECC bytes are expected to be
> stored. No value defaults to an unknown location, while these
> locations can be explicitly set to OOB or interleaved depending if
> the ECC bytes are entirely stored in the OOB area or mixed with
> regular data in the main area (also sometimes referred as
> "syndrome").
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  .../devicetree/bindings/mtd/nand-controller.yaml       | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> index d261b7096c69..4a0798247d2d 100644
> --- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> +++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> @@ -56,6 +56,16 @@ patternProperties:
>            (Linux will handle the calculations). soft_bch is deprecated
>            and should be replaced by soft and nand-ecc-algo.
>  
> +      nand-ecc-placement:
> +        allOf:

You can drop using allOf now, but it still works as is.

Acked-by: Rob Herring <robh@kernel.org>

> +          - $ref: /schemas/types.yaml#/definitions/string
> +          - enum: [ oob, interleaved ]
> +        description:
> +          Location of the ECC bytes. This location is unknown by default
> +          but can be explicitly set to "oob", if all ECC bytes are
> +          known to be stored in the OOB area, or "interleaved" if ECC
> +          bytes will be interleaved with regular data in the main area.
> +
>        nand-ecc-algo:
>          allOf:
>            - $ref: /schemas/types.yaml#/definitions/string
> -- 
> 2.20.1
> 

^ permalink raw reply

* Re: [PATCH v6 00/16] spi: dw: Add generic DW DMA controller support
From: Mark Brown @ 2020-05-29 22:21 UTC (permalink / raw)
  To: Rob Herring
  Cc: Serge Semin, Serge Semin, Ekaterina Skachko, Feng Tang,
	devicetree, Thomas Bogendoerfer, Georgy Vlasov, Pavel Parkhomenko,
	Alexey Kolotnikov, linux-spi, linux-kernel, Vadim Vlasov,
	Alexey Malahov, linux-mips, Andy Shevchenko, Ramil Zaripov,
	Arnd Bergmann, Maxim Kaurkin
In-Reply-To: <20200529212226.GA2984630@bogus>

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

On Fri, May 29, 2020 at 03:22:26PM -0600, Rob Herring wrote:
> On Fri, May 29, 2020 at 06:33:25PM +0100, Mark Brown wrote:

> > Please rebase.  TBH I'd not noticed Rob's review so I just left it
> > waiting for that, there's such a huge backlog there it didn't occur to
> > me that it might've been reviewed.

> Hey, I'm down to about 10 patches now. I think I'll take the rest of the 
> week off.

Ah, nice!  Good to hear.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH] dt-bindings: Merge gpio-usb-b-connector with usb-connector
From: Rob Herring @ 2020-05-29 22:21 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Prashant Malani, Rob Herring, linux-usb, devicetree,
	Greg Kroah-Hartman, linux-kernel
In-Reply-To: <20200529180631.3200680-1-thierry.reding@gmail.com>

On Fri, 29 May 2020 20:06:31 +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> The binding for usb-connector is a superset of gpio-usb-b-connector. One
> major difference is that gpio-usb-b-connector requires at least one of
> the vbus-gpios and id-gpios properties to be specified. Merge the two
> bindings by adding the compatible string combination for the GPIO USB-B
> variant and an extra conditional for the required properties list to the
> usb-connector.yaml file.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  .../bindings/connector/usb-connector.yaml     | 39 +++++++++++++++++--
>  .../devicetree/bindings/usb/usb-conn-gpio.txt | 30 --------------
>  2 files changed, 35 insertions(+), 34 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/usb/usb-conn-gpio.txt
> 

Applied, thanks!

^ permalink raw reply

* Re: [PATCH v2 2/4] dt-bindings: pinctrl: Document optional BCM7211 wake-up interrupts
From: Rob Herring @ 2020-05-29 22:22 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Rob Herring, Linus Walleij, Scott Branden,
	maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Geert Uytterhoeven, Ray Jui, Stefan Wahren, Matti Vaittinen,
	linux-kernel, open list:PIN CONTROL SUBSYSTEM,
	Nicolas Saenz Julienne,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE
In-Reply-To: <20200529191522.27938-3-f.fainelli@gmail.com>

On Fri, 29 May 2020 12:15:20 -0700, Florian Fainelli wrote:
> BCM7211 supports wake-up interrupts in the form of optional interrupt
> lines, one per bank, plus the "all banks" interrupt line.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  .../devicetree/bindings/pinctrl/brcm,bcm2835-gpio.txt         | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH V2] dt-bindings: regulator: Convert anatop regulator to json-schema
From: Mark Brown @ 2020-05-29 22:43 UTC (permalink / raw)
  To: lgirdwood, robh+dt, Anson Huang, linux-kernel, devicetree,
	paul.liu
  Cc: Linux-imx
In-Reply-To: <1590717551-20772-1-git-send-email-Anson.Huang@nxp.com>

On Fri, 29 May 2020 09:59:11 +0800, Anson Huang wrote:
> Convert the anatop regulator binding to DT schema format using json-schema.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/1] dt-bindings: regulator: Convert anatop regulator to json-schema
      commit: 81227f49bd272cbcd9bb4650b250519c8aa22065

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply

* Re: [PATCH v3 4/7] watchdog: dw_wdt: Support devices with non-fixed TOP values
From: Guenter Roeck @ 2020-05-29 22:50 UTC (permalink / raw)
  To: Serge Semin
  Cc: Wim Van Sebroeck, Serge Semin, Alexey Malahov,
	Thomas Bogendoerfer, Arnd Bergmann, Rob Herring, linux-mips,
	devicetree, linux-watchdog, linux-kernel
In-Reply-To: <20200526154123.24402-5-Sergey.Semin@baikalelectronics.ru>

On Tue, May 26, 2020 at 06:41:20PM +0300, Serge Semin wrote:
> In case if the DW Watchdog IP core is synthesised with
> WDT_USE_FIX_TOP == false, the TOP interval indexes make the device
> to load a custom periods to the counter. These periods are hardwired
> at the IP synthesis stage and can be within [2^8, 2^(WDT_CNT_WIDTH - 1)].
> Alas their values can't be detected at runtime and must be somehow
> supplied to the driver so one could properly determine the watchdog
> timeout intervals. For this purpose we suggest to have a vendor-
> specific dts property "snps,watchdog-tops" utilized, which would
> provide an array of sixteen counter values. At device probe stage they
> will be used to initialize the watchdog device timeouts determined
> from the array values and current clocks source rate.
> 
> In order to have custom TOP values supported the driver must be
> altered in the following way. First of all the fixed-top values
> ready-to-use array must be determined for compatibility with currently
> supported devices, which were synthesised with WDT_USE_FIX_TOP == true.
> It will be used if either fixed TOP feature is detected being enabled or
> no custom TOPs are fetched from the device dt node. Secondly at the probe
> stage we must initialize an array of the watchdog timeouts corresponding
> to the detected TOPs list and the reference clock rate. For generality the
> procedure of initialization is designed in a way to support the TOPs array
> with no limitations on the items order or value. Finally the watchdog
> period search methods should be altered to support the new timeouts data
> structure.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: devicetree@vger.kernel.org

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changelog v2:
> - Rearrange SoBs.
> - Add "ms" suffix to the methods returning msec and convert the methods
>   with no "ms" suffix to return a timeout in sec.
> - Make sure minimum timeout is at least 1 sec.
> - Refactor the timeouts calculation procedure to retain the timeouts in
>   the ascending order.
> - Make sure there is no integer overflow in milliseconds calculation. It
>   is saved in a dedicated uint field of the timeout structure.
> ---
>  drivers/watchdog/dw_wdt.c | 191 +++++++++++++++++++++++++++++++++-----
>  1 file changed, 167 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> index fba21de2bbad..693c0d1fd796 100644
> --- a/drivers/watchdog/dw_wdt.c
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -13,6 +13,8 @@
>   */
>  
>  #include <linux/bitops.h>
> +#include <linux/limits.h>
> +#include <linux/kernel.h>
>  #include <linux/clk.h>
>  #include <linux/delay.h>
>  #include <linux/err.h>
> @@ -34,21 +36,40 @@
>  #define WDOG_CURRENT_COUNT_REG_OFFSET	    0x08
>  #define WDOG_COUNTER_RESTART_REG_OFFSET     0x0c
>  #define WDOG_COUNTER_RESTART_KICK_VALUE	    0x76
> +#define WDOG_COMP_PARAMS_1_REG_OFFSET       0xf4
> +#define WDOG_COMP_PARAMS_1_USE_FIX_TOP      BIT(6)
>  
> -/* The maximum TOP (timeout period) value that can be set in the watchdog. */
> -#define DW_WDT_MAX_TOP		15
> +/* There are sixteen TOPs (timeout periods) that can be set in the watchdog. */
> +#define DW_WDT_NUM_TOPS		16
> +#define DW_WDT_FIX_TOP(_idx)	(1U << (16 + _idx))
>  
>  #define DW_WDT_DEFAULT_SECONDS	30
>  
> +static const u32 dw_wdt_fix_tops[DW_WDT_NUM_TOPS] = {
> +	DW_WDT_FIX_TOP(0), DW_WDT_FIX_TOP(1), DW_WDT_FIX_TOP(2),
> +	DW_WDT_FIX_TOP(3), DW_WDT_FIX_TOP(4), DW_WDT_FIX_TOP(5),
> +	DW_WDT_FIX_TOP(6), DW_WDT_FIX_TOP(7), DW_WDT_FIX_TOP(8),
> +	DW_WDT_FIX_TOP(9), DW_WDT_FIX_TOP(10), DW_WDT_FIX_TOP(11),
> +	DW_WDT_FIX_TOP(12), DW_WDT_FIX_TOP(13), DW_WDT_FIX_TOP(14),
> +	DW_WDT_FIX_TOP(15)
> +};
> +
>  static bool nowayout = WATCHDOG_NOWAYOUT;
>  module_param(nowayout, bool, 0);
>  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started "
>  		 "(default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
>  
> +struct dw_wdt_timeout {
> +	u32 top_val;
> +	unsigned int sec;
> +	unsigned int msec;
> +};
> +
>  struct dw_wdt {
>  	void __iomem		*regs;
>  	struct clk		*clk;
>  	unsigned long		rate;
> +	struct dw_wdt_timeout	timeouts[DW_WDT_NUM_TOPS];
>  	struct watchdog_device	wdd;
>  	struct reset_control	*rst;
>  	/* Save/restore */
> @@ -64,20 +85,66 @@ static inline int dw_wdt_is_enabled(struct dw_wdt *dw_wdt)
>  		WDOG_CONTROL_REG_WDT_EN_MASK;
>  }
>  
> -static inline int dw_wdt_top_in_seconds(struct dw_wdt *dw_wdt, unsigned top)
> +static unsigned int dw_wdt_find_best_top(struct dw_wdt *dw_wdt,
> +					 unsigned int timeout, u32 *top_val)
>  {
> +	int idx;
> +
>  	/*
> -	 * There are 16 possible timeout values in 0..15 where the number of
> -	 * cycles is 2 ^ (16 + i) and the watchdog counts down.
> +	 * Find a TOP with timeout greater or equal to the requested number.
> +	 * Note we'll select a TOP with maximum timeout if the requested
> +	 * timeout couldn't be reached.
>  	 */
> -	return (1U << (16 + top)) / dw_wdt->rate;
> +	for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) {
> +		if (dw_wdt->timeouts[idx].sec >= timeout)
> +			break;
> +	}
> +
> +	if (idx == DW_WDT_NUM_TOPS)
> +		--idx;
> +
> +	*top_val = dw_wdt->timeouts[idx].top_val;
> +
> +	return dw_wdt->timeouts[idx].sec;
>  }
>  
> -static int dw_wdt_get_top(struct dw_wdt *dw_wdt)
> +static unsigned int dw_wdt_get_min_timeout(struct dw_wdt *dw_wdt)
>  {
> -	int top = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET) & 0xF;
> +	int idx;
> +
> +	/*
> +	 * We'll find a timeout greater or equal to one second anyway because
> +	 * the driver probe would have failed if there was none.
> +	 */
> +	for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) {
> +		if (dw_wdt->timeouts[idx].sec)
> +			break;
> +	}
>  
> -	return dw_wdt_top_in_seconds(dw_wdt, top);
> +	return dw_wdt->timeouts[idx].sec;
> +}
> +
> +static unsigned int dw_wdt_get_max_timeout_ms(struct dw_wdt *dw_wdt)
> +{
> +	struct dw_wdt_timeout *timeout = &dw_wdt->timeouts[DW_WDT_NUM_TOPS - 1];
> +	u64 msec;
> +
> +	msec = (u64)timeout->sec * MSEC_PER_SEC + timeout->msec;
> +
> +	return msec < UINT_MAX ? msec : UINT_MAX;
> +}
> +
> +static unsigned int dw_wdt_get_timeout(struct dw_wdt *dw_wdt)
> +{
> +	int top_val = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET) & 0xF;
> +	int idx;
> +
> +	for (idx = 0; idx < DW_WDT_NUM_TOPS; ++idx) {
> +		if (dw_wdt->timeouts[idx].top_val == top_val)
> +			break;
> +	}
> +
> +	return dw_wdt->timeouts[idx].sec;
>  }
>  
>  static int dw_wdt_ping(struct watchdog_device *wdd)
> @@ -93,17 +160,10 @@ static int dw_wdt_ping(struct watchdog_device *wdd)
>  static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
>  {
>  	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
> -	int i, top_val = DW_WDT_MAX_TOP;
> +	unsigned int timeout;
> +	u32 top_val;
>  
> -	/*
> -	 * Iterate over the timeout values until we find the closest match. We
> -	 * always look for >=.
> -	 */
> -	for (i = 0; i <= DW_WDT_MAX_TOP; ++i)
> -		if (dw_wdt_top_in_seconds(dw_wdt, i) >= top_s) {
> -			top_val = i;
> -			break;
> -		}
> +	timeout = dw_wdt_find_best_top(dw_wdt, top_s, &top_val);
>  
>  	/*
>  	 * Set the new value in the watchdog.  Some versions of dw_wdt
> @@ -120,7 +180,7 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
>  	 * wdd->max_hw_heartbeat_ms
>  	 */
>  	if (top_s * 1000 <= wdd->max_hw_heartbeat_ms)
> -		wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
> +		wdd->timeout = timeout;
>  	else
>  		wdd->timeout = top_s;
>  
> @@ -238,6 +298,86 @@ static int dw_wdt_resume(struct device *dev)
>  
>  static SIMPLE_DEV_PM_OPS(dw_wdt_pm_ops, dw_wdt_suspend, dw_wdt_resume);
>  
> +/*
> + * In case if DW WDT IP core is synthesized with fixed TOP feature disabled the
> + * TOPs array can be arbitrary ordered with nearly any sixteen uint numbers
> + * depending on the system engineer imagination. The next method handles the
> + * passed TOPs array to pre-calculate the effective timeouts and to sort the
> + * TOP items out in the ascending order with respect to the timeouts.
> + */
> +
> +static void dw_wdt_handle_tops(struct dw_wdt *dw_wdt, const u32 *tops)
> +{
> +	struct dw_wdt_timeout tout, *dst;
> +	int val, tidx;
> +	u64 msec;
> +
> +	/*
> +	 * We walk over the passed TOPs array and calculate corresponding
> +	 * timeouts in seconds and milliseconds. The milliseconds granularity
> +	 * is needed to distinguish the TOPs with very close timeouts and to
> +	 * set the watchdog max heartbeat setting further.
> +	 */
> +	for (val = 0; val < DW_WDT_NUM_TOPS; ++val) {
> +		tout.top_val = val;
> +		tout.sec = tops[val] / dw_wdt->rate;
> +		msec = (u64)tops[val] * MSEC_PER_SEC;
> +		do_div(msec, dw_wdt->rate);
> +		tout.msec = msec - ((u64)tout.sec * MSEC_PER_SEC);
> +
> +		/*
> +		 * Find a suitable place for the current TOP in the timeouts
> +		 * array so that the list is remained in the ascending order.
> +		 */
> +		for (tidx = 0; tidx < val; ++tidx) {
> +			dst = &dw_wdt->timeouts[tidx];
> +			if (tout.sec > dst->sec || (tout.sec == dst->sec &&
> +			    tout.msec >= dst->msec))
> +				continue;
> +			else
> +				swap(*dst, tout);
> +		}
> +
> +		dw_wdt->timeouts[val] = tout;
> +	}
> +}
> +
> +static int dw_wdt_init_timeouts(struct dw_wdt *dw_wdt, struct device *dev)
> +{
> +	u32 data, of_tops[DW_WDT_NUM_TOPS];
> +	const u32 *tops;
> +	int ret;
> +
> +	/*
> +	 * Retrieve custom or fixed counter values depending on the
> +	 * WDT_USE_FIX_TOP flag found in the component specific parameters
> +	 * #1 register.
> +	 */
> +	data = readl(dw_wdt->regs + WDOG_COMP_PARAMS_1_REG_OFFSET);
> +	if (data & WDOG_COMP_PARAMS_1_USE_FIX_TOP) {
> +		tops = dw_wdt_fix_tops;
> +	} else {
> +		ret = of_property_read_variable_u32_array(dev_of_node(dev),
> +			"snps,watchdog-tops", of_tops, DW_WDT_NUM_TOPS,
> +			DW_WDT_NUM_TOPS);
> +		if (ret < 0) {
> +			dev_warn(dev, "No valid TOPs array specified\n");
> +			tops = dw_wdt_fix_tops;
> +		} else {
> +			tops = of_tops;
> +		}
> +	}
> +
> +	/* Convert the specified TOPs into an array of watchdog timeouts. */
> +	dw_wdt_handle_tops(dw_wdt, tops);
> +	if (!dw_wdt->timeouts[DW_WDT_NUM_TOPS - 1].sec) {
> +		dev_err(dev, "No any valid TOP detected\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int dw_wdt_drv_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -275,12 +415,15 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  
>  	reset_control_deassert(dw_wdt->rst);
>  
> +	ret = dw_wdt_init_timeouts(dw_wdt, dev);
> +	if (ret)
> +		goto out_disable_clk;
> +
>  	wdd = &dw_wdt->wdd;
>  	wdd->info = &dw_wdt_ident;
>  	wdd->ops = &dw_wdt_ops;
> -	wdd->min_timeout = 1;
> -	wdd->max_hw_heartbeat_ms =
> -		dw_wdt_top_in_seconds(dw_wdt, DW_WDT_MAX_TOP) * 1000;
> +	wdd->min_timeout = dw_wdt_get_min_timeout(dw_wdt);
> +	wdd->max_hw_heartbeat_ms = dw_wdt_get_max_timeout_ms(dw_wdt);
>  	wdd->parent = dev;
>  
>  	watchdog_set_drvdata(wdd, dw_wdt);
> @@ -293,7 +436,7 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  	 * devicetree.
>  	 */
>  	if (dw_wdt_is_enabled(dw_wdt)) {
> -		wdd->timeout = dw_wdt_get_top(dw_wdt);
> +		wdd->timeout = dw_wdt_get_timeout(dw_wdt);
>  		set_bit(WDOG_HW_RUNNING, &wdd->status);
>  	} else {
>  		wdd->timeout = DW_WDT_DEFAULT_SECONDS;

^ permalink raw reply

* Re: [PATCH v3 5/7] watchdog: dw_wdt: Support devices with asynch clocks
From: Guenter Roeck @ 2020-05-29 22:52 UTC (permalink / raw)
  To: Serge Semin
  Cc: Wim Van Sebroeck, Serge Semin, Alexey Malahov,
	Thomas Bogendoerfer, Arnd Bergmann, Rob Herring, linux-mips,
	devicetree, linux-watchdog, linux-kernel
In-Reply-To: <20200526154123.24402-6-Sergey.Semin@baikalelectronics.ru>

On Tue, May 26, 2020 at 06:41:21PM +0300, Serge Semin wrote:
> DW Watchdog IP core can be synthesised with asynchronous timer/APB
> clocks support (WDT_ASYNC_CLK_MODE_ENABLE == 1). In this case
> separate clock signals are supposed to be used to feed watchdog timer
> and APB interface of the device. Currently the driver supports
> the synchronous mode only. Since there is no way to determine which
> mode was actually activated for device from its registers, we have to
> rely on the platform device configuration data. If optional "pclk"
> clock source is supplied, we consider the device working in asynchronous
> mode, otherwise the driver falls back to the synchronous configuration.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: devicetree@vger.kernel.org

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changelog v2:
> - Rearrange SoBs.
> ---
>  drivers/watchdog/dw_wdt.c | 48 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 43 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> index 693c0d1fd796..efbc36872670 100644
> --- a/drivers/watchdog/dw_wdt.c
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -68,6 +68,7 @@ struct dw_wdt_timeout {
>  struct dw_wdt {
>  	void __iomem		*regs;
>  	struct clk		*clk;
> +	struct clk		*pclk;
>  	unsigned long		rate;
>  	struct dw_wdt_timeout	timeouts[DW_WDT_NUM_TOPS];
>  	struct watchdog_device	wdd;
> @@ -274,6 +275,7 @@ static int dw_wdt_suspend(struct device *dev)
>  	dw_wdt->control = readl(dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
>  	dw_wdt->timeout = readl(dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
>  
> +	clk_disable_unprepare(dw_wdt->pclk);
>  	clk_disable_unprepare(dw_wdt->clk);
>  
>  	return 0;
> @@ -287,6 +289,12 @@ static int dw_wdt_resume(struct device *dev)
>  	if (err)
>  		return err;
>  
> +	err = clk_prepare_enable(dw_wdt->pclk);
> +	if (err) {
> +		clk_disable_unprepare(dw_wdt->clk);
> +		return err;
> +	}
> +
>  	writel(dw_wdt->timeout, dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
>  	writel(dw_wdt->control, dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
>  
> @@ -393,9 +401,18 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  	if (IS_ERR(dw_wdt->regs))
>  		return PTR_ERR(dw_wdt->regs);
>  
> -	dw_wdt->clk = devm_clk_get(dev, NULL);
> -	if (IS_ERR(dw_wdt->clk))
> -		return PTR_ERR(dw_wdt->clk);
> +	/*
> +	 * Try to request the watchdog dedicated timer clock source. It must
> +	 * be supplied if asynchronous mode is enabled. Otherwise fallback
> +	 * to the common timer/bus clocks configuration, in which the very
> +	 * first found clock supply both timer and APB signals.
> +	 */
> +	dw_wdt->clk = devm_clk_get(dev, "tclk");
> +	if (IS_ERR(dw_wdt->clk)) {
> +		dw_wdt->clk = devm_clk_get(dev, NULL);
> +		if (IS_ERR(dw_wdt->clk))
> +			return PTR_ERR(dw_wdt->clk);
> +	}
>  
>  	ret = clk_prepare_enable(dw_wdt->clk);
>  	if (ret)
> @@ -407,10 +424,27 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  		goto out_disable_clk;
>  	}
>  
> +	/*
> +	 * Request APB clock if device is configured with async clocks mode.
> +	 * In this case both tclk and pclk clocks are supposed to be specified.
> +	 * Alas we can't know for sure whether async mode was really activated,
> +	 * so the pclk phandle reference is left optional. If it couldn't be
> +	 * found we consider the device configured in synchronous clocks mode.
> +	 */
> +	dw_wdt->pclk = devm_clk_get_optional(dev, "pclk");
> +	if (IS_ERR(dw_wdt->pclk)) {
> +		ret = PTR_ERR(dw_wdt->pclk);
> +		goto out_disable_clk;
> +	}
> +
> +	ret = clk_prepare_enable(dw_wdt->pclk);
> +	if (ret)
> +		goto out_disable_clk;
> +
>  	dw_wdt->rst = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
>  	if (IS_ERR(dw_wdt->rst)) {
>  		ret = PTR_ERR(dw_wdt->rst);
> -		goto out_disable_clk;
> +		goto out_disable_pclk;
>  	}
>  
>  	reset_control_deassert(dw_wdt->rst);
> @@ -449,10 +483,13 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  
>  	ret = watchdog_register_device(wdd);
>  	if (ret)
> -		goto out_disable_clk;
> +		goto out_disable_pclk;
>  
>  	return 0;
>  
> +out_disable_pclk:
> +	clk_disable_unprepare(dw_wdt->pclk);
> +
>  out_disable_clk:
>  	clk_disable_unprepare(dw_wdt->clk);
>  	return ret;
> @@ -464,6 +501,7 @@ static int dw_wdt_drv_remove(struct platform_device *pdev)
>  
>  	watchdog_unregister_device(&dw_wdt->wdd);
>  	reset_control_assert(dw_wdt->rst);
> +	clk_disable_unprepare(dw_wdt->pclk);
>  	clk_disable_unprepare(dw_wdt->clk);
>  
>  	return 0;

^ permalink raw reply

* Re: [PATCH v3 1/7] dt-bindings: watchdog: Convert DW WDT binding to DT schema
From: Guenter Roeck @ 2020-05-29 22:53 UTC (permalink / raw)
  To: Serge Semin
  Cc: Wim Van Sebroeck, Rob Herring, Serge Semin, Rob Herring,
	Alexey Malahov, Thomas Bogendoerfer, Arnd Bergmann, linux-mips,
	linux-watchdog, devicetree, linux-kernel
In-Reply-To: <20200526154123.24402-2-Sergey.Semin@baikalelectronics.ru>

On Tue, May 26, 2020 at 06:41:17PM +0300, Serge Semin wrote:
> Modern device tree bindings are supposed to be created as YAML-files
> in accordance with dt-schema. This commit replaces the DW Watchdog
> legacy bare text bindings with YAML file. As before the binding states
> that the corresponding dts node is supposed to have a registers
> range, a watchdog timer references clock source, optional reset line and
> pre-timeout interrupt.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-mips@vger.kernel.org

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changelog v2:
> - Rearrange SoBs.
> - Discard BE copyright header.
> - Replace "additionalProperties: false" with "unevaluatedProperties: false"
>   property.
> - Discard interrupts property from the required properties list.
> - Remove a label definition from the binding example.
> - Move the asynchronous APB3 clock support into a dedicated patch.
> ---
>  .../devicetree/bindings/watchdog/dw_wdt.txt   | 24 ---------
>  .../bindings/watchdog/snps,dw-wdt.yaml        | 50 +++++++++++++++++++
>  2 files changed, 50 insertions(+), 24 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/watchdog/dw_wdt.txt
>  create mode 100644 Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/dw_wdt.txt b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
> deleted file mode 100644
> index eb0914420c7c..000000000000
> --- a/Documentation/devicetree/bindings/watchdog/dw_wdt.txt
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -Synopsys Designware Watchdog Timer
> -
> -Required Properties:
> -
> -- compatible	: Should contain "snps,dw-wdt"
> -- reg		: Base address and size of the watchdog timer registers.
> -- clocks	: phandle + clock-specifier for the clock that drives the
> -		watchdog timer.
> -
> -Optional Properties:
> -
> -- interrupts	: The interrupt used for the watchdog timeout warning.
> -- resets	: phandle pointing to the system reset controller with
> -		line index for the watchdog.
> -
> -Example:
> -
> -	watchdog0: wd@ffd02000 {
> -		compatible = "snps,dw-wdt";
> -		reg = <0xffd02000 0x1000>;
> -		interrupts = <0 171 4>;
> -		clocks = <&per_base_clk>;
> -		resets = <&rst WDT0_RESET>;
> -	};
> diff --git a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> new file mode 100644
> index 000000000000..4f6944756ab4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/watchdog/snps,dw-wdt.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys Designware Watchdog Timer
> +
> +allOf:
> +  - $ref: "watchdog.yaml#"
> +
> +maintainers:
> +  - Jamie Iles <jamie@jamieiles.com>
> +
> +properties:
> +  compatible:
> +    const: snps,dw-wdt
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    description: DW Watchdog pre-timeout interrupt
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: Watchdog timer reference clock
> +
> +  resets:
> +    description: Phandle to the DW Watchdog reset lane
> +    maxItems: 1
> +
> +unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +examples:
> +  - |
> +    watchdog@ffd02000 {
> +      compatible = "snps,dw-wdt";
> +      reg = <0xffd02000 0x1000>;
> +      interrupts = <0 171 4>;
> +      clocks = <&per_base_clk>;
> +      resets = <&wdt_rst>;
> +    };
> +...

^ permalink raw reply

* Re: [PATCH v3 2/7] dt-bindings: watchdog: dw-wdt: Support devices with asynch clocks
From: Guenter Roeck @ 2020-05-29 22:53 UTC (permalink / raw)
  To: Serge Semin
  Cc: Wim Van Sebroeck, Rob Herring, Serge Semin, Rob Herring,
	Alexey Malahov, Thomas Bogendoerfer, Arnd Bergmann, linux-mips,
	linux-watchdog, devicetree, linux-kernel
In-Reply-To: <20200526154123.24402-3-Sergey.Semin@baikalelectronics.ru>

On Tue, May 26, 2020 at 06:41:18PM +0300, Serge Semin wrote:
> DW Watchdog IP core can be synthesised with asynchronous timer/APB
> clocks support (WDT_ASYNC_CLK_MODE_ENABLE == 1). In this case
> separate clock signals are supposed to be used to feed watchdog timer
> and APB interface of the device. Let's update the DW Watchdog DT node
> schema so it would support the optional APB3 bus clock specified along
> with the mandatory watchdog timer reference clock.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-mips@vger.kernel.org

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changelog v2:
> - It's a new patch unpinned from the previous one.
> ---
>  .../devicetree/bindings/watchdog/snps,dw-wdt.yaml         | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> index 4f6944756ab4..5bf6dc6377f3 100644
> --- a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> @@ -24,8 +24,16 @@ properties:
>      maxItems: 1
>  
>    clocks:
> +    minItems: 1
>      items:
>        - description: Watchdog timer reference clock
> +      - description: APB3 interface clock
> +
> +  clock-names:
> +    minItems: 1
> +    items:
> +      - const: tclk
> +      - const: pclk
>  
>    resets:
>      description: Phandle to the DW Watchdog reset lane

^ permalink raw reply

* Re: [PATCH v3 3/7] dt-bindings: watchdog: dw-wdt: Add watchdog TOPs array property
From: Guenter Roeck @ 2020-05-29 22:54 UTC (permalink / raw)
  To: Serge Semin
  Cc: Wim Van Sebroeck, Rob Herring, Serge Semin, Rob Herring,
	Alexey Malahov, Thomas Bogendoerfer, Arnd Bergmann, linux-mips,
	linux-watchdog, devicetree, linux-kernel
In-Reply-To: <20200526154123.24402-4-Sergey.Semin@baikalelectronics.ru>

On Tue, May 26, 2020 at 06:41:19PM +0300, Serge Semin wrote:
> In case if DW Watchdog IP core is built with WDT_USE_FIX_TOP == false,
> a custom timeout periods are used to preset the timer counter. In
> this case that periods should be specified in a new "snps,watchdog-tops"
> property of the DW watchdog dts node.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: linux-mips@vger.kernel.org

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changelog v2:
> - Rearrange SoBs.
> - Move $ref to the root level of the "snps,watchdog-tops" property
>   so does the constraints.
> - Add default TOP values array.
> - Discard the label definition from the new bindings example.
> 
> Changelog v3:
> - Remove items property and move the minItems and maxItems constraints to
>   the root level of the snps,watchdog-tops property.
> ---
>  .../bindings/watchdog/snps,dw-wdt.yaml        | 32 +++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> index 5bf6dc6377f3..d9fc7bb851b1 100644
> --- a/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> +++ b/Documentation/devicetree/bindings/watchdog/snps,dw-wdt.yaml
> @@ -39,6 +39,23 @@ properties:
>      description: Phandle to the DW Watchdog reset lane
>      maxItems: 1
>  
> +  snps,watchdog-tops:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description: |
> +      DW APB Watchdog custom timer intervals - Timeout Period ranges (TOPs).
> +      Each TOP is a number loaded into the watchdog counter at the moment of
> +      the timer restart. The counter decrementing happens each tick of the
> +      reference clock. Therefore the TOPs array is equivalent to an array of
> +      the timer expiration intervals supported by the DW APB Watchdog. Note
> +      DW APB Watchdog IP-core might be synthesized with fixed TOP values,
> +      in which case this property is unnecessary with default TOPs utilized.
> +    default: [0x0001000 0x0002000 0x0004000 0x0008000
> +      0x0010000 0x0020000 0x0040000 0x0080000
> +      0x0100000 0x0200000 0x0400000 0x0800000
> +      0x1000000 0x2000000 0x4000000 0x8000000]
> +    minItems: 16
> +    maxItems: 16
> +
>  unevaluatedProperties: false
>  
>  required:
> @@ -55,4 +72,19 @@ examples:
>        clocks = <&per_base_clk>;
>        resets = <&wdt_rst>;
>      };
> +
> +  - |
> +    watchdog@ffd02000 {
> +      compatible = "snps,dw-wdt";
> +      reg = <0xffd02000 0x1000>;
> +      interrupts = <0 171 4>;
> +      clocks = <&per_base_clk>;
> +      clock-names = "tclk";
> +      snps,watchdog-tops = <0x000000FF 0x000001FF 0x000003FF
> +                            0x000007FF 0x0000FFFF 0x0001FFFF
> +                            0x0003FFFF 0x0007FFFF 0x000FFFFF
> +                            0x001FFFFF 0x003FFFFF 0x007FFFFF
> +                            0x00FFFFFF 0x01FFFFFF 0x03FFFFFF
> +                            0x07FFFFFF>;
> +    };
>  ...

^ permalink raw reply

* Re: [PATCH v3 6/7] watchdog: dw_wdt: Add pre-timeouts support
From: Guenter Roeck @ 2020-05-29 23:02 UTC (permalink / raw)
  To: Serge Semin
  Cc: Wim Van Sebroeck, Serge Semin, Alexey Malahov,
	Thomas Bogendoerfer, Arnd Bergmann, Rob Herring, linux-mips,
	devicetree, linux-watchdog, linux-kernel
In-Reply-To: <20200526154123.24402-7-Sergey.Semin@baikalelectronics.ru>

On Tue, May 26, 2020 at 06:41:22PM +0300, Serge Semin wrote:
> DW Watchdog can rise an interrupt in case if IRQ request mode is enabled
> and timer reaches the zero value. In this case the IRQ lane is left
> pending until either the next watchdog kick event (watchdog restart) or
> until the WDT_EOI register is read or the device/system reset. This
> interface can be used to implement the pre-timeout functionality
> optionally provided by the Linux kernel watchdog devices.
> 
> IRQ mode provides a two stages timeout interface. It means the IRQ is
> raised when the counter reaches zero, while the system reset occurs only
> after subsequent timeout if the timer restart is not performed. Due to
> this peculiarity the pre-timeout value is actually set to the achieved
> hardware timeout, while the real watchdog timeout is considered to be
> twice as much of it. This applies a significant limitation on the
> pre-timeout values, so current implementation supports either zero value,
> which disables the pre-timeout events, or non-zero values, which imply
> the pre-timeout to be at least half of the current watchdog timeout.
> 
> Note that we ask the interrupt controller to detect the rising-edge
> pre-timeout interrupts to prevent the high-level-IRQs flood, since
> if the pre-timeout happens, the IRQ lane will be left pending until
> it's cleared by the timer restart.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: devicetree@vger.kernel.org

Nitpick below, but I don't really know what to do about it, so

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changelog v2:
> - Rearrange SoBs.
> - Make the Pre-timeout IRQ being optionally supported.
> ---
>  drivers/watchdog/dw_wdt.c | 138 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 130 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> index efbc36872670..3cd7c485cd70 100644
> --- a/drivers/watchdog/dw_wdt.c
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -22,6 +22,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
> +#include <linux/interrupt.h>
>  #include <linux/of.h>
>  #include <linux/pm.h>
>  #include <linux/platform_device.h>
> @@ -36,6 +37,8 @@
>  #define WDOG_CURRENT_COUNT_REG_OFFSET	    0x08
>  #define WDOG_COUNTER_RESTART_REG_OFFSET     0x0c
>  #define WDOG_COUNTER_RESTART_KICK_VALUE	    0x76
> +#define WDOG_INTERRUPT_STATUS_REG_OFFSET    0x10
> +#define WDOG_INTERRUPT_CLEAR_REG_OFFSET     0x14
>  #define WDOG_COMP_PARAMS_1_REG_OFFSET       0xf4
>  #define WDOG_COMP_PARAMS_1_USE_FIX_TOP      BIT(6)
>  
> @@ -59,6 +62,11 @@ module_param(nowayout, bool, 0);
>  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started "
>  		 "(default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
>  
> +enum dw_wdt_rmod {
> +	DW_WDT_RMOD_RESET = 1,
> +	DW_WDT_RMOD_IRQ = 2
> +};
> +
>  struct dw_wdt_timeout {
>  	u32 top_val;
>  	unsigned int sec;
> @@ -70,6 +78,7 @@ struct dw_wdt {
>  	struct clk		*clk;
>  	struct clk		*pclk;
>  	unsigned long		rate;
> +	enum dw_wdt_rmod	rmod;
>  	struct dw_wdt_timeout	timeouts[DW_WDT_NUM_TOPS];
>  	struct watchdog_device	wdd;
>  	struct reset_control	*rst;
> @@ -86,6 +95,20 @@ static inline int dw_wdt_is_enabled(struct dw_wdt *dw_wdt)
>  		WDOG_CONTROL_REG_WDT_EN_MASK;
>  }
>  
> +static void dw_wdt_update_mode(struct dw_wdt *dw_wdt, enum dw_wdt_rmod rmod)
> +{
> +	u32 val;
> +
> +	val = readl(dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
> +	if (rmod == DW_WDT_RMOD_IRQ)
> +		val |= WDOG_CONTROL_REG_RESP_MODE_MASK;
> +	else
> +		val &= ~WDOG_CONTROL_REG_RESP_MODE_MASK;
> +	writel(val, dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
> +
> +	dw_wdt->rmod = rmod;
> +}
> +
>  static unsigned int dw_wdt_find_best_top(struct dw_wdt *dw_wdt,
>  					 unsigned int timeout, u32 *top_val)
>  {
> @@ -145,7 +168,11 @@ static unsigned int dw_wdt_get_timeout(struct dw_wdt *dw_wdt)
>  			break;
>  	}
>  
> -	return dw_wdt->timeouts[idx].sec;
> +	/*
> +	 * In IRQ mode due to the two stages counter, the actual timeout is
> +	 * twice greater than the TOP setting.
> +	 */
> +	return dw_wdt->timeouts[idx].sec * dw_wdt->rmod;
>  }
>  
>  static int dw_wdt_ping(struct watchdog_device *wdd)
> @@ -164,7 +191,20 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
>  	unsigned int timeout;
>  	u32 top_val;
>  
> -	timeout = dw_wdt_find_best_top(dw_wdt, top_s, &top_val);
> +	/*
> +	 * Note IRQ mode being enabled means having a non-zero pre-timeout
> +	 * setup. In this case we try to find a TOP as close to the half of the
> +	 * requested timeout as possible since DW Watchdog IRQ mode is designed
> +	 * in two stages way - first timeout rises the pre-timeout interrupt,
> +	 * second timeout performs the system reset. So basically the effective
> +	 * watchdog-caused reset happens after two watchdog TOPs elapsed.
> +	 */
> +	timeout = dw_wdt_find_best_top(dw_wdt, DIV_ROUND_UP(top_s, dw_wdt->rmod),
> +				       &top_val);
> +	if (dw_wdt->rmod == DW_WDT_RMOD_IRQ)
> +		wdd->pretimeout = timeout;
> +	else
> +		wdd->pretimeout = 0;
>  
>  	/*
>  	 * Set the new value in the watchdog.  Some versions of dw_wdt
> @@ -175,25 +215,47 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
>  	writel(top_val | top_val << WDOG_TIMEOUT_RANGE_TOPINIT_SHIFT,
>  	       dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
>  
> +	/* Kick new TOP value into the watchdog counter if activated. */
> +	if (watchdog_active(wdd))
> +		dw_wdt_ping(wdd);
> +
>  	/*
>  	 * In case users set bigger timeout value than HW can support,
>  	 * kernel(watchdog_dev.c) helps to feed watchdog before
>  	 * wdd->max_hw_heartbeat_ms
>  	 */
>  	if (top_s * 1000 <= wdd->max_hw_heartbeat_ms)
> -		wdd->timeout = timeout;
> +		wdd->timeout = timeout * dw_wdt->rmod;
>  	else
>  		wdd->timeout = top_s;
>  
>  	return 0;
>  }
>  
> +static int dw_wdt_set_pretimeout(struct watchdog_device *wdd, unsigned int req)
> +{
> +	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
> +
> +	/*
> +	 * We ignore actual value of the timeout passed from user-space
> +	 * using it as a flag whether the pretimeout functionality is intended
> +	 * to be activated.
> +	 */
> +	dw_wdt_update_mode(dw_wdt, req ? DW_WDT_RMOD_IRQ : DW_WDT_RMOD_RESET);
> +	dw_wdt_set_timeout(wdd, wdd->timeout);
> +
> +	return 0;
> +}
> +
>  static void dw_wdt_arm_system_reset(struct dw_wdt *dw_wdt)
>  {
>  	u32 val = readl(dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
>  
> -	/* Disable interrupt mode; always perform system reset. */
> -	val &= ~WDOG_CONTROL_REG_RESP_MODE_MASK;
> +	/* Disable/enable interrupt mode depending on the RMOD flag. */
> +	if (dw_wdt->rmod == DW_WDT_RMOD_IRQ)
> +		val |= WDOG_CONTROL_REG_RESP_MODE_MASK;
> +	else
> +		val &= ~WDOG_CONTROL_REG_RESP_MODE_MASK;
>  	/* Enable watchdog. */
>  	val |= WDOG_CONTROL_REG_WDT_EN_MASK;
>  	writel(val, dw_wdt->regs + WDOG_CONTROL_REG_OFFSET);
> @@ -231,6 +293,7 @@ static int dw_wdt_restart(struct watchdog_device *wdd,
>  	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
>  
>  	writel(0, dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
> +	dw_wdt_update_mode(dw_wdt, DW_WDT_RMOD_RESET);
>  	if (dw_wdt_is_enabled(dw_wdt))
>  		writel(WDOG_COUNTER_RESTART_KICK_VALUE,
>  		       dw_wdt->regs + WDOG_COUNTER_RESTART_REG_OFFSET);
> @@ -246,9 +309,19 @@ static int dw_wdt_restart(struct watchdog_device *wdd,
>  static unsigned int dw_wdt_get_timeleft(struct watchdog_device *wdd)
>  {
>  	struct dw_wdt *dw_wdt = to_dw_wdt(wdd);
> +	unsigned int sec;
> +	u32 val;
> +
> +	val = readl(dw_wdt->regs + WDOG_CURRENT_COUNT_REG_OFFSET);
> +	sec = val / dw_wdt->rate;
>  
> -	return readl(dw_wdt->regs + WDOG_CURRENT_COUNT_REG_OFFSET) /
> -		dw_wdt->rate;
> +	if (dw_wdt->rmod == DW_WDT_RMOD_IRQ) {
> +		val = readl(dw_wdt->regs + WDOG_INTERRUPT_STATUS_REG_OFFSET);
> +		if (!val)
> +			sec += wdd->pretimeout;
> +	}
> +
> +	return sec;
>  }
>  
>  static const struct watchdog_info dw_wdt_ident = {
> @@ -257,16 +330,41 @@ static const struct watchdog_info dw_wdt_ident = {
>  	.identity	= "Synopsys DesignWare Watchdog",
>  };
>  
> +static const struct watchdog_info dw_wdt_pt_ident = {
> +	.options	= WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT |
> +			  WDIOF_PRETIMEOUT | WDIOF_MAGICCLOSE,
> +	.identity	= "Synopsys DesignWare Watchdog",
> +};
> +
>  static const struct watchdog_ops dw_wdt_ops = {
>  	.owner		= THIS_MODULE,
>  	.start		= dw_wdt_start,
>  	.stop		= dw_wdt_stop,
>  	.ping		= dw_wdt_ping,
>  	.set_timeout	= dw_wdt_set_timeout,
> +	.set_pretimeout	= dw_wdt_set_pretimeout,
>  	.get_timeleft	= dw_wdt_get_timeleft,
>  	.restart	= dw_wdt_restart,
>  };
>  
> +static irqreturn_t dw_wdt_irq(int irq, void *devid)
> +{
> +	struct dw_wdt *dw_wdt = devid;
> +	u32 val;
> +
> +	/*
> +	 * We don't clear the IRQ status. It's supposed to be done by the
> +	 * following ping operations.
> +	 */
> +	val = readl(dw_wdt->regs + WDOG_INTERRUPT_STATUS_REG_OFFSET);
> +	if (!val)
> +		return IRQ_NONE;
> +
> +	watchdog_notify_pretimeout(&dw_wdt->wdd);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  #ifdef CONFIG_PM_SLEEP
>  static int dw_wdt_suspend(struct device *dev)
>  {
> @@ -447,6 +545,31 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  		goto out_disable_pclk;
>  	}
>  
> +	/* Enable normal reset without pre-timeout by default. */
> +	dw_wdt_update_mode(dw_wdt, DW_WDT_RMOD_RESET);
> +
> +	/*
> +	 * Pre-timeout IRQ is optional, since some hardware may lack support
> +	 * of it. Note we must request rising-edge IRQ, since the lane is left
> +	 * pending either until the next watchdog kick event or up to the
> +	 * system reset.
> +	 */
> +	ret = platform_get_irq_optional(pdev, 0);
> +	if (ret >= 0) {

I keep seeing notes that an interrupt value of 0 is invalid.

> +		ret = devm_request_irq(dev, ret, dw_wdt_irq,
> +				       IRQF_SHARED | IRQF_TRIGGER_RISING,
> +				       pdev->name, dw_wdt);
> +		if (ret)
> +			goto out_disable_pclk;
> +
> +		dw_wdt->wdd.info = &dw_wdt_pt_ident;
> +	} else {
> +		if (ret == -EPROBE_DEFER)
> +			goto out_disable_pclk;
> +
> +		dw_wdt->wdd.info = &dw_wdt_ident;
> +	}
> +
>  	reset_control_deassert(dw_wdt->rst);
>  
>  	ret = dw_wdt_init_timeouts(dw_wdt, dev);
> @@ -454,7 +577,6 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  		goto out_disable_clk;
>  
>  	wdd = &dw_wdt->wdd;
> -	wdd->info = &dw_wdt_ident;
>  	wdd->ops = &dw_wdt_ops;
>  	wdd->min_timeout = dw_wdt_get_min_timeout(dw_wdt);
>  	wdd->max_hw_heartbeat_ms = dw_wdt_get_max_timeout_ms(dw_wdt);

^ permalink raw reply

* Re: [PATCH] i2c: add 'single-master' property to generic bindings
From: Wolfram Sang @ 2020-05-29 23:02 UTC (permalink / raw)
  To: Rob Herring; +Cc: linux-i2c, devicetree, Laine Jaakko EXT
In-Reply-To: <20200529220228.GA3052199@bogus>

[-- Attachment #1: Type: text/plain, Size: 790 bytes --]

Hi Rob,

thanks for the review!

> Could you just have different timeouts for clearing stalled bus. You 
> know quickly if 'single-master' is set, but have to wait longer if not?

Timeouts are a difficult topic with I2C; there is no timeout defined.
However, if you want to start communictaing and don't have a 'bus idle'
condition, then the new property makes a difference. With
"single-master", we know the bus is stalled. With "multi-master" it
could be another master communicating.

> Note that we need to add a bunch of these properties to dt-schema 
> i2c-controller.yaml. I hadn't done that because I want to dual license 
> in the process, but lots of folks have touched i2c.txt IIRC.

What is your motivation for dual licensing?

All the best,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v3 7/7] watchdog: dw_wdt: Add DebugFS files
From: Guenter Roeck @ 2020-05-29 23:03 UTC (permalink / raw)
  To: Serge Semin
  Cc: Wim Van Sebroeck, Serge Semin, Alexey Malahov,
	Thomas Bogendoerfer, Arnd Bergmann, Rob Herring, linux-mips,
	devicetree, linux-watchdog, linux-kernel
In-Reply-To: <20200526154123.24402-8-Sergey.Semin@baikalelectronics.ru>

On Tue, May 26, 2020 at 06:41:23PM +0300, Serge Semin wrote:
> For the sake of the easier device-driver debug procedure, we added a
> DebugFS file with the controller registers state. It's available only if
> kernel is configured with DebugFS support.
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: devicetree@vger.kernel.org

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> 
> Changelog v2:
> - Rearrange SoBs.
> - Discard timeout/pretimeout/ping/enable DebugFS nodes. Registers state
>   dump node is only left.
> ---
>  drivers/watchdog/dw_wdt.c | 68 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> index 3cd7c485cd70..012681baaa6d 100644
> --- a/drivers/watchdog/dw_wdt.c
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -28,6 +28,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/reset.h>
>  #include <linux/watchdog.h>
> +#include <linux/debugfs.h>
>  
>  #define WDOG_CONTROL_REG_OFFSET		    0x00
>  #define WDOG_CONTROL_REG_WDT_EN_MASK	    0x01
> @@ -39,8 +40,14 @@
>  #define WDOG_COUNTER_RESTART_KICK_VALUE	    0x76
>  #define WDOG_INTERRUPT_STATUS_REG_OFFSET    0x10
>  #define WDOG_INTERRUPT_CLEAR_REG_OFFSET     0x14
> +#define WDOG_COMP_PARAMS_5_REG_OFFSET       0xe4
> +#define WDOG_COMP_PARAMS_4_REG_OFFSET       0xe8
> +#define WDOG_COMP_PARAMS_3_REG_OFFSET       0xec
> +#define WDOG_COMP_PARAMS_2_REG_OFFSET       0xf0
>  #define WDOG_COMP_PARAMS_1_REG_OFFSET       0xf4
>  #define WDOG_COMP_PARAMS_1_USE_FIX_TOP      BIT(6)
> +#define WDOG_COMP_VERSION_REG_OFFSET        0xf8
> +#define WDOG_COMP_TYPE_REG_OFFSET           0xfc
>  
>  /* There are sixteen TOPs (timeout periods) that can be set in the watchdog. */
>  #define DW_WDT_NUM_TOPS		16
> @@ -85,6 +92,10 @@ struct dw_wdt {
>  	/* Save/restore */
>  	u32			control;
>  	u32			timeout;
> +
> +#ifdef CONFIG_DEBUG_FS
> +	struct dentry		*dbgfs_dir;
> +#endif
>  };
>  
>  #define to_dw_wdt(wdd)	container_of(wdd, struct dw_wdt, wdd)
> @@ -484,6 +495,59 @@ static int dw_wdt_init_timeouts(struct dw_wdt *dw_wdt, struct device *dev)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_DEBUG_FS
> +
> +#define DW_WDT_DBGFS_REG(_name, _off) \
> +{				      \
> +	.name = _name,		      \
> +	.offset = _off		      \
> +}
> +
> +static const struct debugfs_reg32 dw_wdt_dbgfs_regs[] = {
> +	DW_WDT_DBGFS_REG("cr", WDOG_CONTROL_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("torr", WDOG_TIMEOUT_RANGE_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("ccvr", WDOG_CURRENT_COUNT_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("crr", WDOG_COUNTER_RESTART_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("stat", WDOG_INTERRUPT_STATUS_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("param5", WDOG_COMP_PARAMS_5_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("param4", WDOG_COMP_PARAMS_4_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("param3", WDOG_COMP_PARAMS_3_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("param2", WDOG_COMP_PARAMS_2_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("param1", WDOG_COMP_PARAMS_1_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("version", WDOG_COMP_VERSION_REG_OFFSET),
> +	DW_WDT_DBGFS_REG("type", WDOG_COMP_TYPE_REG_OFFSET)
> +};
> +
> +static void dw_wdt_dbgfs_init(struct dw_wdt *dw_wdt)
> +{
> +	struct device *dev = dw_wdt->wdd.parent;
> +	struct debugfs_regset32 *regset;
> +
> +	regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
> +	if (!regset)
> +		return;
> +
> +	regset->regs = dw_wdt_dbgfs_regs;
> +	regset->nregs = ARRAY_SIZE(dw_wdt_dbgfs_regs);
> +	regset->base = dw_wdt->regs;
> +
> +	dw_wdt->dbgfs_dir = debugfs_create_dir(dev_name(dev), NULL);
> +
> +	debugfs_create_regset32("registers", 0444, dw_wdt->dbgfs_dir, regset);
> +}
> +
> +static void dw_wdt_dbgfs_clear(struct dw_wdt *dw_wdt)
> +{
> +	debugfs_remove_recursive(dw_wdt->dbgfs_dir);
> +}
> +
> +#else /* !CONFIG_DEBUG_FS */
> +
> +static void dw_wdt_dbgfs_init(struct dw_wdt *dw_wdt) {}
> +static void dw_wdt_dbgfs_clear(struct dw_wdt *dw_wdt) {}
> +
> +#endif /* !CONFIG_DEBUG_FS */
> +
>  static int dw_wdt_drv_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -607,6 +671,8 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto out_disable_pclk;
>  
> +	dw_wdt_dbgfs_init(dw_wdt);
> +
>  	return 0;
>  
>  out_disable_pclk:
> @@ -621,6 +687,8 @@ static int dw_wdt_drv_remove(struct platform_device *pdev)
>  {
>  	struct dw_wdt *dw_wdt = platform_get_drvdata(pdev);
>  
> +	dw_wdt_dbgfs_clear(dw_wdt);
> +
>  	watchdog_unregister_device(&dw_wdt->wdd);
>  	reset_control_assert(dw_wdt->rst);
>  	clk_disable_unprepare(dw_wdt->pclk);

^ permalink raw reply

* [PATCH v4 0/9] clocksource/drivers/timer-atmel-tcb: add sama5d2 support
From: Alexandre Belloni @ 2020-05-29 23:27 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thomas Gleixner, Nicolas Ferre, Sebastian Andrzej Siewior,
	kamel.bouhara, linux-arm-kernel, linux-kernel, devicetree,
	Alexandre Belloni

Hello,

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 v4:
 - Rework binding documentation

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   | 155 ++++++++++++++++++
 arch/arm/boot/dts/sama5d2.dtsi                |  12 +-
 drivers/clocksource/timer-atmel-tcb.c         | 101 +++++++-----
 include/soc/at91/atmel_tcb.h                  |   5 +
 5 files changed, 224 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

-- 
2.26.2


^ permalink raw reply

* [PATCH v4 2/9] dt-bindings: microchip: atmel,at91rm9200-tcb: add sama5d2 compatible
From: Alexandre Belloni @ 2020-05-29 23:27 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thomas Gleixner, Nicolas Ferre, Sebastian Andrzej Siewior,
	kamel.bouhara, linux-arm-kernel, linux-kernel, devicetree,
	Alexandre Belloni, Rob Herring
In-Reply-To: <20200529232749.299627-1-alexandre.belloni@bootlin.com>

The sama5d2 TC block TIMER_CLOCK1 is different from the at91sam9x5 one.
Instead of being MCK / 2, it is the TCB GCLK.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Reviewed-by: Rob Herring <robh+dt@kernel.org>
---
Reviewed by tag taken from:
https://lore.kernel.org/linux-arm-kernel/20200526225046.GA534667@bogus/

 .../soc/microchip/atmel,at91rm9200-tcb.yaml   | 42 +++++++++++++++----
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/soc/microchip/atmel,at91rm9200-tcb.yaml b/Documentation/devicetree/bindings/soc/microchip/atmel,at91rm9200-tcb.yaml
index 9d680e0b9109..d226fd7d5258 100644
--- a/Documentation/devicetree/bindings/soc/microchip/atmel,at91rm9200-tcb.yaml
+++ b/Documentation/devicetree/bindings/soc/microchip/atmel,at91rm9200-tcb.yaml
@@ -19,6 +19,7 @@ properties:
       - enum:
           - atmel,at91rm9200-tcb
           - atmel,at91sam9x5-tcb
+          - atmel,sama5d2-tcb
       - const: simple-mfd
       - const: syscon
 
@@ -36,15 +37,6 @@ properties:
     description:
       List of clock names. Always includes t0_clk and slow clk. Also includes
       t1_clk and t2_clk if a clock per channel is available.
-    oneOf:
-      - items:
-        - const: t0_clk
-        - const: slow_clk
-      - items:
-        - const: t0_clk
-        - const: t1_clk
-        - const: t2_clk
-        - const: slow_clk
     minItems: 2
     maxItems: 4
 
@@ -75,6 +67,38 @@ patternProperties:
       - compatible
       - reg
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: atmel,sama5d2-tcb
+    then:
+      properties:
+        clocks:
+          minItems: 3
+          maxItems: 3
+        clock-names:
+          items:
+            - const: t0_clk
+            - const: gclk
+            - const: slow_clk
+    else:
+      properties:
+        clocks:
+          minItems: 2
+          maxItems: 4
+        clock-names:
+          oneOf:
+            - items:
+              - const: t0_clk
+              - const: slow_clk
+            - items:
+              - const: t0_clk
+              - const: t1_clk
+              - const: t2_clk
+              - const: slow_clk
+
 required:
   - compatible
   - reg
-- 
2.26.2


^ permalink raw reply related

* [PATCH v4 6/9] clocksource/drivers/timer-atmel-tcb: fill tcb_config
From: Alexandre Belloni @ 2020-05-29 23:27 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thomas Gleixner, Nicolas Ferre, Sebastian Andrzej Siewior,
	kamel.bouhara, linux-arm-kernel, linux-kernel, devicetree,
	Alexandre Belloni
In-Reply-To: <20200529232749.299627-1-alexandre.belloni@bootlin.com>

Use the tcb_config and struct atmel_tcb_config to get the timer counter
width. This is necessary because atmel_tcb_config will be extended later
on.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/clocksource/timer-atmel-tcb.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/timer-atmel-tcb.c b/drivers/clocksource/timer-atmel-tcb.c
index b255a4a1a36b..423af2f9835f 100644
--- a/drivers/clocksource/timer-atmel-tcb.c
+++ b/drivers/clocksource/timer-atmel-tcb.c
@@ -348,9 +348,17 @@ static void __init tcb_setup_single_chan(struct atmel_tc *tc, int mck_divisor_id
 
 static const u8 atmel_tcb_divisors[] = { 2, 8, 32, 128 };
 
+static struct atmel_tcb_config tcb_rm9200_config = {
+	.counter_width = 16,
+};
+
+static struct atmel_tcb_config tcb_sam9x5_config = {
+	.counter_width = 32,
+};
+
 static const struct of_device_id atmel_tcb_of_match[] = {
-	{ .compatible = "atmel,at91rm9200-tcb", .data = (void *)16, },
-	{ .compatible = "atmel,at91sam9x5-tcb", .data = (void *)32, },
+	{ .compatible = "atmel,at91rm9200-tcb", .data = &tcb_rm9200_config, },
+	{ .compatible = "atmel,at91sam9x5-tcb", .data = &tcb_sam9x5_config, },
 	{ /* sentinel */ }
 };
 
@@ -398,7 +406,11 @@ static int __init tcb_clksrc_init(struct device_node *node)
 	}
 
 	match = of_match_node(atmel_tcb_of_match, node->parent);
-	bits = (uintptr_t)match->data;
+	if (!match)
+		return -ENODEV;
+
+	tc.tcb_config = match->data;
+	bits = tc.tcb_config->counter_width;
 
 	for (i = 0; i < ARRAY_SIZE(tc.irq); i++)
 		writel(ATMEL_TC_ALL_IRQ, tc.regs + ATMEL_TC_REG(i, IDR));
-- 
2.26.2


^ permalink raw reply related

* [PATCH v4 5/9] clocksource/drivers/timer-atmel-tcb: rework 32khz clock selection
From: Alexandre Belloni @ 2020-05-29 23:27 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thomas Gleixner, Nicolas Ferre, Sebastian Andrzej Siewior,
	kamel.bouhara, linux-arm-kernel, linux-kernel, devicetree,
	Alexandre Belloni
In-Reply-To: <20200529232749.299627-1-alexandre.belloni@bootlin.com>

On all the supported SoCs, the slow clock is always ATMEL_TC_TIMER_CLOCK5,
avoid looking it up and pass it directly to setup_clkevents.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/clocksource/timer-atmel-tcb.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/clocksource/timer-atmel-tcb.c b/drivers/clocksource/timer-atmel-tcb.c
index 7427b07495a8..b255a4a1a36b 100644
--- a/drivers/clocksource/timer-atmel-tcb.c
+++ b/drivers/clocksource/timer-atmel-tcb.c
@@ -346,7 +346,7 @@ static void __init tcb_setup_single_chan(struct atmel_tc *tc, int mck_divisor_id
 	writel(ATMEL_TC_SYNC, tcaddr + ATMEL_TC_BCR);
 }
 
-static const u8 atmel_tcb_divisors[5] = { 2, 8, 32, 128, 0, };
+static const u8 atmel_tcb_divisors[] = { 2, 8, 32, 128 };
 
 static const struct of_device_id atmel_tcb_of_match[] = {
 	{ .compatible = "atmel,at91rm9200-tcb", .data = (void *)16, },
@@ -362,7 +362,6 @@ static int __init tcb_clksrc_init(struct device_node *node)
 	u64 (*tc_sched_clock)(void);
 	u32 rate, divided_rate = 0;
 	int best_divisor_idx = -1;
-	int clk32k_divisor_idx = -1;
 	int bits;
 	int i;
 	int ret;
@@ -416,12 +415,6 @@ static int __init tcb_clksrc_init(struct device_node *node)
 		unsigned divisor = atmel_tcb_divisors[i];
 		unsigned tmp;
 
-		/* remember 32 KiHz clock for later */
-		if (!divisor) {
-			clk32k_divisor_idx = i;
-			continue;
-		}
-
 		tmp = rate / divisor;
 		pr_debug("TC: %u / %-3u [%d] --> %u\n", rate, divisor, i, tmp);
 		if (best_divisor_idx > 0) {
@@ -467,7 +460,7 @@ static int __init tcb_clksrc_init(struct device_node *node)
 		goto err_disable_t1;
 
 	/* channel 2:  periodic and oneshot timer support */
-	ret = setup_clkevents(&tc, clk32k_divisor_idx);
+	ret = setup_clkevents(&tc, ATMEL_TC_TIMER_CLOCK5);
 	if (ret)
 		goto err_unregister_clksrc;
 
-- 
2.26.2


^ permalink raw reply related

* [PATCH v4 7/9] clocksource/drivers/timer-atmel-tcb: stop using the 32kHz for clockevents
From: Alexandre Belloni @ 2020-05-29 23:27 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thomas Gleixner, Nicolas Ferre, Sebastian Andrzej Siewior,
	kamel.bouhara, linux-arm-kernel, linux-kernel, devicetree,
	Alexandre Belloni
In-Reply-To: <20200529232749.299627-1-alexandre.belloni@bootlin.com>

Stop using the slow clock as the clock source for 32 bit counters because
even at 10MHz, they are able to handle delays up to two minutes. This
provides a way better resolution.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/clocksource/timer-atmel-tcb.c | 61 ++++++++++++++-------------
 1 file changed, 32 insertions(+), 29 deletions(-)

diff --git a/drivers/clocksource/timer-atmel-tcb.c b/drivers/clocksource/timer-atmel-tcb.c
index 423af2f9835f..8fcd4d74c54b 100644
--- a/drivers/clocksource/timer-atmel-tcb.c
+++ b/drivers/clocksource/timer-atmel-tcb.c
@@ -27,9 +27,10 @@
  *   - Some chips support 32 bit counter. A single channel is used for
  *     this 32 bit free-running counter. the second channel is not used.
  *
- *   - The third channel may be used to provide a 16-bit clockevent
- *     source, used in either periodic or oneshot mode.  This runs
- *     at 32 KiHZ, and can handle delays of up to two seconds.
+ *   - The third channel may be used to provide a clockevent source, used in
+ *   either periodic or oneshot mode. For 16-bit counter its runs at 32 KiHZ,
+ *   and can handle delays of up to two seconds. For 32-bit counters, it runs at
+ *   the same rate as the clocksource
  *
  * REVISIT behavior during system suspend states... we should disable
  * all clocks and save the power.  Easily done for clockevent devices,
@@ -47,6 +48,8 @@ static struct
 } tcb_cache[3];
 static u32 bmr_cache;
 
+static const u8 atmel_tcb_divisors[] = { 2, 8, 32, 128 };
+
 static u64 tc_get_cycles(struct clocksource *cs)
 {
 	unsigned long	flags;
@@ -151,13 +154,6 @@ static struct tc_clkevt_device *to_tc_clkevt(struct clock_event_device *clkevt)
 	return container_of(clkevt, struct tc_clkevt_device, clkevt);
 }
 
-/* For now, we always use the 32K clock ... this optimizes for NO_HZ,
- * because using one of the divided clocks would usually mean the
- * tick rate can never be less than several dozen Hz (vs 0.5 Hz).
- *
- * A divided clock could be good for high resolution timers, since
- * 30.5 usec resolution can seem "low".
- */
 static u32 timer_clock;
 
 static int tc_shutdown(struct clock_event_device *d)
@@ -183,7 +179,7 @@ static int tc_set_oneshot(struct clock_event_device *d)
 
 	clk_enable(tcd->clk);
 
-	/* slow clock, count up to RC, then irq and stop */
+	/* count up to RC, then irq and stop */
 	writel(timer_clock | ATMEL_TC_CPCSTOP | ATMEL_TC_WAVE |
 		     ATMEL_TC_WAVESEL_UP_AUTO, regs + ATMEL_TC_REG(2, CMR));
 	writel(ATMEL_TC_CPCS, regs + ATMEL_TC_REG(2, IER));
@@ -205,7 +201,7 @@ static int tc_set_periodic(struct clock_event_device *d)
 	 */
 	clk_enable(tcd->clk);
 
-	/* slow clock, count up to RC, then irq and restart */
+	/* count up to RC, then irq and restart */
 	writel(timer_clock | ATMEL_TC_WAVE | ATMEL_TC_WAVESEL_UP_AUTO,
 		     regs + ATMEL_TC_REG(2, CMR));
 	writel((32768 + HZ / 2) / HZ, tcaddr + ATMEL_TC_REG(2, RC));
@@ -256,47 +252,56 @@ static irqreturn_t ch2_irq(int irq, void *handle)
 	return IRQ_NONE;
 }
 
-static int __init setup_clkevents(struct atmel_tc *tc, int clk32k_divisor_idx)
+static int __init setup_clkevents(struct atmel_tc *tc, int divisor_idx)
 {
+	u32 rate;
 	int ret;
 	struct clk *t2_clk = tc->clk[2];
 	int irq = tc->irq[2];
-
-	ret = clk_prepare_enable(tc->slow_clk);
-	if (ret)
-		return ret;
+	int bits = tc->tcb_config->counter_width;
 
 	/* try to enable t2 clk to avoid future errors in mode change */
 	ret = clk_prepare_enable(t2_clk);
-	if (ret) {
-		clk_disable_unprepare(tc->slow_clk);
+	if (ret)
 		return ret;
-	}
-
-	clk_disable(t2_clk);
 
 	clkevt.regs = tc->regs;
 	clkevt.clk = t2_clk;
 
-	timer_clock = clk32k_divisor_idx;
+	if (bits == 32) {
+		timer_clock = divisor_idx;
+		rate = clk_get_rate(t2_clk) / atmel_tcb_divisors[divisor_idx];
+	} else {
+		ret = clk_prepare_enable(tc->slow_clk);
+		if (ret) {
+			clk_disable_unprepare(t2_clk);
+			return ret;
+		}
+
+		rate = clk_get_rate(tc->slow_clk);
+		timer_clock = ATMEL_TC_TIMER_CLOCK5;
+	}
+
+	clk_disable(t2_clk);
 
 	clkevt.clkevt.cpumask = cpumask_of(0);
 
 	ret = request_irq(irq, ch2_irq, IRQF_TIMER, "tc_clkevt", &clkevt);
 	if (ret) {
 		clk_unprepare(t2_clk);
-		clk_disable_unprepare(tc->slow_clk);
+		if (bits != 32)
+			clk_disable_unprepare(tc->slow_clk);
 		return ret;
 	}
 
-	clockevents_config_and_register(&clkevt.clkevt, 32768, 1, 0xffff);
+	clockevents_config_and_register(&clkevt.clkevt, rate, 1, BIT(bits) - 1);
 
 	return ret;
 }
 
 #else /* !CONFIG_GENERIC_CLOCKEVENTS */
 
-static int __init setup_clkevents(struct atmel_tc *tc, int clk32k_divisor_idx)
+static int __init setup_clkevents(struct atmel_tc *tc, int divisor_idx)
 {
 	/* NOTHING */
 	return 0;
@@ -346,8 +351,6 @@ static void __init tcb_setup_single_chan(struct atmel_tc *tc, int mck_divisor_id
 	writel(ATMEL_TC_SYNC, tcaddr + ATMEL_TC_BCR);
 }
 
-static const u8 atmel_tcb_divisors[] = { 2, 8, 32, 128 };
-
 static struct atmel_tcb_config tcb_rm9200_config = {
 	.counter_width = 16,
 };
@@ -472,7 +475,7 @@ static int __init tcb_clksrc_init(struct device_node *node)
 		goto err_disable_t1;
 
 	/* channel 2:  periodic and oneshot timer support */
-	ret = setup_clkevents(&tc, ATMEL_TC_TIMER_CLOCK5);
+	ret = setup_clkevents(&tc, best_divisor_idx);
 	if (ret)
 		goto err_unregister_clksrc;
 
-- 
2.26.2


^ permalink raw reply related

* [PATCH v4 8/9] clocksource/drivers/timer-atmel-tcb: allow selecting first divider
From: Alexandre Belloni @ 2020-05-29 23:27 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thomas Gleixner, Nicolas Ferre, Sebastian Andrzej Siewior,
	kamel.bouhara, linux-arm-kernel, linux-kernel, devicetree,
	Alexandre Belloni
In-Reply-To: <20200529232749.299627-1-alexandre.belloni@bootlin.com>

The divider selection algorithm never allowed to get index 0. It was also
continuing to look for dividers, trying to find the slow clock selection.
This is not necessary anymore.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/clocksource/timer-atmel-tcb.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/timer-atmel-tcb.c b/drivers/clocksource/timer-atmel-tcb.c
index 8fcd4d74c54b..ccb77b9cb489 100644
--- a/drivers/clocksource/timer-atmel-tcb.c
+++ b/drivers/clocksource/timer-atmel-tcb.c
@@ -432,10 +432,8 @@ static int __init tcb_clksrc_init(struct device_node *node)
 
 		tmp = rate / divisor;
 		pr_debug("TC: %u / %-3u [%d] --> %u\n", rate, divisor, i, tmp);
-		if (best_divisor_idx > 0) {
-			if (tmp < 5 * 1000 * 1000)
-				continue;
-		}
+		if ((best_divisor_idx >= 0) && (tmp < 5 * 1000 * 1000))
+			break;
 		divided_rate = tmp;
 		best_divisor_idx = i;
 	}
-- 
2.26.2


^ permalink raw reply related

* [PATCH v4 9/9] clocksource/drivers/timer-atmel-tcb: add sama5d2 support
From: Alexandre Belloni @ 2020-05-29 23:27 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thomas Gleixner, Nicolas Ferre, Sebastian Andrzej Siewior,
	kamel.bouhara, linux-arm-kernel, linux-kernel, devicetree,
	Alexandre Belloni
In-Reply-To: <20200529232749.299627-1-alexandre.belloni@bootlin.com>

The first divisor for the sama5d2 is actually the gclk selector. Because
the currently remaining divisors are fitting the use case, currently ensure
it is skipped.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/clocksource/timer-atmel-tcb.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-atmel-tcb.c b/drivers/clocksource/timer-atmel-tcb.c
index ccb77b9cb489..e373b02d509a 100644
--- a/drivers/clocksource/timer-atmel-tcb.c
+++ b/drivers/clocksource/timer-atmel-tcb.c
@@ -359,9 +359,15 @@ static struct atmel_tcb_config tcb_sam9x5_config = {
 	.counter_width = 32,
 };
 
+static struct atmel_tcb_config tcb_sama5d2_config = {
+	.counter_width = 32,
+	.has_gclk = 1,
+};
+
 static const struct of_device_id atmel_tcb_of_match[] = {
 	{ .compatible = "atmel,at91rm9200-tcb", .data = &tcb_rm9200_config, },
 	{ .compatible = "atmel,at91sam9x5-tcb", .data = &tcb_sam9x5_config, },
+	{ .compatible = "atmel,sama5d2-tcb", .data = &tcb_sama5d2_config, },
 	{ /* sentinel */ }
 };
 
@@ -426,7 +432,10 @@ static int __init tcb_clksrc_init(struct device_node *node)
 
 	/* How fast will we be counting?  Pick something over 5 MHz.  */
 	rate = (u32) clk_get_rate(t0_clk);
-	for (i = 0; i < ARRAY_SIZE(atmel_tcb_divisors); i++) {
+	i = 0;
+	if (tc.tcb_config->has_gclk)
+		i = 1;
+	for (; i < ARRAY_SIZE(atmel_tcb_divisors); i++) {
 		unsigned divisor = atmel_tcb_divisors[i];
 		unsigned tmp;
 
-- 
2.26.2


^ permalink raw reply related

* [PATCH v4 4/9] ARM: at91: add atmel tcb capabilities
From: Alexandre Belloni @ 2020-05-29 23:27 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thomas Gleixner, Nicolas Ferre, Sebastian Andrzej Siewior,
	kamel.bouhara, linux-arm-kernel, linux-kernel, devicetree,
	Alexandre Belloni
In-Reply-To: <20200529232749.299627-1-alexandre.belloni@bootlin.com>

From: Kamel Bouhara <kamel.bouhara@bootlin.com>

Some atmel socs have extra tcb capabilities that allow using a generic
clock source or enabling a quadrature decoder.

Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 include/soc/at91/atmel_tcb.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/soc/at91/atmel_tcb.h b/include/soc/at91/atmel_tcb.h
index c3c7200ce151..1d7071dc0bca 100644
--- a/include/soc/at91/atmel_tcb.h
+++ b/include/soc/at91/atmel_tcb.h
@@ -36,9 +36,14 @@ struct clk;
 /**
  * struct atmel_tcb_config - SoC data for a Timer/Counter Block
  * @counter_width: size in bits of a timer counter register
+ * @has_gclk: boolean indicating if a timer counter has a generic clock
+ * @has_qdec: boolean indicating if a timer counter has a quadrature
+ * decoder.
  */
 struct atmel_tcb_config {
 	size_t	counter_width;
+	bool    has_gclk;
+	bool    has_qdec;
 };
 
 /**
-- 
2.26.2


^ permalink raw reply related

* [PATCH v4 1/9] dt-bindings: atmel-tcb: convert bindings to json-schema
From: Alexandre Belloni @ 2020-05-29 23:27 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Thomas Gleixner, Nicolas Ferre, Sebastian Andrzej Siewior,
	kamel.bouhara, linux-arm-kernel, linux-kernel, devicetree,
	Alexandre Belloni, Rob Herring
In-Reply-To: <20200529232749.299627-1-alexandre.belloni@bootlin.com>

Convert Atmel Timer Counter Blocks bindings to DT schema format using
json-schema.

Also move it out of mfd as it is not and has never been related to mfd.

Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
Changes in v4:
 - use oneOf to describe possible clock-names list

 .../devicetree/bindings/mfd/atmel-tcb.txt     |  56 --------
 .../soc/microchip/atmel,at91rm9200-tcb.yaml   | 131 ++++++++++++++++++
 2 files changed, 131 insertions(+), 56 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-tcb.txt
 create mode 100644 Documentation/devicetree/bindings/soc/microchip/atmel,at91rm9200-tcb.yaml

diff --git a/Documentation/devicetree/bindings/mfd/atmel-tcb.txt b/Documentation/devicetree/bindings/mfd/atmel-tcb.txt
deleted file mode 100644
index c4a83e364cb6..000000000000
--- a/Documentation/devicetree/bindings/mfd/atmel-tcb.txt
+++ /dev/null
@@ -1,56 +0,0 @@
-* Device tree bindings for Atmel Timer Counter Blocks
-- compatible: Should be "atmel,<chip>-tcb", "simple-mfd", "syscon".
-  <chip> can be "at91rm9200" or "at91sam9x5"
-- reg: Should contain registers location and length
-- #address-cells: has to be 1
-- #size-cells: has to be 0
-- interrupts: Should contain all interrupts for the TC block
-  Note that you can specify several interrupt cells if the TC
-  block has one interrupt per channel.
-- clock-names: tuple listing input clock names.
-	Required elements: "t0_clk", "slow_clk"
-	Optional elements: "t1_clk", "t2_clk"
-- clocks: phandles to input clocks.
-
-The TCB can expose multiple subdevices:
- * a timer
-   - compatible: Should be "atmel,tcb-timer"
-   - reg: Should contain the TCB channels to be used. If the
-     counter width is 16 bits (at91rm9200-tcb), two consecutive
-     channels are needed. Else, only one channel will be used.
-
-Examples:
-
-One interrupt per TC block:
-	tcb0: timer@fff7c000 {
-		compatible = "atmel,at91rm9200-tcb", "simple-mfd", "syscon";
-		#address-cells = <1>;
-		#size-cells = <0>;
-		reg = <0xfff7c000 0x100>;
-		interrupts = <18 4>;
-		clocks = <&tcb0_clk>, <&clk32k>;
-		clock-names = "t0_clk", "slow_clk";
-
-		timer@0 {
-			compatible = "atmel,tcb-timer";
-			reg = <0>, <1>;
-		};
-
-		timer@2 {
-			compatible = "atmel,tcb-timer";
-			reg = <2>;
-		};
-	};
-
-One interrupt per TC channel in a TC block:
-	tcb1: timer@fffdc000 {
-		compatible = "atmel,at91rm9200-tcb", "simple-mfd", "syscon";
-		#address-cells = <1>;
-		#size-cells = <0>;
-		reg = <0xfffdc000 0x100>;
-		interrupts = <26 4>, <27 4>, <28 4>;
-		clocks = <&tcb1_clk>, <&clk32k>;
-		clock-names = "t0_clk", "slow_clk";
-	};
-
-
diff --git a/Documentation/devicetree/bindings/soc/microchip/atmel,at91rm9200-tcb.yaml b/Documentation/devicetree/bindings/soc/microchip/atmel,at91rm9200-tcb.yaml
new file mode 100644
index 000000000000..9d680e0b9109
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/microchip/atmel,at91rm9200-tcb.yaml
@@ -0,0 +1,131 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/soc/microchip/atmel,at91rm9200-tcb.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Atmel Timer Counter Block
+
+maintainers:
+  - Alexandre Belloni <alexandre.belloni@bootlin.com>
+
+description: |
+  The Atmel (now Microchip) SoCs have timers named Timer Counter Block. Each
+  timer has three channels with two counters each.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - atmel,at91rm9200-tcb
+          - atmel,at91sam9x5-tcb
+      - const: simple-mfd
+      - const: syscon
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    description:
+      List of interrupts. One interrupt per TCB channel if available or one
+      interrupt for the TC block
+    minItems: 1
+    maxItems: 3
+
+  clock-names:
+    description:
+      List of clock names. Always includes t0_clk and slow clk. Also includes
+      t1_clk and t2_clk if a clock per channel is available.
+    oneOf:
+      - items:
+        - const: t0_clk
+        - const: slow_clk
+      - items:
+        - const: t0_clk
+        - const: t1_clk
+        - const: t2_clk
+        - const: slow_clk
+    minItems: 2
+    maxItems: 4
+
+  clocks:
+    minItems: 2
+    maxItems: 4
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+patternProperties:
+  "^timer@[0-2]$":
+    description: The timer block channels that are used as timers.
+    type: object
+    properties:
+      compatible:
+        const: atmel,tcb-timer
+      reg:
+        description:
+          List of channels to use for this particular timer.
+        minItems: 1
+        maxItems: 3
+
+    required:
+      - compatible
+      - reg
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - '#address-cells'
+  - '#size-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    /* One interrupt per TC block: */
+        tcb0: timer@fff7c000 {
+                compatible = "atmel,at91rm9200-tcb", "simple-mfd", "syscon";
+                #address-cells = <1>;
+                #size-cells = <0>;
+                reg = <0xfff7c000 0x100>;
+                interrupts = <18 4>;
+                clocks = <&tcb0_clk>, <&clk32k>;
+                clock-names = "t0_clk", "slow_clk";
+
+                timer@0 {
+                        compatible = "atmel,tcb-timer";
+                        reg = <0>, <1>;
+                };
+
+                timer@2 {
+                        compatible = "atmel,tcb-timer";
+                        reg = <2>;
+                };
+        };
+
+    /* One interrupt per TC channel in a TC block: */
+        tcb1: timer@fffdc000 {
+                compatible = "atmel,at91rm9200-tcb", "simple-mfd", "syscon";
+                #address-cells = <1>;
+                #size-cells = <0>;
+                reg = <0xfffdc000 0x100>;
+                interrupts = <26 4>, <27 4>, <28 4>;
+                clocks = <&tcb1_clk>, <&clk32k>;
+                clock-names = "t0_clk", "slow_clk";
+
+                timer@0 {
+                        compatible = "atmel,tcb-timer";
+                        reg = <0>;
+                };
+
+                timer@1 {
+                        compatible = "atmel,tcb-timer";
+                        reg = <1>;
+                };
+        };
-- 
2.26.2


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox