* [PATCH 0/7] arm64: marvell: add cryptographic engine support for 7k/8k
From: Herbert Xu @ 2017-04-12 6:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87efwyj53q.fsf@free-electrons.com>
On Tue, Apr 11, 2017 at 06:12:25PM +0200, Gregory CLEMENT wrote:
>
> > I have no problems with the crypto bits.
>
> Does it means that you agree that I apply the arm-soc related patches
> (from 4 to 7) to my mvebu trees?
Yes sure.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [GIT PULL] PCI: Support for configurable PCI endpoint
From: Kishon Vijay Abraham I @ 2017-04-12 5:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170411193447.GA14778@bhelgaas-glaptop.roam.corp.google.com>
Hi Bjorn,
On Wednesday 12 April 2017 01:04 AM, Bjorn Helgaas wrote:
> On Mon, Apr 10, 2017 at 10:43:28AM -0500, Bjorn Helgaas wrote:
>> On Wed, Apr 05, 2017 at 02:22:20PM +0530, Kishon Vijay Abraham I wrote:
>>> Hi Bjorn,
>>>
>>> Please find the pull request for PCI endpoint support below. I've
>>> also included all the history here.
>>
>> Thanks, I applied these (with v7 of the first patch) to pci/host-designware
>> for v4.12.
>
> Ok, sorry, I screwed this up. I think my branch actually had v5, not
> v6. But I *think* I fixed it. Here's the diff from my branch to your
> git tree. Apparently you haven't pushed the v7 patch there, so I
> *think* the diff below is the diff between v6 and v7 of that first
> patch.
I just checked your pci/host-designware branch and it looks correct. Thanks for
sorting this out.
Cheers
Kishon
^ permalink raw reply
* [PATCH] Revert "arm64: Increase the max granular size"
From: Imran Khan @ 2017-04-12 5:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CADAEsF_Hr=mPspvuPsQtKWiSDu6oCjfyy0rGwWrF9EJo-ZO1JA@mail.gmail.com>
On 4/7/2017 7:36 AM, Ganesh Mahendran wrote:
> 2017-04-06 23:58 GMT+08:00 Catalin Marinas <catalin.marinas@arm.com>:
>> On Thu, Apr 06, 2017 at 12:52:13PM +0530, Imran Khan wrote:
>>> On 4/5/2017 10:13 AM, Imran Khan wrote:
>>>>> We may have to revisit this logic and consider L1_CACHE_BYTES the
>>>>> _minimum_ of cache line sizes in arm64 systems supported by the kernel.
>>>>> Do you have any benchmarks on Cavium boards that would show significant
>>>>> degradation with 64-byte L1_CACHE_BYTES vs 128?
>>>>>
>>>>> For non-coherent DMA, the simplest is to make ARCH_DMA_MINALIGN the
>>>>> _maximum_ of the supported systems:
>>>>>
>>>>> diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
>>>>> index 5082b30bc2c0..4b5d7b27edaf 100644
>>>>> --- a/arch/arm64/include/asm/cache.h
>>>>> +++ b/arch/arm64/include/asm/cache.h
>>>>> @@ -18,17 +18,17 @@
>>>>>
>>>>> #include <asm/cachetype.h>
>>>>>
>>>>> -#define L1_CACHE_SHIFT 7
>>>>> +#define L1_CACHE_SHIFT 6
>>>>> #define L1_CACHE_BYTES (1 << L1_CACHE_SHIFT)
>>>>>
>>>>> /*
>>>>> * Memory returned by kmalloc() may be used for DMA, so we must make
>>>>> - * sure that all such allocations are cache aligned. Otherwise,
>>>>> - * unrelated code may cause parts of the buffer to be read into the
>>>>> - * cache before the transfer is done, causing old data to be seen by
>>>>> - * the CPU.
>>>>> + * sure that all such allocations are aligned to the maximum *known*
>>>>> + * cache line size on ARMv8 systems. Otherwise, unrelated code may cause
>>>>> + * parts of the buffer to be read into the cache before the transfer is
>>>>> + * done, causing old data to be seen by the CPU.
>>>>> */
>>>>> -#define ARCH_DMA_MINALIGN L1_CACHE_BYTES
>>>>> +#define ARCH_DMA_MINALIGN (128)
>>>>>
>>>>> #ifndef __ASSEMBLY__
>>>>>
>>>>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>>>>> index 392c67eb9fa6..30bafca1aebf 100644
>>>>> --- a/arch/arm64/kernel/cpufeature.c
>>>>> +++ b/arch/arm64/kernel/cpufeature.c
>>>>> @@ -976,9 +976,9 @@ void __init setup_cpu_features(void)
>>>>> if (!cwg)
>>>>> pr_warn("No Cache Writeback Granule information, assuming
>>>>> cache line size %d\n",
>>>>> cls);
>>>>> - if (L1_CACHE_BYTES < cls)
>>>>> - pr_warn("L1_CACHE_BYTES smaller than the Cache Writeback Granule (%d < %d)\n",
>>>>> - L1_CACHE_BYTES, cls);
>>>>> + if (ARCH_DMA_MINALIGN < cls)
>>>>> + pr_warn("ARCH_DMA_MINALIGN smaller than the Cache Writeback Granule (%d < %d)\n",
>>>>> + ARCH_DMA_MINALIGN, cls);
>>>>> }
>>>>>
>>>>> static bool __maybe_unused
>>>>
>>>> This change was discussed at: [1] but was not concluded as apparently no one
>>>> came back with test report and numbers. After including this change in our
>>>> local kernel we are seeing significant throughput improvement. For example with:
>>>>
>>>> iperf -c 192.168.1.181 -i 1 -w 128K -t 60
>>>>
>>>> The average throughput is improving by about 30% (230Mbps from 180Mbps).
>>>> Could you please let us know if this change can be included in upstream kernel.
>>>>
>>>> [1]: https://groups.google.com/forum/#!topic/linux.kernel/P40yDB90ePs
>>>
>>> Could you please provide some feedback about the above mentioned query ?
>>
>> Do you have an explanation on the performance variation when
>> L1_CACHE_BYTES is changed? We'd need to understand how the network stack
>> is affected by L1_CACHE_BYTES, in which context it uses it (is it for
>> non-coherent DMA?).
>
> network stack use SKB_DATA_ALIGN to align.
> ---
> #define SKB_DATA_ALIGN(X) (((X) + (SMP_CACHE_BYTES - 1)) & \
> ~(SMP_CACHE_BYTES - 1))
>
> #define SMP_CACHE_BYTES L1_CACHE_BYTES
> ---
> I think this is the reason of performance regression.
>
Yes this is the reason for performance regression. Due to increases L1 cache alignment the
object is coming from next kmalloc slab and skb->truesize is changing from 2304 bytes to
4352 bytes. This in turn increases sk_wmem_alloc which causes queuing of less send buffers.
>>
>> The Cavium guys haven't shown any numbers (IIUC) to back the
>> L1_CACHE_BYTES performance improvement but I would not revert the
>> original commit since ARCH_DMA_MINALIGN definitely needs to cover the
>> maximum available cache line size, which is 128 for them.
>
> how about define L1_CACHE_SHIFT like below:
> ---
> #ifdef CONFIG_ARM64_L1_CACHE_SHIFT
> #define L1_CACHE_SHIFT CONFIG_ARM64_L1_CACHE_SHIFT
> #else
> #define L1_CACHE_SHIFT 7
> endif
> ---
>
> Thanks
>
>>
>> --
>> Catalin
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a\nmember of the Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* [RFC PATCH 1/7] iommu/arm-smmu-v3: Introduce smmu option PAGE0_REGS_ONLY for Silicon errata.
From: Linu Cherian @ 2017-04-12 5:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <d89ecbd2-ed1b-b13b-a7e4-ff2478655203@arm.com>
On Tue, Apr 11, 2017 at 9:12 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 11/04/17 15:42, linucherian at gmail.com wrote:
>> From: Linu Cherian <linu.cherian@cavium.com>
>>
>> Cavium 99xx SMMU implementation doesn't support page 1 register space
>> and PAGE0_REGS_ONLY option will be enabled as an errata workaround.
>
> Ugh :(
>
>> This option when turned on, replaces all page 1 offsets used for
>> EVTQ_PROD/CONS, PRIQ_PROD/CONS register access with page 0 offsets.
>
> I think it might be neater to have something like:
>
> arm_smmu_page1(smmu) {
> if (smmu->quirk)
> return smmu->base;
> return smmu->base + 64k;
> }
>
> and use it as the base in the appropriate places, rather than override
> the individual registers. Much like ARM_SMMU_GR0_NS in the SMMUv2 driver.
>
IIUC, we need to change the offsets as well for this,
Like,
#define ARM_SMMU_EVTQ_BASE 0xa0
-#define ARM_SMMU_EVTQ_PROD 0x100a8
-#define ARM_SMMU_EVTQ_CONS 0x100ac
+#define ARM_SMMU_EVTQ_PROD 0xa8
+#define ARM_SMMU_EVTQ_CONS 0xac
#define ARM_SMMU_EVTQ_IRQ_CFG0 0xb0
#define ARM_SMMU_EVTQ_IRQ_CFG1 0xb8
#define ARM_SMMU_EVTQ_IRQ_CFG2 0xbc
#define ARM_SMMU_PRIQ_BASE 0xc0
-#define ARM_SMMU_PRIQ_PROD 0x100c8
-#define ARM_SMMU_PRIQ_CONS 0x100cc
+#define ARM_SMMU_PRIQ_PROD 0xc8
+#define ARM_SMMU_PRIQ_CONS 0xcc
But, it appears difficult to take this approach, at least with
arm_smmu_init_one_queue function.
This function takes both page0 register offset and and page1 register
offset as an argument.
So, we might need to do additional checks in this function to decide,
whether to use
a pag0 base or page1 base.
Thanks.
^ permalink raw reply
* [RFC PATCH 3/3] cpufreq: imx6q: refine clk operations
From: Dong Aisheng @ 2017-04-12 4:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491969809-20154-1-git-send-email-aisheng.dong@nxp.com>
Use clk_bulk_get to ease the driver clocks handling.
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Anson Huang <anson.huang@nxp.com>
Cc: Robin Gong <yibin.gong@nxp.com>
Cc: Bai Ping <ping.bai@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: Octavian Purdila <octavian.purdila@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
drivers/cpufreq/imx6q-cpufreq.c | 119 ++++++++++++++++++----------------------
1 file changed, 53 insertions(+), 66 deletions(-)
diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index 7719b02..6158910 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -24,15 +24,29 @@ static struct regulator *arm_reg;
static struct regulator *pu_reg;
static struct regulator *soc_reg;
-static struct clk *arm_clk;
-static struct clk *pll1_sys_clk;
-static struct clk *pll1_sw_clk;
-static struct clk *step_clk;
-static struct clk *pll2_pfd2_396m_clk;
-
-/* clk used by i.MX6UL */
-static struct clk *pll2_bus_clk;
-static struct clk *secondary_sel_clk;
+enum IMX6_CPUFREQ_CLKS {
+ ARM,
+ PLL1_SYS,
+ STEP,
+ PLL1_SW,
+ PLL2_PFD2_396M,
+ /* MX6UL requires two more clks */
+ PLL2_BUS,
+ SECONDARY_SEL,
+};
+#define IMX6Q_CPUFREQ_CLK_NUM 5
+#define IMX6UL_CPUFREQ_CLK_NUM 7
+
+static int num_clks;
+static struct clk_bulk_data clks[] = {
+ { .id = "arm" },
+ { .id = "pll1_sys" },
+ { .id = "step" },
+ { .id = "pll1_sw" },
+ { .id = "pll2_pfd2_396m" },
+ { .id = "pll2_bus" },
+ { .id = "secondary_sel" },
+};
static struct device *cpu_dev;
static bool free_opp;
@@ -51,7 +65,7 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index)
new_freq = freq_table[index].frequency;
freq_hz = new_freq * 1000;
- old_freq = clk_get_rate(arm_clk) / 1000;
+ old_freq = clk_get_rate(clks[ARM].clk) / 1000;
opp = dev_pm_opp_find_freq_ceil(cpu_dev, &freq_hz);
if (IS_ERR(opp)) {
@@ -109,25 +123,27 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index)
* voltage of 528MHz, so lower the CPU frequency to one
* half before changing CPU frequency.
*/
- clk_set_rate(arm_clk, (old_freq >> 1) * 1000);
- clk_set_parent(pll1_sw_clk, pll1_sys_clk);
- if (freq_hz > clk_get_rate(pll2_pfd2_396m_clk))
- clk_set_parent(secondary_sel_clk, pll2_bus_clk);
+ clk_set_rate(clks[ARM].clk, (old_freq >> 1) * 1000);
+ clk_set_parent(clks[PLL1_SW].clk, clks[PLL1_SYS].clk);
+ if (freq_hz > clk_get_rate(clks[PLL2_PFD2_396M].clk))
+ clk_set_parent(clks[SECONDARY_SEL].clk,
+ clks[PLL2_BUS].clk);
else
- clk_set_parent(secondary_sel_clk, pll2_pfd2_396m_clk);
- clk_set_parent(step_clk, secondary_sel_clk);
- clk_set_parent(pll1_sw_clk, step_clk);
+ clk_set_parent(clks[SECONDARY_SEL].clk,
+ clks[PLL2_PFD2_396M].clk);
+ clk_set_parent(clks[STEP].clk, clks[SECONDARY_SEL].clk);
+ clk_set_parent(clks[PLL1_SW].clk, clks[STEP].clk);
} else {
- clk_set_parent(step_clk, pll2_pfd2_396m_clk);
- clk_set_parent(pll1_sw_clk, step_clk);
- if (freq_hz > clk_get_rate(pll2_pfd2_396m_clk)) {
- clk_set_rate(pll1_sys_clk, new_freq * 1000);
- clk_set_parent(pll1_sw_clk, pll1_sys_clk);
+ clk_set_parent(clks[STEP].clk, clks[PLL2_PFD2_396M].clk);
+ clk_set_parent(clks[PLL1_SW].clk, clks[STEP].clk);
+ if (freq_hz > clk_get_rate(clks[PLL2_PFD2_396M].clk)) {
+ clk_set_rate(clks[PLL1_SYS].clk, new_freq * 1000);
+ clk_set_parent(clks[PLL1_SW].clk, clks[PLL1_SYS].clk);
}
}
/* Ensure the arm clock divider is what we expect */
- ret = clk_set_rate(arm_clk, new_freq * 1000);
+ ret = clk_set_rate(clks[ARM].clk, new_freq * 1000);
if (ret) {
dev_err(cpu_dev, "failed to set clock rate: %d\n", ret);
regulator_set_voltage_tol(arm_reg, volt_old, 0);
@@ -161,7 +177,7 @@ static int imx6q_set_target(struct cpufreq_policy *policy, unsigned int index)
static int imx6q_cpufreq_init(struct cpufreq_policy *policy)
{
- policy->clk = arm_clk;
+ policy->clk = clks[ARM].clk;
return cpufreq_generic_init(policy, freq_table, transition_latency);
}
@@ -197,27 +213,14 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev)
return -ENOENT;
}
- arm_clk = clk_get(cpu_dev, "arm");
- pll1_sys_clk = clk_get(cpu_dev, "pll1_sys");
- pll1_sw_clk = clk_get(cpu_dev, "pll1_sw");
- step_clk = clk_get(cpu_dev, "step");
- pll2_pfd2_396m_clk = clk_get(cpu_dev, "pll2_pfd2_396m");
- if (IS_ERR(arm_clk) || IS_ERR(pll1_sys_clk) || IS_ERR(pll1_sw_clk) ||
- IS_ERR(step_clk) || IS_ERR(pll2_pfd2_396m_clk)) {
- dev_err(cpu_dev, "failed to get clocks\n");
- ret = -ENOENT;
- goto put_clk;
- }
+ if (of_machine_is_compatible("fsl,imx6ul"))
+ num_clks = IMX6UL_CPUFREQ_CLK_NUM;
+ else
+ num_clks = IMX6Q_CPUFREQ_CLK_NUM;
- if (of_machine_is_compatible("fsl,imx6ul")) {
- pll2_bus_clk = clk_get(cpu_dev, "pll2_bus");
- secondary_sel_clk = clk_get(cpu_dev, "secondary_sel");
- if (IS_ERR(pll2_bus_clk) || IS_ERR(secondary_sel_clk)) {
- dev_err(cpu_dev, "failed to get clocks specific to imx6ul\n");
- ret = -ENOENT;
- goto put_clk;
- }
- }
+ ret = clk_bulk_get(cpu_dev, num_clks, clks);
+ if (ret)
+ goto put_node;
arm_reg = regulator_get(cpu_dev, "arm");
pu_reg = regulator_get_optional(cpu_dev, "pu");
@@ -354,22 +357,11 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev)
regulator_put(pu_reg);
if (!IS_ERR(soc_reg))
regulator_put(soc_reg);
-put_clk:
- if (!IS_ERR(arm_clk))
- clk_put(arm_clk);
- if (!IS_ERR(pll1_sys_clk))
- clk_put(pll1_sys_clk);
- if (!IS_ERR(pll1_sw_clk))
- clk_put(pll1_sw_clk);
- if (!IS_ERR(step_clk))
- clk_put(step_clk);
- if (!IS_ERR(pll2_pfd2_396m_clk))
- clk_put(pll2_pfd2_396m_clk);
- if (!IS_ERR(pll2_bus_clk))
- clk_put(pll2_bus_clk);
- if (!IS_ERR(secondary_sel_clk))
- clk_put(secondary_sel_clk);
+
+ clk_bulk_put(num_clks, clks);
+put_node:
of_node_put(np);
+
return ret;
}
@@ -383,13 +375,8 @@ static int imx6q_cpufreq_remove(struct platform_device *pdev)
if (!IS_ERR(pu_reg))
regulator_put(pu_reg);
regulator_put(soc_reg);
- clk_put(arm_clk);
- clk_put(pll1_sys_clk);
- clk_put(pll1_sw_clk);
- clk_put(step_clk);
- clk_put(pll2_pfd2_396m_clk);
- clk_put(pll2_bus_clk);
- clk_put(secondary_sel_clk);
+
+ clk_bulk_put(num_clks, clks);
return 0;
}
--
2.7.4
^ permalink raw reply related
* [RFC PATCH 2/3] clk: add managed version of clk_bulk_get
From: Dong Aisheng @ 2017-04-12 4:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491969809-20154-1-git-send-email-aisheng.dong@nxp.com>
This patch introduces the managed version of clk_bulk_get.
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Anson Huang <anson.huang@nxp.com>
Cc: Robin Gong <yibin.gong@nxp.com>
Cc: Bai Ping <ping.bai@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: Octavian Purdila <octavian.purdila@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
drivers/clk/clk-devres.c | 36 ++++++++++++++++++++++++++++++++++++
include/linux/clk.h | 22 +++++++++++++++++++++-
2 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
index 3a218c3..c7fb31d 100644
--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c
@@ -34,6 +34,42 @@ struct clk *devm_clk_get(struct device *dev, const char *id)
}
EXPORT_SYMBOL(devm_clk_get);
+struct clk_bulk_devres {
+ struct clk_bulk_data *clks;
+ int num_clks;
+};
+
+static void devm_clk_bulk_release(struct device *dev, void *res)
+{
+ struct clk_bulk_devres *devres = res;
+
+ clk_bulk_put(devres->num_clks, devres->clks);
+}
+
+int devm_clk_bulk_get(struct device *dev, int num_clks,
+ struct clk_bulk_data *clks)
+{
+ struct clk_bulk_devres *devres;
+ int ret;
+
+ devres = devres_alloc(devm_clk_bulk_release,
+ sizeof(*devres), GFP_KERNEL);
+ if (!devres)
+ return -ENOMEM;
+
+ ret = clk_bulk_get(dev, num_clks, clks);
+ if (!ret) {
+ devres->clks = clks;
+ devres->num_clks = num_clks;
+ devres_add(dev, devres);
+ } else {
+ devres_free(devres);
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(devm_clk_bulk_get);
+
static int devm_clk_match(struct device *dev, void *res, void *data)
{
struct clk **c = res;
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 1d05b66..3fc6010 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -278,11 +278,25 @@ struct clk *clk_get(struct device *dev, const char *id);
*
* clk_bulk_get should not be called from within interrupt context.
*/
-
int __must_check clk_bulk_get(struct device *dev, int num_clks,
struct clk_bulk_data *clks);
/**
+ * devm_clk_bulk_get - managed get multiple clk consumers
+ * @dev: device for clock "consumer"
+ * @num_clks: the number of clk_bulk_data
+ * @clks: the clk_bulk_data table of consumer
+ *
+ * Return 0 on success, an errno on failure.
+ *
+ * This helper function allows drivers to get several regulator
+ * consumers in one operation with management, the clks will
+ * automatically be freed when the device is unbound.
+ */
+int __must_check devm_clk_bulk_get(struct device *dev, int num_clks,
+ struct clk_bulk_data *clks);
+
+/**
* devm_clk_get - lookup and obtain a managed reference to a clock producer.
* @dev: device for clock "consumer"
* @id: clock consumer ID
@@ -554,6 +568,12 @@ static inline struct clk *devm_clk_get(struct device *dev, const char *id)
return NULL;
}
+static inline int devm_clk_bulk_get(struct device *dev, int num_clks,
+ struct clk_bulk_data *clks)
+{
+ return NULL;
+}
+
static inline struct clk *devm_get_clk_from_child(struct device *dev,
struct device_node *np, const char *con_id)
{
--
2.7.4
^ permalink raw reply related
* [RFC PATCH 1/3] clk: add clk_bulk_get accessories
From: Dong Aisheng @ 2017-04-12 4:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491969809-20154-1-git-send-email-aisheng.dong@nxp.com>
These helper function allows drivers to get several clk consumers in
one operation. If any of the clk cannot be acquired then any clks
that were got will be put before returning to the caller.
This can relieve the driver owners' life who needs to handle many clocks,
as well as each clock error reporting.
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Anson Huang <anson.huang@nxp.com>
Cc: Robin Gong <yibin.gong@nxp.com>
Cc: Bai Ping <ping.bai@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: Octavian Purdila <octavian.purdila@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
drivers/clk/clk.c | 99 ++++++++++++++++++++++++++++++++++++++++++++
drivers/clk/clkdev.c | 42 +++++++++++++++++++
include/linux/clk.h | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 256 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index cddddbe..622f8069 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -520,6 +520,23 @@ void clk_unprepare(struct clk *clk)
}
EXPORT_SYMBOL_GPL(clk_unprepare);
+/**
+ * clk_bulk_unprepare - undo preparation of a bulk of clock sources
+ * @num_clks: the number of clk_bulk_data
+ * @clks: the clk_bulk_data table being ungated
+ *
+ * clk_bulk_unprepare may sleep, which differentiates it from clk_bulk_disable.
+ * Returns 0 on success, -EERROR otherwise.
+ */
+void clk_bulk_unprepare(int num_clks, struct clk_bulk_data *clks)
+{
+ int i;
+
+ for (i = 0; i < num_clks; i++)
+ clk_unprepare(clks[i].clk);
+}
+EXPORT_SYMBOL_GPL(clk_bulk_unprepare);
+
static int clk_core_prepare(struct clk_core *core)
{
int ret = 0;
@@ -584,6 +601,38 @@ int clk_prepare(struct clk *clk)
}
EXPORT_SYMBOL_GPL(clk_prepare);
+/**
+ * clk_bulk_prepare - prepare a bulk of clocks
+ * @num_clks: the number of clk_bulk_data
+ * @clks: the clk_bulk_data table being ungated
+ *
+ * clk_bulk_prepare may sleep, which differentiates it from clk_bulk_enable.
+ * Returns 0 on success, -EERROR otherwise.
+ */
+
+int __must_check clk_bulk_prepare(int num_clks, struct clk_bulk_data *clks)
+{
+ int ret;
+ int i;
+
+ for (i = 0; i < num_clks; i++) {
+ ret = clk_prepare(clks[i].clk);
+ if (ret) {
+ pr_err("Failed to prepare clk '%s': %d\n",
+ clks[i].id, ret);
+ goto err;
+ }
+ }
+
+ return 0;
+
+err:
+ while (--i >= 0)
+ clk_unprepare(clks[i].clk);
+
+ return ret;
+}
+
static void clk_core_disable(struct clk_core *core)
{
lockdep_assert_held(&enable_lock);
@@ -640,6 +689,24 @@ void clk_disable(struct clk *clk)
}
EXPORT_SYMBOL_GPL(clk_disable);
+/**
+ * clk_bulk_disable - gate a bulk of clocks
+ * @num_clks: the number of clk_bulk_data
+ * @clks: the clk_bulk_data table being ungated
+ *
+ * clk_bulk_disable must not sleep, which differentiates it from
+ * clk_bulk_unprepare. clk_bulk_disable must be called before
+ * clk_bulk_unprepare.
+ */
+void clk_bulk_disable(int num_clks, struct clk_bulk_data *clks)
+{
+ int i;
+
+ for (i = 0; i < num_clks; i++)
+ clk_disable(clks[i].clk);
+}
+EXPORT_SYMBOL_GPL(clk_bulk_disable);
+
static int clk_core_enable(struct clk_core *core)
{
int ret = 0;
@@ -709,6 +776,38 @@ int clk_enable(struct clk *clk)
}
EXPORT_SYMBOL_GPL(clk_enable);
+/**
+ * clk_bulk_enable - ungate a bulk of clocks
+ * @num_clks: the number of clk_bulk_data
+ * @clks: the clk_bulk_data table being ungated
+ *
+ * clk_bulk_enable must not sleep
+ * Returns 0 on success, -EERROR otherwise.
+ */
+int clk_bulk_enable(int num_clks, struct clk_bulk_data *clks)
+{
+ int ret;
+ int i;
+
+ for (i = 0; i < num_clks; i++) {
+ ret = clk_enable(clks[i].clk);
+ if (ret) {
+ pr_err("Failed to enable clk '%s': %d\n",
+ clks[i].id, ret);
+ goto err;
+ }
+ }
+
+ return 0;
+
+err:
+ while (--i >= 0)
+ clk_put(clks[i].clk);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(clk_bulk_enable);
+
static int clk_core_prepare_enable(struct clk_core *core)
{
int ret;
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index bb8a77a..141f2e6 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -209,12 +209,54 @@ struct clk *clk_get(struct device *dev, const char *con_id)
}
EXPORT_SYMBOL(clk_get);
+int clk_bulk_get(struct device *dev, int num_clks,
+ struct clk_bulk_data *clks)
+{
+ int ret;
+ int i;
+
+ for (i = 0; i < num_clks; i++)
+ clks[i].clk = NULL;
+
+ for (i = 0; i < num_clks; i++) {
+ clks[i].clk = clk_get(dev, clks[i].id);
+ if (IS_ERR(clks[i].clk)) {
+ ret = PTR_ERR(clks[i].clk);
+ dev_err(dev, "Failed to get clk '%s': %d\n",
+ clks[i].id, ret);
+ clks[i].clk = NULL;
+ goto err;
+ }
+
+ }
+
+ return 0;
+
+err:
+ while (--i >= 0)
+ clk_put(clks[i].clk);
+
+ return ret;
+}
+EXPORT_SYMBOL(clk_bulk_get);
+
void clk_put(struct clk *clk)
{
__clk_put(clk);
}
EXPORT_SYMBOL(clk_put);
+void clk_bulk_put(int num_clks, struct clk_bulk_data *clks)
+{
+ int i;
+
+ for (i = 0; i < num_clks; i++) {
+ clk_put(clks[i].clk);
+ clks[i].clk = NULL;
+ }
+}
+EXPORT_SYMBOL_GPL(clk_bulk_put);
+
static void __clkdev_add(struct clk_lookup *cl)
{
mutex_lock(&clocks_mutex);
diff --git a/include/linux/clk.h b/include/linux/clk.h
index e9d36b3..1d05b66 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -77,6 +77,21 @@ struct clk_notifier_data {
unsigned long new_rate;
};
+/**
+ * struct clk_bulk_data - Data used for bulk clk operations.
+ *
+ * @id: clock consumer ID
+ * @clk: struct clk * to store the associated clock
+ *
+ * The CLK APIs provide a series of clk_bulk_() API calls as
+ * a convenience to consumers which require multiple clks. This
+ * structure is used to manage data for these calls.
+ */
+struct clk_bulk_data {
+ const char *id;
+ struct clk *clk;
+};
+
#ifdef CONFIG_COMMON_CLK
/**
@@ -185,12 +200,19 @@ static inline bool clk_is_match(const struct clk *p, const struct clk *q)
*/
#ifdef CONFIG_HAVE_CLK_PREPARE
int clk_prepare(struct clk *clk);
+int __must_check clk_bulk_prepare(int num_clks, struct clk_bulk_data *clks);
#else
static inline int clk_prepare(struct clk *clk)
{
might_sleep();
return 0;
}
+
+static inline int clk_bulk_prepare(int num_clks, struct clk_bulk_data *clks)
+{
+ might_sleep();
+ return 0;
+}
#endif
/**
@@ -204,11 +226,16 @@ static inline int clk_prepare(struct clk *clk)
*/
#ifdef CONFIG_HAVE_CLK_PREPARE
void clk_unprepare(struct clk *clk);
+void clk_bulk_unprepare(int num_clks, struct clk_bulk_data *clks);
#else
static inline void clk_unprepare(struct clk *clk)
{
might_sleep();
}
+static inline void clk_bulk_unprepare(int num_clks, struct clk_bulk_data *clks)
+{
+ might_sleep();
+}
#endif
#ifdef CONFIG_HAVE_CLK
@@ -230,6 +257,32 @@ static inline void clk_unprepare(struct clk *clk)
struct clk *clk_get(struct device *dev, const char *id);
/**
+ * clk_bulk_get - lookup and obtain a number of references to clock producer.
+ * @dev: device for clock "consumer"
+ * @num_clks: the number of clk_bulk_data
+ * @clks: the clk_bulk_data table of consumer
+ *
+ * This helper function allows drivers to get several clk consumers in one
+ * operation. If any of the clk cannot be acquired then any clks
+ * that were obtained will be freed before returning to the caller.
+ *
+ * Returns 0 if all clocks specified in clk_bulk_data table are obtained
+ * successfully, or valid IS_ERR() condition containing errno.
+ * The implementation uses @dev and @clk_bulk_data.id to determine the
+ * clock consumer, and thereby the clock producer.
+ * (IOW, @id may be identical strings, but clk_get may return different
+ * clock producers depending on @dev.) The clock returned is stored in
+ * each @clk_bulk_data.clk field.
+ *
+ * Drivers must assume that the clock source is not enabled.
+ *
+ * clk_bulk_get should not be called from within interrupt context.
+ */
+
+int __must_check clk_bulk_get(struct device *dev, int num_clks,
+ struct clk_bulk_data *clks);
+
+/**
* devm_clk_get - lookup and obtain a managed reference to a clock producer.
* @dev: device for clock "consumer"
* @id: clock consumer ID
@@ -279,6 +332,20 @@ struct clk *devm_get_clk_from_child(struct device *dev,
int clk_enable(struct clk *clk);
/**
+ * clk_bulk_enable - inform the system when the bulk of clock source should
+ * be running.
+ * @num_clks: the number of clk_bulk_data
+ * @clks: the clk_bulk_data table of consumer
+ *
+ * If the clock can not be enabled/disabled all, this should return success.
+ *
+ * May be called from atomic contexts.
+ *
+ * Returns success (0) or negative errno.
+ */
+int __must_check clk_bulk_enable(int num_clks, struct clk_bulk_data *clks);
+
+/**
* clk_disable - inform the system when the clock source is no longer required.
* @clk: clock source
*
@@ -295,6 +362,24 @@ int clk_enable(struct clk *clk);
void clk_disable(struct clk *clk);
/**
+ * clk_bulk_disable - inform the system when the bulk of clock source is no
+ * longer required.
+ * @num_clks: the number of clk_bulk_data
+ * @clks: the clk_bulk_data table of consumer
+ *
+ * Inform the system that a bulk of clock source is no longer required by
+ * a driver and may be shut down.
+ *
+ * May be called from atomic contexts.
+ *
+ * Implementation detail: if the bulk of clock source is shared between
+ * multiple drivers, clk_bulk_enable() calls must be balanced by the
+ * same number of clk_bulk_disable() calls for the clock source to be
+ * disabled.
+ */
+void clk_bulk_disable(int num_clks, struct clk_bulk_data *clks);
+
+/**
* clk_get_rate - obtain the current clock rate (in Hz) for a clock source.
* This is only valid once the clock source has been enabled.
* @clk: clock source
@@ -314,6 +399,19 @@ unsigned long clk_get_rate(struct clk *clk);
void clk_put(struct clk *clk);
/**
+ * clk_bulk_put - "free" the clock source
+ * @num_clks: the number of clk_bulk_data
+ * @clks: the clk_bulk_data table of consumer
+ *
+ * Note: drivers must ensure that all clk_bulk_enable calls made on this
+ * clock source are balanced by clk_bulk_disable calls prior to calling
+ * this function.
+ *
+ * clk_bulk_put should not be called from within interrupt context.
+ */
+void clk_bulk_put(int num_clks, struct clk_bulk_data *clks);
+
+/**
* devm_clk_put - "free" a managed clock source
* @dev: device used to acquire the clock
* @clk: clock source acquired with devm_clk_get()
@@ -445,6 +543,12 @@ static inline struct clk *clk_get(struct device *dev, const char *id)
return NULL;
}
+static inline int clk_bulk_get(struct device *dev, int num_clks,
+ struct clk_bulk_data *clks)
+{
+ return NULL;
+}
+
static inline struct clk *devm_clk_get(struct device *dev, const char *id)
{
return NULL;
@@ -458,6 +562,8 @@ static inline struct clk *devm_get_clk_from_child(struct device *dev,
static inline void clk_put(struct clk *clk) {}
+static inline void clk_bulk_put(int num_clks, struct clk_bulk_data *clks) {}
+
static inline void devm_clk_put(struct device *dev, struct clk *clk) {}
static inline int clk_enable(struct clk *clk)
@@ -465,8 +571,17 @@ static inline int clk_enable(struct clk *clk)
return 0;
}
+static inline int clk_bulk_enable(int num_clks, struct clk_bulk_data *clks)
+{
+ return 0;
+}
+
static inline void clk_disable(struct clk *clk) {}
+
+static inline void clk_bulk_disable(int num_clks,
+ struct clk_bulk_data *clks) {}
+
static inline unsigned long clk_get_rate(struct clk *clk)
{
return 0;
--
2.7.4
^ permalink raw reply related
* [RFC PATCH 0/3] clk: introduce clk_bulk_get accessories
From: Dong Aisheng @ 2017-04-12 4:03 UTC (permalink / raw)
To: linux-arm-kernel
These helper function allows drivers to get several clk consumers in
one operation. If any of the clk cannot be acquired then any clks
that were got will be put before returning to the caller.
The idea firstly came out when i wanted to clean up and optimize
imx6q-cpufreq driver which needs to handle a lot of clocks as it
becomes a bit mess.
e.g. drivers/cpufreq/imx6q-cpufreq.c
You will see we need get 7 clocks during driver probe.
(Add there will be more, e.g. pll1, pll1_bypass, pll1_bypass_src,
in the future when adding busfreq support).
static int imx6q_cpufreq_probe(struct platform_device *pdev)
{
....
arm_clk = clk_get(cpu_dev, "arm");
pll1_sys_clk = clk_get(cpu_dev, "pll1_sys");
pll1_sw_clk = clk_get(cpu_dev, "pll1_sw");
step_clk = clk_get(cpu_dev, "step");
pll2_pfd2_396m_clk = clk_get(cpu_dev, "pll2_pfd2_396m");
if (IS_ERR(arm_clk) || IS_ERR(pll1_sys_clk) || IS_ERR(pll1_sw_clk) ||
IS_ERR(step_clk) || IS_ERR(pll2_pfd2_396m_clk)) {
dev_err(cpu_dev, "failed to get clocks\n");
ret = -ENOENT;
goto put_clk;
}
if (of_machine_is_compatible("fsl,imx6ul")) {
pll2_bus_clk = clk_get(cpu_dev, "pll2_bus");
secondary_sel_clk = clk_get(cpu_dev, "secondary_sel");
if (IS_ERR(pll2_bus_clk) || IS_ERR(secondary_sel_clk)) {
dev_err(cpu_dev, "failed to get clocks specific to imx6ul\n");
ret = -ENOENT;
goto put_clk;
}
}
....
put_clk:
if (!IS_ERR(arm_clk))
clk_put(arm_clk);
if (!IS_ERR(pll1_sys_clk))
clk_put(pll1_sys_clk);
...........
}
Together with the err path handling for each clocks, it does make
things a bit ugly.
Since we already have regulator_bulk_get accessories, i thought we
probably could introduce clk_bulk_get as well to handle such case to
ease the driver owners' life.
Besides IMX cpufreq driver, there is also some similar cases
in kernel which could befinit from this api as well.
e.g.
drivers/cpufreq/tegra124-cpufreq.c
drivers/cpufreq/s3c2412-cpufreq.c
sound/soc/samsung/smdk_spdif.c
arch/arm/mach-omap1/serial.c
...
And actually, if we handle clocks more than 3, then it might be
worthy to try, which there is quite many manay in kernel and
that probably could save a lot codes.
This is a RFC patch intending to bring up the idea to discuss.
Comments are welcome and appreciated!
Dong Aisheng (3):
clk: add clk_bulk_get accessories
clk: add managed version of clk_bulk_get
cpufreq: imx6q: refine clk operations
drivers/clk/clk-devres.c | 36 +++++++++++
drivers/clk/clk.c | 99 +++++++++++++++++++++++++++++
drivers/clk/clkdev.c | 42 +++++++++++++
drivers/cpufreq/imx6q-cpufreq.c | 119 ++++++++++++++++-------------------
include/linux/clk.h | 135 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 365 insertions(+), 66 deletions(-)
--
2.7.4
^ permalink raw reply
* [GIT PULL] ARM: aspeed: devicetree for 4.12
From: Joel Stanley @ 2017-04-12 2:59 UTC (permalink / raw)
To: linux-arm-kernel
Hello Arnd, Olof,
These are the Aspeed devicetree changes for 4.12.
The following changes since commit c1ae3cfa0e89fa1a7ecc4c99031f5e9ae99d9201:
Linux 4.11-rc1 (2017-03-05 12:59:56 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/joel/aspeed.git
tags/aspeed-4.12-devicetree
for you to fetch changes up to 78a2569fa60497347babeddb24a367946e258283:
arm: dts: aspeed: Describe ADCs for AST2400/AST2500 (2017-04-07
13:09:47 +0930)
----------------------------------------------------------------
Aspeed devicetree updates for 4.12
Updates to the device tree to include upstreamed drivers:
- SPI flash controller
- Watchdog
- ADC
In addition we describe some of the clocks so that upstream kernels can
boot on hardware.
----------------------------------------------------------------
C?dric Le Goater (3):
ARM: dts: aspeed: add SPI controller bindings
ARM: dts: aspeed: Add SPI controller bindings to Romulus
ARM: dts: aspeed: Add a fastread property
Joel Stanley (3):
ARM: dts: aspeed: Make G5 clocks fixed
ARM: dts: aspeed: Make G4 clocks fixed
ARM: dts: aspeed: Update watchdog compatible strings
Lei YU (1):
ARM: dts: aspeed: romulus: Add UART1
Rick Altherr (1):
arm: dts: aspeed: Describe ADCs for AST2400/AST2500
arch/arm/boot/dts/aspeed-ast2500-evb.dts | 22 ++++
arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dts | 18 +++
arch/arm/boot/dts/aspeed-bmc-opp-romulus.dts | 36 ++++++
arch/arm/boot/dts/aspeed-g4.dtsi | 109 +++++++++++++-----
arch/arm/boot/dts/aspeed-g5.dtsi | 156 +++++++++++++++++++-------
5 files changed, 271 insertions(+), 70 deletions(-)
^ permalink raw reply
* [GIT PULL] ARM: aspeed: defconfig for 4.12
From: Joel Stanley @ 2017-04-12 2:59 UTC (permalink / raw)
To: linux-arm-kernel
Hello Arnd, Olof,
These are the Aspeed defconfig changes for 4.12.
The following changes since commit c1ae3cfa0e89fa1a7ecc4c99031f5e9ae99d9201:
Linux 4.11-rc1 (2017-03-05 12:59:56 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/joel/aspeed.git
tags/aspeed-4.12-defconfig
for you to fetch changes up to 97bae47812dbbd220e354bc9f15c3b3c8b481a86:
ARM: configs: aspeed: Add new drivers (2017-04-07 15:29:50 +0930)
----------------------------------------------------------------
Aspeed defconfig updates for 4.12
Add newly unstreamed drivers to the defconfigs for the Aspeed SoCs.
The defconfigs are reworked to be useful for development and testing
of OpenBMC on these systems.
----------------------------------------------------------------
C?dric Le Goater (1):
ARM: aspeed: Add CONFIG_SPI_ASPEED_SMC
Joel Stanley (2):
ARM: configs: aspeed: Update configs for BMC systems
ARM: configs: aspeed: Add new drivers
arch/arm/configs/aspeed_g4_defconfig | 112 ++++++++++++++++++++++++++++-------
arch/arm/configs/aspeed_g5_defconfig | 111 ++++++++++++++++++++++++++--------
2 files changed, 175 insertions(+), 48 deletions(-)
^ permalink raw reply
* [PATCH 14/14] arm64: pmuv3: use arm_pmu ACPI framework
From: Hanjun Guo @ 2017-04-12 2:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170314105108.GA15248@leverpostej>
On 2017/3/14 18:51, Mark Rutland wrote:
> Hi Ganapatrao,
>
> On Tue, Mar 14, 2017 at 11:30:17AM +0530, Ganapatrao Kulkarni wrote:
>>> +int __init armv8_pmu_driver_init(void)
>>> +{
>>> + if (acpi_disabled)
>>> + return platform_driver_register(&armv8_pmu_driver);
>>> + else
>>> + return arm_pmu_acpi_probe(armv8_pmuv3_init);
>> i think this function needs to be updated to
>> probe SoC specific init (armv8_a53_pmu_init, armv8_vulcan_pmu_init
>> etc) as well.
> Only exposing generic PMUv3 here was a deliberate decision, based on
> prior discussions, so as to keep things consistent on ACPI platforms
> regardless of which CPU types the PMU code is immediately aware of.
>
> I should have documented this better; sorry about that.
>
> All events which can be enumerated via the PMU ID registers will be
> exposed via sysfs.
>
> Other events (e.g. those which are implementation defined) will not be
> exposed via sysfs, but can still be used as raw events.
Hmm, this means inconsistency of pmu events for system booted with
ACPI and DT (DT may represent more events), is there any plan to support
events not in generic PMUv3 for further steps in the future?
Thanks
Hanjun
^ permalink raw reply
* [RFC PATCH 4/7] ACPICA: IORT: Add SMMuV3 model definitions.
From: Hanjun Guo @ 2017-04-12 2:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CA+sq2CfXQAM-ooNRQBbYNYc_T=p2yrvA2FYD=2_=_-Z9kQwVcA@mail.gmail.com>
On 2017/4/12 0:57, Sunil Kovvuri wrote:
> On Tue, Apr 11, 2017 at 9:29 PM, Robin Murphy <robin.murphy@arm.com> wrote:
>> On 11/04/17 15:42, linucherian at gmail.com wrote:
>>> From: Linu Cherian <linu.cherian@cavium.com>
>>>
>>> Add SMMuV3 model definitions.
>>>
>>> Signed-off-by: Linu Cherian <linu.cherian@cavium.com>
>>> ---
>>> include/acpi/actbl2.h | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>>> index 2b4af07..9db67d6 100644
>>> --- a/include/acpi/actbl2.h
>>> +++ b/include/acpi/actbl2.h
>>> @@ -778,6 +778,11 @@ struct acpi_iort_smmu {
>>> #define ACPI_IORT_SMMU_CORELINK_MMU400 0x00000002 /* ARM Corelink MMU-400 */
>>> #define ACPI_IORT_SMMU_CORELINK_MMU500 0x00000003 /* ARM Corelink MMU-500 */
>>>
>>> +#define ACPI_IORT_SMMU_V3 0x00000000 /* Generic SMMUv3 */
>>> +#define ACPI_IORT_SMMU_CORELINK_MMU600 0x00000001 /* ARM Corelink MMU-600 */
>>> +#define ACPI_IORT_SMMU_V3_HISILICON 0x00000002 /* HiSilicon SMMUv3 */
>>> +#define ACPI_IORT_SMMU_V3_CAVIUM_CN99XX 0x00000003 /* Cavium CN99xx SMMUv3 */
>>
>> None of those models are listed in the current IORT spec.
>
> As mentioned in the cover letter, we are in the process of getting
> model no added for
> our silicon in the soon to be published updated IORT spec. Meanwhile
> we wanted to take
> feedback on the errata patches from experts. Hence patches were
> submitted as RFC.
Thanks. The name for Hisilicon SMMUv3 might be changed, we need to wait
for the released IORT spec.
Thanks
Hanjun
^ permalink raw reply
* [PATCH 6/6] regulator: anatop: set default voltage selector for pcie
From: Dong Aisheng @ 2017-04-12 1:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491962327-12477-1-git-send-email-aisheng.dong@nxp.com>
Set the initial voltage selector for vddpcie in case it's disabled
by default.
This fixes the below warning:
20c8000.anatop:regulator-vddpcie: Failed to read a valid default voltage selector.
anatop_regulator: probe of 20c8000.anatop:regulator-vddpcie failed with error -22
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Robin Gong <yibin.gong@nxp.com>
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
drivers/regulator/anatop-regulator.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 6da0b20..910adfd 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -301,6 +301,11 @@ static int anatop_regulator_probe(struct platform_device *pdev)
if (!sreg->sel && rdesc->name && !strcmp(rdesc->name, "vddpu"))
sreg->sel = 22;
+ /* set the default voltage of the pcie phy to be 1.100v */
+ if (!sreg->sel && rdesc->name &&
+ !strcmp(rdesc->name, "vddpcie"))
+ sreg->sel = 0x10;
+
if (!sreg->bypass && !sreg->sel) {
dev_err(&pdev->dev, "Failed to read a valid default voltage selector.\n");
return -EINVAL;
--
2.7.4
^ permalink raw reply related
* [PATCH 5/6] regulator: anatop-regulator: make regulator-name using optionally
From: Dong Aisheng @ 2017-04-12 1:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491962327-12477-1-git-send-email-aisheng.dong@nxp.com>
rdesc->name/regulator-name is optional according to standard regulator
binding doc. Use it conditionally to avoid a kernel NULL point crash.
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Robin Gong <yibin.gong@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
drivers/regulator/anatop-regulator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 19eb1f4..6da0b20 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -298,7 +298,7 @@ static int anatop_regulator_probe(struct platform_device *pdev)
* a sane default until imx6-cpufreq was probed and changes the
* voltage to the correct value. In this case we set 1.25V.
*/
- if (!sreg->sel && !strcmp(rdesc->name, "vddpu"))
+ if (!sreg->sel && rdesc->name && !strcmp(rdesc->name, "vddpu"))
sreg->sel = 22;
if (!sreg->bypass && !sreg->sel) {
--
2.7.4
^ permalink raw reply related
* [PATCH 4/6] regulator: anatop: remove unneeded name field of struct anatop_regulator
From: Dong Aisheng @ 2017-04-12 1:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491962327-12477-1-git-send-email-aisheng.dong@nxp.com>
sreg->name is only used as an intermediate assign of rdesc->name, plus
another strcmp. Since we already have rdesc->name, no need it anymore.
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Robin Gong <yibin.gong@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
drivers/regulator/anatop-regulator.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 9481730..19eb1f4 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -39,7 +39,6 @@
#define LDO_FET_FULL_ON 0x1f
struct anatop_regulator {
- const char *name;
u32 control_reg;
struct regmap *anatop;
int vol_bit_shift;
@@ -194,12 +193,12 @@ static int anatop_regulator_probe(struct platform_device *pdev)
if (!sreg)
return -ENOMEM;
- of_property_read_string(np, "regulator-name", &sreg->name);
rdesc = &sreg->rdesc;
- rdesc->name = sreg->name;
rdesc->type = REGULATOR_VOLTAGE;
rdesc->owner = THIS_MODULE;
+ of_property_read_string(np, "regulator-name", &rdesc->name);
+
initdata = of_get_regulator_init_data(dev, np, rdesc);
if (!initdata)
return -ENOMEM;
@@ -299,7 +298,7 @@ static int anatop_regulator_probe(struct platform_device *pdev)
* a sane default until imx6-cpufreq was probed and changes the
* voltage to the correct value. In this case we set 1.25V.
*/
- if (!sreg->sel && !strcmp(sreg->name, "vddpu"))
+ if (!sreg->sel && !strcmp(rdesc->name, "vddpu"))
sreg->sel = 22;
if (!sreg->bypass && !sreg->sel) {
--
2.7.4
^ permalink raw reply related
* [PATCH 3/6] regulator: anatop: use of_property_read_string to read the name
From: Dong Aisheng @ 2017-04-12 1:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491962327-12477-1-git-send-email-aisheng.dong@nxp.com>
sreg->name is a string, so use a more proper api to read back the string
instead of of_get_property.
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Robin Gong <yibin.gong@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
drivers/regulator/anatop-regulator.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 2a97ada..9481730 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -193,7 +193,8 @@ static int anatop_regulator_probe(struct platform_device *pdev)
sreg = devm_kzalloc(dev, sizeof(*sreg), GFP_KERNEL);
if (!sreg)
return -ENOMEM;
- sreg->name = of_get_property(np, "regulator-name", NULL);
+
+ of_property_read_string(np, "regulator-name", &sreg->name);
rdesc = &sreg->rdesc;
rdesc->name = sreg->name;
rdesc->type = REGULATOR_VOLTAGE;
--
2.7.4
^ permalink raw reply related
* [PATCH 2/6] regulator: anatop: only set supply regulator when it actually exists
From: Dong Aisheng @ 2017-04-12 1:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1491962327-12477-1-git-send-email-aisheng.dong@nxp.com>
Mandatorily set the initdata->supply_regulator while it actually not
exist will cause regulator core to resolve supply each time whenever
a new regulator registered which is meaningless and waste CPU mips.
We can observe more than one hundred times of iteration of resolving
during a MX6Q SDB board booting up.
This patch adds the condition check for vin-supply to avoid the issue.
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Robin Gong <yibin.gong@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
drivers/regulator/anatop-regulator.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index 46b9c2c..2a97ada 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -203,7 +203,9 @@ static int anatop_regulator_probe(struct platform_device *pdev)
if (!initdata)
return -ENOMEM;
- initdata->supply_regulator = "vin";
+ if (of_find_property(np, "vin-supply", NULL))
+ initdata->supply_regulator = "vin";
+
sreg->initdata = initdata;
anatop_np = of_get_parent(np);
--
2.7.4
^ permalink raw reply related
* [PATCH 1/6] regulator: anatop: check return value of of_get_regulator_init_data
From: Dong Aisheng @ 2017-04-12 1:58 UTC (permalink / raw)
To: linux-arm-kernel
Should check the return value of of_get_regulator_init_data before
using it.
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Robin Gong <yibin.gong@nxp.com>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
drivers/regulator/anatop-regulator.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c
index b041f27..46b9c2c 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -200,6 +200,9 @@ static int anatop_regulator_probe(struct platform_device *pdev)
rdesc->owner = THIS_MODULE;
initdata = of_get_regulator_init_data(dev, np, rdesc);
+ if (!initdata)
+ return -ENOMEM;
+
initdata->supply_regulator = "vin";
sreg->initdata = initdata;
--
2.7.4
^ permalink raw reply related
* [PATCH v6 17/39] platform: add video-multiplexer subdevice driver
From: Steve Longerbeam @ 2017-04-12 0:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170404124732.GD3288@valkosipuli.retiisi.org.uk>
On 04/04/2017 05:47 AM, Sakari Ailus wrote:
> Hi Steve, Philipp and Pavel,
>
> On Mon, Mar 27, 2017 at 05:40:34PM -0700, Steve Longerbeam wrote:
>> From: Philipp Zabel <p.zabel@pengutronix.de>
>>
>> This driver can handle SoC internal and external video bus multiplexers,
>> controlled either by register bit fields or by a GPIO. The subdevice
>> passes through frame interval and mbus configuration of the active input
>> to the output side.
>
> The MUX framework is already in linux-next. Could you use that instead of
> adding new driver + bindings that are not compliant with the MUX framework?
> I don't think it'd be much of a change in terms of code, using the MUX
> framework appears quite simple.
I would prefer to wait on this, and get what we have merged now so I can
unload all these patches first.
Also this is Philipp's driver, so again I would prefer to get this
merged as-is and then Philipp can address these issues in a future
patch. But I will add my comments below...
>
> In general the driver looks pretty good, especially regarding the user space
> API implementation which is important for use with other drivers.
>
> I have some more detailed comments below.
>
<snip>
>> +
>> +struct vidsw {
>> + struct v4l2_subdev subdev;
>> + unsigned int num_pads;
>
> You could use subdev.entity.num_pads instead of caching the value locally.
Agreed.
>
>> + struct media_pad *pads;
>> + struct v4l2_mbus_framefmt *format_mbus;
>> + struct v4l2_of_endpoint *endpoint;
>> + struct regmap_field *field;
>> + struct gpio_desc *gpio;
>> + int active;
>> +};
>> +
>> +static inline struct vidsw *v4l2_subdev_to_vidsw(struct v4l2_subdev *sd)
>> +{
>> + return container_of(sd, struct vidsw, subdev);
>> +}
>> +
>> +static void vidsw_set_active(struct vidsw *vidsw, int active)
>> +{
>> + vidsw->active = active;
>> + if (active < 0)
>> + return;
>> +
>> + dev_dbg(vidsw->subdev.dev, "setting %d active\n", active);
>> +
>> + if (vidsw->field)
>> + regmap_field_write(vidsw->field, active);
>> + else if (vidsw->gpio)
>> + gpiod_set_value(vidsw->gpio, active);
>> +}
>> +
>> +static int vidsw_link_setup(struct media_entity *entity,
>> + const struct media_pad *local,
>> + const struct media_pad *remote, u32 flags)
>> +{
>> + struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
>> + struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
>> +
>> + /* We have no limitations on enabling or disabling our output link */
>> + if (local->index == vidsw->num_pads - 1)
>> + return 0;
>> +
>> + dev_dbg(sd->dev, "link setup %s -> %s", remote->entity->name,
>> + local->entity->name);
>> +
>> + if (!(flags & MEDIA_LNK_FL_ENABLED)) {
>> + if (local->index == vidsw->active) {
>> + dev_dbg(sd->dev, "going inactive\n");
>> + vidsw->active = -1;
>> + }
>> + return 0;
>> + }
>> +
>> + if (vidsw->active >= 0) {
>> + struct media_pad *pad;
>> +
>> + if (vidsw->active == local->index)
>> + return 0;
>> +
>> + pad = media_entity_remote_pad(&vidsw->pads[vidsw->active]);
>> + if (pad) {
>> + struct media_link *link;
>> + int ret;
>> +
>> + link = media_entity_find_link(pad,
>> + &vidsw->pads[vidsw->active]);
>> + if (link) {
>> + ret = __media_entity_setup_link(link, 0);
>
> I wouldn't implicitly disable a link, even if only one can be active at a
> given time. No other drivers do that either.
>
> Perhaps returning an error might be a better thing to do: if you're
> reconfiguring the pipeline anyway, there are likely issues elsewhere in it.
>
> We could also change the behaviour later to allow implicit changes but we
> can't later on go the other way without breaking the user space.
I think this whole if (vidsw->active >= 0) { ... } block should be
removed. This is left-over from the first implementation that tried
to propagate link setup upstream. This is not working yet, so for now
I think this should be removed.
<snip>
>
>>
>> +
>> +static bool vidsw_endpoint_disabled(struct device_node *ep)
>> +{
>> + struct device_node *rpp;
>> +
>> + if (!of_device_is_available(ep))
>
> ep here is the endpoint, whereas the argument to of_device_is_available()
> should correspond to the actual device.
Agreed, I think this if statement should be removed, and...
>
>> + return true;
>> +
>> + rpp = of_graph_get_remote_port_parent(ep);
>> + if (!rpp)
>> + return true;
this if statement can also be removed, since that is
handled automatically by of_device_is_available() below.
>> +
>> + return !of_device_is_available(rpp);
>> +}
>> +
>> +static int vidsw_async_init(struct vidsw *vidsw, struct device_node *node)
>
> I think I'd arrange this closer to probe as it's related to probe directly.
> Up to you.
>
>> +{
>> + struct device_node *ep;
>> + u32 portno;
>> + int numports;
>> + int ret;
>> + int i;
>> + bool active_link = false;
>> +
>> + numports = vidsw->num_pads;
>> +
>> + for (i = 0; i < numports - 1; i++)
>> + vidsw->pads[i].flags = MEDIA_PAD_FL_SINK;
>> + vidsw->pads[numports - 1].flags = MEDIA_PAD_FL_SOURCE;
>> +
>> + vidsw->subdev.entity.function = MEDIA_ENT_F_VID_MUX;
>> + ret = media_entity_pads_init(&vidsw->subdev.entity, numports,
>> + vidsw->pads);
>> + if (ret < 0)
>> + return ret;
>> +
>> + vidsw->subdev.entity.ops = &vidsw_ops;
>> +
>> + for_each_endpoint_of_node(node, ep) {
>> + struct v4l2_of_endpoint endpoint;
>> +
>> + v4l2_of_parse_endpoint(ep, &endpoint);
>> +
>> + portno = endpoint.base.port;
>> + if (portno >= numports - 1)
>> + continue;
>> +
>> + if (vidsw_endpoint_disabled(ep)) {
>> + dev_dbg(vidsw->subdev.dev,
>> + "port %d disabled\n", portno);
>> + continue;
>> + }
>> +
>> + vidsw->endpoint[portno] = endpoint;
>> +
>> + if (portno == vidsw->active)
>> + active_link = true;
>> + }
>> +
>> + for (portno = 0; portno < numports - 1; portno++) {
>> + if (!vidsw->endpoint[portno].base.local_node)
>> + continue;
>> +
>> + /* If the active input is not connected, use another */
>> + if (!active_link) {
>> + vidsw_set_active(vidsw, portno);
>> + active_link = true;
>> + }
>> + }
>> +
>> + return v4l2_async_register_subdev(&vidsw->subdev);
>> +}
>> +
>> +int vidsw_g_mbus_config(struct v4l2_subdev *sd, struct v4l2_mbus_config *cfg)
>
> We should get rid of g_mbus_config() in the long run, but as we don't have
> the alternative (frame descriptors) isn't up to the job yet I guess it's ok.
> I don't think we'll have too many users for the video switch right now.
>
>> +{
>> + struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
>> + struct media_pad *pad;
>> + int ret;
>> +
>> + if (vidsw->active == -1) {
>> + dev_err(sd->dev, "no configuration for inactive mux\n");
>> + return -EINVAL;
>> + }
>> +
>> + /*
>> + * Retrieve media bus configuration from the entity connected to the
>> + * active input
>> + */
>> + pad = media_entity_remote_pad(&vidsw->pads[vidsw->active]);
>> + if (pad) {
>> + sd = media_entity_to_v4l2_subdev(pad->entity);
>> + ret = v4l2_subdev_call(sd, video, g_mbus_config, cfg);
>> + if (ret == -ENOIOCTLCMD)
>> + pad = NULL;
>> + else if (ret < 0) {
>> + dev_err(sd->dev, "failed to get source configuration\n");
>> + return ret;
>> + }
>> + }
>> + if (!pad) {
>> + /* Mirror the input side on the output side */
>> + cfg->type = vidsw->endpoint[vidsw->active].bus_type;
>> + if (cfg->type == V4L2_MBUS_PARALLEL ||
>> + cfg->type == V4L2_MBUS_BT656)
>> + cfg->flags = vidsw->endpoint[vidsw->active].bus.parallel.flags;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int vidsw_s_stream(struct v4l2_subdev *sd, int enable)
>> +{
>> + struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
>> + struct v4l2_subdev *upstream_sd;
>> + struct media_pad *pad;
>> +
>> + if (vidsw->active == -1) {
>> + dev_err(sd->dev, "Can not start streaming on inactive mux\n");
>> + return -EINVAL;
>> + }
>> +
>> + pad = media_entity_remote_pad(&sd->entity.pads[vidsw->active]);
>> + if (!pad) {
>> + dev_err(sd->dev, "Failed to find remote source pad\n");
>> + return -ENOLINK;
>> + }
>> +
>> + if (!is_media_entity_v4l2_subdev(pad->entity)) {
>> + dev_err(sd->dev, "Upstream entity is not a v4l2 subdev\n");
>> + return -ENODEV;
>> + }
>> +
>> + upstream_sd = media_entity_to_v4l2_subdev(pad->entity);
>> +
>> + return v4l2_subdev_call(upstream_sd, video, s_stream, enable);
>
> Now that we'll have more than two drivers involved in the same pipeline it
> becomes necessary to define the behaviour of s_stream() throughout the
> pipeline --- i.e. whose responsibility is it to call s_stream() on the
> sub-devices in the pipeline?
In the case of imx-media, the capture device calls set stream on the
whole pipeline in the start_streaming() callback. This subdev call is
actually a NOOP for imx-media, because the upstream entity has already
started streaming. Again I think this should be removed. It also
enforces a stream order that some MC drivers may have a problem with.
For the remaining comments I'll let Philipp respond.
Steve
>
> I can submit a patch for that. I think the way you do it here is good, as it
> enables the caller to choose the appropriate behaviour, i.e. start the local
> device before or after the upstream sub-device.
>
>> +}
>> +
>> +static const struct v4l2_subdev_video_ops vidsw_subdev_video_ops = {
>> + .g_mbus_config = vidsw_g_mbus_config,
>> + .s_stream = vidsw_s_stream,
>> +};
>> +
>> +static struct v4l2_mbus_framefmt *
>> +__vidsw_get_pad_format(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_pad_config *cfg,
>> + unsigned int pad, u32 which)
>> +{
>> + struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
>> +
>> + switch (which) {
>> + case V4L2_SUBDEV_FORMAT_TRY:
>> + return v4l2_subdev_get_try_format(sd, cfg, pad);
>> + case V4L2_SUBDEV_FORMAT_ACTIVE:
>> + return &vidsw->format_mbus[pad];
>> + default:
>> + return NULL;
>> + }
>> +}
>> +
>> +static int vidsw_get_format(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_format *sdformat)
>> +{
>> + sdformat->format = *__vidsw_get_pad_format(sd, cfg, sdformat->pad,
>> + sdformat->which);
>> + return 0;
>> +}
>> +
>> +static int vidsw_set_format(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_format *sdformat)
>> +{
>> + struct vidsw *vidsw = v4l2_subdev_to_vidsw(sd);
>> + struct v4l2_mbus_framefmt *mbusformat;
>> +
>> + if (sdformat->pad >= vidsw->num_pads)
>> + return -EINVAL;
>
> This check is already performed in v4l2-subdev.c.
>
>> +
>> + mbusformat = __vidsw_get_pad_format(sd, cfg, sdformat->pad,
>> + sdformat->which);
>> + if (!mbusformat)
>> + return -EINVAL;
>> +
>> + /* Output pad mirrors active input pad, no limitations on input pads */
>
> Source and sink pads.
>
>> + if (sdformat->pad == (vidsw->num_pads - 1) && vidsw->active >= 0)
>
> I think it'd be cleaner to test for the pad flag instead of the number. Or,
> add a macro to obtain the source pad number.
>
>> + sdformat->format = vidsw->format_mbus[vidsw->active];
>> +
>> + *mbusformat = sdformat->format;
>> +
>> + return 0;
>> +}
>> +
>> +static struct v4l2_subdev_pad_ops vidsw_pad_ops = {
>> + .get_fmt = vidsw_get_format,
>> + .set_fmt = vidsw_set_format,
>> +};
>> +
>> +static struct v4l2_subdev_ops vidsw_subdev_ops = {
>> + .pad = &vidsw_pad_ops,
>> + .video = &vidsw_subdev_video_ops,
>> +};
>> +
>> +static int of_get_reg_field(struct device_node *node, struct reg_field *field)
>> +{
>> + u32 bit_mask;
>> + int ret;
>> +
>> + ret = of_property_read_u32(node, "reg", &field->reg);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = of_property_read_u32(node, "bit-mask", &bit_mask);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = of_property_read_u32(node, "bit-shift", &field->lsb);
>> + if (ret < 0)
>> + return ret;
>
> I think the above would look nice in a MUX driver. :-)
>
>> +
>> + field->msb = field->lsb + fls(bit_mask) - 1;
>> +
>> + return 0;
>> +}
>> +
>> +static int vidsw_probe(struct platform_device *pdev)
>> +{
>> + struct device_node *np = pdev->dev.of_node;
>> + struct of_endpoint endpoint;
>> + struct device_node *ep;
>> + struct reg_field field;
>> + struct vidsw *vidsw;
>> + struct regmap *map;
>> + unsigned int num_pads;
>> + int ret;
>> +
>> + vidsw = devm_kzalloc(&pdev->dev, sizeof(*vidsw), GFP_KERNEL);
>> + if (!vidsw)
>> + return -ENOMEM;
>> +
>> + platform_set_drvdata(pdev, vidsw);
>> +
>> + v4l2_subdev_init(&vidsw->subdev, &vidsw_subdev_ops);
>> + snprintf(vidsw->subdev.name, sizeof(vidsw->subdev.name), "%s",
>> + np->name);
>> + vidsw->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> + vidsw->subdev.dev = &pdev->dev;
>> +
>> + /*
>> + * The largest numbered port is the output port. It determines
>> + * total number of pads
>> + */
>> + num_pads = 0;
>
> You can initialise num_pads in variable declaration.
>
>> + for_each_endpoint_of_node(np, ep) {
>> + of_graph_parse_endpoint(ep, &endpoint);
>> + num_pads = max(num_pads, endpoint.port + 1);
>
> Port numbers come directly from DT.
>
> Shouldn't num_pads be only the number of pads that have links with actual
> physical connections? I.e. if a device is disabled, it shouldn't be
> counted here.
>
>> + }
>> +
>> + if (num_pads < 2) {
>> + dev_err(&pdev->dev, "Not enough ports %d\n", num_pads);
>> + return -EINVAL;
>> + }
>> +
>> + ret = of_get_reg_field(np, &field);
>> + if (ret == 0) {
>> + map = syscon_node_to_regmap(np->parent);
>> + if (!map) {
>> + dev_err(&pdev->dev, "Failed to get syscon register map\n");
>> + return PTR_ERR(map);
>> + }
>> +
>> + vidsw->field = devm_regmap_field_alloc(&pdev->dev, map, field);
>> + if (IS_ERR(vidsw->field)) {
>> + dev_err(&pdev->dev, "Failed to allocate regmap field\n");
>> + return PTR_ERR(vidsw->field);
>> + }
>> +
>> + regmap_field_read(vidsw->field, &vidsw->active);
>> + } else {
>> + if (num_pads > 3) {
>> + dev_err(&pdev->dev, "Too many ports %d\n", num_pads);
>> + return -EINVAL;
>> + }
>> +
>> + vidsw->gpio = devm_gpiod_get(&pdev->dev, NULL, GPIOD_OUT_LOW);
>> + if (IS_ERR(vidsw->gpio)) {
>> + dev_warn(&pdev->dev,
>> + "could not request control gpio: %d\n", ret);
>> + vidsw->gpio = NULL;
>> + }
>> +
>> + vidsw->active = gpiod_get_value(vidsw->gpio) ? 1 : 0;
>> + }
>> +
>> + vidsw->num_pads = num_pads;
>> + vidsw->pads = devm_kzalloc(&pdev->dev, sizeof(*vidsw->pads) * num_pads,
>> + GFP_KERNEL);
>> + vidsw->format_mbus = devm_kzalloc(&pdev->dev,
>> + sizeof(*vidsw->format_mbus) * num_pads, GFP_KERNEL);
>> + vidsw->endpoint = devm_kzalloc(&pdev->dev,
>> + sizeof(*vidsw->endpoint) * (num_pads - 1), GFP_KERNEL);
>> +
>> + ret = vidsw_async_init(vidsw, np);
>> + if (ret)
>> + return ret;
>> +
>> + return 0;
>> +}
>> +
>> +static int vidsw_remove(struct platform_device *pdev)
>> +{
>> + struct vidsw *vidsw = platform_get_drvdata(pdev);
>> + struct v4l2_subdev *sd = &vidsw->subdev;
>> +
>> + v4l2_async_unregister_subdev(sd);
>> + media_entity_cleanup(&sd->entity);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id vidsw_dt_ids[] = {
>> + { .compatible = "video-multiplexer", },
>> + { /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, vidsw_dt_ids);
>> +
>> +static struct platform_driver vidsw_driver = {
>> + .probe = vidsw_probe,
>> + .remove = vidsw_remove,
>> + .driver = {
>> + .of_match_table = vidsw_dt_ids,
>> + .name = "video-multiplexer",
>> + },
>> +};
>> +
>> +module_platform_driver(vidsw_driver);
>> +
>> +MODULE_DESCRIPTION("video stream multiplexer");
>> +MODULE_AUTHOR("Sascha Hauer, Pengutronix");
>> +MODULE_AUTHOR("Philipp Zabel, Pengutronix");
>> +MODULE_LICENSE("GPL");
>
^ permalink raw reply
* [PATCH 1/5] arm64: memory-hotplug: Add MEMORY_HOTPLUG, MEMORY_HOTREMOVE, MEMORY_PROBE
From: kbuild test robot @ 2017-04-12 0:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2edd1a6b64ea9b4c9b8126d840844d624fe8e0d4.1491920513.git.ar@linux.vnet.ibm.com>
Hi Scott,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.11-rc6]
[cannot apply to arm64/for-next/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Andrea-Reale/Memory-hotplug-support-for-arm64-complete-patchset/20170412-022242
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
Note: the linux-review/Andrea-Reale/Memory-hotplug-support-for-arm64-complete-patchset/20170412-022242 HEAD bffdda51b562ca7bb9baf7642e14c8d03d44602d builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
drivers/built-in.o: In function `memory_probe_store':
>> drivers/base/memory.c:501: undefined reference to `memory_add_physaddr_to_nid'
drivers/base/memory.c:501:(.text+0x1a5ef8): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `memory_add_physaddr_to_nid'
vim +501 drivers/base/memory.c
3947be19 Dave Hansen 2005-10-29 485 #ifdef CONFIG_ARCH_MEMORY_PROBE
3947be19 Dave Hansen 2005-10-29 486 static ssize_t
10fbcf4c Kay Sievers 2011-12-21 487 memory_probe_store(struct device *dev, struct device_attribute *attr,
28812fe1 Andi Kleen 2010-01-05 488 const char *buf, size_t count)
3947be19 Dave Hansen 2005-10-29 489 {
3947be19 Dave Hansen 2005-10-29 490 u64 phys_addr;
cb5490a5 John Allen 2016-01-14 491 int nid, ret;
61b94fea Anton Blanchard 2011-09-15 492 unsigned long pages_per_block = PAGES_PER_SECTION * sections_per_block;
3947be19 Dave Hansen 2005-10-29 493
b69deb2b Zhang Zhen 2014-08-06 494 ret = kstrtoull(buf, 0, &phys_addr);
b69deb2b Zhang Zhen 2014-08-06 495 if (ret)
b69deb2b Zhang Zhen 2014-08-06 496 return ret;
3947be19 Dave Hansen 2005-10-29 497
61b94fea Anton Blanchard 2011-09-15 498 if (phys_addr & ((pages_per_block << PAGE_SHIFT) - 1))
61b94fea Anton Blanchard 2011-09-15 499 return -EINVAL;
61b94fea Anton Blanchard 2011-09-15 500
bc02af93 Yasunori Goto 2006-06-27 @501 nid = memory_add_physaddr_to_nid(phys_addr);
6add7cd6 Nathan Fontenot 2011-01-31 502 ret = add_memory(nid, phys_addr,
cb5490a5 John Allen 2016-01-14 503 MIN_MEMORY_BLOCK_SIZE * sections_per_block);
cb5490a5 John Allen 2016-01-14 504
6add7cd6 Nathan Fontenot 2011-01-31 505 if (ret)
9f0af69b Nikanth Karthikesan 2011-03-24 506 goto out;
6add7cd6 Nathan Fontenot 2011-01-31 507
9f0af69b Nikanth Karthikesan 2011-03-24 508 ret = count;
9f0af69b Nikanth Karthikesan 2011-03-24 509 out:
:::::: The code at line 501 was first introduced by commit
:::::: bc02af93dd2bbddce1b55e0a493f833a1b7cf140 [PATCH] pgdat allocation for new node add (specify node id)
:::::: TO: Yasunori Goto <y-goto@jp.fujitsu.com>
:::::: CC: Linus Torvalds <torvalds@g5.osdl.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 34552 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170412/736f7a6b/attachment-0001.gz>
^ permalink raw reply
* "Consolidate get_dma_ops" breaks Xen on ARM
From: Stefano Stabellini @ 2017-04-11 23:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170411133649.GB27857@e104818-lin.cambridge.arm.com>
On Tue, 11 Apr 2017, Catalin Marinas wrote:
> On Tue, Apr 11, 2017 at 01:43:28PM +0100, Julien Grall wrote:
> > On 11/04/17 02:14, Bart Van Assche wrote:
> > > On 04/10/17 17:31, Stefano Stabellini wrote:
> > >> I think the reason is that, as you can see, if (dev && dev->dma_ops),
> > >> dev->dma_ops is returned, while before this changes, xen_dma_ops was
> > >> returned on Xen on ARM.
> > >>
> > >> Unfortunately DMA cannot work properly without using the appropriate
> > >> xen_dma_ops. See drivers/xen/swiotlb-xen.c and arch/arm/xen/mm.c for
> > >> more details. (The problem is easy to spot, but I wasn't CC'ed on the
> > >> patch.)
> > >>
> > >> I don't know how to solve this problem without introducing some sort of
> > >> if (xen()) in include/linux/dma-mapping.h.
> > >
> > > Sorry but I don't have access to an ARM development system. Does your
> > > comment apply to dev == NULL only, dev != NULL only or perhaps to both?
> > > If your comment applies to dev != NULL only, can you check whether
> > > adding something like set_dma_ops(dev, get_arch_dma_ops(NULL)) to the
> > > appropriate ARM arch_setup_dma_ops() function is sufficient?
> >
> > If I understand correctly, set_dma_ops will replace dev->dma_ops with
> > Xen DMA ops.
> >
> > However, Xen DMA ops will need in some places to call the device
> > specific DMA ops (see __generic_dma_ops(...)). So I think replacing
> > dev->dma_ops is not a solution here.
> >
> > The hackish patch below is fixing the problem for both ARM64 and ARM32.
> >
> > diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> > index 0977317c6835..43a73ddeec7a 100644
> > --- a/include/linux/dma-mapping.h
> > +++ b/include/linux/dma-mapping.h
> > @@ -174,6 +174,8 @@ int dma_mmap_from_coherent(struct device *dev, struct vm_area_struct *vma,
> > #include <asm/dma-mapping.h>
> > static inline const struct dma_map_ops *get_dma_ops(struct device *dev)
> > {
> > + if (xen_initial_domain())
> > + return xen_dma_ops;
> > if (dev && dev->dma_ops)
> > return dev->dma_ops;
> > return get_arch_dma_ops(dev ? dev->bus : NULL);
>
> If we do this, I guess there is no need to check for
> xen_initial_domain() in the get_arch_dma_ops() function. Anyway, this
> hunk would break the other architectures since xen_dma_ops is only
> defined for arm and arm64.
>
> > It is not nice as this is common code, but I can't find a better solution
> > so far. Any opinions?
>
> A different hack would be to avoid the generic get_dma_ops
> implementation on arm with some #ifdef hacks above.
>
> Yet another way would be for dom0 to always set dev->dma_ops to
> xen_dma_ops and preserve the real dma_ops somewhere under dev->archdata.
> You could intercept the arch_setup_dma_ops() function for this or use
> bus_register_notifier() (though I think the former is easier). The Xen
> code making use of the real dma_ops would have to dig them out from
> dev->archdata.
This is a good suggestion, Catalin. Thank you. See below. Is that what
you have in mind? Julien could you test it, please? If it is the right
approach, I'll submit the patch properly and rename __generic_dma_ops to
xen_generic_dma_ops or something.
diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h
index 220ba20..36ec9c8 100644
--- a/arch/arm/include/asm/device.h
+++ b/arch/arm/include/asm/device.h
@@ -16,6 +16,9 @@ struct dev_archdata {
#ifdef CONFIG_ARM_DMA_USE_IOMMU
struct dma_iommu_mapping *mapping;
#endif
+#ifdef CONFIG_XEN
+ const struct dma_map_ops *dev_dma_ops;
+#endif
bool dma_coherent;
};
diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
index 7166569..680d3f3 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -16,19 +16,9 @@
extern const struct dma_map_ops arm_dma_ops;
extern const struct dma_map_ops arm_coherent_dma_ops;
-static inline const struct dma_map_ops *__generic_dma_ops(struct device *dev)
-{
- if (dev && dev->dma_ops)
- return dev->dma_ops;
- return &arm_dma_ops;
-}
-
static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
{
- if (xen_initial_domain())
- return xen_dma_ops;
- else
- return __generic_dma_ops(NULL);
+ return &arm_dma_ops;
}
#define HAVE_ARCH_DMA_SUPPORTED 1
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 63eabb0..82d61eb 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -2396,6 +2396,11 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
dma_ops = arm_get_dma_map_ops(coherent);
set_dma_ops(dev, dma_ops);
+
+ if (xen_initial_domain()) {
+ dev->archdata.dev_dma_ops = dev->dma_ops;
+ dev->dma_ops = xen_dma_ops;
+ }
}
void arch_teardown_dma_ops(struct device *dev)
diff --git a/arch/arm64/include/asm/device.h b/arch/arm64/include/asm/device.h
index 73d5bab..5a5fa47 100644
--- a/arch/arm64/include/asm/device.h
+++ b/arch/arm64/include/asm/device.h
@@ -20,6 +20,9 @@ struct dev_archdata {
#ifdef CONFIG_IOMMU_API
void *iommu; /* private IOMMU data */
#endif
+#ifdef CONFIG_XEN
+ const struct dma_map_ops *dev_dma_ops;
+#endif
bool dma_coherent;
};
diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
index 505756c..5392dbe 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -27,11 +27,8 @@
#define DMA_ERROR_CODE (~(dma_addr_t)0)
extern const struct dma_map_ops dummy_dma_ops;
-static inline const struct dma_map_ops *__generic_dma_ops(struct device *dev)
+static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
{
- if (dev && dev->dma_ops)
- return dev->dma_ops;
-
/*
* We expect no ISA devices, and all other DMA masters are expected to
* have someone call arch_setup_dma_ops at device creation time.
@@ -39,14 +36,6 @@ static inline const struct dma_map_ops *__generic_dma_ops(struct device *dev)
return &dummy_dma_ops;
}
-static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
-{
- if (xen_initial_domain())
- return xen_dma_ops;
- else
- return __generic_dma_ops(NULL);
-}
-
void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
const struct iommu_ops *iommu, bool coherent);
#define arch_setup_dma_ops arch_setup_dma_ops
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 81cdb2e..e574c39 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -977,4 +977,9 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
dev->archdata.dma_coherent = coherent;
__iommu_setup_dma_ops(dev, dma_base, size, iommu);
+
+ if (xen_initial_domain()) {
+ dev->archdata.dev_dma_ops = dev->dma_ops;
+ dev->dma_ops = xen_dma_ops;
+ }
}
diff --git a/include/xen/arm/page-coherent.h b/include/xen/arm/page-coherent.h
index 95ce6ac..b0a2bfc 100644
--- a/include/xen/arm/page-coherent.h
+++ b/include/xen/arm/page-coherent.h
@@ -2,8 +2,16 @@
#define _ASM_ARM_XEN_PAGE_COHERENT_H
#include <asm/page.h>
+#include <asm/dma-mapping.h>
#include <linux/dma-mapping.h>
+static inline const struct dma_map_ops *__generic_dma_ops(struct device *dev)
+{
+ if (dev && dev->archdata.dev_dma_ops)
+ return dev->archdata.dev_dma_ops;
+ return get_arch_dma_ops(NULL);
+}
+
void __xen_dma_map_page(struct device *hwdev, struct page *page,
dma_addr_t dev_addr, unsigned long offset, size_t size,
enum dma_data_direction dir, unsigned long attrs);
^ permalink raw reply related
* [PATCH 2/2] ARM: dts: imx7: add USDHC NAND clock to SDHC instances
From: Fabio Estevam @ 2017-04-11 23:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170411025956.GA9067@b29396-OptiPlex-7040>
On Mon, Apr 10, 2017 at 11:59 PM, Dong Aisheng <dongas86@gmail.com> wrote:
> This is caused by ahb_root_clk gets disabled accidently and system hangs.
>
> Because this patch defines ipg_root_clk earlier before its parent
> (ahb_root_clk) got registered, then it will be marked as a orphan clk
> temporarily. Until the parent ahb_root_clk got registered, the clk core
> will reparent it to the newly found parent. (see __clk_core_init() function).
>
> Due to CLK_SET_RATE_PARENT flag, the ahb clk will be enabled during
> set_parent operation and then disabled after that.
> Then system hang cause we still get no chance to run init_on clks.
>
> I just send out a proper fix patch with correct register sequence.
Excellent, thanks!
^ permalink raw reply
* [PATCH v3 21/32] powerpc: include default ioremap_nopost() implementation
From: Benjamin Herrenschmidt @ 2017-04-11 23:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170411142456.GB6821@red-moon>
On Tue, 2017-04-11 at 15:24 +0100, Lorenzo Pieralisi wrote:
> Ok, point taken. BTW, may I ask you guys to have a look into this
> please ?
>
> https://lkml.org/lkml/2017/4/6/743
>
> It is a side effect of this thread (v2), not sure why <asm/io.h>
> on powerpc has to include <linux/io.h>.
Not sure how we ended up with that... it's odd indeed.
Michael ? Any reason we can't just remove it ?
Cheers,
Ben.
^ permalink raw reply
* [PATCH v3 04/32] asm-generic: add ioremap_nopost() remap interface
From: Benjamin Herrenschmidt @ 2017-04-11 23:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170411143138.GC6821@red-moon>
On Tue, 2017-04-11 at 15:31 +0100, Lorenzo Pieralisi wrote:
> > This is a semantic that simply *cannot* be generically provided accross
> > architectures as a mapping attribute.
>
> I agree that a default implementation does not make much sense. The
> only solution to this, if we want the ioremap_nopost to be made available
> to generic code (and drivers - ie DT PCI host bridge drivers on ARM/ARM64
> are not arch code), is to make the ioremap_nopost() call return NULL
> unless overriden by arch code that can provide its semantics.
That would be a better option.
You might be able to implement a fallback, for example by having the
config ops do a read back from the bridge.
Ben.
^ permalink raw reply
* [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings
From: Benjamin Herrenschmidt @ 2017-04-11 23:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170411140857.GA6821@red-moon>
On Tue, 2017-04-11 at 15:08 +0100, Lorenzo Pieralisi wrote:
> On Tue, Apr 11, 2017 at 11:38:26PM +1000, Benjamin Herrenschmidt wrote:
> > On Tue, 2017-04-11 at 13:28 +0100, Lorenzo Pieralisi wrote:
> > > This patch series[1] is a v3 of a previous version:
> > >
> > > v2: https://lkml.org/lkml/2017/3/27/220
> >
> > I am not a fan of this at All.
> >
> > That whole concept of "ioremap_nopost" is simply not applicable to the
> > majority of architectures and certainly not in a way that can apply to
> > arbitrary mappings.
> >
> > It's also very wrong to provide a "default" operation whose semantics
> > are weaker than what it's supposed to implement. Very wrong actually.
> > People will use it assuming the non-posted behaviour and things will
> > break in subtle way when it cannot be provided.
>
> Well, what's very wrong for you it is not very wrong for others
> (it is just v3, that's fine, see thread below).
Maybe, but I don't see in what universe it is ok to have something
defined for the stronger ordering semantics it provide silently
fallback to weaker semantics.
> I can easily make ioremap_nopost() mirror ioremap_uc() (ie return
> NULL unless overriden so that basically you can't use in on an arch
> that can't provide its semantics) but then that becomes very wrong
> for other reviewers.
Those reviewers are WRONG :-)
> https://lkml.org/lkml/2017/4/6/396
>
> > What exactly are you trying to fix here ?
>
> I wrote in the commit logs and cover letter what I am fixing here.
Right right, what *actual bug you have observed* are you trying to fix
?
I'm pretty such close to all non-x86 archs had that "problem" since the
dawn of time and it has never hurt anybody.
That said, I don't think it makes sense to "solve" it by creating a
"generic" mapping semantic that is basically impossible to implement on
most architectures out there (and cannot be emulated).
This is a problem to be solved by the bridge itself. If ARM has a
mapping attribute to make stores non-posted, keep this an ARM specific
attribute at this stage I'd say.
> Anyway:
>
> "The PCI specifications (Rev 3.0, 3.2.5 "Transaction Ordering and
> Posting") mandate non-posted configuration transactions. As further
> highlighted in the PCIe specifications (4.0 - Rev0.3, "Ordering
> Considerations for the Enhanced Configuration Access Mechanism"),
> through ECAM and ECAM-derivative configuration mechanism, the memory
> mapped transactions from the host CPU into Configuration Requests on the
> PCI express fabric may create ordering problems for software because
> writes to memory address are typically posted transactions (unless the
> architecture can enforce through virtual address mapping non-posted
> write transactions behaviour) but writes to Configuration Space are not
> posted on the PCI express fabric."
>
> On ARM64:
>
> "This rule is reinforced by the ARM v8 architecture reference manual
> (issue A.k, Early Write Acknowledgment) that explicitly recommends
> that No Early Write Acknowledgment attribute should be used to map
> PCI configuration (write) transactions."
>
> > If a given PCIe host bridge (architecture specific) require a special
> > sauce to provide the illusion of non-posting, then implement this in
> > the actual root complex code.
> >
> > BTW. I'm pretty sure we "accidentally" made config writes posted at
> > least to the PHB on a number of powerpc systems forever and we *never*
> > had a problem because of it ;)
>
> Ok so we should ignore the PCIe specifications and ARM v8 reference
> manual waiting for a kernel bug to appear ? Is that what you suggest
> doing ?
>
> Lorenzo
>
> > > v2 -> v3:
> > > - Created a default ioremap_nopost() implementation in a
> > > separate
> > > ??asm-generic header and patched all arches to make use of it
> > > - Removed PCI drivers patches from the series to simplify the
> > > ??review, they will be posted separately once the
> > > ioremap_nopost()
> > > ??interface is settled
> > > - Fixed devm_ioremap_* BUS offset comments and implemented
> > > ??nopost interface on top of it
> > > - Added collected tags
> > >
> > > v1: https://lkml.org/lkml/2017/2/27/228
> > >
> > > v1 -> v2:
> > > - Changed pci_remap_cfgspace() to more generic ioremap_nopost()
> > > ??interface
> > > - Added pgprot_nonposted
> > > - Fixed build errors on arches not relying on asm-generic
> > > headers
> > > - Added PCI versatile host controller driver patch
> > > - Added missing config space remapping to hisilicon host
> > > controller
> > >
> > > ---------------------
> > > Original cover letter
> > > ---------------------
> > >
> > > PCI local bus specifications (Rev3.0, 3.2.5 "Transaction Ordering
> > > and Posting") strictly require PCI configuration and I/O Address
> > > space
> > > write transactions to be non-posted.
> > >
> > > Current crop of DT/ACPI PCI host controllers drivers relies on
> > > the ioremap interface to map ECAM and ECAM-derivative PCI config
> > > regions and pci_remap_iospace() to create a VMA for mapping
> > > PCI host bridge I/O Address space transactions to CPU virtual address
> > > space.
> > >
> > > On some platforms (ie ARM/ARM64) ioremap fails to comply with the PCI
> > > configuration non-posted write transactions requirement, because it
> > > provides a memory mapping that issues "bufferable" or, in PCI terms
> > > "posted" write transactions. Likewise, the current
> > > pci_remap_iospace()
> > > implementation maps the physical address range that the PCI
> > > translates
> > > to I/O space cycles to virtual address space through pgprot_device()
> > > attributes that on eg ARM64 provides a memory mapping issuing
> > > posted writes transactions, which is not PCI specifications
> > > compliant.
> > >
> > > This patch series[1] addresses both issues in one go:
> > >
> > > - It updates the pci_remap_iospace() function to use a page mapping
> > > ? that guarantees non-posted write transactions for I/O space
> > > addresses
> > > - It adds a kernel API to remap PCI config space resources, so that
> > > ? architecture can override it with a mapping implementation that
> > > ? guarantees PCI specifications compliancy wrt non-posted write
> > > ? configuration transactions
> > > - It updates all PCI host controller implementations (and the generic
> > > ? ECAM layer) to use the newly introduced mapping interface
> > >
> > > Tested on Juno ECAM based interface (DT/ACPI).
> > >
> > > Non-ECAM PCI host controller drivers patches need checking to make
> > > sure that:
> > >
> > > - I patched the correct resource region mapping for config space
> > > - There are not any other ways to ensure posted-write completion
> > > ? in the respective pci_ops that make the relevant patch unnecessary
> > >
> > > [1]
> > > git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git
> > > pci/config-io-mappings-fix-v3
> > >
> > > Lorenzo Pieralisi (32):
> > > ? PCI: remove __weak tag from pci_remap_iospace()
> > > ? asm-generic/pgtable.h: introduce pgprot_nonposted remap attribute
> > > ? PCI: fix pci_remap_iospace() remap attribute
> > > ? asm-generic: add ioremap_nopost() remap interface
> > > ? alpha: include default ioremap_nopost() implementation
> > > ? avr32: include default ioremap_nopost() implementation
> > > ? arc: include default ioremap_nopost() implementation
> > > ? cris: include default ioremap_nopost() implementation
> > > ? frv: include default ioremap_nopost() implementation
> > > ? hexagon: include default ioremap_nopost() implementation
> > > ? ia64: include default ioremap_nopost() implementation
> > > ? m32r: include default ioremap_nopost() implementation
> > > ? m68k: include default ioremap_nopost() implementation
> > > ? metag: include default ioremap_nopost() implementation
> > > ? microblaze: include default ioremap_nopost() implementation
> > > ? mips: include default ioremap_nopost() implementation
> > > ? mn10300: include default ioremap_nopost() implementation
> > > ? nios2: include default ioremap_nopost() implementation
> > > ? openrisc: include default ioremap_nopost() implementation
> > > ? parisc: include default ioremap_nopost() implementation
> > > ? powerpc: include default ioremap_nopost() implementation
> > > ? s390: include default ioremap_nopost() implementation
> > > ? sh: include default ioremap_nopost() implementation
> > > ? sparc: include default ioremap_nopost() implementation
> > > ? tile: include default ioremap_nopost() implementation
> > > ? unicore32: include default ioremap_nopost() implementation
> > > ? x86: include default ioremap_nopost() implementation
> > > ? xtensa: include default ioremap_nopost() implementation
> > > ? arm64: implement ioremap_nopost() interface
> > > ? arm: implement ioremap_nopost() interface
> > > ? lib: fix Devres devm_ioremap_* offset parameter kerneldoc
> > > description
> > > ? lib: implement Devres ioremap_nopost() interface
> > >
> > > ?Documentation/driver-model/devres.txt |??3 ++
> > > ?arch/alpha/include/asm/io.h???????????|??1 +
> > > ?arch/arc/include/asm/io.h?????????????|??1 +
> > > ?arch/arm/include/asm/io.h?????????????|??9 ++++
> > > ?arch/arm/mm/ioremap.c?????????????????|??7 +++
> > > ?arch/arm/mm/nommu.c???????????????????|??9 ++++
> > > ?arch/arm64/include/asm/io.h???????????| 12 +++++
> > > ?arch/avr32/include/asm/io.h???????????|??1 +
> > > ?arch/cris/include/asm/io.h????????????|??1 +
> > > ?arch/frv/include/asm/io.h?????????????|??1 +
> > > ?arch/hexagon/include/asm/io.h?????????|??2 +
> > > ?arch/ia64/include/asm/io.h????????????|??1 +
> > > ?arch/m32r/include/asm/io.h????????????|??1 +
> > > ?arch/m68k/include/asm/io.h????????????|??1 +
> > > ?arch/metag/include/asm/io.h???????????|??2 +
> > > ?arch/microblaze/include/asm/io.h??????|??1 +
> > > ?arch/mips/include/asm/io.h????????????|??1 +
> > > ?arch/mn10300/include/asm/io.h?????????|??1 +
> > > ?arch/nios2/include/asm/io.h???????????|??1 +
> > > ?arch/openrisc/include/asm/io.h????????|??2 +
> > > ?arch/parisc/include/asm/io.h??????????|??1 +
> > > ?arch/powerpc/include/asm/io.h?????????|??1 +
> > > ?arch/s390/include/asm/io.h????????????|??1 +
> > > ?arch/sh/include/asm/io.h??????????????|??1 +
> > > ?arch/sparc/include/asm/io.h???????????|??1 +
> > > ?arch/tile/include/asm/io.h????????????|??1 +
> > > ?arch/unicore32/include/asm/io.h???????|??1 +
> > > ?arch/x86/include/asm/io.h?????????????|??1 +
> > > ?arch/xtensa/include/asm/io.h??????????|??1 +
> > > ?drivers/pci/pci.c?????????????????????|??4 +-
> > > ?include/asm-generic/ioremap-nopost.h??|??9 ++++
> > > ?include/asm-generic/pgtable.h?????????|??4 ++
> > > ?include/linux/device.h????????????????|??2 +
> > > ?include/linux/io.h????????????????????|??2 +
> > > ?lib/devres.c??????????????????????????| 84
> > > +++++++++++++++++++++++++++++++++--
> > > ?35 files changed, 167 insertions(+), 5 deletions(-)
> > > ?create mode 100644 include/asm-generic/ioremap-nopost.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