* [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Rob Herring @ 2016-10-10 21:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <fc0ca2b3140287cbccf5c176a12b7dce25d06a47.1475853198.git-series.gregory.clement@free-electrons.com>
On Fri, Oct 07, 2016 at 05:22:51PM +0200, Gregory CLEMENT wrote:
> From: Ziji Hu <huziji@marvell.com>
>
> Marvell Xenon SDHC can support eMMC/SD/SDIO.
> Add Xenon-specific properties.
> Also add properties for Xenon PHY setting.
>
> Signed-off-by: Hu Ziji <huziji@marvell.com>
> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt | 164 +++++++-
> MAINTAINERS | 1 +-
> 2 files changed, 165 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
>
> diff --git a/Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt b/Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
> new file mode 100644
> index 000000000000..8b25ad28ebbd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
> @@ -0,0 +1,164 @@
> +Marvell's Xenon SDHCI Controller device tree bindings
> +This file documents differences between the core mmc properties
> +described by mmc.txt and the properties used by the Xenon implementation.
> +
> +A single Xenon IP can support multiple slots.
> +Each slot acts as an independent SDHC. It owns independent resources, such
> +as register sets clock and PHY.
Is the phy really part of the same block?
> +Each slot should have an independent device tree node.
> +
> +Required Properties:
> +- compatible: should be "marvell,sdhci-xenon" or "marvell,armada-3700-sdhci".
Perhaps some consistent ordering (w/ -sdhci on the end).
> +
> +- Input Clock Name
Your formatting of properties is a bit strange. Please restructure like
most bindings so the property names are before all the description.
> + Some SOCs require additional clock for AXI bus.
Those SoCs should have a specific compatible string and you need to
define which compatible strings have 2 clocks vs. 1 clock.
> + The input clock for Xenon IP core should be named as "core".
> + The optional AXI clock should be named as "axi".
> + - clocks = <&core_clk>, <&axi_clock>;
> + - clock-names = "core", "axi";
> +
> +- Register Set Size
Is this a property name?
> + Different Xenon SDHC release has different register set size.
> + The specific size should also refer to the SOC implementation.
> +
> +Optional Properties:
> +- Slot Index
> + A single Xenon IP can support multiple slots.
> + During initialization, each slot should set corresponding setting bit in
> + some Xenon-specific registers. The corresponding bit is determined by
> + this property.
> + - xenon,slotno = <slot_index>;
Slots should probably be represented as child nodes with the reg
property being the slot number.
Also, xenon is not a vendor prefix.
> + If this property is not provided, Xenon IP should contain only one slot
> + and the slot index will be 0x0 by default.
> +
> +- PHY Type
You're going to need to come of with a common binding for this.
> + Xenon support mutilple types of PHYs.
> + To select eMMC 5.1 PHY, set:
> + - xenon,phy-type = "emmc 5.1 phy"
> + eMMC 5.1 PHY is the default choice if this property is not provided.
> + To select eMMC 5.0 PHY, set:
> + - xenon,phy-type = "emmc 5.0 phy"
> + To select SDH PHY, set:
> + - xenon,phy-type = "sdh phy"
> + Please note that eMMC PHY is a general PHY for eMMC/SD/SDIO, other than for
> + eMMC only.
> +
> +- Customized eMMC PHY Parameters
> + Some boards require different values of some specific eMMC PHY parameters.
> + Some SOCs also require specific workaround to set eMMC PHY.
> + These properties enable diverse boards to customize the eMMC PHY.
> + The supported eMMC PHY parameters are listed in below. All those properties
> + are only available for eMMC PHY 5.1 and eMMC PHY 5.0.
> + ZNR
> + valid range = [0:0x1F].
> + ZNR is set as 0xF by default if this property is not provided.
> + - xenon,phy-znr = <value>;
> +
> + ZPR
> + valid range = [0:0x1F].
> + ZPR is set as 0xF by default if this property is not provided.
> + - xenon,phy-zpr = <value>;
marvell is the vendor prefix.
> +
> + Number of successful tuning times
> + Set the number of required consecutive successful sampling points used to
> + identify a valid sampling window, in tuning process.
> + Valid range = [1:7]. Set as 0x4 by default if this property is not provided.
> + - xenon,phy-nr-tun-times = <nr_times>;
> +
> + Divider for TUN_STEP
> + Set the divider for calculating TUN_STEP.
> + Set as 64 by default if this property is not provided.
> + - xenon,phy-tun-step-divider = <divider>;
> +
> + Force PHY into slow mode.
> + Only available when bus frequency lower than 50MHz in SDR mde.
> + Disabled by default. Please do not enable it unless it is necessary.
> + - xenon,phy-slow-mode;
> +
> +- Mask Conflict Error Report
> + Disable Conflict Error alert on some SOC. Disabled by default.
> + xenon,mask-conflict-err;
> +
> +- Re-tuning Counter
> + Xenon SDHC SOC usually doesn't provide re-tuning counter in
> + Capabilities Register 3 Bit[11:8].
> + This property provides the re-tuning counter.
> + xenon,tuning-count = <count>;
> + If this property is not set, default re-tuning counter will
> + be set as 0x9 in driver.
> +
> +- SOC PHY PAD Voltage Control register
> + Some SOCs have SOC PHY PAD Voltage Control register outside Xenon IP.
> + This register sets SOC PHY PAD Voltage to keep aligh with Vccq.
> + Two properties provide information of this control register.
> + These two properties are only valid when "marvell,armada-3700-sdhci"
> + is selected. Both of them must be provided when "marvell,armada-3700-sdhci"
> + is selected.
> + - xenon,pad-type
> + Two types: "sd" and "fixed-1-8v".
> + If "sd" is slected, SOC PHY PAD is set as 3.3V at the beginning and is
> + switched to 1.8V when SD in UHS-I.
> + If "fixed-1-8v" is slected, SOC PHY PAD is fixed 1.8V, such as for eMMC.
You should be able to existing, common properties for i/o voltage
capabilities/constraints.
> + - reg
> + Physical address and size of SOC PHY PAD register.
> + Append after Xenon SDHC register space, as a second register field.
> +
> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
> + in below.
> +
> +Example:
> +- For eMMC slot:
> +
> + sdhci at aa0000 {
> + compatible = "marvell,sdhci-xenon";
> + reg = <0xaa0000 0x1000>;
> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
> + clocks = <&emmcclk>;
> + clock-names = "core";
> + xenon,slotno = <0>;
> + xenon,phy-type = "emmc 5.1 phy";
> + bus-width = <8>;
> + tuning-count = <11>;
> + };
> +
> +- For SD/SDIO slot:
> +
> + sdhci at ab0000 {
> + compatible = "marvell,sdhci-xenon";
> + reg = <0xab0000 0x1000>;
> + interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
> + vqmmc-supply = <&sd_regulator>;
> + clocks = <&sdclk>;
> + clock-names = "core";
> + bus-width = <4>;
> + tuning-count = <9>;
> + };
> +
> +- For eMMC slot with compatible "marvell,armada-3700-sdhci":
> +
> + sdhci at aa0000 {
> + compatible = "marvell,armada-3700-sdhci";
> + reg = <0xaa0000 0x1000>,
> + <phy_addr 0x4>;
> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>
> + clocks = <&emmcclk>;
> + clock-names = "core";
> + bus-width = <8>;
> +
> + xenon,pad-type = "fixed-1-8v";
> + };
> +
> +- For SD/SDIO slot with compatible "marvell,armada-3700-sdhci":
> +
> + sdhci at ab0000 {
> + compatible = "marvell,armada-3700-sdhci";
> + reg = <0xab0000 0x1000>,
> + <phy_addr 0x4>;
> + interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>
> + vqmmc-supply = <&sd_regulator>;
> + clocks = <&sdclk>;
> + clock-names = "core";
> + bus-width = <4>;
> +
> + xenon,pad-type = "sd";
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 89adcd57aa25..4aa0eac9bfc7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7582,6 +7582,7 @@ MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
> M: Ziji Hu <huziji@marvell.com>
> L: linux-mmc at vger.kernel.org
> S: Supported
> +F: Documentation/devicetree/bindings/mmc/marvell,sdhci-xenon.txt
>
> MATROX FRAMEBUFFER DRIVER
> L: linux-fbdev at vger.kernel.org
> --
> git-series 0.8.10
^ permalink raw reply
* [PATCH v2 3/8] PM / Domains: Allow domain power states to be read from DT
From: Ulf Hansson @ 2016-10-10 21:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161010150301.GA44885@linaro.org>
[...]
>>> +int of_genpd_parse_idle_states(struct device_node *dn,
>>> + struct genpd_power_state **states, int *n)
>>
>>
>> Instead of taking **states as a parameter, let's instead return it as
>> a pointer for the allocated struct. In case of failures, let's return
>> ERR_PTR().
>>
> Hmm.. I thought about it. There are 2 return values from this function.
> If we return a pointer to the allocated memory, we still have to return
> the size of it as an argument. I wasn't happy splitting the return
> values in 2 different places.
Ahh, right. So let's keep it as is!
[...]
Kind regards
Uffe
^ permalink raw reply
* [PATCH 2/2] arm64: Add missing kexec dist files
From: Geoff Levand @ 2016-10-10 21:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1476134309.git.geoff@infradead.org>
Signed-off-by: Geoff Levand <geoff@infradead.org>
---
kexec/arch/arm64/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kexec/arch/arm64/Makefile b/kexec/arch/arm64/Makefile
index d729df8..74b677f 100644
--- a/kexec/arch/arm64/Makefile
+++ b/kexec/arch/arm64/Makefile
@@ -20,7 +20,9 @@ arm64_VIRT_TO_PHYS =
arm64_PHYS_TO_VIRT =
dist += $(arm64_KEXEC_SRCS) \
+ kexec/arch/arm64/include/arch/options.h \
kexec/arch/arm64/crashdump-arm64.h \
+ kexec/arch/arm64/image-header.h \
kexec/arch/arm64/kexec-arm64.h \
kexec/arch/arm64/Makefile
--
2.7.4
^ permalink raw reply related
* [PATCH 0/2] arm64 kexec-tools fixups
From: Geoff Levand @ 2016-10-10 21:22 UTC (permalink / raw)
To: linux-arm-kernel
Hi Simon,
Fixes for 'make tarball'. Please apply.
-Geoff
The following changes since commit 6e8804f9ca67bee65aedd5e1cdf504737ab5c7ed:
multiboot: Use the "reserved" type for non-ram zones (2016-10-07 11:54:44 +0900)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/geoff/kexec-tools.git for-merge-arm64-dist
for you to fetch changes up to 3552a3e85a910688d932535549b35354a54927ad:
arm64: Add missing kexec dist files (2016-10-10 14:15:21 -0700)
----------------------------------------------------------------
Geoff Levand (2):
arm64: Cleanup kexec Makefile
arm64: Add missing kexec dist files
kexec/arch/arm64/Makefile | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH 1/2] arm64: Cleanup kexec Makefile
From: Geoff Levand @ 2016-10-10 21:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1476134309.git.geoff@infradead.org>
Put files in alphabetical order, reformat whitspace.
Signed-off-by: Geoff Levand <geoff@infradead.org>
---
kexec/arch/arm64/Makefile | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/kexec/arch/arm64/Makefile b/kexec/arch/arm64/Makefile
index 37414dc..d729df8 100644
--- a/kexec/arch/arm64/Makefile
+++ b/kexec/arch/arm64/Makefile
@@ -1,17 +1,18 @@
arm64_FS2DT += kexec/fs2dt.c
-arm64_FS2DT_INCLUDE += -include $(srcdir)/kexec/arch/arm64/kexec-arm64.h \
- -include $(srcdir)/kexec/arch/arm64/crashdump-arm64.h
+arm64_FS2DT_INCLUDE += \
+ -include $(srcdir)/kexec/arch/arm64/crashdump-arm64.h \
+ -include $(srcdir)/kexec/arch/arm64/kexec-arm64.h
arm64_DT_OPS += kexec/dt-ops.c
arm64_CPPFLAGS += -I $(srcdir)/kexec/
arm64_KEXEC_SRCS += \
+ kexec/arch/arm64/crashdump-arm64.c \
kexec/arch/arm64/kexec-arm64.c \
- kexec/arch/arm64/kexec-image-arm64.c \
kexec/arch/arm64/kexec-elf-arm64.c \
- kexec/arch/arm64/crashdump-arm64.c
+ kexec/arch/arm64/kexec-image-arm64.c
arm64_ARCH_REUSE_INITRD =
arm64_ADD_SEGMENT =
@@ -19,9 +20,9 @@ arm64_VIRT_TO_PHYS =
arm64_PHYS_TO_VIRT =
dist += $(arm64_KEXEC_SRCS) \
- kexec/arch/arm64/Makefile \
+ kexec/arch/arm64/crashdump-arm64.h \
kexec/arch/arm64/kexec-arm64.h \
- kexec/arch/arm64/crashdump-arm64.h
+ kexec/arch/arm64/Makefile
ifdef HAVE_LIBFDT
--
2.7.4
^ permalink raw reply related
* [PATCH v3 3/6] pwm: imx: support output polarity inversion
From: Stefan Agner @ 2016-10-10 21:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161007151129.6043-4-bhuvanchandra.dv@toradex.com>
On 2016-10-07 08:11, Bhuvanchandra DV wrote:
> From: Lothar Wassmann <LW@KARO-electronics.de>
>
> The i.MX pwm unit on i.MX27 and newer SoCs provides a configurable output
> polarity. This patch adds support to utilize this feature where available.
>
> Signed-off-by: Lothar Wa?mann <LW@KARO-electronics.de>
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> Documentation/devicetree/bindings/pwm/imx-pwm.txt | 6 +--
> drivers/pwm/pwm-imx.c | 51 +++++++++++++++++++++--
> 2 files changed, 51 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> index e00c2e9..c61bdf8 100644
> --- a/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> +++ b/Documentation/devicetree/bindings/pwm/imx-pwm.txt
> @@ -6,8 +6,8 @@ Required properties:
> - "fsl,imx1-pwm" for PWM compatible with the one integrated on i.MX1
> - "fsl,imx27-pwm" for PWM compatible with the one integrated on i.MX27
> - reg: physical base address and length of the controller's registers
> -- #pwm-cells: should be 2. See pwm.txt in this directory for a description of
> - the cells format.
> +- #pwm-cells: 2 for i.MX1 and 3 for i.MX27 and newer SoCs. See pwm.txt
> + in this directory for a description of the cells format.
> - clocks : Clock specifiers for both ipg and per clocks.
> - clock-names : Clock names should include both "ipg" and "per"
> See the clock consumer binding,
> @@ -17,7 +17,7 @@ See the clock consumer binding,
> Example:
>
> pwm1: pwm at 53fb4000 {
> - #pwm-cells = <2>;
> + #pwm-cells = <3>;
> compatible = "fsl,imx53-pwm", "fsl,imx27-pwm";
> reg = <0x53fb4000 0x4000>;
> clocks = <&clks IMX5_CLK_PWM1_IPG_GATE>,
> diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> index d600fd5..c37d223 100644
> --- a/drivers/pwm/pwm-imx.c
> +++ b/drivers/pwm/pwm-imx.c
> @@ -38,6 +38,7 @@
> #define MX3_PWMCR_DOZEEN (1 << 24)
> #define MX3_PWMCR_WAITEN (1 << 23)
> #define MX3_PWMCR_DBGEN (1 << 22)
> +#define MX3_PWMCR_POUTC (1 << 18)
> #define MX3_PWMCR_CLKSRC_IPG_HIGH (2 << 16)
> #define MX3_PWMCR_CLKSRC_IPG (1 << 16)
> #define MX3_PWMCR_SWR (1 << 3)
> @@ -180,6 +181,9 @@ static int imx_pwm_config_v2(struct pwm_chip *chip,
> if (enable)
> cr |= MX3_PWMCR_EN;
>
> + if (pwm->args.polarity == PWM_POLARITY_INVERSED)
> + cr |= MX3_PWMCR_POUTC;
> +
This seems wrong to me, the config callback is meant for period/duty
cycle only. The set_polarity callback should get called in case a
different polarity is requested.
> writel(cr, imx->mmio_base + MX3_PWMCR);
>
> return 0;
> @@ -240,27 +244,62 @@ static void imx_pwm_disable(struct pwm_chip
> *chip, struct pwm_device *pwm)
> clk_disable_unprepare(imx->clk_per);
> }
>
> -static struct pwm_ops imx_pwm_ops = {
> +static int imx_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
> + enum pwm_polarity polarity)
> +{
> + struct imx_chip *imx = to_imx_chip(chip);
> + u32 val;
> +
> + if (polarity == pwm->args.polarity)
> + return 0;
I don't think that this is right. Today, pwm_apply_args (in
include/linux/pwm.h) copies the polarity from args to state.polarity,
which is then passed as polarity argument to this function. So this will
always return 0 afaict.
I would just drop that.
There is probably one little problem in the current state of affairs: If
the bootloader makes use of a PWM channel with inverted state, then the
kernel would not know about that and currently assume a wrong initial
state... I guess at one point in time we should implement the state
retrieval callback and move to the new atomic PWM API, which would mean
to implement apply callback.
--
Stefan
> +
> + val = readl(imx->mmio_base + MX3_PWMCR);
> +
> + if (polarity == PWM_POLARITY_INVERSED)
> + val |= MX3_PWMCR_POUTC;
> + else
> + val &= ~MX3_PWMCR_POUTC;
> +
> + writel(val, imx->mmio_base + MX3_PWMCR);
> +
> + dev_dbg(imx->chip.dev, "%s: polarity set to %s\n", __func__,
> + polarity == PWM_POLARITY_INVERSED ? "inverted" : "normal");
> +
> + return 0;
> +}
> +
> +static struct pwm_ops imx_pwm_ops_v1 = {
> .enable = imx_pwm_enable,
> .disable = imx_pwm_disable,
> .config = imx_pwm_config,
> .owner = THIS_MODULE,
> };
>
> +static struct pwm_ops imx_pwm_ops_v2 = {
> + .enable = imx_pwm_enable,
> + .disable = imx_pwm_disable,
> + .set_polarity = imx_pwm_set_polarity,
> + .config = imx_pwm_config,
> + .owner = THIS_MODULE,
> +};
> +
> struct imx_pwm_data {
> int (*config)(struct pwm_chip *chip,
> struct pwm_device *pwm, int duty_ns, int period_ns);
> void (*set_enable)(struct pwm_chip *chip, bool enable);
> + struct pwm_ops *pwm_ops;
> };
>
> static struct imx_pwm_data imx_pwm_data_v1 = {
> .config = imx_pwm_config_v1,
> .set_enable = imx_pwm_set_enable_v1,
> + .pwm_ops = &imx_pwm_ops_v1,
> };
>
> static struct imx_pwm_data imx_pwm_data_v2 = {
> .config = imx_pwm_config_v2,
> .set_enable = imx_pwm_set_enable_v2,
> + .pwm_ops = &imx_pwm_ops_v2,
> };
>
> static const struct of_device_id imx_pwm_dt_ids[] = {
> @@ -282,6 +321,8 @@ static int imx_pwm_probe(struct platform_device *pdev)
> if (!of_id)
> return -ENODEV;
>
> + data = of_id->data;
> +
> imx = devm_kzalloc(&pdev->dev, sizeof(*imx), GFP_KERNEL);
> if (imx == NULL)
> return -ENOMEM;
> @@ -300,18 +341,22 @@ static int imx_pwm_probe(struct platform_device *pdev)
> return PTR_ERR(imx->clk_ipg);
> }
>
> - imx->chip.ops = &imx_pwm_ops;
> + imx->chip.ops = data->pwm_ops;
> imx->chip.dev = &pdev->dev;
> imx->chip.base = -1;
> imx->chip.npwm = 1;
> imx->chip.can_sleep = true;
> + if (data->pwm_ops->set_polarity) {
> + dev_dbg(&pdev->dev, "PWM supports output inversion\n");
> + imx->chip.of_xlate = of_pwm_xlate_with_flags;
> + imx->chip.of_pwm_n_cells = 3;
> + }
>
> r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> imx->mmio_base = devm_ioremap_resource(&pdev->dev, r);
> if (IS_ERR(imx->mmio_base))
> return PTR_ERR(imx->mmio_base);
>
> - data = of_id->data;
> imx->config = data->config;
> imx->set_enable = data->set_enable;
^ permalink raw reply
* [PATCH 1/1 v8] ARM: imx: Added perf functionality to mmdc driver
From: Zhi Li @ 2016-10-10 20:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHrpEqQE5w_+4tn1SnFs8rQnWXRb86LDMSGxE7xCUDE=8WnbyQ@mail.gmail.com>
On Mon, Sep 26, 2016 at 11:40 AM, Zhi Li <lznuaa@gmail.com> wrote:
> On Mon, Sep 19, 2016 at 12:57 PM, Frank Li <Frank.Li@nxp.com> wrote:
>> From: Zhengyu Shen <zhengyu.shen@nxp.com>
>>
>> MMDC is a multi-mode DDR controller that supports DDR3/DDR3L x16/x32/x64
>> and LPDDR2 two channel x16/x32 memory types. MMDC is configurable, high
>> performance, and optimized. MMDC is present on i.MX6 Quad and i.MX6
>> QuadPlus devices, but this driver only supports i.MX6 Quad at the moment.
>> MMDC provides registers for performance counters which read via this
>> driver to help debug memory throughput and similar issues.
>>
>> $ perf stat -a -e mmdc/busy-cycles/,mmdc/read-accesses/,mmdc/read-bytes/,mmdc/total-cycles/,mmdc/write-accesses/,mmdc/write-bytes/ dd if=/dev/zero of=/dev/null bs=1M count=5000
>> Performance counter stats for 'dd if=/dev/zero of=/dev/null bs=1M count=5000':
>>
>> 898021787 mmdc/busy-cycles/
>> 14819600 mmdc/read-accesses/
>> 471.30 MB mmdc/read-bytes/
>> 2815419216 mmdc/total-cycles/
>> 13367354 mmdc/write-accesses/
>> 427.76 MB mmdc/write-bytes/
>>
>> 5.334757334 seconds time elapsed
>>
>> Signed-off-by: Zhengyu Shen <zhengyu.shen@nxp.com>
>> Signed-off-by: Frank Li <frank.li@nxp.com>
>> ---
>
> Mark:
> Any additional comments for this version?
>
Shawn:
No any new comment for more than 2 weeks.
Did you plan accept this patch?
> best regards
> Frank Li
>
>> Changes from v7 to v8
>> fix a group event check problem, should be slibling.
>>
>> Changes from v6 to v7
>> use mmdc_pmu prefix
>> remove unnecessary check
>> improve group event check according to mark's feedback.
>> check pmu_mmdc->mmdc_events[cfg] at event_add
>> only check == 0 at event_del
>>
>> Changes from v5 to v6
>> Improve group event error handle
>>
>> Changes from v4 to v5
>> Remove mmdc_pmu:irq
>> remove static variable cpuhp_mmdc_pmu
>> remove spin_lock
>> check is_sampling_event(event)
>> remove unnecessary cast
>> use hw_perf_event::prev_count
>>
>> Changes from v3 to v4:
>> Tested and fixed crash relating to removing events with perf fuzzer
>> Adjusted formatting
>> Moved all perf event code under CONFIG_PERF_EVENTS
>> Switched cpuhp_setup_state to cpuhp_setup_state_nocalls
>>
>> Changes from v2 to v3:
>> Use WARN_ONCE instead of returning generic error values
>> Replace CPU Notifiers with newer state machine hotplug
>> Added additional checks on event_init for grouping and sampling
>> Remove useless mmdc_enable_profiling function
>> Added comments
>> Moved start index of events from 0x01 to 0x00
>> Added a counter to pmu_mmdc to only stop hrtimer after all events are finished
>> Replace readl_relaxed and writel_relaxed with readl and writel
>> Removed duplicate update function
>> Used devm_kasprintf when naming mmdcs probed
>>
>> Changes from v1 to v2:
>> Added cpumask and migration handling support to driver
>> Validated event during event_init
>> Added code to properly stop counters
>> Used perf_invalid_context instead of perf_sw_context
>> Added hrtimer to poll for overflow
>> Added better description
>> Added support for multiple mmdcs
>>
>> arch/arm/mach-imx/mmdc.c | 459 ++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 457 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-imx/mmdc.c b/arch/arm/mach-imx/mmdc.c
>> index db9621c..d82d14c 100644
>> --- a/arch/arm/mach-imx/mmdc.c
>> +++ b/arch/arm/mach-imx/mmdc.c
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright 2011 Freescale Semiconductor, Inc.
>> + * Copyright 2011,2016 Freescale Semiconductor, Inc.
>> * Copyright 2011 Linaro Ltd.
>> *
>> * The code contained herein is licensed under the GNU General Public
>> @@ -10,12 +10,16 @@
>> * http://www.gnu.org/copyleft/gpl.html
>> */
>>
>> +#include <linux/hrtimer.h>
>> #include <linux/init.h>
>> +#include <linux/interrupt.h>
>> #include <linux/io.h>
>> #include <linux/module.h>
>> #include <linux/of.h>
>> #include <linux/of_address.h>
>> #include <linux/of_device.h>
>> +#include <linux/perf_event.h>
>> +#include <linux/slab.h>
>>
>> #include "common.h"
>>
>> @@ -27,8 +31,458 @@
>> #define BM_MMDC_MDMISC_DDR_TYPE 0x18
>> #define BP_MMDC_MDMISC_DDR_TYPE 0x3
>>
>> +#define TOTAL_CYCLES 0x0
>> +#define BUSY_CYCLES 0x1
>> +#define READ_ACCESSES 0x2
>> +#define WRITE_ACCESSES 0x3
>> +#define READ_BYTES 0x4
>> +#define WRITE_BYTES 0x5
>> +
>> +/* Enables, resets, freezes, overflow profiling*/
>> +#define DBG_DIS 0x0
>> +#define DBG_EN 0x1
>> +#define DBG_RST 0x2
>> +#define PRF_FRZ 0x4
>> +#define CYC_OVF 0x8
>> +
>> +#define MMDC_MADPCR0 0x410
>> +#define MMDC_MADPSR0 0x418
>> +#define MMDC_MADPSR1 0x41C
>> +#define MMDC_MADPSR2 0x420
>> +#define MMDC_MADPSR3 0x424
>> +#define MMDC_MADPSR4 0x428
>> +#define MMDC_MADPSR5 0x42C
>> +
>> +#define MMDC_NUM_COUNTERS 6
>> +
>> +#define to_mmdc_pmu(p) container_of(p, struct mmdc_pmu, pmu)
>> +
>> static int ddr_type;
>>
>> +#ifdef CONFIG_PERF_EVENTS
>> +
>> +static DEFINE_IDA(mmdc_ida);
>> +
>> +PMU_EVENT_ATTR_STRING(total-cycles, mmdc_pmu_total_cycles, "event=0x00")
>> +PMU_EVENT_ATTR_STRING(busy-cycles, mmdc_pmu_busy_cycles, "event=0x01")
>> +PMU_EVENT_ATTR_STRING(read-accesses, mmdc_pmu_read_accesses, "event=0x02")
>> +PMU_EVENT_ATTR_STRING(write-accesses, mmdc_pmu_write_accesses, "config=0x03")
>> +PMU_EVENT_ATTR_STRING(read-bytes, mmdc_pmu_read_bytes, "event=0x04")
>> +PMU_EVENT_ATTR_STRING(read-bytes.unit, mmdc_pmu_read_bytes_unit, "MB");
>> +PMU_EVENT_ATTR_STRING(read-bytes.scale, mmdc_pmu_read_bytes_scale, "0.000001");
>> +PMU_EVENT_ATTR_STRING(write-bytes, mmdc_pmu_write_bytes, "event=0x05")
>> +PMU_EVENT_ATTR_STRING(write-bytes.unit, mmdc_pmu_write_bytes_unit, "MB");
>> +PMU_EVENT_ATTR_STRING(write-bytes.scale, mmdc_pmu_write_bytes_scale, "0.000001");
>> +
>> +struct mmdc_pmu {
>> + struct pmu pmu;
>> + void __iomem *mmdc_base;
>> + cpumask_t cpu;
>> + struct hrtimer hrtimer;
>> + unsigned int active_events;
>> + struct device *dev;
>> + struct perf_event *mmdc_events[MMDC_NUM_COUNTERS];
>> + struct hlist_node node;
>> +};
>> +
>> +/*
>> + * Polling period is set to one second, overflow of total-cycles (the fastest
>> + * increasing counter) takes ten seconds so one second is safe
>> + */
>> +static unsigned int mmdc_pmu_poll_period_us = 1000000;
>> +
>> +module_param_named(pmu_pmu_poll_period_us, mmdc_pmu_poll_period_us, uint,
>> + S_IRUGO | S_IWUSR);
>> +
>> +static ktime_t mmdc_pmu_timer_period(void)
>> +{
>> + return ns_to_ktime((u64)mmdc_pmu_poll_period_us * 1000);
>> +}
>> +
>> +static ssize_t mmdc_pmu_cpumask_show(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> + struct mmdc_pmu *pmu_mmdc = dev_get_drvdata(dev);
>> +
>> + return cpumap_print_to_pagebuf(true, buf, &pmu_mmdc->cpu);
>> +}
>> +
>> +static struct device_attribute mmdc_pmu_cpumask_attr =
>> + __ATTR(cpumask, S_IRUGO, mmdc_pmu_cpumask_show, NULL);
>> +
>> +static struct attribute *mmdc_pmu_cpumask_attrs[] = {
>> + &mmdc_pmu_cpumask_attr.attr,
>> + NULL,
>> +};
>> +
>> +static struct attribute_group mmdc_pmu_cpumask_attr_group = {
>> + .attrs = mmdc_pmu_cpumask_attrs,
>> +};
>> +
>> +static struct attribute *mmdc_pmu_events_attrs[] = {
>> + &mmdc_pmu_total_cycles.attr.attr,
>> + &mmdc_pmu_busy_cycles.attr.attr,
>> + &mmdc_pmu_read_accesses.attr.attr,
>> + &mmdc_pmu_write_accesses.attr.attr,
>> + &mmdc_pmu_read_bytes.attr.attr,
>> + &mmdc_pmu_read_bytes_unit.attr.attr,
>> + &mmdc_pmu_read_bytes_scale.attr.attr,
>> + &mmdc_pmu_write_bytes.attr.attr,
>> + &mmdc_pmu_write_bytes_unit.attr.attr,
>> + &mmdc_pmu_write_bytes_scale.attr.attr,
>> + NULL,
>> +};
>> +
>> +static struct attribute_group mmdc_pmu_events_attr_group = {
>> + .name = "events",
>> + .attrs = mmdc_pmu_events_attrs,
>> +};
>> +
>> +PMU_FORMAT_ATTR(event, "config:0-63");
>> +static struct attribute *mmdc_pmu_format_attrs[] = {
>> + &format_attr_event.attr,
>> + NULL,
>> +};
>> +
>> +static struct attribute_group mmdc_pmu_format_attr_group = {
>> + .name = "format",
>> + .attrs = mmdc_pmu_format_attrs,
>> +};
>> +
>> +static const struct attribute_group *attr_groups[] = {
>> + &mmdc_pmu_events_attr_group,
>> + &mmdc_pmu_format_attr_group,
>> + &mmdc_pmu_cpumask_attr_group,
>> + NULL,
>> +};
>> +
>> +static u32 mmdc_pmu_read_counter(struct mmdc_pmu *pmu_mmdc, int cfg)
>> +{
>> + void __iomem *mmdc_base, *reg;
>> +
>> + mmdc_base = pmu_mmdc->mmdc_base;
>> +
>> + switch (cfg) {
>> + case TOTAL_CYCLES:
>> + reg = mmdc_base + MMDC_MADPSR0;
>> + break;
>> + case BUSY_CYCLES:
>> + reg = mmdc_base + MMDC_MADPSR1;
>> + break;
>> + case READ_ACCESSES:
>> + reg = mmdc_base + MMDC_MADPSR2;
>> + break;
>> + case WRITE_ACCESSES:
>> + reg = mmdc_base + MMDC_MADPSR3;
>> + break;
>> + case READ_BYTES:
>> + reg = mmdc_base + MMDC_MADPSR4;
>> + break;
>> + case WRITE_BYTES:
>> + reg = mmdc_base + MMDC_MADPSR5;
>> + break;
>> + default:
>> + return WARN_ONCE(1,
>> + "invalid configuration %d for mmdc counter", cfg);
>> + }
>> + return readl(reg);
>> +}
>> +
>> +static int mmdc_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
>> +{
>> + struct mmdc_pmu *pmu_mmdc = hlist_entry_safe(node, struct mmdc_pmu, node);
>> + int target;
>> +
>> + if (!cpumask_test_and_clear_cpu(cpu, &pmu_mmdc->cpu))
>> + return 0;
>> +
>> + target = cpumask_any_but(cpu_online_mask, cpu);
>> + if (target >= nr_cpu_ids)
>> + return 0;
>> +
>> + perf_pmu_migrate_context(&pmu_mmdc->pmu, cpu, target);
>> + cpumask_set_cpu(target, &pmu_mmdc->cpu);
>> +
>> + return 0;
>> +}
>> +
>> +static bool mmdc_pmu_group_event_is_valid(struct perf_event *event,
>> + struct pmu *pmu,
>> + unsigned long *used_counters)
>> +{
>> + int cfg = event->attr.config;
>> +
>> + if (is_software_event(event))
>> + return true;
>> +
>> + if (event->pmu != pmu)
>> + return false;
>> +
>> + return !test_and_set_bit(cfg, used_counters);
>> +}
>> +
>> +/*
>> + * Each event has a single fixed-purpose counter, so we can only have a
>> + * single active event for each at any point in time. Here we just check
>> + * for duplicates, and rely on mmdc_pmu_event_init to verify that the HW
>> + * event numbers are valid.
>> + */
>> +static bool mmdc_pmu_group_is_valid(struct perf_event *event)
>> +{
>> + struct pmu *pmu = event->pmu;
>> + struct perf_event *leader = event->group_leader;
>> + struct perf_event *sibling;
>> + unsigned long counter_mask = 0;
>> +
>> + set_bit(leader->attr.config, &counter_mask);
>> +
>> + if (event != leader) {
>> + if (!mmdc_pmu_group_event_is_valid(event, pmu, &counter_mask))
>> + return false;
>> + }
>> +
>> + list_for_each_entry(sibling, &leader->sibling_list, group_entry) {
>> + if (!mmdc_pmu_group_event_is_valid(sibling, pmu, &counter_mask))
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static int mmdc_pmu_event_init(struct perf_event *event)
>> +{
>> + struct mmdc_pmu *pmu_mmdc = to_mmdc_pmu(event->pmu);
>> + int cfg = event->attr.config;
>> +
>> + if (event->attr.type != event->pmu->type)
>> + return -ENOENT;
>> +
>> + if (is_sampling_event(event) || event->attach_state & PERF_ATTACH_TASK)
>> + return -EOPNOTSUPP;
>> +
>> + if (event->cpu < 0) {
>> + dev_warn(pmu_mmdc->dev, "Can't provide per-task data!\n");
>> + return -EOPNOTSUPP;
>> + }
>> +
>> + if (event->attr.exclude_user ||
>> + event->attr.exclude_kernel ||
>> + event->attr.exclude_hv ||
>> + event->attr.exclude_idle ||
>> + event->attr.exclude_host ||
>> + event->attr.exclude_guest ||
>> + event->attr.sample_period)
>> + return -EINVAL;
>> +
>> + if (cfg < 0 || cfg >= MMDC_NUM_COUNTERS)
>> + return -EINVAL;
>> +
>> + if (!mmdc_pmu_group_is_valid(event))
>> + return -EINVAL;
>> +
>> + event->cpu = cpumask_first(&pmu_mmdc->cpu);
>> + return 0;
>> +}
>> +
>> +static void mmdc_pmu_event_update(struct perf_event *event)
>> +{
>> + struct mmdc_pmu *pmu_mmdc = to_mmdc_pmu(event->pmu);
>> + struct hw_perf_event *hwc = &event->hw;
>> + u64 delta, prev_raw_count, new_raw_count;
>> +
>> + do {
>> + prev_raw_count = local64_read(&hwc->prev_count);
>> + new_raw_count = mmdc_pmu_read_counter(pmu_mmdc,
>> + event->attr.config);
>> + } while (local64_cmpxchg(&hwc->prev_count, prev_raw_count,
>> + new_raw_count) != prev_raw_count);
>> +
>> + delta = (new_raw_count - prev_raw_count) & 0xFFFFFFFF;
>> +
>> + local64_add(delta, &event->count);
>> +}
>> +
>> +static void mmdc_pmu_event_start(struct perf_event *event, int flags)
>> +{
>> + struct mmdc_pmu *pmu_mmdc = to_mmdc_pmu(event->pmu);
>> + struct hw_perf_event *hwc = &event->hw;
>> + void __iomem *mmdc_base, *reg;
>> +
>> + mmdc_base = pmu_mmdc->mmdc_base;
>> + reg = mmdc_base + MMDC_MADPCR0;
>> +
>> + /*
>> + * hrtimer is required because mmdc does not provide an interrupt so
>> + * polling is necessary
>> + */
>> + hrtimer_start(&pmu_mmdc->hrtimer, mmdc_pmu_timer_period(),
>> + HRTIMER_MODE_REL_PINNED);
>> +
>> + local64_set(&hwc->prev_count, 0);
>> +
>> + writel(DBG_RST, reg);
>> + writel(DBG_EN, reg);
>> +}
>> +
>> +static int mmdc_pmu_event_add(struct perf_event *event, int flags)
>> +{
>> + struct mmdc_pmu *pmu_mmdc = to_mmdc_pmu(event->pmu);
>> + struct hw_perf_event *hwc = &event->hw;
>> +
>> + int cfg = event->attr.config;
>> +
>> + if (flags & PERF_EF_START)
>> + mmdc_pmu_event_start(event, flags);
>> +
>> + if (pmu_mmdc->mmdc_events[cfg] != NULL)
>> + return -EAGAIN;
>> +
>> + pmu_mmdc->mmdc_events[cfg] = event;
>> + pmu_mmdc->active_events++;
>> +
>> + local64_set(&hwc->prev_count, mmdc_pmu_read_counter(pmu_mmdc, cfg));
>> +
>> + return 0;
>> +}
>> +
>> +static void mmdc_pmu_event_stop(struct perf_event *event, int flags)
>> +{
>> + struct mmdc_pmu *pmu_mmdc = to_mmdc_pmu(event->pmu);
>> + void __iomem *mmdc_base, *reg;
>> +
>> + mmdc_base = pmu_mmdc->mmdc_base;
>> + reg = mmdc_base + MMDC_MADPCR0;
>> +
>> + writel(PRF_FRZ, reg);
>> + mmdc_pmu_event_update(event);
>> +}
>> +
>> +static void mmdc_pmu_event_del(struct perf_event *event, int flags)
>> +{
>> + struct mmdc_pmu *pmu_mmdc = to_mmdc_pmu(event->pmu);
>> + int cfg = event->attr.config;
>> +
>> + pmu_mmdc->mmdc_events[cfg] = NULL;
>> + pmu_mmdc->active_events--;
>> +
>> + if (pmu_mmdc->active_events == 0)
>> + hrtimer_cancel(&pmu_mmdc->hrtimer);
>> +
>> + mmdc_pmu_event_stop(event, PERF_EF_UPDATE);
>> +}
>> +
>> +static void mmdc_pmu_overflow_handler(struct mmdc_pmu *pmu_mmdc)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < MMDC_NUM_COUNTERS; i++) {
>> + struct perf_event *event = pmu_mmdc->mmdc_events[i];
>> +
>> + if (event)
>> + mmdc_pmu_event_update(event);
>> + }
>> +}
>> +
>> +static enum hrtimer_restart mmdc_pmu_timer_handler(struct hrtimer *hrtimer)
>> +{
>> + struct mmdc_pmu *pmu_mmdc = container_of(hrtimer, struct mmdc_pmu,
>> + hrtimer);
>> +
>> + mmdc_pmu_overflow_handler(pmu_mmdc);
>> + hrtimer_forward_now(hrtimer, mmdc_pmu_timer_period());
>> +
>> + return HRTIMER_RESTART;
>> +}
>> +
>> +static int mmdc_pmu_init(struct mmdc_pmu *pmu_mmdc,
>> + void __iomem *mmdc_base, struct device *dev)
>> +{
>> + int mmdc_num;
>> +
>> + *pmu_mmdc = (struct mmdc_pmu) {
>> + .pmu = (struct pmu) {
>> + .task_ctx_nr = perf_invalid_context,
>> + .attr_groups = attr_groups,
>> + .event_init = mmdc_pmu_event_init,
>> + .add = mmdc_pmu_event_add,
>> + .del = mmdc_pmu_event_del,
>> + .start = mmdc_pmu_event_start,
>> + .stop = mmdc_pmu_event_stop,
>> + .read = mmdc_pmu_event_update,
>> + },
>> + .mmdc_base = mmdc_base,
>> + .dev = dev,
>> + .active_events = 0,
>> + };
>> +
>> + mmdc_num = ida_simple_get(&mmdc_ida, 0, 0, GFP_KERNEL);
>> +
>> + return mmdc_num;
>> +}
>> +
>> +static int imx_mmdc_remove(struct platform_device *pdev)
>> +{
>> + struct mmdc_pmu *pmu_mmdc = platform_get_drvdata(pdev);
>> +
>> + perf_pmu_unregister(&pmu_mmdc->pmu);
>> + cpuhp_remove_state_nocalls(CPUHP_ONLINE);
>> + kfree(pmu_mmdc);
>> + return 0;
>> +}
>> +
>> +static int imx_mmdc_perf_init(struct platform_device *pdev, void __iomem *mmdc_base)
>> +{
>> + struct mmdc_pmu *pmu_mmdc;
>> + char *name;
>> + int mmdc_num;
>> + int ret;
>> +
>> + pmu_mmdc = kzalloc(sizeof(*pmu_mmdc), GFP_KERNEL);
>> + if (!pmu_mmdc) {
>> + pr_err("failed to allocate PMU device!\n");
>> + return -ENOMEM;
>> + }
>> +
>> + mmdc_num = mmdc_pmu_init(pmu_mmdc, mmdc_base, &pdev->dev);
>> + if (mmdc_num == 0)
>> + name = "mmdc";
>> + else
>> + name = devm_kasprintf(&pdev->dev,
>> + GFP_KERNEL, "mmdc%d", mmdc_num);
>> +
>> + hrtimer_init(&pmu_mmdc->hrtimer, CLOCK_MONOTONIC,
>> + HRTIMER_MODE_REL);
>> + pmu_mmdc->hrtimer.function = mmdc_pmu_timer_handler;
>> +
>> + cpuhp_state_add_instance_nocalls(CPUHP_ONLINE,
>> + &pmu_mmdc->node);
>> + cpumask_set_cpu(smp_processor_id(), &pmu_mmdc->cpu);
>> + ret = cpuhp_setup_state_multi(CPUHP_AP_NOTIFY_ONLINE,
>> + "MMDC_ONLINE", NULL,
>> + mmdc_pmu_offline_cpu);
>> + if (ret) {
>> + pr_err("cpuhp_setup_state_multi failure\n");
>> + goto pmu_register_err;
>> + }
>> +
>> + ret = perf_pmu_register(&(pmu_mmdc->pmu), name, -1);
>> + platform_set_drvdata(pdev, pmu_mmdc);
>> + if (ret)
>> + goto pmu_register_err;
>> + return 0;
>> +
>> +pmu_register_err:
>> + pr_warn("MMDC Perf PMU failed (%d), disabled\n", ret);
>> + hrtimer_cancel(&pmu_mmdc->hrtimer);
>> + kfree(pmu_mmdc);
>> + return ret;
>> +}
>> +
>> +#else
>> +#define imx_mmdc_remove NULL
>> +#define imx_mmdc_perf_init(pdev, mmdc_base) 0
>> +#endif
>> +
>> static int imx_mmdc_probe(struct platform_device *pdev)
>> {
>> struct device_node *np = pdev->dev.of_node;
>> @@ -62,7 +516,7 @@ static int imx_mmdc_probe(struct platform_device *pdev)
>> return -EBUSY;
>> }
>>
>> - return 0;
>> + return imx_mmdc_perf_init(pdev, mmdc_base);
>> }
>>
>> int imx_mmdc_get_ddr_type(void)
>> @@ -81,6 +535,7 @@ static struct platform_driver imx_mmdc_driver = {
>> .of_match_table = imx_mmdc_dt_ids,
>> },
>> .probe = imx_mmdc_probe,
>> + .remove = imx_mmdc_remove,
>> };
>>
>> static int __init imx_mmdc_init(void)
>> --
>> 2.5.2
>>
^ permalink raw reply
* [PATCH] PM / AVS: rockchip-io: make the log more consistent
From: Heiko Stuebner @ 2016-10-10 20:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1476103462-7559-1-git-send-email-shawn.lin@rock-chips.com>
Am Montag, 10. Oktober 2016, 20:44:22 CEST schrieb Shawn Lin:
> When testing SD hotplug automatically, I got bunch of
> useless log like this:
>
> [ 588.357838] mmc0: card 0007 removed
> [ 589.492664] rockchip-iodomain ff770000.syscon:io-domains: Setting to
> 3300000 done [ 589.500698] vccio_sd: ramp_delay not set
> [ 589.504817] rockchip-iodomain ff770000.syscon:io-domains: Setting to
> 3300000 done [ 589.669705] rockchip-iodomain ff770000.syscon:io-domains:
> Setting to 3300000 done [ 589.677593] vccio_sd: ramp_delay not set
> [ 589.681581] rockchip-iodomain ff770000.syscon:io-domains: Setting to
> 1800000 done [ 590.032820] dwmmc_rockchip ff0c0000.dwmmc: Successfully
> tuned phase to 140 [ 590.039725] mmc0: new ultra high speed SDR50 SDHC
> card at address 0007 [ 590.046641] mmcblk0: mmc0:0007 SD32G 29.3 GiB
> [ 590.052163] mmcblk0: p1
>
> Moreover the code is intent to print the 'uV' for debug but
> later print it using dev_info. It looks more like to me that
> it should be the real intention of the code. Anyway, let's
> mark this verbose log as debug message.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
reducing the amount of logs, especially for information not generally needed
looks sane
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> ---
>
> drivers/power/avs/rockchip-io-domain.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/power/avs/rockchip-io-domain.c
> b/drivers/power/avs/rockchip-io-domain.c index 01b6d3f..56bce19 100644
> --- a/drivers/power/avs/rockchip-io-domain.c
> +++ b/drivers/power/avs/rockchip-io-domain.c
> @@ -143,7 +143,7 @@ static int rockchip_iodomain_notify(struct
> notifier_block *nb, if (ret && event == REGULATOR_EVENT_PRE_VOLTAGE_CHANGE)
> return NOTIFY_BAD;
>
> - dev_info(supply->iod->dev, "Setting to %d done\n", uV);
> + dev_dbg(supply->iod->dev, "Setting to %d done\n", uV);
> return NOTIFY_OK;
> }
^ permalink raw reply
* [PATCH v19 03/12] add bindings document for altera freeze bridge
From: atull @ 2016-10-10 18:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161008204946.GC11595@rob-hp-laptop>
On Sat, 8 Oct 2016, Rob Herring wrote:
> On Wed, Sep 28, 2016 at 01:21:51PM -0500, Alan Tull wrote:
> > Add bindings document for the Altera Freeze Bridge. A Freeze
> > Bridge is used to gate traffic to/from a region of a FPGA
> > such that that region can be reprogrammed. The Freeze Bridge
> > exist in FPGA fabric that is not currently being reconfigured.
> >
> > Signed-off-by: Alan Tull <atull@opensource.altera.com>
> > Signed-off-by: Matthew Gerlach <mgerlach@opensource.altera.com>
> > ---
> > v19: Added in v19 of patchset, uses fpga image info struct
> > ---
> > .../bindings/fpga/altera-freeze-bridge.txt | 23 ++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/fpga/altera-freeze-bridge.txt
> >
> > diff --git a/Documentation/devicetree/bindings/fpga/altera-freeze-bridge.txt b/Documentation/devicetree/bindings/fpga/altera-freeze-bridge.txt
> > new file mode 100644
> > index 0000000..97ecc11
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/fpga/altera-freeze-bridge.txt
> > @@ -0,0 +1,23 @@
> > +Altera Freeze Bridge Controller Driver
> > +
> > +The Altera Freeze Bridge Controller manages one or more freeze bridges.
> > +The controller can freeze/disable the bridges which prevents signal
> > +changes from passing through the bridge. The controller can also
> > +unfreeze/enable the bridges which allows traffic to pass through the
> > +bridge normally.
> > +
> > +Required properties:
> > +- compatible : Should contain "altr,freeze-bridge-controller"
>
> Only one version of the h/w?
The h/w has a version register. Currently the driver reads
the reg and exits if rev != 2. Future version support can be
keyed off this register.
>
> > +- regs : base address and size for freeze bridge module
> > +
> > +Optional properties:
> > +- bridge-enable : 0 if driver should disable bridge at startup
> > + 1 if driver should enable bridge at startup
> > + Default is to leave bridge in current state.
> > +
> > +Example:
> > + freeze_controller at 100000450 {
>
> Underscore...
Yes.
>
> > + compatible = "altr,freeze-bridge-controller";
> > + regs = <0x1000 0x10>;
> > + bridge-enable = <0>;
> > + };
> > --
> > 2.9.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe devicetree" in
> > the body of a message to majordomo at vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH v2] sdhci-esdhc-imx: Correct two register accesses
From: Aaron Brice @ 2016-10-10 18:39 UTC (permalink / raw)
To: linux-arm-kernel
- The DMA error interrupt bit is in a different position as
compared to the sdhci standard. This is accounted for in
many cases, but not handled in the case of clearing the
INT_STATUS register by writing a 1 to that location.
- The HOST_CONTROL register is very different as compared to
the sdhci standard. This is accounted for in the write
case, but not when read back out (which it is in the sdhci
code).
Signed-off-by: Dave Russell <david.russell@datasoft.com>
Signed-off-by: Aaron Brice <aaron.brice@datasoft.com>
Acked-by: Dong Aisheng <aisheng.dong@nxp.com>
---
v1 -> v2:
- rename long_val to val
drivers/mmc/host/sdhci-esdhc-imx.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 1f54fd8..7123ef9 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -346,7 +346,8 @@ static void esdhc_writel_le(struct sdhci_host *host, u32 val, int reg)
struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
u32 data;
- if (unlikely(reg == SDHCI_INT_ENABLE || reg == SDHCI_SIGNAL_ENABLE)) {
+ if (unlikely(reg == SDHCI_INT_ENABLE || reg == SDHCI_SIGNAL_ENABLE ||
+ reg == SDHCI_INT_STATUS)) {
if ((val & SDHCI_INT_CARD_INT) && !esdhc_is_usdhc(imx_data)) {
/*
* Clear and then set D3CD bit to avoid missing the
@@ -555,6 +556,25 @@ static void esdhc_writew_le(struct sdhci_host *host, u16 val, int reg)
esdhc_clrset_le(host, 0xffff, val, reg);
}
+static u8 esdhc_readb_le(struct sdhci_host *host, int reg)
+{
+ u8 ret;
+ u32 val;
+
+ switch (reg) {
+ case SDHCI_HOST_CONTROL:
+ val = readl(host->ioaddr + reg);
+
+ ret = val & SDHCI_CTRL_LED;
+ ret |= (val >> 5) & SDHCI_CTRL_DMA_MASK;
+ ret |= (val & ESDHC_CTRL_4BITBUS);
+ ret |= (val & ESDHC_CTRL_8BITBUS) << 3;
+ return ret;
+ }
+
+ return readb(host->ioaddr + reg);
+}
+
static void esdhc_writeb_le(struct sdhci_host *host, u8 val, int reg)
{
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -947,6 +967,7 @@ static void esdhc_set_timeout(struct sdhci_host *host, struct mmc_command *cmd)
static struct sdhci_ops sdhci_esdhc_ops = {
.read_l = esdhc_readl_le,
.read_w = esdhc_readw_le,
+ .read_b = esdhc_readb_le,
.write_l = esdhc_writel_le,
.write_w = esdhc_writew_le,
.write_b = esdhc_writeb_le,
--
2.7.4
^ permalink raw reply related
* [PATCH 1/3] fpga manager: Add cyclonespi driver for Altera fpgas
From: atull @ 2016-10-10 18:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAtXAHfax5qve7bpxNoObZOZGZk1CoJkusMecMXyMc_w1JBbvw@mail.gmail.com>
On Fri, 7 Oct 2016, Moritz Fischer wrote:
> Hi Alan,
>
> On Fri, Oct 7, 2016 at 11:21 AM, atull <atull@opensource.altera.com> wrote:
>
> > Moritz, Can you remind me what that issue was there (or point me to
> > that email, I can't find it)? I don't think I had a problem with that
> > in your case. In general I think if these drivers can take the
> > bitstream that comes from the manufacturer's tools and stuff it into
> > the FPGA, then we are accomplishing what we want. So I am OK with
> > this here. The intent of the driver is to load a standard rbf, same
> > as the other Altera FPGA drivers.
>
> My first version of the zynq fpga manager had byte swapping in there
> and detection of which format was used. Greg even (accidentially)
> merged my initial code
> which I then cleaned up in 4d10eaff5bfc69997a769f9c83b749f0a8c542fa
> ("fpga: zynq-fpga: Change fw format to handle bin instead of bit.") to
> address the review
> comments.
>
> I do see your point about useability, and if this is something that
> keeps coming up
> we could pull that into the framework with a flag to SWAP or as part
> of the image_information
> struct.
>
> Thoughts?
I agree. Some day we may want to see it useful to extend
the flow to do that. Currently it's not needed so we can
wait until we see clearly what is needed.
Alan
>
> Cheers,
>
> Moritz
>
> [1] https://mail-archive.com/linux-kernel at vger.kernel.org/msg995245.html
>
^ permalink raw reply
* [PATCH 1/3] fpga manager: Add cyclonespi driver for Altera fpgas
From: atull @ 2016-10-10 18:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4b4432c04b4ea92a2af814e3d7866c33f2eb12ea.1475783742.git.stillcompiling@gmail.com>
On Thu, 6 Oct 2016, Joshua Clayton wrote:
> cyclonespi loads fpga firmware over spi, using the "passive serial"
> interface on Altera Cyclone FPGAS.
>
> one of the simpler ways to set up an fpga at runtime.
> The signal interface is close to unidirectional spi with lsb first.
>
> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
> ---
> drivers/fpga/Kconfig | 6 ++
> drivers/fpga/Makefile | 1 +
> drivers/fpga/cyclonespi.c | 173 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 180 insertions(+)
> create mode 100644 drivers/fpga/cyclonespi.c
>
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index cd84934..ccad5b1 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -13,6 +13,12 @@ config FPGA
>
Hi Joshua,
This looks really good. Mostly minor comments.
> if FPGA
>
> +config FPGA_MGR_CYCLONE_SPI
Suggestions here and below to keep PS in the name of things.
Such as FPGA_MGR_CYCLONE_PS_SPI.
> + tristate "Altera Cyclone V SPI"
"Altera Cyclone V Passive Serial over SPI" ?
> + depends on SPI
> + help
> + FPGA manager driver support for Altera Cyclone V over SPI
> +
> config FPGA_MGR_SOCFPGA
> tristate "Altera SOCFPGA FPGA Manager"
> depends on ARCH_SOCFPGA
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 8d83fc6..c03f40de 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -6,5 +6,6 @@
> obj-$(CONFIG_FPGA) += fpga-mgr.o
>
> # FPGA Manager Drivers
> +obj-$(CONFIG_FPGA_MGR_CYCLONE_SPI) += cyclonespi.o
cyclone-ps-spi.o and change name of c file.
> obj-$(CONFIG_FPGA_MGR_SOCFPGA) += socfpga.o
> obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA) += zynq-fpga.o
> diff --git a/drivers/fpga/cyclonespi.c b/drivers/fpga/cyclonespi.c
> new file mode 100644
> index 0000000..1ffa67c
> --- /dev/null
> +++ b/drivers/fpga/cyclonespi.c
> @@ -0,0 +1,173 @@
> +/**
> + * Copyright (c) 2015 United Western Technologies, Corporation
> + *
> + * Joshua Clayton <stillcompiling@gmail.com>
> + *
> + * Manage Altera fpga firmware that is loaded over spi.
> + * Firmware must be in binary "rbf" format.
> + * Works on Cyclone V. Should work on cyclone series.
> + * May work on other Altera fpgas.
> + *
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of_gpio.h>
> +#include <linux/spi/spi.h>
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Joshua Clayton <stillcompiling@gmail.com>");
> +MODULE_DESCRIPTION("Module to load Altera FPGA firmware over spi");
Please move these 3 MODULE_* lines to the bottom of this file.
> +
> +struct cyclonespi_conf {
> + struct gpio_desc *reset;
> + struct gpio_desc *status;
> + struct spi_device *spi;
> +};
> +
> +static const struct of_device_id of_ef_match[] = {
> + { .compatible = "altr,cyclonespi-fpga-mgr", },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, of_ef_match);
> +
> +static enum fpga_mgr_states cyclonespi_state(struct fpga_manager *mgr)
> +{
> + return mgr->state;
fpga-mgr.c's fpga_mgr_register() reads the initial state
from this function. Please return one of the values of enum
fpga_mgr_states here. If state isn't known, then use
FPGA_MGR_STATE_UNKNOWN.
> +}
> +
> +static inline u32 revbit8x4(u32 n)
> +{
> + n = ((n & 0xF0F0F0F0UL) >> 4) | ((n & 0x0F0F0F0FUL) << 4);
> + n = ((n & 0xCCCCCCCCUL) >> 2) | ((n & 0x33333333UL) << 2);
> + n = ((n & 0xAAAAAAAAUL) >> 1) | ((n & 0x55555555UL) << 1);
> + return n;
> +}
> +
> +static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
> + const char *buf, size_t count)
> +{
> + struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
> + u32 *fw32 = (u32 *)buf;
> + const u32 *fw_end = (u32 *)(buf + count);
> +
> + if (flags & FPGA_MGR_PARTIAL_RECONFIG) {
> + dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
> + return -EINVAL;
> + }
> +
> + gpiod_set_value(conf->reset, 0);
> + udelay(50);
Moritz brought up whether the delay/sleep values should be
configurable. It looks like these are set values and have
probably been increased for good measure. These could be
macros named to contain symbol names in the datasheet's PS
timing waveform, if that is where these are coming from.
The timing diagram has the Tcf2st0 as 800nSec max and
Tcf2st1 (below) as 40uSec. I guess either we are looking at
different specs or you just found that you got better
results if you padded the values out?
I would normally suggest a convention of macro names
that maps neatly to the value names in the datasheet
such as CYCLONE_PS_TCF2ST0_USEC. The problem is
the units.
> + if (gpiod_get_value(conf->status) == 1) {
> + dev_err(&mgr->dev, "Status pin should be low.\n");
> + return -EIO;
> + }
> +
> + gpiod_set_value(conf->reset, 1);
> + msleep(1);
CYCLONE_PS_TCF2ST1_USEC? I'm not sure what to suggest
because the units in the timing diagram I'm looking at are
nanoseconds but this ended up being a mSec.
> + if (gpiod_get_value(conf->status) == 0) {
> + dev_err(&mgr->dev, "Status pin not ready.\n");
> + return -EIO;
> + }
> +
> + /* set buffer to lsb first */
> + while (fw32 < fw_end) {
> + *fw32 = revbit8x4(*fw32);
> + fw32++;
> + }
> +
> + return 0;
> +}
> +
> +static int cyclonespi_write(struct fpga_manager *mgr, const char *buf,
> + size_t count)
Please fix checkpatch.pl warnings. Here and a few other places it
says "CHECK: Alignment should match open parenthesis"
> +{
> + struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
> + const char *fw_data = buf;
> + const char *fw_data_end = fw_data + count;
> +
> + while (fw_data < fw_data_end) {
> + int ret;
> + int stride = fw_data_end - fw_data;
> +
> + if (stride > SZ_4K)
> + stride = SZ_4K;
> +
> + ret = spi_write(conf->spi, fw_data, stride);
> + if (ret) {
> + dev_err(&mgr->dev, "spi error in firmware write: %d\n",
> + ret);
> + return ret;
> + }
> + fw_data += stride;
> + }
> +
> + return 0;
> +}
> +
> +static int cyclonespi_write_complete(struct fpga_manager *mgr, u32 flags)
> +{
> + struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
> +
> + if (gpiod_get_value(conf->status) == 0) {
> + dev_err(&mgr->dev, "Error during configuration.\n");
> + return -EIO;
> + }
Does your hardware give you access to the CONF_DONE pin?
If so, can you get better status info from that?
> +
> + return 0;
> +}
> +
> +static const struct fpga_manager_ops cyclonespi_ops = {
> + .state = cyclonespi_state,
> + .write_init = cyclonespi_write_init,
> + .write = cyclonespi_write,
> + .write_complete = cyclonespi_write_complete,
> +};
> +
> +static int cyclonespi_probe(struct spi_device *spi)
> +{
> + struct cyclonespi_conf *conf = devm_kzalloc(&spi->dev, sizeof(*conf),
> + GFP_KERNEL);
> +
> + if (!conf)
> + return -ENOMEM;
> +
> + conf->spi = spi;
> + conf->reset = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(conf->reset)) {
> + dev_err(&spi->dev, "Failed to get reset gpio: %ld\n",
> + PTR_ERR(conf->reset));
> + return PTR_ERR(conf->reset);
> + }
> +
> + conf->status = devm_gpiod_get(&spi->dev, "status", GPIOD_IN);
> + if (IS_ERR(conf->status)) {
> + dev_err(&spi->dev, "Failed to get status gpio: %ld\n",
> + PTR_ERR(conf->status));
> + return PTR_ERR(conf->status);
> + }
> +
> + return fpga_mgr_register(&spi->dev, "Altera SPI FPGA Manager",
> + &cyclonespi_ops, conf);
> +}
> +
> +static int cyclonespi_remove(struct spi_device *spi)
> +{
> + fpga_mgr_unregister(&spi->dev);
> +
> + return 0;
> +}
> +
> +static struct spi_driver cyclonespi_driver = {
> + .driver = {
> + .name = "cyclonespi",
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(of_ef_match),
> + },
> + .probe = cyclonespi_probe,
> + .remove = cyclonespi_remove,
> +};
> +
> +module_spi_driver(cyclonespi_driver)
> --
> 2.7.4
>
>
^ permalink raw reply
* [PATCH] arm64: mmu: set the contiguous for kernel mappings when appropriate
From: Ard Biesheuvel @ 2016-10-10 18:12 UTC (permalink / raw)
To: linux-arm-kernel
Now that we no longer allow live kernel PMDs to be split, it is safe to
start using the contiguous bit for kernel mappings. So set the contiguous
bit in the kernel page mappings for regions whose size and alignment are
suitable for this.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/mm/mmu.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 05615a3fdc6f..c491025c6a70 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -98,8 +98,11 @@ static phys_addr_t __init early_pgtable_alloc(void)
static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
unsigned long end, unsigned long pfn,
pgprot_t prot,
- phys_addr_t (*pgtable_alloc)(void))
+ phys_addr_t (*pgtable_alloc)(void),
+ bool allow_block_mappings)
{
+ pgprot_t prot_cont = __pgprot(pgprot_val(prot) | PTE_CONT);
+ bool cont = false;
pte_t *pte;
BUG_ON(pmd_sect(*pmd));
@@ -115,7 +118,20 @@ static void alloc_init_pte(pmd_t *pmd, unsigned long addr,
pte = pte_set_fixmap_offset(pmd, addr);
do {
- set_pte(pte, pfn_pte(pfn, prot));
+ /*
+ * Set the contiguous bit for the subsequent group of PTEs if
+ * its size and alignment are suitable.
+ */
+ if (((addr | PFN_PHYS(pfn)) & ~CONT_MASK) == 0)
+ cont = allow_block_mappings && end - addr >= CONT_SIZE;
+
+ /*
+ * Ensure that we do not change the contiguous bit once this
+ * PTE has been assigned.
+ */
+ BUG_ON(!pte_none(*pte) && (cont ^ !!(pte_val(*pte) & PTE_CONT)));
+
+ set_pte(pte, pfn_pte(pfn, cont ? prot_cont : prot));
pfn++;
} while (pte++, addr += PAGE_SIZE, addr != end);
@@ -166,7 +182,8 @@ static void alloc_init_pmd(pud_t *pud, unsigned long addr, unsigned long end,
}
} else {
alloc_init_pte(pmd, addr, next, __phys_to_pfn(phys),
- prot, pgtable_alloc);
+ prot, pgtable_alloc,
+ allow_block_mappings);
}
phys += next - addr;
} while (pmd++, addr = next, addr != end);
--
2.7.4
^ permalink raw reply related
* [PATCH] clk: hi6220: use CLK_OF_DECLARE_DRIVER for sysctrl and mediactrl clock init
From: John Stultz @ 2016-10-10 17:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475933892-16021-1-git-send-email-shawn.guo@linaro.org>
On Sat, Oct 8, 2016 at 6:38 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> The hi6220-sysctrl and hi6220-mediactrl are not only clock provider but
> also reset controller. It worked fine that single sysctrl/mediactrl
> device node in DT can be used to initialize clock driver and populate
> platform device for reset controller. But it stops working after
> commit 989eafd0b609 ("clk: core: Avoid double initialization of clocks")
> gets merged. The commit sets flag OF_POPULATED during clock
> initialization to skip the platform device populating for the same
> device node. On hi6220, it effectively makes hi6220-sysctrl reset
> driver not probe any more.
>
> The patch changes hi6220 sysctrl and mediactrl clock init macro from
> CLK_OF_DECLARE to CLK_OF_DECLARE_DRIVER, so that the reset driver using
> the same hardware block can continue working.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Tested-by: John Stultz <john.stultz@linaro.org>
I hit this as well last week when 989eafd0b609 ("clk: core: Avoid
double initialization of clocks") landed, which killed graphics on my
HiKey.
My workaround was a bit hackish, as I don't really know when one
should use OF_DECLARE vs OF_DECLARE_DRIVER, but I also converted the
hi6220_clk_ao and hi6220_clk_power to the _DRIVER side. Your patch
seems to work just as well for me, but I wanted to double check with
you that the ao/power clks didn't need the conversion as well.
thanks
-john
^ permalink raw reply
* [PATCH v2 5/8] dt/bindings: Update binding for PM domain idle states
From: Sudeep Holla @ 2016-10-10 17:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161010164342.GC44885@linaro.org>
On 10/10/16 17:43, Lina Iyer wrote:
> On Mon, Oct 10 2016 at 09:45 -0600, Sudeep Holla wrote:
>>
>>
>> On 07/10/16 23:36, Lina Iyer wrote:
>>> Update DT bindings to describe idle states of PM domains.
>>>
>>> This patch is based on the original patch by Marc Titinger.
>>>
>>> Cc: <devicetree@vger.kernel.org>
>>> Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>>> Acked-by: Rob Herring <robh@kernel.org>
>>> ---
>>> .../devicetree/bindings/power/power_domain.txt | 38
>>> ++++++++++++++++++++++
>>> 1 file changed, 38 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt
>>> b/Documentation/devicetree/bindings/power/power_domain.txt
>>> index 025b5e7..7f8f27e 100644
>>> --- a/Documentation/devicetree/bindings/power/power_domain.txt
>>> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
>>> @@ -29,6 +29,10 @@ Optional properties:
>>> specified by this binding. More details about power domain
>>> specifier are
>>> available in the next section.
>>>
>>> +- domain-idle-states : A phandle of an idle-state that shall be
>>> soaked into a
>>> + generic domain power state. The idle state
>>> definitions are
>>> + compatible with arm,idle-state specified in [1].
>>> +
>>
>> Please do add the following details to the binding. IMO, this binding is
>> not complete in terms of specification as there are few open questions:
>>
>> 1. What not define a standard compatible instead of "arm,idle-state" ?
>> I agree it can be used, but as part of this *generic* binding, IMO
>> it's better to have something generic and can be used by devices.
>> Otherwise, this binding becomes CPU specific, that too ARM CPU
>> specific.
>>
> We had gone down this path of having a separate DT bindings for domains
> that is not arm,idle-state. See RFC patches. But the binding did closely
> match this and it so was suggested that we use arm,idle-state which is
> already defined.
>
Are you referring to [1] here ? Sorry, I did some search quickly and
could find this, wanted to check if I am looking at the right one ?
--
Regards,
Sudeep
[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-June/353261.html
^ permalink raw reply
* [PATCH 1/5] PCI: aardvark: Name private struct pointer "advk" consistently
From: Bjorn Helgaas @ 2016-10-10 17:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161007220734.5f43e444@free-electrons.com>
Hi Thomas,
On Fri, Oct 07, 2016 at 10:07:34PM +0200, Thomas Petazzoni wrote:
> Hello,
>
> On Fri, 07 Oct 2016 11:20:53 -0500, Bjorn Helgaas wrote:
> > Use a device-specific name, "advk", for struct advk_pcie pointers to hint
> > that this is device-specific information. No functional change intended.
> >
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > ---
> > drivers/pci/host/pci-aardvark.c | 370 +++++++++++++++++++--------------------
> > 1 file changed, 183 insertions(+), 187 deletions(-)
>
> For the entire series:
>
> Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Sorry to iterate on this even more, but I renamed the local variables
from "advk" to "advk_pcie" since some of these are SoCs with drivers
other than just PCI, and I renamed "advk_readl" to "advk_pcie_readl"
similarly. I repushed the branch.
Bjorn
^ permalink raw reply
* [PATCH v2 5/8] dt/bindings: Update binding for PM domain idle states
From: Sudeep Holla @ 2016-10-10 17:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161010164342.GC44885@linaro.org>
On 10/10/16 17:43, Lina Iyer wrote:
> On Mon, Oct 10 2016 at 09:45 -0600, Sudeep Holla wrote:
>>
>>
>> On 07/10/16 23:36, Lina Iyer wrote:
>>> Update DT bindings to describe idle states of PM domains.
>>>
>>> This patch is based on the original patch by Marc Titinger.
>>>
>>> Cc: <devicetree@vger.kernel.org>
>>> Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>>> Acked-by: Rob Herring <robh@kernel.org>
>>> ---
>>> .../devicetree/bindings/power/power_domain.txt | 38
>>> ++++++++++++++++++++++
>>> 1 file changed, 38 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt
>>> b/Documentation/devicetree/bindings/power/power_domain.txt
>>> index 025b5e7..7f8f27e 100644
>>> --- a/Documentation/devicetree/bindings/power/power_domain.txt
>>> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
>>> @@ -29,6 +29,10 @@ Optional properties:
>>> specified by this binding. More details about power domain
>>> specifier are
>>> available in the next section.
>>>
>>> +- domain-idle-states : A phandle of an idle-state that shall be
>>> soaked into a
>>> + generic domain power state. The idle state
>>> definitions are
>>> + compatible with arm,idle-state specified in [1].
>>> +
>>
>> Please do add the following details to the binding. IMO, this binding is
>> not complete in terms of specification as there are few open questions:
>>
>> 1. What not define a standard compatible instead of "arm,idle-state" ?
>> I agree it can be used, but as part of this *generic* binding, IMO
>> it's better to have something generic and can be used by devices.
>> Otherwise, this binding becomes CPU specific, that too ARM CPU
>> specific.
>>
> We had gone down this path of having a separate DT bindings for domains
> that is not arm,idle-state. See RFC patches. But the binding did closely
> match this and it so was suggested that we use arm,idle-state which is
> already defined.
>
Either we say this binding is ARM CPU specific or generic, I can't
understand this mix 'n' match really. You have removed all the CPUIdle
stuff from this series which is good and makes it simpler, but linking
it to only "arm,idle-state" make be feel it's not generic. OK I will
have a look at the RFC as why generic compatible was rejected.
>> 2. Now taking CPU as a special device, how does this co-exist with the
>> cpu-idle-states ? Better to have some description may be in the ARM
>> CPU idle binding document(not here of-course)
>>
> The is a binding for a generic PM domain. This has no bearing on the CPU
> or its idle states. Its just that the data is compatible with
> arm,idle-state.
>
I understand that but it's not that simple which I assume you *do*
agree. Hence may need bit of an explanation in the binding(not here
of-course as I mentioned earlier, but in the CPU Idle bindings).
Please consider DT bindings as any other specification. All I am
asking is more description in the binding.
>> 3. I still haven't seen any explanation for not considering complete
>> hierarchical power domain representation which was raised in earlier
>> versions. I had provided example for the proposal. I just saw them
>> already in use in the upstream kernel by Renasas. e.g.:
>> arch/arm/boot/dts/r8a73a4.dtsi
>>
> Hierarchical power domains have been available for few years in DT. The
> OF features of domains have always supported it. Platforms are free to
> define domains in hierarchy they seem fit for their SoCs. This is a
> feature that is available today and is not being modified in these
> patches. It will be creating confusion if I talk about hierarchical
> domains which are obvious and irrelevant to this series.
>
Agreed and sorry if I created any confusion. But this binding doesn't
clearly state how to build up the hierarchy if the leaf node is not a
power-domain node and I am just trying have those clarifications in the
binding. It would be good if those details are *explicitly* mentioned in
the binding, not this particularly, but in CPU Idle one when you
introduce the user of that.
>> How does that fit with your proposal, though you have not made one
>> yet for CPUs in this binding ? In the above file, CPUs have either
>> own power domain inside the L2 one which is cluster level power
>> domain.
>>
> Again, this series is not about the CPUs. This is about adding features
> to genpd that may be used in other contexts including cpuidle in the
> future.
>
Yes I understand and hence I was confused as why I don't see an
*generic* compatible but just *arm,idle-states* in the example.
>> One must be able to get answers to these above questions with this
>> binding. Until then it's *incomplete* though it may be correct.
>>
> I have always tried to answer all your questions. If anything remains
> unclarified pls. bring it up.
>
It's not about questions, and definitely you have answered all my
questions, no argument there at all. Now we need to make those useful
discussions part of this binding so that it's *self explanatory* and
one need not refer back these discussions when writing DT for some
different SoC which differs from this. Again that could be part of
your CPUIdle series, I just raised it here as it got mixed sense from
this series. It was hard to be not to associate CPUIdle for reasons
mentioned above.
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH v3 3/6] pwm: imx: support output polarity inversion
From: Rob Herring @ 2016-10-10 17:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161007151129.6043-4-bhuvanchandra.dv@toradex.com>
On Fri, Oct 07, 2016 at 08:41:26PM +0530, Bhuvanchandra DV wrote:
> From: Lothar Wassmann <LW@KARO-electronics.de>
>
> The i.MX pwm unit on i.MX27 and newer SoCs provides a configurable output
> polarity. This patch adds support to utilize this feature where available.
>
> Signed-off-by: Lothar Wa?mann <LW@KARO-electronics.de>
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Bhuvanchandra DV <bhuvanchandra.dv@toradex.com>
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> Documentation/devicetree/bindings/pwm/imx-pwm.txt | 6 +--
Acked-by: Rob Herring <robh@kernel.org>
> drivers/pwm/pwm-imx.c | 51 +++++++++++++++++++++--
> 2 files changed, 51 insertions(+), 6 deletions(-)
^ permalink raw reply
* [PATCH 2/3] arm64: hw_breakpoint: Handle inexact watchpoint addresses
From: Pratyush Anand @ 2016-10-10 17:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJt8pk9WOhebSXpbahbbLscqJcY1OOqHiA_9hHC0zmHruRPxWQ@mail.gmail.com>
On Friday 07 October 2016 10:54 PM, Pavel Labath wrote:
> However, I did notice that it does not work if we watch an address
>> which is at some offset from address programmed. For example, it works
>> when byte_mask is 0x3, but it does not work if byte_mask if 0x2 (which
>> is supported by hardware).
>>
>> I do have some patches to resolve that.
>>
>> https://github.com/pratyushanand/linux/commits/perf/upstream_arm64_devel
>>
>> I will send them for review comment after some testing.
This branch has been updated with a test code as well. So far they work
fine at my end. Planning to send them tomorrow for review.
~Pratyush
^ permalink raw reply
* [PATCH v2 5/8] dt/bindings: Update binding for PM domain idle states
From: Lina Iyer @ 2016-10-10 16:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4e55d6ad-8de7-72d7-7512-5e4ff2aabc79@arm.com>
On Mon, Oct 10 2016 at 09:45 -0600, Sudeep Holla wrote:
>
>
>On 07/10/16 23:36, Lina Iyer wrote:
>>Update DT bindings to describe idle states of PM domains.
>>
>>This patch is based on the original patch by Marc Titinger.
>>
>>Cc: <devicetree@vger.kernel.org>
>>Signed-off-by: Marc Titinger <mtitinger+renesas@baylibre.com>
>>Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>>Acked-by: Rob Herring <robh@kernel.org>
>>---
>> .../devicetree/bindings/power/power_domain.txt | 38 ++++++++++++++++++++++
>> 1 file changed, 38 insertions(+)
>>
>>diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>>index 025b5e7..7f8f27e 100644
>>--- a/Documentation/devicetree/bindings/power/power_domain.txt
>>+++ b/Documentation/devicetree/bindings/power/power_domain.txt
>>@@ -29,6 +29,10 @@ Optional properties:
>> specified by this binding. More details about power domain specifier are
>> available in the next section.
>>
>>+- domain-idle-states : A phandle of an idle-state that shall be soaked into a
>>+ generic domain power state. The idle state definitions are
>>+ compatible with arm,idle-state specified in [1].
>>+
>
>Please do add the following details to the binding. IMO, this binding is
>not complete in terms of specification as there are few open questions:
>
>1. What not define a standard compatible instead of "arm,idle-state" ?
> I agree it can be used, but as part of this *generic* binding, IMO
> it's better to have something generic and can be used by devices.
> Otherwise, this binding becomes CPU specific, that too ARM CPU
> specific.
>
We had gone down this path of having a separate DT bindings for domains
that is not arm,idle-state. See RFC patches. But the binding did closely
match this and it so was suggested that we use arm,idle-state which is
already defined.
>2. Now taking CPU as a special device, how does this co-exist with the
> cpu-idle-states ? Better to have some description may be in the ARM
> CPU idle binding document(not here of-course)
>
The is a binding for a generic PM domain. This has no bearing on the CPU
or its idle states. Its just that the data is compatible with
arm,idle-state.
>3. I still haven't seen any explanation for not considering complete
> hierarchical power domain representation which was raised in earlier
> versions. I had provided example for the proposal. I just saw them
> already in use in the upstream kernel by Renasas. e.g.:
> arch/arm/boot/dts/r8a73a4.dtsi
>
Hierarchical power domains have been available for few years in DT. The
OF features of domains have always supported it. Platforms are free to
define domains in hierarchy they seem fit for their SoCs. This is a
feature that is available today and is not being modified in these
patches. It will be creating confusion if I talk about hierarchical
domains which are obvious and irrelevant to this series.
> How does that fit with your proposal, though you have not made one
> yet for CPUs in this binding ? In the above file, CPUs have either
> own power domain inside the L2 one which is cluster level power
> domain.
>
Again, this series is not about the CPUs. This is about adding features
to genpd that may be used in other contexts including cpuidle in the
future.
>One must be able to get answers to these above questions with this
>binding. Until then it's *incomplete* though it may be correct.
>
I have always tried to answer all your questions. If anything remains
unclarified pls. bring it up.
Thanks,
Lina
^ permalink raw reply
* [PATCH 2/3] doc: dt: add cyclone-spi binding document
From: atull @ 2016-10-10 16:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAtXAHeZLBT-y8xTKgG4PnZA6DH1z5=FoaM4ipC3pTEVU9KR+Q@mail.gmail.com>
On Fri, 7 Oct 2016, Moritz Fischer wrote:
> > +referred to as "passive serial".
> > +The passive serial link is not technically spi, and might require extra
> > +circuits in order to play nicely with other spi slaves on the same bus.
> > +
> > +See https://www.altera.com/literature/hb/cyc/cyc_c51013.pdf
> > +
> > +Required properties:
> > +- compatible : should contain "altr,cyclonespi-fpga-mgr"
>
> Alan, do you guys have any input on the compat string?
We want to make it clear that this is for fpga configuration
using a specific programming method (passive serial).
How about altr,cyclone-ps-spi ?
>
> I think generally the bindings should go before the actual usage in
> your patch series. Meaning you wanna document the binding
> before you use it. I think this patch should be [1/3].
I agree.
Thanks,
Alan
>
> Cheers,
>
> Moritz
>
^ permalink raw reply
* Restartable Sequences benchmarks (was: Re: [PATCH v2] arm: Added support for getcpu() vDSO using TPIDRURW)
From: Mathieu Desnoyers @ 2016-10-10 16:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161010152948.GD14561@arm.com>
----- On Oct 10, 2016, at 5:29 PM, Will Deacon will.deacon at arm.com wrote:
> Hi Fredrik,
>
> [adding Mathieu -- background is getcpu() in userspace for arm]
>
> On Thu, Oct 06, 2016 at 12:17:07AM +0200, Fredrik Markstr?m wrote:
>> On Wed, Oct 5, 2016 at 9:53 PM, Russell King - ARM Linux <linux@armlinux.org.uk
>> > wrote:
>> > On Wed, Oct 05, 2016 at 06:48:05PM +0100, Robin Murphy wrote:
>> >> On 05/10/16 17:39, Fredrik Markstr?m wrote:
>> >> > The approach I suggested below with the vDSO data page will obviously
>> >> > not work on smp, so suggestions are welcome.
>> >>
>> >> Well, given that it's user-writeable, is there any reason an application
>> >> which cares couldn't simply run some per-cpu threads to call getcpu()
>> >> once and cache the result in TPIDRURW themselves? That would appear to
>> >> both raise no compatibility issues and work with existing kernels.
>> >
>> > There is - the contents of TPIDRURW is thread specific, and it moves
>> > with the thread between CPU cores. So, if a thread was running on CPU0
>> > when it cached the getcpu() value in TPIDRURW, and then migrated to CPU1,
>> > TPIDRURW would still contain 0.
>> >
>> > I'm also not in favour of changing the TPIDRURW usage to be a storage
>> > repository for the CPU number - it's far too specific a usage and seems
>> > like a waste of hardware resources to solve one problem.
>>
>> Ok, but right now it's nothing but a (architecture specific) piece of TLS,
>> which we have generic mechanisms for. From my point of view that is a waste of
>> hardware resources.
>>
>> > As Mark says, it's an ABI breaking change too, even if it is under a config
>> option.
>>
>> I can't argue with that. If it's an ABI it's an ABI, even if I can't imagine
>> why anyone would use it over normal tls... but then again, people probably do.
>>
>> So in conclusion I agree and give up.
>
> Rather than give up, you could take a look at the patches from Mathieu
> Desnoyers, that tackle this in a very different way. It's also the reason
> we've been holding off implementing an optimised getcpu in the arm64 vdso,
> because it might all well be replaced by the new restartable sequences
> approach:
>
> http://lkml.kernel.org/r/1471637274-13583-1-git-send-email-mathieu.desnoyers at efficios.com
>
> He's also got support for arch/arm/ in that series, so you could take
> them for a spin. The main thing missing at the moment is justification
> for the feature using real-world code, as requested by Linus:
>
> http://lkml.kernel.org/r/CA+55aFz+Q33m1+ju3ANaznBwYCcWo9D9WDr2=p0YLEF4gJF12g at mail.gmail.com
>
> so if your per-cpu buffer use-case is compelling in its own right (as
> opposed to a micro-benchmark), then you could chime in over there.
>
> Will
FYI, I've adapted lttng-ust ring buffer (as a POC) to rseq in a dev
branch. I see interesting speedups. See top 3-4 commits of
https://github.com/compudj/lttng-ust-dev/tree/rseq-integration
(start with "Use rseq for...").
On x86-64, we have a 7ns speedup over sched_getcpu on x86-64, and
30ns speedup by using rseq atomics on x86-64, which brings the cost
per event record down to about 100ns/event. This replaces 3 atomic
operations on the fast path. (37% speedup)
On arm32, the cpu_id acceleration gives a 327 ns/event speed increase,
which brings speed to 2000ns/event. Note that reading time on that
system does not use the vDSO (old glibc), so it implies a system call.
This accounts for 857ns/events. I don't observe speed increase nor
slowdown by using rseq instead of ll/sc atomic operations on that
specific board (Cubietruck, only has 2 cores). I suspect that boards
with more core will benefit more of replacing ll/sc by rseq atomics.
If we don't account the overhead of reading time through system call,
we get a 22% speedup.
I have extra benchmarks in this branch:
https://github.com/compudj/rseq-test
Updated ref for current rseq-enabled kernel based on 4.8:
https://github.com/compudj/linux-percpu-dev/tree/rseq-fallback
(ARM64 port would be welcome!) :)
As Will pointed out, what I'm currently looking for is real-life
benchmarks that shows benefits of rseq. I fear that the microbenchmarks
I have for the lttng-ust tracer may be dismissed as being too specific.
Most heavy users of LTTng-UST are closed source applications, so it's
not easy for me to provide numbers in real-life scenarios.
The major use-case besides per-cpu buffering/tracing AFAIU is memory
allocators. It will mainly benefit in use-cases where there are more
threads than cores in a multithreaded application. This mainly makes
sense if threads are either dedicated to specific tasks, and therefore
are often idle, or in use-cases where worker threads are expected to
block (else, if threads are not expected to block, the application
should simply have one thread per core).
Dave Watson had interesting RSS shrinkage on this stress-test program:
http://locklessinc.com/downloads/t-test1.c modified to have 500 threads.
It uses jemalloc modified to use rseq.
I reproduced it on my laptop with 100 threads, 50000 loops:
4-core, 100 threads, 50000 loops.
malloc:
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
10136 compudj 20 0 2857840 24756 1348 R 379.4 0.4 3:49.50 t-test1
real 3m20.830s
user 3m22.164s
sys 9m40.936s
upstream jemalloc:
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
21234 compudj 20 0 2227124 49300 2280 S 306.2 0.7 2:26.97 t-test1
real 1m3.297s
user 3m19.616s
sys 0m8.500s
rseq jemalloc 4.2.1:
PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
25652 compudj 20 0 877956 35624 3260 S 301.2 0.5 1:26.07 t-test1
real 0m27.639s
user 1m18.172s
sys 0m1.752s
The next step to translate this into a "real-life" number would be to
run rseq-jemalloc on a facebook node, but Dave has been in vacation for
the past few weeks. Perhaps someone else at Facebook or Google could
look into this ?
Cheers,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* [PATCH RESEND] ARM: dts: keystone-k2*: Increase SPI Flash partition size for U-Boot
From: Santosh Shilimkar @ 2016-10-10 16:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161010143144.GB1041@n2100.armlinux.org.uk>
Vignesh,
On 10/10/2016 7:31 AM, Russell King - ARM Linux wrote:
> On Mon, Oct 10, 2016 at 07:41:41PM +0530, Vignesh R wrote:
>> U-Boot SPI Boot image is now more than 512KB for Keystone2 devices and
>> cannot fit into existing partition. So, increase the SPI Flash partition
>> for U-Boot to 1MB for all Keystone2 devices.
>>
>> Signed-off-by: Vignesh R <vigneshr@ti.com>
>> ---
>>
>> This was submitted to v4.9 merge window but was never picked up:
>> https://patchwork.kernel.org/patch/9135023/
Another point is, if you want me to pick your patch, please copy
me next time :-). AFAIK, am seeing this patch in my inbox first time.
>
> I think you need to explain why it's safe to change the layout of the
> flash partitions like this.
>
> - What is this "misc" partition?
>
> - Why is it safe to move the "misc" partition in this way?
>
> - Do users need to do anything with data stored in the "misc" partition
> when changing kernels?
>
> If the "misc" partition is simply unused space on the flash device, why
> list it in DT?
>
Thanks Russell. Yes, above clarification would be good to get first.
^ permalink raw reply
* [PATCH] kernel: irq: fix build failure
From: Sudip Mukherjee @ 2016-10-10 15:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475775403-27207-1-git-send-email-sudipm.mukherjee@gmail.com>
On Thursday 06 October 2016 11:06 PM, Sudip Mukherjee wrote:
> The allmodconfig build of powerpc is failing with the error:
> ERROR: ".irq_set_parent" [drivers/mfd/tps65217.ko] undefined!
>
> export the symbol to fix the failure.
Hi Thomas,
powerpc and arm allmodconfig builds still fails with the same error.
Build logs of next-20161010 are at:
arm at https://travis-ci.org/sudipm-mukherjee/parport/jobs/166321467
powerpc at https://travis-ci.org/sudipm-mukherjee/parport/jobs/166321473
Regards
Sudip
>
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> ---
>
> build log is at:
> https://travis-ci.org/sudipm-mukherjee/parport/jobs/165418652
>
> kernel/irq/manage.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 0c5f1a5..5072814 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -721,6 +721,7 @@ int irq_set_parent(int irq, int parent_irq)
> irq_put_desc_unlock(desc, flags);
> return 0;
> }
> +EXPORT_SYMBOL(irq_set_parent);
> #endif
>
> /*
>
^ permalink raw reply
* [PATCH v13 03/15] iommu/dma: Allow MSI-only cookies
From: Robin Murphy @ 2016-10-10 15:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b6771e0c-6f2d-21c1-05ac-8258b2bec462@redhat.com>
On 10/10/16 15:47, Auger Eric wrote:
> Hi Robin,
>
> On 10/10/2016 16:26, Robin Murphy wrote:
>> Hi Alex, Eric,
>>
>> On 06/10/16 21:17, Alex Williamson wrote:
>>> On Thu, 6 Oct 2016 08:45:19 +0000
>>> Eric Auger <eric.auger@redhat.com> wrote:
>>>
>>>> From: Robin Murphy <robin.murphy@arm.com>
>>>>
>>>> IOMMU domain users such as VFIO face a similar problem to DMA API ops
>>>> with regard to mapping MSI messages in systems where the MSI write is
>>>> subject to IOMMU translation. With the relevant infrastructure now in
>>>> place for managed DMA domains, it's actually really simple for other
>>>> users to piggyback off that and reap the benefits without giving up
>>>> their own IOVA management, and without having to reinvent their own
>>>> wheel in the MSI layer.
>>>>
>>>> Allow such users to opt into automatic MSI remapping by dedicating a
>>>> region of their IOVA space to a managed cookie.
>>>>
>>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>
>>>> ---
>>>>
>>>> v1 -> v2:
>>>> - compared to Robin's version
>>>> - add NULL last param to iommu_dma_init_domain
>>>> - set the msi_geometry aperture
>>>> - I removed
>>>> if (base < U64_MAX - size)
>>>> reserve_iova(iovad, iova_pfn(iovad, base + size), ULONG_MAX);
>>>> don't get why we would reserve something out of the scope of the iova domain?
>>>> what do I miss?
>>>> ---
>>>> drivers/iommu/dma-iommu.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>>> include/linux/dma-iommu.h | 9 +++++++++
>>>> 2 files changed, 49 insertions(+)
>>>>
>>>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>>> index c5ab866..11da1a0 100644
>>>> --- a/drivers/iommu/dma-iommu.c
>>>> +++ b/drivers/iommu/dma-iommu.c
>>>> @@ -716,3 +716,43 @@ void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
>>>> msg->address_lo += lower_32_bits(msi_page->iova);
>>>> }
>>>> }
>>>> +
>>>> +/**
>>>> + * iommu_get_dma_msi_region_cookie - Configure a domain for MSI remapping only
>>>
>>> Should this perhaps be iommu_setup_dma_msi_region_cookie, or something
>>> along those lines. I'm not sure what we're get'ing. Thanks,
>>
>> What we're getting is private third-party resources for the iommu_domain
>> given in the argument. It's a get/put rather than alloc/free model since
>> we operate opaquely on the domain as a container, rather than on the
>> actual resource in question (an IOVA allocator).
>>
>> Since this particular use case is slightly different from the normal
>> flow and has special initialisation requirements, it seemed a lot
>> cleaner to simply combine that initialisation operation with the
>> prerequisite "get" into a single call. Especially as it helps emphasise
>> that this is not 'normal' DMA cookie usage.
>
> I renamed iommu_get_dma_msi_region_cookie into
> iommu_setup_dma_msi_region. Is it a problem for you?
I'd still prefer not to completely disguise the fact that it's
performing a get_cookie(), which ultimately still needs to be matched by
a put_cookie() somewhere. Really, VFIO should be doing the latter itself
before freeing the domain, as there's not strictly any guarantee that
the underlying IOMMU driver knows anything about this.
Robin.
>>
>>>
>>> Alex
>>>
>>>> + * @domain: IOMMU domain to prepare
>>>> + * @base: Base address of IOVA region to use as the MSI remapping aperture
>>>> + * @size: Size of the desired MSI aperture
>>>> + *
>>>> + * Users who manage their own IOVA allocation and do not want DMA API support,
>>>> + * but would still like to take advantage of automatic MSI remapping, can use
>>>> + * this to initialise their own domain appropriately.
>>>> + */
>>>> +int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
>>>> + dma_addr_t base, u64 size)
>>>> +{
>>>> + struct iommu_dma_cookie *cookie;
>>>> + struct iova_domain *iovad;
>>>> + int ret;
>>>> +
>>>> + if (domain->type == IOMMU_DOMAIN_DMA)
>>>> + return -EINVAL;
>>>> +
>>>> + ret = iommu_get_dma_cookie(domain);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + ret = iommu_dma_init_domain(domain, base, size, NULL);
>>>> + if (ret) {
>>>> + iommu_put_dma_cookie(domain);
>>>> + return ret;
>>>> + }
>>
>> It *is* necessary to explicitly reserve the upper part of the IOVA
>> domain here - the aforementioned "special initialisation" - because
>> dma_32bit_pfn is only an optimisation hint to prevent the allocator
>> walking down from the very top of the the tree every time when devices
>> with different DMA masks share a domain (I'm in two minds as to whether
>> to tweak the way the iommu-dma code uses it in this respect, now that I
>> fully understand things). The only actual upper limit to allocation is
>> the DMA mask passed into each alloc_iova() call, so if we want to ensure
>> IOVAs are really allocated within this specific region, we have to carve
>> out everything above it.
>
> thank you for the explanation. So I will restore the reserve then.
>
> Thanks
>
> Eric
>>
>> Robin.
>>
>>>> +
>>>> + domain->msi_geometry.aperture_start = base;
>>>> + domain->msi_geometry.aperture_end = base + size - 1;
>>>> +
>>>> + cookie = domain->iova_cookie;
>>>> + iovad = &cookie->iovad;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +EXPORT_SYMBOL(iommu_get_dma_msi_region_cookie);
>>>> diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
>>>> index 32c5890..1c55413 100644
>>>> --- a/include/linux/dma-iommu.h
>>>> +++ b/include/linux/dma-iommu.h
>>>> @@ -67,6 +67,9 @@ int iommu_dma_mapping_error(struct device *dev, dma_addr_t dma_addr);
>>>> /* The DMA API isn't _quite_ the whole story, though... */
>>>> void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
>>>>
>>>> +int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
>>>> + dma_addr_t base, u64 size);
>>>> +
>>>> #else
>>>>
>>>> struct iommu_domain;
>>>> @@ -90,6 +93,12 @@ static inline void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
>>>> {
>>>> }
>>>>
>>>> +static inline int iommu_get_dma_msi_region_cookie(struct iommu_domain *domain,
>>>> + dma_addr_t base, u64 size)
>>>> +{
>>>> + return -ENODEV;
>>>> +}
>>>> +
>>>> #endif /* CONFIG_IOMMU_DMA */
>>>> #endif /* __KERNEL__ */
>>>> #endif /* __DMA_IOMMU_H */
>>>
>>
>
^ 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