Re: [PATCHv7 3/3] edac: altera: Add EDAC support for Altera SoC SDRAM Controller.

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

 




On Tue, Jul 08, 2014 at 01:31:09PM +0200, Pavel Machek wrote:
> > +	read_reg = readl(mc_vbase + DRAMADDRW);
> > +
> > +	width = readl(mc_vbase + DRAMIFWIDTH);
> > +
> > +	col = (read_reg & DRAMADDRW_COLBIT_MASK) >>
> > +		DRAMADDRW_COLBIT_LSB;
> > +	row = (read_reg & DRAMADDRW_ROWBIT_MASK) >>
> > +		DRAMADDRW_ROWBIT_LSB;
> > +	bank = (read_reg & DRAMADDRW_BANKBIT_MASK) >>
> > +		DRAMADDRW_BANKBIT_LSB;
> > +	cs = (read_reg & DRAMADDRW_CSBIT_MASK) >>
> > +		DRAMADDRW_CSBIT_LSB;
> 
> As I said, all the defines only make this harder to read. The code is
> pretty obvious if you put numbers in here...

Since when it is a good coding practice to put naked numbers instead of
descriptive macro names??!

Now, I can understand that the macro names could be made more
descriptive so that you can understand what they mean, but naked
numbers?! You must be joking although 1st of April is long gone.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux