netdev - Re: [PATCH] PA Semi PWRficient Ethernet driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <45BEAE9E.4000802@pobox.com>
Date:	Mon, 29 Jan 2007 21:34:06 -0500
From:	Jeff Garzik <jgarzik@...ox.com>
To:	Olof Johansson <olof@...om.net>
CC:	Stephen Hemminger <shemminger@...ux-foundation.org>,
	netdev@...r.kernel.org
Subject: Re: [PATCH] PA Semi PWRficient Ethernet driver

Olof Johansson wrote:
> On Mon, Jan 29, 2007 at 10:22:33AM -0800, Stephen Hemminger wrote:
>> Basic initalization, setup comments.
> 
> Thanks, fixes have been incorporated and will be reposted. Most of them
> was obviously just my lack of diligence. See however the two below.
> 
>>> +static irqreturn_t pasemi_mac_tx_intr(int irq, void *data)
>>> +{
>>> +	struct net_device *dev = data;
>>> +	struct pasemi_mac *mac = netdev_priv(dev);
>>> +	unsigned int reg;
>>> +
>>> +	pasemi_mac_clean_tx(mac);
>>> +
>>> +	reg = PAS_IOB_DMA_TXCH_RESET_PINTC | PAS_IOB_DMA_TXCH_RESET_SINTC;
>>> +	if (*mac->tx_status & PAS_STATUS_TIMER)
>>> +		reg |= PAS_IOB_DMA_TXCH_RESET_TINTC;
>>> +
>>> +	pci_write_config_dword(mac->iob_pdev, PAS_IOB_DMA_TXCH_RESET(mac->dma_txch),
>>> +			       reg);
>>> +
>>> +	return IRQ_HANDLED;
>>> +}
>> To do shared IRQ's properly you need to check to see if
>> this is your device IRQ or not. Maybe reading config value?
> 
> Right now it's guaranteed that the interrupts will not be shared. They're
> fixed for the on-chip devices, and no other driver should be binding to
> the same channels (and thus irqs).
> 
> If it changes in the future, the driver would need other rework as well.

Nonetheless, it is far more sane to check for work, and return if no 
work.  Who knows if the hardware will signal an interrupt early or late.


>>> +static struct pci_driver pasemi_mac_driver = {
>>> +   .name = "pasemi_mac",
>>> +   .id_table = pasemi_mac_pci_tbl,
>>> +   .probe = pasemi_mac_probe,
>> Don't you need a remove routine?
> 
> No hotplug support at this time, so I didn't see any use in providing one.

module remove.

	Jeff



-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html