lkml.org 
[lkml]   [2020]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/2] drm/bridge: Add tc358768 driver
From
Date
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

\
 
 \ /
  Last update: 2020-01-22 10:16    [W:0.151 / U:0.084 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site