Message ID | 20200527175635.5558-1-zhouyanjie@wanyeetech.com |
---|---|
Headers | show |
Series | Add support for the X1830 and fix bugs for X1000. | expand |
Quoting Zhou Yanjie (2020-05-27 10:56:35) > @@ -40,8 +43,47 @@ > #define OPCR_SPENDN0 BIT(7) > #define OPCR_SPENDN1 BIT(6) > > +/* bits within the USBPCR register */ > +#define USBPCR_SIDDQ BIT(21) > +#define USBPCR_OTG_DISABLE BIT(20) > + > static struct ingenic_cgu *cgu; > > +static int x1000_usb_phy_enable(struct clk_hw *hw) > +{ > + void __iomem *reg_opcr = cgu->base + CGU_REG_OPCR; > + void __iomem *reg_usbpcr = cgu->base + CGU_REG_USBPCR; > + > + writel(readl(reg_opcr) | OPCR_SPENDN0, reg_opcr); Please include linux/io.h for writel/readl. > + writel(readl(reg_usbpcr) & ~USBPCR_OTG_DISABLE & ~USBPCR_SIDDQ, reg_usbpcr); > + return 0; > +} > + > +static void x1000_usb_phy_disable(struct clk_hw *hw) > +{ > + void __iomem *reg_opcr = cgu->base + CGU_REG_OPCR; > + void __iomem *reg_usbpcr = cgu->base + CGU_REG_USBPCR; > + > + writel(readl(reg_opcr) & ~OPCR_SPENDN0, reg_opcr); > + writel(readl(reg_usbpcr) | USBPCR_OTG_DISABLE | USBPCR_SIDDQ, reg_usbpcr); > +} > + > +static int x1000_usb_phy_is_enabled(struct clk_hw *hw) > +{ > + void __iomem *reg_opcr = cgu->base + CGU_REG_OPCR; > + void __iomem *reg_usbpcr = cgu->base + CGU_REG_USBPCR; > + > + return (readl(reg_opcr) & OPCR_SPENDN0) && > + !(readl(reg_usbpcr) & USBPCR_SIDDQ) && > + !(readl(reg_usbpcr) & USBPCR_OTG_DISABLE); > +} > + > +static const struct clk_ops x1000_otg_phy_ops = { > + .enable = x1000_usb_phy_enable, > + .disable = x1000_usb_phy_disable, > + .is_enabled = x1000_usb_phy_is_enabled, > +}; > + > static const s8 pll_od_encoding[8] = { > 0x0, 0x1, -1, 0x2, -1, -1, -1, 0x3, > }; > @@ -277,4 +377,4 @@ static void __init x1000_cgu_init(struct device_node *np) > > ingenic_cgu_register_syscore_ops(cgu); > } > -CLK_OF_DECLARE(x1000_cgu, "ingenic,x1000-cgu", x1000_cgu_init); > +CLK_OF_DECLARE_DRIVER(x1000_cgu, "ingenic,x1000-cgu", x1000_cgu_init); Why does this change to DECLARE_DRIVER? Can you please add a comment here in the code indicating what other driver is probing this compatible string?
Quoting Zhou Yanjie (2020-05-27 10:56:33) > diff --git a/drivers/clk/ingenic/x1830-cgu.c b/drivers/clk/ingenic/x1830-cgu.c > new file mode 100644 > index 000000000000..29a637f4a2cc > --- /dev/null > +++ b/drivers/clk/ingenic/x1830-cgu.c > @@ -0,0 +1,443 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * X1830 SoC CGU driver > + * Copyright (c) 2019 \u5468\u7430\u6770 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> > + */ > + > +#include <linux/clk-provider.h> > +#include <linux/delay.h> Add linux/io.h here. > +#include <linux/of.h> > + > +#include <dt-bindings/clock/x1830-cgu.h> > + > +#include "cgu.h" [...] > + return; > + } > + > + ingenic_cgu_register_syscore_ops(cgu); > +} > +CLK_OF_DECLARE_DRIVER(x1830_cgu, "ingenic,x1830-cgu", x1830_cgu_init); Same question about why this is DECLARE_DRIVER.
Hi Stephen, 在 2020/5/28 上午9:12, Stephen Boyd 写道: > Quoting Zhou Yanjie (2020-05-27 10:56:35) >> @@ -40,8 +43,47 @@ >> #define OPCR_SPENDN0 BIT(7) >> #define OPCR_SPENDN1 BIT(6) >> >> +/* bits within the USBPCR register */ >> +#define USBPCR_SIDDQ BIT(21) >> +#define USBPCR_OTG_DISABLE BIT(20) >> + >> static struct ingenic_cgu *cgu; >> >> +static int x1000_usb_phy_enable(struct clk_hw *hw) >> +{ >> + void __iomem *reg_opcr = cgu->base + CGU_REG_OPCR; >> + void __iomem *reg_usbpcr = cgu->base + CGU_REG_USBPCR; >> + >> + writel(readl(reg_opcr) | OPCR_SPENDN0, reg_opcr); > Please include linux/io.h for writel/readl. Sure, I'll add it. >> + writel(readl(reg_usbpcr) & ~USBPCR_OTG_DISABLE & ~USBPCR_SIDDQ, reg_usbpcr); >> + return 0; >> +} >> + >> +static void x1000_usb_phy_disable(struct clk_hw *hw) >> +{ >> + void __iomem *reg_opcr = cgu->base + CGU_REG_OPCR; >> + void __iomem *reg_usbpcr = cgu->base + CGU_REG_USBPCR; >> + >> + writel(readl(reg_opcr) & ~OPCR_SPENDN0, reg_opcr); >> + writel(readl(reg_usbpcr) | USBPCR_OTG_DISABLE | USBPCR_SIDDQ, reg_usbpcr); >> +} >> + >> +static int x1000_usb_phy_is_enabled(struct clk_hw *hw) >> +{ >> + void __iomem *reg_opcr = cgu->base + CGU_REG_OPCR; >> + void __iomem *reg_usbpcr = cgu->base + CGU_REG_USBPCR; >> + >> + return (readl(reg_opcr) & OPCR_SPENDN0) && >> + !(readl(reg_usbpcr) & USBPCR_SIDDQ) && >> + !(readl(reg_usbpcr) & USBPCR_OTG_DISABLE); >> +} >> + >> +static const struct clk_ops x1000_otg_phy_ops = { >> + .enable = x1000_usb_phy_enable, >> + .disable = x1000_usb_phy_disable, >> + .is_enabled = x1000_usb_phy_is_enabled, >> +}; >> + >> static const s8 pll_od_encoding[8] = { >> 0x0, 0x1, -1, 0x2, -1, -1, -1, 0x3, >> }; >> @@ -277,4 +377,4 @@ static void __init x1000_cgu_init(struct device_node *np) >> >> ingenic_cgu_register_syscore_ops(cgu); >> } >> -CLK_OF_DECLARE(x1000_cgu, "ingenic,x1000-cgu", x1000_cgu_init); >> +CLK_OF_DECLARE_DRIVER(x1000_cgu, "ingenic,x1000-cgu", x1000_cgu_init); > Why does this change to DECLARE_DRIVER? Can you please add a comment > here in the code indicating what other driver is probing this compatible > string? Yes, CGU has some children devices, this is useful for probing children devices in the case where the device node is compatible with "simple-mfd" (see commit 03d570e1a4dc for a reference). Thanks and best regards!
Hi Stephen, 在 2020/5/28 上午9:13, Stephen Boyd 写道: > Quoting Zhou Yanjie (2020-05-27 10:56:33) >> diff --git a/drivers/clk/ingenic/x1830-cgu.c b/drivers/clk/ingenic/x1830-cgu.c >> new file mode 100644 >> index 000000000000..29a637f4a2cc >> --- /dev/null >> +++ b/drivers/clk/ingenic/x1830-cgu.c >> @@ -0,0 +1,443 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * X1830 SoC CGU driver >> + * Copyright (c) 2019 \u5468\u7430\u6770 (Zhou Yanjie) <zhouyanjie@wanyeetech.com> >> + */ >> + >> +#include <linux/clk-provider.h> >> +#include <linux/delay.h> > Add linux/io.h here. Sure. >> +#include <linux/of.h> >> + >> +#include <dt-bindings/clock/x1830-cgu.h> >> + >> +#include "cgu.h" > [...] >> + return; >> + } >> + >> + ingenic_cgu_register_syscore_ops(cgu); >> +} >> +CLK_OF_DECLARE_DRIVER(x1830_cgu, "ingenic,x1830-cgu", x1830_cgu_init); > Same question about why this is DECLARE_DRIVER. CGU has some children devices, this is useful for probing children devices in the case where the device node is compatible with "simple-mfd" (see commit 03d570e1a4dc for a reference). Thanks and best regards!