Re: [PATCH 1/8] sata_mv more cosmetics

[Mark Lord <liml@rtr.ca>]

Jeff Garzik wrote:
> Grant Grundler wrote:
>> I think you can associate any names you want with whatever the
>> non-public documentation is calling the registers by adding a comment
>> in the header files where the bit masks and register offsets are defined.
>> Or vice versa. Which ever way works for you.
>>
>> I prefer to use the non-public register names in the code only because it
>> will be easier for Marvell engineers to participate in _maintaining_
>> this driver. I think getting involved with open source developement
>> is a good career developement experience. And the device drivers for
>> "obsolete" HW allows them to take more risks/make mistakes
>> without getting into serious trouble with the company.
> 
> 
> The trouble, though, comes with following that logic in every driver, 
> making the collective body less accessible to anyone not _intimately_ 
> tied into the hardware _and_ possessing NDA'd docs.
> 
> Further, it is obvious with _most_ drivers in Linux that engineers at 
> the hardware vendor do _not_ participate in maintaining drivers for 
> their older hardware, so I also wish to be careful and avoid punishing 
> the majority to help a minority.
> 
> I put a significant value in having more-readable code like
> 
>     status = mr32(IRQ_STAT);    /* read IRQ status from PPB */
> 
> rather than
> 
>     status = READ_REG(ICR5PPB);    /* read IRQ status from PPB */
> 
> because the casual reader is more likely to understand the first 
> example, even though it deviates from the string of line noise some 
> engineer writing Verilog at 4:00am decided was a good register name.
..

There's a patch out for this now -- just waiting for you to pick it up.
It uses "main_irq_cause/mask" now instead of merely "main_cause/mask".

This ought to be understandable at a glance to anyone,
even those of us with chipset docs.  :)

Cheers