From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AFC6017CA17; Fri, 21 Feb 2025 21:59:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740175173; cv=none; b=MXxAwRU+55gZbuyG1yE21XXfZt6CizqvGPnUscS00g2ZgQmcSEH4qOxfPYIa9aOD/Mg80FeGkgCEPxdNSOQV6/X/ACWf5c+pao8byFMJQ3Pc2IjYT//IMFx4TFcuUpe0+5QFktPZh+GGHD9O6E0VwMVbOxPbYATrRRq621zReI0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1740175173; c=relaxed/simple; bh=wkNJXN5KmQia3qc0vnjRQKnCI197Q8XmFWs2pvWUs9g=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=pqbY8HCmrzLKIuqdJkVz0KTQ3WIuxNgqwAHk4Su9q1KdR74T20VL4cZLLt3TSgZYpC/O1dA3bCFkDpSOmMwNG2AvjUdzlm/GKMHE4r44oqyURASD4s0DrwNm+AdNpBhV0UyqATj9tXJPIJwMYBqNdgrbhyQuDYqm14CTtooW8IQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=d3JvzTu4; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="d3JvzTu4" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0B10EC4CED6; Fri, 21 Feb 2025 21:59:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1740175173; bh=wkNJXN5KmQia3qc0vnjRQKnCI197Q8XmFWs2pvWUs9g=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=d3JvzTu4zJkPsn2esLlCE1CGA91SJ2u7qCHOGM7lRAhIeStRB1AN0NRWOA+EYfGKf bJPC9nez6oZJ76okTk6TXXpx2d06B6NkbSzWUuREp70iTqfe1WfcRtrNJWctmz8oy+ 4as1RFekBsFyOdL2JcaaQZZSibrOd2j5Pc8ikPmYz3qGNZb3QR5ZAvkL/obyaez1Eg cYyZyaw3PNh13RpCD0PF/Nz9ywsqD0FBPqwXi0MOqYaadr+NDrLhvshBslEgO5B596 jgaePrm3nRrjseizd3WJ+XxwOExSd9FWP7vNxz198B35V6nKvg7iC8kibMWzLeICI5 yL+1Wn2yCUL6w== Date: Fri, 21 Feb 2025 15:59:31 -0600 From: Rob Herring To: Sebastian Reichel , Danilo Krummrich , Viresh Kumar Cc: Daniel Almeida , Michael Turquette , Stephen Boyd , Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , =?iso-8859-1?Q?Bj=F6rn?= Roy Baron , Benno Lossin , Andreas Hindborg , Alice Ryhl , Trevor Gross , Russell King , linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org Subject: Re: [PATCH V2 2/2] rust: Add basic bindings for clk APIs Message-ID: <20250221215931.GA134397-robh@kernel.org> References: <4AD8A8F3-EA7E-4FBE-9F0D-58CF7BB09ED5@collabora.com> Precedence: bulk X-Mailing-List: linux-clk@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Fri, Feb 21, 2025 at 04:28:18PM +0100, Sebastian Reichel wrote: > Hi, > > On Fri, Feb 21, 2025 at 03:47:57PM +0100, Danilo Krummrich wrote: > > On Fri, Feb 21, 2025 at 11:29:21AM -0300, Daniel Almeida wrote: > > > > On 21 Feb 2025, at 10:56, Danilo Krummrich wrote: > > > > On Fri, Feb 21, 2025 at 12:03:39PM +0530, Viresh Kumar wrote: > > > >> +/// A simple implementation of `struct clk` from the C code. > > > >> +#[repr(transparent)] > > > >> +pub struct Clk(*mut bindings::clk); > > > > > > > > I remember that Stephen explained that NULL is valid value for struct clk. As a > > > > consequence, all functions implemented for `Clk` have to consider this. > > > > > > I am a bit confused here. If NULL is valid, then why should we have to specifically > > > consider that in the functions? No functions so far explicitly dereferences that value, > > > they only pass it to the clk framework. > > > > This was badly phrased, the current implementation does not need to consider it > > indeed. What I meant is that we have to consider it potentially. Especially, > > when adding new functionality later on. For instance, when accessing fields of > > struct clk directly. > > Just a drive-by comment - the current implementation will never have > a NULL clock anyways. That is only returned by the clk_get functions > with the _optional() suffix. You are right now only using clk_get(), > which will instead returns ERR_PTR(-ENOENT). It would be nice to handle the optional case from the start. Otherwise, driver writers handle optional or not optional themselves. The not optional case is typically some form of error message duplicated in every driver. Every foo_get() needs foo_get_optional(), so let's figure out the rust way to handle this once for everyone. Rob