Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 07/13] ASoC: ti: davinci-i2s: Add TDM support
From: Bastien Curutchet @ 2024-04-02  7:12 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Peter Ujfalusi, Jaroslav Kysela, Takashi Iwai,
	Bastien Curutchet
  Cc: linux-sound, devicetree, linux-kernel, alsa-devel,
	Thomas Petazzoni, herve.codina, christophercordahi
In-Reply-To: <20240402071213.11671-1-bastien.curutchet@bootlin.com>

TDM is not supported by the McBSP driver. The McBSP datasheet does not
name explicitly TDM as a supported format but it is possible to configure
the McBSP to do TDM if all slots are used by McBSP.

Add TDM support. It uses single-phase frame. Slot width is used to
compute the McBSP's word length.

Implement the set_tdm_slot() hook of snd_soc_dai_ops struct. It only
supports TDM if all slots are used by McBSP.

The snd_soc_dai_driver's channels_max is updated from 2 to 128.

This was tested with BP_FC format on a platform designed off of
DAVINCI/OMAP_L138. A check is done in davinci_i2s_set_dai_fmt() to
prevent TDM to be used with BC_FC and BC_FP formats.

Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
 sound/soc/ti/davinci-i2s.c | 92 +++++++++++++++++++++++++++++++++++---
 1 file changed, 87 insertions(+), 5 deletions(-)

diff --git a/sound/soc/ti/davinci-i2s.c b/sound/soc/ti/davinci-i2s.c
index a2c7c812ea50..f9a67c2bc2f5 100644
--- a/sound/soc/ti/davinci-i2s.c
+++ b/sound/soc/ti/davinci-i2s.c
@@ -160,6 +160,9 @@ struct davinci_mcbsp_dev {
 	unsigned int fmt;
 	int clk_div;
 	bool i2s_accurate_sck;
+
+	int tdm_slots;
+	int slot_width;
 };
 
 static inline void davinci_mcbsp_write_reg(struct davinci_mcbsp_dev *dev,
@@ -213,6 +216,63 @@ static void davinci_mcbsp_stop(struct davinci_mcbsp_dev *dev, int playback)
 	toggle_clock(dev, playback);
 }
 
