[U-Boot] [PATCH] pwm: add MACRO to limit some code which only for rk3288

[Simon Glass]

Hi Kever,

On 7 July 2016 at 20:45, Kever Yang <kever.yang at rock-chips.com> wrote:
> The grf setting for rkpwm is only need in rk3288, other SoCs like
> RK3399 which also use rkpwm do not need set the grf, let's add a
> MACRO to make the code only for RK3288.
>
> Change-Id: I167a4e8cf925e840d4bbbcfb1437aaed52b81477

patman will automatically remove these for you.

> Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
> ---
>  drivers/pwm/rk_pwm.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/pwm/rk_pwm.c b/drivers/pwm/rk_pwm.c
> index 2d289a4..e34adf0 100644
> --- a/drivers/pwm/rk_pwm.c
> +++ b/drivers/pwm/rk_pwm.c
> @@ -13,8 +13,10 @@
>  #include <syscon.h>
>  #include <asm/io.h>
>  #include <asm/arch/clock.h>
> +#ifdef CONFIG_ROCKCHIP_RK3288
>  #include <asm/arch/cru_rk3288.h>
>  #include <asm/arch/grf_rk3288.h>
> +#endif
>  #include <asm/arch/pwm.h>
>  #include <power/regulator.h>
>
> @@ -22,7 +24,9 @@ DECLARE_GLOBAL_DATA_PTR;
>
>  struct rk_pwm_priv {
>         struct rk3288_pwm *regs;
> +#ifdef CONFIG_ROCKCHIP_RK3288
>         struct rk3288_grf *grf;
> +#endif
>  };
>
>  static int rk_pwm_set_config(struct udevice *dev, uint channel, uint period_ns,
> @@ -65,19 +69,23 @@ static int rk_pwm_ofdata_to_platdata(struct udevice *dev)
>         struct regmap *map;
>
>         priv->regs = (struct rk3288_pwm *)dev_get_addr(dev);
> +#ifdef CONFIG_ROCKCHIP_RK3288
>         map = syscon_get_regmap_by_driver_data(ROCKCHIP_SYSCON_GRF);
>         if (IS_ERR(map))
>                 return PTR_ERR(map);
>         priv->grf = regmap_get_range(map, 0);
> +#endif

I'd like to find a better way. Do all chips have a grf? If so then
perhaps we can have a function like grf_enable_pwm() in the core SoC
code and call it here. The #ifdef can be there.

Or perhaps we should have a general soc.c, with its own struct
containing pointers to grf, sgrf, etc. It can be set up at the start,
and then provide a soc_enable_pwm() function to enable the pwm.

What do you think?

>
>         return 0;
>  }
>
>  static int rk_pwm_probe(struct udevice *dev)
>  {
> +#ifdef CONFIG_ROCKCHIP_RK3288
>         struct rk_pwm_priv *priv = dev_get_priv(dev);
>
>         rk_setreg(&priv->grf->soc_con2, 1 << 0);
> +#endif
>
>         return 0;
>  }
> --
> 1.9.1
>
>

Regards,
Simon