Hi Tomi,
On Mon, Feb 28, 2011 at 2:40 PM, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote:
<snip>
>> + oc = oh->opt_clks;
>> + oc_count = oh->opt_clks_cnt;
>> +
>> + /* copy roles of opt_clocks available from hwmod into pdata */
>> + pdata.dss_opt_clk_roles = kzalloc(sizeof(char *) * oc_count,
>> + GFP_KERNEL);
>> + if (!pdata.dss_opt_clk_roles) {
>> + pr_err("could not allocate memory for dss_opt_clk_roles\n");
>> + return -ENOMEM;
>> + }
>> +
>> + for (i = 0; i < oc_count; i++) {
>> + int oc_length = strlen(oc[i].role);
>> +
>> + pdata.dss_opt_clk_roles[i] = kzalloc(oc_length + 1,
>> + GFP_KERNEL);
>> + if (!pdata.dss_opt_clk_roles[i]) {
>> + int j = i - 1;
>> +
>> + pr_err("kzalloc failed for dss_opt_clk[%i]\n", i);
>> + for (; j >= 0; j--)
>> + kfree(pdata.dss_opt_clk_roles[j]);
>> +
>> + kfree(pdata.dss_opt_clk_roles);
>> +
>> + return -ENOMEM;
>> + }
>> +
>> + strncpy(pdata.dss_opt_clk_roles[i], oc[i].role, oc_length+1);
>> + }
>
> Do you actually need to copy the role strings? Can they every change? If
> they are const data, you could just point to the original strings.
Right; no, I don't think they can change, so I could point to the
original strings.
>
> And is there even need to create clk_roles array. If I'm not mistaken,
> the only use for this data is for the opt_clock_available() function. We
> could just have an opt_clock_available() function pointer in the
> omap_display_platform_data, and the function itself would be in
> display.c. And the function could use the original hwmod data.
Ok, but in that case, we do the same hwmod lookup everytime, is it?
That may not be so bad though, considering we check that very rarely
[in dsshw_probe=>dss_get_clocks() path].
Ok, will do and send the updated one soon.
~Sumit.
>
> Tomi
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
