Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 3/3] ASoC: ti: Add custom machine driver for j721e EVM (CPB and IVI)
From: Peter Ujfalusi @ 2020-05-12 13:16 UTC (permalink / raw)
  To: broonie, lgirdwood, robh+dt; +Cc: alsa-devel, devicetree, linux-kernel
In-Reply-To: <20200512131633.32668-1-peter.ujfalusi@ti.com>

The audio support on the board is using pcm3168a codec connected to McASP10
serializers in parallel setup.
The pcm3168a SCKI clock is coming via the j721e AUDIO_REFCLK2 pin.
In order to support 48KHz and 44.1KHz family of sampling rates the parent clock
for AUDIO_REFCLK2 needs to be changed between PLL4 (for 48KHz) and PLL15 (for
44.1KHz). The same PLLs are used for McASP10's AUXCLK clock via different
HSDIVIDER.

Generic card can not be used for the board as we need to switch between
clock paths for different sampling rate families and also need to change
the slot_width between 16 and 24 bit audio.

The audio support on the Infotainment Expansion Board consists of McASP0
connected to two pcm3168a codecs with dedicated set of serializers to each.
The SCKI for pcm3168a is sourced from j721e AUDIO_REFCLK0 pin.
It is extending the audio support on the CPB.

Due to the fact that the same PLL4/15 is used by both domains (CPB/IVI)
there are cross restriction on sampling rates.

The IVI side is represented as multicodec setup.

PCMs available on a plain CPB (no IVI addon):
hw:0,0 - cpb playback (8 channels)
hw:0,1 - cpb capture (6 channels)

When the IVI addon is present, additional two PCMs will be present:
hw:0,2 - ivi multicodec playback (16 channels)
hw:0,3 - ivi multicodec capture (12 channels)

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/ti/Kconfig     |   8 +
 sound/soc/ti/Makefile    |   2 +
 sound/soc/ti/j721e-evm.c | 867 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 877 insertions(+)
 create mode 100644 sound/soc/ti/j721e-evm.c

diff --git a/sound/soc/ti/Kconfig b/sound/soc/ti/Kconfig
index c5408c129f34..53df545efe0a 100644
--- a/sound/soc/ti/Kconfig
+++ b/sound/soc/ti/Kconfig
@@ -219,5 +219,13 @@ config SND_SOC_DM365_VOICE_CODEC_MODULE
 	  The is an internal symbol needed to ensure that the codec
 	  and MFD driver can be built as loadable modules if necessary.
 
+config SND_SOC_J721E_EVM
+	tristate "SoC Audio support for j721e EVM"
+	depends on ARCH_K3_J721E_SOC || COMPILE_TEST
+	select SND_SOC_PCM3168A_I2C
+	select SND_SOC_DAVINCI_MCASP
+	help
+	  Say Y if you want to add support for SoC audio on j721e Common
+	  Processor Board and Infotainment expansion board.
 endmenu
 
diff --git a/sound/soc/ti/Makefile b/sound/soc/ti/Makefile
index ea48c6679cc7..a21e5b0061de 100644
--- a/sound/soc/ti/Makefile
+++ b/sound/soc/ti/Makefile
@@ -34,6 +34,7 @@ snd-soc-omap-abe-twl6040-objs := omap-abe-twl6040.o
 snd-soc-ams-delta-objs := ams-delta.o
 snd-soc-omap-hdmi-objs := omap-hdmi.o
 snd-soc-osk5912-objs := osk5912.o
+snd-soc-j721e-evm-objs := j721e-evm.o
 
 obj-$(CONFIG_SND_SOC_DAVINCI_EVM) += snd-soc-davinci-evm.o
 obj-$(CONFIG_SND_SOC_NOKIA_N810) += snd-soc-n810.o
@@ -44,3 +45,4 @@ obj-$(CONFIG_SND_SOC_OMAP_ABE_TWL6040) += snd-soc-omap-abe-twl6040.o
 obj-$(CONFIG_SND_SOC_OMAP_AMS_DELTA) += snd-soc-ams-delta.o
 obj-$(CONFIG_SND_SOC_OMAP_HDMI) += snd-soc-omap-hdmi.o
 obj-$(CONFIG_SND_SOC_OMAP_OSK5912) += snd-soc-osk5912.o
