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 E2975C47422 for ; Fri, 26 Jan 2024 15:34:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type:Cc: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=ARa9trUtUJ+zdxBBeat3fgSIDo9nUeQKTDf9qgMcs6A=; b=BXEz8QiOfBccmsNbmqQy+Yr8Xa XetVRLeLJaLZrz9sTLfmF36hP0djuzvf9AWPXj9ioYLLI2cbXZtrAVf+Y6AU9YhNh/uOrMmk/mt4b U1d16Mbb5AZHDrNMZgvcSAHEx30xSt3iBATAmW8weoREbjot6/ZjCDsgR73jSrP01L8h4wQZa923i UV/tEoIQ7r/NgkBH024HHe80yj0hs51lFYDpRJNEaAeFnCXSt8MUcQeHVf6H+ixqkW7+E+iAhYxMp I1Siujqa1niQZ3LbHdvMPvHoUd+aZXRaDdqqOYAEN4sRmQOWE94OpTUOd4OfqAMG0DX+nrrvwSF0+ 1iMPPESg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rTODG-00000004Xcs-3ewv; Fri, 26 Jan 2024 15:33:50 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rTODC-00000004Xc5-3A68 for linux-arm-kernel@lists.infradead.org; Fri, 26 Jan 2024 15:33:48 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 955EA623DC; Fri, 26 Jan 2024 15:33:15 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C6E19C433C7; Fri, 26 Jan 2024 15:33:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1706283195; bh=mAzKWZzPkM6ug0onv0vRe/nRxVt4tmp8a5kHaDgKrWc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=M9d1S/owaLykjwZWoE/JjcSyfLO7y4RC6T8y6akAgrduNm62lUq45zOIYoRkDGeGQ bdV4MyfhzQ3kZ2yklUTH4mBzQkaA7EDoW7q65hvlDrJwnfW3wL7Oh/mvn3nWO8m2+5 RUSeikg8Npwu33rkmN3Hu9DeqalWWeet9m+tN4rQPIdyfHXaGd2QwyKz2JNdi4/Ia6 Y3vgbXw724n07b6UrC9832j8q8EOkse6XYAuGUXS7CtCoIfdNj9QrdBbnUexpMBbsD yjqmhSlc8gkmG99WflAqiOHbM51tbxHxPcEPGPsK3TeQ5aOGrgsQizpvqiMUvHOzcO XLAUrWVhHl+bg== Date: Fri, 26 Jan 2024 15:33:08 +0000 From: Conor Dooley To: Dharma.B@microchip.com Subject: Re: [PATCH v3 3/3] dt-bindings: mfd: atmel,hlcdc: Convert to DT schema format Message-ID: <20240126-uncommon-stout-dd3243e6b43f@spud> References: <20240118-recent-glorified-fd35d72e006e@spud> <20240119-character-mardi-43571d7fe7d5@wendy> <20240122-stark-duress-2f59294dcf27@spud> <4906b7e2-0ddb-4d3c-a48b-e16278f2d649@microchip.com> <20240124-lend-emerald-1028fe65cc39@spud> <20240125-proved-passage-7fa128f828db@wendy> <51da296d-a8a9-417a-8875-3b5e866a89a3@microchip.com> MIME-Version: 1.0 In-Reply-To: <51da296d-a8a9-417a-8875-3b5e866a89a3@microchip.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240126_073346_891821_F8364885 X-CRM114-Status: GOOD ( 37.88 ) 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: , Cc: alexandre.belloni@bootlin.com, linux-pwm@vger.kernel.org, Linux4Microchip@microchip.com, dri-devel@lists.freedesktop.org, Conor.Dooley@microchip.com, thierry.reding@gmail.com, krzysztof.kozlowski+dt@linaro.org, claudiu.beznea@tuxon.dev, airlied@gmail.com, sam@ravnborg.org, lee@kernel.org, u.kleine-koenig@pengutronix.de, devicetree@vger.kernel.org, conor+dt@kernel.org, tzimmermann@suse.de, maarten.lankhorst@linux.intel.com, mripard@kernel.org, robh+dt@kernel.org, linux-arm-kernel@lists.infradead.org, bbrezillon@kernel.org, linux-kernel@vger.kernel.org, daniel@ffwll.ch Content-Type: multipart/mixed; boundary="===============4645476841650585447==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============4645476841650585447== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="N7HfhWSgZScgqIKp" Content-Disposition: inline --N7HfhWSgZScgqIKp Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jan 26, 2024 at 02:22:42PM +0000, Dharma.B@microchip.com wrote: > On 25/01/24 1:57 pm, Conor Dooley - M52691 wrote: > >=20 > >>> If the lvds pll is an input to the hlcdc, you need to add it here. > >>> From your description earlier it does sound like it is an input to > >>> the hlcdc, but now you are claiming that it is not. > >> > >> The LVDS PLL serves as an input to both the LCDC and LVDSC > >=20 > > Then it should be an input to both the LCDC and LVDSC in the devicetree. >=20 > For the LVDSC to operate, the presence of the LVDS PLL is crucial. Howeve= r, in the case of the LCDC, LVDS PLL is not essential for its operation unl= ess LVDS interface is used and when it is used lvds driver will take care o= f preparing and enabling the LVDS PLL. Please fix your line wrapping, not sure what's going on here, but these lines are super long. > Consequently, it seems that there might not be any significant actions we= can take within the LCD driver regarding the LVDS PLL. You should be getting a reference to the clock and calling enable on it etc, even if the LVDSC is also doing so. That will allow the clock framework to correctly track users. > If there are no intentions to utilize it within the driver, is it necessa= ry to explicitly designate it as an input in the device tree? The binding describes the hardware, so yes it should be there. What the driver implementation does with the clock is not relevant. That said, I think the driver should actually be using it, as I wrote above. >=20 > If yes, I will update the bindings with optional LVDS PLL clock. >=20 > clock-names: > items: > - const: periph_clk > - const: sys_clk > - const: slow_clk > - const: lvds_pll # Optional clock This looks correct, but the comment is not needed. Setting minItems: 3 does this for you. > >> with the > >> LVDS_PLL multiplied by 7 for the Pixel clock to the LVDS PHY, and > >=20 > > Are you sure? The diagram doesn't show a multiplier, the 7x comment > > there seems to be showing relations? >=20 > Sorry,=20 > LVDS PLL =3D (PCK * 7) goes to LVDSC PHY > PCK =3D (LVDS PLL / 7) goes to LCDC I'll take your word for it :) > >> LVDS_PLL divided by 7 for the Pixel clock to the LCDC. > >=20 > >> I am inclined to believe that appropriately configuring and enabling it > >> in the LVDS driver would be the appropriate course of action. > >=20 > > We're talking about bindings here, not drivers, but I would imagine that > > if two peripherals are using the same clock then both of them should be > > getting a reference to and enabling that clock so that the clock > > framework can correctly track the users. > >=20 > >>> I don't know your hardware, so I have no idea which of the two is > >>> correct, but it sounds like the former. Without digging into how this > >>> works my assumption about the hardware here looks like is that the lv= ds > >>> controller is a clock provider, > >> > >> It's a PLL clock from PMC. > >> > >>> and that the lvds controller's clock is > >>> an optional input for the hlcdc. > >> > >> Again it's a PLL clock from PMC. > >> > >> Please refer Section 39.3 > >> https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/Produ= ctDocuments/DataSheets/SAM9X7-Series-Data-Sheet-DS60001813.pdf > >=20 > > It is not the same exact clock as you pointed out above though, so the > > by 7 divider should be modelled. >=20 > Modelled in mfd binding? If possible, could you please provide an example= for better clarity? Thank you. Whatever node corresponds to the register range controlling this PLL should be a "clock-controller" (like any other clock provider does). Your PMC should have this property. I don't know if the correct location is the mfd node or somewhere else, you'll have to check your docs. Thanks, Conor. --N7HfhWSgZScgqIKp Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZbPQtAAKCRB4tDGHoIJi 0hAfAP9oD7w7XXefbTe7aCamQ784UR9nGzozBzN9AoLVCrxKGAD+JD5kiMlu70l/ 6YBAmgN41j1kRbKlAUFnrV4Y2INzNgU= =MfAt -----END PGP SIGNATURE----- --N7HfhWSgZScgqIKp-- --===============4645476841650585447== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============4645476841650585447==--