* at_hdmac does not release lock before the callback - deadlock
From: Jouko Haapaluoma @ 2014-01-27 14:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <C9F8415089354E4FB76F4548B3515AF484286F61@edb1.wapice.localdomain>
After further investigation it seems that the deadlocks would happen only when CONFIG_SMP is enabled or RT_PREEMPT patch is used. When CONFIG_SMP or RT_PREEMPT is not used, the spinlocks will only disable preemption which does not cause a deadlock. However, with the CONFIG_SMP or the RT_PREEMPT patch spinlocks are actual locks and deadlocks can occur.
This might explain why this problem hasn't been noticed before. At least the Atmel serial driver from linux4sam git causes deadlocks with the RT_PREEMPT because of this issue.
BR,
Jouko Haapaluoma
^ permalink raw reply
* [PATCH 0/9] ARM: dts: imx: remove the use of pingrp macros
From: Rob Herring @ 2014-01-27 14:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390668191-20289-1-git-send-email-shawn.guo@linaro.org>
On Sat, Jan 25, 2014 at 10:43 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> Hi Rob,
>
> In order to solve a pinctrl data efficiency problem, we introduced
> pingrp macros [1] in this development cycle as the base of board dts
> support. The whole imx-dt-3.14 pull request has been held by Olof for
> a few weeks because he wants to get a general approval from DT folks
> on this change. But unfortunately it appears that you are not fond of
> this change.
>
> I just spent the day to create a patch series against imx-dt-3.14 to
> remove these pingrp macros. May I get your nod on this quick
> turn-around, so that we do not miss the merge window?
I've only skimmed thru some of the patches, but in general it looks fine to me:
Acked-by: Rob Herring <robh@kernel.org>
Rob
> Hi Olof,
>
> I guess we do not have to shut the door for imx-dt-3.14 if you and DT
> folks are happy with this patch series, which is a quite straight
> forward search&replace change?
>
> Shawn
>
> [1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/275912/
>
> Shawn Guo (9):
> ARM: dts: imx6qdl: remove the use of pingrp macros
> ARM: dts: imx6sl: remove the use of pingrp macros
> ARM: dts: imx53: remove the use of pingrp macros
> ARM: dts: imx51: remove the use of pingrp macros
> ARM: dts: imx50: remove the use of pingrp macros
> ARM: dts: imx35: remove the use of pingrp macros
> ARM: dts: imx25: remove the use of pingrp macros
> ARM: dts: imx27: remove the use of pingrp macros
> ARM: dts: vf610: remove the use of pingrp macros
>
> arch/arm/boot/dts/imx25-eukrea-cpuimx25.dtsi | 17 +-
> .../boot/dts/imx25-eukrea-mbimxsd25-baseboard.dts | 56 ++-
> arch/arm/boot/dts/imx25-pingrp.h | 81 ---
> arch/arm/boot/dts/imx25.dtsi | 2 +-
> arch/arm/boot/dts/imx27-apf27.dts | 26 +-
> arch/arm/boot/dts/imx27-apf27dev.dts | 65 ++-
> arch/arm/boot/dts/imx27-phytec-phycard-s-rdk.dts | 27 +-
> arch/arm/boot/dts/imx27-phytec-phycard-s-som.dts | 26 +-
> arch/arm/boot/dts/imx27-phytec-phycore-rdk.dts | 23 +-
> arch/arm/boot/dts/imx27-phytec-phycore-som.dtsi | 38 +-
> arch/arm/boot/dts/imx27-pingrp.h | 151 ------
> arch/arm/boot/dts/imx27.dtsi | 2 +-
> arch/arm/boot/dts/imx35-pingrp.h | 104 ----
> arch/arm/boot/dts/imx35.dtsi | 1 -
> arch/arm/boot/dts/imx50-evk.dts | 28 +-
> arch/arm/boot/dts/imx50-pingrp.h | 146 ------
> arch/arm/boot/dts/imx50.dtsi | 2 +-
> arch/arm/boot/dts/imx51-apf51.dts | 26 +-
> arch/arm/boot/dts/imx51-apf51dev.dts | 64 ++-
> arch/arm/boot/dts/imx51-babbage.dts | 142 +++++-
> arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi | 26 +-
> .../boot/dts/imx51-eukrea-mbimxsd51-baseboard.dts | 31 +-
> arch/arm/boot/dts/imx51-pingrp.h | 249 ---------
> arch/arm/boot/dts/imx51.dtsi | 2 +-
> arch/arm/boot/dts/imx53-ard.dts | 18 +-
> arch/arm/boot/dts/imx53-evk.dts | 51 +-
> arch/arm/boot/dts/imx53-m53evk.dts | 128 ++++-
> arch/arm/boot/dts/imx53-pingrp.h | 352 -------------
> arch/arm/boot/dts/imx53-qsb.dts | 88 +++-
> arch/arm/boot/dts/imx53-smd.dts | 77 ++-
> arch/arm/boot/dts/imx53-tqma53.dtsi | 89 +++-
> arch/arm/boot/dts/imx53-voipac-bsb.dts | 21 +-
> arch/arm/boot/dts/imx53-voipac-dmm-668.dtsi | 47 +-
> arch/arm/boot/dts/imx53.dtsi | 2 +-
> arch/arm/boot/dts/imx6dl-hummingboard.dts | 5 +-
> arch/arm/boot/dts/imx6dl.dtsi | 1 -
> arch/arm/boot/dts/imx6q-arm2.dts | 81 ++-
> arch/arm/boot/dts/imx6q-cm-fx6.dts | 44 +-
> arch/arm/boot/dts/imx6q-dmo-edmqmx6.dts | 47 +-
> arch/arm/boot/dts/imx6q-gw5400-a.dts | 75 ++-
> arch/arm/boot/dts/imx6q-phytec-pfla02.dtsi | 53 +-
> arch/arm/boot/dts/imx6q-sbc6x.dts | 37 +-
> arch/arm/boot/dts/imx6q-udoo.dts | 33 +-
> arch/arm/boot/dts/imx6q.dtsi | 1 -
> arch/arm/boot/dts/imx6qdl-gw51xx.dtsi | 77 ++-
> arch/arm/boot/dts/imx6qdl-gw52xx.dtsi | 88 +++-
> arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 93 +++-
> arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 93 +++-
> arch/arm/boot/dts/imx6qdl-microsom.dtsi | 5 +-
> arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi | 56 ++-
> arch/arm/boot/dts/imx6qdl-pingrp.h | 532 --------------------
> arch/arm/boot/dts/imx6qdl-sabreauto.dtsi | 139 ++++-
> arch/arm/boot/dts/imx6qdl-sabrelite.dtsi | 56 ++-
> arch/arm/boot/dts/imx6qdl-sabresd.dtsi | 86 +++-
> arch/arm/boot/dts/imx6qdl-wandboard.dtsi | 78 ++-
> arch/arm/boot/dts/imx6sl-evk.dts | 120 ++++-
> arch/arm/boot/dts/imx6sl-pingrp.h | 148 ------
> arch/arm/boot/dts/imx6sl.dtsi | 1 -
> arch/arm/boot/dts/vf610-cosmic.dts | 17 +-
> arch/arm/boot/dts/vf610-pingrp.h | 127 -----
> arch/arm/boot/dts/vf610-twr.dts | 42 +-
> arch/arm/boot/dts/vf610.dtsi | 2 +-
> 62 files changed, 2163 insertions(+), 2182 deletions(-)
> delete mode 100644 arch/arm/boot/dts/imx25-pingrp.h
> delete mode 100644 arch/arm/boot/dts/imx27-pingrp.h
> delete mode 100644 arch/arm/boot/dts/imx35-pingrp.h
> delete mode 100644 arch/arm/boot/dts/imx50-pingrp.h
> delete mode 100644 arch/arm/boot/dts/imx51-pingrp.h
> delete mode 100644 arch/arm/boot/dts/imx53-pingrp.h
> delete mode 100644 arch/arm/boot/dts/imx6qdl-pingrp.h
> delete mode 100644 arch/arm/boot/dts/imx6sl-pingrp.h
> delete mode 100644 arch/arm/boot/dts/vf610-pingrp.h
>
> --
> 1.7.9.5
>
>
^ permalink raw reply
* [PATCH V6 1/2] PHY: Exynos: Add Exynos5250 SATA PHY driver
From: Yuvaraj Kumar @ 2014-01-27 13:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52D00C33.4060305@samsung.com>
On Fri, Jan 10, 2014 at 8:35 PM, Tomasz Figa <t.figa@samsung.com> wrote:
> Hi Yuvaraj,
>
> In general this version looks pretty good, but I have some questions inline.
>
> On 10.01.2014 08:00, Yuvaraj Kumar C D wrote:
> [snip]
>
>> diff --git a/drivers/phy/phy-exynos5250-sata-i2c.c
>> b/drivers/phy/phy-exynos5250-sata-i2c.c
>> new file mode 100644
>> index 0000000..206e337
>> --- /dev/null
>> +++ b/drivers/phy/phy-exynos5250-sata-i2c.c
>> @@ -0,0 +1,40 @@
>> +/*
>> + * Copyright (C) 2013 Samsung Electronics Co.Ltd
>> + * Author:
>> + * Yuvaraj C D <yuvaraj.cd@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> it
>> + * under the terms of the GNU General Public License as published by
>> the
>> + * Free Software Foundation; either version 2 of the License, or (at
>> your
>> + * option) any later version.
>> + *
>> + */
>> +
>> +#include <linux/i2c.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +
>> +static int exynos_sata_i2c_probe(struct i2c_client *client,
>> + const struct i2c_device_id *i2c_id)
>> +{
>> + return 0;
>> +}
>> +
>> +static const struct i2c_device_id sataphy_i2c_device_match[] = {
>> + { "exynos-sataphy-i2c", 0 },
>> +};
>> +
>> +static struct i2c_driver sataphy_i2c_driver = {
>> + .probe = exynos_sata_i2c_probe,
>> + .id_table = sataphy_i2c_device_match,
>> + .driver = {
>> + .name = "exynos-sataphy-i2c",
>> + .owner = THIS_MODULE,
>> + },
>> +};
>> +
>> +static int __init exynos5250_phy_i2c_init(void)
>> +{
>> + return i2c_add_driver(&sataphy_i2c_driver);
>> +}
>> +module_init(exynos5250_phy_i2c_init);
>
>
> Hmm, is this driver even necessary now?
>
> Wolfram, would it be possible to use an i2c_client without a driver bound to
> it?
>
>
>> diff --git a/drivers/phy/phy-exynos5250-sata.c
>> b/drivers/phy/phy-exynos5250-sata.c
>> new file mode 100644
>> index 0000000..6e5ff8d
>> --- /dev/null
>> +++ b/drivers/phy/phy-exynos5250-sata.c
>> @@ -0,0 +1,238 @@
>> +/*
>> + * Samsung SATA SerDes(PHY) driver
>> + *
>> + * Copyright (C) 2013 Samsung Electronics Co., Ltd.
>> + * Authors: Girish K S <ks.giri@samsung.com>
>> + * Yuvaraj Kumar C D <yuvaraj.cd@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.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/io.h>
>> +#include <linux/i2c.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/mfd/syscon.h>
>> +
>> +#define EXYNOS5_SATA_RESET 0x4
>> +#define RESET_CMN_RST_N (1 << 1)
>> +#define LINK_RESET 0xF0000
>
>
> nit: Lowercase is preferred in hexadecimal notation.
> + all other occurrences in this file.
>
>
>> +#define EXYNOS5_SATA_MODE0 0x10
>> +#define EXYNOS5_SATAPHY_PMU_ENABLE (1 << 0)
>> +#define SATA_SPD_GEN3 (2 << 0)
>> +#define EXYNOS5_SATA_CTRL0 0x14
>> +#define CTRL0_P0_PHY_CALIBRATED_SEL (1 << 9)
>> +#define CTRL0_P0_PHY_CALIBRATED (1 << 8)
>> +#define EXYNOS5_SATA_PHSATA_CTRLM 0xE0
>> +#define PHCTRLM_REF_RATE (1 << 1)
>> +#define PHCTRLM_HIGH_SPEED (1 << 0)
>> +#define EXYNOS5_SATA_PHSATA_STATM 0xF0
>> +#define PHSTATM_PLL_LOCKED (1 << 0)
>> +#define EXYNOS_SATA_PHY_EN (1 << 0)
>> +#define SATAPHY_CONTROL_OFFSET 0x0724
>> +
>> +struct exynos_sata_phy {
>> + struct phy *phy;
>> + struct clk *phyclk;
>> + void __iomem *regs;
>> + void __iomem *pmureg;
>> + struct i2c_client *client;
>> +};
>> +
>> +static bool wait_for_reg_status(void __iomem *base, u32 reg, u32
>> checkbit,
>> + u32 status)
>> +{
>> + unsigned long timeout = jiffies + usecs_to_jiffies(1000);
>
>
> nit: It would be better to define the timeout using a macro to not use magic
> numbers.
>
>
>> +
>> + while (time_before(jiffies, timeout)) {
>> + if ((readl(base + reg) & checkbit) == status)
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static int exynos_sata_phy_power_on(struct phy *phy)
>> +{
>> + struct exynos_sata_phy *sata_phy = phy_get_drvdata(phy);
>> +
>> + regmap_update_bits(sata_phy->pmureg, SATAPHY_CONTROL_OFFSET,
>> + EXYNOS5_SATAPHY_PMU_ENABLE, EXYNOS_SATA_PHY_EN);
>
>
> regmap_update_bits can return an error. Wouldn't it be better to return it
> as return value of this function instead of returning 0 all the time? As a
> side effect, this would make the function smaller by two lines.
>
>
>> +
>> + return 0;
>> +}
>> +
>> +static int exynos_sata_phy_power_off(struct phy *phy)
>> +{
>> + struct exynos_sata_phy *sata_phy = phy_get_drvdata(phy);
>> +
>> + regmap_update_bits(sata_phy->pmureg, SATAPHY_CONTROL_OFFSET,
>> + EXYNOS5_SATAPHY_PMU_ENABLE, ~EXYNOS_SATA_PHY_EN);
>
>
> Same here.
>
>
>> +
>> + return 0;
>> +}
>> +
>> +static int exynos_sata_phy_init(struct phy *phy)
>> +{
>> + u32 val = 0;
>> + int ret = 0;
>> + u8 buf[] = { 0x3A, 0x0B };
>> + struct exynos_sata_phy *sata_phy = phy_get_drvdata(phy);
>> +
>> + regmap_update_bits(sata_phy->pmureg, SATAPHY_CONTROL_OFFSET,
>> + EXYNOS5_SATAPHY_PMU_ENABLE, EXYNOS_SATA_PHY_EN);
>
>
> regmap_update_bits returns an error code.
>
>
>> +
>> + writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
>> +
>> + val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
>> + val |= 0xFF;
>> + writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
>> +
>> + val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
>> + val |= LINK_RESET;
>> + writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
>> +
>> + val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
>> + val |= RESET_CMN_RST_N;
>> + writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
>> +
>> + val = readl(sata_phy->regs + EXYNOS5_SATA_PHSATA_CTRLM);
>> + val &= ~PHCTRLM_REF_RATE;
>> + writel(val, sata_phy->regs + EXYNOS5_SATA_PHSATA_CTRLM);
>> +
>> + /* High speed enable for Gen3 */
>> + val = readl(sata_phy->regs + EXYNOS5_SATA_PHSATA_CTRLM);
>> + val |= PHCTRLM_HIGH_SPEED;
>> + writel(val, sata_phy->regs + EXYNOS5_SATA_PHSATA_CTRLM);
>> +
>> + val = readl(sata_phy->regs + EXYNOS5_SATA_CTRL0);
>> + val |= CTRL0_P0_PHY_CALIBRATED_SEL | CTRL0_P0_PHY_CALIBRATED;
>> + writel(val, sata_phy->regs + EXYNOS5_SATA_CTRL0);
>> +
>> + val = readl(sata_phy->regs + EXYNOS5_SATA_MODE0);
>> + val |= SATA_SPD_GEN3;
>> + writel(val, sata_phy->regs + EXYNOS5_SATA_MODE0);
>> +
>> + ret = i2c_master_send(sata_phy->client, buf, sizeof(buf));
>> + if (ret < 0)
>> + return -ENXIO;
>
>
> Wouldn't it be better to return the same error code as i2c_master_send
> returned?
>
>
>> +
>> + /* release cmu reset */
>> + val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
>> + val &= ~RESET_CMN_RST_N;
>> + writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
>> +
>> + val = readl(sata_phy->regs + EXYNOS5_SATA_RESET);
>> + val |= RESET_CMN_RST_N;
>> + writel(val, sata_phy->regs + EXYNOS5_SATA_RESET);
>> +
>> + return (wait_for_reg_status(sata_phy->regs,
>> EXYNOS5_SATA_PHSATA_STATM,
>> + PHSTATM_PLL_LOCKED, 1)) ? 0 : -EINVAL;
>> +
>
>
> nit: Stray blank line.
>
> Also it might be more readable after making wait_for_reg_status() return an
> integer error code (0 and e.g. -EFAULT) and rewriting the last line to:
>
> ret = wait_for_reg_status(sata_phy->regs,
> EXYNOS5_SATA_PHSATA_STATM,
> PHSTATM_PLL_LOCKED, 1);
> if (ret < 0)
> dev_err(&sata_phy->client->dev,
> "PHY PLL locking failed\n");
>
> return ret;
>
> By the way, isn't this initialization really needed whenever the PHY is
> powered on?
>
>
>> +}
>> +
>> +static struct phy_ops exynos_sata_phy_ops = {
>> + .init = exynos_sata_phy_init,
>> + .power_on = exynos_sata_phy_power_on,
>> + .power_off = exynos_sata_phy_power_off,
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static int exynos_sata_phy_probe(struct platform_device *pdev)
>> +{
>> + struct exynos_sata_phy *sata_phy;
>> + struct device *dev = &pdev->dev;
>> + struct resource *res;
>> + struct phy_provider *phy_provider;
>> + struct device_node *node;
>> + int ret = 0;
>> +
>> + sata_phy = devm_kzalloc(dev, sizeof(*sata_phy), GFP_KERNEL);
>> + if (!sata_phy)
>> + return -ENOMEM;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +
>> + sata_phy->regs = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(sata_phy->regs))
>> + return PTR_ERR(sata_phy->regs);
>> +
>> + sata_phy->pmureg = syscon_regmap_lookup_by_phandle(dev->of_node,
>> + "samsung,syscon-phandle");
>
>
> pmureg is defined as (void __iomem *) in struct exynos_sata_phy, but
> syscon_regmap_lookup_by_phandle() returns (struct regmap *). Moreover it
> does not return NULL on error, but rather ERR_PTR(). Please correct this.
>
>
>> + if (!sata_phy->pmureg) {
>> + dev_err(dev, "syscon regmap lookup failed.\n");
>> + return PTR_ERR(sata_phy->pmureg);
>> + }
>> +
>> + node = of_parse_phandle(dev->of_node,
>> + "samsung,exynos-sataphy-i2c-phandle", 0);
>> + if (!node)
>> + return -ENODEV;
>
>
> An error here means that a required DT property was not specified or was
> specified incorrectly. IMHO -EINVAL would be better here.
>
>
>> +
>> + sata_phy->client = of_find_i2c_device_by_node(node);
>> + if (!sata_phy->client)
>> + return -EPROBE_DEFER;
>> +
>> + dev_set_drvdata(dev, sata_phy);
>> +
>> + sata_phy->phyclk = devm_clk_get(dev, "sata_phyctrl");
>> + if (IS_ERR(sata_phy->phyclk)) {
>> + dev_err(dev, "failed to get clk for PHY\n");
>> + return PTR_ERR(sata_phy->phyclk);
>> + }
>> +
>> + ret = clk_prepare_enable(sata_phy->phyclk);
>> + if (ret < 0) {
>> + dev_err(dev, "failed to enable source clk\n");
>> + return ret;
>> + }
>> +
>> + sata_phy->phy = devm_phy_create(dev, &exynos_sata_phy_ops, NULL);
>> + if (IS_ERR(sata_phy->phy)) {
>> + clk_disable_unprepare(sata_phy->phyclk);
>> + dev_err(dev, "failed to create PHY\n");
>> + return PTR_ERR(sata_phy->phy);
>> + }
>> +
>> + phy_set_drvdata(sata_phy->phy, sata_phy);
>> +
>> + phy_provider = devm_of_phy_provider_register(dev,
>> + of_phy_simple_xlate);
>> + if (IS_ERR(phy_provider)) {
>> + clk_disable_unprepare(sata_phy->phyclk);
>> + return PTR_ERR(phy_provider);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id exynos_sata_phy_of_match[] = {
>> + { .compatible = "samsung,exynos5250-sata-phy" },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, exynos_sata_phy_of_match);
>> +
>> +static struct platform_driver exynos_sata_phy_driver = {
>> + .probe = exynos_sata_phy_probe,
>
>
> If this driver can be compiled as module, don't you also need remove?
I tried this and found as such there is no resources hold by this
driver to handle in remove case.
This driver is used by ahci_platform driver and when load
phy-exynos5250-sata,driver usage count is 1.
so if we unload ahci_platform driver first and then
phy-exynos5250-sata.ko it cleanely unload this module.
>
> Best regards,
> Tomasz
^ permalink raw reply
* [RFC/PATCH] ARM: dove: Remove UBI support from defconfig
From: Ezequiel Garcia @ 2014-01-27 13:26 UTC (permalink / raw)
To: linux-arm-kernel
As NAND support is not enabled by default, it's hard to see
why we'd want to have UBI support. Let's remove it.
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
arch/arm/configs/dove_defconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/configs/dove_defconfig b/arch/arm/configs/dove_defconfig
index 1101054..7242b11 100644
--- a/arch/arm/configs/dove_defconfig
+++ b/arch/arm/configs/dove_defconfig
@@ -48,7 +48,6 @@ CONFIG_MTD_CFI_INTELEXT=y
CONFIG_MTD_CFI_STAA=y
CONFIG_MTD_PHYSMAP=y
CONFIG_MTD_M25P80=y
-CONFIG_MTD_UBI=y
CONFIG_BLK_DEV_LOOP=y
CONFIG_BLK_DEV_RAM=y
CONFIG_BLK_DEV_RAM_COUNT=1
--
1.8.1.5
^ permalink raw reply related
* [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Srikanth Thokala @ 2014-01-27 13:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E54849.2000208@metafoo.de>
Hi Lars/Vinod,
On Sun, Jan 26, 2014 at 11:09 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 01/26/2014 02:59 PM, Vinod Koul wrote:
>> On Fri, Jan 24, 2014 at 02:24:27PM +0100, Lars-Peter Clausen wrote:
>>> On 01/24/2014 12:16 PM, Srikanth Thokala wrote:
>>>> Hi Lars,
>>>>
>>>> On Thu, Jan 23, 2014 at 4:55 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>>>>> On 01/22/2014 05:52 PM, Srikanth Thokala wrote:
>>>>> [...]
>>>>>> +/**
>>>>>> + * xilinx_vdma_device_control - Configure DMA channel of the device
>>>>>> + * @dchan: DMA Channel pointer
>>>>>> + * @cmd: DMA control command
>>>>>> + * @arg: Channel configuration
>>>>>> + *
>>>>>> + * Return: '0' on success and failure value on error
>>>>>> + */
>>>>>> +static int xilinx_vdma_device_control(struct dma_chan *dchan,
>>>>>> + enum dma_ctrl_cmd cmd, unsigned long arg)
>>>>>> +{
>>>>>> + struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan);
>>>>>> +
>>>>>> + switch (cmd) {
>>>>>> + case DMA_TERMINATE_ALL:
>>>>>> + xilinx_vdma_terminate_all(chan);
>>>>>> + return 0;
>>>>>> + case DMA_SLAVE_CONFIG:
>>>>>> + return xilinx_vdma_slave_config(chan,
>>>>>> + (struct xilinx_vdma_config *)arg);
>>>>>
>>>>> You really shouldn't be overloading the generic API with your own semantics.
>>>>> DMA_SLAVE_CONFIG should take a dma_slave_config and nothing else.
>>>>
>>>> Ok. The driver needs few additional configuration from the slave
>>>> device like Vertical
>>>> Size, Horizontal Size, Stride etc., for the DMA transfers, in that case do you
>>>> suggest me to define a separate dma_ctrl_cmd like the one FSLDMA_EXTERNAL_START
>>>> defined for Freescale drivers?
>>>
>>> In my opinion it is not a good idea to have driver implement a generic API,
>>> but at the same time let the driver have custom semantics for those API
>>> calls. It's a bit like having a gpio driver that expects 23 and 42 as the
>>> values passed to gpio_set_value instead of 0 and 1. It completely defeats
>>> the purpose of a generic API, namely that you are able to write generic code
>>> that makes use of the API without having to know about which implementation
>>> API it is talking to. The dmaengine framework provides the
>>> dmaengine_prep_interleaved_dma() function to setup two dimensional
>>> transfers, e.g. take a look at sirf-dma.c or imx-dma.c.
>>
>> The question here i think would be waht this device supports? Is the hardware
>> capable of doing interleaved transfers, then would make sense.
>
> The hardware does 2D transfers. The parameters for a transfer are height,
> width and stride. That's only a subset of what interleaved transfers can be
> (xt->num_frames must be one for 2d transfers). But if I remember correctly
> there has been some discussion on this in the past and the result of that
> discussion was that using interleaved transfers for 2D transfers is
> preferred over adding a custom API for 2D transfers.
I went through the prep_interleaved_dma API and I see only one descriptor
is prepared per API call (i.e. per frame). As our IP supports upto 16 frame
buffers (can be more in future), isn't it less efficient compared to the
prep_slave_sg where we get a single sg list and can prepare all the descriptors
(of non-contiguous buffers) in one go? Correct me, if am wrong and let me
know your opinions.
Srikanth
>
> - Lars
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* [PATCH RFC v2 1/2] Documentation: arm: add cache DT bindings
From: Russell King - ARM Linux @ 2014-01-27 12:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140121114845.GA2598@e103592.cambridge.arm.com>
On Tue, Jan 21, 2014 at 11:49:01AM +0000, Dave Martin wrote:
> I do have a worry that because the kernel won't normally use this
> information, by default it will get pasted between .dts files, won't get
> tested and will be wrong rather often. It also violates the DT principle
> that probeable information should not be present in the DT -- ePAPR
> obviously envisages systems where cache geometry information is not
> probeable, but that's not the case for architected caches on ARM, except
> in rare cases where the CLIDR is wrong.
That statement is wrong. There are caches on ARM CPUs where there is no
CLIDR register. I suggest reading the earlier DDI0100 revisions.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply
* at_hdmac does not release lock before the callback - deadlock
From: Jouko Haapaluoma @ 2014-01-27 12:58 UTC (permalink / raw)
To: linux-arm-kernel
Hello
It seems that the at_hdmac driver keeps the atchan->lock even when the device driver callback is called.
This will cause a deadlock in those cases when the device driver calls certain DMA Engine methods
from the callback function.
The DMA Engine documentation (https://www.kernel.org/doc/Documentation/dmaengine.txt) states:
"
Although the async_tx API specifies that completion callback
routines cannot submit any new operations, this is not the
case for slave/cyclic DMA.
For slave DMA, the subsequent transaction may not be available
for submission prior to callback function being invoked, so
slave DMA callbacks are permitted to prepare and submit a new
transaction.
For cyclic DMA, a callback function may wish to terminate the
DMA via dmaengine_terminate_all().
Therefore, it is important that DMA engine drivers drop any
locks before calling the callback function which may cause a
deadlock.
"
According to the documentation it seems that the at_hdmac driver violates the DMA Engine API because
locking is not released before entering the callback function.
One possible deadlock situation:
atc_tasklet()
spin_lock_irqsave(&atchan->lock)
atc_handle_cyclic()
callback()
dmaengine_terminate_all()
atc_control()
spin_lock_irqsave(&atchan->lock)
BR,
Jouko Haapaluoma
^ permalink raw reply
* [PATCH RFC v2 2/2] Documentation: arm: define DT C-states bindings
From: Antti P Miettinen @ 2014-01-27 12:48 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140127114125.GD16639@e102568-lin.cambridge.arm.com>
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> That's why I defined the worst case. How did you implemented it in your
> idle drivers ? That would help generalize it, after all these bindings
> are there to simplify drivers upstreaming, feedback welcome.
Currently we do not handle this well downstream either. The problem
with worst case is that the absolute worst case can be really bad and
probability of it might be very low. Sorry - no ready answer :-)
--Antti
^ permalink raw reply
* [PATCH 0/9] setting the table for integration of cpuidle with the scheduler
From: Peter Zijlstra @ 2014-01-27 12:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390802904-28399-1-git-send-email-nicolas.pitre@linaro.org>
On Mon, Jan 27, 2014 at 01:08:15AM -0500, Nicolas Pitre wrote:
> As everyone should know by now, we want to integrate the cpuidle
> governor with the scheduler for a more efficient idling of CPUs.
> In order to help the transition, this small patch series moves the
> existing interaction with cpuidle from architecture code to generic
> core code. No functional change should have occurred yet.
>
> The ARM, PPC, SH and X86 architectures are concerned. Small cleanups
> to ARM and ARM64 are also included. I don't know yet the best path for
> those patches to get into mainline, but it is probably best if they
> stay together. So ACKs from architecture maintainers would be greatly
> appreciated.
>
>
> arch/arm/kernel/process.c | 21 +++---------
> arch/arm/kernel/setup.c | 7 ++++
> arch/arm64/kernel/process.c | 5 ---
> arch/arm64/kernel/setup.c | 7 ++++
> arch/powerpc/platforms/pseries/processor_idle.c | 5 +++
> arch/powerpc/platforms/pseries/setup.c | 34 ++++++++-----------
> arch/sh/kernel/idle.c | 4 +--
> arch/x86/kernel/process.c | 5 +--
> include/linux/cpu.h | 1 -
> kernel/Makefile | 1 -
> kernel/cpu/Makefile | 1 -
> kernel/sched/Makefile | 2 +-
> kernel/{cpu => sched}/idle.c | 6 ++--
> 13 files changed, 44 insertions(+), 55 deletions(-)
Thomas, any objections to this? It looks like a sensible thing to do.
^ permalink raw reply
* [PATCH 1/9] ARM: get rid of arch_cpu_idle_prepare()
From: Russell King - ARM Linux @ 2014-01-27 12:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390802904-28399-2-git-send-email-nicolas.pitre@linaro.org>
On Mon, Jan 27, 2014 at 01:08:16AM -0500, Nicolas Pitre wrote:
> ARM and ARM64 are the only two architectures implementing
> arch_cpu_idle_prepare() simply to call local_fiq_enable().
>
> We have secondary_start_kernel() already calling local_fiq_enable() and
> this is done a second time in arch_cpu_idle_prepare() in that case. And
> enabling FIQs has nothing to do with idling the CPU to start with.
>
> So let's introduce init_fiq_boot_cpu() to take care of FIQs on the boot
> CPU and remove arch_cpu_idle_prepare(). This is now done a bit earlier
> at late_initcall time but this shouldn't make a difference in practice
> i.e. when FIQs are actually used.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
> arch/arm/kernel/process.c | 5 -----
> arch/arm/kernel/setup.c | 7 +++++++
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index 92f7b15dd2..725b8c95e0 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -142,11 +142,6 @@ static void default_idle(void)
> local_irq_enable();
> }
>
> -void arch_cpu_idle_prepare(void)
> -{
> - local_fiq_enable();
> -}
> -
> void arch_cpu_idle_enter(void)
> {
> ledtrig_cpu(CPU_LED_IDLE_START);
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 987a7f5bce..d027b1a6fe 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -789,6 +789,13 @@ static int __init init_machine_late(void)
> }
> late_initcall(init_machine_late);
>
> +static int __init init_fiq_boot_cpu(void)
> +{
> + local_fiq_enable();
> + return 0;
> +}
> +late_initcall(init_fiq_boot_cpu);
arch_cpu_idle_prepare() gets called from the swapper thread, and changes
the swapper thread's CPSR. init_fiq_boot_cpu() gets called from PID1, the
init thread, and changes the init thread's CPSR, which will already have
FIQs enabled by way of how kernel threads are created.
Hence, the above code fragment has no effect what so ever, and those
platforms using FIQs will not have FIQs delivered if they're idle
(because the swapper will have FIQs masked at the CPU.)
NAK.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".
^ permalink raw reply
* [PATCH] arm64: add DSB after icache flush in __flush_icache_all()
From: Catalin Marinas @ 2014-01-27 12:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390823984-23046-1-git-send-email-vkale@apm.com>
On Mon, Jan 27, 2014 at 11:59:44AM +0000, Vinayak Kale wrote:
> Add DSB after icache flush operation.
>
> Signed-off-by: Vinayak Kale <vkale@apm.com>
I think we should also mention that this function is used for user
addresses and an ISB is not required because of an exception return
before executing user instructions.
--
Catalin
^ permalink raw reply
* [PATCH] arm64: add DSB after icache flush in __flush_icache_all()
From: Will Deacon @ 2014-01-27 12:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390823984-23046-1-git-send-email-vkale@apm.com>
Hi Vinayak,
On Mon, Jan 27, 2014 at 11:59:44AM +0000, Vinayak Kale wrote:
> Add DSB after icache flush operation.
Please elaborate a bit on what this achieves (i.e. completion of the
maintenance operation).
> Signed-off-by: Vinayak Kale <vkale@apm.com>
> ---
> arch/arm64/include/asm/cacheflush.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h
> index fea9ee3..88932498 100644
> --- a/arch/arm64/include/asm/cacheflush.h
> +++ b/arch/arm64/include/asm/cacheflush.h
> @@ -116,6 +116,7 @@ extern void flush_dcache_page(struct page *);
> static inline void __flush_icache_all(void)
> {
> asm("ic ialluis");
This needs a "memory" clobber to prevent re-ordering by GCC. We should
probably check the rest of the code for other occurrences of this too.
> + dsb();
Can you make a corresponding change for arch/arm/ too, please? I think we're
missing the barrier there as well.
Will
^ permalink raw reply
* [PATCH v3] audit: Add generic compat syscall support
From: Catalin Marinas @ 2014-01-27 12:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E5F56F.2030502@linaro.org>
On Mon, Jan 27, 2014 at 05:58:07AM +0000, AKASHI Takahiro wrote:
> Catalin and audit maintainers,
>
> On 01/23/2014 11:51 PM, Catalin Marinas wrote:
> > On Fri, Jan 17, 2014 at 08:03:15AM +0000, AKASHI Takahiro wrote:
> >> diff --git a/lib/compat_audit.c b/lib/compat_audit.c
> >> new file mode 100644
> >> index 0000000..94f6480
> >> --- /dev/null
> >> +++ b/lib/compat_audit.c
> >> @@ -0,0 +1,51 @@
> >> +#include <linux/init.h>
> >> +#include <linux/types.h>
> >> +/* FIXME: this might be architecture dependent */
> >> +#include <asm/unistd_32.h>
> >
> > It most likely is architecture dependent.
>
> I'm wondering what name is the most appropriate in this case.
> Most archictures have __NR_xyz definitions in "unistd_32.h",
> but arm64 doesn't have it, instead "unistd32." which contains
> only __SYSCALL(xyz, NO). Confusing?
I don't think we should introduce a new file (or at least it should be
named something containing "audit" to make it clearer).
> >> +int audit_classify_compat_syscall(int abi, unsigned syscall)
> >> +{
> >> + switch (syscall) {
> >> +#ifdef __NR_open
> >> + case __NR_open:
> >> + return 2;
> >> +#endif
> >> +#ifdef __NR_openat
> >> + case __NR_openat:
> >> + return 3;
> >> +#endif
> >> +#ifdef __NR_socketcall
> >> + case __NR_socketcall:
> >> + return 4;
> >> +#endif
> >> + case __NR_execve:
> >> + return 5;
> >> + default:
> >> + return 1;
> >> + }
> >> +}
> >
> > BTW, since they aren't many, you could get the arch code to define
> > __NR_compat_open etc. explicitly and use these. On arm64 we have a few
> > of these defined to avoid name collision in signal handling code.
>
> Again, most architecture have their own unistd32.h for compat system calls,
> and use __NR_open-like naming.
> It's unlikely for these archs to migrate to "generic compat" auditing,
> but I believe that '__NR_open'-like naming is better because we may be able to avoid
> arch-specific changes even for future(?) syscall-related enhancements in audit.
My preference is as above, a few __NR_compat_* (just those required by
audit) defined in unistd.h but I'm not an audit maintainer.
--
Catalin
^ permalink raw reply
* [PATCH 6/9] PPC: remove redundant cpuidle_idle_call()
From: Preeti U Murthy @ 2014-01-27 11:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390802904-28399-7-git-send-email-nicolas.pitre@linaro.org>
Hi Nicolas,
On 01/27/2014 11:38 AM, Nicolas Pitre wrote:
> The core idle loop now takes care of it. However a few things need
> checking:
>
> - Invocation of cpuidle_idle_call() in pseries_lpar_idle() happened
> through arch_cpu_idle() and was therefore always preceded by a call
> to ppc64_runlatch_off(). To preserve this property now that
> cpuidle_idle_call() is invoked directly from core code, a call to
> ppc64_runlatch_off() has been added to idle_loop_prolog() in
> platforms/pseries/processor_idle.c.
>
> - Similarly, cpuidle_idle_call() was followed by ppc64_runlatch_off()
> so a call to the later has been added to idle_loop_epilog().
>
> - And since arch_cpu_idle() always made sure to re-enable IRQs if they
> were not enabled, this is now
> done in idle_loop_epilog() as well.
>
> The above was made in order to keep the execution flow close to the
> original. I don't know if that was strictly necessary. Someone well
> aquainted with the platform details might find some room for possible
> optimizations.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
> arch/powerpc/platforms/pseries/processor_idle.c | 5 ++++
> arch/powerpc/platforms/pseries/setup.c | 34 ++++++++++---------------
> 2 files changed, 19 insertions(+), 20 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/processor_idle.c b/arch/powerpc/platforms/pseries/processor_idle.c
> index a166e38bd6..72ddfe3d2f 100644
> --- a/arch/powerpc/platforms/pseries/processor_idle.c
> +++ b/arch/powerpc/platforms/pseries/processor_idle.c
> @@ -33,6 +33,7 @@ static struct cpuidle_state *cpuidle_state_table;
>
> static inline void idle_loop_prolog(unsigned long *in_purr)
> {
> + ppc64_runlatch_off();
> *in_purr = mfspr(SPRN_PURR);
> /*
> * Indicate to the HV that we are idle. Now would be
> @@ -49,6 +50,10 @@ static inline void idle_loop_epilog(unsigned long in_purr)
> wait_cycles += mfspr(SPRN_PURR) - in_purr;
> get_lppaca()->wait_state_cycles = cpu_to_be64(wait_cycles);
> get_lppaca()->idle = 0;
> +
> + if (irqs_disabled())
> + local_irq_enable();
> + ppc64_runlatch_on();
> }
>
> static int snooze_loop(struct cpuidle_device *dev,
> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
> index c1f1908587..7604c19d54 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -39,7 +39,6 @@
> #include <linux/irq.h>
> #include <linux/seq_file.h>
> #include <linux/root_dev.h>
> -#include <linux/cpuidle.h>
> #include <linux/of.h>
> #include <linux/kexec.h>
>
> @@ -356,29 +355,24 @@ early_initcall(alloc_dispatch_log_kmem_cache);
>
> static void pseries_lpar_idle(void)
> {
> - /* This would call on the cpuidle framework, and the back-end pseries
> - * driver to go to idle states
> + /*
> + * Default handler to go into low thread priority and possibly
> + * low power mode by cedeing processor to hypervisor
> */
> - if (cpuidle_idle_call()) {
> - /* On error, execute default handler
> - * to go into low thread priority and possibly
> - * low power mode by cedeing processor to hypervisor
> - */
>
> - /* Indicate to hypervisor that we are idle. */
> - get_lppaca()->idle = 1;
> + /* Indicate to hypervisor that we are idle. */
> + get_lppaca()->idle = 1;
>
> - /*
> - * Yield the processor to the hypervisor. We return if
> - * an external interrupt occurs (which are driven prior
> - * to returning here) or if a prod occurs from another
> - * processor. When returning here, external interrupts
> - * are enabled.
> - */
> - cede_processor();
> + /*
> + * Yield the processor to the hypervisor. We return if
> + * an external interrupt occurs (which are driven prior
> + * to returning here) or if a prod occurs from another
> + * processor. When returning here, external interrupts
> + * are enabled.
> + */
> + cede_processor();
>
> - get_lppaca()->idle = 0;
> - }
> + get_lppaca()->idle = 0;
> }
>
> /*
>
Reviewed-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
The consequence of this would be for other Power platforms like PowerNV,
we will need to invoke ppc_runlatch_off() and ppc_runlatch_on() in each
of the idle routines since the idle_loop_prologue() and
idle_loop_epilogue() are not invoked by them, but we will take care of this.
Regards
Preeti U Murthy
^ permalink raw reply
* [PATCH] arm64: add DSB after icache flush in __flush_icache_all()
From: Vinayak Kale @ 2014-01-27 11:59 UTC (permalink / raw)
To: linux-arm-kernel
Add DSB after icache flush operation.
Signed-off-by: Vinayak Kale <vkale@apm.com>
---
arch/arm64/include/asm/cacheflush.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h
index fea9ee3..88932498 100644
--- a/arch/arm64/include/asm/cacheflush.h
+++ b/arch/arm64/include/asm/cacheflush.h
@@ -116,6 +116,7 @@ extern void flush_dcache_page(struct page *);
static inline void __flush_icache_all(void)
{
asm("ic ialluis");
+ dsb();
}
#define flush_dcache_mmap_lock(mapping) \
--
1.7.9.5
^ permalink raw reply related
* [PATCH RFC v2 2/2] Documentation: arm: define DT C-states bindings
From: Lorenzo Pieralisi @ 2014-01-27 11:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140125.101546.1134862463481697494.apm@brigitte.kvy.fi>
On Sat, Jan 25, 2014 at 08:15:46AM +0000, Antti P Miettinen wrote:
> From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Subject: [PATCH RFC v2 2/2] Documentation: arm: define DT C-states bindings
> Date: Mon, 20 Jan 2014 17:47:59 +0000
> > + - latency
> > + Usage: Required
> > + Value type: <prop-encoded-array>
> > + Definition: List of u32 values representing worst case latency
> > + in microseconds required to enter and exit the
> > + C-state, one value per OPP [2]. The list should
> > + be specified in the same order as the operating
> > + points property list of the cpu this state is
> > + valid on.
> > + If no OPP bindings are present, the latency value
> > + is associated with the current OPP of CPUs in the
> > + system.
>
> I'm afraid the CPU OPP is not enough to capture the variance in
> latencies. Especially memory frequency affects some of the latencies
> very stronly.
That's why I defined the worst case. How did you implemented it in your
idle drivers ? That would help generalize it, after all these bindings
are there to simplify drivers upstreaming, feedback welcome.
Thanks,
Lorenzo
^ permalink raw reply
* [PATCH v5 07/14] ahci-platform: "Library-ise" ahci_probe functionality
From: Hans de Goede @ 2014-01-27 11:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E63D08.6080704@ti.com>
Hi,
On 01/27/2014 12:03 PM, Roger Quadros wrote:
> On 01/27/2014 12:51 PM, Hans de Goede wrote:
>> Hi,
>>
>> On 01/27/2014 11:39 AM, Roger Quadros wrote:
>>> Hi,
>>>
>>> On 01/22/2014 09:04 PM, Hans de Goede wrote:
>>
>> <snip>
>>
>>>> --- a/include/linux/ahci_platform.h
>>>> +++ b/include/linux/ahci_platform.h
>>>> @@ -20,7 +20,13 @@
>>>> struct device;
>>>> struct ata_port_info;
>>>> struct ahci_host_priv;
>>>> +struct platform_device;
>>>>
>>>> +/*
>>>> + * Note ahci_platform_data is deprecated. New drivers which need to override
>>>> + * any of these, should instead declare there own platform_driver struct, and
>>>> + * use ahci_platform* functions in their own probe, suspend and resume methods.
>>>> + */
>>>> struct ahci_platform_data {
>>>> int (*init)(struct device *dev, struct ahci_host_priv *hpriv);
>>>> void (*exit)(struct device *dev);
>>>> @@ -35,5 +41,13 @@ int ahci_platform_enable_clks(struct ahci_host_priv *hpriv);
>>>> void ahci_platform_disable_clks(struct ahci_host_priv *hpriv);
>>>> int ahci_platform_enable_resources(struct ahci_host_priv *hpriv);
>>>> void ahci_platform_disable_resources(struct ahci_host_priv *hpriv);
>>>> +struct ahci_host_priv *ahci_platform_get_resources(
>>>> + struct platform_device *pdev);
>>>
>>> Why not use 'struct device' as the argument?
>>
>> Because of calls to platform_get_resource inside the function.
>>
>>>> +void ahci_platform_put_resources(struct ahci_host_priv *hpriv);
>>>
>>> Can we have 'struct device' as the argument? Else it becomes
>>> impossible to get 'struct device' from 'hpriv' if we need to call e.g.
>>> pm_runtime_*() APIs.
>>
>> The plan for is for this function to go away once we have a
>> devm version of of_clk_get, so if you need to put pm_runtime_calls
>> somewhere, please don't put them here. This sounds like something which
>> should go in enable / disable resources instead ?
>
> OK. I need to add pm_runtime_enable() + pm_runtime_get_sync() during
> initialization and pm_runtime_put_sync() + pm_runtime_disable() during cleanup.
Note that enable / disable resources will get called by (the default implementations
of) suspend / resume too.
If that is undesirable then I take back what I said before and
ahci_platform_put_resources' prototype should be changed to:
void ahci_platform_put_resources(struct device *dev, struct ahci_host_priv *hpriv);
And we will need to keep it around even after we get devm_of_clk_get.
> If ahci_platform_enable/disable_resources is the right place then we must be
> able to access struct device from there.
Right, and if not we need to access it from ahci_platform_put_resources(),
which is in essence the same problem.
> Is it a good to add 'struct device *dev' into the 'struct ahci_host_priv'?
> Then you can leave this series as is and i'll add a new patch for that.
Normally we get a device * as argument, and get to hpriv like this:
struct ata_host *host = dev_get_drvdata(dev);
struct ahci_host_priv *hpriv = host->private_data;
So having a dev * in hpriv is normally not useful.
But the ata_host gets allocated after the first ahci_platform_enable_resources
call, so we cannot use this there. Likewise disable_resources / put_resources
is used in error handling paths in probe where we don't have an ata_host yet,
so my vote goes to adding a "struct device *dev" as first argument, like I
suggested above for ahci_platform_put_resources.
This can be done as an add-on patch (if you do don't forget to also fix
ahci_sunxi.c and ahci_imx.c), or I can respin my series to have this from
day one.
If you want me to do a respin, please let me know which fix you'll need
(the put_resources or the enable/disable one).
Thanks & Regards,
Hans
^ permalink raw reply
* [PATCH 17/20] clocksource / arch_timer: Use ACPI GTDT table to initialize arch timer
From: Mark Rutland @ 2014-01-27 11:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1389961514-13562-18-git-send-email-hanjun.guo@linaro.org>
On Fri, Jan 17, 2014 at 12:25:11PM +0000, Hanjun Guo wrote:
> ACPI GTDT (Generic Timer Description Table) contains information for
> arch timer initialization, this patch use this table to probe arm timer.
>
> GTDT table is used for ARM/ARM64 only, please refer to chapter 5.2.24
> of ACPI 5.0 spec for detailed inforamtion
>
> Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> ---
> drivers/clocksource/arm_arch_timer.c | 100 +++++++++++++++++++++++++++++-----
> 1 file changed, 85 insertions(+), 15 deletions(-)
[...]
> +static void __init register_arch_interrupt(u32 interrupt, u32 flags,
> + int *arch_timer_ppi)
> +{
> + int trigger, polarity;
> +
> + if (!interrupt || !arch_timer_ppi)
> + return;
> +
> + trigger = (flags & ACPI_GTDT_INTERRUPT_MODE) ? ACPI_EDGE_SENSITIVE
> + : ACPI_LEVEL_SENSITIVE;
> +
> + polarity = (flags & ACPI_GTDT_INTERRUPT_POLARITY) ? ACPI_ACTIVE_LOW
> + : ACPI_ACTIVE_HIGH;
> +
> + *arch_timer_ppi = acpi_register_gsi(NULL, interrupt, trigger,
> + polarity);
> +}
Why does this take a pointer to the irq rather than returning the irq
(as with irq_of_parse_and_map)?
This looks awfully generic. Are the timer interrupts encoded specially
or is this useful for parsing other interrupts?
> +
> +static int __init acpi_parse_gtdt(struct acpi_table_header *table)
> +{
> + struct acpi_table_gtdt *gtdt;
> +
> + gtdt = (struct acpi_table_gtdt *)table;
> + if (!gtdt)
> + return -EINVAL;
> +
> + arch_timer_rate = arch_timer_get_cntfrq();
> +
> + if (!arch_timer_rate) {
> + pr_warn("arch_timer: Could not get frequency from CNTFREG\n");
s/CNTFREG/CNTFREQ/
This is probably worth a pr_err at least, the system is unlikely to get
very far if the timers don't work.
Thanks,
Mark.
^ permalink raw reply
* [PATCH v2] ARM: mm: Fix stage-2 device memory attributes
From: Marc Zyngier @ 2014-01-27 11:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140124233703.GA889@cbox>
On 24/01/14 23:37, Christoffer Dall wrote:
> On Sat, Jan 04, 2014 at 08:27:23AM -0800, Christoffer Dall wrote:
>> The stage-2 memory attributes are distinct from the Hyp memory
>> attributes and the Stage-1 memory attributes. We were using the stage-1
>> memory attributes for stage-2 mappings causing device mappings to be
>> mapped as normal memory. Add the S2 equivalent defines for memory
>> attributes and fix the comments explaining the defines while at it.
>>
>> Add a prot_pte_s2 field to the mem_type struct and fill out the field
>> for device mappings accordingly.
>>
>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
>> ---
>> Changelog[v2]:
>> - Guard the use of L_PTE_S2 defines with s2_policy to allow non-LPAE compiles.
>>
>> arch/arm/include/asm/pgtable-3level.h | 20 +++++++++++++-------
>> arch/arm/mm/mm.h | 1 +
>> arch/arm/mm/mmu.c | 15 ++++++++++++++-
>> 3 files changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/pgtable-3level.h b/arch/arm/include/asm/pgtable-3level.h
>> index 4f95039..d5e04d6 100644
>> --- a/arch/arm/include/asm/pgtable-3level.h
>> +++ b/arch/arm/include/asm/pgtable-3level.h
>> @@ -120,13 +120,19 @@
>> /*
>> * 2nd stage PTE definitions for LPAE.
>> */
>> -#define L_PTE_S2_MT_UNCACHED (_AT(pteval_t, 0x5) << 2) /* MemAttr[3:0] */
>> -#define L_PTE_S2_MT_WRITETHROUGH (_AT(pteval_t, 0xa) << 2) /* MemAttr[3:0] */
>> -#define L_PTE_S2_MT_WRITEBACK (_AT(pteval_t, 0xf) << 2) /* MemAttr[3:0] */
>> -#define L_PTE_S2_RDONLY (_AT(pteval_t, 1) << 6) /* HAP[1] */
>> -#define L_PTE_S2_RDWR (_AT(pteval_t, 3) << 6) /* HAP[2:1] */
>> -
>> -#define L_PMD_S2_RDWR (_AT(pmdval_t, 3) << 6) /* HAP[2:1] */
>> +#define L_PTE_S2_MT_UNCACHED (_AT(pteval_t, 0x0) << 2) /* strongly ordered */
>> +#define L_PTE_S2_MT_WRITETHROUGH (_AT(pteval_t, 0xa) << 2) /* normal inner write-through */
>> +#define L_PTE_S2_MT_WRITEBACK (_AT(pteval_t, 0xf) << 2) /* normal inner write-back */
>> +#define L_PTE_S2_MT_DEV_SHARED (_AT(pteval_t, 0x1) << 2) /* device */
>> +#define L_PTE_S2_MT_DEV_NONSHARED (_AT(pteval_t, 0x1) << 2) /* device */
>> +#define L_PTE_S2_MT_DEV_WC (_AT(pteval_t, 0x5) << 2) /* normal non-cacheable */
>> +#define L_PTE_S2_MT_DEV_CACHED (_AT(pteval_t, 0xf) << 2) /* normal inner write-back */
>> +#define L_PTE_S2_MT_MASK (_AT(pteval_t, 0xf) << 2)
>> +
>> +#define L_PTE_S2_RDONLY (_AT(pteval_t, 1) << 6) /* HAP[1] */
>> +#define L_PTE_S2_RDWR (_AT(pteval_t, 3) << 6) /* HAP[2:1] */
>> +
>> +#define L_PMD_S2_RDWR (_AT(pmdval_t, 3) << 6) /* HAP[2:1] */
>>
>> /*
>> * Hyp-mode PL2 PTE definitions for LPAE.
>> diff --git a/arch/arm/mm/mm.h b/arch/arm/mm/mm.h
>> index d5a982d..7ea641b 100644
>> --- a/arch/arm/mm/mm.h
>> +++ b/arch/arm/mm/mm.h
>> @@ -38,6 +38,7 @@ static inline pmd_t *pmd_off_k(unsigned long virt)
>>
>> struct mem_type {
>> pteval_t prot_pte;
>> + pteval_t prot_pte_s2;
>> pmdval_t prot_l1;
>> pmdval_t prot_sect;
>> unsigned int domain;
>> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
>> index 580ef2d..44d571f 100644
>> --- a/arch/arm/mm/mmu.c
>> +++ b/arch/arm/mm/mmu.c
>> @@ -231,36 +231,48 @@ __setup("noalign", noalign_setup);
>> #endif /* ifdef CONFIG_CPU_CP15 / else */
>>
>> #define PROT_PTE_DEVICE L_PTE_PRESENT|L_PTE_YOUNG|L_PTE_DIRTY|L_PTE_XN
>> +#define PROT_PTE_S2_DEVICE PROT_PTE_DEVICE
>> #define PROT_SECT_DEVICE PMD_TYPE_SECT|PMD_SECT_AP_WRITE
>>
>> static struct mem_type mem_types[] = {
>> [MT_DEVICE] = { /* Strongly ordered / ARMv6 shared device */
>> .prot_pte = PROT_PTE_DEVICE | L_PTE_MT_DEV_SHARED |
>> L_PTE_SHARED,
>> + .prot_pte_s2 = s2_policy(PROT_PTE_S2_DEVICE) |
>> + s2_policy(L_PTE_S2_MT_DEV_SHARED) |
>> + L_PTE_SHARED,
>> .prot_l1 = PMD_TYPE_TABLE,
>> .prot_sect = PROT_SECT_DEVICE | PMD_SECT_S,
>> .domain = DOMAIN_IO,
>> },
>> [MT_DEVICE_NONSHARED] = { /* ARMv6 non-shared device */
>> .prot_pte = PROT_PTE_DEVICE | L_PTE_MT_DEV_NONSHARED,
>> + .prot_pte_s2 = s2_policy(PROT_PTE_S2_DEVICE) |
>> + s2_policy(L_PTE_S2_MT_DEV_NONSHARED),
>> .prot_l1 = PMD_TYPE_TABLE,
>> .prot_sect = PROT_SECT_DEVICE,
>> .domain = DOMAIN_IO,
>> },
>> [MT_DEVICE_CACHED] = { /* ioremap_cached */
>> .prot_pte = PROT_PTE_DEVICE | L_PTE_MT_DEV_CACHED,
>> + .prot_pte_s2 = s2_policy(PROT_PTE_S2_DEVICE) |
>> + s2_policy(L_PTE_S2_MT_DEV_CACHED),
>> .prot_l1 = PMD_TYPE_TABLE,
>> .prot_sect = PROT_SECT_DEVICE | PMD_SECT_WB,
>> .domain = DOMAIN_IO,
>> },
>> [MT_DEVICE_WC] = { /* ioremap_wc */
>> .prot_pte = PROT_PTE_DEVICE | L_PTE_MT_DEV_WC,
>> + .prot_pte_s2 = s2_policy(PROT_PTE_S2_DEVICE) |
>> + s2_policy(L_PTE_S2_MT_DEV_WC),
>> .prot_l1 = PMD_TYPE_TABLE,
>> .prot_sect = PROT_SECT_DEVICE,
>> .domain = DOMAIN_IO,
>> },
>> [MT_UNCACHED] = {
>> .prot_pte = PROT_PTE_DEVICE,
>> + .prot_pte_s2 = s2_policy(PROT_PTE_S2_DEVICE) |
>> + s2_policy(L_PTE_S2_MT_UNCACHED),
>> .prot_l1 = PMD_TYPE_TABLE,
>> .prot_sect = PMD_TYPE_SECT | PMD_SECT_XN,
>> .domain = DOMAIN_IO,
>> @@ -458,7 +470,8 @@ static void __init build_mem_type_table(void)
>> cp = &cache_policies[cachepolicy];
>> vecs_pgprot = kern_pgprot = user_pgprot = cp->pte;
>> s2_pgprot = cp->pte_s2;
>> - hyp_device_pgprot = s2_device_pgprot = mem_types[MT_DEVICE].prot_pte;
>> + hyp_device_pgprot = mem_types[MT_DEVICE].prot_pte;
>> + s2_device_pgprot = mem_types[MT_DEVICE].prot_pte_s2;
>>
>> /*
>> * ARMv6 and above have extended page tables.
>> --
>> 1.8.5
>>
>
> Ping?
Lost that one in the flurry of endianness bike shedding messages.
The change makes sense to me. arm64 uses a slightly different approach,
by using a PTE_S2_MEMATTR macro, but I'm not sure that would work for ARM.
Russell, Catalin: could you please have a look at this?
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH 0/2] Replace /include/ (dtc) with #include
From: Pankaj Dubey @ 2014-01-27 11:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1389348970-5498-1-git-send-email-pankaj.dubey@samsung.com>
On 01/10/2014 07:16 PM, Pankaj Dubey wrote:
> Replace /include/ (dtc) with #include (C pre-processor) for all ARM64
> based SoC dts files.
>
> Pankaj Dubey (2):
> arm64: dts: use #include for all AppliedMicro device tree files
> arm64: dts: use #include for all ARM fast model device tree file
>
> arch/arm64/boot/dts/apm-mustang.dts | 2 +-
> arch/arm64/boot/dts/rtsm_ve-aemv8a.dts | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
Gentle ping.
Best Regards,
Pankaj Dubey
^ permalink raw reply
* [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Srikanth Thokala @ 2014-01-27 11:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140126135933.GD10628@intel.com>
Hi Vinod,
On Sun, Jan 26, 2014 at 7:29 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Fri, Jan 24, 2014 at 02:24:27PM +0100, Lars-Peter Clausen wrote:
>> On 01/24/2014 12:16 PM, Srikanth Thokala wrote:
>> > Hi Lars,
>> >
>> > On Thu, Jan 23, 2014 at 4:55 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> >> On 01/22/2014 05:52 PM, Srikanth Thokala wrote:
>> >> [...]
>> >>> +/**
>> >>> + * xilinx_vdma_device_control - Configure DMA channel of the device
>> >>> + * @dchan: DMA Channel pointer
>> >>> + * @cmd: DMA control command
>> >>> + * @arg: Channel configuration
>> >>> + *
>> >>> + * Return: '0' on success and failure value on error
>> >>> + */
>> >>> +static int xilinx_vdma_device_control(struct dma_chan *dchan,
>> >>> + enum dma_ctrl_cmd cmd, unsigned long arg)
>> >>> +{
>> >>> + struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan);
>> >>> +
>> >>> + switch (cmd) {
>> >>> + case DMA_TERMINATE_ALL:
>> >>> + xilinx_vdma_terminate_all(chan);
>> >>> + return 0;
>> >>> + case DMA_SLAVE_CONFIG:
>> >>> + return xilinx_vdma_slave_config(chan,
>> >>> + (struct xilinx_vdma_config *)arg);
>> >>
>> >> You really shouldn't be overloading the generic API with your own semantics.
>> >> DMA_SLAVE_CONFIG should take a dma_slave_config and nothing else.
>> >
>> > Ok. The driver needs few additional configuration from the slave
>> > device like Vertical
>> > Size, Horizontal Size, Stride etc., for the DMA transfers, in that case do you
>> > suggest me to define a separate dma_ctrl_cmd like the one FSLDMA_EXTERNAL_START
>> > defined for Freescale drivers?
>>
>> In my opinion it is not a good idea to have driver implement a generic API,
>> but at the same time let the driver have custom semantics for those API
>> calls. It's a bit like having a gpio driver that expects 23 and 42 as the
>> values passed to gpio_set_value instead of 0 and 1. It completely defeats
>> the purpose of a generic API, namely that you are able to write generic code
>> that makes use of the API without having to know about which implementation
>> API it is talking to. The dmaengine framework provides the
>> dmaengine_prep_interleaved_dma() function to setup two dimensional
>> transfers, e.g. take a look at sirf-dma.c or imx-dma.c.
>
> The question here i think would be waht this device supports? Is the hardware
> capable of doing interleaved transfers, then would make sense.
>
> While we do try to get users use dma_slave_config, but there will always be
> someone who have specfic params. If we can generalize then we might want to add
> to the dma_slave_config as well
There are many configuration parameters which are specific to IP and I
would like to
give an overview of some of parameteres here:
1) Park Mode ('cfg->park'): In Park mode, engine will park on frame
referenced by
'cfg->park_frm', so user will have control on each frame in this mode.
2) Interrupt Coalesce ('cfg->coalesce'): Used for setting interrupt
threshold. This value
determines the number of frame buffers to process. To use this feature,
'cfg->frm_cnt_en' should be set.
3) Frame Synchronization Source ('cfg->ext_fsync'): Can be an
external/internal frame
synchronization source. Used to synchronize one channel (MM2S/S2MM) with
another (S2MM/MM2S) channel.
4) Genlock Synchronization ('cfg->genlock'): Used to avoid mismatch rate between
master and slave. In master mode (cfg->master), frames are not dropped and
slave can drop frames to adjust to master frame rate.
And in future, this Engine being a soft IP, we could expect some more additional
parameters. Isn't a good idea to have a private member in dma_slave_config for
sharing additional configuration between slave device and dma engine? Or a new
dma_ctrl_cmd like FSLDMA_EXTERNAL_START?
Srikanth
>
> --
> ~Vinod
> --/EX
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* [PATCH] arm64: dts: add <dt-bindings/> symlink
From: Pankaj Dubey @ 2014-01-27 11:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140110115432.21fb03af@ipc1.ka-ro>
On 01/10/2014 07:54 PM, Lothar Wa?mann wrote:
> Hi,
>
> Pankaj Dubey wrote:
>> On 01/10/2014 06:22 PM, Lothar Wa?mann wrote:
>>> Hi,
>>>
>>> Pankaj Dubey wrote:
>>>> Add symlink to include/dt-bindings from arch/arm64/boot/dts/include/ to
>>>> match the ones in ARM architectures so that preprocessed device
>>>> tree files can include various useful constant definitions.
>>>>
>>>> See commit c58299a (kbuild: create an "include chroot" for DT bindings)
>>>> merged in v3.10-rc1 for details.
>>>>
>>>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>>> ---
>>>> arch/arm64/boot/dts/include/dt-bindings | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>> create mode 120000 arch/arm64/boot/dts/include/dt-bindings
>>>>
>>>> diff --git a/arch/arm64/boot/dts/include/dt-bindings b/arch/arm64/boot/dts/include/dt-bindings
>>>> new file mode 120000
>>>> index 0000000..08c00e4
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/include/dt-bindings
>>>> @@ -0,0 +1 @@
>>>> +../../../../../include/dt-bindings
>>>> \ No newline at end of file
>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> I just created symlink file to "include/dt-bindings", and when created
>> patch it is showing this line
>> at the end of patch. I do not have much idea why this came in patch? But
>> when I checked similar patches
>> it looks like it is required.
>>
> I noticed after sending the email, that the file in question was
> actually only a symlink. The message seems to be a 'feature' of
> 'patch' when dealing with symlinks.
>
>
> Lothar Wa?mann
Gentle Ping.
Best Regards,
Pankaj Dubey
^ permalink raw reply
* [PATCH v5 07/14] ahci-platform: "Library-ise" ahci_probe functionality
From: Roger Quadros @ 2014-01-27 11:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E63A1F.6080301@redhat.com>
On 01/27/2014 12:51 PM, Hans de Goede wrote:
> Hi,
>
> On 01/27/2014 11:39 AM, Roger Quadros wrote:
>> Hi,
>>
>> On 01/22/2014 09:04 PM, Hans de Goede wrote:
>
> <snip>
>
>>> --- a/include/linux/ahci_platform.h
>>> +++ b/include/linux/ahci_platform.h
>>> @@ -20,7 +20,13 @@
>>> struct device;
>>> struct ata_port_info;
>>> struct ahci_host_priv;
>>> +struct platform_device;
>>>
>>> +/*
>>> + * Note ahci_platform_data is deprecated. New drivers which need to override
>>> + * any of these, should instead declare there own platform_driver struct, and
>>> + * use ahci_platform* functions in their own probe, suspend and resume methods.
>>> + */
>>> struct ahci_platform_data {
>>> int (*init)(struct device *dev, struct ahci_host_priv *hpriv);
>>> void (*exit)(struct device *dev);
>>> @@ -35,5 +41,13 @@ int ahci_platform_enable_clks(struct ahci_host_priv *hpriv);
>>> void ahci_platform_disable_clks(struct ahci_host_priv *hpriv);
>>> int ahci_platform_enable_resources(struct ahci_host_priv *hpriv);
>>> void ahci_platform_disable_resources(struct ahci_host_priv *hpriv);
>>> +struct ahci_host_priv *ahci_platform_get_resources(
>>> + struct platform_device *pdev);
>>
>> Why not use 'struct device' as the argument?
>
> Because of calls to platform_get_resource inside the function.
>
>>> +void ahci_platform_put_resources(struct ahci_host_priv *hpriv);
>>
>> Can we have 'struct device' as the argument? Else it becomes
>> impossible to get 'struct device' from 'hpriv' if we need to call e.g.
>> pm_runtime_*() APIs.
>
> The plan for is for this function to go away once we have a
> devm version of of_clk_get, so if you need to put pm_runtime_calls
> somewhere, please don't put them here. This sounds like something which
> should go in enable / disable resources instead ?
OK. I need to add pm_runtime_enable() + pm_runtime_get_sync() during
initialization and pm_runtime_put_sync() + pm_runtime_disable() during cleanup.
If ahci_platform_enable/disable_resources is the right place then we must be
able to access struct device from there.
Is it a good to add 'struct device *dev' into the 'struct ahci_host_priv'?
Then you can leave this series as is and i'll add a new patch for that.
If there is a better way, please let me know.
cheers,
-roger
^ permalink raw reply
* [PATCH v5 07/14] ahci-platform: "Library-ise" ahci_probe functionality
From: Hans de Goede @ 2014-01-27 10:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E63778.5000509@ti.com>
Hi,
On 01/27/2014 11:39 AM, Roger Quadros wrote:
> Hi,
>
> On 01/22/2014 09:04 PM, Hans de Goede wrote:
<snip>
>> --- a/include/linux/ahci_platform.h
>> +++ b/include/linux/ahci_platform.h
>> @@ -20,7 +20,13 @@
>> struct device;
>> struct ata_port_info;
>> struct ahci_host_priv;
>> +struct platform_device;
>>
>> +/*
>> + * Note ahci_platform_data is deprecated. New drivers which need to override
>> + * any of these, should instead declare there own platform_driver struct, and
>> + * use ahci_platform* functions in their own probe, suspend and resume methods.
>> + */
>> struct ahci_platform_data {
>> int (*init)(struct device *dev, struct ahci_host_priv *hpriv);
>> void (*exit)(struct device *dev);
>> @@ -35,5 +41,13 @@ int ahci_platform_enable_clks(struct ahci_host_priv *hpriv);
>> void ahci_platform_disable_clks(struct ahci_host_priv *hpriv);
>> int ahci_platform_enable_resources(struct ahci_host_priv *hpriv);
>> void ahci_platform_disable_resources(struct ahci_host_priv *hpriv);
>> +struct ahci_host_priv *ahci_platform_get_resources(
>> + struct platform_device *pdev);
>
> Why not use 'struct device' as the argument?
Because of calls to platform_get_resource inside the function.
>> +void ahci_platform_put_resources(struct ahci_host_priv *hpriv);
>
> Can we have 'struct device' as the argument? Else it becomes
> impossible to get 'struct device' from 'hpriv' if we need to call e.g.
> pm_runtime_*() APIs.
The plan for is for this function to go away once we have a
devm version of of_clk_get, so if you need to put pm_runtime_calls
somewhere, please don't put them here. This sounds like something which
should go in enable / disable resources instead ?
Regards,
Hans
^ permalink raw reply
* [PATCH] drivers: bus: fix CCI driver kcalloc call parameters swap
From: Lorenzo Pieralisi @ 2014-01-27 10:50 UTC (permalink / raw)
To: linux-arm-kernel
This patch fixes a bug/typo in the CCI driver kcalloc usage
that inadvertently swapped the parameters order in the
kcalloc call and went unnoticed.
Reported-by: Xia Feng <xiafeng@allwinnertech.com>
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
drivers/bus/arm-cci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index b6739cb..962fd35 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -979,7 +979,7 @@ static int cci_probe(void)
nb_cci_ports = cci_config->nb_ace + cci_config->nb_ace_lite;
- ports = kcalloc(sizeof(*ports), nb_cci_ports, GFP_KERNEL);
+ ports = kcalloc(nb_cci_ports, sizeof(*ports), GFP_KERNEL);
if (!ports)
return -ENOMEM;
--
1.8.2.2
^ permalink raw reply related
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