+static int davinci_i2s_tdm_word_length(int tdm_slot_width)
+{
+	switch (tdm_slot_width) {
+	case 8:
+		return DAVINCI_MCBSP_WORD_8;
+	case 12:
+		return DAVINCI_MCBSP_WORD_12;
+	case 16:
+		return DAVINCI_MCBSP_WORD_16;
+	case 20:
+		return DAVINCI_MCBSP_WORD_20;
+	case 24:
+		return DAVINCI_MCBSP_WORD_24;
+	case 32:
+		return DAVINCI_MCBSP_WORD_32;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int davinci_i2s_set_tdm_slot(struct snd_soc_dai *cpu_dai,
+				    unsigned int tx_mask,
+				    unsigned int rx_mask,
+				    int slots, int slot_width)
+{
+	struct davinci_mcbsp_dev *dev = snd_soc_dai_get_drvdata(cpu_dai);
+
+	dev_dbg(dev->dev, "slots %d, slot_width %d\n", slots, slot_width);
+
+	if (slots > 128 || !slots) {
+		dev_err(dev->dev, "Invalid number of slots\n");
+		return -EINVAL;
+	}
+
+	if (rx_mask != (1 << slots) - 1) {
+		dev_err(dev->dev, "Invalid RX mask (0x%08x) : all slots must be used by McBSP\n",
+			rx_mask);
+		return -EINVAL;
+	}
+
+	if (tx_mask != (1 << slots) - 1) {
+		dev_err(dev->dev, "Invalid TX mask (0x%08x) : all slots must be used by McBSP\n",
+			tx_mask);
+		return -EINVAL;
+	}
+
+	if (davinci_i2s_tdm_word_length(slot_width) < 0) {
+		dev_err(dev->dev, "%s: Unsupported slot_width %d\n", __func__, slot_width);
+		return -EINVAL;
+	}
+
+	dev->tdm_slots = slots;
+	dev->slot_width = slot_width;
+
+	return 0;
+}
+
 #define DEFAULT_BITPERSAMPLE	16
 
 static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
@@ -238,6 +298,11 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 			DAVINCI_MCBSP_PCR_CLKRM;
 		break;
 	case SND_SOC_DAIFMT_BC_FP:
+		if (dev->tdm_slots || dev->slot_width) {
+			dev_err(dev->dev, "TDM is not supported for BC_FP format\n");
+			return -EINVAL;
+		}
+
 		/*
 		 * McBSP CLKR pin is the input for the Sample Rate Generator.
 		 * McBSP FSR and FSX are driven by the Sample Rate Generator.
@@ -246,6 +311,11 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		pcr |= DAVINCI_MCBSP_PCR_SCLKME;
 		break;
 	case SND_SOC_DAIFMT_BC_FC:
+		if (dev->tdm_slots || dev->slot_width) {
+			dev_err(dev->dev, "TDM is not supported for BC_FC format\n");
+			return -EINVAL;
+		}
+
 		/* codec is master */
 		pcr = 0;
 		break;
@@ -383,7 +453,13 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	master = dev->fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK;
 	fmt = params_format(params);
-	mcbsp_word_length = asp_word_length[fmt];
+	if (dev->slot_width)
+		mcbsp_word_length = davinci_i2s_tdm_word_length(dev->slot_width);
+	else
+		mcbsp_word_length = asp_word_length[fmt];
+
+	if (mcbsp_word_length < 0)
+		return mcbsp_word_length;
 
 	switch (master) {
 	case SND_SOC_DAIFMT_BP_FP:
@@ -483,8 +559,13 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 	switch (master) {
 	case SND_SOC_DAIFMT_BP_FP:
 	case SND_SOC_DAIFMT_BP_FC:
-		rcr |= DAVINCI_MCBSP_RCR_RFRLEN1(0);
-		xcr |= DAVINCI_MCBSP_XCR_XFRLEN1(0);
+		if (dev->tdm_slots > 0) {
+			rcr |= DAVINCI_MCBSP_RCR_RFRLEN1(dev->tdm_slots - 1);
+			xcr |= DAVINCI_MCBSP_XCR_XFRLEN1(dev->tdm_slots - 1);
+		} else {
+			rcr |= DAVINCI_MCBSP_RCR_RFRLEN1(0);
+			xcr |= DAVINCI_MCBSP_XCR_XFRLEN1(0);
+		}
 		break;
 	case SND_SOC_DAIFMT_BC_FC:
 	case SND_SOC_DAIFMT_BC_FP:
@@ -609,19 +690,20 @@ static const struct snd_soc_dai_ops davinci_i2s_dai_ops = {
 	.hw_params	= davinci_i2s_hw_params,
 	.set_fmt	= davinci_i2s_set_dai_fmt,
 	.set_clkdiv	= davinci_i2s_dai_set_clkdiv,
+	.set_tdm_slot   = davinci_i2s_set_tdm_slot,
 
 };
 
 static struct snd_soc_dai_driver davinci_i2s_dai = {
 	.playback = {
 		.channels_min = 2,
-		.channels_max = 2,
+		.channels_max = 128,
 		.rates = DAVINCI_I2S_RATES,
 		.formats = DAVINCI_I2S_FORMATS,
 	},
 	.capture = {
 		.channels_min = 2,
-		.channels_max = 2,
+		.channels_max = 128,
 		.rates = DAVINCI_I2S_RATES,
 		.formats = DAVINCI_I2S_FORMATS,
 	},
-- 
2.44.0


^ permalink raw reply related

* [PATCH v2 08/13] ASoC: ti: davinci-i2s: Add handling of BP_FC format
From: Bastien Curutchet @ 2024-04-02  7:12 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Peter Ujfalusi, Jaroslav Kysela, Takashi Iwai,
	Bastien Curutchet
  Cc: linux-sound, devicetree, linux-kernel, alsa-devel,
	Thomas Petazzoni, herve.codina, christophercordahi
In-Reply-To: <20240402071213.11671-1-bastien.curutchet@bootlin.com>

McBSP is able to drive bit clock and consume frame clock but BP_FC
format is not handled by McBSP driver.

Add BP_FC format support.
When BP_FC is selected:
  - CLKX and CLKR are configured as outputs
  - The sample rate generator is configured to be able to provide bit
    clock.

Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
 sound/soc/ti/davinci-i2s.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/sound/soc/ti/davinci-i2s.c b/sound/soc/ti/davinci-i2s.c
index f9a67c2bc2f5..e51f05cda941 100644
--- a/sound/soc/ti/davinci-i2s.c
+++ b/sound/soc/ti/davinci-i2s.c
@@ -310,6 +310,12 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		pcr = DAVINCI_MCBSP_PCR_FSRM | DAVINCI_MCBSP_PCR_FSXM;
 		pcr |= DAVINCI_MCBSP_PCR_SCLKME;
 		break;
+	case SND_SOC_DAIFMT_BP_FC:
+		/* cpu is bitclock provider */
+		pcr = DAVINCI_MCBSP_PCR_CLKXM |
+			DAVINCI_MCBSP_PCR_CLKRM;
+		break;
+
 	case SND_SOC_DAIFMT_BC_FC:
 		if (dev->tdm_slots || dev->slot_width) {
 			dev_err(dev->dev, "TDM is not supported for BC_FC format\n");
@@ -500,6 +506,23 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 		clk_div &= 0xFF;
 		srgr |= clk_div;
 		break;
+	case SND_SOC_DAIFMT_BP_FC:
+		if (dev->ext_clk) {
+			freq = clk_get_rate(dev->ext_clk);
+		} else {
+			freq = clk_get_rate(dev->clk);
+			srgr = DAVINCI_MCBSP_SRGR_CLKSM;
+		}
+		if (dev->tdm_slots && dev->slot_width) {
+			clk_div = freq / (params->rate_num * params->rate_den)
+				 / (dev->tdm_slots * dev->slot_width) - 1;
+		} else {
+			clk_div = freq / (mcbsp_word_length * 16) /
+				  params->rate_num * params->rate_den;
+		}
+		clk_div &= 0xFF;
+		srgr |= clk_div;
+		break;
 	case SND_SOC_DAIFMT_BC_FC:
 		/* Clock and frame sync given from external sources */
 		i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS);
-- 
2.44.0


^ permalink raw reply related

* [PATCH v2 09/13] ASoC: ti: davinci-i2s: Enable unexpected frame pulses detection
From: Bastien Curutchet @ 2024-04-02  7:12 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Peter Ujfalusi, Jaroslav Kysela, Takashi Iwai,
	Bastien Curutchet
  Cc: linux-sound, devicetree, linux-kernel, alsa-devel,
	Thomas Petazzoni, herve.codina, christophercordahi
In-Reply-To: <20240402071213.11671-1-bastien.curutchet@bootlin.com>

McBSP can generate a SYNCERR when unexpected frame pulses are
detected. The driver always disables this feature and ignore the
unexpected frame pulses.

Enable the generation of SYNCERR by the McBSP. Unexpected frame
pulses are not ignored anymore.

Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
 sound/soc/ti/davinci-i2s.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/ti/davinci-i2s.c b/sound/soc/ti/davinci-i2s.c
index e51f05cda941..82a0dfa07cec 100644
--- a/sound/soc/ti/davinci-i2s.c
+++ b/sound/soc/ti/davinci-i2s.c
@@ -441,8 +441,10 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 	struct davinci_mcbsp_dev *dev = snd_soc_dai_get_drvdata(dai);
 	struct snd_interval *i = NULL;
 	int mcbsp_word_length, master;
-	unsigned int rcr, xcr, clk_div, freq, framesize;
+	unsigned int clk_div, freq, framesize;
 	unsigned int srgr = 0;
+	unsigned int rcr = 0;
+	unsigned int xcr = 0;
 	u32 spcr;
 	snd_pcm_format_t fmt;
 	unsigned element_cnt = 1;
@@ -539,8 +541,6 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 	}
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr);
 
-	rcr = DAVINCI_MCBSP_RCR_RFIG;
-	xcr = DAVINCI_MCBSP_XCR_XFIG;
 	if (dev->mode == MOD_DSP_B) {
 		rcr |= DAVINCI_MCBSP_RCR_RDATDLY(0);
 		xcr |= DAVINCI_MCBSP_XCR_XDATDLY(0);
-- 
2.44.0


^ permalink raw reply related

* [PATCH v2 10/13] ASoC: ti: davinci-i2s: Link free-run mode to SND_SOC_DAIFMT_[GATED/CONT]
From: Bastien Curutchet @ 2024-04-02  7:12 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Peter Ujfalusi, Jaroslav Kysela, Takashi Iwai,
	Bastien Curutchet
  Cc: linux-sound, devicetree, linux-kernel, alsa-devel,
	Thomas Petazzoni, herve.codina, christophercordahi
In-Reply-To: <20240402071213.11671-1-bastien.curutchet@bootlin.com>

McBSP has free-running mode where serial clocks continue to run during
emulation halts. This mode is always enabled by the driver.

Set free-running mode when SND_SOC_DAIFMT_CONT is selected by DAI
format, unset it when SND_SOC_DAIFMT_GATED is selected.

Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
 sound/soc/ti/davinci-i2s.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/sound/soc/ti/davinci-i2s.c b/sound/soc/ti/davinci-i2s.c
index 82a0dfa07cec..7fb41987175b 100644
--- a/sound/soc/ti/davinci-i2s.c
+++ b/sound/soc/ti/davinci-i2s.c
@@ -280,6 +280,7 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 {
 	struct davinci_mcbsp_dev *dev = snd_soc_dai_get_drvdata(cpu_dai);
 	unsigned int pcr;
+	unsigned int spcr;
 	unsigned int srgr;
 	bool inv_fs = false;
 	/* Attention srgr is updated by hw_params! */
@@ -288,6 +289,23 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		DAVINCI_MCBSP_SRGR_FWID(DEFAULT_BITPERSAMPLE - 1);
 
 	dev->fmt = fmt;
+
+	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
+	switch (fmt & SND_SOC_DAIFMT_CLOCK_MASK) {
+	case SND_SOC_DAIFMT_CONT:
+		spcr |= DAVINCI_MCBSP_SPCR_FREE;
+		dev_dbg(dev->dev, "Free-running mode ON\n");
+		break;
+	case SND_SOC_DAIFMT_GATED:
+		spcr &= ~DAVINCI_MCBSP_SPCR_FREE;
+		dev_dbg(dev->dev, "Free-running mode OFF\n");
+		break;
+	default:
+		dev_err(dev->dev, "Invalid clock gating\n");
+		return -EINVAL;
+	}
+	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
+
 	/* set master/slave audio interface */
 	switch (fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) {
 	case SND_SOC_DAIFMT_BP_FP:
@@ -452,10 +470,10 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 	/* general line settings */
 	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
 	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
-		spcr |= DAVINCI_MCBSP_SPCR_RINTM(3) | DAVINCI_MCBSP_SPCR_FREE;
+		spcr |= DAVINCI_MCBSP_SPCR_RINTM(3);
 		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
 	} else {
-		spcr |= DAVINCI_MCBSP_SPCR_XINTM(3) | DAVINCI_MCBSP_SPCR_FREE;
+		spcr |= DAVINCI_MCBSP_SPCR_XINTM(3);
 		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
 	}
 
-- 
2.44.0


^ permalink raw reply related

* [PATCH v2 11/13] ASoC: ti: davinci-i2s: Add S24_LE to supported formats
From: Bastien Curutchet @ 2024-04-02  7:12 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Peter Ujfalusi, Jaroslav Kysela, Takashi Iwai,
	Bastien Curutchet
  Cc: linux-sound, devicetree, linux-kernel, alsa-devel,
	Thomas Petazzoni, herve.codina, christophercordahi
In-Reply-To: <20240402071213.11671-1-bastien.curutchet@bootlin.com>

S24_LE is supported by McBSP but not by the driver.

Add S24_LE to driver's supported formats. Using it enables the sign
extension in DRR (Data Receive Register). The other formats are kept
with the zero extension in DRR.

Remove data_type table as it is no longer used.

Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
 sound/soc/ti/davinci-i2s.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/sound/soc/ti/davinci-i2s.c b/sound/soc/ti/davinci-i2s.c
index 7fb41987175b..fb1e09c78bdf 100644
--- a/sound/soc/ti/davinci-i2s.c
+++ b/sound/soc/ti/davinci-i2s.c
@@ -61,6 +61,9 @@
 
 #define DAVINCI_MCBSP_SPCR_RRST		(1 << 0)
 #define DAVINCI_MCBSP_SPCR_RINTM(v)	((v) << 4)
+#define DAVINCI_MCBSP_SPCR_RJUST(v)	((v) << 13)
+#define DAVINCI_MCBSP_SPCR_RJUST_Z_LE	DAVINCI_MCBSP_SPCR_RJUST(0)
+#define DAVINCI_MCBSP_SPCR_RJUST_S_LE	DAVINCI_MCBSP_SPCR_RJUST(1)
 #define DAVINCI_MCBSP_SPCR_XRST		(1 << 16)
 #define DAVINCI_MCBSP_SPCR_XINTM(v)	((v) << 20)
 #define DAVINCI_MCBSP_SPCR_GRST		(1 << 22)
@@ -107,15 +110,10 @@ enum {
 	DAVINCI_MCBSP_WORD_32,
 };
 
-static const unsigned char data_type[SNDRV_PCM_FORMAT_S32_LE + 1] = {
-	[SNDRV_PCM_FORMAT_S8]		= 1,
-	[SNDRV_PCM_FORMAT_S16_LE]	= 2,
-	[SNDRV_PCM_FORMAT_S32_LE]	= 4,
-};
-
 static const unsigned char asp_word_length[SNDRV_PCM_FORMAT_S32_LE + 1] = {
 	[SNDRV_PCM_FORMAT_S8]		= DAVINCI_MCBSP_WORD_8,
 	[SNDRV_PCM_FORMAT_S16_LE]	= DAVINCI_MCBSP_WORD_16,
+	[SNDRV_PCM_FORMAT_S24_LE]	= DAVINCI_MCBSP_WORD_24,
 	[SNDRV_PCM_FORMAT_S32_LE]	= DAVINCI_MCBSP_WORD_32,
 };
 
@@ -467,8 +465,23 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 	snd_pcm_format_t fmt;
 	unsigned element_cnt = 1;
 
-	/* general line settings */
 	spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
+
+	/* Determine xfer data type */
+	fmt = params_format(params);
+	switch (fmt) {
+	case SNDRV_PCM_FORMAT_S16_LE:
+	case SNDRV_PCM_FORMAT_S32_LE:
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+		spcr |= DAVINCI_MCBSP_SPCR_RJUST_S_LE;
+		break;
+	default:
+		dev_warn(dev->dev, "davinci-i2s: unsupported PCM format\n");
+		return -EINVAL;
+	}
+
+	/* general line settings */
 	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
 		spcr |= DAVINCI_MCBSP_SPCR_RINTM(3);
 		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
@@ -566,12 +579,6 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 		rcr |= DAVINCI_MCBSP_RCR_RDATDLY(1);
 		xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1);
 	}
-	/* Determine xfer data type */
-	fmt = params_format(params);
-	if ((fmt > SNDRV_PCM_FORMAT_S32_LE) || !data_type[fmt]) {
-		printk(KERN_WARNING "davinci-i2s: unsupported PCM format\n");
-		return -EINVAL;
-	}
 
 	if (params_channels(params) == 2) {
 		element_cnt = 2;
@@ -710,6 +717,7 @@ static void davinci_i2s_shutdown(struct snd_pcm_substream *substream,
 
 #define DAVINCI_I2S_RATES	SNDRV_PCM_RATE_8000_96000
 #define DAVINCI_I2S_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE | \
+				 SNDRV_PCM_FMTBIT_S24_LE | \
 				 SNDRV_PCM_FMTBIT_S32_LE)
 
 static int davinci_i2s_dai_probe(struct snd_soc_dai *dai)
-- 
2.44.0


^ permalink raw reply related

* [PATCH v2 12/13] ASoC: dt-bindings: davinci-mcbsp: Add the 'ti,T1-framing-{rx/tx}' flags
From: Bastien Curutchet @ 2024-04-02  7:12 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Peter Ujfalusi, Jaroslav Kysela, Takashi Iwai,
	Bastien Curutchet
  Cc: linux-sound, devicetree, linux-kernel, alsa-devel,
	Thomas Petazzoni, herve.codina, christophercordahi
In-Reply-To: <20240402071213.11671-1-bastien.curutchet@bootlin.com>

McBSP's data delay can be configured from 0 to 2 bit clock periods. 0 is
used for DSP_B format, 1 for DSP_A format. A data delay of 2 bit clock
periods can be used to interface to 'T1 framing' devices where data
stream is preceded by a 'framing bit'. This 2 bit clock data delay is
not described in the bindings.

Add two flags 'ti,T1-framing-[rx/tx]' to enable a data delay of 2
bit clock periods in reception or transmission.

Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
 .../devicetree/bindings/sound/davinci-mcbsp.yaml   | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/davinci-mcbsp.yaml b/Documentation/devicetree/bindings/sound/davinci-mcbsp.yaml
index 0c2b1936c6a1..4fa677023827 100644
--- a/Documentation/devicetree/bindings/sound/davinci-mcbsp.yaml
+++ b/Documentation/devicetree/bindings/sound/davinci-mcbsp.yaml
@@ -67,6 +67,20 @@ properties:
   "#sound-dai-cells":
     const: 0
 
+  ti,T1-framing-tx:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      If the property is present, tx data delay is set to 2 bit clock periods.
+      McBSP will insert a blank period (high-impedance period) before the first
+      data bit. This can be used to interface to T1-framing devices.
+
+  ti,T1-framing-rx:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      If the property is present, rx data delay is set to 2 bit clock periods.
+      McBSP will discard the bit preceding the data stream (called framing bit).
+      This can be used to interface to T1-framing devices.
+
 required:
   - "#sound-dai-cells"
   - compatible
-- 
2.44.0


^ permalink raw reply related

* [PATCH v2 13/13] ASoC: ti: davinci-i2s: Add T1 framing support
From: Bastien Curutchet @ 2024-04-02  7:12 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Peter Ujfalusi, Jaroslav Kysela, Takashi Iwai,
	Bastien Curutchet
  Cc: linux-sound, devicetree, linux-kernel, alsa-devel,
	Thomas Petazzoni, herve.codina, christophercordahi
In-Reply-To: <20240402071213.11671-1-bastien.curutchet@bootlin.com>

McBSP's data delay can be configured from 0 to 2 bit clock periods. 0 is
used for DSP_B format, 1 is used for DSP_A format, 2 is unused.

A data delay of 2 bit clock periods can be used to interface to
'T1 framing' devices where data stream is preceded by a 'framing bit'. On
transmission, McBSP inserts a blank period (high-impedance period)
before the first data bit to leave an opportunity for other devices to
set this 'framing bit'. On reception, McBSP discards the 'framing bit'
that precedes the data stream.

Add support for the 'framing bit' according to the
'ti,T1-framing-[tx/rx]' device-tree properties. If a flag is present,
the data delay is set to 2 bit clock periods regardless of the selected
DAI format.

Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
 sound/soc/ti/davinci-i2s.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/sound/soc/ti/davinci-i2s.c b/sound/soc/ti/davinci-i2s.c
index fb1e09c78bdf..0f15a743c798 100644
--- a/sound/soc/ti/davinci-i2s.c
+++ b/sound/soc/ti/davinci-i2s.c
@@ -161,6 +161,9 @@ struct davinci_mcbsp_dev {
 
 	int tdm_slots;
 	int slot_width;
+
+	bool tx_framing_bit;
+	bool rx_framing_bit;
 };
 
 static inline void davinci_mcbsp_write_reg(struct davinci_mcbsp_dev *dev,
@@ -580,6 +583,15 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 		xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1);
 	}
 
+	if (dev->tx_framing_bit) {
+		xcr &= ~DAVINCI_MCBSP_XCR_XDATDLY(1);
+		xcr |= DAVINCI_MCBSP_XCR_XDATDLY(2);
+	}
+	if (dev->rx_framing_bit) {
+		rcr &= ~DAVINCI_MCBSP_RCR_RDATDLY(1);
+		rcr |= DAVINCI_MCBSP_RCR_RDATDLY(2);
+	}
+
 	if (params_channels(params) == 2) {
 		element_cnt = 2;
 		if (double_fmt[fmt] && dev->enable_channel_combine) {
@@ -796,6 +808,9 @@ static int davinci_i2s_probe(struct platform_device *pdev)
 
 	dev->base = io_base;
 
+	dev->tx_framing_bit = of_property_read_bool(pdev->dev.of_node, "ti,T1-framing-tx");
+	dev->rx_framing_bit = of_property_read_bool(pdev->dev.of_node, "ti,T1-framing-rx");
+
 	/* setup DMA, first TX, then RX */
 	dma_data = &dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK];
 	dma_data->addr = (dma_addr_t)(mem->start + DAVINCI_MCBSP_DXR_REG);
-- 
2.44.0


^ permalink raw reply related

* Re: [PATCH] dt-bindings: timer: renesas,tmu: Make interrupt-names required
From: Geert Uytterhoeven @ 2024-04-02  7:14 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Daniel Lezcano, Thomas Gleixner, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Laurent Pinchart, linux-renesas-soc, devicetree,
	linux-kernel
In-Reply-To: <4ef1eb4e-b1f8-4b5c-9280-5834f946fcde@linaro.org>

Hi Krzysztof,

On Fri, Mar 29, 2024 at 6:42 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 29/03/2024 13:37, Geert Uytterhoeven wrote:
> > Now all in-tree users have been updated with interrupt-names properties
> > according to commit 0076a37a426b6c85 ("dt-bindings: timer: renesas,tmu:
> > Document input capture interrupt"), make interrupt-names required.
>
> Would be nice to see here *why* they should be required, e.g. "Linux
> driver needs them since commit foobar").

The driver doesn't use the names, nor the optional input capture
interrupt yet.

This is just part of the continuous improvement of Very Old and Immature
DT Bindings, dating back to the days when most animals could still talk,
and before DT became self-aware.

I could add that (or something simpler ;-) for v2, but as the reason
is basically the "for clarity" in the linked commit....

> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> >  Documentation/devicetree/bindings/timer/renesas,tmu.yaml | 1 +
> >  1 file changed, 1 insertion(+)
>
> Anyway:
>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v3 1/4] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel
From: Dmitry Baryshkov @ 2024-04-02  7:23 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Sumit Semwal, Caleb Connolly, Neil Armstrong, Jessica Zhang,
	Sam Ravnborg, David Airlie, Daniel Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, dri-devel, devicetree,
	linux-kernel, linux-arm-msm, Vinod Koul, Caleb Connolly
In-Reply-To: <9fbb9058-ccfe-436d-b413-b3ba27e4e5f9@linaro.org>

On Tue, 2 Apr 2024 at 09:31, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 02/04/2024 01:51, Dmitry Baryshkov wrote:
> > From: Sumit Semwal <sumit.semwal@linaro.org>
> >
> > LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel present on Google Pixel 3
> > phones.
> >
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> > [caleb: convert to yaml]
> > Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
>
> Tags missing.
>
> `b4 trailers -u`

Excuse me, I keep on forgetting it.

-- 
With best wishes
Dmitry

^ permalink raw reply

* Re: [PATCH AUTOSEL 6.8 36/98] arm64: dts: sc8280xp: correct DMIC2 and DMIC3 pin config node names
From: Johan Hovold @ 2024-04-02  7:23 UTC (permalink / raw)
  To: Sasha Levin
  Cc: linux-kernel, stable, Krzysztof Kozlowski, Bjorn Andersson,
	konrad.dybcio, robh, krzysztof.kozlowski+dt, conor+dt,
	linux-arm-msm, devicetree
In-Reply-To: <20240329123919.3087149-36-sashal@kernel.org>

On Fri, Mar 29, 2024 at 08:37:07AM -0400, Sasha Levin wrote:
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> [ Upstream commit 61474b18e762671a69b2df9665f3cec5c87a38af ]
> 
> Correct the TLMM pin configuration and muxing node names used for DMIC2
> and DMIC3 (dmic01 -> dmic23).  This has no functional impact, but
> improves code readability and avoids any confusion when reading the DTS.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Link: https://lore.kernel.org/r/20240212172335.124845-1-krzysztof.kozlowski@linaro.org
> Signed-off-by: Bjorn Andersson <andersson@kernel.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>

This is not a bug fix. Please drop from all stable queues (e.g. 6.8 and
6.6).

Johan

^ permalink raw reply

* Re: [PATCH v3] dt-bindings: ata: ahci-da850: Convert to dtschema
From: Damien Le Moal @ 2024-04-02  7:25 UTC (permalink / raw)
  To: Animesh Agarwal
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-ide,
	devicetree, linux-kernel
In-Reply-To: <20240327064354.17384-1-animeshagarwal28@gmail.com>

On 3/27/24 15:43, Animesh Agarwal wrote:
> Convert the ahci-da850 bindings to DT schema.
> 
> Signed-off-by: Animesh Agarwal <animeshagarwal28@gmail.com>

Applied to for-6.10. Thanks !

-- 
Damien Le Moal
Western Digital Research


^ permalink raw reply

* Re: [PATCH v2 17/18] dt-bindings: pci: rockchip,rk3399-pcie-ep: Add ep-gpios property
From: Krzysztof Kozlowski @ 2024-04-02  7:33 UTC (permalink / raw)
  To: Damien Le Moal, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Kishon Vijay Abraham I, Shawn Lin, Krzysztof Wilczyński,
	Bjorn Helgaas, Heiko Stuebner, linux-pci, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree
  Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
	Wilfred Mallawa, Niklas Cassel
In-Reply-To: <49ecab2e-8f36-47be-a1b0-1bb0089dab0f@kernel.org>

On 02/04/2024 01:36, Damien Le Moal wrote:
> On 4/1/24 18:57, Krzysztof Kozlowski wrote:
>> On 01/04/2024 01:06, Damien Le Moal wrote:
>>> On 3/30/24 18:16, Krzysztof Kozlowski wrote:
>>>> On 30/03/2024 05:19, Damien Le Moal wrote:
>>>>> From: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>>>>
>>>>> Describe the `ep-gpios` property which is used to map the PERST# input
>>>>> signal for endpoint mode.
>>>>>
>>>>> Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>>>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
>>>>> ---
>>>>>  .../devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml       | 3 +++
>>>>>  1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>>>> index 6b62f6f58efe..9331d44d6963 100644
>>>>> --- a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>>>> +++ b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>>>> @@ -30,6 +30,9 @@ properties:
>>>>>      maximum: 32
>>>>>      default: 32
>>>>>  
>>>>> +  ep-gpios:
>>>>> +    description: Input GPIO configured for the PERST# signal.
>>>>
>>>> Missing maxItems. But more important: why existing property perst-gpios,
>>>> which you already have there in common schema, is not correct for this case?
>>>
>>> I am confused... Where do you find perst-gpios defined for the rk3399 ?
>>> Under Documentation/devicetree/bindings/pci/, the only schema I see using
>>> perst-gpios property are for the qcom (Qualcomm) controllers.
>>
>> You are right, it's so far only in Qualcomm.
>>
>>> The RC bindings for the rockchip rk3399 PCIe controller
>>> (pci/rockchip,rk3399-pcie.yaml) already define the ep-gpios property. So if
>>
>> Any reason why this cannot be named like GPIO? Is there already a user
>> of this in Linux kernel? Commit msg says nothing about this, so that's
>> why I would expect name matching the signal.
> 
> The RC-mode PCIe controller node of the rk3399 DTS already defines the ep-gpios
> property for RC side PERST# signal handling. So we simply reused the exact same
> name to be consistent between RC and EP. I personnally have no preferences. If
> there is an effort to rename such signal with some preferred pattern, I will
> follow. For the EP node, there was no PERST signal handling in the driver and
> no property defined for it, so any name is fine. "perst-gpios" would indeed be
> a better name, but again, given that the RC controller node has ep-gpios, we
> reused that. What is your recommendation here ?

Actually I don't know, perst and ep would work for me. If you do not
have code for this in the driver yet (nothing is shared between ep and
host), then maybe let's go with perst to match the actual name.

Anyway, you need maxItems. I sent a patch for the other binding:
https://lore.kernel.org/all/20240401100058.15749-1-krzysztof.kozlowski@linaro.org/

Best regards,
Krzysztof


^ permalink raw reply

* [PATCH] arch: arm: mxc: phyCARD-i.MX27: Add USB support
From: Michael Grzeschik @ 2024-04-02  7:35 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
  Cc: devicetree, imx, linux-arm-kernel, linux-kernel,
	Michael Grzeschik

This patch adds the pinmux and nodes for usbotg and usbh2.

In v6 revision of the pca100 the usb phys were changed to usb3320 which
are connected by their reset pins. We add the phy configuration to the
description.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 .../dts/nxp/imx/imx27-phytec-phycard-s-som.dtsi    | 78 ++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/arch/arm/boot/dts/nxp/imx/imx27-phytec-phycard-s-som.dtsi b/arch/arm/boot/dts/nxp/imx/imx27-phytec-phycard-s-som.dtsi
index abc9233c5a1b1..31b3fc972abbf 100644
--- a/arch/arm/boot/dts/nxp/imx/imx27-phytec-phycard-s-som.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/imx27-phytec-phycard-s-som.dtsi
@@ -15,6 +15,22 @@ memory@a0000000 {
 		device_type = "memory";
 		reg = <0xa0000000 0x08000000>; /* 128MB */
 	};
+
+	usbotgphy: usbotgphy {
+		compatible = "usb-nop-xceiv";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_usbotgphy>;
+		reset-gpios = <&gpio2 25 GPIO_ACTIVE_LOW>;
+		#phy-cells = <0>;
+	};
+
+	usbh2phy: usbh2phy {
+		compatible = "usb-nop-xceiv";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_usbh2phy>;
+		reset-gpios = <&gpio2 22 GPIO_ACTIVE_LOW>;
+		#phy-cells = <0>;
+	};
 };
 
 &cspi1 {
@@ -84,6 +100,52 @@ MX27_PAD_NFRE_B__NFRE_B 0x0
 				MX27_PAD_NFWE_B__NFWE_B 0x0
 			>;
 		};
+
+		pinctrl_usbotgphy: usbotgphygrp {
+			fsl,pins = <
+				MX27_PAD_USBH1_RCV__GPIO2_25		0x1 /* reset gpio */
+			>;
+		};
+
+		pinctrl_usbotg: usbotggrp {
+			fsl,pins = <
+				MX27_PAD_USBOTG_CLK__USBOTG_CLK		0x0
+				MX27_PAD_USBOTG_DIR__USBOTG_DIR		0x0
+				MX27_PAD_USBOTG_NXT__USBOTG_NXT		0x0
+				MX27_PAD_USBOTG_STP__USBOTG_STP		0x0
+				MX27_PAD_USBOTG_DATA0__USBOTG_DATA0	0x0
+				MX27_PAD_USBOTG_DATA1__USBOTG_DATA1	0x0
+				MX27_PAD_USBOTG_DATA2__USBOTG_DATA2	0x0
+				MX27_PAD_USBOTG_DATA3__USBOTG_DATA3	0x0
+				MX27_PAD_USBOTG_DATA4__USBOTG_DATA4	0x0
+				MX27_PAD_USBOTG_DATA5__USBOTG_DATA5	0x0
+				MX27_PAD_USBOTG_DATA6__USBOTG_DATA6	0x0
+				MX27_PAD_USBOTG_DATA7__USBOTG_DATA7	0x0
+			>;
+		};
+
+		pinctrl_usbh2phy: usbh2phygrp {
+			fsl,pins = <
+				MX27_PAD_USBH1_SUSP__GPIO2_22		0x0 /* reset gpio */
+			>;
+		};
+
+		pinctrl_usbh2: usbh2grp {
+			fsl,pins = <
+				MX27_PAD_USBH2_CLK__USBH2_CLK		0x0
+				MX27_PAD_USBH2_DIR__USBH2_DIR		0x0
+				MX27_PAD_USBH2_NXT__USBH2_NXT		0x0
+				MX27_PAD_USBH2_STP__USBH2_STP		0x0
+				MX27_PAD_CSPI2_SCLK__USBH2_DATA0	0x0
+				MX27_PAD_CSPI2_MOSI__USBH2_DATA1	0x0
+				MX27_PAD_CSPI2_MISO__USBH2_DATA2	0x0
+				MX27_PAD_CSPI2_SS1__USBH2_DATA3		0x0
+				MX27_PAD_CSPI2_SS2__USBH2_DATA4		0x0
+				MX27_PAD_CSPI1_SS2__USBH2_DATA5		0x0
+				MX27_PAD_CSPI2_SS0__USBH2_DATA6		0x0
+				MX27_PAD_USBH2_DATA7__USBH2_DATA7	0x0
+			>;
+		};
 	};
 };
 
@@ -95,3 +157,19 @@ &nfc {
 	nand-on-flash-bbt;
 	status = "okay";
 };
+
+&usbotg {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_usbotg>;
+	phy_type = "ulpi";
+	phys = <&usbotgphy>;
+	status = "okay";
+};
+
+&usbh2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_usbh2>;
+	phy_type = "ulpi";
+	phys = <&usbh2phy>;
+	status = "okay";
+};

