* Re: [PATCH v2 02/30] pinctrl: mediatek: reuse pinctrl driver for mt7623
From: Sean Wang @ 2017-05-01 6:44 UTC (permalink / raw)
To: Linus Walleij
Cc: Rob Herring, Matthias Brugger, John Crispin, Mark Rutland,
Russell King, devicetree@vger.kernel.org,
moderated list:ARM/Mediatek SoC support,
linux-arm-kernel@lists.infradead.org, linux-gpio@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CACRpkdbR-k3wgD+7KoV6d7yg1uduXpa9MO0+woEibQYPeYzgYw@mail.gmail.com>
On Fri, 2017-04-28 at 10:01 +0200, Linus Walleij wrote:
> On Wed, Apr 26, 2017 at 11:25 AM, <sean.wang@mediatek.com> wrote:
>
> > From: Sean Wang <sean.wang@mediatek.com>
> >
> > mt7623 pinctrl driver can be compatible with mt2701 one,
> > so the patch reuses the driver and deletes those redundant
> > ones.
> >
> > Cc: John Crispin <john@phrozen.org>
> > Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>
> Partly correct.
>
> > "mediatek,mt6397-pinctrl", compatible with mt6397 pinctrl.
> > - "mediatek,mt7623-pinctrl", compatible with mt7623 pinctrl.
>
> NO don't do this.
>
> "compatible" means exactly this: this hardware is compatible with
> this driver. That is why we have it!
>
> So instead of mt7623 pretending to be mt2701, let the mt2701 driver
> list that it is compatible with mt7623, simple.
>
> So patch pinctrl-mt2701.c mt2701_pctrl_match[] instead.
>
Hi Linus,
really appreciate your clear guidance and reviewing on this
I will fix it up in the next version
Sean
> Yours,
> Linus Walleij
^ permalink raw reply
* Re: [PATCH 2/2] dmaengine: Add STM32 MDMA driver
From: Vinod Koul @ 2017-05-01 6:12 UTC (permalink / raw)
To: Pierre Yves MORDRET
Cc: M'boumba Cedric Madianga, mark.rutland@arm.com,
devicetree@vger.kernel.org, Alexandre TORGUE,
linux-kernel@vger.kernel.org, robh+dt@kernel.org,
mcoquelin.stm32@gmail.com, dmaengine@vger.kernel.org,
dan.j.williams@intel.com, linux-arm-kernel@lists.infradead.org
In-Reply-To: <d1d92f79-ce3c-e057-7f47-1fd5e42fa5dc@st.com>
On Wed, Apr 26, 2017 at 12:35:46PM +0000, Pierre Yves MORDRET wrote:
> On 04/06/2017 09:08 AM, Vinod Koul wrote:
> >> +static int stm32_mdma_get_width(struct stm32_mdma_chan *chan,
> >> + enum dma_slave_buswidth width)
> >> +{
> >> + switch (width) {
> >> + case DMA_SLAVE_BUSWIDTH_1_BYTE:
> >> + return STM32_MDMA_BYTE;
> >> + case DMA_SLAVE_BUSWIDTH_2_BYTES:
> >> + return STM32_MDMA_HALF_WORD;
> >> + case DMA_SLAVE_BUSWIDTH_4_BYTES:
> >> + return STM32_MDMA_WORD;
> >> + case DMA_SLAVE_BUSWIDTH_8_BYTES:
> >> + return STM32_MDMA_DOUBLE_WORD;
> >
> > IIUC we can do this with ffs()
>
> I don't believe we can do that. This function translates DMA_SLAVE enum
> into internal register representation.
Yes and internal represenation seemed to be ffs() of dmanegine one.
> >> +subsys_initcall(stm32_mdma_init);
> >
> > why subsys?
> >
>
> subsys_initcall level is to ensure MDMA is going to be probed before its
> clients
Please use deffered probe approach for that
--
~Vinod
^ permalink raw reply
* Re: [PATCH 2/5] dmaengine: Add STM32 DMAMUX driver
From: Vinod Koul @ 2017-05-01 6:03 UTC (permalink / raw)
To: Pierre Yves MORDRET
Cc: M'boumba Cedric Madianga, mark.rutland@arm.com,
devicetree@vger.kernel.org, Alexandre TORGUE,
linux-kernel@vger.kernel.org, robh+dt@kernel.org,
mcoquelin.stm32@gmail.com, dmaengine@vger.kernel.org,
dan.j.williams@intel.com, linux-arm-kernel@lists.infradead.org
In-Reply-To: <d18cb12d-1fd1-39d2-501a-18c000b9982e@st.com>
On Wed, Apr 26, 2017 at 09:17:37AM +0000, Pierre Yves MORDRET wrote:
> >> +
> >> + ret = of_property_read_u32(node, "dma-channels",
> >> + &dmamux->dmamux_channels);
> >
> > can we have property_xxx calls alone, that way driver is not strictly
> > dependent on of
>
> Can you please explain what you are asking for ? Not sure to understand
> correctly.
Use device_property_read_u32() which is a generic property API.
> >> +static int __init stm32_dmamux_init(void)
> >> +{
> >> + return platform_driver_register(&stm32_dmamux_driver);
> >> +}
> >> +arch_initcall(stm32_dmamux_init);
> >
> > why not module init, wouldnt defer probe solve the dependencies
> >
>
> The reason behind many devices (device_initcall level) rely on DMAs. If
> init is deferred DMAMUX driver will be probed twice if dependents rely
> on it. This sounds not a good call. This explains arch_initcall level.
Why not deferred probe was introduced to help with dependencies...
--
~Vinod
^ permalink raw reply
* [PATCH v2 2/2] ARM: sun8i: a83t: Replace underscores with hyphens in pinmux node names
From: Chen-Yu Tsai @ 2017-05-01 3:14 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170501031408.10469-1-wens-jdAy2FN1RRM@public.gmane.org>
We should use hyphens and not underscores in device node names.
Replace the ones that were just added.
Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index aecde8be53bc..c0a1e4f74b89 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -174,7 +174,7 @@
#interrupt-cells = <3>;
#gpio-cells = <3>;
- mmc0_pins: mmc0_pins {
+ mmc0_pins: mmc0-pins {
pins = "PF0", "PF1", "PF2",
"PF3", "PF4", "PF5";
function = "mmc0";
@@ -182,12 +182,12 @@
bias-pull-up;
};
- uart0_pb_pins: uart0_pb_pins {
+ uart0_pb_pins: uart0-pb-pins {
pins = "PB9", "PB10";
function = "uart0";
};
- uart0_pf_pins: uart0_pf_pins {
+ uart0_pf_pins: uart0-pf-pins {
pins = "PF2", "PF4";
function = "uart0";
};
--
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 v2 1/2] ARM: sun8i: a83t: Drop leading zeroes from device node addresses
From: Chen-Yu Tsai @ 2017-05-01 3:14 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170501031408.10469-1-wens-jdAy2FN1RRM@public.gmane.org>
Kbuild now complains about leading zeroes in the address portion of
device node names.
Get rid of them all, except for the uart device node. U-boot currently
hard codes the device node path. We can remove the leading zero for
the uart once we teach U-boot to use the aliases or stdout-path
property.
Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
arch/arm/boot/dts/sun8i-a83t.dtsi | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 5f5c10c04dd3..aecde8be53bc 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -162,7 +162,7 @@
#size-cells = <1>;
ranges;
- pio: pinctrl@01c20800 {
+ pio: pinctrl@1c20800 {
compatible = "allwinner,sun8i-a83t-pinctrl";
interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>,
@@ -193,7 +193,7 @@
};
};
- timer@01c20c00 {
+ timer@1c20c00 {
compatible = "allwinner,sun4i-a10-timer";
reg = <0x01c20c00 0xa0>;
interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>,
@@ -201,7 +201,7 @@
clocks = <&osc24M>;
};
- watchdog@01c20ca0 {
+ watchdog@1c20ca0 {
compatible = "allwinner,sun6i-a31-wdt";
reg = <0x01c20ca0 0x20>;
interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
@@ -218,7 +218,7 @@
status = "disabled";
};
- gic: interrupt-controller@01c81000 {
+ gic: interrupt-controller@1c81000 {
compatible = "arm,cortex-a7-gic", "arm,cortex-a15-gic";
reg = <0x01c81000 0x1000>,
<0x01c82000 0x2000>,
--
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 v2 0/2] ARM: sun8i: a83t: device tree cleanup
From: Chen-Yu Tsai @ 2017-05-01 3:14 UTC (permalink / raw)
To: Maxime Ripard
Cc: Chen-Yu Tsai, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Hi Maxime,
Here's v2 of the A83T device tree cleanup patches. I dropped the change
to the uart device node name for now.
Also added a second patch changing the underscores in device node names
I just added to hyphens. AFAIK that is the preferred naming scheme.
Please squash it into "ARM: sun8i: a83t: Rename pinmux setting names".
Regards
ChenYu
Chen-Yu Tsai (2):
ARM: sun8i: a83t: Drop leading zeroes from device node addresses
ARM: sun8i: a83t: Replace underscores with hyphens in pinmux node
names
arch/arm/boot/dts/sun8i-a83t.dtsi | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
--
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
* Re: [PATCH v3 1/2] net: dsa: b53: Add compatible strings for the Cygnus-family BCM11360.
From: David Miller @ 2017-05-01 2:48 UTC (permalink / raw)
To: eric-WhKQ6XTQaPysTnJN9+BGXg
Cc: f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
vivien.didelot-4ysUXcep3aM1wj+D4I0NRVaTQe2KTcn/,
andrew-g2DYL2Zd6BY, netdev-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
rjui-dY08KVG/lbpWk0Htik3J/w, sbranden-dY08KVG/lbpWk0Htik3J/w,
jonmason-dY08KVG/lbpWk0Htik3J/w
In-Reply-To: <20170428222204.7103-1-eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
From: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
Date: Fri, 28 Apr 2017 15:22:03 -0700
> Cygnus is a small family of SoCs, of which we currently have
> devicetree for BCM11360 and BCM58300. The 11360's B53 is mostly the
> same as 58xx, just requiring a tiny bit of setup that was previously
> missing.
>
> v2: Reorder the entry in the docs (suggestion by Scott Branden), add
> missing '"'
>
> Signed-off-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
> Reviewed-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
The second patch with the DTS file update doesn't apply cleanly
at all to net-next.
So I'm dropping this series.
--
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: [PATCH 1/3] ARM: dts: rockchip: Move cros-ec-sbs to rk3288-veyron-chromebook-sbs
From: Paul Kocialkowski @ 2017-04-30 20:56 UTC (permalink / raw)
To: Heiko Stuebner, briannorris-F7+t8E8rja9g9hUCZPvPmw
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
Russell King
In-Reply-To: <2369975.a4dWayqU5d@phil>
[-- Attachment #1: Type: text/plain, Size: 5700 bytes --]
Le dimanche 30 avril 2017 à 22:37 +0200, Heiko Stuebner a écrit :
> Hi Paul,
>
> Am Sonntag, 30. April 2017, 20:30:52 CEST schrieb Paul Kocialkowski:
> > This moves the cros-ec-sbs dtsi to a new rk3288-veyron-chromebook-sbs
> > dtsi since it only concerns rk3288 veyron Chromebooks.
> >
> > Other Chromebooks (such as the tegra124 nyans) also have sbs batteries
> > and don't use this dtsi, that only makes sense when used with
> > rk3288-veyron-chromebook anyway.
>
> That isn't true. The gru series (rk3399-based) also uses the
> sbs-battery [0]. And while it is currently limited to Rockchip-based
> Chromebooks it is nevertheless used on more than one platform, so
> the probability is high that it will be used in future series as well.
That's good to know, but as pointed out, other cros devices are using a sbs
battery without this header, so such a generic name isn't really a good fit.
Note that &charger has to be defined (after my subsequent patches), which it is
for devices that also include rk3288-veyron-chromebook, but not necessarily
others.
Overall, I think having one -sbs dtsi file makes sense here because there is
already a rk3288-veyron-chromebook dtsi that veyron chromebooks use. That file
cannot contain the battery bindings because minnie has a different one and it
would be a bit silly to copy it over all devices. That definitely makes sense.
As for other devices, I don't see why we should have a separate include file for
the battery instead of having it in the device's dts. I think this should be the
case on gru/kevin.
Also maybe not *all* gru-based devices will turn out to use a SBS battery, so it
seems early to include this header in the gru dtsi. One last point, gru/kevin
currently don't define a charger, which will break my subsequent patch (that is
however needed for the veyrons that use this file).
To me, it seems that there's little advantage and major drawbacks in keeping
this file the way it is.
> Also, it might be nice to also include some Chromeos people (there should
> be some in the git logs, like Brian who submitted the Gru patches), as they
> might be able to provide more detailed input.
That's a good point, thanks for including them.
>
> Heiko
>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/a
> rch/arm64/boot/dts/rockchip/rk3399-gru.dtsi#n886
>
> >
> > Signed-off-by: Paul Kocialkowski <contact-W9ppeneeCTY@public.gmane.org>
> > ---
> > .../boot/dts/{cros-ec-sbs.dtsi => rk3288-veyron-chromebook-sbs.dtsi} | 0
> > arch/arm/boot/dts/rk3288-veyron-jaq.dts | 2
> > +-
> > arch/arm/boot/dts/rk3288-veyron-jerry.dts | 2
> > +-
> > arch/arm/boot/dts/rk3288-veyron-pinky.dts | 2
> > +-
> > arch/arm/boot/dts/rk3288-veyron-speedy.dts | 2
> > +-
> > 5 files changed, 4 insertions(+), 4 deletions(-)
> > rename arch/arm/boot/dts/{cros-ec-sbs.dtsi => rk3288-veyron-chromebook-
> > sbs.dtsi} (100%)
> >
> > diff --git a/arch/arm/boot/dts/cros-ec-sbs.dtsi b/arch/arm/boot/dts/rk3288-
> > veyron-chromebook-sbs.dtsi
> > similarity index 100%
> > rename from arch/arm/boot/dts/cros-ec-sbs.dtsi
> > rename to arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi
> > diff --git a/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> > b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> > index d33f5763c39c..f217a978e47a 100644
> > --- a/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> > +++ b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> > @@ -45,7 +45,7 @@
> > /dts-v1/;
> >
> > #include "rk3288-veyron-chromebook.dtsi"
> > -#include "cros-ec-sbs.dtsi"
> > +#include "rk3288-veyron-chromebook-sbs.dtsi"
> >
> > / {
> > model = "Google Jaq";
> > diff --git a/arch/arm/boot/dts/rk3288-veyron-jerry.dts
> > b/arch/arm/boot/dts/rk3288-veyron-jerry.dts
> > index cdea751f2a8c..bec607574165 100644
> > --- a/arch/arm/boot/dts/rk3288-veyron-jerry.dts
> > +++ b/arch/arm/boot/dts/rk3288-veyron-jerry.dts
> > @@ -44,7 +44,7 @@
> >
> > /dts-v1/;
> > #include "rk3288-veyron-chromebook.dtsi"
> > -#include "cros-ec-sbs.dtsi"
> > +#include "rk3288-veyron-chromebook-sbs.dtsi"
> >
> > / {
> > model = "Google Jerry";
> > diff --git a/arch/arm/boot/dts/rk3288-veyron-pinky.dts
> > b/arch/arm/boot/dts/rk3288-veyron-pinky.dts
> > index 995cff42fa43..c81ad5bf1121 100644
> > --- a/arch/arm/boot/dts/rk3288-veyron-pinky.dts
> > +++ b/arch/arm/boot/dts/rk3288-veyron-pinky.dts
> > @@ -44,7 +44,7 @@
> >
> > /dts-v1/;
> > #include "rk3288-veyron-chromebook.dtsi"
> > -#include "cros-ec-sbs.dtsi"
> > +#include "rk3288-veyron-chromebook-sbs.dtsi"
> >
> > / {
> > model = "Google Pinky";
> > diff --git a/arch/arm/boot/dts/rk3288-veyron-speedy.dts
> > b/arch/arm/boot/dts/rk3288-veyron-speedy.dts
> > index cc0b78cefe34..8aea9c3ff6e2 100644
> > --- a/arch/arm/boot/dts/rk3288-veyron-speedy.dts
> > +++ b/arch/arm/boot/dts/rk3288-veyron-speedy.dts
> > @@ -44,7 +44,7 @@
> >
> > /dts-v1/;
> > #include "rk3288-veyron-chromebook.dtsi"
> > -#include "cros-ec-sbs.dtsi"
> > +#include "rk3288-veyron-chromebook-sbs.dtsi"
> >
> > / {
> > model = "Google Speedy";
> >
>
>
--
Paul Kocialkowski, developer of free digital technology and hardware support
Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 1/2] input: keyboard: DT bindings for the D-Link DIR-685 touchpad
From: Linus Walleij @ 2017-04-30 20:55 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input; +Cc: Linus Walleij, devicetree
This adds device tree bindings for the D-Link DIR-685 touchpad.
It's a simple homebrewn touchpad on I2C.
Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
.../bindings/input/dlink,dir685-touchpad.txt | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/dlink,dir685-touchpad.txt
diff --git a/Documentation/devicetree/bindings/input/dlink,dir685-touchpad.txt b/Documentation/devicetree/bindings/input/dlink,dir685-touchpad.txt
new file mode 100644
index 000000000000..25bc538a6e39
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/dlink,dir685-touchpad.txt
@@ -0,0 +1,21 @@
+* D-Link DIR-685 Touchpad
+
+This is a I2C one-off touchpad controller based on the Cypress Semiconductor
+CY8C214 MCU with some firmware in its internal 8KB flash. The circuit
+board inside the router is named E119921.
+
+The touchpad device node should be placed inside an I2C bus node.
+
+Required properties:
+- compatible: must be "dlink,dir685-touchpad"
+- reg: the I2C address of the touchpad
+- interrupts: reference to the interrupt number
+
+Example:
+
+touchpad@26 {
+ compatible = "dlink,dir685-touchpad";
+ reg = <0x26>;
+ interrupt-parent = <&gpio0>;
+ interrupts = <17 IRQ_TYPE_EDGE_FALLING>;
+};
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v3 1/4] of: remove *phandle properties from expanded device tree
From: Frank Rowand @ 2017-04-30 20:49 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all-JC7UmRfGjtg, Rob Herring,
stephen.boyd-QSEj5FYQhm4dnm+yROfE0A,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <201704300820.jFYE4Inc%fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
On 04/29/17 17:22, kbuild test robot wrote:
> Hi Frank,
>
> [auto build test ERROR on robh/for-next]
> [also build test ERROR on v4.11-rc8 next-20170428]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/frowand-list-gmail-com/of-remove-phandle-properties-from-expanded-device-tree/20170426-000149
> base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
> config: s390-allmodconfig (attached as .config)
> compiler: s390x-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
> wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=s390
>
> All errors (new ones prefixed by >>):
>
> In file included from include/linux/kobject.h:21:0,
> from include/linux/device.h:17,
> from include/linux/node.h:17,
> from include/linux/cpu.h:16,
> from drivers/of/base.c:25:
> drivers/of/base.c: In function '__of_add_phandle_sysfs':
>>> drivers/of/base.c:198:23: error: 'pp' undeclared (first use in this function)
> sysfs_bin_attr_init(&pp->attr);
> ^
Thanks for the report!
A patch to fix this is now submitted to Rob.
-Frank
> include/linux/sysfs.h:54:3: note: in definition of macro 'sysfs_attr_init'
> (attr)->key = &__key; \
> ^~~~
> drivers/of/base.c:198:2: note: in expansion of macro 'sysfs_bin_attr_init'
> sysfs_bin_attr_init(&pp->attr);
> ^~~~~~~~~~~~~~~~~~~
> drivers/of/base.c:198:23: note: each undeclared identifier is reported only once for each function it appears in
> sysfs_bin_attr_init(&pp->attr);
> ^
> include/linux/sysfs.h:54:3: note: in definition of macro 'sysfs_attr_init'
> (attr)->key = &__key; \
> ^~~~
> drivers/of/base.c:198:2: note: in expansion of macro 'sysfs_bin_attr_init'
> sysfs_bin_attr_init(&pp->attr);
> ^~~~~~~~~~~~~~~~~~~
>
> vim +/pp +198 drivers/of/base.c
>
> 19 */
> 20
> 21 #define pr_fmt(fmt) "OF: " fmt
> 22
> 23 #include <linux/console.h>
> 24 #include <linux/ctype.h>
> > 25 #include <linux/cpu.h>
> 26 #include <linux/module.h>
> 27 #include <linux/of.h>
> 28 #include <linux/of_device.h>
> 29 #include <linux/of_graph.h>
> 30 #include <linux/spinlock.h>
> 31 #include <linux/slab.h>
> 32 #include <linux/string.h>
> 33 #include <linux/proc_fs.h>
> 34
> 35 #include "of_private.h"
> 36
> 37 LIST_HEAD(aliases_lookup);
> 38
> 39 struct device_node *of_root;
> 40 EXPORT_SYMBOL(of_root);
> 41 struct device_node *of_chosen;
> 42 struct device_node *of_aliases;
> 43 struct device_node *of_stdout;
> 44 static const char *of_stdout_options;
> 45
> 46 struct kset *of_kset;
> 47
> 48 /*
> 49 * Used to protect the of_aliases, to hold off addition of nodes to sysfs.
> 50 * This mutex must be held whenever modifications are being made to the
> 51 * device tree. The of_{attach,detach}_node() and
> 52 * of_{add,remove,update}_property() helpers make sure this happens.
> 53 */
> 54 DEFINE_MUTEX(of_mutex);
> 55
> 56 /* use when traversing tree through the child, sibling,
> 57 * or parent members of struct device_node.
> 58 */
> 59 DEFINE_RAW_SPINLOCK(devtree_lock);
> 60
> 61 int of_n_addr_cells(struct device_node *np)
> 62 {
> 63 const __be32 *ip;
> 64
> 65 do {
> 66 if (np->parent)
> 67 np = np->parent;
> 68 ip = of_get_property(np, "#address-cells", NULL);
> 69 if (ip)
> 70 return be32_to_cpup(ip);
> 71 } while (np->parent);
> 72 /* No #address-cells property for the root node */
> 73 return OF_ROOT_NODE_ADDR_CELLS_DEFAULT;
> 74 }
> 75 EXPORT_SYMBOL(of_n_addr_cells);
> 76
> 77 int of_n_size_cells(struct device_node *np)
> 78 {
> 79 const __be32 *ip;
> 80
> 81 do {
> 82 if (np->parent)
> 83 np = np->parent;
> 84 ip = of_get_property(np, "#size-cells", NULL);
> 85 if (ip)
> 86 return be32_to_cpup(ip);
> 87 } while (np->parent);
> 88 /* No #size-cells property for the root node */
> 89 return OF_ROOT_NODE_SIZE_CELLS_DEFAULT;
> 90 }
> 91 EXPORT_SYMBOL(of_n_size_cells);
> 92
> 93 #ifdef CONFIG_NUMA
> 94 int __weak of_node_to_nid(struct device_node *np)
> 95 {
> 96 return NUMA_NO_NODE;
> 97 }
> 98 #endif
> 99
> 100 #ifndef CONFIG_OF_DYNAMIC
> 101 static void of_node_release(struct kobject *kobj)
> 102 {
> 103 /* Without CONFIG_OF_DYNAMIC, no nodes gets freed */
> 104 }
> 105 #endif /* CONFIG_OF_DYNAMIC */
> 106
> 107 struct kobj_type of_node_ktype = {
> 108 .release = of_node_release,
> 109 };
> 110
> 111 static ssize_t of_node_property_read(struct file *filp, struct kobject *kobj,
> 112 struct bin_attribute *bin_attr, char *buf,
> 113 loff_t offset, size_t count)
> 114 {
> 115 struct property *pp = container_of(bin_attr, struct property, attr);
> 116 return memory_read_from_buffer(buf, count, &offset, pp->value, pp->length);
> 117 }
> 118
> 119 static ssize_t of_node_phandle_read(struct file *filp, struct kobject *kobj,
> 120 struct bin_attribute *bin_attr, char *buf,
> 121 loff_t offset, size_t count)
> 122 {
> 123 phandle phandle;
> 124 struct device_node *np;
> 125
> 126 np = container_of(bin_attr, struct device_node, attr_phandle);
> 127 phandle = cpu_to_be32(np->phandle);
> 128 return memory_read_from_buffer(buf, count, &offset, &phandle,
> 129 sizeof(phandle));
> 130 }
> 131
> 132 /* always return newly allocated name, caller must free after use */
> 133 static const char *safe_name(struct kobject *kobj, const char *orig_name)
> 134 {
> 135 const char *name = orig_name;
> 136 struct kernfs_node *kn;
> 137 int i = 0;
> 138
> 139 /* don't be a hero. After 16 tries give up */
> 140 while (i < 16 && (kn = sysfs_get_dirent(kobj->sd, name))) {
> 141 sysfs_put(kn);
> 142 if (name != orig_name)
> 143 kfree(name);
> 144 name = kasprintf(GFP_KERNEL, "%s#%i", orig_name, ++i);
> 145 }
> 146
> 147 if (name == orig_name) {
> 148 name = kstrdup(orig_name, GFP_KERNEL);
> 149 } else {
> 150 pr_warn("Duplicate name in %s, renamed to \"%s\"\n",
> 151 kobject_name(kobj), name);
> 152 }
> 153 return name;
> 154 }
> 155
> 156 int __of_add_property_sysfs(struct device_node *np, struct property *pp)
> 157 {
> 158 int rc;
> 159
> 160 /* Important: Don't leak passwords */
> 161 bool secure = strncmp(pp->name, "security-", 9) == 0;
> 162
> 163 if (!IS_ENABLED(CONFIG_SYSFS))
> 164 return 0;
> 165
> 166 if (!of_kset || !of_node_is_attached(np))
> 167 return 0;
> 168
> 169 sysfs_bin_attr_init(&pp->attr);
> 170 pp->attr.attr.name = safe_name(&np->kobj, pp->name);
> 171 pp->attr.attr.mode = secure ? S_IRUSR : S_IRUGO;
> 172 pp->attr.size = secure ? 0 : pp->length;
> 173 pp->attr.read = of_node_property_read;
> 174
> 175 rc = sysfs_create_bin_file(&np->kobj, &pp->attr);
> 176 WARN(rc, "error adding attribute %s to node %s\n", pp->name, np->full_name);
> 177 return rc;
> 178 }
> 179
> 180 /*
> 181 * In the imported device tree (fdt), phandle is a property. In the
> 182 * internal data structure it is instead stored in the struct device_node.
> 183 * Make phandle visible in sysfs as if it was a property.
> 184 */
> 185 int __of_add_phandle_sysfs(struct device_node *np)
> 186 {
> 187 int rc;
> 188
> 189 if (!IS_ENABLED(CONFIG_SYSFS))
> 190 return 0;
> 191
> 192 if (!of_kset || !of_node_is_attached(np))
> 193 return 0;
> 194
> 195 if (!np->phandle || np->phandle == 0xffffffff)
> 196 return 0;
> 197
> > 198 sysfs_bin_attr_init(&pp->attr);
> 199 np->attr_phandle.attr.name = "phandle";
> 200 np->attr_phandle.attr.mode = 0444;
> 201 np->attr_phandle.size = sizeof(np->phandle);
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
--
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] of: undeclared variable when CONFIG_DEBUG_LOCK_ALLOC
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-04-30 20:45 UTC (permalink / raw)
To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
An undeclared variable was used in a macro that evaluates to nothing
when CONFIG_DEBUG_LOCK_ALLOC is not defined. Change to use the correct
variable that does exist.
---
reported by kbuild test robot on on robh/for-next
https://lkml.org/lkml/2017/4/29/134
drivers/of/base.c: In function '__of_add_phandle_sysfs':
>> drivers/of/base.c:198:23: error: 'pp' undeclared (first use in this function)
sysfs_bin_attr_init(&pp->attr);
Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
drivers/of/base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 9fe42346925b..8a7ca2a49ba8 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -195,7 +195,7 @@ int __of_add_phandle_sysfs(struct device_node *np)
if (!np->phandle || np->phandle == 0xffffffff)
return 0;
- sysfs_bin_attr_init(&pp->attr);
+ sysfs_bin_attr_init(&np->attr_phandle);
np->attr_phandle.attr.name = "phandle";
np->attr_phandle.attr.mode = 0444;
np->attr_phandle.size = sizeof(np->phandle);
--
Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
--
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
* Re: [PATCH 1/3] ARM: dts: rockchip: Move cros-ec-sbs to rk3288-veyron-chromebook-sbs
From: Heiko Stuebner @ 2017-04-30 20:37 UTC (permalink / raw)
To: Paul Kocialkowski, briannorris
Cc: Mark Rutland, devicetree, linux-kernel, Russell King,
linux-rockchip, Rob Herring, linux-arm-kernel
In-Reply-To: <20170430183054.24563-1-contact@paulk.fr>
Hi Paul,
Am Sonntag, 30. April 2017, 20:30:52 CEST schrieb Paul Kocialkowski:
> This moves the cros-ec-sbs dtsi to a new rk3288-veyron-chromebook-sbs
> dtsi since it only concerns rk3288 veyron Chromebooks.
>
> Other Chromebooks (such as the tegra124 nyans) also have sbs batteries
> and don't use this dtsi, that only makes sense when used with
> rk3288-veyron-chromebook anyway.
That isn't true. The gru series (rk3399-based) also uses the
sbs-battery [0]. And while it is currently limited to Rockchip-based
Chromebooks it is nevertheless used on more than one platform, so
the probability is high that it will be used in future series as well.
Also, it might be nice to also include some Chromeos people (there should
be some in the git logs, like Brian who submitted the Gru patches), as they
might be able to provide more detailed input.
Heiko
[0] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi#n886
>
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
> .../boot/dts/{cros-ec-sbs.dtsi => rk3288-veyron-chromebook-sbs.dtsi} | 0
> arch/arm/boot/dts/rk3288-veyron-jaq.dts | 2 +-
> arch/arm/boot/dts/rk3288-veyron-jerry.dts | 2 +-
> arch/arm/boot/dts/rk3288-veyron-pinky.dts | 2 +-
> arch/arm/boot/dts/rk3288-veyron-speedy.dts | 2 +-
> 5 files changed, 4 insertions(+), 4 deletions(-)
> rename arch/arm/boot/dts/{cros-ec-sbs.dtsi => rk3288-veyron-chromebook-sbs.dtsi} (100%)
>
> diff --git a/arch/arm/boot/dts/cros-ec-sbs.dtsi b/arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi
> similarity index 100%
> rename from arch/arm/boot/dts/cros-ec-sbs.dtsi
> rename to arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi
> diff --git a/arch/arm/boot/dts/rk3288-veyron-jaq.dts b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> index d33f5763c39c..f217a978e47a 100644
> --- a/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> +++ b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
> @@ -45,7 +45,7 @@
> /dts-v1/;
>
> #include "rk3288-veyron-chromebook.dtsi"
> -#include "cros-ec-sbs.dtsi"
> +#include "rk3288-veyron-chromebook-sbs.dtsi"
>
> / {
> model = "Google Jaq";
> diff --git a/arch/arm/boot/dts/rk3288-veyron-jerry.dts b/arch/arm/boot/dts/rk3288-veyron-jerry.dts
> index cdea751f2a8c..bec607574165 100644
> --- a/arch/arm/boot/dts/rk3288-veyron-jerry.dts
> +++ b/arch/arm/boot/dts/rk3288-veyron-jerry.dts
> @@ -44,7 +44,7 @@
>
> /dts-v1/;
> #include "rk3288-veyron-chromebook.dtsi"
> -#include "cros-ec-sbs.dtsi"
> +#include "rk3288-veyron-chromebook-sbs.dtsi"
>
> / {
> model = "Google Jerry";
> diff --git a/arch/arm/boot/dts/rk3288-veyron-pinky.dts b/arch/arm/boot/dts/rk3288-veyron-pinky.dts
> index 995cff42fa43..c81ad5bf1121 100644
> --- a/arch/arm/boot/dts/rk3288-veyron-pinky.dts
> +++ b/arch/arm/boot/dts/rk3288-veyron-pinky.dts
> @@ -44,7 +44,7 @@
>
> /dts-v1/;
> #include "rk3288-veyron-chromebook.dtsi"
> -#include "cros-ec-sbs.dtsi"
> +#include "rk3288-veyron-chromebook-sbs.dtsi"
>
> / {
> model = "Google Pinky";
> diff --git a/arch/arm/boot/dts/rk3288-veyron-speedy.dts b/arch/arm/boot/dts/rk3288-veyron-speedy.dts
> index cc0b78cefe34..8aea9c3ff6e2 100644
> --- a/arch/arm/boot/dts/rk3288-veyron-speedy.dts
> +++ b/arch/arm/boot/dts/rk3288-veyron-speedy.dts
> @@ -44,7 +44,7 @@
>
> /dts-v1/;
> #include "rk3288-veyron-chromebook.dtsi"
> -#include "cros-ec-sbs.dtsi"
> +#include "rk3288-veyron-chromebook-sbs.dtsi"
>
> / {
> model = "Google Speedy";
>
^ permalink raw reply
* [PATCH] of: undeclared variable when CONFIG_DEBUG_LOCK_ALLOC
From: Frank Rowand @ 2017-04-30 20:00 UTC (permalink / raw)
To: Rob Herring, stephen.boyd; +Cc: Frank Rowand, devicetree, linux-kernel
An undeclared variable was used in a macro that evaluates to nothing
when CONFIG_DEBUG_LOCK_ALLOC is not defined. Change to use the correct
variable that does exist.
---
reported by kbuild test robot on on robh/for-next
https://lkml.org/lkml/2017/4/29/134
drivers/of/base.c: In function '__of_add_phandle_sysfs':
>> drivers/of/base.c:198:23: error: 'pp' undeclared (first use in this function)
sysfs_bin_attr_init(&pp->attr);
Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---
drivers/of/base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 9fe42346925b..8a7ca2a49ba8 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -195,7 +195,7 @@ int __of_add_phandle_sysfs(struct device_node *np)
if (!np->phandle || np->phandle == 0xffffffff)
return 0;
- sysfs_bin_attr_init(&pp->attr);
+ sysfs_bin_attr_init(&np->attr_phandle);
np->attr_phandle.attr.name = "phandle";
np->attr_phandle.attr.mode = 0444;
np->attr_phandle.size = sizeof(np->phandle);
--
Frank Rowand <frank.rowand@sony.com>
^ permalink raw reply related
* [PATCH 3/3] ARM: dts: rockchip: List charger as power supply for minnie
From: Paul Kocialkowski @ 2017-04-30 18:30 UTC (permalink / raw)
To: linux-arm-kernel, linux-rockchip, devicetree, linux-kernel
Cc: Heiko Stuebner, Rob Herring, Mark Rutland, Russell King,
Paul Kocialkowski
In-Reply-To: <20170430183054.24563-1-contact@paulk.fr>
This lists the GPIO charger node as power supply for the battery, which
in turns allows receiving external power notifications and synchronizing
the charging state as soon as possible.
Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
arch/arm/boot/dts/rk3288-veyron-minnie.dts | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/rk3288-veyron-minnie.dts b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
index 544de6027aaa..3f97f33bd783 100644
--- a/arch/arm/boot/dts/rk3288-veyron-minnie.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-minnie.dts
@@ -151,6 +151,7 @@
battery: bq27500@55 {
compatible = "ti,bq27500";
reg = <0x55>;
+ power-supplies = <&charger>;
};
};
--
2.12.2
^ permalink raw reply related
* [PATCH 2/3] ARM: dts: rockchip: List charger as power supply for sbs battery
From: Paul Kocialkowski @ 2017-04-30 18:30 UTC (permalink / raw)
To: linux-arm-kernel, linux-rockchip, devicetree, linux-kernel
Cc: Heiko Stuebner, Rob Herring, Mark Rutland, Russell King,
Paul Kocialkowski
In-Reply-To: <20170430183054.24563-1-contact@paulk.fr>
This lists the GPIO charger node as power supply for the battery, which
in turns allows receiving external power notifications and synchronizing
the charging state as soon as possible.
Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi | 1 +
arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi b/arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi
index 71f5c5ecce46..8e4d2b9a35e1 100644
--- a/arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi
@@ -48,5 +48,6 @@
reg = <0xb>;
sbs,i2c-retry-count = <2>;
sbs,poll-retry-count = <1>;
+ power-supplies = <&charger>;
};
};
diff --git a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
index d752a315f884..fd4a3886c94b 100644
--- a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
@@ -99,7 +99,7 @@
pwm-delay-us = <10000>;
};
- gpio-charger {
+ charger: gpio-charger {
compatible = "gpio-charger";
charger-type = "mains";
gpios = <&gpio0 RK_PB0 GPIO_ACTIVE_HIGH>;
--
2.12.2
^ permalink raw reply related
* [PATCH 1/3] ARM: dts: rockchip: Move cros-ec-sbs to rk3288-veyron-chromebook-sbs
From: Paul Kocialkowski @ 2017-04-30 18:30 UTC (permalink / raw)
To: linux-arm-kernel, linux-rockchip, devicetree, linux-kernel
Cc: Heiko Stuebner, Rob Herring, Mark Rutland, Russell King,
Paul Kocialkowski
This moves the cros-ec-sbs dtsi to a new rk3288-veyron-chromebook-sbs
dtsi since it only concerns rk3288 veyron Chromebooks.
Other Chromebooks (such as the tegra124 nyans) also have sbs batteries
and don't use this dtsi, that only makes sense when used with
rk3288-veyron-chromebook anyway.
Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
.../boot/dts/{cros-ec-sbs.dtsi => rk3288-veyron-chromebook-sbs.dtsi} | 0
arch/arm/boot/dts/rk3288-veyron-jaq.dts | 2 +-
arch/arm/boot/dts/rk3288-veyron-jerry.dts | 2 +-
arch/arm/boot/dts/rk3288-veyron-pinky.dts | 2 +-
arch/arm/boot/dts/rk3288-veyron-speedy.dts | 2 +-
5 files changed, 4 insertions(+), 4 deletions(-)
rename arch/arm/boot/dts/{cros-ec-sbs.dtsi => rk3288-veyron-chromebook-sbs.dtsi} (100%)
diff --git a/arch/arm/boot/dts/cros-ec-sbs.dtsi b/arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi
similarity index 100%
rename from arch/arm/boot/dts/cros-ec-sbs.dtsi
rename to arch/arm/boot/dts/rk3288-veyron-chromebook-sbs.dtsi
diff --git a/arch/arm/boot/dts/rk3288-veyron-jaq.dts b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
index d33f5763c39c..f217a978e47a 100644
--- a/arch/arm/boot/dts/rk3288-veyron-jaq.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-jaq.dts
@@ -45,7 +45,7 @@
/dts-v1/;
#include "rk3288-veyron-chromebook.dtsi"
-#include "cros-ec-sbs.dtsi"
+#include "rk3288-veyron-chromebook-sbs.dtsi"
/ {
model = "Google Jaq";
diff --git a/arch/arm/boot/dts/rk3288-veyron-jerry.dts b/arch/arm/boot/dts/rk3288-veyron-jerry.dts
index cdea751f2a8c..bec607574165 100644
--- a/arch/arm/boot/dts/rk3288-veyron-jerry.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-jerry.dts
@@ -44,7 +44,7 @@
/dts-v1/;
#include "rk3288-veyron-chromebook.dtsi"
-#include "cros-ec-sbs.dtsi"
+#include "rk3288-veyron-chromebook-sbs.dtsi"
/ {
model = "Google Jerry";
diff --git a/arch/arm/boot/dts/rk3288-veyron-pinky.dts b/arch/arm/boot/dts/rk3288-veyron-pinky.dts
index 995cff42fa43..c81ad5bf1121 100644
--- a/arch/arm/boot/dts/rk3288-veyron-pinky.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-pinky.dts
@@ -44,7 +44,7 @@
/dts-v1/;
#include "rk3288-veyron-chromebook.dtsi"
-#include "cros-ec-sbs.dtsi"
+#include "rk3288-veyron-chromebook-sbs.dtsi"
/ {
model = "Google Pinky";
diff --git a/arch/arm/boot/dts/rk3288-veyron-speedy.dts b/arch/arm/boot/dts/rk3288-veyron-speedy.dts
index cc0b78cefe34..8aea9c3ff6e2 100644
--- a/arch/arm/boot/dts/rk3288-veyron-speedy.dts
+++ b/arch/arm/boot/dts/rk3288-veyron-speedy.dts
@@ -44,7 +44,7 @@
/dts-v1/;
#include "rk3288-veyron-chromebook.dtsi"
-#include "cros-ec-sbs.dtsi"
+#include "rk3288-veyron-chromebook-sbs.dtsi"
/ {
model = "Google Speedy";
--
2.12.2
^ permalink raw reply related
* Re: [PATCH 0/4] TI Bluetooth serdev support
From: Sebastian Reichel @ 2017-04-30 16:04 UTC (permalink / raw)
To: Adam Ford
Cc: Mark Rutland, Rob Herring, Johan Hedberg, devicetree,
Gustavo Padovan, Marcel Holtmann, Wei Xu, linux-bluetooth,
Eyal Reizer, netdev, Satish Patel, linux-arm-kernel
In-Reply-To: <CAHCN7xJUxZm1qKAxT0kaK6qoDg+HWOJK7sTH-q9za4HJuUwe8Q@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1160 bytes --]
Hi,
On Sun, Apr 30, 2017 at 10:14:20AM -0500, Adam Ford wrote:
> On Wed, Apr 5, 2017 at 1:30 PM, Rob Herring <robh@kernel.org> wrote:
> > This series adds serdev support to the HCI LL protocol used on TI BT
> > modules and enables support on HiKey board with with the WL1835 module.
> > With this the custom TI UIM daemon and btattach are no longer needed.
>
> Without UIM daemon, what instruction do you use to load the BT firmware?
>
> I was thinking 'hciattach' but I was having trouble. I was hoping you
> might have some insight.
>
> hciattach -t 30 -s 115200 /dev/ttymxc1 texas 3000000 flow Just
> returns a timeout.
>
> I modified my i.MX6 device tree per the binding documentation and
> setup the regulators and enable GPIO pins.
If you configured everything correctly no userspace interaction is
required. The driver should request the firmware automatically once
you power up the bluetooth device.
Apart from DT changes make sure, that the following options are
enabled and check dmesg for any hints.
CONFIG_SERIAL_DEV_BUS
CONFIG_SERIAL_DEV_CTRL_TTYPORT
CONFIG_BT_HCIUART
CONFIG_BT_HCIUART_LL
-- Sebastian
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCHv2 2/2] iio: adc: add driver for the ti-adc084s021 chip
From: Jonathan Cameron @ 2017-04-30 15:51 UTC (permalink / raw)
To: Mårten Lindahl
Cc: knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
pmeerw-jW+XmwGofnusTnJN9+BGXg, linux-iio-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mårten Lindahl
In-Reply-To: <1493556822-2601-3-git-send-email-marten.lindahl-VrBV9hrLPhE@public.gmane.org>
On 30/04/17 13:53, Mårten Lindahl wrote:
> From: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
>
> This adds support for the Texas Instruments ADC084S021 ADC chip.
>
> Signed-off-by: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
A few more bits inline. Mostly stuff that has come up
in the V2 changes (and the inevitable bits I missed the
first time!)
Jonathan
> ---
> Changes in v2:
> - Removed most #defines in favor of inlines
> - Corrected channel macro
> - Removed configuration array with only one item
> - Updated func adc084s021_adc_conversion to use be16_to_cpu
> - Added IIO_CHAN_INFO_SCALE to func adc084s021_read_raw
> - Use iio_device_claim_direct_mode in func adc084s021_read_raw
> - Removed documentation for standard driver functions
> - Changed retval to ret everywhere
> - Removed dynamic alloc for data buffer in trigger handler
> - Keeping mutex for all iterations in trigger handler
> - Removed usage of events in this driver
> - Removed info log in probe
> - Use spi_message_init_with_transfers for spi message structs
> - Use preenable and postdisable functions for regulator
> - Inserted blank line before last return in all functions
>
> drivers/iio/adc/Kconfig | 12 ++
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/ti-adc084s021.c | 294 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 307 insertions(+)
> create mode 100644 drivers/iio/adc/ti-adc084s021.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index dedae7a..13141e5 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -560,6 +560,18 @@ config TI_ADC0832
> This driver can also be built as a module. If so, the module will be
> called ti-adc0832.
>
> +config TI_ADC084S021
> + tristate "Texas Instruments ADC084S021"
> + depends on SPI
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + If you say yes here you get support for Texas Instruments ADC084S021
> + chips.
> +
> + This driver can also be built as a module. If so, the module will be
> + called ti-adc084s021.
> +
> config TI_ADC12138
> tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
> depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d001262..b1a6158 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -51,6 +51,7 @@ obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
> obj-$(CONFIG_STM32_ADC) += stm32-adc.o
> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
> obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
> +obj-$(CONFIG_TI_ADC084S021) += ti-adc084s021.o
> obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
> obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
> obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
> diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c
> new file mode 100644
> index 0000000..2dce257
> --- /dev/null
> +++ b/drivers/iio/adc/ti-adc084s021.c
> @@ -0,0 +1,294 @@
> +/**
> + * Copyright (C) 2017 Axis Communications AB
> + *
> + * Driver for Texas Instruments' ADC084S021 ADC chip.
> + * Datasheets can be found here:
> + * http://www.ti.com/lit/ds/symlink/adc084s021.pdf
> + *
> + * 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
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/spi/spi.h>
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define ADC084S021_DRIVER_NAME "adc084s021"
> +
> +struct adc084s021 {
> + struct spi_device *spi;
> + struct spi_message message;
> + struct spi_transfer spi_trans[2];
> + struct regulator *reg;
> + struct mutex lock;
> + /*
> + * DMA (thus cache coherency maintenance) requires the
> + * transfer buffers to live in their own cache lines.
> + */
> + union {
> + u16 tx_buf;
> + __be16 rx_buf;
> + } ____cacheline_aligned;
> +};
> +
> +/**
> + * Channel specification
> + */
Comment doesn't add much so I'd drop it.
> +#define ADC084S021_VOLTAGE_CHANNEL(num) \
> + { \
> + .type = IIO_VOLTAGE, \
> + .channel = (num), \
> + .address = (num) << 3, \
This does feel a little pointless as you can just do the shift inline and
use the channel value instead. Doesn't really matter though.
> + .indexed = 1, \
> + .scan_index = (num), \
> + .scan_type = { \
> + .sign = 'u', \
> + .realbits = 8, \
> + .storagebits = 16, \
> + .shift = 4, \
> + .endianness = IIO_BE, \
> + }, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
> + }
> +
> +static const struct iio_chan_spec adc084s021_channels[] = {
> + ADC084S021_VOLTAGE_CHANNEL(0),
> + ADC084S021_VOLTAGE_CHANNEL(1),
> + ADC084S021_VOLTAGE_CHANNEL(2),
> + ADC084S021_VOLTAGE_CHANNEL(3),
> + IIO_CHAN_SOFT_TIMESTAMP(4),
> +};
> +
> +/**
> + * Read an ADC channel and return its value.
> + *
> + * @adc: The ADC SPI data.
> + * @channel: The IIO channel data structure.
> + */
> +static int adc084s021_adc_conversion(struct adc084s021 *adc,
> + struct iio_chan_spec const *channel)
> +{
> + u8 value;
> + int ret;
> +
> + adc->tx_buf = channel->address;
> +
> + /* Do the transfer */
> + ret = spi_sync(adc->spi, &adc->message);
> + if (ret < 0)
> + return ret;
> +
> + value = (be16_to_cpu(adc->rx_buf) >> channel->scan_type.shift) & 0xff;You want to do this for the read_raw sysfs calls, but not the buffered ones
(where this shift and mask is made userspace's problem).
> +
> + dev_dbg(&adc->spi->dev, "value 0x%02X on channel %d\n",
> + value, channel->channel);
> +
> + return value;
> +}
> +
> +static int adc084s021_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *channel, int *val,
> + int *val2, long mask)
> +{
> + struct adc084s021 *adc = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret < 0)
> + return ret;
> +
Unless I'm dozing off this morning, you don't have any power..
So you'll need to enable the regulator first in this path.
Note that the runtime power management autosuspend stuff can
be good for this as it stops the power cycling if a short burst
of readings is taken.
> + ret = adc084s021_adc_conversion(adc, channel);
> + iio_device_release_direct_mode(indio_dev);
> + if (ret < 0)
> + return ret;
> +
> + *val = ret;
> +
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + ret = regulator_get_voltage(adc->reg);
Given the regulator is currently off as far as this device is concerned
it's possible the answer will be 0...
> + if (ret < 0)
> + return ret;
> +
> + *val = ret / 1000;
> +
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +/**
> + * Read enabled ADC channels and push data to the buffer.
> + *
> + * @irq: The interrupt number (not used).
> + * @pollfunc: Pointer to the poll func.
> + */
> +static irqreturn_t adc084s021_buffer_trigger_handler(int irq, void *pollfunc)
> +{
> + struct iio_poll_func *pf = pollfunc;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct adc084s021 *adc = iio_priv(indio_dev);
> + __be16 data[8] = {0}; /* 4 * 16-bit words of data + 8 bytes timestamp */
> + int scan_index;
> + int i = 0;
> +
> + mutex_lock(&adc->lock);
> +
> + for_each_set_bit(scan_index, indio_dev->active_scan_mask,
> + indio_dev->masklength) {
> + const struct iio_chan_spec *channel =
> + &indio_dev->channels[scan_index];
> + data[i++] = adc084s021_adc_conversion(adc, channel);
This is clearly a rather simplistic way of handling the read outs.
Perfectly good for an initial version (which may never get improved
on!) but you could do the spi transfers for a set of channels much
more efficiently.
Figure 1 on the datasheet shows how the control register for the next
read can be written in parallel with the previous read. That would
mean each scan took N + 1 2 byte transfers rather than 2N as currently.
I'm not particularly suggesting you do this, but thought I'd just
comment on the possibility as it is a common situation with spi
ADCs (sometimes you get a greater lag between setup and read out
making this even fiddlier!)
If you do want to do this, the trick is to do your transfer setup
and creation stuff in preenable so that it can be customised for
whatever channels are enabled.
> + }
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, data,
> + iio_get_time_ns(indio_dev));
> + mutex_unlock(&adc->lock);
> + iio_trigger_notify_done(indio_dev->trig);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int adc084s021_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + struct adc084s021 *adc = iio_priv(indio_dev);
> +
> + return regulator_enable(adc->reg);
> +}
> +
> +static int adc084s021_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> + struct adc084s021 *adc = iio_priv(indio_dev);
> +
> + return regulator_disable(adc->reg);
> +}
> +
> +static const struct iio_info adc084s021_info = {
> + .read_raw = adc084s021_read_raw,
> + .driver_module = THIS_MODULE,
> +};
> +
> +static const struct iio_buffer_setup_ops adc084s021_buffer_setup_ops = {
> + .preenable = adc084s021_buffer_preenable,
> + .postenable = iio_triggered_buffer_postenable,
> + .predisable = iio_triggered_buffer_predisable,
> + .postdisable = adc084s021_buffer_postdisable,
> +};
> +
> +static int adc084s021_probe(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev;
> + struct adc084s021 *adc;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
> + if (!indio_dev) {
> + dev_err(&spi->dev, "Failed to allocate IIO device\n");
> + return -ENOMEM;
> + }
> +
> + adc = iio_priv(indio_dev);
> + adc->spi = spi;
> + spi->bits_per_word = 8;
> +
> + /* Update the SPI device with config and connect the iio dev */
> + ret = spi_setup(spi);
> + if (ret) {
> + dev_err(&spi->dev, "Failed to update SPI device\n");
> + return ret;
> + }
> + spi_set_drvdata(spi, indio_dev);
> +
> + /* Initiate the Industrial I/O device */
> + indio_dev->dev.parent = &spi->dev;
> + indio_dev->dev.of_node = spi->dev.of_node;
> + indio_dev->name = spi_get_device_id(spi)->name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &adc084s021_info;
> + indio_dev->channels = adc084s021_channels;
> + indio_dev->num_channels = ARRAY_SIZE(adc084s021_channels);
> +
> + /* Create SPI transfer for channel reads */
> + adc->spi_trans[0].tx_buf = &adc->tx_buf;
> + adc->spi_trans[0].len = 2;
> + adc->spi_trans[0].speed_hz = spi->max_speed_hz;
You don't need to set these for individual transfers unless they
are different from how the spi bus has been set up...
It's handled in __spi_validate in drivers/spi/spi.c
list_for_each_entry(xfer, &message->transfers, transfer_list) {
message->frame_length += xfer->len;
if (!xfer->bits_per_word)
xfer->bits_per_word = spi->bits_per_word;
if (!xfer->speed_hz)
xfer->speed_hz = spi->max_speed_hz;
...
So it will set them spi->... in the core if you haven't overridden.
> + adc->spi_trans[0].bits_per_word = spi->bits_per_word;
> + adc->spi_trans[1].rx_buf = &adc->rx_buf;
> + adc->spi_trans[1].len = 2;
> + adc->spi_trans[1].speed_hz = spi->max_speed_hz;
> + adc->spi_trans[1].bits_per_word = spi->bits_per_word;
> +
> + spi_message_init_with_transfers(&adc->message, adc->spi_trans, 2);
> +
> + adc->reg = devm_regulator_get(&spi->dev, "vref");
> + if (IS_ERR(adc->reg))
> + return PTR_ERR(adc->reg);
> +
> + mutex_init(&adc->lock);
> +
> + /* Setup triggered buffer with pollfunction */
> + ret = iio_triggered_buffer_setup(indio_dev, NULL,
> + adc084s021_buffer_trigger_handler,
> + &adc084s021_buffer_setup_ops);
> + if (ret) {
> + dev_err(&spi->dev, "Failed to setup triggered buffer\n");
> + return ret;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + dev_err(&spi->dev, "Failed to register IIO device\n");
> + iio_triggered_buffer_cleanup(indio_dev);
With devm usage this won't be needed any more.
Hmm. We should improve the error message from the core as it'll allow us
to remove a lot of boiler plate in drivers. Ah well, a project for
another day.
> + }
> +
> + return ret;
> +}
> +
> +static int adc084s021_remove(struct spi_device *spi)
> +{
> + struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +
> + iio_device_unregister(indio_dev);
> + iio_triggered_buffer_cleanup(indio_dev);
Now you have moved to dynamically enabling the regulator, it makes
sense to use the devm_ versions of iio_device_register and
iio_triggered_buffer setup.
With those, you'll be able to get rid of the remove callback entirely as
there will be nothing to do.
> +
> + return 0;
> +}
> +
> +static const struct of_device_id adc084s021_of_match[] = {
> + { .compatible = "ti,adc084s021", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, adc084s021_of_match);
> +
> +static const struct spi_device_id adc084s021_id[] = {
> + { ADC084S021_DRIVER_NAME, 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(spi, adc084s021_id);
> +
> +static struct spi_driver adc084s021_driver = {
> + .driver = {
> + .name = ADC084S021_DRIVER_NAME,
> + .of_match_table = of_match_ptr(adc084s021_of_match),
> + },
> + .probe = adc084s021_probe,
> + .remove = adc084s021_remove,
> + .id_table = adc084s021_id,
> +};
> +module_spi_driver(adc084s021_driver);
> +
> +MODULE_AUTHOR("Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>");
> +MODULE_DESCRIPTION("Texas Instruments ADC084S021");
> +MODULE_LICENSE("GPL v2");
> +MODULE_VERSION("1.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
* Re: [PATCH 0/4] TI Bluetooth serdev support
From: Adam Ford @ 2017-04-30 15:14 UTC (permalink / raw)
To: Rob Herring
Cc: Marcel Holtmann, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Johan Hedberg,
Gustavo Padovan, Satish Patel, Wei Xu, Eyal Reizer,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20170405183005.20570-1-robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On Wed, Apr 5, 2017 at 1:30 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> This series adds serdev support to the HCI LL protocol used on TI BT
> modules and enables support on HiKey board with with the WL1835 module.
> With this the custom TI UIM daemon and btattach are no longer needed.
Without UIM daemon, what instruction do you use to load the BT firmware?
I was thinking 'hciattach' but I was having trouble. I was hoping you
might have some insight.
hciattach -t 30 -s 115200 /dev/ttymxc1 texas 3000000 flow Just
returns a timeout.
I modified my i.MX6 device tree per the binding documentation and
setup the regulators and enable GPIO pins.
adam
>
> The series is available on this git branch[1]. Patch 2 is just clean-up
> and can be applied independently. Patch 3 is dependent on the series
> "Nokia H4+ support". I'd suggest both series are merged thru the BT tree.
>
> Rob
>
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git ti-bluetooth
>
> Rob Herring (4):
> dt-bindings: net: Add TI WiLink shared transport binding
> bluetooth: hci_uart: remove unused hci_uart_init_tty
> bluetooth: hci_uart: add LL protocol serdev driver support
> arm64: dts: hikey: add WL1835 Bluetooth device node
>
> .../devicetree/bindings/net/ti,wilink-st.txt | 35 +++
> arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 5 +
> drivers/bluetooth/hci_ldisc.c | 19 --
> drivers/bluetooth/hci_ll.c | 261 ++++++++++++++++++++-
> drivers/bluetooth/hci_uart.h | 1 -
> 5 files changed, 300 insertions(+), 21 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/ti,wilink-st.txt
>
> --
> 2.10.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
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: [PATCH] iio: adc: add driver for the ti-adc084s021 chip
From: Jonathan Cameron @ 2017-04-30 15:13 UTC (permalink / raw)
To: Mårten Lindahl
Cc: Peter Meerwald-Stadler, Mårten Lindahl,
lars-Qo5EllUWu/uELgA04lAiVw, linux-iio-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493288121.21359.33.camel-VrBV9hrLPhE@public.gmane.org>
On 27/04/17 11:15, Mårten Lindahl wrote:
> On Sun, 2017-04-23 at 20:13 +0100, Jonathan Cameron wrote:
>> On 21/04/17 21:19, Peter Meerwald-Stadler wrote:
>>>
>>>> From: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
>>>
>>> comments below
>> A few more from me. Laptop battery gone flat so not so thorough on top few lines!
>>
>> Jonathan
>
> Hi, and thanks for your comments! I have a new patch where I resolved it
> all. But before I send it I wonder about your comments about the events.
> Please see below.
>
> Thanks,
> Mårten
>>>
>>>> This adds support for the Texas Instruments ADC084S021 ADC chip.
>>>>
>>>> Signed-off-by: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
>>>> ---
>>>> .../devicetree/bindings/iio/adc/ti-adc084s021.txt | 25 ++
>>>> drivers/iio/adc/Kconfig | 12 +
>>>> drivers/iio/adc/Makefile | 1 +
>>>> drivers/iio/adc/ti-adc084s021.c | 342 +++++++++++++++++++++
>>>> 4 files changed, 380 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
>>>> create mode 100644 drivers/iio/adc/ti-adc084s021.c
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
>>>> new file mode 100644
>>>> index 0000000..921eb46
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
>>>> @@ -0,0 +1,25 @@
>>>> +* Texas Instruments' ADC084S021
>>>> +
>>>> +Required properties:
>>>> + - compatible : Must be "ti,adc084s021"
>>>> + - reg : SPI chip select number for the device
>>>> + - vref-supply : The regulator supply for ADC reference voltage
>>>> + - spi-max-frequency : Definition as per Documentation/devicetree/bindings/spi/spi-bus.txt
>>>> +
>>>> +Optional properties:
>>>> + - spi-cpol : SPI inverse clock polarity, as per spi-bus bindings
>>>> + - spi-cpha : SPI shifted clock phase (CPHA), as per spi-bus bindings
>>>> + - spi-cs-high : SPI chip select active high, as per spi-bus bindings
>>>> +
>>>> +
>>>> +Example:
>>>> +adc@0 {
>>>> + compatible = "ti,adc084s021";
>>>> + reg = <0>;
>>>> + vref-supply = <&adc_vref>;
>>>> + spi-cpol;
>>>> + spi-cpha;
>>>> + spi-cs-high;
>>>> + spi-max-frequency = <16000000>;
>>>> + pl022,com-mode = <0x2>; /* DMA */
>>>
>>> what is this?
>>>
>>>> +};
>>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>>> index dedae7a..13141e5 100644
>>>> --- a/drivers/iio/adc/Kconfig
>>>> +++ b/drivers/iio/adc/Kconfig
>>>> @@ -560,6 +560,18 @@ config TI_ADC0832
>>>> This driver can also be built as a module. If so, the module will be
>>>> called ti-adc0832.
>>>>
>>>> +config TI_ADC084S021
>>>> + tristate "Texas Instruments ADC084S021"
>>>> + depends on SPI
>>>> + select IIO_BUFFER
>>>> + select IIO_TRIGGERED_BUFFER
>>>> + help
>>>> + If you say yes here you get support for Texas Instruments ADC084S021
>>>> + chips.
>>>> +
>>>> + This driver can also be built as a module. If so, the module will be
>>>> + called ti-adc084s021.
>>>> +
>>>> config TI_ADC12138
>>>> tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
>>>> depends on SPI
>>>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>>>> index d001262..b1a6158 100644
>>>> --- a/drivers/iio/adc/Makefile
>>>> +++ b/drivers/iio/adc/Makefile
>>>> @@ -51,6 +51,7 @@ obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
>>>> obj-$(CONFIG_STM32_ADC) += stm32-adc.o
>>>> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>>>> obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
>>>> +obj-$(CONFIG_TI_ADC084S021) += ti-adc084s021.o
>>>> obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
>>>> obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
>>>> obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
>>>> diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c
>>>> new file mode 100644
>>>> index 0000000..4f33b91
>>>> --- /dev/null
>>>> +++ b/drivers/iio/adc/ti-adc084s021.c
>>>> @@ -0,0 +1,342 @@
>>>> +/**
>>>> + * Copyright (C) 2017 Axis Communications AB
>>>> + *
>>>> + * Driver for Texas Instruments' ADC084S021 ADC chip.
>>>> + * Datasheets can be found here:
>>>> + * http://www.ti.com/lit/ds/symlink/adc084s021.pdf
>>>> + *
>>>> + * 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
>>>> + * published by the Free Software Foundation.
>>>> + */
>>>> +
>>>> +#include <linux/err.h>
>>>> +#include <linux/spi/spi.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/interrupt.h>
>>>> +#include <linux/iio/iio.h>
>>>> +#include <linux/iio/buffer.h>
>>>> +#include <linux/iio/events.h>
>>>> +#include <linux/iio/triggered_buffer.h>
>>>> +#include <linux/iio/trigger_consumer.h>
>>>> +#include <linux/regulator/consumer.h>
>>>> +
>>>> +#define MODULE_NAME "adc084s021"
>>>> +#define DRIVER_VERSION "1.0"
>>>
>>> is only used once at the very end...
>>>
>>>> +#define ADC_RESOLUTION 8
>> Put it inline...
>>>> +#define ADC_N_CHANNELS 4
>> Belongs inline. No additional info provided by having it here.
>>>
>>> we want a consistent prefix, such as ADC084S021_
>>>
>>>> +
>>>> +struct adc084s021_configuration {
>>>> + const struct iio_chan_spec *channels;
>>>> + u8 num_channels;
>>>
>>> no need for u8, perhaps unsigned?
>>>
>>>> +};
>>>> +
>>>> +struct adc084s021 {
>>>> + struct spi_device *spi;
>>>> + struct spi_message message;
>>>> + struct spi_transfer spi_trans[2];
>>>> + struct regulator *reg;
>>>> + struct mutex lock;
>>>> + /*
>>>> + * DMA (thus cache coherency maintenance) requires the
>>>> + * transfer buffers to live in their own cache lines.
>>>> + */
>>>> + union {
>>>> + u8 tx_buf[2];
>>>> + u8 rx_buf[2];
>>>> + } ____cacheline_aligned;
>>>> + u8 cur_adc_values[ADC_N_CHANNELS];
>>>> +};
>>>> +
>>>> +/**
>>>> + * Event triggered when value changes on a channel
>>>> + */
>>>> +static const struct iio_event_spec adc084s021_event = {
>>>> + .type = IIO_EV_TYPE_CHANGE,
>>>> + .dir = IIO_EV_DIR_NONE,
>>>> +};
>> Not the intent of that type of event at all.
>>>> +
>>>> +/**
>>>> + * Channel specification
>>>> + */
>>>> +#define ADC084S021_VOLTAGE_CHANNEL(num) \
>>>> + { \
>>>> + .type = IIO_VOLTAGE, \
>>>> + .channel = (num), \
>>>> + .address = (num << 3), \
>>>
>>> parenthesis should be around (num)
>>>
>>>> + .indexed = 1, \
>>>> + .scan_index = num, \
>>>
>>> parenthesis?
>>>
>>>> + .scan_type = { \
>>>> + .sign = 'u', \
>>>> + .realbits = 8, \
>>>> + .storagebits = 32, \
>>>> + .shift = 24 - ((num << 3)), \
>>>
>>> no need for (( )) around the expression, parenthesis should be around num
>>>
>>> the shift doesn't make sense, you are shifting in
>>>
>>> adc084s021_adc_conversion() already and return just 8 bits (so storagebits
>>> should be 8)?
>>>
>>> you could/should use realbits = 8, storagebits = 16, shift = 4 and
>>> endianness = IIO_BE if I read Figure 1 of the datasheet correctly
>>>
>>>> + }, \
>>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
>>>
>>> likely this is missing _SCALE
>>> IIO expects data to be returned in millivolt, see
>>> Documentation/ABI/testing/sysfs-bus-iio
>>>
>>>> + .event_spec = &adc084s021_event, \
>>>> + .num_event_specs = 1, \
>>>
>>> ARRAY_SIZE(&adc084s021_event) to be extensible?
>>>
>>>> + }
>>>> +
>>>> +static const struct iio_chan_spec adc084s021_channels[] = {
>>>> + ADC084S021_VOLTAGE_CHANNEL(0),
>>>> + ADC084S021_VOLTAGE_CHANNEL(1),
>>>> + ADC084S021_VOLTAGE_CHANNEL(2),
>>>> + ADC084S021_VOLTAGE_CHANNEL(3),
>>>> + IIO_CHAN_SOFT_TIMESTAMP(4),
>>>> +};
>>>> +
>>>> +static const struct adc084s021_configuration adc084s021_config[] = {
>>>> + { adc084s021_channels, ARRAY_SIZE(adc084s021_channels) },
>>>
>>> for just one configuration, this is not really needed; so you plan/forsee
>>> more chips being added soonish?
>>>
>>>> +};
>>>> +
>>>> +/**
>>>> + * Read an ADC channel and return its value.
>>>> + *
>>>> + * @adc: The ADC SPI data.
>>>> + * @channel: The IIO channel data structure.
>>>> + */
>>>> +static int adc084s021_adc_conversion(struct adc084s021 *adc,
>>>> + struct iio_chan_spec const *channel)
>>>> +{
>>>> + u16 value;
>>>
>>> value should be u8, but is not really needed
>>>
>>>> + int ret;
>>>> +
>>>> + mutex_lock(&adc->lock);
>>>> + adc->tx_buf[0] = channel->address;
>>>> +
>>>> + /* Do the transfer */
>>>> + ret = spi_sync(adc->spi, &adc->message);
>>>> +
>>>
>>> no newline here please
>>>
>>>> + if (ret < 0) {
>>>> + mutex_unlock(&adc->lock);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + value = (adc->rx_buf[0] << 4) | (adc->rx_buf[1] >> 4);
>>>
>>> I recommend using __be16 for rx_buf and
>>> ret = (be16_to_cpu(adc->rx_buf) >> 4) & 0xff;
>>>
>>>> + mutex_unlock(&adc->lock);
>>>> +
>>>> + dev_dbg(&adc->spi->dev, "value 0x%02X on channel %d\n",
>>>> + value, channel->channel);
>>>> + return value;
>>>> +}
>>>> +
>>>> +/**
>>>> + * Make a readout of requested IIO channel info.
>>>
>>> no need to document this, it is found in every IIO driver...
>>>
>>>> + *
>>>> + * @indio_dev: The industrial I/O device.
>>>> + * @channel: The IIO channel data structure.
>>>> + * @val: First element of value (integer).
>>>> + * @val2: Second element of value (fractional).
>>>> + * @mask: The info_mask to read.
>>>> + */
>>>> +static int adc084s021_read_raw(struct iio_dev *indio_dev,
>>>> + struct iio_chan_spec const *channel, int *val,
>>>> + int *val2, long mask)
>>>> +{
>>>> + struct adc084s021 *adc = iio_priv(indio_dev);
>>>> + int retval;
>>>
>>> how about using ret everywhere?
>> Agreed.
>>>
>>>> +
>>>> + switch (mask) {
>>>> + case IIO_CHAN_INFO_RAW:
>>>
>>> probably want iio_device_claim_direct_mode() so to not interfere with
>>> buffered accessBorderline case as all you will do is queue up additional spi transfers.
>> At least after you have dropped the unusual events stuff.
>>
>> Arguably this will mean sysfs reads could make the buffered data flow
>> less deterministic though so maybe on claiming direct mode (which
>> prevents it running concurrently with buffered capture.
>>>
>>>> + retval = adc084s021_adc_conversion(adc, channel);
>>>> + if (retval < 0)
>>>> + return retval;
>>>> +
>>>> + *val = retval;
>>>> + return IIO_VAL_INT;
>>>> +
>>>> + default:
>>>> + return -EINVAL;
>>>> + }
>>>> +}
>>>> +
>>>> +/**
>>>> + * Read enabled ADC channels and push data to the buffer.
>>>> + *
>>>> + * @irq: The interrupt number (not used).
>>>> + * @pollfunc: Pointer to the poll func.
>>>> + */
>>>> +static irqreturn_t adc084s021_trigger_handler(int irq, void *pollfunc)
>>>> +{
>>>> + struct iio_poll_func *pf = pollfunc;
>>>> + struct iio_dev *indio_dev = pf->indio_dev;
>>>> + struct adc084s021 *adc = iio_priv(indio_dev);
>>>> + u8 *data;
>>>> + s64 timestamp;
>>>> + int value, scan_index;
>>>> +
>>>> + data = kzalloc(indio_dev->scan_bytes, GFP_KERNEL);
>>>
>>> pre-allocate buffer with maximum size statically; allocation is
>>> potentially expensive; don't forget space for the timestamp and padding...
>>>
>>>> + if (!data) {
>>>> + iio_trigger_notify_done(indio_dev->trig);
>>>> + return IRQ_NONE;
>>>> + }
>>>> +
>>>> + timestamp = iio_get_time_ns(indio_dev);
>>>> +
>>>> + for_each_set_bit(scan_index, indio_dev->active_scan_mask,
>>>> + indio_dev->masklength) {
>>>> + const struct iio_chan_spec *channel =
>>>> + &indio_dev->channels[scan_index];
>>>> + value = adc084s021_adc_conversion(adc, channel);
>>>
>>> lock is taken and released for each channel, probably want to do it just
>>> once?
>>>
>>>> + data[scan_index] = value;
>>>> +
>>>> + /*
>>>> + * Compare read data to previous read. If it differs send
>>>> + * event notification for affected channel.
>>>> + */
>>>> + if (adc->cur_adc_values[scan_index] != (u8)value) {
>>>
>>> cur_adc_values is not initialized (probably set to 0);
>>> so on first read, should the notification be sent?
>> ? This is 'unusual' to say the least. Why the events given the data
>> is available via the buffer. If you really want to do this stuff, it
>> ought to be in userspace.
>>
>> Are you trying to emulate the filtering input does?
>
> The buffer is continuously updated with readings that may have the same
> data for many iterations, so my idea was to send an event with timestamp
> so that whenever some "new" (changed) data is written there it can be
> more easily located in the buffer by the consumer, by matching event
> timestamp with buffer timestamp. Is there another way to do this or
> should the buffer readings be continuously analysed by another module or
> application?
>
> When prototyping this driver I even tweaked the IIO_EVENT_CODE to
> include the new value in the event signal itself. But I wasn't brave
> enough to show it here :) Please tell me, am I totally wrong by even
> trying that idea?
It's an interesting thought, but I wouldn't do it the way you have.
If this makes sense it ought to be a core facility rather than implemented
in every driver. The obvious method would be something like:
1) A new consumer device that does the filtering.
2) Configfs based interface to attach it to an existing driver.
Now this falls into the area where a hard coded set of filters to
create the events may not be the way to go (not flexible enough).
Possibly we'd be looking at ebpf programs loaded into the kernel to
do this (it's quite similar in a way to network packet filtering).
I'd also be tempted to not do it as events, but rather via a buffer
that just contains data when the values have changed.
Anyhow, the one place it doesn't belong is in an initially submission
of a new driver as it complicates matters and ties together generic
infrastructure with a particular driver submission.
So pull it out for now.
Jonathan
>
>>>
>>>> + adc->cur_adc_values[scan_index] = (u8)value;
>>>> + iio_push_event(indio_dev,
>>>> + IIO_EVENT_CODE(IIO_VOLTAGE, 0,
>>>> + IIO_NO_MOD, IIO_EV_DIR_NONE,
>>>> + IIO_EV_TYPE_CHANGE,
>>>> + channel->channel, 0, 0),
>>>> + timestamp);
>>>> + dev_dbg(&indio_dev->dev,
>>>> + "new value on ch%d: 0x%02X (ts %llu)\n",
>>>> + channel->channel, value, timestamp);
>>>> + }
>>>> + }
>>>> +
>>>> + iio_push_to_buffers_with_timestamp(indio_dev, data, timestamp);
>>>> + iio_trigger_notify_done(indio_dev->trig);
>>>> + kfree(data);
>> blank line here please.
>>>> + return IRQ_HANDLED;
>>>> +}
>>>> +
>>>> +static const struct iio_info adc084s021_info = {
>>>> + .read_raw = adc084s021_read_raw,
>>>> + .driver_module = THIS_MODULE,
>>>> +};
>>>> +
>>>> +/**
>>>> + * Create and register ADC IIO device for SPI.
>>>
>>> comment is rather pointless
>>>
>>>> + */
>>>> +static int adc084s021_probe(struct spi_device *spi)
>>>> +{
>>>> + struct iio_dev *indio_dev;
>>>> + struct adc084s021 *adc;
>>>> + int config = spi_get_device_id(spi)->driver_data;
>>>> + int retval;
>>>
>>> ret maybe
>>>
>>>> +
>>>> + /* Allocate an Industrial I/O device */
>>>
>>> obviously...
>> :) Yeah, clear out any comments that don't tell us much.
>>>
>>>> + indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
>>>> + if (!indio_dev) {
>>>> + dev_err(&spi->dev, "Failed to allocate IIO device\n");
>>>> + return -ENOMEM;
>>>> + }
>>>> +
>>>> + adc = iio_priv(indio_dev);
>>>> + adc->spi = spi;
>>>> + spi->bits_per_word = ADC_RESOLUTION;
>> This surprised me somewhat as I was expecting an odd value for this
>> (and was going to ask if standard power of 2 options would work).
>> If it had been inline, this would have required slightly less review
>> time which I always like (particularly when crammed into an airline seat!)
>>>> +
>>>> + /* Update the SPI device with config and connect the iio dev */
>>>> + retval = spi_setup(spi);
>>>> + if (retval) {
>>>> + dev_err(&spi->dev, "Failed to update SPI device\n");
>>>> + return retval;
>>>> + }
>>>> + spi_set_drvdata(spi, indio_dev);
>>>> +
>>>> + /* Initiate the Industrial I/O device */
>> :>> + indio_dev->dev.parent = &spi->dev;
>>>> + indio_dev->dev.of_node = spi->dev.of_node;
>>>> + indio_dev->name = spi_get_device_id(spi)->name;
>>>> + indio_dev->modes = INDIO_DIRECT_MODE;
>>>> + indio_dev->info = &adc084s021_info;
>>>> + indio_dev->channels = adc084s021_config[config].channels;
>>>> + indio_dev->num_channels = adc084s021_config[config].num_channels;
>>>
>>> could do it directly as long there is just one configuration
>> That would certainly be preferable unless V2 is going to show up
>> with multiple options!
>>>
>>>> +
>>>> + /* Create SPI transfer for channel reads */
>>>> + adc->spi_trans[0].tx_buf = &adc->tx_buf[0];
>>>> + adc->spi_trans[0].len = 2;
>>>> + adc->spi_trans[0].speed_hz = spi->max_speed_hz;
>>>> + adc->spi_trans[0].bits_per_word = spi->bits_per_word;
>>>> + adc->spi_trans[1].rx_buf = &adc->rx_buf[0];
>>>> + adc->spi_trans[1].len = 2;
>>>> + adc->spi_trans[1].speed_hz = spi->max_speed_hz;
>>>> + adc->spi_trans[1].bits_per_word = spi->bits_per_word;
>>>> +
>>>> + /* Setup SPI message for channel reads */
>>>> + spi_message_init(&adc->message);
>>>> + spi_message_add_tail(&adc->spi_trans[0], &adc->message);
>>>> + spi_message_add_tail(&adc->spi_trans[1], &adc->message);
>> spi_init_with_transfers to save a bit of boilerplate.
>>>> +
>>>> + adc->reg = devm_regulator_get(&spi->dev, "vref");
>>>> + if (IS_ERR(adc->reg))
>>>> + return PTR_ERR(adc->reg);
>>>> +
>>>> + retval = regulator_enable(adc->reg);
>>>> + if (retval < 0)
>>>> + return retval;
>> Given we either have slow sysfs accesses or know we have buffered
>> access on going. Have you considered enabling and disabling
>> the regulator dynamically? The fact that this often makes sense is
>> why Mark and co from the regulators side of things haven't provided
>> a devm_regulator_enable... You would need a preenable and postdisable
>> to make sure it was on for buffered access.
>>>> +
>>>> + mutex_init(&adc->lock);
>>>> +
>>>> + /* Setup triggered buffer with pollfunction */
>>>> + retval = iio_triggered_buffer_setup(indio_dev, NULL,
>>>
>>> devm_()
>> I disagree. It would change the ordering wrt to the regulator_enable.
>> Now, obviously it won't actually matter, but it will make the code
>> less obviously correct and for the small burden of extra unwind
>> code I'd keep using the non devm version.
>>>
>>>> + adc084s021_trigger_handler, NULL);
>>>> + if (retval) {
>>>> + dev_err(&spi->dev, "Failed to setup triggered buffer\n");
>>>> + goto buffer_setup_failed;
>>>> + }
>>>> +
>>>> + retval = iio_device_register(indio_dev);
>>>> + if (retval) {
>>>> + dev_err(&spi->dev, "Failed to register IIO device\n");
>> Hmm. If we were to flesh out some error messages for the few
>> cases where there is no info provided in iio_device_register we could
>> probably clean out a fair bit of boiler plate reporting in drivers.
>> Ah well, one for the future!
>>>> + goto device_register_failed;
>>>> + }
>>>> +
>>>> + dev_info(&spi->dev, "probed!\n");
>>>
>>> no logging please, just outputs clutter
>>>
>>>> + return 0;
>>>> +
>>>> +device_register_failed:
>>>> + iio_triggered_buffer_cleanup(indio_dev);
>>>> +buffer_setup_failed:
>>>> + regulator_disable(adc->reg);
>>>> + return retval;
>>>> +}
>>>> +
>>>> +/**
>>>> + * Unregister ADC IIO device for SPI.
>>>> + */
>>>> +static int adc084s021_remove(struct spi_device *spi)
>>>> +{
>>>> + struct iio_dev *indio_dev = spi_get_drvdata(spi);
>>>> + struct adc084s021 *adc = iio_priv(indio_dev);
>>>> +
>>>> + iio_device_unregister(indio_dev);
>>>> + iio_triggered_buffer_cleanup(indio_dev);
>>>> + regulator_disable(adc->reg);
>> blank line here preferred (slightly!)
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static const struct of_device_id adc084s021_of_match[] = {
>>>> + { .compatible = "ti,adc084s021", },
>>>> + {},
>>>> +};
>>>> +
>>>> +MODULE_DEVICE_TABLE(of, adc084s021_of_match);
>>>> +
>>>> +static const struct spi_device_id adc084s021_id[] = {
>>>> + { MODULE_NAME, 0},
>>>> + {}
>>>> +};
>>>> +
>>>> +MODULE_DEVICE_TABLE(spi, adc084s021_id);
>>>> +
>>>> +static struct spi_driver adc084s021_driver = {
>>>> + .driver = {
>>>> + .name = MODULE_NAME,
>>>> + .of_match_table = of_match_ptr(adc084s021_of_match),
>>>> + },
>>>> + .probe = adc084s021_probe,
>>>> + .remove = adc084s021_remove,
>>>> + .id_table = adc084s021_id,
>>>> +};
>>>> +
>> Convention often says to not bother with a blank line here or before
>> the MODULE_DEVICE_TABLE above. Gives a visual indication that these macros
>> are being passed the structures.
>> (really minor point!)
>>>> +module_spi_driver(adc084s021_driver);
>>>> +
>>>> +MODULE_AUTHOR("Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>");
>>>> +MODULE_DESCRIPTION("Texas Instruments ADC084S021");
>>>> +MODULE_LICENSE("GPL v2");
>>>> +MODULE_VERSION(DRIVER_VERSION);
>>>>
>>>
>>
>
>
--
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
* [PATCHv2 2/2] iio: adc: add driver for the ti-adc084s021 chip
From: Mårten Lindahl @ 2017-04-30 12:53 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
pmeerw-jW+XmwGofnusTnJN9+BGXg, linux-iio-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mårten Lindahl
In-Reply-To: <1493556822-2601-1-git-send-email-marten.lindahl-VrBV9hrLPhE@public.gmane.org>
From: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
This adds support for the Texas Instruments ADC084S021 ADC chip.
Signed-off-by: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
---
Changes in v2:
- Removed most #defines in favor of inlines
- Corrected channel macro
- Removed configuration array with only one item
- Updated func adc084s021_adc_conversion to use be16_to_cpu
- Added IIO_CHAN_INFO_SCALE to func adc084s021_read_raw
- Use iio_device_claim_direct_mode in func adc084s021_read_raw
- Removed documentation for standard driver functions
- Changed retval to ret everywhere
- Removed dynamic alloc for data buffer in trigger handler
- Keeping mutex for all iterations in trigger handler
- Removed usage of events in this driver
- Removed info log in probe
- Use spi_message_init_with_transfers for spi message structs
- Use preenable and postdisable functions for regulator
- Inserted blank line before last return in all functions
drivers/iio/adc/Kconfig | 12 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ti-adc084s021.c | 294 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 307 insertions(+)
create mode 100644 drivers/iio/adc/ti-adc084s021.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index dedae7a..13141e5 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -560,6 +560,18 @@ config TI_ADC0832
This driver can also be built as a module. If so, the module will be
called ti-adc0832.
+config TI_ADC084S021
+ tristate "Texas Instruments ADC084S021"
+ depends on SPI
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+ help
+ If you say yes here you get support for Texas Instruments ADC084S021
+ chips.
+
+ This driver can also be built as a module. If so, the module will be
+ called ti-adc084s021.
+
config TI_ADC12138
tristate "Texas Instruments ADC12130/ADC12132/ADC12138"
depends on SPI
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d001262..b1a6158 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
obj-$(CONFIG_STM32_ADC) += stm32-adc.o
obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
+obj-$(CONFIG_TI_ADC084S021) += ti-adc084s021.o
obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c
new file mode 100644
index 0000000..2dce257
--- /dev/null
+++ b/drivers/iio/adc/ti-adc084s021.c
@@ -0,0 +1,294 @@
+/**
+ * Copyright (C) 2017 Axis Communications AB
+ *
+ * Driver for Texas Instruments' ADC084S021 ADC chip.
+ * Datasheets can be found here:
+ * http://www.ti.com/lit/ds/symlink/adc084s021.pdf
+ *
+ * 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
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/err.h>
+#include <linux/spi/spi.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/regulator/consumer.h>
+
+#define ADC084S021_DRIVER_NAME "adc084s021"
+
+struct adc084s021 {
+ struct spi_device *spi;
+ struct spi_message message;
+ struct spi_transfer spi_trans[2];
+ struct regulator *reg;
+ struct mutex lock;
+ /*
+ * DMA (thus cache coherency maintenance) requires the
+ * transfer buffers to live in their own cache lines.
+ */
+ union {
+ u16 tx_buf;
+ __be16 rx_buf;
+ } ____cacheline_aligned;
+};
+
+/**
+ * Channel specification
+ */
+#define ADC084S021_VOLTAGE_CHANNEL(num) \
+ { \
+ .type = IIO_VOLTAGE, \
+ .channel = (num), \
+ .address = (num) << 3, \
+ .indexed = 1, \
+ .scan_index = (num), \
+ .scan_type = { \
+ .sign = 'u', \
+ .realbits = 8, \
+ .storagebits = 16, \
+ .shift = 4, \
+ .endianness = IIO_BE, \
+ }, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
+ }
+
+static const struct iio_chan_spec adc084s021_channels[] = {
+ ADC084S021_VOLTAGE_CHANNEL(0),
+ ADC084S021_VOLTAGE_CHANNEL(1),
+ ADC084S021_VOLTAGE_CHANNEL(2),
+ ADC084S021_VOLTAGE_CHANNEL(3),
+ IIO_CHAN_SOFT_TIMESTAMP(4),
+};
+
+/**
+ * Read an ADC channel and return its value.
+ *
+ * @adc: The ADC SPI data.
+ * @channel: The IIO channel data structure.
+ */
+static int adc084s021_adc_conversion(struct adc084s021 *adc,
+ struct iio_chan_spec const *channel)
+{
+ u8 value;
+ int ret;
+
+ adc->tx_buf = channel->address;
+
+ /* Do the transfer */
+ ret = spi_sync(adc->spi, &adc->message);
+ if (ret < 0)
+ return ret;
+
+ value = (be16_to_cpu(adc->rx_buf) >> channel->scan_type.shift) & 0xff;
+
+ dev_dbg(&adc->spi->dev, "value 0x%02X on channel %d\n",
+ value, channel->channel);
+
+ return value;
+}
+
+static int adc084s021_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *channel, int *val,
+ int *val2, long mask)
+{
+ struct adc084s021 *adc = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret < 0)
+ return ret;
+
+ ret = adc084s021_adc_conversion(adc, channel);
+ iio_device_release_direct_mode(indio_dev);
+ if (ret < 0)
+ return ret;
+
+ *val = ret;
+
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ ret = regulator_get_voltage(adc->reg);
+ if (ret < 0)
+ return ret;
+
+ *val = ret / 1000;
+
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+/**
+ * Read enabled ADC channels and push data to the buffer.
+ *
+ * @irq: The interrupt number (not used).
+ * @pollfunc: Pointer to the poll func.
+ */
+static irqreturn_t adc084s021_buffer_trigger_handler(int irq, void *pollfunc)
+{
+ struct iio_poll_func *pf = pollfunc;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct adc084s021 *adc = iio_priv(indio_dev);
+ __be16 data[8] = {0}; /* 4 * 16-bit words of data + 8 bytes timestamp */
+ int scan_index;
+ int i = 0;
+
+ mutex_lock(&adc->lock);
+
+ for_each_set_bit(scan_index, indio_dev->active_scan_mask,
+ indio_dev->masklength) {
+ const struct iio_chan_spec *channel =
+ &indio_dev->channels[scan_index];
+ data[i++] = adc084s021_adc_conversion(adc, channel);
+ }
+
+ iio_push_to_buffers_with_timestamp(indio_dev, data,
+ iio_get_time_ns(indio_dev));
+ mutex_unlock(&adc->lock);
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
+static int adc084s021_buffer_preenable(struct iio_dev *indio_dev)
+{
+ struct adc084s021 *adc = iio_priv(indio_dev);
+
+ return regulator_enable(adc->reg);
+}
+
+static int adc084s021_buffer_postdisable(struct iio_dev *indio_dev)
+{
+ struct adc084s021 *adc = iio_priv(indio_dev);
+
+ return regulator_disable(adc->reg);
+}
+
+static const struct iio_info adc084s021_info = {
+ .read_raw = adc084s021_read_raw,
+ .driver_module = THIS_MODULE,
+};
+
+static const struct iio_buffer_setup_ops adc084s021_buffer_setup_ops = {
+ .preenable = adc084s021_buffer_preenable,
+ .postenable = iio_triggered_buffer_postenable,
+ .predisable = iio_triggered_buffer_predisable,
+ .postdisable = adc084s021_buffer_postdisable,
+};
+
+static int adc084s021_probe(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev;
+ struct adc084s021 *adc;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*adc));
+ if (!indio_dev) {
+ dev_err(&spi->dev, "Failed to allocate IIO device\n");
+ return -ENOMEM;
+ }
+
+ adc = iio_priv(indio_dev);
+ adc->spi = spi;
+ spi->bits_per_word = 8;
+
+ /* Update the SPI device with config and connect the iio dev */
+ ret = spi_setup(spi);
+ if (ret) {
+ dev_err(&spi->dev, "Failed to update SPI device\n");
+ return ret;
+ }
+ spi_set_drvdata(spi, indio_dev);
+
+ /* Initiate the Industrial I/O device */
+ indio_dev->dev.parent = &spi->dev;
+ indio_dev->dev.of_node = spi->dev.of_node;
+ indio_dev->name = spi_get_device_id(spi)->name;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->info = &adc084s021_info;
+ indio_dev->channels = adc084s021_channels;
+ indio_dev->num_channels = ARRAY_SIZE(adc084s021_channels);
+
+ /* Create SPI transfer for channel reads */
+ adc->spi_trans[0].tx_buf = &adc->tx_buf;
+ adc->spi_trans[0].len = 2;
+ adc->spi_trans[0].speed_hz = spi->max_speed_hz;
+ adc->spi_trans[0].bits_per_word = spi->bits_per_word;
+ adc->spi_trans[1].rx_buf = &adc->rx_buf;
+ adc->spi_trans[1].len = 2;
+ adc->spi_trans[1].speed_hz = spi->max_speed_hz;
+ adc->spi_trans[1].bits_per_word = spi->bits_per_word;
+
+ spi_message_init_with_transfers(&adc->message, adc->spi_trans, 2);
+
+ adc->reg = devm_regulator_get(&spi->dev, "vref");
+ if (IS_ERR(adc->reg))
+ return PTR_ERR(adc->reg);
+
+ mutex_init(&adc->lock);
+
+ /* Setup triggered buffer with pollfunction */
+ ret = iio_triggered_buffer_setup(indio_dev, NULL,
+ adc084s021_buffer_trigger_handler,
+ &adc084s021_buffer_setup_ops);
+ if (ret) {
+ dev_err(&spi->dev, "Failed to setup triggered buffer\n");
+ return ret;
+ }
+
+ ret = iio_device_register(indio_dev);
+ if (ret) {
+ dev_err(&spi->dev, "Failed to register IIO device\n");
+ iio_triggered_buffer_cleanup(indio_dev);
+ }
+
+ return ret;
+}
+
+static int adc084s021_remove(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev = spi_get_drvdata(spi);
+
+ iio_device_unregister(indio_dev);
+ iio_triggered_buffer_cleanup(indio_dev);
+
+ return 0;
+}
+
+static const struct of_device_id adc084s021_of_match[] = {
+ { .compatible = "ti,adc084s021", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, adc084s021_of_match);
+
+static const struct spi_device_id adc084s021_id[] = {
+ { ADC084S021_DRIVER_NAME, 0},
+ {}
+};
+MODULE_DEVICE_TABLE(spi, adc084s021_id);
+
+static struct spi_driver adc084s021_driver = {
+ .driver = {
+ .name = ADC084S021_DRIVER_NAME,
+ .of_match_table = of_match_ptr(adc084s021_of_match),
+ },
+ .probe = adc084s021_probe,
+ .remove = adc084s021_remove,
+ .id_table = adc084s021_id,
+};
+module_spi_driver(adc084s021_driver);
+
+MODULE_AUTHOR("Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>");
+MODULE_DESCRIPTION("Texas Instruments ADC084S021");
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION("1.0");
--
2.1.4
^ permalink raw reply related
* [PATCHv2 1/2] dt-bindings: iio: adc: add driver for the ti-adc084s021 chip
From: Mårten Lindahl @ 2017-04-30 12:53 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
pmeerw-jW+XmwGofnusTnJN9+BGXg, linux-iio-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mårten Lindahl
In-Reply-To: <1493556822-2601-1-git-send-email-marten.lindahl-VrBV9hrLPhE@public.gmane.org>
From: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
This adds support for the Texas Instruments ADC084S021 ADC chip.
Signed-off-by: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
---
Changes in v2:
- Updated the 'Required properties' section
- Removed the 'Optional properties' section
.../devicetree/bindings/iio/adc/ti-adc084s021.txt | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
new file mode 100644
index 0000000..4259e50
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
@@ -0,0 +1,19 @@
+* Texas Instruments' ADC084S021
+
+Required properties:
+ - compatible : Must be "ti,adc084s021"
+ - reg : SPI chip select number for the device
+ - vref-supply : The regulator supply for ADC reference voltage
+ - spi-cpol : Per spi-bus bindings
+ - spi-cpha : Per spi-bus bindings
+ - spi-max-frequency : Per spi-bus bindings
+
+Example:
+adc@0 {
+ compatible = "ti,adc084s021";
+ reg = <0>;
+ vref-supply = <&adc_vref>;
+ spi-cpol;
+ spi-cpha;
+ spi-max-frequency = <16000000>;
+};
--
2.1.4
--
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 v2 0/2] add driver for the ti-adc084s021 chip
From: Mårten Lindahl @ 2017-04-30 12:53 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
pmeerw-jW+XmwGofnusTnJN9+BGXg, linux-iio-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mårten Lindahl
From: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
This adds support for the Texas Instruments ADC084S021 ADC chip.
---
Changes in v2:
- Split dt-bindings and iio/adc into separate patches
- Updated dt-bindings after Robs comments
- Removed most #defines to inlines
- Corrected channel macro
- Removed configuration array with only one item
- Updated func adc084s021_adc_conversion to use be16_to_cpu
- Added IIO_CHAN_INFO_SCALE to func adc084s021_read_raw
- Use iio_device_claim_direct_mode in func adc084s021_read_raw
- Removed documentation for standard driver functions
- Changed retval to ret everywhere
- Removed dynamic alloc for data buffer in trigger handler
- Keeping mutex for all iterations in trigger handler
- Removed usage of events in this driver
- Removed info log in probe
- Use spi_message_init_with_transfers for spi message structs
- Use preenable and postdisable functions for regulator
- Inserted blank line before last return in all functions
Mårten Lindahl (2):
dt-bindings: iio: adc: add driver for the ti-adc084s021 chip
iio: adc: add driver for the ti-adc084s021 chip
.../devicetree/bindings/iio/adc/ti-adc084s021.txt | 19 ++
drivers/iio/adc/Kconfig | 12 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/ti-adc084s021.c | 294 +++++++++++++++++++++
4 files changed, 326 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
create mode 100644 drivers/iio/adc/ti-adc084s021.c
--
2.1.4
--
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: [PATCH V4 1/9] PM / OPP: Allow OPP table to be used for power-domains
From: Mark Brown @ 2017-04-30 12:49 UTC (permalink / raw)
To: Sudeep Holla
Cc: Rajendra Nayak, Viresh Kumar, Rafael Wysocki, ulf.hansson,
Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd,
linaro-kernel, linux-pm, linux-kernel, Vincent Guittot, robh+dt,
lina.iyer, devicetree
In-Reply-To: <c66322f5-e2eb-5556-a5a5-14998a2f195d@arm.com>
[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]
On Thu, Apr 27, 2017 at 10:42:49AM +0100, Sudeep Holla wrote:
> On 26/04/17 14:55, Mark Brown wrote:
> > As I'm getting fed up of saying: if the values you are setting are not
> > voltages and do not behave like voltages then the hardware should not be
> > represented as a voltage regulator since if they are represented as
> > voltage regulators things will expect to be able to control them as
> > voltage regulators. This hardware is quite clearly providing OPPs
> > directly, I would expect this to be handled in the OPP code somehow.
> I agree with you that we need to be absolutely sure on what it actually
> represents.
> But as more and more platform are pushing such power controls to
> dedicated M3 or similar processors, we need abstraction. Though we are
> controlling hardware, we do so indirectly. Since there were discussions
> around device tree representing hardware vs platform, I tend to think,
> we are moving towards platform(something similar to ACPI).
I don't think there's a meaningful hardware/platform distinction here -
in terms of what DT is describing the platform bit is just what the
hardware (the microcontrollers) happen to do, DT doesn't much care about
that though.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] iio: adc: add driver for the ti-adc084s021 chip
From: Mårten Lindahl @ 2017-04-30 12:42 UTC (permalink / raw)
To: Rob Herring
Cc: Mårten Lindahl, jic23-DgEjT+Ai2ygdnm+yROfE0A,
knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
pmeerw-jW+XmwGofnusTnJN9+BGXg, linux-iio-u79uwXL29TY76Z2rM5mHXA,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170428135248.du6smmzktqldj4u4@rob-hp-laptop>
On Fri, 2017-04-28 at 08:52 -0500, Rob Herring wrote:
> On Fri, Apr 21, 2017 at 04:58:23PM +0200, Mårten Lindahl wrote:
> > From: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
> >
> > This adds support for the Texas Instruments ADC084S021 ADC chip.
> >
> > Signed-off-by: Mårten Lindahl <martenli-VrBV9hrLPhE@public.gmane.org>
> > ---
> > .../devicetree/bindings/iio/adc/ti-adc084s021.txt | 25 ++
>
> It's preferred to put bindings in a separate patch.
I am sorry for that. I split this into its own patch in v2.
>
> > drivers/iio/adc/Kconfig | 12 +
> > drivers/iio/adc/Makefile | 1 +
> > drivers/iio/adc/ti-adc084s021.c | 342 +++++++++++++++++++++
> > 4 files changed, 380 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
> > create mode 100644 drivers/iio/adc/ti-adc084s021.c
> >
> > diff --git a/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt b/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
> > new file mode 100644
> > index 0000000..921eb46
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/ti-adc084s021.txt
> > @@ -0,0 +1,25 @@
> > +* Texas Instruments' ADC084S021
> > +
> > +Required properties:
> > + - compatible : Must be "ti,adc084s021"
> > + - reg : SPI chip select number for the device
> > + - vref-supply : The regulator supply for ADC reference voltage
> > + - spi-max-frequency : Definition as per Documentation/devicetree/bindings/spi/spi-bus.txt
> > +
> > +Optional properties:
> > + - spi-cpol : SPI inverse clock polarity, as per spi-bus bindings
> > + - spi-cpha : SPI shifted clock phase (CPHA), as per spi-bus bindings
> > + - spi-cs-high : SPI chip select active high, as per spi-bus bindings
>
> How are these optional? A given device should have specific properties
> required here.
No, they are not optional. I removed this section in v2 and updated the
required properties section.
>
> Also, no need to define them, "per spi-bus bindings" is enough.
Fixed in v2.
>
> Rob
Thanks,
Mårten
--
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