Dear Stefano Babic,
In message 4D380C25.20901@denx.de you wrote:
- if (readl(&ccm->pdr3) & MXC_CCM_PDR3_UART_M_U)
freq = get_mcu_main_clk();- else
freq = decode_pll(readl(&ccm->ppctl),CONFIG_MX35_HCLK_FREQ);Braces needed
checkpatch (and generally accepted in linux, as I can see) requires that single statements must not be surrounded by braces. checkpatch returns a warning, explaining that braces are not needed.
I see in the past some comments requiring to remove braces, but it you prefer to add them. IMHO it is better to follow the same codestyle as linux, using the same tools as checkpatch. I do not know why we have two different results from checkpatch, I try to investigate. I had prefer to
I was running an older version of checkpatch...
- case USB_CLK:
usb_prdf = (reg4 >> 25) & 0x7;usb_podf = (reg4 >> 22) & 0x7;if (reg4 & 0x200)pll = get_mcu_main_clk();elsepll = decode_pll(readl(&ccm->ppctl),CONFIG_MX35_HCLK_FREQ);Ditto. Please fix globally.
See my previous comment. I would prefer to not have a different rule in u-boot, and not go against some provided tool (we use both checkpatch) if not strictly required.
Did you try it?
For me, both
+ if (reg4 & 0x200) + pll = get_mcu_main_clk(); + else + pll = decode_pll(readl(&ccm->ppctl), + CONFIG_MX35_HCLK_FREQ);
and
+ if (reg4 & 0x200) { + pll = get_mcu_main_clk(); + } else { + pll = decode_pll(readl(&ccm->ppctl), + CONFIG_MX35_HCLK_FREQ); + }
generate _NO_ warnings with checkpatch.
I feel that when the "single statement" is split across several lines, eventually even including blank lines (see yesterday's discussion here), then braces are needed.
Indeed they should. Why don't you autogenerate these, then?
We have all the tools in place, use them.
I will see how to use them.
See tools/scripts/make-asm-offsets
Note: the following remark is a question, NOT a change request:
Would it not be possible to reduce all these terrible lists? As far as I can tell, the list is built sequentially, with both arguments to _MXC_BUILD_NON_GPIO_PIN() being incremented by 4 for the next register. This begs for automatic code generation, doesn't it?
I do not know if it helps. The list follows exactly the description in user manual, and, if you can see a rule for MX35_PIN_A*, it is not so simply to find one for other pins, specially for the MXC_BUILD_GPIO_PIN. At least, the list is at the moment coherent for all i.MX processors (ok, ugly for all). The name of the pin cannot be generated, and it is the name found in manual. Miore as generated, the list is sorted....
OK, thanks for the explanation.
Best regards,
Wolfgang Denk