[U-Boot] [PATCH v4 01/11] serial: ns16550: Support clocks via phandle

[Paul Burton]

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);

Thanks,
     Paul