Received-SPF: pass (google.com: domain of linux-kernel+bounces-38176-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Thu, 25 Jan 2024 08:27:55 +0000
From: Conor Dooley <conor.dooley@microchip.com>
To: <Dharma.B@microchip.com>
CC: <conor@kernel.org>, <sam@ravnborg.org>, <bbrezillon@kernel.org>,
	<maarten.lankhorst@linux.intel.com>, <mripard@kernel.org>,
	<tzimmermann@suse.de>, <airlied@gmail.com>, <daniel@ffwll.ch>,
	<robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>,
	<conor+dt@kernel.org>, <Nicolas.Ferre@microchip.com>,
	<alexandre.belloni@bootlin.com>, <claudiu.beznea@tuxon.dev>,
	<dri-devel@lists.freedesktop.org>, <devicetree@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
	<lee@kernel.org>, <thierry.reding@gmail.com>,
	<u.kleine-koenig@pengutronix.de>, <linux-pwm@vger.kernel.org>,
	<Linux4Microchip@microchip.com>
Subject: Re: [PATCH v3 3/3] dt-bindings: mfd: atmel,hlcdc: Convert to DT
 schema format
Message-ID: <20240125-proved-passage-7fa128f828db@wendy>
References: <20240118092612.117491-1-dharma.b@microchip.com>
 <20240118092612.117491-4-dharma.b@microchip.com>
 <20240118-recent-glorified-fd35d72e006e@spud>
 <c33868c8-dc42-4800-885c-5e5f24c2044e@microchip.com>
 <20240119-character-mardi-43571d7fe7d5@wendy>
 <da60f9f3-f955-4a87-a020-5710185953c0@microchip.com>
 <20240122-stark-duress-2f59294dcf27@spud>
 <4906b7e2-0ddb-4d3c-a48b-e16278f2d649@microchip.com>
 <20240124-lend-emerald-1028fe65cc39@spud>
 <c3c30bf2-e7c2-4861-bfdf-519a7afde476@microchip.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="KV2dTBGvLz55uW11"
Content-Disposition: inline
In-Reply-To: <c3c30bf2-e7c2-4861-bfdf-519a7afde476@microchip.com>

--KV2dTBGvLz55uW11
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable


> > 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.
>=20
> The LVDS PLL serves as an input to both the LCDC and LVDSC

Then it should be an input to both the LCDC and LVDSC in the devicetree.

> with the=20
> 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?

> LVDS_PLL divided by 7 for the Pixel clock to the LCDC.

> I am inclined to believe that appropriately configuring and enabling it=
=20
> in the LVDS driver would be the appropriate course of action.

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.

> > 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 lvds
> > controller is a clock provider,
>=20
> It's a PLL clock from PMC.
>=20
> > and that the lvds controller's clock is
> > an optional input for the hlcdc.
>=20
> Again it's a PLL clock from PMC.
>=20
> Please refer Section 39.3=20
> https://ww1.microchip.com/downloads/aemDocuments/documents/MPU32/ProductD=
ocuments/DataSheets/SAM9X7-Series-Data-Sheet-DS60001813.pdf

It is not the same exact clock as you pointed out above though, so the
by 7 divider should be modelled.

> > Can you please explain what provides the lvds pll clock and show an
> > example of how you think the devictree would look with "the lvds pll in
> > the lvds dt node"?
>=20
> Sure, Please see the below example
>=20
> The typical lvds node will look like
>=20
>                  lvds_controller: lvds-controller@f8060000 {
>                          compatible =3D "microchip,sam9x7-lvds";
>                          reg =3D <0xf8060000 0x100>;
>                          interrupts =3D <56 IRQ_TYPE_LEVEL_HIGH 0>;
>                          clocks =3D <&pmc PMC_TYPE_PERIPHERAL 56>, <&pmc=
=20
> PMC_TYPE_CORE PMC_LVDSPLL>;
>                          clock-names =3D "pclk", "lvds_pll_clk";
>                          status =3D "disabled";
>                  };

In isolation, this looks fine.

Cheers,
Conor.

--KV2dTBGvLz55uW11
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZbIbiwAKCRB4tDGHoIJi
0h+8AQCpYI71+drl2FDrbEGThmMOFS/iz1jS+CPJczvYePVUqwD/ZilClwywAcjj
BSSdZJ4KLwpHGZIe4dxZJbifZWhXPAY=
=rMTy
-----END PGP SIGNATURE-----

--KV2dTBGvLz55uW11--