Re: [PATCH 7/8] [media] exynos-gsc: Use clk bulk API

[kbuild test robot]

Hi Maciej,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.16-rc2 next-20180219]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Maciej-Purski/Use-clk-bulk-API-in-exynos5433-drivers/20180220-054431
base:   git://linuxtv.org/media_tree.git master
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64 

All warnings (new ones prefixed by >>):

   drivers/media/platform/exynos-gsc/gsc-core.c: In function 'gsc_probe':
>> drivers/media/platform/exynos-gsc/gsc-core.c:1190:8: warning: passing 
>> argument 3 of 'devm_clk_bulk_alloc' discards 'const' qualifier from pointer 
>> target type [-Wdiscarded-qualifiers]
   drv_data->clk_names);
   ^~~~
   In file included from drivers/media/platform/exynos-gsc/gsc-core.c:25:0:
   include/linux/clk.h:654:37: note: expected 'const char **' but argument is 
of type 'const char * const*'
static inline struct clk_bulk_data *devm_clk_bulk_alloc(struct device *dev,
^~~

vim +1190 drivers/media/platform/exynos-gsc/gsc-core.c

  1144  
  1145  static int gsc_probe(struct platform_device *pdev)
  1146  {
  1147  struct gsc_dev *gsc;
  1148  struct resource *res;
  1149  struct device *dev = >dev;
  1150  const struct gsc_driverdata *drv_data = 
of_device_get_match_data(dev);
  1151  int ret;
  1152  
  1153  gsc = devm_kzalloc(dev, sizeof(struct gsc_dev), GFP_KERNEL);
  1154  if (!gsc)
  1155  return -ENOMEM;
  1156  
  1157  ret = of_alias_get_id(pdev->dev.of_node, "gsc");
  1158  if (ret < 0)
  1159  return ret;
  1160  
  1161  if (drv_data == _v_100_drvdata)
  1162  dev_info(dev, "compatible 'exynos5-gsc' is 
deprecated\n");
  1163  
  1164  gsc->id = ret;
  1165  if (gsc->id >= drv_data->num_entities) {
  1166  dev_err(dev, "Invalid platform device id: %d\n", 
gsc->id);
  1167  return -EINVAL;
  1168  }
  1169  
  1170  gsc->num_clocks = drv_data->num_clocks;
  1171  gsc->variant = drv_data->variant[gsc->id];
  1172  gsc->pdev = pdev;
  1173  
  1174  init_waitqueue_head(>irq_queue);
  1175  spin_lock_init(>slock);
  1176  mutex_init(>lock);
  1177  
  1178  res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  1179  gsc->regs = devm_ioremap_resource(dev, res);
  1180  if (IS_ERR(gsc->regs))
  1181  return PTR_ERR(gsc->regs);
  1182  
  1183  res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
  1184  if (!res) {
  1185  dev_err(dev, "failed to get IRQ resource\n");
  1186  return -ENXIO;
  1187  }
  1188  
  1189  gsc->clocks = devm_clk_bulk_alloc(dev, gsc->num_clocks,
> 1190drv_data->clk_names);
  1191  if (IS_ERR(gsc->clocks))
  1192  return PTR_ERR(gsc->clocks);
  1193  
  1194  ret = devm_clk_bulk_get(dev, gsc->num_clocks,
  1195  gsc->clocks);
  1196  if (ret)
  1197  return ret;
  1198  
  1199  ret = clk_bulk_prepare_enable(gsc->num_clocks, gsc->clocks);
  1200  if (ret)
  1201  return ret;
  1202  
  1203  ret = devm_request_irq(dev, res->start, gsc_irq_handler,
  1204  0, pdev->name, gsc);
  1205  if (ret) {
  1206  dev_err(dev, "failed to install irq (%d)\n", ret);
  1207  goto err_clk;
  1208  }
  1209  
  1210  ret = v4l2_device_register(dev, >v4l2_dev);
  1211  if (ret)
  1212  goto err_clk;
  1213  
  1214  ret = gsc_register_m2m_device(gsc);
  1215  if (ret)
  1216  goto err_v4l2;
  1217  
  1218  platform_set_drvdata(pdev, gsc);
  1219  
  1220  gsc_hw_set_sw_reset(gsc);
  1221  gsc_wait_reset(gsc);
  1222  
  1223  vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32));
  1224  
  1225  dev_dbg(dev, "gsc-%d registered successfully\n", gsc->id);
  1226  
  1227  pm_runtime_set_active(dev);
  1228  pm_runtime_enable(dev);
  1229  
  1230  return 0;
  1231  
  1232  err_v4l2:
  1233  v4l2_device_unregister(>v4l2_dev);
  1234  err_clk:
  1235  

[PATCH 7/8] [media] exynos-gsc: Use clk bulk API

[Maciej Purski]

Using bulk clk functions simplifies the driver's code. Use devm_clk_bulk
functions instead of iterating over an array of clks.

Signed-off-by: Maciej Purski 
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 55 ++--
 drivers/media/platform/exynos-gsc/gsc-core.h |  2 +-
 2 files changed, 20 insertions(+), 37 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c 
b/drivers/media/platform/exynos-gsc/gsc-core.c
index 17854a3..fa7e993 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -1149,7 +1149,6 @@ static int gsc_probe(struct platform_device *pdev)
struct device *dev = >dev;
const struct gsc_driverdata *drv_data = of_device_get_match_data(dev);
int ret;
-   int i;
 
gsc = devm_kzalloc(dev, sizeof(struct gsc_dev), GFP_KERNEL);
if (!gsc)
@@ -1187,25 +1186,19 @@ static int gsc_probe(struct platform_device *pdev)
return -ENXIO;
}
 
