[U-Boot] [PATCH 12/35] arm: socfpga: clock: Implant order into bit definitions

Marek Vasut marex at denx.de
Tue Sep 16 23:59:31 CEST 2014


On Tuesday, September 16, 2014 at 10:18:32 AM, Wolfgang Denk wrote:
> Dear Marek,
> 
> In message <201409152348.15411.marex at denx.de> you wrote:
> > >         cm_write_bypass(
> > >         
> > >                 CLKMGR_BYPASS_PERPLLSRC_SET(
> > >                 CLKMGR_BYPASS_PERPLLSRC_SELECT_EOSC1) |
> > >                 CLKMGR_BYPASS_SDRPLLSRC_SET(
> > >                 CLKMGR_BYPASS_SDRPLLSRC_SELECT_EOSC1) |
> > > 		
> > > 		CLKMGR_BYPASS_PERPLL_SET(CLKMGR_BYPASS_ENABLE)
> > > 		
> > >                 CLKMGR_BYPASS_SDRPLL_SET(CLKMGR_BYPASS_ENABLE) |
> > >                 CLKMGR_BYPASS_MAINPLL_SET(CLKMGR_BYPASS_ENABLE));
> 
> ...
> 
> > > But yes, there are more cleanups that could be done there.
> > 
> > You surely wanted to say "must be done where" ;-) I've yet to decide if
> > I'll do it before this patchset or after ; I am more inclined doing it
> > as a subsequent patch to allow Altera guys review the changes and match
> > them with their existing code. And only then do the cleanup , probably
> > automatically using some script.
> > 
> > What do you think, Wolfgang, Pavel, Dinh, others ... ?
> 
> Well, frankly speaking, the code above (and many similar use cases)
> are an unreadable and unmaintainable mess.  I understand that parts of
> this are computer-generated definitions, but in my opinion there are
> several serious disadvantages:

Full ACK on this, the header files are full of dark chaos so far.

[...]

> So as far as I am concerned I do not intend to block this with a plain
> NAK, but I would be really happy if this whole clock manager code
> would be put on the list of things that need rework, and we don't
> forget this about more recent tasks coming.

Will be in V2.

Best regards,
Marek Vasut


More information about the U-Boot mailing list