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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 22E0AC3DA59 for ; Tue, 16 Jul 2024 22:28:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Date:To:Cc:From:Subject: References:In-Reply-To:Content-Transfer-Encoding:MIME-Version:Content-Type: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=vIj7IqTfNWV2+LinFWy5HbZaoPcVjkUOoe8PWb3G0hI=; b=wU9HiyoVmAiXTAkiZbdkcpSI1x GiV3n6XDVnWs1cT2m+YUckm0CSehcpWnmpCrh7n3WdL2DLaveV2BzhuaPw3o1bvkVyiX/KhB21TEj A30M03J055NFCJBaqqoEK9FIGvw7WELYC2mv0vXl1WkuEeBGdAaFGi75RVRj79VuO8uz6nSa8XcMf VdLYZcAUyuekYms5jJk/pYYAw6UvcInwMtZacNb7XUi1h+eooF4lpbK9yRSy2Nw+gUNHDpXo0AkFd jrMlSQ9OpIAzB/EVK71Ndf6IV6Adr08001zK832LHB1+1/V4/w6hF5AlN3dab5cv3YhOfW44dWd7Z ZCgwfADQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sTqes-0000000BxkZ-3Eiq; Tue, 16 Jul 2024 22:28:31 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sTqeX-0000000Bxe8-1Vyg for linux-arm-kernel@lists.infradead.org; Tue, 16 Jul 2024 22:28:11 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 23AE461113; Tue, 16 Jul 2024 22:28:08 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B7C32C116B1; Tue, 16 Jul 2024 22:28:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1721168887; bh=NfPZkGC+r85TH2hak7B3uZ5xb2lxTRsz0Snd8QaZPZA=; h=In-Reply-To:References:Subject:From:Cc:To:Date:From; b=Hce1rIEZ0Thyin4pnSSKLVdjqAvQt5o4Bi8Wljh+Q4reiza4WyXEcCRTWY+Z3l6iF yziZSPyhD3Y05kE8JY+eHXFL3qoH4h8/TUFDFWa+BkQe5w41h3WMkO7R5Ji55Hd4gn D1eXAIgPvKD4LltPe4u+48sJnfTki9tQqM1Pe3U/e5KEHLp++QOpzlpzm0RNg1p2cI X21mE4Px7LjIdWjLAkbqg97IMG4ri907ymCqjEjQLG60JAj94NbqyiWJ98r3kGhV9U v0L0F7+GfRqT5g+WKvPkjPxgtq+egE6G/63jwYwCFbhHOKJSKYrYYkWiRWRlVHB9ZZ 0YT9Jt4a4HwEA== Message-ID: <2abcd440664067d95b1ac0e765ad55a3.sboyd@kernel.org> Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable In-Reply-To: <20240716103025.1198495-4-claudiu.beznea.uj@bp.renesas.com> References: <20240716103025.1198495-1-claudiu.beznea.uj@bp.renesas.com> <20240716103025.1198495-4-claudiu.beznea.uj@bp.renesas.com> Subject: Re: [PATCH v2 03/11] clk: renesas: clk-vbattb: Add VBATTB clock driver From: Stephen Boyd Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-rtc@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org, claudiu.beznea@tuxon.dev, Claudiu Beznea To: Claudiu , alexandre.belloni@bootlin.com, conor+dt@kernel.org, geert+renesas@glider.be, krzk+dt@kernel.org, lee@kernel.org, magnus.damm@gmail.com, mturquette@baylibre.com, p.zabel@pengutronix.de, robh@kernel.org Date: Tue, 16 Jul 2024 15:28:05 -0700 User-Agent: alot/0.10 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240716_152809_502646_E50F5F2D X-CRM114-Status: GOOD ( 19.09 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Quoting Claudiu (2024-07-16 03:30:17) > diff --git a/drivers/clk/renesas/clk-vbattb.c b/drivers/clk/renesas/clk-v= battb.c > new file mode 100644 > index 000000000000..8effe141fc0b > --- /dev/null > +++ b/drivers/clk/renesas/clk-vbattb.c > @@ -0,0 +1,212 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * VBATTB clock driver > + * > + * Copyright (C) 2024 Renesas Electronics Corp. > + */ > + > +#include > +#include Prefer clk providers to not be clk consumers. > +#include > +#include > +#include > +#include > +#include Is of_platform.h used? Include mod_devicetable.h for of_device_id. > +#include > + > +#define VBATTB_BKSCCR 0x0 > +#define VBATTB_BKSCCR_SOSEL BIT(6) > +#define VBATTB_SOSCCR2 0x8 > +#define VBATTB_SOSCCR2_SOSTP2 BIT(0) [..] > + > +static int vbattb_clk_probe(struct platform_device *pdev) > +{ > + struct device_node *np =3D pdev->dev.of_node; > + struct clk_parent_data parent_data =3D {}; > + struct device *dev =3D &pdev->dev; > + struct clk_init_data init =3D {}; > + struct vbattb_clk *vbclk; > + u32 load_capacitance; > + struct clk_hw *hw; > + int ret, bypass; > + > + vbclk =3D devm_kzalloc(dev, sizeof(*vbclk), GFP_KERNEL); > + if (!vbclk) > + return -ENOMEM; > + > + vbclk->base =3D devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(vbclk->base)) > + return PTR_ERR(vbclk->base); > + > + bypass =3D vbattb_clk_need_bypass(dev); This is a tri-state bool :( > + if (bypass < 0) { > + return bypass; > + } else if (bypass) { > + parent_data.fw_name =3D "clkin"; > + bypass =3D VBATTB_BKSCCR_SOSEL; And now it is a mask value. > + } else { > + parent_data.fw_name =3D "xin"; > + } > + > + ret =3D of_property_read_u32(np, "renesas,vbattb-load-nanofarads"= , &load_capacitance); > + if (ret) > + return ret; > + > + ret =3D vbattb_clk_validate_load_capacitance(vbclk, load_capacita= nce); > + if (ret) > + return ret; > + > + vbattb_clk_update_bits(vbclk->base, VBATTB_BKSCCR, VBATTB_BKSCCR_= SOSEL, bypass); Please don't overload 'bypass'. Use two variables or a conditional. I also wonder if this is really a mux, and either assigned-clock-parents should be used, or the clk_ops should have an init routine that looks at which parent is present by determining the index and then use that to set the mux. The framework can take care of failing to set the other parent when it isn't present. > + > + spin_lock_init(&vbclk->lock); > + > + init.name =3D "vbattclk"; > + init.ops =3D &vbattb_clk_ops; > + init.parent_data =3D &parent_data; > + init.num_parents =3D 1; > + init.flags =3D 0; > + > + vbclk->hw.init =3D &init; > + hw =3D &vbclk->hw; > + > + ret =3D devm_clk_hw_register(dev, hw); > + if (ret) > + return ret; > + > + return of_clk_add_hw_provider(np, of_clk_hw_simple_get, hw); > +} > + > +static const struct of_device_id vbattb_clk_match[] =3D { > + { .compatible =3D "renesas,r9a08g045-vbattb-clk" }, > + { /* sentinel */ } > +}; Any MODULE_DEVICE_TABLE?