* linux-next: manual merge of the clk tree with the arm-soc tree
From: Tomasz Figa @ 2014-01-26 21:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E57744.5030807@gmail.com>
On 26.01.2014 21:59, Tomasz Figa wrote:
> Hi Stephen,
>
> On 13.01.2014 06:06, Stephen Rothwell wrote:
>> Hi Mike,
>>
>> Today's linux-next merge of the clk tree got a conflict in
>> drivers/clk/samsung/clk-exynos4.c between commit 86576fbe201b ("clk:
>> samsung: exynos4: Fix definition of div_mmc_pre4 divider") from the
>> arm-soc tree and commit 2d7382375054 ("clk: exynos4: replace clock ID
>> private enums with IDs from DT header") from the clk tree.
>>
>> I fixed it up (see below) and can carry the fix as necessary (no action
>> is required).
>
> It seems like somehow this fix-up has been lost in action and the
> conflict ended up being merged incorrectly in linux-next. Could you take
> a look?
Ehh, it's a false alarm fortunately. I've been looking at wrong ref,
silly me. Sorry for the noise.
Best regards,
Tomasz
^ permalink raw reply
* linux-next: manual merge of the clk tree with the arm-soc tree
From: Tomasz Figa @ 2014-01-26 20:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140113160630.cf720268510f5e0931c24284@canb.auug.org.au>
Hi Stephen,
On 13.01.2014 06:06, Stephen Rothwell wrote:
> Hi Mike,
>
> Today's linux-next merge of the clk tree got a conflict in
> drivers/clk/samsung/clk-exynos4.c between commit 86576fbe201b ("clk:
> samsung: exynos4: Fix definition of div_mmc_pre4 divider") from the
> arm-soc tree and commit 2d7382375054 ("clk: exynos4: replace clock ID
> private enums with IDs from DT header") from the clk tree.
>
> I fixed it up (see below) and can carry the fix as necessary (no action
> is required).
It seems like somehow this fix-up has been lost in action and the
conflict ended up being merged incorrectly in linux-next. Could you take
a look?
Best regards,
Tomasz
^ permalink raw reply
* [PATCH] arm64: add workaround for ambiguous C99 stdint.h types
From: Ard Biesheuvel @ 2014-01-26 20:30 UTC (permalink / raw)
To: linux-arm-kernel
In a way similar to ARM commit 09096f6a0ee2 ("ARM: 7822/1: add workaround
for ambiguous C99 stdint.h types"), this patch redefines the macros that
are used in stdint.h so its definitions of uint64_t and int64_t are
compatible with those of the kernel.
In order to do so, drop types.h from generic-y and create a specific arm64
version identical to the generic one with just the #define overrides added.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/include/asm/Kbuild | 1 -
arch/arm64/include/asm/types.h | 26 ++++++++++++++++++++++++++
2 files changed, 26 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/types.h
diff --git a/arch/arm64/include/asm/Kbuild b/arch/arm64/include/asm/Kbuild
index d0ff25de67ca..93e0653e8c65 100644
--- a/arch/arm64/include/asm/Kbuild
+++ b/arch/arm64/include/asm/Kbuild
@@ -44,7 +44,6 @@ generic-y += termbits.h
generic-y += termios.h
generic-y += topology.h
generic-y += trace_clock.h
-generic-y += types.h
generic-y += unaligned.h
generic-y += user.h
generic-y += vga.h
diff --git a/arch/arm64/include/asm/types.h b/arch/arm64/include/asm/types.h
new file mode 100644
index 000000000000..6519296da003
--- /dev/null
+++ b/arch/arm64/include/asm/types.h
@@ -0,0 +1,26 @@
+#ifndef __ASM_TYPES_H
+#define __ASM_TYPES_H
+
+#include <asm-generic/int-ll64.h>
+
+/*
+ * For Aarch64, there is some ambiguity in the definition of the types below
+ * between the kernel and GCC itself. This is usually not a big deal, but it
+ * causes trouble when including GCC's version of 'stdint.h' (this is the file
+ * that gets included when you #include <stdint.h> on a -ffreestanding build).
+ * As this file also gets included implicitly when including 'arm_neon.h' (the
+ * NEON intrinsics support header), we need the following to work around the
+ * issue if we want to use NEON intrinsics in the kernel.
+ */
+
+#ifdef __INT64_TYPE__
+#undef __INT64_TYPE__
+#define __INT64_TYPE__ __signed__ long long
+#endif
+
+#ifdef __UINT64_TYPE__
+#undef __UINT64_TYPE__
+#define __UINT64_TYPE__ unsigned long long
+#endif
+
+#endif /* __ASM_TYPES_H */
--
1.8.3.2
^ permalink raw reply related
* Freescale FEC packet loss
From: Marek Vasut @ 2014-01-26 19:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390762590.2735.39.camel@deadeye.wl.decadent.org.uk>
On Sunday, January 26, 2014 at 07:56:30 PM, Ben Hutchings wrote:
> On Wed, 2014-01-22 at 22:55 +0100, Marek Vasut wrote:
> > Hi guys,
> >
> > I am running stock Linux 3.13 on i.MX6Q SabreLite board. The CPU is
> > i.MX6Q TO 1.0 .
> >
> > I am hitting a WARNING when I use the FEC ethernet to transfer data, thus
> > I started investigating this problem. TL;DR I am not able to figure this
> > problem out, so I am not attaching a patch :-(
> >
> > Steps to reproduce:
> > -------------------
> > 1) Boot stock Linux 3.13 on i.MX6Q SabreLite board
> > 2) Plug in an SD card into one of the SD slots (I use the full-size one)
> > 3) Plug in an USB stick into one of the USB ports (I use the upper one)
> > 4) Plug in an ethernet cable into the board
> >
> > -> Connect the other side into a gigabit-capable PC
>
> [...]
>
> I think there are known problems with 1000BASE-T on the Sabre Lite
> board.
This is MX6-wide thing, not sabrelite specific actually.
> Two possible workarounds are to limit the PHY to 100BASE-TX
> (should be doable with ethtool) or force it to be clock master for
> 1000BASE-T (requires a driver patch).
Can you please elaborate on the later ? I don't quite understand that.
> The vendor kernel apparently does both!
More like the vendor kernel papers over this bug.
> Matthew Garrett has been trying to implement a workaround in a
> clean way.
Do you have any pointers about this please ?
Best regards,
Marek Vasut
^ permalink raw reply
* Freescale FEC packet loss
From: Ben Hutchings @ 2014-01-26 18:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201401222255.29467.marex@denx.de>
On Wed, 2014-01-22 at 22:55 +0100, Marek Vasut wrote:
> Hi guys,
>
> I am running stock Linux 3.13 on i.MX6Q SabreLite board. The CPU is i.MX6Q TO
> 1.0 .
>
> I am hitting a WARNING when I use the FEC ethernet to transfer data, thus I
> started investigating this problem. TL;DR I am not able to figure this problem
> out, so I am not attaching a patch :-(
>
> Steps to reproduce:
> -------------------
> 1) Boot stock Linux 3.13 on i.MX6Q SabreLite board
> 2) Plug in an SD card into one of the SD slots (I use the full-size one)
> 3) Plug in an USB stick into one of the USB ports (I use the upper one)
> 4) Plug in an ethernet cable into the board
> -> Connect the other side into a gigabit-capable PC
[...]
I think there are known problems with 1000BASE-T on the Sabre Lite
board. Two possible workarounds are to limit the PHY to 100BASE-TX
(should be doable with ethtool) or force it to be clock master for
1000BASE-T (requires a driver patch). The vendor kernel apparently does
both! Matthew Garrett has been trying to implement a workaround in a
clean way.
Ben.
--
Ben Hutchings
If the facts do not conform to your theory, they must be disposed of.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 828 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140126/b5b61309/attachment-0001.sig>
^ permalink raw reply
* [PATCH v3 2/5] ASoC: tda998x: add a codec driver for the TDA998x
From: Jean-Francois Moine @ 2014-01-26 18:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1391274627.git.moinejf@free.fr>
This patch adds a CODEC driver for the NXP TDA998x HDMI transmitter.
The CODEC handles both I2S and S/PDIF input and does dynamic input
switch in the TDA998x I2C driver on start/stop audio streaming.
This driver is DT only and it is loaded from its DT description as
a subnode in the TDA998x node.
Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 4 +
sound/soc/codecs/Kconfig | 6 ++
sound/soc/codecs/Makefile | 2 +
sound/soc/codecs/tda998x.c | 216 ++++++++++++++++++++++++++++++++++++++
4 files changed, 228 insertions(+)
create mode 100644 sound/soc/codecs/tda998x.c
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 2643be4..68f0b7b 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -20,6 +20,7 @@
#include <linux/hdmi.h>
#include <linux/module.h>
#include <linux/irq.h>
+#include <linux/of_platform.h>
#include <sound/asoundef.h>
#include <drm/drmP.h>
@@ -1387,6 +1388,9 @@ tda998x_encoder_init(struct i2c_client *client,
priv->vip_cntrl_2 = video;
}
+ /* load the optional CODEC */
+ of_platform_populate(np, NULL, NULL, &client->dev);
+
return 0;
fail:
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index b33b45d..747e387 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -352,6 +352,12 @@ config SND_SOC_STAC9766
config SND_SOC_TAS5086
tristate
+config SND_SOC_TDA998X
+ tristate
+ depends on OF
+ default y if DRM_I2C_NXP_TDA998X=y
+ default m if DRM_I2C_NXP_TDA998X=m
+
config SND_SOC_TLV320AIC23
tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index bc12676..a53d09e 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -62,6 +62,7 @@ snd-soc-sta32x-objs := sta32x.o
snd-soc-sta529-objs := sta529.o
snd-soc-stac9766-objs := stac9766.o
snd-soc-tas5086-objs := tas5086.o
+snd-soc-tda998x-objs := tda998x.o
snd-soc-tlv320aic23-objs := tlv320aic23.o
snd-soc-tlv320aic26-objs := tlv320aic26.o
snd-soc-tlv320aic3x-objs := tlv320aic3x.o
@@ -192,6 +193,7 @@ obj-$(CONFIG_SND_SOC_STA32X) += snd-soc-sta32x.o
obj-$(CONFIG_SND_SOC_STA529) += snd-soc-sta529.o
obj-$(CONFIG_SND_SOC_STAC9766) += snd-soc-stac9766.o
obj-$(CONFIG_SND_SOC_TAS5086) += snd-soc-tas5086.o
+obj-$(CONFIG_SND_SOC_TDA998X) += snd-soc-tda998x.o
obj-$(CONFIG_SND_SOC_TLV320AIC23) += snd-soc-tlv320aic23.o
obj-$(CONFIG_SND_SOC_TLV320AIC26) += snd-soc-tlv320aic26.o
obj-$(CONFIG_SND_SOC_TLV320AIC3X) += snd-soc-tlv320aic3x.o
diff --git a/sound/soc/codecs/tda998x.c b/sound/soc/codecs/tda998x.c
new file mode 100644
index 0000000..34d7086
--- /dev/null
+++ b/sound/soc/codecs/tda998x.c
@@ -0,0 +1,216 @@
+/*
+ * ALSA SoC TDA998X driver
+ *
+ * This driver is used by the NXP TDA998x HDMI transmitter.
+ *
+ * Copyright (C) 2014 Jean-Francois Moine
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <sound/soc.h>
+#include <sound/pcm.h>
+#include <linux/of.h>
+#include <linux/i2c.h>
+#include <drm/drm_encoder_slave.h>
+#include <drm/i2c/tda998x.h>
+
+#define TDA998X_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
+ SNDRV_PCM_FMTBIT_S20_3LE | \
+ SNDRV_PCM_FMTBIT_S24_LE | \
+ SNDRV_PCM_FMTBIT_S32_LE)
+
+struct tda_priv {
+ struct i2c_client *i2c_client;
+ struct snd_soc_codec *codec;
+ u8 ports[2];
+ int dai_id;
+ u8 *eld;
+};
+
+static int tda_get_encoder(struct tda_priv *priv)
+{
+ struct snd_soc_codec *codec = priv->codec;
+ struct device_node *np;
+
+ /* get the parent tda998x device */
+ np = of_get_parent(codec->dev->of_node);
+ if (!np || !of_device_is_compatible(np, "nxp,tda998x")) {
+ dev_err(codec->dev, "no or bad parent!\n");
+ return -EINVAL;
+ }
+ priv->i2c_client = of_find_i2c_device_by_node(np);
+ of_node_put(np);
+ return 0;
+}
+
+static int tda_start_stop(struct tda_priv *priv)
+{
+ int port;
+
+ /* give the audio parameters to the HDMI encoder */
+ if (priv->dai_id == AFMT_I2S)
+ port = priv->ports[0];
+ else
+ port = priv->ports[1];
+ tda998x_audio_update(priv->i2c_client, priv->dai_id, port);
+ return 0;
+}
+
+static int tda_startup(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct tda_priv *priv = snd_soc_codec_get_drvdata(dai->codec);
+
+ /* memorize the used DAI */
+ priv->dai_id = dai->id;
+
+ /* start the TDA998x audio */
+ return tda_start_stop(priv);
+}
+
+static void tda_shutdown(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct tda_priv *priv = snd_soc_codec_get_drvdata(dai->codec);
+
+ priv->dai_id = 0; /* streaming stop */
+ tda_start_stop(priv);
+}
+
+static const struct snd_soc_dai_ops tda_ops = {
+ .startup = tda_startup,
+ .shutdown = tda_shutdown,
+};
+
+static const struct snd_soc_dai_driver tda998x_dai[] = {
+ {
+ .name = "i2s-hifi",
+ .id = AFMT_I2S,
+ .playback = {
+ .stream_name = "HDMI I2S Playback",
+ .channels_min = 1,
+ .channels_max = 8,
+ .rates = SNDRV_PCM_RATE_CONTINUOUS,
+ .rate_min = 5512,
+ .rate_max = 192000,
+ .formats = TDA998X_FORMATS,
+ },
+ .ops = &tda_ops,
+ },
+ {
+ .name = "spdif-hifi",
+ .id = AFMT_SPDIF,
+ .playback = {
+ .stream_name = "HDMI SPDIF Playback",
+ .channels_min = 1,
+ .channels_max = 2,
+ .rates = SNDRV_PCM_RATE_CONTINUOUS,
+ .rate_min = 22050,
+ .rate_max = 192000,
+ .formats = TDA998X_FORMATS,
+ },
+ .ops = &tda_ops,
+ },
+};
+
+static const struct snd_soc_dapm_widget tda_widgets[] = {
+ SND_SOC_DAPM_OUTPUT("hdmi-out"),
+};
+static const struct snd_soc_dapm_route tda_routes[] = {
+ { "hdmi-out", NULL, "HDMI I2S Playback" },
+ { "hdmi-out", NULL, "HDMI SPDIF Playback" },
+};
+
+static int tda_probe(struct snd_soc_codec *codec)
+{
+ struct tda_priv *priv;
+ struct device_node *np = codec->dev->of_node;
+ int i, j, ret;
+ const char *p;
+
+ priv = devm_kzalloc(codec->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+ snd_soc_codec_set_drvdata(codec, priv);
+ priv->codec = codec;
+
+ /* get the audio input ports (I2s and S/PDIF) */
+ for (i = 0; i < 2; i++) {
+ u32 port;
+
+ ret = of_property_read_u32_index(np, "audio-ports", i, &port);
+ if (ret) {
+ if (i == 0)
+ dev_err(codec->dev,
+ "bad or missing audio-ports\n");
+ break;
+ }
+ ret = of_property_read_string_index(np, "audio-port-names",
+ i, &p);
+ if (ret) {
+ dev_err(codec->dev,
+ "missing audio-port-names[%d]\n", i);
+ break;
+ }
+ if (strcmp(p, "i2s") == 0) {
+ j = 0;
+ } else if (strcmp(p, "spdif") == 0) {
+ j = 1;
+ } else {
+ dev_err(codec->dev,
+ "bad audio-port-names '%s'\n", p);
+ break;
+ }
+ priv->ports[j] = port;
+ }
+
+ /* get the tda998x device */
+ return tda_get_encoder(priv);
+}
+
+static const struct snd_soc_codec_driver soc_codec_tda998x = {
+ .probe = tda_probe,
+ .dapm_widgets = tda_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(tda_widgets),
+ .dapm_routes = tda_routes,
+ .num_dapm_routes = ARRAY_SIZE(tda_routes),
+};
+
+static int tda998x_dev_probe(struct platform_device *pdev)
+{
+ return snd_soc_register_codec(&pdev->dev,
+ &soc_codec_tda998x,
+ tda998x_dai, ARRAY_SIZE(tda998x_dai));
+}
+
+static int tda998x_dev_remove(struct platform_device *pdev)
+{
+ snd_soc_unregister_codec(&pdev->dev);
+ return 0;
+}
+
+static const struct of_device_id tda998x_codec_ids[] = {
+ { .compatible = "nxp,tda998x-codec", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, tda998x_codec_ids);
+
+static struct platform_driver tda998x_driver = {
+ .probe = tda998x_dev_probe,
+ .remove = tda998x_dev_remove,
+ .driver = {
+ .name = "tda998x-codec",
+ .owner = THIS_MODULE,
+ .of_match_table = tda998x_codec_ids,
+ },
+};
+
+module_platform_driver(tda998x_driver);
+
+MODULE_AUTHOR("Jean-Francois Moine");
+MODULE_DESCRIPTION("TDA998x codec driver");
+MODULE_LICENSE("GPL");
--
1.9.rc1
^ permalink raw reply related
* [PATCH v2 2/5] ASoC: tda998x: add a codec driver for TDA998x
From: Jean-Francois Moine @ 2014-01-26 18:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1391081933.git.moinejf@free.fr>
This patch adds a CODEC driver for the NXP TDA998x HDMI transmitter.
The CODEC handles both I2S and S/PDIF input and does dynamic input
switch in the TDA998x I2C driver on audio streaming start/stop.
Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
sound/soc/codecs/Kconfig | 6 ++
sound/soc/codecs/Makefile | 2 +
sound/soc/codecs/tda998x.c | 237 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 245 insertions(+)
create mode 100644 sound/soc/codecs/tda998x.c
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index b33b45d..747e387 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -352,6 +352,12 @@ config SND_SOC_STAC9766
config SND_SOC_TAS5086
tristate
+config SND_SOC_TDA998X
+ tristate
+ depends on OF
+ default y if DRM_I2C_NXP_TDA998X=y
+ default m if DRM_I2C_NXP_TDA998X=m
+
config SND_SOC_TLV320AIC23
tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index bc12676..a53d09e 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -62,6 +62,7 @@ snd-soc-sta32x-objs := sta32x.o
snd-soc-sta529-objs := sta529.o
snd-soc-stac9766-objs := stac9766.o
snd-soc-tas5086-objs := tas5086.o
+snd-soc-tda998x-objs := tda998x.o
snd-soc-tlv320aic23-objs := tlv320aic23.o
snd-soc-tlv320aic26-objs := tlv320aic26.o
snd-soc-tlv320aic3x-objs := tlv320aic3x.o
@@ -192,6 +193,7 @@ obj-$(CONFIG_SND_SOC_STA32X) += snd-soc-sta32x.o
obj-$(CONFIG_SND_SOC_STA529) += snd-soc-sta529.o
obj-$(CONFIG_SND_SOC_STAC9766) += snd-soc-stac9766.o
obj-$(CONFIG_SND_SOC_TAS5086) += snd-soc-tas5086.o
+obj-$(CONFIG_SND_SOC_TDA998X) += snd-soc-tda998x.o
obj-$(CONFIG_SND_SOC_TLV320AIC23) += snd-soc-tlv320aic23.o
obj-$(CONFIG_SND_SOC_TLV320AIC26) += snd-soc-tlv320aic26.o
obj-$(CONFIG_SND_SOC_TLV320AIC3X) += snd-soc-tlv320aic3x.o
diff --git a/sound/soc/codecs/tda998x.c b/sound/soc/codecs/tda998x.c
new file mode 100644
index 0000000..585cdb6
--- /dev/null
+++ b/sound/soc/codecs/tda998x.c
@@ -0,0 +1,237 @@
+/*
+ * ALSA SoC TDA998X driver
+ *
+ * This driver is used by the NXP TDA998x HDMI transmitter.
+ *
+ * Copyright (C) 2014 Jean-Francois Moine
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <sound/soc.h>
+#include <sound/pcm.h>
+#include <linux/of.h>
+#include <linux/i2c.h>
+#include <drm/drm_encoder_slave.h>
+#include <drm/i2c/tda998x.h>
+
+#define TDA998X_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
+ SNDRV_PCM_FMTBIT_S20_3LE | \
+ SNDRV_PCM_FMTBIT_S24_LE | \
+ SNDRV_PCM_FMTBIT_S32_LE)
+
+struct tda_priv {
+ struct i2c_client *i2c_client;
+ struct snd_soc_codec *codec;
+ u8 ports[2];
+ int dai_id;
+ u8 *eld;
+};
+
+static void tda_get_encoder(struct tda_priv *priv)
+{
+ struct snd_soc_codec *codec = priv->codec;
+ struct device_node *np;
+ struct i2c_client *i2c_client;
+ static const struct of_device_id tda_dt[] = {
+ { .compatible = "nxp,tda998x" },
+ { },
+ };
+
+ /* search the tda998x device */
+ np = of_find_matching_node_and_match(NULL, tda_dt, NULL);
+ if (!np || !of_device_is_available(np)) {
+ dev_err(codec->dev, "No tda998x in DT\n");
+ return;
+ }
+ i2c_client = of_find_i2c_device_by_node(np);
+ of_node_put(np);
+ if (!i2c_client) {
+ dev_err(codec->dev, "no tda998x i2c client\n");
+ return;
+ }
+ if (!i2c_get_clientdata(i2c_client)) {
+ dev_err(codec->dev, "tda998x not initialized\n");
+ return;
+ }
+
+ priv->i2c_client = i2c_client;
+}
+
+static int tda_start_stop(struct tda_priv *priv)
+{
+ int port;
+
+ if (!priv->i2c_client) {
+ tda_get_encoder(priv);
+ if (!priv->i2c_client)
+ return -EINVAL;
+ }
+
+ /* give the audio parameters to the HDMI encoder */
+ if (priv->dai_id == AFMT_I2S)
+ port = priv->ports[0];
+ else
+ port = priv->ports[1];
+ tda998x_audio_update(priv->i2c_client, priv->dai_id, port);
+ return 0;
+}
+
+static int tda_startup(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct tda_priv *priv = snd_soc_codec_get_drvdata(dai->codec);
+
+ /* memorize the used DAI */
+ priv->dai_id = dai->id;
+
+ /* start the TDA998x audio */
+ return tda_start_stop(priv);
+}
+
+static void tda_shutdown(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct tda_priv *priv = snd_soc_codec_get_drvdata(dai->codec);
+
+ priv->dai_id = 0; /* streaming stop */
+ tda_start_stop(priv);
+}
+
+static const struct snd_soc_dai_ops tda_ops = {
+ .startup = tda_startup,
+ .shutdown = tda_shutdown,
+};
+
+static const struct snd_soc_dai_driver tda998x_dai[] = {
+ {
+ .name = "i2s-hifi",
+ .id = AFMT_I2S,
+ .playback = {
+ .stream_name = "HDMI I2S Playback",
+ .channels_min = 1,
+ .channels_max = 8,
+ .rates = SNDRV_PCM_RATE_CONTINUOUS,
+ .rate_min = 5512,
+ .rate_max = 192000,
+ .formats = TDA998X_FORMATS,
+
+ },
+ .ops = &tda_ops,
+ },
+ {
+ .name = "spdif-hifi",
+ .id = AFMT_SPDIF,
+ .playback = {
+ .stream_name = "HDMI SPDIF Playback",
+ .channels_min = 1,
+ .channels_max = 2,
+ .rates = SNDRV_PCM_RATE_CONTINUOUS,
+ .rate_min = 22050,
+ .rate_max = 192000,
+ .formats = TDA998X_FORMATS,
+ },
+ .ops = &tda_ops,
+ },
+};
+
+static const struct snd_soc_dapm_widget tda_widgets[] = {
+ SND_SOC_DAPM_OUTPUT("hdmi-out"),
+};
+static const struct snd_soc_dapm_route tda_routes[] = {
+ { "hdmi-out", NULL, "HDMI I2S Playback" },
+ { "hdmi-out", NULL, "HDMI SPDIF Playback" },
+};
+
+static int tda_probe(struct snd_soc_codec *codec)
+{
+ struct tda_priv *priv;
+ struct device_node *np;
+ int i, j, ret;
+ const char *p;
+
+ priv = devm_kzalloc(codec->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+ snd_soc_codec_set_drvdata(codec, priv);
+ priv->codec = codec;
+
+ /* get the audio input ports (I2s and S/PDIF) */
+ np = codec->dev->of_node;
+ for (i = 0; i < 2; i++) {
+ u32 port;
+
+ ret = of_property_read_u32_index(np, "audio-ports", i, &port);
+ if (ret) {
+ if (i == 0)
+ dev_err(codec->dev,
+ "bad or missing audio-ports\n");
+ break;
+ }
+ ret = of_property_read_string_index(np, "audio-port-names",
+ i, &p);
+ if (ret) {
+ dev_err(codec->dev,
+ "missing audio-port-names[%d]\n", i);
+ break;
+ }
+ if (strcmp(p, "i2s") == 0) {
+ j = 0;
+ } else if (strcmp(p, "spdif") == 0) {
+ j = 1;
+ } else {
+ dev_err(codec->dev,
+ "bad audio-port-names '%s'\n", p);
+ break;
+ }
+ priv->ports[j] = port;
+ }
+
+ return 0;
+}
+
+static const struct snd_soc_codec_driver soc_codec_tda998x = {
+ .probe = tda_probe,
+ .dapm_widgets = tda_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(tda_widgets),
+ .dapm_routes = tda_routes,
+ .num_dapm_routes = ARRAY_SIZE(tda_routes),
+};
+
+static int tda998x_dev_probe(struct platform_device *pdev)
+{
+ return snd_soc_register_codec(&pdev->dev,
+ &soc_codec_tda998x,
+ tda998x_dai, ARRAY_SIZE(tda998x_dai));
+}
+
+static int tda998x_dev_remove(struct platform_device *pdev)
+{
+ snd_soc_unregister_codec(&pdev->dev);
+ return 0;
+}
+
+static const struct of_device_id tda998x_codec_ids[] = {
+ { .compatible = "nxp,tda998x-codec", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, tda998x_codec_ids);
+
+static struct platform_driver tda998x_driver = {
+ .probe = tda998x_dev_probe,
+ .remove = tda998x_dev_remove,
+ .driver = {
+ .name = "tda998x-codec",
+ .owner = THIS_MODULE,
+ .of_match_table = tda998x_codec_ids,
+ },
+};
+
+module_platform_driver(tda998x_driver);
+
+MODULE_AUTHOR("Jean-Francois Moine");
+MODULE_DESCRIPTION("TDA998x codec driver");
+MODULE_LICENSE("GPL");
--
1.9.rc1
^ permalink raw reply related
* [PATCH 2/4] ASoC: tda998x: add a codec driver for TDA998x
From: Jean-Francois Moine @ 2014-01-26 18:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1390813480.git.moinejf@free.fr>
This patch adds a CODEC driver for the NXP TDA998x HDMI transmitter.
The CODEC handles both I2S and S/PDIF input and does dynamic input
switch in the TDA998x I2C driver on audio streaming start/stop.
Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
sound/soc/codecs/Kconfig | 7 ++
sound/soc/codecs/Makefile | 2 +
sound/soc/codecs/tda998x.c | 227 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 236 insertions(+)
create mode 100644 sound/soc/codecs/tda998x.c
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index b33b45d..7cec76e 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -71,6 +71,7 @@ config SND_SOC_ALL_CODECS
select SND_SOC_STA529 if I2C
select SND_SOC_STAC9766 if SND_SOC_AC97_BUS
select SND_SOC_TAS5086 if I2C
+ select SND_SOC_TDA998X if I2C
select SND_SOC_TLV320AIC23 if I2C
select SND_SOC_TLV320AIC26 if SPI_MASTER
select SND_SOC_TLV320AIC32X4 if I2C
@@ -352,6 +353,12 @@ config SND_SOC_STAC9766
config SND_SOC_TAS5086
tristate
+config SND_SOC_TDA998X
+ tristate
+ depends on OF
+ default y if DRM_I2C_NXP_TDA998X=y
+ default m if DRM_I2C_NXP_TDA998X=m
+
config SND_SOC_TLV320AIC23
tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index bc12676..a53d09e 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -62,6 +62,7 @@ snd-soc-sta32x-objs := sta32x.o
snd-soc-sta529-objs := sta529.o
snd-soc-stac9766-objs := stac9766.o
snd-soc-tas5086-objs := tas5086.o
+snd-soc-tda998x-objs := tda998x.o
snd-soc-tlv320aic23-objs := tlv320aic23.o
snd-soc-tlv320aic26-objs := tlv320aic26.o
snd-soc-tlv320aic3x-objs := tlv320aic3x.o
@@ -192,6 +193,7 @@ obj-$(CONFIG_SND_SOC_STA32X) += snd-soc-sta32x.o
obj-$(CONFIG_SND_SOC_STA529) += snd-soc-sta529.o
obj-$(CONFIG_SND_SOC_STAC9766) += snd-soc-stac9766.o
obj-$(CONFIG_SND_SOC_TAS5086) += snd-soc-tas5086.o
+obj-$(CONFIG_SND_SOC_TDA998X) += snd-soc-tda998x.o
obj-$(CONFIG_SND_SOC_TLV320AIC23) += snd-soc-tlv320aic23.o
obj-$(CONFIG_SND_SOC_TLV320AIC26) += snd-soc-tlv320aic26.o
obj-$(CONFIG_SND_SOC_TLV320AIC3X) += snd-soc-tlv320aic3x.o
diff --git a/sound/soc/codecs/tda998x.c b/sound/soc/codecs/tda998x.c
new file mode 100644
index 0000000..d724f7d
--- /dev/null
+++ b/sound/soc/codecs/tda998x.c
@@ -0,0 +1,227 @@
+/*
+ * ALSA SoC TDA998X driver
+ *
+ * This driver is used by the NXP TDA998x HDMI transmitter.
+ *
+ * Copyright (C) 2014 Jean-Francois Moine
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <sound/soc.h>
+#include <sound/pcm.h>
+#include <linux/of.h>
+#include <linux/i2c.h>
+#include <drm/drm_encoder_slave.h>
+#include <drm/i2c/tda998x.h>
+
+#define TDA998X_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
+ SNDRV_PCM_FMTBIT_S20_3LE | \
+ SNDRV_PCM_FMTBIT_S24_LE | \
+ SNDRV_PCM_FMTBIT_S32_LE)
+
+struct tda_priv {
+ struct i2c_client *i2c_client;
+ struct snd_soc_codec *codec;
+ u8 ports[2];
+ int dai_id;
+ u8 *eld;
+};
+
+static void tda_get_encoder(struct tda_priv *priv)
+{
+ struct snd_soc_codec *codec = priv->codec;
+ struct device_node *np;
+ struct i2c_client *i2c_client;
+ static const struct of_device_id tda_dt[] = {
+ { .compatible = "nxp,tda998x" },
+ { },
+ };
+
+ /* search the tda998x device */
+ np = of_find_matching_node_and_match(NULL, tda_dt, NULL);
+ if (!np || !of_device_is_available(np)) {
+ dev_err(codec->dev, "No tda998x in DT\n");
+ return;
+ }
+ i2c_client = of_find_i2c_device_by_node(np);
+ of_node_put(np);
+ if (!i2c_client) {
+ dev_err(codec->dev, "no tda998x i2c client\n");
+ return;
+ }
+ if (!i2c_get_clientdata(i2c_client)) {
+ dev_err(codec->dev, "tda998x not initialized\n");
+ return;
+ }
+
+ priv->i2c_client = i2c_client;
+}
+
+static int tda_start_stop(struct tda_priv *priv,
+ int start)
+{
+ int format, port;
+
+ if (!priv->i2c_client) {
+ tda_get_encoder(priv);
+ if (!priv->i2c_client)
+ return -EINVAL;
+ }
+
+ /* give the audio input type and ports to the HDMI encoder */
+ format = start ? priv->dai_id : 0;
+ switch (format) {
+ case AFMT_I2S:
+ port = priv->ports[0];
+ break;
+ default:
+ case AFMT_SPDIF:
+ port = priv->ports[1];
+ break;
+ }
+ tda998x_audio_update(priv->i2c_client, format, port);
+ return 0;
+}
+
+static int tda_startup(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct tda_priv *priv = snd_soc_codec_get_drvdata(dai->codec);
+
+ /* memorize the used DAI */
+ priv->dai_id = dai->id;
+
+ /* start the TDA998x audio */
+ return tda_start_stop(priv, 1);
+}
+
+static void tda_shutdown(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct tda_priv *priv = snd_soc_codec_get_drvdata(dai->codec);
+
+ priv->dai_id = 0;
+ tda_start_stop(priv, 0);
+}
+
+static const struct snd_soc_dai_ops tda_ops = {
+ .startup = tda_startup,
+ .shutdown = tda_shutdown,
+};
+
+static const struct snd_soc_dai_driver tda998x_dai[] = {
+ {
+ .name = "i2s-hifi",
+ .id = AFMT_I2S,
+ .playback = {
+ .stream_name = "HDMI I2S Playback",
+ .channels_min = 1,
+ .channels_max = 8,
+ .rates = SNDRV_PCM_RATE_CONTINUOUS,
+ .rate_min = 5512,
+ .rate_max = 192000,
+ .formats = TDA998X_FORMATS,
+
+ },
+ .ops = &tda_ops,
+ },
+ {
+ .name = "spdif-hifi",
+ .id = AFMT_SPDIF,
+ .playback = {
+ .stream_name = "HDMI SPDIF Playback",
+ .channels_min = 1,
+ .channels_max = 2,
+ .rates = SNDRV_PCM_RATE_CONTINUOUS,
+ .rate_min = 22050,
+ .rate_max = 192000,
+ .formats = TDA998X_FORMATS,
+ },
+ .ops = &tda_ops,
+ },
+};
+
+static const struct snd_soc_dapm_widget tda_widgets[] = {
+ SND_SOC_DAPM_OUTPUT("hdmi-out"),
+};
+static const struct snd_soc_dapm_route tda_routes[] = {
+ { "hdmi-out", NULL, "HDMI I2S Playback" },
+ { "hdmi-out", NULL, "HDMI SPDIF Playback" },
+};
+
+static int tda_probe(struct snd_soc_codec *codec)
+{
+ struct tda_priv *priv;
+ struct device_node *np;
+ int i, ret;
+
+ priv = devm_kzalloc(codec->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+ snd_soc_codec_set_drvdata(codec, priv);
+ priv->codec = codec;
+
+ /* get the audio input ports (I2s and S/PDIF) */
+ np = codec->dev->of_node;
+ for (i = 0; i < 2; i++) {
+ u32 port;
+
+ ret = of_property_read_u32_index(np, "audio-ports", i, &port);
+ if (ret) {
+ if (i == 0)
+ dev_err(codec->dev,
+ "bad or missing audio-ports\n");
+ break;
+ }
+ priv->ports[i] = port;
+ }
+
+ return 0;
+}
+
+static const struct snd_soc_codec_driver soc_codec_tda998x = {
+ .probe = tda_probe,
+ .dapm_widgets = tda_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(tda_widgets),
+ .dapm_routes = tda_routes,
+ .num_dapm_routes = ARRAY_SIZE(tda_routes),
+};
+
+static int tda998x_dev_probe(struct platform_device *pdev)
+{
+ return snd_soc_register_codec(&pdev->dev,
+ &soc_codec_tda998x,
+ tda998x_dai, ARRAY_SIZE(tda998x_dai));
+}
+
+static int tda998x_dev_remove(struct platform_device *pdev)
+{
+ snd_soc_unregister_codec(&pdev->dev);
+ return 0;
+}
+
+static const struct of_device_id tda998x_codec_ids[] = {
+ { .compatible = "nxp,tda998x-codec", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, tda998x_codec_ids);
+
+static struct platform_driver tda998x_driver = {
+ .probe = tda998x_dev_probe,
+ .remove = tda998x_dev_remove,
+ .driver = {
+ .name = "tda998x-codec",
+ .owner = THIS_MODULE,
+ .of_match_table = tda998x_codec_ids,
+ },
+};
+
+module_platform_driver(tda998x_driver);
+
+MODULE_AUTHOR("Jean-Francois Moine");
+MODULE_DESCRIPTION("TDA998x codec driver");
+MODULE_LICENSE("GPL");
--
1.8.5.3
^ permalink raw reply related
* [PATCH v3 1/5] drm/i2c: tda998x: add a function for dynamic audio input switch
From: Jean-Francois Moine @ 2014-01-26 18:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1391274627.git.moinejf@free.fr>
When both I2S and S/PDIF are wired from the audio device to the
TDA998x, the user or some internal mechanism may choose to do audio
streaming via either inputs.
This patch adds an exported function in the TDA998x driver which
initializes the audio input parameters according to the audio
subsystem.
The audio format values in the encoder configuration interface are
changed to non null values so that the value 0 is used in the audio
function to indicate that audio streaming is stopped.
As the audio clock depends on the input type, it is set so.
Then, the configuration value audio_clk_cfg is now ignored.
Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 48 ++++++++++++++++++++++++++++++++++++---
include/drm/i2c/tda998x.h | 7 ++++--
2 files changed, 50 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 2f97290..2643be4 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -35,6 +35,7 @@ struct tda998x_priv {
struct i2c_client *hdmi;
uint16_t rev;
uint8_t current_page;
+ u8 audio_active;
int dpms;
bool is_hdmi_sink;
u8 vip_cntrl_0;
@@ -640,12 +641,11 @@ static void
tda998x_configure_audio(struct tda998x_priv *priv,
struct drm_display_mode *mode, struct tda998x_encoder_params *p)
{
- uint8_t buf[6], clksel_aip, clksel_fs, cts_n, adiv;
+ uint8_t buf[6], clksel_aip, clksel_fs, cts_n, adiv, aclk;
uint32_t n;
/* Enable audio ports */
reg_write(priv, REG_ENA_AP, p->audio_cfg);
- reg_write(priv, REG_ENA_ACLK, p->audio_clk_cfg);
/* Set audio input source */
switch (p->audio_format) {
@@ -654,6 +654,7 @@ tda998x_configure_audio(struct tda998x_priv *priv,
clksel_aip = AIP_CLKSEL_AIP_SPDIF;
clksel_fs = AIP_CLKSEL_FS_FS64SPDIF;
cts_n = CTS_N_M(3) | CTS_N_K(3);
+ aclk = 0; /* no clock */
break;
case AFMT_I2S:
@@ -661,6 +662,7 @@ tda998x_configure_audio(struct tda998x_priv *priv,
clksel_aip = AIP_CLKSEL_AIP_I2S;
clksel_fs = AIP_CLKSEL_FS_ACLK;
cts_n = CTS_N_M(3) | CTS_N_K(3);
+ aclk = 1; /* clock enable */
break;
default:
@@ -672,6 +674,7 @@ tda998x_configure_audio(struct tda998x_priv *priv,
reg_clear(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_LAYOUT |
AIP_CNTRL_0_ACR_MAN); /* auto CTS */
reg_write(priv, REG_CTS_N, cts_n);
+ reg_write(priv, REG_ENA_ACLK, aclk);
/*
* Audio input somehow depends on HDMI line rate which is
@@ -728,6 +731,37 @@ tda998x_configure_audio(struct tda998x_priv *priv,
tda998x_write_aif(priv, p);
}
+/* tda998x codec interface */
+void tda998x_audio_update(struct i2c_client *client,
+ int format,
+ int port)
+{
+ struct tda998x_priv *priv = i2c_get_clientdata(client);
+ struct tda998x_encoder_params *p = &priv->params;
+
+ /* if the audio output is active, it may be a second start or a stop */
+ if (format == 0 || priv->audio_active) {
+ if (format == 0) {
+ priv->audio_active = 0;
+ reg_write(priv, REG_ENA_AP, 0);
+ }
+ return;
+ }
+ priv->audio_active = 1;
+
+ p->audio_cfg = port;
+
+ /* don't restart audio if same input format */
+ if (format == p->audio_format) {
+ reg_write(priv, REG_ENA_AP, p->audio_cfg);
+ return;
+ }
+ p->audio_format = format;
+
+ tda998x_configure_audio(priv, &priv->encoder->crtc->hwmode, p);
+}
+EXPORT_SYMBOL_GPL(tda998x_audio_update);
+
/* DRM encoder functions */
static void
@@ -750,6 +784,9 @@ tda998x_encoder_set_config(struct drm_encoder *encoder, void *params)
(p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
priv->params = *p;
+
+ if (p->audio_cfg)
+ priv->audio_active = 1;
}
static void
@@ -999,7 +1036,7 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
tda998x_write_avi(priv, adjusted_mode);
- if (priv->params.audio_cfg)
+ if (priv->audio_active)
tda998x_configure_audio(priv, adjusted_mode,
&priv->params);
}
@@ -1239,10 +1276,15 @@ tda998x_encoder_init(struct i2c_client *client,
if (!priv)
return -ENOMEM;
+ i2c_set_clientdata(client, priv);
+
priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(2) | VIP_CNTRL_0_SWAP_B(3);
priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(0) | VIP_CNTRL_1_SWAP_D(1);
priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(4) | VIP_CNTRL_2_SWAP_F(5);
+ priv->params.audio_frame[1] = 1; /* channels - 1 */
+ priv->params.audio_sample_rate = 48000; /* 48kHz */
+
priv->current_page = 0xff;
priv->hdmi = client;
priv->cec = i2c_new_dummy(client->adapter, 0x34);
diff --git a/include/drm/i2c/tda998x.h b/include/drm/i2c/tda998x.h
index 3e419d9..7e4806d 100644
--- a/include/drm/i2c/tda998x.h
+++ b/include/drm/i2c/tda998x.h
@@ -20,11 +20,14 @@ struct tda998x_encoder_params {
u8 audio_frame[6];
enum {
- AFMT_SPDIF,
- AFMT_I2S
+ AFMT_I2S = 1,
+ AFMT_SPDIF
} audio_format;
unsigned audio_sample_rate;
};
+void tda998x_audio_update(struct i2c_client *client,
+ int format,
+ int port);
#endif
--
1.9.rc1
^ permalink raw reply related
* [PATCH v2 1/5] drm/i2c: tda998x: add a function for dynamic audio input switch
From: Jean-Francois Moine @ 2014-01-26 18:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1391081933.git.moinejf@free.fr>
When both I2S and S/PDIF are wired from the audio device to the
TDA998x, the user or some internal mechanism may choose to do audio
streaming via either inputs.
This patch adds an exported function in the TDA998x driver which
initializes the audio input parameters according to the audio
subsystem.
The audio format values in the encoder configuration interface are
changed to non null values so that the value 0 is used in the audio
function to indicate that audio streaming is stopped.
As the audio clock depends on the input type, it is set so.
Then, the configuration value audio_clk_cfg is now ignored.
Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 48 ++++++++++++++++++++++++++++++++++++---
include/drm/i2c/tda998x.h | 7 ++++--
2 files changed, 50 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 2f97290..2643be4 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -35,6 +35,7 @@ struct tda998x_priv {
struct i2c_client *hdmi;
uint16_t rev;
uint8_t current_page;
+ u8 audio_active;
int dpms;
bool is_hdmi_sink;
u8 vip_cntrl_0;
@@ -640,12 +641,11 @@ static void
tda998x_configure_audio(struct tda998x_priv *priv,
struct drm_display_mode *mode, struct tda998x_encoder_params *p)
{
- uint8_t buf[6], clksel_aip, clksel_fs, cts_n, adiv;
+ uint8_t buf[6], clksel_aip, clksel_fs, cts_n, adiv, aclk;
uint32_t n;
/* Enable audio ports */
reg_write(priv, REG_ENA_AP, p->audio_cfg);
- reg_write(priv, REG_ENA_ACLK, p->audio_clk_cfg);
/* Set audio input source */
switch (p->audio_format) {
@@ -654,6 +654,7 @@ tda998x_configure_audio(struct tda998x_priv *priv,
clksel_aip = AIP_CLKSEL_AIP_SPDIF;
clksel_fs = AIP_CLKSEL_FS_FS64SPDIF;
cts_n = CTS_N_M(3) | CTS_N_K(3);
+ aclk = 0; /* no clock */
break;
case AFMT_I2S:
@@ -661,6 +662,7 @@ tda998x_configure_audio(struct tda998x_priv *priv,
clksel_aip = AIP_CLKSEL_AIP_I2S;
clksel_fs = AIP_CLKSEL_FS_ACLK;
cts_n = CTS_N_M(3) | CTS_N_K(3);
+ aclk = 1; /* clock enable */
break;
default:
@@ -672,6 +674,7 @@ tda998x_configure_audio(struct tda998x_priv *priv,
reg_clear(priv, REG_AIP_CNTRL_0, AIP_CNTRL_0_LAYOUT |
AIP_CNTRL_0_ACR_MAN); /* auto CTS */
reg_write(priv, REG_CTS_N, cts_n);
+ reg_write(priv, REG_ENA_ACLK, aclk);
/*
* Audio input somehow depends on HDMI line rate which is
@@ -728,6 +731,37 @@ tda998x_configure_audio(struct tda998x_priv *priv,
tda998x_write_aif(priv, p);
}
+/* tda998x codec interface */
+void tda998x_audio_update(struct i2c_client *client,
+ int format,
+ int port)
+{
+ struct tda998x_priv *priv = i2c_get_clientdata(client);
+ struct tda998x_encoder_params *p = &priv->params;
+
+ /* if the audio output is active, it may be a second start or a stop */
+ if (format == 0 || priv->audio_active) {
+ if (format == 0) {
+ priv->audio_active = 0;
+ reg_write(priv, REG_ENA_AP, 0);
+ }
+ return;
+ }
+ priv->audio_active = 1;
+
+ p->audio_cfg = port;
+
+ /* don't restart audio if same input format */
+ if (format == p->audio_format) {
+ reg_write(priv, REG_ENA_AP, p->audio_cfg);
+ return;
+ }
+ p->audio_format = format;
+
+ tda998x_configure_audio(priv, &priv->encoder->crtc->hwmode, p);
+}
+EXPORT_SYMBOL_GPL(tda998x_audio_update);
+
/* DRM encoder functions */
static void
@@ -750,6 +784,9 @@ tda998x_encoder_set_config(struct drm_encoder *encoder, void *params)
(p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
priv->params = *p;
+
+ if (p->audio_cfg)
+ priv->audio_active = 1;
}
static void
@@ -999,7 +1036,7 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
tda998x_write_avi(priv, adjusted_mode);
- if (priv->params.audio_cfg)
+ if (priv->audio_active)
tda998x_configure_audio(priv, adjusted_mode,
&priv->params);
}
@@ -1239,10 +1276,15 @@ tda998x_encoder_init(struct i2c_client *client,
if (!priv)
return -ENOMEM;
+ i2c_set_clientdata(client, priv);
+
priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(2) | VIP_CNTRL_0_SWAP_B(3);
priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(0) | VIP_CNTRL_1_SWAP_D(1);
priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(4) | VIP_CNTRL_2_SWAP_F(5);
+ priv->params.audio_frame[1] = 1; /* channels - 1 */
+ priv->params.audio_sample_rate = 48000; /* 48kHz */
+
priv->current_page = 0xff;
priv->hdmi = client;
priv->cec = i2c_new_dummy(client->adapter, 0x34);
diff --git a/include/drm/i2c/tda998x.h b/include/drm/i2c/tda998x.h
index 3e419d9..7e4806d 100644
--- a/include/drm/i2c/tda998x.h
+++ b/include/drm/i2c/tda998x.h
@@ -20,11 +20,14 @@ struct tda998x_encoder_params {
u8 audio_frame[6];
enum {
- AFMT_SPDIF,
- AFMT_I2S
+ AFMT_I2S = 1,
+ AFMT_SPDIF
} audio_format;
unsigned audio_sample_rate;
};
+void tda998x_audio_update(struct i2c_client *client,
+ int format,
+ int port);
#endif
--
1.9.rc1
^ permalink raw reply related
* [PATCH 1/4] drm/i2c: tda998x: add a function for dynamic audio input switch
From: Jean-Francois Moine @ 2014-01-26 18:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1390813480.git.moinejf@free.fr>
When both I2S and S/PDIF are wired from the audio device to the
TDA998x, the user or some internal mechanism may choose to do audio
streaming via either inputs.
This patch adds an exported function in the TDA998x driver which
initializes the audio input parameters according to the audio
subsystem.
The audio format values in the encoder configuration interface are
changed to non null values so that the value 0 is used in the audio
function to indicate that audio streaming is stopped.
Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
drivers/gpu/drm/i2c/tda998x_drv.c | 43 ++++++++++++++++++++++++++++++++++++++-
include/drm/i2c/tda998x.h | 7 +++++--
2 files changed, 47 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index e5bbaf2..186c751 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -34,6 +34,7 @@ struct tda998x_priv {
struct i2c_client *hdmi;
uint16_t rev;
uint8_t current_page;
+ u8 audio_active;
int dpms;
bool is_hdmi_sink;
u8 vip_cntrl_0;
@@ -729,6 +730,38 @@ tda998x_configure_audio(struct tda998x_priv *priv,
tda998x_write_aif(priv, p);
}
+/* tda998x codec interface */
+void tda998x_audio_update(struct i2c_client *client,
+ int format,
+ int port)
+{
+ struct tda998x_priv *priv = i2c_get_clientdata(client);
+ struct tda998x_encoder_params *p = &priv->params;
+
+ /* if the audio output is active, it may be a second start or a stop */
+ if (format == 0 || priv->audio_active) {
+ if (format == 0) {
+ priv->audio_active = 0;
+ reg_write(priv, REG_ENA_AP, 0);
+ }
+ return;
+ }
+
+ p->audio_cfg = port;
+
+ /* don't restart audio if same input format */
+ if (format == p->audio_format) {
+ priv->audio_active = 1;
+ reg_write(priv, REG_ENA_AP, p->audio_cfg);
+ return;
+ }
+ p->audio_format = format;
+ priv->audio_active = 1;
+
+ tda998x_configure_audio(priv, &priv->encoder->crtc->hwmode, p);
+}
+EXPORT_SYMBOL_GPL(tda998x_audio_update);
+
/* DRM encoder functions */
static void
@@ -751,6 +784,9 @@ tda998x_encoder_set_config(struct drm_encoder *encoder, void *params)
(p->mirr_f ? VIP_CNTRL_2_MIRR_F : 0);
priv->params = *p;
+
+ if (p->audio_cfg)
+ priv->audio_active = 1;
}
static void
@@ -1001,7 +1037,7 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder,
tda998x_write_avi(priv, adj_mode);
- if (priv->params.audio_cfg)
+ if (priv->audio_active)
tda998x_configure_audio(priv, adj_mode, &priv->params);
}
}
@@ -1233,10 +1269,15 @@ tda998x_encoder_init(struct i2c_client *client,
if (!priv)
return -ENOMEM;
+ i2c_set_clientdata(client, priv);
+
priv->vip_cntrl_0 = VIP_CNTRL_0_SWAP_A(2) | VIP_CNTRL_0_SWAP_B(3);
priv->vip_cntrl_1 = VIP_CNTRL_1_SWAP_C(0) | VIP_CNTRL_1_SWAP_D(1);
priv->vip_cntrl_2 = VIP_CNTRL_2_SWAP_E(4) | VIP_CNTRL_2_SWAP_F(5);
+ priv->params.audio_frame[1] = 1; /* channels - 1 */
+ priv->params.audio_sample_rate = 48000; /* 48kHz */
+
priv->current_page = 0xff;
priv->hdmi = client;
priv->cec = i2c_new_dummy(client->adapter, 0x34);
diff --git a/include/drm/i2c/tda998x.h b/include/drm/i2c/tda998x.h
index f3bb25c..0459931 100644
--- a/include/drm/i2c/tda998x.h
+++ b/include/drm/i2c/tda998x.h
@@ -19,11 +19,14 @@ struct tda998x_encoder_params {
u8 audio_frame[6];
enum {
- AFMT_SPDIF,
- AFMT_I2S
+ AFMT_I2S = 1,
+ AFMT_SPDIF
} audio_format;
unsigned audio_sample_rate;
};
+void tda998x_audio_update(struct i2c_client *client,
+ int format,
+ int port);
#endif
--
1.8.5.3
^ permalink raw reply related
* [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Lars-Peter Clausen @ 2014-01-26 17:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140126142436.GF10628@intel.com>
On 01/26/2014 03:24 PM, Vinod Koul wrote:
> On Wed, Jan 22, 2014 at 10:22:45PM +0530, Srikanth Thokala wrote:
>> This is the driver for the AXI Video Direct Memory Access (AXI
>> VDMA) core, which is a soft Xilinx IP core that provides high-
>> bandwidth direct memory access between memory and AXI4-Stream
>> type video target peripherals. The core provides efficient two
>> dimensional DMA operations with independent asynchronous read
> ok here is tha catch, do you want to support interleaved API rather?
>
>> +* DMA client + +Required properties: +- dmas: a list of <[Video DMA device
>> phandle] [Channel ID]> pairs, + where Channel ID is '0' for write/tx and
>> '1' for read/rx + channel. +- dma-names: a list of DMA channel names, one
>> per "dmas" entry + +Example: +++++++++ + +vdmatest_0: vdmatest at 0 { +
>> compatible ="xlnx,axi-vdma-test-1.00.a"; + dmas = <&axi_vdma_0 0 +
>> &axi_vdma_0 1>; + dma-names = "vdma0", "vdma1"; +} ;
> Need ack from DT folks. ALso would be better to split the binding to a separate
> patch
>
>
>> +/**
>> + * struct xilinx_vdma_chan - Driver specific VDMA channel structure
>> + * @xdev: Driver specific device structure
>> + * @ctrl_offset: Control registers offset
>> + * @desc_offset: TX descriptor registers offset
>> + * @completed_cookie: Maximum cookie completed
>> + * @cookie: The current cookie
>> + * @lock: Descriptor operation lock
>> + * @pending_list: Descriptors waiting
>> + * @active_desc: Active descriptor
>> + * @done_list: Complete descriptors
>> + * @common: DMA common channel
>> + * @desc_pool: Descriptors pool
>> + * @dev: The dma device
>> + * @irq: Channel IRQ
>> + * @id: Channel ID
>> + * @direction: Transfer direction
>> + * @num_frms: Number of frames
>> + * @has_sg: Support scatter transfers
>> + * @genlock: Support genlock mode
>> + * @err: Channel has errors
>> + * @tasklet: Cleanup work after irq
>> + * @config: Device configuration info
>> + * @flush_on_fsync: Flush on Frame sync
>> + */
>> +struct xilinx_vdma_chan {
>> + struct xilinx_vdma_device *xdev;
>> + u32 ctrl_offset;
>> + u32 desc_offset;
>> + dma_cookie_t completed_cookie;
>> + dma_cookie_t cookie;
>> + spinlock_t lock;
>> + struct list_head pending_list;
>> + struct xilinx_vdma_tx_descriptor *active_desc;
>> + struct list_head done_list;
>> + struct dma_chan common;
>> + struct dma_pool *desc_pool;
>> + struct device *dev;
>> + int irq;
>> + int id;
>> + enum dma_transfer_direction direction;
> why should channel have a direction... descriptor should have direction and not
> the channel!
The channel only supports transfers in one direction. Either from memory to
peripheral or from peripheral to memory, that's fixed and can't be changed
at runtime. The driver needs to know which direction the channel supports so
it can reject transfers with the wrong direction.
[...]
>> +
>
>> + * xilinx_vdma_tx_status - Get VDMA transaction status
>> + * @dchan: DMA channel
>> + * @cookie: Transaction identifier
>> + * @txstate: Transaction state
>> + *
>> + * Return: DMA transaction status
>> + */
>> +static enum dma_status xilinx_vdma_tx_status(struct dma_chan *dchan,
>> + dma_cookie_t cookie,
>> + struct dma_tx_state *txstate)
>> +{
>> + struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan);
>> + dma_cookie_t last_used;
>> + dma_cookie_t last_complete;
>> +
>> + xilinx_vdma_chan_desc_cleanup(chan);
>> +
>> + last_used = dchan->cookie;
>> + last_complete = chan->completed_cookie;
>> +
>> + dma_set_tx_state(txstate, last_complete, last_used, 0);
>> +
>> + return dma_async_is_complete(cookie, last_complete, last_used);
> no residue calculation?
>
The hardware doesn't support that.
>> +/**
>> + * xilinx_vdma_prep_slave_sg - prepare a descriptor for a DMA_SLAVE transaction
>> + * @dchan: DMA channel
>> + * @sgl: scatterlist to transfer to/from
>> + * @sg_len: number of entries in @sgl
>> + * @dir: DMA direction
>> + * @flags: transfer ack flags
>> + * @context: unused
>> + *
>> + * Return: Async transaction descriptor on success and NULL on failure
>> + */
>> +static struct dma_async_tx_descriptor *
>> +xilinx_vdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
>> + unsigned int sg_len, enum dma_transfer_direction dir,
>> + unsigned long flags, void *context)
> okay now am worried, this is supposed to memcpy DMA so why slave-sg??
The DMA is either from memory to peripheral or from peripheral to memory
depending on the direction. So slave sg should be fine.
>
> Looking at the driver overall, IMHO we need to do:
> - use the virt-dma to simplfy the cookie handling and perhpasn the descrptors
> too!
> - Perhpas use interleaved API..?
> - I dont think we should use the slave API as this seems memcpy case!
>
^ permalink raw reply
* [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Lars-Peter Clausen @ 2014-01-26 17:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140126140318.GE10628@intel.com>
On 01/26/2014 03:03 PM, Vinod Koul wrote:
> On Thu, Jan 23, 2014 at 03:07:32PM +0100, Lars-Peter Clausen wrote:
>> On 01/23/2014 03:00 PM, Andy Shevchenko wrote:
>>> On Thu, 2014-01-23 at 14:50 +0100, Lars-Peter Clausen wrote:
>>>> On 01/23/2014 02:38 PM, Shevchenko, Andriy wrote:
>>>>> On Thu, 2014-01-23 at 12:25 +0100, Lars-Peter Clausen wrote:
>>>>>> On 01/22/2014 05:52 PM, Srikanth Thokala wrote:
>>>>>
>>>>> [...]
>>>>>
>>>>>>> + /* Request the interrupt */
>>>>>>> + chan->irq = irq_of_parse_and_map(node, 0);
>>>>>>> + err = devm_request_irq(xdev->dev, chan->irq, xilinx_vdma_irq_handler,
>>>>>>> + IRQF_SHARED, "xilinx-vdma-controller", chan);
>>>>>>
>>>>>> This is a clasic example of where to not use devm_request_irq. 'chan' is
>>>>>> accessed in the interrupt handler, but if you use devm_request_irq 'chan'
>>>>>> will be freed before the interrupt handler has been released, which means
>>>>>> there is now a race condition where the interrupt handler can access already
>>>>>> freed memory.ta
>>>>>
>>>>> Could you elaborate this case? As far as I understood managed resources
>>>>> are a kind of stack pile. In this case you have no such condition. Where
>>>>> am I wrong?
>>>>
>>>> The stacked stuff is only ran after the remove() function. Which means that
>>>> you call dma_async_device_unregister() before the interrupt handler is
>>>> freed. Another issue with the interrupt handler is a bit hidden. The driver
>>>> does not call tasklet_kill in the remove function. Which it should though to
>>>> make sure that the tasklet does not race against the freeing of the memory.
>>>> And in order to make sure that the tasklet is not rescheduled you need to
>>>> free the irq before killing the tasklet, since the interrupt handler
>>>> schedules the tasklet.
>>>
>>> So, you mean devm_request_irq() will race in any DMA driver?
>>
>> Most likely yes. devm_request_irq() is race condition prone for the majority
>> of device driver. You have to be really careful if you want to use it.
>>
>>>
>>> I think the proper solution is to disable all device work in
>>> the .remove() and devm will care about resources.
>>
>> As long as the interrupt handler is registered it can be called, the only
>> proper solution is to make sure that the order in which resources are torn
>> down is correct.
> Wouldn't it work if we register the irq last in the probe. That wil ensure on
> success the channel is always valid.
Yes, but only if the irq is not device managed. All device managed resources
will be freed after the remove function has been called. Which is to late in
our case since we make sure that the tasklet is not running in the remove
function.
- Lars
^ permalink raw reply
* [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Lars-Peter Clausen @ 2014-01-26 17:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140126135933.GD10628@intel.com>
On 01/26/2014 02:59 PM, Vinod Koul wrote:
> On Fri, Jan 24, 2014 at 02:24:27PM +0100, Lars-Peter Clausen wrote:
>> On 01/24/2014 12:16 PM, Srikanth Thokala wrote:
>>> Hi Lars,
>>>
>>> On Thu, Jan 23, 2014 at 4:55 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>>>> On 01/22/2014 05:52 PM, Srikanth Thokala wrote:
>>>> [...]
>>>>> +/**
>>>>> + * xilinx_vdma_device_control - Configure DMA channel of the device
>>>>> + * @dchan: DMA Channel pointer
>>>>> + * @cmd: DMA control command
>>>>> + * @arg: Channel configuration
>>>>> + *
>>>>> + * Return: '0' on success and failure value on error
>>>>> + */
>>>>> +static int xilinx_vdma_device_control(struct dma_chan *dchan,
>>>>> + enum dma_ctrl_cmd cmd, unsigned long arg)
>>>>> +{
>>>>> + struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan);
>>>>> +
>>>>> + switch (cmd) {
>>>>> + case DMA_TERMINATE_ALL:
>>>>> + xilinx_vdma_terminate_all(chan);
>>>>> + return 0;
>>>>> + case DMA_SLAVE_CONFIG:
>>>>> + return xilinx_vdma_slave_config(chan,
>>>>> + (struct xilinx_vdma_config *)arg);
>>>>
>>>> You really shouldn't be overloading the generic API with your own semantics.
>>>> DMA_SLAVE_CONFIG should take a dma_slave_config and nothing else.
>>>
>>> Ok. The driver needs few additional configuration from the slave
>>> device like Vertical
>>> Size, Horizontal Size, Stride etc., for the DMA transfers, in that case do you
>>> suggest me to define a separate dma_ctrl_cmd like the one FSLDMA_EXTERNAL_START
>>> defined for Freescale drivers?
>>
>> In my opinion it is not a good idea to have driver implement a generic API,
>> but at the same time let the driver have custom semantics for those API
>> calls. It's a bit like having a gpio driver that expects 23 and 42 as the
>> values passed to gpio_set_value instead of 0 and 1. It completely defeats
>> the purpose of a generic API, namely that you are able to write generic code
>> that makes use of the API without having to know about which implementation
>> API it is talking to. The dmaengine framework provides the
>> dmaengine_prep_interleaved_dma() function to setup two dimensional
>> transfers, e.g. take a look at sirf-dma.c or imx-dma.c.
>
> The question here i think would be waht this device supports? Is the hardware
> capable of doing interleaved transfers, then would make sense.
The hardware does 2D transfers. The parameters for a transfer are height,
width and stride. That's only a subset of what interleaved transfers can be
(xt->num_frames must be one for 2d transfers). But if I remember correctly
there has been some discussion on this in the past and the result of that
discussion was that using interleaved transfers for 2D transfers is
preferred over adding a custom API for 2D transfers.
- Lars
^ permalink raw reply
* [PATCH 1/3] mmc: add support for power-on sequencing through DT
From: Tomasz Figa @ 2014-01-26 17:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52DEBDA7.7030200@samsung.com>
On 21.01.2014 19:34, Tomasz Figa wrote:
> Hi,
>
> On 20.01.2014 04:56, Olof Johansson wrote:
>> This patch enables support for power-on sequencing of SDIO peripherals
>> through DT.
>>
>> In general, it's quite common that wifi modules and other similar
>> peripherals have several signals in addition to the SDIO interface that
>> needs wiggling before the module will power on. It's common to have a
>> reference clock, one or several power rails and one or several lines
>> for reset/enable type functions.
>>
>> The binding as written today introduces a number of reset gpios,
>> a regulator and a clock specifier. The code will handle up to 2 gpio
>> reset lines, but it's trivial to increase to more than that if needed
>> at some point.
>>
>> Implementation-wise, the MMC core has been changed to handle this during
>> host power up, before the host interface is powered on. I have not yet
>> implemented the power-down side, I wanted people to have a chance for
>> reporting back w.r.t. issues (or comments on the bindings) first.
>>
>> I have not tested the regulator portion, since the system and module
>> I'm working on doesn't need one (Samsung Chromebook with Marvell
>> 8797-based wifi). Testing of those portions (and reporting back) would
>> be appreciated.
>
> While I fully agree that this is an important problem that needs to be
> solved, I really don't think this is the right way, because:
>
> a) power-up sequence is really specific to the MMC device and often it's
> not simply a matter of switching on one regulator or one clock, e.g.
> specific time constraints need to be met.
>
> b) you can have WLAN chips in which SDIO is just one of the options to
> use as host interface, which may be also HSIC, I2C or UART. Really. See
> [1].
>
> c) this is leaking device specific details to generic host code, which
> isn't really elegant.
>
> Now, to make this a bit more constructive, [2] is a solution that I came
> up with (not perfect either), which simply adds a separate platform
> device for the low level part of the chip. I believe this is a better
> solution because:
>
> a) you can often see such WLAN/BT combo chip as a set of separate
> devices, e.g. SDIO WLAN, UART BT and a simple PMIC or management IC,
> which provides power/reset control, out of band signalling and etc. for
> the first two, so it isn't that bad to have a separate device node for
> the last one,
>
> b) you have full freedom of defining your DT binding with whatever data
> you need, any number of clocks, regulators, GPIOs and even out of band
> interrupts (IMHO the most important one).
>
> c) you can implement power-on, power-off sequences as needed for your
> particular device,
>
> d) you have full separation of device-specific data from MMC core (or
> any other subsystem simply used as a way to perform I/O to the chip).
>
> Now what's missing there is a way to signal the MMC core or any other
> transport that a device showed up and the controller should be woken up
> out of standby and scan of the bus initialized. This could be done by
> explicitly specifying the device as a subnode of the
> MMC/UART/USB(HSIC)/I2C or whatever with a link (phandle) to the power
> controller of the chip or the other way around - a link to the
> MMC/UART/... controller from the power controller node.
I've looked a bit around MMC core code and got some basic idea how
things look. I will definitely need some guidance, or at least some
opinions, from MMC guys, as some MMC core changes are unavoidable.
Now, the device-specific code is not really an issue, existing drivers
usually already have their ways of powering the chips on and off, based
on platform data. Everything needed here is to retrieve needed resources
(GPIOs, clocks, regulators) using DT, which should be trivial.
The worse part is the interaction between MMC and power controller
driver (the platform driver part of WLAN driver, if you look at brcmfmac
as an example). I believe that we need following things:
a) A way to tell the MMC controller that there is no card detection
mechanism available on given slot and it also should not be polling the
slot to check card presence. Something like a "manual card detect" that
would be triggered by another kernel entity that controls whether the
MMC device is present (i.e. WLAN driver). We already have "broken-cd"
property, but it only implies the former, wasting time on needless polling.
b) A mechanism to bind the power controller to used MMC slot. Something
like "mmc-bus = <&mmc2>;" property in device node of the power
controller and a function like of_find_mmc_controller_by_node(), which
would be an MMC counterpart of I2C's of_find_i2c_adapter_by_node(). To
avoid races, it should probably take a reference on MMC host that would
have to be dropped explicitly whenever it is not needed anymore.
c) A method to notify the MMC subsystem that card presence has changed.
We already have something like this in drivers/mmc/core/slot-gpio.c, but
used for a simple GPIO-based card detection. If the main part of
mmc_gpio_cd_irqt() could be turned into an exported helper, e.g.
mmc_force_card_detect(host) then basically we would have everything needed.
Unfortunately, I don't have more time left for today to create patches
and test them, so for now, I'd like to hear opinion of MMC maintainers
about this approach. Do you find this acceptable?
By the way, it seems like slot-gpio.c could replace a lot of custom GPIO
card detection code used in MMC host drivers, e.g. sdhci-s3c. Is there
any reason why it couldn't?
Best regards,
Tomasz
^ permalink raw reply
* [PATCH v4 07/18] watchdog: orion: Handle IRQ
From: Thomas Petazzoni @ 2014-01-26 14:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140126131445.GA14713@localhost>
Dear Ezequiel Garcia,
On Sun, 26 Jan 2014 10:14:46 -0300, Ezequiel Garcia wrote:
> > I don't quiite understand the first sentence of this commit log, and
> > the commit title looks wrong. Maybe a bad copy/paste or something?
> >
>
> Hm... yes it doesn't look right. It should read:
>
> "DT-enabled platforms, where the irqchip driver for the brigde interrupt
> controller is available, can handle the watchdog IRQ properly. Therefore,
> we request the interrupt and add a dummy handler that merely calls panic()".
Ok.
> I guess we can re-phrase it be a bit more readable.
>
> Why does th commit title looks wrong? By requesting the IRQ we are
> "handling it", no?
Right, but it looks "truncated". Maybe something like:
watchdog: orion: handle irq to avoid having to clear BRIDGE_CLAUSE
or something like that (adjust to the actual reality, I haven't
followed all the implications).
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Vinod Koul @ 2014-01-26 14:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390409565-4200-2-git-send-email-sthokal@xilinx.com>
On Wed, Jan 22, 2014 at 10:22:45PM +0530, Srikanth Thokala wrote:
> This is the driver for the AXI Video Direct Memory Access (AXI
> VDMA) core, which is a soft Xilinx IP core that provides high-
> bandwidth direct memory access between memory and AXI4-Stream
> type video target peripherals. The core provides efficient two
> dimensional DMA operations with independent asynchronous read
ok here is tha catch, do you want to support interleaved API rather?
> +* DMA client + +Required properties: +- dmas: a list of <[Video DMA device
> phandle] [Channel ID]> pairs, + where Channel ID is '0' for write/tx and
> '1' for read/rx + channel. +- dma-names: a list of DMA channel names, one
> per "dmas" entry + +Example: +++++++++ + +vdmatest_0: vdmatest at 0 { +
> compatible ="xlnx,axi-vdma-test-1.00.a"; + dmas = <&axi_vdma_0 0 +
> &axi_vdma_0 1>; + dma-names = "vdma0", "vdma1"; +} ;
Need ack from DT folks. ALso would be better to split the binding to a separate
patch
> +/**
> + * struct xilinx_vdma_chan - Driver specific VDMA channel structure
> + * @xdev: Driver specific device structure
> + * @ctrl_offset: Control registers offset
> + * @desc_offset: TX descriptor registers offset
> + * @completed_cookie: Maximum cookie completed
> + * @cookie: The current cookie
> + * @lock: Descriptor operation lock
> + * @pending_list: Descriptors waiting
> + * @active_desc: Active descriptor
> + * @done_list: Complete descriptors
> + * @common: DMA common channel
> + * @desc_pool: Descriptors pool
> + * @dev: The dma device
> + * @irq: Channel IRQ
> + * @id: Channel ID
> + * @direction: Transfer direction
> + * @num_frms: Number of frames
> + * @has_sg: Support scatter transfers
> + * @genlock: Support genlock mode
> + * @err: Channel has errors
> + * @tasklet: Cleanup work after irq
> + * @config: Device configuration info
> + * @flush_on_fsync: Flush on Frame sync
> + */
> +struct xilinx_vdma_chan {
> + struct xilinx_vdma_device *xdev;
> + u32 ctrl_offset;
> + u32 desc_offset;
> + dma_cookie_t completed_cookie;
> + dma_cookie_t cookie;
> + spinlock_t lock;
> + struct list_head pending_list;
> + struct xilinx_vdma_tx_descriptor *active_desc;
> + struct list_head done_list;
> + struct dma_chan common;
> + struct dma_pool *desc_pool;
> + struct device *dev;
> + int irq;
> + int id;
> + enum dma_transfer_direction direction;
why should channel have a direction... descriptor should have direction and not
the channel!
> +/**
> + * xilinx_vdma_free_tx_descriptor - Free transaction descriptor
> + * @chan: Driver specific VDMA channel
> + * @desc: VDMA transaction descriptor
> + */
> +static void
> +xilinx_vdma_free_tx_descriptor(struct xilinx_vdma_chan *chan,
> + struct xilinx_vdma_tx_descriptor *desc)
> +{
> + struct xilinx_vdma_tx_segment *segment, *next;
> +
> + if (!desc)
> + return;
> +
> + list_for_each_entry_safe(segment, next, &desc->segments, node) {
do you want to use _safe. Isee that this is called for cleanup while lock held,
and in other case within another _safe iterator!
> + list_del(&segment->node);
> + xilinx_vdma_free_tx_segment(chan, segment);
> + }
> +
> + kfree(desc);
> +}
> +
> +/* Required functions */
> +
> + * xilinx_vdma_do_tasklet - Schedule completion tasklet
> + * @data: Pointer to the Xilinx VDMA channel structure
> + */
> +static void xilinx_vdma_do_tasklet(unsigned long data)
> +{
> + struct xilinx_vdma_chan *chan = (struct xilinx_vdma_chan *)data;
> +
> + xilinx_vdma_chan_desc_cleanup(chan);
> +}
> +
> +/**
> + * xilinx_vdma_alloc_chan_resources - Allocate channel resources
> + * @dchan: DMA channel
> + *
> + * Return: '1' on success and failure value on error
naaah, we dont do that, pls use standard notation of 0 on success
Also API wants you to return descriptors allocated here!
> + */
> +static int xilinx_vdma_alloc_chan_resources(struct dma_chan *dchan)
> +{
> + struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan);
> +
> + /* Has this channel already been allocated? */
> + if (chan->desc_pool)
> + return 1;
> +
> + /*
> + * We need the descriptor to be aligned to 64bytes
> + * for meeting Xilinx VDMA specification requirement.
> + */
> + chan->desc_pool = dma_pool_create("xilinx_vdma_desc_pool",
> + chan->dev,
> + sizeof(struct xilinx_vdma_tx_segment),
> + __alignof__(struct xilinx_vdma_tx_segment), 0);
> + if (!chan->desc_pool) {
> + dev_err(chan->dev,
> + "unable to allocate channel %d descriptor pool\n",
> + chan->id);
> + return -ENOMEM;
> + }
> +
> + tasklet_init(&chan->tasklet, xilinx_vdma_do_tasklet,
> + (unsigned long)chan);
> +
> + chan->completed_cookie = DMA_MIN_COOKIE;
> + chan->cookie = DMA_MIN_COOKIE;
Can you use virtual dma implementation to simplfy your implemenattion of lists,
cookies (driver/dma/virt-dma.c)
> + /* There is at least one descriptor free to be allocated */
???
> + return 1;
> +}
> +
> + * xilinx_vdma_tx_status - Get VDMA transaction status
> + * @dchan: DMA channel
> + * @cookie: Transaction identifier
> + * @txstate: Transaction state
> + *
> + * Return: DMA transaction status
> + */
> +static enum dma_status xilinx_vdma_tx_status(struct dma_chan *dchan,
> + dma_cookie_t cookie,
> + struct dma_tx_state *txstate)
> +{
> + struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan);
> + dma_cookie_t last_used;
> + dma_cookie_t last_complete;
> +
> + xilinx_vdma_chan_desc_cleanup(chan);
> +
> + last_used = dchan->cookie;
> + last_complete = chan->completed_cookie;
> +
> + dma_set_tx_state(txstate, last_complete, last_used, 0);
> +
> + return dma_async_is_complete(cookie, last_complete, last_used);
no residue calculation?
> +}
> +
> + * xilinx_vdma_start - Start VDMA channel
> + * @chan: Driver specific VDMA channel
> + */
> +static void xilinx_vdma_start(struct xilinx_vdma_chan *chan)
> +{
> + int loop = XILINX_VDMA_LOOP_COUNT + 1;
> +
> + vdma_ctrl_set(chan, XILINX_VDMA_REG_DMACR, XILINX_VDMA_DMACR_RUNSTOP);
> +
> + /* Wait for the hardware to start */
> + while (loop--)
> + if (!(vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR) &
> + XILINX_VDMA_DMASR_HALTED))
> + break;
wouldnt do while be better than doing than increamenting loop by 1 above and
using in while!
> +
> + if (!loop) {
> + dev_err(chan->dev, "Cannot start channel %p: %x\n",
> + chan, vdma_ctrl_read(chan, XILINX_VDMA_REG_DMASR));
> +
> + chan->err = true;
> + }
> +
> + return;
> +}
> +
> +/**
> + * xilinx_vdma_prep_slave_sg - prepare a descriptor for a DMA_SLAVE transaction
> + * @dchan: DMA channel
> + * @sgl: scatterlist to transfer to/from
> + * @sg_len: number of entries in @sgl
> + * @dir: DMA direction
> + * @flags: transfer ack flags
> + * @context: unused
> + *
> + * Return: Async transaction descriptor on success and NULL on failure
> + */
> +static struct dma_async_tx_descriptor *
> +xilinx_vdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
> + unsigned int sg_len, enum dma_transfer_direction dir,
> + unsigned long flags, void *context)
okay now am worried, this is supposed to memcpy DMA so why slave-sg??
Looking at the driver overall, IMHO we need to do:
- use the virt-dma to simplfy the cookie handling and perhpasn the descrptors
too!
- Perhpas use interleaved API..?
- I dont think we should use the slave API as this seems memcpy case!
--
~Vinod
^ permalink raw reply
* [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Vinod Koul @ 2014-01-26 14:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E12224.9060506@metafoo.de>
On Thu, Jan 23, 2014 at 03:07:32PM +0100, Lars-Peter Clausen wrote:
> On 01/23/2014 03:00 PM, Andy Shevchenko wrote:
> > On Thu, 2014-01-23 at 14:50 +0100, Lars-Peter Clausen wrote:
> >> On 01/23/2014 02:38 PM, Shevchenko, Andriy wrote:
> >>> On Thu, 2014-01-23 at 12:25 +0100, Lars-Peter Clausen wrote:
> >>>> On 01/22/2014 05:52 PM, Srikanth Thokala wrote:
> >>>
> >>> [...]
> >>>
> >>>>> + /* Request the interrupt */
> >>>>> + chan->irq = irq_of_parse_and_map(node, 0);
> >>>>> + err = devm_request_irq(xdev->dev, chan->irq, xilinx_vdma_irq_handler,
> >>>>> + IRQF_SHARED, "xilinx-vdma-controller", chan);
> >>>>
> >>>> This is a clasic example of where to not use devm_request_irq. 'chan' is
> >>>> accessed in the interrupt handler, but if you use devm_request_irq 'chan'
> >>>> will be freed before the interrupt handler has been released, which means
> >>>> there is now a race condition where the interrupt handler can access already
> >>>> freed memory.ta
> >>>
> >>> Could you elaborate this case? As far as I understood managed resources
> >>> are a kind of stack pile. In this case you have no such condition. Where
> >>> am I wrong?
> >>
> >> The stacked stuff is only ran after the remove() function. Which means that
> >> you call dma_async_device_unregister() before the interrupt handler is
> >> freed. Another issue with the interrupt handler is a bit hidden. The driver
> >> does not call tasklet_kill in the remove function. Which it should though to
> >> make sure that the tasklet does not race against the freeing of the memory.
> >> And in order to make sure that the tasklet is not rescheduled you need to
> >> free the irq before killing the tasklet, since the interrupt handler
> >> schedules the tasklet.
> >
> > So, you mean devm_request_irq() will race in any DMA driver?
>
> Most likely yes. devm_request_irq() is race condition prone for the majority
> of device driver. You have to be really careful if you want to use it.
>
> >
> > I think the proper solution is to disable all device work in
> > the .remove() and devm will care about resources.
>
> As long as the interrupt handler is registered it can be called, the only
> proper solution is to make sure that the order in which resources are torn
> down is correct.
Wouldn't it work if we register the irq last in the probe. That wil ensure on
success the channel is always valid.
Also the tasklet is required to be killed not just in your .remove but also in
drivers .suspend handler, you dont want handler to be invoked after you returned
from your suspend
--
~Vinod
^ permalink raw reply
* [PATCH v2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support
From: Vinod Koul @ 2014-01-26 13:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E2698B.6070001@metafoo.de>
On Fri, Jan 24, 2014 at 02:24:27PM +0100, Lars-Peter Clausen wrote:
> On 01/24/2014 12:16 PM, Srikanth Thokala wrote:
> > Hi Lars,
> >
> > On Thu, Jan 23, 2014 at 4:55 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> >> On 01/22/2014 05:52 PM, Srikanth Thokala wrote:
> >> [...]
> >>> +/**
> >>> + * xilinx_vdma_device_control - Configure DMA channel of the device
> >>> + * @dchan: DMA Channel pointer
> >>> + * @cmd: DMA control command
> >>> + * @arg: Channel configuration
> >>> + *
> >>> + * Return: '0' on success and failure value on error
> >>> + */
> >>> +static int xilinx_vdma_device_control(struct dma_chan *dchan,
> >>> + enum dma_ctrl_cmd cmd, unsigned long arg)
> >>> +{
> >>> + struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan);
> >>> +
> >>> + switch (cmd) {
> >>> + case DMA_TERMINATE_ALL:
> >>> + xilinx_vdma_terminate_all(chan);
> >>> + return 0;
> >>> + case DMA_SLAVE_CONFIG:
> >>> + return xilinx_vdma_slave_config(chan,
> >>> + (struct xilinx_vdma_config *)arg);
> >>
> >> You really shouldn't be overloading the generic API with your own semantics.
> >> DMA_SLAVE_CONFIG should take a dma_slave_config and nothing else.
> >
> > Ok. The driver needs few additional configuration from the slave
> > device like Vertical
> > Size, Horizontal Size, Stride etc., for the DMA transfers, in that case do you
> > suggest me to define a separate dma_ctrl_cmd like the one FSLDMA_EXTERNAL_START
> > defined for Freescale drivers?
>
> In my opinion it is not a good idea to have driver implement a generic API,
> but at the same time let the driver have custom semantics for those API
> calls. It's a bit like having a gpio driver that expects 23 and 42 as the
> values passed to gpio_set_value instead of 0 and 1. It completely defeats
> the purpose of a generic API, namely that you are able to write generic code
> that makes use of the API without having to know about which implementation
> API it is talking to. The dmaengine framework provides the
> dmaengine_prep_interleaved_dma() function to setup two dimensional
> transfers, e.g. take a look at sirf-dma.c or imx-dma.c.
The question here i think would be waht this device supports? Is the hardware
capable of doing interleaved transfers, then would make sense.
While we do try to get users use dma_slave_config, but there will always be
someone who have specfic params. If we can generalize then we might want to add
to the dma_slave_config as well
--
~Vinod
^ permalink raw reply
* [BUG] FL1009: xHCI host not responding to stop endpoint command.
From: Thomas Petazzoni @ 2014-01-26 13:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87sisfjeba.fsf@natisbad.org>
Dear Arnaud Ebalard,
On Thu, 23 Jan 2014 09:24:41 +0100, Arnaud Ebalard wrote:
> The various Armada-based devices I have are NAS which do not have PCIe
> slots to plug additional devices (everything is soldered). I don't know
> which device Thomas used for its tests. Just in case, I also added Willy
> in CC: who have various boards and may also have done more test with
> additional PCIe devices and CONFIG_PCI_MSI enabled on 3.13 kernel.
The device I've used to test MSI is a e1000e PCIe Intel network card.
It uses one MSI interrupt, so admittedly, the MSI testing is quite
limited for now.
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v4 08/18] watchdog: orion: Make RSTOUT register a separate resource
From: Ezequiel Garcia @ 2014-01-26 13:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <52E4003E.4070106@roeck-us.net>
On Sat, Jan 25, 2014 at 10:19:42AM -0800, Guenter Roeck wrote:
> On 01/22/2014 03:05 PM, Ezequiel Garcia wrote:
> > In order to support other SoC, it's required to distinguish
> > the 'control' timer register, from the 'rstout' register
> > that enables system reset on watchdog expiration.
> >
> > To prevent a compatibility break, this commit adds a fallback
> > to a hardcoded RSTOUT address.
> >
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > ---
> > .../devicetree/bindings/watchdog/marvel.txt | 6 ++-
> > arch/arm/mach-dove/include/mach/bridge-regs.h | 1 +
> > arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 1 +
> > arch/arm/mach-mv78xx0/include/mach/bridge-regs.h | 1 +
> > arch/arm/mach-orion5x/include/mach/bridge-regs.h | 1 +
> > arch/arm/plat-orion/common.c | 10 +++--
> > drivers/watchdog/orion_wdt.c | 44 +++++++++++++++++++++-
> > 7 files changed, 56 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/watchdog/marvel.txt b/Documentation/devicetree/bindings/watchdog/marvel.txt
> > index 0731fbd..1544fe9 100644
> > --- a/Documentation/devicetree/bindings/watchdog/marvel.txt
> > +++ b/Documentation/devicetree/bindings/watchdog/marvel.txt
> > @@ -3,7 +3,9 @@
> > Required Properties:
> >
> > - Compatibility : "marvell,orion-wdt"
> > -- reg : Address of the timer registers
> > +- reg : Should contain two entries: first one with the
> > + timer control address, second one with the
> > + rstout enable address.
> >
> > Optional properties:
> >
> > @@ -14,7 +16,7 @@ Example:
> >
> > wdt at 20300 {
> > compatible = "marvell,orion-wdt";
> > - reg = <0x20300 0x28>;
> > + reg = <0x20300 0x28>, <0x20108 0x4>;
> > interrupts = <3>;
> > timeout-sec = <10>;
> > status = "okay";
> > diff --git a/arch/arm/mach-dove/include/mach/bridge-regs.h b/arch/arm/mach-dove/include/mach/bridge-regs.h
> > index 5362df3..f4a5b34 100644
> > --- a/arch/arm/mach-dove/include/mach/bridge-regs.h
> > +++ b/arch/arm/mach-dove/include/mach/bridge-regs.h
> > @@ -21,6 +21,7 @@
> > #define CPU_CTRL_PCIE1_LINK 0x00000008
> >
> > #define RSTOUTn_MASK (BRIDGE_VIRT_BASE + 0x0108)
> > +#define RSTOUTn_MASK_PHYS (BRIDGE_PHYS_BASE + 0x0108)
> > #define SOFT_RESET_OUT_EN 0x00000004
> >
> > #define SYSTEM_SOFT_RESET (BRIDGE_VIRT_BASE + 0x010c)
> > diff --git a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> > index 8b9d1c9..60f6421 100644
> > --- a/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> > +++ b/arch/arm/mach-kirkwood/include/mach/bridge-regs.h
> > @@ -21,6 +21,7 @@
> > #define CPU_RESET 0x00000002
> >
> > #define RSTOUTn_MASK (BRIDGE_VIRT_BASE + 0x0108)
> > +#define RSTOUTn_MASK_PHYS (BRIDGE_PHYS_BASE + 0x0108)
> > #define SOFT_RESET_OUT_EN 0x00000004
> >
> > #define SYSTEM_SOFT_RESET (BRIDGE_VIRT_BASE + 0x010c)
> > diff --git a/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h b/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h
> > index 5f03484..e20d6da 100644
> > --- a/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h
> > +++ b/arch/arm/mach-mv78xx0/include/mach/bridge-regs.h
> > @@ -15,6 +15,7 @@
> > #define L2_WRITETHROUGH 0x00020000
> >
> > #define RSTOUTn_MASK (BRIDGE_VIRT_BASE + 0x0108)
> > +#define RSTOUTn_MASK_PHYS (BRIDGE_PHYS_BASE + 0x0108)
> > #define SOFT_RESET_OUT_EN 0x00000004
> >
> > #define SYSTEM_SOFT_RESET (BRIDGE_VIRT_BASE + 0x010c)
> > diff --git a/arch/arm/mach-orion5x/include/mach/bridge-regs.h b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
> > index f727d03..5766e3f 100644
> > --- a/arch/arm/mach-orion5x/include/mach/bridge-regs.h
> > +++ b/arch/arm/mach-orion5x/include/mach/bridge-regs.h
> > @@ -18,6 +18,7 @@
> > #define CPU_CTRL (ORION5X_BRIDGE_VIRT_BASE + 0x104)
> >
> > #define RSTOUTn_MASK (ORION5X_BRIDGE_VIRT_BASE + 0x108)
> > +#define RSTOUTn_MASK_PHYS (ORION5X_BRIDGE_PHYS_BASE + 0x108)
> >
> > #define CPU_SOFT_RESET (ORION5X_BRIDGE_VIRT_BASE + 0x10c)
> >
> > diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
> > index c66d163..3375037 100644
> > --- a/arch/arm/plat-orion/common.c
> > +++ b/arch/arm/plat-orion/common.c
> > @@ -594,14 +594,16 @@ void __init orion_spi_1_init(unsigned long mapbase)
> > /*****************************************************************************
> > * Watchdog
> > ****************************************************************************/
> > -static struct resource orion_wdt_resource =
> > - DEFINE_RES_MEM(TIMER_PHYS_BASE, 0x28);
> > +static struct resource orion_wdt_resource[] = {
> > + DEFINE_RES_MEM(TIMER_PHYS_BASE, 0x04),
> > + DEFINE_RES_MEM(RSTOUTn_MASK_PHYS, 0x04),
> > +};
> >
> > static struct platform_device orion_wdt_device = {
> > .name = "orion_wdt",
> > .id = -1,
> > - .num_resources = 1,
> > - .resource = &orion_wdt_resource,
> > + .num_resources = ARRAY_SIZE(orion_wdt_resource),
> > + .resource = orion_wdt_resource,
> > };
> >
> > void __init orion_wdt_init(void)
> > diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
> > index f5e7b17..ba8eea9d 100644
> > --- a/drivers/watchdog/orion_wdt.c
> > +++ b/drivers/watchdog/orion_wdt.c
> > @@ -26,6 +26,12 @@
> > #include <linux/of.h>
> > #include <mach/bridge-regs.h>
> >
> > +/* RSTOUT mask register physical address for Orion5x, Kirkwood and Dove */
> > +#define ORION_RSTOUT_MASK_OFFSET 0x20108
> > +
> > +/* Internal registers can be configured at any 1 MiB aligned address */
> > +#define INTERNAL_REGS_MASK ~(SZ_1M - 1)
> > +
> > /*
> > * Watchdog timer block registers.
> > */
> > @@ -44,6 +50,7 @@ static unsigned int wdt_max_duration; /* (seconds) */
> > static struct clk *clk;
> > static unsigned int wdt_tclk;
> > static void __iomem *wdt_reg;
> > +static void __iomem *wdt_rstout;
> >
> > static int orion_wdt_ping(struct watchdog_device *wdt_dev)
> > {
> > @@ -64,14 +71,14 @@ static int orion_wdt_start(struct watchdog_device *wdt_dev)
> > atomic_io_modify(wdt_reg + TIMER_CTRL, WDT_EN, WDT_EN);
> >
> > /* Enable reset on watchdog */
> > - atomic_io_modify(RSTOUTn_MASK, WDT_RESET_OUT_EN, WDT_RESET_OUT_EN);
> > + atomic_io_modify(wdt_rstout, WDT_RESET_OUT_EN, WDT_RESET_OUT_EN);
> > return 0;
> > }
> >
> > static int orion_wdt_stop(struct watchdog_device *wdt_dev)
> > {
> > /* Disable reset on watchdog */
> > - atomic_io_modify(RSTOUTn_MASK, WDT_RESET_OUT_EN, 0);
> > + atomic_io_modify(wdt_rstout, WDT_RESET_OUT_EN, 0);
> >
> > /* Disable watchdog timer */
> > atomic_io_modify(wdt_reg + TIMER_CTRL, WDT_EN, 0);
> > @@ -116,6 +123,33 @@ static irqreturn_t orion_wdt_irq(int irq, void *devid)
> > return IRQ_HANDLED;
> > }
> >
> > +/*
> > + * The original devicetree binding for this driver specified only
> > + * one memory resource, so in order to keep DT backwards compatibility
> > + * we try to fallback to a hardcoded register address, if the resource
> > + * is missing from the devicetree.
> > + */
> > +static void __iomem *try_rstout_ioremap(struct platform_device *pdev,
> > + phys_addr_t internal_regs)
> > +{
> > + struct resource *res;
> > + phys_addr_t rstout;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > + if (res)
> > + return devm_ioremap(&pdev->dev, res->start,
> > + resource_size(res));
> > +
> > + /* This workaround works only for "orion-wdt", DT-enabled */
> > + if (!of_device_is_compatible(pdev->dev.of_node, "marvell,orion-wdt"))
> > + return NULL;
> > +
> > + rstout = internal_regs + ORION_RSTOUT_MASK_OFFSET;
> > +
> > + WARN(1, FW_BUG "falling back to harcoded RSTOUT reg 0x%x\n", rstout);
>
> WARN seems to be a bit excessive here. Is that on purpose (sorry if that was discussed and I missed it) ?
>
> Assuming it is on purpose
>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>
Yes, it's on purpose. We want users to notice this and be aware they have
a broken dtb (hence the sign firmware bug).
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v4 08/18] watchdog: orion: Make RSTOUT register a separate resource
From: Ezequiel Garcia @ 2014-01-26 13:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140126091753.2910a39f@skate>
On Sun, Jan 26, 2014 at 09:17:53AM +0100, Thomas Petazzoni wrote:
[..]
> > +/*
> > + * The original devicetree binding for this driver specified only
> > + * one memory resource, so in order to keep DT backwards compatibility
> > + * we try to fallback to a hardcoded register address, if the resource
> > + * is missing from the devicetree.
> > + */
> > +static void __iomem *try_rstout_ioremap(struct platform_device *pdev,
> > + phys_addr_t internal_regs)
>
> Why is it called "try" ? It actually does the mapping. So I would
> prefer the function to be named:
>
> orion_wdt_ioremap_rstout()
>
Ah, yes. This is a left over from the previous attempt.
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v4 07/18] watchdog: orion: Handle IRQ
From: Ezequiel Garcia @ 2014-01-26 13:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140126090412.37f4932f@skate>
On Sun, Jan 26, 2014 at 09:04:12AM +0100, Thomas Petazzoni wrote:
> Dear Ezequiel Garcia,
>
> On Wed, 22 Jan 2014 20:05:04 -0300, Ezequiel Garcia wrote:
> > DT-enabled where an irqchip driver for the brigde interrupt controller is
> > available can handle the watchdog IRQ properly. Therefore, we request
> > the interruption and add a dummy handler that merely calls panic().
>
> I don't quiite understand the first sentence of this commit log, and
> the commit title looks wrong. Maybe a bad copy/paste or something?
>
Hm... yes it doesn't look right. It should read:
"DT-enabled platforms, where the irqchip driver for the brigde interrupt
controller is available, can handle the watchdog IRQ properly. Therefore,
we request the interrupt and add a dummy handler that merely calls panic()".
I guess we can re-phrase it be a bit more readable.
Why does th commit title looks wrong? By requesting the IRQ we are
"handling it", no?
> >
> > This is done in order to have an initial 'ack' of the interruption,
>
> interruption -> interrupt
>
Right.
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH] dma: fix vchan_cookie_complete() debug print
From: Vinod Koul @ 2014-01-26 11:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20140120132829.GD15937@n2100.arm.linux.org.uk>
On Mon, Jan 20, 2014 at 01:28:29PM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 20, 2014 at 05:33:01PM +0530, Vinod Koul wrote:
> > On Mon, Jan 20, 2014 at 11:28:22AM +0000, Russell King - ARM Linux wrote:
> > > On Mon, Jan 20, 2014 at 03:29:17PM +0530, Vinod Koul wrote:
> > > > On Fri, Dec 06, 2013 at 04:42:09PM +0100, Jonas Jensen wrote:
> > > > > vd->tx.cookie is set zero on dma_cookie_complete(),
> > > > > save to local before printing it.
> > > > >
> > > > > Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
> > > > > ---
> > > > >
> > > > > Notes:
> > > > > dev_vdbg() could also be moved to happen earlier, what do you prefer?
> > > > This would be preferred IMHO. Also pls cc dmaengine at vger on this
> > >
> > > I prefer this version - it means that the verbose debug printk doesn't
> > > impact the completion timing when printk is expensive (eg, because its
> > > outputting via a serial port.)
> > But if you know your printk is costly, do you want to enable these?
>
> dev_vdbg() is for verbose debugging - you only enable it if you really
> need to. Even so, it should have _minimal_ impact where possible. That's
> why I prefer the first patch, because we mark the cookie as being
> complete _before_ we call the verbose debugging, which isn't going to add
> milliseconds to that.
Sure this version is better approach in that respect as it makes it debug
aognostic! Both mine and Dan's comment were trying to simlify by ignoring debug
option, but yes i do agree to you point here. So this patch will be applied!
> If you don't care about debugging, then getting rid of the dev_vdbg().
> But really, I could pull rank and say that this is *my* file, I get to
> choose how stuff should be done here - I'd prefer not to but...
That is not required!
--
~Vinod
^ permalink raw reply
* [PATCH v4 08/18] watchdog: orion: Make RSTOUT register a separate resource
From: Thomas Petazzoni @ 2014-01-26 8:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1390431915-5115-9-git-send-email-ezequiel.garcia@free-electrons.com>
Dear Ezequiel Garcia,
On Wed, 22 Jan 2014 20:05:05 -0300, Ezequiel Garcia wrote:
> +/* RSTOUT mask register physical address for Orion5x, Kirkwood and Dove */
> +#define ORION_RSTOUT_MASK_OFFSET 0x20108
> +
> +/* Internal registers can be configured at any 1 MiB aligned address */
> +#define INTERNAL_REGS_MASK ~(SZ_1M - 1)
I'm not a big fan of hardcoding the internal register window size in a
driver. I would have preferred to have used the offset between the
watchdog registers and the rstout register, but since this one is
*before* the watchdog registers, it would have to be a negative offset.
Not sure how to handle this.
> +/*
> + * The original devicetree binding for this driver specified only
> + * one memory resource, so in order to keep DT backwards compatibility
> + * we try to fallback to a hardcoded register address, if the resource
> + * is missing from the devicetree.
> + */
> +static void __iomem *try_rstout_ioremap(struct platform_device *pdev,
> + phys_addr_t internal_regs)
Why is it called "try" ? It actually does the mapping. So I would
prefer the function to be named:
orion_wdt_ioremap_rstout()
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox