On 4.8.2016 11:59, Paul Burton wrote:
> On 04/08/16 10:50, Michal Simek wrote:
>>> @@ -352,6 +353,8 @@ int ns16550_serial_ofdata_to_platdata(struct
>>> udevice *dev)
>>> {
>>> struct ns16550_platdata *plat = dev->platdata;
>>> fdt_addr_t addr;
>>> + struct clk clk;
>>> + int err;
>>>
>>> /* try Processor Local Bus device first */
>>> addr = dev_get_addr(dev);
>>> @@ -397,9 +400,19 @@ int ns16550_serial_ofdata_to_platdata(struct
>>> udevice *dev)
>>> "reg-offset", 0);
>>> plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
>>> "reg-shift", 0);
>>> - plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
>>> - "clock-frequency",
>>> - CONFIG_SYS_NS16550_CLK);
>>> +
>>> + err = clk_get_by_index(dev, 0, &clk);
>>> + if (!err) {
>>> + plat->clock = clk_get_rate(&clk);
>>> + } else if (err != -ENODEV) {
>>> + debug("ns16550 failed to get clock\n");
>>> + return err;
>>> + }
>>> +
>>> + if (!plat->clock)
>>> + plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
>>> + "clock-frequency",
>>> + CONFIG_SYS_NS16550_CLK);
>>> if (!plat->clock) {
>>> debug("ns16550 clock not defined\n");
>>> return -EINVAL;
>>>
>>
>>
>> NACK. It is missing dependency on clk uclass which is not enabled by
>> default on Microblaze. Maybe also on others. You should add some ifs
>> around.
>>
>> Thanks,
>> Michal
>
> I'm not sure #ifdef'ing in this code is the nicest solution. How about
> if we allow -ENOSYS through the error handling above like -ENODEV, and
> provide the dummy clk_get_by_* implementations when CONFIG_CLK is
> disabled? As in:
>
> diff --git a/include/clk.h b/include/clk.h
> index 161bc28..328f364 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -59,7 +59,7 @@ struct clk {
> unsigned long id;
> };
>
> -#if CONFIG_IS_ENABLED(OF_CONTROL)
> +#if CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(CONFIG_CLK)
> struct phandle_2_cell;
> int clk_get_by_index_platdata(struct udevice *dev, int index,
> struct phandle_2_cell *cells, struct clk
> *clk);
No problem with it from my side.
Just to make sure that you can build it on other archs and there is no
big overhead.
Thanks,
Michal
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
