* [PATCH v2 00/13] ASoC: ti: davinci-i2s: Add features to McBSP driver
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
This series aims to add some features to McBSP driver.
Convert bindings from .txt to .yaml.
Add possibility to use an external clock as sample rate generator's
input.
Add handling of new formats (TDM, S24_LE, BP_FC).
Enable the detection of unexpected frame pulses.
Set the clock free-running mode according to SND_SOC_DAIFMT_[GATED/CONT]
configuration in DAI format.
Add ti,T1-framing[tx/rx] properties in DT. They allow to set the data
delay to two bit-clock periods.
This has been tested on a platform designed off of the DAVINCI/OMAP-L138
connected to 3 daisy-chained AD7767. An external clock drives the
sample rate generator through the CLKS pin.
The hardware I have only allowed me to test acquisition side of McBSP.
It is connected to a 6 channels TDM and acts as Bit clock provider and
Frame clock consumer.
Change log v1 -> v2:
PATCH 1 (bindings):
* Drop power-domains property's description
* Drop the unused label 'mcbsp0' in example
* Add <> around each entry of the 'dmas' property
* Add 'Reviewed-by: Rob Herring <robh@kernel.org>'
PATCH 2 (bindings):
* Drop the 'ti,enable-sync-err' flag
* Drop the 'ti,disable-free-run' flag
* Add 'Reviewed-by: Rob Herring <robh@kernel.org>'
PATCH 4:
* In probe() use dev_err for fixed error
PATCH 7 (TDM):
* set playback.max_channels to 128
* Add a check on tx_mask as the one done for rx_mask
* Allow TDM with BP_FP format
PATCH 9:
* Detection of unexpected frame pulses is enabled by default
PATCH 10:
* Free-running mode is selected by the DAI format through
SND_SOC_DAIFMT_[CONT/GATED]
PATCH 12:
* drop the 'ti,drive-dx' property
* add 'ti,T1-framing-[rx/tx]' properties
PATCH 13:
* Drop the drive_dx part
* Add support for 'T1 framing' with data delay set to 2 bit-clock
periods
Bastien Curutchet (13):
ASoC: dt-bindings: davinci-mcbsp: convert McBSP bindings to yaml
schema
ASoC: dt-bindings: davinci-mcbsp: Add optional clock
ASoC: ti: davinci-i2s: Remove the unused clk_input_pin attribute
ASoC: ti: davinci-i2s: Replace dev_err with dev_err_probe
ASoC: ti: davinci-i2s: Use external clock to drive sample rate
generator
ASoC: ti: davinci-i2s: Delete unnecessary assignment
ASoC: ti: davinci-i2s: Add TDM support
ASoC: ti: davinci-i2s: Add handling of BP_FC format
ASoC: ti: davinci-i2s: Enable unexpected frame pulses detection
ASoC: ti: davinci-i2s: Link free-run mode to
SND_SOC_DAIFMT_[GATED/CONT]
ASoC: ti: davinci-i2s: Add S24_LE to supported formats
ASoC: dt-bindings: davinci-mcbsp: Add the 'ti,T1-framing-{rx/tx}'
flags
ASoC: ti: davinci-i2s: Add T1 framing support
.../bindings/sound/davinci-mcbsp.txt | 50 ----
.../bindings/sound/davinci-mcbsp.yaml | 113 +++++++
include/linux/platform_data/davinci_asp.h | 15 -
sound/soc/ti/davinci-i2s.c | 278 ++++++++++++++----
4 files changed, 333 insertions(+), 123 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
create mode 100644 Documentation/devicetree/bindings/sound/davinci-mcbsp.yaml
--
2.44.0
^ permalink raw reply
* [PATCH v2 01/13] ASoC: dt-bindings: davinci-mcbsp: convert McBSP bindings to yaml schema
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>
Convert the binding for McBSP controllers for TI SoCs from txt
to YAML schema.
Add properties 'clocks', 'clock-names', 'power-domains' and
'#sound-dai-cells' which were missing from the txt file.
Add '#sound-dai-cells' and 'clocks' in the example which were missing
from the txt file.
Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../bindings/sound/davinci-mcbsp.txt | 50 ----------
.../bindings/sound/davinci-mcbsp.yaml | 95 +++++++++++++++++++
2 files changed, 95 insertions(+), 50 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
create mode 100644 Documentation/devicetree/bindings/sound/davinci-mcbsp.yaml
diff --git a/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt b/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
deleted file mode 100644
index 3ffc2562fb31..000000000000
--- a/Documentation/devicetree/bindings/sound/davinci-mcbsp.txt
+++ /dev/null
@@ -1,50 +0,0 @@
-Texas Instruments DaVinci McBSP module
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-This binding describes the "Multi-channel Buffered Serial Port" (McBSP)
-audio interface found in some TI DaVinci processors like the OMAP-L138 or AM180x.
-
-
-Required properties:
-~~~~~~~~~~~~~~~~~~~~
-- compatible :
- "ti,da850-mcbsp" : for DA850, AM180x and OPAM-L138 platforms
-
-- reg : physical base address and length of the controller memory mapped
- region(s).
-- reg-names : Should contain:
- * "mpu" for the main registers (required).
- * "dat" for the data FIFO (optional).
-
-- dmas: three element list of DMA controller phandles, DMA request line and
- TC channel ordered triplets.
-- dma-names: identifier string for each DMA request line in the dmas property.
- These strings correspond 1:1 with the ordered pairs in dmas. The dma
- identifiers must be "rx" and "tx".
-
-Optional properties:
-~~~~~~~~~~~~~~~~~~~~
-- interrupts : Interrupt numbers for McBSP
-- interrupt-names : Known interrupt names are "rx" and "tx"
-
-- pinctrl-0: Should specify pin control group used for this controller.
-- pinctrl-names: Should contain only one value - "default", for more details
- please refer to pinctrl-bindings.txt
-
-Example (AM1808):
-~~~~~~~~~~~~~~~~~
-
-mcbsp0: mcbsp@1d10000 {
- compatible = "ti,da850-mcbsp";
- pinctrl-names = "default";
- pinctrl-0 = <&mcbsp0_pins>;
-
- reg = <0x00110000 0x1000>,
- <0x00310000 0x1000>;
- reg-names = "mpu", "dat";
- interrupts = <97 98>;
- interrupt-names = "rx", "tx";
- dmas = <&edma0 3 1
- &edma0 2 1>;
- dma-names = "tx", "rx";
-};
diff --git a/Documentation/devicetree/bindings/sound/davinci-mcbsp.yaml b/Documentation/devicetree/bindings/sound/davinci-mcbsp.yaml
new file mode 100644
index 000000000000..139b594dd192
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/davinci-mcbsp.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/davinci-mcbsp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: McBSP Controller for TI SoCs
+
+maintainers:
+ - Bastien Curutchet <bastien.curutchet@bootlin.com>
+
+allOf:
+ - $ref: dai-common.yaml#
+
+properties:
+ compatible:
+ enum:
+ - ti,da850-mcbsp
+
+ reg:
+ minItems: 1
+ items:
+ - description: CFG registers
+ - description: data registers
+
+ reg-names:
+ minItems: 1
+ items:
+ - const: mpu
+ - const: dat
+
+ dmas:
+ items:
+ - description: transmission DMA channel
+ - description: reception DMA channel
+
+ dma-names:
+ items:
+ - const: tx
+ - const: rx
+
+ interrupts:
+ items:
+ - description: RX interrupt
+ - description: TX interrupt
+
+ interrupt-names:
+ items:
+ - const: rx
+ - const: tx
+
+ clocks:
+ items:
+ - description: functional clock
+
+ clock-names:
+ items:
+ - const: fck
+
+ power-domains:
+ maxItems: 1
+
+ "#sound-dai-cells":
+ const: 0
+
+required:
+ - "#sound-dai-cells"
+ - compatible
+ - reg
+ - reg-names
+ - dmas
+ - dma-names
+ - clocks
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ mcbsp0@1d10000 {
+ #sound-dai-cells = <0>;
+ compatible = "ti,da850-mcbsp";
+ pinctrl-names = "default";
+ pinctrl-0 = <&mcbsp0_pins>;
+
+ reg = <0x111000 0x1000>,
+ <0x311000 0x1000>;
+ reg-names = "mpu", "dat";
+ interrupts = <97>, <98>;
+ interrupt-names = "rx", "tx";
+ dmas = <&edma0 3 1>,
+ <&edma0 2 1>;
+ dma-names = "tx", "rx";
+
+ clocks = <&psc1 14>;
+ };
--
2.44.0
^ permalink raw reply related
* [PATCH v2 02/13] ASoC: dt-bindings: davinci-mcbsp: Add optional clock
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>
The McBSP uses an internal sample rate generator to provide bit clock
or frame clock. This sample rate generator can be programmed to be
driven by McBSP's internal clock source or by an external clock source
(located on CLKS pin). The external clock source is not described in
the bindings.
Add an optional clock item that allows to select an external clock as
sample rate generator's input.
Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/sound/davinci-mcbsp.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/davinci-mcbsp.yaml b/Documentation/devicetree/bindings/sound/davinci-mcbsp.yaml
index 139b594dd192..0c2b1936c6a1 100644
--- a/Documentation/devicetree/bindings/sound/davinci-mcbsp.yaml
+++ b/Documentation/devicetree/bindings/sound/davinci-mcbsp.yaml
@@ -50,12 +50,16 @@ properties:
- const: tx
clocks:
+ minItems: 1
items:
- description: functional clock
+ - description: external input clock for sample rate generator.
clock-names:
+ minItems: 1
items:
- const: fck
+ - const: clks
power-domains:
maxItems: 1
--
2.44.0
^ permalink raw reply related
* [PATCH v2 03/13] ASoC: ti: davinci-i2s: Remove the unused clk_input_pin attribute
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>
The clk_input_pin attribute of davinci_mcbsp_dev struct is not set since
commit 257ade78b601 ("ASoC: davinci-i2s: Convert to use edma-pcm").
Remove the attribute.
Keep the behaviour of the MCBSP_CLKR case as MCBSP_CLKR == 0.
I can't test the BC_FP format so I added back the initial comment that
was removed by commit ec6375533748 ("ASoC: DaVinci: Added selection of
clk input pin for McBSP"). This was the last dependency to
linux/platform_data/davinci_asp.h so it is not included anymore.
Remove the enum mcbsp_clk_input_pin from davinci_asp.h as it is not used
anywhere else.
Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
include/linux/platform_data/davinci_asp.h | 15 --------------
sound/soc/ti/davinci-i2s.c | 24 ++++-------------------
2 files changed, 4 insertions(+), 35 deletions(-)
diff --git a/include/linux/platform_data/davinci_asp.h b/include/linux/platform_data/davinci_asp.h
index c8645b2ed3c0..b9c8520b4bd3 100644
--- a/include/linux/platform_data/davinci_asp.h
+++ b/include/linux/platform_data/davinci_asp.h
@@ -25,16 +25,6 @@ struct davinci_mcasp_pdata {
unsigned sram_size_capture;
struct gen_pool *sram_pool;
- /*
- * If McBSP peripheral gets the clock from an external pin,
- * there are three chooses, that are MCBSP_CLKX, MCBSP_CLKR
- * and MCBSP_CLKS.
- * Depending on different hardware connections it is possible
- * to use this setting to change the behaviour of McBSP
- * driver.
- */
- int clk_input_pin;
-
/*
* This flag works when both clock and FS are outputs for the cpu
* and makes clock more accurate (FS is not symmetrical and the
@@ -91,11 +81,6 @@ enum {
MCASP_VERSION_OMAP, /* OMAP4/5 */
};
-enum mcbsp_clk_input_pin {
- MCBSP_CLKR = 0, /* as in DM365 */
- MCBSP_CLKS,
-};
-
#define INACTIVE_MODE 0
#define TX_MODE 1
#define RX_MODE 2
diff --git a/sound/soc/ti/davinci-i2s.c b/sound/soc/ti/davinci-i2s.c
index 07c8b2259208..5c906641640e 100644
--- a/sound/soc/ti/davinci-i2s.c
+++ b/sound/soc/ti/davinci-i2s.c
@@ -19,7 +19,6 @@
#include <linux/delay.h>
#include <linux/io.h>
#include <linux/clk.h>
-#include <linux/platform_data/davinci_asp.h>
#include <sound/core.h>
#include <sound/pcm.h>
@@ -159,7 +158,6 @@ struct davinci_mcbsp_dev {
unsigned int fmt;
int clk_div;
- int clk_input_pin;
bool i2s_accurate_sck;
};
@@ -239,26 +237,12 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
DAVINCI_MCBSP_PCR_CLKRM;
break;
case SND_SOC_DAIFMT_BC_FP:
- pcr = DAVINCI_MCBSP_PCR_FSRM | DAVINCI_MCBSP_PCR_FSXM;
/*
- * Selection of the clock input pin that is the
- * input for the Sample Rate Generator.
- * McBSP FSR and FSX are driven by the Sample Rate
- * Generator.
+ * McBSP CLKR pin is the input for the Sample Rate Generator.
+ * McBSP FSR and FSX are driven by the Sample Rate Generator.
*/
- switch (dev->clk_input_pin) {
- case MCBSP_CLKS:
- pcr |= DAVINCI_MCBSP_PCR_CLKXM |
- DAVINCI_MCBSP_PCR_CLKRM;
- break;
- case MCBSP_CLKR:
- pcr |= DAVINCI_MCBSP_PCR_SCLKME;
- break;
- default:
- dev_err(dev->dev, "bad clk_input_pin\n");
- return -EINVAL;
- }
-
+ pcr = DAVINCI_MCBSP_PCR_FSRM | DAVINCI_MCBSP_PCR_FSXM;
+ pcr |= DAVINCI_MCBSP_PCR_SCLKME;
break;
case SND_SOC_DAIFMT_BC_FC:
/* codec is master */
--
2.44.0
^ permalink raw reply related
* [PATCH v2 04/13] ASoC: ti: davinci-i2s: Replace dev_err with dev_err_probe
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>
In probe(), the dev_err() is used for every returned error.
Replace dev_err() with dev_err_probe() where -EPROBE_DEFER can be
returned.
Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
sound/soc/ti/davinci-i2s.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/ti/davinci-i2s.c b/sound/soc/ti/davinci-i2s.c
index 5c906641640e..cd64f1384e18 100644
--- a/sound/soc/ti/davinci-i2s.c
+++ b/sound/soc/ti/davinci-i2s.c
@@ -708,7 +708,7 @@ static int davinci_i2s_probe(struct platform_device *pdev)
ret = edma_pcm_platform_register(&pdev->dev);
if (ret) {
- dev_err(&pdev->dev, "register PCM failed: %d\n", ret);
+ dev_err_probe(&pdev->dev, ret, "register PCM failed\n");
goto err_unregister_component;
}
--
2.44.0
^ permalink raw reply related
* [PATCH v2 05/13] ASoC: ti: davinci-i2s: Use external clock to drive sample rate generator
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 internal sample rate generator can be programed to be driven by
its internal clock or by an external clock source located on CLKS pin.
The external clock source case is not handled by the driver.
Handle an optional clock related to this external clock source. If
present, the driver uses the clock located on CLKS pin as input for the
sample rate generator. Thus, the external clock rate is used to compute
divisors. If this optional clock is not present, the sample rate
generator is driven by the McBSP's functional clock.
Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
sound/soc/ti/davinci-i2s.c | 65 ++++++++++++++++++++++++++++----------
1 file changed, 49 insertions(+), 16 deletions(-)
diff --git a/sound/soc/ti/davinci-i2s.c b/sound/soc/ti/davinci-i2s.c
index cd64f1384e18..578b4ae28b71 100644
--- a/sound/soc/ti/davinci-i2s.c
+++ b/sound/soc/ti/davinci-i2s.c
@@ -134,6 +134,7 @@ struct davinci_mcbsp_dev {
int mode;
u32 pcr;
struct clk *clk;
+ struct clk *ext_clk;
/*
* Combining both channels into 1 element will at least double the
* amount of time between servicing the dma channel, increase
@@ -364,7 +365,8 @@ 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, srgr, clk_div, freq, framesize;
+ unsigned int rcr, xcr, clk_div, freq, framesize;
+ unsigned int srgr = 0;
u32 spcr;
snd_pcm_format_t fmt;
unsigned element_cnt = 1;
@@ -385,9 +387,13 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
switch (master) {
case SND_SOC_DAIFMT_BP_FP:
- freq = clk_get_rate(dev->clk);
- srgr = DAVINCI_MCBSP_SRGR_FSGM |
- DAVINCI_MCBSP_SRGR_CLKSM;
+ if (dev->ext_clk) {
+ freq = clk_get_rate(dev->ext_clk);
+ } else {
+ freq = clk_get_rate(dev->clk);
+ srgr = DAVINCI_MCBSP_SRGR_CLKSM;
+ }
+ srgr |= DAVINCI_MCBSP_SRGR_FSGM;
srgr |= DAVINCI_MCBSP_SRGR_FWID(mcbsp_word_length *
8 - 1);
if (dev->i2s_accurate_sck) {
@@ -691,12 +697,36 @@ static int davinci_i2s_probe(struct platform_device *pdev)
return -ENODEV;
}
- dev->clk = clk_get(&pdev->dev, NULL);
+ /*
+ * The optional is there for backward compatibility.
+ * If 'fck' is not present, the clk_get(dev, NULL) that follows may find something
+ */
+ dev->clk = devm_clk_get_optional(&pdev->dev, "fck");
if (IS_ERR(dev->clk))
- return -ENODEV;
- ret = clk_enable(dev->clk);
+ return dev_err_probe(&pdev->dev, PTR_ERR(dev->clk), "Invalid functional clock\n");
+ if (!dev->clk) {
+ dev->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(dev->clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(dev->clk),
+ "Missing functional clock\n");
+ }
+
+ dev->ext_clk = devm_clk_get_optional(&pdev->dev, "clks");
+ if (IS_ERR(dev->ext_clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(dev->ext_clk), "Invalid external clock\n");
+
+ ret = clk_prepare_enable(dev->clk);
if (ret)
- goto err_put_clk;
+ return ret;
+
+ if (dev->ext_clk) {
+ dev_dbg(&pdev->dev, "External clock used for sample rate generator\n");
+ ret = clk_prepare_enable(dev->ext_clk);
+ if (ret) {
+ dev_err_probe(&pdev->dev, ret, "Failed to enable external clock\n");
+ goto err_disable_clk;
+ }
+ }
dev->dev = &pdev->dev;
dev_set_drvdata(&pdev->dev, dev);
@@ -704,7 +734,7 @@ static int davinci_i2s_probe(struct platform_device *pdev)
ret = snd_soc_register_component(&pdev->dev, &davinci_i2s_component,
&davinci_i2s_dai, 1);
if (ret != 0)
- goto err_release_clk;
+ goto err_disable_ext_clk;
ret = edma_pcm_platform_register(&pdev->dev);
if (ret) {
@@ -716,10 +746,12 @@ static int davinci_i2s_probe(struct platform_device *pdev)
err_unregister_component:
snd_soc_unregister_component(&pdev->dev);
-err_release_clk:
- clk_disable(dev->clk);
-err_put_clk:
- clk_put(dev->clk);
+err_disable_ext_clk:
+ if (dev->ext_clk)
+ clk_disable_unprepare(dev->ext_clk);
+err_disable_clk:
+ clk_disable_unprepare(dev->clk);
+
return ret;
}
@@ -729,9 +761,10 @@ static void davinci_i2s_remove(struct platform_device *pdev)
snd_soc_unregister_component(&pdev->dev);
- clk_disable(dev->clk);
- clk_put(dev->clk);
- dev->clk = NULL;
+ clk_disable_unprepare(dev->clk);
+
+ if (dev->ext_clk)
+ clk_disable_unprepare(dev->ext_clk);
}
static const struct of_device_id davinci_i2s_match[] __maybe_unused = {
--
2.44.0
^ permalink raw reply related
* [PATCH v2 06/13] ASoC: ti: davinci-i2s: Delete unnecessary assignment
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>
In davinci_i2s_hw_params(), mcbsp_word_length is set twice to
asp_word_length[fmt].
Remove second unnecessary assignment.
Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
sound/soc/ti/davinci-i2s.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/sound/soc/ti/davinci-i2s.c b/sound/soc/ti/davinci-i2s.c
index 578b4ae28b71..a2c7c812ea50 100644
--- a/sound/soc/ti/davinci-i2s.c
+++ b/sound/soc/ti/davinci-i2s.c
@@ -479,7 +479,6 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
return -EINVAL;
}
}
- mcbsp_word_length = asp_word_length[fmt];
switch (master) {
case SND_SOC_DAIFMT_BP_FP:
--
2.44.0
^ permalink raw reply related
* [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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox