[PATCH 2/2] drm: bridge: icn6211: Add support for external REFCLK

[Robert Foss]

Hey Marek,

On Mon, 1 Aug 2022 at 15:18, Marek Vasut <marex at denx.de> wrote:
>
> The ICN6211 is capable of deriving its internal PLL clock from either
> MIPI DSI HS clock, external REFCLK clock, or even internal oscillator.
> Currently supported is only the first option. Add support for external
> REFCLK clock input in addition to that.
>
> There is little difference between these options, except that in case
> of MIPI DSI HS clock input, the HS clock are pre-divided by a fixed /4
> divider before being fed to the PLL input, while in case of external
> REFCLK, the RECLK clock are fed directly into the PLL input.
>
> Per exceptionally poor documentation, the REFCLK must be in range of
> 10..154 MHz.
>
> Signed-off-by: Marek Vasut <marex at denx.de>
> Cc: Jagan Teki <jagan at amarulasolutions.com>
> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Cc: Linus Walleij <linus.walleij at linaro.org>
> Cc: Robert Foss <robert.foss at linaro.org>
> Cc: Sam Ravnborg <sam at ravnborg.org>
> Cc: dri-devel at lists.freedesktop.org
> ---
>  drivers/gpu/drm/bridge/chipone-icn6211.c | 39 +++++++++++++++++++++---
>  1 file changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c b/drivers/gpu/drm/bridge/chipone-icn6211.c
> index 65966f280cf4e..7ee1858bab321 100644
> --- a/drivers/gpu/drm/bridge/chipone-icn6211.c
> +++ b/drivers/gpu/drm/bridge/chipone-icn6211.c
> @@ -11,6 +11,7 @@
>
>  #include <linux/bitfield.h>
>  #include <linux/bits.h>
> +#include <linux/clk.h>
>  #include <linux/delay.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/i2c.h>
> @@ -151,6 +152,8 @@ struct chipone {
>         struct regulator *vdd1;
>         struct regulator *vdd2;
>         struct regulator *vdd3;
> +       struct clk *refclk;
> +       unsigned long refclk_rate;
>         bool interface_i2c;
>  };
>
> @@ -273,7 +276,10 @@ static void chipone_configure_pll(struct chipone *icn,
>          * It seems the PLL input clock after applying P pre-divider have
>          * to be lower than 20 MHz.
>          */
> -       fin = icn->dsi->hs_rate / 4; /* in Hz */
> +       if (icn->refclk)
> +               fin = icn->refclk_rate;
> +       else
> +               fin = icn->dsi->hs_rate / 4; /* in Hz */
>
>         /* Minimum value of P predivider for PLL input in 5..20 MHz */
>         p_min = clamp(DIV_ROUND_UP(fin, 20000000), 1U, 31U);
> @@ -318,16 +324,18 @@ static void chipone_configure_pll(struct chipone *icn,
>         best_p_pot = !(best_p & 1);
>
>         dev_dbg(icn->dev,
> -               "PLL: P[3:0]=%d P[4]=2*%d M=%d S[7:5]=2^%d delta=%d => DSI f_in=%d Hz ; DPI f_out=%d Hz\n",
> +               "PLL: P[3:0]=%d P[4]=2*%d M=%d S[7:5]=2^%d delta=%d => DSI f_in(%s)=%d Hz ; DPI f_out=%d Hz\n",
>                 best_p >> best_p_pot, best_p_pot, best_m, best_s + 1,
> -               min_delta, fin, (fin * best_m) / (best_p << (best_s + 1)));
> +               min_delta, icn->refclk ? "EXT" : "DSI", fin,
> +               (fin * best_m) / (best_p << (best_s + 1)));
>
>         ref_div = PLL_REF_DIV_P(best_p >> best_p_pot) | PLL_REF_DIV_S(best_s);
>         if (best_p_pot) /* Prefer /2 pre-divider */
>                 ref_div |= PLL_REF_DIV_Pe;
>
> -       /* Clock source selection fixed to MIPI DSI clock lane */
> -       chipone_writeb(icn, PLL_CTRL(6), PLL_CTRL_6_MIPI_CLK);
> +       /* Clock source selection either external clock or MIPI DSI clock lane */
> +       chipone_writeb(icn, PLL_CTRL(6),
> +                      icn->refclk ? PLL_CTRL_6_EXTERNAL : PLL_CTRL_6_MIPI_CLK);
>         chipone_writeb(icn, PLL_REF_DIV, ref_div);
>         chipone_writeb(icn, PLL_INT(0), best_m);
>  }
> @@ -463,6 +471,11 @@ static void chipone_atomic_pre_enable(struct drm_bridge *bridge,
>                                       "failed to enable VDD3 regulator: %d\n", ret);
>         }
>
> +       ret = clk_prepare_enable(icn->refclk);
> +       if (ret)
> +               DRM_DEV_ERROR(icn->dev,
> +                             "failed to enable RECLK clock: %d\n", ret);
> +
>         gpiod_set_value(icn->enable_gpio, 1);
>
>         usleep_range(10000, 11000);
> @@ -473,6 +486,8 @@ static void chipone_atomic_post_disable(struct drm_bridge *bridge,
>  {
>         struct chipone *icn = bridge_to_chipone(bridge);
>
> +       clk_disable_unprepare(icn->refclk);
> +
>         if (icn->vdd1)
>                 regulator_disable(icn->vdd1);
>
> @@ -618,6 +633,20 @@ static int chipone_parse_dt(struct chipone *icn)
>         struct device *dev = icn->dev;
>         int ret;
>
> +       icn->refclk = devm_clk_get_optional(dev, "refclk");
> +       if (IS_ERR(icn->refclk)) {
> +               ret = PTR_ERR(icn->refclk);
> +               DRM_DEV_ERROR(dev, "failed to get REFCLK clock: %d\n", ret);
> +               return ret;
> +       } else if (icn->refclk) {
> +               icn->refclk_rate = clk_get_rate(icn->refclk);
> +               if (icn->refclk_rate < 10000000 || icn->refclk_rate > 154000000) {
> +                       DRM_DEV_ERROR(dev, "REFCLK out of range: %ld Hz\n",
> +                                     icn->refclk_rate);
> +                       return -EINVAL;
> +               }
> +       }
> +
>         icn->vdd1 = devm_regulator_get_optional(dev, "vdd1");
>         if (IS_ERR(icn->vdd1)) {
>                 ret = PTR_ERR(icn->vdd1);
> --
> 2.35.1
>

This patch looks good to me, but it doesn't apply on drm-misc-next. Do
you mind re-spinning it?

Rob.