On 12/13/18 11:47 AM, Hannes Reinecke wrote:
> Add a 'link' argument to ata_dev_classify() so that we can call
> ata_link_dbg() with the correct device and drop the DPRINTK usage.
This patch also:
- does actual ata_dev_classify() conversion to not use DPRINTK()
- adds printing of unknown signatures in ata_dev_classify()
- removes DPRINTK() instance from sata_fsl_softreset()
Please document above changes in the patch description.
Also please:
- preserve __func__ printing in the DPRINTK() conversion
- use dev_dbg() directly instead of ata_link_dbg()
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> Signed-off-by: Hannes Reinecke <hare@xxxxxxxx>
> ---
> drivers/ata/ahci.c | 2 +-
> drivers/ata/ahci.h | 2 +-
> drivers/ata/libahci.c | 9 +++++----
> drivers/ata/libata-core.c | 18 +++++++++++-------
> drivers/ata/libata-sff.c | 2 +-
> drivers/ata/sata_fsl.c | 11 +++++------
> drivers/ata/sata_inic162x.c | 2 +-
> drivers/ata/sata_sil24.c | 2 +-
> drivers/scsi/libsas/sas_ata.c | 3 ++-
> include/linux/libata.h | 3 ++-
> 10 files changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 89d9980f3de4..cda9cdd2495b 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -826,7 +826,7 @@ static int ahci_avn_hardreset(struct ata_link *link, unsigned int *class,
> hpriv->start_engine(ap);
>
> if (online)
> - *class = ahci_dev_classify(ap);
> + *class = ahci_dev_classify(link);
>
> ata_link_dbg(link, "EXIT, rc=%d, class=%u\n", rc, *class);
> return rc;
> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> index ef356e70e6de..65df818e5e09 100644
> --- a/drivers/ata/ahci.h
> +++ b/drivers/ata/ahci.h
> @@ -403,7 +403,7 @@ extern struct ata_port_operations ahci_ops;
> extern struct ata_port_operations ahci_platform_ops;
> extern struct ata_port_operations ahci_pmp_retry_srst_ops;
>
> -unsigned int ahci_dev_classify(struct ata_port *ap);
> +unsigned int ahci_dev_classify(struct ata_link *link);
> void ahci_fill_cmd_slot(struct ahci_port_priv *pp, unsigned int tag,
> u32 opts);
> void ahci_save_initial_config(struct device *dev,
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index d6c93ce83504..421a40731ed3 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -1281,8 +1281,9 @@ static void ahci_dev_config(struct ata_device *dev)
> }
> }
>
> -unsigned int ahci_dev_classify(struct ata_port *ap)
> +unsigned int ahci_dev_classify(struct ata_link *link)
> {
> + struct ata_port *ap = link->ap;
> void __iomem *port_mmio = ahci_port_base(ap);
> struct ata_taskfile tf;
> u32 tmp;
> @@ -1293,7 +1294,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_dev_classify(link, &tf);
> }
> EXPORT_SYMBOL_GPL(ahci_dev_classify);
>
> @@ -1464,7 +1465,7 @@ int ahci_do_softreset(struct ata_link *link, unsigned int *class,
> reason = "device not ready";
> goto fail;
> } else
> - *class = ahci_dev_classify(ap);
> + *class = ahci_dev_classify(link);
>
> /* re-enable FBS if disabled before */
> if (fbs_disabled)
> @@ -1574,7 +1575,7 @@ int ahci_do_hardreset(struct ata_link *link, unsigned int *class,
> hpriv->start_engine(ap);
>
> if (*online)
> - *class = ahci_dev_classify(ap);
> + *class = ahci_dev_classify(link);
>
> ata_link_dbg(link, "%s: EXIT, rc=%d, class=%u\n", __func__, rc, *class);
> return rc;
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index f4b5a61b1c63..a36f24de399b 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -1046,6 +1046,7 @@ const char *sata_spd_string(unsigned int spd)
>
> /**
> * ata_dev_classify - determine device type based on ATA-spec signature
> + * @link: ATA device link for the device to be identified
> * @tf: ATA taskfile register set for device to be identified
> *
> * Determine from taskfile register contents whether a device is
> @@ -1059,7 +1060,8 @@ const char *sata_spd_string(unsigned int spd)
> * 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_dev_classify(const struct ata_taskfile *tf)
> +unsigned int ata_dev_classify(struct ata_link *link,
> + const struct ata_taskfile *tf)
> {
> /* Apple's open source Darwin code hints that some devices only
> * put a proper signature into the LBA mid/high registers,
> @@ -1083,31 +1085,33 @@ unsigned int ata_dev_classify(const struct ata_taskfile *tf)
> * ata_dev_read_id().
> */
> if ((tf->lbam == 0) && (tf->lbah == 0)) {
> - DPRINTK("found ATA device by sig\n");
> + ata_link_dbg(link, "found ATA device by sig\n");
> return ATA_DEV_ATA;
> }
>
> if ((tf->lbam == 0x14) && (tf->lbah == 0xeb)) {
> - DPRINTK("found ATAPI device by sig\n");
> + ata_link_dbg(link, "found ATAPI device by sig\n");
> return ATA_DEV_ATAPI;
> }
>
> if ((tf->lbam == 0x69) && (tf->lbah == 0x96)) {
> - DPRINTK("found PMP device by sig\n");
> + ata_link_dbg(link, "found PMP device by sig\n");
> return ATA_DEV_PMP;
> }
>
> if ((tf->lbam == 0x3c) && (tf->lbah == 0xc3)) {
> - DPRINTK("found SEMB device by sig (could be ATA device)\n");
> + ata_link_dbg(link,
> + "found SEMB device by sig (could be ATA device)\n");
> return ATA_DEV_SEMB;
> }
>
> if ((tf->lbam == 0xcd) && (tf->lbah == 0xab)) {
> - DPRINTK("found ZAC device by sig\n");
> + ata_link_dbg(link, "found ZAC device by sig\n");
> return ATA_DEV_ZAC;
> }
>
> - DPRINTK("unknown device\n");
> + ata_link_dbg(link, "unknown device signature %02x %02x\n",
> + tf->lbam, tf->lbah);
> return ATA_DEV_UNKNOWN;
> }
>
> diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
> index 3326c3b02c61..48b27f4e52fe 100644
> --- a/drivers/ata/libata-sff.c
> +++ b/drivers/ata/libata-sff.c
> @@ -1850,7 +1850,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_dev_classify(dev->link, &tf);
>
> if (class == ATA_DEV_UNKNOWN) {
> /* If the device failed diagnostic, it's likely to
> diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
> index 8bb129c87a3f..6260ea9e180f 100644
> --- a/drivers/ata/sata_fsl.c
> +++ b/drivers/ata/sata_fsl.c
> @@ -797,9 +797,9 @@ static void sata_fsl_port_stop(struct ata_port *ap)
> kfree(pp);
> }
>
> -static unsigned int sata_fsl_dev_classify(struct ata_port *ap)
> +static unsigned int sata_fsl_dev_classify(struct ata_link *link)
> {
> - struct sata_fsl_host_priv *host_priv = ap->host->private_data;
> + struct sata_fsl_host_priv *host_priv = link->ap->host->private_data;
> void __iomem *hcr_base = host_priv->hcr_base;
> struct ata_taskfile tf;
> u32 temp;
> @@ -815,7 +815,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_dev_classify(link, &tf);
> }
>
> static int sata_fsl_hardreset(struct ata_link *link, unsigned int *class,
> @@ -917,7 +917,7 @@ static int sata_fsl_hardreset(struct ata_link *link, unsigned int *class,
> } else {
> ata_port_info(ap, "Signature Update detected @ %d msecs\n",
> jiffies_to_msecs(jiffies - start_jiffies));
> - *class = sata_fsl_dev_classify(ap);
> + *class = sata_fsl_dev_classify(link);
> return 0;
> }
>
> @@ -1050,9 +1050,8 @@ static int sata_fsl_softreset(struct ata_link *link, unsigned int *class,
> * would have been updated
> */
>
> - *class = sata_fsl_dev_classify(ap);
> + *class = sata_fsl_dev_classify(link);
>
> - ata_link_dbg(link, "class = %d\n", *class);
> VPRINTK("ccreg = 0x%x\n", ioread32(hcr_base + CC));
> VPRINTK("cereg = 0x%x\n", ioread32(hcr_base + CE));
> }
> diff --git a/drivers/ata/sata_inic162x.c b/drivers/ata/sata_inic162x.c
> index e0bcf9b2dab0..3b2c38623b57 100644
> --- a/drivers/ata/sata_inic162x.c
> +++ b/drivers/ata/sata_inic162x.c
> @@ -649,7 +649,7 @@ static int inic_hardreset(struct ata_link *link, unsigned int *class,
> }
>
> inic_tf_read(ap, &tf);
> - *class = ata_dev_classify(&tf);
> + *class = ata_dev_classify(link, &tf);
> }
>
> return 0;
> diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
> index 4b00dce151eb..f041f00342c4 100644
> --- a/drivers/ata/sata_sil24.c
> +++ b/drivers/ata/sata_sil24.c
> @@ -687,7 +687,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_dev_classify(link, &tf);
>
> ata_link_dbg(link, "%s: EXIT, class=%u\n", __func__, *class);
> return 0;
> diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
> index 4f6cdf53e913..06736b2c7a97 100644
> --- a/drivers/scsi/libsas/sas_ata.c
> +++ b/drivers/scsi/libsas/sas_ata.c
> @@ -620,6 +620,7 @@ void sas_ata_task_abort(struct sas_task *task)
>
> static int sas_get_ata_command_set(struct domain_device *dev)
> {
> + struct ata_port *ap = dev->sata_dev.ap;
> struct dev_to_host_fis *fis =
> (struct dev_to_host_fis *) dev->frame_rcvd;
> struct ata_taskfile tf;
> @@ -629,7 +630,7 @@ static int sas_get_ata_command_set(struct domain_device *dev)
>
> ata_tf_from_fis((const u8 *)fis, &tf);
>
> - return ata_dev_classify(&tf);
> + return ata_dev_classify(&ap->link, &tf);
> }
>
> void sas_probe_sata(struct asd_sas_port *port)
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 7b2f039d3d21..808a96a8cb1d 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -1180,7 +1180,8 @@ extern int ata_std_qc_defer(struct ata_queued_cmd *qc);
> extern void 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_dev_classify(struct ata_link *link,
> + 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);
>
