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=-4.0 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 DD3DCC433E7 for ; Fri, 9 Oct 2020 11:41:54 +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 BD3E822267 for ; Fri, 9 Oct 2020 11:41:52 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=alsa-project.org header.i=@alsa-project.org header.b="geyiBE1T"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="HDcHBEhX" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BD3E822267 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 17820950; Fri, 9 Oct 2020 13:41:00 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 17820950 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1602243710; bh=djPO21ZI0rtD0x7eWZ96qPf9XDpj0bJs7WKf+/73JqY=; h=Date:From:To:Subject:References:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=geyiBE1T5XrXQVs/zz2KfypWujc7dipvpMvZKx0mV+rMmG7lCE1OsIqtSwV8DufhY wmWUMTlFklusV6b5AeaDFtGuPLX3B9eU3gwwRud3eA+g/k2y6x6Dx7QUyxtiyCj5Gg Nxqvos+/Bmtm1KyWBd9tuMrwhfnNPZZQdjqNeWTU= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 93652F8014D; Fri, 9 Oct 2020 13:40:59 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 3C80CF8015B; Fri, 9 Oct 2020 13:40:58 +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 7D518F80148 for ; Fri, 9 Oct 2020 13:40:55 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 7D518F80148 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="HDcHBEhX" Received: from localhost (unknown [213.57.247.131]) (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 B0C5D22261; Fri, 9 Oct 2020 11:40:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1602243651; bh=djPO21ZI0rtD0x7eWZ96qPf9XDpj0bJs7WKf+/73JqY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=HDcHBEhXf9UNRMliUnYR5v/8aiRm+ZG2p1kwmx+R6ybNi/ZkFjm3mN6dMyfcIU4+S uKEcBdce3uL7cTIvO5tRMdnzsiEFSfeSNdI4A0YFG1/wIwR0gRYqZZILGkHBdIeBkQ 7dXXGUweVqCk/CE2Geya+U0+J54NbsDjvcRzMtW0= Date: Fri, 9 Oct 2020 14:40:47 +0300 From: Leon Romanovsky To: Pierre-Louis Bossart Subject: Re: [PATCH v2 1/6] Add ancillary bus support Message-ID: <20201009114047.GQ13580@unreal> References: <20201006170241.GM1874917@unreal> <20201007192610.GD3964015@unreal> <1e2a38ac-e259-f955-07ad-602431ad354b@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1e2a38ac-e259-f955-07ad-602431ad354b@linux.intel.com> Cc: "alsa-devel@alsa-project.org" , "kuba@kernel.org" , "parav@mellanox.com" , "tiwai@suse.de" , "netdev@vger.kernel.org" , "ranjani.sridharan@linux.intel.com" , "fred.oh@linux.intel.com" , "linux-rdma@vger.kernel.org" , "dledford@redhat.com" , "broonie@kernel.org" , Parav Pandit , Jason Gunthorpe , "gregkh@linuxfoundation.org" , "Ertman, David M" , "Williams, Dan J" , "Saleem, Shiraz" , "davem@davemloft.net" , "Patil, Kiran" 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: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On Thu, Oct 08, 2020 at 08:29:00AM -0500, Pierre-Louis Bossart wrote: > > > > > > > But ... since the init() function is performing both device_init and > > > > > > device_add - it should probably be called ancillary_device_register, > > > > > > and we are back to a single exported API for both register and > > > > > > unregister. > > > > > > > > > > Kind reminder that we introduced the two functions to allow the > > > > > caller to know if it needed to free memory when initialize() fails, > > > > > and it didn't need to free memory when add() failed since > > > > > put_device() takes care of it. If you have a single init() function > > > > > it's impossible to know which behavior to select on error. > > > > > > > > > > I also have a case with SoundWire where it's nice to first > > > > > initialize, then set some data and then add. > > > > > > > > > > > > > The flow as outlined by Parav above does an initialize as the first > > > > step, so every error path out of the function has to do a > > > > put_device(), so you would never need to manually free the memory in > > > the setup function. > > > > It would be freed in the release call. > > > > > > err = ancillary_device_initialize(); > > > if (err) > > > return ret; > > > > > > where is the put_device() here? if the release function does any sort of > > > kfree, then you'd need to do it manually in this case. > > Since device_initialize() failed, put_device() cannot be done here. > > So yes, pseudo code should have shown, > > if (err) { > > kfree(adev); > > return err; > > } > > This doesn't work if the adev is part of a larger structure allocated by the > parent, which is pretty much the intent to extent the basic bus and pass > additional information which can be accessed with container_of(). Please take a look how ib_alloc_device() is implemented. It does all that you wrote above in very similar manner to netdev_alloc. In a nutshell, ib_alloc_device receives needed size from the user and requires from the users to extend their structures below "general" one. Thanks