[PATCH 0/2] libata: introduce ATA_FLAG_NO_LOG_PAGE

[PATCH 0/2] libata: introduce ATA_FLAG_NO_LOG_PAGE

[Andreas Werner]

This patchset add a new ata port flag ATA_FLAG_NO_LOG_PAGE to be able
to blacklist ports/controller which e.g. locks up on a log page read.

This flag is added to the sata_fsl driver which is the first affected
one.

The lockup was detected on Freescale P1013/P1022, T4240 using a ATP
mSATA.
The device failed during initialisation if the SATA device includes the
devslp feature.

With this patchset, we blacklist the fsl sata controller and return
a error on any attempt to read a log page. This allows us to access
the mSATA.

Andreas Werner (2):
  libata-eh.c: Introduce new ata port flag for controller which lockup
    on read log page
  ata/sata_fsl.c: add ATA_FLAG_NO_LOG_PAGE to blacklist the controller
    for log page reads

 drivers/ata/libata-eh.c | 9 +++++++++
 drivers/ata/sata_fsl.c  | 3 ++-
 include/linux/libata.h  | 1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

-- 
2.6.2

[PATCH 1/2] libata-eh.c: Introduce new ata port flag for controller which lockup on read log page

[Andreas Werner]

Some controller lockup on a ata_read_log_page.
Add new ata port flag ATA_FLAG_NO_LOG_PAGE which can used
to blacklist a controller.

If this flag is set, any attempt to read a log page returns an error
without actually issuing the command.

Signed-off-by: Andreas Werner <andreas.werner@men.de>
---
 drivers/ata/libata-eh.c | 9 +++++++++
 include/linux/libata.h  | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index cb0508a..f3e6277 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1505,12 +1505,21 @@ static const char *ata_err_string(unsigned int err_mask)
 unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
 			       u8 page, void *buf, unsigned int sectors)
 {
+	unsigned long ap_flags = dev->link->ap->flags;
 	struct ata_taskfile tf;
 	unsigned int err_mask;
 	bool dma = false;
 
 	DPRINTK("read log page - log 0x%x, page 0x%x\n", log, page);
 
+	/*
+	 * Return error without actually issuing the
+	 * command on controller which e.g. lockup
+	 * on a read log page.
+	 */
+	if (flags & ATA_FLAG_NO_LOG_PAGE)
+		return AC_ERR_DEV;
+
 retry:
 	ata_tf_init(dev, &tf);
 	if (dev->dma_mode && ata_id_has_read_log_dma_ext(dev->id) &&
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 83577f8..600c1e0 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -210,6 +210,7 @@ enum {
 	ATA_FLAG_SLAVE_POSS	= (1 << 0), /* host supports slave dev */
 					    /* (doesn't imply presence) */
 	ATA_FLAG_SATA		= (1 << 1),
+	ATA_FLAG_NO_LOG_PAGE	= (1 << 5), /* do not issue log page read */
 	ATA_FLAG_NO_ATAPI	= (1 << 6), /* No ATAPI support */
 	ATA_FLAG_PIO_DMA	= (1 << 7), /* PIO cmds via DMA */
 	ATA_FLAG_PIO_LBA48	= (1 << 8), /* Host DMA engine is LBA28 only */
-- 
2.6.2

Re: [PATCH 1/2] libata-eh.c: Introduce new ata port flag for controller which lockup on read log page

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2827 bytes --]

Hi Andreas,

[auto build test WARNING on tj-libata/for-next]
[also build test WARNING on v4.4-rc3 next-20151203]

url:    https://github.com/0day-ci/linux/commits/Andreas-Werner/libata-introduce-ATA_FLAG_NO_LOG_PAGE/20151204-235659
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tj/libata for-next
config: i386-randconfig-x003-12041200 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/linux/linkage.h:4:0,
                    from include/linux/kernel.h:6,
                    from drivers/ata/libata-eh.c:35:
   drivers/ata/libata-eh.c: In function 'ata_read_log_page':
   drivers/ata/libata-eh.c:1520:6: error: 'flags' undeclared (first use in this function)
     if (flags & ATA_FLAG_NO_LOG_PAGE)
         ^
   include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
     if (__builtin_constant_p((cond)) ? !!(cond) :   \
                               ^
>> drivers/ata/libata-eh.c:1520:2: note: in expansion of macro 'if'
     if (flags & ATA_FLAG_NO_LOG_PAGE)
     ^
   drivers/ata/libata-eh.c:1520:6: note: each undeclared identifier is reported only once for each function it appears in
     if (flags & ATA_FLAG_NO_LOG_PAGE)
         ^
   include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
     if (__builtin_constant_p((cond)) ? !!(cond) :   \
                               ^
>> drivers/ata/libata-eh.c:1520:2: note: in expansion of macro 'if'
     if (flags & ATA_FLAG_NO_LOG_PAGE)
     ^
   drivers/ata/libata-eh.c:1508:16: warning: unused variable 'ap_flags' [-Wunused-variable]
     unsigned long ap_flags = dev->link->ap->flags;
                   ^

vim +/if +1520 drivers/ata/libata-eh.c

  1504	 */
  1505	unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
  1506				       u8 page, void *buf, unsigned int sectors)
  1507	{
  1508		unsigned long ap_flags = dev->link->ap->flags;
  1509		struct ata_taskfile tf;
  1510		unsigned int err_mask;
  1511		bool dma = false;
  1512	
  1513		DPRINTK("read log page - log 0x%x, page 0x%x\n", log, page);
  1514	
  1515		/*
  1516		 * Return error without actually issuing the
  1517		 * command on controller which e.g. lockup
  1518		 * on a read log page.
  1519		 */
> 1520		if (flags & ATA_FLAG_NO_LOG_PAGE)
  1521			return AC_ERR_DEV;
  1522	
  1523	retry:
  1524		ata_tf_init(dev, &tf);
  1525		if (dev->dma_mode && ata_id_has_read_log_dma_ext(dev->id) &&
  1526		    !(dev->horkage & ATA_HORKAGE_NO_NCQ_LOG)) {
  1527			tf.command = ATA_CMD_READ_LOG_DMA_EXT;
  1528			tf.protocol = ATA_PROT_DMA;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 30965 bytes --]

Re: [PATCH 1/2] libata-eh.c: Introduce new ata port flag for controller which lockup on read log page

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1891 bytes --]

Hi Andreas,

[auto build test ERROR on tj-libata/for-next]
[also build test ERROR on v4.4-rc3 next-20151203]

url:    https://github.com/0day-ci/linux/commits/Andreas-Werner/libata-introduce-ATA_FLAG_NO_LOG_PAGE/20151204-235659
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tj/libata for-next
config: i386-randconfig-x008-12041200 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   drivers/ata/libata-eh.c: In function 'ata_read_log_page':
>> drivers/ata/libata-eh.c:1520:6: error: 'flags' undeclared (first use in this function)
     if (flags & ATA_FLAG_NO_LOG_PAGE)
         ^
   drivers/ata/libata-eh.c:1520:6: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/ata/libata-eh.c:1508:16: warning: unused variable 'ap_flags' [-Wunused-variable]
     unsigned long ap_flags = dev->link->ap->flags;
                   ^

vim +/flags +1520 drivers/ata/libata-eh.c

  1502	 *	RETURNS:
  1503	 *	0 on success, AC_ERR_* mask otherwise.
  1504	 */
  1505	unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
  1506				       u8 page, void *buf, unsigned int sectors)
  1507	{
> 1508		unsigned long ap_flags = dev->link->ap->flags;
  1509		struct ata_taskfile tf;
  1510		unsigned int err_mask;
  1511		bool dma = false;
  1512	
  1513		DPRINTK("read log page - log 0x%x, page 0x%x\n", log, page);
  1514	
  1515		/*
  1516		 * Return error without actually issuing the
  1517		 * command on controller which e.g. lockup
  1518		 * on a read log page.
  1519		 */
> 1520		if (flags & ATA_FLAG_NO_LOG_PAGE)
  1521			return AC_ERR_DEV;
  1522	
  1523	retry:

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 29818 bytes --]

[PATCH 2/2] ata/sata_fsl.c: add ATA_FLAG_NO_LOG_PAGE to blacklist the controller for log page reads

[Andreas Werner]

Every attempt to issue a read log page command lockup the controller.
The command is currently sent if the sata device includes the devlsp feature
to read out the timing data.
This attempt to read the data, locks up the controller and the device
is not recognzied correctly (failed to set xfermode) and cannot be accessed.

This was found on Freescale P1013/P1022 and T4240 CPUs
using a ATP IG mSATA 4GB with the devslp feature.

fsl-sata ff718000.sata: Sata FSL Platform/CSB Driver init
[    1.254195] scsi0 : sata_fsl
[    1.256004] ata1: SATA max UDMA/133 irq 74
[    1.370666] fsl-gianfar ethernet.3: enabled errata workarounds, flags: 0x4
[    1.470671] fsl-gianfar ethernet.4: enabled errata workarounds, flags: 0x4
[    1.775584] ata1: Signature Update detected @ 504 msecs
[    1.947594] ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
[    1.948366] ata1.00: ATA-8: ATP IG mSATA, 20150311, max UDMA/133
[    1.948371] ata1.00: 7732368 sectors, multi 0: LBA
[    1.948843] ata1.00: failed to get Identify Device Data, Emask 0x1
[    1.948857] ata1.00: failed to set xfermode (err_mask=0x40)
[    7.467557] ata1: Signature Update detected @ 504 msecs
[    7.639560] ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
[    7.651320] ata1.00: failed to get Identify Device Data, Emask 0x1
[    7.651360] ata1.00: failed to set xfermode (err_mask=0x40)
[    7.655628] ata1: limiting SATA link speed to 1.5 Gbps
[    7.659458] ata1.00: limiting speed to UDMA/133:PIO3
[   13.163554] ata1: Signature Update detected @ 504 msecs
[   13.335558] ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
[   13.347298] ata1.00: failed to get Identify Device Data, Emask 0x1
[   13.347334] ata1.00: failed to set xfermode (err_mask=0x40)
[   13.351601] ata1.00: disabled
[   13.353278] ata1: exception Emask 0x50 SAct 0x0 SErr 0x800 action 0x6 frozen t4
[   13.359281] ata1: SError: { HostInt }
[   13.361644] ata1: hard resetting link

Signed-off-by: Andreas Werner <andreas.werner@men.de>
---
 drivers/ata/sata_fsl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index 5389579..a723ae9 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -45,7 +45,8 @@ enum {
 	SATA_FSL_MAX_PRD_DIRECT	= 16,	/* Direct PRDT entries */
 
 	SATA_FSL_HOST_FLAGS	= (ATA_FLAG_SATA | ATA_FLAG_PIO_DMA |
-				ATA_FLAG_PMP | ATA_FLAG_NCQ | ATA_FLAG_AN),
+				   ATA_FLAG_PMP | ATA_FLAG_NCQ |
+				   ATA_FLAG_AN | ATA_FLAG_NO_LOG_PAGE),
 
 	SATA_FSL_MAX_CMDS	= SATA_FSL_QUEUE_DEPTH,
 	SATA_FSL_CMD_HDR_SIZE	= 16,	/* 4 DWORDS */
-- 
2.6.2