[U-Boot] [PATCH 01/50] dm: clk: Add support for decoding clocks from the device tree

Simon Glass
2016-01-15T13:21:57Z

Hi Masahiro,

On 14 January 2016 at 03:11, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Hi Simon,
>
>
>
>> @@ -12,6 +12,8 @@
>> #include <dm/lists.h>
>> #include <dm/root.h>
>>
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> ulong clk_get_rate(struct udevice *dev)
>> {
>> struct clk_ops *ops = clk_get_ops(dev);
>> @@ -62,6 +64,32 @@ int clk_get_id(struct udevice *dev, int args_count, uint32_t *args)
>> return ops->get_id(dev, args_count, args);
>> }
>>
>> +int clk_get_by_index(struct udevice *dev, int index, struct udevice **clk_devp,
>> + int *periphp)
>
>
> This function causes NULL pointer access
> if called with clk_devp == NULL.

Yes, don't do that. Do you think the function comment is unclear?

>
>
> You can decrease the number of arguments
> if this function returns periph ID.

Right, but how to handle 0?

>
>
>> +{
>> + struct fdtdec_phandle_args args;
>> + int ret;
>> +
>> + ret = fdtdec_parse_phandle_with_args(gd->fdt_blob, dev->of_offset,
>> + "clocks", "#clock-cells", 0, index,
>> + &args);
>> + if (ret) {
>> + debug("%s: fdtdec_parse_phandle_with_args failed: err=%d\n",
>> + __func__, ret);
>> + return ret;
>> + }
>> +
>> + ret = uclass_get_device_by_of_offset(UCLASS_CLK, args.node, clk_devp);
>> + if (ret) {
>> + debug("%s: uclass_get_device_by_of_offset failed: err=%d\n",
>> + __func__, ret);
>> + return ret;
>> + }
>> + *periphp = args.args_count > 0 ? args.args[0] : -1;
>
>
> Do you want to let this function fail against #clock-cells == 0?

At present it doesn't. That's why I didn't have it return the
peripheral ID. What could we return to signify that the function
succeeded but there was no peripheral ID?

>
>
>
> This code should be compiled only when OF_CONTROL is on.

OK, I'll update it once we sort out the above.

Regards,
Simon