[U-Boot] [PATCH v4 02/12] omap4: cleanup pin mux data

Wolfgang Denk
2011-07-29T08:56:13Z

Dear Aneesh V,

In message <4E32722C.6020707@ti.com> you wrote:
> Dear Wolfgang,
>
> On Friday 29 July 2011 12:56 AM, Wolfgang Denk wrote:
> > Dear Aneesh V,
> >
> > In message<1311233298-17265-3-git-send-email-aneesh@ti.com> you wrote:
> >> - separate mux settings into essential and non essential parts
> >> - essential part is board independent as of now(so move it
> >> to SoC directory). Will help in having single SPL for all
> >> boards.
> >> - Non-essential part(the pins not essential for u-boot to function)
> >> need to be phased out eventually.
> >> - Correct mux data by aligning to the latest settings in x-loader
> > ...
> >> + {USBB1_ULPITLL_CLK, (IEN | OFF_EN | OFF_IN | M1)}, /* hsi1_cawake */
> >> + {USBB1_ULPITLL_STP, (IEN | OFF_EN | OFF_IN | M1)}, /* hsi1_cadata */
> >> + {USBB1_ULPITLL_DIR, (IEN | OFF_EN | OFF_IN | M1)}, /* hsi1_caflag */
> >> + {USBB1_ULPITLL_NXT, (OFF_EN | M1)}, /* hsi1_acready */
> >> + {USBB1_ULPITLL_DAT0, (OFF_EN | M1)}, /* hsi1_acwake */
> >> + {USBB1_ULPITLL_DAT1, (OFF_EN | M1)}, /* hsi1_acdata */
> >> + {USBB1_ULPITLL_DAT2, (OFF_EN | M1)}, /* hsi1_acflag */
> >> + {USBB1_ULPITLL_DAT3, (IEN | OFF_EN | OFF_IN | M1)}, /* hsi1_caready */
> >
> > Lines too long, please fix globally.
>
> This table looks better and readable if each row is a single line. I
> had mentioned this in the commit log. Does it make sense to make an
> exception for this?

I agree that it does not make sense to split the lines.

But it seems there is potential to reduce the line length: in may
cases, you can just reduce the indentation of the comments.

In many other places the commets are just useless and should be
omitted. See for example the (old, existing) lines:

{CAM_SHUTTER, (OFF_EN | OFF_PD | OFF_OUT_PTD | M0)}, /* cam_shutter */
{CAM_STROBE, (OFF_EN | OFF_PD | OFF_OUT_PTD | M0)}, /* cam_strobe */
...
{USBB1_ULPITLL_DAT4, (IEN | OFF_EN | OFF_PD | OFF_IN | M4)}, /* usbb1_ulpiphy_dat4 */
{USBB1_ULPITLL_DAT5, (IEN | OFF_EN | OFF_PD | OFF_IN | M4)}, /* usbb1_ulpiphy_dat5 */
{USBB1_ULPITLL_DAT6, (IEN | OFF_EN | OFF_PD | OFF_IN | M4)}, /* usbb1_ulpiphy_dat6 */
...
etc.

A comment that just repeats the content of the first argument without
any additional information carries zero information and should just
be removed.

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: wd at denx.de
If you use modules, you pay the price. Sane embedded solutions
running in "tight" environments don't use modules :-)
-- Benjamin Herrenschmidt in <1258234866.2140.451.camel@pasglop>