[PATCH v4 1/3] ASoC: pcm179x: Split into core and SPI parts
The pcm179x family supports both SPI and I2C for configuration. This patch splits the driver into core and SPI parts, in preparation for I2C support. Reviewed-by: Johan Hovold Signed-off-by: Jacob Siverskog --- sound/soc/codecs/Kconfig | 11 +-- sound/soc/codecs/Makefile | 2 ++ sound/soc/codecs/pcm179x-spi.c | 72 ++ sound/soc/codecs/pcm179x.c | 52 +++--- sound/soc/codecs/pcm179x.h | 5 +++ 5 files changed, 99 insertions(+), 43 deletions(-) create mode 100644 sound/soc/codecs/pcm179x-spi.c diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 50693c8..ae720f1 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -87,7 +87,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_ML26124 if I2C select SND_SOC_NAU8825 if I2C select SND_SOC_PCM1681 if I2C - select SND_SOC_PCM179X if SPI_MASTER + select SND_SOC_PCM179X_SPI if SPI_MASTER select SND_SOC_PCM3008 select SND_SOC_PCM3168A_I2C if I2C select SND_SOC_PCM3168A_SPI if SPI_MASTER @@ -527,8 +527,15 @@ config SND_SOC_PCM1681 depends on I2C config SND_SOC_PCM179X - tristate "Texas Instruments PCM179X CODEC" + tristate + +config SND_SOC_PCM179X_SPI + tristate "Texas Instruments PCM179X CODEC (SPI)" depends on SPI_MASTER + select SND_SOC_PCM179X + help + Enable support for Texas Instruments PCM179x CODEC. + Select this if your PCM179x is connected via an SPI bus. config SND_SOC_PCM3008 tristate diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index d44f7d3..56e94d8 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -81,6 +81,7 @@ snd-soc-ml26124-objs := ml26124.o snd-soc-nau8825-objs := nau8825.o snd-soc-pcm1681-objs := pcm1681.o snd-soc-pcm179x-codec-objs := pcm179x.o +snd-soc-pcm179x-spi-objs := pcm179x-spi.o snd-soc-pcm3008-objs := pcm3008.o snd-soc-pcm3168a-objs := pcm3168a.o snd-soc-pcm3168a-i2c-objs := pcm3168a-i2c.o @@ -285,6 +286,7 @@ obj-$(CONFIG_SND_SOC_ML26124) += snd-soc-ml26124.o obj-$(CONFIG_SND_SOC_NAU8825) += snd-soc-nau8825.o obj-$(CONFIG_SND_SOC_PCM1681) += snd-soc-pcm1681.o obj-$(CONFIG_SND_SOC_PCM179X) += snd-soc-pcm179x-codec.o +obj-$(CONFIG_SND_SOC_PCM179X_SPI) += snd-soc-pcm179x-spi.o obj-$(CONFIG_SND_SOC_PCM3008) += snd-soc-pcm3008.o obj-$(CONFIG_SND_SOC_PCM3168A) += snd-soc-pcm3168a.o obj-$(CONFIG_SND_SOC_PCM3168A_I2C) += snd-soc-pcm3168a-i2c.o diff --git a/sound/soc/codecs/pcm179x-spi.c b/sound/soc/codecs/pcm179x-spi.c new file mode 100644 index 000..da924d4 --- /dev/null +++ b/sound/soc/codecs/pcm179x-spi.c @@ -0,0 +1,72 @@ +/* + * PCM179X ASoC SPI driver + * + * Copyright (c) Amarula Solutions B.V. 2013 + * + * Michael Trimarchi + * + * 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. + */ + +#include +#include +#include +#include + +#include "pcm179x.h" + +static int pcm179x_spi_probe(struct spi_device *spi) +{ + struct regmap *regmap; + int ret; + + regmap = devm_regmap_init_spi(spi, _regmap_config); + if (IS_ERR(regmap)) { + ret = PTR_ERR(regmap); + dev_err(>dev, "Failed to allocate regmap: %d\n", ret); + return ret; + } + + return pcm179x_common_init(>dev, regmap); +} + +static int pcm179x_spi_remove(struct spi_device *spi) +{ + return pcm179x_common_exit(>dev); +} + +static const struct of_device_id pcm179x_of_match[] = { + { .compatible = "ti,pcm1792a", }, + { } +}; +MODULE_DEVICE_TABLE(of, pcm179x_of_match); + +static const struct spi_device_id pcm179x_spi_ids[] = { + { "pcm179x", 0 }, + { }, +}; +MODULE_DEVICE_TABLE(spi, pcm179x_spi_ids); + +static struct spi_driver pcm179x_spi_driver = { + .driver = { + .name = "pcm179x", + .of_match_table = of_match_ptr(pcm179x_of_match), + }, + .id_table = pcm179x_spi_ids, + .probe = pcm179x_spi_probe, + .remove = pcm179x_spi_remove, +}; + +module_spi_driver(pcm179x_spi_driver); + +MODULE_DESCRIPTION("ASoC PCM179X SPI driver"); +MODULE_AUTHOR("Michael Trimarchi "); +MODULE_LICENSE("GPL"); diff --git a/sound/soc/codecs/pcm179x.c b/sound/soc/codecs/pcm179x.c index a56c7b7..8f20f70 100644 --- a/sound/soc/codecs/pcm179x.c +++ b/so
[PATCH v4 2/3] ASoC: pcm179x: Add I2C interface driver
The PCM179x family supports both SPI and I2C. This patch adds support for the I2C interface. Reviewed-by: Johan Hovold Signed-off-by: Jacob Siverskog --- .../devicetree/bindings/sound/pcm179x.txt | 11 +++- sound/soc/codecs/Kconfig | 9 +++ sound/soc/codecs/Makefile | 2 + sound/soc/codecs/pcm179x-i2c.c | 73 ++ 4 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 sound/soc/codecs/pcm179x-i2c.c diff --git a/Documentation/devicetree/bindings/sound/pcm179x.txt b/Documentation/devicetree/bindings/sound/pcm179x.txt index 4ae70d3..436c2b2 100644 --- a/Documentation/devicetree/bindings/sound/pcm179x.txt +++ b/Documentation/devicetree/bindings/sound/pcm179x.txt @@ -1,6 +1,6 @@ Texas Instruments pcm179x DT bindings -This driver supports the SPI bus. +This driver supports both the I2C and SPI bus. Required properties: @@ -9,6 +9,11 @@ Required properties: For required properties on SPI, please consult Documentation/devicetree/bindings/spi/spi-bus.txt +Required properties on I2C: + + - reg: the I2C address + + Examples: codec_spi: 1792a@0 { @@ -16,3 +21,7 @@ Examples: spi-max-frequency = <60>; }; + codec_i2c: 1792a@4c { + compatible = "ti,pcm1792a"; + reg = <0x4c>; + }; diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index ae720f1..3a8fcf1 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -87,6 +87,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_ML26124 if I2C select SND_SOC_NAU8825 if I2C select SND_SOC_PCM1681 if I2C + select SND_SOC_PCM179X_I2C if I2C select SND_SOC_PCM179X_SPI if SPI_MASTER select SND_SOC_PCM3008 select SND_SOC_PCM3168A_I2C if I2C @@ -529,6 +530,14 @@ config SND_SOC_PCM1681 config SND_SOC_PCM179X tristate +config SND_SOC_PCM179X_I2C + tristate "Texas Instruments PCM179X CODEC (I2C)" + depends on I2C + select SND_SOC_PCM179X + help + Enable support for Texas Instruments PCM179x CODEC. + Select this if your PCM179x is connected via an I2C bus. + config SND_SOC_PCM179X_SPI tristate "Texas Instruments PCM179X CODEC (SPI)" depends on SPI_MASTER diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index 56e94d8..9acd777 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -81,6 +81,7 @@ snd-soc-ml26124-objs := ml26124.o snd-soc-nau8825-objs := nau8825.o snd-soc-pcm1681-objs := pcm1681.o snd-soc-pcm179x-codec-objs := pcm179x.o +snd-soc-pcm179x-i2c-objs := pcm179x-i2c.o snd-soc-pcm179x-spi-objs := pcm179x-spi.o snd-soc-pcm3008-objs := pcm3008.o snd-soc-pcm3168a-objs := pcm3168a.o @@ -286,6 +287,7 @@ obj-$(CONFIG_SND_SOC_ML26124) += snd-soc-ml26124.o obj-$(CONFIG_SND_SOC_NAU8825) += snd-soc-nau8825.o obj-$(CONFIG_SND_SOC_PCM1681) += snd-soc-pcm1681.o obj-$(CONFIG_SND_SOC_PCM179X) += snd-soc-pcm179x-codec.o +obj-$(CONFIG_SND_SOC_PCM179X_I2C) += snd-soc-pcm179x-i2c.o obj-$(CONFIG_SND_SOC_PCM179X_SPI) += snd-soc-pcm179x-spi.o obj-$(CONFIG_SND_SOC_PCM3008) += snd-soc-pcm3008.o obj-$(CONFIG_SND_SOC_PCM3168A) += snd-soc-pcm3168a.o diff --git a/sound/soc/codecs/pcm179x-i2c.c b/sound/soc/codecs/pcm179x-i2c.c new file mode 100644 index 000..4118106 --- /dev/null +++ b/sound/soc/codecs/pcm179x-i2c.c @@ -0,0 +1,73 @@ +/* + * PCM179X ASoC I2C driver + * + * Copyright (c) Teenage Engineering AB 2016 + * + * Jacob Siverskog + * + * 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. + */ + +#include +#include +#include +#include + +#include "pcm179x.h" + +static int pcm179x_i2c_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct regmap *regmap; + int ret; + + regmap = devm_regmap_init_i2c(client, _regmap_config); + if (IS_ERR(regmap)) { + ret = PTR_ERR(regmap); + dev_err(>dev, "Failed to allocate regmap: %d\n", ret); + return ret; + } + + return pcm179x_common_init(>dev, regmap); +} + +static int pcm179x_i2c_remove(struct i2c_client *client) +{ + return pcm179x_common_exit(>dev); +} + +static const struct of_device_id pcm179x_of_match[] = { + { .compatible = "ti,pcm1792a", }, + { } +};
[PATCH v4 3/3] ASoC: pcm179x: Support continuous rates
According to the PCM179x data sheets sampling frequencies between 10 kHz and 200 kHz are supported. Specify support in the driver. Tested with PCM1791A. References: http://www.ti.com/lit/ds/symlink/pcm1791a.pdf http://www.ti.com/lit/ds/symlink/pcm1792a.pdf http://www.ti.com/lit/ds/symlink/pcm1795.pdf http://www.ti.com/lit/ds/symlink/pcm1796.pdf Reviewed-by: Johan Hovold Signed-off-by: Jacob Siverskog --- sound/soc/codecs/pcm179x.c | 4 +++- sound/soc/codecs/pcm179x.h | 4 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/sound/soc/codecs/pcm179x.c b/sound/soc/codecs/pcm179x.c index 8f20f70..06a6657 100644 --- a/sound/soc/codecs/pcm179x.c +++ b/sound/soc/codecs/pcm179x.c @@ -187,7 +187,9 @@ static struct snd_soc_dai_driver pcm179x_dai = { .stream_name = "Playback", .channels_min = 2, .channels_max = 2, - .rates = PCM1792A_RATES, + .rates = SNDRV_PCM_RATE_CONTINUOUS, + .rate_min = 1, + .rate_max = 20, .formats = PCM1792A_FORMATS, }, .ops = _dai_ops, }; diff --git a/sound/soc/codecs/pcm179x.h b/sound/soc/codecs/pcm179x.h index c4eea4d..11e3312 100644 --- a/sound/soc/codecs/pcm179x.h +++ b/sound/soc/codecs/pcm179x.h @@ -17,10 +17,6 @@ #ifndef __PCM179X_H__ #define __PCM179X_H__ -#define PCM1792A_RATES (SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_8000_48000 | \ - SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 | \ - SNDRV_PCM_RATE_192000) - #define PCM1792A_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE | \ SNDRV_PCM_FMTBIT_S16_LE) -- 2.7.0
[PATCH v4 0/3] ASoC: pcm179x: Add I2C support, declare support for continuous rates
This series add support for I2C and declares support for continuous sample rates. Please note that this has only been tested using I2C and PCM1791A. I'd be grateful if someone can test it with other devices in the PCM179X family and/or SPI. These patches are against linux-next. [V4]: Move regmap error check to where the regmaps are allocated. Minor cosmetic changes. [V3]: Allocation of private data moved to common code. [V2]: Fix build issues found by kbuild test robot by adding static keyword and proper symbol exports. Jacob Siverskog (3): ASoC: pcm179x: Split into core and SPI parts ASoC: pcm179x: Add I2C interface driver ASoC: pcm179x: Support continuous rates .../devicetree/bindings/sound/pcm179x.txt | 11 +++- sound/soc/codecs/Kconfig | 20 +- sound/soc/codecs/Makefile | 4 ++ sound/soc/codecs/pcm179x-i2c.c | 73 ++ sound/soc/codecs/pcm179x-spi.c | 72 + sound/soc/codecs/pcm179x.c | 56 + sound/soc/codecs/pcm179x.h | 9 +-- 7 files changed, 196 insertions(+), 49 deletions(-) create mode 100644 sound/soc/codecs/pcm179x-i2c.c create mode 100644 sound/soc/codecs/pcm179x-spi.c -- 2.7.0
Re: [PATCH v3 1/3] ASoC: pcm179x: Split into core and SPI parts
On Thu, Jan 21, 2016 at 6:56 PM, Johan Hovold wrote: > On Thu, Jan 21, 2016 at 05:27:58PM +0100, Michael Trimarchi wrote: >> Hi >> >> On Thu, Jan 21, 2016 at 4:36 PM, Johan Hovold wrote: >> > On Thu, Jan 21, 2016 at 04:26:56PM +0100, Jacob Siverskog wrote: >> >> The pcm179x family supports both SPI and I2C for configuration. This >> >> patch splits the driver into core and SPI parts, in preparation for >> >> I2C support. >> >> >> >> Reviewed-by: Johan Hovold >> >> Signed-off-by: Jacob Siverskog >> >> --- >> > >> >> diff --git a/sound/soc/codecs/pcm179x-spi.c >> >> b/sound/soc/codecs/pcm179x-spi.c >> >> new file mode 100644 >> >> index 000..5842add9 >> >> --- /dev/null >> >> +++ b/sound/soc/codecs/pcm179x-spi.c > >> >> -static int pcm179x_spi_probe(struct spi_device *spi) >> >> +int pcm179x_common_init(struct device *dev, struct regmap *regmap) >> >> { >> >> struct pcm179x_private *pcm179x; >> >> int ret; >> >> >> >> - pcm179x = devm_kzalloc(>dev, sizeof(struct pcm179x_private), >> >> + if (IS_ERR(regmap)) { >> >> + ret = PTR_ERR(regmap); >> >> + dev_err(dev, "Failed to register regmap: %d\n", ret); >> >> + return ret; >> >> + } >> > >> > This looks weird. I think you should check for error where you do the >> > allocation even if this means a four more lines of code in total. >> > >> >> agree on that Ok. >> >> >> + >> >> + pcm179x = devm_kzalloc(dev, sizeof(struct pcm179x_private), >> >> GFP_KERNEL); >> >> if (!pcm179x) >> >> return -ENOMEM; >> >> >> >> - spi_set_drvdata(spi, pcm179x); >> >> - >> >> - pcm179x->regmap = devm_regmap_init_spi(spi, _regmap); >> >> - if (IS_ERR(pcm179x->regmap)) { >> >> - ret = PTR_ERR(pcm179x->regmap); >> >> - dev_err(>dev, "Failed to register regmap: %d\n", ret); >> >> - return ret; >> >> - } >> >> + pcm179x->regmap = regmap; >> >> + dev_set_drvdata(dev, pcm179x); >> >> >> >> snd_soc_codec_set_drvdata >> >> Is this more "codec" like? > > I'd say, only if you also add a codec probe callback and use it from > there. The other codec drivers appear to be consistent on that. I agree. Using snd_soc_codec_set_drvdata would mean larger logical changes that I don't really see the purpose of doing. All other codec drivers that are separated in a similar fashion as this patch use dev_set_drvdata.
Re: [PATCH v3 1/3] ASoC: pcm179x: Split into core and SPI parts
On Thu, Jan 21, 2016 at 6:56 PM, Johan Hovold <jo...@kernel.org> wrote: > On Thu, Jan 21, 2016 at 05:27:58PM +0100, Michael Trimarchi wrote: >> Hi >> >> On Thu, Jan 21, 2016 at 4:36 PM, Johan Hovold <jo...@kernel.org> wrote: >> > On Thu, Jan 21, 2016 at 04:26:56PM +0100, Jacob Siverskog wrote: >> >> The pcm179x family supports both SPI and I2C for configuration. This >> >> patch splits the driver into core and SPI parts, in preparation for >> >> I2C support. >> >> >> >> Reviewed-by: Johan Hovold <jo...@kernel.org> >> >> Signed-off-by: Jacob Siverskog <jacob@teenage.engineering> >> >> --- >> > >> >> diff --git a/sound/soc/codecs/pcm179x-spi.c >> >> b/sound/soc/codecs/pcm179x-spi.c >> >> new file mode 100644 >> >> index 000..5842add9 >> >> --- /dev/null >> >> +++ b/sound/soc/codecs/pcm179x-spi.c > >> >> -static int pcm179x_spi_probe(struct spi_device *spi) >> >> +int pcm179x_common_init(struct device *dev, struct regmap *regmap) >> >> { >> >> struct pcm179x_private *pcm179x; >> >> int ret; >> >> >> >> - pcm179x = devm_kzalloc(>dev, sizeof(struct pcm179x_private), >> >> + if (IS_ERR(regmap)) { >> >> + ret = PTR_ERR(regmap); >> >> + dev_err(dev, "Failed to register regmap: %d\n", ret); >> >> + return ret; >> >> + } >> > >> > This looks weird. I think you should check for error where you do the >> > allocation even if this means a four more lines of code in total. >> > >> >> agree on that Ok. >> >> >> + >> >> + pcm179x = devm_kzalloc(dev, sizeof(struct pcm179x_private), >> >> GFP_KERNEL); >> >> if (!pcm179x) >> >> return -ENOMEM; >> >> >> >> - spi_set_drvdata(spi, pcm179x); >> >> - >> >> - pcm179x->regmap = devm_regmap_init_spi(spi, _regmap); >> >> - if (IS_ERR(pcm179x->regmap)) { >> >> - ret = PTR_ERR(pcm179x->regmap); >> >> - dev_err(>dev, "Failed to register regmap: %d\n", ret); >> >> - return ret; >> >> - } >> >> + pcm179x->regmap = regmap; >> >> + dev_set_drvdata(dev, pcm179x); >> >> >> >> snd_soc_codec_set_drvdata >> >> Is this more "codec" like? > > I'd say, only if you also add a codec probe callback and use it from > there. The other codec drivers appear to be consistent on that. I agree. Using snd_soc_codec_set_drvdata would mean larger logical changes that I don't really see the purpose of doing. All other codec drivers that are separated in a similar fashion as this patch use dev_set_drvdata.
[PATCH v4 2/3] ASoC: pcm179x: Add I2C interface driver
The PCM179x family supports both SPI and I2C. This patch adds support for the I2C interface. Reviewed-by: Johan Hovold <jo...@kernel.org> Signed-off-by: Jacob Siverskog <jacob@teenage.engineering> --- .../devicetree/bindings/sound/pcm179x.txt | 11 +++- sound/soc/codecs/Kconfig | 9 +++ sound/soc/codecs/Makefile | 2 + sound/soc/codecs/pcm179x-i2c.c | 73 ++ 4 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 sound/soc/codecs/pcm179x-i2c.c diff --git a/Documentation/devicetree/bindings/sound/pcm179x.txt b/Documentation/devicetree/bindings/sound/pcm179x.txt index 4ae70d3..436c2b2 100644 --- a/Documentation/devicetree/bindings/sound/pcm179x.txt +++ b/Documentation/devicetree/bindings/sound/pcm179x.txt @@ -1,6 +1,6 @@ Texas Instruments pcm179x DT bindings -This driver supports the SPI bus. +This driver supports both the I2C and SPI bus. Required properties: @@ -9,6 +9,11 @@ Required properties: For required properties on SPI, please consult Documentation/devicetree/bindings/spi/spi-bus.txt +Required properties on I2C: + + - reg: the I2C address + + Examples: codec_spi: 1792a@0 { @@ -16,3 +21,7 @@ Examples: spi-max-frequency = <60>; }; + codec_i2c: 1792a@4c { + compatible = "ti,pcm1792a"; + reg = <0x4c>; + }; diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index ae720f1..3a8fcf1 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -87,6 +87,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_ML26124 if I2C select SND_SOC_NAU8825 if I2C select SND_SOC_PCM1681 if I2C + select SND_SOC_PCM179X_I2C if I2C select SND_SOC_PCM179X_SPI if SPI_MASTER select SND_SOC_PCM3008 select SND_SOC_PCM3168A_I2C if I2C @@ -529,6 +530,14 @@ config SND_SOC_PCM1681 config SND_SOC_PCM179X tristate +config SND_SOC_PCM179X_I2C + tristate "Texas Instruments PCM179X CODEC (I2C)" + depends on I2C + select SND_SOC_PCM179X + help + Enable support for Texas Instruments PCM179x CODEC. + Select this if your PCM179x is connected via an I2C bus. + config SND_SOC_PCM179X_SPI tristate "Texas Instruments PCM179X CODEC (SPI)" depends on SPI_MASTER diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index 56e94d8..9acd777 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -81,6 +81,7 @@ snd-soc-ml26124-objs := ml26124.o snd-soc-nau8825-objs := nau8825.o snd-soc-pcm1681-objs := pcm1681.o snd-soc-pcm179x-codec-objs := pcm179x.o +snd-soc-pcm179x-i2c-objs := pcm179x-i2c.o snd-soc-pcm179x-spi-objs := pcm179x-spi.o snd-soc-pcm3008-objs := pcm3008.o snd-soc-pcm3168a-objs := pcm3168a.o @@ -286,6 +287,7 @@ obj-$(CONFIG_SND_SOC_ML26124) += snd-soc-ml26124.o obj-$(CONFIG_SND_SOC_NAU8825) += snd-soc-nau8825.o obj-$(CONFIG_SND_SOC_PCM1681) += snd-soc-pcm1681.o obj-$(CONFIG_SND_SOC_PCM179X) += snd-soc-pcm179x-codec.o +obj-$(CONFIG_SND_SOC_PCM179X_I2C) += snd-soc-pcm179x-i2c.o obj-$(CONFIG_SND_SOC_PCM179X_SPI) += snd-soc-pcm179x-spi.o obj-$(CONFIG_SND_SOC_PCM3008) += snd-soc-pcm3008.o obj-$(CONFIG_SND_SOC_PCM3168A) += snd-soc-pcm3168a.o diff --git a/sound/soc/codecs/pcm179x-i2c.c b/sound/soc/codecs/pcm179x-i2c.c new file mode 100644 index 000..4118106 --- /dev/null +++ b/sound/soc/codecs/pcm179x-i2c.c @@ -0,0 +1,73 @@ +/* + * PCM179X ASoC I2C driver + * + * Copyright (c) Teenage Engineering AB 2016 + * + * Jacob Siverskog <jacob@teenage.engineering> + * + * 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. + */ + +#include +#include +#include +#include + +#include "pcm179x.h" + +static int pcm179x_i2c_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct regmap *regmap; + int ret; + + regmap = devm_regmap_init_i2c(client, _regmap_config); + if (IS_ERR(regmap)) { + ret = PTR_ERR(regmap); + dev_err(>dev, "Failed to allocate regmap: %d\n", ret); + return ret; + } + + return pcm179x_common_init(>dev, regmap); +} + +static int pcm179x_i2c_remove(struct i2c_client *client) +{ + return pcm179x_common_exit(>dev); +} + +sta
[PATCH v4 1/3] ASoC: pcm179x: Split into core and SPI parts
The pcm179x family supports both SPI and I2C for configuration. This patch splits the driver into core and SPI parts, in preparation for I2C support. Reviewed-by: Johan Hovold <jo...@kernel.org> Signed-off-by: Jacob Siverskog <jacob@teenage.engineering> --- sound/soc/codecs/Kconfig | 11 +-- sound/soc/codecs/Makefile | 2 ++ sound/soc/codecs/pcm179x-spi.c | 72 ++ sound/soc/codecs/pcm179x.c | 52 +++--- sound/soc/codecs/pcm179x.h | 5 +++ 5 files changed, 99 insertions(+), 43 deletions(-) create mode 100644 sound/soc/codecs/pcm179x-spi.c diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 50693c8..ae720f1 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -87,7 +87,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_ML26124 if I2C select SND_SOC_NAU8825 if I2C select SND_SOC_PCM1681 if I2C - select SND_SOC_PCM179X if SPI_MASTER + select SND_SOC_PCM179X_SPI if SPI_MASTER select SND_SOC_PCM3008 select SND_SOC_PCM3168A_I2C if I2C select SND_SOC_PCM3168A_SPI if SPI_MASTER @@ -527,8 +527,15 @@ config SND_SOC_PCM1681 depends on I2C config SND_SOC_PCM179X - tristate "Texas Instruments PCM179X CODEC" + tristate + +config SND_SOC_PCM179X_SPI + tristate "Texas Instruments PCM179X CODEC (SPI)" depends on SPI_MASTER + select SND_SOC_PCM179X + help + Enable support for Texas Instruments PCM179x CODEC. + Select this if your PCM179x is connected via an SPI bus. config SND_SOC_PCM3008 tristate diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index d44f7d3..56e94d8 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -81,6 +81,7 @@ snd-soc-ml26124-objs := ml26124.o snd-soc-nau8825-objs := nau8825.o snd-soc-pcm1681-objs := pcm1681.o snd-soc-pcm179x-codec-objs := pcm179x.o +snd-soc-pcm179x-spi-objs := pcm179x-spi.o snd-soc-pcm3008-objs := pcm3008.o snd-soc-pcm3168a-objs := pcm3168a.o snd-soc-pcm3168a-i2c-objs := pcm3168a-i2c.o @@ -285,6 +286,7 @@ obj-$(CONFIG_SND_SOC_ML26124) += snd-soc-ml26124.o obj-$(CONFIG_SND_SOC_NAU8825) += snd-soc-nau8825.o obj-$(CONFIG_SND_SOC_PCM1681) += snd-soc-pcm1681.o obj-$(CONFIG_SND_SOC_PCM179X) += snd-soc-pcm179x-codec.o +obj-$(CONFIG_SND_SOC_PCM179X_SPI) += snd-soc-pcm179x-spi.o obj-$(CONFIG_SND_SOC_PCM3008) += snd-soc-pcm3008.o obj-$(CONFIG_SND_SOC_PCM3168A) += snd-soc-pcm3168a.o obj-$(CONFIG_SND_SOC_PCM3168A_I2C) += snd-soc-pcm3168a-i2c.o diff --git a/sound/soc/codecs/pcm179x-spi.c b/sound/soc/codecs/pcm179x-spi.c new file mode 100644 index 000..da924d4 --- /dev/null +++ b/sound/soc/codecs/pcm179x-spi.c @@ -0,0 +1,72 @@ +/* + * PCM179X ASoC SPI driver + * + * Copyright (c) Amarula Solutions B.V. 2013 + * + * Michael Trimarchi <mich...@amarulasolutions.com> + * + * 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. + */ + +#include +#include +#include +#include + +#include "pcm179x.h" + +static int pcm179x_spi_probe(struct spi_device *spi) +{ + struct regmap *regmap; + int ret; + + regmap = devm_regmap_init_spi(spi, _regmap_config); + if (IS_ERR(regmap)) { + ret = PTR_ERR(regmap); + dev_err(>dev, "Failed to allocate regmap: %d\n", ret); + return ret; + } + + return pcm179x_common_init(>dev, regmap); +} + +static int pcm179x_spi_remove(struct spi_device *spi) +{ + return pcm179x_common_exit(>dev); +} + +static const struct of_device_id pcm179x_of_match[] = { + { .compatible = "ti,pcm1792a", }, + { } +}; +MODULE_DEVICE_TABLE(of, pcm179x_of_match); + +static const struct spi_device_id pcm179x_spi_ids[] = { + { "pcm179x", 0 }, + { }, +}; +MODULE_DEVICE_TABLE(spi, pcm179x_spi_ids); + +static struct spi_driver pcm179x_spi_driver = { + .driver = { + .name = "pcm179x", + .of_match_table = of_match_ptr(pcm179x_of_match), + }, + .id_table = pcm179x_spi_ids, + .probe = pcm179x_spi_probe, + .remove = pcm179x_spi_remove, +}; + +module_spi_driver(pcm179x_spi_driver); + +MODULE_DESCRIPTION("ASoC PCM179X SPI driver"); +MODULE_AUTHOR("Michael Trimarchi <mich...@amarulasolutions.com>"); +MODULE_LICENSE("GPL"); diff --git a/s
[PATCH v4 0/3] ASoC: pcm179x: Add I2C support, declare support for continuous rates
This series add support for I2C and declares support for continuous sample rates. Please note that this has only been tested using I2C and PCM1791A. I'd be grateful if someone can test it with other devices in the PCM179X family and/or SPI. These patches are against linux-next. [V4]: Move regmap error check to where the regmaps are allocated. Minor cosmetic changes. [V3]: Allocation of private data moved to common code. [V2]: Fix build issues found by kbuild test robot by adding static keyword and proper symbol exports. Jacob Siverskog (3): ASoC: pcm179x: Split into core and SPI parts ASoC: pcm179x: Add I2C interface driver ASoC: pcm179x: Support continuous rates .../devicetree/bindings/sound/pcm179x.txt | 11 +++- sound/soc/codecs/Kconfig | 20 +- sound/soc/codecs/Makefile | 4 ++ sound/soc/codecs/pcm179x-i2c.c | 73 ++ sound/soc/codecs/pcm179x-spi.c | 72 + sound/soc/codecs/pcm179x.c | 56 + sound/soc/codecs/pcm179x.h | 9 +-- 7 files changed, 196 insertions(+), 49 deletions(-) create mode 100644 sound/soc/codecs/pcm179x-i2c.c create mode 100644 sound/soc/codecs/pcm179x-spi.c -- 2.7.0
[PATCH v4 3/3] ASoC: pcm179x: Support continuous rates
According to the PCM179x data sheets sampling frequencies between 10 kHz and 200 kHz are supported. Specify support in the driver. Tested with PCM1791A. References: http://www.ti.com/lit/ds/symlink/pcm1791a.pdf http://www.ti.com/lit/ds/symlink/pcm1792a.pdf http://www.ti.com/lit/ds/symlink/pcm1795.pdf http://www.ti.com/lit/ds/symlink/pcm1796.pdf Reviewed-by: Johan Hovold <jo...@kernel.org> Signed-off-by: Jacob Siverskog <jacob@teenage.engineering> --- sound/soc/codecs/pcm179x.c | 4 +++- sound/soc/codecs/pcm179x.h | 4 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/sound/soc/codecs/pcm179x.c b/sound/soc/codecs/pcm179x.c index 8f20f70..06a6657 100644 --- a/sound/soc/codecs/pcm179x.c +++ b/sound/soc/codecs/pcm179x.c @@ -187,7 +187,9 @@ static struct snd_soc_dai_driver pcm179x_dai = { .stream_name = "Playback", .channels_min = 2, .channels_max = 2, - .rates = PCM1792A_RATES, + .rates = SNDRV_PCM_RATE_CONTINUOUS, + .rate_min = 1, + .rate_max = 20, .formats = PCM1792A_FORMATS, }, .ops = _dai_ops, }; diff --git a/sound/soc/codecs/pcm179x.h b/sound/soc/codecs/pcm179x.h index c4eea4d..11e3312 100644 --- a/sound/soc/codecs/pcm179x.h +++ b/sound/soc/codecs/pcm179x.h @@ -17,10 +17,6 @@ #ifndef __PCM179X_H__ #define __PCM179X_H__ -#define PCM1792A_RATES (SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_8000_48000 | \ - SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 | \ - SNDRV_PCM_RATE_192000) - #define PCM1792A_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE | \ SNDRV_PCM_FMTBIT_S16_LE) -- 2.7.0
[PATCH v3 3/3] ASoC: pcm179x: Support continuous rates
According to the PCM179x data sheets sampling frequencies between 10 kHz and 200 kHz are supported. Specify support in the driver. Tested with PCM1791A. References: http://www.ti.com/lit/ds/symlink/pcm1791a.pdf http://www.ti.com/lit/ds/symlink/pcm1792a.pdf http://www.ti.com/lit/ds/symlink/pcm1795.pdf http://www.ti.com/lit/ds/symlink/pcm1796.pdf Reviewed-by: Johan Hovold Signed-off-by: Jacob Siverskog --- sound/soc/codecs/pcm179x.c | 4 +++- sound/soc/codecs/pcm179x.h | 4 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/sound/soc/codecs/pcm179x.c b/sound/soc/codecs/pcm179x.c index eba451f..d492b5a 100644 --- a/sound/soc/codecs/pcm179x.c +++ b/sound/soc/codecs/pcm179x.c @@ -187,7 +187,9 @@ static struct snd_soc_dai_driver pcm179x_dai = { .stream_name = "Playback", .channels_min = 2, .channels_max = 2, - .rates = PCM1792A_RATES, + .rates = SNDRV_PCM_RATE_CONTINUOUS, + .rate_min = 1, + .rate_max = 20, .formats = PCM1792A_FORMATS, }, .ops = _dai_ops, }; diff --git a/sound/soc/codecs/pcm179x.h b/sound/soc/codecs/pcm179x.h index c4eea4d..11e3312 100644 --- a/sound/soc/codecs/pcm179x.h +++ b/sound/soc/codecs/pcm179x.h @@ -17,10 +17,6 @@ #ifndef __PCM179X_H__ #define __PCM179X_H__ -#define PCM1792A_RATES (SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_8000_48000 | \ - SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 | \ - SNDRV_PCM_RATE_192000) - #define PCM1792A_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE | \ SNDRV_PCM_FMTBIT_S16_LE) -- 2.7.0
[PATCH v3 2/3] ASoC: pcm179x: Add I2C interface driver
The PCM179x family supports both SPI and I2C. This patch adds support for the I2C interface. Reviewed-by: Johan Hovold Signed-off-by: Jacob Siverskog --- .../devicetree/bindings/sound/pcm179x.txt | 11 +++- sound/soc/codecs/Kconfig | 9 +++ sound/soc/codecs/Makefile | 2 + sound/soc/codecs/pcm179x-i2c.c | 64 ++ 4 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 sound/soc/codecs/pcm179x-i2c.c diff --git a/Documentation/devicetree/bindings/sound/pcm179x.txt b/Documentation/devicetree/bindings/sound/pcm179x.txt index 4ae70d3..436c2b2 100644 --- a/Documentation/devicetree/bindings/sound/pcm179x.txt +++ b/Documentation/devicetree/bindings/sound/pcm179x.txt @@ -1,6 +1,6 @@ Texas Instruments pcm179x DT bindings -This driver supports the SPI bus. +This driver supports both the I2C and SPI bus. Required properties: @@ -9,6 +9,11 @@ Required properties: For required properties on SPI, please consult Documentation/devicetree/bindings/spi/spi-bus.txt +Required properties on I2C: + + - reg: the I2C address + + Examples: codec_spi: 1792a@0 { @@ -16,3 +21,7 @@ Examples: spi-max-frequency = <60>; }; + codec_i2c: 1792a@4c { + compatible = "ti,pcm1792a"; + reg = <0x4c>; + }; diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 3866717..a27da1b 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -83,6 +83,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_ML26124 if I2C select SND_SOC_NAU8825 if I2C select SND_SOC_PCM1681 if I2C + select SND_SOC_PCM179X_I2C if I2C select SND_SOC_PCM179X_SPI if SPI_MASTER select SND_SOC_PCM3008 select SND_SOC_PCM512x_I2C if I2C @@ -502,6 +503,14 @@ config SND_SOC_PCM1681 config SND_SOC_PCM179X tristate +config SND_SOC_PCM179X_I2C + tristate "Texas Instruments PCM179X CODEC (I2C)" + depends on I2C + select SND_SOC_PCM179X + help + Enable support for Texas Instruments PCM179x CODEC. + Select this if your PCM179x is connected via an I2C bus. + config SND_SOC_PCM179X_SPI tristate "Texas Instruments PCM179X CODEC (SPI)" depends on SPI_MASTER diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index 0fa454c..972395e 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -77,6 +77,7 @@ snd-soc-ml26124-objs := ml26124.o snd-soc-nau8825-objs := nau8825.o snd-soc-pcm1681-objs := pcm1681.o snd-soc-pcm179x-codec-objs := pcm179x.o +snd-soc-pcm179x-i2c-objs := pcm179x-i2c.o snd-soc-pcm179x-spi-objs := pcm179x-spi.o snd-soc-pcm3008-objs := pcm3008.o snd-soc-pcm512x-objs := pcm512x.o @@ -276,6 +277,7 @@ obj-$(CONFIG_SND_SOC_ML26124) += snd-soc-ml26124.o obj-$(CONFIG_SND_SOC_NAU8825) += snd-soc-nau8825.o obj-$(CONFIG_SND_SOC_PCM1681) += snd-soc-pcm1681.o obj-$(CONFIG_SND_SOC_PCM179X) += snd-soc-pcm179x-codec.o +obj-$(CONFIG_SND_SOC_PCM179X_I2C) += snd-soc-pcm179x-i2c.o obj-$(CONFIG_SND_SOC_PCM179X_SPI) += snd-soc-pcm179x-spi.o obj-$(CONFIG_SND_SOC_PCM3008) += snd-soc-pcm3008.o obj-$(CONFIG_SND_SOC_PCM512x) += snd-soc-pcm512x.o diff --git a/sound/soc/codecs/pcm179x-i2c.c b/sound/soc/codecs/pcm179x-i2c.c new file mode 100644 index 000..ee73576 --- /dev/null +++ b/sound/soc/codecs/pcm179x-i2c.c @@ -0,0 +1,64 @@ +/* + * PCM179X ASoC I2C driver + * + * Copyright (c) Teenage Engineering AB 2016 + * + * Jacob Siverskog + * + * 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. + */ + +#include +#include +#include +#include + +#include "pcm179x.h" + +static int pcm179x_i2c_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + return pcm179x_common_init(>dev, + devm_regmap_init_i2c(client, _regmap_config)); +} + +static int pcm179x_i2c_remove(struct i2c_client *client) +{ + return pcm179x_common_exit(>dev); +} + +static const struct of_device_id pcm179x_of_match[] = { + { .compatible = "ti,pcm1792a", }, + { } +}; +MODULE_DEVICE_TABLE(of, pcm179x_of_match); + +static const struct i2c_device_id pcm179x_i2c_ids[] = { + { "pcm179x", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, pcm179x_i2c_ids); + +static struct i2c_driver
[PATCH v3 1/3] ASoC: pcm179x: Split into core and SPI parts
The pcm179x family supports both SPI and I2C for configuration. This patch splits the driver into core and SPI parts, in preparation for I2C support. Reviewed-by: Johan Hovold Signed-off-by: Jacob Siverskog --- sound/soc/codecs/Kconfig | 11 ++-- sound/soc/codecs/Makefile | 2 ++ sound/soc/codecs/pcm179x-spi.c | 63 ++ sound/soc/codecs/pcm179x.c | 57 -- sound/soc/codecs/pcm179x.h | 5 5 files changed, 96 insertions(+), 42 deletions(-) create mode 100644 sound/soc/codecs/pcm179x-spi.c diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 0f279c2..3866717 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -83,7 +83,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_ML26124 if I2C select SND_SOC_NAU8825 if I2C select SND_SOC_PCM1681 if I2C - select SND_SOC_PCM179X if SPI_MASTER + select SND_SOC_PCM179X_SPI if SPI_MASTER select SND_SOC_PCM3008 select SND_SOC_PCM512x_I2C if I2C select SND_SOC_PCM512x_SPI if SPI_MASTER @@ -500,8 +500,15 @@ config SND_SOC_PCM1681 depends on I2C config SND_SOC_PCM179X - tristate "Texas Instruments PCM179X CODEC" + tristate + +config SND_SOC_PCM179X_SPI + tristate "Texas Instruments PCM179X CODEC (SPI)" depends on SPI_MASTER + select SND_SOC_PCM179X + help + Enable support for Texas Instruments PCM179x CODEC. + Select this if your PCM179x is connected via an SPI bus. config SND_SOC_PCM3008 tristate diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index c2d2ced..0fa454c 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -77,6 +77,7 @@ snd-soc-ml26124-objs := ml26124.o snd-soc-nau8825-objs := nau8825.o snd-soc-pcm1681-objs := pcm1681.o snd-soc-pcm179x-codec-objs := pcm179x.o +snd-soc-pcm179x-spi-objs := pcm179x-spi.o snd-soc-pcm3008-objs := pcm3008.o snd-soc-pcm512x-objs := pcm512x.o snd-soc-pcm512x-i2c-objs := pcm512x-i2c.o @@ -275,6 +276,7 @@ obj-$(CONFIG_SND_SOC_ML26124) += snd-soc-ml26124.o obj-$(CONFIG_SND_SOC_NAU8825) += snd-soc-nau8825.o obj-$(CONFIG_SND_SOC_PCM1681) += snd-soc-pcm1681.o obj-$(CONFIG_SND_SOC_PCM179X) += snd-soc-pcm179x-codec.o +obj-$(CONFIG_SND_SOC_PCM179X_SPI) += snd-soc-pcm179x-spi.o obj-$(CONFIG_SND_SOC_PCM3008) += snd-soc-pcm3008.o obj-$(CONFIG_SND_SOC_PCM512x) += snd-soc-pcm512x.o obj-$(CONFIG_SND_SOC_PCM512x_I2C) += snd-soc-pcm512x-i2c.o diff --git a/sound/soc/codecs/pcm179x-spi.c b/sound/soc/codecs/pcm179x-spi.c new file mode 100644 index 000..5842add9 --- /dev/null +++ b/sound/soc/codecs/pcm179x-spi.c @@ -0,0 +1,63 @@ +/* + * PCM179X ASoC SPI driver + * + * Copyright (c) Amarula Solutions B.V. 2013 + * + * Michael Trimarchi + * + * 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. + */ + +#include +#include +#include +#include + +#include "pcm179x.h" + +static int pcm179x_spi_probe(struct spi_device *spi) +{ + return pcm179x_common_init(>dev, + devm_regmap_init_spi(spi, _regmap_config)); +} + +static int pcm179x_spi_remove(struct spi_device *spi) +{ + return pcm179x_common_exit(>dev); +} + +static const struct of_device_id pcm179x_of_match[] = { + { .compatible = "ti,pcm1792a", }, + { } +}; +MODULE_DEVICE_TABLE(of, pcm179x_of_match); + +static const struct spi_device_id pcm179x_spi_ids[] = { + { "pcm179x", 0 }, + { }, +}; +MODULE_DEVICE_TABLE(spi, pcm179x_spi_ids); + +static struct spi_driver pcm179x_spi_driver = { + .driver = { + .name = "pcm179x", + .of_match_table = of_match_ptr(pcm179x_of_match), + }, + .id_table = pcm179x_spi_ids, + .probe = pcm179x_spi_probe, + .remove = pcm179x_spi_remove, +}; + +module_spi_driver(pcm179x_spi_driver); + +MODULE_DESCRIPTION("ASoC PCM179X SPI driver"); +MODULE_AUTHOR("Michael Trimarchi "); +MODULE_LICENSE("GPL"); diff --git a/sound/soc/codecs/pcm179x.c b/sound/soc/codecs/pcm179x.c index a56c7b7..eba451f 100644 --- a/sound/soc/codecs/pcm179x.c +++ b/sound/soc/codecs/pcm179x.c @@ -20,7 +20,6 @@ #include #include #include -#include #include #include @@ -29,7 +28,6 @@ #include #include #include -#include #include "pcm179x.h" @@ -194,13 +192,7 @@ static
[PATCH v3 0/3] ASoC: pcm179x: Add I2C support, declare support for continuous rates
This series add support for I2C and declares support for continuous sample rates. Please note that this has only been tested using I2C and PCM1791A. I'd be grateful if someone can test it with other devices in the PCM179X family and/or SPI. These patches are against linux-next. [V3]: Allocation of private data moved to common code. [V2]: Fix build issues found by kbuild test robot by adding static keyword and proper symbol exports. Jacob Siverskog (3): ASoC: pcm179x: Split into core and SPI parts ASoC: pcm179x: Add I2C interface driver ASoC: pcm179x: Support continuous rates .../devicetree/bindings/sound/pcm179x.txt | 11 +++- sound/soc/codecs/Kconfig | 20 ++- sound/soc/codecs/Makefile | 4 ++ sound/soc/codecs/pcm179x-i2c.c | 64 ++ sound/soc/codecs/pcm179x-spi.c | 63 + sound/soc/codecs/pcm179x.c | 61 +++-- sound/soc/codecs/pcm179x.h | 9 +-- 7 files changed, 184 insertions(+), 48 deletions(-) create mode 100644 sound/soc/codecs/pcm179x-i2c.c create mode 100644 sound/soc/codecs/pcm179x-spi.c -- 2.7.0
[PATCH v3 1/3] ASoC: pcm179x: Split into core and SPI parts
The pcm179x family supports both SPI and I2C for configuration. This patch splits the driver into core and SPI parts, in preparation for I2C support. Reviewed-by: Johan Hovold <jo...@kernel.org> Signed-off-by: Jacob Siverskog <jacob@teenage.engineering> --- sound/soc/codecs/Kconfig | 11 ++-- sound/soc/codecs/Makefile | 2 ++ sound/soc/codecs/pcm179x-spi.c | 63 ++ sound/soc/codecs/pcm179x.c | 57 -- sound/soc/codecs/pcm179x.h | 5 5 files changed, 96 insertions(+), 42 deletions(-) create mode 100644 sound/soc/codecs/pcm179x-spi.c diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 0f279c2..3866717 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -83,7 +83,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_ML26124 if I2C select SND_SOC_NAU8825 if I2C select SND_SOC_PCM1681 if I2C - select SND_SOC_PCM179X if SPI_MASTER + select SND_SOC_PCM179X_SPI if SPI_MASTER select SND_SOC_PCM3008 select SND_SOC_PCM512x_I2C if I2C select SND_SOC_PCM512x_SPI if SPI_MASTER @@ -500,8 +500,15 @@ config SND_SOC_PCM1681 depends on I2C config SND_SOC_PCM179X - tristate "Texas Instruments PCM179X CODEC" + tristate + +config SND_SOC_PCM179X_SPI + tristate "Texas Instruments PCM179X CODEC (SPI)" depends on SPI_MASTER + select SND_SOC_PCM179X + help + Enable support for Texas Instruments PCM179x CODEC. + Select this if your PCM179x is connected via an SPI bus. config SND_SOC_PCM3008 tristate diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index c2d2ced..0fa454c 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -77,6 +77,7 @@ snd-soc-ml26124-objs := ml26124.o snd-soc-nau8825-objs := nau8825.o snd-soc-pcm1681-objs := pcm1681.o snd-soc-pcm179x-codec-objs := pcm179x.o +snd-soc-pcm179x-spi-objs := pcm179x-spi.o snd-soc-pcm3008-objs := pcm3008.o snd-soc-pcm512x-objs := pcm512x.o snd-soc-pcm512x-i2c-objs := pcm512x-i2c.o @@ -275,6 +276,7 @@ obj-$(CONFIG_SND_SOC_ML26124) += snd-soc-ml26124.o obj-$(CONFIG_SND_SOC_NAU8825) += snd-soc-nau8825.o obj-$(CONFIG_SND_SOC_PCM1681) += snd-soc-pcm1681.o obj-$(CONFIG_SND_SOC_PCM179X) += snd-soc-pcm179x-codec.o +obj-$(CONFIG_SND_SOC_PCM179X_SPI) += snd-soc-pcm179x-spi.o obj-$(CONFIG_SND_SOC_PCM3008) += snd-soc-pcm3008.o obj-$(CONFIG_SND_SOC_PCM512x) += snd-soc-pcm512x.o obj-$(CONFIG_SND_SOC_PCM512x_I2C) += snd-soc-pcm512x-i2c.o diff --git a/sound/soc/codecs/pcm179x-spi.c b/sound/soc/codecs/pcm179x-spi.c new file mode 100644 index 000..5842add9 --- /dev/null +++ b/sound/soc/codecs/pcm179x-spi.c @@ -0,0 +1,63 @@ +/* + * PCM179X ASoC SPI driver + * + * Copyright (c) Amarula Solutions B.V. 2013 + * + * Michael Trimarchi <mich...@amarulasolutions.com> + * + * 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. + */ + +#include +#include +#include +#include + +#include "pcm179x.h" + +static int pcm179x_spi_probe(struct spi_device *spi) +{ + return pcm179x_common_init(>dev, + devm_regmap_init_spi(spi, _regmap_config)); +} + +static int pcm179x_spi_remove(struct spi_device *spi) +{ + return pcm179x_common_exit(>dev); +} + +static const struct of_device_id pcm179x_of_match[] = { + { .compatible = "ti,pcm1792a", }, + { } +}; +MODULE_DEVICE_TABLE(of, pcm179x_of_match); + +static const struct spi_device_id pcm179x_spi_ids[] = { + { "pcm179x", 0 }, + { }, +}; +MODULE_DEVICE_TABLE(spi, pcm179x_spi_ids); + +static struct spi_driver pcm179x_spi_driver = { + .driver = { + .name = "pcm179x", + .of_match_table = of_match_ptr(pcm179x_of_match), + }, + .id_table = pcm179x_spi_ids, + .probe = pcm179x_spi_probe, + .remove = pcm179x_spi_remove, +}; + +module_spi_driver(pcm179x_spi_driver); + +MODULE_DESCRIPTION("ASoC PCM179X SPI driver"); +MODULE_AUTHOR("Michael Trimarchi <mich...@amarulasolutions.com>"); +MODULE_LICENSE("GPL"); diff --git a/sound/soc/codecs/pcm179x.c b/sound/soc/codecs/pcm179x.c index a56c7b7..eba451f 100644 --- a/sound/soc/codecs/pcm179x.c +++ b/sound/soc/codecs/pcm179x.c @@ -20,7 +20,6 @@ #include #include #include -#include #include #include @@
[PATCH v3 0/3] ASoC: pcm179x: Add I2C support, declare support for continuous rates
This series add support for I2C and declares support for continuous sample rates. Please note that this has only been tested using I2C and PCM1791A. I'd be grateful if someone can test it with other devices in the PCM179X family and/or SPI. These patches are against linux-next. [V3]: Allocation of private data moved to common code. [V2]: Fix build issues found by kbuild test robot by adding static keyword and proper symbol exports. Jacob Siverskog (3): ASoC: pcm179x: Split into core and SPI parts ASoC: pcm179x: Add I2C interface driver ASoC: pcm179x: Support continuous rates .../devicetree/bindings/sound/pcm179x.txt | 11 +++- sound/soc/codecs/Kconfig | 20 ++- sound/soc/codecs/Makefile | 4 ++ sound/soc/codecs/pcm179x-i2c.c | 64 ++ sound/soc/codecs/pcm179x-spi.c | 63 + sound/soc/codecs/pcm179x.c | 61 +++-- sound/soc/codecs/pcm179x.h | 9 +-- 7 files changed, 184 insertions(+), 48 deletions(-) create mode 100644 sound/soc/codecs/pcm179x-i2c.c create mode 100644 sound/soc/codecs/pcm179x-spi.c -- 2.7.0
[PATCH v3 2/3] ASoC: pcm179x: Add I2C interface driver
The PCM179x family supports both SPI and I2C. This patch adds support for the I2C interface. Reviewed-by: Johan Hovold <jo...@kernel.org> Signed-off-by: Jacob Siverskog <jacob@teenage.engineering> --- .../devicetree/bindings/sound/pcm179x.txt | 11 +++- sound/soc/codecs/Kconfig | 9 +++ sound/soc/codecs/Makefile | 2 + sound/soc/codecs/pcm179x-i2c.c | 64 ++ 4 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 sound/soc/codecs/pcm179x-i2c.c diff --git a/Documentation/devicetree/bindings/sound/pcm179x.txt b/Documentation/devicetree/bindings/sound/pcm179x.txt index 4ae70d3..436c2b2 100644 --- a/Documentation/devicetree/bindings/sound/pcm179x.txt +++ b/Documentation/devicetree/bindings/sound/pcm179x.txt @@ -1,6 +1,6 @@ Texas Instruments pcm179x DT bindings -This driver supports the SPI bus. +This driver supports both the I2C and SPI bus. Required properties: @@ -9,6 +9,11 @@ Required properties: For required properties on SPI, please consult Documentation/devicetree/bindings/spi/spi-bus.txt +Required properties on I2C: + + - reg: the I2C address + + Examples: codec_spi: 1792a@0 { @@ -16,3 +21,7 @@ Examples: spi-max-frequency = <60>; }; + codec_i2c: 1792a@4c { + compatible = "ti,pcm1792a"; + reg = <0x4c>; + }; diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 3866717..a27da1b 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -83,6 +83,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_ML26124 if I2C select SND_SOC_NAU8825 if I2C select SND_SOC_PCM1681 if I2C + select SND_SOC_PCM179X_I2C if I2C select SND_SOC_PCM179X_SPI if SPI_MASTER select SND_SOC_PCM3008 select SND_SOC_PCM512x_I2C if I2C @@ -502,6 +503,14 @@ config SND_SOC_PCM1681 config SND_SOC_PCM179X tristate +config SND_SOC_PCM179X_I2C + tristate "Texas Instruments PCM179X CODEC (I2C)" + depends on I2C + select SND_SOC_PCM179X + help + Enable support for Texas Instruments PCM179x CODEC. + Select this if your PCM179x is connected via an I2C bus. + config SND_SOC_PCM179X_SPI tristate "Texas Instruments PCM179X CODEC (SPI)" depends on SPI_MASTER diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index 0fa454c..972395e 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -77,6 +77,7 @@ snd-soc-ml26124-objs := ml26124.o snd-soc-nau8825-objs := nau8825.o snd-soc-pcm1681-objs := pcm1681.o snd-soc-pcm179x-codec-objs := pcm179x.o +snd-soc-pcm179x-i2c-objs := pcm179x-i2c.o snd-soc-pcm179x-spi-objs := pcm179x-spi.o snd-soc-pcm3008-objs := pcm3008.o snd-soc-pcm512x-objs := pcm512x.o @@ -276,6 +277,7 @@ obj-$(CONFIG_SND_SOC_ML26124) += snd-soc-ml26124.o obj-$(CONFIG_SND_SOC_NAU8825) += snd-soc-nau8825.o obj-$(CONFIG_SND_SOC_PCM1681) += snd-soc-pcm1681.o obj-$(CONFIG_SND_SOC_PCM179X) += snd-soc-pcm179x-codec.o +obj-$(CONFIG_SND_SOC_PCM179X_I2C) += snd-soc-pcm179x-i2c.o obj-$(CONFIG_SND_SOC_PCM179X_SPI) += snd-soc-pcm179x-spi.o obj-$(CONFIG_SND_SOC_PCM3008) += snd-soc-pcm3008.o obj-$(CONFIG_SND_SOC_PCM512x) += snd-soc-pcm512x.o diff --git a/sound/soc/codecs/pcm179x-i2c.c b/sound/soc/codecs/pcm179x-i2c.c new file mode 100644 index 000..ee73576 --- /dev/null +++ b/sound/soc/codecs/pcm179x-i2c.c @@ -0,0 +1,64 @@ +/* + * PCM179X ASoC I2C driver + * + * Copyright (c) Teenage Engineering AB 2016 + * + * Jacob Siverskog <jacob@teenage.engineering> + * + * 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. + */ + +#include +#include +#include +#include + +#include "pcm179x.h" + +static int pcm179x_i2c_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + return pcm179x_common_init(>dev, + devm_regmap_init_i2c(client, _regmap_config)); +} + +static int pcm179x_i2c_remove(struct i2c_client *client) +{ + return pcm179x_common_exit(>dev); +} + +static const struct of_device_id pcm179x_of_match[] = { + { .compatible = "ti,pcm1792a", }, + { } +}; +MODULE_DEVICE_TABLE(of, pcm179x_of_match); + +static const struct i2c_device_id pcm179x_i2c_ids[] = { + { "pcm179x", 0 }, + {
[PATCH v3 3/3] ASoC: pcm179x: Support continuous rates
According to the PCM179x data sheets sampling frequencies between 10 kHz and 200 kHz are supported. Specify support in the driver. Tested with PCM1791A. References: http://www.ti.com/lit/ds/symlink/pcm1791a.pdf http://www.ti.com/lit/ds/symlink/pcm1792a.pdf http://www.ti.com/lit/ds/symlink/pcm1795.pdf http://www.ti.com/lit/ds/symlink/pcm1796.pdf Reviewed-by: Johan Hovold <jo...@kernel.org> Signed-off-by: Jacob Siverskog <jacob@teenage.engineering> --- sound/soc/codecs/pcm179x.c | 4 +++- sound/soc/codecs/pcm179x.h | 4 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/sound/soc/codecs/pcm179x.c b/sound/soc/codecs/pcm179x.c index eba451f..d492b5a 100644 --- a/sound/soc/codecs/pcm179x.c +++ b/sound/soc/codecs/pcm179x.c @@ -187,7 +187,9 @@ static struct snd_soc_dai_driver pcm179x_dai = { .stream_name = "Playback", .channels_min = 2, .channels_max = 2, - .rates = PCM1792A_RATES, + .rates = SNDRV_PCM_RATE_CONTINUOUS, + .rate_min = 1, + .rate_max = 20, .formats = PCM1792A_FORMATS, }, .ops = _dai_ops, }; diff --git a/sound/soc/codecs/pcm179x.h b/sound/soc/codecs/pcm179x.h index c4eea4d..11e3312 100644 --- a/sound/soc/codecs/pcm179x.h +++ b/sound/soc/codecs/pcm179x.h @@ -17,10 +17,6 @@ #ifndef __PCM179X_H__ #define __PCM179X_H__ -#define PCM1792A_RATES (SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_8000_48000 | \ - SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 | \ - SNDRV_PCM_RATE_192000) - #define PCM1792A_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE | \ SNDRV_PCM_FMTBIT_S16_LE) -- 2.7.0
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram
On Wed, Jan 20, 2016 at 4:48 PM, Eric Dumazet wrote: > On Wed, 2016-01-20 at 16:06 +0100, Jacob Siverskog wrote: >> On Tue, Jan 5, 2016 at 3:39 PM, Eric Dumazet wrote: >> > On Tue, 2016-01-05 at 15:34 +0100, Jacob Siverskog wrote: >> >> On Tue, Jan 5, 2016 at 3:14 PM, Eric Dumazet >> >> wrote: >> > >> >> > >> >> > You might build a kernel with KASAN support to get maybe more chances to >> >> > trigger the bug. >> >> > >> >> > ( https://www.kernel.org/doc/Documentation/kasan.txt ) >> >> > >> >> >> >> Ah. Doesn't seem to be supported on arm(32) unfortunately. >> > >> > Then you could at least use standard debugging features : >> > >> > CONFIG_SLAB=y >> > CONFIG_SLABINFO=y >> > CONFIG_DEBUG_SLAB=y >> > CONFIG_DEBUG_SLAB_LEAK=y >> > >> > (Or equivalent SLUB options) >> > >> > and >> > >> > CONFIG_DEBUG_PAGEALLOC=y >> > >> > (If arm(32) has CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y) >> >> I tried with those enabled and while toggling power on the Bluetooth >> interface I usually get this after a few iterations: >> kernel: Bluetooth: Unable to push skb to HCI core(-6) > > Well, this code seems to be quite buggy. > > I do not have time to audit it, but 5 minutes are enough to spot 2 > issues. > > skb, once given to another queue/layer should not be accessed anymore. > Ok. Unfortunately I still see the slab corruption even with your changes.
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram
On Tue, Jan 5, 2016 at 3:39 PM, Eric Dumazet wrote: > On Tue, 2016-01-05 at 15:34 +0100, Jacob Siverskog wrote: >> On Tue, Jan 5, 2016 at 3:14 PM, Eric Dumazet wrote: > >> > >> > You might build a kernel with KASAN support to get maybe more chances to >> > trigger the bug. >> > >> > ( https://www.kernel.org/doc/Documentation/kasan.txt ) >> > >> >> Ah. Doesn't seem to be supported on arm(32) unfortunately. > > Then you could at least use standard debugging features : > > CONFIG_SLAB=y > CONFIG_SLABINFO=y > CONFIG_DEBUG_SLAB=y > CONFIG_DEBUG_SLAB_LEAK=y > > (Or equivalent SLUB options) > > and > > CONFIG_DEBUG_PAGEALLOC=y > > (If arm(32) has CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y) I tried with those enabled and while toggling power on the Bluetooth interface I usually get this after a few iterations: kernel: Bluetooth: Unable to push skb to HCI core(-6) kernel: (stc): proto stack 4's ->recv failed kernel: Slab corruption (Not tainted): skbuff_head_cache start=c08a8a00, len=176 kernel: 0a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6a 6b 6b a5 jkk. kernel: Prev obj: start=c08a8940, len=176 kernel: 000: 00 00 00 00 00 00 00 00 31 73 52 00 43 17 2b 14 1sR.C.+. kernel: 010: 00 00 00 00 00 00 00 00 04 00 00 00 01 00 00 00 kernel: Next obj: start=c08a8ac0, len=176 kernel: 000: 00 00 00 00 00 00 00 00 01 42 f6 50 36 17 2b 14 .B.P6.+. kernel: 010: 00 00 00 00 00 00 00 00 04 00 00 00 01 00 00 00 The "Unable to push skb" and "recv failed" lines always appear before the corruption. Unfortunately, the corruptions occur also with your patch.
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram
On Tue, Jan 5, 2016 at 3:39 PM, Eric Dumazet <eric.duma...@gmail.com> wrote: > On Tue, 2016-01-05 at 15:34 +0100, Jacob Siverskog wrote: >> On Tue, Jan 5, 2016 at 3:14 PM, Eric Dumazet <eric.duma...@gmail.com> wrote: > >> > >> > You might build a kernel with KASAN support to get maybe more chances to >> > trigger the bug. >> > >> > ( https://www.kernel.org/doc/Documentation/kasan.txt ) >> > >> >> Ah. Doesn't seem to be supported on arm(32) unfortunately. > > Then you could at least use standard debugging features : > > CONFIG_SLAB=y > CONFIG_SLABINFO=y > CONFIG_DEBUG_SLAB=y > CONFIG_DEBUG_SLAB_LEAK=y > > (Or equivalent SLUB options) > > and > > CONFIG_DEBUG_PAGEALLOC=y > > (If arm(32) has CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y) I tried with those enabled and while toggling power on the Bluetooth interface I usually get this after a few iterations: kernel: Bluetooth: Unable to push skb to HCI core(-6) kernel: (stc): proto stack 4's ->recv failed kernel: Slab corruption (Not tainted): skbuff_head_cache start=c08a8a00, len=176 kernel: 0a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6a 6b 6b a5 jkk. kernel: Prev obj: start=c08a8940, len=176 kernel: 000: 00 00 00 00 00 00 00 00 31 73 52 00 43 17 2b 14 1sR.C.+. kernel: 010: 00 00 00 00 00 00 00 00 04 00 00 00 01 00 00 00 kernel: Next obj: start=c08a8ac0, len=176 kernel: 000: 00 00 00 00 00 00 00 00 01 42 f6 50 36 17 2b 14 .B.P6.+. kernel: 010: 00 00 00 00 00 00 00 00 04 00 00 00 01 00 00 00 The "Unable to push skb" and "recv failed" lines always appear before the corruption. Unfortunately, the corruptions occur also with your patch.
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram
On Wed, Jan 20, 2016 at 4:48 PM, Eric Dumazet <eric.duma...@gmail.com> wrote: > On Wed, 2016-01-20 at 16:06 +0100, Jacob Siverskog wrote: >> On Tue, Jan 5, 2016 at 3:39 PM, Eric Dumazet <eric.duma...@gmail.com> wrote: >> > On Tue, 2016-01-05 at 15:34 +0100, Jacob Siverskog wrote: >> >> On Tue, Jan 5, 2016 at 3:14 PM, Eric Dumazet <eric.duma...@gmail.com> >> >> wrote: >> > >> >> > >> >> > You might build a kernel with KASAN support to get maybe more chances to >> >> > trigger the bug. >> >> > >> >> > ( https://www.kernel.org/doc/Documentation/kasan.txt ) >> >> > >> >> >> >> Ah. Doesn't seem to be supported on arm(32) unfortunately. >> > >> > Then you could at least use standard debugging features : >> > >> > CONFIG_SLAB=y >> > CONFIG_SLABINFO=y >> > CONFIG_DEBUG_SLAB=y >> > CONFIG_DEBUG_SLAB_LEAK=y >> > >> > (Or equivalent SLUB options) >> > >> > and >> > >> > CONFIG_DEBUG_PAGEALLOC=y >> > >> > (If arm(32) has CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y) >> >> I tried with those enabled and while toggling power on the Bluetooth >> interface I usually get this after a few iterations: >> kernel: Bluetooth: Unable to push skb to HCI core(-6) > > Well, this code seems to be quite buggy. > > I do not have time to audit it, but 5 minutes are enough to spot 2 > issues. > > skb, once given to another queue/layer should not be accessed anymore. > Ok. Unfortunately I still see the slab corruption even with your changes.
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram
On Tue, Jan 5, 2016 at 3:14 PM, Eric Dumazet wrote: > On Tue, 2016-01-05 at 12:07 +0100, Jacob Siverskog wrote: >> On Mon, Jan 4, 2016 at 4:25 PM, Eric Dumazet wrote: >> > On Mon, 2016-01-04 at 10:10 +0100, Jacob Siverskog wrote: >> >> On Wed, Dec 30, 2015 at 11:30 PM, Cong Wang >> >> wrote: >> >> > On Wed, Dec 30, 2015 at 6:30 AM, Jacob Siverskog >> >> > wrote: >> >> >> On Wed, Dec 30, 2015 at 2:26 PM, Eric Dumazet >> >> >> wrote: >> >> >>> How often can you trigger this bug ? >> >> >> >> >> >> Ok. I don't have a good repro to trigger it unfortunately, I've seen >> >> >> it just a >> >> >> few times when bringing up/down network interfaces. Does the trace >> >> >> give any clue? >> >> >> >> >> > >> >> > A little bit. You need to help people to narrow down the problem >> >> > because there are too many places using skb->next and skb->prev. >> >> > >> >> > Since you mentioned it seems related to network interface flip, >> >> > what network interfaces are you using? What's is your TC setup? >> >> > >> >> > Thanks. >> >> >> >> The system contains only one physical network interface (TI WL1837, >> >> wl18xx module). >> >> The state prior to the crash was as follows: >> >> - One virtual network interface active (as STA, associated with access >> >> point) >> >> - Bluetooth (BLE only) active (same physical chip, co-existence, >> >> btwilink/st_drv modules) >> >> >> >> Actions made around the time of the crash: >> >> - Bluetooth disabled >> >> - One additional virtual network interface brought up (also as STA) >> >> >> >> I believe the crash occurred between these two actions. I just saw >> >> that there are some interesting events in the log prior to the crash: >> >> kernel: Bluetooth: Unable to push skb to HCI core(-6) >> >> kernel: (stc): proto stack 4's ->recv failed >> >> kernel: (stc): remove_channel_from_table: id 3 >> >> kernel: (stc): remove_channel_from_table: id 2 >> >> kernel: (stc): remove_channel_from_table: id 4 >> >> kernel: (stc): all chnl_ids unregistered >> >> kernel: (stk) :ldisc_install = 0(stc): st_tty_close >> >> >> >> The first print is from btwilink.c. However, I can't see the >> >> connection between Bluetooth (BLE) and UDP/IPv6 (we're not using >> >> 6LoWPAN or anything similar). >> >> >> >> Thanks, Jacob >> > >> > Definitely these details are useful ;) >> > >> > Could you try : >> > >> > diff --git a/drivers/misc/ti-st/st_core.c b/drivers/misc/ti-st/st_core.c >> > index 6e3af8b42cdd..0c99a74fb895 100644 >> > --- a/drivers/misc/ti-st/st_core.c >> > +++ b/drivers/misc/ti-st/st_core.c >> > @@ -912,7 +912,9 @@ void st_core_exit(struct st_data_s *st_gdata) >> > skb_queue_purge(_gdata->txq); >> > skb_queue_purge(_gdata->tx_waitq); >> > kfree_skb(st_gdata->rx_skb); >> > + st_gdata->rx_skb = NULL; >> > kfree_skb(st_gdata->tx_skb); >> > + st_gdata->tx_skb = NULL; >> > /* TTY ldisc cleanup */ >> > err = tty_unregister_ldisc(N_TI_WL); >> > if (err) >> > >> > >> >> Sure. Since I don't have a good way to trigger the initial issue, I >> can't really know if there is a difference with your patch. However, >> normal usage seems to work as expected with your patch. I've tried to >> reproduce the initial issue with and without your patch repeatedly for >> hours and have not seen any crash in any of the runs so far. >> -- > > You might build a kernel with KASAN support to get maybe more chances to > trigger the bug. > > ( https://www.kernel.org/doc/Documentation/kasan.txt ) > Ah. Doesn't seem to be supported on arm(32) unfortunately. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram
On Mon, Jan 4, 2016 at 4:25 PM, Eric Dumazet wrote: > On Mon, 2016-01-04 at 10:10 +0100, Jacob Siverskog wrote: >> On Wed, Dec 30, 2015 at 11:30 PM, Cong Wang wrote: >> > On Wed, Dec 30, 2015 at 6:30 AM, Jacob Siverskog >> > wrote: >> >> On Wed, Dec 30, 2015 at 2:26 PM, Eric Dumazet wrote: >> >>> How often can you trigger this bug ? >> >> >> >> Ok. I don't have a good repro to trigger it unfortunately, I've seen it >> >> just a >> >> few times when bringing up/down network interfaces. Does the trace >> >> give any clue? >> >> >> > >> > A little bit. You need to help people to narrow down the problem >> > because there are too many places using skb->next and skb->prev. >> > >> > Since you mentioned it seems related to network interface flip, >> > what network interfaces are you using? What's is your TC setup? >> > >> > Thanks. >> >> The system contains only one physical network interface (TI WL1837, >> wl18xx module). >> The state prior to the crash was as follows: >> - One virtual network interface active (as STA, associated with access point) >> - Bluetooth (BLE only) active (same physical chip, co-existence, >> btwilink/st_drv modules) >> >> Actions made around the time of the crash: >> - Bluetooth disabled >> - One additional virtual network interface brought up (also as STA) >> >> I believe the crash occurred between these two actions. I just saw >> that there are some interesting events in the log prior to the crash: >> kernel: Bluetooth: Unable to push skb to HCI core(-6) >> kernel: (stc): proto stack 4's ->recv failed >> kernel: (stc): remove_channel_from_table: id 3 >> kernel: (stc): remove_channel_from_table: id 2 >> kernel: (stc): remove_channel_from_table: id 4 >> kernel: (stc): all chnl_ids unregistered >> kernel: (stk) :ldisc_install = 0(stc): st_tty_close >> >> The first print is from btwilink.c. However, I can't see the >> connection between Bluetooth (BLE) and UDP/IPv6 (we're not using >> 6LoWPAN or anything similar). >> >> Thanks, Jacob > > Definitely these details are useful ;) > > Could you try : > > diff --git a/drivers/misc/ti-st/st_core.c b/drivers/misc/ti-st/st_core.c > index 6e3af8b42cdd..0c99a74fb895 100644 > --- a/drivers/misc/ti-st/st_core.c > +++ b/drivers/misc/ti-st/st_core.c > @@ -912,7 +912,9 @@ void st_core_exit(struct st_data_s *st_gdata) > skb_queue_purge(_gdata->txq); > skb_queue_purge(_gdata->tx_waitq); > kfree_skb(st_gdata->rx_skb); > + st_gdata->rx_skb = NULL; > kfree_skb(st_gdata->tx_skb); > + st_gdata->tx_skb = NULL; > /* TTY ldisc cleanup */ > err = tty_unregister_ldisc(N_TI_WL); > if (err) > > Sure. Since I don't have a good way to trigger the initial issue, I can't really know if there is a difference with your patch. However, normal usage seems to work as expected with your patch. I've tried to reproduce the initial issue with and without your patch repeatedly for hours and have not seen any crash in any of the runs so far. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram
On Tue, Jan 5, 2016 at 3:14 PM, Eric Dumazet <eric.duma...@gmail.com> wrote: > On Tue, 2016-01-05 at 12:07 +0100, Jacob Siverskog wrote: >> On Mon, Jan 4, 2016 at 4:25 PM, Eric Dumazet <eric.duma...@gmail.com> wrote: >> > On Mon, 2016-01-04 at 10:10 +0100, Jacob Siverskog wrote: >> >> On Wed, Dec 30, 2015 at 11:30 PM, Cong Wang <xiyou.wangc...@gmail.com> >> >> wrote: >> >> > On Wed, Dec 30, 2015 at 6:30 AM, Jacob Siverskog >> >> > <jacob@teenage.engineering> wrote: >> >> >> On Wed, Dec 30, 2015 at 2:26 PM, Eric Dumazet <eduma...@google.com> >> >> >> wrote: >> >> >>> How often can you trigger this bug ? >> >> >> >> >> >> Ok. I don't have a good repro to trigger it unfortunately, I've seen >> >> >> it just a >> >> >> few times when bringing up/down network interfaces. Does the trace >> >> >> give any clue? >> >> >> >> >> > >> >> > A little bit. You need to help people to narrow down the problem >> >> > because there are too many places using skb->next and skb->prev. >> >> > >> >> > Since you mentioned it seems related to network interface flip, >> >> > what network interfaces are you using? What's is your TC setup? >> >> > >> >> > Thanks. >> >> >> >> The system contains only one physical network interface (TI WL1837, >> >> wl18xx module). >> >> The state prior to the crash was as follows: >> >> - One virtual network interface active (as STA, associated with access >> >> point) >> >> - Bluetooth (BLE only) active (same physical chip, co-existence, >> >> btwilink/st_drv modules) >> >> >> >> Actions made around the time of the crash: >> >> - Bluetooth disabled >> >> - One additional virtual network interface brought up (also as STA) >> >> >> >> I believe the crash occurred between these two actions. I just saw >> >> that there are some interesting events in the log prior to the crash: >> >> kernel: Bluetooth: Unable to push skb to HCI core(-6) >> >> kernel: (stc): proto stack 4's ->recv failed >> >> kernel: (stc): remove_channel_from_table: id 3 >> >> kernel: (stc): remove_channel_from_table: id 2 >> >> kernel: (stc): remove_channel_from_table: id 4 >> >> kernel: (stc): all chnl_ids unregistered >> >> kernel: (stk) :ldisc_install = 0(stc): st_tty_close >> >> >> >> The first print is from btwilink.c. However, I can't see the >> >> connection between Bluetooth (BLE) and UDP/IPv6 (we're not using >> >> 6LoWPAN or anything similar). >> >> >> >> Thanks, Jacob >> > >> > Definitely these details are useful ;) >> > >> > Could you try : >> > >> > diff --git a/drivers/misc/ti-st/st_core.c b/drivers/misc/ti-st/st_core.c >> > index 6e3af8b42cdd..0c99a74fb895 100644 >> > --- a/drivers/misc/ti-st/st_core.c >> > +++ b/drivers/misc/ti-st/st_core.c >> > @@ -912,7 +912,9 @@ void st_core_exit(struct st_data_s *st_gdata) >> > skb_queue_purge(_gdata->txq); >> > skb_queue_purge(_gdata->tx_waitq); >> > kfree_skb(st_gdata->rx_skb); >> > + st_gdata->rx_skb = NULL; >> > kfree_skb(st_gdata->tx_skb); >> > + st_gdata->tx_skb = NULL; >> > /* TTY ldisc cleanup */ >> > err = tty_unregister_ldisc(N_TI_WL); >> > if (err) >> > >> > >> >> Sure. Since I don't have a good way to trigger the initial issue, I >> can't really know if there is a difference with your patch. However, >> normal usage seems to work as expected with your patch. I've tried to >> reproduce the initial issue with and without your patch repeatedly for >> hours and have not seen any crash in any of the runs so far. >> -- > > You might build a kernel with KASAN support to get maybe more chances to > trigger the bug. > > ( https://www.kernel.org/doc/Documentation/kasan.txt ) > Ah. Doesn't seem to be supported on arm(32) unfortunately. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram
On Mon, Jan 4, 2016 at 4:25 PM, Eric Dumazet <eric.duma...@gmail.com> wrote: > On Mon, 2016-01-04 at 10:10 +0100, Jacob Siverskog wrote: >> On Wed, Dec 30, 2015 at 11:30 PM, Cong Wang <xiyou.wangc...@gmail.com> wrote: >> > On Wed, Dec 30, 2015 at 6:30 AM, Jacob Siverskog >> > <jacob@teenage.engineering> wrote: >> >> On Wed, Dec 30, 2015 at 2:26 PM, Eric Dumazet <eduma...@google.com> wrote: >> >>> How often can you trigger this bug ? >> >> >> >> Ok. I don't have a good repro to trigger it unfortunately, I've seen it >> >> just a >> >> few times when bringing up/down network interfaces. Does the trace >> >> give any clue? >> >> >> > >> > A little bit. You need to help people to narrow down the problem >> > because there are too many places using skb->next and skb->prev. >> > >> > Since you mentioned it seems related to network interface flip, >> > what network interfaces are you using? What's is your TC setup? >> > >> > Thanks. >> >> The system contains only one physical network interface (TI WL1837, >> wl18xx module). >> The state prior to the crash was as follows: >> - One virtual network interface active (as STA, associated with access point) >> - Bluetooth (BLE only) active (same physical chip, co-existence, >> btwilink/st_drv modules) >> >> Actions made around the time of the crash: >> - Bluetooth disabled >> - One additional virtual network interface brought up (also as STA) >> >> I believe the crash occurred between these two actions. I just saw >> that there are some interesting events in the log prior to the crash: >> kernel: Bluetooth: Unable to push skb to HCI core(-6) >> kernel: (stc): proto stack 4's ->recv failed >> kernel: (stc): remove_channel_from_table: id 3 >> kernel: (stc): remove_channel_from_table: id 2 >> kernel: (stc): remove_channel_from_table: id 4 >> kernel: (stc): all chnl_ids unregistered >> kernel: (stk) :ldisc_install = 0(stc): st_tty_close >> >> The first print is from btwilink.c. However, I can't see the >> connection between Bluetooth (BLE) and UDP/IPv6 (we're not using >> 6LoWPAN or anything similar). >> >> Thanks, Jacob > > Definitely these details are useful ;) > > Could you try : > > diff --git a/drivers/misc/ti-st/st_core.c b/drivers/misc/ti-st/st_core.c > index 6e3af8b42cdd..0c99a74fb895 100644 > --- a/drivers/misc/ti-st/st_core.c > +++ b/drivers/misc/ti-st/st_core.c > @@ -912,7 +912,9 @@ void st_core_exit(struct st_data_s *st_gdata) > skb_queue_purge(_gdata->txq); > skb_queue_purge(_gdata->tx_waitq); > kfree_skb(st_gdata->rx_skb); > + st_gdata->rx_skb = NULL; > kfree_skb(st_gdata->tx_skb); > + st_gdata->tx_skb = NULL; > /* TTY ldisc cleanup */ > err = tty_unregister_ldisc(N_TI_WL); > if (err) > > Sure. Since I don't have a good way to trigger the initial issue, I can't really know if there is a difference with your patch. However, normal usage seems to work as expected with your patch. I've tried to reproduce the initial issue with and without your patch repeatedly for hours and have not seen any crash in any of the runs so far. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram
On Wed, Dec 30, 2015 at 11:30 PM, Cong Wang wrote: > On Wed, Dec 30, 2015 at 6:30 AM, Jacob Siverskog > wrote: >> On Wed, Dec 30, 2015 at 2:26 PM, Eric Dumazet wrote: >>> How often can you trigger this bug ? >> >> Ok. I don't have a good repro to trigger it unfortunately, I've seen it just >> a >> few times when bringing up/down network interfaces. Does the trace >> give any clue? >> > > A little bit. You need to help people to narrow down the problem > because there are too many places using skb->next and skb->prev. > > Since you mentioned it seems related to network interface flip, > what network interfaces are you using? What's is your TC setup? > > Thanks. The system contains only one physical network interface (TI WL1837, wl18xx module). The state prior to the crash was as follows: - One virtual network interface active (as STA, associated with access point) - Bluetooth (BLE only) active (same physical chip, co-existence, btwilink/st_drv modules) Actions made around the time of the crash: - Bluetooth disabled - One additional virtual network interface brought up (also as STA) I believe the crash occurred between these two actions. I just saw that there are some interesting events in the log prior to the crash: kernel: Bluetooth: Unable to push skb to HCI core(-6) kernel: (stc): proto stack 4's ->recv failed kernel: (stc): remove_channel_from_table: id 3 kernel: (stc): remove_channel_from_table: id 2 kernel: (stc): remove_channel_from_table: id 4 kernel: (stc): all chnl_ids unregistered kernel: (stk) :ldisc_install = 0(stc): st_tty_close The first print is from btwilink.c. However, I can't see the connection between Bluetooth (BLE) and UDP/IPv6 (we're not using 6LoWPAN or anything similar). Thanks, Jacob -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram
On Wed, Dec 30, 2015 at 11:30 PM, Cong Wang <xiyou.wangc...@gmail.com> wrote: > On Wed, Dec 30, 2015 at 6:30 AM, Jacob Siverskog > <jacob@teenage.engineering> wrote: >> On Wed, Dec 30, 2015 at 2:26 PM, Eric Dumazet <eduma...@google.com> wrote: >>> How often can you trigger this bug ? >> >> Ok. I don't have a good repro to trigger it unfortunately, I've seen it just >> a >> few times when bringing up/down network interfaces. Does the trace >> give any clue? >> > > A little bit. You need to help people to narrow down the problem > because there are too many places using skb->next and skb->prev. > > Since you mentioned it seems related to network interface flip, > what network interfaces are you using? What's is your TC setup? > > Thanks. The system contains only one physical network interface (TI WL1837, wl18xx module). The state prior to the crash was as follows: - One virtual network interface active (as STA, associated with access point) - Bluetooth (BLE only) active (same physical chip, co-existence, btwilink/st_drv modules) Actions made around the time of the crash: - Bluetooth disabled - One additional virtual network interface brought up (also as STA) I believe the crash occurred between these two actions. I just saw that there are some interesting events in the log prior to the crash: kernel: Bluetooth: Unable to push skb to HCI core(-6) kernel: (stc): proto stack 4's ->recv failed kernel: (stc): remove_channel_from_table: id 3 kernel: (stc): remove_channel_from_table: id 2 kernel: (stc): remove_channel_from_table: id 4 kernel: (stc): all chnl_ids unregistered kernel: (stk) :ldisc_install = 0(stc): st_tty_close The first print is from btwilink.c. However, I can't see the connection between Bluetooth (BLE) and UDP/IPv6 (we're not using 6LoWPAN or anything similar). Thanks, Jacob -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram
On Wed, Dec 30, 2015 at 2:26 PM, Eric Dumazet wrote: > On Wed, Dec 30, 2015 at 6:14 AM, Jacob Siverskog > wrote: > >> Ok. Thanks for your feedback. How do you believe the issue could be >> solved? Investigating it gives: >> >> static inline void __skb_unlink(struct sk_buff *skb, struct sk_buff_head >> *list) >> { >> struct sk_buff *next, *prev; >> >> list->qlen--; >> 51c: e2433001 sub r3, r3, #1 >> 520: e58b3074 str r3, [fp, #116] ; 0x74 >> next = skb->next; >> prev = skb->prev; >> 524: e894000c ldm r4, {r2, r3} >> skb->next = skb->prev = NULL; >> 528: e5841000 str r1, [r4] >> 52c: e5841004 str r1, [r4, #4] >> next->prev = prev; >> 530: e5823004 str r3, [r2, #4] <-- >> trapping instruction (r2 NULL) >> >> Register contents: >> r7 : c58cfe1c r6 : c06351d0 r5 : c77810ac r4 : c583eac0 >> r3 : r2 : r1 : r0 : 2013 >> >> If I understand this correctly, then r4 = skb, r2 = next, r3 = prev. >> >> Should there be a check for this in __skb_try_recv_datagram? > > At this point corruption already happened. > We can not possibly detect every possible corruption caused by bugs > elsewhere in the kernel and just 'recover' at this point. > We must indeed find the root cause and fix it, instead of trying to hide it. > > How often can you trigger this bug ? Ok. I don't have a good repro to trigger it unfortunately, I've seen it just a few times when bringing up/down network interfaces. Does the trace give any clue? [] (__skb_recv_datagram) from [] (udpv6_recvmsg+0x1d0/0x6d0) [] (udpv6_recvmsg) from [] (inet_recvmsg+0x38/0x4c) [] (inet_recvmsg) from [] (___sys_recvmsg+0x94/0x170) [] (___sys_recvmsg) from [] (__sys_recvmsg+0x3c/0x6c) [] (__sys_recvmsg) from [] (ret_fast_syscall+0x0/0x3c) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram
On Tue, Dec 29, 2015 at 9:08 PM, David Miller wrote: > From: Rainer Weikusat > Date: Tue, 29 Dec 2015 19:42:36 + > >> Jacob Siverskog writes: >>> This should fix a NULL pointer dereference I encountered (dump >>> below). Since __skb_unlink is called while walking, >>> skb_queue_walk_safe should be used. >> >> The code in question is: > ... >> __skb_unlink is only called prior to returning from the function. >> Consequently, it won't affect the skb_queue_walk code. > > Agreed, this patch doesn't fix anything. Ok. Thanks for your feedback. How do you believe the issue could be solved? Investigating it gives: static inline void __skb_unlink(struct sk_buff *skb, struct sk_buff_head *list) { struct sk_buff *next, *prev; list->qlen--; 51c: e2433001 sub r3, r3, #1 520: e58b3074 str r3, [fp, #116] ; 0x74 next = skb->next; prev = skb->prev; 524: e894000c ldm r4, {r2, r3} skb->next = skb->prev = NULL; 528: e5841000 str r1, [r4] 52c: e5841004 str r1, [r4, #4] next->prev = prev; 530: e5823004 str r3, [r2, #4] <-- trapping instruction (r2 NULL) Register contents: r7 : c58cfe1c r6 : c06351d0 r5 : c77810ac r4 : c583eac0 r3 : r2 : r1 : r0 : 2013 If I understand this correctly, then r4 = skb, r2 = next, r3 = prev. Should there be a check for this in __skb_try_recv_datagram? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram
On Tue, Dec 29, 2015 at 9:08 PM, David Miller <da...@davemloft.net> wrote: > From: Rainer Weikusat <rweiku...@mobileactivedefense.com> > Date: Tue, 29 Dec 2015 19:42:36 + > >> Jacob Siverskog <jacob@teenage.engineering> writes: >>> This should fix a NULL pointer dereference I encountered (dump >>> below). Since __skb_unlink is called while walking, >>> skb_queue_walk_safe should be used. >> >> The code in question is: > ... >> __skb_unlink is only called prior to returning from the function. >> Consequently, it won't affect the skb_queue_walk code. > > Agreed, this patch doesn't fix anything. Ok. Thanks for your feedback. How do you believe the issue could be solved? Investigating it gives: static inline void __skb_unlink(struct sk_buff *skb, struct sk_buff_head *list) { struct sk_buff *next, *prev; list->qlen--; 51c: e2433001 sub r3, r3, #1 520: e58b3074 str r3, [fp, #116] ; 0x74 next = skb->next; prev = skb->prev; 524: e894000c ldm r4, {r2, r3} skb->next = skb->prev = NULL; 528: e5841000 str r1, [r4] 52c: e5841004 str r1, [r4, #4] next->prev = prev; 530: e5823004 str r3, [r2, #4] <-- trapping instruction (r2 NULL) Register contents: r7 : c58cfe1c r6 : c06351d0 r5 : c77810ac r4 : c583eac0 r3 : r2 : r1 : r0 : 2013 If I understand this correctly, then r4 = skb, r2 = next, r3 = prev. Should there be a check for this in __skb_try_recv_datagram? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram
On Wed, Dec 30, 2015 at 2:26 PM, Eric Dumazet <eduma...@google.com> wrote: > On Wed, Dec 30, 2015 at 6:14 AM, Jacob Siverskog > <jacob@teenage.engineering> wrote: > >> Ok. Thanks for your feedback. How do you believe the issue could be >> solved? Investigating it gives: >> >> static inline void __skb_unlink(struct sk_buff *skb, struct sk_buff_head >> *list) >> { >> struct sk_buff *next, *prev; >> >> list->qlen--; >> 51c: e2433001 sub r3, r3, #1 >> 520: e58b3074 str r3, [fp, #116] ; 0x74 >> next = skb->next; >> prev = skb->prev; >> 524: e894000c ldm r4, {r2, r3} >> skb->next = skb->prev = NULL; >> 528: e5841000 str r1, [r4] >> 52c: e5841004 str r1, [r4, #4] >> next->prev = prev; >> 530: e5823004 str r3, [r2, #4] <-- >> trapping instruction (r2 NULL) >> >> Register contents: >> r7 : c58cfe1c r6 : c06351d0 r5 : c77810ac r4 : c583eac0 >> r3 : r2 : r1 : r0 : 2013 >> >> If I understand this correctly, then r4 = skb, r2 = next, r3 = prev. >> >> Should there be a check for this in __skb_try_recv_datagram? > > At this point corruption already happened. > We can not possibly detect every possible corruption caused by bugs > elsewhere in the kernel and just 'recover' at this point. > We must indeed find the root cause and fix it, instead of trying to hide it. > > How often can you trigger this bug ? Ok. I don't have a good repro to trigger it unfortunately, I've seen it just a few times when bringing up/down network interfaces. Does the trace give any clue? [] (__skb_recv_datagram) from [] (udpv6_recvmsg+0x1d0/0x6d0) [] (udpv6_recvmsg) from [] (inet_recvmsg+0x38/0x4c) [] (inet_recvmsg) from [] (___sys_recvmsg+0x94/0x170) [] (___sys_recvmsg) from [] (__sys_recvmsg+0x3c/0x6c) [] (__sys_recvmsg) from [] (ret_fast_syscall+0x0/0x3c) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram
033 c0002029 c0087514 fc40: 0004 0817 c5a74c80 c58cfd48 0004 c58ce000 fc60: c7781040 c0019aa4 0817 c00164f8 fc80: 0817 c0016288 0004 c0607b14 c58cfd48 c58ce000 fca0: c7781040 c00092c0 c5957a00 c7738cf0 c7744940 c7158000 c7738cf0 fcc0: c0321e3c c5aa287c 004c9b81 c7744940 fce0: c58ce000 c03080a8 c5957a68 0001 c06351d0 fd00: c5941340 c5aa2800 c5aa287c c7744940 000e c58ce000 fd20: c5aa287c c037e5f0 0065 c02fc0a8 6093 c58cfd7c fd40: c58cfe20 c0013120 2013 c583eac0 c77810ac fd60: c06351d0 c58cfe1c c58cfe20 c58ce000 c7781040 2013 c58cfd98 fd80: c0398f1c c02fc0a8 6093 0051 0010 0029 0008 fda0: c02fbc64 0040 c778115c c03a9ed8 c58cfdc4 0065 fdc0: 005d 00ff fb00 0004 c0398d4c fde0: c7781040 c58cff6c 22f8 be8383ec c06351d0 c06351d0 c0398f1c fe00: c58cfe24 c58cfe70 be8383e4 0040 c77812a8 fe20: c58cfe2c 0004 c0398d4c c58cff6c c6a25c00 c58cfeb8 be8383ec fe40: be838408 c0367a2c c58cfe5c c58cff6c fe60: c6a25c00 c58cff6c 0040 c02efff4 be838874 be8388c0 22f8 fe80: c0618d78 ff14338a 9abb1afe b2b617e7 0ab4 b2b99e16 0ab4 fea0: c6a1fcc0 c0608374 c0608374 0001 e914000a fec0: 80fe ff14338a 9abb1afe 0004 c00499ac fee0: c77ac000 0002 c063a848 c0049c9c c5a74c80 ff00: c77ac018 c7123040 2dfb 0c95f107 c58cff78 05106300 2dfb 0051 ff20: be83ac08 0001 0018 be83ac08 0008 0004 be8383ec ff40: c6a25c00 0129 c000f3a4 c58ce000 c02f0d74 be83ac08 ff60: c58cff78 fff7 c58cfeb8 22f8 ff80: c58cfe78 0001 be838408 0400 be838890 be83883f ffa0: c000f1e0 be838890 be83883f 000e be8383ec ffc0: be838890 be83883f 0129 be838848 be838844 ffe0: 0006a228 be8383c8 ea5c b6f39fd8 6010 000e [] (inet_sock_destruct) from [] (sk_destruct+0x18/0xe0) [] (sk_destruct) from [] (inet_release+0x44/0x70) [] (inet_release) from [] (sock_release+0x20/0x98) [] (sock_release) from [] (sock_close+0xc/0x14) [] (sock_close) from [] (__fput+0x80/0x1fc) [] (__fput) from [] (task_work_run+0x6c/0x9c) [] (task_work_run) from [] (do_exit+0x2ac/0x95c) [] (do_exit) from [] (die+0x180/0x3b0) [] (die) from [] (__do_kernel_fault.part.0+0x64/0x1e4) [] (__do_kernel_fault.part.0) from [] (do_page_fault+0x270/0x298) [] (do_page_fault) from [] (do_DataAbort+0x38/0xb4) [] (do_DataAbort) from [] (__dabt_svc+0x40/0x60) Exception stack(0xc58cfd48 to 0xc58cfd90) fd40: 2013 c583eac0 c77810ac fd60: c06351d0 c58cfe1c c58cfe20 c58ce000 c7781040 2013 c58cfd98 fd80: c0398f1c c02fc0a8 6093 [] (__dabt_svc) from [] (__skb_recv_datagram+0x428/0x598) [] (__skb_recv_datagram) from [] (udpv6_recvmsg+0x1d0/0x6d0) [] (udpv6_recvmsg) from [] (inet_recvmsg+0x38/0x4c) [] (inet_recvmsg) from [] (___sys_recvmsg+0x94/0x170) [] (___sys_recvmsg) from [] (__sys_recvmsg+0x3c/0x6c) [] (__sys_recvmsg) from [] (ret_fast_syscall+0x0/0x3c) Code: e5842074 e8930006 e5835000 e5835004 (e5812004) Signed-off-by: Jacob Siverskog --- net/core/datagram.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/core/datagram.c b/net/core/datagram.c index fa9dc64..e8b3cab 100644 --- a/net/core/datagram.c +++ b/net/core/datagram.c @@ -201,7 +201,7 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags, struct sk_buff **last) { struct sk_buff_head *queue = >sk_receive_queue; - struct sk_buff *skb; + struct sk_buff *skb, *next; unsigned long cpu_flags; /* * Caller is allowed not to check sk->sk_err before skb_recv_datagram() @@ -222,7 +222,7 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags, *last = (struct sk_buff *)queue; spin_lock_irqsave(>lock, cpu_flags); - skb_queue_walk(queue, skb) { + skb_queue_walk_safe(queue, skb, next) { *last = skb; *peeked = skb->peeked; if (flags & MSG_PEEK) { -- 2.6.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram
033 c0002029 c0087514 fc40: 0004 0817 c5a74c80 c58cfd48 0004 c58ce000 fc60: c7781040 c0019aa4 0817 c00164f8 fc80: 0817 c0016288 0004 c0607b14 c58cfd48 c58ce000 fca0: c7781040 c00092c0 c5957a00 c7738cf0 c7744940 c7158000 c7738cf0 fcc0: c0321e3c c5aa287c 004c9b81 c7744940 fce0: c58ce000 c03080a8 c5957a68 0001 c06351d0 fd00: c5941340 c5aa2800 c5aa287c c7744940 000e c58ce000 fd20: c5aa287c c037e5f0 0065 c02fc0a8 6093 c58cfd7c fd40: c58cfe20 c0013120 2013 c583eac0 c77810ac fd60: c06351d0 c58cfe1c c58cfe20 c58ce000 c7781040 2013 c58cfd98 fd80: c0398f1c c02fc0a8 6093 0051 0010 0029 0008 fda0: c02fbc64 0040 c778115c c03a9ed8 c58cfdc4 0065 fdc0: 005d 00ff fb00 0004 c0398d4c fde0: c7781040 c58cff6c 22f8 be8383ec c06351d0 c06351d0 c0398f1c fe00: c58cfe24 c58cfe70 be8383e4 0040 c77812a8 fe20: c58cfe2c 0004 c0398d4c c58cff6c c6a25c00 c58cfeb8 be8383ec fe40: be838408 c0367a2c c58cfe5c c58cff6c fe60: c6a25c00 c58cff6c 0040 c02efff4 be838874 be8388c0 22f8 fe80: c0618d78 ff14338a 9abb1afe b2b617e7 0ab4 b2b99e16 0ab4 fea0: c6a1fcc0 c0608374 c0608374 0001 e914000a fec0: 80fe ff14338a 9abb1afe 0004 c00499ac fee0: c77ac000 0002 c063a848 c0049c9c c5a74c80 ff00: c77ac018 c7123040 2dfb 0c95f107 c58cff78 05106300 2dfb 0051 ff20: be83ac08 0001 0018 be83ac08 0008 0004 be8383ec ff40: c6a25c00 0129 c000f3a4 c58ce000 c02f0d74 be83ac08 ff60: c58cff78 fff7 c58cfeb8 22f8 ff80: c58cfe78 0001 be838408 0400 be838890 be83883f ffa0: c000f1e0 be838890 be83883f 000e be8383ec ffc0: be838890 be83883f 0129 be838848 be838844 ffe0: 0006a228 be8383c8 ea5c b6f39fd8 6010 000e [] (inet_sock_destruct) from [] (sk_destruct+0x18/0xe0) [] (sk_destruct) from [] (inet_release+0x44/0x70) [] (inet_release) from [] (sock_release+0x20/0x98) [] (sock_release) from [] (sock_close+0xc/0x14) [] (sock_close) from [] (__fput+0x80/0x1fc) [] (__fput) from [] (task_work_run+0x6c/0x9c) [] (task_work_run) from [] (do_exit+0x2ac/0x95c) [] (do_exit) from [] (die+0x180/0x3b0) [] (die) from [] (__do_kernel_fault.part.0+0x64/0x1e4) [] (__do_kernel_fault.part.0) from [] (do_page_fault+0x270/0x298) [] (do_page_fault) from [] (do_DataAbort+0x38/0xb4) [] (do_DataAbort) from [] (__dabt_svc+0x40/0x60) Exception stack(0xc58cfd48 to 0xc58cfd90) fd40: 2013 c583eac0 c77810ac fd60: c06351d0 c58cfe1c c58cfe20 c58ce000 c7781040 2013 c58cfd98 fd80: c0398f1c c02fc0a8 6093 [] (__dabt_svc) from [] (__skb_recv_datagram+0x428/0x598) [] (__skb_recv_datagram) from [] (udpv6_recvmsg+0x1d0/0x6d0) [] (udpv6_recvmsg) from [] (inet_recvmsg+0x38/0x4c) [] (inet_recvmsg) from [] (___sys_recvmsg+0x94/0x170) [] (___sys_recvmsg) from [] (__sys_recvmsg+0x3c/0x6c) [] (__sys_recvmsg) from [] (ret_fast_syscall+0x0/0x3c) Code: e5842074 e8930006 e5835000 e5835004 (e5812004) Signed-off-by: Jacob Siverskog <jacob@teenage.engineering> --- net/core/datagram.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/core/datagram.c b/net/core/datagram.c index fa9dc64..e8b3cab 100644 --- a/net/core/datagram.c +++ b/net/core/datagram.c @@ -201,7 +201,7 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags, struct sk_buff **last) { struct sk_buff_head *queue = >sk_receive_queue; - struct sk_buff *skb; + struct sk_buff *skb, *next; unsigned long cpu_flags; /* * Caller is allowed not to check sk->sk_err before skb_recv_datagram() @@ -222,7 +222,7 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags, *last = (struct sk_buff *)queue; spin_lock_irqsave(>lock, cpu_flags); - skb_queue_walk(queue, skb) { + skb_queue_walk_safe(queue, skb, next) { *last = skb; *peeked = skb->peeked; if (flags & MSG_PEEK) { -- 2.6.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/1] clk: si5351: Add PLL soft reset
This is according to figure 12 ("I2C Programming Procedure") in "Si5351A/B/C Data Sheet" (https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351-B.pdf). Without the PLL soft reset, we were unable to get three outputs working at the same time. According to Silicon Labs support, performing PLL soft reset will only be noticeable if the PLL parameters have been changed. Signed-off-by: Jacob Siverskog Signed-off-by: Jens Rudberg Acked-by: Sebastian Hesselbarth --- drivers/clk/clk-si5351.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c index e346b22..850316a 100644 --- a/drivers/clk/clk-si5351.c +++ b/drivers/clk/clk-si5351.c @@ -1091,6 +1091,13 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, unsigned long rate, si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num, SI5351_CLK_POWERDOWN, 0); + /* +* Do a pll soft reset on both plls, needed in some cases to get +* all outputs running. +*/ + si5351_reg_write(hwdata->drvdata, SI5351_PLL_RESET, +SI5351_PLL_RESET_A | SI5351_PLL_RESET_B); + dev_dbg(>drvdata->client->dev, "%s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n", __func__, clk_hw_get_name(hw), (1 << rdiv), -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 0/1] clk: si5351: Add PLL soft reset
Hi! Changes in v3: - Fix multiline comment style Changes in v2: - Output disabling and power down removed in order to prevent breaking systems requiring always-enabled clocks - Cosmetic changes Jacob Jacob Siverskog (1): clk: si5351: Add PLL soft reset drivers/clk/clk-si5351.c | 7 +++ 1 file changed, 7 insertions(+) -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] clk: si5351: Add PLL soft reset
This is according to figure 12 ("I2C Programming Procedure") in "Si5351A/B/C Data Sheet" (https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351-B.pdf). Without the PLL soft reset, we were unable to get three outputs working at the same time. According to Silicon Labs support, performing PLL soft reset will only be noticeable if the PLL parameters have been changed. Signed-off-by: Jacob Siverskog Signed-off-by: Jens Rudberg --- drivers/clk/clk-si5351.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c index e346b22..984c058 100644 --- a/drivers/clk/clk-si5351.c +++ b/drivers/clk/clk-si5351.c @@ -1091,6 +1091,12 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, unsigned long rate, si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num, SI5351_CLK_POWERDOWN, 0); + /* do a pll soft reset on both plls, needed in some cases to get all +* outputs running +*/ + si5351_reg_write(hwdata->drvdata, SI5351_PLL_RESET, +SI5351_PLL_RESET_A | SI5351_PLL_RESET_B); + dev_dbg(>drvdata->client->dev, "%s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n", __func__, clk_hw_get_name(hw), (1 << rdiv), -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] clk: si5351: Add PLL soft reset
Hi! A representative from SiLabs writes: "All the currently published versions of the Si5351 (v1.0) and AN619 (v0.6) do not include the “Si5351A/C only” disclaimer. Based on this and our current understanding, I see no issue performing a PLLB reset for ‘B’ type devices.". Hence, it should not be any issues always performing the PLL reset. Changes in v2: - Output disabling and power down removed in order to prevent breaking systems requiring always-enabled clocks - Cosmetic changes Jacob Jacob Siverskog (1): clk: si5351: Add PLL soft reset drivers/clk/clk-si5351.c | 6 ++ 1 file changed, 6 insertions(+) -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: si5351: Add setup steps
Hi Sebastian! On Thu, Nov 19, 2015 at 10:00 PM, Sebastian Hesselbarth wrote: > On 19.11.2015 14:40, Jacob Siverskog wrote: >> This is according to figure 12 ("I2C Programming Procedure") in >> "Si5351A/B/C Data Sheet" >> (https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351-B.pdf). >> >> Without the PLL soft reset, we were unable to get three outputs >> working at the same time. >> >> According to Silicon Labs support, performing PLL soft reset will only >> be noticable if the PLL parameters have been changed. >> >> Signed-off-by: Jacob Siverskog >> Signed-off-by: Jens Rudberg >> --- >> drivers/clk/clk-si5351.c | 13 + >> 1 file changed, 13 insertions(+) > > Jacob, > > thanks for the patches! However, besides Mareks comments I have > some more below. > >> diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c >> index e346b22..110de35 100644 >> --- a/drivers/clk/clk-si5351.c >> +++ b/drivers/clk/clk-si5351.c >> @@ -1091,6 +1091,11 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, >> unsigned long rate, >> si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num, >> SI5351_CLK_POWERDOWN, 0); >> >> + /* do a pll soft reset on both plls, needed in some cases to get all >> + * outputs running */ >> + si5351_reg_write(hwdata->drvdata, SI5351_PLL_RESET, >> + SI5351_PLL_RESET_A | SI5351_PLL_RESET_B); >> + >> dev_dbg(>drvdata->client->dev, >> "%s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n", >> __func__, clk_hw_get_name(hw), (1 << rdiv), >> @@ -1359,6 +1364,14 @@ static int si5351_i2c_probe(struct i2c_client *client, >> return PTR_ERR(drvdata->regmap); >> } >> >> + /* Disable outputs */ >> + si5351_reg_write(drvdata, SI5351_OUTPUT_ENABLE_CTRL, 0xff); >> + >> + /* Power down all output drivers */ >> + for (n = 0; n < 8; n++) { >> + si5351_reg_write(drvdata, SI5351_CLK0_CTRL + n, 0x80); >> + } > > Is disabling outputs and clock drivers required? > > If we disable the clocks here unconditionally, it will > break those systems that require specific outputs to be always > enabled. Consider one clock output driving your SoC or similar. > > If it is really required, you'll have to mention that in the > patch description and we need to find a way to tag specific > outputs to be never disabled. The only change that we needed in order to get our setup working was the PLL soft reset. I added the other bits to conform more to the recommended programming procedure. Let's skip the output disabling and power down in order to not break the use case you're mentioning. Thanks, Jacob > > Sebastian > >> /* Disable interrupts */ >> si5351_reg_write(drvdata, SI5351_INTERRUPT_MASK, 0xf0); >> /* Ensure pll select is on XTAL for Si5351A/B */ >> > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: si5351: Add setup steps
Hi Marek! Thanks for your feedback. On Thu, Nov 19, 2015 at 3:18 PM, Belisko Marek wrote: > Hi Jacob, > > On Thu, Nov 19, 2015 at 2:40 PM, Jacob Siverskog > wrote: >> This is according to figure 12 ("I2C Programming Procedure") in >> "Si5351A/B/C Data Sheet" >> (https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351-B.pdf). >> >> Without the PLL soft reset, we were unable to get three outputs >> working at the same time. >> >> According to Silicon Labs support, performing PLL soft reset will only >> be noticable if the PLL parameters have been changed. >> >> Signed-off-by: Jacob Siverskog >> Signed-off-by: Jens Rudberg >> --- >> drivers/clk/clk-si5351.c | 13 + >> 1 file changed, 13 insertions(+) >> >> diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c >> index e346b22..110de35 100644 >> --- a/drivers/clk/clk-si5351.c >> +++ b/drivers/clk/clk-si5351.c >> @@ -1091,6 +1091,11 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, >> unsigned long rate, >> si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num, >> SI5351_CLK_POWERDOWN, 0); >> >> + /* do a pll soft reset on both plls, needed in some cases to get all >> +* outputs running */ > wrong format of multi line comment, please look at > Documentation/CodingStyle >> + si5351_reg_write(hwdata->drvdata, SI5351_PLL_RESET, >> +SI5351_PLL_RESET_A | SI5351_PLL_RESET_B); > according datasheet PLL_RESET_A/B are self clearing bits > valid for Si5351A/C only > so probably we need to add some code for B model Well, this is interesting. In the latest data sheet (that I've been reading, https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351-B.pdf), this is not mentioned at all. For the register descriptions, it only refers to "AN619 Generating a register map" (http://www.silabs.com/Support%20Documents/TechnicalDocs/AN619.pdf), where no such caveat exists. I found an old datasheet on my computer that has the same phrasing as yours. I have sent an e-mail to our SiLabs representative regarding this matter. >> + >> dev_dbg(>drvdata->client->dev, >> "%s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n", >> __func__, clk_hw_get_name(hw), (1 << rdiv), >> @@ -1359,6 +1364,14 @@ static int si5351_i2c_probe(struct i2c_client *client, >> return PTR_ERR(drvdata->regmap); >> } >> >> + /* Disable outputs */ >> + si5351_reg_write(drvdata, SI5351_OUTPUT_ENABLE_CTRL, 0xff); >> + >> + /* Power down all output drivers */ >> + for (n = 0; n < 8; n++) { >> + si5351_reg_write(drvdata, SI5351_CLK0_CTRL + n, 0x80); >> + } > ^^^ single line after for statement doesn't need curly > braces (if you run ./scripts/check_patch it will complain) I will fix all embarrassing cosmetic issues for the next version of the patch. >> + >> /* Disable interrupts */ >> si5351_reg_write(drvdata, SI5351_INTERRUPT_MASK, 0xf0); >> /* Ensure pll select is on XTAL for Si5351A/B */ >> -- >> 2.6.3 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ > > BR, > > marek > > -- > as simple and primitive as possible > - > Marek Belisko - OPEN-NANDRA > Freelance Developer > > Ruska Nova Ves 219 | Presov, 08005 Slovak Republic > Tel: +421 915 052 184 > skype: marekwhite > twitter: #opennandra > web: http://open-nandra.com Thanks, Jacob -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] clk: si5351: Add PLL soft reset
This is according to figure 12 ("I2C Programming Procedure") in "Si5351A/B/C Data Sheet" (https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351-B.pdf). Without the PLL soft reset, we were unable to get three outputs working at the same time. According to Silicon Labs support, performing PLL soft reset will only be noticeable if the PLL parameters have been changed. Signed-off-by: Jacob Siverskog <jacob@teenage.engineering> Signed-off-by: Jens Rudberg <jens@teenage.engineering> --- drivers/clk/clk-si5351.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c index e346b22..984c058 100644 --- a/drivers/clk/clk-si5351.c +++ b/drivers/clk/clk-si5351.c @@ -1091,6 +1091,12 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, unsigned long rate, si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num, SI5351_CLK_POWERDOWN, 0); + /* do a pll soft reset on both plls, needed in some cases to get all +* outputs running +*/ + si5351_reg_write(hwdata->drvdata, SI5351_PLL_RESET, +SI5351_PLL_RESET_A | SI5351_PLL_RESET_B); + dev_dbg(>drvdata->client->dev, "%s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n", __func__, clk_hw_get_name(hw), (1 << rdiv), -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] clk: si5351: Add PLL soft reset
Hi! A representative from SiLabs writes: "All the currently published versions of the Si5351 (v1.0) and AN619 (v0.6) do not include the “Si5351A/C only” disclaimer. Based on this and our current understanding, I see no issue performing a PLLB reset for ‘B’ type devices.". Hence, it should not be any issues always performing the PLL reset. Changes in v2: - Output disabling and power down removed in order to prevent breaking systems requiring always-enabled clocks - Cosmetic changes Jacob Jacob Siverskog (1): clk: si5351: Add PLL soft reset drivers/clk/clk-si5351.c | 6 ++ 1 file changed, 6 insertions(+) -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 0/1] clk: si5351: Add PLL soft reset
Hi! Changes in v3: - Fix multiline comment style Changes in v2: - Output disabling and power down removed in order to prevent breaking systems requiring always-enabled clocks - Cosmetic changes Jacob Jacob Siverskog (1): clk: si5351: Add PLL soft reset drivers/clk/clk-si5351.c | 7 +++ 1 file changed, 7 insertions(+) -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/1] clk: si5351: Add PLL soft reset
This is according to figure 12 ("I2C Programming Procedure") in "Si5351A/B/C Data Sheet" (https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351-B.pdf). Without the PLL soft reset, we were unable to get three outputs working at the same time. According to Silicon Labs support, performing PLL soft reset will only be noticeable if the PLL parameters have been changed. Signed-off-by: Jacob Siverskog <jacob@teenage.engineering> Signed-off-by: Jens Rudberg <jens@teenage.engineering> Acked-by: Sebastian Hesselbarth <sebastian.hesselba...@gmail.com> --- drivers/clk/clk-si5351.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c index e346b22..850316a 100644 --- a/drivers/clk/clk-si5351.c +++ b/drivers/clk/clk-si5351.c @@ -1091,6 +1091,13 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, unsigned long rate, si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num, SI5351_CLK_POWERDOWN, 0); + /* +* Do a pll soft reset on both plls, needed in some cases to get +* all outputs running. +*/ + si5351_reg_write(hwdata->drvdata, SI5351_PLL_RESET, +SI5351_PLL_RESET_A | SI5351_PLL_RESET_B); + dev_dbg(>drvdata->client->dev, "%s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n", __func__, clk_hw_get_name(hw), (1 << rdiv), -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: si5351: Add setup steps
Hi Marek! Thanks for your feedback. On Thu, Nov 19, 2015 at 3:18 PM, Belisko Marek <marek.beli...@gmail.com> wrote: > Hi Jacob, > > On Thu, Nov 19, 2015 at 2:40 PM, Jacob Siverskog > <jacob@teenage.engineering> wrote: >> This is according to figure 12 ("I2C Programming Procedure") in >> "Si5351A/B/C Data Sheet" >> (https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351-B.pdf). >> >> Without the PLL soft reset, we were unable to get three outputs >> working at the same time. >> >> According to Silicon Labs support, performing PLL soft reset will only >> be noticable if the PLL parameters have been changed. >> >> Signed-off-by: Jacob Siverskog <jacob@teenage.engineering> >> Signed-off-by: Jens Rudberg <jens@teenage.engineering> >> --- >> drivers/clk/clk-si5351.c | 13 + >> 1 file changed, 13 insertions(+) >> >> diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c >> index e346b22..110de35 100644 >> --- a/drivers/clk/clk-si5351.c >> +++ b/drivers/clk/clk-si5351.c >> @@ -1091,6 +1091,11 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, >> unsigned long rate, >> si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num, >> SI5351_CLK_POWERDOWN, 0); >> >> + /* do a pll soft reset on both plls, needed in some cases to get all >> +* outputs running */ > wrong format of multi line comment, please look at > Documentation/CodingStyle >> + si5351_reg_write(hwdata->drvdata, SI5351_PLL_RESET, >> +SI5351_PLL_RESET_A | SI5351_PLL_RESET_B); > according datasheet PLL_RESET_A/B are self clearing bits > valid for Si5351A/C only > so probably we need to add some code for B model Well, this is interesting. In the latest data sheet (that I've been reading, https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351-B.pdf), this is not mentioned at all. For the register descriptions, it only refers to "AN619 Generating a register map" (http://www.silabs.com/Support%20Documents/TechnicalDocs/AN619.pdf), where no such caveat exists. I found an old datasheet on my computer that has the same phrasing as yours. I have sent an e-mail to our SiLabs representative regarding this matter. >> + >> dev_dbg(>drvdata->client->dev, >> "%s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n", >> __func__, clk_hw_get_name(hw), (1 << rdiv), >> @@ -1359,6 +1364,14 @@ static int si5351_i2c_probe(struct i2c_client *client, >> return PTR_ERR(drvdata->regmap); >> } >> >> + /* Disable outputs */ >> + si5351_reg_write(drvdata, SI5351_OUTPUT_ENABLE_CTRL, 0xff); >> + >> + /* Power down all output drivers */ >> + for (n = 0; n < 8; n++) { >> + si5351_reg_write(drvdata, SI5351_CLK0_CTRL + n, 0x80); >> + } > ^^^ single line after for statement doesn't need curly > braces (if you run ./scripts/check_patch it will complain) I will fix all embarrassing cosmetic issues for the next version of the patch. >> + >> /* Disable interrupts */ >> si5351_reg_write(drvdata, SI5351_INTERRUPT_MASK, 0xf0); >> /* Ensure pll select is on XTAL for Si5351A/B */ >> -- >> 2.6.3 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ > > BR, > > marek > > -- > as simple and primitive as possible > - > Marek Belisko - OPEN-NANDRA > Freelance Developer > > Ruska Nova Ves 219 | Presov, 08005 Slovak Republic > Tel: +421 915 052 184 > skype: marekwhite > twitter: #opennandra > web: http://open-nandra.com Thanks, Jacob -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: si5351: Add setup steps
Hi Sebastian! On Thu, Nov 19, 2015 at 10:00 PM, Sebastian Hesselbarth <sebastian.hesselba...@gmail.com> wrote: > On 19.11.2015 14:40, Jacob Siverskog wrote: >> This is according to figure 12 ("I2C Programming Procedure") in >> "Si5351A/B/C Data Sheet" >> (https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351-B.pdf). >> >> Without the PLL soft reset, we were unable to get three outputs >> working at the same time. >> >> According to Silicon Labs support, performing PLL soft reset will only >> be noticable if the PLL parameters have been changed. >> >> Signed-off-by: Jacob Siverskog <jacob@teenage.engineering> >> Signed-off-by: Jens Rudberg <jens@teenage.engineering> >> --- >> drivers/clk/clk-si5351.c | 13 + >> 1 file changed, 13 insertions(+) > > Jacob, > > thanks for the patches! However, besides Mareks comments I have > some more below. > >> diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c >> index e346b22..110de35 100644 >> --- a/drivers/clk/clk-si5351.c >> +++ b/drivers/clk/clk-si5351.c >> @@ -1091,6 +1091,11 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, >> unsigned long rate, >> si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num, >> SI5351_CLK_POWERDOWN, 0); >> >> + /* do a pll soft reset on both plls, needed in some cases to get all >> + * outputs running */ >> + si5351_reg_write(hwdata->drvdata, SI5351_PLL_RESET, >> + SI5351_PLL_RESET_A | SI5351_PLL_RESET_B); >> + >> dev_dbg(>drvdata->client->dev, >> "%s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n", >> __func__, clk_hw_get_name(hw), (1 << rdiv), >> @@ -1359,6 +1364,14 @@ static int si5351_i2c_probe(struct i2c_client *client, >> return PTR_ERR(drvdata->regmap); >> } >> >> + /* Disable outputs */ >> + si5351_reg_write(drvdata, SI5351_OUTPUT_ENABLE_CTRL, 0xff); >> + >> + /* Power down all output drivers */ >> + for (n = 0; n < 8; n++) { >> + si5351_reg_write(drvdata, SI5351_CLK0_CTRL + n, 0x80); >> + } > > Is disabling outputs and clock drivers required? > > If we disable the clocks here unconditionally, it will > break those systems that require specific outputs to be always > enabled. Consider one clock output driving your SoC or similar. > > If it is really required, you'll have to mention that in the > patch description and we need to find a way to tag specific > outputs to be never disabled. The only change that we needed in order to get our setup working was the PLL soft reset. I added the other bits to conform more to the recommended programming procedure. Let's skip the output disabling and power down in order to not break the use case you're mentioning. Thanks, Jacob > > Sebastian > >> /* Disable interrupts */ >> si5351_reg_write(drvdata, SI5351_INTERRUPT_MASK, 0xf0); >> /* Ensure pll select is on XTAL for Si5351A/B */ >> > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] clk: si5351: Add setup steps
This is according to figure 12 ("I2C Programming Procedure") in "Si5351A/B/C Data Sheet" (https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351-B.pdf). Without the PLL soft reset, we were unable to get three outputs working at the same time. According to Silicon Labs support, performing PLL soft reset will only be noticable if the PLL parameters have been changed. Signed-off-by: Jacob Siverskog Signed-off-by: Jens Rudberg --- drivers/clk/clk-si5351.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c index e346b22..110de35 100644 --- a/drivers/clk/clk-si5351.c +++ b/drivers/clk/clk-si5351.c @@ -1091,6 +1091,11 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, unsigned long rate, si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num, SI5351_CLK_POWERDOWN, 0); + /* do a pll soft reset on both plls, needed in some cases to get all +* outputs running */ + si5351_reg_write(hwdata->drvdata, SI5351_PLL_RESET, +SI5351_PLL_RESET_A | SI5351_PLL_RESET_B); + dev_dbg(>drvdata->client->dev, "%s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n", __func__, clk_hw_get_name(hw), (1 << rdiv), @@ -1359,6 +1364,14 @@ static int si5351_i2c_probe(struct i2c_client *client, return PTR_ERR(drvdata->regmap); } + /* Disable outputs */ + si5351_reg_write(drvdata, SI5351_OUTPUT_ENABLE_CTRL, 0xff); + + /* Power down all output drivers */ + for (n = 0; n < 8; n++) { + si5351_reg_write(drvdata, SI5351_CLK0_CTRL + n, 0x80); + } + /* Disable interrupts */ si5351_reg_write(drvdata, SI5351_INTERRUPT_MASK, 0xf0); /* Ensure pll select is on XTAL for Si5351A/B */ -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] clk: si5351: Add setup steps
This is according to figure 12 ("I2C Programming Procedure") in "Si5351A/B/C Data Sheet" (https://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351-B.pdf). Without the PLL soft reset, we were unable to get three outputs working at the same time. According to Silicon Labs support, performing PLL soft reset will only be noticable if the PLL parameters have been changed. Signed-off-by: Jacob Siverskog <jacob@teenage.engineering> Signed-off-by: Jens Rudberg <jens@teenage.engineering> --- drivers/clk/clk-si5351.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c index e346b22..110de35 100644 --- a/drivers/clk/clk-si5351.c +++ b/drivers/clk/clk-si5351.c @@ -1091,6 +1091,11 @@ static int si5351_clkout_set_rate(struct clk_hw *hw, unsigned long rate, si5351_set_bits(hwdata->drvdata, SI5351_CLK0_CTRL + hwdata->num, SI5351_CLK_POWERDOWN, 0); + /* do a pll soft reset on both plls, needed in some cases to get all +* outputs running */ + si5351_reg_write(hwdata->drvdata, SI5351_PLL_RESET, +SI5351_PLL_RESET_A | SI5351_PLL_RESET_B); + dev_dbg(>drvdata->client->dev, "%s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n", __func__, clk_hw_get_name(hw), (1 << rdiv), @@ -1359,6 +1364,14 @@ static int si5351_i2c_probe(struct i2c_client *client, return PTR_ERR(drvdata->regmap); } + /* Disable outputs */ + si5351_reg_write(drvdata, SI5351_OUTPUT_ENABLE_CTRL, 0xff); + + /* Power down all output drivers */ + for (n = 0; n < 8; n++) { + si5351_reg_write(drvdata, SI5351_CLK0_CTRL + n, 0x80); + } + /* Disable interrupts */ si5351_reg_write(drvdata, SI5351_INTERRUPT_MASK, 0xf0); /* Ensure pll select is on XTAL for Si5351A/B */ -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ti-st: use worker instead of calling st_int_write in wake up
From: Muhammad Hamza Farooq The wake up method is called with the port lock held. The st_int_write method calls port->ops->write with tries to acquire the lock again, causing CPU to wait infinitely. Right way to do is to write data to port in worker thread. Signed-off-by: Muhammad Hamza Farooq Signed-off-by: Jacob Siverskog --- drivers/misc/ti-st/st_core.c | 18 -- include/linux/ti_wilink_st.h | 1 + 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/misc/ti-st/st_core.c b/drivers/misc/ti-st/st_core.c index c8c6a36..6e3af8b 100644 --- a/drivers/misc/ti-st/st_core.c +++ b/drivers/misc/ti-st/st_core.c @@ -460,6 +460,13 @@ static void st_int_enqueue(struct st_data_s *st_gdata, struct sk_buff *skb) * - TTY layer when write's finished * - st_write (in context of the protocol stack) */ +static void work_fn_write_wakeup(struct work_struct *work) +{ + struct st_data_s *st_gdata = container_of(work, struct st_data_s, + work_write_wakeup); + + st_tx_wakeup((void *)st_gdata); +} void st_tx_wakeup(struct st_data_s *st_data) { struct sk_buff *skb; @@ -812,8 +819,12 @@ static void st_tty_wakeup(struct tty_struct *tty) /* don't do an wakeup for now */ clear_bit(TTY_DO_WRITE_WAKEUP, >flags); - /* call our internal wakeup */ - st_tx_wakeup((void *)st_gdata); + /* +* schedule the internal wakeup instead of calling directly to +* avoid lockup (port->lock needed in tty->ops->write is +* already taken here +*/ + schedule_work(_gdata->work_write_wakeup); } static void st_tty_flush_buffer(struct tty_struct *tty) @@ -881,6 +892,9 @@ int st_core_init(struct st_data_s **core_data) pr_err("unable to un-register ldisc"); return err; } + + INIT_WORK(_gdata->work_write_wakeup, work_fn_write_wakeup); + *core_data = st_gdata; return 0; } diff --git a/include/linux/ti_wilink_st.h b/include/linux/ti_wilink_st.h index c78dcfe..9b366ee 100644 --- a/include/linux/ti_wilink_st.h +++ b/include/linux/ti_wilink_st.h @@ -158,6 +158,7 @@ struct st_data_s { unsigned long ll_state; void *kim_data; struct tty_struct *tty; + struct work_struct work_write_wakeup; }; /* -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ti-st: use worker instead of calling st_int_write in wake up
From: Muhammad Hamza Farooq <mfar...@visteon.com> The wake up method is called with the port lock held. The st_int_write method calls port->ops->write with tries to acquire the lock again, causing CPU to wait infinitely. Right way to do is to write data to port in worker thread. Signed-off-by: Muhammad Hamza Farooq <mfar...@visteon.com> Signed-off-by: Jacob Siverskog <jacob@teenage.engineering> --- drivers/misc/ti-st/st_core.c | 18 -- include/linux/ti_wilink_st.h | 1 + 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/misc/ti-st/st_core.c b/drivers/misc/ti-st/st_core.c index c8c6a36..6e3af8b 100644 --- a/drivers/misc/ti-st/st_core.c +++ b/drivers/misc/ti-st/st_core.c @@ -460,6 +460,13 @@ static void st_int_enqueue(struct st_data_s *st_gdata, struct sk_buff *skb) * - TTY layer when write's finished * - st_write (in context of the protocol stack) */ +static void work_fn_write_wakeup(struct work_struct *work) +{ + struct st_data_s *st_gdata = container_of(work, struct st_data_s, + work_write_wakeup); + + st_tx_wakeup((void *)st_gdata); +} void st_tx_wakeup(struct st_data_s *st_data) { struct sk_buff *skb; @@ -812,8 +819,12 @@ static void st_tty_wakeup(struct tty_struct *tty) /* don't do an wakeup for now */ clear_bit(TTY_DO_WRITE_WAKEUP, >flags); - /* call our internal wakeup */ - st_tx_wakeup((void *)st_gdata); + /* +* schedule the internal wakeup instead of calling directly to +* avoid lockup (port->lock needed in tty->ops->write is +* already taken here +*/ + schedule_work(_gdata->work_write_wakeup); } static void st_tty_flush_buffer(struct tty_struct *tty) @@ -881,6 +892,9 @@ int st_core_init(struct st_data_s **core_data) pr_err("unable to un-register ldisc"); return err; } + + INIT_WORK(_gdata->work_write_wakeup, work_fn_write_wakeup); + *core_data = st_gdata; return 0; } diff --git a/include/linux/ti_wilink_st.h b/include/linux/ti_wilink_st.h index c78dcfe..9b366ee 100644 --- a/include/linux/ti_wilink_st.h +++ b/include/linux/ti_wilink_st.h @@ -158,6 +158,7 @@ struct st_data_s { unsigned long ll_state; void *kim_data; struct tty_struct *tty; + struct work_struct work_write_wakeup; }; /* -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/