---
base-commit: 5bab5dc780c9ed0c69fc2f828015532acf4a7848
change-id: 20240328-pca100-a600ac4384e7

Best regards,
-- 
Michael Grzeschik <m.grzeschik@pengutronix.de>


^ permalink raw reply related

* Re: [PATCH v3] dt-bindings: ata: ahci-da850: Convert to dtschema
From: Animesh Agarwal @ 2024-04-02  7:37 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-ide,
	devicetree, linux-kernel
In-Reply-To: <24c917f6-9ff8-4bca-8cc2-5c14c64c2c9b@kernel.org>

On Tue, Apr 2, 2024 at 12:55 PM Damien Le Moal <dlemoal@kernel.org> wrote:
> Applied to for-6.10. Thanks !

Thanks for your time Damien.

---
Animesh Agarwal

^ permalink raw reply

* Re: [PATCH v2 17/18] dt-bindings: pci: rockchip,rk3399-pcie-ep: Add ep-gpios property
From: Damien Le Moal @ 2024-04-02  7:38 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Kishon Vijay Abraham I, Shawn Lin, Krzysztof Wilczyński,
	Bjorn Helgaas, Heiko Stuebner, linux-pci, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree
  Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
	Wilfred Mallawa, Niklas Cassel
In-Reply-To: <57d5d6ea-5fef-423c-9f85-5f295bfa4c5f@linaro.org>

On 4/2/24 16:33, Krzysztof Kozlowski wrote:
> On 02/04/2024 01:36, Damien Le Moal wrote:
>> On 4/1/24 18:57, Krzysztof Kozlowski wrote:
>>> On 01/04/2024 01:06, Damien Le Moal wrote:
>>>> On 3/30/24 18:16, Krzysztof Kozlowski wrote:
>>>>> On 30/03/2024 05:19, Damien Le Moal wrote:
>>>>>> From: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>>>>>
>>>>>> Describe the `ep-gpios` property which is used to map the PERST# input
>>>>>> signal for endpoint mode.
>>>>>>
>>>>>> Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>>>>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
>>>>>> ---
>>>>>>  .../devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml       | 3 +++
>>>>>>  1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>>>>> index 6b62f6f58efe..9331d44d6963 100644
>>>>>> --- a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>>>>> @@ -30,6 +30,9 @@ properties:
>>>>>>      maximum: 32
>>>>>>      default: 32
>>>>>>  
>>>>>> +  ep-gpios:
>>>>>> +    description: Input GPIO configured for the PERST# signal.
>>>>>
>>>>> Missing maxItems. But more important: why existing property perst-gpios,
>>>>> which you already have there in common schema, is not correct for this case?
>>>>
>>>> I am confused... Where do you find perst-gpios defined for the rk3399 ?
>>>> Under Documentation/devicetree/bindings/pci/, the only schema I see using
>>>> perst-gpios property are for the qcom (Qualcomm) controllers.
>>>
>>> You are right, it's so far only in Qualcomm.
>>>
>>>> The RC bindings for the rockchip rk3399 PCIe controller
>>>> (pci/rockchip,rk3399-pcie.yaml) already define the ep-gpios property. So if
>>>
>>> Any reason why this cannot be named like GPIO? Is there already a user
>>> of this in Linux kernel? Commit msg says nothing about this, so that's
>>> why I would expect name matching the signal.
>>
>> The RC-mode PCIe controller node of the rk3399 DTS already defines the ep-gpios
>> property for RC side PERST# signal handling. So we simply reused the exact same
>> name to be consistent between RC and EP. I personnally have no preferences. If
>> there is an effort to rename such signal with some preferred pattern, I will
>> follow. For the EP node, there was no PERST signal handling in the driver and
>> no property defined for it, so any name is fine. "perst-gpios" would indeed be
>> a better name, but again, given that the RC controller node has ep-gpios, we
>> reused that. What is your recommendation here ?
> 
> Actually I don't know, perst and ep would work for me. If you do not
> have code for this in the driver yet (nothing is shared between ep and
> host), then maybe let's go with perst to match the actual name.

