* 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 2/4] pinctrl: st: add stid127 support
From: srinivas kandagatla @ 2014-01-31 12:30 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-3-git-send-email-patrice.chotard@st.com>
Looks good for me.
Acked-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
On 30/01/14 14:55, Patrice CHOTARD wrote:
> From: Alexandre TORGUE <alexandre.torgue@st.com>
>
> Add STid127 PIOs (psouth, pwest, peast) in pinctrl.
>
> Signed-off-by: alexandre torgue <alexandre.torgue@st.com>
> ---
> drivers/pinctrl/pinctrl-st.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
> index 9cadc68..01227de 100644
> --- a/drivers/pinctrl/pinctrl-st.c
> +++ b/drivers/pinctrl/pinctrl-st.c
> @@ -336,6 +336,27 @@ static const struct st_pctl_data stih416_data = {
> .alt = 0, .oe = 40, .pu = 50, .od = 60, .rt = 100,
> };
>
> +/* STid127 data */
> +static const struct st_pctl_data stid127_data = {
> + .rt_style = st_retime_style_dedicated,
> + /* reuse stih416 delays as they are identical */
> + .input_delays = stih416_delays,
> + .ninput_delays = 14,
> + /* reuse stih416 delays as they are identical */
> + .output_delays = stih416_delays,
> + .noutput_delays = 14,
> + .alt = 0, .oe = 8, .pu = 10, .od = 12, .rt = 14,
> +};
> +
> +static const struct st_pctl_data stid127_psouth_data = {
> + .rt_style = st_retime_style_dedicated,
> + .input_delays = stid127_delays,
> + .ninput_delays = 14,
> + .output_delays = stid127_delays,
> + .noutput_delays = 14,
> + .alt = 0, .oe = 7, .pu = 9, .od = 11, .rt = 13,
> +};
> +
> /* Low level functions.. */
> static inline int st_gpio_bank(int gpio)
> {
> @@ -1264,6 +1285,10 @@ static struct of_device_id st_pctl_of_match[] = {
> { .compatible = "st,stih416-rear-pinctrl", .data = &stih416_data},
> { .compatible = "st,stih416-fvdp-fe-pinctrl", .data = &stih416_data},
> { .compatible = "st,stih416-fvdp-lite-pinctrl", .data = &stih416_data},
> + { .compatible = "st,stid127-pwest-pinctrl", .data = &stid127_data },
> + { .compatible = "st,stid127-psouth-pinctrl",
> + .data = &stid127_psouth_data },
> + { .compatible = "st,stid127-peast-pinctrl", .data = &stid127_data },
> { /* sentinel */ }
> };
>
>
^ permalink raw reply
* Re: [PATCH v2 5/6] Documentation: devicetree: sja1000: add reg-io-width binding
From: Marc Kleine-Budde @ 2014-01-31 12:29 UTC (permalink / raw)
To: Florian Vaussard, Wolfgang Grandegger
Cc: linux-can, netdev, linux-kernel, Grant Likely, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, devicetree
In-Reply-To: <1391164513-11529-6-git-send-email-florian.vaussard@epfl.ch>
[-- Attachment #1: Type: text/plain, Size: 870 bytes --]
On 01/31/2014 11:35 AM, Florian Vaussard wrote:
> 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>
I think it makes sense to squash into patch 6.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]
^ permalink raw reply
* Re: [PATCH 1/4] ARM: STi: add stid127 soc support
From: srinivas kandagatla @ 2014-01-31 12:27 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: devicetree, Russell King, kernel, Linus Walleij, linux-kernel,
Patrice CHOTARD, Stuart Menefy, alexandre.torgue, Rob Herring,
Grant Likely, Giuseppe Cavallaro, maxime.coquelin
In-Reply-To: <201401301939.08302.arnd@arndb.de>
Hi Arnd,
On 30/01/14 18:39, Arnd Bergmann wrote:
> Actually reading the code in this file shows that the L2 cache
> initialization is the only nonstandard thing in there. We should
> really find a way to get rid of the entire function.
I think this will get rid of lot of code left in board-dt.
>
> Sorry if I missed the initial review, but can you explain
> why this is needed to start with?
On ST SoCs the default value for L2 AUX_CTRL register is 0x0, so we set
the way-size explicit here.
Thanks,
srini
^ permalink raw reply
* [PATCH v3 5/5] ARM: sunxi: Enable A31 SPI and SID in the defconfig
From: Maxime Ripard @ 2014-01-31 10:55 UTC (permalink / raw)
To: Mark Brown, Mike Turquette, Emilio Lopez
Cc: 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, Maxime Ripard
In-Reply-To: <1391165752-1819-1-git-send-email-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
arch/arm/configs/sunxi_defconfig | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
index 3e2259b..b5df4a5 100644
--- a/arch/arm/configs/sunxi_defconfig
+++ b/arch/arm/configs/sunxi_defconfig
@@ -24,6 +24,7 @@ CONFIG_IP_PNP_BOOTP=y
# CONFIG_WIRELESS is not set
CONFIG_DEVTMPFS=y
CONFIG_DEVTMPFS_MOUNT=y
+CONFIG_EEPROM_SUNXI_SID=y
CONFIG_NETDEVICES=y
CONFIG_SUN4I_EMAC=y
# CONFIG_NET_CADENCE is not set
@@ -48,6 +49,8 @@ CONFIG_I2C=y
# CONFIG_I2C_COMPAT is not set
CONFIG_I2C_CHARDEV=y
CONFIG_I2C_MV64XXX=y
+CONFIG_SPI=y
+CONFIG_SPI_SUN6I=y
CONFIG_GPIO_SYSFS=y
# CONFIG_HWMON is not set
CONFIG_WATCHDOG=y
--
1.8.4.2
^ permalink raw reply related
* [PATCH v3 4/5] ARM: sun6i: dt: Add SPI controllers to the A31 DTSI
From: Maxime Ripard @ 2014-01-31 10:55 UTC (permalink / raw)
To: Mark Brown, Mike Turquette, Emilio Lopez
Cc: 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, Maxime Ripard
In-Reply-To: <1391165752-1819-1-git-send-email-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
The A31 has 4 SPI controllers. Add them in the DTSI.
Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
arch/arm/boot/dts/sun6i-a31.dtsi | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 0eea325..57af66f 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -340,6 +340,46 @@
status = "disabled";
};
+ spi0: spi@01c68000 {
+ compatible = "allwinner,sun6i-a31-spi";
+ reg = <0x01c68000 0x1000>;
+ interrupts = <0 65 4>;
+ clocks = <&ahb1_gates 20>, <&spi0_clk>;
+ clock-names = "ahb", "mod";
+ resets = <&ahb1_rst 20>;
+ status = "disabled";
+ };
+
+ spi1: spi@01c69000 {
+ compatible = "allwinner,sun6i-a31-spi";
+ reg = <0x01c69000 0x1000>;
+ interrupts = <0 66 4>;
+ clocks = <&ahb1_gates 21>, <&spi1_clk>;
+ clock-names = "ahb", "mod";
+ resets = <&ahb1_rst 21>;
+ status = "disabled";
+ };
+
+ spi2: spi@01c6a000 {
+ compatible = "allwinner,sun6i-a31-spi";
+ reg = <0x01c6a000 0x1000>;
+ interrupts = <0 67 4>;
+ clocks = <&ahb1_gates 22>, <&spi2_clk>;
+ clock-names = "ahb", "mod";
+ resets = <&ahb1_rst 22>;
+ status = "disabled";
+ };
+
+ spi3: spi@01c6b000 {
+ compatible = "allwinner,sun6i-a31-spi";
+ reg = <0x01c6b000 0x1000>;
+ interrupts = <0 68 4>;
+ clocks = <&ahb1_gates 23>, <&spi3_clk>;
+ clock-names = "ahb", "mod";
+ resets = <&ahb1_rst 23>;
+ status = "disabled";
+ };
+
gic: interrupt-controller@01c81000 {
compatible = "arm,cortex-a7-gic", "arm,cortex-a15-gic";
reg = <0x01c81000 0x1000>,
--
1.8.4.2
^ permalink raw reply related
* [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver
From: Maxime Ripard @ 2014-01-31 10:55 UTC (permalink / raw)
To: Mark Brown, Mike Turquette, Emilio Lopez
Cc: 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, Maxime Ripard
In-Reply-To: <1391165752-1819-1-git-send-email-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
The Allwinner A31 has a new SPI controller IP compared to the older Allwinner
SoCs.
It supports DMA, but the driver only does PIO for now, and DMA will be
supported eventually.
Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
.../devicetree/bindings/spi/spi-sun6i.txt | 24 ++
drivers/spi/Kconfig | 6 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-sun6i.c | 473 +++++++++++++++++++++
4 files changed, 504 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txt
create mode 100644 drivers/spi/spi-sun6i.c
diff --git a/Documentation/devicetree/bindings/spi/spi-sun6i.txt b/Documentation/devicetree/bindings/spi/spi-sun6i.txt
new file mode 100644
index 0000000..21de73d
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-sun6i.txt
@@ -0,0 +1,24 @@
+Allwinner A31 SPI controller
+
+Required properties:
+- compatible: Should be "allwinner,sun6i-a31-spi".
+- reg: Should contain register location and length.
+- interrupts: Should contain interrupt.
+- clocks: phandle to the clocks feeding the SPI controller. Two are
+ needed:
+ - "ahb": the gated AHB parent clock
+ - "mod": the parent module clock
+- clock-names: Must contain the clock names described just above
+- resets: phandle to the reset controller asserting this device in
+ reset
+
+Example:
+
+spi1: spi@01c69000 {
+ compatible = "allwinner,sun6i-a31-spi";
+ reg = <0x01c69000 0x1000>;
+ interrupts = <0 66 4>;
+ clocks = <&ahb1_gates 21>, <&spi1_clk>;
+ clock-names = "ahb", "mod";
+ resets = <&ahb1_rst 21>;
+};
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 5072b71..24f3b85 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -446,6 +446,12 @@ config SPI_SIRF
help
SPI driver for CSR SiRFprimaII SoCs
+config SPI_SUN6I
+ tristate "Allwinner A31 SPI controller"
+ depends on ARCH_SUNXI || COMPILE_TEST
+ help
+ This enables using the SPI controller on the Allwinner A31 SoCs.
+
config SPI_MXS
tristate "Freescale MXS SPI controller"
depends on ARCH_MXS
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 95af48d..13b6ccf 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_SPI_SH_HSPI) += spi-sh-hspi.o
obj-$(CONFIG_SPI_SH_MSIOF) += spi-sh-msiof.o
obj-$(CONFIG_SPI_SH_SCI) += spi-sh-sci.o
obj-$(CONFIG_SPI_SIRF) += spi-sirf.o
+obj-$(CONFIG_SPI_SUN6I) += spi-sun6i.o
obj-$(CONFIG_SPI_TEGRA114) += spi-tegra114.o
obj-$(CONFIG_SPI_TEGRA20_SFLASH) += spi-tegra20-sflash.o
obj-$(CONFIG_SPI_TEGRA20_SLINK) += spi-tegra20-slink.o
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
new file mode 100644
index 0000000..1747892
--- /dev/null
+++ b/drivers/spi/spi-sun6i.c
@@ -0,0 +1,473 @@
+/*
+ * Copyright (C) 2012 - 2014 Allwinner Tech
+ * Pan Nan <pannan-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf@public.gmane.org>
+ *
+ * Copyright (C) 2014 Maxime Ripard
+ * Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+#include <linux/workqueue.h>
+
+#include <linux/spi/spi.h>
+
+#define SUN6I_FIFO_DEPTH 128
+
+#define SUN6I_GBL_CTL_REG 0x04
+#define SUN6I_GBL_CTL_BUS_ENABLE BIT(0)
+#define SUN6I_GBL_CTL_MASTER BIT(1)
+#define SUN6I_GBL_CTL_TP BIT(7)
+#define SUN6I_GBL_CTL_RST BIT(31)
+
+#define SUN6I_TFR_CTL_REG 0x08
+#define SUN6I_TFR_CTL_CPHA BIT(0)
+#define SUN6I_TFR_CTL_CPOL BIT(1)
+#define SUN6I_TFR_CTL_SPOL BIT(2)
+#define SUN6I_TFR_CTL_CS_MASK 0x3
+#define SUN6I_TFR_CTL_CS(cs) (((cs) & SUN6I_TFR_CTL_CS_MASK) << 4)
+#define SUN6I_TFR_CTL_CS_MANUAL BIT(6)
+#define SUN6I_TFR_CTL_CS_LEVEL BIT(7)
+#define SUN6I_TFR_CTL_DHB BIT(8)
+#define SUN6I_TFR_CTL_FBS BIT(12)
+#define SUN6I_TFR_CTL_XCH BIT(31)
+
+#define SUN6I_INT_CTL_REG 0x10
+#define SUN6I_INT_CTL_RF_OVF BIT(8)
+#define SUN6I_INT_CTL_TC BIT(12)
+
+#define SUN6I_INT_STA_REG 0x14
+
+#define SUN6I_FIFO_CTL_REG 0x18
+#define SUN6I_FIFO_CTL_RF_RST BIT(15)
+#define SUN6I_FIFO_CTL_TF_RST BIT(31)
+
+#define SUN6I_FIFO_STA_REG 0x1c
+#define SUN6I_FIFO_STA_RF_CNT_MASK 0x7f
+#define SUN6I_FIFO_STA_RF_CNT_BITS 0
+#define SUN6I_FIFO_STA_TF_CNT_MASK 0x7f
+#define SUN6I_FIFO_STA_TF_CNT_BITS 16
+
+#define SUN6I_CLK_CTL_REG 0x24
+#define SUN6I_CLK_CTL_CDR2_MASK 0xff
+#define SUN6I_CLK_CTL_CDR2(div) (((div) & SUN6I_CLK_CTL_CDR2_MASK) << 0)
+#define SUN6I_CLK_CTL_CDR1_MASK 0xf
+#define SUN6I_CLK_CTL_CDR1(div) (((div) & SUN6I_CLK_CTL_CDR1_MASK) << 8)
+#define SUN6I_CLK_CTL_DRS BIT(12)
+
+#define SUN6I_BURST_CNT_REG 0x30
+#define SUN6I_BURST_CNT(cnt) ((cnt) & 0xffffff)
+
+#define SUN6I_XMIT_CNT_REG 0x34
+#define SUN6I_XMIT_CNT(cnt) ((cnt) & 0xffffff)
+
+#define SUN6I_BURST_CTL_CNT_REG 0x38
+#define SUN6I_BURST_CTL_CNT_STC(cnt) ((cnt) & 0xffffff)
+
+#define SUN6I_TXDATA_REG 0x200
+#define SUN6I_RXDATA_REG 0x300
+
+struct sun6i_spi {
+ struct spi_master *master;
+ void __iomem *base_addr;
+ struct clk *hclk;
+ struct clk *mclk;
+ struct reset_control *rstc;
+
+ struct completion done;
+
+ const u8 *tx_buf;
+ u8 *rx_buf;
+ int len;
+};
+
+static inline u32 sun6i_spi_read(struct sun6i_spi *sspi, u32 reg)
+{
+ return readl(sspi->base_addr + reg);
+}
+
+static inline void sun6i_spi_write(struct sun6i_spi *sspi, u32 reg, u32 value)
+{
+ writel(value, sspi->base_addr + reg);
+}
+
+static inline void sun6i_spi_drain_fifo(struct sun6i_spi *sspi, int len)
+{
+ u32 reg, cnt;
+ u8 byte;
+
+ /* See how much data is available */
+ reg = sun6i_spi_read(sspi, SUN6I_FIFO_STA_REG);
+ reg &= SUN6I_FIFO_STA_RF_CNT_MASK;
+ cnt = reg >> SUN6I_FIFO_STA_RF_CNT_BITS;
+
+ if (len > cnt)
+ len = cnt;
+
+ while (len--) {
+ byte = readb(sspi->base_addr + SUN6I_RXDATA_REG);
+ if (sspi->rx_buf)
+ *sspi->rx_buf++ = byte;
+ }
+}
+
+static inline void sun6i_spi_fill_fifo(struct sun6i_spi *sspi, int len)
+{
+ u8 byte;
+
+ if (len > sspi->len)
+ len = sspi->len;
+
+ while (len--) {
+ byte = sspi->tx_buf ? *sspi->tx_buf++ : 0;
+ writeb(byte, sspi->base_addr + SUN6I_TXDATA_REG);
+ sspi->len--;
+ }
+}
+
+static void sun6i_spi_set_cs(struct spi_device *spi, bool enable)
+{
+ struct sun6i_spi *sspi = spi_master_get_devdata(spi->master);
+ u32 reg;
+
+ reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
+ reg &= ~SUN6I_TFR_CTL_CS_MASK;
+ reg |= SUN6I_TFR_CTL_CS(spi->chip_select);
+
+ if (enable)
+ reg |= SUN6I_TFR_CTL_CS_LEVEL;
+ else
+ reg &= ~SUN6I_TFR_CTL_CS_LEVEL;
+
+ sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
+}
+
+
+static int sun6i_spi_transfer_one(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *tfr)
+{
+ struct sun6i_spi *sspi = spi_master_get_devdata(master);
+ unsigned int mclk_rate, div, timeout;
+ unsigned int tx_len = 0;
+ int ret = 0;
+ u32 reg;
+
+ /* We don't support transfer larger than the FIFO */
+ if (tfr->len > SUN6I_FIFO_DEPTH)
+ return -EINVAL;
+
+ reinit_completion(&sspi->done);
+ sspi->tx_buf = tfr->tx_buf;
+ sspi->rx_buf = tfr->rx_buf;
+ sspi->len = tfr->len;
+
+ /* Clear pending interrupts */
+ sun6i_spi_write(sspi, SUN6I_INT_STA_REG, ~0);
+
+ /* Reset FIFO */
+ sun6i_spi_write(sspi, SUN6I_FIFO_CTL_REG,
+ SUN6I_FIFO_CTL_RF_RST | SUN6I_FIFO_CTL_TF_RST);
+
+ /*
+ * Setup the transfer control register: Chip Select,
+ * polarities, etc.
+ */
+ reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
+
+ if (spi->mode & SPI_CPOL)
+ reg |= SUN6I_TFR_CTL_CPOL;
+ else
+ reg &= ~SUN6I_TFR_CTL_CPOL;
+
+ if (spi->mode & SPI_CPHA)
+ reg |= SUN6I_TFR_CTL_CPHA;
+ else
+ reg &= ~SUN6I_TFR_CTL_CPHA;
+
+ if (spi->mode & SPI_LSB_FIRST)
+ reg |= SUN6I_TFR_CTL_FBS;
+ else
+ reg &= ~SUN6I_TFR_CTL_FBS;
+
+ /*
+ * If it's a TX only transfer, we don't want to fill the RX
+ * FIFO with bogus data
+ */
+ if (sspi->rx_buf)
+ reg &= ~SUN6I_TFR_CTL_DHB;
+ else
+ reg |= SUN6I_TFR_CTL_DHB;
+
+ /* We want to control the chip select manually */
+ reg |= SUN6I_TFR_CTL_CS_MANUAL;
+
+ sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg);
+
+ /* Ensure that we have a parent clock fast enough */
+ mclk_rate = clk_get_rate(sspi->mclk);
+ if (mclk_rate < (2 * spi->max_speed_hz)) {
+ clk_set_rate(sspi->mclk, 2 * spi->max_speed_hz);
+ mclk_rate = clk_get_rate(sspi->mclk);
+ }
+
+ /*
+ * Setup clock divider.
+ *
+ * We have two choices there. Either we can use the clock
+ * divide rate 1, which is calculated thanks to this formula:
+ * SPI_CLK = MOD_CLK / (2 ^ cdr)
+ * Or we can use CDR2, which is calculated with the formula:
+ * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
+ * Wether we use the former or the latter is set through the
+ * DRS bit.
+ *
+ * First try CDR2, and if we can't reach the expected
+ * frequency, fall back to CDR1.
+ */
+ div = mclk_rate / (2 * spi->max_speed_hz);
+ if (div <= (SUN6I_CLK_CTL_CDR2_MASK + 1)) {
+ if (div > 0)
+ div--;
+
+ reg = SUN6I_CLK_CTL_CDR2(div) | SUN6I_CLK_CTL_DRS;
+ } else {
+ div = ilog2(mclk_rate) - ilog2(spi->max_speed_hz);
+ reg = SUN6I_CLK_CTL_CDR1(div);
+ }
+
+ sun6i_spi_write(sspi, SUN6I_CLK_CTL_REG, reg);
+
+ /* Setup the transfer now... */
+ if (sspi->tx_buf)
+ tx_len = tfr->len;
+
+ /* Setup the counters */
+ sun6i_spi_write(sspi, SUN6I_BURST_CNT_REG, SUN6I_BURST_CNT(tfr->len));
+ sun6i_spi_write(sspi, SUN6I_XMIT_CNT_REG, SUN6I_XMIT_CNT(tx_len));
+ sun6i_spi_write(sspi, SUN6I_BURST_CTL_CNT_REG,
+ SUN6I_BURST_CTL_CNT_STC(tx_len));
+
+ /* Fill the TX FIFO */
+ sun6i_spi_fill_fifo(sspi, SUN6I_FIFO_DEPTH);
+
+ /* Enable the interrupts */
+ sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, SUN6I_INT_CTL_TC);
+
+ /* Start the transfer */
+ reg = sun6i_spi_read(sspi, SUN6I_TFR_CTL_REG);
+ sun6i_spi_write(sspi, SUN6I_TFR_CTL_REG, reg | SUN6I_TFR_CTL_XCH);
+
+ timeout = wait_for_completion_timeout(&sspi->done,
+ msecs_to_jiffies(1000));
+ if (!timeout) {
+ ret = -ETIMEDOUT;
+ goto out;
+ }
+
+ sun6i_spi_drain_fifo(sspi, SUN6I_FIFO_DEPTH);
+
+out:
+ sun6i_spi_write(sspi, SUN6I_INT_CTL_REG, 0);
+
+ return ret;
+}
+
+static irqreturn_t sun6i_spi_handler(int irq, void *dev_id)
+{
+ struct sun6i_spi *sspi = dev_id;
+ u32 status = sun6i_spi_read(sspi, SUN6I_INT_STA_REG);
+
+ /* Transfer complete */
+ if (status & SUN6I_INT_CTL_TC) {
+ sun6i_spi_write(sspi, SUN6I_INT_STA_REG, SUN6I_INT_CTL_TC);
+ complete(&sspi->done);
+ return IRQ_HANDLED;
+ }
+
+ return IRQ_NONE;
+}
+
+static int sun6i_spi_runtime_resume(struct device *dev)
+{
+ struct spi_master *master = dev_get_drvdata(dev);
+ struct sun6i_spi *sspi = spi_master_get_devdata(master);
+ int ret;
+
+ ret = clk_prepare_enable(sspi->hclk);
+ if (ret) {
+ dev_err(dev, "Couldn't enable AHB clock\n");
+ goto out;
+ }
+
+ ret = clk_prepare_enable(sspi->mclk);
+ if (ret) {
+ dev_err(dev, "Couldn't enable module clock\n");
+ goto err;
+ }
+
+ ret = reset_control_deassert(sspi->rstc);
+ if (ret) {
+ dev_err(dev, "Couldn't deassert the device from reset\n");
+ goto err2;
+ }
+
+ sun6i_spi_write(sspi, SUN6I_GBL_CTL_REG,
+ SUN6I_GBL_CTL_BUS_ENABLE | SUN6I_GBL_CTL_MASTER | SUN6I_GBL_CTL_TP);
+
+ return 0;
+
+err2:
+ clk_disable_unprepare(sspi->mclk);
+err:
+ clk_disable_unprepare(sspi->hclk);
+out:
+ return ret;
+}
+
+static int sun6i_spi_runtime_suspend(struct device *dev)
+{
+ struct spi_master *master = dev_get_drvdata(dev);
+ struct sun6i_spi *sspi = spi_master_get_devdata(master);
+
+ reset_control_assert(sspi->rstc);
+ clk_disable_unprepare(sspi->mclk);
+ clk_disable_unprepare(sspi->hclk);
+
+ return 0;
+}
+
+static int sun6i_spi_probe(struct platform_device *pdev)
+{
+ struct spi_master *master;
+ struct sun6i_spi *sspi;
+ struct resource *res;
+ int ret = 0, irq;
+
+ 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;
+ }
+
+ platform_set_drvdata(pdev, master);
+ sspi = spi_master_get_devdata(master);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ sspi->base_addr = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(sspi->base_addr))
+ return PTR_ERR(sspi->base_addr);
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "No spi IRQ specified\n");
+ return -ENXIO;
+ }
+
+ ret = devm_request_irq(&pdev->dev, irq, sun6i_spi_handler,
+ 0, "sun6i-spi", sspi);
+ if (ret) {
+ dev_err(&pdev->dev, "Cannot request IRQ\n");
+ return ret;
+ }
+
+ sspi->master = master;
+ master->bus_num = -1;
+ master->set_cs = sun6i_spi_set_cs;
+ master->transfer_one = sun6i_spi_transfer_one;
+ master->num_chipselect = 4;
+ master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST;
+ master->dev.of_node = pdev->dev.of_node;
+ master->auto_runtime_pm = true;
+
+ sspi->hclk = devm_clk_get(&pdev->dev, "ahb");
+ if (IS_ERR(sspi->hclk)) {
+ dev_err(&pdev->dev, "Unable to acquire AHB clock\n");
+ return PTR_ERR(sspi->hclk);
+ }
+
+ sspi->mclk = devm_clk_get(&pdev->dev, "mod");
+ if (IS_ERR(sspi->mclk)) {
+ dev_err(&pdev->dev, "Unable to acquire module clock\n");
+ return PTR_ERR(sspi->mclk);
+ }
+
+ init_completion(&sspi->done);
+
+ sspi->rstc = devm_reset_control_get(&pdev->dev, NULL);
+ if (IS_ERR(sspi->rstc)) {
+ dev_err(&pdev->dev, "Couldn't get reset controller\n");
+ return PTR_ERR(sspi->rstc);
+ }
+
+ 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;
+ }
+ }
+
+ ret = devm_spi_register_master(&pdev->dev, master);
+ if (ret) {
+ dev_err(&pdev->dev, "cannot register SPI master\n");
+ goto err;
+ }
+
+ return 0;
+
+err:
+ if (!pm_runtime_enabled(&pdev->dev))
+ sun6i_spi_runtime_suspend(&pdev->dev);
+
+ pm_runtime_disable(&pdev->dev);
+ return ret;
+}
+
+static int sun6i_spi_remove(struct platform_device *pdev)
+{
+ pm_runtime_disable(&pdev->dev);
+
+ return 0;
+}
+
+static const struct of_device_id sun6i_spi_match[] = {
+ { .compatible = "allwinner,sun6i-a31-spi", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, sun6i_spi_match);
+
+static const struct dev_pm_ops sun6i_spi_pm_ops = {
+ .runtime_resume = sun6i_spi_runtime_resume,
+ .runtime_suspend = sun6i_spi_runtime_suspend,
+};
+
+static struct platform_driver sun6i_spi_driver = {
+ .probe = sun6i_spi_probe,
+ .remove = sun6i_spi_remove,
+ .driver = {
+ .name = "sun6i-spi",
+ .owner = THIS_MODULE,
+ .of_match_table = sun6i_spi_match,
+ .pm = &sun6i_spi_pm_ops,
+ },
+};
+module_platform_driver(sun6i_spi_driver);
+
+MODULE_AUTHOR("Pan Nan <pannan-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf@public.gmane.org>");
+MODULE_AUTHOR("Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
+MODULE_DESCRIPTION("Allwinner A31 SPI controller driver");
+MODULE_LICENSE("GPL");
--
1.8.4.2
^ permalink raw reply related
* [PATCH v3 2/5] ARM: sun6i: dt: Add PLL6 and SPI module clocks
From: Maxime Ripard @ 2014-01-31 10:55 UTC (permalink / raw)
To: Mark Brown, Mike Turquette, Emilio Lopez
Cc: 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, Maxime Ripard
In-Reply-To: <1391165752-1819-1-git-send-email-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
The module clocks in the A31 are still compatible with the A10 one. Add the SPI
module clocks and the PLL6 in the device tree to allow their use by the SPI
controllers.
Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
arch/arm/boot/dts/sun6i-a31.dtsi | 46 ++++++++++++++++++++++++++++++++--------
1 file changed, 37 insertions(+), 9 deletions(-)
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 5256ad9..0eea325 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -73,16 +73,12 @@
clocks = <&osc24M>;
};
- /*
- * This is a dummy clock, to be used as placeholder on
- * other mux clocks when a specific parent clock is not
- * yet implemented. It should be dropped when the driver
- * is complete.
- */
- pll6: pll6 {
+ pll6: clk@01c20028 {
#clock-cells = <0>;
- compatible = "fixed-clock";
- clock-frequency = <0>;
+ compatible = "allwinner,sun6i-a31-pll6-clk";
+ reg = <0x01c20028 0x4>;
+ clocks = <&osc24M>;
+ clock-output-names = "pll6";
};
cpu: cpu@01c20050 {
@@ -182,6 +178,38 @@
"apb2_uart1", "apb2_uart2", "apb2_uart3",
"apb2_uart4", "apb2_uart5";
};
+
+ spi0_clk: clk@01c200a0 {
+ #clock-cells = <0>;
+ compatible = "allwinner,sun4i-mod0-clk";
+ reg = <0x01c200a0 0x4>;
+ clocks = <&osc24M>, <&pll6>;
+ clock-output-names = "spi0";
+ };
+
+ spi1_clk: clk@01c200a4 {
+ #clock-cells = <0>;
+ compatible = "allwinner,sun4i-mod0-clk";
+ reg = <0x01c200a4 0x4>;
+ clocks = <&osc24M>, <&pll6>;
+ clock-output-names = "spi1";
+ };
+
+ spi2_clk: clk@01c200a8 {
+ #clock-cells = <0>;
+ compatible = "allwinner,sun4i-mod0-clk";
+ reg = <0x01c200a8 0x4>;
+ clocks = <&osc24M>, <&pll6>;
+ clock-output-names = "spi2";
+ };
+
+ spi3_clk: clk@01c200ac {
+ #clock-cells = <0>;
+ compatible = "allwinner,sun4i-mod0-clk";
+ reg = <0x01c200ac 0x4>;
+ clocks = <&osc24M>, <&pll6>;
+ clock-output-names = "spi3";
+ };
};
soc@01c00000 {
--
1.8.4.2
^ permalink raw reply related
* [PATCH v3 1/5] clk: sunxi: Add support for PLL6 on the A31
From: Maxime Ripard @ 2014-01-31 10:55 UTC (permalink / raw)
To: Mark Brown, Mike Turquette, Emilio Lopez
Cc: 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, Maxime Ripard
In-Reply-To: <1391165752-1819-1-git-send-email-maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
The A31 has a slightly different PLL6 clock. Add support for this new clock in
our driver.
Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
Documentation/devicetree/bindings/clock/sunxi.txt | 1 +
drivers/clk/sunxi/clk-sunxi.c | 45 +++++++++++++++++++++++
2 files changed, 46 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
index c2cb762..954845c 100644
--- a/Documentation/devicetree/bindings/clock/sunxi.txt
+++ b/Documentation/devicetree/bindings/clock/sunxi.txt
@@ -11,6 +11,7 @@ Required properties:
"allwinner,sun6i-a31-pll1-clk" - for the main PLL clock on A31
"allwinner,sun4i-pll5-clk" - for the PLL5 clock
"allwinner,sun4i-pll6-clk" - for the PLL6 clock
+ "allwinner,sun6i-a31-pll6-clk" - for the PLL6 clock on A31
"allwinner,sun4i-cpu-clk" - for the CPU multiplexer clock
"allwinner,sun4i-axi-clk" - for the AXI clock
"allwinner,sun4i-axi-gates-clk" - for the AXI gates
diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 659e4ea..990ad5d 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -249,7 +249,38 @@ static void sun4i_get_pll5_factors(u32 *freq, u32 parent_rate,
*n = DIV_ROUND_UP(div, (*k+1));
}
+/**
+ * sun6i_a31_get_pll6_factors() - calculates n, k factors for A31 PLL6
+ * PLL6 rate is calculated as follows
+ * rate = parent_rate * n * (k + 1) / 2
+ * parent_rate is always 24Mhz
+ */
+
+static void sun6i_a31_get_pll6_factors(u32 *freq, u32 parent_rate,
+ u8 *n, u8 *k, u8 *m, u8 *p)
+{
+ u8 div;
+
+ /*
+ * We always have 24MHz / 2, so we can just say that our
+ * parent clock is 12MHz.
+ */
+ parent_rate = parent_rate / 2;
+
+ /* Normalize value to a parent_rate multiple (24M / 2) */
+ div = *freq / parent_rate;
+ *freq = parent_rate * div;
+
+ /* we were called to round the frequency, we can now return */
+ if (n == NULL)
+ return;
+
+ *k = div / 32;
+ if (*k > 3)
+ *k = 3;
+ *n = DIV_ROUND_UP(div, (*k+1));
+}
/**
* sun4i_get_apb1_factors() - calculates m, p factors for APB1
@@ -416,6 +447,13 @@ static struct clk_factors_config sun4i_pll5_config = {
.kwidth = 2,
};
+static struct clk_factors_config sun6i_a31_pll6_config = {
+ .nshift = 8,
+ .nwidth = 5,
+ .kshift = 4,
+ .kwidth = 2,
+};
+
static struct clk_factors_config sun4i_apb1_config = {
.mshift = 0,
.mwidth = 5,
@@ -457,6 +495,12 @@ static const struct factors_data sun4i_pll5_data __initconst = {
.getter = sun4i_get_pll5_factors,
};
+static const struct factors_data sun6i_a31_pll6_data __initconst = {
+ .enable = 31,
+ .table = &sun6i_a31_pll6_config,
+ .getter = sun6i_a31_get_pll6_factors,
+};
+
static const struct factors_data sun4i_apb1_data __initconst = {
.table = &sun4i_apb1_config,
.getter = sun4i_get_apb1_factors,
@@ -972,6 +1016,7 @@ free_clkdata:
static const struct of_device_id clk_factors_match[] __initconst = {
{.compatible = "allwinner,sun4i-pll1-clk", .data = &sun4i_pll1_data,},
{.compatible = "allwinner,sun6i-a31-pll1-clk", .data = &sun6i_a31_pll1_data,},
+ {.compatible = "allwinner,sun6i-a31-pll6-clk", .data = &sun6i_a31_pll6_data,},
{.compatible = "allwinner,sun4i-apb1-clk", .data = &sun4i_apb1_data,},
{.compatible = "allwinner,sun4i-mod0-clk", .data = &sun4i_mod0_data,},
{.compatible = "allwinner,sun7i-a20-out-clk", .data = &sun7i_a20_out_data,},
--
1.8.4.2
^ permalink raw reply related
* [PATCH v3 0/5] Add Allwinner A31 SPI controller support
From: Maxime Ripard @ 2014-01-31 10:55 UTC (permalink / raw)
To: Mark Brown, Mike Turquette, Emilio Lopez
Cc: 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, Maxime Ripard
Hi everyone,
This patchset brings support for the SPI controller found in the
Allwinner A31 SoC.
Even though the controller supports DMA, the driver only supports PIO
mode for now. This driver will be used to bring up and test DMA on the
SoC, so support for the DMA will come eventually.
It doesn't support transfer larger than the FIFO size (128 bytes) for
now, I expect it to be fixed in the future.
Thanks!
Maxime
Changes from v2:
- Removed the select on runtime_pm
- Fixed the clock error messages
- Trigger the CS manually, and honour the enable bit in set_cs
- Convert to devm_* functions
- Remove useless clk_disable_unprepare in probe
Changes from v1:
- Switched to using the transfer_one and set_cs callbacks
- Switched to using runtime_pm
- Report an error when we try to do a transfer larger than the FIFO
size, instead of silently timeouting.
- Added a Kconfig symbol
- Move the clock ratio change at transfer time
- Fixed the PLL6 cell size in the DTSI
- A few fixes here and there: typos, etc.
Maxime Ripard (5):
clk: sunxi: Add support for PLL6 on the A31
ARM: sun6i: dt: Add PLL6 and SPI module clocks
spi: sunxi: Add Allwinner A31 SPI controller driver
ARM: sun6i: dt: Add SPI controllers to the A31 DTSI
ARM: sunxi: Enable A31 SPI and SID in the defconfig
Documentation/devicetree/bindings/clock/sunxi.txt | 1 +
.../devicetree/bindings/spi/spi-sun6i.txt | 24 ++
arch/arm/boot/dts/sun6i-a31.dtsi | 86 +++-
arch/arm/configs/sunxi_defconfig | 3 +
drivers/clk/sunxi/clk-sunxi.c | 45 ++
drivers/spi/Kconfig | 6 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-sun6i.c | 473 +++++++++++++++++++++
8 files changed, 630 insertions(+), 9 deletions(-)
create mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txt
create mode 100644 drivers/spi/spi-sun6i.c
--
1.8.4.2
^ permalink raw reply
* [PATCH v2 5/6] Documentation: devicetree: sja1000: add reg-io-width binding
From: Florian Vaussard @ 2014-01-31 10:35 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde
Cc: linux-can, netdev, linux-kernel, florian.vaussard, Grant Likely,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
devicetree
In-Reply-To: <1391164513-11529-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 v2 00/21] pinctrl: mvebu: restructure and remove hardcoded addresses from Dove pinctrl
From: Sebastian Hesselbarth @ 2014-01-31 10:22 UTC (permalink / raw)
To: Linus Walleij
Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Thomas Petazzoni,
Russell King, devicetree@vger.kernel.org,
linux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CACRpkda5HRxpLWWizT_58SJ6Fb=TQpO=Y5yFThpZkUbUYmU8fg@mail.gmail.com>
On 01/31/14 11:17, Linus Walleij wrote:
> On Tue, Jan 28, 2014 at 1:39 AM, Sebastian Hesselbarth
> <sebastian.hesselbarth@gmail.com> wrote:
>
>> This patch set is one required step for Dove to hop into mach-mvebu.
>> Until now, pinctrl-dove was hardcoding some registers that do not
>> directly belong to MPP core registers. This is not compatible with
>> what we want for mach-mvebu.
>
> All this stuff is really nice, so if you can get Thomas to add his
> review-tag on this stuff and maybe have some Tested-by:s
> from other parties, I'd be happy to pull the end result to the
> pinctrl tree, or alternatively provide my ACK for you to take this
> through ARM SoC.
Luckily, we have a "pinctrl/mvebu" subfolder these patches will mess
with. I think it will be better to keep then all together and take
them through the MVEBU branch. This way we can have an eye on it and
base off subsequent patches without bothering you.
I'll come back to request your ACK as soon as we have sorted it out.
Sebastian
^ permalink raw reply
* Re: [PATCH v2 00/21] pinctrl: mvebu: restructure and remove hardcoded addresses from Dove pinctrl
From: Linus Walleij @ 2014-01-31 10:17 UTC (permalink / raw)
To: Sebastian Hesselbarth
Cc: Jason Cooper, Andrew Lunn, Gregory Clement, Thomas Petazzoni,
Russell King, devicetree@vger.kernel.org,
linux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1390869573-27624-1-git-send-email-sebastian.hesselbarth@gmail.com>
On Tue, Jan 28, 2014 at 1:39 AM, Sebastian Hesselbarth
<sebastian.hesselbarth@gmail.com> wrote:
> This patch set is one required step for Dove to hop into mach-mvebu.
> Until now, pinctrl-dove was hardcoding some registers that do not
> directly belong to MPP core registers. This is not compatible with
> what we want for mach-mvebu.
All this stuff is really nice, so if you can get Thomas to add his
review-tag on this stuff and maybe have some Tested-by:s
from other parties, I'd be happy to pull the end result to the
pinctrl tree, or alternatively provide my ACK for you to take this
through ARM SoC.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver
From: Maxime Ripard @ 2014-01-31 8:11 UTC (permalink / raw)
To: Kevin Hilman
Cc: Mark Brown, 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: <CAGa+x854huSmiWmEv2EgOPUgZKcp3iitNaBvKXt8DiEj8msSVg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3608 bytes --]
Hi Kevin,
On Thu, Jan 30, 2014 at 03:52:16PM -0800, Kevin Hilman wrote:
> On Wed, Jan 29, 2014 at 5:32 AM, Maxime Ripard
> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > On Wed, Jan 29, 2014 at 12:25:20PM +0000, Mark Brown wrote:
> >> On Wed, Jan 29, 2014 at 12:10:48PM +0100, Maxime Ripard wrote:
> >>
> >> > +config SPI_SUN6I
> >> > + tristate "Allwinner A31 SPI controller"
> >> > + depends on ARCH_SUNXI || COMPILE_TEST
> >> > + select PM_RUNTIME
> >> > + help
> >> > + This enables using the SPI controller on the Allwinner A31 SoCs.
> >> > +
> >>
> >> A select of PM_RUNTIME is both surprising and odd - why is that there?
> >> The usual idiom is that the device starts out powered up (flagged using
> >> pm_runtime_set_active()) and then runtime PM then suspends it when it's
> >> compiled in. That way if for some reason people want to avoid runtime
> >> PM they can still use the device.
> >
> > Since pm_runtime_set_active and all the pm_runtime* callbacks in
> > general are defined to pretty much empty functions, how the
> > suspend/resume callbacks are called then? Obviously, we need them to
> > be run, hence why I added the select here, but now I'm seeing a
> > construct like what's following acceptable then?
>
> Even with your 'select', The runtime PM callbacks will never be called
> in the current driver. pm_runtime_enable() doesn't do any runtime PM
> transitions. It just allows transitions to happen when they're
> triggered by _get()/_put()/etc.
Actually, pm_runtime_get_sync is called by the SPI framework whenever
the device needs to be used. And pm_runtime_put whenever it's not used
anymore, so the callbacks are actually called.
>
> > pm_runtime_enable(&pdev->dev);
> > if (!pm_runtime_enabled(&pdev->dev))
> > sun6i_spi_runtime_resume(&pdev->dev);
>
> Similarily here, it's not the pm_runtime_enable that will fail when
> runtime PM is disabled (or not built-in), it's a pm_runtime_get_sync()
> that will fail.
In the case where pm_runtime is disabled, pm_runtime_enabled will only
return false, and hence the resume callback will be called. get_sync
will fail too when the framework will call it, but since the device is
already initialized, it's fine I guess.
> What you want is something like this in ->probe()
>
> sun6i_spi_runtime_resume();
> /* now, device is always activated whether or not runtime PM is enabled */
> pm_runtime_enable();
> pm_runtime_set_active(); /* tells runtime PM core device is
> already active */
> pm_runtime_get_sync();
>
> This 'get' will increase the usecount, but not actually call the
> callbacks because we told the RPM core that the device was already
> activated with _set_active().
>
> And then, in ->remove(), you'll want
>
> pm_runtime_put();
> pm_runtime_disable();
>
> And if runtime PM is not enabled in the kernel, then the device will
> be left on (which is kinda what you want if you didn't build runtime
> PM into the kernel.)
Yes, but that also mean that the device is actually on after the
probe, even if it's never going to be used. From what I got reading
the pm_runtime code, the suspend callback is called only whenever you
call _put, so the device will be suspended only after it's been used
the first time, right?
Wouldn't it be better if it was suspended by default, and just waken
up whenever the framework needs it?
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 v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Srikanth Thokala @ 2014-01-31 6:52 UTC (permalink / raw)
To: Srikanth Thokala
Cc: Vinod Koul, Lars-Peter Clausen,
dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
michal.simek-gjFFaj9aHVfQT0dZR+AlfA, Grant Likely,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA,
dmaengine-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CA+mB=1Jwjn+KSO9yFr=LNVjk3khf0b-LLQamYf+nnSF4bODGPA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Vinod,
On Mon, Jan 27, 2014 at 4:36 PM, Srikanth Thokala <sthokal-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> Hi Vinod,
>
> On Sun, Jan 26, 2014 at 7:29 PM, Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>> 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-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> 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 question here i think would be waht this device supports? Is the hardware
>> capable of doing interleaved transfers, then would make sense.
>>
>> While we do try to get users use dma_slave_config, but there will always be
>> someone who have specfic params. If we can generalize then we might want to add
>> to the dma_slave_config as well
>
> There are many configuration parameters which are specific to IP and I
> would like to
> give an overview of some of parameteres here:
>
> 1) Park Mode ('cfg->park'): In Park mode, engine will park on frame
> referenced by
> 'cfg->park_frm', so user will have control on each frame in this mode.
>
> 2) Interrupt Coalesce ('cfg->coalesce'): Used for setting interrupt
> threshold. This value
> determines the number of frame buffers to process. To use this feature,
> 'cfg->frm_cnt_en' should be set.
>
> 3) Frame Synchronization Source ('cfg->ext_fsync'): Can be an
> external/internal frame
> synchronization source. Used to synchronize one channel (MM2S/S2MM) with
> another (S2MM/MM2S) channel.
>
> 4) Genlock Synchronization ('cfg->genlock'): Used to avoid mismatch rate between
> master and slave. In master mode (cfg->master), frames are not dropped and
> slave can drop frames to adjust to master frame rate.
>
> And in future, this Engine being a soft IP, we could expect some more additional
> parameters. Isn't a good idea to have a private member in dma_slave_config for
> sharing additional configuration between slave device and dma engine? Or a new
> dma_ctrl_cmd like FSLDMA_EXTERNAL_START?
Ping?
>
> Srikanth
>
>>
>> --
>> ~Vinod
>> --/EX
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
--
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] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Srikanth Thokala @ 2014-01-31 6:51 UTC (permalink / raw)
To: Vinod Koul
Cc: Srikanth Thokala, Lars-Peter Clausen,
dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
michal.simek-gjFFaj9aHVfQT0dZR+AlfA, Grant Likely,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA,
dmaengine-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140128031324.GH10628-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Hi Vinod,
On Tue, Jan 28, 2014 at 8:43 AM, Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, Jan 27, 2014 at 06:42:36PM +0530, Srikanth Thokala wrote:
>> Hi Lars/Vinod,
>> >> The question here i think would be waht this device supports? Is the hardware
>> >> capable of doing interleaved transfers, then would make sense.
>> >
>> > The hardware does 2D transfers. The parameters for a transfer are height,
>> > width and stride. That's only a subset of what interleaved transfers can be
>> > (xt->num_frames must be one for 2d transfers). But if I remember correctly
>> > there has been some discussion on this in the past and the result of that
>> > discussion was that using interleaved transfers for 2D transfers is
>> > preferred over adding a custom API for 2D transfers.
>>
>> I went through the prep_interleaved_dma API and I see only one descriptor
>> is prepared per API call (i.e. per frame). As our IP supports upto 16 frame
>> buffers (can be more in future), isn't it less efficient compared to the
>> prep_slave_sg where we get a single sg list and can prepare all the descriptors
>> (of non-contiguous buffers) in one go? Correct me, if am wrong and let me
>> know your opinions.
> Well the descriptor maybe one, but that can represent multiple frames, for
> example 16 as in your case. Can you read up the documentation of how multiple
> frames are passed. Pls see include/linux/dmaengine.h
>
> /**
> * Interleaved Transfer Request
> * ----------------------------
> * A chunk is collection of contiguous bytes to be transfered.
> * The gap(in bytes) between two chunks is called inter-chunk-gap(ICG).
> * ICGs may or maynot change between chunks.
> * A FRAME is the smallest series of contiguous {chunk,icg} pairs,
> * that when repeated an integral number of times, specifies the transfer.
> * A transfer template is specification of a Frame, the number of times
> * it is to be repeated and other per-transfer attributes.
> *
> * Practically, a client driver would have ready a template for each
> * type of transfer it is going to need during its lifetime and
> * set only 'src_start' and 'dst_start' before submitting the requests.
> *
> *
> * | Frame-1 | Frame-2 | ~ | Frame-'numf' |
> * |====....==.===...=...|====....==.===...=...| ~ |====....==.===...=...|
> *
> * == Chunk size
> * ... ICG
> */
Yes, it can handle multiple frames specified by 'numf' each of size
'frame_size * sgl[0].size'.
But, I see it only works if all the frames' memory is contiguous and
in this case we
can just increment 'src_start' by the total frame size 'numf' number
of times to fill in
for each HW descriptor (each frame is one HW descriptor). So, there
is no issue when the
memory is contiguous. If the frames are non contiguous, we have to
call this API for each
frame (hence for each descriptor), as the src_start for each frame is
different. Is it correct?
FYI: This hardware has an inbuilt Scatter-Gather engine.
Srikanth
>
> --
> ~Vinod
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
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 v5 4/4] [media] exynos-scaler: Add DT bindings for SCALER driver
From: Shaik Ameer Basha @ 2014-01-31 6:47 UTC (permalink / raw)
To: Tomasz Figa
Cc: Shaik Ameer Basha, LMML, linux-samsung-soc, devicetree,
linux-arm-kernel, Sylwester Nawrocki, Pawel Osciak, Hans Verkuil,
Mauro Carvalho Chehab
In-Reply-To: <52E29051.3070906@samsung.com>
Hi Tomasz,
Thanks for the review.
Will consider all your comments in the next version of patch series.
Regards,
Shaik Ameer Basha
On Sat, Jan 25, 2014 at 1:09 AM, Tomasz Figa <t.figa@samsung.com> wrote:
> Hi Shaik,
>
>
> On 09.01.2014 04:28, Shaik Ameer Basha wrote:
>>
>> This patch adds the DT binding documentation for the
>> Exynos5420/5410 based SCALER device driver.
>>
>> Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
>> Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> ---
>> .../devicetree/bindings/media/exynos5-scaler.txt | 22
>> ++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/media/exynos5-scaler.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/exynos5-scaler.txt
>> b/Documentation/devicetree/bindings/media/exynos5-scaler.txt
>> new file mode 100644
>> index 0000000..9328e7d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/exynos5-scaler.txt
>> @@ -0,0 +1,22 @@
>> +* Samsung Exynos5 SCALER device
>> +
>> +SCALER is used for scaling, blending, color fill and color space
>> +conversion on EXYNOS[5420/5410] SoCs.
>> +
>> +Required properties:
>> +- compatible: should be "samsung,exynos5420-scaler" or
>> + "samsung,exynos5410-scaler"
>> +- reg: should contain SCALER physical address location and length
>> +- interrupts: should contain SCALER interrupt number
>
>
> s/number/specifier/
>
>
>> +- clocks: should contain the SCALER clock specifier, from the
>> + common clock bindings
>
>
> s/specifier/phandle and specifier pair for each clock listed in clock-names
> property/
>
> s/from/according to/
>
>
>> +- clock-names: should be "scaler"
>
>
> should contain exactly one entry:
> - "scaler" - IP bus clock.
>
> Also this patch should be first in the series to let the driver added in
> further patches use already present bindings.
>
> Best regards,
> Tomasz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
>
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver
From: Kevin Hilman @ 2014-01-31 5:06 UTC (permalink / raw)
To: Felipe Balbi
Cc: Maxime Ripard, Mark Brown, 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: <20140131022954.GB8163-HgARHv6XitL9zxVx7UNMDg@public.gmane.org>
On Thu, Jan 30, 2014 at 6:29 PM, Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> wrote:
> Hi,
>
> On Thu, Jan 30, 2014 at 03:52:16PM -0800, Kevin Hilman wrote:
>> On Wed, Jan 29, 2014 at 5:32 AM, Maxime Ripard
>> <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>> > On Wed, Jan 29, 2014 at 12:25:20PM +0000, Mark Brown wrote:
>> >> On Wed, Jan 29, 2014 at 12:10:48PM +0100, Maxime Ripard wrote:
>> >>
>> >> > +config SPI_SUN6I
>> >> > + tristate "Allwinner A31 SPI controller"
>> >> > + depends on ARCH_SUNXI || COMPILE_TEST
>> >> > + select PM_RUNTIME
>> >> > + help
>> >> > + This enables using the SPI controller on the Allwinner A31 SoCs.
>> >> > +
>> >>
>> >> A select of PM_RUNTIME is both surprising and odd - why is that there?
>> >> The usual idiom is that the device starts out powered up (flagged using
>> >> pm_runtime_set_active()) and then runtime PM then suspends it when it's
>> >> compiled in. That way if for some reason people want to avoid runtime
>> >> PM they can still use the device.
>> >
>> > Since pm_runtime_set_active and all the pm_runtime* callbacks in
>> > general are defined to pretty much empty functions, how the
>> > suspend/resume callbacks are called then? Obviously, we need them to
>> > be run, hence why I added the select here, but now I'm seeing a
>> > construct like what's following acceptable then?
>>
>> Even with your 'select', The runtime PM callbacks will never be called
>> in the current driver. pm_runtime_enable() doesn't do any runtime PM
>> transitions. It just allows transitions to happen when they're
>> triggered by _get()/_put()/etc.
>>
>> > pm_runtime_enable(&pdev->dev);
>> > if (!pm_runtime_enabled(&pdev->dev))
>> > sun6i_spi_runtime_resume(&pdev->dev);
>>
>> Similarily here, it's not the pm_runtime_enable that will fail when
>> runtime PM is disabled (or not built-in), it's a pm_runtime_get_sync()
>> that will fail.
>>
>> What you want is something like this in ->probe()
>>
>> sun6i_spi_runtime_resume();
>> /* now, device is always activated whether or not runtime PM is enabled */
>> pm_runtime_enable();
>> pm_runtime_set_active(); /* tells runtime PM core device is
>> already active */
>
> shouldn't this be done before pm_runtime_enable() ?
hmm, possibly yes. I was doing this from the top of my head without
looking to closely at the code.
>> pm_runtime_get_sync();
>>
>> This 'get' will increase the usecount, but not actually call the
>> callbacks because we told the RPM core that the device was already
>> activated with _set_active().
>>
>> And then, in ->remove(), you'll want
>>
>> pm_runtime_put();
>
> in ->remove() you actually want a put_sync() right ? You don't want to
> schedule anything since you're just about to disable pm_runtime.
Yes, you're correct.
Thanks for the corrections.
Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" 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
* Pls Confirm
From: Tete Appiah @ 2014-01-31 4:55 UTC (permalink / raw)
Hello,
I am Tete Appiah Head, Personnel and Business Banking in
Ghana. In 2010, A customer made a fixed Number Deposit $14.6Million
This investor died four years ago leaving no WILL.This is an opportunity
for me to claim the said fund through a trustworthy foreigner, If
interested I will give you more details as soon as i received your
respond.
I wait your response,
Tete Appiah
--
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: recommended action for bootloaders regarding modifying device-tree nodes
From: Tim Harvey @ 2014-01-31 4:44 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Jason Cooper, Grant Likely,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
u-boot-0aAXYlwwYIKGBzrmiIFOJg
In-Reply-To: <20140130211512.GD13372-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
On Thu, Jan 30, 2014 at 1:15 PM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
>
> On Thu, Jan 30, 2014 at 03:45:58PM -0500, Jason Cooper wrote:
>
> > 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?
>
> I can share what we do here.. In our systems the serial EEPROM is only
> 256 bytes, so storing things in DT format would be challenging.
>
> What we do is have a master DTB that has the union of all our
> configurations. The boot process has a very simple bit of code that
> runs down the DTB in binary format and replaces entire
> OF_DT_BEGIN_NODE->OF_DT_END_NODE regions with OF_DT_NOP.
>
> The NOP approach is very simple, no other changes (eg offset
> recalculation) needs to be done to the DT, so we can do this process
> with a very small code footprint and without libfdt.
>
> Choosing which sections to drop is done with some combination of
> hardwired code and searching for specific property patterns. There are
> also a few places where placeholder sections are directly fixed up, eg
> a mac address is written into a placeholder of 0s, etc.
>
> So an example might be
>
> optional_peripheral@10000 {
> orc,board-style = <1>;
> [..]
> }
>
> Eg The board-style number comes from the EEPROM and if board-style !=
> 1 then the entire stanza is replaced with NOP.
>
> Jason
Jason,
Sounds pretty much like what we are doing. I am using u-boot and my
current code looks like this:
/*
* Peripheral Config:
* remove nodes by alias path if EEPROM config tells us the
* peripheral is not loaded on the board.
*/
if (!test_bit(EECONFIG_ETH0, info->config))
fdt_del_node_and_alias(blob, "ethernet0");
if (!test_bit(EECONFIG_ETH1, info->config))
fdt_del_node_and_alias(blob, "ethernet1");
if (!test_bit(EECONFIG_HDMI_OUT, info->config))
fdt_del_node_and_alias(blob, "hdmi_out");
if (!test_bit(EECONFIG_SATA, info->config))
fdt_del_node_and_alias(blob, "ahci0");
if (!test_bit(EECONFIG_PCIE, info->config))
fdt_del_node_and_alias(blob, "pcie");
if (!test_bit(EECONFIG_SSI0, info->config))
fdt_del_node_and_alias(blob, "ssi0");
if (!test_bit(EECONFIG_SSI1, info->config))
fdt_del_node_and_alias(blob, "ssi1");
...
I've submitted my code to u-boot and have been asked if its more
appropriate to remove nodes as I'm doing above or to mark them as
'disabled'. From what I can tell there really isn't a rule or
recommendation for this so I think I'll keep doing what I'm doing
above.
Thanks!
Tim
--
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: recommended action for bootloaders regarding modifying device-tree nodes
From: Tim Harvey @ 2014-01-31 4:39 UTC (permalink / raw)
To: Jason Cooper
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
u-boot-0aAXYlwwYIKGBzrmiIFOJg,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Grant Likely
In-Reply-To: <20140130204558.GC29184-u4khhh1J0LxI1Ri9qeTfzeTW4wlIGRCZ@public.gmane.org>
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.
>> 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.
Tim
>
> thx,
>
> Jason.
>
>> Tim Harvey - Principal Software Engineer
>> Gateworks Corporation
>
> btw - one of my first embedded projects was on one of your boards. An
> ixp425 with 4 mini-pci slots.
>
--
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 6/8] SPEAr13xx: Fixup: Move SPEAr1340 SATA platform code to phy driver
From: Pratyush Anand @ 2014-01-31 4:12 UTC (permalink / raw)
To: Arnd Bergmann
Cc: devicetree@vger.kernel.org, Mohit KUMAR DCG, spear-devel,
Kishon Vijay Abraham I, linux-ide@vger.kernel.org, Viresh Kumar,
Tejun Heo, linux-arm-kernel@lists.infradead.org
In-Reply-To: <201401301421.00990.arnd@arndb.de>
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.
>
> > +- reg : offset and length of the PHY register set.
> > +- misc: phandle for the syscon node to access misc registers
> > +- #phy-cells : from the generic PHY bindings, must be 2.
> > + - 1st arg: phandle to the phy node.
> > + - 2nd arg: 0 if phy (in 1st arg) is to be used for sata else 1.
> > + - 3rd arg: Instance id of the phy (in 1st arg).
>
> I would count "arg" differently: There are three cells, and the first
> cell is the phandle, while the second and third cells contain the first
> and second argument.
Ok..will modify accordingly.
>
> The third cell seems redundant, more on that below.
>
> > + ahci0: ahci@b1000000 {
> > compatible = "snps,spear-ahci";
> > reg = <0xb1000000 0x10000>;
> > interrupts = <0 68 0x4>;
> > + phys = <&miphy0 0 0>;
> > + phy-names = "ahci-phy";
> > status = "disabled";
> > };
> >
> > - ahci@b1800000 {
> > + ahci1: ahci@b1800000 {
> > compatible = "snps,spear-ahci";
> > reg = <0xb1800000 0x10000>;
> > interrupts = <0 69 0x4>;
> > + phys = <&miphy1 0 1>;
> > + phy-names = "ahci-phy";
> > status = "disabled";
> > };
> >
> > - ahci@b4000000 {
> > + ahci2: ahci@b4000000 {
> > compatible = "snps,spear-ahci";
> > reg = <0xb4000000 0x10000>;
> > interrupts = <0 70 0x4>;
> > + phys = <&miphy2 0 2>;
> > + phy-names = "ahci-phy";
> > status = "disabled";
> > };
>
> In each case, the number of the phy 'miphyX' is identical to the
> third cell, and I suspect this is by design. In the driver, the
> 'id' field is set in the xlate function, but I could not find any
> place where it actually gets used, so unless you know that it's
> needed, I'd suggest simply removing it.
It has not been used in this patch, as SATA support is currently only
for SPEAr1340, where we have only one instance.
Will be using it in PCIe for SPEAr1310 where 3 instances are present.
>
> Even if you need it, it may be better to have the instance encoded
> in the phy node itself, since it's a property of the phy hardware
> (e.g. if you have to pass the number into a generic register that
> is global to all phys.
Ok..ll do that.
>
> Alternatively, you could have a different representation, where you
> have a single DT device node representing all three PHYs, with
> "reg = <0xeb800000 0xc000>;" In that case, all sata devices would
> point to the same phy node and pass the instance id so the phy
> driver can operated the correct register set.
Instance ID is mainly needed to manipulate wrapper register present
within SPEAr13xx misc space. We have a single register in misc space
having bit fields controlling all 3 phys, and there we need this id.
>
> > +static int spear1340_sata_miphy_init(struct spear13xx_phy_priv *phypriv)
> > +{
> > + regmap_update_bits(phypriv->misc, SPEAR1340_PCIE_SATA_CFG,
> > + SPEAR1340_PCIE_SATA_CFG_MASK, SPEAR1340_SATA_CFG_VAL);
> > + regmap_update_bits(phypriv->misc, SPEAR1340_PCIE_MIPHY_CFG,
> > + SPEAR1340_PCIE_MIPHY_CFG_MASK,
> > + SPEAR1340_PCIE_SATA_MIPHY_CFG_SATA_25M_CRYSTAL_CLK);
> > + /* Switch on sata power domain */
> > + regmap_update_bits(phypriv->misc, SPEAR1340_PCM_CFG,
> > + SPEAR1340_PCM_CFG_SATA_POWER_EN,
> > + SPEAR1340_PCM_CFG_SATA_POWER_EN);
> > + msleep(20);
> > + /* Disable PCIE SATA Controller reset */
> > + regmap_update_bits(phypriv->misc, SPEAR1340_PERIP1_SW_RST,
> > + SPEAR1340_PERIP1_SW_RST_SATA, 0);
> > + msleep(20);
> > +
> > + return 0;
> > +}
>
> I guess some of the parts above can eventually get moved into other
> drivers (reset controller, power domains) that get called directly
> by the SATA driver (e.g. though reset_device()). Since that won't
> impact the PHY binding, it seems fine to leave it here for now.
thanks :)
Regards
Pratyush
>
> Arnd
^ permalink raw reply
* Re: [PATCH v2 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver
From: Felipe Balbi @ 2014-01-31 2:29 UTC (permalink / raw)
To: Kevin Hilman
Cc: Maxime Ripard, Mark Brown, Mike Turquette, Emilio Lopez,
linux-sunxi, linux-spi, linux-arm-kernel,
devicetree@vger.kernel.org, LKML, kevin.z.m.zh, sunny, shuge,
zhuzhenhua
In-Reply-To: <CAGa+x854huSmiWmEv2EgOPUgZKcp3iitNaBvKXt8DiEj8msSVg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2589 bytes --]
Hi,
On Thu, Jan 30, 2014 at 03:52:16PM -0800, Kevin Hilman wrote:
> On Wed, Jan 29, 2014 at 5:32 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Wed, Jan 29, 2014 at 12:25:20PM +0000, Mark Brown wrote:
> >> On Wed, Jan 29, 2014 at 12:10:48PM +0100, Maxime Ripard wrote:
> >>
> >> > +config SPI_SUN6I
> >> > + tristate "Allwinner A31 SPI controller"
> >> > + depends on ARCH_SUNXI || COMPILE_TEST
> >> > + select PM_RUNTIME
> >> > + help
> >> > + This enables using the SPI controller on the Allwinner A31 SoCs.
> >> > +
> >>
> >> A select of PM_RUNTIME is both surprising and odd - why is that there?
> >> The usual idiom is that the device starts out powered up (flagged using
> >> pm_runtime_set_active()) and then runtime PM then suspends it when it's
> >> compiled in. That way if for some reason people want to avoid runtime
> >> PM they can still use the device.
> >
> > Since pm_runtime_set_active and all the pm_runtime* callbacks in
> > general are defined to pretty much empty functions, how the
> > suspend/resume callbacks are called then? Obviously, we need them to
> > be run, hence why I added the select here, but now I'm seeing a
> > construct like what's following acceptable then?
>
> Even with your 'select', The runtime PM callbacks will never be called
> in the current driver. pm_runtime_enable() doesn't do any runtime PM
> transitions. It just allows transitions to happen when they're
> triggered by _get()/_put()/etc.
>
> > pm_runtime_enable(&pdev->dev);
> > if (!pm_runtime_enabled(&pdev->dev))
> > sun6i_spi_runtime_resume(&pdev->dev);
>
> Similarily here, it's not the pm_runtime_enable that will fail when
> runtime PM is disabled (or not built-in), it's a pm_runtime_get_sync()
> that will fail.
>
> What you want is something like this in ->probe()
>
> sun6i_spi_runtime_resume();
> /* now, device is always activated whether or not runtime PM is enabled */
> pm_runtime_enable();
> pm_runtime_set_active(); /* tells runtime PM core device is
> already active */
shouldn't this be done before pm_runtime_enable() ?
> pm_runtime_get_sync();
>
> This 'get' will increase the usecount, but not actually call the
> callbacks because we told the RPM core that the device was already
> activated with _set_active().
>
> And then, in ->remove(), you'll want
>
> pm_runtime_put();
in ->remove() you actually want a put_sync() right ? You don't want to
schedule anything since you're just about to disable pm_runtime.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 00/21] pinctrl: mvebu: restructure and remove hardcoded addresses from Dove pinctrl
From: Sebastian Hesselbarth @ 2014-01-31 2:18 UTC (permalink / raw)
To: Andrew Lunn
Cc: Jason Cooper, Gregory Clement, Thomas Petazzoni, Russell King,
Linus Walleij, devicetree, linux-doc, linux-arm-kernel,
linux-kernel
In-Reply-To: <20140130202500.GN10864@lunn.ch>
On 01/30/2014 09:25 PM, Andrew Lunn wrote:
> On Thu, Jan 30, 2014 at 07:50:34PM +0100, Sebastian Hesselbarth wrote:
>> On 01/30/2014 07:29 PM, Andrew Lunn wrote:
>>> On Tue, Jan 28, 2014 at 01:39:12AM +0100, Sebastian Hesselbarth wrote:
>>>> This patch set is one required step for Dove to hop into mach-mvebu.
>>>> Until now, pinctrl-dove was hardcoding some registers that do not
>>>> directly belong to MPP core registers. This is not compatible with
>>>> what we want for mach-mvebu.
>>>
>>> I think there might be something wrong here....
>>
>> There _is_ something wrong. I'll have a look at it. For the record,
>> what SoC are you testing with? From the base address, I guess it is
>> Kirkwood?
>
> Yes, Kirkwood. Sorry for not saying.
This time I push a branch before sending out the patches. Also, I
think I'll postpone removal of hardcoded addresses until this is
sorted out. The patch set was growing way to quick and I have to
do this step-by-step for me and everybody else to actually understand ;)
So, at least the MVEBU guys should test the following branch on
their SoCs. Again, I have tested Dove and now confirmed that settings
are still correct. The others are compile-tested.
https://github.com/shesselba/linux-dove.git unstable/mvebu-pinctrl-v3.14_v3
@Thomas, Gregory: Do you think that the above branch will be
restructured enough allow support for orion5x and mv78xx0? I had a
quick look at mach-{orion5x,mv78xx0}/mpp.h and didn't see anything
weird.
Sebastian
^ permalink raw reply
* Re: Extending OPP bindings
From: Nishanth Menon @ 2014-01-31 0:43 UTC (permalink / raw)
To: Sudeep Holla, devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org,
Lorenzo Pieralisi, Mark Rutland, Charles Garcia-Tobin,
Rob Herring, grant.likely@linaro.org, Morten Rasmussen, Shawn Guo,
mturquette@linaro.org, Mark Brown, Eduardo Valentin,
Sudeep KarkadaNagesha
In-Reply-To: <52EA570A.8040501@arm.com>
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).
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.
> 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)
if you see issues with the insertion sort not functioning, it is a bug
and should be easy to track down and fix.
> 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"
>
> 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 :(
>
> 2. sharing opps: I have tried to address this issue previously[1] but unable
> to conclude yet on this.
yes - more details in [3] - which is a more interesting discussion
there - lets revive it in that context.
It is a valid concern and IMHO a great idea - yeah we already have a
thread started.
>
> 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.
>
> 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.
>
> (*) 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.
> [0] http://www.spinics.net/lists/arm-kernel/msg301971.html
> [1] http://www.spinics.net/lists/cpufreq/msg07911.html
> [2] http://www.spinics.net/lists/cpufreq/msg09169.html
[1] http://marc.info/?t=125546601600001&r=1&w=2
[2] http://marc.info/?l=linux-omap&m=125474840119392&w=2
[3] http://marc.info/?t=138063448000008&r=1&w=2
[4] http://marc.info/?l=linux-pm&m=138451581304412&w=2
[5]
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/regulator/ti-abb-regulator.c
[6]
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/power/avs/smartreflex.c
--
Regards,
Nishanth Menon
^ 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