Re: [PATCH v4 02/10] PCI: imx6: add imx6sx pcie support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





> 在 2014年9月30日,下午10:54,"Lucas Stach" <l.stach@xxxxxxxxxxxxxx> 写道:
> 
> Am Dienstag, den 30.09.2014, 17:36 +0800 schrieb Richard Zhu:
>> - imx6sx pcie has its own standalone pcie power supply.
>> In order to turn on the imx6sx pcie power during
>> initialization. Add the pcie regulator and the gpc regmap
>> into the imx6sx pcie structure.
>> - imx6sx pcie has the new added reset mechanism, add the
>> reset operations into the initialization.
>> - Register one PM call-back, enter/exit L2 state of the ASPM
>> during system suspend/resume.
>> - disp_axi clock is required by pcie inbound axi port actually.
>> Add one more clock named pcie_inbound_axi for imx6sx pcie.
>> 
>> Signed-off-by: Richard Zhu <r65037@xxxxxxxxxxxxx>
>> ---
>> drivers/pci/host/pci-imx6.c | 163 ++++++++++++++++++++++++++++++++++++++------
>> 1 file changed, 144 insertions(+), 19 deletions(-)
>> 
>> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
>> index eac96fb..c74e87d 100644
>> --- a/drivers/pci/host/pci-imx6.c
>> +++ b/drivers/pci/host/pci-imx6.c
>> @@ -22,8 +22,10 @@
>> #include <linux/pci.h>
>> #include <linux/platform_device.h>
>> #include <linux/regmap.h>
>> +#include <linux/regulator/consumer.h>
>> #include <linux/resource.h>
>> #include <linux/signal.h>
>> +#include <linux/syscore_ops.h>
>> #include <linux/types.h>
>> #include <linux/interrupt.h>
>> 
>> @@ -35,11 +37,15 @@ struct imx6_pcie {
>>    int            reset_gpio;
>>    struct clk        *pcie_bus;
>>    struct clk        *pcie_phy;
>> +    struct clk        *pcie_inbound_axi;
>>    struct clk        *pcie;
>>    struct pcie_port    pp;
>>    struct regmap        *iomuxc_gpr;
>> +    struct regmap        *gpc_ips_reg;
>> +    struct regulator    *pcie_regulator;
> 
> I would expect this to be named pcie_phy_regulator.
Ok.
> 
>>    void __iomem        *mem_base;
>> };
>> +static struct imx6_pcie *imx6_pcie;
> 
> No. This is just bad style. You have the pointer available embedded in
> other structures at all relevant places. No global statics please.
Ok.
> 
>> 
>> /* PCIe Root Complex registers (memory-mapped) */
>> #define PCIE_RC_LCR                0x7c
>> @@ -77,6 +83,18 @@ struct imx6_pcie {
>> #define PHY_RX_OVRD_IN_LO_RX_DATA_EN (1 << 5)
>> #define PHY_RX_OVRD_IN_LO_RX_PLL_EN (1 << 3)
>> 
>> +/* GPC PCIE PHY bit definitions */
>> +#define GPC_CNTR            0
>> +#define GPC_CNTR_PCIE_PHY_PUP_REQ    BIT(7)
>> +
>> +static inline bool is_imx6sx_pcie(struct imx6_pcie *imx6_pcie)
>> +{
>> +    struct pcie_port *pp = &imx6_pcie->pp;
>> +    struct device_node *np = pp->dev->of_node;
>> +
>> +    return of_device_is_compatible(np, "fsl,imx6sx-pcie");
>> +}
>> +
>> static int pcie_phy_poll_ack(void __iomem *dbi_base, int exp_val)
>> {
>>    u32 val;
>> @@ -275,18 +293,29 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
>>        goto err_pcie;
>>    }
>> 
>> -    /* power up core phy and enable ref clock */
>> -    regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
>> -            IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
>> -    /*
>> -     * the async reset input need ref clock to sync internally,
>> -     * when the ref clock comes after reset, internal synced
>> -     * reset time is too short , cannot meet the requirement.
>> -     * add one ~10us delay here.
>> -     */
>> -    udelay(10);
>> -    regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
>> -            IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
>> +    if (is_imx6sx_pcie(imx6_pcie)) {
>> +        ret = clk_prepare_enable(imx6_pcie->pcie_inbound_axi);
>> +        if (ret) {
>> +            dev_err(pp->dev, "unable to enable pcie clock\n");
>> +            goto err_inbound_axi;
>> +        }
>> +
>> +        regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>> +                IMX6SX_GPR12_PCIE_TEST_PD, 0);
>> +    } else {
>> +        /* power up core phy and enable ref clock */
>> +        regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
>> +                IMX6Q_GPR1_PCIE_TEST_PD, 0);
>> +        /*
>> +         * the async reset input need ref clock to sync internally,
>> +         * when the ref clock comes after reset, internal synced
>> +         * reset time is too short , cannot meet the requirement.
>> +         * add one ~10us delay here.
>> +         */
>> +        udelay(10);
>> +        regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
>> +                IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
>> +    }
>> 
>>    /* allow the clocks to stabilize */
>>    usleep_range(200, 500);
>> @@ -297,8 +326,19 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
>>        msleep(100);
>>        gpio_set_value(imx6_pcie->reset_gpio, 1);
>>    }
>> +
>> +    /*
>> +     * Release the PCIe PHY reset here, that we have set in
>> +     * imx6_pcie_init_phy() now
>> +     */
>> +    if (is_imx6sx_pcie(imx6_pcie))
>> +        regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
>> +                IMX6SX_GPR5_PCIE_BTNRST, 0);
>> +
>>    return 0;
>> 
>> +err_inbound_axi:
>> +    clk_disable_unprepare(imx6_pcie->pcie);
>> err_pcie:
>>    clk_disable_unprepare(imx6_pcie->pcie_bus);
>> err_pcie_bus:
>> @@ -311,6 +351,26 @@ err_pcie_phy:
>> static void imx6_pcie_init_phy(struct pcie_port *pp)
>> {
>>    struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
>> +    int ret;
>> +
>> +    /* Power up the separate domain available on i.MX6SX */
>> +    if (is_imx6sx_pcie(imx6_pcie)) {
>> +        /* Force PCIe PHY reset */
>> +        regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
>> +                IMX6SX_GPR5_PCIE_BTNRST,
>> +                IMX6SX_GPR5_PCIE_BTNRST);
>> +
>> +        regmap_update_bits(imx6_pcie->gpc_ips_reg, GPC_CNTR,
>> +                GPC_CNTR_PCIE_PHY_PUP_REQ,
>> +                GPC_CNTR_PCIE_PHY_PUP_REQ);
>> +        regulator_set_voltage(imx6_pcie->pcie_regulator,
>> +                1100000, 1100000);
>> +        ret = regulator_enable(imx6_pcie->pcie_regulator);
>> +        if (ret)
>> +            dev_info(pp->dev, "failed to enable pcie regulator.\n");
>> +        regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>> +                IMX6SX_GPR12_RX_EQ_MASK, IMX6SX_GPR12_RX_EQ_2);
>> +    }
>> 
>>    regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>>            IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
>> @@ -319,7 +379,7 @@ static void imx6_pcie_init_phy(struct pcie_port *pp)
>>    regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>>            IMX6Q_GPR12_DEVICE_TYPE, PCI_EXP_TYPE_ROOT_PORT << 12);
>>    regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>> -            IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
>> +            IMX6Q_GPR12_LOS_LEVEL, IMX6Q_GPR12_LOS_LEVEL_9);
>> 
>>    regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
>>            IMX6Q_GPR8_TX_DEEMPH_GEN1, 0 << 0);
>> @@ -377,7 +437,8 @@ static int imx6_pcie_start_link(struct pcie_port *pp)
>> 
>>    /* Start LTSSM. */
>>    regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>> -            IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
>> +            IMX6Q_GPR12_PCIE_CTL_2,
>> +            IMX6Q_GPR12_PCIE_CTL_2);
>> 
>>    ret = imx6_pcie_wait_for_link(pp);
>>    if (ret)
>> @@ -553,9 +614,50 @@ static int __init imx6_add_pcie_port(struct pcie_port *pp,
>>    return 0;
>> }
>> 
>> +#ifdef CONFIG_PM_SLEEP
>> +static int pci_imx_suspend(void)
>> +{
>> +    if (is_imx6sx_pcie(imx6_pcie)) {
>> +        /* PM_TURN_OFF */
>> +        regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>> +                IMX6SX_GPR12_PCIE_PM_TURN_OFF,
>> +                IMX6SX_GPR12_PCIE_PM_TURN_OFF);
>> +        udelay(10);
>> +        regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>> +                IMX6SX_GPR12_PCIE_PM_TURN_OFF, 0);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void pci_imx_resume(void)
>> +{
>> +    struct pcie_port *pp = &imx6_pcie->pp;
>> +
>> +    if (is_imx6sx_pcie(imx6_pcie)) {
>> +        /* Reset iMX6SX PCIe */
>> +        regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
>> +                IMX6SX_GPR5_PCIE_PERST, IMX6SX_GPR5_PCIE_PERST);
>> +        regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
>> +                IMX6SX_GPR5_PCIE_PERST, 0);
>> +        /*
>> +         * controller maybe turn off, re-configure again
>> +         */
>> +        dw_pcie_setup_rc(pp);
>> +
>> +        if (IS_ENABLED(CONFIG_PCI_MSI))
>> +            dw_pcie_msi_cfg_restore(pp);
>> +    }
>> +}
>> +
>> +static struct syscore_ops pci_imx_syscore_ops = {
>> +    .suspend = pci_imx_suspend,
>> +    .resume = pci_imx_resume,
>> +};
>> +#endif
>> +
>> static int __init imx6_pcie_probe(struct platform_device *pdev)
>> {
>> -    struct imx6_pcie *imx6_pcie;
>>    struct pcie_port *pp;
>>    struct device_node *np = pdev->dev.of_node;
>>    struct resource *dbi_base;
>> @@ -572,7 +674,7 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
>>    hook_fault_code(16 + 6, imx6q_pcie_abort_handler, SIGBUS, 0,
>>        "imprecise external abort");
>> 
>> -    dbi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +    dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbi");
> 
> You are breaking old devicetrees here. "rc_dbi" isn't a documented name
> and isn't available on old DTs. Besides the imx6q DT uses just "dbi" as
> the name. Don't touch this code, it does exactly the right thing by
> grabbing the first reg resource.

Ok, changes would be removed.

> 
>>    pp->dbi_base = devm_ioremap_resource(&pdev->dev, dbi_base);
>>    if (IS_ERR(pp->dbi_base))
>>        return PTR_ERR(pp->dbi_base);
>> @@ -610,9 +712,28 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
>>        return PTR_ERR(imx6_pcie->pcie);
>>    }
>> 
>> -    /* Grab GPR config register range */
>> -    imx6_pcie->iomuxc_gpr =
>> -         syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
>> +    if (is_imx6sx_pcie(imx6_pcie)) {
>> +        imx6_pcie->pcie_inbound_axi = devm_clk_get(&pdev->dev,
>> +                "pcie_inbound_axi");
>> +        if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {
>> +            dev_err(&pdev->dev,
>> +                "pcie clock source missing or invalid\n");
>> +            return PTR_ERR(imx6_pcie->pcie_inbound_axi);
>> +        }
>> +
>> +        imx6_pcie->pcie_regulator = devm_regulator_get(pp->dev,
>> +                "pcie-phy");
>> +
>> +        imx6_pcie->iomuxc_gpr =
>> +             syscon_regmap_lookup_by_compatible
>> +             ("fsl,imx6sx-iomuxc-gpr");
>> +        imx6_pcie->gpc_ips_reg =
>> +             syscon_regmap_lookup_by_compatible("fsl,imx6sx-gpc");
>> +    } else {
>> +        imx6_pcie->iomuxc_gpr =
>> +            syscon_regmap_lookup_by_compatible
>> +            ("fsl,imx6q-iomuxc-gpr");
>> +    }
>>    if (IS_ERR(imx6_pcie->iomuxc_gpr)) {
>>        dev_err(&pdev->dev, "unable to find iomuxc registers\n");
>>        return PTR_ERR(imx6_pcie->iomuxc_gpr);
>> @@ -623,6 +744,9 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
>>        return ret;
>> 
>>    platform_set_drvdata(pdev, imx6_pcie);
>> +#ifdef CONFIG_PM_SLEEP
>> +    register_syscore_ops(&pci_imx_syscore_ops);
>> +#endif
>>    return 0;
>> }
>> 
>> @@ -636,6 +760,7 @@ static void imx6_pcie_shutdown(struct platform_device *pdev)
>> 
>> static const struct of_device_id imx6_pcie_of_match[] = {
>>    { .compatible = "fsl,imx6q-pcie", },
>> +    { .compatible = "fsl,imx6sx-pcie", },
>>    {},
>> };
>> MODULE_DEVICE_TABLE(of, imx6_pcie_of_match);
> 
> -- 
> Pengutronix e.K.             | Lucas Stach                 |
> Industrial Linux Solutions   | http://www.pengutronix.de/  |
> 
> 
Best regards
Richard?韬{.n?????%??檩??w?{.n???{炳???骅w*jg????????G??⒏⒎?:+v????????????"??????




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux