| Message ID | 1576752109-24497-1-git-send-email-sricharan@codeaurora.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add minimal boot support for IPQ6018 | expand |
Quoting Sricharan R (2019-12-19 02:41:47) > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig > index 3b33ef1..d0392f1 100644 > --- a/drivers/clk/qcom/Kconfig > +++ b/drivers/clk/qcom/Kconfig > @@ -95,6 +95,14 @@ config IPQ_GCC_4019 > Say Y if you want to use peripheral devices such as UART, SPI, > i2c, USB, SD/eMMC, etc. > > +config IPQ_GCC_6018 > + tristate "IPQ6018 Global Clock Controller" > + help > + Support for global clock controller on ipq6018 devices. > + Say Y if you want to use peripheral devices such as UART, SPI, > + i2c, USB, SD/eMMC, etc. Select this for the root clock > + of ipq6018. What is the root clock of ipq6018? > + > config IPQ_GCC_806X > tristate "IPQ806x Global Clock Controller" > help > diff --git a/drivers/clk/qcom/gcc-ipq6018.c b/drivers/clk/qcom/gcc-ipq6018.c > new file mode 100644 > index 0000000..b6f0148 > --- /dev/null > +++ b/drivers/clk/qcom/gcc-ipq6018.c > @@ -0,0 +1,4674 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > + */ > + > +#include <linux/kernel.h> > +#include <linux/err.h> > +#include <linux/platform_device.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/clk-provider.h> > +#include <linux/regmap.h> > + > +#include <linux/reset-controller.h> > +#include <dt-bindings/clock/qcom,gcc-ipq6018.h> > +#include <dt-bindings/reset/qcom,gcc-ipq6018.h> > + > +#include "common.h" > +#include "clk-regmap.h" > +#include "clk-pll.h" > +#include "clk-rcg.h" > +#include "clk-branch.h" > +#include "clk-alpha-pll.h" > +#include "clk-regmap-divider.h" > +#include "clk-regmap-mux.h" > +#include "reset.h" > + > +#define F(f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) } > + > +enum { > + P_XO, > + P_BIAS_PLL, > + P_UNIPHY0_RX, > + P_UNIPHY0_TX, > + P_UNIPHY1_RX, > + P_BIAS_PLL_NSS_NOC, > + P_UNIPHY1_TX, > + P_PCIE20_PHY0_PIPE, > + P_USB3PHY_0_PIPE, > + P_GPLL0, > + P_GPLL0_DIV2, > + P_GPLL2, > + P_GPLL4, > + P_GPLL6, > + P_SLEEP_CLK, > + P_UBI32_PLL, > + P_NSS_CRYPTO_PLL, > + P_PI_SLEEP, > +}; > + > +static const struct clk_parent_data gcc_xo_gpll0_gpll0_out_main_div2[] = { > + { .fw_name = "xo", .name = "xo"}, > + { .fw_name = "gpll0", .name = "gpll0"}, > + { .fw_name = "gpll0_out_main_div2", .name = "gpll0_out_main_div2"}, Because we aren't migrating this from existing DT to new DT we should be able to leave out .name in all these structs. That's the legacy fallback mechanism used to migrate DT over to the new way. > +}; > + > +static const struct parent_map gcc_xo_gpll0_gpll0_out_main_div2_map[] = { > + { P_XO, 0 }, > + { P_GPLL0, 1 }, > + { P_GPLL0_DIV2, 4 }, > +}; > + [...] > + > +static int gcc_ipq6018_probe(struct platform_device *pdev) > +{ > + int i, ret; > + struct regmap *regmap; > + struct clk *clk; > + struct device *dev = &pdev->dev; > + > + regmap = qcom_cc_map(pdev, &gcc_ipq6018_desc); > + if (IS_ERR(regmap)) > + return PTR_ERR(regmap); > + > + for (i = 0; i < ARRAY_SIZE(gcc_ipq6018_hws); i++) { > + clk = devm_clk_register(&pdev->dev, gcc_ipq6018_hws[i]); > + if (IS_ERR(clk)) > + return PTR_ERR(clk); > + } > + > + clk_register_fixed_rate(dev, "pcie20_phy0_pipe_clk", NULL, 0, 250000000); Why do we need to register this? Can it come from DT then? Also what if it fails? And what if really_probe fails? Then we'll need to undo this registration. Ideally this is created somewhere else. > + > + /* Disable SW_COLLAPSE for USB0 GDSCR */ > + regmap_update_bits(regmap, 0x3e078, BIT(0), 0x0); > + /* Enable SW_OVERRIDE for USB0 GDSCR */ > + regmap_update_bits(regmap, 0x3e078, BIT(2), BIT(2)); > + /* Disable SW_COLLAPSE for USB1 GDSCR */ > + regmap_update_bits(regmap, 0x3f078, BIT(0), 0x0); > + /* Enable SW_OVERRIDE for USB1 GDSCR */ > + regmap_update_bits(regmap, 0x3f078, BIT(2), BIT(2)); > + > + /* SW Workaround for UBI Huyara PLL */ > + regmap_update_bits(regmap, 0x2501c, BIT(26), BIT(26)); > + > + clk_alpha_pll_configure(&ubi32_pll_main, regmap, &ubi32_pll_config); > + > + clk_alpha_pll_configure(&nss_crypto_pll_main, regmap, > + &nss_crypto_pll_config); > + > + ret = qcom_cc_really_probe(pdev, &gcc_ipq6018_desc, regmap); > + > + dev_dbg(&pdev->dev, "Registered ipq6018 clock provider"); Please remove this and just return the result of really_probe. > + > + return ret; > +}
Quoting Sricharan R (2019-12-19 02:41:42) > The IPQ6018 is Qualcomm\u2019s 802.11ax SoC for Routers, > Gateways and Access Points. > > This series adds minimal board boot support for ipq6018-cp01 board. > > [v2] > * Splitted dt bindings and driver into different patches. Added missing bindings > and some style changes. > * Added ipq6018 schema > * Addressed review comments for gcc clock bindings. > * Removed all clk critical flags, removed 1/1 factor clocks, moved to new > way of specifying clk parents, and addressed other review comments. > * Sorted nodes based on address, name, label. Removed unused clock nodes, > Addressed other review comments. Can you just send the clk and clk binding to me in a different series? I don't want to review all the other patches in the same thread.
Hi Stephen, Thanks for the review. On 12/27/2019 7:03 AM, Stephen Boyd wrote: > Quoting Sricharan R (2019-12-19 02:41:47) >> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig >> index 3b33ef1..d0392f1 100644 >> --- a/drivers/clk/qcom/Kconfig >> +++ b/drivers/clk/qcom/Kconfig >> @@ -95,6 +95,14 @@ config IPQ_GCC_4019 >> Say Y if you want to use peripheral devices such as UART, SPI, >> i2c, USB, SD/eMMC, etc. >> >> +config IPQ_GCC_6018 >> + tristate "IPQ6018 Global Clock Controller" >> + help >> + Support for global clock controller on ipq6018 devices. >> + Say Y if you want to use peripheral devices such as UART, SPI, >> + i2c, USB, SD/eMMC, etc. Select this for the root clock >> + of ipq6018. > > What is the root clock of ipq6018? > root clock is 'xo'. But i guess this statement is not correct. will remove that line. >> + >> config IPQ_GCC_806X >> tristate "IPQ806x Global Clock Controller" >> help >> diff --git a/drivers/clk/qcom/gcc-ipq6018.c b/drivers/clk/qcom/gcc-ipq6018.c >> new file mode 100644 >> index 0000000..b6f0148 >> --- /dev/null >> +++ b/drivers/clk/qcom/gcc-ipq6018.c >> @@ -0,0 +1,4674 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2018, The Linux Foundation. All rights reserved. >> + */ >> + >> +#include <linux/kernel.h> >> +#include <linux/err.h> >> +#include <linux/platform_device.h> >> +#include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/of_device.h> >> +#include <linux/clk-provider.h> >> +#include <linux/regmap.h> >> + >> +#include <linux/reset-controller.h> >> +#include <dt-bindings/clock/qcom,gcc-ipq6018.h> >> +#include <dt-bindings/reset/qcom,gcc-ipq6018.h> >> + >> +#include "common.h" >> +#include "clk-regmap.h" >> +#include "clk-pll.h" >> +#include "clk-rcg.h" >> +#include "clk-branch.h" >> +#include "clk-alpha-pll.h" >> +#include "clk-regmap-divider.h" >> +#include "clk-regmap-mux.h" >> +#include "reset.h" >> + >> +#define F(f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) } >> + >> +enum { >> + P_XO, >> + P_BIAS_PLL, >> + P_UNIPHY0_RX, >> + P_UNIPHY0_TX, >> + P_UNIPHY1_RX, >> + P_BIAS_PLL_NSS_NOC, >> + P_UNIPHY1_TX, >> + P_PCIE20_PHY0_PIPE, >> + P_USB3PHY_0_PIPE, >> + P_GPLL0, >> + P_GPLL0_DIV2, >> + P_GPLL2, >> + P_GPLL4, >> + P_GPLL6, >> + P_SLEEP_CLK, >> + P_UBI32_PLL, >> + P_NSS_CRYPTO_PLL, >> + P_PI_SLEEP, >> +}; >> + >> +static const struct clk_parent_data gcc_xo_gpll0_gpll0_out_main_div2[] = { >> + { .fw_name = "xo", .name = "xo"}, >> + { .fw_name = "gpll0", .name = "gpll0"}, >> + { .fw_name = "gpll0_out_main_div2", .name = "gpll0_out_main_div2"}, > > Because we aren't migrating this from existing DT to new DT we should be > able to leave out .name in all these structs. That's the legacy fallback > mechanism used to migrate DT over to the new way. > ok will fix it. >> +}; >> + >> +static const struct parent_map gcc_xo_gpll0_gpll0_out_main_div2_map[] = { >> + { P_XO, 0 }, >> + { P_GPLL0, 1 }, >> + { P_GPLL0_DIV2, 4 }, >> +}; >> + > [...] >> + >> +static int gcc_ipq6018_probe(struct platform_device *pdev) >> +{ >> + int i, ret; >> + struct regmap *regmap; >> + struct clk *clk; >> + struct device *dev = &pdev->dev; >> + >> + regmap = qcom_cc_map(pdev, &gcc_ipq6018_desc); >> + if (IS_ERR(regmap)) >> + return PTR_ERR(regmap); >> + >> + for (i = 0; i < ARRAY_SIZE(gcc_ipq6018_hws); i++) { >> + clk = devm_clk_register(&pdev->dev, gcc_ipq6018_hws[i]); >> + if (IS_ERR(clk)) >> + return PTR_ERR(clk); >> + } >> + >> + clk_register_fixed_rate(dev, "pcie20_phy0_pipe_clk", NULL, 0, 250000000); > > Why do we need to register this? Can it come from DT then? Also what if > it fails? And what if really_probe fails? Then we'll need to undo this > registration. Ideally this is created somewhere else. > ok, will move this in to the actual clk list. >> + >> + /* Disable SW_COLLAPSE for USB0 GDSCR */ >> + regmap_update_bits(regmap, 0x3e078, BIT(0), 0x0); >> + /* Enable SW_OVERRIDE for USB0 GDSCR */ >> + regmap_update_bits(regmap, 0x3e078, BIT(2), BIT(2)); >> + /* Disable SW_COLLAPSE for USB1 GDSCR */ >> + regmap_update_bits(regmap, 0x3f078, BIT(0), 0x0); >> + /* Enable SW_OVERRIDE for USB1 GDSCR */ >> + regmap_update_bits(regmap, 0x3f078, BIT(2), BIT(2)); >> + >> + /* SW Workaround for UBI Huyara PLL */ >> + regmap_update_bits(regmap, 0x2501c, BIT(26), BIT(26)); >> + >> + clk_alpha_pll_configure(&ubi32_pll_main, regmap, &ubi32_pll_config); >> + >> + clk_alpha_pll_configure(&nss_crypto_pll_main, regmap, >> + &nss_crypto_pll_config); >> + >> + ret = qcom_cc_really_probe(pdev, &gcc_ipq6018_desc, regmap); >> + >> + dev_dbg(&pdev->dev, "Registered ipq6018 clock provider"); > > Please remove this and just return the result of really_probe. > ok, will fix it Regards, Sricharan
Hi Stephen, On 12/27/2019 7:03 AM, Stephen Boyd wrote: > Quoting Sricharan R (2019-12-19 02:41:42) >> The IPQ6018 is Qualcomm\u2019s 802.11ax SoC for Routers, >> Gateways and Access Points. >> >> This series adds minimal board boot support for ipq6018-cp01 board. >> >> [v2] >> * Splitted dt bindings and driver into different patches. Added missing bindings >> and some style changes. >> * Added ipq6018 schema >> * Addressed review comments for gcc clock bindings. >> * Removed all clk critical flags, removed 1/1 factor clocks, moved to new >> way of specifying clk parents, and addressed other review comments. >> * Sorted nodes based on address, name, label. Removed unused clock nodes, >> Addressed other review comments. > > Can you just send the clk and clk binding to me in a different series? > I don't want to review all the other patches in the same thread. > Sure, will send separately. Regards, Sricharan