Re: [PATCH 1/8] sata_mv more cosmetics

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

 



On Fri, Apr 25, 2008 at 6:46 AM, Mark Lord <liml@xxxxxx> wrote:
...
>  But there are just *so many* irq cause / mask registers in these chips,
>  (one for PCI, one for the host function, and one per-port for EDMA,
>  plus the SATA SError + mask, ..), that even I was getting confused
>  while working on the code.

Yeah, it's confusing because most of the bits from one "interrupt"
(quotes explained in the next paragraph) register are rolled up into one bit
and available in another register. It seems like someone thought it
would be more efficient to read one register for all possible "causes"
and then have evidence a second register read is really necessary
(because a bit was set). Unfortunately, the result was the default
programming model as suggested by Marvell asks for at least two
MMIO reads just to find out if any IO completions are pending. 'Nuf said.
Kudos to Mark for figuring out how to reduce that to one MMIO read.

WRT "Interrupt", only the main cause (register can actually
generate interrupts. All the other "interrupt" registers
are effectively status registers. If a bit gets set in the main cause
register and it's not masked, an interrupt will get generated
(subject to any coalescing rules that have been enabled).
This is important to know for MSI support where we are dealing
with exclusive edge triggered interrupts instead of PCI shared
level triggered interrupts.

>  So, for now, they've been renamed back to something closer to
>  what is used in the (NDA only) documentation.
>
>  I did consider main_irq_{cause,mask} as an even better set of names there,
>  but in the end went for the shortened versions to avoid hitting the
>  pedantic 80-character line "limits" too often.

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.

hth,
grant
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux