* Re: Extending OPP bindings
From: Lorenzo Pieralisi @ 2014-01-31 12:46 UTC (permalink / raw)
To: Nishanth Menon
Cc: Sudeep Holla, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org,
Mark Rutland, Charles Garcia-Tobin, Rob Herring,
grant.likely@linaro.org, Morten Rasmussen, Shawn Guo,
mturquette@linaro.org, Mark Brown, Eduardo Valentin
In-Reply-To: <52EAF1CA.3040702@ti.com>
Hi Nishanth,
On Fri, Jan 31, 2014 at 12:43:54AM +0000, Nishanth Menon wrote:
> Hi Sudeep,
>
> On 01/30/2014 07:43 AM, Sudeep Holla wrote:
>
> >
> > I am looking into a couple shortcomings in the current OPP bindings and
> > how to address them. Feel free to add to the list if you think of any more
> > issues that needs to be addressed or if and how any problem mentioned below
> > can be handled with the existing bindings.
> >
> > 1. indexing: currently there are no indices in the operating-points.
> indexing is based on frequency which is why the accessors use
> frequency to pull out the OPP data.
>
> indexing is a horrible idea - on platforms where OPP may be disabled
> or enabled for various reasons(see arch/arm/mach-imx/mach-imx6q.c,
> arch/arm/mach-omap2/board-omap3beagle.c etc) - the indexing you see in
> dts is just a myth that may not exist once the nodes are loaded and
> operated upon depending on SoC variations (example efuse describing
> which OPPs can be used and which not).
I do not understand why. As long as a mapping from DT to data structures
in the kernel is done at boot, I can see how by indexing device nodes
can refer to specific OPPs. If they are disabled, amen, as long as we
can point at specific operating points that should be ok.
Indexing does not mean that the index in the DT is the same as the one
allocated by the OS. Indexing is there to point at specific OPPs from
different DT nodes, a good example are clock bindings and that's exactly
how they work.
Can you provide me with an example where the indexing would go wrong
please ?
Certainly relying on implicit ordering is not great either, actually I
would say that it is broken.
> That said, the original OPP[1][2] series discussion started by trying
> to kill indexing precisely for the same reason.. once you have it - it
> becomes just crazy to deal with.
I could not find any "index killing" :) discussion in there, but I will
keep reading.
> > It's assumed that the list is either in ascending or descending
> > order of frequency but not explicit in the binding document.
> > There are arch/arm/boot/dts/* files with opps in both styles.
> it should not matter -> opp library should do an insertion sort and
> organize it in ascending order once all the data is inserted. (line
> 449ish in opp.c)
That's OS details and they must not be mandated by the bindings.
We cannot rely on word of mouth for things to work, I do not like
implicit rules, so the bindings should speficy the ordering or better
a way to reference OPPs.
> if you see issues with the insertion sort not functioning, it is a bug
> and should be easy to track down and fix.
Problem is not the insertion sort, problem is that DT bindings do not
define a way to refer to a specific OPP, unless we do that implicitly
and again, I rest my case, that is broken.
> > Few other bindings like thermal defines bindings like
> > cooling-{min,max}-state assuming some order which is broken IMO.
> Now that you bring it up, I missed it :(.. yeah, I might have
> preferred it to be min frequency and max_frequency - I agree it is
> probably broken. I'd let Eduardo comment more about it.
>
> >
> > One such use-case that came up recently[0] is the c-state latencies
> > which could be different for each OPP. It would be good if the
> > latencies are specified with the indices to OPP table to avoid
> > inconsistency between the bindings.
>
> You can define C states based on frequencies as well - which really
> makes sense - since that sounds really like our constraint (say
> valid-at-frequency "xyz"
I do not think that's generic enough, I do not like the idea of looking
up frequencies (eg a C-state target_residency does not depend on the
frequency only - ie voltage and other factors play a role, it really
depends on an operating point- looking it up by frequency is misleading IMO).
> > It's mainly to avoid issues due to inconsistency and duplication
> > on data(frequency) in multiple bindings requiring it.
> >
> > Once we have indices to each on the OPP entries, then other binding
> > using it can refer to OPP with phandle and OPP index/specifier pairs
> > very similar to clock provider and consumer.
>
> Having used indexing in OMAP platforms, indexing is a problem waiting
> to happen unfortunately :(
Please be specific, I do not see why it is a problem.
[...]
> > 3. latencies(*): currently the latency that the CPU/memory access is unavailable
> > during an OPP transition is generic i.e. same from any OPP to any
> > other OPP. Does it make sense to have this per-OPP entry ?
>
> Why modify OPP when you are describing something else? you are
> describing "latency at a frequency" - just because an OPP definition
> as it stands right now is {frequency, voltage} tuple, makes it a very
> attractive target to keep extending it -> believe me, we have done
> that in the past ->arch/arm/mach-omap2/opp4xxx_data.c efuse register
> describing AVS per frequency is tempting..
>
> why not have memory-latency-per-opp = <frequency latency>?
>
> that allows OPP definitions to change in the future, but the
> definition remain constant.
>
> That said -> consider the following usecase: AM335x, OMAP3,4... (i
> will use omap4 as an example)
> MPU@300MHz and bus (on which LPDDR2 memory is) at 100MHz
> AND
> MPU@300MHz and bus (on which LPDDR2 memory is) at 200MHz
>
> are both valid with different memory access latencies. tying it down
> to OPP for MPU is just plain wrong - as it ignores other factors.
I think it is time we were defining OPPs for the system as a whole, the
issue has been brought up for C-state dependencies as well.
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-January/228042.html
We can easily define a global node that encompasses OPPs for various
devices, and then map the OPPs for devices to a global OPP table.
(totally made-up DT bindings, just to get the message across)
opps {
operating-points = <......>;
operating-point-cells = <2>;
cpu-opps {
/* where index maps to a global OPP above where the CPU OPP is valid */
operating-points = <...... index>;
};
memory-opp {
/*
where index maps to a global OPP above where the memory OPP is
valid
*/
operating-points = <...... index>;
};
};
> > 4. power(*): A measure of maximum power dissipation in an OPP state.
> > This might be useful measure for power aware scheduling ?
> Umm.. this is a hard nut to crack -> I had considered that previously
> as well -> In reality the leakage characteristics of the SoC
> distribution varies dramatically depending on which end of the
> distribution you look for a specific process node. in my company, we
> typically use cold, hot,nominal devices, this is some form or other
> (example - Samsung calls it "SoC's ASV group" [4]) - and every SoC
> company comes up with some strategy or other to control it optimally
> -> TI uses ABB[5], AVS[6] - etc... - not an unique problem -> so what
> will "power" mean? we cannot create dts per SoC part.
Yes, that's a hard nut to crack. Probably we can define a reference value,
to be debated.
> > (*) these are already part of P-state in ACPI(refer struct acpi_processor_px
> > in include/acpi/processor.h)
>
> Hmm.. what do we do with legacy processors that dont support ACPI or
> what ever our latest ARM term is for the equivalent?
>
> >
> > Apart from these I have seen on-going discussion for Samsung Exynos CPUFreq[2]
> > which might have some feedback for OPP bindings.
> >
> > It would be good to consolidate the shortcomings found so far, that could
> > help in extending the current OPP bindings.
>
> I hope this discussion helps. open to more views as well.
It helps a lot, thank you, please keep it going.
Thanks,
Lorenzo
^ permalink raw reply
* Re: [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver
From: Mark Brown @ 2014-01-31 12:48 UTC (permalink / raw)
To: Maxime Ripard
Cc: Mike Turquette, Emilio Lopez, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kevin.z.m.zh-Re5JQEeQqe8AvxtiuMwx3w,
sunny-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf,
shuge-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf,
zhuzhenhua-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf
In-Reply-To: <1391165752-1819-4-git-send-email-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 751 bytes --]
On Fri, Jan 31, 2014 at 11:55:50AM +0100, Maxime Ripard wrote:
> + master = devm_spi_alloc_master(&pdev->dev, sizeof(struct sun6i_spi));
> + if (!master) {
> + dev_err(&pdev->dev, "Unable to allocate SPI Master\n");
> + return -ENOMEM;
> + }
This now depends on your other series which as I said doesn't look like
the best approach.
> + pm_runtime_enable(&pdev->dev);
> + if (!pm_runtime_enabled(&pdev->dev)) {
> + ret = sun6i_spi_runtime_resume(&pdev->dev);
> + if (ret) {
> + dev_err(&pdev->dev, "Couldn't resume the device\n");
> + return ret;
> + }
> + }
No, as discussed don't do this - notice how other drivers aren't written
this way either. Like I said leave the device powered on startup and
then let it be idled by runtime PM.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] ARM: dts: Add support of STid127 Soc.
From: srinivas kandagatla @ 2014-01-31 12:50 UTC (permalink / raw)
To: Patrice CHOTARD, Stuart Menefy, Russell King, linux-arm-kernel,
kernel, linux-kernel, Linus Walleij, Grant Likely, Rob Herring,
devicetree
Cc: Giuseppe Cavallaro, alexandre.torgue, maxime.coquelin
In-Reply-To: <1391093744-19905-4-git-send-email-patrice.chotard@st.com>
Hi Patrice,
On 30/01/14 14:55, Patrice CHOTARD wrote:
> From: Alexandre TORGUE <alexandre.torgue@st.com>
>
> The STid127 integrates all harware components to function as a cable modem
s/harware/hardware/
s/STid127/STiD127
Please fix other instances too.
> or, in combination with a back end device, as a Gateway set top boxe.
s/boxe/box/
>
> Supported devices:
> -UART0
> -UART2
>
> Signed-off-by: alexandre torgue <alexandre.torgue@st.com>
> ---
> arch/arm/boot/dts/stid127-clock.dtsi | 31 ++++
> arch/arm/boot/dts/stid127-pinctrl.dtsi | 245 ++++++++++++++++++++++++++++++++
> arch/arm/boot/dts/stid127.dtsi | 130 +++++++++++++++++
> 3 files changed, 406 insertions(+)
> create mode 100644 arch/arm/boot/dts/stid127-clock.dtsi
> create mode 100644 arch/arm/boot/dts/stid127-pinctrl.dtsi
> create mode 100644 arch/arm/boot/dts/stid127.dtsi
>
> diff --git a/arch/arm/boot/dts/stid127-clock.dtsi b/arch/arm/boot/dts/stid127-clock.dtsi
> new file mode 100644
> index 0000000..c6cafa9
> --- /dev/null
> +++ b/arch/arm/boot/dts/stid127-clock.dtsi
> @@ -0,0 +1,31 @@
> +/*
> + * Copyright (C) 2013 STMicroelectronics (R&D) Limited
2014
> + * Author(s): Giuseppe Cavallaro <peppe.cavallaro@st.com>
> + * Alexandre Torgue <alexandre.torgue@st.com>
> + *
> + * 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.
> + */
> +/ {Acked-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
> + clocks {
> + /*
> + * ARM Peripheral clock for timers
> + */
> + arm_periph_clk: arm_periph_clk {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <100000000>;
> + };
> + /*
> + * Bootloader initialized system infrastructure clock for
> + * serial devices.
> + */
> + CLK_IC_LP_HD: clockgenA0@29 {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <100000000>;
> + clock-output-names = "CLK_IC_LP_HD";
> + };
> + };
> +};
> diff --git a/arch/arm/boot/dts/stid127-pinctrl.dtsi b/arch/arm/boot/dts/stid127-pinctrl.dtsi
> new file mode 100644
> index 0000000..3fa66f3
> --- /dev/null
> +++ b/arch/arm/boot/dts/stid127-pinctrl.dtsi
> @@ -0,0 +1,245 @@
> +/*
> + * Copyright (C) 2012 STMicroelectronics Limited.
2014
> + * Author(s): Giuseppe Cavallaro <peppe.cavallaro@st.com>
> + * Alexandre Torgue <alexandre.torgue@st.com>
> + *
> + * 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
> + * publishhed by the Free Software Foundation.
s/publishhed/published/
> + */
> +#include "st-pincfg.h"
> +/ {
> + aliases {
> + gpio0 = &PIO0;
> + gpio1 = &PIO1;
> + gpio2 = &PIO2;
> + gpio3 = &PIO3;
> + gpio4 = &PIO4;
> + gpio5 = &PIO5;
> + gpio6 = &PIO6;
> + gpio7 = &PIO7;
> + gpio8 = &PIO8;
> + gpio9 = &PIO9;
> + gpio10 = &PIO10;
> + gpio11 = &PIO11;
> + gpio12 = &PIO12;
> + gpio13 = &PIO13;
> + gpio14 = &PIO14;
> + gpio15 = &PIO15;
> + gpio16 = &PIO16;
> + gpio17 = &PIO17;
> + gpio18 = &PIO18;
> + gpio19 = &PIO19;
> + gpio20 = &PIO20;
> + gpio21 = &PIO21;
> + gpio22 = &PIO22;
> +
> + };
> +
> + soc {
> + pin-controller-pwest {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "st,stid127-pwest-pinctrl";
> + st,syscfg = <&syscfg_pwest>;
> + ranges = <0 0xfebe0000 0x8000>;
Can you add interrupt controller support to the pin controller.
Patches are in : https://lkml.org/lkml/2014/1/16/342
All these patches are due to be in v3.15.
> +
> + PIO0: gpio@febe0000 {
> + gpio-controller;
> + #gpio-cells = <1>;
> + reg = <0 0x100>;
> + interrupts = <0 149 0>;
> + st,bank-name = "PIO0";
> + };
> + rx = <&PIO3 0 ALT2 IN>;
> + };
> + };
> + };
> +
> + };
unnecessary empty line here.
> diff --git a/arch/arm/boot/dts/stid127.dtsi b/arch/arm/boot/dts/stid127.dtsi
> new file mode 100644
> index 0000000..a6f0b8fe
> --- /dev/null
> +++ b/arch/arm/boot/dts/stid127.dtsi
> @@ -0,0 +1,130 @@
> +/*
> + * Copyright (C) 2013 STMicroelectronics Limited.
2014
> + * Author(s): Giuseppe Cavallaro <peppe.cavallaro@st.com>
> + * Alexandre Torgue <alexandre.torgue@st.com>
> + *
> + * 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
> + * publishhed by the Free Software Foundation.
s/publishhed/published/
> + */
> +#include "stid127-pinctrl.dtsi"
> +#include "stid127-clock.dtsi"
> +/ {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a9";
> + reg = <0>;
> + };
> + cpu@1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a9";
> + reg = <1>;
> + };
> + };
> +
> + intc: interrupt-controller@fffe1000 {
> + compatible = "arm,cortex-a9-gic";
> + #interrupt-cells = <3>;
> + interrupt-controller;
> + reg = <0xfffe1000 0x1000>,
> + <0xfffe0100 0x100>;
> + };
> +
> + scu@fffe0000 {
> + compatible = "arm,cortex-a9-scu";
> + reg = <0xfffe0000 0x1000>;
> + };
> +
> + timer@fffe0200 {
> + interrupt-parent = <&intc>;
> + compatible = "arm,cortex-a9-global-timer";
> + reg = <0xfffe0200 0x100>;
> + interrupts = <1 11 0x04>;
> + clocks = <&arm_periph_clk>;
Fix the tab spacing here.
Thanks,
srini
>
^ permalink raw reply
* Re: [PATCH 4/4] ARM: dts: add B2112 board support
From: srinivas kandagatla @ 2014-01-31 12:51 UTC (permalink / raw)
To: Patrice CHOTARD, Stuart Menefy, Russell King, linux-arm-kernel,
kernel, linux-kernel, Linus Walleij, Grant Likely, Rob Herring,
devicetree
Cc: Giuseppe Cavallaro, alexandre.torgue, maxime.coquelin
In-Reply-To: <1391093744-19905-5-git-send-email-patrice.chotard@st.com>
On 30/01/14 14:55, Patrice CHOTARD wrote:
> From: Alexandre TORGUE <alexandre.torgue@st.com>
>
> Add support for B2112 board based on STiD127 SoC.
>
> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@st.com>
> ---
> arch/arm/boot/dts/Makefile | 3 ++-
> arch/arm/boot/dts/stid127-b2112.dts | 35 +++++++++++++++++++++++++++++++++++
> 2 files changed, 37 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm/boot/dts/stid127-b2112.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index d57c1a6..7173dca 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -247,7 +247,8 @@ dtb-$(CONFIG_ARCH_SPEAR6XX)+= spear600-evb.dtb
> dtb-$(CONFIG_ARCH_STI)+= stih415-b2000.dtb \
> stih416-b2000.dtb \
> stih415-b2020.dtb \
> - stih416-b2020.dtb
> + stih416-b2020.dtb \
> + stid127-b2112.dtb
> dtb-$(CONFIG_ARCH_SUNXI) += \
> sun4i-a10-a1000.dtb \
> sun4i-a10-cubieboard.dtb \
> diff --git a/arch/arm/boot/dts/stid127-b2112.dts b/arch/arm/boot/dts/stid127-b2112.dts
> new file mode 100644
> index 0000000..b4507e3
> --- /dev/null
> +++ b/arch/arm/boot/dts/stid127-b2112.dts
> @@ -0,0 +1,35 @@
> +/*
> + * Copyright (C) 2013 STMicroelectronics Limited.
2014
> + * Author: Alexandre Torgue <alexandre.torgue@st.com>
> + *
> + * 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
> + * publishhed by the Free Software Foundation.
s/publishhed/published/
> + */
> +/dts-v1/;
> +#include "stid127.dtsi"
> +
> +/ {
> + model = "STiD127 B2112 Board";
> + compatible = "st,stid127", "st,stid127-b2112";
> +
> + memory{
> + device_type = "memory";
> + reg = <0x40000000 0x10000000>;
> + };
> +
> + chosen {
> + bootargs = "console=ttyAS0,115200";
> + linux,stdout-path = &uart2;
> + };
> +
> + aliases {
> + ttyAS0 = &uart2;
> + };
> +
> + soc {
> + uart2: serial@fe532000{
> + status = "okay";
> + };
> + };
> +};
>
^ permalink raw reply
* [PATCH] ARM: dts: k2hk-evm: rename clock node to clocks
From: Grygorii Strashko @ 2014-01-31 13:31 UTC (permalink / raw)
To: santosh.shilimkar, Rob Herring, Kumar Gala
Cc: ivan.khoronzhuk, Grygorii Strashko, linux-arm-kernel, devicetree
Fix type in clock(s) node name: "clock"-->"clocks".
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
arch/arm/boot/dts/k2hk-evm.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/k2hk-evm.dts b/arch/arm/boot/dts/k2hk-evm.dts
index eaefdfe..c02c3a8 100644
--- a/arch/arm/boot/dts/k2hk-evm.dts
+++ b/arch/arm/boot/dts/k2hk-evm.dts
@@ -15,7 +15,7 @@
compatible = "ti,keystone-evm";
soc {
- clock {
+ clocks {
refclksys: refclksys {
#clock-cells = <0>;
compatible = "fixed-clock";
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 4/5] Documentation: devicetree: sja1000: add reg-io-width binding
From: Florian Vaussard @ 2014-01-31 13:34 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde
Cc: Andreas Larsson, linux-can, netdev, sparclinux, linux-kernel,
florian.vaussard, Grant Likely, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala, devicetree
In-Reply-To: <1391175277-19833-1-git-send-email-florian.vaussard@epfl.ch>
Add the reg-io-width property to describe the width of the memory
accesses.
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: devicetree@vger.kernel.org
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
Documentation/devicetree/bindings/net/can/sja1000.txt | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/can/sja1000.txt b/Documentation/devicetree/bindings/net/can/sja1000.txt
index f2105a4..b4a6d53 100644
--- a/Documentation/devicetree/bindings/net/can/sja1000.txt
+++ b/Documentation/devicetree/bindings/net/can/sja1000.txt
@@ -12,6 +12,10 @@ Required properties:
Optional properties:
+- reg-io-width : Specify the size (in bytes) of the IO accesses that
+ should be performed on the device. Valid value is 1, 2 or 4.
+ Default to 1 (8 bits).
+
- nxp,external-clock-frequency : Frequency of the external oscillator
clock in Hz. Note that the internal clock frequency used by the
SJA1000 is half of that value. If not specified, a default value
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH v5 13/14] ARM: sun4i: dts: Add ahci / sata support
From: Maxime Ripard @ 2014-01-31 13:45 UTC (permalink / raw)
To: Hans de Goede
Cc: Tejun Heo, Oliver Schinagl, Richard Zhu, Roger Quadros,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <1390417489-5354-14-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 4238 bytes --]
Hi Hans,
On Wed, Jan 22, 2014 at 08:04:48PM +0100, Hans de Goede wrote:
> From: Oliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>
>
> This patch adds sunxi sata support to A10 boards that have such a connector.
> Some boards also feature a regulator via a GPIO and support for this is also
> added.
>
> Signed-off-by: Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> arch/arm/boot/dts/sun4i-a10-a1000.dts | 4 ++++
> arch/arm/boot/dts/sun4i-a10-cubieboard.dts | 6 +++++
> arch/arm/boot/dts/sun4i-a10.dtsi | 8 +++++++
> arch/arm/boot/dts/sunxi-ahci-reg.dtsi | 38 ++++++++++++++++++++++++++++++
I'm still half convinced about this at the moment, given the number of
platforms we support, we can always change it back if things become too messy.
> 4 files changed, 56 insertions(+)
> create mode 100644 arch/arm/boot/dts/sunxi-ahci-reg.dtsi
>
> diff --git a/arch/arm/boot/dts/sun4i-a10-a1000.dts b/arch/arm/boot/dts/sun4i-a10-a1000.dts
> index aef8207..3fb7305 100644
> --- a/arch/arm/boot/dts/sun4i-a10-a1000.dts
> +++ b/arch/arm/boot/dts/sun4i-a10-a1000.dts
> @@ -48,6 +48,10 @@
> status = "okay";
> };
>
> + ahci: sata@01c18000 {
> + status = "okay";
> + };
> +
> pinctrl@01c20800 {
> mmc0_cd_pin_a1000: mmc0_cd_pin@0 {
> allwinner,pins = "PH1";
> diff --git a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
> index f50fb2b..6ae1110 100644
> --- a/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
> +++ b/arch/arm/boot/dts/sun4i-a10-cubieboard.dts
> @@ -12,6 +12,7 @@
>
> /dts-v1/;
> /include/ "sun4i-a10.dtsi"
> +/include/ "sunxi-ahci-reg.dtsi"
>
> / {
> model = "Cubietech Cubieboard";
> @@ -51,6 +52,11 @@
> status = "okay";
> };
>
> + ahci: sata@01c18000 {
> + target-supply = <®_ahci_5v>;
> + status = "okay";
> + };
> +
> pinctrl@01c20800 {
> mmc0_cd_pin_cubieboard: mmc0_cd_pin@0 {
> allwinner,pins = "PH1";
> diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
> index 4736dd2..198dcda 100644
> --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> @@ -331,6 +331,14 @@
> status = "disabled";
> };
>
> + ahci: sata@01c18000 {
> + compatible = "allwinner,sun4i-a10-ahci";
To be consistent with the rest of the sun4i devices compatible, It
should be sun4i-ahci.
However, since these devices don't use the same compatible pattern as
their own machine compatible, and are consisent with the rest of the
compatibles for the other SoCs, we can probably make this a go to
transition progressively to this pattern.
I'll cook up some patches for the other devices.
> + reg = <0x01c18000 0x1000>;
> + interrupts = <56>;
> + clocks = <&pll6 0>, <&ahb_gates 25>;
> + status = "disabled";
> + };
> +
> intc: interrupt-controller@01c20400 {
> compatible = "allwinner,sun4i-ic";
> reg = <0x01c20400 0x400>;
> diff --git a/arch/arm/boot/dts/sunxi-ahci-reg.dtsi b/arch/arm/boot/dts/sunxi-ahci-reg.dtsi
> new file mode 100644
> index 0000000..955b197
> --- /dev/null
> +++ b/arch/arm/boot/dts/sunxi-ahci-reg.dtsi
> @@ -0,0 +1,38 @@
> +/*
> + * sunxi boards sata target power supply common code
> + *
> + * Copyright 2014 - Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +/ {
> + soc@01c00000 {
> + ahci_pwr_pin_a: ahci_pwr_pin@0 {
> + allwinner,pins = "PB8";
> + allwinner,function = "gpio_out";
> + allwinner,drive = <0>;
> + allwinner,pull = <0>;
> + };
This should be under the pinctrl node.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v5 14/14] ARM: sun7i: dts: Add ahci / sata support
From: Maxime Ripard @ 2014-01-31 13:46 UTC (permalink / raw)
To: Hans de Goede
Cc: Tejun Heo, Oliver Schinagl, Richard Zhu, Roger Quadros,
linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <1390417489-5354-15-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 4019 bytes --]
On Wed, Jan 22, 2014 at 08:04:49PM +0100, Hans de Goede wrote:
> This patch adds sunxi sata support to A20 boards that have such a connector.
> Some boards also feature a regulator via a GPIO and support for this is also
> added.
>
> Signed-off-by: Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> arch/arm/boot/dts/sun7i-a20-cubieboard2.dts | 6 ++++++
> arch/arm/boot/dts/sun7i-a20-cubietruck.dts | 20 ++++++++++++++++++++
> arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts | 6 ++++++
> arch/arm/boot/dts/sun7i-a20.dtsi | 8 ++++++++
> 4 files changed, 40 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> index 48777cd..1cab521 100644
> --- a/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
> @@ -13,6 +13,7 @@
>
> /dts-v1/;
> /include/ "sun7i-a20.dtsi"
> +/include/ "sunxi-ahci-reg.dtsi"
>
> / {
> model = "Cubietech Cubieboard2";
> @@ -28,6 +29,11 @@
> status = "okay";
> };
>
> + ahci: sata@01c18000 {
> + target-supply = <®_ahci_5v>;
> + status = "okay";
> + };
> +
> pinctrl@01c20800 {
> mmc0_cd_pin_cubieboard2: mmc0_cd_pin@0 {
> allwinner,pins = "PH1";
> diff --git a/arch/arm/boot/dts/sun7i-a20-cubietruck.dts b/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
> index 2684f27..1247ea1 100644
> --- a/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-cubietruck.dts
> @@ -13,6 +13,7 @@
>
> /dts-v1/;
> /include/ "sun7i-a20.dtsi"
> +/include/ "sunxi-ahci-reg.dtsi"
>
> / {
> model = "Cubietech Cubietruck";
> @@ -28,6 +29,11 @@
> status = "okay";
> };
>
> + ahci: sata@01c18000 {
> + target-supply = <®_ahci_5v>;
> + status = "okay";
> + };
> +
> pinctrl@01c20800 {
> mmc0_cd_pin_cubietruck: mmc0_cd_pin@0 {
> allwinner,pins = "PH1";
> @@ -36,6 +42,13 @@
> allwinner,pull = <0>;
> };
>
> + ahci_pwr_pin_cubietruck: ahci_pwr_pin@1 {
> + allwinner,pins = "PH12";
> + allwinner,function = "gpio_out";
> + allwinner,drive = <0>;
> + allwinner,pull = <0>;
> + };
> +
> led_pins_cubietruck: led_pins@0 {
> allwinner,pins = "PH7", "PH11", "PH20", "PH21";
> allwinner,function = "gpio_out";
> @@ -84,4 +97,11 @@
> gpios = <&pio 7 7 0>;
> };
> };
> +
> + regulators {
> + reg_ahci_5v: ahci-5v {
> + pinctrl-0 = <&ahci_pwr_pin_cubietruck>;
> + gpio = <&pio 7 12 0>;
> + };
> + };
> };
> diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
> index bf6f6c8..f135886 100644
> --- a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
> @@ -13,6 +13,7 @@
>
> /dts-v1/;
> /include/ "sun7i-a20.dtsi"
> +/include/ "sunxi-ahci-reg.dtsi"
>
> / {
> model = "Olimex A20-Olinuxino Micro";
> @@ -37,6 +38,11 @@
> status = "okay";
> };
>
> + ahci: sata@01c18000 {
> + target-supply = <®_ahci_5v>;
> + status = "okay";
> + };
> +
> pinctrl@01c20800 {
> mmc0_cd_pin_olinuxinom: mmc0_cd_pin@0 {
> allwinner,pins = "PH1";
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> index c9c123a..0657bad 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -347,6 +347,14 @@
> status = "disabled";
> };
>
> + ahci: sata@01c18000 {
> + compatible = "allwinner,sun4i-a10-ahci";
> + reg = <0x01c18000 0x1000>;
> + interrupts = <0 56 1>;
This is supposed to be a level triggered interrupt, not an edge
triggered one.
Thanks for your work!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] ARM: dts: k2hk-evm: rename clock node to clocks
From: Sergei Shtylyov @ 2014-01-31 14:11 UTC (permalink / raw)
To: Grygorii Strashko, santosh.shilimkar, Rob Herring, Kumar Gala
Cc: ivan.khoronzhuk, linux-arm-kernel, devicetree
In-Reply-To: <1391175066-13862-1-git-send-email-grygorii.strashko@ti.com>
Hello.
On 31-01-2014 17:31, Grygorii Strashko wrote:
> Fix type in clock(s) node name: "clock"-->"clocks".
Maybe "typo"? :-)
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
WBR, Sergei
^ permalink raw reply
* [PATCH 07/10] watchdog: xilinx: Fix OF binding
From: Michal Simek @ 2014-01-31 14:18 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
monstr-pSz03upnqPeHXe+LvDLADg
Cc: Wim Van Sebroeck, Grant Likely, Rob Herring,
linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <ed4cb31c9a549343e6b492cf62fc962d39034bd0.1391177880.git.michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2171 bytes --]
Use of_property_read_u32 functions to clean OF probing.
Signed-off-by: Michal Simek <michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
---
drivers/watchdog/of_xilinx_wdt.c | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
index c229cc4..475440a6 100644
--- a/drivers/watchdog/of_xilinx_wdt.c
+++ b/drivers/watchdog/of_xilinx_wdt.c
@@ -147,8 +147,7 @@ static u32 xwdt_selftest(struct xwdt_device *xdev)
static int xwdt_probe(struct platform_device *pdev)
{
int rc;
- u32 *tmptr;
- u32 *pfreq;
+ u32 pfreq, enable_once;
struct resource *res;
struct xwdt_device *xdev;
bool no_timeout = false;
@@ -168,28 +167,24 @@ static int xwdt_probe(struct platform_device *pdev)
if (IS_ERR(xdev->base))
return PTR_ERR(xdev->base);
- pfreq = (u32 *)of_get_property(pdev->dev.of_node,
- "clock-frequency", NULL);
-
- if (pfreq == NULL) {
+ rc = of_property_read_u32(pdev->dev.of_node, "clock-frequency", &pfreq);
+ if (rc) {
dev_warn(&pdev->dev,
"The watchdog clock frequency cannot be obtained\n");
no_timeout = true;
}
- tmptr = (u32 *)of_get_property(pdev->dev.of_node,
- "xlnx,wdt-interval", NULL);
- if (tmptr == NULL) {
+ rc = of_property_read_u32(pdev->dev.of_node, "xlnx,wdt-interval",
+ &xdev->wdt_interval);
+ if (rc) {
dev_warn(&pdev->dev,
"Parameter \"xlnx,wdt-interval\" not found\n");
no_timeout = true;
- } else {
- xdev->wdt_interval = *tmptr;
}
- tmptr = (u32 *)of_get_property(pdev->dev.of_node,
- "xlnx,wdt-enable-once", NULL);
- if (tmptr == NULL) {
+ rc = of_property_read_u32(pdev->dev.of_node, "xlnx,wdt-enable-once",
+ &enable_once);
+ if (!rc && enable_once) {
dev_warn(&pdev->dev,
"Parameter \"xlnx,wdt-enable-once\" not found\n");
watchdog_set_nowayout(xilinx_wdt_wdd, true);
@@ -201,7 +196,7 @@ static int xwdt_probe(struct platform_device *pdev)
*/
if (!no_timeout)
xilinx_wdt_wdd->timeout = 2 * ((1 << xdev->wdt_interval) /
- *pfreq);
+ pfreq);
spin_lock_init(&xdev->spinlock);
watchdog_set_drvdata(xilinx_wdt_wdd, xdev);
--
1.8.2.3
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply related
* [PATCH 09/10] watchdog: xilinx: Add missing binding
From: Michal Simek @ 2014-01-31 14:18 UTC (permalink / raw)
To: linux-kernel, monstr
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Rob Landley, devicetree, linux-doc, linux-arm-kernel
In-Reply-To: <ed4cb31c9a549343e6b492cf62fc962d39034bd0.1391177880.git.michal.simek@xilinx.com>
[-- Attachment #1: Type: text/plain, Size: 1358 bytes --]
Document current driver binding.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
.../devicetree/bindings/watchdog/of-xilinx-wdt.txt | 23 ++++++++++++++++++++++
1 file changed, 23 insertions(+)
create mode 100644 Documentation/devicetree/bindings/watchdog/of-xilinx-wdt.txt
diff --git a/Documentation/devicetree/bindings/watchdog/of-xilinx-wdt.txt b/Documentation/devicetree/bindings/watchdog/of-xilinx-wdt.txt
new file mode 100644
index 0000000..6d63782
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/of-xilinx-wdt.txt
@@ -0,0 +1,23 @@
+Xilinx AXI/PLB soft-core watchdog Device Tree Bindings
+---------------------------------------------------------
+
+Required properties:
+- compatible : Should be "xlnx,xps-timebase-wdt-1.00.a" or
+ "xlnx,xps-timebase-wdt-1.01.a".
+- reg : Physical base address and size
+
+Optional properties:
+- clock-frequency : Frequency of clock in Hz
+- xlnx,wdt-enable-once : 0 - Watchdog can be restarted
+ 1 - Watchdog can be enabled just once
+- xlnx,wdt-interval : Watchdog timeout interval in 2^<val> clock cycles,
+ <val> is integer from 8 to 31.
+
+Example:
+axi-timebase-wdt@40100000 {
+ clock-frequency = <50000000>;
+ compatible = "xlnx,xps-timebase-wdt-1.00.a";
+ reg = <0x40100000 0x10000>;
+ xlnx,wdt-enable-once = <0x0>;
+ xlnx,wdt-interval = <0x1b>;
+} ;
--
1.8.2.3
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply related
* Re: [PATCH 3/3] net: via-rhine: add OF bus binding
From: Ben Hutchings @ 2014-01-31 14:24 UTC (permalink / raw)
To: Alexey Charkov
Cc: netdev, Tony Prisk,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Roger Luethi,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CABjd4Yx5HtbGjo_ZsOJ5SvqFQZey1fv5K36PD=D9fUUn0SanVQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2176 bytes --]
On Tue, 2014-01-28 at 22:31 +0400, Alexey Charkov wrote:
> 2014/1/28 Ben Hutchings <ben-/+tVBieCtBitmTQ+vhA3Yw@public.gmane.org>:
> > On Mon, 2014-01-27 at 19:34 +0400, Alexey Charkov wrote:
> >> 2014/1/27 Ben Hutchings <ben-/+tVBieCtBitmTQ+vhA3Yw@public.gmane.org>:
> >> > On Mon, 2014-01-27 at 15:51 +0400, Alexey Charkov wrote:
> >> >> This should make the driver usable with VIA/WonderMedia ARM-based
> >> >> Systems-on-Chip integrated Rhine III adapters. Note that these
> >> >> are always in MMIO mode, and don't have any known EEPROM.
> >> > [...]
> >> >> --- a/drivers/net/ethernet/via/Kconfig
> >> >> +++ b/drivers/net/ethernet/via/Kconfig
> >> >> @@ -19,7 +19,7 @@ if NET_VENDOR_VIA
> >> >>
> >> >> config VIA_RHINE
> >> >> tristate "VIA Rhine support"
> >> >> - depends on PCI
> >> >> + depends on (PCI || USE_OF)
> >> >> select CRC32
> >> >> select MII
> >> >> ---help---
> >> >
> >> > This seems like the right thing to do, but it means you need to add
> >> > #ifdef CONFIG_PCI and #ifdef CONFIG_USE_OF around the driver structures
> >> > and related functions.
> >>
> >> Frankly, I would like to avoid that if possible (as pointed out in the
> >> cover email), as I believe we would get a cleaner driver without
> >> #ifdef. This is also the way it was done in via-velocity, and it works
> >> just fine.
> >
> > OK, I'm surprised that all the PCI functions have dummy definitions.
> >
> >> > You should compile-test in configurations that have just one of those
> >> > dependencies enabled.
> >>
> >> This has been compile-tested and runtime-tested in OF-only
> >> configuration on WM8950, and Roger also tested it in PCI-only
> >> configuration, so it seems to work fine.
> > [...]
> >
> > Good, then I have no objection.
>
> Thanks Ben! Would it be fine to add your Reviewed-by at the next
> iteration, once I fix indentation of function arguments and
> dev_is_pci()?
Sorry, I don't think I know enough to claim that I've reviewed the whole
thing properly.
Ben.
--
Ben Hutchings
It is easier to write an incorrect program than to understand a correct one.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [PATCH] ARM: dts: k2hk-evm: rename clock node to clocks
From: Santosh Shilimkar @ 2014-01-31 14:36 UTC (permalink / raw)
To: Grygorii Strashko
Cc: ivan.khoronzhuk, devicetree, Rob Herring, linux-arm-kernel,
Kumar Gala
In-Reply-To: <1391175066-13862-1-git-send-email-grygorii.strashko@ti.com>
On Friday 31 January 2014 08:31 AM, Grygorii Strashko wrote:
> Fix type in clock(s) node name: "clock"-->"clocks".
>
s/type/typo ;-)
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
No need to resend, I will fix it up while applying it
> arch/arm/boot/dts/k2hk-evm.dts | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/k2hk-evm.dts b/arch/arm/boot/dts/k2hk-evm.dts
> index eaefdfe..c02c3a8 100644
> --- a/arch/arm/boot/dts/k2hk-evm.dts
> +++ b/arch/arm/boot/dts/k2hk-evm.dts
> @@ -15,7 +15,7 @@
> compatible = "ti,keystone-evm";
>
> soc {
> - clock {
> + clocks {
> refclksys: refclksys {
> #clock-cells = <0>;
> compatible = "fixed-clock";
>
^ permalink raw reply
* Re: [PATCH v2] ARM: keystone: dts: drop "msmcsram" clock node
From: Santosh Shilimkar @ 2014-01-31 14:45 UTC (permalink / raw)
To: Ivan Khoronzhuk, robh+dt, galak
Cc: mark.rutland, devicetree, linux, pawel.moll, ijc+devicetree,
linux-kernel, linux-arm-kernel
In-Reply-To: <1391105400-31602-1-git-send-email-ivan.khoronzhuk@ti.com>
On Thursday 30 January 2014 01:10 PM, Ivan Khoronzhuk wrote:
> At late init all unused clocks are disabled. So clocks that were not
> get before will be gated. In Keysone 2 SoC we have at least one
> necessary clock that is not used by any driver - "msmcsram". This
> clock is necessary, because it supplies the Multicore Shared Memory
> Controller (MSMC). MSMC is the coherency interconnect and all the
> coherent masters are connected to it including devices which are not
> under Linux OS control. MSMC clock should not be touched even in low
> power states.
>
> So drop the clock node, otherwise 'clk_ignore_unused' parameter will
> disable the clock leading to system stall.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com>
> ---
> Based on
> git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux-keystone.git
> keystone/master
>
> v1..v2:
> - dropped "msmcsram" clock node, instead disabling
> - updated comment message
>
Thanks for the update. Would be good to get blessing from DT folks.
I want to send this one during the rc's since its a bug fix.
> arch/arm/boot/dts/keystone-clocks.dtsi | 10 ----------
> 1 file changed, 10 deletions(-)
>
> diff --git a/arch/arm/boot/dts/keystone-clocks.dtsi b/arch/arm/boot/dts/keystone-clocks.dtsi
> index 2363593..98ad488 100644
> --- a/arch/arm/boot/dts/keystone-clocks.dtsi
> +++ b/arch/arm/boot/dts/keystone-clocks.dtsi
> @@ -327,16 +327,6 @@ clocks {
> domain-id = <6>;
> };
>
> - clkmsmcsram: clkmsmcsram {
> - #clock-cells = <0>;
> - compatible = "ti,keystone,psc-clock";
> - clocks = <&chipclk1>;
> - clock-output-names = "msmcsram";
> - reg = <0x02350038 0xb00>, <0x0235001c 0x400>;
> - reg-names = "control", "domain";
> - domain-id = <7>;
> - };
> -
> clkgem0: clkgem0 {
> #clock-cells = <0>;
> compatible = "ti,keystone,psc-clock";
>
^ permalink raw reply
* Re: [PATCH V3 6/8] SPEAr13xx: Fixup: Move SPEAr1340 SATA platform code to phy driver
From: Arnd Bergmann @ 2014-01-31 15:27 UTC (permalink / raw)
To: Pratyush Anand
Cc: Mohit KUMAR DCG, Viresh Kumar, Tejun Heo, Kishon Vijay Abraham I,
spear-devel, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org, linux-ide@vger.kernel.org
In-Reply-To: <20140131041204.GC2618@pratyush-vbox>
On Friday 31 January 2014, Pratyush Anand wrote:
> On Thu, Jan 30, 2014 at 09:21:00PM +0800, Arnd Bergmann wrote:
> > On Thursday 30 January 2014, Mohit Kumar wrote:
> > >
> > > diff --git a/Documentation/devicetree/bindings/phy/spear13xx-miphy.txt b/Documentation/devicetree/bindings/phy/spear13xx-miphy.txt
> > > new file mode 100644
> > > index 0000000..208b37d
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/spear13xx-miphy.txt
> > > @@ -0,0 +1,8 @@
> > > +Required properties:
> > > +- compatible : should be "st,spear1340-sata-pcie-phy".
> >
> > Just for confirmation: This phy is by design only capable of driving
> > sata or pcie, but nothing else if reused in a different SoC, right?
> >
> > If the phy is actually more generic than that, I'd suggest changing
> > the name, otherwise it's ok.
>
> OK, we will give a generic name as it can be used for sata/pcie/usb3.0.
> Although, phy register definition may vary from version to version,
> but that can be managed,as and when support of new soc is added.
It probably doesn't hurt to already define a list of possible
modes in the binding that you already know about. That way, you
don't have to update the binding in sync with the driver if you
add another mode, such as USB.
Arnd
^ permalink raw reply
* Re: Extending OPP bindings
From: Mark Brown @ 2014-01-31 15:46 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Nishanth Menon, Mark Rutland, devicetree@vger.kernel.org,
mturquette@linaro.org, linux-pm@vger.kernel.org, Eduardo Valentin,
Rob Herring, Sudeep Holla, grant.likely@linaro.org, Shawn Guo,
Morten Rasmussen, linux-arm-kernel@lists.infradead.org,
Charles Garcia-Tobin
In-Reply-To: <20140131124620.GC2616@e102568-lin.cambridge.arm.com>
[-- Attachment #1: Type: text/plain, Size: 1151 bytes --]
On Fri, Jan 31, 2014 at 12:46:20PM +0000, Lorenzo Pieralisi wrote:
> Indexing does not mean that the index in the DT is the same as the one
> allocated by the OS. Indexing is there to point at specific OPPs from
> different DT nodes, a good example are clock bindings and that's exactly
> how they work.
> Can you provide me with an example where the indexing would go wrong
> please ?
This does depend on what you mean by indexing but if you're talking
about having bindings where you have things that refer to the nth
element in an array in that form then it can become hard to read the
resulting DT and the potential for errors is increased. Changes to the
original table need to take into account all the references to the table
and (especially if the number of entries gets large) it can be prone to
miscounting when checking references.
With OPPs this might happen if new operating points are characterised or
some are removed, for example due to new SoC variants or binning.
> Certainly relying on implicit ordering is not great either, actually I
> would say that it is broken.
Yes, that too - some sort of explict reference does help.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] ARM: dts: k2hk-evm: rename clock node to clocks
From: Grygorii Strashko @ 2014-01-31 16:03 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: ivan.khoronzhuk, devicetree, Rob Herring, linux-arm-kernel,
Kumar Gala
In-Reply-To: <52EBB4F6.3050202@ti.com>
On 01/31/2014 04:36 PM, Santosh Shilimkar wrote:
> On Friday 31 January 2014 08:31 AM, Grygorii Strashko wrote:
>> Fix type in clock(s) node name: "clock"-->"clocks".
>>
> s/type/typo ;-)
>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
> No need to resend, I will fix it up while applying it
Thanks
>
>> arch/arm/boot/dts/k2hk-evm.dts | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/k2hk-evm.dts b/arch/arm/boot/dts/k2hk-evm.dts
>> index eaefdfe..c02c3a8 100644
>> --- a/arch/arm/boot/dts/k2hk-evm.dts
>> +++ b/arch/arm/boot/dts/k2hk-evm.dts
>> @@ -15,7 +15,7 @@
>> compatible = "ti,keystone-evm";
>>
>> soc {
>> - clock {
>> + clocks {
>> refclksys: refclksys {
>> #clock-cells = <0>;
>> compatible = "fixed-clock";
>>
>
^ permalink raw reply
* Re: [PATCH v2 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver
From: Mark Brown @ 2014-01-31 16:03 UTC (permalink / raw)
To: Maxime Ripard
Cc: Kevin Hilman, Mike Turquette, Emilio Lopez,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-spi-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, LKML,
kevin.z.m.zh-Re5JQEeQqe8AvxtiuMwx3w,
sunny-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf,
shuge-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf,
zhuzhenhua-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf
In-Reply-To: <20140131081147.GC2950@lukather>
[-- Attachment #1: Type: text/plain, Size: 399 bytes --]
On Fri, Jan 31, 2014 at 09:11:47AM +0100, Maxime Ripard wrote:
> Wouldn't it be better if it was suspended by default, and just waken
> up whenever the framework needs it?
The aim should be to come out of probe in that state if runtime PM is
enabled but don't do it with these hacks, do it by idling the device
at the end of probe for example. Apart from anything else this code
just looks ugly.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: recommended action for bootloaders regarding modifying device-tree nodes
From: Jason Cooper @ 2014-01-31 16:07 UTC (permalink / raw)
To: Tim Harvey
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
u-boot-0aAXYlwwYIKGBzrmiIFOJg,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Grant Likely
In-Reply-To: <CAJ+vNU2zis+1DUCzmegUFw+Z6XATBEPJ+yqMsttp4ENcckHUYA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thu, Jan 30, 2014 at 08:39:00PM -0800, Tim Harvey wrote:
> On Thu, Jan 30, 2014 at 12:45 PM, Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org> wrote:
> > Hi Tim,
> >
> > On Thu, Jan 30, 2014 at 01:11:18AM -0800, Tim Harvey wrote:
> >> My approach has been to define a per-baseboard device-tree in Linux
> >> for a 'fully loaded' board, then remove nodes which the EEPROM claims
> >> are not present in the bootloader before it passes the DTB to the
> >> kernel. I do this by defining aliases in the device-tree for the
> >> peripherals that are 'optional' so that the bootloader itself does not
> >> need to know the details about how the device is connected.
> >
> > This is more of a process question: Is there any information captured
> > in your EEPROM that can't be represented in the dtb? iow, at the point
> > when you write the EEPROM, why not write the dtb to it as configured?
> >
> > You could have pre-configured dtsi fragments for each config option, and
> > then dynamically create the board dts from the order.
> >
> > I only ask because it would solve the problem below. However, there's a
> > lot more to changing a manufacturing process than meets the eye. :)
> >
>
> our eeprom config section is only 40 bytes. It contains a SKU string,
> mac addrs, and some bitwise fields for the various optional components
> that we can subload.
Ok.
> >> Is it more appropriate for the bootloader to 'remove' nodes for
> >> devices that are not physically present or should I be setting their
> >> status property to 'disabled' instead? I'm not clear if either option
> >> really has any pros or cons.
> >
> > That depends on how you have it structured. Is it a valid dtb?
> > Meaning, do you have four nodes all at the same register address?
> > Perhaps you could provide an example dts?
>
> yes its a valid dtb - it is just the superset of everything the
> baseboard (ie schematic design) can support.
>
> A good example is a custom SKU of a baseboard with ethernet subloaded.
> If the EEPROM says there is no ethernet mac or phy, I would want to
> remove or disable the ethernet node from the devicetree.
>
> Another example would be a node for 'gpio-pps' (GPIO based
> pulse-per-second) support. A baseboard design that has a GPS with its
> PPS signal tied to a GPIO would define this in the device-tree, but if
> the EEPROM says the GPS isn't loaded, I would want to remove or
> disable the gps-pps node.
I think JasonG's approach is the way to go (inserting nops). But it's a
matter of preference as long as the output is a valid dtb.
on a side note: Do you still have to tie your dtb to a version of the
kernel?
thx,
Jason.
--
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 v2 08/11] of: Increase MAX_PHANDLE_ARGS
From: Rob Herring @ 2014-01-31 16:24 UTC (permalink / raw)
To: Andreas Herrmann
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Will Deacon,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
Rob Herring, Grant Likely,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <20140130174546.GF13543@alberich>
On Thu, Jan 30, 2014 at 11:45 AM, Andreas Herrmann
<andreas.herrmann-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org> wrote:
> On Wed, Jan 29, 2014 at 12:26:35PM -0500, Suravee Suthikulanit wrote:
>> On 1/29/2014 11:16 AM, Andreas Herrmann wrote:
>> > On Wed, Jan 29, 2014 at 11:59:12AM -0500, Suravee Suthikulanit wrote:
>> >> On 1/29/2014 10:57 AM, Rob Herring wrote:
>> >>>>> diff --git a/include/linux/of.h b/include/linux/of.h
>> >>>>>>> index 276c546..24e1b28 100644
>> >>>>>>> --- a/include/linux/of.h
>> >>>>>>> +++ b/include/linux/of.h
>> >>>>>>> @@ -67,7 +67,7 @@ struct device_node {
>> >>>>>>> #endif
>> >>>>>>> };
>> >>>>>>>
>> >>>>>>> -#define MAX_PHANDLE_ARGS 8
>> >>>>>>> +#define MAX_PHANDLE_ARGS 16
>> >>>>>
>> >>>>>
>> >>>>> Since the MMU-500 specify "Number of SMRs" upto 128 registers, shouldn't
>> >>>>> this be changed to be able to support 128 StreamIDs as well? Although I am
>> >>>>> not sure if this would be too big to have on the stack per Rob's comment in
>> >>>>> the previous patch set.
>> >>> Do you actually need 128 now? If not, then we can deal with that when
>> >>> we get there. There are lots of things in spec's that are not actually
>> >>> implemented or supported.
>> >>
>> >> Actually, we are using 32 on the AMD system. So, do you think we can set
>> >> this to 32 instead?
>> >
>> > I think that's ok.
>> >
>> > But are we really talking about number of SMRs or number of StreamIDs
>> > per master device here? Ie. are you just having 32 SMRs for an SMMU on
>> > your AMD system or do you have master devices which have 32 StreamIDs?
>> >
>> > If it's just number of SMRs we don't need to modify this macro.
>
>> I am referring to the case where each mmu-master can have upto 32 streamID.
>
> Rob,
>
> Do you agree on increasing MAX_PHANDLE_ARGS to 32?
Yes, but more than that will require a closer look. Please get this
into next early in the cycle.
> Or should this be done when someone (e.g. Suravee) submits a DTS
> update with an SMMU node description containing more than 16 stream
> IDs for a master device?
Well, I am inclined to not care having seen no upstream activity for
AMD's platform.
Rob
^ permalink raw reply
* Re: Extending OPP bindings
From: Rob Herring @ 2014-01-31 17:17 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Nishanth Menon, Sudeep Holla, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org,
Mark Rutland, Charles Garcia-Tobin, Rob Herring,
grant.likely@linaro.org, Morten Rasmussen, Shawn Guo,
mturquette@linaro.org, Mark Brown, Eduardo Valentin
In-Reply-To: <20140131124620.GC2616@e102568-lin.cambridge.arm.com>
On Fri, Jan 31, 2014 at 6:46 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> Hi Nishanth,
>
> On Fri, Jan 31, 2014 at 12:43:54AM +0000, Nishanth Menon wrote:
>> Hi Sudeep,
>>
>> On 01/30/2014 07:43 AM, Sudeep Holla wrote:
>>
>> >
>> > I am looking into a couple shortcomings in the current OPP bindings and
>> > how to address them. Feel free to add to the list if you think of any more
>> > issues that needs to be addressed or if and how any problem mentioned below
>> > can be handled with the existing bindings.
>> >
>> > 1. indexing: currently there are no indices in the operating-points.
>> indexing is based on frequency which is why the accessors use
>> frequency to pull out the OPP data.
>>
>> indexing is a horrible idea - on platforms where OPP may be disabled
>> or enabled for various reasons(see arch/arm/mach-imx/mach-imx6q.c,
>> arch/arm/mach-omap2/board-omap3beagle.c etc) - the indexing you see in
>> dts is just a myth that may not exist once the nodes are loaded and
>> operated upon depending on SoC variations (example efuse describing
>> which OPPs can be used and which not).
>
> I do not understand why. As long as a mapping from DT to data structures
> in the kernel is done at boot, I can see how by indexing device nodes
> can refer to specific OPPs. If they are disabled, amen, as long as we
> can point at specific operating points that should be ok.
>
> Indexing does not mean that the index in the DT is the same as the one
> allocated by the OS. Indexing is there to point at specific OPPs from
> different DT nodes, a good example are clock bindings and that's exactly
> how they work.
That is not a good comparison. With clocks, you are describing which
physical signal coming out of a hardware block much like interrupts.
Granted the h/w is not typically documented that way for clocks
although the numbering could correspond to register bit locations as
interrupt numbers do. With OPP indexes, they are a totally made up
software construct.
Perhaps OPPs should be nodes so new fields can be added easily and
then you have a phandle for each OPP.
>
> Can you provide me with an example where the indexing would go wrong
> please ?
Putting s/w indexes into DT. Using cell-index for uarts to define
their tty number was one example.
> Certainly relying on implicit ordering is not great either, actually I
> would say that it is broken.
>
>> That said, the original OPP[1][2] series discussion started by trying
>> to kill indexing precisely for the same reason.. once you have it - it
>> becomes just crazy to deal with.
>
> I could not find any "index killing" :) discussion in there, but I will
> keep reading.
>
>> > It's assumed that the list is either in ascending or descending
>> > order of frequency but not explicit in the binding document.
>> > There are arch/arm/boot/dts/* files with opps in both styles.
>> it should not matter -> opp library should do an insertion sort and
>> organize it in ascending order once all the data is inserted. (line
>> 449ish in opp.c)
>
> That's OS details and they must not be mandated by the bindings.
> We cannot rely on word of mouth for things to work, I do not like
> implicit rules, so the bindings should speficy the ordering or better
> a way to reference OPPs.
>
>> if you see issues with the insertion sort not functioning, it is a bug
>> and should be easy to track down and fix.
>
> Problem is not the insertion sort, problem is that DT bindings do not
> define a way to refer to a specific OPP, unless we do that implicitly
> and again, I rest my case, that is broken.
>
>> > Few other bindings like thermal defines bindings like
>> > cooling-{min,max}-state assuming some order which is broken IMO.
>> Now that you bring it up, I missed it :(.. yeah, I might have
>> preferred it to be min frequency and max_frequency - I agree it is
>> probably broken. I'd let Eduardo comment more about it.
>>
>> >
>> > One such use-case that came up recently[0] is the c-state latencies
>> > which could be different for each OPP. It would be good if the
>> > latencies are specified with the indices to OPP table to avoid
>> > inconsistency between the bindings.
>>
>> You can define C states based on frequencies as well - which really
>> makes sense - since that sounds really like our constraint (say
>> valid-at-frequency "xyz"
>
> I do not think that's generic enough, I do not like the idea of looking
> up frequencies (eg a C-state target_residency does not depend on the
> frequency only - ie voltage and other factors play a role, it really
> depends on an operating point- looking it up by frequency is misleading IMO).
It seems we are trying to fit a square peg into a round hole here.
Rob
^ permalink raw reply
* Re: [PATCH v5 00/20] Armada 370/XP watchdog support
From: Willy Tarreau @ 2014-01-31 17:25 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: Lior Amsalem, devicetree, Jason Cooper, Tawfik Bayouk,
Andrew Lunn, Daniel Lezcano, Wim Van Sebroeck, Arnd Bergmann,
Guenter Roeck, Gregory Clement, Sebastian Hesselbarth,
linux-arm-kernel, linux-watchdog
In-Reply-To: <20140129181949.GB27143@localhost>
On Wed, Jan 29, 2014 at 03:19:50PM -0300, Ezequiel Garcia wrote:
> On Mon, Jan 27, 2014 at 12:27:00PM -0300, Ezequiel Garcia wrote:
> > A new round, mostly fixing some minor nitpicks.
> >
>
> If anyone wants to give this a test, here's a public branch:
>
> https://github.com/MISL-EBU-System-SW/mainline-public/tree/wdt_for_v3.14_v5
Tested-By: Willy Tarreau <w@1wt.eu>
Works like a charm on the mirabox, I've long been missing a hardware
watchdog to save me from unplugging the power cord after some hangs
during development.
Thanks,
Willy
^ permalink raw reply
* Re: [PATCH v7 2/2] mmc: sdhci-msm: Initial support for MSM chipsets
From: Courtney Cavin @ 2014-01-31 17:31 UTC (permalink / raw)
To: Georgi Djakov
Cc: linux-mmc@vger.kernel.org, cjb@laptop.org,
devicetree@vger.kernel.org, grant.likely@linaro.org,
rob.herring@calxeda.com, pawel.moll@arm.com, mark.rutland@arm.com,
swarren@wwwdotorg.org, ijc+devicetree@hellion.org.uk,
galak@codeaurora.org, rob@landley.net, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
subhashj@codeaurora.org
In-Reply-To: <52EA8A07.6080008@mm-sol.com>
On Thu, Jan 30, 2014 at 06:21:11PM +0100, Georgi Djakov wrote:
> Hi,
>
> Apologies for the delayed reply.
>
> On 12/09/2013 07:00 PM, Courtney Cavin wrote:
> > On Wed, Nov 06, 2013 at 04:56:45PM +0100, Georgi Djakov wrote:
> >> This platform driver adds the initial support of Secure
> >> Digital Host Controller Interface compliant controller
> >> found in Qualcomm MSM chipsets.
> >>
> >> Signed-off-by: Georgi Djakov <gdjakov@mm-sol.com>
[...]
> >
> > I couldn't get v6 running without the 5 quirks you submitted in [1].
> > Aren't these also attributes of the controller itself? If so, shouldn't they
> > be part of this patch series, and included here?
> >
>
> Most of the quirks are for older hardware revisions. On what board/chip
> are you trying to run it? I will post v8 shortly. Could you try with it
> please?
I see. This was initially checked on a Rev 1 SoC. I will test v8 on a
Rev 2.
-Courtney
^ permalink raw reply
* Re: [PATCH 07/10] watchdog: xilinx: Fix OF binding
From: Rob Herring @ 2014-01-31 17:33 UTC (permalink / raw)
To: Michal Simek
Cc: linux-kernel@vger.kernel.org, Michal Simek, Wim Van Sebroeck,
Grant Likely, Rob Herring, linux-watchdog,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
In-Reply-To: <26d172891694e4041e73b7dae85a1848adf38034.1391177880.git.michal.simek@xilinx.com>
On Fri, Jan 31, 2014 at 8:18 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> Use of_property_read_u32 functions to clean OF probing.
The subject is a bit misleading as this doesn't really fix anything.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> drivers/watchdog/of_xilinx_wdt.c | 25 ++++++++++---------------
> 1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/watchdog/of_xilinx_wdt.c b/drivers/watchdog/of_xilinx_wdt.c
> index c229cc4..475440a6 100644
> --- a/drivers/watchdog/of_xilinx_wdt.c
> +++ b/drivers/watchdog/of_xilinx_wdt.c
> @@ -147,8 +147,7 @@ static u32 xwdt_selftest(struct xwdt_device *xdev)
> static int xwdt_probe(struct platform_device *pdev)
> {
> int rc;
> - u32 *tmptr;
> - u32 *pfreq;
> + u32 pfreq, enable_once;
> struct resource *res;
> struct xwdt_device *xdev;
> bool no_timeout = false;
> @@ -168,28 +167,24 @@ static int xwdt_probe(struct platform_device *pdev)
> if (IS_ERR(xdev->base))
> return PTR_ERR(xdev->base);
>
> - pfreq = (u32 *)of_get_property(pdev->dev.of_node,
> - "clock-frequency", NULL);
> -
> - if (pfreq == NULL) {
> + rc = of_property_read_u32(pdev->dev.of_node, "clock-frequency", &pfreq);
> + if (rc) {
> dev_warn(&pdev->dev,
> "The watchdog clock frequency cannot be obtained\n");
> no_timeout = true;
You can kill this...
> }
>
> - tmptr = (u32 *)of_get_property(pdev->dev.of_node,
> - "xlnx,wdt-interval", NULL);
> - if (tmptr == NULL) {
> + rc = of_property_read_u32(pdev->dev.of_node, "xlnx,wdt-interval",
> + &xdev->wdt_interval);
> + if (rc) {
> dev_warn(&pdev->dev,
> "Parameter \"xlnx,wdt-interval\" not found\n");
> no_timeout = true;
and this...
> - } else {
> - xdev->wdt_interval = *tmptr;
> }
>
> - tmptr = (u32 *)of_get_property(pdev->dev.of_node,
> - "xlnx,wdt-enable-once", NULL);
> - if (tmptr == NULL) {
> + rc = of_property_read_u32(pdev->dev.of_node, "xlnx,wdt-enable-once",
> + &enable_once);
> + if (!rc && enable_once) {
> dev_warn(&pdev->dev,
> "Parameter \"xlnx,wdt-enable-once\" not found\n");
> watchdog_set_nowayout(xilinx_wdt_wdd, true);
> @@ -201,7 +196,7 @@ static int xwdt_probe(struct platform_device *pdev)
> */
> if (!no_timeout)
and use "if (pfreq && xdev->wdt_interval)" if you initialize pfreq to 0.
> xilinx_wdt_wdd->timeout = 2 * ((1 << xdev->wdt_interval) /
> - *pfreq);
> + pfreq);
Is the wdog really usable if the timeout properties are missing? Seems
like you should fail to probe rather than warn.
Rob
^ permalink raw reply
* Re: [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Andy Gross @ 2014-01-31 17:44 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Srikanth Thokala, dan.j.williams, vinod.koul, michal.simek,
Grant Likely, robh+dt, linux-arm-kernel,
linux-kernel@vger.kernel.org, devicetree, dmaengine
In-Reply-To: <52E2698B.6070001@metafoo.de>
On Fri, Jan 24, 2014 at 02:24:27PM +0100, Lars-Peter Clausen wrote:
> On 01/24/2014 12:16 PM, Srikanth Thokala wrote:
> > Hi Lars,
> >
> > On Thu, Jan 23, 2014 at 4:55 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> >> On 01/22/2014 05:52 PM, Srikanth Thokala wrote:
> >> [...]
> >>> +/**
> >>> + * xilinx_vdma_device_control - Configure DMA channel of the device
> >>> + * @dchan: DMA Channel pointer
> >>> + * @cmd: DMA control command
> >>> + * @arg: Channel configuration
> >>> + *
> >>> + * Return: '0' on success and failure value on error
> >>> + */
> >>> +static int xilinx_vdma_device_control(struct dma_chan *dchan,
> >>> + enum dma_ctrl_cmd cmd, unsigned long arg)
> >>> +{
> >>> + struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan);
> >>> +
> >>> + switch (cmd) {
> >>> + case DMA_TERMINATE_ALL:
> >>> + xilinx_vdma_terminate_all(chan);
> >>> + return 0;
> >>> + case DMA_SLAVE_CONFIG:
> >>> + return xilinx_vdma_slave_config(chan,
> >>> + (struct xilinx_vdma_config *)arg);
> >>
> >> You really shouldn't be overloading the generic API with your own semantics.
> >> DMA_SLAVE_CONFIG should take a dma_slave_config and nothing else.
> >
> > Ok. The driver needs few additional configuration from the slave
> > device like Vertical
> > Size, Horizontal Size, Stride etc., for the DMA transfers, in that case do you
> > suggest me to define a separate dma_ctrl_cmd like the one FSLDMA_EXTERNAL_START
> > defined for Freescale drivers?
>
> In my opinion it is not a good idea to have driver implement a generic API,
> but at the same time let the driver have custom semantics for those API
> calls. It's a bit like having a gpio driver that expects 23 and 42 as the
> values passed to gpio_set_value instead of 0 and 1. It completely defeats
> the purpose of a generic API, namely that you are able to write generic code
> that makes use of the API without having to know about which implementation
> API it is talking to. The dmaengine framework provides the
> dmaengine_prep_interleaved_dma() function to setup two dimensional
> transfers, e.g. take a look at sirf-dma.c or imx-dma.c.
>
The comments in the include/linux/dmaengine.h state that if you have
non-generic, non-fixed configuration then you can just create your own
structure and embed the dma_slave_config. Using the container_of you can get
back your structure.
I agree that we should always use the generic structure if possible, but
sometimes there are some non-standard things that you have to do for your
hardware. I am currently in a bind for adding some quirky features that are
required by peripherals who want to use the QCOM DMA devices.
If the context field in prep_slave_sg and prep_dma_cyclic was exposed to
everyone, that would allow an easy way to pass in hardware specific
configuration without bastardizing the slave_config. I noticed that rapidio is
the only consumer of that field and that they have their own prep function.
If we are not going to allow people to do their own slave_config when they need
to, then we need to remove the comments from the include file and expose the
context to the dmaengine_prep_slave_sg and dmaengine_prep_dma_cyclic.
--
sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
^ 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