Grant Grundler wrote:
On Jan 25, 2008 8:43 AM, Ke Wei <kewei.mv@xxxxxxxxx> wrote:
The attached is Marvell 6440 SAS/SATA driver. I will try to send email
by git-send-email.
I know this isn't part of this patch:
#define mr32(reg) readl(regs + MVS_##reg)
#define mw32(reg,val) writel((val), regs + MVS_##reg)
But can "regs" be declared a parameter to the macro?
This is a common technique in drivers (especially net drivers),
eliminating the redundant-across-the-entire-driver argument passed to
each register read or write. The result is infinitely more readable and
compact.
val = mr32(IRQ_STAT);
immediately communicates all the necessary information you need.
+ MODE_AUTO_DET_PORT7 = (1U << 15), /* port0 SAS/SATA autodetect */
+ MODE_AUTO_DET_PORT6 = (1U << 14),
+ MODE_AUTO_DET_PORT5 = (1U << 13),
+ MODE_AUTO_DET_PORT4 = (1U << 12),
+ MODE_AUTO_DET_PORT3 = (1U << 11),
+ MODE_AUTO_DET_PORT2 = (1U << 10),
+ MODE_AUTO_DET_PORT1 = (1U << 9),
+ MODE_AUTO_DET_PORT0 = (1U << 8),
These really aren't needed.
Like James noted, without public docs, we don't want to be removing any
hardware definitions.
Have to stop for now...but I'm wonder how this driver ever worked
given the number of HW register bits that were changed (assuming
they were wrong before this patch). And this driver looks _alot_
better than the previous Marvell drivers I've looked at. Please keep
up the good work! :)
Before this patch, the driver did not work. As I do with all other
drivers I write, I write the entire driver from the datasheet before
testing anything.
Jeff
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
