* [PATCH v8 3/8] PWM: add pwm-stm32 DT bindings
From: Benjamin Gaignard @ 2017-01-18 14:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484749251-14445-1-git-send-email-benjamin.gaignard@st.com>
Define bindings for pwm-stm32
version 8:
- reword st,breakinput description.
version 6:
- change st,breakinput parameter format to make it usuable on stm32f7 too.
version 2:
- use parameters instead of compatible of handle the hardware configuration
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
Acked-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/pwm/pwm-stm32.txt | 35 ++++++++++++++++++++++
1 file changed, 35 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
diff --git a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
new file mode 100644
index 0000000..6dd0403
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
@@ -0,0 +1,35 @@
+STMicroelectronics STM32 Timers PWM bindings
+
+Must be a sub-node of an STM32 Timers device tree node.
+See ../mfd/stm32-timers.txt for details about the parent node.
+
+Required parameters:
+- compatible: Must be "st,stm32-pwm".
+- pinctrl-names: Set to "default".
+- pinctrl-0: List of phandles pointing to pin configuration nodes for PWM module.
+ For Pinctrl properties see ../pinctrl/pinctrl-bindings.txt
+
+Optional parameters:
+- st,breakinput: One or two <index level filter> to describe break input configurations.
+ "index" indicates on which break input (0 or 1) the configuration
+ should be applied.
+ "level" gives the active level (0=low or 1=high) of the input signal
+ for this configuration.
+ "filter" gives the filtering value to be applied.
+
+Example:
+ timers at 40010000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "st,stm32-timers";
+ reg = <0x40010000 0x400>;
+ clocks = <&rcc 0 160>;
+ clock-names = "clk_int";
+
+ pwm {
+ compatible = "st,stm32-pwm";
+ pinctrl-0 = <&pwm1_pins>;
+ pinctrl-names = "default";
+ st,breakinput = <0 1 5>;
+ };
+ };
--
1.9.1
^ permalink raw reply related
* [PATCH v8 4/8] PWM: add PWM driver for STM32 plaftorm
From: Benjamin Gaignard @ 2017-01-18 14:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484749251-14445-1-git-send-email-benjamin.gaignard@st.com>
This driver adds support for PWM driver on STM32 platform.
The SoC have multiple instances of the hardware IP and each
of them could have small differences: number of channels,
complementary output, auto reload register size...
version 8:
- fix comments done by Thierry on version 7
version 6:
- change st,breakinput parameter to make it usuable for stm32f7 too.
version 4:
- detect at probe time hardware capabilities
- fix comments done on v2 and v3
- use PWM atomic ops
version 2:
- only keep one comptatible
- use DT parameters to discover hardware block configuration
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
drivers/pwm/Kconfig | 9 ++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-stm32.c | 398 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 408 insertions(+)
create mode 100644 drivers/pwm/pwm-stm32.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index f92dd41..2d0cfaa 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -397,6 +397,15 @@ config PWM_STI
To compile this driver as a module, choose M here: the module
will be called pwm-sti.
+config PWM_STM32
+ tristate "STMicroelectronics STM32 PWM"
+ depends on MFD_STM32_TIMERS || COMPILE_TEST
+ help
+ Generic PWM framework driver for STM32 SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-stm32.
+
config PWM_STMPE
bool "STMPE expander PWM export"
depends on MFD_STMPE
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index a48bdb5..346a83b 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_PWM_ROCKCHIP) += pwm-rockchip.o
obj-$(CONFIG_PWM_SAMSUNG) += pwm-samsung.o
obj-$(CONFIG_PWM_SPEAR) += pwm-spear.o
obj-$(CONFIG_PWM_STI) += pwm-sti.o
+obj-$(CONFIG_PWM_STM32) += pwm-stm32.o
obj-$(CONFIG_PWM_STMPE) += pwm-stmpe.o
obj-$(CONFIG_PWM_SUN4I) += pwm-sun4i.o
obj-$(CONFIG_PWM_TEGRA) += pwm-tegra.o
diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
new file mode 100644
index 0000000..3780afd
--- /dev/null
+++ b/drivers/pwm/pwm-stm32.c
@@ -0,0 +1,398 @@
+/*
+ * Copyright (C) STMicroelectronics 2016
+ *
+ * Author: Gerald Baeza <gerald.baeza@st.com>
+ *
+ * License terms: GNU General Public License (GPL), version 2
+ *
+ * Inspired by timer-stm32.c from Maxime Coquelin
+ * pwm-atmel.c from Bo Shen
+ */
+
+#include <linux/mfd/stm32-timers.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+
+#define CCMR_CHANNEL_SHIFT 8
+#define CCMR_CHANNEL_MASK 0xFF
+#define MAX_BREAKINPUT 2
+
+struct stm32_pwm {
+ struct pwm_chip chip;
+ struct device *dev;
+ struct clk *clk;
+ struct regmap *regmap;
+ u32 max_arr;
+ bool have_complementary_output;
+};
+
+struct stm32_breakinput {
+ u32 index;
+ u32 level;
+ u32 filter;
+};
+
+static inline struct stm32_pwm *to_stm32_pwm_dev(struct pwm_chip *chip)
+{
+ return container_of(chip, struct stm32_pwm, chip);
+}
+
+static u32 active_channels(struct stm32_pwm *dev)
+{
+ u32 ccer;
+
+ regmap_read(dev->regmap, TIM_CCER, &ccer);
+
+ return ccer & TIM_CCER_CCXE;
+}
+
+static int write_ccrx(struct stm32_pwm *dev, int ch, u32 value)
+{
+ switch (ch) {
+ case 0:
+ return regmap_write(dev->regmap, TIM_CCR1, value);
+ case 1:
+ return regmap_write(dev->regmap, TIM_CCR2, value);
+ case 2:
+ return regmap_write(dev->regmap, TIM_CCR3, value);
+ case 3:
+ return regmap_write(dev->regmap, TIM_CCR4, value);
+ }
+ return -EINVAL;
+}
+
+static int stm32_pwm_config(struct stm32_pwm *priv, int ch,
+ int duty_ns, int period_ns)
+{
+ unsigned long long prd, div, dty;
+ unsigned int prescaler = 0;
+ u32 ccmr, mask, shift;
+
+ /* Period and prescaler values depends on clock rate */
+ div = (unsigned long long)clk_get_rate(priv->clk) * period_ns;
+
+ do_div(div, NSEC_PER_SEC);
+ prd = div;
+
+ while (div > priv->max_arr) {
+ prescaler++;
+ div = prd;
+ do_div(div, prescaler + 1);
+ }
+
+ prd = div;
+
+ if (prescaler > MAX_TIM_PSC)
+ return -EINVAL;
+
+ /*
+ * All channels share the same prescaler and counter so when two
+ * channels are active at the same time we can't change them
+ */
+ if (active_channels(priv) & ~(1 << ch * 4)) {
+ u32 psc, arr;
+
+ regmap_read(priv->regmap, TIM_PSC, &psc);
+ regmap_read(priv->regmap, TIM_ARR, &arr);
+
+ if ((psc != prescaler) || (arr != prd - 1))
+ return -EBUSY;
+ }
+
+ regmap_write(priv->regmap, TIM_PSC, prescaler);
+ regmap_write(priv->regmap, TIM_ARR, prd - 1);
+ regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
+
+ /* Calculate the duty cycles */
+ dty = prd * duty_ns;
+ do_div(dty, period_ns);
+
+ write_ccrx(priv, ch, dty);
+
+ /* Configure output mode */
+ shift = (ch & 0x1) * CCMR_CHANNEL_SHIFT;
+ ccmr = (TIM_CCMR_PE | TIM_CCMR_M1) << shift;
+ mask = CCMR_CHANNEL_MASK << shift;
+
+ if (ch < 2)
+ regmap_update_bits(priv->regmap, TIM_CCMR1, mask, ccmr);
+ else
+ regmap_update_bits(priv->regmap, TIM_CCMR2, mask, ccmr);
+
+ regmap_update_bits(priv->regmap, TIM_BDTR,
+ TIM_BDTR_MOE | TIM_BDTR_AOE,
+ TIM_BDTR_MOE | TIM_BDTR_AOE);
+
+ return 0;
+}
+
+static int stm32_pwm_set_polarity(struct stm32_pwm *priv, int ch,
+ enum pwm_polarity polarity)
+{
+ u32 mask;
+
+ mask = TIM_CCER_CC1P << (ch * 4);
+ if (priv->have_complementary_output)
+ mask |= TIM_CCER_CC1NP << (ch * 4);
+
+ regmap_update_bits(priv->regmap, TIM_CCER, mask,
+ polarity == PWM_POLARITY_NORMAL ? 0 : mask);
+
+ return 0;
+}
+
+static int stm32_pwm_enable(struct stm32_pwm *priv, int ch)
+{
+ u32 mask;
+ int ret;
+
+ ret = clk_enable(priv->clk);
+ if (ret)
+ return ret;
+
+ /* Enable channel */
+ mask = TIM_CCER_CC1E << (ch * 4);
+ if (priv->have_complementary_output)
+ mask |= TIM_CCER_CC1NE << (ch * 4);
+
+ regmap_update_bits(priv->regmap, TIM_CCER, mask, mask);
+
+ /* Make sure that registers are updated */
+ regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
+
+ /* Enable controller */
+ regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, TIM_CR1_CEN);
+
+ return 0;
+}
+
+static void stm32_pwm_disable(struct stm32_pwm *priv, int ch)
+{
+ u32 mask;
+
+ /* Disable channel */
+ mask = TIM_CCER_CC1E << (ch * 4);
+ if (priv->have_complementary_output)
+ mask |= TIM_CCER_CC1NE << (ch * 4);
+
+ regmap_update_bits(priv->regmap, TIM_CCER, mask, 0);
+
+ /* When all channels are disabled, we can disable the controller */
+ if (!active_channels(priv))
+ regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, 0);
+
+ clk_disable(priv->clk);
+}
+
+static int stm32_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ struct pwm_state curstate;
+ bool enabled;
+ struct stm32_pwm *priv = to_stm32_pwm_dev(chip);
+ int ret;
+
+ enabled = pwm->state.enabled;
+
+ if (enabled && !state->enabled) {
+ stm32_pwm_disable(priv, pwm->hwpwm);
+ return 0;
+ }
+
+ if (state->polarity != curstate.polarity)
+ stm32_pwm_set_polarity(priv, pwm->hwpwm, state->polarity);
+
+ ret = stm32_pwm_config(priv, pwm->hwpwm,
+ state->duty_cycle, state->period);
+ if (ret)
+ return ret;
+
+ if (!enabled && state->enabled)
+ ret = stm32_pwm_enable(priv, pwm->hwpwm);
+
+ return ret;
+}
+
+static const struct pwm_ops stm32pwm_ops = {
+ .owner = THIS_MODULE,
+ .apply = stm32_pwm_apply,
+};
+
+static int stm32_pwm_set_breakinput(struct stm32_pwm *priv,
+ int index, int level, int filter)
+{
+ u32 bke = (index == 0) ? TIM_BDTR_BKE : TIM_BDTR_BK2E;
+ int shift = (index == 0) ? TIM_BDTR_BKF_SHIFT : TIM_BDTR_BK2F_SHIFT;
+ u32 mask = (index == 0) ? TIM_BDTR_BKE | TIM_BDTR_BKP | TIM_BDTR_BKF
+ : TIM_BDTR_BK2E | TIM_BDTR_BK2P | TIM_BDTR_BK2F;
+ u32 bdtr = bke;
+
+ /*
+ * The both bits could be set since only one will be wrote
+ * due to mask value.
+ */
+ if (level)
+ bdtr |= TIM_BDTR_BKP | TIM_BDTR_BK2P;
+
+ bdtr |= (filter & TIM_BDTR_BKF_MASK) << shift;
+
+ regmap_update_bits(priv->regmap, TIM_BDTR, mask, bdtr);
+
+ regmap_read(priv->regmap, TIM_BDTR, &bdtr);
+
+ return (bdtr & bke) ? 0 : -EINVAL;
+}
+
+static int stm32_pwm_apply_breakinputs(struct stm32_pwm *priv,
+ struct device_node *np)
+{
+ struct stm32_breakinput breakinput[MAX_BREAKINPUT];
+ int nb, ret, i, array_size;
+
+ nb = of_property_count_elems_of_size(np, "st,breakinput",
+ sizeof(struct stm32_breakinput));
+
+ /*
+ * Because "st,breakinput" parameter is optional do not make probe
+ * failed if it doesn't exist.
+ */
+ if (nb <= 0)
+ return 0;
+
+ if (nb > MAX_BREAKINPUT)
+ return -EINVAL;
+
+ array_size = nb * sizeof(struct stm32_breakinput) / sizeof(u32);
+ ret = of_property_read_u32_array(np, "st,breakinput",
+ (u32 *)breakinput, array_size);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < nb && !ret; i++) {
+ ret = stm32_pwm_set_breakinput(priv,
+ breakinput[i].index,
+ breakinput[i].level,
+ breakinput[i].filter);
+ }
+
+ return ret;
+}
+
+static void stm32_pwm_detect_complementary(struct stm32_pwm *priv)
+{
+ u32 ccer;
+
+ /*
+ * If complementary bit doesn't exist writing 1 will have no
+ * effect so we can detect it.
+ */
+ regmap_update_bits(priv->regmap,
+ TIM_CCER, TIM_CCER_CC1NE, TIM_CCER_CC1NE);
+ regmap_read(priv->regmap, TIM_CCER, &ccer);
+ regmap_update_bits(priv->regmap, TIM_CCER, TIM_CCER_CC1NE, 0);
+
+ priv->have_complementary_output = (ccer != 0);
+}
+
+static int stm32_pwm_detect_channels(struct stm32_pwm *priv)
+{
+ u32 ccer;
+ int npwm = 0;
+
+ /*
+ * If channels enable bits don't exist writing 1 will have no
+ * effect so we can detect and count them.
+ */
+ regmap_update_bits(priv->regmap,
+ TIM_CCER, TIM_CCER_CCXE, TIM_CCER_CCXE);
+ regmap_read(priv->regmap, TIM_CCER, &ccer);
+ regmap_update_bits(priv->regmap, TIM_CCER, TIM_CCER_CCXE, 0);
+
+ if (ccer & TIM_CCER_CC1E)
+ npwm++;
+
+ if (ccer & TIM_CCER_CC2E)
+ npwm++;
+
+ if (ccer & TIM_CCER_CC3E)
+ npwm++;
+
+ if (ccer & TIM_CCER_CC4E)
+ npwm++;
+
+ return npwm;
+}
+
+static int stm32_pwm_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ struct stm32_timers *ddata = dev_get_drvdata(pdev->dev.parent);
+ struct stm32_pwm *priv;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->regmap = ddata->regmap;
+ priv->clk = ddata->clk;
+ priv->max_arr = ddata->max_arr;
+
+ if (!priv->regmap || !priv->clk)
+ return -EINVAL;
+
+ ret = stm32_pwm_apply_breakinputs(priv, np);
+ if (ret)
+ return ret;
+
+ stm32_pwm_detect_complementary(priv);
+
+ priv->chip.base = -1;
+ priv->chip.dev = dev;
+ priv->chip.ops = &stm32pwm_ops;
+ priv->chip.npwm = stm32_pwm_detect_channels(priv);
+
+ ret = pwmchip_add(&priv->chip);
+ if (ret < 0)
+ return ret;
+
+ platform_set_drvdata(pdev, priv);
+
+ return 0;
+}
+
+static int stm32_pwm_remove(struct platform_device *pdev)
+{
+ struct stm32_pwm *priv = platform_get_drvdata(pdev);
+ unsigned int i;
+
+ for (i = 0; i < priv->chip.npwm; i++)
+ pwm_disable(&priv->chip.pwms[i]);
+
+ pwmchip_remove(&priv->chip);
+
+ return 0;
+}
+
+static const struct of_device_id stm32_pwm_of_match[] = {
+ { .compatible = "st,stm32-pwm", },
+ { /* end node */ },
+};
+MODULE_DEVICE_TABLE(of, stm32_pwm_of_match);
+
+static struct platform_driver stm32_pwm_driver = {
+ .probe = stm32_pwm_probe,
+ .remove = stm32_pwm_remove,
+ .driver = {
+ .name = "stm32-pwm",
+ .of_match_table = stm32_pwm_of_match,
+ },
+};
+module_platform_driver(stm32_pwm_driver);
+
+MODULE_ALIAS("platform: stm32-pwm");
+MODULE_DESCRIPTION("STMicroelectronics STM32 PWM driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1
^ permalink raw reply related
* [PATCH v8 5/8] IIO: add bindings for STM32 timer trigger driver
From: Benjamin Gaignard @ 2017-01-18 14:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484749251-14445-1-git-send-email-benjamin.gaignard@st.com>
Define bindings for STM32 timer trigger
version 8:
- reword "reg" parameter description
version 4:
- remove triggers enumeration from DT
- add reg parameter
version 3:
- change file name
- add cross reference with mfd bindings
version 2:
- only keep one compatible
- add DT parameters to set lists of the triggers:
one list describe the triggers created by the device
another one give the triggers accepted by the device
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
Acked-by: Jonathan Cameron <jic23@kernel.org>
---
.../bindings/iio/timer/stm32-timer-trigger.txt | 23 ++++++++++++++++++++++
1 file changed, 23 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/timer/stm32-timer-trigger.txt
diff --git a/Documentation/devicetree/bindings/iio/timer/stm32-timer-trigger.txt b/Documentation/devicetree/bindings/iio/timer/stm32-timer-trigger.txt
new file mode 100644
index 0000000..55a653d
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/timer/stm32-timer-trigger.txt
@@ -0,0 +1,23 @@
+STMicroelectronics STM32 Timers IIO timer bindings
+
+Must be a sub-node of an STM32 Timers device tree node.
+See ../mfd/stm32-timers.txt for details about the parent node.
+
+Required parameters:
+- compatible: Must be "st,stm32-timer-trigger".
+- reg: Identify trigger hardware block.
+
+Example:
+ timers at 40010000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "st,stm32-timers";
+ reg = <0x40010000 0x400>;
+ clocks = <&rcc 0 160>;
+ clock-names = "clk_int";
+
+ timer at 0 {
+ compatible = "st,stm32-timer-trigger";
+ reg = <0>;
+ };
+ };
--
1.9.1
^ permalink raw reply related
* [PATCH v8 6/8] IIO: add STM32 timer trigger driver
From: Benjamin Gaignard @ 2017-01-18 14:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484749251-14445-1-git-send-email-benjamin.gaignard@st.com>
Timers IPs can be used to generate triggers for other IPs like
DAC or ADC.
Each trigger may result of timer internals signals like counter enable,
reset or edge, this configuration could be done through "master_mode"
device attribute.
Since triggers could be used by DAC or ADC their names are defined
in include/ nux/iio/timer/stm32-timer-trigger.h and is_stm32_iio_timer_trigger
function could be used to check if the trigger is valid or not.
"trgo" trigger have a "sampling_frequency" attribute which allow to configure
timer sampling frequency.
version 8:
- change kernel version from 4.10 to 4.11 in ABI documentation
version 7:
- remove all iio_device related code
- move driver into trigger directory
version 5:
- simplify tables of triggers
- only create an IIO device when needed
version 4:
- get triggers configuration from "reg" in DT
- add tables of triggers
- sampling frequency is enable/disable when writing in trigger
sampling_frequency attribute
- no more use of interruptions
version 3:
- change compatible to "st,stm32-timer-trigger"
- fix attributes access right
- use string instead of int for master_mode and slave_mode
- document device attributes in sysfs-bus-iio-timer-stm32
version 2:
- keep only one compatible
- use st,input-triggers-names and st,output-triggers-names
to know which triggers are accepted and/or create by the device
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
Acked-by: Jonathan Cameron <jic23@kernel.org>
---
.../ABI/testing/sysfs-bus-iio-timer-stm32 | 29 ++
drivers/iio/trigger/Kconfig | 9 +
drivers/iio/trigger/Makefile | 1 +
drivers/iio/trigger/stm32-timer-trigger.c | 342 +++++++++++++++++++++
include/linux/iio/timer/stm32-timer-trigger.h | 62 ++++
5 files changed, 443 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
create mode 100644 drivers/iio/trigger/stm32-timer-trigger.c
create mode 100644 include/linux/iio/timer/stm32-timer-trigger.h
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
new file mode 100644
index 0000000..6534a60
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
@@ -0,0 +1,29 @@
+What: /sys/bus/iio/devices/triggerX/master_mode_available
+KernelVersion: 4.11
+Contact: benjamin.gaignard at st.com
+Description:
+ Reading returns the list possible master modes which are:
+ - "reset" : The UG bit from the TIMx_EGR register is used as trigger output (TRGO).
+ - "enable" : The Counter Enable signal CNT_EN is used as trigger output.
+ - "update" : The update event is selected as trigger output.
+ For instance a master timer can then be used as a prescaler for a slave timer.
+ - "compare_pulse" : The trigger output send a positive pulse when the CC1IF flag is to be set.
+ - "OC1REF" : OC1REF signal is used as trigger output.
+ - "OC2REF" : OC2REF signal is used as trigger output.
+ - "OC3REF" : OC3REF signal is used as trigger output.
+ - "OC4REF" : OC4REF signal is used as trigger output.
+
+What: /sys/bus/iio/devices/triggerX/master_mode
+KernelVersion: 4.11
+Contact: benjamin.gaignard at st.com
+Description:
+ Reading returns the current master modes.
+ Writing set the master mode
+
+What: /sys/bus/iio/devices/triggerX/sampling_frequency
+KernelVersion: 4.11
+Contact: benjamin.gaignard at st.com
+Description:
+ Reading returns the current sampling frequency.
+ Writing an value different of 0 set and start sampling.
+ Writing 0 stop sampling.
diff --git a/drivers/iio/trigger/Kconfig b/drivers/iio/trigger/Kconfig
index 809b2e7..e4d4e63 100644
--- a/drivers/iio/trigger/Kconfig
+++ b/drivers/iio/trigger/Kconfig
@@ -24,6 +24,15 @@ config IIO_INTERRUPT_TRIGGER
To compile this driver as a module, choose M here: the
module will be called iio-trig-interrupt.
+config IIO_STM32_TIMER_TRIGGER
+ tristate "STM32 Timer Trigger"
+ depends on (ARCH_STM32 && OF && MFD_STM32_TIMERS) || COMPILE_TEST
+ help
+ Select this option to enable STM32 Timer Trigger
+
+ To compile this driver as a module, choose M here: the
+ module will be called stm32-timer-trigger.
+
config IIO_TIGHTLOOP_TRIGGER
tristate "A kthread based hammering loop trigger"
depends on IIO_SW_TRIGGER
diff --git a/drivers/iio/trigger/Makefile b/drivers/iio/trigger/Makefile
index aab4dc2..5c4ecd3 100644
--- a/drivers/iio/trigger/Makefile
+++ b/drivers/iio/trigger/Makefile
@@ -6,5 +6,6 @@
obj-$(CONFIG_IIO_HRTIMER_TRIGGER) += iio-trig-hrtimer.o
obj-$(CONFIG_IIO_INTERRUPT_TRIGGER) += iio-trig-interrupt.o
+obj-$(CONFIG_IIO_STM32_TIMER_TRIGGER) += stm32-timer-trigger.o
obj-$(CONFIG_IIO_SYSFS_TRIGGER) += iio-trig-sysfs.o
obj-$(CONFIG_IIO_TIGHTLOOP_TRIGGER) += iio-trig-loop.o
diff --git a/drivers/iio/trigger/stm32-timer-trigger.c b/drivers/iio/trigger/stm32-timer-trigger.c
new file mode 100644
index 0000000..994b96d
--- /dev/null
+++ b/drivers/iio/trigger/stm32-timer-trigger.c
@@ -0,0 +1,342 @@
+/*
+ * Copyright (C) STMicroelectronics 2016
+ *
+ * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
+ *
+ * License terms: GNU General Public License (GPL), version 2
+ */
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/timer/stm32-timer-trigger.h>
+#include <linux/iio/trigger.h>
+#include <linux/mfd/stm32-timers.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#define MAX_TRIGGERS 6
+
+/* List the triggers created by each timer */
+static const void *triggers_table[][MAX_TRIGGERS] = {
+ { TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4,},
+ { TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4,},
+ { TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4,},
+ { TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4,},
+ { TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4,},
+ { TIM6_TRGO,},
+ { TIM7_TRGO,},
+ { TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4,},
+ { TIM9_TRGO, TIM9_CH1, TIM9_CH2,},
+ { }, /* timer 10 */
+ { }, /* timer 11 */
+ { TIM12_TRGO, TIM12_CH1, TIM12_CH2,},
+};
+
+struct stm32_timer_trigger {
+ struct device *dev;
+ struct regmap *regmap;
+ struct clk *clk;
+ u32 max_arr;
+ const void *triggers;
+};
+
+static int stm32_timer_start(struct stm32_timer_trigger *priv,
+ unsigned int frequency)
+{
+ unsigned long long prd, div;
+ int prescaler = 0;
+ u32 ccer, cr1;
+
+ /* Period and prescaler values depends of clock rate */
+ div = (unsigned long long)clk_get_rate(priv->clk);
+
+ do_div(div, frequency);
+
+ prd = div;
+
+ /*
+ * Increase prescaler value until we get a result that fit
+ * with auto reload register maximum value.
+ */
+ while (div > priv->max_arr) {
+ prescaler++;
+ div = prd;
+ do_div(div, (prescaler + 1));
+ }
+ prd = div;
+
+ if (prescaler > MAX_TIM_PSC) {
+ dev_err(priv->dev, "prescaler exceeds the maximum value\n");
+ return -EINVAL;
+ }
+
+ /* Check if nobody else use the timer */
+ regmap_read(priv->regmap, TIM_CCER, &ccer);
+ if (ccer & TIM_CCER_CCXE)
+ return -EBUSY;
+
+ regmap_read(priv->regmap, TIM_CR1, &cr1);
+ if (!(cr1 & TIM_CR1_CEN))
+ clk_enable(priv->clk);
+
+ regmap_write(priv->regmap, TIM_PSC, prescaler);
+ regmap_write(priv->regmap, TIM_ARR, prd - 1);
+ regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
+
+ /* Force master mode to update mode */
+ regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS, 0x20);
+
+ /* Make sure that registers are updated */
+ regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
+
+ /* Enable controller */
+ regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, TIM_CR1_CEN);
+
+ return 0;
+}
+
+static void stm32_timer_stop(struct stm32_timer_trigger *priv)
+{
+ u32 ccer, cr1;
+
+ regmap_read(priv->regmap, TIM_CCER, &ccer);
+ if (ccer & TIM_CCER_CCXE)
+ return;
+
+ regmap_read(priv->regmap, TIM_CR1, &cr1);
+ if (cr1 & TIM_CR1_CEN)
+ clk_disable(priv->clk);
+
+ /* Stop timer */
+ regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, 0);
+ regmap_write(priv->regmap, TIM_PSC, 0);
+ regmap_write(priv->regmap, TIM_ARR, 0);
+
+ /* Make sure that registers are updated */
+ regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
+}
+
+static ssize_t stm32_tt_store_frequency(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct iio_trigger *trig = to_iio_trigger(dev);
+ struct stm32_timer_trigger *priv = iio_trigger_get_drvdata(trig);
+ unsigned int freq;
+ int ret;
+
+ ret = kstrtouint(buf, 10, &freq);
+ if (ret)
+ return ret;
+
+ if (freq == 0) {
+ stm32_timer_stop(priv);
+ } else {
+ ret = stm32_timer_start(priv, freq);
+ if (ret)
+ return ret;
+ }
+
+ return len;
+}
+
+static ssize_t stm32_tt_read_frequency(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct iio_trigger *trig = to_iio_trigger(dev);
+ struct stm32_timer_trigger *priv = iio_trigger_get_drvdata(trig);
+ u32 psc, arr, cr1;
+ unsigned long long freq = 0;
+
+ regmap_read(priv->regmap, TIM_CR1, &cr1);
+ regmap_read(priv->regmap, TIM_PSC, &psc);
+ regmap_read(priv->regmap, TIM_ARR, &arr);
+
+ if (psc && arr && (cr1 & TIM_CR1_CEN)) {
+ freq = (unsigned long long)clk_get_rate(priv->clk);
+ do_div(freq, psc);
+ do_div(freq, arr);
+ }
+
+ return sprintf(buf, "%d\n", (unsigned int)freq);
+}
+
+static IIO_DEV_ATTR_SAMP_FREQ(0660,
+ stm32_tt_read_frequency,
+ stm32_tt_store_frequency);
+
+static char *master_mode_table[] = {
+ "reset",
+ "enable",
+ "update",
+ "compare_pulse",
+ "OC1REF",
+ "OC2REF",
+ "OC3REF",
+ "OC4REF"
+};
+
+static ssize_t stm32_tt_show_master_mode(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+ u32 cr2;
+
+ regmap_read(priv->regmap, TIM_CR2, &cr2);
+ cr2 = (cr2 & TIM_CR2_MMS) >> TIM_CR2_MMS_SHIFT;
+
+ return snprintf(buf, PAGE_SIZE, "%s\n", master_mode_table[cr2]);
+}
+
+static ssize_t stm32_tt_store_master_mode(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct stm32_timer_trigger *priv = iio_priv(indio_dev);
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(master_mode_table); i++) {
+ if (!strncmp(master_mode_table[i], buf,
+ strlen(master_mode_table[i]))) {
+ regmap_update_bits(priv->regmap, TIM_CR2,
+ TIM_CR2_MMS, i << TIM_CR2_MMS_SHIFT);
+ /* Make sure that registers are updated */
+ regmap_update_bits(priv->regmap, TIM_EGR,
+ TIM_EGR_UG, TIM_EGR_UG);
+ return len;
+ }
+ }
+
+ return -EINVAL;
+}
+
+static IIO_CONST_ATTR(master_mode_available,
+ "reset enable update compare_pulse OC1REF OC2REF OC3REF OC4REF");
+
+static IIO_DEVICE_ATTR(master_mode, 0660,
+ stm32_tt_show_master_mode,
+ stm32_tt_store_master_mode,
+ 0);
+
+static struct attribute *stm32_trigger_attrs[] = {
+ &iio_dev_attr_sampling_frequency.dev_attr.attr,
+ &iio_dev_attr_master_mode.dev_attr.attr,
+ &iio_const_attr_master_mode_available.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group stm32_trigger_attr_group = {
+ .attrs = stm32_trigger_attrs,
+};
+
+static const struct attribute_group *stm32_trigger_attr_groups[] = {
+ &stm32_trigger_attr_group,
+ NULL,
+};
+
+static const struct iio_trigger_ops timer_trigger_ops = {
+ .owner = THIS_MODULE,
+};
+
+static int stm32_setup_iio_triggers(struct stm32_timer_trigger *priv)
+{
+ int ret;
+ const char * const *cur = priv->triggers;
+
+ while (cur && *cur) {
+ struct iio_trigger *trig;
+
+ trig = devm_iio_trigger_alloc(priv->dev, "%s", *cur);
+ if (!trig)
+ return -ENOMEM;
+
+ trig->dev.parent = priv->dev->parent;
+ trig->ops = &timer_trigger_ops;
+
+ /*
+ * sampling frequency and master mode attributes
+ * should only be available on trgo trigger which
+ * is always the first in the list.
+ */
+ if (cur == priv->triggers)
+ trig->dev.groups = stm32_trigger_attr_groups;
+
+ iio_trigger_set_drvdata(trig, priv);
+
+ ret = devm_iio_trigger_register(priv->dev, trig);
+ if (ret)
+ return ret;
+ cur++;
+ }
+
+ return 0;
+}
+
+/**
+ * is_stm32_timer_trigger
+ * @trig: trigger to be checked
+ *
+ * return true if the trigger is a valid stm32 iio timer trigger
+ * either return false
+ */
+bool is_stm32_timer_trigger(struct iio_trigger *trig)
+{
+ return (trig->ops == &timer_trigger_ops);
+}
+EXPORT_SYMBOL(is_stm32_timer_trigger);
+
+static int stm32_timer_trigger_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct stm32_timer_trigger *priv;
+ struct stm32_timers *ddata = dev_get_drvdata(pdev->dev.parent);
+ unsigned int index;
+ int ret;
+
+ if (of_property_read_u32(dev->of_node, "reg", &index))
+ return -EINVAL;
+
+ if (index >= ARRAY_SIZE(triggers_table))
+ return -EINVAL;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+
+ if (!priv)
+ return -ENOMEM;
+
+ priv->dev = dev;
+ priv->regmap = ddata->regmap;
+ priv->clk = ddata->clk;
+ priv->max_arr = ddata->max_arr;
+ priv->triggers = triggers_table[index];
+
+ ret = stm32_setup_iio_triggers(priv);
+ if (ret)
+ return ret;
+
+ platform_set_drvdata(pdev, priv);
+
+ return 0;
+}
+
+static const struct of_device_id stm32_trig_of_match[] = {
+ { .compatible = "st,stm32-timer-trigger", },
+ { /* end node */ },
+};
+MODULE_DEVICE_TABLE(of, stm32_trig_of_match);
+
+static struct platform_driver stm32_timer_trigger_driver = {
+ .probe = stm32_timer_trigger_probe,
+ .driver = {
+ .name = "stm32-timer-trigger",
+ .of_match_table = stm32_trig_of_match,
+ },
+};
+module_platform_driver(stm32_timer_trigger_driver);
+
+MODULE_ALIAS("platform: stm32-timer-trigger");
+MODULE_DESCRIPTION("STMicroelectronics STM32 Timer Trigger driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/iio/timer/stm32-timer-trigger.h b/include/linux/iio/timer/stm32-timer-trigger.h
new file mode 100644
index 0000000..55535ae
--- /dev/null
+++ b/include/linux/iio/timer/stm32-timer-trigger.h
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) STMicroelectronics 2016
+ *
+ * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
+ *
+ * License terms: GNU General Public License (GPL), version 2
+ */
+
+#ifndef _STM32_TIMER_TRIGGER_H_
+#define _STM32_TIMER_TRIGGER_H_
+
+#define TIM1_TRGO "tim1_trgo"
+#define TIM1_CH1 "tim1_ch1"
+#define TIM1_CH2 "tim1_ch2"
+#define TIM1_CH3 "tim1_ch3"
+#define TIM1_CH4 "tim1_ch4"
+
+#define TIM2_TRGO "tim2_trgo"
+#define TIM2_CH1 "tim2_ch1"
+#define TIM2_CH2 "tim2_ch2"
+#define TIM2_CH3 "tim2_ch3"
+#define TIM2_CH4 "tim2_ch4"
+
+#define TIM3_TRGO "tim3_trgo"
+#define TIM3_CH1 "tim3_ch1"
+#define TIM3_CH2 "tim3_ch2"
+#define TIM3_CH3 "tim3_ch3"
+#define TIM3_CH4 "tim3_ch4"
+
+#define TIM4_TRGO "tim4_trgo"
+#define TIM4_CH1 "tim4_ch1"
+#define TIM4_CH2 "tim4_ch2"
+#define TIM4_CH3 "tim4_ch3"
+#define TIM4_CH4 "tim4_ch4"
+
+#define TIM5_TRGO "tim5_trgo"
+#define TIM5_CH1 "tim5_ch1"
+#define TIM5_CH2 "tim5_ch2"
+#define TIM5_CH3 "tim5_ch3"
+#define TIM5_CH4 "tim5_ch4"
+
+#define TIM6_TRGO "tim6_trgo"
+
+#define TIM7_TRGO "tim7_trgo"
+
+#define TIM8_TRGO "tim8_trgo"
+#define TIM8_CH1 "tim8_ch1"
+#define TIM8_CH2 "tim8_ch2"
+#define TIM8_CH3 "tim8_ch3"
+#define TIM8_CH4 "tim8_ch4"
+
+#define TIM9_TRGO "tim9_trgo"
+#define TIM9_CH1 "tim9_ch1"
+#define TIM9_CH2 "tim9_ch2"
+
+#define TIM12_TRGO "tim12_trgo"
+#define TIM12_CH1 "tim12_ch1"
+#define TIM12_CH2 "tim12_ch2"
+
+bool is_stm32_timer_trigger(struct iio_trigger *trig);
+
+#endif
--
1.9.1
^ permalink raw reply related
* [PATCH v8 7/8] ARM: dts: stm32: add Timers driver for stm32f429 MCU
From: Benjamin Gaignard @ 2017-01-18 14:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484749251-14445-1-git-send-email-benjamin.gaignard@st.com>
Add Timers and it sub-nodes into DT for stm32f429 family.
version 6:
- split patch in two: one for SoC family and one for stm32f469
discovery board.
version 5:
- rename gptimer node to timers
- re-order timers node par addresses
version 4:
- remove unwanted indexing in pwm@ and timer@ node name
- use "reg" instead of additional parameters to set timer
configuration
version 3:
- use "st,stm32-timer-trigger" in DT
version 2:
- use parameters to describe hardware capabilities
- do not use references for pwm and iio timer subnodes
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
arch/arm/boot/dts/stm32f429.dtsi | 275 +++++++++++++++++++++++++++++++++++++++
1 file changed, 275 insertions(+)
diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index e4dae0e..2f85691 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -355,6 +355,21 @@
slew-rate = <2>;
};
};
+
+ pwm1_pins: pwm at 1 {
+ pins {
+ pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
+ <STM32F429_PB13_FUNC_TIM1_CH1N>,
+ <STM32F429_PB12_FUNC_TIM1_BKIN>;
+ };
+ };
+
+ pwm3_pins: pwm at 3 {
+ pins {
+ pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
+ <STM32F429_PB5_FUNC_TIM3_CH2>;
+ };
+ };
};
rcc: rcc at 40023810 {
@@ -426,6 +441,266 @@
interrupts = <80>;
clocks = <&rcc 0 38>;
};
+
+ timers2: timers at 40000000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "st,stm32-timers";
+ reg = <0x40000000 0x400>;
+ clocks = <&rcc 0 128>;
+ clock-names = "int";
+ status = "disabled";
+
+ pwm {
+ compatible = "st,stm32-pwm";
+ status = "disabled";
+ };
+
+ timer at 1 {
+ compatible = "st,stm32-timer-trigger";
+ reg = <1>;
+ status = "disabled";
+ };
+ };
+
+ timers3: timers at 40000400 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "st,stm32-timers";
+ reg = <0x40000400 0x400>;
+ clocks = <&rcc 0 129>;
+ clock-names = "int";
+ status = "disabled";
+
+ pwm {
+ compatible = "st,stm32-pwm";
+ status = "disabled";
+ };
+
+ timer at 2 {
+ compatible = "st,stm32-timer-trigger";
+ reg = <2>;
+ status = "disabled";
+ };
+ };
+
+ timers4: timers at 40000800 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "st,stm32-timers";
+ reg = <0x40000800 0x400>;
+ clocks = <&rcc 0 130>;
+ clock-names = "int";
+ status = "disabled";
+
+ pwm {
+ compatible = "st,stm32-pwm";
+ status = "disabled";
+ };
+
+ timer at 3 {
+ compatible = "st,stm32-timer-trigger";
+ reg = <3>;
+ status = "disabled";
+ };
+ };
+
+ timers5: timers at 40000c00 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "st,stm32-timers";
+ reg = <0x40000C00 0x400>;
+ clocks = <&rcc 0 131>;
+ clock-names = "int";
+ status = "disabled";
+
+ pwm {
+ compatible = "st,stm32-pwm";
+ status = "disabled";
+ };
+
+ timer at 4 {
+ compatible = "st,stm32-timer-trigger";
+ reg = <4>;
+ status = "disabled";
+ };
+ };
+
+ timers6: timers at 40001000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "st,stm32-timers";
+ reg = <0x40001000 0x400>;
+ clocks = <&rcc 0 132>;
+ clock-names = "int";
+ status = "disabled";
+
+ timer at 5 {
+ compatible = "st,stm32-timer-trigger";
+ reg = <5>;
+ status = "disabled";
+ };
+ };
+
+ timers7: timers at 40001400 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "st,stm32-timers";
+ reg = <0x40001400 0x400>;
+ clocks = <&rcc 0 133>;
+ clock-names = "int";
+ status = "disabled";
+
+ timer at 6 {
+ compatible = "st,stm32-timer-trigger";
+ reg = <6>;
+ status = "disabled";
+ };
+ };
+
+ timers12: timers at 40001800 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "st,stm32-timers";
+ reg = <0x40001800 0x400>;
+ clocks = <&rcc 0 134>;
+ clock-names = "int";
+ status = "disabled";
+
+ pwm {
+ compatible = "st,stm32-pwm";
+ status = "disabled";
+ };
+
+ timer at 11 {
+ compatible = "st,stm32-timer-trigger";
+ reg = <11>;
+ status = "disabled";
+ };
+ };
+
+ timers13: timers at 40001c00 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "st,stm32-timers";
+ reg = <0x40001C00 0x400>;
+ clocks = <&rcc 0 135>;
+ clock-names = "int";
+ status = "disabled";
+
+ pwm {
+ compatible = "st,stm32-pwm";
+ status = "disabled";
+ };
+ };
+
+ timers14: timers at 40002000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "st,stm32-timers";
+ reg = <0x40002000 0x400>;
+ clocks = <&rcc 0 136>;
+ clock-names = "int";
+ status = "disabled";
+
+ pwm {
+ compatible = "st,stm32-pwm";
+ status = "disabled";
+ };
+ };
+
+ timers1: timers at 40010000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "st,stm32-timers";
+ reg = <0x40010000 0x400>;
+ clocks = <&rcc 0 160>;
+ clock-names = "int";
+ status = "disabled";
+
+ pwm {
+ compatible = "st,stm32-pwm";
+ status = "disabled";
+ };
+
+ timer at 0 {
+ compatible = "st,stm32-timer-trigger";
+ reg = <0>;
+ status = "disabled";
+ };
+ };
+
+ timers8: timers at 40010400 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "st,stm32-timers";
+ reg = <0x40010400 0x400>;
+ clocks = <&rcc 0 161>;
+ clock-names = "int";
+ status = "disabled";
+
+ pwm {
+ compatible = "st,stm32-pwm";
+ status = "disabled";
+ };
+
+ timer at 7 {
+ compatible = "st,stm32-timer-trigger";
+ reg = <7>;
+ status = "disabled";
+ };
+ };
+
+ timers9: timers at 40014000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "st,stm32-timers";
+ reg = <0x40014000 0x400>;
+ clocks = <&rcc 0 176>;
+ clock-names = "int";
+ status = "disabled";
+
+ pwm {
+ compatible = "st,stm32-pwm";
+ status = "disabled";
+ };
+
+ timer at 8 {
+ compatible = "st,stm32-timer-trigger";
+ reg = <8>;
+ status = "disabled";
+ };
+ };
+
+ timers10: timers at 40014400 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "st,stm32-timers";
+ reg = <0x40014400 0x400>;
+ clocks = <&rcc 0 177>;
+ clock-names = "int";
+ status = "disabled";
+
+ pwm {
+ compatible = "st,stm32-pwm";
+ status = "disabled";
+ };
+ };
+
+ timers11: timers at 40014800 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "st,stm32-timers";
+ reg = <0x40014800 0x400>;
+ clocks = <&rcc 0 178>;
+ clock-names = "int";
+ status = "disabled";
+
+ pwm {
+ compatible = "st,stm32-pwm";
+ status = "disabled";
+ };
+ };
};
};
--
1.9.1
^ permalink raw reply related
* [PATCH v8 8/8] ARM: dts: stm32: Enable pwm1 and pwm3 for stm32f469-disco
From: Benjamin Gaignard @ 2017-01-18 14:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484749251-14445-1-git-send-email-benjamin.gaignard@st.com>
Define and enable pwm1 and pwm3 for stm32f469 discovery board
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
arch/arm/boot/dts/stm32f469-disco.dts | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
index 8a163d7..92552d3 100644
--- a/arch/arm/boot/dts/stm32f469-disco.dts
+++ b/arch/arm/boot/dts/stm32f469-disco.dts
@@ -81,3 +81,31 @@
&usart3 {
status = "okay";
};
+
+&timers1 {
+ status = "okay";
+
+ pwm {
+ pinctrl-0 = <&pwm1_pins>;
+ pinctrl-names = "default";
+ status = "okay";
+ };
+
+ timer at 0 {
+ status = "okay";
+ };
+};
+
+&timers3 {
+ status = "okay";
+
+ pwm {
+ pinctrl-0 = <&pwm3_pins>;
+ pinctrl-names = "default";
+ status = "okay";
+ };
+
+ timer at 2 {
+ status = "okay";
+ };
+};
--
1.9.1
^ permalink raw reply related
* CONFIG_PCIEASPM breaks PCIe on Marvell Armada 385 machine
From: Bjorn Helgaas @ 2017-01-18 14:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <e0ccb7a0-9cc7-c595-7f06-e63c0927f36d@caviumnetworks.com>
On Tue, Jan 17, 2017 at 03:37:10PM -0800, David Daney wrote:
> On 01/17/2017 02:22 PM, Bjorn Helgaas wrote:
> >[+cc David]
> >
> >On Tue, Jan 17, 2017 at 09:02:58PM +0000, Russell King - ARM Linux wrote:
> >>On Tue, Jan 17, 2017 at 07:34:14PM +0000, Russell King - ARM Linux wrote:
> >>>Uwe, can you try:
> >>>
> >>>setpci -s <whatever-the-id-of-the-root-is-it's-blanked-out-in-the-above> \
> >>> 0x50.w=0x60
> >>>
> >>>and see whether it remains alive (you can check by reading the root
> >>>register 0x52.w - bit 12 should be set once bit 11 clears again.
> >>
> >>For reference, this I got wrong...
> >>
> >>0xf1041a04 bit 0 indicates link status (0 = link up, 1 = link down).
> >>
> >>>If that's successful, maybe setting the common clock bit on the PCIe
> >>>device is what's causing the problem, in which case:
> >>>
> >>>setpci -s 02:00.0 0x80.w=0x40
> >>>setpci -s <whatever-the-id-of-the-root-is-it's-blanked-out-in-the-above> \
> >>> 0x50.w=0x60
> >>
> >>Having worked with Uwe over IRC, it seems that any request to retrain
> >>causes the link to go down, either with or without the common clock bit
> >>set:
> >>
> >># setpci -s 2.0 0x50.w=0x60
> >># setpci -s 2.0 0x52.w
> >>0011
> >># memtool md 0xf1041a04+4
> >>f1041a04: 00010201
> >>... reboot ...
> >># setpci -s 2.0 0x50.w=0x20
> >># memtool md 0xf1041a04+4
> >>f1041a04: 00010201
> >>
> >>which doesn't point towards ASPM itself, but the problem is caused by
> >>a side effect of ASPM's setup code which always triggers a retrain.
> >>
> >>Bit 5 in that register is documented (at least in the Armada 370 docs
> >>and Armada XP docs I have) as:
> >>
> >>5 RetrnLnk RW Retrain Link
> >> 0x0 This bit forces the device to initiate link retraining.
> >> Always returns 0 when read.
> >> NOTE: If configured as an Endpoint, this field is
> >> reserved and has no effect.
> >>
> >>Bjorn, are you aware of similar situations where a request for the PCIe
> >>link to be retrained causes it to fail?
>
>
> Link (re)training can fail for several reasons including, but not
> limited to:
>
> - Poor signal propagation through the
> chips/packages/boards/connectors, also known as Signal Integrity
> (SI) problmes.
>
> - Incorrect implementation, in hardware, of link training protocols
> at either end of the link
>
> Usually, system and PCIe device vendors do a lot of testing and
> signal analysis across a variety of configurations with the end goal
> being that PCIe looks like a bullet-proof interconnect to the end
> consumer.
>
> Unfortunatly, sometimes it doesn't work. In these cases, the
> vendors of the devices on each end of the link tend to point fingers
> at the link partner for being detective in some way.
>
> This patch:
>
> >
> >The only one that comes to mind is this patch from David (CC'd) that
> >avoids ASPM-related retrains when we know the link doesn't support ASPM:
> >http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e53f9a28bee3
> >
>
> Is an attempt to work around the problem from the system (host) end.
> If the system vendor knows a priori that a defective PCIe device is
> present in the system, the PCIe root port can be configured to
> indicate no ASPM is supported, resulting (with the patch) in no link
> retraining being attempted.
>
> To me it feels that we need a black list of devices that fail at a
> high rate in the link retraining, that when encountered would
> disable ASPM on the link where they reside.
I should have asked you for details about the defective devices
related to e53f9a28bee3 :) If we had included that in the changelog,
we would have something to seed a blacklist with.
There are several situations other than ASPM where link retraining is
required per spec (rate change, error handling, etc), and I guess we'd
have to avoid all of them. So I suppose e53f9a28bee3 avoids the most
obvious failures, but maybe we could still see issues in those other
cases.
Bjorn
^ permalink raw reply
* [RFC PATCH v5 1/7] dma: Take into account dma_pfn_offset
From: Vladimir Murzin @ 2017-01-18 14:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170118134447.GG9529@suse.de>
On 18/01/17 13:47, Joerg Roedel wrote:
> On Wed, Jan 18, 2017 at 11:13:17AM +0000, Vladimir Murzin wrote:
>> Even though dma-noop-ops assumes 1:1 memory mapping DMA memory range
>> can be different to RAM. For example, ARM STM32F4 MCU offers the
>> possibility to remap SDRAM from 0xc000_0000 to 0x0 to get CPU
>> performance boost, but DMA continue to see SDRAM at 0xc000_0000. This
>> difference in mapping is handled via device-tree "dma-range" property
>> which leads to dev->dma_pfn_offset is set nonzero. To handle such
>> cases take dma_pfn_offset into account.
>>
>> Cc: Joerg Roedel <jroedel@suse.de>
>> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>> Reported-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>> ---
>> lib/dma-noop.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/dma-noop.c b/lib/dma-noop.c
>> index 3d766e7..a14eee5 100644
>> --- a/lib/dma-noop.c
>> +++ b/lib/dma-noop.c
>> @@ -7,6 +7,7 @@
>> #include <linux/mm.h>
>> #include <linux/dma-mapping.h>
>> #include <linux/scatterlist.h>
>> +#include <linux/pfn.h>
>>
>> static void *dma_noop_alloc(struct device *dev, size_t size,
>> dma_addr_t *dma_handle, gfp_t gfp,
>> @@ -16,7 +17,8 @@ static void *dma_noop_alloc(struct device *dev, size_t size,
>>
>> ret = (void *)__get_free_pages(gfp, get_order(size));
>> if (ret)
>> - *dma_handle = virt_to_phys(ret);
>> + *dma_handle = virt_to_phys(ret) - PFN_PHYS(dev->dma_pfn_offset);
>> +
>
> If you need to do a '-' operation here, the offset is basically a
> cpu_pfn_offset for the device. Is that correct?
>
dma_pfn_offset is calculated as PFN_DOWN(paddr - dma_addr) in
of_dma_configure(), it is why '-' is used here.
Cheers
Vladimir
>
> Joerg
>
>
^ permalink raw reply
* [PATCH 3/5] ARM64: dts: meson-gxbb-p20x: add reset for the ethernet PHY
From: Martin Blumenstingl @ 2017-01-18 14:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484735769.26620.69.camel@baylibre.com>
On Wed, Jan 18, 2017 at 11:36 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> On Tue, 2017-01-17 at 22:26 +0100, Martin Blumenstingl wrote:
>> On Tue, Jan 17, 2017 at 8:22 PM, Jerome Brunet <jbrunet@baylibre.com>
>> wrote:
>> >
>> > On Sat, 2016-12-03 at 00:47 +0100, Martin Blumenstingl wrote:
>> > >
>> > > This resets the ethernet PHY during boot to get the PHY into a
>> > > "clean"
>> > > state. While here also specify the phy-handle of the ethmac node
>> > > to
>> > > make the PHY configuration similar to the one we have on GXL
>> > > devices.
>> > >
>> > > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemai
>> > > l.co
>> > > m>
>> > > Tested-by: Neil Armstrong <narmstrong@baylibre.com>
>> > > ---
>> > > arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 15
>> > > +++++++++++++++
>> > > 1 file changed, 15 insertions(+)
>> > >
>> > > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
>> > > b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
>> > > index 203be28..2abc553 100644
>> > > --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
>> > > +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
>> > > @@ -134,10 +134,25 @@
>> > > pinctrl-names = "default";
>> > > };
>> > >
>> > > +&mdio0 {
>> > > + ethernet_phy0: ethernet-phy at 0 {
>> > > + compatible = "ethernet-phy-ieee802.3-c22";
>> > > + reg = <0>;
>> >
>> > Sorry for the late reply.
>> > I just tried on the p200 and this patch (serie) breaks the network
>> > on
>> > it. The PHY is not detected anymore.
>> >
>> > From the KSZ9031 Datasheet : "PHY Address 0h is supported as the
>> > unique
>> > PHY address only; it is not supported as the broadcast PHY address
>> > [...]"
>> >
>> > So we can't just use the broadcast address here:
>> > reg should be <3>.
>> OK, I'll fix that in a follow-up. as mentioned in the other thread:
>> can you confirm that the PHY ID is 0x00221620?
>
> That's right.
thanks for confirming this
>> It seems that I also broke meson-gxbb-nexbox-a95x.dts with that
>> series
>> (no idea why that slipped through): according to the photos from
>> Neil's wiki [0] this seems to use an IC+ 10/100 ethernet PHY
>> (probably
>> an IP101A)
>
> IP101GR from what I can see on the board.
thanks :)
>>
>> >
>> > >
>> > > + };
>> > > +};
>> > > +
>> > > ðmac {
>> > > status = "okay";
>> > > pinctrl-0 = <ð_rgmii_pins>;
>> > > pinctrl-names = "default";
>> > > +
>> > > + phy-handle = <ðernet_phy0>;
>> > > +
>> > > + snps,reset-gpio = <&gpio GPIOZ_14 0>;
>> > > + snps,reset-delays-us = <0 10000 1000000>;
>> > > + snps,reset-active-low;
>> > > +
>> > > + phy-mode = "rgmii";
>> >
>> > We can't define this in p20x. actually the p201 uses an rmii.
>> > I have not idea about gpio reset, or the phy address for the p201.
>> >
>> > I suppose it would be better to move this to meson-gxbb_p200.dts
>> >
>> > I don't know if anybody has a p201, but until we can confirm a
>> > working
>> > Ethernet configuration, we should probably drop it for the p201
>> >
>> > Of course the problem was already there before this patch ...
>> indeed, that seems to be a problem.
>> the GXBB Nexbox A95x I mentioned above is based on the p201 board.
>> Amlogic's .dts also toggles GPIOZ_14 for it: [1]
>> chances are high that it also features an IP101A PHY (this is pure
>> speculation though).
>
> Agreed, chances are high it'll be this way.
>
>>
>> I propose four patches to fix all this situation:
>> - add the GPIOZ_14 reset and an ethernet_phy0 (with reg = <0>) to
>> meson-gxbb-nexbox-a95x.dts
>> - add the ethernet_phy0 node with reg = <0> (and ideally I also want
>> to include the KSZ9031 PHY ID) to meson-gxbb-p200.dtsi along with
>> phy-mode = "rgmii"
>
> you mean with reg = <3> ?
you're right, another typo there!
>> - add the ethernet_phy0 node with reg = <0> (without any PHY ID as we
>> don't know which one is used) to meson-gxbb-p201.dtsi along with
>> phy-mode = "rgmii"
>> - remove "phy-mode" and ethernet_phy0 from meson-gxbb-p20x.dtsi and
>> add a comment that both, the RGMII and the RMII PHY have GPIOZ_14
>> connected to their reset line
>
> I'm not too sure about these last 2 points. As you mentioned, we are
> only speculating regarding the p201. I would prefer to avoid putting
> things we are not sure about in the DT (as much as possible).
>
> We could disable the ethernet for p201:
> 1) if nobody complains, situation remains unchanged, ethernet is not
> working on that board
> 2) if someone complains, then we will be able to some facts from
> him/her, and get it working properly.
>
> After all, that's not a regression. With what we had in the meson-gxbb-
> p20x.dtsi so far, ethernet never worked on that board.
fine for me as well
^ permalink raw reply
* [PATCH] [media] s5p-mfc: Align stream buffer and CPB buffer to 512
From: Andrzej Hajda @ 2017-01-18 14:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484732223-24670-1-git-send-email-smitha.t@samsung.com>
Hi Smitha,
On 18.01.2017 10:37, Smitha T Murthy wrote:
> >From MFCv6 onwards encoder stream buffer and decoder CPB buffer
Unexpected char at the beginning.
> need to be aligned with 512.
Patch below adds checks only if buffer size is multiple of 512, am I right?
If yes, please precise the subject, for example "...CPB buffer size need
to be...".
>
> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> ---
> drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 9 +++++++++
> drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h | 3 +++
> 2 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> index d6f207e..57da798 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> @@ -408,8 +408,15 @@ static int s5p_mfc_set_dec_stream_buffer_v6(struct s5p_mfc_ctx *ctx,
> struct s5p_mfc_dev *dev = ctx->dev;
> const struct s5p_mfc_regs *mfc_regs = dev->mfc_regs;
> struct s5p_mfc_buf_size *buf_size = dev->variant->buf_size;
> + size_t cpb_buf_size;
>
> mfc_debug_enter();
> + cpb_buf_size = ALIGN(buf_size->cpb, CPB_ALIGN);
Since buf_size->cpb is constant of know size there is no need to align
it here.
> + if (strm_size >= set_strm_size_max(cpb_buf_size)) {
> + mfc_debug(2, "Decrease strm_size : %u -> %zu, gap : %d\n",
> + strm_size, set_strm_size_max(cpb_buf_size), CPB_ALIGN);
> + strm_size = set_strm_size_max(cpb_buf_size);
> + }
As I understand strm_size here is a size of buffer to be decoded, why it
cannot be equal to buf_size->cpb? Commit message says nothing about it.
> mfc_debug(2, "inst_no: %d, buf_addr: 0x%08x,\n"
> "buf_size: 0x%08x (%d)\n",
> ctx->inst_no, buf_addr, strm_size, strm_size);
> @@ -519,6 +526,8 @@ static int s5p_mfc_set_enc_stream_buffer_v6(struct s5p_mfc_ctx *ctx,
> struct s5p_mfc_dev *dev = ctx->dev;
> const struct s5p_mfc_regs *mfc_regs = dev->mfc_regs;
>
> + size = ALIGN(size, 512);
> +
Shouldn't be CPB_ALIGN instead of 512? And more importantly size is a
length of buffer for encoded stream,
by up-aligning you tell MFC that it can write beyond the buffer, it
could potentially overwrite random memory? Am I right?
> writel(addr, mfc_regs->e_stream_buffer_addr); /* 16B align */
> writel(size, mfc_regs->e_stream_buffer_size);
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
> index 8055848..16a7b1d 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h
> @@ -40,6 +40,9 @@
> #define FRAME_DELTA_H264_H263 1
> #define TIGHT_CBR_MAX 10
>
> +#define CPB_ALIGN 512
> +#define set_strm_size_max(cpb_max) ((cpb_max) - CPB_ALIGN)
Name of the macro is misleading.
Regards
Andrzej
> +
> struct s5p_mfc_hw_ops *s5p_mfc_init_hw_ops_v6(void);
> const struct s5p_mfc_regs *s5p_mfc_init_regs_v6_plus(struct s5p_mfc_dev *dev);
> #endif /* S5P_MFC_OPR_V6_H_ */
^ permalink raw reply
* [PATCH] ARM: mm: add testcases for RODATA
From: Mark Rutland @ 2017-01-18 14:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170118135310.GA4733@pjb1027-Latitude-E5410>
On Wed, Jan 18, 2017 at 10:53:10PM +0900, Jinbum Park wrote:
> This patch adds testcases for the CONFIG_DEBUG_RODATA option.
> It's similar to x86's testcases.
> It tests read-only mapped data and page-size aligned rodata section.
I note that LKDTM already has a similar test (though it just has a raw
write, and will crash the kernel).
> + asm volatile(
> + "0: str %[zero], [%[rodata_test]]\n"
> + " mov %[rslt], %[zero]\n"
> + "1:\n"
> + ".pushsection .text.fixup,\"ax\"\n"
> + ".align 2\n"
> + "2:\n"
> + "b 1b\n"
> + ".popsection\n"
> + ".pushsection __ex_table,\"a\"\n"
> + ".align 3\n"
> + ".long 0b, 2b\n"
> + ".popsection\n"
> + : [rslt] "=r" (result)
> + : [zero] "r" (0UL), [rodata_test] "r" (&rodata_test_data)
> + );
This is the only architecture-specific part of the file.
Rather than duplicating the logic from x86, can't we use generic
infrastructure for this part, and move the existing test into a shared
location?
e.g. could we change to KERNEL_DS and use put_user here?
> + if (!result) {
> + pr_err("rodata_test: test data was not read only\n");
> + return -ENODEV;
> + }
> +
> + /* test 3: check the value hasn't changed */
> + /* If this test fails, we managed to overwrite the data */
> + if (!rodata_test_data) {
> + pr_err("rodata_test: Test 3 fails (end data)\n");
> + return -ENODEV;
> + }
> +
> + /* test 4: check if the rodata section is 4Kb aligned */
> + start = (unsigned long)__start_rodata;
> + end = (unsigned long)__end_rodata;
> + if (start & (PAGE_SIZE - 1)) {
> + pr_err("rodata_test: .rodata is not 4k aligned\n");
> + return -ENODEV;
> + }
> + if (end & (PAGE_SIZE - 1)) {
> + pr_err("rodata_test: .rodata end is not 4k aligned\n");
> + return -ENODEV;
> + }
s/4k/page/ in the prints, if this becomes generic.
Thanks,
Mark.
^ permalink raw reply
* Initializing MAC address at run-time
From: Mark Rutland @ 2017-01-18 14:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <e083ed68-0e8e-380e-23bd-5ad387c88575@free.fr>
On Wed, Jan 18, 2017 at 03:03:57PM +0100, Mason wrote:
> Hello,
>
> When my system boots up, eth0 is given a seemingly random MAC address.
>
> [ 0.950734] nb8800 26000.ethernet eth0: MAC address ba:de:d6:38:b8:38
> [ 0.957334] nb8800 26000.ethernet eth0: MAC address 6e:f1:48:de:d6:c4
>
>
> The DT node for eth0 is:
>
> eth0: ethernet at 26000 {
> compatible = "sigma,smp8734-ethernet";
> reg = <0x26000 0x800>;
> interrupts = <38 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&clkgen SYS_CLK>;
> };
>
> Documentation/devicetree/bindings/net/ethernet.txt mentions
> - local-mac-address: array of 6 bytes, specifies the MAC address that was
> assigned to the network device;
>
> And indeed, if I define this property, eth0 ends up with the MAC address
> I specify in the device tree. But of course, I don't want all my boards
> to share the same MAC address. Every interface has a unique MAC address.
>
> In fact, the boot loader (not Uboot, a custom non-DT boot loader) stores
> the MAC address somewhere in MMIO space, in some weird custom format.
>
> So, at init, I can find the MAC address, and dynamically insert the
> "local-mac-address" property in the eth0 node.
To me it sounds very convoluted to do this from the kernel, to pass
information back to itself. I don't think this is the best way to handle
this.
> Is there another (better) way to do this?
>
> I'll post my code below, for illustration purpose.
>
> Mark suggested this can be done from user-space, but I can't do that,
> because I'm using an NFS rootfs, so I need the network before I even
> have a user-space. And the DHCP server is configured to serve different
> root filesystems, based on the MAC address.
That's not quite what I said. I asked whether your information was
coming from userspace or from a kernel driver.
My suggestion was that this should be done in the probe path somehow,
by describing the relationship between the ethernet controller and the
device containing the MAC information.
e.g. on the ethernet device, have a phandle (and perhaps some other
args) describinng the device containing the MAC, and how to extract it.
That way, in the ethernet probe path we can go and look up the MAC
address from the provider of that information.
> I need to do something similar with the NAND partitions. The boot loader
> stores the partition offsets somewhere, and I need to pass this info
> to the NAND framework, so I assumed that inserting the corresponding
> properties at run-time was the correct way to do it.
I would say similar could happen here.
Thanks,
Mark.
^ permalink raw reply
* imx6ul: power-up using the RTC
From: Guy Shapiro @ 2017-01-18 14:49 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
I'm trying to use the low power RTC of the i.MX6UL to start from
power-off state.
I started by hopefully running:
# echo +30 > /sys/class/rtc/rtc0/wakealarm && shutdown -h now
The system was powered down, but it didn't come up after 30 seconds as
expected.
So I dug into the datasheet and the source...
To activate the power on alarm, the flags SRTC_ENV, LPTA_EN and LPWUI_EN
on the SNVS_LP Control register (LPCR) should be asserted. The wakeup
time should be written to the SNVS_LP Time alarm register (LPTA). The
code that does this is on drivers/rtc/rtc-snvs.c:snvs_rtc_set_alarm().
The first problem I found was with the use of the syscon-poweroff
driver. The "Turn off System Power" flag is part of the same register
(LPCR). The current code of syscon-poweroff set the register to the
"mask" property from the device tree on power off, overriding all the
existing flags.
After setting the "mask" property on the device tree to 0x6b instead of
0x60 (asserting the mentioned bits), the system do power up on timer, as
expected.
However, I didn't like the idea of keeping those flags on even when no
one set the alarm.
As a quick test, I modified the syscon-poweroff driver to ignore the
bits that are not on the mask:
diff --git a/drivers/power/reset/syscon-poweroff.c
b/drivers/power/reset/syscon-poweroff.c
index b683383..a5da02b 100644
--- a/drivers/power/reset/syscon-poweroff.c
+++ b/drivers/power/reset/syscon-poweroff.c
@@ -33,7 +33,7 @@ static u32 mask;
static void syscon_poweroff(void)
{
/* Issue the poweroff */
- regmap_write(map, offset, mask);
+ regmap_update_bits(map, offset, mask, mask);
mdelay(1000);
After applying this fix, the wake up alarm didn't work. Strangely, when
I added some debug prints to investigate the case, it worked again
(sometimes... depends on the exact places I add the prints).
I suspect that some other driver clears the flags during the power down,
but I couldn't find such driver.
Do you have any clue what code may change this register during the
shutdown process?
Any other insights are welcomed as well :)
Guy.
^ permalink raw reply related
* [PATCH V7 05/10] acpi: apei: handle SEA notification type for ARMv8
From: James Morse @ 2017-01-18 14:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484244924-24786-6-git-send-email-tbaicar@codeaurora.org>
Hi Tyler,
On 12/01/17 18:15, Tyler Baicar wrote:
> ARM APEI extension proposal added SEA (Synchrounous External
Nit: Synchronous
> Abort) notification type for ARMv8.
> Add a new GHES error source handling function for SEA. If an error
> source's notification type is SEA, then this function can be registered
> into the SEA exception handler. That way GHES will parse and report
> SEA exceptions when they occur.
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 2acbc60..87efe26 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -767,6 +772,62 @@ static struct notifier_block ghes_notifier_sci = {
> .notifier_call = ghes_notify_sci,
> };
>
> +#ifdef CONFIG_HAVE_ACPI_APEI_SEA
> +static LIST_HEAD(ghes_sea);
> +
> +static int ghes_notify_sea(struct notifier_block *this,
> + unsigned long event, void *data)
> +{
> + struct ghes *ghes;
> + int ret = NOTIFY_DONE;
> +
> + nmi_enter();
Can we move this into the arch code? Its because we got here from a
synchronous-exception that makes this nmi-like, I think it only makes sense for
it be called from under /arch/.
Where did the rcu_read_lock() go? I can see its missing from ghes_notify_nmi()
too, but I don't know enough about RCU to know if that's safe!
The second paragraph in the comment above rcu_read_lock() describes it as
preventing call_rcu() during a read-side critical section that was running
concurrently. Doesn't this mean we can race with ghes_sea_remove() on another
CPU because we wait for the wrong grace period?
The same comment talks about how these read-side critical sections can nest, so
I think its quite safe to make these 'lock' calls here.
> + list_for_each_entry_rcu(ghes, &ghes_sea, list) {
> + if (!ghes_proc(ghes))
> + ret = NOTIFY_OK;
> + }
> + nmi_exit();
> +
> + return ret;
> +}
> +
> +static struct notifier_block ghes_notifier_sea = {
> + .notifier_call = ghes_notify_sea,
> +};
> +
> +static int ghes_sea_add(struct ghes *ghes)
> +{
> + mutex_lock(&ghes_list_mutex);
> + if (list_empty(&ghes_sea))
> + register_sea_notifier(&ghes_notifier_sea);
> + list_add_rcu(&ghes->list, &ghes_sea);
> + mutex_unlock(&ghes_list_mutex);
> + return 0;
> +}
> +
> +static void ghes_sea_remove(struct ghes *ghes)
> +{
> + mutex_lock(&ghes_list_mutex);
> + list_del_rcu(&ghes->list);
> + if (list_empty(&ghes_sea))
> + unregister_sea_notifier(&ghes_notifier_sea);
> + mutex_unlock(&ghes_list_mutex);
ghes_nmi_remove() has:
> /*
> * To synchronize with NMI handler, ghes can only be
> * freed after NMI handler finishes.
> */
> synchronize_rcu()
This 'waits until a grace period has elapsed'. This is because ghes_remove()
goes and kfree()s the ghes object while another CPU may be holding that entry in
the list in ghes_notify_sea().
> +}
> +#else /* CONFIG_HAVE_ACPI_APEI_SEA */
> +static inline int ghes_sea_add(struct ghes *ghes)
> +{
> + pr_err(GHES_PFX "ID: %d, trying to add SEA notification which is not supported\n",
> + ghes->generic->header.source_id);
> + return -ENOTSUPP;
> +}
> +
> +static inline void ghes_sea_remove(struct ghes *ghes)
> +{
> + pr_err(GHES_PFX "ID: %d, trying to remove SEA notification which is not supported\n",
> + ghes->generic->header.source_id);
> +}
> +#endif /* CONFIG_HAVE_ACPI_APEI_SEA */
> +
> #ifdef CONFIG_HAVE_ACPI_APEI_NMI
> /*
> * printk is not safe in NMI context. So in NMI handler, we allocate
> @@ -1011,6 +1072,14 @@ static int ghes_probe(struct platform_device *ghes_dev)
> case ACPI_HEST_NOTIFY_EXTERNAL:
> case ACPI_HEST_NOTIFY_SCI:
> break;
> + case ACPI_HEST_NOTIFY_SEA:
> + if (!IS_ENABLED(CONFIG_HAVE_ACPI_APEI_SEA)) {
> + pr_warn(GHES_PFX "Generic hardware error source: %d notified via SEA is not supported\n",
> + generic->header.source_id);
> + rc = -ENOTSUPP;
> + goto err;
> + }
> + break;
> case ACPI_HEST_NOTIFY_NMI:
> if (!IS_ENABLED(CONFIG_HAVE_ACPI_APEI_NMI)) {
> pr_warn(GHES_PFX "Generic hardware error source: %d notified via NMI interrupt is not supported!\n",
> @@ -1022,6 +1091,13 @@ static int ghes_probe(struct platform_device *ghes_dev)
> pr_warning(GHES_PFX "Generic hardware error source: %d notified via local interrupt is not supported!\n",
> generic->header.source_id);
> goto err;
> + case ACPI_HEST_NOTIFY_GPIO:
> + case ACPI_HEST_NOTIFY_SEI:
> + case ACPI_HEST_NOTIFY_GSIV:
These three weren't mentioned in the commit message. I guess they are drive-by
cleanup?
> + pr_warn(GHES_PFX "Generic hardware error source: %d notified via notification type %u is not supported\n",
> + generic->header.source_id, generic->header.source_id);
> + rc = -ENOTSUPP;
> + goto err;
> default:
> pr_warning(FW_WARN GHES_PFX "Unknown notification type: %u for generic hardware error source: %d\n",
> generic->notify.type, generic->header.source_id);
Thanks,
James
^ permalink raw reply
* [PATCH v3 6/9] kvm: arm/arm64: Add host pmu to support VM introspection
From: Punit Agrawal @ 2017-01-18 14:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c2382140-b7ac-11d8-89f0-bf421e38f844@arm.com>
Marc Zyngier <marc.zyngier@arm.com> writes:
> On 18/01/17 13:01, Punit Agrawal wrote:
>> Mark Rutland <mark.rutland@arm.com> writes:
>>
>>> On Wed, Jan 18, 2017 at 11:21:21AM +0000, Marc Zyngier wrote:
>>>> On 10/01/17 11:38, Punit Agrawal wrote:
>>>>> +#define VM_MASK GENMASK_ULL(31, 0)
>>>>> +#define EVENT_MASK GENMASK_ULL(32, 39)
>>>>> +#define EVENT_SHIFT (32)
>>>>> +
>>>>> +#define to_pid(cfg) ((cfg) & VM_MASK)
>>>>> +#define to_event(cfg) (((cfg) & EVENT_MASK) >> EVENT_SHIFT)
>>>>> +
>>>>> +PMU_FORMAT_ATTR(vm, "config:0-31");
>>>>> +PMU_FORMAT_ATTR(event, "config:32-39");
>>>>
>>>> I'm a bit confused by these. Can't you get the PID of the VM you're
>>>> tracing directly from perf, without having to encode things?
>>
>> With perf attached to a PID, the event gets scheduled out when the task
>> is context switched. As the PID of the controlling process was used,
>> none of the vCPU events were counted.
>>
>>> And if you
>>>> can't, surely this should be a function of the size of pid_t?
>>
>> Agreed. I'll update above if we decide to carry on with this
>> approach. More below...
>>
>>>>
>>>> Mark, can you shine some light here?
>>>
>>> AFAICT, this is not necessary.
>>>
>>> The perf_event_open() syscall takes a PID separately from the
>>> perf_event_attr. i.e. we should be able to do:
>>>
>>> // monitor a particular vCPU
>>> perf_event_open(attr, vcpupid, -1, -1, 0)
>>>
>>> ... or ..
>>>
>>> // monitor a particular vCPU on a pCPU
>>> perf_event_open(attr, vcpupid, cpu, -1, 0)
>>>
>>> ... or ...
>>>
>>> // monitor all vCPUs on a pCPU
>>> perf_event_open(attr, -1, cpu, -1, 0)
>>>
>>> ... so this shouldn't be necessary. AFAICT, this is a SW PMU, so there
>>> should be no issue with using the perf_sw_context.
>>
>> I might have missed it but none of the modes of invoking perf_event_open
>> allow monitoring a set of process, i.e., all vcpus belonging to a
>> particular VM, which was one of the aims and a feature I was carrying
>> over from the previous version. If we do not care about this...
>>
>>>
>>> If this is a bodge to avoid opening a perf_event per vCPU thread, then I
>>> completely disagree with the approach. This would be better handled in
>>> userspace by discovering the set of threads and opening events for
>>> each.
>>
>> ... then requiring userspace to invoke perf_event_open perf vCPU will
>> simplify this patch.
>>
>> Marc, any objections?
>
> Not so far, but I'm curious to find out how you determine which thread
> is a vcpu, let alone a given vcpu.
I should've clarified in my reply that I wasn't looking to support the
third instance from Mark's examples above - "monitor all vCPUs on a
pCPU". I think it'll be quite expensive to figure out which threads from
a given pool are vCPUs.
For the other instances, we only need to find the vCPU for a given
pid. Userspace will hand us a pid that needs to be checked against vCPUs
to establish that it is a valid vCPU pid (here I was looking to use
kvm_vcpu->pid and kvm->pid introduced in Patch 2).
This will happen when setting up the event and the vCPU can be cached
for later use.
>
> Thanks,
>
> M.
^ permalink raw reply
* [PATCH 01/11] [media] s5p-mfc: Rename IS_MFCV8 macro
From: Andrzej Hajda @ 2017-01-18 14:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484733729-25371-2-git-send-email-smitha.t@samsung.com>
On 18.01.2017 11:01, Smitha T Murthy wrote:
> This patch renames macro IS_MFCV8 to IS_MFCV8_PLUS so that the MFCv8
> code can be resued for MFCv10.10 support. Since the MFCv8 specific code
> holds good for MFC v10.10 also.
>
> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
Acked-by: Andrzej Hajda <a.hajda@samsung.com>
--
Regards
Andrzej
> ---
> drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 2 +-
> drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c | 2 +-
> drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 2 +-
> drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 18 +++++++++---------
> 4 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> index ab23236..b45d18c 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> @@ -722,7 +722,7 @@ struct mfc_control {
> #define IS_TWOPORT(dev) (dev->variant->port_num == 2 ? 1 : 0)
> #define IS_MFCV6_PLUS(dev) (dev->variant->version >= 0x60 ? 1 : 0)
> #define IS_MFCV7_PLUS(dev) (dev->variant->version >= 0x70 ? 1 : 0)
> -#define IS_MFCV8(dev) (dev->variant->version >= 0x80 ? 1 : 0)
> +#define IS_MFCV8_PLUS(dev) (dev->variant->version >= 0x80 ? 1 : 0)
>
> #define MFC_V5_BIT BIT(0)
> #define MFC_V6_BIT BIT(1)
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
> index cc88871..484af6b 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
> @@ -427,7 +427,7 @@ int s5p_mfc_wakeup(struct s5p_mfc_dev *dev)
> s5p_mfc_clear_cmds(dev);
> s5p_mfc_clean_dev_int_flags(dev);
> /* 3. Send MFC wakeup command and wait for completion*/
> - if (IS_MFCV8(dev))
> + if (IS_MFCV8_PLUS(dev))
> ret = s5p_mfc_v8_wait_wakeup(dev);
> else
> ret = s5p_mfc_wait_wakeup(dev);
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> index 367ef8e..0ec2928 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
> @@ -1177,7 +1177,7 @@ void s5p_mfc_dec_init(struct s5p_mfc_ctx *ctx)
> struct v4l2_format f;
> f.fmt.pix_mp.pixelformat = V4L2_PIX_FMT_H264;
> ctx->src_fmt = find_format(&f, MFC_FMT_DEC);
> - if (IS_MFCV8(ctx->dev))
> + if (IS_MFCV8_PLUS(ctx->dev))
> f.fmt.pix_mp.pixelformat = V4L2_PIX_FMT_NV12M;
> else if (IS_MFCV6_PLUS(ctx->dev))
> f.fmt.pix_mp.pixelformat = V4L2_PIX_FMT_NV12MT_16X16;
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> index 57da798..0572521 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> @@ -74,7 +74,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
> ctx->luma_size, ctx->chroma_size, ctx->mv_size);
> mfc_debug(2, "Totals bufs: %d\n", ctx->total_dpb_count);
> } else if (ctx->type == MFCINST_ENCODER) {
> - if (IS_MFCV8(dev))
> + if (IS_MFCV8_PLUS(dev))
> ctx->tmv_buffer_size = S5P_FIMV_NUM_TMV_BUFFERS_V6 *
> ALIGN(S5P_FIMV_TMV_BUFFER_SIZE_V8(mb_width, mb_height),
> S5P_FIMV_TMV_BUFFER_ALIGN_V6);
> @@ -89,7 +89,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
> ctx->chroma_dpb_size = ALIGN((mb_width * mb_height) *
> S5P_FIMV_CHROMA_MB_TO_PIXEL_V6,
> S5P_FIMV_CHROMA_DPB_BUFFER_ALIGN_V6);
> - if (IS_MFCV8(dev))
> + if (IS_MFCV8_PLUS(dev))
> ctx->me_buffer_size = ALIGN(S5P_FIMV_ME_BUFFER_SIZE_V8(
> ctx->img_width, ctx->img_height,
> mb_width, mb_height),
> @@ -110,7 +110,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
> switch (ctx->codec_mode) {
> case S5P_MFC_CODEC_H264_DEC:
> case S5P_MFC_CODEC_H264_MVC_DEC:
> - if (IS_MFCV8(dev))
> + if (IS_MFCV8_PLUS(dev))
> ctx->scratch_buf_size =
> S5P_FIMV_SCRATCH_BUF_SIZE_H264_DEC_V8(
> mb_width,
> @@ -167,7 +167,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
> ctx->bank1.size = ctx->scratch_buf_size;
> break;
> case S5P_MFC_CODEC_VP8_DEC:
> - if (IS_MFCV8(dev))
> + if (IS_MFCV8_PLUS(dev))
> ctx->scratch_buf_size =
> S5P_FIMV_SCRATCH_BUF_SIZE_VP8_DEC_V8(
> mb_width,
> @@ -182,7 +182,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
> ctx->bank1.size = ctx->scratch_buf_size;
> break;
> case S5P_MFC_CODEC_H264_ENC:
> - if (IS_MFCV8(dev))
> + if (IS_MFCV8_PLUS(dev))
> ctx->scratch_buf_size =
> S5P_FIMV_SCRATCH_BUF_SIZE_H264_ENC_V8(
> mb_width,
> @@ -215,7 +215,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
> ctx->bank2.size = 0;
> break;
> case S5P_MFC_CODEC_VP8_ENC:
> - if (IS_MFCV8(dev))
> + if (IS_MFCV8_PLUS(dev))
> ctx->scratch_buf_size =
> S5P_FIMV_SCRATCH_BUF_SIZE_VP8_ENC_V8(
> mb_width,
> @@ -366,7 +366,7 @@ static void s5p_mfc_dec_calc_dpb_size_v6(struct s5p_mfc_ctx *ctx)
>
> ctx->luma_size = calc_plane(ctx->img_width, ctx->img_height);
> ctx->chroma_size = calc_plane(ctx->img_width, (ctx->img_height >> 1));
> - if (IS_MFCV8(ctx->dev)) {
> + if (IS_MFCV8_PLUS(ctx->dev)) {
> /* MFCv8 needs additional 64 bytes for luma,chroma dpb*/
> ctx->luma_size += S5P_FIMV_D_ALIGN_PLANE_SIZE_V8;
> ctx->chroma_size += S5P_FIMV_D_ALIGN_PLANE_SIZE_V8;
> @@ -454,7 +454,7 @@ static int s5p_mfc_set_dec_frame_buffer_v6(struct s5p_mfc_ctx *ctx)
> writel(buf_addr1, mfc_regs->d_scratch_buffer_addr);
> writel(ctx->scratch_buf_size, mfc_regs->d_scratch_buffer_size);
>
> - if (IS_MFCV8(dev)) {
> + if (IS_MFCV8_PLUS(dev)) {
> writel(ctx->img_width,
> mfc_regs->d_first_plane_dpb_stride_size);
> writel(ctx->img_width,
> @@ -2120,7 +2120,7 @@ static unsigned int s5p_mfc_get_crop_info_v_v6(struct s5p_mfc_ctx *ctx)
> S5P_FIMV_E_ENCODED_SOURCE_SECOND_ADDR_V7);
> R(e_vp8_options, S5P_FIMV_E_VP8_OPTIONS_V7);
>
> - if (!IS_MFCV8(dev))
> + if (!IS_MFCV8_PLUS(dev))
> goto done;
>
> /* Initialize registers used in MFC v8 only.
^ permalink raw reply
* [PATCH 3/3] bcm63xx_enet: avoid uninitialized variable warning
From: Arnd Bergmann @ 2017-01-18 14:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170118145306.1004008-1-arnd@arndb.de>
gcc-7 and probably earlier versions get confused by this function
and print a harmless warning:
drivers/net/ethernet/broadcom/bcm63xx_enet.c: In function 'bcm_enet_open':
drivers/net/ethernet/broadcom/bcm63xx_enet.c:1130:3: error: 'phydev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
This adds an initialization for the 'phydev' variable when it is unused
and changes the check to test for that NULL pointer to make it clear
that we always pass a valid pointer here.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/broadcom/bcm63xx_enet.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index 1ad904680ca6..f0a0bcb31b7e 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -913,6 +913,8 @@ static int bcm_enet_open(struct net_device *dev)
priv->old_link = 0;
priv->old_duplex = -1;
priv->old_pause = -1;
+ } else {
+ phydev = NULL;
}
/* mask all interrupts and request them */
@@ -1083,7 +1085,7 @@ static int bcm_enet_open(struct net_device *dev)
enet_dmac_writel(priv, priv->dma_chan_int_mask,
ENETDMAC_IRMASK, priv->tx_chan);
- if (priv->has_phy)
+ if (phydev)
phy_start(phydev);
else
bcm_enet_adjust_link(dev);
@@ -1126,7 +1128,7 @@ static int bcm_enet_open(struct net_device *dev)
free_irq(dev->irq, dev);
out_phy_disconnect:
- if (priv->has_phy)
+ if (phydev)
phy_disconnect(phydev);
return ret;
--
2.9.0
^ permalink raw reply related
* [RFC PATCH v5 1/7] dma: Take into account dma_pfn_offset
From: Robin Murphy @ 2017-01-18 14:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170118134447.GG9529@suse.de>
On 18/01/17 13:47, Joerg Roedel wrote:
> On Wed, Jan 18, 2017 at 11:13:17AM +0000, Vladimir Murzin wrote:
>> Even though dma-noop-ops assumes 1:1 memory mapping DMA memory range
>> can be different to RAM. For example, ARM STM32F4 MCU offers the
>> possibility to remap SDRAM from 0xc000_0000 to 0x0 to get CPU
>> performance boost, but DMA continue to see SDRAM at 0xc000_0000. This
>> difference in mapping is handled via device-tree "dma-range" property
>> which leads to dev->dma_pfn_offset is set nonzero. To handle such
>> cases take dma_pfn_offset into account.
>>
>> Cc: Joerg Roedel <jroedel@suse.de>
>> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>> Reported-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>> ---
>> lib/dma-noop.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/dma-noop.c b/lib/dma-noop.c
>> index 3d766e7..a14eee5 100644
>> --- a/lib/dma-noop.c
>> +++ b/lib/dma-noop.c
>> @@ -7,6 +7,7 @@
>> #include <linux/mm.h>
>> #include <linux/dma-mapping.h>
>> #include <linux/scatterlist.h>
>> +#include <linux/pfn.h>
>>
>> static void *dma_noop_alloc(struct device *dev, size_t size,
>> dma_addr_t *dma_handle, gfp_t gfp,
>> @@ -16,7 +17,8 @@ static void *dma_noop_alloc(struct device *dev, size_t size,
>>
>> ret = (void *)__get_free_pages(gfp, get_order(size));
>> if (ret)
>> - *dma_handle = virt_to_phys(ret);
>> + *dma_handle = virt_to_phys(ret) - PFN_PHYS(dev->dma_pfn_offset);
>> +
>
> If you need to do a '-' operation here, the offset is basically a
> cpu_pfn_offset for the device. Is that correct?
Effectively, yes. The value of dev->dma_pfn_offset can be thought of as
the physical (CPU) PFN to which "bus PFN" 0 (for the given device)
corresponds. On the Keystone platform which begat this machinery, the
DRAM is at 0x08_000_0000-0x09_ffff_ffff, but there is an I/O-coherent
alias of the first 2GB which appears at 0x00_8000_0000-0x00_ffff_ffff to
devices - thus on that platform 32-bit devices get a dma_mask of
0x7fff_ffff and a dma_pfn_offset of 0x7_8000_0000 >> PAGE_SHIFT, and
everything works out.
The STM32 situation is a bit funkier, as it's actually the CPU which is
making use of a DRAM alias there, but we achieve the same effect by
giving all the DMA masters a (negative) offset to compensate, such that
subtracting the offset translates addresses from the alias region back
to the "real" physical address when handed off to a DMA master.
Robin.
>
>
> Joerg
>
^ permalink raw reply
* [PATCH v3 6/9] kvm: arm/arm64: Add host pmu to support VM introspection
From: Punit Agrawal @ 2017-01-18 14:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170118134558.GF28063@arm.com>
Will Deacon <will.deacon@arm.com> writes:
> On Wed, Jan 18, 2017 at 01:01:40PM +0000, Punit Agrawal wrote:
>> Mark Rutland <mark.rutland@arm.com> writes:
>>
>> > On Wed, Jan 18, 2017 at 11:21:21AM +0000, Marc Zyngier wrote:
>> >> On 10/01/17 11:38, Punit Agrawal wrote:
>> >> > +#define VM_MASK GENMASK_ULL(31, 0)
>> >> > +#define EVENT_MASK GENMASK_ULL(32, 39)
>> >> > +#define EVENT_SHIFT (32)
>> >> > +
>> >> > +#define to_pid(cfg) ((cfg) & VM_MASK)
>> >> > +#define to_event(cfg) (((cfg) & EVENT_MASK) >> EVENT_SHIFT)
>> >> > +
>> >> > +PMU_FORMAT_ATTR(vm, "config:0-31");
>> >> > +PMU_FORMAT_ATTR(event, "config:32-39");
>> >>
>> >> I'm a bit confused by these. Can't you get the PID of the VM you're
>> >> tracing directly from perf, without having to encode things?
>>
>> With perf attached to a PID, the event gets scheduled out when the task
>> is context switched. As the PID of the controlling process was used,
>> none of the vCPU events were counted.
>
> So it sounds like userspace needs to deal with this by attaching to the PIDs
> of the vCPUs. Given that perf kvm seems to have knowledge of vCPUs, it would
> be nice to know why that logic isn't reusable here. Take a look in
> tools/perf/builtin-kvm.c and if it's not up to the job, then perhaps it can
> be improved.
Thanks for the pointer. I'll have a play with perf kvm to see if it can
be leveraged here.
>
> Will
> _______________________________________________
> kvmarm mailing list
> kvmarm at lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply
* [PATCH 3/3] arm64: dts: exynos: Use macros for pinctrl configuration on Exynos7
From: Krzysztof Kozlowski @ 2017-01-18 15:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484718141-28785-4-git-send-email-pankaj.dubey@samsung.com>
On Wed, Jan 18, 2017 at 11:12:21AM +0530, Pankaj Dubey wrote:
> Usage of DTS macros instead of hard-coded numbers makes code easier to
> read. One does not have to remember which value means pull-up/down or
> specific driver strength.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm64/boot/dts/exynos/exynos7-espresso.dts | 4 +-
> arch/arm64/boot/dts/exynos/exynos7-pinctrl.dtsi | 302 ++++++++++++------------
> 2 files changed, 154 insertions(+), 152 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> index c528dd5..1b2db9f0 100644
> --- a/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos7-espresso.dts
> @@ -328,8 +328,8 @@
> &pinctrl_alive {
> pmic_irq: pmic-irq {
> samsung,pins = "gpa0-2";
> - samsung,pin-pud = <3>;
> - samsung,pin-drv = <3>;
> + samsung,pin-pud = <EXYNOS_PIN_PULL_UP>;
> + samsung,pin-drv = <EXYNOS4_PIN_DRV_LV3>;
> };
> };
This is not equal (value 3 => 1). If the change was intended, please do
in separate patch.
Rest looks fine.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH 02/11] [media] s5p-mfc: Adding initial support for MFC v10.10
From: Andrzej Hajda @ 2017-01-18 15:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484733729-25371-3-git-send-email-smitha.t@samsung.com>
On 18.01.2017 11:02, Smitha T Murthy wrote:
> Adding the support for MFC v10.10, with new register file and
> necessary hw control, decoder, encoder and structural changes.
>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: devicetree at vger.kernel.org
> Signed-off-by: Smitha T Murthy <smitha.t@samsung.com>
> ---
> .../devicetree/bindings/media/s5p-mfc.txt | 1 +
> drivers/media/platform/s5p-mfc/regs-mfc-v10.h | 36 ++++++++++++++++
> drivers/media/platform/s5p-mfc/s5p_mfc.c | 30 +++++++++++++
> drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 4 +-
> drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c | 4 ++
> drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 44 +++++++++++---------
> drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 21 +++++----
> drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 9 +++-
> drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h | 2 +
> 9 files changed, 118 insertions(+), 33 deletions(-)
> create mode 100644 drivers/media/platform/s5p-mfc/regs-mfc-v10.h
>
> diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> index 2c90128..b70c613 100644
> --- a/Documentation/devicetree/bindings/media/s5p-mfc.txt
> +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
> @@ -13,6 +13,7 @@ Required properties:
> (c) "samsung,mfc-v7" for MFC v7 present in Exynos5420 SoC
> (d) "samsung,mfc-v8" for MFC v8 present in Exynos5800 SoC
> (e) "samsung,exynos5433-mfc" for MFC v8 present in Exynos5433 SoC
> + (f) "samsung,mfc-v10" for MFC v10 present in a variant of Exynos7 SoC
Could you specify explicitly SoC version(s), Exynos7 is misleading.
Btw are there plans to upstream platforms using this MFC?
>
> - reg : Physical base address of the IP registers and length of memory
> mapped region.
> diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> new file mode 100644
> index 0000000..bd671a5
> --- /dev/null
> +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h
> @@ -0,0 +1,36 @@
> +/*
> + * Register definition file for Samsung MFC V10.x Interface (FIMV) driver
> + *
> + * Copyright (c) 2017 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com/
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _REGS_MFC_V10_H
> +#define _REGS_MFC_V10_H
> +
> +#include <linux/sizes.h>
> +#include "regs-mfc-v8.h"
> +
> +/* MFCv10 register definitions*/
> +#define S5P_FIMV_MFC_CLOCK_OFF_V10 0x7120
> +#define S5P_FIMV_MFC_STATE_V10 0x7124
> +
> +/* MFCv10 Context buffer sizes */
> +#define MFC_CTX_BUF_SIZE_V10 (30 * SZ_1K) /* 30KB */
> +#define MFC_H264_DEC_CTX_BUF_SIZE_V10 (2 * SZ_1M) /* 2MB */
> +#define MFC_OTHER_DEC_CTX_BUF_SIZE_V10 (20 * SZ_1K) /* 20KB */
> +#define MFC_H264_ENC_CTX_BUF_SIZE_V10 (100 * SZ_1K) /* 100KB */
> +#define MFC_OTHER_ENC_CTX_BUF_SIZE_V10 (15 * SZ_1K) /* 15KB */
> +
> +/* MFCv10 variant defines */
> +#define MAX_FW_SIZE_V10 (SZ_1M) /* 1MB */
> +#define MAX_CPB_SIZE_V10 (3 * SZ_1M) /* 3MB */
> +#define MFC_VERSION_V10 0xA0
> +#define MFC_NUM_PORTS_V10 1
> +
> +#endif /*_REGS_MFC_V10_H*/
> +
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index bb0a588..a043cce 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -1542,6 +1542,33 @@ static int s5p_mfc_resume(struct device *dev)
> .num_clocks = 3,
> };
>
> +static struct s5p_mfc_buf_size_v6 mfc_buf_size_v10 = {
> + .dev_ctx = MFC_CTX_BUF_SIZE_V10,
> + .h264_dec_ctx = MFC_H264_DEC_CTX_BUF_SIZE_V10,
> + .other_dec_ctx = MFC_OTHER_DEC_CTX_BUF_SIZE_V10,
> + .h264_enc_ctx = MFC_H264_ENC_CTX_BUF_SIZE_V10,
> + .other_enc_ctx = MFC_OTHER_ENC_CTX_BUF_SIZE_V10,
> +};
> +
> +static struct s5p_mfc_buf_size buf_size_v10 = {
> + .fw = MAX_FW_SIZE_V10,
> + .cpb = MAX_CPB_SIZE_V10,
> + .priv = &mfc_buf_size_v10,
> +};
> +
> +static struct s5p_mfc_buf_align mfc_buf_align_v10 = {
> + .base = 0,
> +};
> +
> +static struct s5p_mfc_variant mfc_drvdata_v10 = {
> + .version = MFC_VERSION_V10,
> + .version_bit = MFC_V10_BIT,
> + .port_num = MFC_NUM_PORTS_V10,
> + .buf_size = &buf_size_v10,
> + .buf_align = &mfc_buf_align_v10,
> + .fw_name[0] = "s5p-mfc-v10.fw",
Is firmware file publicly available? Sent to firmware repository?
> +};
> +
> static const struct of_device_id exynos_mfc_match[] = {
> {
> .compatible = "samsung,mfc-v5",
> @@ -1558,6 +1585,9 @@ static int s5p_mfc_resume(struct device *dev)
> }, {
> .compatible = "samsung,exynos5433-mfc",
> .data = &mfc_drvdata_v8_5433,
> + }, {
> + .compatible = "samsung,mfc-v10",
> + .data = &mfc_drvdata_v10,
> },
> {},
> };
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> index b45d18c..1941c63 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
> @@ -23,7 +23,7 @@
> #include <media/v4l2-ioctl.h>
> #include <media/videobuf2-v4l2.h>
> #include "regs-mfc.h"
> -#include "regs-mfc-v8.h"
> +#include "regs-mfc-v10.h"
>
> #define S5P_MFC_NAME "s5p-mfc"
>
> @@ -723,11 +723,13 @@ struct mfc_control {
> #define IS_MFCV6_PLUS(dev) (dev->variant->version >= 0x60 ? 1 : 0)
> #define IS_MFCV7_PLUS(dev) (dev->variant->version >= 0x70 ? 1 : 0)
> #define IS_MFCV8_PLUS(dev) (dev->variant->version >= 0x80 ? 1 : 0)
> +#define IS_MFCV10(dev) (dev->variant->version >= 0xA0 ? 1 : 0)
>
> #define MFC_V5_BIT BIT(0)
> #define MFC_V6_BIT BIT(1)
> #define MFC_V7_BIT BIT(2)
> #define MFC_V8_BIT BIT(3)
> +#define MFC_V10_BIT BIT(5)
I think you can use BIT(4) here.
Regards
Andrzej
^ permalink raw reply
* [PATCH 2/2] ARM: DTS: Fix register map for virt-capable GIC
From: Javier Martinez Canillas @ 2017-01-18 15:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1484736811-24002-3-git-send-email-marc.zyngier@arm.com>
Hello Marc,
On 01/18/2017 07:53 AM, Marc Zyngier wrote:
> Since everybody copied my own mistake from the DT binding example,
> let's address all the offenders in one swift go.
>
> Most of them got the CPU interface size wrong (4kB, while it should
> be 8kB), except for both keystone platforms which got the control
> interface wrong (4kB instead of 8kB).
>
> In the couple of cases were I knew for sure what implementation
> was used, I've added the "arm,gic-400" compatible string. I'm 99%
> sure that this is what everyong is using, but short of having the
> TRM for all the other SoCs, I've let them alone.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm/boot/dts/exynos3250.dtsi | 2 +-
> arch/arm/boot/dts/exynos5.dtsi | 2 +-
> arch/arm/boot/dts/exynos5260.dtsi | 2 +-
> arch/arm/boot/dts/exynos5440.dtsi | 2 +-
I've looked at the Exynos5250 and Exynos5420 TRM and both say that adopts
GIC-400, so I think it's safe for you to also update the compatible in the
exynos5.dtsi. Unfortunately I don't have manuals for 3250, 5260 and 5440.
The register map changes looks good to me, so for Exynos:
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply
* [PATCH v3 6/9] kvm: arm/arm64: Add host pmu to support VM introspection
From: Mark Rutland @ 2017-01-18 15:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87fukgmnf0.fsf@e105922-lin.cambridge.arm.com>
On Wed, Jan 18, 2017 at 02:51:31PM +0000, Punit Agrawal wrote:
> I should've clarified in my reply that I wasn't looking to support the
> third instance from Mark's examples above - "monitor all vCPUs on a
> pCPU". I think it'll be quite expensive to figure out which threads from
> a given pool are vCPUs.
I'm not sure I follow why you would need to do that?
In that case, we'd open a CPU-bound perf event for the pCPU, which would
get installed in the CPU context immediately. It would be present for
all tasks.
Given it's present for all tasks, we don't need to figure out which
happen to have vCPUs. The !vCPU tasks simply shouldn't trigger events.
Am I missing something?
> For the other instances, we only need to find the vCPU for a given
> pid. Userspace will hand us a pid that needs to be checked against vCPUs
> to establish that it is a valid vCPU pid (here I was looking to use
> kvm_vcpu->pid and kvm->pid introduced in Patch 2).
Thinking about this further, a pid is not a unique identifier for either
a vCPU or a VM.
A single task (which has a single pid), could own multiple VMs, each
with multiple vCPUs. A thread pool (with several pids) could share those
arbitrarily. So we need VM and vCPU IDs which are distinct from pids or
tids.
I see that struct kvm_vcpu has a vcpu_id (which from a glance appears to
be local to the kvm instance). It's not clear to me if a kvm instance
could be shared by multiple processes, or if we can get away with a
process-local ID.
Thanks,
Mark.
^ permalink raw reply
* [PATCH v9 2/5] i2c: Add STM32F4 I2C driver
From: M'boumba Cedric Madianga @ 2017-01-18 15:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170117193752.e6hju25w74bb4i4z@pengutronix.de>
Hello Uwe,
2017-01-17 20:37 GMT+01:00 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> Hello,
>
> On Tue, Jan 17, 2017 at 04:26:58PM +0100, M'boumba Cedric Madianga wrote:
>> +static void stm32f4_i2c_set_rise_time(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + u32 freq = DIV_ROUND_UP(i2c_dev->parent_rate, HZ_TO_MHZ);
>> + u32 trise;
>> +
>> + /*
>> + * These bits must be programmed with the maximum SCL rise time given in
>> + * the I2C bus specification, incremented by 1.
>> + *
>> + * In standard mode, the maximum allowed SCL rise time is 1000 ns.
>> + * If, in the I2C_CR2 register, the value of FREQ[5:0] bits is equal to
>> + * 0x08 so period = 125 ns therefore the TRISE[5:0] bits must be
>> + * programmed with 09h.(1000 ns / 125 ns = 8 + 1)
>
> * programmed with 0x9.
> (1000 ns / 125 ns = 8)
>
>> + * So, for I2C standard mode TRISE = FREQ[5:0] + 1
>> + *
>> + * In fast mode, the maximum allowed SCL rise time is 300 ns.
>> + * If, in the I2C_CR2 register, the value of FREQ[5:0] bits is equal to
>> + * 0x08 so period = 125 ns therefore the TRISE[5:0] bits must be
>> + * programmed with 03h.(300 ns / 125 ns = 2 + 1)
>
> as above s/03h/0x3/;
ok
> s/.(/. (/;
ok
> s/+ 1//;
This formula is use to understand how we find the result 0x3
So, 0x3 => 300 ns / 125ns = 2 + 1
>
>> + * So, for I2C fast mode TRISE = FREQ[5:0] * 300 / 1000 + 1
>> + */
>> + if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD)
>> + trise = freq + 1;
>> + else
>> + trise = freq * 300 / 1000 + 1;
>
> I'd use
>
> * 3 / 10
>
> without downside and lesser chance to overflow.
There is no chance of overflow as the max freq value allowed is 46
>
>> +
>> + writel_relaxed(STM32F4_I2C_TRISE_VALUE(trise),
>> + i2c_dev->base + STM32F4_I2C_TRISE);
>> +}
>> +
>> +static void stm32f4_i2c_set_speed_mode(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> + u32 val;
>> + u32 ccr = 0;
>> +
>> + if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD) {
>> + /*
>> + * In standard mode:
>> + * t_scl_high = t_scl_low = CCR * I2C parent clk period
>> + * So to reach 100 kHz, we have:
>> + * CCR = I2C parent rate / 100 kHz >> 1
>> + *
>> + * For example with parent rate = 2 MHz:
>> + * CCR = 2000000 / (100000 << 1) = 10
>> + * t_scl_high = t_scl_low = 10 * (1 / 2000000) = 5000 ns
>> + * t_scl_high + t_scl_low = 10000 ns so 100 kHz is reached
>> + */
>> + val = i2c_dev->parent_rate / (100000 << 1);
>> + } else {
>> + /*
>> + * In fast mode, we compute CCR with duty = 0 as with low
>> + * frequencies we are not able to reach 400 kHz.
>> + * In that case:
>> + * t_scl_high = CCR * I2C parent clk period
>> + * t_scl_low = 2 * CCR * I2C parent clk period
>> + * So, CCR = I2C parent rate / (400 kHz * 3)
>> + *
>> + * For example with parent rate = 6 MHz:
>> + * CCR = 6000000 / (400000 * 3) = 5
>> + * t_scl_high = 5 * (1 / 6000000) = 833 ns > 600 ns
>> + * t_scl_low = 2 * 5 * (1 / 6000000) = 1667 ns > 1300 ns
>> + * t_scl_high + t_scl_low = 2500 ns so 400 kHz is reached
>> + */
>
> Huh, that's surprising. So you don't use DUTY any more. I found two
> hints in the manual that contradict here:
Yes with the above formula we could use duty = 0 by default
>
> f_{PCLK1} must be at least 2 MHz to achieve Sm mode I2C frequencies
STM32F4_I2C_MIN_STANDARD_FREQ = 2
> It must be at least 4 MHz to achieve Fm mode I2C frequencies.
STM32F4_I2C_MIN_FAST_FREQ = 6
> It must be a multiple of 10MHz to reach the 400 kHz maximum I2C Fm mode clock.
If we use this rule only 3 values are allowed 10 Mhz, 20 Mhz, 30 Mhz and 40 Mhz.
It is very restrictive.
So I don't take it into account in order to have more frequencies even
if 400 Khz is not reached.
Indeed, in many cases we are very close to 400 Khz.
For example, the default I2C parent clock in my board is 45 Mhz
I reach 395 kHz in theory and 390 kHz by testing.
I am in Fast mode but not with the max freq but very close.
>
> and
>
> [...]
> If DUTY = 1: (to reach 400 kHz)
>
> Strange.
>
>> + val = DIV_ROUND_UP(i2c_dev->parent_rate, 400000 * 3);
>
> the manual reads:
>
> The minimum allowed value is 0x04, except in FAST DUTY mode
> where the minimum allowed value is 0x01
>
> You don't check for that, right?
As the minimum freq value is 6 Mhz in fast mode the minimum CCR is 5
as described in the comment.
So I don't need to check that again as it is already done by checking
parent frequency.
> CCR is 11 bits wide. A comment confirming that this cannot overflow
> would be nice.
Again there is no chance of overflow thanks to parent frequency check
>
> + /* select Fast Mode */
>> + ccr |= STM32F4_I2C_CCR_FS;
ok
>
> I didn't check the rest of the code, so let's assume it's good :-)
ok Thanks
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-K?nig |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [RFC PATCH v2 03/10] arm64: KVM: Save/restore the host SPE state when entering/leaving a VM
From: Will Deacon @ 2017-01-18 15:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <61308f80-3bcd-6bff-2c87-dccb58a0a84c@arm.com>
Hi Marc,
Thanks for having a look.
On Mon, Jan 16, 2017 at 11:25:37AM +0000, Marc Zyngier wrote:
> On 13/01/17 16:03, Will Deacon wrote:
> > The SPE buffer is virtually addressed, using the page tables of the CPU
> > MMU. Unusually, this means that the EL0/1 page table may be live whilst
> > we're executing at EL2 on non-VHE configurations. When VHE is in use,
> > we can use the same property to profile the guest behind its back.
> >
> > This patch adds the relevant disabling and flushing code to KVM so that
> > the host can make use of SPE without corrupting guest memory, and any
> > attempts by a guest to use SPE will result in a trap.
> >
> > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > Cc: Alex Benn?e <alex.bennee@linaro.org>
> > Cc: Christoffer Dall <christoffer.dall@linaro.org>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> > arch/arm64/include/asm/kvm_arm.h | 3 ++
> > arch/arm64/include/asm/kvm_host.h | 7 ++++-
> > arch/arm64/kvm/debug.c | 6 ++++
> > arch/arm64/kvm/hyp/debug-sr.c | 66 +++++++++++++++++++++++++++++++++++++--
> > arch/arm64/kvm/hyp/switch.c | 13 +++++++-
> > 5 files changed, 91 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> > index 2a2752b5b6aa..6e99978e83bd 100644
> > --- a/arch/arm64/include/asm/kvm_arm.h
> > +++ b/arch/arm64/include/asm/kvm_arm.h
> > @@ -188,6 +188,9 @@
> > #define CPTR_EL2_DEFAULT 0x000033ff
> >
> > /* Hyp Debug Configuration Register bits */
> > +#define MDCR_EL2_TPMS (1 << 14)
> > +#define MDCR_EL2_E2PB_MASK (UL(0x3))
> > +#define MDCR_EL2_E2PB_SHIFT (UL(12))
> > #define MDCR_EL2_TDRA (1 << 11)
> > #define MDCR_EL2_TDOSA (1 << 10)
> > #define MDCR_EL2_TDA (1 << 9)
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index e5050388e062..443b387021f2 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -229,7 +229,12 @@ struct kvm_vcpu_arch {
> >
> > /* Pointer to host CPU context */
> > kvm_cpu_context_t *host_cpu_context;
> > - struct kvm_guest_debug_arch host_debug_state;
> > + struct {
> > + /* {Break,watch}point registers */
> > + struct kvm_guest_debug_arch regs;
> > + /* Statistical profiling extension */
> > + u64 pmscr_el1;
> > + } host_debug_state;
> >
> > /* VGIC state */
> > struct vgic_cpu vgic_cpu;
> > diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> > index 47e5f0feaee8..dbadfaf850a7 100644
> > --- a/arch/arm64/kvm/debug.c
> > +++ b/arch/arm64/kvm/debug.c
> > @@ -95,6 +95,7 @@ void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
> > * - Performance monitors (MDCR_EL2_TPM/MDCR_EL2_TPMCR)
> > * - Debug ROM Address (MDCR_EL2_TDRA)
> > * - OS related registers (MDCR_EL2_TDOSA)
> > + * - Statistical profiler (MDCR_EL2_TPMS/MDCR_EL2_E2PB)
> > *
> > * Additionally, KVM only traps guest accesses to the debug registers if
> > * the guest is not actively using them (see the KVM_ARM64_DEBUG_DIRTY
> > @@ -110,8 +111,13 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
> >
> > trace_kvm_arm_setup_debug(vcpu, vcpu->guest_debug);
> >
> > + /*
> > + * This also clears MDCR_EL2_E2PB_MASK to disable guest access
> > + * to the profiling buffer.
> > + */
> > vcpu->arch.mdcr_el2 = __this_cpu_read(mdcr_el2) & MDCR_EL2_HPMN_MASK;
> > vcpu->arch.mdcr_el2 |= (MDCR_EL2_TPM |
> > + MDCR_EL2_TPMS |
>
> This is RES0 on CPUs that do not implement SPE. Can we make this
> conditional on SPE being available?
I could, but it would introduce a runtime ID register check for the physical
CPU on the fastpath, because big/little systems are not required to be
symmetric in this regard. We already have code that writes to bits that
might be RES0 but have been defined by later versions of the architecture.
In fact, setting the TPM bit here is RES0 if the PMU is not implemented.
Furthermore, the architecture does state that writing a value to a RES0
field "must have no effect on the operation of the PE". The purpose of
the fields is for future architecture extension, which in this case has
been defined by SPE.
> > +static void __hyp_text __debug_save_spe_nvhe(u64 *pmscr_el1)
> > +{
> > + u64 reg;
> > +
> > + /* SPE present on this CPU? */
> > + if (!cpuid_feature_extract_unsigned_field(read_sysreg(id_aa64dfr0_el1),
> > + ID_AA64DFR0_PMSVER_SHIFT))
> > + return;
> > +
> > + /* Yes; is it owned by EL3? */
> > + reg = read_sysreg_s(PMBIDR_EL1);
> > + if (reg & PMBIDR_EL1_P)
> > + return;
> > +
> > + /* No; is the host actually using the thing? */
> > + reg = read_sysreg_s(PMBLIMITR_EL1);
> > + if (!(reg & PMBLIMITR_EL1_E))
> > + return;
> > +
> > + /* Yes; save the control register and disable data generation */
> > + *pmscr_el1 = read_sysreg_s(PMSCR_EL1);
> > + write_sysreg_s(0, PMSCR_EL1);
> > + isb();
> > +
> > + /* Now drain all buffered data to memory */
> > + psb_csync();
> > + dsb(nsh);
>
> So let's hope that nobody will build a non-VHE, SPE capable system.
> Because this feels like hitting the brakes each time we enter the guest
> (if SPE is in use, that is...).
Yup, not much we can do about that :(
> > +static hyp_alternate_select(__debug_save_spe,
> > + __debug_save_spe_nvhe, __debug_save_spe_vhe,
> > + ARM64_HAS_VIRT_HOST_EXTN);
> > +
> > +static void __hyp_text __debug_restore_spe(u64 pmscr_el1)
> > +{
> > + if (!pmscr_el1)
> > + return;
> > +
> > + /* The host page table is installed, but not yet synchronised */
> > + isb();
>
> I believe you can avoid that on VHE (host page tables haven't changed).
It should already be skipped because pmscr_el1 is never set for VHE
(__debug_save_spe_vhe is a NOP).
> Also, if you now depend on the debug restore being run after the rest of
> the sysregs, please add a comment in switch.c::__kvm_vcpu_run so that we
> remember about that dependency.
Sure.
> > +
> > + /* Re-enable data generation */
> > + write_sysreg_s(pmscr_el1, PMSCR_EL1);
>
> Same thing here, as we never disabled SPE the first place on VHE (and
> we're probably only writing zeroes there).
Yup, check the conditional :)
Will
^ 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