All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] ASoC: ti: davinci-mcasp: Handle incomplete DT node gracefully
@ 2020-11-06  7:25 Peter Ujfalusi
  2020-11-06  7:25 ` [PATCH 1/4] ASoC: ti: davinci-mcasp: Use platform_get_irq_byname_optional Peter Ujfalusi
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Peter Ujfalusi @ 2020-11-06  7:25 UTC (permalink / raw
  To: broonie, lgirdwood; +Cc: nm, tony, alsa-devel, tomi.valkeinen

Hi,

The series is inspired by the effort to standardize TI's arm64 dtsi files to keep
all nodes in 'okay' state and let the board dts files disable not needed
peripherals (and not properly configured):
https://lore.kernel.org/lkml/20201104224356.18040-1-nm@ti.com/

In the unlikely (or likely?) event when the dts misses to disable the McASP node
which is not configured we currenly and luckily just manage to not crash as we
had fixup code in place for legacy pdata boots.
This however prints out a message which does not really help to identify the
issue.

This series will reduce some of the noise during boot (first patch) then
adds the needed changes to handle the variations of 'okay':
A - have all required DT properties for audio
B - gpiochip is enabled

A && !B  -> everything is OK for audio, no gpiochip registered
A && B   -> everything is OK for audio, gpiochip is registered
!A && B  -> audio is not OK, gpiochip is registered. dev_dbg() for audio and do
            not register SOC DAI and PCM
!A && !B ->  audio is not OK, no gpiochip. dev_err() and fail the probe

Regards,
Peter
---
Peter Ujfalusi (4):
  ASoC: ti: davinci-mcasp: Use platform_get_irq_byname_optional
  ASoC: ti: davinci-mcasp: Remove legacy dma_request parsing
  ASoC: ti: davinci-mcasp: Simplify the configuration parameter handling
  ASoC: ti: davinci-mcasp: Handle missing required DT properties

 sound/soc/ti/davinci-mcasp.c | 294 ++++++++++++++---------------------
 1 file changed, 119 insertions(+), 175 deletions(-)

-- 
Peter

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/4] ASoC: ti: davinci-mcasp: Use platform_get_irq_byname_optional
  2020-11-06  7:25 [PATCH 0/4] ASoC: ti: davinci-mcasp: Handle incomplete DT node gracefully Peter Ujfalusi
@ 2020-11-06  7:25 ` Peter Ujfalusi
  2020-11-06  7:25 ` [PATCH 2/4] ASoC: ti: davinci-mcasp: Remove legacy dma_request parsing Peter Ujfalusi
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Peter Ujfalusi @ 2020-11-06  7:25 UTC (permalink / raw
  To: broonie, lgirdwood; +Cc: nm, tony, alsa-devel, tomi.valkeinen

Depending on the integration of McASP either the 'common' or the
'rx' and 'tx' or only the 'tx' interrupt number is valid, provided.

By switching to platform_get_irq_byname_optional() we can clean up the
bootlog from messages like:

davinci-mcasp 2ba0000.mcasp: IRQ common not found

The irq number == 0 is not valid, fix the check at the same time.

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

diff --git a/sound/soc/ti/davinci-mcasp.c b/sound/soc/ti/davinci-mcasp.c
index 31488593c4f1..91238fe92edb 100644
--- a/sound/soc/ti/davinci-mcasp.c
+++ b/sound/soc/ti/davinci-mcasp.c
@@ -2202,8 +2202,8 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 
 	mcasp->dev = &pdev->dev;
 
-	irq = platform_get_irq_byname(pdev, "common");
-	if (irq >= 0) {
+	irq = platform_get_irq_byname_optional(pdev, "common");
+	if (irq > 0) {
 		irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_common",
 					  dev_name(&pdev->dev));
 		if (!irq_name) {
@@ -2223,8 +2223,8 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 		mcasp->irq_request[SNDRV_PCM_STREAM_CAPTURE] = ROVRN;
 	}
 
-	irq = platform_get_irq_byname(pdev, "rx");
-	if (irq >= 0) {
+	irq = platform_get_irq_byname_optional(pdev, "rx");
+	if (irq > 0) {
 		irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_rx",
 					  dev_name(&pdev->dev));
 		if (!irq_name) {
@@ -2242,8 +2242,8 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 		mcasp->irq_request[SNDRV_PCM_STREAM_CAPTURE] = ROVRN;
 	}
 
-	irq = platform_get_irq_byname(pdev, "tx");
-	if (irq >= 0) {
+	irq = platform_get_irq_byname_optional(pdev, "tx");
+	if (irq > 0) {
 		irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_tx",
 					  dev_name(&pdev->dev));
 		if (!irq_name) {
-- 
Peter

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] ASoC: ti: davinci-mcasp: Remove legacy dma_request parsing
  2020-11-06  7:25 [PATCH 0/4] ASoC: ti: davinci-mcasp: Handle incomplete DT node gracefully Peter Ujfalusi
  2020-11-06  7:25 ` [PATCH 1/4] ASoC: ti: davinci-mcasp: Use platform_get_irq_byname_optional Peter Ujfalusi
