* [PATCH 2/5] ARM: dts: imx7-colibri: fix PMIC voltages
From: Stefan Agner @ 2017-03-29 23:21 UTC (permalink / raw)
To: shawnguo, kernel
Cc: fabio.estevam, robh+dt, mark.rutland, linux-arm-kernel,
devicetree, linux-kernel, Stefan Agner
In-Reply-To: <20170329232113.19078-1-stefan@agner.ch>
Fix wrong voltage of PWR_EN_+V3.3 rail. The error had no noticeable
effect since no consumer explicitly requested a specific voltage.
Also use round voltages as it is common in other device trees.
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
arch/arm/boot/dts/imx7-colibri.dtsi | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi b/arch/arm/boot/dts/imx7-colibri.dtsi
index a171545478be..47eab920666f 100644
--- a/arch/arm/boot/dts/imx7-colibri.dtsi
+++ b/arch/arm/boot/dts/imx7-colibri.dtsi
@@ -151,29 +151,29 @@
regulators {
reg_DCDC1: DCDC1 { /* V1.0_SOC */
- regulator-min-microvolt = <975000>;
- regulator-max-microvolt = <1125000>;
+ regulator-min-microvolt = <1000000>;
+ regulator-max-microvolt = <1100000>;
regulator-boot-on;
regulator-always-on;
};
reg_DCDC2: DCDC2 { /* V1.1_ARM */
- regulator-min-microvolt = <975000>;
- regulator-max-microvolt = <1125000>;
+ regulator-min-microvolt = <975000>;
+ regulator-max-microvolt = <1100000>;
regulator-boot-on;
regulator-always-on;
};
reg_DCDC3: DCDC3 { /* V1.8 */
- regulator-min-microvolt = <1775000>;
- regulator-max-microvolt = <1825000>;
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
regulator-boot-on;
regulator-always-on;
};
reg_DCDC4: DCDC4 { /* V1.35_DRAM */
- regulator-min-microvolt = <1325000>;
- regulator-max-microvolt = <1375000>;
+ regulator-min-microvolt = <1350000>;
+ regulator-max-microvolt = <1350000>;
regulator-boot-on;
regulator-always-on;
};
@@ -185,29 +185,29 @@
};
reg_LDO2: LDO2 { /* +V1.8_SD */
- regulator-min-microvolt = <1775000>;
- regulator-max-microvolt = <3325000>;
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
regulator-boot-on;
regulator-always-on;
};
reg_LDO3: LDO3 { /* PWR_EN_+V3.3_LPSR */
- regulator-min-microvolt = <3275000>;
- regulator-max-microvolt = <3325000>;
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
regulator-boot-on;
regulator-always-on;
};
reg_LDO4: LDO4 { /* V1.8_LPSR */
- regulator-min-microvolt = <1775000>;
- regulator-max-microvolt = <1825000>;
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
regulator-boot-on;
regulator-always-on;
};
reg_LDO5: LDO5 { /* PWR_EN_+V3.3 */
- regulator-min-microvolt = <1775000>;
- regulator-max-microvolt = <1825000>;
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
regulator-boot-on;
regulator-always-on;
};
--
2.12.1
^ permalink raw reply related
* [PATCH 3/5] ARM: dts: imx7-colibri: allow to disable Ethernet rail
From: Stefan Agner @ 2017-03-29 23:21 UTC (permalink / raw)
To: shawnguo, kernel
Cc: fabio.estevam, robh+dt, mark.rutland, linux-arm-kernel,
devicetree, linux-kernel, Stefan Agner
In-Reply-To: <20170329232113.19078-1-stefan@agner.ch>
The regulator-always-on property on the Ethernet rail prevents Linux
from disabling the rail when Ethernet is shut down (suspend or simply
link down). With this change the regulator framework will disable the
rail when the Ethernet PHY is not used, saving power especially on
carrier board not using Ethernet.
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
arch/arm/boot/dts/imx7-colibri.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi b/arch/arm/boot/dts/imx7-colibri.dtsi
index 47eab920666f..cbcab937b905 100644
--- a/arch/arm/boot/dts/imx7-colibri.dtsi
+++ b/arch/arm/boot/dts/imx7-colibri.dtsi
@@ -181,7 +181,7 @@
reg_LDO1: LDO1 { /* PWR_EN_+V3.3_ETH */
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <3300000>;
- regulator-always-on;
+ regulator-boot-on;
};
reg_LDO2: LDO2 { /* +V1.8_SD */
--
2.12.1
^ permalink raw reply related
* [PATCH 4/5] ARM: dts: imx7-colibri: remove 1.8V fixed regulator
From: Stefan Agner @ 2017-03-29 23:21 UTC (permalink / raw)
To: shawnguo, kernel
Cc: fabio.estevam, robh+dt, mark.rutland, linux-arm-kernel,
devicetree, linux-kernel, Stefan Agner
In-Reply-To: <20170329232113.19078-1-stefan@agner.ch>
The ADC is directly supplied by the PMIC 1.8V rail, remove the
superfluous fixed regulator.
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
arch/arm/boot/dts/imx7-colibri.dtsi | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi b/arch/arm/boot/dts/imx7-colibri.dtsi
index cbcab937b905..2d87489f9105 100644
--- a/arch/arm/boot/dts/imx7-colibri.dtsi
+++ b/arch/arm/boot/dts/imx7-colibri.dtsi
@@ -60,13 +60,6 @@
regulator-max-microvolt = <3300000>;
};
- reg_vref_1v8: regulator-vref-1v8 {
- compatible = "regulator-fixed";
- regulator-name = "vref-1v8";
- regulator-min-microvolt = <1800000>;
- regulator-max-microvolt = <1800000>;
- };
-
sound {
compatible = "simple-audio-card";
simple-audio-card,name = "imx7-sgtl5000";
@@ -85,11 +78,11 @@
};
&adc1 {
- vref-supply = <®_vref_1v8>;
+ vref-supply = <®_DCDC3>;
};
&adc2 {
- vref-supply = <®_vref_1v8>;
+ vref-supply = <®_DCDC3>;
};
&cpu0 {
--
2.12.1
^ permalink raw reply related
* [PATCH 5/5] ARM: dts: imx7-colibri: add Carrier Board 3.3V/5V regulators
From: Stefan Agner @ 2017-03-29 23:21 UTC (permalink / raw)
To: shawnguo, kernel
Cc: fabio.estevam, robh+dt, mark.rutland, linux-arm-kernel,
devicetree, linux-kernel, Stefan Agner
In-Reply-To: <20170329232113.19078-1-stefan@agner.ch>
Model the Carrier Board power distribution by adding a fixed 3.3V
and 5V regulator. The 3.3V regulator is connected to the backlight
as well as the display supply. The 5V regulator is used to supply
USB VBUS.
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi | 17 +++++++++++++++++
arch/arm/boot/dts/imx7d-colibri-eval-v3.dts | 1 +
2 files changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
index 7a98a45a0d95..18bebd6d8d47 100644
--- a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
+++ b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
@@ -48,6 +48,7 @@
panel: panel {
compatible = "edt,et057090dhu";
backlight = <&bl>;
+ power-supply = <®_3v3>;
port {
panel_in: endpoint {
@@ -55,11 +56,27 @@
};
};
};
+
+ reg_3v3: regulator-3v3 {
+ compatible = "regulator-fixed";
+ regulator-name = "3.3V";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ };
+
+ reg_5v0: regulator-5v0 {
+ compatible = "regulator-fixed";
+ regulator-name = "5V";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ };
};
&bl {
brightness-levels = <0 4 8 16 32 64 128 255>;
default-brightness-level = <6>;
+ power-supply = <®_3v3>;
+
status = "okay";
};
diff --git a/arch/arm/boot/dts/imx7d-colibri-eval-v3.dts b/arch/arm/boot/dts/imx7d-colibri-eval-v3.dts
index bd01d2cc642d..a608a14d8c85 100644
--- a/arch/arm/boot/dts/imx7d-colibri-eval-v3.dts
+++ b/arch/arm/boot/dts/imx7d-colibri-eval-v3.dts
@@ -57,6 +57,7 @@
regulator-min-microvolt = <5000000>;
regulator-max-microvolt = <5000000>;
gpio = <&gpio4 7 GPIO_ACTIVE_LOW>;
+ vin-supply = <®_5v0>;
};
};
--
2.12.1
^ permalink raw reply related
* Re: [PATCH 1/2] leds: Add driver for Qualcomm LPG
From: Bjorn Andersson @ 2017-03-30 0:09 UTC (permalink / raw)
To: Pavel Machek
Cc: Rob Herring, Richard Purdie, Jacek Anaszewski, linux-kernel,
linux-leds, linux-arm-msm, Mark Rutland, devicetree
In-Reply-To: <20170329222301.GB7977@amd>
On Wed 29 Mar 15:23 PDT 2017, Pavel Machek wrote:
> On Wed 2017-03-29 12:07:25, Bjorn Andersson wrote:
> > On Tue 28 Mar 19:17 PDT 2017, Rob Herring wrote:
> >
> > > On Thu, Mar 23, 2017 at 09:37:49PM +0100, Pavel Machek wrote:
> > > > Hi!
> > > >
> > > > > The Light Pulse Generator (LPG) is a PWM-block found in a wide range of
> > > > > PMICs from Qualcomm. It can operate on fixed parameters or based on a
> > > > > lookup-table, altering the duty cycle over time - which provides the
> > > > > means for e.g. hardware assisted transitions of LED brightness.
> > > >
> > > > Ok, this is not first hardware that supports something like this. We
> > > > have similar hardware that can do blinking on Nokia N900 -- please
> > > > take a look at leds-lp55*.c
> > >
> > > And perhaps some alignment on the bindings too if the N900 has bindings.
> > >
> >
> > There is a binding for ti,lp55xx, but there's nothing I can reuse from
> > that binding...because it's completely different hardware.
>
> Agreed, if you drop the pattern stuff from the binding, at least for now.
>
I do not have a strong preference to expose these knobs in devicetree
and I do fear that finding some common "pattern" bindings that suits
everyone will be very difficult.
So I'll drop them from the binding for now.
> > > > And it would be really good to provide hardware abstraction. We really
> > > > don't want to have different userspace for LPG and for N900 and for
> > >
> > > I'm interested in what this looks like as several AOSP platforms do
> > > tri-color LEDs with custom sysfs extensions.
> >
> > How to model RGB LEDs has been discussed many times before and I was
> > hoping for that discussion to come to some conclusion during the last 2
> > years, but now I couldn't wait more - we need this driver for
> > db820c.
>
> If you want driver merged quickly, I believe the best way would be to
> leave out pattern support for now. We can merge the basic driver
> easily to 4.12.
>
I'm not that much in a hurry and would rather see that we resolve any
outstanding issues with the implementation of the pattern handling.
But regardless of this we still have the problem that the typical
Qualcomm PMIC has 8 LPG-blocks and any triple could be driving a
RGB-LED. So we would have to create some sort of in-driver-wrapper
around any three instances exposing them as a single LED to the user.
I rather expose the individual channels and make sure that when we
trigger a blink operation or enable a pattern (i.e. the two operations
that do require synchronization) we will perform that synchronization
under the hood.
> > With this driver, as with many existing, you will have 3 LEDs that you
> > set independently.
> >
> > I did implement blinking by using the PWM straight off, so you can't set
> > brightness or synchronize the multiple channels. Perhaps this should be
> > changed to use the ramp generator.
> >
> > To synchronize patterns I suggest that we extend the LUT binding to
> > describe groups and when any LPG trigger a restart of the pattern-walker
> > we trigger all that are grouped.
> >
> > These two changes combined allows you to set brightness and blink with a
> > RGB-LED.
> >
> >
> > But I will have to dig up some hardware that uses the LPG for driving a
> > RGB-LED to be able to test this (and I do prefer that to be done with
> > some incremental patches at some later time, if acceptable).
>
> Incremental patches sound like a good idea, yes.
>
> I'd say that testing with actual RGB LED is not a requirement... as
> long as we design reasonable interface where the synchronizaction will
> be easy.
>
As this relates to the board layout (which LPG-channels are hooked to a
RGB) I think it makes sense to expose a mechanism in devicetree to
indicate which channels should have their pattern/blink synchronized.
We should be able to extend the LUT (the hardware that actually
implements the pattern-walker logic) with a DT-property like:
qcom,synchronize-group-0 = <1, 2, 3>;
qcom,synchronize-group-1 = <5, 6, 7>;
And whenever we configure a pattern involving one of the affected LEDs
from a group we start all of them.
I'll implement this in a separate patch and include in version 2 as
well.
Regards,
Bjorn
^ permalink raw reply
* [PATCH] ARM: dts: Add devicetree for the Raspberry Pi 3, for arm32 (v4)
From: Eric Anholt @ 2017-03-30 0:26 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Lee Jones,
Florian Fainelli, Olof Johansson, Rob Herring, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Stephen Warren,
Stefan Wahren, bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
Gerd Hoffmann, Eric Anholt
Raspbian and Fedora have decided to support the Pi3 in 32-bit mode for
now, so it's useful to be able to test that mode on an upstream
kernel. It's also been useful for me to use the same board for 32-bit
and 64-bit development.
Signed-off-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
---
v1: Gerd's patch that put the ../../../arm64/... link in the Makefile
v2: Michael's patch that #included from ../../../arm64/... in a new
bcm2837-rpi-3-b.dts.
v3: Mine, using symlinks to make sure that we don't break the split DT
tree.
v4: Rely on the new include/arm64 symlink.
Assuming positive review feedback, I assume it would be acceptable to
merge the shared/dt-symlinks branch in a PR of my own for the 32-bit
DT branch?
arch/arm/boot/dts/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 011808490fed..27d258cb50f2 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -72,6 +72,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
bcm2835-rpi-b-plus.dtb \
bcm2835-rpi-a-plus.dtb \
bcm2836-rpi-2-b.dtb \
+ include/arm64/broadcom/bcm2837-rpi-3-b.dtb \
bcm2835-rpi-zero.dtb
dtb-$(CONFIG_ARCH_BCM_5301X) += \
bcm4708-asus-rt-ac56u.dtb \
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 0/6] bus: brcmstb_gisb: add support for GISBv7 arbiter
From: Doug Berger @ 2017-03-30 0:29 UTC (permalink / raw)
To: mark.rutland-5wv7dgnIgG8
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
gregory.0xf0-Re5JQEeQqe8AvxtiuMwx3w,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
treding-DDmLM1+adcrQT0dZR+AlfA, jonathanh-DDmLM1+adcrQT0dZR+AlfA,
olof-nZhT3qVonbNeoWH0uzbU5w, mirza.krak-Re5JQEeQqe8AvxtiuMwx3w,
suzuki.poulose-5wv7dgnIgG8, bgolaszewski-rdvid1DuHRBWk0Htik3J/w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8, Doug Berger
This patch set contains changes to enable the GISB arbiter driver
on the latest ARM64 architecture Set-Top Box chips from Broadcom.
Since the ARM64 architecture does not support the hooking of low
level fault handlers the driver has been adjusted to depend solely
on GISB interrupts and notify events to provide diagnostic
messaging. The GISB hardware still triggers bus faults for the
processor so the default low-level aborts will occur and will be
handled based on the architecture specific kernel implementation.
While this tends to obsure the GISB error messaging it is still
reasonable so the same approach is applied to the ARM architecture
for consistency.
The patches also correct some issues with the existing driver
and add the new register map for the GISBv7 hardware first
appearing in the BCM7278 device.
Changes since v2 at [2]:
- Removed code associated with enabling the hooking of ARM64
memory faults as well as the code that hooks them for both
ARM and ARM64 architectures within the GISB arbiter driver.
- Created separate notifiers for die and panic events to
prevent list corruption, and substituted the notifier return
name for the previous magic number (Thanks Mark!)
Changes since v1 at [1]:
- Removed code associated with hooking SError handling in favor
of a registered notifier (Thanks Mark!)
- Removed an unnecessary explicit cast (Thanks Gregory!)
[1] https://lkml.org/lkml/2017/3/24/413
[2] https://lkml.org/lkml/2017/3/28/1125
Doug Berger (6):
bus: brcmstb_gisb: Use register offsets with writes too
bus: brcmstb_gisb: correct support for 64-bit address output
bus: brcmstb_gisb: add notifier handling
bus: brcmstb_gisb: remove low-level ARM hooks
bus: brcmstb_gisb: enable driver for ARM64 architecture
bus: brcmstb_gisb: update to support new revision
.../devicetree/bindings/bus/brcm,gisb-arb.txt | 3 +-
drivers/bus/Kconfig | 2 +-
drivers/bus/brcmstb_gisb.c | 121 ++++++++++++---------
3 files changed, 75 insertions(+), 51 deletions(-)
--
2.12.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3 1/6] bus: brcmstb_gisb: Use register offsets with writes too
From: Doug Berger @ 2017-03-30 0:29 UTC (permalink / raw)
To: mark.rutland
Cc: robh+dt, computersforpeace, gregory.0xf0, f.fainelli,
bcm-kernel-feedback-list, linus.walleij, treding, jonathanh, olof,
mirza.krak, suzuki.poulose, bgolaszewski, devicetree,
linux-kernel, linux-arm-kernel, catalin.marinas, will.deacon,
Doug Berger
In-Reply-To: <20170330002914.7371-1-opendmb@gmail.com>
This commit corrects the bug introduced in commit f80835875d3d
("bus: brcmstb_gisb: Look up register offsets in a table") such
that gisb_write() translates the register enumeration into an
offset from the base address for writes as well as reads.
Fixes: f80835875d3d ("bus: brcmstb_gisb: Look up register offsets in a table")
Signed-off-by: Doug Berger <opendmb@gmail.com>
Acked-by: Gregory Fong <gregory.0xf0@gmail.com>
---
drivers/bus/brcmstb_gisb.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
index 72fe0a5a8bf3..a94598d0945a 100644
--- a/drivers/bus/brcmstb_gisb.c
+++ b/drivers/bus/brcmstb_gisb.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2014 Broadcom Corporation
+ * Copyright (C) 2014-2017 Broadcom
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -127,9 +127,9 @@ static void gisb_write(struct brcmstb_gisb_arb_device *gdev, u32 val, int reg)
return;
if (gdev->big_endian)
- iowrite32be(val, gdev->base + reg);
+ iowrite32be(val, gdev->base + offset);
else
- iowrite32(val, gdev->base + reg);
+ iowrite32(val, gdev->base + offset);
}
static ssize_t gisb_arb_get_timeout(struct device *dev,
--
2.12.0
^ permalink raw reply related
* [PATCH v3 2/6] bus: brcmstb_gisb: correct support for 64-bit address output
From: Doug Berger @ 2017-03-30 0:29 UTC (permalink / raw)
To: mark.rutland
Cc: robh+dt, computersforpeace, gregory.0xf0, f.fainelli,
bcm-kernel-feedback-list, linus.walleij, treding, jonathanh, olof,
mirza.krak, suzuki.poulose, bgolaszewski, devicetree,
linux-kernel, linux-arm-kernel, catalin.marinas, will.deacon,
Doug Berger
In-Reply-To: <20170330002914.7371-1-opendmb@gmail.com>
The GISB bus can support addresses beyond 32-bits. So this commit
corrects support for reading a captured 64-bit address into a 64-bit
variable by obtaining the high bits from the ARB_ERR_CAP_HI_ADDR
register (when present) and then outputting the full 64-bit value.
It also removes unused definitions.
Fixes: 44127b771d9c ("bus: add Broadcom GISB bus arbiter timeout/error handler")
Signed-off-by: Doug Berger <opendmb@gmail.com>
Acked-by: Gregory Fong <gregory.0xf0@gmail.com>
---
drivers/bus/brcmstb_gisb.c | 36 ++++++++++++++++++++----------------
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
index a94598d0945a..017c37b9c7c1 100644
--- a/drivers/bus/brcmstb_gisb.c
+++ b/drivers/bus/brcmstb_gisb.c
@@ -37,8 +37,6 @@
#define ARB_ERR_CAP_CLEAR (1 << 0)
#define ARB_ERR_CAP_STATUS_TIMEOUT (1 << 12)
#define ARB_ERR_CAP_STATUS_TEA (1 << 11)
-#define ARB_ERR_CAP_STATUS_BS_SHIFT (1 << 2)
-#define ARB_ERR_CAP_STATUS_BS_MASK 0x3c
#define ARB_ERR_CAP_STATUS_WRITE (1 << 1)
#define ARB_ERR_CAP_STATUS_VALID (1 << 0)
@@ -47,7 +45,6 @@ enum {
ARB_ERR_CAP_CLR,
ARB_ERR_CAP_HI_ADDR,
ARB_ERR_CAP_ADDR,
- ARB_ERR_CAP_DATA,
ARB_ERR_CAP_STATUS,
ARB_ERR_CAP_MASTER,
};
@@ -57,7 +54,6 @@ static const int gisb_offsets_bcm7038[] = {
[ARB_ERR_CAP_CLR] = 0x0c4,
[ARB_ERR_CAP_HI_ADDR] = -1,
[ARB_ERR_CAP_ADDR] = 0x0c8,
- [ARB_ERR_CAP_DATA] = 0x0cc,
[ARB_ERR_CAP_STATUS] = 0x0d0,
[ARB_ERR_CAP_MASTER] = -1,
};
@@ -67,7 +63,6 @@ static const int gisb_offsets_bcm7400[] = {
[ARB_ERR_CAP_CLR] = 0x0c8,
[ARB_ERR_CAP_HI_ADDR] = -1,
[ARB_ERR_CAP_ADDR] = 0x0cc,
- [ARB_ERR_CAP_DATA] = 0x0d0,
[ARB_ERR_CAP_STATUS] = 0x0d4,
[ARB_ERR_CAP_MASTER] = 0x0d8,
};
@@ -77,7 +72,6 @@ static const int gisb_offsets_bcm7435[] = {
[ARB_ERR_CAP_CLR] = 0x168,
[ARB_ERR_CAP_HI_ADDR] = -1,
[ARB_ERR_CAP_ADDR] = 0x16c,
- [ARB_ERR_CAP_DATA] = 0x170,
[ARB_ERR_CAP_STATUS] = 0x174,
[ARB_ERR_CAP_MASTER] = 0x178,
};
@@ -87,7 +81,6 @@ static const int gisb_offsets_bcm7445[] = {
[ARB_ERR_CAP_CLR] = 0x7e4,
[ARB_ERR_CAP_HI_ADDR] = 0x7e8,
[ARB_ERR_CAP_ADDR] = 0x7ec,
- [ARB_ERR_CAP_DATA] = 0x7f0,
[ARB_ERR_CAP_STATUS] = 0x7f4,
[ARB_ERR_CAP_MASTER] = 0x7f8,
};
@@ -109,9 +102,13 @@ static u32 gisb_read(struct brcmstb_gisb_arb_device *gdev, int reg)
{
int offset = gdev->gisb_offsets[reg];
- /* return 1 if the hardware doesn't have ARB_ERR_CAP_MASTER */
- if (offset == -1)
- return 1;
+ if (offset < 0) {
+ /* return 1 if the hardware doesn't have ARB_ERR_CAP_MASTER */
+ if (reg == ARB_ERR_CAP_MASTER)
+ return 1;
+ else
+ return 0;
+ }
if (gdev->big_endian)
return ioread32be(gdev->base + offset);
@@ -119,6 +116,16 @@ static u32 gisb_read(struct brcmstb_gisb_arb_device *gdev, int reg)
return ioread32(gdev->base + offset);
}
+static u64 gisb_read_address(struct brcmstb_gisb_arb_device *gdev)
+{
+ u64 value;
+
+ value = gisb_read(gdev, ARB_ERR_CAP_ADDR);
+ value |= (u64)gisb_read(gdev, ARB_ERR_CAP_HI_ADDR) << 32;
+
+ return value;
+}
+
static void gisb_write(struct brcmstb_gisb_arb_device *gdev, u32 val, int reg)
{
int offset = gdev->gisb_offsets[reg];
@@ -185,7 +192,7 @@ static int brcmstb_gisb_arb_decode_addr(struct brcmstb_gisb_arb_device *gdev,
const char *reason)
{
u32 cap_status;
- unsigned long arb_addr;
+ u64 arb_addr;
u32 master;
const char *m_name;
char m_fmt[11];
@@ -197,10 +204,7 @@ static int brcmstb_gisb_arb_decode_addr(struct brcmstb_gisb_arb_device *gdev,
return 1;
/* Read the address and master */
- arb_addr = gisb_read(gdev, ARB_ERR_CAP_ADDR) & 0xffffffff;
-#if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT))
- arb_addr |= (u64)gisb_read(gdev, ARB_ERR_CAP_HI_ADDR) << 32;
-#endif
+ arb_addr = gisb_read_address(gdev);
master = gisb_read(gdev, ARB_ERR_CAP_MASTER);
m_name = brcmstb_gisb_master_to_str(gdev, master);
@@ -209,7 +213,7 @@ static int brcmstb_gisb_arb_decode_addr(struct brcmstb_gisb_arb_device *gdev,
m_name = m_fmt;
}
- pr_crit("%s: %s at 0x%lx [%c %s], core: %s\n",
+ pr_crit("%s: %s at 0x%llx [%c %s], core: %s\n",
__func__, reason, arb_addr,
cap_status & ARB_ERR_CAP_STATUS_WRITE ? 'W' : 'R',
cap_status & ARB_ERR_CAP_STATUS_TIMEOUT ? "timeout" : "",
--
2.12.0
^ permalink raw reply related
* [PATCH v3 3/6] bus: brcmstb_gisb: add notifier handling
From: Doug Berger @ 2017-03-30 0:29 UTC (permalink / raw)
To: mark.rutland
Cc: treding, f.fainelli, mirza.krak, catalin.marinas, linus.walleij,
suzuki.poulose, will.deacon, linux-kernel, jonathanh,
bgolaszewski, devicetree, robh+dt, bcm-kernel-feedback-list,
gregory.0xf0, olof, Doug Berger, computersforpeace,
linux-arm-kernel
In-Reply-To: <20170330002914.7371-1-opendmb@gmail.com>
Check for GISB arbitration errors through a chained notifier
when a process dies or a kernel panic occurs. This allows a
meaningful GISB diagnostic message to occur along with other
diagnostic information from the event.
Signed-off-by: Doug Berger <opendmb@gmail.com>
---
drivers/bus/brcmstb_gisb.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
index 017c37b9c7c1..a2e1c5c25a3a 100644
--- a/drivers/bus/brcmstb_gisb.c
+++ b/drivers/bus/brcmstb_gisb.c
@@ -24,6 +24,9 @@
#include <linux/of.h>
#include <linux/bitops.h>
#include <linux/pm.h>
+#include <linux/kernel.h>
+#include <linux/kdebug.h>
+#include <linux/notifier.h>
#ifdef CONFIG_ARM
#include <asm/bug.h>
@@ -283,6 +286,36 @@ static irqreturn_t brcmstb_gisb_tea_handler(int irq, void *dev_id)
return IRQ_HANDLED;
}
+/*
+ * Dump out gisb errors on die or panic.
+ */
+static int dump_gisb_error(struct notifier_block *self, unsigned long v,
+ void *p);
+
+static struct notifier_block gisb_die_notifier = {
+ .notifier_call = dump_gisb_error,
+};
+
+static struct notifier_block gisb_panic_notifier = {
+ .notifier_call = dump_gisb_error,
+};
+
+static int dump_gisb_error(struct notifier_block *self, unsigned long v,
+ void *p)
+{
+ struct brcmstb_gisb_arb_device *gdev;
+ const char *reason = "panic";
+
+ if (self == &gisb_die_notifier)
+ reason = "die";
+
+ /* iterate over each GISB arb registered handlers */
+ list_for_each_entry(gdev, &brcmstb_gisb_arb_device_list, next)
+ brcmstb_gisb_arb_decode_addr(gdev, reason);
+
+ return NOTIFY_DONE;
+}
+
static DEVICE_ATTR(gisb_arb_timeout, S_IWUSR | S_IRUGO,
gisb_arb_get_timeout, gisb_arb_set_timeout);
@@ -390,6 +423,12 @@ static int __init brcmstb_gisb_arb_probe(struct platform_device *pdev)
board_be_handler = brcmstb_bus_error_handler;
#endif
+ if (list_is_singular(&brcmstb_gisb_arb_device_list)) {
+ register_die_notifier(&gisb_die_notifier);
+ atomic_notifier_chain_register(&panic_notifier_list,
+ &gisb_panic_notifier);
+ }
+
dev_info(&pdev->dev, "registered mem: %p, irqs: %d, %d\n",
gdev->base, timeout_irq, tea_irq);
--
2.12.0
^ permalink raw reply related
* [PATCH v3 4/6] bus: brcmstb_gisb: remove low-level ARM hooks
From: Doug Berger @ 2017-03-30 0:29 UTC (permalink / raw)
To: mark.rutland-5wv7dgnIgG8
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
gregory.0xf0-Re5JQEeQqe8AvxtiuMwx3w,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
treding-DDmLM1+adcrQT0dZR+AlfA, jonathanh-DDmLM1+adcrQT0dZR+AlfA,
olof-nZhT3qVonbNeoWH0uzbU5w, mirza.krak-Re5JQEeQqe8AvxtiuMwx3w,
suzuki.poulose-5wv7dgnIgG8, bgolaszewski-rdvid1DuHRBWk0Htik3J/w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8, Doug Berger
In-Reply-To: <20170330002914.7371-1-opendmb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
The ARM architecture can provide meaningful diagnostic
output from the GISB arbiter solely from interrupts and
notifiers without the need to hook the low level fault
handlers.
Signed-off-by: Doug Berger <opendmb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/bus/brcmstb_gisb.c | 30 ------------------------------
1 file changed, 30 deletions(-)
diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
index a2e1c5c25a3a..3fbc116e6b95 100644
--- a/drivers/bus/brcmstb_gisb.c
+++ b/drivers/bus/brcmstb_gisb.c
@@ -28,11 +28,6 @@
#include <linux/kdebug.h>
#include <linux/notifier.h>
-#ifdef CONFIG_ARM
-#include <asm/bug.h>
-#include <asm/signal.h>
-#endif
-
#ifdef CONFIG_MIPS
#include <asm/traps.h>
#endif
@@ -228,27 +223,6 @@ static int brcmstb_gisb_arb_decode_addr(struct brcmstb_gisb_arb_device *gdev,
return 0;
}
-#ifdef CONFIG_ARM
-static int brcmstb_bus_error_handler(unsigned long addr, unsigned int fsr,
- struct pt_regs *regs)
-{
- int ret = 0;
- struct brcmstb_gisb_arb_device *gdev;
-
- /* iterate over each GISB arb registered handlers */
- list_for_each_entry(gdev, &brcmstb_gisb_arb_device_list, next)
- ret |= brcmstb_gisb_arb_decode_addr(gdev, "bus error");
- /*
- * If it was an imprecise abort, then we need to correct the
- * return address to be _after_ the instruction.
- */
- if (fsr & (1 << 10))
- regs->ARM_pc += 4;
-
- return ret;
-}
-#endif
-
#ifdef CONFIG_MIPS
static int brcmstb_bus_error_handler(struct pt_regs *regs, int is_fixup)
{
@@ -415,10 +389,6 @@ static int __init brcmstb_gisb_arb_probe(struct platform_device *pdev)
list_add_tail(&gdev->next, &brcmstb_gisb_arb_device_list);
-#ifdef CONFIG_ARM
- hook_fault_code(22, brcmstb_bus_error_handler, SIGBUS, 0,
- "imprecise external abort");
-#endif
#ifdef CONFIG_MIPS
board_be_handler = brcmstb_bus_error_handler;
#endif
--
2.12.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 5/6] bus: brcmstb_gisb: enable driver for ARM64 architecture
From: Doug Berger @ 2017-03-30 0:29 UTC (permalink / raw)
To: mark.rutland-5wv7dgnIgG8
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
computersforpeace-Re5JQEeQqe8AvxtiuMwx3w,
gregory.0xf0-Re5JQEeQqe8AvxtiuMwx3w,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
treding-DDmLM1+adcrQT0dZR+AlfA, jonathanh-DDmLM1+adcrQT0dZR+AlfA,
olof-nZhT3qVonbNeoWH0uzbU5w, mirza.krak-Re5JQEeQqe8AvxtiuMwx3w,
suzuki.poulose-5wv7dgnIgG8, bgolaszewski-rdvid1DuHRBWk0Htik3J/w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8, Doug Berger
In-Reply-To: <20170330002914.7371-1-opendmb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
The ARM64 architecture can provide meaningful diagnostic
output from the GISB arbiter solely from interrupts and
notifiers without the need to hook the low level fault
handlers.
Signed-off-by: Doug Berger <opendmb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/bus/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 0a52da439abf..d2a5f1184022 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -57,7 +57,7 @@ config ARM_CCN
config BRCMSTB_GISB_ARB
bool "Broadcom STB GISB bus arbiter"
- depends on ARM || MIPS
+ depends on ARM || ARM64 || MIPS
default ARCH_BRCMSTB || BMIPS_GENERIC
help
Driver for the Broadcom Set Top Box System-on-a-chip internal bus
--
2.12.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 6/6] bus: brcmstb_gisb: update to support new revision
From: Doug Berger @ 2017-03-30 0:29 UTC (permalink / raw)
To: mark.rutland
Cc: robh+dt, computersforpeace, gregory.0xf0, f.fainelli,
bcm-kernel-feedback-list, linus.walleij, treding, jonathanh, olof,
mirza.krak, suzuki.poulose, bgolaszewski, devicetree,
linux-kernel, linux-arm-kernel, catalin.marinas, will.deacon,
Doug Berger
In-Reply-To: <20170330002914.7371-1-opendmb@gmail.com>
The 7278 introduces a new version of this core. This
commit adds support for that revision.
Signed-off-by: Doug Berger <opendmb@gmail.com>
---
Documentation/devicetree/bindings/bus/brcm,gisb-arb.txt | 3 ++-
drivers/bus/brcmstb_gisb.c | 10 ++++++++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/bus/brcm,gisb-arb.txt b/Documentation/devicetree/bindings/bus/brcm,gisb-arb.txt
index 1eceefb20f01..8a6c3c2e58fe 100644
--- a/Documentation/devicetree/bindings/bus/brcm,gisb-arb.txt
+++ b/Documentation/devicetree/bindings/bus/brcm,gisb-arb.txt
@@ -3,7 +3,8 @@ Broadcom GISB bus Arbiter controller
Required properties:
- compatible:
- "brcm,gisb-arb" or "brcm,bcm7445-gisb-arb" for 28nm chips
+ "brcm,bcm7278-gisb-arb" for V7 28nm chips
+ "brcm,gisb-arb" or "brcm,bcm7445-gisb-arb" for other 28nm chips
"brcm,bcm7435-gisb-arb" for newer 40nm chips
"brcm,bcm7400-gisb-arb" for older 40nm chips and all 65nm chips
"brcm,bcm7038-gisb-arb" for 130nm chips
diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
index 3fbc116e6b95..68ac3e93b600 100644
--- a/drivers/bus/brcmstb_gisb.c
+++ b/drivers/bus/brcmstb_gisb.c
@@ -56,6 +56,15 @@ static const int gisb_offsets_bcm7038[] = {
[ARB_ERR_CAP_MASTER] = -1,
};
+static const int gisb_offsets_bcm7278[] = {
+ [ARB_TIMER] = 0x008,
+ [ARB_ERR_CAP_CLR] = 0x7f8,
+ [ARB_ERR_CAP_HI_ADDR] = -1,
+ [ARB_ERR_CAP_ADDR] = 0x7e0,
+ [ARB_ERR_CAP_STATUS] = 0x7f0,
+ [ARB_ERR_CAP_MASTER] = 0x7f4,
+};
+
static const int gisb_offsets_bcm7400[] = {
[ARB_TIMER] = 0x00c,
[ARB_ERR_CAP_CLR] = 0x0c8,
@@ -307,6 +316,7 @@ static const struct of_device_id brcmstb_gisb_arb_of_match[] = {
{ .compatible = "brcm,bcm7445-gisb-arb", .data = gisb_offsets_bcm7445 },
{ .compatible = "brcm,bcm7435-gisb-arb", .data = gisb_offsets_bcm7435 },
{ .compatible = "brcm,bcm7400-gisb-arb", .data = gisb_offsets_bcm7400 },
+ { .compatible = "brcm,bcm7278-gisb-arb", .data = gisb_offsets_bcm7278 },
{ .compatible = "brcm,bcm7038-gisb-arb", .data = gisb_offsets_bcm7038 },
{ },
};
--
2.12.0
^ permalink raw reply related
* [PATCH 1/2] clk: imx7d: fix USDHC NAND clock
From: Stefan Agner @ 2017-03-30 0:50 UTC (permalink / raw)
To: shawnguo-DgEjT+Ai2ygdnm+yROfE0A, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ
Cc: aisheng.dong-3arQi8VN3Tc, fabio.estevam-3arQi8VN3Tc,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Stefan Agner
The USDHC NAND root clock is not gated by any CCM clock gate. Remove
the bogus gate definition.
Signed-off-by: Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org>
---
The IMX7D_NAND_USDHC_BUS_ROOT_CLK clock is also in clks_init_on.
In a quick test I removed IMX7D_NAND_USDHC_BUS_ROOT_CLK from
clks_init_on, and the system including SD-cards seemed to work
fine... So I guess we could remove the clock from clks_init_on
after the two both changes got applied, any thoughts?
The gate 0x4130 was actually the DRAM gate, hence disabling that
clock lead to disabling this gate, and hence a crash before this
fixes... Maybe that was the reason it landed in clks_init_on...?
--
Stefan
drivers/clk/imx/clk-imx7d.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
index ae1d31be906e..4466acaacb71 100644
--- a/drivers/clk/imx/clk-imx7d.c
+++ b/drivers/clk/imx/clk-imx7d.c
@@ -724,7 +724,7 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
clks[IMX7D_MAIN_AXI_ROOT_DIV] = imx_clk_divider2("axi_post_div", "axi_pre_div", base + 0x8800, 0, 6);
clks[IMX7D_DISP_AXI_ROOT_DIV] = imx_clk_divider2("disp_axi_post_div", "disp_axi_pre_div", base + 0x8880, 0, 6);
clks[IMX7D_ENET_AXI_ROOT_DIV] = imx_clk_divider2("enet_axi_post_div", "enet_axi_pre_div", base + 0x8900, 0, 6);
- clks[IMX7D_NAND_USDHC_BUS_ROOT_DIV] = imx_clk_divider2("nand_usdhc_post_div", "nand_usdhc_pre_div", base + 0x8980, 0, 6);
+ clks[IMX7D_NAND_USDHC_BUS_ROOT_CLK] = imx_clk_divider2("nand_usdhc_root_clk", "nand_usdhc_pre_div", base + 0x8980, 0, 6);
clks[IMX7D_AHB_CHANNEL_ROOT_DIV] = imx_clk_divider2("ahb_post_div", "ahb_pre_div", base + 0x9000, 0, 6);
clks[IMX7D_DRAM_ROOT_DIV] = imx_clk_divider2("dram_post_div", "dram_cg", base + 0x9880, 0, 3);
clks[IMX7D_DRAM_PHYM_ALT_ROOT_DIV] = imx_clk_divider2("dram_phym_alt_post_div", "dram_phym_alt_pre_div", base + 0xa000, 0, 3);
@@ -797,7 +797,6 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
clks[IMX7D_ENET_AXI_ROOT_CLK] = imx_clk_gate4("enet_axi_root_clk", "enet_axi_post_div", base + 0x4060, 0);
clks[IMX7D_OCRAM_CLK] = imx_clk_gate4("ocram_clk", "axi_post_div", base + 0x4110, 0);
clks[IMX7D_OCRAM_S_CLK] = imx_clk_gate4("ocram_s_clk", "ahb_post_div", base + 0x4120, 0);
- clks[IMX7D_NAND_USDHC_BUS_ROOT_CLK] = imx_clk_gate4("nand_usdhc_root_clk", "nand_usdhc_post_div", base + 0x4130, 0);
clks[IMX7D_AHB_CHANNEL_ROOT_CLK] = imx_clk_gate4("ahb_root_clk", "ahb_post_div", base + 0x4200, 0);
clks[IMX7D_DRAM_ROOT_CLK] = imx_clk_gate4("dram_root_clk", "dram_post_div", base + 0x4130, 0);
clks[IMX7D_DRAM_PHYM_ROOT_CLK] = imx_clk_gate4("dram_phym_root_clk", "dram_phym_cg", base + 0x4130, 0);
--
2.12.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 2/2] ARM: dts: imx7: add USDHC NAND clock to SDHC instances
From: Stefan Agner @ 2017-03-30 0:50 UTC (permalink / raw)
To: shawnguo, kernel, sboyd
Cc: aisheng.dong, fabio.estevam, robh+dt, mark.rutland,
linux-arm-kernel, devicetree, linux-clk, linux-kernel,
Stefan Agner
In-Reply-To: <20170330005029.6472-1-stefan@agner.ch>
The USDHC instances need the USDHC NAND clock in order to operate.
Add the clock as ahb bus clock.
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
arch/arm/boot/dts/imx7s.dtsi | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 5d3a43b8de20..5794febb19a4 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -936,7 +936,7 @@
reg = <0x30b40000 0x10000>;
interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX7D_CLK_DUMMY>,
- <&clks IMX7D_CLK_DUMMY>,
+ <&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>,
<&clks IMX7D_USDHC1_ROOT_CLK>;
clock-names = "ipg", "ahb", "per";
bus-width = <4>;
@@ -948,7 +948,7 @@
reg = <0x30b50000 0x10000>;
interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX7D_CLK_DUMMY>,
- <&clks IMX7D_CLK_DUMMY>,
+ <&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>,
<&clks IMX7D_USDHC2_ROOT_CLK>;
clock-names = "ipg", "ahb", "per";
bus-width = <4>;
@@ -960,7 +960,7 @@
reg = <0x30b60000 0x10000>;
interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX7D_CLK_DUMMY>,
- <&clks IMX7D_CLK_DUMMY>,
+ <&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>,
<&clks IMX7D_USDHC3_ROOT_CLK>;
clock-names = "ipg", "ahb", "per";
bus-width = <4>;
--
2.12.1
^ permalink raw reply related
* Re: [PATCH v5 6/9] coresight: add support for CPU debug module
From: Leo Yan @ 2017-03-30 1:03 UTC (permalink / raw)
To: Mike Leach
Cc: Mathieu Poirier, Jonathan Corbet, Rob Herring, Mark Rutland,
Wei Xu, Catalin Marinas, Will Deacon, Andy Gross, David Brown,
Michael Turquette, Stephen Boyd, Guodong Xu, John Stultz,
linux-doc, linux-kernel, devicetree, linux-arm-kernel,
linux-arm-msm, linux-soc, linux-clk, Suzuki K. Poulose, Sudeep
In-Reply-To: <CAJ9a7VhQRuVOCJkjPc9RANGo=fgA8wjtB65dC5f19ZX22a5FHQ@mail.gmail.com>
On Wed, Mar 29, 2017 at 03:56:23PM +0100, Mike Leach wrote:
[...]
> >> > + /*
> >> > + * Unfortunately the CPU cannot be powered up, so return
> >> > + * back and later has no permission to access other
> >> > + * registers. For this case, should set 'idle_constraint'
> >> > + * to ensure CPU power domain is enabled!
> >> > + */
> >> > + if (!(drvdata->edprsr & EDPRSR_PU)) {
> >> > + pr_err("%s: power up request for CPU%d failed\n",
> >> > + __func__, drvdata->cpu);
> >> > + goto out;
> >> > + }
> >> > +
> >> > +out_powered_up:
> >> > + debug_os_unlock(drvdata);
> >> > +
> >> > + /*
> >> > + * At this point the CPU is powered up, so set the no powerdown
> >> > + * request bit so we don't lose power and emulate power down.
> >> > + */
> >> > + drvdata->edprsr = readl(drvdata->base + EDPRCR);
> >> > + drvdata->edprsr |= EDPRCR_COREPURQ | EDPRCR_CORENPDRQ;
> >>
> >> If we are here the core is already up. Shouldn't we need to set
> >> EDPRCR_CORENPDRQ only?
> >
> > Yeah. Will fix.
>
> No - EDPRCR_COREPURQ and EDPRCR_CORENPDRQ have different semantics and purposes
>
> EDPRCR_COREPURQ is in the debug power domain an is tied to an external
> debug request that should be an input to the external (to the PE)
> system power controller.
> The requirement is that the system power controller powers up the core
> domain and does not power it down while it remains asserted.
>
> EDPRCR_CORENPDRQ is in the core power domain and thus to the specific
> core only. This ensures that any power control software running on
> that core should emulate a power down if this is set to one.
I'm curious the exact meaning for "power control software".
Does it mean EDPRCR_CORENPDRQ should be checked by kernel or PSCI
liked code in ARM trusted firmware to avoid to run CPU power off flow?
Or will EDPRCR_CORENPDRQ assert CPU external signal to notify power
controller so power controller emulate a power down?
> We cannot know the power control design of the system, so the safe
> solution is to set both bits.
Thanks a lot for the suggestion. Will set both bits.
Thanks,
Leo Yan
^ permalink raw reply
* Re: [PATCH v5 6/9] coresight: add support for CPU debug module
From: Leo Yan @ 2017-03-30 1:18 UTC (permalink / raw)
To: Mike Leach
Cc: Suzuki K Poulose, Jonathan Corbet, Rob Herring, Mark Rutland,
Wei Xu, Catalin Marinas, Will Deacon, Andy Gross, David Brown,
Michael Turquette, Stephen Boyd, Mathieu Poirier, Guodong Xu,
John Stultz, linux-doc, linux-kernel, devicetree,
linux-arm-kernel, linux-arm-msm, linux-soc, linux-clk,
Sudeep Holla <sudee>
In-Reply-To: <CAJ9a7VgsnFYpOGVjD4m7+EjFXkBoRqVR2F2Pgtn-T0NB+YP6tg@mail.gmail.com>
On Wed, Mar 29, 2017 at 04:17:19PM +0100, Mike Leach wrote:
[...]
> >> >+out_powered_up:
> >> >+ debug_os_unlock(drvdata);
> >>
> >> Question: Do we need a matching debug_os_lock() once we are done ?
> >
> > I have checked ARM ARMv8, but there have no detailed description for
> > this. I refered coresight-etmv4 code and Mike's pseudo code, ther have
> > no debug_os_lock() related operations.
> >
> > Mike, Mathieu, could you also help confirm this?
> >
>
> Debug OS lock / unlock allows the power management code running on the
> core to lock out the external debugger while the debug registers are
> saved/restored during a core power event.
>
> e.g. A sequence such as this might occur in a correctly programmed system....
>
> debug_os_lock()
> save_debug_regs() // visible from core power domain - incl breakpoints etc
> save_etm_regs()
> ... // other stuff prior to core power down,
> <power_down_core>
>
> Followed by...
>
> <power_up_core>
> restore_etm_regs()
> restore_debug_regs() // visible from core power domain - incl breakpoints etc
> debug_os_unlock()
>
> The value is 1 (locked) if cold resetting into AArch64 - it is
> expected that some system software will set this to 0 as part of the
> boot process.
> The lock prevents write access to the external debug registers so we
> need to clear it to set up the external debug registers we are using.
This description is conflict with upper restoring flows. During
restore_debug_regs(), the os lock is locked so how it can write
external debug register to restore context?
> This suggests that it should be restored as we found it when done.
Thanks,
Leo Yan
^ permalink raw reply
* Re: [PATCH v5 6/9] coresight: add support for CPU debug module
From: Leo Yan @ 2017-03-30 1:59 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Jonathan Corbet, Rob Herring, Mark Rutland, Wei Xu,
Catalin Marinas, Will Deacon, Andy Gross, David Brown,
Michael Turquette, Stephen Boyd, Guodong Xu, John Stultz,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-soc-u79uwXL29TY76Z2rM5mHXA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
mike.leach-QSEj5FYQhm4dnm+yROfE0A, Suzuki.Poulose-5wv7dgnIgG8,
sudeep.holla-5wv7dgnIgG8
In-Reply-To: <20170329165535.GB24889-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Wed, Mar 29, 2017 at 10:55:35AM -0600, Mathieu Poirier wrote:
[...]
> > So this is why add "idle_constraint" as a central place to control
> > power domain for CPU debug purpose and I also think this is more
> > friendly for hardware design, e.g. some platforms can enable partial
> > low power states to save power and avoid overheat after using this
> > driver.
> >
> > How about you think for this?
>
> Like Sudeep pointed out we should concentrate on doing the right thing,
> that is work with EDPRSR.PU, EDPRCR.COREPURQ and EDPRCR.CORENPDRQ.
Agree, and I think we have aligned for this.
> Anything outside of that becomes platform specific and can't be handled in
> this driver.
Sorry I argue a bit for this just want to make things more clear and
if can have better method.
Though the issue is platform specific, but the code is to seek common
method to handle them. So the driver has no any platform specific code.
I read again for Suziki's suggestion: "4) Should document the fact that,
on some platforms, the user may have to disable CPUidle explicitly to
get the driver working. But let us not make it the default. The user
with a not so ideal platform could add "nohlt" and get it working."
So I'm not strong to resist and if this is alignment yet, I should
document well for this but doesn't handle it in driver (keep driver
simple).
Thanks,
Leo Yan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH] of: Fix DMA configuration for non-DT masters
From: Oza Oza @ 2017-03-30 3:21 UTC (permalink / raw)
To: Robin Murphy
Cc: Linux IOMMU, devicetree-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <b02ace9b-f8f3-269d-6db4-3c1b1ea1e816-5wv7dgnIgG8@public.gmane.org>
On Wed, Mar 29, 2017 at 11:12 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
> On 29/03/17 06:46, Oza Oza wrote:
>> On Wed, Mar 29, 2017 at 10:23 AM, Oza Oza <oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>> On Wed, Mar 29, 2017 at 12:27 AM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
>>>> For PCI masters not represented in DT, we pass the OF node of their
>>>> associated host bridge to of_dma_configure(), such that they can inherit
>>>> the appropriate DMA configuration from whatever is described there.
>>>> Unfortunately, whilst this has worked for the "dma-coherent" property,
>>>> it turns out to miss the case where the host bridge node has a non-empty
>>>> "dma-ranges", since nobody is expecting the 'device' to be a bus itself.
>>>>
>>>> It transpires, though, that the de-facto interface since the prototype
>>>> change in 1f5c69aa51f9 ("of: Move of_dma_configure() to device.c to help
>>>> re-use") is very clear-cut: either the master_np argument is redundant
>>>> with dev->of_node, or dev->of_node is NULL and master_np is the relevant
>>>> parent bus. Let's ratify that behaviour, then teach the whole
>>>> of_dma_configure() pipeline to cope with both cases properly.
>>>>
>>>> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
>
> [...]
>
>>>
>>> pci and memory mapped device world is different.
>
> ???
>
> No they aren't. There is nothing magic about PCI. PCI *is* a
> memory-mapped bus.
>
> The only PCI-specific aspect here is the Linux code path which passes a
> host controller node into of_dma_configure() where the latter expects a
> node for the actual endpoint device. It managed to work for
> "dma-coherent", because that may appear either directly on a device node
> or on any of its parent buses, but "dma-ranges" is *only* valid for DT
> nodes representing buses, thus reveals that using a parent bus to stand
> in for a device isn't actually correct. I only posted that horrible hack
> patch to prove the point that having a child node to represent the
> actual device is indeed sufficient to make of_dma_configure() work
> correctly for PCI masters as-is (modulo the other issues).
>
>> of course if you talk
>>> from iommu perspective, all the master are same for IOMMU
>
> I don't understand what you mean by that. There's nothing about IOMMUs
> here, it's purely about parsing DT properties correctly.
>
>>> but the original intention of the patch is to try to solve 2 problems.
>>> please refer to https://lkml.org/lkml/2017/3/29/10
>
> One patch should not solve two separate problems anyway. Taking a step
> back, there are at least 3 things that this discussion has brought up:
>
> 1) The way in which we call of_dma_configure() for PCI devices causes
> the "dma-ranges" property on PCI host controllers to be ignored.
>
> 2) of_dma_get_range() only considers the first entry in any "dma-ranges"
> property.
>
> 3) When of_dma_configure() does set a DMA mask, there is nothing on
> arm64 and other architectures to prevent drivers overriding that with a
> wider mask later.
>
> Those are 3 separate problems, to solve with 3 or more separate patches,
> and I have deliberately put the second and third to one side for the
> moment. This patch fixes problem 1.
>
>>> 1) expose generic API for pci world clients to configure their
>>> dma-ranges. right now there is none.
>>> 2) same API can be used by IOMMU to derive dma_mask.
>>>
>>> while the current patch posted to handle dma-ranges for both memory
>>> mapped and pci devices, which I think is overdoing.
>
> No, of_dma_configure() handles the "dma-ranges" property as it is
> defined in the DT spec to describe the mapping between a child bus
> address space and a parent bus address space. Whether those
> memory-mapped parent and child buses are PCI, ISA, AMBA, HyperTransport,
> or anything else is irrelevant other than for the encoding of the
> address specifiers. All this patch does is sort out the cases where we
> have a real device node to start at, from the cases where we don't and
> so start directly at the device's parent bus node instead.
>
>>> besides we have different configuration of dma-ranges based on iommu
>>> is enabled or not enabled.
>
> That doesn't sound right, unless you mean the firmware actually programs
> the host controller's AXI bridge differently for system configurations
> where the IOMMU is expected to be used or not? (and even then, I don't
> really see why it would be necessary to do that...)
>
>>> #define PCIE_DMA_RANGES dma-ranges = <0x43000000 0x00 0x80000000 0x00
>>> 0x80000000 0x00 0x80000000 \
>>> 0x43000000 0x08 0x00000000 0x08
>>> 0x00000000 0x08 0x00000000 \
>>> 0x43000000 0x80 0x00000000 0x80
>>> 0x00000000 0x40 0x00000000>;
>>> Not sure if this patch will consider above dma-ranges.
>>>
>>> my suggestion is to leave existing of_dma_get_range as is, and have
>>> new function for pci world as discussed in
>>> please refer to https://lkml.org/lkml/2017/3/29/10
>
> And then we keep adding new functions to do the exact same thing for
> every other discoverable bus type whose bridge is be described in DT? I
> fail to see how that is in any way better than simply fixing the
> existing code to work as it was intended to.
>
> of_dma_get_ranges() uses of_translate_dma_address() just the same way as
> existing PowerPC PCI code does, which further disproves your assertion
> that parsing PCI addresses is somehow special - it's really only a
> matter of getting the right number of address cells in order to to read
> a child address to give to of_translate_dma_address() in the first
> place. Incidentally, I now notice that the proposed
> of_pci_get_dma_ranges() is incomplete as it doesn't use
> of_translate_dma_address() or otherwise traverse upwards through the
> dma-ranges of any further parent buses.
>
>>>
>>> Regards,
>>> Oza.
>>
>> also I see that, in case of multiple ranges of_dma_get_range doesnt handle that.
>> and also it is not meant to handle.
>
> Yes, the existing code doesn't handle multiple dma-ranges entries,
> because nobody's had the need to implement that so far, and this patch
> does not change that because it's fixing a separate problem.
>
> Now, of course of_dma_get_range() *should* be capable of handling
> multiple entries regardless of this patch, and I'm going to write *that*
> patch right now (it's going to be a case of adding a handful of lines
> which probably won't even conflict with this one at all). If we had a
> bunch of different range parsing functions, we'd then have to duplicate
> the equivalent logic across all of them, which is clearly undesirable
> when it can easily be avoided altogether.
>
> Robin.
>
>> so with this patch will return wrong size and hence wrong dma_mask.
>> having said, I think it is better to separate pci world dma-ranges
>> function on of_pci.c
>>
>> for which my discussion with Rob already, (same thread)
>> https://lkml.org/lkml/2017/3/29/10
>> Waiting for Rob's viewpoint on this.
>>
>>
>> Regards,
>> Oza.
>>
>
In my opinion NAKing to this, because
1) some reasons already mentioned in https://lkml.org/lkml/2017/3/29/10
2) also of_dma_get_range is supposed to handle traditional dma-ranges
(not pci one)
3) we need separate function for pci users such as iproc or rcar SOCs
to have their dma-ranges parsed,
which can be extended later for e.g. pci flags have some meanings.
besides of_dma_configure is written to pass only single out parameter
(..., *dma_addr, *size)
handling multiple ranges here, and passing only one range out is not desirable.
also you would not like to handle pci flags (the first portion of DT
entry) in this function if required in future.
this new function will be similar to of_pci_get_host_bridge_resources
https://lkml.org/lkml/2017/3/29/10
which I am writing/improving right now..
Regards,
Oza.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH 1/3] of/pci: dma-ranges to account highest possible host bridge dma_mask
From: Oza Oza via iommu @ 2017-03-30 3:26 UTC (permalink / raw)
To: Robin Murphy
Cc: Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux IOMMU,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <CAMSpPPd7awDB_JL0_4pY9qiSXeLt3QzpSL95XogyE9h0kgyq_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Mar 29, 2017 at 10:13 AM, Oza Oza <oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> On Tue, Mar 28, 2017 at 7:59 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
>> On 28/03/17 06:27, Oza Oza wrote:
>>> On Mon, Mar 27, 2017 at 8:16 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>>> On Sat, Mar 25, 2017 at 12:31 AM, Oza Pawandeep <oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>>>> it is possible that PCI device supports 64-bit DMA addressing,
>>>>> and thus it's driver sets device's dma_mask to DMA_BIT_MASK(64),
>>>>> however PCI host bridge may have limitations on the inbound
>>>>> transaction addressing. As an example, consider NVME SSD device
>>>>> connected to iproc-PCIe controller.
>>>>>
>>>>> Currently, the IOMMU DMA ops only considers PCI device dma_mask
>>>>> when allocating an IOVA. This is particularly problematic on
>>>>> ARM/ARM64 SOCs where the IOMMU (i.e. SMMU) translates IOVA to
>>>>> PA for in-bound transactions only after PCI Host has forwarded
>>>>> these transactions on SOC IO bus. This means on such ARM/ARM64
>>>>> SOCs the IOVA of in-bound transactions has to honor the addressing
>>>>> restrictions of the PCI Host.
>>>>>
>>>>> current pcie frmework and of framework integration assumes dma-ranges
>>>>> in a way where memory-mapped devices define their dma-ranges.
>>>>> dma-ranges: (child-bus-address, parent-bus-address, length).
>>>>>
>>>>> but iproc based SOCs and even Rcar based SOCs has PCI world dma-ranges.
>>>>> dma-ranges = <0x43000000 0x00 0x00 0x00 0x00 0x80 0x00>;
>>>>
>>>> If you implement a common function, then I expect to see other users
>>>> converted to use it. There's also PCI hosts in arch/powerpc that parse
>>>> dma-ranges.
>>>
>>> the common function should be similar to what
>>> of_pci_get_host_bridge_resources is doing right now.
>>> it parses ranges property right now.
>>>
>>> the new function would look look following.
>>>
>>> of_pci_get_dma_ranges(struct device_node *dev, struct list_head *resources)
>>> where resources would return the dma-ranges.
>>>
>>> but right now if you see the patch, of_dma_configure calls the new
>>> function, which actually returns the largest possible size.
>>> so this new function has to be generic in a way where other PCI hosts
>>> can use it. but certainly iproc(Broadcom SOC) , rcar based SOCs can
>>> use it for sure.
>>>
>>> although having powerpc using it; is a separate exercise, since I do
>>> not have any access to other PCI hosts such as powerpc. but we can
>>> workout with them on thsi forum if required.
>>>
>>> so overall, of_pci_get_dma_ranges has to serve following 2 purposes.
>>>
>>> 1) it has to return largest possible size to of_dma_configure to
>>> generate largest possible dma_mask.
>>>
>>> 2) it also has to return resources (dma-ranges) parsed, to the users.
>>>
>>> so to address above needs
>>>
>>> of_pci_get_dma_ranges(struct device_node *dev, struct list_head
>>> *resources, u64 *size)
>>>
>>> dev -> device node.
>>> resources -> dma-ranges in allocated list.
>>> size -> highest possible size to generate possible dma_mask for
>>> of_dma_configure.
>>>
>>> let em know how this sounds.
>>
>> Note that the point of passing PCI host bridges into of_dma_configure()
>> in the first place was to avoid having some separate PCI-specific path
>> for DMA configuration. I worry that introducing bus-specific dma-ranges
>> parsing largely defeats that, since we end up with the worst of both
>> worlds; effectively-duplicated code, and/or a load of extra complexity
>> to then attempt to reconverge the divergent paths (there really
>> shouldn't be any need to allocate a list of anything). Given that
>> of_translate_dma_address() is already bus-agnostic, it hardly seems
>> justifiable for its caller not to be so as well, especially when it
>> mostly just comes down to getting the right #address-cells value.
>>
>> The patch below is actually enough to make typical cases work, but is
>> vile, so I'm not seriously considering it (hence I've not bothered
>> making IOMMU configuration handle all circumstances). What it has served
>> to do, though, is give me a clear idea of how to properly sort out the
>> not-quite-right device/parent assumptions between of_dma_configure() and
>> of_dma_get_range() rather than bodging around them any further - stay tuned.
>>
>> Robin.
>>
>> ----->8-----
>> From: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
>> Subject: [PATCH] of/pci: Use child node for DMA configuration
>>
>> of_dma_configure() expects to be passed an OF node representing the
>> device being configured - for PCI devices we currently pass the node of
>> the appropriate host controller, which sort of works for inherited
>> properties which may appear at any level, like "dma-coherent", but falls
>> apart for properties which actually care about specific device-parent
>> relationships, like "dma-ranges".
>>
>> Solve this by attempting to find a suitable child node if the PCI
>> hierarchy is actually represented in DT, and if not then faking one up
>> as a last resort, to make all of DMA configuration work as expected.
>>
>> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
>> ---
>> drivers/iommu/of_iommu.c | 3 ++-
>> drivers/pci/of.c | 24 ++++++++++++++++++++++++
>> drivers/pci/probe.c | 14 +++++++++++++-
>> include/linux/pci.h | 3 +++
>> 4 files changed, 42 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>> index 2683e9fc0dcf..35c97b945c15 100644
>> --- a/drivers/iommu/of_iommu.c
>> +++ b/drivers/iommu/of_iommu.c
>> @@ -146,7 +146,8 @@ const struct iommu_ops *of_iommu_configure(struct
>> device *dev,
>> int idx = 0;
>>
>> if (dev_is_pci(dev))
>> - return of_pci_iommu_configure(to_pci_dev(dev), master_np);
>> + return of_pci_iommu_configure(to_pci_dev(dev),
>> + of_get_next_parent(master_np));
>>
>> /*
>> * We don't currently walk up the tree looking for a parent IOMMU.
>> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
>> index e112da11630e..96eece1c423d 100644
>> --- a/drivers/pci/of.c
>> +++ b/drivers/pci/of.c
>> @@ -88,3 +88,27 @@ struct irq_domain
>> *pci_host_bridge_of_msi_domain(struct pci_bus *bus)
>> return NULL;
>> #endif
>> }
>> +
>> +struct device_node *pci_dev_get_dma_of_node(struct pci_dev *dev)
>> +{
>> + struct device_node *np;
>> + struct pci_bus *bus = dev->bus;
>> +
>> + /* Is the device itself actually described in the DT? */
>> + np = of_node_get(dev->dev.of_node);
>> + if (np)
>> + return np;
>> +
>> + /* Or is some intermediate bridge? That would do... */
>> + for (bus = dev->bus; bus->parent; bus = bus->parent) {
>> + np = of_node_get(bus->bridge->of_node);
>> + if (np)
>> + return np;
>> + }
>> +
>> + /* Failing that, any child of the same host bridge? */
>> + if (bus->bridge->parent)
>> + np = of_get_next_child(bus->bridge->parent->of_node, NULL);
>> +
>> + return np;
>> +}
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index dfc9a2794141..4f7ade64aa3e 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1906,7 +1906,19 @@ static void pci_dma_configure(struct pci_dev *dev)
>>
>> if (IS_ENABLED(CONFIG_OF) &&
>> bridge->parent && bridge->parent->of_node) {
>> - of_dma_configure(&dev->dev, bridge->parent->of_node);
>> + struct device_node *np, tmp = {0};
>> +
>> + np = pci_dev_get_dma_of_node(dev);
>> + if (!np) {
>> + np = &tmp;
>> + of_node_set_flag(np, OF_DETACHED);
>> + of_node_init(np);
>> + tmp.parent = bridge->parent->of_node;
>> + }
>> +
>> + of_dma_configure(&dev->dev, np);
>> + if (np != &tmp)
>> + of_node_put(np);
>> } else if (has_acpi_companion(bridge)) {
>> struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
>> enum dev_dma_attr attr = acpi_get_dma_attr(adev);
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index eb3da1a04e6c..94ecd1817f58 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -2086,6 +2086,7 @@ void pci_release_of_node(struct pci_dev *dev);
>> void pci_set_bus_of_node(struct pci_bus *bus);
>> void pci_release_bus_of_node(struct pci_bus *bus);
>> struct irq_domain *pci_host_bridge_of_msi_domain(struct pci_bus *bus);
>> +struct device_node *pci_dev_get_dma_of_node(struct pci_dev *dev);
>>
>> /* Arch may override this (weak) */
>> struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus);
>> @@ -2110,6 +2111,8 @@ static inline struct device_node *
>> pci_device_to_OF_node(const struct pci_dev *pdev) { return NULL; }
>> static inline struct irq_domain *
>> pci_host_bridge_of_msi_domain(struct pci_bus *bus) { return NULL; }
>> +static inline struct device_node *
>> +pci_dev_get_dma_of_node(struct pci_dev *dev) { return NULL; }
>> #endif /* CONFIG_OF */
>>
>> #ifdef CONFIG_ACPI
>>
>
> pci and memory mapped device world is different. of course if you talk
> from iommu perspective, all the master are same for IOMMU
> but the original intention of the patch is to try to solve 2 problems.
>
> 1) expose generic API for pci world clients to configure their
> dma-ranges. right now there is none.
> 2) same API can be used by IOMMU to derive dma_mask.
>
> the implementation tries to handle dma-ranges for both memory mapped
> and pci devices, which I think is overkill.
> besides we have different configuration of dma-ranges based on iommu
> is enabled or not enabled.
>
> #define PCIE_DMA_RANGES dma-ranges = <0x43000000 0x00 0x80000000 0x00
> 0x80000000 0x00 0x80000000 \
> 0x43000000 0x08 0x00000000 0x08
> 0x00000000 0x08 0x00000000 \
> 0x43000000 0x80 0x00000000 0x80
> 0x00000000 0x40 0x00000000>;
>
>
> the of_dma_get_range is written with respect to traditional memory
> ranges, they were not meant for pci dma-ranges.
>
> Regards,
> Oza.
Rob, gentle reminder on your viewpoint. my take on whole thing is as follows:
1) of_dma_get_range is supposed to handle traditional dma-ranges
(not pci one)
2) we need separate function for pci users such as iproc or rcar SOCs
to have their dma-ranges parsed,
which can be extended later for e.g. pci flags have some meanings.
besides of_dma_configure is written to pass only single out parameter
(..., *dma_addr, *size)
handling multiple ranges here, and passing only one range out is not desirable.
also you would not like to handle pci flags in of_dma_get_range (the
first portion of DT
entry) in this function if required in future.
this new function will be similar to of_pci_get_host_bridge_resources
which I am writing/improving right now..
Regards,
Oza.
^ permalink raw reply
* [PATCH v6] hwmon: ads7828 optional parameters from the device tree
From: Sam Povilus @ 2017-03-30 3:46 UTC (permalink / raw)
To: wsa, jdelvare, linux, linux-hwmon, linux-i2c, devicetree, robh+dt,
mark.rutland
Cc: Sam Povilus
Adding the ability for the ads7828 and ads7830 to use the device tree to
get their optional parameters, instead of using platform devices. This
allows people using custom boards to also use the ads7828 in a non-default
manner.
v2: conforming to coding style
v3: changing from "_" to "-" for device tree entries
v4: using regulator subsystem for voltage source, renaming differential
input property
v5: checking for invalid vref values sooner and returning error if they are
bad, not warning user.
v6: optimization per Guenter Roeck
Signed-off-by: Sam Povilus <kernel.development@povil.us>
---
.../devicetree/bindings/hwmon/ads7828.txt | 26 ++++++++++++++++++++++
drivers/hwmon/ads7828.c | 15 +++++++++++++
2 files changed, 41 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/ads7828.txt
diff --git a/Documentation/devicetree/bindings/hwmon/ads7828.txt b/Documentation/devicetree/bindings/hwmon/ads7828.txt
new file mode 100644
index 000000000000..a1251b19079d
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ads7828.txt
@@ -0,0 +1,26 @@
+ads7828 properties
+
+Required properties:
+- compatible: Should be one of
+ ti,ads7828
+ ti,ads7830
+- reg: I2C address
+
+Optional properties:
+
+- ti,differential-input
+ Set to use the device in differential mode.
+- vref
+ The external reference on the device is set to this regulators output. If it
+ does not exists the internal reference will be used and output by the ads78xx
+ on the "external vref" pin.
+
+ Example ADS7828 node:
+
+ ads7828: ads@48 {
+ comatible = "ti,ads7828";
+ reg = <0x48>;
+ vref-supply = <&vref>;
+ ti,differential-input;
+ };
+
diff --git a/drivers/hwmon/ads7828.c b/drivers/hwmon/ads7828.c
index ee396ff167d9..88330a233b1d 100644
--- a/drivers/hwmon/ads7828.c
+++ b/drivers/hwmon/ads7828.c
@@ -34,6 +34,7 @@
#include <linux/platform_data/ads7828.h>
#include <linux/regmap.h>
#include <linux/slab.h>
+#include <linux/regulator/consumer.h>
/* The ADS7828 registers */
#define ADS7828_CMD_SD_SE 0x80 /* Single ended inputs */
@@ -118,9 +119,11 @@ static int ads7828_probe(struct i2c_client *client,
struct ads7828_data *data;
struct device *hwmon_dev;
unsigned int vref_mv = ADS7828_INT_VREF_MV;
+ unsigned int vref_uv;
bool diff_input = false;
bool ext_vref = false;
unsigned int regval;
+ struct regulator *reg;
data = devm_kzalloc(dev, sizeof(struct ads7828_data), GFP_KERNEL);
if (!data)
@@ -131,6 +134,18 @@ static int ads7828_probe(struct i2c_client *client,
ext_vref = pdata->ext_vref;
if (ext_vref && pdata->vref_mv)
vref_mv = pdata->vref_mv;
+ } else if (dev->of_node) {
+ diff_input = of_property_read_bool(dev->of_node,
+ "ti,differential-input");
+ reg = devm_regulator_get_optional(dev, "vref");
+ if (!IS_ERR(reg)) {
+ vref_uv = regulator_get_voltage(reg);
+ vref_mv = DIV_ROUND_CLOSEST(vref_uv, 1000);
+ if (vref_mv < ADS7828_EXT_VREF_MV_MIN ||
+ vref_mv > ADS7828_EXT_VREF_MV_MAX)
+ return -EINVAL;
+ ext_vref = true;
+ }
}
/* Bound Vref with min/max values */
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v6 0/4] Broadcom SBA RAID support
From: Vinod Koul @ 2017-03-30 4:49 UTC (permalink / raw)
To: Jassi Brar
Cc: Anup Patel, Rob Herring, Mark Rutland, Herbert Xu,
David S . Miller, Dan Williams, Ray Jui, Scott Branden, Jon Mason,
Rob Rice, BCM Kernel Feedback, dmaengine, Device Tree,
Linux ARM Kernel, Linux Kernel, linux-crypto, linux-raid
In-Reply-To: <CAALAos-P6CDdWTJxg8YQfT1V+iCxUcbYHiQ35EdZzz9PnN3cuA@mail.gmail.com>
On Wed, Mar 29, 2017 at 11:35:43AM +0530, Anup Patel wrote:
> On Tue, Mar 21, 2017 at 2:48 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> > On Tue, Mar 21, 2017 at 02:17:21PM +0530, Anup Patel wrote:
> >> On Tue, Mar 21, 2017 at 2:00 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> >> > On Mon, Mar 06, 2017 at 03:13:24PM +0530, Anup Patel wrote:
> >> >> The Broadcom SBA RAID is a stream-based device which provides
> >> >> RAID5/6 offload.
> >> >>
> >> >> It requires a SoC specific ring manager (such as Broadcom FlexRM
> >> >> ring manager) to provide ring-based programming interface. Due to
> >> >> this, the Broadcom SBA RAID driver (mailbox client) implements
> >> >> DMA device having one DMA channel using a set of mailbox channels
> >> >> provided by Broadcom SoC specific ring manager driver (mailbox
> >> >> controller).
> >> >>
> >> >> The Broadcom SBA RAID hardware requires PQ disk position instead
> >> >> of PQ disk coefficient. To address this, we have added raid_gflog
> >> >> table which will help driver to convert PQ disk coefficient to PQ
> >> >> disk position.
> >> >>
> >> >> This patchset is based on Linux-4.11-rc1 and depends on patchset
> >> >> "[PATCH v5 0/2] Broadcom FlexRM ring manager support"
> >> >
> >> > Okay I applied and was about to push when I noticed this :(
> >> >
> >> > So what is the status of this..?
> >>
> >> PATCH2 is Acked but PATCH1 is under-review. Currently, its
> >> v6 of that patchset.
> >>
> >> The only dependency on that patchset is the changes in
> >> brcm-message.h which are required by this BCM-SBA-RAID
> >> driver.
> >>
> >> @Jassi,
> >> Can you please have a look at PATCH v6?
> >
> > And I would need an immutable branch/tag once merged. I am going to keep
> > this series pending till then.
>
> The Broadcom FlexRM patchset is pickedup by Jassi and
> can be found in mailbox-for-next branch of
> git://git.linaro.org/landing-teams/working/fujitsu/integration
>
> Both patchset (Broadcom FlexRM patchset and this one) are
> also available in sba-raid-v7 branch of
> https://github.com/Broadcom/arm64-linux.git
Jassi,
Can you provide an immutable branch/tag please for this, latter is
preferred.
Btw didn't find your tree in MAINTAINERS..
>
> Regards,
> Anup
--
~Vinod
^ permalink raw reply
* Re: [PATCH V11 6/6] thermal: bcm2835: add thermal driver for bcm2835 SoC
From: Eduardo Valentin @ 2017-03-30 4:57 UTC (permalink / raw)
To: Stefan Wahren
Cc: Zhang Rui, kernel, Eric Anholt, Rob Herring, Frank Rowand,
Florian Fainelli, linux-rpi-kernel, devicetree, linux-pm
In-Reply-To: <20170329045807.GC2582@localhost.localdomain>
On Tue, Mar 28, 2017 at 09:58:09PM -0700, Eduardo Valentin wrote:
> On Sun, Mar 12, 2017 at 10:11:05PM +0000, Stefan Wahren wrote:
> > Add basic thermal driver for bcm2835 SoC.
> >
> > This driver currently make sure that tsense HW block is set up
> > correctly.
> >
> > Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > Acked-by: Eric Anholt <eric@anholt.net>
>
> I see no issue with this driver at this point.
>
> As I mentioned, the full series needs to go together. You either get an
> Ack from OF maintainers, and I take the series, or you can add my
>
> Acked-by: Eduardo Valentin <edubezval@gmail.com>
>
> after fixing the small comment from Nobuhiro.
>
Based on other drivers that require negative values, I assume patches 1
and 2 of these series are needed, but more for enforcing the API, given
that the current of-thermal driver still retrieves the negative values
from DTB [1].
A third option on top of what I suggested above is to split the series.
Given that you could still get the proper coefficients
with current of-thermal, I would suggest you to split this series into
two (patches 1-2, 3-6) in your next version, so we avoid futher delays
on the driver, while patches 1-2 waits for acks from OF side.
[1] - https://patchwork.kernel.org/patch/9612613/
^ permalink raw reply
* Re: [PATCH v4 19/23] drivers/fsi: Add GPIO based FSI master
From: Joel Stanley @ 2017-03-30 5:48 UTC (permalink / raw)
To: Christopher Bostic
Cc: Mark Rutland, devicetree, Benjamin Herrenschmidt, Andrew Jeffery,
Greg KH, Russell King, rostedt, Linux Kernel Mailing List,
Rob Herring, Jeremy Kerr, Edward A . James, Alistair Popple,
mingo, linux-arm-kernel
In-Reply-To: <20170329174340.89109-20-cbostic@linux.vnet.ibm.com>
On Thu, Mar 30, 2017 at 4:13 AM, Christopher Bostic
<cbostic@linux.vnet.ibm.com> wrote:
> From: Chris Bostic <cbostic@linux.vnet.ibm.com>
>
> Implement a FSI master using GPIO. Will generate FSI protocol for
> read and write commands to particular addresses. Sends master command
> and waits for and decodes a slave response.
>
> Includes changes from Edward A. James <eajames@us.ibm.com> and Jeremy
> Kerr <jk@ozlabs.org>.
>
> Signed-off-by: Edward A. James <eajames@us.ibm.com>
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
> Signed-off-by: Chris Bostic <cbostic@linux.vnet.ibm.com>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts | 10 +
> arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts | 12 +
> drivers/fsi/Kconfig | 11 +
> drivers/fsi/Makefile | 1 +
> drivers/fsi/fsi-master-gpio.c | 614 ++++++++++++++++++++++++++
> 5 files changed, 648 insertions(+)
> create mode 100644 drivers/fsi/fsi-master-gpio.c
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
> index 1d2fc1e..cf7d7d7 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts
> @@ -29,6 +29,16 @@
> reg = <0x5f000000 0x01000000>; /* 16M */
> };
> };
> +
> + gpio-fsi {
> + compatible = "fsi-master-gpio", "fsi-master";
> +
> + clock-gpios = <&gpio ASPEED_GPIO(A, 4) GPIO_ACTIVE_HIGH>;
> + data-gpios = <&gpio ASPEED_GPIO(A, 5) GPIO_ACTIVE_HIGH>;
> + mux-gpios = <&gpio ASPEED_GPIO(A, 6) GPIO_ACTIVE_HIGH>;
> + enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
> + trans-gpios = <&gpio ASPEED_GPIO(H, 6) GPIO_ACTIVE_HIGH>;
> + };
> };
>
> &uart5 {
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> index 7a3b2b5..2fd7db7 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts
> @@ -29,6 +29,18 @@
> reg = <0xbf000000 0x01000000>; /* 16M */
> };
> };
> +
> + gpio-fsi {
> + compatible = "fsi-master-gpio", "fsi-master";
> +
> + status = "okay";
> +
> + clock-gpios = <&gpio ASPEED_GPIO(AA, 0) GPIO_ACTIVE_HIGH>;
> + data-gpios = <&gpio ASPEED_GPIO(AA, 2) GPIO_ACTIVE_HIGH>;
> + mux-gpios = <&gpio ASPEED_GPIO(A, 6) GPIO_ACTIVE_HIGH>;
> + enable-gpios = <&gpio ASPEED_GPIO(D, 0) GPIO_ACTIVE_HIGH>;
> + trans-gpios = <&gpio ASPEED_GPIO(R, 2) GPIO_ACTIVE_HIGH>;
> + };
> };
I'm not sure what happened here. The changes to the device trees
should not be in this patch.
Cheers,
Joel
>
> &uart5 {
> diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
> index 04c1a0e..9cf8345 100644
> --- a/drivers/fsi/Kconfig
> +++ b/drivers/fsi/Kconfig
> @@ -9,4 +9,15 @@ config FSI
> ---help---
> FSI - the FRU Support Interface - is a simple bus for low-level
> access to POWER-based hardware.
> +
> +if FSI
> +
> +config FSI_MASTER_GPIO
> + tristate "GPIO-based FSI master"
> + depends on FSI && GPIOLIB
> + ---help---
> + This option enables a FSI master driver using GPIO lines.
> +
> +endif
> +
> endmenu
> diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
> index db0e5e7..ed28ac0 100644
> --- a/drivers/fsi/Makefile
> +++ b/drivers/fsi/Makefile
> @@ -1,2 +1,3 @@
>
> obj-$(CONFIG_FSI) += fsi-core.o
> +obj-$(CONFIG_FSI_MASTER_GPIO) += fsi-master-gpio.o
> diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
> new file mode 100644
> index 0000000..0bf5caa
> --- /dev/null
> +++ b/drivers/fsi/fsi-master-gpio.c
> @@ -0,0 +1,614 @@
> +/*
> + * A FSI master controller, using a simple GPIO bit-banging interface
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/fsi.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +#include "fsi-master.h"
> +
> +#define FSI_GPIO_STD_DLY 1 /* Standard pin delay in nS */
> +#define FSI_ECHO_DELAY_CLOCKS 16 /* Number clocks for echo delay */
> +#define FSI_PRE_BREAK_CLOCKS 50 /* Number clocks to prep for break */
> +#define FSI_BREAK_CLOCKS 256 /* Number of clocks to issue break */
> +#define FSI_POST_BREAK_CLOCKS 16000 /* Number clocks to set up cfam */
> +#define FSI_INIT_CLOCKS 5000 /* Clock out any old data */
> +#define FSI_GPIO_STD_DELAY 10 /* Standard GPIO delay in nS */
> + /* todo: adjust down as low as */
> + /* possible or eliminate */
> +#define FSI_GPIO_CMD_DPOLL 0x2
> +#define FSI_GPIO_CMD_TERM 0x3f
> +#define FSI_GPIO_CMD_ABS_AR 0x4
> +
> +#define FSI_GPIO_DPOLL_CLOCKS 100 /* < 21 will cause slave to hang */
> +
> +/* Bus errors */
> +#define FSI_GPIO_ERR_BUSY 1 /* Slave stuck in busy state */
> +#define FSI_GPIO_RESP_ERRA 2 /* Any (misc) Error */
> +#define FSI_GPIO_RESP_ERRC 3 /* Slave reports master CRC error */
> +#define FSI_GPIO_MTOE 4 /* Master time out error */
> +#define FSI_GPIO_CRC_INVAL 5 /* Master reports slave CRC error */
> +
> +/* Normal slave responses */
> +#define FSI_GPIO_RESP_BUSY 1
> +#define FSI_GPIO_RESP_ACK 0
> +#define FSI_GPIO_RESP_ACKD 4
> +
> +#define FSI_GPIO_MAX_BUSY 100
> +#define FSI_GPIO_MTOE_COUNT 1000
> +#define FSI_GPIO_DRAIN_BITS 20
> +#define FSI_GPIO_CRC_SIZE 4
> +#define FSI_GPIO_MSG_ID_SIZE 2
> +#define FSI_GPIO_MSG_RESPID_SIZE 2
> +#define FSI_GPIO_PRIME_SLAVE_CLOCKS 100
> +
> +static DEFINE_SPINLOCK(fsi_gpio_cmd_lock); /* lock around fsi commands */
> +
> +struct fsi_master_gpio {
> + struct fsi_master master;
> + struct device *dev;
> + struct gpio_desc *gpio_clk;
> + struct gpio_desc *gpio_data;
> + struct gpio_desc *gpio_trans; /* Voltage translator */
> + struct gpio_desc *gpio_enable; /* FSI enable */
> + struct gpio_desc *gpio_mux; /* Mux control */
> +};
> +
> +#define to_fsi_master_gpio(m) container_of(m, struct fsi_master_gpio, master)
> +
> +struct fsi_gpio_msg {
> + uint64_t msg;
> + uint8_t bits;
> +};
> +
> +static void clock_toggle(struct fsi_master_gpio *master, int count)
> +{
> + int i;
> +
> + for (i = 0; i < count; i++) {
> + ndelay(FSI_GPIO_STD_DLY);
> + gpiod_set_value(master->gpio_clk, 0);
> + ndelay(FSI_GPIO_STD_DLY);
> + gpiod_set_value(master->gpio_clk, 1);
> + }
> +}
> +
> +static int sda_in(struct fsi_master_gpio *master)
> +{
> + int in;
> +
> + ndelay(FSI_GPIO_STD_DLY);
> + in = gpiod_get_value(master->gpio_data);
> + return in ? 1 : 0;
> +}
> +
> +static void sda_out(struct fsi_master_gpio *master, int value)
> +{
> + gpiod_set_value(master->gpio_data, value);
> +}
> +
> +static void set_sda_input(struct fsi_master_gpio *master)
> +{
> + gpiod_direction_input(master->gpio_data);
> + if (master->gpio_trans)
> + gpiod_set_value(master->gpio_trans, 0);
> +}
> +
> +static void set_sda_output(struct fsi_master_gpio *master, int value)
> +{
> + if (master->gpio_trans)
> + gpiod_set_value(master->gpio_trans, 1);
> + gpiod_direction_output(master->gpio_data, value);
> +}
> +
> +static void clock_zeros(struct fsi_master_gpio *master, int count)
> +{
> + set_sda_output(master, 1);
> + clock_toggle(master, count);
> +}
> +
> +static void serial_in(struct fsi_master_gpio *master, struct fsi_gpio_msg *msg,
> + uint8_t num_bits)
> +{
> + uint8_t bit, in_bit;
> +
> + set_sda_input(master);
> +
> + for (bit = 0; bit < num_bits; bit++) {
> + clock_toggle(master, 1);
> + in_bit = sda_in(master);
> + msg->msg <<= 1;
> + msg->msg |= ~in_bit & 0x1; /* Data is negative active */
> + }
> + msg->bits += num_bits;
> +}
> +
> +static void serial_out(struct fsi_master_gpio *master,
> + const struct fsi_gpio_msg *cmd)
> +{
> + uint8_t bit;
> + uint64_t msg = ~cmd->msg; /* Data is negative active */
> + uint64_t sda_mask = 0x1ULL << (cmd->bits - 1);
> + uint64_t last_bit = ~0;
> + int next_bit;
> +
> + if (!cmd->bits) {
> + dev_warn(master->dev, "trying to output 0 bits\n");
> + return;
> + }
> + set_sda_output(master, 0);
> +
> + /* Send the start bit */
> + sda_out(master, 0);
> + clock_toggle(master, 1);
> +
> + /* Send the message */
> + for (bit = 0; bit < cmd->bits; bit++) {
> + next_bit = (msg & sda_mask) >> (cmd->bits - 1);
> + if (last_bit ^ next_bit) {
> + sda_out(master, next_bit);
> + last_bit = next_bit;
> + }
> + clock_toggle(master, 1);
> + msg <<= 1;
> + }
> +}
> +
> +static void msg_push_bits(struct fsi_gpio_msg *msg, uint64_t data, int bits)
> +{
> + msg->msg <<= bits;
> + msg->msg |= data & ((1ull << bits) - 1);
> + msg->bits += bits;
> +}
> +
> +static void msg_push_crc(struct fsi_gpio_msg *msg)
> +{
> + uint8_t crc;
> + int top;
> +
> + top = msg->bits & 0x3;
> +
> + /* start bit, and any non-aligned top bits */
> + crc = fsi_crc4(0,
> + 1 << top | msg->msg >> (msg->bits - top),
> + top + 1);
> +
> + /* aligned bits */
> + crc = fsi_crc4(crc, msg->msg, msg->bits - top);
> +
> + msg_push_bits(msg, crc, 4);
> +}
> +
> +static void build_abs_ar_command(struct fsi_gpio_msg *cmd,
> + uint8_t id, uint32_t addr, size_t size, const void *data)
> +{
> + bool write = !!data;
> + uint8_t ds;
> + int i;
> +
> + cmd->bits = 0;
> + cmd->msg = 0;
> +
> + msg_push_bits(cmd, id, 2);
> + msg_push_bits(cmd, FSI_GPIO_CMD_ABS_AR, 3);
> + msg_push_bits(cmd, write ? 0 : 1, 1);
> +
> + /*
> + * The read/write size is encoded in the lower bits of the address
> + * (as it must be naturally-aligned), and the following ds bit.
> + *
> + * size addr:1 addr:0 ds
> + * 1 x x 0
> + * 2 x 0 1
> + * 4 0 1 1
> + *
> + */
> + ds = size > 1 ? 1 : 0;
> + addr &= ~(size - 1);
> + if (size == 4)
> + addr |= 1;
> +
> + msg_push_bits(cmd, addr & ((1 << 21) - 1), 21);
> + msg_push_bits(cmd, ds, 1);
> + for (i = 0; write && i < size; i++)
> + msg_push_bits(cmd, ((uint8_t *)data)[i], 8);
> +
> + msg_push_crc(cmd);
> +}
> +
> +static void build_dpoll_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
> +{
> + cmd->bits = 0;
> + cmd->msg = 0;
> +
> + msg_push_bits(cmd, slave_id, 2);
> + msg_push_bits(cmd, FSI_GPIO_CMD_DPOLL, 3);
> + msg_push_crc(cmd);
> +}
> +
> +static void echo_delay(struct fsi_master_gpio *master)
> +{
> + set_sda_output(master, 1);
> + clock_toggle(master, FSI_ECHO_DELAY_CLOCKS);
> +}
> +
> +static void build_term_command(struct fsi_gpio_msg *cmd, uint8_t slave_id)
> +{
> + cmd->bits = 0;
> + cmd->msg = 0;
> +
> + msg_push_bits(cmd, slave_id, 2);
> + msg_push_bits(cmd, FSI_GPIO_CMD_TERM, 6);
> + msg_push_crc(cmd);
> +}
> +
> +/*
> + * Store information on master errors so handler can detect and clean
> + * up the bus
> + */
> +static void fsi_master_gpio_error(struct fsi_master_gpio *master, int error)
> +{
> +
> +}
> +
> +static int read_one_response(struct fsi_master_gpio *master,
> + uint8_t data_size, struct fsi_gpio_msg *msgp, uint8_t *tagp)
> +{
> + struct fsi_gpio_msg msg;
> + uint8_t id, tag;
> + uint32_t crc;
> + int i;
> +
> + /* wait for the start bit */
> + for (i = 0; i < FSI_GPIO_MTOE_COUNT; i++) {
> + msg.bits = 0;
> + msg.msg = 0;
> + serial_in(master, &msg, 1);
> + if (msg.msg)
> + break;
> + }
> + if (i >= FSI_GPIO_MTOE_COUNT) {
> + dev_dbg(master->dev,
> + "Master time out waiting for response\n");
> + fsi_master_gpio_error(master, FSI_GPIO_MTOE);
> + return -EIO;
> + }
> +
> + msg.bits = 0;
> + msg.msg = 0;
> +
> + /* Read slave ID & response tag */
> + serial_in(master, &msg, 4);
> +
> + id = (msg.msg >> FSI_GPIO_MSG_RESPID_SIZE) & 0x3;
> + tag = msg.msg & 0x3;
> +
> + /* if we have an ACK, and we're expecting data, clock the
> + * data in too
> + */
> + if (tag == FSI_GPIO_RESP_ACK && data_size)
> + serial_in(master, &msg, data_size * 8);
> +
> + /* read CRC */
> + serial_in(master, &msg, FSI_GPIO_CRC_SIZE);
> +
> + /* we have a whole message now; check CRC */
> + crc = fsi_crc4(0, 1, 1);
> + crc = fsi_crc4(crc, msg.msg, msg.bits);
> + if (crc) {
> + dev_dbg(master->dev, "ERR response CRC\n");
> + fsi_master_gpio_error(master, FSI_GPIO_CRC_INVAL);
> + return -EIO;
> + }
> +
> + if (msgp)
> + *msgp = msg;
> + if (tagp)
> + *tagp = tag;
> +
> + return 0;
> +}
> +
> +static int issue_term(struct fsi_master_gpio *master, uint8_t slave)
> +{
> + struct fsi_gpio_msg cmd;
> + uint8_t tag;
> + int rc;
> +
> + build_term_command(&cmd, slave);
> + serial_out(master, &cmd);
> + echo_delay(master);
> +
> + rc = read_one_response(master, 0, NULL, &tag);
> + if (rc) {
> + dev_err(master->dev,
> + "TERM failed; lost communication with slave\n");
> + return -EIO;
> + } else if (tag != FSI_GPIO_RESP_ACK) {
> + dev_err(master->dev, "TERM failed; response %d\n", tag);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int poll_for_response(struct fsi_master_gpio *master,
> + uint8_t slave, uint8_t size, void *data)
> +{
> + struct fsi_gpio_msg response, cmd;
> + int busy_count = 0, rc, i;
> + uint8_t tag;
> +
> +retry:
> + rc = read_one_response(master, size, &response, &tag);
> + if (rc)
> + return rc;
> +
> + switch (tag) {
> + case FSI_GPIO_RESP_ACK:
> + if (size && data) {
> + uint64_t val = response.msg;
> + /* clear crc & mask */
> + val >>= 4;
> + val &= (1ull << (size * 8)) - 1;
> +
> + for (i = 0; i < size; i++) {
> + ((uint8_t *)data)[size-i-1] =
> + val & 0xff;
> + val >>= 8;
> + }
> + }
> + break;
> + case FSI_GPIO_RESP_BUSY:
> + /*
> + * Its necessary to clock slave before issuing
> + * d-poll, not indicated in the hardware protocol
> + * spec. < 20 clocks causes slave to hang, 21 ok.
> + */
> + clock_zeros(master, FSI_GPIO_DPOLL_CLOCKS);
> + if (busy_count++ < FSI_GPIO_MAX_BUSY) {
> + build_dpoll_command(&cmd, slave);
> + serial_out(master, &cmd);
> + echo_delay(master);
> + goto retry;
> + }
> + dev_warn(master->dev,
> + "ERR slave is stuck in busy state, issuing TERM\n");
> + issue_term(master, slave);
> + rc = -EIO;
> + break;
> +
> + case FSI_GPIO_RESP_ERRA:
> + case FSI_GPIO_RESP_ERRC:
> + dev_dbg(master->dev, "ERR%c received: 0x%x\n",
> + tag == FSI_GPIO_RESP_ERRA ? 'A' : 'C',
> + (int)response.msg);
> + fsi_master_gpio_error(master, response.msg);
> + rc = -EIO;
> + break;
> + }
> +
> + /* Clock the slave enough to be ready for next operation */
> + clock_zeros(master, FSI_GPIO_PRIME_SLAVE_CLOCKS);
> + return rc;
> +}
> +
> +static int fsi_master_gpio_xfer(struct fsi_master_gpio *master, uint8_t slave,
> + struct fsi_gpio_msg *cmd, size_t resp_len, void *resp)
> +{
> + unsigned long flags;
> + int rc;
> +
> + spin_lock_irqsave(&fsi_gpio_cmd_lock, flags);
> + serial_out(master, cmd);
> + echo_delay(master);
> + rc = poll_for_response(master, slave, resp_len, resp);
> + spin_unlock_irqrestore(&fsi_gpio_cmd_lock, flags);
> +
> + return rc;
> +}
> +
> +static int fsi_master_gpio_read(struct fsi_master *_master, int link,
> + uint8_t id, uint32_t addr, void *val, size_t size)
> +{
> + struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
> + struct fsi_gpio_msg cmd;
> +
> + if (link != 0)
> + return -ENODEV;
> +
> + build_abs_ar_command(&cmd, id, addr, size, NULL);
> + return fsi_master_gpio_xfer(master, id, &cmd, size, val);
> +}
> +
> +static int fsi_master_gpio_write(struct fsi_master *_master, int link,
> + uint8_t id, uint32_t addr, const void *val, size_t size)
> +{
> + struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
> + struct fsi_gpio_msg cmd;
> +
> + if (link != 0)
> + return -ENODEV;
> +
> + build_abs_ar_command(&cmd, id, addr, size, val);
> + return fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
> +}
> +
> +static int fsi_master_gpio_term(struct fsi_master *_master,
> + int link, uint8_t id)
> +{
> + struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
> + struct fsi_gpio_msg cmd;
> +
> + if (link != 0)
> + return -ENODEV;
> +
> + build_term_command(&cmd, id);
> + return fsi_master_gpio_xfer(master, id, &cmd, 0, NULL);
> +}
> +
> +/*
> + * Issue a break command on link
> + */
> +static int fsi_master_gpio_break(struct fsi_master *_master, int link)
> +{
> + struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
> +
> + if (link != 0)
> + return -ENODEV;
> +
> + set_sda_output(master, 1);
> + sda_out(master, 1);
> + clock_toggle(master, FSI_PRE_BREAK_CLOCKS);
> + sda_out(master, 0);
> + clock_toggle(master, FSI_BREAK_CLOCKS);
> + echo_delay(master);
> + sda_out(master, 1);
> + clock_toggle(master, FSI_POST_BREAK_CLOCKS);
> +
> + /* Wait for logic reset to take effect */
> + udelay(200);
> +
> + return 0;
> +}
> +
> +static void fsi_master_gpio_init(struct fsi_master_gpio *master)
> +{
> + if (master->gpio_mux)
> + gpiod_direction_output(master->gpio_mux, 1);
> + if (master->gpio_trans)
> + gpiod_direction_output(master->gpio_trans, 1);
> + if (master->gpio_enable)
> + gpiod_direction_output(master->gpio_enable, 1);
> + gpiod_direction_output(master->gpio_clk, 1);
> + gpiod_direction_output(master->gpio_data, 1);
> +
> + /* todo: evaluate if clocks can be reduced */
> + clock_zeros(master, FSI_INIT_CLOCKS);
> +}
> +
> +static int fsi_master_gpio_link_enable(struct fsi_master *_master, int link)
> +{
> + struct fsi_master_gpio *master = to_fsi_master_gpio(_master);
> +
> + if (link != 0)
> + return -ENODEV;
> + if (master->gpio_enable)
> + gpiod_set_value(master->gpio_enable, 1);
> +
> + return 0;
> +}
> +
> +static void fsi_master_gpio_release(struct device *dev)
> +{
> + struct fsi_master_gpio *master = to_fsi_master_gpio(
> + dev_to_fsi_master(dev));
> +
> + kfree(master);
> +}
> +
> +static int fsi_master_gpio_probe(struct platform_device *pdev)
> +{
> + struct fsi_master_gpio *master;
> + struct gpio_desc *gpio;
> +
> + master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
> + if (!master)
> + return -ENOMEM;
> +
> + master->dev = &pdev->dev;
> + master->master.dev.parent = master->dev;
> + master->master.dev.release = fsi_master_gpio_release;
> +
> + gpio = devm_gpiod_get(&pdev->dev, "clock", 0);
> + if (IS_ERR(gpio)) {
> + dev_err(&pdev->dev, "failed to get clock gpio\n");
> + return PTR_ERR(gpio);
> + }
> + master->gpio_clk = gpio;
> +
> + gpio = devm_gpiod_get(&pdev->dev, "data", 0);
> + if (IS_ERR(gpio)) {
> + dev_err(&pdev->dev, "failed to get data gpio\n");
> + return PTR_ERR(gpio);
> + }
> + master->gpio_data = gpio;
> +
> + /* Optional GPIOs */
> + gpio = devm_gpiod_get_optional(&pdev->dev, "trans", 0);
> + if (IS_ERR(gpio)) {
> + dev_err(&pdev->dev, "failed to get trans gpio\n");
> + return PTR_ERR(gpio);
> + }
> + master->gpio_trans = gpio;
> +
> + gpio = devm_gpiod_get_optional(&pdev->dev, "enable", 0);
> + if (IS_ERR(gpio)) {
> + dev_err(&pdev->dev, "failed to get enable gpio\n");
> + return PTR_ERR(gpio);
> + }
> + master->gpio_enable = gpio;
> +
> + gpio = devm_gpiod_get_optional(&pdev->dev, "mux", 0);
> + if (IS_ERR(gpio)) {
> + dev_err(&pdev->dev, "failed to get mux gpio\n");
> + return PTR_ERR(gpio);
> + }
> + master->gpio_mux = gpio;
> +
> + master->master.n_links = 1;
> + master->master.read = fsi_master_gpio_read;
> + master->master.write = fsi_master_gpio_write;
> + master->master.term = fsi_master_gpio_term;
> + master->master.send_break = fsi_master_gpio_break;
> + master->master.link_enable = fsi_master_gpio_link_enable;
> + platform_set_drvdata(pdev, master);
> +
> + fsi_master_gpio_init(master);
> +
> + fsi_master_register(&master->master);
> +
> + return 0;
> +}
> +
> +
> +static int fsi_master_gpio_remove(struct platform_device *pdev)
> +{
> + struct fsi_master_gpio *master = platform_get_drvdata(pdev);
> +
> + devm_gpiod_put(&pdev->dev, master->gpio_clk);
> + devm_gpiod_put(&pdev->dev, master->gpio_data);
> + if (master->gpio_trans)
> + devm_gpiod_put(&pdev->dev, master->gpio_trans);
> + if (master->gpio_enable)
> + devm_gpiod_put(&pdev->dev, master->gpio_enable);
> + if (master->gpio_mux)
> + devm_gpiod_put(&pdev->dev, master->gpio_mux);
> + fsi_master_unregister(&master->master);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id fsi_master_gpio_match[] = {
> + { .compatible = "fsi-master-gpio" },
> + { },
> +};
> +
> +static struct platform_driver fsi_master_gpio_driver = {
> + .driver = {
> + .name = "fsi-master-gpio",
> + .of_match_table = fsi_master_gpio_match,
> + },
> + .probe = fsi_master_gpio_probe,
> + .remove = fsi_master_gpio_remove,
> +};
> +
> +module_platform_driver(fsi_master_gpio_driver);
> +MODULE_LICENSE("GPL");
> --
> 1.8.2.2
>
^ permalink raw reply
* Re: [PATCH RFC 1/3] thermal: allow hwmon devices to be created for of-thermal zones
From: Eduardo Valentin @ 2017-03-30 5:59 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Rob Herring, Mark Rutland, Andrew Lunn, Jason Cooper,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Gregory Clement, Zhang Rui,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Sebastian Hesselbarth
In-Reply-To: <20170324144029.GG7909-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org>
Hello,
On Fri, Mar 24, 2017 at 02:40:29PM +0000, Russell King - ARM Linux wrote:
> On Fri, Mar 24, 2017 at 01:23:13PM +0000, Russell King - ARM Linux wrote:
> > On Mon, Mar 20, 2017 at 12:15:52PM -0500, Rob Herring wrote:
> > > On Sun, Mar 12, 2017 at 07:07:40PM +0000, Russell King wrote:
> > > > Allow hwmon devices to be optionally created for of-thermal zones,
> > > > rather than permanently denying them "in case" there is a hwmon
> > > > driver duplicating the thermal driver.
> > > >
> > > > Signed-off-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
> > > > ---
> > > > Without this, "sensors" is unable to report the temperature of the
> > > > Dove SoC when it supports thermal zones.
> > > >
> > > > Documentation/devicetree/bindings/thermal/thermal.txt | 3 +++
> > > > arch/arm/boot/dts/dove.dtsi | 1 +
> > > > drivers/thermal/of-thermal.c | 3 ++-
> > > > 3 files changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/thermal/thermal.txt b/Documentation/devicetree/bindings/thermal/thermal.txt
> > > > index 88b6ea1ad290..1478735fff85 100644
> > > > --- a/Documentation/devicetree/bindings/thermal/thermal.txt
> > > > +++ b/Documentation/devicetree/bindings/thermal/thermal.txt
> > > > @@ -175,6 +175,9 @@ containing trip nodes and one sub-node containing all the zone cooling maps.
> > > > 2000mW, while on a 10'' tablet is around
> > > > 4500mW.
> > > >
> > > > +- linux,hwmon: Allow Linux to create hwmon devices for the thermal
> > > > + Type: bool zone.
> > > > +
> > > > Note: The delay properties are bound to the maximum dT/dt (temperature
> > > > derivative over time) in two situations for a thermal zone:
> > > > (i) - when passive cooling is activated (polling-delay-passive); and
> > > > diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
> > > > index 40fb98687230..00f5971cd039 100644
> > > > --- a/arch/arm/boot/dts/dove.dtsi
> > > > +++ b/arch/arm/boot/dts/dove.dtsi
> > > > @@ -804,6 +804,7 @@
> > > >
> > > > thermal-zones {
> > > > soc-thermal {
> > > > + linux,hwmon;
New DT properties have to get an Ack from OF maintainers.
> > >
> > > I'd prefer to see a black or white list of sensor compatibles here. Then
> > > the dtb doesn't need to change if things move between hwmon and thermal
> > > zones.
> >
> > I'm not sure what you mean, and I can't see how that would be coded up.
> > The thermal layer doesn't give the option of individually selecting
> > which sensors have hwmon stuff created - it seems to be all or nothing.
> > I suspect what you're asking would require something of a rewrite of
> > the thermal code. The sensors are registered completely independently
> > of parsing the thermal zone data.
>
I agree here that Rob could have been more clear. Maybe he just wants to
avoid DTB changes if the driver decides to move from one subsystem to
the other.
> Okay, I'm convinced that what you're asking for (if I'm understanding
> correctly) is just not possible with how the thermal code is (horribly)
> architected at the moment.
>
> While trying to understand how all the thermal zone stuff fits together,
> I found this comment:
>
> /* For now, thermal framework supports only 1 sensor per zone */
> ret = of_parse_phandle_with_args(child, "thermal-sensors",
> "#thermal-sensor-cells",
> 0, &sensor_specs);
>
> so it's really not setup to deal with multiple sensors in DT right now
> (and there's no checking by the code that this is even the case.)
>
Yes, the thermal core code still does not really support multiple
sensors per zone. Its original design decision to have one sensor
representing one zone is still kept, even though there has been attempts
to change this. So, the of thermal code, although the DT thermal
descriptors support multiple sensors per zone, is still limited to one
sensor per zone, otherwise it would be pretty much a fork from thermal
core.
> The way the code is architected, the thermal zone devices and hwmon
> support is setup _completely_ independently from attaching the single
> sensor to the zone - the setup of the thermal zone devices and
> registering the sysfs entry happens before the sensors.
>
> The hwmon support is coded such that there is one hwmon device for all
> thermal zones. Each thermal zone device exports exactly one hwmon
> temperature sensor value with one set of trip points.
>
Based on git log, the hwmon support was historically written some time
before some of the recent changes on the hwmon subsystem. Looking at
both of them now, I must say the thermal_hwmon.c needs to be rewritten,
given that most of the sysfs interfaces are already handled by the hwmon
core. Also, there seams to be thermal subsystem awareness now on hwmon
core.
> A thermal zone device can support multiple sensors, but from the code
> and the above comment, the code does not support this - the sensors
> are not individually exported through hwmon.
As I mentioned above, the DT thermal descriptor does not limit the
relationship to 1:1, but the code is limited, yes.
>
> So, I think what you're asking for is not possible and is based upon
> a mis-understanding - thermal does not export sensors, but exports the
> _zone_ itself as a hwmon sensor. The zone temperature is exported,
> along with its thresholds.
>
> Obviously, the separation of thermal vs hwmon is purely a Linux
> phenomenon, and I've wondered why people have to rewrite hwmon drivers
> as thermal drivers just to make use of the thermal subsystem - it seems
> to me like NIH syndrome. I can't see why hwmon isn't able to export
> individual temperature sensors to thermal and have thermal import the
> values from there.
>
Not sure if I got this point, but I do not see drivers being rewritten.
That is more like, organically, some drivers went into hwmon, some went
into thermal subsystem. I believe most of the drivers in the thermal
subsystem decided to go that route due to the control algorithm to keep
up on silicon temperature, given that most of them are on zones to
control temperature of cores.
> For example, you could have a hwmon device that has multiple temperature
> sensors (some do) which monitor different parts of the system, and a
> hwmon device should be able to be incorporated into a thermal zone for
> management.
>
I think this is the idea behind the most recent changes related to
thermal on the hwmon core, so hwmon temperature sensors could be exposed
as zones, and therefore, system engineers could benefit of the
temperature control in kernel. But I frankly did not really use this
late support to really judge to which extent we get on the control.
> That doesn't seem possible, you have to implement a thermal driver to
> interface your sensor to the thermal stuff, and then have the _zone_
> statistics exported through hwmon.
>
> If you want individual sensors exported, you have to implement a driver
> which has a dual-personality, it has to register with hwmon and thermal.
>
> I'd say that, arguably, the whole "no_hwmon" thing in thermal is also
> wrong - it may _seem_ to be correct because thermal only supports one
> sensor, and we don't want such a dual-personality driver to have two
> entries in sysfs, but that's rather moot when you consider that one is
> exporting the zone (which has DT configured thresholds) and one is
> exporting the sensor itself.
>
> So, my conclusion is that this is all rather messed up, and certainly
> beyond my willingness to put in a lot of effort, creating lots of
> patches to try and bring the code to a point where what you're asking
> for is possible (iow, where we export individual sensors to hwmon,
> and make individual sensors exportable.)
>
> Therefore, I believe that my implementation is entirely reasonable -
> the linux,hwmon flag indicates whether the state of the thermal _zone_
> (not the _sensors_) should be exported via hwmon.
>
Yeah, here your patch seams to be willing to use the thermal_hwmon
support, which goes from thermal subsystem to hwmon. And yes, the
no_hwmon, in this case, is used to avoid having two hwmon sysfs entry
for the same sensor, one coming from a real hwmon driver, another one
exported from the thermal zone.
The only reason one would still want to have both hwmon interfaces, it
would be in case a thermal zone has, for example, some extrapolation
rule defined in DT, and therefore reading the raw hwmon sysfs entry
would not necessarily mean you have the same thermal zone temperature
value. Then again, the zone temperature value can still, be read from
the thermal zone sysfs entry. But,...
Thinking of this use case, and considering that more and more drivers
are now using extrapolation rules on DT, at least the simple linear
extrapolation, then I would suggest simply always adding the hwmon interface
from of thermal, instead. In this way, userspace that knows only about
hwmon entries may be able to know of the raw values and the extrapolated
values based on the zone. Then again, this needs to be clear to avoid
confusion, so people understand the differentiation.
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ 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