Re: [PATCH 03/68] libata: Add ata_port_classify() helper

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

 



On 12/21/21 16:20, Hannes Reinecke wrote:
> Add an ata_port_classify() helper to print out the results from
> the device classification and remove the debugging statements
> from ata_dev_classify().
> 
> Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
> ---
>  drivers/ata/libahci.c          |  2 +-
>  drivers/ata/libata-core.c      | 21 +++++----------------
>  drivers/ata/libata-sff.c       |  2 +-
>  drivers/ata/libata-transport.c | 27 +++++++++++++++++++++++++++
>  drivers/ata/sata_fsl.c         |  2 +-
>  drivers/ata/sata_inic162x.c    |  2 +-
>  drivers/ata/sata_sil24.c       |  2 +-
>  include/linux/libata.h         |  2 ++
>  8 files changed, 39 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index 94edbc89a48c..b7b460560a92 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -1300,7 +1300,7 @@ unsigned int ahci_dev_classify(struct ata_port *ap)
>  	tf.lbal		= (tmp >> 8)	& 0xff;
>  	tf.nsect	= (tmp)		& 0xff;
>  
> -	return ata_dev_classify(&tf);
> +	return ata_port_classify(ap, &tf);
>  }
>  EXPORT_SYMBOL_GPL(ahci_dev_classify);
>  
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 72f56c32fe83..296cca54017a 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -1007,32 +1007,21 @@ unsigned int ata_dev_classify(const struct ata_taskfile *tf)
>  	 * SEMB signature.  This is worked around in
>  	 * ata_dev_read_id().
>  	 */
> -	if ((tf->lbam == 0) && (tf->lbah == 0)) {
> -		DPRINTK("found ATA device by sig\n");
> +	if ((tf->lbam == 0) && (tf->lbah == 0))
>  		return ATA_DEV_ATA;
> -	}
>  
> -	if ((tf->lbam == 0x14) && (tf->lbah == 0xeb)) {
> -		DPRINTK("found ATAPI device by sig\n");
> +	if ((tf->lbam == 0x14) && (tf->lbah == 0xeb))
>  		return ATA_DEV_ATAPI;
> -	}
>  
> -	if ((tf->lbam == 0x69) && (tf->lbah == 0x96)) {
> -		DPRINTK("found PMP device by sig\n");
> +	if ((tf->lbam == 0x69) && (tf->lbah == 0x96))
>  		return ATA_DEV_PMP;
> -	}
>  
> -	if ((tf->lbam == 0x3c) && (tf->lbah == 0xc3)) {
> -		DPRINTK("found SEMB device by sig (could be ATA device)\n");
> +	if ((tf->lbam == 0x3c) && (tf->lbah == 0xc3))
>  		return ATA_DEV_SEMB;
> -	}
>  
> -	if ((tf->lbam == 0xcd) && (tf->lbah == 0xab)) {
> -		DPRINTK("found ZAC device by sig\n");
> +	if ((tf->lbam == 0xcd) && (tf->lbah == 0xab))
>  		return ATA_DEV_ZAC;
> -	}

When applying, I dropped the unnecessary extra inner brackets in the
conditions.

>  
> -	DPRINTK("unknown device\n");
>  	return ATA_DEV_UNKNOWN;
>  }
>  EXPORT_SYMBOL_GPL(ata_dev_classify);
> diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
> index 39c026f3948c..a119fabe0919 100644
> --- a/drivers/ata/libata-sff.c
> +++ b/drivers/ata/libata-sff.c
> @@ -1853,7 +1853,7 @@ unsigned int ata_sff_dev_classify(struct ata_device *dev, int present,
>  		return ATA_DEV_NONE;
>  
>  	/* determine if device is ATA or ATAPI */
> -	class = ata_dev_classify(&tf);
> +	class = ata_port_classify(ap, &tf);
>  
>  	if (class == ATA_DEV_UNKNOWN) {
>  		/* If the device failed diagnostic, it's likely to
> diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
> index 4162d625fc92..8f44ad772a0e 100644
> --- a/drivers/ata/libata-transport.c
> +++ b/drivers/ata/libata-transport.c
> @@ -321,6 +321,33 @@ int ata_tport_add(struct device *parent,
>  	return error;
>  }
>  
> +/**
> + *     ata_port_classify - determine device type based on ATA-spec signature
> + *     @ap: ATA port device on which the classification should be run
> + *     @tf: ATA taskfile register set for device to be identified
> + *
> + *     A wrapper around ata_dev_classify() to provide additional logging
> + *
> + *     RETURNS:
> + *     Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, %ATA_DEV_PMP,
> + *     %ATA_DEV_ZAC, or %ATA_DEV_UNKNOWN the event of failure.
> + */
> +unsigned int ata_port_classify(struct ata_port *ap,
> +			       const struct ata_taskfile *tf)
> +{
> +	int i;
> +	unsigned int class = ata_dev_classify(tf);
> +
> +	/* Start with index '1' to skip the 'unknown' entry */
> +	for (i = 1; i < ARRAY_SIZE(ata_class_names); i++) {
> +		if (ata_class_names[i].value == class)
> +			ata_port_dbg(ap, "found %s device by sig\n",
> +				     ata_class_names[i].name);

This needs a "return class;" here. Otherwise, the following message is
always printed.

> +	}
> +	ata_port_info(ap, "found unknown device (class %u)\n", class);
> +	return class;
> +}
> +EXPORT_SYMBOL_GPL(ata_port_classify);
>  
>  /*
>   * ATA link attributes
> diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
> index ec52511ae60f..7504d9fbff2a 100644
> --- a/drivers/ata/sata_fsl.c
> +++ b/drivers/ata/sata_fsl.c
> @@ -814,7 +814,7 @@ static unsigned int sata_fsl_dev_classify(struct ata_port *ap)
>  	tf.lbal = (temp >> 8) & 0xff;
>  	tf.nsect = temp & 0xff;
>  
> -	return ata_dev_classify(&tf);
> +	return ata_port_classify(ap, &tf);
>  }
>  
>  static int sata_fsl_hardreset(struct ata_link *link, unsigned int *class,
> diff --git a/drivers/ata/sata_inic162x.c b/drivers/ata/sata_inic162x.c
> index e517bd8822a5..b6239dae524a 100644
> --- a/drivers/ata/sata_inic162x.c
> +++ b/drivers/ata/sata_inic162x.c
> @@ -657,7 +657,7 @@ static int inic_hardreset(struct ata_link *link, unsigned int *class,
>  		}
>  
>  		inic_tf_read(ap, &tf);
> -		*class = ata_dev_classify(&tf);
> +		*class = ata_port_classify(ap, &tf);
>  	}
>  
>  	return 0;
> diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
> index f99ec6f7d7c0..7e9c1945dc81 100644
> --- a/drivers/ata/sata_sil24.c
> +++ b/drivers/ata/sata_sil24.c
> @@ -680,7 +680,7 @@ static int sil24_softreset(struct ata_link *link, unsigned int *class,
>  	}
>  
>  	sil24_read_tf(ap, 0, &tf);
> -	*class = ata_dev_classify(&tf);
> +	*class = ata_port_classify(ap, &tf);
>  
>  	DPRINTK("EXIT, class=%u\n", *class);
>  	return 0;
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 2a8404b26083..235fdbeb19ea 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -1160,6 +1160,8 @@ extern enum ata_completion_errors ata_noop_qc_prep(struct ata_queued_cmd *qc);
>  extern void ata_sg_init(struct ata_queued_cmd *qc, struct scatterlist *sg,
>  		 unsigned int n_elem);
>  extern unsigned int ata_dev_classify(const struct ata_taskfile *tf);
> +extern unsigned int ata_port_classify(struct ata_port *ap,
> +				      const struct ata_taskfile *tf);
>  extern void ata_dev_disable(struct ata_device *adev);
>  extern void ata_id_string(const u16 *id, unsigned char *s,
>  			  unsigned int ofs, unsigned int len);


-- 
Damien Le Moal
Western Digital Research



[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