* Applied "ASoC: sunxi: Add support for A23/A33/H3 codec's analog path controls" to the asoc tree
From: Mark Brown @ 2016-11-22 19:13 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Mark Rutland, devicetree, alsa-devel, Liam Girdwood, Rob Herring,
linux-kernel, Mark Brown, Maxime Ripard, Mylene Josserand,
Lee Jones, linux-arm-kernel
In-Reply-To: <20161112064648.26779-3-wens@csie.org>
The patch
ASoC: sunxi: Add support for A23/A33/H3 codec's analog path controls
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From ba2ff3027b5ab4a96b9d2832822311c3ccbf3011 Mon Sep 17 00:00:00 2001
From: Chen-Yu Tsai <wens@csie.org>
Date: Sat, 12 Nov 2016 14:46:40 +0800
Subject: [PATCH] ASoC: sunxi: Add support for A23/A33/H3 codec's analog path
controls
The internal codec on A23/A33/H3 is split into 2 parts. The
analog path controls are routed through an embedded custom register
bus accessed through the PRCM block.
The SoCs share a common set of inputs, outputs, and audio paths.
The following table lists the differences.
----------------------------------------
| Feature \ SoC | A23 | A33 | H3 |
----------------------------------------
| Headphone | v | v | |
----------------------------------------
| Line Out | | | v |
----------------------------------------
| Phone In/Out | v | v | |
----------------------------------------
Add an ASoC component driver for it. This should be tied to the codec
audio card as an auxiliary device. This patch adds the commont paths
and controls, and variant specific headphone out and line out.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/sunxi/Kconfig | 8 +
sound/soc/sunxi/Makefile | 1 +
sound/soc/sunxi/sun8i-codec-analog.c | 665 +++++++++++++++++++++++++++++++++++
3 files changed, 674 insertions(+)
create mode 100644 sound/soc/sunxi/sun8i-codec-analog.c
diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
index dd2368297fd3..6c344e16aca4 100644
--- a/sound/soc/sunxi/Kconfig
+++ b/sound/soc/sunxi/Kconfig
@@ -9,6 +9,14 @@ config SND_SUN4I_CODEC
Select Y or M to add support for the Codec embedded in the Allwinner
A10 and affiliated SoCs.
+config SND_SUN8I_CODEC_ANALOG
+ tristate "Allwinner sun8i Codec Analog Controls Support"
+ depends on MACH_SUN8I || COMPILE_TEST
+ select REGMAP
+ help
+ Say Y or M if you want to add support for the analog controls for
+ the codec embedded in newer Allwinner SoCs.
+
config SND_SUN4I_I2S
tristate "Allwinner A10 I2S Support"
select SND_SOC_GENERIC_DMAENGINE_PCM
diff --git a/sound/soc/sunxi/Makefile b/sound/soc/sunxi/Makefile
index 604c7b842837..241c0df9ca0c 100644
--- a/sound/soc/sunxi/Makefile
+++ b/sound/soc/sunxi/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_SND_SUN4I_CODEC) += sun4i-codec.o
obj-$(CONFIG_SND_SUN4I_I2S) += sun4i-i2s.o
obj-$(CONFIG_SND_SUN4I_SPDIF) += sun4i-spdif.o
+obj-$(CONFIG_SND_SUN8I_CODEC_ANALOG) += sun8i-codec-analog.o
diff --git a/sound/soc/sunxi/sun8i-codec-analog.c b/sound/soc/sunxi/sun8i-codec-analog.c
new file mode 100644
index 000000000000..222bbd440b1e
--- /dev/null
+++ b/sound/soc/sunxi/sun8i-codec-analog.c
@@ -0,0 +1,665 @@
+/*
+ * This driver supports the analog controls for the internal codec
+ * found in Allwinner's A31s, A23, A33 and H3 SoCs.
+ *
+ * Copyright 2016 Chen-Yu Tsai <wens@csie.org>
+ *
+ * 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 <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <sound/soc.h>
+#include <sound/soc-dapm.h>
+#include <sound/tlv.h>
+
+/* Codec analog control register offsets and bit fields */
+#define SUN8I_ADDA_HP_VOLC 0x00
+#define SUN8I_ADDA_HP_VOLC_PA_CLK_GATE 7
+#define SUN8I_ADDA_HP_VOLC_HP_VOL 0
+#define SUN8I_ADDA_LOMIXSC 0x01
+#define SUN8I_ADDA_LOMIXSC_MIC1 6
+#define SUN8I_ADDA_LOMIXSC_MIC2 5
+#define SUN8I_ADDA_LOMIXSC_PHONE 4
+#define SUN8I_ADDA_LOMIXSC_PHONEN 3
+#define SUN8I_ADDA_LOMIXSC_LINEINL 2
+#define SUN8I_ADDA_LOMIXSC_DACL 1
+#define SUN8I_ADDA_LOMIXSC_DACR 0
+#define SUN8I_ADDA_ROMIXSC 0x02
+#define SUN8I_ADDA_ROMIXSC_MIC1 6
+#define SUN8I_ADDA_ROMIXSC_MIC2 5
+#define SUN8I_ADDA_ROMIXSC_PHONE 4
+#define SUN8I_ADDA_ROMIXSC_PHONEP 3
+#define SUN8I_ADDA_ROMIXSC_LINEINR 2
+#define SUN8I_ADDA_ROMIXSC_DACR 1
+#define SUN8I_ADDA_ROMIXSC_DACL 0
+#define SUN8I_ADDA_DAC_PA_SRC 0x03
+#define SUN8I_ADDA_DAC_PA_SRC_DACAREN 7
+#define SUN8I_ADDA_DAC_PA_SRC_DACALEN 6
+#define SUN8I_ADDA_DAC_PA_SRC_RMIXEN 5
+#define SUN8I_ADDA_DAC_PA_SRC_LMIXEN 4
+#define SUN8I_ADDA_DAC_PA_SRC_RHPPAMUTE 3
+#define SUN8I_ADDA_DAC_PA_SRC_LHPPAMUTE 2
+#define SUN8I_ADDA_DAC_PA_SRC_RHPIS 1
+#define SUN8I_ADDA_DAC_PA_SRC_LHPIS 0
+#define SUN8I_ADDA_PHONEIN_GCTRL 0x04
+#define SUN8I_ADDA_PHONEIN_GCTRL_PHONEPG 4
+#define SUN8I_ADDA_PHONEIN_GCTRL_PHONENG 0
+#define SUN8I_ADDA_LINEIN_GCTRL 0x05
+#define SUN8I_ADDA_LINEIN_GCTRL_LINEING 4
+#define SUN8I_ADDA_LINEIN_GCTRL_PHONEG 0
+#define SUN8I_ADDA_MICIN_GCTRL 0x06
+#define SUN8I_ADDA_MICIN_GCTRL_MIC1G 4
+#define SUN8I_ADDA_MICIN_GCTRL_MIC2G 0
+#define SUN8I_ADDA_PAEN_HP_CTRL 0x07
+#define SUN8I_ADDA_PAEN_HP_CTRL_HPPAEN 7
+#define SUN8I_ADDA_PAEN_HP_CTRL_LINEOUTEN 7 /* H3 specific */
+#define SUN8I_ADDA_PAEN_HP_CTRL_HPCOM_FC 5
+#define SUN8I_ADDA_PAEN_HP_CTRL_COMPTEN 4
+#define SUN8I_ADDA_PAEN_HP_CTRL_PA_ANTI_POP_CTRL 2
+#define SUN8I_ADDA_PAEN_HP_CTRL_LTRNMUTE 1
+#define SUN8I_ADDA_PAEN_HP_CTRL_RTLNMUTE 0
+#define SUN8I_ADDA_PHONEOUT_CTRL 0x08
+#define SUN8I_ADDA_PHONEOUT_CTRL_PHONEOUTG 5
+#define SUN8I_ADDA_PHONEOUT_CTRL_PHONEOUTEN 4
+#define SUN8I_ADDA_PHONEOUT_CTRL_PHONEOUT_MIC1 3
+#define SUN8I_ADDA_PHONEOUT_CTRL_PHONEOUT_MIC2 2
+#define SUN8I_ADDA_PHONEOUT_CTRL_PHONEOUT_RMIX 1
+#define SUN8I_ADDA_PHONEOUT_CTRL_PHONEOUT_LMIX 0
+#define SUN8I_ADDA_PHONE_GAIN_CTRL 0x09
+#define SUN8I_ADDA_PHONE_GAIN_CTRL_LINEOUT_VOL 3
+#define SUN8I_ADDA_PHONE_GAIN_CTRL_PHONEPREG 0
+#define SUN8I_ADDA_MIC2G_CTRL 0x0a
+#define SUN8I_ADDA_MIC2G_CTRL_MIC2AMPEN 7
+#define SUN8I_ADDA_MIC2G_CTRL_MIC2BOOST 4
+#define SUN8I_ADDA_MIC2G_CTRL_LINEOUTLEN 3
+#define SUN8I_ADDA_MIC2G_CTRL_LINEOUTREN 2
+#define SUN8I_ADDA_MIC2G_CTRL_LINEOUTLSRC 1
+#define SUN8I_ADDA_MIC2G_CTRL_LINEOUTRSRC 0
+#define SUN8I_ADDA_MIC1G_MICBIAS_CTRL 0x0b
+#define SUN8I_ADDA_MIC1G_MICBIAS_CTRL_HMICBIASEN 7
+#define SUN8I_ADDA_MIC1G_MICBIAS_CTRL_MMICBIASEN 6
+#define SUN8I_ADDA_MIC1G_MICBIAS_CTRL_HMICBIAS_MODE 5
+#define SUN8I_ADDA_MIC1G_MICBIAS_CTRL_MIC1AMPEN 3
+#define SUN8I_ADDA_MIC1G_MICBIAS_CTRL_MIC1BOOST 0
+#define SUN8I_ADDA_LADCMIXSC 0x0c
+#define SUN8I_ADDA_LADCMIXSC_MIC1 6
+#define SUN8I_ADDA_LADCMIXSC_MIC2 5
+#define SUN8I_ADDA_LADCMIXSC_PHONE 4
+#define SUN8I_ADDA_LADCMIXSC_PHONEN 3
+#define SUN8I_ADDA_LADCMIXSC_LINEINL 2
+#define SUN8I_ADDA_LADCMIXSC_OMIXRL 1
+#define SUN8I_ADDA_LADCMIXSC_OMIXRR 0
+#define SUN8I_ADDA_RADCMIXSC 0x0d
+#define SUN8I_ADDA_RADCMIXSC_MIC1 6
+#define SUN8I_ADDA_RADCMIXSC_MIC2 5
+#define SUN8I_ADDA_RADCMIXSC_PHONE 4
+#define SUN8I_ADDA_RADCMIXSC_PHONEP 3
+#define SUN8I_ADDA_RADCMIXSC_LINEINR 2
+#define SUN8I_ADDA_RADCMIXSC_OMIXR 1
+#define SUN8I_ADDA_RADCMIXSC_OMIXL 0
+#define SUN8I_ADDA_RES 0x0e
+#define SUN8I_ADDA_RES_MMICBIAS_SEL 4
+#define SUN8I_ADDA_RES_PA_ANTI_POP_CTRL 0
+#define SUN8I_ADDA_ADC_AP_EN 0x0f
+#define SUN8I_ADDA_ADC_AP_EN_ADCREN 7
+#define SUN8I_ADDA_ADC_AP_EN_ADCLEN 6
+#define SUN8I_ADDA_ADC_AP_EN_ADCG 0
+
+/* Analog control register access bits */
+#define ADDA_PR 0x0 /* PRCM base + 0x1c0 */
+#define ADDA_PR_RESET BIT(28)
+#define ADDA_PR_WRITE BIT(24)
+#define ADDA_PR_ADDR_SHIFT 16
+#define ADDA_PR_ADDR_MASK GENMASK(4, 0)
+#define ADDA_PR_DATA_IN_SHIFT 8
+#define ADDA_PR_DATA_IN_MASK GENMASK(7, 0)
+#define ADDA_PR_DATA_OUT_SHIFT 0
+#define ADDA_PR_DATA_OUT_MASK GENMASK(7, 0)
+
+/* regmap access bits */
+static int adda_reg_read(void *context, unsigned int reg, unsigned int *val)
+{
+ void __iomem *base = (void __iomem *)context;
+ u32 tmp;
+
+ /* De-assert reset */
+ writel(readl(base) | ADDA_PR_RESET, base);
+
+ /* Clear write bit */
+ writel(readl(base) & ~ADDA_PR_WRITE, base);
+
+ /* Set register address */
+ tmp = readl(base);
+ tmp &= ~(ADDA_PR_ADDR_MASK << ADDA_PR_ADDR_SHIFT);
+ tmp |= (reg & ADDA_PR_ADDR_MASK) << ADDA_PR_ADDR_SHIFT;
+ writel(tmp, base);
+
+ /* Read back value */
+ *val = readl(base) & ADDA_PR_DATA_OUT_MASK;
+
+ return 0;
+}
+
+static int adda_reg_write(void *context, unsigned int reg, unsigned int val)
+{
+ void __iomem *base = (void __iomem *)context;
+ u32 tmp;
+
+ /* De-assert reset */
+ writel(readl(base) | ADDA_PR_RESET, base);
+
+ /* Set register address */
+ tmp = readl(base);
+ tmp &= ~(ADDA_PR_ADDR_MASK << ADDA_PR_ADDR_SHIFT);
+ tmp |= (reg & ADDA_PR_ADDR_MASK) << ADDA_PR_ADDR_SHIFT;
+ writel(tmp, base);
+
+ /* Set data to write */
+ tmp = readl(base);
+ tmp &= ~(ADDA_PR_DATA_IN_MASK << ADDA_PR_DATA_IN_SHIFT);
+ tmp |= (val & ADDA_PR_DATA_IN_MASK) << ADDA_PR_DATA_IN_SHIFT;
+ writel(tmp, base);
+
+ /* Set write bit to signal a write */
+ writel(readl(base) | ADDA_PR_WRITE, base);
+
+ /* Clear write bit */
+ writel(readl(base) & ~ADDA_PR_WRITE, base);
+
+ return 0;
+}
+
+static const struct regmap_config adda_pr_regmap_cfg = {
+ .name = "adda-pr",
+ .reg_bits = 5,
+ .reg_stride = 1,
+ .val_bits = 8,
+ .reg_read = adda_reg_read,
+ .reg_write = adda_reg_write,
+ .fast_io = true,
+ .max_register = 24,
+};
+
+/* mixer controls */
+static const struct snd_kcontrol_new sun8i_codec_mixer_controls[] = {
+ SOC_DAPM_DOUBLE_R("DAC Playback Switch",
+ SUN8I_ADDA_LOMIXSC,
+ SUN8I_ADDA_ROMIXSC,
+ SUN8I_ADDA_LOMIXSC_DACL, 1, 0),
+ SOC_DAPM_DOUBLE_R("DAC Reversed Playback Switch",
+ SUN8I_ADDA_LOMIXSC,
+ SUN8I_ADDA_ROMIXSC,
+ SUN8I_ADDA_LOMIXSC_DACR, 1, 0),
+ SOC_DAPM_DOUBLE_R("Line In Playback Switch",
+ SUN8I_ADDA_LOMIXSC,
+ SUN8I_ADDA_ROMIXSC,
+ SUN8I_ADDA_LOMIXSC_LINEINL, 1, 0),
+ SOC_DAPM_DOUBLE_R("Mic1 Playback Switch",
+ SUN8I_ADDA_LOMIXSC,
+ SUN8I_ADDA_ROMIXSC,
+ SUN8I_ADDA_LOMIXSC_MIC1, 1, 0),
+ SOC_DAPM_DOUBLE_R("Mic2 Playback Switch",
+ SUN8I_ADDA_LOMIXSC,
+ SUN8I_ADDA_ROMIXSC,
+ SUN8I_ADDA_LOMIXSC_MIC2, 1, 0),
+};
+
+/* ADC mixer controls */
+static const struct snd_kcontrol_new sun8i_codec_adc_mixer_controls[] = {
+ SOC_DAPM_DOUBLE_R("Mixer Capture Switch",
+ SUN8I_ADDA_LADCMIXSC,
+ SUN8I_ADDA_RADCMIXSC,
+ SUN8I_ADDA_LADCMIXSC_OMIXRL, 1, 0),
+ SOC_DAPM_DOUBLE_R("Mixer Reversed Capture Switch",
+ SUN8I_ADDA_LADCMIXSC,
+ SUN8I_ADDA_RADCMIXSC,
+ SUN8I_ADDA_LADCMIXSC_OMIXRR, 1, 0),
+ SOC_DAPM_DOUBLE_R("Line In Capture Switch",
+ SUN8I_ADDA_LADCMIXSC,
+ SUN8I_ADDA_RADCMIXSC,
+ SUN8I_ADDA_LADCMIXSC_LINEINL, 1, 0),
+ SOC_DAPM_DOUBLE_R("Mic1 Capture Switch",
+ SUN8I_ADDA_LADCMIXSC,
+ SUN8I_ADDA_RADCMIXSC,
+ SUN8I_ADDA_LADCMIXSC_MIC1, 1, 0),
+ SOC_DAPM_DOUBLE_R("Mic2 Capture Switch",
+ SUN8I_ADDA_LADCMIXSC,
+ SUN8I_ADDA_RADCMIXSC,
+ SUN8I_ADDA_LADCMIXSC_MIC2, 1, 0),
+};
+
+/* volume / mute controls */
+static const DECLARE_TLV_DB_SCALE(sun8i_codec_out_mixer_pregain_scale,
+ -450, 150, 0);
+static const DECLARE_TLV_DB_RANGE(sun8i_codec_mic_gain_scale,
+ 0, 0, TLV_DB_SCALE_ITEM(0, 0, 0),
+ 1, 7, TLV_DB_SCALE_ITEM(2400, 300, 0),
+);
+
+static const struct snd_kcontrol_new sun8i_codec_common_controls[] = {
+ /* Mixer pre-gains */
+ SOC_SINGLE_TLV("Line In Playback Volume", SUN8I_ADDA_LINEIN_GCTRL,
+ SUN8I_ADDA_LINEIN_GCTRL_LINEING,
+ 0x7, 0, sun8i_codec_out_mixer_pregain_scale),
+ SOC_SINGLE_TLV("Mic1 Playback Volume", SUN8I_ADDA_MICIN_GCTRL,
+ SUN8I_ADDA_MICIN_GCTRL_MIC1G,
+ 0x7, 0, sun8i_codec_out_mixer_pregain_scale),
+ SOC_SINGLE_TLV("Mic2 Playback Volume",
+ SUN8I_ADDA_MICIN_GCTRL, SUN8I_ADDA_MICIN_GCTRL_MIC2G,
+ 0x7, 0, sun8i_codec_out_mixer_pregain_scale),
+
+ /* Microphone Amp boost gains */
+ SOC_SINGLE_TLV("Mic1 Boost Volume", SUN8I_ADDA_MIC1G_MICBIAS_CTRL,
+ SUN8I_ADDA_MIC1G_MICBIAS_CTRL_MIC1BOOST, 0x7, 0,
+ sun8i_codec_mic_gain_scale),
+ SOC_SINGLE_TLV("Mic2 Boost Volume", SUN8I_ADDA_MIC2G_CTRL,
+ SUN8I_ADDA_MIC2G_CTRL_MIC2BOOST, 0x7, 0,
+ sun8i_codec_mic_gain_scale),
+
+ /* ADC */
+ SOC_SINGLE_TLV("ADC Gain Capture Volume", SUN8I_ADDA_ADC_AP_EN,
+ SUN8I_ADDA_ADC_AP_EN_ADCG, 0x7, 0,
+ sun8i_codec_out_mixer_pregain_scale),
+};
+
+static const struct snd_soc_dapm_widget sun8i_codec_common_widgets[] = {
+ /* ADC */
+ SND_SOC_DAPM_ADC("Left ADC", NULL, SUN8I_ADDA_ADC_AP_EN,
+ SUN8I_ADDA_ADC_AP_EN_ADCLEN, 0),
+ SND_SOC_DAPM_ADC("Right ADC", NULL, SUN8I_ADDA_ADC_AP_EN,
+ SUN8I_ADDA_ADC_AP_EN_ADCREN, 0),
+
+ /* DAC */
+ SND_SOC_DAPM_DAC("Left DAC", NULL, SUN8I_ADDA_DAC_PA_SRC,
+ SUN8I_ADDA_DAC_PA_SRC_DACALEN, 0),
+ SND_SOC_DAPM_DAC("Right DAC", NULL, SUN8I_ADDA_DAC_PA_SRC,
+ SUN8I_ADDA_DAC_PA_SRC_DACAREN, 0),
+ /*
+ * Due to this component and the codec belonging to separate DAPM
+ * contexts, we need to manually link the above widgets to their
+ * stream widgets at the card level.
+ */
+
+ /* Line In */
+ SND_SOC_DAPM_INPUT("LINEIN"),
+
+ /* Microphone inputs */
+ SND_SOC_DAPM_INPUT("MIC1"),
+ SND_SOC_DAPM_INPUT("MIC2"),
+
+ /* Microphone Bias */
+ SND_SOC_DAPM_SUPPLY("MBIAS", SUN8I_ADDA_MIC1G_MICBIAS_CTRL,
+ SUN8I_ADDA_MIC1G_MICBIAS_CTRL_MMICBIASEN,
+ 0, NULL, 0),
+
+ /* Mic input path */
+ SND_SOC_DAPM_PGA("Mic1 Amplifier", SUN8I_ADDA_MIC1G_MICBIAS_CTRL,
+ SUN8I_ADDA_MIC1G_MICBIAS_CTRL_MIC1AMPEN, 0, NULL, 0),
+ SND_SOC_DAPM_PGA("Mic2 Amplifier", SUN8I_ADDA_MIC2G_CTRL,
+ SUN8I_ADDA_MIC2G_CTRL_MIC2AMPEN, 0, NULL, 0),
+
+ /* Mixers */
+ SND_SOC_DAPM_MIXER("Left Mixer", SUN8I_ADDA_DAC_PA_SRC,
+ SUN8I_ADDA_DAC_PA_SRC_LMIXEN, 0,
+ sun8i_codec_mixer_controls,
+ ARRAY_SIZE(sun8i_codec_mixer_controls)),
+ SND_SOC_DAPM_MIXER("Right Mixer", SUN8I_ADDA_DAC_PA_SRC,
+ SUN8I_ADDA_DAC_PA_SRC_RMIXEN, 0,
+ sun8i_codec_mixer_controls,
+ ARRAY_SIZE(sun8i_codec_mixer_controls)),
+ SND_SOC_DAPM_MIXER("Left ADC Mixer", SUN8I_ADDA_ADC_AP_EN,
+ SUN8I_ADDA_ADC_AP_EN_ADCLEN, 0,
+ sun8i_codec_adc_mixer_controls,
+ ARRAY_SIZE(sun8i_codec_adc_mixer_controls)),
+ SND_SOC_DAPM_MIXER("Right ADC Mixer", SUN8I_ADDA_ADC_AP_EN,
+ SUN8I_ADDA_ADC_AP_EN_ADCREN, 0,
+ sun8i_codec_adc_mixer_controls,
+ ARRAY_SIZE(sun8i_codec_adc_mixer_controls)),
+};
+
+static const struct snd_soc_dapm_route sun8i_codec_common_routes[] = {
+ /* Microphone Routes */
+ { "Mic1 Amplifier", NULL, "MIC1"},
+ { "Mic2 Amplifier", NULL, "MIC2"},
+
+ /* Left Mixer Routes */
+ { "Left Mixer", "DAC Playback Switch", "Left DAC" },
+ { "Left Mixer", "DAC Reversed Playback Switch", "Right DAC" },
+ { "Left Mixer", "Line In Playback Switch", "LINEIN" },
+ { "Left Mixer", "Mic1 Playback Switch", "Mic1 Amplifier" },
+ { "Left Mixer", "Mic2 Playback Switch", "Mic2 Amplifier" },
+
+ /* Right Mixer Routes */
+ { "Right Mixer", "DAC Playback Switch", "Right DAC" },
+ { "Right Mixer", "DAC Reversed Playback Switch", "Left DAC" },
+ { "Right Mixer", "Line In Playback Switch", "LINEIN" },
+ { "Right Mixer", "Mic1 Playback Switch", "Mic1 Amplifier" },
+ { "Right Mixer", "Mic2 Playback Switch", "Mic2 Amplifier" },
+
+ /* Left ADC Mixer Routes */
+ { "Left ADC Mixer", "Mixer Capture Switch", "Left Mixer" },
+ { "Left ADC Mixer", "Mixer Reversed Capture Switch", "Right Mixer" },
+ { "Left ADC Mixer", "Line In Capture Switch", "LINEIN" },
+ { "Left ADC Mixer", "Mic1 Capture Switch", "Mic1 Amplifier" },
+ { "Left ADC Mixer", "Mic2 Capture Switch", "Mic2 Amplifier" },
+
+ /* Right ADC Mixer Routes */
+ { "Right ADC Mixer", "Mixer Capture Switch", "Right Mixer" },
+ { "Right ADC Mixer", "Mixer Reversed Capture Switch", "Left Mixer" },
+ { "Right ADC Mixer", "Line In Capture Switch", "LINEIN" },
+ { "Right ADC Mixer", "Mic1 Capture Switch", "Mic1 Amplifier" },
+ { "Right ADC Mixer", "Mic2 Capture Switch", "Mic2 Amplifier" },
+
+ /* ADC Routes */
+ { "Left ADC", NULL, "Left ADC Mixer" },
+ { "Right ADC", NULL, "Right ADC Mixer" },
+};
+
+/* headphone specific controls, widgets, and routes */
+static const DECLARE_TLV_DB_SCALE(sun8i_codec_hp_vol_scale, -6300, 100, 1);
+static const struct snd_kcontrol_new sun8i_codec_headphone_controls[] = {
+ SOC_SINGLE_TLV("Headphone Playback Volume",
+ SUN8I_ADDA_HP_VOLC,
+ SUN8I_ADDA_HP_VOLC_HP_VOL, 0x3f, 0,
+ sun8i_codec_hp_vol_scale),
+ SOC_DOUBLE("Headphone Playback Switch",
+ SUN8I_ADDA_DAC_PA_SRC,
+ SUN8I_ADDA_DAC_PA_SRC_LHPPAMUTE,
+ SUN8I_ADDA_DAC_PA_SRC_RHPPAMUTE, 1, 0),
+};
+
+static const char * const sun8i_codec_hp_src_enum_text[] = {
+ "DAC", "Mixer",
+};
+
+static SOC_ENUM_DOUBLE_DECL(sun8i_codec_hp_src_enum,
+ SUN8I_ADDA_DAC_PA_SRC,
+ SUN8I_ADDA_DAC_PA_SRC_LHPIS,
+ SUN8I_ADDA_DAC_PA_SRC_RHPIS,
+ sun8i_codec_hp_src_enum_text);
+
+static const struct snd_kcontrol_new sun8i_codec_hp_src[] = {
+ SOC_DAPM_ENUM("Headphone Source Playback Route",
+ sun8i_codec_hp_src_enum),
+};
+
+static const struct snd_soc_dapm_widget sun8i_codec_headphone_widgets[] = {
+ SND_SOC_DAPM_MUX("Headphone Source Playback Route",
+ SND_SOC_NOPM, 0, 0, sun8i_codec_hp_src),
+ SND_SOC_DAPM_OUT_DRV("Headphone Amp", SUN8I_ADDA_PAEN_HP_CTRL,
+ SUN8I_ADDA_PAEN_HP_CTRL_HPPAEN, 0, NULL, 0),
+ SND_SOC_DAPM_SUPPLY("HPCOM Protection", SUN8I_ADDA_PAEN_HP_CTRL,
+ SUN8I_ADDA_PAEN_HP_CTRL_COMPTEN, 0, NULL, 0),
+ SND_SOC_DAPM_REG(snd_soc_dapm_supply, "HPCOM", SUN8I_ADDA_PAEN_HP_CTRL,
+ SUN8I_ADDA_PAEN_HP_CTRL_HPCOM_FC, 0x3, 0x3, 0),
+ SND_SOC_DAPM_OUTPUT("HP"),
+};
+
+static const struct snd_soc_dapm_route sun8i_codec_headphone_routes[] = {
+ { "Headphone Source Playback Route", "DAC", "Left DAC" },
+ { "Headphone Source Playback Route", "DAC", "Right DAC" },
+ { "Headphone Source Playback Route", "Mixer", "Left Mixer" },
+ { "Headphone Source Playback Route", "Mixer", "Right Mixer" },
+ { "Headphone Amp", NULL, "Headphone Source Playback Route" },
+ { "HPCOM", NULL, "HPCOM Protection" },
+ { "HP", NULL, "Headphone Amp" },
+};
+
+static int sun8i_codec_add_headphone(struct snd_soc_component *cmpnt)
+{
+ struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(cmpnt);
+ struct device *dev = cmpnt->dev;
+ int ret;
+
+ ret = snd_soc_add_component_controls(cmpnt,
+ sun8i_codec_headphone_controls,
+ ARRAY_SIZE(sun8i_codec_headphone_controls));
+ if (ret) {
+ dev_err(dev, "Failed to add Headphone controls: %d\n", ret);
+ return ret;
+ }
+
+ ret = snd_soc_dapm_new_controls(dapm, sun8i_codec_headphone_widgets,
+ ARRAY_SIZE(sun8i_codec_headphone_widgets));
+ if (ret) {
+ dev_err(dev, "Failed to add Headphone DAPM widgets: %d\n", ret);
+ return ret;
+ }
+
+ ret = snd_soc_dapm_add_routes(dapm, sun8i_codec_headphone_routes,
+ ARRAY_SIZE(sun8i_codec_headphone_routes));
+ if (ret) {
+ dev_err(dev, "Failed to add Headphone DAPM routes: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+/* hmic specific widget */
+static const struct snd_soc_dapm_widget sun8i_codec_hmic_widgets[] = {
+ SND_SOC_DAPM_SUPPLY("HBIAS", SUN8I_ADDA_MIC1G_MICBIAS_CTRL,
+ SUN8I_ADDA_MIC1G_MICBIAS_CTRL_HMICBIASEN,
+ 0, NULL, 0),
+};
+
+static int sun8i_codec_add_hmic(struct snd_soc_component *cmpnt)
+{
+ struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(cmpnt);
+ struct device *dev = cmpnt->dev;
+ int ret;
+
+ ret = snd_soc_dapm_new_controls(dapm, sun8i_codec_hmic_widgets,
+ ARRAY_SIZE(sun8i_codec_hmic_widgets));
+ if (ret)
+ dev_err(dev, "Failed to add Mic3 DAPM widgets: %d\n", ret);
+
+ return ret;
+}
+
+/* line out specific controls, widgets and routes */
+static const DECLARE_TLV_DB_RANGE(sun8i_codec_lineout_vol_scale,
+ 0, 1, TLV_DB_SCALE_ITEM(TLV_DB_GAIN_MUTE, 0, 1),
+ 2, 31, TLV_DB_SCALE_ITEM(-4350, 150, 0),
+);
+static const struct snd_kcontrol_new sun8i_codec_lineout_controls[] = {
+ SOC_SINGLE_TLV("Line Out Playback Volume",
+ SUN8I_ADDA_PHONE_GAIN_CTRL,
+ SUN8I_ADDA_PHONE_GAIN_CTRL_LINEOUT_VOL, 0x1f, 0,
+ sun8i_codec_lineout_vol_scale),
+ SOC_DOUBLE("Line Out Playback Switch",
+ SUN8I_ADDA_MIC2G_CTRL,
+ SUN8I_ADDA_MIC2G_CTRL_LINEOUTLEN,
+ SUN8I_ADDA_MIC2G_CTRL_LINEOUTREN, 1, 0),
+};
+
+static const char * const sun8i_codec_lineout_src_enum_text[] = {
+ "Stereo", "Mono Differential",
+};
+
+static SOC_ENUM_DOUBLE_DECL(sun8i_codec_lineout_src_enum,
+ SUN8I_ADDA_MIC2G_CTRL,
+ SUN8I_ADDA_MIC2G_CTRL_LINEOUTLSRC,
+ SUN8I_ADDA_MIC2G_CTRL_LINEOUTRSRC,
+ sun8i_codec_lineout_src_enum_text);
+
+static const struct snd_kcontrol_new sun8i_codec_lineout_src[] = {
+ SOC_DAPM_ENUM("Line Out Source Playback Route",
+ sun8i_codec_lineout_src_enum),
+};
+
+static const struct snd_soc_dapm_widget sun8i_codec_lineout_widgets[] = {
+ SND_SOC_DAPM_MUX("Line Out Source Playback Route",
+ SND_SOC_NOPM, 0, 0, sun8i_codec_lineout_src),
+ /* It is unclear if this is a buffer or gate, model it as a supply */
+ SND_SOC_DAPM_SUPPLY("Line Out Enable", SUN8I_ADDA_PAEN_HP_CTRL,
+ SUN8I_ADDA_PAEN_HP_CTRL_LINEOUTEN, 0, NULL, 0),
+ SND_SOC_DAPM_OUTPUT("LINEOUT"),
+};
+
+static const struct snd_soc_dapm_route sun8i_codec_lineout_routes[] = {
+ { "Line Out Source Playback Route", "Stereo", "Left Mixer" },
+ { "Line Out Source Playback Route", "Stereo", "Right Mixer" },
+ { "Line Out Source Playback Route", "Mono Differential", "Left Mixer" },
+ { "Line Out Source Playback Route", "Mono Differential", "Right Mixer" },
+ { "LINEOUT", NULL, "Line Out Source Playback Route" },
+ { "LINEOUT", NULL, "Line Out Enable", },
+};
+
+static int sun8i_codec_add_lineout(struct snd_soc_component *cmpnt)
+{
+ struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(cmpnt);
+ struct device *dev = cmpnt->dev;
+ int ret;
+
+ ret = snd_soc_add_component_controls(cmpnt,
+ sun8i_codec_lineout_controls,
+ ARRAY_SIZE(sun8i_codec_lineout_controls));
+ if (ret) {
+ dev_err(dev, "Failed to add Line Out controls: %d\n", ret);
+ return ret;
+ }
+
+ ret = snd_soc_dapm_new_controls(dapm, sun8i_codec_lineout_widgets,
+ ARRAY_SIZE(sun8i_codec_lineout_widgets));
+ if (ret) {
+ dev_err(dev, "Failed to add Line Out DAPM widgets: %d\n", ret);
+ return ret;
+ }
+
+ ret = snd_soc_dapm_add_routes(dapm, sun8i_codec_lineout_routes,
+ ARRAY_SIZE(sun8i_codec_lineout_routes));
+ if (ret) {
+ dev_err(dev, "Failed to add Line Out DAPM routes: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+struct sun8i_codec_analog_quirks {
+ bool has_headphone;
+ bool has_hmic;
+ bool has_lineout;
+};
+
+static const struct sun8i_codec_analog_quirks sun8i_a23_quirks = {
+ .has_headphone = true,
+ .has_hmic = true,
+};
+
+static const struct sun8i_codec_analog_quirks sun8i_h3_quirks = {
+ .has_lineout = true,
+};
+
+static int sun8i_codec_analog_cmpnt_probe(struct snd_soc_component *cmpnt)
+{
+ struct device *dev = cmpnt->dev;
+ const struct sun8i_codec_analog_quirks *quirks;
+ int ret;
+
+ /*
+ * This would never return NULL unless someone directly registers a
+ * platform device matching this driver's name, without specifying a
+ * device tree node.
+ */
+ quirks = of_device_get_match_data(dev);
+
+ /* Add controls, widgets, and routes for individual features */
+
+ if (quirks->has_headphone) {
+ ret = sun8i_codec_add_headphone(cmpnt);
+ if (ret)
+ return ret;
+ }
+
+ if (quirks->has_hmic) {
+ sun8i_codec_add_hmic(cmpnt);
+ if (ret)
+ return ret;
+ }
+
+ if (quirks->has_lineout) {
+ ret = sun8i_codec_add_lineout(cmpnt);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct snd_soc_component_driver sun8i_codec_analog_cmpnt_drv = {
+ .controls = sun8i_codec_common_controls,
+ .num_controls = ARRAY_SIZE(sun8i_codec_common_controls),
+ .dapm_widgets = sun8i_codec_common_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sun8i_codec_common_widgets),
+ .dapm_routes = sun8i_codec_common_routes,
+ .num_dapm_routes = ARRAY_SIZE(sun8i_codec_common_routes),
+ .probe = sun8i_codec_analog_cmpnt_probe,
+};
+
+static const struct of_device_id sun8i_codec_analog_of_match[] = {
+ {
+ .compatible = "allwinner,sun8i-a23-codec-analog",
+ .data = &sun8i_a23_quirks,
+ },
+ {
+ .compatible = "allwinner,sun8i-h3-codec-analog",
+ .data = &sun8i_h3_quirks,
+ },
+ {}
+};
+MODULE_DEVICE_TABLE(of, sun8i_codec_analog_of_match);
+
+static int sun8i_codec_analog_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ struct regmap *regmap;
+ void __iomem *base;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(base)) {
+ dev_err(&pdev->dev, "Failed to map the registers\n");
+ return PTR_ERR(base);
+ }
+
+ regmap = devm_regmap_init(&pdev->dev, NULL, base, &adda_pr_regmap_cfg);
+ if (IS_ERR(regmap)) {
+ dev_err(&pdev->dev, "Failed to create regmap\n");
+ return PTR_ERR(regmap);
+ }
+
+ return devm_snd_soc_register_component(&pdev->dev,
+ &sun8i_codec_analog_cmpnt_drv,
+ NULL, 0);
+}
+
+static struct platform_driver sun8i_codec_analog_driver = {
+ .driver = {
+ .name = "sun8i-codec-analog",
+ .of_match_table = sun8i_codec_analog_of_match,
+ },
+ .probe = sun8i_codec_analog_probe,
+};
+module_platform_driver(sun8i_codec_analog_driver);
+
+MODULE_DESCRIPTION("Allwinner internal codec analog controls driver");
+MODULE_AUTHOR("Chen-Yu Tsai <wens@csie.org>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:sun8i-codec-analog");
--
2.10.2
^ permalink raw reply related
* Re: [PATCH 0/2] serial: introduce DSR handling property
From: Uwe Kleine-König @ 2016-11-22 19:09 UTC (permalink / raw)
To: Christoph Fritz
Cc: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Jiri Slaby,
Geert Uytterhoeven, Arnd Bergmann,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1479834851-32442-1-git-send-email-chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
On Tue, Nov 22, 2016 at 06:14:09PM +0100, Christoph Fritz wrote:
> This patchset introduces device-tree property "disable-dsr" including
> documentation and usage.
>
> Christoph Fritz (2):
> doc: DT: add generic serial property to disable DSR
I'd add the documentation for all handshaking lines, not only dsr. And
I'd call it disable-hw-dsr, otherwise
{
disable-dsr;
dsr-gpios = <&gpio1 2 GPIO_ACTIVE_LOW>;
}
looks strange. (Not 100% sure this would make sense, but I think it
does.)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] of: base: add support to get machine model name
From: Frank Rowand @ 2016-11-22 18:44 UTC (permalink / raw)
To: Rob Herring, Sudeep Holla
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <582F5DC0.4080804-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hi Rob,
On 11/18/16 12:00, Frank Rowand wrote:
> On 11/18/16 06:46, Rob Herring wrote:
>> On Thu, Nov 17, 2016 at 03:32:54PM +0000, Sudeep Holla wrote:
>>> Currently platforms/drivers needing to get the machine model name are
>>> replicating the same snippet of code. In some case, the OF reference
>>> counting is either missing or incorrect.
>>>
>>> This patch adds support to read the machine model name either using
>>> the "model" or the "compatible" property in the device tree root node
>>> to the core OF/DT code.
>>>
>>> This can be used to remove all the duplicate code snippets doing exactly
>>> same thing later.
>>>
>>> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>> Cc: Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> Cc: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>>> Signed-off-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
>>> ---
>>> drivers/of/base.c | 32 ++++++++++++++++++++++++++++++++
>>> include/linux/of.h | 6 ++++++
>>> 2 files changed, 38 insertions(+)
>>>
>>> Hi Rob,
>>>
>>> It would be good if we can target this for v4.10, so that we have no
>>> dependencies to push PATCH 2/2 in v4.11
>>
>> Applied.
>>
>> Rob
>>
>
> A little fast on the trigger Rob.
>
> -Frank
This patch adds a function that leads to conflating the "model" property
and the "compatible" property. This leads to opaque, confusing and unclear
code where ever it is used. I think it is not good for the device tree
framework to contribute to writing unclear code.
Further, only two of the proposed users of this new function appear to
be proper usage. I do not think that the small amount of reduced lines
of code is a good trade off for the reduced code clarity and for the
potential for future mis-use of this function.
Can I convince you to revert this patch?
If not, will you accept a patch to change the function name to more
clearly indicate what it does? (One possible name would be
of_model_or_1st_compatible().)
-Frank
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] PM / Domains: Introduce domain-performance-state binding
From: Vincent Guittot @ 2016-11-22 18:34 UTC (permalink / raw)
To: Kevin Hilman
Cc: Viresh Kumar, Rob Herring, Rafael Wysocki,
linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org,
linux-kernel, Mark Rutland, Ulf Hansson, Lina Iyer,
devicetree@vger.kernel.org, Stephen Boyd, Nayak Rajendra
In-Reply-To: <m260nfz8mr.fsf@baylibre.com>
On 22 November 2016 at 19:12, Kevin Hilman <khilman@baylibre.com> wrote:
> Viresh Kumar <viresh.kumar@linaro.org> writes:
>
>> On 21-11-16, 09:07, Rob Herring wrote:
>>> On Fri, Nov 18, 2016 at 02:53:12PM +0530, Viresh Kumar wrote:
>>> > Some platforms have the capability to configure the performance state of
>>> > their Power Domains. The performance levels are represented by positive
>>> > integer values, a lower value represents lower performance state.
>>> >
>>> > The power-domains until now were only concentrating on the idle state
>>> > management of the device and this needs to change in order to reuse the
>>> > infrastructure of power domains for active state management.
>>> >
>>> > This patch introduces a new optional property for the consumers of the
>>> > power-domains: domain-performance-state.
>>> >
>>> > If the consumers don't need the capability of switching to different
>>> > domain performance states at runtime, then they can simply define their
>>> > required domain performance state in their node directly. Otherwise the
>>> > consumers can define their requirements with help of other
>>> > infrastructure, for example the OPP table.
>>> >
>>> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>>> > ---
>>> > Documentation/devicetree/bindings/power/power_domain.txt | 6 ++++++
>>> > 1 file changed, 6 insertions(+)
>>> >
>>> > diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>>> > index e1650364b296..db42eacf8b5c 100644
>>> > --- a/Documentation/devicetree/bindings/power/power_domain.txt
>>> > +++ b/Documentation/devicetree/bindings/power/power_domain.txt
>>> > @@ -106,6 +106,12 @@ domain provided by the 'parent' power controller.
>>> > - power-domains : A phandle and PM domain specifier as defined by bindings of
>>> > the power controller specified by phandle.
>>> >
>>> > +Optional properties:
>>> > +- domain-performance-state: A positive integer value representing the minimum
>>> > + performance level (of the parent domain) required by the consumer for its
>>> > + working. The integer value '1' represents the lowest performance level and the
>>> > + highest value represents the highest performance level.
>>>
>>> How does one come up with the range of values?
>>
>> Why would we need a range here? The value here represents the minimum 'state'
>> and the assumption is that everything above that level would be fine. So the
>> range is automatically: domain-performance-state -> MAX.
>>
>>> It seems like you are
>>> just making up numbers. Couldn't the domain performance level be an OPP
>>> in the sense that it is a collection of clock frequencies and voltage
>>> settings?
>>
>> The clock is going to be handled by the device itself (at least for the case we
>> have today) and the performance-state lies with the power-domain which is
>> configured separately. If the performance level includes both clk and voltage,
>> then why would we need to show the clock rates in the DT ? Wouldn't a
>> performance level be enough in such cases?
>
> I think the question is: what does the performance-level of a domain
> actually mean? Or, what are the units?
>
> Depending on the SoC, there's probably a few things this could mean. It
> might mean is that an underlying bus/interconnect can be configured to
> guarantee a specific bandwidth or throughput. That in turn might mean
> that that bus/interconnect might have to be set at a specific
> frequency/voltage.
>
> In your case, IIUC, you're just passing some magic value to some
> firmware running on a micro-controller, but under the hood that uC is
> probably configuring a frequency/voltage someplace.
In the case described by Viresh, it's only about setting the voltage
of a power domain that is shared between different devices. these
devices wants to run at different frequency (set by the devices) but
we have to select a Volateg value that will match with the constraint
of all devices (in this case the highest voltage)
>
> So, if we're going to have a generic DT binding for this, it needs to be
> something that's useful on platforms that are not using magic numbers
> managed by a uC as well.
>
> Kevin
>
>
>
>
>
>
>
>
^ permalink raw reply
* Re: [PATCH v4 1/2] i2c: aspeed: added driver for Aspeed I2C
From: Kachalov Anton @ 2016-11-22 18:23 UTC (permalink / raw)
To: Cédric Le Goater, Brendan Higgins,
wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <28078e6d-f5e4-da59-e226-baa9a60a4b19-Bxea+6Xhats@public.gmane.org>
Hello.
I would like to add my five cents here. Do not limit the bus_clk with 400kHz (FM) while HighSpeed is above 1MHz (above FM+ devices). I've successfully tested FM+ (1Mhz) in a quite big i2c network (a number of pca9600, pca9675, pca9848) with at least eight AST2150 SoCs on the common bus.
BTW. Just a lame question. If the device isn't designed to work on the higher speed (like standard of FM) while the bus selected as FM+, would those kind of devices just unoperate or may have undefined behavior and disturb the SDA/SCL? Just wondering to dynamically slowdown down to 100Khz (if needed) for the specific slave, but keep high rate (FM+) at the normal operation.
--- source/drivers/i2c/busses/i2c-aspeed.c.orig 2016-11-18 19:17:41.000000000 +0300
+++ source/drivers/i2c/busses/i2c-aspeed.c 2016-11-18 19:17:49.682092658 +0300
@@ -310,7 +310,7 @@ static void ast_i2c_dev_init(struct ast_
bus->bus_clk, clk_get_rate(bus->pclk));
/* Set AC Timing */
- if(bus->bus_clk / 1000 > 400) {
+ if(bus->bus_clk / 1000 > 1000) {
ast_i2c_write(bus, ast_i2c_read(bus, I2C_FUN_CTRL_REG) |
AST_I2CD_M_HIGH_SPEED_EN |
AST_I2CD_M_SDA_DRIVE_1T_EN |
22.11.2016, 12:07, "Cédric Le Goater" <clg-Bxea+6Xhats@public.gmane.org>:
> Hello Brendan,
>
> A few comments below,
>
> On 11/05/2016 02:58 AM, Brendan Higgins wrote:
>> Added initial master and slave support for Aspeed I2C controller.
>> Supports fourteen busses present in ast24xx and ast25xx BMC SoCs by
>> Aspeed.
>>
>> Signed-off-by: Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>> ---
>> Changes for v2:
>> - Added single module_init (multiple was breaking some builds).
>> Changes for v3:
>> - Removed "bus" device tree param; now extracted from bus address offset
>> Changes for v4:
>> - I2C adapter number is now generated dynamically unless specified in alias.
>> ---
>> drivers/i2c/busses/Kconfig | 10 +
>> drivers/i2c/busses/Makefile | 1 +
>> drivers/i2c/busses/i2c-aspeed.c | 807 ++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 818 insertions(+)
>> create mode 100644 drivers/i2c/busses/i2c-aspeed.c
>>
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index d252276..b6caa5d 100644
>> --- a/drivers/i2c/busses/Kconfig
>> +++ b/drivers/i2c/busses/Kconfig
>> @@ -1009,6 +1009,16 @@ config I2C_RCAR
>> This driver can also be built as a module. If so, the module
>> will be called i2c-rcar.
>>
>> +config I2C_ASPEED
>> + tristate "Aspeed AST2xxx SoC I2C Controller"
>> + depends on ARCH_ASPEED
>> + help
>> + If you say yes to this option, support will be included for the
>> + Aspeed AST2xxx SoC I2C controller.
>> +
>> + This driver can also be built as a module. If so, the module
>> + will be called i2c-aspeed.
>> +
>> comment "External I2C/SMBus adapter drivers"
>>
>> config I2C_DIOLAN_U2C
>> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
>> index 29764cc..826e780 100644
>> --- a/drivers/i2c/busses/Makefile
>> +++ b/drivers/i2c/busses/Makefile
>> @@ -99,6 +99,7 @@ obj-$(CONFIG_I2C_XILINX) += i2c-xiic.o
>> obj-$(CONFIG_I2C_XLR) += i2c-xlr.o
>> obj-$(CONFIG_I2C_XLP9XX) += i2c-xlp9xx.o
>> obj-$(CONFIG_I2C_RCAR) += i2c-rcar.o
>> +obj-$(CONFIG_I2C_ASPEED) += i2c-aspeed.o
>>
>> # External I2C/SMBus adapter drivers
>> obj-$(CONFIG_I2C_DIOLAN_U2C) += i2c-diolan-u2c.o
>> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
>> new file mode 100644
>> index 0000000..88e078a
>> --- /dev/null
>> +++ b/drivers/i2c/busses/i2c-aspeed.c
>> @@ -0,0 +1,807 @@
>> +/*
>> + * I2C adapter for the ASPEED I2C bus.
>> + *
>> + * Copyright (C) 2012-2020 ASPEED Technology Inc.
>> + * Copyright 2016 IBM Corporation
>> + * Copyright 2016 Google, Inc.
>> + *
>> + * 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/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/i2c.h>
>> +#include <linux/irq.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/errno.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/completion.h>
>> +#include <linux/slab.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/err.h>
>> +#include <linux/clk.h>
>> +
>> +/* I2C Register */
>> +#define ASPEED_I2C_FUN_CTRL_REG 0x00
>> +#define ASPEED_I2C_AC_TIMING_REG1 0x04
>> +#define ASPEED_I2C_AC_TIMING_REG2 0x08
>> +#define ASPEED_I2C_INTR_CTRL_REG 0x0c
>> +#define ASPEED_I2C_INTR_STS_REG 0x10
>> +#define ASPEED_I2C_CMD_REG 0x14
>> +#define ASPEED_I2C_DEV_ADDR_REG 0x18
>> +#define ASPEED_I2C_BYTE_BUF_REG 0x20
>> +#define ASPEED_I2C_OFFSET_START 0x40
>> +#define ASPEED_I2C_OFFSET_INCREMENT 0x40
>> +
>> +#define ASPEED_I2C_NUM_BUS 14
>> +
>> +/* Global Register Definition */
>> +/* 0x00 : I2C Interrupt Status Register */
>> +/* 0x08 : I2C Interrupt Target Assignment */
>> +
>> +/* Device Register Definition */
>> +/* 0x00 : I2CD Function Control Register */
>> +#define ASPEED_I2CD_MULTI_MASTER_DIS BIT(15)
>> +#define ASPEED_I2CD_SDA_DRIVE_1T_EN BIT(8)
>> +#define ASPEED_I2CD_M_SDA_DRIVE_1T_EN BIT(7)
>> +#define ASPEED_I2CD_M_HIGH_SPEED_EN BIT(6)
>> +#define ASPEED_I2CD_SLAVE_EN BIT(1)
>> +#define ASPEED_I2CD_MASTER_EN BIT(0)
>> +
>> +/* 0x08 : I2CD Clock and AC Timing Control Register #2 */
>> +#define ASPEED_NO_TIMEOUT_CTRL 0
>> +
>> +
>> +/* 0x0c : I2CD Interrupt Control Register &
>> + * 0x10 : I2CD Interrupt Status Register
>> + *
>> + * These share bit definitions, so use the same values for the enable &
>> + * status bits.
>> + */
>> +#define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT BIT(14)
>> +#define ASPEED_I2CD_INTR_BUS_RECOVER_DONE BIT(13)
>> +#define ASPEED_I2CD_INTR_SLAVE_MATCH BIT(7)
>> +#define ASPEED_I2CD_INTR_SCL_TIMEOUT BIT(6)
>> +#define ASPEED_I2CD_INTR_ABNORMAL BIT(5)
>> +#define ASPEED_I2CD_INTR_NORMAL_STOP BIT(4)
>> +#define ASPEED_I2CD_INTR_ARBIT_LOSS BIT(3)
>> +#define ASPEED_I2CD_INTR_RX_DONE BIT(2)
>> +#define ASPEED_I2CD_INTR_TX_NAK BIT(1)
>> +#define ASPEED_I2CD_INTR_TX_ACK BIT(0)
>> +
>> +/* 0x14 : I2CD Command/Status Register */
>> +#define ASPEED_I2CD_SCL_LINE_STS BIT(18)
>> +#define ASPEED_I2CD_SDA_LINE_STS BIT(17)
>> +#define ASPEED_I2CD_BUS_BUSY_STS BIT(16)
>> +#define ASPEED_I2CD_BUS_RECOVER_CMD BIT(11)
>> +
>> +/* Command Bit */
>> +#define ASPEED_I2CD_M_STOP_CMD BIT(5)
>> +#define ASPEED_I2CD_M_S_RX_CMD_LAST BIT(4)
>> +#define ASPEED_I2CD_M_RX_CMD BIT(3)
>> +#define ASPEED_I2CD_S_TX_CMD BIT(2)
>> +#define ASPEED_I2CD_M_TX_CMD BIT(1)
>> +#define ASPEED_I2CD_M_START_CMD BIT(0)
>> +
>> +/* 0x18 : I2CD Slave Device Address Register */
>> +#define ASPEED_I2CD_DEV_ADDR_MASK GENMASK(6, 0)
>> +
>> +enum aspeed_i2c_slave_state {
>> + ASPEED_I2C_SLAVE_START,
>> + ASPEED_I2C_SLAVE_READ_REQUESTED,
>> + ASPEED_I2C_SLAVE_READ_PROCESSED,
>> + ASPEED_I2C_SLAVE_WRITE_REQUESTED,
>> + ASPEED_I2C_SLAVE_WRITE_RECEIVED,
>> + ASPEED_I2C_SLAVE_STOP,
>> +};
>> +
>> +struct aspeed_i2c_bus {
>> + struct i2c_adapter adap;
>> + struct device *dev;
>> + void __iomem *base;
>> + spinlock_t lock;
>> + struct completion cmd_complete;
>> + int irq;
>> + /* Transaction state. */
>> + struct i2c_msg *msg;
>> + int msg_pos;
>> + u32 cmd_err;
>> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
>> + struct i2c_client *slave;
>> + enum aspeed_i2c_slave_state slave_state;
>> +#endif
>> +};
>> +
>> +struct aspeed_i2c_controller {
>> + struct device *dev;
>> + void __iomem *base;
>> + int irq;
>> + struct irq_domain *irq_domain;
>> +};
>> +
>> +static inline void aspeed_i2c_write(struct aspeed_i2c_bus *bus, u32 val,
>> + u32 reg)
>> +{
>> + writel(val, bus->base + reg);
>> +}
>> +
>> +static inline u32 aspeed_i2c_read(struct aspeed_i2c_bus *bus, u32 reg)
>> +{
>> + return readl(bus->base + reg);
>> +}
>> +
>> +static u8 aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
>> +{
>> + u32 command;
>> + unsigned long time_left;
>> + unsigned long flags;
>> + int ret = 0;
>> +
>> + spin_lock_irqsave(&bus->lock, flags);
>> + command = aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG);
>> + /* Bus is idle: no recovery needed. */
>> + if ((command & ASPEED_I2CD_SDA_LINE_STS) &&
>> + (command & ASPEED_I2CD_SCL_LINE_STS))
>> + goto out;
>> +
>> + dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
>> + command);
>> +
>> + /* Bus held: put bus in stop state. */
>> + if ((command & ASPEED_I2CD_SDA_LINE_STS) &&
>> + !(command & ASPEED_I2CD_SCL_LINE_STS)) {
>> + aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD,
>> + ASPEED_I2C_CMD_REG);
>> + reinit_completion(&bus->cmd_complete);
>> + spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> + time_left = wait_for_completion_interruptible_timeout(
>> + &bus->cmd_complete, bus->adap.timeout * HZ);
>> +
>> + spin_lock_irqsave(&bus->lock, flags);
>> + if (time_left == 0)
>> + ret = -ETIMEDOUT;
>> + else if (bus->cmd_err)
>> + ret = -EIO;
>> + /* Bus error. */
>> + } else if (!(command & ASPEED_I2CD_SDA_LINE_STS)) {
>> + aspeed_i2c_write(bus, ASPEED_I2CD_BUS_RECOVER_CMD,
>> + ASPEED_I2C_CMD_REG);
>> + reinit_completion(&bus->cmd_complete);
>> + spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> + time_left = wait_for_completion_interruptible_timeout(
>> + &bus->cmd_complete, bus->adap.timeout * HZ);
>> +
>> + spin_lock_irqsave(&bus->lock, flags);
>> + if (time_left == 0)
>> + ret = -ETIMEDOUT;
>> + else if (bus->cmd_err)
>> + ret = -EIO;
>> + /* Recovery failed. */
>> + else if (!(aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG) &
>> + ASPEED_I2CD_SDA_LINE_STS))
>> + ret = -EIO;
>> + }
>> +
>> +out:
>> + spin_unlock_irqrestore(&bus->lock, flags);
>> + return ret;
>> +}
>> +
>> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
>> +static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
>> +{
>> + bool irq_handled = true;
>> + u32 command;
>> + u32 irq_status;
>> + u32 status_ack = 0;
>> + u8 value;
>> + struct i2c_client *slave = bus->slave;
>> +
>> + spin_lock(&bus->lock);
>> + if (!slave) {
>> + irq_handled = false;
>> + goto out;
>> + }
>> + command = aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG);
>> + irq_status = aspeed_i2c_read(bus, ASPEED_I2C_INTR_STS_REG);
>> +
>> + /* Slave was requested, restart state machine. */
>> + if (irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
>> + status_ack |= ASPEED_I2CD_INTR_SLAVE_MATCH;
>> + bus->slave_state = ASPEED_I2C_SLAVE_START;
>> + }
>> + /* Slave is not currently active, irq was for someone else. */
>> + if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
>> + irq_handled = false;
>> + goto out;
>> + }
>> +
>> + dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
>> + irq_status, command);
>> +
>> + /* Slave was sent something. */
>> + if (irq_status & ASPEED_I2CD_INTR_RX_DONE) {
>> + value = aspeed_i2c_read(bus, ASPEED_I2C_BYTE_BUF_REG) >> 8;
>> + /* Handle address frame. */
>> + if (bus->slave_state == ASPEED_I2C_SLAVE_START) {
>> + if (value & 0x1)
>> + bus->slave_state =
>> + ASPEED_I2C_SLAVE_READ_REQUESTED;
>> + else
>> + bus->slave_state =
>> + ASPEED_I2C_SLAVE_WRITE_REQUESTED;
>> + }
>> + status_ack |= ASPEED_I2CD_INTR_RX_DONE;
>> + }
>> +
>> + /* Slave was asked to stop. */
>> + if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
>> + status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
>> + bus->slave_state = ASPEED_I2C_SLAVE_STOP;
>> + }
>> + if (irq_status & ASPEED_I2CD_INTR_TX_NAK) {
>> + status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>> + bus->slave_state = ASPEED_I2C_SLAVE_STOP;
>> + }
>> +
>> + if (bus->slave_state == ASPEED_I2C_SLAVE_READ_REQUESTED) {
>> + if (irq_status & ASPEED_I2CD_INTR_TX_ACK)
>> + dev_err(bus->dev, "Unexpected ACK on read request.\n");
>> + bus->slave_state = ASPEED_I2C_SLAVE_READ_PROCESSED;
>> +
>> + i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value);
>> + aspeed_i2c_write(bus, value, ASPEED_I2C_BYTE_BUF_REG);
>> + aspeed_i2c_write(bus, ASPEED_I2CD_S_TX_CMD, ASPEED_I2C_CMD_REG);
>> + } else if (bus->slave_state == ASPEED_I2C_SLAVE_READ_PROCESSED) {
>> + status_ack |= ASPEED_I2CD_INTR_TX_ACK;
>> + if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK))
>> + dev_err(bus->dev,
>> + "Expected ACK after processed read.\n");
>> + i2c_slave_event(slave, I2C_SLAVE_READ_PROCESSED, &value);
>> + aspeed_i2c_write(bus, value, ASPEED_I2C_BYTE_BUF_REG);
>> + aspeed_i2c_write(bus, ASPEED_I2CD_S_TX_CMD, ASPEED_I2C_CMD_REG);
>> + } else if (bus->slave_state == ASPEED_I2C_SLAVE_WRITE_REQUESTED) {
>> + bus->slave_state = ASPEED_I2C_SLAVE_WRITE_RECEIVED;
>> + i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value);
>> + } else if (bus->slave_state == ASPEED_I2C_SLAVE_WRITE_RECEIVED) {
>> + i2c_slave_event(slave, I2C_SLAVE_WRITE_RECEIVED, &value);
>> + } else if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
>> + i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
>> + }
>> +
>> + if (status_ack != irq_status)
>> + dev_err(bus->dev,
>> + "irq handled != irq. expected %x, but was %x\n",
>> + irq_status, status_ack);
>> + aspeed_i2c_write(bus, status_ack, ASPEED_I2C_INTR_STS_REG);
>> +
>> +out:
>> + spin_unlock(&bus->lock);
>> + return irq_handled;
>> +}
>> +#endif
>> +
>> +static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
>> +{
>> + const u32 errs = ASPEED_I2CD_INTR_ARBIT_LOSS |
>> + ASPEED_I2CD_INTR_ABNORMAL |
>> + ASPEED_I2CD_INTR_SCL_TIMEOUT |
>> + ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |
>> + ASPEED_I2CD_INTR_TX_NAK;
>> + u32 irq_status;
>> +
>> + spin_lock(&bus->lock);
>> + irq_status = aspeed_i2c_read(bus, ASPEED_I2C_INTR_STS_REG);
>> + bus->cmd_err = irq_status & errs;
>> +
>> + dev_dbg(bus->dev, "master irq status 0x%08x\n", irq_status);
>> +
>> + /* No message to transfer. */
>> + if (bus->cmd_err ||
>> + (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) ||
>> + (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE)) {
>> + complete(&bus->cmd_complete);
>> + goto out;
>> + } else if (!bus->msg || bus->msg_pos >= bus->msg->len)
>> + goto out;
>> +
>> + if ((bus->msg->flags & I2C_M_RD) &&
>> + (irq_status & ASPEED_I2CD_INTR_RX_DONE)) {
>> + bus->msg->buf[bus->msg_pos++] = aspeed_i2c_read(
>> + bus, ASPEED_I2C_BYTE_BUF_REG) >> 8;
>> + if (bus->msg_pos + 1 < bus->msg->len)
>> + aspeed_i2c_write(bus, ASPEED_I2CD_M_RX_CMD,
>> + ASPEED_I2C_CMD_REG);
>> + else if (bus->msg_pos < bus->msg->len)
>> + aspeed_i2c_write(bus, ASPEED_I2CD_M_RX_CMD |
>> + ASPEED_I2CD_M_S_RX_CMD_LAST,
>> + ASPEED_I2C_CMD_REG);
>> + } else if (!(bus->msg->flags & I2C_M_RD) &&
>> + (irq_status & ASPEED_I2CD_INTR_TX_ACK)) {
>> + aspeed_i2c_write(bus, bus->msg->buf[bus->msg_pos++],
>> + ASPEED_I2C_BYTE_BUF_REG);
>> + aspeed_i2c_write(bus, ASPEED_I2CD_M_TX_CMD, ASPEED_I2C_CMD_REG);
>> + }
>
> is it safe to start a new transaction when in a interrupt handler ?
>
>> + /* Transmission complete: notify caller. */
>> + if (bus->msg_pos >= bus->msg->len)
>> + complete(&bus->cmd_complete);
>> +out:
>> + aspeed_i2c_write(bus, irq_status, ASPEED_I2C_INTR_STS_REG);
>
> so this is clearing the interrupt status just after having started
> a new transaction on the bus. It looks unsafe to me as it could
> clear the status of the just started transaction.
>
>> + spin_unlock(&bus->lock);
>> + return true;
>> +}
>> +
>> +static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
>> +{
>> + struct aspeed_i2c_bus *bus = dev_id;
>> +
>> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
>> + if (aspeed_i2c_slave_irq(bus)) {
>> + dev_dbg(bus->dev, "irq handled by slave.\n");
>> + return IRQ_HANDLED;
>> + }
>> +#endif
>> + if (aspeed_i2c_master_irq(bus)) {
>> + dev_dbg(bus->dev, "irq handled by master.\n");
>> + return IRQ_HANDLED;
>> + }
>> + dev_err(bus->dev, "irq not handled properly!\n");
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int aspeed_i2c_master_single_xfer(struct i2c_adapter *adap,
>> + struct i2c_msg *msg)
>> +{
>> + struct aspeed_i2c_bus *bus = adap->algo_data;
>> + unsigned long flags;
>> + u8 slave_addr;
>> + u32 command = ASPEED_I2CD_M_START_CMD | ASPEED_I2CD_M_TX_CMD;
>> + int ret = msg->len;
>> + unsigned long time_left;
>> +
>> + spin_lock_irqsave(&bus->lock, flags);
>> + bus->msg = msg;
>> + bus->msg_pos = 0;
>> + slave_addr = msg->addr << 1;
>> + if (msg->flags & I2C_M_RD) {
>> + slave_addr |= 1;
>> + command |= ASPEED_I2CD_M_RX_CMD;
>> + if (msg->len == 1)
>> + command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
>> + }
>> + aspeed_i2c_write(bus, slave_addr, ASPEED_I2C_BYTE_BUF_REG);
>> + aspeed_i2c_write(bus, command, ASPEED_I2C_CMD_REG);
>> + reinit_completion(&bus->cmd_complete);
>> + spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> + time_left = wait_for_completion_interruptible_timeout(
>> + &bus->cmd_complete, bus->adap.timeout * HZ * msg->len);
>
> Why do you multiply by * msg->len ?
>
>> + if (time_left == 0)
>> + return -ETIMEDOUT;
>> +
>> + spin_lock_irqsave(&bus->lock, flags);
>> + if (bus->cmd_err)
>> + ret = -EIO;
>> + bus->msg = NULL;
>> + spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> + return ret;
>> +}
>> +
>> +static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
>> + struct i2c_msg *msgs, int num)
>> +{
>> + struct aspeed_i2c_bus *bus = adap->algo_data;
>> + int ret;
>> + int i;
>> + unsigned long flags;
>> + unsigned long time_left;
>> +
>> + /* If bus is busy, attempt recovery. We assume a single master
>> + * environment.
>> + */
>> + if (aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG) &
>> + ASPEED_I2CD_BUS_BUSY_STS) {
>> + ret = aspeed_i2c_recover_bus(bus);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + for (i = 0; i < num; i++) {
>> + ret = aspeed_i2c_master_single_xfer(adap, &msgs[i]);
>> + if (ret < 0)
>> + break;
>> + /* TODO: Support other forms of I2C protocol mangling. */
>> + if (msgs[i].flags & I2C_M_STOP) {
>> + spin_lock_irqsave(&bus->lock, flags);
>> + aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD,
>> + ASPEED_I2C_CMD_REG);
>> + reinit_completion(&bus->cmd_complete);
>> + spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> + time_left = wait_for_completion_interruptible_timeout(
>> + &bus->cmd_complete,
>> + bus->adap.timeout * HZ);
>> + if (time_left == 0)
>> + return -ETIMEDOUT;
>> + }
>> + }
>> +
>> + spin_lock_irqsave(&bus->lock, flags);
>> + aspeed_i2c_write(bus, ASPEED_I2CD_M_STOP_CMD, ASPEED_I2C_CMD_REG);
>> + reinit_completion(&bus->cmd_complete);
>> + spin_unlock_irqrestore(&bus->lock, flags);
>> +
>> + time_left = wait_for_completion_interruptible_timeout(
>> + &bus->cmd_complete, bus->adap.timeout * HZ);
>> + if (time_left == 0)
>> + return -ETIMEDOUT;
>> +
>> + /* If nothing went wrong, return number of messages transferred. */
>> + if (ret < 0)
>> + return ret;
>> + else
>> + return i;
>> +}
>> +
>> +static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
>> +{
>> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
>> +}
>> +
>> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
>> +static int aspeed_i2c_reg_slave(struct i2c_client *client)
>> +{
>> + struct aspeed_i2c_bus *bus;
>> + unsigned long flags;
>> + u32 addr_reg_val;
>> + u32 func_ctrl_reg_val;
>> +
>> + bus = client->adapter->algo_data;
>> + spin_lock_irqsave(&bus->lock, flags);
>> + if (bus->slave) {
>> + spin_unlock_irqrestore(&bus->lock, flags);
>> + return -EINVAL;
>> + }
>> +
>> + /* Set slave addr. */
>> + addr_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_DEV_ADDR_REG);
>> + addr_reg_val &= ~ASPEED_I2CD_DEV_ADDR_MASK;
>> + addr_reg_val |= client->addr & ASPEED_I2CD_DEV_ADDR_MASK;
>> + aspeed_i2c_write(bus, addr_reg_val, ASPEED_I2C_DEV_ADDR_REG);
>> +
>> + /* Switch from master mode to slave mode. */
>> + func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
>> + func_ctrl_reg_val &= ~ASPEED_I2CD_MASTER_EN;
>> + func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
>> + aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
>> +
>> + bus->slave = client;
>> + bus->slave_state = ASPEED_I2C_SLAVE_STOP;
>> + spin_unlock_irqrestore(&bus->lock, flags);
>> + return 0;
>> +}
>> +
>> +static int aspeed_i2c_unreg_slave(struct i2c_client *client)
>> +{
>> + struct aspeed_i2c_bus *bus = client->adapter->algo_data;
>> + unsigned long flags;
>> + u32 func_ctrl_reg_val;
>> +
>> + spin_lock_irqsave(&bus->lock, flags);
>> + if (!bus->slave) {
>> + spin_unlock_irqrestore(&bus->lock, flags);
>> + return -EINVAL;
>> + }
>> +
>> + /* Switch from slave mode to master mode. */
>> + func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
>> + func_ctrl_reg_val &= ~ASPEED_I2CD_SLAVE_EN;
>> + func_ctrl_reg_val |= ASPEED_I2CD_MASTER_EN;
>> + aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
>> +
>> + bus->slave = NULL;
>> + spin_unlock_irqrestore(&bus->lock, flags);
>> + return 0;
>> +}
>> +#endif
>> +
>> +static const struct i2c_algorithm aspeed_i2c_algo = {
>> + .master_xfer = aspeed_i2c_master_xfer,
>> + .functionality = aspeed_i2c_functionality,
>> +#if IS_ENABLED(CONFIG_I2C_SLAVE)
>> + .reg_slave = aspeed_i2c_reg_slave,
>> + .unreg_slave = aspeed_i2c_unreg_slave,
>> +#endif
>> +};
>> +
>> +static u32 aspeed_i2c_get_clk_reg_val(u32 divider_ratio)
>> +{
>> + unsigned int inc = 0, div;
>> + u32 scl_low, scl_high, data;
>> +
>> + for (div = 0; divider_ratio >= 16; div++) {
>> + inc |= (divider_ratio & 1);
>> + divider_ratio >>= 1;
>> + }
>> + divider_ratio += inc;
>> + scl_low = (divider_ratio >> 1) - 1;
>> + scl_high = divider_ratio - scl_low - 2;
>> + data = 0x77700300 | (scl_high << 16) | (scl_low << 12) | div;
>> + return data;
>> +}
>> +
>> +static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
>> + struct platform_device *pdev)
>> +{
>> + struct clk *pclk;
>> + u32 clk_freq;
>> + u32 divider_ratio;
>> + int ret;
>> +
>> + pclk = devm_clk_get(&pdev->dev, NULL);
>> + if (IS_ERR(pclk)) {
>> + dev_err(&pdev->dev, "clk_get failed\n");
>> + return PTR_ERR(pclk);
>> + }
>> + ret = of_property_read_u32(pdev->dev.of_node,
>> + "clock-frequency", &clk_freq);
>> + if (ret < 0) {
>> + dev_err(&pdev->dev,
>> + "Could not read clock-frequency property\n");
>> + clk_freq = 100000;
>> + }
>> + divider_ratio = clk_get_rate(pclk) / clk_freq;
>> + /* We just need the clock rate, we don't actually use the clk object. */
>> + devm_clk_put(&pdev->dev, pclk);
>> +
>> + /* Set AC Timing */
>> + if (clk_freq / 1000 > 400) {
>> + aspeed_i2c_write(bus, aspeed_i2c_read(bus,
>> + ASPEED_I2C_FUN_CTRL_REG) |
>> + ASPEED_I2CD_M_HIGH_SPEED_EN |
>> + ASPEED_I2CD_M_SDA_DRIVE_1T_EN |
>> + ASPEED_I2CD_SDA_DRIVE_1T_EN,
>> + ASPEED_I2C_FUN_CTRL_REG);
>> +
>> + aspeed_i2c_write(bus, 0x3, ASPEED_I2C_AC_TIMING_REG2);
>> + aspeed_i2c_write(bus, aspeed_i2c_get_clk_reg_val(divider_ratio),
>> + ASPEED_I2C_AC_TIMING_REG1);
>> + } else {
>> + aspeed_i2c_write(bus, aspeed_i2c_get_clk_reg_val(divider_ratio),
>> + ASPEED_I2C_AC_TIMING_REG1);
>> + aspeed_i2c_write(bus, ASPEED_NO_TIMEOUT_CTRL,
>> + ASPEED_I2C_AC_TIMING_REG2);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void noop(struct irq_data *data) { }
>> +
>> +static struct irq_chip aspeed_i2c_irqchip = {
>> + .name = "ast-i2c",
>> + .irq_unmask = noop,
>> + .irq_mask = noop,
>> +};
>> +
>> +static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>> +{
>> + struct aspeed_i2c_bus *bus;
>> + struct aspeed_i2c_controller *controller =
>> + dev_get_drvdata(pdev->dev.parent);
>> + struct resource *res;
>> + int ret, irq;
>> + u32 hwirq;
>> +
>> + bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
>> + if (!bus)
>> + return -ENOMEM;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + bus->base = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(bus->base))
>> + return PTR_ERR(bus->base);
>> +
>> + bus->irq = platform_get_irq(pdev, 0);
>> + if (bus->irq < 0)
>> + return -ENXIO;
>> + ret = of_property_read_u32(pdev->dev.of_node, "interrupts", &hwirq);
>> + if (ret < 0) {
>> + dev_err(&pdev->dev, "no I2C 'interrupts' property\n");
>> + return -ENXIO;
>> + }
>> + irq = irq_create_mapping(controller->irq_domain, hwirq);
>> + irq_set_chip_data(irq, controller);
>> + irq_set_chip_and_handler(irq, &aspeed_i2c_irqchip, handle_simple_irq);
>> + ret = devm_request_irq(&pdev->dev, bus->irq, aspeed_i2c_bus_irq,
>> + 0, dev_name(&pdev->dev), bus);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to request interrupt\n");
>> + return -ENXIO;
>> + }
>> +
>> + /* Initialize the I2C adapter */
>> + spin_lock_init(&bus->lock);
>> + init_completion(&bus->cmd_complete);
>> + bus->adap.owner = THIS_MODULE;
>> + bus->adap.retries = 0;
>> + bus->adap.timeout = 5;
>> + bus->adap.algo = &aspeed_i2c_algo;
>> + bus->adap.algo_data = bus;
>> + bus->adap.dev.parent = &pdev->dev;
>> + bus->adap.dev.of_node = pdev->dev.of_node;
>> + snprintf(bus->adap.name, sizeof(bus->adap.name), "Aspeed i2c");
>> +
>> + bus->dev = &pdev->dev;
>> +
>> + /* reset device: disable master & slave functions */
>> + aspeed_i2c_write(bus, 0, ASPEED_I2C_FUN_CTRL_REG);
>> +
>> + ret = aspeed_i2c_init_clk(bus, pdev);
>> + if (ret < 0)
>> + return ret;
>> +
>> + /* Enable Master Mode */
>> + aspeed_i2c_write(bus, aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG) |
>> + ASPEED_I2CD_MASTER_EN |
>> + ASPEED_I2CD_MULTI_MASTER_DIS, ASPEED_I2C_FUN_CTRL_REG);
>> +
>> + /* Set interrupt generation of I2C controller */
>> + aspeed_i2c_write(bus, ASPEED_I2CD_INTR_SDA_DL_TIMEOUT |
>> + ASPEED_I2CD_INTR_BUS_RECOVER_DONE |
>> + ASPEED_I2CD_INTR_SCL_TIMEOUT |
>> + ASPEED_I2CD_INTR_ABNORMAL |
>> + ASPEED_I2CD_INTR_NORMAL_STOP |
>> + ASPEED_I2CD_INTR_ARBIT_LOSS |
>> + ASPEED_I2CD_INTR_RX_DONE |
>> + ASPEED_I2CD_INTR_TX_NAK |
>> + ASPEED_I2CD_INTR_TX_ACK,
>> + ASPEED_I2C_INTR_CTRL_REG);
>> +
>> + ret = i2c_add_adapter(&bus->adap);
>> + if (ret < 0)
>> + return -ENXIO;
>> +
>> + platform_set_drvdata(pdev, bus);
>> +
>> + dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
>> + bus->adap.nr, bus->irq);
>> +
>> + return 0;
>> +}
>> +
>> +static int aspeed_i2c_remove_bus(struct platform_device *pdev)
>> +{
>> + struct aspeed_i2c_bus *bus = platform_get_drvdata(pdev);
>> +
>> + i2c_del_adapter(&bus->adap);
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id aspeed_i2c_bus_of_table[] = {
>> + { .compatible = "aspeed,ast2400-i2c-bus", },
>> + { .compatible = "aspeed,ast2500-i2c-bus", },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);
>> +
>> +static struct platform_driver aspeed_i2c_bus_driver = {
>> + .probe = aspeed_i2c_probe_bus,
>> + .remove = aspeed_i2c_remove_bus,
>> + .driver = {
>> + .name = "ast-i2c-bus",
>> + .of_match_table = aspeed_i2c_bus_of_table,
>> + },
>> +};
>> +
>> +static void aspeed_i2c_controller_irq(struct irq_desc *desc)
>> +{
>> + struct aspeed_i2c_controller *c = irq_desc_get_handler_data(desc);
>> + unsigned long p, status;
>> + unsigned int bus_irq;
>> +
>> + status = readl(c->base);
>> + for_each_set_bit(p, &status, ASPEED_I2C_NUM_BUS) {
>> + bus_irq = irq_find_mapping(c->irq_domain, p);
>> + generic_handle_irq(bus_irq);
>> + }
>> +}
>> +
>> +static int aspeed_i2c_probe_controller(struct platform_device *pdev)
>> +{
>> + struct aspeed_i2c_controller *controller;
>> + struct device_node *np;
>> + struct resource *res;
>> +
>> + controller = kzalloc(sizeof(*controller), GFP_KERNEL);
>
> use devm_kzalloc() may be ?
>
> C.
>
>> + if (!controller)
>> + return -ENOMEM;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + controller->base = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(controller->base))
>> + return PTR_ERR(controller->base);
>> +
>> + controller->irq = platform_get_irq(pdev, 0);
>> + if (controller->irq < 0)
>> + return -ENXIO;
>> +
>> + controller->irq_domain = irq_domain_add_linear(pdev->dev.of_node,
>> + ASPEED_I2C_NUM_BUS, &irq_domain_simple_ops, NULL);
>> + if (!controller->irq_domain)
>> + return -ENXIO;
>> + controller->irq_domain->name = "ast-i2c-domain";
>> +
>> + irq_set_chained_handler_and_data(controller->irq,
>> + aspeed_i2c_controller_irq, controller);
>> +
>> + controller->dev = &pdev->dev;
>> +
>> + platform_set_drvdata(pdev, controller);
>> +
>> + dev_info(controller->dev, "i2c controller registered, irq %d\n",
>> + controller->irq);
>> +
>> + for_each_child_of_node(pdev->dev.of_node, np) {
>> + of_platform_device_create(np, NULL, &pdev->dev);
>> + of_node_put(np);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int aspeed_i2c_remove_controller(struct platform_device *pdev)
>> +{
>> + struct aspeed_i2c_controller *controller = platform_get_drvdata(pdev);
>> +
>> + irq_domain_remove(controller->irq_domain);
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id aspeed_i2c_controller_of_table[] = {
>> + { .compatible = "aspeed,ast2400-i2c-controller", },
>> + { .compatible = "aspeed,ast2500-i2c-controller", },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, aspeed_i2c_controller_of_table);
>> +
>> +static struct platform_driver aspeed_i2c_controller_driver = {
>> + .probe = aspeed_i2c_probe_controller,
>> + .remove = aspeed_i2c_remove_controller,
>> + .driver = {
>> + .name = "ast-i2c-controller",
>> + .of_match_table = aspeed_i2c_controller_of_table,
>> + },
>> +};
>> +
>> +static int __init aspeed_i2c_driver_init(void)
>> +{
>> + int ret;
>> +
>> + ret = platform_driver_register(&aspeed_i2c_controller_driver);
>> + if (ret < 0)
>> + return ret;
>> + return platform_driver_register(&aspeed_i2c_bus_driver);
>> +}
>> +module_init(aspeed_i2c_driver_init);
>> +
>> +static void __exit aspeed_i2c_driver_exit(void)
>> +{
>> + platform_driver_unregister(&aspeed_i2c_bus_driver);
>> + platform_driver_unregister(&aspeed_i2c_controller_driver);
>> +}
>> +module_exit(aspeed_i2c_driver_exit);
>> +
>> +MODULE_AUTHOR("Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>");
>> +MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
>> +MODULE_LICENSE("GPL");
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver
From: Frank Rowand @ 2016-11-22 18:21 UTC (permalink / raw)
To: Sekhar Nori, Bartosz Golaszewski, Kevin Hilman, Michael Turquette,
Rob Herring, Mark Rutland, Peter Ujfalusi, Russell King
Cc: LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha,
Tomi Valkeinen, David Airlie, Laurent Pinchart, Sudeep Holla
In-Reply-To: <800171a8-2e2c-2afb-f96d-800a17eaa17a-l0cyMroinI0@public.gmane.org>
On 11/21/16 22:25, Sekhar Nori wrote:
> Hi Frank,
>
> On Tuesday 22 November 2016 07:13 AM, Frank Rowand wrote:
>> On 11/21/16 08:33, Sekhar Nori wrote:
>>> On Monday 31 October 2016 08:15 PM, Bartosz Golaszewski wrote:
>>>> +static int da8xx_ddrctl_probe(struct platform_device *pdev)
>>>> +{
>>>> + const struct da8xx_ddrctl_config_knob *knob;
>>>> + const struct da8xx_ddrctl_setting *setting;
>>>> + struct device_node *node;
>>>> + struct resource *res;
>>>> + void __iomem *ddrctl;
>>>> + struct device *dev;
>>>> + u32 reg;
>>>> +
>>>> + dev = &pdev->dev;
>>>> + node = dev->of_node;
>>>> +
>>>> + setting = da8xx_ddrctl_get_board_settings();
>>>> + if (!setting) {
>>>> + dev_err(dev, "no settings for board '%s'\n",
>>>> + of_flat_dt_get_machine_name());
>>>> + return -EINVAL;
>>>> + }
>>>
>>> This causes a section mismatch because of_flat_dt_get_machine_name()
>>> has an __init annotation. I did not notice that before, sorry.
>>>
>>> It can be fixed with a patch like below:
>>>
>>> ---8<---
>>> diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c
>>> index a20e7bbbcbe0..9ca5aab3ac54 100644
>>> --- a/drivers/memory/da8xx-ddrctl.c
>>> +++ b/drivers/memory/da8xx-ddrctl.c
>>> @@ -102,6 +102,18 @@ static const struct da8xx_ddrctl_setting *da8xx_ddrctl_get_board_settings(void)
>>> return NULL;
>>> }
>>>
>>> +static const char* da8xx_ddrctl_get_machine_name(void)
>>> +{
>>> + const char *str;
>>> + int ret;
>>> +
>>> + ret = of_property_read_string(of_root, "model", &str);
>>> + if (ret)
>>> + ret = of_property_read_string(of_root, "compatible", &str);
>>> +
>>> + return str;
>>> +}
>>> +
>>> static int da8xx_ddrctl_probe(struct platform_device *pdev)
>>> {
>>> const struct da8xx_ddrctl_config_knob *knob;
>>> @@ -118,7 +130,7 @@ static int da8xx_ddrctl_probe(struct platform_device *pdev)
>>> setting = da8xx_ddrctl_get_board_settings();
>>> if (!setting) {
>>> dev_err(dev, "no settings for board '%s'\n",
>>> - of_flat_dt_get_machine_name());
>>
>> da8xx_ddrctl_get_board_settings() tries to match based on the "compatible"
>> property in the root node. The "model" property in the root node has
>> nothing to do with the failure to match. So creating and then using
>> da8xx_ddrctl_get_machine_name() to potentially report model is not useful.
>>
>> It should be sufficient to simply report that no compatible matched.
>
> I agree with you on this. Even if model name is printed, you will have
> to go back and check the compatible anyway. But I think it will be
> useful to print the compatible instead of just reporting that nothing
> matched.
>
> Bartosz, if you agree too, could you send a fix patch just printing the
> compatible?
Please note that the compatible property might contain several strings, not just
a single string.
>
> Thanks,
> Sekhar
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] PM / Domains: Introduce domain-performance-state binding
From: Kevin Hilman @ 2016-11-22 18:12 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rob Herring, Rafael Wysocki, linaro-kernel, linux-pm,
linux-kernel, Mark Rutland, Ulf Hansson, Vincent Guittot,
Lina Iyer, devicetree, Stephen Boyd, Nayak Rajendra
In-Reply-To: <20161122031717.GE10014@vireshk-i7>
Viresh Kumar <viresh.kumar@linaro.org> writes:
> On 21-11-16, 09:07, Rob Herring wrote:
>> On Fri, Nov 18, 2016 at 02:53:12PM +0530, Viresh Kumar wrote:
>> > Some platforms have the capability to configure the performance state of
>> > their Power Domains. The performance levels are represented by positive
>> > integer values, a lower value represents lower performance state.
>> >
>> > The power-domains until now were only concentrating on the idle state
>> > management of the device and this needs to change in order to reuse the
>> > infrastructure of power domains for active state management.
>> >
>> > This patch introduces a new optional property for the consumers of the
>> > power-domains: domain-performance-state.
>> >
>> > If the consumers don't need the capability of switching to different
>> > domain performance states at runtime, then they can simply define their
>> > required domain performance state in their node directly. Otherwise the
>> > consumers can define their requirements with help of other
>> > infrastructure, for example the OPP table.
>> >
>> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> > ---
>> > Documentation/devicetree/bindings/power/power_domain.txt | 6 ++++++
>> > 1 file changed, 6 insertions(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>> > index e1650364b296..db42eacf8b5c 100644
>> > --- a/Documentation/devicetree/bindings/power/power_domain.txt
>> > +++ b/Documentation/devicetree/bindings/power/power_domain.txt
>> > @@ -106,6 +106,12 @@ domain provided by the 'parent' power controller.
>> > - power-domains : A phandle and PM domain specifier as defined by bindings of
>> > the power controller specified by phandle.
>> >
>> > +Optional properties:
>> > +- domain-performance-state: A positive integer value representing the minimum
>> > + performance level (of the parent domain) required by the consumer for its
>> > + working. The integer value '1' represents the lowest performance level and the
>> > + highest value represents the highest performance level.
>>
>> How does one come up with the range of values?
>
> Why would we need a range here? The value here represents the minimum 'state'
> and the assumption is that everything above that level would be fine. So the
> range is automatically: domain-performance-state -> MAX.
>
>> It seems like you are
>> just making up numbers. Couldn't the domain performance level be an OPP
>> in the sense that it is a collection of clock frequencies and voltage
>> settings?
>
> The clock is going to be handled by the device itself (at least for the case we
> have today) and the performance-state lies with the power-domain which is
> configured separately. If the performance level includes both clk and voltage,
> then why would we need to show the clock rates in the DT ? Wouldn't a
> performance level be enough in such cases?
I think the question is: what does the performance-level of a domain
actually mean? Or, what are the units?
Depending on the SoC, there's probably a few things this could mean. It
might mean is that an underlying bus/interconnect can be configured to
guarantee a specific bandwidth or throughput. That in turn might mean
that that bus/interconnect might have to be set at a specific
frequency/voltage.
In your case, IIUC, you're just passing some magic value to some
firmware running on a micro-controller, but under the hood that uC is
probably configuring a frequency/voltage someplace.
So, if we're going to have a generic DT binding for this, it needs to be
something that's useful on platforms that are not using magic numbers
managed by a uC as well.
Kevin
^ permalink raw reply
* Re: [PATCH 5/10] dt: bindings: Add bindings for Marvell Xenon SD Host Controller
From: Gregory CLEMENT @ 2016-11-22 17:23 UTC (permalink / raw)
To: Rob Herring
Cc: Ziji Hu, Ulf Hansson, Adrian Hunter, linux-mmc, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, devicetree, Thomas Petazzoni,
linux-arm-kernel, Jack(SH) Zhu, Jimmy Xu, Jisheng Zhang,
Nadav Haklai, Ryan Gao, Doug Jones, Shiwu Zhang, Victor Gu,
Wei(SOCP) Liu, Wilson Ding, Xueping Liu
In-Reply-To: <15b06a12-ed69-03a7-ccc7-0c133ce1ac1e@marvell.com>
Hi Rob,
On jeu., nov. 10 2016, Ziji Hu <huziji@marvell.com> wrote:
[...]
>>> +
>>> +- reg:
>>> + * For "marvell,xenon-sdhci", one register area for Xenon IP.
>>> +
>>> + * For "marvell,armada-3700-sdhci", two register areas.
>>> + The first one for Xenon IP register. The second one for the Armada 3700 SOC
>>> + PHY PAD Voltage Control register.
>>> + Please follow the examples with compatible "marvell,armada-3700-sdhci"
>>> + in below.
>>> + Please also check property marvell,pad-type in below.
>>> +
>>> +Optional Properties:
>>> +- marvell,xenon-slotno:
>>
>> Multiple slots should be represented as child nodes IMO. I think some
>> other bindings already do this.
>>
>
> All the slots are entirely independent.
> I prefer to consider it as multiple independent SDHCs placed in
> a single IP, instead of that a IP contains multiple child slots.
It was indeed what I tried to show in my answer for the 1st version:
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-October/461860.html
Maybe you missed it.
You also mentioned other bindings using child nodes, but for this one
we have one controller with only one set of register with multiple slots
(Atmel is an example). Here each slot have it own set of register.
Actually giving the fact that each slot is controlled by a different set
of register I wonder why the hardware can't also deduce the slot number
from the address register. For me it looks like an hardware bug but we
have to deal with it.
Do you still think we needchild node here?
>
> It is unlike the implementation which put multiple slots behind PCIe EP interface. sdhci-pci.c will handle each slot init one by one.
> If Xenon SDHC slots are represented as child nodes, there should also be a main entry in Xenon driver to init each child node one by one.
> In my very own opinion, it is inconvenient and unnecessary.
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 5/7] add bindings for stm32 IIO timer drivers
From: Lee Jones @ 2016-11-22 17:18 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: Lars-Peter Clausen, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Mark Rutland,
alexandre.torgue-qxv4g6HH51o, devicetree-u79uwXL29TY76Z2rM5mHXA,
Linux Kernel Mailing List, Thierry Reding,
linux-pwm-u79uwXL29TY76Z2rM5mHXA, jic23-DgEjT+Ai2ygdnm+yROfE0A,
knaack.h-Mmb7MZpHnFY, Peter Meerwald-Stadler,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Fabrice Gasnier, Gerald Baeza, Arnaud Pouliquen, Linus Walleij,
Linaro Kernel Mailman List, Benjamin Gaignard
In-Reply-To: <CA+M3ks6ZoOND4VobU65OntEYU-f_XoNCV4wNZZ0_dYoOxy73+w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, 22 Nov 2016, Benjamin Gaignard wrote:
> [snip]
> >> + "st,stm32-iio-timer5"
> >> + "st,stm32-iio-timer6"
> >> + "st,stm32-iio-timer7"
> >> + "st,stm32-iio-timer8"
> >> + "st,stm32-iio-timer9"
> >> + "st,stm32-iio-timer10"
> >> + "st,stm32-iio-timer11"
> >> + "st,stm32-iio-timer12"
> >> + "st,stm32-iio-timer13"
> >> + "st,stm32-iio-timer14"
> >
> > We can't do this. This is a binding for a driver, not for the hardware.
> >
>
> Unfortunately each instance for the hardware IP have little
> differences like which triggers they could accept or size of the
> counter register,
> and I doesn't have value inside the hardware to distinguish them so
> the only way I found is to use compatible.
Can't you represent these as properties?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH 2/2] serial: imx: make DSR irq handling conditional
From: Christoph Fritz @ 2016-11-22 17:14 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Jiri Slaby,
Geert Uytterhoeven, Arnd Bergmann
Cc: u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1479834851-32442-1-git-send-email-chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
This patch makes use of device-tree property disable-dsr. Disabling
DSR can be necessary on i.MX6SX to quirk buggy hardware, for more
info see commit 276b891e3879 ("ARM: dts: imx6sx: document SION
necessity of ENET1_REF_CLK1").
Signed-off-by: Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
drivers/tty/serial/imx.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 0df2b1c..bd85a69a4 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -207,6 +207,7 @@ struct imx_port {
unsigned int dte_mode:1;
unsigned int irda_inv_rx:1;
unsigned int irda_inv_tx:1;
+ unsigned int dsr:1;
unsigned short trcv_delay; /* transceiver delay */
struct clk *clk_ipg;
struct clk *clk_per;
@@ -1219,7 +1220,8 @@ static int imx_startup(struct uart_port *port)
/*
* Finally, clear and enable interrupts
*/
- writel(USR1_RTSD | USR1_DTRD, sport->port.membase + USR1);
+ temp = USR1_RTSD | (sport->dsr ? USR1_DTRD : 0);
+ writel(temp, sport->port.membase + USR1);
writel(USR2_ORE, sport->port.membase + USR2);
if (sport->dma_is_inited && !sport->dma_is_enabled)
@@ -1259,7 +1261,7 @@ static int imx_startup(struct uart_port *port)
* now, too.
*/
temp |= IMX21_UCR3_RXDMUXSEL | UCR3_ADNIMP |
- UCR3_DTRDEN | UCR3_RI | UCR3_DCD;
+ (sport->dsr ? UCR3_DTRDEN : 0) | UCR3_RI | UCR3_DCD;
if (sport->dte_mode)
temp &= ~(UCR3_RI | UCR3_DCD);
@@ -1987,6 +1989,9 @@ static int serial_imx_probe_dt(struct imx_port *sport,
if (of_get_property(np, "fsl,dte-mode", NULL))
sport->dte_mode = 1;
+ if (!of_property_read_bool(np, "disable-dsr"))
+ sport->dsr = 1;
+
return 0;
}
#else
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 1/2] doc: DT: add generic serial property to disable DSR
From: Christoph Fritz @ 2016-11-22 17:14 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Jiri Slaby,
Geert Uytterhoeven, Arnd Bergmann
Cc: u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1479834851-32442-1-git-send-email-chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Introduce a generic serial property to disable DSR events which
can be necessary for buggy hardware.
Signed-off-by: Christoph Fritz <chf.fritz-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
Documentation/devicetree/bindings/serial/serial.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/serial/serial.txt b/Documentation/devicetree/bindings/serial/serial.txt
index fd970f7..26e274e 100644
--- a/Documentation/devicetree/bindings/serial/serial.txt
+++ b/Documentation/devicetree/bindings/serial/serial.txt
@@ -25,6 +25,8 @@ Optional properties:
Note that this property is mutually-exclusive with "cts-gpios" and
"rts-gpios" above.
+ - disable-dsr: The presence of this property disables DSR events reporting.
+
Examples:
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 0/2] serial: introduce DSR handling property
From: Christoph Fritz @ 2016-11-22 17:14 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Jiri Slaby,
Geert Uytterhoeven, Arnd Bergmann
Cc: u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
This patchset introduces device-tree property "disable-dsr" including
documentation and usage.
Christoph Fritz (2):
doc: DT: add generic serial property to disable DSR
serial: imx: make DSR irq handling conditional
Documentation/devicetree/bindings/serial/serial.txt | 2 ++
drivers/tty/serial/imx.c | 9 +++++++--
2 files changed, 9 insertions(+), 2 deletions(-)
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] ARM: dts: sunxi: Add num-cs for A20 spi nodes
From: Emmanuel Vadot @ 2016-11-22 17:06 UTC (permalink / raw)
To: robh+dt, mark.rutland, linux, maxime.ripard, wens
Cc: devicetree, Emmanuel Vadot, linux-kernel, linux-arm-kernel
The spi0 controller on the A20 have up to 4 CS (Chip Select) while the
others three only have 1.
Add the num-cs property to each node.
Signed-off-by: Emmanuel Vadot <manu@bidouilliste.com>
---
arch/arm/boot/dts/sun7i-a20.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 94cf5a1..ed21982 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -871,6 +871,7 @@
status = "disabled";
#address-cells = <1>;
#size-cells = <0>;
+ num-cs = 4;
};
spi1: spi@01c06000 {
@@ -885,6 +886,7 @@
status = "disabled";
#address-cells = <1>;
#size-cells = <0>;
+ num-cs = 1;
};
emac: ethernet@01c0b000 {
@@ -1037,6 +1039,7 @@
status = "disabled";
#address-cells = <1>;
#size-cells = <0>;
+ num-cs = 1;
};
ahci: sata@01c18000 {
@@ -1079,6 +1082,7 @@
status = "disabled";
#address-cells = <1>;
#size-cells = <0>;
+ num-cs = 1;
};
pio: pinctrl@01c20800 {
--
2.9.2
^ permalink raw reply related
* Re: [PATCH 5/7] add bindings for stm32 IIO timer drivers
From: Lars-Peter Clausen @ 2016-11-22 17:02 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: Mark Rutland, devicetree, Linaro Kernel Mailman List,
alexandre.torgue, linux-pwm, linux-iio, Linus Walleij,
Arnaud Pouliquen, Linux Kernel Mailing List, robh+dt,
Thierry Reding, linux-arm-kernel, Peter Meerwald-Stadler,
knaack.h, Gerald Baeza, Fabrice Gasnier, Lee Jones, jic23,
Benjamin Gaignard
In-Reply-To: <CA+M3ks6ZoOND4VobU65OntEYU-f_XoNCV4wNZZ0_dYoOxy73+w@mail.gmail.com>
On 11/22/2016 06:01 PM, Benjamin Gaignard wrote:
> [snip]
>>> + "st,stm32-iio-timer5"
>>> + "st,stm32-iio-timer6"
>>> + "st,stm32-iio-timer7"
>>> + "st,stm32-iio-timer8"
>>> + "st,stm32-iio-timer9"
>>> + "st,stm32-iio-timer10"
>>> + "st,stm32-iio-timer11"
>>> + "st,stm32-iio-timer12"
>>> + "st,stm32-iio-timer13"
>>> + "st,stm32-iio-timer14"
>>
>> We can't do this. This is a binding for a driver, not for the hardware.
>>
>
> Unfortunately each instance for the hardware IP have little
> differences like which triggers they could accept or size of the
> counter register,
> and I doesn't have value inside the hardware to distinguish them so
> the only way I found is to use compatible.
But IIO is not a piece of hardware, its a software framework in the Linux
kernel.
^ permalink raw reply
* Re: [PATCH 5/7] add bindings for stm32 IIO timer drivers
From: Benjamin Gaignard @ 2016-11-22 17:01 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Lee Jones, robh+dt, Mark Rutland, alexandre.torgue, devicetree,
Linux Kernel Mailing List, Thierry Reding, linux-pwm, jic23,
knaack.h, Peter Meerwald-Stadler, linux-iio, linux-arm-kernel,
Fabrice Gasnier, Gerald Baeza, Arnaud Pouliquen, Linus Walleij,
Linaro Kernel Mailman List, Benjamin Gaignard
In-Reply-To: <b9a82ce7-745a-1c25-f97f-68aaf0551f3b@metafoo.de>
[snip]
>> + "st,stm32-iio-timer5"
>> + "st,stm32-iio-timer6"
>> + "st,stm32-iio-timer7"
>> + "st,stm32-iio-timer8"
>> + "st,stm32-iio-timer9"
>> + "st,stm32-iio-timer10"
>> + "st,stm32-iio-timer11"
>> + "st,stm32-iio-timer12"
>> + "st,stm32-iio-timer13"
>> + "st,stm32-iio-timer14"
>
> We can't do this. This is a binding for a driver, not for the hardware.
>
Unfortunately each instance for the hardware IP have little
differences like which triggers they could accept or size of the
counter register,
and I doesn't have value inside the hardware to distinguish them so
the only way I found is to use compatible.
^ permalink raw reply
* Re: [PATCH 7/7] add stm32 multi-functions timer driver in DT
From: Alexandre Torgue @ 2016-11-22 17:00 UTC (permalink / raw)
To: Benjamin Gaignard, lee.jones, robh+dt, mark.rutland, devicetree,
linux-kernel, thierry.reding, linux-pwm, jic23, knaack.h, lars,
pmeerw, linux-iio, linux-arm-kernel
Cc: linaro-kernel, Benjamin Gaignard, linus.walleij, arnaud.pouliquen,
gerald.baeza, fabrice.gasnier
In-Reply-To: <1479831207-32699-8-git-send-email-benjamin.gaignard@st.com>
Hi Benjamin,
On 11/22/2016 05:13 PM, Benjamin Gaignard wrote:
> Add timers MFD and childs into DT for stm32f4.
> Define and enable pwm1 and pwm3 for stm32f469 discovery board
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
If you have to send a v2 for this series please change commit header by:
"ARM: dts: stm32: ..." (if not I will do it by myself)
> ---
> arch/arm/boot/dts/stm32f429.dtsi | 246 ++++++++++++++++++++++++++++++++++
> arch/arm/boot/dts/stm32f469-disco.dts | 29 ++++
> 2 files changed, 275 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> index bca491d..28a0fe9 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -355,6 +355,21 @@
> slew-rate = <2>;
> };
> };
> +
> + pwm1_pins: pwm@1 {
> + pins {
> + pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
> + <STM32F429_PB13_FUNC_TIM1_CH1N>,
> + <STM32F429_PB12_FUNC_TIM1_BKIN>;
> + };
> + };
> +
> + pwm3_pins: pwm@3 {
> + pins {
> + pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
> + <STM32F429_PB5_FUNC_TIM3_CH2>;
> + };
> + };
> };
>
> rcc: rcc@40023810 {
> @@ -426,6 +441,237 @@
> interrupts = <80>;
> clocks = <&rcc 0 38>;
> };
> +
> + mfd_timer1: mfdtimer1@40010000 {
> + compatible = "st,stm32-mfd-timer1";
> + reg = <0x40010000 0x400>;
> + clocks = <&rcc 0 160>;
> + clock-names = "mfd_timer_clk";
> + interrupts = <27>;
> + status = "disabled";
> +
> + pwm1: pwm1@40010000 {
> + compatible = "st,stm32-pwm1";
> + status = "disabled";
> + };
> +
> + iiotimer1: iiotimer1@40010000 {
> + compatible = "st,stm32-iio-timer1";
> + status = "disabled";
> + };
> + };
> +
> + mfd_timer2: mfdtimer2@40000000 {
> + compatible = "st,stm32-mfd-timer2";
> + reg = <0x40000000 0x400>;
> + clocks = <&rcc 0 128>;
> + clock-names = "mfd_timer_clk";
> + interrupts = <28>;
> + status = "disabled";
> +
> + pwm2: pwm2@40000000 {
> + compatible = "st,stm32-pwm2";
> + status = "disabled";
> + };
> + iiotimer2: iiotimer2@40000000 {
> + compatible = "st,stm32-iio-timer2";
> + status = "disabled";
> + };
> + };
> +
> + mfd_timer3: mfdtimer3@40000400 {
> + compatible = "st,stm32-mfd-timer3";
> + reg = <0x40000400 0x400>;
> + clocks = <&rcc 0 129>;
> + clock-names = "mfd_timer_clk";
> + interrupts = <29>;
> + status = "disabled";
> +
> + pwm3: pwm3@40000400 {
> + compatible = "st,stm32-pwm3";
> + status = "disabled";
> + };
> + iiotimer3: iiotimer3@40000400 {
> + compatible = "st,stm32-iio-timer3";
> + status = "disabled";
> + };
> + };
> +
> + mfd_timer4: mfdtimer4@40000800 {
> + compatible = "st,stm32-mfd-timer4";
> + reg = <0x40000800 0x400>;
> + clocks = <&rcc 0 130>;
> + clock-names = "mfd_timer_clk";
> + interrupts = <30>;
> + status = "disabled";
> +
> + pwm4: pwm4@40000800 {
> + compatible = "st,stm32-pwm4";
> + status = "disabled";
> + };
> + iiotimer4: iiotimer4@40000800 {
> + compatible = "st,stm32-iio-timer4";
> + status = "disabled";
> + };
> + };
> +
> + mfd_timer5: mfdtimer5@40000C00 {
> + compatible = "st,stm32-mfd-timer5";
> + reg = <0x40000C00 0x400>;
> + clocks = <&rcc 0 131>;
> + clock-names = "mfd_timer_clk";
> + interrupts = <50>;
> + status = "disabled";
> +
> + pwm5: pwm5@40000C00 {
> + compatible = "st,stm32-pwm5";
> + status = "disabled";
> + };
> + iiotimer5: iiotimer5@40000800 {
> + compatible = "st,stm32-iio-timer5";
> + status = "disabled";
> + };
> + };
> +
> + mfd_timer6: mfdtimer6@40001000 {
> + compatible = "st,stm32-mfd-timer6";
> + reg = <0x40001000 0x400>;
> + clocks = <&rcc 0 132>;
> + clock-names = "mfd_timer_clk";
> + interrupts = <54>;
> + status = "disabled";
> +
> + iiotimer6: iiotimer6@40001000 {
> + compatible = "st,stm32-iio-timer6";
> + status = "disabled";
> + };
> + };
> +
> + mfd_timer7: mfdtimer7@40001400 {
> + compatible = "st,stm32-mfd-timer7";
> + reg = <0x40001400 0x400>;
> + clocks = <&rcc 0 133>;
> + clock-names = "mfd_timer_clk";
> + interrupts = <55>;
> + status = "disabled";
> +
> + iiotimer7: iiotimer7@40001400 {
> + compatible = "st,stm32-iio-timer7";
> + status = "disabled";
> + };
> + };
> +
> + mfd_timer8: mfdtimer8@40010400 {
> + compatible = "st,stm32-mfd-timer8";
> + reg = <0x40010400 0x400>;
> + clocks = <&rcc 0 161>;
> + clock-names = "mfd_timer_clk";
> + interrupts = <46>;
> + status = "disabled";
> +
> + pwm8: pwm8@40010400 {
> + compatible = "st,stm32-pwm8";
> + status = "disabled";
> + };
> +
> + iiotimer8: iiotimer7@40010400 {
> + compatible = "st,stm32-iio-timer8";
> + status = "disabled";
> + };
> + };
> +
> + mfd_timer9: mfdtimer9@40014000 {
> + compatible = "st,stm32-mfd-timer9";
> + reg = <0x40014000 0x400>;
> + clocks = <&rcc 0 176>;
> + clock-names = "mfd_timer_clk";
> + interrupts = <24>;
> + status = "disabled";
> +
> + pwm9: pwm9@40014000 {
> + compatible = "st,stm32-pwm9";
> + status = "disabled";
> + };
> +
> + iiotimer9: iiotimer9@40014000 {
> + compatible = "st,stm32-iio-timer9";
> + status = "disabled";
> + };
> + };
> +
> + mfd_timer10: mfdtimer10@40014400 {
> + compatible = "st,stm32-mfd-timer10";
> + reg = <0x40014400 0x400>;
> + clocks = <&rcc 0 177>;
> + clock-names = "mfd_timer_clk";
> + interrupts = <25>;
> + status = "disabled";
> +
> + pwm10: pwm10@40014400 {
> + compatible = "st,stm32-pwm10";
> + status = "disabled";
> + };
> + };
> +
> + mfd_timer11: mfdtimer11@40014800 {
> + compatible = "st,stm32-mfd-timer11";
> + reg = <0x40014800 0x400>;
> + clocks = <&rcc 0 178>;
> + clock-names = "mfd_timer_clk";
> + interrupts = <26>;
> + status = "disabled";
> +
> + pwm11: pwm11@40014800 {
> + compatible = "st,stm32-pwm11";
> + status = "disabled";
> + };
> + };
> +
> + mfd_timer12: mfdtimer12@40001800 {
> + compatible = "st,stm32-mfd-timer12";
> + reg = <0x40001800 0x400>;
> + clocks = <&rcc 0 134>;
> + clock-names = "mfd_timer_clk";
> + interrupts = <43>;
> + status = "disabled";
> +
> + pwm12: pwm12@40001800 {
> + compatible = "st,stm32-pwm12";
> + status = "disabled";
> + };
> + iiotimer12: iiotimer12@40001800 {
> + compatible = "st,stm32-iio-timer12";
> + status = "disabled";
> + };
> + };
> +
> + mfd_timer13: mfdtimer13@40001C00 {
> + compatible = "st,stm32-mfd-timer13";
> + reg = <0x40001C00 0x400>;
> + clocks = <&rcc 0 135>;
> + clock-names = "mfd_timer_clk";
> + interrupts = <44>;
> + status = "disabled";
> +
> + pwm13: pwm13@40001C00 {
> + compatible = "st,stm32-pwm13";
> + status = "disabled";
> + };
> + };
> +
> + mfd_timer14: mfdtimer14@40002000 {
> + compatible = "st,stm32-mfd-timer14";
> + reg = <0x40002000 0x400>;
> + clocks = <&rcc 0 136>;
> + clock-names = "mfd_timer_clk";
> + interrupts = <45>;
> + status = "disabled";
> +
> + pwm14: pwm14@40002000 {
> + compatible = "st,stm32-pwm14";
> + status = "disabled";
> + };
> + };
> };
> };
>
> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
> index 8a163d7..a8f1788 100644
> --- a/arch/arm/boot/dts/stm32f469-disco.dts
> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
> @@ -81,3 +81,32 @@
> &usart3 {
> status = "okay";
> };
> +
> +&mfd_timer1 {
> + status = "okay";
> +};
> +
> +&pwm1 {
> + pinctrl-0 = <&pwm1_pins>;
> + pinctrl-names = "default";
> + st,breakinput-polarity = <0>;
> + status = "okay";
> +};
> +
> +&iiotimer1 {
> + status = "okay";
> +};
> +
> +&mfd_timer3 {
> + status = "okay";
> +};
> +
> +&pwm3 {
> + pinctrl-0 = <&pwm3_pins>;
> + pinctrl-names = "default";
> + status = "okay";
> +};
> +
> +&iiotimer3 {
> + status = "okay";
> +};
>
^ permalink raw reply
* [RFC PATCH 2/2] arm64: dts: enable the MUSB controller of Pine64 in host-only mode
From: Icenowy Zheng @ 2016-11-22 16:59 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Hans de Goede
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20161122165902.62543-1-icenowy-ymACFijhrKM@public.gmane.org>
A64 has a MUSB controller wired to the USB PHY 0, which is connected
to the upper USB Type-A port of Pine64.
As the port is a Type-A female port, enable it in host-only mode in the
device tree, which makes devices with USB Type-A male port can work on
this port (which is originally designed by Pine64 team).
Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
---
Peripheral mode is also proven to work, with dr_mode changed to "peripheral"
and using a Type-A to Type-A cable.
arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
index f9a11e6..cf91051 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
@@ -81,6 +81,11 @@
status = "okay";
};
+&usb_otg {
+ dr_mode = "host";
+ status = "okay";
+};
+
&usbphy {
status = "okay";
};
--
2.10.2
^ permalink raw reply related
* [RFC PATCH 1/2] arm64: dts: add MUSB node to Allwinner A64 dtsi
From: Icenowy Zheng @ 2016-11-22 16:59 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai, Hans de Goede
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
Allwinner A64 SoC has a MUSB controller like the one in A33, so add
a node for it, just use the compatible of A33 MUSB.
Host mode is tested to work properly on Pine64 and will be added into
the device tree of Pine64 in next patch.
Peripheral mode is also tested on Pine64, by changing dr_mode property
of usb_otg node and use a non-standard USB Type-A to Type-A cable.
Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
---
This patchset depends on my patch which adds usbphy to A64 dtsi.
( http://lists.infradead.org/pipermail/linux-arm-kernel/2016-November/469561.html )
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 2572dd6..261324a 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -122,6 +122,19 @@
#size-cells = <1>;
ranges;
+ usb_otg: usb@01c19000 {
+ compatible = "allwinner,sun8i-a33-musb";
+ reg = <0x01c19000 0x0400>;
+ clocks = <&ccu CLK_BUS_OTG>;
+ resets = <&ccu RST_BUS_OTG>;
+ interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "mc";
+ phys = <&usbphy 0>;
+ phy-names = "usb";
+ extcon = <&usbphy 0>;
+ status = "disabled";
+ };
+
usbphy: phy@01c19400 {
compatible = "allwinner,sun50i-a64-usb-phy";
reg = <0x01c19400 0x14>,
--
2.10.2
^ permalink raw reply related
* Re: [PATCH 5/7] add bindings for stm32 IIO timer drivers
From: Lars-Peter Clausen @ 2016-11-22 16:53 UTC (permalink / raw)
To: Benjamin Gaignard, lee.jones-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
alexandre.torgue-qxv4g6HH51o, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
linux-pwm-u79uwXL29TY76Z2rM5mHXA, jic23-DgEjT+Ai2ygdnm+yROfE0A,
knaack.h-Mmb7MZpHnFY, pmeerw-jW+XmwGofnusTnJN9+BGXg,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: fabrice.gasnier-qxv4g6HH51o, gerald.baeza-qxv4g6HH51o,
arnaud.pouliquen-qxv4g6HH51o,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
linaro-kernel-cunTk1MwBs8s++Sfvej+rw, Benjamin Gaignard
In-Reply-To: <1479831207-32699-6-git-send-email-benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
On 11/22/2016 05:13 PM, Benjamin Gaignard wrote:
> Define bindings for stm32 IIO timer
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
> ---
> .../bindings/iio/timer/stm32-iio-timer.txt | 33 ++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/timer/stm32-iio-timer.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/timer/stm32-iio-timer.txt b/Documentation/devicetree/bindings/iio/timer/stm32-iio-timer.txt
> new file mode 100644
> index 0000000..b80025e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/timer/stm32-iio-timer.txt
> @@ -0,0 +1,33 @@
> +timer IIO trigger bindings for STM32
> +
> +Must be a child of STM32 multifunctions timer driver
> +
> +Required parameters:
> +- compatible: must be one of the follow value:
> + "st,stm32-iio-timer1"
> + "st,stm32-iio-timer2"
> + "st,stm32-iio-timer3"
> + "st,stm32-iio-timer4"
> + "st,stm32-iio-timer5"
> + "st,stm32-iio-timer6"
> + "st,stm32-iio-timer7"
> + "st,stm32-iio-timer8"
> + "st,stm32-iio-timer9"
> + "st,stm32-iio-timer10"
> + "st,stm32-iio-timer11"
> + "st,stm32-iio-timer12"
> + "st,stm32-iio-timer13"
> + "st,stm32-iio-timer14"
We can't do this. This is a binding for a driver, not for the hardware.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/7] add binding for stm32 multifunctions timer driver
From: Lee Jones @ 2016-11-22 16:52 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: robh+dt, mark.rutland, alexandre.torgue, devicetree, linux-kernel,
thierry.reding, linux-pwm, jic23, knaack.h, lars, pmeerw,
linux-iio, linux-arm-kernel, fabrice.gasnier, gerald.baeza,
arnaud.pouliquen, linus.walleij, linaro-kernel, Benjamin Gaignard
In-Reply-To: <1479831207-32699-2-git-send-email-benjamin.gaignard@st.com>
On Tue, 22 Nov 2016, Benjamin Gaignard wrote:
> Add bindings information for stm32 timer MFD
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
> .../devicetree/bindings/mfd/stm32-timer.txt | 53 ++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/stm32-timer.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/stm32-timer.txt b/Documentation/devicetree/bindings/mfd/stm32-timer.txt
> new file mode 100644
> index 0000000..3cefce1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/stm32-timer.txt
> @@ -0,0 +1,53 @@
> +STM32 multifunctions timer driver
"STM32 Multi-Function Timer/PWM device bindings"
Doesn't this shared device have a better name?
> +stm32 timer MFD allow to handle at the same time pwm and IIO timer devices
No need for this sentence.
> +Required parameters:
> +- compatible: must be one of the follow value:
> + "st,stm32-mfd-timer1"
> + "st,stm32-mfd-timer2"
> + "st,stm32-mfd-timer3"
> + "st,stm32-mfd-timer4"
> + "st,stm32-mfd-timer5"
> + "st,stm32-mfd-timer6"
> + "st,stm32-mfd-timer7"
> + "st,stm32-mfd-timer8"
> + "st,stm32-mfd-timer9"
> + "st,stm32-mfd-timer10"
> + "st,stm32-mfd-timer11"
> + "st,stm32-mfd-timer12"
> + "st,stm32-mfd-timer13"
> + "st,stm32-mfd-timer14"
We don't normally number devices.
What's stopping you from simply doing:
pwm1: pwm1@40010000 {
compatible = "st,stm32-pwm";
};
pwm2: pwm1@40020000 {
compatible = "st,stm32-pwm";
};
pwm3: pwm1@40030000 {
compatible = "st,stm32-pwm";
};
> +- reg : Physical base address and length of the controller's
> + registers.
> +- clock-names: Set to "mfd_timer_clk".
How many clocks are there?
If only 1, you don't need this property.
"mfd_timer_clk" is not the correct name.
What is it called in the datasheet?
> +- clocks: Phandle of the clock used by the timer module.
"Phandle to the clock ..."
> + For Clk properties, please refer to [1].
> +- interrupts : Reference to the timer interrupt
Reference to?
See how other binding documents describe this property.
> +Optional parameters:
> +- resets : Reference to a reset controller asserting the timer
As above.
> +Optional subnodes:
Either use ":" or " :" or "<tab>:", but keep it consistent.
> +- pwm: See Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> +- iiotimer: See Documentation/devicetree/bindings/iio/timer/stm32-iio-timer.txt
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
Use the relative paths "../clock/", "../pwm/", "../iio/".
> +Example:
> + mfd_timer1: mfdtimer1@40010000 {
This is not an "MFD timer". MFD is a Linuxisum.
> + compatible = "st,stm32-mfd-timer1";
Better description required.
> + reg = <0x40010000 0x400>;
> + clocks = <&rcc 0 160>;
> + clock-names = "mfd_timer_clk";
> + interrupts = <27>;
> +
> + pwm1: pwm1@40010000 {
> + compatible = "st,stm32-pwm1";
> + };
> +
> + iiotimer1: iiotimer1@40010000 {
> + compatible = "st,stm32-iio-timer1";
> + };
> + };
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH 2/7] add MFD for stm32 timer IP
From: Lee Jones @ 2016-11-22 16:41 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
alexandre.torgue-qxv4g6HH51o, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
linux-pwm-u79uwXL29TY76Z2rM5mHXA, jic23-DgEjT+Ai2ygdnm+yROfE0A,
knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
pmeerw-jW+XmwGofnusTnJN9+BGXg, linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
fabrice.gasnier-qxv4g6HH51o, gerald.baeza-qxv4g6HH51o,
arnaud.pouliquen-qxv4g6HH51o,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
linaro-kernel-cunTk1MwBs8s++Sfvej+rw, Benjamin Gaignard
In-Reply-To: <20161122163018.GI10134-Re9dqnLqz4GzQB+pC5nmwQ@public.gmane.org>
On Tue, 22 Nov 2016, Lee Jones wrote:
> On Tue, 22 Nov 2016, Benjamin Gaignard wrote:
>
> > This hardware block could at used at same time for PWM generation
> > and IIO timer for other IPs like DAC, ADC or other timers.
> > PWM and IIO timer configuration are mixed in the same registers
> > so we need a MFD to be able to share those registers.
> >
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
> > ---
> > drivers/mfd/Kconfig | 10 ++
> > drivers/mfd/Makefile | 2 +
> > drivers/mfd/stm32-mfd-timer.c | 236 ++++++++++++++++++++++++++++++++++++
> > include/linux/mfd/stm32-mfd-timer.h | 78 ++++++++++++
> > 4 files changed, 326 insertions(+)
> > create mode 100644 drivers/mfd/stm32-mfd-timer.c
> > create mode 100644 include/linux/mfd/stm32-mfd-timer.h
>
> This driver is going to need a re-write.
>
> However, it's difficult to provide suggestions, since I've been left
> off of the Cc: list for all the other patches.
>
> Please re-send the set with all of the Maintainers Cc'ed on all of
> the patches.
Scrap that -- they all just came trickling through!
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index c6df644..63aee36 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -1607,6 +1607,15 @@ config MFD_STW481X
> > in various ST Microelectronics and ST-Ericsson embedded
> > Nomadik series.
> >
> > +config MFD_STM32_TIMER
> > + tristate "Support for STM32 multifunctions timer"
> > + select MFD_CORE
> > + select REGMAP
> > + depends on ARCH_STM32
> > + depends on OF
> > + help
> > + Select multifunction driver (pwm, IIO trigger) for stm32 timers
> > +
> > menu "Multimedia Capabilities Port drivers"
> > depends on ARCH_SA1100
> >
> > @@ -1644,4 +1653,5 @@ config MFD_VEXPRESS_SYSREG
> > on the ARM Ltd. Versatile Express board.
> >
> > endmenu
> > +
> > endif
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index 9834e66..b348c3e 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -211,3 +211,5 @@ obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
> > obj-$(CONFIG_MFD_MT6397) += mt6397-core.o
> >
> > obj-$(CONFIG_MFD_ALTERA_A10SR) += altera-a10sr.o
> > +
> > +obj-$(CONFIG_MFD_STM32_TIMER) += stm32-mfd-timer.o
> > diff --git a/drivers/mfd/stm32-mfd-timer.c b/drivers/mfd/stm32-mfd-timer.c
> > new file mode 100644
> > index 0000000..67e7db3
> > --- /dev/null
> > +++ b/drivers/mfd/stm32-mfd-timer.c
> > @@ -0,0 +1,236 @@
> > +/*
> > + * stm32-timer.c
> > + *
> > + * Copyright (C) STMicroelectronics 2016
> > + * Author: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org> for STMicroelectronics.
> > + * License terms: GNU General Public License (GPL), version 2
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +
> > +#include <linux/mfd/stm32-mfd-timer.h>
> > +
> > +static const struct stm32_mfd_timer_cfg mfd_cells_cfg[] = {
> > + {
> > + .pwm_name = "pwm1",
> > + .pwm_compatible = "st,stm32-pwm1",
> > + .trigger_name = "iiotimer1",
> > + .trigger_compatible = "st,stm32-iio-timer1",
> > + },
> > + {
> > + .pwm_name = "pwm2",
> > + .pwm_compatible = "st,stm32-pwm2",
> > + .trigger_name = "iiotimer2",
> > + .trigger_compatible = "st,stm32-iio-timer2",
> > + },
> > + {
> > + .pwm_name = "pwm3",
> > + .pwm_compatible = "st,stm32-pwm3",
> > + .trigger_name = "iiotimer3",
> > + .trigger_compatible = "st,stm32-iio-timer3",
> > + },
> > + {
> > + .pwm_name = "pwm4",
> > + .pwm_compatible = "st,stm32-pwm4",
> > + .trigger_name = "iiotimer4",
> > + .trigger_compatible = "st,stm32-iio-timer4",
> > + },
> > + {
> > + .pwm_name = "pwm5",
> > + .pwm_compatible = "st,stm32-pwm5",
> > + .trigger_name = "iiotimer5",
> > + .trigger_compatible = "st,stm32-iio-timer5",
> > + },
> > + {
> > + .trigger_name = "iiotimer6",
> > + .trigger_compatible = "st,stm32-iio-timer6",
> > + },
> > + {
> > + .trigger_name = "iiotimer7",
> > + .trigger_compatible = "st,stm32-iio-timer7",
> > + },
> > + {
> > + .pwm_name = "pwm8",
> > + .pwm_compatible = "st,stm32-pwm8",
> > + .trigger_name = "iiotimer8",
> > + .trigger_compatible = "st,stm32-iio-timer8",
> > + },
> > + {
> > + .pwm_name = "pwm9",
> > + .pwm_compatible = "st,stm32-pwm9",
> > + .trigger_name = "iiotimer9",
> > + .trigger_compatible = "st,stm32-iio-timer9",
> > + },
> > + {
> > + .pwm_name = "pwm10",
> > + .pwm_compatible = "st,stm32-pwm10",
> > + },
> > + {
> > + .pwm_name = "pwm11",
> > + .pwm_compatible = "st,stm32-pwm11",
> > + },
> > + {
> > + .pwm_name = "pwm12",
> > + .pwm_compatible = "st,stm32-pwm12",
> > + .trigger_name = "iiotimer12",
> > + .trigger_compatible = "st,stm32-iio-timer12",
> > + },
> > + {
> > + .pwm_name = "pwm13",
> > + .pwm_compatible = "st,stm32-pwm13",
> > + },
> > + {
> > + .pwm_name = "pwm14",
> > + .pwm_compatible = "st,stm32-pwm14",
> > + },
> > +};
> > +
> > +static const struct of_device_id stm32_timer_of_match[] = {
> > + {
> > + .compatible = "st,stm32-mfd-timer1",
> > + .data = &mfd_cells_cfg[0],
> > + },
> > + {
> > + .compatible = "st,stm32-mfd-timer2",
> > + .data = &mfd_cells_cfg[1],
> > + },
> > + {
> > + .compatible = "st,stm32-mfd-timer3",
> > + .data = &mfd_cells_cfg[2],
> > + },
> > + {
> > + .compatible = "st,stm32-mfd-timer4",
> > + .data = &mfd_cells_cfg[3],
> > + },
> > + {
> > + .compatible = "st,stm32-mfd-timer5",
> > + .data = &mfd_cells_cfg[4],
> > + },
> > + {
> > + .compatible = "st,stm32-mfd-timer6",
> > + .data = &mfd_cells_cfg[5],
> > + },
> > + {
> > + .compatible = "st,stm32-mfd-timer7",
> > + .data = &mfd_cells_cfg[6],
> > + },
> > + {
> > + .compatible = "st,stm32-mfd-timer8",
> > + .data = &mfd_cells_cfg[7],
> > + },
> > + {
> > + .compatible = "st,stm32-mfd-timer9",
> > + .data = &mfd_cells_cfg[8],
> > + },
> > + {
> > + .compatible = "st,stm32-mfd-timer10",
> > + .data = &mfd_cells_cfg[9],
> > + },
> > + {
> > + .compatible = "st,stm32-mfd-timer11",
> > + .data = &mfd_cells_cfg[10],
> > + },
> > + {
> > + .compatible = "st,stm32-mfd-timer12",
> > + .data = &mfd_cells_cfg[11],
> > + },
> > + {
> > + .compatible = "st,stm32-mfd-timer13",
> > + .data = &mfd_cells_cfg[12],
> > + },
> > + {
> > + .compatible = "st,stm32-mfd-timer14",
> > + .data = &mfd_cells_cfg[13],
> > + },
> > +};
> > +
> > +static const struct regmap_config stm32_timer_regmap_cfg = {
> > + .reg_bits = 32,
> > + .val_bits = 32,
> > + .reg_stride = sizeof(u32),
> > + .max_register = 0x400,
> > + .fast_io = true,
> > +};
> > +
> > +static int stm32_mfd_timer_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct device_node *np = dev->of_node;
> > + struct stm32_mfd_timer_dev *mfd;
> > + struct resource *res;
> > + int ret, nb_cells = 0;
> > + struct mfd_cell *cell = NULL;
> > + void __iomem *mmio;
> > +
> > + mfd = devm_kzalloc(dev, sizeof(*mfd), GFP_KERNEL);
> > + if (!mfd)
> > + return -ENOMEM;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + if (!res)
> > + return -ENOMEM;
> > +
> > + mmio = devm_ioremap_resource(dev, res);
> > + if (IS_ERR(mmio))
> > + return PTR_ERR(mmio);
> > +
> > + mfd->regmap = devm_regmap_init_mmio_clk(dev, "mfd_timer_clk", mmio,
> > + &stm32_timer_regmap_cfg);
> > + if (IS_ERR(mfd->regmap))
> > + return PTR_ERR(mfd->regmap);
> > +
> > + mfd->clk = devm_clk_get(dev, NULL);
> > + if (IS_ERR(mfd->clk))
> > + return PTR_ERR(mfd->clk);
> > +
> > + mfd->irq = platform_get_irq(pdev, 0);
> > + if (mfd->irq < 0)
> > + return -EINVAL;
> > +
> > + /* populate data structure depending on compatibility */
> > + if (!of_match_node(stm32_timer_of_match, np)->data)
> > + return -EINVAL;
> > +
> > + mfd->cfg =
> > + (struct stm32_mfd_timer_cfg *)of_match_node(stm32_timer_of_match, np)->data;
> > +
> > + if (mfd->cfg->pwm_name && mfd->cfg->pwm_compatible) {
> > + cell = &mfd->cells[nb_cells++];
> > + cell->name = mfd->cfg->pwm_name;
> > + cell->of_compatible = mfd->cfg->pwm_compatible;
> > + cell->platform_data = mfd;
> > + cell->pdata_size = sizeof(*mfd);
> > + }
> > +
> > + if (mfd->cfg->trigger_name && mfd->cfg->trigger_compatible) {
> > + cell = &mfd->cells[nb_cells++];
> > + cell->name = mfd->cfg->trigger_name;
> > + cell->of_compatible = mfd->cfg->trigger_compatible;
> > + cell->platform_data = mfd;
> > + cell->pdata_size = sizeof(*mfd);
> > + }
> > +
> > + ret = devm_mfd_add_devices(&pdev->dev, pdev->id, mfd->cells,
> > + nb_cells, NULL, 0, NULL);
> > + if (ret)
> > + return ret;
> > +
> > + platform_set_drvdata(pdev, mfd);
> > +
> > + return 0;
> > +}
> > +
> > +static struct platform_driver stm32_mfd_timer_driver = {
> > + .probe = stm32_mfd_timer_probe,
> > + .driver = {
> > + .name = "stm32-mfd-timer",
> > + .of_match_table = stm32_timer_of_match,
> > + },
> > +};
> > +module_platform_driver(stm32_mfd_timer_driver);
> > +
> > +MODULE_DESCRIPTION("STMicroelectronics STM32 Timer MFD");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/linux/mfd/stm32-mfd-timer.h b/include/linux/mfd/stm32-mfd-timer.h
> > new file mode 100644
> > index 0000000..4a79c22
> > --- /dev/null
> > +++ b/include/linux/mfd/stm32-mfd-timer.h
> > @@ -0,0 +1,78 @@
> > +/*
> > + * stm32-mfd-timer.h
> > + *
> > + * Copyright (C) STMicroelectronics 2016
> > + * Author: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org> for STMicroelectronics.
> > + * License terms: GNU General Public License (GPL), version 2
> > + */
> > +
> > +#ifndef _LINUX_MFD_STM32_TIMER_H_
> > +#define _LINUX_MFD_STM32_TIMER_H_
> > +
> > +#include <linux/clk.h>
> > +#include <linux/mfd/core.h>
> > +#include <linux/regmap.h>
> > +#include <linux/reset.h>
> > +
> > +#define TIM_CR1 0x00 /* Control Register 1 */
> > +#define TIM_CR2 0x04 /* Control Register 2 */
> > +#define TIM_SMCR 0x08 /* Slave mode control reg */
> > +#define TIM_DIER 0x0C /* DMA/interrupt register */
> > +#define TIM_SR 0x10 /* Status register */
> > +#define TIM_EGR 0x14 /* Event Generation Reg */
> > +#define TIM_CCMR1 0x18 /* Capt/Comp 1 Mode Reg */
> > +#define TIM_CCMR2 0x1C /* Capt/Comp 2 Mode Reg */
> > +#define TIM_CCER 0x20 /* Capt/Comp Enable Reg */
> > +#define TIM_PSC 0x28 /* Prescaler */
> > +#define TIM_ARR 0x2c /* Auto-Reload Register */
> > +#define TIM_CCR1 0x34 /* Capt/Comp Register 1 */
> > +#define TIM_CCR2 0x38 /* Capt/Comp Register 2 */
> > +#define TIM_CCR3 0x3C /* Capt/Comp Register 3 */
> > +#define TIM_CCR4 0x40 /* Capt/Comp Register 4 */
> > +#define TIM_BDTR 0x44 /* Break and Dead-Time Reg */
> > +
> > +#define TIM_CR1_CEN BIT(0) /* Counter Enable */
> > +#define TIM_CR1_ARPE BIT(7) /* Auto-reload Preload Ena */
> > +#define TIM_CR2_MMS (BIT(4) | BIT(5) | BIT(6)) /* Master mode selection */
> > +#define TIM_SMCR_SMS (BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
> > +#define TIM_SMCR_TS (BIT(4) | BIT(5) | BIT(6)) /* Trigger selection */
> > +#define TIM_DIER_UIE BIT(0) /* Update interrupt */
> > +#define TIM_SR_UIF BIT(0) /* Update interrupt flag */
> > +#define TIM_EGR_UG BIT(0) /* Update Generation */
> > +#define TIM_CCMR_PE BIT(3) /* Channel Preload Enable */
> > +#define TIM_CCMR_M1 (BIT(6) | BIT(5)) /* Channel PWM Mode 1 */
> > +#define TIM_CCER_CC1E BIT(0) /* Capt/Comp 1 out Ena */
> > +#define TIM_CCER_CC1P BIT(1) /* Capt/Comp 1 Polarity */
> > +#define TIM_CCER_CC1NE BIT(2) /* Capt/Comp 1N out Ena */
> > +#define TIM_CCER_CC1NP BIT(3) /* Capt/Comp 1N Polarity */
> > +#define TIM_CCER_CCXE (BIT(0) | BIT(4) | BIT(8) | BIT(12))
> > +#define TIM_BDTR_BKE BIT(12) /* Break input enable */
> > +#define TIM_BDTR_BKP BIT(13) /* Break input polarity */
> > +#define TIM_BDTR_AOE BIT(14) /* Automatic Output Enable */
> > +#define TIM_BDTR_MOE BIT(15) /* Main Output Enable */
> > +
> > +#define STM32_TIMER_CELLS 2
> > +#define MAX_TIM_PSC 0xFFFF
> > +
> > +struct stm32_mfd_timer_cfg {
> > + const char *pwm_name;
> > + const char *pwm_compatible;
> > + const char *trigger_name;
> > + const char *trigger_compatible;
> > +};
> > +
> > +struct stm32_mfd_timer_dev {
> > + /* Device data */
> > + struct device *dev;
> > + struct clk *clk;
> > + int irq;
> > +
> > + /* Registers mapping */
> > + struct regmap *regmap;
> > +
> > + /* Private data */
> > + struct mfd_cell cells[STM32_TIMER_CELLS];
> > + struct stm32_mfd_timer_cfg *cfg;
> > +};
> > +
> > +#endif
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH 2/7] add MFD for stm32 timer IP
From: Benjamin Gaignard @ 2016-11-22 16:40 UTC (permalink / raw)
To: Lee Jones
Cc: robh+dt, Mark Rutland, alexandre.torgue, devicetree,
Linux Kernel Mailing List, Thierry Reding, linux-pwm, jic23,
knaack.h, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio,
linux-arm-kernel, Fabrice Gasnier, Gerald Baeza, Arnaud Pouliquen,
Linus Walleij, Linaro Kernel Mailman List, Benjamin Gaignard
In-Reply-To: <20161122164106.GJ10134@dell.home>
Your comments are welcome on all of them ;-)
2016-11-22 17:41 GMT+01:00 Lee Jones <lee.jones@linaro.org>:
> On Tue, 22 Nov 2016, Lee Jones wrote:
>
>> On Tue, 22 Nov 2016, Benjamin Gaignard wrote:
>>
>> > This hardware block could at used at same time for PWM generation
>> > and IIO timer for other IPs like DAC, ADC or other timers.
>> > PWM and IIO timer configuration are mixed in the same registers
>> > so we need a MFD to be able to share those registers.
>> >
>> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> > ---
>> > drivers/mfd/Kconfig | 10 ++
>> > drivers/mfd/Makefile | 2 +
>> > drivers/mfd/stm32-mfd-timer.c | 236 ++++++++++++++++++++++++++++++++++++
>> > include/linux/mfd/stm32-mfd-timer.h | 78 ++++++++++++
>> > 4 files changed, 326 insertions(+)
>> > create mode 100644 drivers/mfd/stm32-mfd-timer.c
>> > create mode 100644 include/linux/mfd/stm32-mfd-timer.h
>>
>> This driver is going to need a re-write.
>>
>> However, it's difficult to provide suggestions, since I've been left
>> off of the Cc: list for all the other patches.
>>
>> Please re-send the set with all of the Maintainers Cc'ed on all of
>> the patches.
>
> Scrap that -- they all just came trickling through!
>
>> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> > index c6df644..63aee36 100644
>> > --- a/drivers/mfd/Kconfig
>> > +++ b/drivers/mfd/Kconfig
>> > @@ -1607,6 +1607,15 @@ config MFD_STW481X
>> > in various ST Microelectronics and ST-Ericsson embedded
>> > Nomadik series.
>> >
>> > +config MFD_STM32_TIMER
>> > + tristate "Support for STM32 multifunctions timer"
>> > + select MFD_CORE
>> > + select REGMAP
>> > + depends on ARCH_STM32
>> > + depends on OF
>> > + help
>> > + Select multifunction driver (pwm, IIO trigger) for stm32 timers
>> > +
>> > menu "Multimedia Capabilities Port drivers"
>> > depends on ARCH_SA1100
>> >
>> > @@ -1644,4 +1653,5 @@ config MFD_VEXPRESS_SYSREG
>> > on the ARM Ltd. Versatile Express board.
>> >
>> > endmenu
>> > +
>> > endif
>> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> > index 9834e66..b348c3e 100644
>> > --- a/drivers/mfd/Makefile
>> > +++ b/drivers/mfd/Makefile
>> > @@ -211,3 +211,5 @@ obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
>> > obj-$(CONFIG_MFD_MT6397) += mt6397-core.o
>> >
>> > obj-$(CONFIG_MFD_ALTERA_A10SR) += altera-a10sr.o
>> > +
>> > +obj-$(CONFIG_MFD_STM32_TIMER) += stm32-mfd-timer.o
>> > diff --git a/drivers/mfd/stm32-mfd-timer.c b/drivers/mfd/stm32-mfd-timer.c
>> > new file mode 100644
>> > index 0000000..67e7db3
>> > --- /dev/null
>> > +++ b/drivers/mfd/stm32-mfd-timer.c
>> > @@ -0,0 +1,236 @@
>> > +/*
>> > + * stm32-timer.c
>> > + *
>> > + * Copyright (C) STMicroelectronics 2016
>> > + * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
>> > + * License terms: GNU General Public License (GPL), version 2
>> > + */
>> > +
>> > +#include <linux/device.h>
>> > +#include <linux/init.h>
>> > +#include <linux/module.h>
>> > +#include <linux/of.h>
>> > +
>> > +#include <linux/mfd/stm32-mfd-timer.h>
>> > +
>> > +static const struct stm32_mfd_timer_cfg mfd_cells_cfg[] = {
>> > + {
>> > + .pwm_name = "pwm1",
>> > + .pwm_compatible = "st,stm32-pwm1",
>> > + .trigger_name = "iiotimer1",
>> > + .trigger_compatible = "st,stm32-iio-timer1",
>> > + },
>> > + {
>> > + .pwm_name = "pwm2",
>> > + .pwm_compatible = "st,stm32-pwm2",
>> > + .trigger_name = "iiotimer2",
>> > + .trigger_compatible = "st,stm32-iio-timer2",
>> > + },
>> > + {
>> > + .pwm_name = "pwm3",
>> > + .pwm_compatible = "st,stm32-pwm3",
>> > + .trigger_name = "iiotimer3",
>> > + .trigger_compatible = "st,stm32-iio-timer3",
>> > + },
>> > + {
>> > + .pwm_name = "pwm4",
>> > + .pwm_compatible = "st,stm32-pwm4",
>> > + .trigger_name = "iiotimer4",
>> > + .trigger_compatible = "st,stm32-iio-timer4",
>> > + },
>> > + {
>> > + .pwm_name = "pwm5",
>> > + .pwm_compatible = "st,stm32-pwm5",
>> > + .trigger_name = "iiotimer5",
>> > + .trigger_compatible = "st,stm32-iio-timer5",
>> > + },
>> > + {
>> > + .trigger_name = "iiotimer6",
>> > + .trigger_compatible = "st,stm32-iio-timer6",
>> > + },
>> > + {
>> > + .trigger_name = "iiotimer7",
>> > + .trigger_compatible = "st,stm32-iio-timer7",
>> > + },
>> > + {
>> > + .pwm_name = "pwm8",
>> > + .pwm_compatible = "st,stm32-pwm8",
>> > + .trigger_name = "iiotimer8",
>> > + .trigger_compatible = "st,stm32-iio-timer8",
>> > + },
>> > + {
>> > + .pwm_name = "pwm9",
>> > + .pwm_compatible = "st,stm32-pwm9",
>> > + .trigger_name = "iiotimer9",
>> > + .trigger_compatible = "st,stm32-iio-timer9",
>> > + },
>> > + {
>> > + .pwm_name = "pwm10",
>> > + .pwm_compatible = "st,stm32-pwm10",
>> > + },
>> > + {
>> > + .pwm_name = "pwm11",
>> > + .pwm_compatible = "st,stm32-pwm11",
>> > + },
>> > + {
>> > + .pwm_name = "pwm12",
>> > + .pwm_compatible = "st,stm32-pwm12",
>> > + .trigger_name = "iiotimer12",
>> > + .trigger_compatible = "st,stm32-iio-timer12",
>> > + },
>> > + {
>> > + .pwm_name = "pwm13",
>> > + .pwm_compatible = "st,stm32-pwm13",
>> > + },
>> > + {
>> > + .pwm_name = "pwm14",
>> > + .pwm_compatible = "st,stm32-pwm14",
>> > + },
>> > +};
>> > +
>> > +static const struct of_device_id stm32_timer_of_match[] = {
>> > + {
>> > + .compatible = "st,stm32-mfd-timer1",
>> > + .data = &mfd_cells_cfg[0],
>> > + },
>> > + {
>> > + .compatible = "st,stm32-mfd-timer2",
>> > + .data = &mfd_cells_cfg[1],
>> > + },
>> > + {
>> > + .compatible = "st,stm32-mfd-timer3",
>> > + .data = &mfd_cells_cfg[2],
>> > + },
>> > + {
>> > + .compatible = "st,stm32-mfd-timer4",
>> > + .data = &mfd_cells_cfg[3],
>> > + },
>> > + {
>> > + .compatible = "st,stm32-mfd-timer5",
>> > + .data = &mfd_cells_cfg[4],
>> > + },
>> > + {
>> > + .compatible = "st,stm32-mfd-timer6",
>> > + .data = &mfd_cells_cfg[5],
>> > + },
>> > + {
>> > + .compatible = "st,stm32-mfd-timer7",
>> > + .data = &mfd_cells_cfg[6],
>> > + },
>> > + {
>> > + .compatible = "st,stm32-mfd-timer8",
>> > + .data = &mfd_cells_cfg[7],
>> > + },
>> > + {
>> > + .compatible = "st,stm32-mfd-timer9",
>> > + .data = &mfd_cells_cfg[8],
>> > + },
>> > + {
>> > + .compatible = "st,stm32-mfd-timer10",
>> > + .data = &mfd_cells_cfg[9],
>> > + },
>> > + {
>> > + .compatible = "st,stm32-mfd-timer11",
>> > + .data = &mfd_cells_cfg[10],
>> > + },
>> > + {
>> > + .compatible = "st,stm32-mfd-timer12",
>> > + .data = &mfd_cells_cfg[11],
>> > + },
>> > + {
>> > + .compatible = "st,stm32-mfd-timer13",
>> > + .data = &mfd_cells_cfg[12],
>> > + },
>> > + {
>> > + .compatible = "st,stm32-mfd-timer14",
>> > + .data = &mfd_cells_cfg[13],
>> > + },
>> > +};
>> > +
>> > +static const struct regmap_config stm32_timer_regmap_cfg = {
>> > + .reg_bits = 32,
>> > + .val_bits = 32,
>> > + .reg_stride = sizeof(u32),
>> > + .max_register = 0x400,
>> > + .fast_io = true,
>> > +};
>> > +
>> > +static int stm32_mfd_timer_probe(struct platform_device *pdev)
>> > +{
>> > + struct device *dev = &pdev->dev;
>> > + struct device_node *np = dev->of_node;
>> > + struct stm32_mfd_timer_dev *mfd;
>> > + struct resource *res;
>> > + int ret, nb_cells = 0;
>> > + struct mfd_cell *cell = NULL;
>> > + void __iomem *mmio;
>> > +
>> > + mfd = devm_kzalloc(dev, sizeof(*mfd), GFP_KERNEL);
>> > + if (!mfd)
>> > + return -ENOMEM;
>> > +
>> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> > + if (!res)
>> > + return -ENOMEM;
>> > +
>> > + mmio = devm_ioremap_resource(dev, res);
>> > + if (IS_ERR(mmio))
>> > + return PTR_ERR(mmio);
>> > +
>> > + mfd->regmap = devm_regmap_init_mmio_clk(dev, "mfd_timer_clk", mmio,
>> > + &stm32_timer_regmap_cfg);
>> > + if (IS_ERR(mfd->regmap))
>> > + return PTR_ERR(mfd->regmap);
>> > +
>> > + mfd->clk = devm_clk_get(dev, NULL);
>> > + if (IS_ERR(mfd->clk))
>> > + return PTR_ERR(mfd->clk);
>> > +
>> > + mfd->irq = platform_get_irq(pdev, 0);
>> > + if (mfd->irq < 0)
>> > + return -EINVAL;
>> > +
>> > + /* populate data structure depending on compatibility */
>> > + if (!of_match_node(stm32_timer_of_match, np)->data)
>> > + return -EINVAL;
>> > +
>> > + mfd->cfg =
>> > + (struct stm32_mfd_timer_cfg *)of_match_node(stm32_timer_of_match, np)->data;
>> > +
>> > + if (mfd->cfg->pwm_name && mfd->cfg->pwm_compatible) {
>> > + cell = &mfd->cells[nb_cells++];
>> > + cell->name = mfd->cfg->pwm_name;
>> > + cell->of_compatible = mfd->cfg->pwm_compatible;
>> > + cell->platform_data = mfd;
>> > + cell->pdata_size = sizeof(*mfd);
>> > + }
>> > +
>> > + if (mfd->cfg->trigger_name && mfd->cfg->trigger_compatible) {
>> > + cell = &mfd->cells[nb_cells++];
>> > + cell->name = mfd->cfg->trigger_name;
>> > + cell->of_compatible = mfd->cfg->trigger_compatible;
>> > + cell->platform_data = mfd;
>> > + cell->pdata_size = sizeof(*mfd);
>> > + }
>> > +
>> > + ret = devm_mfd_add_devices(&pdev->dev, pdev->id, mfd->cells,
>> > + nb_cells, NULL, 0, NULL);
>> > + if (ret)
>> > + return ret;
>> > +
>> > + platform_set_drvdata(pdev, mfd);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static struct platform_driver stm32_mfd_timer_driver = {
>> > + .probe = stm32_mfd_timer_probe,
>> > + .driver = {
>> > + .name = "stm32-mfd-timer",
>> > + .of_match_table = stm32_timer_of_match,
>> > + },
>> > +};
>> > +module_platform_driver(stm32_mfd_timer_driver);
>> > +
>> > +MODULE_DESCRIPTION("STMicroelectronics STM32 Timer MFD");
>> > +MODULE_LICENSE("GPL");
>> > diff --git a/include/linux/mfd/stm32-mfd-timer.h b/include/linux/mfd/stm32-mfd-timer.h
>> > new file mode 100644
>> > index 0000000..4a79c22
>> > --- /dev/null
>> > +++ b/include/linux/mfd/stm32-mfd-timer.h
>> > @@ -0,0 +1,78 @@
>> > +/*
>> > + * stm32-mfd-timer.h
>> > + *
>> > + * Copyright (C) STMicroelectronics 2016
>> > + * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
>> > + * License terms: GNU General Public License (GPL), version 2
>> > + */
>> > +
>> > +#ifndef _LINUX_MFD_STM32_TIMER_H_
>> > +#define _LINUX_MFD_STM32_TIMER_H_
>> > +
>> > +#include <linux/clk.h>
>> > +#include <linux/mfd/core.h>
>> > +#include <linux/regmap.h>
>> > +#include <linux/reset.h>
>> > +
>> > +#define TIM_CR1 0x00 /* Control Register 1 */
>> > +#define TIM_CR2 0x04 /* Control Register 2 */
>> > +#define TIM_SMCR 0x08 /* Slave mode control reg */
>> > +#define TIM_DIER 0x0C /* DMA/interrupt register */
>> > +#define TIM_SR 0x10 /* Status register */
>> > +#define TIM_EGR 0x14 /* Event Generation Reg */
>> > +#define TIM_CCMR1 0x18 /* Capt/Comp 1 Mode Reg */
>> > +#define TIM_CCMR2 0x1C /* Capt/Comp 2 Mode Reg */
>> > +#define TIM_CCER 0x20 /* Capt/Comp Enable Reg */
>> > +#define TIM_PSC 0x28 /* Prescaler */
>> > +#define TIM_ARR 0x2c /* Auto-Reload Register */
>> > +#define TIM_CCR1 0x34 /* Capt/Comp Register 1 */
>> > +#define TIM_CCR2 0x38 /* Capt/Comp Register 2 */
>> > +#define TIM_CCR3 0x3C /* Capt/Comp Register 3 */
>> > +#define TIM_CCR4 0x40 /* Capt/Comp Register 4 */
>> > +#define TIM_BDTR 0x44 /* Break and Dead-Time Reg */
>> > +
>> > +#define TIM_CR1_CEN BIT(0) /* Counter Enable */
>> > +#define TIM_CR1_ARPE BIT(7) /* Auto-reload Preload Ena */
>> > +#define TIM_CR2_MMS (BIT(4) | BIT(5) | BIT(6)) /* Master mode selection */
>> > +#define TIM_SMCR_SMS (BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
>> > +#define TIM_SMCR_TS (BIT(4) | BIT(5) | BIT(6)) /* Trigger selection */
>> > +#define TIM_DIER_UIE BIT(0) /* Update interrupt */
>> > +#define TIM_SR_UIF BIT(0) /* Update interrupt flag */
>> > +#define TIM_EGR_UG BIT(0) /* Update Generation */
>> > +#define TIM_CCMR_PE BIT(3) /* Channel Preload Enable */
>> > +#define TIM_CCMR_M1 (BIT(6) | BIT(5)) /* Channel PWM Mode 1 */
>> > +#define TIM_CCER_CC1E BIT(0) /* Capt/Comp 1 out Ena */
>> > +#define TIM_CCER_CC1P BIT(1) /* Capt/Comp 1 Polarity */
>> > +#define TIM_CCER_CC1NE BIT(2) /* Capt/Comp 1N out Ena */
>> > +#define TIM_CCER_CC1NP BIT(3) /* Capt/Comp 1N Polarity */
>> > +#define TIM_CCER_CCXE (BIT(0) | BIT(4) | BIT(8) | BIT(12))
>> > +#define TIM_BDTR_BKE BIT(12) /* Break input enable */
>> > +#define TIM_BDTR_BKP BIT(13) /* Break input polarity */
>> > +#define TIM_BDTR_AOE BIT(14) /* Automatic Output Enable */
>> > +#define TIM_BDTR_MOE BIT(15) /* Main Output Enable */
>> > +
>> > +#define STM32_TIMER_CELLS 2
>> > +#define MAX_TIM_PSC 0xFFFF
>> > +
>> > +struct stm32_mfd_timer_cfg {
>> > + const char *pwm_name;
>> > + const char *pwm_compatible;
>> > + const char *trigger_name;
>> > + const char *trigger_compatible;
>> > +};
>> > +
>> > +struct stm32_mfd_timer_dev {
>> > + /* Device data */
>> > + struct device *dev;
>> > + struct clk *clk;
>> > + int irq;
>> > +
>> > + /* Registers mapping */
>> > + struct regmap *regmap;
>> > +
>> > + /* Private data */
>> > + struct mfd_cell cells[STM32_TIMER_CELLS];
>> > + struct stm32_mfd_timer_cfg *cfg;
>> > +};
>> > +
>> > +#endif
>>
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
--
Benjamin Gaignard
Graphic Study Group
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH 2/7] add MFD for stm32 timer IP
From: Lee Jones @ 2016-11-22 16:30 UTC (permalink / raw)
To: Benjamin Gaignard
Cc: robh+dt, mark.rutland, alexandre.torgue, devicetree, linux-kernel,
thierry.reding, linux-pwm, jic23, knaack.h, lars, pmeerw,
linux-iio, linux-arm-kernel, fabrice.gasnier, gerald.baeza,
arnaud.pouliquen, linus.walleij, linaro-kernel, Benjamin Gaignard
In-Reply-To: <1479831207-32699-3-git-send-email-benjamin.gaignard@st.com>
On Tue, 22 Nov 2016, Benjamin Gaignard wrote:
> This hardware block could at used at same time for PWM generation
> and IIO timer for other IPs like DAC, ADC or other timers.
> PWM and IIO timer configuration are mixed in the same registers
> so we need a MFD to be able to share those registers.
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
> drivers/mfd/Kconfig | 10 ++
> drivers/mfd/Makefile | 2 +
> drivers/mfd/stm32-mfd-timer.c | 236 ++++++++++++++++++++++++++++++++++++
> include/linux/mfd/stm32-mfd-timer.h | 78 ++++++++++++
> 4 files changed, 326 insertions(+)
> create mode 100644 drivers/mfd/stm32-mfd-timer.c
> create mode 100644 include/linux/mfd/stm32-mfd-timer.h
This driver is going to need a re-write.
However, it's difficult to provide suggestions, since I've been left
off of the Cc: list for all the other patches.
Please re-send the set with all of the Maintainers Cc'ed on all of
the patches.
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index c6df644..63aee36 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1607,6 +1607,15 @@ config MFD_STW481X
> in various ST Microelectronics and ST-Ericsson embedded
> Nomadik series.
>
> +config MFD_STM32_TIMER
> + tristate "Support for STM32 multifunctions timer"
> + select MFD_CORE
> + select REGMAP
> + depends on ARCH_STM32
> + depends on OF
> + help
> + Select multifunction driver (pwm, IIO trigger) for stm32 timers
> +
> menu "Multimedia Capabilities Port drivers"
> depends on ARCH_SA1100
>
> @@ -1644,4 +1653,5 @@ config MFD_VEXPRESS_SYSREG
> on the ARM Ltd. Versatile Express board.
>
> endmenu
> +
> endif
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 9834e66..b348c3e 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -211,3 +211,5 @@ obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
> obj-$(CONFIG_MFD_MT6397) += mt6397-core.o
>
> obj-$(CONFIG_MFD_ALTERA_A10SR) += altera-a10sr.o
> +
> +obj-$(CONFIG_MFD_STM32_TIMER) += stm32-mfd-timer.o
> diff --git a/drivers/mfd/stm32-mfd-timer.c b/drivers/mfd/stm32-mfd-timer.c
> new file mode 100644
> index 0000000..67e7db3
> --- /dev/null
> +++ b/drivers/mfd/stm32-mfd-timer.c
> @@ -0,0 +1,236 @@
> +/*
> + * stm32-timer.c
> + *
> + * Copyright (C) STMicroelectronics 2016
> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
> + * License terms: GNU General Public License (GPL), version 2
> + */
> +
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +
> +#include <linux/mfd/stm32-mfd-timer.h>
> +
> +static const struct stm32_mfd_timer_cfg mfd_cells_cfg[] = {
> + {
> + .pwm_name = "pwm1",
> + .pwm_compatible = "st,stm32-pwm1",
> + .trigger_name = "iiotimer1",
> + .trigger_compatible = "st,stm32-iio-timer1",
> + },
> + {
> + .pwm_name = "pwm2",
> + .pwm_compatible = "st,stm32-pwm2",
> + .trigger_name = "iiotimer2",
> + .trigger_compatible = "st,stm32-iio-timer2",
> + },
> + {
> + .pwm_name = "pwm3",
> + .pwm_compatible = "st,stm32-pwm3",
> + .trigger_name = "iiotimer3",
> + .trigger_compatible = "st,stm32-iio-timer3",
> + },
> + {
> + .pwm_name = "pwm4",
> + .pwm_compatible = "st,stm32-pwm4",
> + .trigger_name = "iiotimer4",
> + .trigger_compatible = "st,stm32-iio-timer4",
> + },
> + {
> + .pwm_name = "pwm5",
> + .pwm_compatible = "st,stm32-pwm5",
> + .trigger_name = "iiotimer5",
> + .trigger_compatible = "st,stm32-iio-timer5",
> + },
> + {
> + .trigger_name = "iiotimer6",
> + .trigger_compatible = "st,stm32-iio-timer6",
> + },
> + {
> + .trigger_name = "iiotimer7",
> + .trigger_compatible = "st,stm32-iio-timer7",
> + },
> + {
> + .pwm_name = "pwm8",
> + .pwm_compatible = "st,stm32-pwm8",
> + .trigger_name = "iiotimer8",
> + .trigger_compatible = "st,stm32-iio-timer8",
> + },
> + {
> + .pwm_name = "pwm9",
> + .pwm_compatible = "st,stm32-pwm9",
> + .trigger_name = "iiotimer9",
> + .trigger_compatible = "st,stm32-iio-timer9",
> + },
> + {
> + .pwm_name = "pwm10",
> + .pwm_compatible = "st,stm32-pwm10",
> + },
> + {
> + .pwm_name = "pwm11",
> + .pwm_compatible = "st,stm32-pwm11",
> + },
> + {
> + .pwm_name = "pwm12",
> + .pwm_compatible = "st,stm32-pwm12",
> + .trigger_name = "iiotimer12",
> + .trigger_compatible = "st,stm32-iio-timer12",
> + },
> + {
> + .pwm_name = "pwm13",
> + .pwm_compatible = "st,stm32-pwm13",
> + },
> + {
> + .pwm_name = "pwm14",
> + .pwm_compatible = "st,stm32-pwm14",
> + },
> +};
> +
> +static const struct of_device_id stm32_timer_of_match[] = {
> + {
> + .compatible = "st,stm32-mfd-timer1",
> + .data = &mfd_cells_cfg[0],
> + },
> + {
> + .compatible = "st,stm32-mfd-timer2",
> + .data = &mfd_cells_cfg[1],
> + },
> + {
> + .compatible = "st,stm32-mfd-timer3",
> + .data = &mfd_cells_cfg[2],
> + },
> + {
> + .compatible = "st,stm32-mfd-timer4",
> + .data = &mfd_cells_cfg[3],
> + },
> + {
> + .compatible = "st,stm32-mfd-timer5",
> + .data = &mfd_cells_cfg[4],
> + },
> + {
> + .compatible = "st,stm32-mfd-timer6",
> + .data = &mfd_cells_cfg[5],
> + },
> + {
> + .compatible = "st,stm32-mfd-timer7",
> + .data = &mfd_cells_cfg[6],
> + },
> + {
> + .compatible = "st,stm32-mfd-timer8",
> + .data = &mfd_cells_cfg[7],
> + },
> + {
> + .compatible = "st,stm32-mfd-timer9",
> + .data = &mfd_cells_cfg[8],
> + },
> + {
> + .compatible = "st,stm32-mfd-timer10",
> + .data = &mfd_cells_cfg[9],
> + },
> + {
> + .compatible = "st,stm32-mfd-timer11",
> + .data = &mfd_cells_cfg[10],
> + },
> + {
> + .compatible = "st,stm32-mfd-timer12",
> + .data = &mfd_cells_cfg[11],
> + },
> + {
> + .compatible = "st,stm32-mfd-timer13",
> + .data = &mfd_cells_cfg[12],
> + },
> + {
> + .compatible = "st,stm32-mfd-timer14",
> + .data = &mfd_cells_cfg[13],
> + },
> +};
> +
> +static const struct regmap_config stm32_timer_regmap_cfg = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = sizeof(u32),
> + .max_register = 0x400,
> + .fast_io = true,
> +};
> +
> +static int stm32_mfd_timer_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct stm32_mfd_timer_dev *mfd;
> + struct resource *res;
> + int ret, nb_cells = 0;
> + struct mfd_cell *cell = NULL;
> + void __iomem *mmio;
> +
> + mfd = devm_kzalloc(dev, sizeof(*mfd), GFP_KERNEL);
> + if (!mfd)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENOMEM;
> +
> + mmio = devm_ioremap_resource(dev, res);
> + if (IS_ERR(mmio))
> + return PTR_ERR(mmio);
> +
> + mfd->regmap = devm_regmap_init_mmio_clk(dev, "mfd_timer_clk", mmio,
> + &stm32_timer_regmap_cfg);
> + if (IS_ERR(mfd->regmap))
> + return PTR_ERR(mfd->regmap);
> +
> + mfd->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(mfd->clk))
> + return PTR_ERR(mfd->clk);
> +
> + mfd->irq = platform_get_irq(pdev, 0);
> + if (mfd->irq < 0)
> + return -EINVAL;
> +
> + /* populate data structure depending on compatibility */
> + if (!of_match_node(stm32_timer_of_match, np)->data)
> + return -EINVAL;
> +
> + mfd->cfg =
> + (struct stm32_mfd_timer_cfg *)of_match_node(stm32_timer_of_match, np)->data;
> +
> + if (mfd->cfg->pwm_name && mfd->cfg->pwm_compatible) {
> + cell = &mfd->cells[nb_cells++];
> + cell->name = mfd->cfg->pwm_name;
> + cell->of_compatible = mfd->cfg->pwm_compatible;
> + cell->platform_data = mfd;
> + cell->pdata_size = sizeof(*mfd);
> + }
> +
> + if (mfd->cfg->trigger_name && mfd->cfg->trigger_compatible) {
> + cell = &mfd->cells[nb_cells++];
> + cell->name = mfd->cfg->trigger_name;
> + cell->of_compatible = mfd->cfg->trigger_compatible;
> + cell->platform_data = mfd;
> + cell->pdata_size = sizeof(*mfd);
> + }
> +
> + ret = devm_mfd_add_devices(&pdev->dev, pdev->id, mfd->cells,
> + nb_cells, NULL, 0, NULL);
> + if (ret)
> + return ret;
> +
> + platform_set_drvdata(pdev, mfd);
> +
> + return 0;
> +}
> +
> +static struct platform_driver stm32_mfd_timer_driver = {
> + .probe = stm32_mfd_timer_probe,
> + .driver = {
> + .name = "stm32-mfd-timer",
> + .of_match_table = stm32_timer_of_match,
> + },
> +};
> +module_platform_driver(stm32_mfd_timer_driver);
> +
> +MODULE_DESCRIPTION("STMicroelectronics STM32 Timer MFD");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/stm32-mfd-timer.h b/include/linux/mfd/stm32-mfd-timer.h
> new file mode 100644
> index 0000000..4a79c22
> --- /dev/null
> +++ b/include/linux/mfd/stm32-mfd-timer.h
> @@ -0,0 +1,78 @@
> +/*
> + * stm32-mfd-timer.h
> + *
> + * Copyright (C) STMicroelectronics 2016
> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
> + * License terms: GNU General Public License (GPL), version 2
> + */
> +
> +#ifndef _LINUX_MFD_STM32_TIMER_H_
> +#define _LINUX_MFD_STM32_TIMER_H_
> +
> +#include <linux/clk.h>
> +#include <linux/mfd/core.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +
> +#define TIM_CR1 0x00 /* Control Register 1 */
> +#define TIM_CR2 0x04 /* Control Register 2 */
> +#define TIM_SMCR 0x08 /* Slave mode control reg */
> +#define TIM_DIER 0x0C /* DMA/interrupt register */
> +#define TIM_SR 0x10 /* Status register */
> +#define TIM_EGR 0x14 /* Event Generation Reg */
> +#define TIM_CCMR1 0x18 /* Capt/Comp 1 Mode Reg */
> +#define TIM_CCMR2 0x1C /* Capt/Comp 2 Mode Reg */
> +#define TIM_CCER 0x20 /* Capt/Comp Enable Reg */
> +#define TIM_PSC 0x28 /* Prescaler */
> +#define TIM_ARR 0x2c /* Auto-Reload Register */
> +#define TIM_CCR1 0x34 /* Capt/Comp Register 1 */
> +#define TIM_CCR2 0x38 /* Capt/Comp Register 2 */
> +#define TIM_CCR3 0x3C /* Capt/Comp Register 3 */
> +#define TIM_CCR4 0x40 /* Capt/Comp Register 4 */
> +#define TIM_BDTR 0x44 /* Break and Dead-Time Reg */
> +
> +#define TIM_CR1_CEN BIT(0) /* Counter Enable */
> +#define TIM_CR1_ARPE BIT(7) /* Auto-reload Preload Ena */
> +#define TIM_CR2_MMS (BIT(4) | BIT(5) | BIT(6)) /* Master mode selection */
> +#define TIM_SMCR_SMS (BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
> +#define TIM_SMCR_TS (BIT(4) | BIT(5) | BIT(6)) /* Trigger selection */
> +#define TIM_DIER_UIE BIT(0) /* Update interrupt */
> +#define TIM_SR_UIF BIT(0) /* Update interrupt flag */
> +#define TIM_EGR_UG BIT(0) /* Update Generation */
> +#define TIM_CCMR_PE BIT(3) /* Channel Preload Enable */
> +#define TIM_CCMR_M1 (BIT(6) | BIT(5)) /* Channel PWM Mode 1 */
> +#define TIM_CCER_CC1E BIT(0) /* Capt/Comp 1 out Ena */
> +#define TIM_CCER_CC1P BIT(1) /* Capt/Comp 1 Polarity */
> +#define TIM_CCER_CC1NE BIT(2) /* Capt/Comp 1N out Ena */
> +#define TIM_CCER_CC1NP BIT(3) /* Capt/Comp 1N Polarity */
> +#define TIM_CCER_CCXE (BIT(0) | BIT(4) | BIT(8) | BIT(12))
> +#define TIM_BDTR_BKE BIT(12) /* Break input enable */
> +#define TIM_BDTR_BKP BIT(13) /* Break input polarity */
> +#define TIM_BDTR_AOE BIT(14) /* Automatic Output Enable */
> +#define TIM_BDTR_MOE BIT(15) /* Main Output Enable */
> +
> +#define STM32_TIMER_CELLS 2
> +#define MAX_TIM_PSC 0xFFFF
> +
> +struct stm32_mfd_timer_cfg {
> + const char *pwm_name;
> + const char *pwm_compatible;
> + const char *trigger_name;
> + const char *trigger_compatible;
> +};
> +
> +struct stm32_mfd_timer_dev {
> + /* Device data */
> + struct device *dev;
> + struct clk *clk;
> + int irq;
> +
> + /* Registers mapping */
> + struct regmap *regmap;
> +
> + /* Private data */
> + struct mfd_cell cells[STM32_TIMER_CELLS];
> + struct stm32_mfd_timer_cfg *cfg;
> +};
> +
> +#endif
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH v2 1/2] ARM64: dts: Add support for Meson GXM
From: Kevin Hilman @ 2016-11-22 16:27 UTC (permalink / raw)
To: Neil Armstrong
Cc: carlo-KA+7E9HrN00dnm+yROfE0A,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161122100046.25899-2-narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> writes:
> Following the Amlogic Linux kernel, it seem the only differences
> between the GXL and GXM SoCs are the CPU Clusters.
>
> This commit renames the gxl-s905d-p23x DTSI in a common file for
> S905D p23x and S912 q20x boards.
>
> Then adds a meson-gxm dtsi and reproduce the P23x to Q20x boards
> dts files since the S905D and S912 SoCs shares the same pinout
> and the P23x and Q20x boards are identical.
>
> Signed-off-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
Applied v2 of both patches,
Kevin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] ARM64: dts: meson-gxl: Add support for Nexbox A95X
From: Kevin Hilman @ 2016-11-22 16:24 UTC (permalink / raw)
To: Neil Armstrong
Cc: carlo, linux-amlogic, linux-arm-kernel, linux-kernel, devicetree
In-Reply-To: <20161122114154.17566-1-narmstrong@baylibre.com>
Neil Armstrong <narmstrong@baylibre.com> writes:
> The Nexbox A95X exists with a Meson GXBB (S905) Soc or a Meson GXL SoC (S905X).
> Add the S905X variant which uses the internal PHY instead of an external PHY.
Missing SoB?
Kevin
^ 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