[PATCH 0/5] smiapp cleanups, retry probe if getting clock fails

[PATCH 0/5] smiapp cleanups, retry probe if getting clock fails

[Sakari Ailus]

Hi all,

These patches contain cleanups for the smiapp driver and return
-EPROBE_DEFER if getting the clock fails.

-- 
Kind regards,
Sakari

[PATCH 1/5] smiapp: Unify enforced and need-based 8-bit read

[Sakari Ailus]

From: Sakari Ailus <sakari.ailus@iki.fi>

Unify enforced 8-bit read access with that based on actual need.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-regs.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-regs.c b/drivers/media/i2c/smiapp/smiapp-regs.c
index 6b6c20b..1e501c0 100644
--- a/drivers/media/i2c/smiapp/smiapp-regs.c
+++ b/drivers/media/i2c/smiapp/smiapp-regs.c
@@ -188,7 +188,8 @@ int smiapp_read_no_quirk(struct smiapp_sensor *sensor, u32 reg, u32 *val)
 				   SMIAPP_QUIRK_FLAG_8BIT_READ_ONLY));
 }
 
-int smiapp_read(struct smiapp_sensor *sensor, u32 reg, u32 *val)
+static int smiapp_read_quirk(struct smiapp_sensor *sensor, u32 reg, u32 *val,
+			     bool force8)
 {
 	int rval;
 
@@ -199,21 +200,20 @@ int smiapp_read(struct smiapp_sensor *sensor, u32 reg, u32 *val)
 	if (rval < 0)
 		return rval;
 
+	if (force8)
+		return __smiapp_read(sensor, reg, val, true);
+
 	return smiapp_read_no_quirk(sensor, reg, val);
 }
 
-int smiapp_read_8only(struct smiapp_sensor *sensor, u32 reg, u32 *val)
+int smiapp_read(struct smiapp_sensor *sensor, u32 reg, u32 *val)
 {
-	int rval;
-
-	*val = 0;
-	rval = smiapp_call_quirk(sensor, reg_access, false, &reg, val);
-	if (rval == -ENOIOCTLCMD)
-		return 0;
-	if (rval < 0)
-		return rval;
+	return smiapp_read_quirk(sensor, reg, val, false);
+}
 
-	return __smiapp_read(sensor, reg, val, true);
+int smiapp_read_8only(struct smiapp_sensor *sensor, u32 reg, u32 *val)
+{
+	return smiapp_read_quirk(sensor, reg, val, true);
 }
 
 int smiapp_write_no_quirk(struct smiapp_sensor *sensor, u32 reg, u32 val)
-- 
2.7.4

[PATCH 2/5] smiapp: Rename smiapp_platform_data as smiapp_hwconfig

[Sakari Ailus]

This is really configuration to the driver originating from DT or
elsewhere. Do not call it platform data.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c  | 140 ++++++++++++++++----------------
 drivers/media/i2c/smiapp/smiapp-quirk.c |   4 +-
 drivers/media/i2c/smiapp/smiapp.h       |   2 +-
 include/media/i2c/smiapp.h              |   2 +-
 4 files changed, 74 insertions(+), 74 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index d08ab6c..92a6859 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -625,12 +625,12 @@ static int smiapp_init_late_controls(struct smiapp_sensor *sensor)
 				0, max_value, 1, max_value);
 	}
 
-	for (max = 0; sensor->platform_data->op_sys_clock[max + 1]; max++);
+	for (max = 0; sensor->hwcfg->op_sys_clock[max + 1]; max++);
 
 	sensor->link_freq = v4l2_ctrl_new_int_menu(
 		&sensor->src->ctrl_handler, &smiapp_ctrl_ops,
 		V4L2_CID_LINK_FREQ, __fls(*valid_link_freqs),
-		__ffs(*valid_link_freqs), sensor->platform_data->op_sys_clock);
+		__ffs(*valid_link_freqs), sensor->hwcfg->op_sys_clock);
 
 	return sensor->src->ctrl_handler.error;
 }
@@ -833,8 +833,8 @@ static int smiapp_get_mbus_formats(struct smiapp_sensor *sensor)
 
 		pll->bits_per_pixel = f->compressed;
 
