From 3c696ac4173678b62e74e26644d9f3d662973bfa Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Tue, 10 Dec 2019 10:53:48 -0800 Subject: [PATCH 01/10] ata: ahci_brcm: Manage reset line during suspend/resume We were not managing the reset line during suspend/resume, but this needs to be done to ensure that the controller can exit low power modes correctly, especially with deep sleep suspend mode that may reset parts of the logic. Reviewed-by: Hans de Goede Signed-off-by: Florian Fainelli Signed-off-by: Jens Axboe --- drivers/ata/ahci_brcm.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c index 66a570d0da83..76612577a59a 100644 --- a/drivers/ata/ahci_brcm.c +++ b/drivers/ata/ahci_brcm.c @@ -343,10 +343,16 @@ static int brcm_ahci_suspend(struct device *dev) struct ata_host *host = dev_get_drvdata(dev); struct ahci_host_priv *hpriv = host->private_data; struct brcm_ahci_priv *priv = hpriv->plat_data; + int ret; brcm_sata_phys_disable(priv); - return ahci_platform_suspend(dev); + ret = ahci_platform_suspend(dev); + + if (!IS_ERR_OR_NULL(priv->rcdev)) + reset_control_assert(priv->rcdev); + + return ret; } static int brcm_ahci_resume(struct device *dev) @@ -354,7 +360,12 @@ static int brcm_ahci_resume(struct device *dev) struct ata_host *host = dev_get_drvdata(dev); struct ahci_host_priv *hpriv = host->private_data; struct brcm_ahci_priv *priv = hpriv->plat_data; - int ret; + int ret = 0; + + if (!IS_ERR_OR_NULL(priv->rcdev)) + ret = reset_control_deassert(priv->rcdev); + if (ret) + return ret; /* Make sure clocks are turned on before re-configuration */ ret = ahci_platform_enable_clks(hpriv); From 7de9b1688c1d4a4c9267e65338ae8d6d0d025625 Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Tue, 10 Dec 2019 10:53:49 -0800 Subject: [PATCH 02/10] ata: ahci_brcm: Add a shutdown callback Make sure that we quiesce the controller and shut down the clocks in a shutdown callback. Reviewed-by: Hans de Goede Signed-off-by: Florian Fainelli Signed-off-by: Jens Axboe --- drivers/ata/ahci_brcm.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c index 76612577a59a..58e1a6e5478d 100644 --- a/drivers/ata/ahci_brcm.c +++ b/drivers/ata/ahci_brcm.c @@ -532,11 +532,26 @@ static int brcm_ahci_remove(struct platform_device *pdev) return 0; } +static void brcm_ahci_shutdown(struct platform_device *pdev) +{ + int ret; + + /* All resources releasing happens via devres, but our device, unlike a + * proper remove is not disappearing, therefore using + * brcm_ahci_suspend() here which does explicit power management is + * appropriate. + */ + ret = brcm_ahci_suspend(&pdev->dev); + if (ret) + dev_err(&pdev->dev, "failed to shutdown\n"); +} + static SIMPLE_DEV_PM_OPS(ahci_brcm_pm_ops, brcm_ahci_suspend, brcm_ahci_resume); static struct platform_driver brcm_ahci_driver = { .probe = brcm_ahci_probe, .remove = brcm_ahci_remove, + .shutdown = brcm_ahci_shutdown, .driver = { .name = DRV_NAME, .of_match_table = ahci_of_match, From 52fa562db5ecd2c4ae91a1106844877b02af7031 Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Tue, 10 Dec 2019 10:53:50 -0800 Subject: [PATCH 03/10] dt-bindings: ata: Document BCM7216 AHCI controller compatible The BCM7216 AHCI controller makes use of a specific reset controller line/name, document the compatible string and the optional reset properties. Reviewed-by: Rob Herring Reviewed-by: Hans de Goede Signed-off-by: Florian Fainelli Signed-off-by: Jens Axboe --- Documentation/devicetree/bindings/ata/brcm,sata-brcm.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/ata/brcm,sata-brcm.txt b/Documentation/devicetree/bindings/ata/brcm,sata-brcm.txt index 7713a413c6a7..b9ae4ce4a0a0 100644 --- a/Documentation/devicetree/bindings/ata/brcm,sata-brcm.txt +++ b/Documentation/devicetree/bindings/ata/brcm,sata-brcm.txt @@ -5,6 +5,7 @@ Each SATA controller should have its own node. Required properties: - compatible : should be one or more of + "brcm,bcm7216-ahci" "brcm,bcm7425-ahci" "brcm,bcm7445-ahci" "brcm,bcm-nsp-ahci" @@ -14,6 +15,12 @@ Required properties: - reg-names : "ahci" and "top-ctrl" - interrupts : interrupt mapping for SATA IRQ +Optional properties: + +- reset: for "brcm,bcm7216-ahci" must be a valid reset phandle + pointing to the RESCAL reset controller provider node. +- reset-names: for "brcm,bcm7216-ahci", must be "rescal". + Also see ahci-platform.txt. Example: From c345ec6a50e9f5d2800da2179adc8f6ea1dfe042 Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Tue, 10 Dec 2019 10:53:51 -0800 Subject: [PATCH 04/10] ata: ahci_brcm: Support BCM7216 reset controller name BCM7216 uses a different reset controller name which is "rescal" instead of "ahci", match the compatible string to account for that minor difference, the reset is otherwise identical to how other generations of SATA controllers work. Reviewed-by: Hans de Goede Signed-off-by: Florian Fainelli Signed-off-by: Jens Axboe --- drivers/ata/ahci_brcm.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c index 58e1a6e5478d..13ceca687104 100644 --- a/drivers/ata/ahci_brcm.c +++ b/drivers/ata/ahci_brcm.c @@ -73,6 +73,7 @@ enum brcm_ahci_version { BRCM_SATA_BCM7425 = 1, BRCM_SATA_BCM7445, BRCM_SATA_NSP, + BRCM_SATA_BCM7216, }; enum brcm_ahci_quirks { @@ -415,6 +416,7 @@ static const struct of_device_id ahci_of_match[] = { {.compatible = "brcm,bcm7445-ahci", .data = (void *)BRCM_SATA_BCM7445}, {.compatible = "brcm,bcm63138-ahci", .data = (void *)BRCM_SATA_BCM7445}, {.compatible = "brcm,bcm-nsp-ahci", .data = (void *)BRCM_SATA_NSP}, + {.compatible = "brcm,bcm7216-ahci", .data = (void *)BRCM_SATA_BCM7216}, {}, }; MODULE_DEVICE_TABLE(of, ahci_of_match); @@ -423,6 +425,7 @@ static int brcm_ahci_probe(struct platform_device *pdev) { const struct of_device_id *of_id; struct device *dev = &pdev->dev; + const char *reset_name = NULL; struct brcm_ahci_priv *priv; struct ahci_host_priv *hpriv; struct resource *res; @@ -444,8 +447,13 @@ static int brcm_ahci_probe(struct platform_device *pdev) if (IS_ERR(priv->top_ctrl)) return PTR_ERR(priv->top_ctrl); - /* Reset is optional depending on platform */ - priv->rcdev = devm_reset_control_get(&pdev->dev, "ahci"); + /* Reset is optional depending on platform and named differently */ + if (priv->version == BRCM_SATA_BCM7216) + reset_name = "rescal"; + else + reset_name = "ahci"; + + priv->rcdev = devm_reset_control_get(&pdev->dev, reset_name); if (!IS_ERR_OR_NULL(priv->rcdev)) reset_control_deassert(priv->rcdev); From ed87ad196dab6a757352d39c4c3fbc599340fe2c Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Tue, 7 Jan 2020 23:14:59 +0100 Subject: [PATCH 05/10] ata: brcm: mark PM functions as __maybe_unused The new shutdown callback causes a link failure: drivers/ata/ahci_brcm.c: In function 'brcm_ahci_shutdown': drivers/ata/ahci_brcm.c:552:8: error: implicit declaration of function 'brcm_ahci_suspend'; did you mean 'brcm_ahci_shutdown'? [-Werror=implicit-function-declaration] ret = brcm_ahci_suspend(&pdev->dev); ^~~~~~~~~~~~~~~~~ Remove the incorrect #ifdef and use __maybe_unused annotations instead to make this more robust. Fixes: 7de9b1688c1d ("ata: ahci_brcm: Add a shutdown callback") Acked-by: Florian Fainelli Signed-off-by: Arnd Bergmann Signed-off-by: Jens Axboe --- drivers/ata/ahci_brcm.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c index 13ceca687104..239333d11b88 100644 --- a/drivers/ata/ahci_brcm.c +++ b/drivers/ata/ahci_brcm.c @@ -338,7 +338,6 @@ static const struct ata_port_info ahci_brcm_port_info = { .port_ops = &ahci_brcm_platform_ops, }; -#ifdef CONFIG_PM_SLEEP static int brcm_ahci_suspend(struct device *dev) { struct ata_host *host = dev_get_drvdata(dev); @@ -348,7 +347,10 @@ static int brcm_ahci_suspend(struct device *dev) brcm_sata_phys_disable(priv); - ret = ahci_platform_suspend(dev); + if (IS_ENABLED(CONFIG_PM_SLEEP)) + ret = ahci_platform_suspend(dev); + else + ret = 0; if (!IS_ERR_OR_NULL(priv->rcdev)) reset_control_assert(priv->rcdev); @@ -356,7 +358,7 @@ static int brcm_ahci_suspend(struct device *dev) return ret; } -static int brcm_ahci_resume(struct device *dev) +static int __maybe_unused brcm_ahci_resume(struct device *dev) { struct ata_host *host = dev_get_drvdata(dev); struct ahci_host_priv *hpriv = host->private_data; @@ -405,7 +407,6 @@ out_disable_phys: ahci_platform_disable_clks(hpriv); return ret; } -#endif static struct scsi_host_template ahci_platform_sht = { AHCI_SHT(DRV_NAME), From 6fedae3cad8b122c8b0afb26a0569d9910255edd Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Tue, 7 Jan 2020 23:15:00 +0100 Subject: [PATCH 06/10] ata: brcm: fix reset controller API usage While fixing another issue in this driver I noticed it uses IS_ERR_OR_NULL(), which is almost always a mistake. Change the driver to use the proper devm_reset_control_get_optional() interface instead and remove the checks except for the one that checks for a failure in that function. Fixes: 2b2c47d9e1fe ("ata: ahci_brcm: Allow optional reset controller to be used") Signed-off-by: Arnd Bergmann Signed-off-by: Jens Axboe --- drivers/ata/ahci_brcm.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c index 239333d11b88..7ac1141c6ad0 100644 --- a/drivers/ata/ahci_brcm.c +++ b/drivers/ata/ahci_brcm.c @@ -352,8 +352,7 @@ static int brcm_ahci_suspend(struct device *dev) else ret = 0; - if (!IS_ERR_OR_NULL(priv->rcdev)) - reset_control_assert(priv->rcdev); + reset_control_assert(priv->rcdev); return ret; } @@ -365,8 +364,7 @@ static int __maybe_unused brcm_ahci_resume(struct device *dev) struct brcm_ahci_priv *priv = hpriv->plat_data; int ret = 0; - if (!IS_ERR_OR_NULL(priv->rcdev)) - ret = reset_control_deassert(priv->rcdev); + ret = reset_control_deassert(priv->rcdev); if (ret) return ret; @@ -454,9 +452,11 @@ static int brcm_ahci_probe(struct platform_device *pdev) else reset_name = "ahci"; - priv->rcdev = devm_reset_control_get(&pdev->dev, reset_name); - if (!IS_ERR_OR_NULL(priv->rcdev)) - reset_control_deassert(priv->rcdev); + priv->rcdev = devm_reset_control_get_optional(&pdev->dev, reset_name); + if (IS_ERR(priv->rcdev)) + return PTR_ERR(priv->rcdev); + + reset_control_deassert(priv->rcdev); hpriv = ahci_platform_get_resources(pdev, 0); if (IS_ERR(hpriv)) { @@ -520,8 +520,7 @@ out_disable_phys: out_disable_clks: ahci_platform_disable_clks(hpriv); out_reset: - if (!IS_ERR_OR_NULL(priv->rcdev)) - reset_control_assert(priv->rcdev); + reset_control_assert(priv->rcdev); return ret; } From 1a0600d112e32eac893d2120207da7887f345495 Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Fri, 17 Jan 2020 15:53:12 -0800 Subject: [PATCH 07/10] ata: ahci_brcm: Perform reset after obtaining resources Resources such as clocks, PHYs, regulators are likely to get a probe deferral return code, which could lead to the AHCI controller being reset a few times until it gets successfully probed. Since this is typically the most time consuming operation, move it after the resources have been acquired. Signed-off-by: Florian Fainelli Signed-off-by: Jens Axboe --- drivers/ata/ahci_brcm.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c index 7ac1141c6ad0..e32c8fe729ff 100644 --- a/drivers/ata/ahci_brcm.c +++ b/drivers/ata/ahci_brcm.c @@ -456,13 +456,9 @@ static int brcm_ahci_probe(struct platform_device *pdev) if (IS_ERR(priv->rcdev)) return PTR_ERR(priv->rcdev); - reset_control_deassert(priv->rcdev); - hpriv = ahci_platform_get_resources(pdev, 0); - if (IS_ERR(hpriv)) { - ret = PTR_ERR(hpriv); - goto out_reset; - } + if (IS_ERR(hpriv)) + return PTR_ERR(hpriv); hpriv->plat_data = priv; hpriv->flags = AHCI_HFLAG_WAKE_BEFORE_STOP | AHCI_HFLAG_NO_WRITE_TO_RO; @@ -479,6 +475,10 @@ static int brcm_ahci_probe(struct platform_device *pdev) break; } + ret = reset_control_deassert(priv->rcdev); + if (ret) + return ret; + ret = ahci_platform_enable_clks(hpriv); if (ret) goto out_reset; From 272ecd60a636a8508997ff997ddfa143474e7c81 Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Fri, 17 Jan 2020 15:53:13 -0800 Subject: [PATCH 08/10] ata: ahci_brcm: BCM7216 reset is self de-asserting The BCM7216 reset controller line is self-deasserting, unlike other platforms, so make use of reset_control_reset() instead of reset_control_deassert(). Signed-off-by: Florian Fainelli Signed-off-by: Jens Axboe --- drivers/ata/ahci_brcm.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c index e32c8fe729ff..6853dbb4131d 100644 --- a/drivers/ata/ahci_brcm.c +++ b/drivers/ata/ahci_brcm.c @@ -352,7 +352,8 @@ static int brcm_ahci_suspend(struct device *dev) else ret = 0; - reset_control_assert(priv->rcdev); + if (priv->version != BRCM_SATA_BCM7216) + reset_control_assert(priv->rcdev); return ret; } @@ -364,7 +365,10 @@ static int __maybe_unused brcm_ahci_resume(struct device *dev) struct brcm_ahci_priv *priv = hpriv->plat_data; int ret = 0; - ret = reset_control_deassert(priv->rcdev); + if (priv->version == BRCM_SATA_BCM7216) + ret = reset_control_reset(priv->rcdev); + else + ret = reset_control_deassert(priv->rcdev); if (ret) return ret; @@ -475,7 +479,10 @@ static int brcm_ahci_probe(struct platform_device *pdev) break; } - ret = reset_control_deassert(priv->rcdev); + if (priv->version == BRCM_SATA_BCM7216) + ret = reset_control_reset(priv->rcdev); + else + ret = reset_control_deassert(priv->rcdev); if (ret) return ret; @@ -520,7 +527,8 @@ out_disable_phys: out_disable_clks: ahci_platform_disable_clks(hpriv); out_reset: - reset_control_assert(priv->rcdev); + if (priv->version != BRCM_SATA_BCM7216) + reset_control_assert(priv->rcdev); return ret; } From ffa302efe84ed8aa84aaf061ec4448036880b934 Mon Sep 17 00:00:00 2001 From: Chen Zhou Date: Tue, 21 Jan 2020 09:28:27 +0800 Subject: [PATCH 09/10] ata: pata_macio: fix comparing pointer to 0 Fixes coccicheck warning: ./drivers/ata/pata_macio.c:982:31-32: WARNING comparing pointer to 0, suggest !E Compare pointer-typed values to NULL rather than 0. Acked-by: Bartlomiej Zolnierkiewicz Signed-off-by: Chen Zhou Signed-off-by: Jens Axboe --- drivers/ata/pata_macio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/ata/pata_macio.c b/drivers/ata/pata_macio.c index 1bfd0154dad5..e47a28271f5b 100644 --- a/drivers/ata/pata_macio.c +++ b/drivers/ata/pata_macio.c @@ -979,7 +979,7 @@ static void pata_macio_invariants(struct pata_macio_priv *priv) priv->aapl_bus_id = bidp ? *bidp : 0; /* Fixup missing Apple bus ID in case of media-bay */ - if (priv->mediabay && bidp == 0) + if (priv->mediabay && !bidp) priv->aapl_bus_id = 1; } From 7e053d3e820bedd7ea4dad598d40bfc7639428c6 Mon Sep 17 00:00:00 2001 From: Alex Shi Date: Tue, 21 Jan 2020 16:48:49 +0800 Subject: [PATCH 10/10] ata/acard_ahci: remove unused variable n_elem No one care the varible acard_ahci in func acard_ahci_qc_prep. better to remove it. Signed-off-by: Alex Shi Cc: Jens Axboe Cc: linux-ide@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Jens Axboe --- drivers/ata/acard-ahci.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/ata/acard-ahci.c b/drivers/ata/acard-ahci.c index 46dc54d18f0b..2a04e8abd397 100644 --- a/drivers/ata/acard-ahci.c +++ b/drivers/ata/acard-ahci.c @@ -218,7 +218,6 @@ static enum ata_completion_errors acard_ahci_qc_prep(struct ata_queued_cmd *qc) void *cmd_tbl; u32 opts; const u32 cmd_fis_len = 5; /* five dwords */ - unsigned int n_elem; /* * Fill in command table information. First, the header, @@ -232,9 +231,8 @@ static enum ata_completion_errors acard_ahci_qc_prep(struct ata_queued_cmd *qc) memcpy(cmd_tbl + AHCI_CMD_TBL_CDB, qc->cdb, qc->dev->cdb_len); } - n_elem = 0; if (qc->flags & ATA_QCFLAG_DMAMAP) - n_elem = acard_ahci_fill_sg(qc, cmd_tbl); + acard_ahci_fill_sg(qc, cmd_tbl); /* * Fill in command slot information.