> Il 03/04/24 18:20, Lorenzo Bianconi ha scritto:
> > Introduce EN7581 clock support to clk-en7523 driver.
> >
> > Tested-by: Zhengping Zhang <zhengping.zhang@xxxxxxxxxx>
> > Signed-off-by: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
> > ---
> > drivers/clk/clk-en7523.c | 130 +++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 125 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
> > index c7def87b74c6..51a6c0cc7f58 100644
> > --- a/drivers/clk/clk-en7523.c
> > +++ b/drivers/clk/clk-en7523.c
> > @@ -4,13 +4,16 @@
> > #include <linux/clk-provider.h>
> > #include <linux/io.h>
> > #include <linux/of.h>
> > +#include <linux/of_device.h>
> > #include <linux/platform_device.h>
> > #include <dt-bindings/clock/en7523-clk.h>
> > #define REG_PCI_CONTROL 0x88
> > #define REG_PCI_CONTROL_PERSTOUT BIT(29)
> > #define REG_PCI_CONTROL_PERSTOUT1 BIT(26)
> > +#define REG_PCI_CONTROL_REFCLK_EN0 BIT(23)
> > #define REG_PCI_CONTROL_REFCLK_EN1 BIT(22)
> > +#define REG_PCI_CONTROL_PERSTOUT2 BIT(16)
> > #define REG_GSW_CLK_DIV_SEL 0x1b4
> > #define REG_EMI_CLK_DIV_SEL 0x1b8
> > #define REG_BUS_CLK_DIV_SEL 0x1bc
> > @@ -18,10 +21,25 @@
> > #define REG_SPI_CLK_FREQ_SEL 0x1c8
> > #define REG_NPU_CLK_DIV_SEL 0x1fc
> > #define REG_CRYPTO_CLKSRC 0x200
> > -#define REG_RESET_CONTROL 0x834
> > +#define REG_RESET_CONTROL2 0x830
>
> Wait what? The RESET2 register comes before RESET1 ?!?!
>
> Is this a typo? :-)
actually not :)
>
> > +#define REG_RESET2_CONTROL_PCIE2 BIT(27)
> > +#define REG_RESET_CONTROL1 0x834
> > #define REG_RESET_CONTROL_PCIEHB BIT(29)
> > #define REG_RESET_CONTROL_PCIE1 BIT(27)
> > #define REG_RESET_CONTROL_PCIE2 BIT(26)
> > +/* EN7581 */
> > +#define REG_PCIE0_MEM 0x00
> > +#define REG_PCIE0_MEM_MASK 0x04
> > +#define REG_PCIE1_MEM 0x08
> > +#define REG_PCIE1_MEM_MASK 0x0c
> > +#define REG_PCIE2_MEM 0x10
> > +#define REG_PCIE2_MEM_MASK 0x14
> > +#define REG_PCIE_RESET_OPEN_DRAIN 0x018c
> > +#define REG_PCIE_RESET_OPEN_DRAIN_MASK GENMASK(2, 0)
> > +#define REG_NP_SCU_PCIC 0x88
> > +#define REG_NP_SCU_SSTR 0x9c
> > +#define REG_PCIE_XSI0_SEL_MASK GENMASK(14, 13)
> > +#define REG_PCIE_XSI1_SEL_MASK GENMASK(12, 11)
> > struct en_clk_desc {
> > int id;
> > @@ -207,14 +225,14 @@ static int en7523_pci_prepare(struct clk_hw *hw)
> > usleep_range(1000, 2000);
> > /* Reset to default */
> > - val = readl(np_base + REG_RESET_CONTROL);
> > + val = readl(np_base + REG_RESET_CONTROL1);
> > mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
> > REG_RESET_CONTROL_PCIEHB;
> > - writel(val & ~mask, np_base + REG_RESET_CONTROL);
> > + writel(val & ~mask, np_base + REG_RESET_CONTROL1);
> > usleep_range(1000, 2000);
> > - writel(val | mask, np_base + REG_RESET_CONTROL);
> > + writel(val | mask, np_base + REG_RESET_CONTROL1);
> > msleep(100);
> > - writel(val & ~mask, np_base + REG_RESET_CONTROL);
> > + writel(val & ~mask, np_base + REG_RESET_CONTROL1);
> > usleep_range(5000, 10000);
> > /* Release device */
> > @@ -262,6 +280,64 @@ static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
> > return &cg->hw;
> > }
> > +static int en7581_pci_is_enabled(struct clk_hw *hw)
> > +{
> > + struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
> > + u32 val, mask;
> > +
> > + mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1;
> > + val = readl(cg->base + REG_PCI_CONTROL);
> > + return (val & mask) == mask;
> > +}
> > +
> > +static int en7581_pci_prepare(struct clk_hw *hw)
> > +{
> > + struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
> > + void __iomem *np_base = cg->base;
> > + u32 val, mask;
> > +
> > + mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
> > + REG_RESET_CONTROL_PCIEHB;
> > + val = readl(np_base + REG_RESET_CONTROL1);
> > + writel(val & ~mask, np_base + REG_RESET_CONTROL1);
> > + val = readl(np_base + REG_RESET_CONTROL2);
> > + writel(val & ~REG_RESET2_CONTROL_PCIE2, np_base + REG_RESET_CONTROL2);
> > + usleep_range(5000, 10000);
> > +
> > + mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1 |
> > + REG_PCI_CONTROL_PERSTOUT1 | REG_PCI_CONTROL_PERSTOUT2 |
> > + REG_PCI_CONTROL_PERSTOUT;
>
> I'm not sure that this is actually something to control in a clock driver...
>
> the right thing to do would be to add a reset controller to this clock driver
> and then assert/deassert reset in the PCIe PHY/MAC driver.
>
> Perhaps REFCLK_EN0/EN1 can be manipulated in a .enable() callback, treating
> this really just as what it appears to really be: a gate clock! (hint: check
> clk-gate.c)
ack, I will look into it.
>
> > + val = readl(np_base + REG_PCI_CONTROL);
> > + writel(val | mask, np_base + REG_PCI_CONTROL);
> > + msleep(250);
> > +
> > + return 0;
> > +}
> > +
> > +static void en7581_pci_unprepare(struct clk_hw *hw)
> > +{
> > + struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
> > + void __iomem *np_base = cg->base;
> > + u32 val, mask;
> > +
> > + mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1 |
>
> ...and this should be a clk-gate .disable() callback, I guess :-)
ack, I will look into it.
>
> > + REG_PCI_CONTROL_PERSTOUT1 | REG_PCI_CONTROL_PERSTOUT2 |
> > + REG_PCI_CONTROL_PERSTOUT;
> > + val = readl(np_base + REG_PCI_CONTROL);
> > + writel(val & ~mask, np_base + REG_PCI_CONTROL);
> > + usleep_range(1000, 2000);
> > +
> > + mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
> > + REG_RESET_CONTROL_PCIEHB;
> > + val = readl(np_base + REG_RESET_CONTROL1);
> > + writel(val | mask, np_base + REG_RESET_CONTROL1);
> > + mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2;
> > + writel(val | mask, np_base + REG_RESET_CONTROL1);
> > + val = readl(np_base + REG_RESET_CONTROL2);
> > + writel(val | REG_RESET_CONTROL_PCIE2, np_base + REG_RESET_CONTROL2);
> > + msleep(100);
> > +}
> > +
> > static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_data *clk_data,
> > void __iomem *base, void __iomem *np_base)
> > {
> > @@ -291,6 +367,37 @@ static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_dat
> > clk_data->num = EN7523_NUM_CLOCKS;
> > }
> > +static int en7581_clk_hw_init(struct platform_device *pdev,
> > + void __iomem *base,
> > + void __iomem *np_base)
> > +{
> > + void __iomem *pb_base;
> > + u32 val;
> > +
> > + pb_base = devm_platform_ioremap_resource(pdev, 2);
> > + if (IS_ERR(pb_base))
> > + return PTR_ERR(pb_base);
> > +
> > + val = readl(np_base + REG_NP_SCU_SSTR);
> > + val &= ~(REG_PCIE_XSI0_SEL_MASK | REG_PCIE_XSI1_SEL_MASK);
> > + writel(val, np_base + REG_NP_SCU_SSTR);
> > + val = readl(np_base + REG_NP_SCU_PCIC);
> > + writel(val | 3, np_base + REG_NP_SCU_PCIC);
>
> What is 3?
>
> #define SOMETHING 3 ??
actullay I do not know what it means since write in the pcie_ctrl subfield of
REG_NP_SCU_PCIC but it is a GENMASK(7, 0) and I do not have any more info
about it.
>
> > +
> > + writel(0x20000000, pb_base + REG_PCIE0_MEM);
> > + writel(0xfc000000, pb_base + REG_PCIE0_MEM_MASK);
> > + writel(0x24000000, pb_base + REG_PCIE1_MEM);
> > + writel(0xfc000000, pb_base + REG_PCIE1_MEM_MASK);
> > + writel(0x28000000, pb_base + REG_PCIE2_MEM);
> > + writel(0xfc000000, pb_base + REG_PCIE2_MEM_MASK);
>
> And... this is .. some BIT() and some GENMASK() as far as I understand...
> do we have any clue about what you're setting to those registers?
same as above, they seems undocumented.
@airoha folks: any input about them?
>
> Can MediaTek/Airoha help with this, please?
>
> #define SOMETHING BIT(29) /* this is 0x20000000 */
> #define SOME_MASK GENMASK(31, 26) /* this is 0xfc00000 */
>
> > +
> > + val = readl(base + REG_PCIE_RESET_OPEN_DRAIN);
> > + writel(val | REG_PCIE_RESET_OPEN_DRAIN_MASK,
> > + base + REG_PCIE_RESET_OPEN_DRAIN);
> > +
> > + return 0;
> > +}
> > +
> > static int en7523_clk_probe(struct platform_device *pdev)
> > {
> > struct device_node *node = pdev->dev.of_node;
> > @@ -306,6 +413,12 @@ static int en7523_clk_probe(struct platform_device *pdev)
> > if (IS_ERR(np_base))
> > return PTR_ERR(np_base);
> > + if (of_device_is_compatible(node, "airoha,en7581-scu")) {
> > + r = en7581_clk_hw_init(pdev, base, np_base);
> > + if (r)
> > + return r;
> > + }
> > +
> > clk_data = devm_kzalloc(&pdev->dev,
> > struct_size(clk_data, hws, EN7523_NUM_CLOCKS),
> > GFP_KERNEL);
> > @@ -329,8 +442,15 @@ static const struct clk_ops en7523_pcie_ops = {
> > .unprepare = en7523_pci_unprepare,
> > };
>
> static const struct clk_en7523_pdata en7581_pdata = {
> .init = en7581_clk_hw_init, /* if (pdata->init) pdata->init(x, y, z) */
> .ops = en7581_pcie_ops,
> };
>
> or, alternatively:
>
> static const struct .... = {
> .init = ...,
> .ops = (const struct clk_ops) {
> .is_enabled = en7581_pci_is_enabled,
> .... etc
> }
ack, I will fix it.
Regards,
Lorenzo
> };
>
> Cheers,
> Angelo
>
> > +static const struct clk_ops en7581_pcie_ops = {
> > + .is_enabled = en7581_pci_is_enabled,
> > + .prepare = en7581_pci_prepare,
> > + .unprepare = en7581_pci_unprepare,
> > +};
> > +
> > static const struct of_device_id of_match_clk_en7523[] = {
> > { .compatible = "airoha,en7523-scu", .data = &en7523_pcie_ops },
> > + { .compatible = "airoha,en7581-scu", .data = &en7581_pcie_ops },
> > { /* sentinel */ }
> > };
>
> -
Attachment:
signature.asc
Description: PGP signature