-   for (i = 0; i < gsc->num_clocks; i++) {
-   gsc->clock[i] = devm_clk_get(dev, drv_data->clk_names[i]);
-   if (IS_ERR(gsc->clock[i])) {
-   dev_err(dev, "failed to get clock: %s\n",
-   drv_data->clk_names[i]);
-   return PTR_ERR(gsc->clock[i]);
-   }
-   }
+   gsc->clocks = devm_clk_bulk_alloc(dev, gsc->num_clocks,
+ drv_data->clk_names);
+   if (IS_ERR(gsc->clocks))
+   return PTR_ERR(gsc->clocks);
 
-   for (i = 0; i < gsc->num_clocks; i++) {
-   ret = clk_prepare_enable(gsc->clock[i]);
-   if (ret) {
-   dev_err(dev, "clock prepare failed for clock: %s\n",
-   drv_data->clk_names[i]);
-   while (--i >= 0)
-   clk_disable_unprepare(gsc->clock[i]);
-   return ret;
-   }
-   }
+   ret = devm_clk_bulk_get(dev, gsc->num_clocks,
+   gsc->clocks);
+   if (ret)
+   return ret;
+
+   ret = clk_bulk_prepare_enable(gsc->num_clocks, gsc->clocks);
+   if (ret)
+   return ret;
 
ret = devm_request_irq(dev, res->start, gsc_irq_handler,
0, pdev->name, gsc);
@@ -1239,15 +1232,14 @@ static int gsc_probe(struct platform_device *pdev)
 err_v4l2:
v4l2_device_unregister(>v4l2_dev);
 err_clk:
-   for (i = gsc->num_clocks - 1; i >= 0; i--)
-   clk_disable_unprepare(gsc->clock[i]);
+   clk_bulk_disable_unprepare(gsc->num_clocks, gsc->clocks);
+
return ret;
 }
 
 static int gsc_remove(struct platform_device *pdev)
 {
struct gsc_dev *gsc = platform_get_drvdata(pdev);
-   int i;
 
pm_runtime_get_sync(>dev);
 
@@ -1255,8 +1247,7 @@ static int gsc_remove(struct platform_device *pdev)
v4l2_device_unregister(>v4l2_dev);
 
vb2_dma_contig_clear_max_seg_size(>dev);
-   for (i = 0; i < gsc->num_clocks; i++)
-   clk_disable_unprepare(gsc->clock[i]);
+   clk_bulk_disable_unprepare(gsc->num_clocks, gsc->clocks);
 
pm_runtime_put_noidle(>dev);
pm_runtime_disable(>dev);
@@ -1307,18 +1298,12 @@ static int gsc_runtime_resume(struct device *dev)
 {
struct gsc_dev *gsc = dev_get_drvdata(dev);
int ret = 0;
-   int i;
 
pr_debug("gsc%d: state: 0x%lx\n", gsc->id, gsc->state);
 
-   for (i = 0; i < gsc->num_clocks; i++) {
-   ret = clk_prepare_enable(gsc->clock[i]);
-   if (ret) {
-   while (--i >= 0)
-   clk_disable_unprepare(gsc->clock[i]);
-   return ret;
-   }
-   }
+   ret = clk_bulk_prepare_enable(gsc->num_clocks, gsc->clocks);
+   if (ret)
+   return ret;
 
gsc_hw_set_sw_reset(gsc);
gsc_wait_reset(gsc);
@@ -1331,14 +1316,12 @@ static int gsc_runtime_suspend(struct device *dev)
 {
struct gsc_dev *gsc = dev_get_drvdata(dev);
int ret = 0;
-   int i;
 
ret = gsc_m2m_suspend(gsc);
if (ret)
return ret;
 
-   for (i = gsc->num_clocks - 1; i >= 0; i--)
-   clk_disable_unprepare(gsc->clock[i]);
+   clk_bulk_disable_unprepare(gsc->num_clocks, gsc->clocks);
 
pr_debug("gsc%d: state: 0x%lx\n", gsc->id, gsc->state);
return ret;
diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h 
b/drivers/media/platform/exynos-gsc/gsc-core.h
index 715d9c9d..08ff7b9 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.h
+++ b/drivers/media/platform/exynos-gsc/gsc-core.h
@@ -334,7 +334,7 @@ struct gsc_dev {
struct gsc_variant  *variant;
u16 id;
int