Dear Stefano Babic,
In message <4d380c25.20...@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();
> >> + else
> >> + pll = 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
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
"You're just jealous." "What, of an overgrown puppy with a single-
figure IQ?" - Terry Pratchett, _Moving Pictures_
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
