The AHCI resources management within ahci_brcm.c is a little convoluted, largely because it historically had a dedicated clock that was managed within this file in the downstream tree. Once brough upstream though, the clock was left to be managed by libahci_platform.c which is entirely appropriate. This patch series ensures that the AHCI resources are fetched and enabled before any register access is done, thus avoiding bus errors on platforms which clock gate the controller by default. As a result we need to re-arrange the suspend() and resume() functions in order to avoid accessing registers after the clocks have been turned off respectively before the clocks have been turned on. Finally, we can refactor brcm_ahci_get_portmask() in order to fetch the number of ports from hpriv->mmio which is now accessible without jumping through hoops like we used to do. The commit pointed in the Fixes tag is both old and new enough not to require major headaches for backporting of this patch.
Unified · Split
| @@ -213,19 +213,12 @@ static void brcm_sata_phys_disable(struct brcm_ahci_priv *priv) · 212 unmodified lines | |||
| 213 | brcm_sata_phy_disable(priv, i); | 213 | brcm_sata_phy_disable(priv, i); |
| 214 | } | 214 | } |
| 215 | 215 | ||
| 216 | static u32 brcm_ahci_get_portmask(struct platform_device *pdev, | 216 | static u32 brcm_ahci_get_portmask(struct ahci_host_priv *hpriv, |
| 217 | struct brcm_ahci_priv *priv) | 217 | struct brcm_ahci_priv *priv) |
| 218 | { | 218 | { |
| 219 | void __iomem *ahci; | ||
| 220 | struct resource *res; | ||
| 221 | u32 impl; | 219 | u32 impl; |
| 222 | 220 | ||
| 223 | res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ahci"); | 221 | impl = readl(hpriv->mmio + HOST_PORTS_IMPL); |
| 224 | ahci = devm_ioremap_resource(&pdev->dev, res); | ||
| 225 | if (IS_ERR(ahci)) | ||
| 226 | return 0; | ||
| 227 | |||
| 228 | impl = readl(ahci + HOST_PORTS_IMPL); | ||
| 229 | 222 | ||
| 230 | if (fls(impl) > SATA_TOP_MAX_PHYS) | 223 | if (fls(impl) > SATA_TOP_MAX_PHYS) |
| 231 | dev_warn(priv->dev, "warning: more ports than PHYs (%#x)\n", | 224 | dev_warn(priv->dev, "warning: more ports than PHYs (%#x)\n", |
| @@ -233,9 +226,6 @@ static u32 brcm_ahci_get_portmask(struct platform_device *pdev, · 1 unmodified line | |||
| 233 | else if (!impl) | 226 | else if (!impl) |
| 234 | dev_info(priv->dev, "no ports found\n"); | 227 | dev_info(priv->dev, "no ports found\n"); |
| 235 | 228 | ||
| 236 | devm_iounmap(&pdev->dev, ahci); | ||
| 237 | devm_release_mem_region(&pdev->dev, res->start, resource_size(res)); | ||
| 238 | |||
| 239 | return impl; | 229 | return impl; |
| 240 | } | 230 | } |
| 241 | 231 | ||
| @@ -347,11 +337,10 @@ static int brcm_ahci_suspend(struct device *dev) · 105 unmodified lines | |||
| 347 | struct ata_host *host = dev_get_drvdata(dev); | 337 | struct ata_host *host = dev_get_drvdata(dev); |
| 348 | struct ahci_host_priv *hpriv = host->private_data; | 338 | struct ahci_host_priv *hpriv = host->private_data; |
| 349 | struct brcm_ahci_priv *priv = hpriv->plat_data; | 339 | struct brcm_ahci_priv *priv = hpriv->plat_data; |
| 350 | int ret; | ||
| 351 | 340 | ||
| 352 | ret = ahci_platform_suspend(dev); | ||
| 353 | brcm_sata_phys_disable(priv); | 341 | brcm_sata_phys_disable(priv); |
| 354 | return ret; | 342 | |
| 343 | return ahci_platform_suspend(dev); | ||
| 355 | } | 344 | } |
| 356 | 345 | ||
| 357 | static int brcm_ahci_resume(struct device *dev) | 346 | static int brcm_ahci_resume(struct device *dev) |
| @@ -359,11 +348,44 @@ static int brcm_ahci_resume(struct device *dev) · 1 unmodified line | |||
| 359 | struct ata_host *host = dev_get_drvdata(dev); | 348 | struct ata_host *host = dev_get_drvdata(dev); |
| 360 | struct ahci_host_priv *hpriv = host->private_data; | 349 | struct ahci_host_priv *hpriv = host->private_data; |
| 361 | struct brcm_ahci_priv *priv = hpriv->plat_data; | 350 | struct brcm_ahci_priv *priv = hpriv->plat_data; |
| 351 | int ret; | ||
| 352 | |||
| 353 | /* Make sure clocks are turned on before re-configuration */ | ||
| 354 | ret = ahci_platform_enable_clks(hpriv); | ||
| 355 | if (ret) | ||
| 356 | return ret; | ||
| 362 | 357 | ||
| 363 | brcm_sata_init(priv); | 358 | brcm_sata_init(priv); |
| 364 | brcm_sata_phys_enable(priv); | 359 | brcm_sata_phys_enable(priv); |
| 365 | brcm_sata_alpm_init(hpriv); | 360 | brcm_sata_alpm_init(hpriv); |
| 366 | return ahci_platform_resume(dev); | 361 | |
| 362 | /* Since we had to enable clocks earlier on, we cannot use | ||
| 363 | * ahci_platform_resume() as-is since a second call to | ||
| 364 | * ahci_platform_enable_resources() would bump up the resources | ||
| 365 | * (regulators, clocks, PHYs) count artificially so we copy the part | ||
| 366 | * after ahci_platform_enable_resources(). | ||
| 367 | */ | ||
| 368 | ret = ahci_platform_enable_phys(hpriv); | ||
| 369 | if (ret) | ||
| 370 | goto out_disable_phys; | ||
| 371 | |||
| 372 | ret = ahci_platform_resume_host(dev); | ||
| 373 | if (ret) | ||
| 374 | goto out_disable_platform_phys; | ||
| 375 | |||
| 376 | /* We resumed so update PM runtime state */ | ||
| 377 | pm_runtime_disable(dev); | ||
| 378 | pm_runtime_set_active(dev); | ||
| 379 | pm_runtime_enable(dev); | ||
| 380 | |||
| 381 | return 0; | ||
| 382 | |||
| 383 | out_disable_platform_phys: | ||
| 384 | ahci_platform_disable_phys(hpriv); | ||
| 385 | out_disable_phys: | ||
| 386 | brcm_sata_phys_disable(priv); | ||
| 387 | ahci_platform_disable_clks(hpriv); | ||
| 388 | return ret; | ||
| 367 | } | 389 | } |
| 368 | #endif | 390 | #endif |
| 369 | 391 | ||
| @@ -416,38 +438,63 @@ static int brcm_ahci_probe(struct platform_device *pdev) · 46 unmodified lines | |||
| 416 | priv->quirks |= BRCM_AHCI_QUIRK_SKIP_PHY_ENABLE; | 438 | priv->quirks |= BRCM_AHCI_QUIRK_SKIP_PHY_ENABLE; |
| 417 | } | 439 | } |
| 418 | 440 | ||
| 441 | hpriv = ahci_platform_get_resources(pdev, 0); | ||
| 442 | if (IS_ERR(hpriv)) { | ||
| 443 | ret = PTR_ERR(hpriv); | ||
| 444 | goto out_reset; | ||
| 445 | } | ||
| 446 | |||
| 447 | ret = ahci_platform_enable_clks(hpriv); | ||
| 448 | if (ret) | ||
| 449 | goto out_reset; | ||
| 450 | |||
| 451 | /* Must be first so as to configure endianness including that | ||
| 452 | * of the standard AHCI register space. | ||
| 453 | */ | ||
| 419 | brcm_sata_init(priv); | 454 | brcm_sata_init(priv); |
| 420 | 455 | ||
| 421 | priv->port_mask = brcm_ahci_get_portmask(pdev, priv); | 456 | /* Initializes priv->port_mask which is used below */ |
| 422 | if (!priv->port_mask) | 457 | priv->port_mask = brcm_ahci_get_portmask(hpriv, priv); |
| 423 | return -ENODEV; | 458 | if (!priv->port_mask) { |
| 459 | ret = -ENODEV; | ||
| 460 | goto out_disable_clks; | ||
| 461 | } | ||
| 424 | 462 | ||
| 463 | /* Must be done before ahci_platform_enable_phys() */ | ||
| 425 | brcm_sata_phys_enable(priv); | 464 | brcm_sata_phys_enable(priv); |
| 426 | 465 | ||
| 427 | hpriv = ahci_platform_get_resources(pdev, 0); | ||
| 428 | if (IS_ERR(hpriv)) | ||
| 429 | return PTR_ERR(hpriv); | ||
| 430 | hpriv->plat_data = priv; | 466 | hpriv->plat_data = priv; |
| 431 | hpriv->flags = AHCI_HFLAG_WAKE_BEFORE_STOP; | 467 | hpriv->flags = AHCI_HFLAG_WAKE_BEFORE_STOP; |
| 432 | 468 | ||
| 433 | brcm_sata_alpm_init(hpriv); | 469 | brcm_sata_alpm_init(hpriv); |
| 434 | 470 | ||
| 435 | ret = ahci_platform_enable_resources(hpriv); | ||
| 436 | if (ret) | ||
| 437 | return ret; | ||
| 438 | |||
| 439 | if (priv->quirks & BRCM_AHCI_QUIRK_NO_NCQ) | 471 | if (priv->quirks & BRCM_AHCI_QUIRK_NO_NCQ) |
| 440 | hpriv->flags |= AHCI_HFLAG_NO_NCQ; | 472 | hpriv->flags |= AHCI_HFLAG_NO_NCQ; |
| 441 | hpriv->flags |= AHCI_HFLAG_NO_WRITE_TO_RO; | 473 | hpriv->flags |= AHCI_HFLAG_NO_WRITE_TO_RO; |
| 442 | 474 | ||
| 475 | ret = ahci_platform_enable_phys(hpriv); | ||
| 476 | if (ret) | ||
| 477 | goto out_disable_phys; | ||
| 478 | |||
| 443 | ret = ahci_platform_init_host(pdev, hpriv, &ahci_brcm_port_info, | 479 | ret = ahci_platform_init_host(pdev, hpriv, &ahci_brcm_port_info, |
| 444 | &ahci_platform_sht); | 480 | &ahci_platform_sht); |
| 445 | if (ret) | 481 | if (ret) |
| 446 | return ret; | 482 | goto out_disable_platform_phys; |
| 447 | 483 | ||
| 448 | dev_info(dev, "Broadcom AHCI SATA3 registered\n"); | 484 | dev_info(dev, "Broadcom AHCI SATA3 registered\n"); |
| 449 | 485 | ||
| 450 | return 0; | 486 | return 0; |
| 487 | |||
| 488 | out_disable_platform_phys: | ||
| 489 | ahci_platform_disable_phys(hpriv); | ||
| 490 | out_disable_phys: | ||
| 491 | brcm_sata_phys_disable(priv); | ||
| 492 | out_disable_clks: | ||
| 493 | ahci_platform_disable_clks(hpriv); | ||
| 494 | out_reset: | ||
| 495 | if (!IS_ERR_OR_NULL(priv->rcdev)) | ||
| 496 | reset_control_assert(priv->rcdev); | ||
| 497 | return ret; | ||
| 451 | } | 498 | } |
| 452 | 499 | ||
| 453 | static int brcm_ahci_remove(struct platform_device *pdev) | 500 | static int brcm_ahci_remove(struct platform_device *pdev) |
| @@ -457,12 +504,12 @@ static int brcm_ahci_remove(struct platform_device *pdev) · 3 unmodified lines | |||
| 457 | struct brcm_ahci_priv *priv = hpriv->plat_data; | 504 | struct brcm_ahci_priv *priv = hpriv->plat_data; |
| 458 | int ret; | 505 | int ret; |
| 459 | 506 | ||
| 507 | brcm_sata_phys_disable(priv); | ||
| 508 | |||
| 460 | ret = ata_platform_remove_one(pdev); | 509 | ret = ata_platform_remove_one(pdev); |
| 461 | if (ret) | 510 | if (ret) |
| 462 | return ret; | 511 | return ret; |
| 463 | 512 | ||
| 464 | brcm_sata_phys_disable(priv); | ||
| 465 | |||
| 466 | return 0; | 513 | return 0; |
| 467 | } | 514 | } |
| 468 | 515 | ||