From mboxrd@z Thu Jan  1 00:00:00 1970
From: Eric Nelson <eric.nelson@boundarydevices.com>
Date: Fri, 17 Apr 2015 08:53:10 -0700
Subject: [U-Boot] [PATCH] video,
 ipu: make ldb clock frequenz overwriteable through board code
In-Reply-To: <5530BC0B.6050301@denx.de>
References: <1428826797-576-1-git-send-email-hs@denx.de>
 <552F020C.6030503@boundarydevices.com> <5530BC0B.6050301@denx.de>
Message-ID: <55312C66.8060705@boundarydevices.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Heiko,

On 04/17/2015 12:53 AM, Heiko Schocher wrote:

>>> +int ipu_set_ldb_clock(int rate);
>>>   #endif
>>> diff --git a/drivers/video/ipu_common.c b/drivers/video/ipu_common.c
>>> index 1a209d4..dc054bc 100644
>>> --- a/drivers/video/ipu_common.c
>>> +++ b/drivers/video/ipu_common.c
>>> @@ -1198,3 +1198,14 @@ ipu_color_space_t
>>> format_to_colorspace(uint32_t fmt)
>>>       }
>>>       return RGB;
>>>   }
>>> +
>>> +/* should removed when clk framework is availiable */
>>> +int ipu_set_ldb_clock(int rate)
>>> +{
>>
>> This forces a tight dependency on when this is called that
>> using ldb_clk directly would prevent.
> 
> Sorry, did not understand you here ...
> 

The variable g_ldb is initialized in ipu_probe(), so this routine
must be called at a very particular time.

You must have noticed it, since you put in a check for NULL.

>>> +    if (g_ldb_clk == NULL)
>>> +        return -ENOENT;
>>> +

If you replace this with ldb_clk.rate, the dependency goes
away.

>>> +    g_ldb_clk->rate = rate;
>>> +
>>> +    return 0;
>>> +}
>>>

Separately, I don't see any reason not to have g_ldb_clk
statically initialized... i.e.

struct clk *g_ldb_clk = &ldb_clk;