[U-Boot] [EXT] Re: [RFC 1/3] scsi: ata: Add DM SCSI interface to support None AHCI sata driver

Peng Ma peng.ma at nxp.com
Thu Sep 26 03:15:04 UTC 2019


Hi Robert,

Thanks very much for your comments, please see my comments inline.

>-----Original Message-----
>From: Robert Hancock <hancock at sedsystems.ca>
>Sent: 2019年9月25日 23:50
>To: Peng Ma <peng.ma at nxp.com>; sjg at chromium.org; Prabhakar X
><prabhakar.kushwaha at nxp.com>; jagan at openedev.com;
>bmeng.cn at gmail.com; Andy Tang <andy.tang at nxp.com>
>Cc: andre.przywara at arm.com; michal.simek at xilinx.com;
>u-boot at lists.denx.de; smoch at web.de; sr at denx.de
>Subject: [EXT] Re: [U-Boot] [RFC 1/3] scsi: ata: Add DM SCSI interface to
>support None AHCI sata driver
>
>Caution: EXT Email
>
>On 2019-09-25 3:54 a.m., Peng Ma wrote:
>> In driver/ata. If the sata driver support AHCI mode, there will
>> provides a complete set of SCSI interface. If the sata is not support
>> AHCI
>> mode(NONE_AHCI) there will not provides the SCSI interface.
>>
>> This patch is to support SCSI interface for None AHCI sata such as
>> fsl_sata.c sil_sata.c etc.
>
>The patch rationale seems strange - SATA controllers don't inherently provide a
>SCSI interface regardless of whether they are AHCI or not. In the Linux kernel,
>SATA devices are supported using the SCSI layer via SCSI to ATA translation,
>which is a standardized mapping, largely in order to reuse a bunch of code -
>and U-Boot could choose to do the same
>- but AHCI controllers are handled the same way as all other SATA controllers in
>that regard by Linux, so I'm not sure why that distinction is being made here.
>
[Peng Ma] 
For Linux:
If the sata controller fallow AHCI:
	Vendor_init ----> ahci_platform_init_host(struct ata_port_operations ahci_ops) ---> ata_host_register ----> ata_scsi_add_hosts ......
If the sata controller not fallow AHCI:
	Vendor_init(struct ata_port_operations sata_xxx _ops) ----> ata_host_register ----> ata_scsi_add_hosts ......

For U-boot:
If the sata controller fallow AHCI:
The devices:
	UCLASS_BLK---->UCLASS_SCSI---->UCLASS_AHCI(just call ata interface)
The drivers:
	Vendor_init ----> ahci_probe_scsi(struct scsi_ops scsi_ops) then the devices will bind(ahci_bind_scsi) the driver during uboot init.
If the sata controller not fallow AHCI:
	Not Support:
	We shold support for None AHCI sata with scsi interface like as follows:
The devices:
	UCLASS_BLK---->UCLASS_SCSI---->UCLASS_NONE_AHCI
The drivers:
	Vendor_init (struct scsi_ops scsi_ops) then the devices will bind(ata_bind_scsi) the driver during uboot init.

Note: On U-boot the The ATA lib just provide some common interface.
Best Regards,
Peng

>>
>> Signed-off-by: Peng Ma <peng.ma at nxp.com>
>> ---
>>  drivers/ata/Kconfig      |  17 +++-
>>  drivers/ata/Makefile     |   2 +
>>  drivers/ata/ata-uclass.c |  16 ++++
>>  drivers/ata/ata.c        | 244
>+++++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/ata/sata.c       |   2 +-
>>  drivers/ata/scsi_ata.h   |  43 +++++++++
>>  include/dm/uclass-id.h   |   1 +
>>  7 files changed, 323 insertions(+), 2 deletions(-)  create mode
>> 100644 drivers/ata/ata-uclass.c  create mode 100644 drivers/ata/ata.c
>> create mode 100644 drivers/ata/scsi_ata.h
>>
>> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig index
>> 87636ae..3d6db2e 100644
>> --- a/drivers/ata/Kconfig
>> +++ b/drivers/ata/Kconfig
>> @@ -7,6 +7,13 @@ config AHCI
>>         operations at present. The block device interface has not been
>converted
>>         to driver model.
>>
>> +config NONE_AHCI
>> +     bool "Support None AHCI SATA controllers with driver model"
>> +     depends on DM
>> +     help
>> +       This enables a uclass for disk controllers in U-Boot. such as AHCI. It
>> +       support None AHCI sata with DM mode
>> +
>>  config SATA
>>       bool "Support SATA controllers"
>>       select HAVE_BLOCK_DEVICE
>> @@ -32,6 +39,15 @@ config SCSI_AHCI
>>       help
>>         Enable this to allow interfacing SATA devices via the SCSI layer.
>>
>> +config SCSI_NONE_AHCI
>> +     bool "Enable SCSI interface to None AHCI SATA devices"
>> +     select LIBATA
>> +     select SCSI
>> +     select DM_SCSI
>> +     help
>> +       Enable this to allow interfacing None AHCI SATA devices via the DM
>> +       SCSI layer.
>> +
>>  menu "SATA/SCSI device support"
>>
>>  config AHCI_PCI
>> @@ -49,7 +65,6 @@ config SATA_CEVA
>>         ZynqMP. Support up to 2 external devices. Complient with SATA
>3.1 and
>>         AHCI 1.3 specifications with hot-plug detect feature.
>>
>> -
>>  config DWC_AHCI
>>       bool "Enable Synopsys DWC AHCI driver support"
>>       select SCSI_AHCI
>> diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile index
>> 6e03384..cddbdc8 100644
>> --- a/drivers/ata/Makefile
>> +++ b/drivers/ata/Makefile
>> @@ -6,8 +6,10 @@
>>  obj-$(CONFIG_DWC_AHCI) += dwc_ahci.o
>>  obj-$(CONFIG_FSL_AHCI) += fsl_ahci.o
>>  obj-$(CONFIG_AHCI) += ahci-uclass.o
>> +obj-$(CONFIG_NONE_AHCI) += ata-uclass.o
>>  obj-$(CONFIG_AHCI_PCI) += ahci-pci.o
>>  obj-$(CONFIG_SCSI_AHCI) += ahci.o
>> +obj-$(CONFIG_SCSI_NONE_AHCI) += ata.o
>>  obj-$(CONFIG_DWC_AHSATA) += dwc_ahsata.o
>>  obj-$(CONFIG_FSL_SATA) += fsl_sata.o
>>  obj-$(CONFIG_LIBATA) += libata.o
>> diff --git a/drivers/ata/ata-uclass.c b/drivers/ata/ata-uclass.c new
>> file mode 100644 index 0000000..e2cc499
>> --- /dev/null
>> +++ b/drivers/ata/ata-uclass.c
>> @@ -0,0 +1,16 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (c) 2019 NXP, Inc
>> + * Written by Peng Ma<peng.ma at nxp.com>  */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <scsi.h>
>> +#include "scsi_ata.h"
>> +
>> +UCLASS_DRIVER(ata) = {
>> +     .id             = UCLASS_NONE_AHCI,
>> +     .name           = "ata",
>> +     .per_device_auto_alloc_size = sizeof(struct ata_uc_priv), };
>> diff --git a/drivers/ata/ata.c b/drivers/ata/ata.c new file mode
>> 100644 index 0000000..bdb7403
>> --- /dev/null
>> +++ b/drivers/ata/ata.c
>> @@ -0,0 +1,244 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) NXP, Inc. 2019.
>> + * Author: Peng Ma<peng.ma at nxp.com>
>> + *
>> + * with the reference on libata and none ahci drvier in kernel
>> + *
>> + * This driver provides a DM SCSI interface to None AHCI SATA.
>> + */
>> +
>> +#include <common.h>
>> +#include <dm/lists.h>
>> +#include <dm.h>
>> +#include <scsi.h>
>> +#include <libata.h>
>> +#include <sata.h>
>> +#include <malloc.h>
>> +#include <memalign.h>
>> +#include <fis.h>
>> +
>> +#include "scsi_ata.h"
>> +
>> +int ata_bind_scsi(struct udevice *ata_dev, struct udevice **devp) {
>> +     int ret;
>> +     struct udevice *dev;
>> +
>> +     ret =  device_bind_driver(ata_dev, "ata_scsi", "ata_scsi", &dev);
>> +     if (ret)
>> +             return ret;
>> +     *devp = dev;
>> +
>> +     return 0;
>> +}
>> +
>> +static int scsi_exec_internal(struct udevice *dev, struct scsi_cmd *pccb,
>> +                           bool is_write) {
>> +     u32 temp;
>> +     u16 blocks = 0;
>> +     int ret = -ENODEV;
>> +     lbaint_t start = 0;
>> +     u8 port = pccb->target;
>> +     u8 *buffer = pccb->pdata;
>> +     struct ata_uc_priv *uc_priv = dev_get_uclass_priv(dev);
>> +
>> +     /* Retrieve the base LBA number from the ccb structure. */
>> +     if (pccb->cmd[0] == SCSI_READ16) {
>> +             memcpy(&start, pccb->cmd + 2, 8);
>> +             start = be64_to_cpu(start);
>> +     } else {
>> +             memcpy(&temp, pccb->cmd + 2, 4);
>> +             start = be32_to_cpu(temp);
>> +     }
>> +
>> +     if (pccb->cmd[0] == SCSI_READ16)
>> +             blocks = (((u16)pccb->cmd[13]) << 8) |
>((u16)pccb->cmd[14]);
>> +     else
>> +             blocks = (((u16)pccb->cmd[7]) << 8) |
>> + ((u16)pccb->cmd[8]);
>> +
>> +     debug("scsi_ata: %s %u blocks starting from lba 0x" LBAFU "\n",
>> +           is_write ?  "write" : "read", blocks, start);
>> +
>> +     if (is_write)
>> +             if (uc_priv->write)
>> +                     ret = uc_priv->write(dev, start, blocks, buffer,
>port);
>> +             else
>> +                     pr_err("scsi_ata: write ptr is NULL\n");
>> +     else
>> +             if (uc_priv->read)
>> +                     ret = uc_priv->read(dev, start, blocks, buffer,
>port);
>> +             else
>> +                     pr_err("scsi_ata: read ptr is NULL\n");
>> +     return ret;
>> +}
>> +
>> +static char *fsl_ata_id_strcpy(u16 *target, u16 *src, int len) {
>> +     int i;
>> +
>> +     for (i = 0; i < len / 2; i++)
>> +             target[i] = src[i];
>> +
>> +     return (char *)target;
>> +}
>> +
>> +static int ata_scsiop_inquiry(struct udevice *dev, struct scsi_cmd
>> +*pccb) {
>> +     u8 port;
>> +     u16 *idbuf;
>> +     struct ata_uc_priv *uc_priv = dev_get_uclass_priv(dev);
>> +
>> +     ALLOC_CACHE_ALIGN_BUFFER(u16, tmpid, ATA_ID_WORDS);
>> +
>> +     /* Clean ccb data buffer */
>> +     memset(pccb->pdata, 0, pccb->datalen);
>> +
>> +     if (pccb->datalen <= 35)
>> +             return 0;
>> +
>> +     /* Read id from sata */
>> +     port = pccb->target;
>> +
>> +     if (uc_priv->ata_inquiry) {
>> +             uc_priv->ata_inquiry(dev, (u16 *)tmpid, port);
>> +     } else {
>> +             pr_err("scsi_ata: ata_inquiry ptr is NULL\n");
>> +             return -ENODEV;
>> +     }
>> +
>> +     if (!uc_priv->ataid[port]) {
>> +             uc_priv->ataid[port] = malloc(ATA_ID_WORDS * 2);
>> +             if (!uc_priv->ataid[port]) {
>> +                     pr_err("%s: No memory for ataid[port]\n",
>__func__);
>> +                     return -ENOMEM;
>> +             }
>> +     }
>> +
>> +     idbuf = uc_priv->ataid[port];
>> +
>> +     memcpy(idbuf, tmpid, ATA_ID_WORDS * 2);
>> +
>> +     memcpy(&pccb->pdata[8], "ATA     ", 8);
>> +     fsl_ata_id_strcpy((u16 *)&pccb->pdata[16], &idbuf[ATA_ID_PROD],
>16);
>> +     fsl_ata_id_strcpy((u16 *)&pccb->pdata[32],
>> + &idbuf[ATA_ID_FW_REV], 4);
>> +
>> +#ifdef DEBUG
>> +     ata_dump_id(idbuf);
>> +#endif
>> +     return 0;
>> +}
>> +
>> +/*
>> + * SCSI READ CAPACITY10 command operation.
>> + */
>> +static int ata_scsiop_read_capacity10(struct ata_uc_priv *uc_priv,
>> +                                   struct scsi_cmd *pccb) {
>> +     u32 cap;
>> +     u64 cap64;
>> +     u32 block_size;
>> +
>> +     if (!uc_priv->ataid[pccb->target]) {
>> +             pr_err("scsi_ata: SCSI READ CAPACITY10 command
>failure.");
>> +             pr_err("\tNo ATA info!\n");
>> +             pr_err("\tPlease run SCSI command INQUIRY first!\n");
>> +             return -EPERM;
>> +     }
>> +
>> +     cap64 = ata_id_n_sectors(uc_priv->ataid[pccb->target]);
>> +     if (cap64 > 0x100000000ULL)
>> +             cap64 = 0xffffffff;
>> +
>> +     cap = cpu_to_be32(cap64);
>> +     memcpy(pccb->pdata, &cap, sizeof(cap));
>> +
>> +     block_size = cpu_to_be32((u32)512);
>> +     memcpy(&pccb->pdata[4], &block_size, 4);
>> +
>> +     return 0;
>> +}
>> +
>> +/*
>> + * SCSI READ CAPACITY16 command operation.
>> + */
>> +static int ata_scsiop_read_capacity16(struct ata_uc_priv *uc_priv,
>> +                                   struct scsi_cmd *pccb) {
>> +     u64 cap;
>> +     u64 block_size;
>> +
>> +     if (!uc_priv->ataid[pccb->target]) {
>> +             pr_err("scsi_ata: SCSI READ CAPACITY16 command
>failure.");
>> +             pr_err("\tNo ATA info!\n");
>> +             pr_err("\tPlease run SCSI command INQUIRY first!\n");
>> +             return -EPERM;
>> +     }
>> +
>> +     cap = ata_id_n_sectors(uc_priv->ataid[pccb->target]);
>> +     cap = cpu_to_be64(cap);
>> +     memcpy(pccb->pdata, &cap, sizeof(cap));
>> +
>> +     block_size = cpu_to_be64((u64)512);
>> +     memcpy(&pccb->pdata[8], &block_size, 8);
>> +
>> +     return 0;
>> +}
>> +
>> +/*
>> + * SCSI TEST UNIT READY command operation.
>> + */
>> +static int ata_scsiop_test_unit_ready(struct ata_uc_priv *uc_priv,
>> +                                   struct scsi_cmd *pccb) {
>> +     return (uc_priv->ataid[pccb->target]) ? 0 : -EPERM; }
>> +
>> +static int ata_scsi_exec(struct udevice *dev, struct scsi_cmd *pccb)
>> +{
>> +     struct ata_uc_priv *uc_priv = dev_get_uclass_priv(dev->parent);
>> +     int ret;
>> +
>> +     switch (pccb->cmd[0]) {
>> +     case SCSI_READ16:
>> +     case SCSI_READ10:
>> +             ret = scsi_exec_internal(dev->parent, pccb, 0);
>> +             break;
>> +     case SCSI_WRITE10:
>> +             ret = scsi_exec_internal(dev->parent, pccb, 1);
>> +             break;
>> +     case SCSI_RD_CAPAC10:
>> +             ret = ata_scsiop_read_capacity10(uc_priv, pccb);
>> +             break;
>> +     case SCSI_RD_CAPAC16:
>> +             ret = ata_scsiop_read_capacity16(uc_priv, pccb);
>> +             break;
>> +     case SCSI_TST_U_RDY:
>> +             ret = ata_scsiop_test_unit_ready(uc_priv, pccb);
>> +             break;
>> +     case SCSI_INQUIRY:
>> +             ret = ata_scsiop_inquiry(dev->parent, pccb);
>> +             break;
>> +     default:
>> +             pr_err("Unsupport SCSI command 0x%02x\n",
>pccb->cmd[0]);
>> +             return -ENOTSUPP;
>> +     }
>> +
>> +     if (ret) {
>> +             debug("SCSI command 0x%02x ret errno %d\n",
>pccb->cmd[0], ret);
>> +             return ret;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +struct scsi_ops ata_scsi_ops = {
>> +     .exec           = ata_scsi_exec,
>> +};
>> +
>> +U_BOOT_DRIVER(ata_scsi) = {
>> +     .name           = "ata_scsi",
>> +     .id             = UCLASS_SCSI,
>> +     .ops            = &ata_scsi_ops,
>> +};
>> diff --git a/drivers/ata/sata.c b/drivers/ata/sata.c index
>> e384b80..1fee375 100644
>> --- a/drivers/ata/sata.c
>> +++ b/drivers/ata/sata.c
>> @@ -83,7 +83,7 @@ static unsigned long sata_bwrite(struct blk_desc
>> *block_dev, lbaint_t start,  }  #endif
>>
>> -#ifndef CONFIG_AHCI
>> +#ifndef CONFIG_SCSI
>>  int __sata_initialize(void)
>>  {
>>       int rc, ret = -1;
>> diff --git a/drivers/ata/scsi_ata.h b/drivers/ata/scsi_ata.h new file
>> mode 100644 index 0000000..472a562
>> --- /dev/null
>> +++ b/drivers/ata/scsi_ata.h
>> @@ -0,0 +1,43 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * Copyright (C) NXP Inc. 2019.
>> + * Author: Peng Ma<peng.ma at nxp.com>
>> + */
>> +#ifndef _AHCI_H_
>> +#define _AHCI_H_
>> +
>> +#include <pci.h>
>> +
>> +#define ATA_MAX_PORTS                32
>> +/**
>> + * struct ata_uc_priv - information about an ATA controller
>> + *
>> + * When driver model is used, this is accessible using
>> +dev_get_uclass_priv(dev)
>> + * where dev is the controller (although at present it sometimes stands
>alone).
>> + */
>> +struct ata_uc_priv {
>> +#if defined(CONFIG_DM_PCI) || defined(CONFIG_DM_SCSI)
>> +     struct udevice *dev;
>> +#else
>> +     pci_dev_t       dev;
>> +#endif
>> +     u16     *ataid[ATA_MAX_PORTS];
>> +
>> +     ulong (*read)(struct udevice *dev, lbaint_t start, u16 blkcnt,
>> +                   void *buffer, u8 port);
>> +     ulong (*write)(struct udevice *dev, lbaint_t start, u16 blkcnt,
>> +                    const void *buffer, u8 port);
>> +     int (*ata_inquiry)(struct udevice *dev, u16 *buffer, u8 port);
>> +};
>> +
>> +/**
>> + * ata_bind_scsi() - bind a new SCSI bus as a child
>> + *
>> + * Note that the SCSI bus device will itself bind block devices
>> + *
>> + * @ahci_dev: AHCI parent device
>> + * @devp: Returns new SCSI bus device
>> + * @return 0 if OK, -ve on error
>> + */
>> +int ata_bind_scsi(struct udevice *ahci_dev, struct udevice **devp);
>> +#endif
>> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index
>> d4d9610..087cd2d 100644
>> --- a/include/dm/uclass-id.h
>> +++ b/include/dm/uclass-id.h
>> @@ -29,6 +29,7 @@ enum uclass_id {
>>       /* U-Boot uclasses start here - in alphabetical order */
>>       UCLASS_ADC,             /* Analog-to-digital converter */
>>       UCLASS_AHCI,            /* SATA disk controller */
>> +     UCLASS_NONE_AHCI,       /* SATA disk controller of None AHCI */
>>       UCLASS_AUDIO_CODEC,     /* Audio codec with control and data
>path */
>>       UCLASS_AXI,             /* AXI bus */
>>       UCLASS_BLK,             /* Block device */
>>
>
>--
>Robert Hancock
>Senior Software Developer
>SED Systems, a division of Calian Ltd.
>Email: hancock at sedsystems.ca


More information about the U-Boot mailing list