Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <87mt1u7fql.wl-kuninori.morimoto.gx@renesas.com>
From:   Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
To:     Herve Codina <herve.codina@bootlin.com>
Cc:     Liam Girdwood <lgirdwood@gmail.com>,
        Mark Brown <broonie@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Jonathan Cameron <jic23@kernel.org>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Jaroslav Kysela <perex@perex.cz>,
        Takashi Iwai <tiwai@suse.com>, alsa-devel@alsa-project.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-iio@vger.kernel.org,
        Christophe Leroy <christophe.leroy@csgroup.eu>,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [PATCH v2 9/9] ASoC: simple-card: Handle additional devices
In-Reply-To: <20230523151223.109551-10-herve.codina@bootlin.com>
References: <20230523151223.109551-1-herve.codina@bootlin.com>
        <20230523151223.109551-10-herve.codina@bootlin.com>
User-Agent: Wanderlust/2.15.9 Emacs/27.1 Mule/6.0
Content-Type: text/plain; charset=US-ASCII
Date:   Wed, 24 May 2023 00:08:50 +0000
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?XnSTsSsIYS1Vy1mxjgx1D7+1muwRLljFdqYY9c51C5Cdn/TCHTLWL0R7yydO?=
 =?us-ascii?Q?MjIGyj73TcQD1wTDxvuuqcOqy/x3FLesUK07AZLzQuD+yFFOQaHaWxBPGhgG?=
 =?us-ascii?Q?MmpbBm1Nz5r6eO71UMORzzMa92eTWGdAazP0kvc0ZbqIobGrdPK/3NFQswXZ?=
 =?us-ascii?Q?zx1JQd8uTK7H1O+Xsm8Z10l79qpWvwmcSfDtCMrfVrSqVYpyOv2ocVi98DYc?=
 =?us-ascii?Q?t6YOiRhJ4ffx2z4HzTDqx1Dj9uQBJVfPO0VPpiPEqE9pArz53vFGRJtcYWsi?=
 =?us-ascii?Q?FUAUqQItS1MNFAZE4lhvaGMhzTqSF6wiTWK9ZtmYDr5VoVoLPGh95kMkzSOY?=
 =?us-ascii?Q?HX6Kxwx9+gyzlJ5B6Jb56GcW++SoWnJ/d4tfY1+di+Vjwa2mZP6OL5f+TARD?=
 =?us-ascii?Q?7s4ZYUSxkKO+3hoR83r4c05S6ErH9tsagbEysxbDrJHTQhT0o+t2vNZjIXzI?=
 =?us-ascii?Q?XcvW0ObuCdTjv7DVmugCEvXqVlRJGWMd82Kzd6A4NtqOzbQVddN/Jf+694Cf?=
 =?us-ascii?Q?sr72dKEt/pllsON4dAHTicX8/tVmQI2Kej/3kR0ICCp5zJzAWN9m9elMrWKz?=
 =?us-ascii?Q?sAKqC0OJDCXHCbT7WpmxABAizf2dD+F9S5UxWaCvREBW08+EEToACp7/2BcJ?=
 =?us-ascii?Q?HeI3q1lN64ipL16CcufIY7nsR0nOwxcpy9nMXf3SFXhzNzo4SqywYIntwkXl?=
 =?us-ascii?Q?2WbQEmDAaAhRbD1HZNt51fFzvSiPrv5X0nB/oL1BDQLHbDzFzyA1Xy1N0Tbe?=
 =?us-ascii?Q?zo3O9ZQ8yUv+AWEbQa2vgKbiSWfHiXkbGx8+Gomqcnu0o0eTkPfvE5wHB42t?=
 =?us-ascii?Q?czYdEuow6GhUm7P6sAlzY3+B3lL3kfQt6x9pwGcuQK8tHgCkOQphSW6cQC/5?=
 =?us-ascii?Q?3YM8wes5Sz1+QasNQHj9XC1r3HDFHVnQOmpljsnvC6gshGZGBGMI0GToJ9rD?=
 =?us-ascii?Q?QZcRTd322LEYR/L1aQK9a/Sf8cfyFNUVEvjA0MwsdVS7oZqVGWepmNHWE0QX?=
 =?us-ascii?Q?YYpiSEqSjl7v1QXn27QpmrXYPyTNxT+tvIZjsWf5rK+oRiQ368QYJoNcFl0A?=
 =?us-ascii?Q?Ohk4TT4uMcwW9CVzdvOcvO3jevuOm7YtaLNwqxtiqjnj9yetD9+tnh9zisSP?=
 =?us-ascii?Q?meDxtAi0LIa1Pd+W7P9y7Id8glHZsjLcUgVgWYL4E7Qa4Y8nYV+ChTv0QK4j?=
 =?us-ascii?Q?yrqBDFQ+03fylHhdzhVTpyAxQIkg+vT+hLdmaSioGOdjwpB9NPxjlFXY6VHA?=
 =?us-ascii?Q?T+el1cqk+D4JY7FNVjQCEyB6l68gNWOMyO5NjdwtL75o00DrRUMsCVTXOUOL?=
 =?us-ascii?Q?NryyvjRRDTS6W4JrFgMpKBz36/Pk4yqJ/smtW4PqJ/fCVMgYytT5bkO80okZ?=
 =?us-ascii?Q?/XRoeuT+bkWAXxXhjfU786WMhhaCs+XLThV+cpFaEgPCzwvqF6PEHHSAuhkc?=
 =?us-ascii?Q?ZagQx6GkqWJssWpsCAAtkJfkJRhGDVIPMm68eVOftqKy//9bphecs2VCfzYv?=
 =?us-ascii?Q?1D4bkm1SZovI6nrkQQwW1ZuYPiDkiXWgQwi4DRzna1rQviW4lmovBtjU5P8E?=
 =?us-ascii?Q?A5DwHvyn6EU9IHa2eWtzZfOOoODzqSLzFOl2eoh6lTR+t7v4q2yqD5MUW/1A?=
 =?us-ascii?Q?eDp+5dxmtAR8GgwiuEv5pRc=3D?=
X-OriginatorOrg: renesas.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5dc2cd7b-2052-4e69-293d-08db5beb0da3
X-MS-Exchange-CrossTenant-AuthSource: OS3PR01MB8426.jpnprd01.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 May 2023 00:08:50.8285
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 53d82571-da19-47e4-9cb4-625a166a4a2a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: dTjhNs5BjB7ttYIzNXRFDc/p/0AZ1euRrC+MJTkVRvOT17t4uS5nSqzIGeXgikttcsMQ1EqUgnBxAleLX22Hk9bSDBrriHR1weduwg3x6Q9zu5jWnNWivXstWlgMi7Z+
X-MS-Exchange-Transport-CrossTenantHeadersStamped: TYWPR01MB9662
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


Hi

> An additional-devs subnode can be present in the simple-card top node.
> This subnode is used to declared some "virtual" additional devices.
> 
> Create related devices from this subnode and avoid this subnode presence
> to interfere with the already supported subnodes analysis.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---

simple-card is used in many boards, but is old.
Adding new feature on audio-graph-card/audio-graph-card2 instead of simple-card
is my ideal, but it is OK.

simple-card is possible to handle multiple DAI links by using
"dai-link" node on 1 Sound Card. see

	https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/sound/simple-card.yaml?h=v6.4-rc3#n294

Is this "additional-devs" available only one per 1 Card ?
If it is possible to use 1 additional-devs per 1 DAI link, I think this patch want to
care "dai-link".
Or adding temporally NOTE or FIXME message like /* NOTE: it doesn't support dai-link so far */
is good idea.

>  static int asoc_simple_probe(struct platform_device *pdev)
>  {
>  	struct asoc_simple_priv *priv;
> @@ -688,6 +731,11 @@ static int asoc_simple_probe(struct platform_device *pdev)
>  		return ret;
>  
>  	if (np && of_device_is_available(np)) {
> +		ret = simple_populate_aux(priv);
> +		if (ret < 0) {
> +			dev_err_probe(dev, ret, "populate aux error\n");
> +			goto err;
> +		}
>  
>  		ret = simple_parse_of(priv, li);
>  		if (ret < 0) {
> -- 
> 2.40.1
> 

Calling simple_populate_aux() before calling simple_parse_of() is possible,
but looks strange for me.
see below

> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index 5a5e4ecd0f61..4992ab433d6a 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
(snip)
> @@ -359,6 +360,8 @@ static int __simple_for_each_link(struct asoc_simple_priv *priv,
>  		is_top = 1;
>  	}
>  
> +	add_devs = of_get_child_by_name(top, PREFIX "additional-devs");

I think better position to call simple_populate_aux() is here.
But __simple_for_each_link() will be called multiple times for CPU and for Codec.
So maybe you want to calling it under CPU turn.

	 /* NOTE: it doesn't support dai-link so far */
	add_devs = of_get_child_by_name(top, PREFIX "additional-devs");
	if (add_devs && li->cpu) {
		ret = simple_populate_aux(priv);
		...
	}

Thank you for your help !!

Best regards
---
Kuninori Morimoto