* [PATCH v2 4/5] ARM: OMAP2+: Add pdata quirk for sys_clkout2 for omap3 DBB056
From: Nishanth Menon @ 2014-01-22 19:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390417460-3134-5-git-send-email-chf.fritz@googlemail.com>
On 01/22/2014 01:04 PM, Christoph Fritz wrote:
> Full device tree support for clock control is not yet accomplished. Until
> then, configure the 24Mhz of sys_clkout2 to feed an USB-Hub here.
>
> Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
> ---
> arch/arm/mach-omap2/pdata-quirks.c | 37 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
> index a58590f..9ef7ca8 100644
> --- a/arch/arm/mach-omap2/pdata-quirks.c
> +++ b/arch/arm/mach-omap2/pdata-quirks.c
> @@ -171,6 +171,43 @@ static void __init am3517_evm_legacy_init(void)
> }
> static void __init omap3_dbb056_legacy_init(void)
> {
> + struct clk *clkout2;
> + struct clk *cm96fck;
> +
> + /* Reparent clkout2 to 96M_FCK */
> + pr_info("a83x-quirk: Late Reparent clkout2 to 96M_FCK\n");
what'd be a83x?
> + clkout2 = clk_get(NULL, "clkout2_src_ck");
> + if(clkout2 < 0) {
> + pr_err("a83x-quirk: couldn't get clkout2_src_ck\n");
> + return;
> + }
> + cm96fck = clk_get(NULL, "cm_96m_fck");
> + if(cm96fck < 0) {
> + pr_err("a83x-quirk: couldn't get cm_96m_fck\n");
> + return;
> + }
> + if(clk_set_parent(clkout2, cm96fck) < 0) {
> + pr_err("a83x-quirk: couldn't reparent clkout2_src_ck\n");
> + return;
> + }
yep - we have bunch of similar code in drivers/clk/ti -> but we'd need
a generic property to handle this.
> +
> + /* Set clkout2 to 24MHz for internal usb hub*/
> + pr_info("a83x-quirk: Set clkout2 to 24MHz for internal usb hub\n");
> + clkout2 = clk_get(NULL, "sys_clkout2");
> + if(clkout2 < 0) {
> + pr_err("a83x-quirk: couldn't get sys_clkout2\n");
> + return;
> + }
> + if(clk_set_rate(clkout2, 24000000) < 0) {
same here.
> + printk(KERN_ERR "board-omap3evm: couldn't set sys_clkout2 rate\n");
"board-omap3evm:" copy paste?
any reason why not pr_err?
> + return;
> + }
> + if(clk_prepare_enable(clkout2) < 0) {
> + pr_err("a83x-quirk: couldn't enable sys_clkout2\n");
> + return;
> + }
> +
> + /* Initialize display */
> omap3_dbb056_display_init_of();
> }
> #endif /* CONFIG_ARCH_OMAP3 */
>
looking at the coding style, I assume we'd missed running
checkpatch.pl --strict?
ERROR: space required before the open parenthesis '('
#44: FILE: arch/arm/mach-omap2/pdata-quirks.c:180:
+ if(clkout2 < 0) {
ERROR: space required before the open parenthesis '('
#49: FILE: arch/arm/mach-omap2/pdata-quirks.c:185:
+ if(cm96fck < 0) {
ERROR: space required before the open parenthesis '('
#53: FILE: arch/arm/mach-omap2/pdata-quirks.c:189:
+ if(clk_set_parent(clkout2, cm96fck) < 0) {
ERROR: space required before the open parenthesis '('
#61: FILE: arch/arm/mach-omap2/pdata-quirks.c:197:
+ if(clkout2 < 0) {
ERROR: space required before the open parenthesis '('
#65: FILE: arch/arm/mach-omap2/pdata-quirks.c:201:
+ if(clk_set_rate(clkout2, 24000000) < 0) {
WARNING: Prefer netdev_err(netdev, ... then dev_err(dev, ... then
pr_err(... to printk(KERN_ERR ...
#66: FILE: arch/arm/mach-omap2/pdata-quirks.c:202:
+ printk(KERN_ERR "board-omap3evm: couldn't set sys_clkout2 rate\n");
ERROR: space required before the open parenthesis '('
#69: FILE: arch/arm/mach-omap2/pdata-quirks.c:205:
+ if(clk_prepare_enable(clkout2) < 0) {
total: 6 errors, 1 warnings, 0 checks, 43 lines checked
has style problems, please review.
If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
you might want to run something like aiaiai[1] or
kernel_patch_verify[2] or patman... etc.. to help you clean up for
minimum needs.
[1] https://lwn.net/Articles/488992/
[2] https://github.com/nmenon/kernel_patch_verify
--
Regards,
Nishanth Menon
^ permalink raw reply
* [PATCH] dts: edmqmx6: some fixes
From: Silvio F @ 2014-01-22 19:39 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
please review and add this patches.
Thanks and Cheers,
Silvio F.
^ permalink raw reply
* [PATCH 1/2] ARM: dts: imx6: edmqmx6: fix wrong usbotg pingroup
From: Silvio F @ 2014-01-22 19:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390419567-2643-1-git-send-email-silvio.fricke@gmail.com>
Signed-off-by: Silvio F <silvio.fricke@gmail.com>
---
arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts b/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts
index b8a1ff4..6706d1b 100644
--- a/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts
+++ b/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts
@@ -266,7 +266,7 @@
};
pinctrl_usbotg: usbotggrp {
- fsl,pins = <MX6QDL_USBOTG_PINGRP1>;
+ fsl,pins = <MX6QDL_USBOTG_PINGRP2>;
};
pinctrl_usdhc3: usdhc3grp {
--
1.8.5.2
^ permalink raw reply related
* [PATCH 2/2] ARM: dts: imx6: edmqmx6: remove wrong gpio in usb_otg_vbus regulator
From: Silvio F @ 2014-01-22 19:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390419567-2643-1-git-send-email-silvio.fricke@gmail.com>
Signed-off-by: Silvio F <silvio.fricke@gmail.com>
---
arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts b/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts
index 6706d1b..de54e46 100644
--- a/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts
+++ b/arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts
@@ -46,7 +46,6 @@
regulator-name = "usb_otg_vbus";
regulator-min-microvolt = <5000000>;
regulator-max-microvolt = <5000000>;
- gpio = <&gpio7 12 0>;
};
reg_usb_host1: regulator at 2 {
--
1.8.5.2
^ permalink raw reply related
* [PATCH v2 1/5] ARM: dts: omap3: Add support for INCOstartec a83x module
From: Nishanth Menon @ 2014-01-22 19:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390417460-3134-2-git-send-email-chf.fritz@googlemail.com>
On 01/22/2014 01:04 PM, Christoph Fritz wrote:
> INCOstartec LILLY-A83X module is a TI DM3730xx100 (OMAP3) SoC
> computer-on-module.
>
> This patch adds device tree support for most of its features.
>
> Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
> ---
> arch/arm/boot/dts/omap3-lilly-a83x.dtsi | 445 +++++++++++++++++++++++++++++++
> 1 file changed, 445 insertions(+)
> create mode 100644 arch/arm/boot/dts/omap3-lilly-a83x.dtsi
>
> diff --git a/arch/arm/boot/dts/omap3-lilly-a83x.dtsi b/arch/arm/boot/dts/omap3-lilly-a83x.dtsi
> new file mode 100644
> index 0000000..5e2137a
> --- /dev/null
> +++ b/arch/arm/boot/dts/omap3-lilly-a83x.dtsi
> @@ -0,0 +1,445 @@
> +/*
> + * Copyright (C) 2014 Christoph Fritz <chf.fritzc@googlemail.com>
> + *
> + * 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 "omap36xx.dtsi"
> +
> +/ {
> + model = "INCOstartec LILLY-A83X module (DM3730)";
> + compatible = "incostartec,omap3-lilly-a83x", "ti,omap36xx", "ti,omap3";
> +
> + chosen {
> + bootargs = "console=ttyO0,115200n8 vt.global_cursor_default=0 consoleblank=0";
> + };
> +
> + memory {
> + device_type = "memory";
> + reg = <0x80000000 0x8000000>; /* 128 MB */
> + };
> +
> + leds {
> + compatible = "gpio-leds";
> +
> + heartbeat1 {
> + label = "lilly-a83x::led1";
> + gpios = <&gpio1 29 GPIO_ACTIVE_LOW>;
> + linux,default-trigger = "default-on";
> + };
> +
> + };
> +
> + sound {
> + compatible = "ti,omap-twl4030";
> + ti,model = "lilly-a83x";
> +
> + ti,mcbsp = <&mcbsp2>;
> + ti,codec = <&twl_audio>;
> + };
> +
> + regulators {
> + compatible = "simple-bus";
Shrug, just moving the fixed regulator to root also will do the job,
not sure simple-bus much sense here :(
> + reg_vcc3: vcc3 {
> + compatible = "regulator-fixed";
> + regulator-name = "VCC3";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-always-on;
> + };
> + };
> +
> + hsusb1_phy: hsusb1_phy {
> + compatible = "usb-nop-xceiv";
> + vcc-supply = <®_vcc3>;
> + };
> +};
> +
> +&omap3_pmx_wkup {
> + pinctrl-names = "default";
> +
> + lan9221_pins: pinmux_lan9221_pins {
> + pinctrl-single,pins = <
> + 0x5A (PIN_INPUT | MUX_MODE4) /* gpio_129 */
umm.. you might want to follow the convention as you followed later in
comments.
> + >;
> + };
> +
> + tsc2048_pins: pinmux_tsc2048_pins {
> + pinctrl-single,pins = <
> + 0x16 (PIN_INPUT_PULLUP | MUX_MODE4) /* gpio_8 */
umm.. you might want to follow the convention as you followed later in
comments.
> + >;
> + };
> +
> + mmc1cd_pins: pinmux_mmc1cd_pins {
> + pinctrl-single,pins = <
> + 0x56 (PIN_INPUT | MUX_MODE4) /* gpio_126 */
umm.. you might want to follow the convention as you followed later in
comments.
> + >;
> + };
> +};
> +
> +&omap3_pmx_core {
> + pinctrl-names = "default";
> +
> + gpio1_pins: pinmux_gpio1_pins {
> + pinctrl-single,pins = <
> + 0x5ca (PIN_OUTPUT_PULLDOWN | MUX_MODE4) /* gpio_29 */
umm.. you might want to follow the convention as you followed later in
comments.
> + >;
> + };
> +
> + uart1_pins: pinmux_uart1_pins {
> + pinctrl-single,pins = <
> + 0x14c (PIN_OUTPUT | MUX_MODE0) /* uart1_tx.uart1_tx */
> + 0x14e (PIN_OUTPUT | MUX_MODE0) /* uart1_rts.uart1_rts */
> + 0x150 (PIN_INPUT | MUX_MODE0) /* uart1_cts.uart1_cts */
> + 0x152 (PIN_INPUT | MUX_MODE0) /* uart1_rx.uart1_rx */
> + >;
you may be interested in include/dt-bindings/pinctrl/omap.h
OMAP3_CORE1_IOPAD, OMAP3_CORE2_IOPAD as needed here.
> + };
> +
> + uart2_pins: pinmux_uart2_pins {
> + pinctrl-single,pins = <
> + 0x140 (PIN_OUTPUT | MUX_MODE1) /* mcbsp3_clkx.uart2_tx */
> + 0x142 (PIN_INPUT | MUX_MODE1) /* mcbsp3_fsx.uart2_rx */
> + >;
> + };
> +
> + uart3_pins: pinmux_uart3_pins {
> + pinctrl-single,pins = <
> + 0x16e (PIN_INPUT | MUX_MODE0) /* uart3_rx_irrx.uart3_rx_irrx */
> + 0x170 (PIN_OUTPUT | MUX_MODE0) /* uart3_tx_irtx.uart3_tx_irtx */
> + >;
> + };
> +
> + i2c1_pins: pinmux_i2c1_pins {
> + pinctrl-single,pins = <
> + 0x18a (PIN_INPUT_PULLUP | MUX_MODE0) /* i2c1_scl */
> + 0x18c (PIN_INPUT_PULLUP | MUX_MODE0) /* i2c1_sda */
umm.. you might want to follow the convention as you followed
elsewhere in comments.
> + >;
> + };
> +
> + i2c2_pins: pinmux_i2c2_pins {
> + pinctrl-single,pins = <
> + 0x18e (PIN_INPUT | MUX_MODE0) /* i2c2_scl.i2c2_scl */
> + 0x190 (PIN_INPUT | MUX_MODE0) /* i2c2_sda.i2c2_sda */
> + >;
> + };
> +
> + i2c3_pins: pinmux_i2c3_pins {
> + pinctrl-single,pins = <
> + 0x192 (PIN_INPUT | MUX_MODE0) /* i2c3_scl.i2c3_scl */
> + 0x194 (PIN_INPUT | MUX_MODE0) /* i2c3_sda.i2c3_sda */
> + >;
> + };
> +
> + hsusb1_pins: pinmux_hsusb1_pins {
> + pinctrl-single,pins = <
> + 0x5a8 (PIN_OUTPUT | MUX_MODE3) /* etk_clk.hsusb1_stp */
&omap3_pmx_core2 and OMAP3_CORE2_IOPAD probably here. and probably see
similar usage in other board dtsi.
> + 0x5aa (PIN_INPUT | MUX_MODE3) /* etk_ctl.hsusb1_clk */
> + 0x5ac (PIN_INPUT | MUX_MODE3) /* etk_d0.hsusb1_data0 */
> + 0x5ae (PIN_INPUT | MUX_MODE3) /* etk_d1.hsusb1_data1 */
> + 0x5b0 (PIN_INPUT | MUX_MODE3) /* etk_d2.hsusb1_data2 */
> + 0x5b2 (PIN_INPUT | MUX_MODE3) /* etk_d3.hsusb1_data7 */
> + 0x5b4 (PIN_INPUT | MUX_MODE3) /* etk_d4.hsusb1_data4 */
> + 0x5b6 (PIN_INPUT | MUX_MODE3) /* etk_d5.hsusb1_data5 */
> + 0x5b8 (PIN_INPUT | MUX_MODE3) /* etk_d6.hsusb1_data6 */
> + 0x5ba (PIN_INPUT | MUX_MODE3) /* etk_d7.hsusb1_data3 */
> + 0x5bc (PIN_INPUT | MUX_MODE3) /* etk_d8.hsusb1_dir */
> + 0x5be (PIN_INPUT | MUX_MODE3) /* etk_d9.hsusb1_nxt */
> +
> + /* GPIO 128 controls USB-Hub reset. But USB-Phy its
> + * reset can't be controlled. So we clamp this GPIO to
> + * high (PIN_OFF_OUTPUT_HIGH) to always enable USB-Hub.
> + */
> +
> + 0x1ae (PIN_OUTPUT_PULLUP | PIN_OFF_OUTPUT_HIGH | MUX_MODE4) /* gpio_182 */
> + >;
> + };
> +
> + hsusb_otg_pins: pinmux_hsusb_otg_pins {
> + pinctrl-single,pins = <
> + 0x172 (PIN_INPUT | MUX_MODE0) /* hsusb0_clk.hsusb0_clk */
> + 0x174 (PIN_OUTPUT | MUX_MODE0) /* hsusb0_stp.hsusb0_stp */
> + 0x176 (PIN_INPUT | MUX_MODE0) /* hsusb0_dir.hsusb0_dir */
> + 0x178 (PIN_INPUT | MUX_MODE0) /* hsusb0_nxt.hsusb0_nxt */
> + 0x17a (PIN_INPUT | MUX_MODE0) /* hsusb0_data0.hsusb0_data0 */
> + 0x17c (PIN_INPUT | MUX_MODE0) /* hsusb0_data1.hsusb0_data1 */
> + 0x17e (PIN_INPUT | MUX_MODE0) /* hsusb0_data2.hsusb0_data2 */
> + 0x180 (PIN_INPUT | MUX_MODE0) /* hsusb0_data3.hsusb0_data3 */
> + 0x182 (PIN_INPUT | MUX_MODE0) /* hsusb0_data4.hsusb0_data4 */
> + 0x184 (PIN_INPUT | MUX_MODE0) /* hsusb0_data5.hsusb0_data5 */
> + 0x186 (PIN_INPUT | MUX_MODE0) /* hsusb0_data6.hsusb0_data6 */
> + 0x188 (PIN_INPUT | MUX_MODE0) /* hsusb0_data7.hsusb0_data7 */
> + >;
> + };
> +
> + mmc1_pins: pinmux_mmc1_pins {
> + pinctrl-single,pins = <
> + 0x114 (PIN_INPUT_PULLUP | MUX_MODE0) /* sdmmc1_clk.sdmmc1_clk */
> + 0x116 (PIN_INPUT_PULLUP | MUX_MODE0) /* sdmmc1_cmd.sdmmc1_cmd */
> + 0x118 (PIN_INPUT_PULLUP | MUX_MODE0) /* sdmmc1_dat0.sdmmc1_dat0 */
> + 0x11a (PIN_INPUT_PULLUP | MUX_MODE0) /* sdmmc1_dat1.sdmmc1_dat1 */
> + 0x11c (PIN_INPUT_PULLUP | MUX_MODE0) /* sdmmc1_dat2.sdmmc1_dat2 */
> + 0x11e (PIN_INPUT_PULLUP | MUX_MODE0) /* sdmmc1_dat3.sdmmc1_dat3 */
> + >;
> + };
> +
> + spi2_pins: pinmux_spi2_pins {
> + pinctrl-single,pins = <
> + 0x1a6 (PIN_INPUT_PULLDOWN | MUX_MODE0) /* mcspi2_clk.mcspi2_clk */
> + 0x1a8 (PIN_INPUT_PULLDOWN | MUX_MODE0) /* mcspi2_simo.mcspi2_simo */
> + 0x1aa (PIN_INPUT_PULLDOWN | MUX_MODE0) /* mcspi2_somi.mcspi2_somi */
> + 0x1ac (PIN_OUTPUT | MUX_MODE0) /* mcspi2_cs0.mcspi2_cs0 */
> + >;
> + };
> +};
> +
> +&gpio1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&gpio1_pins>;
> +};
> +
> +&i2c1 {
> + clock-frequency = <2600000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c1_pins>;
> +
> + twl: twl at 48 {
> + reg = <0x48>;
> + interrupts = <7>; /* SYS_NIRQ cascaded to intc */
> + interrupt-parent = <&intc>;
> +
> + twl_audio: audio {
> + compatible = "ti,twl4030-audio";
> + codec {
> + };
> + };
> + };
> +};
> +
> +#include "twl4030.dtsi"
> +#include "twl4030_omap3.dtsi"
> +
> +&twl {
> + vmmc1: regulator-vmmc1 {
> + regulator-always-on;
> + };
> +
> + vdd1: regulator-vdd1 {
> + regulator-always-on;
> + };
> +
> + vdd2: regulator-vdd2 {
> + regulator-always-on;
> + };
I hope you have covered all required regulators here including the
ones you might need for IO.1P8 perhaps?
> +};
> +
> +&i2c2 {
> + clock-frequency = <2600000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c2_pins>;
> +};
> +
> +&i2c3 {
> + clock-frequency = <2600000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c3_pins>;
> + gpiom1: gpio at 20 {
> + compatible = "mcp,mcp23017";
> + gpio-controller;
> + #gpio-cells = <2>;
> + reg = <0x20>;
> + };
> +};
> +
> +&uart1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart1_pins>;
> +};
> +
> +&uart2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart2_pins>;
> +};
> +
> +&uart3 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart3_pins>;
> +};
> +
> +&uart4 {
> + status = "disabled";
> +};
> +
> +&mmc1 {
> + reg = <0x4809c000 0x400>;
little curious as to why this. is that to override length 0x200 to
0x400? that belongs to soc.dtsi then.
> + cd-gpios = <&gpio4 30 IRQ_TYPE_LEVEL_LOW>;
> + cd-inverted;
> + vmmc-supply = <&vmmc1>;
> + bus-width = <4>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&mmc1_pins &mmc1cd_pins>;
> + cap-sdio-irq;
> + cap-sd-highspeed;
> + cap-mmc-highspeed;
> +};
> +
> +&mmc2 {
> + status = "disabled";
> +};
> +
> +&mmc3 {
> + status = "disabled";
> +};
> +
> +&mcspi2 {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&spi2_pins>;
> +
> + tsc2046 at 0 {
> + reg = <0>; /* CS0 */
> + compatible = "ti,tsc2046";
> + interrupt-parent = <&gpio1>;
> + interrupts = <8 0>; /* boot6 / gpio_8 */
> + spi-max-frequency = <1000000>;
> + pendown-gpio = <&gpio1 8 0>;
> + vcc-supply = <®_vcc3>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&tsc2048_pins>;
> +
> + ti,x-min = <300>;
> + ti,x-max = <3000>;
> + ti,y-min = <600>;
> + ti,y-max = <3600>;
> + ti,x-plate-ohms = <80>;
> + ti,pressure-max = <255>;
> + ti,swap-xy;
> +
> + linux,wakeup;
> + };
> +};
> +
> +&usbhsehci {
> + phys = <&hsusb1_phy>;
> +};
> +
> +&usbhshost {
> + pinctrl-names = "default";
> + pinctrl-0 = <&hsusb1_pins>;
> + num-ports = <2>;
> + port1-mode = "ehci-phy";
> +};
> +
> +&usb_otg_hs {
> + pinctrl-names = "default";
> + pinctrl-0 = <&hsusb_otg_pins>;
> + interface-type = <0>;
> + usb-phy = <&usb2_phy>;
> + phys = <&usb2_phy>;
> + phy-names = "usb2-phy";
> + mode = <3>;
> + power = <50>;
> +};
> +
> +&gpmc {
> + ranges = <0 0 0x30000000 0x1000000>,
> + <7 0 0x15000000 0x01000000>;
> +
> + nand at 0,0 {
> + reg = <0 0 0x1000000>;
> + nand-bus-width = <16>;
> + ti,nand-ecc-opt = "bch8";
> + /* no elm on omap3 */
> +
> + gpmc,mux-add-data = <0>;
> + gpmc,device-nand;
> + gpmc,device-width = <2>;
> + gpmc,wait-pin = <0>;
> + gpmc,wait-monitoring-ns = <0>;
> + gpmc,burst-length= <4>;
> + gpmc,cs-on-ns = <0>;
> + gpmc,cs-rd-off-ns = <100>;
> + gpmc,cs-wr-off-ns = <100>;
> + gpmc,adv-on-ns = <0>;
> + gpmc,adv-rd-off-ns = <100>;
> + gpmc,adv-wr-off-ns = <100>;
> + gpmc,oe-on-ns = <5>;
> + gpmc,oe-off-ns = <75>;
> + gpmc,we-on-ns = <5>;
> + gpmc,we-off-ns = <75>;
> + gpmc,rd-cycle-ns = <100>;
> + gpmc,wr-cycle-ns = <100>;
> + gpmc,access-ns = <60>;
> + gpmc,page-burst-access-ns = <5>;
> + gpmc,bus-turnaround-ns = <0>;
> + gpmc,cycle2cycle-samecsen;
> + gpmc,cycle2cycle-delay-ns = <50>;
> + gpmc,wr-data-mux-bus-ns = <75>;
> + gpmc,wr-access-ns = <155>;
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + partition at 0 {
> + label = "MLO";
> + reg = <0 0x80000>;
> + };
> +
> + partition at 0x80000 {
> + label = "u-boot";
> + reg = <0x80000 0x1e0000>;
> + };
> +
> + partition at 0x260000 {
> + label = "u-boot-environment";
> + reg = <0x260000 0x20000>;
> + };
> +
> + partition at 0x280000 {
> + label = "kernel";
> + reg = <0x280000 0x500000>;
> + };
> +
> + partition at 0x780000 {
> + label = "filesystem";
> + reg = <0x780000 0xf880000>;
> + };
> + };
> +
> + ethernet at 7,0 {
> + compatible = "smsc,lan9221", "smsc,lan9115";
> + bank-width = <2>;
> + gpmc,mux-add-data = <2>;
> + gpmc,cs-on-ns = <10>;
> + gpmc,cs-rd-off-ns = <60>;
> + gpmc,cs-wr-off-ns = <60>;
> + gpmc,adv-on-ns = <0>;
> + gpmc,adv-rd-off-ns = <10>;
> + gpmc,adv-wr-off-ns = <10>;
> + gpmc,oe-on-ns = <10>;
> + gpmc,oe-off-ns = <60>;
> + gpmc,we-on-ns = <10>;
> + gpmc,we-off-ns = <60>;
> + gpmc,rd-cycle-ns = <100>;
> + gpmc,wr-cycle-ns = <100>;
> + gpmc,access-ns = <50>;
> + gpmc,page-burst-access-ns = <5>;
> + gpmc,bus-turnaround-ns = <0>;
> + gpmc,cycle2cycle-delay-ns = <75>;
> + gpmc,wr-data-mux-bus-ns = <15>;
> + gpmc,wr-access-ns = <75>;
> + gpmc,cycle2cycle-samecsen;
> + gpmc,cycle2cycle-diffcsen;
> + vddvario-supply = <®_vcc3>;
> + vdd33a-supply = <®_vcc3>;
> + reg-io-width = <4>;
> + interrupt-parent = <&gpio5>;
> + interrupts = <1 0x2>;
> + reg = <7 0 0xff>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&lan9221_pins>;
> + phy-mode = "mii";
> + };
> +};
>
--
Regards,
Nishanth Menon
^ permalink raw reply
* [PATCH] clk: Fix notifier documentation
From: Soren Brinkmann @ 2014-01-22 19:48 UTC (permalink / raw)
To: linux-arm-kernel
Contradicting to documenation, the notifier callbacks do receive
the original clock rate in struct clk_notifier_data.old_rate and the new
frequency struct clk_notifier_data.new_rate, independent of the
notification reason.
This behavior also seems to make more sense, since callbacks can use the
same code to deterimine whether clocks are scaled up or down. Something
which would not even possible in the post-rate-change case if the
behavior was as documented.
Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
Hi Mike,
I am working with some clock notifiers and if my results are correct the
notifiers behave differently from how they are documented.
I think the actual behavior makes more sense than the documented and my original
plan was to change the behavior, but it seems I might get away with a
doc-update.
Thanks,
S?ren
drivers/clk/clk.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 2cf2ea6b77a1..26825db03e64 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1983,20 +1983,11 @@ EXPORT_SYMBOL_GPL(devm_clk_unregister);
* re-enter into the clk framework by calling any top-level clk APIs;
* this will cause a nested prepare_lock mutex.
*
- * Pre-change notifier callbacks will be passed the current, pre-change
- * rate of the clk via struct clk_notifier_data.old_rate. The new,
- * post-change rate of the clk is passed via struct
+ * In all notification cases cases (pre, post and abort rate change) the
+ * original clock rate is passed to the callback via struct
+ * clk_notifier_data.old_rate and the new frequency is passed via struct
* clk_notifier_data.new_rate.
*
- * Post-change notifiers will pass the now-current, post-change rate of
- * the clk in both struct clk_notifier_data.old_rate and struct
- * clk_notifier_data.new_rate.
- *
- * Abort-change notifiers are effectively the opposite of pre-change
- * notifiers: the original pre-change clk rate is passed in via struct
- * clk_notifier_data.new_rate and the failed post-change rate is passed
- * in via struct clk_notifier_data.old_rate.
- *
* clk_notifier_register() must be called from non-atomic context.
* Returns -EINVAL if called with null arguments, -ENOMEM upon
* allocation failure; otherwise, passes along the return value of
--
1.8.5.3
^ permalink raw reply related
* [PATCH 04/11] iommu/arm-smmu: Introduce automatic stream-id-masking
From: Andreas Herrmann @ 2014-01-22 20:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140122152622.GD14108@mudshark.cambridge.arm.com>
On Wed, Jan 22, 2014 at 03:26:22PM +0000, Will Deacon wrote:
> Hi Andreas,
>
> This patch always requires some extra brain cycles when reviewing!
>
> On Thu, Jan 16, 2014 at 12:44:16PM +0000, Andreas Herrmann wrote:
> > Try to determine a mask that can be used for all StreamIDs of a master
> > device. This allows to use just one SMR group instead of
> > number-of-streamids SMR groups for a master device.
> >
> > Changelog:
>
> You can put the change log and notes after the '---' so they don't appear in
> the commit log, although the commit message could probably use a brief
> description of your algorithm.
Yep, will fix this.
> > * Sorting of stream IDs (to make usage of S2CR independend of sequence of
> > stream IDs in DT)
> > - intentionally not implemented
> > - code does not rely on sorting
> > - in fact sorting might make things worse with this simple
> > implementation
> > + Example: master with stream IDs 4, 5, 6, 0xe, 0xf requires 3
> > SMRs when IDs are specified in this sorted order (one to map 4,
> > 5, one to map 6, one to map 0xe, 0xf) but just 2 SMRs when
> > specified as 4, 5, 0xe, 0xf, 6 (one to map 4, 5, 0xe, 0xf and
> > one SMR to map 6)
> > - thus by modifying the DT information you can affect the number of
> > S2CRs required for stream matching
> > => I'd say "use common sense" when specifying stream IDs for a master
> > device in DT.
>
> Then we probably want a comment in the driver helping people work out what
> the best ordering is.
Hmm, yes indeed. Or maybe deliver some tool to calculate the best
ordering. (Doing this in kernel is overkill but calculate it once and
store the result in DTB seems to be the right approach.)
> > @@ -1025,10 +1030,109 @@ static void arm_smmu_domain_destroy(struct iommu_domain *domain)
> > kfree(smmu_domain);
> > }
> >
> > +static int determine_smr_mask(struct arm_smmu_device *smmu,
> > + struct arm_smmu_master *master,
> > + struct arm_smmu_smr *smr, int start, int order)
> > +{
> > + u16 i, zero_bits_mask, one_bits_mask, const_mask;
> > + int nr;
> > +
> > + nr = 1 << order;
> > +
> > + if (nr == 1) {
> > + /* no mask, use streamid to match and be done with it */
> > + smr->mask = 0;
> > + smr->id = master->streamids[start];
> > + return 0;
> > + }
> > +
> > + zero_bits_mask = 0;
> > + one_bits_mask = 0xffff;
> > + for (i = start; i < start + nr; i++) {
> > + zero_bits_mask |= master->streamids[i]; /* const 0 bits */
> > + one_bits_mask &= master->streamids[i]; /* const 1 bits */
> > + }
> > + zero_bits_mask = ~zero_bits_mask;
> > +
> > + /* bits having constant values (either 0 or 1) */
> > + const_mask = zero_bits_mask | one_bits_mask;
> > +
> > + i = hweight16(~const_mask);
> > + if ((1 << i) == nr) {
> > + smr->mask = ~const_mask;
> > + smr->id = one_bits_mask;
>
> This part always confuses me. Why do we check (1 << i) against nr?
(I think, I could change this check to (i == order).)
The reason for this check is:
If there is a mismatch it means that the calculated mask/id pair
either covers
(1) more than nr stream IDs (that were used for the calculation)
(then (1<<i) > nr)
or
(2) less than nr stream IDs (that were used for the calculation)
(then (1<<i) < nr)
Only if there is a match, then we know that the calculated mask/id
pair match exactly nr stream IDs (and the matched IDs are those used
for the calculation).
> In fact, in your example where we have SIDs {4,5,e,f,6}, then we'll
> call this initially with start = 0, order = 2 and try to allocate an
> smr for {4,5,e,f}. That will succeed with mask 1011b and id 0100b,
> but the mask has a hamming weight of 3, which is != nr (2).
Arrgh, which means that the example is complete bogus:
4 0b00100
5 0b00101
0xe 0b01110
0xf 0b01111
calculation of const bit masks:
- bits being always zero: 0xfff0 (zero_bits_mask)
- bits being always one: 0x0004 (one_bits_mask)
- thus for const_mask we get: 0xfff4
hweight16(0x1011) != nr (1 << 2). So the algorithm would refuse to use
the calculated mask/id pair to program an SMR. Another iteration would
be done and one SMR for 4, 5 and after that another SMR for 0xe, 0xf
would be set up. The point is that with mask 1011b and id 0100b not
just 4, 5, 0xe, 0xf would be mapped but in fact all following stream
IDs would be mapped:
0b0100 (4)
0b0101 (5)
0b0110 (6)
0b0111 (7)
0b1100 (0xc)
0b1101 (0xd)
0b1110 (0xe)
0b1111 (0xf)
It's eight stream IDs that would be mapped but we considered only 4
stream IDs for the cacluation. We have to dismiss the mask/id pair.
I really should have written down the mask/id stuff to avoid
this wrong example. For a correct example I should have used
4, 5, 6, 0xc, 0xd.
4 0b00100
5 0b00101
0xc 0b01100
0xd 0b01101
calculation of const bit masks:
- bits being always zero: 0xfff2 (zero_bits_mask)
- bits being always one: 0x0004 (one_bits_mask)
- thus for const_mask we get: 0xfff6
(hweight16(0b1001) = 2 which matches order = 2)
With mask 0b1001 and id 0b0100 following IDs match:
0100
0101
1100
1101
0-3, 8-0xb don't match (bit 2 is not set, but it should)
6, 7, 0xe, 0xf don't match (bit 1 is set, but it shouldn't)
> Where am I getting this wrong?
No.
But my example was crap. So providing a tool to correctly calculate
things is required I think.
> I also still need to convince myself that we can't end up generating
> smrs which match the same SID. Is that what your check above is
> trying to handle?
Yes, that should do the trick.
> > +static int determine_smr_mapping(struct arm_smmu_device *smmu,
> > + struct arm_smmu_master *master,
> > + struct arm_smmu_smr *smrs, int max_smrs)
> > +{
> > + int nr_sid, nr, i, bit, start;
> > +
> > + /*
> > + * This function is called only once -- when a master is added
> > + * to a domain. If master->num_s2crs != 0 then this master
> > + * was already added to a domain.
> > + */
> > + BUG_ON(master->num_s2crs);
>
> I think I'd rather WARN and return -EINVAL. We needn't kill the kernel for
> this.
Agreed.
> > +
> > + start = nr = 0;
> > + nr_sid = master->num_streamids;
> > + do {
> > + /*
> > + * largest power-of-2 number of streamids for which to
> > + * determine a usable mask/id pair for stream matching
> > + */
> > + bit = fls(nr_sid);
>
> If you use __fls...
>
> > + if (!bit)
> > + return 0;
> > +
> > + /*
> > + * iterate over power-of-2 numbers to determine
> > + * largest possible mask/id pair for stream matching
> > + * of next 2**i streamids
> > + */
> > + for (i = bit - 1; i >= 0; i--) {
>
> ... then you don't need this -1.
Ok.
> > + if(!determine_smr_mask(smmu, master,
>
> Cosmetic: space after 'if'.
Oops, sorry.
> > /* It worked! Now, poke the actual hardware */
> > - for (i = 0; i < master->num_streamids; ++i) {
> > + for (i = 0; i < master->num_s2crs; ++i) {
> > u32 reg = SMR_VALID | smrs[i].id << SMR_ID_SHIFT |
> > smrs[i].mask << SMR_MASK_SHIFT;
> > + dev_dbg(smmu->dev, "SMR%d: 0x%x\n", smrs[i].idx, reg);
>
> I think we can drop the dev_dbg statements from this patch.
Ok.
Andreas
^ permalink raw reply
* [PATCH v2 06/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
From: Sebastian Hesselbarth @ 2014-01-22 20:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140122173417.GT18269@obsidianresearch.com>
On 01/22/2014 06:34 PM, Jason Gunthorpe wrote:
> On Wed, Jan 22, 2014 at 01:49:05PM -0300, Ezequiel Garcia wrote:
>>> Looking at this patch in isolation it looks to me like the clear
>>> bridge lines should be replaced with a request_irq (as that does the
>>> clear) - is the request_irq in the wrong spot?
>>
>> In that case, I thought that requesting the IRQ at probe time was enough
>> to ensure the BRIDGE_CAUSE would be cleared by the time the watchdog is
>> started. However, after reading through the irqchip code again, I'm no longer
>> sure this is the case.
>
> The watchdog should ideally be fully stopped before request_irq so
> there is no possible race.
>
>> It looks like the BRIDGE_CAUSE register is cleared when the interruption
>> is acked (which happens in the handler if I understood the code right).
>> So requesting the IRQ is useless...
>
> IMHO, the IRQ stuff should clear out pending edge triggered interrupts
> at request_irq time. It makes no sense to take an interrupt for a
> stale edge event.
>
> I had always assumed the core code did this via irq_gc_ack_clr_bit -
> but I don't see an obvious path..
>
>> Sebastian: If the above is correct, do you think we can add a cause clear to
>> the orion irqchip? (supposing it's harmful) Something like this:
Ezequiel,
irqchip/irq-orion.c does mask all interrupts but you are right, it
should also clear pending interrupts right after that.
So
/* mask all interrupts */
writel(0, gc->reg_base + ORION_BRIDGE_IRQ_MASK);
should become
/* mask and clear all interrupts */
writel(0, gc->reg_base + ORION_BRIDGE_IRQ_MASK);
writel(~0, gc->reg_base + ORION_BRIDGE_IRQ_CAUSE);
Could also be clear on write 0, I'll check that. I already had some
beer, so I'll postpone any patches till tomorrow.
Clearing BRIDGE_CAUSE will only clear all currently pending upstream
IRQs, of course. If WDT IRQ will be re-raised right after that in
BRIDGE_CAUSE depends on the actual HW implementation, i.e. we do no
clear the causing IRQ itself but just what it raised in BRIDGE_CAUSE.
So, you should also clear WDT's irq in the driver yourself to clear a
possible pending upstream BRIDGE_CAUSE.
Sebastian
^ permalink raw reply
* [PATCH 2/2] ehci-platform: Add support for controllers with big-endian regs / descriptors
From: Jonas Gorski @ 2014-01-22 20:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E01BDA.8000606@redhat.com>
Hi,
On Wed, 22 Jan 2014 20:28:26 +0100
Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 01/21/2014 08:39 PM, Florian Fainelli wrote:
> > 2014/1/21 Hans de Goede <hdegoede@redhat.com>:
> >> This uses the already documented devicetree booleans for this.
> >
> > (I would greatly appreciate if you could CC people who gave you
> > feedback on this before)
>
> Will do.
>
> > A more informative commit message would be welcome, along with a
> > reference to which Device Tree binding documentation you are referring
> > to.
>
> I've added a reference to the bindings doc in the commit msg for my next version.
>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >> drivers/usb/host/Kconfig | 3 +++
> >> drivers/usb/host/ehci-platform.c | 33 +++++++++++++++++++++++++++++++--
> >> 2 files changed, 34 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> >> index 237d7b1..4af41f3 100644
> >> --- a/drivers/usb/host/Kconfig
> >> +++ b/drivers/usb/host/Kconfig
> >> @@ -256,6 +256,9 @@ config USB_EHCI_ATH79
> >> config USB_EHCI_HCD_PLATFORM
> >> tristate "Generic EHCI driver for a platform device"
> >> depends on !PPC_OF
> >> + # Support BE on architectures which have readl_be
> >> + select USB_EHCI_BIG_ENDIAN_DESC if (AVR32 || MIPS || MICROBLAZE || SPARC || PPC32 || PPC64)
> >> + select USB_EHCI_BIG_ENDIAN_MMIO if (AVR32 || MIPS || MICROBLAZE || SPARC || PPC32 || PPC64)
> >
> > I do not think this is that simple nor correct for at least Microblaze
> > and MIPS since they can run in either BE or LE mode, and those
> > specific platforms should already do the proper select at the
> > board/SoC level. This *might* be correct for SPARC, PPC32 and PPC64,
> > although I believe some specific PPC64 boards can run in little-endian
> > mode like the P-series, SPARC might too.
> >
> > It seems to me that you should not touch this and keep the existing
> > selects in place, if it turns out that the selects are missing the
> > error messages you added below are catching those misuses.
>
> As discussed with Alan, I will drop these lines from my next version.
>
> >> default n
> >> ---help---
> >> Adds an EHCI host driver for a generic platform device, which
> >> diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
> >> index d8aebc0..5888abb 100644
> >> --- a/drivers/usb/host/ehci-platform.c
> >> +++ b/drivers/usb/host/ehci-platform.c
> >> @@ -55,8 +55,10 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
> >>
> >> hcd->has_tt = pdata->has_tt;
> >> ehci->has_synopsys_hc_bug = pdata->has_synopsys_hc_bug;
> >> - ehci->big_endian_desc = pdata->big_endian_desc;
> >> - ehci->big_endian_mmio = pdata->big_endian_mmio;
> >> + if (pdata->big_endian_desc)
> >> + ehci->big_endian_desc = 1;
> >> + if (pdata->big_endian_mmio)
> >> + ehci->big_endian_mmio = 1;
> >>
> >> if (pdata->pre_setup) {
> >> retval = pdata->pre_setup(hcd);
> >> @@ -142,6 +144,7 @@ static int ehci_platform_probe(struct platform_device *dev)
> >> struct resource *res_mem;
> >> struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev);
> >> struct ehci_platform_priv *priv;
> >> + struct ehci_hcd *ehci;
> >> int err, irq, clk = 0;
> >>
> >> if (usb_disabled())
> >> @@ -177,8 +180,34 @@ static int ehci_platform_probe(struct platform_device *dev)
> >> platform_set_drvdata(dev, hcd);
> >> dev->dev.platform_data = pdata;
> >> priv = hcd_to_ehci_priv(hcd);
> >> + ehci = hcd_to_ehci(hcd);
> >>
> >> if (pdata == &ehci_platform_defaults && dev->dev.of_node) {
> >> + if (of_property_read_bool(dev->dev.of_node, "big-endian-regs"))
> >> + ehci->big_endian_mmio = 1;
> >> +
> >> + if (of_property_read_bool(dev->dev.of_node, "big-endian-desc"))
> >> + ehci->big_endian_desc = 1;
> >> +
> >> + if (of_property_read_bool(dev->dev.of_node, "big-endian"))
> >> + ehci->big_endian_mmio = ehci->big_endian_desc = 1;
> >
> > Ok, so I am confused now, should you update
> > pdata->ehci_big_endian_{desc,mmio} here or is it valid to directly
> > modify ehci->big_endian_{desc,mmio}, is not there any risk to undo
> > what is done in ehci_platform_reset(), or is ehci_platform_reset()
> > only called for non-DT cases?
>
> Both the pdata checks in ehci_platform_reset() and the dt checks here only
> ever set these flags, neither code path clears them. And in the dt case pdata
> will be NULL and vice versa.
If it's safe to set ehci->big_endian_{desc,mmio} from the _probe()
routine, then maybe the pdata sets in _reset() should be moved into here
instead of adding extra cludges/checks into _reset().
>
> >
> >> +
> >> +#ifndef CONFIG_USB_EHCI_BIG_ENDIAN_MMIO
> >> + if (ehci->big_endian_mmio) {
> >> + dev_err(&dev->dev,
> >> + "Error big-endian-regs not compiled in\n");
> >
> > I do not think using the Device Tree property name would be very
> > informative since this is supposed to guard against misconfigurations
> > for both DT and non-DT enabled platforms
>
> Nope this is in a dt only code path.
Then these built-in checks could be done for both paths, as !DT also
has the issue, just doesn't warn/error out with it.
I also agree with Florian that the error message could be better
worded, or should be at least have a ":" after "Error". When seeing
the error message the fist time, I would wonder what "error
big-endian-regs" are, and why I would want to have them ;)
Finally, IS_ENABLED()* allows you to drop those ugly #ifndefs:
if (ehci->big_endian_mmio &&
!IS_ENABLED(CONFIG_USB_EHCI_BIG_ENDIAN_MMIO)) {
dev_err(&dev->dev,
"Error big-endian-regs not compiled in\n");
looks much nicer IMHO ;)
Regards
Jonas
*Yes, I know IS_BUILTIN() would be more formal correct, but IS_ENABLED()
reads IMHO nicer, and I doubt these config symbols will ever go
tristate ;)
^ permalink raw reply
* [PATCH 3/5] ARM: firmware: enable Trusted Foundations by default
From: Stephen Warren @ 2014-01-22 20:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390299016-14105-4-git-send-email-acourbot@nvidia.com>
On 01/21/2014 03:10 AM, Alexandre Courbot wrote:
> As discussed previously (https://lkml.org/lkml/2013/11/26/289), enable
> Trusted Foundation support by default since it already depends on a
> supporting architecture being selected.
>
> Doing so allows us to remove it from tegra_defconfig.
> arch/arm/configs/tegra_defconfig | 1 -
> arch/arm/firmware/Kconfig | 1 +
Can we split out the defconfig and code changes into separate patches?
They need to go through seperate branches, possibly even separate repos.
The defconfig change might not even be necessary; at some point I'll
just rebuild it via "make tegra_defconfig; make savedefconfig" on top of
some linux-next that includes the Kconfig change, and it'll happen
automatically. Still, I guess there's no harm explicitly sending the patch.
^ permalink raw reply
* [PATCH 4/5] ARM: trusted_foundations: implement do_idle()
From: Stephen Warren @ 2014-01-22 20:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390299016-14105-5-git-send-email-acourbot@nvidia.com>
On 01/21/2014 03:10 AM, Alexandre Courbot wrote:
> Support the do_idle() firmware call, which is necessary to properly
> support cpuidle.
> diff --git a/arch/arm/firmware/trusted_foundations.c b/arch/arm/firmware/trusted_foundations.c
> +#define TF_CPU_PM 0xfffffffc
> +#define TF_CPU_PM_LP0 0xffffffe3
> +#define TF_CPU_PM_LP1 0xffffffe6
> +#define TF_CPU_PM_LP1_NO_MC_CLK 0xffffffe5
> +#define TF_CPU_PM_LP2 0xffffffe4
> +#define TF_CPU_PM_LP2_NOFLUSH_L2 0xffffffe7
Hmm. This must be Tegra-specific, not generic to any TF client, since
aren't the names of the suspend states (LP0, LP1, LP2) entirely specific
to Tegra?
^ permalink raw reply
* [PATCH 5/5] ARM: tegra: cpuidle: use firmware call for power down
From: Stephen Warren @ 2014-01-22 20:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390299016-14105-6-git-send-email-acourbot@nvidia.com>
On 01/21/2014 03:10 AM, Alexandre Courbot wrote:
> Invoke the do_idle() firmware call before suspending a CPU so that the
> underlying firmware (if any) can take necessary action.
> diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
> @@ -45,6 +46,8 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev,
>
> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
>
> + call_firmware_op(do_idle);
> +
> cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
>
> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
Don't you need to have the kernel also *not* do something when entering
idle; doesn't the FW op replace some of the register writes that the
kernel would otherwise be doing?
^ permalink raw reply
* [PATCH v8 3/4] ohci-platform: Add support for controllers with big-endian regs / descriptors
From: Sergei Shtylyov @ 2014-01-22 20:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390419177-6481-4-git-send-email-hdegoede@redhat.com>
Hello.
On 01/22/2014 10:32 PM, Hans de Goede wrote:
> Note this commit uses the same devicetree booleans for this as the ones
> already existing in the usb-ehci bindings, see:
> Documentation/devicetree/bindings/usb/usb-ehci.txt
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
[...]
> diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
> index b2d0e1e..71e9d8e 100644
> --- a/drivers/usb/host/ohci-platform.c
> +++ b/drivers/usb/host/ohci-platform.c
[...]
> @@ -164,8 +165,34 @@ static int ohci_platform_probe(struct platform_device *dev)
[...]
> +#ifndef CONFIG_USB_OHCI_BIG_ENDIAN_MMIO
> + if (ohci->flags & OHCI_QUIRK_BE_MMIO) {
> + dev_err(&dev->dev,
> + "Error big-endian-regs not compiled in\n");
> + err = -EINVAL;
> + goto err_put_hcd;
> + }
> +#endif
> +#ifndef CONFIG_USB_OHCI_BIG_ENDIAN_DESC
> + if (ohci->flags & OHCI_QUIRK_BE_DESC) {
> + dev_err(&dev->dev,
> + "Error big-endian-desc not compiled in\n");
Small grammar nits (sorry): some punctuation like comma or semicolon
wouldn't hurt after "Error" word here and above. The same for ehci-platfrom.c.
Also, "support" word wouldn't hurt after the property names in the same message.
WBR, Sergei
^ permalink raw reply
* [PATCH 0/5] ARM: firmware: improvements to Trusted Foundations support
From: Stephen Warren @ 2014-01-22 20:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390299016-14105-1-git-send-email-acourbot@nvidia.com>
On 01/21/2014 03:10 AM, Alexandre Courbot wrote:
> These (mostly minor) patches fix a few typos, improve points that
> were agreed upon when the Trusted Foundation series was initially
> submitted, and more importantly add support for the do_idle() firmware
> operation that is needed for cpuidle to be supported. Tegra's cpuidle
> driver is also updated accordingly.
>
> These patches should be the last step before the device trees for NVIDIA
> SHIELD and Tegra Note 7 can be submitted.
Russell, once these patches are reviewed, should Alex submit them to the
ARM patch tracker, or will you Ack them so they can go through the Tegra
tree? Either way I can put them in a separate branch based on 3.14-rc1
in order to easily resolve any conflicts.
^ permalink raw reply
* [PATCH v2 5/7] ARM: perf_event: Fully support Krait CPU PMU events
From: Stephen Boyd @ 2014-01-22 20:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52DEBE6B.7000904@codeaurora.org>
On 01/21/14 10:37, Stephen Boyd wrote:
> On 01/21/14 10:07, Will Deacon wrote:
>> Do you need isbs to ensure the pmresrn side-effects have happened, or are
>> the registers self-synchronising? Similarly for your other IMP DEF
>> registers.
> There aren't any isbs in the downstream android sources so I assume
> they're self synchronizing. I'll confirm with the CPU designers to make
> sure.
>
CPU folks say no need for isb. They mentioned that the lack of an isb
after the armv7_pmnc_enable_counter() call will leave the action of
enabling the counter "in-flight". The window is probably pretty short on
an SMP kernel because of the spin_unlock right after with the barriers
in it, but the same can't be said for a UP kernel.
Also, the fuzzer didn't find anything else, but I found a bug in the
bitmap logic, updated and reran the fuzzer this morning. Everything
looks good.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
* [PATCH v2 06/15] watchdog: orion: Remove unneeded BRIDGE_CAUSE clear
From: Jason Gunthorpe @ 2014-01-22 20:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E02AB6.7040104@gmail.com>
> Clearing BRIDGE_CAUSE will only clear all currently pending upstream
> IRQs, of course. If WDT IRQ will be re-raised right after that in
> BRIDGE_CAUSE depends on the actual HW implementation, i.e. we do no
> clear the causing IRQ itself but just what it raised in BRIDGE_CAUSE.
Which is why it makes no sense to clear it one time at kernel start.
Either you only get new edge triggered interrupts after request_irq
(sane behavior) or you might sometimes get an old pending edge
triggered interrupt after request_irq (crazy behavior).
Clearing BRIDGE_CAUSE at kernel start only shortens the racy window it
doesn't eliminate it.
In the more familiar level triggered world the driver would go to the
device and ensure it wasn't asserting an IRQ level and then do the
request_irq. This guarentees it won't get an interrupt callback.
In a edge triggered world the driver should go to the device an ensure
that it won't create a new IRQ, then do request_irq - confident that
there will NEVER be a call to the IRQ handler, under any
circumstances.
So I think edge triggered interrupts need to ack any possible old edge
trigger in the cause register before the first unmask - eg in the
setup callback.
> So, you should also clear WDT's irq in the driver yourself to clear a
> possible pending upstream BRIDGE_CAUSE.
Which isn't possible - the BRIDGE_CAUSE is owned by the irq driver and
it must be cleared there, and it must only be cleared after the wdt
has been stopped so it doesn't set it again.
Notice that Ezequiel has added an IRQ handler that just calls panic,
so a spurious interrupt call is VERY VERY BAD.
Jason
^ permalink raw reply
* [PATCH 2/2] ehci-platform: Add support for controllers with big-endian regs / descriptors
From: Hans de Goede @ 2014-01-22 20:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140122213434.00000f25@openwrt.org>
Hi,
On 01/22/2014 09:34 PM, Jonas Gorski wrote:
> Hi,
>
> On Wed, 22 Jan 2014 20:28:26 +0100
> Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 01/21/2014 08:39 PM, Florian Fainelli wrote:
>>> 2014/1/21 Hans de Goede <hdegoede@redhat.com>:
>>>> This uses the already documented devicetree booleans for this.
>>>
>>> (I would greatly appreciate if you could CC people who gave you
>>> feedback on this before)
>>
>> Will do.
>>
>>> A more informative commit message would be welcome, along with a
>>> reference to which Device Tree binding documentation you are referring
>>> to.
>>
>> I've added a reference to the bindings doc in the commit msg for my next version.
>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>> drivers/usb/host/Kconfig | 3 +++
>>>> drivers/usb/host/ehci-platform.c | 33 +++++++++++++++++++++++++++++++--
>>>> 2 files changed, 34 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>>>> index 237d7b1..4af41f3 100644
>>>> --- a/drivers/usb/host/Kconfig
>>>> +++ b/drivers/usb/host/Kconfig
>>>> @@ -256,6 +256,9 @@ config USB_EHCI_ATH79
>>>> config USB_EHCI_HCD_PLATFORM
>>>> tristate "Generic EHCI driver for a platform device"
>>>> depends on !PPC_OF
>>>> + # Support BE on architectures which have readl_be
>>>> + select USB_EHCI_BIG_ENDIAN_DESC if (AVR32 || MIPS || MICROBLAZE || SPARC || PPC32 || PPC64)
>>>> + select USB_EHCI_BIG_ENDIAN_MMIO if (AVR32 || MIPS || MICROBLAZE || SPARC || PPC32 || PPC64)
>>>
>>> I do not think this is that simple nor correct for at least Microblaze
>>> and MIPS since they can run in either BE or LE mode, and those
>>> specific platforms should already do the proper select at the
>>> board/SoC level. This *might* be correct for SPARC, PPC32 and PPC64,
>>> although I believe some specific PPC64 boards can run in little-endian
>>> mode like the P-series, SPARC might too.
>>>
>>> It seems to me that you should not touch this and keep the existing
>>> selects in place, if it turns out that the selects are missing the
>>> error messages you added below are catching those misuses.
>>
>> As discussed with Alan, I will drop these lines from my next version.
>>
>>>> default n
>>>> ---help---
>>>> Adds an EHCI host driver for a generic platform device, which
>>>> diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
>>>> index d8aebc0..5888abb 100644
>>>> --- a/drivers/usb/host/ehci-platform.c
>>>> +++ b/drivers/usb/host/ehci-platform.c
>>>> @@ -55,8 +55,10 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
>>>>
>>>> hcd->has_tt = pdata->has_tt;
>>>> ehci->has_synopsys_hc_bug = pdata->has_synopsys_hc_bug;
>>>> - ehci->big_endian_desc = pdata->big_endian_desc;
>>>> - ehci->big_endian_mmio = pdata->big_endian_mmio;
>>>> + if (pdata->big_endian_desc)
>>>> + ehci->big_endian_desc = 1;
>>>> + if (pdata->big_endian_mmio)
>>>> + ehci->big_endian_mmio = 1;
>>>>
>>>> if (pdata->pre_setup) {
>>>> retval = pdata->pre_setup(hcd);
>>>> @@ -142,6 +144,7 @@ static int ehci_platform_probe(struct platform_device *dev)
>>>> struct resource *res_mem;
>>>> struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev);
>>>> struct ehci_platform_priv *priv;
>>>> + struct ehci_hcd *ehci;
>>>> int err, irq, clk = 0;
>>>>
>>>> if (usb_disabled())
>>>> @@ -177,8 +180,34 @@ static int ehci_platform_probe(struct platform_device *dev)
>>>> platform_set_drvdata(dev, hcd);
>>>> dev->dev.platform_data = pdata;
>>>> priv = hcd_to_ehci_priv(hcd);
>>>> + ehci = hcd_to_ehci(hcd);
>>>>
>>>> if (pdata == &ehci_platform_defaults && dev->dev.of_node) {
>>>> + if (of_property_read_bool(dev->dev.of_node, "big-endian-regs"))
>>>> + ehci->big_endian_mmio = 1;
>>>> +
>>>> + if (of_property_read_bool(dev->dev.of_node, "big-endian-desc"))
>>>> + ehci->big_endian_desc = 1;
>>>> +
>>>> + if (of_property_read_bool(dev->dev.of_node, "big-endian"))
>>>> + ehci->big_endian_mmio = ehci->big_endian_desc = 1;
>>>
>>> Ok, so I am confused now, should you update
>>> pdata->ehci_big_endian_{desc,mmio} here or is it valid to directly
>>> modify ehci->big_endian_{desc,mmio}, is not there any risk to undo
>>> what is done in ehci_platform_reset(), or is ehci_platform_reset()
>>> only called for non-DT cases?
>>
>> Both the pdata checks in ehci_platform_reset() and the dt checks here only
>> ever set these flags, neither code path clears them. And in the dt case pdata
>> will be NULL and vice versa.
>
> If it's safe to set ehci->big_endian_{desc,mmio} from the _probe()
> routine, then maybe the pdata sets in _reset() should be moved into here
> instead of adding extra cludges/checks into _reset().
That seems like an entire separate patch / improvement on top of adding support
for big-endian controllers to the dt code.
If people won't to improve on thus further and / or clean things up further I'm
sure the usb-subsys maintainers would welcome patches.
<snip more suggestions best done in a separate patch written by someone else>
Regards,
Hans
^ permalink raw reply
* [PATCH 2/2] ehci-platform: Add support for controllers with big-endian regs / descriptors
From: Hans de Goede @ 2014-01-22 21:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E02F84.70705@redhat.com>
Hi,
On 01/22/2014 09:52 PM, Hans de Goede wrote:
> Hi,
>
> On 01/22/2014 09:34 PM, Jonas Gorski wrote:
>> Hi,
>>
>> On Wed, 22 Jan 2014 20:28:26 +0100
>> Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>> On 01/21/2014 08:39 PM, Florian Fainelli wrote:
>>>> 2014/1/21 Hans de Goede <hdegoede@redhat.com>:
>>>>> This uses the already documented devicetree booleans for this.
>>>>
>>>> (I would greatly appreciate if you could CC people who gave you
>>>> feedback on this before)
>>>
>>> Will do.
>>>
>>>> A more informative commit message would be welcome, along with a
>>>> reference to which Device Tree binding documentation you are referring
>>>> to.
>>>
>>> I've added a reference to the bindings doc in the commit msg for my next version.
>>>
>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>> ---
>>>>> drivers/usb/host/Kconfig | 3 +++
>>>>> drivers/usb/host/ehci-platform.c | 33 +++++++++++++++++++++++++++++++--
>>>>> 2 files changed, 34 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
>>>>> index 237d7b1..4af41f3 100644
>>>>> --- a/drivers/usb/host/Kconfig
>>>>> +++ b/drivers/usb/host/Kconfig
>>>>> @@ -256,6 +256,9 @@ config USB_EHCI_ATH79
>>>>> config USB_EHCI_HCD_PLATFORM
>>>>> tristate "Generic EHCI driver for a platform device"
>>>>> depends on !PPC_OF
>>>>> + # Support BE on architectures which have readl_be
>>>>> + select USB_EHCI_BIG_ENDIAN_DESC if (AVR32 || MIPS || MICROBLAZE || SPARC || PPC32 || PPC64)
>>>>> + select USB_EHCI_BIG_ENDIAN_MMIO if (AVR32 || MIPS || MICROBLAZE || SPARC || PPC32 || PPC64)
>>>>
>>>> I do not think this is that simple nor correct for at least Microblaze
>>>> and MIPS since they can run in either BE or LE mode, and those
>>>> specific platforms should already do the proper select at the
>>>> board/SoC level. This *might* be correct for SPARC, PPC32 and PPC64,
>>>> although I believe some specific PPC64 boards can run in little-endian
>>>> mode like the P-series, SPARC might too.
>>>>
>>>> It seems to me that you should not touch this and keep the existing
>>>> selects in place, if it turns out that the selects are missing the
>>>> error messages you added below are catching those misuses.
>>>
>>> As discussed with Alan, I will drop these lines from my next version.
>>>
>>>>> default n
>>>>> ---help---
>>>>> Adds an EHCI host driver for a generic platform device, which
>>>>> diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
>>>>> index d8aebc0..5888abb 100644
>>>>> --- a/drivers/usb/host/ehci-platform.c
>>>>> +++ b/drivers/usb/host/ehci-platform.c
>>>>> @@ -55,8 +55,10 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
>>>>>
>>>>> hcd->has_tt = pdata->has_tt;
>>>>> ehci->has_synopsys_hc_bug = pdata->has_synopsys_hc_bug;
>>>>> - ehci->big_endian_desc = pdata->big_endian_desc;
>>>>> - ehci->big_endian_mmio = pdata->big_endian_mmio;
>>>>> + if (pdata->big_endian_desc)
>>>>> + ehci->big_endian_desc = 1;
>>>>> + if (pdata->big_endian_mmio)
>>>>> + ehci->big_endian_mmio = 1;
>>>>>
>>>>> if (pdata->pre_setup) {
>>>>> retval = pdata->pre_setup(hcd);
>>>>> @@ -142,6 +144,7 @@ static int ehci_platform_probe(struct platform_device *dev)
>>>>> struct resource *res_mem;
>>>>> struct usb_ehci_pdata *pdata = dev_get_platdata(&dev->dev);
>>>>> struct ehci_platform_priv *priv;
>>>>> + struct ehci_hcd *ehci;
>>>>> int err, irq, clk = 0;
>>>>>
>>>>> if (usb_disabled())
>>>>> @@ -177,8 +180,34 @@ static int ehci_platform_probe(struct platform_device *dev)
>>>>> platform_set_drvdata(dev, hcd);
>>>>> dev->dev.platform_data = pdata;
>>>>> priv = hcd_to_ehci_priv(hcd);
>>>>> + ehci = hcd_to_ehci(hcd);
>>>>>
>>>>> if (pdata == &ehci_platform_defaults && dev->dev.of_node) {
>>>>> + if (of_property_read_bool(dev->dev.of_node, "big-endian-regs"))
>>>>> + ehci->big_endian_mmio = 1;
>>>>> +
>>>>> + if (of_property_read_bool(dev->dev.of_node, "big-endian-desc"))
>>>>> + ehci->big_endian_desc = 1;
>>>>> +
>>>>> + if (of_property_read_bool(dev->dev.of_node, "big-endian"))
>>>>> + ehci->big_endian_mmio = ehci->big_endian_desc = 1;
>>>>
>>>> Ok, so I am confused now, should you update
>>>> pdata->ehci_big_endian_{desc,mmio} here or is it valid to directly
>>>> modify ehci->big_endian_{desc,mmio}, is not there any risk to undo
>>>> what is done in ehci_platform_reset(), or is ehci_platform_reset()
>>>> only called for non-DT cases?
>>>
>>> Both the pdata checks in ehci_platform_reset() and the dt checks here only
>>> ever set these flags, neither code path clears them. And in the dt case pdata
>>> will be NULL and vice versa.
>>
>> If it's safe to set ehci->big_endian_{desc,mmio} from the _probe()
>> routine, then maybe the pdata sets in _reset() should be moved into here
>> instead of adding extra cludges/checks into _reset().
>
> That seems like an entire separate patch / improvement on top of adding support
> for big-endian controllers to the dt code.
>
> If people won't to improve on thus further and / or clean things up further I'm
> sure the usb-subsys maintainers would welcome patches.
s/won't to improve on thus/want to improve this/
>
> <snip more suggestions best done in a separate patch written by someone else>
>
> Regards,
>
> Hans
Regards,
Hans
^ permalink raw reply
* [PATCH 1/2] ARM: omapfb: add coherent dma memory support
From: Ivaylo Dimitrov @ 2014-01-22 21:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52D146E8.4090105@gmail.com>
Hmm, maybe this https://lkml.org/lkml/2014/1/22/386 will solve our issues
Ivo
^ permalink raw reply
* [PATCH 2/2] ehci-platform: Add support for controllers with big-endian regs / descriptors
From: Alan Stern @ 2014-01-22 21:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140122213434.00000f25@openwrt.org>
On Wed, 22 Jan 2014, Jonas Gorski wrote:
> If it's safe to set ehci->big_endian_{desc,mmio} from the _probe()
> routine, then maybe the pdata sets in _reset() should be moved into here
> instead of adding extra cludges/checks into _reset().
Why? What difference would it make?
Alan Stern
^ permalink raw reply
* [PATCH v3 02/24] drm/i2c: tda998x: check more I/O errors
From: Russell King - ARM Linux @ 2014-01-22 21:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140119195838.348f8d6d@armhf>
On Sun, Jan 19, 2014 at 07:58:38PM +0100, Jean-Francois Moine wrote:
> This patch adds more error checking inn I2C I/O functions.
> In case of I/O error, this permits to avoid writing in bad controller
> pages, a bad chipset detection or looping when getting the EDID.
>
> Tested-by: Russell King <rmk+kernel@arm.linux.org.uk>
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
Thanks.
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply
* [PATCH v3 03/24] drm/i2c: tda998x: code cleanup
From: Russell King - ARM Linux @ 2014-01-22 21:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140119195838.372fbbaa@armhf>
On Sun, Jan 19, 2014 at 07:58:38PM +0100, Jean-Francois Moine wrote:
> This patch:
> - replaces ARRAY_SIZE() by sizeof() when a number of bytes is needed,
> - adds a linefeed in an error message and
> - removes an useless variable setting.
>
> Tested-by: Russell King <rmk+kernel@arm.linux.org.uk>
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
Thanks.
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply
* [PATCH v3 07/24] drm/i2c: tda998x: set the video mode from the adjusted value
From: Russell King - ARM Linux @ 2014-01-22 21:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140119195840.1ecab03b@armhf>
On Sun, Jan 19, 2014 at 07:58:40PM +0100, Jean-Francois Moine wrote:
> This patch uses always the adjusted video mode instead of a mix of
> original and adjusted mode.
>
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
Nothing obviously wrong and appears to work, thanks.
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
Tested-by: Russell King <rmk+kernel@arm.linux.org.uk>
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply
* [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Levente Kurusa @ 2014-01-22 21:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390409565-4200-2-git-send-email-sthokal@xilinx.com>
Hello,
2014/1/22 Srikanth Thokala <sthokal@xilinx.com>:
> This is the driver for the AXI Video Direct Memory Access (AXI
> VDMA) core, which is a soft Xilinx IP core that provides high-
> bandwidth direct memory access between memory and AXI4-Stream
> type video target peripherals. The core provides efficient two
> dimensional DMA operations with independent asynchronous read
> and write channel operation.
>
> This module works on Zynq (ARM Based SoC) and Microblaze platforms.
>
> Signed-off-by: Srikanth Thokala <sthokal@xilinx.com>
> ---
Another two remarks, after you fixed them ( or not :-) )
you can have my:
Reviewed-by: Levente Kurusa <levex@linux.com>
Oh, and next time please if you post a patch that fixes something I pointed out,
CC me as I had a hard time finding this patch, thanks. :-)
> NOTE:
> 1. Created a separate directory 'dma/xilinx' as Xilinx has two more
> DMA IPs and we are also planning to upstream these drivers soon.
> 2. Rebased on v3.13.0-rc8
>
> Changes in v2:
> - Removed DMA Test client module from the patchset as suggested
> by Andy Shevchenko
> - Removed device-id DT property, as suggested by Arnd Bergmann
> - Properly documented DT bindings as suggested by Arnd Bergmann
> - Returning with error, if registration of DMA to node fails
> - Fixed typo errors
> - Used BIT() macro at applicable places
> - Added missing header file to the patchset
> - Changed copyright year to include 2014
> ---
> .../devicetree/bindings/dma/xilinx/xilinx_vdma.txt | 75 +
> drivers/dma/Kconfig | 14 +
> drivers/dma/Makefile | 1 +
> drivers/dma/xilinx/Makefile | 1 +
> drivers/dma/xilinx/xilinx_vdma.c | 1486 ++++++++++++++++++++
> include/linux/amba/xilinx_dma.h | 50 +
> 6 files changed, 1627 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
> create mode 100644 drivers/dma/xilinx/Makefile
> create mode 100644 drivers/dma/xilinx/xilinx_vdma.c
> create mode 100644 include/linux/amba/xilinx_dma.h
>
> diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
> new file mode 100644
> index 0000000..ab8be1a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
> @@ -0,0 +1,75 @@
> +Xilinx AXI VDMA engine, it does transfers between memory and video devices.
> +It can be configured to have one channel or two channels. If configured
> +as two channels, one is to transmit to the video device and another is
> +to receive from the video device.
> +
> +Required properties:
> +- compatible: Should be "xlnx,axi-vdma-1.00.a"
> +- #dma-cells: Should be <1>, see "dmas" property below
> +- reg: Should contain VDMA registers location and length.
> +- xlnx,num-fstores: Should be the number of framebuffers as configured in h/w.
> +- dma-channel child node: Should have atleast one channel and can have upto
> + two channels per device. This node specifies the properties of each
> + DMA channel (see child node properties below).
> +
> +Optional properties:
> +- xlnx,include-sg: Tells whether configured for Scatter-mode in
> + the hardware.
> [...]
> +
> +/**
> + * xilinx_vdma_is_running - Check if VDMA channel is running
> + * @chan: Driver specific VDMA channel
> + *
> + * Return: '1' if running, '0' if not.
> + */
> +static int xilinx_vdma_is_running(struct xilinx_vdma_chan *chan)
> +{
> + return !(vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR) &
> + XILINX_VDMA_DMASR_HALTED) &&
> + (vdma_ctrl_read(chan, XILINX_VDMA_REG_DMACR) &
> + XILINX_VDMA_DMACR_RUNSTOP);
> +}
> +
> +/**
> + * xilinx_vdma_is_idle - Check if VDMA channel is idle
> + * @chan: Driver specific VDMA channel
> + *
> + * Return: '1' if idle, '0' if not.
> + */
> +static int xilinx_vdma_is_idle(struct xilinx_vdma_chan *chan)
> +{
> + return vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR) &
> + XILINX_VDMA_DMASR_IDLE;
> +}
> +
> +/**
> + * xilinx_vdma_halt - Halt VDMA channel
> + * @chan: Driver specific VDMA channel
> + */
> +static void xilinx_vdma_halt(struct xilinx_vdma_chan *chan)
> +{
> + int loop = XILINX_VDMA_LOOP_COUNT + 1;
> +
> + vdma_ctrl_clr(chan, XILINX_VDMA_REG_DMACR, XILINX_VDMA_DMACR_RUNSTOP);
> +
> + /* Wait for the hardware to halt */
> + while (loop--)
> + if (vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR) &
> + XILINX_VDMA_DMASR_HALTED)
> + break;
> +
> + if (!loop) {
> + dev_err(chan->dev, "Cannot stop channel %p: %x\n",
> + chan, vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR));
> + chan->err = true;
> + }
> +
> + return;
> +}
> +
> +/**
> + * xilinx_vdma_start - Start VDMA channel
> + * @chan: Driver specific VDMA channel
> + */
> +static void xilinx_vdma_start(struct xilinx_vdma_chan *chan)
> +{
> + int loop = XILINX_VDMA_LOOP_COUNT + 1;
> +
> + vdma_ctrl_set(chan, XILINX_VDMA_REG_DMACR, XILINX_VDMA_DMACR_RUNSTOP);
> +
> + /* Wait for the hardware to start */
> + while (loop--)
> + if (!(vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR) &
> + XILINX_VDMA_DMASR_HALTED))
> + break;
> +
> + if (!loop) {
> + dev_err(chan->dev, "Cannot start channel %p: %x\n",
> + chan, vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR));
> +
> + chan->err = true;
> + }
> +
> + return;
> +}
> +
> +/**
> + * xilinx_vdma_start_transfer - Starts VDMA transfer
> + * @chan: Driver specific channel struct pointer
> + */
> +static void xilinx_vdma_start_transfer(struct xilinx_vdma_chan *chan)
> +{
> + struct xilinx_vdma_config *config = &chan->config;
> + struct xilinx_vdma_tx_descriptor *desc;
> + unsigned long flags;
> + u32 reg;
> + struct xilinx_vdma_tx_segment *head, *tail = NULL;
> +
> + if (chan->err)
> + return;
> +
> + spin_lock_irqsave(&chan->lock, flags);
> +
> + /* There's already an active descriptor, bail out. */
> + if (chan->active_desc)
> + goto out_unlock;
> +
> + if (list_empty(&chan->pending_list))
> + goto out_unlock;
> +
> + desc = list_first_entry(&chan->pending_list,
> + struct xilinx_vdma_tx_descriptor, node);
> +
> + /* If it is SG mode and hardware is busy, cannot submit */
> + if (chan->has_sg && xilinx_vdma_is_running(chan) &&
> + !xilinx_vdma_is_idle(chan)) {
> + dev_dbg(chan->dev, "DMA controller still busy\n");
> + goto out_unlock;
> + }
> +
> + if (chan->err)
> + goto out_unlock;
> +
> + /*
> + * If hardware is idle, then all descriptors on the running lists are
> + * done, start new transfers
> + */
> + if (chan->has_sg) {
> + head = list_first_entry(&desc->segments,
> + struct xilinx_vdma_tx_segment, node);
> + tail = list_entry(desc->segments.prev,
> + struct xilinx_vdma_tx_segment, node);
> +
> + vdma_ctrl_write(chan, XILINX_VDMA_REG_CURDESC, head->phys);
> + }
> +
> + /* Configure the hardware using info in the config structure */
> + reg = vdma_ctrl_read(chan, XILINX_VDMA_REG_DMACR);
> +
> + if (config->frm_cnt_en)
> + reg |= XILINX_VDMA_DMACR_FRAMECNT_EN;
> + else
> + reg &= ~XILINX_VDMA_DMACR_FRAMECNT_EN;
> +
> + /*
> + * With SG, start with circular mode, so that BDs can be fetched.
> + * In direct register mode, if not parking, enable circular mode
> + */
> + if (chan->has_sg || !config->park)
> + reg |= XILINX_VDMA_DMACR_CIRC_EN;
> +
> + if (config->park)
> + reg &= ~XILINX_VDMA_DMACR_CIRC_EN;
> +
> + vdma_ctrl_write(chan, XILINX_VDMA_REG_DMACR, reg);
> +
> + if (config->park && (config->park_frm >= 0) &&
> + (config->park_frm < chan->num_frms)) {
> + if (chan->direction == DMA_MEM_TO_DEV)
> + vdma_write(chan, XILINX_VDMA_REG_PARK_PTR,
> + config->park_frm <<
> + XILINX_VDMA_PARK_PTR_RD_REF_SHIFT);
> + else
> + vdma_write(chan, XILINX_VDMA_REG_PARK_PTR,
> + config->park_frm <<
> + XILINX_VDMA_PARK_PTR_WR_REF_SHIFT);
> + }
> +
> + /* Start the hardware */
> + xilinx_vdma_start(chan);
> +
> + if (chan->err)
> + goto out_unlock;
> +
> + /* Start the transfer */
> + if (chan->has_sg) {
> + vdma_ctrl_write(chan, XILINX_VDMA_REG_TAILDESC, tail->phys);
> + } else {
> + struct xilinx_vdma_tx_segment *segment;
> + int i = 0;
> +
> + list_for_each_entry(segment, &desc->segments, node)
> + vdma_desc_write(chan,
> + XILINX_VDMA_REG_START_ADDRESS(i++),
> + segment->hw.buf_addr);
> +
> + vdma_desc_write(chan, XILINX_VDMA_REG_HSIZE, config->hsize);
> + vdma_desc_write(chan, XILINX_VDMA_REG_FRMDLY_STRIDE,
> + (config->frm_dly <<
> + XILINX_VDMA_FRMDLY_STRIDE_FRMDLY_SHIFT) |
> + (config->stride <<
> + XILINX_VDMA_FRMDLY_STRIDE_STRIDE_SHIFT));
> + vdma_desc_write(chan, XILINX_VDMA_REG_VSIZE, config->vsize);
> + }
> +
> + list_del(&desc->node);
> + chan->active_desc = desc;
> +
> +out_unlock:
> + spin_unlock_irqrestore(&chan->lock, flags);
> +}
> +
> +/**
> + * xilinx_vdma_issue_pending - Issue pending transactions
> + * @dchan: DMA channel
> + */
> +static void xilinx_vdma_issue_pending(struct dma_chan *dchan)
> +{
> + struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan);
> +
> + xilinx_vdma_start_transfer(chan);
> +}
> +
> +/**
> + * xilinx_vdma_complete_descriptor - Mark the active descriptor as complete
> + * @chan : xilinx DMA channel
> + *
> + * CONTEXT: hardirq
> + */
> +static void xilinx_vdma_complete_descriptor(struct xilinx_vdma_chan *chan)
> +{
> + struct xilinx_vdma_tx_descriptor *desc;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&chan->lock, flags);
> +
> + desc = chan->active_desc;
> + if (!desc) {
> + dev_dbg(chan->dev, "no running descriptors\n");
> + goto out_unlock;
> + }
> +
> + list_add_tail(&desc->node, &chan->done_list);
> +
> + /* Update the completed cookie and reset the active descriptor. */
> + chan->completed_cookie = desc->async_tx.cookie;
> + chan->active_desc = NULL;
> +
> +out_unlock:
> + spin_unlock_irqrestore(&chan->lock, flags);
> +}
> +
> +/**
> + * xilinx_vdma_reset - Reset VDMA channel
> + * @chan: Driver specific VDMA channel
> + *
> + * Return: '0' on success and failure value on error
> + */
> +static int xilinx_vdma_reset(struct xilinx_vdma_chan *chan)
> +{
> + int loop = XILINX_VDMA_LOOP_COUNT + 1;
> + u32 tmp;
> +
> + vdma_ctrl_set(chan, XILINX_VDMA_REG_DMACR, XILINX_VDMA_DMACR_RESET);
> +
> + tmp = vdma_ctrl_read(chan, XILINX_VDMA_REG_DMACR) &
> + XILINX_VDMA_DMACR_RESET;
> +
> + /* Wait for the hardware to finish reset */
> + while (loop-- && tmp)
> + tmp = vdma_ctrl_read(chan, XILINX_VDMA_REG_DMACR) &
> + XILINX_VDMA_DMACR_RESET;
> +
> + if (!loop) {
> + dev_err(chan->dev, "reset timeout, cr %x, sr %x\n",
> + vdma_ctrl_read(chan, XILINX_VDMA_REG_DMACR),
> + vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR));
> + return -ETIMEDOUT;
> + }
> +
> + chan->err = false;
> +
> + return 0;
> +}
> +
> +/**
> + * xilinx_vdma_chan_reset - Reset VDMA channel and enable interrupts
> + * @chan: Driver specific VDMA channel
> + *
> + * Return: '0' on success and failure value on error
> + */
> +static int xilinx_vdma_chan_reset(struct xilinx_vdma_chan *chan)
> +{
> + int err;
> +
> + /* Reset VDMA */
> + err = xilinx_vdma_reset(chan);
> + if (err)
> + return err;
> +
> + /* Enable interrupts */
> + vdma_ctrl_set(chan, XILINX_VDMA_REG_DMACR,
> + XILINX_VDMA_DMAXR_ALL_IRQ_MASK);
> +
> + return 0;
> +}
> +
> +/**
> + * xilinx_vdma_irq_handler - VDMA Interrupt handler
> + * @irq: IRQ number
> + * @data: Pointer to the Xilinx VDMA channel structure
> + *
> + * Return: IRQ_HANDLED/IRQ_NONE
> + */
> +static irqreturn_t xilinx_vdma_irq_handler(int irq, void *data)
> +{
> + struct xilinx_vdma_chan *chan = data;
> + u32 status;
> +
> + /* Read the status and ack the interrupts. */
> + status = vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR);
> + if (!(status & XILINX_VDMA_DMAXR_ALL_IRQ_MASK))
> + return IRQ_NONE;
> +
> + vdma_ctrl_write(chan, XILINX_VDMA_REG_DMASR,
> + status & XILINX_VDMA_DMAXR_ALL_IRQ_MASK);
> +
> + if (status & XILINX_VDMA_DMASR_ERR_IRQ) {
> + /*
> + * An error occurred. If C_FLUSH_ON_FSYNC is enabled and the
> + * error is recoverable, ignore it. Otherwise flag the error.
> + *
> + * Only recoverable errors can be cleared in the DMASR register,
> + * make sure not to write to other error bits to 1.
> + */
> + u32 errors = status & XILINX_VDMA_DMASR_ALL_ERR_MASK;
> + vdma_ctrl_write(chan, XILINX_VDMA_REG_DMASR,
> + errors & XILINX_VDMA_DMASR_ERR_RECOVER_MASK);
> +
> + if (!chan->flush_on_fsync ||
> + (errors & ~XILINX_VDMA_DMASR_ERR_RECOVER_MASK)) {
> + dev_err(chan->dev,
> + "Channel %p has errors %x, cdr %x tdr %x\n",
> + chan, errors,
> + vdma_ctrl_read(chan, XILINX_VDMA_REG_CURDESC),
> + vdma_ctrl_read(chan, XILINX_VDMA_REG_TAILDESC));
> + chan->err = true;
> + }
> + }
> +
> + if (status & XILINX_VDMA_DMASR_DLY_CNT_IRQ) {
> + /*
> + * Device takes too long to do the transfer when user requires
> + * responsiveness.
> + */
> + dev_dbg(chan->dev, "Inter-packet latency too long\n");
> + }
> +
> + if (status & XILINX_VDMA_DMASR_FRM_CNT_IRQ) {
> + xilinx_vdma_complete_descriptor(chan);
> + xilinx_vdma_start_transfer(chan);
> + }
> +
> + tasklet_schedule(&chan->tasklet);
> + return IRQ_HANDLED;
> +}
> +
> +/**
> + * xilinx_vdma_tx_submit - Submit DMA transaction
> + * @tx: Async transaction descriptor
> + *
> + * Return: cookie value on success and failure value on error
> + */
> +static dma_cookie_t xilinx_vdma_tx_submit(struct dma_async_tx_descriptor *tx)
> +{
> + struct xilinx_vdma_tx_descriptor *desc = to_vdma_tx_descriptor(tx);
> + struct xilinx_vdma_chan *chan = to_xilinx_chan(tx->chan);
> + struct xilinx_vdma_tx_segment *segment;
> + dma_cookie_t cookie;
> + unsigned long flags;
> + int err;
> +
> + if (chan->err) {
> + /*
> + * If reset fails, need to hard reset the system.
> + * Channel is no longer functional
> + */
> + err = xilinx_vdma_chan_reset(chan);
> + if (err < 0)
> + return err;
> + }
> +
> + spin_lock_irqsave(&chan->lock, flags);
> +
> + /* Assign cookies to all of the segments that make up this transaction.
> + * Use the cookie of the last segment as the transaction cookie.
> + */
> + cookie = chan->cookie;
> +
> + list_for_each_entry(segment, &desc->segments, node) {
> + if (cookie < DMA_MAX_COOKIE)
> + cookie++;
> + else
> + cookie = DMA_MIN_COOKIE;
> +
> + segment->cookie = cookie;
> + }
> +
> + tx->cookie = cookie;
> + chan->cookie = cookie;
> +
> + /* Append the transaction to the pending transactions queue. */
> + list_add_tail(&desc->node, &chan->pending_list);
> +
> + spin_unlock_irqrestore(&chan->lock, flags);
> +
> + return cookie;
> +}
> +
> +/**
> + * xilinx_vdma_prep_slave_sg - prepare a descriptor for a DMA_SLAVE transaction
> + * @dchan: DMA channel
> + * @sgl: scatterlist to transfer to/from
> + * @sg_len: number of entries in @sgl
> + * @dir: DMA direction
> + * @flags: transfer ack flags
> + * @context: unused
> + *
> + * Return: Async transaction descriptor on success and NULL on failure
> + */
> +static struct dma_async_tx_descriptor *
> +xilinx_vdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
> + unsigned int sg_len, enum dma_transfer_direction dir,
> + unsigned long flags, void *context)
> +{
> + struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan);
> + struct xilinx_vdma_tx_descriptor *desc;
> + struct xilinx_vdma_tx_segment *segment;
> + struct xilinx_vdma_tx_segment *prev = NULL;
> + struct scatterlist *sg;
> + int i;
> +
> + if (chan->direction != dir || sg_len == 0)
> + return NULL;
> +
> + /* Enforce one sg entry for one frame. */
> + if (sg_len != chan->num_frms) {
> + dev_err(chan->dev,
> + "number of entries %d not the same as num stores %d\n",
> + sg_len, chan->num_frms);
> + return NULL;
> + }
> +
> + /* Allocate a transaction descriptor. */
> + desc = xilinx_vdma_alloc_tx_descriptor(chan);
> + if (!desc)
> + return NULL;
> +
> + dma_async_tx_descriptor_init(&desc->async_tx, &chan->common);
> + desc->async_tx.tx_submit = xilinx_vdma_tx_submit;
> + desc->async_tx.cookie = 0;
> + async_tx_ack(&desc->async_tx);
> +
> + /* Build the list of transaction segments. */
> + for_each_sg(sgl, sg, sg_len, i) {
> + struct xilinx_vdma_desc_hw *hw;
> +
> + /* Allocate the link descriptor from DMA pool */
> + segment = xilinx_vdma_alloc_tx_segment(chan);
> + if (!segment)
> + goto error;
> +
> + /* Fill in the hardware descriptor */
> + hw = &segment->hw;
> + hw->buf_addr = sg_dma_address(sg);
> + hw->vsize = chan->config.vsize;
> + hw->hsize = chan->config.hsize;
> + hw->stride = (chan->config.frm_dly <<
> + XILINX_VDMA_FRMDLY_STRIDE_FRMDLY_SHIFT) |
> + (chan->config.stride <<
> + XILINX_VDMA_FRMDLY_STRIDE_STRIDE_SHIFT);
> + if (prev)
> + prev->hw.next_desc = segment->phys;
> +
> + /* Insert the segment into the descriptor segments list. */
> + list_add_tail(&segment->node, &desc->segments);
> +
> + prev = segment;
> + }
> +
> + /* Link the last hardware descriptor with the first. */
> + segment = list_first_entry(&desc->segments,
> + struct xilinx_vdma_tx_segment, node);
> + prev->hw.next_desc = segment->phys;
> +
> + return &desc->async_tx;
> +
> +error:
> + xilinx_vdma_free_tx_descriptor(chan, desc);
> + return NULL;
> +}
> +
> +/**
> + * xilinx_vdma_terminate_all - Halt the channel and free descriptors
> + * @chan: Driver specific VDMA Channel pointer
> + */
> +static void xilinx_vdma_terminate_all(struct xilinx_vdma_chan *chan)
> +{
> + /* Halt the DMA engine */
> + xilinx_vdma_halt(chan);
> +
> + /* Remove and free all of the descriptors in the lists */
> + xilinx_vdma_free_descriptors(chan);
> +}
> +
> +/**
> + * xilinx_vdma_slave_config - Configure VDMA channel
> + * Run-time configuration for Axi VDMA, supports:
> + * . halt the channel
> + * . configure interrupt coalescing and inter-packet delay threshold
> + * . start/stop parking
> + * . enable genlock
> + * . set transfer information using config struct
> + *
> + * @chan: Driver specific VDMA Channel pointer
> + * @cfg: Channel configuration pointer
> + *
> + * Return: '0' on success and failure value on error
> + */
> +static int xilinx_vdma_slave_config(struct xilinx_vdma_chan *chan,
> + struct xilinx_vdma_config *cfg)
> +{
> + u32 dmacr;
> +
> + if (cfg->reset)
> + return xilinx_vdma_chan_reset(chan);
> +
> + dmacr = vdma_ctrl_read(chan, XILINX_VDMA_REG_DMACR);
> +
> + /* If vsize is -1, it is park-related operations */
> + if (cfg->vsize == -1) {
> + if (cfg->park)
> + dmacr &= ~XILINX_VDMA_DMACR_CIRC_EN;
> + else
> + dmacr |= XILINX_VDMA_DMACR_CIRC_EN;
> +
> + vdma_ctrl_write(chan, XILINX_VDMA_REG_DMACR, dmacr);
> + return 0;
> + }
> +
> + /* If hsize is -1, it is interrupt threshold settings */
> + if (cfg->hsize == -1) {
> + if (cfg->coalesc <= XILINX_VDMA_DMACR_FRAME_COUNT_MAX) {
> + dmacr &= ~XILINX_VDMA_DMACR_FRAME_COUNT_MASK;
> + dmacr |= cfg->coalesc <<
> + XILINX_VDMA_DMACR_FRAME_COUNT_SHIFT;
> + chan->config.coalesc = cfg->coalesc;
> + }
> +
> + if (cfg->delay <= XILINX_VDMA_DMACR_DELAY_MAX) {
> + dmacr &= ~XILINX_VDMA_DMACR_DELAY_MASK;
> + dmacr |= cfg->delay << XILINX_VDMA_DMACR_DELAY_SHIFT;
> + chan->config.delay = cfg->delay;
> + }
> +
> + vdma_ctrl_write(chan, XILINX_VDMA_REG_DMACR, dmacr);
> + return 0;
> + }
> +
> + /* Transfer information */
> + chan->config.vsize = cfg->vsize;
> + chan->config.hsize = cfg->hsize;
> + chan->config.stride = cfg->stride;
> + chan->config.frm_dly = cfg->frm_dly;
> + chan->config.park = cfg->park;
> +
> + /* genlock settings */
> + chan->config.gen_lock = cfg->gen_lock;
> + chan->config.master = cfg->master;
> +
> + if (cfg->gen_lock && chan->genlock) {
> + dmacr |= XILINX_VDMA_DMACR_GENLOCK_EN;
> + dmacr |= cfg->master << XILINX_VDMA_DMACR_MASTER_SHIFT;
> + }
> +
> + chan->config.frm_cnt_en = cfg->frm_cnt_en;
> + if (cfg->park)
> + chan->config.park_frm = cfg->park_frm;
> + else
> + chan->config.park_frm = -1;
> +
> + chan->config.coalesc = cfg->coalesc;
> + chan->config.delay = cfg->delay;
> + if (cfg->coalesc <= XILINX_VDMA_DMACR_FRAME_COUNT_MAX) {
> + dmacr |= cfg->coalesc << XILINX_VDMA_DMACR_FRAME_COUNT_SHIFT;
> + chan->config.coalesc = cfg->coalesc;
> + }
> +
> + if (cfg->delay <= XILINX_VDMA_DMACR_DELAY_MAX) {
> + dmacr |= cfg->delay << XILINX_VDMA_DMACR_DELAY_SHIFT;
> + chan->config.delay = cfg->delay;
> + }
> +
> + /* FSync Source selection */
> + dmacr &= ~XILINX_VDMA_DMACR_FSYNCSRC_MASK;
> + dmacr |= cfg->ext_fsync << XILINX_VDMA_DMACR_FSYNCSRC_SHIFT;
> +
> + vdma_ctrl_write(chan, XILINX_VDMA_REG_DMACR, dmacr);
> + return 0;
> +}
> +
> +/**
> + * 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);
> + default:
> + return -ENXIO;
> + }
> +}
> +
> +/* -----------------------------------------------------------------------------
> + * Probe and remove
> + */
> +
> +/**
> + * xilinx_vdma_chan_remove - Per Channel remove function
> + * @chan: Driver specific VDMA channel
> + */
> +static void xilinx_vdma_chan_remove(struct xilinx_vdma_chan *chan)
> +{
> + /* Disable all interrupts */
> + vdma_ctrl_clr(chan, XILINX_VDMA_REG_DMACR,
> + XILINX_VDMA_DMAXR_ALL_IRQ_MASK);
> +
> + list_del(&chan->common.device_node);
> +}
> +
> +/**
> + * xilinx_vdma_chan_probe - Per Channel Probing
> + * It get channel features from the device tree entry and
> + * initialize special channel handling routines
> + *
> + * @xdev: Driver specific device structure
> + * @node: Device node
> + *
> + * Return: '0' on success and failure value on error
> + */
> +static int xilinx_vdma_chan_probe(struct xilinx_vdma_device *xdev,
> + struct device_node *node)
> +{
> + struct xilinx_vdma_chan *chan;
> + bool has_dre = false;
> + u32 value;
> + int err;
> +
> + /* Allocate and initialize the channel structure */
> + chan = devm_kzalloc(xdev->dev, sizeof(*chan), GFP_KERNEL);
> + if (!chan)
> + return -ENOMEM;
> +
> + chan->dev = xdev->dev;
> + chan->xdev = xdev;
> + chan->has_sg = xdev->has_sg;
> +
> + spin_lock_init(&chan->lock);
> + INIT_LIST_HEAD(&chan->pending_list);
> + INIT_LIST_HEAD(&chan->done_list);
> +
> + /* Retrieve the channel properties from the device tree */
> + has_dre = of_property_read_bool(node, "xlnx,include-dre");
> +
> + chan->genlock = of_property_read_bool(node, "xlnx,genlock-mode");
> +
> + err = of_property_read_u32(node, "xlnx,datawidth", &value);
> + if (!err) {
> + u32 width = value >> 3; /* Convert bits to bytes */
> +
> + /* If data width is greater than 8 bytes, DRE is not in hw */
> + if (width > 8)
> + has_dre = false;
> +
> + if (!has_dre)
> + xdev->common.copy_align = fls(width - 1);
> + } else {
> + dev_err(xdev->dev, "missing xlnx,datawidth property\n");
> + return err;
> + }
Can you please convert this to:
if (err) {
dev_err(...);
return err;
}
That way we can avoid the else clause.
> +
> + if (of_device_is_compatible(node, "xlnx,axi-vdma-mm2s-channel")) {
> + chan->direction = DMA_MEM_TO_DEV;
> + chan->id = 0;
> +
> + chan->ctrl_offset = XILINX_VDMA_MM2S_CTRL_OFFSET;
> + chan->desc_offset = XILINX_VDMA_MM2S_DESC_OFFSET;
> +
> + if (xdev->flush_on_fsync == XILINX_VDMA_FLUSH_BOTH ||
> + xdev->flush_on_fsync == XILINX_VDMA_FLUSH_MM2S)
> + chan->flush_on_fsync = true;
> + } else if (of_device_is_compatible(node,
> + "xlnx,axi-vdma-s2mm-channel")) {
> + chan->direction = DMA_DEV_TO_MEM;
> + chan->id = 1;
> +
> + chan->ctrl_offset = XILINX_VDMA_S2MM_CTRL_OFFSET;
> + chan->desc_offset = XILINX_VDMA_S2MM_DESC_OFFSET;
> +
> + if (xdev->flush_on_fsync == XILINX_VDMA_FLUSH_BOTH ||
> + xdev->flush_on_fsync == XILINX_VDMA_FLUSH_S2MM)
> + chan->flush_on_fsync = true;
> + } else {
> + dev_err(xdev->dev, "Invalid channel compatible node\n");
> + return -EINVAL;
> + }
> +
> + /* Request the interrupt */
> + chan->irq = irq_of_parse_and_map(node, 0);
> + err = devm_request_irq(xdev->dev, chan->irq, xilinx_vdma_irq_handler,
> + IRQF_SHARED, "xilinx-vdma-controller", chan);
> + if (err) {
> + dev_err(xdev->dev, "unable to request IRQ\n");
It might be worth to also tell the IRQ number that failed
to register.
> + return err;
> + }
> +
> + /* Initialize the DMA channel and add it to the DMA engine channels
> + * list.
> + */
> + chan->common.device = &xdev->common;
> +
> + list_add_tail(&chan->common.device_node, &xdev->common.channels);
> + xdev->chan[chan->id] = chan;
> +
> + /* Reset the channel */
> + err = xilinx_vdma_chan_reset(chan);
> + if (err < 0) {
> + dev_err(xdev->dev, "Reset channel failed\n");
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * struct of_dma_filter_xilinx_args - Channel filter args
> + * @dev: DMA device structure
> + * @chan_id: Channel id
> + */
> +struct of_dma_filter_xilinx_args {
> + struct dma_device *dev;
> + u32 chan_id;
> +};
> +
> +/**
> + * xilinx_vdma_dt_filter - VDMA channel filter function
> + * @chan: DMA channel pointer
> + * @param: Filter match value
> + *
> + * Return: true/false based on the result
> + */
> +static bool xilinx_vdma_dt_filter(struct dma_chan *chan, void *param)
> +{
> + struct of_dma_filter_xilinx_args *args = param;
> +
> + return chan->device == args->dev && chan->chan_id == args->chan_id;
> +}
> +
> +/**
> + * of_dma_xilinx_xlate - Translation function
> + * @dma_spec: Pointer to DMA specifier as found in the device tree
> + * @ofdma: Pointer to DMA controller data
> + *
> + * Return: DMA channel pointer on success and NULL on error
> + */
> +static struct dma_chan *of_dma_xilinx_xlate(struct of_phandle_args *dma_spec,
> + struct of_dma *ofdma)
> +{
> + struct of_dma_filter_xilinx_args args;
> + dma_cap_mask_t cap;
> +
> + args.dev = ofdma->of_dma_data;
> + if (!args.dev)
> + return NULL;
> +
> + if (dma_spec->args_count != 1)
> + return NULL;
> +
> + dma_cap_zero(cap);
> + dma_cap_set(DMA_SLAVE, cap);
> +
> + args.chan_id = dma_spec->args[0];
> +
> + return dma_request_channel(cap, xilinx_vdma_dt_filter, &args);
> +}
> +
> +/**
> + * xilinx_vdma_probe - Driver probe function
> + * @pdev: Pointer to the platform_device structure
> + *
> + * Return: '0' on success and failure value on error
> + */
> +static int xilinx_vdma_probe(struct platform_device *pdev)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct xilinx_vdma_device *xdev;
> + struct device_node *child;
> + struct resource *io;
> + u32 num_frames;
> + int i, err;
> +
> + dev_info(&pdev->dev, "Probing xilinx axi vdma engine\n");
> +
> + /* Allocate and initialize the DMA engine structure */
> + xdev = devm_kzalloc(&pdev->dev, sizeof(*xdev), GFP_KERNEL);
> + if (!xdev)
> + return -ENOMEM;
> +
> + xdev->dev = &pdev->dev;
> +
> + /* Request and map I/O memory */
> + io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + xdev->regs = devm_ioremap_resource(&pdev->dev, io);
> + if (IS_ERR(xdev->regs))
> + return PTR_ERR(xdev->regs);
> +
> + /* Retrieve the DMA engine properties from the device tree */
> + xdev->has_sg = of_property_read_bool(node, "xlnx,include-sg");
> +
> + err = of_property_read_u32(node, "xlnx,num-fstores", &num_frames);
> + if (err < 0) {
> + dev_err(xdev->dev, "missing xlnx,num-fstores property\n");
> + return err;
> + }
> +
> + of_property_read_u32(node, "xlnx,flush-fsync", &xdev->flush_on_fsync);
Error check?
> +
> + /* Initialize the DMA engine */
> + xdev->common.dev = &pdev->dev;
> +
> + INIT_LIST_HEAD(&xdev->common.channels);
> + dma_cap_set(DMA_SLAVE, xdev->common.cap_mask);
> + dma_cap_set(DMA_PRIVATE, xdev->common.cap_mask);
> +
> + xdev->common.device_alloc_chan_resources =
> + xilinx_vdma_alloc_chan_resources;
> + xdev->common.device_free_chan_resources =
> + xilinx_vdma_free_chan_resources;
> + xdev->common.device_prep_slave_sg = xilinx_vdma_prep_slave_sg;
> + xdev->common.device_control = xilinx_vdma_device_control;
> + xdev->common.device_tx_status = xilinx_vdma_tx_status;
> + xdev->common.device_issue_pending = xilinx_vdma_issue_pending;
> +
> + platform_set_drvdata(pdev, xdev);
> +
> + /* Initialize the channels */
> + for_each_child_of_node(node, child) {
> + err = xilinx_vdma_chan_probe(xdev, child);
> + if (err < 0)
> + goto error;
> + }
> +
> + for (i = 0; i < XILINX_VDMA_MAX_CHANS_PER_DEVICE; i++) {
> + if (xdev->chan[i])
> + xdev->chan[i]->num_frms = num_frames;
> + }
> +
> + /* Register the DMA engine with the core */
> + dma_async_device_register(&xdev->common);
> +
> + err = of_dma_controller_register(node, of_dma_xilinx_xlate,
> + &xdev->common);
> + if (err < 0) {
> + dev_err(&pdev->dev, "Unable to register DMA to DT\n");
> + dma_async_device_unregister(&xdev->common);
> + goto error;
> + }
> +
> + return 0;
> +
> +error:
> + for (i = 0; i < XILINX_VDMA_MAX_CHANS_PER_DEVICE; i++) {
> + if (xdev->chan[i])
> + xilinx_vdma_chan_remove(xdev->chan[i]);
> + }
> +
> + return err;
> +}
> +
> +/**
> + * xilinx_vdma_remove - Driver remove function
> + * @pdev: Pointer to the platform_device structure
> + *
> + * Return: Always '0'
> + */
> +static int xilinx_vdma_remove(struct platform_device *pdev)
> +{
> + struct xilinx_vdma_device *xdev;
> + int i;
> +
> + of_dma_controller_free(pdev->dev.of_node);
> +
> + xdev = platform_get_drvdata(pdev);
> + dma_async_device_unregister(&xdev->common);
> +
> + for (i = 0; i < XILINX_VDMA_MAX_CHANS_PER_DEVICE; i++) {
> + if (xdev->chan[i])
> + xilinx_vdma_chan_remove(xdev->chan[i]);
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id xilinx_vdma_of_ids[] = {
> + { .compatible = "xlnx,axi-vdma-1.00.a",},
> + {}
> +};
> +
> +static struct platform_driver xilinx_vdma_driver = {
> + .driver = {
> + .name = "xilinx-vdma",
> + .owner = THIS_MODULE,
> + .of_match_table = xilinx_vdma_of_ids,
> + },
> + .probe = xilinx_vdma_probe,
> + .remove = xilinx_vdma_remove,
> +};
> +
> +module_platform_driver(xilinx_vdma_driver);
> +
> +MODULE_AUTHOR("Xilinx, Inc.");
> +MODULE_DESCRIPTION("Xilinx VDMA driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/amba/xilinx_dma.h b/include/linux/amba/xilinx_dma.h
> new file mode 100644
> index 0000000..48a8c8b
> --- /dev/null
> +++ b/include/linux/amba/xilinx_dma.h
> @@ -0,0 +1,50 @@
> +/*
> + * Xilinx DMA Engine drivers support header file
> + *
> + * Copyright (C) 2010-2014 Xilinx, Inc. All rights reserved.
> + *
> + * This 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.
> + */
> +
> +#ifndef __DMA_XILINX_DMA_H
> +#define __DMA_XILINX_DMA_H
> +
> +#include <linux/dma-mapping.h>
> +#include <linux/dmaengine.h>
> +
> +/**
> + * struct xilinx_vdma_config - VDMA Configuration structure
> + * @vsize: Vertical size
> + * @hsize: Horizontal size
> + * @stride: Stride
> + * @frm_dly: Frame delay
> + * @gen_lock: Whether in gen-lock mode
> + * @master: Master that it syncs to
> + * @frm_cnt_en: Enable frame count enable
> + * @park: Whether wants to park
> + * @park_frm: Frame to park on
> + * @coalesc: Interrupt coalescing threshold
> + * @delay: Delay counter
> + * @reset: Reset Channel
> + * @ext_fsync: External Frame Sync source
> + */
> +struct xilinx_vdma_config {
> + int vsize;
> + int hsize;
> + int stride;
> + int frm_dly;
> + int gen_lock;
> + int master;
> + int frm_cnt_en;
> + int park;
> + int park_frm;
> + int coalesc;
> + int delay;
> + int reset;
> + int ext_fsync;
> +};
> +
> +#endif
> --
--
Regards,
Levente Kurusa
^ permalink raw reply
* [PATCH v3 10/24] drm/i2c: tda998x: don't read write-only registers
From: Russell King - ARM Linux @ 2014-01-22 21:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140119195842.24c40e22@armhf>
On Sun, Jan 19, 2014 at 07:58:42PM +0100, Jean-Francois Moine wrote:
> This patch takes care of the write-only registers of the tda998x.
>
> The value 'MAT_CONTRL_MAT_SC(1)' in the register MAT_CONTRL has been
> set as it is at reset time.
>
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
> ---
> v3
> - remarks from Russell King
> - don't move the sync polarity setting after the setting of the
> register TBG_CNTRL_0 which must be the last setting of the
> init sequence
This is better, except I find that there's an additional change in this
version which wasn't in the original patch 9:
> /* must be last register set: */
> - reg_clear(priv, REG_TBG_CNTRL_0, TBG_CNTRL_0_SYNC_ONCE);
> + reg_write(priv, REG_TBG_CNTRL_0, 0);
Register changes which have a potential effect shouldn't be part of a
patch which is really only trying to avoid reading from write only
registers.
This could be a potential functional change - and it's probably one
which Rob Clark should at least be made aware of. As I commented last
time, when you're changing register values in an otherwise innocuous
patch, you should comment about them in the patch description.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ 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