Linux ATA/IDE development
 help / color / mirror / Atom feed
* [PATCH v3 2/2] dt-bindings: ata: add DT bindings for MediaTek SATA controller
From: Ryder Lee @ 2017-08-12  1:31 UTC (permalink / raw)
  To: Hans de Goede, Tejun Heo, Rob Herring
  Cc: devicetree, linux-mediatek, linux-kernel, linux-ide, Long Cheng,
	Ryder Lee
In-Reply-To: <cover.1502501342.git.ryder.lee@mediatek.com>

Add DT bindings for the onboard SATA controller present on the MediaTek
SoCs.

Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
---
 Documentation/devicetree/bindings/ata/ahci-mtk.txt | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/ahci-mtk.txt

diff --git a/Documentation/devicetree/bindings/ata/ahci-mtk.txt b/Documentation/devicetree/bindings/ata/ahci-mtk.txt
new file mode 100644
index 0000000..96c8d22
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/ahci-mtk.txt
@@ -0,0 +1,51 @@
+MediaTek Serial ATA controller
+
+Required properties:
+ - compatible	   : Must be "mediatek,soc-model-ahci", "mediatek,mtk-ahci".
+		     When using "mediatek,mtk-ahci" compatible strings, you
+		     need SoC specific ones in addition, one of:
+		     - "mediatek,mt7622-ahci"
+ - reg		   : Physical base addresses and length of register sets.
+ - interrupts	   : Interrupt associated with the SATA device.
+ - interrupt-names : Associated name must be: "hostc".
+ - clocks	   : A list of phandle and clock specifier pairs, one for each
+		     entry in clock-names.
+ - clock-names	   : Associated names must be: "ahb", "axi", "asic", "rbc", "pm".
+ - phys		   : A phandle and PHY specifier pair for the PHY port.
+ - phy-names	   : Associated name must be: "sata-phy".
+ - ports-implemented : See ./ahci-platform.txt for details.
+
+Optional properties:
+ - power-domains   : A phandle and power domain specifier pair to the power
+		     domain which is responsible for collapsing and restoring
+		     power to the peripheral.
+ - resets	   : Must contain an entry for each entry in reset-names.
+		     See ../reset/reset.txt for details.
+ - reset-names	   : Associated names must be: "axi", "sw", "reg".
+ - mediatek,phy-mode : A phandle to the system controller, used to enable
+		       SATA function.
+
+Example:
+
+	sata: sata@1a200000 {
+		compatible = "mediatek,mt7622-ahci",
+			     "mediatek,mtk-ahci";
+		reg = <0 0x1a200000 0 0x1100>;
+		interrupts = <GIC_SPI 233 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "hostc";
+		clocks = <&pciesys CLK_SATA_AHB_EN>,
+			 <&pciesys CLK_SATA_AXI_EN>,
+			 <&pciesys CLK_SATA_ASIC_EN>,
+			 <&pciesys CLK_SATA_RBC_EN>,
+			 <&pciesys CLK_SATA_PM_EN>;
+		clock-names = "ahb", "axi", "asic", "rbc", "pm";
+		phys = <&u3port1 PHY_TYPE_SATA>;
+		phy-names = "sata-phy";
+		ports-implemented = <0x1>;
+		power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
+		resets = <&pciesys MT7622_SATA_AXI_BUS_RST>,
+			 <&pciesys MT7622_SATA_PHY_SW_RST>,
+			 <&pciesys MT7622_SATA_PHY_REG_RST>;
+		reset-names = "axi", "sw", "reg";
+		mediatek,phy-mode = <&pciesys>;
+	};
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH 000/102] Convert drivers to explicit reset API
From: Wolfram Sang @ 2017-08-12 11:43 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Andrew Lunn, Prashant Gaikwad, Heiko Stuebner, Peter Chen,
	Linus Walleij, DRI, Marc Dietrich, Rakesh Iyer,
	Peter Meerwald-Stadler, linux-clk, Xinliang Liu, Chanwoo Choi,
	Alan Stern, Jiri Slaby, Michael Turquette, Guenter Roeck,
	Ohad Ben-Cohen, linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Thomas Gleixner, Vincent Abriou, Bin Liu, Greg Kroah-Hartman,
	USB list
In-Reply-To: <1500885221.2391.50.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>


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


> Thanks for the hint and the references. It seems this turned out okay,
> but I wouldn't dare to introduce such macro horror^Wmagic.
> I'd rather have all users converted to the _exclusive/_shared function
> calls and maybe then replace the internal __reset_control_get with
> Thomas' suggestion.

I didn't follow the discussion closely. Shall I still apply the i2c
patches?


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply

* Re: [PATCH v3 1/2] ata: mediatek: add support for MediaTek SATA controller
From: Tejun Heo @ 2017-08-13 18:38 UTC (permalink / raw)
  To: Ryder Lee
  Cc: Hans de Goede, Rob Herring, devicetree, linux-mediatek,
	linux-kernel, linux-ide, Long Cheng
In-Reply-To: <cf04d6bf9cf27911023b962ccd8639f063d32739.1502501342.git.ryder.lee@mediatek.com>

On Sat, Aug 12, 2017 at 09:31:03AM +0800, Ryder Lee wrote:
> This adds support the AHCI-compliant Serial ATA controller present
> on MediaTek SoCs.
> 
> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>

Looks good to me.

 Acked-by: Tejun Heo <tj@kernel.org>

If others are okay with it, please let me know how the patches should
be routed.  I can push both through libata, both can be routed through
dt tree, or we can handle them separately.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH 000/102] Convert drivers to explicit reset API
From: Philipp Zabel @ 2017-08-14  7:36 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Linus Walleij, Dmitry Torokhov, Thomas Petazzoni, lkml,
	Andrew Lunn, Prashant Gaikwad, Heiko Stuebner, Peter Chen, DRI,
	Marc Dietrich, Rakesh Iyer, Peter Meerwald-Stadler, linux-clk,
	Wim Van Sebroeck, Xinliang Liu, Chanwoo Choi, Alan Stern,
	Jiri Slaby, Michael Turquette, Guenter
In-Reply-To: <20170812114357.v4ru75dw5hq7wemx@ninjato>

On Sat, 2017-08-12 at 13:43 +0200, Wolfram Sang wrote:
> > Thanks for the hint and the references. It seems this turned out
> > okay,
> > but I wouldn't dare to introduce such macro horror^Wmagic.
> > I'd rather have all users converted to the _exclusive/_shared
> > function
> > calls and maybe then replace the internal __reset_control_get with
> > Thomas' suggestion.
> 
> I didn't follow the discussion closely. Shall I still apply the i2c
> patches?

Yes, please.

regards
Philipp

^ permalink raw reply

* Re: [PATCH v3 1/2] ata: mediatek: add support for MediaTek SATA controller
From: Matthias Brugger @ 2017-08-14 11:41 UTC (permalink / raw)
  To: Tejun Heo, Ryder Lee
  Cc: devicetree, Hans de Goede, linux-kernel, linux-ide, Rob Herring,
	linux-mediatek, Long Cheng
In-Reply-To: <20170813183854.GA1438922@devbig577.frc2.facebook.com>



On 08/13/2017 08:38 PM, Tejun Heo wrote:
> On Sat, Aug 12, 2017 at 09:31:03AM +0800, Ryder Lee wrote:
>> This adds support the AHCI-compliant Serial ATA controller present
>> on MediaTek SoCs.
>>
>> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> 
> Looks good to me.
> 
>   Acked-by: Tejun Heo <tj@kernel.org>
> 
> If others are okay with it, please let me know how the patches should
> be routed.  I can push both through libata, both can be routed through
> dt tree, or we can handle them separately.
> 

Please take the device tree bindings through your tree as well.
You might want to wait for a Ack from a device tree maintainer though.

Thanks,
Matthias

^ permalink raw reply

* (unknown), 
From: mitch_128 @ 2017-08-15  4:40 UTC (permalink / raw)
  To: linux-ide

[-- Attachment #1: 9176321037.zip --]
[-- Type: application/zip, Size: 10388 bytes --]

^ permalink raw reply

* [PATCH] PNP: ide: constify pnp_device_id
From: Arvind Yadav @ 2017-08-16  4:54 UTC (permalink / raw)
  To: davem; +Cc: linux-kernel, linux-ide

pnp_device_id are not supposed to change at runtime. All functions
working with pnp_device_id provided by <linux/pnp.h> work with
const pnp_device_id. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/ide/ide-pnp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ide/ide-pnp.c b/drivers/ide/ide-pnp.c
index f5f2b62..859ddab 100644
--- a/drivers/ide/ide-pnp.c
+++ b/drivers/ide/ide-pnp.c
@@ -22,7 +22,7 @@
 #define DRV_NAME "ide-pnp"
 
 /* Add your devices here :)) */
-static struct pnp_device_id idepnp_devices[] = {
+static const struct pnp_device_id idepnp_devices[] = {
 	/* Generic ESDI/IDE/ATA compatible hard disk controller */
 	{.id = "PNP0600", .driver_data = 0},
 	{.id = ""}
-- 
2.7.4


^ permalink raw reply related

* [PATCH] sata: ahci-da850: Fix some error handling paths in 'ahci_da850_probe()'
From: Christophe JAILLET @ 2017-08-16  5:31 UTC (permalink / raw)
  To: tj; +Cc: linux-ide, linux-kernel, kernel-janitors, Christophe JAILLET

'rc' is known to be 0 at this point.
If 'platform_get_resource()' or 'devm_ioremap()' fail, return -ENOMEM
instead of 0 which means success.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/ata/ahci_da850.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
index 1a50cd3b4233..eb46cad4d514 100644
--- a/drivers/ata/ahci_da850.c
+++ b/drivers/ata/ahci_da850.c
@@ -216,12 +216,16 @@ static int ahci_da850_probe(struct platform_device *pdev)
 		return rc;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (!res)
+	if (!res) {
+		rc = -ENOMEM;
 		goto disable_resources;
+	}
 
 	pwrdn_reg = devm_ioremap(dev, res->start, resource_size(res));
-	if (!pwrdn_reg)
+	if (!pwrdn_reg) {
+		rc = -ENOMEM;
 		goto disable_resources;
+	}
 
 	da850_sata_init(dev, pwrdn_reg, hpriv->mmio, mpy);
 
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH] sata: ahci-da850: Fix some error handling paths in 'ahci_da850_probe()'
From: Sergei Shtylyov @ 2017-08-16  9:04 UTC (permalink / raw)
  To: Christophe JAILLET, tj; +Cc: linux-ide, linux-kernel, kernel-janitors
In-Reply-To: <20170816053103.19773-1-christophe.jaillet@wanadoo.fr>

Hello!

On 8/16/2017 8:31 AM, Christophe JAILLET wrote:

> 'rc' is known to be 0 at this point.
> If 'platform_get_resource()' or 'devm_ioremap()' fail, return -ENOMEM
> instead of 0 which means success.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
>   drivers/ata/ahci_da850.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c
> index 1a50cd3b4233..eb46cad4d514 100644
> --- a/drivers/ata/ahci_da850.c
> +++ b/drivers/ata/ahci_da850.c
> @@ -216,12 +216,16 @@ static int ahci_da850_probe(struct platform_device *pdev)
>   		return rc;
>   
>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> -	if (!res)
> +	if (!res) {
> +		rc = -ENOMEM;

    -ENODEV would fit better here.

[...]

MBR, Sergei

^ permalink raw reply

* Re: [PATCH] sata: ahci-da850: Fix some error handling paths in 'ahci_da850_probe()'
From: Tejun Heo @ 2017-08-16 14:40 UTC (permalink / raw)
  To: Christophe JAILLET
  Cc: linux-ide, linux-kernel, kernel-janitors, Sergei Shtylyov
In-Reply-To: <20170816053103.19773-1-christophe.jaillet@wanadoo.fr>

Hello,

On Wed, Aug 16, 2017 at 07:31:03AM +0200, Christophe JAILLET wrote:
> 'rc' is known to be 0 at this point.
> If 'platform_get_resource()' or 'devm_ioremap()' fail, return -ENOMEM
> instead of 0 which means success.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Applied to libata/for-4.13-fixes w/ Sergei's suggestion applied.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH] PNP: ide: constify pnp_device_id
From: David Miller @ 2017-08-16 18:10 UTC (permalink / raw)
  To: arvind.yadav.cs; +Cc: linux-kernel, linux-ide
In-Reply-To: <60c34272432b3411cd5d55728c62481e133f32a6.1502859040.git.arvind.yadav.cs@gmail.com>

From: Arvind Yadav <arvind.yadav.cs@gmail.com>
Date: Wed, 16 Aug 2017 10:24:32 +0530

> pnp_device_id are not supposed to change at runtime. All functions
> working with pnp_device_id provided by <linux/pnp.h> work with
> const pnp_device_id. So mark the non-const structs as const.
> 
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>

Applied.

^ permalink raw reply

* Re: No I/O errors reported after SATA link hard reset
From: Bernd Schubert @ 2017-08-17  9:24 UTC (permalink / raw)
  To: Tejun Heo, linux-ide; +Cc: Gionatan Danti, linux-scsi
In-Reply-To: <fe5cc200a4cba71cb9e5e6a980699805@assyoma.it>

[This seems to be libata error handling and not scsi, so I added more CCs]

On 08/17/2017 12:27 AM, Gionatan Danti wrote:
> Hi list,
> some time ago, I had a filesystem corruption on a simple, two disks
> RAID1 MD array. On the affected machine, /var/log/messages shown some
> "failed command: WRITE FPDMA QUEUED" entries, but *no* action (ie: kick
> off disk) was taken by MDRAID. I tracked down the problem to an instable
> power supply (switching power rail/connector solved the problem).
> 
> In the latest day I had some spare time and I am now able to regularly
> replicate the problem. Basically, when a short powerloss happens, the
> scsi midlayer logs some failed operations, but does *not* pass these
> errors to higher layer. In other words, no I/O error is returned to the
> calling application. This is the reason why MDRAID did not kick off the
> instable disk on the machine with corrupted filesystem.
> 
> To replicated the problem, I wrote a large random file on a small MD
> RAID1 array, pulling off the power of one disk from about 2 seconds. The
> file write operation stopped for some seconds, than recovered. Running
> an array check resulted in a high number of mismatch_cnt sectors. Dmesg
> logged the following lines:
> 
> Aug 16 16:04:02 blackhole kernel: ata6.00: exception Emask 0x50 SAct
> 0x7fffffff SErr 0x90a00 action 0xe frozen
> Aug 16 16:04:02 blackhole kernel: ata6.00: irq_stat 0x00400000, PHY RDY
> changed
> Aug 16 16:04:02 blackhole kernel: ata6: SError: { Persist HostInt
> PHYRdyChg 10B8B }
> Aug 16 16:04:02 blackhole kernel: ata6.00: failed command: WRITE FPDMA
> QUEUED
> Aug 16 16:04:02 blackhole kernel: ata6.00: cmd
> 61/00:00:10:82:09/04:00:00:00:00/40 tag 0 ncq 524288 out#012         res
> 40/00:d8:10:72:09/00:00:00:00:00/40 Emask 0x50 (ATA bus error)
> Aug 16 16:04:02 blackhole kernel: ata6.00: status: { DRDY }
> ...
> Aug 16 16:04:02 blackhole kernel: ata6.00: failed command: WRITE FPDMA
> QUEUED
> Aug 16 16:04:02 blackhole kernel: ata6.00: cmd
> 61/00:f0:10:7e:09/04:00:00:00:00/40 tag 30 ncq 524288 out#012        
> res 40/00:d8:10:72:09/00:00:00:00:00/40 Emask 0x50 (ATA bus error)
> Aug 16 16:04:02 blackhole kernel: ata6.00: status: { DRDY }
> Aug 16 16:04:02 blackhole kernel: ata6: hard resetting link
> Aug 16 16:04:03 blackhole kernel: ata6: SATA link down (SStatus 0
> SControl 310)
> Aug 16 16:04:04 blackhole kernel: ata6: hard resetting link
> Aug 16 16:04:14 blackhole kernel: ata6: softreset failed (device not ready)
> Aug 16 16:04:14 blackhole kernel: ata6: hard resetting link
> Aug 16 16:04:24 blackhole kernel: ata6: softreset failed (device not ready)
> Aug 16 16:04:24 blackhole kernel: ata6: hard resetting link
> Aug 16 16:04:35 blackhole kernel: ata6: link is slow to respond, please
> be patient (ready=0)
> Aug 16 16:04:42 blackhole kernel: ata6: SATA link down (SStatus 0
> SControl 310)
> Aug 16 16:04:46 blackhole kernel: ata6: hard resetting link
> Aug 16 16:04:46 blackhole kernel: ata3: exception Emask 0x10 SAct 0x0
> SErr 0x40d0202 action 0xe frozen
> Aug 16 16:04:46 blackhole kernel: ata3: irq_stat 0x00400000, PHY RDY
> changed
> Aug 16 16:04:46 blackhole kernel: ata3: SError: { RecovComm Persist
> PHYRdyChg CommWake 10B8B DevExch }
> Aug 16 16:04:46 blackhole kernel: ata3: hard resetting link
> Aug 16 16:04:51 blackhole kernel: ata3: softreset failed (device not ready)
> Aug 16 16:04:51 blackhole kernel: ata3: applying PMP SRST workaround and
> retrying
> Aug 16 16:04:51 blackhole kernel: ata3: SATA link up 3.0 Gbps (SStatus
> 123 SControl 300)
> Aug 16 16:04:51 blackhole kernel: ata3.00: configured for UDMA/133
> Aug 16 16:04:51 blackhole kernel: ata3: EH complete
> Aug 16 16:04:52 blackhole kernel: ata6: softreset failed (device not ready)
> Aug 16 16:04:52 blackhole kernel: ata6: applying PMP SRST workaround and
> retrying
> Aug 16 16:04:52 blackhole kernel: ata6: SATA link up 1.5 Gbps (SStatus
> 113 SControl 310)
> Aug 16 16:04:52 blackhole kernel: ata6.00: configured for UDMA/133
> Aug 16 16:04:52 blackhole kernel: ata6: EH complete
> 
> As you can see, while failed SATA operation were logged in dmesg (and
> /var/log/messages), no I/O errors where returned to the upper layer
> (MDRAID) or the calling application. I had to say that I *fully expect*
> some inconsistencies: after all, removing the power wipes the volatile
> disk's DRAM cache, which means data loss. However, I really expected
> some I/O errors to be thrown to the higher layers, causing visible
> reactions (ie: a disks pushed out the array). With no I/O errors
> returned, the higher layer application are effectively blind.
> 
> More concerning is the fact that these undetected errors can make their
> way even when the higher application consistently calls sync() and/or
> fsync. In other words, it seems than even acknowledged writes can fail
> in this manner (and this is consistent with the first machine corrupting
> its filesystem due to journal trashing - XFS journal surely uses sync()
> where appropriate). The mechanism seems the following:
> 
> - an higher layer application issue sync();
> - a write barrier is generated;
> - a first FLUSH CACHE command is sent to the disk;
> - data are written to the disk's DRAM cache;
> - power is lost! The volatile cache lose its content;
> - power is re-established and the disk become responsive again;
> - a second FLUSH CACHE command is sent to the disk;
> - the disk acks each SATA command, but real data are lost.
> 
> As a side note, when the power loss or SATA cable disconnection is
> relatively long (over 10 seconds, as by eh timeout), the SATA disks
> become disconnected (and the MD layer acts accordlying):
> 
> Aug 16 16:12:20 blackhole kernel: ata6.00: exception Emask 0x50 SAct
> 0x7fffffff SErr 0x490a00 action 0xe frozen
> Aug 16 16:12:20 blackhole kernel: ata6.00: irq_stat 0x08000000,
> interface fatal error
> Aug 16 16:12:20 blackhole kernel: ata6: SError: { Persist HostInt
> PHYRdyChg 10B8B Handshk }
> Aug 16 16:12:20 blackhole kernel: ata6.00: failed command: WRITE FPDMA
> QUEUED
> Aug 16 16:12:20 blackhole kernel: ata6.00: cmd
> 61/00:00:38:88:09/04:00:00:00:00/40 tag 0 ncq 524288 out#012         res
> 40/00:d8:38:f4:09/00:00:00:00:00/40 Emask 0x50 (ATA bus error)
> Aug 16 16:12:20 blackhole kernel: ata6.00: status: { DRDY }
> ...
> Aug 16 16:12:32 blackhole kernel: sd 5:0:0:0: [sdf] FAILED Result:
> hostbyte=DID_OK driverbyte=DRIVER_SENSE
> Aug 16 16:12:32 blackhole kernel: sd 5:0:0:0: [sdf] Sense Key : Illegal
> Request [current] [descriptor]
> Aug 16 16:12:32 blackhole kernel: sd 5:0:0:0: [sdf] Add. Sense:
> Unaligned write command
> Aug 16 16:12:32 blackhole kernel: sd 5:0:0:0: [sdf] CDB: Write(10) 2a 00
> 00 09 88 38 00 04 00 00
> Aug 16 16:12:32 blackhole kernel: blk_update_request: 23 callbacks
> suppressed
> Aug 16 16:12:32 blackhole kernel: blk_update_request: I/O error, dev
> sdf, sector 624696
> 
> Now, I have few questions:
> - is the above explanation plausible, or I am (horribly) missing something?
> - why the scsi midlevel does not respond to a power loss event by
> immediately offlining the disks?
> - is the scsi midlevel behavior configurable (I know I can lower eh
> timeout, but is this the right solution)?
> - how to deal with this problem (other than being 100% sure power is
> never lost by any disks)?


I added the ata mailing list and Tejun.
I already wanted to report the same issue, as a flaky cable caused
libata error handling on one of my systems at home. ATA EH succeeded for
several weeks until several file systems on that system reported
corruption (btrfs and ext4). Failed commands I can see from syslog are
"READ FPDMA QUEUED" and "FLUSH CACHE EXT", but I'm not sure if it is
complete, as the log file is on btrfs and it reports checksum mismatch
for that file. Kernel version is 4.4.0-81-ubuntu, I have not checked yet
if they applied any libata patches.


Thanks,
Bernd


^ permalink raw reply

* Re: No I/O errors reported after SATA link hard reset
From: Tejun Heo @ 2017-08-17 12:48 UTC (permalink / raw)
  To: Bernd Schubert; +Cc: linux-ide, Gionatan Danti, linux-scsi
In-Reply-To: <13561110-4e59-303a-8e3d-dd60c1bafba8@fastmail.fm>

Hello,

On Thu, Aug 17, 2017 at 11:24:22AM +0200, Bernd Schubert wrote:
> > More concerning is the fact that these undetected errors can make their
> > way even when the higher application consistently calls sync() and/or
> > fsync. In other words, it seems than even acknowledged writes can fail
> > in this manner (and this is consistent with the first machine corrupting
> > its filesystem due to journal trashing - XFS journal surely uses sync()
> > where appropriate). The mechanism seems the following:
> > 
> > - an higher layer application issue sync();
> > - a write barrier is generated;
> > - a first FLUSH CACHE command is sent to the disk;
> > - data are written to the disk's DRAM cache;
> > - power is lost! The volatile cache lose its content;
> > - power is re-established and the disk become responsive again;
> > - a second FLUSH CACHE command is sent to the disk;
> > - the disk acks each SATA command, but real data are lost.

Recovered errors aren't reported as IO errors and at least from link
state proper there's no way for the driver to tell apart link
glitches and buffer-erasing power issues.

> > Now, I have few questions:
> > - is the above explanation plausible, or I am (horribly) missing something?

For the most part, yes.  To be more accurate, the failure is coming
from libata not being able to tell apart link glitches from the device
getting reset due to power issues.

> > - why the scsi midlevel does not respond to a power loss event by
> > immediately offlining the disks?

Because we don't wanna be ditching disks on temporary link glitches,
which do happen once in a while.

> > - is the scsi midlevel behavior configurable (I know I can lower eh
> > timeout, but is this the right solution)?
> > - how to deal with this problem (other than being 100% sure power is
> > never lost by any disks)?

So, the right way to deal with the problem probably is making use of
the SMART counter which indicates power loss events and verify that
the counter hasn't increased over link issues.  If it changed, the
device should be detached and re-probed, which will make it come back
as a different block device.  Unfortunately, I haven't had the chance
to actually implement that.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: No I/O errors reported after SATA link hard reset
From: Bernd Schubert @ 2017-08-17 13:18 UTC (permalink / raw)
  To: Tejun Heo, Bernd Schubert; +Cc: linux-ide, Gionatan Danti, linux-scsi
In-Reply-To: <20170817124821.GA3238792@devbig577.frc2.facebook.com>

Hello Tejun,

On 08/17/2017 02:48 PM, Tejun Heo wrote:
> Hello,
> 
> On Thu, Aug 17, 2017 at 11:24:22AM +0200, Bernd Schubert wrote:
>>> More concerning is the fact that these undetected errors can make their
>>> way even when the higher application consistently calls sync() and/or
>>> fsync. In other words, it seems than even acknowledged writes can fail
>>> in this manner (and this is consistent with the first machine corrupting
>>> its filesystem due to journal trashing - XFS journal surely uses sync()
>>> where appropriate). The mechanism seems the following:
>>>
>>> - an higher layer application issue sync();
>>> - a write barrier is generated;
>>> - a first FLUSH CACHE command is sent to the disk;
>>> - data are written to the disk's DRAM cache;
>>> - power is lost! The volatile cache lose its content;
>>> - power is re-established and the disk become responsive again;
>>> - a second FLUSH CACHE command is sent to the disk;
>>> - the disk acks each SATA command, but real data are lost.
> 
> Recovered errors aren't reported as IO errors and at least from link
> state proper there's no way for the driver to tell apart link
> glitches and buffer-erasing power issues.
> 
>>> Now, I have few questions:
>>> - is the above explanation plausible, or I am (horribly) missing something?
> 
> For the most part, yes.  To be more accurate, the failure is coming
> from libata not being able to tell apart link glitches from the device
> getting reset due to power issues.

So for Gionatan the root cause was an instable power supply, but in my
case there wasn't any power loss, there were just failed sata commands.
I'm not sure if this was a port or cable issue - once I changed port and
sata cable the errors disappeared. I didn't change the power supply or
power cable. I'm now basically fighting with the data corruption that
caused - for btrfs it at least has a checksum, but I didn't have ext4
checksum enabled, so it is hard to figure out which files are corrupts -
silent data corruption is not well handled by backups either.

> 
>>> - why the scsi midlevel does not respond to a power loss event by
>>> immediately offlining the disks?
> 
> Because we don't wanna be ditching disks on temporary link glitches,
> which do happen once in a while.
> 
>>> - is the scsi midlevel behavior configurable (I know I can lower eh
>>> timeout, but is this the right solution)?
>>> - how to deal with this problem (other than being 100% sure power is
>>> never lost by any disks)?
> 
> So, the right way to deal with the problem probably is making use of
> the SMART counter which indicates power loss events and verify that
> the counter hasn't increased over link issues.  If it changed, the
> device should be detached and re-probed, which will make it come back
> as a different block device.  Unfortunately, I haven't had the chance
> to actually implement that.

Is it possible that sata eh recovery sends resets to the device, which
makes it evict its cache?

Thanks,
Bernd




^ permalink raw reply

* Re: No I/O errors reported after SATA link hard reset
From: Tejun Heo @ 2017-08-17 13:25 UTC (permalink / raw)
  To: Bernd Schubert; +Cc: Bernd Schubert, linux-ide, Gionatan Danti, linux-scsi
In-Reply-To: <559dc5fe-c162-41e6-7f14-0f3837b64585@fastmail.fm>

Hello,

On Thu, Aug 17, 2017 at 03:18:06PM +0200, Bernd Schubert wrote:
> So for Gionatan the root cause was an instable power supply, but in my
> case there wasn't any power loss, there were just failed sata commands.
> I'm not sure if this was a port or cable issue - once I changed port and
> sata cable the errors disappeared. I didn't change the power supply or
> power cable. I'm now basically fighting with the data corruption that
> caused - for btrfs it at least has a checksum, but I didn't have ext4
> checksum enabled, so it is hard to figure out which files are corrupts -
> silent data corruption is not well handled by backups either.

No idea there.  Retried and recovered errors shouldn't cause data
corruptions.  Flaky power can behave in unexpected ways tho.  What
happens if you hook up the drive on a different power supply but
revert to the port / cable which showed the problem?  What does your
SMART counters say across those failures?

> Is it possible that sata eh recovery sends resets to the device, which
> makes it evict its cache?

That'd be a very broken device.  It sure is theoretically possible but
I haven't seen any reports on such behaviors yet.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: No I/O errors reported after SATA link hard reset
From: Bernd Schubert @ 2017-08-17 13:43 UTC (permalink / raw)
  To: Tejun Heo, Bernd Schubert; +Cc: linux-ide, Gionatan Danti, linux-scsi
In-Reply-To: <20170817132519.GD3238792@devbig577.frc2.facebook.com>



On 08/17/2017 03:25 PM, Tejun Heo wrote:
> Hello,
> 
> On Thu, Aug 17, 2017 at 03:18:06PM +0200, Bernd Schubert wrote:
>> So for Gionatan the root cause was an instable power supply, but in my
>> case there wasn't any power loss, there were just failed sata commands.
>> I'm not sure if this was a port or cable issue - once I changed port and
>> sata cable the errors disappeared. I didn't change the power supply or
>> power cable. I'm now basically fighting with the data corruption that
>> caused - for btrfs it at least has a checksum, but I didn't have ext4
>> checksum enabled, so it is hard to figure out which files are corrupts -
>> silent data corruption is not well handled by backups either.
> 
> No idea there.  Retried and recovered errors shouldn't cause data
> corruptions.  Flaky power can behave in unexpected ways tho.  What
> happens if you hook up the drive on a different power supply but
> revert to the port / cable which showed the problem?  What does your
> SMART counters say across those failures?

Hmm, well, I think I through away the cable already, and I also don't
have spare power supplies at home. It also wasn't that easy to reproduce
the errors, they came up when my wife was working on her system - not
when I was controlling it ;)

> 
>> Is it possible that sata eh recovery sends resets to the device, which
>> makes it evict its cache?
> 
> That'd be a very broken device.  It sure is theoretically possible but
> I haven't seen any reports on such behaviors yet.

I wonder if we just couldn't make the error handler to report issues for
people who are running raid. Gionatans powerloss and my unclear
corruption issue probably wouldn't have happened if the upper md layer
would have gotten an information that it should report errors instead of
recovering them. Although I admit it is a difficult decision what to
with link glitches.


Thanks,
Bernd

^ permalink raw reply

* Re: No I/O errors reported after SATA link hard reset
From: Gionatan Danti @ 2017-08-17 14:15 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Bernd Schubert, linux-ide, linux-scsi, Tejun Heo
In-Reply-To: <20170817124821.GA3238792@devbig577.frc2.facebook.com>

Hi Tejun,

Il 17-08-2017 14:48 Tejun Heo ha scritto:
> Recovered errors aren't reported as IO errors and at least from link
> state proper there's no way for the driver to tell apart link
> glitches and buffer-erasing power issues.

Ok, so *this* is the root cause of the problem: libata not identifying 
spurious link renegotiations vs brief powerloss/powerup events. Out of 
curiosity: is this a SATA-specific problem (ie: in the SATA 
specification), or even SAS disks are affected?

>> > - why the scsi midlevel does not respond to a power loss event by
>> > immediately offlining the disks?
> 
> Because we don't wanna be ditching disks on temporary link glitches,
> which do happen once in a while.

Any chances to report I/O errors to the upper layers *without* offlining 
the device? In this manner, upper layers (ie: MDRAID) can act in a more 
informate way. For example: single disk device will simple retry the 
failed operation, while MDRAID can take the "badblocks" code path to 
deal with the error.

> So, the right way to deal with the problem probably is making use of
> the SMART counter which indicates power loss events and verify that
> the counter hasn't increased over link issues.  If it changed, the
> device should be detached and re-probed, which will make it come back
> as a different block device.  Unfortunately, I haven't had the chance
> to actually implement that.

This is a very good idea, maybe I can implement it in userspace with a 
simple, fast polling scheme (for example, each 60 seconds). Such a 
polling would not prevent all corruption scenarios, but will at least 
timely inform the user.

Thanks.

-- 
Danti Gionatan
Supporto Tecnico
Assyoma S.r.l. - www.assyoma.it
email: g.danti@assyoma.it - info@assyoma.it
GPG public key ID: FF5F32A8

^ permalink raw reply

* Re: No I/O errors reported after SATA link hard reset
From: Gionatan Danti @ 2017-08-17 14:23 UTC (permalink / raw)
  To: Bernd Schubert; +Cc: Tejun Heo, Bernd Schubert, linux-ide, linux-scsi
In-Reply-To: <559dc5fe-c162-41e6-7f14-0f3837b64585@fastmail.fm>

Hi Bernd,

Il 17-08-2017 15:18 Bernd Schubert ha scritto:
> 
> So for Gionatan the root cause was an instable power supply, but in my
> case there wasn't any power loss, there were just failed sata commands.

I tried many times to replicate the error by briefly 
disconnecting/reconnecting the SATA cable, but I had *no* corruption in 
this case. Sure, this was my experience, but a bad-behaving disk 
firmware can do all sort of bad things with the volatile cache, 
especially when renegotiating the host link.

I my case, I did *not* change the power supply, rather the SATA power 
cable: my theory is that, as the previous cable was shared between the 
two disks, somewhat low-voltage spiked find their ways and the second 
disk simply "rebooted". The new cable is dedicated to the SATA disk wich 
was previously failing.

What concern my is that, reading the linux-raid mailing list, many user 
have historycally reported high mismatch count in RAID1 arrays. These 
mismatches were generally discarded saying "RAID1 is prone to false 
positives" but, in my experience, these "false mismatches" are quite 
rare. What it means is that many users *are probably suffering* from my 
(and your) problem, without never realizing that...

Regards.

-- 
Danti Gionatan
Supporto Tecnico
Assyoma S.r.l. - www.assyoma.it
email: g.danti@assyoma.it - info@assyoma.it
GPG public key ID: FF5F32A8

^ permalink raw reply

* Re: No I/O errors reported after SATA link hard reset
From: Tejun Heo @ 2017-08-17 14:46 UTC (permalink / raw)
  To: Gionatan Danti; +Cc: Bernd Schubert, linux-ide, linux-scsi
In-Reply-To: <4debd4d8dea1d534ef555ceae4429435@assyoma.it>

Hello,

On Thu, Aug 17, 2017 at 04:15:35PM +0200, Gionatan Danti wrote:
> Ok, so *this* is the root cause of the problem: libata not
> identifying spurious link renegotiations vs brief powerloss/powerup
> events. Out of curiosity: is this a SATA-specific problem (ie: in
> the SATA specification), or even SAS disks are affected?

No idea about SAS.  They're identical at the link layer tho.

> >Because we don't wanna be ditching disks on temporary link glitches,
> >which do happen once in a while.
> 
> Any chances to report I/O errors to the upper layers *without*
> offlining the device? In this manner, upper layers (ie: MDRAID) can
> act in a more informate way. For example: single disk device will
> simple retry the failed operation, while MDRAID can take the
> "badblocks" code path to deal with the error.

Upper layer can request to avoid retrying on errors but it won't help
too much.  It doesn't have much to do with specific commands.  A power
event can take place without any command in flight and lose the
buffered data.  Unless upper layer is tracking all that's being
written, there isn't much it can do outside doing full scan.  This is
a condition which should be handled from the driver side.

> >So, the right way to deal with the problem probably is making use of
> >the SMART counter which indicates power loss events and verify that
> >the counter hasn't increased over link issues.  If it changed, the
> >device should be detached and re-probed, which will make it come back
> >as a different block device.  Unfortunately, I haven't had the chance
> >to actually implement that.
> 
> This is a very good idea, maybe I can implement it in userspace with
> a simple, fast polling scheme (for example, each 60 seconds). Such a
> polling would not prevent all corruption scenarios, but will at
> least timely inform the user.

Yeah, looking into getting it implemented on the kernel side.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: No I/O errors reported after SATA link hard reset
From: Gionatan Danti @ 2017-08-17 15:01 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-ide, linux-scsi, Tejun Heo
In-Reply-To: <20170817144657.GF3238792@devbig577.frc2.facebook.com>

Il 17-08-2017 16:46 Tejun Heo ha scritto:
> Upper layer can request to avoid retrying on errors but it won't help
> too much.  It doesn't have much to do with specific commands.  A power
> event can take place without any command in flight and lose the
> buffered data.  Unless upper layer is tracking all that's being
> written, there isn't much it can do outside doing full scan.  This is
> a condition which should be handled from the driver side.

True, I was not thinking about buffered (delayed) writes. However, for 
synchronized writes it should be possible: after all, for sync() writes 
the application is waiting for its completion. This means that if a 
powerloss/link renegotiation is detected between *the two FLUSH_CACHE 
commands*, and I/O error can be reported to the calling application.

What about disk supporting FUAs? Are they unaffected by this problem? If 
my understand it properly, torn writes remain a potential, but 
inevitable, problem when facing powerloss conditions.

By the way, when speaking about a "full scan" your are referring to full 
bus scanning/enumeration? Will it change devices name when 
re-discovering them?

> Yeah, looking into getting it implemented on the kernel side.

Great! Are your thinking about a polling approach or an event-driven 
one?

Regards.

-- 
Danti Gionatan
Supporto Tecnico
Assyoma S.r.l. - www.assyoma.it
email: g.danti@assyoma.it - info@assyoma.it
GPG public key ID: FF5F32A8

^ permalink raw reply

* Re: [PATCH v3 2/2] dt-bindings: ata: add DT bindings for MediaTek SATA controller
From: Rob Herring @ 2017-08-17 20:29 UTC (permalink / raw)
  To: Ryder Lee
  Cc: Hans de Goede, Tejun Heo, devicetree, linux-mediatek,
	linux-kernel, linux-ide, Long Cheng
In-Reply-To: <1efe9aad6d81d88ef315c993b7f1aed25be109f5.1502501342.git.ryder.lee@mediatek.com>

On Sat, Aug 12, 2017 at 09:31:04AM +0800, Ryder Lee wrote:
> Add DT bindings for the onboard SATA controller present on the MediaTek
> SoCs.
> 
> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> ---
>  Documentation/devicetree/bindings/ata/ahci-mtk.txt | 51 ++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ata/ahci-mtk.txt
> 
> diff --git a/Documentation/devicetree/bindings/ata/ahci-mtk.txt b/Documentation/devicetree/bindings/ata/ahci-mtk.txt
> new file mode 100644
> index 0000000..96c8d22
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/ahci-mtk.txt
> @@ -0,0 +1,51 @@
> +MediaTek Serial ATA controller
> +
> +Required properties:
> + - compatible	   : Must be "mediatek,soc-model-ahci", "mediatek,mtk-ahci".

Instead of "soc-model", pick something checkpatch.pl understands. IIRC, 
"<chip>" is one.

With that,

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* [PATCH 08/12] ide-floppy: Use blk_rq_is_scsi()
From: Bart Van Assche @ 2017-08-17 23:23 UTC (permalink / raw)
  To: Jens Axboe
  Cc: linux-block, Christoph Hellwig, Bart Van Assche, David S . Miller,
	linux-ide
In-Reply-To: <20170817232311.25948-1-bart.vanassche@wdc.com>

This patch does not change any functionality.

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: linux-ide@vger.kernel.org
---
 drivers/ide/ide-floppy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 627b1f62a749..3ddd88219906 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -72,7 +72,7 @@ static int ide_floppy_callback(ide_drive_t *drive, int dsc)
 		drive->failed_pc = NULL;
 
 	if (pc->c[0] == GPCMD_READ_10 || pc->c[0] == GPCMD_WRITE_10 ||
-	    (req_op(rq) == REQ_OP_SCSI_IN || req_op(rq) == REQ_OP_SCSI_OUT))
+	    blk_rq_is_scsi(rq))
 		uptodate = 1; /* FIXME */
 	else if (pc->c[0] == GPCMD_REQUEST_SENSE) {
 
-- 
2.14.0

^ permalink raw reply related

* [PATCH v4 0/2] Add support for MediaTek AHCI SATA
From: Ryder Lee @ 2017-08-18  1:13 UTC (permalink / raw)
  To: Hans de Goede, Tejun Heo
  Cc: Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-ide-u79uwXL29TY76Z2rM5mHXA, Long Cheng, Ryder Lee

Hi,

This patch series add support for AHCI compatible SATA controller, and it is
compliant with the ahci 1.3 and sata 3.0 specification. This driver is slightly
different than ahci_platform.c (e.g., reset control, subsystem setting).

changes since v4:
- update binding text: Instead of "soc-model", pick something checkpatch.pl understands

changes since v3:
- update binding text: fix a typo and modify compatible strings.

changes since v2:
- according to Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting reset lines").
  replace devm_reset_control_get_optional() by devm_reset_control_get_optional_exclusive().

changes since v1:
- update binding text: add missing "specifier pairs" descriptions.
- fix kbuild test warning: fix the error handling.

Ryder Lee (2):
  ata: mediatek: add support for MediaTek SATA controller
  dt-bindings: ata: add DT bindings for MediaTek SATA controller

 Documentation/devicetree/bindings/ata/ahci-mtk.txt |  51 ++++++
 drivers/ata/Kconfig                                |  10 ++
 drivers/ata/Makefile                               |   1 +
 drivers/ata/ahci_mtk.c                             | 196 +++++++++++++++++++++
 4 files changed, 258 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/ahci-mtk.txt
 create mode 100644 drivers/ata/ahci_mtk.c

-- 
1.9.1

--
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

* [PATCH v4 1/2] ata: mediatek: add support for MediaTek SATA controller
From: Ryder Lee @ 2017-08-18  1:13 UTC (permalink / raw)
  To: Hans de Goede, Tejun Heo
  Cc: Rob Herring, devicetree, linux-mediatek, linux-kernel, linux-ide,
	Long Cheng, Ryder Lee
In-Reply-To: <cover.1503018631.git.ryder.lee@mediatek.com>

This adds support the AHCI-compliant Serial ATA controller present
on MediaTek SoCs.

Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
Acked-by: Tejun Heo <tj@kernel.org>
---
 drivers/ata/Kconfig    |  10 +++
 drivers/ata/Makefile   |   1 +
 drivers/ata/ahci_mtk.c | 196 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 207 insertions(+)
 create mode 100644 drivers/ata/ahci_mtk.c

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 363fc53..488c937 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -153,6 +153,16 @@ config AHCI_CEVA
 
 	  If unsure, say N.
 
+config AHCI_MTK
+	tristate "MediaTek AHCI SATA support"
+	depends on ARCH_MEDIATEK
+	select MFD_SYSCON
+	help
+	  This option enables support for the MediaTek SoC's
+	  onboard AHCI SATA controller.
+
+	  If unsure, say N.
+
 config AHCI_MVEBU
 	tristate "Marvell EBU AHCI SATA support"
 	depends on ARCH_MVEBU
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index a26ef5a..ff9cd2e 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_AHCI_CEVA)		+= ahci_ceva.o libahci.o libahci_platform.o
 obj-$(CONFIG_AHCI_DA850)	+= ahci_da850.o libahci.o libahci_platform.o
 obj-$(CONFIG_AHCI_DM816)	+= ahci_dm816.o libahci.o libahci_platform.o
 obj-$(CONFIG_AHCI_IMX)		+= ahci_imx.o libahci.o libahci_platform.o
+obj-$(CONFIG_AHCI_MTK)		+= ahci_mtk.o libahci.o libahci_platform.o
 obj-$(CONFIG_AHCI_MVEBU)	+= ahci_mvebu.o libahci.o libahci_platform.o
 obj-$(CONFIG_AHCI_OCTEON)	+= ahci_octeon.o
 obj-$(CONFIG_AHCI_SUNXI)	+= ahci_sunxi.o libahci.o libahci_platform.o
diff --git a/drivers/ata/ahci_mtk.c b/drivers/ata/ahci_mtk.c
new file mode 100644
index 0000000..80854f7
--- /dev/null
+++ b/drivers/ata/ahci_mtk.c
@@ -0,0 +1,196 @@
+/*
+ * MeidaTek AHCI SATA driver
+ *
+ * Copyright (c) 2017 MediaTek Inc.
+ * Author: Ryder Lee <ryder.lee@mediatek.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/ahci_platform.h>
+#include <linux/kernel.h>
+#include <linux/libata.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include "ahci.h"
+
+#define DRV_NAME		"ahci"
+
+#define SYS_CFG			0x14
+#define SYS_CFG_SATA_MSK	GENMASK(31, 30)
+#define SYS_CFG_SATA_EN		BIT(31)
+
+struct mtk_ahci_plat {
+	struct regmap *mode;
+	struct reset_control *axi_rst;
+	struct reset_control *sw_rst;
+	struct reset_control *reg_rst;
+};
+
+static const struct ata_port_info ahci_port_info = {
+	.flags		= AHCI_FLAG_COMMON,
+	.pio_mask	= ATA_PIO4,
+	.udma_mask	= ATA_UDMA6,
+	.port_ops	= &ahci_platform_ops,
+};
+
+static struct scsi_host_template ahci_platform_sht = {
+	AHCI_SHT(DRV_NAME),
+};
+
+static int mtk_ahci_platform_resets(struct ahci_host_priv *hpriv,
+				    struct device *dev)
+{
+	struct mtk_ahci_plat *plat = hpriv->plat_data;
+	int err;
+
+	/* reset AXI bus and PHY part */
+	plat->axi_rst = devm_reset_control_get_optional_exclusive(dev, "axi");
+	if (PTR_ERR(plat->axi_rst) == -EPROBE_DEFER)
+		return PTR_ERR(plat->axi_rst);
+
+	plat->sw_rst = devm_reset_control_get_optional_exclusive(dev, "sw");
+	if (PTR_ERR(plat->sw_rst) == -EPROBE_DEFER)
+		return PTR_ERR(plat->sw_rst);
+
+	plat->reg_rst = devm_reset_control_get_optional_exclusive(dev, "reg");
+	if (PTR_ERR(plat->reg_rst) == -EPROBE_DEFER)
+		return PTR_ERR(plat->reg_rst);
+
+	err = reset_control_assert(plat->axi_rst);
+	if (err) {
+		dev_err(dev, "failed to assert AXI bus\n");
+		return err;
+	}
+
+	err = reset_control_assert(plat->sw_rst);
+	if (err) {
+		dev_err(dev, "failed to assert PHY digital part\n");
+		return err;
+	}
+
+	err = reset_control_assert(plat->reg_rst);
+	if (err) {
+		dev_err(dev, "failed to assert PHY register part\n");
+		return err;
+	}
+
+	err = reset_control_deassert(plat->reg_rst);
+	if (err) {
+		dev_err(dev, "failed to deassert PHY register part\n");
+		return err;
+	}
+
+	err = reset_control_deassert(plat->sw_rst);
+	if (err) {
+		dev_err(dev, "failed to deassert PHY digital part\n");
+		return err;
+	}
+
+	err = reset_control_deassert(plat->axi_rst);
+	if (err) {
+		dev_err(dev, "failed to deassert AXI bus\n");
+		return err;
+	}
+
+	return 0;
+}
+
+static int mtk_ahci_parse_property(struct ahci_host_priv *hpriv,
+				   struct device *dev)
+{
+	struct mtk_ahci_plat *plat = hpriv->plat_data;
+	struct device_node *np = dev->of_node;
+
+	/* enable SATA function if needed */
+	if (of_find_property(np, "mediatek,phy-mode", NULL)) {
+		plat->mode = syscon_regmap_lookup_by_phandle(
+					np, "mediatek,phy-mode");
+		if (IS_ERR(plat->mode)) {
+			dev_err(dev, "missing phy-mode phandle\n");
+			return PTR_ERR(plat->mode);
+		}
+
+		regmap_update_bits(plat->mode, SYS_CFG, SYS_CFG_SATA_MSK,
+				   SYS_CFG_SATA_EN);
+	}
+
+	of_property_read_u32(np, "ports-implemented", &hpriv->force_port_map);
+
+	return 0;
+}
+
+static int mtk_ahci_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mtk_ahci_plat *plat;
+	struct ahci_host_priv *hpriv;
+	int err;
+
+	plat = devm_kzalloc(dev, sizeof(*plat), GFP_KERNEL);
+	if (!plat)
+		return -ENOMEM;
+
+	hpriv = ahci_platform_get_resources(pdev);
+	if (IS_ERR(hpriv))
+		return PTR_ERR(hpriv);
+
+	hpriv->plat_data = plat;
+
+	err = mtk_ahci_parse_property(hpriv, dev);
+	if (err)
+		return err;
+
+	err = mtk_ahci_platform_resets(hpriv, dev);
+	if (err)
+		return err;
+
+	err = ahci_platform_enable_resources(hpriv);
+	if (err)
+		return err;
+
+	err = ahci_platform_init_host(pdev, hpriv, &ahci_port_info,
+				      &ahci_platform_sht);
+	if (err)
+		goto disable_resources;
+
+	return 0;
+
+disable_resources:
+	ahci_platform_disable_resources(hpriv);
+	return err;
+}
+
+static SIMPLE_DEV_PM_OPS(ahci_pm_ops, ahci_platform_suspend,
+			 ahci_platform_resume);
+
+static const struct of_device_id ahci_of_match[] = {
+	{ .compatible = "mediatek,mtk-ahci", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ahci_of_match);
+
+static struct platform_driver mtk_ahci_driver = {
+	.probe = mtk_ahci_probe,
+	.remove = ata_platform_remove_one,
+	.driver = {
+		.name = DRV_NAME,
+		.of_match_table = ahci_of_match,
+		.pm = &ahci_pm_ops,
+	},
+};
+module_platform_driver(mtk_ahci_driver);
+
+MODULE_DESCRIPTION("MeidaTek SATA AHCI Driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

^ permalink raw reply related

* [PATCH v4 2/2] dt-bindings: ata: add DT bindings for MediaTek SATA controller
From: Ryder Lee @ 2017-08-18  1:13 UTC (permalink / raw)
  To: Hans de Goede, Tejun Heo
  Cc: Rob Herring, devicetree, linux-mediatek, linux-kernel, linux-ide,
	Long Cheng, Ryder Lee
In-Reply-To: <cover.1503018631.git.ryder.lee@mediatek.com>

Add DT bindings for the onboard SATA controller present on the MediaTek
SoCs.

Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/ata/ahci-mtk.txt | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ata/ahci-mtk.txt

diff --git a/Documentation/devicetree/bindings/ata/ahci-mtk.txt b/Documentation/devicetree/bindings/ata/ahci-mtk.txt
new file mode 100644
index 0000000..d2aa696
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/ahci-mtk.txt
@@ -0,0 +1,51 @@
+MediaTek Serial ATA controller
+
+Required properties:
+ - compatible	   : Must be "mediatek,<chip>-ahci", "mediatek,mtk-ahci".
+		     When using "mediatek,mtk-ahci" compatible strings, you
+		     need SoC specific ones in addition, one of:
+		     - "mediatek,mt7622-ahci"
+ - reg		   : Physical base addresses and length of register sets.
+ - interrupts	   : Interrupt associated with the SATA device.
+ - interrupt-names : Associated name must be: "hostc".
+ - clocks	   : A list of phandle and clock specifier pairs, one for each
+		     entry in clock-names.
+ - clock-names	   : Associated names must be: "ahb", "axi", "asic", "rbc", "pm".
+ - phys		   : A phandle and PHY specifier pair for the PHY port.
+ - phy-names	   : Associated name must be: "sata-phy".
+ - ports-implemented : See ./ahci-platform.txt for details.
+
+Optional properties:
+ - power-domains   : A phandle and power domain specifier pair to the power
+		     domain which is responsible for collapsing and restoring
+		     power to the peripheral.
+ - resets	   : Must contain an entry for each entry in reset-names.
+		     See ../reset/reset.txt for details.
+ - reset-names	   : Associated names must be: "axi", "sw", "reg".
+ - mediatek,phy-mode : A phandle to the system controller, used to enable
+		       SATA function.
+
+Example:
+
+	sata: sata@1a200000 {
+		compatible = "mediatek,mt7622-ahci",
+			     "mediatek,mtk-ahci";
+		reg = <0 0x1a200000 0 0x1100>;
+		interrupts = <GIC_SPI 233 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "hostc";
+		clocks = <&pciesys CLK_SATA_AHB_EN>,
+			 <&pciesys CLK_SATA_AXI_EN>,
+			 <&pciesys CLK_SATA_ASIC_EN>,
+			 <&pciesys CLK_SATA_RBC_EN>,
+			 <&pciesys CLK_SATA_PM_EN>;
+		clock-names = "ahb", "axi", "asic", "rbc", "pm";
+		phys = <&u3port1 PHY_TYPE_SATA>;
+		phy-names = "sata-phy";
+		ports-implemented = <0x1>;
+		power-domains = <&scpsys MT7622_POWER_DOMAIN_HIF0>;
+		resets = <&pciesys MT7622_SATA_AXI_BUS_RST>,
+			 <&pciesys MT7622_SATA_PHY_SW_RST>,
+			 <&pciesys MT7622_SATA_PHY_REG_RST>;
+		reset-names = "axi", "sw", "reg";
+		mediatek,phy-mode = <&pciesys>;
+	};
-- 
1.9.1

^ permalink raw reply related


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