Hi Andrzej,
On 22/01/2020 10.46, Andrzej Hajda wrote:
+struct tc358768_priv {
- struct device *dev;
- struct regmap *regmap;
- struct gpio_desc *reset_gpio;
- struct regulator_bulk_data supplies[ARRAY_SIZE(tc358768_supplies)];
- struct clk *refclk;
- struct mipi_dsi_host dsi_host;
- struct drm_bridge bridge;
- struct tc358768_dsi_output output;
Since tc358768_dsi_output is used only here, you can define it here as well, up to you.
I think I have done it like this to avoid thinking about prefixes for what is under the tc358768_dsi_output. I'll take a look if it will look better unpacked here.
I though rather about in-place anonymous struct definition:
+ struct tc358768_dsi_output { + struct mipi_dsi_device *dev; + struct drm_panel *panel; + struct drm_bridge *bridge; + } output;
But, as I said - up to you.
I see. I think I will keep how it was. They are in proximity, so easy to check.
- refclk = clk_get_rate(priv->refclk);
- best_diff = UINT_MAX;
- best_pll = 0;
- best_prd = 0;
- best_fbd = 0;
- for (prd = 0; prd < 16; ++prd) {
u32 divisor = (prd + 1) * (1 << frs);u32 fbd;for (fbd = 0; fbd < 512; ++fbd) {u32 pll, diff;pll = (u32)div_u64((u64)refclk * (fbd + 1), divisor);if (pll >= max_pll || pll < min_pll)continue;diff = max(pll, target_pll) - min(pll, target_pll);if (diff < best_diff) {best_diff = diff;best_pll = pll;best_prd = prd;best_fbd = fbd;}if (best_diff == 0)break;}if (best_diff == 0)break;why another check here?
To break out from the top for() loop also in case exact match has been found.
Ahh, OK. So maybe you should put "if (diff == 0) goto found" inside "if (diff < best_diff)" block, in such case goto is not considered harmful :), and is more readable.
Exactly my thoughts ;)
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki