Re: [PATCH] wilc1000: write value to WILC_INTR2_ENABLE register
Hi,
On Thu, Feb 25, 2021 at 09:09:30AM +0200, Kalle Valo wrote:
> writes:
>
> > On 24/02/21 10:13 pm, Kalle Valo wrote:
> >> EXTERNAL EMAIL: Do not click links or open attachments unless you
> >> know the content is safe
> >>
> >> Marcus Folkesson writes:
> >>
> >>> Write the value instead of reading it twice.
> >>>
> >>> Fixes: 5e63a598441a ("staging: wilc1000: added 'wilc_' prefix for
> >>> function in wilc_sdio.c file")
> >>>
> >>> Signed-off-by: Marcus Folkesson
> >>> ---
> >>> drivers/net/wireless/microchip/wilc1000/sdio.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c
> >>> b/drivers/net/wireless/microchip/wilc1000/sdio.c
> >>> index 351ff909ab1c..e14b9fc2c67a 100644
> >>> --- a/drivers/net/wireless/microchip/wilc1000/sdio.c
> >>> +++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
> >>> @@ -947,7 +947,7 @@ static int wilc_sdio_sync_ext(struct wilc *wilc, int
> >>> nint)
> >>> for (i = 0; (i < 3) && (nint > 0); i++, nint--)
> >>> reg |= BIT(i);
> >>>
> >>> - ret = wilc_sdio_read_reg(wilc, WILC_INTR2_ENABLE,
> >>> ®);
> >>> + ret = wilc_sdio_write_reg(wilc, WILC_INTR2_ENABLE,
> >>> reg);
> >>
> >> To me it looks like the bug existed before commit 5e63a598441a:
> >
> >
> > Yes, you are correct. The bug existed from commit c5c77ba18ea6:
> >
> > https://git.kernel.org/linus/c5c77ba18ea6
>
> So the fixes tag should be:
>
> Fixes: c5c77ba18ea6 ("staging: wilc1000: Add SDIO/SPI 802.11 driver")
You are right.
>
> I can change that during commit, ok?
Please do, thanks!
>
> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Best regards
Marcus Folkesson
signature.asc
Description: PGP signature
[PATCH] wilc1000: write value to WILC_INTR2_ENABLE register
Write the value instead of reading it twice.
Fixes: 5e63a598441a ("staging: wilc1000: added 'wilc_' prefix for function in
wilc_sdio.c file")
Signed-off-by: Marcus Folkesson
---
drivers/net/wireless/microchip/wilc1000/sdio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/microchip/wilc1000/sdio.c
b/drivers/net/wireless/microchip/wilc1000/sdio.c
index 351ff909ab1c..e14b9fc2c67a 100644
--- a/drivers/net/wireless/microchip/wilc1000/sdio.c
+++ b/drivers/net/wireless/microchip/wilc1000/sdio.c
@@ -947,7 +947,7 @@ static int wilc_sdio_sync_ext(struct wilc *wilc, int nint)
for (i = 0; (i < 3) && (nint > 0); i++, nint--)
reg |= BIT(i);
- ret = wilc_sdio_read_reg(wilc, WILC_INTR2_ENABLE, ®);
+ ret = wilc_sdio_write_reg(wilc, WILC_INTR2_ENABLE, reg);
if (ret) {
dev_err(&func->dev,
"Failed write reg (%08x)...\n",
--
2.30.0
Re: [PATCH 1/2] watchdog: sama5d4: fix timeout-sec usage
On Fri, Sep 14, 2018 at 12:13:38PM +0200, Romain Izard wrote:
> When using watchdog_init_timeout to update the default timeout value,
> an error means that there is no "timeout-sec" in the relevant device
> tree node.
>
> This should not prevent binding of the driver to the device.
>
> Fixes: 976932e40036 ("watchdog: sama5d4: make use of timeout-secs provided in
> devicetree")
> Signed-off-by: Romain Izard
Reviewed-by: Marcus Folkesson
[PATCH] iio: dac: ti-dac5571: make vref regulator optional
The `vref` regulator is declared as optional in the device-tree binding,
but the driver does require it.
Go for the device-tree binding and make the `vref` regulator optional.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/ti-dac5571.c | 30 ++
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/dac/ti-dac5571.c b/drivers/iio/dac/ti-dac5571.c
index f6dcd8bce2b0..bf21cc312096 100644
--- a/drivers/iio/dac/ti-dac5571.c
+++ b/drivers/iio/dac/ti-dac5571.c
@@ -251,6 +251,9 @@ static int dac5571_read_raw(struct iio_dev *indio_dev,
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
+ if (!data->vref)
+ return -EOPNOTSUPP;
+
ret = regulator_get_voltage(data->vref);
if (ret < 0)
return ret;
@@ -335,13 +338,21 @@ static int dac5571_probe(struct i2c_client *client,
indio_dev->num_channels = spec->num_channels;
data->spec = spec;
- data->vref = devm_regulator_get(dev, "vref");
- if (IS_ERR(data->vref))
- return PTR_ERR(data->vref);
+ data->vref = devm_regulator_get_optional(dev, "vref");
+ if (IS_ERR(data->vref)) {
+ if (PTR_ERR(data->vref) == -ENODEV) {
+ data->vref = NULL;
+ } else {
+ dev_err(dev, "failed to get regulator (%ld)\n",
+ PTR_ERR(data->vref));
+ return PTR_ERR(data->vref);
+ }
- ret = regulator_enable(data->vref);
- if (ret < 0)
- return ret;
+ } else {
+ ret = regulator_enable(data->vref);
+ if (ret)
+ return ret;
+ }
mutex_init(&data->lock);
@@ -373,7 +384,9 @@ static int dac5571_probe(struct i2c_client *client,
return 0;
err:
- regulator_disable(data->vref);
+ if (data->vref)
+ regulator_disable(data->vref);
+
return ret;
}
@@ -383,7 +396,8 @@ static int dac5571_remove(struct i2c_client *i2c)
struct dac5571_data *data = iio_priv(indio_dev);
iio_device_unregister(indio_dev);
- regulator_disable(data->vref);
+ if (data->vref)
+ regulator_disable(data->vref);
return 0;
}
--
2.18.0
[PATCH] iio: dac: ti-dac5571: provide of_match_table to driver
Use the created list of of_device_id's as a match table.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/ti-dac5571.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iio/dac/ti-dac5571.c b/drivers/iio/dac/ti-dac5571.c
index e39d1e901353..f6dcd8bce2b0 100644
--- a/drivers/iio/dac/ti-dac5571.c
+++ b/drivers/iio/dac/ti-dac5571.c
@@ -421,6 +421,7 @@ MODULE_DEVICE_TABLE(i2c, dac5571_id);
static struct i2c_driver dac5571_driver = {
.driver = {
.name = "ti-dac5571",
+ .of_match_table = of_match_ptr(dac5571_of_id),
},
.probe= dac5571_probe,
.remove = dac5571_remove,
--
2.18.0
[PATCH] iio: dac: mcp4922: fix error handling in mcp4922_write_raw
Do not try to write negative values and make sure that the write goes well.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/mcp4922.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/dac/mcp4922.c b/drivers/iio/dac/mcp4922.c
index bf9aa3fc0534..b5190d1dae8e 100644
--- a/drivers/iio/dac/mcp4922.c
+++ b/drivers/iio/dac/mcp4922.c
@@ -94,17 +94,22 @@ static int mcp4922_write_raw(struct iio_dev *indio_dev,
long mask)
{
struct mcp4922_state *state = iio_priv(indio_dev);
+ int ret;
if (val2 != 0)
return -EINVAL;
switch (mask) {
case IIO_CHAN_INFO_RAW:
- if (val > GENMASK(chan->scan_type.realbits-1, 0))
+ if (val < 0 || val > GENMASK(chan->scan_type.realbits - 1, 0))
return -EINVAL;
val <<= chan->scan_type.shift;
- state->value[chan->channel] = val;
- return mcp4922_spi_write(state, chan->channel, val);
+
+ ret = mcp4922_spi_write(state, chan->channel, val);
+ if (!ret)
+ state->value[chan->channel] = val;
+ return ret;
+
default:
return -EINVAL;
}
--
2.18.0
Re: [PATCH v2 2/3] dt-bindings: iio: dac: add bindings for ltc1660
On Tue, Aug 21, 2018 at 09:31:25PM +0200, Marcus Folkesson wrote:
> LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter (DAC)
> with eight individual channels.
>
> Signed-off-by: Marcus Folkesson
Rob, sorry I missed your tag.
Reviewed-by: Rob Herring
> ---
>
> Notes:
> v2:
> - rename file, ltc166x -> ltc1660
>
> .../devicetree/bindings/iio/dac/ltc1660.txt | 21
> +
> 1 file changed, 21 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/dac/ltc1660.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
> b/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
> new file mode 100644
> index ..c5b5f22d6c64
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
> @@ -0,0 +1,21 @@
> +* Linear Technology Micropower octal 8-Bit and 10-Bit DACs
> +
> +Required properties:
> + - compatible: Must be one of the following:
> + "lltc,ltc1660"
> + "lltc,ltc1665"
> + - reg: SPI chip select number for the device
> + - vref-supply: Phandle to the voltage reference supply
> +
> +Recommended properties:
> + - spi-max-frequency: Definition as per
> + Documentation/devicetree/bindings/spi/spi-bus.txt.
> + Max frequency for this chip is 5 MHz.
> +
> +Example:
> +dac@0 {
> + compatible = "lltc,ltc1660";
> + reg = <0>;
> + spi-max-frequency = <500>;
> + vref-supply = <&vref_reg>;
> +};
> --
> 2.11.0.rc2
>
[PATCH v2 3/3] MAINTAINERS: add entry for ltc1660 DAC driver
Add entry for ltc1660 DAC driver and add myself as maintainer of this driver. Signed-off-by: Marcus Folkesson --- Notes: v2: - rename enumerated files, ltc166x* -> ltc1660* MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9276da915d9d..3db1edaf68bd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8363,6 +8363,13 @@ L: linux-s...@vger.kernel.org S: Maintained F: drivers/scsi/sym53c8xx_2/ +LTC1660 DAC DRIVER +M: Marcus Folkesson +L: linux-...@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/iio/dac/ltc1660.txt +F: drivers/iio/dac/ltc1660.c + LTC4261 HARDWARE MONITOR DRIVER M: Guenter Roeck L: linux-hw...@vger.kernel.org -- 2.11.0.rc2
[PATCH v2 2/3] dt-bindings: iio: dac: add bindings for ltc1660
LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter (DAC)
with eight individual channels.
Signed-off-by: Marcus Folkesson
---
Notes:
v2:
- rename file, ltc166x -> ltc1660
.../devicetree/bindings/iio/dac/ltc1660.txt | 21 +
1 file changed, 21 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/dac/ltc1660.txt
diff --git a/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
b/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
new file mode 100644
index ..c5b5f22d6c64
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/ltc1660.txt
@@ -0,0 +1,21 @@
+* Linear Technology Micropower octal 8-Bit and 10-Bit DACs
+
+Required properties:
+ - compatible: Must be one of the following:
+ "lltc,ltc1660"
+ "lltc,ltc1665"
+ - reg: SPI chip select number for the device
+ - vref-supply: Phandle to the voltage reference supply
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 5 MHz.
+
+Example:
+dac@0 {
+ compatible = "lltc,ltc1660";
+ reg = <0>;
+ spi-max-frequency = <500>;
+ vref-supply = <&vref_reg>;
+};
--
2.11.0.rc2
[PATCH v2 1/3] iio: dac: add support for ltc1660
LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter
(DAC) with eight individual channels.
Signed-off-by: Marcus Folkesson
---
Notes:
v2:
- rename all instances of ltc166x to ltc1660
- read regulator value "in-place" instead of cache voltage
- fix error handling in ltc1660_write_raw
drivers/iio/dac/Kconfig | 10 ++
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/ltc1660.c | 250 ++
3 files changed, 261 insertions(+)
create mode 100644 drivers/iio/dac/ltc1660.c
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 76db0768e454..cbee80bd111e 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -120,6 +120,16 @@ config AD5624R_SPI
Say yes here to build support for Analog Devices AD5624R, AD5644R and
AD5664R converters (DAC). This driver uses the common SPI interface.
+config LTC1660
+ tristate "Linear Technology LTC1660/LTC1665 DAC SPI driver"
+ depends on SPI
+ help
+ Say yes here to build support for Linear Technology
+ LTC1660 and LTC1665 Digital to Analog Converters.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ltc1660.
+
config LTC2632
tristate "Linear Technology LTC2632-12/10/8 DAC spi driver"
depends on SPI
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 81e710ed7491..e446eeb09c85 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_CIO_DAC) += cio-dac.o
obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
obj-$(CONFIG_DS4424) += ds4424.o
obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
+obj-$(CONFIG_LTC1660) += ltc1660.o
obj-$(CONFIG_LTC2632) += ltc2632.o
obj-$(CONFIG_M62332) += m62332.o
obj-$(CONFIG_MAX517) += max517.o
diff --git a/drivers/iio/dac/ltc1660.c b/drivers/iio/dac/ltc1660.c
new file mode 100644
index ..10866838c72a
--- /dev/null
+++ b/drivers/iio/dac/ltc1660.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Linear Technology LTC1665/LTC1660, 8 channels DAC
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define LTC1660_REG_WAKE 0x0
+#define LTC1660_REG_DAC_A 0x1
+#define LTC1660_REG_DAC_B 0x2
+#define LTC1660_REG_DAC_C 0x3
+#define LTC1660_REG_DAC_D 0x4
+#define LTC1660_REG_DAC_E 0x5
+#define LTC1660_REG_DAC_F 0x6
+#define LTC1660_REG_DAC_G 0x7
+#define LTC1660_REG_DAC_H 0x8
+#define LTC1660_REG_SLEEP 0xe
+
+#define LTC1660_NUM_CHANNELS 8
+
+static const struct regmap_config ltc1660_regmap_config = {
+ .reg_bits = 4,
+ .val_bits = 12,
+};
+
+enum ltc1660_supported_device_ids {
+ ID_LTC1660,
+ ID_LTC1665,
+};
+
+struct ltc1660_priv {
+ struct spi_device *spi;
+ struct regmap *regmap;
+ struct regulator *vref_reg;
+ unsigned int value[LTC1660_NUM_CHANNELS];
+ unsigned int vref_mv;
+};
+
+static int ltc1660_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val,
+ int *val2,
+ long mask)
+{
+ struct ltc1660_priv *priv = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ *val = priv->value[chan->channel];
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ *val = regulator_get_voltage(priv->vref_reg);
+ if (*val < 0) {
+ dev_err(&priv->spi->dev, "failed to read vref
regulator: %d\n",
+ *val);
+ return *val;
+ }
+
+ /* Convert to mV */
+ *val /= 1000;
+ *val2 = chan->scan_type.realbits;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ltc1660_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long mask)
+{
+ struct ltc1660_priv *priv = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ if (val2 != 0)
+ return -EINVAL;
+
+ if (val < 0 || val > GENMASK(chan->scan_type.realbits - 1, 0))
+ return -EINVAL;
+
+ ret = regmap_write(priv->regmap, chan->channel,
+ (val << chan->scan_type.shift));
+ if (!ret)
+ priv->value[chan->channel] = val;
+
+ return ret;
+ default:
+ return -EINVAL;
+ }
+}
+
+#define LTC1660_CHAN(chan, bits) {
Re: [PATCH v6 1/2] iio: light: Add support for vishay vcnl4035
dev_err(&data->client->dev, "iio triggered buffer setup
> failed\n");
> + return ret;
> + }
> +
> + /* IRQ to trigger mapping */
> + ret = devm_request_threaded_irq(&data->client->dev, data->client->irq,
> + NULL, vcnl4035_drdy_irq_thread,
> + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> + VCNL4035_IRQ_NAME, indio_dev);
> + if (ret < 0) {
> + dev_err(&data->client->dev, "request irq %d for trigger0
> failed\n",
> + data->client->irq);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int vcnl4035_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct vcnl4035_data *data;
> + struct iio_dev *indio_dev;
> + struct regmap *regmap;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + regmap = devm_regmap_init_i2c(client, &vcnl4035_regmap_config);
> + if (IS_ERR(regmap)) {
> + dev_err(&client->dev, "regmap_init failed!\n");
> + return PTR_ERR(regmap);
> + }
> +
> + data = iio_priv(indio_dev);
> + i2c_set_clientdata(client, indio_dev);
> + data->client = client;
> + data->regmap = regmap;
> +
> + indio_dev->dev.parent = &client->dev;
> + indio_dev->info = &vcnl4035_info;
> + indio_dev->name = VCNL4035_DRV_NAME;
> + indio_dev->channels = vcnl4035_channels;
> + indio_dev->num_channels = ARRAY_SIZE(vcnl4035_channels);
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + ret = vcnl4035_init(data);
> + if (ret < 0) {
> + dev_err(&client->dev, "vcnl4035 chip init failed\n");
> + return ret;
> + }
> +
> + if (client->irq > 0) {
> + ret = vcnl4035_probe_trigger(indio_dev);
> + if (ret < 0) {
> + dev_err(&client->dev, "vcnl4035 unable init trigger\n");
> + goto fail_poweroff;
> + }
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret)
> + goto fail_poweroff;
> +
> + ret = pm_runtime_set_active(&client->dev);
> + if (ret < 0)
> + goto fail_unregister;
> +
> + pm_runtime_enable(&client->dev);
> + pm_runtime_set_autosuspend_delay(&client->dev, VCNL4035_SLEEP_DELAY_MS);
> + pm_runtime_use_autosuspend(&client->dev);
> +
> + return 0;
> +
> +fail_unregister:
> + iio_device_unregister(indio_dev);
> +fail_poweroff:
> + vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_DISABLE);
> + return ret;
> +}
> +
> +static int vcnl4035_remove(struct i2c_client *client)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> + iio_device_unregister(indio_dev);
> +
> + pm_runtime_dont_use_autosuspend(&client->dev);
> + pm_runtime_disable(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> + pm_runtime_put_noidle(&client->dev);
> +
> + return vcnl4035_set_als_power_state(iio_priv(indio_dev),
> + VCNL4035_MODE_ALS_DISABLE);
> +}
> +
> +#ifdef CONFIG_PM
> +static int __maybe_unused vcnl4035_runtime_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> + struct vcnl4035_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + ret = vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_DISABLE);
> + regcache_mark_dirty(data->regmap);
> +
> + return ret;
> +}
> +
> +static int __maybe_unused vcnl4035_runtime_resume(struct device *dev)
> +{
> + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> + struct vcnl4035_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + regcache_sync(data->regmap);
> + ret = vcnl4035_set_als_power_state(data, VCNL4035_MODE_ALS_ENABLE);
> + if (ret < 0)
> + return ret;
> +
> + /* wait for 1 ALS integration cycle */
> + msleep(data->als_it_val * 100);
> +
> + return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops vcnl4035_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> + SET_RUNTIME_PM_OPS(vcnl4035_runtime_suspend,
> +vcnl4035_runtime_resume, NULL)
> +};
> +
> +static const struct of_device_id vcnl4035_of_match[] = {
> + { .compatible = "vishay,vcnl4035", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, vcnl4035_of_match);
> +
> +static struct i2c_driver vcnl4035_driver = {
> + .driver = {
> + .name = VCNL4035_DRV_NAME,
> + .pm = &vcnl4035_pm_ops,
> + .of_match_table = of_match_ptr(vcnl4035_of_match),
Please correct me if I'm wrong here.
Use of_match_ptr() and not provide an ACPI match table will not make
this driver work on ACPI systems.
The reason is that the ACPI match function will fall back on
of_match_table if no acpi_match_table is provided, and the
of_match_ptr() macro will simple evaluate to NULL if not CONFIG_OF
is set.
In other words, I think we should change this to just
.of_match_table = &vcnl4035_of_match,
> + },
> + .probe = vcnl4035_probe,
> + .remove = vcnl4035_remove,
> +};
> +
> +module_i2c_driver(vcnl4035_driver);
> +
> +MODULE_AUTHOR("Parthiban Nallathambi ");
> +MODULE_DESCRIPTION("VCNL4035 Ambient Light Sensor driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.14.4
>
Best regards
Marcus Folkesson
signature.asc
Description: PGP signature
Re: [PATCH v4 1/3] iio: adc: add support for mcp3911
On Sun, Aug 19, 2018 at 08:29:43PM +0100, Jonathan Cameron wrote: > On Sun, 19 Aug 2018 21:17:51 +0200 > Marcus Folkesson wrote: > > > On Sun, Aug 19, 2018 at 08:02:57PM +0100, Jonathan Cameron wrote: > > > On Wed, 8 Aug 2018 10:09:15 +0200 > > > Marcus Folkesson wrote: > > > > > > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > > > > > > > Co-Developed-by: Kent Gustavsson > > > checkpatch points out.. > > > Co-developed-by is the 'official' form of the tag. I'll fixup. > > > (first time I've seen one of these ;) > > > > Yep, I also noticed that checkpatch is not Co-developed-by-aware yet. > > I tried to fix it, but Perl is just such a terrible language that I > > considered it impossible. > > The version in mainline seems to be aware, it just wants > Co-developed-by not Co-Developed-by! > > (lower case d) I just noticed that the tag was renamed from Co-Developed-by to Co-developed-by in e474785a12b46230ecf9f3663166b0de1175bcdc. I must have been reading the documentation from an older tree... /Marcus signature.asc Description: PGP signature
Re: [PATCH v4 1/3] iio: adc: add support for mcp3911
On Sun, Aug 19, 2018 at 08:02:57PM +0100, Jonathan Cameron wrote: > On Wed, 8 Aug 2018 10:09:15 +0200 > Marcus Folkesson wrote: > > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > > > Co-Developed-by: Kent Gustavsson > checkpatch points out.. > Co-developed-by is the 'official' form of the tag. I'll fixup. > (first time I've seen one of these ;) Yep, I also noticed that checkpatch is not Co-developed-by-aware yet. I tried to fix it, but Perl is just such a terrible language that I considered it impossible. > > > Signed-off-by: Kent Gustavsson > > Signed-off-by: Marcus Folkesson > Applied to the togreg branch of iio.git and pushed out as testing for the > autobuilders to play with it. > > A few really trivial comments inline. Also some slightly odd alignment > I'll fix up that I haven't noted, but not every parameter second line > lines up with the opening brackets correctly. Thank you. > > Jonathan > > > --- > > > > Notes: > > v4: > > - remove defined(CONFIG_OF) and of_match_ptr() macro > > - do not check adc->clki as clock api is NULL-aware > > - add Kent as co-developer > > v3: > > - rename adc_clk to clki > > - add error handling/cleanup for clock > > v2: > > - cleanups and bugfixes (thanks Peter Meerwald-Stadler) > > - drop hardware gain > > - use the presence or lack of regulator to indicate if we go > > for internal or external voltage reference > > - do not store device node in private struct > > - drop support to set width in devicetree > > - use the presence or lack of clock to indicate if we go for > > internal or external clock > > > > drivers/iio/adc/Kconfig | 10 ++ > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/mcp3911.c | 361 > > ++ > > 3 files changed, 372 insertions(+) > > create mode 100644 drivers/iio/adc/mcp3911.c > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index 15606f237480..f9a41fa96fcc 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -501,6 +501,16 @@ config MCP3422 > > This driver can also be built as a module. If so, the module will be > > called mcp3422. > > > > +config MCP3911 > > + tristate "Microchip Technology MCP3911 driver" > > + depends on SPI > > + help > > + Say yes here to build support for Microchip Technology's MCP3911 > > + analog to digital converter. > > + > > + This driver can also be built as a module. If so, the module will be > > + called mcp3911. > > + > > config MEDIATEK_MT6577_AUXADC > > tristate "MediaTek AUXADC driver" > > depends on ARCH_MEDIATEK || COMPILE_TEST > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > index 28a9423997f3..3cfebfff7d26 100644 > > --- a/drivers/iio/adc/Makefile > > +++ b/drivers/iio/adc/Makefile > > @@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o > > obj-$(CONFIG_MAX9611) += max9611.o > > obj-$(CONFIG_MCP320X) += mcp320x.o > > obj-$(CONFIG_MCP3422) += mcp3422.o > > +obj-$(CONFIG_MCP3911) += mcp3911.o > > obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o > > obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o > > obj-$(CONFIG_MESON_SARADC) += meson_saradc.o > > diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c > > new file mode 100644 > > index ..540c8fbcce89 > > --- /dev/null > > +++ b/drivers/iio/adc/mcp3911.c > > @@ -0,0 +1,361 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Driver for Microchip MCP3911, Two-channel Analog Front End > > + * > > + * Copyright (C) 2018 Marcus Folkesson > > + * Copyright (C) 2018 Kent Gustavsson > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define MCP3911_REG_CHANNEL0 0x00 > > +#define MCP3911_REG_CHANNEL1 0x03 > > +#define MCP3911_REG_MOD0x06 > > +#define MCP3911_REG_PHASE 0x07 > > +#define MCP3911_REG_GAIN 0x09 > > + > > +#define MCP3911_REG_STATUSCOM 0x0a > > +#define
Re: [PATCH 1/3] iio: dac: add support for ltc166x
Hi Jonathan,
Thanks for your comments!
On Sun, Aug 19, 2018 at 05:38:50PM +0100, Jonathan Cameron wrote:
> On Sat, 11 Aug 2018 22:02:24 +0200
> Marcus Folkesson wrote:
>
> > LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter
> > (DAC) with eight individual channels.
> >
> > Signed-off-by: Marcus Folkesson
>
> So first rule we try to stick to that this breaks is never use wild cards
> in drivers. You probably wouldn't believe how often this has gone wrong with
> a manufacturer deciding to use other values that wild card covers...
Hah, I can imagine. I will switch do use ltc1660 all over the place.
>
> Please pick a part and name everything after that. Either one is fine.
>
> A few really minor things inline. Nice little driver.
>
> Thanks,
>
> Jonathan
>
>
> > ---
> > drivers/iio/dac/Kconfig | 10 ++
> > drivers/iio/dac/Makefile | 1 +
> > drivers/iio/dac/ltc166x.c | 244
> > ++
> > 3 files changed, 255 insertions(+)
> > create mode 100644 drivers/iio/dac/ltc166x.c
> >
> > diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> > index 76db0768e454..04cfa6bb9dc1 100644
> > --- a/drivers/iio/dac/Kconfig
> > +++ b/drivers/iio/dac/Kconfig
> > @@ -120,6 +120,16 @@ config AD5624R_SPI
> > Say yes here to build support for Analog Devices AD5624R, AD5644R and
> > AD5664R converters (DAC). This driver uses the common SPI interface.
> >
> > +config LTC166X
> > + tristate "Linear Technology LTC1660/LTC1665 DAC SPI driver"
> > + depends on SPI
> > + help
> > + Say yes here to build support for Linear Technology
> > + LTC1660 and LTC1665 Digital to Analog Converters.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called ltc166x.
> > +
> > config LTC2632
> > tristate "Linear Technology LTC2632-12/10/8 DAC spi driver"
> > depends on SPI
> > diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
> > index 81e710ed7491..380749c87c26 100644
> > --- a/drivers/iio/dac/Makefile
> > +++ b/drivers/iio/dac/Makefile
> > @@ -26,6 +26,7 @@ obj-$(CONFIG_CIO_DAC) += cio-dac.o
> > obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
> > obj-$(CONFIG_DS4424) += ds4424.o
> > obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
> > +obj-$(CONFIG_LTC166X) += ltc166x.o
> > obj-$(CONFIG_LTC2632) += ltc2632.o
> > obj-$(CONFIG_M62332) += m62332.o
> > obj-$(CONFIG_MAX517) += max517.o
> > diff --git a/drivers/iio/dac/ltc166x.c b/drivers/iio/dac/ltc166x.c
> > new file mode 100644
> > index ..0031f2b50f14
> > --- /dev/null
> > +++ b/drivers/iio/dac/ltc166x.c
> > @@ -0,0 +1,244 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for Linear Technology LTC1665/LTC1660, 8 channels DAC
> > + *
> > + * Copyright (C) 2018 Marcus Folkesson
> > + */
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define LTC166X_REG_WAKE 0x0
> > +#define LTC166X_REG_DAC_A 0x1
> > +#define LTC166X_REG_DAC_B 0x2
> > +#define LTC166X_REG_DAC_C 0x3
> > +#define LTC166X_REG_DAC_D 0x4
> > +#define LTC166X_REG_DAC_E 0x5
> > +#define LTC166X_REG_DAC_F 0x6
> > +#define LTC166X_REG_DAC_G 0x7
> > +#define LTC166X_REG_DAC_H 0x8
> > +#define LTC166X_REG_SLEEP 0xe
> > +
> > +#define LTC166X_NUM_CHANNELS 8
> > +
> > +static const struct regmap_config ltc166x_regmap_config = {
> > + .reg_bits = 4,
> > + .val_bits = 12,
> > +};
> > +
> > +enum ltc166x_supported_device_ids {
> > + ID_LTC1660,
> > + ID_LTC1665,
> > +};
> > +
> > +struct ltc166x_priv {
> > + struct spi_device *spi;
> > + struct regmap *regmap;
> > + struct regulator *vref_reg;
> > + unsigned int value[LTC166X_NUM_CHANNELS];
> > + unsigned int vref_mv;
> > +};
> > +
> > +static int ltc166x_read_raw(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + int *val,
> > + int *val2,
> > + long mask)
> > +{
> > + struct ltc166x_priv *priv = iio_priv(indio_dev);
> > +
> > + switch (mask) {
> > + case IIO_CHAN_INFO_RAW:
> > + *val = priv->value[chan->channel];
> > + return IIO_VAL_INT;
> > + case IIO_CHAN_INFO_SCAL
Re: [PATCH] iio: accel: cros_ec_accel_legacy: Mark expected switch fall-throughs
Hi Gutavo,
Sorry for the delay.
On Wed, Aug 15, 2018 at 12:50:10PM -0500, Gustavo A. R. Silva wrote:
> Hi Marcus,
>
> On 8/15/18 12:27 PM, Marcus Folkesson wrote:
> > Hi,
> >
> > On Wed, Aug 15, 2018 at 11:38:52AM -0500, Gustavo A. R. Silva wrote:
> >> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> >> where we are expecting to fall through.
> >>
> >> Addresses-Coverity-ID: 1462408 ("Missing break in switch")
> >> Signed-off-by: Gustavo A. R. Silva
> >> ---
> >> drivers/iio/accel/cros_ec_accel_legacy.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c
> >> b/drivers/iio/accel/cros_ec_accel_legacy.c
> >> index 063e89e..d609654 100644
> >> --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> >> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> >> @@ -385,8 +385,10 @@ static int cros_ec_accel_legacy_probe(struct
> >> platform_device *pdev)
> >>switch (i) {
> >>case X:
> >>ec_accel_channels[X].scan_index = Y;
> >> + /* fall through */
> >>case Y:
> >>ec_accel_channels[Y].scan_index = X;
> >> + /* fall through */
> >>case Z:
> >>ec_accel_channels[Z].scan_index = Z;
> >>}
> >
> > Hum, I'm not sure we are supposed to fall through here, even if it does
> > not hurt to do so.
>
> Yeah. You're right. It doesn't hurt but is actually redundant. I think
> the original intention was to break instead of falling through.
>
> > I even think we can remove the switch and put that outside the for-loop,
> > e.g:
> >
> > ec_accel_channels[X].scan_index = Y;
> > ec_accel_channels[Y].scan_index = X;
> > ec_accel_channels[Z].scan_index = Z;
> >
> > for (i = X ; i < MAX_AXIS; i++) {
> > if (state->sensor_num == MOTIONSENSE_LOC_LID && i != Y)
> > state->sign[i] = -1;
> > else
> > state->sign[i] = 1;
> > }
> >
>
> I like this, but the code clearly depends on MAX_AXIS. So, if MAX_AXIS
> will be always 3, then the change you suggest is just fine. Otherwise,
> it seems that adding a break to each case is the right way to go.
>
> What do you think?
Well, I guess it is a matter of taste after all.
I don't think the number of axis will change, but just put the break in
place is good enough.
Anyway, If we choose to not use the switch, I think we should remove the
for-loop as well, eg:
ec_accel_channels[X].scan_index = Y;
ec_accel_channels[Y].scan_index = X;
ec_accel_channels[Z].scan_index = Z;
if (state->sensor_num == MOTIONSENSE_LOC_LID) {
state->sign[X] = -1;
state->sign[Y] = 1;
state->sign[Z] = -1;
} else {
state->sign[X] = 1;
state->sign[Y] = 1;
state->sign[Z] = 1;
}
But someone else may like to give their point of view on this change.
>
> Thanks for the feedback.
> --
> Gustavo
Best regards
Marcus Folkesson
signature.asc
Description: PGP signature
Re: [PATCH] iio: accel: cros_ec_accel_legacy: Mark expected switch fall-throughs
Hi,
On Wed, Aug 15, 2018 at 11:38:52AM -0500, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Addresses-Coverity-ID: 1462408 ("Missing break in switch")
> Signed-off-by: Gustavo A. R. Silva
> ---
> drivers/iio/accel/cros_ec_accel_legacy.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c
> b/drivers/iio/accel/cros_ec_accel_legacy.c
> index 063e89e..d609654 100644
> --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> @@ -385,8 +385,10 @@ static int cros_ec_accel_legacy_probe(struct
> platform_device *pdev)
> switch (i) {
> case X:
> ec_accel_channels[X].scan_index = Y;
> + /* fall through */
> case Y:
> ec_accel_channels[Y].scan_index = X;
> + /* fall through */
> case Z:
> ec_accel_channels[Z].scan_index = Z;
> }
Hum, I'm not sure we are supposed to fall through here, even if it does
not hurt to do so.
I even think we can remove the switch and put that outside the for-loop,
e.g:
ec_accel_channels[X].scan_index = Y;
ec_accel_channels[Y].scan_index = X;
ec_accel_channels[Z].scan_index = Z;
for (i = X ; i < MAX_AXIS; i++) {
if (state->sensor_num == MOTIONSENSE_LOC_LID && i != Y)
state->sign[i] = -1;
else
state->sign[i] = 1;
}
Best regards,
Marcus Folkesson
> --
> 2.7.4
>
signature.asc
Description: PGP signature
[PATCH 1/3] iio: dac: add support for ltc166x
LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter
(DAC) with eight individual channels.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/Kconfig | 10 ++
drivers/iio/dac/Makefile | 1 +
drivers/iio/dac/ltc166x.c | 244 ++
3 files changed, 255 insertions(+)
create mode 100644 drivers/iio/dac/ltc166x.c
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 76db0768e454..04cfa6bb9dc1 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -120,6 +120,16 @@ config AD5624R_SPI
Say yes here to build support for Analog Devices AD5624R, AD5644R and
AD5664R converters (DAC). This driver uses the common SPI interface.
+config LTC166X
+ tristate "Linear Technology LTC1660/LTC1665 DAC SPI driver"
+ depends on SPI
+ help
+ Say yes here to build support for Linear Technology
+ LTC1660 and LTC1665 Digital to Analog Converters.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ltc166x.
+
config LTC2632
tristate "Linear Technology LTC2632-12/10/8 DAC spi driver"
depends on SPI
diff --git a/drivers/iio/dac/Makefile b/drivers/iio/dac/Makefile
index 81e710ed7491..380749c87c26 100644
--- a/drivers/iio/dac/Makefile
+++ b/drivers/iio/dac/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_CIO_DAC) += cio-dac.o
obj-$(CONFIG_DPOT_DAC) += dpot-dac.o
obj-$(CONFIG_DS4424) += ds4424.o
obj-$(CONFIG_LPC18XX_DAC) += lpc18xx_dac.o
+obj-$(CONFIG_LTC166X) += ltc166x.o
obj-$(CONFIG_LTC2632) += ltc2632.o
obj-$(CONFIG_M62332) += m62332.o
obj-$(CONFIG_MAX517) += max517.o
diff --git a/drivers/iio/dac/ltc166x.c b/drivers/iio/dac/ltc166x.c
new file mode 100644
index ..0031f2b50f14
--- /dev/null
+++ b/drivers/iio/dac/ltc166x.c
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Linear Technology LTC1665/LTC1660, 8 channels DAC
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define LTC166X_REG_WAKE 0x0
+#define LTC166X_REG_DAC_A 0x1
+#define LTC166X_REG_DAC_B 0x2
+#define LTC166X_REG_DAC_C 0x3
+#define LTC166X_REG_DAC_D 0x4
+#define LTC166X_REG_DAC_E 0x5
+#define LTC166X_REG_DAC_F 0x6
+#define LTC166X_REG_DAC_G 0x7
+#define LTC166X_REG_DAC_H 0x8
+#define LTC166X_REG_SLEEP 0xe
+
+#define LTC166X_NUM_CHANNELS 8
+
+static const struct regmap_config ltc166x_regmap_config = {
+ .reg_bits = 4,
+ .val_bits = 12,
+};
+
+enum ltc166x_supported_device_ids {
+ ID_LTC1660,
+ ID_LTC1665,
+};
+
+struct ltc166x_priv {
+ struct spi_device *spi;
+ struct regmap *regmap;
+ struct regulator *vref_reg;
+ unsigned int value[LTC166X_NUM_CHANNELS];
+ unsigned int vref_mv;
+};
+
+static int ltc166x_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val,
+ int *val2,
+ long mask)
+{
+ struct ltc166x_priv *priv = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ *val = priv->value[chan->channel];
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ *val = priv->vref_mv;
+ *val2 = chan->scan_type.realbits;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ltc166x_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long mask)
+{
+ struct ltc166x_priv *priv = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ if (val2 != 0)
+ return -EINVAL;
+ if (val > GENMASK(chan->scan_type.realbits-1, 0))
+ return -EINVAL;
+ priv->value[chan->channel] = val;
+ val <<= chan->scan_type.shift;
+ return regmap_write(priv->regmap, chan->channel, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+#define LTC166X_CHAN(chan, bits) { \
+ .type = IIO_VOLTAGE,\
+ .indexed = 1, \
+ .output = 1,\
+ .channel = chan,\
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .scan_type = { \
+ .sign = 'u',\
+ .realbits = (bits), \
+ .storage
[PATCH 2/3] dt-bindings: iio: dac: add bindings for ltc166x
LTC1665/LTC1660 is a 8/10-bit Digital-to-Analog Converter (DAC)
with eight individual channels.
Signed-off-by: Marcus Folkesson
---
.../devicetree/bindings/iio/dac/ltc166x.txt | 21 +
1 file changed, 21 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/dac/ltc166x.txt
diff --git a/Documentation/devicetree/bindings/iio/dac/ltc166x.txt
b/Documentation/devicetree/bindings/iio/dac/ltc166x.txt
new file mode 100644
index ..c5b5f22d6c64
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/dac/ltc166x.txt
@@ -0,0 +1,21 @@
+* Linear Technology Micropower octal 8-Bit and 10-Bit DACs
+
+Required properties:
+ - compatible: Must be one of the following:
+ "lltc,ltc1660"
+ "lltc,ltc1665"
+ - reg: SPI chip select number for the device
+ - vref-supply: Phandle to the voltage reference supply
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 5 MHz.
+
+Example:
+dac@0 {
+ compatible = "lltc,ltc1660";
+ reg = <0>;
+ spi-max-frequency = <500>;
+ vref-supply = <&vref_reg>;
+};
--
2.11.0.rc2
[PATCH 3/3] MAINTAINERS: add entry for ltc166x DAC driver
Add entry for ltc166x DAC driver and add myself as maintainer of this driver. Signed-off-by: Marcus Folkesson --- MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9276da915d9d..2dc4c773fb2e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8363,6 +8363,13 @@ L: linux-s...@vger.kernel.org S: Maintained F: drivers/scsi/sym53c8xx_2/ +LTC166X DAC DRIVER +M: Marcus Folkesson +L: linux-...@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/iio/dac/ltc166x.txt +F: drivers/iio/dac/ltc166x.c + LTC4261 HARDWARE MONITOR DRIVER M: Guenter Roeck L: linux-hw...@vger.kernel.org -- 2.11.0.rc2
[RESEND PATCH] iio: dac: max517: avoid using CONFIG_PM_SLEEP
This is already handled by SIMPLE_DEV_PM_OPS().
Signed-off-by: Marcus Folkesson
---
Somehow git-send-email messed up (?!) the format for (at least) gmail, so
resend it.
Mutt still read the mail correct though.
drivers/iio/dac/max517.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/dac/max517.c b/drivers/iio/dac/max517.c
index 1d853247a205..451d10e323cf 100644
--- a/drivers/iio/dac/max517.c
+++ b/drivers/iio/dac/max517.c
@@ -113,15 +113,14 @@ static int max517_write_raw(struct iio_dev *indio_dev,
return ret;
}
-#ifdef CONFIG_PM_SLEEP
-static int max517_suspend(struct device *dev)
+static int __maybe_unused max517_suspend(struct device *dev)
{
u8 outbuf = COMMAND_PD;
return i2c_master_send(to_i2c_client(dev), &outbuf, 1);
}
-static int max517_resume(struct device *dev)
+static int __maybe_unused max517_resume(struct device *dev)
{
u8 outbuf = 0;
@@ -129,10 +128,6 @@ static int max517_resume(struct device *dev)
}
static SIMPLE_DEV_PM_OPS(max517_pm_ops, max517_suspend, max517_resume);
-#define MAX517_PM_OPS (&max517_pm_ops)
-#else
-#define MAX517_PM_OPS NULL
-#endif
static const struct iio_info max517_info = {
.read_raw = max517_read_raw,
@@ -229,7 +224,7 @@ MODULE_DEVICE_TABLE(i2c, max517_id);
static struct i2c_driver max517_driver = {
.driver = {
.name = MAX517_DRV_NAME,
- .pm = MAX517_PM_OPS,
+ .pm = &max517_pm_ops,
},
.probe = max517_probe,
.remove = max517_remove,
--
2.11.0.rc2
[RESEND PATCH] iio: dac: mcp4725: avoid using CONFIG_PM_SLEEP
This is already handled by SIMPLE_DEV_PM_OPS().
Signed-off-by: Marcus Folkesson
---
Somehow git-send-email messed up (?!) the format for (at least) gmail, so
resend it.
Mutt still read the mail correct though.
drivers/iio/dac/mcp4725.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
index 8b5aad4c32d9..6d71fd905e29 100644
--- a/drivers/iio/dac/mcp4725.c
+++ b/drivers/iio/dac/mcp4725.c
@@ -45,7 +45,7 @@ struct mcp4725_data {
struct regulator *vref_reg;
};
-static int mcp4725_suspend(struct device *dev)
+static int __maybe_unused mcp4725_suspend(struct device *dev)
{
struct mcp4725_data *data = iio_priv(i2c_get_clientdata(
to_i2c_client(dev)));
@@ -58,7 +58,7 @@ static int mcp4725_suspend(struct device *dev)
return i2c_master_send(data->client, outbuf, 2);
}
-static int mcp4725_resume(struct device *dev)
+static int __maybe_unused mcp4725_resume(struct device *dev)
{
struct mcp4725_data *data = iio_priv(i2c_get_clientdata(
to_i2c_client(dev)));
@@ -71,13 +71,7 @@ static int mcp4725_resume(struct device *dev)
return i2c_master_send(data->client, outbuf, 2);
}
-
-#ifdef CONFIG_PM_SLEEP
static SIMPLE_DEV_PM_OPS(mcp4725_pm_ops, mcp4725_suspend, mcp4725_resume);
-#define MCP4725_PM_OPS (&mcp4725_pm_ops)
-#else
-#define MCP4725_PM_OPS NULL
-#endif
static ssize_t mcp4725_store_eeprom(struct device *dev,
struct device_attribute *attr, const char *buf, size_t len)
@@ -547,7 +541,7 @@ static struct i2c_driver mcp4725_driver = {
.driver = {
.name = MCP4725_DRV_NAME,
.of_match_table = of_match_ptr(mcp4725_of_match),
- .pm = MCP4725_PM_OPS,
+ .pm = &mcp4725_pm_ops,
},
.probe = mcp4725_probe,
.remove = mcp4725_remove,
--
2.11.0.rc2
[RESEND PATCH] iio: dac: max5821: avoid using CONFIG_PM_SLEEP
This is already handled by SIMPLE_DEV_PM_OPS().
Signed-off-by: Marcus Folkesson
---
Somehow git-send-email messed up (?!) the format for (at least) gmail, so
resend it.
Mutt still read the mail correct though.
drivers/iio/dac/max5821.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/dac/max5821.c b/drivers/iio/dac/max5821.c
index d0ecc1fdd8fc..f0cf6903dcd2 100644
--- a/drivers/iio/dac/max5821.c
+++ b/drivers/iio/dac/max5821.c
@@ -270,8 +270,7 @@ static int max5821_write_raw(struct iio_dev *indio_dev,
}
}
-#ifdef CONFIG_PM_SLEEP
-static int max5821_suspend(struct device *dev)
+static int __maybe_unused max5821_suspend(struct device *dev)
{
u8 outbuf[2] = { MAX5821_EXTENDED_COMMAND_MODE,
MAX5821_EXTENDED_DAC_A |
@@ -281,7 +280,7 @@ static int max5821_suspend(struct device *dev)
return i2c_master_send(to_i2c_client(dev), outbuf, 2);
}
-static int max5821_resume(struct device *dev)
+static int __maybe_unused max5821_resume(struct device *dev)
{
u8 outbuf[2] = { MAX5821_EXTENDED_COMMAND_MODE,
MAX5821_EXTENDED_DAC_A |
@@ -292,10 +291,6 @@ static int max5821_resume(struct device *dev)
}
static SIMPLE_DEV_PM_OPS(max5821_pm_ops, max5821_suspend, max5821_resume);
-#define MAX5821_PM_OPS (&max5821_pm_ops)
-#else
-#define MAX5821_PM_OPS NULL
-#endif /* CONFIG_PM_SLEEP */
static const struct iio_info max5821_info = {
.read_raw = max5821_read_raw,
@@ -392,7 +387,7 @@ static struct i2c_driver max5821_driver = {
.driver = {
.name = "max5821",
.of_match_table = max5821_of_match,
- .pm = MAX5821_PM_OPS,
+ .pm = &max5821_pm_ops,
},
.probe = max5821_probe,
.remove = max5821_remove,
--
2.11.0.rc2
Re: [PATCH v4 5/6] iio:adxl372: Add sampling frequency support
6400");
> +
> +static struct attribute *adxl372_attributes[] = {
> + &iio_const_attr_sampling_frequency_available.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group adxl372_attrs_group = {
> + .attrs = adxl372_attributes,
> +};
> +
> static const struct iio_info adxl372_info = {
> + .attrs = &adxl372_attrs_group,
> .read_raw = adxl372_read_raw,
> + .write_raw = adxl372_write_raw,
> .debugfs_reg_access = &adxl372_reg_access,
> .hwfifo_set_watermark = adxl372_set_watermark,
> };
> --
> 2.7.4
Best regards,
Marcus Folkesson
>
signature.asc
Description: PGP signature
Re: [PATCH v4 1/6] iio: adxl372: New driver for Analog Devices ADXL372 Accelerometer
Hi Stefan,
On Mon, Aug 06, 2018 at 03:04:42PM +0300, Stefan Popa wrote:
> This patch adds basic support for Analog Devices ADXL372 SPI-Bus
> Three-Axis Digital Accelerometer.
>
> The device is probed and configured the with some initial default
> values. With this basic driver, it is possible to read raw acceleration
> data.
>
> Datasheet:
> http://www.analog.com/media/en/technical-documentation/data-sheets/ADXL372.pdf
>
> Signed-off-by: Stefan Popa
> ---
> MAINTAINERS | 6 +
> drivers/iio/accel/Kconfig | 11 +
> drivers/iio/accel/Makefile | 1 +
> drivers/iio/accel/adxl372.c | 525
>
> 4 files changed, 543 insertions(+)
> create mode 100644 drivers/iio/accel/adxl372.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 60b1028..2ba47bb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -543,6 +543,12 @@ W:
> http://ez.analog.com/community/linux-device-drivers
> S: Supported
> F: drivers/input/misc/adxl34x.c
>
> +ADXL372 THREE-AXIS DIGITAL ACCELEROMETER DRIVER
> +M: Stefan Popa
> +W: http://ez.analog.com/community/linux-device-drivers
> +S: Supported
> +F: drivers/iio/accel/adxl372.c
> +
> AF9013 MEDIA DRIVER
> M: Antti Palosaari
> L: linux-me...@vger.kernel.org
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 62ae7e5..1b496ef 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -60,6 +60,17 @@ config ADXL345_SPI
> will be called adxl345_spi and you will also get adxl345_core
> for the core module.
>
> +config ADXL372
> + tristate "Analog Devices ADXL372 3-Axis Accelerometer Driver"
> + depends on SPI
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + Say yes here to add support for the Analog Devices ADXL372 triaxial
> + acceleration sensor.
> + To compile this driver as a module, choose M here: the
> + module will be called adxl372.
> +
> config BMA180
> tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver"
> depends on I2C
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 636d4d1..5758ffc 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_ADIS16209) += adis16209.o
> obj-$(CONFIG_ADXL345) += adxl345_core.o
> obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
> obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
> +obj-$(CONFIG_ADXL372) += adxl372.o
> obj-$(CONFIG_BMA180) += bma180.o
> obj-$(CONFIG_BMA220) += bma220_spi.o
> obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> new file mode 100644
> index 000..db9ecd2
> --- /dev/null
> +++ b/drivers/iio/accel/adxl372.c
> @@ -0,0 +1,525 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * ADXL372 3-Axis Digital Accelerometer SPI driver
> + *
> + * Copyright 2018 Analog Devices Inc.
> + */
Please make the SPDX-identifier and MODULE_LICENCE match here as well.
Either
SPDX-License-Identifier: GPL-2.0+
MODULE_LICENSE("GPL")
or
SPDX-License-Identifier: GPL-2.0
MODULE_LICENSE("GPL v2")
Thanks!
Marcus Folkesson
signature.asc
Description: PGP signature
[PATCH] iio: dac: max517: avoid using CONFIG_PM_SLEEP
SIMPLE_DEV_PM_OPS() is already doing this.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/max517.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/dac/max517.c b/drivers/iio/dac/max517.c
index 1d853247a205..451d10e323cf 100644
--- a/drivers/iio/dac/max517.c
+++ b/drivers/iio/dac/max517.c
@@ -113,15 +113,14 @@ static int max517_write_raw(struct iio_dev *indio_dev,
return ret;
}
-#ifdef CONFIG_PM_SLEEP
-static int max517_suspend(struct device *dev)
+static int __maybe_unused max517_suspend(struct device *dev)
{
u8 outbuf = COMMAND_PD;
return i2c_master_send(to_i2c_client(dev), &outbuf, 1);
}
-static int max517_resume(struct device *dev)
+static int __maybe_unused max517_resume(struct device *dev)
{
u8 outbuf = 0;
@@ -129,10 +128,6 @@ static int max517_resume(struct device *dev)
}
static SIMPLE_DEV_PM_OPS(max517_pm_ops, max517_suspend, max517_resume);
-#define MAX517_PM_OPS (&max517_pm_ops)
-#else
-#define MAX517_PM_OPS NULL
-#endif
static const struct iio_info max517_info = {
.read_raw = max517_read_raw,
@@ -229,7 +224,7 @@ MODULE_DEVICE_TABLE(i2c, max517_id);
static struct i2c_driver max517_driver = {
.driver = {
.name = MAX517_DRV_NAME,
- .pm = MAX517_PM_OPS,
+ .pm = &max517_pm_ops,
},
.probe = max517_probe,
.remove = max517_remove,
--
2.11.0.rc2
[PATCH] iio: dac: mcp4725: avoid using CONFIG_PM_SLEEP
SIMPLE_DEV_PM_OPS() is already doing this.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/mcp4725.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/dac/mcp4725.c b/drivers/iio/dac/mcp4725.c
index 8b5aad4c32d9..6d71fd905e29 100644
--- a/drivers/iio/dac/mcp4725.c
+++ b/drivers/iio/dac/mcp4725.c
@@ -45,7 +45,7 @@ struct mcp4725_data {
struct regulator *vref_reg;
};
-static int mcp4725_suspend(struct device *dev)
+static int __maybe_unused mcp4725_suspend(struct device *dev)
{
struct mcp4725_data *data = iio_priv(i2c_get_clientdata(
to_i2c_client(dev)));
@@ -58,7 +58,7 @@ static int mcp4725_suspend(struct device *dev)
return i2c_master_send(data->client, outbuf, 2);
}
-static int mcp4725_resume(struct device *dev)
+static int __maybe_unused mcp4725_resume(struct device *dev)
{
struct mcp4725_data *data = iio_priv(i2c_get_clientdata(
to_i2c_client(dev)));
@@ -71,13 +71,7 @@ static int mcp4725_resume(struct device *dev)
return i2c_master_send(data->client, outbuf, 2);
}
-
-#ifdef CONFIG_PM_SLEEP
static SIMPLE_DEV_PM_OPS(mcp4725_pm_ops, mcp4725_suspend, mcp4725_resume);
-#define MCP4725_PM_OPS (&mcp4725_pm_ops)
-#else
-#define MCP4725_PM_OPS NULL
-#endif
static ssize_t mcp4725_store_eeprom(struct device *dev,
struct device_attribute *attr, const char *buf, size_t len)
@@ -547,7 +541,7 @@ static struct i2c_driver mcp4725_driver = {
.driver = {
.name = MCP4725_DRV_NAME,
.of_match_table = of_match_ptr(mcp4725_of_match),
- .pm = MCP4725_PM_OPS,
+ .pm = &mcp4725_pm_ops,
},
.probe = mcp4725_probe,
.remove = mcp4725_remove,
--
2.11.0.rc2
[PATCH] iio: dac: max5821: avoid using CONFIG_PM_SLEEP
SIMPLE_DEV_PM_OPS() is already doing this.
Signed-off-by: Marcus Folkesson
---
drivers/iio/dac/max5821.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/dac/max5821.c b/drivers/iio/dac/max5821.c
index d0ecc1fdd8fc..f0cf6903dcd2 100644
--- a/drivers/iio/dac/max5821.c
+++ b/drivers/iio/dac/max5821.c
@@ -270,8 +270,7 @@ static int max5821_write_raw(struct iio_dev *indio_dev,
}
}
-#ifdef CONFIG_PM_SLEEP
-static int max5821_suspend(struct device *dev)
+static int __maybe_unused max5821_suspend(struct device *dev)
{
u8 outbuf[2] = { MAX5821_EXTENDED_COMMAND_MODE,
MAX5821_EXTENDED_DAC_A |
@@ -281,7 +280,7 @@ static int max5821_suspend(struct device *dev)
return i2c_master_send(to_i2c_client(dev), outbuf, 2);
}
-static int max5821_resume(struct device *dev)
+static int __maybe_unused max5821_resume(struct device *dev)
{
u8 outbuf[2] = { MAX5821_EXTENDED_COMMAND_MODE,
MAX5821_EXTENDED_DAC_A |
@@ -292,10 +291,6 @@ static int max5821_resume(struct device *dev)
}
static SIMPLE_DEV_PM_OPS(max5821_pm_ops, max5821_suspend, max5821_resume);
-#define MAX5821_PM_OPS (&max5821_pm_ops)
-#else
-#define MAX5821_PM_OPS NULL
-#endif /* CONFIG_PM_SLEEP */
static const struct iio_info max5821_info = {
.read_raw = max5821_read_raw,
@@ -392,7 +387,7 @@ static struct i2c_driver max5821_driver = {
.driver = {
.name = "max5821",
.of_match_table = max5821_of_match,
- .pm = MAX5821_PM_OPS,
+ .pm = &max5821_pm_ops,
},
.probe = max5821_probe,
.remove = max5821_remove,
--
2.11.0.rc2
Re: [PATCH v6 1/6] iio: adxl372: New driver for Analog Devices ADXL372 Accelerometer
Hi Stefan,
On Fri, Aug 10, 2018 at 11:46:18AM +0300, Stefan Popa wrote:
> This patch adds basic support for Analog Devices ADXL372 SPI-Bus
> Three-Axis Digital Accelerometer.
>
> The device is probed and configured the with some initial default
> values. With this basic driver, it is possible to read raw acceleration
> data.
>
> Datasheet:
> http://www.analog.com/media/en/technical-documentation/data-sheets/ADXL372.pdf
>
> Signed-off-by: Stefan Popa
> ---
> MAINTAINERS | 6 +
> drivers/iio/accel/Kconfig | 11 +
> drivers/iio/accel/Makefile | 1 +
> drivers/iio/accel/adxl372.c | 525
>
> 4 files changed, 543 insertions(+)
> create mode 100644 drivers/iio/accel/adxl372.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 60b1028..2ba47bb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -543,6 +543,12 @@ W:
> http://ez.analog.com/community/linux-device-drivers
> S: Supported
> F: drivers/input/misc/adxl34x.c
>
> +ADXL372 THREE-AXIS DIGITAL ACCELEROMETER DRIVER
> +M: Stefan Popa
> +W: http://ez.analog.com/community/linux-device-drivers
> +S: Supported
> +F: drivers/iio/accel/adxl372.c
> +
> AF9013 MEDIA DRIVER
> M: Antti Palosaari
> L: linux-me...@vger.kernel.org
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index 62ae7e5..1b496ef 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -60,6 +60,17 @@ config ADXL345_SPI
> will be called adxl345_spi and you will also get adxl345_core
> for the core module.
>
> +config ADXL372
> + tristate "Analog Devices ADXL372 3-Axis Accelerometer Driver"
> + depends on SPI
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + help
> + Say yes here to add support for the Analog Devices ADXL372 triaxial
> + acceleration sensor.
> + To compile this driver as a module, choose M here: the
> + module will be called adxl372.
> +
> config BMA180
> tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver"
> depends on I2C
> diff --git a/drivers/iio/accel/Makefile b/drivers/iio/accel/Makefile
> index 636d4d1..5758ffc 100644
> --- a/drivers/iio/accel/Makefile
> +++ b/drivers/iio/accel/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_ADIS16209) += adis16209.o
> obj-$(CONFIG_ADXL345) += adxl345_core.o
> obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
> obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
> +obj-$(CONFIG_ADXL372) += adxl372.o
> obj-$(CONFIG_BMA180) += bma180.o
> obj-$(CONFIG_BMA220) += bma220_spi.o
> obj-$(CONFIG_BMC150_ACCEL) += bmc150-accel-core.o
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> new file mode 100644
> index 000..db9ecd2
> --- /dev/null
> +++ b/drivers/iio/accel/adxl372.c
> @@ -0,0 +1,525 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
The SPDX identifier "GPL-2.0+" is "GPL v2 or later", and the MODULE_LICENSE
"GPL v2" is "GPL v2 only"
See include/linux/module.h ::
* "GPL" [GNU Public License v2 or later]
* "GPL v2"[GNU Public License v2]
Please make them match :-)
[snip]
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>
Best regards
Marcus Folkesson
signature.asc
Description: PGP signature
[PATCH v4 1/3] iio: adc: add support for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Co-Developed-by: Kent Gustavsson
Signed-off-by: Kent Gustavsson
Signed-off-by: Marcus Folkesson
---
Notes:
v4:
- remove defined(CONFIG_OF) and of_match_ptr() macro
- do not check adc->clki as clock api is NULL-aware
- add Kent as co-developer
v3:
- rename adc_clk to clki
- add error handling/cleanup for clock
v2:
- cleanups and bugfixes (thanks Peter Meerwald-Stadler)
- drop hardware gain
- use the presence or lack of regulator to indicate if we go for
internal or external voltage reference
- do not store device node in private struct
- drop support to set width in devicetree
- use the presence or lack of clock to indicate if we go for internal
or external clock
drivers/iio/adc/Kconfig | 10 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mcp3911.c | 361 ++
3 files changed, 372 insertions(+)
create mode 100644 drivers/iio/adc/mcp3911.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 15606f237480..f9a41fa96fcc 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -501,6 +501,16 @@ config MCP3422
This driver can also be built as a module. If so, the module will be
called mcp3422.
+config MCP3911
+ tristate "Microchip Technology MCP3911 driver"
+ depends on SPI
+ help
+ Say yes here to build support for Microchip Technology's MCP3911
+ analog to digital converter.
+
+ This driver can also be built as a module. If so, the module will be
+ called mcp3911.
+
config MEDIATEK_MT6577_AUXADC
tristate "MediaTek AUXADC driver"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 28a9423997f3..3cfebfff7d26 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o
obj-$(CONFIG_MAX9611) += max9611.o
obj-$(CONFIG_MCP320X) += mcp320x.o
obj-$(CONFIG_MCP3422) += mcp3422.o
+obj-$(CONFIG_MCP3911) += mcp3911.o
obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
new file mode 100644
index ..540c8fbcce89
--- /dev/null
+++ b/drivers/iio/adc/mcp3911.c
@@ -0,0 +1,361 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Microchip MCP3911, Two-channel Analog Front End
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ * Copyright (C) 2018 Kent Gustavsson
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MCP3911_REG_CHANNEL0 0x00
+#define MCP3911_REG_CHANNEL1 0x03
+#define MCP3911_REG_MOD0x06
+#define MCP3911_REG_PHASE 0x07
+#define MCP3911_REG_GAIN 0x09
+
+#define MCP3911_REG_STATUSCOM 0x0a
+#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4)
+#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3)
+#define MCP3911_STATUSCOM_EN_OFFCALBIT(2)
+#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1)
+
+#define MCP3911_REG_CONFIG 0x0c
+#define MCP3911_CONFIG_CLKEXT BIT(1)
+#define MCP3911_CONFIG_VREFEXT BIT(2)
+
+#define MCP3911_REG_OFFCAL_CH0 0x0e
+#define MCP3911_REG_GAINCAL_CH00x11
+#define MCP3911_REG_OFFCAL_CH1 0x14
+#define MCP3911_REG_GAINCAL_CH10x17
+#define MCP3911_REG_VREFCAL0x1a
+
+#define MCP3911_CHANNEL(x) (MCP3911_REG_CHANNEL0 + x * 3)
+#define MCP3911_OFFCAL(x) (MCP3911_REG_OFFCAL_CH0 + x * 6)
+
+/* Internal voltage reference in uV */
+#define MCP3911_INT_VREF_UV120
+
+#define MCP3911_REG_READ(reg, id) reg) << 1) | ((id) << 5) | (1 <<
0)) & 0xff)
+#define MCP3911_REG_WRITE(reg, id) reg) << 1) | ((id) << 5) | (0 <<
0)) & 0xff)
+
+#define MCP3911_NUM_CHANNELS 2
+
+struct mcp3911 {
+ struct spi_device *spi;
+ struct mutex lock;
+ struct regulator *vref;
+ struct clk *clki;
+ u32 dev_addr;
+};
+
+static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
+{
+ int ret;
+
+ reg = MCP3911_REG_READ(reg, adc->dev_addr);
+ ret = spi_write_then_read(adc->spi, ®, 1, val, len);
+ if (ret < 0)
+ return ret;
+
+ be32_to_cpus(val);
+ *val >>= ((4 - len) * 8);
+ dev_dbg(&adc->spi->dev, "reading 0x%x from register 0x%x\n", *val,
+ reg>>1);
+ return ret;
+}
+
+static int mcp3911_write(struct mcp3911 *adc, u8 reg, u32 val, u8 len)
[PATCH v4 3/3] MAINTAINERS: Add entry for mcp3911 ADC driver
Add an entry for mcp3911 ADC driver and add myself and Kent Gustavsson as maintainers of this driver. Co-Developed-by: Kent Gustavsson Signed-off-by: Kent Gustavsson Signed-off-by: Marcus Folkesson --- Notes: v4: - add Kent as co-developer v3: - no changes v2: - no changes MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 79bb02ff812f..9276da915d9d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9271,6 +9271,14 @@ L: net...@vger.kernel.org S: Maintained F: drivers/net/ethernet/microchip/lan743x_* +MICROCHIP / ATMEL MCP3911 ADC DRIVER +M: Marcus Folkesson +M: Kent Gustavsson +L: linux-...@vger.kernel.org +S: Supported +F: drivers/iio/adc/mcp3911.c +F: Documentation/devicetree/bindings/iio/adc/mcp3911.txt + MICROCHIP USB251XB DRIVER M: Richard Leitner L: linux-...@vger.kernel.org -- 2.11.0.rc2
[PATCH v4 2/3] dt-bindings: iio: adc: add bindings for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Co-Developed-by: Kent Gustavsson
Signed-off-by: Kent Gustavsson
Signed-off-by: Marcus Folkesson
Reviewed-by: Rob Herring
---
Notes:
v4:
- remove reference to Documentation/.../interrupts.txt
- add Kent as co-developer
v3:
- add bindings for interrupt
- prefix device-addr with `microchip`
- drop `clock-names`
v2:
- drop channel width
- drop `external_vref`
- replace `external-clock` with proper clock bindings
.../devicetree/bindings/iio/adc/mcp3911.txt| 30 ++
1 file changed, 30 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/mcp3911.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
new file mode 100644
index ..3071f48fb30b
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
@@ -0,0 +1,30 @@
+* Microchip MCP3911 Dual channel analog front end (ADC)
+
+Required properties:
+ - compatible: Should be "microchip,mcp3911"
+ - reg: SPI chip select number for the device
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 20MHz.
+
+Optional properties:
+ - clocks: Phandle and clock identifier for sampling clock
+ - interrupt-parent: Phandle to the parent interrupt controller
+ - interrupts: IRQ line for the ADC
+ - microchip,device-addr: Device address when multiple MCP3911 chips are
present on the
+ same SPI bus. Valid values are 0-3. Defaults to 0.
+ - vref-supply: Phandle to the external reference voltage supply.
+
+Example:
+adc@0 {
+ compatible = "microchip,mcp3911";
+ reg = <0>;
+ interrupt-parent = <&gpio5>;
+ interrupts = <15 IRQ_TYPE_EDGE_RISING>;
+ spi-max-frequency = <2000>;
+ microchip,device-addr = <0>;
+ vref-supply = <&vref_reg>;
+ clocks = <&xtal>;
+};
--
2.11.0.rc2
Re: [PATCH v3 1/3] iio: adc: add support for mcp3911
Hi Andy and Jonathan,
On Fri, Aug 03, 2018 at 11:09:22PM +0100, Jonathan Cameron wrote:
> On Thu, 2 Aug 2018 22:52:00 +0300
> Andy Shevchenko wrote:
>
> > On Thu, Aug 2, 2018 at 10:15 PM, Marcus Folkesson
> > wrote:
> > > MCP3911 is a dual channel Analog Front End (AFE) containing two
> > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
> > >
> > > Signed-off-by: Marcus Folkesson
> >
> > > Signed-off-by: Kent Gustavsson
> >
> > What is this? Why it's here (presense and location in the message)?
> To clarify... If Kent wrote the patch and you are simply acting
> as gatekeeper / upstreamer you should set the mail to be from Kent
> and put your own Signed-off after his to basically act as a submaintainer
> certifying you believe his sign off and all that entails.
>
> If it is a bit of a joint effort then that's fine but for copyright
> purposes there should be some indication of the split.
First, thank you Andy for noticing.
I actually intended to use Co-Developed-by (a pretty new tag)
in combination with Signed-off-by.
But the tag must have disappeared in some preparation stage..
From Documentation/process/submitting-patches.rst ::
A Co-Developed-by: states that the patch was also created by another
developer
along with the original author. This is useful at times when multiple
people
work on a single patch. Note, this person also needs to have a
Signed-off-by:
line in the patch as well.
I will switch order and add the Co-Developed-by-tag.
Is this correct?
Co-Developed-by: Kent Gustavsson
Signed-off-by: Kent Gustavsson
Signed-off-by: Marcus Folkesson
>
>
> >
> > > + * Copyright (C) 2018 Kent Gustavsson
> >
> > > + *
> >
> > Redundant.
> >
> > > +static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
> > > +{
> > > + int ret;
> > > +
> > > + reg = MCP3911_REG_READ(reg, adc->dev_addr);
> > > + ret = spi_write_then_read(adc->spi, ®, 1, val, len);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + be32_to_cpus(val);
> > > + *val >>= ((4 - len) * 8);
> > > + dev_dbg(&adc->spi->dev, "reading 0x%x from register 0x%x\n", *val,
> > > + reg>>1);
> > > + return ret;
> > > +}
> > > +
> > > +static int mcp3911_write(struct mcp3911 *adc, u8 reg, u32 val, u8 len)
> > > +{
> > > + dev_dbg(&adc->spi->dev, "writing 0x%x to register 0x%x\n", val,
> > > reg);
> > > +
> > > + val <<= (3 - len) * 8;
> > > + cpu_to_be32s(&val);
> > > + val |= MCP3911_REG_WRITE(reg, adc->dev_addr);
> > > +
> > > + return spi_write(adc->spi, &val, len + 1);
> > > +}
> >
> > Can't you use REGMAP_SPI ?
> My instinct is not really. The magic device-addr doesn't help.
> You could work around it but it's not that nice and you'd have
> to create the regmap mapping on the fly or carry static ones
> for each value of htat.
>
>
>
> >
> > > + of_property_read_u32(of_node, "device-addr", &adc->dev_addr);
> > > + if (adc->dev_addr > 3) {
> > > + dev_err(&adc->spi->dev,
> > > + "invalid device address (%i). Must be in
> > > range 0-3.\n",
> > > + adc->dev_addr);
> > > + return -EINVAL;
> > > + }
> > > + dev_dbg(&adc->spi->dev, "use device address %i\n",
> > > adc->dev_addr);
> >
> > Isn't what we called CS (chip select)?
> Nope. We went around this in an earlier revision. It's an address transmitted
> in the control byte to allow you to 'share' a chip select line between
> multiple
> chips (crazy but true).
>
> >
> > > + if (IS_ERR(adc->vref)) {
> >
> > > +
> >
> > Redundant.
> >
> > > + if (adc->clki)
> >
> > Seems to be redundant if clock is optional (it should be NULL and API
> > is NULL-aware).
> >
> > > + clk_disable_unprepare(adc->clki);
> >
> > > + if (adc->clki)
> > > + clk_disable_unprepare(adc->clki);
> >
> > Ditto.
> >
> > > +#if defined(CONFIG_OF)
> >
> > This prevents playing with the device on ACPI enabled platforms.
I will remove the defined(CONFIG_OF) and not use the of_match_ptr()
macro.
> Yeah, that trickery is still little known (and I forget about it from
> time to time).
>
> The upshot for those who have missed this is you can use a magic
> acpi device to instantiate based on device tree bindings :)
>
> So we need to strip all this 'obvious' protection stuff out of drivers.
Jonathan,
Do you want me to do the same changes for drivers in iio/ ?
I'm probably not the only one looking at other drivers when writing my
own, so I guess this is a rather frequent issue for new drivers?
>
> >
> > > + .of_match_table = of_match_ptr(mcp3911_dt_ids),
> >
> > Ditto for macro.
> >
>
Best regards,
Marcus Folkesson
signature.asc
Description: PGP signature
[PATCH v3 3/3] MAINTAINERS: Add entry for mcp3911 ADC driver
Add an entry for mcp3911 ADC driver and add myself and Kent Gustavsson as maintainers of this driver. Signed-off-by: Marcus Folkesson Signed-off-by: Kent Gustavsson --- Notes: v3: - no changes v2: - no changes MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 79bb02ff812f..9276da915d9d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9271,6 +9271,14 @@ L: net...@vger.kernel.org S: Maintained F: drivers/net/ethernet/microchip/lan743x_* +MICROCHIP / ATMEL MCP3911 ADC DRIVER +M: Marcus Folkesson +M: Kent Gustavsson +L: linux-...@vger.kernel.org +S: Supported +F: drivers/iio/adc/mcp3911.c +F: Documentation/devicetree/bindings/iio/adc/mcp3911.txt + MICROCHIP USB251XB DRIVER M: Richard Leitner L: linux-...@vger.kernel.org -- 2.11.0.rc2
[PATCH v3 2/3] dt-bindings: iio: adc: add bindings for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
Notes:
v3:
- add bindings for interrupt
- prefix device-addr with `microchip`
- drop `clock-names`
v2:
- drop channel width
- drop `external_vref`
- replace `external-clock` with proper clock bindings
.../devicetree/bindings/iio/adc/mcp3911.txt| 32 ++
1 file changed, 32 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/mcp3911.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
new file mode 100644
index ..432da7a28783
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
@@ -0,0 +1,32 @@
+* Microchip MCP3911 Dual channel analog front end (ADC)
+
+Required properties:
+ - compatible: Should be "microchip,mcp3911"
+ - reg: SPI chip select number for the device
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 20MHz.
+
+Optional properties:
+ - clocks: Phandle and clock identifier for sampling clock
+ - interrupt-parent: Phandle to the parent interrupt controller
+ see:
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+ - interrupts: IRQ line for the ADC
+ see:
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+ - microchip,device-addr: Device address when multiple MCP3911 chips are
present on the
+ same SPI bus. Valid values are 0-3. Defaults to 0.
+ - vref-supply: Phandle to the external reference voltage supply.
+
+Example:
+adc@0 {
+ compatible = "microchip,mcp3911";
+ reg = <0>;
+ interrupt-parent = <&gpio5>;
+ interrupts = <15 IRQ_TYPE_EDGE_RISING>;
+ spi-max-frequency = <2000>;
+ microchip,device-addr = <0>;
+ vref-supply = <&vref_reg>;
+ clocks = <&xtal>;
+};
--
2.11.0.rc2
[PATCH v3 1/3] iio: adc: add support for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
Notes:
v3:
- rename adc_clk to clki
- add error handling/cleanup for clock
v2:
- cleanups and bugfixes (thanks Peter Meerwald-Stadler)
- drop hardware gain
- use the presence or lack of regulator to indicate if we go for
internal or external voltage reference
- do not store device node in private struct
- drop support to set width in devicetree
- use the presence or lack of clock to indicate if we go for internal
or external clock
drivers/iio/adc/Kconfig | 10 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mcp3911.c | 367 ++
3 files changed, 378 insertions(+)
create mode 100644 drivers/iio/adc/mcp3911.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 15606f237480..f9a41fa96fcc 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -501,6 +501,16 @@ config MCP3422
This driver can also be built as a module. If so, the module will be
called mcp3422.
+config MCP3911
+ tristate "Microchip Technology MCP3911 driver"
+ depends on SPI
+ help
+ Say yes here to build support for Microchip Technology's MCP3911
+ analog to digital converter.
+
+ This driver can also be built as a module. If so, the module will be
+ called mcp3911.
+
config MEDIATEK_MT6577_AUXADC
tristate "MediaTek AUXADC driver"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 28a9423997f3..3cfebfff7d26 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o
obj-$(CONFIG_MAX9611) += max9611.o
obj-$(CONFIG_MCP320X) += mcp320x.o
obj-$(CONFIG_MCP3422) += mcp3422.o
+obj-$(CONFIG_MCP3911) += mcp3911.o
obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
new file mode 100644
index ..fca8e6a304ec
--- /dev/null
+++ b/drivers/iio/adc/mcp3911.c
@@ -0,0 +1,367 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Microchip MCP3911, Two-channel Analog Front End
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ * Copyright (C) 2018 Kent Gustavsson
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MCP3911_REG_CHANNEL0 0x00
+#define MCP3911_REG_CHANNEL1 0x03
+#define MCP3911_REG_MOD0x06
+#define MCP3911_REG_PHASE 0x07
+#define MCP3911_REG_GAIN 0x09
+
+#define MCP3911_REG_STATUSCOM 0x0a
+#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4)
+#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3)
+#define MCP3911_STATUSCOM_EN_OFFCALBIT(2)
+#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1)
+
+#define MCP3911_REG_CONFIG 0x0c
+#define MCP3911_CONFIG_CLKEXT BIT(1)
+#define MCP3911_CONFIG_VREFEXT BIT(2)
+
+#define MCP3911_REG_OFFCAL_CH0 0x0e
+#define MCP3911_REG_GAINCAL_CH00x11
+#define MCP3911_REG_OFFCAL_CH1 0x14
+#define MCP3911_REG_GAINCAL_CH10x17
+#define MCP3911_REG_VREFCAL0x1a
+
+#define MCP3911_CHANNEL(x) (MCP3911_REG_CHANNEL0 + x * 3)
+#define MCP3911_OFFCAL(x) (MCP3911_REG_OFFCAL_CH0 + x * 6)
+
+/* Internal voltage reference in uV */
+#define MCP3911_INT_VREF_UV120
+
+#define MCP3911_REG_READ(reg, id) reg) << 1) | ((id) << 5) | (1 <<
0)) & 0xff)
+#define MCP3911_REG_WRITE(reg, id) reg) << 1) | ((id) << 5) | (0 <<
0)) & 0xff)
+
+#define MCP3911_NUM_CHANNELS 2
+
+struct mcp3911 {
+ struct spi_device *spi;
+ struct mutex lock;
+ struct regulator *vref;
+ struct clk *clki;
+ u32 dev_addr;
+};
+
+static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
+{
+ int ret;
+
+ reg = MCP3911_REG_READ(reg, adc->dev_addr);
+ ret = spi_write_then_read(adc->spi, ®, 1, val, len);
+ if (ret < 0)
+ return ret;
+
+ be32_to_cpus(val);
+ *val >>= ((4 - len) * 8);
+ dev_dbg(&adc->spi->dev, "reading 0x%x from register 0x%x\n", *val,
+ reg>>1);
+ return ret;
+}
+
+static int mcp3911_write(struct mcp3911 *adc, u8 reg, u32 val, u8 len)
+{
+ dev_dbg(&adc->spi->dev, "writing 0x%x to register 0x%x\n", val, reg);
+
+ val <<= (3 - len) * 8;
+ cpu_to_be32s(&val);
+ val |= MCP3911_REG
Re: [PATCH v2 1/3] iio: adc: add support for mcp3911
Hi Jonathan, Sorry for the delay, I've been away from keyboard for a few days. On Sun, Jul 29, 2018 at 12:44:22PM +0100, Jonathan Cameron wrote: > On Tue, 24 Jul 2018 20:30:02 +0200 > Marcus Folkesson wrote: > > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > > > Signed-off-by: Marcus Folkesson > > Signed-off-by: Kent Gustavsson > Hi > > I think you missed cleaning up the clock enable in the remove. > If it is safe to not do so it should also be safe in the error path > of probe. I've actually noticed this and had it prepared in v4 :-) > > A couple of trivial other things jumped out at me whilst reading. Thank you so much, I will fix these. > > Thanks, > > Jonathan > > --- > > > > Notes: > > v2: > > - cleanups and bugfixes (thanks Peter Meerwald-Stadler) > > - drop hardware gain > > - use the presence or lack of regulator to indicate if we go > > for internal or external voltage reference > > - do not store device node in private struct > > - drop support to set width in devicetree > > - use the presence or lack of clock to indicate if we go for > > internal or external clock > > > > drivers/iio/adc/Kconfig | 10 ++ > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/mcp3911.c | 366 > > ++ > > 3 files changed, 377 insertions(+) > > create mode 100644 drivers/iio/adc/mcp3911.c > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index 15606f237480..f9a41fa96fcc 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -501,6 +501,16 @@ config MCP3422 > > This driver can also be built as a module. If so, the module will be > > called mcp3422. > > > > +config MCP3911 > > + tristate "Microchip Technology MCP3911 driver" > > + depends on SPI > > + help > > + Say yes here to build support for Microchip Technology's MCP3911 > > + analog to digital converter. > > + > > + This driver can also be built as a module. If so, the module will be > > + called mcp3911. > > + > > config MEDIATEK_MT6577_AUXADC > > tristate "MediaTek AUXADC driver" > > depends on ARCH_MEDIATEK || COMPILE_TEST > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > index 28a9423997f3..3cfebfff7d26 100644 > > --- a/drivers/iio/adc/Makefile > > +++ b/drivers/iio/adc/Makefile > > @@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o > > obj-$(CONFIG_MAX9611) += max9611.o > > obj-$(CONFIG_MCP320X) += mcp320x.o > > obj-$(CONFIG_MCP3422) += mcp3422.o > > +obj-$(CONFIG_MCP3911) += mcp3911.o > > obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o > > obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o > > obj-$(CONFIG_MESON_SARADC) += meson_saradc.o > > diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c > > new file mode 100644 > > index ..29aa39930ead > > --- /dev/null > > +++ b/drivers/iio/adc/mcp3911.c > > @@ -0,0 +1,366 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Driver for Microchip MCP3911, Two-channel Analog Front End > > + * > > + * Copyright (C) 2018 Marcus Folkesson > > + * Copyright (C) 2018 Kent Gustavsson > > + * > > + */ > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define MCP3911_REG_CHANNEL0 0x00 > > +#define MCP3911_REG_CHANNEL1 0x03 > > +#define MCP3911_REG_MOD0x06 > > +#define MCP3911_REG_PHASE 0x07 > > +#define MCP3911_REG_GAIN 0x09 > > + > > +#define MCP3911_REG_STATUSCOM 0x0a > > +#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4) > > +#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3) > > +#define MCP3911_STATUSCOM_EN_OFFCALBIT(2) > > +#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1) > > + > > +#define MCP3911_REG_CONFIG 0x0c > > +#define MCP3911_CONFIG_CLKEXT BIT(1) > > +#define MCP3911_CONFIG_VREFEXT BIT(2) > > + > > +#define MCP3911_REG_OFFCAL_CH0 0x0e > > +#define MCP3911_REG_GAINCAL_CH00x11 > > +#define MCP3911_REG_OFFCAL_CH1 0x14
Re: [PATCH v2 2/3] dt-bindings: iio: adc: add bindings for mcp3911
Hello Rob, Thank you for the review. On Wed, Jul 25, 2018 at 11:51:17AM -0600, Rob Herring wrote: > On Tue, Jul 24, 2018 at 08:30:03PM +0200, Marcus Folkesson wrote: > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > > > Signed-off-by: Marcus Folkesson > > Signed-off-by: Kent Gustavsson > > --- > > > > Notes: > > v2: > > - drop channel width > > - drop `external_vref` > > - replace `external-clock` with proper clock bindings > > > > .../devicetree/bindings/iio/adc/mcp3911.txt| 28 > > ++ > > 1 file changed, 28 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/iio/adc/mcp3911.txt > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/mcp3911.txt > > b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt > > new file mode 100644 > > index ..af5472f51a84 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt > > @@ -0,0 +1,28 @@ > > +* Microchip MCP3911 Dual channel analog front end (ADC) > > + > > +Required properties: > > + - compatible: Should be "microchip,mcp3911" > > + - reg: SPI chip select number for the device > > + > > +Recommended properties: > > + - spi-max-frequency: Definition as per > > +Documentation/devicetree/bindings/spi/spi-bus.txt. > > +Max frequency for this chip is 20MHz. > > + > > +Optional properties: > > + - device-addr: Device address when multiple MCP3911 chips are present on > > the > > + same SPI bus. Valid values are 0-3. Defaults to 0. > > Isn't this the reg value? > The reg value defines which chip select the chip hangs on. The chip has support to connect up to four chips on the same SPI bus, including the same chip select signal. The chips are separated by the device-addr as it is part of the communication protocol. When reading the description for device-addr, I agree that it fits well for the reg property as well.. > > + - vref-supply: Phandle to the external reference voltage supply. > > + - clocks: Phandle and clock identifier (see clock-names) > > + - clock-names: "adc_clk" for the ADC (sampling) clock > > Datasheet calls this clki (or mclk internally). Or just drop clock-names > as it is pointless > when there is only 1 clock. I will drop the clock-names, thanks! > > Also DR handling as an IRQ is missing. I have considered using the DR signal, but as we not support buffering yet I did not see any point in using it. From the datasheet, section 7.1 :: These registers [channel registers] are latched when an ADC read communication occurs. When a data ready event occurs during a read communication, the most current ADC data is also latched to avoid data corruption issues. The three bytes of each channel are updated synchronously at a DRCLK rate. The three bytes can be accessed separately if needed but are refreshed synchronously. So it should be safe to read the values independently of the DR signal. Or maybe I'm missing something. Best regards Marcus Folkesson signature.asc Description: PGP signature
[PATCH v2 1/3] iio: adc: add support for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
Notes:
v2:
- cleanups and bugfixes (thanks Peter Meerwald-Stadler)
- drop hardware gain
- use the presence or lack of regulator to indicate if we go for
internal or external voltage reference
- do not store device node in private struct
- drop support to set width in devicetree
- use the presence or lack of clock to indicate if we go for internal
or external clock
drivers/iio/adc/Kconfig | 10 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mcp3911.c | 366 ++
3 files changed, 377 insertions(+)
create mode 100644 drivers/iio/adc/mcp3911.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 15606f237480..f9a41fa96fcc 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -501,6 +501,16 @@ config MCP3422
This driver can also be built as a module. If so, the module will be
called mcp3422.
+config MCP3911
+ tristate "Microchip Technology MCP3911 driver"
+ depends on SPI
+ help
+ Say yes here to build support for Microchip Technology's MCP3911
+ analog to digital converter.
+
+ This driver can also be built as a module. If so, the module will be
+ called mcp3911.
+
config MEDIATEK_MT6577_AUXADC
tristate "MediaTek AUXADC driver"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 28a9423997f3..3cfebfff7d26 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o
obj-$(CONFIG_MAX9611) += max9611.o
obj-$(CONFIG_MCP320X) += mcp320x.o
obj-$(CONFIG_MCP3422) += mcp3422.o
+obj-$(CONFIG_MCP3911) += mcp3911.o
obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
new file mode 100644
index ..29aa39930ead
--- /dev/null
+++ b/drivers/iio/adc/mcp3911.c
@@ -0,0 +1,366 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Microchip MCP3911, Two-channel Analog Front End
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ * Copyright (C) 2018 Kent Gustavsson
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MCP3911_REG_CHANNEL0 0x00
+#define MCP3911_REG_CHANNEL1 0x03
+#define MCP3911_REG_MOD0x06
+#define MCP3911_REG_PHASE 0x07
+#define MCP3911_REG_GAIN 0x09
+
+#define MCP3911_REG_STATUSCOM 0x0a
+#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4)
+#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3)
+#define MCP3911_STATUSCOM_EN_OFFCALBIT(2)
+#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1)
+
+#define MCP3911_REG_CONFIG 0x0c
+#define MCP3911_CONFIG_CLKEXT BIT(1)
+#define MCP3911_CONFIG_VREFEXT BIT(2)
+
+#define MCP3911_REG_OFFCAL_CH0 0x0e
+#define MCP3911_REG_GAINCAL_CH00x11
+#define MCP3911_REG_OFFCAL_CH1 0x14
+#define MCP3911_REG_GAINCAL_CH10x17
+#define MCP3911_REG_VREFCAL0x1a
+
+#define MCP3911_CHANNEL(x) (MCP3911_REG_CHANNEL0 + x * 3)
+#define MCP3911_OFFCAL(x) (MCP3911_REG_OFFCAL_CH0 + x * 6)
+
+/* Internal voltage reference in uV */
+#define MCP3911_INT_VREF_UV120
+
+#define MCP3911_REG_READ(reg, id) reg) << 1) | ((id) << 5) | (1 <<
0)) & 0xff)
+#define MCP3911_REG_WRITE(reg, id) reg) << 1) | ((id) << 5) | (0 <<
0)) & 0xff)
+
+#define MCP3911_NUM_CHANNELS 2
+
+struct mcp3911 {
+ struct spi_device *spi;
+ struct mutex lock;
+ struct regulator *vref;
+ struct clk *adc_clk;
+ u32 dev_addr;
+};
+
+static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
+{
+ int ret;
+
+ reg = MCP3911_REG_READ(reg, adc->dev_addr);
+ ret = spi_write_then_read(adc->spi, ®, 1, val, len);
+ if (ret < 0)
+ return ret;
+
+ be32_to_cpus(val);
+ *val >>= ((4 - len) * 8);
+ dev_dbg(&adc->spi->dev, "reading 0x%x from register 0x%x\n", *val,
+ reg>>1);
+ return ret;
+}
+
+static int mcp3911_write(struct mcp3911 *adc, u8 reg, u32 val, u8 len)
+{
+ dev_dbg(&adc->spi->dev, "writing 0x%x to register 0x%x\n", val, reg);
+
+ val <<= (3 - len) * 8;
+ cpu_to_be32s(&val);
+ val |= MCP3911_REG_WRITE(reg, adc->dev_addr);
+
+ return spi_write(adc->spi, &va
[PATCH v2 3/3] MAINTAINERS: Add entry for mcp3911 ADC driver
Add an entry for mcp3911 ADC driver and add myself and Kent Gustavsson as maintainers of this driver. Signed-off-by: Marcus Folkesson Signed-off-by: Kent Gustavsson --- Notes: v2: - no changes MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 79bb02ff812f..9276da915d9d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9271,6 +9271,14 @@ L: net...@vger.kernel.org S: Maintained F: drivers/net/ethernet/microchip/lan743x_* +MICROCHIP / ATMEL MCP3911 ADC DRIVER +M: Marcus Folkesson +M: Kent Gustavsson +L: linux-...@vger.kernel.org +S: Supported +F: drivers/iio/adc/mcp3911.c +F: Documentation/devicetree/bindings/iio/adc/mcp3911.txt + MICROCHIP USB251XB DRIVER M: Richard Leitner L: linux-...@vger.kernel.org -- 2.11.0.rc2
[PATCH v2 2/3] dt-bindings: iio: adc: add bindings for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
Notes:
v2:
- drop channel width
- drop `external_vref`
- replace `external-clock` with proper clock bindings
.../devicetree/bindings/iio/adc/mcp3911.txt| 28 ++
1 file changed, 28 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/mcp3911.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
new file mode 100644
index ..af5472f51a84
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
@@ -0,0 +1,28 @@
+* Microchip MCP3911 Dual channel analog front end (ADC)
+
+Required properties:
+ - compatible: Should be "microchip,mcp3911"
+ - reg: SPI chip select number for the device
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 20MHz.
+
+Optional properties:
+ - device-addr: Device address when multiple MCP3911 chips are present on the
+ same SPI bus. Valid values are 0-3. Defaults to 0.
+ - vref-supply: Phandle to the external reference voltage supply.
+ - clocks: Phandle and clock identifier (see clock-names)
+ - clock-names: "adc_clk" for the ADC (sampling) clock
+
+Example:
+adc@0 {
+ compatible = "microchip,mcp3911";
+ reg = <0>;
+ spi-max-frequency = <2000>;
+ device-addr = <0>;
+ vref-supply = <&vref_reg>;
+ clocks = <&xtal>;
+ clock-names = "adc_clk";
+};
--
2.11.0.rc2
Re: [PATCH] input: pxrc - do not store USB device in private struct
Hello Dmitry, On Tue, Jul 24, 2018 at 02:38:04AM +, Dmitry Torokhov wrote: > Hi Marcus, > > On Mon, Jul 16, 2018 at 04:40:14PM +0200, Marcus Folkesson wrote: > > The USB device is only needed during setup, so put it back after > > initialization and do not store it in our private struct. > > > > Also, the USB device is a parent of USB interface so our driver > > model rules ensure that USB device should not disappear while > > interface device is still there. > > So not keep a refcount on the device is safe. > > > > Reported-by: Alexey Khoroshilov > > Signed-off-by: Marcus Folkesson > > --- > > drivers/input/joystick/pxrc.c | 22 -- > > 1 file changed, 12 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/input/joystick/pxrc.c b/drivers/input/joystick/pxrc.c > > index 07a0dbd3ced2..46a7acb747bf 100644 > > --- a/drivers/input/joystick/pxrc.c > > +++ b/drivers/input/joystick/pxrc.c > ... > > > @@ -204,23 +204,25 @@ static int pxrc_probe(struct usb_interface *intf, > > return -ENOMEM; > > > > mutex_init(&pxrc->pm_mutex); > > - pxrc->udev = usb_get_dev(interface_to_usbdev(intf)); > > + udev = usb_get_dev(interface_to_usbdev(intf)); > > There is really no need to "get" device for the probe duration, or in > general, when you are not storing the reference to it. > > I posted series with an updated version of this patch plus couple more > cleanups/fixes, and would appreciate if you could give it a spin. Thank you for doing this. I have reviewed the patchset and tested on real hardware, and it looks good to me. For what it's worth: Reviewed-by: Marcus Folkesson Tested-by: Marcus Folkesson On the whole patchset. > > Thanks. > > -- > Dmitry Best regards Marcus Folkesson signature.asc Description: PGP signature
Re: [PATCH 2/3] dt-bindings: iio: adc: add bindings for mcp3911
Hi Jonathan, On Sun, Jul 22, 2018 at 09:11:11AM +0100, Jonathan Cameron wrote: > On Sat, 21 Jul 2018 21:59:22 +0200 > Marcus Folkesson wrote: > [snip] > > +Optional properties: > > + - device-addr: Device address when multiple MCP3911 chips are present on > > the > > + same SPI bus. Valid values are 0-3. Defaults to 0. > > + - external-clock: Use external clock instead of crystal oscillator. > As mentioned, in the code, can we use the standard fixed clock bindings here. > We don't actually care what the value is, but it might be nice to be able to > power down the clock if we are suspending or something.. > > > + - external-vref: Use external voltage reference > > + - vref-supply:Phandle to the external reference voltage supply. (only > > valid in combination with `external-vref`) > > Just use the optional regulator stuff and get rid of the bool. > > > + - ch0-width: width for channel0. Valid widths are 16 and 24bits. > > + - ch1-width: width for channel1. Valid widths are 16 and 24bits. > > As I asked in the code, are these a function of the wiring etc or are > they something we should really be leaving up to userspace (with a sensible > default). > I agree with all of your comments. I will remove the channel width properties and fix the regulator/clock bindings. Thanks, Best regards Marcus Folkesson signature.asc Description: PGP signature
Re: [PATCH 1/3] iio: adc: add support for mcp3911
Hi Jonathan, Thanks, all good catches. On Sun, Jul 22, 2018 at 09:08:38AM +0100, Jonathan Cameron wrote: > On Sat, 21 Jul 2018 23:19:48 +0200 (CEST) > Peter Meerwald-Stadler wrote: > > > Hello, > > > > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > > > some comments below... > > +CC Mark for the unusual SPI addressing stuff. I'm mostly interested in what > precedent there is for bindings etc. > Yep, I'm not entirely sure that the SPI framework can handle multiple clients on the same CS. The reason why we created device-addr is that the chip supports that and may have hardcoded chip address from factory. The chip address is also part of the protocol so we have to specify it. > > > > > Signed-off-by: Marcus Folkesson > > > Signed-off-by: Kent Gustavsson > > > --- > > > drivers/iio/adc/Kconfig | 10 ++ > > > drivers/iio/adc/Makefile | 1 + > > > drivers/iio/adc/mcp3911.c | 444 > > > ++ > > > 3 files changed, 455 insertions(+) > > > create mode 100644 drivers/iio/adc/mcp3911.c > > > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > > index 15606f237480..f9a41fa96fcc 100644 > > > --- a/drivers/iio/adc/Kconfig > > > +++ b/drivers/iio/adc/Kconfig > > > @@ -501,6 +501,16 @@ config MCP3422 > > > This driver can also be built as a module. If so, the module will be > > > called mcp3422. > > > > > > +config MCP3911 > > > + tristate "Microchip Technology MCP3911 driver" > > > + depends on SPI > > > + help > > > + Say yes here to build support for Microchip Technology's MCP3911 > > > + analog to digital converter. > > > + > > > + This driver can also be built as a module. If so, the module will be > > > + called mcp3911. > > > + > > > config MEDIATEK_MT6577_AUXADC > > > tristate "MediaTek AUXADC driver" > > > depends on ARCH_MEDIATEK || COMPILE_TEST > > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > > index 28a9423997f3..3cfebfff7d26 100644 > > > --- a/drivers/iio/adc/Makefile > > > +++ b/drivers/iio/adc/Makefile > > > @@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o > > > obj-$(CONFIG_MAX9611) += max9611.o > > > obj-$(CONFIG_MCP320X) += mcp320x.o > > > obj-$(CONFIG_MCP3422) += mcp3422.o > > > +obj-$(CONFIG_MCP3911) += mcp3911.o > > > obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o > > > obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o > > > obj-$(CONFIG_MESON_SARADC) += meson_saradc.o > > > diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c > > > new file mode 100644 > > > index ..be74cb15827b > > > --- /dev/null > > > +++ b/drivers/iio/adc/mcp3911.c > > > @@ -0,0 +1,444 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Driver for Microchip MCP3911, Two-channel Analog Front End > > > + * > > > + * Copyright (C) 2018 Marcus Folkesson > > > + * Copyright (C) 2018 Kent Gustavsson > > > + * > > No need for blank line here. > Ok > > > + */ > > > + > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > + > > > +#define MCP3911_REG_CHANNEL0 0x00 > > > +#define MCP3911_REG_CHANNEL1 0x03 > > > +#define MCP3911_REG_MOD 0x06 > > > +#define MCP3911_REG_PHASE0x07 > > > + > > > +#define MCP3911_REG_GAIN 0x09 > > > +#define MCP3911_GAIN_MASK(ch)(0x7 << 3*ch) > > > > space around * operator, maybe parenthesis around variable, i.e > > (0x07 << (3 * (ch))) > > > > > +#define MCP3911_GAIN_VAL(ch, val)((val << 3*ch) & > > > MCP3911_GAIN_MASK(ch)) > > > + > > > +#define MCP3911_REG_STATUSCOM0x0a > > > +#define MCP3911_STATUSCOM_CH1_24WIDTHBIT(4) > > > +#define MCP3911_STATUSCOM_CH0_24WIDTHBIT(3) > > > +#define MCP3911_STATUSCOM_EN_OFFCAL BIT(2) > > > +#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1) > > > + > > > +#define MCP3911_REG_CONFIG 0x0c > > > +#define
Re: [PATCH 1/3] iio: adc: add support for mcp3911
Hi Peter, Thanks for the comments! On Sat, Jul 21, 2018 at 11:19:48PM +0200, Peter Meerwald-Stadler wrote: > Hello, > > > MCP3911 is a dual channel Analog Front End (AFE) containing two > > synchronous sampling delta-sigma Analog-to-Digital Converters (ADC). > > some comments below... > > > Signed-off-by: Marcus Folkesson > > Signed-off-by: Kent Gustavsson > > --- > > drivers/iio/adc/Kconfig | 10 ++ > > drivers/iio/adc/Makefile | 1 + > > drivers/iio/adc/mcp3911.c | 444 > > ++ > > 3 files changed, 455 insertions(+) > > create mode 100644 drivers/iio/adc/mcp3911.c > > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig > > index 15606f237480..f9a41fa96fcc 100644 > > --- a/drivers/iio/adc/Kconfig > > +++ b/drivers/iio/adc/Kconfig > > @@ -501,6 +501,16 @@ config MCP3422 > > This driver can also be built as a module. If so, the module will be > > called mcp3422. > > > > +config MCP3911 > > + tristate "Microchip Technology MCP3911 driver" > > + depends on SPI > > + help > > + Say yes here to build support for Microchip Technology's MCP3911 > > + analog to digital converter. > > + > > + This driver can also be built as a module. If so, the module will be > > + called mcp3911. > > + > > config MEDIATEK_MT6577_AUXADC > > tristate "MediaTek AUXADC driver" > > depends on ARCH_MEDIATEK || COMPILE_TEST > > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile > > index 28a9423997f3..3cfebfff7d26 100644 > > --- a/drivers/iio/adc/Makefile > > +++ b/drivers/iio/adc/Makefile > > @@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o > > obj-$(CONFIG_MAX9611) += max9611.o > > obj-$(CONFIG_MCP320X) += mcp320x.o > > obj-$(CONFIG_MCP3422) += mcp3422.o > > +obj-$(CONFIG_MCP3911) += mcp3911.o > > obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o > > obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o > > obj-$(CONFIG_MESON_SARADC) += meson_saradc.o > > diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c > > new file mode 100644 > > index ..be74cb15827b > > --- /dev/null > > +++ b/drivers/iio/adc/mcp3911.c > > @@ -0,0 +1,444 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Driver for Microchip MCP3911, Two-channel Analog Front End > > + * > > + * Copyright (C) 2018 Marcus Folkesson > > + * Copyright (C) 2018 Kent Gustavsson > > + * > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define MCP3911_REG_CHANNEL0 0x00 > > +#define MCP3911_REG_CHANNEL1 0x03 > > +#define MCP3911_REG_MOD0x06 > > +#define MCP3911_REG_PHASE 0x07 > > + > > +#define MCP3911_REG_GAIN 0x09 > > +#define MCP3911_GAIN_MASK(ch) (0x7 << 3*ch) > > space around * operator, maybe parenthesis around variable, i.e > (0x07 << (3 * (ch))) > Will do > > +#define MCP3911_GAIN_VAL(ch, val) ((val << 3*ch) & MCP3911_GAIN_MASK(ch)) > > + > > +#define MCP3911_REG_STATUSCOM 0x0a > > +#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4) > > +#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3) > > +#define MCP3911_STATUSCOM_EN_OFFCALBIT(2) > > +#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1) > > + > > +#define MCP3911_REG_CONFIG 0x0c > > +#define MCP3911_CONFIG_CLKEXT BIT(1) > > +#define MCP3911_CONFIG_VREFEXT BIT(2) > > + > > +#define MCP3911_REG_OFFCAL_CH0 0x0e > > +#define MCP3911_REG_GAINCAL_CH00x11 > > +#define MCP3911_REG_OFFCAL_CH1 0x14 > > +#define MCP3911_REG_GAINCAL_CH10x17 > > +#define MCP3911_REG_VREFCAL0x1a > > + > > +#define MCP3911_CHANNEL(x) (MCP3911_REG_CHANNEL0 + x * 3) > > +#define MCP3911_OFFCAL(x) (MCP3911_REG_OFFCAL_CH0 + x * 6) > > +#define MCP3911_GAINCAL(x) (MCP3911_REG_GAINCAL_CH0 + x * 6) > > + > > delete newline > Will do > > + > > +/* Internal voltage reference in uV */ > > +#define MCP3911_INT_VREF_UV120 > > + > > +#define REG_READ(reg, id) (((reg << 1) | (id << 5) | (1 << 0)) & 0xff) > > +#define REG_WRITE(reg, id) (((reg << 1) | (
[PATCH 2/3] dt-bindings: iio: adc: add bindings for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
.../devicetree/bindings/iio/adc/mcp3911.txt| 33 ++
1 file changed, 33 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/mcp3911.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
new file mode 100644
index ..e233ee94ad96
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/mcp3911.txt
@@ -0,0 +1,33 @@
+* Microchip MCP3911 Dual channel analog front end (ADC)
+
+Required properties:
+ - compatible: Should be "microchip,mcp3911"
+ - reg: SPI chip select number for the device
+
+Recommended properties:
+ - spi-max-frequency: Definition as per
+Documentation/devicetree/bindings/spi/spi-bus.txt.
+Max frequency for this chip is 20MHz.
+
+Optional properties:
+ - device-addr: Device address when multiple MCP3911 chips are present on the
+ same SPI bus. Valid values are 0-3. Defaults to 0.
+ - external-clock: Use external clock instead of crystal oscillator.
+ - external-vref: Use external voltage reference
+ - vref-supply:Phandle to the external reference voltage supply. (only
valid in combination with `external-vref`)
+ - ch0-width: width for channel0. Valid widths are 16 and 24bits.
+ - ch1-width: width for channel1. Valid widths are 16 and 24bits.
+
+
+Example:
+adc@0 {
+ compatible = "microchip,mcp3911";
+ reg = <0>;
+ spi-max-frequency = <2000>;
+ device-addr = <0>;
+ ch0-width = <16>;
+ ch1-width = <24>;
+ external-clock;
+ external-vref;
+ vref-supply = <&vref_reg>;
+};
--
2.11.0.rc2
[PATCH 3/3] MAINTAINERS: Add entry for mcp3911 ADC driver
Add an entry for mcp3911 ADC driver and add myself and Kent Gustavsson as maintainers of this driver. Signed-off-by: Marcus Folkesson Signed-off-by: Kent Gustavsson --- MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 79bb02ff812f..9276da915d9d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9271,6 +9271,14 @@ L: net...@vger.kernel.org S: Maintained F: drivers/net/ethernet/microchip/lan743x_* +MICROCHIP / ATMEL MCP3911 ADC DRIVER +M: Marcus Folkesson +M: Kent Gustavsson +L: linux-...@vger.kernel.org +S: Supported +F: drivers/iio/adc/mcp3911.c +F: Documentation/devicetree/bindings/iio/adc/mcp3911.txt + MICROCHIP USB251XB DRIVER M: Richard Leitner L: linux-...@vger.kernel.org -- 2.11.0.rc2
[PATCH 1/3] iio: adc: add support for mcp3911
MCP3911 is a dual channel Analog Front End (AFE) containing two
synchronous sampling delta-sigma Analog-to-Digital Converters (ADC).
Signed-off-by: Marcus Folkesson
Signed-off-by: Kent Gustavsson
---
drivers/iio/adc/Kconfig | 10 ++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/mcp3911.c | 444 ++
3 files changed, 455 insertions(+)
create mode 100644 drivers/iio/adc/mcp3911.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 15606f237480..f9a41fa96fcc 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -501,6 +501,16 @@ config MCP3422
This driver can also be built as a module. If so, the module will be
called mcp3422.
+config MCP3911
+ tristate "Microchip Technology MCP3911 driver"
+ depends on SPI
+ help
+ Say yes here to build support for Microchip Technology's MCP3911
+ analog to digital converter.
+
+ This driver can also be built as a module. If so, the module will be
+ called mcp3911.
+
config MEDIATEK_MT6577_AUXADC
tristate "MediaTek AUXADC driver"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 28a9423997f3..3cfebfff7d26 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_MAX1363) += max1363.o
obj-$(CONFIG_MAX9611) += max9611.o
obj-$(CONFIG_MCP320X) += mcp320x.o
obj-$(CONFIG_MCP3422) += mcp3422.o
+obj-$(CONFIG_MCP3911) += mcp3911.o
obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
obj-$(CONFIG_MEN_Z188_ADC) += men_z188_adc.o
obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
diff --git a/drivers/iio/adc/mcp3911.c b/drivers/iio/adc/mcp3911.c
new file mode 100644
index ..be74cb15827b
--- /dev/null
+++ b/drivers/iio/adc/mcp3911.c
@@ -0,0 +1,444 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Microchip MCP3911, Two-channel Analog Front End
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ * Copyright (C) 2018 Kent Gustavsson
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MCP3911_REG_CHANNEL0 0x00
+#define MCP3911_REG_CHANNEL1 0x03
+#define MCP3911_REG_MOD0x06
+#define MCP3911_REG_PHASE 0x07
+
+#define MCP3911_REG_GAIN 0x09
+#define MCP3911_GAIN_MASK(ch) (0x7 << 3*ch)
+#define MCP3911_GAIN_VAL(ch, val) ((val << 3*ch) & MCP3911_GAIN_MASK(ch))
+
+#define MCP3911_REG_STATUSCOM 0x0a
+#define MCP3911_STATUSCOM_CH1_24WIDTH BIT(4)
+#define MCP3911_STATUSCOM_CH0_24WIDTH BIT(3)
+#define MCP3911_STATUSCOM_EN_OFFCALBIT(2)
+#define MCP3911_STATUSCOM_EN_GAINCAL BIT(1)
+
+#define MCP3911_REG_CONFIG 0x0c
+#define MCP3911_CONFIG_CLKEXT BIT(1)
+#define MCP3911_CONFIG_VREFEXT BIT(2)
+
+#define MCP3911_REG_OFFCAL_CH0 0x0e
+#define MCP3911_REG_GAINCAL_CH00x11
+#define MCP3911_REG_OFFCAL_CH1 0x14
+#define MCP3911_REG_GAINCAL_CH10x17
+#define MCP3911_REG_VREFCAL0x1a
+
+#define MCP3911_CHANNEL(x) (MCP3911_REG_CHANNEL0 + x * 3)
+#define MCP3911_OFFCAL(x) (MCP3911_REG_OFFCAL_CH0 + x * 6)
+#define MCP3911_GAINCAL(x) (MCP3911_REG_GAINCAL_CH0 + x * 6)
+
+
+/* Internal voltage reference in uV */
+#define MCP3911_INT_VREF_UV120
+
+#define REG_READ(reg, id) (((reg << 1) | (id << 5) | (1 << 0)) & 0xff)
+#define REG_WRITE(reg, id) (((reg << 1) | (id << 5) | (0 << 0)) & 0xff)
+
+#define MCP3911_NUM_CHANNELS 2
+
+
+struct mcp3911 {
+ struct spi_device *spi;
+ struct device_node *np;
+ struct mutex lock;
+
+ u32 gain[MCP3911_NUM_CHANNELS];
+ u32 width[MCP3911_NUM_CHANNELS];
+
+ u32 dev_addr;
+ bool vrefext;
+ struct regulator *vref;
+};
+
+static int mcp3911_read(struct mcp3911 *adc, u8 reg, u32 *val, u8 len)
+{
+ int ret;
+
+ reg = REG_READ(reg, adc->dev_addr);
+ ret = spi_write_then_read(adc->spi, ®, 1, val, len);
+ if (ret < 0)
+ return ret;
+
+ *val <<= ((4-len)*8);
+ be32_to_cpus(val);
+ dev_dbg(&adc->spi->dev, "Reading 0x%x from register 0x%x\n", *val,
+ reg>>1);
+ return ret;
+}
+
+static int mcp3911_write(struct mcp3911 *adc, u8 reg, u32 val, u8 len)
+{
+ dev_dbg(&adc->spi->dev, "Writing 0x%x to register 0x%x\n", val, reg);
+
+ cpu_to_be32s(&val);
+ val >>= (3-len)*8;
+ val |= REG_WRITE(reg, adc->dev_addr);
+
+ return spi_write(adc->spi, &val, len+1);
+}
+
+static int mcp3911_update(struct mcp3911 *adc, u8 reg, u32 mask,
+ u32 val, u8 len)
+{
+ u32 tmp;
+ int
[PATCH] input: pxrc - do not store USB device in private struct
The USB device is only needed during setup, so put it back after
initialization and do not store it in our private struct.
Also, the USB device is a parent of USB interface so our driver
model rules ensure that USB device should not disappear while
interface device is still there.
So not keep a refcount on the device is safe.
Reported-by: Alexey Khoroshilov
Signed-off-by: Marcus Folkesson
---
drivers/input/joystick/pxrc.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/input/joystick/pxrc.c b/drivers/input/joystick/pxrc.c
index 07a0dbd3ced2..46a7acb747bf 100644
--- a/drivers/input/joystick/pxrc.c
+++ b/drivers/input/joystick/pxrc.c
@@ -27,7 +27,6 @@ MODULE_DEVICE_TABLE(usb, pxrc_table);
struct pxrc {
struct input_dev*input;
- struct usb_device *udev;
struct usb_interface*intf;
struct urb *urb;
struct mutexpm_mutex;
@@ -120,7 +119,7 @@ static void pxrc_close(struct input_dev *input)
mutex_unlock(&pxrc->pm_mutex);
}
-static int pxrc_usb_init(struct pxrc *pxrc)
+static int pxrc_usb_init(struct pxrc *pxrc, struct usb_device *udev)
{
struct usb_endpoint_descriptor *epirq;
unsigned int pipe;
@@ -145,7 +144,7 @@ static int pxrc_usb_init(struct pxrc *pxrc)
}
usb_set_intfdata(pxrc->intf, pxrc);
- usb_make_path(pxrc->udev, pxrc->phys, sizeof(pxrc->phys));
+ usb_make_path(udev, pxrc->phys, sizeof(pxrc->phys));
strlcat(pxrc->phys, "/input0", sizeof(pxrc->phys));
pxrc->urb = usb_alloc_urb(0, GFP_KERNEL);
@@ -154,8 +153,8 @@ static int pxrc_usb_init(struct pxrc *pxrc)
goto error;
}
- pipe = usb_rcvintpipe(pxrc->udev, pxrc->epaddr),
- usb_fill_int_urb(pxrc->urb, pxrc->udev, pipe, pxrc->data, pxrc->bsize,
+ pipe = usb_rcvintpipe(udev, pxrc->epaddr),
+ usb_fill_int_urb(pxrc->urb, udev, pipe, pxrc->data, pxrc->bsize,
pxrc_usb_irq, pxrc, 1);
error:
@@ -164,7 +163,7 @@ static int pxrc_usb_init(struct pxrc *pxrc)
}
-static int pxrc_input_init(struct pxrc *pxrc)
+static int pxrc_input_init(struct pxrc *pxrc, struct usb_device *udev)
{
pxrc->input = devm_input_allocate_device(&pxrc->intf->dev);
if (pxrc->input == NULL) {
@@ -174,7 +173,7 @@ static int pxrc_input_init(struct pxrc *pxrc)
pxrc->input->name = "PXRC Flight Controller Adapter";
pxrc->input->phys = pxrc->phys;
- usb_to_input_id(pxrc->udev, &pxrc->input->id);
+ usb_to_input_id(udev, &pxrc->input->id);
pxrc->input->open = pxrc_open;
pxrc->input->close = pxrc_close;
@@ -197,6 +196,7 @@ static int pxrc_probe(struct usb_interface *intf,
const struct usb_device_id *id)
{
struct pxrc *pxrc;
+ struct usb_device *udev;
int retval;
pxrc = devm_kzalloc(&intf->dev, sizeof(*pxrc), GFP_KERNEL);
@@ -204,23 +204,25 @@ static int pxrc_probe(struct usb_interface *intf,
return -ENOMEM;
mutex_init(&pxrc->pm_mutex);
- pxrc->udev = usb_get_dev(interface_to_usbdev(intf));
+ udev = usb_get_dev(interface_to_usbdev(intf));
pxrc->intf = intf;
- retval = pxrc_usb_init(pxrc);
+ retval = pxrc_usb_init(pxrc, udev);
if (retval)
goto error;
- retval = pxrc_input_init(pxrc);
+ retval = pxrc_input_init(pxrc, udev);
if (retval)
goto err_free_urb;
+ usb_put_dev(udev);
return 0;
err_free_urb:
usb_free_urb(pxrc->urb);
error:
+ usb_put_dev(udev);
return retval;
}
--
2.18.0
Re: [PATCH] Input: pxrc - fix leak of usb_device
On Sat, Jul 14, 2018 at 08:51:09AM +, Dmitry Torokhov wrote: > On Sat, Jul 14, 2018 at 10:09:20AM +0200, Marcus Folkesson wrote: > > Hi Alexey, > > > > Good catch! > > > > On Fri, Jul 13, 2018 at 11:07:57PM +0300, Alexey Khoroshilov wrote: > > > pxrc_probe() calls usb_get_dev(), but there is no usb_put_dev() > > > anywhere in the driver. > > > > > > The patch adds one to error handling code and to pxrc_disconnect(). > > > > > > Found by Linux Driver Verification project (linuxtesting.org). > > > > > > Signed-off-by: Alexey Khoroshilov > > > > Reviewed-by: Marcus Folkesson > > Hmm, the biggest question however if we need to "take" the device, as I > do not think interface can outlive the device, and whether we actually > need to store it in pxrc, as we only need it during set up, as far as I > can see. Yep, the device is only used during setup. I interpret the comments for usb_get_dev() as you should take a reference count on the device even if you only use the interface, but I could be wrong. >From usb_get_dev():: * usb_get_dev - increments the reference count of the usb device structure * @dev: the device being referenced * * Each live reference to a device should be refcounted. * * Drivers for USB interfaces should normally record such references in * their probe() methods, when they bind to an interface, and release * them by calling usb_put_dev(), in their disconnect() methods. I can fix the driver to not take the device if that is what we want. If not Alexey want to fix it of course, it is his catch :-) > > Thanks. > > -- > Dmitry Best regards Marcus Folkesson
Re: [PATCH] Input: pxrc - fix leak of usb_device
Hi Alexey, Good catch! On Fri, Jul 13, 2018 at 11:07:57PM +0300, Alexey Khoroshilov wrote: > pxrc_probe() calls usb_get_dev(), but there is no usb_put_dev() > anywhere in the driver. > > The patch adds one to error handling code and to pxrc_disconnect(). > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Alexey Khoroshilov Reviewed-by: Marcus Folkesson > --- > drivers/input/joystick/pxrc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/input/joystick/pxrc.c b/drivers/input/joystick/pxrc.c > index 07a0dbd3ced2..0a31de63ac8e 100644 > --- a/drivers/input/joystick/pxrc.c > +++ b/drivers/input/joystick/pxrc.c > @@ -221,6 +221,7 @@ static int pxrc_probe(struct usb_interface *intf, > usb_free_urb(pxrc->urb); > > error: > + usb_put_dev(pxrc->udev); > return retval; > } > > @@ -229,6 +230,7 @@ static void pxrc_disconnect(struct usb_interface *intf) > struct pxrc *pxrc = usb_get_intfdata(intf); > > usb_free_urb(pxrc->urb); > + usb_put_dev(pxrc->udev); > usb_set_intfdata(intf, NULL); > } > > -- > 2.7.4 >
[PATCH v4 2/3] Documentation: usb: add documentation for USB CCID Gadget Device
Add documentation to give a brief description on how to use the CCID Gadget Device. This includes a description for all attributes followed by an example on how to setup the device with ConfigFS. Reviewed-by: Randy Dunlap Signed-off-by: Marcus Folkesson --- Notes: v4: - clarify how to use /dev/ccidg and BULK endpoind (thanks Randy) v3: - correct the grammer (thanks Randy) v2: - add the missing changelog text Documentation/usb/gadget_ccid.rst | 269 ++ 1 file changed, 269 insertions(+) create mode 100644 Documentation/usb/gadget_ccid.rst diff --git a/Documentation/usb/gadget_ccid.rst b/Documentation/usb/gadget_ccid.rst new file mode 100644 index ..c8f25080c9e7 --- /dev/null +++ b/Documentation/usb/gadget_ccid.rst @@ -0,0 +1,269 @@ +.. SPDX-License-Identifier: GPL-2.0 + + +CCID Gadget + + +:Author: Marcus Folkesson + +Introduction + + +The CCID Gadget will present itself as a CCID device to the host system. +The device supports two endpoints for now; BULK IN and BULK OUT. +These endpoints are exposed to userspace via /dev/ccidgX where `X` is +the device enumeration number. +Use read(2) and write(3) to operate on BULK_IN and BULK_OUT respectively. + +All CCID commands are sent on the BULK-OUT endpoint. Each command sent to the CCID +has an associated ending response. Some commands can also have intermediate +responses. The response is sent on the BULK-IN endpoint. +See Figure 3-3 in the CCID Specification [1]_ for more details. + +The CCID commands must be handled in userspace since the driver is only working +as a transport layer for the TPDUs. + + +CCID Commands +-- + +All CCID commands begins with a 10-byte header followed by an optional +data field depending on message type. + +++--+---+--+ +| Offset | Field| Size | Description | +++==+===+==+ +| 0 | bMessageType | 1 | Type of message | +++--+---+--+ +| 1 | dwLength | 4 | Message specific data length | +|| | | | +++--+---+--+ +| 5 | bSlot| 1 | Identifies the slot number | +|| | | for this command | +++--+---+--+ +| 6 | bSeq | 1 | Sequence number for command | +++--+---+--+ +| 7 | ... | 3 | Fields depends on message type | +++--+---+--+ +| 10 | abData | array | Message specific data (OPTIONAL) | +++--+---+--+ + + +Multiple CCID gadgets +-- + +It is possible to create multiple instances of the CCID gadget, however, +a much more flexible way is to create one gadget and set the `nslots` attribute +to the number of desired CCID devices. + +All CCID commands specify which slot is the receiver in the `bSlot` field +of the CCID header. + +Usage += + +Access from userspace +-- +All communication is by read(2) and write(2) to the corresponding /dev/ccidg* device. +Only one file descriptor is allowed to be open to the device at a time. + +The buffer size provided to read(2) **must be at least** 522 (10 bytes header + 512 bytes payload) +bytes as we are working with whole commands. + +The buffer size provided to write(2) **may not exceed** 522 (10 bytes header + 512 bytes payload) +bytes as we are working with whole commands. + + +Configuration with configfs + + +ConfigFS is used to create and configure the CCID gadget. +In order to get a device to work as intended, a few attributes must +be considered. + +The attributes are described below followed by an example. + +features +~ + +The `feature` attribute writes to the dwFeatures field in the class descriptor. +See Table 5.1-1 Smart Card Device Descriptors in the CCID Specification [1]_. + +The value indicates what intelligent features the CCID has. +These values are available to user application as defined in ccid.h [2]_. +The default value is 0x. + +The value is a bitwise OR operation performed on the following values: + ++++ +| Value | Description| ++++ +| 0x | No special characteristics
[PATCH v4 3/3] MAINTAINERS: add USB CCID Gadget Device
Add MAINTAINERS entry for USB CCID Gadget Device Signed-off-by: Marcus Folkesson --- Notes: v4: - No changes v3: - No changes v2: - No changes MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 078fd80f664f..e77c3d2bec89 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14541,6 +14541,14 @@ L: linux-s...@vger.kernel.org S: Maintained F: drivers/usb/storage/uas.c +USB CCID GADGET +M: Marcus Folkesson +L: linux-...@vger.kernel.org +S: Maintained +F: drivers/usb/gadget/function/f_ccid.* +F: include/uapi/linux/usb/ccid.h +F: Documentation/usb/gadget_ccid.rst + USB CDC ETHERNET DRIVER M: Oliver Neukum L: linux-...@vger.kernel.org -- 2.18.0
[RESEND PATCH v2] MAINTAINERS: add PhoenixRC Flight Controller Adapter
Add MAINTAINERS entry for PhoenixRC Flight Controller Adapter Signed-off-by: Marcus Folkesson --- Notes: v2: Add proper mailing list MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9ce5062dc028..d8b5d9f810e4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11103,6 +11103,13 @@ S: Maintained F: include/linux/personality.h F: include/uapi/linux/personality.h +PHOENIX RC FLIGHT CONTROLLER ADAPTER +M: Marcus Folkesson +L: linux-in...@vger.kernel.org +S: Maintained +F: Documentation/input/devices/pxrc.rst +F: drivers/input/joystick/pxrc.c + PHONET PROTOCOL M: Remi Denis-Courmont S: Supported -- 2.16.2
Re: [PATCH v2 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
Hi Andrzej, Thank you for reviewing. On Mon, May 28, 2018 at 11:12:27AM +0200, Andrzej Pietrasiewicz wrote: > W dniu 28.05.2018 o 10:38, Marcus Folkesson pisze: > > Hi Andrzej, > > > > On Mon, May 28, 2018 at 09:04:51AM +0200, Andrzej Pietrasiewicz wrote: > >> Mi Marcus, > >> > >> W dniu 26.05.2018 o 23:19, Marcus Folkesson pisze: > >>> Chip Card Interface Device (CCID) protocol is a USB protocol that > >>> allows a smartcard device to be connected to a computer via a card > >>> reader using a standard USB interface, without the need for each > >>> manufacturer > >>> of smartcards to provide its own reader or protocol. > >>> > >>> This gadget driver makes Linux show up as a CCID device to the host and > >>> let a > >>> userspace daemon act as the smartcard. > >>> > >>> This is useful when the Linux gadget itself should act as a cryptographic > >>> device or forward APDUs to an embedded smartcard device. > >>> > >>> Signed-off-by: Marcus Folkesson > >>> --- > >> > >>> > >>> +config USB_CONFIGFS_CCID > >>> + bool "Chip Card Interface Device (CCID)" > >>> + depends on USB_CONFIGFS > >>> + select USB_F_CCID > >>> + help > >>> + The CCID function driver provides generic emulation of a > >>> + Chip Card Interface Device (CCID). > >>> + > >>> + You will need a user space server talking to /dev/ccidg*, > >>> + since the kernel itself does not implement CCID/TPDU/APDU > >>> + protocol. > >> > >> Your function needs a userspace daemon to work. > >> It seems you want to use FunctionFS for such a purpose > >> instead of creating a new function. > >> > >> Andrzej > > > >>> + since the kernel itself does not implement CCID/TPDU/APDU > > Oops, the driver does handle CCID. > > Which parts of code do this handling? My bad, I was thinking about the USB descriptors and endpoints setup. That is of cause not part of the CCID protocol. > > Is there any kind of state machine usual for protocols? > If the protocol is stateless then isn't it just a data format then? The protocol is stateless. > > Which part of this handling must be done in kernel and why? > > Does the said handling do anything other than forwarding the > traffic between USB and a character device? No, it forward the CCID messages to the character device to be handled by the application. > > What is the character device used for? I know: read, write and poll. > But why? To do what? It is used for the application to fetch, interpret and then perform actions depending on commands. > > > > > Well, yes, It needs an application that perform the "smartcard operations", > > such as > > generate keys or sign data, as this depends on how it should be used. > > > > The actual smartcard operations could for example be in software, > > use a crypto engine in SoC or external HSM (Hardware Security Module). > > > > Without the application, the gadget shows up as a smart card reader > > with an unconnected smartcard. > > > > Does showing up as anything require anything other than merely > providing USB descriptors? I guess. > > Andrzej Thank you, Marcus
Re: [PATCH v2 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
Hi Andrzej, On Mon, May 28, 2018 at 09:04:51AM +0200, Andrzej Pietrasiewicz wrote: > Mi Marcus, > > W dniu 26.05.2018 o 23:19, Marcus Folkesson pisze: > > Chip Card Interface Device (CCID) protocol is a USB protocol that > > allows a smartcard device to be connected to a computer via a card > > reader using a standard USB interface, without the need for each > > manufacturer > > of smartcards to provide its own reader or protocol. > > > > This gadget driver makes Linux show up as a CCID device to the host and let > > a > > userspace daemon act as the smartcard. > > > > This is useful when the Linux gadget itself should act as a cryptographic > > device or forward APDUs to an embedded smartcard device. > > > > Signed-off-by: Marcus Folkesson > > --- > > > > > +config USB_CONFIGFS_CCID > > + bool "Chip Card Interface Device (CCID)" > > + depends on USB_CONFIGFS > > + select USB_F_CCID > > + help > > + The CCID function driver provides generic emulation of a > > + Chip Card Interface Device (CCID). > > + > > + You will need a user space server talking to /dev/ccidg*, > > + since the kernel itself does not implement CCID/TPDU/APDU > > + protocol. > > Your function needs a userspace daemon to work. > It seems you want to use FunctionFS for such a purpose > instead of creating a new function. > > Andrzej > > + since the kernel itself does not implement CCID/TPDU/APDU Oops, the driver does handle CCID. Well, yes, It needs an application that perform the "smartcard operations", such as generate keys or sign data, as this depends on how it should be used. The actual smartcard operations could for example be in software, use a crypto engine in SoC or external HSM (Hardware Security Module). Without the application, the gadget shows up as a smart card reader with an unconnected smartcard. I guess it could be accomplished with FunctionFS as well. Best regards Marcus Folkesson
Re: [PATCH v2 2/3] Documentation: usb: add documentation for USB CCID Gadget Device
Hi Randy, On Sun, May 27, 2018 at 04:36:24PM -0700, Randy Dunlap wrote: > Hi, > > I have a few documentation comments below... > > On 05/26/2018 02:19 PM, Marcus Folkesson wrote: > > Add documentation to give a brief description on how to use the > > CCID Gadget Device. > > This includes a description for all attributes followed by an example on > > how to setup the device with ConfigFS. > > > > Signed-off-by: Marcus Folkesson > > --- > > Documentation/usb/gadget_ccid.rst | 267 > > ++ > > 1 file changed, 267 insertions(+) > > create mode 100644 Documentation/usb/gadget_ccid.rst > > > > diff --git a/Documentation/usb/gadget_ccid.rst > > b/Documentation/usb/gadget_ccid.rst > > new file mode 100644 > > index ..5ac806b14604 > > --- /dev/null > > +++ b/Documentation/usb/gadget_ccid.rst > > @@ -0,0 +1,267 @@ > > +.. SPDX-License-Identifier: GPL-2.0 > > + > > + > > +CCID Gadget > > + > > + > > +:Author: Marcus Folkesson > > + > > +Introduction > > + > > + > > +The CCID Gadget will present itself as a CCID device to the host system. > > +The device supports two endpoints for now; BULK IN and BULK OUT. > > +These endpoints is exposed to userspace via /dev/ccidg*. > >are exposed > > > + > > +All CCID commands are sent on the BULK-OUT endpoint. Each command sent to > > the CCID > > +has an associated ending response. Some commands can also have intermediate > > +responses. The response is sent on the BULK-IN endpoint. > > +See Figure 3-3 in the CCID Specification [1]_ for more details. > > + > > +The CCID commands must be handled in userspace since the driver is only > > working > > +as a transport layer for the TPDUs. > > + > > + > > +CCID Commands > > +-- > > + > > +All CCID commands begins with a 10 bytes header followed by an optional > > with a 10-byte header > (or maybe that's a locale difference) > > > +data field depending on message type. > > + > > +++--+---+--+ > > +| Offset | Field| Size | Description | > > +++==+===+==+ > > +| 0 | bMessageType | 1 | Type of message | > > +++--+---+--+ > > +| 1 | dwLength | 4 | Message specific data length | > > +|| | | | > > +++--+---+--+ > > +| 5 | bSlot| 1 | Identifies the slot number | > > +|| | | for this command | > > +++--+---+--+ > > +| 6 | bSeq | 1 | Sequence number for command | > > +++--+---+--+ > > +| 7 | ... | 3 | Fields depends on message type | > > +++--+---+--+ > > +| 10 | abData | array | Message specific data (OPTIONAL) | > > +++--+---+--+ > > + > > + > > +Multiple CCID gadgets > > +-- > > + > > +It is possible to create multiple instances of the CCID gadget, however, > > +a much more flexible way is to create one gadget and set the `nslots` > > attribute > > +to the number of desired CCID devices. > > + > > +All CCID commands specifies which slot that is the receiver in the `bSlot` > > field > > specify which slot is the receiver > > > +of the CCID header. > > + > > +Usage > > += > > + > > +Access from userspace > > +-- > > +All communication is by read(2) and write(2) to the corresponding > > /dev/ccidg* device. > > +Only one filedescriptor is allowed to be open to the device at a time. > > file descriptor > > > + > > +The buffer size provided to read(2) **must be at least** 522 (10 bytes > > header + 512 bytes payload) > > +bytes as we are working with whole commands. > > + > > +The buffer size provided to write(2) **may not exceed** 522 (10 bytes > > header + 512 bytes payload) > > +byte
Re: [PATCH 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
On Sat, May 26, 2018 at 10:56:52PM +0200, Greg Kroah-Hartman wrote: > On Sat, May 26, 2018 at 10:33:59PM +0200, Marcus Folkesson wrote: > > Signed-off-by: Marcus Folkesson > > I can't take patches without any changelog text. And why would you > submit a patch over 1000 lines without any? I'm sorry, obviously not much went well for me yesterday. The changlelog was "fixup:ed" away and now I noticed that my patch version history did not follow for v2. Is it better to send a v3 with updated version history or wait for more comments? I'll start review my patches better before sending them. Thank you for your time and comments. > > Please fix. > > thanks, > > greg k-h
[PATCH v2 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
Chip Card Interface Device (CCID) protocol is a USB protocol that
allows a smartcard device to be connected to a computer via a card
reader using a standard USB interface, without the need for each manufacturer
of smartcards to provide its own reader or protocol.
This gadget driver makes Linux show up as a CCID device to the host and let a
userspace daemon act as the smartcard.
This is useful when the Linux gadget itself should act as a cryptographic
device or forward APDUs to an embedded smartcard device.
Signed-off-by: Marcus Folkesson
---
drivers/usb/gadget/Kconfig | 17 +
drivers/usb/gadget/function/Makefile | 1 +
drivers/usb/gadget/function/f_ccid.c | 988 +++
drivers/usb/gadget/function/f_ccid.h | 91
include/uapi/linux/usb/ccid.h| 93
5 files changed, 1190 insertions(+)
create mode 100644 drivers/usb/gadget/function/f_ccid.c
create mode 100644 drivers/usb/gadget/function/f_ccid.h
create mode 100644 include/uapi/linux/usb/ccid.h
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 31cce7805eb2..bdebdf1ffa2b 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -149,6 +149,9 @@ config USB_LIBCOMPOSITE
config USB_F_ACM
tristate
+config USB_F_CCID
+ tristate
+
config USB_F_SS_LB
tristate
@@ -248,6 +251,20 @@ config USB_CONFIGFS_ACM
ACM serial link. This function can be used to interoperate with
MS-Windows hosts or with the Linux-USB "cdc-acm" driver.
+config USB_CONFIGFS_CCID
+ bool "Chip Card Interface Device (CCID)"
+ depends on USB_CONFIGFS
+ select USB_F_CCID
+ help
+ The CCID function driver provides generic emulation of a
+ Chip Card Interface Device (CCID).
+
+ You will need a user space server talking to /dev/ccidg*,
+ since the kernel itself does not implement CCID/TPDU/APDU
+ protocol.
+
+ For more information, see Documentation/usb/gadget_ccid.rst.
+
config USB_CONFIGFS_OBEX
bool "Object Exchange Model (CDC OBEX)"
depends on USB_CONFIGFS
diff --git a/drivers/usb/gadget/function/Makefile
b/drivers/usb/gadget/function/Makefile
index 5d3a6cf02218..629851009e1a 100644
--- a/drivers/usb/gadget/function/Makefile
+++ b/drivers/usb/gadget/function/Makefile
@@ -9,6 +9,7 @@ ccflags-y +=
-I$(srctree)/drivers/usb/gadget/udc/
# USB Functions
usb_f_acm-y:= f_acm.o
obj-$(CONFIG_USB_F_ACM)+= usb_f_acm.o
+obj-$(CONFIG_USB_F_CCID) += f_ccid.o
usb_f_ss_lb-y := f_loopback.o f_sourcesink.o
obj-$(CONFIG_USB_F_SS_LB) += usb_f_ss_lb.o
obj-$(CONFIG_USB_U_SERIAL) += u_serial.o
diff --git a/drivers/usb/gadget/function/f_ccid.c
b/drivers/usb/gadget/function/f_ccid.c
new file mode 100644
index ..9ff8615ca303
--- /dev/null
+++ b/drivers/usb/gadget/function/f_ccid.c
@@ -0,0 +1,988 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * f_ccid.c -- Chip Card Interface Device (CCID) function Driver
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "f_ccid.h"
+#include "u_f.h"
+
+/* Number of tx requests to allocate */
+#define N_TX_REQS 4
+
+/* Maximum number of devices */
+#define CCID_MINORS 4
+
+struct ccidg_bulk_dev {
+ atomic_t is_open;
+ atomic_t rx_req_busy;
+ wait_queue_head_t read_wq;
+ wait_queue_head_t write_wq;
+ struct usb_request *rx_req;
+ atomic_t rx_done;
+ struct list_head tx_idle;
+};
+
+struct f_ccidg {
+ struct usb_function_instancefunc_inst;
+ struct usb_function function;
+ spinlock_t lock;
+ atomic_t online;
+
+ /* Character device */
+ struct cdev cdev;
+ int minor;
+
+ /* Dynamic attributes */
+ u32 features;
+ u32 protocols;
+ u8 pinsupport;
+ u8 nslots;
+ u8 lcdlayout;
+
+ /* Endpoints */
+ struct usb_ep *in;
+ struct usb_ep *out;
+ struct ccidg_bulk_dev bulk_dev;
+};
+
+/* Interface Descriptor: */
+static struct usb_interface_descriptor ccid_interface_desc = {
+ .bLength = USB_DT_INTERFACE_SIZE,
+ .bDescriptorType = USB_DT_INTERFACE,
+ .bNumEndpoints =2,
+ .bInterfaceClass = USB_CLASS_CSCID,
+ .bInterfaceSubClass = 0,
+ .bInterfaceProtocol = 0,
+};
+
+/* CCID Class Descriptor */
+static struct ccid_class_descriptor ccid_class_desc = {
+ .bLength = sizeof(ccid_class_desc),
+ .bDescriptorType = CCID_DECRIPTOR_TYPE,
+ .bcdCCID = CCID1_10,
+ /* .bMaxSlotIndex = DYNAMIC */
+ .bVoltageSupport = CCID_VOLTS_3_0,
+ /* .dwProtocols = DYNAMIC */
+ .dwDefaultClock
[PATCH v2 2/3] Documentation: usb: add documentation for USB CCID Gadget Device
Add documentation to give a brief description on how to use the CCID Gadget Device. This includes a description for all attributes followed by an example on how to setup the device with ConfigFS. Signed-off-by: Marcus Folkesson --- Documentation/usb/gadget_ccid.rst | 267 ++ 1 file changed, 267 insertions(+) create mode 100644 Documentation/usb/gadget_ccid.rst diff --git a/Documentation/usb/gadget_ccid.rst b/Documentation/usb/gadget_ccid.rst new file mode 100644 index ..5ac806b14604 --- /dev/null +++ b/Documentation/usb/gadget_ccid.rst @@ -0,0 +1,267 @@ +.. SPDX-License-Identifier: GPL-2.0 + + +CCID Gadget + + +:Author: Marcus Folkesson + +Introduction + + +The CCID Gadget will present itself as a CCID device to the host system. +The device supports two endpoints for now; BULK IN and BULK OUT. +These endpoints is exposed to userspace via /dev/ccidg*. + +All CCID commands are sent on the BULK-OUT endpoint. Each command sent to the CCID +has an associated ending response. Some commands can also have intermediate +responses. The response is sent on the BULK-IN endpoint. +See Figure 3-3 in the CCID Specification [1]_ for more details. + +The CCID commands must be handled in userspace since the driver is only working +as a transport layer for the TPDUs. + + +CCID Commands +-- + +All CCID commands begins with a 10 bytes header followed by an optional +data field depending on message type. + +++--+---+--+ +| Offset | Field| Size | Description | +++==+===+==+ +| 0 | bMessageType | 1 | Type of message | +++--+---+--+ +| 1 | dwLength | 4 | Message specific data length | +|| | | | +++--+---+--+ +| 5 | bSlot| 1 | Identifies the slot number | +|| | | for this command | +++--+---+--+ +| 6 | bSeq | 1 | Sequence number for command | +++--+---+--+ +| 7 | ... | 3 | Fields depends on message type | +++--+---+--+ +| 10 | abData | array | Message specific data (OPTIONAL) | +++--+---+--+ + + +Multiple CCID gadgets +-- + +It is possible to create multiple instances of the CCID gadget, however, +a much more flexible way is to create one gadget and set the `nslots` attribute +to the number of desired CCID devices. + +All CCID commands specifies which slot that is the receiver in the `bSlot` field +of the CCID header. + +Usage += + +Access from userspace +-- +All communication is by read(2) and write(2) to the corresponding /dev/ccidg* device. +Only one filedescriptor is allowed to be open to the device at a time. + +The buffer size provided to read(2) **must be at least** 522 (10 bytes header + 512 bytes payload) +bytes as we are working with whole commands. + +The buffer size provided to write(2) **may not exceed** 522 (10 bytes header + 512 bytes payload) +bytes as we are working with whole commands. + + +Configuration with configfs + + +ConfigFS is used to create and configure the CCID gadget. +In order to get a device to work as intended, a few attributes must +be considered. + +The attributes is described below followed by an example. + +features +~ + +The `feature` attribute writes to the dwFeatures field in the class descriptor. +See Table 5.1-1 Smart Card Device Descriptors in the CCID Specification [1]_. + +The value indicates what intelligent features the CCID has. +These values are available to user application as defines in ccid.h [2]_. +The default value is 0x. + +The value is a bitwise OR operation performed on the following values: + ++++ +| Value | Description| ++++ +| 0x | No special characteristics | ++++ +| 0x0002 | Automatic parameter configuration based on ATR data| ++++ +| 0x0004 | Automatic activation of ICC on inserting
[PATCH v2 3/3] MAINTAINERS: add USB CCID Gadget Device
Add MAINTAINERS entry for USB CCID Gadget Device Signed-off-by: Marcus Folkesson --- MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 078fd80f664f..e77c3d2bec89 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14541,6 +14541,14 @@ L: linux-s...@vger.kernel.org S: Maintained F: drivers/usb/storage/uas.c +USB CCID GADGET +M: Marcus Folkesson +L: linux-...@vger.kernel.org +S: Maintained +F: drivers/usb/gadget/function/f_ccid.* +F: include/uapi/linux/usb/ccid.h +F: Documentation/usb/gadget_ccid.rst + USB CDC ETHERNET DRIVER M: Oliver Neukum L: linux-...@vger.kernel.org -- 2.16.2
[PATCH 2/3] Documentation: usb: add documentation for USB CCID Gadget Device
Signed-off-by: Marcus Folkesson --- Documentation/usb/gadget_ccid.rst | 267 ++ 1 file changed, 267 insertions(+) create mode 100644 Documentation/usb/gadget_ccid.rst diff --git a/Documentation/usb/gadget_ccid.rst b/Documentation/usb/gadget_ccid.rst new file mode 100644 index ..5ac806b14604 --- /dev/null +++ b/Documentation/usb/gadget_ccid.rst @@ -0,0 +1,267 @@ +.. SPDX-License-Identifier: GPL-2.0 + + +CCID Gadget + + +:Author: Marcus Folkesson + +Introduction + + +The CCID Gadget will present itself as a CCID device to the host system. +The device supports two endpoints for now; BULK IN and BULK OUT. +These endpoints is exposed to userspace via /dev/ccidg*. + +All CCID commands are sent on the BULK-OUT endpoint. Each command sent to the CCID +has an associated ending response. Some commands can also have intermediate +responses. The response is sent on the BULK-IN endpoint. +See Figure 3-3 in the CCID Specification [1]_ for more details. + +The CCID commands must be handled in userspace since the driver is only working +as a transport layer for the TPDUs. + + +CCID Commands +-- + +All CCID commands begins with a 10 bytes header followed by an optional +data field depending on message type. + +++--+---+--+ +| Offset | Field| Size | Description | +++==+===+==+ +| 0 | bMessageType | 1 | Type of message | +++--+---+--+ +| 1 | dwLength | 4 | Message specific data length | +|| | | | +++--+---+--+ +| 5 | bSlot| 1 | Identifies the slot number | +|| | | for this command | +++--+---+--+ +| 6 | bSeq | 1 | Sequence number for command | +++--+---+--+ +| 7 | ... | 3 | Fields depends on message type | +++--+---+--+ +| 10 | abData | array | Message specific data (OPTIONAL) | +++--+---+--+ + + +Multiple CCID gadgets +-- + +It is possible to create multiple instances of the CCID gadget, however, +a much more flexible way is to create one gadget and set the `nslots` attribute +to the number of desired CCID devices. + +All CCID commands specifies which slot that is the receiver in the `bSlot` field +of the CCID header. + +Usage += + +Access from userspace +-- +All communication is by read(2) and write(2) to the corresponding /dev/ccidg* device. +Only one filedescriptor is allowed to be open to the device at a time. + +The buffer size provided to read(2) **must be at least** 522 (10 bytes header + 512 bytes payload) +bytes as we are working with whole commands. + +The buffer size provided to write(2) **may not exceed** 522 (10 bytes header + 512 bytes payload) +bytes as we are working with whole commands. + + +Configuration with configfs + + +ConfigFS is used to create and configure the CCID gadget. +In order to get a device to work as intended, a few attributes must +be considered. + +The attributes is described below followed by an example. + +features +~ + +The `feature` attribute writes to the dwFeatures field in the class descriptor. +See Table 5.1-1 Smart Card Device Descriptors in the CCID Specification [1]_. + +The value indicates what intelligent features the CCID has. +These values are available to user application as defines in ccid.h [2]_. +The default value is 0x. + +The value is a bitwise OR operation performed on the following values: + ++++ +| Value | Description| ++++ +| 0x | No special characteristics | ++++ +| 0x0002 | Automatic parameter configuration based on ATR data| ++++ +| 0x0004 | Automatic activation of ICC on inserting | ++++ +| 0x0008 | Automatic ICC voltage selection| ++++ +| 0x0010
[PATCH 3/3] MAINTAINERS: add USB CCID Gadget Device
Add MAINTAINERS entry for USB CCID Gadget Device Signed-off-by: Marcus Folkesson --- MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 078fd80f664f..e77c3d2bec89 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14541,6 +14541,14 @@ L: linux-s...@vger.kernel.org S: Maintained F: drivers/usb/storage/uas.c +USB CCID GADGET +M: Marcus Folkesson +L: linux-...@vger.kernel.org +S: Maintained +F: drivers/usb/gadget/function/f_ccid.* +F: include/uapi/linux/usb/ccid.h +F: Documentation/usb/gadget_ccid.rst + USB CDC ETHERNET DRIVER M: Oliver Neukum L: linux-...@vger.kernel.org -- 2.16.2
[PATCH 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device
Signed-off-by: Marcus Folkesson
---
drivers/usb/gadget/Kconfig | 17 +
drivers/usb/gadget/function/Makefile | 1 +
drivers/usb/gadget/function/f_ccid.c | 988 +++
drivers/usb/gadget/function/f_ccid.h | 91
include/uapi/linux/usb/ccid.h| 93
5 files changed, 1190 insertions(+)
create mode 100644 drivers/usb/gadget/function/f_ccid.c
create mode 100644 drivers/usb/gadget/function/f_ccid.h
create mode 100644 include/uapi/linux/usb/ccid.h
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 31cce7805eb2..bdebdf1ffa2b 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -149,6 +149,9 @@ config USB_LIBCOMPOSITE
config USB_F_ACM
tristate
+config USB_F_CCID
+ tristate
+
config USB_F_SS_LB
tristate
@@ -248,6 +251,20 @@ config USB_CONFIGFS_ACM
ACM serial link. This function can be used to interoperate with
MS-Windows hosts or with the Linux-USB "cdc-acm" driver.
+config USB_CONFIGFS_CCID
+ bool "Chip Card Interface Device (CCID)"
+ depends on USB_CONFIGFS
+ select USB_F_CCID
+ help
+ The CCID function driver provides generic emulation of a
+ Chip Card Interface Device (CCID).
+
+ You will need a user space server talking to /dev/ccidg*,
+ since the kernel itself does not implement CCID/TPDU/APDU
+ protocol.
+
+ For more information, see Documentation/usb/gadget_ccid.rst.
+
config USB_CONFIGFS_OBEX
bool "Object Exchange Model (CDC OBEX)"
depends on USB_CONFIGFS
diff --git a/drivers/usb/gadget/function/Makefile
b/drivers/usb/gadget/function/Makefile
index 5d3a6cf02218..629851009e1a 100644
--- a/drivers/usb/gadget/function/Makefile
+++ b/drivers/usb/gadget/function/Makefile
@@ -9,6 +9,7 @@ ccflags-y +=
-I$(srctree)/drivers/usb/gadget/udc/
# USB Functions
usb_f_acm-y:= f_acm.o
obj-$(CONFIG_USB_F_ACM)+= usb_f_acm.o
+obj-$(CONFIG_USB_F_CCID) += f_ccid.o
usb_f_ss_lb-y := f_loopback.o f_sourcesink.o
obj-$(CONFIG_USB_F_SS_LB) += usb_f_ss_lb.o
obj-$(CONFIG_USB_U_SERIAL) += u_serial.o
diff --git a/drivers/usb/gadget/function/f_ccid.c
b/drivers/usb/gadget/function/f_ccid.c
new file mode 100644
index ..9ff8615ca303
--- /dev/null
+++ b/drivers/usb/gadget/function/f_ccid.c
@@ -0,0 +1,988 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * f_ccid.c -- Chip Card Interface Device (CCID) function Driver
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "f_ccid.h"
+#include "u_f.h"
+
+/* Number of tx requests to allocate */
+#define N_TX_REQS 4
+
+/* Maximum number of devices */
+#define CCID_MINORS 4
+
+struct ccidg_bulk_dev {
+ atomic_t is_open;
+ atomic_t rx_req_busy;
+ wait_queue_head_t read_wq;
+ wait_queue_head_t write_wq;
+ struct usb_request *rx_req;
+ atomic_t rx_done;
+ struct list_head tx_idle;
+};
+
+struct f_ccidg {
+ struct usb_function_instancefunc_inst;
+ struct usb_function function;
+ spinlock_t lock;
+ atomic_t online;
+
+ /* Character device */
+ struct cdev cdev;
+ int minor;
+
+ /* Dynamic attributes */
+ u32 features;
+ u32 protocols;
+ u8 pinsupport;
+ u8 nslots;
+ u8 lcdlayout;
+
+ /* Endpoints */
+ struct usb_ep *in;
+ struct usb_ep *out;
+ struct ccidg_bulk_dev bulk_dev;
+};
+
+/* Interface Descriptor: */
+static struct usb_interface_descriptor ccid_interface_desc = {
+ .bLength = USB_DT_INTERFACE_SIZE,
+ .bDescriptorType = USB_DT_INTERFACE,
+ .bNumEndpoints =2,
+ .bInterfaceClass = USB_CLASS_CSCID,
+ .bInterfaceSubClass = 0,
+ .bInterfaceProtocol = 0,
+};
+
+/* CCID Class Descriptor */
+static struct ccid_class_descriptor ccid_class_desc = {
+ .bLength = sizeof(ccid_class_desc),
+ .bDescriptorType = CCID_DECRIPTOR_TYPE,
+ .bcdCCID = CCID1_10,
+ /* .bMaxSlotIndex = DYNAMIC */
+ .bVoltageSupport = CCID_VOLTS_3_0,
+ /* .dwProtocols = DYNAMIC */
+ .dwDefaultClock = 3580,
+ .dwMaximumClock = 3580,
+ .bNumClockSupported = 0,
+ .dwDataRate = 9600,
+ .dwMaxDataRate =9600,
+ .bNumDataRatesSupported = 0,
+ .dwMaxIFSD =0,
+ .dwSynchProtocols = 0,
+ .dwMechanical = 0,
+ /* .dwFeatures =DYNAMIC */
+
+ /* extended APDU level Message Length */
+ .dwMaxCCIDMessageLength = 0x200,
+ .bClassGetResponse =0x0,
+ .bClassEnvelope =
[PATCH v2] MAINTAINERS: add PhoenixRC Flight Controller Adapter
Add MAINTAINERS entry for PhoenixRC Flight Controller Adapter Signed-off-by: Marcus Folkesson --- Notes: v2: Add proper mailing list MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9ce5062dc028..d8b5d9f810e4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11103,6 +11103,13 @@ S: Maintained F: include/linux/personality.h F: include/uapi/linux/personality.h +PHOENIX RC FLIGHT CONTROLLER ADAPTER +M: Marcus Folkesson +L: linux-in...@vger.kernel.org +S: Maintained +F: Documentation/input/devices/pxrc.rst +F: drivers/input/joystick/pxrc.c + PHONET PROTOCOL M: Remi Denis-Courmont S: Supported -- 2.16.2
[PATCH] MAINTAINERS: add PhoenixRC Flight Controller Adapter
Add MAINTAINERS entry for PhoenixRC Flight Controller Adapter Signed-off-by: Marcus Folkesson --- MAINTAINERS | 6 ++ 1 file changed, 6 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9051a9ca24a2..feb02e9568e6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7,6 +7,12 @@ S: Maintained F: include/linux/personality.h F: include/uapi/linux/personality.h +PHOENIX RC FLIGHT CONTROLLER ADAPTER +M Marcus Folkesson +S: Maintained +F: Documentation/input/devices/pxrc.rst +F: drivers/input/joystick/pxrc.c + PHONET PROTOCOL M: Remi Denis-Courmont S: Supported -- 2.16.2
Re: [PATCH 2/2] input: misc: Add Spreadtrum vibrator driver
Hi Xiaotong,
On Tue, Apr 17, 2018 at 11:18:24AM +0800, Baolin Wang wrote:
> From: Xiaotong Lu
[snip]
> +static int sc27xx_vibra_probe(struct platform_device *pdev)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct vibra_info *info;
> + int ret;
> +
> + info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + info->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + if (!info->regmap) {
> + dev_err(&pdev->dev, "failed to get vibrator regmap.\n");
> + return -ENODEV;
> + }
> +
> + ret = of_property_read_u32(node, "reg", &info->base);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to get vibrator base address.\n");
> + return ret;
> + }
> +
> + info->input_dev = devm_input_allocate_device(&pdev->dev);
> + if (!info->input_dev) {
> + dev_err(&pdev->dev, "failed to allocate input device.\n");
> + return -ENOMEM;
> + }
> +
> + info->input_dev->name = "sc27xx:vibrator";
> + info->input_dev->id.version = 0;
> + info->input_dev->dev.parent = pdev->dev.parent;
> + info->input_dev->close = sc27xx_vibra_close;
> +
> + input_set_drvdata(info->input_dev, info);
> + input_set_capability(info->input_dev, EV_FF, FF_RUMBLE);
> + INIT_WORK(&info->play_work, sc27xx_vibra_play_work);
> + info->enabled = false;
> +
> + ret = input_ff_create_memless(info->input_dev, NULL, sc27xx_vibra_play);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register vibrator to FF.\n");
> + return ret;
> + }
> +
> + ret = input_register_device(info->input_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register input device.\n");
> + input_ff_destroy(info->input_dev);
I'm not sure how the input_ff is freed for managed devices.
Either you don't have to destroy it here, or you also need to destroy it
in a release() function.
> + return ret;
> + }
> +
> + ret = sc27xx_vibra_hw_init(info);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to initialize the vibrator.\n");
> + input_ff_destroy(info->input_dev);
> + input_unregister_device(info->input_dev);
No need to unregister managed input devices.
> + return ret;
> + }
Cheers,
Marcus Folkesson
[PATCH 3/3] input: gamecon: avoid using __set_bit() for capabilities
input_set_capability() and input_set_abs_param() will do it for you.
Signed-off-by: Marcus Folkesson
---
drivers/input/joystick/gamecon.c | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/input/joystick/gamecon.c b/drivers/input/joystick/gamecon.c
index 2ffb2e8bdc3b..e3a9ef3d5f5a 100644
--- a/drivers/input/joystick/gamecon.c
+++ b/drivers/input/joystick/gamecon.c
@@ -862,7 +862,7 @@ static int gc_setup_pad(struct gc *gc, int idx, int
pad_type)
case GC_N64:
for (i = 0; i < 10; i++)
- __set_bit(gc_n64_btn[i], input_dev->keybit);
+ input_set_capability(input_dev, EV_KEY, gc_n64_btn[i]);
for (i = 0; i < 2; i++) {
input_set_abs_params(input_dev, ABS_X + i, -127, 126,
0, 2);
@@ -879,26 +879,27 @@ static int gc_setup_pad(struct gc *gc, int idx, int
pad_type)
break;
case GC_SNESMOUSE:
- __set_bit(BTN_LEFT, input_dev->keybit);
- __set_bit(BTN_RIGHT, input_dev->keybit);
- __set_bit(REL_X, input_dev->relbit);
- __set_bit(REL_Y, input_dev->relbit);
+ input_set_capability(input_dev, EV_KEY, BTN_LEFT);
+ input_set_capability(input_dev, EV_KEY, BTN_RIGHT);
+ input_set_capability(input_dev, EV_REL, REL_X);
+ input_set_capability(input_dev, EV_REL, REL_Y);
break;
case GC_SNES:
for (i = 4; i < 8; i++)
- __set_bit(gc_snes_btn[i], input_dev->keybit);
+ input_set_capability(input_dev, EV_KEY, gc_snes_btn[i]);
/* fall through */
case GC_NES:
for (i = 0; i < 4; i++)
- __set_bit(gc_snes_btn[i], input_dev->keybit);
+ input_set_capability(input_dev, EV_KEY, gc_snes_btn[i]);
break;
case GC_MULTI2:
- __set_bit(BTN_THUMB, input_dev->keybit);
+ input_set_capability(input_dev, EV_KEY, BTN_THUMB);
/* fall through */
case GC_MULTI:
- __set_bit(BTN_TRIGGER, input_dev->keybit);
+ input_set_capability(input_dev, EV_KEY, BTN_TRIGGER);
+ /* fall through */
break;
case GC_PSX:
@@ -906,15 +907,16 @@ static int gc_setup_pad(struct gc *gc, int idx, int
pad_type)
input_set_abs_params(input_dev,
gc_psx_abs[i], 4, 252, 0, 2);
for (i = 0; i < 12; i++)
- __set_bit(gc_psx_btn[i], input_dev->keybit);
+ input_set_capability(input_dev, EV_KEY, gc_psx_btn[i]);
+ break;
break;
case GC_DDR:
for (i = 0; i < 4; i++)
- __set_bit(gc_psx_ddr_btn[i], input_dev->keybit);
+ input_set_capability(input_dev, EV_KEY,
gc_psx_ddr_btn[i]);
for (i = 0; i < 12; i++)
- __set_bit(gc_psx_btn[i], input_dev->keybit);
+ input_set_capability(input_dev, EV_KEY, gc_psx_btn[i]);
break;
}
--
2.16.2
[PATCH 2/3] input: as5011: avoid using __set_bit() for capabilities
input_set_capability() and input_set_abs_param() will do it for you. Signed-off-by: Marcus Folkesson --- drivers/input/joystick/as5011.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/input/joystick/as5011.c b/drivers/input/joystick/as5011.c index 005d852a06e9..f051993c568e 100644 --- a/drivers/input/joystick/as5011.c +++ b/drivers/input/joystick/as5011.c @@ -269,9 +269,7 @@ static int as5011_probe(struct i2c_client *client, input_dev->id.bustype = BUS_I2C; input_dev->dev.parent = &client->dev; - __set_bit(EV_KEY, input_dev->evbit); - __set_bit(EV_ABS, input_dev->evbit); - __set_bit(BTN_JOYSTICK, input_dev->keybit); + input_set_capability(input_dev, EV_KEY, BTN_JOYSTICK); input_set_abs_params(input_dev, ABS_X, AS5011_MIN_AXIS, AS5011_MAX_AXIS, AS5011_FUZZ, AS5011_FLAT); -- 2.16.2
[PATCH 1/3] input: xpad: avoid using __set_bit() for capabilities
input_set_capability() and input_set_abs_param() will do it for you.
Signed-off-by: Marcus Folkesson
---
drivers/input/joystick/xpad.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 9d2688f3f961..cbf54082281d 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1568,7 +1568,6 @@ static void xpad_close(struct input_dev *dev)
static void xpad_set_up_abs(struct input_dev *input_dev, signed short abs)
{
struct usb_xpad *xpad = input_get_drvdata(input_dev);
- set_bit(abs, input_dev->absbit);
switch (abs) {
case ABS_X:
@@ -1628,10 +1627,7 @@ static int xpad_init_input(struct usb_xpad *xpad)
input_dev->close = xpad_close;
}
- __set_bit(EV_KEY, input_dev->evbit);
-
if (!(xpad->mapping & MAP_STICKS_TO_NULL)) {
- __set_bit(EV_ABS, input_dev->evbit);
/* set up axes */
for (i = 0; xpad_abs[i] >= 0; i++)
xpad_set_up_abs(input_dev, xpad_abs[i]);
@@ -1639,21 +1635,21 @@ static int xpad_init_input(struct usb_xpad *xpad)
/* set up standard buttons */
for (i = 0; xpad_common_btn[i] >= 0; i++)
- __set_bit(xpad_common_btn[i], input_dev->keybit);
+ input_set_capability(input_dev, EV_KEY, xpad_common_btn[i]);
/* set up model-specific ones */
if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype == XTYPE_XBOX360W ||
xpad->xtype == XTYPE_XBOXONE) {
for (i = 0; xpad360_btn[i] >= 0; i++)
- __set_bit(xpad360_btn[i], input_dev->keybit);
+ input_set_capability(input_dev, EV_KEY, xpad360_btn[i]);
} else {
for (i = 0; xpad_btn[i] >= 0; i++)
- __set_bit(xpad_btn[i], input_dev->keybit);
+ input_set_capability(input_dev, EV_KEY, xpad_btn[i]);
}
if (xpad->mapping & MAP_DPAD_TO_BUTTONS) {
for (i = 0; xpad_btn_pad[i] >= 0; i++)
- __set_bit(xpad_btn_pad[i], input_dev->keybit);
+ input_set_capability(input_dev, EV_KEY,
xpad_btn_pad[i]);
}
/*
@@ -1670,7 +1666,8 @@ static int xpad_init_input(struct usb_xpad *xpad)
if (xpad->mapping & MAP_TRIGGERS_TO_BUTTONS) {
for (i = 0; xpad_btn_triggers[i] >= 0; i++)
- __set_bit(xpad_btn_triggers[i], input_dev->keybit);
+ input_set_capability(input_dev, EV_KEY,
+ xpad_btn_triggers[i]);
} else {
for (i = 0; xpad_abs_triggers[i] >= 0; i++)
xpad_set_up_abs(input_dev, xpad_abs_triggers[i]);
--
2.16.2
Re: [PATCH v3 1/7] watchdog: sama5d4: make use of timeout-secs provided in devicetree
On Fri, Mar 16, 2018 at 06:56:27PM -0700, Guenter Roeck wrote: > On 03/16/2018 06:37 AM, Marcus Folkesson wrote: > > Hi, > > > > Am I supposed to do something more to make this patchset picked up? > > > > Did you check linux-next ? :-) Thank you /Marcus > > Guenter > signature.asc Description: PGP signature
[PATCH v6] watchdog: add SPDX identifiers for watchdog subsystem
- Add SPDX identifier - Remove boiler plate license text - If MODULE_LICENSE and boiler plate does not match, go for boiler plate license Signed-off-by: Marcus Folkesson Acked-by: Adam Thomson Acked-by: Baruch Siach Acked-by: Charles Keepax Acked-by: Keiji Hayashibara Acked-by: Johannes Thumshirn Acked-by: Florian Fainelli Acked-by: Mans Rullgard Acked-by: Matthias Brugger Acked-by: Michal Simek Acked-by: Neil Armstrong Acked-by: Nicolas Ferre Acked-by: Thierry Reding Acked-by: Tomas Winkler Acked-by: Patrice Chotard Acked-by: William Breathitt Gray Reviewed-by: Eric Anholt --- Notes: v6: - Only include those drivers that has been acked-by. drivers/watchdog/da9052_wdt.c drivers/watchdog/da9055_wdt.c drivers/watchdog/da9062_wdt.c drivers/watchdog/da9063_wdt.c Acked-by: Adam Thomson drivers/watchdog/digicolor_wdt.c Acked-by: Baruch Siach drivers/watchdog/wm831x_wdt.c drivers/watchdog/wm8350_wdt.c Acked-by: Charles Keepax drivers/watchdog/ar7_wdt.c drivers/watchdog/bcm2835_wdt.c drivers/watchdog/bcm47xx_wdt.c drivers/watchdog/bcm63xx_wdt.c drivers/watchdog/bcm7038_wdt.c drivers/watchdog/bcm_kona_wdt.c drivers/watchdog/mtx-1_wdt.c Acked-by: Florian Fainelli drivers/watchdog/uniphier_wdt.c Acked-by: Keiji Hayashibara drivers/watchdog/mena21_wdt Acked-by: Johannes Thumshirn drivers/watchdog/tangox_wdt.c Acked-by: Mans Rullgard drivers/watchdog/mtk_wdt.c Acked-by: Matthias Brugger drivers/watchdog/cadence_wdt.c drivers/watchdog/of_xilinx_wdt.c Acked-by: Michal Simek drivers/watchdog/meson_gxbb_wdt.c Acked-by: Neil Armstrong drivers/watchdog/at91rm9200_wdt.c drivers/watchdog/at91sam9_wdt.c drivers/watchdog/at91sam9_wdt.h drivers/watchdog/sama5d4_wdt.c Acked-by: Nicolas Ferre drivers/watchdog/tegra_wdt.c Acked-by: Thierry Reding drivers/watchdog/mei_wdt.c Acked-by: Tomas Winkler drivers/watchdog/st_lpc_wdt.c Acked-by: Patrice Chotard drivers/watchdog/ebc-c384_wdt.c Acked-by: William Breathitt Gray v5: - Add missing SPDX tag for Kconfig, rc32434_wdt and rdc321x_wdt v4: - Drop coh901327_wdt since it allready is a pending patch v3: - Keep license text for ebc-c384_wdt v2: - Put back removed copyright texts for meson_gxbb_wdt and coh901327_wdt - Change to BSD-3-Clause for meson_gxbb_wdt v1: Please have an extra look at meson_gxbb_wdt.c drivers/watchdog/ar7_wdt.c| 14 +-- drivers/watchdog/at91rm9200_wdt.c | 5 +--- drivers/watchdog/at91sam9_wdt.c | 5 +--- drivers/watchdog/at91sam9_wdt.h | 5 +--- drivers/watchdog/bcm2835_wdt.c| 5 +--- drivers/watchdog/bcm47xx_wdt.c| 5 +--- drivers/watchdog/bcm63xx_wdt.c| 5 +--- drivers/watchdog/bcm7038_wdt.c| 12 ++ drivers/watchdog/bcm_kona_wdt.c | 9 +-- drivers/watchdog/cadence_wdt.c| 5 +--- drivers/watchdog/da9052_wdt.c | 6 + drivers/watchdog/da9055_wdt.c | 6 + drivers/watchdog/da9062_wdt.c | 10 +--- drivers/watchdog/da9063_wdt.c | 5 +--- drivers/watchdog/digicolor_wdt.c | 5 +--- drivers/watchdog/ebc-c384_wdt.c | 1 + drivers/watchdog/mei_wdt.c| 12 ++ drivers/watchdog/mena21_wdt.c | 4 +--- drivers/watchdog/meson_gxbb_wdt.c | 50 +-- drivers/watchdog/mtk_wdt.c| 11 + drivers/watchdog/mtx-1_wdt.c | 11 + drivers/watchdog/of_xilinx_wdt.c | 8 ++- drivers/watchdog/st_lpc_wdt.c | 6 + drivers/watchdog/tangox_wdt.c | 6 + drivers/watchdog/tegra_wdt.c | 10 +--- drivers/watchdog/uniphier_wdt.c | 10 +--- drivers/watchdog/wm831x_wdt.c | 5 +--- drivers/watchdog/wm8350_wdt.c | 5 +--- 28 files changed, 31 insertions(+), 210 deletions(-) diff --git a/drivers/watchdog/ar7_wdt.c b/drivers/watchdog/ar7_wdt.c index 6d5ae251e309..ee1ab12ab04f 100644 --- a/drivers/watchdog/ar7_wdt.c +++ b/drivers/watchdog/ar7_wdt.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0+ /* * drivers/watchdog/ar7_wdt.c * @@ -8,19 +9,6 @@ * National Semiconductor SCx200 Watchdog support * Copyright (c) 2001,2002 Christer Weinigel * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License
Re: Fix deadlocks in autosuspend
Ping. Would someone please have a look? Thanks, Marcus Folkesson On Wed, Feb 28, 2018 at 02:37:57PM +0100, Marcus Folkesson wrote: > Hello, > > I have not recieved any feedback on these so I resend them. > > I got this deadlock on my own driver (pxrc) when using the same > construction. > > Please have a look > > Here is a clip from > [PATCH v3] input: pxrc: new driver for PhoenixRC Flight Controller Adapter [1] > that describes the problem. > > ---8<-- > Also, I think we have a deadlock in the synaptics_usb driver. > > When the device is suspended and someone is open the device, the input > subsystem will call input_open_device() which takes the > input_dev->mutex and then call input_dev->open(). > > synusb_open() has a call to usb_autopm_get_interface() which will > result in a call to the registered resume-function if the device is > suspended. (see Documentation/driver-api/usb/power-manaement.rst). > > In the case of snaptics_usb, it will take the input_dev->mutex in the > resume function. > > I have no synaptic mouse, but tested to put the same code into my > driver just to confirm, and got the following dump: > > [ 9215.626476] INFO: task input-events:8590 blocked for more than 120 seconds. > [ 9215.626495] Not tainted 4.15.0-rc8-ARCH+ #6 > [ 9215.626500] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables > this message. > [ 9215.626507] input-eventsD0 8590 4394 0x0004 > [ 9215.626520] Call Trace: > [ 9215.626546] ? __schedule+0x236/0x850 > [ 9215.626559] schedule+0x2f/0x90 > [ 9215.626569] schedule_preempt_disabled+0x11/0x20 > [ 9215.626579] __mutex_lock.isra.0+0x1aa/0x520 > [ 9215.626609] ? usb_runtime_suspend+0x70/0x70 [usbcore] > [ 9215.626622] ? pxrc_resume+0x37/0x70 [pxrc] > [ 9215.626632] pxrc_resume+0x37/0x70 [pxrc] > [ 9215.626655] usb_resume_interface.isra.2+0x39/0xe0 [usbcore] > [ 9215.626676] usb_resume_both+0xd2/0x120 [usbcore] > [ 9215.626688] __rpm_callback+0xb6/0x1f0 > [ 9215.626699] rpm_callback+0x1f/0x70 > [ 9215.626718] ? usb_runtime_suspend+0x70/0x70 [usbcore] > [ 9215.626726] rpm_resume+0x4e2/0x7f0 > [ 9215.626737] rpm_resume+0x582/0x7f0 > [ 9215.626749] __pm_runtime_resume+0x3a/0x50 > [ 9215.626767] usb_autopm_get_interface+0x1d/0x50 [usbcore] > [ 9215.626780] pxrc_open+0x17/0x8d [pxrc] > [ 9215.626791] input_open_device+0x70/0xa0 > [ 9215.626804] evdev_open+0x183/0x1c0 [evdev] > [ 9215.626819] chrdev_open+0xa0/0x1b0 > [ 9215.626830] ? cdev_put.part.1+0x20/0x20 > [ 9215.626840] do_dentry_open+0x1ad/0x2c0 > [ 9215.626855] path_openat+0x576/0x1300 > [ 9215.626868] ? alloc_set_pte+0x22c/0x520 > [ 9215.626883] ? filemap_map_pages+0x19b/0x340 > [ 9215.626893] do_filp_open+0x9b/0x110 > [ 9215.626908] ? __check_object_size+0x9d/0x190 > [ 9215.626920] ? __alloc_fd+0xaf/0x160 > [ 9215.626931] ? do_sys_open+0x1bd/0x250 > [ 9215.626942] do_sys_open+0x1bd/0x250 > [ 9215.626956] entry_SYSCALL_64_fastpath+0x20/0x83 > [ 9215.626967] RIP: 0033:0x7fbf6358f7ae > > > tablet/pegasus_notetaker.c and touchscreen/usbtouchscreen.c has the same > construction (taking input_dev->mutex in resume/suspend and call > usb_autopm_get_interface() in open()). > > I will create a separate "pm_mutex" to use instead of input_dev->mutex > to get rid of the lockups in those drivers > > ---8<-- > > > [1] https://lkml.org/lkml/2018/1/20/191 > > Thanks, > > Best regards > Marcus Folkesson > signature.asc Description: PGP signature
Re: [RESEND PATCH v5] input: pxrc: new driver for PhoenixRC Flight Controller Adapter
ping. I do not want to nag, but would someone please have a look at this? Thanks, Marcus Folkesson On Sun, Feb 18, 2018 at 05:17:46PM +0100, Marcus Folkesson wrote: > This driver let you plug in your RC controller to the adapter and > use it as input device in various RC simulators. > > Signed-off-by: Marcus Folkesson > --- > > v5: > - Drop autosuspend support > - Use pm_mutex instead of input_dev->mutex > - Use pxrc->is_open instead of input_dev->users > v4: > - Add call to usb_mark_last_busy() in irq > - Move code from pxrc_resume() to pxrc_reset_resume() > v3: > - Use RUDDER and MISC instead of TILT_X and TILT_Y > - Drop kref and anchor > - Rework URB handling > - Add PM support > v2: > - Change module license to GPLv2 to match SPDX tag > > > Documentation/input/devices/pxrc.rst | 57 +++ > drivers/input/joystick/Kconfig | 9 ++ > drivers/input/joystick/Makefile | 1 + > drivers/input/joystick/pxrc.c| 303 > +++ > 4 files changed, 370 insertions(+) > create mode 100644 Documentation/input/devices/pxrc.rst > create mode 100644 drivers/input/joystick/pxrc.c > > diff --git a/Documentation/input/devices/pxrc.rst > b/Documentation/input/devices/pxrc.rst > new file mode 100644 > index ..ca11f646bae8 > --- /dev/null > +++ b/Documentation/input/devices/pxrc.rst > @@ -0,0 +1,57 @@ > +=== > +pxrc - PhoenixRC Flight Controller Adapter > +=== > + > +:Author: Marcus Folkesson > + > +This driver let you use your own RC controller plugged into the > +adapter that comes with PhoenixRC [1]_ or other compatible adapters. > + > +The adapter supports 7 analog channels and 1 digital input switch. > + > +Notes > += > + > +Many RC controllers is able to configure which stick goes to which channel. > +This is also configurable in most simulators, so a matching is not necessary. > + > +The driver is generating the following input event for analog channels: > + > ++-++ > +| Channel | Event | > ++=++ > +| 1 | ABS_X | > ++-++ > +| 2 | ABS_Y | > ++-++ > +| 3 | ABS_RX| > ++-++ > +| 4 | ABS_RY| > ++-++ > +| 5 | ABS_RUDDER| > ++-++ > +| 6 | ABS_THROTTLE | > ++-++ > +| 7 | ABS_MISC | > ++-++ > + > +The digital input switch is generated as an `BTN_A` event. > + > +Manual Testing > +== > + > +To test this driver's functionality you may use `input-event` which is part > of > +the `input layer utilities` suite [2]_. > + > +For example:: > + > +> modprobe pxrc > +> input-events > + > +To print all input events from input `devnr`. > + > +References > +== > + > +.. [1] http://www.phoenix-sim.com/ > +.. [2] https://www.kraxel.org/cgit/input/ > diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig > index f3c2f6ea8b44..332c0cc1b2ab 100644 > --- a/drivers/input/joystick/Kconfig > +++ b/drivers/input/joystick/Kconfig > @@ -351,4 +351,13 @@ config JOYSTICK_PSXPAD_SPI_FF > > To drive rumble motor a dedicated power supply is required. > > +config JOYSTICK_PXRC > + tristate "PhoenixRC Flight Controller Adapter" > + depends on USB_ARCH_HAS_HCD > + depends on USB > + help > + Say Y here if you want to use the PhoenixRC Flight Controller Adapter. > + > + To compile this driver as a module, choose M here: the > + module will be called pxrc. > endif > diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile > index 67651efda2e1..dd0492ebbed7 100644 > --- a/drivers/input/joystick/Makefile > +++ b/drivers/input/joystick/Makefile > @@ -23,6 +23,7 @@ obj-$(CONFIG_JOYSTICK_JOYDUMP) += joydump.o > obj-$(CONFIG_JOYSTICK_MAGELLAN) += magellan.o > obj-$(CONFIG_JOYSTICK_MAPLE) += maplecontrol.o > obj-$(CONFIG_JOYSTICK_PSXPAD_SPI)+= psxpad-spi.o > +obj-$(CONFIG_JOYSTICK_PXRC) += pxrc.o > obj-$(CONFIG_JOYSTICK_SIDEWINDER)+= sidewinder.o > obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o > obj-$(CONFIG_JOYSTICK_SPACEORB) += spaceorb.o > diff --git a/drivers/input/joystick/pxrc.c b/drivers/input/joys
Re: [PATCH v3 1/7] watchdog: sama5d4: make use of timeout-secs provided in devicetree
Hi,
Am I supposed to do something more to make this patchset picked up?
Thanks,
Marcus Folkesson
On Sun, Feb 11, 2018 at 09:08:41PM +0100, Marcus Folkesson wrote:
> watchdog_init_timeout() will allways pick timeout_param since it
> defaults to a valid timeout.
>
> Following best practice described in
> Documentation/watchdog/watchdog-kernel-api.txt to make use of
> the parameter logic.
>
> Signed-off-by: Marcus Folkesson
> ---
>
> Notes:
> v3:
> - Use wdd->timeout instead of wdt_timeout when print out
> timout in probe function.
>
> drivers/watchdog/sama5d4_wdt.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
> index 0ae947c3d7bc..255169916dbb 100644
> --- a/drivers/watchdog/sama5d4_wdt.c
> +++ b/drivers/watchdog/sama5d4_wdt.c
> @@ -33,7 +33,7 @@ struct sama5d4_wdt {
> unsigned long last_ping;
> };
>
> -static int wdt_timeout = WDT_DEFAULT_TIMEOUT;
> +static int wdt_timeout;
> static bool nowayout = WATCHDOG_NOWAYOUT;
>
> module_param(wdt_timeout, int, 0);
> @@ -212,7 +212,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> wdd = &wdt->wdd;
> - wdd->timeout = wdt_timeout;
> + wdd->timeout = WDT_DEFAULT_TIMEOUT;
> wdd->info = &sama5d4_wdt_info;
> wdd->ops = &sama5d4_wdt_ops;
> wdd->min_timeout = MIN_WDT_TIMEOUT;
> @@ -273,7 +273,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, wdt);
>
> dev_info(&pdev->dev, "initialized (timeout = %d sec, nowayout = %d)\n",
> - wdt_timeout, nowayout);
> + wdd->timeout, nowayout);
>
> return 0;
> }
> --
> 2.15.1
>
signature.asc
Description: PGP signature
Re: [PATCH 1/2] dt-bindings: watchdog: Add Nuvoton NPCM description
On Fri, Mar 02, 2018 at 05:07:45PM +1030, Joel Stanley wrote:
> These bindings describe the watchdog IP as used by the Nuvoton NPCM750
> (Poleg) BMC SoC.
>
> Signed-off-by: Joel Stanley
> ---
> .../bindings/watchdog/nuvoton,npcm-wdt.txt | 25
> ++
> 1 file changed, 25 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt
>
> diff --git a/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt
> b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt
> new file mode 100644
> index ..599db288337e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/nuvoton,npcm-wdt.txt
> @@ -0,0 +1,25 @@
> +Nuvoton NPCM Watchdog
> +
> +Nuvoton NPCM timer module provides five 24-bit timer counters, and a
> watchdog.
> +The watchdog supports a pre-timeout interrupt that fires 10ms before the
> +expiry.
> +
> +Required properties:
> +- compatible : "nuvoton,npcm750-wdt" for NPCM750 (Poleg).
> +- reg : Offset and length of the register set for the device.
> +- interrupts : Contain the timer interrupt with flags for
> +falling edge.
> +
> +Required clocking property, have to be one of:
> +- clocks : phandle of timer reference clock.
> +- clock-frequency : The frequency in Hz of the clock that drives the NPCM7xx
> +timer (usually 2500).
Optional properties:
- timeout-sec : Contains the watchdog timeout in seconds
watchdog_init_timeout() will fix this for you.
> +
> +Example:
> +
> +timer@f00080c1 {
> +compatible = "nuvoton,npcm750-wdt";
> +interrupts = ;
> +reg = <0xf00080c1 0x4>;
> +clocks = <&clk NPCM7XX_CLK_TIMER>;
> +};
> --
> 2.15.1
>
Best regards
Marcus Folkesson
signature.asc
Description: PGP signature
Re: [PATCH 2/2] watchdog: Add Nuvoton NPCM watchdog driver
truct watchdog_device *wdd)
> +{
> + return container_of(wdd, struct npcm_wdt, wdd);
> +}
> +
> +static int npcm_wdt_ping(struct watchdog_device *wdd)
> +{
> + struct npcm_wdt *wdt = to_npcm_wdt(wdd);
> + u32 val;
> +
> + val = readl(wdt->reg);
> + writel(val | NPCM_WTR, wdt->reg);
> +
> + return 0;
> +}
> +
> +static int npcm_wdt_start(struct watchdog_device *wdd)
> +{
> + struct npcm_wdt *wdt = to_npcm_wdt(wdd);
> + u32 val;
> +
> + val = NPCM_WTRE | NPCM_WTE | NPCM_WTR | NPCM_WTIE;
> +
> + if (wdd->timeout < 2)
> + val |= 0x800;
> + else if (wdd->timeout < 3)
> + val |= 0x420;
> + else if (wdd->timeout < 6)
> + val |= 0x810;
> + else if (wdd->timeout < 11)
> + val |= 0x430;
> + else if (wdd->timeout < 22)
> + val |= 0x820;
> + else if (wdd->timeout < 44)
> + val |= 0xC00;
> + else if (wdd->timeout < 87)
> + val |= 0x830;
> + else if (wdd->timeout < 173)
> + val |= 0xC10;
> + else if (wdd->timeout < 688)
> + val |= 0xC20;
> + else if (wdd->timeout < 2751)
> + val |= 0xC30;
> + else
> + val |= 0x830;
> +
> + writel(val, wdt->reg);
> +
> + return 0;
> +}
> +
> +static int npcm_wdt_stop(struct watchdog_device *wdd)
> +{
> + struct npcm_wdt *wdt = to_npcm_wdt(wdd);
> +
> + writel(0, wdt->reg);
> +
> + return 0;
> +}
> +
> +
> +static int npcm_wdt_set_timeout(struct watchdog_device *wdd,
> + unsigned int timeout)
> +{
> + wdd->timeout = timeout; /* New timeout */
> + if (watchdog_active(wdd))
> + npcm_wdt_start(wdd);
> +
> + return 0;
> +}
> +
> +static irqreturn_t npcm_wdt_interrupt(int irq, void *data)
> +{
> + struct npcm_wdt *wdt = data;
> +
> + watchdog_notify_pretimeout(&wdt->wdd);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int npcm_wdt_restart(struct watchdog_device *wdd,
> + unsigned long action, void *data)
> +{
> + struct npcm_wdt *wdt = to_npcm_wdt(wdd);
> +
> + writel(NPCM_WTR | NPCM_WTRE | NPCM_WTE, wdt->reg);
> + udelay(1000);
> +
> + return 0;
> +}
> +
> +static const struct watchdog_info npcm_wdt_info = {
> + .identity = KBUILD_MODNAME,
> + .options= WDIOF_SETTIMEOUT
> + | WDIOF_KEEPALIVEPING
> + | WDIOF_MAGICCLOSE,
> +};
> +
> +static const struct watchdog_ops npcm_wdt_ops = {
> + .owner = THIS_MODULE,
> + .start = npcm_wdt_start,
> + .stop = npcm_wdt_stop,
> + .ping = npcm_wdt_ping,
> + .set_timeout = npcm_wdt_set_timeout,
> + .restart = npcm_wdt_restart,
> +};
> +
> +static int npcm_wdt_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct npcm_wdt *wdt;
> + struct resource *res;
> + int irq;
> + int ret;
> +
> + wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
> + if (!wdt)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + wdt->reg = devm_ioremap_resource(dev, res);
> + if (IS_ERR(wdt->reg))
> + return PTR_ERR(wdt->reg);
> +
> + irq = platform_get_irq(pdev, 0);
> + if (!irq)
> + return -EINVAL;
> +
> + wdt->dev = dev;
> + wdt->wdd.info = &npcm_wdt_info;
> + wdt->wdd.ops = &npcm_wdt_ops;
> + wdt->wdd.min_timeout = 1;
> + wdt->wdd.max_timeout = 2751;
> + wdt->wdd.parent = dev;
> +
> + wdt->wdd.timeout = 86;
> + watchdog_init_timeout(&wdt->wdd, 0, dev);
watchdog_init_timeout() will also read the `timeout-sec` property from
the devicetree if present, which could be a good thing.
See drivers/watchdog/watchdog_core.c for implementation.
Just put it as an optional property in the dt-bindings.
> +
> + ret = devm_request_irq(dev, irq, npcm_wdt_interrupt, 0,
> + "watchdog", wdt);
> + if (ret)
> + return ret;
> +
> + ret = devm_watchdog_register_device(dev, &wdt->wdd);
> + if (ret) {
> + dev_err(dev, "failed to register watchdog\n");
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, wdt);
Do we need to set drvdata? I don't see that we are using it?
> + dev_info(dev, "NPCM watchdog driver enabled\n");
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id npcm_wdt_match[] = {
> + {.compatible = "nuvoton,npcm750-wdt"},
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, npcm_wdt_match);
> +#endif
> +
> +static struct platform_driver npcm_wdt_driver = {
> + .probe = npcm_wdt_probe,
> + .driver = {
> + .name = "npcm-wdt",
> + .of_match_table = of_match_ptr(npcm_wdt_match),
> + },
> +};
> +module_platform_driver(npcm_wdt_driver);
> +
> +MODULE_AUTHOR("Joel Stanley");
> +MODULE_DESCRIPTION("Watchdog driver for NPCM");
> +MODULE_LICENSE("GPL");
^^
Either "GPL v2" here or change SPDX identifier.
> --
> 2.15.1
Overall I think it looks good.
Thanks,
Marcus Folkesson
signature.asc
Description: PGP signature
[PATCH v5] watchdog: add SPDX identifiers for watchdog subsystem
- Add SPDX identifier - Remove boiler plate license text - If MODULE_LICENSE and boiler plate does not match, go for boiler plate license Cc: Philippe Ombredanne Signed-off-by: Marcus Folkesson Acked-by: Adam Thomson Acked-by: Baruch Siach Acked-by: Charles Keepax Acked-by: Florian Fainelli Acked-by: Keiji Hayashibara Acked-by: Johannes Thumshirn Acked-by: Mans Rullgard Acked-by: Matthias Brugger Acked-by: Michal Simek Acked-by: Neil Armstrong Acked-by: Nicolas Ferre Acked-by: Thierry Reding Acked-by: Tomas Winkler Acked-by: Patrice Chotard Acked-by: William Breathitt Gray Reviewed-by: Eric Anholt --- Notes: v5: - Add missing SPDX tag for Kconfig, rc32434_wdt and rdc321x_wdt v4: - Drop coh901327_wdt since it allready is a pending patch v3: - Keep license text for ebc-c384_wdt v2: - Put back removed copyright texts for meson_gxbb_wdt and coh901327_wdt - Change to BSD-3-Clause for meson_gxbb_wdt v1: Please have an extra look at meson_gxbb_wdt.c drivers/watchdog/Kconfig | 2 +- drivers/watchdog/acquirewdt.c | 6 +--- drivers/watchdog/advantechwdt.c| 6 +--- drivers/watchdog/alim1535_wdt.c| 6 +--- drivers/watchdog/alim7101_wdt.c| 1 + drivers/watchdog/ar7_wdt.c | 14 +- drivers/watchdog/asm9260_wdt.c | 2 +- drivers/watchdog/aspeed_wdt.c | 5 +--- drivers/watchdog/at91rm9200_wdt.c | 5 +--- drivers/watchdog/at91sam9_wdt.c| 5 +--- drivers/watchdog/at91sam9_wdt.h| 5 +--- drivers/watchdog/ath79_wdt.c | 4 +-- drivers/watchdog/atlas7_wdt.c | 2 +- drivers/watchdog/bcm2835_wdt.c | 5 +--- drivers/watchdog/bcm47xx_wdt.c | 5 +--- drivers/watchdog/bcm63xx_wdt.c | 5 +--- drivers/watchdog/bcm7038_wdt.c | 12 ++-- drivers/watchdog/bcm_kona_wdt.c| 9 +- drivers/watchdog/bfin_wdt.c| 2 +- drivers/watchdog/booke_wdt.c | 5 +--- drivers/watchdog/cadence_wdt.c | 5 +--- drivers/watchdog/cpu5wdt.c | 15 +- drivers/watchdog/cpwd.c| 1 + drivers/watchdog/da9052_wdt.c | 6 +--- drivers/watchdog/da9055_wdt.c | 6 +--- drivers/watchdog/da9062_wdt.c | 10 +-- drivers/watchdog/da9063_wdt.c | 5 +--- drivers/watchdog/davinci_wdt.c | 7 ++--- drivers/watchdog/diag288_wdt.c | 1 + drivers/watchdog/digicolor_wdt.c | 5 +--- drivers/watchdog/dw_wdt.c | 6 +--- drivers/watchdog/ebc-c384_wdt.c| 1 + drivers/watchdog/ep93xx_wdt.c | 7 ++--- drivers/watchdog/eurotechwdt.c | 6 +--- drivers/watchdog/f71808e_wdt.c | 16 +-- drivers/watchdog/ftwdt010_wdt.c| 6 ++-- drivers/watchdog/gef_wdt.c | 6 +--- drivers/watchdog/geodewdt.c| 5 +--- drivers/watchdog/gpio_wdt.c| 5 +--- drivers/watchdog/hpwdt.c | 7 ++--- drivers/watchdog/i6300esb.c| 6 +--- drivers/watchdog/iTCO_vendor_support.c | 9 +- drivers/watchdog/iTCO_wdt.c| 10 +-- drivers/watchdog/ib700wdt.c| 6 +--- drivers/watchdog/ibmasr.c | 3 +- drivers/watchdog/ie6xx_wdt.c | 18 ++-- drivers/watchdog/imgpdc_wdt.c | 5 +--- drivers/watchdog/imx2_wdt.c| 5 +--- drivers/watchdog/indydog.c | 6 +--- drivers/watchdog/intel-mid_wdt.c | 6 ++-- drivers/watchdog/intel_scu_watchdog.c | 18 ++-- drivers/watchdog/intel_scu_watchdog.h | 16 +-- drivers/watchdog/iop_wdt.c | 16 ++- drivers/watchdog/it8712f_wdt.c | 10 +-- drivers/watchdog/it87_wdt.c| 10 +-- drivers/watchdog/ixp4xx_wdt.c | 6 ++-- drivers/watchdog/jz4740_wdt.c | 10 +-- drivers/watchdog/kempld_wdt.c | 12 ++-- drivers/watchdog/ks8695_wdt.c | 6 ++-- drivers/watchdog/lantiq_wdt.c | 7 ++--- drivers/watchdog/loongson1_wdt.c | 5 +--- drivers/watchdog/lpc18xx_wdt.c | 5 +--- drivers/watchdog/m54xx_wdt.c | 6 ++-- drivers/watchdog/machzwd.c | 11 +--- drivers/watchdog/max63xx_wdt.c | 5 +--- drivers/watchdog/max77620_wdt.c| 5 +--- drivers/watchdog/mei_wdt.c | 12 ++-- drivers/watchdog/mena21_wdt.c | 4 +-- drivers/watchdog/menf21bmc_wdt.c | 8 ++ drivers/watchdog/meson_gxbb_wdt.c | 50 +- drivers/watchdog/meson_wdt.c | 6 +--- drivers/watchdog/mixcomwd.c| 6 +--- drivers/watchdog/moxart_wdt.c | 7 ++--- drivers/watchdog/mpc8xxx_wdt.c | 6 +--- drivers/watchdog/mt7621_wdt.c | 5 +--- drivers/watchdog/mtk_wdt.c | 11 +--- drivers/watchdog/mtx-1_wdt.c | 11
[PATCH] usb: usb-skeleton: make MODULE_LICENSE and SPDX tag match
GPL v2 is the original license according to the old license text.
See f64cdd0e94f1faf3b7f2b03af71f70dc6d8da0c2.
Signed-off-by: Marcus Folkesson
---
drivers/usb/usb-skeleton.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/usb-skeleton.c b/drivers/usb/usb-skeleton.c
index 26ca0ec01fd5..c3ddd0f1f449 100644
--- a/drivers/usb/usb-skeleton.c
+++ b/drivers/usb/usb-skeleton.c
@@ -640,4 +640,4 @@ static struct usb_driver skel_driver = {
module_usb_driver(skel_driver);
-MODULE_LICENSE("GPL");
+MODULE_LICENSE("GPL v2");
--
2.16.2
Re: [PATCH v4 1/4] HID: add driver for Valve Steam Controller
Rodrigo,
On Wed, Feb 28, 2018 at 11:49:26PM +0100, Rodrigo Rivas Costa wrote:
> On Wed, Feb 28, 2018 at 09:21:15PM +0200, Andy Shevchenko wrote:
> > On Wed, Feb 28, 2018 at 8:43 PM, Rodrigo Rivas Costa
> > wrote:
> > > There are two ways to connect the Steam Controller: directly to the USB
> > > or with the USB wireless adapter. Both methods are similar, but the
> > > wireless adapter can connect up to 4 devices at the same time.
> > >
> > > The wired device will appear as 3 interfaces: a virtual mouse, a virtual
> > > keyboard and a custom HID device.
> > >
> > > The wireless device will appear as 5 interfaces: a virtual keyboard and
> > > 4 custom HID devices, that will remain silent until a device is actually
> > > connected.
> > >
> > > The custom HID device has a report descriptor with all vendor specific
> > > usages, so the hid-generic is not very useful. In a PC/SteamBox Valve
> > > Steam Client provices a software translation by using direct USB access
> > > and a creates a uinput virtual gamepad.
> > >
> > > This driver was reverse engineered to provide direct kernel support in
> > > case you cannot, or do not want to, use Valve Steam Client. It disables
> > > the virtual keyboard and mouse, as they are not so useful when you have
> > > a working gamepad.
> >
> >
> > > +// SPDX-License-Identifier: GPL-2.0
> >
> > > +MODULE_LICENSE("GPL");
> >
> > Not the same.
>
> Hmmm... I copied from usb-skeleton.c, IIRC...
> I'll change to GPL-2.0+, that would be correct, I think.
Yep, the usb-skeleton.c is wrong.
I have prepared a patch, just not submitted it yet..
GPL-2.0+ is "GPLv2 or later" if that is what you want.
Best regards
Marcus Folkesson
[RESEND PATCH 2/6] input: synaptics_usb: do not rely on input_dev->users
If the device is unused and suspended, a call to open will cause the
device to autoresume through the call to usb_autopm_get_interface().
input_dev->users is already incremented by the input subsystem,
therefore this expression will always be evaluated to true:
if ((input_dev->users || (synusb->flags & SYNUSB_IO_ALWAYS)) &&
usb_submit_urb(synusb->urb, GFP_NOIO) < 0) {
retval = -EIO;
}
The same URB will then be fail when resubmitted in synusb_open().
Introduce synusb->is_open to keep track of the state instead.
Signed-off-by: Marcus Folkesson
---
drivers/input/mouse/synaptics_usb.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/input/mouse/synaptics_usb.c
b/drivers/input/mouse/synaptics_usb.c
index 2c66913cf5a2..e2b726751220 100644
--- a/drivers/input/mouse/synaptics_usb.c
+++ b/drivers/input/mouse/synaptics_usb.c
@@ -84,6 +84,7 @@ struct synusb {
/* serialize access to open/suspend */
struct mutex pm_mutex;
+ bool is_open;
/* input device related data structures */
struct input_dev *input;
@@ -266,6 +267,7 @@ static int synusb_open(struct input_dev *dev)
}
synusb->intf->needs_remote_wakeup = 1;
+ synusb->is_open = 1;
out:
mutex_unlock(&synusb->pm_mutex);
@@ -283,6 +285,7 @@ static void synusb_close(struct input_dev *dev)
mutex_lock(&synusb->pm_mutex);
usb_kill_urb(synusb->urb);
synusb->intf->needs_remote_wakeup = 0;
+ synusb->is_open = 0;
mutex_unlock(&synusb->pm_mutex);
if (!autopm_error)
@@ -485,12 +488,11 @@ static int synusb_suspend(struct usb_interface *intf,
pm_message_t message)
static int synusb_resume(struct usb_interface *intf)
{
struct synusb *synusb = usb_get_intfdata(intf);
- struct input_dev *input_dev = synusb->input;
int retval = 0;
mutex_lock(&synusb->pm_mutex);
- if ((input_dev->users || (synusb->flags & SYNUSB_IO_ALWAYS)) &&
+ if ((synusb->is_open || (synusb->flags & SYNUSB_IO_ALWAYS)) &&
usb_submit_urb(synusb->urb, GFP_NOIO) < 0) {
retval = -EIO;
}
@@ -513,10 +515,9 @@ static int synusb_pre_reset(struct usb_interface *intf)
static int synusb_post_reset(struct usb_interface *intf)
{
struct synusb *synusb = usb_get_intfdata(intf);
- struct input_dev *input_dev = synusb->input;
int retval = 0;
- if ((input_dev->users || (synusb->flags & SYNUSB_IO_ALWAYS)) &&
+ if ((synusb->is_open || (synusb->flags & SYNUSB_IO_ALWAYS)) &&
usb_submit_urb(synusb->urb, GFP_NOIO) < 0) {
retval = -EIO;
}
--
2.16.2
Fix deadlocks in autosuspend
Hello, I have not recieved any feedback on these so I resend them. I got this deadlock on my own driver (pxrc) when using the same construction. Please have a look Here is a clip from [PATCH v3] input: pxrc: new driver for PhoenixRC Flight Controller Adapter [1] that describes the problem. ---8<-- Also, I think we have a deadlock in the synaptics_usb driver. When the device is suspended and someone is open the device, the input subsystem will call input_open_device() which takes the input_dev->mutex and then call input_dev->open(). synusb_open() has a call to usb_autopm_get_interface() which will result in a call to the registered resume-function if the device is suspended. (see Documentation/driver-api/usb/power-manaement.rst). In the case of snaptics_usb, it will take the input_dev->mutex in the resume function. I have no synaptic mouse, but tested to put the same code into my driver just to confirm, and got the following dump: [ 9215.626476] INFO: task input-events:8590 blocked for more than 120 seconds. [ 9215.626495] Not tainted 4.15.0-rc8-ARCH+ #6 [ 9215.626500] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 9215.626507] input-eventsD0 8590 4394 0x0004 [ 9215.626520] Call Trace: [ 9215.626546] ? __schedule+0x236/0x850 [ 9215.626559] schedule+0x2f/0x90 [ 9215.626569] schedule_preempt_disabled+0x11/0x20 [ 9215.626579] __mutex_lock.isra.0+0x1aa/0x520 [ 9215.626609] ? usb_runtime_suspend+0x70/0x70 [usbcore] [ 9215.626622] ? pxrc_resume+0x37/0x70 [pxrc] [ 9215.626632] pxrc_resume+0x37/0x70 [pxrc] [ 9215.626655] usb_resume_interface.isra.2+0x39/0xe0 [usbcore] [ 9215.626676] usb_resume_both+0xd2/0x120 [usbcore] [ 9215.626688] __rpm_callback+0xb6/0x1f0 [ 9215.626699] rpm_callback+0x1f/0x70 [ 9215.626718] ? usb_runtime_suspend+0x70/0x70 [usbcore] [ 9215.626726] rpm_resume+0x4e2/0x7f0 [ 9215.626737] rpm_resume+0x582/0x7f0 [ 9215.626749] __pm_runtime_resume+0x3a/0x50 [ 9215.626767] usb_autopm_get_interface+0x1d/0x50 [usbcore] [ 9215.626780] pxrc_open+0x17/0x8d [pxrc] [ 9215.626791] input_open_device+0x70/0xa0 [ 9215.626804] evdev_open+0x183/0x1c0 [evdev] [ 9215.626819] chrdev_open+0xa0/0x1b0 [ 9215.626830] ? cdev_put.part.1+0x20/0x20 [ 9215.626840] do_dentry_open+0x1ad/0x2c0 [ 9215.626855] path_openat+0x576/0x1300 [ 9215.626868] ? alloc_set_pte+0x22c/0x520 [ 9215.626883] ? filemap_map_pages+0x19b/0x340 [ 9215.626893] do_filp_open+0x9b/0x110 [ 9215.626908] ? __check_object_size+0x9d/0x190 [ 9215.626920] ? __alloc_fd+0xaf/0x160 [ 9215.626931] ? do_sys_open+0x1bd/0x250 [ 9215.626942] do_sys_open+0x1bd/0x250 [ 9215.626956] entry_SYSCALL_64_fastpath+0x20/0x83 [ 9215.626967] RIP: 0033:0x7fbf6358f7ae tablet/pegasus_notetaker.c and touchscreen/usbtouchscreen.c has the same construction (taking input_dev->mutex in resume/suspend and call usb_autopm_get_interface() in open()). I will create a separate "pm_mutex" to use instead of input_dev->mutex to get rid of the lockups in those drivers ---8<-- [1] https://lkml.org/lkml/2018/1/20/191 Thanks, Best regards Marcus Folkesson
[RESEND PATCH 4/6] input: pegasus_notetaker: do not rely on input_dev->users
If the device is unused and suspended, a call to open will cause the
device to autoresume through the call to usb_autopm_get_interface().
input_dev->users is already incremented by the input subsystem,
therefore this expression will always be evaluated to true:
if (pegasus->dev->users && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
retval = -EIO;
The same URB will then be fail when resubmitted in pegasus_open().
Introduce pegasus->is_open to keep track of the state instead.
Signed-off-by: Marcus Folkesson
---
drivers/input/tablet/pegasus_notetaker.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/input/tablet/pegasus_notetaker.c
b/drivers/input/tablet/pegasus_notetaker.c
index 9ab1ed5e20e7..74c93e09c337 100644
--- a/drivers/input/tablet/pegasus_notetaker.c
+++ b/drivers/input/tablet/pegasus_notetaker.c
@@ -80,6 +80,7 @@ struct pegasus {
/* serialize access to open/suspend */
struct mutex pm_mutex;
+ bool is_open;
char name[128];
char phys[64];
@@ -232,6 +233,7 @@ static int pegasus_open(struct input_dev *dev)
if (error)
goto err_kill_urb;
+ pegasus->is_open = 1;
mutex_unlock(&pegasus->pm_mutex);
return 0;
@@ -251,6 +253,7 @@ static void pegasus_close(struct input_dev *dev)
mutex_lock(&pegasus->pm_mutex);
usb_kill_urb(pegasus->irq);
cancel_work_sync(&pegasus->init);
+ pegasus->is_open = 0;
mutex_unlock(&pegasus->pm_mutex);
usb_autopm_put_interface(pegasus->intf);
@@ -415,7 +418,7 @@ static int pegasus_resume(struct usb_interface *intf)
int retval = 0;
mutex_lock(&pegasus->pm_mutex);
- if (pegasus->dev->users && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
+ if (pegasus->is_open && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
retval = -EIO;
mutex_unlock(&pegasus->pm_mutex);
@@ -428,7 +431,7 @@ static int pegasus_reset_resume(struct usb_interface *intf)
int retval = 0;
mutex_lock(&pegasus->pm_mutex);
- if (pegasus->dev->users) {
+ if (pegasus->is_open) {
retval = pegasus_set_mode(pegasus, PEN_MODE_XY,
NOTETAKER_LED_MOUSE);
if (!retval && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
--
2.16.2
[RESEND PATCH 3/6] input: pagasus_notetaker: fix deadlock in autosuspend
usb_autopm_get_interface() that is called in pegasus_open() does an
autoresume if the device is suspended.
input_dev->mutex used in pegasus_resume() is in this case already
taken by the input subsystem and will cause a deadlock.
Signed-off-by: Marcus Folkesson
---
drivers/input/tablet/pegasus_notetaker.c | 25 +++--
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/input/tablet/pegasus_notetaker.c
b/drivers/input/tablet/pegasus_notetaker.c
index 47de5a81172f..9ab1ed5e20e7 100644
--- a/drivers/input/tablet/pegasus_notetaker.c
+++ b/drivers/input/tablet/pegasus_notetaker.c
@@ -41,6 +41,7 @@
#include
#include
#include
+#include
/* USB HID defines */
#define USB_REQ_GET_REPORT 0x01
@@ -76,6 +77,10 @@ struct pegasus {
struct usb_device *usbdev;
struct usb_interface *intf;
struct urb *irq;
+
+ /* serialize access to open/suspend */
+ struct mutex pm_mutex;
+
char name[128];
char phys[64];
struct work_struct init;
@@ -216,6 +221,7 @@ static int pegasus_open(struct input_dev *dev)
if (error)
return error;
+ mutex_lock(&pegasus->pm_mutex);
pegasus->irq->dev = pegasus->usbdev;
if (usb_submit_urb(pegasus->irq, GFP_KERNEL)) {
error = -EIO;
@@ -226,12 +232,14 @@ static int pegasus_open(struct input_dev *dev)
if (error)
goto err_kill_urb;
+ mutex_unlock(&pegasus->pm_mutex);
return 0;
err_kill_urb:
usb_kill_urb(pegasus->irq);
cancel_work_sync(&pegasus->init);
err_autopm_put:
+ mutex_unlock(&pegasus->pm_mutex);
usb_autopm_put_interface(pegasus->intf);
return error;
}
@@ -240,8 +248,11 @@ static void pegasus_close(struct input_dev *dev)
{
struct pegasus *pegasus = input_get_drvdata(dev);
+ mutex_lock(&pegasus->pm_mutex);
usb_kill_urb(pegasus->irq);
cancel_work_sync(&pegasus->init);
+ mutex_unlock(&pegasus->pm_mutex);
+
usb_autopm_put_interface(pegasus->intf);
}
@@ -274,6 +285,8 @@ static int pegasus_probe(struct usb_interface *intf,
goto err_free_mem;
}
+ mutex_init(&pegasus->pm_mutex);
+
pegasus->usbdev = dev;
pegasus->dev = input_dev;
pegasus->intf = intf;
@@ -388,10 +401,10 @@ static int pegasus_suspend(struct usb_interface *intf,
pm_message_t message)
{
struct pegasus *pegasus = usb_get_intfdata(intf);
- mutex_lock(&pegasus->dev->mutex);
+ mutex_lock(&pegasus->pm_mutex);
usb_kill_urb(pegasus->irq);
cancel_work_sync(&pegasus->init);
- mutex_unlock(&pegasus->dev->mutex);
+ mutex_unlock(&pegasus->pm_mutex);
return 0;
}
@@ -401,10 +414,10 @@ static int pegasus_resume(struct usb_interface *intf)
struct pegasus *pegasus = usb_get_intfdata(intf);
int retval = 0;
- mutex_lock(&pegasus->dev->mutex);
+ mutex_lock(&pegasus->pm_mutex);
if (pegasus->dev->users && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
retval = -EIO;
- mutex_unlock(&pegasus->dev->mutex);
+ mutex_unlock(&pegasus->pm_mutex);
return retval;
}
@@ -414,14 +427,14 @@ static int pegasus_reset_resume(struct usb_interface
*intf)
struct pegasus *pegasus = usb_get_intfdata(intf);
int retval = 0;
- mutex_lock(&pegasus->dev->mutex);
+ mutex_lock(&pegasus->pm_mutex);
if (pegasus->dev->users) {
retval = pegasus_set_mode(pegasus, PEN_MODE_XY,
NOTETAKER_LED_MOUSE);
if (!retval && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
retval = -EIO;
}
- mutex_unlock(&pegasus->dev->mutex);
+ mutex_unlock(&pegasus->pm_mutex);
return retval;
}
--
2.16.2
[RESEND PATCH 5/6] input: usbtouchscreen: fix deadlock in autosuspend
usb_autopm_get_interface() that is called in usbtouch_open() does an
autoresume if the device is suspended.
input_dev->mutex used in usbtouch_resume() is in this case already
taken by the input subsystem and will cause a deadlock.
Signed-off-by: Marcus Folkesson
---
drivers/input/touchscreen/usbtouchscreen.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/input/touchscreen/usbtouchscreen.c
b/drivers/input/touchscreen/usbtouchscreen.c
index 2c41107240de..e964658203d8 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -54,6 +54,7 @@
#include
#include
#include
+#include
#define DRIVER_VERSION "v0.6"
@@ -112,6 +113,7 @@ struct usbtouch_usb {
struct usb_interface *interface;
struct input_dev *input;
struct usbtouch_device_info *type;
+ struct mutex pm_mutex; /* serialize access to open/suspend */
char name[128];
char phys[64];
void *priv;
@@ -1455,6 +1457,7 @@ static int usbtouch_open(struct input_dev *input)
if (r < 0)
goto out;
+ mutex_lock(&usbtouch->pm_mutex);
if (!usbtouch->type->irq_always) {
if (usb_submit_urb(usbtouch->irq, GFP_KERNEL)) {
r = -EIO;
@@ -1464,6 +1467,7 @@ static int usbtouch_open(struct input_dev *input)
usbtouch->interface->needs_remote_wakeup = 1;
out_put:
+ mutex_unlock(&usbtouch->pm_mutex);
usb_autopm_put_interface(usbtouch->interface);
out:
return r;
@@ -1474,8 +1478,11 @@ static void usbtouch_close(struct input_dev *input)
struct usbtouch_usb *usbtouch = input_get_drvdata(input);
int r;
+ mutex_lock(&usbtouch->pm_mutex);
if (!usbtouch->type->irq_always)
usb_kill_urb(usbtouch->irq);
+ mutex_lock(&usbtouch->pm_mutex);
+
r = usb_autopm_get_interface(usbtouch->interface);
usbtouch->interface->needs_remote_wakeup = 0;
if (!r)
@@ -1498,10 +1505,10 @@ static int usbtouch_resume(struct usb_interface *intf)
struct input_dev *input = usbtouch->input;
int result = 0;
- mutex_lock(&input->mutex);
+ mutex_lock(&usbtouch->pm_mutex);
if (input->users || usbtouch->type->irq_always)
result = usb_submit_urb(usbtouch->irq, GFP_NOIO);
- mutex_unlock(&input->mutex);
+ mutex_unlock(&usbtouch->pm_mutex);
return result;
}
@@ -1524,10 +1531,10 @@ static int usbtouch_reset_resume(struct usb_interface
*intf)
}
/* restart IO if needed */
- mutex_lock(&input->mutex);
+ mutex_lock(&usbtouch->pm_mutex);
if (input->users)
err = usb_submit_urb(usbtouch->irq, GFP_NOIO);
- mutex_unlock(&input->mutex);
+ mutex_unlock(&usbtouch->pm_mutex);
return err;
}
--
2.16.2
[RESEND PATCH 6/6] input: usbtouchscreen: do not rely on input_dev->users
If the device is unused and suspended, a call to open will cause the
device to autoresume through the call to usb_autopm_get_interface().
input_dev->users is already incremented by the input subsystem,
therefore this expression will always be evaluated to true:
if (input->users || usbtouch->type->irq_always)
result = usb_submit_urb(usbtouch->irq, GFP_NOIO);
The same URB will then be fail when resubmitted in usbtouch_open().
Introduce usbtouch->is_open to keep track of the state instead.
Signed-off-by: Marcus Folkesson
---
drivers/input/touchscreen/usbtouchscreen.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/input/touchscreen/usbtouchscreen.c
b/drivers/input/touchscreen/usbtouchscreen.c
index e964658203d8..0356ad792e40 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -114,6 +114,7 @@ struct usbtouch_usb {
struct input_dev *input;
struct usbtouch_device_info *type;
struct mutex pm_mutex; /* serialize access to open/suspend */
+ bool is_open;
char name[128];
char phys[64];
void *priv;
@@ -1466,6 +1467,7 @@ static int usbtouch_open(struct input_dev *input)
}
usbtouch->interface->needs_remote_wakeup = 1;
+ usbtouch->is_open = 1;
out_put:
mutex_unlock(&usbtouch->pm_mutex);
usb_autopm_put_interface(usbtouch->interface);
@@ -1481,6 +1483,7 @@ static void usbtouch_close(struct input_dev *input)
mutex_lock(&usbtouch->pm_mutex);
if (!usbtouch->type->irq_always)
usb_kill_urb(usbtouch->irq);
+ usbtouch->is_open = 0;
mutex_lock(&usbtouch->pm_mutex);
r = usb_autopm_get_interface(usbtouch->interface);
@@ -1502,11 +1505,10 @@ static int usbtouch_suspend
static int usbtouch_resume(struct usb_interface *intf)
{
struct usbtouch_usb *usbtouch = usb_get_intfdata(intf);
- struct input_dev *input = usbtouch->input;
int result = 0;
mutex_lock(&usbtouch->pm_mutex);
- if (input->users || usbtouch->type->irq_always)
+ if (usbtouch->is_open || usbtouch->type->irq_always)
result = usb_submit_urb(usbtouch->irq, GFP_NOIO);
mutex_unlock(&usbtouch->pm_mutex);
@@ -1516,7 +1518,6 @@ static int usbtouch_resume(struct usb_interface *intf)
static int usbtouch_reset_resume(struct usb_interface *intf)
{
struct usbtouch_usb *usbtouch = usb_get_intfdata(intf);
- struct input_dev *input = usbtouch->input;
int err = 0;
/* reinit the device */
@@ -1532,7 +1533,7 @@ static int usbtouch_reset_resume(struct usb_interface
*intf)
/* restart IO if needed */
mutex_lock(&usbtouch->pm_mutex);
- if (input->users)
+ if (usbtouch->is_open)
err = usb_submit_urb(usbtouch->irq, GFP_NOIO);
mutex_unlock(&usbtouch->pm_mutex);
--
2.16.2
[RESEND PATCH 1/6] input: synaptics_usb: fix deadlock in autosuspend
usb_autopm_get_interface() that is called in synusb_open() does an
autoresume if the device is suspended.
input_dev->mutex used in synusb_resume() is in this case already
taken by the input subsystem and will cause a deadlock.
Signed-off-by: Marcus Folkesson
---
drivers/input/mouse/synaptics_usb.c | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/input/mouse/synaptics_usb.c
b/drivers/input/mouse/synaptics_usb.c
index cb7d15d826d0..2c66913cf5a2 100644
--- a/drivers/input/mouse/synaptics_usb.c
+++ b/drivers/input/mouse/synaptics_usb.c
@@ -82,6 +82,9 @@ struct synusb {
struct urb *urb;
unsigned char *data;
+ /* serialize access to open/suspend */
+ struct mutex pm_mutex;
+
/* input device related data structures */
struct input_dev *input;
char name[128];
@@ -252,6 +255,7 @@ static int synusb_open(struct input_dev *dev)
return retval;
}
+ mutex_lock(&synusb->pm_mutex);
retval = usb_submit_urb(synusb->urb, GFP_KERNEL);
if (retval) {
dev_err(&synusb->intf->dev,
@@ -264,6 +268,7 @@ static int synusb_open(struct input_dev *dev)
synusb->intf->needs_remote_wakeup = 1;
out:
+ mutex_unlock(&synusb->pm_mutex);
usb_autopm_put_interface(synusb->intf);
return retval;
}
@@ -275,8 +280,10 @@ static void synusb_close(struct input_dev *dev)
autopm_error = usb_autopm_get_interface(synusb->intf);
+ mutex_lock(&synusb->pm_mutex);
usb_kill_urb(synusb->urb);
synusb->intf->needs_remote_wakeup = 0;
+ mutex_unlock(&synusb->pm_mutex);
if (!autopm_error)
usb_autopm_put_interface(synusb->intf);
@@ -315,6 +322,7 @@ static int synusb_probe(struct usb_interface *intf,
synusb->udev = udev;
synusb->intf = intf;
synusb->input = input_dev;
+ mutex_init(&synusb->pm_mutex);
synusb->flags = id->driver_info;
if (synusb->flags & SYNUSB_COMBO) {
@@ -466,11 +474,10 @@ static void synusb_disconnect(struct usb_interface *intf)
static int synusb_suspend(struct usb_interface *intf, pm_message_t message)
{
struct synusb *synusb = usb_get_intfdata(intf);
- struct input_dev *input_dev = synusb->input;
- mutex_lock(&input_dev->mutex);
+ mutex_lock(&synusb->pm_mutex);
usb_kill_urb(synusb->urb);
- mutex_unlock(&input_dev->mutex);
+ mutex_unlock(&synusb->pm_mutex);
return 0;
}
@@ -481,14 +488,14 @@ static int synusb_resume(struct usb_interface *intf)
struct input_dev *input_dev = synusb->input;
int retval = 0;
- mutex_lock(&input_dev->mutex);
+ mutex_lock(&synusb->pm_mutex);
if ((input_dev->users || (synusb->flags & SYNUSB_IO_ALWAYS)) &&
usb_submit_urb(synusb->urb, GFP_NOIO) < 0) {
retval = -EIO;
}
- mutex_unlock(&input_dev->mutex);
+ mutex_unlock(&synusb->pm_mutex);
return retval;
}
@@ -496,9 +503,8 @@ static int synusb_resume(struct usb_interface *intf)
static int synusb_pre_reset(struct usb_interface *intf)
{
struct synusb *synusb = usb_get_intfdata(intf);
- struct input_dev *input_dev = synusb->input;
- mutex_lock(&input_dev->mutex);
+ mutex_lock(&synusb->pm_mutex);
usb_kill_urb(synusb->urb);
return 0;
@@ -515,7 +521,7 @@ static int synusb_post_reset(struct usb_interface *intf)
retval = -EIO;
}
- mutex_unlock(&input_dev->mutex);
+ mutex_unlock(&synusb->pm_mutex);
return retval;
}
--
2.16.2
[RESEND PATCH v5] input: pxrc: new driver for PhoenixRC Flight Controller Adapter
This driver let you plug in your RC controller to the adapter and
use it as input device in various RC simulators.
Signed-off-by: Marcus Folkesson
---
v5:
- Drop autosuspend support
- Use pm_mutex instead of input_dev->mutex
- Use pxrc->is_open instead of input_dev->users
v4:
- Add call to usb_mark_last_busy() in irq
- Move code from pxrc_resume() to pxrc_reset_resume()
v3:
- Use RUDDER and MISC instead of TILT_X and TILT_Y
- Drop kref and anchor
- Rework URB handling
- Add PM support
v2:
- Change module license to GPLv2 to match SPDX tag
Documentation/input/devices/pxrc.rst | 57 +++
drivers/input/joystick/Kconfig | 9 ++
drivers/input/joystick/Makefile | 1 +
drivers/input/joystick/pxrc.c| 303 +++
4 files changed, 370 insertions(+)
create mode 100644 Documentation/input/devices/pxrc.rst
create mode 100644 drivers/input/joystick/pxrc.c
diff --git a/Documentation/input/devices/pxrc.rst
b/Documentation/input/devices/pxrc.rst
new file mode 100644
index ..ca11f646bae8
--- /dev/null
+++ b/Documentation/input/devices/pxrc.rst
@@ -0,0 +1,57 @@
+===
+pxrc - PhoenixRC Flight Controller Adapter
+===
+
+:Author: Marcus Folkesson
+
+This driver let you use your own RC controller plugged into the
+adapter that comes with PhoenixRC [1]_ or other compatible adapters.
+
+The adapter supports 7 analog channels and 1 digital input switch.
+
+Notes
+=
+
+Many RC controllers is able to configure which stick goes to which channel.
+This is also configurable in most simulators, so a matching is not necessary.
+
+The driver is generating the following input event for analog channels:
+
++-++
+| Channel | Event |
++=++
+| 1 | ABS_X |
++-++
+| 2 | ABS_Y |
++-++
+| 3 | ABS_RX|
++-++
+| 4 | ABS_RY|
++-++
+| 5 | ABS_RUDDER|
++-++
+| 6 | ABS_THROTTLE |
++-++
+| 7 | ABS_MISC |
++-++
+
+The digital input switch is generated as an `BTN_A` event.
+
+Manual Testing
+==
+
+To test this driver's functionality you may use `input-event` which is part of
+the `input layer utilities` suite [2]_.
+
+For example::
+
+> modprobe pxrc
+> input-events
+
+To print all input events from input `devnr`.
+
+References
+==
+
+.. [1] http://www.phoenix-sim.com/
+.. [2] https://www.kraxel.org/cgit/input/
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index f3c2f6ea8b44..332c0cc1b2ab 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -351,4 +351,13 @@ config JOYSTICK_PSXPAD_SPI_FF
To drive rumble motor a dedicated power supply is required.
+config JOYSTICK_PXRC
+ tristate "PhoenixRC Flight Controller Adapter"
+ depends on USB_ARCH_HAS_HCD
+ depends on USB
+ help
+ Say Y here if you want to use the PhoenixRC Flight Controller Adapter.
+
+ To compile this driver as a module, choose M here: the
+ module will be called pxrc.
endif
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 67651efda2e1..dd0492ebbed7 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_JOYSTICK_JOYDUMP)+= joydump.o
obj-$(CONFIG_JOYSTICK_MAGELLAN)+= magellan.o
obj-$(CONFIG_JOYSTICK_MAPLE) += maplecontrol.o
obj-$(CONFIG_JOYSTICK_PSXPAD_SPI) += psxpad-spi.o
+obj-$(CONFIG_JOYSTICK_PXRC)+= pxrc.o
obj-$(CONFIG_JOYSTICK_SIDEWINDER) += sidewinder.o
obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o
obj-$(CONFIG_JOYSTICK_SPACEORB)+= spaceorb.o
diff --git a/drivers/input/joystick/pxrc.c b/drivers/input/joystick/pxrc.c
new file mode 100644
index ..07a0dbd3ced2
--- /dev/null
+++ b/drivers/input/joystick/pxrc.c
@@ -0,0 +1,303 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Phoenix RC Flight Controller Adapter
+ *
+ * Copyright (C) 2018 Marcus Folkesson
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define PXRC_VENDOR_ID (0x1781)
+#define PXRC_PRODUCT_ID(0x0898)
+
+static const struct usb_device_id pxrc_table[] = {
+ { USB_DEVICE(PXRC_VENDOR_ID, PXRC_PRODUCT_ID) },
+ { }
+};
+MODULE_DEVICE_TABLE(usb, pxrc_table);
+
+struct pxrc {
+ struct input_dev*input;
+ struct usb_device *udev;
+ struct usb_interfac
Re: [v3,04/11] watchdog/hpwdt: white space changes
On Sat, Feb 17, 2018 at 12:32:08PM -0700, Jerry Hoemann wrote:
> On Sat, Feb 17, 2018 at 08:17:34AM -0800, Guenter Roeck wrote:
> > On Thu, Feb 15, 2018 at 04:43:53PM -0700, Jerry Hoemann wrote:
> > > Minor white space changes and some name clean up.
> > >
> > > Signed-off-by: Jerry Hoemann
> > > ---
> > > MODULE_DEVICE_TABLE(pci, hpwdt_devices);
> > >
> > > @@ -102,7 +100,7 @@ static int hpwdt_time_left(void)
> > > return TICKS_TO_SECS(ioread16(hpwdt_timer_reg));
> > > }
> > >
> > > -#ifdef CONFIG_HPWDT_NMI_DECODING
> > > +#ifdef CONFIG_HPWDT_NMI_DECODING /* { */
> > > static int hpwdt_my_nmi(void)
> > > {
> > > return ioread8(hpwdt_nmistat) & 0x6;
> > > @@ -133,7 +131,7 @@ static int hpwdt_pretimeout(unsigned int ulReason,
> > > struct pt_regs *regs)
> > >
> > > return NMI_HANDLED;
> > > }
> > > -#endif /* CONFIG_HPWDT_NMI_DECODING */
> > > +#endif /* } */
> >
> > I disagree with those changes. While I don't object to adding the '{'
> > per se, I find it very useful to have the 'CONFIG_HPWDT_NMI_DECODING'
> > with an endif to be able to associate it with the matching #ifdef.
Well, it does not follow our coding style.
Documentation/process/coding-style.rst:
At the end of any non-trivial #if or #ifdef block (more than a few
lines),
place a comment after the #endif on the same line, noting the
conditional
expression used. For instance:
.. code-block:: c
#ifdef CONFIG_SOMETHING
...
#endif /* CONFIG_SOMETHING */
>
> The matching /* { */ and /* } */ allow for quickly the finding of the
> matching ifdef/endif.
>
> In the "vim" editor, the command '%' will take one from one curly paren to its
> matching curly paren...
'%' in vim let you jump between matching ifdef/endif as well.
>
> There is a similar sequence for emacs.
>
> >
> > Deferring to Wim.
> >
> > Guenter
> >
>
> --
>
> -
> Jerry Hoemann Software Engineer Hewlett Packard Enterprise
> -
Thanks,
Marcus Folkesson
signature.asc
Description: PGP signature
Re: [PATCH v2 08/11] watchdog/hpwdt: Programable Pretimeout NMI
.ping = hpwdt_ping,
> .set_timeout= hpwdt_settimeout,
> .get_timeleft = hpwdt_gettimeleft,
> +#ifdef CONFIG_HPWDT_NMI_DECODING /* { */
> + .set_pretimeout = hpwdt_set_pretimeout,
> +#endif /* } */
> };
>
> static struct watchdog_device hpwdt_dev = {
> @@ -304,6 +328,9 @@ static struct watchdog_device hpwdt_dev = {
> .min_timeout= 1,
> .max_timeout= HPWDT_MAX_TIMER,
> .timeout= DEFAULT_MARGIN,
> +#ifdef CONFIG_HPWDT_NMI_DECODING /* { */
> + .pretimeout = PRETIMEOUT_SEC,
> +#endif /* } */
> };
>
>
> @@ -322,6 +349,9 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped
> once started (default="
> #ifdef CONFIG_HPWDT_NMI_DECODING /* { */
> module_param(allow_kdump, int, 0444);
> MODULE_PARM_DESC(allow_kdump, "Start a kernel dump after NMI occurs");
> +
> +module_param(pretimeout, bool, 0444);
> +MODULE_PARM_DESC(pretimeout, "Watchdog pretimeout enabled");
> #endif /* } */
>
> module_pci_driver(hpwdt_driver);
> --
> 2.13.6
Best regards
Marcus Folkesson
>
signature.asc
Description: PGP signature
Re: [PATCH v2 06/11] watchdog/hpwdt: Modify to use watchdog core.
pci_dev *dev)
> if (retval)
> goto error2;
>
> - dev_info(&dev->dev,
> - "HPE Watchdog Timer Driver: NMI decoding initialized"
> - ", allow kernel dump: %s (default = 1/ON)\n",
> - (allow_kdump == 0) ? "OFF" : "ON");
> + dev_info(&dev->dev, "HPE Watchdog Timer Driver: NMI decoding
> initialized");
> return 0;
>
> error2:
> @@ -381,31 +236,32 @@ static int hpwdt_probe(struct pci_dev *dev, const
> struct pci_device_id *ent)
> hpwdt_timer_con = pci_mem_addr + 0x72;
>
> /* Make sure that timer is disabled until /dev/watchdog is opened */
> - hpwdt_stop();
> -
> - /* Make sure that we have a valid soft_margin */
> - if (hpwdt_change_timer(soft_margin))
> - hpwdt_change_timer(DEFAULT_MARGIN);
> + hpwdt_stop(&hpwdt_dev);
>
> /* Initialize NMI Decoding functionality */
> retval = hpwdt_init_nmi_decoding(dev);
> if (retval != 0)
> goto error_init_nmi_decoding;
>
> - retval = misc_register(&hpwdt_miscdev);
> + retval = watchdog_register_device(&hpwdt_dev);
> if (retval < 0) {
> - dev_warn(&dev->dev,
> - "Unable to register miscdev on minor=%d (err=%d).\n",
> - WATCHDOG_MINOR, retval);
> - goto error_misc_register;
> + dev_warn(&dev->dev, "Unable to register hpe watchdog
> (err=%d).\n", retval);
> + goto error_wd_register;
> + }
> +
> + watchdog_set_nowayout(&hpwdt_dev, nowayout);
> + if (watchdog_init_timeout(&hpwdt_dev, soft_margin, NULL)) {
> + dev_warn(&dev->dev, "Invalid soft_margin: %d. Using default\n",
> soft_margin);
> + soft_margin = DEFAULT_MARGIN;
> }
In this case watchdog_init_timout() will:
1) Check if soft_margin is valid
2) if not, keep hpwdt_dev.timout intact (which is already set in
declaration of hpwdt_dev)
So we could remove the condition and only keep
watchdog_init_timeout(&hpwdt_dev, soft_margin, NULL);
Also, we need to set an invalid initial value for soft_margin to make
the logic for watchdog_init_timeout work.
i.e
- static unsigned int soft_margin = DEFAULT_MARGIN; /* in seconds */
+ static unsigned int soft_margin;
>
> dev_info(&dev->dev, "HPE Watchdog Timer Driver: %s"
> ", timer margin: %d seconds (nowayout=%d).\n",
> - HPWDT_VERSION, soft_margin, nowayout);
print hpwdt_dev.timeout instead of soft_margin
> + HPWDT_VERSION, hpwdt_dev.timeout, nowayout);
> +
> return 0;
>
> -error_misc_register:
> +error_wd_register:
> hpwdt_exit_nmi_decoding();
> error_init_nmi_decoding:
> pci_iounmap(dev, pci_mem_addr);
> @@ -417,9 +273,9 @@ static int hpwdt_probe(struct pci_dev *dev, const struct
> pci_device_id *ent)
> static void hpwdt_exit(struct pci_dev *dev)
> {
> if (!nowayout)
> - hpwdt_stop();
> + hpwdt_stop(&hpwdt_dev);
>
> - misc_deregister(&hpwdt_miscdev);
> + watchdog_unregister_device(&hpwdt_dev);
> hpwdt_exit_nmi_decoding();
> pci_iounmap(dev, pci_mem_addr);
> pci_disable_device(dev);
> @@ -432,6 +288,25 @@ static struct pci_driver hpwdt_driver = {
> .remove = hpwdt_exit,
> };
>
> +
> +static const struct watchdog_ops hpwdt_ops = {
> + .owner = THIS_MODULE,
> + .start = hpwdt_start,
> + .stop = hpwdt_stop,
> + .ping = hpwdt_ping,
> + .set_timeout= hpwdt_settimeout,
> + .get_timeleft = hpwdt_gettimeleft,
> +};
> +
> +static struct watchdog_device hpwdt_dev = {
> + .info = &hpwdt_info,
> + .ops= &hpwdt_ops,
> + .min_timeout= 1,
> + .max_timeout= HPWDT_MAX_TIMER,
> + .timeout= DEFAULT_MARGIN,
> +};
> +
> +
> MODULE_AUTHOR("Jerry Hoemann");
> MODULE_DESCRIPTION("hpe watchdog driver");
> MODULE_LICENSE("GPL");
> --
> 2.13.6
>
Best regards
Marcus Folkesson
signature.asc
Description: PGP signature
[PATCH v3 3/7] watchdog: sirfsoc: allow setting timeout in devicetree
watchdog_init_timeout() will allways pick timeout_param since it
defaults to a valid timeout.
By following best practice described in
Documentation/watchdog/watchdog-kernel-api.txt, it also
let us to set timout-sec property in devicetree.
Signed-off-by: Marcus Folkesson
Reviewed-by: Guenter Roeck
---
Documentation/devicetree/bindings/watchdog/sirfsoc_wdt.txt | 4
drivers/watchdog/sirfsoc_wdt.c | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/watchdog/sirfsoc_wdt.txt
b/Documentation/devicetree/bindings/watchdog/sirfsoc_wdt.txt
index 9cbc76c89b2b..0dce5e3100b4 100644
--- a/Documentation/devicetree/bindings/watchdog/sirfsoc_wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/sirfsoc_wdt.txt
@@ -5,10 +5,14 @@ Required properties:
- reg: Address range of tick timer/WDT register set
- interrupts: interrupt number to the cpu
+Optional properties:
+- timeout-sec : Contains the watchdog timeout in seconds
+
Example:
timer@b002 {
compatible = "sirf,prima2-tick";
reg = <0xb002 0x1000>;
interrupts = <0>;
+ timeout-sec = <30>;
};
diff --git a/drivers/watchdog/sirfsoc_wdt.c b/drivers/watchdog/sirfsoc_wdt.c
index 4eea351e09b0..ac0c9d2c4aee 100644
--- a/drivers/watchdog/sirfsoc_wdt.c
+++ b/drivers/watchdog/sirfsoc_wdt.c
@@ -29,7 +29,7 @@
#define SIRFSOC_WDT_MAX_TIMEOUT(10 * 60) /* 10 mins */
#define SIRFSOC_WDT_DEFAULT_TIMEOUT30 /* 30 secs */
-static unsigned int timeout = SIRFSOC_WDT_DEFAULT_TIMEOUT;
+static unsigned int timeout;
static bool nowayout = WATCHDOG_NOWAYOUT;
module_param(timeout, uint, 0);
--
2.15.1
[PATCH v3 6/7] watchdog: meson: allow setting timeout in devicetree
watchdog_init_timeout() will allways pick timeout_param since it
defaults to a valid timeout.
By following best practice described in
Documentation/watchdog/watchdog-kernel-api.txt, it also
let us to set timout-sec property in devicetree.
Signed-off-by: Marcus Folkesson
Reviewed-by: Guenter Roeck
---
Documentation/devicetree/bindings/watchdog/meson-wdt.txt | 4
drivers/watchdog/meson_wdt.c | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/watchdog/meson-wdt.txt
b/Documentation/devicetree/bindings/watchdog/meson-wdt.txt
index 8a6d84cb36c9..7588cc3971bf 100644
--- a/Documentation/devicetree/bindings/watchdog/meson-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/meson-wdt.txt
@@ -9,9 +9,13 @@ Required properties:
"amlogic,meson8m2-wdt" and "amlogic,meson8b-wdt" on Meson8m2 SoCs
- reg : Specifies base physical address and size of the registers.
+Optional properties:
+- timeout-sec: contains the watchdog timeout in seconds.
+
Example:
wdt: watchdog@c1109900 {
compatible = "amlogic,meson6-wdt";
reg = <0xc1109900 0x8>;
+ timeout-sec = <10>;
};
diff --git a/drivers/watchdog/meson_wdt.c b/drivers/watchdog/meson_wdt.c
index 304274c67735..cd0275a6cdac 100644
--- a/drivers/watchdog/meson_wdt.c
+++ b/drivers/watchdog/meson_wdt.c
@@ -36,7 +36,7 @@
#define MESON_SEC_TO_TC(s, c) ((s) * (c))
static bool nowayout = WATCHDOG_NOWAYOUT;
-static unsigned int timeout = MESON_WDT_TIMEOUT;
+static unsigned int timeout;
struct meson_wdt_data {
unsigned int enable;
--
2.15.1
[PATCH v3 5/7] watchdog: mtk: allow setting timeout in devicetree
watchdog_init_timeout() will allways pick timeout_param since it
defaults to a valid timeout.
By following best practice described in
Documentation/watchdog/watchdog-kernel-api.txt, it also
let us to set timout-sec property in devicetree.
Signed-off-by: Marcus Folkesson
Reviewed-by: Guenter Roeck
---
Documentation/devicetree/bindings/watchdog/mtk-wdt.txt | 4
drivers/watchdog/mtk_wdt.c | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
b/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
index 5b38a30e608c..859dee167b91 100644
--- a/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/mtk-wdt.txt
@@ -11,9 +11,13 @@ Required properties:
- reg : Specifies base physical address and size of the registers.
+Optional properties:
+- timeout-sec: contains the watchdog timeout in seconds.
+
Example:
wdt: watchdog@1000 {
compatible = "mediatek,mt6589-wdt";
reg = <0x1000 0x18>;
+ timeout-sec = <10>;
};
diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
index 7ed417a765c7..fcdc10ec28a3 100644
--- a/drivers/watchdog/mtk_wdt.c
+++ b/drivers/watchdog/mtk_wdt.c
@@ -57,7 +57,7 @@
#define DRV_VERSION"1.0"
static bool nowayout = WATCHDOG_NOWAYOUT;
-static unsigned int timeout = WDT_MAX_TIMEOUT;
+static unsigned int timeout;
struct mtk_wdt_dev {
struct watchdog_device wdt_dev;
--
2.15.1
[PATCH v3 2/7] watchdog: sunxi: allow setting timeout in devicetree
watchdog_init_timeout() will allways pick timeout_param since it
defaults to a valid timeout.
By following best practice described in
Documentation/watchdog/watchdog-kernel-api.txt, it also
let us to set timout-sec property in devicetree.
Signed-off-by: Marcus Folkesson
Reviewed-by: Guenter Roeck
---
Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt | 4
drivers/watchdog/sunxi_wdt.c | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
b/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
index 62dd5baad70e..49900e72f6b1 100644
--- a/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
@@ -6,9 +6,13 @@ Required properties:
"allwinner,sun6i-a31-wdt"
- reg : Specifies base physical address and size of the registers.
+Optional properties:
+- timeout-sec : Contains the watchdog timeout in seconds
+
Example:
wdt: watchdog@1c20c90 {
compatible = "allwinner,sun4i-a10-wdt";
reg = <0x01c20c90 0x10>;
+ timeout-sec = <10>;
};
diff --git a/drivers/watchdog/sunxi_wdt.c b/drivers/watchdog/sunxi_wdt.c
index 9728fa32c357..55f166bec0ca 100644
--- a/drivers/watchdog/sunxi_wdt.c
+++ b/drivers/watchdog/sunxi_wdt.c
@@ -39,7 +39,7 @@
#define DRV_VERSION"1.0"
static bool nowayout = WATCHDOG_NOWAYOUT;
-static unsigned int timeout = WDT_MAX_TIMEOUT;
+static unsigned int timeout;
/*
* This structure stores the register offsets for different variants
--
2.15.1
[PATCH v3 1/7] watchdog: sama5d4: make use of timeout-secs provided in devicetree
watchdog_init_timeout() will allways pick timeout_param since it
defaults to a valid timeout.
Following best practice described in
Documentation/watchdog/watchdog-kernel-api.txt to make use of
the parameter logic.
Signed-off-by: Marcus Folkesson
---
Notes:
v3:
- Use wdd->timeout instead of wdt_timeout when print out
timout in probe function.
drivers/watchdog/sama5d4_wdt.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
index 0ae947c3d7bc..255169916dbb 100644
--- a/drivers/watchdog/sama5d4_wdt.c
+++ b/drivers/watchdog/sama5d4_wdt.c
@@ -33,7 +33,7 @@ struct sama5d4_wdt {
unsigned long last_ping;
};
-static int wdt_timeout = WDT_DEFAULT_TIMEOUT;
+static int wdt_timeout;
static bool nowayout = WATCHDOG_NOWAYOUT;
module_param(wdt_timeout, int, 0);
@@ -212,7 +212,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
return -ENOMEM;
wdd = &wdt->wdd;
- wdd->timeout = wdt_timeout;
+ wdd->timeout = WDT_DEFAULT_TIMEOUT;
wdd->info = &sama5d4_wdt_info;
wdd->ops = &sama5d4_wdt_ops;
wdd->min_timeout = MIN_WDT_TIMEOUT;
@@ -273,7 +273,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, wdt);
dev_info(&pdev->dev, "initialized (timeout = %d sec, nowayout = %d)\n",
-wdt_timeout, nowayout);
+wdd->timeout, nowayout);
return 0;
}
--
2.15.1
[PATCH v3 7/7] watchdog: coh901327: make use of timeout-secs provided in devicetree
watchdog_init_timeout() will allways pick timeout_param since it
defaults to a valid timeout.
Following best practice described in
Documentation/watchdog/watchdog-kernel-api.txt to make use of
the parameter logic.
Signed-off-by: Marcus Folkesson
---
Notes:
v3:
- Reformat and use coh901327_wdt.timeout instead of margin when print
out
timout in probe function.
v2:
- Set .timeout in coh901327_wdt structure declaration.
- Set .min_timeout to 1 instead of 0. I could not find a datasheet
for coh901327, so I'm not sure if 0 is valid. However, 0 seems
wrong to me and most driver has 1 as min value. If it should
be 0, please let me know and I have to set another initial
value for margin.
drivers/watchdog/coh901327_wdt.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/watchdog/coh901327_wdt.c b/drivers/watchdog/coh901327_wdt.c
index 4410337f4f7f..500af8a7ec5a 100644
--- a/drivers/watchdog/coh901327_wdt.c
+++ b/drivers/watchdog/coh901327_wdt.c
@@ -67,7 +67,9 @@
#define U300_WDOG_IFR_WILL_BARK_IRQ_FORCE_ENABLE 0x0001U
/* Default timeout in seconds = 1 minute */
-static unsigned int margin = 60;
+#define U300_WDOG_DEFAULT_TIMEOUT 60
+
+static unsigned int margin;
static int irq;
static void __iomem *virtbase;
static struct device *parent;
@@ -235,8 +237,9 @@ static struct watchdog_device coh901327_wdt = {
* timeout register is max
* 0x7FFF = 327670ms ~= 327s.
*/
- .min_timeout = 0,
+ .min_timeout = 1,
.max_timeout = 327,
+ .timeout = U300_WDOG_DEFAULT_TIMEOUT,
};
static int __exit coh901327_remove(struct platform_device *pdev)
@@ -315,16 +318,15 @@ static int __init coh901327_probe(struct platform_device
*pdev)
goto out_no_irq;
}
- ret = watchdog_init_timeout(&coh901327_wdt, margin, dev);
- if (ret < 0)
- coh901327_wdt.timeout = 60;
+ watchdog_init_timeout(&coh901327_wdt, margin, dev);
coh901327_wdt.parent = dev;
ret = watchdog_register_device(&coh901327_wdt);
if (ret)
goto out_no_wdog;
- dev_info(dev, "initialized. timer margin=%d sec\n", margin);
+ dev_info(dev, "initialized. (timeout=%d sec)\n",
+ coh901327_wdt.timeout);
return 0;
out_no_wdog:
--
2.15.1
[PATCH v3 4/7] watchdog: pnx4008: make use of timeout-secs provided in devicetree
watchdog_init_timeout() will allways pick timeout_param since it defaults to a valid timeout. Following best practice described in Documentation/watchdog/watchdog-kernel-api.txt to make use of the parameter logic. Signed-off-by: Marcus Folkesson Reviewed-by: Guenter Roeck --- drivers/watchdog/pnx4008_wdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/watchdog/pnx4008_wdt.c b/drivers/watchdog/pnx4008_wdt.c index 0529aed158a4..8e261799c84e 100644 --- a/drivers/watchdog/pnx4008_wdt.c +++ b/drivers/watchdog/pnx4008_wdt.c @@ -78,7 +78,7 @@ #define WDOG_COUNTER_RATE 1300 /*the counter clock is 13 MHz fixed */ static bool nowayout = WATCHDOG_NOWAYOUT; -static unsigned int heartbeat = DEFAULT_HEARTBEAT; +static unsigned int heartbeat; static DEFINE_SPINLOCK(io_lock); static void __iomem*wdt_base; -- 2.15.1
[PATCH 1/4] watchdog: uniphier: change order for setting default timeout
watchdog_init_timeout() will preserve wdd->timeout value if no parameter
nor timeout-secs dt property is set.
Signed-off-by: Marcus Folkesson
---
drivers/watchdog/uniphier_wdt.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/watchdog/uniphier_wdt.c b/drivers/watchdog/uniphier_wdt.c
index 0ea2339d9702..0e4f8d53ce3c 100644
--- a/drivers/watchdog/uniphier_wdt.c
+++ b/drivers/watchdog/uniphier_wdt.c
@@ -212,11 +212,10 @@ static int uniphier_wdt_probe(struct platform_device
*pdev)
wdev->wdt_dev.ops = &uniphier_wdt_ops;
wdev->wdt_dev.max_timeout = WDT_PERIOD_MAX;
wdev->wdt_dev.min_timeout = WDT_PERIOD_MIN;
+ wdev->wdt_dev.timeout = WDT_DEFAULT_TIMEOUT;
wdev->wdt_dev.parent = dev;
- if (watchdog_init_timeout(&wdev->wdt_dev, timeout, dev) < 0) {
- wdev->wdt_dev.timeout = WDT_DEFAULT_TIMEOUT;
- }
+ watchdog_init_timeout(&wdev->wdt_dev, timeout, dev);
watchdog_set_nowayout(&wdev->wdt_dev, nowayout);
watchdog_stop_on_reboot(&wdev->wdt_dev);
--
2.15.1
[PATCH 4/4] watchdog: lpc18xx: remove assignment of unused ret-value
Signed-off-by: Marcus Folkesson --- drivers/watchdog/lpc18xx_wdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/watchdog/lpc18xx_wdt.c b/drivers/watchdog/lpc18xx_wdt.c index b4221f43cd94..331cadb459ac 100644 --- a/drivers/watchdog/lpc18xx_wdt.c +++ b/drivers/watchdog/lpc18xx_wdt.c @@ -265,7 +265,7 @@ static int lpc18xx_wdt_probe(struct platform_device *pdev) lpc18xx_wdt->wdt_dev.parent = dev; watchdog_set_drvdata(&lpc18xx_wdt->wdt_dev, lpc18xx_wdt); - ret = watchdog_init_timeout(&lpc18xx_wdt->wdt_dev, heartbeat, dev); + watchdog_init_timeout(&lpc18xx_wdt->wdt_dev, heartbeat, dev); __lpc18xx_wdt_set_timeout(lpc18xx_wdt); -- 2.15.1
[PATCH 3/4] watchdog: gpio: change order for setting default timeout
watchdog_init_timeout() will preserve wdd->timeout value if no parameter nor timeout-secs dt property is set. Signed-off-by: Marcus Folkesson --- drivers/watchdog/gpio_wdt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c index cb66c2f99ff1..d0e8203f7a60 100644 --- a/drivers/watchdog/gpio_wdt.c +++ b/drivers/watchdog/gpio_wdt.c @@ -156,9 +156,9 @@ static int gpio_wdt_probe(struct platform_device *pdev) priv->wdd.min_timeout = SOFT_TIMEOUT_MIN; priv->wdd.max_hw_heartbeat_ms = hw_margin; priv->wdd.parent= &pdev->dev; + priv->wdd.timeout = SOFT_TIMEOUT_DEF; - if (watchdog_init_timeout(&priv->wdd, 0, &pdev->dev) < 0) - priv->wdd.timeout = SOFT_TIMEOUT_DEF; + watchdog_init_timeout(&priv->wdd, 0, &pdev->dev); watchdog_stop_on_reboot(&priv->wdd); -- 2.15.1
