Received-SPF: pass (google.com: domain of linux-kernel+bounces-62003-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <61af7b121b23fe8ed06df5348692f862b9b125bf.camel@gmail.com>
Subject: Re: [PATCH v2 2/2] phy: rockchip: Add Samsung HDMI/eDP Combo PHY
 driver
From: Luis de Arquer <ldearquer@gmail.com>
To: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>, Rob Herring
 <robh+dt@kernel.org>, Krzysztof Kozlowski
 <krzysztof.kozlowski+dt@linaro.org>,  Conor Dooley <conor+dt@kernel.org>,
 Heiko Stuebner <heiko@sntech.de>, Johan Jonker <jbx6244@gmail.com>, 
 Sebastian Reichel <sebastian.reichel@collabora.com>, Sascha Hauer
 <s.hauer@pengutronix.de>, Andy Yan <andy.yan@rock-chips.com>, Vinod Koul
 <vkoul@kernel.org>, Kishon Vijay Abraham I <kishon@kernel.org>, Philipp
 Zabel <p.zabel@pengutronix.de>, Algea Cao <algea.cao@rock-chips.com>
Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, 
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org, 
	linux-phy@lists.infradead.org, kernel@collabora.com
Date: Mon, 12 Feb 2024 17:44:43 +0100
In-Reply-To: <20240205-phy-hdptx-v2-2-a7150814c047@collabora.com>
References: <20240205-phy-hdptx-v2-0-a7150814c047@collabora.com>
	 <20240205-phy-hdptx-v2-2-a7150814c047@collabora.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4-0ubuntu2 
Precedence: bulk
MIME-Version: 1.0

Hi Cristian,

On Mon, 2024-02-05 at 13:24 +0200, Cristian Ciocaltea wrote:
> +
> +static bool hdptx_phy_clk_pll_calc(unsigned int data_rate,
> +				   struct ropll_config *cfg)
> +{
> +	const unsigned int fout =3D data_rate / 2, fref =3D 24000;
> +	unsigned long k =3D 0, lc, k_sub, lc_sub;
> +	unsigned int fvco, sdc;
> +	u32 mdiv, sdiv, n =3D 8;
> +
> +	for (sdiv =3D 16; sdiv >=3D 1; sdiv--) {
> +		if (sdiv % 2 && sdiv !=3D 1)
> +			continue;
> +
> +		fvco =3D fout * sdiv;
> +
> +		if (fvco < 2000000 || fvco > 4000000)
> +			continue;
> +

What about adding a check to data_rate, maybe like

if (fout > 0x0FFFFFFF)
	return false;

or similar, before the for loop, to keep the multiplication safe?

Right now it would be redundant, given that data_rate was, at some
point, encoded in 28 bits within bus_width. But can prevent future pain,
especially after changing to phy_configure_opts_hdmi.

Luis