-		for (j = 0; sensor->platform_data->op_sys_clock[j]; j++) {
-			pll->link_freq = sensor->platform_data->op_sys_clock[j];
+		for (j = 0; sensor->hwcfg->op_sys_clock[j]; j++) {
+			pll->link_freq = sensor->hwcfg->op_sys_clock[j];
 
 			rval = smiapp_pll_try(sensor, pll);
 			dev_dbg(&client->dev, "link freq %u Hz, bpp %u %s\n",
@@ -1032,22 +1032,22 @@ static int smiapp_change_cci_addr(struct smiapp_sensor *sensor)
 	int rval;
 	u32 val;
 
-	client->addr = sensor->platform_data->i2c_addr_dfl;
+	client->addr = sensor->hwcfg->i2c_addr_dfl;
 
 	rval = smiapp_write(sensor,
 			    SMIAPP_REG_U8_CCI_ADDRESS_CONTROL,
-			    sensor->platform_data->i2c_addr_alt << 1);
+			    sensor->hwcfg->i2c_addr_alt << 1);
 	if (rval)
 		return rval;
 
-	client->addr = sensor->platform_data->i2c_addr_alt;
+	client->addr = sensor->hwcfg->i2c_addr_alt;
 
 	/* verify addr change went ok */
 	rval = smiapp_read(sensor, SMIAPP_REG_U8_CCI_ADDRESS_CONTROL, &val);
 	if (rval)
 		return rval;
 
-	if (val != sensor->platform_data->i2c_addr_alt << 1)
+	if (val != sensor->hwcfg->i2c_addr_alt << 1)
 		return -ENODEV;
 
 	return 0;
@@ -1061,13 +1061,13 @@ static int smiapp_change_cci_addr(struct smiapp_sensor *sensor)
 static int smiapp_setup_flash_strobe(struct smiapp_sensor *sensor)
 {
 	struct smiapp_flash_strobe_parms *strobe_setup;
-	unsigned int ext_freq = sensor->platform_data->ext_clk;
+	unsigned int ext_freq = sensor->hwcfg->ext_clk;
 	u32 tmp;
 	u32 strobe_adjustment;
 	u32 strobe_width_high_rs;
 	int rval;
 
-	strobe_setup = sensor->platform_data->strobe_setup;
+	strobe_setup = sensor->hwcfg->strobe_setup;
 
 	/*
 	 * How to calculate registers related to strobe length. Please
@@ -1179,7 +1179,7 @@ static int smiapp_setup_flash_strobe(struct smiapp_sensor *sensor)
 			    strobe_setup->trigger);
 
 out:
-	sensor->platform_data->strobe_setup->trigger = 0;
+	sensor->hwcfg->strobe_setup->trigger = 0;
 
 	return rval;
 }
@@ -1201,9 +1201,9 @@ static int smiapp_power_on(struct smiapp_sensor *sensor)
 	}
 	usleep_range(1000, 1000);
 
-	if (sensor->platform_data->set_xclk)
-		rval = sensor->platform_data->set_xclk(
-			&sensor->src->sd, sensor->platform_data->ext_clk);
+	if (sensor->hwcfg->set_xclk)
+		rval = sensor->hwcfg->set_xclk(
+			&sensor->src->sd, sensor->hwcfg->ext_clk);
 	else
 		rval = clk_prepare_enable(sensor->ext_clk);
 	if (rval < 0) {
@@ -1212,10 +1212,10 @@ static int smiapp_power_on(struct smiapp_sensor *sensor)
 	}
 	usleep_range(1000, 1000);
 
-	if (gpio_is_valid(sensor->platform_data->xshutdown))
-		gpio_set_value(sensor->platform_data->xshutdown, 1);
+	if (gpio_is_valid(sensor->hwcfg->xshutdown))
+		gpio_set_value(sensor->hwcfg->xshutdown, 1);
 
-	sleep = SMIAPP_RESET_DELAY(sensor->platform_data->ext_clk);
+	sleep = SMIAPP_RESET_DELAY(sensor->hwcfg->ext_clk);
 	usleep_range(sleep, sleep);
 
 	/*
@@ -1229,7 +1229,7 @@ static int smiapp_power_on(struct smiapp_sensor *sensor)
 	 * is found.
 	 */
 
-	if (sensor->platform_data->i2c_addr_alt) {
+	if (sensor->hwcfg->i2c_addr_alt) {
 		rval = smiapp_change_cci_addr(sensor);
 		if (rval) {
 			dev_err(&client->dev, "cci address change error\n");
@@ -1244,7 +1244,7 @@ static int smiapp_power_on(struct smiapp_sensor *sensor)
 		goto out_cci_addr_fail;
 	}
 
-	if (sensor->platform_data->i2c_addr_alt) {
+	if (sensor->hwcfg->i2c_addr_alt) {
 		rval = smiapp_change_cci_addr(sensor);
 		if (rval) {
 			dev_err(&client->dev, "cci address change error\n");
@@ -1261,14 +1261,14 @@ static int smiapp_power_on(struct smiapp_sensor *sensor)
 
 	rval = smiapp_write(
 		sensor, SMIAPP_REG_U16_EXTCLK_FREQUENCY_MHZ,
-		sensor->platform_data->ext_clk / (1000000 / (1 << 8)));
+		sensor->hwcfg->ext_clk / (1000000 / (1 << 8)));
 	if (rval) {
 		dev_err(&client->dev, "extclk frequency set failed\n");
 		goto out_cci_addr_fail;
 	}
 
 	rval = smiapp_write(sensor, SMIAPP_REG_U8_CSI_LANE_MODE,
-			    sensor->platform_data->lanes - 1);
+			    sensor->hwcfg->lanes - 1);
 	if (rval) {
 		dev_err(&client->dev, "csi lane mode set failed\n");
 		goto out_cci_addr_fail;
@@ -1282,7 +1282,7 @@ static int smiapp_power_on(struct smiapp_sensor *sensor)
 	}
 
 	rval = smiapp_write(sensor, SMIAPP_REG_U8_CSI_SIGNALLING_MODE,
-			    sensor->platform_data->csi_signalling_mode);
+			    sensor->hwcfg->csi_signalling_mode);
 	if (rval) {
 		dev_err(&client->dev, "csi signalling mode set failed\n");
 		goto out_cci_addr_fail;
@@ -1322,10 +1322,10 @@ static int smiapp_power_on(struct smiapp_sensor *sensor)
 	return 0;
 
 out_cci_addr_fail:
-	if (gpio_is_valid(sensor->platform_data->xshutdown))
-		gpio_set_value(sensor->platform_data->xshutdown, 0);
-	if (sensor->platform_data->set_xclk)
-		sensor->platform_data->set_xclk(&sensor->src->sd, 0);
+	if (gpio_is_valid(sensor->hwcfg->xshutdown))
+		gpio_set_value(sensor->hwcfg->xshutdown, 0);
+	if (sensor->hwcfg->set_xclk)
+		sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
 	else
 		clk_disable_unprepare(sensor->ext_clk);
 
@@ -1343,15 +1343,15 @@ static void smiapp_power_off(struct smiapp_sensor *sensor)
 	 * really see a power off and next time the cci address change
 	 * will fail. So do a soft reset explicitly here.
 	 */
-	if (sensor->platform_data->i2c_addr_alt)
+	if (sensor->hwcfg->i2c_addr_alt)
 		smiapp_write(sensor,
 			     SMIAPP_REG_U8_SOFTWARE_RESET,
 			     SMIAPP_SOFTWARE_RESET);
 
-	if (gpio_is_valid(sensor->platform_data->xshutdown))
-		gpio_set_value(sensor->platform_data->xshutdown, 0);
-	if (sensor->platform_data->set_xclk)
-		sensor->platform_data->set_xclk(&sensor->src->sd, 0);
+	if (gpio_is_valid(sensor->hwcfg->xshutdown))
+		gpio_set_value(sensor->hwcfg->xshutdown, 0);
+	if (sensor->hwcfg->set_xclk)
+		sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
 	else
 		clk_disable_unprepare(sensor->ext_clk);
 	usleep_range(5000, 5000);
@@ -1491,8 +1491,8 @@ static int smiapp_start_streaming(struct smiapp_sensor *sensor)
 	if ((sensor->limits[SMIAPP_LIMIT_FLASH_MODE_CAPABILITY] &
 	     (SMIAPP_FLASH_MODE_CAPABILITY_SINGLE_STROBE |
 	      SMIAPP_FLASH_MODE_CAPABILITY_MULTIPLE_STROBE)) &&
-	    sensor->platform_data->strobe_setup != NULL &&
-	    sensor->platform_data->strobe_setup->trigger != 0) {
+	    sensor->hwcfg->strobe_setup != NULL &&
+	    sensor->hwcfg->strobe_setup->trigger != 0) {
 		rval = smiapp_setup_flash_strobe(sensor);
 		if (rval)
 			goto out;
@@ -2309,7 +2309,7 @@ smiapp_sysfs_nvm_read(struct device *dev, struct device_attribute *attr,
 
 	if (!sensor->nvm_size) {
 		/* NVM not read yet - read it now */
-		sensor->nvm_size = sensor->platform_data->nvm_size;
+		sensor->nvm_size = sensor->hwcfg->nvm_size;
 		if (smiapp_set_power(subdev, 1) < 0)
 			return -ENODEV;
 		if (smiapp_read_nvm(sensor, sensor->nvm)) {
@@ -2554,7 +2554,7 @@ static int smiapp_init(struct smiapp_sensor *sensor)
 		return PTR_ERR(sensor->vana);
 	}
 
-	if (!sensor->platform_data->set_xclk) {
+	if (!sensor->hwcfg->set_xclk) {
 		sensor->ext_clk = devm_clk_get(&client->dev, NULL);
 		if (IS_ERR(sensor->ext_clk)) {
 			dev_err(&client->dev, "could not get clock\n");
@@ -2562,23 +2562,23 @@ static int smiapp_init(struct smiapp_sensor *sensor)
 		}
 
 		rval = clk_set_rate(sensor->ext_clk,
-				    sensor->platform_data->ext_clk);
+				    sensor->hwcfg->ext_clk);
 		if (rval < 0) {
 			dev_err(&client->dev,
 				"unable to set clock freq to %u\n",
-				sensor->platform_data->ext_clk);
+				sensor->hwcfg->ext_clk);
 			return rval;
 		}
 	}
 
-	if (gpio_is_valid(sensor->platform_data->xshutdown)) {
+	if (gpio_is_valid(sensor->hwcfg->xshutdown)) {
 		rval = devm_gpio_request_one(
-			&client->dev, sensor->platform_data->xshutdown, 0,
+			&client->dev, sensor->hwcfg->xshutdown, 0,
 			"SMIA++ xshutdown");
 		if (rval < 0) {
 			dev_err(&client->dev,
 				"unable to acquire reset gpio %d\n",
-				sensor->platform_data->xshutdown);
+				sensor->hwcfg->xshutdown);
 			return rval;
 		}
 	}
@@ -2612,7 +2612,7 @@ static int smiapp_init(struct smiapp_sensor *sensor)
 	 *
 	 * Rotation also changes the bayer pattern.
 	 */
-	if (sensor->platform_data->module_board_orient ==
+	if (sensor->hwcfg->module_board_orient ==
 	    SMIAPP_MODULE_BOARD_ORIENT_180)
 		sensor->hvflip_inv_mask = SMIAPP_IMAGE_ORIENTATION_HFLIP |
 					  SMIAPP_IMAGE_ORIENTATION_VFLIP;
@@ -2661,9 +2661,9 @@ static int smiapp_init(struct smiapp_sensor *sensor)
 	/* SMIA++ NVM initialization - it will be read from the sensor
 	 * when it is first requested by userspace.
 	 */
-	if (sensor->minfo.smiapp_version && sensor->platform_data->nvm_size) {
+	if (sensor->minfo.smiapp_version && sensor->hwcfg->nvm_size) {
 		sensor->nvm = devm_kzalloc(&client->dev,
-				sensor->platform_data->nvm_size, GFP_KERNEL);
+				sensor->hwcfg->nvm_size, GFP_KERNEL);
 		if (sensor->nvm == NULL) {
 			dev_err(&client->dev, "nvm buf allocation failed\n");
 			rval = -ENOMEM;
@@ -2706,8 +2706,8 @@ static int smiapp_init(struct smiapp_sensor *sensor)
 
 	/* prepare PLL configuration input values */
 	pll->bus_type = SMIAPP_PLL_BUS_TYPE_CSI2;
-	pll->csi2.lanes = sensor->platform_data->lanes;
-	pll->ext_clk_freq_hz = sensor->platform_data->ext_clk;
+	pll->csi2.lanes = sensor->hwcfg->lanes;
+	pll->ext_clk_freq_hz = sensor->hwcfg->ext_clk;
 	pll->scale_n = sensor->limits[SMIAPP_LIMIT_SCALER_N_MIN];
 	/* Profile 0 sensors have no separate OP clock branch. */
 	if (sensor->minfo.smiapp_profile == SMIAPP_PROFILE_0)
@@ -2984,9 +2984,9 @@ static int smiapp_resume(struct device *dev)
 
 #endif /* CONFIG_PM */
 
-static struct smiapp_platform_data *smiapp_get_pdata(struct device *dev)
+static struct smiapp_hwconfig *smiapp_get_hwconfig(struct device *dev)
 {
-	struct smiapp_platform_data *pdata;
+	struct smiapp_hwconfig *hwcfg;
 	struct v4l2_of_endpoint *bus_cfg;
 	struct device_node *ep;
 	int i;
@@ -3003,58 +3003,58 @@ static struct smiapp_platform_data *smiapp_get_pdata(struct device *dev)
 	if (IS_ERR(bus_cfg))
 		goto out_err;
 
-	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
-	if (!pdata)
+	hwcfg = devm_kzalloc(dev, sizeof(*hwcfg), GFP_KERNEL);
+	if (!hwcfg)
 		goto out_err;
 
 	switch (bus_cfg->bus_type) {
 	case V4L2_MBUS_CSI2:
-		pdata->csi_signalling_mode = SMIAPP_CSI_SIGNALLING_MODE_CSI2;
+		hwcfg->csi_signalling_mode = SMIAPP_CSI_SIGNALLING_MODE_CSI2;
 		break;
 		/* FIXME: add CCP2 support. */
 	default:
 		goto out_err;
 	}
 
-	pdata->lanes = bus_cfg->bus.mipi_csi2.num_data_lanes;
-	dev_dbg(dev, "lanes %u\n", pdata->lanes);
+	hwcfg->lanes = bus_cfg->bus.mipi_csi2.num_data_lanes;
+	dev_dbg(dev, "lanes %u\n", hwcfg->lanes);
 
 	/* xshutdown GPIO is optional */
-	pdata->xshutdown = of_get_named_gpio(dev->of_node, "reset-gpios", 0);
+	hwcfg->xshutdown = of_get_named_gpio(dev->of_node, "reset-gpios", 0);
 
 	/* NVM size is not mandatory */
 	of_property_read_u32(dev->of_node, "nokia,nvm-size",
-				    &pdata->nvm_size);
+				    &hwcfg->nvm_size);
 
 	rval = of_property_read_u32(dev->of_node, "clock-frequency",
-				    &pdata->ext_clk);
+				    &hwcfg->ext_clk);
 	if (rval) {
 		dev_warn(dev, "can't get clock-frequency\n");
 		goto out_err;
 	}
 
-	dev_dbg(dev, "reset %d, nvm %d, clk %d, csi %d\n", pdata->xshutdown,
-		pdata->nvm_size, pdata->ext_clk, pdata->csi_signalling_mode);
+	dev_dbg(dev, "reset %d, nvm %d, clk %d, csi %d\n", hwcfg->xshutdown,
+		hwcfg->nvm_size, hwcfg->ext_clk, hwcfg->csi_signalling_mode);
 
 	if (!bus_cfg->nr_of_link_frequencies) {
 		dev_warn(dev, "no link frequencies defined\n");
 		goto out_err;
 	}
 
-	pdata->op_sys_clock = devm_kcalloc(
+	hwcfg->op_sys_clock = devm_kcalloc(
 		dev, bus_cfg->nr_of_link_frequencies + 1 /* guardian */,
-		sizeof(*pdata->op_sys_clock), GFP_KERNEL);
-	if (!pdata->op_sys_clock)
+		sizeof(*hwcfg->op_sys_clock), GFP_KERNEL);
+	if (!hwcfg->op_sys_clock)
 		goto out_err;
 
 	for (i = 0; i < bus_cfg->nr_of_link_frequencies; i++) {
-		pdata->op_sys_clock[i] = bus_cfg->link_frequencies[i];
-		dev_dbg(dev, "freq %d: %lld\n", i, pdata->op_sys_clock[i]);
+		hwcfg->op_sys_clock[i] = bus_cfg->link_frequencies[i];
+		dev_dbg(dev, "freq %d: %lld\n", i, hwcfg->op_sys_clock[i]);
 	}
 
 	v4l2_of_free_endpoint(bus_cfg);
 	of_node_put(ep);
-	return pdata;
+	return hwcfg;
 
 out_err:
 	v4l2_of_free_endpoint(bus_cfg);
@@ -3066,17 +3066,17 @@ static int smiapp_probe(struct i2c_client *client,
 			const struct i2c_device_id *devid)
 {
 	struct smiapp_sensor *sensor;
-	struct smiapp_platform_data *pdata = smiapp_get_pdata(&client->dev);
+	struct smiapp_hwconfig *hwcfg = smiapp_get_hwconfig(&client->dev);
 	int rval;
 
-	if (pdata == NULL)
+	if (hwcfg == NULL)
 		return -ENODEV;
 
 	sensor = devm_kzalloc(&client->dev, sizeof(*sensor), GFP_KERNEL);
 	if (sensor == NULL)
 		return -ENOMEM;
 
-	sensor->platform_data = pdata;
+	sensor->hwcfg = hwcfg;
 	mutex_init(&sensor->mutex);
 	mutex_init(&sensor->power_mutex);
 	sensor->src = &sensor->ssds[sensor->ssds_used];
@@ -3119,10 +3119,10 @@ static int smiapp_remove(struct i2c_client *client)
 	v4l2_async_unregister_subdev(subdev);
 
 	if (sensor->power_count) {
-		if (gpio_is_valid(sensor->platform_data->xshutdown))
-			gpio_set_value(sensor->platform_data->xshutdown, 0);
-		if (sensor->platform_data->set_xclk)
-			sensor->platform_data->set_xclk(&sensor->src->sd, 0);
+		if (gpio_is_valid(sensor->hwcfg->xshutdown))
+			gpio_set_value(sensor->hwcfg->xshutdown, 0);
+		if (sensor->hwcfg->set_xclk)
+			sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
 		else
 			clk_disable_unprepare(sensor->ext_clk);
 		sensor->power_count = 0;
diff --git a/drivers/media/i2c/smiapp/smiapp-quirk.c b/drivers/media/i2c/smiapp/smiapp-quirk.c
index abf9ea7..d7e22bc 100644
--- a/drivers/media/i2c/smiapp/smiapp-quirk.c
+++ b/drivers/media/i2c/smiapp/smiapp-quirk.c
@@ -178,13 +178,13 @@ static int jt8ev1_post_poweron(struct smiapp_sensor *sensor)
 	if (rval < 0)
 		return rval;
 
-	switch (sensor->platform_data->ext_clk) {
+	switch (sensor->hwcfg->ext_clk) {
 	case 9600000:
 		return smiapp_write_8s(sensor, regs_96,
 				       ARRAY_SIZE(regs_96));
 	default:
 		dev_warn(&client->dev, "no MSRs for %d Hz ext_clk\n",
-			 sensor->platform_data->ext_clk);
+			 sensor->hwcfg->ext_clk);
 		return 0;
 	}
 }
diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h
index 2174f89..6ff095a 100644
--- a/drivers/media/i2c/smiapp/smiapp.h
+++ b/drivers/media/i2c/smiapp/smiapp.h
@@ -197,7 +197,7 @@ struct smiapp_sensor {
 	struct smiapp_subdev *binner;
 	struct smiapp_subdev *scaler;
 	struct smiapp_subdev *pixel_array;
-	struct smiapp_platform_data *platform_data;
+	struct smiapp_hwconfig *hwcfg;
 	struct regulator *vana;
 	struct clk *ext_clk;
 	u32 limits[SMIAPP_LIMIT_LAST];
diff --git a/include/media/i2c/smiapp.h b/include/media/i2c/smiapp.h
index 029142d..a4a1b51 100644
--- a/include/media/i2c/smiapp.h
+++ b/include/media/i2c/smiapp.h
@@ -57,7 +57,7 @@ struct smiapp_flash_strobe_parms {
 	u8 trigger;
 };
 
-struct smiapp_platform_data {
+struct smiapp_hwconfig {
 	/*
 	 * Change the cci address if i2c_addr_alt is set.
 	 * Both default and alternate cci addr need to be present
-- 
2.7.4

[PATCH 3/5] smiapp: Return -EPROBE_DEFER if the clock cannot be obtained

[Sakari Ailus]

The clock may be provided by a driver which is yet to probe.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 92a6859..aaf5299 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2558,7 +2558,7 @@ static int smiapp_init(struct smiapp_sensor *sensor)
 		sensor->ext_clk = devm_clk_get(&client->dev, NULL);
 		if (IS_ERR(sensor->ext_clk)) {
 			dev_err(&client->dev, "could not get clock\n");
-			return PTR_ERR(sensor->ext_clk);
+			return -EPROBE_DEFER;
 		}
 
 		rval = clk_set_rate(sensor->ext_clk,
-- 
2.7.4

Re: [PATCH 3/5] smiapp: Return -EPROBE_DEFER if the clock cannot be obtained

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1312 bytes --]

Hi,

On Wed, Aug 31, 2016 at 10:42:03AM +0300, Sakari Ailus wrote:
> The clock may be provided by a driver which is yet to probe.

This probably fixes N950 with built-in drivers, where I could see
smiapp fails due to missing clk. I have not yet further anaylsed it,
since more important parts like display output are also not yet
working.

> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/i2c/smiapp/smiapp-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
> index 92a6859..aaf5299 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -2558,7 +2558,7 @@ static int smiapp_init(struct smiapp_sensor *sensor)
>  		sensor->ext_clk = devm_clk_get(&client->dev, NULL);
>  		if (IS_ERR(sensor->ext_clk)) {
>  			dev_err(&client->dev, "could not get clock\n");
> -			return PTR_ERR(sensor->ext_clk);
> +			return -EPROBE_DEFER;
>  		}
>  
>  		rval = clk_set_rate(sensor->ext_clk,

With the error being rewritten to EPROBE_DEFER, the actual error
number should be part of the error message:

dev_err(&client->dev, "could not get clock (%d)\n", PTR_ERR(sensor->ext_clk));

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v1.1 3/5] smiapp: Return -EPROBE_DEFER if the clock cannot be obtained

[Sakari Ailus]

The clock may be provided by a driver which is yet to probe. Print the
actual error code as well.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
since v1:
- Add printing of the original error code

 drivers/media/i2c/smiapp/smiapp-core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 92a6859..103e335 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2557,8 +2557,9 @@ static int smiapp_init(struct smiapp_sensor *sensor)
 	if (!sensor->hwcfg->set_xclk) {
 		sensor->ext_clk = devm_clk_get(&client->dev, NULL);
 		if (IS_ERR(sensor->ext_clk)) {
-			dev_err(&client->dev, "could not get clock\n");
-			return PTR_ERR(sensor->ext_clk);
+			dev_err(&client->dev, "could not get clock (%ld)\n",
+				PTR_ERR(sensor->ext_clk));
+			return -EPROBE_DEFER;
 		}
 
 		rval = clk_set_rate(sensor->ext_clk,
-- 
2.7.4

Re: [PATCH v1.1 3/5] smiapp: Return -EPROBE_DEFER if the clock cannot be obtained

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 359 bytes --]

Hi,

On Wed, Aug 31, 2016 at 03:57:57PM +0300, Sakari Ailus wrote:
> The clock may be provided by a driver which is yet to probe. Print the
> actual error code as well.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>
> ---
> since v1:
> - Add printing of the original error code

Reviewed-By: Sebastian Reichel <sre@kernel.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v1.1 5/5] smiapp: Switch to gpiod API for GPIO control

[Sakari Ailus]

Switch from the old gpio API to the new descriptor based gpiod API.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
- Remove xshutdown field in smiapp_hwconfig, and SMIAPP_NO_XSHUTDOWN macro

 drivers/media/i2c/smiapp/smiapp-core.c | 36 +++++++++++-----------------------
 drivers/media/i2c/smiapp/smiapp.h      |  1 +
 include/media/i2c/smiapp.h             |  3 ---
 3 files changed, 12 insertions(+), 28 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 103e335..1ecc9a4 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -24,8 +24,8 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/module.h>
-#include <linux/of_gpio.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/smiapp.h>
@@ -1212,8 +1212,7 @@ static int smiapp_power_on(struct smiapp_sensor *sensor)
 	}
 	usleep_range(1000, 1000);
 
-	if (gpio_is_valid(sensor->hwcfg->xshutdown))
-		gpio_set_value(sensor->hwcfg->xshutdown, 1);
+	gpiod_set_value(sensor->xshutdown, 1);
 
 	sleep = SMIAPP_RESET_DELAY(sensor->hwcfg->ext_clk);
 	usleep_range(sleep, sleep);
@@ -1322,8 +1321,7 @@ static int smiapp_power_on(struct smiapp_sensor *sensor)
 	return 0;
 
 out_cci_addr_fail:
-	if (gpio_is_valid(sensor->hwcfg->xshutdown))
-		gpio_set_value(sensor->hwcfg->xshutdown, 0);
+	gpiod_set_value(sensor->xshutdown, 0);
 	if (sensor->hwcfg->set_xclk)
 		sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
 	else
@@ -1348,8 +1346,7 @@ static void smiapp_power_off(struct smiapp_sensor *sensor)
 			     SMIAPP_REG_U8_SOFTWARE_RESET,
 			     SMIAPP_SOFTWARE_RESET);
 
-	if (gpio_is_valid(sensor->hwcfg->xshutdown))
-		gpio_set_value(sensor->hwcfg->xshutdown, 0);
+	gpiod_set_value(sensor->xshutdown, 0);
 	if (sensor->hwcfg->set_xclk)
 		sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
 	else
@@ -2572,17 +2569,10 @@ static int smiapp_init(struct smiapp_sensor *sensor)
 		}
 	}
 
-	if (gpio_is_valid(sensor->hwcfg->xshutdown)) {
-		rval = devm_gpio_request_one(
-			&client->dev, sensor->hwcfg->xshutdown, 0,
-			"SMIA++ xshutdown");
-		if (rval < 0) {
-			dev_err(&client->dev,
-				"unable to acquire reset gpio %d\n",
-				sensor->hwcfg->xshutdown);
-			return rval;
-		}
-	}
+	sensor->xshutdown = devm_gpiod_get_optional(&client->dev, "xshutdown",
+						    GPIOD_OUT_LOW);
+	if (!sensor->xshutdown)
+		dev_dbg(&client->dev, "no xshutdown GPIO available\n");
 
 	rval = smiapp_power_on(sensor);
 	if (rval)
@@ -3020,9 +3010,6 @@ static struct smiapp_hwconfig *smiapp_get_hwconfig(struct device *dev)
 	hwcfg->lanes = bus_cfg->bus.mipi_csi2.num_data_lanes;
 	dev_dbg(dev, "lanes %u\n", hwcfg->lanes);
 
-	/* xshutdown GPIO is optional */
-	hwcfg->xshutdown = of_get_named_gpio(dev->of_node, "reset-gpios", 0);
-
 	/* NVM size is not mandatory */
 	of_property_read_u32(dev->of_node, "nokia,nvm-size",
 				    &hwcfg->nvm_size);
@@ -3034,8 +3021,8 @@ static struct smiapp_hwconfig *smiapp_get_hwconfig(struct device *dev)
 		goto out_err;
 	}
 
-	dev_dbg(dev, "reset %d, nvm %d, clk %d, csi %d\n", hwcfg->xshutdown,
-		hwcfg->nvm_size, hwcfg->ext_clk, hwcfg->csi_signalling_mode);
+	dev_dbg(dev, "nvm %d, clk %d, csi %d\n", hwcfg->nvm_size,
+		hwcfg->ext_clk, hwcfg->csi_signalling_mode);
 
 	if (!bus_cfg->nr_of_link_frequencies) {
 		dev_warn(dev, "no link frequencies defined\n");
@@ -3120,8 +3107,7 @@ static int smiapp_remove(struct i2c_client *client)
 	v4l2_async_unregister_subdev(subdev);
 
 	if (sensor->power_count) {
-		if (gpio_is_valid(sensor->hwcfg->xshutdown))
-			gpio_set_value(sensor->hwcfg->xshutdown, 0);
+		gpiod_set_value(sensor->xshutdown, 0);
 		if (sensor->hwcfg->set_xclk)
 			sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
 		else
diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h
index 6ff095a..c504bd8 100644
--- a/drivers/media/i2c/smiapp/smiapp.h
+++ b/drivers/media/i2c/smiapp/smiapp.h
@@ -200,6 +200,7 @@ struct smiapp_sensor {
 	struct smiapp_hwconfig *hwcfg;
 	struct regulator *vana;
 	struct clk *ext_clk;
+	struct gpio_desc *xshutdown;
 	u32 limits[SMIAPP_LIMIT_LAST];
 	u8 nbinning_subtypes;
 	struct smiapp_binning_subtype binning_subtypes[SMIAPP_BINNING_SUBTYPES];
diff --git a/include/media/i2c/smiapp.h b/include/media/i2c/smiapp.h
index a4a1b51..eacc3f4 100644
--- a/include/media/i2c/smiapp.h
+++ b/include/media/i2c/smiapp.h
@@ -36,8 +36,6 @@
 #define SMIAPP_CSI_SIGNALLING_MODE_CCP2_DATA_STROBE	1
 #define SMIAPP_CSI_SIGNALLING_MODE_CSI2			2
 
-#define SMIAPP_NO_XSHUTDOWN	-1
-
 /*
  * Sometimes due to board layout considerations the camera module can be
  * mounted rotated. The typical rotation used is 180 degrees which can be
@@ -77,7 +75,6 @@ struct smiapp_hwconfig {
 	struct smiapp_flash_strobe_parms *strobe_setup;
 
 	int (*set_xclk)(struct v4l2_subdev *sd, int hz);
-	int32_t xshutdown;		/* gpio or SMIAPP_NO_XSHUTDOWN */
 };
 
 #endif /* __SMIAPP_H_  */
-- 
2.7.4

Re: [PATCH v1.1 5/5] smiapp: Switch to gpiod API for GPIO control

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1179 bytes --]

Hi,

On Wed, Aug 31, 2016 at 04:00:56PM +0300, Sakari Ailus wrote:
> Switch from the old gpio API to the new descriptor based gpiod API.
>
> [...]
>
> @@ -2572,17 +2569,10 @@ static int smiapp_init(struct smiapp_sensor *sensor)
>  		}
>  	}
>  
> -	if (gpio_is_valid(sensor->hwcfg->xshutdown)) {
> -		rval = devm_gpio_request_one(
> -			&client->dev, sensor->hwcfg->xshutdown, 0,
> -			"SMIA++ xshutdown");
> -		if (rval < 0) {
> -			dev_err(&client->dev,
> -				"unable to acquire reset gpio %d\n",
> -				sensor->hwcfg->xshutdown);
> -			return rval;
> -		}
> -	}
> +	sensor->xshutdown = devm_gpiod_get_optional(&client->dev, "xshutdown",
> +						    GPIOD_OUT_LOW);
> +	if (!sensor->xshutdown)
> +		dev_dbg(&client->dev, "no xshutdown GPIO available\n");

devm_gpiod_get_optional may return an error pointer, e.g. for
-EPROBE_DEFER, so you should add:

if (IS_ERR(sensor->xshutdown)) {
    rval = PTR_ERR(sensor->xshutdown);
    dev_err(&client->dev, "Could not get gpio (%ld)\n", rval);
    return rval;
}

> [...]

Otherwise the patch looks fine, so with this fixed:

Reviewed-By: Sebastian Reichel <sre@kernel.org>

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v1.2 5/5] smiapp: Switch to gpiod API for GPIO control

[Sakari Ailus]

Switch from the old gpio API to the new descriptor based gpiod API.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
since v1.1:
- Return the error if devm_gpiod_get_optional() fails. NULL is returned if
  there's no GPIO defined.
- No debug print is performed on lack of the GPIO. The GPIO framework
  already does this.

 drivers/media/i2c/smiapp/smiapp-core.c | 36 +++++++++++-----------------------
 drivers/media/i2c/smiapp/smiapp.h      |  1 +
 include/media/i2c/smiapp.h             |  3 ---
 3 files changed, 12 insertions(+), 28 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 103e335..061f44a 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -24,8 +24,8 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/module.h>
-#include <linux/of_gpio.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/smiapp.h>
@@ -1212,8 +1212,7 @@ static int smiapp_power_on(struct smiapp_sensor *sensor)
 	}
 	usleep_range(1000, 1000);
 
-	if (gpio_is_valid(sensor->hwcfg->xshutdown))
-		gpio_set_value(sensor->hwcfg->xshutdown, 1);
+	gpiod_set_value(sensor->xshutdown, 1);
 
 	sleep = SMIAPP_RESET_DELAY(sensor->hwcfg->ext_clk);
 	usleep_range(sleep, sleep);
@@ -1322,8 +1321,7 @@ static int smiapp_power_on(struct smiapp_sensor *sensor)
 	return 0;
 
 out_cci_addr_fail:
-	if (gpio_is_valid(sensor->hwcfg->xshutdown))
-		gpio_set_value(sensor->hwcfg->xshutdown, 0);
+	gpiod_set_value(sensor->xshutdown, 0);
 	if (sensor->hwcfg->set_xclk)
 		sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
 	else
@@ -1348,8 +1346,7 @@ static void smiapp_power_off(struct smiapp_sensor *sensor)
 			     SMIAPP_REG_U8_SOFTWARE_RESET,
 			     SMIAPP_SOFTWARE_RESET);
 
-	if (gpio_is_valid(sensor->hwcfg->xshutdown))
-		gpio_set_value(sensor->hwcfg->xshutdown, 0);
+	gpiod_set_value(sensor->xshutdown, 0);
 	if (sensor->hwcfg->set_xclk)
 		sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
 	else
@@ -2572,17 +2569,10 @@ static int smiapp_init(struct smiapp_sensor *sensor)
 		}
 	}
 
-	if (gpio_is_valid(sensor->hwcfg->xshutdown)) {
-		rval = devm_gpio_request_one(
-			&client->dev, sensor->hwcfg->xshutdown, 0,
-			"SMIA++ xshutdown");
-		if (rval < 0) {
-			dev_err(&client->dev,
-				"unable to acquire reset gpio %d\n",
-				sensor->hwcfg->xshutdown);
-			return rval;
-		}
-	}
+	sensor->xshutdown = devm_gpiod_get_optional(&client->dev, "xshutdown",
+						    GPIOD_OUT_LOW);
+	if (IS_ERR(sensor->xshutdown))
+		return PTR_ERR(sensor->xshutdown);
 
 	rval = smiapp_power_on(sensor);
 	if (rval)
@@ -3020,9 +3010,6 @@ static struct smiapp_hwconfig *smiapp_get_hwconfig(struct device *dev)
 	hwcfg->lanes = bus_cfg->bus.mipi_csi2.num_data_lanes;
 	dev_dbg(dev, "lanes %u\n", hwcfg->lanes);
 
-	/* xshutdown GPIO is optional */
-	hwcfg->xshutdown = of_get_named_gpio(dev->of_node, "reset-gpios", 0);
-
 	/* NVM size is not mandatory */
 	of_property_read_u32(dev->of_node, "nokia,nvm-size",
 				    &hwcfg->nvm_size);
@@ -3034,8 +3021,8 @@ static struct smiapp_hwconfig *smiapp_get_hwconfig(struct device *dev)
 		goto out_err;
 	}
 
-	dev_dbg(dev, "reset %d, nvm %d, clk %d, csi %d\n", hwcfg->xshutdown,
-		hwcfg->nvm_size, hwcfg->ext_clk, hwcfg->csi_signalling_mode);
+	dev_dbg(dev, "nvm %d, clk %d, csi %d\n", hwcfg->nvm_size,
+		hwcfg->ext_clk, hwcfg->csi_signalling_mode);
 
 	if (!bus_cfg->nr_of_link_frequencies) {
 		dev_warn(dev, "no link frequencies defined\n");
@@ -3120,8 +3107,7 @@ static int smiapp_remove(struct i2c_client *client)
 	v4l2_async_unregister_subdev(subdev);
 
 	if (sensor->power_count) {
-		if (gpio_is_valid(sensor->hwcfg->xshutdown))
-			gpio_set_value(sensor->hwcfg->xshutdown, 0);
+		gpiod_set_value(sensor->xshutdown, 0);
 		if (sensor->hwcfg->set_xclk)
 			sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
 		else
diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h
index 6ff095a..c504bd8 100644
--- a/drivers/media/i2c/smiapp/smiapp.h
+++ b/drivers/media/i2c/smiapp/smiapp.h
@@ -200,6 +200,7 @@ struct smiapp_sensor {
 	struct smiapp_hwconfig *hwcfg;
 	struct regulator *vana;
 	struct clk *ext_clk;
+	struct gpio_desc *xshutdown;
 	u32 limits[SMIAPP_LIMIT_LAST];
 	u8 nbinning_subtypes;
 	struct smiapp_binning_subtype binning_subtypes[SMIAPP_BINNING_SUBTYPES];
diff --git a/include/media/i2c/smiapp.h b/include/media/i2c/smiapp.h
index a4a1b51..eacc3f4 100644
--- a/include/media/i2c/smiapp.h
+++ b/include/media/i2c/smiapp.h
@@ -36,8 +36,6 @@
 #define SMIAPP_CSI_SIGNALLING_MODE_CCP2_DATA_STROBE	1
 #define SMIAPP_CSI_SIGNALLING_MODE_CSI2			2
 
-#define SMIAPP_NO_XSHUTDOWN	-1
-
 /*
  * Sometimes due to board layout considerations the camera module can be
  * mounted rotated. The typical rotation used is 180 degrees which can be
@@ -77,7 +75,6 @@ struct smiapp_hwconfig {
 	struct smiapp_flash_strobe_parms *strobe_setup;
 
 	int (*set_xclk)(struct v4l2_subdev *sd, int hz);
-	int32_t xshutdown;		/* gpio or SMIAPP_NO_XSHUTDOWN */
 };
 
 #endif /* __SMIAPP_H_  */
-- 
2.7.4

Re: [PATCH v1.2 5/5] smiapp: Switch to gpiod API for GPIO control

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 745 bytes --]

Hi,

On Thu, Sep 01, 2016 at 09:48:28AM +0300, Sakari Ailus wrote:
> Switch from the old gpio API to the new descriptor based gpiod API.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> since v1.1:
> - Return the error if devm_gpiod_get_optional() fails. NULL is returned if
>   there's no GPIO defined.
> - No debug print is performed on lack of the GPIO. The GPIO framework
>   already does this.
> 
>  drivers/media/i2c/smiapp/smiapp-core.c | 36 +++++++++++-----------------------
>  drivers/media/i2c/smiapp/smiapp.h      |  1 +
>  include/media/i2c/smiapp.h             |  3 ---
>  3 files changed, 12 insertions(+), 28 deletions(-)

Reviewed-By: Sebastian Reichel <sre@kernel.org>

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 4/5] smiapp: Constify the regs argument to smiapp_write_8s()

[Sakari Ailus]

The data may now be const as well.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-quirk.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-quirk.c b/drivers/media/i2c/smiapp/smiapp-quirk.c
index d7e22bc..cb128ea 100644
--- a/drivers/media/i2c/smiapp/smiapp-quirk.c
+++ b/drivers/media/i2c/smiapp/smiapp-quirk.c
@@ -26,7 +26,7 @@ static int smiapp_write_8(struct smiapp_sensor *sensor, u16 reg, u8 val)
 }
 
 static int smiapp_write_8s(struct smiapp_sensor *sensor,
-			   struct smiapp_reg_8 *regs, int len)
+			   const struct smiapp_reg_8 *regs, int len)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
 	int rval;
@@ -71,7 +71,7 @@ static int jt8ew9_limits(struct smiapp_sensor *sensor)
 
 static int jt8ew9_post_poweron(struct smiapp_sensor *sensor)
 {
-	struct smiapp_reg_8 regs[] = {
+	const struct smiapp_reg_8 regs[] = {
 		{ 0x30a3, 0xd8 }, /* Output port control : LVDS ports only */
 		{ 0x30ae, 0x00 }, /* 0x0307 pll_multiplier maximum value on PLL input 9.6MHz ( 19.2MHz is divided on pre_pll_div) */
 		{ 0x30af, 0xd0 }, /* 0x0307 pll_multiplier maximum value on PLL input 9.6MHz ( 19.2MHz is divided on pre_pll_div) */
@@ -115,7 +115,7 @@ const struct smiapp_quirk smiapp_jt8ew9_quirk = {
 static int imx125es_post_poweron(struct smiapp_sensor *sensor)
 {
 	/* Taken from v02. No idea what the other two are. */
-	struct smiapp_reg_8 regs[] = {
+	const struct smiapp_reg_8 regs[] = {
 		/*
 		 * 0x3302: clk during frame blanking:
 		 * 0x00 - HS mode, 0x01 - LP11
@@ -145,8 +145,7 @@ static int jt8ev1_post_poweron(struct smiapp_sensor *sensor)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
 	int rval;
-
-	struct smiapp_reg_8 regs[] = {
+	const struct smiapp_reg_8 regs[] = {
 		{ 0x3031, 0xcd }, /* For digital binning (EQ_MONI) */
 		{ 0x30a3, 0xd0 }, /* FLASH STROBE enable */
 		{ 0x3237, 0x00 }, /* For control of pulse timing for ADC */
@@ -167,8 +166,7 @@ static int jt8ev1_post_poweron(struct smiapp_sensor *sensor)
 		{ 0x33cf, 0xec }, /* For Black sun */
 		{ 0x3328, 0x80 }, /* Ugh. No idea what's this. */
 	};
-
-	struct smiapp_reg_8 regs_96[] = {
+	const struct smiapp_reg_8 regs_96[] = {
 		{ 0x30ae, 0x00 }, /* For control of ADC clock */
 		{ 0x30af, 0xd0 },
 		{ 0x30b0, 0x01 },
-- 
2.7.4

[PATCH 5/5] smiapp: Switch to gpiod API for GPIO control

[Sakari Ailus]

Switch from the old gpio API to the new descriptor based gpiod API.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c | 28 +++++++++++++++-------------
 drivers/media/i2c/smiapp/smiapp.h      |  1 +
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index aaf5299..d07e060 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -24,8 +24,8 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/module.h>
-#include <linux/of_gpio.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/smiapp.h>
@@ -1212,8 +1212,7 @@ static int smiapp_power_on(struct smiapp_sensor *sensor)
 	}
 	usleep_range(1000, 1000);
 
-	if (gpio_is_valid(sensor->hwcfg->xshutdown))
-		gpio_set_value(sensor->hwcfg->xshutdown, 1);
+	gpiod_set_value(sensor->xshutdown, 1);
 
 	sleep = SMIAPP_RESET_DELAY(sensor->hwcfg->ext_clk);
 	usleep_range(sleep, sleep);
@@ -1322,8 +1321,7 @@ static int smiapp_power_on(struct smiapp_sensor *sensor)
 	return 0;
 
 out_cci_addr_fail:
-	if (gpio_is_valid(sensor->hwcfg->xshutdown))
-		gpio_set_value(sensor->hwcfg->xshutdown, 0);
+	gpiod_set_value(sensor->xshutdown, 0);
 	if (sensor->hwcfg->set_xclk)
 		sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
 	else
@@ -1348,8 +1346,7 @@ static void smiapp_power_off(struct smiapp_sensor *sensor)
 			     SMIAPP_REG_U8_SOFTWARE_RESET,
 			     SMIAPP_SOFTWARE_RESET);
 
-	if (gpio_is_valid(sensor->hwcfg->xshutdown))
-		gpio_set_value(sensor->hwcfg->xshutdown, 0);
+	gpiod_set_value(sensor->xshutdown, 0);
 	if (sensor->hwcfg->set_xclk)
 		sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
 	else
@@ -2571,7 +2568,11 @@ static int smiapp_init(struct smiapp_sensor *sensor)
 		}
 	}
 
-	if (gpio_is_valid(sensor->hwcfg->xshutdown)) {
+	if (client->dev.of_node) {
+		sensor->xshutdown =
+			devm_gpiod_get_optional(&client->dev, "xshutdown",
+						GPIOD_OUT_LOW);
+	} else if (gpio_is_valid(sensor->hwcfg->xshutdown)) {
 		rval = devm_gpio_request_one(
 			&client->dev, sensor->hwcfg->xshutdown, 0,
 			"SMIA++ xshutdown");
@@ -2581,8 +2582,13 @@ static int smiapp_init(struct smiapp_sensor *sensor)
 				sensor->hwcfg->xshutdown);
 			return rval;
 		}
+
+		sensor->xshutdown = gpio_to_desc(sensor->hwcfg->xshutdown);
 	}
 
+	if (!sensor->xshutdown)
+		dev_dbg(&client->dev, "no xshutdown GPIO available\n");
+
 	rval = smiapp_power_on(sensor);
 	if (rval)
 		return -ENODEV;
@@ -3019,9 +3025,6 @@ static struct smiapp_hwconfig *smiapp_get_hwconfig(struct device *dev)
 	hwcfg->lanes = bus_cfg->bus.mipi_csi2.num_data_lanes;
 	dev_dbg(dev, "lanes %u\n", hwcfg->lanes);
 
-	/* xshutdown GPIO is optional */
-	hwcfg->xshutdown = of_get_named_gpio(dev->of_node, "reset-gpios", 0);
-
 	/* NVM size is not mandatory */
 	of_property_read_u32(dev->of_node, "nokia,nvm-size",
 				    &hwcfg->nvm_size);
@@ -3119,8 +3122,7 @@ static int smiapp_remove(struct i2c_client *client)
 	v4l2_async_unregister_subdev(subdev);
 
 	if (sensor->power_count) {
-		if (gpio_is_valid(sensor->hwcfg->xshutdown))
-			gpio_set_value(sensor->hwcfg->xshutdown, 0);
+		gpiod_set_value(sensor->xshutdown, 0);
 		if (sensor->hwcfg->set_xclk)
 			sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
 		else
diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h
index 6ff095a..c504bd8 100644
--- a/drivers/media/i2c/smiapp/smiapp.h
+++ b/drivers/media/i2c/smiapp/smiapp.h
@@ -200,6 +200,7 @@ struct smiapp_sensor {
 	struct smiapp_hwconfig *hwcfg;
 	struct regulator *vana;
 	struct clk *ext_clk;
+	struct gpio_desc *xshutdown;
 	u32 limits[SMIAPP_LIMIT_LAST];
 	u8 nbinning_subtypes;
 	struct smiapp_binning_subtype binning_subtypes[SMIAPP_BINNING_SUBTYPES];
-- 
2.7.4

Re: [PATCH 5/5] smiapp: Switch to gpiod API for GPIO control

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]

Hi Sakari,

On Wed, Aug 31, 2016 at 10:42:05AM +0300, Sakari Ailus wrote:
> -	if (gpio_is_valid(sensor->hwcfg->xshutdown)) {
> +	if (client->dev.of_node) {
> +		sensor->xshutdown =
> +			devm_gpiod_get_optional(&client->dev, "xshutdown",
> +						GPIOD_OUT_LOW);
> +	} else if (gpio_is_valid(sensor->hwcfg->xshutdown)) {
>  		rval = devm_gpio_request_one(
>  			&client->dev, sensor->hwcfg->xshutdown, 0,
>  			"SMIA++ xshutdown");
> @@ -2581,8 +2582,13 @@ static int smiapp_init(struct smiapp_sensor *sensor)
>  				sensor->hwcfg->xshutdown);
>  			return rval;
>  		}
> +
> +		sensor->xshutdown = gpio_to_desc(sensor->hwcfg->xshutdown);
>  	}

You can drop the devm_gpio_request_one() part and xshutdown from
smiapp_platform_data. The gpiod consumer interface can also be
used with data provided from boardfiles as documented in
Documentation/gpio/board.txt, section "Platform Data". It basically
works like assigning regulators to devices from platform data.

You will obviously have to change every platform_device users of
smiapp, but it looks like upstream has none:

~/linux/arch $ git grep smiapp_platform_data || echo "Not found"
Not found

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 5/5] smiapp: Switch to gpiod API for GPIO control

[Sakari Ailus]

Hi Sebastian,

Thanks for the review!

On 08/31/16 15:09, Sebastian Reichel wrote:
> Hi Sakari,
> 
> On Wed, Aug 31, 2016 at 10:42:05AM +0300, Sakari Ailus wrote:
>> -	if (gpio_is_valid(sensor->hwcfg->xshutdown)) {
>> +	if (client->dev.of_node) {
>> +		sensor->xshutdown =
>> +			devm_gpiod_get_optional(&client->dev, "xshutdown",
>> +						GPIOD_OUT_LOW);
>> +	} else if (gpio_is_valid(sensor->hwcfg->xshutdown)) {
>>  		rval = devm_gpio_request_one(
>>  			&client->dev, sensor->hwcfg->xshutdown, 0,
>>  			"SMIA++ xshutdown");
>> @@ -2581,8 +2582,13 @@ static int smiapp_init(struct smiapp_sensor *sensor)
>>  				sensor->hwcfg->xshutdown);
>>  			return rval;
>>  		}
>> +
>> +		sensor->xshutdown = gpio_to_desc(sensor->hwcfg->xshutdown);
>>  	}
> 
> You can drop the devm_gpio_request_one() part and xshutdown from
> smiapp_platform_data. The gpiod consumer interface can also be
> used with data provided from boardfiles as documented in
> Documentation/gpio/board.txt, section "Platform Data". It basically
> works like assigning regulators to devices from platform data.

Good point. I'll fix that.

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

Re: [PATCH 0/5] smiapp cleanups, retry probe if getting clock fails

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 294 bytes --]

Hi,

On Wed, Aug 31, 2016 at 10:42:00AM +0300, Sakari Ailus wrote:
> These patches contain cleanups for the smiapp driver and return
> -EPROBE_DEFER if getting the clock fails.

Apart from comments on patches 3 & 5 the patchset is

Reviewed-By: Sebastian Reichel <sre@kernel.org>

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v1.1 6/6] smiapp: Remove set_xclk() callback from hwconfig

[Sakari Ailus]

The clock framework is generally so well supported that there's no reason
to keep this one around.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/smiapp/smiapp-core.c | 49 ++++++++++++----------------------
 include/media/i2c/smiapp.h             |  2 --
 2 files changed, 17 insertions(+), 34 deletions(-)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 1ecc9a4..05ab0d0 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -1201,11 +1201,7 @@ static int smiapp_power_on(struct smiapp_sensor *sensor)
 	}
 	usleep_range(1000, 1000);
 
-	if (sensor->hwcfg->set_xclk)
-		rval = sensor->hwcfg->set_xclk(
-			&sensor->src->sd, sensor->hwcfg->ext_clk);
-	else
-		rval = clk_prepare_enable(sensor->ext_clk);
+	rval = clk_prepare_enable(sensor->ext_clk);
 	if (rval < 0) {
 		dev_dbg(&client->dev, "failed to enable xclk\n");
 		goto out_xclk_fail;
@@ -1322,10 +1318,7 @@ static int smiapp_power_on(struct smiapp_sensor *sensor)
 
 out_cci_addr_fail:
 	gpiod_set_value(sensor->xshutdown, 0);
-	if (sensor->hwcfg->set_xclk)
-		sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
-	else
-		clk_disable_unprepare(sensor->ext_clk);
+	clk_disable_unprepare(sensor->ext_clk);
 
 out_xclk_fail:
 	regulator_disable(sensor->vana);
@@ -1347,10 +1340,7 @@ static void smiapp_power_off(struct smiapp_sensor *sensor)
 			     SMIAPP_SOFTWARE_RESET);
 
 	gpiod_set_value(sensor->xshutdown, 0);
-	if (sensor->hwcfg->set_xclk)
-		sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
-	else
-		clk_disable_unprepare(sensor->ext_clk);
+	clk_disable_unprepare(sensor->ext_clk);
 	usleep_range(5000, 5000);
 	regulator_disable(sensor->vana);
 	sensor->streaming = false;
@@ -2551,22 +2541,20 @@ static int smiapp_init(struct smiapp_sensor *sensor)
 		return PTR_ERR(sensor->vana);
 	}
 
-	if (!sensor->hwcfg->set_xclk) {
-		sensor->ext_clk = devm_clk_get(&client->dev, NULL);
-		if (IS_ERR(sensor->ext_clk)) {
-			dev_err(&client->dev, "could not get clock (%ld)\n",
-				PTR_ERR(sensor->ext_clk));
-			return -EPROBE_DEFER;
-		}
+	sensor->ext_clk = devm_clk_get(&client->dev, NULL);
+	if (IS_ERR(sensor->ext_clk)) {
+		dev_err(&client->dev, "could not get clock (%ld)\n",
+			PTR_ERR(sensor->ext_clk));
+		return -EPROBE_DEFER;
+	}
 
-		rval = clk_set_rate(sensor->ext_clk,
-				    sensor->hwcfg->ext_clk);
-		if (rval < 0) {
-			dev_err(&client->dev,
-				"unable to set clock freq to %u\n",
-				sensor->hwcfg->ext_clk);
-			return rval;
-		}
+	rval = clk_set_rate(sensor->ext_clk,
+			    sensor->hwcfg->ext_clk);
+	if (rval < 0) {
+		dev_err(&client->dev,
+			"unable to set clock freq to %u\n",
+			sensor->hwcfg->ext_clk);
+		return rval;
 	}
 
 	sensor->xshutdown = devm_gpiod_get_optional(&client->dev, "xshutdown",
@@ -3108,10 +3096,7 @@ static int smiapp_remove(struct i2c_client *client)
 
 	if (sensor->power_count) {
 		gpiod_set_value(sensor->xshutdown, 0);
-		if (sensor->hwcfg->set_xclk)
-			sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
-		else
-			clk_disable_unprepare(sensor->ext_clk);
+		clk_disable_unprepare(sensor->ext_clk);
 		sensor->power_count = 0;
 	}
 
diff --git a/include/media/i2c/smiapp.h b/include/media/i2c/smiapp.h
index eacc3f4..635007e 100644
--- a/include/media/i2c/smiapp.h
+++ b/include/media/i2c/smiapp.h
@@ -73,8 +73,6 @@ struct smiapp_hwconfig {
 	enum smiapp_module_board_orient module_board_orient;
 
 	struct smiapp_flash_strobe_parms *strobe_setup;
-
-	int (*set_xclk)(struct v4l2_subdev *sd, int hz);
 };
 
 #endif /* __SMIAPP_H_  */
-- 
2.7.4

Re: [PATCH v1.1 6/6] smiapp: Remove set_xclk() callback from hwconfig

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]

Hi,

On Wed, Aug 31, 2016 at 04:01:37PM +0300, Sakari Ailus wrote:
> The clock framework is generally so well supported that there's no reason
> to keep this one around.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/i2c/smiapp/smiapp-core.c | 49 ++++++++++++----------------------
>  include/media/i2c/smiapp.h             |  2 --
>  2 files changed, 17 insertions(+), 34 deletions(-)

Reviewed-By: Sebastian Reichel <sre@kernel.org>

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]