That works for me. The other simple solution would be to move the RC node
ep-gpios description to the common schema pci/rockchip,rk3399-pcie-common.yaml,
maybe ? Otherwise, perst-gpios like the Qualcomm schemas would be nice too.

> 
> Anyway, you need maxItems. I sent a patch for the other binding:
> https://lore.kernel.org/all/20240401100058.15749-1-krzysztof.kozlowski@linaro.org/

Thanks for that.

> 
> Best regards,
> Krzysztof
> 

-- 
Damien Le Moal
Western Digital Research


^ permalink raw reply

* Re: [PATCH v2 17/18] dt-bindings: pci: rockchip,rk3399-pcie-ep: Add ep-gpios property
From: Damien Le Moal @ 2024-04-02  7:38 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Kishon Vijay Abraham I, Shawn Lin, Krzysztof Wilczyński,
	Bjorn Helgaas, Heiko Stuebner, linux-pci, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree
  Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
	Wilfred Mallawa, Niklas Cassel
In-Reply-To: <57d5d6ea-5fef-423c-9f85-5f295bfa4c5f@linaro.org>

On 4/2/24 16:33, Krzysztof Kozlowski wrote:
> On 02/04/2024 01:36, Damien Le Moal wrote:
>> On 4/1/24 18:57, Krzysztof Kozlowski wrote:
>>> On 01/04/2024 01:06, Damien Le Moal wrote:
>>>> On 3/30/24 18:16, Krzysztof Kozlowski wrote:
>>>>> On 30/03/2024 05:19, Damien Le Moal wrote:
>>>>>> From: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>>>>>
>>>>>> Describe the `ep-gpios` property which is used to map the PERST# input
>>>>>> signal for endpoint mode.
>>>>>>
>>>>>> Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>>>>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
>>>>>> ---
>>>>>>  .../devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml       | 3 +++
>>>>>>  1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>>>>> index 6b62f6f58efe..9331d44d6963 100644
>>>>>> --- a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>>>>> @@ -30,6 +30,9 @@ properties:
>>>>>>      maximum: 32
>>>>>>      default: 32
>>>>>>  
>>>>>> +  ep-gpios:
>>>>>> +    description: Input GPIO configured for the PERST# signal.
>>>>>
>>>>> Missing maxItems. But more important: why existing property perst-gpios,
>>>>> which you already have there in common schema, is not correct for this case?
>>>>
>>>> I am confused... Where do you find perst-gpios defined for the rk3399 ?
>>>> Under Documentation/devicetree/bindings/pci/, the only schema I see using
>>>> perst-gpios property are for the qcom (Qualcomm) controllers.
>>>
>>> You are right, it's so far only in Qualcomm.
>>>
>>>> The RC bindings for the rockchip rk3399 PCIe controller
>>>> (pci/rockchip,rk3399-pcie.yaml) already define the ep-gpios property. So if
>>>
>>> Any reason why this cannot be named like GPIO? Is there already a user
>>> of this in Linux kernel? Commit msg says nothing about this, so that's
>>> why I would expect name matching the signal.
>>
>> The RC-mode PCIe controller node of the rk3399 DTS already defines the ep-gpios
>> property for RC side PERST# signal handling. So we simply reused the exact same
>> name to be consistent between RC and EP. I personnally have no preferences. If
>> there is an effort to rename such signal with some preferred pattern, I will
>> follow. For the EP node, there was no PERST signal handling in the driver and
>> no property defined for it, so any name is fine. "perst-gpios" would indeed be
>> a better name, but again, given that the RC controller node has ep-gpios, we
>> reused that. What is your recommendation here ?
> 
> Actually I don't know, perst and ep would work for me. If you do not
> have code for this in the driver yet (nothing is shared between ep and
> host), then maybe let's go with perst to match the actual name.

Forgot to add: the driver code for the EP PERST gpio handling is added in patch
18 of the series, after this one.

> 
> Anyway, you need maxItems. I sent a patch for the other binding:
> https://lore.kernel.org/all/20240401100058.15749-1-krzysztof.kozlowski@linaro.org/
> 
> Best regards,
> Krzysztof
> 

-- 
Damien Le Moal
Western Digital Research


^ permalink raw reply

* Re: [PATCH v6 3/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
From: Cristian Marussi @ 2024-04-02  7:48 UTC (permalink / raw)
  To: Peng Fan
  Cc: Andy Shevchenko, Peng Fan (OSS), Sudeep Holla, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Linus Walleij, Dan Carpenter,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-gpio@vger.kernel.org, Oleksii Moisieiev
In-Reply-To: <DU0PR04MB9417E797F4E0F7BB6154B3BE88382@DU0PR04MB9417.eurprd04.prod.outlook.com>

On Sun, Mar 31, 2024 at 01:44:28PM +0000, Peng Fan wrote:
> Hi Andy,
> 

Hi Peng,


> > Subject: Re: [PATCH v6 3/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol
> > protocol basic support
> > 
> > Sat, Mar 23, 2024 at 08:15:16PM +0800, Peng Fan (OSS) kirjoitti:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > Add basic implementation of the SCMI v3.2 pincontrol protocol.
> > 
> > ...
> > 
> > >  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
> > >  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
> > > scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o
> > > system.o voltage.o powercap.o
> > 
> > Actually you want to have := here.
> > 
> > > +scmi-protocols-y += pinctrl.o
> > 
> > 
> > 
> > >  scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y)
> > > $(scmi-transport-y)
> > 
> > Side note: The -objs has to be -y
> > 
> > ...
> > 
> > > +#include <linux/module.h>
> > > +#include <linux/scmi_protocol.h>
> > > +#include <linux/slab.h>
> > 
> > This is semi-random list of headers. Please, follow IWYU principle (include
> > what you use). There are a lot of inclusions I see missing (just in the context of
> > this page I see bits.h, types.h, and  asm/byteorder.h).
> 
> Is there any documentation about this requirement?
> Some headers are already included by others.
> 

Andy made (mostly) the same remarks on this same patch ~1-year ago on
this same patch while it was posted by Oleksii.

And I told that time that most of the remarks around devm_ usage were
wrong due to how the SCMI core handles protocol initialization (using a
devres group transparently).

This is what I answered that time.

https://lore.kernel.org/linux-arm-kernel/ZJ78hBcjAhiU+ZBO@e120937-lin/#t

I wont repeat myself, but, in a nutshell the memory allocation like it
is now is fine: a bit happens via devm_ at protocol initialization, the
other is doe via explicit kmalloc at runtime and freed via kfree at
remove time (if needed...i.e. checking the present flag of some structs)

I'll made further remarks on v7 that you just posted.

Thanks,
Cristian


^ permalink raw reply

* Re: [PATCH 07/10] dt-bindings: iio: dac: add bindings doc for AD9739A
From: Nuno Sá @ 2024-04-02  7:49 UTC (permalink / raw)
  To: Krzysztof Kozlowski, David Lechner, nuno.sa
  Cc: linux-iio, devicetree, Dragos Bogdan, Jonathan Cameron,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Olivier Moysan
In-Reply-To: <3d876e90-4821-4c00-b880-6966b53c70a7@linaro.org>

On Sat, 2024-03-30 at 19:27 +0100, Krzysztof Kozlowski wrote:
> On 29/03/2024 20:06, David Lechner wrote:
> > 
> > > +
> > > +  reset-gpios:
> > > +    maxItems: 1
> > > +
> > > +  vdd_3_3-supply:
> > > +    description: 3.3V Digital input supply.
> > 
> > vdd33-supply would better match the datasheet name
> 
> And would conform to coding style. No underscores in names.
> 
> Nuno,
> To avoid comments like this one, please read and follow DTS coding style
> when you work with DTS or Devicetree bindings.
> 

Noted...

Thanks!
- Nuno Sá

> 


^ permalink raw reply

* Re: [PATCH 07/10] dt-bindings: iio: dac: add bindings doc for AD9739A
From: Nuno Sá @ 2024-04-02  7:50 UTC (permalink / raw)
  To: David Lechner, nuno.sa
  Cc: linux-iio, devicetree, Dragos Bogdan, Jonathan Cameron,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Olivier Moysan
In-Reply-To: <CAMknhBHA3LOhZicUCG32RyiV4+OROJymzm0Qc=wqhfo3u=8vnQ@mail.gmail.com>

On Fri, 2024-03-29 at 14:06 -0500, David Lechner wrote:
> On Thu, Mar 28, 2024 at 8:23 AM Nuno Sa via B4 Relay
> <devnull+nuno.sa.analog.com@kernel.org> wrote:
> > 
> > From: Nuno Sa <nuno.sa@analog.com>
> > 
> > This adds the bindings documentation for the 14 bit
> > RF Digital-to-Analog converter.
> > 
> > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > ---
> >  .../devicetree/bindings/iio/dac/adi,ad9739a.yaml   | 88 ++++++++++++++++++++++
> >  MAINTAINERS                                        |  8 ++
> >  2 files changed, 96 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad9739a.yaml
> > b/Documentation/devicetree/bindings/iio/dac/adi,ad9739a.yaml
> > new file mode 100644
> > index 000000000000..24bcec763a9b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad9739a.yaml
> > @@ -0,0 +1,88 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/dac/adi,ad9739a.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices AD9739A RF DAC
> > +
> > +maintainers:
> > +  - Dragos Bogdan <dragos.bogdan@analog.com>
> > +  - Nuno Sa <nuno.sa@analog.com>
> > +
> > +description: |
> > +  The AD9739A is a 14-bit, 2.5 GSPS high performance RF DACs that are capable
> > +  of synthesizing wideband signals from dc up to 3 GHz.
> > +
> > + 
> > https://www.analog.com/media/en/technical-documentation/data-sheets/ad9737a_9739a.pdf
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,ad9739a
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> 
> I also see an IRQ pin on the datasheet. Do we need an interrupts
> property for that?
> 

Likely we'll never use it. But yes, it can be documented in the bindings...

> > +
> > +  reset-gpios:
> > +    maxItems: 1
> > +
> > +  vdd_3_3-supply:
> > +    description: 3.3V Digital input supply.
> 
> vdd33-supply would better match the datasheet name
> 
> > +
> > +  vdd-supply:
> > +    description: 1.8V Digital input supply.
> > +
> > +  vdda-supply:
> > +    description: 3.3V Analog input supply.
> > +
> > +  vddc-supply:
> > +    description: 1.8V Clock input supply.
> 
> I see a VREF voltage reference input in the datasheet. Do we need a vref-supply?

Same as the IRQ pin.

- Nuno Sá


^ permalink raw reply

* Re: [PATCH 06/10] dt-bindings: iio: dac: add bindings doc for AXI DAC driver
From: Nuno Sá @ 2024-04-02  7:51 UTC (permalink / raw)
  To: David Lechner, nuno.sa
  Cc: linux-iio, devicetree, Dragos Bogdan, Jonathan Cameron,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Olivier Moysan
In-Reply-To: <CAMknhBHKcy9KO=8vuAvF0twY4x14sFq3FLW4ygORMn2G5k7WSg@mail.gmail.com>

On Fri, 2024-03-29 at 13:46 -0500, David Lechner wrote:
> On Thu, Mar 28, 2024 at 8:22 AM Nuno Sa via B4 Relay
> <devnull+nuno.sa.analog.com@kernel.org> wrote:
> > 
> > From: Nuno Sa <nuno.sa@analog.com>
> > 
> > This adds the bindings documentation for the AXI DAC driver.
> > 
> > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > ---
> >  .../devicetree/bindings/iio/dac/adi,axi-dac.yaml   | 62 ++++++++++++++++++++++
> >  MAINTAINERS                                        |  7 +++
> >  2 files changed, 69 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml
> > b/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml
> > new file mode 100644
> > index 000000000000..1018fd274f04
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/dac/adi,axi-dac.yaml
> > @@ -0,0 +1,62 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/dac/adi,axi-dac.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices AXI DAC IP core
> > +
> > +maintainers:
> > +  - Nuno Sa <nuno.sa@analog.com>
> > +
> > +description: |
> > +  Analog Devices Generic AXI DAC IP core for interfacing a DAC device
> > +  with a high speed serial (JESD204B/C) or source synchronous parallel
> > +  interface (LVDS/CMOS).
> > +  Usually, some other interface type (i.e SPI) is used as a control
> > +  interface for the actual DAC, while this IP core will interface
> > +  to the data-lines of the DAC and handle the streaming of data into
> > +  memory via DMA.
> 
> Isn't it the other way around for DAC, from memory to hardware?

Good catch! Yes, copy paste from the axi-adc and obviously missed this :)

- Nuno Sá

^ permalink raw reply

* Re: [PATCH v3 6/7] PCI: dwc: rcar-gen4: Add support for r8a779g0
From: Geert Uytterhoeven @ 2024-04-02  7:53 UTC (permalink / raw)
  To: Yoshihiro Shimoda
  Cc: lpieralisi, kw, robh, bhelgaas, krzysztof.kozlowski+dt, conor+dt,
	jingoohan1, mani, marek.vasut+renesas, linux-pci, devicetree,
	linux-renesas-soc
In-Reply-To: <20240401023942.134704-7-yoshihiro.shimoda.uh@renesas.com>

Hi Shimoda-san,

On Mon, Apr 1, 2024 at 4:40 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> This driver previously supported r8a779f0 (R-Car S4-8). Add support
> for r8a779g0 (R-Car V4H).
>
> To support r8a779g0, it requires specific firmware.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Thanks for your patch!

> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c

> +static int rcar_gen4_pcie_update_phy_firmware(struct rcar_gen4_pcie *rcar)
> +{
> +       const u32 check_addr[] = { 0x00101018, 0x00101118, 0x00101021, 0x00101121};
> +       struct dw_pcie *dw = &rcar->dw;
> +       const struct firmware *fw;
> +       unsigned int i, timeout;
> +       u32 data;
> +       int ret;
> +
> +       ret = request_firmware(&fw, RCAR_GEN4_PCIE_FIRMWARE_NAME, dw->dev);
> +       if (ret) {
> +               dev_err(dw->dev, "%s: Requesting firmware failed\n", __func__);
> +               return ret;
> +       }
> +
> +       for (i = 0; i < (fw->size / 2); i++) {
> +               data = fw->data[i * 2] | fw->data[(i * 2) + 1] << 8;
> +               timeout = 100;
> +retry_data:
> +               dw_pcie_writel_dbi(dw, PRTLGC89, RCAR_GEN4_PCIE_FIRMWARE_BASE_ADDR + i);
> +               dw_pcie_writel_dbi(dw, PRTLGC90, data);
> +               if (rcar_gen4_pcie_reg_check_bit(rcar, PRTLGC89, BIT(30)) < 0) {

If you would invert the logic here, you could "break" here, ...

> +                       if (!(--timeout)) {
> +                               ret = -ETIMEDOUT;
> +                               goto exit;
> +                       }
> +                       usleep_range(100, 200);
> +                       goto retry_data;

... and convert "retry_data: ... goto retry_data" into "do { ... } while (1)",
avoiding the goto.

> +               }
> +       }
> +
> +       rcar_gen4_pcie_phy_reg_update_bits(rcar, RCAR_GEN4_PCIE_PHY_0f8, BIT(17), BIT(17));
> +
> +       for (i = 0; i < ARRAY_SIZE(check_addr); i++) {
> +               timeout = 100;
> +retry_check:
> +               dw_pcie_writel_dbi(dw, PRTLGC89, check_addr[i]);
> +               ret = rcar_gen4_pcie_reg_check_bit(rcar, PRTLGC89, BIT(30));
> +               ret |= rcar_gen4_pcie_reg_check_bit(rcar, PRTLGC90, BIT(0));
> +               if (ret < 0) {
> +                       if (!(--timeout)) {
> +                               ret = -ETIMEDOUT;
> +                               goto exit;
> +                       }
> +                       usleep_range(100, 200);
> +                       goto retry_check;

Likewise.

> +               }
> +       }
> +
> +       ret = 0;
> +exit:
> +       release_firmware(fw);
> +
> +       return ret;
> +}

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v2 17/18] dt-bindings: pci: rockchip,rk3399-pcie-ep: Add ep-gpios property
From: Damien Le Moal @ 2024-04-02  7:55 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Kishon Vijay Abraham I, Shawn Lin, Krzysztof Wilczyński,
	Bjorn Helgaas, Heiko Stuebner, linux-pci, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree
  Cc: linux-rockchip, linux-arm-kernel, Rick Wertenbroek,
	Wilfred Mallawa, Niklas Cassel
In-Reply-To: <80c4c37b-8c5c-4628-a455-fcccfc3b3730@kernel.org>

On 4/2/24 16:38, Damien Le Moal wrote:
> On 4/2/24 16:33, Krzysztof Kozlowski wrote:
>> On 02/04/2024 01:36, Damien Le Moal wrote:
>>> On 4/1/24 18:57, Krzysztof Kozlowski wrote:
>>>> On 01/04/2024 01:06, Damien Le Moal wrote:
>>>>> On 3/30/24 18:16, Krzysztof Kozlowski wrote:
>>>>>> On 30/03/2024 05:19, Damien Le Moal wrote:
>>>>>>> From: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>>>>>>
>>>>>>> Describe the `ep-gpios` property which is used to map the PERST# input
>>>>>>> signal for endpoint mode.
>>>>>>>
>>>>>>> Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>>>>>>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
>>>>>>> ---
>>>>>>>  .../devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml       | 3 +++
>>>>>>>  1 file changed, 3 insertions(+)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>>>>>> index 6b62f6f58efe..9331d44d6963 100644
>>>>>>> --- a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>>>>>> +++ b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>>>>>> @@ -30,6 +30,9 @@ properties:
>>>>>>>      maximum: 32
>>>>>>>      default: 32
>>>>>>>  
>>>>>>> +  ep-gpios:
>>>>>>> +    description: Input GPIO configured for the PERST# signal.
>>>>>>
>>>>>> Missing maxItems. But more important: why existing property perst-gpios,
>>>>>> which you already have there in common schema, is not correct for this case?
>>>>>
>>>>> I am confused... Where do you find perst-gpios defined for the rk3399 ?
>>>>> Under Documentation/devicetree/bindings/pci/, the only schema I see using
>>>>> perst-gpios property are for the qcom (Qualcomm) controllers.
>>>>
>>>> You are right, it's so far only in Qualcomm.
>>>>
>>>>> The RC bindings for the rockchip rk3399 PCIe controller
>>>>> (pci/rockchip,rk3399-pcie.yaml) already define the ep-gpios property. So if
>>>>
>>>> Any reason why this cannot be named like GPIO? Is there already a user
>>>> of this in Linux kernel? Commit msg says nothing about this, so that's
>>>> why I would expect name matching the signal.
>>>
>>> The RC-mode PCIe controller node of the rk3399 DTS already defines the ep-gpios
>>> property for RC side PERST# signal handling. So we simply reused the exact same
>>> name to be consistent between RC and EP. I personnally have no preferences. If
>>> there is an effort to rename such signal with some preferred pattern, I will
>>> follow. For the EP node, there was no PERST signal handling in the driver and
>>> no property defined for it, so any name is fine. "perst-gpios" would indeed be
>>> a better name, but again, given that the RC controller node has ep-gpios, we
>>> reused that. What is your recommendation here ?
>>
>> Actually I don't know, perst and ep would work for me. If you do not
>> have code for this in the driver yet (nothing is shared between ep and
>> host), then maybe let's go with perst to match the actual name.
> 
> That works for me. The other simple solution would be to move the RC node
> ep-gpios description to the common schema pci/rockchip,rk3399-pcie-common.yaml,
> maybe ? Otherwise, perst-gpios like the Qualcomm schemas would be nice too.

Thinking more about this, I think moving the ep-gpios description to the common
schema is the right thing to do given that the driver uses common code between
RC and EP to get that property. But if that is not acceptable, I can rename it
and get that property in the controller EP mode initialization code. That will
be add a little more code in the driver.

> 
>>
>> Anyway, you need maxItems. I sent a patch for the other binding:
>> https://lore.kernel.org/all/20240401100058.15749-1-krzysztof.kozlowski@linaro.org/
> 
> Thanks for that.
> 
>>
>> Best regards,
>> Krzysztof
>>
> 

-- 
Damien Le Moal
Western Digital Research


^ permalink raw reply

* [PATCH v1 0/5] pwm: Introduce pwm driver for the Loongson family chips
From: Binbin Zhou @ 2024-04-02  7:58 UTC (permalink / raw)
  To: Binbin Zhou, Huacai Chen, Uwe Kleine-König, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: Huacai Chen, loongson-kernel, linux-pwm, devicetree, Xuerui Wang,
	loongarch, Binbin Zhou

Hi all:

This patchset introduce a generic PWM framework driver for Loongson family.
Each PWM has one pulse width output signal and one pulse input signal to be measured.
It can be found on Loongson-2K series cpus and Loongson LS7A bridge chips.

Thanks.

Binbin Zhou (5):
  dt-bindings: pwm: Add Loongson PWM controller
  pwm: Add Loongson PWM controller support
  LoongArch: dts: Add PWM support to Loongson-2K0500
  LoongArch: dts: Add PWM support to Loongson-2K1000
  LoongArch: dts: Add PWM support to Loongson-2K2000

 .../devicetree/bindings/pwm/pwm-loongson.yaml |  64 ++++
 MAINTAINERS                                   |   7 +
 arch/loongarch/boot/dts/loongson-2k0500.dtsi  |  20 ++
 .../boot/dts/loongson-2k1000-ref.dts          |  24 ++
 arch/loongarch/boot/dts/loongson-2k1000.dtsi  |  42 ++-
 arch/loongarch/boot/dts/loongson-2k2000.dtsi  |  60 ++++
 drivers/pwm/Kconfig                           |  10 +
 drivers/pwm/Makefile                          |   1 +
 drivers/pwm/pwm-loongson.c                    | 300 ++++++++++++++++++
 9 files changed, 527 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
 create mode 100644 drivers/pwm/pwm-loongson.c

-- 
2.43.0


^ permalink raw reply

* [PATCH v1 1/5] dt-bindings: pwm: Add Loongson PWM controller
From: Binbin Zhou @ 2024-04-02  7:58 UTC (permalink / raw)
  To: Binbin Zhou, Huacai Chen, Uwe Kleine-König, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: Huacai Chen, loongson-kernel, linux-pwm, devicetree, Xuerui Wang,
	loongarch, Binbin Zhou
In-Reply-To: <cover.1711953223.git.zhoubinbin@loongson.cn>

Add Loongson PWM controller binding with DT schema format using
json-schema.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 .../devicetree/bindings/pwm/pwm-loongson.yaml | 64 +++++++++++++++++++
 MAINTAINERS                                   |  6 ++
 2 files changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/pwm-loongson.yaml

diff --git a/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml b/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
new file mode 100644
index 000000000000..d25904468353
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/pwm-loongson.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Loongson PWM Controller
+
+maintainers:
+  - Binbin Zhou <zhoubinbin@loongson.cn>
+
+description:
+  It is the generic PWM framework driver for Loongson family.
+  Each PWM has one pulse width output signal and one pulse input
+  signal to be measured.
+  It can be found on Loongson-2K series cpus and Loongson LS7A bridge chips.
+
+allOf:
+  - $ref: pwm.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - const: loongson,ls7a-pwm
+      - items:
+          - enum:
+              - loongson,ls2k0500-pwm
+              - loongson,ls2k1000-pwm
+              - loongson,ls2k2000-pwm
+          - const: loongson,ls7a-pwm
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  '#pwm-cells':
+    const: 3
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - '#pwm-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/clock/loongson,ls2k-clk.h>
+    pwm@1fe22000 {
+        compatible = "loongson,ls2k1000-pwm", "loongson,ls7a-pwm";
+        reg = <0x1fe22000 0x10>;
+        interrupt-parent = <&liointc0>;
+        interrupts = <24 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&clk LOONGSON2_APB_CLK>;
+        #pwm-cells = <3>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 96496dbcae6d..9c4f3186e7f4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12753,6 +12753,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/i2c/loongson,ls2x-i2c.yaml
 F:	drivers/i2c/busses/i2c-ls2x.c
 
+LOONGSON PWM DRIVER
+M:	Binbin Zhou <zhoubinbin@loongson.cn>
+L:	linux-pwm@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/pwm/pwm-loongson.yaml
+
 LOONGSON-2 SOC SERIES CLOCK DRIVER
 M:	Yinbo Zhu <zhuyinbo@loongson.cn>
 L:	linux-clk@vger.kernel.org
-- 
2.43.0


^ permalink raw reply related

* [PATCH v1 3/5] LoongArch: dts: Add PWM support to Loongson-2K0500
From: Binbin Zhou @ 2024-04-02  7:58 UTC (permalink / raw)
  To: Binbin Zhou, Huacai Chen, Uwe Kleine-König, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: Huacai Chen, loongson-kernel, linux-pwm, devicetree, Xuerui Wang,
	loongarch, Binbin Zhou
In-Reply-To: <cover.1711953223.git.zhoubinbin@loongson.cn>

The module is supported, enable it.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 arch/loongarch/boot/dts/loongson-2k0500.dtsi | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/loongarch/boot/dts/loongson-2k0500.dtsi b/arch/loongarch/boot/dts/loongson-2k0500.dtsi
index 413b83366367..86e86a267779 100644
--- a/arch/loongarch/boot/dts/loongson-2k0500.dtsi
+++ b/arch/loongarch/boot/dts/loongson-2k0500.dtsi
@@ -148,6 +148,26 @@ eiointc: interrupt-controller@1fe11600 {
 			interrupts = <3>;
 		};
 
+		pwm@1ff5c000 {
+			compatible = "loongson,ls2k0500-pwm", "loongson,ls7a-pwm";
+			reg = <0x0 0x1ff5c000 0x0 0x10>;
+			interrupt-parent = <&eiointc>;
+			interrupts = <20>;
+			clocks = <&clk LOONGSON2_APB_CLK>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
+		pwm@1ff5c010 {
+			compatible = "loongson,ls2k0500-pwm", "loongson,ls7a-pwm";
+			reg = <0x0 0x1ff5c010 0x0 0x10>;
+			interrupt-parent = <&eiointc>;
+			interrupts = <21>;
+			clocks = <&clk LOONGSON2_APB_CLK>;
+			#pwm-cells = <3>;
+			status = "disabled";
+		};
+
 		gmac0: ethernet@1f020000 {
 			compatible = "snps,dwmac-3.70a";
 			reg = <0x0 0x1f020000 0x0 0x10000>;
-- 
2.43.0


^ permalink raw reply related


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