+obj-$(CONFIG_SND_SOC_J721E_EVM) += snd-soc-j721e-evm.o
diff --git a/sound/soc/ti/j721e-evm.c b/sound/soc/ti/j721e-evm.c
new file mode 100644
index 000000000000..c16c4a24d933
--- /dev/null
+++ b/sound/soc/ti/j721e-evm.c
@@ -0,0 +1,867 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com
+ *  Author: Peter Ujfalusi <peter.ujfalusi@ti.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+#include "davinci-mcasp.h"
+
+/*
+ * Maximum number of configuration entries for prefixes:
+ * CPB: 2 (mcasp10 + codec)
+ * IVI: 3 (mcasp0 + 2x codec)
+ */
+#define J721E_CODEC_CONF_COUNT	5
+
+#define J721E_AUDIO_DOMAIN_CPB	0
+#define J721E_AUDIO_DOMAIN_IVI	1
+
+#define J721E_CLK_PARENT_48000	0
+#define J721E_CLK_PARENT_44100	1
+
+#define J721E_MAX_CLK_HSDIV	128
+#define PCM1368A_MAX_SYSCLK	36864000
+
+#define J721E_DAI_FMT		(SND_SOC_DAIFMT_RIGHT_J | \
+				 SND_SOC_DAIFMT_NB_NF |   \
+				 SND_SOC_DAIFMT_CBS_CFS)
+
+enum j721e_board_type {
+	J721E_BOARD_CPB = 1,
+	J721E_BOARD_CPB_IVI,
+};
+
+struct j721e_audio_match_data {
+	enum j721e_board_type board_type;
+	int num_links;
+};
+
+static unsigned int ratios_for_pcm3168a[] = {
+	256,
+	512,
+	768,
+};
+
+struct j721e_audio_clocks {
+	struct clk *target;
+	struct clk *parent[2];
+};
+
+struct j721e_audio_domain {
+	struct j721e_audio_clocks codec;
+	struct j721e_audio_clocks mcasp;
+	int parent_clk_id;
+
+	int active;
+	unsigned int active_link;
+	unsigned int rate;
+};
+
+struct j721e_priv {
+	struct device *dev;
+	struct snd_soc_card card;
+	struct snd_soc_dai_link *dai_links;
+	struct snd_soc_codec_conf codec_conf[J721E_CODEC_CONF_COUNT];
+	struct snd_interval rate_range;
+	const struct j721e_audio_match_data *match_data;
+	u32 pll_rates[2];
+	unsigned int hsdiv_rates[2];
+
+	struct j721e_audio_domain audio_domains[2];
+
+	struct mutex mutex;
+};
+
+static const struct snd_soc_dapm_widget j721e_cpb_dapm_widgets[] = {
+	SND_SOC_DAPM_HP("CPB Stereo HP 1", NULL),
+	SND_SOC_DAPM_HP("CPB Stereo HP 2", NULL),
+	SND_SOC_DAPM_HP("CPB Stereo HP 3", NULL),
+	SND_SOC_DAPM_LINE("CPB Line Out", NULL),
+	SND_SOC_DAPM_MIC("CPB Stereo Mic 1", NULL),
+	SND_SOC_DAPM_MIC("CPB Stereo Mic 2", NULL),
+	SND_SOC_DAPM_LINE("CPB Line In", NULL),
+};
+
+static const struct snd_soc_dapm_route j721e_cpb_dapm_routes[] = {
+	{"CPB Stereo HP 1", NULL, "codec-1 AOUT1L"},
+	{"CPB Stereo HP 1", NULL, "codec-1 AOUT1R"},
+	{"CPB Stereo HP 2", NULL, "codec-1 AOUT2L"},
+	{"CPB Stereo HP 2", NULL, "codec-1 AOUT2R"},
+	{"CPB Stereo HP 3", NULL, "codec-1 AOUT3L"},
+	{"CPB Stereo HP 3", NULL, "codec-1 AOUT3R"},
+	{"CPB Line Out", NULL, "codec-1 AOUT4L"},
+	{"CPB Line Out", NULL, "codec-1 AOUT4R"},
+
+	{"codec-1 AIN1L", NULL, "CPB Stereo Mic 1"},
+	{"codec-1 AIN1R", NULL, "CPB Stereo Mic 1"},
+	{"codec-1 AIN2L", NULL, "CPB Stereo Mic 2"},
+	{"codec-1 AIN2R", NULL, "CPB Stereo Mic 2"},
+	{"codec-1 AIN3L", NULL, "CPB Line In"},
+	{"codec-1 AIN3R", NULL, "CPB Line In"},
+};
+
+static const struct snd_soc_dapm_widget j721e_ivi_codec_a_dapm_widgets[] = {
+	SND_SOC_DAPM_LINE("IVI A Line Out 1", NULL),
+	SND_SOC_DAPM_LINE("IVI A Line Out 2", NULL),
+	SND_SOC_DAPM_LINE("IVI A Line Out 3", NULL),
+	SND_SOC_DAPM_LINE("IVI A Line Out 4", NULL),
+	SND_SOC_DAPM_MIC("IVI A Stereo Mic 1", NULL),
+	SND_SOC_DAPM_MIC("IVI A Stereo Mic 2", NULL),
+	SND_SOC_DAPM_LINE("IVI A Line In", NULL),
+};
+
+static const struct snd_soc_dapm_route j721e_codec_a_dapm_routes[] = {
+	{"IVI A Line Out 1", NULL, "codec-a AOUT1L"},
+	{"IVI A Line Out 1", NULL, "codec-a AOUT1R"},
+	{"IVI A Line Out 2", NULL, "codec-a AOUT2L"},
+	{"IVI A Line Out 2", NULL, "codec-a AOUT2R"},
+	{"IVI A Line Out 3", NULL, "codec-a AOUT3L"},
+	{"IVI A Line Out 3", NULL, "codec-a AOUT3R"},
+	{"IVI A Line Out 4", NULL, "codec-a AOUT4L"},
+	{"IVI A Line Out 4", NULL, "codec-a AOUT4R"},
+
+	{"codec-a AIN1L", NULL, "IVI A Stereo Mic 1"},
+	{"codec-a AIN1R", NULL, "IVI A Stereo Mic 1"},
+	{"codec-a AIN2L", NULL, "IVI A Stereo Mic 2"},
+	{"codec-a AIN2R", NULL, "IVI A Stereo Mic 2"},
+	{"codec-a AIN3L", NULL, "IVI A Line In"},
+	{"codec-a AIN3R", NULL, "IVI A Line In"},
+};
+
+static const struct snd_soc_dapm_widget j721e_ivi_codec_b_dapm_widgets[] = {
+	SND_SOC_DAPM_LINE("IVI B Line Out 1", NULL),
+	SND_SOC_DAPM_LINE("IVI B Line Out 2", NULL),
+	SND_SOC_DAPM_LINE("IVI B Line Out 3", NULL),
+	SND_SOC_DAPM_LINE("IVI B Line Out 4", NULL),
+	SND_SOC_DAPM_MIC("IVI B Stereo Mic 1", NULL),
+	SND_SOC_DAPM_MIC("IVI B Stereo Mic 2", NULL),
+	SND_SOC_DAPM_LINE("IVI B Line In", NULL),
+};
+
+static const struct snd_soc_dapm_route j721e_codec_b_dapm_routes[] = {
+	{"IVI B Line Out 1", NULL, "codec-b AOUT1L"},
+	{"IVI B Line Out 1", NULL, "codec-b AOUT1R"},
+	{"IVI B Line Out 2", NULL, "codec-b AOUT2L"},
+	{"IVI B Line Out 2", NULL, "codec-b AOUT2R"},
+	{"IVI B Line Out 3", NULL, "codec-b AOUT3L"},
+	{"IVI B Line Out 3", NULL, "codec-b AOUT3R"},
+	{"IVI B Line Out 4", NULL, "codec-b AOUT4L"},
+	{"IVI B Line Out 4", NULL, "codec-b AOUT4R"},
+
+	{"codec-b AIN1L", NULL, "IVI B Stereo Mic 1"},
+	{"codec-b AIN1R", NULL, "IVI B Stereo Mic 1"},
+	{"codec-b AIN2L", NULL, "IVI B Stereo Mic 2"},
+	{"codec-b AIN2R", NULL, "IVI B Stereo Mic 2"},
+	{"codec-b AIN3L", NULL, "IVI B Line In"},
+	{"codec-b AIN3R", NULL, "IVI B Line In"},
+};
+
+static int j721e_configure_refclk(struct j721e_priv *priv,
+				  unsigned int audio_domain, unsigned int rate)
+{
+	struct j721e_audio_domain *domain = &priv->audio_domains[audio_domain];
+	unsigned int scki;
+	int ret = -EINVAL;
+	int i, clk_id;
+
+	if (!(rate % 8000))
+		clk_id = J721E_CLK_PARENT_48000;
+	else if (!(rate % 11025))
+		clk_id = J721E_CLK_PARENT_44100;
+	else
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(ratios_for_pcm3168a); i++) {
+		scki = ratios_for_pcm3168a[i] * rate;
+
+		if (priv->pll_rates[clk_id] / scki <= J721E_MAX_CLK_HSDIV) {
+			ret = 0;
+			break;
+		}
+	}
+
+	if (ret) {
+		dev_err(priv->dev, "No valid clock configuration for %u Hz\n",
+			rate);
+		return ret;
+	}
+
+	if (priv->hsdiv_rates[domain->parent_clk_id] != scki) {
+		dev_dbg(priv->dev,
+			"%s configuration for %u Hz: %s, %dxFS (SCKI: %u Hz)\n",
+			audio_domain == J721E_AUDIO_DOMAIN_CPB ? "CPB" : "IVI",
+			rate,
+			clk_id == J721E_CLK_PARENT_48000 ? "PLL4" : "PLL15",
+			ratios_for_pcm3168a[i], scki);
+
+		if (domain->parent_clk_id != clk_id) {
+			ret = clk_set_parent(domain->codec.target,
+					     domain->codec.parent[clk_id]);
+			if (ret)
+				return ret;
+
+			ret = clk_set_parent(domain->mcasp.target,
+					     domain->mcasp.parent[clk_id]);
+			if (ret)
+				return ret;
+
+			domain->parent_clk_id = clk_id;
+		}
+
+		ret = clk_set_rate(domain->codec.target, scki);
+		if (ret) {
+			dev_err(priv->dev, "codec set rate failed for %u Hz\n",
+				scki);
+			return ret;
+		}
+
+		ret = clk_set_rate(domain->mcasp.target, scki);
+		if (!ret) {
+			priv->hsdiv_rates[domain->parent_clk_id] = scki;
+		} else {
+			dev_err(priv->dev, "mcasp set rate failed for %u Hz\n",
+				scki);
+			return ret;
+		}
+	}
+
+	return ret;
+}
+
+static int j721e_rule_rate(struct snd_pcm_hw_params *params,
+			   struct snd_pcm_hw_rule *rule)
+{
+	struct snd_interval *t = rule->private;
+
+	return snd_interval_refine(hw_param_interval(params, rule->var), t);
+}
+
+static int j721e_audio_startup(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct j721e_priv *priv = snd_soc_card_get_drvdata(rtd->card);
+	unsigned int domain_id = rtd->dai_link->id;
+	struct j721e_audio_domain *domain = &priv->audio_domains[domain_id];
+	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+	struct snd_soc_dai *codec_dai;
+	unsigned int active_rate;
+	int ret = 0;
+	int i;
+
+	mutex_lock(&priv->mutex);
+
+	domain->active++;
+
+	if (priv->audio_domains[J721E_AUDIO_DOMAIN_CPB].rate)
+		active_rate = priv->audio_domains[J721E_AUDIO_DOMAIN_CPB].rate;
+	else
+		active_rate = priv->audio_domains[J721E_AUDIO_DOMAIN_IVI].rate;
+
+	if (active_rate)
+		ret = snd_pcm_hw_constraint_single(substream->runtime,
+						   SNDRV_PCM_HW_PARAM_RATE,
+						   active_rate);
+	else
+		ret = snd_pcm_hw_rule_add(substream->runtime, 0,
+					  SNDRV_PCM_HW_PARAM_RATE,
+					  j721e_rule_rate, &priv->rate_range,
+					  SNDRV_PCM_HW_PARAM_RATE, -1);
+
+	mutex_unlock(&priv->mutex);
+
+	if (ret)
+		return ret;
+
+	/* Reset TDM slots to 32 */
+	ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0x3, 0x3, 2, 32);
+	if (ret && ret != -ENOTSUPP)
+		return ret;
+
+	for_each_rtd_codec_dais(rtd, i, codec_dai) {
+		ret = snd_soc_dai_set_tdm_slot(codec_dai, 0x3, 0x3, 2, 32);
+		if (ret && ret != -ENOTSUPP)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int j721e_audio_hw_params(struct snd_pcm_substream *substream,
+				 struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_card *card = rtd->card;
+	struct j721e_priv *priv = snd_soc_card_get_drvdata(card);
+	unsigned int domain_id = rtd->dai_link->id;
+	struct j721e_audio_domain *domain = &priv->audio_domains[domain_id];
+	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+	struct snd_soc_dai *codec_dai;
+	unsigned int sysclk_rate;
+	int slot_width = 32;
+	int ret;
+	int i;
+
+	mutex_lock(&priv->mutex);
+
+	if (domain->rate && domain->rate != params_rate(params)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (params_width(params) == 16)
+		slot_width = 16;
+
+	ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0x3, 0x3, 2, slot_width);
+	if (ret && ret != -ENOTSUPP)
+		goto out;
+
+	for_each_rtd_codec_dais(rtd, i, codec_dai) {
+		ret = snd_soc_dai_set_tdm_slot(codec_dai, 0x3, 0x3, 2,
+					       slot_width);
+		if (ret && ret != -ENOTSUPP)
+			return ret;
+	}
+
+	ret = j721e_configure_refclk(priv, domain_id, params_rate(params));
+	if (ret)
+		goto out;
+
+	sysclk_rate = priv->hsdiv_rates[domain->parent_clk_id];
+	for_each_rtd_codec_dais(rtd, i, codec_dai) {
+		ret = snd_soc_dai_set_sysclk(codec_dai, 0, sysclk_rate,
+					     SND_SOC_CLOCK_IN);
+		if (ret && ret != -ENOTSUPP) {
+			dev_err(priv->dev,
+				"codec set_sysclk failed for %u Hz\n",
+				sysclk_rate);
+			goto out;
+		}
+	}
+
+	ret = snd_soc_dai_set_sysclk(cpu_dai, MCASP_CLK_HCLK_AUXCLK,
+				     sysclk_rate, SND_SOC_CLOCK_IN);
+
+	if (ret && ret != -ENOTSUPP) {
+		dev_err(priv->dev, "mcasp set_sysclk failed for %u Hz\n",
+			sysclk_rate);
+	} else {
+		domain->rate = params_rate(params);
+		ret = 0;
+	}
+
+out:
+	mutex_unlock(&priv->mutex);
+	return ret;
+}
+
+static void j721e_audio_shutdown(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct j721e_priv *priv = snd_soc_card_get_drvdata(rtd->card);
+	unsigned int domain_id = rtd->dai_link->id;
+	struct j721e_audio_domain *domain = &priv->audio_domains[domain_id];
+
+	mutex_lock(&priv->mutex);
+
+	domain->active--;
+	if (!domain->active) {
+		domain->rate = 0;
+		domain->active_link = 0;
+	}
+
+	mutex_unlock(&priv->mutex);
+}
+
+static const struct snd_soc_ops j721e_audio_ops = {
+	.startup = j721e_audio_startup,
+	.hw_params = j721e_audio_hw_params,
+	.shutdown = j721e_audio_shutdown,
+};
+
+static int j721e_audio_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct j721e_priv *priv = snd_soc_card_get_drvdata(rtd->card);
+	unsigned int domain_id = rtd->dai_link->id;
+	struct j721e_audio_domain *domain = &priv->audio_domains[domain_id];
+	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+	struct snd_soc_dai *codec_dai;
+	unsigned int sysclk_rate;
+	int i, ret;
+
+	/* Set up initial clock configuration */
+	ret = j721e_configure_refclk(priv, domain_id, 48000);
+	if (ret)
+		return ret;
+
+	sysclk_rate = priv->hsdiv_rates[domain->parent_clk_id];
+	for_each_rtd_codec_dais(rtd, i, codec_dai) {
+		ret = snd_soc_dai_set_sysclk(codec_dai, 0, sysclk_rate,
+					     SND_SOC_CLOCK_IN);
+		if (ret && ret != -ENOTSUPP)
+			return ret;
+	}
+
+	ret = snd_soc_dai_set_sysclk(cpu_dai, MCASP_CLK_HCLK_AUXCLK,
+				     sysclk_rate, SND_SOC_CLOCK_IN);
+	if (ret && ret != -ENOTSUPP)
+		return ret;
+
+	/* Set initial tdm slots */
+	ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0x3, 0x3, 2, 32);
+	if (ret && ret != -ENOTSUPP)
+		return ret;
+
+	for_each_rtd_codec_dais(rtd, i, codec_dai) {
+		ret = snd_soc_dai_set_tdm_slot(codec_dai, 0x3, 0x3, 2, 32);
+		if (ret && ret != -ENOTSUPP)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int j721e_audio_init_ivi(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_dapm_context *dapm = &rtd->card->dapm;
+
+	snd_soc_dapm_new_controls(dapm, j721e_ivi_codec_a_dapm_widgets,
+				  ARRAY_SIZE(j721e_ivi_codec_a_dapm_widgets));
+	snd_soc_dapm_add_routes(dapm, j721e_codec_a_dapm_routes,
+				ARRAY_SIZE(j721e_codec_a_dapm_routes));
+	snd_soc_dapm_new_controls(dapm, j721e_ivi_codec_b_dapm_widgets,
+				  ARRAY_SIZE(j721e_ivi_codec_b_dapm_widgets));
+	snd_soc_dapm_add_routes(dapm, j721e_codec_b_dapm_routes,
+				ARRAY_SIZE(j721e_codec_b_dapm_routes));
+
+	return j721e_audio_init(rtd);
+}
+
+static int j721e_get_clocks(struct device *dev,
+			    struct j721e_audio_clocks *clocks, char *prefix)
+{
+	struct clk *parent;
+	char *clk_name;
+	int ret;
+
+	clocks->target = devm_clk_get(dev, prefix);
+	if (IS_ERR(clocks->target)) {
+		ret = PTR_ERR(clocks->target);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to acquire %s': %d\n",
+				prefix, ret);
+		return ret;
+	}
+
+	clk_name = kasprintf(GFP_KERNEL, "%s-48000", prefix);
+	if (clk_name) {
+		parent = devm_clk_get(dev, clk_name);
+		kfree(clk_name);
+		if (IS_ERR(parent)) {
+			ret = PTR_ERR(parent);
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "failed to acquire %s': %d\n",
+					prefix, ret);
+			return ret;
+		}
+		clocks->parent[J721E_CLK_PARENT_48000] = parent;
+	} else {
+		return -ENOMEM;
+	}
+
+	clk_name = kasprintf(GFP_KERNEL, "%s-44100", prefix);
+	if (clk_name) {
+		parent = devm_clk_get(dev, clk_name);
+		kfree(clk_name);
+		if (IS_ERR(parent)) {
+			ret = PTR_ERR(parent);
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "failed to acquire %s': %d\n",
+					prefix, ret);
+			return ret;
+		}
+		clocks->parent[J721E_CLK_PARENT_44100] = parent;
+	} else {
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static const struct j721e_audio_match_data j721e_cpb_data = {
+	.board_type = J721E_BOARD_CPB,
+	.num_links = 2, /* CPB pcm3168a */
+};
+
+static const struct j721e_audio_match_data j721e_cpb_ivi_data = {
+	.board_type = J721E_BOARD_CPB_IVI,
+	.num_links = 4, /* CPB pcm3168a + 2x IVI pcm3168a */
+};
+
+static const struct of_device_id j721e_audio_of_match[] = {
+	{
+		.compatible = "ti,j721e-cpb-audio",
+		.data = &j721e_cpb_data,
+	}, {
+		.compatible = "ti,j721e-cpb-ivi-audio",
+		.data = &j721e_cpb_ivi_data,
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, j721e_audio_of_match);
+
+static int j721e_calculate_rate_range(struct j721e_priv *priv)
+{
+	unsigned int min_rate, max_rate, pll_rate;
+	struct clk *pll;
+
+	pll = clk_get(priv->dev, "pll15");
+	if (IS_ERR(pll)) {
+		int ret = PTR_ERR(pll);
+
+		if (ret != -EPROBE_DEFER)
+			dev_err(priv->dev, "failed to acquire PLL15 clock\n");
+
+		return ret;
+	}
+	priv->pll_rates[J721E_CLK_PARENT_44100] = clk_get_rate(pll);
+	clk_put(pll);
+
+	pll_rate = priv->pll_rates[J721E_CLK_PARENT_44100];
+	min_rate = pll_rate / J721E_MAX_CLK_HSDIV;
+	min_rate /= ratios_for_pcm3168a[ARRAY_SIZE(ratios_for_pcm3168a) - 1];
+
+	pll = clk_get(priv->dev, "pll4");
+	if (IS_ERR(pll)) {
+		int ret = PTR_ERR(pll);
+
+		if (ret != -EPROBE_DEFER)
+			dev_err(priv->dev, "failed to acquire PLL4 clock\n");
+
+		return ret;
+	}
+	priv->pll_rates[J721E_CLK_PARENT_48000] = clk_get_rate(pll);
+	clk_put(pll);
+
+	pll_rate = priv->pll_rates[J721E_CLK_PARENT_48000];
+	if (pll_rate > PCM1368A_MAX_SYSCLK)
+		pll_rate = PCM1368A_MAX_SYSCLK;
+
+	max_rate = pll_rate / ratios_for_pcm3168a[0];
+
+	snd_interval_any(&priv->rate_range);
+	priv->rate_range.min = min_rate;
+	priv->rate_range.max = max_rate;
+
+	return 0;
+}
+
+static int j721e_soc_probe_cpb(struct j721e_priv *priv, int *link_idx,
+			       int *conf_idx)
+{
+	struct device_node *node = priv->dev->of_node;
+	struct snd_soc_dai_link_component *compnent;
+	struct device_node *dai_node, *codec_node;
+	struct j721e_audio_domain *domain;
+	int comp_count, comp_idx;
+	int ret;
+
+	dai_node = of_parse_phandle(node, "ti,cpb-mcasp", 0);
+	if (!dai_node) {
+		dev_err(priv->dev, "CPB McASP node is not provided\n");
+		return -EINVAL;
+	}
+
+	codec_node = of_parse_phandle(node, "ti,cpb-codec", 0);
+	if (!codec_node) {
+		dev_err(priv->dev, "CPB codec node is not provided\n");
+		return -EINVAL;
+	}
+
+	domain = &priv->audio_domains[J721E_AUDIO_DOMAIN_CPB];
+	ret = j721e_get_clocks(priv->dev, &domain->codec, "audio-refclk2");
+	if (ret)
+		return ret;
+
+	ret = j721e_get_clocks(priv->dev, &domain->mcasp, "cpb-mcasp");
+	if (ret)
+		return ret;
+
+	/*
+	 * Common Processor Board, two links
+	 * Link 1: McASP10 -> pcm3168a_1 DAC
+	 * Link 2: McASP10 <- pcm3168a_1 ADC
+	 */
+	comp_count = 6;
+	compnent = devm_kzalloc(priv->dev, comp_count * sizeof(*compnent),
+				GFP_KERNEL);
+	if (!compnent)
+		return -ENOMEM;
+
+	comp_idx = 0;
+	priv->dai_links[*link_idx].cpus = &compnent[comp_idx++];
+	priv->dai_links[*link_idx].num_cpus = 1;
+	priv->dai_links[*link_idx].codecs = &compnent[comp_idx++];
+	priv->dai_links[*link_idx].num_codecs = 1;
+	priv->dai_links[*link_idx].platforms = &compnent[comp_idx++];
+	priv->dai_links[*link_idx].num_platforms = 1;
+
+	priv->dai_links[*link_idx].name = "CPB PCM3168A Playback";
+	priv->dai_links[*link_idx].stream_name = "CPB PCM3168A Analog";
+	priv->dai_links[*link_idx].cpus->of_node = dai_node;
+	priv->dai_links[*link_idx].platforms->of_node = dai_node;
+	priv->dai_links[*link_idx].codecs->of_node = codec_node;
+	priv->dai_links[*link_idx].codecs->dai_name = "pcm3168a-dac";
+	priv->dai_links[*link_idx].playback_only = 1;
+	priv->dai_links[*link_idx].id = J721E_AUDIO_DOMAIN_CPB;
+	priv->dai_links[*link_idx].dai_fmt = J721E_DAI_FMT;
+	priv->dai_links[*link_idx].init = j721e_audio_init;
+	priv->dai_links[*link_idx].ops = &j721e_audio_ops;
+	(*link_idx)++;
+
+	priv->dai_links[*link_idx].cpus = &compnent[comp_idx++];
+	priv->dai_links[*link_idx].num_cpus = 1;
+	priv->dai_links[*link_idx].codecs = &compnent[comp_idx++];
+	priv->dai_links[*link_idx].num_codecs = 1;
+	priv->dai_links[*link_idx].platforms = &compnent[comp_idx++];
+	priv->dai_links[*link_idx].num_platforms = 1;
+
+	priv->dai_links[*link_idx].name = "CPB PCM3168A Capture";
+	priv->dai_links[*link_idx].stream_name = "CPB PCM3168A Analog";
+	priv->dai_links[*link_idx].cpus->of_node = dai_node;
+	priv->dai_links[*link_idx].platforms->of_node = dai_node;
+	priv->dai_links[*link_idx].codecs->of_node = codec_node;
+	priv->dai_links[*link_idx].codecs->dai_name = "pcm3168a-adc";
+	priv->dai_links[*link_idx].capture_only = 1;
+	priv->dai_links[*link_idx].id = J721E_AUDIO_DOMAIN_CPB;
+	priv->dai_links[*link_idx].dai_fmt = J721E_DAI_FMT;
+	priv->dai_links[*link_idx].init = j721e_audio_init;
+	priv->dai_links[*link_idx].ops = &j721e_audio_ops;
+	(*link_idx)++;
+
+	priv->codec_conf[*conf_idx].dlc.of_node = codec_node;
+	priv->codec_conf[*conf_idx].name_prefix = "codec-1";
+	(*conf_idx)++;
+	priv->codec_conf[*conf_idx].dlc.of_node = dai_node;
+	priv->codec_conf[*conf_idx].name_prefix = "McASP10";
+	(*conf_idx)++;
+
+	return 0;
+}
+
+static int j721e_soc_probe_ivi(struct j721e_priv *priv, int *link_idx,
+			       int *conf_idx)
+{
+	struct device_node *node = priv->dev->of_node;
+	struct snd_soc_dai_link_component *compnent;
+	struct device_node *dai_node, *codeca_node, *codecb_node;
+	struct j721e_audio_domain *domain;
+	int comp_count, comp_idx;
+	int ret;
+
+	if (priv->match_data->board_type != J721E_BOARD_CPB_IVI)
+		return 0;
+
+	dai_node = of_parse_phandle(node, "ti,ivi-mcasp", 0);
+	if (!dai_node) {
+		dev_err(priv->dev, "IVI McASP node is not provided\n");
+		return -EINVAL;
+	}
+
+	codeca_node = of_parse_phandle(node, "ti,ivi-codec-a", 0);
+	if (!codeca_node) {
+		dev_err(priv->dev, "IVI codec-a node is not provided\n");
+		return -EINVAL;
+	}
+
+	codecb_node = of_parse_phandle(node, "ti,ivi-codec-b", 0);
+	if (!codecb_node) {
+		dev_warn(priv->dev, "IVI codec-b node is not provided\n");
+		return 0;
+	}
+
+	domain = &priv->audio_domains[J721E_AUDIO_DOMAIN_IVI];
+	ret = j721e_get_clocks(priv->dev, &domain->codec,
+			       "audio-refclk0");
+	if (ret)
+		return ret;
+
+	ret = j721e_get_clocks(priv->dev, &domain->mcasp, "ivi-mcasp");
+	if (ret)
+		return ret;
+
+	/*
+	 * IVI extension, two links
+	 * Link 1: McASP0 -> pcm3168a_a DAC
+	 *		  \> pcm3168a_b DAC
+	 * Link 2: McASP0 <- pcm3168a_a ADC
+	 *		   \ pcm3168a_b ADC
+	 */
+	comp_count = 8;
+	compnent = devm_kzalloc(priv->dev, comp_count * sizeof(*compnent),
+				GFP_KERNEL);
+	if (!compnent)
+		return -ENOMEM;
+
+	comp_idx = 0;
+	priv->dai_links[*link_idx].cpus = &compnent[comp_idx++];
+	priv->dai_links[*link_idx].num_cpus = 1;
+	priv->dai_links[*link_idx].platforms = &compnent[comp_idx++];
+	priv->dai_links[*link_idx].num_platforms = 1;
+	priv->dai_links[*link_idx].codecs = &compnent[comp_idx];
+	priv->dai_links[*link_idx].num_codecs = 2;
+	comp_idx += 2;
+
+	priv->dai_links[*link_idx].name = "IVI 2xPCM3168A Playback";
+	priv->dai_links[*link_idx].stream_name = "IVI 2xPCM3168A Analog";
+	priv->dai_links[*link_idx].cpus->of_node = dai_node;
+	priv->dai_links[*link_idx].platforms->of_node = dai_node;
+	priv->dai_links[*link_idx].codecs[0].of_node = codeca_node;
+	priv->dai_links[*link_idx].codecs[0].dai_name = "pcm3168a-dac";
+	priv->dai_links[*link_idx].codecs[1].of_node = codecb_node;
+	priv->dai_links[*link_idx].codecs[1].dai_name = "pcm3168a-dac";
+	priv->dai_links[*link_idx].playback_only = 1;
+	priv->dai_links[*link_idx].id = J721E_AUDIO_DOMAIN_IVI;
+	priv->dai_links[*link_idx].dai_fmt = J721E_DAI_FMT;
+	priv->dai_links[*link_idx].init = j721e_audio_init_ivi;
+	priv->dai_links[*link_idx].ops = &j721e_audio_ops;
+	(*link_idx)++;
+
+	priv->dai_links[*link_idx].cpus = &compnent[comp_idx++];
+	priv->dai_links[*link_idx].num_cpus = 1;
+	priv->dai_links[*link_idx].platforms = &compnent[comp_idx++];
+	priv->dai_links[*link_idx].num_platforms = 1;
+	priv->dai_links[*link_idx].codecs = &compnent[comp_idx];
+	priv->dai_links[*link_idx].num_codecs = 2;
+
+	priv->dai_links[*link_idx].name = "IVI 2xPCM3168A Capture";
+	priv->dai_links[*link_idx].stream_name = "IVI 2xPCM3168A Analog";
+	priv->dai_links[*link_idx].cpus->of_node = dai_node;
+	priv->dai_links[*link_idx].platforms->of_node = dai_node;
+	priv->dai_links[*link_idx].codecs[0].of_node = codeca_node;
+	priv->dai_links[*link_idx].codecs[0].dai_name = "pcm3168a-adc";
+	priv->dai_links[*link_idx].codecs[1].of_node = codecb_node;
+	priv->dai_links[*link_idx].codecs[1].dai_name = "pcm3168a-adc";
+	priv->dai_links[*link_idx].capture_only = 1;
+	priv->dai_links[*link_idx].id = J721E_AUDIO_DOMAIN_IVI;
+	priv->dai_links[*link_idx].dai_fmt = J721E_DAI_FMT;
+	priv->dai_links[*link_idx].init = j721e_audio_init;
+	priv->dai_links[*link_idx].ops = &j721e_audio_ops;
+	(*link_idx)++;
+
+	priv->codec_conf[*conf_idx].dlc.of_node = codeca_node;
+	priv->codec_conf[*conf_idx].name_prefix = "codec-a";
+	(*conf_idx)++;
+
+	priv->codec_conf[*conf_idx].dlc.of_node = codecb_node;
+	priv->codec_conf[*conf_idx].name_prefix = "codec-b";
+	(*conf_idx)++;
+
+	priv->codec_conf[*conf_idx].dlc.of_node = dai_node;
+	priv->codec_conf[*conf_idx].name_prefix = "McASP0";
+	(*conf_idx)++;
+
+	return 0;
+}
+
+static int j721e_soc_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct snd_soc_card *card;
+	const struct of_device_id *match;
+	struct j721e_priv *priv;
+	int link_cnt, conf_cnt, ret;
+
+	if (!node) {
+		dev_err(&pdev->dev, "of node is missing.\n");
+		return -ENODEV;
+	}
+
+	match = of_match_node(j721e_audio_of_match, node);
+	if (!match) {
+		dev_err(&pdev->dev, "No compatible match found\n");
+		return -ENODEV;
+	}
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->match_data = match->data;
+
+	priv->dai_links = devm_kcalloc(&pdev->dev, priv->match_data->num_links,
+				       sizeof(*priv->dai_links), GFP_KERNEL);
+	if (!priv->dai_links)
+		return -ENOMEM;
+
+	priv->audio_domains[J721E_AUDIO_DOMAIN_CPB].parent_clk_id = -1;
+	priv->audio_domains[J721E_AUDIO_DOMAIN_IVI].parent_clk_id = -1;
+	priv->dev = &pdev->dev;
+	card = &priv->card;
+	card->dev = &pdev->dev;
+	card->owner = THIS_MODULE;
+	card->dapm_widgets = j721e_cpb_dapm_widgets;
+	card->num_dapm_widgets = ARRAY_SIZE(j721e_cpb_dapm_widgets);
+	card->dapm_routes = j721e_cpb_dapm_routes;
+	card->num_dapm_routes = ARRAY_SIZE(j721e_cpb_dapm_routes);
+	card->fully_routed = 1;
+
+	if (snd_soc_of_parse_card_name(card, "model")) {
+		dev_err(&pdev->dev, "Card name is not provided\n");
+		return -ENODEV;
+	}
+
+	link_cnt = 0;
+	conf_cnt = 0;
+	ret = j721e_soc_probe_cpb(priv, &link_cnt, &conf_cnt);
+	if (ret)
+		return ret;
+
+	ret = j721e_soc_probe_ivi(priv, &link_cnt, &conf_cnt);
+	if (ret)
+		return ret;
+
+	card->dai_link = priv->dai_links;
+	card->num_links = link_cnt;
+
+	card->codec_conf = priv->codec_conf;
+	card->num_configs = conf_cnt;
+
+	ret = j721e_calculate_rate_range(priv);
+	if (ret)
+		return ret;
+
+	snd_soc_card_set_drvdata(card, priv);
+
+	mutex_init(&priv->mutex);
+	ret = devm_snd_soc_register_card(&pdev->dev, card);
+	if (ret)
+		dev_err(&pdev->dev, "devm_snd_soc_register_card() failed: %d\n",
+			ret);
+
+	return ret;
+}
+
+static struct platform_driver j721e_soc_driver = {
+	.driver = {
+		.name = "j721e-audio",
+		.pm = &snd_soc_pm_ops,
+		.of_match_table = of_match_ptr(j721e_audio_of_match),
+	},
+	.probe = j721e_soc_probe,
+};
+
+module_platform_driver(j721e_soc_driver);
+
+MODULE_AUTHOR("Peter Ujfalusi <peter.ujfalusi@ti.com>");
+MODULE_DESCRIPTION("ASoC machine driver for j721e Common Processor Board");
+MODULE_LICENSE("GPL v2");
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


^ permalink raw reply related

* [PATCH v2 1/3] ASoC: ti: davinci-mcasp: Specify stream_name for playback/capture
From: Peter Ujfalusi @ 2020-05-12 13:16 UTC (permalink / raw)
  To: broonie, lgirdwood, robh+dt; +Cc: alsa-devel, devicetree, linux-kernel
In-Reply-To: <20200512131633.32668-1-peter.ujfalusi@ti.com>

In order to dai stream widgets to be created the stream_name must be set.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/ti/davinci-mcasp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/ti/davinci-mcasp.c b/sound/soc/ti/davinci-mcasp.c
index 7a7db743dc5b..15d84493466f 100644
--- a/sound/soc/ti/davinci-mcasp.c
+++ b/sound/soc/ti/davinci-mcasp.c
@@ -1623,12 +1623,14 @@ static struct snd_soc_dai_driver davinci_mcasp_dai[] = {
 		.name		= "davinci-mcasp.0",
 		.probe		= davinci_mcasp_dai_probe,
 		.playback	= {
+			.stream_name = "IIS Playback",
 			.channels_min	= 1,
 			.channels_max	= 32 * 16,
 			.rates 		= DAVINCI_MCASP_RATES,
 			.formats	= DAVINCI_MCASP_PCM_FMTS,
 		},
 		.capture 	= {
+			.stream_name = "IIS Capture",
 			.channels_min 	= 1,
 			.channels_max	= 32 * 16,
 			.rates 		= DAVINCI_MCASP_RATES,
@@ -1642,6 +1644,7 @@ static struct snd_soc_dai_driver davinci_mcasp_dai[] = {
 		.name		= "davinci-mcasp.1",
 		.probe		= davinci_mcasp_dai_probe,
 		.playback 	= {
+			.stream_name = "DIT Playback",
 			.channels_min	= 1,
 			.channels_max	= 384,
 			.rates		= DAVINCI_MCASP_RATES,
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


^ permalink raw reply related

* [PATCH v2] dt-bindings: nvmem: stm32: new property for data access
From: Etienne Carriere @ 2020-05-12 13:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-stm32, linux-arm-kernel, devicetree, fabrice.gasnier,
	alexandre.torgue, mcoquelin.stm32, robh+dt, srinivas.kandagatla,
	Etienne Carriere

From: Etienne Carriere <etienne.carriere@st.com>

Introduce boolean property st,non-secure-otp for OTP data located
in a factory programmed area that only secure firmware can access
by default and that shall be reachable from the non-secure world.

This change also allows additional properties for NVMEM nodes that
were forbidden prior this change.

Signed-off-by: Etienne Carriere <etienne.carriere@st.com>
---
Changes since v1:
  Change nvmem.yaml to allow additional properties in NVMEM nodes.

Link to v1:
  https://lore.kernel.org/patchwork/patch/1239028/

 .../devicetree/bindings/nvmem/nvmem.yaml        |  2 --
 .../bindings/nvmem/st,stm32-romem.yaml          | 17 +++++++++++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
index 65980224d550..b459f9dba6c9 100644
--- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
@@ -67,8 +67,6 @@ patternProperties:
     required:
       - reg
 
-    additionalProperties: false
-
 examples:
   - |
       #include <dt-bindings/gpio/gpio.h>
diff --git a/Documentation/devicetree/bindings/nvmem/st,stm32-romem.yaml b/Documentation/devicetree/bindings/nvmem/st,stm32-romem.yaml
index d84deb4774a4..c11c99f085d7 100644
--- a/Documentation/devicetree/bindings/nvmem/st,stm32-romem.yaml
+++ b/Documentation/devicetree/bindings/nvmem/st,stm32-romem.yaml
@@ -24,6 +24,18 @@ properties:
       - st,stm32f4-otp
       - st,stm32mp15-bsec
 
+patternProperties:
+  "^.*@[0-9a-f]+$":
+    type: object
+
+    properties:
+      st,non-secure-otp:
+        description: |
+          This property explicits a factory programmed area that both secure
+          and non-secure worlds can access. It is needed when, by default, the
+          related area can only be reached by the secure world.
+        type: boolean
+
 required:
   - "#address-cells"
   - "#size-cells"
@@ -41,6 +53,11 @@ examples:
       calib@22c {
         reg = <0x22c 0x2>;
       };
+
+      mac_addr@e4 {
+        reg = <0xe4 0x8>;
+        st,non-secure-otp;
+      };
     };
 
 ...
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v2 3/5] pinctrl: lochnagar: Move binding over to dtschema
From: Linus Walleij @ 2020-05-12 13:06 UTC (permalink / raw)
  To: Charles Keepax
  Cc: Rob Herring, Lee Jones, Mark Brown, Michael Turquette,
	Stephen Boyd, Jean Delvare, Guenter Roeck, Liam Girdwood,
	linux-kernel@vger.kernel.org,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	patches
In-Reply-To: <20200504154757.17519-3-ckeepax@opensource.cirrus.com>

On Mon, May 4, 2020 at 5:48 PM Charles Keepax
<ckeepax@opensource.cirrus.com> wrote:

> Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> ---
>
> Changes since v1:
>  - Moved sub-node into MFD file leaving just the properties in here
>  - Removed contains on the compatible
>  - Added a required -pins suffix for pinmux/ctrl nodes
>  - Added some extra blank lines for readability

Backed out v1 since I see there is some discussion on these still (sorry for
missing this).

I'll hold this off until there is consensus.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH] dt-bindings: eeprom: at24: Fix list of page sizes for writing
From: Wolfram Sang @ 2020-05-12 12:56 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Bartosz Golaszewski, Rob Herring, linux-i2c, devicetree
In-Reply-To: <20200512122450.20205-1-geert+renesas@glider.be>

[-- Attachment #1: Type: text/plain, Size: 289 bytes --]

On Tue, May 12, 2020 at 02:24:47PM +0200, Geert Uytterhoeven wrote:
> "258" is an odd power-of-two ;-)

Geez, you got two additional for free, Geert ;)

> Obviously this is a typo, and the intended value is "256".

Obviously.

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH v8 02/10] OPP: Add helpers for reading the binding properties
From: Georgi Djakov @ 2020-05-12 12:53 UTC (permalink / raw)
  To: vireshk, nm, sboyd, rjw, saravanak, sibis, mka
  Cc: robh+dt, rnayak, bjorn.andersson, vincent.guittot, jcrouse,
	evgreen, linux-pm, devicetree, linux-kernel, georgi.djakov
In-Reply-To: <20200512125327.1868-1-georgi.djakov@linaro.org>

From: Saravana Kannan <saravanak@google.com>

The opp-hz DT property is not mandatory and we may use another property
as a key in the OPP table. Add helper functions to simplify the reading
and comparing the keys.

Signed-off-by: Saravana Kannan <saravanak@google.com>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Sibi Sankar <sibis@codeaurora.org>
Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
v8:
* Keep the existing behavior: goto free_opp only if !is_genpd
* Picked reviewed-by tags.

 drivers/opp/core.c | 15 +++++++++++++--
 drivers/opp/of.c   | 45 +++++++++++++++++++++++++++------------------
 drivers/opp/opp.h  |  1 +
 3 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index e4f01e7771a2..ce7e4103ec09 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1286,11 +1286,21 @@ static bool _opp_supported_by_regulators(struct dev_pm_opp *opp,
 	return true;
 }
 
+int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2)
+{
+	if (opp1->rate != opp2->rate)
+		return opp1->rate < opp2->rate ? -1 : 1;
+	if (opp1->level != opp2->level)
+		return opp1->level < opp2->level ? -1 : 1;
+	return 0;
+}
+
 static int _opp_is_duplicate(struct device *dev, struct dev_pm_opp *new_opp,
 			     struct opp_table *opp_table,
 			     struct list_head **head)
 {
 	struct dev_pm_opp *opp;
+	int opp_cmp;
 
 	/*
 	 * Insert new OPP in order of increasing frequency and discard if
@@ -1301,12 +1311,13 @@ static int _opp_is_duplicate(struct device *dev, struct dev_pm_opp *new_opp,
 	 * loop.
 	 */
 	list_for_each_entry(opp, &opp_table->opp_list, node) {
-		if (new_opp->rate > opp->rate) {
+		opp_cmp = _opp_compare_key(new_opp, opp);
+		if (opp_cmp > 0) {
 			*head = &opp->node;
 			continue;
 		}
 
-		if (new_opp->rate < opp->rate)
+		if (opp_cmp < 0)
 			return 0;
 
 		/* Duplicate OPPs */
diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 9cd8f0adacae..5179f034751a 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -521,6 +521,28 @@ void dev_pm_opp_of_remove_table(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table);
 
+static int _read_opp_key(struct dev_pm_opp *new_opp, struct device_node *np,
+			 bool *rate_not_available)
+{
+	u64 rate;
+	int ret;
+
+	ret = of_property_read_u64(np, "opp-hz", &rate);
+	if (!ret) {
+		/*
+		 * Rate is defined as an unsigned long in clk API, and so
+		 * casting explicitly to its type. Must be fixed once rate is 64
+		 * bit guaranteed in clk API.
+		 */
+		new_opp->rate = (unsigned long)rate;
+	}
+	*rate_not_available = !!ret;
+
+	of_property_read_u32(np, "opp-level", &new_opp->level);
+
+	return ret;
+}
+
 /**
  * _opp_add_static_v2() - Allocate static OPPs (As per 'v2' DT bindings)
  * @opp_table:	OPP table
@@ -558,26 +580,13 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table,
 	if (!new_opp)
 		return ERR_PTR(-ENOMEM);
 
-	ret = of_property_read_u64(np, "opp-hz", &rate);
-	if (ret < 0) {
-		/* "opp-hz" is optional for devices like power domains. */
-		if (!opp_table->is_genpd) {
-			dev_err(dev, "%s: opp-hz not found\n", __func__);
-			goto free_opp;
-		}
-
-		rate_not_available = true;
-	} else {
-		/*
-		 * Rate is defined as an unsigned long in clk API, and so
-		 * casting explicitly to its type. Must be fixed once rate is 64
-		 * bit guaranteed in clk API.
-		 */
-		new_opp->rate = (unsigned long)rate;
+	ret = _read_opp_key(new_opp, np, &rate_not_available);
+	/* The key is optional for devices like power domains. */
+	if (ret < 0 && !opp_table->is_genpd) {
+		dev_err(dev, "%s: opp key field not found\n", __func__);
+		goto free_opp;
 	}
 
-	of_property_read_u32(np, "opp-level", &new_opp->level);
-
 	/* Check if the OPP supports hardware's hierarchy of versions or not */
 	if (!_opp_is_supported(dev, opp_table, np)) {
 		dev_dbg(dev, "OPP not supported by hardware: %llu\n", rate);
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index d14e27102730..bcadb1e328a4 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -211,6 +211,7 @@ struct opp_device *_add_opp_dev(const struct device *dev, struct opp_table *opp_
 void _dev_pm_opp_find_and_remove_table(struct device *dev);
 struct dev_pm_opp *_opp_allocate(struct opp_table *opp_table);
 void _opp_free(struct dev_pm_opp *opp);
+int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2);
 int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct opp_table *opp_table, bool rate_not_available);
 int _opp_add_v1(struct opp_table *opp_table, struct device *dev, unsigned long freq, long u_volt, bool dynamic);
 void _dev_pm_opp_cpumask_remove_table(const struct cpumask *cpumask, int last_cpu);

^ permalink raw reply related

* [PATCH v8 08/10] cpufreq: dt: Validate all interconnect paths
From: Georgi Djakov @ 2020-05-12 12:53 UTC (permalink / raw)
  To: vireshk, nm, sboyd, rjw, saravanak, sibis, mka
  Cc: robh+dt, rnayak, bjorn.andersson, vincent.guittot, jcrouse,
	evgreen, linux-pm, devicetree, linux-kernel, georgi.djakov
In-Reply-To: <20200512125327.1868-1-georgi.djakov@linaro.org>

Currently when we check for the available resources, we assume that there
is only one interconnect path, but in fact it could be more than one. Do
some validation to determine the number of paths and verify if each one
of them is available.

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
v8:
* New patch.

 drivers/cpufreq/cpufreq-dt.c | 49 ++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 4ecef3257532..3dd28c2c1633 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -91,12 +91,54 @@ static const char *find_supply_name(struct device *dev)
 	return name;
 }
 
+static int find_icc_paths(struct device *dev)
+{
+	struct device_node *np;
+	struct icc_path **paths;
+	int i, count, num_paths;
+	int ret = 0;
+
+	np = of_node_get(dev->of_node);
+	if (!np)
+		return 0;
+
+	count = of_count_phandle_with_args(np, "interconnects",
+					   "#interconnect-cells");
+	of_node_put(np);
+	if (count < 0)
+		return 0;
+
+	/* two phandles when #interconnect-cells = <1> */
+	if (count % 2) {
+		dev_err(dev, "%s: Invalid interconnects values\n", __func__);
+		return -EINVAL;
+	}
+
+	num_paths = count / 2;
+	paths = kcalloc(num_paths, sizeof(*paths), GFP_KERNEL);
+	if (!paths)
+		return -ENOMEM;
+
+	for (i = 0; i < num_paths; i++) {
+		paths[i] = of_icc_get_by_index(dev, i);
+		ret = PTR_ERR_OR_ZERO(paths[i]);
+		if (ret)
+			break;
+	}
+
+	while (i--)
+		icc_put(paths[i]);
+
+	kfree(paths);
+
+	return ret;
+}
+
 static int resources_available(void)
 {
 	struct device *cpu_dev;
 	struct regulator *cpu_reg;
 	struct clk *cpu_clk;
-	struct icc_path *cpu_path;
 	int ret = 0;
 	const char *name;
 
@@ -123,8 +165,7 @@ static int resources_available(void)
 
 	clk_put(cpu_clk);
 
-	cpu_path = of_icc_get(cpu_dev, NULL);
-	ret = PTR_ERR_OR_ZERO(cpu_path);
+	ret = find_icc_paths(cpu_dev);
 	if (ret) {
 		if (ret == -EPROBE_DEFER)
 			dev_dbg(cpu_dev, "defer icc path: %d\n", ret);
@@ -134,8 +175,6 @@ static int resources_available(void)
 		return ret;
 	}
 
-	icc_put(cpu_path);
-
 	name = find_supply_name(cpu_dev);
 	/* Platform doesn't require regulator */
 	if (!name)

^ permalink raw reply related

* [PATCH v8 07/10] cpufreq: dt: Add support for interconnect bandwidth scaling
From: Georgi Djakov @ 2020-05-12 12:53 UTC (permalink / raw)
  To: vireshk, nm, sboyd, rjw, saravanak, sibis, mka
  Cc: robh+dt, rnayak, bjorn.andersson, vincent.guittot, jcrouse,
	evgreen, linux-pm, devicetree, linux-kernel, georgi.djakov
In-Reply-To: <20200512125327.1868-1-georgi.djakov@linaro.org>

In addition to clocks and regulators, some devices can scale the bandwidth
of their on-chip interconnect - for example between CPU and DDR memory. Add
support for that, so that platforms which support it can make use of it.

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
v8:
* Picked reviewed-by tag.
* Added a separate patch to check the number of paths in DT and validate
  each one of them.

 drivers/cpufreq/Kconfig      |  1 +
 drivers/cpufreq/cpufreq-dt.c | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
index c3e6bd59e920..db2ad54ee67f 100644
--- a/drivers/cpufreq/Kconfig
+++ b/drivers/cpufreq/Kconfig
@@ -217,6 +217,7 @@ config CPUFREQ_DT
 
 config CPUFREQ_DT_PLATDEV
 	bool
+	depends on INTERCONNECT || !INTERCONNECT
 	help
 	  This adds a generic DT based cpufreq platdev driver for frequency
 	  management.  This creates a 'cpufreq-dt' platform device, on the
diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 26fe8dfb9ce6..4ecef3257532 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -13,6 +13,7 @@
 #include <linux/cpufreq.h>
 #include <linux/cpumask.h>
 #include <linux/err.h>
+#include <linux/interconnect.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/pm_opp.h>
@@ -95,6 +96,7 @@ static int resources_available(void)
 	struct device *cpu_dev;
 	struct regulator *cpu_reg;
 	struct clk *cpu_clk;
+	struct icc_path *cpu_path;
 	int ret = 0;
 	const char *name;
 
@@ -121,6 +123,19 @@ static int resources_available(void)
 
 	clk_put(cpu_clk);
 
+	cpu_path = of_icc_get(cpu_dev, NULL);
+	ret = PTR_ERR_OR_ZERO(cpu_path);
+	if (ret) {
+		if (ret == -EPROBE_DEFER)
+			dev_dbg(cpu_dev, "defer icc path: %d\n", ret);
+		else
+			dev_err(cpu_dev, "failed to get icc path: %d\n", ret);
+
+		return ret;
+	}
+
+	icc_put(cpu_path);
+
 	name = find_supply_name(cpu_dev);
 	/* Platform doesn't require regulator */
 	if (!name)

^ permalink raw reply related

* [PATCH v8 10/10] OPP: Add support for setting interconnect-tags
From: Georgi Djakov @ 2020-05-12 12:53 UTC (permalink / raw)
  To: vireshk, nm, sboyd, rjw, saravanak, sibis, mka
  Cc: robh+dt, rnayak, bjorn.andersson, vincent.guittot, jcrouse,
	evgreen, linux-pm, devicetree, linux-kernel, georgi.djakov
In-Reply-To: <20200512125327.1868-1-georgi.djakov@linaro.org>

From: Sibi Sankar <sibis@codeaurora.org>

Add support for setting tags on icc paths associated with
the opp_table.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
v8:
* New patch, picked from here:
  https://lore.kernel.org/r/20200504202243.5476-11-sibis@codeaurora.org

 drivers/opp/of.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 3a64f2aa0f86..fd148d54022f 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -336,6 +336,7 @@ int _of_find_icc_paths(struct opp_table *opp_table, struct device *dev)
 {
 	struct device_node *np;
 	int ret, i, count, num_paths;
+	u32 tag;
 
 	np = of_node_get(dev->of_node);
 	if (!np)
@@ -344,20 +345,25 @@ int _of_find_icc_paths(struct opp_table *opp_table, struct device *dev)
 	count = of_count_phandle_with_args(np, "interconnects",
 					   "#interconnect-cells");
 	of_node_put(np);
-	if (count < 0)
-		return 0;
+	if (count < 0) {
+		ret = 0;
+		goto put_np;
+	}
 
 	/* two phandles when #interconnect-cells = <1> */
 	if (count % 2) {
 		dev_err(dev, "%s: Invalid interconnects values\n", __func__);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto put_np;
 	}
 
 	num_paths = count / 2;
 	opp_table->paths = kcalloc(num_paths, sizeof(*opp_table->paths),
 				   GFP_KERNEL);
-	if (!opp_table->paths)
-		return -ENOMEM;
+	if (!opp_table->paths) {
+		ret = -ENOMEM;
+		goto put_np;
+	}
 
 	for (i = 0; i < num_paths; i++) {
 		opp_table->paths[i] = of_icc_get_by_index(dev, i);
@@ -369,8 +375,14 @@ int _of_find_icc_paths(struct opp_table *opp_table, struct device *dev)
 			}
 			goto err;
 		}
+
+		/* Set tag if present */
+		if (!of_property_read_u32_index(np, "interconnect-tags",
+						i, &tag))
+			icc_set_tag(opp_table->paths[i], tag);
 	}
 	opp_table->path_count = num_paths;
+	of_node_put(np);
 
 	return 0;
 
@@ -380,6 +392,8 @@ int _of_find_icc_paths(struct opp_table *opp_table, struct device *dev)
 
 	kfree(opp_table->paths);
 	opp_table->paths = NULL;
+put_np:
+	of_node_put(np);
 
 	return ret;
 }

^ permalink raw reply related

* [PATCH v8 09/10] dt-bindings: interconnect: Add interconnect-tags bindings
From: Georgi Djakov @ 2020-05-12 12:53 UTC (permalink / raw)
  To: vireshk, nm, sboyd, rjw, saravanak, sibis, mka
  Cc: robh+dt, rnayak, bjorn.andersson, vincent.guittot, jcrouse,
	evgreen, linux-pm, devicetree, linux-kernel, georgi.djakov
In-Reply-To: <20200512125327.1868-1-georgi.djakov@linaro.org>

From: Sibi Sankar <sibis@codeaurora.org>

Add interconnect-tags bindings to enable passing of optional
tag information to the interconnect framework.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
v8:
* New patch, picked from here:
  https://lore.kernel.org/r/20200504202243.5476-10-sibis@codeaurora.org

 .../devicetree/bindings/interconnect/interconnect.txt        | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/interconnect/interconnect.txt b/Documentation/devicetree/bindings/interconnect/interconnect.txt
index 6f5d23a605b7..c1a226a934e5 100644
--- a/Documentation/devicetree/bindings/interconnect/interconnect.txt
+++ b/Documentation/devicetree/bindings/interconnect/interconnect.txt
@@ -55,6 +55,11 @@ interconnect-names : List of interconnect path name strings sorted in the same
 			 * dma-mem: Path from the device to the main memory of
 			            the system
 
+interconnect-tags : List of interconnect path tags sorted in the same order as the
+		    interconnects property. Consumers can append a specific tag to
+		    the path and pass this information to the interconnect framework
+		    to do aggregation based on the attached tag.
+
 Example:
 
 	sdhci@7864000 {

^ permalink raw reply related

* [PATCH v8 06/10] OPP: Update the bandwidth on OPP frequency changes
From: Georgi Djakov @ 2020-05-12 12:53 UTC (permalink / raw)
  To: vireshk, nm, sboyd, rjw, saravanak, sibis, mka
  Cc: robh+dt, rnayak, bjorn.andersson, vincent.guittot, jcrouse,
	evgreen, linux-pm, devicetree, linux-kernel, georgi.djakov
In-Reply-To: <20200512125327.1868-1-georgi.djakov@linaro.org>

If the OPP bandwidth values are populated, we want to switch also the
interconnect bandwidth in addition to frequency and voltage.

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Sibi Sankar <sibis@codeaurora.org>
Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
v8:
* Picked reviewed-by tags.

 drivers/opp/core.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index a3dd0bc9b9f6..b4cc4b12d57b 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -808,7 +808,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 	unsigned long freq, old_freq, temp_freq;
 	struct dev_pm_opp *old_opp, *opp;
 	struct clk *clk;
-	int ret;
+	int ret, i;
 
 	opp_table = _find_opp_table(dev);
 	if (IS_ERR(opp_table)) {
@@ -909,6 +909,17 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 			dev_err(dev, "Failed to set required opps: %d\n", ret);
 	}
 
+	if (!ret && opp_table->paths) {
+		for (i = 0; i < opp_table->path_count; i++) {
+			ret = icc_set_bw(opp_table->paths[i],
+					 opp->bandwidth[i].avg,
+					 opp->bandwidth[i].peak);
+			if (ret)
+				dev_err(dev, "Failed to set bandwidth[%d]: %d\n",
+					i, ret);
+		}
+	}
+
 put_opp:
 	dev_pm_opp_put(opp);
 put_old_opp:

^ permalink raw reply related

* [PATCH v8 04/10] OPP: Add support for parsing interconnect bandwidth
From: Georgi Djakov @ 2020-05-12 12:53 UTC (permalink / raw)
  To: vireshk, nm, sboyd, rjw, saravanak, sibis, mka
  Cc: robh+dt, rnayak, bjorn.andersson, vincent.guittot, jcrouse,
	evgreen, linux-pm, devicetree, linux-kernel, georgi.djakov
In-Reply-To: <20200512125327.1868-1-georgi.djakov@linaro.org>

The OPP bindings now support bandwidth values, so add support to parse it
from device tree and store it into the new dev_pm_opp_icc_bw struct, which
is part of the dev_pm_opp.

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
v8:
* Drop bandwidth requests and free memory in _opp_table_kref_release.
* Take into account the supply_count in struct size calculations.
* Free the temporary arrays for peak and average bandwidth.
* Fix the check for opp-level.
* Use dev_warn insted of dev_dbg.
* Rename _of_find_icc_paths to _of_find_icc_paths.
* Rename the variable count to supply_count.

 drivers/opp/Kconfig    |   1 +
 drivers/opp/core.c     |  27 +++++++--
 drivers/opp/of.c       | 121 ++++++++++++++++++++++++++++++++++++++++-
 drivers/opp/opp.h      |   9 +++
 include/linux/pm_opp.h |  12 ++++
 5 files changed, 164 insertions(+), 6 deletions(-)

diff --git a/drivers/opp/Kconfig b/drivers/opp/Kconfig
index 35dfc7e80f92..230d2b84436c 100644
--- a/drivers/opp/Kconfig
+++ b/drivers/opp/Kconfig
@@ -2,6 +2,7 @@
 config PM_OPP
 	bool
 	select SRCU
+	depends on INTERCONNECT || !INTERCONNECT
 	---help---
 	  SOCs have a standard set of tuples consisting of frequency and
 	  voltage pairs that the device will support per voltage domain. This
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index ce7e4103ec09..a3dd0bc9b9f6 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -999,6 +999,12 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)
 				ret);
 	}
 
+	/* Find interconnect path(s) for the device */
+	ret = _of_find_icc_paths(opp_table, dev);
+	if (ret)
+		dev_warn(dev, "%s: Error finding interconnect paths: %d\n",
+			 __func__, ret);
+
 	BLOCKING_INIT_NOTIFIER_HEAD(&opp_table->head);
 	INIT_LIST_HEAD(&opp_table->opp_list);
 	kref_init(&opp_table->kref);
@@ -1057,6 +1063,7 @@ static void _opp_table_kref_release(struct kref *kref)
 {
 	struct opp_table *opp_table = container_of(kref, struct opp_table, kref);
 	struct opp_device *opp_dev, *temp;
+	int i;
 
 	_of_clear_opp_table(opp_table);
 
@@ -1064,6 +1071,12 @@ static void _opp_table_kref_release(struct kref *kref)
 	if (!IS_ERR(opp_table->clk))
 		clk_put(opp_table->clk);
 
+	if (opp_table->paths) {
+		for (i = 0; i < opp_table->path_count; i++)
+			icc_put(opp_table->paths[i]);
+		kfree(opp_table->paths);
+	}
+
 	WARN_ON(!list_empty(&opp_table->opp_list));
 
 	list_for_each_entry_safe(opp_dev, temp, &opp_table->dev_list, node) {
@@ -1243,19 +1256,22 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_remove_all_dynamic);
 struct dev_pm_opp *_opp_allocate(struct opp_table *table)
 {
 	struct dev_pm_opp *opp;
-	int count, supply_size;
+	int supply_count, supply_size, icc_size;
 
 	/* Allocate space for at least one supply */
-	count = table->regulator_count > 0 ? table->regulator_count : 1;
-	supply_size = sizeof(*opp->supplies) * count;
+	supply_count = table->regulator_count > 0 ? table->regulator_count : 1;
+	supply_size = sizeof(*opp->supplies) * supply_count;
+	icc_size = sizeof(*opp->bandwidth) * table->path_count;
 
 	/* allocate new OPP node and supplies structures */
-	opp = kzalloc(sizeof(*opp) + supply_size, GFP_KERNEL);
+	opp = kzalloc(sizeof(*opp) + supply_size + icc_size, GFP_KERNEL);
+
 	if (!opp)
 		return NULL;
 
 	/* Put the supplies at the end of the OPP structure as an empty array */
 	opp->supplies = (struct dev_pm_opp_supply *)(opp + 1);
+	opp->bandwidth = (struct dev_pm_opp_icc_bw *)(opp->supplies + supply_count);
 	INIT_LIST_HEAD(&opp->node);
 
 	return opp;
@@ -1290,6 +1306,9 @@ int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2)
 {
 	if (opp1->rate != opp2->rate)
 		return opp1->rate < opp2->rate ? -1 : 1;
+	if (opp1->bandwidth && opp2->bandwidth &&
+	    opp1->bandwidth[0].peak != opp2->bandwidth[0].peak)
+		return opp1->bandwidth[0].peak < opp2->bandwidth[0].peak ? -1 : 1;
 	if (opp1->level != opp2->level)
 		return opp1->level < opp2->level ? -1 : 1;
 	return 0;
diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 5179f034751a..d139ad8c8f4f 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -332,6 +332,58 @@ static int _of_opp_alloc_required_opps(struct opp_table *opp_table,
 	return ret;
 }
 
+int _of_find_icc_paths(struct opp_table *opp_table, struct device *dev)
+{
+	struct device_node *np;
+	int ret, i, count, num_paths;
+
+	np = of_node_get(dev->of_node);
+	if (!np)
+		return 0;
+
+	count = of_count_phandle_with_args(np, "interconnects",
+					   "#interconnect-cells");
+	of_node_put(np);
+	if (count < 0)
+		return 0;
+
+	/* two phandles when #interconnect-cells = <1> */
+	if (count % 2) {
+		dev_err(dev, "%s: Invalid interconnects values\n", __func__);
+		return -EINVAL;
+	}
+
+	num_paths = count / 2;
+	opp_table->paths = kcalloc(num_paths, sizeof(*opp_table->paths),
+				   GFP_KERNEL);
+	if (!opp_table->paths)
+		return -ENOMEM;
+
+	for (i = 0; i < num_paths; i++) {
+		opp_table->paths[i] = of_icc_get_by_index(dev, i);
+		if (IS_ERR(opp_table->paths[i])) {
+			ret = PTR_ERR(opp_table->paths[i]);
+			if (ret != -EPROBE_DEFER) {
+				dev_err(dev, "%s: Unable to get path%d: %d\n",
+					__func__, i, ret);
+			}
+			goto err;
+		}
+	}
+	opp_table->path_count = num_paths;
+
+	return 0;
+
+err:
+	while (i--)
+		icc_put(opp_table->paths[i]);
+
+	kfree(opp_table->paths);
+	opp_table->paths = NULL;
+
+	return ret;
+}
+
 static bool _opp_is_supported(struct device *dev, struct opp_table *opp_table,
 			      struct device_node *np)
 {
@@ -524,8 +576,11 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table);
 static int _read_opp_key(struct dev_pm_opp *new_opp, struct device_node *np,
 			 bool *rate_not_available)
 {
+	struct property *peak, *avg;
+	u32 *peak_bw, *avg_bw;
 	u64 rate;
-	int ret;
+	int ret, i, count;
+	bool found = false;
 
 	ret = of_property_read_u64(np, "opp-hz", &rate);
 	if (!ret) {
@@ -535,10 +590,72 @@ static int _read_opp_key(struct dev_pm_opp *new_opp, struct device_node *np,
 		 * bit guaranteed in clk API.
 		 */
 		new_opp->rate = (unsigned long)rate;
+		found = true;
 	}
 	*rate_not_available = !!ret;
 
-	of_property_read_u32(np, "opp-level", &new_opp->level);
+	peak = of_find_property(np, "opp-peak-kBps", NULL);
+	if (peak) {
+		/*
+		 * Bandwidth consists of peak and average (optional) values:
+		 * opp-peak-kBps = <path1_value path2_value>;
+		 * opp-avg-kBps = <path1_value path2_value>;
+		 */
+		count = peak->length / sizeof(u32);
+		peak_bw = kmalloc_array(count, sizeof(*peak_bw), GFP_KERNEL);
+		if (!peak_bw)
+			return -ENOMEM;
+
+		ret = of_property_read_u32_array(np, "opp-peak-kBps", peak_bw,
+						 count);
+		if (ret) {
+			pr_err("%s: Error parsing opp-peak-kBps: %d\n",
+			       __func__, ret);
+			goto free_peak_bw;
+		}
+
+		for (i = 0; i < count; i++)
+			new_opp->bandwidth[i].peak = kBps_to_icc(peak_bw[i]);
+
+		found = true;
+		kfree(peak_bw);
+	}
+
+	avg = of_find_property(np, "opp-avg-kBps", NULL);
+	if (peak && avg) {
+		count = avg->length / sizeof(u32);
+		avg_bw = kmalloc_array(count, sizeof(*avg_bw), GFP_KERNEL);
+		if (!avg_bw) {
+			ret = -ENOMEM;
+			goto free_peak_bw;
+		}
+
+		ret = of_property_read_u32_array(np, "opp-avg-kBps", avg_bw,
+						 count);
+		if (ret) {
+			pr_err("%s: Error parsing opp-avg-kBps: %d\n",
+			       __func__, ret);
+			goto free_avg_bw;
+		}
+
+		for (i = 0; i < count; i++)
+			new_opp->bandwidth[i].avg = kBps_to_icc(avg_bw[i]);
+
+		kfree(avg_bw);
+	}
+
+	if (!of_property_read_u32(np, "opp-level", &new_opp->level))
+		found = true;
+
+	if (found)
+		return 0;
+
+	return ret;
+
+free_avg_bw:
+	kfree(avg_bw);
+free_peak_bw:
+	kfree(peak_bw);
 
 	return ret;
 }
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index bcadb1e328a4..17d45119d9bc 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -12,6 +12,7 @@
 #define __DRIVER_OPP_H__
 
 #include <linux/device.h>
+#include <linux/interconnect.h>
 #include <linux/kernel.h>
 #include <linux/kref.h>
 #include <linux/list.h>
@@ -59,6 +60,7 @@ extern struct list_head opp_tables;
  * @rate:	Frequency in hertz
  * @level:	Performance level
  * @supplies:	Power supplies voltage/current values
+ * @bandwidth:	Interconnect bandwidth values
  * @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's
  *		frequency from any other OPP's frequency.
  * @required_opps: List of OPPs that are required by this OPP.
@@ -81,6 +83,7 @@ struct dev_pm_opp {
 	unsigned int level;
 
 	struct dev_pm_opp_supply *supplies;
+	struct dev_pm_opp_icc_bw *bandwidth;
 
 	unsigned long clock_latency_ns;
 
@@ -146,6 +149,8 @@ enum opp_table_access {
  * @regulator_count: Number of power supply regulators. Its value can be -1
  * (uninitialized), 0 (no opp-microvolt property) or > 0 (has opp-microvolt
  * property).
+ * @paths: Interconnect path handles
+ * @path_count: Number of interconnect paths
  * @genpd_performance_state: Device's power domain support performance state.
  * @is_genpd: Marks if the OPP table belongs to a genpd.
  * @set_opp: Platform specific set_opp callback
@@ -189,6 +194,8 @@ struct opp_table {
 	struct clk *clk;
 	struct regulator **regulators;
 	int regulator_count;
+	struct icc_path **paths;
+	unsigned int path_count;
 	bool genpd_performance_state;
 	bool is_genpd;
 
@@ -224,12 +231,14 @@ void _of_clear_opp_table(struct opp_table *opp_table);
 struct opp_table *_managed_opp(struct device *dev, int index);
 void _of_opp_free_required_opps(struct opp_table *opp_table,
 				struct dev_pm_opp *opp);
+int _of_find_icc_paths(struct opp_table *opp_table, struct device *dev);
 #else
 static inline void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index) {}
 static inline void _of_clear_opp_table(struct opp_table *opp_table) {}
 static inline struct opp_table *_managed_opp(struct device *dev, int index) { return NULL; }
 static inline void _of_opp_free_required_opps(struct opp_table *opp_table,
 					      struct dev_pm_opp *opp) {}
+static inline int _of_find_icc_paths(struct opp_table *opp_table, struct device *dev) { return 0; }
 #endif
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 747861816f4f..cfceb0290401 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -41,6 +41,18 @@ struct dev_pm_opp_supply {
 	unsigned long u_amp;
 };
 
+/**
+ * struct dev_pm_opp_icc_bw - Interconnect bandwidth values
+ * @avg:	Average bandwidth corresponding to this OPP (in icc units)
+ * @peak:	Peak bandwidth corresponding to this OPP (in icc units)
+ *
+ * This structure stores the bandwidth values for a single interconnect path.
+ */
+struct dev_pm_opp_icc_bw {
+	u32 avg;
+	u32 peak;
+};
+
 /**
  * struct dev_pm_opp_info - OPP freq/voltage/current values
  * @rate:	Target clk rate in hz

^ permalink raw reply related

* [PATCH v8 05/10] OPP: Add sanity checks in _read_opp_key()
From: Georgi Djakov @ 2020-05-12 12:53 UTC (permalink / raw)
  To: vireshk, nm, sboyd, rjw, saravanak, sibis, mka
  Cc: robh+dt, rnayak, bjorn.andersson, vincent.guittot, jcrouse,
	evgreen, linux-pm, devicetree, linux-kernel, georgi.djakov
In-Reply-To: <20200512125327.1868-1-georgi.djakov@linaro.org>

When we read the OPP keys, it would be nice to do some sanity checks
of the values we get from DT and see if they match with the information
that is populated in the OPP table. Let's pass a pointer of the table,
so that we can do some validation.

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Sibi Sankar <sibis@codeaurora.org>
Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
v8:
* Picked reviewed-by tags.

 drivers/opp/of.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index d139ad8c8f4f..3a64f2aa0f86 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -573,8 +573,8 @@ void dev_pm_opp_of_remove_table(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table);
 
-static int _read_opp_key(struct dev_pm_opp *new_opp, struct device_node *np,
-			 bool *rate_not_available)
+static int _read_opp_key(struct dev_pm_opp *new_opp, struct opp_table *table,
+			 struct device_node *np, bool *rate_not_available)
 {
 	struct property *peak, *avg;
 	u32 *peak_bw, *avg_bw;
@@ -602,6 +602,12 @@ static int _read_opp_key(struct dev_pm_opp *new_opp, struct device_node *np,
 		 * opp-avg-kBps = <path1_value path2_value>;
 		 */
 		count = peak->length / sizeof(u32);
+		if (table->path_count != count) {
+			pr_err("%s: Mismatch between opp-peak-kBps and paths (%d %d)\n",
+			       __func__, count, table->path_count);
+			return -EINVAL;
+		}
+
 		peak_bw = kmalloc_array(count, sizeof(*peak_bw), GFP_KERNEL);
 		if (!peak_bw)
 			return -ENOMEM;
@@ -624,6 +630,13 @@ static int _read_opp_key(struct dev_pm_opp *new_opp, struct device_node *np,
 	avg = of_find_property(np, "opp-avg-kBps", NULL);
 	if (peak && avg) {
 		count = avg->length / sizeof(u32);
+		if (table->path_count != count) {
+			pr_err("%s: Mismatch between opp-avg-kBps and paths (%d %d)\n",
+			       __func__, count, table->path_count);
+			ret = -EINVAL;
+			goto free_peak_bw;
+		}
+
 		avg_bw = kmalloc_array(count, sizeof(*avg_bw), GFP_KERNEL);
 		if (!avg_bw) {
 			ret = -ENOMEM;
@@ -697,7 +710,7 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table,
 	if (!new_opp)
 		return ERR_PTR(-ENOMEM);
 
-	ret = _read_opp_key(new_opp, np, &rate_not_available);
+	ret = _read_opp_key(new_opp, opp_table, np, &rate_not_available);
 	/* The key is optional for devices like power domains. */
 	if (ret < 0 && !opp_table->is_genpd) {
 		dev_err(dev, "%s: opp key field not found\n", __func__);

^ permalink raw reply related

* [PATCH v8 03/10] interconnect: Add of_icc_get_by_index() helper function
From: Georgi Djakov @ 2020-05-12 12:53 UTC (permalink / raw)
  To: vireshk, nm, sboyd, rjw, saravanak, sibis, mka
  Cc: robh+dt, rnayak, bjorn.andersson, vincent.guittot, jcrouse,
	evgreen, linux-pm, devicetree, linux-kernel, georgi.djakov
In-Reply-To: <20200512125327.1868-1-georgi.djakov@linaro.org>

This is the same as the traditional of_icc_get() function, but the
difference is that it takes index as an argument, instead of name.

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Sibi Sankar <sibis@codeaurora.org>
Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
v8:
* Dropped unneeded initializations.
* Picked reviewed-by tags.

 drivers/interconnect/core.c  | 72 +++++++++++++++++++++++++++---------
 include/linux/interconnect.h |  6 +++
 2 files changed, 60 insertions(+), 18 deletions(-)

diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index 2c6515e3ecf1..aba5d38ea9d1 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -351,9 +351,9 @@ static struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec)
 }
 
 /**
- * of_icc_get() - get a path handle from a DT node based on name
+ * of_icc_get_by_index() - get a path handle from a DT node based on index
  * @dev: device pointer for the consumer device
- * @name: interconnect path name
+ * @idx: interconnect path index
  *
  * This function will search for a path between two endpoints and return an
  * icc_path handle on success. Use icc_put() to release constraints when they
@@ -365,13 +365,12 @@ static struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec)
  * Return: icc_path pointer on success or ERR_PTR() on error. NULL is returned
  * when the API is disabled or the "interconnects" DT property is missing.
  */
-struct icc_path *of_icc_get(struct device *dev, const char *name)
+struct icc_path *of_icc_get_by_index(struct device *dev, int idx)
 {
-	struct icc_path *path = ERR_PTR(-EPROBE_DEFER);
+	struct icc_path *path;
 	struct icc_node *src_node, *dst_node;
-	struct device_node *np = NULL;
+	struct device_node *np;
 	struct of_phandle_args src_args, dst_args;
-	int idx = 0;
 	int ret;
 
 	if (!dev || !dev->of_node)
@@ -391,12 +390,6 @@ struct icc_path *of_icc_get(struct device *dev, const char *name)
 	 * lets support only global ids and extend this in the future if needed
 	 * without breaking DT compatibility.
 	 */
-	if (name) {
-		idx = of_property_match_string(np, "interconnect-names", name);
-		if (idx < 0)
-			return ERR_PTR(idx);
-	}
-
 	ret = of_parse_phandle_with_args(np, "interconnects",
 					 "#interconnect-cells", idx * 2,
 					 &src_args);
@@ -439,12 +432,8 @@ struct icc_path *of_icc_get(struct device *dev, const char *name)
 		return path;
 	}
 
-	if (name)
-		path->name = kstrdup_const(name, GFP_KERNEL);
-	else
-		path->name = kasprintf(GFP_KERNEL, "%s-%s",
-				       src_node->name, dst_node->name);
-
+	path->name = kasprintf(GFP_KERNEL, "%s-%s",
+			       src_node->name, dst_node->name);
 	if (!path->name) {
 		kfree(path);
 		return ERR_PTR(-ENOMEM);
@@ -452,6 +441,53 @@ struct icc_path *of_icc_get(struct device *dev, const char *name)
 
 	return path;
 }
+EXPORT_SYMBOL_GPL(of_icc_get_by_index);
+
+/**
+ * of_icc_get() - get a path handle from a DT node based on name
+ * @dev: device pointer for the consumer device
+ * @name: interconnect path name
+ *
+ * This function will search for a path between two endpoints and return an
+ * icc_path handle on success. Use icc_put() to release constraints when they
+ * are not needed anymore.
+ * If the interconnect API is disabled, NULL is returned and the consumer
+ * drivers will still build. Drivers are free to handle this specifically,
+ * but they don't have to.
+ *
+ * Return: icc_path pointer on success or ERR_PTR() on error. NULL is returned
+ * when the API is disabled or the "interconnects" DT property is missing.
+ */
+struct icc_path *of_icc_get(struct device *dev, const char *name)
+{
+	struct device_node *np;
+	int idx = 0;
+
+	if (!dev || !dev->of_node)
+		return ERR_PTR(-ENODEV);
+
+	np = dev->of_node;
+
+	/*
+	 * When the consumer DT node do not have "interconnects" property
+	 * return a NULL path to skip setting constraints.
+	 */
+	if (!of_find_property(np, "interconnects", NULL))
+		return NULL;
+
+	/*
+	 * We use a combination of phandle and specifier for endpoint. For now
+	 * lets support only global ids and extend this in the future if needed
+	 * without breaking DT compatibility.
+	 */
+	if (name) {
+		idx = of_property_match_string(np, "interconnect-names", name);
+		if (idx < 0)
+			return ERR_PTR(idx);
+	}
+
+	return of_icc_get_by_index(dev, idx);
+}
 EXPORT_SYMBOL_GPL(of_icc_get);
 
 /**
diff --git a/include/linux/interconnect.h b/include/linux/interconnect.h
index d70a914cba11..34e97231a6ab 100644
--- a/include/linux/interconnect.h
+++ b/include/linux/interconnect.h
@@ -28,6 +28,7 @@ struct device;
 struct icc_path *icc_get(struct device *dev, const int src_id,
 			 const int dst_id);
 struct icc_path *of_icc_get(struct device *dev, const char *name);
+struct icc_path *of_icc_get_by_index(struct device *dev, int idx);
 void icc_put(struct icc_path *path);
 int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw);
 void icc_set_tag(struct icc_path *path, u32 tag);
@@ -46,6 +47,11 @@ static inline struct icc_path *of_icc_get(struct device *dev,
 	return NULL;
 }
 
+static inline struct icc_path *of_icc_get_by_index(struct device *dev, int idx)
+{
+	return NULL;
+}
+
 static inline void icc_put(struct icc_path *path)
 {
 }

^ permalink raw reply related

* [PATCH v8 01/10] dt-bindings: opp: Introduce opp-peak-kBps and opp-avg-kBps bindings
From: Georgi Djakov @ 2020-05-12 12:53 UTC (permalink / raw)
  To: vireshk, nm, sboyd, rjw, saravanak, sibis, mka
  Cc: robh+dt, rnayak, bjorn.andersson, vincent.guittot, jcrouse,
	evgreen, linux-pm, devicetree, linux-kernel, georgi.djakov,
	Rob Herring
In-Reply-To: <20200512125327.1868-1-georgi.djakov@linaro.org>

From: Saravana Kannan <saravanak@google.com>

Interconnects often quantify their performance points in terms of
bandwidth. So, add opp-peak-kBps (required) and opp-avg-kBps (optional) to
allow specifying Bandwidth OPP tables in DT.

opp-peak-kBps is a required property that replaces opp-hz for Bandwidth OPP
tables.

opp-avg-kBps is an optional property that can be used in Bandwidth OPP
tables.

Signed-off-by: Saravana Kannan <saravanak@google.com>
Reviewed-by: Sibi Sankar <sibis@codeaurora.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
v8:
* Picked reviewed-by tags.
* Changes on wording.

 Documentation/devicetree/bindings/opp/opp.txt   | 17 ++++++++++++++---
 .../devicetree/bindings/property-units.txt      |  4 ++++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index 68592271461f..9d16d417e9be 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -83,9 +83,14 @@ properties.
 
 Required properties:
 - opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. This is a
-  required property for all device nodes but devices like power domains. The
-  power domain nodes must have another (implementation dependent) property which
-  uniquely identifies the OPP nodes.
+  required property for all device nodes, unless another "required" property to
+  uniquely identify the OPP nodes exists. Devices like power domains must have
+  another (implementation dependent) property.
+
+- opp-peak-kBps: Peak bandwidth in kilobytes per second, expressed as an array
+  of 32-bit big-endian integers. Each element of the array represents the
+  peak bandwidth value of each interconnect path. The number of elements should
+  match the number of interconnect paths.
 
 Optional properties:
 - opp-microvolt: voltage in micro Volts.
@@ -132,6 +137,12 @@ Optional properties:
 - opp-level: A value representing the performance level of the device,
   expressed as a 32-bit integer.
 
+- opp-avg-kBps: Average bandwidth in kilobytes per second, expressed as an array
+  of 32-bit big-endian integers. Each element of the array represents the
+  average bandwidth value of each interconnect path. The number of elements
+  should match the number of interconnect paths. This property is only
+  meaningful in OPP tables where opp-peak-kBps is present.
+
 - clock-latency-ns: Specifies the maximum possible transition latency (in
   nanoseconds) for switching to this OPP from any other OPP.
 
diff --git a/Documentation/devicetree/bindings/property-units.txt b/Documentation/devicetree/bindings/property-units.txt
index e9b8360b3288..c80a110c1e26 100644
--- a/Documentation/devicetree/bindings/property-units.txt
+++ b/Documentation/devicetree/bindings/property-units.txt
@@ -41,3 +41,7 @@ Temperature
 Pressure
 ----------------------------------------
 -kpascal	: kilopascal
+
+Throughput
+----------------------------------------
+-kBps		: kilobytes per second

^ permalink raw reply related

* [PATCH v8 00/10] Introduce OPP bandwidth bindings
From: Georgi Djakov @ 2020-05-12 12:53 UTC (permalink / raw)
  To: vireshk, nm, sboyd, rjw, saravanak, sibis, mka
  Cc: robh+dt, rnayak, bjorn.andersson, vincent.guittot, jcrouse,
	evgreen, linux-pm, devicetree, linux-kernel, georgi.djakov

Here is a proposal to extend the OPP bindings with bandwidth based on
a few previous discussions [1] and patchsets from me [2][3] and Saravana
[4][5][6][7][8][9].

Changes in v8:
* Addressed review comments from Matthias, Sibi and Viresh.
* Picked reviewed-by tags.
* Picked Sibi's interconnect-tag patches into this patchset.

Changes in v7: https://lore.kernel.org/r/20200424155404.10746-1-georgi.djakov@linaro.org
* This version is combination of both patchsets by Saravana and me, based
on [3] and [9].
* The latest version of DT bindings from Saravana is used here, with a
minor change of using arrays instead of single integers for opp-peak-kBps
and opp-avg-kBps. This is needed to support multiple interconnect paths.
* The concept of having multiple OPP tables per device has been dropped,
as it was nacked by Viresh.
* Various reviews comments have been addressed and some patches are
split, and there are also some new patches. Thanks to Viresh, Sibi and
others for providing feedback!

With this version of the patchset, the CPU/GPU to DDR bandwidth scaling
will look like this in DT:

One interconnect path (no change from Saravana's v6 patches):

cpu@0 {
	operating-points-v2 = <&cpu_opp_table>;
	interconnects = <&noc1 MASTER1 &noc2 SLAVE1>,
};

cpu_opp_table: cpu_opp_table {
	compatible = "operating-points-v2";

	opp-800000000 {
		opp-hz = /bits/ 64 <800000000>;
		opp-peak-kBps = <1525000>;
		opp-avg-kBps = <457000>;
	};

	opp-998400000 {
		opp-hz = /bits/ 64 <998400000>;
		opp-peak-kBps = <7614000>;
		opp-avg-kBps = <2284000>;
	};
};

Two interconnect paths:

cpu@0 {
	operating-points-v2 = <&cpu_opp_table>;
	interconnects = <&noc1 MASTER1 &noc2 SLAVE1>,
			<&noc3 MASTER2 &noc4 SLAVE2>;
};

cpu_opp_table: cpu_opp_table {
	compatible = "operating-points-v2";

	opp-800000000 {
		opp-hz = /bits/ 64 <800000000>;
		opp-peak-kBps = <1525000 2000>;
		opp-avg-kBps = <457000 1000>;
	};

	opp-998400000 {
		opp-hz = /bits/ 64 <998400000>;
		opp-peak-kBps = <7614000 4000>;
		opp-avg-kBps = <2284000 2000>;
	};
};

------

Every functional block on a SoC can contribute to the system power
efficiency by expressing its own bandwidth needs (to memory or other SoC
modules). This will allow the system to save power when high throughput
is not required (and also provide maximum throughput when needed).

There are at least three ways for a device to determine its bandwidth
needs:
	1. The device can dynamically calculate the needed bandwidth
based on some known variable. For example: UART (baud rate), I2C (fast
mode, high-speed mode, etc), USB (specification version, data transfer
type), SDHC (SD standard, clock rate, bus-width), Video Encoder/Decoder
(video format, resolution, frame-rate)

	2. There is a hardware specific value. For example: hardware
specific constant value (e.g. for PRNG) or use-case specific value that
is hard-coded.

	3. Predefined SoC/board specific bandwidth values. For example:
CPU or GPU bandwidth is related to the current core frequency and both
bandwidth and frequency are scaled together.

This patchset is trying to address point 3 above by extending the OPP
bindings to support predefined SoC/board bandwidth values and adds
support in cpufreq-dt to scale the interconnect between the CPU and the
DDR together with frequency and voltage.

[1] https://patchwork.kernel.org/patch/10577315/
[2] https://lore.kernel.org/r/20190313090010.20534-1-georgi.djakov@linaro.org/
[3] https://lore.kernel.org/r/20190423132823.7915-1-georgi.djakov@linaro.org/
[4] https://lore.kernel.org/r/20190608044339.115026-1-saravanak@google.com
[5] https://lore.kernel.org/r/20190614041733.120807-1-saravanak@google.com
[6] https://lore.kernel.org/r/20190703011020.151615-1-saravanak@google.com
[7] https://lore.kernel.org/r/20190726231558.175130-1-saravanak@google.com
[8] https://lore.kernel.org/r/20190807223111.230846-1-saravanak@google.com
[9] https://lore.kernel.org/r/20191207002424.201796-1-saravanak@google.com

Georgi Djakov (6):
  interconnect: Add of_icc_get_by_index() helper function
  OPP: Add support for parsing interconnect bandwidth
  OPP: Add sanity checks in _read_opp_key()
  OPP: Update the bandwidth on OPP frequency changes
  cpufreq: dt: Add support for interconnect bandwidth scaling
  cpufreq: dt: Validate all interconnect paths

Saravana Kannan (2):
  dt-bindings: opp: Introduce opp-peak-kBps and opp-avg-kBps bindings
  OPP: Add helpers for reading the binding properties

Sibi Sankar (2):
  dt-bindings: interconnect: Add interconnect-tags bindings
  OPP: Add support for setting interconnect-tags

 .../bindings/interconnect/interconnect.txt    |   5 +
 Documentation/devicetree/bindings/opp/opp.txt |  17 +-
 .../devicetree/bindings/property-units.txt    |   4 +
 drivers/cpufreq/Kconfig                       |   1 +
 drivers/cpufreq/cpufreq-dt.c                  |  54 +++++
 drivers/interconnect/core.c                   |  72 +++++--
 drivers/opp/Kconfig                           |   1 +
 drivers/opp/core.c                            |  55 ++++-
 drivers/opp/of.c                              | 189 ++++++++++++++++--
 drivers/opp/opp.h                             |  10 +
 include/linux/interconnect.h                  |   6 +
 include/linux/pm_opp.h                        |  12 ++
 12 files changed, 380 insertions(+), 46 deletions(-)


^ permalink raw reply

* Re: [PATCH net-next v2] dt-bindings: net: Convert UniPhier AVE4 controller to json-schema
From: Rob Herring @ 2020-05-12 12:51 UTC (permalink / raw)
  To: Kunihiko Hayashi
  Cc: David S . Miller, Masahiro Yamada, netdev, devicetree,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel@vger.kernel.org
In-Reply-To: <1589268410-17066-1-git-send-email-hayashi.kunihiko@socionext.com>

On Tue, May 12, 2020 at 2:27 AM Kunihiko Hayashi
<hayashi.kunihiko@socionext.com> wrote:
>
> Convert the UniPhier AVE4 controller binding to DT schema format.
>
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> ---
>
> Changes since v1:
> - Set true to phy-mode and phy-handle instead of $ref
> - Add mac-address and local-mac-address for existing dts warning
>
>  .../bindings/net/socionext,uniphier-ave4.txt       |  64 ------------
>  .../bindings/net/socionext,uniphier-ave4.yaml      | 111 +++++++++++++++++++++
>  MAINTAINERS                                        |   2 +-
>  3 files changed, 112 insertions(+), 65 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
>  create mode 100644 Documentation/devicetree/bindings/net/socionext,uniphier-ave4.yaml

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v3 10/16] gpio: add a reusable generic gpio_chip using regmap
From: Bartosz Golaszewski @ 2020-05-12 12:48 UTC (permalink / raw)
  To: Michael Walle
  Cc: Andy Shevchenko, linux-gpio, linux-devicetree, LKML, linux-hwmon,
	linux-pwm, LINUXWATCHDOG, arm-soc, Linus Walleij, Rob Herring,
	Jean Delvare, Guenter Roeck, Lee Jones, Thierry Reding,
	Uwe Kleine-König, Wim Van Sebroeck, Shawn Guo, Li Yang,
	Thomas Gleixner, Jason Cooper, Marc Zyngier, Mark Brown,
	Greg Kroah-Hartman
In-Reply-To: <20200423174543.17161-11-michael@walle.cc>

czw., 23 kwi 2020 o 19:46 Michael Walle <michael@walle.cc> napisał(a):
>
> There are quite a lot simple GPIO controller which are using regmap to
> access the hardware. This driver tries to be a base to unify existing
> code into one place. This won't cover everything but it should be a good
> starting point.
>
> It does not implement its own irq_chip because there is already a
> generic one for regmap based devices. Instead, the irq_chip will be
> instantiated in the parent driver and its irq domain will be associate
> to this driver.
>
> For now it consists of the usual registers, like set (and an optional
> clear) data register, an input register and direction registers.
> Out-of-the-box, it supports consecutive register mappings and mappings
> where the registers have gaps between them with a linear mapping between
> GPIO offset and bit position. For weirder mappings the user can register
> its own .xlate().
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---

This looks good to me. Please see some nits below.

>  drivers/gpio/Kconfig        |   4 +
>  drivers/gpio/Makefile       |   1 +
>  drivers/gpio/gpio-regmap.c  | 343 ++++++++++++++++++++++++++++++++++++
>  include/linux/gpio-regmap.h |  69 ++++++++
>  4 files changed, 417 insertions(+)
>  create mode 100644 drivers/gpio/gpio-regmap.c
>  create mode 100644 include/linux/gpio-regmap.h
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 8ef2179fb999..ae3a49a2e970 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -73,6 +73,10 @@ config GPIO_GENERIC
>         depends on HAS_IOMEM # Only for IOMEM drivers
>         tristate
>
> +config GPIO_REGMAP
> +       depends on REGMAP
> +       tristate
> +
>  # put drivers in the right section, in alphabetical order
>
>  # This symbol is selected by both I2C and SPI expanders
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index b2cfc21a97f3..93e139fdfa57 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_GPIO_SYSFS)      += gpiolib-sysfs.o
>  obj-$(CONFIG_GPIO_ACPI)                += gpiolib-acpi.o
>
>  # Device drivers. Generally keep list sorted alphabetically
> +obj-$(CONFIG_GPIO_REGMAP)      += gpio-regmap.o
>  obj-$(CONFIG_GPIO_GENERIC)     += gpio-generic.o
>
>  # directly supported by gpio-generic
> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> new file mode 100644
> index 000000000000..04939c0a22f9
> --- /dev/null
> +++ b/drivers/gpio/gpio-regmap.c
> @@ -0,0 +1,343 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * regmap based generic GPIO driver
> + *
> + * Copyright 2019 Michael Walle <michael@walle.cc>
> + */
> +
> +#include <linux/gpio/driver.h>
> +#include <linux/gpio-regmap.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +struct gpio_regmap {
> +       struct device *parent;
> +       struct regmap *regmap;
> +       struct gpio_chip gpio_chip;
> +
> +       int reg_stride;
> +       int ngpio_per_reg;
> +       unsigned int reg_dat_base;
> +       unsigned int reg_set_base;
> +       unsigned int reg_clr_base;
> +       unsigned int reg_dir_in_base;
> +       unsigned int reg_dir_out_base;
> +
> +       int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
> +                             unsigned int offset, unsigned int *reg,
> +                             unsigned int *mask);
> +
> +       void *driver_data;
> +};
> +
> +static unsigned int gpio_regmap_addr(unsigned int addr)
> +{
> +       return (addr == GPIO_REGMAP_ADDR_ZERO) ? 0 : addr;
> +}
> +
> +/**
> + * gpio_regmap_simple_xlate() - translate base/offset to reg/mask
> + *
> + * Use a simple linear mapping to translate the offset to the bitmask.
> + */
> +static int gpio_regmap_simple_xlate(struct gpio_regmap *gpio,
> +                                   unsigned int base, unsigned int offset,
> +                                   unsigned int *reg, unsigned int *mask)
> +{
> +       unsigned int line = offset % gpio->ngpio_per_reg;
> +       unsigned int stride = offset / gpio->ngpio_per_reg;
> +
> +       *reg = base + stride * gpio->reg_stride;
> +       *mask = BIT(line);
> +
> +       return 0;
> +}
> +
> +static int gpio_regmap_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +       struct gpio_regmap *gpio = gpiochip_get_data(chip);
> +       unsigned int base, val, reg, mask;
> +       int ret;
> +
> +       /* we might not have an output register if we are input only */
> +       if (gpio->reg_dat_base)
> +               base = gpio_regmap_addr(gpio->reg_dat_base);
> +       else
> +               base = gpio_regmap_addr(gpio->reg_set_base);
> +
> +       ret = gpio->reg_mask_xlate(gpio, base, offset, &reg, &mask);
> +       if (ret)
> +               return ret;
> +
> +       ret = regmap_read(gpio->regmap, reg, &val);
> +       if (ret)
> +               return ret;
> +
> +       return (val & mask) ? 1 : 0;
> +}
> +
> +static void gpio_regmap_set(struct gpio_chip *chip, unsigned int offset,
> +                           int val)
> +{
> +       struct gpio_regmap *gpio = gpiochip_get_data(chip);
> +       unsigned int base = gpio_regmap_addr(gpio->reg_set_base);
> +       unsigned int reg, mask;
> +
> +       gpio->reg_mask_xlate(gpio, base, offset, &reg, &mask);
> +       if (val)
> +               regmap_update_bits(gpio->regmap, reg, mask, mask);
> +       else
> +               regmap_update_bits(gpio->regmap, reg, mask, 0);
> +}
> +
> +static void gpio_regmap_set_with_clear(struct gpio_chip *chip,
> +                                      unsigned int offset, int val)
> +{
> +       struct gpio_regmap *gpio = gpiochip_get_data(chip);
> +       unsigned int base, reg, mask;
> +
> +       if (val)
> +               base = gpio_regmap_addr(gpio->reg_set_base);
> +       else
> +               base = gpio_regmap_addr(gpio->reg_clr_base);
> +
> +       gpio->reg_mask_xlate(gpio, base, offset, &reg, &mask);
> +       regmap_write(gpio->regmap, reg, mask);
> +}
> +
> +static int gpio_regmap_get_direction(struct gpio_chip *chip,
> +                                    unsigned int offset)
> +{
> +       struct gpio_regmap *gpio = gpiochip_get_data(chip);
> +       unsigned int base, val, reg, mask;
> +       int invert, ret;
> +
> +       if (gpio->reg_dir_out_base) {
> +               base = gpio_regmap_addr(gpio->reg_dir_out_base);
> +               invert = 0;
> +       } else if (gpio->reg_dir_in_base) {
> +               base = gpio_regmap_addr(gpio->reg_dir_in_base);
> +               invert = 1;
> +       } else {
> +               return GPIO_LINE_DIRECTION_IN;
> +       }
> +
> +       ret = gpio->reg_mask_xlate(gpio, base, offset, &reg, &mask);
> +       if (ret)
> +               return ret;
> +
> +       ret = regmap_read(gpio->regmap, reg, &val);
> +       if (ret)
> +               return ret;
> +
> +       if (!!(val & mask) ^ invert)
> +               return GPIO_LINE_DIRECTION_OUT;
> +       else
> +               return GPIO_LINE_DIRECTION_IN;
> +}
> +
> +static int gpio_regmap_set_direction(struct gpio_chip *chip,
> +                                    unsigned int offset, bool output)
> +{
> +       struct gpio_regmap *gpio = gpiochip_get_data(chip);
> +       unsigned int base, val, reg, mask;
> +       int invert, ret;
> +
> +       if (gpio->reg_dir_out_base) {
> +               base = gpio_regmap_addr(gpio->reg_dir_out_base);
> +               invert = 0;
> +       } else if (gpio->reg_dir_in_base) {
> +               base = gpio_regmap_addr(gpio->reg_dir_in_base);
> +               invert = 1;
> +       } else {
> +               return 0;
> +       }
> +
> +       ret = gpio->reg_mask_xlate(gpio, base, offset, &reg, &mask);
> +       if (ret)
> +               return ret;
> +
> +       if (!invert)
> +               val = (output) ? mask : 0;
> +       else
> +               val = (output) ? 0 : mask;
> +
> +       return regmap_update_bits(gpio->regmap, reg, mask, val);
> +}
> +
> +static int gpio_regmap_direction_input(struct gpio_chip *chip,
> +                                      unsigned int offset)
> +{
> +       return gpio_regmap_set_direction(chip, offset, false);
> +}
> +
> +static int gpio_regmap_direction_output(struct gpio_chip *chip,
> +                                       unsigned int offset, int value)
> +{
> +       gpio_regmap_set(chip, offset, value);
> +
> +       return gpio_regmap_set_direction(chip, offset, true);
> +}
> +
> +void gpio_regmap_set_drvdata(struct gpio_regmap *gpio, void *data)
> +{
> +       gpio->driver_data = data;
> +}
> +EXPORT_SYMBOL_GPL(gpio_regmap_set_drvdata);
> +
> +void *gpio_regmap_get_drvdata(struct gpio_regmap *gpio)
> +{
> +       return gpio->driver_data;
> +}
> +EXPORT_SYMBOL_GPL(gpio_regmap_get_drvdata);
> +
> +/**
> + * gpio_regmap_register() - Register a generic regmap GPIO controller
> + *
> + * @gpio: gpio_regmap device to register
> + *
> + * Returns 0 on success or an errno on failure.
> + */
> +struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config)
> +{
> +       struct gpio_regmap *gpio;
> +       struct gpio_chip *chip;
> +       int ret;
> +
> +       if (!config->parent)
> +               return ERR_PTR(-EINVAL);
> +
> +       if (!config->ngpio)
> +               return ERR_PTR(-EINVAL);
> +
> +       /* we need at least one */
> +       if (!config->reg_dat_base && !config->reg_set_base)
> +               return ERR_PTR(-EINVAL);
> +
> +       /* we don't support having both registers simulaniously for now */
> +       if (config->reg_dir_out_base && config->reg_dir_in_base)
> +               return ERR_PTR(-EINVAL);
> +
> +       gpio = kzalloc(sizeof(*gpio), GFP_KERNEL);
> +       if (!gpio)
> +               return ERR_PTR(-ENOMEM);
> +
> +       gpio->parent = config->parent;
> +       gpio->regmap = config->regmap;
> +       gpio->ngpio_per_reg = config->ngpio_per_reg;
> +       gpio->reg_stride = config->reg_stride;
> +       gpio->reg_mask_xlate = config->reg_mask_xlate;
> +       gpio->reg_set_base = config->reg_set_base;
> +       gpio->reg_clr_base = config->reg_clr_base;
> +       gpio->reg_dir_in_base = config->reg_dir_in_base;
> +       gpio->reg_dir_out_base = config->reg_dir_out_base;
> +
> +       /* if not set, assume there is only one register */
> +       if (!gpio->ngpio_per_reg)
> +               gpio->ngpio_per_reg = config->ngpio;
> +
> +       /* if not set, assume they are consecutive */
> +       if (!gpio->reg_stride)
> +               gpio->reg_stride = 1;
> +
> +       if (!gpio->reg_mask_xlate)
> +               gpio->reg_mask_xlate = gpio_regmap_simple_xlate;
> +
> +       chip = &gpio->gpio_chip;
> +       chip->parent = config->parent;
> +       chip->base = -1;
> +       chip->ngpio = config->ngpio;
> +       chip->can_sleep = true;
> +
> +       if (!chip->label)
> +               chip->label = dev_name(config->parent);
> +       else
> +               chip->label = config->label;
> +
> +       chip->get = gpio_regmap_get;
> +       if (gpio->reg_set_base && gpio->reg_clr_base)
> +               chip->set = gpio_regmap_set_with_clear;
> +       else if (gpio->reg_set_base)
> +               chip->set = gpio_regmap_set;
> +
> +       if (gpio->reg_dir_in_base || gpio->reg_dir_out_base) {
> +               chip->get_direction = gpio_regmap_get_direction;
> +               chip->direction_input = gpio_regmap_direction_input;
> +               chip->direction_output = gpio_regmap_direction_output;
> +       }
> +
> +       ret = gpiochip_add_data(chip, gpio);
> +       if (ret < 0) {
> +               kfree(gpio);

Maybe add a second label at the end of the function?

> +               return ERR_PTR(ret);
> +       }
> +
> +       if (config->irq_domain) {
> +               ret = gpiochip_irqchip_add_domain(chip, config->irq_domain);
> +               if (ret < 0)
> +                       goto err;
> +       }
> +
> +       return gpio;
> +
> +err:
> +       gpiochip_remove(chip);
> +       kfree(gpio);
> +       return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(gpio_regmap_register);
> +
> +/**
> + * gpio_regmap_unregister() - Unregister a generic regmap GPIO controller
> + *
> + * @gpio: gpio_regmap device to unregister
> + */
> +void gpio_regmap_unregister(struct gpio_regmap *gpio)
> +{
> +       gpiochip_remove(&gpio->gpio_chip);
> +       kfree(gpio);
> +}
> +EXPORT_SYMBOL_GPL(gpio_regmap_unregister);
> +
> +static void devm_gpio_regmap_unregister(struct device *dev, void *res)
> +{
> +       gpio_regmap_unregister(*(struct gpio_regmap **)res);
> +}
> +
> +/**
> + * devm_gpio_regmap_register() - resource managed gpio_regmap_register()
> + *
> + * @dev: device that is registering this GPIO device
> + * @gpio: gpio_regmap device to register
> + *
> + * Managed gpio_regmap_register(). For generic regmap GPIO device registered by
> + * this function, gpio_regmap_unregister() is automatically called on driver
> + * detach. See gpio_regmap_register() for more information.
> + */
> +struct gpio_regmap *devm_gpio_regmap_register(struct device *dev,
> +                                             const struct gpio_regmap_config *config)
> +{
> +       struct gpio_regmap **ptr, *gpio;
> +
> +       ptr = devres_alloc(devm_gpio_regmap_unregister, sizeof(*ptr),
> +                          GFP_KERNEL);
> +       if (!ptr)
> +               return ERR_PTR(-ENOMEM);
> +
> +       gpio = gpio_regmap_register(config);
> +
> +       if (!IS_ERR(gpio)) {
> +               *ptr = gpio;
> +               devres_add(dev, ptr);
> +       } else {
> +               devres_free(ptr);
> +       }
> +
> +       return gpio;
> +}
> +EXPORT_SYMBOL_GPL(devm_gpio_regmap_register);
> +
> +MODULE_AUTHOR("Michael Walle <michael@walle.cc>");
> +MODULE_DESCRIPTION("GPIO generic regmap driver core");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/gpio-regmap.h b/include/linux/gpio-regmap.h
> new file mode 100644
> index 000000000000..a868cbcde6e9
> --- /dev/null
> +++ b/include/linux/gpio-regmap.h
> @@ -0,0 +1,69 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef _LINUX_GPIO_REGMAP_H
> +#define _LINUX_GPIO_REGMAP_H
> +
> +struct gpio_regmap;
> +
> +#define GPIO_REGMAP_ADDR_ZERO ((unsigned long)(-1))
> +#define GPIO_REGMAP_ADDR(addr) ((addr) ? : GPIO_REGMAP_ADDR_ZERO)
> +

What if the addr is actually 0? Maybe drop GPIO_REGMAP_ADDR and
require users to set unused registers to GPIO_REGMAP_ADDR_ZERO?

> +/**
> + * struct gpio_regmap_config - Description of a generic regmap gpio_chip.
> + *
> + * @parent:            The parent device
> + * @regmap:            The regmap used to access the registers
> + *                     given, the name of the device is used
> + * @label:             (Optional) Descriptive name for GPIO controller.
> + *                     If not given, the name of the device is used.
> + * @ngpio:             Number of GPIOs
> + * @reg_dat_base:      (Optional) (in) register base address
> + * @reg_set_base:      (Optional) set register base address
> + * @reg_clr_base:      (Optional) clear register base address
> + * @reg_dir_in_base:   (Optional) out setting register base address
> + * @reg_dir_out_base:  (Optional) in setting register base address

The two above are inverted I think? Also: why the limitation of only
supporting one at a time?

> + * @reg_stride:                (Optional) May be set if the registers (of the
> + *                     same type, dat, set, etc) are not consecutive.
> + * @ngpio_per_reg:     Number of GPIOs per register
> + * @irq_domain:                (Optional) IRQ domain if the controller is
> + *                     interrupt-capable
> + * @reg_mask_xlate:     (Optional) Translates base address and GPIO
> + *                     offset to a register/bitmask pair. If not
> + *                     given the default gpio_regmap_simple_xlate()
> + *                     is used.
> + *
> + * The reg_mask_xlate translates a given base address and GPIO offset to
> + * register and mask pair. The base address is one of the given reg_*_base.
> + *
> + * All base addresses may have the special value GPIO_REGMAP_ADDR_ZERO
> + * which forces the address to the value 0.
> + */
> +struct gpio_regmap_config {
> +       struct device *parent;
> +       struct regmap *regmap;
> +
> +       const char *label;
> +       int ngpio;
> +
> +       unsigned int reg_dat_base;
> +       unsigned int reg_set_base;
> +       unsigned int reg_clr_base;
> +       unsigned int reg_dir_in_base;
> +       unsigned int reg_dir_out_base;
> +       int reg_stride;
> +       int ngpio_per_reg;
> +       struct irq_domain *irq_domain;
> +
> +       int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
> +                             unsigned int offset, unsigned int *reg,
> +                             unsigned int *mask);
> +};
> +
> +struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config);
> +void gpio_regmap_unregister(struct gpio_regmap *gpio);
> +struct gpio_regmap *devm_gpio_regmap_register(struct device *dev,
> +                                             const struct gpio_regmap_config *config);
> +void gpio_regmap_set_drvdata(struct gpio_regmap *gpio, void *data);
> +void *gpio_regmap_get_drvdata(struct gpio_regmap *gpio);
> +
> +#endif /* _LINUX_GPIO_REGMAP_H */
> --
> 2.20.1
>

^ permalink raw reply

* [PATCH v2 3/4] dt-bindings: serial: Add binding for rs485 bus termination GPIO
From: Lukas Wunner @ 2020-05-12 12:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring
  Cc: Matwey V. Kornilov, Giulio Benetti, Heiko Stuebner,
	Andy Shevchenko, Christoph Muellner, linux-serial, devicetree
In-Reply-To: <cover.1589285873.git.lukas@wunner.de>

Commit e8759ad17d41 ("serial: uapi: Add support for bus termination")
introduced the ability to enable rs485 bus termination from user space.
So far the feature is only used by a single driver, 8250_exar.c, using a
hardcoded GPIO pin specific to Siemens IOT2040 products.

Provide for a more generic solution by allowing specification of an
rs485 bus termination GPIO pin in the device tree.  An upcoming commit
implements support for this pin for any 8250 driver.  The binding is
used in device trees of the "Revolution Pi" PLCs offered by KUNBUS.

[Heiko Stuebner converted the binding to YAML, hence his Signed-off-by.]

Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
---
 Documentation/devicetree/bindings/serial/rs485.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/rs485.yaml b/Documentation/devicetree/bindings/serial/rs485.yaml
index d4beaf11222d..a9ad17864889 100644
--- a/Documentation/devicetree/bindings/serial/rs485.yaml
+++ b/Documentation/devicetree/bindings/serial/rs485.yaml
@@ -43,3 +43,7 @@ properties:
   rs485-rx-during-tx:
    description: enables the receiving of data even while sending data.
    $ref: /schemas/types.yaml#/definitions/flag
+
+  rs485-term-gpios:
+    description: GPIO pin to enable RS485 bus termination.
+    maxItems: 1
-- 
2.26.2


^ permalink raw reply related

* [PATCH v2 0/4] rs485 bus termination GPIO
From: Lukas Wunner @ 2020-05-12 12:40 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring
  Cc: Matwey V. Kornilov, Giulio Benetti, Heiko Stuebner,
	Andy Shevchenko, Christoph Muellner, linux-serial, devicetree

Define a device tree binding for an rs485 bus termination GPIO
(patch [3/4]), amend the serial core to retrieve the GPIO from
the device tree and amend the default ->rs485_config() callback
for 8250 drivers to change the GPIO on request from user space
(patch [4/4]).

Retrieving the GPIO from the device tree may fail, so allow
uart_get_rs485_mode() to return an errno and change all callers
to check for failure (patch [2/4]).

Fix a bug in the 8250 core if retrieval of the GPIO initially
fails with -EPROBE_DEFER and is later retried (patch [1/4]).


Changes v1 -> v2:

Patch [1/4]:
* Drop unnecessary "else" after "if ... goto" statement. (Andy)
* Document applicability to older kernels in commit message. (Andy)
* Add Fixes tag. (Andy)

Patch [4/4]:
* Drop unnecessary devm_gpiod_put(). (Andy)
* Use GPIOD_OUT_LOW macro for brevity. (Andy)
* Document the rationale for disabling termination by default.
* Drop nonsensical read of GPIO after setting its direction to out.


Link to v1:
https://lore.kernel.org/linux-serial/cover.1588505407.git.lukas@wunner.de/


Lukas Wunner (4):
  serial: 8250: Avoid error message on reprobe
  serial: Allow uart_get_rs485_mode() to return errno
  dt-bindings: serial: Add binding for rs485 bus termination GPIO
  serial: 8250: Support rs485 bus termination GPIO

 .../devicetree/bindings/serial/rs485.yaml     |  4 ++++
 drivers/tty/serial/8250/8250_core.c           | 18 ++++++++++-----
 drivers/tty/serial/8250/8250_port.c           |  4 ++++
 drivers/tty/serial/ar933x_uart.c              |  6 +++--
 drivers/tty/serial/atmel_serial.c             |  6 +++--
 drivers/tty/serial/fsl_lpuart.c               |  5 ++++-
 drivers/tty/serial/imx.c                      |  6 ++++-
 drivers/tty/serial/omap-serial.c              |  4 +++-
 drivers/tty/serial/serial_core.c              | 22 ++++++++++++++++++-
 drivers/tty/serial/stm32-usart.c              |  8 +++----
 include/linux/serial_core.h                   |  4 +++-
 11 files changed, 69 insertions(+), 18 deletions(-)

-- 
2.26.2


^ permalink raw reply

* Re: [PATCH 07/11] ASoC: mmp-sspa: Prepare/unprepare the clocks
From: Mark Brown @ 2020-05-12 12:45 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Liam Girdwood, Michael Turquette, Stephen Boyd, Rob Herring,
	linux-clk, devicetree, linux-kernel, linux-media
In-Reply-To: <20200511210134.1224532-8-lkundrak@v3.sk>

[-- Attachment #1: Type: text/plain, Size: 253 bytes --]

On Mon, May 11, 2020 at 11:01:30PM +0200, Lubomir Rintel wrote:
> The driver enables the clocks without preparing them and disables
> without unpreparing afterwards. Fix that.

This fix should've been earlier in the series so it could be sent as a
fix.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH v2 4/6] dmaengine: dw: Print warning if multi-block is unsupported
From: Serge Semin @ 2020-05-12 12:42 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Serge Semin, Mark Brown, Vinod Koul, Viresh Kumar, Dan Williams,
	Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
	Arnd Bergmann, Rob Herring, linux-mips, devicetree, dmaengine,
	Linux Kernel Mailing List
In-Reply-To: <20200511210800.GP185537@smile.fi.intel.com>

Vinod,

Could you join the discussion for a little bit?

In order to properly fix the problem discussed in this topic, we need to
introduce an additional capability exported by DMA channel handlers on per-channel
basis. It must be a number, which would indicate an upper limitation of the SG list
entries amount.
Something like this would do it:
struct dma_slave_caps {
...
	unsigned int max_sg_nents;
...
};
As Andy suggested it's value should be interpreted as:
0          - unlimited number of entries,
1:MAX_UINT - actual limit to the number of entries.

In addition to that seeing the dma_get_slave_caps() method provide the caps only
by getting them from the DMA device descriptor, while we need to have an info on
per-channel basis, it would be good to introduce a new DMA-device callback like:
struct dma_device {
...
	int (*device_caps)(struct dma_chan *chan,
			   struct dma_slave_caps *caps);
...
};
So the DMA driver could override the generic DMA device capabilities with the
values specific to the DMA channels. Such functionality will be also helpful for
the max-burst-len parameter introduced by this patchset, since depending on the
IP-core synthesis parameters it may be channel-specific.

Alternatively we could just introduce a new fields to the dma_chan structure and
retrieve the new caps values from them in the dma_get_slave_caps() method.
Though the solution with callback I like better.

What is your opinion about this? What solution you'd prefer?

On Tue, May 12, 2020 at 12:08:00AM +0300, Andy Shevchenko wrote:
> On Tue, May 12, 2020 at 12:07:14AM +0300, Andy Shevchenko wrote:
> > On Mon, May 11, 2020 at 10:32:55PM +0300, Serge Semin wrote:
> > > On Mon, May 11, 2020 at 04:58:53PM +0300, Andy Shevchenko wrote:
> > > > On Mon, May 11, 2020 at 4:48 PM Serge Semin
> > > > <Sergey.Semin@baikalelectronics.ru> wrote:
> > > > >
> > > > > On Mon, May 11, 2020 at 12:58:13PM +0100, Mark Brown wrote:
> > > > > > On Mon, May 11, 2020 at 05:10:16AM +0300, Serge Semin wrote:
> > > > > >
> > > > > > > Alas linearizing the SPI messages won't help in this case because the DW DMA
> > > > > > > driver will split it into the max transaction chunks anyway.
> > > > > >
> > > > > > That sounds like you need to also impose a limit on the maximum message
> > > > > > size as well then, with that you should be able to handle messages up
> > > > > > to whatever that limit is.  There's code for that bit already, so long
> > > > > > as the limit is not too low it should be fine for most devices and
> > > > > > client drivers can see the limit so they can be updated to work with it
> > > > > > if needed.
> > > > >
> > > > > Hmm, this might work. The problem will be with imposing such limitation through
> > > > > the DW APB SSI driver. In order to do this I need to know:
> > > > > 1) Whether multi-block LLP is supported by the DW DMA controller.
> > > > > 2) Maximum DW DMA transfer block size.
> > > > > Then I'll be able to use this information in the can_dma() callback to enable
> > > > > the DMA xfers only for the safe transfers. Did you mean something like this when
> > > > > you said "There's code for that bit already" ? If you meant the max_dma_len
> > > > > parameter, then setting it won't work, because it just limits the SG items size
> > > > > not the total length of a single transfer.
> > > > >
> > > > > So the question is of how to export the multi-block LLP flag from DW DMAc
> > > > > driver. Andy?
> > > > 
> > > > I'm not sure I understand why do you need this being exported. Just
> > > > always supply SG list out of single entry and define the length
> > > > according to the maximum segment size (it's done IIRC in SPI core).
> > > 
> > > Finally I see your point. So you suggest to feed the DMA engine with SG list
> > > entries one-by-one instead of sending all of them at once in a single
> > > dmaengine_prep_slave_sg() -> dmaengine_submit() -> dma_async_issue_pending()
> > > session. Hm, this solution will work, but there is an issue. There is no
> > > guarantee, that Tx and Rx SG lists are symmetric, consisting of the same
> > > number of items with the same sizes. It depends on the Tx/Rx buffers physical
> > > address alignment and their offsets within the memory pages. Though this
> > > problem can be solved by making the Tx and Rx SG lists symmetric. I'll have
> > > to implement a clever DMA IO loop, which would extract the DMA
> > > addresses/lengths from the SG entries and perform the single-buffer DMA 
> > > transactions with the DMA buffers of the same length.
> > > 
> > > Regarding noLLP being exported. Obviously I intended to solve the problem in a
> > > generic way since the problem is common for noLLP DW APB SSI/DW DMAC combination.
> > > In order to do this we need to know whether the multi-block LLP feature is
> > > unsupported by the DW DMA controller. We either make such info somehow exported
> > > from the DW DMA driver, so the DMA clients (like Dw APB SSI controller driver)
> > > could be ready to work around the problem; or just implement a flag-based quirk
> > > in the DMA client driver, which would be enabled in the platform-specific basis
> > > depending on the platform device actually detected (for instance, a specific
> > > version of the DW APB SSI IP). AFAICS You'd prefer the later option. 
> > 
> > So, we may extend the struct of DMA parameters to tell the consumer amount of entries (each of which is no longer than maximum segment size) it can afford:
> > - 0: Auto (DMA driver handles any cases itself)
> > - 1: Only single entry
> > - 2: Up to two...
> 
> It will left implementation details (or i.o.w. obstacles or limitation) why DMA
> can't do otherwise.

Sounds good. Thanks for assistance.

-Sergey

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

^ permalink raw reply

* Re: [PATCH v2 2/6] dt-bindings: dma: dw: Add max burst transaction length property
From: Andy Shevchenko @ 2020-05-12 12:38 UTC (permalink / raw)
  To: Serge Semin
  Cc: Serge Semin, Vinod Koul, Viresh Kumar, Rob Herring,
	Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
	Arnd Bergmann, Dan Williams, linux-mips, dmaengine, devicetree,
	linux-kernel
In-Reply-To: <20200512114946.x777yb6bhe22ccn5@mobilestation>

On Tue, May 12, 2020 at 02:49:46PM +0300, Serge Semin wrote:
> On Tue, May 12, 2020 at 12:08:04PM +0300, Andy Shevchenko wrote:
> > On Tue, May 12, 2020 at 12:35:31AM +0300, Serge Semin wrote:
> > > On Tue, May 12, 2020 at 12:01:38AM +0300, Andy Shevchenko wrote:
> > > > On Mon, May 11, 2020 at 11:05:28PM +0300, Serge Semin wrote:
> > > > > On Fri, May 08, 2020 at 02:12:42PM +0300, Andy Shevchenko wrote:
> > > > > > On Fri, May 08, 2020 at 01:53:00PM +0300, Serge Semin wrote:

...

I leave it to Rob and Vinod.
It won't break our case, so, feel free with your approach.

P.S. Perhaps at some point we need to
1) convert properties to be u32 (it will simplify things);
2) convert legacy ones to proper format ('-' instead of '_', vendor prefix added);
3) parse them in core with device property API.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH 2/3] ASoC: fsl_esai: Add support for imx8qm
From: Mark Brown @ 2020-05-12 12:38 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: Shengjiu Wang,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux-ALSA, Timur Tabi, Xiubo Li, Liam Girdwood, linuxppc-dev,
	Takashi Iwai, Nicolin Chen, Rob Herring, Fabio Estevam,
	linux-kernel
In-Reply-To: <CAA+D8ANK+Sd=nPeDZpd_=fQRFOdLtKgvsCmfQ_fRU3RCjMY+rQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 823 bytes --]

On Tue, May 12, 2020 at 10:48:41AM +0800, Shengjiu Wang wrote:
> On Wed, May 6, 2020 at 10:33 AM Shengjiu Wang <shengjiu.wang@gmail.com> wrote:
> > On Fri, May 1, 2020 at 6:23 PM Mark Brown <broonie@kernel.org> wrote:

> > > > EDMA requires the period size to be multiple of maxburst. Otherwise
> > > > the remaining bytes are not transferred and thus noise is produced.

> > > If this constraint comes from the DMA controller then normally you'd
> > > expect the DMA controller integration to be enforcing this - is there no
> > > information in the DMA API that lets us know that this constraint is
> > > there?

> > No, I can't find one API for this.
> > Do you have a recommendation?

> could you please recommend which DMA API can I use?

Not off-hand, you'd probably need to extend the API to export the
information.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH v2 3/6] dmaengine: dw: Set DMA device max segment size parameter
From: Andy Shevchenko @ 2020-05-12 12:35 UTC (permalink / raw)
  To: Serge Semin
  Cc: Serge Semin, Vineet Gupta, Vinod Koul, Viresh Kumar, Dan Williams,
	Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
	Arnd Bergmann, Rob Herring, linux-mips, devicetree, dmaengine,
	linux-kernel
In-Reply-To: <20200511211622.yuh3ls2ay76yaxrf@mobilestation>

On Tue, May 12, 2020 at 12:16:22AM +0300, Serge Semin wrote:
> On Fri, May 08, 2020 at 02:21:52PM +0300, Andy Shevchenko wrote:
> > On Fri, May 08, 2020 at 01:53:01PM +0300, Serge Semin wrote:
> > > Maximum block size DW DMAC configuration corresponds to the max segment
> > > size DMA parameter in the DMA core subsystem notation. Lets set it with a
> > > value specific to the probed DW DMA controller. It shall help the DMA
> > > clients to create size-optimized SG-list items for the controller. This in
> > > turn will cause less dw_desc allocations, less LLP reinitializations,
> > > better DMA device performance.

> > Yeah, I have locally something like this and I didn't dare to upstream because
> > there is an issue. We have this information per DMA controller, while we
> > actually need this on per DMA channel basis.
> > 
> > Above will work only for synthesized DMA with all channels having same block
> > size. That's why above conditional is not needed anyway.
> 
> Hm, I don't really see why the conditional isn't needed and this won't work. As
> you can see in the loop above Initially I find a maximum of all channels maximum
> block sizes and use it then as a max segment size parameter for the whole device.
> If the DW DMA controller has the same max block size of all channels, then it
> will be found. If the channels've been synthesized with different block sizes,
> then the optimization will work for the one with greatest block size. The SG
> list entries of the channels with lesser max block size will be split up
> by the DW DMAC driver, which would have been done anyway without
> max_segment_size being set. Here we at least provide the optimization for the
> channels with greatest max block size.
> 
> I do understand that it would be good to have this parameter setup on per generic
> DMA channel descriptor basis. But DMA core and device descriptor doesn't provide
> such facility, so setting at least some justified value is a good idea.
> 
> > 
> > OTOH, I never saw the DesignWare DMA to be synthesized differently (I remember
> > that Intel Medfield has interesting settings, but I don't remember if DMA
> > channels are different inside the same controller).
> > 
> > Vineet, do you have any information that Synopsys customers synthesized DMA
> > controllers with different channel characteristics inside one DMA IP?
> 
> AFAICS the DW DMAC channels can be synthesized with different max block size.
> The IP core supports such configuration. So we can't assume that such DMAC
> release can't be found in a real hardware just because we've never seen one.
> No matter what Vineet will have to say in response to your question.

My point here that we probably can avoid complications till we have real
hardware where it's different. As I said I don't remember a such, except
*maybe* Intel Medfield, which is quite outdated and not supported for wider
audience anyway.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox