diff mbox

[v7,10/42] clk: davinci: New driver for davinci PSC clocks

Message ID 1519071723-31790-11-git-send-email-david@lechnology.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Lechner Feb. 19, 2018, 8:21 p.m. UTC
This adds a new driver for mach-davinci PSC clocks. This is porting the
code from arch/arm/mach-davinci/psc.c to the common clock framework and
is converting it to use regmap to simplify the code. Additionally, it
adds device tree support for these clocks.

Note: although there are similar clocks for TI Keystone we are not able
to share the code for a few reasons. The keystone clocks are device tree
only and use legacy one-node-per-clock bindings. Also the keystone
driver makes the assumption that there is only one PSC per SoC and uses
global variables, but here we have two controllers per SoC.

Signed-off-by: David Lechner <david@lechnology.com>
---

v7 changes:
- convert to platform device
- rename lpsc field to md
- rename PSC to LPSC where appropriate
- rename LPSC_ARM_RATE to LPSC_SET_RATE_PARENT
- add genpd provider
- add reset provider

v6 changes:
- use GENMASK
- add quirk flag for FORCE bit
- add quirk flag for propagating set_rate
- fix writing to PDSTAT instead of PDCTL
- remove unused doc comment parameter
- change davinci_psc_register_clocks() to handle registering clkdev entries


 drivers/clk/davinci/Makefile |   2 +
 drivers/clk/davinci/psc.c    | 495 +++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/davinci/psc.h    |  89 ++++++++
 3 files changed, 586 insertions(+)
 create mode 100644 drivers/clk/davinci/psc.c
 create mode 100644 drivers/clk/davinci/psc.h

Comments

Bartosz Golaszewski Feb. 28, 2018, 12:38 p.m. UTC | #1
2018-02-19 21:21 GMT+01:00 David Lechner <david@lechnology.com>:
> This adds a new driver for mach-davinci PSC clocks. This is porting the
> code from arch/arm/mach-davinci/psc.c to the common clock framework and
> is converting it to use regmap to simplify the code. Additionally, it
> adds device tree support for these clocks.
>
> Note: although there are similar clocks for TI Keystone we are not able
> to share the code for a few reasons. The keystone clocks are device tree
> only and use legacy one-node-per-clock bindings. Also the keystone
> driver makes the assumption that there is only one PSC per SoC and uses
> global variables, but here we have two controllers per SoC.
>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>
> v7 changes:
> - convert to platform device
> - rename lpsc field to md
> - rename PSC to LPSC where appropriate
> - rename LPSC_ARM_RATE to LPSC_SET_RATE_PARENT
> - add genpd provider
> - add reset provider
>
> v6 changes:
> - use GENMASK
> - add quirk flag for FORCE bit
> - add quirk flag for propagating set_rate
> - fix writing to PDSTAT instead of PDCTL
> - remove unused doc comment parameter
> - change davinci_psc_register_clocks() to handle registering clkdev entries
>
>
>  drivers/clk/davinci/Makefile |   2 +
>  drivers/clk/davinci/psc.c    | 495 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/clk/davinci/psc.h    |  89 ++++++++
>  3 files changed, 586 insertions(+)
>  create mode 100644 drivers/clk/davinci/psc.c
>  create mode 100644 drivers/clk/davinci/psc.h
>
> diff --git a/drivers/clk/davinci/Makefile b/drivers/clk/davinci/Makefile
> index d471386..cd1bf2c 100644
> --- a/drivers/clk/davinci/Makefile
> +++ b/drivers/clk/davinci/Makefile
> @@ -8,4 +8,6 @@ obj-$(CONFIG_ARCH_DAVINCI_DM355)        += pll-dm355.o
>  obj-$(CONFIG_ARCH_DAVINCI_DM365)       += pll-dm365.o
>  obj-$(CONFIG_ARCH_DAVINCI_DM644x)      += pll-dm644x.o
>  obj-$(CONFIG_ARCH_DAVINCI_DM646x)      += pll-dm646x.o
> +
> +obj-y += psc.o
>  endif
> diff --git a/drivers/clk/davinci/psc.c b/drivers/clk/davinci/psc.c
> new file mode 100644
> index 0000000..8e08ac8
> --- /dev/null
> +++ b/drivers/clk/davinci/psc.c
> @@ -0,0 +1,495 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Clock driver for TI Davinci PSC controllers
> + *
> + * Copyright (C) 2017 David Lechner <david@lechnology.com>
> + *
> + * Based on: drivers/clk/keystone/gate.c
> + * Copyright (C) 2013 Texas Instruments.
> + *     Murali Karicheri <m-karicheri2@ti.com>
> + *     Santosh Shilimkar <santosh.shilimkar@ti.com>
> + *
> + * And: arch/arm/mach-davinci/psc.c
> + * Copyright (C) 2006 Texas Instruments.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/clk.h>
> +#include <linux/clkdev.h>
> +#include <linux/err.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_clock.h>
> +#include <linux/pm_domain.h>
> +#include <linux/regmap.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +#include "psc.h"
> +
> +/* PSC register offsets */
> +#define EPCPR                  0x070
> +#define PTCMD                  0x120
> +#define PTSTAT                 0x128
> +#define PDSTAT(n)              (0x200 + 4 * (n))
> +#define PDCTL(n)               (0x300 + 4 * (n))
> +#define MDSTAT(n)              (0x800 + 4 * (n))
> +#define MDCTL(n)               (0xa00 + 4 * (n))
> +
> +/* PSC module states */
> +enum davinci_lpsc_state {
> +       LPSC_STATE_SWRSTDISABLE = 0,
> +       LPSC_STATE_SYNCRST      = 1,
> +       LPSC_STATE_DISABLE      = 2,
> +       LPSC_STATE_ENABLE       = 3,
> +};
> +
> +#define MDSTAT_STATE_MASK      GENMASK(5, 0)
> +#define MDSTAT_MCKOUT          BIT(12)
> +#define PDSTAT_STATE_MASK      GENMASK(4, 0)
> +#define MDCTL_FORCE            BIT(31)
> +#define MDCTL_LRESET           BIT(8)
> +#define PDCTL_EPCGOOD          BIT(8)
> +#define PDCTL_NEXT             BIT(0)
> +
> +struct davinci_psc_data {
> +       struct clk_onecell_data clk_data;
> +       struct genpd_onecell_data pm_data;
> +       struct reset_controller_dev rcdev;
> +};
> +
> +/**
> + * struct davinci_lpsc_clk - LPSC clock structure
> + * @hw: clk_hw for the LPSC
> + * @pm_domain: power domain for the LPSC
> + * @regmap: PSC MMIO region
> + * @md: Module domain (LPSC module id)
> + * @pd: Power domain
> + * @flags: LPSC_* quirk flags
> + */
> +struct davinci_lpsc_clk {
> +       struct clk_hw hw;
> +       struct generic_pm_domain pm_domain;
> +       struct regmap *regmap;
> +       u32 md;
> +       u32 pd;
> +       u32 flags;
> +};
> +
> +#define to_davinci_psc_data(x) container_of(x, struct davinci_psc_data, x)
> +#define to_davinci_lpsc_clk(x) container_of(x, struct davinci_lpsc_clk, x)
> +
> +static void davinci_lpsc_config(struct davinci_lpsc_clk *lpsc,
> +                               enum davinci_lpsc_state next_state)
> +{
> +       u32 epcpr, pdstat, mdstat, ptstat;
> +
> +       regmap_write_bits(lpsc->regmap, MDCTL(lpsc->md), MDSTAT_STATE_MASK,
> +                         next_state);
> +
> +       if (lpsc->flags & LPSC_FORCE)
> +               regmap_write_bits(lpsc->regmap, MDCTL(lpsc->md), MDCTL_FORCE,
> +                                 MDCTL_FORCE);
> +
> +       regmap_read(lpsc->regmap, PDSTAT(lpsc->pd), &pdstat);
> +       if ((pdstat & PDSTAT_STATE_MASK) == 0) {
> +               regmap_write_bits(lpsc->regmap, PDCTL(lpsc->pd), PDCTL_NEXT,
> +                                 PDCTL_NEXT);
> +
> +               regmap_write(lpsc->regmap, PTCMD, BIT(lpsc->pd));
> +
> +               regmap_read_poll_timeout(lpsc->regmap, EPCPR, epcpr,
> +                                        epcpr & BIT(lpsc->pd), 0, 0);
> +
> +               regmap_write_bits(lpsc->regmap, PDCTL(lpsc->pd), PDCTL_EPCGOOD,
> +                                 PDCTL_EPCGOOD);
> +       } else {
> +               regmap_write(lpsc->regmap, PTCMD, BIT(lpsc->pd));
> +       }
> +
> +       regmap_read_poll_timeout(lpsc->regmap, PTSTAT, ptstat,
> +                                !(ptstat & BIT(lpsc->pd)), 0, 0);
> +
> +       regmap_read_poll_timeout(lpsc->regmap, MDSTAT(lpsc->md), mdstat,
> +                                (mdstat & MDSTAT_STATE_MASK) == next_state,
> +                                0, 0);
> +}
> +
> +static int davinci_lpsc_clk_enable(struct clk_hw *hw)
> +{
> +       struct davinci_lpsc_clk *lpsc = to_davinci_lpsc_clk(hw);
> +
> +       davinci_lpsc_config(lpsc, LPSC_STATE_ENABLE);
> +
> +       return 0;
> +}
> +
> +static void davinci_lpsc_clk_disable(struct clk_hw *hw)
> +{
> +       struct davinci_lpsc_clk *lpsc = to_davinci_lpsc_clk(hw);
> +
> +       davinci_lpsc_config(lpsc, LPSC_STATE_DISABLE);
> +}
> +
> +static int davinci_lpsc_clk_is_enabled(struct clk_hw *hw)
> +{
> +       struct davinci_lpsc_clk *lpsc = to_davinci_lpsc_clk(hw);
> +       u32 mdstat;
> +
> +       regmap_read(lpsc->regmap, MDSTAT(lpsc->md), &mdstat);
> +
> +       return (mdstat & MDSTAT_MCKOUT) ? 1 : 0;
> +}
> +
> +static const struct clk_ops davinci_lpsc_clk_ops = {
> +       .enable         = davinci_lpsc_clk_enable,
> +       .disable        = davinci_lpsc_clk_disable,
> +       .is_enabled     = davinci_lpsc_clk_is_enabled,
> +};
> +
> +static int davinci_psc_genpd_attach_dev(struct generic_pm_domain *pm_domain,
> +                                       struct device *dev)
> +{
> +       struct davinci_lpsc_clk *lpsc = to_davinci_lpsc_clk(pm_domain);
> +       struct clk *clk = lpsc->hw.clk;
> +       int ret;
> +
> +       ret = pm_clk_create(dev);
> +       if (ret < 0)
> +               return ret;
> +

Hi David,

I think I found the reason for the strange crashes we were
experiencing (emac core->name being NULL) thanks to Sekhar who pointed
me in the right direction.

The mdio driver fails to probe with v7 due to the supplied clock rate
being wrong. Before failing we register the emac clock with
pm_clk_add_clk(). When clock_ops puts the clock, it decreases the
reference count of the clock, but we never actually increased it in
the first place in the line above. The core clock code then destroys
the associated clk_core structure. When the next user comes around (in
our case the clk debug functions) the system crashes.

I believe there to be two issues: one is with v7 - we need to increase
the clock reference count in davinci_psc_genpd_attach_dev().

Second is the error path in the clock framework - we should remove the
destroyed clk_core from the debug list, which is not being done now.

Why we even need to track the refcount of clk_core is a mistery for me
though. Stephen, Mike?

Best regards,
Bartosz Golaszewski

> +       ret = pm_clk_add_clk(dev, clk);
> +       if (ret < 0) {
> +               pm_clk_destroy(dev);
> +               return ret;
> +       }
> +
> +       return 0;
> +}
> +
> +static void davinci_psc_genpd_detach_dev(struct generic_pm_domain *pm_domain,
> +                                        struct device *dev)
> +{
> +       struct davinci_lpsc_clk *lpsc = to_davinci_lpsc_clk(pm_domain);
> +       struct clk *clk = lpsc->hw.clk;
> +
> +       pm_clk_remove_clk(dev, clk);
> +       pm_clk_destroy(dev);
> +}
> +
> +/**
> + * davinci_lpsc_clk_register - register LPSC clock
> + * @name: name of this clock
> + * @parent_name: name of clock's parent
> + * @regmap: PSC MMIO region
> + * @md: local PSC number
> + * @pd: power domain
> + * @flags: LPSC_* flags
> + */
> +static struct davinci_lpsc_clk *
> +davinci_lpsc_clk_register(struct device *dev, const char *name,
> +                         const char *parent_name, struct regmap *regmap,
> +                         u32 md, u32 pd, u32 flags)
> +{
> +       struct clk_init_data init;
> +       struct davinci_lpsc_clk *lpsc;
> +       int ret;
> +       bool is_on;
> +
> +       lpsc = devm_kzalloc(dev, sizeof(*lpsc), GFP_KERNEL);
> +       if (!lpsc)
> +               return ERR_PTR(-ENOMEM);
> +
> +       init.name = name;
> +       init.ops = &davinci_lpsc_clk_ops;
> +       init.parent_names = (parent_name ? &parent_name : NULL);
> +       init.num_parents = (parent_name ? 1 : 0);
> +       init.flags = 0;
> +
> +       if (flags & LPSC_ALWAYS_ENABLED)
> +               init.flags |= CLK_IS_CRITICAL;
> +
> +       if (flags & LPSC_SET_RATE_PARENT)
> +               init.flags |= CLK_SET_RATE_PARENT;
> +
> +       lpsc->regmap = regmap;
> +       lpsc->hw.init = &init;
> +       lpsc->md = md;
> +       lpsc->pd = pd;
> +       lpsc->flags = flags;
> +
> +       ret = devm_clk_hw_register(dev, &lpsc->hw);
> +       if (ret < 0)
> +               return ERR_PTR(ret);
> +
> +       lpsc->pm_domain.name = devm_kasprintf(dev, GFP_KERNEL, "%s: %s",
> +                                             dev_name(dev), name);
> +       lpsc->pm_domain.attach_dev = davinci_psc_genpd_attach_dev;
> +       lpsc->pm_domain.detach_dev = davinci_psc_genpd_detach_dev;
> +       lpsc->pm_domain.flags = GENPD_FLAG_PM_CLK;
> +
> +       is_on = davinci_lpsc_clk_is_enabled(&lpsc->hw);
> +       pm_genpd_init(&lpsc->pm_domain, NULL, is_on);
> +
> +       return lpsc;
> +}
> +
> +static int davinci_lpsc_clk_reset(struct clk *clk, bool reset)
> +{
> +       struct clk_hw *hw = __clk_get_hw(clk);
> +       struct davinci_lpsc_clk *lpsc = to_davinci_lpsc_clk(hw);
> +       u32 mdctl;
> +
> +       if (IS_ERR_OR_NULL(lpsc))
> +               return -EINVAL;
> +
> +       mdctl = reset ? 0 : MDCTL_LRESET;
> +       regmap_write_bits(lpsc->regmap, MDCTL(lpsc->md), MDCTL_LRESET, mdctl);
> +
> +       return 0;
> +}
> +
> +/*
> + * REVISIT: These exported functions can be removed after a non-DT lookup is
> + * added to the reset controller framework and the davinci-rproc driver is
> + * updated to use the generic reset controller framework.
> + */
> +
> +int davinci_clk_reset_assert(struct clk *clk)
> +{
> +       return davinci_lpsc_clk_reset(clk, true);
> +}
> +EXPORT_SYMBOL(davinci_clk_reset_assert);
> +
> +int davinci_clk_reset_deassert(struct clk *clk)
> +{
> +       return davinci_lpsc_clk_reset(clk, false);
> +}
> +EXPORT_SYMBOL(davinci_clk_reset_deassert);
> +
> +static int davinci_psc_reset_assert(struct reset_controller_dev *rcdev,
> +                                   unsigned long id)
> +{
> +       struct davinci_psc_data *psc = to_davinci_psc_data(rcdev);
> +       struct clk *clk = psc->clk_data.clks[id];
> +
> +       return davinci_lpsc_clk_reset(clk, true);
> +}
> +
> +static int davinci_psc_reset_deassert(struct reset_controller_dev *rcdev,
> +                                     unsigned long id)
> +{
> +       struct davinci_psc_data *psc = to_davinci_psc_data(rcdev);
> +       struct clk *clk = psc->clk_data.clks[id];
> +
> +       return davinci_lpsc_clk_reset(clk, false);
> +}
> +
> +static const struct reset_control_ops davinci_psc_reset_ops = {
> +       .assert         = davinci_psc_reset_assert,
> +       .deassert       = davinci_psc_reset_deassert,
> +};
> +
> +static int davinci_psc_reset_of_xlate(struct reset_controller_dev *rcdev,
> +                                     const struct of_phandle_args *reset_spec)
> +{
> +       struct of_phandle_args clkspec = *reset_spec; /* discard const qualifier */
> +       struct clk *clk;
> +       struct clk_hw *hw;
> +       struct davinci_lpsc_clk *lpsc;
> +
> +       /* the clock node is the same as the reset node */
> +       clk = of_clk_get_from_provider(&clkspec);
> +       if (IS_ERR(clk))
> +               return PTR_ERR(clk);
> +
> +       hw = __clk_get_hw(clk);
> +       lpsc = to_davinci_lpsc_clk(hw);
> +       clk_put(clk);
> +
> +       /* not all modules support local reset */
> +       if (!(lpsc->flags & LPSC_LOCAL_RESET))
> +               return -EINVAL;
> +
> +       return lpsc->md;
> +}
> +
> +static const struct regmap_config davinci_psc_regmap_config = {
> +       .reg_bits       = 32,
> +       .reg_stride     = 4,
> +       .val_bits       = 32,
> +};
> +
> +static struct davinci_psc_data *
> +__davinci_psc_register_clocks(struct device *dev,
> +                             const struct davinci_lpsc_clk_info *info,
> +                             int num_clks,
> +                             void __iomem *base)
> +{
> +       struct davinci_psc_data *psc;
> +       struct clk **clks;
> +       struct generic_pm_domain **pm_domains;
> +       struct regmap *regmap;
> +       int i, ret;
> +
> +       psc = devm_kzalloc(dev, sizeof(*psc), GFP_KERNEL);
> +       if (!psc)
> +               return ERR_PTR(-ENOMEM);
> +
> +       clks = devm_kmalloc_array(dev, num_clks, sizeof(*clks), GFP_KERNEL);
> +       if (!clks)
> +               return ERR_PTR(-ENOMEM);
> +
> +       psc->clk_data.clks = clks;
> +       psc->clk_data.clk_num = num_clks;
> +
> +       /*
> +        * init array with error so that of_clk_src_onecell_get() doesn't
> +        * return NULL for gaps in the sparse array
> +        */
> +       for (i = 0; i < num_clks; i++)
> +               clks[i] = ERR_PTR(-ENOENT);
> +
> +       pm_domains = devm_kcalloc(dev, num_clks, sizeof(*pm_domains), GFP_KERNEL);
> +       if (!pm_domains)
> +               return ERR_PTR(-ENOMEM);
> +
> +       psc->pm_data.domains = pm_domains;
> +       psc->pm_data.num_domains = num_clks;
> +
> +       regmap = devm_regmap_init_mmio(dev, base, &davinci_psc_regmap_config);
> +       if (IS_ERR(regmap))
> +               return ERR_CAST(regmap);
> +
> +       for (; info->name; info++) {
> +               struct davinci_lpsc_clk *lpsc;
> +
> +               lpsc = davinci_lpsc_clk_register(dev, info->name, info->parent,
> +                                                regmap, info->md, info->pd,
> +                                                info->flags);
> +               if (IS_ERR(lpsc)) {
> +                       dev_warn(dev, "Failed to register %s (%ld)\n",
> +                                info->name, PTR_ERR(lpsc));
> +                       continue;
> +               }
> +
> +               clks[info->md] = lpsc->hw.clk;
> +               pm_domains[info->md] = &lpsc->pm_domain;
> +       }
> +
> +       psc->rcdev.ops = &davinci_psc_reset_ops;
> +       psc->rcdev.owner = THIS_MODULE;
> +       psc->rcdev.of_node = dev->of_node;
> +       psc->rcdev.of_reset_n_cells = 1;
> +       psc->rcdev.of_xlate = davinci_psc_reset_of_xlate;
> +       psc->rcdev.nr_resets = num_clks;
> +
> +       ret = devm_reset_controller_register(dev, &psc->rcdev);
> +       if (ret < 0)
> +               dev_warn(dev, "Failed to register reset controller (%d)\n", ret);
> +
> +       return psc;
> +}
> +
> +int davinci_psc_register_clocks(struct device *dev,
> +                               const struct davinci_lpsc_clk_info *info,
> +                               u8 num_clks,
> +                               void __iomem *base)
> +{
> +       struct davinci_psc_data *psc;
> +
> +       psc = __davinci_psc_register_clocks(dev, info, num_clks, base);
> +       if (IS_ERR(psc))
> +               return PTR_ERR(psc);
> +
> +       for (; info->name; info++) {
> +               const struct davinci_lpsc_clkdev_info *cdevs = info->cdevs;
> +               struct clk *clk = psc->clk_data.clks[info->md];
> +
> +               if (!cdevs || IS_ERR_OR_NULL(clk))
> +                       continue;
> +
> +               for (; cdevs->con_id || cdevs->dev_id; cdevs++)
> +                       clk_register_clkdev(clk, cdevs->con_id, cdevs->dev_id);
> +       }
> +
> +       return 0;
> +}
> +
> +int of_davinci_psc_clk_init(struct device *dev,
> +                           const struct davinci_lpsc_clk_info *info,
> +                           u8 num_clks,
> +                           void __iomem *base)
> +{
> +       struct device_node *node = dev->of_node;
> +       struct davinci_psc_data *psc;
> +
> +       psc = __davinci_psc_register_clocks(dev, info, num_clks, base);
> +       if (IS_ERR(psc))
> +               return PTR_ERR(psc);
> +
> +       of_genpd_add_provider_onecell(node, &psc->pm_data);
> +
> +       of_clk_add_provider(node, of_clk_src_onecell_get, &psc->clk_data);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id davinci_psc_of_match[] = {
> +       { }
> +};
> +
> +static const struct platform_device_id davinci_psc_id_table[] = {
> +       { }
> +};
> +
> +typedef int (*davinci_psc_init)(struct device *dev, void __iomem *base);
> +
> +static int davinci_psc_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       const struct of_device_id *of_id;
> +       davinci_psc_init psc_init = NULL;
> +       struct resource *res;
> +       void __iomem *base;
> +
> +       of_id = of_match_device(davinci_psc_of_match, dev);
> +       if (of_id)
> +               psc_init = of_id->data;
> +       else if (pdev->id_entry)
> +               psc_init = (void *)pdev->id_entry->driver_data;
> +
> +       if (!psc_init) {
> +               dev_err(dev, "unable to find driver data\n");
> +               return -EINVAL;
> +       }
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       base = devm_ioremap_resource(dev, res);
> +       if (IS_ERR(base)) {
> +               dev_err(dev, "ioremap failed\n");
> +               return PTR_ERR(base);
> +       }
> +
> +       return psc_init(dev, base);
> +}
> +
> +static struct platform_driver davinci_psc_driver = {
> +       .probe          = davinci_psc_probe,
> +       .driver         = {
> +               .name           = "davinci-psc-clk",
> +               .of_match_table = davinci_psc_of_match,
> +       },
> +       .id_table       = davinci_psc_id_table,
> +};
> +
> +static int __init davinci_psc_driver_init(void)
> +{
> +       return platform_driver_register(&davinci_psc_driver);
> +}
> +
> +/* has to be postcore_initcall because davinci_gpio depend on PSC clocks */
> +postcore_initcall(davinci_psc_driver_init);
> diff --git a/drivers/clk/davinci/psc.h b/drivers/clk/davinci/psc.h
> new file mode 100644
> index 0000000..c342d54
> --- /dev/null
> +++ b/drivers/clk/davinci/psc.h
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Clock driver for TI Davinci PSC controllers
> + *
> + * Copyright (C) 2018 David Lechner <david@lechnology.com>
> + */
> +
> +#ifndef __CLK_DAVINCI_PSC_H__
> +#define __CLK_DAVINCI_PSC_H__
> +
> +#include <linux/clk-provider.h>
> +#include <linux/types.h>
> +
> +/* PSC quirk flags */
> +#define LPSC_ALWAYS_ENABLED    BIT(0) /* never disable this clock */
> +#define LPSC_SET_RATE_PARENT   BIT(1) /* propagate set_rate to parent clock */
> +#define LPSC_FORCE             BIT(2) /* requires MDCTL FORCE bit */
> +#define LPSC_LOCAL_RESET       BIT(3) /* acts as reset provider */
> +
> +struct davinci_lpsc_clkdev_info {
> +       const char *con_id;
> +       const char *dev_id;
> +};
> +
> +#define LPSC_CLKDEV(c, d) {    \
> +       .con_id = (c),          \
> +       .dev_id = (d)           \
> +}
> +
> +#define LPSC_CLKDEV1(n, c, d) \
> +static const struct davinci_lpsc_clkdev_info n[] __initconst = {       \
> +       LPSC_CLKDEV((c), (d)),                                          \
> +       { }                                                             \
> +}
> +
> +#define LPSC_CLKDEV2(n, c1, d1, c2, d2) \
> +static const struct davinci_lpsc_clkdev_info n[] __initconst = {       \
> +       LPSC_CLKDEV((c1), (d1)),                                        \
> +       LPSC_CLKDEV((c2), (d2)),                                        \
> +       { }                                                             \
> +}
> +
> +#define LPSC_CLKDEV3(n, c1, d1, c2, d2, c3, d3) \
> +static const struct davinci_lpsc_clkdev_info n[] __initconst = {       \
> +       LPSC_CLKDEV((c1), (d1)),                                        \
> +       LPSC_CLKDEV((c2), (d2)),                                        \
> +       LPSC_CLKDEV((c3), (d3)),                                        \
> +       { }                                                             \
> +}
> +
> +/**
> + * davinci_lpsc_clk_info - LPSC module-specific clock information
> + * @name: the clock name
> + * @parent: the parent clock name
> + * @cdevs: optional array of clkdev lookup table info
> + * @md: the local module domain (LPSC id)
> + * @pd: the power domain id
> + * @flags: bitmask of LPSC_* flags
> + */
> +struct davinci_lpsc_clk_info {
> +       const char *name;
> +       const char *parent;
> +       const struct davinci_lpsc_clkdev_info *cdevs;
> +       u32 md;
> +       u32 pd;
> +       unsigned long flags;
> +};
> +
> +#define LPSC(m, d, n, p, c, f) \
> +{                              \
> +       .name   = #n,           \
> +       .parent = #p,           \
> +       .cdevs  = (c),          \
> +       .md     = (m),          \
> +       .pd     = (d),          \
> +       .flags  = (f),          \
> +}
> +
> +int davinci_psc_register_clocks(struct device *dev,
> +                               const struct davinci_lpsc_clk_info *info,
> +                               u8 num_clks,
> +                               void __iomem *base);
> +
> +int of_davinci_psc_clk_init(struct device *dev,
> +                           const struct davinci_lpsc_clk_info *info,
> +                           u8 num_clks,
> +                           void __iomem *base);
> +
> +#endif /* __CLK_DAVINCI_PSC_H__ */
> --
> 2.7.4
>
David Lechner Feb. 28, 2018, 9:40 p.m. UTC | #2
On 02/28/2018 06:38 AM, Bartosz Golaszewski wrote:
> 
> I think I found the reason for the strange crashes we were
> experiencing (emac core->name being NULL) thanks to Sekhar who pointed
> me in the right direction.
> 
> The mdio driver fails to probe with v7 due to the supplied clock rate
> being wrong. Before failing we register the emac clock with
> pm_clk_add_clk(). When clock_ops puts the clock, it decreases the
> reference count of the clock, but we never actually increased it in
> the first place in the line above. The core clock code then destroys
> the associated clk_core structure. When the next user comes around (in
> our case the clk debug functions) the system crashes.
> 
> I believe there to be two issues: one is with v7 - we need to increase
> the clock reference count in davinci_psc_genpd_attach_dev().
> 
> Second is the error path in the clock framework - we should remove the
> destroyed clk_core from the debug list, which is not being done now.
> 
> Why we even need to track the refcount of clk_core is a mistery for me
> though. Stephen, Mike?
> 
> Best regards,
> Bartosz Golaszewski

Great find. I figured it had to be something like this, but I wasn't
able to reproduce the problem yet.

I suppose it is time to spin up a v8 with some fixes.
Bartosz Golaszewski March 1, 2018, 8:36 a.m. UTC | #3
2018-02-28 22:40 GMT+01:00 David Lechner <david@lechnology.com>:
> On 02/28/2018 06:38 AM, Bartosz Golaszewski wrote:
>>
>>
>> I think I found the reason for the strange crashes we were
>> experiencing (emac core->name being NULL) thanks to Sekhar who pointed
>> me in the right direction.
>>
>> The mdio driver fails to probe with v7 due to the supplied clock rate
>> being wrong. Before failing we register the emac clock with
>> pm_clk_add_clk(). When clock_ops puts the clock, it decreases the
>> reference count of the clock, but we never actually increased it in
>> the first place in the line above. The core clock code then destroys
>> the associated clk_core structure. When the next user comes around (in
>> our case the clk debug functions) the system crashes.
>>
>> I believe there to be two issues: one is with v7 - we need to increase
>> the clock reference count in davinci_psc_genpd_attach_dev().
>>
>> Second is the error path in the clock framework - we should remove the
>> destroyed clk_core from the debug list, which is not being done now.
>>
>> Why we even need to track the refcount of clk_core is a mistery for me
>> though. Stephen, Mike?
>>
>> Best regards,
>> Bartosz Golaszewski
>
>
> Great find. I figured it had to be something like this, but I wasn't
> able to reproduce the problem yet.
>
> I suppose it is time to spin up a v8 with some fixes.

I still don't know why the mdio clock rate is much lower than in
mainline though. Any ideas?

Thanks,
Bart
David Lechner March 1, 2018, 4:44 p.m. UTC | #4
On 03/01/2018 02:36 AM, Bartosz Golaszewski wrote:
> 2018-02-28 22:40 GMT+01:00 David Lechner <david@lechnology.com>:
>> On 02/28/2018 06:38 AM, Bartosz Golaszewski wrote:
>>>
>>>
>>> I think I found the reason for the strange crashes we were
>>> experiencing (emac core->name being NULL) thanks to Sekhar who pointed
>>> me in the right direction.
>>>
>>> The mdio driver fails to probe with v7 due to the supplied clock rate
>>> being wrong. Before failing we register the emac clock with
>>> pm_clk_add_clk(). When clock_ops puts the clock, it decreases the
>>> reference count of the clock, but we never actually increased it in
>>> the first place in the line above. The core clock code then destroys
>>> the associated clk_core structure. When the next user comes around (in
>>> our case the clk debug functions) the system crashes.
>>>
>>> I believe there to be two issues: one is with v7 - we need to increase
>>> the clock reference count in davinci_psc_genpd_attach_dev().
>>>
>>> Second is the error path in the clock framework - we should remove the
>>> destroyed clk_core from the debug list, which is not being done now.
>>>
>>> Why we even need to track the refcount of clk_core is a mistery for me
>>> though. Stephen, Mike?
>>>
>>> Best regards,
>>> Bartosz Golaszewski
>>
>>
>> Great find. I figured it had to be something like this, but I wasn't
>> able to reproduce the problem yet.
>>
>> I suppose it is time to spin up a v8 with some fixes.
> 
> I still don't know why the mdio clock rate is much lower than in
> mainline though. Any ideas?
> 
> Thanks,
> Bart
> 

Now that you have fixed the crash, can you answer the questions I have
asked earlier?

> Can you post the output of this command so that I can see how your
clocks are setup:

cat /sys/kernel/debug/clk/clk_summary

> Using your workaround, can you run:

cat /sys/kernel/debug/pm_genpd/pm_genpd_summary

If you see:
   1e27000.clock-controller: emac  off-0

then genpd is not working like it is supposed to. You should see something
like this for device that are working:
           1e27000.clock-controller: uart2  on
     /devices/platform/soc@1c00000/1d0d000.serial        active
Bartosz Golaszewski March 2, 2018, 5:39 p.m. UTC | #5
2018-03-01 17:44 GMT+01:00 David Lechner <david@lechnology.com>:
> On 03/01/2018 02:36 AM, Bartosz Golaszewski wrote:
>>
>> 2018-02-28 22:40 GMT+01:00 David Lechner <david@lechnology.com>:
>>>
>>> On 02/28/2018 06:38 AM, Bartosz Golaszewski wrote:
>>>>
>>>>
>>>>
>>>> I think I found the reason for the strange crashes we were
>>>> experiencing (emac core->name being NULL) thanks to Sekhar who pointed
>>>> me in the right direction.
>>>>
>>>> The mdio driver fails to probe with v7 due to the supplied clock rate
>>>> being wrong. Before failing we register the emac clock with
>>>> pm_clk_add_clk(). When clock_ops puts the clock, it decreases the
>>>> reference count of the clock, but we never actually increased it in
>>>> the first place in the line above. The core clock code then destroys
>>>> the associated clk_core structure. When the next user comes around (in
>>>> our case the clk debug functions) the system crashes.
>>>>
>>>> I believe there to be two issues: one is with v7 - we need to increase
>>>> the clock reference count in davinci_psc_genpd_attach_dev().
>>>>
>>>> Second is the error path in the clock framework - we should remove the
>>>> destroyed clk_core from the debug list, which is not being done now.
>>>>
>>>> Why we even need to track the refcount of clk_core is a mistery for me
>>>> though. Stephen, Mike?
>>>>
>>>> Best regards,
>>>> Bartosz Golaszewski
>>>
>>>
>>>
>>> Great find. I figured it had to be something like this, but I wasn't
>>> able to reproduce the problem yet.
>>>
>>> I suppose it is time to spin up a v8 with some fixes.
>>
>>
>> I still don't know why the mdio clock rate is much lower than in
>> mainline though. Any ideas?
>>
>> Thanks,
>> Bart
>>
>
> Now that you have fixed the crash, can you answer the questions I have
> asked earlier?
>
>> Can you post the output of this command so that I can see how your
>
> clocks are setup:
>
> cat /sys/kernel/debug/clk/clk_summary
>
>> Using your workaround, can you run:
>
>
> cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
>
> If you see:
>   1e27000.clock-controller: emac  off-0
>
> then genpd is not working like it is supposed to. You should see something
> like this for device that are working:
>           1e27000.clock-controller: uart2  on
>     /devices/platform/soc@1c00000/1d0d000.serial        active

I used of_clk_get() in the genpd attach callback so the crash no
longer happens, but I still can't boot it over NFS due to mdio
failing. Do you have any idea why the clock rate differs between v7
and mainline?

From the logs I can see that genpd domains are correctly registered,
and the provider is added (you should probably skip setting up the
domains in legacy mode though), the pm clocks are enabled (after being
disabled by mdio after its failed probe()) but the boot process gets
stuck after the kernel gets an IP address over DHCP (which is strange
because apparently it had some kind of network connection).

On Monday I'll prepare a small ramfs and boot over tftp only and see from there.

Best regards,
Bartosz
Bartosz Golaszewski March 5, 2018, 1:02 p.m. UTC | #6
2018-03-01 17:44 GMT+01:00 David Lechner <david@lechnology.com>:
> On 03/01/2018 02:36 AM, Bartosz Golaszewski wrote:
>>
>> 2018-02-28 22:40 GMT+01:00 David Lechner <david@lechnology.com>:
>>>
>>> On 02/28/2018 06:38 AM, Bartosz Golaszewski wrote:
>>>>
>>>>
>>>>
>>>> I think I found the reason for the strange crashes we were
>>>> experiencing (emac core->name being NULL) thanks to Sekhar who pointed
>>>> me in the right direction.
>>>>
>>>> The mdio driver fails to probe with v7 due to the supplied clock rate
>>>> being wrong. Before failing we register the emac clock with
>>>> pm_clk_add_clk(). When clock_ops puts the clock, it decreases the
>>>> reference count of the clock, but we never actually increased it in
>>>> the first place in the line above. The core clock code then destroys
>>>> the associated clk_core structure. When the next user comes around (in
>>>> our case the clk debug functions) the system crashes.
>>>>
>>>> I believe there to be two issues: one is with v7 - we need to increase
>>>> the clock reference count in davinci_psc_genpd_attach_dev().
>>>>
>>>> Second is the error path in the clock framework - we should remove the
>>>> destroyed clk_core from the debug list, which is not being done now.
>>>>
>>>> Why we even need to track the refcount of clk_core is a mistery for me
>>>> though. Stephen, Mike?
>>>>
>>>> Best regards,
>>>> Bartosz Golaszewski
>>>
>>>
>>>
>>> Great find. I figured it had to be something like this, but I wasn't
>>> able to reproduce the problem yet.
>>>
>>> I suppose it is time to spin up a v8 with some fixes.
>>
>>
>> I still don't know why the mdio clock rate is much lower than in
>> mainline though. Any ideas?
>>
>> Thanks,
>> Bart
>>
>
> Now that you have fixed the crash, can you answer the questions I have
> asked earlier?
>
>> Can you post the output of this command so that I can see how your
>
> clocks are setup:
>
> cat /sys/kernel/debug/clk/clk_summary
>
>> Using your workaround, can you run:
>
>
> cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
>
> If you see:
>   1e27000.clock-controller: emac  off-0
>
> then genpd is not working like it is supposed to. You should see something
> like this for device that are working:
>           1e27000.clock-controller: uart2  on
>     /devices/platform/soc@1c00000/1d0d000.serial        active

Hi David, Sekhar,

I tried booting the board today over tftp but didn't succeed. I then
switched to a normal boot from SD card and the boot process froze at
the same moment (right after the DHCP config, or after rtc config if I
disabled DHCP in bootargs). I then realized that the emac clock can't
be the culprit. After some digging I found out that the late_initcall
to clk_disable_unused() disables sysclk6 - the parent of the arm
clock, which of course freezes the device.

If I remove the call to clk_disable_unused(), I can boot just fine.

The following other clocks are disabled before pll0_sysclk6:
pll1_sysclk3
pll0_obsclk
pll0_sysclk7

davinci_lpsc_clk_enable() is never called for these clocks - in fact
it's not called for any parent that's not explicitly defined in
psc-da850.c - I believe this may be one of the reasons. I will get
back to debugging it tomorrow.

Best regards,
Bartosz Golaszewski
David Lechner March 5, 2018, 4:23 p.m. UTC | #7
On 03/05/2018 07:02 AM, Bartosz Golaszewski wrote:
> 2018-03-01 17:44 GMT+01:00 David Lechner <david@lechnology.com>:
>> On 03/01/2018 02:36 AM, Bartosz Golaszewski wrote:
>>>
>>> 2018-02-28 22:40 GMT+01:00 David Lechner <david@lechnology.com>:
>>>>
>>>> On 02/28/2018 06:38 AM, Bartosz Golaszewski wrote:
>>>>>
>>>>>
>>>>>
>>>>> I think I found the reason for the strange crashes we were
>>>>> experiencing (emac core->name being NULL) thanks to Sekhar who pointed
>>>>> me in the right direction.
>>>>>
>>>>> The mdio driver fails to probe with v7 due to the supplied clock rate
>>>>> being wrong. Before failing we register the emac clock with
>>>>> pm_clk_add_clk(). When clock_ops puts the clock, it decreases the
>>>>> reference count of the clock, but we never actually increased it in
>>>>> the first place in the line above. The core clock code then destroys
>>>>> the associated clk_core structure. When the next user comes around (in
>>>>> our case the clk debug functions) the system crashes.
>>>>>
>>>>> I believe there to be two issues: one is with v7 - we need to increase
>>>>> the clock reference count in davinci_psc_genpd_attach_dev().
>>>>>
>>>>> Second is the error path in the clock framework - we should remove the
>>>>> destroyed clk_core from the debug list, which is not being done now.
>>>>>
>>>>> Why we even need to track the refcount of clk_core is a mistery for me
>>>>> though. Stephen, Mike?
>>>>>
>>>>> Best regards,
>>>>> Bartosz Golaszewski
>>>>
>>>>
>>>>
>>>> Great find. I figured it had to be something like this, but I wasn't
>>>> able to reproduce the problem yet.
>>>>
>>>> I suppose it is time to spin up a v8 with some fixes.
>>>
>>>
>>> I still don't know why the mdio clock rate is much lower than in
>>> mainline though. Any ideas?
>>>
>>> Thanks,
>>> Bart
>>>
>>
>> Now that you have fixed the crash, can you answer the questions I have
>> asked earlier?
>>
>>> Can you post the output of this command so that I can see how your
>>
>> clocks are setup:
>>
>> cat /sys/kernel/debug/clk/clk_summary
>>
>>> Using your workaround, can you run:
>>
>>
>> cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
>>
>> If you see:
>>    1e27000.clock-controller: emac  off-0
>>
>> then genpd is not working like it is supposed to. You should see something
>> like this for device that are working:
>>            1e27000.clock-controller: uart2  on
>>      /devices/platform/soc@1c00000/1d0d000.serial        active
> 
> Hi David, Sekhar,
> 
> I tried booting the board today over tftp but didn't succeed. I then
> switched to a normal boot from SD card and the boot process froze at
> the same moment (right after the DHCP config, or after rtc config if I
> disabled DHCP in bootargs). I then realized that the emac clock can't
> be the culprit. After some digging I found out that the late_initcall
> to clk_disable_unused() disables sysclk6 - the parent of the arm
> clock, which of course freezes the device.
> 
> If I remove the call to clk_disable_unused(), I can boot just fine.
> 
> The following other clocks are disabled before pll0_sysclk6:
> pll1_sysclk3
> pll0_obsclk
> pll0_sysclk7
> 
> davinci_lpsc_clk_enable() is never called for these clocks - in fact
> it's not called for any parent that's not explicitly defined in
> psc-da850.c - I believe this may be one of the reasons. I will get
> back to debugging it tomorrow.
> 
> Best regards,
> Bartosz Golaszewski
> 

Thanks for continuing to dig into this. I think I know what needs to
be done now. I think I don't have the dependencies quite right where
the PSC clocks are being registered before the PLL clocks, in which
case they aren't getting the correct parent clock.
David Lechner March 5, 2018, 5:46 p.m. UTC | #8
On 03/05/2018 10:23 AM, David Lechner wrote:
> On 03/05/2018 07:02 AM, Bartosz Golaszewski wrote:
>> 2018-03-01 17:44 GMT+01:00 David Lechner <david@lechnology.com>:
>>> On 03/01/2018 02:36 AM, Bartosz Golaszewski wrote:
>>>>
>>>> 2018-02-28 22:40 GMT+01:00 David Lechner <david@lechnology.com>:
>>>>>
>>>>> On 02/28/2018 06:38 AM, Bartosz Golaszewski wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> I think I found the reason for the strange crashes we were
>>>>>> experiencing (emac core->name being NULL) thanks to Sekhar who pointed
>>>>>> me in the right direction.
>>>>>>
>>>>>> The mdio driver fails to probe with v7 due to the supplied clock rate
>>>>>> being wrong. Before failing we register the emac clock with
>>>>>> pm_clk_add_clk(). When clock_ops puts the clock, it decreases the
>>>>>> reference count of the clock, but we never actually increased it in
>>>>>> the first place in the line above. The core clock code then destroys
>>>>>> the associated clk_core structure. When the next user comes around (in
>>>>>> our case the clk debug functions) the system crashes.
>>>>>>
>>>>>> I believe there to be two issues: one is with v7 - we need to increase
>>>>>> the clock reference count in davinci_psc_genpd_attach_dev().
>>>>>>
>>>>>> Second is the error path in the clock framework - we should remove the
>>>>>> destroyed clk_core from the debug list, which is not being done now.
>>>>>>
>>>>>> Why we even need to track the refcount of clk_core is a mistery for me
>>>>>> though. Stephen, Mike?
>>>>>>
>>>>>> Best regards,
>>>>>> Bartosz Golaszewski
>>>>>
>>>>>
>>>>>
>>>>> Great find. I figured it had to be something like this, but I wasn't
>>>>> able to reproduce the problem yet.
>>>>>
>>>>> I suppose it is time to spin up a v8 with some fixes.
>>>>
>>>>
>>>> I still don't know why the mdio clock rate is much lower than in
>>>> mainline though. Any ideas?
>>>>
>>>> Thanks,
>>>> Bart
>>>>
>>>
>>> Now that you have fixed the crash, can you answer the questions I have
>>> asked earlier?
>>>
>>>> Can you post the output of this command so that I can see how your
>>>
>>> clocks are setup:
>>>
>>> cat /sys/kernel/debug/clk/clk_summary
>>>
>>>> Using your workaround, can you run:
>>>
>>>
>>> cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
>>>
>>> If you see:
>>>    1e27000.clock-controller: emac  off-0
>>>
>>> then genpd is not working like it is supposed to. You should see something
>>> like this for device that are working:
>>>            1e27000.clock-controller: uart2  on
>>>      /devices/platform/soc@1c00000/1d0d000.serial        active
>>
>> Hi David, Sekhar,
>>
>> I tried booting the board today over tftp but didn't succeed. I then
>> switched to a normal boot from SD card and the boot process froze at
>> the same moment (right after the DHCP config, or after rtc config if I
>> disabled DHCP in bootargs). I then realized that the emac clock can't
>> be the culprit. After some digging I found out that the late_initcall
>> to clk_disable_unused() disables sysclk6 - the parent of the arm
>> clock, which of course freezes the device.
>>
>> If I remove the call to clk_disable_unused(), I can boot just fine.
>>
>> The following other clocks are disabled before pll0_sysclk6:
>> pll1_sysclk3
>> pll0_obsclk
>> pll0_sysclk7
>>
>> davinci_lpsc_clk_enable() is never called for these clocks - in fact
>> it's not called for any parent that's not explicitly defined in
>> psc-da850.c - I believe this may be one of the reasons. I will get
>> back to debugging it tomorrow.
>>
>> Best regards,
>> Bartosz Golaszewski
>>
> 
> Thanks for continuing to dig into this. I think I know what needs to
> be done now. I think I don't have the dependencies quite right where
> the PSC clocks are being registered before the PLL clocks, in which
> case they aren't getting the correct parent clock.
> 

Bartosz,

One more thing to check: I think I had some typos in da850.dtsi where
I wrote clock_names instead of clock-names. Please make sure this is
fixed in your working branch.
Stephen Boyd March 16, 2018, 5:55 p.m. UTC | #9
Quoting Bartosz Golaszewski (2018-02-28 04:38:38)
> 2018-02-19 21:21 GMT+01:00 David Lechner <david@lechnology.com>:
> 
> I believe there to be two issues: one is with v7 - we need to increase
> the clock reference count in davinci_psc_genpd_attach_dev().
> 
> Second is the error path in the clock framework - we should remove the
> destroyed clk_core from the debug list, which is not being done now.
> 
> Why we even need to track the refcount of clk_core is a mistery for me
> though. Stephen, Mike?
> 

Which part of the code are we talking about? I see that
__clk_core_init() calls clk_debug_register() when ret == 0 and that
looks fine. I do wonder why clk_debug_register() even returns a value
though because we ignore it.
Bartosz Golaszewski March 19, 2018, 10:26 a.m. UTC | #10
2018-03-16 18:55 GMT+01:00 Stephen Boyd <sboyd@kernel.org>:
> Quoting Bartosz Golaszewski (2018-02-28 04:38:38)
>> 2018-02-19 21:21 GMT+01:00 David Lechner <david@lechnology.com>:
>>
>> I believe there to be two issues: one is with v7 - we need to increase
>> the clock reference count in davinci_psc_genpd_attach_dev().
>>
>> Second is the error path in the clock framework - we should remove the
>> destroyed clk_core from the debug list, which is not being done now.
>>
>> Why we even need to track the refcount of clk_core is a mistery for me
>> though. Stephen, Mike?
>>
>
> Which part of the code are we talking about? I see that
> __clk_core_init() calls clk_debug_register() when ret == 0 and that
> looks fine. I do wonder why clk_debug_register() even returns a value
> though because we ignore it.

Hi Stephen,

invalid usage of clock pm ops was the culprit here. I just had not
understood why we count references in the CCF - now I get it so
nevermind this mail.

Thanks,
Bart
diff mbox

Patch

diff --git a/drivers/clk/davinci/Makefile b/drivers/clk/davinci/Makefile
index d471386..cd1bf2c 100644
--- a/drivers/clk/davinci/Makefile
+++ b/drivers/clk/davinci/Makefile
@@ -8,4 +8,6 @@  obj-$(CONFIG_ARCH_DAVINCI_DM355)	+= pll-dm355.o
 obj-$(CONFIG_ARCH_DAVINCI_DM365)	+= pll-dm365.o
 obj-$(CONFIG_ARCH_DAVINCI_DM644x)	+= pll-dm644x.o
 obj-$(CONFIG_ARCH_DAVINCI_DM646x)	+= pll-dm646x.o
+
+obj-y += psc.o
 endif
diff --git a/drivers/clk/davinci/psc.c b/drivers/clk/davinci/psc.c
new file mode 100644
index 0000000..8e08ac8
--- /dev/null
+++ b/drivers/clk/davinci/psc.c
@@ -0,0 +1,495 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Clock driver for TI Davinci PSC controllers
+ *
+ * Copyright (C) 2017 David Lechner <david@lechnology.com>
+ *
+ * Based on: drivers/clk/keystone/gate.c
+ * Copyright (C) 2013 Texas Instruments.
+ *	Murali Karicheri <m-karicheri2@ti.com>
+ *	Santosh Shilimkar <santosh.shilimkar@ti.com>
+ *
+ * And: arch/arm/mach-davinci/psc.c
+ * Copyright (C) 2006 Texas Instruments.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clk.h>
+#include <linux/clkdev.h>
+#include <linux/err.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_domain.h>
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include "psc.h"
+
+/* PSC register offsets */
+#define EPCPR			0x070
+#define PTCMD			0x120
+#define PTSTAT			0x128
+#define PDSTAT(n)		(0x200 + 4 * (n))
+#define PDCTL(n)		(0x300 + 4 * (n))
+#define MDSTAT(n)		(0x800 + 4 * (n))
+#define MDCTL(n)		(0xa00 + 4 * (n))
+
+/* PSC module states */
+enum davinci_lpsc_state {
+	LPSC_STATE_SWRSTDISABLE	= 0,
+	LPSC_STATE_SYNCRST	= 1,
+	LPSC_STATE_DISABLE	= 2,
+	LPSC_STATE_ENABLE	= 3,
+};
+
+#define MDSTAT_STATE_MASK	GENMASK(5, 0)
+#define MDSTAT_MCKOUT		BIT(12)
+#define PDSTAT_STATE_MASK	GENMASK(4, 0)
+#define MDCTL_FORCE		BIT(31)
+#define MDCTL_LRESET		BIT(8)
+#define PDCTL_EPCGOOD		BIT(8)
+#define PDCTL_NEXT		BIT(0)
+
+struct davinci_psc_data {
+	struct clk_onecell_data clk_data;
+	struct genpd_onecell_data pm_data;
+	struct reset_controller_dev rcdev;
+};
+
+/**
+ * struct davinci_lpsc_clk - LPSC clock structure
+ * @hw: clk_hw for the LPSC
+ * @pm_domain: power domain for the LPSC
+ * @regmap: PSC MMIO region
+ * @md: Module domain (LPSC module id)
+ * @pd: Power domain
+ * @flags: LPSC_* quirk flags
+ */
+struct davinci_lpsc_clk {
+	struct clk_hw hw;
+	struct generic_pm_domain pm_domain;
+	struct regmap *regmap;
+	u32 md;
+	u32 pd;
+	u32 flags;
+};
+
+#define to_davinci_psc_data(x) container_of(x, struct davinci_psc_data, x)
+#define to_davinci_lpsc_clk(x) container_of(x, struct davinci_lpsc_clk, x)
+
+static void davinci_lpsc_config(struct davinci_lpsc_clk *lpsc,
+				enum davinci_lpsc_state next_state)
+{
+	u32 epcpr, pdstat, mdstat, ptstat;
+
+	regmap_write_bits(lpsc->regmap, MDCTL(lpsc->md), MDSTAT_STATE_MASK,
+			  next_state);
+
+	if (lpsc->flags & LPSC_FORCE)
+		regmap_write_bits(lpsc->regmap, MDCTL(lpsc->md), MDCTL_FORCE,
+				  MDCTL_FORCE);
+
+	regmap_read(lpsc->regmap, PDSTAT(lpsc->pd), &pdstat);
+	if ((pdstat & PDSTAT_STATE_MASK) == 0) {
+		regmap_write_bits(lpsc->regmap, PDCTL(lpsc->pd), PDCTL_NEXT,
+				  PDCTL_NEXT);
+
+		regmap_write(lpsc->regmap, PTCMD, BIT(lpsc->pd));
+
+		regmap_read_poll_timeout(lpsc->regmap, EPCPR, epcpr,
+					 epcpr & BIT(lpsc->pd), 0, 0);
+
+		regmap_write_bits(lpsc->regmap, PDCTL(lpsc->pd), PDCTL_EPCGOOD,
+				  PDCTL_EPCGOOD);
+	} else {
+		regmap_write(lpsc->regmap, PTCMD, BIT(lpsc->pd));
+	}
+
+	regmap_read_poll_timeout(lpsc->regmap, PTSTAT, ptstat,
+				 !(ptstat & BIT(lpsc->pd)), 0, 0);
+
+	regmap_read_poll_timeout(lpsc->regmap, MDSTAT(lpsc->md), mdstat,
+				 (mdstat & MDSTAT_STATE_MASK) == next_state,
+				 0, 0);
+}
+
+static int davinci_lpsc_clk_enable(struct clk_hw *hw)
+{
+	struct davinci_lpsc_clk *lpsc = to_davinci_lpsc_clk(hw);
+
+	davinci_lpsc_config(lpsc, LPSC_STATE_ENABLE);
+
+	return 0;
+}
+
+static void davinci_lpsc_clk_disable(struct clk_hw *hw)
+{
+	struct davinci_lpsc_clk *lpsc = to_davinci_lpsc_clk(hw);
+
+	davinci_lpsc_config(lpsc, LPSC_STATE_DISABLE);
+}
+
+static int davinci_lpsc_clk_is_enabled(struct clk_hw *hw)
+{
+	struct davinci_lpsc_clk *lpsc = to_davinci_lpsc_clk(hw);
+	u32 mdstat;
+
+	regmap_read(lpsc->regmap, MDSTAT(lpsc->md), &mdstat);
+
+	return (mdstat & MDSTAT_MCKOUT) ? 1 : 0;
+}
+
+static const struct clk_ops davinci_lpsc_clk_ops = {
+	.enable		= davinci_lpsc_clk_enable,
+	.disable	= davinci_lpsc_clk_disable,
+	.is_enabled	= davinci_lpsc_clk_is_enabled,
+};
+
+static int davinci_psc_genpd_attach_dev(struct generic_pm_domain *pm_domain,
+					struct device *dev)
+{
+	struct davinci_lpsc_clk *lpsc = to_davinci_lpsc_clk(pm_domain);
+	struct clk *clk = lpsc->hw.clk;
+	int ret;
+
+	ret = pm_clk_create(dev);
+	if (ret < 0)
+		return ret;
+
+	ret = pm_clk_add_clk(dev, clk);
+	if (ret < 0) {
+		pm_clk_destroy(dev);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void davinci_psc_genpd_detach_dev(struct generic_pm_domain *pm_domain,
+					 struct device *dev)
+{
+	struct davinci_lpsc_clk *lpsc = to_davinci_lpsc_clk(pm_domain);
+	struct clk *clk = lpsc->hw.clk;
+
+	pm_clk_remove_clk(dev, clk);
+	pm_clk_destroy(dev);
+}
+
+/**
+ * davinci_lpsc_clk_register - register LPSC clock
+ * @name: name of this clock
+ * @parent_name: name of clock's parent
+ * @regmap: PSC MMIO region
+ * @md: local PSC number
+ * @pd: power domain
+ * @flags: LPSC_* flags
+ */
+static struct davinci_lpsc_clk *
+davinci_lpsc_clk_register(struct device *dev, const char *name,
+			  const char *parent_name, struct regmap *regmap,
+			  u32 md, u32 pd, u32 flags)
+{
+	struct clk_init_data init;
+	struct davinci_lpsc_clk *lpsc;
+	int ret;
+	bool is_on;
+
+	lpsc = devm_kzalloc(dev, sizeof(*lpsc), GFP_KERNEL);
+	if (!lpsc)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &davinci_lpsc_clk_ops;
+	init.parent_names = (parent_name ? &parent_name : NULL);
+	init.num_parents = (parent_name ? 1 : 0);
+	init.flags = 0;
+
+	if (flags & LPSC_ALWAYS_ENABLED)
+		init.flags |= CLK_IS_CRITICAL;
+
+	if (flags & LPSC_SET_RATE_PARENT)
+		init.flags |= CLK_SET_RATE_PARENT;
+
+	lpsc->regmap = regmap;
+	lpsc->hw.init = &init;
+	lpsc->md = md;
+	lpsc->pd = pd;
+	lpsc->flags = flags;
+
+	ret = devm_clk_hw_register(dev, &lpsc->hw);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	lpsc->pm_domain.name = devm_kasprintf(dev, GFP_KERNEL, "%s: %s",
+					      dev_name(dev), name);
+	lpsc->pm_domain.attach_dev = davinci_psc_genpd_attach_dev;
+	lpsc->pm_domain.detach_dev = davinci_psc_genpd_detach_dev;
+	lpsc->pm_domain.flags = GENPD_FLAG_PM_CLK;
+
+	is_on = davinci_lpsc_clk_is_enabled(&lpsc->hw);
+	pm_genpd_init(&lpsc->pm_domain, NULL, is_on);
+
+	return lpsc;
+}
+
+static int davinci_lpsc_clk_reset(struct clk *clk, bool reset)
+{
+	struct clk_hw *hw = __clk_get_hw(clk);
+	struct davinci_lpsc_clk *lpsc = to_davinci_lpsc_clk(hw);
+	u32 mdctl;
+
+	if (IS_ERR_OR_NULL(lpsc))
+		return -EINVAL;
+
+	mdctl = reset ? 0 : MDCTL_LRESET;
+	regmap_write_bits(lpsc->regmap, MDCTL(lpsc->md), MDCTL_LRESET, mdctl);
+
+	return 0;
+}
+
+/*
+ * REVISIT: These exported functions can be removed after a non-DT lookup is
+ * added to the reset controller framework and the davinci-rproc driver is
+ * updated to use the generic reset controller framework.
+ */
+
+int davinci_clk_reset_assert(struct clk *clk)
+{
+	return davinci_lpsc_clk_reset(clk, true);
+}
+EXPORT_SYMBOL(davinci_clk_reset_assert);
+
+int davinci_clk_reset_deassert(struct clk *clk)
+{
+	return davinci_lpsc_clk_reset(clk, false);
+}
+EXPORT_SYMBOL(davinci_clk_reset_deassert);
+
+static int davinci_psc_reset_assert(struct reset_controller_dev *rcdev,
+				    unsigned long id)
+{
+	struct davinci_psc_data *psc = to_davinci_psc_data(rcdev);
+	struct clk *clk = psc->clk_data.clks[id];
+
+	return davinci_lpsc_clk_reset(clk, true);
+}
+
+static int davinci_psc_reset_deassert(struct reset_controller_dev *rcdev,
+				      unsigned long id)
+{
+	struct davinci_psc_data *psc = to_davinci_psc_data(rcdev);
+	struct clk *clk = psc->clk_data.clks[id];
+
+	return davinci_lpsc_clk_reset(clk, false);
+}
+
+static const struct reset_control_ops davinci_psc_reset_ops = {
+	.assert		= davinci_psc_reset_assert,
+	.deassert	= davinci_psc_reset_deassert,
+};
+
+static int davinci_psc_reset_of_xlate(struct reset_controller_dev *rcdev,
+				      const struct of_phandle_args *reset_spec)
+{
+	struct of_phandle_args clkspec = *reset_spec; /* discard const qualifier */
+	struct clk *clk;
+	struct clk_hw *hw;
+	struct davinci_lpsc_clk *lpsc;
+
+	/* the clock node is the same as the reset node */
+	clk = of_clk_get_from_provider(&clkspec);
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	hw = __clk_get_hw(clk);
+	lpsc = to_davinci_lpsc_clk(hw);
+	clk_put(clk);
+
+	/* not all modules support local reset */
+	if (!(lpsc->flags & LPSC_LOCAL_RESET))
+		return -EINVAL;
+
+	return lpsc->md;
+}
+
+static const struct regmap_config davinci_psc_regmap_config = {
+	.reg_bits	= 32,
+	.reg_stride	= 4,
+	.val_bits	= 32,
+};
+
+static struct davinci_psc_data *
+__davinci_psc_register_clocks(struct device *dev,
+			      const struct davinci_lpsc_clk_info *info,
+			      int num_clks,
+			      void __iomem *base)
+{
+	struct davinci_psc_data *psc;
+	struct clk **clks;
+	struct generic_pm_domain **pm_domains;
+	struct regmap *regmap;
+	int i, ret;
+
+	psc = devm_kzalloc(dev, sizeof(*psc), GFP_KERNEL);
+	if (!psc)
+		return ERR_PTR(-ENOMEM);
+
+	clks = devm_kmalloc_array(dev, num_clks, sizeof(*clks), GFP_KERNEL);
+	if (!clks)
+		return ERR_PTR(-ENOMEM);
+
+	psc->clk_data.clks = clks;
+	psc->clk_data.clk_num = num_clks;
+
+	/*
+	 * init array with error so that of_clk_src_onecell_get() doesn't
+	 * return NULL for gaps in the sparse array
+	 */
+	for (i = 0; i < num_clks; i++)
+		clks[i] = ERR_PTR(-ENOENT);
+
+	pm_domains = devm_kcalloc(dev, num_clks, sizeof(*pm_domains), GFP_KERNEL);
+	if (!pm_domains)
+		return ERR_PTR(-ENOMEM);
+
+	psc->pm_data.domains = pm_domains;
+	psc->pm_data.num_domains = num_clks;
+
+	regmap = devm_regmap_init_mmio(dev, base, &davinci_psc_regmap_config);
+	if (IS_ERR(regmap))
+		return ERR_CAST(regmap);
+
+	for (; info->name; info++) {
+		struct davinci_lpsc_clk *lpsc;
+
+		lpsc = davinci_lpsc_clk_register(dev, info->name, info->parent,
+						 regmap, info->md, info->pd,
+						 info->flags);
+		if (IS_ERR(lpsc)) {
+			dev_warn(dev, "Failed to register %s (%ld)\n",
+				 info->name, PTR_ERR(lpsc));
+			continue;
+		}
+
+		clks[info->md] = lpsc->hw.clk;
+		pm_domains[info->md] = &lpsc->pm_domain;
+	}
+
+	psc->rcdev.ops = &davinci_psc_reset_ops;
+	psc->rcdev.owner = THIS_MODULE;
+	psc->rcdev.of_node = dev->of_node;
+	psc->rcdev.of_reset_n_cells = 1;
+	psc->rcdev.of_xlate = davinci_psc_reset_of_xlate;
+	psc->rcdev.nr_resets = num_clks;
+
+	ret = devm_reset_controller_register(dev, &psc->rcdev);
+	if (ret < 0)
+		dev_warn(dev, "Failed to register reset controller (%d)\n", ret);
+
+	return psc;
+}
+
+int davinci_psc_register_clocks(struct device *dev,
+				const struct davinci_lpsc_clk_info *info,
+				u8 num_clks,
+				void __iomem *base)
+{
+	struct davinci_psc_data *psc;
+
+	psc = __davinci_psc_register_clocks(dev, info, num_clks, base);
+	if (IS_ERR(psc))
+		return PTR_ERR(psc);
+
+	for (; info->name; info++) {
+		const struct davinci_lpsc_clkdev_info *cdevs = info->cdevs;
+		struct clk *clk = psc->clk_data.clks[info->md];
+
+		if (!cdevs || IS_ERR_OR_NULL(clk))
+			continue;
+
+		for (; cdevs->con_id || cdevs->dev_id; cdevs++)
+			clk_register_clkdev(clk, cdevs->con_id, cdevs->dev_id);
+	}
+
+	return 0;
+}
+
+int of_davinci_psc_clk_init(struct device *dev,
+			    const struct davinci_lpsc_clk_info *info,
+			    u8 num_clks,
+			    void __iomem *base)
+{
+	struct device_node *node = dev->of_node;
+	struct davinci_psc_data *psc;
+
+	psc = __davinci_psc_register_clocks(dev, info, num_clks, base);
+	if (IS_ERR(psc))
+		return PTR_ERR(psc);
+
+	of_genpd_add_provider_onecell(node, &psc->pm_data);
+
+	of_clk_add_provider(node, of_clk_src_onecell_get, &psc->clk_data);
+
+	return 0;
+}
+
+static const struct of_device_id davinci_psc_of_match[] = {
+	{ }
+};
+
+static const struct platform_device_id davinci_psc_id_table[] = {
+	{ }
+};
+
+typedef int (*davinci_psc_init)(struct device *dev, void __iomem *base);
+
+static int davinci_psc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct of_device_id *of_id;
+	davinci_psc_init psc_init = NULL;
+	struct resource *res;
+	void __iomem *base;
+
+	of_id = of_match_device(davinci_psc_of_match, dev);
+	if (of_id)
+		psc_init = of_id->data;
+	else if (pdev->id_entry)
+		psc_init = (void *)pdev->id_entry->driver_data;
+
+	if (!psc_init) {
+		dev_err(dev, "unable to find driver data\n");
+		return -EINVAL;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base)) {
+		dev_err(dev, "ioremap failed\n");
+		return PTR_ERR(base);
+	}
+
+	return psc_init(dev, base);
+}
+
+static struct platform_driver davinci_psc_driver = {
+	.probe		= davinci_psc_probe,
+	.driver		= {
+		.name		= "davinci-psc-clk",
+		.of_match_table	= davinci_psc_of_match,
+	},
+	.id_table	= davinci_psc_id_table,
+};
+
+static int __init davinci_psc_driver_init(void)
+{
+	return platform_driver_register(&davinci_psc_driver);
+}
+
+/* has to be postcore_initcall because davinci_gpio depend on PSC clocks */
+postcore_initcall(davinci_psc_driver_init);
diff --git a/drivers/clk/davinci/psc.h b/drivers/clk/davinci/psc.h
new file mode 100644
index 0000000..c342d54
--- /dev/null
+++ b/drivers/clk/davinci/psc.h
@@ -0,0 +1,89 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Clock driver for TI Davinci PSC controllers
+ *
+ * Copyright (C) 2018 David Lechner <david@lechnology.com>
+ */
+
+#ifndef __CLK_DAVINCI_PSC_H__
+#define __CLK_DAVINCI_PSC_H__
+
+#include <linux/clk-provider.h>
+#include <linux/types.h>
+
+/* PSC quirk flags */
+#define LPSC_ALWAYS_ENABLED	BIT(0) /* never disable this clock */
+#define LPSC_SET_RATE_PARENT	BIT(1) /* propagate set_rate to parent clock */
+#define LPSC_FORCE		BIT(2) /* requires MDCTL FORCE bit */
+#define LPSC_LOCAL_RESET	BIT(3) /* acts as reset provider */
+
+struct davinci_lpsc_clkdev_info {
+	const char *con_id;
+	const char *dev_id;
+};
+
+#define LPSC_CLKDEV(c, d) {	\
+	.con_id = (c),		\
+	.dev_id = (d)		\
+}
+
+#define LPSC_CLKDEV1(n, c, d) \
+static const struct davinci_lpsc_clkdev_info n[] __initconst = {	\
+	LPSC_CLKDEV((c), (d)),						\
+	{ }								\
+}
+
+#define LPSC_CLKDEV2(n, c1, d1, c2, d2) \
+static const struct davinci_lpsc_clkdev_info n[] __initconst = {	\
+	LPSC_CLKDEV((c1), (d1)),					\
+	LPSC_CLKDEV((c2), (d2)),					\
+	{ }								\
+}
+
+#define LPSC_CLKDEV3(n, c1, d1, c2, d2, c3, d3) \
+static const struct davinci_lpsc_clkdev_info n[] __initconst = {	\
+	LPSC_CLKDEV((c1), (d1)),					\
+	LPSC_CLKDEV((c2), (d2)),					\
+	LPSC_CLKDEV((c3), (d3)),					\
+	{ }								\
+}
+
+/**
+ * davinci_lpsc_clk_info - LPSC module-specific clock information
+ * @name: the clock name
+ * @parent: the parent clock name
+ * @cdevs: optional array of clkdev lookup table info
+ * @md: the local module domain (LPSC id)
+ * @pd: the power domain id
+ * @flags: bitmask of LPSC_* flags
+ */
+struct davinci_lpsc_clk_info {
+	const char *name;
+	const char *parent;
+	const struct davinci_lpsc_clkdev_info *cdevs;
+	u32 md;
+	u32 pd;
+	unsigned long flags;
+};
+
+#define LPSC(m, d, n, p, c, f)	\
+{				\
+	.name	= #n,		\
+	.parent	= #p,		\
+	.cdevs	= (c),		\
+	.md	= (m),		\
+	.pd	= (d),		\
+	.flags	= (f),		\
+}
+
+int davinci_psc_register_clocks(struct device *dev,
+				const struct davinci_lpsc_clk_info *info,
+				u8 num_clks,
+				void __iomem *base);
+
+int of_davinci_psc_clk_init(struct device *dev,
+			    const struct davinci_lpsc_clk_info *info,
+			    u8 num_clks,
+			    void __iomem *base);
+
+#endif /* __CLK_DAVINCI_PSC_H__ */