* [PATCH V4 1/2] arm64: dts: imx8m: add mu node
From: peng.fan @ 2020-06-03 3:35 UTC (permalink / raw)
To: shawnguo, fabio.estevam, kernel, aisheng.dong, robh+dt, sboyd,
linux
Cc: linux-arm-kernel, linux-kernel, linux-imx, leonard.crestez,
daniel.baluta, l.stach, devicetree, linux-clk, Peng Fan
In-Reply-To: <1591155360-26173-1-git-send-email-peng.fan@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
Add mu node to let A53 could communicate with M Core.
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
arch/arm64/boot/dts/freescale/imx8mm.dtsi | 8 ++++++++
arch/arm64/boot/dts/freescale/imx8mn.dtsi | 8 ++++++++
arch/arm64/boot/dts/freescale/imx8mp.dtsi | 8 ++++++++
arch/arm64/boot/dts/freescale/imx8mq.dtsi | 8 ++++++++
4 files changed, 32 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index aaf6e71101a1..d9e787ea246f 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -775,6 +775,14 @@
status = "disabled";
};
+ mu: mailbox@30aa0000 {
+ compatible = "fsl,imx8mm-mu", "fsl,imx6sx-mu";
+ reg = <0x30aa0000 0x10000>;
+ interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MM_CLK_MU_ROOT>;
+ #mbox-cells = <2>;
+ };
+
usdhc1: mmc@30b40000 {
compatible = "fsl,imx8mm-usdhc", "fsl,imx7d-usdhc";
reg = <0x30b40000 0x10000>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index 9a4b65a267d4..3dca1fb34ea3 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -675,6 +675,14 @@
status = "disabled";
};
+ mu: mailbox@30aa0000 {
+ compatible = "fsl,imx8mn-mu", "fsl,imx6sx-mu";
+ reg = <0x30aa0000 0x10000>;
+ interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MN_CLK_MU_ROOT>;
+ #mbox-cells = <2>;
+ };
+
usdhc1: mmc@30b40000 {
compatible = "fsl,imx8mn-usdhc", "fsl,imx7d-usdhc";
reg = <0x30b40000 0x10000>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 45e2c0a4e889..1bc14bb44d90 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -621,6 +621,14 @@
status = "disabled";
};
+ mu: mailbox@30aa0000 {
+ compatible = "fsl,imx8mp-mu", "fsl,imx6sx-mu";
+ reg = <0x30aa0000 0x10000>;
+ interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MP_CLK_MU_ROOT>;
+ #mbox-cells = <2>;
+ };
+
i2c5: i2c@30ad0000 {
compatible = "fsl,imx8mp-i2c", "fsl,imx21-i2c";
#address-cells = <1>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 978f8122c0d2..3e762919d61f 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -959,6 +959,14 @@
status = "disabled";
};
+ mu: mailbox@30aa0000 {
+ compatible = "fsl,imx8mq-mu", "fsl,imx6sx-mu";
+ reg = <0x30aa0000 0x10000>;
+ interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk IMX8MQ_CLK_MU_ROOT>;
+ #mbox-cells = <2>;
+ };
+
usdhc1: mmc@30b40000 {
compatible = "fsl,imx8mq-usdhc",
"fsl,imx7d-usdhc";
--
2.16.4
^ permalink raw reply related
* [PATCH V4 2/2] clk: imx8mp: add mu root clk
From: peng.fan @ 2020-06-03 3:36 UTC (permalink / raw)
To: shawnguo, fabio.estevam, kernel, aisheng.dong, robh+dt, sboyd,
linux
Cc: linux-arm-kernel, linux-kernel, linux-imx, leonard.crestez,
daniel.baluta, l.stach, devicetree, linux-clk, Peng Fan
In-Reply-To: <1591155360-26173-1-git-send-email-peng.fan@nxp.com>
From: Peng Fan <peng.fan@nxp.com>
Add mu root clk for mu mailbox usage.
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/clk/imx/clk-imx8mp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
index b4d9db9d5bf1..ca747712400f 100644
--- a/drivers/clk/imx/clk-imx8mp.c
+++ b/drivers/clk/imx/clk-imx8mp.c
@@ -680,6 +680,7 @@ static int imx8mp_clocks_probe(struct platform_device *pdev)
hws[IMX8MP_CLK_I2C2_ROOT] = imx_clk_hw_gate4("i2c2_root_clk", "i2c2", ccm_base + 0x4180, 0);
hws[IMX8MP_CLK_I2C3_ROOT] = imx_clk_hw_gate4("i2c3_root_clk", "i2c3", ccm_base + 0x4190, 0);
hws[IMX8MP_CLK_I2C4_ROOT] = imx_clk_hw_gate4("i2c4_root_clk", "i2c4", ccm_base + 0x41a0, 0);
+ hws[IMX8MP_CLK_MU_ROOT] = imx_clk_hw_gate4("mu_root_clk", "ipg_root", ccm_base + 0x4210, 0);
hws[IMX8MP_CLK_OCOTP_ROOT] = imx_clk_hw_gate4("ocotp_root_clk", "ipg_root", ccm_base + 0x4220, 0);
hws[IMX8MP_CLK_PCIE_ROOT] = imx_clk_hw_gate4("pcie_root_clk", "pcie_aux", ccm_base + 0x4250, 0);
hws[IMX8MP_CLK_PWM1_ROOT] = imx_clk_hw_gate4("pwm1_root_clk", "pwm1", ccm_base + 0x4280, 0);
--
2.16.4
^ permalink raw reply related
* Re: [PATCH 06/12] PM / devfreq: Add cpu based scaling support to passive_governor
From: Chanwoo Choi @ 2020-06-03 4:07 UTC (permalink / raw)
To: andrew-sh.cheng
Cc: MyungJoo Ham, Kyungmin Park, Rob Herring, Mark Rutland,
Matthias Brugger, Rafael J . Wysocki, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Liam Girdwood, Mark Brown,
devicetree, srv_heupstream, linux-pm, linux-kernel,
linux-mediatek, Sibi Sankar, linux-arm-kernel
In-Reply-To: <1591098190.30729.15.camel@mtksdaap41>
Hi Andrew-sh.Cheng,
Do you know that why cannot show the patches sent from you on mailing list?
Even if you sent them to linux-pm mailing list, I cannot find
your patches on linux-pm's patchwork[1] and others.
[1] https://patchwork.kernel.org/project/linux-pm/list/
Could you find you patch on mailing list?
Do you use git send-email when you send these patches?
I used the thunderbird tool and gmail for reading the patches.
When I tried to read the original source of this patch,
it looks like that the body of patch is encoded.
I cannot read the plain text of patch body.
- When gmail, use 'Show original'
- When thunderbird, use 'More -> View Source'
If I'm missing something to check this patch,
please let me know. I'll fix my environment.
It is strange situation on my case.
On 6/2/20 8:43 PM, andrew-sh.cheng wrote:
> On Thu, 2020-05-28 at 15:14 +0900, Chanwoo Choi wrote:
>> 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'
> Get it.
If you add the blank line before/after of your reply,
it is better to catch your reply. Please add the blank line for me.
>> .
>>
>>> + 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.
> Get it.
> For cpu_freq, I separate it into "unsigned long cpu_curr_freq" and
> "unsigned int cpu_curr_freq_khz"
>>
>>
>>> + 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.
> Get it.
> Will modify them for readability.
>>
>>> +
>>> + 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.
> Get it.
>>
>>
>>> + } 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);
> Get it.
>>
>>> + }
>>> + 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.
> Get it.
>>
>>> +
>>> + 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.
> It is good to change current_freq to curr_freq, but why should it us
> 'unsigned long'?
> I think it is 'unsigned int'.
I think that 'curr_freq' is proper. Yes, it is 'unsigned int'.
When you changing the cpu frequency to device frequency,
recommend to handle them between unsigned int and unsigned long.
>>
>>> + 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'.
> Get it.
>>
>>> + struct cpufreq_policy *policy;
>>> + struct device *cpu_dev;
>>> + unsigned int cpu;
>>> + int ret;
>>> +
>>> + get_online_cpus();
>>
>> Add blank line.
> Get it.
>>
>>> + 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.
>>
>>
> Get it.
>>
>>> + 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.
> Get it.
>>> +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;
>>
> Get it.
>>>
>>> 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;
>> }
> Not fully understanding.
> Do you mean expanding cpufreq_passive_register()?
Yes and rename it for both cpufreq and devfreq.
> I think leave it in function will be with clean for this code segment.
I want that one function handle the notifier register
for both cpufreq and devfreq so that we make it more simply as following:
On the step hanling the governor event, don't need to consider
the type of parent device of devfreq deivce with this style.
case DEVFREQ_GOV_START:
ret = register_notifier(...);
break;
case DEVFREQ_GOV_STOP:
ret = unregister_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)
> Get it.
ditto. As I aboved commented.
>>
>>> 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.
> Get it.
>>
>>> + unsigned int min_freq;
>>> + unsigned int max_freq;
>>> + unsigned int first_cpu;
>>> + struct device *dev;
>>
>> How about changing the name 'dev' to 'cpu_dev'?
> Okay.
>>
>>
>>> + 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;
> Get it.
>>
>>> +
>>> +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.
> Get it.
>>
>>> + * @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
^ permalink raw reply
* Re: [PATCH 06/12] PM / devfreq: Add cpu based scaling support to passive_governor
From: Chanwoo Choi @ 2020-06-03 4:12 UTC (permalink / raw)
To: andrew-sh.cheng
Cc: MyungJoo Ham, Kyungmin Park, Rob Herring, Mark Rutland,
Matthias Brugger, Rafael J . Wysocki, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Liam Girdwood, Mark Brown,
devicetree, srv_heupstream, linux-pm, linux-kernel,
linux-mediatek, Sibi Sankar, linux-arm-kernel
In-Reply-To: <1591100614.1804.1.camel@mtksdaap41>
Hi Andrew-sh.Cheng,
On 6/2/20 9:23 PM, andrew-sh.cheng wrote:
> On Thu, 2020-05-28 at 16:17 +0900, Chanwoo Choi wrote:
>> Hi Andrew-sh.Cheng,
>>
>> The exynos-bus.c used the passive governor.
>> Even if don't make the problem because DEVFREQ_PARENT_DEV is zero,
>> you need to initialize the parent_type with DEVFREQ_PARENT_DEV as following:
>>
>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
>> index 8fa8eb541373..1c71c47bc2ac 100644
>> --- a/drivers/devfreq/exynos-bus.c
>> +++ b/drivers/devfreq/exynos-bus.c
>> @@ -369,6 +369,7 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus,
>> return -ENOMEM;
>>
>> passive_data->parent = parent_devfreq;
>> + passive_data->parent_type = DEVFREQ_PARENT_DEV;
>>
>> /* Add devfreq device for exynos bus with passive governor */
>> bus->devfreq = devm_devfreq_add_device(dev, profile, DEVFREQ_GOV_PASSIVE,
> Hi Chanwoo Choi,
> Do you just remind me to initialize it to DEVFREQ_PARENT_DEV whn use
> this governor?
Yes. This change was not included in this patchset.
> I will do it and thank you for reminding.
Thanks.
(snip)
And, this patchset doesn't include the dt-binding example
and any real example in devicetree. If possible, I recommend
you better to update dt-binding document with example.
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply
* Re: [PATCH V6 4/5] clk: qcom: Add ipq6018 apss clock controller
From: Stephen Boyd @ 2020-06-03 5:31 UTC (permalink / raw)
To: Sivaprakash Murugesan, agross, bjorn.andersson, devicetree,
linux-arm-msm, linux-clk, linux-kernel, mturquette, robh+dt
In-Reply-To: <4266555a-5e4f-4340-1b3c-487a70805751@codeaurora.org>
Quoting Sivaprakash Murugesan (2020-06-02 03:47:20)
>
> On 6/2/2020 1:06 AM, Stephen Boyd wrote:
> > Quoting Sivaprakash Murugesan (2020-06-01 05:41:15)
> >> On 5/28/2020 7:29 AM, Stephen Boyd wrote:
> >>> Quoting Sivaprakash Murugesan (2020-05-27 05:24:51)
> >>>> diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
> >>>> new file mode 100644
> >>>> index 0000000..004f7e1
> >>>> --- /dev/null
> >>>> +++ b/drivers/clk/qcom/apss-ipq6018.c
> >>>> @@ -0,0 +1,106 @@
> >>>> + P_XO,
> >>>> + P_APSS_PLL_EARLY,
> >>>> +};
> >>>> +
> >>>> +static const struct clk_parent_data parents_apcs_alias0_clk_src[] = {
> >>>> + { .fw_name = "xo" },
> >>>> + { .fw_name = "pll" },
> >>> This pll clk is not described in the binding. Please add it there.
> >> Sorry I did not get this, this PLL is not directly defined in this
> >> driver and it comes
> >>
> >> from dts. do you still want to describe it in binding?
> >>
> > Yes, there should be a clock-names property for "pll" and a clocks
> > property in the binding document. I didn't see that.
>
> These are defined in
>
> https://lkml.org/lkml/2020/5/27/658and
>
> https://lkml.org/lkml/2020/5/27/659
>
> it has been defined as part of mailbox binding, since this driver does
>
> not have a dts node and it is child of apcs mailbox driver.
>
Ah alright. Sounds good.
^ permalink raw reply
* Re: [PATCH] h8300: dts: Fix /chosen:stdout-path
From: Masahiro Yamada @ 2020-06-03 5:46 UTC (permalink / raw)
To: Geert Uytterhoeven, Rob Herring
Cc: Yoshinori Sato, DTML, moderated list:H8/300 ARCHITECTURE,
Linux Kernel Mailing List
In-Reply-To: <20200325095326.10875-1-geert+renesas@glider.be>
On Wed, Mar 25, 2020 at 6:53 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
>
> arch/h8300/boot/dts/h8s_sim.dts:11.3-25: Warning (chosen_node_stdout_path): /chosen:stdout-path: property is not a string
> arch/h8300/boot/dts/h8300h_sim.dts:11.3-25: Warning (chosen_node_stdout_path): /chosen:stdout-path: property is not a string
>
> Drop the angle brackets to fix this.
>
> A similar fix was already applied to arch/h8300/boot/dts/edosk2674.dts
> in commit 780ffcd51cb28717 ("h8300: register address fix").
>
> Fixes: 38d6bded13084d50 ("h8300: devicetree source")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Masahiro Yamada <masahiroy@kernel.org>
Unfortunately, h8300 maintainer is not responding...
How to get this in?
Perhaps, Rob can pick this up?
Thanks.
> ---
> arch/h8300/boot/dts/h8300h_sim.dts | 2 +-
> arch/h8300/boot/dts/h8s_sim.dts | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/h8300/boot/dts/h8300h_sim.dts b/arch/h8300/boot/dts/h8300h_sim.dts
> index 595398b9d0180a80..e1d4d9b7f6b40c04 100644
> --- a/arch/h8300/boot/dts/h8300h_sim.dts
> +++ b/arch/h8300/boot/dts/h8300h_sim.dts
> @@ -8,7 +8,7 @@
>
> chosen {
> bootargs = "earlyprintk=h8300-sim";
> - stdout-path = <&sci0>;
> + stdout-path = &sci0;
> };
> aliases {
> serial0 = &sci0;
> diff --git a/arch/h8300/boot/dts/h8s_sim.dts b/arch/h8300/boot/dts/h8s_sim.dts
> index 932cc3c5a81bcdd2..4848e40e607ecc1d 100644
> --- a/arch/h8300/boot/dts/h8s_sim.dts
> +++ b/arch/h8300/boot/dts/h8s_sim.dts
> @@ -8,7 +8,7 @@
>
> chosen {
> bootargs = "earlyprintk=h8300-sim";
> - stdout-path = <&sci0>;
> + stdout-path = &sci0;
> };
> aliases {
> serial0 = &sci0;
> --
> 2.17.1
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* [PATCH 3/3] dt-bindings: spi: Convert imx lpspi to json-schema
From: Anson Huang @ 2020-06-03 6:13 UTC (permalink / raw)
To: broonie, robh+dt, shawnguo, s.hauer, kernel, festevam, marex,
linux-spi, devicetree, linux-arm-kernel, linux-kernel
Cc: Linux-imx
In-Reply-To: <1591164809-13964-1-git-send-email-Anson.Huang@nxp.com>
Convert the i.MX LPSPI binding to DT schema format using json-schema
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
.../devicetree/bindings/spi/spi-fsl-lpspi.txt | 29 -----------
.../devicetree/bindings/spi/spi-fsl-lpspi.yaml | 60 ++++++++++++++++++++++
2 files changed, 60 insertions(+), 29 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/spi/spi-fsl-lpspi.txt
create mode 100644 Documentation/devicetree/bindings/spi/spi-fsl-lpspi.yaml
diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-lpspi.txt b/Documentation/devicetree/bindings/spi/spi-fsl-lpspi.txt
deleted file mode 100644
index e71b81a..0000000
--- a/Documentation/devicetree/bindings/spi/spi-fsl-lpspi.txt
+++ /dev/null
@@ -1,29 +0,0 @@
-* Freescale Low Power SPI (LPSPI) for i.MX
-
-Required properties:
-- compatible :
- - "fsl,imx7ulp-spi" for LPSPI compatible with the one integrated on i.MX7ULP soc
- - "fsl,imx8qxp-spi" for LPSPI compatible with the one integrated on i.MX8QXP soc
-- reg : address and length of the lpspi master registers
-- interrupt-parent : core interrupt controller
-- interrupts : lpspi interrupt
-- clocks : lpspi clock specifier. Its number and order need to correspond to the
- value in clock-names.
-- clock-names : Corresponding to per clock and ipg clock in "clocks"
- respectively. In i.MX7ULP, it only has per clk, so use CLK_DUMMY
- to fill the "ipg" blank.
-- spi-slave : spi slave mode support. In slave mode, add this attribute without
- value. In master mode, remove it.
-
-Examples:
-
-lpspi2: lpspi@40290000 {
- compatible = "fsl,imx7ulp-spi";
- reg = <0x40290000 0x10000>;
- interrupt-parent = <&intc>;
- interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&clks IMX7ULP_CLK_LPSPI2>,
- <&clks IMX7ULP_CLK_DUMMY>;
- clock-names = "per", "ipg";
- spi-slave;
-};
diff --git a/Documentation/devicetree/bindings/spi/spi-fsl-lpspi.yaml b/Documentation/devicetree/bindings/spi/spi-fsl-lpspi.yaml
new file mode 100644
index 0000000..d18e2b0f
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-fsl-lpspi.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/spi-fsl-lpspi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale Low Power SPI (LPSPI) for i.MX
+
+maintainers:
+ - Anson Huang <Anson.Huang@nxp.com>
+
+allOf:
+ - $ref: "/schemas/spi/spi-controller.yaml#"
+
+properties:
+ compatible:
+ enum:
+ - fsl,imx7ulp-spi
+ - fsl,imx8qxp-spi
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: SoC SPI per clock
+ - description: SoC SPI ipg clock
+ maxItems: 2
+
+ clock-names:
+ items:
+ - const: per
+ - const: ipg
+ maxItems: 2
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+
+examples:
+ - |
+ #include <dt-bindings/clock/imx7ulp-clock.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ spi@40290000 {
+ compatible = "fsl,imx7ulp-spi";
+ reg = <0x40290000 0x10000>;
+ interrupt-parent = <&intc>;
+ interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clks IMX7ULP_CLK_LPSPI2>,
+ <&clks IMX7ULP_CLK_DUMMY>;
+ clock-names = "per", "ipg";
+ spi-slave;
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH 2/3] dt-bindings: spi: Convert imx cspi to json-schema
From: Anson Huang @ 2020-06-03 6:13 UTC (permalink / raw)
To: broonie, robh+dt, shawnguo, s.hauer, kernel, festevam, marex,
linux-spi, devicetree, linux-arm-kernel, linux-kernel
Cc: Linux-imx
In-Reply-To: <1591164809-13964-1-git-send-email-Anson.Huang@nxp.com>
Convert the i.MX CSPI binding to DT schema format using json-schema,
update compatible, remove obsolete properties "fsl,spi-num-chipselects"
and update the example based on latest DT file.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
.../devicetree/bindings/spi/fsl-imx-cspi.txt | 56 -------------
.../devicetree/bindings/spi/fsl-imx-cspi.yaml | 97 ++++++++++++++++++++++
2 files changed, 97 insertions(+), 56 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
create mode 100644 Documentation/devicetree/bindings/spi/fsl-imx-cspi.yaml
diff --git a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
deleted file mode 100644
index 33bc58f..0000000
--- a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
+++ /dev/null
@@ -1,56 +0,0 @@
-* Freescale (Enhanced) Configurable Serial Peripheral Interface
- (CSPI/eCSPI) for i.MX
-
-Required properties:
-- compatible :
- - "fsl,imx1-cspi" for SPI compatible with the one integrated on i.MX1
- - "fsl,imx21-cspi" for SPI compatible with the one integrated on i.MX21
- - "fsl,imx27-cspi" for SPI compatible with the one integrated on i.MX27
- - "fsl,imx31-cspi" for SPI compatible with the one integrated on i.MX31
- - "fsl,imx35-cspi" for SPI compatible with the one integrated on i.MX35
- - "fsl,imx51-ecspi" for SPI compatible with the one integrated on i.MX51
- - "fsl,imx53-ecspi" for SPI compatible with the one integrated on i.MX53 and later Soc
- - "fsl,imx8mq-ecspi" for SPI compatible with the one integrated on i.MX8MQ
- - "fsl,imx8mm-ecspi" for SPI compatible with the one integrated on i.MX8MM
- - "fsl,imx8mn-ecspi" for SPI compatible with the one integrated on i.MX8MN
- - "fsl,imx8mp-ecspi" for SPI compatible with the one integrated on i.MX8MP
-- reg : Offset and length of the register set for the device
-- interrupts : Should contain CSPI/eCSPI interrupt
-- clocks : Clock specifiers for both ipg and per clocks.
-- clock-names : Clock names should include both "ipg" and "per"
-See the clock consumer binding,
- Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-Recommended properties:
-- cs-gpios : GPIOs to use as chip selects, see spi-bus.txt. While the native chip
-select lines can be used, they appear to always generate a pulse between each
-word of a transfer. Most use cases will require GPIO based chip selects to
-generate a valid transaction.
-
-Optional properties:
-- num-cs : Number of total chip selects, see spi-bus.txt.
-- dmas: DMA specifiers for tx and rx dma. See the DMA client binding,
-Documentation/devicetree/bindings/dma/dma.txt.
-- dma-names: DMA request names, if present, should include "tx" and "rx".
-- fsl,spi-rdy-drctl: Integer, representing the value of DRCTL, the register
-controlling the SPI_READY handling. Note that to enable the DRCTL consideration,
-the SPI_READY mode-flag needs to be set too.
-Valid values are: 0 (disabled), 1 (edge-triggered burst) and 2 (level-triggered burst).
-
-Obsolete properties:
-- fsl,spi-num-chipselects : Contains the number of the chipselect
-
-Example:
-
-ecspi@70010000 {
- #address-cells = <1>;
- #size-cells = <0>;
- compatible = "fsl,imx51-ecspi";
- reg = <0x70010000 0x4000>;
- interrupts = <36>;
- cs-gpios = <&gpio3 24 0>, /* GPIO3_24 */
- <&gpio3 25 0>; /* GPIO3_25 */
- dmas = <&sdma 3 7 1>, <&sdma 4 7 2>;
- dma-names = "rx", "tx";
- fsl,spi-rdy-drctl = <1>;
-};
diff --git a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.yaml b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.yaml
new file mode 100644
index 0000000..cac023d
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.yaml
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/fsl-imx-cspi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale (Enhanced) Configurable Serial Peripheral Interface (CSPI/eCSPI) for i.MX
+
+maintainers:
+ - Shawn Guo <shawn.guo@linaro.org>
+
+allOf:
+ - $ref: "/schemas/spi/spi-controller.yaml#"
+
+properties:
+ compatible:
+ oneOf:
+ - const: fsl,imx1-cspi
+ - const: fsl,imx21-cspi
+ - const: fsl,imx27-cspi
+ - const: fsl,imx31-cspi
+ - const: fsl,imx35-cspi
+ - const: fsl,imx51-ecspi
+ - const: fsl,imx53-ecspi
+ - items:
+ - enum:
+ - fsl,imx50-ecspi
+ - fsl,imx6q-ecspi
+ - fsl,imx6sx-ecspi
+ - fsl,imx6sl-ecspi
+ - fsl,imx6sll-ecspi
+ - fsl,imx6ul-ecspi
+ - fsl,imx7d-ecspi
+ - fsl,imx8mq-ecspi
+ - fsl,imx8mm-ecspi
+ - fsl,imx8mn-ecspi
+ - fsl,imx8mp-ecspi
+ - const: fsl,imx51-ecspi
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: SoC SPI ipg clock
+ - description: SoC SPI per clock
+ maxItems: 2
+
+ clock-names:
+ items:
+ - const: ipg
+ - const: per
+ maxItems: 2
+
+ dmas:
+ items:
+ - description: DMA controller phandle and request line for RX
+ - description: DMA controller phandle and request line for TX
+
+ dma-names:
+ items:
+ - const: rx
+ - const: tx
+
+ fsl,spi-rdy-drctl:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ Integer, representing the value of DRCTL, the register controlling
+ the SPI_READY handling. Note that to enable the DRCTL consideration,
+ the SPI_READY mode-flag needs to be set too.
+ Valid values are: 0 (disabled), 1 (edge-triggered burst) and 2 (level-triggered burst).
+ enum: [0, 1, 2]
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+
+examples:
+ - |
+ #include <dt-bindings/clock/imx5-clock.h>
+
+ spi@70010000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "fsl,imx51-ecspi";
+ reg = <0x70010000 0x4000>;
+ interrupts = <36>;
+ clocks = <&clks IMX5_CLK_ECSPI1_IPG_GATE>,
+ <&clks IMX5_CLK_ECSPI1_PER_GATE>;
+ clock-names = "ipg", "per";
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH 1/3] dt-bindings: spi: Convert mxs spi to json-schema
From: Anson Huang @ 2020-06-03 6:13 UTC (permalink / raw)
To: broonie, robh+dt, shawnguo, s.hauer, kernel, festevam, marex,
linux-spi, devicetree, linux-arm-kernel, linux-kernel
Cc: Linux-imx
In-Reply-To: <1591164809-13964-1-git-send-email-Anson.Huang@nxp.com>
Convert the MXS SPI binding to DT schema format using json-schema
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Documentation/devicetree/bindings/spi/mxs-spi.txt | 26 ----------
Documentation/devicetree/bindings/spi/mxs-spi.yaml | 55 ++++++++++++++++++++++
2 files changed, 55 insertions(+), 26 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/spi/mxs-spi.txt
create mode 100644 Documentation/devicetree/bindings/spi/mxs-spi.yaml
diff --git a/Documentation/devicetree/bindings/spi/mxs-spi.txt b/Documentation/devicetree/bindings/spi/mxs-spi.txt
deleted file mode 100644
index 3499b73..0000000
--- a/Documentation/devicetree/bindings/spi/mxs-spi.txt
+++ /dev/null
@@ -1,26 +0,0 @@
-* Freescale MX233/MX28 SSP/SPI
-
-Required properties:
-- compatible: Should be "fsl,<soc>-spi", where soc is "imx23" or "imx28"
-- reg: Offset and length of the register set for the device
-- interrupts: Should contain SSP ERROR interrupt
-- dmas: DMA specifier, consisting of a phandle to DMA controller node
- and SSP DMA channel ID.
- Refer to dma.txt and fsl-mxs-dma.txt for details.
-- dma-names: Must be "rx-tx".
-
-Optional properties:
-- clock-frequency : Input clock frequency to the SPI block in Hz.
- Default is 160000000 Hz.
-
-Example:
-
-ssp0: ssp@80010000 {
- #address-cells = <1>;
- #size-cells = <0>;
- compatible = "fsl,imx28-spi";
- reg = <0x80010000 0x2000>;
- interrupts = <96>;
- dmas = <&dma_apbh 0>;
- dma-names = "rx-tx";
-};
diff --git a/Documentation/devicetree/bindings/spi/mxs-spi.yaml b/Documentation/devicetree/bindings/spi/mxs-spi.yaml
new file mode 100644
index 0000000..ef99d12
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/mxs-spi.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/mxs-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale MX233/MX28 SSP/SPI
+
+maintainers:
+ - Marek Vasut <marex@denx.de>
+
+allOf:
+ - $ref: "/schemas/spi/spi-controller.yaml#"
+
+properties:
+ compatible:
+ enum:
+ - fsl,imx23-spi
+ - fsl,imx28-spi
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ dmas:
+ maxItems: 1
+
+ dma-names:
+ const: rx-tx
+
+ clock-frequency:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: input clock frequency to the SPI block in Hz.
+ default: 160000000
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - dmas
+ - dma-names
+
+examples:
+ - |
+ spi@80010000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "fsl,imx28-spi";
+ reg = <0x80010000 0x2000>;
+ interrupts = <96>;
+ dmas = <&dma_apbh 0>;
+ dma-names = "rx-tx";
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH 0/3] Convert mxs/imx spi/cspi/lpspi binding to json-schema
From: Anson Huang @ 2020-06-03 6:13 UTC (permalink / raw)
To: broonie, robh+dt, shawnguo, s.hauer, kernel, festevam, marex,
linux-spi, devicetree, linux-arm-kernel, linux-kernel
Cc: Linux-imx
This patch series converts mxs/imx spi/cspi/lpspi binding to json-schema.
In fsl-imx-cspi.yaml, also update compatible, remove obsolete properties
"fsl,spi-num-chipselects" and update the example based on latest DT file;
In spi-fsl-lpspi.yaml, the original maintainer's email address
pandy.gao@nxp.com is no longer valid, so I use mine.
Anson Huang (3):
dt-bindings: spi: Convert mxs spi to json-schema
dt-bindings: spi: Convert imx cspi to json-schema
dt-bindings: spi: Convert imx lpspi to json-schema
.../devicetree/bindings/spi/fsl-imx-cspi.txt | 56 -------------
.../devicetree/bindings/spi/fsl-imx-cspi.yaml | 97 ++++++++++++++++++++++
Documentation/devicetree/bindings/spi/mxs-spi.txt | 26 ------
Documentation/devicetree/bindings/spi/mxs-spi.yaml | 55 ++++++++++++
.../devicetree/bindings/spi/spi-fsl-lpspi.txt | 29 -------
.../devicetree/bindings/spi/spi-fsl-lpspi.yaml | 60 +++++++++++++
6 files changed, 212 insertions(+), 111 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
create mode 100644 Documentation/devicetree/bindings/spi/fsl-imx-cspi.yaml
delete mode 100644 Documentation/devicetree/bindings/spi/mxs-spi.txt
create mode 100644 Documentation/devicetree/bindings/spi/mxs-spi.yaml
delete mode 100644 Documentation/devicetree/bindings/spi/spi-fsl-lpspi.txt
create mode 100644 Documentation/devicetree/bindings/spi/spi-fsl-lpspi.yaml
--
2.7.4
^ permalink raw reply
* Re: Security Random Number Generator support
From: Neal Liu @ 2020-06-03 7:29 UTC (permalink / raw)
To: Marc Zyngier, Julius Werner, Ard Biesheuvel
Cc: Neal Liu,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Herbert Xu, Arnd Bergmann, Greg Kroah-Hartman, Sean Wang, lkml,
wsd_upstream, Crystal Guo (郭晶), Rob Herring,
Linux Crypto Mailing List, Matt Mackall, Matthias Brugger,
linux-mediatek@lists.infradead.org, Linux ARM
In-Reply-To: <85dfc0142d3879d50c0ba18bcc71e199@misterjones.org>
On Tue, 2020-06-02 at 21:02 +0800, Marc Zyngier wrote:
> On 2020-06-02 13:14, Ard Biesheuvel wrote:
> > On Tue, 2 Jun 2020 at 10:15, Neal Liu <neal.liu@mediatek.com> wrote:
> >>
> >> These patch series introduce a security random number generator
> >> which provides a generic interface to get hardware rnd from Secure
> >> state. The Secure state can be Arm Trusted Firmware(ATF), Trusted
> >> Execution Environment(TEE), or even EL2 hypervisor.
> >>
> >> Patch #1..2 adds sec-rng kernel driver for Trustzone based SoCs.
> >> For security awareness SoCs on ARMv8 with TrustZone enabled,
> >> peripherals like entropy sources is not accessible from normal world
> >> (linux) and rather accessible from secure world (HYP/ATF/TEE) only.
> >> This driver aims to provide a generic interface to Arm Trusted
> >> Firmware or Hypervisor rng service.
> >>
> >>
> >> changes since v1:
> >> - rename mt67xx-rng to mtk-sec-rng since all MediaTek ARMv8 SoCs can
> >> reuse
> >> this driver.
> >> - refine coding style and unnecessary check.
> >>
> >> changes since v2:
> >> - remove unused comments.
> >> - remove redundant variable.
> >>
> >> changes since v3:
> >> - add dt-bindings for MediaTek rng with TrustZone enabled.
> >> - revise HWRNG SMC call fid.
> >>
> >> changes since v4:
> >> - move bindings to the arm/firmware directory.
> >> - revise driver init flow to check more property.
> >>
> >> changes since v5:
> >> - refactor to more generic security rng driver which
> >> is not platform specific.
> >>
> >> *** BLURB HERE ***
> >>
> >> Neal Liu (2):
> >> dt-bindings: rng: add bindings for sec-rng
> >> hwrng: add sec-rng driver
> >>
> >
> > There is no reason to model a SMC call as a driver, and represent it
> > via a DT node like this.
>
> +1.
>
> > It would be much better if this SMC interface is made truly generic,
> > and wired into the arch_get_random() interface, which can be used much
> > earlier.
>
> Wasn't there a plan to standardize a SMC call to rule them all?
>
> M.
Could you give us a hint how to make this SMC interface more generic in
addition to my approach?
There is no (easy) way to get platform-independent SMC function ID,
which is why we encode it into device tree, and provide a generic
driver. In this way, different devices can be mapped and then get
different function ID internally.
^ permalink raw reply
* Re: [PATCH v7 21/24] iommu/arm-smmu-v3: Add stall support for platform devices
From: Jean-Philippe Brucker @ 2020-06-03 7:38 UTC (permalink / raw)
To: Shameerali Kolothum Thodi
Cc: devicetree@vger.kernel.org, kevin.tian@intel.com,
fenghua.yu@intel.com, linux-pci@vger.kernel.org,
felix.kuehling@amd.com, robin.murphy@arm.com,
christian.koenig@amd.com, hch@infradead.org, jgg@ziepe.ca,
iommu@lists.linux-foundation.org, catalin.marinas@arm.com,
zhangfei.gao@linaro.org, will@kernel.org, linux-mm@kvack.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <c165fe41230f49baba991f1a416a4739@huawei.com>
On Tue, Jun 02, 2020 at 12:12:30PM +0000, Shameerali Kolothum Thodi wrote:
> > > > > > + if (ssid_valid)
> > > > > > + flt->prm.flags |=
> > > > IOMMU_FAULT_PAGE_REQUEST_PASID_VALID;
> > > > >
> > > > > Do we need to set this for STALL mode only support? I had an issue
> > > > > with this being set on a vSVA POC based on our D06 zip
> > > > > device(which is a "fake " pci dev that supports STALL mode but no
> > > > > PRI). The issue is, CMDQ_OP_RESUME doesn't have any ssid or SSV
> > > > > params and works on sid
> > > > and stag only.
> > > >
> > > > I don't understand the problem, arm_smmu_page_response() doesn't set
> > > > SSID or SSV when sending a CMDQ_OP_RESUME. Could you detail the flow
> > > > of a stall event and RESUME command in your prototype? Are you
> > > > getting issues with the host driver or the guest driver?
> > >
> > > The issue is on the host side iommu_page_response(). The flow is
> > > something like below.
> > >
> > > Stall: Host:-
> > >
> > > arm_smmu_handle_evt()
> > > iommu_report_device_fault()
> > > vfio_pci_iommu_dev_fault_handler()
> > >
> > > Stall: Qemu:-
> > >
> > > vfio_dma_fault_notifier_handler()
> > > inject_faults()
> > > smmuv3_inject_faults()
> > >
> > > Stall: Guest:-
> > >
> > > arm_smmu_handle_evt()
> > > iommu_report_device_fault()
> > > iommu_queue_iopf
> > > ...
> > > iopf_handle_group()
> > > iopf_handle_single()
> > > handle_mm_fault()
> > > iopf_complete()
> > > iommu_page_response()
> > > arm_smmu_page_response()
> > > arm_smmu_cmdq_issue_cmd(CMDQ_OP_RESUME)
> > >
> > > Resume: Qemu:-
> > >
> > > smmuv3_cmdq_consume(SMMU_CMD_RESUME)
> > > smmuv3_notify_page_resp()
> > > vfio:ioctl(page_response) --> struct iommu_page_response is filled
> > > with only version, grpid and code.
> > >
> > > Resume: Host:-
> > > ioctl(page_response)
> > > iommu_page_response() --> fails as the pending req has PASID_VALID
> > flag
> > > set and it checks for a match.
> >
> > I believe the fix needs to be here. It's also wrong for PRI since not all PCIe
> > endpoint require a PASID in the page response. Could you try the attached
> > patch?
>
> Going through the patch, yes, that will definitely fix the issue. But isn't it better if
> the request itself indicate whether it expects a response msg with a valid pasid or
> not? The response msg can come from userspace as well(vSVA) and if for some reason
> doesn't set it for a req that expects pasid then it should be an error, right? In the temp
> fix I had, I introduced another flag to indicate the endpoint has PRI support or not and
> used that to verify the pasid requirement. But for the PRI case you mentioned
> above, not sure it is easy to get that information or not. May be I am complicating things
> here :)
No you're right, we shouldn't send back malformed responses to the SMMU. I
suppose we can store a flag "PASID required" in the fault and check that
against the response. If we have to discard the guest's response, then we
can either fake a response (abort the stall) right away, or wait for the
response timeout to kick, which will do the same.
Thanks,
Jean
^ permalink raw reply
* Re: Security Random Number Generator support
From: Marc Zyngier @ 2020-06-03 7:40 UTC (permalink / raw)
To: Neal Liu
Cc: Julius Werner, Ard Biesheuvel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Herbert Xu, Arnd Bergmann, Greg Kroah-Hartman, Sean Wang,
linux-mediatek, lkml, wsd_upstream, Rob Herring,
Linux Crypto Mailing List, Matt Mackall, Matthias Brugger,
Crystal Guo (郭晶), Linux ARM
In-Reply-To: <1591169342.4878.9.camel@mtkswgap22>
On 2020-06-03 08:29, Neal Liu wrote:
> On Tue, 2020-06-02 at 21:02 +0800, Marc Zyngier wrote:
>> On 2020-06-02 13:14, Ard Biesheuvel wrote:
>> > On Tue, 2 Jun 2020 at 10:15, Neal Liu <neal.liu@mediatek.com> wrote:
>> >>
>> >> These patch series introduce a security random number generator
>> >> which provides a generic interface to get hardware rnd from Secure
>> >> state. The Secure state can be Arm Trusted Firmware(ATF), Trusted
>> >> Execution Environment(TEE), or even EL2 hypervisor.
>> >>
>> >> Patch #1..2 adds sec-rng kernel driver for Trustzone based SoCs.
>> >> For security awareness SoCs on ARMv8 with TrustZone enabled,
>> >> peripherals like entropy sources is not accessible from normal world
>> >> (linux) and rather accessible from secure world (HYP/ATF/TEE) only.
>> >> This driver aims to provide a generic interface to Arm Trusted
>> >> Firmware or Hypervisor rng service.
>> >>
>> >>
>> >> changes since v1:
>> >> - rename mt67xx-rng to mtk-sec-rng since all MediaTek ARMv8 SoCs can
>> >> reuse
>> >> this driver.
>> >> - refine coding style and unnecessary check.
>> >>
>> >> changes since v2:
>> >> - remove unused comments.
>> >> - remove redundant variable.
>> >>
>> >> changes since v3:
>> >> - add dt-bindings for MediaTek rng with TrustZone enabled.
>> >> - revise HWRNG SMC call fid.
>> >>
>> >> changes since v4:
>> >> - move bindings to the arm/firmware directory.
>> >> - revise driver init flow to check more property.
>> >>
>> >> changes since v5:
>> >> - refactor to more generic security rng driver which
>> >> is not platform specific.
>> >>
>> >> *** BLURB HERE ***
>> >>
>> >> Neal Liu (2):
>> >> dt-bindings: rng: add bindings for sec-rng
>> >> hwrng: add sec-rng driver
>> >>
>> >
>> > There is no reason to model a SMC call as a driver, and represent it
>> > via a DT node like this.
>>
>> +1.
>>
>> > It would be much better if this SMC interface is made truly generic,
>> > and wired into the arch_get_random() interface, which can be used much
>> > earlier.
>>
>> Wasn't there a plan to standardize a SMC call to rule them all?
>>
>> M.
>
> Could you give us a hint how to make this SMC interface more generic in
> addition to my approach?
> There is no (easy) way to get platform-independent SMC function ID,
> which is why we encode it into device tree, and provide a generic
> driver. In this way, different devices can be mapped and then get
> different function ID internally.
The idea is simply to have *one* single ID that caters for all
implementations, just like we did for PSCI at the time. This
requires ARM to edict a standard, which is what I was referring
to above.
There is zero benefit in having a platform-dependent ID. It just
pointlessly increases complexity, and means we cannot use the RNG
before the firmware tables are available (yes, we need it that
early).
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Re: Security Random Number Generator support
From: Neal Liu @ 2020-06-03 7:54 UTC (permalink / raw)
To: Marc Zyngier
Cc: Neal Liu, Julius Werner, Ard Biesheuvel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Herbert Xu, Arnd Bergmann, Greg Kroah-Hartman, Sean Wang,
linux-mediatek, lkml, wsd_upstream, Rob Herring,
Linux Crypto Mailing List, Matt Mackall, Matthias Brugger,
Crystal Guo (郭晶), Linux ARM
In-Reply-To: <fcbe37f6f9cbcde24f9c28bc504f1f0e@kernel.org>
On Wed, 2020-06-03 at 08:40 +0100, Marc Zyngier wrote:
> On 2020-06-03 08:29, Neal Liu wrote:
> > On Tue, 2020-06-02 at 21:02 +0800, Marc Zyngier wrote:
> >> On 2020-06-02 13:14, Ard Biesheuvel wrote:
> >> > On Tue, 2 Jun 2020 at 10:15, Neal Liu <neal.liu@mediatek.com> wrote:
> >> >>
> >> >> These patch series introduce a security random number generator
> >> >> which provides a generic interface to get hardware rnd from Secure
> >> >> state. The Secure state can be Arm Trusted Firmware(ATF), Trusted
> >> >> Execution Environment(TEE), or even EL2 hypervisor.
> >> >>
> >> >> Patch #1..2 adds sec-rng kernel driver for Trustzone based SoCs.
> >> >> For security awareness SoCs on ARMv8 with TrustZone enabled,
> >> >> peripherals like entropy sources is not accessible from normal world
> >> >> (linux) and rather accessible from secure world (HYP/ATF/TEE) only.
> >> >> This driver aims to provide a generic interface to Arm Trusted
> >> >> Firmware or Hypervisor rng service.
> >> >>
> >> >>
> >> >> changes since v1:
> >> >> - rename mt67xx-rng to mtk-sec-rng since all MediaTek ARMv8 SoCs can
> >> >> reuse
> >> >> this driver.
> >> >> - refine coding style and unnecessary check.
> >> >>
> >> >> changes since v2:
> >> >> - remove unused comments.
> >> >> - remove redundant variable.
> >> >>
> >> >> changes since v3:
> >> >> - add dt-bindings for MediaTek rng with TrustZone enabled.
> >> >> - revise HWRNG SMC call fid.
> >> >>
> >> >> changes since v4:
> >> >> - move bindings to the arm/firmware directory.
> >> >> - revise driver init flow to check more property.
> >> >>
> >> >> changes since v5:
> >> >> - refactor to more generic security rng driver which
> >> >> is not platform specific.
> >> >>
> >> >> *** BLURB HERE ***
> >> >>
> >> >> Neal Liu (2):
> >> >> dt-bindings: rng: add bindings for sec-rng
> >> >> hwrng: add sec-rng driver
> >> >>
> >> >
> >> > There is no reason to model a SMC call as a driver, and represent it
> >> > via a DT node like this.
> >>
> >> +1.
> >>
> >> > It would be much better if this SMC interface is made truly generic,
> >> > and wired into the arch_get_random() interface, which can be used much
> >> > earlier.
> >>
> >> Wasn't there a plan to standardize a SMC call to rule them all?
> >>
> >> M.
> >
> > Could you give us a hint how to make this SMC interface more generic in
> > addition to my approach?
> > There is no (easy) way to get platform-independent SMC function ID,
> > which is why we encode it into device tree, and provide a generic
> > driver. In this way, different devices can be mapped and then get
> > different function ID internally.
>
> The idea is simply to have *one* single ID that caters for all
> implementations, just like we did for PSCI at the time. This
> requires ARM to edict a standard, which is what I was referring
> to above.
>
> There is zero benefit in having a platform-dependent ID. It just
> pointlessly increases complexity, and means we cannot use the RNG
> before the firmware tables are available (yes, we need it that
> early).
>
> M.
Do you know which ARM expert could edict this standard?
Or is there any chance that we can make one? And be reviewed by
maintainers?
^ permalink raw reply
* Re: [PATCH v3 06/10] arm64: dts: actions: Add DMA Controller for S700
From: Manivannan Sadhasivam @ 2020-06-03 8:30 UTC (permalink / raw)
To: Amit Singh Tomar, andre.przywara, afaerber, vkoul, robh+dt
Cc: dan.j.williams, cristian.ciocaltea, linux-kernel,
linux-arm-kernel, linux-actions, devicetree
In-Reply-To: <1591119192-18538-7-git-send-email-amittomer25@gmail.com>
On 2 June 2020 11:03:08 PM IST, Amit Singh Tomar <amittomer25@gmail.com> wrote:
>This commit adds DAM controller present on Actions S700, it differs
DMA
>from
>S900 in terms of number of dma channels and requests.
>
>Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
>---
>Changes since v2:
> * added power-domain property as sps
> is enabled now and DMA needs it.
>Changes since v1:
> * No Change.
>Changes since RFC:
> * No Change.
>---
> arch/arm64/boot/dts/actions/s700.dtsi | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
>diff --git a/arch/arm64/boot/dts/actions/s700.dtsi
>b/arch/arm64/boot/dts/actions/s700.dtsi
>index f8eb72bb4125..605594dd7a0e 100644
>--- a/arch/arm64/boot/dts/actions/s700.dtsi
>+++ b/arch/arm64/boot/dts/actions/s700.dtsi
>@@ -6,6 +6,7 @@
> #include <dt-bindings/clock/actions,s700-cmu.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/reset/actions,s700-reset.h>
>+#include <dt-bindings/power/owl-s700-powergate.h>
Sort this alphabetically.
Thanks,
Mani
>
> / {
> compatible = "actions,s700";
>@@ -244,5 +245,19 @@
> <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> };
>+
>+ dma: dma-controller@e0230000 {
>+ compatible = "actions,s700-dma";
>+ reg = <0x0 0xe0230000 0x0 0x1000>;
>+ interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
>+ <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>,
>+ <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>,
>+ <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
>+ #dma-cells = <1>;
>+ dma-channels = <10>;
>+ dma-requests = <44>;
>+ clocks = <&cmu CLK_DMAC>;
>+ power-domains = <&sps S700_PD_DMA>;
>+ };
> };
> };
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: (EXT) [PATCH v8 00/13] add ecspi ERR009165 for i.mx6/7 soc family
From: Matthias Schiffer @ 2020-06-03 8:31 UTC (permalink / raw)
To: Robin Gong
Cc: devicetree, linux-kernel, linux-spi, linux-imx, kernel,
linux-arm-kernel, mark.rutland, broonie, robh+dt, catalin.marinas,
vkoul, will.deacon, shawnguo, festevam, s.hauer, martin.fuzzey,
u.kleine-koenig, dan.j.williams, Markus Niebel
In-Reply-To: <1590006865-20900-1-git-send-email-yibin.gong@nxp.com>
On Thu, 2020-05-21 at 04:34 +0800, Robin Gong wrote:
> There is ecspi ERR009165 on i.mx6/7 soc family, which cause FIFO
> transfer to be send twice in DMA mode. Please get more information
> from:
> https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf. The workaround is
> adding
> new sdma ram script which works in XCH mode as PIO inside sdma
> instead
> of SMC mode, meanwhile, 'TX_THRESHOLD' should be 0. The issue should
> be
> exist on all legacy i.mx6/7 soc family before i.mx6ul.
> NXP fix this design issue from i.mx6ul, so newer chips including
> i.mx6ul/
> 6ull/6sll do not need this workaroud anymore. All other i.mx6/7/8
> chips
> still need this workaroud. This patch set add new 'fsl,imx6ul-ecspi'
> for ecspi driver and 'ecspi_fixed' in sdma driver to choose if need
> errata
> or not.
> The first two reverted patches should be the same issue, though, it
> seems 'fixed' by changing to other shp script. Hope Sean or Sascha
> could
> have the chance to test this patch set if could fix their issues.
> Besides, enable sdma support for i.mx8mm/8mq and fix ecspi1 not work
> on i.mx8mm because the event id is zero.
>
> PS:
> Please get sdma firmware from below linux-firmware and copy it to
> your
> local rootfs /lib/firmware/imx/sdma.
Hello Robin,
we have tried out this series, and there seems to be an issue with the
PIO fallback. We are testing on an i.MX6Q board, and our kernel is a
mostly-unmodified 5.4, on which we backported all SDMA patches from
next-20200602 (imx-sdma.c is identical to next-20200602 version), and
then applied this whole series.
We build the SDMA driver as a kernel module, which is loaded by udev,
so the root filesystem is ready and the SDMA firmware can be loaded.
The behaviour we're seeing is the following:
1. As long as the SDMA driver is not loaded, initializing spi_imx will
be deferred
2. imx_sdma is loaded. The SDMA firmware is not yet loaded at this
point
3. spi_imx is initialized and an SPI-NOR flash is probed. To load the
BFPT, the driver will attempt to use DMA; this will fail with EINVAL as
long as the SDMA firmware is not ready, so the fallback to PIO happens
(4. SDMA firmware is ready, subsequent SPI transfers use DMA)
The problem happens in step 3: Whenever the driver falls back to PIO,
the received data is corrupt. The behaviour is specific to the
fallback: When I disable DMA completely via spi_imx.use_dma, or when
the timing is lucky and the SDMA firmware gets loaded before the flash
is probed, no corruption can be observed.
Kind regards,
Matthias
>
https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/imx/sdma
>
> v2:
> 1.Add commit log for reverted patches.
> 2.Add comment for 'ecspi_fixed' in sdma driver.
> 3.Add 'fsl,imx6sll-ecspi' compatible instead of 'fsl,imx6ul-ecspi'
> rather than remove.
> v3:
> 1.Confirm with design team make sure ERR009165 fixed on
> i.mx6ul/i.mx6ull
> /i.mx6sll, not fixed on i.mx8m/8mm and other i.mx6/7 legacy
> chips.
> Correct dts related dts patch in v2.
> 2.Clean eratta information in binding doc and new 'tx_glitch_fixed'
> flag
> in spi-imx driver to state ERR009165 fixed or not.
> 3.Enlarge burst size to fifo size for tx since tx_wml set to 0 in
> the
> errata workaroud, thus improve performance as possible.
> v4:
> 1.Add Ack tag from Mark and Vinod
> 2.Remove checking 'event_id1' zero as 'event_id0'.
> v5:
> 1.Add the last patch for compatible with the current uart driver
> which
> using rom script, so both uart ram script and rom script
> supported
> in latest firmware, by default uart rom script used. UART driver
> will be broken without this patch.
> v6:
> 1.Resend after rebase the latest next branch.
> 2.Remove below No.13~No.15 patches of v5 because they were
> mergered.
> ARM: dts: imx6ul: add dma support on ecspi
> ARM: dts: imx6sll: correct sdma compatible
> arm64: defconfig: Enable SDMA on i.mx8mq/8mm
> 3.Revert "dmaengine: imx-sdma: fix context cache" since
> 'context_loaded' removed.
> v7:
> 1.Put the last patch 13/13 'Revert "dmaengine: imx-sdma: fix
> context
> cache"' to the ahead of 03/13 'Revert "dmaengine: imx-sdma:
> refine
> to load context only once" so that no building waring during
> comes out
> during bisect.
> 2.Address Sascha's comments, including eliminating any i.mx6sx in
> this
> series, adding new 'is_imx6ul_ecspi()' instead imx in imx51 and
> taking
> care SMC bit for PIO.
> 3.Add back missing 'Reviewed-by' tag on 08/15(v5):09/13(v7)
> 'spi: imx: add new i.mx6ul compatible name in binding doc'
> v8:
> 1.remove 0003-Revert-dmaengine-imx-sdma-fix-context-cache.patch and
> merge
> it into 04/13 of v7
> 2.add 0005-spi-imx-fallback-to-PIO-if-dma-setup-failure.patch for
> no any
> ecspi function broken even if sdma firmware not updated.
> 3.merge 'tx.dst_maxburst' changes in the two continous patches into
> one
> patch to avoid confusion.
> 4.fix typo 'duplicated'.
>
> Robin Gong (13):
> Revert "ARM: dts: imx6q: Use correct SDMA script for SPI5 core"
> Revert "ARM: dts: imx6: Use correct SDMA script for SPI cores"
> Revert "dmaengine: imx-sdma: refine to load context only once"
> dmaengine: imx-sdma: remove duplicated sdma_load_context
> spi: imx: fallback to PIO if dma setup failure
> dmaengine: imx-sdma: add mcu_2_ecspi script
> spi: imx: fix ERR009165
> spi: imx: remove ERR009165 workaround on i.mx6ul
> spi: imx: add new i.mx6ul compatible name in binding doc
> dmaengine: imx-sdma: remove ERR009165 on i.mx6ul
> dma: imx-sdma: add i.mx6ul compatible name
> dmaengine: imx-sdma: fix ecspi1 rx dma not work on i.mx8mm
> dmaengine: imx-sdma: add uart rom script
>
> .../devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
> .../devicetree/bindings/spi/fsl-imx-cspi.txt | 1 +
> arch/arm/boot/dts/imx6q.dtsi | 2 +-
> arch/arm/boot/dts/imx6qdl.dtsi | 8 +-
> drivers/dma/imx-sdma.c | 67 ++++++++++
> ------
> drivers/spi/spi-imx.c | 92
> +++++++++++++++++++---
> include/linux/platform_data/dma-imx-sdma.h | 8 +-
> 7 files changed, 135 insertions(+), 44 deletions(-)
>
^ permalink raw reply
* [PATCH v4 1/4] iio: chemical: scd30: add core driver
From: Tomasz Duszynski @ 2020-06-03 8:44 UTC (permalink / raw)
To: linux-iio
Cc: linux-kernel, devicetree, robh+dt, jic23, andy.shevchenko, pmeerw,
Tomasz Duszynski
In-Reply-To: <20200603084441.33952-1-tomasz.duszynski@octakon.com>
Add Sensirion SCD30 carbon dioxide core driver.
Signed-off-by: Tomasz Duszynski <tomasz.duszynski@octakon.com>
---
Documentation/ABI/testing/sysfs-bus-iio-scd30 | 34 +
MAINTAINERS | 6 +
drivers/iio/chemical/Kconfig | 11 +
drivers/iio/chemical/Makefile | 1 +
drivers/iio/chemical/scd30.h | 78 ++
drivers/iio/chemical/scd30_core.c | 761 ++++++++++++++++++
6 files changed, 891 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
create mode 100644 drivers/iio/chemical/scd30.h
create mode 100644 drivers/iio/chemical/scd30_core.c
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-scd30 b/Documentation/ABI/testing/sysfs-bus-iio-scd30
new file mode 100644
index 000000000000..b9712f390bec
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-scd30
@@ -0,0 +1,34 @@
+What: /sys/bus/iio/devices/iio:deviceX/calibration_auto_enable
+Date: June 2020
+KernelVersion: 5.8
+Contact: linux-iio@vger.kernel.org
+Description:
+ Contaminants build-up in the measurement chamber or optical
+ elements deterioration leads to sensor drift.
+
+ One can compensate for sensor drift by using automatic self
+ calibration procedure (asc).
+
+ Writing 1 or 0 to this attribute will respectively activate or
+ deactivate asc.
+
+ Upon reading current asc status is returned.
+
+What: /sys/bus/iio/devices/iio:deviceX/calibration_forced_value
+Date: June 2020
+KernelVersion: 5.8
+Contact: linux-iio@vger.kernel.org
+Description:
+ Contaminants build-up in the measurement chamber or optical
+ elements deterioration leads to sensor drift.
+
+ One can compensate for sensor drift by using forced
+ recalibration (frc). This is useful in case there's known
+ co2 reference available nearby the sensor.
+
+ Picking value from the range [400 1 2000] and writing it to the
+ sensor will set frc.
+
+ Upon reading current frc value is returned. Note that after
+ power cycling default value (i.e 400) is returned even though
+ internally sensor had recalibrated itself.
diff --git a/MAINTAINERS b/MAINTAINERS
index 60ed2963efaa..41a509cca6f1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15137,6 +15137,12 @@ S: Maintained
F: drivers/misc/phantom.c
F: include/uapi/linux/phantom.h
+SENSIRION SCD30 CARBON DIOXIDE SENSOR DRIVER
+M: Tomasz Duszynski <tomasz.duszynski@octakon.com>
+S: Maintained
+F: drivers/iio/chemical/scd30.h
+F: drivers/iio/chemical/scd30_core.c
+
SENSIRION SPS30 AIR POLLUTION SENSOR DRIVER
M: Tomasz Duszynski <tduszyns@gmail.com>
S: Maintained
diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index 7f21afd73b1c..99e852b67e55 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -85,6 +85,17 @@ config PMS7003
To compile this driver as a module, choose M here: the module will
be called pms7003.
+config SCD30_CORE
+ tristate "SCD30 carbon dioxide sensor driver"
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
+ help
+ Say Y here to build support for the Sensirion SCD30 sensor with carbon
+ dioxide, relative humidity and temperature sensing capabilities.
+
+ To compile this driver as a module, choose M here: the module will
+ be called scd30_core.
+
config SENSIRION_SGP30
tristate "Sensirion SGPxx gas sensors"
depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index aba4167db745..c9804b041ecd 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_BME680_SPI) += bme680_spi.o
obj-$(CONFIG_CCS811) += ccs811.o
obj-$(CONFIG_IAQCORE) += ams-iaq-core.o
obj-$(CONFIG_PMS7003) += pms7003.o
+obj-$(CONFIG_SCD30_CORE) += scd30_core.o
obj-$(CONFIG_SENSIRION_SGP30) += sgp30.o
obj-$(CONFIG_SPS30) += sps30.o
obj-$(CONFIG_VZ89X) += vz89x.o
diff --git a/drivers/iio/chemical/scd30.h b/drivers/iio/chemical/scd30.h
new file mode 100644
index 000000000000..f60127bfe0f4
--- /dev/null
+++ b/drivers/iio/chemical/scd30.h
@@ -0,0 +1,78 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _SCD30_H
+#define _SCD30_H
+
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/regulator/consumer.h>
+#include <linux/types.h>
+
+struct scd30_state;
+
+enum scd30_cmd {
+ /* start continuous measurement with pressure compensation */
+ CMD_START_MEAS,
+ /* stop continuous measurement */
+ CMD_STOP_MEAS,
+ /* set/get measurement interval */
+ CMD_MEAS_INTERVAL,
+ /* check whether new measurement is ready */
+ CMD_MEAS_READY,
+ /* get measurement */
+ CMD_READ_MEAS,
+ /* turn on/off automatic self calibration */
+ CMD_ASC,
+ /* set/get forced recalibration value */
+ CMD_FRC,
+ /* set/get temperature offset */
+ CMD_TEMP_OFFSET,
+ /* get firmware version */
+ CMD_FW_VERSION,
+ /* reset sensor */
+ CMD_RESET,
+ /*
+ * Command for altitude compensation was omitted intentionally because
+ * the same can be achieved by means of CMD_START_MEAS which takes
+ * pressure above the sea level as an argument.
+ */
+};
+
+#define SCD30_MEAS_COUNT 3
+
+typedef int (*scd30_command_t)(struct scd30_state *state, enum scd30_cmd cmd, u16 arg,
+ void *response, int size);
+
+struct scd30_state {
+ /* serialize access to the device */
+ struct mutex lock;
+ struct device *dev;
+ struct regulator *vdd;
+ struct completion meas_ready;
+ /*
+ * priv pointer is solely for serdev driver private data. We keep it
+ * here because driver_data inside dev has been already used for iio and
+ * struct serdev_device doesn't have one.
+ */
+ void *priv;
+ int irq;
+ /*
+ * no way to retrieve current ambient pressure compensation value from
+ * the sensor so keep one around
+ */
+ u16 pressure_comp;
+ u16 meas_interval;
+ int meas[SCD30_MEAS_COUNT];
+
+ scd30_command_t command;
+};
+
+int scd30_suspend(struct device *dev);
+int scd30_resume(struct device *dev);
+
+static __maybe_unused SIMPLE_DEV_PM_OPS(scd30_pm_ops, scd30_suspend, scd30_resume);
+
+int scd30_probe(struct device *dev, int irq, const char *name, void *priv, scd30_command_t command);
+
+#endif
diff --git a/drivers/iio/chemical/scd30_core.c b/drivers/iio/chemical/scd30_core.c
new file mode 100644
index 000000000000..cf640a00f7ec
--- /dev/null
+++ b/drivers/iio/chemical/scd30_core.c
@@ -0,0 +1,761 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sensirion SCD30 carbon dioxide sensor core driver
+ *
+ * Copyright (c) 2020 Tomasz Duszynski <tomasz.duszynski@octakon.com>
+ */
+#include <linux/bits.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/types.h>
+#include <linux/interrupt.h>
+#include <linux/irqreturn.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regulator/consumer.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+#include <asm/byteorder.h>
+
+#include "scd30.h"
+
+#define SCD30_PRESSURE_COMP_MIN_MBAR 700
+#define SCD30_PRESSURE_COMP_MAX_MBAR 1400
+#define SCD30_PRESSURE_COMP_DEFAULT 1013
+#define SCD30_MEAS_INTERVAL_MIN_S 2
+#define SCD30_MEAS_INTERVAL_MAX_S 1800
+#define SCD30_MEAS_INTERVAL_DEFAULT SCD30_MEAS_INTERVAL_MIN_S
+#define SCD30_FRC_MIN_PPM 400
+#define SCD30_FRC_MAX_PPM 2000
+#define SCD30_TEMP_OFFSET_MAX 655360
+#define SCD30_EXTRA_TIMEOUT_PER_S 250
+
+enum {
+ SCD30_CONC,
+ SCD30_TEMP,
+ SCD30_HR,
+};
+
+static int scd30_command_write(struct scd30_state *state, enum scd30_cmd cmd, u16 arg)
+{
+ return state->command(state, cmd, arg, NULL, 0);
+}
+
+static int scd30_command_read(struct scd30_state *state, enum scd30_cmd cmd, u16 *val)
+{
+ __be16 tmp;
+ int ret;
+
+ ret = state->command(state, cmd, 0, &tmp, sizeof(tmp));
+ *val = be16_to_cpup(&tmp);
+
+ return ret;
+}
+
+static int scd30_reset(struct scd30_state *state)
+{
+ int ret;
+ u16 val;
+
+ ret = scd30_command_write(state, CMD_RESET, 0);
+ if (ret)
+ return ret;
+
+ /* sensor boots up within 2 secs */
+ msleep(2000);
+ /*
+ * Power-on-reset causes sensor to produce some glitch on i2c bus and
+ * some controllers end up in error state. Try to recover by placing
+ * any data on the bus.
+ */
+ scd30_command_read(state, CMD_MEAS_READY, &val);
+
+ return 0;
+}
+
+/* simplified float to fixed point conversion with a scaling factor of 0.01 */
+static int scd30_float_to_fp(int float32)
+{
+ int fraction, shift,
+ mantissa = float32 & GENMASK(22, 0),
+ sign = float32 & BIT(31) ? -1 : 1,
+ exp = (float32 & ~BIT(31)) >> 23;
+
+ /* special case 0 */
+ if (!exp && !mantissa)
+ return 0;
+
+ exp -= 127;
+ if (exp < 0) {
+ exp = -exp;
+ /* return values ranging from 1 to 99 */
+ return sign * ((((BIT(23) + mantissa) * 100) >> 23) >> exp);
+ }
+
+ /* return values starting at 100 */
+ shift = 23 - exp;
+ float32 = BIT(exp) + (mantissa >> shift);
+ fraction = mantissa & GENMASK(shift - 1, 0);
+
+ return sign * (float32 * 100 + ((fraction * 100) >> shift));
+}
+
+static int scd30_read_meas(struct scd30_state *state)
+{
+ int i, ret;
+
+ ret = state->command(state, CMD_READ_MEAS, 0, state->meas, sizeof(state->meas));
+ if (ret)
+ return ret;
+
+ be32_to_cpu_array(state->meas, state->meas, ARRAY_SIZE(state->meas));
+
+ for (i = 0; i < ARRAY_SIZE(state->meas); i++)
+ state->meas[i] = scd30_float_to_fp(state->meas[i]);
+
+ /*
+ * co2 is left unprocessed while temperature and humidity are scaled
+ * to milli deg C and milli percent respectively.
+ */
+ state->meas[SCD30_TEMP] *= 10;
+ state->meas[SCD30_HR] *= 10;
+
+ return 0;
+}
+
+static int scd30_wait_meas_irq(struct scd30_state *state)
+{
+ int ret, timeout;
+
+ reinit_completion(&state->meas_ready);
+ enable_irq(state->irq);
+ timeout = msecs_to_jiffies(state->meas_interval * (1000 + SCD30_EXTRA_TIMEOUT_PER_S));
+ ret = wait_for_completion_interruptible_timeout(&state->meas_ready, timeout);
+ if (ret > 0)
+ ret = 0;
+ else if (!ret)
+ ret = -ETIMEDOUT;
+
+ disable_irq(state->irq);
+
+ return ret;
+}
+
+static int scd30_wait_meas_poll(struct scd30_state *state)
+{
+ int timeout = state->meas_interval * SCD30_EXTRA_TIMEOUT_PER_S, tries = 5;
+
+ do {
+ int ret;
+ u16 val;
+
+ ret = scd30_command_read(state, CMD_MEAS_READY, &val);
+ if (ret)
+ return -EIO;
+
+ /* new measurement available */
+ if (val)
+ break;
+
+ msleep_interruptible(timeout);
+ } while (--tries);
+
+ return tries ? 0 : -ETIMEDOUT;
+}
+
+static int scd30_read_poll(struct scd30_state *state)
+{
+ int ret;
+
+ ret = scd30_wait_meas_poll(state);
+ if (ret)
+ return ret;
+
+ return scd30_read_meas(state);
+}
+
+static int scd30_read(struct scd30_state *state)
+{
+ if (state->irq > 0)
+ return scd30_wait_meas_irq(state);
+
+ return scd30_read_poll(state);
+}
+
+static int scd30_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct scd30_state *state = iio_priv(indio_dev);
+ int ret = -EINVAL;
+ u16 tmp;
+
+ mutex_lock(&state->lock);
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ case IIO_CHAN_INFO_PROCESSED:
+ ret = iio_device_claim_direct_mode(indio_dev);
+ if (ret)
+ break;
+
+ ret = scd30_read(state);
+ if (ret) {
+ iio_device_release_direct_mode(indio_dev);
+ break;
+ }
+
+ *val = state->meas[chan->address];
+ iio_device_release_direct_mode(indio_dev);
+ ret = IIO_VAL_INT;
+ break;
+ case IIO_CHAN_INFO_SCALE:
+ *val = 0;
+ *val2 = 1;
+ ret = IIO_VAL_INT_PLUS_MICRO;
+ break;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ ret = scd30_command_read(state, CMD_MEAS_INTERVAL, &tmp);
+ if (ret)
+ break;
+
+ *val = 0;
+ *val2 = 1000000000 / tmp;
+ ret = IIO_VAL_INT_PLUS_NANO;
+ break;
+ case IIO_CHAN_INFO_CALIBSCALE:
+ *val = state->pressure_comp / 10;
+ *val2 = (state->pressure_comp % 10) * 100000;
+ ret = IIO_VAL_INT_PLUS_MICRO;
+ break;
+ case IIO_CHAN_INFO_CALIBBIAS:
+ ret = scd30_command_read(state, CMD_TEMP_OFFSET, &tmp);
+ if (ret)
+ break;
+
+ *val = tmp;
+ ret = IIO_VAL_INT;
+ break;
+ }
+ mutex_unlock(&state->lock);
+
+ return ret;
+}
+
+static int scd30_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct scd30_state *state = iio_priv(indio_dev);
+ int ret = -EINVAL;
+
+ mutex_lock(&state->lock);
+ switch (mask) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ if (val)
+ break;
+
+ val = 1000000000 / val2;
+ if (val < SCD30_MEAS_INTERVAL_MIN_S || val > SCD30_MEAS_INTERVAL_MAX_S)
+ break;
+
+ ret = scd30_command_write(state, CMD_MEAS_INTERVAL, val);
+ if (ret)
+ break;
+
+ state->meas_interval = val;
+ break;
+ case IIO_CHAN_INFO_CALIBSCALE:
+ val = (val * 1000000 + val2) / 100000;
+ if (val < SCD30_PRESSURE_COMP_MIN_MBAR || val > SCD30_PRESSURE_COMP_MAX_MBAR)
+ break;
+
+ ret = scd30_command_write(state, CMD_START_MEAS, val);
+ if (ret)
+ break;
+
+ state->pressure_comp = val;
+ break;
+ case IIO_CHAN_INFO_CALIBBIAS:
+ if (val < 0 || val > SCD30_TEMP_OFFSET_MAX)
+ break;
+ /*
+ * Manufacturer does not explicitly specify min/max sensible
+ * values hence check is omitted for simplicity.
+ */
+ ret = scd30_command_write(state, CMD_TEMP_OFFSET / 10, val);
+ }
+ mutex_unlock(&state->lock);
+
+ return ret;
+}
+
+static int scd30_write_raw_get_fmt(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+ long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ return IIO_VAL_INT_PLUS_NANO;
+ case IIO_CHAN_INFO_CALIBSCALE:
+ return IIO_VAL_INT_PLUS_MICRO;
+ case IIO_CHAN_INFO_CALIBBIAS:
+ return IIO_VAL_INT;
+ }
+
+ return -EINVAL;
+}
+
+static const int scd30_pressure_calibscale_available[] = {
+ SCD30_PRESSURE_COMP_MIN_MBAR / 10, 0,
+ 0, 100000,
+ SCD30_PRESSURE_COMP_MAX_MBAR / 10, 0,
+};
+
+static const int scd30_temp_calibbias_available[] = {
+ 0, 10, SCD30_TEMP_OFFSET_MAX,
+};
+
+static int scd30_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length, long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_CALIBSCALE:
+ *vals = scd30_pressure_calibscale_available;
+ *type = IIO_VAL_INT_PLUS_MICRO;
+
+ return IIO_AVAIL_RANGE;
+ case IIO_CHAN_INFO_CALIBBIAS:
+ *vals = scd30_temp_calibbias_available;
+ *type = IIO_VAL_INT;
+
+ return IIO_AVAIL_RANGE;
+ }
+
+ return -EINVAL;
+}
+
+static ssize_t sampling_frequency_available_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ int i = SCD30_MEAS_INTERVAL_MIN_S;
+ ssize_t len = 0;
+
+ do {
+ len += scnprintf(buf + len, PAGE_SIZE - len, "0.%09u ", 1000000000 / i);
+ /*
+ * Not all values fit PAGE_SIZE buffer hence print every 6th
+ * (each frequency differs by 6s in time domain from the
+ * adjacent). Unlisted but valid ones are still accepted.
+ */
+ i += 6;
+ } while (i <= SCD30_MEAS_INTERVAL_MAX_S);
+
+ buf[len - 1] = '\n';
+
+ return len;
+}
+
+static ssize_t calibration_auto_enable_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct scd30_state *state = iio_priv(indio_dev);
+ int ret;
+ u16 val;
+
+ mutex_lock(&state->lock);
+ ret = scd30_command_read(state, CMD_ASC, &val);
+ mutex_unlock(&state->lock);
+
+ return ret ?: sprintf(buf, "%d\n", val);
+}
+
+static ssize_t calibration_auto_enable_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct scd30_state *state = iio_priv(indio_dev);
+ int ret;
+ u16 val;
+
+ ret = kstrtou16(buf, 0, &val);
+ if (ret)
+ return ret;
+
+ mutex_lock(&state->lock);
+ ret = scd30_command_write(state, CMD_ASC, !!val);
+ mutex_unlock(&state->lock);
+
+ return ret ?: len;
+}
+
+static ssize_t calibration_forced_value_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct scd30_state *state = iio_priv(indio_dev);
+ int ret;
+ u16 val;
+
+ mutex_lock(&state->lock);
+ ret = scd30_command_read(state, CMD_FRC, &val);
+ mutex_unlock(&state->lock);
+
+ return ret ?: sprintf(buf, "%d\n", val);
+}
+
+static ssize_t calibration_forced_value_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct scd30_state *state = iio_priv(indio_dev);
+ int ret;
+ u16 val;
+
+ ret = kstrtou16(buf, 0, &val);
+ if (ret)
+ return ret;
+
+ if (val < SCD30_FRC_MIN_PPM || val > SCD30_FRC_MAX_PPM)
+ return -EINVAL;
+
+ mutex_lock(&state->lock);
+ ret = scd30_command_write(state, CMD_FRC, val);
+ mutex_unlock(&state->lock);
+
+ return ret ?: len;
+}
+
+static IIO_DEVICE_ATTR_RO(sampling_frequency_available, 0);
+static IIO_DEVICE_ATTR_RW(calibration_auto_enable, 0);
+static IIO_DEVICE_ATTR_RW(calibration_forced_value, 0);
+
+static struct attribute *scd30_attrs[] = {
+ &iio_dev_attr_sampling_frequency_available.dev_attr.attr,
+ &iio_dev_attr_calibration_auto_enable.dev_attr.attr,
+ &iio_dev_attr_calibration_forced_value.dev_attr.attr,
+ NULL
+};
+
+static const struct attribute_group scd30_attr_group = {
+ .attrs = scd30_attrs,
+};
+
+static const struct iio_info scd30_info = {
+ .attrs = &scd30_attr_group,
+ .read_raw = scd30_read_raw,
+ .write_raw = scd30_write_raw,
+ .write_raw_get_fmt = scd30_write_raw_get_fmt,
+ .read_avail = scd30_read_avail,
+};
+
+#define SCD30_CHAN_SCAN_TYPE(_sign, _realbits) .scan_type = { \
+ .sign = _sign, \
+ .realbits = _realbits, \
+ .storagebits = 32, \
+ .endianness = IIO_CPU, \
+}
+
+static const struct iio_chan_spec scd30_channels[] = {
+ {
+ .type = IIO_PRESSURE,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_CALIBSCALE),
+ .info_mask_separate_available = BIT(IIO_CHAN_INFO_CALIBSCALE),
+ .scan_index = -1,
+ },
+ {
+ .type = IIO_CONCENTRATION,
+ .channel2 = IIO_MOD_CO2,
+ .address = SCD30_CONC,
+ .scan_index = SCD30_CONC,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE),
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+ .modified = 1,
+
+ SCD30_CHAN_SCAN_TYPE('u', 20),
+ },
+ {
+ .type = IIO_TEMP,
+ .address = SCD30_TEMP,
+ .scan_index = SCD30_TEMP,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+ BIT(IIO_CHAN_INFO_CALIBBIAS),
+ .info_mask_separate_available = BIT(IIO_CHAN_INFO_CALIBBIAS),
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+
+ SCD30_CHAN_SCAN_TYPE('s', 18),
+ },
+ {
+ .type = IIO_HUMIDITYRELATIVE,
+ .address = SCD30_HR,
+ .scan_index = SCD30_HR,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+
+ SCD30_CHAN_SCAN_TYPE('u', 17),
+ },
+ IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+int __maybe_unused scd30_suspend(struct device *dev)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+ struct scd30_state *state = iio_priv(indio_dev);
+ int ret;
+
+ ret = scd30_command_write(state, CMD_STOP_MEAS, 0);
+ if (ret)
+ return ret;
+
+ return regulator_disable(state->vdd);
+}
+EXPORT_SYMBOL(scd30_suspend);
+
+int __maybe_unused scd30_resume(struct device *dev)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(dev);
+ struct scd30_state *state = iio_priv(indio_dev);
+ int ret;
+
+ ret = regulator_enable(state->vdd);
+ if (ret)
+ return ret;
+
+ return scd30_command_write(state, CMD_START_MEAS, state->pressure_comp);
+}
+EXPORT_SYMBOL(scd30_resume);
+
+static void scd30_stop_meas(void *data)
+{
+ struct scd30_state *state = data;
+
+ scd30_command_write(state, CMD_STOP_MEAS, 0);
+}
+
+static void scd30_disable_regulator(void *data)
+{
+ struct scd30_state *state = data;
+
+ regulator_disable(state->vdd);
+}
+
+static irqreturn_t scd30_irq_handler(int irq, void *priv)
+{
+ struct iio_dev *indio_dev = priv;
+
+ if (iio_buffer_enabled(indio_dev)) {
+ iio_trigger_poll(indio_dev->trig);
+
+ return IRQ_HANDLED;
+ }
+
+ return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t scd30_irq_thread_handler(int irq, void *priv)
+{
+ struct iio_dev *indio_dev = priv;
+ struct scd30_state *state = iio_priv(indio_dev);
+ int ret;
+
+ ret = scd30_read_meas(state);
+ if (ret)
+ goto out;
+
+ complete_all(&state->meas_ready);
+out:
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t scd30_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct scd30_state *state = iio_priv(indio_dev);
+ struct {
+ int data[SCD30_MEAS_COUNT];
+ s64 ts __aligned(8);
+ } scan = { 0, };
+ int ret;
+
+ mutex_lock(&state->lock);
+ if (!iio_trigger_using_own(indio_dev))
+ ret = scd30_read_poll(state);
+ else
+ ret = scd30_read_meas(state);
+ memcpy(scan.data, state->meas, sizeof(state->meas));
+ mutex_unlock(&state->lock);
+ if (ret)
+ goto out;
+
+ iio_push_to_buffers_with_timestamp(indio_dev, &scan, iio_get_time_ns(indio_dev));
+out:
+ iio_trigger_notify_done(indio_dev->trig);
+ return IRQ_HANDLED;
+}
+
+static int scd30_set_trigger_state(struct iio_trigger *trig, bool state)
+{
+ struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+ struct scd30_state *st = iio_priv(indio_dev);
+
+ if (state)
+ enable_irq(st->irq);
+ else
+ disable_irq(st->irq);
+
+ return 0;
+}
+
+static const struct iio_trigger_ops scd30_trigger_ops = {
+ .set_trigger_state = scd30_set_trigger_state,
+ .validate_device = iio_trigger_validate_own_device,
+};
+
+static int scd30_setup_trigger(struct iio_dev *indio_dev)
+{
+ struct scd30_state *state = iio_priv(indio_dev);
+ struct device *dev = indio_dev->dev.parent;
+ struct iio_trigger *trig;
+ int ret;
+
+ trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name, indio_dev->id);
+ if (!trig) {
+ dev_err(dev, "failed to allocate trigger\n");
+ return -ENOMEM;
+ }
+
+ trig->dev.parent = dev;
+ trig->ops = &scd30_trigger_ops;
+ iio_trigger_set_drvdata(trig, indio_dev);
+
+ ret = devm_iio_trigger_register(dev, trig);
+ if (ret)
+ return ret;
+
+ indio_dev->trig = iio_trigger_get(trig);
+
+ ret = devm_request_threaded_irq(dev, state->irq, scd30_irq_handler,
+ scd30_irq_thread_handler, IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ indio_dev->name, indio_dev);
+ if (ret)
+ dev_err(dev, "failed to request irq\n");
+
+ /*
+ * Interrupt is enabled just before taking a fresh measurement
+ * and disabled afterwards. This means we need to disable it here
+ * to keep calls to enable/disable balanced.
+ */
+ disable_irq(state->irq);
+
+ return ret;
+}
+
+int scd30_probe(struct device *dev, int irq, const char *name, void *priv,
+ scd30_command_t command)
+{
+ static const unsigned long scd30_scan_masks[] = { 0x07, 0x00 };
+ struct scd30_state *state;
+ struct iio_dev *indio_dev;
+ int ret;
+ u16 val;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ state = iio_priv(indio_dev);
+ state->dev = dev;
+ state->priv = priv;
+ state->irq = irq;
+ state->pressure_comp = SCD30_PRESSURE_COMP_DEFAULT;
+ state->meas_interval = SCD30_MEAS_INTERVAL_DEFAULT;
+ state->command = command;
+ mutex_init(&state->lock);
+ init_completion(&state->meas_ready);
+
+ dev_set_drvdata(dev, indio_dev);
+
+ indio_dev->dev.parent = dev;
+ indio_dev->info = &scd30_info;
+ indio_dev->name = name;
+ indio_dev->channels = scd30_channels;
+ indio_dev->num_channels = ARRAY_SIZE(scd30_channels);
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->available_scan_masks = scd30_scan_masks;
+
+ state->vdd = devm_regulator_get(dev, "vdd");
+ if (IS_ERR(state->vdd)) {
+ if (PTR_ERR(state->vdd) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+
+ dev_err(dev, "failed to get regulator\n");
+ return PTR_ERR(state->vdd);
+ }
+
+ ret = regulator_enable(state->vdd);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(dev, scd30_disable_regulator, state);
+ if (ret)
+ return ret;
+
+ ret = scd30_reset(state);
+ if (ret) {
+ dev_err(dev, "failed to reset device: %d\n", ret);
+ return ret;
+ }
+
+ if (state->irq > 0) {
+ ret = scd30_setup_trigger(indio_dev);
+ if (ret) {
+ dev_err(dev, "failed to setup trigger: %d\n", ret);
+ return ret;
+ }
+ }
+
+ ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL, scd30_trigger_handler, NULL);
+ if (ret)
+ return ret;
+
+ ret = scd30_command_read(state, CMD_FW_VERSION, &val);
+ if (ret) {
+ dev_err(dev, "failed to read firmware version: %d\n", ret);
+ return ret;
+ }
+ dev_info(dev, "firmware version: %d.%d\n", val >> 8, (char)val);
+
+ ret = scd30_command_write(state, CMD_MEAS_INTERVAL, state->meas_interval);
+ if (ret) {
+ dev_err(dev, "failed to set measurement interval: %d\n", ret);
+ return ret;
+ }
+
+ ret = scd30_command_write(state, CMD_START_MEAS, state->pressure_comp);
+ if (ret) {
+ dev_err(dev, "failed to start measurement: %d\n", ret);
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(dev, scd30_stop_meas, state);
+ if (ret)
+ return ret;
+
+ return devm_iio_device_register(dev, indio_dev);
+}
+EXPORT_SYMBOL(scd30_probe);
+
+MODULE_AUTHOR("Tomasz Duszynski <tomasz.duszynski@octakon.com>");
+MODULE_DESCRIPTION("Sensirion SCD30 carbon dioxide sensor core driver");
+MODULE_LICENSE("GPL v2");
--
2.27.0
^ permalink raw reply related
* [PATCH v4 4/4] dt-bindings: iio: scd30: add device binding file
From: Tomasz Duszynski @ 2020-06-03 8:44 UTC (permalink / raw)
To: linux-iio
Cc: linux-kernel, devicetree, robh+dt, jic23, andy.shevchenko, pmeerw,
Tomasz Duszynski
In-Reply-To: <20200603084441.33952-1-tomasz.duszynski@octakon.com>
Add SCD30 sensor binding file.
Signed-off-by: Tomasz Duszynski <tomasz.duszynski@octakon.com>
---
.../iio/chemical/sensirion,scd30.yaml | 68 +++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 69 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd30.yaml
diff --git a/Documentation/devicetree/bindings/iio/chemical/sensirion,scd30.yaml b/Documentation/devicetree/bindings/iio/chemical/sensirion,scd30.yaml
new file mode 100644
index 000000000000..40d87346ff4c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/chemical/sensirion,scd30.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/chemical/sensirion,scd30.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sensirion SCD30 carbon dioxide sensor
+
+maintainers:
+ - Tomasz Duszynski <tomasz.duszynski@octakon.com>
+
+description: |
+ Air quality sensor capable of measuring co2 concentration, temperature
+ and relative humidity.
+
+properties:
+ compatible:
+ enum:
+ - sensirion,scd30
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ vdd-supply: true
+
+ sensirion,sel-gpios:
+ description: GPIO connected to the SEL line
+ maxItems: 1
+
+ sensirion,pwm-gpios:
+ description: GPIO connected to the PWM line
+ maxItems: 1
+
+required:
+ - compatible
+
+additionalProperties: false
+
+examples:
+ - |
+ # include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ co2-sensor@61 {
+ compatible = "sensirion,scd30";
+ reg = <0x61>;
+ vdd-supply = <&vdd>;
+ interrupt-parent = <&gpio0>;
+ interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+ };
+ };
+ - |
+ # include <dt-bindings/interrupt-controller/irq.h>
+ serial {
+ co2-sensor {
+ compatible = "sensirion,scd30";
+ vdd-supply = <&vdd>;
+ interrupt-parent = <&gpio0>;
+ interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+ };
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 5db4b446c8ba..0ab9cf39e051 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15140,6 +15140,7 @@ F: include/uapi/linux/phantom.h
SENSIRION SCD30 CARBON DIOXIDE SENSOR DRIVER
M: Tomasz Duszynski <tomasz.duszynski@octakon.com>
S: Maintained
+F: Documentation/devicetree/bindings/iio/chemical/sensirion,scd30.yaml
F: drivers/iio/chemical/scd30.h
F: drivers/iio/chemical/scd30_core.c
F: drivers/iio/chemical/scd30_i2c.c
--
2.27.0
^ permalink raw reply related
* [PATCH v4 2/4] iio: chemical: scd30: add I2C interface driver
From: Tomasz Duszynski @ 2020-06-03 8:44 UTC (permalink / raw)
To: linux-iio
Cc: linux-kernel, devicetree, robh+dt, jic23, andy.shevchenko, pmeerw,
Tomasz Duszynski
In-Reply-To: <20200603084441.33952-1-tomasz.duszynski@octakon.com>
Add I2C interface driver for the SCD30 sensor.
Signed-off-by: Tomasz Duszynski <tomasz.duszynski@octakon.com>
---
MAINTAINERS | 1 +
drivers/iio/chemical/Kconfig | 11 +++
drivers/iio/chemical/Makefile | 1 +
drivers/iio/chemical/scd30_i2c.c | 139 +++++++++++++++++++++++++++++++
4 files changed, 152 insertions(+)
create mode 100644 drivers/iio/chemical/scd30_i2c.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 41a509cca6f1..13aed3473b7e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15142,6 +15142,7 @@ M: Tomasz Duszynski <tomasz.duszynski@octakon.com>
S: Maintained
F: drivers/iio/chemical/scd30.h
F: drivers/iio/chemical/scd30_core.c
+F: drivers/iio/chemical/scd30_i2c.c
SENSIRION SPS30 AIR POLLUTION SENSOR DRIVER
M: Tomasz Duszynski <tduszyns@gmail.com>
diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index 99e852b67e55..970d34888c2e 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -96,6 +96,17 @@ config SCD30_CORE
To compile this driver as a module, choose M here: the module will
be called scd30_core.
+config SCD30_I2C
+ tristate "SCD30 carbon dioxide sensor I2C driver"
+ depends on SCD30_CORE && I2C
+ select CRC8
+ help
+ Say Y here to build support for the Sensirion SCD30 I2C interface
+ driver.
+
+ To compile this driver as a module, choose M here: the module will
+ be called scd30_i2c.
+
config SENSIRION_SGP30
tristate "Sensirion SGPxx gas sensors"
depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index c9804b041ecd..0966ca34e34b 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_CCS811) += ccs811.o
obj-$(CONFIG_IAQCORE) += ams-iaq-core.o
obj-$(CONFIG_PMS7003) += pms7003.o
obj-$(CONFIG_SCD30_CORE) += scd30_core.o
+obj-$(CONFIG_SCD30_I2C) += scd30_i2c.o
obj-$(CONFIG_SENSIRION_SGP30) += sgp30.o
obj-$(CONFIG_SPS30) += sps30.o
obj-$(CONFIG_VZ89X) += vz89x.o
diff --git a/drivers/iio/chemical/scd30_i2c.c b/drivers/iio/chemical/scd30_i2c.c
new file mode 100644
index 000000000000..875892a070ee
--- /dev/null
+++ b/drivers/iio/chemical/scd30_i2c.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sensirion SCD30 carbon dioxide sensor i2c driver
+ *
+ * Copyright (c) 2020 Tomasz Duszynski <tomasz.duszynski@octakon.com>
+ *
+ * I2C slave address: 0x61
+ */
+#include <linux/crc8.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <asm/unaligned.h>
+
+#include "scd30.h"
+
+#define SCD30_I2C_MAX_BUF_SIZE 18
+#define SCD30_I2C_CRC8_POLYNOMIAL 0x31
+
+static u16 scd30_i2c_cmd_lookup_tbl[] = {
+ [CMD_START_MEAS] = 0x0010,
+ [CMD_STOP_MEAS] = 0x0104,
+ [CMD_MEAS_INTERVAL] = 0x4600,
+ [CMD_MEAS_READY] = 0x0202,
+ [CMD_READ_MEAS] = 0x0300,
+ [CMD_ASC] = 0x5306,
+ [CMD_FRC] = 0x5204,
+ [CMD_TEMP_OFFSET] = 0x5403,
+ [CMD_FW_VERSION] = 0xd100,
+ [CMD_RESET] = 0xd304,
+};
+
+DECLARE_CRC8_TABLE(scd30_i2c_crc8_tbl);
+
+static int scd30_i2c_xfer(struct scd30_state *state, char *txbuf, int txsize,
+ char *rxbuf, int rxsize)
+{
+ struct i2c_client *client = to_i2c_client(state->dev);
+ int ret;
+
+ /*
+ * repeated start is not supported hence instead of sending two i2c
+ * messages in a row we send one by one
+ */
+ ret = i2c_master_send(client, txbuf, txsize);
+ if (ret < 0)
+ return ret;
+ if (ret != txsize)
+ return -EIO;
+
+ if (!rxbuf)
+ return 0;
+
+ ret = i2c_master_recv(client, rxbuf, rxsize);
+ if (ret < 0)
+ return ret;
+ if (ret != rxsize)
+ return -EIO;
+
+ return 0;
+}
+
+static int scd30_i2c_command(struct scd30_state *state, enum scd30_cmd cmd, u16 arg,
+ void *response, int size)
+{
+ char buf[SCD30_I2C_MAX_BUF_SIZE];
+ char *rsp = response;
+ int i, ret;
+ char crc;
+
+ put_unaligned_be16(scd30_i2c_cmd_lookup_tbl[cmd], buf);
+ i = 2;
+
+ if (rsp) {
+ /* each two bytes are followed by a crc8 */
+ size += size / 2;
+ } else {
+ put_unaligned_be16(arg, buf + i);
+ crc = crc8(scd30_i2c_crc8_tbl, buf + i, 2, CRC8_INIT_VALUE);
+ i += 2;
+ buf[i] = crc;
+ i += 1;
+
+ /* commands below don't take an argument */
+ if ((cmd == CMD_STOP_MEAS) || (cmd == CMD_RESET))
+ i -= 3;
+ }
+
+ ret = scd30_i2c_xfer(state, buf, i, buf, size);
+ if (ret)
+ return ret;
+
+ /* validate received data and strip off crc bytes */
+ for (i = 0; i < size; i += 3) {
+ crc = crc8(scd30_i2c_crc8_tbl, buf + i, 2, CRC8_INIT_VALUE);
+ if (crc != buf[i + 2]) {
+ dev_err(state->dev, "data integrity check failed\n");
+ return -EIO;
+ }
+
+ *rsp++ = buf[i];
+ *rsp++ = buf[i + 1];
+ }
+
+ return 0;
+}
+
+static int scd30_i2c_probe(struct i2c_client *client)
+{
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+ return -EOPNOTSUPP;
+
+ crc8_populate_msb(scd30_i2c_crc8_tbl, SCD30_I2C_CRC8_POLYNOMIAL);
+
+ return scd30_probe(&client->dev, client->irq, client->name, NULL, scd30_i2c_command);
+}
+
+static const struct of_device_id scd30_i2c_of_match[] = {
+ { .compatible = "sensirion,scd30" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, scd30_i2c_of_match);
+
+static struct i2c_driver scd30_i2c_driver = {
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = scd30_i2c_of_match,
+ .pm = &scd30_pm_ops,
+ },
+ .probe_new = scd30_i2c_probe,
+};
+module_i2c_driver(scd30_i2c_driver);
+
+MODULE_AUTHOR("Tomasz Duszynski <tomasz.duszynski@octakon.com>");
+MODULE_DESCRIPTION("Sensirion SCD30 carbon dioxide sensor i2c driver");
+MODULE_LICENSE("GPL v2");
--
2.27.0
^ permalink raw reply related
* [PATCH v4 3/4] iio: chemical: scd30: add serial interface driver
From: Tomasz Duszynski @ 2020-06-03 8:44 UTC (permalink / raw)
To: linux-iio
Cc: linux-kernel, devicetree, robh+dt, jic23, andy.shevchenko, pmeerw,
Tomasz Duszynski
In-Reply-To: <20200603084441.33952-1-tomasz.duszynski@octakon.com>
Add serial interface driver for the SCD30 sensor.
Signed-off-by: Tomasz Duszynski <tomasz.duszynski@octakon.com>
---
MAINTAINERS | 1 +
drivers/iio/chemical/Kconfig | 11 ++
drivers/iio/chemical/Makefile | 1 +
drivers/iio/chemical/scd30_serial.c | 263 ++++++++++++++++++++++++++++
4 files changed, 276 insertions(+)
create mode 100644 drivers/iio/chemical/scd30_serial.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 13aed3473b7e..5db4b446c8ba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15143,6 +15143,7 @@ S: Maintained
F: drivers/iio/chemical/scd30.h
F: drivers/iio/chemical/scd30_core.c
F: drivers/iio/chemical/scd30_i2c.c
+F: drivers/iio/chemical/scd30_serial.c
SENSIRION SPS30 AIR POLLUTION SENSOR DRIVER
M: Tomasz Duszynski <tduszyns@gmail.com>
diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index 970d34888c2e..10bb431bc3ce 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -107,6 +107,17 @@ config SCD30_I2C
To compile this driver as a module, choose M here: the module will
be called scd30_i2c.
+config SCD30_SERIAL
+ tristate "SCD30 carbon dioxide sensor serial driver"
+ depends on SCD30_CORE && SERIAL_DEV_BUS
+ select CRC16
+ help
+ Say Y here to build support for the Sensirion SCD30 serial interface
+ driver.
+
+ To compile this driver as a module, choose M here: the module will
+ be called scd30_serial.
+
config SENSIRION_SGP30
tristate "Sensirion SGPxx gas sensors"
depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index 0966ca34e34b..fef63dd5bf92 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_IAQCORE) += ams-iaq-core.o
obj-$(CONFIG_PMS7003) += pms7003.o
obj-$(CONFIG_SCD30_CORE) += scd30_core.o
obj-$(CONFIG_SCD30_I2C) += scd30_i2c.o
+obj-$(CONFIG_SCD30_SERIAL) += scd30_serial.o
obj-$(CONFIG_SENSIRION_SGP30) += sgp30.o
obj-$(CONFIG_SPS30) += sps30.o
obj-$(CONFIG_VZ89X) += vz89x.o
diff --git a/drivers/iio/chemical/scd30_serial.c b/drivers/iio/chemical/scd30_serial.c
new file mode 100644
index 000000000000..06f85eb1a4dd
--- /dev/null
+++ b/drivers/iio/chemical/scd30_serial.c
@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sensirion SCD30 carbon dioxide sensor serial driver
+ *
+ * Copyright (c) 2020 Tomasz Duszynski <tomasz.duszynski@octakon.com>
+ */
+#include <linux/crc16.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/iio/iio.h>
+#include <linux/jiffies.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/serdev.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <asm/unaligned.h>
+
+#include "scd30.h"
+
+#define SCD30_SERDEV_ADDR 0x61
+#define SCD30_SERDEV_WRITE 0x06
+#define SCD30_SERDEV_READ 0x03
+#define SCD30_SERDEV_MAX_BUF_SIZE 17
+#define SCD30_SERDEV_RX_HEADER_SIZE 3
+#define SCD30_SERDEV_CRC_SIZE 2
+#define SCD30_SERDEV_TIMEOUT msecs_to_jiffies(200)
+
+struct scd30_serdev_priv {
+ struct completion meas_ready;
+ char *buf;
+ int num_expected;
+ int num;
+};
+
+static u16 scd30_serdev_cmd_lookup_tbl[] = {
+ [CMD_START_MEAS] = 0x0036,
+ [CMD_STOP_MEAS] = 0x0037,
+ [CMD_MEAS_INTERVAL] = 0x0025,
+ [CMD_MEAS_READY] = 0x0027,
+ [CMD_READ_MEAS] = 0x0028,
+ [CMD_ASC] = 0x003a,
+ [CMD_FRC] = 0x0039,
+ [CMD_TEMP_OFFSET] = 0x003b,
+ [CMD_FW_VERSION] = 0x0020,
+ [CMD_RESET] = 0x0034,
+};
+
+static u16 scd30_serdev_calc_crc(const char *buf, int size)
+{
+ return crc16(0xffff, buf, size);
+}
+
+static int scd30_serdev_xfer(struct scd30_state *state, char *txbuf, int txsize,
+ char *rxbuf, int rxsize)
+{
+ struct serdev_device *serdev = to_serdev_device(state->dev);
+ struct scd30_serdev_priv *priv = state->priv;
+ int ret;
+
+ priv->buf = rxbuf;
+ priv->num_expected = rxsize;
+ priv->num = 0;
+
+ ret = serdev_device_write(serdev, txbuf, txsize, SCD30_SERDEV_TIMEOUT);
+ if (ret < 0)
+ return ret;
+ if (ret != txsize)
+ return -EIO;
+
+ ret = wait_for_completion_interruptible_timeout(&priv->meas_ready, SCD30_SERDEV_TIMEOUT);
+ if (ret < 0)
+ return ret;
+ if (!ret)
+ return -ETIMEDOUT;
+
+ return 0;
+}
+
+static int scd30_serdev_command(struct scd30_state *state, enum scd30_cmd cmd, u16 arg,
+ void *response, int size)
+{
+ /*
+ * Communication over serial line is based on modbus protocol (or rather
+ * its variation called modbus over serial to be precise). Upon
+ * receiving a request device should reply with response.
+ *
+ * Frame below represents a request message. Each field takes
+ * exactly one byte.
+ *
+ * +------+------+-----+-----+-------+-------+-----+-----+
+ * | dev | op | reg | reg | byte1 | byte0 | crc | crc |
+ * | addr | code | msb | lsb | | | lsb | msb |
+ * +------+------+-----+-----+-------+-------+-----+-----+
+ *
+ * The message device replies with depends on the 'op code' field from
+ * the request. In case it was set to SCD30_SERDEV_WRITE sensor should
+ * reply with unchanged request. Otherwise 'op code' was set to
+ * SCD30_SERDEV_READ and response looks like the one below. As with
+ * request, each field takes one byte.
+ *
+ * +------+------+--------+-------+-----+-------+-----+-----+
+ * | dev | op | num of | byte0 | ... | byteN | crc | crc |
+ * | addr | code | bytes | | | | lsb | msb |
+ * +------+------+--------+-------+-----+-------+-----+-----+
+ */
+ char txbuf[SCD30_SERDEV_MAX_BUF_SIZE] = { SCD30_SERDEV_ADDR },
+ rxbuf[SCD30_SERDEV_MAX_BUF_SIZE];
+ int ret, rxsize, txsize = 2;
+ char *rsp = response;
+ u16 crc;
+
+ put_unaligned_be16(scd30_serdev_cmd_lookup_tbl[cmd], txbuf + txsize);
+ txsize += 2;
+
+ if (rsp) {
+ txbuf[1] = SCD30_SERDEV_READ;
+ if (cmd == CMD_READ_MEAS)
+ /* number of u16 words to read */
+ put_unaligned_be16(size / 2, txbuf + txsize);
+ else
+ put_unaligned_be16(0x0001, txbuf + txsize);
+ txsize += 2;
+ crc = scd30_serdev_calc_crc(txbuf, txsize);
+ put_unaligned_le16(crc, txbuf + txsize);
+ txsize += 2;
+ rxsize = SCD30_SERDEV_RX_HEADER_SIZE + size + SCD30_SERDEV_CRC_SIZE;
+ } else {
+ if ((cmd == CMD_STOP_MEAS) || (cmd == CMD_RESET))
+ arg = 0x0001;
+
+ txbuf[1] = SCD30_SERDEV_WRITE;
+ put_unaligned_be16(arg, txbuf + txsize);
+ txsize += 2;
+ crc = scd30_serdev_calc_crc(txbuf, txsize);
+ put_unaligned_le16(crc, txbuf + txsize);
+ txsize += 2;
+ rxsize = txsize;
+ }
+
+ ret = scd30_serdev_xfer(state, txbuf, txsize, rxbuf, rxsize);
+ if (ret)
+ return ret;
+
+ switch (txbuf[1]) {
+ case SCD30_SERDEV_WRITE:
+ if (memcmp(txbuf, rxbuf, txsize)) {
+ dev_err(state->dev, "wrong message received\n");
+ return -EIO;
+ }
+ break;
+ case SCD30_SERDEV_READ:
+ if (rxbuf[2] != (rxsize - SCD30_SERDEV_RX_HEADER_SIZE - SCD30_SERDEV_CRC_SIZE)) {
+ dev_err(state->dev, "received data size does not match header\n");
+ return -EIO;
+ }
+
+ rxsize -= SCD30_SERDEV_CRC_SIZE;
+ crc = get_unaligned_le16(rxbuf + rxsize);
+ if (crc != scd30_serdev_calc_crc(rxbuf, rxsize)) {
+ dev_err(state->dev, "data integrity check failed\n");
+ return -EIO;
+ }
+
+ rxsize -= SCD30_SERDEV_RX_HEADER_SIZE;
+ memcpy(rsp, rxbuf + SCD30_SERDEV_RX_HEADER_SIZE, rxsize);
+ break;
+ default:
+ dev_err(state->dev, "received unknown op code\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int scd30_serdev_receive_buf(struct serdev_device *serdev,
+ const unsigned char *buf, size_t size)
+{
+ struct iio_dev *indio_dev = dev_get_drvdata(&serdev->dev);
+ struct scd30_serdev_priv *priv;
+ struct scd30_state *state;
+ int num;
+
+ if (!indio_dev)
+ return 0;
+
+ state = iio_priv(indio_dev);
+ priv = state->priv;
+
+ /* just in case sensor puts some unexpected bytes on the bus */
+ if (!priv->buf)
+ return 0;
+
+ if (priv->num + size >= priv->num_expected)
+ num = priv->num_expected - priv->num;
+ else
+ num = size;
+
+ memcpy(priv->buf + priv->num, buf, num);
+ priv->num += num;
+
+ if (priv->num == priv->num_expected) {
+ priv->buf = NULL;
+ complete(&priv->meas_ready);
+ }
+
+ return num;
+}
+
+static const struct serdev_device_ops scd30_serdev_ops = {
+ .receive_buf = scd30_serdev_receive_buf,
+ .write_wakeup = serdev_device_write_wakeup,
+};
+
+static int scd30_serdev_probe(struct serdev_device *serdev)
+{
+ struct device *dev = &serdev->dev;
+ struct scd30_serdev_priv *priv;
+ int irq, ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ init_completion(&priv->meas_ready);
+ serdev_device_set_client_ops(serdev, &scd30_serdev_ops);
+
+ ret = devm_serdev_device_open(dev, serdev);
+ if (ret)
+ return ret;
+
+ serdev_device_set_baudrate(serdev, 19200);
+ serdev_device_set_flow_control(serdev, false);
+
+ ret = serdev_device_set_parity(serdev, SERDEV_PARITY_NONE);
+ if (ret)
+ return ret;
+
+ irq = fwnode_irq_get(dev_fwnode(dev), 0);
+
+ return scd30_probe(dev, irq, KBUILD_MODNAME, priv, scd30_serdev_command);
+}
+
+static const struct of_device_id scd30_serdev_of_match[] = {
+ { .compatible = "sensirion,scd30" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, scd30_serdev_of_match);
+
+static struct serdev_device_driver scd30_serdev_driver = {
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = scd30_serdev_of_match,
+ .pm = &scd30_pm_ops,
+ },
+ .probe = scd30_serdev_probe,
+};
+module_serdev_device_driver(scd30_serdev_driver);
+
+MODULE_AUTHOR("Tomasz Duszynski <tomasz.duszynski@octakon.com>");
+MODULE_DESCRIPTION("Sensirion SCD30 carbon dioxide sensor serial driver");
+MODULE_LICENSE("GPL v2");
--
2.27.0
^ permalink raw reply related
* [PATCH v4 0/4] Add support for SCD30 sensor
From: Tomasz Duszynski @ 2020-06-03 8:44 UTC (permalink / raw)
To: linux-iio
Cc: linux-kernel, devicetree, robh+dt, jic23, andy.shevchenko, pmeerw,
Tomasz Duszynski
Following series adds support for Sensirion SCD30 sensor module capable of
measuring carbon dioxide, temperature and relative humidity. CO2 measurements
base on NDIR principle while temperature and relative humidity are measured by
the on board SHT31. As for sensor communication, both I2C and serial interfaces
are supported.
v4:
* improve formatting
* improve error handling readability
* fix message validity check on serial write
v3:
* simplify code by scaling temperature & humidity in _read_meas()
* update realbits in scan types
* s/adjecent/adjacent
* drop IIO_CHAN_INFO_RAW from _write_raw_get_fmt because there's no raw
output channel
* rework locking in _read_raw
* fix endianess problem on BE machine
* align timestamp properly before pushing to buffers
* explain why interrupt gets disabled after registration
* add trigger validation
* drop SCALE for temperature and humidity channel as they are processed
* register action which stops measuring after starting measurements
* spit generic calibration attr into two doing specific things
* add comment explaining why priv in struct scd30_state is for
* rename node in binding example to co2-sensor
v2:
* move asm/byteorder.h towards the bottom of include list
* make channel address names in enum more specific
* add postfixes to defines and extra comments
* drop unneeded i2c include from scd30 header
* break generic command sending function into specialized options
* expose automatic calibration and forced calibration via the same attr
* use SAMP_FREQ to set frequency instead of meas_interval attr
* use CALISCALE to set pressure compensation instead of pressure_comp attr
* use CALIBBIAS to set temperature offset instead of temp_offset attr
* fix order in MAINTAINERS
* drop attribute allowing one to reset sensor
* as we have dt probing drop board file based probing (i2c_device_id)
* merge patches touching related files
* use fwnode API to retrieve interrupt from dt
* fix interrupt-parent spelling
* change binding license
* drop supply from required property
Tomasz Duszynski (4):
iio: chemical: scd30: add core driver
iio: chemical: scd30: add I2C interface driver
iio: chemical: scd30: add serial interface driver
dt-bindings: iio: scd30: add device binding file
Documentation/ABI/testing/sysfs-bus-iio-scd30 | 34 +
.../iio/chemical/sensirion,scd30.yaml | 68 ++
MAINTAINERS | 9 +
drivers/iio/chemical/Kconfig | 33 +
drivers/iio/chemical/Makefile | 3 +
drivers/iio/chemical/scd30.h | 78 ++
drivers/iio/chemical/scd30_core.c | 761 ++++++++++++++++++
drivers/iio/chemical/scd30_i2c.c | 139 ++++
drivers/iio/chemical/scd30_serial.c | 263 ++++++
9 files changed, 1388 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd30.yaml
create mode 100644 drivers/iio/chemical/scd30.h
create mode 100644 drivers/iio/chemical/scd30_core.c
create mode 100644 drivers/iio/chemical/scd30_i2c.c
create mode 100644 drivers/iio/chemical/scd30_serial.c
--
2.27.0
^ permalink raw reply
* [PATCH v3 0/6] PCI: uniphier: Add features for UniPhier PCIe host controller
From: Kunihiko Hayashi @ 2020-06-03 8:54 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
Rob Herring, Masahiro Yamada
Cc: linux-pci, devicetree, linux-arm-kernel, linux-kernel,
Masami Hiramatsu, Jassi Brar, Kunihiko Hayashi
This series adds some features for UniPhier PCIe host controller.
- Add support for PME and AER invoked by MSI interrupt
- Add iATU register view support for PCIe version >= 4.80
- Add an error message when failing to get phy driver
This adds a new function called by MSI handler in DesignWare PCIe framework,
that invokes PME and AER funcions to detect the factor from SoC-dependent
registers.
Changes since v2:
- Avoid printing phy error message in case of EPROBE_DEFER
- Fix iATU register mapping method
- dt-bindings: Add Acked-by: line
- Fix typos in commit messages
- Use devm_platform_ioremap_resource_byname()
Changes since v1:
- Add check if struct resource is NULL
- Fix warning in the type of dev_err() argument
Kunihiko Hayashi (6):
PCI: dwc: Add msi_host_isr() callback
PCI: uniphier: Add misc interrupt handler to invoke PME and AER
dt-bindings: PCI: uniphier: Add iATU register description
PCI: uniphier: Add iATU register support
PCI: uniphier: Add error message when failed to get phy
PCI: uniphier: Use devm_platform_ioremap_resource_byname()
.../devicetree/bindings/pci/uniphier-pcie.txt | 1 +
drivers/pci/controller/dwc/pcie-designware-host.c | 8 +--
drivers/pci/controller/dwc/pcie-designware.h | 1 +
drivers/pci/controller/dwc/pcie-uniphier.c | 69 +++++++++++++++++-----
4 files changed, 60 insertions(+), 19 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v3 2/6] PCI: uniphier: Add misc interrupt handler to invoke PME and AER
From: Kunihiko Hayashi @ 2020-06-03 8:54 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
Rob Herring, Masahiro Yamada
Cc: linux-pci, devicetree, linux-arm-kernel, linux-kernel,
Masami Hiramatsu, Jassi Brar, Kunihiko Hayashi, Marc Zyngier
In-Reply-To: <1591174481-13975-1-git-send-email-hayashi.kunihiko@socionext.com>
The misc interrupts consisting of PME, AER, and Link event, is handled
by INTx handler, however, these interrupts should be also handled by
MSI handler.
This adds the function uniphier_pcie_misc_isr() that handles misc
intterupts, which is called from both INTx and MSI handlers.
This function detects PME and AER interrupts with the status register,
and invoke PME and AER drivers related to INTx or MSI.
And this sets the mask for misc interrupts from INTx if MSI is enabled
and sets the mask for misc interrupts from MSI if MSI is disabled.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
drivers/pci/controller/dwc/pcie-uniphier.c | 53 +++++++++++++++++++++++-------
1 file changed, 42 insertions(+), 11 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
index a5401a0..a8dda39 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier.c
@@ -44,7 +44,9 @@
#define PCL_SYS_AUX_PWR_DET BIT(8)
#define PCL_RCV_INT 0x8108
+#define PCL_RCV_INT_ALL_INT_MASK GENMASK(28, 25)
#define PCL_RCV_INT_ALL_ENABLE GENMASK(20, 17)
+#define PCL_RCV_INT_ALL_MSI_MASK GENMASK(12, 9)
#define PCL_CFG_BW_MGT_STATUS BIT(4)
#define PCL_CFG_LINK_AUTO_BW_STATUS BIT(3)
#define PCL_CFG_AER_RC_ERR_MSI_STATUS BIT(2)
@@ -167,7 +169,15 @@ static void uniphier_pcie_stop_link(struct dw_pcie *pci)
static void uniphier_pcie_irq_enable(struct uniphier_pcie_priv *priv)
{
- writel(PCL_RCV_INT_ALL_ENABLE, priv->base + PCL_RCV_INT);
+ u32 val;
+
+ val = PCL_RCV_INT_ALL_ENABLE;
+ if (pci_msi_enabled())
+ val |= PCL_RCV_INT_ALL_INT_MASK;
+ else
+ val |= PCL_RCV_INT_ALL_MSI_MASK;
+
+ writel(val, priv->base + PCL_RCV_INT);
writel(PCL_RCV_INTX_ALL_ENABLE, priv->base + PCL_RCV_INTX);
}
@@ -231,28 +241,48 @@ static const struct irq_domain_ops uniphier_intx_domain_ops = {
.map = uniphier_pcie_intx_map,
};
-static void uniphier_pcie_irq_handler(struct irq_desc *desc)
+static void uniphier_pcie_misc_isr(struct pcie_port *pp)
{
- struct pcie_port *pp = irq_desc_get_handler_data(desc);
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
- struct irq_chip *chip = irq_desc_get_chip(desc);
- unsigned long reg;
- u32 val, bit, virq;
+ u32 val, virq;
- /* INT for debug */
val = readl(priv->base + PCL_RCV_INT);
if (val & PCL_CFG_BW_MGT_STATUS)
dev_dbg(pci->dev, "Link Bandwidth Management Event\n");
+
if (val & PCL_CFG_LINK_AUTO_BW_STATUS)
dev_dbg(pci->dev, "Link Autonomous Bandwidth Event\n");
- if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS)
- dev_dbg(pci->dev, "Root Error\n");
- if (val & PCL_CFG_PME_MSI_STATUS)
- dev_dbg(pci->dev, "PME Interrupt\n");
+
+ if (pci_msi_enabled()) {
+ if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS) {
+ dev_dbg(pci->dev, "Root Error Status\n");
+ virq = irq_linear_revmap(pp->irq_domain, 0);
+ generic_handle_irq(virq);
+ }
+
+ if (val & PCL_CFG_PME_MSI_STATUS) {
+ dev_dbg(pci->dev, "PME Interrupt\n");
+ virq = irq_linear_revmap(pp->irq_domain, 0);
+ generic_handle_irq(virq);
+ }
+ }
writel(val, priv->base + PCL_RCV_INT);
+}
+
+static void uniphier_pcie_irq_handler(struct irq_desc *desc)
+{
+ struct pcie_port *pp = irq_desc_get_handler_data(desc);
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ unsigned long reg;
+ u32 val, bit, virq;
+
+ /* misc interrupt */
+ uniphier_pcie_misc_isr(pp);
/* INTx */
chained_irq_enter(chip, desc);
@@ -330,6 +360,7 @@ static int uniphier_pcie_host_init(struct pcie_port *pp)
static const struct dw_pcie_host_ops uniphier_pcie_host_ops = {
.host_init = uniphier_pcie_host_init,
+ .msi_host_isr = uniphier_pcie_misc_isr,
};
static int uniphier_add_pcie_port(struct uniphier_pcie_priv *priv,
--
2.7.4
^ permalink raw reply related
* [PATCH v3 1/6] PCI: dwc: Add msi_host_isr() callback
From: Kunihiko Hayashi @ 2020-06-03 8:54 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
Rob Herring, Masahiro Yamada
Cc: linux-pci, devicetree, linux-arm-kernel, linux-kernel,
Masami Hiramatsu, Jassi Brar, Kunihiko Hayashi, Marc Zyngier
In-Reply-To: <1591174481-13975-1-git-send-email-hayashi.kunihiko@socionext.com>
This adds msi_host_isr() callback function support to describe
SoC-dependent service triggered by MSI.
For example, when AER interrupt is triggered by MSI, the callback function
reads SoC-dependent registers and detects that the interrupt is from AER,
and invoke AER interrupts related to MSI.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
drivers/pci/controller/dwc/pcie-designware-host.c | 8 ++++----
drivers/pci/controller/dwc/pcie-designware.h | 1 +
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 0a4a5aa..9b628a2 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -112,13 +112,13 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
static void dw_chained_msi_isr(struct irq_desc *desc)
{
struct irq_chip *chip = irq_desc_get_chip(desc);
- struct pcie_port *pp;
+ struct pcie_port *pp = irq_desc_get_handler_data(desc);
- chained_irq_enter(chip, desc);
+ if (pp->ops->msi_host_isr)
+ pp->ops->msi_host_isr(pp);
- pp = irq_desc_get_handler_data(desc);
+ chained_irq_enter(chip, desc);
dw_handle_msi_irq(pp);
-
chained_irq_exit(chip, desc);
}
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 656e00f..e741967 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -170,6 +170,7 @@ struct dw_pcie_host_ops {
void (*scan_bus)(struct pcie_port *pp);
void (*set_num_vectors)(struct pcie_port *pp);
int (*msi_host_init)(struct pcie_port *pp);
+ void (*msi_host_isr)(struct pcie_port *pp);
};
struct pcie_port {
--
2.7.4
^ permalink raw reply related
* [PATCH v3 6/6] PCI: uniphier: Use devm_platform_ioremap_resource_byname()
From: Kunihiko Hayashi @ 2020-06-03 8:54 UTC (permalink / raw)
To: Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han, Gustavo Pimentel,
Rob Herring, Masahiro Yamada
Cc: linux-pci, devicetree, linux-arm-kernel, linux-kernel,
Masami Hiramatsu, Jassi Brar, Kunihiko Hayashi
In-Reply-To: <1591174481-13975-1-git-send-email-hayashi.kunihiko@socionext.com>
Use devm_platform_ioremap_resource_byname() to simplify the code a bit.
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
drivers/pci/controller/dwc/pcie-uniphier.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
index 3b51561..ce47622 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier.c
@@ -452,8 +452,7 @@ static int uniphier_pcie_probe(struct platform_device *pdev)
if (IS_ERR(priv->pci.atu_base))
priv->pci.atu_base = NULL;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "link");
- priv->base = devm_ioremap_resource(dev, res);
+ priv->base = devm_platform_ioremap_resource_byname(pdev, "link");
if (IS_ERR(priv->base))
return PTR_ERR(priv->base);
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox