[PATCH v4 1/3] ASoC: pcm179x: Split into core and SPI parts

2016-01-22 Thread Jacob Siverskog
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

2016-01-22 Thread Jacob Siverskog
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

2016-01-22 Thread Jacob Siverskog
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

2016-01-22 Thread Jacob Siverskog
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

2016-01-22 Thread Jacob Siverskog
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

2016-01-22 Thread Jacob Siverskog
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

2016-01-22 Thread Jacob Siverskog
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

2016-01-22 Thread Jacob Siverskog
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

2016-01-22 Thread Jacob Siverskog
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

2016-01-22 Thread Jacob Siverskog
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

2016-01-21 Thread Jacob Siverskog
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

2016-01-21 Thread Jacob Siverskog
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

2016-01-21 Thread Jacob Siverskog
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

2016-01-21 Thread Jacob Siverskog
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

2016-01-21 Thread Jacob Siverskog
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

2016-01-21 Thread Jacob Siverskog
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

2016-01-21 Thread Jacob Siverskog
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

2016-01-21 Thread Jacob Siverskog
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

2016-01-20 Thread Jacob Siverskog
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

2016-01-20 Thread Jacob Siverskog
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

2016-01-20 Thread Jacob Siverskog
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

2016-01-20 Thread Jacob Siverskog
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

2016-01-05 Thread Jacob Siverskog
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

2016-01-05 Thread Jacob Siverskog
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

2016-01-05 Thread Jacob Siverskog
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

2016-01-05 Thread Jacob Siverskog
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

2016-01-04 Thread Jacob Siverskog
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

2016-01-04 Thread Jacob Siverskog
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

2015-12-30 Thread Jacob Siverskog
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

2015-12-30 Thread Jacob Siverskog
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

2015-12-30 Thread Jacob Siverskog
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

2015-12-30 Thread Jacob Siverskog
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

2015-12-29 Thread Jacob Siverskog
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

2015-12-29 Thread Jacob Siverskog
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

2015-11-20 Thread Jacob Siverskog
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

2015-11-20 Thread Jacob Siverskog
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

2015-11-20 Thread Jacob Siverskog
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

2015-11-20 Thread Jacob Siverskog
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

2015-11-20 Thread Jacob Siverskog
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

2015-11-20 Thread Jacob Siverskog
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

2015-11-20 Thread Jacob Siverskog
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

2015-11-20 Thread Jacob Siverskog
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

2015-11-20 Thread Jacob Siverskog
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

2015-11-20 Thread Jacob Siverskog
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

2015-11-20 Thread Jacob Siverskog
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

2015-11-20 Thread Jacob Siverskog
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

2015-11-19 Thread Jacob Siverskog
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

2015-11-19 Thread Jacob Siverskog
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

2015-09-11 Thread Jacob Siverskog
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

2015-09-11 Thread Jacob Siverskog
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/