git.openwrt.org Git - openwrt/staging/jow.git/blob

   1 From 0b6b245f8fcff205f097ce1c1bba4760f8b4f859 Mon Sep 17 00:00:00 2001
   2 From: Naushir Patuck <naush@raspberrypi.com>
   3 Date: Fri, 31 Mar 2023 10:33:51 +0100
   4 Subject: [PATCH] drivers: media: imx708: Tidy-ups to address upstream
   5  review comments
   6
   7 This commit addresses vaious tidy-ups requesed for upstreaming the
   8 IMX708 driver. Notably:
   9
  10 - Remove #define IMX708_NUM_SUPPLIES and use ARRAY_SIZE() directly
  11 - Use dev_err_probe where possible in imx708_probe()
  12 - Fix error handling paths in imx708_probe()
  13
  14 Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
  15 ---
  16  drivers/media/i2c/imx708.c | 61 +++++++++++++++++---------------------
  17  1 file changed, 28 insertions(+), 33 deletions(-)
  18
  19 --- a/drivers/media/i2c/imx708.c
  20 +++ b/drivers/media/i2c/imx708.c
  21 @@ -792,8 +792,6 @@ static const char * const imx708_supply_
  22         "VDDL",  /* IF (1.8V) supply */
  23  };
  24
  25 -#define IMX708_NUM_SUPPLIES ARRAY_SIZE(imx708_supply_name)
  26 -
  27  /*
  28   * Initialisation delay between XCLR low->high and the moment when the sensor
  29   * can start capture (i.e. can leave software standby), given by T7 in the
  30 @@ -815,7 +813,7 @@ struct imx708 {
  31         u32 xclk_freq;
  32
  33         struct gpio_desc *reset_gpio;
  34 -       struct regulator_bulk_data supplies[IMX708_NUM_SUPPLIES];
  35 +       struct regulator_bulk_data supplies[ARRAY_SIZE(imx708_supply_name)];
  36
  37         struct v4l2_ctrl_handler ctrl_handler;
  38         /* V4L2 Controls */
  39 @@ -935,9 +933,10 @@ static int imx708_write_regs(struct imx7
  40  {
  41         struct i2c_client *client = v4l2_get_subdevdata(&imx708->sd);
  42         unsigned int i;
  43 -       int ret;
  44
  45         for (i = 0; i < len; i++) {
  46 +               int ret;
  47 +
  48                 ret = imx708_write_reg(imx708, regs[i].address, 1, regs[i].val);
  49                 if (ret) {
  50                         dev_err_ratelimited(&client->dev,
  51 @@ -1025,8 +1024,6 @@ static int imx708_open(struct v4l2_subde
  52
  53  static int imx708_set_exposure(struct imx708 *imx708, unsigned int val)
  54  {
  55 -       int ret;
  56 -
  57         val = max(val, imx708->mode->exposure_lines_min);
  58         val -= val % imx708->mode->exposure_lines_step;
  59
  60 @@ -1034,11 +1031,9 @@ static int imx708_set_exposure(struct im
  61          * In HDR mode this will set the longest exposure. The sensor
  62          * will automatically divide the medium and short ones by 4,16.
  63          */
  64 -       ret = imx708_write_reg(imx708, IMX708_REG_EXPOSURE,
  65 -                              IMX708_REG_VALUE_16BIT,
  66 -                              val >> imx708->long_exp_shift);
  67 -
  68 -       return ret;
  69 +       return imx708_write_reg(imx708, IMX708_REG_EXPOSURE,
  70 +                               IMX708_REG_VALUE_16BIT,
  71 +                               val >> imx708->long_exp_shift);
  72  }
  73
  74  static void imx708_adjust_exposure_range(struct imx708 *imx708,
  75 @@ -1071,7 +1066,7 @@ static int imx708_set_analogue_gain(stru
  76
  77  static int imx708_set_frame_length(struct imx708 *imx708, unsigned int val)
  78  {
  79 -       int ret = 0;
  80 +       int ret;
  81
  82         imx708->long_exp_shift = 0;
  83
  84 @@ -1091,8 +1086,8 @@ static int imx708_set_frame_length(struc
  85
  86  static void imx708_set_framing_limits(struct imx708 *imx708)
  87  {
  88 -       unsigned int hblank;
  89         const struct imx708_mode *mode = imx708->mode;
  90 +       unsigned int hblank;
  91
  92         __v4l2_ctrl_modify_range(imx708->pixel_rate,
  93                                  mode->pixel_rate, mode->pixel_rate,
  94 @@ -1599,7 +1594,7 @@ static int imx708_power_on(struct device
  95         struct imx708 *imx708 = to_imx708(sd);
  96         int ret;
  97
  98 -       ret = regulator_bulk_enable(IMX708_NUM_SUPPLIES,
  99 +       ret = regulator_bulk_enable(ARRAY_SIZE(imx708_supply_name),
 100                                     imx708->supplies);
 101         if (ret) {
 102                 dev_err(&client->dev, "%s: failed to enable regulators\n",
 103 @@ -1621,7 +1616,8 @@ static int imx708_power_on(struct device
 104         return 0;
 105
 106  reg_off:
 107 -       regulator_bulk_disable(IMX708_NUM_SUPPLIES, imx708->supplies);
 108 +       regulator_bulk_disable(ARRAY_SIZE(imx708_supply_name),
 109 +                              imx708->supplies);
 110         return ret;
 111  }
 112
 113 @@ -1632,7 +1628,8 @@ static int imx708_power_off(struct devic
 114         struct imx708 *imx708 = to_imx708(sd);
 115
 116         gpiod_set_value_cansleep(imx708->reset_gpio, 0);
 117 -       regulator_bulk_disable(IMX708_NUM_SUPPLIES, imx708->supplies);
 118 +       regulator_bulk_disable(ARRAY_SIZE(imx708_supply_name),
 119 +                              imx708->supplies);
 120         clk_disable_unprepare(imx708->xclk);
 121
 122         /* Force reprogramming of the common registers when powered up again. */
 123 @@ -1679,11 +1676,11 @@ static int imx708_get_regulators(struct
 124         struct i2c_client *client = v4l2_get_subdevdata(&imx708->sd);
 125         unsigned int i;
 126
 127 -       for (i = 0; i < IMX708_NUM_SUPPLIES; i++)
 128 +       for (i = 0; i < ARRAY_SIZE(imx708_supply_name); i++)
 129                 imx708->supplies[i].supply = imx708_supply_name[i];
 130
 131         return devm_regulator_bulk_get(&client->dev,
 132 -                                      IMX708_NUM_SUPPLIES,
 133 +                                      ARRAY_SIZE(imx708_supply_name),
 134                                        imx708->supplies);
 135  }
 136
 137 @@ -1956,23 +1953,19 @@ static int imx708_probe(struct i2c_clien
 138
 139         /* Get system clock (xclk) */
 140         imx708->xclk = devm_clk_get(dev, NULL);
 141 -       if (IS_ERR(imx708->xclk)) {
 142 -               dev_err(dev, "failed to get xclk\n");
 143 -               return PTR_ERR(imx708->xclk);
 144 -       }
 145 +       if (IS_ERR(imx708->xclk))
 146 +               return dev_err_probe(dev, PTR_ERR(imx708->xclk),
 147 +                                    "failed to get xclk\n");
 148
 149         imx708->xclk_freq = clk_get_rate(imx708->xclk);
 150 -       if (imx708->xclk_freq != IMX708_XCLK_FREQ) {
 151 -               dev_err(dev, "xclk frequency not supported: %d Hz\n",
 152 -                       imx708->xclk_freq);
 153 -               return -EINVAL;
 154 -       }
 155 +       if (imx708->xclk_freq != IMX708_XCLK_FREQ)
 156 +               return dev_err_probe(dev, -EINVAL,
 157 +                                    "xclk frequency not supported: %d Hz\n",
 158 +                                    imx708->xclk_freq);
 159
 160         ret = imx708_get_regulators(imx708);
 161 -       if (ret) {
 162 -               dev_err(dev, "failed to get regulators\n");
 163 -               return ret;
 164 -       }
 165 +       if (ret)
 166 +               return dev_err_probe(dev, ret, "failed to get regulators\n");
 167
 168         /* Request optional enable pin */
 169         imx708->reset_gpio = devm_gpiod_get_optional(dev, "reset",
 170 @@ -2001,7 +1994,7 @@ static int imx708_probe(struct i2c_clien
 171         /* This needs the pm runtime to be registered. */
 172         ret = imx708_init_controls(imx708);
 173         if (ret)
 174 -               goto error_power_off;
 175 +               goto error_pm_runtime;
 176
 177         /* Initialize subdev */
 178         imx708->sd.internal_ops = &imx708_internal_ops;
 179 @@ -2033,9 +2026,11 @@ error_media_entity:
 180  error_handler_free:
 181         imx708_free_controls(imx708);
 182
 183 -error_power_off:
 184 +error_pm_runtime:
 185         pm_runtime_disable(&client->dev);
 186         pm_runtime_set_suspended(&client->dev);
 187 +
 188 +error_power_off:
 189         imx708_power_off(&client->dev);
 190
 191         return ret;