* Re: [PATCH v2 2/4] MFD: TPS65218: Add driver for the TPS65218 PMIC
From: Lee Jones @ 2014-02-07 11:12 UTC (permalink / raw)
To: Keerthy
Cc: rob.herring, pawel.moll, mark.rutland, swarren, ijc+devicetree,
rob, sameo, grant.likely, lgirdwood, broonie, devicetree,
linux-doc, linux-kernel, linux-omap
In-Reply-To: <1391665814-18814-3-git-send-email-j-keerthy@ti.com>
On Thu, 06 Feb 2014, Keerthy wrote:
> The TPS65218 chip is a power management IC for Portable Navigation Systems
> and Tablet Computing devices. It contains the following components:
>
> - Regulators.
> - Over Temperature warning and Shut down.
>
> This patch adds support for tps65218 mfd device. At this time only
> the regulator functionality is made available.
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
> drivers/mfd/Kconfig | 15 +++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/tps65218.c | 281 +++++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/tps65218.h | 284 ++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 581 insertions(+)
> create mode 100644 drivers/mfd/tps65218.c
> create mode 100644 include/linux/mfd/tps65218.h
Applied, thanks.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 3/8] mfd: da9055: Add DT support for PMIC
From: Lee Jones @ 2014-02-07 11:03 UTC (permalink / raw)
To: Adam Thomson
Cc: Mark Brown, alsa-devel, linux-kernel, devicetree, Rob Herring,
Linus Walleij, Dmitry Torokhov, Alessandro Zummo, Guenter Roeck
In-Reply-To: <46f41a14205ac39de3adb8c87d2a6dba557ad8d1.1391705989.git.Adam.Thomson.Opensource@diasemi.com>
> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> ---
> Documentation/devicetree/bindings/mfd/da9055.txt | 73 ++++++++++++++++++++++
> drivers/mfd/da9055-i2c.c | 8 +++
Can you break this up please.
Bindings should be separate to the binding document.
> 2 files changed, 81 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mfd/da9055.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/da9055.txt b/Documentation/devicetree/bindings/mfd/da9055.txt
> new file mode 100644
> index 0000000..f903c3f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/da9055.txt
> @@ -0,0 +1,73 @@
> +* Dialog DA9055 Power Management Integrated Circuit (PMIC)
> +
> +DA9055 consists of a large and varied group of sub-devices (I2C Only):
> +
> +Device Supply Names Description
> +------ ------------ -----------
> +da9055-gpio : : GPIOs
> +da9055-regulator : : Regulators
> +da9055-onkey : : On key
> +da9055-rtc : : RTC
> +da9055-hwmon : : ADC
> +da9055-watchdog : : Watchdog
> +
> +The CODEC device in DA9055 has a separate, configurable I2C address and so
> +is instantiated separately from the PMIC.
> +
> +For details on accompanying CODEC I2C device, see the following:
> +Documentation/devicetree/bindings/sound/da9055.txt
> +
> +======
> +
> +Required properties:
> +- compatible : Should be "dlg,da9055-pmic"
> +- reg: Specifies the I2C slave address (defaults to 0x5a but can be modified)
> +- interrupt-parent: Specifies the phandle of the interrupt controller to which
> + the IRQs from da9055 are delivered to.
> +- interrupts: IRQ line info for da9055 chip.
> +- interrupt-controller: da9055 has internal IRQs (has own IRQ domain).
> +- #interrupt-cells: Should be 1, is the local IRQ number for da9055.
> +
> +Sub-nodes:
> +- regulators : Contain the regulator nodes. The DA9055 regulators are
> + bound using their names as listed below:
> +
> + buck1 : regulator BUCK1
> + buck2 : regulator BUCK2
> + ldo1 : regulator LDO1
> + ldo2 : regulator LDO2
> + ldo3 : regulator LDO3
> + ldo4 : regulator LDO4
> + ldo5 : regulator LDO5
> + ldo6 : regulator LDO6
> +
> + The bindings details of individual regulator device can be found in:
> + Documentation/devicetree/bindings/regulator/regulator.txt
> +
> +
> +Example:
> +
> + pmic: da9055-pmic@5a {
> + compatible = "dlg,da9055-pmic";
> + reg = <0x5a>;
> + interrupt-parent = <&intc>;
> + interrupts = <5 0x8>;
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + regulators {
> + buck1: BUCK1 {
> + regulator-min-microvolt = <725000>;
> + regulator-max-microvolt = <2075000>;
> + };
> +
> + buck2: BUCK2 {
> + regulator-min-microvolt = <925000>;
> + regulator-max-microvolt = <2500000>;
> + };
> + ldo1: LDO1 {
> + regulator-min-microvolt = <900000>;
> + regulator-max-microvolt = <3300000>;
> + };
> + };
> + };
> diff --git a/drivers/mfd/da9055-i2c.c b/drivers/mfd/da9055-i2c.c
> index 8103e43..366a3e2 100644
> --- a/drivers/mfd/da9055-i2c.c
> +++ b/drivers/mfd/da9055-i2c.c
> @@ -15,6 +15,8 @@
> #include <linux/device.h>
> #include <linux/i2c.h>
> #include <linux/err.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>
> #include <linux/mfd/da9055/core.h>
>
> @@ -66,6 +68,11 @@ static struct i2c_device_id da9055_i2c_id[] = {
> };
> MODULE_DEVICE_TABLE(i2c, da9055_i2c_id);
>
> +static const struct of_device_id da9055_of_match[] = {
> + { .compatible = "dlg,da9055-pmic", .data = (void *)&da9055_i2c_id[0] },
Entangling the i2c_device_id table with the of_device_id table sounds
a bit too wacky for my liking. Where do you even use it?
> + { }
> +};
> +
> static struct i2c_driver da9055_i2c_driver = {
> .probe = da9055_i2c_probe,
> .remove = da9055_i2c_remove,
> @@ -73,6 +80,7 @@ static struct i2c_driver da9055_i2c_driver = {
> .driver = {
> .name = "da9055-pmic",
> .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(da9055_of_match),
> },
> };
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v2 1/8] ASoC: da9055: Fix device registration of PMIC and CODEC devices
From: Lee Jones @ 2014-02-07 10:56 UTC (permalink / raw)
To: Adam Thomson
Cc: Mark Brown, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Linus Walleij,
Dmitry Torokhov, Alessandro Zummo, Guenter Roeck
In-Reply-To: <6a398d176b0b2bf8792d27bd5e2995fd96afb32d.1391705989.git.Adam.Thomson.Opensource-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org>
> Currently the I2C device Ids conflict for the MFD and CODEC so
> cannot be both instantiated on one platform. This patch updates
> the Ids and names to make them unique from each other.
>
> It should be noted that the I2C addresses for both PMIC and CODEC
> are modifiable so instantiation of the two are kept as separate
> devices, rather than instantiating the CODEC from the MFD code.
>
> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org>
> Acked-by: Mark Brown <broonie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> drivers/mfd/da9055-i2c.c | 12 ++++++++++--
> sound/soc/codecs/da9055.c | 11 +++++++++--
> 2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mfd/da9055-i2c.c b/drivers/mfd/da9055-i2c.c
> index 13af7e5..8103e43 100644
> --- a/drivers/mfd/da9055-i2c.c
> +++ b/drivers/mfd/da9055-i2c.c
> @@ -53,17 +53,25 @@ static int da9055_i2c_remove(struct i2c_client *i2c)
> return 0;
> }
>
> +/*
> + * DO NOT change the device Ids. The naming is intentionally specific as both
> + * the PMIC and CODEC parts of this chip are instantiated separately as I2C
> + * devices (both have configurable I2C addresses, and are to all intents and
> + * purposes separate). As a result there are specific DA9055 ids for PMIC
> + * and CODEC, which must be different to operate together.
> + */
I'm not sure this comment is required.
Most device IDs are named this way.
> static struct i2c_device_id da9055_i2c_id[] = {
> - {"da9055", 0},
> + {"da9055-pmic", 0},
> { }
> };
> +MODULE_DEVICE_TABLE(i2c, da9055_i2c_id);
>
> static struct i2c_driver da9055_i2c_driver = {
> .probe = da9055_i2c_probe,
> .remove = da9055_i2c_remove,
> .id_table = da9055_i2c_id,
> .driver = {
> - .name = "da9055",
> + .name = "da9055-pmic",
> .owner = THIS_MODULE,
> },
> };
> diff --git a/sound/soc/codecs/da9055.c b/sound/soc/codecs/da9055.c
> index 52b79a4..4228126 100644
> --- a/sound/soc/codecs/da9055.c
> +++ b/sound/soc/codecs/da9055.c
> @@ -1523,8 +1523,15 @@ static int da9055_remove(struct i2c_client *client)
> return 0;
> }
>
> +/*
> + * DO NOT change the device Ids. The naming is intentionally specific as both
> + * the CODEC and PMIC parts of this chip are instantiated separately as I2C
> + * devices (both have configurable I2C addresses, and are to all intents and
> + * purposes separate). As a result there are specific DA9055 Ids for CODEC
> + * and PMIC, which must be different to operate together.
> + */
And here, really, no need.
> static const struct i2c_device_id da9055_i2c_id[] = {
> - { "da9055", 0 },
> + { "da9055-codec", 0 },
> { }
> };
> MODULE_DEVICE_TABLE(i2c, da9055_i2c_id);
> @@ -1532,7 +1539,7 @@ MODULE_DEVICE_TABLE(i2c, da9055_i2c_id);
> /* I2C codec control layer */
> static struct i2c_driver da9055_i2c_driver = {
> .driver = {
> - .name = "da9055",
> + .name = "da9055-codec",
> .owner = THIS_MODULE,
> },
> .probe = da9055_i2c_probe,
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2 3/3] ARM: STi: Add STiH416 ethernet support.
From: srinivas.kandagatla @ 2014-02-07 10:55 UTC (permalink / raw)
To: netdev
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Rob Landley, Russell King, Srinivas Kandagatla, Stuart Menefy,
Giuseppe Cavallaro, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, kernel, davem
In-Reply-To: <1391770455-24291-1-git-send-email-srinivas.kandagatla@st.com>
From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
This patch adds support to STiH416 SOC, which has two ethernet
snps,dwmac controllers version 3.710. With this patch B2000 and B2020
boards can boot with ethernet in MII and RGMII modes.
Tested on both B2020 and B2000.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
arch/arm/boot/dts/stih416-clock.dtsi | 14 ++++
arch/arm/boot/dts/stih416-pinctrl.dtsi | 109 ++++++++++++++++++++++++++++++++
arch/arm/boot/dts/stih416.dtsi | 44 +++++++++++++
3 files changed, 167 insertions(+)
diff --git a/arch/arm/boot/dts/stih416-clock.dtsi b/arch/arm/boot/dts/stih416-clock.dtsi
index 7026bf1..a6942c7 100644
--- a/arch/arm/boot/dts/stih416-clock.dtsi
+++ b/arch/arm/boot/dts/stih416-clock.dtsi
@@ -37,5 +37,19 @@
clock-frequency = <100000000>;
clock-output-names = "CLK_S_ICN_REG_0";
};
+
+ CLK_S_GMAC0_PHY: clockgenA1@7 {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <25000000>;
+ clock-output-names = "CLK_S_GMAC0_PHY";
+ };
+
+ CLK_S_ETH1_PHY: clockgenA0@7 {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <25000000>;
+ clock-output-names = "CLK_S_ETH1_PHY";
+ };
};
};
diff --git a/arch/arm/boot/dts/stih416-pinctrl.dtsi b/arch/arm/boot/dts/stih416-pinctrl.dtsi
index 8863c38..c4beef2 100644
--- a/arch/arm/boot/dts/stih416-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stih416-pinctrl.dtsi
@@ -132,6 +132,58 @@
};
};
};
+
+ gmac1 {
+ pinctrl_mii1: mii1 {
+ st,pins {
+ txd0 = <&PIO0 0 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+ txd1 = <&PIO0 1 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+ txd2 = <&PIO0 2 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+ txd3 = <&PIO0 3 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+ txer = <&PIO0 4 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+ txen = <&PIO0 5 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+ txclk = <&PIO0 6 ALT1 IN NICLK 0 CLK_A>;
+ col = <&PIO0 7 ALT1 IN BYPASS 1000>;
+
+ mdio = <&PIO1 0 ALT1 OUT BYPASS 1500>;
+ mdc = <&PIO1 1 ALT1 OUT NICLK 0 CLK_A>;
+ crs = <&PIO1 2 ALT1 IN BYPASS 1000>;
+ mdint = <&PIO1 3 ALT1 IN BYPASS 0>;
+ rxd0 = <&PIO1 4 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+ rxd1 = <&PIO1 5 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+ rxd2 = <&PIO1 6 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+ rxd3 = <&PIO1 7 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+
+ rxdv = <&PIO2 0 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+ rx_er = <&PIO2 1 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+ rxclk = <&PIO2 2 ALT1 IN NICLK 0 CLK_A>;
+ phyclk = <&PIO2 3 ALT1 OUT NICLK 0 CLK_A>;
+ };
+ };
+ pinctrl_rgmii1: rgmii1-0 {
+ st,pins {
+ txd0 = <&PIO0 0 ALT1 OUT DE_IO 500 CLK_A>;
+ txd1 = <&PIO0 1 ALT1 OUT DE_IO 500 CLK_A>;
+ txd2 = <&PIO0 2 ALT1 OUT DE_IO 500 CLK_A>;
+ txd3 = <&PIO0 3 ALT1 OUT DE_IO 500 CLK_A>;
+ txen = <&PIO0 5 ALT1 OUT DE_IO 0 CLK_A>;
+ txclk = <&PIO0 6 ALT1 IN NICLK 0 CLK_A>;
+
+ mdio = <&PIO1 0 ALT1 OUT BYPASS 0>;
+ mdc = <&PIO1 1 ALT1 OUT NICLK 0 CLK_A>;
+ rxd0 = <&PIO1 4 ALT1 IN DE_IO 500 CLK_A>;
+ rxd1 = <&PIO1 5 ALT1 IN DE_IO 500 CLK_A>;
+ rxd2 = <&PIO1 6 ALT1 IN DE_IO 500 CLK_A>;
+ rxd3 = <&PIO1 7 ALT1 IN DE_IO 500 CLK_A>;
+
+ rxdv = <&PIO2 0 ALT1 IN DE_IO 500 CLK_A>;
+ rxclk = <&PIO2 2 ALT1 IN NICLK 0 CLK_A>;
+ phyclk = <&PIO2 3 ALT4 OUT NICLK 0 CLK_B>;
+
+ clk125= <&PIO3 7 ALT4 IN NICLK 0 CLK_A>;
+ };
+ };
+ };
};
pin-controller-front {
@@ -322,6 +374,63 @@
};
};
};
+
+ gmac0 {
+ pinctrl_mii0: mii0 {
+ st,pins {
+ mdint = <&PIO13 6 ALT2 IN BYPASS 0>;
+ txen = <&PIO13 7 ALT2 OUT SE_NICLK_IO 0 CLK_A>;
+ txd0 = <&PIO14 0 ALT2 OUT SE_NICLK_IO 0 CLK_A>;
+ txd1 = <&PIO14 1 ALT2 OUT SE_NICLK_IO 0 CLK_A>;
+ txd2 = <&PIO14 2 ALT2 OUT SE_NICLK_IO 0 CLK_B>;
+ txd3 = <&PIO14 3 ALT2 OUT SE_NICLK_IO 0 CLK_B>;
+
+ txclk = <&PIO15 0 ALT2 IN NICLK 0 CLK_A>;
+ txer = <&PIO15 1 ALT2 OUT SE_NICLK_IO 0 CLK_A>;
+ crs = <&PIO15 2 ALT2 IN BYPASS 1000>;
+ col = <&PIO15 3 ALT2 IN BYPASS 1000>;
+ mdio= <&PIO15 4 ALT2 OUT BYPASS 1500>;
+ mdc = <&PIO15 5 ALT2 OUT NICLK 0 CLK_B>;
+
+ rxd0 = <&PIO16 0 ALT2 IN SE_NICLK_IO 0 CLK_A>;
+ rxd1 = <&PIO16 1 ALT2 IN SE_NICLK_IO 0 CLK_A>;
+ rxd2 = <&PIO16 2 ALT2 IN SE_NICLK_IO 0 CLK_A>;
+ rxd3 = <&PIO16 3 ALT2 IN SE_NICLK_IO 0 CLK_A>;
+ rxdv = <&PIO15 6 ALT2 IN SE_NICLK_IO 0 CLK_A>;
+ rx_er = <&PIO15 7 ALT2 IN SE_NICLK_IO 0 CLK_A>;
+ rxclk = <&PIO17 0 ALT2 IN NICLK 0 CLK_A>;
+ phyclk = <&PIO13 5 ALT2 OUT NICLK 0 CLK_B>;
+ };
+ };
+
+ pinctrl_gmii0: gmii0 {
+ st,pins {
+ };
+ };
+ pinctrl_rgmii0: rgmii0 {
+ st,pins {
+ phyclk = <&PIO13 5 ALT4 OUT NICLK 0 CLK_B>;
+ txen = <&PIO13 7 ALT2 OUT DE_IO 0 CLK_A>;
+ txd0 = <&PIO14 0 ALT2 OUT DE_IO 500 CLK_A>;
+ txd1 = <&PIO14 1 ALT2 OUT DE_IO 500 CLK_A>;
+ txd2 = <&PIO14 2 ALT2 OUT DE_IO 500 CLK_B>;
+ txd3 = <&PIO14 3 ALT2 OUT DE_IO 500 CLK_B>;
+ txclk = <&PIO15 0 ALT2 IN NICLK 0 CLK_A>;
+
+ mdio = <&PIO15 4 ALT2 OUT BYPASS 0>;
+ mdc = <&PIO15 5 ALT2 OUT NICLK 0 CLK_B>;
+
+ rxdv = <&PIO15 6 ALT2 IN DE_IO 500 CLK_A>;
+ rxd0 =<&PIO16 0 ALT2 IN DE_IO 500 CLK_A>;
+ rxd1 =<&PIO16 1 ALT2 IN DE_IO 500 CLK_A>;
+ rxd2 =<&PIO16 2 ALT2 IN DE_IO 500 CLK_A>;
+ rxd3 =<&PIO16 3 ALT2 IN DE_IO 500 CLK_A>;
+ rxclk =<&PIO17 0 ALT2 IN NICLK 0 CLK_A>;
+
+ clk125=<&PIO17 6 ALT1 IN NICLK 0 CLK_A>;
+ };
+ };
+ };
};
pin-controller-fvdp-fe {
diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
index 788ba5b..a96055b 100644
--- a/arch/arm/boot/dts/stih416.dtsi
+++ b/arch/arm/boot/dts/stih416.dtsi
@@ -156,5 +156,49 @@
status = "disabled";
};
+
+ ethernet0: dwmac@fe810000 {
+ device_type = "network";
+ compatible = "st,stih416-dwmac", "snps,dwmac", "snps,dwmac-3.710";
+ status = "disabled";
+ reg = <0xfe810000 0x8000>, <0x8bc 0x4>;
+ reg-names = "stmmaceth", "sti-ethconf";
+
+ interrupts = <0 133 0>, <0 134 0>, <0 135 0>;
+ interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
+
+ snps,pbl = <32>;
+ snps,mixed-burst;
+
+ st,syscon = <&syscfg_rear>;
+ resets = <&softreset STIH416_ETH0_SOFTRESET>;
+ reset-names = "stmmaceth";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_mii0>;
+ clock-names = "stmmaceth";
+ clocks = <&CLK_S_GMAC0_PHY>;
+ };
+
+ ethernet1: dwmac@fef08000 {
+ device_type = "network";
+ compatible = "st,stih416-dwmac", "snps,dwmac", "snps,dwmac-3.710";
+ status = "disabled";
+ reg = <0xfef08000 0x8000>, <0x7f0 0x4>;
+ reg-names = "stmmaceth", "sti-ethconf";
+ interrupts = <0 136 0>, <0 137 0>, <0 138 0>;
+ interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
+
+ snps,pbl = <32>;
+ snps,mixed-burst;
+
+ st,syscon = <&syscfg_sbc>;
+
+ resets = <&softreset STIH416_ETH1_SOFTRESET>;
+ reset-names = "stmmaceth";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_mii1>;
+ clock-names = "stmmaceth";
+ clocks = <&CLK_S_ETH1_PHY>;
+ };
};
};
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 2/3] ARM: STi: Add STiH415 ethernet support.
From: srinivas.kandagatla @ 2014-02-07 10:55 UTC (permalink / raw)
To: netdev
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Rob Landley, Russell King, Srinivas Kandagatla, Stuart Menefy,
Giuseppe Cavallaro, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, kernel, davem
In-Reply-To: <1391770455-24291-1-git-send-email-srinivas.kandagatla@st.com>
From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
This patch adds support to STiH415 SOC, which has two ethernet
snps,dwmac controllers version 3.610. With this patch B2000 and B2020
boards can boot with ethernet in MII and RGMII modes.
Tested on both B2020 and B2000.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
arch/arm/boot/dts/stih415-clock.dtsi | 14 ++++
arch/arm/boot/dts/stih415-pinctrl.dtsi | 121 ++++++++++++++++++++++++++++++++
arch/arm/boot/dts/stih415.dtsi | 48 +++++++++++++
arch/arm/boot/dts/stih41x-b2000.dtsi | 22 ++++++
arch/arm/boot/dts/stih41x-b2020.dtsi | 26 +++++++
5 files changed, 231 insertions(+)
diff --git a/arch/arm/boot/dts/stih415-clock.dtsi b/arch/arm/boot/dts/stih415-clock.dtsi
index 174c799..d047dbc 100644
--- a/arch/arm/boot/dts/stih415-clock.dtsi
+++ b/arch/arm/boot/dts/stih415-clock.dtsi
@@ -34,5 +34,19 @@
compatible = "fixed-clock";
clock-frequency = <100000000>;
};
+
+ CLKS_GMAC0_PHY: clockgenA1@7 {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <25000000>;
+ clock-output-names = "CLKS_GMAC0_PHY";
+ };
+
+ CLKS_ETH1_PHY: clockgenA0@7 {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ clock-frequency = <25000000>;
+ clock-output-names = "CLKS_ETH1_PHY";
+ };
};
};
diff --git a/arch/arm/boot/dts/stih415-pinctrl.dtsi b/arch/arm/boot/dts/stih415-pinctrl.dtsi
index 887c5e5..9ca20aa 100644
--- a/arch/arm/boot/dts/stih415-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stih415-pinctrl.dtsi
@@ -119,6 +119,56 @@
};
};
};
+
+ gmac1 {
+ pinctrl_mii1: mii1 {
+ st,pins {
+ txd0 = <&PIO0 0 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+ txd1 = <&PIO0 1 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+ txd2 = <&PIO0 2 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+ txd3 = <&PIO0 3 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+ txer = <&PIO0 4 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+ txen = <&PIO0 5 ALT1 OUT SE_NICLK_IO 0 CLK_A>;
+ txclk = <&PIO0 6 ALT1 IN NICLK 0 CLK_A>;
+ col = <&PIO0 7 ALT1 IN BYPASS 1000>;
+ mdio = <&PIO1 0 ALT1 OUT BYPASS 0>;
+ mdc = <&PIO1 1 ALT1 OUT NICLK 0 CLK_A>;
+ crs = <&PIO1 2 ALT1 IN BYPASS 1000>;
+ mdint = <&PIO1 3 ALT1 IN BYPASS 0>;
+ rxd0 = <&PIO1 4 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+ rxd1 = <&PIO1 5 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+ rxd2 = <&PIO1 6 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+ rxd3 = <&PIO1 7 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+ rxdv = <&PIO2 0 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+ rx_er = <&PIO2 1 ALT1 IN SE_NICLK_IO 0 CLK_A>;
+ rxclk = <&PIO2 2 ALT1 IN NICLK 0 CLK_A>;
+ phyclk = <&PIO2 3 ALT1 IN NICLK 1000 CLK_A>;
+ };
+ };
+
+ pinctrl_rgmii1: rgmii1-0 {
+ st,pins {
+ txd0 = <&PIO0 0 ALT1 OUT DE_IO 1000 CLK_A>;
+ txd1 = <&PIO0 1 ALT1 OUT DE_IO 1000 CLK_A>;
+ txd2 = <&PIO0 2 ALT1 OUT DE_IO 1000 CLK_A>;
+ txd3 = <&PIO0 3 ALT1 OUT DE_IO 1000 CLK_A>;
+ txen = <&PIO0 5 ALT1 OUT DE_IO 0 CLK_A>;
+ txclk = <&PIO0 6 ALT1 IN NICLK 0 CLK_A>;
+ mdio = <&PIO1 0 ALT1 OUT BYPASS 0>;
+ mdc = <&PIO1 1 ALT1 OUT NICLK 0 CLK_A>;
+ rxd0 = <&PIO1 4 ALT1 IN DE_IO 0 CLK_A>;
+ rxd1 = <&PIO1 5 ALT1 IN DE_IO 0 CLK_A>;
+ rxd2 = <&PIO1 6 ALT1 IN DE_IO 0 CLK_A>;
+ rxd3 = <&PIO1 7 ALT1 IN DE_IO 0 CLK_A>;
+
+ rxdv = <&PIO2 0 ALT1 IN DE_IO 500 CLK_A>;
+ rxclk = <&PIO2 2 ALT1 IN NICLK 0 CLK_A>;
+ phyclk = <&PIO2 3 ALT4 OUT NICLK 0 CLK_B>;
+
+ clk125= <&PIO3 7 ALT4 IN NICLK 0 CLK_A>;
+ };
+ };
+ };
};
pin-controller-front {
@@ -284,6 +334,77 @@
};
};
};
+
+ gmac0{
+ pinctrl_mii0: mii0 {
+ st,pins {
+ mdint = <&PIO13 6 ALT2 IN BYPASS 0>;
+ txen = <&PIO13 7 ALT2 OUT SE_NICLK_IO 0 CLK_A>;
+
+ txd0 = <&PIO14 0 ALT2 OUT SE_NICLK_IO 0 CLK_A>;
+ txd1 = <&PIO14 1 ALT2 OUT SE_NICLK_IO 0 CLK_A>;
+ txd2 = <&PIO14 2 ALT2 OUT SE_NICLK_IO 0 CLK_B>;
+ txd3 = <&PIO14 3 ALT2 OUT SE_NICLK_IO 0 CLK_B>;
+
+ txclk = <&PIO15 0 ALT2 IN NICLK 0 CLK_A>;
+ txer = <&PIO15 1 ALT2 OUT SE_NICLK_IO 0 CLK_A>;
+ crs = <&PIO15 2 ALT2 IN BYPASS 1000>;
+ col = <&PIO15 3 ALT2 IN BYPASS 1000>;
+ mdio = <&PIO15 4 ALT2 OUT BYPASS 3000>;
+ mdc = <&PIO15 5 ALT2 OUT NICLK 0 CLK_B>;
+
+ rxd0 = <&PIO16 0 ALT2 IN SE_NICLK_IO 0 CLK_A>;
+ rxd1 = <&PIO16 1 ALT2 IN SE_NICLK_IO 0 CLK_A>;
+ rxd2 = <&PIO16 2 ALT2 IN SE_NICLK_IO 0 CLK_A>;
+ rxd3 = <&PIO16 3 ALT2 IN SE_NICLK_IO 0 CLK_A>;
+ rxdv = <&PIO15 6 ALT2 IN SE_NICLK_IO 0 CLK_A>;
+ rx_er = <&PIO15 7 ALT2 IN SE_NICLK_IO 0 CLK_A>;
+ rxclk = <&PIO17 0 ALT2 IN NICLK 0 CLK_A>;
+ phyclk = <&PIO13 5 ALT2 OUT NICLK 1000 CLK_A>;
+
+ };
+ };
+
+ pinctrl_gmii0: gmii0 {
+ st,pins {
+ mdint = <&PIO13 6 ALT2 IN BYPASS 0>;
+ mdio = <&PIO15 4 ALT2 OUT BYPASS 3000>;
+ mdc = <&PIO15 5 ALT2 OUT NICLK 0 CLK_B>;
+ txen = <&PIO13 7 ALT2 OUT SE_NICLK_IO 3000 CLK_A>;
+
+ txd0 = <&PIO14 0 ALT2 OUT SE_NICLK_IO 3000 CLK_A>;
+ txd1 = <&PIO14 1 ALT2 OUT SE_NICLK_IO 3000 CLK_A>;
+ txd2 = <&PIO14 2 ALT2 OUT SE_NICLK_IO 3000 CLK_B>;
+ txd3 = <&PIO14 3 ALT2 OUT SE_NICLK_IO 3000 CLK_B>;
+ txd4 = <&PIO14 4 ALT2 OUT SE_NICLK_IO 3000 CLK_B>;
+ txd5 = <&PIO14 5 ALT2 OUT SE_NICLK_IO 3000 CLK_B>;
+ txd6 = <&PIO14 6 ALT2 OUT SE_NICLK_IO 3000 CLK_B>;
+ txd7 = <&PIO14 7 ALT2 OUT SE_NICLK_IO 3000 CLK_B>;
+
+ txclk = <&PIO15 0 ALT2 IN NICLK 0 CLK_A>;
+ txer = <&PIO15 1 ALT2 OUT SE_NICLK_IO 3000 CLK_A>;
+ crs = <&PIO15 2 ALT2 IN BYPASS 1000>;
+ col = <&PIO15 3 ALT2 IN BYPASS 1000>;
+ rxdv = <&PIO15 6 ALT2 IN SE_NICLK_IO 1500 CLK_A>;
+ rx_er = <&PIO15 7 ALT2 IN SE_NICLK_IO 1500 CLK_A>;
+
+ rxd0 = <&PIO16 0 ALT2 IN SE_NICLK_IO 1500 CLK_A>;
+ rxd1 = <&PIO16 1 ALT2 IN SE_NICLK_IO 1500 CLK_A>;
+ rxd2 = <&PIO16 2 ALT2 IN SE_NICLK_IO 1500 CLK_A>;
+ rxd3 = <&PIO16 3 ALT2 IN SE_NICLK_IO 1500 CLK_A>;
+ rxd4 = <&PIO16 4 ALT2 IN SE_NICLK_IO 1500 CLK_A>;
+ rxd5 = <&PIO16 5 ALT2 IN SE_NICLK_IO 1500 CLK_A>;
+ rxd6 = <&PIO16 6 ALT2 IN SE_NICLK_IO 1500 CLK_A>;
+ rxd7 = <&PIO16 7 ALT2 IN SE_NICLK_IO 1500 CLK_A>;
+
+ rxclk = <&PIO17 0 ALT2 IN NICLK 0 CLK_A>;
+ clk125 = <&PIO17 6 ALT1 IN NICLK 0 CLK_A>;
+ phyclk = <&PIO13 5 ALT4 OUT NICLK 0 CLK_B>;
+
+
+ };
+ };
+ };
};
pin-controller-left {
diff --git a/arch/arm/boot/dts/stih415.dtsi b/arch/arm/boot/dts/stih415.dtsi
index d52207c..cc9b22b 100644
--- a/arch/arm/boot/dts/stih415.dtsi
+++ b/arch/arm/boot/dts/stih415.dtsi
@@ -147,5 +147,53 @@
status = "disabled";
};
+
+ ethernet0: dwmac@fe810000 {
+ device_type = "network";
+ compatible = "st,stih415-dwmac", "snps,dwmac", "snps,dwmac-3.610";
+ status = "disabled";
+
+ reg = <0xfe810000 0x8000>, <0x148 0x4>;
+ reg-names = "stmmaceth", "sti-ethconf";
+
+ interrupts = <0 147 0>, <0 148 0>, <0 149 0>;
+ interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
+ resets = <&softreset STIH415_ETH0_SOFTRESET>;
+ reset-names = "stmmaceth";
+
+ snps,pbl = <32>;
+ snps,mixed-burst;
+ snps,force_sf_dma_mode;
+
+ st,syscon = <&syscfg_rear>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_mii0>;
+ clock-names = "stmmaceth";
+ clocks = <&CLKS_GMAC0_PHY>;
+ };
+
+ ethernet1: dwmac@fef08000 {
+ device_type = "network";
+ compatible = "st,stih415-dwmac", "snps,dwmac", "snps,dwmac-3.610";
+ status = "disabled";
+ reg = <0xfef08000 0x8000>, <0x74 0x4>;
+ reg-names = "stmmaceth", "sti-ethconf";
+ interrupts = <0 150 0>, <0 151 0>, <0 152 0>;
+ interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
+
+ snps,pbl = <32>;
+ snps,mixed-burst;
+ snps,force_sf_dma_mode;
+
+ st,syscon = <&syscfg_sbc>;
+
+ resets = <&softreset STIH415_ETH1_SOFTRESET>;
+ reset-names = "stmmaceth";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_mii1>;
+ clock-names = "stmmaceth";
+ clocks = <&CLKS_ETH1_PHY>;
+ };
};
};
diff --git a/arch/arm/boot/dts/stih41x-b2000.dtsi b/arch/arm/boot/dts/stih41x-b2000.dtsi
index 1e6aa92..bf65c49 100644
--- a/arch/arm/boot/dts/stih41x-b2000.dtsi
+++ b/arch/arm/boot/dts/stih41x-b2000.dtsi
@@ -20,6 +20,8 @@
aliases {
ttyAS0 = &serial2;
+ ethernet0 = ðernet0;
+ ethernet1 = ðernet1;
};
soc {
@@ -46,5 +48,25 @@
status = "okay";
};
+
+ ethernet0: dwmac@fe810000 {
+ status = "okay";
+ phy-mode = "mii";
+ pinctrl-0 = <&pinctrl_mii0>;
+
+ snps,reset-gpio = <&PIO106 2>;
+ snps,reset-active-low;
+ snps,reset-delays-us = <0 10000 10000>;
+ };
+
+ ethernet1: dwmac@fef08000 {
+ status = "disabled";
+ phy-mode = "mii";
+ st,tx-retime-src = "txclk";
+
+ snps,reset-gpio = <&PIO4 7>;
+ snps,reset-active-low;
+ snps,reset-delays-us = <0 10000 10000>;
+ };
};
};
diff --git a/arch/arm/boot/dts/stih41x-b2020.dtsi b/arch/arm/boot/dts/stih41x-b2020.dtsi
index 0ef0a69..6c9a2ab 100644
--- a/arch/arm/boot/dts/stih41x-b2020.dtsi
+++ b/arch/arm/boot/dts/stih41x-b2020.dtsi
@@ -19,6 +19,7 @@
aliases {
ttyAS0 = &sbc_serial1;
+ ethernet1 = ðernet1;
};
soc {
sbc_serial1: serial@fe531000 {
@@ -60,5 +61,30 @@
i2c@fe541000 {
status = "okay";
};
+
+ /**
+ * ethernet clk routing:
+ * for
+ * max-speed = <1000>;
+ * set
+ * st,tx-retime-src = "clk_125";
+ *
+ * for
+ * max-speed = <100>;
+ * set
+ * st,tx-retime-src = "clkgen";
+ */
+
+ ethernet1: dwmac@fef08000 {
+ status = "okay";
+ phy-mode = "rgmii-id";
+ max-speed = <1000>;
+ st,tx-retime-src = "clk_125";
+ snps,reset-gpio = <&PIO3 0>;
+ snps,reset-active-low;
+ snps,reset-delays-us = <0 10000 10000>;
+
+ pinctrl-0 = <&pinctrl_rgmii1>;
+ };
};
};
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 1/3] net: stmmac:sti: Add STi SOC glue driver.
From: srinivas.kandagatla @ 2014-02-07 10:55 UTC (permalink / raw)
To: netdev
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Rob Landley, Russell King, Srinivas Kandagatla, Stuart Menefy,
Giuseppe Cavallaro, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, kernel, davem
In-Reply-To: <1391770455-24291-1-git-send-email-srinivas.kandagatla@st.com>
From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
STi series SOCs have a glue layer on top of the synopsis gmac IP, this
glue layer needs to be configured before the gmac driver starts using
the IP.
This patch adds a support to this glue layer which is configured via
stmmac setup, init, exit callbacks.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
.../devicetree/bindings/net/sti-dwmac.txt | 58 ++++
drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 +
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c | 331 ++++++++++++++++++++
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 3 +
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 5 +
6 files changed, 409 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/sti-dwmac.txt
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
diff --git a/Documentation/devicetree/bindings/net/sti-dwmac.txt b/Documentation/devicetree/bindings/net/sti-dwmac.txt
new file mode 100644
index 0000000..3dd3d0b
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/sti-dwmac.txt
@@ -0,0 +1,58 @@
+STMicroelectronics SoC DWMAC glue layer controller
+
+The device node has following properties.
+
+Required properties:
+ - compatible : Can be "st,stih415-dwmac", "st,stih416-dwmac" or
+ "st,stid127-dwmac".
+ - reg : Offset of the glue configuration register map in system
+ configuration regmap pointed by st,syscon property and size.
+
+ - reg-names : Should be "sti-ethconf".
+
+ - st,syscon : Should be phandle to system configuration node which
+ encompases this glue registers.
+
+ - st,tx-retime-src: On STi Parts for Giga bit speeds, 125Mhz clocks can be
+ wired up in from different sources. One via TXCLK pin and other via CLK_125
+ pin. This wiring is totally board dependent. However the retiming glue
+ logic should be configured accordingly. Possible values for this property
+
+ "txclk" - if 125Mhz clock is wired up via txclk line.
+ "clk_125" - if 125Mhz clock is wired up via clk_125 line.
+
+ This property is only valid for Giga bit setup( GMII, RGMII), and it is
+ un-used for non-giga bit (MII and RMII) setups. Also note that internal
+ clockgen can not generate stable 125Mhz clock.
+
+ - st,ext-phyclk: This boolean property indicates who is generating the clock
+ for tx and rx. This property is only valid for RMII case where the clock can
+ be generated from the MAC or PHY.
+
+ - clock-names: should be "sti-ethclk".
+ - clocks: Should point to ethernet clockgen which can generate phyclk.
+
+
+Example:
+
+ethernet0: dwmac@fe810000 {
+ device_type = "network";
+ compatible = "st,stih416-dwmac", "snps,dwmac", "snps,dwmac-3.710";
+ reg = <0xfe810000 0x8000>, <0x8bc 0x4>;
+ reg-names = "stmmaceth", "sti-ethconf";
+ interrupts = <0 133 0>, <0 134 0>, <0 135 0>;
+ interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
+ phy-mode = "mii";
+
+ st,syscon = <&syscfg_rear>;
+
+ snps,pbl = <32>;
+ snps,mixed-burst;
+
+ resets = <&softreset STIH416_ETH0_SOFTRESET>;
+ reset-names = "stmmaceth";
+ pinctrl-0 = <&pinctrl_mii0>;
+ pinctrl-names = "default";
+ clocks = <&CLK_S_GMAC0_PHY>;
+ clock-names = "stmmaceth";
+};
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index e2f202e..f2d7c70 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -37,6 +37,17 @@ config DWMAC_SUNXI
stmmac device driver. This driver is used for A20/A31
GMAC ethernet controller.
+config DWMAC_STI
+ bool "STi GMAC support"
+ depends on STMMAC_PLATFORM && ARCH_STI
+ default y
+ ---help---
+ Support for ethernet controller on STi SOCs.
+
+ This selects STi SoC glue layer support for the stmmac
+ device driver. This driver is used on for the STi series
+ SOCs GMAC ethernet controller.
+
config STMMAC_PCI
bool "STMMAC PCI bus support"
depends on STMMAC_ETH && PCI
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index ecadece..dcef287 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -2,6 +2,7 @@ obj-$(CONFIG_STMMAC_ETH) += stmmac.o
stmmac-$(CONFIG_STMMAC_PLATFORM) += stmmac_platform.o
stmmac-$(CONFIG_STMMAC_PCI) += stmmac_pci.o
stmmac-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o
+stmmac-$(CONFIG_DWMAC_STI) += dwmac-sti.o
stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o \
chain_mode.o dwmac_lib.o dwmac1000_core.o dwmac1000_dma.o \
dwmac100_core.o dwmac100_dma.o enh_desc.o norm_desc.o \
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
new file mode 100644
index 0000000..47113c2
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
@@ -0,0 +1,331 @@
+/**
+ * dwmac-sti.c - STMicroelectronics DWMAC Specific Glue layer
+ *
+ * Copyright (C) 2003-2014 STMicroelectronics (R&D) Limited
+ * Author: Srinivas Kandagatla <srinivas.kandagatla@st.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 <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/stmmac.h>
+#include <linux/phy.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
+
+/**
+ * STi GMAC glue logic.
+ * --------------------
+ *
+ * _
+ * | \
+ * --------|0 \ ETH_SEL_INTERNAL_NOTEXT_PHYCLK
+ * phyclk | |___________________________________________
+ * | | | (phyclk-in)
+ * --------|1 / |
+ * int-clk |_ / |
+ * | _
+ * | | \
+ * |_______|0 \ ETH_SEL_TX_RETIME_CLK
+ * | |___________________________
+ * | | (tx-retime-clk)
+ * _______|1 /
+ * | |_ /
+ * _ |
+ * | \ |
+ * --------|0 \ |
+ * clk_125 | |__|
+ * | | ETH_SEL_TXCLK_NOT_CLK125
+ * --------|1 /
+ * txclk |_ /
+ *
+ *
+ * ETH_SEL_INTERNAL_NOTEXT_PHYCLK is valid only for RMII where PHY can
+ * generate 50MHz clock or MAC can generate it.
+ * This bit is configured by "st,ext-phyclk" property.
+ *
+ * ETH_SEL_TXCLK_NOT_CLK125 is only valid for gigabit modes, where the 125Mhz
+ * clock either comes from clk-125 pin or txclk pin. This configuration is
+ * totally driven by the board wiring. This bit is configured by
+ * "st,tx-retime-src" property.
+ *
+ * TXCLK configuration is different for different phy interface modes
+ * and changes according to link speed in modes like RGMII.
+ *
+ * Below table summarizes the clock requirement and clock sources for
+ * supported phy interface modes with link speeds.
+ * ________________________________________________
+ *| PHY_MODE | 1000 Mbit Link | 100 Mbit Link |
+ * ------------------------------------------------
+ *| MII | n/a | 25Mhz |
+ *| | | txclk |
+ * ------------------------------------------------
+ *| GMII | 125Mhz | 25Mhz |
+ *| | clk-125/txclk | txclk |
+ * ------------------------------------------------
+ *| RGMII | 125Mhz | 25Mhz |
+ *| | clk-125/txclk | clkgen |
+ * ------------------------------------------------
+ *| RMII | n/a | 25Mhz |
+ *| | |clkgen/phyclk-in |
+ * ------------------------------------------------
+ *
+ * TX lines are always retimed with a clk, which can vary depending
+ * on the board configuration. Below is the table of these bits
+ * in eth configuration register depending on source of retime clk.
+ *
+ *---------------------------------------------------------------
+ * src | tx_rt_clk | int_not_ext_phyclk | txclk_n_clk125|
+ *---------------------------------------------------------------
+ * txclk | 0 | n/a | 1 |
+ *---------------------------------------------------------------
+ * ck_125| 0 | n/a | 0 |
+ *---------------------------------------------------------------
+ * phyclk| 1 | 0 | n/a |
+ *---------------------------------------------------------------
+ * clkgen| 1 | 1 | n/a |
+ *---------------------------------------------------------------
+ */
+
+ /* Register definition */
+
+ /* 3 bits [8:6]
+ * [6:6] ETH_SEL_TXCLK_NOT_CLK125
+ * [7:7] ETH_SEL_INTERNAL_NOTEXT_PHYCLK
+ * [8:8] ETH_SEL_TX_RETIME_CLK
+ *
+ */
+
+#define TX_RETIME_SRC_MASK GENMASK(8, 6)
+#define ETH_SEL_TX_RETIME_CLK BIT(8)
+#define ETH_SEL_INTERNAL_NOTEXT_PHYCLK BIT(7)
+#define ETH_SEL_TXCLK_NOT_CLK125 BIT(6)
+
+#define ENMII_MASK GENMASK(5, 5)
+#define ENMII BIT(5)
+
+/**
+ * 3 bits [4:2]
+ * 000-GMII/MII
+ * 001-RGMII
+ * 010-SGMII
+ * 100-RMII
+*/
+#define MII_PHY_SEL_MASK GENMASK(4, 2)
+#define ETH_PHY_SEL_RMII BIT(4)
+#define ETH_PHY_SEL_SGMII BIT(3)
+#define ETH_PHY_SEL_RGMII BIT(2)
+#define ETH_PHY_SEL_GMII 0x0
+#define ETH_PHY_SEL_MII 0x0
+
+#define IS_PHY_IF_MODE_RGMII(iface) (iface == PHY_INTERFACE_MODE_RGMII || \
+ iface == PHY_INTERFACE_MODE_RGMII_ID || \
+ iface == PHY_INTERFACE_MODE_RGMII_RXID || \
+ iface == PHY_INTERFACE_MODE_RGMII_TXID)
+
+#define IS_PHY_IF_MODE_GBIT(iface) (IS_PHY_IF_MODE_RGMII(iface) || \
+ iface == PHY_INTERFACE_MODE_GMII)
+
+struct sti_dwmac {
+ int interface;
+ bool ext_phyclk;
+ bool is_tx_retime_src_clk_125;
+ struct clk *clk;
+ int reg;
+ struct device *dev;
+ struct regmap *regmap;
+};
+
+static u32 phy_intf_sels[] = {
+ [PHY_INTERFACE_MODE_MII] = ETH_PHY_SEL_MII,
+ [PHY_INTERFACE_MODE_GMII] = ETH_PHY_SEL_GMII,
+ [PHY_INTERFACE_MODE_RGMII] = ETH_PHY_SEL_RGMII,
+ [PHY_INTERFACE_MODE_RGMII_ID] = ETH_PHY_SEL_RGMII,
+ [PHY_INTERFACE_MODE_SGMII] = ETH_PHY_SEL_SGMII,
+ [PHY_INTERFACE_MODE_RMII] = ETH_PHY_SEL_RMII,
+};
+
+enum {
+ TX_RETIME_SRC_NA = 0,
+ TX_RETIME_SRC_TXCLK = 1,
+ TX_RETIME_SRC_CLK_125,
+ TX_RETIME_SRC_PHYCLK,
+ TX_RETIME_SRC_CLKGEN,
+};
+
+static const char * const tx_retime_srcs[] = {
+ [TX_RETIME_SRC_NA] = "",
+ [TX_RETIME_SRC_TXCLK] = "txclk",
+ [TX_RETIME_SRC_CLK_125] = "clk_125",
+ [TX_RETIME_SRC_PHYCLK] = "phyclk",
+ [TX_RETIME_SRC_CLKGEN] = "clkgen",
+};
+
+static u32 tx_retime_val[] = {
+ [TX_RETIME_SRC_TXCLK] = ETH_SEL_TXCLK_NOT_CLK125,
+ [TX_RETIME_SRC_CLK_125] = 0x0,
+ [TX_RETIME_SRC_PHYCLK] = ETH_SEL_TX_RETIME_CLK,
+ [TX_RETIME_SRC_CLKGEN] = ETH_SEL_TX_RETIME_CLK |
+ ETH_SEL_INTERNAL_NOTEXT_PHYCLK,
+};
+
+static void setup_retime_src(struct sti_dwmac *dwmac, u32 spd)
+{
+ u32 src = 0, freq = 0;
+
+ if (spd == SPEED_100) {
+ if (dwmac->interface == PHY_INTERFACE_MODE_MII ||
+ dwmac->interface == PHY_INTERFACE_MODE_GMII) {
+ src = TX_RETIME_SRC_TXCLK;
+ } else if (dwmac->interface == PHY_INTERFACE_MODE_RMII) {
+ if (dwmac->ext_phyclk) {
+ src = TX_RETIME_SRC_PHYCLK;
+ } else {
+ src = TX_RETIME_SRC_CLKGEN;
+ freq = 50000000;
+ }
+
+ } else if (IS_PHY_IF_MODE_RGMII(dwmac->interface)) {
+ src = TX_RETIME_SRC_CLKGEN;
+ freq = 25000000;
+ }
+
+ if (src == TX_RETIME_SRC_CLKGEN && dwmac->clk)
+ clk_set_rate(dwmac->clk, freq);
+
+ } else if (spd == SPEED_1000) {
+ if (dwmac->is_tx_retime_src_clk_125)
+ src = TX_RETIME_SRC_CLK_125;
+ else
+ src = TX_RETIME_SRC_TXCLK;
+ }
+
+ regmap_update_bits(dwmac->regmap, dwmac->reg,
+ TX_RETIME_SRC_MASK, tx_retime_val[src]);
+}
+
+static void sti_dwmac_exit(struct platform_device *pdev, void *priv)
+{
+ struct sti_dwmac *dwmac = priv;
+
+ if (dwmac->clk)
+ clk_disable_unprepare(dwmac->clk);
+}
+
+static void sti_fix_mac_speed(void *priv, unsigned int spd)
+{
+ struct sti_dwmac *dwmac = priv;
+ setup_retime_src(dwmac, spd);
+ return;
+}
+
+static int sti_dwmac_parse_data(struct sti_dwmac *dwmac,
+ struct platform_device *pdev)
+{
+ struct resource *res;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ struct regmap *regmap;
+ int err;
+
+ if (!np)
+ return -EINVAL;
+
+ res = platform_get_resource_byname(pdev,
+ IORESOURCE_MEM, "sti-ethconf");
+ if (!res)
+ return -ENODATA;
+
+ regmap = syscon_regmap_lookup_by_phandle(np, "st,syscon");
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ dwmac->dev = dev;
+ dwmac->interface = of_get_phy_mode(np);
+ dwmac->regmap = regmap;
+ dwmac->reg = res->start;
+ dwmac->ext_phyclk = of_property_read_bool(np, "st,ext-phyclk");
+
+ dwmac->is_tx_retime_src_clk_125 = false;
+
+ if (IS_PHY_IF_MODE_GBIT(dwmac->interface)) {
+ const char *rs;
+ err = of_property_read_string(np, "st,tx-retime-src", &rs);
+ if (err < 0) {
+ dev_err(dev, "st,tx-retime-src not specified\n");
+ return err;
+ }
+
+ if (!strcasecmp(rs, "clk_125"))
+ dwmac->is_tx_retime_src_clk_125 = true;
+
+ }
+
+ dwmac->clk = devm_clk_get(dev, "sti-ethclk");
+
+ if (IS_ERR(dwmac->clk))
+ dwmac->clk = NULL;
+
+ return 0;
+}
+
+static int sti_dwmac_init(struct platform_device *pdev, void *priv)
+{
+ struct sti_dwmac *dwmac = priv;
+ struct regmap *regmap = dwmac->regmap;
+ int iface = dwmac->interface;
+ u32 reg = dwmac->reg;
+ u32 val, spd;
+
+ if (dwmac->clk)
+ clk_prepare_enable(dwmac->clk);
+
+ regmap_update_bits(regmap, reg, MII_PHY_SEL_MASK,
+ phy_intf_sels[iface]);
+
+ val = (iface == PHY_INTERFACE_MODE_REVMII) ? 0 : ENMII;
+ regmap_update_bits(regmap, reg, ENMII_MASK, val);
+
+ if (IS_PHY_IF_MODE_GBIT(iface))
+ spd = SPEED_1000;
+ else
+ spd = SPEED_100;
+
+ setup_retime_src(dwmac, spd);
+
+ return 0;
+}
+
+static void *sti_dwmac_setup(struct platform_device *pdev)
+{
+ struct sti_dwmac *dwmac;
+ int ret;
+
+ dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
+ if (!dwmac)
+ return ERR_PTR(-ENOMEM);
+
+ ret = sti_dwmac_parse_data(dwmac, pdev);
+ if (ret) {
+ dev_err(&pdev->dev, "Unable to parse OF data\n");
+ return ERR_PTR(ret);
+ }
+
+ return dwmac;
+}
+
+const struct stmmac_of_data sti_gmac_data = {
+ .fix_mac_speed = sti_fix_mac_speed,
+ .setup = sti_dwmac_setup,
+ .init = sti_dwmac_init,
+ .exit = sti_dwmac_exit,
+};
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index d9af26e..f9e60d7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -133,6 +133,9 @@ bool stmmac_eee_init(struct stmmac_priv *priv);
#ifdef CONFIG_DWMAC_SUNXI
extern const struct stmmac_of_data sun7i_gmac_data;
#endif
+#ifdef CONFIG_DWMAC_STI
+extern const struct stmmac_of_data sti_gmac_data;
+#endif
extern struct platform_driver stmmac_pltfr_driver;
static inline int stmmac_register_platform(void)
{
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 5884a7d..c61bc72b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -33,6 +33,11 @@ static const struct of_device_id stmmac_dt_ids[] = {
#ifdef CONFIG_DWMAC_SUNXI
{ .compatible = "allwinner,sun7i-a20-gmac", .data = &sun7i_gmac_data},
#endif
+#ifdef CONFIG_DWMAC_STI
+ { .compatible = "st,stih415-dwmac", .data = &sti_gmac_data},
+ { .compatible = "st,stih416-dwmac", .data = &sti_gmac_data},
+ { .compatible = "st,stih127-dwmac", .data = &sti_gmac_data},
+#endif
/* SoC specific glue layers should come before generic bindings */
{ .compatible = "st,spear600-gmac"},
{ .compatible = "snps,dwmac-3.610"},
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 0/3] net: stmmac: Add STi GMAC ethernet
From: srinivas.kandagatla @ 2014-02-07 10:54 UTC (permalink / raw)
To: netdev
Cc: Mark Rutland, devicetree, Russell King, kernel, Pawel Moll,
Ian Campbell, Srinivas Kandagatla, linux-doc, linux-kernel,
Stuart Menefy, Rob Herring, Rob Landley, Kumar Gala,
Giuseppe Cavallaro, davem, linux-arm-kernel
In-Reply-To: <1391428787-27143-1-git-send-email-srinivas.kandagatla@st.com>
From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
Hi All,
This patch series adds Ethernet support to STi series SOCs STiH415 and STiH416.
STi SOC series integrates dwmac IP from synopsis, however there is a hardware
glue on top of this standard IP, this glue needs to configured before the
actual dwmac can be used. Also the glue logic needs re-configuring when the
link speed changes, This is because the clk source can change as the link
speed changes.
This patch just adds STi specific callbacks into of_data for configuring the
glue layer.
I have rebased my original patches (http://lkml.org/lkml/2013/11/12/243)
to latest stmmac which updates callbacks to suit glue drivers like this.
These patches are tested on b2000 and B2020 with STiH415 and STiH416.
Changes since v1:
- fixed multi-line function call format as suggested by David Miller.
Dave, Can I request you to take the first patch via net tree for v3.15, I can
request Arnd or Olof to take the DT patches via the arm-soc tree for v3.15.
Thanks,
srini
Srinivas Kandagatla (3):
net: stmmac:sti: Add STi SOC glue driver.
ARM: STi: Add STiH415 ethernet support.
ARM: STi: Add STiH416 ethernet support.
.../devicetree/bindings/net/sti-dwmac.txt | 58 ++++
arch/arm/boot/dts/stih415-clock.dtsi | 14 +
arch/arm/boot/dts/stih415-pinctrl.dtsi | 121 +++++++
arch/arm/boot/dts/stih415.dtsi | 48 +++
arch/arm/boot/dts/stih416-clock.dtsi | 14 +
arch/arm/boot/dts/stih416-pinctrl.dtsi | 109 +++++++
arch/arm/boot/dts/stih416.dtsi | 44 +++
arch/arm/boot/dts/stih41x-b2000.dtsi | 22 ++
arch/arm/boot/dts/stih41x-b2020.dtsi | 26 ++
drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 +
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c | 331 ++++++++++++++++++++
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 3 +
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 5 +
14 files changed, 807 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/sti-dwmac.txt
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c
--
1.7.9.5
^ permalink raw reply
* Re: [RFC PATCH 08/15] ata: ahci_platform: Manage SATA PHY
From: Roger Quadros @ 2014-02-07 10:44 UTC (permalink / raw)
To: Arnd Bergmann
Cc: devicetree, balajitk, Sergei Shtylyov, Hans de Goede, tony,
linux-kernel, balbi, kishon, linux-ide, bcousson, Tejun Heo,
linux-omap, linux-arm-kernel
In-Reply-To: <463926864.ALblgUKdgH@wuerfel>
On 02/07/2014 12:39 PM, Arnd Bergmann wrote:
> On Friday 07 February 2014 12:33:38 Roger Quadros wrote:
>>
>> This means we need to make CONFIG_SATA_AHCI_PLATFORM depend on CONFIG_GENERIC_PHY or
>> select it.
>>
>> OR
>>
>> Generic PHY layer must be fixed so that the API's are always built in.
>>
>> What is the better option? I believe making the PHY API's always built in is the better option.
>>
>
> CONFIG_SATA_AHCI_PLATFORM should do
>
> "depends on CONFIG_GENERIC_PHY || !CONFIG_GENERIC_PHY"
>
> which is the Kconfig way of saying that if CONFIG_GENERIC_PHY is a module,
> CONFIG_SATA_AHCI_PLATFORM needs to be a module as well.
>
Ah, that's neat. Thanks :).
cheers,
-roger
^ permalink raw reply
* Re: [PATCH v6 05/19] watchdog: orion: Make sure the watchdog is initially stopped
From: Ezequiel Garcia @ 2014-02-07 10:40 UTC (permalink / raw)
To: Guenter Roeck, Jason Gunthorpe
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-watchdog-u79uwXL29TY76Z2rM5mHXA, Wim Van Sebroeck,
Jason Cooper, Thomas Petazzoni, Gregory Clement, Lior Amsalem,
Sebastian Hesselbarth, Andrew Lunn
In-Reply-To: <52F43ED0.4050808-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
On Thu, Feb 06, 2014 at 06:02:56PM -0800, Guenter Roeck wrote:
> On 02/06/2014 09:20 AM, Ezequiel Garcia wrote:
> > Having the watchdog initially fully stopped is important to avoid
> > any spurious watchdog triggers, in case the registers are not in
> > its reset state.
> >
> > Reviewed-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> > Tested-by: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Tested-by: Willy Tarreau <w@1wt.eu>
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > ---
> > drivers/watchdog/orion_wdt.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> > index 6746033..2dbeee9 100644
> > --- a/drivers/watchdog/orion_wdt.c
> > +++ b/drivers/watchdog/orion_wdt.c
> > @@ -142,6 +142,9 @@ static int orion_wdt_probe(struct platform_device *pdev)
> > orion_wdt.max_timeout = wdt_max_duration;
> > watchdog_init_timeout(&orion_wdt, heartbeat, &pdev->dev);
> >
> > + /* Let's make sure the watchdog is fully stopped */
> > + orion_wdt_stop(&orion_wdt);
> > +
>
> Actually we just had that in another driver, and I stumbled over it there.
>
> Problem with stopping the watchdog in probe unconditionally is that you can
> use it to defeat nowayout: unload the module, then load it again,
> and the watchdog is stopped even if nowayout is true.
>
Hm... I see.
> Is this really what you want ? Or, in other words, what is the problem
> you are trying to solve ?
>
Well, this is related to the discussion about the bootloader not
reseting the watchdog properly, provoking spurious watchdog triggering.
Jason Gunthorpe explained [1] that we needed a particular sequence:
1. Disable WDT
2. Clear bridge
3. Enable WDT
We added the irq handling to satisfy (2), and the watchdog stop for (1).
The watchdog stop was agreed specifically [2].
Ideas?
[1] http://www.spinics.net/lists/arm-kernel/msg302340.html
[2] http://www.spinics.net/lists/arm-kernel/msg302507.html
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH 08/15] ata: ahci_platform: Manage SATA PHY
From: Arnd Bergmann @ 2014-02-07 10:39 UTC (permalink / raw)
To: Roger Quadros
Cc: devicetree, balajitk, Sergei Shtylyov, Hans de Goede, tony,
linux-kernel, balbi, kishon, linux-ide, bcousson, Tejun Heo,
linux-omap, linux-arm-kernel
In-Reply-To: <52F4B682.2000201@ti.com>
On Friday 07 February 2014 12:33:38 Roger Quadros wrote:
>
> This means we need to make CONFIG_SATA_AHCI_PLATFORM depend on CONFIG_GENERIC_PHY or
> select it.
>
> OR
>
> Generic PHY layer must be fixed so that the API's are always built in.
>
> What is the better option? I believe making the PHY API's always built in is the better option.
>
CONFIG_SATA_AHCI_PLATFORM should do
"depends on CONFIG_GENERIC_PHY || !CONFIG_GENERIC_PHY"
which is the Kconfig way of saying that if CONFIG_GENERIC_PHY is a module,
CONFIG_SATA_AHCI_PLATFORM needs to be a module as well.
Arnd
^ permalink raw reply
* Re: [RFC PATCH 08/15] ata: ahci_platform: Manage SATA PHY
From: Roger Quadros @ 2014-02-07 10:33 UTC (permalink / raw)
To: Sergei Shtylyov, kishon
Cc: Tejun Heo, balbi, bcousson, tony, balajitk, linux-omap,
linux-arm-kernel, linux-ide, linux-kernel, devicetree,
Hans de Goede
In-Reply-To: <5240492D.6060204@ti.com>
Hi,
On 09/23/2013 04:59 PM, Roger Quadros wrote:
> On 09/23/2013 03:59 PM, Sergei Shtylyov wrote:
>> Hello.
>>
>> On 23-09-2013 11:37, Roger Quadros wrote:
>>
>>>>> Not sure why you asked -- I'm not using this driver, neither I'm
>>
>>>> Well, you have better grip of what's going on in the embedded world
>>>> than me. I'm mostly curious whether adding dependency on PHY is okay.
>>
>>> There is no hard dependency on presence of PHY. The driver will continue
>>> as usual if devm_phy_get() fails.
>>> I hope selecting GENERIC_PHY in Kconfig is not an issue.
>>
>> Selecting in the AHCI_PLATFORM section? It seems I have overlooked it. No, I don't think it's a good idea. The generic PHY functions seem to be stubbed when GENERIC_PHY=n.
>>
> OK I will remove the select then.
If I remove the select then build fails like so if we set CONFIG_GENERIC_PHY to 'm' and CONFIG_SATA_AHCI_PLATFORM to 'y'
drivers/built-in.o: In function `ahci_platform_enable_resources':
(.text+0x162647): undefined reference to `phy_init'
drivers/built-in.o: In function `ahci_platform_enable_resources':
(.text+0x16267c): undefined reference to `phy_power_on'
drivers/built-in.o: In function `ahci_platform_enable_resources':
(.text+0x162694): undefined reference to `phy_exit'
drivers/built-in.o: In function `ahci_platform_disable_resources':
(.text+0x1626af): undefined reference to `phy_power_off'
drivers/built-in.o: In function `ahci_platform_disable_resources':
(.text+0x1626b7): undefined reference to `phy_exit'
drivers/built-in.o: In function `ahci_platform_get_resources':
(.text+0x162768): undefined reference to `devm_phy_get'
make: *** [vmlinux] Error 1
This means we need to make CONFIG_SATA_AHCI_PLATFORM depend on CONFIG_GENERIC_PHY or
select it.
OR
Generic PHY layer must be fixed so that the API's are always built in.
What is the better option? I believe making the PHY API's always built in is the better option.
cheers,
-roger
^ permalink raw reply
* Re: [PATCH 1/2 v3] i2c: exynos5: add support for HSI2C on Exynos5260 SoC
From: Tomasz Figa @ 2014-02-07 10:21 UTC (permalink / raw)
To: Wolfram Sang, Tomasz Figa
Cc: Naveen Krishna Chatradhi,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
naveenkrishna.ch-Re5JQEeQqe8AvxtiuMwx3w,
kgene.kim-Sze3O3UU22JBDgjK7y7TUQ,
grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
taeggyun.ko-Sze3O3UU22JBDgjK7y7TUQ, balbi-l0cyMroinI0,
cpgs-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <20140207101747.GA2621@katana>
On 07.02.2014 11:17, Wolfram Sang wrote:
> On Thu, Feb 06, 2014 at 02:50:51PM +0100, Tomasz Figa wrote:
>> Also, please use correct addresses of DT ML and Wolfram's e-mail
>> (fixed in this message).
>
> And please don't use In-Reply-To when sending new versions of patches.
> The message threading became hard to read here...
>
+1
It's hard to follow new versions of series that show up under the thread
of old version. If you want to keep reference to old versions, you can
add links to respective threads to change log entries in cover letter.
Best regards,
Tomasz
^ permalink raw reply
* Re: [PATCH v3 0/4] ARM: add omap3 INCOstartec board support
From: Christoph Fritz @ 2014-02-07 10:19 UTC (permalink / raw)
To: Nishanth Menon
Cc: Tero Kristo, Javier Martinez Canillas, Ulf Hansson, Mark Rutland,
Ian Campbell, Tony Lindgren, Tomi Valkeinen,
bcousson-rdvid1DuHRBWk0Htik3J/w, Daniel Mack, Hans J. Koch,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-omap-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1390782797-24105-1-git-send-email-chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
On Mon, 2014-01-27 at 01:33 +0100, Christoph Fritz wrote:
> This set of patches adds board support for an omap3 system from INCOstartec.
> It's based on next-20140124.
>
> Due to a regression since next-20140115 the following errors are present:
>
> - pin sys_clkout2, which gets configured to 24 Mhz by the fourth patch
> in this set, erroneously outputs only 12 Mhz.
> Just out of curiosity, configuring it to 48 Mhz puts out desired 24 Mhz.
>
> - omap_dss, which gets configured by the third patch in this set, fails
> to do 'dss_set_fck_rate(fck);' in
> drivers/video/omap2/dss/dss.c:dss_setup_default_clock() which leads to:
>
> | omapdss_dss: probe of omapdss_dss failed with error -22
> | omapdss CORE error: Failed to initialize DSS platform driver
> | panel-dpi panel-dpi.0: failed to find video source 'dpi.0
>
> Both regressions seem to have something to do with the clock framework.
> Could this be related to the DT clock conversion patches?
Now with dt clock conversion fixes on its way, any chance to get this
upstream?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2 v3] i2c: exynos5: add support for HSI2C on Exynos5260 SoC
From: Wolfram Sang @ 2014-02-07 10:17 UTC (permalink / raw)
To: Tomasz Figa
Cc: Naveen Krishna Chatradhi,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
naveenkrishna.ch-Re5JQEeQqe8AvxtiuMwx3w,
kgene.kim-Sze3O3UU22JBDgjK7y7TUQ,
grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
taeggyun.ko-Sze3O3UU22JBDgjK7y7TUQ, balbi-l0cyMroinI0,
cpgs-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <52F3933B.9090202-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 278 bytes --]
On Thu, Feb 06, 2014 at 02:50:51PM +0100, Tomasz Figa wrote:
> Also, please use correct addresses of DT ML and Wolfram's e-mail
> (fixed in this message).
And please don't use In-Reply-To when sending new versions of patches.
The message threading became hard to read here...
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH v2 15/15] mfd: max14577: Add device tree bindings document
From: Krzysztof Kozlowski @ 2014-02-07 10:04 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Samuel Ortiz, Lee Jones, Mark Brown,
linux-kernel, linux-arm-kernel
Cc: Marek Szyprowski, Bartlomiej Zolnierkiewicz, Kyungmin Park,
Krzysztof Kozlowski, Tomasz Figa, devicetree, Rob Herring,
Pawel Moll, Mark Rutland
In-Reply-To: <1391767487-10017-1-git-send-email-k.kozlowski@samsung.com>
Add document describing device tree bindings for MAX14577 MFD
drivers: MFD core, extcon, regulator and charger.
Both MAX14577 and MAX77836 chipsets are documented.
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Tomasz Figa <t.figa@samsung.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: devicetree@vger.kernel.org
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
Documentation/devicetree/bindings/mfd/max14577.txt | 149 ++++++++++++++++++++
1 file changed, 149 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/max14577.txt
diff --git a/Documentation/devicetree/bindings/mfd/max14577.txt b/Documentation/devicetree/bindings/mfd/max14577.txt
new file mode 100644
index 000000000000..111115b0a6f3
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/max14577.txt
@@ -0,0 +1,149 @@
+Maxim MAX14577/77836 Multi-Function Device
+
+MAX14577 is a Multi-Function Device with Micro-USB Interface Circuit, Li+
+Battery Charger and SFOUT LDO output for powering USB devices. It is
+interfaced to host controller using I2C.
+
+MAX77836 additionally contains PMIC (with two LDO regulators) and Fuel Gauge.
+
+
+Required properties:
+- compatible : Must be "maxim,max14577" or "maxim,max77836".
+- reg : I2C slave address for the max14577 chip (0x25 for max14577/max77836)
+- interrupts : IRQ line for the chip.
+- interrupt-parent : The parent interrupt controller.
+
+
+Optional nodes:
+- max14577-muic/max77836-muic :
+ Node used only by extcon consumers.
+ Required properties:
+ - compatible : "maxim,max14577-muic" or "maxim,max77836-muic"
+
+- regulators :
+ Required properties:
+ - compatible : "maxim,max14577-regulator"
+ or "maxim,max77836-regulator"
+
+ May contain a sub-node per regulator from the list below. Each
+ sub-node should contain the constraints and initialization information
+ for that regulator. See regulator.txt for a description of standard
+ properties for these sub-nodes.
+
+ List of valid regulator names:
+ - for max14577: CHARGER, SAFEOUT.
+ - for max77836: CHARGER, SAFEOUT, LDO1, LDO2.
+
+ The SAFEOUT is a fixed voltage regulator so there is no need to specify
+ voltages for it.
+
+ - charger :
+ Required properties:
+ - compatible : "maxim,max14577-charger"
+ or "maxim,max77836-charger"
+ - maxim,fast-charge-timer : Timer in hours to trigger the
+ INT3/MBCCHGERR interrupt; Valid values:
+ - 5, 6 or 7 (hours),
+ - 0 to disable.
+ - maxim,fast-charge-uamp : Current in uA for Fast Charge;
+ Valid values:
+ - for max14577: 90000 - 950000;
+ - for max77836: 45000 - 475000;
+ - maxim,eoc-uamp : Current in uA for End-Of-Charge mode;
+ Valid values:
+ - for max14577: 50000 - 200000;
+ - for max77836: 5000 - 100000;
+ - maxim,ovp-uvolt : OverVoltage Protection Threshold in uV;
+ In an overvoltage condition, INT asserts and charging
+ stops. Valid values:
+ - 6000000, 6500000, 7000000, 7500000;
+ - maxim,constant-uvolt : Battery Constant Voltage in uV;
+ Valid values:
+ - 4000000 - 4280000 (step by 20000);
+ - 4350000;
+
+
+Example:
+
+#include <dt-bindings/interrupt-controller/irq.h>
+
+max14577@25 {
+ compatible = "maxim,max14577";
+ reg = <0x25>;
+ interrupt-parent = <&gpx1>;
+ interrupts = <5 IRQ_TYPE_NONE>;
+
+ muic: max14577-muic {
+ compatible = "maxim,max14577-muic";
+ };
+
+ regulators {
+ compatible = "maxim,max14577-regulator";
+
+ SAFEOUT {
+ regulator-name = "SAFEOUT";
+ };
+ CHARGER {
+ regulator-name = "CHARGER";
+ regulator-min-microamp = <90000>;
+ regulator-max-microamp = <950000>;
+ regulator-boot-on;
+ };
+ };
+
+ charger {
+ compatible = "maxim,max14577-charger";
+
+ maxim,fast-charge-timer = <6>;
+ maxim,constant-uvolt = <4350000>;
+ maxim,fast-charge-uamp = <450000>;
+ maxim,eoc-uamp = <50000>;
+ maxim,ovp-uvolt = <6500000>;
+ };
+};
+
+
+max77836@25 {
+ compatible = "maxim,max77836";
+ reg = <0x25>;
+ interrupt-parent = <&gpx1>;
+ interrupts = <5 IRQ_TYPE_NONE>;
+
+ muic: max77836-muic {
+ compatible = "maxim,max77836-muic";
+ };
+
+ regulators {
+ compatible = "maxim,max77836-regulator";
+
+ SAFEOUT {
+ regulator-name = "SAFEOUT";
+ };
+ CHARGER {
+ regulator-name = "CHARGER";
+ regulator-min-microamp = <90000>;
+ regulator-max-microamp = <950000>;
+ regulator-boot-on;
+ };
+ LDO1 {
+ regulator-name = "LDO1";
+ regulator-min-microvolt = <2700000>;
+ regulator-max-microvolt = <2700000>;
+ };
+ LDO2 {
+ regulator-name = "LDO2";
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <3950000>;
+ };
+ };
+
+ charger {
+ compatible = "maxim,max77836-charger";
+
+ maxim,fast-charge-timer = <6>;
+ maxim,constant-uvolt = <4350000>;
+ maxim,fast-charge-uamp = <225000>;
+ maxim,eoc-uamp = <7500>;
+ maxim,ovp-uvolt = <6500000>;
+ };
+};
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 2/2] spi: Add Qualcomm QUP SPI controller support
From: Ivan T. Ivanov @ 2014-02-07 9:52 UTC (permalink / raw)
To: Andy Gross
Cc: Mark Brown, Grant Likely, Rob Herring,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Alok Chauhan, Gilad Avidov,
Kiran Gunda, Sagar Dharia
In-Reply-To: <20140207073952.GA2610-zC7DfRvBq/JWk0Htik3J/w@public.gmane.org>
Hi Andy,
On Fri, 2014-02-07 at 01:39 -0600, Andy Gross wrote:
> On Thu, Feb 06, 2014 at 06:57:48PM +0200, Ivan T. Ivanov wrote:
> > From: "Ivan T. Ivanov" <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
> >
> > Qualcomm Universal Peripheral (QUP) core is an AHB slave that
> > provides a common data path (an output FIFO and an input FIFO)
> > for serial peripheral interface (SPI) mini-core. SPI in master mode
> > support up to 50MHz, up to four chip selects, and a programmable
> > data path from 4 bits to 32 bits; MODE0..3 protocols
> >
> > Signed-off-by: Ivan T. Ivanov <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
> > Cc: Alok Chauhan <alokc-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> > Cc: Gilad Avidov <gavidov-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> > Cc: Kiran Gunda <kgunda-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> > Cc: Sagar Dharia <sdharia-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> > ---
> > drivers/spi/Kconfig | 14 +
> > drivers/spi/Makefile | 1 +
> > drivers/spi/spi-qup.c | 898 +++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 913 insertions(+)
> > create mode 100644 drivers/spi/spi-qup.c
> >
> > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> > index ba9310b..bf8ce6b 100644
> > --- a/drivers/spi/Kconfig
> > +++ b/drivers/spi/Kconfig
> > @@ -381,6 +381,20 @@ config SPI_RSPI
> > help
> > SPI driver for Renesas RSPI blocks.
> >
> > +config SPI_QUP
> > + tristate "Qualcomm SPI Support with QUP interface"
> > + depends on ARCH_MSM
>
> I'd change to ARCH_MSM_DT. This ensures the OF component is there.
Ok. will change.
>
> > + help
> > + Qualcomm Universal Peripheral (QUP) core is an AHB slave that
> > + provides a common data path (an output FIFO and an input FIFO)
> > + for serial peripheral interface (SPI) mini-core. SPI in master
> > + mode support up to 50MHz, up to four chip selects, and a
> > + programmable data path from 4 bits to 32 bits; supports numerous
> > + protocol variants.
> > +
> > + This driver can also be built as a module. If so, the module
> > + will be called spi_qup.
> > +
> > config SPI_S3C24XX
> > tristate "Samsung S3C24XX series SPI"
> > depends on ARCH_S3C24XX
> > diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> > index 95af48d..e598147 100644
> > --- a/drivers/spi/Makefile
> > +++ b/drivers/spi/Makefile
> > @@ -59,6 +59,7 @@ spi-pxa2xx-platform-$(CONFIG_SPI_PXA2XX_PXADMA) += spi-pxa2xx-pxadma.o
> > spi-pxa2xx-platform-$(CONFIG_SPI_PXA2XX_DMA) += spi-pxa2xx-dma.o
> > obj-$(CONFIG_SPI_PXA2XX) += spi-pxa2xx-platform.o
> > obj-$(CONFIG_SPI_PXA2XX_PCI) += spi-pxa2xx-pci.o
> > +obj-$(CONFIG_SPI_QUP) += spi-qup.o
> > obj-$(CONFIG_SPI_RSPI) += spi-rspi.o
> > obj-$(CONFIG_SPI_S3C24XX) += spi-s3c24xx-hw.o
> > spi-s3c24xx-hw-y := spi-s3c24xx.o
> > diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
> > new file mode 100644
> > index 0000000..5eb5e8f
> > --- /dev/null
> > +++ b/drivers/spi/spi-qup.c
> > @@ -0,0 +1,898 @@
> > +/*
> > + * Copyright (c) 2008-2014, The Linux foundation. All rights reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License rev 2 and
> > + * only rev 2 as published by the free Software foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or fITNESS fOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/list.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
>
> Remove this for now. No runtime support.
Did you see any particular issue with the implementation
or this is just because this platform didn't have support
for power management?
>
> > +#include <linux/spi/spi.h>
> > +
<snip>
> > +
> > +static int spi_qup_transfer_do(struct spi_qup *controller,
> > + struct spi_qup_device *chip,
> > + struct spi_transfer *xfer)
> > +{
> > + unsigned long timeout;
> > + int ret = -EIO;
> > +
> > + reinit_completion(&controller->done);
> > +
> > + timeout = DIV_ROUND_UP(controller->speed_hz, MSEC_PER_SEC);
> > + timeout = DIV_ROUND_UP(xfer->len * 8, timeout);
> > + timeout = 100 * msecs_to_jiffies(timeout);
> > +
> > + controller->rx_bytes = 0;
> > + controller->tx_bytes = 0;
> > + controller->error = 0;
> > + controller->xfer = xfer;
> > +
> > + if (spi_qup_set_state(controller, QUP_STATE_RUN)) {
> > + dev_warn(controller->dev, "cannot set RUN state\n");
> > + goto exit;
> > + }
> > +
> > + if (spi_qup_set_state(controller, QUP_STATE_PAUSE)) {
> > + dev_warn(controller->dev, "cannot set PAUSE state\n");
> > + goto exit;
> > + }
> > +
> > + spi_qup_fifo_write(controller, xfer);
> > +
> > + if (spi_qup_set_state(controller, QUP_STATE_RUN)) {
> > + dev_warn(controller->dev, "cannot set EXECUTE state\n");
> > + goto exit;
> > + }
> > +
> > + if (!wait_for_completion_timeout(&controller->done, timeout))
> > + ret = -ETIMEDOUT;
> > + else
> > + ret = controller->error;
> > +exit:
> > + controller->xfer = NULL;
>
> Should the manipulation of controller->xfer be protected by spinlock?
:-). Probably. I am wondering, could I avoid locking if firstly place
QUP into RESET state and then access these field. This should stop
all activities in it, right?
>
> > + controller->error = 0;
> > + controller->rx_bytes = 0;
> > + controller->tx_bytes = 0;
> > + spi_qup_set_state(controller, QUP_STATE_RESET);
> > + return ret;
> > +}
> > +
<snip>
> > +
> > +/* set clock freq, clock ramp, bits per work */
> > +static int spi_qup_io_setup(struct spi_device *spi,
> > + struct spi_transfer *xfer)
> > +{
<snip>
> > +
> > + /*
> > + * TODO: In BAM mode mask INPUT and OUTPUT service flags in
> > + * to prevent IRQs on FIFO status change.
> > + */
>
> Remove the TODO. Not necessary. This stuff can be added when it becomes BAM
> enabled.
Ok.
>
> > + writel_relaxed(0, controller->base + QUP_OPERATIONAL_MASK);
> > +
> > + return 0;
> > +}
> > +
> > +static int spi_qup_transfer_one(struct spi_master *master,
> > + struct spi_message *msg)
> > +{
> > + struct spi_qup *controller = spi_master_get_devdata(master);
> > + struct spi_qup_device *chip = spi_get_ctldata(msg->spi);
> > + struct spi_transfer *xfer;
> > + struct spi_device *spi;
> > + unsigned cs_change;
> > + int status;
> > +
> > + spi = msg->spi;
> > + cs_change = 1;
> > + status = 0;
> > +
> > + list_for_each_entry(xfer, &msg->transfers, transfer_list) {
> > +
> > + status = spi_qup_io_setup(spi, xfer);
> > + if (status)
> > + break;
> > +
>
> no locking? This whole code block needs to have some type of mutex_lock to keep
> others from trouncing the hardware while you are doing this transfer.
This is handled by SPI framework.
>
> > + if (cs_change)
> > + spi_qup_assert_cs(controller, chip);
>
> Should the CS be done outside the loop? I'd expect the following sequence to
> happen:
> - change CS
> - Loop and do some transfers
> - deassert CS
>
> In this code, you reinitialize and assert/deassert CS for every transaction.
>
> > +
> > + cs_change = xfer->cs_change;
Not exactly. It is allowed that CS goes inactive after every
transaction. This is how I read struct spi_transfer description.
> > +
> > + /* Do actual transfer */
> > + status = spi_qup_transfer_do(controller, chip, xfer);
> > + if (status)
> > + break;
> > +
> > + msg->actual_length += xfer->len;
> > +
> > + if (xfer->delay_usecs)
> > + udelay(xfer->delay_usecs);
> > +
> > + if (cs_change)
> > + spi_qup_deassert_cs(controller, chip);
> > + }
> > +
> > + if (status || !cs_change)
> > + spi_qup_deassert_cs(controller, chip);
> > +
> > + msg->status = status;
> > + spi_finalize_current_message(master);
> > + return status;
> > +}
> > +
> > +static int spi_qup_probe(struct platform_device *pdev)
> > +{
> > + struct spi_master *master;
> > + struct clk *iclk, *cclk;
> > + struct spi_qup *controller;
> > + struct resource *res;
> > + struct device *dev;
> > + void __iomem *base;
> > + u32 data, max_freq, iomode;
> > + int ret, irq, size;
> > +
> > + dev = &pdev->dev;
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + base = devm_ioremap_resource(dev, res);
> > + if (IS_ERR(base))
> > + return PTR_ERR(base);
> > +
> > + irq = platform_get_irq(pdev, 0);
> > +
> > + if (irq < 0)
> > + return irq;
> > +
> > + cclk = devm_clk_get(dev, "core");
> > + if (IS_ERR(cclk)) {
> > + dev_err(dev, "cannot get core clock\n");
> No need to error print. devm_clk_get already outputs something
Ok.
> > + return PTR_ERR(cclk);
> > + }
> > +
> > + iclk = devm_clk_get(dev, "iface");
> > + if (IS_ERR(iclk)) {
> > + dev_err(dev, "cannot get iface clock\n");
>
> No need to error print. devm_clk_get already outputs something
Ok.
>
> > + return PTR_ERR(iclk);
> > + }
> > +
> > + if (of_property_read_u32(dev->of_node, "spi-max-frequency", &max_freq))
> > + max_freq = 19200000;
>
> I'd set the default to 50MHz as that is the max supported by hardware. I'd just
> set max_freq declaration to 50MHz and then check the value if it is changed via
> DT.
50MHz doesn't seems to be supported on all chip sets. Currently common
denominator on all chip sets, that I can see, is 19.2MHz. I have tried
to test it with more than 19.2MHz on APQ8074 and it fails.
>
> > +
> > + if (!max_freq) {
> > + dev_err(dev, "invalid clock frequency %d\n", max_freq);
> > + return -ENXIO;
> > + }
>
> This is buggy. Remove this and collapse into the of_property_read_u32 if
> statement. On non-zero, check the range for validity.
True. Will fix.
>
> > +
> > + ret = clk_set_rate(cclk, max_freq);
> > + if (ret)
> > + dev_warn(dev, "fail to set SPI frequency %d\n", max_freq);
>
> Bail here?
I don't know. What will be the consequences if controller continue to
operate on its default rate?
>
> > +
> > + ret = clk_prepare_enable(cclk);
> > + if (ret) {
> > + dev_err(dev, "cannot enable core clock\n");
> > + return ret;
> > + }
> > +
> > + ret = clk_prepare_enable(iclk);
> > + if (ret) {
> > + clk_disable_unprepare(cclk);
> > + dev_err(dev, "cannot enable iface clock\n");
> > + return ret;
> > + }
> > +
> > + data = readl_relaxed(base + QUP_HW_VERSION);
> > +
> > + if (data < QUP_HW_VERSION_2_1_1) {
> > + clk_disable_unprepare(cclk);
> > + clk_disable_unprepare(iclk);
> > + dev_err(dev, "v.%08x is not supported\n", data);
> > + return -ENXIO;
> > + }
> > +
> > + master = spi_alloc_master(dev, sizeof(struct spi_qup));
> > + if (!master) {
> > + clk_disable_unprepare(cclk);
> > + clk_disable_unprepare(iclk);
> > + dev_err(dev, "cannot allocate master\n");
> > + return -ENOMEM;
> > + }
> > +
> > + master->bus_num = pdev->id;
> > + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LOOP;
> > + master->num_chipselect = SPI_NUM_CHIPSELECTS;
> > + master->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 32);
> > + master->setup = spi_qup_setup;
> > + master->cleanup = spi_qup_cleanup;
> > + master->transfer_one_message = spi_qup_transfer_one;
> > + master->dev.of_node = pdev->dev.of_node;
> > + master->auto_runtime_pm = true;
>
> Remove this. No runtime support
>
> > +
> > + platform_set_drvdata(pdev, master);
> > +
> > + controller = spi_master_get_devdata(master);
> > +
> > + controller->dev = dev;
> > + controller->base = base;
> > + controller->iclk = iclk;
> > + controller->cclk = cclk;
> > + controller->irq = irq;
> > + controller->max_speed_hz = clk_get_rate(cclk);
> > + controller->speed_hz = controller->max_speed_hz;
> > +
> > + init_completion(&controller->done);
> > +
> > + iomode = readl_relaxed(base + QUP_IO_M_MODES);
> > +
> > + size = QUP_IO_M_OUTPUT_BLOCK_SIZE(iomode);
> > + if (size)
> > + controller->out_blk_sz = size * 16;
> > + else
> > + controller->out_blk_sz = 4;
> > +
> > + size = QUP_IO_M_INPUT_BLOCK_SIZE(iomode);
> > + if (size)
> > + controller->in_blk_sz = size * 16;
> > + else
> > + controller->in_blk_sz = 4;
> > +
> > + size = QUP_IO_M_OUTPUT_FIFO_SIZE(iomode);
> > + controller->out_fifo_sz = controller->out_blk_sz * (2 << size);
> > +
> > + size = QUP_IO_M_INPUT_FIFO_SIZE(iomode);
> > + controller->in_fifo_sz = controller->in_blk_sz * (2 << size);
> > +
> > + dev_info(dev, "v.%08x IN:block:%d, fifo:%d, OUT:block:%d, fifo:%d\n",
> > + data, controller->in_blk_sz, controller->in_fifo_sz,
> > + controller->out_blk_sz, controller->out_fifo_sz);
> > +
> > + writel_relaxed(1, base + QUP_SW_RESET);
> > +
> > + ret = spi_qup_set_state(controller, QUP_STATE_RESET);
> > + if (ret) {
> > + dev_err(dev, "cannot set RESET state\n");
> > + goto error;
> > + }
> > +
> > + writel_relaxed(0, base + QUP_OPERATIONAL);
> > + writel_relaxed(0, base + QUP_IO_M_MODES);
> > + writel_relaxed(0, base + QUP_OPERATIONAL_MASK);
> > + writel_relaxed(SPI_ERROR_CLK_UNDER_RUN | SPI_ERROR_CLK_OVER_RUN,
> > + base + SPI_ERROR_FLAGS_EN);
> > +
> > + writel_relaxed(0, base + SPI_CONFIG);
> > + writel_relaxed(SPI_IO_C_NO_TRI_STATE, base + SPI_IO_CONTROL);
> > +
> > + ret = devm_request_irq(dev, irq, spi_qup_qup_irq,
> > + IRQF_TRIGGER_HIGH, pdev->name, controller);
> > + if (ret) {
> > + dev_err(dev, "cannot request IRQ %d\n", irq);
>
> unnecessary print
Will remove.
>
> > + goto error;
> > + }
> > +
> > + ret = devm_spi_register_master(dev, master);
> > + if (!ret) {
> > + pm_runtime_set_autosuspend_delay(dev, MSEC_PER_SEC);
> > + pm_runtime_use_autosuspend(dev);
> > + pm_runtime_set_active(dev);
> > + pm_runtime_enable(dev);
>
> Remove all the runtime stuff. not supported right now.
>
> > + return ret;
> > + }
> > +error:
> > + clk_disable_unprepare(cclk);
> > + clk_disable_unprepare(iclk);
> > + spi_master_put(master);
> > + return ret;
> > +}
> > +
<snip>
>
> > +
> > +static int spi_qup_remove(struct platform_device *pdev)
> > +{
> > + struct spi_master *master = dev_get_drvdata(&pdev->dev);
> > + struct spi_qup *controller = spi_master_get_devdata(master);
> > +
> > + pm_runtime_get_sync(&pdev->dev);
> > +
>
> Do we need to wait for any current transactions to complete
> and do a devm_free_irq()?
>
> > + clk_disable_unprepare(controller->cclk);
> > + clk_disable_unprepare(controller->iclk);
My understanding is:
Disabling clocks will timeout transaction, if any. Core Device driver
will call: devm_spi_unregister(), which will wait pending transactions
to complete and then remove the SPI master.
> > +
> > + pm_runtime_put_noidle(&pdev->dev);
> > + pm_runtime_disable(&pdev->dev);
> > + return 0;
> > +}
> > +
> > +static struct of_device_id spi_qup_dt_match[] = {
> > + { .compatible = "qcom,spi-qup-v2", },
>
> Need compatible tags of qcom,spi-qup-v2.1.1 (msm8974 v1) or qcom,spi-qup-v2.2.1
> (msm8974 v2)
I am not aware of the difference. My board report v.20020000.
Is there difference of handling these controllers?
Thanks,
Ivan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] can: xilinx CAN controller support.
From: Marc Kleine-Budde @ 2014-02-07 9:37 UTC (permalink / raw)
To: Appana Durga Kedareswara Rao, wg@grandegger.com, Michal Simek,
grant.likely@linaro.org, robh+dt@kernel.org,
linux-can@vger.kernel.org
Cc: netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <8b4dad82-c72a-4e1f-b1af-b8c7964bbf24@TX2EHSMHS043.ehs.local>
[-- Attachment #1: Type: text/plain, Size: 51140 bytes --]
On 02/07/2014 09:42 AM, Appana Durga Kedareswara Rao wrote:
>>> ---
>>> This patch is rebased on the 3.14 rc1 kernel.
>>> ---
>>> .../devicetree/bindings/net/can/xilinx_can.txt | 43 +
>>> drivers/net/can/Kconfig | 8 +
>>> drivers/net/can/Makefile | 1 +
>>> drivers/net/can/xilinx_can.c | 1150 ++++++++++++++++++++
>>> 4 files changed, 1202 insertions(+), 0 deletions(-) create mode
>>> 100644 Documentation/devicetree/bindings/net/can/xilinx_can.txt
>>> create mode 100644 drivers/net/can/xilinx_can.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/can/xilinx_can.txt
>>> b/Documentation/devicetree/bindings/net/can/xilinx_can.txt
>>> new file mode 100644
>>> index 0000000..34f9643
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/can/xilinx_can.txt
>>> @@ -0,0 +1,43 @@
>>> +Xilinx Axi CAN/Zynq CANPS controller Device Tree Bindings
>>> +---------------------------------------------------------
>>> +
>>> +Required properties:
>>> +- compatible : Should be "xlnx,zynq-can-1.00.a" for Zynq
>> CAN
>>> + controllers and "xlnx,axi-can-1.00.a" for Axi CAN
>>> + controllers.
>>> +- reg : Physical base address and size of the Axi CAN/Zynq
>>> + CANPS registers map.
>>> +- interrupts : Property with a value describing the interrupt
>>> + number.
>>> +- interrupt-parent : Must be core interrupt controller
>>> +- clock-names : List of input clock names - "ref_clk",
>> "aper_clk"
>>> + (See clock bindings for details. Two clocks are
>>> + required for Zynq CAN. For Axi CAN
>>> + case it is one(ref_clk)).
>>> +- clocks : Clock phandles (see clock bindings for details).
>>> +- xlnx,can-tx-dpth : Can Tx fifo depth (Required for Axi CAN).
>>> +- xlnx,can-rx-dpth : Can Rx fifo depth (Required for Axi CAN).
>>> +
>>> +
>>> +Example:
>>> +
>>> +For Zynq CANPS Dts file:
>>> + zynq_can_0: zynq-can@e0008000 {
>>> + compatible = "xlnx,zynq-can-1.00.a";
>>> + clocks = <&clkc 19>, <&clkc 36>;
>>> + clock-names = "ref_clk", "aper_clk";
>>> + reg = <0xe0008000 0x1000>;
>>> + interrupts = <0 28 4>;
>>> + interrupt-parent = <&intc>;
>>
>> Above xlnx,can-{rx,tx}-dpth is mentioned as required, but it's not in the
>> Zynq example.
>
> One of the Difference b/w the AXI CAN and zynq CAN is in AXI CAN the fifo depth(tx,rx)
> Is user configurable. But in case of ZYNQ CAN the fifo depth is fixed for tx and rx fifo's(64)
> Xlnx,can-{rx,tx}-dpth is required only for AXI CAN case it is not required for zynq CAN.
> That's why didn't putted that property in device tree.
The device tree should be a hardware only description and should not
hold any user configurable data. Please split your patch into two
patches. The first one should add the driver with a fixed fifo size
(e.g. 0x40) for the AXI, too. The second patch should make the fifo
configurable via device tree.
If it's acceptable to describe the fifo usage by device tree, I'd like
to make it a generic CAN driver option. But we have to look around, e.g.
what the Ethernet driver use to configure their hardware.
>
>>
>>> + };
>>> +For Axi CAN Dts file:
>>> + axi_can_0: axi-can@40000000 {
>>> + compatible = "xlnx,axi-can-1.00.a";
>>> + clocks = <&clkc 0>;
>>> + clock-names = "ref_clk" ;
>>> + reg = <0x40000000 0x10000>;
>>> + interrupt-parent = <&intc>;
>>> + interrupts = <0 59 1>;
>>> + xlnx,can-tx-dpth = <0x40>;
>>> + xlnx,can-rx-dpth = <0x40>;
>>> + };
>>> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig index
>>> d447b88..2344fb5 100644
>>> --- a/drivers/net/can/Kconfig
>>> +++ b/drivers/net/can/Kconfig
>>> @@ -125,6 +125,14 @@ config CAN_GRCAN
>>> endian syntheses of the cores would need some modifications on
>>> the hardware level to work.
>>>
>>> +config CAN_XILINXCAN
>>> + tristate "Xilinx CAN"
>>> + depends on ARCH_ZYNQ || MICROBLAZE
>>> + default n
>>
>> "default n" is default, please remove.
>>
>
> Ok
>
>>> + ---help---
>>> + Xilinx CAN driver. This driver supports both soft AXI CAN IP and
>>> + Zynq CANPS IP.
>>> +
>>> source "drivers/net/can/mscan/Kconfig"
>>>
>>> source "drivers/net/can/sja1000/Kconfig"
>>> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile index
>>> c744039..0b8e11e 100644
>>> --- a/drivers/net/can/Makefile
>>> +++ b/drivers/net/can/Makefile
>>> @@ -25,5 +25,6 @@ obj-$(CONFIG_CAN_JANZ_ICAN3) += janz-
>> ican3.o
>>> obj-$(CONFIG_CAN_FLEXCAN) += flexcan.o
>>> obj-$(CONFIG_PCH_CAN) += pch_can.o
>>> obj-$(CONFIG_CAN_GRCAN) += grcan.o
>>> +obj-$(CONFIG_CAN_XILINXCAN) += xilinx_can.o
>>>
>>> ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG diff --git
>>> a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c new file
>>> mode 100644 index 0000000..c1b2b9d
>>> --- /dev/null
>>> +++ b/drivers/net/can/xilinx_can.c
>>> @@ -0,0 +1,1150 @@
>>> +/* Xilinx CAN device driver
>>> + *
>>> + * Copyright (C) 2012 - 2014 Xilinx, Inc.
>>> + * Copyright (C) 2009 PetaLogix. All rights reserved.
>>> + *
>>> + * Description:
>>> + * This driver is developed for Axi CAN IP and for Zynq CANPS Controller.
>>> + * 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.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/errno.h>
>>> +#include <linux/init.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/io.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/netdevice.h>
>>> +#include <linux/of.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/skbuff.h>
>>> +#include <linux/string.h>
>>> +#include <linux/types.h>
>>> +#include <linux/can/dev.h>
>>> +#include <linux/can/error.h>
>>> +#include <linux/can/led.h>
>>> +
>>> +#define DRIVER_NAME "XILINX_CAN"
>>> +
>>> +/* CAN registers set */
>>> +#define XCAN_SRR_OFFSET 0x00 /* Software reset */
>>> +#define XCAN_MSR_OFFSET 0x04 /* Mode select */
>>> +#define XCAN_BRPR_OFFSET 0x08 /* Baud rate prescaler */
>>> +#define XCAN_BTR_OFFSET 0x0C /* Bit timing */
>>> +#define XCAN_ECR_OFFSET 0x10 /* Error counter */
>>> +#define XCAN_ESR_OFFSET 0x14 /* Error status */
>>> +#define XCAN_SR_OFFSET 0x18 /* Status */
>>> +#define XCAN_ISR_OFFSET 0x1C /* Interrupt status */
>>> +#define XCAN_IER_OFFSET 0x20 /* Interrupt enable */
>>> +#define XCAN_ICR_OFFSET 0x24 /* Interrupt clear */
>>> +#define XCAN_TXFIFO_ID_OFFSET 0x30 /* TX FIFO ID */
>>> +#define XCAN_TXFIFO_DLC_OFFSET 0x34 /* TX FIFO DLC */
>>> +#define XCAN_TXFIFO_DW1_OFFSET 0x38 /* TX FIFO Data
>> Word 1 */
>>> +#define XCAN_TXFIFO_DW2_OFFSET 0x3C /* TX FIFO Data
>> Word 2 */
>>> +#define XCAN_RXFIFO_ID_OFFSET 0x50 /* RX FIFO ID */
>>> +#define XCAN_RXFIFO_DLC_OFFSET 0x54 /* RX FIFO DLC */
>>> +#define XCAN_RXFIFO_DW1_OFFSET 0x58 /* RX FIFO Data
>> Word 1 */
>>> +#define XCAN_RXFIFO_DW2_OFFSET 0x5C /* RX FIFO Data
>> Word 2 */
>>> +
>>> +/* CAN register bit masks - XCAN_<REG>_<BIT>_MASK */
>>> +#define XCAN_SRR_CEN_MASK 0x00000002 /* CAN enable */
>>> +#define XCAN_SRR_RESET_MASK 0x00000001 /* Soft Reset the
>> CAN core */
>>> +#define XCAN_MSR_LBACK_MASK 0x00000002 /* Loop back
>> mode select */
>>> +#define XCAN_MSR_SLEEP_MASK 0x00000001 /* Sleep mode
>> select */
>>> +#define XCAN_BRPR_BRP_MASK 0x000000FF /* Baud rate
>> prescaler */
>>> +#define XCAN_BTR_SJW_MASK 0x00000180 /* Synchronous
>> jump width */
>>> +#define XCAN_BTR_TS2_MASK 0x00000070 /* Time segment
>> 2 */
>>> +#define XCAN_BTR_TS1_MASK 0x0000000F /* Time segment
>> 1 */
>>> +#define XCAN_ECR_REC_MASK 0x0000FF00 /* Receive error
>> counter */
>>> +#define XCAN_ECR_TEC_MASK 0x000000FF /* Transmit error
>> counter */
>>> +#define XCAN_ESR_ACKER_MASK 0x00000010 /* ACK error */
>>> +#define XCAN_ESR_BERR_MASK 0x00000008 /* Bit error */
>>> +#define XCAN_ESR_STER_MASK 0x00000004 /* Stuff error */
>>> +#define XCAN_ESR_FMER_MASK 0x00000002 /* Form error */
>>> +#define XCAN_ESR_CRCER_MASK 0x00000001 /* CRC error */
>>> +#define XCAN_SR_TXFLL_MASK 0x00000400 /* TX FIFO is full
>> */
>>> +#define XCAN_SR_ESTAT_MASK 0x00000180 /* Error status */
>>> +#define XCAN_SR_ERRWRN_MASK 0x00000040 /* Error warning
>> */
>>> +#define XCAN_SR_NORMAL_MASK 0x00000008 /* Normal mode
>> */
>>> +#define XCAN_SR_LBACK_MASK 0x00000002 /* Loop back
>> mode */
>>> +#define XCAN_SR_CONFIG_MASK 0x00000001 /* Configuration
>> mode */
>>> +#define XCAN_IXR_TXFEMP_MASK 0x00004000 /* TX FIFO Empty
>> */
>>> +#define XCAN_IXR_WKUP_MASK 0x00000800 /* Wake up
>> interrupt */
>>> +#define XCAN_IXR_SLP_MASK 0x00000400 /* Sleep
>> interrupt */
>>> +#define XCAN_IXR_BSOFF_MASK 0x00000200 /* Bus off
>> interrupt */
>>> +#define XCAN_IXR_ERROR_MASK 0x00000100 /* Error interrupt
>> */
>>> +#define XCAN_IXR_RXNEMP_MASK 0x00000080 /* RX FIFO
>> NotEmpty intr */
>>> +#define XCAN_IXR_RXOFLW_MASK 0x00000040 /* RX FIFO
>> Overflow intr */
>>> +#define XCAN_IXR_RXOK_MASK 0x00000010 /* Message
>> received intr */
>>> +#define XCAN_IXR_TXOK_MASK 0x00000002 /* TX successful
>> intr */
>>> +#define XCAN_IXR_ARBLST_MASK 0x00000001 /* Arbitration
>> lost intr */
>>> +#define XCAN_IDR_ID1_MASK 0xFFE00000 /* Standard msg
>> identifier */
>>> +#define XCAN_IDR_SRR_MASK 0x00100000 /* Substitute
>> remote TXreq */
>>> +#define XCAN_IDR_IDE_MASK 0x00080000 /* Identifier
>> extension */
>>> +#define XCAN_IDR_ID2_MASK 0x0007FFFE /* Extended
>> message ident */
>>> +#define XCAN_IDR_RTR_MASK 0x00000001 /* Remote TX
>> request */
>>> +#define XCAN_DLCR_DLC_MASK 0xF0000000 /* Data length
>> code */
>>> +
>>> +#define XCAN_INTR_ALL (XCAN_IXR_TXOK_MASK |
>> XCAN_IXR_BSOFF_MASK |\
>>> + XCAN_IXR_WKUP_MASK |
>> XCAN_IXR_SLP_MASK | \
>>> + XCAN_IXR_RXNEMP_MASK |
>> XCAN_IXR_ERROR_MASK | \
>>> + XCAN_IXR_ARBLST_MASK |
>> XCAN_IXR_RXOK_MASK)
>>> +
>>> +/* CAN register bit shift - XCAN_<REG>_<BIT>_SHIFT */
>>> +#define XCAN_BTR_SJW_SHIFT 7 /* Synchronous jump width
>> */
>>> +#define XCAN_BTR_TS2_SHIFT 4 /* Time segment 2 */
>>> +#define XCAN_IDR_ID1_SHIFT 21 /* Standard Messg
>> Identifier */
>>> +#define XCAN_IDR_ID2_SHIFT 1 /* Extended Message
>> Identifier */
>>> +#define XCAN_DLCR_DLC_SHIFT 28 /* Data length code */
>>> +#define XCAN_ESR_REC_SHIFT 8 /* Rx Error Count */
>>> +
>>> +/* CAN frame length constants */
>>> +#define XCAN_ECHO_SKB_MAX 64
>>> +#define XCAN_NAPI_WEIGHT 64
>>> +#define XCAN_FRAME_MAX_DATA_LEN 8
>>> +#define XCAN_TIMEOUT (50 * HZ)
>>> +
>>> +/**
>>> + * struct xcan_priv - This definition define CAN driver instance
>>> + * @can: CAN private data structure.
>>> + * @open_time: For holding timeout values
>>> + * @waiting_ech_skb_index: Pointer for skb
>>> + * @ech_skb_next: This tell the next packet in the queue
>>> + * @waiting_ech_skb_num: Gives the number of packets waiting
>>> + * @xcan_echo_skb_max_tx: Maximum number packets the driver
>> can send
>>> + * @xcan_echo_skb_max_rx: Maximum number packets the driver
>> can receive
>>> + * @napi: NAPI structure
>>> + * @ech_skb_lock: For spinlock purpose
>>> + * @read_reg: For reading data from CAN registers
>>> + * @write_reg: For writing data to CAN registers
>>> + * @dev: Network device data structure
>>> + * @reg_base: Ioremapped address to registers
>>> + * @irq_flags: For request_irq()
>>> + * @aperclk: Pointer to struct clk
>>> + * @devclk: Pointer to struct clk
>>> + */
>>> +struct xcan_priv {
>>> + struct can_priv can;
>>> + int open_time;
>>> + int waiting_ech_skb_index;
>>> + int ech_skb_next;
>>> + int waiting_ech_skb_num;
>>> + int xcan_echo_skb_max_tx;
>>> + int xcan_echo_skb_max_rx;
>>> + struct napi_struct napi;
>>> + spinlock_t ech_skb_lock;
>>> + u32 (*read_reg)(const struct xcan_priv *priv, int reg);
>>> + void (*write_reg)(const struct xcan_priv *priv, int reg, u32 val);
>>
>> Why do you have {read,write}_reg function here?
>
> xcan_write_reg/xcan_read_reg is used because this IP is also in big endian version.
> We won't support It but if someone else want to support that's why putted these function's here.
>
> If you are not ok with this will remove.
As far as I know, the kernel will either be little or big endian, so we
can make a use of an ifdef for here, if we support LE and BE.
>
>>
>>> + struct net_device *dev;
>>> + void __iomem *reg_base;
>>> + unsigned long irq_flags;
>>> + struct clk *aperclk;
>>> + struct clk *devclk;
>>> +};
>>> +
>>> +/* CAN Bittiming constants as per Xilinx CAN specs */ static struct
>>> +can_bittiming_const xcan_bittiming_const = {
Please make it "static const struct"
>>> + .name = DRIVER_NAME,
>>> + .tseg1_min = 1,
>>> + .tseg1_max = 16,
>>> + .tseg2_min = 1,
>>> + .tseg2_max = 8,
>>> + .sjw_max = 4,
>>> + .brp_min = 1,
>>> + .brp_max = 256,
>>> + .brp_inc = 1,
>>> +};
>>> +
>>> +/**
>>> + * xcan_write_reg - Write a value to the device register
>>> + * @priv: Driver private data structure
>>> + * @reg: Register offset
>>> + * @val: Value to write at the Register offset
>>> + *
>>> + * Write data to the paricular CAN register */ static void
>>> +xcan_write_reg(const struct xcan_priv *priv, int reg, u32 val) {
>>> + writel(val, priv->reg_base + reg);
>>> +}
>>> +
>>> +/**
>>> + * xcan_read_reg - Read a value from the device register
>>> + * @priv: Driver private data structure
>>> + * @reg: Register offset
>>> + *
>>> + * Read data from the particular CAN register
>>> + * Return: value read from the CAN register */ static u32
>>> +xcan_read_reg(const struct xcan_priv *priv, int reg) {
>>> + return readl(priv->reg_base + reg);
>>> +}
>>> +
>>> +/**
>>> + * set_reset_mode - Resets the CAN device mode
>>> + * @ndev: Pointer to net_device structure
>>> + *
>>> + * This is the driver reset mode routine.The driver
>>> + * enters into configuration mode.
>>> + *
>>> + * Return: 0 on success and failure value on error */ static int
>>> +set_reset_mode(struct net_device *ndev) {
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> + unsigned long timeout;
>>> +
>>> + priv->can.state = CAN_STATE_STOPPED;
>>> + priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_OFFSET);
>>> +
>>> + timeout = jiffies + XCAN_TIMEOUT;
>>> + while (!(priv->read_reg(priv, XCAN_SR_OFFSET) &
>> XCAN_SR_CONFIG_MASK)) {
>>> + if (time_after(jiffies, timeout)) {
>>> + netdev_warn(ndev, "timedout waiting for config
>> mode\n");
>>> + return -ETIMEDOUT;
>>> + }
>>> + schedule_timeout(1);
>>
>> better use usleep_range()
>>
>
> Ok
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * xcan_set_bittiming - CAN set bit timing routine
>>> + * @ndev: Pointer to net_device structure
>>> + *
>>> + * This is the driver set bittiming routine.
>>> + * Return: 0 on success and failure value on error */ static int
>>> +xcan_set_bittiming(struct net_device *ndev) {
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> + struct can_bittiming *bt = &priv->can.bittiming;
>>> + u32 btr0, btr1;
>>> + u32 is_config_mode;
>>> +
>>> + /* Check whether Xilinx CAN is in configuration mode.
>>> + * It cannot set bit timing if Xilinx CAN is not in configuration mode.
>>> + */
>>> + is_config_mode = priv->read_reg(priv, XCAN_SR_OFFSET) &
>>> + XCAN_SR_CONFIG_MASK;
>>> + if (!is_config_mode) {
>>> + netdev_alert(ndev,
>>> + "Cannot set bittiming can is not in config mode\n");
>>> + return -EPERM;
>>> + }
>>> +
>>> + netdev_dbg(ndev,
>> "brp=%d,prop=%d,phase_seg1:%d,phase_reg2=%d,sjw=%d\n",
>>> + bt->brp, bt->prop_seg, bt->phase_seg1, bt-
>>> phase_seg2,
>>> + bt->sjw);
>>
>> I think this dbg can be removed, as it just prints the userspace values.
>>
> Ok
>
>>> +
>>> + /* Setting Baud Rate prescalar value in BRPR Register */
>>> + btr0 = (bt->brp - 1) & XCAN_BRPR_BRP_MASK;
>>> +
>>> + /* Setting Time Segment 1 in BTR Register */
>>> + btr1 = (bt->prop_seg + bt->phase_seg1 - 1) & XCAN_BTR_TS1_MASK;
>>> +
>>> + /* Setting Time Segment 2 in BTR Register */
>>> + btr1 |= ((bt->phase_seg2 - 1) << XCAN_BTR_TS2_SHIFT) &
>>> + XCAN_BTR_TS2_MASK;
>>> +
>>> + /* Setting Synchronous jump width in BTR Register */
>>> + btr1 |= ((bt->sjw - 1) << XCAN_BTR_SJW_SHIFT) &
>> XCAN_BTR_SJW_MASK;
>>> +
>>> + if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
>>> + netdev_info(ndev, "Doesn't support Triple Sampling\n");
>>
>> no need to check, it's not passed to the driver until you advertise you
>> support it (see priv->can.ctrlmode_supported).
>
>
> Ok
>
>>
>>> + netdev_dbg(ndev, "Setting BTR0=0x%02x BTR1=0x%02x\n", btr0,
>> btr1);
>>> +
>>> + priv->write_reg(priv, XCAN_BRPR_OFFSET, btr0);
>>> + priv->write_reg(priv, XCAN_BTR_OFFSET, btr1);
>>> +
>>> + netdev_dbg(ndev, "BRPR=0x%08x, BTR=0x%08x\n",
>>> + priv->read_reg(priv, XCAN_BRPR_OFFSET),
>>> + priv->read_reg(priv, XCAN_BTR_OFFSET));
>>
>> One of the dbg should be enough.
>>
>
>
> Ok
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * xcan_start - This the drivers start routine
>>> + * @ndev: Pointer to net_device structure
>>> + *
>>> + * This is the drivers start routine.
>>> + * Based on the State of the CAN device it puts
>>> + * the CAN device into a proper mode.
>>> + *
>>> + * Return: 0 always
>>> + */
>>> +static int xcan_start(struct net_device *ndev) {
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> +
>>> + /* Check if it is in reset mode */
>>> + if (priv->can.state != CAN_STATE_STOPPED)
>>> + set_reset_mode(ndev);
>>
>> Please check return value of set_reset_mode
>
> Ok
>
>
>>
>>> +
>>> + /* Enable interrupts */
>>> + priv->write_reg(priv, XCAN_IER_OFFSET, XCAN_INTR_ALL);
>>> +
>>> + /* Check whether it is loopback mode or normal mode */
>>> + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
>>> + /* Put device into loopback mode */
>>> + priv->write_reg(priv, XCAN_MSR_OFFSET,
>> XCAN_MSR_LBACK_MASK);
>>> + else
>>> + /* The device is in normal mode */
>>> + priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
>>> +
>>> + if (priv->can.state == CAN_STATE_STOPPED) {
>>
>> I think your driver is always in CAN_STATE_STOPPED, right?
>
> Usually it in stopped state only(configuration mode) during initialization.
>
>>
>>> + /* Enable Xilinx CAN */
>>> + priv->write_reg(priv, XCAN_SRR_OFFSET,
>> XCAN_SRR_CEN_MASK);
>>> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>> + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
>>> + while ((priv->read_reg(priv, XCAN_SR_OFFSET) &
>>> + XCAN_SR_LBACK_MASK) == 0)
>>> + ;
>>> + } else {
>>> + while ((priv->read_reg(priv, XCAN_SR_OFFSET)
>>> + & XCAN_SR_NORMAL_MASK) == 0)
>>> + ;
>>
>> Please add a timeout to the loops.
>
> Ok
>>
>>> + }
>>> + netdev_dbg(ndev, "status:#x%08x\n",
>>> + priv->read_reg(priv, XCAN_SR_OFFSET));
>>> + }
>>> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * xcan_do_set_mode - This sets the mode of the driver
>>> + * @ndev: Pointer to net_device structure
>>> + * @mode: Tells the mode of the driver
>>> + *
>>> + * This check the drivers state and calls the
>>> + * the corresponding modes to set.
>>> + *
>>> + * Return: 0 on success and failure value on error */ static int
>>> +xcan_do_set_mode(struct net_device *ndev, enum can_mode mode) {
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> + int ret;
>>> +
>>> + netdev_dbg(ndev, "Setting the mode of the driver%s\n", __func__);
>>
>> please remove dbg
>
> Ok
>>
>>> +
>>> + if (!priv->open_time)
>>> + return -EINVAL;
>>
>> please remove open_time completely.
>
> Ok
>
>
>
>>
>>> +
>>> + switch (mode) {
>>> + case CAN_MODE_START:
>>> + ret = xcan_start(ndev);
>>> + if (ret < 0)
>>> + netdev_err(ndev, "xcan_start failed!\n");
>>> +
>>> + if (netif_queue_stopped(ndev))
>>> + netif_wake_queue(ndev);
>>
>> just call wake_queue
>
> Ok. But existing drivers are using netif_wake_queue right?
Sorry, I meant: Remove the if, just call netif_wake_queue directly.
>
>>> + break;
>>> + default:
>>> + ret = -EOPNOTSUPP;
>>> + break;
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +/**
>>> + * xcan_start_xmit - Starts the transmission
>>> + * @skb: sk_buff pointer that contains data to be Txed
>>> + * @ndev: Pointer to net_device structure
>>> + *
>>> + * This function is invoked from upper layers to initiate
>>> +transmission. This
>>> + * function uses the next available free txbuff and populates their
>>> +fields to
>>> + * start the transmission.
>>> + *
>>> + * Return: 0 on success and failure value on error */ static int
>>> +xcan_start_xmit(struct sk_buff *skb, struct net_device *ndev) {
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> + struct net_device_stats *stats = &ndev->stats;
>>> + struct can_frame *cf = (struct can_frame *)skb->data;
>>> + u32 id, dlc, tmp_dw1, tmp_dw2 = 0, data1, data2 = 0;
>>> + unsigned long flags;
>>> +
>>
>> Please add here:
>>
>> if (can_dropped_invalid_skb(dev, skb))
>> return NETDEV_TX_OK;
>
> Ok
>
>
>
>>
>>> + /* Check if the TX buffer is full */
>>> + if (priv->read_reg(priv, XCAN_SR_OFFSET) & XCAN_SR_TXFLL_MASK)
>> {
>>> + netif_stop_queue(ndev);
>>> + netdev_err(ndev, "TX register is still full!\n");
>>> + return NETDEV_TX_BUSY;
>>> + } else if (priv->waiting_ech_skb_num == priv-
>>> xcan_echo_skb_max_tx) {
>>> + netif_stop_queue(ndev);
>>> + netdev_err(ndev, "waiting:0x%08x, max:0x%08x\n",
>>> + priv->waiting_ech_skb_num, priv-
>>> xcan_echo_skb_max_tx);
>>> + return NETDEV_TX_BUSY;
>>> + }
>>
>> You should handle flow control after you put the CAN frame into the
>> hardware, but before activating the TX complete interrutp.
>>
>
> Ok
>
>
>>> + /* Watch carefully on the bit sequence */
>>> + if ((cf->can_id & CAN_EFF_FLAG) == 0) {
>>
>> Nitpick easier to read is:
>>
>>
>> if (cf->can_id & CAN_EFF_FLAG) {
>> /* EFF handling */
>> } else {
>> /* STD handling */
>> }
>>
> Ok
>
>
>>> + /* Standard CAN ID format */
>>> + id = ((cf->can_id & CAN_SFF_MASK) << XCAN_IDR_ID1_SHIFT)
>> &
>>> + XCAN_IDR_ID1_MASK;
>>> +
>>> + if (cf->can_id & CAN_RTR_FLAG)
>>> + /* Extended frames remote TX request */
>>> + id |= XCAN_IDR_SRR_MASK;
>>> + } else {
>>> + /* Extended CAN ID format */
>>> + id = ((cf->can_id & CAN_EFF_MASK) << XCAN_IDR_ID2_SHIFT)
>> &
>>> + XCAN_IDR_ID2_MASK;
>>> + id |= (((cf->can_id & CAN_EFF_MASK) >>
>>> + (CAN_EFF_ID_BITS-CAN_SFF_ID_BITS)) <<
>>> + XCAN_IDR_ID1_SHIFT) & XCAN_IDR_ID1_MASK;
>>> +
>>> + /* The substibute remote TX request bit should be "1"
>>> + * for extended frames as in the Xilinx CAN datasheet
>>> + */
>>> + id |= XCAN_IDR_IDE_MASK | XCAN_IDR_SRR_MASK;
>>> +
>>> + if (cf->can_id & CAN_RTR_FLAG)
>>> + /* Extended frames remote TX request */
>>> + id |= XCAN_IDR_RTR_MASK;
>>> + }
>>> +
>>> + dlc = (cf->can_dlc & 0xf) << XCAN_DLCR_DLC_SHIFT;
>>
>> With the above check can_dlc is valid.
>>
>>> +
>>> + tmp_dw1 = le32_to_cpup((u32 *)(cf->data));
>>> + data1 = htonl(tmp_dw1);
>>
>> This looks broken. cf->data is in big endian, what is the endianess of your
>> registers?
>>
>
> Endianess of our registers is little endian that's why need to do like this.
Is it always little endian or native endianess (e.g. when we're running
a big endian kernel)?
As the driver support only little endian so far, make I suggest to use
be32_to_cpup():
u32 data[2] = { 0, 0 };
if (dlc > 0)
data[0] = be32_to_cpup((__be32 *)(cf->data + 0));
if (dlc > 4)
data[1] = be32_to_cpup((__be32 *)(cf->data + 4));
>
>>> + if (dlc > 4) {
>>> + tmp_dw2 = le32_to_cpup((u32 *)(cf->data + 4));
>>> + data2 = htonl(tmp_dw2);
>>> + }
>>> +
>>> + netdev_dbg(ndev,
>> "tx:id=0x%08x,dlc=0x%08x,d1=0x%08x,d2=0x%08x\n",
>>> + id, dlc, data1, data2);
>>
>> please remove the dbg
>>
>
> Ok
>
>>> + /* Write the Frame to Xilinx CAN TX FIFO */
>>> + priv->write_reg(priv, XCAN_TXFIFO_ID_OFFSET, id);
>>> + priv->write_reg(priv, XCAN_TXFIFO_DLC_OFFSET, dlc);
>>> + priv->write_reg(priv, XCAN_TXFIFO_DW1_OFFSET, data1);
>>> + priv->write_reg(priv, XCAN_TXFIFO_DW2_OFFSET, data2);
>>
>> Which write triggers the transmission?
>>
>
> The last write triggers the transmission.
> If we are sending data less than 4 bytes in that case also we need to
> Fill this( XCAN_TXFIFO_DW2_OFFSET) with default value .
I see, can you please add a comment to the code. What about
XCAN_TXFIFO_DW1_OFFSET in case dlc == 0?
>
>>> + stats->tx_bytes += cf->can_dlc;
>>
>> Can you move the tx_bytes += to your tx-complete routine?
>
> Ok
>>
>>> + ndev->trans_start = jiffies;
>>
>> Please remove
>
> Ok
>
>
>>> +
>>> + can_put_echo_skb(skb, ndev, priv->ech_skb_next);
>>
>> This looks racy, first fill the echo_skb, then start the transmission.
>
> Ok But I didn't understand it clearly.
> Will you please explain a little clear.
This will trigger transmission:
priv->write_reg(priv, XCAN_TXFIFO_DW2_OFFSET, data2);
Then you get a tx-complete interrupt.
The tx-complete handler takes care about the echo skb. But the echo skb
hasn't been filed yet.
Your tx code continues:
can_put_echo_skb(skb, ndev, priv->ech_skb_next);
The echo skb handling is now totlaly messed up :)
>
>
>>> +
>>> + priv->ech_skb_next = (priv->ech_skb_next + 1) %
>>> + priv->xcan_echo_skb_max_tx;
>>> +
>>> + spin_lock_irqsave(&priv->ech_skb_lock, flags);
>>> + priv->waiting_ech_skb_num++;
>>> + spin_unlock_irqrestore(&priv->ech_skb_lock, flags);
>>> +
>>
>> Please move the flow controll handling here.
>>
>
> Ok
>
>>> + return NETDEV_TX_OK;
>>> +}
>>> +
>>> +/**
>>> + * xcan_rx - Is called from CAN isr to complete the received
>>> + * frame processing
>>> + * @ndev: Pointer to net_device structure
>>> + *
>>> + * This function is invoked from the CAN isr(poll) to process the Rx
>>> +frames. It
>>> + * does minimal processing and invokes "netif_receive_skb" to
>>> +complete further
>>> + * processing.
>>> + * Return: 0 on success and negative error value on error */ static
>>> +int xcan_rx(struct net_device *ndev) {
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> + struct net_device_stats *stats = &ndev->stats;
>>> + struct can_frame *cf;
>>> + struct sk_buff *skb;
>>> + u32 id_xcan, dlc, data1, data2;
>>> +
>>> + skb = alloc_can_skb(ndev, &cf);
>>> + if (!skb)
>>> + return -ENOMEM;
>>> +
>>> + /* Read a frame from Xilinx zynq CANPS */
>>> + id_xcan = priv->read_reg(priv, XCAN_RXFIFO_ID_OFFSET);
>>> + dlc = priv->read_reg(priv, XCAN_RXFIFO_DLC_OFFSET) &
>> XCAN_DLCR_DLC_MASK;
>>> + data1 = priv->read_reg(priv, XCAN_RXFIFO_DW1_OFFSET);
>>> + data2 = priv->read_reg(priv, XCAN_RXFIFO_DW2_OFFSET);
>>
>> If you don't use data? below, don't read them in the first place. Better move
>> the read below, where you fill the data of the can_frame.
>>
>
> Ok
>
>> + netdev_dbg(ndev,
>> "rx:id=0x%08x,dlc=0x%08x,d1=0x%08x,d2=0x%08x\n",
>>> + id_xcan, dlc, data1, data2);
>>>
>> please remove dbg
>
> Ok
>
>>
>> +
>>> + /* Change Xilinx CAN data length format to socketCAN data format
>> */
>>> + cf->can_dlc = get_can_dlc((dlc & XCAN_DLCR_DLC_MASK) >>
>>> + XCAN_DLCR_DLC_SHIFT);
>>> +
>>> + /* Change Xilinx CAN ID format to socketCAN ID format */
>>> + if (id_xcan & XCAN_IDR_IDE_MASK) {
>>> + /* The received frame is an Extended format frame */
>>> + cf->can_id = (id_xcan & XCAN_IDR_ID1_MASK) >> 3;
>>> + cf->can_id |= (id_xcan & XCAN_IDR_ID2_MASK) >>
>>> + XCAN_IDR_ID2_SHIFT;
>>> + cf->can_id |= CAN_EFF_FLAG;
>>> + if (id_xcan & XCAN_IDR_RTR_MASK)
>>> + cf->can_id |= CAN_RTR_FLAG;
>>> + } else {
>>> + /* The received frame is a standard format frame */
>>> + cf->can_id = (id_xcan & XCAN_IDR_ID1_MASK) >>
>>> + XCAN_IDR_ID1_SHIFT;
>>> + if (id_xcan & XCAN_IDR_RTR_MASK)
>>> + cf->can_id |= CAN_RTR_FLAG;
>>> + }
>>> +
>>> + /* Change Xilinx CAN data format to socketCAN data format */
>>
>> Don't fill cf->data if RTR is set. The endianess handling looks weird here,
>> too.
>>
> OK
>
>>> + *(u32 *)(cf->data) = ntohl(data1);
>>> + if (cf->can_dlc > 4)
>>> + *(u32 *)(cf->data + 4) = ntohl(data2);
>>> + else
>>> + *(u32 *)(cf->data + 4) = 0;
>>
>> no need to set to zero
>>
> Ok
>>> + stats->rx_bytes += cf->can_dlc;
>>
>> please group rx_bytes and rx_packets handling
>>> +
>
> Ok
>>> + can_led_event(ndev, CAN_LED_EVENT_RX);
>>> +
>>> + netif_receive_skb(skb);
>>> +
>>> + stats->rx_packets++;
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * xcan_err_interrupt - error frame Isr
>>> + * @ndev: net_device pointer
>>> + * @isr: interrupt status register value
>>> + *
>>> + * This is the CAN error interrupt and it will
>>> + * check the the type of error and forward the error
>>> + * frame to upper layers.
>>> + */
>>> +static void xcan_err_interrupt(struct net_device *ndev, u32 isr) {
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> + struct net_device_stats *stats = &ndev->stats;
>>> + struct can_frame *cf;
>>> + struct sk_buff *skb;
>>> + u32 err_status, status;
>>> +
>>> + skb = alloc_can_err_skb(ndev, &cf);
>>> + if (!skb) {
>>> + netdev_err(ndev, "alloc_can_err_skb() failed!\n");
>>> + return;
>>> + }
>>> +
>>> + err_status = priv->read_reg(priv, XCAN_ESR_OFFSET);
>>> + priv->write_reg(priv, XCAN_ESR_OFFSET, err_status);
>>> + status = priv->read_reg(priv, XCAN_SR_OFFSET);
>>> +
>>> + if (isr & XCAN_IXR_BSOFF_MASK) {
>>> + priv->can.state = CAN_STATE_BUS_OFF;
>>> + cf->can_id |= CAN_ERR_BUSOFF;
>>> + priv->can.can_stats.bus_off++;
>>> + /* Leave device in Config Mode in bus-off state */
>>> + priv->write_reg(priv, XCAN_SRR_OFFSET,
>> XCAN_SRR_RESET_MASK);
>>> + can_bus_off(ndev);
>>> + } else if ((status & XCAN_SR_ESTAT_MASK) ==
>> XCAN_SR_ESTAT_MASK) {
>>> + cf->can_id |= CAN_ERR_CRTL;
>>> + priv->can.state = CAN_STATE_ERROR_PASSIVE;
>>> + priv->can.can_stats.error_passive++;
>>> + cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE |
>>> + CAN_ERR_CRTL_TX_PASSIVE;
>>> + } else if (status & XCAN_SR_ERRWRN_MASK) {
>>> + cf->can_id |= CAN_ERR_CRTL;
>>> + priv->can.state = CAN_STATE_ERROR_WARNING;
>>> + priv->can.can_stats.error_warning++;
>>> + cf->data[1] |= CAN_ERR_CRTL_RX_WARNING |
>>> + CAN_ERR_CRTL_TX_WARNING;
>>> + }
>>> +
>>> + /* Check for Arbitration lost interrupt */
>>> + if (isr & XCAN_IXR_ARBLST_MASK) {
>>> + cf->can_id |= CAN_ERR_LOSTARB;
>>> + cf->data[0] = CAN_ERR_LOSTARB_UNSPEC;
>>> + priv->can.can_stats.arbitration_lost++;
>>> + }
>>> +
>>> + /* Check for RX FIFO Overflow interrupt */
>>> + if (isr & XCAN_IXR_RXOFLW_MASK) {
>>> + cf->can_id |= CAN_ERR_CRTL;
>>> + cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
>>> + stats->rx_over_errors++;
>>> + stats->rx_errors++;
>>> + priv->write_reg(priv, XCAN_SRR_OFFSET,
>> XCAN_SRR_RESET_MASK);
>>> + }
>>> +
>>> + /* Check for error interrupt */
>>> + if (isr & XCAN_IXR_ERROR_MASK) {
>>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>> + cf->data[2] |= CAN_ERR_PROT_UNSPEC;
>>> +
>>> + /* Check for Ack error interrupt */
>>> + if (err_status & XCAN_ESR_ACKER_MASK) {
>>> + cf->can_id |= CAN_ERR_ACK;
>>> + cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
>>> + stats->tx_errors++;
>>> + }
>>> +
>>> + /* Check for Bit error interrupt */
>>> + if (err_status & XCAN_ESR_BERR_MASK) {
>>> + cf->can_id |= CAN_ERR_PROT;
>>> + cf->data[2] = CAN_ERR_PROT_BIT;
>>> + stats->tx_errors++;
>>> + }
>>> +
>>> + /* Check for Stuff error interrupt */
>>> + if (err_status & XCAN_ESR_STER_MASK) {
>>> + cf->can_id |= CAN_ERR_PROT;
>>> + cf->data[2] = CAN_ERR_PROT_STUFF;
>>> + stats->rx_errors++;
>>> + }
>>> +
>>> + /* Check for Form error interrupt */
>>> + if (err_status & XCAN_ESR_FMER_MASK) {
>>> + cf->can_id |= CAN_ERR_PROT;
>>> + cf->data[2] = CAN_ERR_PROT_FORM;
>>> + stats->rx_errors++;
>>> + }
>>> +
>>> + /* Check for CRC error interrupt */
>>> + if (err_status & XCAN_ESR_CRCER_MASK) {
>>> + cf->can_id |= CAN_ERR_PROT;
>>> + cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ |
>>> + CAN_ERR_PROT_LOC_CRC_DEL;
>>> + stats->rx_errors++;
>>> + }
>>> + priv->can.can_stats.bus_error++;
>>> + }
>>> +
>>> + netif_rx(skb);
>>> + stats->rx_packets++;
>>> + stats->rx_bytes += cf->can_dlc;
>>> +
>>> + netdev_dbg(ndev, "%s: error status register:0x%x\n",
>>> + __func__, priv->read_reg(priv, XCAN_ESR_OFFSET));
>> }
>>> +
>>> +/**
>>> + * xcan_state_interrupt - It will check the state of the CAN device
>>> + * @ndev: net_device pointer
>>> + * @isr: interrupt status register value
>>> + *
>>> + * This will checks the state of the CAN device
>>> + * and puts the device into appropriate state.
>>> + */
>>> +static void xcan_state_interrupt(struct net_device *ndev, u32 isr) {
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> +
>>> + /* Check for Sleep interrupt if set put CAN device in sleep state */
>>> + if (isr & XCAN_IXR_SLP_MASK)
>>> + priv->can.state = CAN_STATE_SLEEPING;
>>> +
>>> + /* Check for Wake up interrupt if set put CAN device in Active state
>> */
>>> + if (isr & XCAN_IXR_WKUP_MASK)
>>> + priv->can.state = CAN_STATE_ERROR_ACTIVE; }
>>> +
>>> +/**
>>> + * xcan_rx_poll - Poll routine for rx packets (NAPI)
>>> + * @napi: napi structure pointer
>>> + * @quota: Max number of rx packets to be processed.
>>> + *
>>> + * This is the poll routine for rx part.
>>> + * It will process the packets maximux quota value.
>>> + *
>>> + * Return: number of packets received */ static int
>>> +xcan_rx_poll(struct napi_struct *napi, int quota) {
>>> + struct net_device *ndev = napi->dev;
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> + u32 isr, ier;
>>> + int work_done = 0;
>>> +
>>> + isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
>>> + while ((isr & XCAN_IXR_RXNEMP_MASK) && (work_done < quota)) {
>>> + if (isr & XCAN_IXR_RXOK_MASK) {
>>> + priv->write_reg(priv, XCAN_ICR_OFFSET,
>>> + XCAN_IXR_RXOK_MASK);
>>> + if (xcan_rx(ndev) < 0)
>>> + return work_done;
>>> + work_done++;
>>> + } else {
>>> + priv->write_reg(priv, XCAN_ICR_OFFSET,
>>> + XCAN_IXR_RXNEMP_MASK);
>>> + break;
>>> + }
>>> + priv->write_reg(priv, XCAN_ICR_OFFSET,
>> XCAN_IXR_RXNEMP_MASK);
>>> + isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
>>> + }
>>> +
>>> + if (work_done < quota) {
>>> + napi_complete(napi);
>>> + ier = priv->read_reg(priv, XCAN_IER_OFFSET);
>>> + ier |= (XCAN_IXR_RXOK_MASK |
>> XCAN_IXR_RXNEMP_MASK);
>>> + priv->write_reg(priv, XCAN_IER_OFFSET, ier);
>>> + }
>>> + return work_done;
>>> +}
>>> +
>>> +/**
>>> + * xcan_tx_interrupt - Tx Done Isr
>>> + * @ndev: net_device pointer
>>> + */
>>> +static void xcan_tx_interrupt(struct net_device *ndev) {
>>> + unsigned long flags;
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> + struct net_device_stats *stats = &ndev->stats;
>>> + u32 processed = 0, txpackets;
>>> +
>>> + stats->tx_packets++;
>>> + netdev_dbg(ndev, "%s: waiting total:%d,current:%d\n", __func__,
>>> + priv->waiting_ech_skb_num, priv-
>>> waiting_ech_skb_index);
>>> +
>>> + txpackets = priv->waiting_ech_skb_num;
>>> +
>>> + if (txpackets) {
>>> + can_get_echo_skb(ndev, priv->waiting_ech_skb_index);
>>> + priv->waiting_ech_skb_index =
>>> + (priv->waiting_ech_skb_index + 1) %
>>> + priv->xcan_echo_skb_max_tx;
>>> + processed++;
>>> + txpackets--;
>>> + }
>>> +
>>> + spin_lock_irqsave(&priv->ech_skb_lock, flags);
>>> + priv->waiting_ech_skb_num -= processed;
>>> + spin_unlock_irqrestore(&priv->ech_skb_lock, flags);
>>> +
>>> + netdev_dbg(ndev, "%s: waiting total:%d,current:%d\n", __func__,
>>> + priv->waiting_ech_skb_num, priv-
>>> waiting_ech_skb_index);
>>> +
>>> + netif_wake_queue(ndev);
>>> +
>>> + can_led_event(ndev, CAN_LED_EVENT_TX); }
>>> +
>>> +/**
>>> + * xcan_interrupt - CAN Isr
>>> + * @irq: irq number
>>> + * @dev_id: device id poniter
>>> + *
>>> + * This is the xilinx CAN Isr. It checks for the type of interrupt
>>> + * and invokes the corresponding ISR.
>>> + *
>>> + * Return:
>>> + * IRQ_NONE - If CAN device is in sleep mode, IRQ_HANDLED otherwise
>>> +*/ static irqreturn_t xcan_interrupt(int irq, void *dev_id) {
>>> + struct net_device *ndev = (struct net_device *)dev_id;
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> + u32 isr, ier;
>>> +
>>> + if (priv->can.state == CAN_STATE_STOPPED)
>>
>> This should not happen, please remove.
>
> Ok
>>
>>> + return IRQ_NONE;
>>> +
>>> + /* Get the interrupt status from Xilinx CAN */
>>> + isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
>>> + if (!isr)
>>> + return IRQ_NONE;
>>> +
>>> + netdev_dbg(ndev, "%s: isr:#x%08x, err:#x%08x\n", __func__,
>>> + isr, priv->read_reg(priv, XCAN_ESR_OFFSET));
>>> +
>>> + /* Check for the type of interrupt and Processing it */
>>> + if (isr & (XCAN_IXR_SLP_MASK | XCAN_IXR_WKUP_MASK)) {
>>> + priv->write_reg(priv, XCAN_ICR_OFFSET,
>> (XCAN_IXR_SLP_MASK |
>>> + XCAN_IXR_WKUP_MASK));
>>> + xcan_state_interrupt(ndev, isr);
>>> + }
>>> +
>>> + /* Check for Tx interrupt and Processing it */
>>> + if (isr & XCAN_IXR_TXOK_MASK) {
>>> + priv->write_reg(priv, XCAN_ICR_OFFSET,
>> XCAN_IXR_TXOK_MASK);
>>> + xcan_tx_interrupt(ndev);
>>> + }
>>> +
>>> + /* Check for the type of error interrupt and Processing it */
>>> + if (isr & (XCAN_IXR_ERROR_MASK | XCAN_IXR_RXOFLW_MASK |
>>> + XCAN_IXR_BSOFF_MASK |
>> XCAN_IXR_ARBLST_MASK)) {
>>> + priv->write_reg(priv, XCAN_ICR_OFFSET,
>> (XCAN_IXR_ERROR_MASK |
>>> + XCAN_IXR_RXOFLW_MASK |
>> XCAN_IXR_BSOFF_MASK |
>>> + XCAN_IXR_ARBLST_MASK));
>>> + xcan_err_interrupt(ndev, isr);
>>> + }
>>> +
>>> + /* Check for the type of receive interrupt and Processing it */
>>> + if (isr & (XCAN_IXR_RXNEMP_MASK | XCAN_IXR_RXOK_MASK)) {
>>> + ier = priv->read_reg(priv, XCAN_IER_OFFSET);
>>> + ier &= ~(XCAN_IXR_RXNEMP_MASK |
>> XCAN_IXR_RXOK_MASK);
>>> + priv->write_reg(priv, XCAN_IER_OFFSET, ier);
>>> + napi_schedule(&priv->napi);
>>> + }
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +/**
>>> + * xcan_stop - Driver stop routine
>>> + * @ndev: Pointer to net_device structure
>>> + *
>>> + * This is the drivers stop routine. It will disable the
>>> + * interrupts and put the device into configuration mode.
>>> + */
>>> +static void xcan_stop(struct net_device *ndev) {
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> + u32 ier;
>>> +
>>> + /* Disable interrupts and leave the can in configuration mode */
>>> + ier = priv->read_reg(priv, XCAN_IER_OFFSET);
>>> + ier &= ~XCAN_INTR_ALL;
>>> + priv->write_reg(priv, XCAN_IER_OFFSET, ier);
>>> + priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_RESET_MASK);
>>> + priv->can.state = CAN_STATE_STOPPED; }
>>> +
>>> +/**
>>> + * xcan_open - Driver open routine
>>> + * @ndev: Pointer to net_device structure
>>> + *
>>> + * This is the driver open routine.
>>> + * Return: 0 on success and failure value on error */ static int
>>> +xcan_open(struct net_device *ndev) {
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> + int err;
>>> +
>>> + /* Set chip into reset mode */
>>> + err = set_reset_mode(ndev);
>>> + if (err < 0)
>>> + netdev_err(ndev, "mode resetting failed failed!\n");
>>> +
>>> + /* Common open */
>>> + err = open_candev(ndev);
>>> + if (err)
>>> + return err;
>>> +
>>> + err = xcan_start(ndev);
>>> + if (err < 0)
>>> + netdev_err(ndev, "xcan_start failed!\n");
>>> +
>>> +
>>> + can_led_event(ndev, CAN_LED_EVENT_OPEN);
>>> + napi_enable(&priv->napi);
>>> + netif_start_queue(ndev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * xcan_close - Driver close routine
>>> + * @ndev: Pointer to net_device structure
>>> + *
>>> + * Return: 0 always
>>> + */
>>> +static int xcan_close(struct net_device *ndev) {
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> +
>>> + netif_stop_queue(ndev);
>>> + napi_disable(&priv->napi);
>>> + xcan_stop(ndev);
>>> + close_candev(ndev);
>>> +
>>> + can_led_event(ndev, CAN_LED_EVENT_STOP);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * xcan_get_berr_counter - error counter routine
>>> + * @ndev: Pointer to net_device structure
>>> + * @bec: Pointer to can_berr_counter structure
>>> + *
>>> + * This is the driver error counter routine.
>>> + * Return: 0 always
>>> + */
>>> +static int xcan_get_berr_counter(const struct net_device *ndev,
>>> + struct can_berr_counter *bec)
>>> +{
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> +
>>> + bec->txerr = priv->read_reg(priv, XCAN_ECR_OFFSET) &
>> XCAN_ECR_TEC_MASK;
>>> + bec->rxerr = ((priv->read_reg(priv, XCAN_ECR_OFFSET) &
>>> + XCAN_ECR_REC_MASK) >> XCAN_ESR_REC_SHIFT);
>>> + return 0;
>>> +}
>>> +
>>> +static const struct net_device_ops xcan_netdev_ops = {
>>> + .ndo_open = xcan_open,
>>> + .ndo_stop = xcan_close,
>>> + .ndo_start_xmit = xcan_start_xmit,
>>> +};
>>> +
>>> +#ifdef CONFIG_PM_SLEEP
>>> +/**
>>> + * xcan_suspend - Suspend method for the driver
>>> + * @_dev: Address of the platform_device structure
>>> + *
>>> + * Put the driver into low power mode.
>>> + * Return: 0 always
>>> + */
>>> +static int xcan_suspend(struct device *_dev) {
>>> + struct platform_device *pdev = container_of(_dev,
>>> + struct platform_device, dev);
>>> + struct net_device *ndev = platform_get_drvdata(pdev);
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> +
>>> + if (netif_running(ndev)) {
>>> + netif_stop_queue(ndev);
>>> + netif_device_detach(ndev);
>>> + }
>>> +
>>> + priv->write_reg(priv, XCAN_MSR_OFFSET,
>> XCAN_MSR_SLEEP_MASK);
>>> + priv->can.state = CAN_STATE_SLEEPING;
>>> +
>>> + clk_disable(priv->aperclk);
>>> + clk_disable(priv->devclk);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * xcan_resume - Resume from suspend
>>> + * @dev: Address of the platformdevice structure
>>> + *
>>> + * Resume operation after suspend.
>>> + * Return: 0 on success and failure value on error */ static int
>>> +xcan_resume(struct device *dev) {
>>> + struct platform_device *pdev = container_of(dev,
>>> + struct platform_device, dev);
>>> + struct net_device *ndev = platform_get_drvdata(pdev);
>>> + struct xcan_priv *priv = netdev_priv(ndev);
>>> + int ret;
>>> +
>>> + ret = clk_enable(priv->aperclk);
>>> + if (ret) {
>>> + dev_err(dev, "Cannot enable clock.\n");
>>> + return ret;
>>> + }
>>> + ret = clk_enable(priv->devclk);
>>> + if (ret) {
>>> + dev_err(dev, "Cannot enable clock.\n");
>>> + return ret;
>>> + }
>>> +
>>> + priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
>>> + priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_CEN_MASK);
>>> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>> +
>>> + if (netif_running(ndev)) {
>>> + netif_device_attach(ndev);
>>> + netif_start_queue(ndev);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +#endif
>>> +
>>> +static SIMPLE_DEV_PM_OPS(xcan_dev_pm_ops, xcan_suspend,
>> xcan_resume);
>>> +
>>> +/**
>>> + * xcan_probe - Platform registration call
>>> + * @pdev: Handle to the platform device structure
>>> + *
>>> + * This function does all the memory allocation and registration for
>>> +the CAN
>>> + * device.
>>> + *
>>> + * Return: 0 on success and failure value on error */ static int
>>> +xcan_probe(struct platform_device *pdev) {
>>> + struct resource *res; /* IO mem resources */
>>> + struct net_device *ndev;
>>> + struct xcan_priv *priv;
>>> + int ret, fifodep;
>>> +
>>> + /* Create a CAN device instance */
>>> + ndev = alloc_candev(sizeof(struct xcan_priv),
>> XCAN_ECHO_SKB_MAX);
>>> + if (!ndev)
>>> + return -ENOMEM;
>>> +
>>> + priv = netdev_priv(ndev);
>>> + priv->dev = ndev;
>>> + priv->can.bittiming_const = &xcan_bittiming_const;
>>> + priv->can.do_set_bittiming = xcan_set_bittiming;
>>> + priv->can.do_set_mode = xcan_do_set_mode;
>>> + priv->can.do_get_berr_counter = xcan_get_berr_counter;
>>> + priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
>>> + CAN_CTRLMODE_BERR_REPORTING;
>>> + priv->xcan_echo_skb_max_tx = XCAN_ECHO_SKB_MAX;
>>> + priv->xcan_echo_skb_max_rx = XCAN_NAPI_WEIGHT;
>>> +
>>> + /* Get IRQ for the device */
>>> + ndev->irq = platform_get_irq(pdev, 0);
>>> + ret = devm_request_irq(&pdev->dev, ndev->irq, &xcan_interrupt,
>>> + priv->irq_flags, dev_name(&pdev->dev),
>>> + (void *)ndev);
>>
>> We usually request the interrupt on in the open() function
>>
>
>
> Ok Will Move into open routine
This means you cannot use devm_ here and you have to tear down the
interrupt handler in the close function.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]
^ permalink raw reply
* Re: [PATCH v12 2/4] thermal: samsung: change base_common to more meaningful base_second
From: Naveen Krishna Ch @ 2014-02-07 9:35 UTC (permalink / raw)
To: Naveen Krishna Chatradhi
Cc: linux-pm, rui.zhang, eduardo.valentin,
linux-samsung-soc@vger.kernel.org, linux-kernel, amit.daniel,
Kukjin Kim, devicetree, b.zolnierkie, cpgs, t.figa
In-Reply-To: <1387433168-7831-1-git-send-email-ch.naveen@samsung.com>
Hello All,
On 19 December 2013 11:36, Naveen Krishna Chatradhi
<ch.naveen@samsung.com> wrote:
> On Exynos5440 and Exynos5420 there are registers common
> across the TMU channels.
>
> To support that, we introduced a ADDRESS_MULTIPLE flag in the
> driver and the 2nd set of register base and size are provided
> in the "reg" property of the node.
>
> As per Amit's suggestion, this patch changes the base_common
> to base_second and SHARED_MEMORY to ADDRESS_MULTIPLE.
>
> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> Acked-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Reviewed-by: Tomasz Figa <t.figa@samsung.com>
> ---
> Changes since v11:
> Added Reviewed by Tomasz
>
> Changes since v10:
> Documentation rephrased as per comments from Tomasz Figa
>
> .../devicetree/bindings/thermal/exynos-thermal.txt | 4 ++--
> drivers/thermal/samsung/exynos_tmu.c | 14 +++++++-------
> drivers/thermal/samsung/exynos_tmu.h | 4 ++--
> drivers/thermal/samsung/exynos_tmu_data.c | 2 +-
> 4 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
> index 284f530..a1aa602 100644
> --- a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
> @@ -11,8 +11,8 @@
> - reg : Address range of the thermal registers. For soc's which has multiple
> instances of TMU and some registers are shared across all TMU's like
> interrupt related then 2 set of register has to supplied. First set
> - belongs to each instance of TMU and second set belongs to common TMU
> - registers.
> + belongs to register set of TMU instance and second set belongs to
> + registers shared with the TMU instance.
> - interrupts : Should contain interrupt for thermal system
> - clocks : The main clock for TMU device
> - clock-names : Thermal system clock name
> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
> index c493245..bbd0fc3 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -41,7 +41,7 @@
> * @id: identifier of the one instance of the TMU controller.
> * @pdata: pointer to the tmu platform/configuration data
> * @base: base address of the single instance of the TMU controller.
> - * @base_common: base address of the common registers of the TMU controller.
> + * @base_second: base address of the common registers of the TMU controller.
> * @irq: irq number of the TMU controller.
> * @soc: id of the SOC type.
> * @irq_work: pointer to the irq work structure.
> @@ -56,7 +56,7 @@ struct exynos_tmu_data {
> int id;
> struct exynos_tmu_platform_data *pdata;
> void __iomem *base;
> - void __iomem *base_common;
> + void __iomem *base_second;
> int irq;
> enum soc_type soc;
> struct work_struct irq_work;
> @@ -297,7 +297,7 @@ skip_calib_data:
> }
> /*Clear the PMIN in the common TMU register*/
> if (reg->tmu_pmin && !data->id)
> - writel(0, data->base_common + reg->tmu_pmin);
> + writel(0, data->base_second + reg->tmu_pmin);
> out:
> clk_disable(data->clk);
> mutex_unlock(&data->lock);
> @@ -454,7 +454,7 @@ static void exynos_tmu_work(struct work_struct *work)
>
> /* Find which sensor generated this interrupt */
> if (reg->tmu_irqstatus) {
> - val_type = readl(data->base_common + reg->tmu_irqstatus);
> + val_type = readl(data->base_second + reg->tmu_irqstatus);
> if (!((val_type >> data->id) & 0x1))
> goto out;
> }
> @@ -579,7 +579,7 @@ static int exynos_map_dt_data(struct platform_device *pdev)
> * Check if the TMU shares some registers and then try to map the
> * memory of common registers.
> */
> - if (!TMU_SUPPORTS(pdata, SHARED_MEMORY))
> + if (!TMU_SUPPORTS(pdata, ADDRESS_MULTIPLE))
> return 0;
>
> if (of_address_to_resource(pdev->dev.of_node, 1, &res)) {
> @@ -587,9 +587,9 @@ static int exynos_map_dt_data(struct platform_device *pdev)
> return -ENODEV;
> }
>
> - data->base_common = devm_ioremap(&pdev->dev, res.start,
> + data->base_second = devm_ioremap(&pdev->dev, res.start,
> resource_size(&res));
> - if (!data->base_common) {
> + if (!data->base_second) {
> dev_err(&pdev->dev, "Failed to ioremap memory\n");
> return -ENOMEM;
> }
> diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h
> index 980859a..0d6b32f 100644
> --- a/drivers/thermal/samsung/exynos_tmu.h
> +++ b/drivers/thermal/samsung/exynos_tmu.h
> @@ -60,7 +60,7 @@ enum soc_type {
> * state(active/idle) can be checked.
> * TMU_SUPPORT_EMUL_TIME - This features allows to set next temp emulation
> * sample time.
> - * TMU_SUPPORT_SHARED_MEMORY - This feature tells that the different TMU
> + * TMU_SUPPORT_ADDRESS_MULTIPLE - This feature tells that the different TMU
> * sensors shares some common registers.
> * TMU_SUPPORT - macro to compare the above features with the supplied.
> */
> @@ -70,7 +70,7 @@ enum soc_type {
> #define TMU_SUPPORT_FALLING_TRIP BIT(3)
> #define TMU_SUPPORT_READY_STATUS BIT(4)
> #define TMU_SUPPORT_EMUL_TIME BIT(5)
> -#define TMU_SUPPORT_SHARED_MEMORY BIT(6)
> +#define TMU_SUPPORT_ADDRESS_MULTIPLE BIT(6)
>
> #define TMU_SUPPORTS(a, b) (a->features & TMU_SUPPORT_ ## b)
>
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
> index 7cdb04e..1d27069 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.c
> +++ b/drivers/thermal/samsung/exynos_tmu_data.c
> @@ -255,7 +255,7 @@ static const struct exynos_tmu_registers exynos5440_tmu_registers = {
> .type = SOC_ARCH_EXYNOS5440, \
> .registers = &exynos5440_tmu_registers, \
> .features = (TMU_SUPPORT_EMULATION | TMU_SUPPORT_FALLING_TRIP | \
> - TMU_SUPPORT_MULTI_INST | TMU_SUPPORT_SHARED_MEMORY),
> + TMU_SUPPORT_MULTI_INST | TMU_SUPPORT_ADDRESS_MULTIPLE),
>
> struct exynos_tmu_init_data const exynos5440_default_tmu_data = {
> .tmu_data = {
> --
> 1.7.10.4
>
Ping.
--
Shine bright,
(: Nav :)
^ permalink raw reply
* Re: [PATCH v12 3/4] thermal: samsung: Add TMU support for Exynos5420 SoCs
From: Naveen Krishna Ch @ 2014-02-07 9:34 UTC (permalink / raw)
To: Tomasz Figa
Cc: Naveen Krishna Chatradhi, linux-pm-u79uwXL29TY76Z2rM5mHXA,
rui.zhang-ral2JQCrhuEAvxtiuMwx3w, eduardo.valentin-l0cyMroinI0,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
amit.daniel-Sze3O3UU22JBDgjK7y7TUQ, Kukjin Kim,
devicetree-u79uwXL29TY76Z2rM5mHXA,
b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ, cpgs-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <3663519.7JfYNffMTG@amdc1227>
Hello All,
On 19 December 2013 17:04, Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> On Thursday 19 of December 2013 11:36:31 Naveen Krishna Chatradhi wrote:
>> Exynos5420 has 5 TMU channels, the TRIMINFO register is
>> misplaced for TMU channels 2, 3 and 4
>> TRIMINFO at 0x1006c000 contains data for TMU channel 3
>> TRIMINFO at 0x100a0000 contains data for TMU channel 4
>> TRIMINFO at 0x10068000 contains data for TMU channel 2
>>
>> This patch
>> 1 Adds the neccessary register changes and arch information
>> to support Exynos5420 SoCs.
>> 2. Handles the gate clock for misplaced TRIMINFO register
>> 3. Updates the Documentation at
>> Documentation/devicetree/bindings/thermal/exynos-thermal.txt
>>
>> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> Acked-by: Amit Daniel Kachhap <amit.daniel-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---
>> Changes since v11:
>> 1. Added description for clocks in the Documentation
>> 2. corrected the clock name in clk_get() function as per description
>>
>> Changes since v10:
>> 1. using renamed compatible "samsung,exynos5420-tmu-ext-triminfo"
>> and passing same clock as triminfo_apbif clock for channel 2
>> 2. removed the "exynos5420-tmu-triminfo-clk" compatible
>> .../devicetree/bindings/thermal/exynos-thermal.txt | 45 ++++++++-
>> drivers/thermal/samsung/exynos_tmu.c | 52 +++++++++-
>> drivers/thermal/samsung/exynos_tmu.h | 1 +
>> drivers/thermal/samsung/exynos_tmu_data.c | 99 ++++++++++++++++++++
>> drivers/thermal/samsung/exynos_tmu_data.h | 8 ++
>> 5 files changed, 200 insertions(+), 5 deletions(-)
>
> Reviewed-by: Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>
> Best regards,
> Tomasz
>
Ping.
--
Shine bright,
(: Nav :)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v12 1/4] thermal: samsung: replace inten_ bit fields with intclr_
From: Naveen Krishna Ch @ 2014-02-07 9:33 UTC (permalink / raw)
To: Zhang Rui
Cc: Naveen Krishna Chatradhi, linux-pm, eduardo.valentin,
linux-samsung-soc@vger.kernel.org, linux-kernel, amit.daniel,
Kukjin Kim, devicetree, b.zolnierkie, cpgs, t.figa
In-Reply-To: <1388629994.3739.70.camel@rzhang1-mobl4>
Hello All,
On 2 January 2014 08:03, Zhang Rui <rui.zhang@intel.com> wrote:
> On Thu, 2013-12-19 at 11:35 +0530, Naveen Krishna Chatradhi wrote:
>> This patch replaces the inten_rise_shift/mask and inten_fall_shift/mask
>> with intclr_rise_shift/mask and intclr_fall_shift/mask respectively.
>> Currently, inten_rise_shift/mask and inten_fall_shift/mask bits are only used
>> to configure intclr related registers.
>>
>> Description of H/W:
>> The offset for the bits in the CLEAR register are not consistent across TMU
>> modules in Exynso5250, 5420 and 5440.
>>
>> On Exynos5250, the FALL interrupt related en, status and clear bits are
>> available at an offset of
>> 16 in INTEN, INTSTAT registers and at an offset of
>> 12 in INTCLEAR register.
>>
>> On Exynos5420, the FALL interrupt related en, status and clear bits are
>> available at an offset of
>> 16 in INTEN, INTSTAT and INTCLEAR registers.
>>
>> On Exynos5440,
>> the FALL_IRQEN bits are at an offset of 4
>> and the RISE_IRQEN bits are at an offset of 0
>>
>> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
>> Acked-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
>> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>> Reviewed-by: Tomasz Figa <t.figa@samsung.com>
>
> Eduardo,
>
> what do you think of this patch set?
>
> thanks,
> rui
>> ---
>> Changes since v11:
>> Added Reviewed by Tomasz
>>
>> Changes since v10:
>> None
>>
>> drivers/thermal/samsung/exynos_tmu.c | 6 +++---
>> drivers/thermal/samsung/exynos_tmu.h | 16 ++++++++--------
>> drivers/thermal/samsung/exynos_tmu_data.c | 18 +++++++++---------
>> drivers/thermal/samsung/exynos_tmu_data.h | 4 ++--
>> 4 files changed, 22 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
>> index 32f38b9..c493245 100644
>> --- a/drivers/thermal/samsung/exynos_tmu.c
>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>> @@ -237,7 +237,7 @@ skip_calib_data:
>> writeb(pdata->trigger_levels[i], data->base +
>> reg->threshold_th0 + i * sizeof(reg->threshold_th0));
>>
>> - writel(reg->inten_rise_mask, data->base + reg->tmu_intclear);
>> + writel(reg->intclr_rise_mask, data->base + reg->tmu_intclear);
>> } else {
>> /* Write temperature code for rising and falling threshold */
>> for (i = 0;
>> @@ -264,8 +264,8 @@ skip_calib_data:
>> writel(falling_threshold,
>> data->base + reg->threshold_th1);
>>
>> - writel((reg->inten_rise_mask << reg->inten_rise_shift) |
>> - (reg->inten_fall_mask << reg->inten_fall_shift),
>> + writel((reg->intclr_rise_mask << reg->intclr_rise_shift) |
>> + (reg->intclr_fall_mask << reg->intclr_fall_shift),
>> data->base + reg->tmu_intclear);
>>
>> /* if last threshold limit is also present */
>> diff --git a/drivers/thermal/samsung/exynos_tmu.h b/drivers/thermal/samsung/exynos_tmu.h
>> index 3fb6554..980859a 100644
>> --- a/drivers/thermal/samsung/exynos_tmu.h
>> +++ b/drivers/thermal/samsung/exynos_tmu.h
>> @@ -122,10 +122,6 @@ enum soc_type {
>> * @threshold_th3_l0_shift: shift bits of level0 threshold temperature.
>> * @tmu_inten: register containing the different threshold interrupt
>> enable bits.
>> - * @inten_rise_shift: shift bits of all rising interrupt bits.
>> - * @inten_rise_mask: mask bits of all rising interrupt bits.
>> - * @inten_fall_shift: shift bits of all rising interrupt bits.
>> - * @inten_fall_mask: mask bits of all rising interrupt bits.
>> * @inten_rise0_shift: shift bits of rising 0 interrupt bits.
>> * @inten_rise1_shift: shift bits of rising 1 interrupt bits.
>> * @inten_rise2_shift: shift bits of rising 2 interrupt bits.
>> @@ -136,6 +132,10 @@ enum soc_type {
>> * @inten_fall3_shift: shift bits of falling 3 interrupt bits.
>> * @tmu_intstat: Register containing the interrupt status values.
>> * @tmu_intclear: Register for clearing the raised interrupt status.
>> + * @intclr_fall_shift: shift bits for interrupt clear fall 0
>> + * @intclr_rise_shift: shift bits of all rising interrupt bits.
>> + * @intclr_rise_mask: mask bits of all rising interrupt bits.
>> + * @intclr_fall_mask: mask bits of all rising interrupt bits.
>> * @emul_con: TMU emulation controller register.
>> * @emul_temp_shift: shift bits of emulation temperature.
>> * @emul_time_shift: shift bits of emulation time.
>> @@ -191,10 +191,6 @@ struct exynos_tmu_registers {
>> u32 threshold_th3_l0_shift;
>>
>> u32 tmu_inten;
>> - u32 inten_rise_shift;
>> - u32 inten_rise_mask;
>> - u32 inten_fall_shift;
>> - u32 inten_fall_mask;
>> u32 inten_rise0_shift;
>> u32 inten_rise1_shift;
>> u32 inten_rise2_shift;
>> @@ -207,6 +203,10 @@ struct exynos_tmu_registers {
>> u32 tmu_intstat;
>>
>> u32 tmu_intclear;
>> + u32 intclr_fall_shift;
>> + u32 intclr_rise_shift;
>> + u32 intclr_fall_mask;
>> + u32 intclr_rise_mask;
>>
>> u32 emul_con;
>> u32 emul_temp_shift;
>> diff --git a/drivers/thermal/samsung/exynos_tmu_data.c b/drivers/thermal/samsung/exynos_tmu_data.c
>> index 073c292..7cdb04e 100644
>> --- a/drivers/thermal/samsung/exynos_tmu_data.c
>> +++ b/drivers/thermal/samsung/exynos_tmu_data.c
>> @@ -40,13 +40,13 @@ static const struct exynos_tmu_registers exynos4210_tmu_registers = {
>> .threshold_temp = EXYNOS4210_TMU_REG_THRESHOLD_TEMP,
>> .threshold_th0 = EXYNOS4210_TMU_REG_TRIG_LEVEL0,
>> .tmu_inten = EXYNOS_TMU_REG_INTEN,
>> - .inten_rise_mask = EXYNOS4210_TMU_TRIG_LEVEL_MASK,
>> .inten_rise0_shift = EXYNOS_TMU_INTEN_RISE0_SHIFT,
>> .inten_rise1_shift = EXYNOS_TMU_INTEN_RISE1_SHIFT,
>> .inten_rise2_shift = EXYNOS_TMU_INTEN_RISE2_SHIFT,
>> .inten_rise3_shift = EXYNOS_TMU_INTEN_RISE3_SHIFT,
>> .tmu_intstat = EXYNOS_TMU_REG_INTSTAT,
>> .tmu_intclear = EXYNOS_TMU_REG_INTCLEAR,
>> + .intclr_rise_mask = EXYNOS4210_TMU_TRIG_LEVEL_MASK,
>> };
>>
>> struct exynos_tmu_init_data const exynos4210_default_tmu_data = {
>> @@ -112,10 +112,6 @@ static const struct exynos_tmu_registers exynos4412_tmu_registers = {
>> .threshold_th0 = EXYNOS_THD_TEMP_RISE,
>> .threshold_th1 = EXYNOS_THD_TEMP_FALL,
>> .tmu_inten = EXYNOS_TMU_REG_INTEN,
>> - .inten_rise_mask = EXYNOS_TMU_RISE_INT_MASK,
>> - .inten_rise_shift = EXYNOS_TMU_RISE_INT_SHIFT,
>> - .inten_fall_mask = EXYNOS_TMU_FALL_INT_MASK,
>> - .inten_fall_shift = EXYNOS_TMU_FALL_INT_SHIFT,
>> .inten_rise0_shift = EXYNOS_TMU_INTEN_RISE0_SHIFT,
>> .inten_rise1_shift = EXYNOS_TMU_INTEN_RISE1_SHIFT,
>> .inten_rise2_shift = EXYNOS_TMU_INTEN_RISE2_SHIFT,
>> @@ -123,6 +119,10 @@ static const struct exynos_tmu_registers exynos4412_tmu_registers = {
>> .inten_fall0_shift = EXYNOS_TMU_INTEN_FALL0_SHIFT,
>> .tmu_intstat = EXYNOS_TMU_REG_INTSTAT,
>> .tmu_intclear = EXYNOS_TMU_REG_INTCLEAR,
>> + .intclr_fall_shift = EXYNOS_TMU_CLEAR_FALL_INT_SHIFT,
>> + .intclr_rise_shift = EXYNOS_TMU_RISE_INT_SHIFT,
>> + .intclr_rise_mask = EXYNOS_TMU_RISE_INT_MASK,
>> + .intclr_fall_mask = EXYNOS_TMU_FALL_INT_MASK,
>> .emul_con = EXYNOS_EMUL_CON,
>> .emul_temp_shift = EXYNOS_EMUL_DATA_SHIFT,
>> .emul_time_shift = EXYNOS_EMUL_TIME_SHIFT,
>> @@ -217,10 +217,6 @@ static const struct exynos_tmu_registers exynos5440_tmu_registers = {
>> .threshold_th2 = EXYNOS5440_TMU_S0_7_TH2,
>> .threshold_th3_l0_shift = EXYNOS5440_TMU_TH_RISE4_SHIFT,
>> .tmu_inten = EXYNOS5440_TMU_S0_7_IRQEN,
>> - .inten_rise_mask = EXYNOS5440_TMU_RISE_INT_MASK,
>> - .inten_rise_shift = EXYNOS5440_TMU_RISE_INT_SHIFT,
>> - .inten_fall_mask = EXYNOS5440_TMU_FALL_INT_MASK,
>> - .inten_fall_shift = EXYNOS5440_TMU_FALL_INT_SHIFT,
>> .inten_rise0_shift = EXYNOS5440_TMU_INTEN_RISE0_SHIFT,
>> .inten_rise1_shift = EXYNOS5440_TMU_INTEN_RISE1_SHIFT,
>> .inten_rise2_shift = EXYNOS5440_TMU_INTEN_RISE2_SHIFT,
>> @@ -228,6 +224,10 @@ static const struct exynos_tmu_registers exynos5440_tmu_registers = {
>> .inten_fall0_shift = EXYNOS5440_TMU_INTEN_FALL0_SHIFT,
>> .tmu_intstat = EXYNOS5440_TMU_S0_7_IRQ,
>> .tmu_intclear = EXYNOS5440_TMU_S0_7_IRQ,
>> + .intclr_fall_shift = EXYNOS5440_TMU_CLEAR_FALL_INT_SHIFT,
>> + .intclr_rise_shift = EXYNOS5440_TMU_RISE_INT_SHIFT,
>> + .intclr_rise_mask = EXYNOS5440_TMU_RISE_INT_MASK,
>> + .intclr_fall_mask = EXYNOS5440_TMU_FALL_INT_MASK,
>> .tmu_irqstatus = EXYNOS5440_TMU_IRQ_STATUS,
>> .emul_con = EXYNOS5440_TMU_S0_7_DEBUG,
>> .emul_temp_shift = EXYNOS_EMUL_DATA_SHIFT,
>> diff --git a/drivers/thermal/samsung/exynos_tmu_data.h b/drivers/thermal/samsung/exynos_tmu_data.h
>> index a1ea19d..d9495a4 100644
>> --- a/drivers/thermal/samsung/exynos_tmu_data.h
>> +++ b/drivers/thermal/samsung/exynos_tmu_data.h
>> @@ -69,9 +69,10 @@
>> #define EXYNOS_TMU_RISE_INT_MASK 0x111
>> #define EXYNOS_TMU_RISE_INT_SHIFT 0
>> #define EXYNOS_TMU_FALL_INT_MASK 0x111
>> -#define EXYNOS_TMU_FALL_INT_SHIFT 12
>> #define EXYNOS_TMU_CLEAR_RISE_INT 0x111
>> #define EXYNOS_TMU_CLEAR_FALL_INT (0x111 << 12)
>> +#define EXYNOS_TMU_CLEAR_FALL_INT_SHIFT 12
>> +#define EXYNOS5440_TMU_CLEAR_FALL_INT_SHIFT 4
>> #define EXYNOS_TMU_TRIP_MODE_SHIFT 13
>> #define EXYNOS_TMU_TRIP_MODE_MASK 0x7
>> #define EXYNOS_TMU_THERM_TRIP_EN_SHIFT 12
>> @@ -119,7 +120,6 @@
>> #define EXYNOS5440_TMU_RISE_INT_MASK 0xf
>> #define EXYNOS5440_TMU_RISE_INT_SHIFT 0
>> #define EXYNOS5440_TMU_FALL_INT_MASK 0xf
>> -#define EXYNOS5440_TMU_FALL_INT_SHIFT 4
>> #define EXYNOS5440_TMU_INTEN_RISE0_SHIFT 0
>> #define EXYNOS5440_TMU_INTEN_RISE1_SHIFT 1
>> #define EXYNOS5440_TMU_INTEN_RISE2_SHIFT 2
>
>
Ping.
--
Shine bright,
(: Nav :)
^ permalink raw reply
* Re: [PATCH] thermal: exynos: handle gate clock for misplaced TRIMINFO register
From: Naveen Krishna Ch @ 2014-02-07 9:33 UTC (permalink / raw)
To: Zhang Rui
Cc: Naveen Krishna Chatradhi, linux-pm-u79uwXL29TY76Z2rM5mHXA,
eduardo.valentin-l0cyMroinI0,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
amit.daniel-Sze3O3UU22JBDgjK7y7TUQ, Kukjin Kim,
devicetree-u79uwXL29TY76Z2rM5mHXA,
b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ, cpgs-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <1388642820.3739.80.camel@rzhang1-mobl4>
Hello All,
On 2 January 2014 11:37, Zhang Rui <rui.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, 2013-11-07 at 18:12 +0530, Naveen Krishna Chatradhi wrote:
>> On Exynos5420 the TMU(4) for GPU has a seperate clock enable bit from
>> the other TMU channels(0 ~ 3). Hence, accessing TRIMINFO for base_second
>> should be acompanied by enabling the respective clock.
>>
>> This patch which allow for a "clk_sec" clock to be specified in the
>> device-tree which will be ungated when accessing the TRIMINFO register.
>>
>> Signed-off-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>
> Eduardo,
>
> what do you think of this patch?
>
> thanks,
> rui
>> ---
>> drivers/thermal/samsung/exynos_tmu.c | 24 +++++++++++++++++++++++-
>> 1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/samsung/exynos_tmu.c
>> index b54825a..33edd1a 100644
>> --- a/drivers/thermal/samsung/exynos_tmu.c
>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>> @@ -47,6 +47,7 @@
>> * @irq_work: pointer to the irq work structure.
>> * @lock: lock to implement synchronization.
>> * @clk: pointer to the clock structure.
>> + * @clk_sec: pointer to the clock structure for accessing the base_second.
>> * @temp_error1: fused value of the first point trim.
>> * @temp_error2: fused value of the second point trim.
>> * @regulator: pointer to the TMU regulator structure.
>> @@ -61,7 +62,7 @@ struct exynos_tmu_data {
>> enum soc_type soc;
>> struct work_struct irq_work;
>> struct mutex lock;
>> - struct clk *clk;
>> + struct clk *clk, *clk_sec;
>> u8 temp_error1, temp_error2;
>> struct regulator *regulator;
>> struct thermal_sensor_conf *reg_conf;
>> @@ -152,6 +153,8 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
>>
>> mutex_lock(&data->lock);
>> clk_enable(data->clk);
>> + if (!IS_ERR(data->clk_sec))
>> + clk_enable(data->clk_sec);
>>
>> if (TMU_SUPPORTS(pdata, READY_STATUS)) {
>> status = readb(data->base + reg->tmu_status);
>> @@ -306,6 +309,8 @@ skip_calib_data:
>> out:
>> clk_disable(data->clk);
>> mutex_unlock(&data->lock);
>> + if (!IS_ERR(data->clk_sec))
>> + clk_disable(data->clk_sec);
>>
>> return ret;
>> }
>> @@ -457,12 +462,16 @@ static void exynos_tmu_work(struct work_struct *work)
>> const struct exynos_tmu_registers *reg = pdata->registers;
>> unsigned int val_irq, val_type;
>>
>> + if (!IS_ERR(data->clk_sec))
>> + clk_enable(data->clk_sec);
>> /* Find which sensor generated this interrupt */
>> if (reg->tmu_irqstatus) {
>> val_type = readl(data->base_second + reg->tmu_irqstatus);
>> if (!((val_type >> data->id) & 0x1))
>> goto out;
>> }
>> + if (!IS_ERR(data->clk_sec))
>> + clk_disable(data->clk_sec);
>>
>> exynos_report_trigger(data->reg_conf);
>> mutex_lock(&data->lock);
>> @@ -641,6 +650,15 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>> if (ret)
>> return ret;
>>
>> + data->clk_sec = devm_clk_get(&pdev->dev, "tmu_apbif_sec");
>> + if (!IS_ERR(data->clk_sec)) {
>> + ret = clk_prepare(data->clk_sec);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Failed to get clock\n");
>> + return PTR_ERR(data->clk_sec);
>> + }
>> + }
>> +
>> if (pdata->type == SOC_ARCH_EXYNOS4210 ||
>> pdata->type == SOC_ARCH_EXYNOS4412 ||
>> pdata->type == SOC_ARCH_EXYNOS5250 ||
>> @@ -713,6 +731,8 @@ static int exynos_tmu_probe(struct platform_device *pdev)
>> return 0;
>> err_clk:
>> clk_unprepare(data->clk);
>> + if (!IS_ERR(data->clk_sec))
>> + clk_unprepare(data->clk_sec);
>> return ret;
>> }
>>
>> @@ -725,6 +745,8 @@ static int exynos_tmu_remove(struct platform_device *pdev)
>> exynos_unregister_thermal(data->reg_conf);
>>
>> clk_unprepare(data->clk);
>> + if (!IS_ERR(data->clk_sec))
>> + clk_unprepare(data->clk_sec);
>>
>> if (!IS_ERR(data->regulator))
>> regulator_disable(data->regulator);
>
>
Ping.
--
Shine bright,
(: Nav :)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] ARM: dts: qcom: Add RNG device tree node
From: Stanimir Varbanov @ 2014-02-07 9:23 UTC (permalink / raw)
To: devicetree, linux-arm-kernel, linux-arm-msm
Cc: Kumar Gala, Mark Rutland, Rob Herring, Pawel Moll, Ian Campbell,
David Brown, Stephen Boyd, Stanimir Varbanov
Add the necessary DT node to probe the rng driver on
msm8974 platforms.
Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
---
arch/arm/boot/dts/qcom-msm8974.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 9e5dadb101eb..ada0e8216c22 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -117,5 +117,12 @@
clocks = <&gcc GCC_BLSP1_UART2_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>;
clock-names = "core", "iface";
};
+
+ rng@f9bff000 {
+ compatible = "qcom,prng";
+ reg = <0xf9bff000 0x200>;
+ clocks = <&gcc GCC_PRNG_AHB_CLK>;
+ clock-names = "core";
+ };
};
};
--
1.8.4.4
^ permalink raw reply related
* RE: [PATCH] can: xilinx CAN controller support.
From: Appana Durga Kedareswara Rao @ 2014-02-07 8:42 UTC (permalink / raw)
To: Marc Kleine-Budde, wg@grandegger.com, Michal Simek,
grant.likely@linaro.org, robh+dt@kernel.org,
linux-can@vger.kernel.org
Cc: netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <52F382D5.6090706@pengutronix.de>
Hi Marc,
> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
> Sent: Thursday, February 06, 2014 6:11 PM
> To: Appana Durga Kedareswara Rao; wg@grandegger.com; Michal Simek;
> grant.likely@linaro.org; robh+dt@kernel.org; linux-can@vger.kernel.org
> Cc: netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org; Appana Durga
> Kedareswara Rao
> Subject: Re: [PATCH] can: xilinx CAN controller support.
>
> On 02/06/2014 11:19 AM, Kedareswara rao Appana wrote:
> > This patch adds xilinx CAN controller support.
> > This driver supports both ZYNQ CANPS IP and Soft IP AXI CAN
> > controller.
> >
> > Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
>
> First review, see comments inline.
>
> Marc
>
> > ---
> > This patch is rebased on the 3.14 rc1 kernel.
> > ---
> > .../devicetree/bindings/net/can/xilinx_can.txt | 43 +
> > drivers/net/can/Kconfig | 8 +
> > drivers/net/can/Makefile | 1 +
> > drivers/net/can/xilinx_can.c | 1150 ++++++++++++++++++++
> > 4 files changed, 1202 insertions(+), 0 deletions(-) create mode
> > 100644 Documentation/devicetree/bindings/net/can/xilinx_can.txt
> > create mode 100644 drivers/net/can/xilinx_can.c
> >
> > diff --git a/Documentation/devicetree/bindings/net/can/xilinx_can.txt
> > b/Documentation/devicetree/bindings/net/can/xilinx_can.txt
> > new file mode 100644
> > index 0000000..34f9643
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/can/xilinx_can.txt
> > @@ -0,0 +1,43 @@
> > +Xilinx Axi CAN/Zynq CANPS controller Device Tree Bindings
> > +---------------------------------------------------------
> > +
> > +Required properties:
> > +- compatible : Should be "xlnx,zynq-can-1.00.a" for Zynq
> CAN
> > + controllers and "xlnx,axi-can-1.00.a" for Axi CAN
> > + controllers.
> > +- reg : Physical base address and size of the Axi CAN/Zynq
> > + CANPS registers map.
> > +- interrupts : Property with a value describing the interrupt
> > + number.
> > +- interrupt-parent : Must be core interrupt controller
> > +- clock-names : List of input clock names - "ref_clk",
> "aper_clk"
> > + (See clock bindings for details. Two clocks are
> > + required for Zynq CAN. For Axi CAN
> > + case it is one(ref_clk)).
> > +- clocks : Clock phandles (see clock bindings for details).
> > +- xlnx,can-tx-dpth : Can Tx fifo depth (Required for Axi CAN).
> > +- xlnx,can-rx-dpth : Can Rx fifo depth (Required for Axi CAN).
> > +
> > +
> > +Example:
> > +
> > +For Zynq CANPS Dts file:
> > + zynq_can_0: zynq-can@e0008000 {
> > + compatible = "xlnx,zynq-can-1.00.a";
> > + clocks = <&clkc 19>, <&clkc 36>;
> > + clock-names = "ref_clk", "aper_clk";
> > + reg = <0xe0008000 0x1000>;
> > + interrupts = <0 28 4>;
> > + interrupt-parent = <&intc>;
>
> Above xlnx,can-{rx,tx}-dpth is mentioned as required, but it's not in the
> Zynq example.
One of the Difference b/w the AXI CAN and zynq CAN is in AXI CAN the fifo depth(tx,rx)
Is user configurable. But in case of ZYNQ CAN the fifo depth is fixed for tx and rx fifo's(64)
Xlnx,can-{rx,tx}-dpth is required only for AXI CAN case it is not required for zynq CAN.
That's why didn't putted that property in device tree.
>
> > + };
> > +For Axi CAN Dts file:
> > + axi_can_0: axi-can@40000000 {
> > + compatible = "xlnx,axi-can-1.00.a";
> > + clocks = <&clkc 0>;
> > + clock-names = "ref_clk" ;
> > + reg = <0x40000000 0x10000>;
> > + interrupt-parent = <&intc>;
> > + interrupts = <0 59 1>;
> > + xlnx,can-tx-dpth = <0x40>;
> > + xlnx,can-rx-dpth = <0x40>;
> > + };
> > diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig index
> > d447b88..2344fb5 100644
> > --- a/drivers/net/can/Kconfig
> > +++ b/drivers/net/can/Kconfig
> > @@ -125,6 +125,14 @@ config CAN_GRCAN
> > endian syntheses of the cores would need some modifications on
> > the hardware level to work.
> >
> > +config CAN_XILINXCAN
> > + tristate "Xilinx CAN"
> > + depends on ARCH_ZYNQ || MICROBLAZE
> > + default n
>
> "default n" is default, please remove.
>
Ok
> > + ---help---
> > + Xilinx CAN driver. This driver supports both soft AXI CAN IP and
> > + Zynq CANPS IP.
> > +
> > source "drivers/net/can/mscan/Kconfig"
> >
> > source "drivers/net/can/sja1000/Kconfig"
> > diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile index
> > c744039..0b8e11e 100644
> > --- a/drivers/net/can/Makefile
> > +++ b/drivers/net/can/Makefile
> > @@ -25,5 +25,6 @@ obj-$(CONFIG_CAN_JANZ_ICAN3) += janz-
> ican3.o
> > obj-$(CONFIG_CAN_FLEXCAN) += flexcan.o
> > obj-$(CONFIG_PCH_CAN) += pch_can.o
> > obj-$(CONFIG_CAN_GRCAN) += grcan.o
> > +obj-$(CONFIG_CAN_XILINXCAN) += xilinx_can.o
> >
> > ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG diff --git
> > a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c new file
> > mode 100644 index 0000000..c1b2b9d
> > --- /dev/null
> > +++ b/drivers/net/can/xilinx_can.c
> > @@ -0,0 +1,1150 @@
> > +/* Xilinx CAN device driver
> > + *
> > + * Copyright (C) 2012 - 2014 Xilinx, Inc.
> > + * Copyright (C) 2009 PetaLogix. All rights reserved.
> > + *
> > + * Description:
> > + * This driver is developed for Axi CAN IP and for Zynq CANPS Controller.
> > + * 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.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/errno.h>
> > +#include <linux/init.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/netdevice.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/skbuff.h>
> > +#include <linux/string.h>
> > +#include <linux/types.h>
> > +#include <linux/can/dev.h>
> > +#include <linux/can/error.h>
> > +#include <linux/can/led.h>
> > +
> > +#define DRIVER_NAME "XILINX_CAN"
> > +
> > +/* CAN registers set */
> > +#define XCAN_SRR_OFFSET 0x00 /* Software reset */
> > +#define XCAN_MSR_OFFSET 0x04 /* Mode select */
> > +#define XCAN_BRPR_OFFSET 0x08 /* Baud rate prescaler */
> > +#define XCAN_BTR_OFFSET 0x0C /* Bit timing */
> > +#define XCAN_ECR_OFFSET 0x10 /* Error counter */
> > +#define XCAN_ESR_OFFSET 0x14 /* Error status */
> > +#define XCAN_SR_OFFSET 0x18 /* Status */
> > +#define XCAN_ISR_OFFSET 0x1C /* Interrupt status */
> > +#define XCAN_IER_OFFSET 0x20 /* Interrupt enable */
> > +#define XCAN_ICR_OFFSET 0x24 /* Interrupt clear */
> > +#define XCAN_TXFIFO_ID_OFFSET 0x30 /* TX FIFO ID */
> > +#define XCAN_TXFIFO_DLC_OFFSET 0x34 /* TX FIFO DLC */
> > +#define XCAN_TXFIFO_DW1_OFFSET 0x38 /* TX FIFO Data
> Word 1 */
> > +#define XCAN_TXFIFO_DW2_OFFSET 0x3C /* TX FIFO Data
> Word 2 */
> > +#define XCAN_RXFIFO_ID_OFFSET 0x50 /* RX FIFO ID */
> > +#define XCAN_RXFIFO_DLC_OFFSET 0x54 /* RX FIFO DLC */
> > +#define XCAN_RXFIFO_DW1_OFFSET 0x58 /* RX FIFO Data
> Word 1 */
> > +#define XCAN_RXFIFO_DW2_OFFSET 0x5C /* RX FIFO Data
> Word 2 */
> > +
> > +/* CAN register bit masks - XCAN_<REG>_<BIT>_MASK */
> > +#define XCAN_SRR_CEN_MASK 0x00000002 /* CAN enable */
> > +#define XCAN_SRR_RESET_MASK 0x00000001 /* Soft Reset the
> CAN core */
> > +#define XCAN_MSR_LBACK_MASK 0x00000002 /* Loop back
> mode select */
> > +#define XCAN_MSR_SLEEP_MASK 0x00000001 /* Sleep mode
> select */
> > +#define XCAN_BRPR_BRP_MASK 0x000000FF /* Baud rate
> prescaler */
> > +#define XCAN_BTR_SJW_MASK 0x00000180 /* Synchronous
> jump width */
> > +#define XCAN_BTR_TS2_MASK 0x00000070 /* Time segment
> 2 */
> > +#define XCAN_BTR_TS1_MASK 0x0000000F /* Time segment
> 1 */
> > +#define XCAN_ECR_REC_MASK 0x0000FF00 /* Receive error
> counter */
> > +#define XCAN_ECR_TEC_MASK 0x000000FF /* Transmit error
> counter */
> > +#define XCAN_ESR_ACKER_MASK 0x00000010 /* ACK error */
> > +#define XCAN_ESR_BERR_MASK 0x00000008 /* Bit error */
> > +#define XCAN_ESR_STER_MASK 0x00000004 /* Stuff error */
> > +#define XCAN_ESR_FMER_MASK 0x00000002 /* Form error */
> > +#define XCAN_ESR_CRCER_MASK 0x00000001 /* CRC error */
> > +#define XCAN_SR_TXFLL_MASK 0x00000400 /* TX FIFO is full
> */
> > +#define XCAN_SR_ESTAT_MASK 0x00000180 /* Error status */
> > +#define XCAN_SR_ERRWRN_MASK 0x00000040 /* Error warning
> */
> > +#define XCAN_SR_NORMAL_MASK 0x00000008 /* Normal mode
> */
> > +#define XCAN_SR_LBACK_MASK 0x00000002 /* Loop back
> mode */
> > +#define XCAN_SR_CONFIG_MASK 0x00000001 /* Configuration
> mode */
> > +#define XCAN_IXR_TXFEMP_MASK 0x00004000 /* TX FIFO Empty
> */
> > +#define XCAN_IXR_WKUP_MASK 0x00000800 /* Wake up
> interrupt */
> > +#define XCAN_IXR_SLP_MASK 0x00000400 /* Sleep
> interrupt */
> > +#define XCAN_IXR_BSOFF_MASK 0x00000200 /* Bus off
> interrupt */
> > +#define XCAN_IXR_ERROR_MASK 0x00000100 /* Error interrupt
> */
> > +#define XCAN_IXR_RXNEMP_MASK 0x00000080 /* RX FIFO
> NotEmpty intr */
> > +#define XCAN_IXR_RXOFLW_MASK 0x00000040 /* RX FIFO
> Overflow intr */
> > +#define XCAN_IXR_RXOK_MASK 0x00000010 /* Message
> received intr */
> > +#define XCAN_IXR_TXOK_MASK 0x00000002 /* TX successful
> intr */
> > +#define XCAN_IXR_ARBLST_MASK 0x00000001 /* Arbitration
> lost intr */
> > +#define XCAN_IDR_ID1_MASK 0xFFE00000 /* Standard msg
> identifier */
> > +#define XCAN_IDR_SRR_MASK 0x00100000 /* Substitute
> remote TXreq */
> > +#define XCAN_IDR_IDE_MASK 0x00080000 /* Identifier
> extension */
> > +#define XCAN_IDR_ID2_MASK 0x0007FFFE /* Extended
> message ident */
> > +#define XCAN_IDR_RTR_MASK 0x00000001 /* Remote TX
> request */
> > +#define XCAN_DLCR_DLC_MASK 0xF0000000 /* Data length
> code */
> > +
> > +#define XCAN_INTR_ALL (XCAN_IXR_TXOK_MASK |
> XCAN_IXR_BSOFF_MASK |\
> > + XCAN_IXR_WKUP_MASK |
> XCAN_IXR_SLP_MASK | \
> > + XCAN_IXR_RXNEMP_MASK |
> XCAN_IXR_ERROR_MASK | \
> > + XCAN_IXR_ARBLST_MASK |
> XCAN_IXR_RXOK_MASK)
> > +
> > +/* CAN register bit shift - XCAN_<REG>_<BIT>_SHIFT */
> > +#define XCAN_BTR_SJW_SHIFT 7 /* Synchronous jump width
> */
> > +#define XCAN_BTR_TS2_SHIFT 4 /* Time segment 2 */
> > +#define XCAN_IDR_ID1_SHIFT 21 /* Standard Messg
> Identifier */
> > +#define XCAN_IDR_ID2_SHIFT 1 /* Extended Message
> Identifier */
> > +#define XCAN_DLCR_DLC_SHIFT 28 /* Data length code */
> > +#define XCAN_ESR_REC_SHIFT 8 /* Rx Error Count */
> > +
> > +/* CAN frame length constants */
> > +#define XCAN_ECHO_SKB_MAX 64
> > +#define XCAN_NAPI_WEIGHT 64
> > +#define XCAN_FRAME_MAX_DATA_LEN 8
> > +#define XCAN_TIMEOUT (50 * HZ)
> > +
> > +/**
> > + * struct xcan_priv - This definition define CAN driver instance
> > + * @can: CAN private data structure.
> > + * @open_time: For holding timeout values
> > + * @waiting_ech_skb_index: Pointer for skb
> > + * @ech_skb_next: This tell the next packet in the queue
> > + * @waiting_ech_skb_num: Gives the number of packets waiting
> > + * @xcan_echo_skb_max_tx: Maximum number packets the driver
> can send
> > + * @xcan_echo_skb_max_rx: Maximum number packets the driver
> can receive
> > + * @napi: NAPI structure
> > + * @ech_skb_lock: For spinlock purpose
> > + * @read_reg: For reading data from CAN registers
> > + * @write_reg: For writing data to CAN registers
> > + * @dev: Network device data structure
> > + * @reg_base: Ioremapped address to registers
> > + * @irq_flags: For request_irq()
> > + * @aperclk: Pointer to struct clk
> > + * @devclk: Pointer to struct clk
> > + */
> > +struct xcan_priv {
> > + struct can_priv can;
> > + int open_time;
> > + int waiting_ech_skb_index;
> > + int ech_skb_next;
> > + int waiting_ech_skb_num;
> > + int xcan_echo_skb_max_tx;
> > + int xcan_echo_skb_max_rx;
> > + struct napi_struct napi;
> > + spinlock_t ech_skb_lock;
> > + u32 (*read_reg)(const struct xcan_priv *priv, int reg);
> > + void (*write_reg)(const struct xcan_priv *priv, int reg, u32 val);
>
> Why do you have {read,write}_reg function here?
xcan_write_reg/xcan_read_reg is used because this IP is also in big endian version.
We won't support It but if someone else want to support that's why putted these function's here.
If you are not ok with this will remove.
>
> > + struct net_device *dev;
> > + void __iomem *reg_base;
> > + unsigned long irq_flags;
> > + struct clk *aperclk;
> > + struct clk *devclk;
> > +};
> > +
> > +/* CAN Bittiming constants as per Xilinx CAN specs */ static struct
> > +can_bittiming_const xcan_bittiming_const = {
> > + .name = DRIVER_NAME,
> > + .tseg1_min = 1,
> > + .tseg1_max = 16,
> > + .tseg2_min = 1,
> > + .tseg2_max = 8,
> > + .sjw_max = 4,
> > + .brp_min = 1,
> > + .brp_max = 256,
> > + .brp_inc = 1,
> > +};
> > +
> > +/**
> > + * xcan_write_reg - Write a value to the device register
> > + * @priv: Driver private data structure
> > + * @reg: Register offset
> > + * @val: Value to write at the Register offset
> > + *
> > + * Write data to the paricular CAN register */ static void
> > +xcan_write_reg(const struct xcan_priv *priv, int reg, u32 val) {
> > + writel(val, priv->reg_base + reg);
> > +}
> > +
> > +/**
> > + * xcan_read_reg - Read a value from the device register
> > + * @priv: Driver private data structure
> > + * @reg: Register offset
> > + *
> > + * Read data from the particular CAN register
> > + * Return: value read from the CAN register */ static u32
> > +xcan_read_reg(const struct xcan_priv *priv, int reg) {
> > + return readl(priv->reg_base + reg);
> > +}
> > +
> > +/**
> > + * set_reset_mode - Resets the CAN device mode
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * This is the driver reset mode routine.The driver
> > + * enters into configuration mode.
> > + *
> > + * Return: 0 on success and failure value on error */ static int
> > +set_reset_mode(struct net_device *ndev) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + unsigned long timeout;
> > +
> > + priv->can.state = CAN_STATE_STOPPED;
> > + priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_OFFSET);
> > +
> > + timeout = jiffies + XCAN_TIMEOUT;
> > + while (!(priv->read_reg(priv, XCAN_SR_OFFSET) &
> XCAN_SR_CONFIG_MASK)) {
> > + if (time_after(jiffies, timeout)) {
> > + netdev_warn(ndev, "timedout waiting for config
> mode\n");
> > + return -ETIMEDOUT;
> > + }
> > + schedule_timeout(1);
>
> better use usleep_range()
>
Ok
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcan_set_bittiming - CAN set bit timing routine
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * This is the driver set bittiming routine.
> > + * Return: 0 on success and failure value on error */ static int
> > +xcan_set_bittiming(struct net_device *ndev) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + struct can_bittiming *bt = &priv->can.bittiming;
> > + u32 btr0, btr1;
> > + u32 is_config_mode;
> > +
> > + /* Check whether Xilinx CAN is in configuration mode.
> > + * It cannot set bit timing if Xilinx CAN is not in configuration mode.
> > + */
> > + is_config_mode = priv->read_reg(priv, XCAN_SR_OFFSET) &
> > + XCAN_SR_CONFIG_MASK;
> > + if (!is_config_mode) {
> > + netdev_alert(ndev,
> > + "Cannot set bittiming can is not in config mode\n");
> > + return -EPERM;
> > + }
> > +
> > + netdev_dbg(ndev,
> "brp=%d,prop=%d,phase_seg1:%d,phase_reg2=%d,sjw=%d\n",
> > + bt->brp, bt->prop_seg, bt->phase_seg1, bt-
> >phase_seg2,
> > + bt->sjw);
>
> I think this dbg can be removed, as it just prints the userspace values.
>
Ok
> > +
> > + /* Setting Baud Rate prescalar value in BRPR Register */
> > + btr0 = (bt->brp - 1) & XCAN_BRPR_BRP_MASK;
> > +
> > + /* Setting Time Segment 1 in BTR Register */
> > + btr1 = (bt->prop_seg + bt->phase_seg1 - 1) & XCAN_BTR_TS1_MASK;
> > +
> > + /* Setting Time Segment 2 in BTR Register */
> > + btr1 |= ((bt->phase_seg2 - 1) << XCAN_BTR_TS2_SHIFT) &
> > + XCAN_BTR_TS2_MASK;
> > +
> > + /* Setting Synchronous jump width in BTR Register */
> > + btr1 |= ((bt->sjw - 1) << XCAN_BTR_SJW_SHIFT) &
> XCAN_BTR_SJW_MASK;
> > +
> > + if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
> > + netdev_info(ndev, "Doesn't support Triple Sampling\n");
>
> no need to check, it's not passed to the driver until you advertise you
> support it (see priv->can.ctrlmode_supported).
Ok
>
> > + netdev_dbg(ndev, "Setting BTR0=0x%02x BTR1=0x%02x\n", btr0,
> btr1);
> > +
> > + priv->write_reg(priv, XCAN_BRPR_OFFSET, btr0);
> > + priv->write_reg(priv, XCAN_BTR_OFFSET, btr1);
> > +
> > + netdev_dbg(ndev, "BRPR=0x%08x, BTR=0x%08x\n",
> > + priv->read_reg(priv, XCAN_BRPR_OFFSET),
> > + priv->read_reg(priv, XCAN_BTR_OFFSET));
>
> One of the dbg should be enough.
>
Ok
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcan_start - This the drivers start routine
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * This is the drivers start routine.
> > + * Based on the State of the CAN device it puts
> > + * the CAN device into a proper mode.
> > + *
> > + * Return: 0 always
> > + */
> > +static int xcan_start(struct net_device *ndev) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > +
> > + /* Check if it is in reset mode */
> > + if (priv->can.state != CAN_STATE_STOPPED)
> > + set_reset_mode(ndev);
>
> Please check return value of set_reset_mode
Ok
>
> > +
> > + /* Enable interrupts */
> > + priv->write_reg(priv, XCAN_IER_OFFSET, XCAN_INTR_ALL);
> > +
> > + /* Check whether it is loopback mode or normal mode */
> > + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
> > + /* Put device into loopback mode */
> > + priv->write_reg(priv, XCAN_MSR_OFFSET,
> XCAN_MSR_LBACK_MASK);
> > + else
> > + /* The device is in normal mode */
> > + priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
> > +
> > + if (priv->can.state == CAN_STATE_STOPPED) {
>
> I think your driver is always in CAN_STATE_STOPPED, right?
Usually it in stopped state only(configuration mode) during initialization.
>
> > + /* Enable Xilinx CAN */
> > + priv->write_reg(priv, XCAN_SRR_OFFSET,
> XCAN_SRR_CEN_MASK);
> > + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> > + while ((priv->read_reg(priv, XCAN_SR_OFFSET) &
> > + XCAN_SR_LBACK_MASK) == 0)
> > + ;
> > + } else {
> > + while ((priv->read_reg(priv, XCAN_SR_OFFSET)
> > + & XCAN_SR_NORMAL_MASK) == 0)
> > + ;
>
> Please add a timeout to the loops.
Ok
>
> > + }
> > + netdev_dbg(ndev, "status:#x%08x\n",
> > + priv->read_reg(priv, XCAN_SR_OFFSET));
> > + }
> > + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcan_do_set_mode - This sets the mode of the driver
> > + * @ndev: Pointer to net_device structure
> > + * @mode: Tells the mode of the driver
> > + *
> > + * This check the drivers state and calls the
> > + * the corresponding modes to set.
> > + *
> > + * Return: 0 on success and failure value on error */ static int
> > +xcan_do_set_mode(struct net_device *ndev, enum can_mode mode) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + int ret;
> > +
> > + netdev_dbg(ndev, "Setting the mode of the driver%s\n", __func__);
>
> please remove dbg
Ok
>
> > +
> > + if (!priv->open_time)
> > + return -EINVAL;
>
> please remove open_time completely.
Ok
>
> > +
> > + switch (mode) {
> > + case CAN_MODE_START:
> > + ret = xcan_start(ndev);
> > + if (ret < 0)
> > + netdev_err(ndev, "xcan_start failed!\n");
> > +
> > + if (netif_queue_stopped(ndev))
> > + netif_wake_queue(ndev);
>
> just call wake_queue
Ok. But existing drivers are using netif_wake_queue right?
> > + break;
> > + default:
> > + ret = -EOPNOTSUPP;
> > + break;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * xcan_start_xmit - Starts the transmission
> > + * @skb: sk_buff pointer that contains data to be Txed
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * This function is invoked from upper layers to initiate
> > +transmission. This
> > + * function uses the next available free txbuff and populates their
> > +fields to
> > + * start the transmission.
> > + *
> > + * Return: 0 on success and failure value on error */ static int
> > +xcan_start_xmit(struct sk_buff *skb, struct net_device *ndev) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + struct net_device_stats *stats = &ndev->stats;
> > + struct can_frame *cf = (struct can_frame *)skb->data;
> > + u32 id, dlc, tmp_dw1, tmp_dw2 = 0, data1, data2 = 0;
> > + unsigned long flags;
> > +
>
> Please add here:
>
> if (can_dropped_invalid_skb(dev, skb))
> return NETDEV_TX_OK;
Ok
>
> > + /* Check if the TX buffer is full */
> > + if (priv->read_reg(priv, XCAN_SR_OFFSET) & XCAN_SR_TXFLL_MASK)
> {
> > + netif_stop_queue(ndev);
> > + netdev_err(ndev, "TX register is still full!\n");
> > + return NETDEV_TX_BUSY;
> > + } else if (priv->waiting_ech_skb_num == priv-
> >xcan_echo_skb_max_tx) {
> > + netif_stop_queue(ndev);
> > + netdev_err(ndev, "waiting:0x%08x, max:0x%08x\n",
> > + priv->waiting_ech_skb_num, priv-
> >xcan_echo_skb_max_tx);
> > + return NETDEV_TX_BUSY;
> > + }
>
> You should handle flow control after you put the CAN frame into the
> hardware, but before activating the TX complete interrutp.
>
Ok
> > + /* Watch carefully on the bit sequence */
> > + if ((cf->can_id & CAN_EFF_FLAG) == 0) {
>
> Nitpick easier to read is:
>
>
> if (cf->can_id & CAN_EFF_FLAG) {
> /* EFF handling */
> } else {
> /* STD handling */
> }
>
Ok
> > + /* Standard CAN ID format */
> > + id = ((cf->can_id & CAN_SFF_MASK) << XCAN_IDR_ID1_SHIFT)
> &
> > + XCAN_IDR_ID1_MASK;
> > +
> > + if (cf->can_id & CAN_RTR_FLAG)
> > + /* Extended frames remote TX request */
> > + id |= XCAN_IDR_SRR_MASK;
> > + } else {
> > + /* Extended CAN ID format */
> > + id = ((cf->can_id & CAN_EFF_MASK) << XCAN_IDR_ID2_SHIFT)
> &
> > + XCAN_IDR_ID2_MASK;
> > + id |= (((cf->can_id & CAN_EFF_MASK) >>
> > + (CAN_EFF_ID_BITS-CAN_SFF_ID_BITS)) <<
> > + XCAN_IDR_ID1_SHIFT) & XCAN_IDR_ID1_MASK;
> > +
> > + /* The substibute remote TX request bit should be "1"
> > + * for extended frames as in the Xilinx CAN datasheet
> > + */
> > + id |= XCAN_IDR_IDE_MASK | XCAN_IDR_SRR_MASK;
> > +
> > + if (cf->can_id & CAN_RTR_FLAG)
> > + /* Extended frames remote TX request */
> > + id |= XCAN_IDR_RTR_MASK;
> > + }
> > +
> > + dlc = (cf->can_dlc & 0xf) << XCAN_DLCR_DLC_SHIFT;
>
> With the above check can_dlc is valid.
>
> > +
> > + tmp_dw1 = le32_to_cpup((u32 *)(cf->data));
> > + data1 = htonl(tmp_dw1);
>
> This looks broken. cf->data is in big endian, what is the endianess of your
> registers?
>
Endianess of our registers is little endian that's why need to do like this.
> > + if (dlc > 4) {
> > + tmp_dw2 = le32_to_cpup((u32 *)(cf->data + 4));
> > + data2 = htonl(tmp_dw2);
> > + }
> > +
> > + netdev_dbg(ndev,
> "tx:id=0x%08x,dlc=0x%08x,d1=0x%08x,d2=0x%08x\n",
> > + id, dlc, data1, data2);
>
> please remove the dbg
>
Ok
> > + /* Write the Frame to Xilinx CAN TX FIFO */
> > + priv->write_reg(priv, XCAN_TXFIFO_ID_OFFSET, id);
> > + priv->write_reg(priv, XCAN_TXFIFO_DLC_OFFSET, dlc);
> > + priv->write_reg(priv, XCAN_TXFIFO_DW1_OFFSET, data1);
> > + priv->write_reg(priv, XCAN_TXFIFO_DW2_OFFSET, data2);
>
> Which write triggers the transmission?
>
The last write triggers the transmission.
If we are sending data less than 4 bytes in that case also we need to
Fill this( XCAN_TXFIFO_DW2_OFFSET) with default value .
> > + stats->tx_bytes += cf->can_dlc;
>
> Can you move the tx_bytes += to your tx-complete routine?
Ok
>
> > + ndev->trans_start = jiffies;
>
> Please remove
Ok
> > +
> > + can_put_echo_skb(skb, ndev, priv->ech_skb_next);
>
> This looks racy, first fill the echo_skb, then start the transmission.
Ok But I didn't understand it clearly.
Will you please explain a little clear.
> > +
> > + priv->ech_skb_next = (priv->ech_skb_next + 1) %
> > + priv->xcan_echo_skb_max_tx;
> > +
> > + spin_lock_irqsave(&priv->ech_skb_lock, flags);
> > + priv->waiting_ech_skb_num++;
> > + spin_unlock_irqrestore(&priv->ech_skb_lock, flags);
> > +
>
> Please move the flow controll handling here.
>
Ok
> > + return NETDEV_TX_OK;
> > +}
> > +
> > +/**
> > + * xcan_rx - Is called from CAN isr to complete the received
> > + * frame processing
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * This function is invoked from the CAN isr(poll) to process the Rx
> > +frames. It
> > + * does minimal processing and invokes "netif_receive_skb" to
> > +complete further
> > + * processing.
> > + * Return: 0 on success and negative error value on error */ static
> > +int xcan_rx(struct net_device *ndev) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + struct net_device_stats *stats = &ndev->stats;
> > + struct can_frame *cf;
> > + struct sk_buff *skb;
> > + u32 id_xcan, dlc, data1, data2;
> > +
> > + skb = alloc_can_skb(ndev, &cf);
> > + if (!skb)
> > + return -ENOMEM;
> > +
> > + /* Read a frame from Xilinx zynq CANPS */
> > + id_xcan = priv->read_reg(priv, XCAN_RXFIFO_ID_OFFSET);
> > + dlc = priv->read_reg(priv, XCAN_RXFIFO_DLC_OFFSET) &
> XCAN_DLCR_DLC_MASK;
> > + data1 = priv->read_reg(priv, XCAN_RXFIFO_DW1_OFFSET);
> > + data2 = priv->read_reg(priv, XCAN_RXFIFO_DW2_OFFSET);
>
> If you don't use data? below, don't read them in the first place. Better move
> the read below, where you fill the data of the can_frame.
>
Ok
> + netdev_dbg(ndev,
> "rx:id=0x%08x,dlc=0x%08x,d1=0x%08x,d2=0x%08x\n",
> > + id_xcan, dlc, data1, data2);
> >
> please remove dbg
Ok
>
> +
> > + /* Change Xilinx CAN data length format to socketCAN data format
> */
> > + cf->can_dlc = get_can_dlc((dlc & XCAN_DLCR_DLC_MASK) >>
> > + XCAN_DLCR_DLC_SHIFT);
> > +
> > + /* Change Xilinx CAN ID format to socketCAN ID format */
> > + if (id_xcan & XCAN_IDR_IDE_MASK) {
> > + /* The received frame is an Extended format frame */
> > + cf->can_id = (id_xcan & XCAN_IDR_ID1_MASK) >> 3;
> > + cf->can_id |= (id_xcan & XCAN_IDR_ID2_MASK) >>
> > + XCAN_IDR_ID2_SHIFT;
> > + cf->can_id |= CAN_EFF_FLAG;
> > + if (id_xcan & XCAN_IDR_RTR_MASK)
> > + cf->can_id |= CAN_RTR_FLAG;
> > + } else {
> > + /* The received frame is a standard format frame */
> > + cf->can_id = (id_xcan & XCAN_IDR_ID1_MASK) >>
> > + XCAN_IDR_ID1_SHIFT;
> > + if (id_xcan & XCAN_IDR_RTR_MASK)
> > + cf->can_id |= CAN_RTR_FLAG;
> > + }
> > +
> > + /* Change Xilinx CAN data format to socketCAN data format */
>
> Don't fill cf->data if RTR is set. The endianess handling looks weird here,
> too.
>
OK
> > + *(u32 *)(cf->data) = ntohl(data1);
> > + if (cf->can_dlc > 4)
> > + *(u32 *)(cf->data + 4) = ntohl(data2);
> > + else
> > + *(u32 *)(cf->data + 4) = 0;
>
> no need to set to zero
>
Ok
> > + stats->rx_bytes += cf->can_dlc;
>
> please group rx_bytes and rx_packets handling
> > +
Ok
> > + can_led_event(ndev, CAN_LED_EVENT_RX);
> > +
> > + netif_receive_skb(skb);
> > +
> > + stats->rx_packets++;
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcan_err_interrupt - error frame Isr
> > + * @ndev: net_device pointer
> > + * @isr: interrupt status register value
> > + *
> > + * This is the CAN error interrupt and it will
> > + * check the the type of error and forward the error
> > + * frame to upper layers.
> > + */
> > +static void xcan_err_interrupt(struct net_device *ndev, u32 isr) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + struct net_device_stats *stats = &ndev->stats;
> > + struct can_frame *cf;
> > + struct sk_buff *skb;
> > + u32 err_status, status;
> > +
> > + skb = alloc_can_err_skb(ndev, &cf);
> > + if (!skb) {
> > + netdev_err(ndev, "alloc_can_err_skb() failed!\n");
> > + return;
> > + }
> > +
> > + err_status = priv->read_reg(priv, XCAN_ESR_OFFSET);
> > + priv->write_reg(priv, XCAN_ESR_OFFSET, err_status);
> > + status = priv->read_reg(priv, XCAN_SR_OFFSET);
> > +
> > + if (isr & XCAN_IXR_BSOFF_MASK) {
> > + priv->can.state = CAN_STATE_BUS_OFF;
> > + cf->can_id |= CAN_ERR_BUSOFF;
> > + priv->can.can_stats.bus_off++;
> > + /* Leave device in Config Mode in bus-off state */
> > + priv->write_reg(priv, XCAN_SRR_OFFSET,
> XCAN_SRR_RESET_MASK);
> > + can_bus_off(ndev);
> > + } else if ((status & XCAN_SR_ESTAT_MASK) ==
> XCAN_SR_ESTAT_MASK) {
> > + cf->can_id |= CAN_ERR_CRTL;
> > + priv->can.state = CAN_STATE_ERROR_PASSIVE;
> > + priv->can.can_stats.error_passive++;
> > + cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE |
> > + CAN_ERR_CRTL_TX_PASSIVE;
> > + } else if (status & XCAN_SR_ERRWRN_MASK) {
> > + cf->can_id |= CAN_ERR_CRTL;
> > + priv->can.state = CAN_STATE_ERROR_WARNING;
> > + priv->can.can_stats.error_warning++;
> > + cf->data[1] |= CAN_ERR_CRTL_RX_WARNING |
> > + CAN_ERR_CRTL_TX_WARNING;
> > + }
> > +
> > + /* Check for Arbitration lost interrupt */
> > + if (isr & XCAN_IXR_ARBLST_MASK) {
> > + cf->can_id |= CAN_ERR_LOSTARB;
> > + cf->data[0] = CAN_ERR_LOSTARB_UNSPEC;
> > + priv->can.can_stats.arbitration_lost++;
> > + }
> > +
> > + /* Check for RX FIFO Overflow interrupt */
> > + if (isr & XCAN_IXR_RXOFLW_MASK) {
> > + cf->can_id |= CAN_ERR_CRTL;
> > + cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
> > + stats->rx_over_errors++;
> > + stats->rx_errors++;
> > + priv->write_reg(priv, XCAN_SRR_OFFSET,
> XCAN_SRR_RESET_MASK);
> > + }
> > +
> > + /* Check for error interrupt */
> > + if (isr & XCAN_IXR_ERROR_MASK) {
> > + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> > + cf->data[2] |= CAN_ERR_PROT_UNSPEC;
> > +
> > + /* Check for Ack error interrupt */
> > + if (err_status & XCAN_ESR_ACKER_MASK) {
> > + cf->can_id |= CAN_ERR_ACK;
> > + cf->data[3] |= CAN_ERR_PROT_LOC_ACK;
> > + stats->tx_errors++;
> > + }
> > +
> > + /* Check for Bit error interrupt */
> > + if (err_status & XCAN_ESR_BERR_MASK) {
> > + cf->can_id |= CAN_ERR_PROT;
> > + cf->data[2] = CAN_ERR_PROT_BIT;
> > + stats->tx_errors++;
> > + }
> > +
> > + /* Check for Stuff error interrupt */
> > + if (err_status & XCAN_ESR_STER_MASK) {
> > + cf->can_id |= CAN_ERR_PROT;
> > + cf->data[2] = CAN_ERR_PROT_STUFF;
> > + stats->rx_errors++;
> > + }
> > +
> > + /* Check for Form error interrupt */
> > + if (err_status & XCAN_ESR_FMER_MASK) {
> > + cf->can_id |= CAN_ERR_PROT;
> > + cf->data[2] = CAN_ERR_PROT_FORM;
> > + stats->rx_errors++;
> > + }
> > +
> > + /* Check for CRC error interrupt */
> > + if (err_status & XCAN_ESR_CRCER_MASK) {
> > + cf->can_id |= CAN_ERR_PROT;
> > + cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ |
> > + CAN_ERR_PROT_LOC_CRC_DEL;
> > + stats->rx_errors++;
> > + }
> > + priv->can.can_stats.bus_error++;
> > + }
> > +
> > + netif_rx(skb);
> > + stats->rx_packets++;
> > + stats->rx_bytes += cf->can_dlc;
> > +
> > + netdev_dbg(ndev, "%s: error status register:0x%x\n",
> > + __func__, priv->read_reg(priv, XCAN_ESR_OFFSET));
> }
> > +
> > +/**
> > + * xcan_state_interrupt - It will check the state of the CAN device
> > + * @ndev: net_device pointer
> > + * @isr: interrupt status register value
> > + *
> > + * This will checks the state of the CAN device
> > + * and puts the device into appropriate state.
> > + */
> > +static void xcan_state_interrupt(struct net_device *ndev, u32 isr) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > +
> > + /* Check for Sleep interrupt if set put CAN device in sleep state */
> > + if (isr & XCAN_IXR_SLP_MASK)
> > + priv->can.state = CAN_STATE_SLEEPING;
> > +
> > + /* Check for Wake up interrupt if set put CAN device in Active state
> */
> > + if (isr & XCAN_IXR_WKUP_MASK)
> > + priv->can.state = CAN_STATE_ERROR_ACTIVE; }
> > +
> > +/**
> > + * xcan_rx_poll - Poll routine for rx packets (NAPI)
> > + * @napi: napi structure pointer
> > + * @quota: Max number of rx packets to be processed.
> > + *
> > + * This is the poll routine for rx part.
> > + * It will process the packets maximux quota value.
> > + *
> > + * Return: number of packets received */ static int
> > +xcan_rx_poll(struct napi_struct *napi, int quota) {
> > + struct net_device *ndev = napi->dev;
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + u32 isr, ier;
> > + int work_done = 0;
> > +
> > + isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
> > + while ((isr & XCAN_IXR_RXNEMP_MASK) && (work_done < quota)) {
> > + if (isr & XCAN_IXR_RXOK_MASK) {
> > + priv->write_reg(priv, XCAN_ICR_OFFSET,
> > + XCAN_IXR_RXOK_MASK);
> > + if (xcan_rx(ndev) < 0)
> > + return work_done;
> > + work_done++;
> > + } else {
> > + priv->write_reg(priv, XCAN_ICR_OFFSET,
> > + XCAN_IXR_RXNEMP_MASK);
> > + break;
> > + }
> > + priv->write_reg(priv, XCAN_ICR_OFFSET,
> XCAN_IXR_RXNEMP_MASK);
> > + isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
> > + }
> > +
> > + if (work_done < quota) {
> > + napi_complete(napi);
> > + ier = priv->read_reg(priv, XCAN_IER_OFFSET);
> > + ier |= (XCAN_IXR_RXOK_MASK |
> XCAN_IXR_RXNEMP_MASK);
> > + priv->write_reg(priv, XCAN_IER_OFFSET, ier);
> > + }
> > + return work_done;
> > +}
> > +
> > +/**
> > + * xcan_tx_interrupt - Tx Done Isr
> > + * @ndev: net_device pointer
> > + */
> > +static void xcan_tx_interrupt(struct net_device *ndev) {
> > + unsigned long flags;
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + struct net_device_stats *stats = &ndev->stats;
> > + u32 processed = 0, txpackets;
> > +
> > + stats->tx_packets++;
> > + netdev_dbg(ndev, "%s: waiting total:%d,current:%d\n", __func__,
> > + priv->waiting_ech_skb_num, priv-
> >waiting_ech_skb_index);
> > +
> > + txpackets = priv->waiting_ech_skb_num;
> > +
> > + if (txpackets) {
> > + can_get_echo_skb(ndev, priv->waiting_ech_skb_index);
> > + priv->waiting_ech_skb_index =
> > + (priv->waiting_ech_skb_index + 1) %
> > + priv->xcan_echo_skb_max_tx;
> > + processed++;
> > + txpackets--;
> > + }
> > +
> > + spin_lock_irqsave(&priv->ech_skb_lock, flags);
> > + priv->waiting_ech_skb_num -= processed;
> > + spin_unlock_irqrestore(&priv->ech_skb_lock, flags);
> > +
> > + netdev_dbg(ndev, "%s: waiting total:%d,current:%d\n", __func__,
> > + priv->waiting_ech_skb_num, priv-
> >waiting_ech_skb_index);
> > +
> > + netif_wake_queue(ndev);
> > +
> > + can_led_event(ndev, CAN_LED_EVENT_TX); }
> > +
> > +/**
> > + * xcan_interrupt - CAN Isr
> > + * @irq: irq number
> > + * @dev_id: device id poniter
> > + *
> > + * This is the xilinx CAN Isr. It checks for the type of interrupt
> > + * and invokes the corresponding ISR.
> > + *
> > + * Return:
> > + * IRQ_NONE - If CAN device is in sleep mode, IRQ_HANDLED otherwise
> > +*/ static irqreturn_t xcan_interrupt(int irq, void *dev_id) {
> > + struct net_device *ndev = (struct net_device *)dev_id;
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + u32 isr, ier;
> > +
> > + if (priv->can.state == CAN_STATE_STOPPED)
>
> This should not happen, please remove.
Ok
>
> > + return IRQ_NONE;
> > +
> > + /* Get the interrupt status from Xilinx CAN */
> > + isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
> > + if (!isr)
> > + return IRQ_NONE;
> > +
> > + netdev_dbg(ndev, "%s: isr:#x%08x, err:#x%08x\n", __func__,
> > + isr, priv->read_reg(priv, XCAN_ESR_OFFSET));
> > +
> > + /* Check for the type of interrupt and Processing it */
> > + if (isr & (XCAN_IXR_SLP_MASK | XCAN_IXR_WKUP_MASK)) {
> > + priv->write_reg(priv, XCAN_ICR_OFFSET,
> (XCAN_IXR_SLP_MASK |
> > + XCAN_IXR_WKUP_MASK));
> > + xcan_state_interrupt(ndev, isr);
> > + }
> > +
> > + /* Check for Tx interrupt and Processing it */
> > + if (isr & XCAN_IXR_TXOK_MASK) {
> > + priv->write_reg(priv, XCAN_ICR_OFFSET,
> XCAN_IXR_TXOK_MASK);
> > + xcan_tx_interrupt(ndev);
> > + }
> > +
> > + /* Check for the type of error interrupt and Processing it */
> > + if (isr & (XCAN_IXR_ERROR_MASK | XCAN_IXR_RXOFLW_MASK |
> > + XCAN_IXR_BSOFF_MASK |
> XCAN_IXR_ARBLST_MASK)) {
> > + priv->write_reg(priv, XCAN_ICR_OFFSET,
> (XCAN_IXR_ERROR_MASK |
> > + XCAN_IXR_RXOFLW_MASK |
> XCAN_IXR_BSOFF_MASK |
> > + XCAN_IXR_ARBLST_MASK));
> > + xcan_err_interrupt(ndev, isr);
> > + }
> > +
> > + /* Check for the type of receive interrupt and Processing it */
> > + if (isr & (XCAN_IXR_RXNEMP_MASK | XCAN_IXR_RXOK_MASK)) {
> > + ier = priv->read_reg(priv, XCAN_IER_OFFSET);
> > + ier &= ~(XCAN_IXR_RXNEMP_MASK |
> XCAN_IXR_RXOK_MASK);
> > + priv->write_reg(priv, XCAN_IER_OFFSET, ier);
> > + napi_schedule(&priv->napi);
> > + }
> > + return IRQ_HANDLED;
> > +}
> > +
> > +/**
> > + * xcan_stop - Driver stop routine
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * This is the drivers stop routine. It will disable the
> > + * interrupts and put the device into configuration mode.
> > + */
> > +static void xcan_stop(struct net_device *ndev) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + u32 ier;
> > +
> > + /* Disable interrupts and leave the can in configuration mode */
> > + ier = priv->read_reg(priv, XCAN_IER_OFFSET);
> > + ier &= ~XCAN_INTR_ALL;
> > + priv->write_reg(priv, XCAN_IER_OFFSET, ier);
> > + priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_RESET_MASK);
> > + priv->can.state = CAN_STATE_STOPPED; }
> > +
> > +/**
> > + * xcan_open - Driver open routine
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * This is the driver open routine.
> > + * Return: 0 on success and failure value on error */ static int
> > +xcan_open(struct net_device *ndev) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + int err;
> > +
> > + /* Set chip into reset mode */
> > + err = set_reset_mode(ndev);
> > + if (err < 0)
> > + netdev_err(ndev, "mode resetting failed failed!\n");
> > +
> > + /* Common open */
> > + err = open_candev(ndev);
> > + if (err)
> > + return err;
> > +
> > + err = xcan_start(ndev);
> > + if (err < 0)
> > + netdev_err(ndev, "xcan_start failed!\n");
> > +
> > +
> > + can_led_event(ndev, CAN_LED_EVENT_OPEN);
> > + napi_enable(&priv->napi);
> > + netif_start_queue(ndev);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcan_close - Driver close routine
> > + * @ndev: Pointer to net_device structure
> > + *
> > + * Return: 0 always
> > + */
> > +static int xcan_close(struct net_device *ndev) {
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > +
> > + netif_stop_queue(ndev);
> > + napi_disable(&priv->napi);
> > + xcan_stop(ndev);
> > + close_candev(ndev);
> > +
> > + can_led_event(ndev, CAN_LED_EVENT_STOP);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcan_get_berr_counter - error counter routine
> > + * @ndev: Pointer to net_device structure
> > + * @bec: Pointer to can_berr_counter structure
> > + *
> > + * This is the driver error counter routine.
> > + * Return: 0 always
> > + */
> > +static int xcan_get_berr_counter(const struct net_device *ndev,
> > + struct can_berr_counter *bec)
> > +{
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > +
> > + bec->txerr = priv->read_reg(priv, XCAN_ECR_OFFSET) &
> XCAN_ECR_TEC_MASK;
> > + bec->rxerr = ((priv->read_reg(priv, XCAN_ECR_OFFSET) &
> > + XCAN_ECR_REC_MASK) >> XCAN_ESR_REC_SHIFT);
> > + return 0;
> > +}
> > +
> > +static const struct net_device_ops xcan_netdev_ops = {
> > + .ndo_open = xcan_open,
> > + .ndo_stop = xcan_close,
> > + .ndo_start_xmit = xcan_start_xmit,
> > +};
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +/**
> > + * xcan_suspend - Suspend method for the driver
> > + * @_dev: Address of the platform_device structure
> > + *
> > + * Put the driver into low power mode.
> > + * Return: 0 always
> > + */
> > +static int xcan_suspend(struct device *_dev) {
> > + struct platform_device *pdev = container_of(_dev,
> > + struct platform_device, dev);
> > + struct net_device *ndev = platform_get_drvdata(pdev);
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > +
> > + if (netif_running(ndev)) {
> > + netif_stop_queue(ndev);
> > + netif_device_detach(ndev);
> > + }
> > +
> > + priv->write_reg(priv, XCAN_MSR_OFFSET,
> XCAN_MSR_SLEEP_MASK);
> > + priv->can.state = CAN_STATE_SLEEPING;
> > +
> > + clk_disable(priv->aperclk);
> > + clk_disable(priv->devclk);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcan_resume - Resume from suspend
> > + * @dev: Address of the platformdevice structure
> > + *
> > + * Resume operation after suspend.
> > + * Return: 0 on success and failure value on error */ static int
> > +xcan_resume(struct device *dev) {
> > + struct platform_device *pdev = container_of(dev,
> > + struct platform_device, dev);
> > + struct net_device *ndev = platform_get_drvdata(pdev);
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > + int ret;
> > +
> > + ret = clk_enable(priv->aperclk);
> > + if (ret) {
> > + dev_err(dev, "Cannot enable clock.\n");
> > + return ret;
> > + }
> > + ret = clk_enable(priv->devclk);
> > + if (ret) {
> > + dev_err(dev, "Cannot enable clock.\n");
> > + return ret;
> > + }
> > +
> > + priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
> > + priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_CEN_MASK);
> > + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > +
> > + if (netif_running(ndev)) {
> > + netif_device_attach(ndev);
> > + netif_start_queue(ndev);
> > + }
> > +
> > + return 0;
> > +}
> > +#endif
> > +
> > +static SIMPLE_DEV_PM_OPS(xcan_dev_pm_ops, xcan_suspend,
> xcan_resume);
> > +
> > +/**
> > + * xcan_probe - Platform registration call
> > + * @pdev: Handle to the platform device structure
> > + *
> > + * This function does all the memory allocation and registration for
> > +the CAN
> > + * device.
> > + *
> > + * Return: 0 on success and failure value on error */ static int
> > +xcan_probe(struct platform_device *pdev) {
> > + struct resource *res; /* IO mem resources */
> > + struct net_device *ndev;
> > + struct xcan_priv *priv;
> > + int ret, fifodep;
> > +
> > + /* Create a CAN device instance */
> > + ndev = alloc_candev(sizeof(struct xcan_priv),
> XCAN_ECHO_SKB_MAX);
> > + if (!ndev)
> > + return -ENOMEM;
> > +
> > + priv = netdev_priv(ndev);
> > + priv->dev = ndev;
> > + priv->can.bittiming_const = &xcan_bittiming_const;
> > + priv->can.do_set_bittiming = xcan_set_bittiming;
> > + priv->can.do_set_mode = xcan_do_set_mode;
> > + priv->can.do_get_berr_counter = xcan_get_berr_counter;
> > + priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> > + CAN_CTRLMODE_BERR_REPORTING;
> > + priv->xcan_echo_skb_max_tx = XCAN_ECHO_SKB_MAX;
> > + priv->xcan_echo_skb_max_rx = XCAN_NAPI_WEIGHT;
> > +
> > + /* Get IRQ for the device */
> > + ndev->irq = platform_get_irq(pdev, 0);
> > + ret = devm_request_irq(&pdev->dev, ndev->irq, &xcan_interrupt,
> > + priv->irq_flags, dev_name(&pdev->dev),
> > + (void *)ndev);
>
> We usually request the interrupt on in the open() function
>
Ok Will Move into open routine
Regards,
Kedar.
> > + if (ret < 0) {
> > + dev_err(&pdev->dev, "Irq allocation for CAN failed\n");
> > + goto err_free;
> > + }
> > +
> > + spin_lock_init(&priv->ech_skb_lock);
> > + ndev->flags |= IFF_ECHO; /* We support local echo */
> > +
> > + platform_set_drvdata(pdev, ndev);
> > + SET_NETDEV_DEV(ndev, &pdev->dev);
> > + ndev->netdev_ops = &xcan_netdev_ops;
> > +
> > + /* Get the virtual base address for the device */
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + priv->reg_base = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(priv->reg_base)) {
> > + ret = PTR_ERR(priv->reg_base);
> > + goto err_free;
> > + }
> > + ndev->mem_start = res->start;
> > + ndev->mem_end = res->end;
> > +
> > + priv->write_reg = xcan_write_reg;
> > + priv->read_reg = xcan_read_reg;
> > +
> > + /* Getting the CAN devclk info */
> > + priv->devclk = devm_clk_get(&pdev->dev, "ref_clk");
> > + if (IS_ERR(priv->devclk)) {
> > + dev_err(&pdev->dev, "Device clock not found.\n");
> > + ret = PTR_ERR(priv->devclk);
> > + goto err_free;
> > + }
> > +
> > + /* Check for type of CAN device */
> > + if (of_device_is_compatible(pdev->dev.of_node,
> > + "xlnx,zynq-can-1.00.a")) {
> > + priv->aperclk = devm_clk_get(&pdev->dev, "aper_clk");
> > + if (IS_ERR(priv->aperclk)) {
> > + dev_err(&pdev->dev, "aper clock not found\n");
> > + ret = PTR_ERR(priv->aperclk);
> > + goto err_free;
> > + }
> > + } else {
> > + priv->aperclk = priv->devclk;
> > + ret = of_property_read_u32(pdev->dev.of_node,
> > + "xlnx,can-tx-dpth", &fifodep);
> > + if (ret < 0)
> > + goto err_free;
> > + priv->xcan_echo_skb_max_tx = fifodep;
> > + ret = of_property_read_u32(pdev->dev.of_node,
> > + "xlnx,can-rx-dpth", &fifodep);
> > + if (ret < 0)
> > + goto err_free;
> > + priv->xcan_echo_skb_max_rx = fifodep;
> > + }
> > +
> > + ret = clk_prepare_enable(priv->devclk);
> > + if (ret) {
> > + dev_err(&pdev->dev, "unable to enable device clock\n");
> > + goto err_free;
> > + }
> > +
> > + ret = clk_prepare_enable(priv->aperclk);
> > + if (ret) {
> > + dev_err(&pdev->dev, "unable to enable aper clock\n");
> > + goto err_unprepar_disabledev;
> > + }
> > +
> > + priv->can.clock.freq = clk_get_rate(priv->devclk);
> > +
> > + netif_napi_add(ndev, &priv->napi, xcan_rx_poll,
> > + priv->xcan_echo_skb_max_rx);
> > + ret = register_candev(ndev);
> > + if (ret) {
> > + dev_err(&pdev->dev, "fail to register failed (err=%d)\n",
> ret);
> > + goto err_unprepar_disableaper;
> > + }
> > +
> > + devm_can_led_init(ndev);
> > + dev_info(&pdev->dev,
> > + "reg_base=0x%p irq=%d clock=%d, tx fifo
> depth:%d\n",
> > + priv->reg_base, ndev->irq, priv->can.clock.freq,
> > + priv->xcan_echo_skb_max_tx);
> > +
> > + return 0;
> > +
> > +err_unprepar_disableaper:
> > + clk_disable_unprepare(priv->aperclk);
> > +err_unprepar_disabledev:
> > + clk_disable_unprepare(priv->devclk);
> > +err_free:
> > + free_candev(ndev);
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * xcan_remove - Unregister the device after releasing the resources
> > + * @pdev: Handle to the platform device structure
> > + *
> > + * This function frees all the resources allocated to the device.
> > + * Return: 0 always
> > + */
> > +static int xcan_remove(struct platform_device *pdev) {
> > + struct net_device *ndev = platform_get_drvdata(pdev);
> > + struct xcan_priv *priv = netdev_priv(ndev);
> > +
> > + if (set_reset_mode(ndev) < 0)
> > + netdev_err(ndev, "mode resetting failed!\n");
> > +
> > + unregister_candev(ndev);
> > + netif_napi_del(&priv->napi);
> > + clk_disable_unprepare(priv->aperclk);
> > + clk_disable_unprepare(priv->devclk);
> > +
> > + free_candev(ndev);
> > +
> > + return 0;
> > +}
> > +
> > +/* Match table for OF platform binding */ static struct of_device_id
> > +xcan_of_match[] = {
> > + { .compatible = "xlnx,zynq-can-1.00.a", },
> > + { .compatible = "xlnx,axi-can-1.00.a", },
> > + { /* end of list */ },
> > +};
> > +MODULE_DEVICE_TABLE(of, xcan_of_match);
> > +
> > +static struct platform_driver xcan_driver = {
> > + .probe = xcan_probe,
> > + .remove = xcan_remove,
> > + .driver = {
> > + .owner = THIS_MODULE,
> > + .name = DRIVER_NAME,
> > + .pm = &xcan_dev_pm_ops,
> > + .of_match_table = xcan_of_match,
> > + },
> > +};
> > +
> > +module_platform_driver(xcan_driver);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Xilinx Inc");
> > +MODULE_DESCRIPTION("Xilinx CAN interface");
> >
>
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Industrial Linux Solutions | Phone: +49-231-2826-924 |
> Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
^ permalink raw reply
* Re: [PATCH 1/4] ARM: STi: add stid127 soc support
From: srinivas kandagatla @ 2014-02-07 8:08 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Russell King,
kernel-F5mvAk5X5gdBDgjK7y7TUQ, Linus Walleij, Patrice CHOTARD,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Stuart Menefy, Rob Herring,
Grant Likely, Giuseppe Cavallaro, maxime.coquelin-qxv4g6HH51o,
alexandre.torgue-qxv4g6HH51o
In-Reply-To: <201402061746.30248.arnd-r2nGTMty4D4@public.gmane.org>
On 06/02/14 16:46, Arnd Bergmann wrote:
> On Wednesday 05 February 2014, srinivas kandagatla wrote:
>> Currently l2cc bindings has few optional properties like.
>>
>> - arm,data-latency
>> - arm,tag-latency
>> - arm,dirty-latency
>> - arm,filter-ranges
>> - interrupts :
>> - cache-id-part:
>> - wt-override:
>>
>> These does not include properties to set "way-size", "associativity",
>> "enabling prefetching", "Prefetch drop enable", "prefetch offset",
>> "Double linefill" and few more in prefect control register and
>> aux-control register.
>>
>> This is not just a issue with STi SOCs, having a quick look, I can see
>> that few more SOCs have similar requirements to set these properties.
>>
>> We could do two things to get l2 setup automatically on STi SOCS.
>>
>> 1> Either define these properties case-by-case basic, which might be
>> useful for other SOCs too.
>>
>> 2> Or Add new compatible string for STi SoCs so that they can
>> automatically setup these values in cache-l2x0.c
>>
>> Am Ok with either approaches.
>>
>
> I suggested 1 in the past, but the objection that I saw (can't
> find the email at the moment) was that the additional settings
> are "configuration" rather than "hardware properties". What I'd
> really need to know from you is which of properties you listed
> as missing above are actually needed for your platform, and whether
> they can be classified as hardware specific or just configuration.
On STi Platforms we need below properties to got for option 1.
arm,way-size;
arm,instruction-prefetch-enable;
arm,data-prefetch-enable;
we also want a property or a way to set
"Shareable attribute Override Enable" bit in the Auxiliary Control
Register, bit[22].
Thanks,
srini
>
> Arnd
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v1 1/3] net: stmmac:sti: Add STi SOC glue driver.
From: srinivas kandagatla @ 2014-02-07 7:54 UTC (permalink / raw)
To: David Miller
Cc: mark.rutland, devicetree, linux, kernel, pawel.moll,
ijc+devicetree, netdev, linux-doc, linux-kernel, stuart.menefy,
robh+dt, rob, galak, peppe.cavallaro, linux-arm-kernel
In-Reply-To: <20140206.195342.1998479313077409827.davem@davemloft.net>
Thankyou Dave,
On 07/02/14 03:53, David Miller wrote:
> From: <srinivas.kandagatla@st.com>
> Date: Mon, 3 Feb 2014 12:01:08 +0000
>
>> + res = platform_get_resource_byname(pdev,
>> + IORESOURCE_MEM, "sti-ethconf");
>
> This is not the correct way to format multi-line function calls,
> you'll need to fix this up in this entire series.
I will fix this in next version.
>
> The arguments on the second and subsequent lines must start at
> the first column after the openning parenthesis of the function
> call. You must use the appropriate number of both space and
> TAB characters necessary to do so.
>
> If you're only using TAB characters to indent, you're doing it
> wrong.
>
> Thank you.
>
>
Thanks,
srini
^ 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