* [GIT PULL 2/2] ARM: DTS: Keystone update for v5.8
From: Santosh Shilimkar @ 2020-05-28 4:01 UTC (permalink / raw)
To: arm, linux-arm-kernel
Cc: arnd, khilman, santosh.shilimkar, linux-kernel, soc, olof
In-Reply-To: <1590638489-12023-1-git-send-email-santosh.shilimkar@oracle.com>
The following changes since commit 8f3d9f354286745c751374f5f1fcafee6b3f3136:
Linux 5.7-rc1 (2020-04-12 12:35:55 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux-keystone.git tags/keystone_dts_for_5.8
for you to fetch changes up to 644c5a582261ecdf1df41b11d05d10a1cccc0a66:
ARM: dts: keystone: Rename "msmram" node to "sram" (2020-05-27 20:36:32 -0700)
----------------------------------------------------------------
ARM: dts: Keystone update for v5.8
- Rename "msmram" node to "sram"
----------------------------------------------------------------
Krzysztof Kozlowski (1):
ARM: dts: keystone: Rename "msmram" node to "sram"
arch/arm/boot/dts/keystone-k2e.dtsi | 4 ++--
arch/arm/boot/dts/keystone-k2g.dtsi | 4 ++--
arch/arm/boot/dts/keystone-k2hk.dtsi | 4 ++--
arch/arm/boot/dts/keystone-k2l.dtsi | 4 ++--
4 files changed, 8 insertions(+), 8 deletions(-)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [GIT PULL 1/2] soc: TI drivers updates for v5.8
From: Santosh Shilimkar @ 2020-05-28 4:01 UTC (permalink / raw)
To: arm, linux-arm-kernel
Cc: arnd, khilman, santosh.shilimkar, linux-kernel, soc, olof
The following changes since commit 8f3d9f354286745c751374f5f1fcafee6b3f3136:
Linux 5.7-rc1 (2020-04-12 12:35:55 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/ssantosh/linux-keystone.git tags/drivers_soc_for_5.8
for you to fetch changes up to b8b38a8e3cae100f292d756e32c78ab288db8a7d:
drivers: soc: ti: knav_qmss_queue: Make knav_gp_range_ops static (2020-05-27 20:39:14 -0700)
----------------------------------------------------------------
soc: ARM TI update for v5.8
- Platform chipid driver support and associated dts doc update
- Sparse warning fix in Navigator driver
----------------------------------------------------------------
Grygorii Strashko (2):
dt-bindings: soc: ti: add binding for k3 platforms chipid module
soc: ti: add k3 platforms chipid module driver
Samuel Zou (1):
drivers: soc: ti: knav_qmss_queue: Make knav_gp_range_ops static
.../devicetree/bindings/soc/ti/k3-socinfo.yaml | 40 ++++++
drivers/soc/ti/Kconfig | 10 ++
drivers/soc/ti/Makefile | 1 +
drivers/soc/ti/k3-socinfo.c | 152 +++++++++++++++++++++
drivers/soc/ti/knav_qmss_queue.c | 2 +-
5 files changed, 204 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/soc/ti/k3-socinfo.yaml
create mode 100644 drivers/soc/ti/k3-socinfo.c
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 06/12] PM / devfreq: Add cpu based scaling support to passive_governor
From: Chanwoo Choi @ 2020-05-28 5:03 UTC (permalink / raw)
To: Andrew-sh.Cheng, MyungJoo Ham, Kyungmin Park, Rob Herring,
Mark Rutland, Matthias Brugger, Rafael J . Wysocki, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Liam Girdwood, Mark Brown
Cc: devicetree, srv_heupstream, linux-pm, linux-kernel,
Saravana Kannan, linux-mediatek, Sibi Sankar, linux-arm-kernel
In-Reply-To: <20200520034307.20435-7-andrew-sh.cheng@mediatek.com>
Hi Andrew-sh.Cheng,
Thanks for your posting. I like this approach absolutely.
I think that it is necessary. When I developed the embedded product,
I needed this feature always.
I add the comments on below.
On 5/20/20 12:43 PM, Andrew-sh.Cheng wrote:
> From: Saravana Kannan <skannan@codeaurora.org>
>
> Many CPU architectures have caches that can scale independent of the
> CPUs. Frequency scaling of the caches is necessary to make sure that the
> cache is not a performance bottleneck that leads to poor performance and
> power. The same idea applies for RAM/DDR.
>
> To achieve this, this patch adds support for cpu based scaling to the
> passive governor. This is accomplished by taking the current frequency
> of each CPU frequency domain and then adjust the frequency of the cache
> (or any devfreq device) based on the frequency of the CPUs. It listens
> to CPU frequency transition notifiers to keep itself up to date on the
> current CPU frequency.
>
> To decide the frequency of the device, the governor does one of the
> following:
> * Derives the optimal devfreq device opp from required-opps property of
> the parent cpu opp_table.
>
> * Scales the device frequency in proportion to the CPU frequency. So, if
> the CPUs are running at their max frequency, the device runs at its
> max frequency. If the CPUs are running at their min frequency, the
> device runs at its min frequency. It is interpolated for frequencies
> in between.
>
> Andrew-sh.Cheng change
> dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
> to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
> for kernel-5.7
>
> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
> [Sibi: Integrated cpu-freqmap governor into passive_governor]
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> ---
> drivers/devfreq/Kconfig | 2 +
> drivers/devfreq/governor_passive.c | 278 ++++++++++++++++++++++++++++++++++---
> include/linux/devfreq.h | 40 +++++-
> 3 files changed, 299 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 0b1df12e0f21..d9067950af6a 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -73,6 +73,8 @@ config DEVFREQ_GOV_PASSIVE
> device. This governor does not change the frequency by itself
> through sysfs entries. The passive governor recommends that
> devfreq device uses the OPP table to get the frequency/voltage.
> + Alternatively the governor can also be chosen to scale based on
> + the online CPUs current frequency.
>
> comment "DEVFREQ Drivers"
>
> diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
> index 2d67d6c12dce..7dcda02a5bb7 100644
> --- a/drivers/devfreq/governor_passive.c
> +++ b/drivers/devfreq/governor_passive.c
> @@ -8,11 +8,89 @@
> */
>
> #include <linux/module.h>
> +#include <linux/cpu.h>
> +#include <linux/cpufreq.h>
> +#include <linux/cpumask.h>
> #include <linux/device.h>
> #include <linux/devfreq.h>
> +#include <linux/slab.h>
> #include "governor.h"
>
> -static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> +static unsigned int xlate_cpufreq_to_devfreq(struct devfreq_passive_data *data,
Need to change 'unsigned int' to 'unsigned long'.
> + unsigned int cpu)
> +{
> + unsigned int cpu_min, cpu_max, dev_min, dev_max, cpu_percent, max_state;
Better to define them separately as following and then need to rename
the variable. Usually, use the 'min_freq' and 'max_freq' word for
the minimum/maximum frequency.
unsigned int cpu_min_freq, cpu_max_freq, cpu_curr_freq, cpu_percent;
unsigned long dev_min_freq, dev_max_freq, dev_max_state,
The devfreq used 'unsigned long'. The cpufreq used 'unsigned long'
and 'unsigned int'. You need to handle them properly.
> + struct devfreq_cpu_state *cpu_state = data->cpu_state[cpu];
> + struct devfreq *devfreq = (struct devfreq *)data->this;
> + unsigned long *freq_table = devfreq->profile->freq_table;
In this function, use 'cpu' work for cpufreq and use 'dev' for devfreq.
So, I think 'dev_freq_table' is proper name instead of 'freq_table'
for the readability.
freq_table -> dev_freq_table
> + struct dev_pm_opp *opp = NULL, *cpu_opp = NULL;
In the get_target_freq_with_devfreq(), use 'p_opp' indicating
the OPP of parent device. For the consistency, I think that
use 'p_opp' instead of 'cpu_opp'.
> + unsigned long cpu_freq, freq;
Define the 'cpu_freq' on above with cpu_min_freq/cpu_max_freq definition.
cpu_freq -> cpu_curr_freq.
> +
> + if (!cpu_state || cpu_state->first_cpu != cpu ||
> + !cpu_state->opp_table || !devfreq->opp_table)
> + return 0;
> +
> + cpu_freq = cpu_state->freq * 1000;
> + cpu_opp = devfreq_recommended_opp(cpu_state->dev, &cpu_freq, 0);
> + if (IS_ERR(cpu_opp))
> + return 0;
> +
> + opp = dev_pm_opp_xlate_required_opp(cpu_state->opp_table,
> + devfreq->opp_table, cpu_opp);
> + dev_pm_opp_put(cpu_opp);
> +
> + if (!IS_ERR(opp)) {
> + freq = dev_pm_opp_get_freq(opp);
> + dev_pm_opp_put(opp);
Better to add the 'out' goto statement.
If you use 'goto out', you can reduce the one indentation
without 'else' statement.
> + } else {
As I commented, when dev_pm_opp_xlate_required_opp() return successfully
, use 'goto out'. We can remove 'else' and then reduce the unneeded indentation.
> + /* Use Interpolation if required opps is not available */
> + cpu_min = cpu_state->min_freq;
> + cpu_max = cpu_state->max_freq;
> + cpu_freq = cpu_state->freq;
> +
> + if (freq_table) {
> + /* Get minimum frequency according to sorting order */
> + max_state = freq_table[devfreq->profile->max_state - 1];
> + if (freq_table[0] < max_state) {
> + dev_min = freq_table[0];
> + dev_max = max_state;
> + } else {
> + dev_min = max_state;
> + dev_max = freq_table[0];
> + }
> + } else {
> + if (devfreq->user_max_freq_req.data.freq.qos->max_freq.target_value
> + <= devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value)
> + return 0;
> + dev_min =
> + devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value;
> + dev_max =
> + devfreq->user_max_freq_req.data.freq.qos->max_freq.target_value;
I think it is not proper to access the variable of pm_qos structure directly.
Instead of direct access, you have to use the exported PM QoS function such as
- pm_qos_read_value(devfreq->dev.parent, DEV_PM_QOS_MIN_FREQUENCY);
- pm_qos_read_value(devfreq->dev.parent, DEV_PM_QOS_MAX_FREQUENCY);
> + }
> + cpu_percent = ((cpu_freq - cpu_min) * 100) / cpu_max - cpu_min;
> + freq = dev_min + mult_frac(dev_max - dev_min, cpu_percent, 100);
> + }
I think that you better to add 'out' jump label as following:
out:
> +
> + return freq;
> +}
> +
> +static int get_target_freq_with_cpufreq(struct devfreq *devfreq,
> + unsigned long *freq)
> +{
> + struct devfreq_passive_data *p_data =
> + (struct devfreq_passive_data *)devfreq->data;
> + unsigned int cpu, target_freq = 0;
Need to define 'target_freq' with 'unsigned long' type.
> +
> + for_each_online_cpu(cpu)
> + target_freq = max(target_freq,
> + xlate_cpufreq_to_devfreq(p_data, cpu));
> +
> + *freq = target_freq;
> +
> + return 0;
> +}
> +
> +static int get_target_freq_with_devfreq(struct devfreq *devfreq,
> unsigned long *freq)
> {
> struct devfreq_passive_data *p_data
> @@ -23,16 +101,6 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> int i, count, ret = 0;
>
> /*
> - * If the devfreq device with passive governor has the specific method
> - * to determine the next frequency, should use the get_target_freq()
> - * of struct devfreq_passive_data.
> - */
> - if (p_data->get_target_freq) {
> - ret = p_data->get_target_freq(devfreq, freq);
> - goto out;
> - }
> -
> - /*
> * If the parent and passive devfreq device uses the OPP table,
> * get the next frequency by using the OPP table.
> */
> @@ -102,6 +170,37 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> return ret;
> }
>
> +static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> + unsigned long *freq)
> +{
> + struct devfreq_passive_data *p_data =
> + (struct devfreq_passive_data *)devfreq->data;
> + int ret;
> +
> + /*
> + * If the devfreq device with passive governor has the specific method
> + * to determine the next frequency, should use the get_target_freq()
> + * of struct devfreq_passive_data.
> + */
> + if (p_data->get_target_freq)
> + return p_data->get_target_freq(devfreq, freq);
> +
> + switch (p_data->parent_type) {
> + case DEVFREQ_PARENT_DEV:
> + ret = get_target_freq_with_devfreq(devfreq, freq);
> + break;
> + case CPUFREQ_PARENT_DEV:
> + ret = get_target_freq_with_cpufreq(devfreq, freq);
> + break;
> + default:
> + ret = -EINVAL;
> + dev_err(&devfreq->dev, "Invalid parent type\n");
> + break;
> + }
> +
> + return ret;
> +}
> +
> static int update_devfreq_passive(struct devfreq *devfreq, unsigned long freq)
> {
> int ret;
> @@ -156,6 +255,140 @@ static int devfreq_passive_notifier_call(struct notifier_block *nb,
> return NOTIFY_DONE;
> }
>
> +static int cpufreq_passive_notifier_call(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
> + struct devfreq_passive_data *data =
> + container_of(nb, struct devfreq_passive_data, nb);
> + struct devfreq *devfreq = (struct devfreq *)data->this;
> + struct devfreq_cpu_state *cpu_state;
> + struct cpufreq_freqs *freq = ptr;
How about changing 'freq' to 'cpu_freqs'?
In the drivers/cpufreq/cpufreq.c, use 'freqs' name indicating
the instance of 'struct cpufreq_freqs'. And in order to
identfy, how about adding 'cpu_' prefix for variable name?
> + unsigned int current_freq;
Need to define curr_freq with 'unsigned long' type
and better to use 'curr_freq' variable name.
> + int ret;
> +
> + if (event != CPUFREQ_POSTCHANGE || !freq ||
> + !data->cpu_state[freq->policy->cpu])
> + return 0;
> +
> + cpu_state = data->cpu_state[freq->policy->cpu];
> + if (cpu_state->freq == freq->new)
> + return 0;
> +
> + /* Backup current freq and pre-update cpu state freq*/
> + current_freq = cpu_state->freq;
> + cpu_state->freq = freq->new;
> +
> + mutex_lock(&devfreq->lock);
> + ret = update_devfreq(devfreq);
> + mutex_unlock(&devfreq->lock);
> + if (ret) {
> + cpu_state->freq = current_freq;
> + dev_err(&devfreq->dev, "Couldn't update the frequency.\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int cpufreq_passive_register(struct devfreq_passive_data **p_data)
> +{
> + struct devfreq_passive_data *data = *p_data;
> + struct devfreq *devfreq = (struct devfreq *)data->this;
> + struct device *dev = devfreq->dev.parent;
> + struct opp_table *opp_table = NULL;
> + struct devfreq_cpu_state *state;
For the readability, I thinkt 'cpu_state' is proper instead of 'state'.
> + struct cpufreq_policy *policy;
> + struct device *cpu_dev;
> + unsigned int cpu;
> + int ret;
> +
> + get_online_cpus();
Add blank line.
> + data->nb.notifier_call = cpufreq_passive_notifier_call;
> + ret = cpufreq_register_notifier(&data->nb,
> + CPUFREQ_TRANSITION_NOTIFIER);
> + if (ret) {
> + dev_err(dev, "Couldn't register cpufreq notifier.\n");
> + data->nb.notifier_call = NULL;
> + goto out;
> + }
> +
> + /* Populate devfreq_cpu_state */
> + for_each_online_cpu(cpu) {
> + if (data->cpu_state[cpu])
> + continue;
> +
> + policy = cpufreq_cpu_get(cpu);
cpufreq_cpu_get() might return 'NULL'. I think you need to handle
return value as following:
if (!policy) {
ret = -EINVAL;
goto out;
} else if (PTR_ERR(policy) == -EPROBE_DEFER) {
goto out;
} else if (IS_ERR(policy) {
ret = PTR_ERR(policy);
dev_err(dev, "Couldn't get the cpufreq_poliy.\n");
goto out;
}
If cpufreq_cpu_get() return successfully, to do next.
It reduces the one indentaion.
> + if (policy) {
> + state = kzalloc(sizeof(*state), GFP_KERNEL);
> + if (!state) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + cpu_dev = get_cpu_device(cpu);
> + if (!cpu_dev) {
> + dev_err(dev, "Couldn't get cpu device.\n");
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + opp_table = dev_pm_opp_get_opp_table(cpu_dev);
> + if (IS_ERR(devfreq->opp_table)) {
> + ret = PTR_ERR(opp_table);
> + goto out;
> + }
> +
> + state->dev = cpu_dev;
> + state->opp_table = opp_table;
> + state->first_cpu = cpumask_first(policy->related_cpus);
> + state->freq = policy->cur;
> + state->min_freq = policy->cpuinfo.min_freq;
> + state->max_freq = policy->cpuinfo.max_freq;
> + data->cpu_state[cpu] = state;
Add blank line.
> + cpufreq_cpu_put(policy);
> + } else {
> + ret = -EPROBE_DEFER;
> + goto out;
> + }
> + }
Add blank line.
> +out:
> + put_online_cpus();
> + if (ret)
> + return ret;
> +
> + /* Update devfreq */
> + mutex_lock(&devfreq->lock);
> + ret = update_devfreq(devfreq);
> + mutex_unlock(&devfreq->lock);
> + if (ret)
> + dev_err(dev, "Couldn't update the frequency.\n");
> +
> + return ret;
> +}
> +
> +static int cpufreq_passive_unregister(struct devfreq_passive_data **p_data)
> +{
> + struct devfreq_passive_data *data = *p_data;
> + struct devfreq_cpu_state *cpu_state;
> + int cpu;
> +
> + if (data->nb.notifier_call)
> + cpufreq_unregister_notifier(&data->nb,
> + CPUFREQ_TRANSITION_NOTIFIER);
> +
> + for_each_possible_cpu(cpu) {
> + cpu_state = data->cpu_state[cpu];
> + if (cpu_state) {
> + if (cpu_state->opp_table)
> + dev_pm_opp_put_opp_table(cpu_state->opp_table);
> + kfree(cpu_state);
> + cpu_state = NULL;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int devfreq_passive_event_handler(struct devfreq *devfreq,
> unsigned int event, void *data)
> {
> @@ -165,7 +398,7 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
> struct notifier_block *nb = &p_data->nb;
> int ret = 0;
>
> - if (!parent)
> + if (p_data->parent_type == DEVFREQ_PARENT_DEV && !parent)
> return -EPROBE_DEFER;
If you modify the devfreq_passive_event_handler() as following,
you can move this condition for DEVFREQ_PARENT_DEV into
(register|unregister)_parent_dev_notifier.
switch (event) {
case DEVFREQ_GOV_START:
ret = register_parent_dev_notifier(p_data);
break;
case DEVFREQ_GOV_STOP:
ret = unregister_parent_dev_notifier(p_data);
break;
default:
ret = -EINVAL;
break;
}
return ret;
>
> switch (event) {
> @@ -173,13 +406,24 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
> if (!p_data->this)
> p_data->this = devfreq;
>
> - nb->notifier_call = devfreq_passive_notifier_call;
> - ret = devfreq_register_notifier(parent, nb,
> - DEVFREQ_TRANSITION_NOTIFIER);
> + if (p_data->parent_type == DEVFREQ_PARENT_DEV) {
> + nb->notifier_call = devfreq_passive_notifier_call;
> + ret = devfreq_register_notifier(parent, nb,
> + DEVFREQ_TRANSITION_NOTIFIER);
> + } else if (p_data->parent_type == CPUFREQ_PARENT_DEV) {
> + ret = cpufreq_passive_register(&p_data);
I think that we better to collect the code related to notifier registration
into one function like devfreq_pass_register_notifier() instead of
cpufreq_passive_register() as following: I think it is more simple and readable.
If you have more proper function name of register_parent_dev_notifier,
please give your opinion.
int register_parent_dev_notifier(struct devfreq_passive_data **p_data)
switch (p_data->parent_type) {
case DEVFREQ_PARENT_DEV:
nb->notifier_call = devfreq_passive_notifier_call;
ret = devfreq_register_notifier(parent, nb,
break;
case CPUFREQ_PARENT_DEV:
cpufreq_register_notifier(...)
...
break;
}
> + } else {
> + ret = -EINVAL;
> + }
> break;
> case DEVFREQ_GOV_STOP:
> - WARN_ON(devfreq_unregister_notifier(parent, nb,
> - DEVFREQ_TRANSITION_NOTIFIER));
> + if (p_data->parent_type == DEVFREQ_PARENT_DEV)
> + WARN_ON(devfreq_unregister_notifier(parent, nb,
> + DEVFREQ_TRANSITION_NOTIFIER));
> + else if (p_data->parent_type == CPUFREQ_PARENT_DEV)
> + cpufreq_passive_unregister(&p_data);
> + else
> + ret = -EINVAL;
ditto. unregister_parent_dev_notifier(struct devfreq_passive_data **p_data)
> break;
> default:
> break;
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index a4b19d593151..04ce576fd6f1 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -278,6 +278,32 @@ struct devfreq_simple_ondemand_data {
>
> #if IS_ENABLED(CONFIG_DEVFREQ_GOV_PASSIVE)
> /**
> + * struct devfreq_cpu_state - holds the per-cpu state
> + * @freq: the current frequency of the cpu.
> + * @min_freq: the min frequency of the cpu.
> + * @max_freq: the max frequency of the cpu.
> + * @first_cpu: the cpumask of the first cpu of a policy.
> + * @dev: reference to cpu device.
> + * @opp_table: reference to cpu opp table.
> + *
> + * This structure stores the required cpu_state of a cpu.
> + * This is auto-populated by the governor.
> + */
> +struct devfreq_cpu_state {> + unsigned int freq;
It is better to change from 'freq' to 'curr_freq'
for more correct expression.
> + unsigned int min_freq;
> + unsigned int max_freq;
> + unsigned int first_cpu;
> + struct device *dev;
How about changing the name 'dev' to 'cpu_dev'?
> + struct opp_table *opp_table;
> +};
devfreq_cpu_state is only handled by within driver/devfreq/governor_passive.c.
So, you can move it into drivers/devfreq/governor_passive.c
and just add the definition into include/linux/devfreq.h as following:
It is able to prevent the access of variable of 'struct devfreq_cpu_state'
outside.
struct devfreq_cpu_state;
> +
> +enum devfreq_parent_dev_type {
> + DEVFREQ_PARENT_DEV,
> + CPUFREQ_PARENT_DEV,
> +};
> +
> +/**
> * struct devfreq_passive_data - ``void *data`` fed to struct devfreq
> * and devfreq_add_device
> * @parent: the devfreq instance of parent device.
> @@ -288,13 +314,15 @@ struct devfreq_simple_ondemand_data {
> * using governors except for passive governor.
> * If the devfreq device has the specific method to decide
> * the next frequency, should use this callback.
> - * @this: the devfreq instance of own device.
> - * @nb: the notifier block for DEVFREQ_TRANSITION_NOTIFIER list
> + * @parent_type parent type of the device
Need to add ':' at the end of word. -> "parent_type:".
> + * @this: the devfreq instance of own device.
> + * @nb: the notifier block for DEVFREQ_TRANSITION_NOTIFIER list
I knew that you make them with same indentation.
But, actually, it is not related to this patch like clean-up code.
Even if it is not pretty, you better to don't touch 'this' and 'nb' indentaion.
> + * @cpu_state: the state min/max/current frequency of all online cpu's
> *
> * The devfreq_passive_data have to set the devfreq instance of parent
> * device with governors except for the passive governor. But, don't need to
> - * initialize the 'this' and 'nb' field because the devfreq core will handle
> - * them.
> + * initialize the 'this', 'nb' and 'cpu_state' field because the devfreq core
> + * will handle them.
> */
> struct devfreq_passive_data {
> /* Should set the devfreq instance of parent device */
> @@ -303,9 +331,13 @@ struct devfreq_passive_data {
> /* Optional callback to decide the next frequency of passvice device */
> int (*get_target_freq)(struct devfreq *this, unsigned long *freq);
>
> + /* Should set the type of parent device */
> + enum devfreq_parent_dev_type parent_type;
> +
> /* For passive governor's internal use. Don't need to set them */
> struct devfreq *this;
> struct notifier_block nb;
> + struct devfreq_cpu_state *cpu_state[NR_CPUS];
> };
> #endif
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH] iommu/arm-smmu: Add module parameter to set msi iova address
From: Srinath Mannam @ 2020-05-28 5:15 UTC (permalink / raw)
To: Robin Murphy
Cc: Jean-Philippe Brucker, Alex Williamson, Joerg Roedel,
Linux Kernel Mailing List, Eric Auger, iommu, BCM Kernel Feedback,
Will Deacon, Linux ARM
In-Reply-To: <f9b221cf-1c7f-9f95-133b-dca65197b6c2@arm.com>
On Wed, May 27, 2020 at 11:00 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
Thanks Robin for your quick response.
> On 2020-05-27 17:03, Srinath Mannam wrote:
> > This patch gives the provision to change default value of MSI IOVA base
> > to platform's suitable IOVA using module parameter. The present
> > hardcoded MSI IOVA base may not be the accessible IOVA ranges of platform.
>
> That in itself doesn't seem entirely unreasonable; IIRC the current
> address is just an arbitrary choice to fit nicely into Qemu's memory
> map, and there was always the possibility that it wouldn't suit everything.
>
> > Since commit aadad097cd46 ("iommu/dma: Reserve IOVA for PCIe inaccessible
> > DMA address"), inaccessible IOVA address ranges parsed from dma-ranges
> > property are reserved.
>
> That, however, doesn't seem to fit here; iommu-dma maps MSI doorbells
> dynamically, so they aren't affected by reserved regions any more than
> regular DMA pages are. In fact, it explicitly ignores the software MSI
> region, since as the comment says, it *is* the software that manages those.
Yes you are right, we don't see any issues with kernel drivers(PCI EP) because
MSI IOVA allocated dynamically by honouring reserved regions same as DMA pages.
>
> The MSI_IOVA_BASE region exists for VFIO, precisely because in that case
> the kernel *doesn't* control the address space, but still needs some way
> to steal a bit of it for MSIs that the guest doesn't necessarily know
> about, and give userspace a fighting chance of knowing what it's taken.
> I think at the time we discussed the idea of adding something to the
> VFIO uapi such that userspace could move this around if it wanted or
> needed to, but decided we could live without that initially. Perhaps now
> the time has come?
Yes, we see issues only with user-space drivers(DPDK) in which MSI_IOVA_BASE
region is considered to map MSI registers. This patch helps us to fix the issue.
Thanks,
Srinath.
>
> Robin.
>
> > If any platform has the limitaion to access default MSI IOVA, then it can
> > be changed using "arm-smmu.msi_iova_base=0xa0000000" command line argument.
> >
> > Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
> > ---
> > drivers/iommu/arm-smmu.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> > index 4f1a350..5e59c9d 100644
> > --- a/drivers/iommu/arm-smmu.c
> > +++ b/drivers/iommu/arm-smmu.c
> > @@ -72,6 +72,9 @@ static bool disable_bypass =
> > module_param(disable_bypass, bool, S_IRUGO);
> > MODULE_PARM_DESC(disable_bypass,
> > "Disable bypass streams such that incoming transactions from devices that are not attached to an iommu domain will report an abort back to the device and will not be allowed to pass through the SMMU.");
> > +static unsigned long msi_iova_base = MSI_IOVA_BASE;
> > +module_param(msi_iova_base, ulong, S_IRUGO);
> > +MODULE_PARM_DESC(msi_iova_base, "msi iova base address.");
> >
> > struct arm_smmu_s2cr {
> > struct iommu_group *group;
> > @@ -1566,7 +1569,7 @@ static void arm_smmu_get_resv_regions(struct device *dev,
> > struct iommu_resv_region *region;
> > int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
> >
> > - region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
> > + region = iommu_alloc_resv_region(msi_iova_base, MSI_IOVA_LENGTH,
> > prot, IOMMU_RESV_SW_MSI);
> > if (!region)
> > return;
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 0/7] Statsfs: a new ram-based file system for Linux kernel statistics
From: Paolo Bonzini @ 2020-05-28 5:22 UTC (permalink / raw)
To: David Ahern, Jakub Kicinski, Emanuele Giuseppe Esposito
Cc: linux-s390, kvm, linux-doc, netdev, Emanuele Giuseppe Esposito,
linux-kernel, kvm-ppc, Jonathan Adams, Christian Borntraeger,
Andrew Lunn, Alexander Viro, David Rientjes, linux-fsdevel,
linux-mips, linuxppc-dev, linux-arm-kernel, Jim Mattson
In-Reply-To: <b6fa4439-c6b8-63a4-84fd-fbac3d4f10fd@gmail.com>
On 28/05/20 00:21, David Ahern wrote:
> On 5/27/20 3:07 PM, Paolo Bonzini wrote:
>> I see what you meant now. statsfs can also be used to enumerate objects
>> if one is so inclined (with the prototype in patch 7, for example, each
>> network interface becomes a directory).
>
> there are many use cases that have 100's to 1000's have network devices.
> Having a sysfs entry per device already bloats memory usage for these
> use cases; another filesystem with an entry per device makes that worse.
> Really the wrong direction for large scale systems.
Hi David,
IMO the important part for now is having a flexible kernel API for
exposing statistics across multiple subsystems, so that they can be
harvested in an efficient way. The userspace API is secondary, and
multiple APIs can be added to cater for different usecases.
For example, as of the first five patches the memory usage is the same
as what is now in the mainline kernel, since all the patchset does is
take existing debugfs inodes and move them to statsfs. I agree that, if
the concept is extended to the whole kernel, scalability and memory
usage becomes an issue; and indeed, the long-term plan is to support a
binary format that is actually _more_ efficient than the status quo for
large scale systems.
In the meanwhile, the new filesystem can be disabled (see the difference
between "STATS_FS" and "STATS_FS_API") if it imposes undesirable overhead.
Thanks,
Paolo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v10] Add matrix keypad driver support for Mediatek SoCs
From: Fengping Yu @ 2020-05-28 5:33 UTC (permalink / raw)
To: Yingjoe Chen, Dmitry Torokhov, Andy Shevchenko, Marco Felsch
Cc: linux-mediatek, linux-arm-kernel, linux-input
Change since v9:
- modify KEYBOARD_MTK_KPD config dependent item
- remove wakeup member of mtk_keypad struct
- remove default pinctrl state set
- modify request irq failed return value
- add space of kepad matching table
- modify align issue
fengping.yu (3):
dt-bindings: Add keypad devicetree documentation
drivers: input: keyboard: Add mtk keypad driver
configs: defconfig: Add CONFIG_KEYBOARD_MTK_KPD=m
.../devicetree/bindings/input/mtk-kpd.yaml | 95 +++++++++
arch/arm64/configs/defconfig | 1 +
drivers/input/keyboard/Kconfig | 11 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/mtk-kpd.c | 201 ++++++++++++++++++
5 files changed, 309 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.yaml
create mode 100644 drivers/input/keyboard/mtk-kpd.c
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v10 1/3] dt-bindings: Add keypad devicetree documentation
From: Fengping Yu @ 2020-05-28 5:33 UTC (permalink / raw)
To: Yingjoe Chen, Dmitry Torokhov, Andy Shevchenko, Marco Felsch
Cc: fengping.yu, linux-mediatek, linux-arm-kernel, linux-input
In-Reply-To: <20200528053344.25936-1-fengping.yu@mediatek.com>
From: "fengping.yu" <fengping.yu@mediatek.com>
Add Mediatek matrix keypad dt-bindings doc as yaml schema.
Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
---
.../devicetree/bindings/input/mtk-kpd.yaml | 95 +++++++++++++++++++
1 file changed, 95 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/mtk-kpd.yaml
diff --git a/Documentation/devicetree/bindings/input/mtk-kpd.yaml b/Documentation/devicetree/bindings/input/mtk-kpd.yaml
new file mode 100644
index 000000000000..586cd196dd00
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/mtk-kpd.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+version: 1
+
+$id: http://devicetree.org/schemas/input/mtk-keypad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek's Keypad Controller device tree bindings
+
+maintainer:
+ - Fengping Yu <fengping.yu@mediatek.com>
+
+description: |
+ Mediatek's Keypad controller is used to interface a SoC with a matrix-type
+ keypad device. The keypad controller supports multiple row and column lines.
+ A key can be placed at each intersection of a unique row and a unique column.
+ The keypad controller can sense a key-press and key-release and report the
+ event using a interrupt to the cpu.
+
+properties:
+ compatible:
+ oneOf:
+ - const: "mediatek,mt6779-keypad"
+ - const: "mediatek,mt6873-keypad"
+
+ clock-names:
+ description: Names of the clocks listed in clocks property in the same order
+ maxItems: 1
+
+ clocks:
+ description: Must contain one entry, for the module clock
+ refs: devicetree/bindings/clocks/clock-bindings.txt for details.
+
+ interrupts:
+ description: A single interrupt specifier
+ maxItems: 1
+
+ linux,keymap:
+ description: The keymap for keys as described in the binding document
+ refs: devicetree/bindings/input/matrix-keymap.txt
+ minItems: 1
+ maxItems: 16
+
+ pinctrl-0:
+ description: Specify pin control groups used for this controller
+ refs: devicetree/bindings/pinctrl/pinctrl-bindings.txt
+
+ pinctrl-names:
+ description: Names for optional pin modes
+ maxItems: 1
+
+ reg:
+ description: The base address of the Keypad register bank
+ maxItems: 1
+
+ wakeup-source:
+ description: use any event on keypad as wakeup event
+ type: boolean
+
+ keypad,num-columns:
+ description: Number of column lines connected to the keypad controller,
+ it is not equal to PCB columns number, instead you should add required value
+ for each IC
+
+ keypad,num-rows:
+ description: Number of row lines connected to the keypad controller, it is
+ not equal to PCB rows number, instead you should add required value for each IC
+
+ mediatek,debounce-us:
+ description: Debounce interval in microseconds
+ maximum: 256000
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - linux,keymap
+ - pinctrl
+ - clocks
+ - clock-names
+
+examples:
+ - |
+
+ keypad: kp@10010000 {
+ compatible = "mediatek,mt6779-keypad";
+ reg = <0 0x10010000 0 0x1000>;
+ linux,keymap = < MATRIX_KEY(0x00, 0x00, KEY_VOLUMEDOWN) >;
+ interrupts = <GIC_SPI 75 IRQ_TYPE_EDGE_FALLING>;
+ clocks = <&clk26m>;
+ clock-names = "kpd";
+ pinctrl-names = "default";
+ pinctrl-0 = <&kpd_gpios_def_cfg>;
+ };
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v10 2/3] drivers: input: keyboard: Add mtk keypad driver
From: Fengping Yu @ 2020-05-28 5:33 UTC (permalink / raw)
To: Yingjoe Chen, Dmitry Torokhov, Andy Shevchenko, Marco Felsch
Cc: fengping.yu, linux-mediatek, linux-arm-kernel, linux-input
In-Reply-To: <20200528053344.25936-1-fengping.yu@mediatek.com>
From: "fengping.yu" <fengping.yu@mediatek.com>
This adds matrix keypad support for Mediatek SoCs.
Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
---
drivers/input/keyboard/Kconfig | 11 ++
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/mtk-kpd.c | 201 +++++++++++++++++++++++++++++++
3 files changed, 213 insertions(+)
create mode 100644 drivers/input/keyboard/mtk-kpd.c
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 28de965a08d5..c55230c4c344 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -782,6 +782,17 @@ config KEYBOARD_BCM
To compile this driver as a module, choose M here: the
module will be called bcm-keypad.
+config KEYBOARD_MTK_KPD
+ tristate "MediaTek Keypad Support"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ select CONFIG_REGMAP_MMIO
+ select INPUT_MATRIXKMAP
+ help
+ Say Y here if you want to use the keypad on MediaTek SoCs.
+ If unsure, say N.
+ To compile this driver as a module, choose M here: the
+ module will be called mtk-kpd.
+
config KEYBOARD_MTK_PMIC
tristate "MediaTek PMIC keys support"
depends on MFD_MT6397
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 1d689fdd5c00..6c9d852c377e 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_KEYBOARD_MATRIX) += matrix_keypad.o
obj-$(CONFIG_KEYBOARD_MAX7359) += max7359_keypad.o
obj-$(CONFIG_KEYBOARD_MCS) += mcs_touchkey.o
obj-$(CONFIG_KEYBOARD_MPR121) += mpr121_touchkey.o
+obj-$(CONFIG_KEYBOARD_MTK_KPD) += mtk-kpd.o
obj-$(CONFIG_KEYBOARD_MTK_PMIC) += mtk-pmic-keys.o
obj-$(CONFIG_KEYBOARD_NEWTON) += newtonkbd.o
obj-$(CONFIG_KEYBOARD_NOMADIK) += nomadik-ske-keypad.o
diff --git a/drivers/input/keyboard/mtk-kpd.c b/drivers/input/keyboard/mtk-kpd.c
new file mode 100644
index 000000000000..5123dbb00ad1
--- /dev/null
+++ b/drivers/input/keyboard/mtk-kpd.c
@@ -0,0 +1,201 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 MediaTek Inc.
+ * Author Terry Chang <terry.chang@mediatek.com>
+ */
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/input/matrix_keypad.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define MTK_KPD_NAME "mtk-kpd"
+#define MTK_KPD_MEM 0x0004
+#define MTK_KPD_DEBOUNCE 0x0018
+#define MTK_KPD_DEBOUNCE_MASK GENMASK(13, 0)
+#define MTK_KPD_DEBOUNCE_MAX_US 256000
+#define MTK_KPD_NUM_MEMS 5
+#define MTK_KPD_NUM_BITS 136 /* 4*32+8 MEM5 only use 8 BITS */
+
+struct mtk_keypad {
+ struct regmap *regmap;
+ struct input_dev *input_dev;
+ struct clk *clk;
+ void __iomem *base;
+ u32 n_rows;
+ u32 n_cols;
+ DECLARE_BITMAP(keymap_state, MTK_KPD_NUM_BITS);
+};
+
+static const struct regmap_config keypad_regmap_cfg = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = sizeof(u32),
+ .max_register = 36,
+};
+
+static irqreturn_t kpd_irq_handler(int irq, void *dev_id)
+{
+ struct mtk_keypad *keypad = dev_id;
+ unsigned short *keycode = keypad->input_dev->keycode;
+ DECLARE_BITMAP(new_state, MTK_KPD_NUM_BITS);
+ DECLARE_BITMAP(change, MTK_KPD_NUM_BITS);
+ int bit_nr;
+ int pressed;
+ unsigned short code;
+
+ regmap_raw_read(keypad->regmap, MTK_KPD_MEM,
+ new_state, MTK_KPD_NUM_MEMS);
+
+ bitmap_xor(change, new_state, keypad->keymap_state, MTK_KPD_NUM_BITS);
+
+ for_each_set_bit(bit_nr, change, MTK_KPD_NUM_BITS) {
+ /* 1: not pressed, 0: pressed */
+ pressed = !test_bit(bit_nr, new_state);
+ dev_dbg(&keypad->input_dev->dev, "%s",
+ pressed ? "pressed" : "released");
+
+ /* 32bit register only use low 16bit as keypad mem register */
+ code = keycode[bit_nr - 16 * (BITS_TO_U32(bit_nr) - 1)];
+
+ input_report_key(keypad->input_dev, code, pressed);
+ input_sync(keypad->input_dev);
+
+ dev_dbg(&keypad->input_dev->dev,
+ "report Linux keycode = %d\n", code);
+ }
+
+ bitmap_copy(keypad->keymap_state, new_state, MTK_KPD_NUM_BITS);
+
+ return IRQ_HANDLED;
+}
+
+static void kpd_clk_disable(void *data)
+{
+ clk_disable_unprepare(data);
+}
+
+static int kpd_pdrv_probe(struct platform_device *pdev)
+{
+ struct mtk_keypad *keypad;
+ unsigned int irq;
+ u32 debounce;
+ int ret;
+
+ keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
+ if (!keypad)
+ return -ENOMEM;
+
+ keypad->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(keypad->base))
+ return PTR_ERR(keypad->base);
+
+ keypad->regmap = devm_regmap_init_mmio(&pdev->dev,
+ keypad->base,
+ &keypad_regmap_cfg);
+ if (IS_ERR(keypad->regmap)) {
+ dev_err(&pdev->dev,
+ "regmap init failed:%ld\n", PTR_ERR(keypad->regmap));
+ return PTR_ERR(keypad->regmap);
+ }
+
+ bitmap_fill(keypad->keymap_state, MTK_KPD_NUM_BITS);
+
+ keypad->input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!keypad->input_dev) {
+ dev_err(&pdev->dev, "Failed to allocate input dev\n");
+ return -ENOMEM;
+ }
+
+ keypad->input_dev->name = MTK_KPD_NAME;
+ keypad->input_dev->id.bustype = BUS_HOST;
+
+ ret = matrix_keypad_parse_properties(&pdev->dev, &keypad->n_rows,
+ &keypad->n_cols);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to parse keypad params\n");
+ return ret;
+ }
+
+ if (device_property_read_u32(&pdev->dev, "mediatek,debounce-us",
+ &debounce))
+ debounce = 16000;
+
+ if (debounce > MTK_KPD_DEBOUNCE_MAX_US) {
+ dev_err(&pdev->dev, "Debounce time exceeds the maximum allowed time %dus\n",
+ MTK_KPD_DEBOUNCE_MAX_US);
+ return -EINVAL;
+ }
+
+ dev_dbg(&pdev->dev, "n_row=%d n_col=%d debounce=%d\n",
+ keypad->n_rows, keypad->n_cols, debounce);
+
+ ret = matrix_keypad_build_keymap(NULL, NULL,
+ keypad->n_rows,
+ keypad->n_cols,
+ NULL,
+ keypad->input_dev);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to build keymap\n");
+ return ret;
+ }
+
+ regmap_write(keypad->regmap, MTK_KPD_DEBOUNCE,
+ debounce * 32 / 1000 & MTK_KPD_DEBOUNCE_MASK);
+
+ keypad->clk = devm_clk_get(&pdev->dev, "kpd");
+ if (IS_ERR(keypad->clk))
+ return keypad->clk;
+
+ ret = clk_prepare_enable(keypad->clk);
+ if (ret) {
+ dev_err(&pdev->dev, "cannot prepare/enable keypad clock\n");
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(&pdev->dev, kpd_clk_disable,
+ keypad->clk);
+ if (ret)
+ return ret;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ ret = devm_request_threaded_irq(&pdev->dev, irq,
+ NULL, kpd_irq_handler, 0,
+ MTK_KPD_NAME, keypad);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to request IRQ#%d:%d\n",
+ irq, ret);
+ return ret;
+ }
+
+ ret = input_register_device(keypad->input_dev);
+ if (ret)
+ dev_err(&pdev->dev, "Failed to register device\n");
+
+ return ret;
+}
+
+static const struct of_device_id kpd_of_match[] = {
+ { .compatible = "mediatek,mt6779-keypad" },
+ { .compatible = "mediatek,mt6873-keypad" },
+ { /* sentinel */ }
+};
+
+static struct platform_driver kpd_pdrv = {
+ .probe = kpd_pdrv_probe,
+ .driver = {
+ .name = MTK_KPD_NAME,
+ .of_match_table = kpd_of_match,
+ },
+};
+module_platform_driver(kpd_pdrv);
+
+MODULE_AUTHOR("Mediatek Corporation");
+MODULE_DESCRIPTION("MTK Keypad (KPD) Driver");
+MODULE_LICENSE("GPL");
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v10 3/3] configs: defconfig: Add CONFIG_KEYBOARD_MTK_KPD=m
From: Fengping Yu @ 2020-05-28 5:33 UTC (permalink / raw)
To: Yingjoe Chen, Dmitry Torokhov, Andy Shevchenko, Marco Felsch
Cc: fengping.yu, linux-mediatek, linux-arm-kernel, linux-input
In-Reply-To: <20200528053344.25936-1-fengping.yu@mediatek.com>
From: "fengping.yu" <fengping.yu@mediatek.com>
Add Mediatek matrix keypad support in defconfig.
Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 24e534d85045..112ced090b21 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -349,6 +349,7 @@ CONFIG_KEYBOARD_GPIO=y
CONFIG_KEYBOARD_SNVS_PWRKEY=m
CONFIG_KEYBOARD_IMX_SC_KEY=m
CONFIG_KEYBOARD_CROS_EC=y
+CONFIG_KEYBOARD_MTK_KPD=m
CONFIG_INPUT_TOUCHSCREEN=y
CONFIG_TOUCHSCREEN_ATMEL_MXT=m
CONFIG_INPUT_MISC=y
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 1/2] drm/mxsfb: Call drm_crtc_vblank_on/off
From: Daniel Vetter @ 2020-05-28 5:46 UTC (permalink / raw)
To: DRI Development
Cc: Marek Vasut, Fabio Estevam, Daniel Vetter,
Intel Graphics Development, Stefan Agner, NXP Linux Team,
Pengutronix Kernel Team, Daniel Vetter, Shawn Guo, Sascha Hauer,
linux-arm-kernel
In-Reply-To: <20200527094757.1414174-1-daniel.vetter@ffwll.ch>
On Wed, May 27, 2020 at 11:47:56AM +0200, Daniel Vetter wrote:
> mxsfb has vblank support, is atomic, but doesn't call
> drm_crtc_vblank_on/off as it should. Not good.
>
> With my next patch to add the drm_crtc_vblank_reset to helpers this
> means not even the very first crtc enabling will vblanks work anymore,
> since they'll just stay off forever.
>
> Since mxsfb doesn't have any vblank waits of its own in the
> enable/disable flow, nor an enable/disable_vblank callback we can do
> the on/off as the first respectively last operation, and it should all
> work.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org
Ping for some ack/review on this one here, it's holding up the subsystem
wide fix in patch 2.
Thanks, Daniel
> ---
> drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index 497cf443a9af..1891cd6deb2f 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -124,6 +124,7 @@ static void mxsfb_pipe_enable(struct drm_simple_display_pipe *pipe,
> drm_panel_prepare(mxsfb->panel);
> mxsfb_crtc_enable(mxsfb);
> drm_panel_enable(mxsfb->panel);
> + drm_crtc_vblank_on(&pipe->crtc);
> }
>
> static void mxsfb_pipe_disable(struct drm_simple_display_pipe *pipe)
> @@ -133,6 +134,7 @@ static void mxsfb_pipe_disable(struct drm_simple_display_pipe *pipe)
> struct drm_crtc *crtc = &pipe->crtc;
> struct drm_pending_vblank_event *event;
>
> + drm_crtc_vblank_off(&pipe->crtc);
> drm_panel_disable(mxsfb->panel);
> mxsfb_crtc_disable(mxsfb);
> drm_panel_unprepare(mxsfb->panel);
> --
> 2.26.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [V9, 1/2] media: dt-bindings: media: i2c: Document OV02A10 bindings
From: Dongchun Zhu @ 2020-05-28 5:53 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree, Andy Shevchenko, Louis Kuo,
srv_heupstream, Linus Walleij,
Shengnan Wang (王圣男), Tomasz Figa,
Bartosz Golaszewski, Sj Huang, Nicolas Boichat,
moderated list:ARM/Mediatek SoC support, Sakari Ailus,
dongchun.zhu, Matthias Brugger, Cao Bing Bu,
Mauro Carvalho Chehab,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Linux Media Mailing List
In-Reply-To: <CAL_Jsq+sN0SVidTrY0ODXEkzkxYFvG1FTnL0oRQBSKf=ynLdyQ@mail.gmail.com>
Hi Rob,
On Wed, 2020-05-27 at 09:27 -0600, Rob Herring wrote:
> On Wed, May 27, 2020 at 2:51 AM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> >
> > Hi Rob,
> >
> > Thanks for the review. Please see my replies below.
> >
> > On Tue, 2020-05-26 at 12:28 -0600, Rob Herring wrote:
> > > On Sat, May 23, 2020 at 04:41:02PM +0800, Dongchun Zhu wrote:
> > > > Add DT bindings documentation for Omnivision OV02A10 image sensor.
> > > >
> > > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > > ---
> > > > .../bindings/media/i2c/ovti,ov02a10.yaml | 172 +++++++++++++++++++++
> > > > MAINTAINERS | 7 +
> > > > 2 files changed, 179 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml
> > > > new file mode 100644
> > > > index 0000000..56f31b5
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml
> > > > @@ -0,0 +1,172 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > +# Copyright (c) 2020 MediaTek Inc.
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov02a10.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Omnivision OV02A10 CMOS Sensor Device Tree Bindings
> > > > +
> > > > +maintainers:
> > > > + - Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > > +
> > > > +description: |-
> > > > + The Omnivision OV02A10 is a low-cost, high performance, 1/5-inch, 2 megapixel
> > > > + image sensor, which is the latest production derived from Omnivision's CMOS
> > > > + image sensor technology. Ihis chip supports high frame rate speeds up to 30fps
> > > > + @ 1600x1200 (UXGA) resolution transferred over a 1-lane MIPI interface. The
> > > > + sensor output is available via CSI-2 serial data output.
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + const: ovti,ov02a10
> > > > +
> > > > + reg:
> > > > + maxItems: 1
> > > > +
> > > > + clocks:
> > > > + items:
> > > > + - description: top mux camtg clock
> > > > + - description: divider clock
> > > > +
> > > > + clock-names:
> > > > + items:
> > > > + - const: eclk
> > > > + - const: freq_mux
> > > > +
> > > > + clock-frequency:
> > > > + description:
> > > > + Frequency of the eclk clock in Hertz.
> > > > +
> >
> > Rob, shall we use 'maxItems: 1' to constrain property: clock-frequency?
>
> No, because it is a scalar, not an array.
>
Got it.
> > Or could we adopt 'clock-frequency: true' directly here?
>
> As-is is fine.
>
Understood.
> > > > + dovdd-supply:
> > > > + description:
> > > > + Definition of the regulator used as Digital I/O voltage supply.
> > > > +
> >
> > Shall we add 'maxItems: 1' here?
>
> No, supplies are always singular.
>
Fine.
> >
> > > > + avdd-supply:
> > > > + description:
> > > > + Definition of the regulator used as Analog voltage supply.
> > > > +
> >
> > Ditto.
> >
> > > > + dvdd-supply:
> > > > + description:
> > > > + Definition of the regulator used as Digital core voltage supply.
> > > > +
> >
> > Ditto.
> >
> > > > + powerdown-gpios:
> > > > + description:
> > > > + Must be the device tree identifier of the GPIO connected to the
> > > > + PD_PAD pin. This pin is used to place the OV02A10 into Standby mode
> > > > + or Shutdown mode. As the line is active low, it should be
> > > > + marked GPIO_ACTIVE_LOW.
> > >
> > > Need to define how many GPIOs ('maxItems: 1')
> > >
> >
> > It would be fixed like this in next release.
> > powerdown-gpios:
> > maxItems: 1
> > description:
> > Must be the device tree identifier of the GPIO connected to the
> > PD_PAD pin. This pin is used to place the OV02A10 into Standby mode
> > or Shutdown mode. As the line is active low, it should be
> > marked GPIO_ACTIVE_LOW.
> >
Tomasz, I don't know whether you noticed this description.
Here I simply defined one necessary GPIO polarity in DT which driver
settings need to follow.
> > > > +
> > > > + reset-gpios:
> > > > + description:
> > > > + Must be the device tree identifier of the GPIO connected to the
> > > > + RST_PD pin. If specified, it will be asserted during driver probe.
> > > > + As the line is active high, it should be marked GPIO_ACTIVE_HIGH.
> > >
> > > Here too.
> > >
> >
> > Similar as 'powerdown-gpios'.
> > Fixed in next release.
> >
> > > > +
> > > > + rotation:
> > > > + description:
> > > > + Definition of the sensor's placement.
> > > > + allOf:
> > > > + - $ref: "/schemas/types.yaml#/definitions/uint32"
> > > > + - enum:
> > > > + - 0 # Sensor Mounted Upright
> > > > + - 180 # Sensor Mounted Upside Down
> > > > + default: 0
> > > > +
> > > > + ovti,mipi-tx-speed:
> > > > + description:
> > > > + Indication of MIPI transmission speed select, which is to control D-PHY
> > > > + timing setting by adjusting MIPI clock voltage to improve the clock
> > > > + driver capability.
> > > > + allOf:
> > > > + - $ref: "/schemas/types.yaml#/definitions/uint32"
> > > > + - enum:
> > > > + - 0 # 20MHz - 30MHz
> > > > + - 1 # 30MHz - 50MHz
> > > > + - 2 # 50MHz - 75MHz
> > > > + - 3 # 75MHz - 100MHz
> > > > + - 4 # 100MHz - 130MHz
> > > > + default: 3
> > > > +
> > > > + # See ../video-interfaces.txt for details
> > > > + port:
> > > > + type: object
> > > > + additionalProperties: false
> > >
> > > Should have a description of what data the port has.
> > >
> >
> > It would be updated as below in next release.
> > port:
> > type: object
> > additionalProperties: false
> > description:
> > Input port node, single endpoint describing the CSI-2 transmitter.
>
> Output?
>
Sorry for the typo.
Yes, this should be output port node.
> >
> > > > +
> > > > + properties:
> > > > + endpoint:
> > > > + type: object
> > > > + additionalProperties: false
> > > > +
> > > > + properties:
> >
> > Actually I wonder whether we need to declare 'clock-lanes' here?
>
> Yes, if you are using it.
>
Looking back to the upstreamed patches, it seems that there is a deep
divide in the setting of 'clock-lanes' property.
As the last comment I just posed, for OV02A10, 'clock-lanes' should be
set to <0> (clock lane on hardware lane 0).
But here we may follow IMX219 patch, which removed 'clock-lanes'
property due to the fact that sensor hardware does not support lane
reordering.
Rob, Sakari, what's your opinions?
> Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 06/12] PM / devfreq: Add cpu based scaling support to passive_governor
From: Chanwoo Choi @ 2020-05-28 6:14 UTC (permalink / raw)
To: Andrew-sh.Cheng, MyungJoo Ham, Kyungmin Park, Rob Herring,
Mark Rutland, Matthias Brugger, Rafael J . Wysocki, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Liam Girdwood, Mark Brown
Cc: devicetree, srv_heupstream, linux-pm, linux-kernel,
linux-mediatek, Sibi Sankar, linux-arm-kernel
In-Reply-To: <20200520034307.20435-7-andrew-sh.cheng@mediatek.com>
Hi Andrew-sh.Cheng,
Thanks for your posting. I like this approach absolutely.
I think that it is necessary. When I developed the embedded product,
I needed this feature always.
I add the comments on below.
And the following email is not valid. So, I dropped this email
from Cc list.
Saravana Kannan <skannan@codeaurora.org>
On 5/20/20 12:43 PM, Andrew-sh.Cheng wrote:
> From: Saravana Kannan <skannan@codeaurora.org>
>
> Many CPU architectures have caches that can scale independent of the
> CPUs. Frequency scaling of the caches is necessary to make sure that the
> cache is not a performance bottleneck that leads to poor performance and
> power. The same idea applies for RAM/DDR.
>
> To achieve this, this patch adds support for cpu based scaling to the
> passive governor. This is accomplished by taking the current frequency
> of each CPU frequency domain and then adjust the frequency of the cache
> (or any devfreq device) based on the frequency of the CPUs. It listens
> to CPU frequency transition notifiers to keep itself up to date on the
> current CPU frequency.
>
> To decide the frequency of the device, the governor does one of the
> following:
> * Derives the optimal devfreq device opp from required-opps property of
> the parent cpu opp_table.
>
> * Scales the device frequency in proportion to the CPU frequency. So, if
> the CPUs are running at their max frequency, the device runs at its
> max frequency. If the CPUs are running at their min frequency, the
> device runs at its min frequency. It is interpolated for frequencies
> in between.
>
> Andrew-sh.Cheng change
> dev_pm_opp_xlate_opp to dev_pm_opp_xlate_required_opp devfreq->max_freq
> to devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value
> for kernel-5.7
>
> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
> [Sibi: Integrated cpu-freqmap governor into passive_governor]
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com>
> ---
> drivers/devfreq/Kconfig | 2 +
> drivers/devfreq/governor_passive.c | 278 ++++++++++++++++++++++++++++++++++---
> include/linux/devfreq.h | 40 +++++-
> 3 files changed, 299 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 0b1df12e0f21..d9067950af6a 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -73,6 +73,8 @@ config DEVFREQ_GOV_PASSIVE
> device. This governor does not change the frequency by itself
> through sysfs entries. The passive governor recommends that
> devfreq device uses the OPP table to get the frequency/voltage.
> + Alternatively the governor can also be chosen to scale based on
> + the online CPUs current frequency.
>
> comment "DEVFREQ Drivers"
>
> diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
> index 2d67d6c12dce..7dcda02a5bb7 100644
> --- a/drivers/devfreq/governor_passive.c
> +++ b/drivers/devfreq/governor_passive.c
> @@ -8,11 +8,89 @@
> */
>
> #include <linux/module.h>
> +#include <linux/cpu.h>
> +#include <linux/cpufreq.h>
> +#include <linux/cpumask.h>
> #include <linux/device.h>
> #include <linux/devfreq.h>
> +#include <linux/slab.h>
> #include "governor.h"
>
> -static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> +static unsigned int xlate_cpufreq_to_devfreq(struct devfreq_passive_data *data,
Need to change 'unsigned int' to 'unsigned long'.
> + unsigned int cpu)
> +{
> + unsigned int cpu_min, cpu_max, dev_min, dev_max, cpu_percent, max_state;
Better to define them separately as following and then need to rename
the variable. Usually, use the 'min_freq' and 'max_freq' word for
the minimum/maximum frequency.
unsigned int cpu_min_freq, cpu_max_freq, cpu_curr_freq, cpu_percent;
unsigned long dev_min_freq, dev_max_freq, dev_max_state,
The devfreq used 'unsigned long'. The cpufreq used 'unsigned long'
and 'unsigned int'. You need to handle them properly.
> + struct devfreq_cpu_state *cpu_state = data->cpu_state[cpu];
> + struct devfreq *devfreq = (struct devfreq *)data->this;
> + unsigned long *freq_table = devfreq->profile->freq_table;
In this function, use 'cpu' work for cpufreq and use 'dev' for devfreq.
So, I think 'dev_freq_table' is proper name instead of 'freq_table'
for the readability.
freq_table -> dev_freq_table
> + struct dev_pm_opp *opp = NULL, *cpu_opp = NULL;
In the get_target_freq_with_devfreq(), use 'p_opp' indicating
the OPP of parent device. For the consistency, I think that
use 'p_opp' instead of 'cpu_opp'.
> + unsigned long cpu_freq, freq;
Define the 'cpu_freq' on above with cpu_min_freq/cpu_max_freq definition.
cpu_freq -> cpu_curr_freq.
> +
> + if (!cpu_state || cpu_state->first_cpu != cpu ||
> + !cpu_state->opp_table || !devfreq->opp_table)
> + return 0;
> +
> + cpu_freq = cpu_state->freq * 1000;
> + cpu_opp = devfreq_recommended_opp(cpu_state->dev, &cpu_freq, 0);
> + if (IS_ERR(cpu_opp))
> + return 0;
> +
> + opp = dev_pm_opp_xlate_required_opp(cpu_state->opp_table,
> + devfreq->opp_table, cpu_opp);
> + dev_pm_opp_put(cpu_opp);
> +
> + if (!IS_ERR(opp)) {
> + freq = dev_pm_opp_get_freq(opp);
> + dev_pm_opp_put(opp);
Better to add the 'out' goto statement.
If you use 'goto out', you can reduce the one indentation
without 'else' statement.
> + } else {
As I commented, when dev_pm_opp_xlate_required_opp() return successfully
, use 'goto out'. We can remove 'else' and then reduce the unneeded indentation.
> + /* Use Interpolation if required opps is not available */
> + cpu_min = cpu_state->min_freq;
> + cpu_max = cpu_state->max_freq;
> + cpu_freq = cpu_state->freq;
> +
> + if (freq_table) {
> + /* Get minimum frequency according to sorting order */
> + max_state = freq_table[devfreq->profile->max_state - 1];
> + if (freq_table[0] < max_state) {
> + dev_min = freq_table[0];
> + dev_max = max_state;
> + } else {
> + dev_min = max_state;
> + dev_max = freq_table[0];
> + }
> + } else {
> + if (devfreq->user_max_freq_req.data.freq.qos->max_freq.target_value
> + <= devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value)
> + return 0;
> + dev_min =
> + devfreq->user_min_freq_req.data.freq.qos->min_freq.target_value;
> + dev_max =
> + devfreq->user_max_freq_req.data.freq.qos->max_freq.target_value;
I think it is not proper to access the variable of pm_qos structure directly.
Instead of direct access, you have to use the exported PM QoS function such as
- pm_qos_read_value(devfreq->dev.parent, DEV_PM_QOS_MIN_FREQUENCY);
- pm_qos_read_value(devfreq->dev.parent, DEV_PM_QOS_MAX_FREQUENCY);
> + }
> + cpu_percent = ((cpu_freq - cpu_min) * 100) / cpu_max - cpu_min;
> + freq = dev_min + mult_frac(dev_max - dev_min, cpu_percent, 100);
> + }
I think that you better to add 'out' jump label as following:
out:
> +
> + return freq;
> +}
> +
> +static int get_target_freq_with_cpufreq(struct devfreq *devfreq,
> + unsigned long *freq)
> +{
> + struct devfreq_passive_data *p_data =
> + (struct devfreq_passive_data *)devfreq->data;
> + unsigned int cpu, target_freq = 0;
Need to define 'target_freq' with 'unsigned long' type.
> +
> + for_each_online_cpu(cpu)
> + target_freq = max(target_freq,
> + xlate_cpufreq_to_devfreq(p_data, cpu));
> +
> + *freq = target_freq;
> +
> + return 0;
> +}
> +
> +static int get_target_freq_with_devfreq(struct devfreq *devfreq,
> unsigned long *freq)
> {
> struct devfreq_passive_data *p_data
> @@ -23,16 +101,6 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> int i, count, ret = 0;
>
> /*
> - * If the devfreq device with passive governor has the specific method
> - * to determine the next frequency, should use the get_target_freq()
> - * of struct devfreq_passive_data.
> - */
> - if (p_data->get_target_freq) {
> - ret = p_data->get_target_freq(devfreq, freq);
> - goto out;
> - }
> -
> - /*
> * If the parent and passive devfreq device uses the OPP table,
> * get the next frequency by using the OPP table.
> */
> @@ -102,6 +170,37 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> return ret;
> }
>
> +static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> + unsigned long *freq)
> +{
> + struct devfreq_passive_data *p_data =
> + (struct devfreq_passive_data *)devfreq->data;
> + int ret;
> +
> + /*
> + * If the devfreq device with passive governor has the specific method
> + * to determine the next frequency, should use the get_target_freq()
> + * of struct devfreq_passive_data.
> + */
> + if (p_data->get_target_freq)
> + return p_data->get_target_freq(devfreq, freq);
> +
> + switch (p_data->parent_type) {
> + case DEVFREQ_PARENT_DEV:
> + ret = get_target_freq_with_devfreq(devfreq, freq);
> + break;
> + case CPUFREQ_PARENT_DEV:
> + ret = get_target_freq_with_cpufreq(devfreq, freq);
> + break;
> + default:
> + ret = -EINVAL;
> + dev_err(&devfreq->dev, "Invalid parent type\n");
> + break;
> + }
> +
> + return ret;
> +}
> +
> static int update_devfreq_passive(struct devfreq *devfreq, unsigned long freq)
> {
> int ret;
> @@ -156,6 +255,140 @@ static int devfreq_passive_notifier_call(struct notifier_block *nb,
> return NOTIFY_DONE;
> }
>
> +static int cpufreq_passive_notifier_call(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
> + struct devfreq_passive_data *data =
> + container_of(nb, struct devfreq_passive_data, nb);
> + struct devfreq *devfreq = (struct devfreq *)data->this;
> + struct devfreq_cpu_state *cpu_state;
> + struct cpufreq_freqs *freq = ptr;
How about changing 'freq' to 'cpu_freqs'?
In the drivers/cpufreq/cpufreq.c, use 'freqs' name indicating
the instance of 'struct cpufreq_freqs'. And in order to
identfy, how about adding 'cpu_' prefix for variable name?
> + unsigned int current_freq;
Need to define curr_freq with 'unsigned long' type
and better to use 'curr_freq' variable name.
> + int ret;
> +
> + if (event != CPUFREQ_POSTCHANGE || !freq ||
> + !data->cpu_state[freq->policy->cpu])
> + return 0;
> +
> + cpu_state = data->cpu_state[freq->policy->cpu];
> + if (cpu_state->freq == freq->new)
> + return 0;
> +
> + /* Backup current freq and pre-update cpu state freq*/
> + current_freq = cpu_state->freq;
> + cpu_state->freq = freq->new;
> +
> + mutex_lock(&devfreq->lock);
> + ret = update_devfreq(devfreq);
> + mutex_unlock(&devfreq->lock);
> + if (ret) {
> + cpu_state->freq = current_freq;
> + dev_err(&devfreq->dev, "Couldn't update the frequency.\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int cpufreq_passive_register(struct devfreq_passive_data **p_data)
> +{
> + struct devfreq_passive_data *data = *p_data;
> + struct devfreq *devfreq = (struct devfreq *)data->this;
> + struct device *dev = devfreq->dev.parent;
> + struct opp_table *opp_table = NULL;
> + struct devfreq_cpu_state *state;
For the readability, I thinkt 'cpu_state' is proper instead of 'state'.
> + struct cpufreq_policy *policy;
> + struct device *cpu_dev;
> + unsigned int cpu;
> + int ret;
> +
> + get_online_cpus();
Add blank line.
> + data->nb.notifier_call = cpufreq_passive_notifier_call;
> + ret = cpufreq_register_notifier(&data->nb,
> + CPUFREQ_TRANSITION_NOTIFIER);
> + if (ret) {
> + dev_err(dev, "Couldn't register cpufreq notifier.\n");
> + data->nb.notifier_call = NULL;
> + goto out;
> + }
> +
> + /* Populate devfreq_cpu_state */
> + for_each_online_cpu(cpu) {
> + if (data->cpu_state[cpu])
> + continue;
> +
> + policy = cpufreq_cpu_get(cpu);
cpufreq_cpu_get() might return 'NULL'. I think you need to handle
return value as following:
if (!policy) {
ret = -EINVAL;
goto out;
} else if (PTR_ERR(policy) == -EPROBE_DEFER) {
goto out;
} else if (IS_ERR(policy) {
ret = PTR_ERR(policy);
dev_err(dev, "Couldn't get the cpufreq_poliy.\n");
goto out;
}
If cpufreq_cpu_get() return successfully, to do next.
It reduces the one indentaion.
> + if (policy) {
> + state = kzalloc(sizeof(*state), GFP_KERNEL);
> + if (!state) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + cpu_dev = get_cpu_device(cpu);
> + if (!cpu_dev) {
> + dev_err(dev, "Couldn't get cpu device.\n");
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + opp_table = dev_pm_opp_get_opp_table(cpu_dev);
> + if (IS_ERR(devfreq->opp_table)) {
> + ret = PTR_ERR(opp_table);
> + goto out;
> + }
> +
> + state->dev = cpu_dev;
> + state->opp_table = opp_table;
> + state->first_cpu = cpumask_first(policy->related_cpus);
> + state->freq = policy->cur;
> + state->min_freq = policy->cpuinfo.min_freq;
> + state->max_freq = policy->cpuinfo.max_freq;
> + data->cpu_state[cpu] = state;
Add blank line.
> + cpufreq_cpu_put(policy);
> + } else {
> + ret = -EPROBE_DEFER;
> + goto out;
> + }
> + }
Add blank line.
> +out:
> + put_online_cpus();
> + if (ret)
> + return ret;
> +
> + /* Update devfreq */
> + mutex_lock(&devfreq->lock);
> + ret = update_devfreq(devfreq);
> + mutex_unlock(&devfreq->lock);
> + if (ret)
> + dev_err(dev, "Couldn't update the frequency.\n");
> +
> + return ret;
> +}
> +
> +static int cpufreq_passive_unregister(struct devfreq_passive_data **p_data)
> +{
> + struct devfreq_passive_data *data = *p_data;
> + struct devfreq_cpu_state *cpu_state;
> + int cpu;
> +
> + if (data->nb.notifier_call)
> + cpufreq_unregister_notifier(&data->nb,
> + CPUFREQ_TRANSITION_NOTIFIER);
> +
> + for_each_possible_cpu(cpu) {
> + cpu_state = data->cpu_state[cpu];
> + if (cpu_state) {
> + if (cpu_state->opp_table)
> + dev_pm_opp_put_opp_table(cpu_state->opp_table);
> + kfree(cpu_state);
> + cpu_state = NULL;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int devfreq_passive_event_handler(struct devfreq *devfreq,
> unsigned int event, void *data)
> {
> @@ -165,7 +398,7 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
> struct notifier_block *nb = &p_data->nb;
> int ret = 0;
>
> - if (!parent)
> + if (p_data->parent_type == DEVFREQ_PARENT_DEV && !parent)
> return -EPROBE_DEFER;
If you modify the devfreq_passive_event_handler() as following,
you can move this condition for DEVFREQ_PARENT_DEV into
(register|unregister)_parent_dev_notifier.
switch (event) {
case DEVFREQ_GOV_START:
ret = register_parent_dev_notifier(p_data);
break;
case DEVFREQ_GOV_STOP:
ret = unregister_parent_dev_notifier(p_data);
break;
default:
ret = -EINVAL;
break;
}
return ret;
>
> switch (event) {
> @@ -173,13 +406,24 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
> if (!p_data->this)
> p_data->this = devfreq;
>
> - nb->notifier_call = devfreq_passive_notifier_call;
> - ret = devfreq_register_notifier(parent, nb,
> - DEVFREQ_TRANSITION_NOTIFIER);
> + if (p_data->parent_type == DEVFREQ_PARENT_DEV) {
> + nb->notifier_call = devfreq_passive_notifier_call;
> + ret = devfreq_register_notifier(parent, nb,
> + DEVFREQ_TRANSITION_NOTIFIER);
> + } else if (p_data->parent_type == CPUFREQ_PARENT_DEV) {
> + ret = cpufreq_passive_register(&p_data);
I think that we better to collect the code related to notifier registration
into one function like devfreq_pass_register_notifier() instead of
cpufreq_passive_register() as following: I think it is more simple and readable.
If you have more proper function name of register_parent_dev_notifier,
please give your opinion.
int register_parent_dev_notifier(struct devfreq_passive_data **p_data)
switch (p_data->parent_type) {
case DEVFREQ_PARENT_DEV:
nb->notifier_call = devfreq_passive_notifier_call;
ret = devfreq_register_notifier(parent, nb,
break;
case CPUFREQ_PARENT_DEV:
cpufreq_register_notifier(...)
...
break;
}
> + } else {
> + ret = -EINVAL;
> + }
> break;
> case DEVFREQ_GOV_STOP:
> - WARN_ON(devfreq_unregister_notifier(parent, nb,
> - DEVFREQ_TRANSITION_NOTIFIER));
> + if (p_data->parent_type == DEVFREQ_PARENT_DEV)
> + WARN_ON(devfreq_unregister_notifier(parent, nb,
> + DEVFREQ_TRANSITION_NOTIFIER));
> + else if (p_data->parent_type == CPUFREQ_PARENT_DEV)
> + cpufreq_passive_unregister(&p_data);
> + else
> + ret = -EINVAL;
ditto. unregister_parent_dev_notifier(struct devfreq_passive_data **p_data)
> break;
> default:
> break;
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index a4b19d593151..04ce576fd6f1 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -278,6 +278,32 @@ struct devfreq_simple_ondemand_data {
>
> #if IS_ENABLED(CONFIG_DEVFREQ_GOV_PASSIVE)
> /**
> + * struct devfreq_cpu_state - holds the per-cpu state
> + * @freq: the current frequency of the cpu.
> + * @min_freq: the min frequency of the cpu.
> + * @max_freq: the max frequency of the cpu.
> + * @first_cpu: the cpumask of the first cpu of a policy.
> + * @dev: reference to cpu device.
> + * @opp_table: reference to cpu opp table.
> + *
> + * This structure stores the required cpu_state of a cpu.
> + * This is auto-populated by the governor.
> + */
> +struct devfreq_cpu_state {> + unsigned int freq;
It is better to change from 'freq' to 'curr_freq'
for more correct expression.
> + unsigned int min_freq;
> + unsigned int max_freq;
> + unsigned int first_cpu;
> + struct device *dev;
How about changing the name 'dev' to 'cpu_dev'?
> + struct opp_table *opp_table;
> +};
devfreq_cpu_state is only handled by within driver/devfreq/governor_passive.c.
So, you can move it into drivers/devfreq/governor_passive.c
and just add the definition into include/linux/devfreq.h as following:
It is able to prevent the access of variable of 'struct devfreq_cpu_state'
outside.
struct devfreq_cpu_state;
> +
> +enum devfreq_parent_dev_type {
> + DEVFREQ_PARENT_DEV,
> + CPUFREQ_PARENT_DEV,
> +};
> +
> +/**
> * struct devfreq_passive_data - ``void *data`` fed to struct devfreq
> * and devfreq_add_device
> * @parent: the devfreq instance of parent device.
> @@ -288,13 +314,15 @@ struct devfreq_simple_ondemand_data {
> * using governors except for passive governor.
> * If the devfreq device has the specific method to decide
> * the next frequency, should use this callback.
> - * @this: the devfreq instance of own device.
> - * @nb: the notifier block for DEVFREQ_TRANSITION_NOTIFIER list
> + * @parent_type parent type of the device
Need to add ':' at the end of word. -> "parent_type:".
> + * @this: the devfreq instance of own device.
> + * @nb: the notifier block for DEVFREQ_TRANSITION_NOTIFIER list
I knew that you make them with same indentation.
But, actually, it is not related to this patch like clean-up code.
Even if it is not pretty, you better to don't touch 'this' and 'nb' indentaion.
> + * @cpu_state: the state min/max/current frequency of all online cpu's
> *
> * The devfreq_passive_data have to set the devfreq instance of parent
> * device with governors except for the passive governor. But, don't need to
> - * initialize the 'this' and 'nb' field because the devfreq core will handle
> - * them.
> + * initialize the 'this', 'nb' and 'cpu_state' field because the devfreq core
> + * will handle them.
> */
> struct devfreq_passive_data {
> /* Should set the devfreq instance of parent device */
> @@ -303,9 +331,13 @@ struct devfreq_passive_data {
> /* Optional callback to decide the next frequency of passvice device */
> int (*get_target_freq)(struct devfreq *this, unsigned long *freq);
>
> + /* Should set the type of parent device */
> + enum devfreq_parent_dev_type parent_type;
> +
> /* For passive governor's internal use. Don't need to set them */
> struct devfreq *this;
> struct notifier_block nb;
> + struct devfreq_cpu_state *cpu_state[NR_CPUS];
> };
> #endif
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 1/6] prctl.2: ffix use literal hyphens when referencing kernel docs
From: Michael Kerrisk (man-pages) @ 2020-05-28 6:05 UTC (permalink / raw)
To: Dave Martin; +Cc: linux-arch, linux-man, mtk.manpages, linux-arm-kernel
In-Reply-To: <1590614258-24728-2-git-send-email-Dave.Martin@arm.com>
On 5/27/20 11:17 PM, Dave Martin wrote:
> There is one case of a cross-reference to a kernel documentation
> filename that uses unescaped hyphens.
>
> To avoid misrendering, escape these as \- similarly to other
> instances.
>
> Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> ---
> man2/prctl.2 | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/man2/prctl.2 b/man2/prctl.2
> index 968a75a..dc99218 100644
> --- a/man2/prctl.2
> +++ b/man2/prctl.2
> @@ -1261,7 +1261,7 @@ This parameter may enforce a read-only policy which will result in the
> call failing with the error
> .BR ENXIO .
> For further details, see the kernel source file
> -.IR Documentation/admin-guide/kernel-parameters.txt .
> +.IR Documentation/admin\-guide/kernel\-parameters.txt .
> .\"
> .\" prctl PR_TASK_PERF_EVENTS_DISABLE
> .TP
Thanks, Dave. Applied.
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: linux-next: build warning after merge of the aspeed tree
From: Joel Stanley @ 2020-05-28 6:09 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Stephen Rothwell, devicetree, Andrew Jeffery,
Linux Kernel Mailing List, Vijay Khemka, Linux Next Mailing List,
Rob Herring, ARM, Olof Johansson, Manikandan Elumalai,
Devicetree Compiler, David Gibson
In-Reply-To: <CAK8P3a323rPCDDws+us4UYo7ZO6XvkZ13hBChZ40_DwCxBZj_g@mail.gmail.com>
On Fri, 22 May 2020 at 08:16, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Fri, May 22, 2020 at 2:16 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > On Wed, 20 May 2020 07:56:36 +0000 Joel Stanley <joel@jms.id.au> wrote:
> > > I've sent the patch so it applies to the dtc tree. It would be good to
> > > see that change propagate over to -next as others have reported this
> > > warning.
> >
> > These warnings now appear in the arm-soc tree.
>
> Right, I also saw them earlier.
>
> Joel, have you sent your patch to David Gibson for integration into
> upstream dtc?
> I don't know who sent the other patch, but as long as one of them
> gets merged, I'd hope we can pull that into kernel as well.
David asked for some extra features (and a typo fix) before he would
merge it. I'll take a look at that now.
The patch is 20200520075134.1048589-1-joel@jms.id.au on
devicetree-compiler@vger.kernel.org, which doesn't appear to be
archived on lore.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH RFCv2 9/9] arm64: Support async page fault
From: Gavin Shan @ 2020-05-28 6:14 UTC (permalink / raw)
To: Paolo Bonzini, kvmarm
Cc: maz, linux-kernel, shan.gavin, catalin.marinas, will,
linux-arm-kernel
In-Reply-To: <81adf013-3de7-23e6-7648-8aec821b033c@redhat.com>
Hi Paolo,
On 5/27/20 4:48 PM, Paolo Bonzini wrote:
> I definitely appreciate the work, but this is repeating most of the
> mistakes done in the x86 implementation. In particular:
>
> - the page ready signal can be done as an interrupt, rather than an
> exception. This is because "page ready" can be handled asynchronously,
> in contrast to "page not present" which must be done on the same
> instruction that triggers it. You can refer to the recent series from
> Vitaly Kuznetsov that switched "page ready" to an interrupt.
>
Yeah, page ready can be handled asynchronously. I think it would be
nice for x86/arm64 to share same design. x86 has 256 vectors and it
seems 0xec is picked for the purpose. However, arm64 doesn't have so
many (interrupt/exception) vectors and PPI might be appropriate for
the purpose if I'm correct, because it has same INTD for all CPUs.
From this point, it's similar to x86's vector. There are 16 PPIs, which
are in range of 16 to 31, and we might reserve one for this. According
to GICv3/v4 spec, 22 - 30 have been assigned.
> - the page not present is reusing the memory abort exception, and
> there's really no reason to do so. I think it would be best if ARM
> could reserve one ESR exception code for the hypervisor. Mark, any
> ideas how to proceed here?
>
Well, a subclass of ESR exception code, whose DFSC (Data Fault Status
Code) is 0x34, was taken for the purpose in RFCv1. The code is IMPDEF
one and Mark suggested not to do so. I agree the page not present needs a
separately subclass of exception. With that, there will be less conflicts
and complexity. However, the question is which subclass or DFSC code I should
used for the purpose.
> - for x86 we're also thinking of initiating the page fault from the
> exception handler, rather than doing so from the hypervisor before
> injecting the exception. If ARM leads the way here, we would do our
> best to share code when x86 does the same.
>
Sorry, Paolo, I don't follow your idea here. Could you please provide
more details?
> - do not bother with using KVM_ASYNC_PF_SEND_ALWAYS, it's a fringe case
> that adds a lot of complexity.
>
Yeah, I don't consider it so far.
> Also, please include me on further iterations of the series.
>
Sure.
Thanks,
Gavin
[...]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 2/4] PCI: mediatek: Use regmap to get shared pcie-cfg base
From: chuanjia.liu @ 2020-05-28 6:16 UTC (permalink / raw)
To: robh+dt, ryder.lee, matthias.bgg
Cc: devicetree, lorenzo.pieralisi, srv_heupstream, chuanjia.liu,
linux-pci, linux-kernel, jianjun.wang, linux-mediatek, yong.wu,
bhelgaas, linux-arm-kernel, amurray
In-Reply-To: <20200528061648.32078-1-chuanjia.liu@mediatek.com>
From: "chuanjia.liu" <Chuanjia.Liu@mediatek.com>
Use regmap to get shared pcie-cfg base and change
the method to get pcie irq.
Signed-off-by: chuanjia.liu <Chuanjia.Liu@mediatek.com>
---
drivers/pci/controller/pcie-mediatek.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index cb982891b22b..2268d6073eb6 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -14,6 +14,7 @@
#include <linux/irqchip/chained_irq.h>
#include <linux/irqdomain.h>
#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
#include <linux/msi.h>
#include <linux/module.h>
#include <linux/of_address.h>
@@ -23,6 +24,7 @@
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
#include <linux/reset.h>
#include "../pci.h"
@@ -205,6 +207,7 @@ struct mtk_pcie_port {
* struct mtk_pcie - PCIe host information
* @dev: pointer to PCIe device
* @base: IO mapped register base
+ * @cfg: IO mapped register map for PCIe config
* @free_ck: free-run reference clock
* @mem: non-prefetchable memory resource
* @ports: pointer to PCIe port information
@@ -214,6 +217,7 @@ struct mtk_pcie_port {
struct mtk_pcie {
struct device *dev;
void __iomem *base;
+ struct regmap *cfg;
struct clk *free_ck;
struct list_head ports;
@@ -650,7 +654,7 @@ static int mtk_pcie_setup_irq(struct mtk_pcie_port *port,
return err;
}
- port->irq = platform_get_irq(pdev, port->slot);
+ port->irq = platform_get_irq_byname(pdev, "pcie_irq");
irq_set_chained_handler_and_data(port->irq,
mtk_pcie_intr_handler, port);
@@ -673,12 +677,11 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
if (!mem)
return -EINVAL;
- /* MT7622 platforms need to enable LTSSM and ASPM from PCIe subsys */
- if (pcie->base) {
- val = readl(pcie->base + PCIE_SYS_CFG_V2);
- val |= PCIE_CSR_LTSSM_EN(port->slot) |
- PCIE_CSR_ASPM_L1_EN(port->slot);
- writel(val, pcie->base + PCIE_SYS_CFG_V2);
+ /* MT7622/MT7629 platforms need to enable LTSSM and ASPM. */
+ if (pcie->cfg) {
+ val = PCIE_CSR_LTSSM_EN(port->slot) |
+ PCIE_CSR_ASPM_L1_EN(port->slot);
+ regmap_update_bits(pcie->cfg, PCIE_SYS_CFG_V2, val, val);
}
/* Assert all reset signals */
@@ -984,6 +987,7 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)
struct device *dev = pcie->dev;
struct platform_device *pdev = to_platform_device(dev);
struct resource *regs;
+ struct device_node *cfg_node;
int err;
/* get shared registers, which are optional */
@@ -996,6 +1000,13 @@ static int mtk_pcie_subsys_powerup(struct mtk_pcie *pcie)
}
}
+ cfg_node = of_parse_phandle(dev->of_node, "mediatek,pcie-cfg", 0);
+ if (cfg_node) {
+ pcie->cfg = syscon_node_to_regmap(cfg_node);
+ if (IS_ERR(pcie->cfg))
+ return PTR_ERR(pcie->cfg);
+ }
+
pcie->free_ck = devm_clk_get(dev, "free_ck");
if (IS_ERR(pcie->free_ck)) {
if (PTR_ERR(pcie->free_ck) == -EPROBE_DEFER)
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 3/4] arm64: dts: mediatek: Split PCIe node for MT2712/MT7622
From: chuanjia.liu @ 2020-05-28 6:16 UTC (permalink / raw)
To: robh+dt, ryder.lee, matthias.bgg
Cc: devicetree, lorenzo.pieralisi, srv_heupstream, chuanjia.liu,
linux-pci, linux-kernel, jianjun.wang, linux-mediatek, yong.wu,
bhelgaas, linux-arm-kernel, amurray
In-Reply-To: <20200528061648.32078-1-chuanjia.liu@mediatek.com>
From: "chuanjia.liu" <Chuanjia.Liu@mediatek.com>
There are two independent PCIe controllers in MT2712/MT7622 platform,
and each of them should contain an independent MSI domain.
In current architecture, MSI domain will be inherited from the root
bridge, and all of the devices will share the same MSI domain.
Hence that, the PCIe devices will not work properly if the irq number
which required is more than 32.
Split the PCIe node for MT2712/MT7622 platform to fix MSI issue and
comply with the hardware design.
Signed-off-by: chuanjia.liu <Chuanjia.Liu@mediatek.com>
---
arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 75 +++++++++++--------
.../dts/mediatek/mt7622-bananapi-bpi-r64.dts | 16 ++--
arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts | 6 +-
arch/arm64/boot/dts/mediatek/mt7622.dtsi | 68 +++++++++++------
4 files changed, 96 insertions(+), 69 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
index 2cd8b33886e5..ab27ff4a869e 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -797,60 +797,73 @@
};
};
- pcie: pcie@11700000 {
+ pcie1: pcie@112ff000 {
compatible = "mediatek,mt2712-pcie";
device_type = "pci";
- reg = <0 0x11700000 0 0x1000>,
- <0 0x112ff000 0 0x1000>;
- reg-names = "port0", "port1";
+ reg = <0 0x112ff000 0 0x1000>;
+ reg-names = "port1";
#address-cells = <3>;
#size-cells = <2>;
- interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&topckgen CLK_TOP_PE2_MAC_P0_SEL>,
- <&topckgen CLK_TOP_PE2_MAC_P1_SEL>,
- <&pericfg CLK_PERI_PCIE0>,
+ interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "pcie_irq";
+ clocks = <&topckgen CLK_TOP_PE2_MAC_P1_SEL>,
<&pericfg CLK_PERI_PCIE1>;
- clock-names = "sys_ck0", "sys_ck1", "ahb_ck0", "ahb_ck1";
- phys = <&u3port0 PHY_TYPE_PCIE>, <&u3port1 PHY_TYPE_PCIE>;
- phy-names = "pcie-phy0", "pcie-phy1";
+ clock-names = "sys_ck1", "ahb_ck1";
+ phys = <&u3port1 PHY_TYPE_PCIE>;
+ phy-names = "pcie-phy1";
bus-range = <0x00 0xff>;
- ranges = <0x82000000 0 0x20000000 0x0 0x20000000 0 0x10000000>;
+ ranges = <0x82000000 0 0x11400000 0x0 0x11400000 0 0x300000>;
+ status = "disabled";
- pcie0: pcie@0,0 {
- device_type = "pci";
- status = "disabled";
- reg = <0x0000 0 0 0 0>;
+ slot1: pcie@1,0 {
+ reg = <0x0800 0 0 0 0>;
#address-cells = <3>;
#size-cells = <2>;
#interrupt-cells = <1>;
ranges;
interrupt-map-mask = <0 0 0 7>;
- interrupt-map = <0 0 0 1 &pcie_intc0 0>,
- <0 0 0 2 &pcie_intc0 1>,
- <0 0 0 3 &pcie_intc0 2>,
- <0 0 0 4 &pcie_intc0 3>;
- pcie_intc0: interrupt-controller {
+ interrupt-map = <0 0 0 1 &pcie_intc1 0>,
+ <0 0 0 2 &pcie_intc1 1>,
+ <0 0 0 3 &pcie_intc1 2>,
+ <0 0 0 4 &pcie_intc1 3>;
+ pcie_intc1: interrupt-controller {
interrupt-controller;
#address-cells = <0>;
#interrupt-cells = <1>;
};
};
+ };
- pcie1: pcie@1,0 {
- device_type = "pci";
- status = "disabled";
- reg = <0x0800 0 0 0 0>;
+ pcie0: pcie@11700000 {
+ compatible = "mediatek,mt2712-pcie";
+ device_type = "pci";
+ reg = <0 0x11700000 0 0x1000>;
+ reg-names = "port0";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "pcie_irq";
+ clocks = <&topckgen CLK_TOP_PE2_MAC_P0_SEL>,
+ <&pericfg CLK_PERI_PCIE0>;
+ clock-names = "sys_ck0", "ahb_ck0";
+ phys = <&u3port0 PHY_TYPE_PCIE>;
+ phy-names = "pcie-phy0";
+ bus-range = <0x00 0xff>;
+ ranges = <0x82000000 0 0x20000000 0x0 0x20000000 0 0x10000000>;
+ status = "disabled";
+
+ slot0: pcie@0,0 {
+ reg = <0x0000 0 0 0 0>;
#address-cells = <3>;
#size-cells = <2>;
#interrupt-cells = <1>;
ranges;
interrupt-map-mask = <0 0 0 7>;
- interrupt-map = <0 0 0 1 &pcie_intc1 0>,
- <0 0 0 2 &pcie_intc1 1>,
- <0 0 0 3 &pcie_intc1 2>,
- <0 0 0 4 &pcie_intc1 3>;
- pcie_intc1: interrupt-controller {
+ interrupt-map = <0 0 0 1 &pcie_intc0 0>,
+ <0 0 0 2 &pcie_intc0 1>,
+ <0 0 0 3 &pcie_intc0 2>,
+ <0 0 0 4 &pcie_intc0 3>;
+ pcie_intc0: interrupt-controller {
interrupt-controller;
#address-cells = <0>;
#interrupt-cells = <1>;
diff --git a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
index 83e10591e0e5..7574d88cc46a 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts
@@ -207,18 +207,16 @@
};
};
-&pcie {
+&pcie0 {
pinctrl-names = "default";
- pinctrl-0 = <&pcie0_pins>, <&pcie1_pins>;
+ pinctrl-0 = <&pcie0_pins>;
status = "okay";
+};
- pcie@0,0 {
- status = "okay";
- };
-
- pcie@1,0 {
- status = "okay";
- };
+&pcie1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie1_pins>;
+ status = "okay";
};
&pio {
diff --git a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
index 3f783348c66a..2e6b4e37cb7d 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
+++ b/arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts
@@ -183,14 +183,10 @@
};
};
-&pcie {
+&pcie0 {
pinctrl-names = "default";
pinctrl-0 = <&pcie0_pins>;
status = "okay";
-
- pcie@0,0 {
- status = "okay";
- };
};
&pio {
diff --git a/arch/arm64/boot/dts/mediatek/mt7622.dtsi b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
index 339dc9f88f43..d5131c8b6a79 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
@@ -766,45 +766,41 @@
#reset-cells = <1>;
};
- pcie: pcie@1a140000 {
+ pciecfg: pciecfg@1a140000 {
+ compatible = "mediatek,mt7622-pciecfg", "syscon";
+ reg = <0 0x1a140000 0 0x1000>;
+ };
+
+ pcie0: pcie@1a143000 {
compatible = "mediatek,mt7622-pcie";
device_type = "pci";
- reg = <0 0x1a140000 0 0x1000>,
- <0 0x1a143000 0 0x1000>,
- <0 0x1a145000 0 0x1000>;
- reg-names = "subsys", "port0", "port1";
+ reg = <0 0x1a143000 0 0x1000>;
+ reg-names = "port0";
+ mediatek,pcie-cfg = <&pciecfg>;
#address-cells = <3>;
#size-cells = <2>;
- interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_LOW>,
- <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
+ interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "pcie_irq";
clocks = <&pciesys CLK_PCIE_P0_MAC_EN>,
- <&pciesys CLK_PCIE_P1_MAC_EN>,
- <&pciesys CLK_PCIE_P0_AHB_EN>,
<&pciesys CLK_PCIE_P0_AHB_EN>,
<&pciesys CLK_PCIE_P0_AUX_EN>,
- <&pciesys CLK_PCIE_P1_AUX_EN>,
<&pciesys CLK_PCIE_P0_AXI_EN>,
- <&pciesys CLK_PCIE_P1_AXI_EN>,
<&pciesys CLK_PCIE_P0_OBFF_EN>,
- <&pciesys CLK_PCIE_P1_OBFF_EN>,
- <&pciesys CLK_PCIE_P0_PIPE_EN>,
- <&pciesys CLK_PCIE_P1_PIPE_EN>;
- clock-names = "sys_ck0", "sys_ck1", "ahb_ck0", "ahb_ck1",
- "aux_ck0", "aux_ck1", "axi_ck0", "axi_ck1",
- "obff_ck0", "obff_ck1", "pipe_ck0", "pipe_ck1";
+ <&pciesys CLK_PCIE_P0_PIPE_EN>;
+ clock-names = "sys_ck0", "ahb_ck0", "aux_ck0",
+ "axi_ck0", "obff_ck0", "pipe_ck0";
+
power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
bus-range = <0x00 0xff>;
- ranges = <0x82000000 0 0x20000000 0x0 0x20000000 0 0x10000000>;
+ ranges = <0x82000000 0 0x20000000 0x0 0x20000000 0 0x8000000>;
status = "disabled";
- pcie0: pcie@0,0 {
+ slot0: pcie@0,0 {
reg = <0x0000 0 0 0 0>;
#address-cells = <3>;
#size-cells = <2>;
#interrupt-cells = <1>;
ranges;
- status = "disabled";
-
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0 0 0 1 &pcie_intc0 0>,
<0 0 0 2 &pcie_intc0 1>,
@@ -816,15 +812,39 @@
#interrupt-cells = <1>;
};
};
+ };
- pcie1: pcie@1,0 {
+ pcie1: pcie@1a145000 {
+ compatible = "mediatek,mt7622-pcie";
+ device_type = "pci";
+ reg = <0 0x1a145000 0 0x1000>;
+ reg-names = "port1";
+ mediatek,pcie-cfg = <&pciecfg>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ interrupts = <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "pcie_irq";
+ clocks = <&pciesys CLK_PCIE_P1_MAC_EN>,
+ /* designer has connect RC1 with p0_ahb clock */
+ <&pciesys CLK_PCIE_P0_AHB_EN>,
+ <&pciesys CLK_PCIE_P1_AUX_EN>,
+ <&pciesys CLK_PCIE_P1_AXI_EN>,
+ <&pciesys CLK_PCIE_P1_OBFF_EN>,
+ <&pciesys CLK_PCIE_P1_PIPE_EN>;
+ clock-names = "sys_ck1", "ahb_ck1", "aux_ck1",
+ "axi_ck1", "obff_ck1", "pipe_ck1";
+
+ power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
+ bus-range = <0x00 0xff>;
+ ranges = <0x82000000 0 0x28000000 0x0 0x28000000 0 0x8000000>;
+ status = "disabled";
+
+ slot1: pcie@1,0 {
reg = <0x0800 0 0 0 0>;
#address-cells = <3>;
#size-cells = <2>;
#interrupt-cells = <1>;
ranges;
- status = "disabled";
-
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0 0 0 1 &pcie_intc1 0>,
<0 0 0 2 &pcie_intc1 1>,
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 0/4] Spilt PCIe node to comply with hardware design
From: chuanjia.liu @ 2020-05-28 6:16 UTC (permalink / raw)
To: robh+dt, ryder.lee, matthias.bgg
Cc: devicetree, lorenzo.pieralisi, srv_heupstream, chuanjia.liu,
linux-pci, linux-kernel, jianjun.wang, linux-mediatek, yong.wu,
bhelgaas, linux-arm-kernel, amurray
There are two independent PCIe controllers in MT2712/MT7622 platform,
and each of them should contain an independent MSI domain.
In current architecture, MSI domain will be inherited from the root
bridge, and all of the devices will share the same MSI domain.
Hence that, the PCIe devices will not work properly if the irq number
which required is more than 32.
Split the PCIe node for MT2712/MT7622 platform to fix MSI issue and
comply with the hardware design.
change note:
v2: change the allocation of mt2712 PCIe MMIO space due to the allcation
size is not right in v1.
chuanjia.liu (4):
dt-bindings: PCI: Mediatek: Update PCIe binding
PCI: mediatek: Use regmap to get shared pcie-cfg base
arm64: dts: mediatek: Split PCIe node for MT2712/MT7622
ARM: dts: mediatek: Update mt7629 PCIe node
.../bindings/pci/mediatek-pcie-cfg.yaml | 38 +++++
.../devicetree/bindings/pci/mediatek-pcie.txt | 144 +++++++++++-------
arch/arm/boot/dts/mt7629-rfb.dts | 3 +-
arch/arm/boot/dts/mt7629.dtsi | 23 +--
arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 75 +++++----
.../dts/mediatek/mt7622-bananapi-bpi-r64.dts | 16 +-
arch/arm64/boot/dts/mediatek/mt7622-rfb1.dts | 6 +-
arch/arm64/boot/dts/mediatek/mt7622.dtsi | 68 ++++++---
drivers/pci/controller/pcie-mediatek.c | 25 ++-
9 files changed, 258 insertions(+), 140 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 1/4] dt-bindings: PCI: Mediatek: Update PCIe binding
From: chuanjia.liu @ 2020-05-28 6:16 UTC (permalink / raw)
To: robh+dt, ryder.lee, matthias.bgg
Cc: devicetree, lorenzo.pieralisi, srv_heupstream, chuanjia.liu,
linux-pci, linux-kernel, jianjun.wang, linux-mediatek, yong.wu,
bhelgaas, linux-arm-kernel, amurray
In-Reply-To: <20200528061648.32078-1-chuanjia.liu@mediatek.com>
From: "chuanjia.liu" <Chuanjia.Liu@mediatek.com>
There are two independent PCIe controllers in MT2712/MT7622 platform,
and each of them should contain an independent MSI domain.
In current architecture, MSI domain will be inherited from the root
bridge, and all of the devices will share the same MSI domain.
Hence that, the PCIe devices will not work properly if the irq number
which required is more than 32.
Split the PCIe node for MT2712/MT7622 platform to fix MSI issue and
comply with the hardware design.
Signed-off-by: chuanjia.liu <Chuanjia.Liu@mediatek.com>
---
.../bindings/pci/mediatek-pcie-cfg.yaml | 38 +++++
.../devicetree/bindings/pci/mediatek-pcie.txt | 144 +++++++++++-------
2 files changed, 129 insertions(+), 53 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml b/Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
new file mode 100644
index 000000000000..4d2835ab4858
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/mediatek-pcie-cfg.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/mediatek-pcie-cfg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek PCIECFG controller
+
+maintainers:
+ - Chuanjia Liu <chuanjia.liu@mediatek.com>
+ - Jianjun Wang <jianjun.wang@mediatek.com>
+
+description: |
+ The MediaTek PCIECFG controller controls some feature about
+ LTSSM, ASPM and so on.
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - mediatek,mt7622-pciecfg
+ - mediatek,mt7629-pciecfg
+ - const: syscon
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+examples:
+ - |
+ pciecfg: pciecfg@1a140000 {
+ compatible = "mediatek,mt7622-pciecfg", "syscon";
+ reg = <0 0x1a140000 0 0x1000>;
+ };
+...
diff --git a/Documentation/devicetree/bindings/pci/mediatek-pcie.txt b/Documentation/devicetree/bindings/pci/mediatek-pcie.txt
index 7468d666763a..ddae110d4379 100644
--- a/Documentation/devicetree/bindings/pci/mediatek-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/mediatek-pcie.txt
@@ -8,7 +8,7 @@ Required properties:
"mediatek,mt7623-pcie"
"mediatek,mt7629-pcie"
- device_type: Must be "pci"
-- reg: Base addresses and lengths of the PCIe subsys and root ports.
+- reg: Base addresses and lengths of the root ports.
- reg-names: Names of the above areas to use during resource lookup.
- #address-cells: Address representation for root ports (must be 3)
- #size-cells: Size representation for root ports (must be 2)
@@ -19,10 +19,10 @@ Required properties:
- sys_ckN :transaction layer and data link layer clock
Required entries for MT2701/MT7623:
- free_ck :for reference clock of PCIe subsys
- Required entries for MT2712/MT7622:
+ Required entries for MT2712/MT7622/MT7629:
- ahb_ckN :AHB slave interface operating clock for CSR access and RC
initiated MMIO access
- Required entries for MT7622:
+ Required entries for MT7622/MT7629:
- axi_ckN :application layer MMIO channel operating clock
- aux_ckN :pe2_mac_bridge and pe2_mac_core operating clock when
pcie_mac_ck/pcie_pipe_ck is turned off
@@ -47,10 +47,13 @@ Required properties for MT7623/MT2701:
- reset-names: Must be "pcie-rst0", "pcie-rst1", "pcie-rstN".. based on the
number of root ports.
-Required properties for MT2712/MT7622:
+Required properties for MT2712/MT7622/MT7629:
-interrupts: A list of interrupt outputs of the controller, must have one
entry for each PCIe port
+Required properties for MT7622/MT7629:
+- mediatek,pcie-subsys: Should be a phandle of the pciecfg node.
+
In addition, the device tree node must have sub-nodes describing each
PCIe port interface, having the following mandatory properties:
@@ -143,56 +146,73 @@ Examples for MT7623:
Examples for MT2712:
- pcie: pcie@11700000 {
+ pcie1: pcie@112ff000 {
compatible = "mediatek,mt2712-pcie";
device_type = "pci";
- reg = <0 0x11700000 0 0x1000>,
- <0 0x112ff000 0 0x1000>;
- reg-names = "port0", "port1";
+ reg = <0 0x112ff000 0 0x1000>;
+ reg-names = "port1";
#address-cells = <3>;
#size-cells = <2>;
- interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&topckgen CLK_TOP_PE2_MAC_P0_SEL>,
- <&topckgen CLK_TOP_PE2_MAC_P1_SEL>,
- <&pericfg CLK_PERI_PCIE0>,
+ interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "pcie_irq";
+ clocks = <&topckgen CLK_TOP_PE2_MAC_P1_SEL>,
<&pericfg CLK_PERI_PCIE1>;
- clock-names = "sys_ck0", "sys_ck1", "ahb_ck0", "ahb_ck1";
- phys = <&pcie0_phy PHY_TYPE_PCIE>, <&pcie1_phy PHY_TYPE_PCIE>;
- phy-names = "pcie-phy0", "pcie-phy1";
+ clock-names = "sys_ck1", "ahb_ck1";
+ phys = <&u3port1 PHY_TYPE_PCIE>;
+ phy-names = "pcie-phy1";
bus-range = <0x00 0xff>;
- ranges = <0x82000000 0 0x20000000 0x0 0x20000000 0 0x10000000>;
+ ranges = <0x82000000 0 0x11400000 0x0 0x11400000 0 0x300000>;
+ status = "disabled";
- pcie0: pcie@0,0 {
- reg = <0x0000 0 0 0 0>;
+ slot1: pcie@1,0 {
+ reg = <0x0800 0 0 0 0>;
#address-cells = <3>;
#size-cells = <2>;
#interrupt-cells = <1>;
ranges;
interrupt-map-mask = <0 0 0 7>;
- interrupt-map = <0 0 0 1 &pcie_intc0 0>,
- <0 0 0 2 &pcie_intc0 1>,
- <0 0 0 3 &pcie_intc0 2>,
- <0 0 0 4 &pcie_intc0 3>;
- pcie_intc0: interrupt-controller {
+ interrupt-map = <0 0 0 1 &pcie_intc1 0>,
+ <0 0 0 2 &pcie_intc1 1>,
+ <0 0 0 3 &pcie_intc1 2>,
+ <0 0 0 4 &pcie_intc1 3>;
+ pcie_intc1: interrupt-controller {
interrupt-controller;
#address-cells = <0>;
#interrupt-cells = <1>;
};
};
+ };
- pcie1: pcie@1,0 {
- reg = <0x0800 0 0 0 0>;
+ pcie0: pcie@11700000 {
+ compatible = "mediatek,mt2712-pcie";
+ device_type = "pci";
+ reg = <0 0x11700000 0 0x1000>;
+ reg-names = "port0";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "pcie_irq";
+ clocks = <&topckgen CLK_TOP_PE2_MAC_P0_SEL>,
+ <&pericfg CLK_PERI_PCIE0>;
+ clock-names = "sys_ck0", "ahb_ck0";
+ phys = <&u3port0 PHY_TYPE_PCIE>;
+ phy-names = "pcie-phy0";
+ bus-range = <0x00 0xff>;
+ ranges = <0x82000000 0 0x20000000 0x0 0x20000000 0 0x10000000>;
+ status = "disabled";
+
+ slot0: pcie@0,0 {
+ reg = <0x0000 0 0 0 0>;
#address-cells = <3>;
#size-cells = <2>;
#interrupt-cells = <1>;
ranges;
interrupt-map-mask = <0 0 0 7>;
- interrupt-map = <0 0 0 1 &pcie_intc1 0>,
- <0 0 0 2 &pcie_intc1 1>,
- <0 0 0 3 &pcie_intc1 2>,
- <0 0 0 4 &pcie_intc1 3>;
- pcie_intc1: interrupt-controller {
+ interrupt-map = <0 0 0 1 &pcie_intc0 0>,
+ <0 0 0 2 &pcie_intc0 1>,
+ <0 0 0 3 &pcie_intc0 2>,
+ <0 0 0 4 &pcie_intc0 3>;
+ pcie_intc0: interrupt-controller {
interrupt-controller;
#address-cells = <0>;
#interrupt-cells = <1>;
@@ -202,39 +222,31 @@ Examples for MT2712:
Examples for MT7622:
- pcie: pcie@1a140000 {
+ pcie0: pcie@1a143000 {
compatible = "mediatek,mt7622-pcie";
device_type = "pci";
- reg = <0 0x1a140000 0 0x1000>,
- <0 0x1a143000 0 0x1000>,
- <0 0x1a145000 0 0x1000>;
- reg-names = "subsys", "port0", "port1";
+ reg = <0 0x1a143000 0 0x1000>;
+ reg-names = "port0";
+ mediatek,pcie-cfg = <&pciecfg>;
#address-cells = <3>;
#size-cells = <2>;
- interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_LOW>,
- <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
+ interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "pcie_irq";
clocks = <&pciesys CLK_PCIE_P0_MAC_EN>,
- <&pciesys CLK_PCIE_P1_MAC_EN>,
<&pciesys CLK_PCIE_P0_AHB_EN>,
- <&pciesys CLK_PCIE_P1_AHB_EN>,
<&pciesys CLK_PCIE_P0_AUX_EN>,
- <&pciesys CLK_PCIE_P1_AUX_EN>,
<&pciesys CLK_PCIE_P0_AXI_EN>,
- <&pciesys CLK_PCIE_P1_AXI_EN>,
<&pciesys CLK_PCIE_P0_OBFF_EN>,
- <&pciesys CLK_PCIE_P1_OBFF_EN>,
- <&pciesys CLK_PCIE_P0_PIPE_EN>,
- <&pciesys CLK_PCIE_P1_PIPE_EN>;
- clock-names = "sys_ck0", "sys_ck1", "ahb_ck0", "ahb_ck1",
- "aux_ck0", "aux_ck1", "axi_ck0", "axi_ck1",
- "obff_ck0", "obff_ck1", "pipe_ck0", "pipe_ck1";
- phys = <&pcie0_phy PHY_TYPE_PCIE>, <&pcie1_phy PHY_TYPE_PCIE>;
- phy-names = "pcie-phy0", "pcie-phy1";
+ <&pciesys CLK_PCIE_P0_PIPE_EN>;
+ clock-names = "sys_ck0", "ahb_ck0", "aux_ck0",
+ "axi_ck0", "obff_ck0", "pipe_ck0";
+
power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
bus-range = <0x00 0xff>;
- ranges = <0x82000000 0 0x20000000 0x0 0x20000000 0 0x10000000>;
+ ranges = <0x82000000 0 0x20000000 0 0x20000000 0 0x8000000>;
+ status = "disabled";
- pcie0: pcie@0,0 {
+ slot0: pcie@0,0 {
reg = <0x0000 0 0 0 0>;
#address-cells = <3>;
#size-cells = <2>;
@@ -251,8 +263,34 @@ Examples for MT7622:
#interrupt-cells = <1>;
};
};
+ };
+
+ pcie1: pcie@1a145000 {
+ compatible = "mediatek,mt7622-pcie";
+ device_type = "pci";
+ reg = <0 0x1a145000 0 0x1000>;
+ reg-names = "port1";
+ mediatek,pcie-cfg = <&pciecfg>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ interrupts = <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "pcie_irq";
+ clocks = <&pciesys CLK_PCIE_P1_MAC_EN>,
+ /* designer has connect RC1 with p0_ahb clock */
+ <&pciesys CLK_PCIE_P0_AHB_EN>,
+ <&pciesys CLK_PCIE_P1_AUX_EN>,
+ <&pciesys CLK_PCIE_P1_AXI_EN>,
+ <&pciesys CLK_PCIE_P1_OBFF_EN>,
+ <&pciesys CLK_PCIE_P1_PIPE_EN>;
+ clock-names = "sys_ck1", "ahb_ck1", "aux_ck1",
+ "axi_ck1", "obff_ck1", "pipe_ck1";
+
+ power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
+ bus-range = <0x00 0xff>;
+ ranges = <0x82000000 0 0x28000000 0 0x28000000 0 0x8000000>;
+ status = "disabled";
- pcie1: pcie@1,0 {
+ slot1: pcie@1,0 {
reg = <0x0800 0 0 0 0>;
#address-cells = <3>;
#size-cells = <2>;
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 4/4] ARM: dts: mediatek: Update mt7629 PCIe node
From: chuanjia.liu @ 2020-05-28 6:16 UTC (permalink / raw)
To: robh+dt, ryder.lee, matthias.bgg
Cc: devicetree, lorenzo.pieralisi, srv_heupstream, chuanjia.liu,
linux-pci, linux-kernel, jianjun.wang, linux-mediatek, yong.wu,
bhelgaas, linux-arm-kernel, amurray
In-Reply-To: <20200528061648.32078-1-chuanjia.liu@mediatek.com>
From: "chuanjia.liu" <Chuanjia.Liu@mediatek.com>
Remove unused property and add pciecfg node.
Signed-off-by: chuanjia.liu <Chuanjia.Liu@mediatek.com>
---
arch/arm/boot/dts/mt7629-rfb.dts | 3 ++-
arch/arm/boot/dts/mt7629.dtsi | 23 +++++++++++++----------
2 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/arch/arm/boot/dts/mt7629-rfb.dts b/arch/arm/boot/dts/mt7629-rfb.dts
index 9980c10c6e29..eb536cbebd9b 100644
--- a/arch/arm/boot/dts/mt7629-rfb.dts
+++ b/arch/arm/boot/dts/mt7629-rfb.dts
@@ -140,9 +140,10 @@
};
};
-&pcie {
+&pcie1 {
pinctrl-names = "default";
pinctrl-0 = <&pcie_pins>;
+ status = "okay";
};
&pciephy1 {
diff --git a/arch/arm/boot/dts/mt7629.dtsi b/arch/arm/boot/dts/mt7629.dtsi
index 5cbb3d244c75..94567307b842 100644
--- a/arch/arm/boot/dts/mt7629.dtsi
+++ b/arch/arm/boot/dts/mt7629.dtsi
@@ -360,16 +360,21 @@
#reset-cells = <1>;
};
- pcie: pcie@1a140000 {
+ pciecfg: pciecfg@1a140000 {
+ compatible = "mediatek,mt7629-pciecfg", "syscon";
+ reg = <0x1a140000 0x1000>;
+ };
+
+ pcie1: pcie@1a145000 {
compatible = "mediatek,mt7629-pcie";
device_type = "pci";
- reg = <0x1a140000 0x1000>,
- <0x1a145000 0x1000>;
- reg-names = "subsys","port1";
+ reg = <0x1a145000 0x1000>;
+ reg-names = "port1";
+ mediatek,pcie-cfg = <&pciecfg>;
#address-cells = <3>;
#size-cells = <2>;
- interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_LOW>,
- <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
+ interrupts = <GIC_SPI 229 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "pcie_irq";
clocks = <&pciesys CLK_PCIE_P1_MAC_EN>,
<&pciesys CLK_PCIE_P0_AHB_EN>,
<&pciesys CLK_PCIE_P1_AUX_EN>,
@@ -390,21 +395,19 @@
power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
bus-range = <0x00 0xff>;
ranges = <0x82000000 0 0x20000000 0x20000000 0 0x10000000>;
+ status = "disabled";
- pcie1: pcie@1,0 {
- device_type = "pci";
+ slot1: pcie@1,0 {
reg = <0x0800 0 0 0 0>;
#address-cells = <3>;
#size-cells = <2>;
#interrupt-cells = <1>;
ranges;
- num-lanes = <1>;
interrupt-map-mask = <0 0 0 7>;
interrupt-map = <0 0 0 1 &pcie_intc1 0>,
<0 0 0 2 &pcie_intc1 1>,
<0 0 0 3 &pcie_intc1 2>,
<0 0 0 4 &pcie_intc1 3>;
-
pcie_intc1: interrupt-controller {
interrupt-controller;
#address-cells = <0>;
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH RFCv2 7/9] kvm/arm64: Support async page fault
From: Gavin Shan @ 2020-05-28 6:32 UTC (permalink / raw)
To: Marc Zyngier, pbonzini
Cc: Mark Rutland, aarcange, drjones, suzuki.poulose, catalin.marinas,
linux-kernel, eric.auger, james.morse, shan.gavin, will, kvmarm,
linux-arm-kernel
In-Reply-To: <28c74819f42306e66370ddaf88f16918@kernel.org>
Hi Marc,
On 5/27/20 5:37 PM, Marc Zyngier wrote:
> On 2020-05-27 05:05, Gavin Shan wrote:
[...]
>>>> +struct kvm_vcpu_pv_apf_data {
>>>> + __u32 reason;
>>>> + __u8 pad[60];
>>>> + __u32 enabled;
>>>> +};
>>>
>>> What's all the padding for?
>>>
>>
>> The padding is ensure the @reason and @enabled in different cache
>> line. @reason is shared by host/guest, while @enabled is almostly
>> owned by guest.
>
> So you are assuming that a cache line is at most 64 bytes.
> It is actualy implementation defined, and you can probe for it
> by looking at the CTR_EL0 register. There are implementations
> ranging from 32 to 256 bytes in the wild, and let's not mention
> broken big-little implementations here.
>
> [...]
>
Ok, Thanks for your comments and hints.
>>>> +bool kvm_arch_can_inject_async_page_not_present(struct kvm_vcpu *vcpu)
>>>> +{
>>>> + u64 vbar, pc;
>>>> + u32 val;
>>>> + int ret;
>>>> +
>>>> + if (!(vcpu->arch.apf.control_block & KVM_ASYNC_PF_ENABLED))
>>>> + return false;
>>>> +
>>>> + if (vcpu->arch.apf.send_user_only && vcpu_mode_priv(vcpu))
>>>> + return false;
>>>> +
>>>> + /* Pending page fault, which ins't acknowledged by guest */
>>>> + ret = kvm_async_pf_read_cache(vcpu, &val);
>>>> + if (ret || val)
>>>> + return false;
>>>> +
>>>> + /*
>>>> + * Events can't be injected through data abort because it's
>>>> + * going to clobber ELR_EL1, which might not consued (or saved)
>>>> + * by guest yet.
>>>> + */
>>>> + vbar = vcpu_read_sys_reg(vcpu, VBAR_EL1);
>>>> + pc = *vcpu_pc(vcpu);
>>>> + if (pc >= vbar && pc < (vbar + vcpu->arch.apf.no_fault_inst_range))
>>>> + return false;
>>>
>>> Ah, so that's when this `no_fault_inst_range` is for.
>>>
>>> As-is this is not sufficient, and we'll need t be extremely careful
>>> here.
>>>
>>> The vectors themselves typically only have a small amount of stub code,
>>> and the bulk of the non-reentrant exception entry work happens
>>> elsewhere, in a mixture of assembly and C code that isn't even virtually
>>> contiguous with either the vectors or itself.
>>>
>>> It's possible in theory that code in modules (or perhaps in eBPF JIT'd
>>> code) that isn't safe to take a fault from, so even having a contiguous
>>> range controlled by the kernel isn't ideal.
>>>
>>> How does this work on x86?
>>>
>>
>> Yeah, here we just provide a mechanism to forbid injecting data abort. The
>> range is fed by guest through HVC call. So I think it's guest related issue.
>> You had more comments about this in PATCH[9]. I will explain a bit more there.
>>
>> x86 basically relies on EFLAGS[IF] flag. The async page fault can be injected
>> if it's on. Otherwise, it's forbidden. It's workable because exception is
>> special interrupt to x86 if I'm correct.
>>
>> return (vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF) &&
>> !(vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) &
>> (GUEST_INTR_STATE_STI | GUEST_INTR_STATE_MOV_SS));
>
> I really wish this was relying on an architected exception delivery
> mechanism that can be blocked by the guest itself (PSTATE.{I,F,A}).
> Trying to guess based on the PC won't fly. But these signals are
> pretty hard to multiplex with anything else. Like any form of
> non-architected exception injection, I don't see a good path forward
> unless we start considering something like SDEI.
>
> M.
As Paolo mentioned in another reply. There are two types of notifications
sent from host to guest: page_not_present and page_ready. The page_not_present
notification should be delivered synchronously while page_ready can be
delivered asynchronously. He also suggested to reserve a ESR (or DFSC) subclass
for page_not_present. For page_ready, it can be delivered by interrupt, like PPI.
x86 is changing the code to deliver page_ready by interrupt, which was done by
exception previously.
when we use interrupt, instead of exception for page_ready. We won't need the
game of guessing PC.
I assume you prefer to use SDEI for page_not_present, correct? In that case,
what's the current status of SDEI? I mean it has been fully or partially
supported, or we need develop it from the scratch :)
Thanks,
Gavin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH RFCv2 3/9] kvm/arm64: Rename kvm_vcpu_get_hsr() to kvm_vcpu_get_esr()
From: Gavin Shan @ 2020-05-28 6:34 UTC (permalink / raw)
To: Marc Zyngier
Cc: Mark Rutland, aarcange, drjones, suzuki.poulose, catalin.marinas,
linux-kernel, eric.auger, james.morse, shan.gavin, will, kvmarm,
linux-arm-kernel
In-Reply-To: <359dad5546a428ea963781f2728e70bf@kernel.org>
On 5/27/20 5:20 PM, Marc Zyngier wrote:
> On 2020-05-27 03:43, Gavin Shan wrote:
>> Hi Mark,
>>
>> On 5/26/20 8:42 PM, Mark Rutland wrote:
>>> On Fri, May 08, 2020 at 01:29:13PM +1000, Gavin Shan wrote:
>>>> Since kvm/arm32 was removed, this renames kvm_vcpu_get_hsr() to
>>>> kvm_vcpu_get_esr() to it a bit more self-explaining because the
>>>> functions returns ESR instead of HSR on aarch64. This shouldn't
>>>> cause any functional changes.
>>>>
>>>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>>>
>>> I think that this would be a nice cleanup on its own, and could be taken
>>> independently of the rest of this series if it were rebased and sent as
>>> a single patch.
>>>
>>
>> Yeah, I'll see how PATCH[3,4,5] can be posted independently
>> as part of the preparatory work, which is suggested by you
>> in another reply.
>>
>> By the way, I assume the cleanup patches are good enough to
>> target 5.8.rc1/rc2 if you agree.
>
> It's fine to base them on -rc1 or -rc2. They will not be merged
> before 5.9 though.
>
> Thanks,
>
> M.
Sure, Thanks, Marc!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 6/6] dt-bindings: drm: bridge: adi,adv7511.txt: convert to yaml
From: Ricardo Cañuelo @ 2020-05-28 6:36 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Geert Uytterhoeven, michal.simek, Wei Xu, Rob Herring,
Laurent Pinchart, Collabora Kernel ML, Linux ARM
In-Reply-To: <CAMuHMdXQQXOcVuq7Zhfp4qGH0vmLtxp3fdCJ+7VSAMQYSdjsTg@mail.gmail.com>
Hi all,
On mié 27-05-2020 20:18:07, Geert Uytterhoeven wrote:
> > There's currently no requirement that binding schema don't introduce
> > warnings in dts files. That should change when/if we get to a warning
> > free state (probably per platform/family). I don't think we're close
> > on any platform? (If we are, I'd like to start tracking that). It is
> > good to pay attention to the warnings you get though as the schema may
> > not be doing what you expect or the binding really doesn't match
> > reality.
>
> OK.
>
> > > I do my best to avoid introducing regressions when the binding conversions
> > > go upstream.
> >
> > Meaning you fix the dts files or massage the schema to match? If we
> > just adjust schema to match, what's the point in this effort? We
> > should find things wrong or ill defined.
>
> I fix up DTS files, and fast-track those fixes, so they appear upstream before
> the DT binding conversion, where possible.
Thank you everyone for pitching in and for clarifying the procedure,
I'll prepare a new series with the changes that Laurent proposed
(without the patches that Geert already merged).
Cheers,
Ricardo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2] media: exynos4-is: Add missed check for pinctrl_lookup_state()
From: Chuhong Yuan @ 2020-05-28 6:41 UTC (permalink / raw)
Cc: linux-samsung-soc, Chuhong Yuan, linux-kernel,
Krzysztof Kozlowski, Kyungmin Park, Kukjin Kim,
Sylwester Nawrocki, Mauro Carvalho Chehab, linux-arm-kernel,
linux-media
fimc_md_get_pinctrl() misses a check for pinctrl_lookup_state().
Add the missed check to fix it.
Fixes: 4163851f7b99 ("[media] s5p-fimc: Use pinctrl API for camera ports configuration]")
Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
---
Changes in v2:
- Add fixes tag.
drivers/media/platform/exynos4-is/media-dev.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 9aaf3b8060d5..9c31d950cddf 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -1270,6 +1270,9 @@ static int fimc_md_get_pinctrl(struct fimc_md *fmd)
pctl->state_idle = pinctrl_lookup_state(pctl->pinctrl,
PINCTRL_STATE_IDLE);
+ if (IS_ERR(pctl->state_idle))
+ return PTR_ERR(pctl->state_idle);
+
return 0;
}
--
2.26.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 0/2] Introduce PCI_FIXUP_IOMMU
From: Zhangfei Gao @ 2020-05-28 6:46 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: jean-philippe, Lorenzo Pieralisi, Herbert Xu, Arnd Bergmann,
linux-pci, Greg Kroah-Hartman, Joerg Roedel, Hanjun Guo,
Rafael J. Wysocki, linux-kernel, iommu, linux-acpi, Wangzhou,
linux-crypto, Sudeep Holla, Bjorn Helgaas, kenneth-lee-2012,
linux-arm-kernel, Len Brown
In-Reply-To: <20200527181842.GA256680@bjorn-Precision-5520>
Hi, Bjorn
On 2020/5/28 上午2:18, Bjorn Helgaas wrote:
> On Tue, May 26, 2020 at 07:49:07PM +0800, Zhangfei Gao wrote:
>> Some platform devices appear as PCI but are actually on the AMBA bus,
>> and they need fixup in drivers/pci/quirks.c handling iommu_fwnode.
>> Here introducing PCI_FIXUP_IOMMU, which is called after iommu_fwnode
>> is allocated, instead of reusing PCI_FIXUP_FINAL since it will slow
>> down iommu probing as all devices in fixup final list will be
>> reprocessed, suggested by Joerg, [1]
> Is this slowdown significant? We already iterate over every device
> when applying PCI_FIXUP_FINAL quirks, so if we used the existing
> PCI_FIXUP_FINAL, we wouldn't be adding a new loop. We would only be
> adding two more iterations to the loop in pci_do_fixups() that tries
> to match quirks against the current device. I doubt that would be a
> measurable slowdown.
I do not notice the difference when compared fixup_iommu and fixup_final
via get_jiffies_64,
since in our platform no other pci fixup is registered.
Here the plan is adding pci_fixup_device in iommu_fwspec_init,
so if using fixup_final the iteration will be done again here.
>
>> For example:
>> Hisilicon platform device need fixup in
>> drivers/pci/quirks.c handling fwspec->can_stall, which is introduced in [2]
>>
>> +static void quirk_huawei_pcie_sva(struct pci_dev *pdev)
>> +{
>> + struct iommu_fwspec *fwspec;
>> +
>> + pdev->eetlp_prefix_path = 1;
>> + fwspec = dev_iommu_fwspec_get(&pdev->dev);
>> + if (fwspec)
>> + fwspec->can_stall = 1;
>> +}
>> +
>> +DECLARE_PCI_FIXUP_IOMMU(PCI_VENDOR_ID_HUAWEI, 0xa250, quirk_huawei_pcie_sva);
>> +DECLARE_PCI_iFIXUP_IOMMU(PCI_VENDOR_ID_HUAWEI, 0xa251, quirk_huawei_pcie_sva);
>>
>> [1] https://www.spinics.net/lists/iommu/msg44591.html
>> [2] https://www.spinics.net/lists/linux-pci/msg94559.html
> If you reference these in the commit logs, please use lore.kernel.org
> links instead of spinics.
Got it, thanks Bjorn.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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