Linux ATA/IDE development
 help / color / mirror / Atom feed
* Re: Race to power off harming SATA SSDs
From: Boris Brezillon @ 2017-05-08 11:55 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Pavel Machek, David Woodhouse, linux-scsi@vger.kernel.org,
	Hans de Goede, LKML, linux-ide, linux-mtd@lists.infradead.org,
	Henrique de Moraes Holschuh, Tejun Heo
In-Reply-To: <20170508134807.498b3ee7@bbrezillon>

On Mon, 8 May 2017 13:48:07 +0200
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> On Mon, 8 May 2017 13:06:17 +0200
> Richard Weinberger <richard.weinberger@gmail.com> wrote:
> 
> > On Mon, May 8, 2017 at 12:49 PM, Pavel Machek <pavel@ucw.cz> wrote:  
> > > Aha, nice, so it looks like ubifs is a step back here.
> > >
> > > 'clean marker' is a good idea... empty pages have plenty of space.    
> > 
> > If UBI (not UBIFS) faces an empty block, it also re-erases it.  
> 
> Unfortunately, that's not the case, though UBI can easily be patched
> to do that (see below).

Sorry for the noise, I was wrong, UBI already re-erases empty blocks
[1].

[1]http://elixir.free-electrons.com/linux/latest/source/drivers/mtd/ubi/attach.c#L983

^ permalink raw reply

* Re: Race to power off harming SATA SSDs
From: Richard Weinberger @ 2017-05-08 12:13 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Pavel Machek, David Woodhouse, linux-scsi@vger.kernel.org,
	Hans de Goede, LKML, linux-ide, linux-mtd@lists.infradead.org,
	Henrique de Moraes Holschuh, Tejun Heo
In-Reply-To: <20170508134807.498b3ee7@bbrezillon>

Boris,

Am 08.05.2017 um 13:48 schrieb Boris Brezillon:
>>> How do you handle the issue during regular write? Always ignore last
>>> successfully written block?  
> 
> I guess UBIFS can know what was written last, because of the log-based
> approach + the seqnum stored along with FS nodes, but I'm pretty sure
> UBIFS does not re-write the last written block in case of an unclean
> mount. Richard, am I wrong?

Yes. UBIFS has the machinery but uses it differently. When it faces ECC
errors while replying the journal it can recover good data from the LEB.
It assumes that an interrupted write leads always to ECC errors.

>>
>> The last page of a block is inspected and allowed to be corrupted.
> 
> Actually, it's not really about corrupted pages, it's about pages that
> might become unreadable after a few reads.

As stated before, it assumes an ECC error from an interrupted read.

We could automatically re-write everything in UBIFS that was written last
but we don't have this information for data UBI itself wrote since UBI has
no journal.

If unstable bit can be triggered with current systems we can think of a clever
trick to deal with that. So far nobody was able to show me the problem.

Thanks,
//richard

^ permalink raw reply

* Re: Race to power off harming SATA SSDs
From: Pavel Machek @ 2017-05-08 12:32 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Tejun Heo, Henrique de Moraes Holschuh, linux-kernel, linux-scsi,
	linux-ide, Hans de Goede, boris.brezillon, linux-mtd
In-Reply-To: <1494241760.5407.6.camel@infradead.org>

Hi!

> > 'clean marker' is a good idea... empty pages have plenty of space.
> 
> Well... you lose that space permanently. Although I suppose you could
> do things differently and erase a block immediately prior to using it.
> But in that case why ever write the cleanmarker? Just maintain a set of
> blocks that you *will* erase and re-use.

Yes, but erase is slow so that would hurt performance...?

> > How do you handle the issue during regular write? Always ignore last
> > successfully written block?
> 
> Log nodes have a CRC. If you get interrupted during a write, that CRC
> should fail.

Umm. That is not what "unstable bits" issue is about, right?

If you are interrupted during write, you can get into state where
readback will be correct on next boot (CRC, ECC ok), but then the bits
will go back few hours after that. You can't rely on checksums to
detect that.. because the bits will have the right values -- for a while.

> > Do you handle "paired pages" problem on MLC?
> 
> No. It would theoretically be possible, by not considering a write to
> the first page "committed" until the second page of the pair is also
> written. Essentially, it's not far off expanding the existing 'wbuf'
> which we use to gather writes into full pages for NAND, to cover the
> *whole* of the set of pages which are affected by MLC.
> 
> But we mostly consider JFFS2 to be obsolete these days, in favour of
> UBI/UBIFS or other approaches.

Yes, I guess MLC NAND chips are mostly too big for jjfs2.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* 29369
From: f-alfig @ 2017-05-08 12:50 UTC (permalink / raw)
  To: linux-ide

[-- Attachment #1: 609590698025.zip --]
[-- Type: application/zip, Size: 4950 bytes --]

^ permalink raw reply

* Re: Race to power off harming SATA SSDs
From: David Woodhouse @ 2017-05-08 15:40 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Hans de Goede, Ricard Wanderlof, Pavel Machek, Tejun Heo,
	linux-scsi, linux-kernel, linux-ide, linux-mtd,
	Henrique de Moraes Holschuh
In-Reply-To: <20170508135005.0b9b200b@bbrezillon>

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

On Mon, 2017-05-08 at 13:50 +0200, Boris Brezillon wrote:
> On Mon, 08 May 2017 11:13:10 +0100
> David Woodhouse <dwmw2@infradead.org> wrote:
> 
> > 
> > On Mon, 2017-05-08 at 11:09 +0200, Hans de Goede wrote:
> > > 
> > > You're forgetting that the SSD itself (this thread is about SSDs) also has
> > > a major software component which is doing housekeeping all the time, so even
> > > if the main CPU gets reset the SSD's controller may still happily be erasing
> > > blocks.  
> > We're not really talking about SSDs at all any more; we're talking
> > about real flash with real maintainable software.
>
> It's probably a good sign that this new discussion should take place in
> a different thread :-).

Well, maybe. But it was a silly thread in the first place. SATA SSDs
aren't *expected* to be reliable.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 4938 bytes --]

^ permalink raw reply

* Re: Race to power off harming SATA SSDs
From: Pavel Machek @ 2017-05-08 16:43 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: David Woodhouse, Hans de Goede, Ricard Wanderlof, Tejun Heo,
	linux-scsi, linux-kernel, linux-ide, linux-mtd,
	Henrique de Moraes Holschuh
In-Reply-To: <20170508135005.0b9b200b@bbrezillon>

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

On Mon 2017-05-08 13:50:05, Boris Brezillon wrote:
> On Mon, 08 May 2017 11:13:10 +0100
> David Woodhouse <dwmw2@infradead.org> wrote:
> 
> > On Mon, 2017-05-08 at 11:09 +0200, Hans de Goede wrote:
> > > You're forgetting that the SSD itself (this thread is about SSDs) also has
> > > a major software component which is doing housekeeping all the time, so even
> > > if the main CPU gets reset the SSD's controller may still happily be erasing
> > > blocks.  
> > 
> > We're not really talking about SSDs at all any more; we're talking
> > about real flash with real maintainable software.
> 
> It's probably a good sign that this new discussion should take place in
> a different thread :-).

Well, you are right.. and I'm responsible.

What I was trying to point out was that storage people try to treat
SSDs as HDDs... and SSDs are very different. Harddrives mostly survive
powerfails (with emergency parking), while it is very, very difficult
to make SSD survive random powerfail, and we have to make sure we
always powerdown SSDs "cleanly".

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: Race to power off harming SATA SSDs
From: Tejun Heo @ 2017-05-08 17:43 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Boris Brezillon, David Woodhouse, Hans de Goede, Ricard Wanderlof,
	linux-scsi, linux-kernel, linux-ide, linux-mtd,
	Henrique de Moraes Holschuh
In-Reply-To: <20170508164322.GA9781@amd>

Hello,

On Mon, May 08, 2017 at 06:43:22PM +0200, Pavel Machek wrote:
> What I was trying to point out was that storage people try to treat
> SSDs as HDDs... and SSDs are very different. Harddrives mostly survive
> powerfails (with emergency parking), while it is very, very difficult
> to make SSD survive random powerfail, and we have to make sure we
> always powerdown SSDs "cleanly".

We do.

The issue raised is that some SSDs still increment the unexpected
power loss count even after clean shutdown sequence and that the
kernel should wait for some secs before powering off.

We can do that for select devices but I want something more than "this
SMART counter is getting incremented" before doing that.

Thanks.

-- 
tejun

^ permalink raw reply

* RE: Race to power off harming SATA SSDs
From: Atlant Schmidt @ 2017-05-08 18:29 UTC (permalink / raw)
  To: Pavel Machek, Boris Brezillon
  Cc: Ricard Wanderlof, linux-scsi@vger.kernel.org,
	linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org,
	Hans de Goede, linux-mtd@lists.infradead.org,
	Henrique de Moraes Holschuh, Tejun Heo, David Woodhouse
In-Reply-To: <20170508164322.GA9781@amd>

> Well, you are right.. and I'm responsible.
>
> What I was trying to point out was that storage people try to treat SSDs as HDDs...
> and SSDs are very different. Harddrives mostly survive powerfails (with emergency
> parking), while it is very, very difficult to make SSD survive random powerfail,
> and we have to make sure we always powerdown SSDs "cleanly".

  It all depends on the class of SSD that we're discussing.
  "Enterprise class" SSDs will often use either ultracapacitors
  or batteries to allow them to successfully complete all of
  the necessary operations upon a power cut.

This e-mail and the information, including any attachments it contains, are intended to be a confidential communication only to the person or entity to whom it is addressed and may contain information that is privileged. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please immediately notify the sender and destroy the original message. Thank you. Please consider the environment before printing this email.

^ permalink raw reply

* Re: Race to power off harming SATA SSDs
From: Pavel Machek @ 2017-05-08 18:56 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Boris Brezillon, David Woodhouse, Hans de Goede, Ricard Wanderlof,
	linux-scsi, linux-kernel, linux-ide, linux-mtd,
	Henrique de Moraes Holschuh
In-Reply-To: <20170508174303.GA12079@htj.duckdns.org>

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

On Mon 2017-05-08 13:43:03, Tejun Heo wrote:
> Hello,
> 
> On Mon, May 08, 2017 at 06:43:22PM +0200, Pavel Machek wrote:
> > What I was trying to point out was that storage people try to treat
> > SSDs as HDDs... and SSDs are very different. Harddrives mostly survive
> > powerfails (with emergency parking), while it is very, very difficult
> > to make SSD survive random powerfail, and we have to make sure we
> > always powerdown SSDs "cleanly".
> 
> We do.
> 
> The issue raised is that some SSDs still increment the unexpected
> power loss count even after clean shutdown sequence and that the
> kernel should wait for some secs before powering off.
> 
> We can do that for select devices but I want something more than "this
> SMART counter is getting incremented" before doing that.

Well... the SMART counter tells us that the device was not shut down
correctly. Do we have reason to believe that it is _not_ telling us
truth? It is more than one device.

SSDs die when you power them without warning:
http://lkcl.net/reports/ssd_analysis.html

What kind of data would you like to see? "I have been using linux and
my SSD died"? We have had such reports. "I have killed 10 SSDs in a
week then I added one second delay, and this SSD survived 6 months"?


									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: Race to power off harming SATA SSDs
From: Tejun Heo @ 2017-05-08 19:04 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Boris Brezillon, David Woodhouse, Hans de Goede, Ricard Wanderlof,
	linux-scsi, linux-kernel, linux-ide, linux-mtd,
	Henrique de Moraes Holschuh
In-Reply-To: <20170508185615.GA16268@amd>

Hello,

On Mon, May 08, 2017 at 08:56:15PM +0200, Pavel Machek wrote:
> Well... the SMART counter tells us that the device was not shut down
> correctly. Do we have reason to believe that it is _not_ telling us
> truth? It is more than one device.

It also finished power off command successfully.

> SSDs die when you power them without warning:
> http://lkcl.net/reports/ssd_analysis.html
> 
> What kind of data would you like to see? "I have been using linux and
> my SSD died"? We have had such reports. "I have killed 10 SSDs in a
> week then I added one second delay, and this SSD survived 6 months"?

Repeating shutdown cycles and showing that the device actually is in
trouble would be great.  It doesn't have to reach full-on device
failure.  Showing some sign of corruption would be enough - increase
in CRC failure counts, bad block counts (a lot of devices report
remaining reserve or lifetime in one way or the other) and so on.
Right now, it might as well be just the SMART counter being funky.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH 1/4] ata: Add DT bindings for Faraday Technology FTIDE010
From: Linus Walleij @ 2017-05-08 20:26 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Tejun Heo, linux-ide-u79uwXL29TY76Z2rM5mHXA, Janos Laube,
	Paulius Zaleckas, openwrt-devel-p3rKhJxN3npAfugRpC6u6w,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Hans Ulli Kroll, Florian Fainelli,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	John Feng-Hsin Chiang, Greentime Hu
In-Reply-To: <1678111.DmiVNKW3dQ@amdc3058>

On Mon, May 8, 2017 at 12:47 PM, Bartlomiej Zolnierkiewicz
<b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:

> Also for all current drivers we just put timing values (or a logic
> to calculate them from the standard ATA timings) into the driver
> itself and not device tree (as they are based on values are dictated
> by ATA standard and should not change for a given controller type).

I had it like that at first (and I can of course switch it back). But I
came to think this is better.

I was looking at these values from the point that it depends a bit
on the silicon where it is synthesized. So the vendor tree has
things like this:

#ifndef SL2312_FPGA_IDE
static unsigned char PIO_TIMING[5] = { 0xaa, 0xa3, 0xa1, 0x33, 0x31 };
static unsigned char TIMING_MDMA_50M[3] = { 0x66, 0x22, 0x21 };
static unsigned char TIMING_MDMA_66M[3] = { 0x88, 0x32, 0x31 };
static unsigned char TIMING_UDMA_50M[6] = { 0x33, 0x31, 0x21, 0x21,
0x11, 0x91 };
static unsigned char TIMING_UDMA_66M[7] = { 0x44, 0x42, 0x31, 0x21,
0x11, 0x91,  0x91};
#else
static unsigned char PIO_TIMING[5] = { 0x88, 0x82, 0x81, 0x32, 0x21 };
static unsigned char TIMING_MDMA_50M[3] = { 0x33, 0x11, 0x11 };
static unsigned char TIMING_MDMA_66M[3] = { 0x33, 0x11, 0x11 };
static unsigned char TIMING_UDMA_50M[6] = { 0x22, 0x11, 0x11, 0x11 };
static unsigned char TIMING_UDMA_66M[7] = { 0x22, 0x11, 0x11, 0x11 };
#endif

(From D-Link DIR-685 source release from Storlink/Cortina board support.)

So depending on whether they use an FPGA or an ASIC the values are
different, no matter what frequency (50 or 66 MHz) is used. So it is not
derived from frequency.

So I think it makes most sense to have it in the device tree as we don't
know what designs are out there.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/4] ata: Add DT bindings for the Gemini SATA bridge
From: Linus Walleij @ 2017-05-08 20:33 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Tejun Heo, linux-ide, Janos Laube, Paulius Zaleckas,
	openwrt-devel, linux-arm-kernel@lists.infradead.org,
	Hans Ulli Kroll, Florian Fainelli, devicetree@vger.kernel.org,
	John Feng-Hsin Chiang, Greentime Hu
In-Reply-To: <3791456.ZibJefW5cI@amdc3058>

On Mon, May 8, 2017 at 12:49 PM, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
> On Saturday, May 06, 2017 02:10:51 PM Linus Walleij wrote:

>> +  Mode 3: ata0 master <-> sata0
>> +          ata1 slave  <-> sata1
>
> ata0 slave?
>
>> +          ata1 master and slave interfaces brought out
>> +               on IDE pads

Of course. Thanks for reading close, much appreciated!

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH 2/4] ata: Add DT bindings for the Gemini SATA bridge
From: Tom Psyborg @ 2017-05-08 21:16 UTC (permalink / raw)
  To: Linus Walleij
  Cc: openwrt-devel, devicetree@vger.kernel.org, John Feng-Hsin Chiang,
	Paulius Zaleckas, Bartlomiej Zolnierkiewicz, linux-ide,
	Greentime Hu, Tejun Heo, Janos Laube,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <CACRpkdYz9Oka0SGJDTr=y4EN-VfggbuKY2s5X43oXg1dgjh1nw@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 758 bytes --]

Is it ever going to be added so this endless spam can end?


On 8 May 2017 at 22:33, Linus Walleij <linus.walleij@linaro.org> wrote:

> On Mon, May 8, 2017 at 12:49 PM, Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
> > On Saturday, May 06, 2017 02:10:51 PM Linus Walleij wrote:
>
> >> +  Mode 3: ata0 master <-> sata0
> >> +          ata1 slave  <-> sata1
> >
> > ata0 slave?
> >
> >> +          ata1 master and slave interfaces brought out
> >> +               on IDE pads
>
> Of course. Thanks for reading close, much appreciated!
>
> Yours,
> Linus Walleij
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
>

[-- Attachment #1.2: Type: text/html, Size: 1441 bytes --]

[-- Attachment #2: Type: text/plain, Size: 172 bytes --]

_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

^ permalink raw reply

* Re: Race to power off harming SATA SSDs
From: Pavel Machek @ 2017-05-08 21:36 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Boris Brezillon, Hans de Goede, Ricard Wanderlof, Tejun Heo,
	linux-scsi, linux-kernel, linux-ide, linux-mtd,
	Henrique de Moraes Holschuh
In-Reply-To: <1494258011.5407.18.camel@infradead.org>

On Mon 2017-05-08 16:40:11, David Woodhouse wrote:
> On Mon, 2017-05-08 at 13:50 +0200, Boris Brezillon wrote:
> > On Mon, 08 May 2017 11:13:10 +0100
> > David Woodhouse <dwmw2@infradead.org> wrote:
> > 
> > > 
> > > On Mon, 2017-05-08 at 11:09 +0200, Hans de Goede wrote:
> > > > 
> > > > You're forgetting that the SSD itself (this thread is about SSDs) also has
> > > > a major software component which is doing housekeeping all the time, so even
> > > > if the main CPU gets reset the SSD's controller may still happily be erasing
> > > > blocks.  
> > > We're not really talking about SSDs at all any more; we're talking
> > > about real flash with real maintainable software.
> >
> > It's probably a good sign that this new discussion should take place in
> > a different thread :-).
> 
> Well, maybe. But it was a silly thread in the first place. SATA SSDs
> aren't *expected* to be reliable.

Citation needed?

I'm pretty sure SATA SSDs are expected to be reliable, up to maximum
amount of gigabytes written (specified by manufacturer), as long as
you don't cut power without warning.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [PATCH] ide: don't call memcpy with the same source and destination
From: David Miller @ 2017-05-08 21:37 UTC (permalink / raw)
  To: mpatocka; +Cc: linux-ide, linux-kernel, jejb, deller, dave.anglin, linux-parisc
In-Reply-To: <alpine.LRH.2.02.1704141433280.10706@file01.intranet.prod.int.rdu2.redhat.com>

From: Mikulas Patocka <mpatocka@redhat.com>
Date: Fri, 14 Apr 2017 14:35:33 -0400 (EDT)

> The parisc architecture recently reimplemented the memcpy function and
> their reimplementation crashed when source and destination overlapped.
> 
> The crash happened in the function ide_complete_cmd where memcpy is called
> with the same source and destination pointer. According to the C
> specification, memcpy behavior is undefined if the source and destination
> range overlaps. This patches fixes the undefined behavior.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

Applied.

^ permalink raw reply

* Re: [PATCH] ide: use setup_timer
From: David Miller @ 2017-05-08 21:37 UTC (permalink / raw)
  To: geliangtang; +Cc: linux-ide, linux-kernel
In-Reply-To: <964230281324b7c18ec70296a8a3cd1caae13cb6.1490602149.git.geliangtang@gmail.com>

From: Geliang Tang <geliangtang@gmail.com>
Date: Sun,  9 Apr 2017 09:41:32 +0800

> Use setup_timer() instead of init_timer() to simplify the code.
> 
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>

Applied.

^ permalink raw reply

* Re: [OpenWrt-Devel] [PATCH 2/4] ata: Add DT bindings for the Gemini SATA bridge
From: Florian Fainelli @ 2017-05-08 22:52 UTC (permalink / raw)
  To: Tom Psyborg, Linus Walleij
  Cc: openwrt-devel, devicetree@vger.kernel.org, John Feng-Hsin Chiang,
	Paulius Zaleckas, Bartlomiej Zolnierkiewicz, linux-ide,
	Greentime Hu, Tejun Heo, Janos Laube,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAKR_QVJTbLMzqsUTEZJOquZ9T5aQ1_=73WAoRWrm+QK9wFhngA@mail.gmail.com>

On 05/08/2017 02:16 PM, Tom Psyborg wrote:
> Is it ever going to be added so this endless spam can end?

It's the first iteration of the (S)ATA patchset, and if you are not
interested, just ignore the thread.

Linus is doing everyone a great favor here by making sure that this
platform gets properly supported upstream such that the cost of
maintaining in OpenWrt/LEDE/anywhere else comes down to almost zero.

Almost forgot: please don't top post.

> 
> 
> On 8 May 2017 at 22:33, Linus Walleij <linus.walleij@linaro.org
> <mailto:linus.walleij@linaro.org>> wrote:
> 
>     On Mon, May 8, 2017 at 12:49 PM, Bartlomiej Zolnierkiewicz
>     <b.zolnierkie@samsung.com <mailto:b.zolnierkie@samsung.com>> wrote:
>     > On Saturday, May 06, 2017 02:10:51 PM Linus Walleij wrote:
> 
>     >> +  Mode 3: ata0 master <-> sata0
>     >> +          ata1 slave  <-> sata1
>     >
>     > ata0 slave?
>     >
>     >> +          ata1 master and slave interfaces brought out
>     >> +               on IDE pads
> 
>     Of course. Thanks for reading close, much appreciated!
> 
>     Yours,
>     Linus Walleij
>     _______________________________________________
>     openwrt-devel mailing list
>     openwrt-devel@lists.openwrt.org <mailto:openwrt-devel@lists.openwrt.org>
>     https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
>     <https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel>
> 
> 
> 
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
> 


-- 
Florian

^ permalink raw reply

* [PATCH] ahci: Acer SA5-271 SSD Not Detected Fix
From: Sui Chen @ 2017-05-09  6:20 UTC (permalink / raw)
  To: linux-ide; +Cc: tj

On the Acer Switch Alpha 12 (model number: SA5-271), the internal SSD may not
get detected because the port_map and CAP.nr_ports combination causes the driver
to skip the port that is actually connected to the SSD.

This problem occurs occasionally. When this problem occurs, CAP may hold a
value of 0xC734FF00 or 0xC734FF01 and port_map may hold a value of 0x00 or 0x01.
When this problem does not occur, CAP holds a value of 0xC734FF02 and port_map
may hold a value of 0x07. Overriding the CAP value to 0xC734FF02 and port_map to
0x7 noticeably reduces the occurrences of this problem.

Link: https://bugzilla.kernel.org/attachment.cgi?id=253091
Signed-off-by: Sui Chen <suichen6@gmail.com>
Tested-by: Damian Ivanov <damianatorrpm@gmail.com>
---
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 2fc5240..6b29e64 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1364,6 +1364,39 @@ static inline void ahci_gtf_filter_workaround(struct ata_host *host)
 {}
 #endif

+/*
+ * On the Acer Aspire Switch Alpha 12, sometimes all SATA channels
+ * are detected as DUMMY. When this happens, the last byte of the CAP
+ * register becomes 0x00 or 0x01, wrongly indicating there are only
+ * 1 or 2 SATA channels in the system.
+ *
+ * Overriding the last byte in the CAP register to 0x02 (which
+ * indicates to 3 channels) resolves this problem.
+ *
+ * https://bugzilla.kernel.org/show_bug.cgi?id=189471
+ */
+static void acpi_sa5_271_workaround(struct ahci_host_priv *hpriv,
+				    struct pci_dev *pdev)
+{
+	static const struct dmi_system_id sysids[] = {
+		{
+			.ident = "Acer Switch Alpha 12",
+			.matches = {
+				DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
+				DMI_MATCH(DMI_PRODUCT_NAME, "Switch SA5-271")
+			},
+		},
+		{ }
+	};
+	if (true || dmi_check_system(sysids)) {
+		dev_info(&pdev->dev, "enabling Acer Switch Alpha 12 workaround\n");
+		if ((hpriv->saved_cap & 0xC734FF00) == 0xC734FF00) {
+			hpriv->port_map = 0x7;
+			hpriv->cap = 0xC734FF02;
+		}
+	}
+}
+
 #ifdef CONFIG_ARM64
 /*
  * Due to ERRATA#22536, ThunderX needs to handle HOST_IRQ_STAT differently.
@@ -1636,6 +1669,10 @@ static int ahci_init_one(struct pci_dev *pdev, const struct
pci_device_id *ent)
 			 "online status unreliable, applying workaround\n");
 	}

+
+	/* Acer SA5-271 workaround modifies private_data */
+	acpi_sa5_271_workaround(hpriv, pdev);
+
 	/* CAP.NP sometimes indicate the index of the last enabled
 	 * port, at other times, that of the last possible port, so
 	 * determining the maximum port number requires looking at

^ permalink raw reply related

* Re: [OpenWrt-Devel] [PATCH 2/4] ata: Add DT bindings for the Gemini SATA bridge
From: Linus Walleij @ 2017-05-09  6:39 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Tom Psyborg, openwrt-devel, devicetree@vger.kernel.org,
	John Feng-Hsin Chiang, Paulius Zaleckas,
	Bartlomiej Zolnierkiewicz, linux-ide, Greentime Hu, Tejun Heo,
	Janos Laube, linux-arm-kernel@lists.infradead.org
In-Reply-To: <6e0751d2-8f78-c584-2990-c05bc3090214@gmail.com>

On Tue, May 9, 2017 at 12:52 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 05/08/2017 02:16 PM, Tom Psyborg wrote:
>> Is it ever going to be added so this endless spam can end?
>
> It's the first iteration of the (S)ATA patchset, and if you are not
> interested, just ignore the thread.

I mailed with Tom and it turns out he thinks openwrt-devel is getting
spammed with these submissions.

It's true in a sense: the patches are targeted for upstream and not
for the openwrt repo.

It's no big deal, I don't want to unnecessarily increase traffic on
openwrt-devel if it is annoying to some.

> Linus is doing everyone a great favor here by making sure that this
> platform gets properly supported upstream such that the cost of
> maintaining in OpenWrt/LEDE/anywhere else comes down to almost zero.

I am porting to D-Link DIR-685 and DNS-313 as part of the
process, so we have two new high-volume routers/NAS boxes
as part of the process. I don't know how to fix the OpenWRT
install and builds for these in the end though.

It is actually probably an even bigger win though.

We have learnt that Faraday Technology is sprinkling silicon blocks
all over any silicon foundries close to Taiwan. Their stuff appear
to be in a lot of cheap routers, NAS etc. They use the number of
successful deployments of the IP block as a selling point. I guess
these guys are commonly called in to consult when kickstarting
silicon design, simply.

It turns out that this and other silicon vendors such as Grain Media,
Andestech, Moschip etc are using the same silicon blocks, so
a bunch of out-of-tree code is actually just duplicate implementations
of Faraday drivers... we already merged Gemini and MoxaArt in
the upstream kernel so we have a common interrupt chip, timer,
PCI driver, and now this IDE/ATA driver (not the FTIDE200 yet though).

So there is maybe not as much unique silicon
in the world as we have come to think, we need to pay attention
to how register maps look on different things.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH v1] ata: sata_rcar: Handle return value of clk_prepare_enable
From: Arvind Yadav @ 2017-05-09 10:30 UTC (permalink / raw)
  To: tj; +Cc: linux-ide, linux-kernel

Here, Clock enable can failed. So adding an error check for
clk_prepare_enable.

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/ata/sata_rcar.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/sata_rcar.c b/drivers/ata/sata_rcar.c
index 5d38245..7e789c1 100644
--- a/drivers/ata/sata_rcar.c
+++ b/drivers/ata/sata_rcar.c
@@ -890,7 +890,9 @@ static int sata_rcar_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "failed to get access to sata clock\n");
 		return PTR_ERR(priv->clk);
 	}
-	clk_prepare_enable(priv->clk);
+	ret = clk_prepare_enable(priv->clk);
+	if (ret)
+		return ret;
 
 	host = ata_host_alloc(&pdev->dev, 1);
 	if (!host) {
@@ -971,7 +973,9 @@ static int sata_rcar_resume(struct device *dev)
 	struct sata_rcar_priv *priv = host->private_data;
 	void __iomem *base = priv->base;
 
-	clk_prepare_enable(priv->clk);
+	int ret = clk_prepare_enable(priv->clk);
+	if (ret)
+		return ret;
 
 	/* ack and mask */
 	iowrite32(0, base + SATAINTSTAT_REG);
@@ -989,7 +993,9 @@ static int sata_rcar_restore(struct device *dev)
 	struct ata_host *host = dev_get_drvdata(dev);
 	struct sata_rcar_priv *priv = host->private_data;
 
-	clk_prepare_enable(priv->clk);
+	int ret = clk_prepare_enable(priv->clk);
+	if (ret)
+		return ret;
 
 	sata_rcar_setup_port(host);
 
-- 
1.9.1


^ permalink raw reply related

* [PATCH Resend] ahci: Acer SA5-271 SSD Not Detected Fix
From: Sui Chen @ 2017-05-09 12:47 UTC (permalink / raw)
  To: linux-ide; +Cc: tj

(Correction in this resend: fixed function name acer_sa5_271_workaround; fixed
 the always-true condition in the function; fixed description.)

On the Acer Switch Alpha 12 (model number: SA5-271), the internal SSD may not
get detected because the port_map and CAP.nr_ports combination causes the driver
to skip the port that is actually connected to the SSD. More specifically,
either all SATA ports are identified as DUMMY, or all ports get ``link down''
and never get up again.

This problem occurs occasionally. When this problem occurs, CAP may hold a
value of 0xC734FF00 or 0xC734FF01 and port_map may hold a value of 0x00 or 0x01.
When this problem does not occur, CAP holds a value of 0xC734FF02 and port_map
may hold a value of 0x07. Overriding the CAP value to 0xC734FF02 and port_map to
0x7 significantly reduces the occurrence of this problem.

Link: https://bugzilla.kernel.org/attachment.cgi?id=253091
Signed-off-by: Sui Chen <suichen6@gmail.com>
Tested-by: Damian Ivanov <damianatorrpm@gmail.com>
---
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 2fc5240..2b4bcb6 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1364,6 +1364,39 @@ static inline void ahci_gtf_filter_workaround(struct ata_host *host)
 {}
 #endif

+/*
+ * On the Acer Aspire Switch Alpha 12, sometimes all SATA ports are detected
+ * as DUMMY, or detected but eventually get a ``link down'' and never get up
+ * again. When this happens, CAP.NP may hold a value of 0x00 or 0x01, and the
+ * port_map may hold a value of 0x00.
+ *
+ * Overriding CAP.NP to 0x02 and the port_map to 0x7 will reveal all 3 ports
+ * and can significantly reduce the occurrence of the problem.
+ *
+ * https://bugzilla.kernel.org/show_bug.cgi?id=189471
+ */
+static void acer_sa5_271_workaround(struct ahci_host_priv *hpriv,
+				    struct pci_dev *pdev)
+{
+	static const struct dmi_system_id sysids[] = {
+		{
+			.ident = "Acer Switch Alpha 12",
+			.matches = {
+				DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
+				DMI_MATCH(DMI_PRODUCT_NAME, "Switch SA5-271")
+			},
+		},
+		{ }
+	};
+	if (dmi_check_system(sysids)) {
+		dev_info(&pdev->dev, "enabling Acer Switch Alpha 12 workaround\n");
+		if ((hpriv->saved_cap & 0xC734FF00) == 0xC734FF00) {
+			hpriv->port_map = 0x7;
+			hpriv->cap = 0xC734FF02;
+		}
+	}
+}
+
 #ifdef CONFIG_ARM64
 /*
  * Due to ERRATA#22536, ThunderX needs to handle HOST_IRQ_STAT differently.
@@ -1636,6 +1669,10 @@ static int ahci_init_one(struct pci_dev *pdev, const struct
pci_device_id *ent)
 			 "online status unreliable, applying workaround\n");
 	}

+
+	/* Acer SA5-271 workaround modifies private_data */
+	acer_sa5_271_workaround(hpriv, pdev);
+
 	/* CAP.NP sometimes indicate the index of the last enabled
 	 * port, at other times, that of the last possible port, so
 	 * determining the maximum port number requires looking at


^ permalink raw reply related

* Re: [PATCH Resend] ahci: Acer SA5-271 SSD Not Detected Fix
From: Tejun Heo @ 2017-05-09 14:20 UTC (permalink / raw)
  To: Sui Chen; +Cc: linux-ide
In-Reply-To: <086b549a-e2ec-58c4-42af-c5b5e23f67a7@gmail.com>

Hello, Sui.

On Tue, May 09, 2017 at 07:47:22AM -0500, Sui Chen wrote:
> (Correction in this resend: fixed function name acer_sa5_271_workaround; fixed
>  the always-true condition in the function; fixed description.)
> 
> On the Acer Switch Alpha 12 (model number: SA5-271), the internal SSD may not
> get detected because the port_map and CAP.nr_ports combination causes the driver
> to skip the port that is actually connected to the SSD. More specifically,
> either all SATA ports are identified as DUMMY, or all ports get ``link down''
> and never get up again.
> 
> This problem occurs occasionally. When this problem occurs, CAP may hold a
> value of 0xC734FF00 or 0xC734FF01 and port_map may hold a value of 0x00 or 0x01.
> When this problem does not occur, CAP holds a value of 0xC734FF02 and port_map
> may hold a value of 0x07. Overriding the CAP value to 0xC734FF02 and port_map to
> 0x7 significantly reduces the occurrence of this problem.
> 
> Link: https://bugzilla.kernel.org/attachment.cgi?id=253091
> Signed-off-by: Sui Chen <suichen6@gmail.com>
> Tested-by: Damian Ivanov <damianatorrpm@gmail.com>

Applied to libata/for-4.12-fixes w/ stable cc'd and a couple trivial
changes.

> @@ -1636,6 +1669,10 @@ static int ahci_init_one(struct pci_dev *pdev, const struct
> pci_device_id *ent)

The patch was new line damaged here.  You probably wanna make sure
that your mail client isn't breaking long lines automatically.

Thanks!

-- 
tejun

^ permalink raw reply

* Re: [PATCH v1] ata: sata_rcar: Handle return value of clk_prepare_enable
From: Tejun Heo @ 2017-05-09 14:31 UTC (permalink / raw)
  To: Arvind Yadav; +Cc: linux-ide, linux-kernel
In-Reply-To: <1494325828-2725-1-git-send-email-arvind.yadav.cs@gmail.com>

On Tue, May 09, 2017 at 04:00:28PM +0530, Arvind Yadav wrote:
> Here, Clock enable can failed. So adding an error check for
> clk_prepare_enable.
> 
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>

Applied to libata/for-4.12-fixes w/ a couple trivial changes.

Thanks.

-- 
tejun

^ permalink raw reply

* [GIT] IDE
From: David Miller @ 2017-05-09 20:04 UTC (permalink / raw)
  To: torvalds; +Cc: akpm, linux-ide, linux-kernel


Please pull in these two small cleanups in the IDE
layer.

Thanks!

The following changes since commit 2d3e4866dea96b0506395b47bfefb234f2088dac:

  Merge tag 'for-linus' of git://git.kernel.org/pub/scm/virt/kvm/kvm (2017-05-08 12:37:56 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/davem/ide.git 

for you to fetch changes up to acfead32f3f938e456d7d5e1a82c743d30b8b886:

  ide: don't call memcpy with the same source and destination (2017-05-08 17:36:39 -0400)

----------------------------------------------------------------
Geliang Tang (1):
      ide: use setup_timer

Mikulas Patocka (1):
      ide: don't call memcpy with the same source and destination

 drivers/ide/ide-io.c    | 2 +-
 drivers/ide/ide-probe.c | 4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

^ permalink raw reply

* Re: [PATCH v1] ata: sata_rcar: Handle return value of clk_prepare_enable
From: Sergei Shtylyov @ 2017-05-10  9:02 UTC (permalink / raw)
  To: Arvind Yadav, tj; +Cc: linux-ide, linux-kernel
In-Reply-To: <1494325828-2725-1-git-send-email-arvind.yadav.cs@gmail.com>

Hello.

On 5/9/2017 1:30 PM, Arvind Yadav wrote:

> Here, Clock enable can failed. So adding an error check for

    Maybe "here enabling the clock can fail"?

> clk_prepare_enable.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>

    Other than that:

Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

MBR, Sergei


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox