On Fri, May 18, 2018 at 03:15:22PM +0530, Jagan Teki wrote:
> From: Jernej Skrabec <jernej.skra...@siol.net>
> 
> Some SoCs with DW HDMI have multiple possible clock parents, like A64
> and R40.
> 
> Expand HDMI PHY clock driver to support second clock parent.
> 
> Signed-off-by: Jernej Skrabec <jernej.skra...@siol.net>
> Signed-off-by: Jagan Teki <ja...@amarulasolutions.com>
> ---
> Changes for v2:
> - new patch
> 
>  drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h      |  9 ++-
>  drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c     | 33 ++++++++---
>  drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c | 89 
> ++++++++++++++++++++++--------
>  3 files changed, 96 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h 
> b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
> index 79154f0f674a..303189d6602c 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
> +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h
> @@ -98,7 +98,8 @@
>  #define SUN8I_HDMI_PHY_PLL_CFG1_LDO2_EN              BIT(29)
>  #define SUN8I_HDMI_PHY_PLL_CFG1_LDO1_EN              BIT(28)
>  #define SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33     BIT(27)
> -#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL     BIT(26)
> +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK BIT(26)
> +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT       26
>  #define SUN8I_HDMI_PHY_PLL_CFG1_PLLEN                BIT(25)
>  #define SUN8I_HDMI_PHY_PLL_CFG1_LDO_VSET(x)  ((x) << 22)
>  #define SUN8I_HDMI_PHY_PLL_CFG1_UNKNOWN(x)   ((x) << 20)
> @@ -146,7 +147,7 @@
>  struct sun8i_hdmi_phy;
>  
>  struct sun8i_hdmi_phy_variant {
> -     bool has_phy_clk;
> +     int  phy_clk_num;
>       void (*phy_init)(struct sun8i_hdmi_phy *phy);
>       void (*phy_disable)(struct dw_hdmi *hdmi,
>                           struct sun8i_hdmi_phy *phy);
> @@ -160,6 +161,7 @@ struct sun8i_hdmi_phy {
>       struct clk                      *clk_mod;
>       struct clk                      *clk_phy;
>       struct clk                      *clk_pll0;
> +     struct clk                      *clk_pll1;
>       unsigned int                    rcal;
>       struct regmap                   *regs;
>       struct reset_control            *rst_phy;
> @@ -188,6 +190,7 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi *hdmi);
>  void sun8i_hdmi_phy_init(struct sun8i_hdmi_phy *phy);
>  const struct dw_hdmi_phy_ops *sun8i_hdmi_phy_get_ops(void);
>  
> -int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev);
> +int sun8i_phy_clk_create(struct sun8i_hdmi_phy *phy, struct device *dev,
> +                      int clk_num);
>  
>  #endif /* _SUN8I_DW_HDMI_H_ */
> diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c 
> b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
> index 5a52fc489a9d..0eadf087fc46 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
> @@ -183,7 +183,13 @@ static int sun8i_hdmi_phy_config_h3(struct dw_hdmi *hdmi,
>       regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_ANA_CFG1_REG,
>                          SUN8I_HDMI_PHY_ANA_CFG1_TXEN_MASK, 0);
>  
> -     regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, pll_cfg1_init);
> +     /*
> +      * NOTE: We have to be careful not to overwrite PHY parent
> +      * clock selection bit and clock divider.
> +      */
> +     regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
> +                        (u32)~SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK,
> +                        pll_cfg1_init);
>       regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG,
>                          (u32)~SUN8I_HDMI_PHY_PLL_CFG2_PREDIV_MSK,
>                          pll_cfg2_init);
> @@ -232,7 +238,7 @@ static int sun8i_hdmi_phy_config(struct dw_hdmi *hdmi, 
> void *data,
>       regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_DBG_CTRL_REG,
>                          SUN8I_HDMI_PHY_DBG_CTRL_POL_MASK, val);
>  
> -     if (phy->variant->has_phy_clk)
> +     if (phy->variant->phy_clk_num)
>               clk_set_rate(phy->clk_phy, mode->crtc_clock * 1000);
>  
>       return phy->variant->phy_config(hdmi, phy, mode->crtc_clock * 1000);
> @@ -393,7 +399,7 @@ static const struct sun8i_hdmi_phy_variant 
> sun8i_a83t_hdmi_phy = {
>  };
>  
>  static const struct sun8i_hdmi_phy_variant sun8i_h3_hdmi_phy = {
> -     .has_phy_clk = true,
> +     .phy_clk_num = 1,
>       .phy_init = &sun8i_hdmi_phy_init_h3,
>       .phy_disable = &sun8i_hdmi_phy_disable_h3,
>       .phy_config = &sun8i_hdmi_phy_config_h3,
> @@ -464,7 +470,7 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi, 
> struct device_node *node)
>               goto err_put_clk_bus;
>       }
>  
> -     if (phy->variant->has_phy_clk) {
> +     if (phy->variant->phy_clk_num) {
>               phy->clk_pll0 = of_clk_get_by_name(node, "pll-0");
>               if (IS_ERR(phy->clk_pll0)) {
>                       dev_err(dev, "Could not get pll-0 clock\n");
> @@ -472,7 +478,16 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi, 
> struct device_node *node)
>                       goto err_put_clk_mod;
>               }
>  
> -             ret = sun8i_phy_clk_create(phy, dev);
> +             if (phy->variant->phy_clk_num) {
> +                     phy->clk_pll1 = of_clk_get_by_name(node, "pll-1");
> +                     if (IS_ERR(phy->clk_pll1)) {
> +                             dev_err(dev, "Could not get pll-1 clock\n");
> +                             ret = PTR_ERR(phy->clk_pll1);
> +                             goto err_put_clk_mod;
> +                     }
> +             }
> +

You have a bug here. If phy_clk_num == 1, you'll still try to lookup
pll-1.

And this is a bit sloppy, since if phy_clk_num == 3, you won't try to
lookup pll-2 either.

> +             ret = sun8i_phy_clk_create(phy, dev, phy->variant->phy_clk_num);
>               if (ret) {
>                       dev_err(dev, "Couldn't create the PHY clock\n");
>                       goto err_put_clk_pll0;
> @@ -515,8 +530,8 @@ int sun8i_hdmi_phy_probe(struct sun8i_dw_hdmi *hdmi, 
> struct device_node *node)
>  err_put_rst_phy:
>       reset_control_put(phy->rst_phy);
>  err_put_clk_pll0:
> -     if (phy->variant->has_phy_clk)
> -             clk_put(phy->clk_pll0);
> +     clk_put(phy->clk_pll0);
> +     clk_put(phy->clk_pll1);
>  err_put_clk_mod:
>       clk_put(phy->clk_mod);
>  err_put_clk_bus:
> @@ -536,8 +551,8 @@ void sun8i_hdmi_phy_remove(struct sun8i_dw_hdmi *hdmi)
>  
>       reset_control_put(phy->rst_phy);
>  
> -     if (phy->variant->has_phy_clk)
> -             clk_put(phy->clk_pll0);
> +     clk_put(phy->clk_pll0);
> +     clk_put(phy->clk_pll1);
>       clk_put(phy->clk_mod);
>       clk_put(phy->clk_bus);
>  }
> diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c 
> b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c
> index faea449812f8..85b12fc96dbc 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c
> @@ -22,29 +22,36 @@ static int sun8i_phy_clk_determine_rate(struct clk_hw *hw,
>  {
>       unsigned long rate = req->rate;
>       unsigned long best_rate = 0;
> -     struct clk_hw *parent;
> +     struct clk_hw *best_parent = NULL;
> +     struct clk_hw *parent = NULL;
>       int best_div = 1;
> -     int i;
> +     int i, p;
>  
> -     parent = clk_hw_get_parent(hw);
> -
> -     for (i = 1; i <= 16; i++) {
> -             unsigned long ideal = rate * i;
> -             unsigned long rounded;
> -
> -             rounded = clk_hw_round_rate(parent, ideal);
> -
> -             if (rounded == ideal) {
> -                     best_rate = rounded;
> -                     best_div = i;
> -                     break;
> -             }
> +     for (p = 0; p < clk_hw_get_num_parents(hw); p++) {
> +             parent = clk_hw_get_parent_by_index(hw, p);
> +             if (!parent)
> +                     continue;
>  
> -             if (!best_rate ||
> -                 abs(rate - rounded / i) <
> -                 abs(rate - best_rate / best_div)) {
> -                     best_rate = rounded;
> -                     best_div = i;
> +             for (i = 1; i <= 16; i++) {
> +                     unsigned long ideal = rate * i;
> +                     unsigned long rounded;
> +
> +                     rounded = clk_hw_round_rate(parent, ideal);
> +
> +                     if (rounded == ideal) {
> +                             best_rate = rounded;
> +                             best_div = i;
> +                             best_parent = parent;
> +                             break;
> +                     }
> +
> +                     if (!best_rate ||
> +                         abs(rate - rounded / i) <
> +                         abs(rate - best_rate / best_div)) {
> +                             best_rate = rounded;
> +                             best_div = i;
> +                             best_parent = parent;
> +                     }
>               }
>       }
>  
> @@ -95,22 +102,58 @@ static int sun8i_phy_clk_set_rate(struct clk_hw *hw, 
> unsigned long rate,
>       return 0;
>  }
>  
> +static u8 sun8i_phy_clk_get_parent(struct clk_hw *hw)
> +{
> +     struct sun8i_phy_clk *priv = hw_to_phy_clk(hw);
> +     u32 reg;
> +
> +     regmap_read(priv->phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, &reg);
> +     reg = (reg & SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK) >>
> +           SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT;
> +
> +     return reg;
> +}
> +
> +static int sun8i_phy_clk_set_parent(struct clk_hw *hw, u8 index)
> +{
> +     struct sun8i_phy_clk *priv = hw_to_phy_clk(hw);
> +
> +     if (index > 1)
> +             return -EINVAL;
> +
> +     regmap_update_bits(priv->phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG,
> +                        SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK,
> +                        index << SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_SHIFT);
> +
> +     return 0;
> +}
> +

The DT bindings changes and the clk changes should be part of separate
patches.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Attachment: signature.asc
Description: PGP signature

Reply via email to