From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 34589C3A5A8 for ; Wed, 4 Sep 2019 07:37:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0A2382339D for ; Wed, 4 Sep 2019 07:37:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1567582620; bh=9tMEcYhnS4XXxNd9Fq6cg1vvF93E3PyA0c/S3pkKcdk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=nkKflekZD8ueMIY8rlDYNeDtpPvGowiouXSmPc5inTJM+Z0cn7zhp/sY/X0LotjfE +HrBC03U5Jv7ZnxSuH0hWhe2/HrsF0zg6y2d8+qOv9COEC67n2mdvFoYmQpfdaRgzk 3BI4gAHs0/IMccRRwT49oOlm+vSW0ZMryXfrLXrw= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729112AbfIDHg7 (ORCPT ); Wed, 4 Sep 2019 03:36:59 -0400 Received: from mail.kernel.org ([198.145.29.99]:41938 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726033AbfIDHg6 (ORCPT ); Wed, 4 Sep 2019 03:36:58 -0400 Received: from localhost (unknown [122.182.201.156]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id D1A4B22CF7; Wed, 4 Sep 2019 07:36:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1567582617; bh=9tMEcYhnS4XXxNd9Fq6cg1vvF93E3PyA0c/S3pkKcdk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=XSzxaQweCp8UhjDZhBkcedCz7pMSLZKAX3ePxOXWUuITOZeqlNEPfeG+n9r1e1jPJ YCXo4f2BLj6ECIANHnU2q37o9hS2HWrlnLnfPPXTiKgvtyeQXX3r4ixcTRka06JsSg xLedyl45/h64FjeKNQvZy+ZRB8RcMeBCqZdZt6mU= Date: Wed, 4 Sep 2019 13:05:49 +0530 From: Vinod Koul To: Pierre-Louis Bossart Cc: Guennadi Liakhovetski , alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, tiwai@suse.de, broonie@kernel.org, gregkh@linuxfoundation.org, jank@cadence.com, srinivas.kandagatla@linaro.org, slawomir.blauciak@intel.com, Bard liao , Rander Wang , Ranjani Sridharan , Liam Girdwood , Jaroslav Kysela , Takashi Iwai , Zhu Yingjiang , YueHaibing , Kai Vehmanen , Arnd Bergmann Subject: Re: [RFC PATCH 4/5] ASoC: SOF: Intel: hda: add SoundWire stream config/free callbacks Message-ID: <20190904073549.GL2672@vkoul-mobl> References: <20190821201720.17768-1-pierre-louis.bossart@linux.intel.com> <20190821201720.17768-5-pierre-louis.bossart@linux.intel.com> <20190822071835.GA30262@ubuntu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.12.0 (2019-05-25) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 22-08-19, 08:53, Pierre-Louis Bossart wrote: > Thanks for the review Guennadi > > > > +static int sdw_config_stream(void *arg, void *s, void *dai, > > > + void *params, int link_id, int alh_stream_id) > > > > I realise, that these function prototypes aren't being introduced by these > > patches, but just wondering whether such overly generic prototype is really > > a good idea here, whether some of those "void *" pointers could be given > > real types. The first one could be "struct device *" etc. > > In this case the 'arg' parameter is actually a private 'struct snd_sof_dev', > as shown below [1]. We probably want to keep this relatively opaque, this is > a context that doesn't need to be exposed to the SoundWire code. This does look bit ugly. > The dai and params are indeed cases where we could use stronger types, they > are snd_soc_dai and hw_params respectively. I don't recall why the existing > code is this way, Vinod and Sanyog may have the history of this. Yes we wanted to decouple the sdw and audio bits that is the reason why none of the audio types are used here, but I think it should be revisited and perhaps made as: sdw_config_stream(struct device *sdw, struct sdw_callback_ctx *ctx) where the callback context contains all the other args. That would make it look lot neater too and of course use real structs if possible -- ~Vinod From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.5 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 241B8C3A5A8 for ; Wed, 4 Sep 2019 07:37:59 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 20581206BB for ; Wed, 4 Sep 2019 07:37:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=alsa-project.org header.i=@alsa-project.org header.b="mY5fug2v"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="XSzxaQwe" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 20581206BB Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 555A8167A; Wed, 4 Sep 2019 09:37:06 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 555A8167A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1567582676; bh=pFsjPW2J3UIA1S8Zy9ITp4xANIP2pSd02AiGbrF/pyI=; h=Date:From:To:References:In-Reply-To:Cc:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=mY5fug2vZ+gFIu3nd6SB7gDw7TQ1HiD64PSNo8pZEftCfpYrN87P1VlVc7nrCL4zS 2whGyxwXhj2st/juIB6yTo1fxS7/vHSrFEiklHI/kiF40bNHeklMaymCPwkLR4G4+1 pIbvDhukBlK7Alq8Iy6pGGzlpZj4+4q2vSQwul3g= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 6FAFAF80394; Wed, 4 Sep 2019 09:37:05 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 0700EF803A6; Wed, 4 Sep 2019 09:37:04 +0200 (CEST) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id DC258F8011E for ; Wed, 4 Sep 2019 09:37:00 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz DC258F8011E Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="XSzxaQwe" Received: from localhost (unknown [122.182.201.156]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id D1A4B22CF7; Wed, 4 Sep 2019 07:36:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1567582617; bh=9tMEcYhnS4XXxNd9Fq6cg1vvF93E3PyA0c/S3pkKcdk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=XSzxaQweCp8UhjDZhBkcedCz7pMSLZKAX3ePxOXWUuITOZeqlNEPfeG+n9r1e1jPJ YCXo4f2BLj6ECIANHnU2q37o9hS2HWrlnLnfPPXTiKgvtyeQXX3r4ixcTRka06JsSg xLedyl45/h64FjeKNQvZy+ZRB8RcMeBCqZdZt6mU= Date: Wed, 4 Sep 2019 13:05:49 +0530 From: Vinod Koul To: Pierre-Louis Bossart Message-ID: <20190904073549.GL2672@vkoul-mobl> References: <20190821201720.17768-1-pierre-louis.bossart@linux.intel.com> <20190821201720.17768-5-pierre-louis.bossart@linux.intel.com> <20190822071835.GA30262@ubuntu> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.12.0 (2019-05-25) Cc: Guennadi Liakhovetski , alsa-devel@alsa-project.org, Arnd Bergmann , Kai Vehmanen , Liam Girdwood , tiwai@suse.de, gregkh@linuxfoundation.org, Takashi Iwai , YueHaibing , linux-kernel@vger.kernel.org, Ranjani Sridharan , broonie@kernel.org, srinivas.kandagatla@linaro.org, jank@cadence.com, slawomir.blauciak@intel.com, Zhu Yingjiang , Bard liao , Rander Wang Subject: Re: [alsa-devel] [RFC PATCH 4/5] ASoC: SOF: Intel: hda: add SoundWire stream config/free callbacks X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On 22-08-19, 08:53, Pierre-Louis Bossart wrote: > Thanks for the review Guennadi > > > > +static int sdw_config_stream(void *arg, void *s, void *dai, > > > + void *params, int link_id, int alh_stream_id) > > > > I realise, that these function prototypes aren't being introduced by these > > patches, but just wondering whether such overly generic prototype is really > > a good idea here, whether some of those "void *" pointers could be given > > real types. The first one could be "struct device *" etc. > > In this case the 'arg' parameter is actually a private 'struct snd_sof_dev', > as shown below [1]. We probably want to keep this relatively opaque, this is > a context that doesn't need to be exposed to the SoundWire code. This does look bit ugly. > The dai and params are indeed cases where we could use stronger types, they > are snd_soc_dai and hw_params respectively. I don't recall why the existing > code is this way, Vinod and Sanyog may have the history of this. Yes we wanted to decouple the sdw and audio bits that is the reason why none of the audio types are used here, but I think it should be revisited and perhaps made as: sdw_config_stream(struct device *sdw, struct sdw_callback_ctx *ctx) where the callback context contains all the other args. That would make it look lot neater too and of course use real structs if possible -- ~Vinod _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel