linux-kernel - Re: [PATCH v6 2/3] drm/bridge: ti-sn65dsi83: Add ti,lvds-vod-swing optional properties

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <45f90007-3136-4b59-a3fc-6fc0147b8ad8@wanadoo.fr>
Date: Thu, 12 Dec 2024 13:33:32 +0100
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Andrej Picej <andrej.picej@...ik.com>
Cc: dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, imx@...ts.linux.dev,
 linux-arm-kernel@...ts.infradead.org, upstream@...ts.phytec.de,
 andrzej.hajda@...el.com, neil.armstrong@...aro.org, rfoss@...nel.org,
 Laurent.pinchart@...asonboard.com, jonas@...boo.se,
 jernej.skrabec@...il.com, airlied@...il.com, simona@...ll.ch,
 maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de,
 robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 shawnguo@...nel.org, s.hauer@...gutronix.de, kernel@...gutronix.de,
 festevam@...il.com, marex@...x.de
Subject: Re: [PATCH v6 2/3] drm/bridge: ti-sn65dsi83: Add ti,lvds-vod-swing
 optional properties

Le 12/12/2024 à 13:17, Andrej Picej a écrit :
> Add a optional properties to change LVDS output voltage. This should not
> be static as this depends mainly on the connected display voltage
> requirement. We have three properties:
> - "ti,lvds-termination-ohms", which sets near end termination,
> - "ti,lvds-vod-swing-data-microvolt" and
> - "ti,lvds-vod-swing-clock-microvolt" which both set LVDS differential
> output voltage for data and clock lanes. They are defined as an array
> with min and max values. The appropriate bitfield will be set if
> selected constraints can be met.
> 
> If "ti,lvds-termination-ohms" is not defined the default of 200 Ohm near
> end termination will be used. Selecting only one:
> "ti,lvds-vod-swing-data-microvolt" or
> "ti,lvds-vod-swing-clock-microvolt" can be done, but the output voltage
> constraint for only data/clock lanes will be met. Setting both is
> recommended.

...

> +static int sn65dsi83_select_lvds_vod_swing(struct device *dev,
> +	u32 lvds_vod_swing_data[2], u32 lvds_vod_swing_clk[2], u8 lvds_term)
> +{
> +	int i;
> +
> +	for (i = 0; i <= 3; i++) {
> +		if (lvds_vod_swing_data_table[lvds_term][i][0] >= lvds_vod_swing_data[0] &&
> +		lvds_vod_swing_data_table[lvds_term][i][1] <= lvds_vod_swing_data[1] &&
> +		lvds_vod_swing_clock_table[lvds_term][i][0] >= lvds_vod_swing_clk[0] &&
> +		lvds_vod_swing_clock_table[lvds_term][i][1] <= lvds_vod_swing_clk[1])

Adding a few spaces to align things would help reading.

> +			return i;
> +	}
> +
> +	dev_err(dev, "failed to find appropriate LVDS_VOD_SWING configuration\n");
> +	return -EINVAL;
> +}
> +
> +static int sn65dsi83_parse_lvds_endpoint(struct sn65dsi83 *ctx, int channel)
> +{
> +	struct device *dev = ctx->dev;
> +	struct device_node *endpoint;
> +	int endpoint_reg;
> +	/* Set so the property can be freely selected if not defined */
> +	u32 lvds_vod_swing_data[2] = { 0, 1000000 };
> +	u32 lvds_vod_swing_clk[2] = { 0, 1000000 };
> +	/* Set default near end terminataion to 200 Ohm */
> +	u32 lvds_term = 200;
> +	int lvds_vod_swing_conf;
> +	int ret = 0;
> +	int ret_data;
> +	int ret_clock;
> +
> +	if (channel == CHANNEL_A)
> +		endpoint_reg = 2;
> +	else
> +		endpoint_reg = 3;
> +
> +	endpoint = of_graph_get_endpoint_by_regs(dev->of_node, endpoint_reg, -1);
> +
> +	of_property_read_u32(endpoint, "ti,lvds-termination-ohms", &lvds_term);
> +	if (lvds_term == 100)
> +		ctx->lvds_term_conf[channel] = OHM_100;
> +	else if (lvds_term == 200)
> +		ctx->lvds_term_conf[channel] = OHM_200;
> +	else
> +		return -EINVAL;

Should it be:
	else {
		ret = -EINVAL;
		goto exit;
	}
?

> +
> +	ret_data = of_property_read_u32_array(endpoint, "ti,lvds-vod-swing-data-microvolt",
> +					lvds_vod_swing_data, ARRAY_SIZE(lvds_vod_swing_data));
> +	if (ret_data != 0 && ret_data != -EINVAL) {
> +		ret = ret_data;
> +		goto exit;
> +	}
> +
> +	ret_clock = of_property_read_u32_array(endpoint, "ti,lvds-vod-swing-clock-microvolt",
> +					lvds_vod_swing_clk, ARRAY_SIZE(lvds_vod_swing_clk));
> +	if (ret_clock != 0 && ret_clock != -EINVAL) {
> +		ret = ret_clock;
> +		goto exit;
> +	}
> +
> +	/* Use default value if both properties are NOT defined. */
> +	if (ret_data == -EINVAL && ret_clock == -EINVAL)
> +		lvds_vod_swing_conf = 0x1;
> +
> +	/* Use lookup table if any of the two properties is defined. */
> +	if (!ret_data || !ret_clock) {
> +		lvds_vod_swing_conf = sn65dsi83_select_lvds_vod_swing(dev, lvds_vod_swing_data,
> +						lvds_vod_swing_clk, ctx->lvds_term_conf[channel]);
> +		if (lvds_vod_swing_conf < 0) {
> +			ret = lvds_vod_swing_conf;
> +			goto exit;
> +		}
> +	}
> +
> +	ctx->lvds_vod_swing_conf[channel] = lvds_vod_swing_conf;
> +	ret = 0;
> +exit:
> +	of_node_put(endpoint);
> +	return ret;
> +}

...

CJ