Re: [PATCH] Staging: bcm: DDRInit: moved #defines from DDRInit.c to DDRInit.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 19, 2014 at 10:29:42PM -0400, Gary Rookard wrote:
> 
> 
> On Thu, 20 Mar 2014, Greg KH wrote:
> 
> > On Wed, Mar 19, 2014 at 05:30:53PM -0400, Gary Rookard wrote:
> >> I moved the #defines from implementation file DDRInit.c to
> >> the proper specification file DDRInit.h.
> >>
> >> Signed-off-by: Gary Alan Rookard <garyrookard@xxxxxxxxx>
> >>
> >> ---
> >> On branch staging-next
> >>  drivers/staging/bcm/DDRInit.c | 15 ---------------
> >>  drivers/staging/bcm/DDRInit.h | 15 +++++++++++++++
> >>  2 files changed, 15 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/drivers/staging/bcm/DDRInit.c b/drivers/staging/bcm/DDRInit.c
> >> index f1d7cb8..60d300f 100644
> >> --- a/drivers/staging/bcm/DDRInit.c
> >> +++ b/drivers/staging/bcm/DDRInit.c
> >> @@ -2,11 +2,8 @@
> >>
> >>
> >>
> >> -#define DDR_DUMP_INTERNAL_DEVICE_MEMORY 0xBFC02B00
> >> -#define MIPS_CLOCK_REG 0x0f000820
> >>
> >>  /* DDR INIT-133Mhz */
> >> -#define T3_SKIP_CLOCK_PROGRAM_DUMP_133MHZ 12  /* index for 0x0F007000 */
> >>  static struct bcm_ddr_setting asT3_DDRSetting133MHz[] = {  /* DPLL Clock Setting */
> >
> > It seems to make mroe sense here, why put it in a .c file if it's only
> > being referenced in one place?
> >
> >> +#define DDR_DUMP_INTERNAL_DEVICE_MEMORY 0xBFC02B00
> >> +#define MIPS_CLOCK_REG 0x0f000820
> >>
> >> +#define T3_SKIP_CLOCK_PROGRAM_DUMP_133MHZ 12  /* index for 0x0F007000 */
> >> +#define T3_SKIP_CLOCK_PROGRAM_DUMP_80MHZ 10  /* index for 0x0F007000 */
> >> +#define T3_SKIP_CLOCK_PROGRAM_DUMP_100MHZ 13  /* index for 0x0F007000 */
> >> +#define T3B_SKIP_CLOCK_PROGRAM_DUMP_133MHZ 11  /* index for 0x0F007000 */
> >> +#define T3B_SKIP_CLOCK_PROGRAM_DUMP_80MHZ 9  /* index for 0x0F007000 */
> >> +#define T3B_SKIP_CLOCK_PROGRAM_DUMP_100MHZ 9  /* index for 0x0F007000 */
> >> +#define T3LP_SKIP_CLOCK_PROGRAM_DUMP_133MHZ 9  /* index for 0x0F007000 */
> >> +#define T3LP_SKIP_CLOCK_PROGRAM_DUMP_100MHZ 11  /* index for 0x0F007000 */
> >> +#define T3LP_SKIP_CLOCK_PROGRAM_DUMP_80MHZ 9  /* index for 0x0F007000 */
> >> +#define T3LPB_SKIP_CLOCK_PROGRAM_DUMP_160MHZ 7  /* index for 0x0F007000 */
> >> +#define T3LPB_SKIP_CLOCK_PROGRAM_DUMP_133MHZ 7  /* index for 0x0F007000 */
> >> +#define T3LPB_SKIP_CLOCK_PROGRAM_DUMP_100MHZ 8  /* index for 0x0F007000 */
> >> +#define T3LPB_SKIP_CLOCK_PROGRAM_DUMP_80MHZ 7  /* index for 0x0F007000 */
> >
> > That's not very "pretty", if you do this, it should at least align up
> > properly.
> >
> > thanks,
> >
> > greg k-h
> >
> 
> --
> Well, for reasons of uncluttering the code by putting
> things where they belong, it's a best practice. (only the lazy
> don't in my estimation.) 
> DDRInt.h already exists its not a new file creation.
> 
> Gots to keep them seprate :)

No, drivers don't need .h files, you should never have a .h file that is
only used by one .c file, that's pretty pointless in the kernel, just
mush it all together...

greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux