Re: [PATCH] pata_cmd640: don't read CFR pointlessly

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

 



Hello.

Jeff Garzik wrote:

cmd640_hardware_init() reads CFR but doesn't use the value read...

Signed-off-by: Sergei Shtylyov<sshtylyov@xxxxxxxxxxxxx>

---
The patch is against the recent Linus' tree.

  drivers/ata/pata_cmd640.c |    3 ---
  1 file changed, 3 deletions(-)

Index: linux-2.6/drivers/ata/pata_cmd640.c
===================================================================
--- linux-2.6.orig/drivers/ata/pata_cmd640.c
+++ linux-2.6/drivers/ata/pata_cmd640.c
@@ -181,13 +181,10 @@ static struct ata_port_operations cmd640

  static void cmd640_hardware_init(struct pci_dev *pdev)
  {
-    u8 r;
      u8 ctrl;

      /* CMD640 detected, commiserations */
      pci_write_config_byte(pdev, 0x5B, 0x00);
-    /* Get version info */
-    pci_read_config_byte(pdev, CFR,&r);
      /* PIO0 command cycles */

Agreed with this patch, but we lose some information: all that remains is an uncommented 'CFR' definition. Without the code comment, which is deleted in this patch, we have no idea what that register does.

  It does several different things.

Given that the chipset docs are not public AFAIK,

They were public once. And I think I've given you that spec along with other PCI-64x specs for publishing on gkernel.sourceforge.net/specs/ already (can't open sii/cmd64x-specs.tar.bz2 now to check).

this is a net loss of information about this chipset, IMO.

Maybe revise this patch, or add a second patch, which adds a comment where 'CFR' is defined? Something as simple as "/* version info */" would be sufficient.

That wouldn't be a good description. The "official" name of the register is configuration register.

    Jeff

WBR, Sergei

--
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