@ 2020-11-06  7:25 ` Peter Ujfalusi
  2020-11-06  7:25 ` [PATCH 3/4] ASoC: ti: davinci-mcasp: Simplify the configuration parameter handling Peter Ujfalusi
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Peter Ujfalusi @ 2020-11-06  7:25 UTC (permalink / raw
  To: broonie, lgirdwood; +Cc: nm, tony, alsa-devel, tomi.valkeinen

The legacy dma_request (which was holding the DMA request number) is no
longer in use for a long time.
All legacy platforms has been converted to dma_slave_map.

Remove it along with the DT parsing to get tx_dma_channel and
rx_dma_channel.

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

diff --git a/sound/soc/ti/davinci-mcasp.c b/sound/soc/ti/davinci-mcasp.c
index 91238fe92edb..19120bdf747a 100644
--- a/sound/soc/ti/davinci-mcasp.c
+++ b/sound/soc/ti/davinci-mcasp.c
@@ -94,7 +94,6 @@ struct davinci_mcasp {
 	u8	bclk_div;
 	int	streams;
 	u32	irq_request[2];
-	int	dma_request[2];
 
 	int	sysclk_freq;
 	bool	bclk_master;
@@ -1755,7 +1754,6 @@ static struct davinci_mcasp_pdata *davinci_mcasp_set_pdata_from_of(
 	struct davinci_mcasp_pdata *pdata = NULL;
 	const struct of_device_id *match =
 			of_match_device(mcasp_dt_ids, &pdev->dev);
-	struct of_phandle_args dma_spec;
 
 	const u32 *of_serial_dir32;
 	u32 val;
@@ -1810,31 +1808,6 @@ static struct davinci_mcasp_pdata *davinci_mcasp_set_pdata_from_of(
 		pdata->serial_dir = of_serial_dir;
 	}
 
-	ret = of_property_match_string(np, "dma-names", "tx");
-	if (ret < 0)
-		goto nodata;
-
-	ret = of_parse_phandle_with_args(np, "dmas", "#dma-cells", ret,
-					 &dma_spec);
-	if (ret < 0)
-		goto nodata;
-
-	pdata->tx_dma_channel = dma_spec.args[0];
-
-	/* RX is not valid in DIT mode */
-	if (pdata->op_mode != DAVINCI_MCASP_DIT_MODE) {
-		ret = of_property_match_string(np, "dma-names", "rx");
-		if (ret < 0)
-			goto nodata;
-
-		ret = of_parse_phandle_with_args(np, "dmas", "#dma-cells", ret,
-						 &dma_spec);
-		if (ret < 0)
-			goto nodata;
-
-		pdata->rx_dma_channel = dma_spec.args[0];
-	}
-
 	ret = of_property_read_u32(np, "tx-num-evt", &val);
 	if (ret >= 0)
 		pdata->txnumevt = val;
@@ -2127,11 +2100,10 @@ static void davinci_mcasp_get_dt_params(struct davinci_mcasp *mcasp)
 static int davinci_mcasp_probe(struct platform_device *pdev)
 {
 	struct snd_dmaengine_dai_dma_data *dma_data;
-	struct resource *mem, *res, *dat;
+	struct resource *mem, *dat;
 	struct davinci_mcasp_pdata *pdata;
 	struct davinci_mcasp *mcasp;
 	char *irq_name;
-	int *dma;
 	int irq;
 	int ret;
 
@@ -2266,45 +2238,22 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 		mcasp->dat_port = true;
 
 	dma_data = &mcasp->dma_data[SNDRV_PCM_STREAM_PLAYBACK];
+	dma_data->filter_data = "tx";
 	if (dat)
 		dma_data->addr = dat->start;
 	else
 		dma_data->addr = mem->start + davinci_mcasp_txdma_offset(pdata);
 
-	dma = &mcasp->dma_request[SNDRV_PCM_STREAM_PLAYBACK];
-	res = platform_get_resource(pdev, IORESOURCE_DMA, 0);
-	if (res)
-		*dma = res->start;
-	else
-		*dma = pdata->tx_dma_channel;
-
-	/* dmaengine filter data for DT and non-DT boot */
-	if (pdev->dev.of_node)
-		dma_data->filter_data = "tx";
-	else
-		dma_data->filter_data = dma;
 
 	/* RX is not valid in DIT mode */
 	if (mcasp->op_mode != DAVINCI_MCASP_DIT_MODE) {
 		dma_data = &mcasp->dma_data[SNDRV_PCM_STREAM_CAPTURE];
+		dma_data->filter_data = "rx";
 		if (dat)
 			dma_data->addr = dat->start;
 		else
 			dma_data->addr =
 				mem->start + davinci_mcasp_rxdma_offset(pdata);
-
-		dma = &mcasp->dma_request[SNDRV_PCM_STREAM_CAPTURE];
-		res = platform_get_resource(pdev, IORESOURCE_DMA, 1);
-		if (res)
-			*dma = res->start;
-		else
-			*dma = pdata->rx_dma_channel;
-
-		/* dmaengine filter data for DT and non-DT boot */
-		if (pdev->dev.of_node)
-			dma_data->filter_data = "rx";
-		else
-			dma_data->filter_data = dma;
 	}
 
 	if (mcasp->version < MCASP_VERSION_3) {
-- 
Peter

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] ASoC: ti: davinci-mcasp: Simplify the configuration parameter handling
  2020-11-06  7:25 [PATCH 0/4] ASoC: ti: davinci-mcasp: Handle incomplete DT node gracefully Peter Ujfalusi
  2020-11-06  7:25 ` [PATCH 1/4] ASoC: ti: davinci-mcasp: Use platform_get_irq_byname_optional Peter Ujfalusi
  2020-11-06  7:25 ` [PATCH 2/4] ASoC: ti: davinci-mcasp: Remove legacy dma_request parsing Peter Ujfalusi
@ 2020-11-06  7:25 ` Peter Ujfalusi
  2020-11-06  7:25 ` [PATCH 4/4] ASoC: ti: davinci-mcasp: Handle missing required DT properties Peter Ujfalusi
  2020-11-09 19:47 ` [PATCH 0/4] ASoC: ti: davinci-mcasp: Handle incomplete DT node gracefully Mark Brown
  4 siblings, 0 replies; 6+ messages in thread
From: Peter Ujfalusi @ 2020-11-06  7:25 UTC (permalink / raw
  To: broonie, lgirdwood; +Cc: nm, tony, alsa-devel, tomi.valkeinen

Replace the davinci_mcasp_set_pdata_from_of() function which returned a
pdata pointer with davinci_mcasp_get_config() to return an actual error
code and handle all pdata validation and private mcasp struct setup in
there.

Drop the unused ram-size-playback and sram-size-capture query from DT at
the same time.

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

diff --git a/sound/soc/ti/davinci-mcasp.c b/sound/soc/ti/davinci-mcasp.c
index 19120bdf747a..7aba1ccec8c9 100644
--- a/sound/soc/ti/davinci-mcasp.c
+++ b/sound/soc/ti/davinci-mcasp.c
@@ -76,6 +76,7 @@ struct davinci_mcasp_ruledata {
 
 struct davinci_mcasp {
 	struct snd_dmaengine_dai_dma_data dma_data[2];
+	struct davinci_mcasp_pdata *pdata;
 	void __iomem *base;
 	u32 fifo_base;
 	struct device *dev;
@@ -1747,44 +1748,37 @@ static int mcasp_reparent_fck(struct platform_device *pdev)
 	return ret;
 }
 
-static struct davinci_mcasp_pdata *davinci_mcasp_set_pdata_from_of(
-						struct platform_device *pdev)
+static int davinci_mcasp_get_config(struct davinci_mcasp *mcasp,
+				    struct platform_device *pdev)
 {
+	const struct of_device_id *match = of_match_device(mcasp_dt_ids, &pdev->dev);
 	struct device_node *np = pdev->dev.of_node;
 	struct davinci_mcasp_pdata *pdata = NULL;
-	const struct of_device_id *match =
-			of_match_device(mcasp_dt_ids, &pdev->dev);
-
 	const u32 *of_serial_dir32;
 	u32 val;
-	int i, ret = 0;
+	int i;
 
 	if (pdev->dev.platform_data) {
 		pdata = pdev->dev.platform_data;
 		pdata->dismod = DISMOD_LOW;
-		return pdata;
+		goto out;
 	} else if (match) {
 		pdata = devm_kmemdup(&pdev->dev, match->data, sizeof(*pdata),
 				     GFP_KERNEL);
 		if (!pdata)
-			return NULL;
+			return -ENOMEM;
 	} else {
-		/* control shouldn't reach here. something is wrong */
-		ret = -EINVAL;
-		goto nodata;
+		dev_err(&pdev->dev, "No compatible match found\n");
+		return -EINVAL;
 	}
 
-	ret = of_property_read_u32(np, "op-mode", &val);
-	if (ret >= 0)
+	if (of_property_read_u32(np, "op-mode", &val) == 0)
 		pdata->op_mode = val;
 
-	ret = of_property_read_u32(np, "tdm-slots", &val);
-	if (ret >= 0) {
+	if (of_property_read_u32(np, "tdm-slots", &val) == 0) {
 		if (val < 2 || val > 32) {
-			dev_err(&pdev->dev,
-				"tdm-slots must be in rage [2-32]\n");
-			ret = -EINVAL;
-			goto nodata;
+			dev_err(&pdev->dev, "tdm-slots must be in rage [2-32]\n");
+			return -EINVAL;
 		}
 
 		pdata->tdm_slots = val;
@@ -1796,10 +1790,8 @@ static struct davinci_mcasp_pdata *davinci_mcasp_set_pdata_from_of(
 		u8 *of_serial_dir = devm_kzalloc(&pdev->dev,
 						 (sizeof(*of_serial_dir) * val),
 						 GFP_KERNEL);
-		if (!of_serial_dir) {
-			ret = -ENOMEM;
-			goto nodata;
-		}
+		if (!of_serial_dir)
+			return -ENOMEM;
 
 		for (i = 0; i < val; i++)
 			of_serial_dir[i] = be32_to_cpup(&of_serial_dir32[i]);
@@ -1808,24 +1800,16 @@ static struct davinci_mcasp_pdata *davinci_mcasp_set_pdata_from_of(
 		pdata->serial_dir = of_serial_dir;
 	}
 
-	ret = of_property_read_u32(np, "tx-num-evt", &val);
-	if (ret >= 0)
+	if (of_property_read_u32(np, "tx-num-evt", &val) == 0)
 		pdata->txnumevt = val;
 
-	ret = of_property_read_u32(np, "rx-num-evt", &val);
-	if (ret >= 0)
+	if (of_property_read_u32(np, "rx-num-evt", &val) == 0)
 		pdata->rxnumevt = val;
 
-	ret = of_property_read_u32(np, "sram-size-playback", &val);
-	if (ret >= 0)
-		pdata->sram_size_playback = val;
-
-	ret = of_property_read_u32(np, "sram-size-capture", &val);
-	if (ret >= 0)
-		pdata->sram_size_capture = val;
+	if (of_property_read_u32(np, "auxclk-fs-ratio", &val) == 0)
+		mcasp->auxclk_fs_ratio = val;
 
-	ret = of_property_read_u32(np, "dismod", &val);
-	if (ret >= 0) {
+	if (of_property_read_u32(np, "dismod", &val) == 0) {
 		if (val == 0 || val == 2 || val == 3) {
 			pdata->dismod = DISMOD_VAL(val);
 		} else {
@@ -1836,15 +1820,40 @@ static struct davinci_mcasp_pdata *davinci_mcasp_set_pdata_from_of(
 		pdata->dismod = DISMOD_LOW;
 	}
 
-	return  pdata;
+out:
+	mcasp->pdata = pdata;
 
-nodata:
-	if (ret < 0) {
-		dev_err(&pdev->dev, "Error populating platform data, err %d\n",
-			ret);
-		pdata = NULL;
+	mcasp->op_mode = pdata->op_mode;
+	/* sanity check for tdm slots parameter */
+	if (mcasp->op_mode == DAVINCI_MCASP_IIS_MODE) {
+		if (pdata->tdm_slots < 2) {
+			dev_warn(&pdev->dev, "invalid tdm slots: %d\n",
+				 pdata->tdm_slots);
+			mcasp->tdm_slots = 2;
+		} else if (pdata->tdm_slots > 32) {
+			dev_warn(&pdev->dev, "invalid tdm slots: %d\n",
+				 pdata->tdm_slots);
+			mcasp->tdm_slots = 32;
+		} else {
+			mcasp->tdm_slots = pdata->tdm_slots;
+		}
 	}
-	return  pdata;
+
+	mcasp->num_serializer = pdata->num_serializer;
+#ifdef CONFIG_PM
+	mcasp->context.xrsr_regs = devm_kcalloc(&pdev->dev,
+						mcasp->num_serializer, sizeof(u32),
+						GFP_KERNEL);
+	if (!mcasp->context.xrsr_regs)
+		return -ENOMEM;
+#endif
+	mcasp->serial_dir = pdata->serial_dir;
+	mcasp->version = pdata->version;
+	mcasp->txnumevt = pdata->txnumevt;
+	mcasp->rxnumevt = pdata->rxnumevt;
+	mcasp->dismod = pdata->dismod;
+
+	return 0;
 }
 
 enum {
@@ -2083,25 +2092,10 @@ static inline int davinci_mcasp_init_gpiochip(struct davinci_mcasp *mcasp)
 }
 #endif /* CONFIG_GPIOLIB */
 
-static void davinci_mcasp_get_dt_params(struct davinci_mcasp *mcasp)
-{
-	struct device_node *np = mcasp->dev->of_node;
-	int ret;
-	u32 val;
-
-	if (!np)
-		return;
-
-	ret = of_property_read_u32(np, "auxclk-fs-ratio", &val);
-	if (ret >= 0)
-		mcasp->auxclk_fs_ratio = val;
-}
-
 static int davinci_mcasp_probe(struct platform_device *pdev)
 {
 	struct snd_dmaengine_dai_dma_data *dma_data;
 	struct resource *mem, *dat;
-	struct davinci_mcasp_pdata *pdata;
 	struct davinci_mcasp *mcasp;
 	char *irq_name;
 	int irq;
@@ -2117,11 +2111,10 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 	if (!mcasp)
 		return	-ENOMEM;
 
-	pdata = davinci_mcasp_set_pdata_from_of(pdev);
-	if (!pdata) {
-		dev_err(&pdev->dev, "no platform data\n");
-		return -EINVAL;
-	}
+	mcasp->dev = &pdev->dev;
+	ret = davinci_mcasp_get_config(mcasp, pdev);
+	if (ret)
+		return ret;
 
 	mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mpu");
 	if (!mem) {
@@ -2140,40 +2133,6 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 
 	pm_runtime_enable(&pdev->dev);
 
-	mcasp->op_mode = pdata->op_mode;
-	/* sanity check for tdm slots parameter */
-	if (mcasp->op_mode == DAVINCI_MCASP_IIS_MODE) {
-		if (pdata->tdm_slots < 2) {
-			dev_err(&pdev->dev, "invalid tdm slots: %d\n",
-				pdata->tdm_slots);
-			mcasp->tdm_slots = 2;
-		} else if (pdata->tdm_slots > 32) {
-			dev_err(&pdev->dev, "invalid tdm slots: %d\n",
-				pdata->tdm_slots);
-			mcasp->tdm_slots = 32;
-		} else {
-			mcasp->tdm_slots = pdata->tdm_slots;
-		}
-	}
-
-	mcasp->num_serializer = pdata->num_serializer;
-#ifdef CONFIG_PM
-	mcasp->context.xrsr_regs = devm_kcalloc(&pdev->dev,
-					mcasp->num_serializer, sizeof(u32),
-					GFP_KERNEL);
-	if (!mcasp->context.xrsr_regs) {
-		ret = -ENOMEM;
-		goto err;
-	}
-#endif
-	mcasp->serial_dir = pdata->serial_dir;
-	mcasp->version = pdata->version;
-	mcasp->txnumevt = pdata->txnumevt;
-	mcasp->rxnumevt = pdata->rxnumevt;
-	mcasp->dismod = pdata->dismod;
-
-	mcasp->dev = &pdev->dev;
-
 	irq = platform_get_irq_byname_optional(pdev, "common");
 	if (irq > 0) {
 		irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_common",
@@ -2242,7 +2201,7 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 	if (dat)
 		dma_data->addr = dat->start;
 	else
-		dma_data->addr = mem->start + davinci_mcasp_txdma_offset(pdata);
+		dma_data->addr = mem->start + davinci_mcasp_txdma_offset(mcasp->pdata);
 
 
 	/* RX is not valid in DIT mode */
@@ -2253,7 +2212,7 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 			dma_data->addr = dat->start;
 		else
 			dma_data->addr =
-				mem->start + davinci_mcasp_rxdma_offset(pdata);
+				mem->start + davinci_mcasp_rxdma_offset(mcasp->pdata);
 	}
 
 	if (mcasp->version < MCASP_VERSION_3) {
@@ -2306,11 +2265,8 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 	if (ret)
 		goto err;
 
-	davinci_mcasp_get_dt_params(mcasp);
-
-	ret = devm_snd_soc_register_component(&pdev->dev,
-					&davinci_mcasp_component,
-					&davinci_mcasp_dai[pdata->op_mode], 1);
+	ret = devm_snd_soc_register_component(&pdev->dev, &davinci_mcasp_component,
+					      &davinci_mcasp_dai[mcasp->op_mode], 1);
 
 	if (ret != 0)
 		goto err;
-- 
Peter

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/4] ASoC: ti: davinci-mcasp: Handle missing required DT properties
  2020-11-06  7:25 [PATCH 0/4] ASoC: ti: davinci-mcasp: Handle incomplete DT node gracefully Peter Ujfalusi
                   ` (2 preceding siblings ...)
  2020-11-06  7:25 ` [PATCH 3/4] ASoC: ti: davinci-mcasp: Simplify the configuration parameter handling Peter Ujfalusi
@ 2020-11-06  7:25 ` Peter Ujfalusi
  2020-11-09 19:47 ` [PATCH 0/4] ASoC: ti: davinci-mcasp: Handle incomplete DT node gracefully Mark Brown
  4 siblings, 0 replies; 6+ messages in thread
From: Peter Ujfalusi @ 2020-11-06  7:25 UTC (permalink / raw
  To: broonie, lgirdwood; +Cc: nm, tony, alsa-devel, tomi.valkeinen

McASP needs three required properties to be usable for audio:
op-mode, tdm-slots and the serial-dir array.

Instead of probing the driver even without the needed information we should
make sure that all the parameters are provided for operation.

The fact that McASP can act as a GPIO controller for it's pins complicates
this a bit, but as a general rule we can:
- we fail the probe if McASP is not configured to be used as gpiochip
- we will not register the DAI (and PCM) if gpiochip is defined

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

diff --git a/sound/soc/ti/davinci-mcasp.c b/sound/soc/ti/davinci-mcasp.c
index 7aba1ccec8c9..6247ec3d3a09 100644
--- a/sound/soc/ti/davinci-mcasp.c
+++ b/sound/soc/ti/davinci-mcasp.c
@@ -83,6 +83,9 @@ struct davinci_mcasp {
 	struct snd_pcm_substream *substreams[2];
 	unsigned int dai_fmt;
 
+	/* Audio can not be enabled due to missing parameter(s) */
+	bool	missing_audio_param;
+
 	/* McASP specific data */
 	int	tdm_slots;
 	u32	tdm_mask[2];
@@ -1748,6 +1751,17 @@ static int mcasp_reparent_fck(struct platform_device *pdev)
 	return ret;
 }
 
+static bool davinci_mcasp_have_gpiochip(struct davinci_mcasp *mcasp)
+{
+#ifdef CONFIG_OF_GPIO
+	if (mcasp->dev->of_node &&
+	    of_property_read_bool(mcasp->dev->of_node, "gpio-controller"))
+		return true;
+#endif
+
+	return false;
+}
+
 static int davinci_mcasp_get_config(struct davinci_mcasp *mcasp,
 				    struct platform_device *pdev)
 {
@@ -1772,8 +1786,12 @@ static int davinci_mcasp_get_config(struct davinci_mcasp *mcasp,
 		return -EINVAL;
 	}
 
-	if (of_property_read_u32(np, "op-mode", &val) == 0)
+	if (of_property_read_u32(np, "op-mode", &val) == 0) {
 		pdata->op_mode = val;
+	} else {
+		mcasp->missing_audio_param = true;
+		goto out;
+	}
 
 	if (of_property_read_u32(np, "tdm-slots", &val) == 0) {
 		if (val < 2 || val > 32) {
@@ -1782,6 +1800,9 @@ static int davinci_mcasp_get_config(struct davinci_mcasp *mcasp,
 		}
 
 		pdata->tdm_slots = val;
+	} else if (pdata->op_mode == DAVINCI_MCASP_IIS_MODE) {
+		mcasp->missing_audio_param = true;
+		goto out;
 	}
 
 	of_serial_dir32 = of_get_property(np, "serial-dir", &val);
@@ -1798,6 +1819,9 @@ static int davinci_mcasp_get_config(struct davinci_mcasp *mcasp,
 
 		pdata->num_serializer = val;
 		pdata->serial_dir = of_serial_dir;
+	} else {
+		mcasp->missing_audio_param = true;
+		goto out;
 	}
 
 	if (of_property_read_u32(np, "tx-num-evt", &val) == 0)
@@ -1823,6 +1847,16 @@ static int davinci_mcasp_get_config(struct davinci_mcasp *mcasp,
 out:
 	mcasp->pdata = pdata;
 
+	if (mcasp->missing_audio_param) {
+		if (davinci_mcasp_have_gpiochip(mcasp)) {
+			dev_dbg(&pdev->dev, "Missing DT parameter(s) for audio\n");
+			return 0;
+		}
+
+		dev_err(&pdev->dev, "Insufficient DT parameter(s)\n");
+		return -ENODEV;
+	}
+
 	mcasp->op_mode = pdata->op_mode;
 	/* sanity check for tdm slots parameter */
 	if (mcasp->op_mode == DAVINCI_MCASP_IIS_MODE) {
@@ -2072,7 +2106,7 @@ static const struct gpio_chip davinci_mcasp_template_chip = {
 
 static int davinci_mcasp_init_gpiochip(struct davinci_mcasp *mcasp)
 {
-	if (!of_property_read_bool(mcasp->dev->of_node, "gpio-controller"))
+	if (!davinci_mcasp_have_gpiochip(mcasp))
 		return 0;
 
 	mcasp->gpio_chip = davinci_mcasp_template_chip;
@@ -2111,11 +2145,6 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 	if (!mcasp)
 		return	-ENOMEM;
 
-	mcasp->dev = &pdev->dev;
-	ret = davinci_mcasp_get_config(mcasp, pdev);
-	if (ret)
-		return ret;
-
 	mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mpu");
 	if (!mem) {
 		dev_warn(&pdev->dev,
@@ -2131,8 +2160,23 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 	if (IS_ERR(mcasp->base))
 		return PTR_ERR(mcasp->base);
 
+	dev_set_drvdata(&pdev->dev, mcasp);
 	pm_runtime_enable(&pdev->dev);
 
+	mcasp->dev = &pdev->dev;
+	ret = davinci_mcasp_get_config(mcasp, pdev);
+	if (ret)
+		goto err;
+
+	/* All PINS as McASP */
+	pm_runtime_get_sync(mcasp->dev);
+	mcasp_set_reg(mcasp, DAVINCI_MCASP_PFUNC_REG, 0x00000000);
+	pm_runtime_put(mcasp->dev);
+
+	/* Skip audio related setup code if the configuration is not adequat */
+	if (mcasp->missing_audio_param)
+		goto no_audio;
+
 	irq = platform_get_irq_byname_optional(pdev, "common");
 	if (irq > 0) {
 		irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s_common",
@@ -2252,19 +2296,8 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 	if (ret)
 		goto err;
 
-	dev_set_drvdata(&pdev->dev, mcasp);
-
 	mcasp_reparent_fck(pdev);
 
-	/* All PINS as McASP */
-	pm_runtime_get_sync(mcasp->dev);
-	mcasp_set_reg(mcasp, DAVINCI_MCASP_PFUNC_REG, 0x00000000);
-	pm_runtime_put(mcasp->dev);
-
-	ret = davinci_mcasp_init_gpiochip(mcasp);
-	if (ret)
-		goto err;
-
 	ret = devm_snd_soc_register_component(&pdev->dev, &davinci_mcasp_component,
 					      &davinci_mcasp_dai[mcasp->op_mode], 1);
 
@@ -2293,8 +2326,14 @@ static int davinci_mcasp_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	return 0;
+no_audio:
+	ret = davinci_mcasp_init_gpiochip(mcasp);
+	if (ret) {
+		dev_err(&pdev->dev, "gpiochip registration failed: %d\n", ret);
+		goto err;
+	}
 
+	return 0;
 err:
 	pm_runtime_disable(&pdev->dev);
 	return ret;
-- 
Peter

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/4] ASoC: ti: davinci-mcasp: Handle incomplete DT node gracefully
  2020-11-06  7:25 [PATCH 0/4] ASoC: ti: davinci-mcasp: Handle incomplete DT node gracefully Peter Ujfalusi
                   ` (3 preceding siblings ...)
  2020-11-06  7:25 ` [PATCH 4/4] ASoC: ti: davinci-mcasp: Handle missing required DT properties Peter Ujfalusi
@ 2020-11-09 19:47 ` Mark Brown
  4 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2020-11-09 19:47 UTC (permalink / raw
  To: Peter Ujfalusi, lgirdwood; +Cc: nm, tony, alsa-devel, tomi.valkeinen

On Fri, 6 Nov 2020 09:25:47 +0200, Peter Ujfalusi wrote:
> The series is inspired by the effort to standardize TI's arm64 dtsi files to keep
> all nodes in 'okay' state and let the board dts files disable not needed
> peripherals (and not properly configured):
> https://lore.kernel.org/lkml/20201104224356.18040-1-nm@ti.com/
> 
> In the unlikely (or likely?) event when the dts misses to disable the McASP node
> which is not configured we currenly and luckily just manage to not crash as we
> had fixup code in place for legacy pdata boots.
> This however prints out a message which does not really help to identify the
> issue.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/4] ASoC: ti: davinci-mcasp: Use platform_get_irq_byname_optional
      commit: 372c4bd11de1793667e11d19c29fffc80495eeca
[2/4] ASoC: ti: davinci-mcasp: Remove legacy dma_request parsing
      commit: db8793a39b293d5a8983e1713a70a76cb039c2fe
[3/4] ASoC: ti: davinci-mcasp: Simplify the configuration parameter handling
      commit: 1125d925990b8d8166c45396c9281e2a705c97f8
[4/4] ASoC: ti: davinci-mcasp: Handle missing required DT properties
      commit: 1b4fb70e5b28a477478417a7958e0228460ffe68

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-11-09 19:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-06  7:25 [PATCH 0/4] ASoC: ti: davinci-mcasp: Handle incomplete DT node gracefully Peter Ujfalusi
2020-11-06  7:25 ` [PATCH 1/4] ASoC: ti: davinci-mcasp: Use platform_get_irq_byname_optional Peter Ujfalusi
2020-11-06  7:25 ` [PATCH 2/4] ASoC: ti: davinci-mcasp: Remove legacy dma_request parsing Peter Ujfalusi
2020-11-06  7:25 ` [PATCH 3/4] ASoC: ti: davinci-mcasp: Simplify the configuration parameter handling Peter Ujfalusi
2020-11-06  7:25 ` [PATCH 4/4] ASoC: ti: davinci-mcasp: Handle missing required DT properties Peter Ujfalusi
2020-11-09 19:47 ` [PATCH 0/4] ASoC: ti: davinci-mcasp: Handle incomplete DT node gracefully Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.