Re: [PATCH] ide/libata: fix ata_id_is_cfa()

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

 



On Sunday 25 January 2009, Sergei Shtylyov wrote:
> Hello, I wrote:
> > When checking for CFA feature set support, ata_id_is_cfa() tests bit 2 in word
> > 82 of the identify data instead the word 83.  It also checks the ATA revision
> > in the  word 80 instead of usual validity check for the words 82-83 -- word 83
> > bit 14 set, bit 15 cleared.
> >
> > Signed-off-by: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>
> >
> > ---
> > I'm not sure who should queue this patch... it's against Linus' tree.
> >
> >  include/linux/ata.h |    5 ++---
> >  1 files changed, 2 insertions(+), 3 deletions(-)
> >
> > Index: linux-2.6/include/linux/ata.h
> > ===================================================================
> > --- linux-2.6.orig/include/linux/ata.h
> > +++ linux-2.6/include/linux/ata.h
> > @@ -734,9 +734,8 @@ static inline int ata_id_is_cfa(const u1
> >  	if (id[ATA_ID_CONFIG] == 0x848A)	/* Standard CF */
> >  		return 1;
> >  	/* Could be CF hiding as standard ATA */
> > -	if (ata_id_major_version(id) >= 3 &&
> > -	    id[ATA_ID_COMMAND_SET_1] != 0xFFFF &&
> > -	   (id[ATA_ID_COMMAND_SET_1] & (1 << 2)))
> > +	if ((id[ATA_ID_COMMAND_SET_2] & 0xC000) == 0x4000 &&
> > +	    (id[ATA_ID_COMMAND_SET_2] & (1 << 2)))
> >  		return 1;
> >  	return 0;
> >  }
> 
>    Jeff, Bart, I've changed my mind about recasting this patch as Alan 
> has requested, as I do consider it correct now. The version check should 
> be unnecessary since I'm adding the validity bits check (hte fact that 
> ata_id_major_complex() is quite cumbesome way of checking the minumum 
> revision # -- this can be done with a single comparison instead of the 
> bit scan). Note that words 64-127 were marked reserved, and must be set 
> to 0 since ATA-1. Have your say please -- do I still need to recast the 
> patch?

I'm fine with the current version -- validity checking should take care of
handling reserved words (even if they are set to 0xff) and we can always add
more detailed checks later if necessary (which is very unlikely)...

Thanks,
Bart
--
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