Re: [RFC PATCH v2 05/13] clk: bd718x7: Support ROHM BD71828 clk block

[Stephen Boyd]

Quoting Matti Vaittinen (2019-10-24 04:44:40)
> diff --git a/drivers/clk/clk-bd718x7.c b/drivers/clk/clk-bd718x7.c
> index ae6e5baee330..d17a19e04592 100644
> --- a/drivers/clk/clk-bd718x7.c
> +++ b/drivers/clk/clk-bd718x7.c
> @@ -8,6 +8,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/mfd/rohm-bd718x7.h>
> +#include <linux/mfd/rohm-bd71828.h>
>  #include <linux/mfd/rohm-bd70528.h>

It would be really great to not need to include these random header
files in this driver and just use raw numbers somehow. Looks like maybe
it can be done by populating a different device name from the mfd driver
depending on the version of the clk controller desired? Then that can be
matched in this clk driver and we can just put the register info in this
file?

>  #include <linux/clk-provider.h>
>  #include <linux/clkdev.h>
> @@ -21,10 +22,8 @@ struct bd718xx_clk {
>         struct rohm_regmap_dev *mfd;
>  };
>  
> -static int bd71837_clk_set(struct clk_hw *hw, int status)
> +static int bd71837_clk_set(struct bd718xx_clk *c, int status)

should it be unsigned int status? Or maybe u32?

>  {
> -       struct bd718xx_clk *c = container_of(hw, struct bd718xx_clk, hw);
> -
>         return regmap_update_bits(c->mfd->regmap, c->reg, c->mask, status);
>  }
>  
> @@ -33,14 +32,16 @@ static void bd71837_clk_disable(struct clk_hw *hw)
>         int rv;
>         struct bd718xx_clk *c = container_of(hw, struct bd718xx_clk, hw);
>  
> -       rv = bd71837_clk_set(hw, 0);
> +       rv = bd71837_clk_set(c, 0);
>         if (rv)
>                 dev_dbg(&c->pdev->dev, "Failed to disable 32K clk (%d)\n", rv);
>  }
>  
>  static int bd71837_clk_enable(struct clk_hw *hw)
>  {
> -       return bd71837_clk_set(hw, 1);
> +       struct bd718xx_clk *c = container_of(hw, struct bd718xx_clk, hw);
> +
> +       return bd71837_clk_set(c, 0xffffffff);

Because now this is passing -1 to unsigned int taking
regmap_update_bits()?