Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1469067 > unrolled thread

Re: [Patch v8] driver/clk/clk-si5338: Add common clock framework driver for si5338

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [Patch v8] driver/clk/clk-si5338: Add common clock framework  driver for si5338 Stephen Boyd <sboyd@codeaurora.org> - 2016-08-24 06:50 +0200
    Re: [Patch v8] driver/clk/clk-si5338: Add common clock framework  driver for si5338 york sun <york.sun@nxp.com> - 2016-08-24 18:50 +0200

#1469067 — Re: [Patch v8] driver/clk/clk-si5338: Add common clock framework driver for si5338

On 08/24, kbuild test robot wrote:
> 
>   2827		if (drv_type < 0)
>   2828			return drv_type;
>   2829	
>   2830		drv_vdd =  get_drv_vdd(drvdata, i);
>   2831		if (drv_vdd < 0)
>   2832			return drv_vdd;
>   2833	
>   2834		drv_trim = get_drv_trim(drvdata, i);
>   2835		if (drv_trim < 0)
>   2836			return drv_trim;
>   2837	
>   2838		drv_invert = get_drv_invert(drvdata, i);
>   2839		if (drv_invert < 0)
>   2840			return drv_invert;
>   2841	
>   2842		for (j = 0; drv_configs[j].description; j++) {
> > 2843			if (drv_configs[j].fmt == drv_type &&
>   2844			    drv_configs[j].vdd == drv_vdd &&
>   2845			    drv_configs[j].trim == drv_trim &&
>   2846			    (drv_invert | (drv_configs[j].invert >> 2)) ==
>   2847				((drv_configs[j].invert & 3) |
>   2848				 (drv_configs[j].invert>>2)))

It might be better to write it as a set of continues instead so
that it isn't one complicated conditional

				config = &drv_configs[j];
				if (config->fmt != drv_type)
					continue;
				if (config->vdd != drv_vdd)
					continue;
				...
				seq_puts()
				match = 1;
				break;
				

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [next] | [standalone]

#1469573

On 08/23/2016 09:48 PM, Stephen Boyd wrote:
> On 08/24, kbuild test robot wrote:
>>
>>   2827		if (drv_type < 0)
>>   2828			return drv_type;
>>   2829	
>>   2830		drv_vdd =  get_drv_vdd(drvdata, i);
>>   2831		if (drv_vdd < 0)
>>   2832			return drv_vdd;
>>   2833	
>>   2834		drv_trim = get_drv_trim(drvdata, i);
>>   2835		if (drv_trim < 0)
>>   2836			return drv_trim;
>>   2837	
>>   2838		drv_invert = get_drv_invert(drvdata, i);
>>   2839		if (drv_invert < 0)
>>   2840			return drv_invert;
>>   2841	
>>   2842		for (j = 0; drv_configs[j].description; j++) {
>>> 2843			if (drv_configs[j].fmt == drv_type &&
>>   2844			    drv_configs[j].vdd == drv_vdd &&
>>   2845			    drv_configs[j].trim == drv_trim &&
>>   2846			    (drv_invert | (drv_configs[j].invert >> 2)) ==
>>   2847				((drv_configs[j].invert & 3) |
>>   2848				 (drv_configs[j].invert>>2)))
>
> It might be better to write it as a set of continues instead so
> that it isn't one complicated conditional
>
> 				config = &drv_configs[j];
> 				if (config->fmt != drv_type)
> 					continue;
> 				if (config->vdd != drv_vdd)
> 					continue;
> 				...
> 				seq_puts()
> 				match = 1;
> 				break;
> 				

Stephen,

Thanks for the suggestion. I will send an update.

York

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web