Hello,

On Fri, Oct 30, 2015 at 11:01:16PM +0900, Jaedon Shin wrote:
> +static void brcm_sata_quirks(struct platform_device *pdev,
> +                          struct brcm_ahci_priv *priv)
> +{
> +     if (priv->quirks & BRCM_AHCI_QUIRK_NONCQ) {
> +             void __iomem *ctrl = priv->top_ctrl + SATA_TOP_CTRL_BUS_CTRL;
> +             void __iomem *ahci;
> +             struct resource *res;
> +             u32 reg;
> +
> +             res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> +                                                "ahci");
> +             ahci = devm_ioremap_resource(&pdev->dev, res);
> +             if (IS_ERR(ahci))
> +                     return;
> +
> +             reg = brcm_sata_readreg(ctrl);
> +             reg |= OVERRIDE_HWINIT;
> +             brcm_sata_writereg(reg, ctrl);
> +
> +             /* Clear out the NCQ bit so the AHCI driver will not issue
> +              * FPDMA/NCQ commands.
> +              */
> +             reg = readl(ahci + HOST_CAP);
> +             reg &= ~HOST_CAP_NCQ;
> +             writel(reg, ahci + HOST_CAP);
> +
> +             reg = brcm_sata_readreg(ctrl);
> +             reg &= ~OVERRIDE_HWINIT;
> +             brcm_sata_writereg(reg, ctrl);
> +
> +             devm_iounmap(&pdev->dev, ahci);
> +             devm_release_mem_region(&pdev->dev, res->start,
> +                                     resource_size(res));
> +     }
> +}

Does the controller actually need HOST_CAP_NCQ bit turned off to work
correctly?  If the only thing necessary is the host not issuing NCQ
commands, setting AHCI_HFLAG_NO_NCQ should do.

>  static void brcm_sata_init(struct brcm_ahci_priv *priv)
>  {
>       /* Configure endianness */
> @@ -256,6 +297,11 @@ static int brcm_ahci_probe(struct platform_device *pdev)
>       if (IS_ERR(priv->top_ctrl))
>               return PTR_ERR(priv->top_ctrl);
>  
> +     if (of_device_is_compatible(dev->of_node, "brcm,bcm7425-ahci"))
> +             priv->quirks |= BRCM_AHCI_QUIRK_NONCQ;
> +
> +     brcm_sata_quirks(pdev, priv);

What's the point of "branch - set a bit - test the bit" sequence?
Just do

        if (of_device_is_compatiable(...))
                brcm_ahci_disable_ncq(...);

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to