* [PATCH v2 1/6] Documentation: DT: Consolidate SP805 binding docs
From: Ray Jui @ 2018-05-23 20:32 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland,
Frank Rowand, Catalin Marinas, Will Deacon, Robin Murphy
Cc: linux-watchdog, devicetree, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list, Ray Jui
In-Reply-To: <1527107527-8093-1-git-send-email-ray.jui@broadcom.com>
Consolidate two SP805 binding documents "arm,sp805.txt" and
"sp805-wdt.txt" into "arm,sp805.txt" that matches the naming of the
desired compatible string to be used
Signed-off-by: Ray Jui <ray.jui@broadcom.com>
---
.../devicetree/bindings/watchdog/arm,sp805.txt | 20 +++++++++++---
.../devicetree/bindings/watchdog/sp805-wdt.txt | 31 ----------------------
2 files changed, 16 insertions(+), 35 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
diff --git a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
index ca99d64..de93a4d 100644
--- a/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
+++ b/Documentation/devicetree/bindings/watchdog/arm,sp805.txt
@@ -1,17 +1,29 @@
ARM AMBA Primecell SP805 Watchdog
+SP805 WDT is a ARM Primecell Peripheral and has a standard-id register that
+can be used to identify the peripheral type, vendor, and revision.
+This value can be used for driver matching.
+
+As SP805 WDT is a primecell IP, it follows the base bindings specified in
+'arm/primecell.txt'
+
Required properties:
- compatible: Should be "arm,sp805" & "arm,primecell"
- reg: Should contain location and length for watchdog timer register.
-- interrupts: Should contain the list of watchdog timer interrupts.
- clocks: clocks driving the watchdog timer hardware. This list should be 2
- clocks. With 2 clocks, the order is wdogclk clock, apb_pclk.
+ clocks. With 2 clocks, the order is wdog_clk, apb_pclk.
+ wdog_clk can be equal to or be a sub-multiple of the apb_pclk frequency
+- clock-names : Shall be "wdog_clk" for first clock and "apb_pclk" for the
+ second one.
+
+Optional properties:
+- interrupts : Should specify WDT interrupt number.
Example:
watchdog@66090000 {
compatible = "arm,sp805", "arm,primecell";
reg = <0x66090000 0x1000>;
interrupts = <GIC_SPI 406 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&apb_pclk>,<&apb_pclk>;
- clock-names = "wdogclk", "apb_pclk";
+ clocks = <&wdt_clk>, <&apb_pclk>;
+ clock-names = "wdog_clk", "apb_pclk";
};
diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
deleted file mode 100644
index edc4f0e..0000000
--- a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
+++ /dev/null
@@ -1,31 +0,0 @@
-* ARM SP805 Watchdog Timer (WDT) Controller
-
-SP805 WDT is a ARM Primecell Peripheral and has a standard-id register that
-can be used to identify the peripheral type, vendor, and revision.
-This value can be used for driver matching.
-
-As SP805 WDT is a primecell IP, it follows the base bindings specified in
-'arm/primecell.txt'
-
-Required properties:
-- compatible : Should be "arm,sp805-wdt", "arm,primecell"
-- reg : Base address and size of the watchdog timer registers.
-- clocks : From common clock binding.
- First clock is PCLK and the second is WDOGCLK.
- WDOGCLK can be equal to or be a sub-multiple of the PCLK frequency.
-- clock-names : From common clock binding.
- Shall be "apb_pclk" for first clock and "wdog_clk" for the
- second one.
-
-Optional properties:
-- interrupts : Should specify WDT interrupt number.
-
-Examples:
-
- cluster1_core0_watchdog: wdt@c000000 {
- compatible = "arm,sp805-wdt", "arm,primecell";
- reg = <0x0 0xc000000 0x0 0x1000>;
- clocks = <&clockgen 4 3>, <&clockgen 4 3>;
- clock-names = "apb_pclk", "wdog_clk";
- };
-
--
2.1.4
^ permalink raw reply related
* [PATCH v2 0/6]Enhance support for the SP805 WDT
From: Ray Jui @ 2018-05-23 20:32 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Rob Herring, Mark Rutland,
Frank Rowand, Catalin Marinas, Will Deacon, Robin Murphy
Cc: linux-watchdog, devicetree, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list, Ray Jui
This patch series enhances the support for the SP805 watchdog timer.
First of all, 'timeout-sec' devicetree property is added. In addition,
support is also added to allow the driver to reset the watchdog if it
has been detected that watchdot has been started in the bootloader. In
this case, the driver will initiate the ping service from the kernel
watchdog subsystem, before a user mode daemon takes over. This series
also enables SP805 in the default ARM64 defconfig
This patch series is based off v4.17-rc5 and is available on GIHUB:
repo: https://github.com/Broadcom/arm64-linux.git
branch: sp805-wdt-v2
Changes since v1:
- Consolidate two duplicated SP805 binding documents into one
- Slight change of the wdt_is_running implementation per discussion
Ray Jui (6):
Documentation: DT: Consolidate SP805 binding docs
Documentation: DT: Add optional 'timeout-sec' property for sp805
watchdog: sp805: add 'timeout-sec' DT property support
watchdog: sp805: set WDOG_HW_RUNNING when appropriate
arm64: dt: set initial SR watchdog timeout to 60 seconds
arm64: defconfig: add CONFIG_ARM_SP805_WATCHDOG
.../devicetree/bindings/watchdog/arm,sp805.txt | 22 ++++++++++++---
.../devicetree/bindings/watchdog/sp805-wdt.txt | 31 ----------------------
.../arm64/boot/dts/broadcom/stingray/stingray.dtsi | 1 +
arch/arm64/configs/defconfig | 1 +
drivers/watchdog/sp805_wdt.c | 28 ++++++++++++++++++-
5 files changed, 47 insertions(+), 36 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
--
2.1.4
^ permalink raw reply
* Re: [PATCH] arm64: dts: sprd: fix typo in 'remote-endpoint'
From: Rob Herring @ 2018-05-23 20:30 UTC (permalink / raw)
To: Chunyan Zhang, Orson Zhai, Baolin Wang; +Cc: DTML, Linux ARM
In-Reply-To: <CAAfSe-tgKJvzCN6eHCuUR94ECSE8ypAR1eRjhV-VJNztebK9Xw@mail.gmail.com>
On Tue, May 8, 2018 at 8:58 PM, Chunyan Zhang <zhang.lyra@gmail.com> wrote:
> Hi Rob,
>
> On 8 May 2018 at 23:09, Rob Herring <robh@kernel.org> wrote:
>> dtc now warns on incomplete OF graph endpoint connections:
>>
>> arch/arm64/boot/dts/sprd/sp9860g-1h10.dtb: Warning (graph_endpoint): /soc/stm@10006000/port/endpoint: graph connection to node '/soc/funnel@10001000/ports/port@2/endpoint' is not bidirectional
>>
>> The cause is a typo in 'remote-endpoint'.
>
> Thanks for fixing this and,
>
> Acked-by: Chunyan Zhang <zhang.lyra@gmail.com>
Is someone going to apply this? Still seeing warnings in linux-next.
Rob
^ permalink raw reply
* Re: [PATCH] arm64: dts: msm8916: fix Coresight ETF graph connections
From: Rob Herring @ 2018-05-23 20:29 UTC (permalink / raw)
To: devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Andy Gross
Cc: David Brown, Mathieu Poirier, Ivan T . Ivanov, linux-arm-msm
In-Reply-To: <20180508150952.24562-3-robh@kernel.org>
On Tue, May 8, 2018 at 10:09 AM, Rob Herring <robh@kernel.org> wrote:
> The ETF input should be connected to the funnel output, and the ETF
> output should be connected to the replicator input. The labels are wrong
> and these got swapped:
>
> Warning (graph_endpoint): /soc/funnel@821000/ports/port@8/endpoint: graph connection to node '/soc/etf@825000/ports/port@1/endpoint' is not bidirectional
> Warning (graph_endpoint): /soc/replicator@824000/ports/port@2/endpoint: graph connection to node '/soc/etf@825000/ports/port@0/endpoint' is not bidirectional
>
> Fixes: 7c10da373698 ("arm64: dts: qcom: Add msm8916 CoreSight components")
> Cc: Ivan T. Ivanov <ivan.ivanov@linaro.org>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Andy Gross <andy.gross@linaro.org>
> Cc: David Brown <david.brown@linaro.org>
> Cc: linux-arm-msm@vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> arch/arm64/boot/dts/qcom/msm8916.dtsi | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Ping. Still seeing warnings in -next for these.
Rob
^ permalink raw reply
* [PATCH] arm64: dts: stingray: Add otp device node
From: Scott Branden @ 2018-05-23 20:17 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
Florian Fainelli
Cc: BCM Kernel Feedback, devicetree, linux-arm-kernel, linux-kernel,
Scott Branden
Add otp device node for Stingray SOC.
Fixes: 2fa9e9e29ea2 ("arm64: dts: Add GPIO DT nodes for Stingray SOC")
Signed-off-by: Scott Branden <scott.branden@broadcom.com>
---
arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
index 99aaff0..6013478 100644
--- a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
+++ b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
@@ -258,6 +258,13 @@
#include "stingray-clock.dtsi"
+ otp: otp@1c400 {
+ compatible = "brcm,ocotp-v2";
+ reg = <0x0001c400 0x68>;
+ brcm,ocotp-size = <2048>;
+ status = "okay";
+ };
+
gpio_crmu: gpio@24800 {
compatible = "brcm,iproc-gpio";
reg = <0x00024800 0x4c>;
--
2.5.0
^ permalink raw reply related
* Re: [PATCH v3 13/13] soc: rockchip: power-domain: add power domain support for px30
From: Heiko Stübner @ 2018-05-23 20:10 UTC (permalink / raw)
To: Elaine Zhang
Cc: robh+dt, mark.rutland, devicetree, rjw, khilman, ulf.hansson,
linux-pm, linux-arm-kernel, linux-rockchip, linux-kernel, wxt,
xxx, xf, huangtao, Finley Xiao
In-Reply-To: <1527058412-10754-1-git-send-email-zhangqing@rock-chips.com>
Am Mittwoch, 23. Mai 2018, 08:53:32 CEST schrieb Elaine Zhang:
> From: Finley Xiao <finley.xiao@rock-chips.com>
>
> This driver is modified to support PX30 SoC.
>
> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
applied for 4.18 (or later)
Thanks
Heiko
^ permalink raw reply
* Re: [PATCH v3 12/13] dt-bindings: add binding for px30 power domains
From: Heiko Stübner @ 2018-05-23 20:10 UTC (permalink / raw)
To: Elaine Zhang
Cc: robh+dt, mark.rutland, devicetree, rjw, khilman, ulf.hansson,
linux-pm, linux-arm-kernel, linux-rockchip, linux-kernel, wxt,
xxx, xf, huangtao, Finley Xiao
In-Reply-To: <1527058371-10706-1-git-send-email-zhangqing@rock-chips.com>
Am Mittwoch, 23. Mai 2018, 08:52:51 CEST schrieb Elaine Zhang:
> From: Finley Xiao <finley.xiao@rock-chips.com>
>
> Add binding documentation for the power domains
> found on Rockchip PX30 SoCs.
>
> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
applied for 4.18 (or later)
Thanks
Heiko
^ permalink raw reply
* Re: [PATCH v3 11/13] dt-bindings: power: add PX30 SoCs header for power-domain
From: Heiko Stübner @ 2018-05-23 20:10 UTC (permalink / raw)
To: Elaine Zhang
Cc: robh+dt, mark.rutland, devicetree, rjw, khilman, ulf.hansson,
linux-pm, linux-arm-kernel, linux-rockchip, linux-kernel, wxt,
xxx, xf, huangtao, Finley Xiao
In-Reply-To: <1527058341-10658-1-git-send-email-zhangqing@rock-chips.com>
Am Mittwoch, 23. Mai 2018, 08:52:21 CEST schrieb Elaine Zhang:
> From: Finley Xiao <finley.xiao@rock-chips.com>
>
> According to a description from TRM, add all the power domains.
>
> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
applied for 4.18 (or later)
Thanks
Heiko
^ permalink raw reply
* Re: [PATCH v3 10/13] soc: rockchip: power-domain: add power domain support for rk3228
From: Heiko Stübner @ 2018-05-23 20:09 UTC (permalink / raw)
To: Elaine Zhang
Cc: robh+dt, mark.rutland, devicetree, rjw, khilman, ulf.hansson,
linux-pm, linux-arm-kernel, linux-rockchip, linux-kernel, wxt,
xxx, xf, huangtao
In-Reply-To: <1527058323-10611-1-git-send-email-zhangqing@rock-chips.com>
Am Mittwoch, 23. Mai 2018, 08:52:03 CEST schrieb Elaine Zhang:
> This driver is modified to support RK3228 SoC.
>
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
applied for 4.18 (or later)
Thanks
Heiko
^ permalink raw reply
* Re: [PATCH v3 09/13] dt-bindings: add binding for rk3228 power domains
From: Heiko Stübner @ 2018-05-23 20:09 UTC (permalink / raw)
To: Elaine Zhang
Cc: robh+dt, mark.rutland, devicetree, rjw, khilman, ulf.hansson,
linux-pm, linux-arm-kernel, linux-rockchip, linux-kernel, wxt,
xxx, xf, huangtao
In-Reply-To: <1527058301-10553-1-git-send-email-zhangqing@rock-chips.com>
Am Mittwoch, 23. Mai 2018, 08:51:41 CEST schrieb Elaine Zhang:
> Add binding documentation for the power domains
> found on Rockchip RK3228 SoCs.
>
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
> Acked-by: Rob Herring <robh@kernel.org>
applied for 4.18 (or later)
Thanks
Heiko
^ permalink raw reply
* Re: [PATCH v3 08/13] dt-bindings: power: add RK3228 SoCs header for power-domain
From: Heiko Stübner @ 2018-05-23 20:09 UTC (permalink / raw)
To: Elaine Zhang
Cc: robh+dt, mark.rutland, devicetree, rjw, khilman, ulf.hansson,
linux-pm, linux-arm-kernel, linux-rockchip, linux-kernel, wxt,
xxx, xf, huangtao
In-Reply-To: <1527058286-10503-1-git-send-email-zhangqing@rock-chips.com>
Am Mittwoch, 23. Mai 2018, 08:51:26 CEST schrieb Elaine Zhang:
> According to a description from TRM, add all the power domains.
>
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
applied for 4.18 (or later)
Thanks
Heiko
^ permalink raw reply
* Re: [PATCH V4 0/8] net: ethernet: stmmac: add support for stm32mp1
From: David Miller @ 2018-05-23 20:08 UTC (permalink / raw)
To: christophe.roullier
Cc: mark.rutland, mcoquelin.stm32, alexandre.torgue, peppe.cavallaro,
devicetree, linux-arm-kernel, netdev, andrew
In-Reply-To: <1527090479-5263-1-git-send-email-christophe.roullier@st.com>
From: Christophe Roullier <christophe.roullier@st.com>
Date: Wed, 23 May 2018 17:47:51 +0200
> Patches to have Ethernet support on stm32mp1
> Changelog:
> Remark from Rob Herring
> Move Documentation/devicetree/bindings/arm/stm32.txt in
> Documentation/devicetree/bindings/arm/stm32/stm32.txt and create
> Documentation/devicetree/bindings/arm/stm32/stm32-syscon.txt
>
> Replace also in arch/arm/boot/dts/stm32mp157c.dtsi, syscfg: system-config@50020000
> with syscfg: syscon@50020000syscfg: system-config@50020000
Probably the DTS file updates need to go in via the ARM tree, not
mine.
Can you respin a net-next targetted series that has just the driver
code and device tree binding updates?
Thank you!
^ permalink raw reply
* Re: [PATCH v9 2/2] leds: lm3601x: Introduce the lm3601x LED driver
From: Jacek Anaszewski @ 2018-05-23 20:07 UTC (permalink / raw)
To: Dan Murphy, robh+dt, mark.rutland, andy.shevchenko
Cc: devicetree, linux-kernel, linux-leds
In-Reply-To: <20180523115129.21674-2-dmurphy@ti.com>
Hi Dan,
Thank you for the updated set.
On 05/23/2018 01:51 PM, Dan Murphy wrote:
> Introduce the family of LED devices that can
> drive a torch, strobe or IR LED.
>
> The LED driver can be configured with a strobe
> timer to execute a strobe flash. The IR LED
> brightness is controlled via the torch brightness
> register.
>
> The data sheet for each the LM36010 and LM36011
> LED drivers can be found here:
> http://www.ti.com/product/LM36010
> http://www.ti.com/product/LM36011
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>
> v9 - Changed "strobe" to flash in some cases, removed some redundant variable
> assignments, updated a couple of line to make them more readable, converted to
> probe_new from old probe, and change err to ret - https://patchwork.kernel.org/patch/10418731/
>
> v8 - Removed OF Kconfig dependency, change strobe to flash, updated label generation,
> fixed brightness calculation, added mutex_destroy and flash_unregister - https://patchwork.kernel.org/patch/10416163/
> v7 - Numerous fixes to many to list. Highlights are moved from OF APIs to device_property
> APIs, updated LED registration, timeout to reg algorithim, and fixed label generation -
> https://patchwork.kernel.org/patch/10401437/
> v6 - This driver has been heavily modified from v5. There is no longer reading
> of individual child nodes. There are too many changes to list here see -
> https://patchwork.kernel.org/patch/10392123/
> v5 - Fixed magic numbers, change reg cache type, added of_put_node to release
> the dt node ref, and I did not change the remove function to leave the LED in its
> state on driver removal - https://patchwork.kernel.org/patch/10391741/
> v4 - Fixed Cocci issue using ARRAY_SIZE - https://patchwork.kernel.org/patch/10389259/
> v3 - removed wildcard dt compatible, fixed copyright, fixed struct doc, removed
> RO registers from default, added regmap volatile for FLAGS_REG, updated regmap cache type,
> fixed unlock and extra semi colon in strobe_set, removed unnecessary out label
> in led register and fixed checking of the ret in brightness_set - https://patchwork.kernel.org/patch/10386243/
> v2 - Fixed kbuild issue and removed unused cdev_strobe - https://patchwork.kernel.org/patch/10384585/
>
> drivers/leds/Kconfig | 9 +
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-lm3601x.c | 487 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 497 insertions(+)
> create mode 100644 drivers/leds/leds-lm3601x.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 2c896c0e69e1..d95d436e6089 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -145,6 +145,15 @@ config LEDS_LM3692X
> This option enables support for the TI LM3692x family
> of white LED string drivers used for backlighting.
>
> +config LEDS_LM3601X
> + tristate "LED support for LM3601x Chips"
> + depends on LEDS_CLASS && I2C
> + depends on LEDS_CLASS_FLASH
> + select REGMAP_I2C
> + help
> + This option enables support for the TI LM3601x family
> + of flash, torch and indicator classes.
> +
> config LEDS_LOCOMO
> tristate "LED Support for Locomo device"
> depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 91eca81cae82..b79807fe1b67 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -76,6 +76,7 @@ obj-$(CONFIG_LEDS_MLXREG) += leds-mlxreg.o
> obj-$(CONFIG_LEDS_NIC78BX) += leds-nic78bx.o
> obj-$(CONFIG_LEDS_MT6323) += leds-mt6323.o
> obj-$(CONFIG_LEDS_LM3692X) += leds-lm3692x.o
> +obj-$(CONFIG_LEDS_LM3601X) += leds-lm3601x.o
>
> # LED SPI Drivers
> obj-$(CONFIG_LEDS_DAC124S085) += leds-dac124s085.o
> diff --git a/drivers/leds/leds-lm3601x.c b/drivers/leds/leds-lm3601x.c
> new file mode 100644
> index 000000000000..081aa71e43a3
> --- /dev/null
> +++ b/drivers/leds/leds-lm3601x.c
> @@ -0,0 +1,487 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Flash and torch driver for Texas Instruments LM3601X LED
> +// Flash driver chip family
> +// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
> +
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/leds.h>
> +#include <linux/led-class-flash.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <uapi/linux/uleds.h>
> +
> +#define LM3601X_LED_IR 0x0
> +#define LM3601X_LED_TORCH 0x1
> +
> +/* Registers */
> +#define LM3601X_ENABLE_REG 0x01
> +#define LM3601X_CFG_REG 0x02
> +#define LM3601X_LED_FLASH_REG 0x03
> +#define LM3601X_LED_TORCH_REG 0x04
> +#define LM3601X_FLAGS_REG 0x05
> +#define LM3601X_DEV_ID_REG 0x06
> +
> +#define LM3601X_SW_RESET BIT(7)
> +
> +/* Enable Mode bits */
> +#define LM3601X_MODE_STANDBY 0x00
> +#define LM3601X_MODE_IR_DRV BIT(0)
> +#define LM3601X_MODE_TORCH BIT(1)
> +#define LM3601X_MODE_STROBE (BIT(0) | BIT(1))
> +#define LM3601X_STRB_EN BIT(2)
> +#define LM3601X_STRB_EDGE_TRIG BIT(3)
> +#define LM3601X_IVFM_EN BIT(4)
> +
> +#define LM36010_BOOST_LIMIT_28 BIT(5)
> +#define LM36010_BOOST_FREQ_4MHZ BIT(6)
> +#define LM36010_BOOST_MODE_PASS BIT(7)
> +
> +/* Flag Mask */
> +#define LM3601X_FLASH_TIME_OUT BIT(0)
> +#define LM3601X_UVLO_FAULT BIT(1)
> +#define LM3601X_THERM_SHUTDOWN BIT(2)
> +#define LM3601X_THERM_CURR BIT(3)
> +#define LM36010_CURR_LIMIT BIT(4)
> +#define LM3601X_SHORT_FAULT BIT(5)
> +#define LM3601X_IVFM_TRIP BIT(6)
> +#define LM36010_OVP_FAULT BIT(7)
> +
> +#define LM3601X_MAX_TORCH_I_UA 376000
> +#define LM3601X_MIN_TORCH_I_UA 2400
> +#define LM3601X_TORCH_REG_DIV 2965
> +
> +#define LM3601X_MAX_STROBE_I_UA 1500000
> +#define LM3601X_MIN_STROBE_I_UA 11000
> +#define LM3601X_STROBE_REG_DIV 11800
> +
> +#define LM3601X_TIMEOUT_MASK 0x1e
> +#define LM3601X_ENABLE_MASK (LM3601X_MODE_IR_DRV | LM3601X_MODE_TORCH)
> +
> +#define LM3601X_LOWER_STEP_US 40000
> +#define LM3601X_UPPER_STEP_US 200000
> +#define LM3601X_MIN_TIMEOUT_US 40000
> +#define LM3601X_MAX_TIMEOUT_US 1600000
> +#define LM3601X_TIMEOUT_XOVER_US 400000
> +
> +enum lm3601x_type {
> + CHIP_LM36010 = 0,
> + CHIP_LM36011,
> +};
> +
> +/**
> + * struct lm3601x_led -
> + * @fled_cdev: flash LED class device pointer
> + * @client: Pointer to the I2C client
> + * @regmap: Devices register map
> + * @lock: Lock for reading/writing the device
> + * @led_name: LED label for the Torch or IR LED
> + * @flash_timeout: the timeout for the flash
> + * @last_flag: last known flags register value
> + * @torch_current_max: maximum current for the torch
> + * @flash_current_max: maximum current for the flash
> + * @max_flash_timeout: maximum timeout for the flash
> + * @led_mode: The mode to enable either IR or Torch
> + */
> +struct lm3601x_led {
> + struct led_classdev_flash fled_cdev;
> + struct i2c_client *client;
> + struct regmap *regmap;
> + struct mutex lock;
> +
> + char led_name[LED_MAX_NAME_SIZE];
> +
> + unsigned int flash_timeout;
> + unsigned int last_flag;
> +
> + u32 torch_current_max;
> + u32 flash_current_max;
> + u32 max_flash_timeout;
> +
> + u32 led_mode;
> +};
> +
> +static const struct reg_default lm3601x_regmap_defs[] = {
> + { LM3601X_ENABLE_REG, 0x20 },
> + { LM3601X_CFG_REG, 0x15 },
> + { LM3601X_LED_FLASH_REG, 0x00 },
> + { LM3601X_LED_TORCH_REG, 0x00 },
> +};
> +
> +static bool lm3601x_volatile_reg(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case LM3601X_FLAGS_REG:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static const struct regmap_config lm3601x_regmap = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +
> + .max_register = LM3601X_DEV_ID_REG,
> + .reg_defaults = lm3601x_regmap_defs,
> + .num_reg_defaults = ARRAY_SIZE(lm3601x_regmap_defs),
> + .cache_type = REGCACHE_RBTREE,
> + .volatile_reg = lm3601x_volatile_reg,
> +};
> +
> +static struct lm3601x_led *fled_cdev_to_led(struct led_classdev_flash *fled_cdev)
> +{
> + return container_of(fled_cdev, struct lm3601x_led, fled_cdev);
> +}
> +
> +static int lm3601x_read_faults(struct lm3601x_led *led)
> +{
> + int flags_val;
> + int ret;
> +
> + ret = regmap_read(led->regmap, LM3601X_FLAGS_REG, &flags_val);
> + if (ret < 0)
> + return -EIO;
> +
> + led->last_flag = 0;
> +
> + if (flags_val & LM36010_OVP_FAULT)
> + led->last_flag |= LED_FAULT_OVER_VOLTAGE;
> +
> + if (flags_val & (LM3601X_THERM_SHUTDOWN | LM3601X_THERM_CURR))
> + led->last_flag |= LED_FAULT_OVER_TEMPERATURE;
> +
> + if (flags_val & LM3601X_SHORT_FAULT)
> + led->last_flag |= LED_FAULT_SHORT_CIRCUIT;
> +
> + if (flags_val & LM36010_CURR_LIMIT)
> + led->last_flag |= LED_FAULT_OVER_CURRENT;
> +
> + if (flags_val & LM3601X_UVLO_FAULT)
> + led->last_flag |= LED_FAULT_UNDER_VOLTAGE;
> +
> + if (flags_val & LM3601X_IVFM_TRIP)
> + led->last_flag |= LED_FAULT_INPUT_VOLTAGE;
> +
> + if (flags_val & LM3601X_THERM_SHUTDOWN)
> + led->last_flag |= LED_FAULT_LED_OVER_TEMPERATURE;
> +
> + return led->last_flag;
> +}
> +
> +static int lm3601x_brightness_set(struct led_classdev *cdev,
> + enum led_brightness brightness)
> +{
> + struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(cdev);
> + struct lm3601x_led *led = fled_cdev_to_led(fled_cdev);
> + int ret, led_mode_val;
> +
> + mutex_lock(&led->lock);
> +
> + ret = lm3601x_read_faults(led);
> + if (ret < 0)
> + goto out;
> +
> + if (led->led_mode == LM3601X_LED_TORCH)
> + led_mode_val = LM3601X_MODE_TORCH;
> + else
> + led_mode_val = LM3601X_MODE_IR_DRV;
> +
> + if (brightness == LED_OFF) {
> + ret = regmap_update_bits(led->regmap, LM3601X_ENABLE_REG,
> + led_mode_val, LED_OFF);
> + goto out;
> + }
> +
> + ret = regmap_write(led->regmap, LM3601X_LED_TORCH_REG, brightness);
> + if (ret < 0)
> + goto out;
> +
> + ret = regmap_update_bits(led->regmap, LM3601X_ENABLE_REG,
> + LM3601X_MODE_TORCH | LM3601X_MODE_IR_DRV,
> + led_mode_val);
> +out:
> + mutex_unlock(&led->lock);
> + return ret;
> +}
> +
> +static int lm3601x_strobe_set(struct led_classdev_flash *fled_cdev,
> + bool state)
> +{
> + struct lm3601x_led *led = fled_cdev_to_led(fled_cdev);
> + int timeout_reg_val;
> + int current_timeout;
> + int ret;
> +
> + mutex_lock(&led->lock);
> +
> + ret = regmap_read(led->regmap, LM3601X_CFG_REG, ¤t_timeout);
> + if (ret < 0)
> + goto out;
> +
> + if (led->flash_timeout >= LM3601X_TIMEOUT_XOVER_US)
> + timeout_reg_val = led->flash_timeout / LM3601X_UPPER_STEP_US + 0x07;
> + else
> + timeout_reg_val = led->flash_timeout / LM3601X_LOWER_STEP_US - 0x01;
> +
> + if (led->flash_timeout != current_timeout)
> + ret = regmap_update_bits(led->regmap, LM3601X_CFG_REG,
> + LM3601X_TIMEOUT_MASK, timeout_reg_val);
> +
> + if (state)
> + ret = regmap_update_bits(led->regmap, LM3601X_ENABLE_REG,
> + LM3601X_MODE_TORCH | LM3601X_MODE_IR_DRV,
> + LM3601X_MODE_STROBE);
> + else
> + ret = regmap_update_bits(led->regmap, LM3601X_ENABLE_REG,
> + LM3601X_MODE_STROBE, LED_OFF);
> +
> + ret = lm3601x_read_faults(led);
> +out:
> + mutex_unlock(&led->lock);
> + return ret;
> +}
> +
> +static int lm3601x_flash_brightness_set(struct led_classdev_flash *fled_cdev,
> + u32 brightness)
> +{
> + struct lm3601x_led *led = fled_cdev_to_led(fled_cdev);
> + u8 brightness_val;
> + int ret;
> +
> + mutex_lock(&led->lock);
> + ret = lm3601x_read_faults(led);
> + if (ret < 0)
> + goto out;
> +
> + if (brightness == LED_OFF) {
> + ret = regmap_update_bits(led->regmap, LM3601X_ENABLE_REG,
> + LM3601X_MODE_STROBE, LED_OFF);
> + goto out;
> + }
> +
> + brightness_val = brightness / LM3601X_STROBE_REG_DIV;
> +
> + ret = regmap_write(led->regmap, LM3601X_LED_FLASH_REG, brightness_val);
> +out:
> + mutex_unlock(&led->lock);
> + return ret;
> +}
> +
> +static int lm3601x_flash_timeout_set(struct led_classdev_flash *fled_cdev,
> + u32 timeout)
> +{
> + struct lm3601x_led *led = fled_cdev_to_led(fled_cdev);
> +
> + mutex_lock(&led->lock);
> +
> + led->flash_timeout = timeout;
> +
> + mutex_unlock(&led->lock);
> +
> + return 0;
> +}
> +
> +static int lm3601x_strobe_get(struct led_classdev_flash *fled_cdev, bool *state)
> +{
> + struct lm3601x_led *led = fled_cdev_to_led(fled_cdev);
> + int strobe_state;
> + int ret;
> +
> + mutex_lock(&led->lock);
> +
> + ret = regmap_read(led->regmap, LM3601X_ENABLE_REG, &strobe_state);
> + if (ret < 0)
> + goto out;
> +
> + *state = strobe_state & LM3601X_MODE_STROBE;
> +
> +out:
> + mutex_unlock(&led->lock);
> + return ret;
> +}
> +
> +static int lm3601x_flash_fault_get(struct led_classdev_flash *fled_cdev,
> + u32 *fault)
> +{
> + struct lm3601x_led *led = fled_cdev_to_led(fled_cdev);
> +
> + lm3601x_read_faults(led);
> +
> + *fault = led->last_flag;
> +
> + return 0;
> +}
> +
> +static const struct led_flash_ops flash_ops = {
> + .flash_brightness_set = lm3601x_flash_brightness_set,
> + .strobe_set = lm3601x_strobe_set,
> + .strobe_get = lm3601x_strobe_get,
> + .timeout_set = lm3601x_flash_timeout_set,
> + .fault_get = lm3601x_flash_fault_get,
> +};
> +
> +static int lm3601x_register_leds(struct lm3601x_led *led)
> +{
> + struct led_classdev *led_cdev;
> + struct led_flash_setting *setting;
> +
> + led->fled_cdev.ops = &flash_ops;
> +
> + setting = &led->fled_cdev.timeout;
> + setting->min = LM3601X_MIN_TIMEOUT_US;
> + setting->max = led->max_flash_timeout;
> + setting->step = LM3601X_LOWER_STEP_US;
> + setting->val = led->max_flash_timeout;
> +
> + setting = &led->fled_cdev.brightness;
> + setting->min = LM3601X_MIN_STROBE_I_UA;
> + setting->max = led->flash_current_max;
> + setting->step = LM3601X_TORCH_REG_DIV;
> + setting->val = led->flash_current_max;
> +
> + led_cdev = &led->fled_cdev.led_cdev;
> + led_cdev->name = led->led_name;
> + led_cdev->brightness_set_blocking = lm3601x_brightness_set;
> + led_cdev->max_brightness = DIV_ROUND_UP(led->torch_current_max,
> + LM3601X_TORCH_REG_DIV);
> + led_cdev->flags |= LED_DEV_CAP_FLASH;
> +
> + return led_classdev_flash_register(&led->client->dev, &led->fled_cdev);
> +}
> +
> +static int lm3601x_parse_node(struct lm3601x_led *led)
> +{
> + struct fwnode_handle *child = NULL;
> + int ret = -ENODEV;
> + const char *name;
> +
> + child = device_get_next_child_node(&led->client->dev, child);
> + if (!child) {
> + dev_err(&led->client->dev, "No LED Child node\n");
> + return ret;
> + }
> +
> + ret = fwnode_property_read_u32(child, "reg", &led->led_mode);
> + if (ret) {
> + dev_err(&led->client->dev, "reg DT property missing\n");
> + goto out_err;
> + }
> +
> + if (led->led_mode > LM3601X_LED_TORCH ||
> + led->led_mode < LM3601X_LED_IR) {
> + dev_warn(&led->client->dev, "Invalid led mode requested\n");
> + ret = -EINVAL;
> + goto out_err;
> + }
> +
> + ret = fwnode_property_read_string(child, "label", &name);
> + if (ret) {
> + if (led->led_mode == LM3601X_LED_TORCH)
> + name = "torch";
> + else
> + name = "infrared";
> + }
> +
> + snprintf(led->led_name, sizeof(led->led_name),
> + "%s:%s", led->client->name, name);
> +
> + ret = fwnode_property_read_u32(child, "led-max-microamp",
> + &led->torch_current_max);
> + if (ret) {
> + dev_warn(&led->client->dev,
> + "led-max-microamp DT property missing\n");
> + goto out_err;
> + }
> +
> + ret = fwnode_property_read_u32(child, "flash-max-microamp",
> + &led->flash_current_max);
> + if (ret) {
> + dev_warn(&led->client->dev,
> + "flash-max-microamp DT property missing\n");
> + goto out_err;
> + }
> +
> + ret = fwnode_property_read_u32(child, "flash-max-timeout-us",
> + &led->max_flash_timeout);
> + if (ret) {
> + dev_warn(&led->client->dev,
> + "flash-max-timeout-us DT property missing\n");
> + goto out_err;
> + }
> +
> +out_err:
> + fwnode_handle_put(child);
> + return ret;
> +}
> +
> +static int lm3601x_probe(struct i2c_client *client)
> +{
> + struct lm3601x_led *led;
> + int ret;
> +
> + led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL);
> + if (!led)
> + return -ENOMEM;
> +
> + led->client = client;
> + i2c_set_clientdata(client, led);
> +
> + ret = lm3601x_parse_node(led);
> + if (ret)
> + return -ENODEV;
> +
> + led->regmap = devm_regmap_init_i2c(client, &lm3601x_regmap);
> + if (IS_ERR(led->regmap)) {
> + ret = PTR_ERR(led->regmap);
> + dev_err(&client->dev,
> + "Failed to allocate register map: %d\n", ret);
> + return ret;
> + }
> +
> + mutex_init(&led->lock);
> +
> + return lm3601x_register_leds(led);
> +}
> +
> +static int lm3601x_remove(struct i2c_client *client)
> +{
> + struct lm3601x_led *led = i2c_get_clientdata(client);
> +
> + led_classdev_flash_unregister(&led->fled_cdev);
> + mutex_destroy(&led->lock);
> +
> + return regmap_update_bits(led->regmap, LM3601X_ENABLE_REG,
> + LM3601X_ENABLE_MASK,
> + LM3601X_MODE_STANDBY);
> +}
> +
> +static const struct i2c_device_id lm3601x_id[] = {
> + { "LM36010", CHIP_LM36010 },
> + { "LM36011", CHIP_LM36011 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, lm3601x_id);
> +
> +static const struct of_device_id of_lm3601x_leds_match[] = {
> + { .compatible = "ti,lm36010", },
> + { .compatible = "ti,lm36011", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, of_lm3601x_leds_match);
> +
> +static struct i2c_driver lm3601x_i2c_driver = {
> + .driver = {
> + .name = "lm3601x",
> + .of_match_table = of_lm3601x_leds_match,
> + },
> + .probe_new = lm3601x_probe,
> + .remove = lm3601x_remove,
> + .id_table = lm3601x_id,
> +};
> +module_i2c_driver(lm3601x_i2c_driver);
> +
> +MODULE_DESCRIPTION("Texas Instruments Flash Lighting driver for LM3601X");
> +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
> +MODULE_LICENSE("GPL v2");
>
Patch set applied to the for-next branch of linux-leds.git.
--
Best regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH v6 1/2] media: ov2680: dt: Add bindings for OV2680
From: Rob Herring @ 2018-05-23 20:04 UTC (permalink / raw)
To: Rui Miguel Silva
Cc: Rui Miguel Silva, Mauro Carvalho Chehab, Sakari Ailus,
Hans Verkuil, Linux Media Mailing List, Fabio Estevam,
Ryan Harkin, devicetree
In-Reply-To: <m3in7lrnl2.fsf@gmail.com>
On Fri, May 18, 2018 at 10:27 AM, Rui Miguel Silva <rmfrfs@gmail.com> wrote:
> Hi Rob,
> On Fri 18 May 2018 at 14:18, Rob Herring wrote:
>>
>> On Wed, May 09, 2018 at 03:31:58PM +0100, Rui Miguel Silva wrote:
>>>
>>> Add device tree binding documentation for the OV2680 camera sensor.
>>>
>>> CC: devicetree@vger.kernel.org
>>> Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
>>> ---
>>> .../devicetree/bindings/media/i2c/ov2680.txt | 46 +++++++++++++++++++
>>> 1 file changed, 46 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/media/i2c/ov2680.txt
>>
>>
>> Please add acks/reviewed bys on new versions.
>
>
> I have add this to the cover letter [0]:
> - Removed Rob Herring Reviewed-by tag, since bindings have changed since
> his
> ack.
>
> But only now I notice that I did not CC the devicetree list for the all
> series, but only for this particular patch. Sorry about that.
NP. It's better to put this and revision history in the individual
patches. I don't always read cover letters anyways.
Rob
^ permalink raw reply
* Re: [v4 07/11] dt-bindings: hwmon: Add documents for PECI hwmon client drivers
From: Jae Hyun Yoo @ 2018-05-23 20:03 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, Haiyue Wang, Vernon Mauery, James Feist, devicetree,
linux-kernel@vger.kernel.org, Andrew Jeffery, Arnd Bergmann,
Jason M Biils, Joel Stanley
In-Reply-To: <CAL_JsqK9ap=p67HDb_aka809nvpzL3K9VOdbMKJbf3EMR8D2XQ@mail.gmail.com>
On 5/23/2018 12:33 PM, Rob Herring wrote:
> On Wed, May 23, 2018 at 11:37 AM, Jae Hyun Yoo
> <jae.hyun.yoo@linux.intel.com> wrote:
>> On 5/23/2018 8:11 AM, Rob Herring wrote:
>>>
>>> On Tue, May 22, 2018 at 12:18 PM, Jae Hyun Yoo
>>> <jae.hyun.yoo@linux.intel.com> wrote:
>>>>
>>>> On 5/22/2018 9:42 AM, Rob Herring wrote:
>>>>>
>>>>>
>>>>> On Mon, May 21, 2018 at 12:59:05PM -0700, Jae Hyun Yoo wrote:
>>>>>>
>>>>>>
>>>>>> This commit adds dt-bindings documents for PECI hwmon client drivers.
>>>>>>
>>>>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>>>>>> Reviewed-by: Haiyue Wang <haiyue.wang@linux.intel.com>
>>>>>> Reviewed-by: James Feist <james.feist@linux.intel.com>
>>>>>> Reviewed-by: Vernon Mauery <vernon.mauery@linux.intel.com>
>>>>>> Cc: Andrew Jeffery <andrew@aj.id.au>
>>>>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>>>>> Cc: Jason M Biils <jason.m.bills@linux.intel.com>
>>>>>> Cc: Joel Stanley <joel@jms.id.au>
>>>>>> ---
>>>>>> .../bindings/hwmon/peci-cputemp.txt | 23
>>>>>> ++++++++++++++++++
>>>>>> .../bindings/hwmon/peci-dimmtemp.txt | 24
>>>>>> +++++++++++++++++++
>>>>>> 2 files changed, 47 insertions(+)
>>>>>> create mode 100644
>>>>>> Documentation/devicetree/bindings/hwmon/peci-cputemp.txt
>>>>>> create mode 100644
>>>>>> Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt
>>>>>> b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt
>>>>>> new file mode 100644
>>>>>> index 000000000000..2f59aee12d9e
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt
>>>>>> @@ -0,0 +1,23 @@
>>>>>> +Bindings for Intel PECI (Platform Environment Control Interface)
>>>>>> cputemp
>>>>>> driver.
>>>>>> +
>>>>>> +Required properties:
>>>>>> +- compatible : Should be "intel,peci-cputemp".
>>>>>> +- reg : Should contain address of a client CPU. Address range
>>>>>> of
>>>>>> CPU
>>>>>> + clients is starting from 0x30 based on PECI
>>>>>> specification.
>>>>>> +
>>>>>> +Example:
>>>>>> + peci-bus@0 {
>>>>>> + #address-cells = <1>;
>>>>>> + #size-cells = <0>;
>>>>>> + < more properties >
>>>>>> +
>>>>>> + peci-cputemp@30 {
>>>>>> + compatible = "intel,peci-cputemp";
>>>>>> + reg = <0x30>;
>>>>>> + };
>>>>>
>>>>>
>>>>>
>>>>> [...]
>>>>>
>>>>>> + peci-dimmtemp@30 {
>>>>>> + compatible = "intel,peci-dimmtemp";
>>>>>> + reg = <0x30>;
>>>>>> + };
>>>>>
>>>>>
>>>>>
>>>>> As I said in the prior version, 2 nodes at the same address is wrong.
>>>>>
>>>>> Rob
>>>>>
>>>>
>>>> In PECI bus, there is one and only bus host (adapter) and multiple
>>>> clients on a PECI bus, and PECI spec doesn't allow multiple originators
>>>> so only the host device can originate message.
>>>
>>>
>>> Yes, I get that. A single host still has to address slave devices.
>>>
>>>> In this implementation,
>>>> all message transactions on a bus from client driver modules and user
>>>> space will be serialized well in the PECI core bus driver so bus
>>>> occupation and traffic arbitration will be managed well in the PECI core
>>>> bus driver even in case of a bus has 2 client drivers at the same
>>>> address. I'm sure that this implementation doesn't make that kind of
>>>> problem to OS.
>>>
>>>
>>> Multiple clients to a single device is common, but that is a software
>>> problem and doesn't belong in DT.
>>>
>>> I don't think there is a single other case in the kernel where
>>> multiple drivers can bind to the same device at a given bus address.
>>> That is why we have things like MFD. Though in this case, why can't
>>> one hwmon driver register multiple hwmon devices (cpu and dimm temps)?
>>>
>>
>> It was implemented as a single driver until v2 but dimm temps need
>> delayed creation unlikely the cpu temps on hwmon subsystem because of
>> memory training behavior of remote x86 cpus. Since hwmon doesn't allow
>> incremental creation, I had to divide it into two, cputemp and dimmtemp,
>> so that cputemp can be registered immediately when the remote x86 cpu
>> turns on and dimmtemp can be registered by delayed creation. It is the
>> reason why I had to make the two hwmon driver modules that sharing a
>> single device address.
>
> That all sounds like kernel problems to me. Stop designing your DT
> binding around what the kernel can or can't *currently* support.
>
>> Additionally, PECI isn't limited for temperature
>> monitoring feature but it can be used for other functions such as
>> platform management, cpu interface tuning and diagnostics and failure
>> analysis, so in case of adding a new driver for the functions, we should
>> add an another DT node which is sharing the same cpu address.
>
> No, the driver should add support for those additional functions.
> Perhaps you will need to use MFD.
>
Do you mean that the device address sharing is acceptable if I make
these nodes under "simple-mfd"?
Thanks,
-Jae
^ permalink raw reply
* Re: [PATCH 1/2] slimbus: ngd: dt-bindings: Add slim ngd dt bindings
From: Srinivas Kandagatla @ 2018-05-23 20:01 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree, Linux-ALSA, girishm, Greg Kroah-Hartman,
Mark Brown, linux-kernel@vger.kernel.org, Banajit Goswami,
Karthikeyan Ramasubramanian, linux-arm-msm, sdharia
In-Reply-To: <CAL_Jsq+YF+EFfqua_GTFU=cwf8oF3HCPCR09MWOoRCJf0-zVnQ@mail.gmail.com>
On 23/05/18 20:28, Rob Herring wrote:
> On Wed, May 23, 2018 at 1:11 PM, Srinivas Kandagatla
> <srinivas.kandagatla@linaro.org> wrote:
>>
>>
>> On 23/05/18 17:40, Rob Herring wrote:
>>>>
>>>> +
>>>> +- qcom,ngd-id
>>>> + Usage: required
>>>> + Value type: <u32>
>>>> + Definition: ngd instance id in the controller
>>>
>>> Why do you need this?
>>>
>> Please ignore my comment from previous reply.
>>
>> There are more than one instances of ngd in this slim controller.
>> We need this to make sure we are programming the correct one.
>
> Doesn't the parent-child relationship of devices on the bus provide
> that?
Thanks for the hint, that sounds like the actual problem here,
If I represent the node with proper parent-child relationship like this,
it will remove the need of this property and would work perfectly in
case we want to support multiple ngds in future!
slim@91c0000 {
compatible = "qcom,msm8996-slim";
reg = <0x91c0000 0x2C000>;
interrupts = <0 163 0>;
dmas = <&slimbam 3>, <&slimbam 4>;
dma-names = "rx", "tx";
#address-cells = <1>;
#size-cells = <1>;
ngd@1 {
reg = <1>;
#address-cells = <1>;
#size-cells = <1>;
codec@1 {
compatible = "slim217,1a0";
reg = <1 0>;
};
};
};
If you mean to provide consistent numbering to userspace, then
> that's not a DT problem (nor one that Linux plans to solve).
>
No, this is not problem am trying to solve.
>> We also need this instance ID during powering it up using QMI.
>
> Wouldn't that be a QMI ID?
It is passed as parameter to SLIMBUS_QMI_SELECT_INSTANCE_REQ_V01 request.
thanks,
srini
>
> Rob
>
^ permalink raw reply
* Re: [v4 3/6] dt-bindings: fsl-qdma: Add NXP Layerscpae qDMA controller bindings
From: Rob Herring @ 2018-05-23 19:59 UTC (permalink / raw)
To: Wen He
Cc: dmaengine@vger.kernel.org, devicetree@vger.kernel.org, Leo Li,
Jiafei Pan, Jiaheng Fan, Vinod
In-Reply-To: <DB6PR0401MB25031C2EFE66E0D90931C4F6E2950@DB6PR0401MB2503.eurprd04.prod.outlook.com>
Updated Vinod's email...
On Mon, May 21, 2018 at 12:52 AM, Wen He <wen.he_1@nxp.com> wrote:
> Hi Rob,
>
> Please see my comments inline.
>
> Best Regards,
> Wen
>
>> -----Original Message-----
>> From: Rob Herring [mailto:robh@kernel.org]
>> Sent: 2018年5月19日 5:26
>> To: Wen He <wen.he_1@nxp.com>
>> Cc: vinod.koul@intel.com; dmaengine@vger.kernel.org;
>> devicetree@vger.kernel.org; Leo Li <leoyang.li@nxp.com>; Jiafei Pan
>> <jiafei.pan@nxp.com>; Jiaheng Fan <jiaheng.fan@nxp.com>
>> Subject: Re: [v4 3/6] dt-bindings: fsl-qdma: Add NXP Layerscpae qDMA
>> controller bindings
>>
>> On Mon, May 14, 2018 at 08:03:04PM +0800, Wen He wrote:
>> > Document the devicetree bindings for NXP Layerscape qDMA controller
>> > which could be found on NXP QorIQ Layerscape SoCs.
>> >
>> > Signed-off-by: Wen He <wen.he_1@nxp.com>
>> > ---
>> > change in v4:
>> > - Rewrite the bindings document that follows generic DMA bindings
>> > file
>> >
>> > change in v3:
>> > - no change
>> >
>> > change in v2:
>> > - Remove indentation
>> > - Add "Should be" before 'fsl,ls1021a-qdma'
>> > - Replace 'channels' by 'dma-channels'
>> > - Replace 'qdma@8390000' by 'dma-controller@8390000'
>> >
>> > Documentation/devicetree/bindings/dma/fsl-qdma.txt | 41
>> ++++++++++++++++++++
>> > 1 files changed, 41 insertions(+), 0 deletions(-) create mode 100644
>> > Documentation/devicetree/bindings/dma/fsl-qdma.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/dma/fsl-qdma.txt
>> > b/Documentation/devicetree/bindings/dma/fsl-qdma.txt
>> > new file mode 100644
>> > index 0000000..368c4e7
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/dma/fsl-qdma.txt
>> > @@ -0,0 +1,41 @@
>> > +NXP Layerscape SoC qDMA Controller
>> > +==================================
>> > +
>> > +This device follows the generic DMA bindings defined in dma/dma.txt.
>> > +
>> > +Required properties:
>> > +
>> > +- compatible: Must be one of
>> > + "fsl,ls1021a-qdma": for LS1021A Board
>> > + "fsl,ls1043a-qdma": for ls1043A Board
>> > + "fsl,ls1046a-qdma": for ls1046A Board
>> > +- reg: Should contain the register's base address and length.
>> > +- interrupts: Should contain a reference to the interrupt used by
>> this
>> > + device.
>> > +- interrupt-names: Should contain interrupt names:
>> > + "qdma-error": the error interrupt
>> > + "qdma-queue": the queue interrupt
>> > +- queues: Should contain number of queues supported.
>>
>> Needs a vendor prefix.
>>
>
> Does means: The queues filed need a vendor prefix ?
> like 'fsl-queues' ? right?
No, vendor prefixes end with a comma: fsl,queues
Rob
^ permalink raw reply
* Re: [PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties
From: Rob Herring @ 2018-05-23 19:55 UTC (permalink / raw)
To: Laurent Pinchart
Cc: devicetree, open list:MEDIA DRIVERS FOR RENESAS - FCP,
Simon Horman, geert, Niklas Söderlund, Jacopo Mondi,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Linux Media Mailing List
In-Reply-To: <1709653.qERUERh18a@avalon>
On Wed, May 23, 2018 at 2:38 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Wednesday, 23 May 2018 19:29:47 EEST Rob Herring wrote:
>> On Wed, May 16, 2018 at 06:32:27PM +0200, Jacopo Mondi wrote:
>> > Describe the optional endpoint properties for endpoint nodes of port@0
>> > and port@1 of the R-Car VIN driver device tree bindings documentation.
>> >
>> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>> > ---
>> >
>> > Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
>> > 1 file changed, 12 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > b/Documentation/devicetree/bindings/media/rcar_vin.txt index
>> > a19517e1..c53ce4e 100644
>> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on
>> > SoC.
>> >
>> > from external SoC pins described in video-interfaces.txt[1].
>> > Describing more then one endpoint in port 0 is invalid. Only VIN
>> > instances that are connected to external pins should have port 0.
>> >
>> > +
>> > + - Optional properties for endpoint nodes of port@0:
>> > + - hsync-active: active state of the HSYNC signal, 0/1 for
>> > LOW/HIGH
>> > + respectively. Default is active high.
>> > + - vsync-active: active state of the VSYNC signal, 0/1 for
>> > LOW/HIGH
>> > + respectively. Default is active high.
>> > +
>> > + If both HSYNC and VSYNC polarities are not specified, embedded
>> > + synchronization is selected.
>>
>> No need to copy-n-paste from video-interfaces.txt. Just "see
>> video-interfaces.txt" for the description is fine.
>
> I would still explicitly list the properties that apply to this binding. I
> agree that there's no need to copy & paste the description of those properties
> though.
Yes, that's what I meant. List each property with "see
video-interfaces.txt" for the description of each.
Rob
^ permalink raw reply
* Re: [PATCH v2 2/5] gpio: syscon: Add gpio-syscon for rockchip
From: Rob Herring @ 2018-05-23 19:53 UTC (permalink / raw)
To: Heiko Stübner
Cc: Levin Du, open list:ARM/Rockchip SoC..., Wayne Chou, devicetree,
Linus Walleij, linux-kernel@vger.kernel.org,
open list:GPIO SUBSYSTEM, Mark Rutland,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <1685755.J6GI985WX3@diego>
On Wed, May 23, 2018 at 10:12 AM, Heiko Stübner <heiko@sntech.de> wrote:
> Hi Rob, Levin,
>
> sorry for being late to the party.
>
> Am Mittwoch, 23. Mai 2018, 16:43:07 CEST schrieb Rob Herring:
>> On Tue, May 22, 2018 at 9:02 PM, Levin Du <djw@t-chip.com.cn> wrote:
>> > On 2018-05-23 2:02 AM, Rob Herring wrote:
>> >> On Fri, May 18, 2018 at 11:52:05AM +0800, djw@t-chip.com.cn wrote:
>> >>> From: Levin Du <djw@t-chip.com.cn>
>> >>>
>> >>> Some GPIOs sit in the GRF_SOC_CON registers of Rockchip SoCs,
>> >>> which do not belong to the general pinctrl.
>> >>>
>> >>> Adding gpio-syscon support makes controlling regulator or
>> >>> LED using these special pins very easy by reusing existing
>> >>> drivers, such as gpio-regulator and led-gpio.
>> >>>
>> >>> Signed-off-by: Levin Du <djw@t-chip.com.cn>
>> >>>
>> >>> ---
>> >>>
>> >>> Changes in v2:
>> >>> - Rename gpio_syscon10 to gpio_mute in doc
>> >>>
>> >>> Changes in v1:
>> >>> - Refactured for general gpio-syscon usage for Rockchip SoCs.
>> >>> - Add doc rockchip,gpio-syscon.txt
>> >>>
>> >>> .../bindings/gpio/rockchip,gpio-syscon.txt | 41
>> >>>
>> >>> ++++++++++++++++++++++
>> >>>
>> >>> drivers/gpio/gpio-syscon.c | 30
>> >>>
>> >>> ++++++++++++++++
>> >>>
>> >>> 2 files changed, 71 insertions(+)
>> >>> create mode 100644
>> >>>
>> >>> Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt
>> >>>
>> >>> diff --git
>> >>> a/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt
>> >>> b/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt
>> >>> new file mode 100644
>> >>> index 0000000..b1b2a67
>> >>> --- /dev/null
>> >>> +++ b/Documentation/devicetree/bindings/gpio/rockchip,gpio-syscon.txt
>> >>> @@ -0,0 +1,41 @@
>> >>> +* Rockchip GPIO support for GRF_SOC_CON registers
>> >>> +
>> >>> +Required properties:
>> >>> +- compatible: Should contain "rockchip,gpio-syscon".
>> >>> +- gpio-controller: Marks the device node as a gpio controller.
>> >>> +- #gpio-cells: Should be two. The first cell is the pin number and
>> >>> + the second cell is used to specify the gpio polarity:
>> >>> + 0 = Active high,
>> >>> + 1 = Active low.
>> >>
>> >> There's no need for this child node. Just make the parent node a gpio
>> >> controller.
>> >>
>> >> Rob
>> >
>> > Hi Rob, it is not clear to me. Do you suggest that the grf node should be
>> > a
>> > gpio controller,
>> > like below?
>> >
>> > + grf: syscon at ff100000 {
>> > + compatible = "rockchip,gpio-syscon", "rockchip,rk3328-grf",
>> > "syscon", "simple-mfd";
>>
>> Yes, but drop "rockchip,gpio-syscon" and "simple-mfd".
>
> I would disagree quite a bit here. The grf are the "general register files",
> a bunch of registers used for quite a lot of things, and so it seems
> among other users, also a gpio-controller for some more random pins
> not controlled through the regular gpio controllers.
>
> For a more fully stocked grf, please see
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/rk3288.dtsi#n855
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3399.dtsi#n1338
>
> So the gpio controller should definitly also be a subnode.
Sigh, yes, if there are a bunch of functions needing subnodes like the
above, then yes that makes sense. But that's not what has been
presented. Please make some attempt at defining *all* the functions.
An actual binding would be nice, but I'll settle for just a list of
things. The list should have functions that have DT dependencies (like
clocks for phys in the above) because until you do, you don't need
child nodes.
> The gpio in question is called "mute", so I'd think the gpio-syscon driver
> should just define a "rockchip,rk3328-gpio-mute" compatible and contain
> all the register voodoo in the driver itself and not define it in the dt.
Is there really just one GPIO? If it has a defined function, then is
it really GP? Can you control direction? I know Linus W doesn't like
that kind of abuse of GPIO.
> So it should probably look like
>
> grf: syscon at ff100000 {
> compatible = "rockchip,rk3328-grf", "syscon", "simple-mfd";
>
> [all the other syscon sub-devices]
>
> gpio_mute: gpio-mute {
> compatible = "rockchip,rk3328-gpio-mute";
> gpio-controller;
> #gpio-cells = <2>;
> };
>
> [more other syscon sub-devices]
> };
^ permalink raw reply
* Re: [PATCH] clk: Add driver for MAX9485
From: Daniel Mack @ 2018-05-23 19:48 UTC (permalink / raw)
To: Rob Herring; +Cc: mturquette, sboyd, linux-clk, devicetree
In-Reply-To: <20180523183320.GA15306@rob-hp-laptop>
Hi Rob,
Thanks for the review!
On Wednesday, May 23, 2018 08:33 PM, Rob Herring wrote:
> On Mon, May 21, 2018 at 10:58:48AM +0200, Daniel Mack wrote:
>> From: Daniel Mack <zonque@gmail.com>
>>
>> This patch adds a driver for MAX9485, a programmable audio clock generator.
>>
>> The device requires a 27.000 MHz clock input. It can provide a gated
>> buffered output of its input clock and two gated outputs of a PLL that can
>> generate one out of 16 discrete frequencies. There is only one PLL however,
>> so the two gated outputs will always have the same frequency but they can
>> be switched individually.
>>
>> The driver for this device exposes 4 clocks in total:
>>
>> - MAX9485_MCLKOUT: A gated, buffered output of the input clock
>> - MAX9485_CLKOUT: A PLL that can be configured to 16 different
>> discrete frequencies
>> - MAX9485_CLKOUT[1,2]: Two gated outputs for MAX9485_CLKOUT
>>
>> Some PLL output frequencies can be achieved with different register
>> settings. The driver will select the one with lowest jitter in such cases.
>>
>> Signed-off-by: Daniel Mack <daniel@zonque.org>
>> ---
>> .../devicetree/bindings/clock/maxim,max9485.txt | 59 ++++
>> drivers/clk/Kconfig | 8 +
>> drivers/clk/Makefile | 1 +
>> drivers/clk/clk-max9485.c | 379 +++++++++++++++++++++
>> include/dt-bindings/clock/maxim,max9485.h | 19 ++
>
> The 2 DT files should be a separate patch.
Sure, I can do that. In order to keep the series bisectable however,
that would mean that the bindings come in first, and then the actual
implementation because that also depends on maxim,max9485.h. Is that okay?
I agree with the rest of your comments.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH net-next v3 0/7] Add support for QCA8334 switch
From: David Miller @ 2018-05-23 19:47 UTC (permalink / raw)
To: vokac.m
Cc: netdev, linux-kernel, devicetree, f.fainelli, vivien.didelot,
andrew, mark.rutland, robh+dt, michal.vokac
In-Reply-To: <1527056424-14528-1-git-send-email-michal.vokac@ysoft.com>
From: "Michal Vokáč" <vokac.m@gmail.com>
Date: Wed, 23 May 2018 08:20:17 +0200
> This series basically adds support for a QCA8334 ethernet switch to the
> qca8k driver. It is a four-port variant of the already supported seven
> port QCA8337. Register map is the same for the whole familly and all chips
> have the same device ID.
>
> Major part of this series enhances the CPU port setting. Currently the CPU
> port is not set to any sensible defaults compatible with the xGMII
> interface. This series forces the CPU port to its maximum bandwidth and
> also allows to adjust the new defaults using fixed-link device tree
> sub-node.
>
> Alongside these changes I fixed two checkpatch warnings regarding SPDX and
> redundant parentheses.
>
> Changes in v3:
> - Rebased on latest net-next/master.
> - Corrected fixed-link documentation.
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties
From: Laurent Pinchart @ 2018-05-23 19:38 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, linux-renesas-soc, horms, geert, niklas.soderlund,
Jacopo Mondi, linux-arm-kernel, linux-media
In-Reply-To: <20180523162947.GA13661@rob-hp-laptop>
Hi Rob,
On Wednesday, 23 May 2018 19:29:47 EEST Rob Herring wrote:
> On Wed, May 16, 2018 at 06:32:27PM +0200, Jacopo Mondi wrote:
> > Describe the optional endpoint properties for endpoint nodes of port@0
> > and port@1 of the R-Car VIN driver device tree bindings documentation.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >
> > Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt
> > b/Documentation/devicetree/bindings/media/rcar_vin.txt index
> > a19517e1..c53ce4e 100644
> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on
> > SoC.
> >
> > from external SoC pins described in video-interfaces.txt[1].
> > Describing more then one endpoint in port 0 is invalid. Only VIN
> > instances that are connected to external pins should have port 0.
> >
> > +
> > + - Optional properties for endpoint nodes of port@0:
> > + - hsync-active: active state of the HSYNC signal, 0/1 for
> > LOW/HIGH
> > + respectively. Default is active high.
> > + - vsync-active: active state of the VSYNC signal, 0/1 for
> > LOW/HIGH
> > + respectively. Default is active high.
> > +
> > + If both HSYNC and VSYNC polarities are not specified, embedded
> > + synchronization is selected.
>
> No need to copy-n-paste from video-interfaces.txt. Just "see
> video-interfaces.txt" for the description is fine.
I would still explicitly list the properties that apply to this binding. I
agree that there's no need to copy & paste the description of those properties
though.
> > +
> >
> > - port 1 - sub-nodes describing one or more endpoints connected to
> >
> > the VIN from local SoC CSI-2 receivers. The endpoint numbers must
> > use the following schema.
> >
> > @@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
> >
> > - Endpoint 2 - sub-node describing the endpoint connected to
> > CSI40
> > - Endpoint 3 - sub-node describing the endpoint connected to
> > CSI41
> >
> > + Endpoint nodes of port@1 do not support any optional endpoint
> > property. +
> >
> > Device node example for Gen2 platforms
> > --------------------------------------
> >
> > @@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite
> > video input)>
> > vin1ep0: endpoint {
> >
> > remote-endpoint = <&adv7180>;
> >
> > - bus-width = <8>;
> >
> > };
> >
> > };
> >
> > };
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 3/5] watchdog: sp805: set WDOG_HW_RUNNING when appropriate
From: Ray Jui @ 2018-05-23 19:35 UTC (permalink / raw)
To: Guenter Roeck, Robin Murphy
Cc: Scott Branden, Mark Rutland, devicetree, linux-watchdog,
Catalin Marinas, Will Deacon, linux-kernel, Rob Herring,
bcm-kernel-feedback-list, Wim Van Sebroeck, Frank Rowand,
linux-arm-kernel
In-Reply-To: <20180523180920.GB27570@roeck-us.net>
Hi Guenter/Robin,
On 5/23/2018 11:09 AM, Guenter Roeck wrote:
> On Wed, May 23, 2018 at 06:15:14PM +0100, Robin Murphy wrote:
>> On 23/05/18 17:29, Ray Jui wrote:
>>> Hi Robin,
>>>
>>> On 5/23/2018 4:48 AM, Robin Murphy wrote:
>>>> On 23/05/18 08:52, Scott Branden wrote:
>>>>>
>>>>>
>>>>> On 18-05-22 04:24 PM, Ray Jui wrote:
>>>>>> Hi Guenter,
>>>>>>
>>>>>> On 5/22/2018 1:54 PM, Guenter Roeck wrote:
>>>>>>> On Tue, May 22, 2018 at 11:47:18AM -0700, Ray Jui wrote:
>>>>>>>> If the watchdog hardware is already enabled during the boot process,
>>>>>>>> when the Linux watchdog driver loads, it should reset the
>>>>>>>> watchdog and
>>>>>>>> tell the watchdog framework. As a result, ping can be generated from
>>>>>>>> the watchdog framework, until the userspace watchdog daemon
>>>>>>>> takes over
>>>>>>>> control
>>>>>>>>
>>>>>>>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>>>>>>>> Reviewed-by: Vladimir Olovyannikov
>>>>>>>> <vladimir.olovyannikov@broadcom.com>
>>>>>>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>>>>>>> ---
>>>>>>>> drivers/watchdog/sp805_wdt.c | 22 ++++++++++++++++++++++
>>>>>>>> 1 file changed, 22 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/watchdog/sp805_wdt.c
>>>>>>>> b/drivers/watchdog/sp805_wdt.c
>>>>>>>> index 1484609..408ffbe 100644
>>>>>>>> --- a/drivers/watchdog/sp805_wdt.c
>>>>>>>> +++ b/drivers/watchdog/sp805_wdt.c
>>>>>>>> @@ -42,6 +42,7 @@
>>>>>>>> /* control register masks */
>>>>>>>> #define INT_ENABLE (1 << 0)
>>>>>>>> #define RESET_ENABLE (1 << 1)
>>>>>>>> + #define ENABLE_MASK (INT_ENABLE | RESET_ENABLE)
>>>>>>>> #define WDTINTCLR 0x00C
>>>>>>>> #define WDTRIS 0x010
>>>>>>>> #define WDTMIS 0x014
>>>>>>>> @@ -74,6 +75,18 @@ module_param(nowayout, bool, 0);
>>>>>>>> MODULE_PARM_DESC(nowayout,
>>>>>>>> "Set to 1 to keep watchdog running after device release");
>>>>>>>> +/* returns true if wdt is running; otherwise returns false */
>>>>>>>> +static bool wdt_is_running(struct watchdog_device *wdd)
>>>>>>>> +{
>>>>>>>> + struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>>>>>>>> +
>>>>>>>> + if ((readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK) ==
>>>>>>>> + ENABLE_MASK)
>>>>>>>> + return true;
>>>>>>>> + else
>>>>>>>> + return false;
>>>>>>>
>>>>>>> return !!(readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK));
>>>>>>>
>>>>>>
>>>>>> Note ENABLE_MASK contains two bits (INT_ENABLE and RESET_ENABLE);
>>>>>> therefore, a simple !!(expression) would not work? That is, the
>>>>>> masked result needs to be compared against the mask again to ensure
>>>>>> both bits are set, right?
>>>>> Ray - your original code looks correct to me. Easier to read and less
>>>>> prone to errors as shown in the attempted translation to a single
>>>>> statement.
>>>>
>>>> if (<boolean condition>)
>>>> return true;
>>>> else
>>>> return false;
>>>>
>>>> still looks really dumb, though, and IMO is actually harder to read than
>>>> just "return <boolean condition>;" because it forces you to stop and
>>>> double-check that the logic is, in fact, only doing the obvious thing.
>>>
>>> If you can propose a way to modify my original code above to make it more
>>> readable, I'm fine to make the change.
>>
>> Well,
>>
>> return readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK == ENABLE_MASK;
>>
>> would probably be reasonable to anyone other than the 80-column zealots, but
>> removing the silly boolean-to-boolean translation idiom really only
>> emphasises the fact that it's fundamentally a big complex statement; for
>> maximum clarity I'd be inclined to separate the two logical operations (read
>> and comparison), e.g.:
>>
>> u32 wdtcontrol = readl_relaxed(wdt->base + WDTCONTROL);
>>
>> return wdtcontrol & ENABLE_MASK == ENABLE_MASK;
>
> == has higher precendence than bitwise &, so this will need ( ),
> but otherwise I agree.
>
Sure. Let me change the code to the following:
u32 wdtcontrol = readl_relaxed(wdt->base + WDTCONTROL);
return (wdtcontrol & ENABLE_MASK) == ENABLE_MASK;
Thanks.
Ray
>>
>> which is still -3 lines vs. the original.
>>
>>> As I mentioned, I don't think the following change proposed by Guenter
>>> will work due to the reason I pointed out:
>>>
>>> return !!(readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK));
>>
>> FWIW, getting the desired result should only need one logical not swapping
>> for a bitwise one there:
>>
>> return !(~readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK);
>>
>> but that's well into "too clever for its own good" territory ;)
>
> Yes, that would be confusing.
>
>>
>> Robin.
^ permalink raw reply
* Re: [v4 07/11] dt-bindings: hwmon: Add documents for PECI hwmon client drivers
From: Rob Herring @ 2018-05-23 19:33 UTC (permalink / raw)
To: Jae Hyun Yoo
Cc: Mark Rutland, Haiyue Wang, Vernon Mauery, James Feist, devicetree,
linux-kernel@vger.kernel.org, Andrew Jeffery, Arnd Bergmann,
Jason M Biils, Joel Stanley
In-Reply-To: <51752610-d2c1-7fbc-101e-e99346fa29e4@linux.intel.com>
On Wed, May 23, 2018 at 11:37 AM, Jae Hyun Yoo
<jae.hyun.yoo@linux.intel.com> wrote:
> On 5/23/2018 8:11 AM, Rob Herring wrote:
>>
>> On Tue, May 22, 2018 at 12:18 PM, Jae Hyun Yoo
>> <jae.hyun.yoo@linux.intel.com> wrote:
>>>
>>> On 5/22/2018 9:42 AM, Rob Herring wrote:
>>>>
>>>>
>>>> On Mon, May 21, 2018 at 12:59:05PM -0700, Jae Hyun Yoo wrote:
>>>>>
>>>>>
>>>>> This commit adds dt-bindings documents for PECI hwmon client drivers.
>>>>>
>>>>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>>>>> Reviewed-by: Haiyue Wang <haiyue.wang@linux.intel.com>
>>>>> Reviewed-by: James Feist <james.feist@linux.intel.com>
>>>>> Reviewed-by: Vernon Mauery <vernon.mauery@linux.intel.com>
>>>>> Cc: Andrew Jeffery <andrew@aj.id.au>
>>>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>>>> Cc: Jason M Biils <jason.m.bills@linux.intel.com>
>>>>> Cc: Joel Stanley <joel@jms.id.au>
>>>>> ---
>>>>> .../bindings/hwmon/peci-cputemp.txt | 23
>>>>> ++++++++++++++++++
>>>>> .../bindings/hwmon/peci-dimmtemp.txt | 24
>>>>> +++++++++++++++++++
>>>>> 2 files changed, 47 insertions(+)
>>>>> create mode 100644
>>>>> Documentation/devicetree/bindings/hwmon/peci-cputemp.txt
>>>>> create mode 100644
>>>>> Documentation/devicetree/bindings/hwmon/peci-dimmtemp.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt
>>>>> b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt
>>>>> new file mode 100644
>>>>> index 000000000000..2f59aee12d9e
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/hwmon/peci-cputemp.txt
>>>>> @@ -0,0 +1,23 @@
>>>>> +Bindings for Intel PECI (Platform Environment Control Interface)
>>>>> cputemp
>>>>> driver.
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible : Should be "intel,peci-cputemp".
>>>>> +- reg : Should contain address of a client CPU. Address range
>>>>> of
>>>>> CPU
>>>>> + clients is starting from 0x30 based on PECI
>>>>> specification.
>>>>> +
>>>>> +Example:
>>>>> + peci-bus@0 {
>>>>> + #address-cells = <1>;
>>>>> + #size-cells = <0>;
>>>>> + < more properties >
>>>>> +
>>>>> + peci-cputemp@30 {
>>>>> + compatible = "intel,peci-cputemp";
>>>>> + reg = <0x30>;
>>>>> + };
>>>>
>>>>
>>>>
>>>> [...]
>>>>
>>>>> + peci-dimmtemp@30 {
>>>>> + compatible = "intel,peci-dimmtemp";
>>>>> + reg = <0x30>;
>>>>> + };
>>>>
>>>>
>>>>
>>>> As I said in the prior version, 2 nodes at the same address is wrong.
>>>>
>>>> Rob
>>>>
>>>
>>> In PECI bus, there is one and only bus host (adapter) and multiple
>>> clients on a PECI bus, and PECI spec doesn't allow multiple originators
>>> so only the host device can originate message.
>>
>>
>> Yes, I get that. A single host still has to address slave devices.
>>
>>> In this implementation,
>>> all message transactions on a bus from client driver modules and user
>>> space will be serialized well in the PECI core bus driver so bus
>>> occupation and traffic arbitration will be managed well in the PECI core
>>> bus driver even in case of a bus has 2 client drivers at the same
>>> address. I'm sure that this implementation doesn't make that kind of
>>> problem to OS.
>>
>>
>> Multiple clients to a single device is common, but that is a software
>> problem and doesn't belong in DT.
>>
>> I don't think there is a single other case in the kernel where
>> multiple drivers can bind to the same device at a given bus address.
>> That is why we have things like MFD. Though in this case, why can't
>> one hwmon driver register multiple hwmon devices (cpu and dimm temps)?
>>
>
> It was implemented as a single driver until v2 but dimm temps need
> delayed creation unlikely the cpu temps on hwmon subsystem because of
> memory training behavior of remote x86 cpus. Since hwmon doesn't allow
> incremental creation, I had to divide it into two, cputemp and dimmtemp,
> so that cputemp can be registered immediately when the remote x86 cpu
> turns on and dimmtemp can be registered by delayed creation. It is the
> reason why I had to make the two hwmon driver modules that sharing a
> single device address.
That all sounds like kernel problems to me. Stop designing your DT
binding around what the kernel can or can't *currently* support.
> Additionally, PECI isn't limited for temperature
> monitoring feature but it can be used for other functions such as
> platform management, cpu interface tuning and diagnostics and failure
> analysis, so in case of adding a new driver for the functions, we should
> add an another DT node which is sharing the same cpu address.
No, the driver should add support for those additional functions.
Perhaps you will need to use MFD.
Rob
^ permalink raw reply
* Re: [PATCH 1/5] Documentation: DT: Add optional 'timeout-sec' property for sp805
From: Ray Jui @ 2018-05-23 19:29 UTC (permalink / raw)
To: Rob Herring
Cc: Robin Murphy, Wim Van Sebroeck, Guenter Roeck, Mark Rutland,
Frank Rowand, Catalin Marinas, Will Deacon, devicetree,
linux-watchdog, linux-kernel, bcm-kernel-feedback-list,
linux-arm-kernel
In-Reply-To: <20180523185944.GA9989@rob-hp-laptop>
On 5/23/2018 11:59 AM, Rob Herring wrote:
> On Wed, May 23, 2018 at 09:25:49AM -0700, Ray Jui wrote:
>>
>>
>> On 5/23/2018 3:57 AM, Robin Murphy wrote:
>>> On 22/05/18 19:47, Ray Jui wrote:
>>>> Update the SP805 binding document to add optional 'timeout-sec'
>>>> devicetree property
>>>>
>>>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>>> ---
>>>> Documentation/devicetree/bindings/watchdog/sp805-wdt.txt | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>>> b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>>> index edc4f0e..f898a86 100644
>>>> --- a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>>> +++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
>>>> @@ -19,6 +19,8 @@ Required properties:
>>>> Optional properties:
>>>> - interrupts : Should specify WDT interrupt number.
>>>> +- timeout-sec : Should specify default WDT timeout in seconds. If
>>>> unset, the
>>>> + default timeout is 30 seconds
>>>
>>> According to the SP805 TRM, the default interval is dependent on the
>>> rate of WDOGCLK, but would typically be a lot longer than that :/
>>>
>>> On a related note, anyone have any idea why we seem to have two subtly
>>> different SP805 bindings defined?
>
> Sigh.
>
>> Interesting, I did not even know that until you pointed this out (and it's
>> funny that I found that I actually reviewed arm,sp805.txt internally in
>> Broadcom code review).
>>
>> It looks like one was done by Bhupesh Sharma (sp805-wdt.txt) and the other
>> was done by Anup Patel (arm,sp805.txt). Both were merged at the same time
>> around March 20, 2016: 915c56bc01d6. I'd assume both were sent out at around
>> the same time.
>>
>> It sounds like we should definitely remove one of them. Given that
>> sp805-wdt.txt appears to have more detailed descriptions on the use of the
>> clocks, should we remove arm,sp805.txt?
>
> Take whichever text you like, but I prefer filenames using the
> compatible string and the correct string is 'arm,sp805' because '-wdt'
> is redundant. You can probably safely just update all the dts files with
> 'arm,sp805' and just remove 'arm,sp805-wdt' because it is not actually
> used (as the ID registers are).
Okay. I'll consolidate everything into arm,sp805.txt. Will also fix all
DTS files to use "arm,sp805". The fix for actual DTS files will be in a
different patch series.
>
> Rob
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox