* Re: [PATCH] of: overlay: validate offset from property fixups
From: Frank Rowand @ 2018-05-17 16:58 UTC (permalink / raw)
To: Rob Herring, pantelis.antoniou, Pantelis Antoniou
Cc: Dan Carpenter, devicetree, linux-kernel
In-Reply-To: <21ae05b7-e9d4-d518-c341-861c74f32922@gmail.com>
On 05/17/18 09:56, Frank Rowand wrote:
> Hi Rob,
>
> On 05/16/18 21:19, frowand.list@gmail.com wrote:
>> From: Frank Rowand <frank.rowand@sony.com>
>>
>> The smatch static checker marks the data in offset as untrusted,
>> leading it to warn:
>>
>> drivers/of/resolver.c:125 update_usages_of_a_phandle_reference()
>> error: buffer underflow 'prop->value' 's32min-s32max'
>>
>> Add check to verify that offset is within the property data.
>>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
>> ---
>> drivers/of/resolver.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
>> index 65d0b7adfcd4..7edfac6f1914 100644
>> --- a/drivers/of/resolver.c
>> +++ b/drivers/of/resolver.c
>> @@ -122,6 +122,11 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay,
>> goto err_fail;
>> }
>>
>> + if (offset < 0 || offset + sizeof(__be32) > prop->length) {
>> + err = -EINVAL;
>> + goto err_fail;
>> + }
>> +
>> *(__be32 *)(prop->value + offset) = cpu_to_be32(phandle);
>> }
>>
>>
>
> I should have mentioned that this results in a new compile warning
> for W=2 and W=3. The new if statement results in:
>
W=2 warning is:
> drivers/of/resolver.c:125:45: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
W=3 warning is:
drivers/of/resolver.c:125:3: warning: conversion to 'unsigned int' from 'int' may change the sign of the result [-Wsign-conversion]
>
> There are other pre-existing warnings in the same file for comparing
> an integer to prop->length. The correct solution is probably to
> change the type of the length field in struct property to be
> unsigned. I have added that task to my todo list.
>
> -Frank
>
^ permalink raw reply
* Re: [PATCH] of: overlay: validate offset from property fixups
From: Frank Rowand @ 2018-05-17 16:56 UTC (permalink / raw)
To: Rob Herring, pantelis.antoniou, Pantelis Antoniou
Cc: Dan Carpenter, devicetree, linux-kernel
In-Reply-To: <1526530791-18591-1-git-send-email-frowand.list@gmail.com>
Hi Rob,
On 05/16/18 21:19, frowand.list@gmail.com wrote:
> From: Frank Rowand <frank.rowand@sony.com>
>
> The smatch static checker marks the data in offset as untrusted,
> leading it to warn:
>
> drivers/of/resolver.c:125 update_usages_of_a_phandle_reference()
> error: buffer underflow 'prop->value' 's32min-s32max'
>
> Add check to verify that offset is within the property data.
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
> ---
> drivers/of/resolver.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
> index 65d0b7adfcd4..7edfac6f1914 100644
> --- a/drivers/of/resolver.c
> +++ b/drivers/of/resolver.c
> @@ -122,6 +122,11 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay,
> goto err_fail;
> }
>
> + if (offset < 0 || offset + sizeof(__be32) > prop->length) {
> + err = -EINVAL;
> + goto err_fail;
> + }
> +
> *(__be32 *)(prop->value + offset) = cpu_to_be32(phandle);
> }
>
>
I should have mentioned that this results in a new compile warning
for W=2 and W=3. The new if statement results in:
drivers/of/resolver.c:125:45: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
There are other pre-existing warnings in the same file for comparing
an integer to prop->length. The correct solution is probably to
change the type of the length field in struct property to be
unsigned. I have added that task to my todo list.
-Frank
^ permalink raw reply
* Re: [PATCH v7 03/14] clk: qcom: Add CPU clock driver for msm8996
From: kbuild test robot @ 2018-05-17 16:51 UTC (permalink / raw)
Cc: kbuild-all, mturquette, sboyd, robh, mark.rutland, viresh.kumar,
nm, lgirdwood, broonie, andy.gross, david.brown, catalin.marinas,
will.deacon, rjw, linux-clk, devicetree, linux-kernel, linux-pm,
linux-arm-msm, linux-soc, linux-arm-kernel, rnayak, ilialin,
amit.kucheria, nicolas.dechesne, celster, tfinkel
In-Reply-To: <1526375616-16904-4-git-send-email-ilialin@codeaurora.org>
Hi Ilia,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.17-rc5]
[cannot apply to clk/clk-next next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Ilia-Lin/CPU-scaling-support-for-msm8996/20180516-064721
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> drivers/clk/qcom/clk-cpu-8996.c:142:9: sparse: constant 2150400000 is so big it is long
>> drivers/clk/qcom/clk-cpu-8996.c:239:22: sparse: symbol 'clk_cpu_8996_mux_ops' was not declared. Should it be static?
drivers/clk/qcom/clk-cpu-8996.c:324:19: sparse: symbol 'clks' was not declared. Should it be static?
vim +142 drivers/clk/qcom/clk-cpu-8996.c
137
138 static const struct pll_vco alt_pll_vco_modes[] = {
139 VCO(3, 250000000, 500000000),
140 VCO(2, 500000000, 750000000),
141 VCO(1, 750000000, 1000000000),
> 142 VCO(0, 1000000000, 2150400000),
143 };
144
145 static const struct alpha_pll_config altpll_config = {
146 .l = 16,
147 .vco_val = 0x3 << 20,
148 .vco_mask = 0x3 << 20,
149 .config_ctl_val = 0x4001051b,
150 .post_div_mask = 0x3 << 8,
151 .post_div_val = 0x1,
152 .main_output_mask = BIT(0),
153 .early_output_mask = BIT(3),
154 };
155
156 static struct clk_alpha_pll perfcl_alt_pll = {
157 .offset = 0x80100,
158 .regs = alt_pll_regs,
159 .vco_table = alt_pll_vco_modes,
160 .num_vco = ARRAY_SIZE(alt_pll_vco_modes),
161 .flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,
162 .clkr.hw.init = &(struct clk_init_data) {
163 .name = "perfcl_alt_pll",
164 .parent_names = (const char *[]){ "xo" },
165 .num_parents = 1,
166 .ops = &clk_alpha_pll_hwfsm_ops,
167 },
168 };
169
170 static struct clk_alpha_pll pwrcl_alt_pll = {
171 .offset = 0x100,
172 .regs = alt_pll_regs,
173 .vco_table = alt_pll_vco_modes,
174 .num_vco = ARRAY_SIZE(alt_pll_vco_modes),
175 .flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,
176 .clkr.hw.init = &(struct clk_init_data) {
177 .name = "pwrcl_alt_pll",
178 .parent_names = (const char *[]){ "xo" },
179 .num_parents = 1,
180 .ops = &clk_alpha_pll_hwfsm_ops,
181 },
182 };
183
184 /* Mux'es */
185
186 struct clk_cpu_8996_mux {
187 u32 reg;
188 u8 shift;
189 u8 width;
190 struct clk_hw *pll;
191 struct clk_regmap clkr;
192 };
193
194 static inline
195 struct clk_cpu_8996_mux *to_clk_cpu_8996_mux_hw(struct clk_hw *hw)
196 {
197 return container_of(to_clk_regmap(hw), struct clk_cpu_8996_mux, clkr);
198 }
199
200 static u8 clk_cpu_8996_mux_get_parent(struct clk_hw *hw)
201 {
202 u32 val;
203 struct clk_regmap *clkr = to_clk_regmap(hw);
204 struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw);
205 u32 mask = (u32)GENMASK(cpuclk->width - 1, 0);
206
207 regmap_read(clkr->regmap, cpuclk->reg, &val);
208 val >>= (u32)(cpuclk->shift);
209
210 return (u8)(val & mask);
211 }
212
213 static int clk_cpu_8996_mux_set_parent(struct clk_hw *hw, u8 index)
214 {
215 u32 val;
216 struct clk_regmap *clkr = to_clk_regmap(hw);
217 struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw);
218 unsigned int mask = GENMASK(cpuclk->width + cpuclk->shift - 1,
219 cpuclk->shift);
220
221 val = (u32)index;
222 val <<= (u32)(cpuclk->shift);
223
224 return regmap_update_bits(clkr->regmap, cpuclk->reg, mask, val);
225 }
226
227 static int
228 clk_cpu_8996_mux_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
229 {
230 struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw);
231 struct clk_hw *parent = cpuclk->pll;
232
233 req->best_parent_rate = clk_hw_round_rate(parent, req->rate);
234 req->best_parent_hw = parent;
235
236 return 0;
237 }
238
> 239 const struct clk_ops clk_cpu_8996_mux_ops = {
240 .set_parent = clk_cpu_8996_mux_set_parent,
241 .get_parent = clk_cpu_8996_mux_get_parent,
242 .determine_rate = clk_cpu_8996_mux_determine_rate,
243 };
244
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [PATCH v6] gpio: dwapb: Add support for 1 interrupt per port A GPIO
From: Hoan Tran @ 2018-05-17 16:46 UTC (permalink / raw)
To: Lee Jones
Cc: Phil Edworthy, Andy Shevchenko, Hoan Tran, Linus Walleij,
Mark Rutland, Rob Herring, Michel Pollet,
linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20180517062500.GJ5130@dell>
Hi Lee,
On 5/16/18, 11:25 PM, "Lee Jones" <lee.jones@linaro.org> wrote:
On Wed, 16 May 2018, Hoan Tran wrote:
> Hi Phil,
>
> On 5/11/18, 1:31 AM, "Phil Edworthy" <phil.edworthy@renesas.com> wrote:
>
> The DesignWare GPIO IP can be configured for either 1 interrupt or 1
> per GPIO in port A, but the driver currently only supports 1 interrupt.
> See the DesignWare DW_apb_gpio Databook description of the
> 'GPIO_INTR_IO' parameter.
>
> This change allows the driver to work with up to 32 interrupts, it will
> get as many interrupts as specified in the DT 'interrupts' property.
> It doesn't do anything clever with the different interrupts, it just calls
> the same handler used for single interrupt hardware.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
> One point to mention is that I have made it possible for users to have
> unconnected interrupts by specifying holes in the list of interrupts. This is
> done by supporting the interrupts-extended DT prop.
> However, I have no use for this and had to hack some test case for this.
> Perhaps the driver should support 1 interrupt or all GPIOa as interrupts?
>
> v6:
> - Treat DT and ACPI the same as much as possible. Note that we can't use
> platform_get_irq() to get the DT interrupts as they are in the port
> sub-node and hence do not have an associated platform device.
> v5:
> - Rolled ACPI companion code provided by Hoan Tran into this patch.
> v4:
> - Use of_irq_get() instead of of_irq_parse_one()+irq_create_of_mapping()
> v3:
> - Rolled mfd: intel_quark_i2c_gpio fix into this patch to avoid bisect problems
> v2:
> - Replaced interrupt-mask DT prop with support for the interrupts-extended
> prop. This means replacing the call to irq_of_parse_and_map() with calls
> to of_irq_parse_one() and irq_create_of_mapping().
> ---
> .../devicetree/bindings/gpio/snps-dwapb-gpio.txt | 9 +++-
> drivers/gpio/gpio-dwapb.c | 49 +++++++++++++++-------
> drivers/mfd/intel_quark_i2c_gpio.c | 3 +-
> include/linux/platform_data/gpio-dwapb.h | 3 +-
> 4 files changed, 45 insertions(+), 19 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
> index 4a75da7..3c1118b 100644
> --- a/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
> +++ b/Documentation/devicetree/bindings/gpio/snps-dwapb-gpio.txt
> @@ -26,8 +26,13 @@ controller.
> the second encodes the triger flags encoded as described in
> Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> - interrupt-parent : The parent interrupt controller.
> -- interrupts : The interrupt to the parent controller raised when GPIOs
> - generate the interrupts.
> +- interrupts : The interrupts to the parent controller raised when GPIOs
> + generate the interrupts. If the controller provides one combined interrupt
> + for all GPIOs, specify a single interrupt. If the controller provides one
> + interrupt for each GPIO, provide a list of interrupts that correspond to each
> + of the GPIO pins. When specifying multiple interrupts, if any are unconnected,
> + use the interrupts-extended property to specify the interrupts and set the
> + interrupt controller handle for unused interrupts to 0.
> - snps,nr-gpios : The number of pins in the port, a single cell.
> - resets : Reset line for the controller.
>
> Acked-by: Hoan Tran <hoan.tran@amperecomputing.com>
Well that's new. I've never seen a mailer reply like that before.
Which mailer are you using? Might be worth sorting that out.
Yes, it looks like my new email from outlook does not have the correct format. With my old one, it works correctly.
Thanks
Hoan
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH RFC 3/6] hwmon: Add support for RPi voltage sensor
From: Guenter Roeck @ 2018-05-17 16:43 UTC (permalink / raw)
To: Stefan Wahren
Cc: Mark Rutland, devicetree, Jean Delvare, Florian Fainelli,
Scott Branden, Ray Jui, Phil Elwell, Rob Herring, Eric Anholt,
Noralf Trønnes, bcm-kernel-feedback-list, linux-rpi-kernel,
linux-hwmon, linux-arm-kernel
In-Reply-To: <1560428919.21731.1526500741266@email.1und1.de>
On Wed, May 16, 2018 at 09:59:01PM +0200, Stefan Wahren wrote:
> Hi Guenter,
>
> > Guenter Roeck <linux@roeck-us.net> hat am 16. Mai 2018 um 20:21 geschrieben:
> >
> >
> > On Wed, May 16, 2018 at 03:37:04PM +0200, Stefan Wahren wrote:
> > > Currently there is no easy way to detect under-voltage conditions on a remote
> > > Raspberry Pi. This hwmon driver retrieves the state of the under-voltage sensor
> > > via mailbox interface. The handling based on Noralf's modifications to the
> > > downstream firmware driver. In case of an under-voltage condition only an entry
> > > is written to the kernel log.
> > >
> >
> > My major concern is how this is displayed with the 'sensors' command.
> > Can you test and report ?
>
> I get the following output:
> rpi_volt-isa-0000
> Adapter: ISA adapter
> in0: N/A
>
Ok, that works.
> >
> > Of course, it would be much better if the firmware would also report
> > the actual voltage, but I guess we can't have everything.
>
> I think this isn't possible because the hardware only provide a binary value (GPIO).
>
> >
> > More comments inline.
> >
> > Thanks,
> > Guenter
> >
> > > CC: "Noralf Trønnes" <noralf@tronnes.org>
> > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > > ---
> > > drivers/hwmon/Kconfig | 10 ++
> > > drivers/hwmon/Makefile | 1 +
> > > drivers/hwmon/raspberrypi-hwmon.c | 207 ++++++++++++++++++++++++++++++++++++++
> >
> > Please also provide Documentation/hwmon/raspberrypi-hwmon.
> >
> > > 3 files changed, 218 insertions(+)
> > > create mode 100644 drivers/hwmon/raspberrypi-hwmon.c
> > >
> > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > > index 768aed5..7f935cf 100644
> > > --- a/drivers/hwmon/Kconfig
> > > +++ b/drivers/hwmon/Kconfig
> > > @@ -1298,6 +1298,16 @@ config SENSORS_PWM_FAN
> > > This driver can also be built as a module. If so, the module
> > > will be called pwm-fan.
> > >
> > > +config SENSORS_RASPBERRYPI_HWMON
> > > + tristate "Raspberry Pi voltage monitor"
> > > + depends on (ARCH_BCM2835 && RASPBERRYPI_FIRMWARE) || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE)
> > > + help
> > > + If you say yes here you get support for voltage sensor on the
> > > + Raspberry Pi.
> > > +
> > > + This driver can also be built as a module. If so, the module
> > > + will be called raspberrypi-hwmon.
> > > +
> > > config SENSORS_SHT15
> > > tristate "Sensiron humidity and temperature sensors. SHT15 and compat."
> > > depends on GPIOLIB || COMPILE_TEST
> > > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > > index e7d52a3..a929770 100644
> > > --- a/drivers/hwmon/Makefile
> > > +++ b/drivers/hwmon/Makefile
> > > @@ -141,6 +141,7 @@ obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
> > > obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
> > > obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o
> > > obj-$(CONFIG_SENSORS_PWM_FAN) += pwm-fan.o
> > > +obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON) += raspberrypi-hwmon.o
> > > obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o
> > > obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
> > > obj-$(CONFIG_SENSORS_SCH5627) += sch5627.o
> > > diff --git a/drivers/hwmon/raspberrypi-hwmon.c b/drivers/hwmon/raspberrypi-hwmon.c
> > > new file mode 100644
> > > index 0000000..2003f6c
> > > --- /dev/null
> > > +++ b/drivers/hwmon/raspberrypi-hwmon.c
> > > @@ -0,0 +1,207 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Raspberry Pi voltage sensor driver
> > > + *
> > > + * Based on firmware/raspberrypi.c by Noralf Trønnes
> > > + *
> > > + * Copyright (C) 2018 Stefan Wahren <stefan.wahren@i2se.com>
> > > + */
> > > +#include <linux/device.h>
> > > +#include <linux/err.h>
> > > +#include <linux/hwmon.h>
> > > +#include <linux/hwmon-sysfs.h>
> >
> > Unnecessary include
> >
> > > +#include <linux/module.h>
> > > +#include <linux/of_device.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/workqueue.h>
> > > +#include <soc/bcm2835/raspberrypi-firmware.h>
> > > +
> > > +#define UNDERVOLTAGE_STICKY_BIT BIT(16)
> > > +
> > > +struct rpi_hwmon_data {
> > > + struct device *hwmon_dev;
> > > + struct rpi_firmware *fw;
> > > + u32 last_throttled;
> > > + struct delayed_work get_values_poll_work;
> > > +};
> > > +
> > > +static void rpi_firmware_get_throttled(struct rpi_hwmon_data *data)
> > > +{
> > > + u32 new_uv, old_uv, value;
> > > + int ret;
> > > +
> > > + /* Clear sticky bits */
> >
> > Please make more explicit that this is a request/command sent to the firmware.
> >
> > > + value = 0xffff;
> > > +
> > > + ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_THROTTLED,
> > > + &value, sizeof(value));
> > > + if (ret) {
> > > + dev_err_once(data->hwmon_dev, "%s: Failed to get throttled (%d)\n",
> > > + __func__, ret);
> >
> > The function name seems unnecessary.
> >
> > > + return;
> > > + }
> > > +
> > > + new_uv = value & UNDERVOLTAGE_STICKY_BIT;
> > > + old_uv = data->last_throttled & UNDERVOLTAGE_STICKY_BIT;
> > > + data->last_throttled = value;
> > > +
> > > + if (new_uv == old_uv)
> > > + return;
> > > +
> > > + if (new_uv)
> > > + dev_crit(data->hwmon_dev, "Under-voltage detected! (0x%08x)\n",
> > > + value);
> > > + else
> > > + dev_info(data->hwmon_dev, "Voltage normalised (0x%08x)\n",
> > > + value);
> >
> > What value do those hex values provide to the user ?
>
> The actual definition of the bits can be found in the commit log of patch #1. But this isn't very helpful for an end user.
>
I would suggest to drop the value from the log.
> >
> > > +
> > > + sysfs_notify(&data->hwmon_dev->kobj, NULL, "in0_lcrit_alarm");
> > > +}
> > > +
> > > +static void get_values_poll(struct work_struct *work)
> > > +{
> > > + struct rpi_hwmon_data *data;
> > > +
> > > + data = container_of(work, struct rpi_hwmon_data,
> > > + get_values_poll_work.work);
> > > +
> > > + rpi_firmware_get_throttled(data);
> > > +
> > > + /*
> > > + * We can't run faster than the sticky shift (100ms) since we get
> > > + * flipping in the sticky bits that are cleared.
> > > + */
> > > + schedule_delayed_work(&data->get_values_poll_work, 2 * HZ);
> > > +}
> > > +
> > > +static int rpi_read(struct device *dev, enum hwmon_sensor_types type,
> > > + u32 attr, int channel, long *val)
> > > +{
> > > + struct rpi_hwmon_data *data = dev_get_drvdata(dev);
> > > +
> > > + if (type != hwmon_in)
> > > + return -EOPNOTSUPP;
> > > +
> > > + if (attr != hwmon_in_lcrit_alarm)
> > > + return -EOPNOTSUPP;
> > > +
> > > + if (channel)
> > > + return -EOPNOTSUPP;
> > > +
> > There is only one channel, one attribute, and one type supported.
> > As such, the checks are unnecessary.
> >
> > > + *val = !!(data->last_throttled & UNDERVOLTAGE_STICKY_BIT);
> > > + return 0;
> > > +}
> > > +
> > > +static umode_t rpi_is_visible(const void *_data, enum hwmon_sensor_types type,
> > > + u32 attr, int channel)
> > > +{
> > > + if (type != hwmon_in)
> > > + return 0;
> > > +
> > > + if (attr != hwmon_in_lcrit_alarm)
> > > + return 0;
> > > +
> > > + if (channel)
> > > + return 0;
> >
> > Same as above. In the list below, there is not a single conditional attribute.
> > Given that, the checks should be unnecessary, and it should be sufficient to
> > just return 0444.
> >
> > > +
> > > + return 0444;
> > > +}
> > > +
> > > +static const u32 rpi_in_config[] = {
> > > + HWMON_I_LCRIT_ALARM,
> > > + 0
> > > +};
> > > +
> > > +static const struct hwmon_channel_info rpi_in = {
> > > + .type = hwmon_in,
> > > + .config = rpi_in_config,
> > > +};
> > > +
> > > +static const struct hwmon_channel_info *rpi_info[] = {
> > > + &rpi_in,
> > > + NULL
> > > +};
> > > +
> > > +static const struct hwmon_ops rpi_hwmon_ops = {
> > > + .is_visible = rpi_is_visible,
> > > + .read = rpi_read,
> > > +};
> > > +
> > > +static const struct hwmon_chip_info rpi_chip_info = {
> > > + .ops = &rpi_hwmon_ops,
> > > + .info = rpi_info,
> > > +};
> > > +
> > > +static int rpi_hwmon_probe(struct platform_device *pdev)
> > > +{
> > > + struct device *dev = &pdev->dev;
> > > + struct device_node *fw_node;
> > > + struct rpi_hwmon_data *data;
> > > + int ret;
> > > +
> > > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> > > + if (!data)
> > > + return -ENOMEM;
> > > +
> > > + fw_node = of_get_parent(dev->of_node);
> > > + if (!fw_node) {
> > > + dev_err(dev, "Missing firmware node\n");
> > > + return -ENOENT;
> > > + }
> > > +
> > > + data->fw = rpi_firmware_get(fw_node);
> > > + of_node_put(fw_node);
> > > + if (!data->fw)
> > > + return -EPROBE_DEFER;
> > > +
> > > + ret = rpi_firmware_property(data->fw, RPI_FIRMWARE_GET_THROTTLED,
> > > + &data->last_throttled,
> > > + sizeof(data->last_throttled));
> > > + if (ret) {
> > > + dev_info(dev, "Firmware doesn't support GET_THROTTLED\n");
> >
> > If this is an error -> dev_err().
>
> I wasn't sure. If the firmware is too old, we cannot provide this feature and it's a waste of resources to load this driver. On the other side i don't want to confuse people this is something bad.
>
Then I would suggest to return -ENODEV.
Guenter
> Stefan
>
> >
> > > + return -EOPNOTSUPP;
> >
> > or return -ENODEV.
> >
> > > + }
> > > +
> > > + data->hwmon_dev = devm_hwmon_device_register_with_info(dev, "rpi_volt",
> > > + data,
> > > + &rpi_chip_info,
> > > + NULL);
> > > +
> > > + INIT_DELAYED_WORK(&data->get_values_poll_work, get_values_poll);
> > > + platform_set_drvdata(pdev, data);
> > > +
> > > + if (!PTR_ERR_OR_ZERO(data->hwmon_dev))
> > > + schedule_delayed_work(&data->get_values_poll_work, 2 * HZ);
> > > +
> > > + return PTR_ERR_OR_ZERO(data->hwmon_dev);
> > > +}
> > > +
> > > +static int rpi_hwmon_remove(struct platform_device *pdev)
> > > +{
> > > + struct rpi_hwmon_data *data = platform_get_drvdata(pdev);
> > > +
> > > + cancel_delayed_work_sync(&data->get_values_poll_work);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct of_device_id rpi_hwmon_of_match[] = {
> > > + { .compatible = "raspberrypi,bcm2835-hwmon", },
> > > + { /* sentinel */},
> > > +};
> > > +MODULE_DEVICE_TABLE(of, rpi_hwmon_of_match);
> > > +
> > > +static struct platform_driver rpi_hwmon_driver = {
> > > + .probe = rpi_hwmon_probe,
> > > + .remove = rpi_hwmon_remove,
> > > + .driver = {
> > > + .name = "raspberrypi-hwmon",
> > > + .of_match_table = rpi_hwmon_of_match,
> > > + },
> > > +};
> > > +module_platform_driver(rpi_hwmon_driver);
> > > +
> > > +MODULE_AUTHOR("Stefan Wahren <stefan.wahren@i2se.com>");
> > > +MODULE_DESCRIPTION("Raspberry Pi voltage sensor driver");
> > > +MODULE_LICENSE("GPL v2");
> > > --
> > > 2.7.4
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Applied "regulator: of: add property for allowed modes specification" to the regulator tree
From: Mark Brown @ 2018-05-17 16:41 UTC (permalink / raw)
To: David Collins; +Cc: Mark Brown
In-Reply-To: <f3aa166ddd11c8f9813602be68aa5b10e5c90979.1526088289.git.collinsd@codeaurora.org>
The patch
regulator: of: add property for allowed modes specification
has been applied to the regulator tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From d73e2842a3e236a1f58ba498e4d6ae58e18f660a Mon Sep 17 00:00:00 2001
From: David Collins <collinsd@codeaurora.org>
Date: Fri, 11 May 2018 18:46:46 -0700
Subject: [PATCH] regulator: of: add property for allowed modes specification
Add a common device tree property for regulator nodes to support
the specification of allowed operating modes.
Signed-off-by: David Collins <collinsd@codeaurora.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
Documentation/devicetree/bindings/regulator/regulator.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index 2babe15b618d..c627aa08f0da 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -59,6 +59,11 @@ Optional properties:
- regulator-initial-mode: initial operating mode. The set of possible operating
modes depends on the capabilities of every hardware so each device binding
documentation explains which values the regulator supports.
+- regulator-allowed-modes: list of operating modes that software is allowed to
+ configure for the regulator at run-time. Elements may be specified in any
+ order. The set of possible operating modes depends on the capabilities of
+ every hardware so each device binding document explains which values the
+ regulator supports.
- regulator-system-load: Load in uA present on regulator that is not captured by
any consumer request.
- regulator-pull-down: Enable pull down resistor when the regulator is disabled.
--
2.17.0
^ permalink raw reply related
* Applied "regulator: of: add support for allowed modes configuration" to the regulator tree
From: Mark Brown @ 2018-05-17 16:41 UTC (permalink / raw)
To: David Collins; +Cc: Mark Brown
In-Reply-To: <58bb4f965515a67a6cbbc726b0d7b092c22b79d7.1526088289.git.collinsd@codeaurora.org>
The patch
regulator: of: add support for allowed modes configuration
has been applied to the regulator tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 54557ad9737e28b06b632cce723ef2373fe6bf12 Mon Sep 17 00:00:00 2001
From: David Collins <collinsd@codeaurora.org>
Date: Fri, 11 May 2018 18:46:47 -0700
Subject: [PATCH] regulator: of: add support for allowed modes configuration
Add support for configuring the machine constraints
valid_modes_mask element based on a list of allowed modes
specified via a device tree property.
Signed-off-by: David Collins <collinsd@codeaurora.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/regulator/of_regulator.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 0d3f73eacb99..d61fed28fdb9 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -32,7 +32,7 @@ static void of_get_regulation_constraints(struct device_node *np,
struct regulator_state *suspend_state;
struct device_node *suspend_np;
unsigned int mode;
- int ret, i;
+ int ret, i, len;
u32 pval;
constraints->name = of_get_property(np, "regulator-name", NULL);
@@ -136,6 +136,33 @@ static void of_get_regulation_constraints(struct device_node *np,
}
}
+ len = of_property_count_elems_of_size(np, "regulator-allowed-modes",
+ sizeof(u32));
+ if (len > 0) {
+ if (desc && desc->of_map_mode) {
+ for (i = 0; i < len; i++) {
+ ret = of_property_read_u32_index(np,
+ "regulator-allowed-modes", i, &pval);
+ if (ret) {
+ pr_err("%s: couldn't read allowed modes index %d, ret=%d\n",
+ np->name, i, ret);
+ break;
+ }
+ mode = desc->of_map_mode(pval);
+ if (mode == REGULATOR_MODE_INVALID)
+ pr_err("%s: invalid regulator-allowed-modes element %u\n",
+ np->name, pval);
+ else
+ constraints->valid_modes_mask |= mode;
+ }
+ if (constraints->valid_modes_mask)
+ constraints->valid_ops_mask
+ |= REGULATOR_CHANGE_MODE;
+ } else {
+ pr_warn("%s: mode mapping not defined\n", np->name);
+ }
+ }
+
if (!of_property_read_u32(np, "regulator-system-load", &pval))
constraints->system_load = pval;
--
2.17.0
^ permalink raw reply related
* Applied "regulator: bindings: Add properties for coupled regulators" to the regulator tree
From: Mark Brown @ 2018-05-17 16:41 UTC (permalink / raw)
To: Maciej Purski; +Cc: Mark Brown, linux-kernel, devicetree
In-Reply-To: <1520598128-11768-3-git-send-email-m.purski@samsung.com>
The patch
regulator: bindings: Add properties for coupled regulators
has been applied to the regulator tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From f98618b34542706eddc3b66abc271f1c8d8c4a05 Mon Sep 17 00:00:00 2001
From: Maciej Purski <m.purski@samsung.com>
Date: Mon, 23 Apr 2018 16:33:38 +0200
Subject: [PATCH] regulator: bindings: Add properties for coupled regulators
Some regulators require keeping their voltage spread below defined
max_spread.
Add properties to provide information on regulators' coupling.
Signed-off-by: Maciej Purski <m.purski@samsung.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
Documentation/devicetree/bindings/regulator/regulator.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index c627aa08f0da..a7cd36877bfe 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -73,6 +73,11 @@ Optional properties:
0: Disable active discharge.
1: Enable active discharge.
Absence of this property will leave configuration to default.
+- regulator-coupled-with: Regulators with which the regulator
+ is coupled. The linkage is 2-way - all coupled regulators should be linked
+ with each other. A regulator should not be coupled with its supplier.
+- regulator-coupled-max-spread: Max spread between voltages of coupled regulators
+ in microvolts.
Deprecated properties:
- regulator-compatible: If a regulator chip contains multiple
--
2.17.0
^ permalink raw reply related
* Applied "regulator: core: Resolve coupled regulators" to the regulator tree
From: Mark Brown @ 2018-05-17 16:41 UTC (permalink / raw)
To: Maciej Purski; +Cc: Mark Brown, linux-kernel, devicetree
In-Reply-To: <1524494022-22260-5-git-send-email-m.purski@samsung.com>
The patch
regulator: core: Resolve coupled regulators
has been applied to the regulator tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From d3d64537c33956c94cfa2376b87d8917868a0f5e Mon Sep 17 00:00:00 2001
From: Maciej Purski <m.purski@samsung.com>
Date: Mon, 23 Apr 2018 16:33:40 +0200
Subject: [PATCH] regulator: core: Resolve coupled regulators
On Odroid XU3/4 and other Exynos5422 based boards there is a case, that
different devices on the board are supplied by different regulators
with non-fixed voltages. If one of these devices temporarily requires
higher voltage, there might occur a situation that the spread between
two devices' voltages is so high, that there is a risk of changing
'high' and 'low' states on the interconnection between devices powered
by those regulators.
Fill coupling descriptor with data obtained from DTS using previously
defined of_functions. Fail to register a regulator, if some data
inconsistency occurs. If some coupled regulators are not yet registered,
don't fail to register, but try to resolve them in late init call.
Signed-off-by: Maciej Purski <m.purski@samsung.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/regulator/core.c | 100 +++++++++++++++++++++++++++++++++++++++
1 file changed, 100 insertions(+)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 0ca941b53571..6ed568b96c0e 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -4129,6 +4129,96 @@ static int regulator_register_resolve_supply(struct device *dev, void *data)
return 0;
}
+static int regulator_fill_coupling_array(struct regulator_dev *rdev)
+{
+ struct coupling_desc *c_desc = &rdev->coupling_desc;
+ int n_coupled = c_desc->n_coupled;
+ struct regulator_dev *c_rdev;
+ int i;
+
+ for (i = 1; i < n_coupled; i++) {
+ /* already resolved */
+ if (c_desc->coupled_rdevs[i])
+ continue;
+
+ c_rdev = of_parse_coupled_regulator(rdev, i - 1);
+
+ if (c_rdev) {
+ c_desc->coupled_rdevs[i] = c_rdev;
+ c_desc->n_resolved++;
+ }
+ }
+
+ if (rdev->coupling_desc.n_resolved < n_coupled)
+ return -1;
+ else
+ return 0;
+}
+
+static int regulator_register_fill_coupling_array(struct device *dev,
+ void *data)
+{
+ struct regulator_dev *rdev = dev_to_rdev(dev);
+
+ if (!IS_ENABLED(CONFIG_OF))
+ return 0;
+
+ if (regulator_fill_coupling_array(rdev))
+ rdev_dbg(rdev, "unable to resolve coupling\n");
+
+ return 0;
+}
+
+static int regulator_resolve_coupling(struct regulator_dev *rdev)
+{
+ int n_phandles;
+
+ if (!IS_ENABLED(CONFIG_OF))
+ n_phandles = 0;
+ else
+ n_phandles = of_get_n_coupled(rdev);
+
+ if (n_phandles + 1 > MAX_COUPLED) {
+ rdev_err(rdev, "too many regulators coupled\n");
+ return -EPERM;
+ }
+
+ /*
+ * Every regulator should always have coupling descriptor filled with
+ * at least pointer to itself.
+ */
+ rdev->coupling_desc.coupled_rdevs[0] = rdev;
+ rdev->coupling_desc.n_coupled = n_phandles + 1;
+ rdev->coupling_desc.n_resolved++;
+
+ /* regulator isn't coupled */
+ if (n_phandles == 0)
+ return 0;
+
+ /* regulator, which can't change its voltage, can't be coupled */
+ if (!regulator_ops_is_valid(rdev, REGULATOR_CHANGE_VOLTAGE)) {
+ rdev_err(rdev, "voltage operation not allowed\n");
+ return -EPERM;
+ }
+
+ if (rdev->constraints->max_spread <= 0) {
+ rdev_err(rdev, "wrong max_spread value\n");
+ return -EPERM;
+ }
+
+ if (!of_check_coupling_data(rdev))
+ return -EPERM;
+
+ /*
+ * After everything has been checked, try to fill rdevs array
+ * with pointers to regulators parsed from device tree. If some
+ * regulators are not registered yet, retry in late init call
+ */
+ regulator_fill_coupling_array(rdev);
+
+ return 0;
+}
+
/**
* regulator_register - register regulator
* @regulator_desc: regulator to register
@@ -4262,6 +4352,13 @@ regulator_register(const struct regulator_desc *regulator_desc,
if (ret < 0)
goto wash;
+ mutex_lock(®ulator_list_mutex);
+ ret = regulator_resolve_coupling(rdev);
+ mutex_unlock(®ulator_list_mutex);
+
+ if (ret != 0)
+ goto wash;
+
/* add consumers devices */
if (init_data) {
mutex_lock(®ulator_list_mutex);
@@ -4756,6 +4853,9 @@ static int __init regulator_init_complete(void)
class_for_each_device(®ulator_class, NULL, NULL,
regulator_late_cleanup);
+ class_for_each_device(®ulator_class, NULL, NULL,
+ regulator_register_fill_coupling_array);
+
return 0;
}
late_initcall_sync(regulator_init_complete);
--
2.17.0
^ permalink raw reply related
* Applied "regulator: core: Add voltage balancing mechanism" to the regulator tree
From: Mark Brown @ 2018-05-17 16:41 UTC (permalink / raw)
To: Maciej Purski; +Cc: Mark Brown, linux-kernel, devicetree
In-Reply-To: <1516631412-17542-7-git-send-email-m.purski@samsung.com>
The patch
regulator: core: Add voltage balancing mechanism
has been applied to the regulator tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 696861761a58d8c93605b5663824929fb6540f16 Mon Sep 17 00:00:00 2001
From: Maciej Purski <m.purski@samsung.com>
Date: Mon, 23 Apr 2018 16:33:41 +0200
Subject: [PATCH] regulator: core: Add voltage balancing mechanism
On Odroid XU3/4 and other Exynos5422 based boards there is a case, that
different devices on the board are supplied by different regulators
with non-fixed voltages. If one of these devices temporarily requires
higher voltage, there might occur a situation that the spread between
two devices' voltages is so high, that there is a risk of changing
'high' and 'low' states on the interconnection between devices powered
by those regulators.
Introduce new function regulator_balance_voltage(), which
keeps max_spread constraint fulfilled between a group of coupled
regulators. It should be called if a regulator changes its
voltage or after disabling or enabling. Disabled regulators should
follow changes of the enabled ones, but their consumers' demands
shouldn't be taken into account while calculating voltage of other
coupled regulators.
Find voltages, which are closest to suiting all the consumers' demands,
while fulfilling max_spread constraint, keeping the following rules:
- if one regulator is about to rise its voltage, rise others
voltages in order to keep the max_spread
- if a regulator, which has caused rising other regulators, is
lowered, lower other regulators if possible
- if one regulator is about to lower its voltage, but it hasn't caused
rising other regulators, don't change its voltage if it breaks the
max_spread
Change regulators' voltages step by step, keeping max_spread constraint
fulfilled all the time. Function regulator_get_optimal_voltage()
should find the best possible change for the regulator, which doesn't
break max_spread constraint. In function regulator_balance_voltage()
optimize number of steps by finding highest voltage difference on
each iteration.
If a regulator, which is about to change its voltage, is not coupled,
method regulator_get_optimal_voltage() should simply return the lowest
voltage fulfilling consumers' demands.
Coupling should be checked only if the system is in PM_SUSPEND_ON state.
Signed-off-by: Maciej Purski <m.purski@samsung.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/regulator/core.c | 192 +++++++++++++++++++++++++++++++++++++++
1 file changed, 192 insertions(+)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 6ed568b96c0e..225eaca24921 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -105,6 +105,8 @@ static int _notifier_call_chain(struct regulator_dev *rdev,
unsigned long event, void *data);
static int _regulator_do_set_voltage(struct regulator_dev *rdev,
int min_uV, int max_uV);
+static int regulator_balance_voltage(struct regulator_dev *rdev,
+ suspend_state_t state);
static struct regulator *create_regulator(struct regulator_dev *rdev,
struct device *dev,
const char *supply_name);
@@ -3102,6 +3104,196 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
return ret;
}
+static int regulator_get_optimal_voltage(struct regulator_dev *rdev)
+{
+ struct coupling_desc *c_desc = &rdev->coupling_desc;
+ struct regulator_dev **c_rdevs = c_desc->coupled_rdevs;
+ int max_spread = rdev->constraints->max_spread;
+ int n_coupled = c_desc->n_coupled;
+ int desired_min_uV, desired_max_uV, min_current_uV = INT_MAX;
+ int max_current_uV = 0, highest_min_uV = 0, target_uV, possible_uV;
+ int i, ret;
+
+ /* If consumers don't provide any demands, set voltage to min_uV */
+ desired_min_uV = rdev->constraints->min_uV;
+ desired_max_uV = rdev->constraints->max_uV;
+ ret = regulator_check_consumers(rdev,
+ &desired_min_uV,
+ &desired_max_uV, PM_SUSPEND_ON);
+ if (ret < 0)
+ goto out;
+
+ /*
+ * If there are no coupled regulators, simply set the voltage demanded
+ * by consumers.
+ */
+ if (n_coupled == 1) {
+ ret = desired_min_uV;
+ goto out;
+ }
+
+ /* Find highest min desired voltage */
+ for (i = 0; i < n_coupled; i++) {
+ int tmp_min = 0;
+ int tmp_max = INT_MAX;
+
+ if (!_regulator_is_enabled(c_rdevs[i]))
+ continue;
+
+ ret = regulator_check_consumers(c_rdevs[i],
+ &tmp_min,
+ &tmp_max, PM_SUSPEND_ON);
+ if (ret < 0)
+ goto out;
+
+ if (tmp_min > highest_min_uV)
+ highest_min_uV = tmp_min;
+ }
+
+ /*
+ * Let target_uV be equal to the desired one if possible.
+ * If not, set it to minimum voltage, allowed by other coupled
+ * regulators.
+ */
+ target_uV = max(desired_min_uV, highest_min_uV - max_spread);
+
+ /*
+ * Find min and max voltages, which currently aren't
+ * violating max_spread
+ */
+ for (i = 0; i < n_coupled; i++) {
+ int tmp_act;
+
+ /*
+ * Don't check the regulator, which is about
+ * to change voltage
+ */
+ if (c_rdevs[i] == rdev)
+ continue;
+ if (!_regulator_is_enabled(c_rdevs[i]))
+ continue;
+
+ tmp_act = _regulator_get_voltage(c_rdevs[i]);
+ if (tmp_act < 0) {
+ ret = tmp_act;
+ goto out;
+ }
+
+ if (tmp_act < min_current_uV)
+ min_current_uV = tmp_act;
+
+ if (tmp_act > max_current_uV)
+ max_current_uV = tmp_act;
+ }
+
+ /* There aren't any other regulators enabled */
+ if (max_current_uV == 0) {
+ possible_uV = target_uV;
+ } else {
+ /*
+ * Correct target voltage, so as it currently isn't
+ * violating max_spread
+ */
+ possible_uV = max(target_uV, max_current_uV - max_spread);
+ possible_uV = min(possible_uV, min_current_uV + max_spread);
+ }
+
+ if (possible_uV > desired_max_uV) {
+ ret = -EINVAL;
+ goto out;
+ }
+ ret = possible_uV;
+
+out:
+ return ret;
+}
+
+static int regulator_balance_voltage(struct regulator_dev *rdev,
+ suspend_state_t state)
+{
+ struct regulator_dev **c_rdevs;
+ struct regulator_dev *best_rdev;
+ struct coupling_desc *c_desc = &rdev->coupling_desc;
+ int n_coupled;
+ int i, best_delta, best_uV, ret = 1;
+
+ c_rdevs = c_desc->coupled_rdevs;
+ n_coupled = c_desc->n_coupled;
+
+ /*
+ * if system is in a state other than PM_SUSPEND_ON, don't check
+ * other coupled regulators
+ */
+ if (state != PM_SUSPEND_ON)
+ n_coupled = 1;
+
+ /*
+ * Find the best possible voltage change on each loop. Leave the loop
+ * if there isn't any possible change.
+ */
+ while (1) {
+ best_delta = 0;
+ best_uV = 0;
+ best_rdev = NULL;
+
+ /*
+ * Find highest difference between optimal voltage
+ * and current voltage.
+ */
+ for (i = 0; i < n_coupled; i++) {
+ /*
+ * optimal_uV is the best voltage that can be set for
+ * i-th regulator at the moment without violating
+ * max_spread constraint in order to balance
+ * the coupled voltages.
+ */
+ int optimal_uV, current_uV;
+
+ optimal_uV = regulator_get_optimal_voltage(c_rdevs[i]);
+ if (optimal_uV < 0) {
+ ret = optimal_uV;
+ goto out;
+ }
+
+ current_uV = _regulator_get_voltage(c_rdevs[i]);
+ if (current_uV < 0) {
+ ret = optimal_uV;
+ goto out;
+ }
+
+ if (abs(best_delta) < abs(optimal_uV - current_uV)) {
+ best_delta = optimal_uV - current_uV;
+ best_rdev = c_rdevs[i];
+ best_uV = optimal_uV;
+ }
+ }
+
+ /* Nothing to change, return successfully */
+ if (!best_rdev) {
+ ret = 0;
+ goto out;
+ }
+
+ /*
+ * Lock just the supply regulators, as the regulator itself
+ * is already locked by regulator_lock_coupled().
+ */
+ if (best_rdev->supply)
+ regulator_lock_supply(best_rdev->supply->rdev);
+
+ ret = regulator_set_voltage_rdev(best_rdev, best_uV,
+ best_uV, state);
+ if (best_rdev->supply)
+ regulator_unlock_supply(best_rdev->supply->rdev);
+
+ if (ret < 0)
+ goto out;
+ }
+
+out:
+ return ret;
+}
+
/**
* regulator_set_voltage - set regulator output voltage
* @regulator: regulator source
--
2.17.0
^ permalink raw reply related
* Applied "ASoC: qdsp6: q6core: Add q6core driver" to the asoc tree
From: Mark Brown @ 2018-05-17 16:38 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: mark.rutland, devicetree, alsa-devel, bgoswami, rohkumar,
linux-arm-msm, plai, spatakok, linux-kernel, tiwai, robh+dt,
lgirdwood, david.brown, broonie, gregkh, andy.gross,
linux-arm-kernel
In-Reply-To: <20180509125635.5653-9-srinivas.kandagatla@linaro.org>
The patch
ASoC: qdsp6: q6core: Add q6core driver
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From b73e93e183089176beed8a3f80e5f618a52e02fc Mon Sep 17 00:00:00 2001
From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Date: Wed, 9 May 2018 13:56:19 +0100
Subject: [PATCH] ASoC: qdsp6: q6core: Add q6core driver
This patch adds support to core apr service, which is used to query
status of other static and dynamic services on the dsp.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Reviewed-and-tested-by: Rohit kumar <rohitkr@codeaurora.org>
Acked-by: Banajit Goswami <bgoswami@codeaurora.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/qcom/Kconfig | 4 +
sound/soc/qcom/qdsp6/Makefile | 1 +
sound/soc/qcom/qdsp6/q6core.c | 380 ++++++++++++++++++++++++++++++++++
sound/soc/qcom/qdsp6/q6core.h | 15 ++
4 files changed, 400 insertions(+)
create mode 100644 sound/soc/qcom/qdsp6/q6core.c
create mode 100644 sound/soc/qcom/qdsp6/q6core.h
diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
index b44a9fcd7ed3..37ee0d958145 100644
--- a/sound/soc/qcom/Kconfig
+++ b/sound/soc/qcom/Kconfig
@@ -44,10 +44,14 @@ config SND_SOC_APQ8016_SBC
config SND_SOC_QDSP6_COMMON
tristate
+config SND_SOC_QDSP6_CORE
+ tristate
+
config SND_SOC_QDSP6
tristate "SoC ALSA audio driver for QDSP6"
depends on QCOM_APR && HAS_DMA
select SND_SOC_QDSP6_COMMON
+ select SND_SOC_QDSP6_CORE
help
To add support for MSM QDSP6 Soc Audio.
This will enable sound soc platform specific
diff --git a/sound/soc/qcom/qdsp6/Makefile b/sound/soc/qcom/qdsp6/Makefile
index accebdb49306..03b8e89c9731 100644
--- a/sound/soc/qcom/qdsp6/Makefile
+++ b/sound/soc/qcom/qdsp6/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_SND_SOC_QDSP6_COMMON) += q6dsp-common.o
+obj-$(CONFIG_SND_SOC_QDSP6_CORE) += q6core.o
diff --git a/sound/soc/qcom/qdsp6/q6core.c b/sound/soc/qcom/qdsp6/q6core.c
new file mode 100644
index 000000000000..06f03a5fe9bd
--- /dev/null
+++ b/sound/soc/qcom/qdsp6/q6core.c
@@ -0,0 +1,380 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2011-2017, The Linux Foundation. All rights reserved.
+// Copyright (c) 2018, Linaro Limited
+
+#include <linux/slab.h>
+#include <linux/wait.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/sched.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/jiffies.h>
+#include <linux/wait.h>
+#include <linux/soc/qcom/apr.h>
+#include "q6core.h"
+#include "q6dsp-errno.h"
+
+#define ADSP_STATE_READY_TIMEOUT_MS 3000
+#define Q6_READY_TIMEOUT_MS 100
+#define AVCS_CMD_ADSP_EVENT_GET_STATE 0x0001290C
+#define AVCS_CMDRSP_ADSP_EVENT_GET_STATE 0x0001290D
+#define AVCS_GET_VERSIONS 0x00012905
+#define AVCS_GET_VERSIONS_RSP 0x00012906
+#define AVCS_CMD_GET_FWK_VERSION 0x001292c
+#define AVCS_CMDRSP_GET_FWK_VERSION 0x001292d
+
+struct avcs_svc_info {
+ uint32_t service_id;
+ uint32_t version;
+} __packed;
+
+struct avcs_cmdrsp_get_version {
+ uint32_t build_id;
+ uint32_t num_services;
+ struct avcs_svc_info svc_api_info[];
+} __packed;
+
+/* for ADSP2.8 and above */
+struct avcs_svc_api_info {
+ uint32_t service_id;
+ uint32_t api_version;
+ uint32_t api_branch_version;
+} __packed;
+
+struct avcs_cmdrsp_get_fwk_version {
+ uint32_t build_major_version;
+ uint32_t build_minor_version;
+ uint32_t build_branch_version;
+ uint32_t build_subbranch_version;
+ uint32_t num_services;
+ struct avcs_svc_api_info svc_api_info[];
+} __packed;
+
+struct q6core {
+ struct apr_device *adev;
+ wait_queue_head_t wait;
+ uint32_t avcs_state;
+ struct mutex lock;
+ bool resp_received;
+ uint32_t num_services;
+ struct avcs_cmdrsp_get_fwk_version *fwk_version;
+ struct avcs_cmdrsp_get_version *svc_version;
+ bool fwk_version_supported;
+ bool get_state_supported;
+ bool get_version_supported;
+ bool is_version_requested;
+};
+
+static struct q6core *g_core;
+
+static int q6core_callback(struct apr_device *adev, struct apr_resp_pkt *data)
+{
+ struct q6core *core = dev_get_drvdata(&adev->dev);
+ struct aprv2_ibasic_rsp_result_t *result;
+ struct apr_hdr *hdr = &data->hdr;
+
+ result = data->payload;
+ switch (hdr->opcode) {
+ case APR_BASIC_RSP_RESULT:{
+ result = data->payload;
+ switch (result->opcode) {
+ case AVCS_GET_VERSIONS:
+ if (result->status == ADSP_EUNSUPPORTED)
+ core->get_version_supported = false;
+ core->resp_received = true;
+ break;
+ case AVCS_CMD_GET_FWK_VERSION:
+ if (result->status == ADSP_EUNSUPPORTED)
+ core->fwk_version_supported = false;
+ core->resp_received = true;
+ break;
+ case AVCS_CMD_ADSP_EVENT_GET_STATE:
+ if (result->status == ADSP_EUNSUPPORTED)
+ core->get_state_supported = false;
+ core->resp_received = true;
+ break;
+ }
+ break;
+ }
+ case AVCS_CMDRSP_GET_FWK_VERSION: {
+ struct avcs_cmdrsp_get_fwk_version *fwk;
+ int bytes;
+
+ fwk = data->payload;
+ bytes = sizeof(*fwk) + fwk->num_services *
+ sizeof(fwk->svc_api_info[0]);
+
+ core->fwk_version = kzalloc(bytes, GFP_ATOMIC);
+ if (!core->fwk_version)
+ return -ENOMEM;
+
+ memcpy(core->fwk_version, data->payload, bytes);
+
+ core->fwk_version_supported = true;
+ core->resp_received = true;
+
+ break;
+ }
+ case AVCS_GET_VERSIONS_RSP: {
+ struct avcs_cmdrsp_get_version *v;
+ int len;
+
+ v = data->payload;
+
+ len = sizeof(*v) + v->num_services * sizeof(v->svc_api_info[0]);
+
+ core->svc_version = kzalloc(len, GFP_ATOMIC);
+ if (!core->svc_version)
+ return -ENOMEM;
+
+ memcpy(core->svc_version, data->payload, len);
+
+ core->get_version_supported = true;
+ core->resp_received = true;
+
+ break;
+ }
+ case AVCS_CMDRSP_ADSP_EVENT_GET_STATE:
+ core->get_state_supported = true;
+ core->avcs_state = result->opcode;
+
+ core->resp_received = true;
+ break;
+ default:
+ dev_err(&adev->dev, "Message id from adsp core svc: 0x%x\n",
+ hdr->opcode);
+ break;
+ }
+
+ if (core->resp_received)
+ wake_up(&core->wait);
+
+ return 0;
+}
+
+static int q6core_get_fwk_versions(struct q6core *core)
+{
+ struct apr_device *adev = core->adev;
+ struct apr_pkt pkt;
+ int rc;
+
+ pkt.hdr.hdr_field = APR_HDR_FIELD(APR_MSG_TYPE_SEQ_CMD,
+ APR_HDR_LEN(APR_HDR_SIZE), APR_PKT_VER);
+ pkt.hdr.pkt_size = APR_HDR_SIZE;
+ pkt.hdr.opcode = AVCS_CMD_GET_FWK_VERSION;
+
+ rc = apr_send_pkt(adev, &pkt);
+ if (rc < 0)
+ return rc;
+
+ rc = wait_event_timeout(core->wait, (core->resp_received),
+ msecs_to_jiffies(Q6_READY_TIMEOUT_MS));
+ if (rc > 0 && core->resp_received) {
+ core->resp_received = false;
+
+ if (!core->fwk_version_supported)
+ return -ENOTSUPP;
+ else
+ return 0;
+ }
+
+
+ return rc;
+}
+
+static int q6core_get_svc_versions(struct q6core *core)
+{
+ struct apr_device *adev = core->adev;
+ struct apr_pkt pkt;
+ int rc;
+
+ pkt.hdr.hdr_field = APR_HDR_FIELD(APR_MSG_TYPE_SEQ_CMD,
+ APR_HDR_LEN(APR_HDR_SIZE), APR_PKT_VER);
+ pkt.hdr.pkt_size = APR_HDR_SIZE;
+ pkt.hdr.opcode = AVCS_GET_VERSIONS;
+
+ rc = apr_send_pkt(adev, &pkt);
+ if (rc < 0)
+ return rc;
+
+ rc = wait_event_timeout(core->wait, (core->resp_received),
+ msecs_to_jiffies(Q6_READY_TIMEOUT_MS));
+ if (rc > 0 && core->resp_received) {
+ core->resp_received = false;
+ return 0;
+ }
+
+ return rc;
+}
+
+static bool __q6core_is_adsp_ready(struct q6core *core)
+{
+ struct apr_device *adev = core->adev;
+ struct apr_pkt pkt;
+ int rc;
+
+ core->get_state_supported = false;
+
+ pkt.hdr.hdr_field = APR_HDR_FIELD(APR_MSG_TYPE_SEQ_CMD,
+ APR_HDR_LEN(APR_HDR_SIZE), APR_PKT_VER);
+ pkt.hdr.pkt_size = APR_HDR_SIZE;
+ pkt.hdr.opcode = AVCS_CMD_ADSP_EVENT_GET_STATE;
+
+ rc = apr_send_pkt(adev, &pkt);
+ if (rc < 0)
+ return false;
+
+ rc = wait_event_timeout(core->wait, (core->resp_received),
+ msecs_to_jiffies(Q6_READY_TIMEOUT_MS));
+ if (rc > 0 && core->resp_received) {
+ core->resp_received = false;
+
+ if (core->avcs_state)
+ return true;
+ }
+
+ /* assume that the adsp is up if we not support this command */
+ if (!core->get_state_supported)
+ return true;
+
+ return false;
+}
+
+/**
+ * q6core_get_svc_api_info() - Get version number of a service.
+ *
+ * @svc_id: service id of the service.
+ * @ainfo: Valid struct pointer to fill svc api information.
+ *
+ * Return: zero on success and error code on failure or unsupported
+ */
+int q6core_get_svc_api_info(int svc_id, struct q6core_svc_api_info *ainfo)
+{
+ int i;
+ int ret = -ENOTSUPP;
+
+ if (!g_core || !ainfo)
+ return 0;
+
+ mutex_lock(&g_core->lock);
+ if (!g_core->is_version_requested) {
+ if (q6core_get_fwk_versions(g_core) == -ENOTSUPP)
+ q6core_get_svc_versions(g_core);
+ g_core->is_version_requested = true;
+ }
+
+ if (g_core->fwk_version_supported) {
+ for (i = 0; i < g_core->fwk_version->num_services; i++) {
+ struct avcs_svc_api_info *info;
+
+ info = &g_core->fwk_version->svc_api_info[i];
+ if (svc_id != info->service_id)
+ continue;
+
+ ainfo->api_version = info->api_version;
+ ainfo->api_branch_version = info->api_branch_version;
+ ret = 0;
+ break;
+ }
+ } else if (g_core->get_version_supported) {
+ for (i = 0; i < g_core->svc_version->num_services; i++) {
+ struct avcs_svc_info *info;
+
+ info = &g_core->svc_version->svc_api_info[i];
+ if (svc_id != info->service_id)
+ continue;
+
+ ainfo->api_version = info->version;
+ ainfo->api_branch_version = 0;
+ ret = 0;
+ break;
+ }
+ }
+
+ mutex_unlock(&g_core->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(q6core_get_svc_api_info);
+
+/**
+ * q6core_is_adsp_ready() - Get status of adsp
+ *
+ * Return: Will be an true if adsp is ready and false if not.
+ */
+bool q6core_is_adsp_ready(void)
+{
+ unsigned long timeout;
+ bool ret = false;
+
+ if (!g_core)
+ return false;
+
+ mutex_lock(&g_core->lock);
+ timeout = jiffies + msecs_to_jiffies(ADSP_STATE_READY_TIMEOUT_MS);
+ for (;;) {
+ if (__q6core_is_adsp_ready(g_core)) {
+ ret = true;
+ break;
+ }
+
+ if (!time_after(timeout, jiffies)) {
+ ret = false;
+ break;
+ }
+ }
+
+ mutex_unlock(&g_core->lock);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(q6core_is_adsp_ready);
+
+static int q6core_probe(struct apr_device *adev)
+{
+ g_core = kzalloc(sizeof(*g_core), GFP_KERNEL);
+ if (!g_core)
+ return -ENOMEM;
+
+ dev_set_drvdata(&adev->dev, g_core);
+
+ mutex_init(&g_core->lock);
+ g_core->adev = adev;
+ init_waitqueue_head(&g_core->wait);
+ return 0;
+}
+
+static int q6core_exit(struct apr_device *adev)
+{
+ struct q6core *core = dev_get_drvdata(&adev->dev);
+
+ if (core->fwk_version_supported)
+ kfree(core->fwk_version);
+ if (core->get_version_supported)
+ kfree(core->svc_version);
+
+ g_core = NULL;
+ kfree(core);
+
+ return 0;
+}
+
+static const struct of_device_id q6core_device_id[] = {
+ { .compatible = "qcom,q6core" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, q6core_device_id);
+
+static struct apr_driver qcom_q6core_driver = {
+ .probe = q6core_probe,
+ .remove = q6core_exit,
+ .callback = q6core_callback,
+ .driver = {
+ .name = "qcom-q6core",
+ .of_match_table = of_match_ptr(q6core_device_id),
+ },
+};
+
+module_apr_driver(qcom_q6core_driver);
+MODULE_DESCRIPTION("q6 core");
+MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/qcom/qdsp6/q6core.h b/sound/soc/qcom/qdsp6/q6core.h
new file mode 100644
index 000000000000..4105b1d730be
--- /dev/null
+++ b/sound/soc/qcom/qdsp6/q6core.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __Q6CORE_H__
+#define __Q6CORE_H__
+
+struct q6core_svc_api_info {
+ uint32_t service_id;
+ uint32_t api_version;
+ uint32_t api_branch_version;
+};
+
+bool q6core_is_adsp_ready(void);
+int q6core_get_svc_api_info(int svc_id, struct q6core_svc_api_info *ainfo);
+
+#endif /* __Q6CORE_H__ */
--
2.17.0
^ permalink raw reply related
* Re: [PATCH v7 03/14] clk: qcom: Add CPU clock driver for msm8996
From: kbuild test robot @ 2018-05-17 16:31 UTC (permalink / raw)
Cc: kbuild-all, mturquette, sboyd, robh, mark.rutland, viresh.kumar,
nm, lgirdwood, broonie, andy.gross, david.brown, catalin.marinas,
will.deacon, rjw, linux-clk, devicetree, linux-kernel, linux-pm,
linux-arm-msm, linux-soc, linux-arm-kernel, rnayak, ilialin,
amit.kucheria, nicolas.dechesne, celster, tfinkel
In-Reply-To: <1526375616-16904-4-git-send-email-ilialin@codeaurora.org>
[-- Attachment #1: Type: text/plain, Size: 1445 bytes --]
Hi Ilia,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.17-rc5]
[cannot apply to clk/clk-next next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Ilia-Lin/CPU-scaling-support-for-msm8996/20180516-064721
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All warnings (new ones prefixed by >>):
>> drivers/clk/qcom/clk-cpu-8996.c:142:2: warning: this decimal constant is unsigned only in ISO C90
VCO(0, 1000000000, 2150400000),
^~~
vim +142 drivers/clk/qcom/clk-cpu-8996.c
137
138 static const struct pll_vco alt_pll_vco_modes[] = {
139 VCO(3, 250000000, 500000000),
140 VCO(2, 500000000, 750000000),
141 VCO(1, 750000000, 1000000000),
> 142 VCO(0, 1000000000, 2150400000),
143 };
144
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52971 bytes --]
^ permalink raw reply
* Re: [PATCH v6 6/9] dt-bindings: counter: Document stm32 quadrature encoder
From: Rob Herring @ 2018-05-17 16:23 UTC (permalink / raw)
To: William Breathitt Gray
Cc: Jonathan Cameron, Benjamin Gaignard, Fabrice Gasnier, linux-iio,
linux-kernel@vger.kernel.org, devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Mark Rutland
In-Reply-To: <ccdbbfb2833d274b6bfa234aada57e68d9793dd5.1526487615.git.vilhelm.gray@gmail.com>
On Wed, May 16, 2018 at 12:51 PM, William Breathitt Gray
<vilhelm.gray@gmail.com> wrote:
> From: Benjamin Gaignard <benjamin.gaignard@st.com>
v6? Where's v1-v5?
> Add bindings for STM32 Timer quadrature encoder.
> It is a sub-node of STM32 Timer which implement the
> counter part of the hardware.
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> ---
> .../bindings/counter/stm32-timer-cnt.txt | 26 +++++++++++++++++++
> .../devicetree/bindings/mfd/stm32-timers.txt | 7 +++++
> 2 files changed, 33 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt
>
> diff --git a/Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt b/Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt
> new file mode 100644
> index 000000000000..377728128bef
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt
> @@ -0,0 +1,26 @@
> +STMicroelectronics STM32 Timer quadrature encoder
> +
> +STM32 Timer provides quadrature encoder counter mode to detect
'mode' does not sound like a sub-block of the timers block.
> +angular position and direction of rotary elements,
> +from IN1 and IN2 input signals.
> +
> +Must be a sub-node of an STM32 Timer device tree node.
> +See ../mfd/stm32-timers.txt for details about the parent node.
> +
> +Required properties:
> +- compatible: Must be "st,stm32-timer-counter".
> +- pinctrl-names: Set to "default".
> +- pinctrl-0: List of phandles pointing to pin configuration nodes,
> + to set IN1/IN2 pins in mode of operation for Low-Power
> + Timer input on external pin.
> +
> +Example:
> + timers@40010000 {
> + compatible = "st,stm32-timers";
> + ...
> + counter {
> + compatible = "st,stm32-timer-counter";
Is there only 1? How is the counter addressed?
> + pinctrl-names = "default";
> + pinctrl-0 = <&tim1_in_pins>;
> + };
> + };
^ permalink raw reply
* Re: [PATCH] ASoC: ssm2602: Fix ADC powerup sequencing
From: Rob Herring @ 2018-05-17 16:14 UTC (permalink / raw)
To: Marco Felsch
Cc: Mark Rutland, devicetree, Linux-ALSA, Lars-Peter Clausen,
Philipp Zabel, Sascha Hauer, Liam Girdwood, Mark Brown,
Sascha Hauer
In-Reply-To: <20180517133009.26079-1-m.felsch@pengutronix.de>
On Thu, May 17, 2018 at 8:30 AM, Marco Felsch <m.felsch@pengutronix.de> wrote:
> From: Philipp Zabel <p.zabel@pengutronix.de>
>
> According to the ssm2603 data sheet (control register sequencing), the
> digital core should be activated only after all necessary bits in the
> power register are enabled, and a delay determined by the decoupling
> capacitor on the VMID pin has passed. If the digital core is activated
> too early, or even before the ADC is powered up, audible artifacts
> appear at the beginning of the recorded signal.
>
> The digital core is also needed for playback, so when recording starts
> it may already be enabled. This means we cannot get the power sequence
> correct when we want to be able to start recording after playback.
>
> As a workaround put the MIC mute switch into the DAPM routes. This
> way we can keep the recording disabled until the MIC Bias has settled
> and thus get rid of audible artifacts.
>
> The delay we have to wait depends on a board specific capacitor
> connected to the VMID pins, so make the delay configurable in the device
> tree.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> .../devicetree/bindings/sound/adi,ssm2602.txt | 7 +++++
> sound/soc/codecs/ssm2602.c | 30 +++++++++++++++++--
> 2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/adi,ssm2602.txt b/Documentation/devicetree/bindings/sound/adi,ssm2602.txt
> index 3b3302fe399b..9334d48c0462 100644
> --- a/Documentation/devicetree/bindings/sound/adi,ssm2602.txt
> +++ b/Documentation/devicetree/bindings/sound/adi,ssm2602.txt
> @@ -11,9 +11,16 @@ Required properties:
> - reg : the I2C address of the device for I2C, the chip select
> number for SPI.
>
> +Optional properties:
> +
> + - startup-delay-us : delay between powering on and activating the digital
> + core, determined by the decoupling capacitor C on the
> + VMID pin: delay [µs] = C [µF] * 25000 / 3.5
> +
We already have similarly defined property. Please reuse that. See mmc
pwrseq binding.
Also, please split bindings to separate patch.
Rob
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply
* Re: [PATCH v2 17/40] iommu/arm-smmu-v3: Link domains and devices
From: Jonathan Cameron @ 2018-05-17 16:07 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: kvm-u79uwXL29TY76Z2rM5mHXA, linux-pci-u79uwXL29TY76Z2rM5mHXA,
xuzaibo-hv44wF8Li93QT0dZR+AlfA, will.deacon-5wv7dgnIgG8,
okaya-sgV2jX0FEOL9JmXXK+q4OQ, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
ashok.raj-ral2JQCrhuEAvxtiuMwx3w, bharatku-gjFFaj9aHVfQT0dZR+AlfA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA, rfranz-YGCgFSpz5w/QT0dZR+AlfA,
devicetree-u79uwXL29TY76Z2rM5mHXA, rgummal-gjFFaj9aHVfQT0dZR+AlfA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
christian.koenig-5C7GfCeVMHo
In-Reply-To: <20180511190641.23008-18-jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org>
On Fri, 11 May 2018 20:06:18 +0100
Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org> wrote:
> When removing a mapping from a domain, we need to send an invalidation to
> all devices that might have stored it in their Address Translation Cache
> (ATC). In addition when updating the context descriptor of a live domain,
> we'll need to send invalidations for all devices attached to it.
>
> Maintain a list of devices in each domain, protected by a spinlock. It is
> updated every time we attach or detach devices to and from domains.
>
> It needs to be a spinlock because we'll invalidate ATC entries from
> within hardirq-safe contexts, but it may be possible to relax the read
> side with RCU later.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org>
Trivial naming suggestion...
> ---
> drivers/iommu/arm-smmu-v3.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 1d647104bccc..c892f012fb43 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -595,6 +595,11 @@ struct arm_smmu_device {
> struct arm_smmu_master_data {
> struct arm_smmu_device *smmu;
> struct arm_smmu_strtab_ent ste;
> +
> + struct arm_smmu_domain *domain;
> + struct list_head list; /* domain->devices */
More meaningful name perhaps to avoid the need for the comment?
> +
> + struct device *dev;
> };
>
> /* SMMU private data for an IOMMU domain */
> @@ -618,6 +623,9 @@ struct arm_smmu_domain {
> };
>
> struct iommu_domain domain;
> +
> + struct list_head devices;
> + spinlock_t devices_lock;
> };
>
> struct arm_smmu_option_prop {
> @@ -1470,6 +1478,9 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
> }
>
> mutex_init(&smmu_domain->init_mutex);
> + INIT_LIST_HEAD(&smmu_domain->devices);
> + spin_lock_init(&smmu_domain->devices_lock);
> +
> return &smmu_domain->domain;
> }
>
> @@ -1685,7 +1696,17 @@ static void arm_smmu_install_ste_for_dev(struct iommu_fwspec *fwspec)
>
> static void arm_smmu_detach_dev(struct device *dev)
> {
> + unsigned long flags;
> struct arm_smmu_master_data *master = dev->iommu_fwspec->iommu_priv;
> + struct arm_smmu_domain *smmu_domain = master->domain;
> +
> + if (smmu_domain) {
> + spin_lock_irqsave(&smmu_domain->devices_lock, flags);
> + list_del(&master->list);
> + spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
> +
> + master->domain = NULL;
> + }
>
> master->ste.assigned = false;
> arm_smmu_install_ste_for_dev(dev->iommu_fwspec);
> @@ -1694,6 +1715,7 @@ static void arm_smmu_detach_dev(struct device *dev)
> static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> {
> int ret = 0;
> + unsigned long flags;
> struct arm_smmu_device *smmu;
> struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> struct arm_smmu_master_data *master;
> @@ -1729,6 +1751,11 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> }
>
> ste->assigned = true;
> + master->domain = smmu_domain;
> +
> + spin_lock_irqsave(&smmu_domain->devices_lock, flags);
> + list_add(&master->list, &smmu_domain->devices);
> + spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
>
> if (smmu_domain->stage == ARM_SMMU_DOMAIN_BYPASS) {
> ste->s1_cfg = NULL;
> @@ -1847,6 +1874,7 @@ static int arm_smmu_add_device(struct device *dev)
> return -ENOMEM;
>
> master->smmu = smmu;
> + master->dev = dev;
> fwspec->iommu_priv = master;
> }
>
^ permalink raw reply
* Re: [PATCH v2 13/40] vfio: Add support for Shared Virtual Addressing
From: Jonathan Cameron @ 2018-05-17 15:58 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: kvm-u79uwXL29TY76Z2rM5mHXA, linux-pci-u79uwXL29TY76Z2rM5mHXA,
xuzaibo-hv44wF8Li93QT0dZR+AlfA, will.deacon-5wv7dgnIgG8,
okaya-sgV2jX0FEOL9JmXXK+q4OQ, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
ashok.raj-ral2JQCrhuEAvxtiuMwx3w, bharatku-gjFFaj9aHVfQT0dZR+AlfA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA, rfranz-YGCgFSpz5w/QT0dZR+AlfA,
devicetree-u79uwXL29TY76Z2rM5mHXA, rgummal-gjFFaj9aHVfQT0dZR+AlfA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
christian.koenig-5C7GfCeVMHo
In-Reply-To: <20180511190641.23008-14-jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org>
On Fri, 11 May 2018 20:06:14 +0100
Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org> wrote:
> Add two new ioctls for VFIO containers. VFIO_IOMMU_BIND_PROCESS creates a
> bond between a container and a process address space, identified by a
> Process Address Space ID (PASID). Devices in the container append this
> PASID to DMA transactions in order to access the process' address space.
> The process page tables are shared with the IOMMU, and mechanisms such as
> PCI ATS/PRI are used to handle faults. VFIO_IOMMU_UNBIND_PROCESS removes a
> bond created with VFIO_IOMMU_BIND_PROCESS.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org>
A couple of small comments inline..
Jonathan
>
> ---
> v1->v2:
> * Simplify mm_exit
> * Can be built as module again
> ---
> drivers/vfio/vfio_iommu_type1.c | 449 ++++++++++++++++++++++++++++++--
> include/uapi/linux/vfio.h | 76 ++++++
> 2 files changed, 504 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 5c212bf29640..2902774062b8 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -30,6 +30,7 @@
> #include <linux/iommu.h>
> #include <linux/module.h>
> #include <linux/mm.h>
> +#include <linux/ptrace.h>
> #include <linux/rbtree.h>
> #include <linux/sched/signal.h>
> #include <linux/sched/mm.h>
> @@ -60,6 +61,7 @@ MODULE_PARM_DESC(disable_hugepages,
>
> struct vfio_iommu {
> struct list_head domain_list;
> + struct list_head mm_list;
> struct vfio_domain *external_domain; /* domain for external user */
> struct mutex lock;
> struct rb_root dma_list;
> @@ -90,6 +92,14 @@ struct vfio_dma {
> struct vfio_group {
> struct iommu_group *iommu_group;
> struct list_head next;
> + bool sva_enabled;
> +};
> +
> +struct vfio_mm {
> +#define VFIO_PASID_INVALID (-1)
> + int pasid;
> + struct mm_struct *mm;
> + struct list_head next;
> };
>
> /*
> @@ -1238,6 +1248,164 @@ static int vfio_iommu_replay(struct vfio_iommu *iommu,
> return 0;
> }
>
> +static int vfio_iommu_mm_exit(struct device *dev, int pasid, void *data)
> +{
> + struct vfio_mm *vfio_mm;
> + struct vfio_iommu *iommu = data;
> +
> + mutex_lock(&iommu->lock);
> + list_for_each_entry(vfio_mm, &iommu->mm_list, next) {
> + if (vfio_mm->pasid == pasid) {
> + list_del(&vfio_mm->next);
> + kfree(vfio_mm);
> + break;
> + }
> + }
> + mutex_unlock(&iommu->lock);
> +
> + return 0;
> +}
> +
> +static int vfio_iommu_sva_init(struct device *dev, void *data)
> +{
> + return iommu_sva_device_init(dev, IOMMU_SVA_FEAT_IOPF, 0,
> + vfio_iommu_mm_exit);
> +}
> +
> +static int vfio_iommu_sva_shutdown(struct device *dev, void *data)
> +{
> + iommu_sva_device_shutdown(dev);
> +
> + return 0;
> +}
> +
> +struct vfio_iommu_sva_bind_data {
> + struct vfio_mm *vfio_mm;
> + struct vfio_iommu *iommu;
> + int count;
> +};
> +
> +static int vfio_iommu_sva_bind_dev(struct device *dev, void *data)
> +{
> + struct vfio_iommu_sva_bind_data *bind_data = data;
> +
> + /* Multi-device groups aren't support for SVA */
> + if (bind_data->count++)
> + return -EINVAL;
> +
> + return __iommu_sva_bind_device(dev, bind_data->vfio_mm->mm,
> + &bind_data->vfio_mm->pasid,
> + IOMMU_SVA_FEAT_IOPF, bind_data->iommu);
> +}
> +
> +static int vfio_iommu_sva_unbind_dev(struct device *dev, void *data)
> +{
> + struct vfio_mm *vfio_mm = data;
> +
> + return __iommu_sva_unbind_device(dev, vfio_mm->pasid);
> +}
> +
> +static int vfio_iommu_bind_group(struct vfio_iommu *iommu,
> + struct vfio_group *group,
> + struct vfio_mm *vfio_mm)
> +{
> + int ret;
> + bool enabled_sva = false;
> + struct vfio_iommu_sva_bind_data data = {
> + .vfio_mm = vfio_mm,
> + .iommu = iommu,
> + .count = 0,
> + };
> +
> + if (!group->sva_enabled) {
> + ret = iommu_group_for_each_dev(group->iommu_group, NULL,
> + vfio_iommu_sva_init);
> + if (ret)
> + return ret;
> +
> + group->sva_enabled = enabled_sva = true;
> + }
> +
> + ret = iommu_group_for_each_dev(group->iommu_group, &data,
> + vfio_iommu_sva_bind_dev);
> + if (ret && data.count > 1)
Are we safe to run this even if data.count == 1? I assume that at
that point we would always not have an iommu domain associated with the
device so the initial check would error out.
Just be nice to get rid of the special casing in this error path as then
could just do it all under if (ret) and make it visually clearer these
are different aspects of the same error path.
> + iommu_group_for_each_dev(group->iommu_group, vfio_mm,
> + vfio_iommu_sva_unbind_dev);
> + if (ret && enabled_sva) {
> + iommu_group_for_each_dev(group->iommu_group, NULL,
> + vfio_iommu_sva_shutdown);
> + group->sva_enabled = false;
> + }
> +
> + return ret;
> +}
> +
> +static void vfio_iommu_unbind_group(struct vfio_group *group,
> + struct vfio_mm *vfio_mm)
> +{
> + iommu_group_for_each_dev(group->iommu_group, vfio_mm,
> + vfio_iommu_sva_unbind_dev);
> +}
> +
> +static void vfio_iommu_unbind(struct vfio_iommu *iommu,
> + struct vfio_mm *vfio_mm)
> +{
> + struct vfio_group *group;
> + struct vfio_domain *domain;
> +
> + list_for_each_entry(domain, &iommu->domain_list, next)
> + list_for_each_entry(group, &domain->group_list, next)
> + vfio_iommu_unbind_group(group, vfio_mm);
> +}
> +
> +static int vfio_iommu_replay_bind(struct vfio_iommu *iommu,
> + struct vfio_group *group)
> +{
> + int ret = 0;
> + struct vfio_mm *vfio_mm;
> +
> + list_for_each_entry(vfio_mm, &iommu->mm_list, next) {
> + /*
> + * Ensure that mm doesn't exit while we're binding it to the new
> + * group. It may already have exited, and the mm_exit notifier
> + * might be waiting on the IOMMU mutex to remove this vfio_mm
> + * from the list.
> + */
> + if (!mmget_not_zero(vfio_mm->mm))
> + continue;
> + ret = vfio_iommu_bind_group(iommu, group, vfio_mm);
> + /*
> + * Use async to avoid triggering an mm_exit callback right away,
> + * which would block on the mutex that we're holding.
> + */
> + mmput_async(vfio_mm->mm);
> +
> + if (ret)
> + goto out_unbind;
> + }
> +
> + return 0;
> +
> +out_unbind:
> + list_for_each_entry_continue_reverse(vfio_mm, &iommu->mm_list, next)
> + vfio_iommu_unbind_group(group, vfio_mm);
> +
> + return ret;
> +}
> +
> +static void vfio_iommu_free_all_mm(struct vfio_iommu *iommu)
> +{
> + struct vfio_mm *vfio_mm, *next;
> +
> + /*
> + * No need for unbind() here. Since all groups are detached from this
> + * iommu, bonds have been removed.
> + */
> + list_for_each_entry_safe(vfio_mm, next, &iommu->mm_list, next)
> + kfree(vfio_mm);
> + INIT_LIST_HEAD(&iommu->mm_list);
> +}
> +
> /*
> * We change our unmap behavior slightly depending on whether the IOMMU
> * supports fine-grained superpages. IOMMUs like AMD-Vi will use a superpage
> @@ -1313,6 +1481,44 @@ static bool vfio_iommu_has_sw_msi(struct iommu_group *group, phys_addr_t *base)
> return ret;
> }
>
> +static int vfio_iommu_try_attach_group(struct vfio_iommu *iommu,
> + struct vfio_group *group,
> + struct vfio_domain *cur_domain,
> + struct vfio_domain *new_domain)
> +{
> + struct iommu_group *iommu_group = group->iommu_group;
> +
> + /*
> + * Try to match an existing compatible domain. We don't want to
> + * preclude an IOMMU driver supporting multiple bus_types and being
> + * able to include different bus_types in the same IOMMU domain, so
> + * we test whether the domains use the same iommu_ops rather than
> + * testing if they're on the same bus_type.
> + */
> + if (new_domain->domain->ops != cur_domain->domain->ops ||
> + new_domain->prot != cur_domain->prot)
> + return 1;
> +
> + iommu_detach_group(cur_domain->domain, iommu_group);
> +
> + if (iommu_attach_group(new_domain->domain, iommu_group))
> + goto out_reattach;
> +
> + if (vfio_iommu_replay_bind(iommu, group))
> + goto out_detach;
> +
> + return 0;
> +
> +out_detach:
> + iommu_detach_group(new_domain->domain, iommu_group);
> +
> +out_reattach:
> + if (iommu_attach_group(cur_domain->domain, iommu_group))
> + return -EINVAL;
> +
> + return 1;
> +}
> +
> static int vfio_iommu_type1_attach_group(void *iommu_data,
> struct iommu_group *iommu_group)
> {
> @@ -1410,28 +1616,16 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> if (iommu_capable(bus, IOMMU_CAP_CACHE_COHERENCY))
> domain->prot |= IOMMU_CACHE;
>
> - /*
> - * Try to match an existing compatible domain. We don't want to
> - * preclude an IOMMU driver supporting multiple bus_types and being
> - * able to include different bus_types in the same IOMMU domain, so
> - * we test whether the domains use the same iommu_ops rather than
> - * testing if they're on the same bus_type.
> - */
> list_for_each_entry(d, &iommu->domain_list, next) {
> - if (d->domain->ops == domain->domain->ops &&
> - d->prot == domain->prot) {
> - iommu_detach_group(domain->domain, iommu_group);
> - if (!iommu_attach_group(d->domain, iommu_group)) {
> - list_add(&group->next, &d->group_list);
> - iommu_domain_free(domain->domain);
> - kfree(domain);
> - mutex_unlock(&iommu->lock);
> - return 0;
> - }
> -
> - ret = iommu_attach_group(domain->domain, iommu_group);
> - if (ret)
> - goto out_domain;
> + ret = vfio_iommu_try_attach_group(iommu, group, domain, d);
> + if (ret < 0) {
> + goto out_domain;
> + } else if (!ret) {
> + list_add(&group->next, &d->group_list);
> + iommu_domain_free(domain->domain);
> + kfree(domain);
> + mutex_unlock(&iommu->lock);
> + return 0;
> }
> }
>
> @@ -1442,6 +1636,10 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> if (ret)
> goto out_detach;
>
> + ret = vfio_iommu_replay_bind(iommu, group);
> + if (ret)
> + goto out_detach;
> +
> if (resv_msi) {
> ret = iommu_get_msi_cookie(domain->domain, resv_msi_base);
> if (ret)
> @@ -1547,6 +1745,11 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
> continue;
>
> iommu_detach_group(domain->domain, iommu_group);
> + if (group->sva_enabled) {
> + iommu_group_for_each_dev(iommu_group, NULL,
> + vfio_iommu_sva_shutdown);
> + group->sva_enabled = false;
> + }
> list_del(&group->next);
> kfree(group);
> /*
> @@ -1562,6 +1765,7 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
> vfio_iommu_unmap_unpin_all(iommu);
> else
> vfio_iommu_unmap_unpin_reaccount(iommu);
> + vfio_iommu_free_all_mm(iommu);
> }
> iommu_domain_free(domain->domain);
> list_del(&domain->next);
> @@ -1596,6 +1800,7 @@ static void *vfio_iommu_type1_open(unsigned long arg)
> }
>
> INIT_LIST_HEAD(&iommu->domain_list);
> + INIT_LIST_HEAD(&iommu->mm_list);
> iommu->dma_list = RB_ROOT;
> mutex_init(&iommu->lock);
> BLOCKING_INIT_NOTIFIER_HEAD(&iommu->notifier);
> @@ -1630,6 +1835,7 @@ static void vfio_iommu_type1_release(void *iommu_data)
> kfree(iommu->external_domain);
> }
>
> + vfio_iommu_free_all_mm(iommu);
> vfio_iommu_unmap_unpin_all(iommu);
>
> list_for_each_entry_safe(domain, domain_tmp,
> @@ -1658,6 +1864,169 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu)
> return ret;
> }
>
> +static struct mm_struct *vfio_iommu_get_mm_by_vpid(pid_t vpid)
> +{
> + struct mm_struct *mm;
> + struct task_struct *task;
> +
> + task = find_get_task_by_vpid(vpid);
> + if (!task)
> + return ERR_PTR(-ESRCH);
> +
> + /* Ensure that current has RW access on the mm */
> + mm = mm_access(task, PTRACE_MODE_ATTACH_REALCREDS);
> + put_task_struct(task);
> +
> + if (!mm)
> + return ERR_PTR(-ESRCH);
> +
> + return mm;
> +}
> +
> +static long vfio_iommu_type1_bind_process(struct vfio_iommu *iommu,
> + void __user *arg,
> + struct vfio_iommu_type1_bind *bind)
> +{
> + struct vfio_iommu_type1_bind_process params;
> + struct vfio_domain *domain;
> + struct vfio_group *group;
> + struct vfio_mm *vfio_mm;
> + struct mm_struct *mm;
> + unsigned long minsz;
> + int ret = 0;
> +
> + minsz = sizeof(*bind) + sizeof(params);
> + if (bind->argsz < minsz)
> + return -EINVAL;
> +
> + arg += sizeof(*bind);
> + if (copy_from_user(¶ms, arg, sizeof(params)))
> + return -EFAULT;
> +
> + if (params.flags & ~VFIO_IOMMU_BIND_PID)
> + return -EINVAL;
> +
> + if (params.flags & VFIO_IOMMU_BIND_PID) {
> + mm = vfio_iommu_get_mm_by_vpid(params.pid);
> + if (IS_ERR(mm))
> + return PTR_ERR(mm);
> + } else {
> + mm = get_task_mm(current);
> + if (!mm)
> + return -EINVAL;
> + }
> +
> + mutex_lock(&iommu->lock);
> + if (!IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu)) {
> + ret = -EINVAL;
> + goto out_unlock;
> + }
> +
> + list_for_each_entry(vfio_mm, &iommu->mm_list, next) {
> + if (vfio_mm->mm == mm) {
> + params.pasid = vfio_mm->pasid;
> + ret = copy_to_user(arg, ¶ms, sizeof(params)) ?
> + -EFAULT : 0;
> + goto out_unlock;
> + }
> + }
> +
> + vfio_mm = kzalloc(sizeof(*vfio_mm), GFP_KERNEL);
> + if (!vfio_mm) {
> + ret = -ENOMEM;
> + goto out_unlock;
> + }
> +
> + vfio_mm->mm = mm;
> + vfio_mm->pasid = VFIO_PASID_INVALID;
> +
> + list_for_each_entry(domain, &iommu->domain_list, next) {
> + list_for_each_entry(group, &domain->group_list, next) {
> + ret = vfio_iommu_bind_group(iommu, group, vfio_mm);
> + if (ret)
> + goto out_unbind;
> + }
> + }
> +
> + list_add(&vfio_mm->next, &iommu->mm_list);
> +
> + params.pasid = vfio_mm->pasid;
> + ret = copy_to_user(arg, ¶ms, sizeof(params)) ? -EFAULT : 0;
> + if (ret)
> + goto out_delete;
> +
> + mutex_unlock(&iommu->lock);
> + mmput(mm);
> + return 0;
> +
> +out_delete:
> + list_del(&vfio_mm->next);
> +
> +out_unbind:
> + /* Undo all binds that already succeeded */
> + vfio_iommu_unbind(iommu, vfio_mm);
> + kfree(vfio_mm);
> +
> +out_unlock:
> + mutex_unlock(&iommu->lock);
> + mmput(mm);
> + return ret;
> +}
> +
> +static long vfio_iommu_type1_unbind_process(struct vfio_iommu *iommu,
> + void __user *arg,
> + struct vfio_iommu_type1_bind *bind)
> +{
> + int ret = -EINVAL;
> + unsigned long minsz;
> + struct mm_struct *mm;
> + struct vfio_mm *vfio_mm;
> + struct vfio_iommu_type1_bind_process params;
> +
> + minsz = sizeof(*bind) + sizeof(params);
> + if (bind->argsz < minsz)
> + return -EINVAL;
> +
> + arg += sizeof(*bind);
> + if (copy_from_user(¶ms, arg, sizeof(params)))
> + return -EFAULT;
> +
> + if (params.flags & ~VFIO_IOMMU_BIND_PID)
> + return -EINVAL;
> +
> + /*
> + * We can't simply call unbind with the PASID, because the process might
> + * have died and the PASID might have been reallocated to another
> + * process. Instead we need to fetch that process mm by PID again to
> + * make sure we remove the right vfio_mm.
> + */
> + if (params.flags & VFIO_IOMMU_BIND_PID) {
> + mm = vfio_iommu_get_mm_by_vpid(params.pid);
> + if (IS_ERR(mm))
> + return PTR_ERR(mm);
> + } else {
> + mm = get_task_mm(current);
> + if (!mm)
> + return -EINVAL;
> + }
> +
> + ret = -ESRCH;
> + mutex_lock(&iommu->lock);
> + list_for_each_entry(vfio_mm, &iommu->mm_list, next) {
> + if (vfio_mm->mm == mm) {
> + vfio_iommu_unbind(iommu, vfio_mm);
> + list_del(&vfio_mm->next);
> + kfree(vfio_mm);
> + ret = 0;
> + break;
> + }
> + }
> + mutex_unlock(&iommu->lock);
> + mmput(mm);
> +
> + return ret;
> +}
> +
> static long vfio_iommu_type1_ioctl(void *iommu_data,
> unsigned int cmd, unsigned long arg)
> {
> @@ -1728,6 +2097,44 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
>
> return copy_to_user((void __user *)arg, &unmap, minsz) ?
> -EFAULT : 0;
> +
> + } else if (cmd == VFIO_IOMMU_BIND) {
> + struct vfio_iommu_type1_bind bind;
> +
> + minsz = offsetofend(struct vfio_iommu_type1_bind, flags);
> +
> + if (copy_from_user(&bind, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (bind.argsz < minsz)
> + return -EINVAL;
> +
> + switch (bind.flags) {
> + case VFIO_IOMMU_BIND_PROCESS:
> + return vfio_iommu_type1_bind_process(iommu, (void *)arg,
> + &bind);
Can we combine these two blocks given it is only this case statement that is different?
> + default:
> + return -EINVAL;
> + }
> +
> + } else if (cmd == VFIO_IOMMU_UNBIND) {
> + struct vfio_iommu_type1_bind bind;
> +
> + minsz = offsetofend(struct vfio_iommu_type1_bind, flags);
> +
> + if (copy_from_user(&bind, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (bind.argsz < minsz)
> + return -EINVAL;
> +
> + switch (bind.flags) {
> + case VFIO_IOMMU_BIND_PROCESS:
> + return vfio_iommu_type1_unbind_process(iommu, (void *)arg,
> + &bind);
> + default:
> + return -EINVAL;
> + }
> }
>
> return -ENOTTY;
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 1aa7b82e8169..dc07752c8fe8 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -665,6 +665,82 @@ struct vfio_iommu_type1_dma_unmap {
> #define VFIO_IOMMU_ENABLE _IO(VFIO_TYPE, VFIO_BASE + 15)
> #define VFIO_IOMMU_DISABLE _IO(VFIO_TYPE, VFIO_BASE + 16)
>
> +/*
> + * VFIO_IOMMU_BIND_PROCESS
> + *
> + * Allocate a PASID for a process address space, and use it to attach this
> + * process to all devices in the container. Devices can then tag their DMA
> + * traffic with the returned @pasid to perform transactions on the associated
> + * virtual address space. Mapping and unmapping buffers is performed by standard
> + * functions such as mmap and malloc.
> + *
> + * If flag is VFIO_IOMMU_BIND_PID, @pid contains the pid of a foreign process to
> + * bind. Otherwise the current task is bound. Given that the caller owns the
> + * device, setting this flag grants the caller read and write permissions on the
> + * entire address space of foreign process described by @pid. Therefore,
> + * permission to perform the bind operation on a foreign process is governed by
> + * the ptrace access mode PTRACE_MODE_ATTACH_REALCREDS check. See man ptrace(2)
> + * for more information.
> + *
> + * On success, VFIO writes a Process Address Space ID (PASID) into @pasid. This
> + * ID is unique to a process and can be used on all devices in the container.
> + *
> + * On fork, the child inherits the device fd and can use the bonds setup by its
> + * parent. Consequently, the child has R/W access on the address spaces bound by
> + * its parent. After an execv, the device fd is closed and the child doesn't
> + * have access to the address space anymore.
> + *
> + * To remove a bond between process and container, VFIO_IOMMU_UNBIND ioctl is
> + * issued with the same parameters. If a pid was specified in VFIO_IOMMU_BIND,
> + * it should also be present for VFIO_IOMMU_UNBIND. Otherwise unbind the current
> + * task from the container.
> + */
> +struct vfio_iommu_type1_bind_process {
> + __u32 flags;
> +#define VFIO_IOMMU_BIND_PID (1 << 0)
> + __u32 pasid;
> + __s32 pid;
> +};
> +
> +/*
> + * Only mode supported at the moment is VFIO_IOMMU_BIND_PROCESS, which takes
> + * vfio_iommu_type1_bind_process in data.
> + */
> +struct vfio_iommu_type1_bind {
> + __u32 argsz;
> + __u32 flags;
> +#define VFIO_IOMMU_BIND_PROCESS (1 << 0)
> + __u8 data[];
> +};
> +
> +/*
> + * VFIO_IOMMU_BIND - _IOWR(VFIO_TYPE, VFIO_BASE + 22, struct vfio_iommu_bind)
> + *
> + * Manage address spaces of devices in this container. Initially a TYPE1
> + * container can only have one address space, managed with
> + * VFIO_IOMMU_MAP/UNMAP_DMA.
> + *
> + * An IOMMU of type VFIO_TYPE1_NESTING_IOMMU can be managed by both MAP/UNMAP
> + * and BIND ioctls at the same time. MAP/UNMAP acts on the stage-2 (host) page
> + * tables, and BIND manages the stage-1 (guest) page tables. Other types of
> + * IOMMU may allow MAP/UNMAP and BIND to coexist, where MAP/UNMAP controls
> + * non-PASID traffic and BIND controls PASID traffic. But this depends on the
> + * underlying IOMMU architecture and isn't guaranteed.
> + *
> + * Availability of this feature depends on the device, its bus, the underlying
> + * IOMMU and the CPU architecture.
> + *
> + * returns: 0 on success, -errno on failure.
> + */
> +#define VFIO_IOMMU_BIND _IO(VFIO_TYPE, VFIO_BASE + 22)
> +
> +/*
> + * VFIO_IOMMU_UNBIND - _IOWR(VFIO_TYPE, VFIO_BASE + 23, struct vfio_iommu_bind)
> + *
> + * Undo what was done by the corresponding VFIO_IOMMU_BIND ioctl.
> + */
> +#define VFIO_IOMMU_UNBIND _IO(VFIO_TYPE, VFIO_BASE + 23)
> +
> /* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */
>
> /*
^ permalink raw reply
* [PATCH 2/2] ARM: dts: stm32: enable USB OTG HS on stm32mp157c-ev1
From: Amelie Delaunay @ 2018-05-17 15:47 UTC (permalink / raw)
To: Maxime Coquelin, Alexandre Torgue, Rob Herring, Mark Rutland
Cc: linux-arm-kernel, devicetree, linux-kernel, Amelie Delaunay
In-Reply-To: <1526572052-17155-1-git-send-email-amelie.delaunay@st.com>
This patch enables USB OTG HS on stm32mp157c-ev1 in Peripheral mode.
Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
---
arch/arm/boot/dts/stm32mp157c-ev1.dts | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/stm32mp157c-ev1.dts b/arch/arm/boot/dts/stm32mp157c-ev1.dts
index 9382d80..5a0b77e 100644
--- a/arch/arm/boot/dts/stm32mp157c-ev1.dts
+++ b/arch/arm/boot/dts/stm32mp157c-ev1.dts
@@ -103,6 +103,13 @@
};
};
+&usbotg_hs {
+ dr_mode = "peripheral";
+ phys = <&usbphyc_port1 0>;
+ phy-names = "usb2-phy";
+ status = "okay";
+};
+
&usbphyc {
status = "okay";
};
--
2.7.4
^ permalink raw reply related
* [PATCH 1/2] ARM: dts: stm32: add USB OTG HS support for stm32mp157c SoC
From: Amelie Delaunay @ 2018-05-17 15:47 UTC (permalink / raw)
To: Maxime Coquelin, Alexandre Torgue, Rob Herring, Mark Rutland
Cc: linux-arm-kernel, devicetree, linux-kernel, Amelie Delaunay
In-Reply-To: <1526572052-17155-1-git-send-email-amelie.delaunay@st.com>
This patch adds support for USB OTG HS on STM32MP157C SoC.
USB OTG HS controller is based on DWC2 controller.
Signed-off-by: Amelie Delaunay <amelie.delaunay@st.com>
---
arch/arm/boot/dts/stm32mp157c.dtsi | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
index b66f673..6c3815f 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -600,6 +600,21 @@
clocks = <&rcc DMAMUX>;
};
+ usbotg_hs: usb-otg@49000000 {
+ compatible = "snps,dwc2";
+ reg = <0x49000000 0x10000>;
+ clocks = <&rcc USBO_K>;
+ clock-names = "otg";
+ resets = <&rcc USBO_R>;
+ reset-names = "dwc2";
+ interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
+ g-rx-fifo-size = <256>;
+ g-np-tx-fifo-size = <32>;
+ g-tx-fifo-size = <128 128 64 64 64 64 32 32>;
+ dr_mode = "otg";
+ status = "disabled";
+ };
+
rcc: rcc@50000000 {
compatible = "st,stm32mp1-rcc", "syscon";
reg = <0x50000000 0x1000>;
--
2.7.4
^ permalink raw reply related
* [PATCH 0/2] Add USB OTG HS support on stm32mp157c
From: Amelie Delaunay @ 2018-05-17 15:47 UTC (permalink / raw)
To: Maxime Coquelin, Alexandre Torgue, Rob Herring, Mark Rutland
Cc: linux-arm-kernel, devicetree, linux-kernel, Amelie Delaunay
This patchset adds support for USB OTG HS on stm32mp157c SoC.
It also enables USB OTG HS on stm32mp157c-ev1 board in Peripheral mode.
Amelie Delaunay (2):
ARM: dts: stm32: add USB OTG HS support for stm32mp157c SoC
ARM: dts: stm32: enable USB OTG HS on stm32mp157c-ev1
arch/arm/boot/dts/stm32mp157c-ev1.dts | 7 +++++++
arch/arm/boot/dts/stm32mp157c.dtsi | 15 +++++++++++++++
2 files changed, 22 insertions(+)
--
2.7.4
^ permalink raw reply
* Re: [PATCH v2 07/40] iommu: Add a page fault handler
From: Jonathan Cameron @ 2018-05-17 15:25 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: kvm-u79uwXL29TY76Z2rM5mHXA, linux-pci-u79uwXL29TY76Z2rM5mHXA,
xuzaibo-hv44wF8Li93QT0dZR+AlfA, will.deacon-5wv7dgnIgG8,
okaya-sgV2jX0FEOL9JmXXK+q4OQ, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
ashok.raj-ral2JQCrhuEAvxtiuMwx3w, bharatku-gjFFaj9aHVfQT0dZR+AlfA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA, rfranz-YGCgFSpz5w/QT0dZR+AlfA,
devicetree-u79uwXL29TY76Z2rM5mHXA, rgummal-gjFFaj9aHVfQT0dZR+AlfA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
ilias.apalodimas-QSEj5FYQhm4dnm+yROfE0A,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
christian.koenig-5C7GfCeVMHo
In-Reply-To: <20180511190641.23008-8-jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org>
On Fri, 11 May 2018 20:06:08 +0100
Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org> wrote:
> Some systems allow devices to handle I/O Page Faults in the core mm. For
> example systems implementing the PCI PRI extension or Arm SMMU stall
> model. Infrastructure for reporting these recoverable page faults was
> recently added to the IOMMU core for SVA virtualisation. Add a page fault
> handler for host SVA.
>
> IOMMU driver can now instantiate several fault workqueues and link them to
> IOPF-capable devices. Drivers can choose between a single global
> workqueue, one per IOMMU device, one per low-level fault queue, one per
> domain, etc.
>
> When it receives a fault event, supposedly in an IRQ handler, the IOMMU
> driver reports the fault using iommu_report_device_fault(), which calls
> the registered handler. The page fault handler then calls the mm fault
> handler, and reports either success or failure with iommu_page_response().
> When the handler succeeded, the IOMMU retries the access.
>
> The iopf_param pointer could be embedded into iommu_fault_param. But
> putting iopf_param into the iommu_param structure allows us not to care
> about ordering between calls to iopf_queue_add_device() and
> iommu_register_device_fault_handler().
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org>
Hi Jean-Phillipe,
One question below on how we would end up with partial faults left when
doing the queue remove. Code looks fine, but I'm not seeing how that
would happen without buggy hardware... + we presumably have to rely on
the hardware timing out on that request or it's dead anyway...
Jonathan
>
> ---
> v1->v2: multiple queues registered by IOMMU driver
> ---
> drivers/iommu/Kconfig | 4 +
> drivers/iommu/Makefile | 1 +
> drivers/iommu/io-pgfault.c | 363 +++++++++++++++++++++++++++++++++++++
> include/linux/iommu.h | 58 ++++++
> 4 files changed, 426 insertions(+)
> create mode 100644 drivers/iommu/io-pgfault.c
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 38434899e283..09f13a7c4b60 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -79,6 +79,10 @@ config IOMMU_SVA
> select IOMMU_API
> select MMU_NOTIFIER
>
> +config IOMMU_PAGE_FAULT
> + bool
> + select IOMMU_API
> +
> config FSL_PAMU
> bool "Freescale IOMMU support"
> depends on PCI
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 1dbcc89ebe4c..4b744e399a1b 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -4,6 +4,7 @@ obj-$(CONFIG_IOMMU_API) += iommu-traces.o
> obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
> obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
> obj-$(CONFIG_IOMMU_SVA) += iommu-sva.o
> +obj-$(CONFIG_IOMMU_PAGE_FAULT) += io-pgfault.o
> obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
> obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
> obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
> diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
> new file mode 100644
> index 000000000000..321c00dd3a3d
> --- /dev/null
> +++ b/drivers/iommu/io-pgfault.c
> @@ -0,0 +1,363 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Handle device page faults
> + *
> + * Copyright (C) 2018 ARM Ltd.
> + */
> +
> +#include <linux/iommu.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +#include <linux/workqueue.h>
> +
> +/**
> + * struct iopf_queue - IO Page Fault queue
> + * @wq: the fault workqueue
> + * @flush: low-level flush callback
> + * @flush_arg: flush() argument
> + * @refs: references to this structure taken by producers
> + */
> +struct iopf_queue {
> + struct workqueue_struct *wq;
> + iopf_queue_flush_t flush;
> + void *flush_arg;
> + refcount_t refs;
> +};
> +
> +/**
> + * struct iopf_device_param - IO Page Fault data attached to a device
> + * @queue: IOPF queue
> + * @partial: faults that are part of a Page Request Group for which the last
> + * request hasn't been submitted yet.
> + */
> +struct iopf_device_param {
> + struct iopf_queue *queue;
> + struct list_head partial;
> +};
> +
> +struct iopf_context {
> + struct device *dev;
> + struct iommu_fault_event evt;
> + struct list_head head;
> +};
> +
> +struct iopf_group {
> + struct iopf_context last_fault;
> + struct list_head faults;
> + struct work_struct work;
> +};
> +
> +static int iopf_complete(struct device *dev, struct iommu_fault_event *evt,
> + enum page_response_code status)
> +{
> + struct page_response_msg resp = {
> + .addr = evt->addr,
> + .pasid = evt->pasid,
> + .pasid_present = evt->pasid_valid,
> + .page_req_group_id = evt->page_req_group_id,
> + .private_data = evt->iommu_private,
> + .resp_code = status,
> + };
> +
> + return iommu_page_response(dev, &resp);
> +}
> +
> +static enum page_response_code
> +iopf_handle_single(struct iopf_context *fault)
> +{
> + /* TODO */
> + return -ENODEV;
> +}
> +
> +static void iopf_handle_group(struct work_struct *work)
> +{
> + struct iopf_group *group;
> + struct iopf_context *fault, *next;
> + enum page_response_code status = IOMMU_PAGE_RESP_SUCCESS;
> +
> + group = container_of(work, struct iopf_group, work);
> +
> + list_for_each_entry_safe(fault, next, &group->faults, head) {
> + struct iommu_fault_event *evt = &fault->evt;
> + /*
> + * Errors are sticky: don't handle subsequent faults in the
> + * group if there is an error.
> + */
> + if (status == IOMMU_PAGE_RESP_SUCCESS)
> + status = iopf_handle_single(fault);
> +
> + if (!evt->last_req)
> + kfree(fault);
> + }
> +
> + iopf_complete(group->last_fault.dev, &group->last_fault.evt, status);
> + kfree(group);
> +}
> +
> +/**
> + * iommu_queue_iopf - IO Page Fault handler
> + * @evt: fault event
> + * @cookie: struct device, passed to iommu_register_device_fault_handler.
> + *
> + * Add a fault to the device workqueue, to be handled by mm.
> + */
> +int iommu_queue_iopf(struct iommu_fault_event *evt, void *cookie)
> +{
> + struct iopf_group *group;
> + struct iopf_context *fault, *next;
> + struct iopf_device_param *iopf_param;
> +
> + struct device *dev = cookie;
> + struct iommu_param *param = dev->iommu_param;
> +
> + if (WARN_ON(!mutex_is_locked(¶m->lock)))
> + return -EINVAL;
> +
> + if (evt->type != IOMMU_FAULT_PAGE_REQ)
> + /* Not a recoverable page fault */
> + return IOMMU_PAGE_RESP_CONTINUE;
> +
> + /*
> + * As long as we're holding param->lock, the queue can't be unlinked
> + * from the device and therefore cannot disappear.
> + */
> + iopf_param = param->iopf_param;
> + if (!iopf_param)
> + return -ENODEV;
> +
> + if (!evt->last_req) {
> + fault = kzalloc(sizeof(*fault), GFP_KERNEL);
> + if (!fault)
> + return -ENOMEM;
> +
> + fault->evt = *evt;
> + fault->dev = dev;
> +
> + /* Non-last request of a group. Postpone until the last one */
> + list_add(&fault->head, &iopf_param->partial);
> +
> + return IOMMU_PAGE_RESP_HANDLED;
> + }
> +
> + group = kzalloc(sizeof(*group), GFP_KERNEL);
> + if (!group)
> + return -ENOMEM;
> +
> + group->last_fault.evt = *evt;
> + group->last_fault.dev = dev;
> + INIT_LIST_HEAD(&group->faults);
> + list_add(&group->last_fault.head, &group->faults);
> + INIT_WORK(&group->work, iopf_handle_group);
> +
> + /* See if we have partial faults for this group */
> + list_for_each_entry_safe(fault, next, &iopf_param->partial, head) {
> + if (fault->evt.page_req_group_id == evt->page_req_group_id)
> + /* Insert *before* the last fault */
> + list_move(&fault->head, &group->faults);
> + }
> +
> + queue_work(iopf_param->queue->wq, &group->work);
> +
> + /* Postpone the fault completion */
> + return IOMMU_PAGE_RESP_HANDLED;
> +}
> +EXPORT_SYMBOL_GPL(iommu_queue_iopf);
> +
> +/**
> + * iopf_queue_flush_dev - Ensure that all queued faults have been processed
> + * @dev: the endpoint whose faults need to be flushed.
> + *
> + * Users must call this function when releasing a PASID, to ensure that all
> + * pending faults for this PASID have been handled, and won't hit the address
> + * space of the next process that uses this PASID.
> + *
> + * Return 0 on success.
> + */
> +int iopf_queue_flush_dev(struct device *dev)
> +{
> + int ret = 0;
> + struct iopf_queue *queue;
> + struct iommu_param *param = dev->iommu_param;
> +
> + if (!param)
> + return -ENODEV;
> +
> + /*
> + * It is incredibly easy to find ourselves in a deadlock situation if
> + * we're not careful, because we're taking the opposite path as
> + * iommu_queue_iopf:
> + *
> + * iopf_queue_flush_dev() | PRI queue handler
> + * lock(mutex) | iommu_queue_iopf()
> + * queue->flush() | lock(mutex)
> + * wait PRI queue empty |
> + *
> + * So we can't hold the device param lock while flushing. We don't have
> + * to, because the queue or the device won't disappear until all flush
> + * are finished.
> + */
> + mutex_lock(¶m->lock);
> + if (param->iopf_param) {
> + queue = param->iopf_param->queue;
> + } else {
> + ret = -ENODEV;
> + }
> + mutex_unlock(¶m->lock);
> + if (ret)
> + return ret;
> +
> + queue->flush(queue->flush_arg, dev);
> +
> + /*
> + * No need to clear the partial list. All PRGs containing the PASID that
> + * needs to be decommissioned are whole (the device driver made sure of
> + * it before this function was called). They have been submitted to the
> + * queue by the above flush().
> + */
> + flush_workqueue(queue->wq);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(iopf_queue_flush_dev);
> +
> +/**
> + * iopf_queue_add_device - Add producer to the fault queue
> + * @queue: IOPF queue
> + * @dev: device to add
> + */
> +int iopf_queue_add_device(struct iopf_queue *queue, struct device *dev)
> +{
> + int ret = -EINVAL;
> + struct iopf_device_param *iopf_param;
> + struct iommu_param *param = dev->iommu_param;
> +
> + if (!param)
> + return -ENODEV;
> +
> + iopf_param = kzalloc(sizeof(*iopf_param), GFP_KERNEL);
> + if (!iopf_param)
> + return -ENOMEM;
> +
> + INIT_LIST_HEAD(&iopf_param->partial);
> + iopf_param->queue = queue;
> +
> + mutex_lock(¶m->lock);
> + if (!param->iopf_param) {
> + refcount_inc(&queue->refs);
> + param->iopf_param = iopf_param;
> + ret = 0;
> + }
> + mutex_unlock(¶m->lock);
> +
> + if (ret)
> + kfree(iopf_param);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iopf_queue_add_device);
> +
> +/**
> + * iopf_queue_remove_device - Remove producer from fault queue
> + * @dev: device to remove
> + *
> + * Caller makes sure that no more fault is reported for this device, and no more
> + * flush is scheduled for this device.
> + *
> + * Note: safe to call unconditionally on a cleanup path, even if the device
> + * isn't registered to any IOPF queue.
> + *
> + * Return 0 if the device was attached to the IOPF queue
> + */
> +int iopf_queue_remove_device(struct device *dev)
> +{
> + struct iopf_context *fault, *next;
> + struct iopf_device_param *iopf_param;
> + struct iommu_param *param = dev->iommu_param;
> +
> + if (!param)
> + return -EINVAL;
> +
> + mutex_lock(¶m->lock);
> + iopf_param = param->iopf_param;
> + if (iopf_param) {
> + refcount_dec(&iopf_param->queue->refs);
> + param->iopf_param = NULL;
> + }
> + mutex_unlock(¶m->lock);
> + if (!iopf_param)
> + return -EINVAL;
> +
> + list_for_each_entry_safe(fault, next, &iopf_param->partial, head)
> + kfree(fault);
> +
Why would we end up here with partials still in the list? Buggy hardware?
> + /*
> + * No more flush is scheduled, and the caller removed all bonds from
> + * this device. unbind() waited until any concurrent mm_exit() finished,
> + * therefore there is no flush() running anymore and we can free the
> + * param.
> + */
> + kfree(iopf_param);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(iopf_queue_remove_device);
> +
> +/**
> + * iopf_queue_alloc - Allocate and initialize a fault queue
> + * @name: a unique string identifying the queue (for workqueue)
> + * @flush: a callback that flushes the low-level queue
> + * @cookie: driver-private data passed to the flush callback
> + *
> + * The callback is called before the workqueue is flushed. The IOMMU driver must
> + * commit all faults that are pending in its low-level queues at the time of the
> + * call, into the IOPF queue (with iommu_report_device_fault). The callback
> + * takes a device pointer as argument, hinting what endpoint is causing the
> + * flush. When the device is NULL, all faults should be committed.
> + */
> +struct iopf_queue *
> +iopf_queue_alloc(const char *name, iopf_queue_flush_t flush, void *cookie)
> +{
> + struct iopf_queue *queue;
> +
> + queue = kzalloc(sizeof(*queue), GFP_KERNEL);
> + if (!queue)
> + return NULL;
> +
> + /*
> + * The WQ is unordered because the low-level handler enqueues faults by
> + * group. PRI requests within a group have to be ordered, but once
> + * that's dealt with, the high-level function can handle groups out of
> + * order.
> + */
> + queue->wq = alloc_workqueue("iopf_queue/%s", WQ_UNBOUND, 0, name);
> + if (!queue->wq) {
> + kfree(queue);
> + return NULL;
> + }
> +
> + queue->flush = flush;
> + queue->flush_arg = cookie;
> + refcount_set(&queue->refs, 1);
> +
> + return queue;
> +}
> +EXPORT_SYMBOL_GPL(iopf_queue_alloc);
> +
> +/**
> + * iopf_queue_free - Free IOPF queue
> + * @queue: queue to free
> + *
> + * Counterpart to iopf_queue_alloc(). Caller must make sure that all producers
> + * have been removed.
> + */
> +void iopf_queue_free(struct iopf_queue *queue)
> +{
> +
Nitpick : Blank line here doesn't add anything.
> + /* Caller should have removed all producers first */
> + if (WARN_ON(!refcount_dec_and_test(&queue->refs)))
> + return;
> +
> + destroy_workqueue(queue->wq);
> + kfree(queue);
> +}
> +EXPORT_SYMBOL_GPL(iopf_queue_free);
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index faf3390ce89d..fad3a60e1c14 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -461,11 +461,20 @@ struct iommu_fault_param {
> void *data;
> };
>
> +/**
> + * iopf_queue_flush_t - Flush low-level page fault queue
> + *
> + * Report all faults currently pending in the low-level page fault queue
> + */
> +struct iopf_queue;
> +typedef int (*iopf_queue_flush_t)(void *cookie, struct device *dev);
> +
> /**
> * struct iommu_param - collection of per-device IOMMU data
> *
> * @fault_param: IOMMU detected device fault reporting data
> * @sva_param: SVA parameters
> + * @iopf_param: I/O Page Fault queue and data
> *
> * TODO: migrate other per device data pointers under iommu_dev_data, e.g.
> * struct iommu_group *iommu_group;
> @@ -475,6 +484,7 @@ struct iommu_param {
> struct mutex lock;
> struct iommu_fault_param *fault_param;
> struct iommu_sva_param *sva_param;
> + struct iopf_device_param *iopf_param;
> };
>
> int iommu_device_register(struct iommu_device *iommu);
> @@ -874,6 +884,12 @@ static inline int iommu_report_device_fault(struct device *dev, struct iommu_fau
> return 0;
> }
>
> +static inline int iommu_page_response(struct device *dev,
> + struct page_response_msg *msg)
> +{
> + return -ENODEV;
> +}
> +
> static inline int iommu_group_id(struct iommu_group *group)
> {
> return -ENODEV;
> @@ -1038,4 +1054,46 @@ static inline struct mm_struct *iommu_sva_find(int pasid)
> }
> #endif /* CONFIG_IOMMU_SVA */
>
> +#ifdef CONFIG_IOMMU_PAGE_FAULT
> +extern int iommu_queue_iopf(struct iommu_fault_event *evt, void *cookie);
> +
> +extern int iopf_queue_add_device(struct iopf_queue *queue, struct device *dev);
> +extern int iopf_queue_remove_device(struct device *dev);
> +extern int iopf_queue_flush_dev(struct device *dev);
> +extern struct iopf_queue *
> +iopf_queue_alloc(const char *name, iopf_queue_flush_t flush, void *cookie);
> +extern void iopf_queue_free(struct iopf_queue *queue);
> +#else /* CONFIG_IOMMU_PAGE_FAULT */
> +static inline int iommu_queue_iopf(struct iommu_fault_event *evt, void *cookie)
> +{
> + return -ENODEV;
> +}
> +
> +static inline int iopf_queue_add_device(struct iopf_queue *queue,
> + struct device *dev)
> +{
> + return -ENODEV;
> +}
> +
> +static inline int iopf_queue_remove_device(struct device *dev)
> +{
> + return -ENODEV;
> +}
> +
> +static inline int iopf_queue_flush_dev(struct device *dev)
> +{
> + return -ENODEV;
> +}
> +
> +static inline struct iopf_queue *
> +iopf_queue_alloc(const char *name, iopf_queue_flush_t flush, void *cookie)
> +{
> + return NULL;
> +}
> +
> +static inline void iopf_queue_free(struct iopf_queue *queue)
> +{
> +}
> +#endif /* CONFIG_IOMMU_PAGE_FAULT */
> +
> #endif /* __LINUX_IOMMU_H */
^ permalink raw reply
* Re: [PATCH V1 2/5] backlight: qcom-wled: Add support for WLED4 peripheral
From: kgunda @ 2018-05-17 15:10 UTC (permalink / raw)
To: Rob Herring
Cc: Bjorn Andersson, Lee Jones, Daniel Thompson, Jingoo Han,
Jacek Anaszewski, Pavel Machek, Mark Rutland,
Bartlomiej Zolnierkiewicz, Linux LED Subsystem, devicetree,
linux-kernel, dri-devel, linux-fbdev, linux-arm-msm,
linux-arm-msm-owner
In-Reply-To: <CAL_JsqLQvSpB3CiS1sA5AXwbvC0xkd1HqEYsjY+=oripMXSwuA@mail.gmail.com>
On 2018-05-17 18:01, Rob Herring wrote:
> On Thu, May 17, 2018 at 4:47 AM, <kgunda@codeaurora.org> wrote:
>> On 2018-05-08 15:55, kgunda@codeaurora.org wrote:
>>>
>>> On 2018-05-07 21:50, Bjorn Andersson wrote:
>>>>
>>>> On Thu 03 May 02:57 PDT 2018, Kiran Gunda wrote:
>>>>
>>>>> WLED4 peripheral is present on some PMICs like pmi8998
>>>>> and pm660l. It has a different register map and also
>>>>> configurations are different. Add support for it.
>>>>>
>>>>
>>>> Several things are going on in this patch, it needs to be split to
>>>> not hide the functional changes from the structural/renames.
>>>>
>>> Ok. I will split it in the next series.
>>>>>
>>>>> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
>>>>> ---
>>>>> .../bindings/leds/backlight/qcom-wled.txt | 172 ++++-
>>>>> drivers/video/backlight/qcom-wled.c | 749
>>>>> +++++++++++++++------
>>>>> 2 files changed, 696 insertions(+), 225 deletions(-)
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>>>>> b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>>>>> index fb39e32..0ceffa1 100644
>>>>> ---
>>>>> a/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>>>>> +++
>>>>> b/Documentation/devicetree/bindings/leds/backlight/qcom-wled.txt
>>>>> @@ -1,30 +1,129 @@
>>>>> Binding for Qualcomm Technologies, Inc. WLED driver
>>>>>
>>>>> -Required properties:
>>>>> -- compatible: should be "qcom,pm8941-wled"
>>>>> -- reg: slave address
>>>>> -
>>>>> -Optional properties:
>>>>> -- default-brightness: brightness value on boot, value from: 0-4095
>>>>> - default: 2048
>>>>> -- label: The name of the backlight device
>>>>> -- qcom,cs-out: bool; enable current sink output
>>>>> -- qcom,cabc: bool; enable content adaptive backlight control
>>>>> -- qcom,ext-gen: bool; use externally generated modulator signal to
>>>>> dim
>>>>> -- qcom,current-limit: mA; per-string current limit; value from 0
>>>>> to 25
>>>>> - default: 20mA
>>>>> -- qcom,current-boost-limit: mA; boost current limit; one of:
>>>>> - 105, 385, 525, 805, 980, 1260, 1400, 1680
>>>>> - default: 805mA
>>>>> -- qcom,switching-freq: kHz; switching frequency; one of:
>>>>> - 600, 640, 685, 738, 800, 872, 960, 1066, 1200, 1371,
>>>>> - 1600, 1920, 2400, 3200, 4800, 9600,
>>>>> - default: 1600kHz
>>>>> -- qcom,ovp: V; Over-voltage protection limit; one of:
>>>>> - 27, 29, 32, 35
>>>>> - default: 29V
>>>>> -- qcom,num-strings: #; number of led strings attached; value from
>>>>> 1 to
>>>>> 3
>>>>> - default: 2
>>>>> +WLED (White Light Emitting Diode) driver is used for controlling
>>>>> display
>>>>> +backlight that is part of PMIC on Qualcomm Technologies, Inc.
>>>>> reference
>>>>> +platforms. The PMIC is connected to the host processor via SPMI
>>>>> bus.
>>>>> +
>>>>> +- compatible
>>>>> + Usage: required
>>>>> + Value type: <string>
>>>>> + Definition: should be "qcom,pm8941-wled" or
>>>>> "qcom,pmi8998-wled".
>>>>> + or "qcom,pm660l-wled".
>>>>
>>>>
>>>> Better written as
>>>>
>>>> should be one of:
>>>> X
>>>> Y
>>>> Z
>>>>
>>> Will do it in the next series.
>>>>>
>>>>> +
>>>>> +- reg
>>>>> + Usage: required
>>>>> + Value type: <prop encoded array>
>>>>> + Definition: Base address of the WLED modules.
>>>>> +
>>>>> +- interrupts
>>>>> + Usage: optional
>>>>> + Value type: <prop encoded array>
>>>>> + Definition: Interrupts associated with WLED. Interrupts
>>>>> can be
>>>>> + specified as per the encoding listed under
>>>>> + Documentation/devicetree/bindings/spmi/
>>>>> + qcom,spmi-pmic-arb.txt.
>>>>
>>>>
>>>> Better to describe that this should be the "short" and "ovp"
>>>> interrupts
>>>> in this property than in the interrupt-names.
>>>>
>>> Ok. I will do it in the next series.
>>>>>
>>>>> +
>>>>> +- interrupt-names
>>>>> + Usage: optional
>>>>> + Value type: <string>
>>>>> + Definition: Interrupt names associated with the
>>>>> interrupts.
>>>>> + Must be "short" and "ovp". The short circuit
>>>>> detection
>>>>> + is not supported for PM8941.
>>>>> +
>>>>> +- label
>>>>> + Usage: required
>>>>> + Value type: <string>
>>>>> + Definition: The name of the backlight device
>>>>> +
>>>>> +- default-brightness
>>>>> + Usage: optional
>>>>> + Value type: <u32>
>>>>> + Definition: brightness value on boot, value from: 0-4095
>>>>> + Default: 2048
>>>>> +
>>>>> +- qcom,current-limit
>>>>> + Usage: optional
>>>>> + Value type: <u32>
>>>>> + Definition: uA; per-string current limit
>>>>
>>>>
>>>> You can't change unit on an existing property, that breaks any
>>>> existing
>>>> dts using the qcom,pm8941-wled compatible.
>>>>
>>>
>>>>> + value:
>>>>> + For pm8941: from 0 to 25000 with 5000 ua step
>>>>> + Default 20000 uA
>>>>> + For pmi8998: from 0 to 30000 with 5000 ua
>>>>> step
>>>>> + Default 25000 uA.
>>>>
>>>>
>>>> These values could be described just as well in mA, so keep the
>>>> original
>>>> unit - in particular since the boot-limit is in mA...
>>>>
>>> Ok. Will keep the original as is in the next series.
>>
>> Here, I may have to go with the approach as in "qcom,ovp". Because for
>> pm8941
>> the current step is 1 mA (I have wrongly mentioned as 5000uA here) and
>> for
>> PMI8998
>> the current step is 2.5 mA. Hence, I will add another variable
>> "qcom,current-limit-ua"
>> just like "qcom,ovp-mv".
>
> Use unit suffixes defined in bindings/property-units.txt.
>
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks for pointing it ! hope I can use "qcom,current-limit-microamp"
and
"qcom,ovp-millivolt". I am asking this because i found only
"-microvolt".
"-millivolt" is not present in the bindings you pointed.
^ permalink raw reply
* Re: [PATCH v10 25/27] ARM: davinci: add device tree support to timer
From: David Lechner @ 2018-05-17 15:09 UTC (permalink / raw)
To: Sekhar Nori, linux-clk, devicetree, linux-arm-kernel
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Kevin Hilman, Bartosz Golaszewski, Adam Ford, linux-kernel
In-Reply-To: <8fa98514-7651-58cb-595e-6c7f713540b8@ti.com>
On 05/17/2018 09:35 AM, Sekhar Nori wrote:
> Hi David,
>
> On Wednesday 09 May 2018 10:56 PM, David Lechner wrote:
>> This adds device tree support to the davinci timer so that when clocks
>> are moved to device tree, the timer will still work.
>>
>> Signed-off-by: David Lechner <david@lechnology.com>
>> ---
>
>> +static int __init of_davinci_timer_init(struct device_node *np)
>> +{
>> + struct clk *clk;
>> +
>> + clk = of_clk_get(np, 0);
>> + if (IS_ERR(clk)) {
>> + struct of_phandle_args clkspec;
>> +
>> + /*
>> + * Fall back to using ref_clk if the actual clock is not
>> + * available. There will be problems later if the real clock
>> + * source is disabled.
>> + */
>> +
>> + pr_warn("%s: falling back to ref_clk\n", __func__);
>> +
>> + clkspec.np = of_find_node_by_name(NULL, "ref_clk");
>> + if (IS_ERR(clkspec.np)) {
>> + pr_err("%s: No clock available for timer!\n", __func__);
>> + return PTR_ERR(clkspec.np);
>> + }
>> + clk = of_clk_get_from_provider(&clkspec);
>> + of_node_put(clkspec.np);
>> + }
>
> Do we need this error path now?
>
> Thanks,
> Sekhar
>
No, not really.
^ permalink raw reply
* [PATCH] ARM64: dts: sun50i: a64: Add spi flash node for sopine
From: Emmanuel Vadot @ 2018-05-17 15:06 UTC (permalink / raw)
To: maxime.ripard, wens, robh+dt, mark.rutland, catalin.marinas,
will.deacon, linux-arm-kernel, devicetree, linux-kernel
Cc: Emmanuel Vadot
The Sopine and Pine64-LTS have a winbond w25q128 spi flash on spi0.
Add a node for it.
Signed-off-by: Emmanuel Vadot <manu@freebsd.org>
---
arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
index 43418bd881d8..3285aebc06d9 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine.dtsi
@@ -66,6 +66,18 @@
};
};
+&spi0 {
+ status = "okay";
+
+ flash@0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "w25q128", "jedec,spi-nor";
+ reg = <0>;
+ spi-max-frequency = <40000000>;
+ };
+};
+
#include "axp803.dtsi"
®_aldo2 {
--
2.17.0
^ permalink raw reply related
* Re: [PATCH v2 4/4] ARM: PWM: add allwinner sun8i pwm support.
From: Hao Zhang @ 2018-05-17 14:48 UTC (permalink / raw)
To: Maxime Ripard
Cc: Thierry Reding, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Mark Rutland,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, Chen-Yu Tsai, Claudiu Beznea,
linux-gpio-u79uwXL29TY76Z2rM5mHXA, open list,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM/Allwinner sunXi SoC support,
linux-pwm-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20180515111711.l2g4vgsal7yr6dbr@flea>
2018-05-15 19:17 GMT+08:00 Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>:
> Hi,
>
> On Mon, May 14, 2018 at 10:45:44PM +0800, Hao Zhang wrote:
>> 2018-02-26 17:00 GMT+08:00 Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>:
>> > Thanks for respinning this serie. It looks mostly good, but you still
>> > have a quite significant number of checkpatch (--strict) warnings that
>> > you should address.
>>
>> Thanks for reviews :) ,i'm sorry for that, it will be fixed next
>> time. and, besides, in what situation were the checkpatch warning
>> can be ignore？
>
> The only one that can be reasonably be ignored is the long line
> warning, and only if complying to the limit would make it less easy to
> understand.
>
>> >
>> > On Sun, Feb 25, 2018 at 09:53:08PM +0800, hao_zhang wrote:
>> >> +#define CAPTURE_IRQ_ENABLE_REG 0x0010
>> >> +#define CFIE(ch) BIT(ch << 1 + 1)
>> >> +#define CRIE(ch) BIT(ch << 1)
>> >
>> > You should also put your argument between parentheses here (and in all
>> > your other macros).
>>
>> Do you mean like this ?
>> #define CFIE(ch) BIT((ch) << 1 + 1)
>> #define CRIE(ch) BIT((ch) << 1)
>
> Yep, exactly. Otherwise, if you do something like CRIE(1 + 1), the
> result will be BIT(1 + 1 << 1), which will expand to 3, instead of 4.
>
> Also, CFIE looks a bit weird here, is it the offset that is
> incremented, or the value? You should probably have parentheses to
> make it explicit.
The vallue,
BIT(((ch) << 1) + 1) It seem not very nice...
uhmm...
In CAPTURE_IRQ_ENABLE_REG odd number is CFIE, even number is CRIE
each channel has one CFIE and CRIE.
we can also describe like this:
#define CFIE(ch) BIT((ch) * 2 + 1)
#define CRIE(ch) BIT((ch) * 2)
>
>> >
>> >> +static const u16 div_m_table[] = {
>> >> + 1,
>> >> + 2,
>> >> + 4,
>> >> + 8,
>> >> + 16,
>> >> + 32,
>> >> + 64,
>> >> + 128,
>> >> + 256
>> >> +};
>> >
>> > If this is just a power of two, you can use either the power of two /
>> > ilog2 to switch back and forth, instead of using that table.
>>
>> I think using table is more explicit and extended...
>
> If you didn't have a simple mapping between the register values and
> the divider value, then yeah, sure. But it's not the case here.
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH v3] ASoC: ssm2305: Add amplifier driver
From: Lars-Peter Clausen @ 2018-05-17 14:44 UTC (permalink / raw)
To: Marco Felsch, lgirdwood, broonie, robh+dt, mark.rutland
Cc: devicetree, alsa-devel, kernel
In-Reply-To: <20180517135518.29345-1-m.felsch@pengutronix.de>
On 05/17/2018 03:55 PM, Marco Felsch wrote:
> The ssm2305 is a simple Class-D audio amplifier. A application can
> turn on/off the device by a gpio. It's also possible to hardwire the
> shutdown pin.
>
> Tested on a i.MX6 based custom board.
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>
Thanks.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox