[PATCH 00/24] Additional CCS feature support

[PATCH 00/24] Additional CCS feature support

[Sakari Ailus]

Hello everyone,

Here's a set of patches that turn the existing SMIA driver into a MIPI CCS
driver while maintaining SMIA support. A number of bugs in the existing
code are fixed in this set, too.

The changes at this point are primarily focused on dealing with new
mandatory driver features related to PLL configuration (as CCS allows for
much more variation there) and things such as integer conversion from
U16.U16 format instead of float. There are some other new features as well
such as digital gain and support for getting device specific analogue gain
coefficients.

A new feature in CCS is CCS static data which makes it possible to obtain
sensor's capabilities and limits from a file chosen based on sensor
identification. CCS static data is used also for storing MSR registers so
supporting new, CCS compliant devices requires no driver changes.

Also DT bindings are updated accordingly and converted to YAML format.

More information on MIPI CCS can be found here:

<URL:https://www.mipi.org/specifications/camera-command-set>

Comments are welcome.

since the big, big patchset (v2):

- Split into more easily reviewable chunks (this is the first of maybe
  three). The cover page describes the entire big set. This set contains
  support for additional configurability, more bugfixes, power sequence
  alignment with CCS and other tasks postponed in earlier patches.

- Added more documentation (driver, controls, PLL calculator).

- Switch from C99 integer types to Linux types.

- Fix file order in MAINTAINERS.

- Revert MODULE_LICENSE change to "GPL" -> "GPL v2"

- Support automatic PHY control, and default to it instead of using
  UI-based control.

- Remove V4L2_CID_CCS_LUMINANCE_SHADING_CORRECTION and
  V4L2_CID_CCS_SHADING_CORRECTION_CAPABILITY controls. This way we don't
  tell all the device capabilities to the user but it's unlikely anyone
  will really want to know this part precisely.

- Align with CCS power-on sequence.

- Don't do software reset if we have a reset GPIO.

- Bail out in probe if the external clock frequency is zero.

- Activate the luminance control only when shading correction is enabled.

Sakari Ailus (24):
  ccs: Add digital gain support
  ccs: Add support for old-style SMIA digital gain
  ccs: Remove analogue gain field
  ccs: Only add analogue gain control if the device supports it
  v4l: uapi: Add user control base for CCS controls
  Documentation: ccs: Add user documentation for the CCS driver
  v4l: uapi: ccs: Add controls for analogue gain constants
  ccs: Add support for analogue gain coefficient controls
  v4l: uapi: ccs: Add controls for CCS alternative analogue gain
  ccs: Add support for alternate analogue global gain
  ccs: Add debug prints for MSR registers
  v4l: uapi: ccs: Add CCS controls for shading correction
  ccs: Add shading correction and luminance correction level controls
  ccs: Get the endpoint by port rather than any next endpoint
  ccs: Don't change the I²C address just for software reset
  ccs: Only do software reset if we have no hardware reset
  ccs: Wait until software reset is done
  ccs: Hardware requires a delay after starting the clock of lifting
    reset
  ccs: Add a sanity check for external clock frequency
  ccs: Support and default to auto PHY control
  Documentation: Include CCS PLL calculator to CCS driver documentation
  ccs-pll: Switch from standard integer types to kernel ones
  ccs: Switch from standard integer types to kernel ones
  Revert "media: ccs-pll: Fix MODULE_LICENSE"

 .../driver-api/media/drivers/ccs/ccs.rst      |  13 +
 .../userspace-api/media/drivers/ccs.rst       | 110 ++++++
 .../userspace-api/media/drivers/index.rst     |   1 +
 MAINTAINERS                                   |   2 +
 drivers/media/i2c/ccs-pll.c                   | 116 +++----
 drivers/media/i2c/ccs-pll.h                   |  86 ++---
 drivers/media/i2c/ccs/ccs-core.c              | 318 +++++++++++++++---
 drivers/media/i2c/ccs/ccs-reg-access.c        |  29 +-
 drivers/media/i2c/ccs/ccs.h                   |   8 +-
 drivers/media/i2c/ccs/smiapp-reg-defs.h       |   2 +
 include/uapi/linux/ccs.h                      |  18 +
 include/uapi/linux/v4l2-controls.h            |   5 +
 12 files changed, 549 insertions(+), 159 deletions(-)
 create mode 100644 Documentation/userspace-api/media/drivers/ccs.rst
 create mode 100644 include/uapi/linux/ccs.h

-- 
2.29.2

[PATCH 01/24] ccs: Add digital gain support

[Sakari Ailus]

CCS supports global (all-component) digital gain. Add support for it.

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

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index b39ae5f8446b..f1fecc72e247 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -670,6 +670,11 @@ static int ccs_set_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_ANALOGUE_GAIN:
 		rval = ccs_write(sensor, ANALOG_GAIN_CODE_GLOBAL, ctrl->val);
 
+		break;
+
+	case V4L2_CID_DIGITAL_GAIN:
+		rval = ccs_write(sensor, DIGITAL_GAIN_GLOBAL, ctrl->val);
+
 		break;
 	case V4L2_CID_EXPOSURE:
 		rval = ccs_write(sensor, COARSE_INTEGRATION_TIME, ctrl->val);
@@ -739,7 +744,7 @@ static int ccs_init_controls(struct ccs_sensor *sensor)
 	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
 	int rval;
 
-	rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 12);
+	rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 13);
 	if (rval)
 		return rval;
 
@@ -753,6 +758,16 @@ static int ccs_init_controls(struct ccs_sensor *sensor)
 		max(CCS_LIM(sensor, ANALOG_GAIN_CODE_STEP), 1U),
 		CCS_LIM(sensor, ANALOG_GAIN_CODE_MIN));
 
+	if (CCS_LIM(sensor, DIGITAL_GAIN_CAPABILITY) ==
+	    CCS_DIGITAL_GAIN_CAPABILITY_GLOBAL)
+		v4l2_ctrl_new_std(&sensor->pixel_array->ctrl_handler,
+				  &ccs_ctrl_ops, V4L2_CID_DIGITAL_GAIN,
+				  CCS_LIM(sensor, DIGITAL_GAIN_MIN),
+				  CCS_LIM(sensor, DIGITAL_GAIN_MAX),
+				  max(CCS_LIM(sensor, DIGITAL_GAIN_STEP_SIZE),
+				      1U),
+				  0x100);
+
 	/* Exposure limits will be updated soon, use just something here. */
 	sensor->exposure = v4l2_ctrl_new_std(
 		&sensor->pixel_array->ctrl_handler, &ccs_ctrl_ops,
-- 
2.29.2

[PATCH 02/24] ccs: Add support for old-style SMIA digital gain

[Sakari Ailus]

SMIA only has per-component digital gain. Add support for it.

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

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index f1fecc72e247..4b765ac62c0c 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -673,7 +673,34 @@ static int ccs_set_ctrl(struct v4l2_ctrl *ctrl)
 		break;
 
 	case V4L2_CID_DIGITAL_GAIN:
-		rval = ccs_write(sensor, DIGITAL_GAIN_GLOBAL, ctrl->val);
+		if (CCS_LIM(sensor, DIGITAL_GAIN_CAPABILITY) ==
+		    CCS_DIGITAL_GAIN_CAPABILITY_GLOBAL) {
+			rval = ccs_write(sensor, DIGITAL_GAIN_GLOBAL,
+					 ctrl->val);
+			break;
+		}
+
+		rval = ccs_write_addr(sensor,
+				      SMIAPP_REG_U16_DIGITAL_GAIN_GREENR,
+				      ctrl->val);
+		if (rval)
+			break;
+
+		rval = ccs_write_addr(sensor,
+				      SMIAPP_REG_U16_DIGITAL_GAIN_RED,
+				      ctrl->val);
+		if (rval)
+			break;
+
+		rval = ccs_write_addr(sensor,
+				      SMIAPP_REG_U16_DIGITAL_GAIN_BLUE,
+				      ctrl->val);
+		if (rval)
+			break;
+
+		rval = ccs_write_addr(sensor,
+				      SMIAPP_REG_U16_DIGITAL_GAIN_GREENB,
+				      ctrl->val);
 
 		break;
 	case V4L2_CID_EXPOSURE:
@@ -759,7 +786,9 @@ static int ccs_init_controls(struct ccs_sensor *sensor)
 		CCS_LIM(sensor, ANALOG_GAIN_CODE_MIN));
 
 	if (CCS_LIM(sensor, DIGITAL_GAIN_CAPABILITY) ==
-	    CCS_DIGITAL_GAIN_CAPABILITY_GLOBAL)
+	    CCS_DIGITAL_GAIN_CAPABILITY_GLOBAL ||
+	    CCS_LIM(sensor, DIGITAL_GAIN_CAPABILITY) ==
+	    SMIAPP_DIGITAL_GAIN_CAPABILITY_PER_CHANNEL)
 		v4l2_ctrl_new_std(&sensor->pixel_array->ctrl_handler,
 				  &ccs_ctrl_ops, V4L2_CID_DIGITAL_GAIN,
 				  CCS_LIM(sensor, DIGITAL_GAIN_MIN),
diff --git a/drivers/media/i2c/ccs/smiapp-reg-defs.h b/drivers/media/i2c/ccs/smiapp-reg-defs.h
index e80c110ebf3a..177e3e51207a 100644
--- a/drivers/media/i2c/ccs/smiapp-reg-defs.h
+++ b/drivers/media/i2c/ccs/smiapp-reg-defs.h
@@ -535,6 +535,8 @@
 #define SMIAPP_DIGITAL_CROP_CAPABILITY_NONE		0
 #define SMIAPP_DIGITAL_CROP_CAPABILITY_INPUT_CROP	1
 
+#define SMIAPP_DIGITAL_GAIN_CAPABILITY_PER_CHANNEL	1
+
 #define SMIAPP_BINNING_CAPABILITY_NO			0
 #define SMIAPP_BINNING_CAPABILITY_YES			1
 
-- 
2.29.2

[PATCH 03/24] ccs: Remove analogue gain field

[Sakari Ailus]

The analogue gain control was stored to the device specific struct but was
never used. Remove it.

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

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 4b765ac62c0c..706fa811d9b5 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -777,13 +777,12 @@ static int ccs_init_controls(struct ccs_sensor *sensor)
 
 	sensor->pixel_array->ctrl_handler.lock = &sensor->mutex;
 
-	sensor->analog_gain = v4l2_ctrl_new_std(
-		&sensor->pixel_array->ctrl_handler, &ccs_ctrl_ops,
-		V4L2_CID_ANALOGUE_GAIN,
-		CCS_LIM(sensor, ANALOG_GAIN_CODE_MIN),
-		CCS_LIM(sensor, ANALOG_GAIN_CODE_MAX),
-		max(CCS_LIM(sensor, ANALOG_GAIN_CODE_STEP), 1U),
-		CCS_LIM(sensor, ANALOG_GAIN_CODE_MIN));
+	v4l2_ctrl_new_std(&sensor->pixel_array->ctrl_handler, &ccs_ctrl_ops,
+			  V4L2_CID_ANALOGUE_GAIN,
+			  CCS_LIM(sensor, ANALOG_GAIN_CODE_MIN),
+			  CCS_LIM(sensor, ANALOG_GAIN_CODE_MAX),
+			  max(CCS_LIM(sensor, ANALOG_GAIN_CODE_STEP), 1U),
+			  CCS_LIM(sensor, ANALOG_GAIN_CODE_MIN));
 
 	if (CCS_LIM(sensor, DIGITAL_GAIN_CAPABILITY) ==
 	    CCS_DIGITAL_GAIN_CAPABILITY_GLOBAL ||
diff --git a/drivers/media/i2c/ccs/ccs.h b/drivers/media/i2c/ccs/ccs.h
index 356b87c33405..9fc3333f6c4e 100644
--- a/drivers/media/i2c/ccs/ccs.h
+++ b/drivers/media/i2c/ccs/ccs.h
@@ -262,7 +262,6 @@ struct ccs_sensor {
 	unsigned long *valid_link_freqs;
 
 	/* Pixel array controls */
-	struct v4l2_ctrl *analog_gain;
 	struct v4l2_ctrl *exposure;
 	struct v4l2_ctrl *hflip;
 	struct v4l2_ctrl *vflip;
-- 
2.29.2

[PATCH 04/24] ccs: Only add analogue gain control if the device supports it

[Sakari Ailus]

Some devices do not implement analogue gain this way. Only add the control
when a device does have the support.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs/ccs-core.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 706fa811d9b5..47879f9bfe20 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -777,12 +777,16 @@ static int ccs_init_controls(struct ccs_sensor *sensor)
 
 	sensor->pixel_array->ctrl_handler.lock = &sensor->mutex;
 
-	v4l2_ctrl_new_std(&sensor->pixel_array->ctrl_handler, &ccs_ctrl_ops,
-			  V4L2_CID_ANALOGUE_GAIN,
-			  CCS_LIM(sensor, ANALOG_GAIN_CODE_MIN),
-			  CCS_LIM(sensor, ANALOG_GAIN_CODE_MAX),
-			  max(CCS_LIM(sensor, ANALOG_GAIN_CODE_STEP), 1U),
-			  CCS_LIM(sensor, ANALOG_GAIN_CODE_MIN));
+	switch (CCS_LIM(sensor, ANALOG_GAIN_CAPABILITY)) {
+	case CCS_ANALOG_GAIN_CAPABILITY_GLOBAL:
+		v4l2_ctrl_new_std(&sensor->pixel_array->ctrl_handler,
+				  &ccs_ctrl_ops, V4L2_CID_ANALOGUE_GAIN,
+				  CCS_LIM(sensor, ANALOG_GAIN_CODE_MIN),
+				  CCS_LIM(sensor, ANALOG_GAIN_CODE_MAX),
+				  max(CCS_LIM(sensor, ANALOG_GAIN_CODE_STEP),
+				      1U),
+				  CCS_LIM(sensor, ANALOG_GAIN_CODE_MIN));
+	}
 
 	if (CCS_LIM(sensor, DIGITAL_GAIN_CAPABILITY) ==
 	    CCS_DIGITAL_GAIN_CAPABILITY_GLOBAL ||
-- 
2.29.2

[PATCH 05/24] v4l: uapi: Add user control base for CCS controls

[Sakari Ailus]

Add a control base for CCS controls, and reserve 128 controls. Luckily
these numbers are cheap.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 include/uapi/linux/v4l2-controls.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 823b214aac0c..9ead7a8386c8 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -204,6 +204,11 @@ enum v4l2_colorfx {
  * We reserve 16 controls for this driver.
  */
 #define V4L2_CID_USER_CODA_BASE			(V4L2_CID_USER_BASE + 0x10e0)
+/*
+ * The base for MIPI CCS driver controls.
+ * We reserve 128 controls for this driver.
+ */
+#define V4L2_CID_USER_CCS_BASE			(V4L2_CID_USER_BASE + 0x10f0)
 
 /* MPEG-class control IDs */
 /* The MPEG controls are applicable to all codec controls
-- 
2.29.2

[PATCH 06/24] Documentation: ccs: Add user documentation for the CCS driver

[Sakari Ailus]

Add user documentation for the CCS driver. This includes e.g. sub-devices
implemented by the driver.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../userspace-api/media/drivers/ccs.rst       | 59 +++++++++++++++++++
 .../userspace-api/media/drivers/index.rst     |  1 +
 MAINTAINERS                                   |  1 +
 3 files changed, 61 insertions(+)
 create mode 100644 Documentation/userspace-api/media/drivers/ccs.rst

diff --git a/Documentation/userspace-api/media/drivers/ccs.rst b/Documentation/userspace-api/media/drivers/ccs.rst
new file mode 100644
index 000000000000..9f507b0a79b5
--- /dev/null
+++ b/Documentation/userspace-api/media/drivers/ccs.rst
@@ -0,0 +1,59 @@
+.. SPDX-License-Identifier: GPL-2.0-only OR BSD-3-Clause
+
+.. include:: <isonum.txt>
+
+MIPI CCS camera sensor driver
+=============================
+
+The MIPI CCS camera sensor driver is a generic driver for `MIPI CCS
+<https://www.mipi.org/specifications/camera-command-set>`_ compliant
+camera sensors. It exposes three sub-devices representing the pixel array,
+the binner and the scaler.
+
+As the capabilities of individual devices vary, the driver exposes
+interfaces based on the capabilities that exist in hardware.
+
+Pixel Array sub-device
+----------------------
+
+The pixel array sub-device represents the camera sensor's pixel matrix, as well
+as analogue crop functionality present in many compliant devices. The analogue
+crop is configured using the ``V4L2_SEL_TGT_CROP`` on the source pad (0) of the
+entity. The size of the pixel matrix can be obtained by getting the
+``V4L2_SEL_TGT_NATIVE_SIZE`` target.
+
+Binner
+------
+
+The binner sub-device represents the binning functionality on the sensor. For
+that purpose, selection target ``V4L2_SEL_TGT_COMPOSE`` is supported on the
+sink pad (0).
+
+Additionally, if a device has no scaler or digital crop functionality, the
+source pad (1) expses another digital crop selection rectangle that can only
+crop at the end of the lines and frames.
+
+Scaler
+------
+
+The scaler sub-device represents the digital crop and scaling functionality of
+the sensor. The V4L2 selection target ``V4L2_SEL_TGT_CROP`` is used to
+configure the digital crop on the sink pad (0) when digital crop is supported.
+Scaling is configured using selection target ``V4L2_SEL_TGT_COMPOSE`` on the
+sink pad (0) as well.
+
+Additionally, if the scaler sub-device exists, its source pad (1) exposes
+another digital crop selection rectangle that can only crop at the end of the
+lines and frames.
+
+Digital and analogue crop
+-------------------------
+
+Digital crop functionality is referred to as cropping that effectively works by
+dropping some data on the floor. Analogue crop, on the other hand, means that
+the cropped information is never retrieved. In case of camera sensors, the
+analogue data is never read from the pixel matrix that are outside the
+configured selection rectangle that designates crop. The difference has an
+effect in device timing and likely also in power consumption.
+
+**Copyright** |copy| 2020 Intel Corporation
diff --git a/Documentation/userspace-api/media/drivers/index.rst b/Documentation/userspace-api/media/drivers/index.rst
index 05a82f8c0c99..1a9038f5f9fa 100644
--- a/Documentation/userspace-api/media/drivers/index.rst
+++ b/Documentation/userspace-api/media/drivers/index.rst
@@ -31,6 +31,7 @@ For more details see the file COPYING in the source distribution of Linux.
 	:maxdepth: 5
 	:numbered:
 
+	ccs
 	cx2341x-uapi
 	imx-uapi
 	max2175
diff --git a/MAINTAINERS b/MAINTAINERS
index e97d4d9d741e..ab0bf9b8dd72 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11644,6 +11644,7 @@ L:	linux-media@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml
 F:	Documentation/driver-api/media/drivers/ccs/
+F:	Documentation/userspace-api/media/drivers/ccs.rst
 F:	drivers/media/i2c/ccs-pll.c
 F:	drivers/media/i2c/ccs-pll.h
 F:	drivers/media/i2c/ccs/
-- 
2.29.2

[PATCH 07/24] v4l: uapi: ccs: Add controls for analogue gain constants

[Sakari Ailus]

Add a V4L2 controls for analogue gai constants required to control
analogue gain. The values are device specific and thus need to be obtained
from the driver.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../userspace-api/media/drivers/ccs.rst       | 25 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 include/uapi/linux/ccs.h                      | 14 +++++++++++
 3 files changed, 40 insertions(+)
 create mode 100644 include/uapi/linux/ccs.h

diff --git a/Documentation/userspace-api/media/drivers/ccs.rst b/Documentation/userspace-api/media/drivers/ccs.rst
index 9f507b0a79b5..ae02168ccd89 100644
--- a/Documentation/userspace-api/media/drivers/ccs.rst
+++ b/Documentation/userspace-api/media/drivers/ccs.rst
@@ -56,4 +56,29 @@ analogue data is never read from the pixel matrix that are outside the
 configured selection rectangle that designates crop. The difference has an
 effect in device timing and likely also in power consumption.
 
+Private controls
+----------------
+
+The MIPI CCS driver implements a number of private controls under
+``V4L2_CID_USER_BASE_CCS`` to control the MIPI CCS compliant camera sensors.
+
+Analogue gain model
+~~~~~~~~~~~~~~~~~~~
+
+The CCS defines an analogue gain model where the gain can be calculated using
+the following formula:
+
+	gain = m0 * x + c0 / (m1 * x + c1)
+
+Either m0 or c0 will be zero. The constants that are device specific, can be
+obtained from the following controls:
+
+	V4L2_CID_CCS_ANALOGUE_GAIN_M0
+	V4L2_CID_CCS_ANALOGUE_GAIN_M1
+	V4L2_CID_CCS_ANALOGUE_GAIN_C0
+	V4L2_CID_CCS_ANALOGUE_GAIN_C1
+
+The analogue gain (``x`` in the formula) is controlled through
+``V4L2_CID_ANALOGUE_GAIN`` in this case.
+
 **Copyright** |copy| 2020 Intel Corporation
diff --git a/MAINTAINERS b/MAINTAINERS
index ab0bf9b8dd72..bd6ee5c746f5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11648,6 +11648,7 @@ F:	Documentation/userspace-api/media/drivers/ccs.rst
 F:	drivers/media/i2c/ccs-pll.c
 F:	drivers/media/i2c/ccs-pll.h
 F:	drivers/media/i2c/ccs/
+F:	include/uapi/linux/ccs.h
 F:	include/uapi/linux/smiapp.h
 
 MIPS
diff --git a/include/uapi/linux/ccs.h b/include/uapi/linux/ccs.h
new file mode 100644
index 000000000000..2aeea7e0622e
--- /dev/null
+++ b/include/uapi/linux/ccs.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
+/* Copyright (C) 2020 Intel Corporation */
+
+#ifndef __UAPI_CCS_H__
+#define __UAPI_CCS_H__
+
+#include <linux/videodev2.h>
+
+#define V4L2_CID_CCS_ANALOGUE_GAIN_M0		(V4L2_CID_USER_CCS_BASE + 1)
+#define V4L2_CID_CCS_ANALOGUE_GAIN_C0		(V4L2_CID_USER_CCS_BASE + 2)
+#define V4L2_CID_CCS_ANALOGUE_GAIN_M1		(V4L2_CID_USER_CCS_BASE + 3)
+#define V4L2_CID_CCS_ANALOGUE_GAIN_C1		(V4L2_CID_USER_CCS_BASE + 4)
+
+#endif
-- 
2.29.2

[PATCH 08/24] ccs: Add support for analogue gain coefficient controls

[Sakari Ailus]

Add four controls for reading CCS analogue gain coefficients. The
values are constants that are device specific.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs/ccs-core.c | 38 ++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 47879f9bfe20..c51197318c3a 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -28,6 +28,7 @@
 #include <linux/v4l2-mediabus.h>
 #include <media/v4l2-fwnode.h>
 #include <media/v4l2-device.h>
+#include <uapi/linux/ccs.h>
 
 #include "ccs.h"
 
@@ -771,14 +772,46 @@ static int ccs_init_controls(struct ccs_sensor *sensor)
 	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
 	int rval;
 
-	rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 13);
+	rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 17);
 	if (rval)
 		return rval;
 
 	sensor->pixel_array->ctrl_handler.lock = &sensor->mutex;
 
 	switch (CCS_LIM(sensor, ANALOG_GAIN_CAPABILITY)) {
-	case CCS_ANALOG_GAIN_CAPABILITY_GLOBAL:
+	case CCS_ANALOG_GAIN_CAPABILITY_GLOBAL: {
+		struct {
+			const char *name;
+			u32 id;
+			s32 value;
+		} const gain_ctrls[] = {
+			{ "Analogue Gain m0", V4L2_CID_CCS_ANALOGUE_GAIN_M0,
+			  CCS_LIM(sensor, ANALOG_GAIN_M0), },
+			{ "Analogue Gain c0", V4L2_CID_CCS_ANALOGUE_GAIN_C0,
+			  CCS_LIM(sensor, ANALOG_GAIN_C0), },
+			{ "Analogue Gain m1", V4L2_CID_CCS_ANALOGUE_GAIN_M1,
+			  CCS_LIM(sensor, ANALOG_GAIN_M1), },
+			{ "Analogue Gain c1", V4L2_CID_CCS_ANALOGUE_GAIN_C1,
+			  CCS_LIM(sensor, ANALOG_GAIN_C1), },
+		};
+		struct v4l2_ctrl_config ctrl_cfg = {
+			.type = V4L2_CTRL_TYPE_INTEGER,
+			.ops = &ccs_ctrl_ops,
+			.flags = V4L2_CTRL_FLAG_READ_ONLY,
+			.step = 1,
+		};
+		unsigned int i;
+
+		for (i = 0; i < ARRAY_SIZE(gain_ctrls); i++) {
+			ctrl_cfg.name = gain_ctrls[i].name;
+			ctrl_cfg.id = gain_ctrls[i].id;
+			ctrl_cfg.min = ctrl_cfg.max = ctrl_cfg.def =
+				gain_ctrls[i].value;
+
+			v4l2_ctrl_new_custom(&sensor->pixel_array->ctrl_handler,
+					     &ctrl_cfg, NULL);
+		}
+
 		v4l2_ctrl_new_std(&sensor->pixel_array->ctrl_handler,
 				  &ccs_ctrl_ops, V4L2_CID_ANALOGUE_GAIN,
 				  CCS_LIM(sensor, ANALOG_GAIN_CODE_MIN),
@@ -787,6 +820,7 @@ static int ccs_init_controls(struct ccs_sensor *sensor)
 				      1U),
 				  CCS_LIM(sensor, ANALOG_GAIN_CODE_MIN));
 	}
+	}
 
 	if (CCS_LIM(sensor, DIGITAL_GAIN_CAPABILITY) ==
 	    CCS_DIGITAL_GAIN_CAPABILITY_GLOBAL ||
-- 
2.29.2

[PATCH 09/24] v4l: uapi: ccs: Add controls for CCS alternative analogue gain

[Sakari Ailus]

Add two new controls for alternative analogue gain some CCS compliant
camera sensors support.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/userspace-api/media/drivers/ccs.rst | 13 +++++++++++++
 include/uapi/linux/ccs.h                          |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/Documentation/userspace-api/media/drivers/ccs.rst b/Documentation/userspace-api/media/drivers/ccs.rst
index ae02168ccd89..a4bac75e36dd 100644
--- a/Documentation/userspace-api/media/drivers/ccs.rst
+++ b/Documentation/userspace-api/media/drivers/ccs.rst
@@ -81,4 +81,17 @@ obtained from the following controls:
 The analogue gain (``x`` in the formula) is controlled through
 ``V4L2_CID_ANALOGUE_GAIN`` in this case.
 
+Alternate analogue gain model
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The CCS defines another analogue gain model called alternate analogue gain. In
+this case, the formula to calculate actual gain consists of linear and
+exponential parts:
+
+	gain = linear * 2 ^ exponent
+
+The ``linear`` and ``exponent`` factors can be set using the
+``V4L2_CID_CCS_ANALOGUE_LINEAR_GAIN`` and
+``V4L2_CID_CCS_ANALOGUE_EXPONENTIAL_GAIN`` controls, respectively
+
 **Copyright** |copy| 2020 Intel Corporation
diff --git a/include/uapi/linux/ccs.h b/include/uapi/linux/ccs.h
index 2aeea7e0622e..6880958ab7b4 100644
--- a/include/uapi/linux/ccs.h
+++ b/include/uapi/linux/ccs.h
@@ -10,5 +10,7 @@
 #define V4L2_CID_CCS_ANALOGUE_GAIN_C0		(V4L2_CID_USER_CCS_BASE + 2)
 #define V4L2_CID_CCS_ANALOGUE_GAIN_M1		(V4L2_CID_USER_CCS_BASE + 3)
 #define V4L2_CID_CCS_ANALOGUE_GAIN_C1		(V4L2_CID_USER_CCS_BASE + 4)
+#define V4L2_CID_CCS_ANALOGUE_LINEAR_GAIN	(V4L2_CID_USER_CCS_BASE + 5)
+#define V4L2_CID_CCS_ANALOGUE_EXPONENTIAL_GAIN	(V4L2_CID_USER_CCS_BASE + 6)
 
 #endif
-- 
2.29.2

[PATCH 10/24] ccs: Add support for alternate analogue global gain

[Sakari Ailus]

The CCS spec defines an alternative implementation for global analogue
gain. Add support for that in the driver.

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

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index c51197318c3a..7591a52a41a4 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -673,6 +673,17 @@ static int ccs_set_ctrl(struct v4l2_ctrl *ctrl)
 
 		break;
 
+	case V4L2_CID_CCS_ANALOGUE_LINEAR_GAIN:
+		rval = ccs_write(sensor, ANALOG_LINEAR_GAIN_GLOBAL, ctrl->val);
+
+		break;
+
+	case V4L2_CID_CCS_ANALOGUE_EXPONENTIAL_GAIN:
+		rval = ccs_write(sensor, ANALOG_EXPONENTIAL_GAIN_GLOBAL,
+				 ctrl->val);
+
+		break;
+
 	case V4L2_CID_DIGITAL_GAIN:
 		if (CCS_LIM(sensor, DIGITAL_GAIN_CAPABILITY) ==
 		    CCS_DIGITAL_GAIN_CAPABILITY_GLOBAL) {
@@ -819,6 +830,50 @@ static int ccs_init_controls(struct ccs_sensor *sensor)
 				  max(CCS_LIM(sensor, ANALOG_GAIN_CODE_STEP),
 				      1U),
 				  CCS_LIM(sensor, ANALOG_GAIN_CODE_MIN));
+	}
+		break;
+
+	case CCS_ANALOG_GAIN_CAPABILITY_ALTERNATE_GLOBAL: {
+		struct {
+			const char *name;
+			u32 id;
+			u16 min, max, step;
+		} const gain_ctrls[] = {
+			{
+				"Analogue Linear Gain",
+				V4L2_CID_CCS_ANALOGUE_LINEAR_GAIN,
+				CCS_LIM(sensor, ANALOG_LINEAR_GAIN_MIN),
+				CCS_LIM(sensor, ANALOG_LINEAR_GAIN_MAX),
+				max(CCS_LIM(sensor,
+					    ANALOG_LINEAR_GAIN_STEP_SIZE),
+				    1U),
+			},
+			{
+				"Analogue Exponential Gain",
+				V4L2_CID_CCS_ANALOGUE_EXPONENTIAL_GAIN,
+				CCS_LIM(sensor, ANALOG_EXPONENTIAL_GAIN_MIN),
+				CCS_LIM(sensor, ANALOG_EXPONENTIAL_GAIN_MAX),
+				max(CCS_LIM(sensor,
+					    ANALOG_EXPONENTIAL_GAIN_STEP_SIZE),
+				    1U),
+			},
+		};
+		struct v4l2_ctrl_config ctrl_cfg = {
+			.type = V4L2_CTRL_TYPE_INTEGER,
+			.ops = &ccs_ctrl_ops,
+		};
+		unsigned int i;
+
+		for (i = 0; i < ARRAY_SIZE(gain_ctrls); i++) {
+			ctrl_cfg.name = gain_ctrls[i].name;
+			ctrl_cfg.min = ctrl_cfg.def = gain_ctrls[i].min;
+			ctrl_cfg.max = gain_ctrls[i].max;
+			ctrl_cfg.step = gain_ctrls[i].step;
+			ctrl_cfg.id = gain_ctrls[i].id;
+
+			v4l2_ctrl_new_custom(&sensor->pixel_array->ctrl_handler,
+					     &ctrl_cfg, NULL);
+		}
 	}
 	}
 
-- 
2.29.2

[PATCH 11/24] ccs: Add debug prints for MSR registers

[Sakari Ailus]

Also print out MSR registers written to the sensor. This isn't entirely
optimal as the debug strings are produced even if they're not used but
that isn't really a grave issue --- the I²C bus is very slow anyway.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs/ccs-reg-access.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/media/i2c/ccs/ccs-reg-access.c b/drivers/media/i2c/ccs/ccs-reg-access.c
index b776af2a3c33..5f0705952198 100644
--- a/drivers/media/i2c/ccs/ccs-reg-access.c
+++ b/drivers/media/i2c/ccs/ccs-reg-access.c
@@ -387,12 +387,20 @@ int ccs_write_data_regs(struct ccs_sensor *sensor, struct ccs_reg *regs,
 
 		for (j = 0; j < regs->len;
 		     j += msg.len - 2, regdata += msg.len - 2) {
+			char printbuf[(MAX_WRITE_LEN << 1) +
+				      1 /* \0 */] = { 0 };
 			int rval;
 
 			msg.len = min(regs->len - j, MAX_WRITE_LEN);
 
+			bin2hex(printbuf, regdata, msg.len);
+			dev_dbg(&client->dev,
+				"writing msr reg 0x%4.4x value 0x%s\n",
+				regs->addr + j, printbuf);
+
 			put_unaligned_be16(regs->addr + j, buf);
 			memcpy(buf + 2, regdata, msg.len);
+
 			msg.len += 2;
 
 			rval = ccs_write_retry(client, &msg);
-- 
2.29.2

[PATCH 12/24] v4l: uapi: ccs: Add CCS controls for shading correction

[Sakari Ailus]

Add V4L2 controls for controlling CCS lens shading correction as well as
conveying its capabilities.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/userspace-api/media/drivers/ccs.rst | 13 +++++++++++++
 include/uapi/linux/ccs.h                          |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/Documentation/userspace-api/media/drivers/ccs.rst b/Documentation/userspace-api/media/drivers/ccs.rst
index a4bac75e36dd..e0666d564574 100644
--- a/Documentation/userspace-api/media/drivers/ccs.rst
+++ b/Documentation/userspace-api/media/drivers/ccs.rst
@@ -94,4 +94,17 @@ The ``linear`` and ``exponent`` factors can be set using the
 ``V4L2_CID_CCS_ANALOGUE_LINEAR_GAIN`` and
 ``V4L2_CID_CCS_ANALOGUE_EXPONENTIAL_GAIN`` controls, respectively
 
+Shading correction
+~~~~~~~~~~~~~~~~~~
+
+The CCS standard supports lens shading correction. The feature can be controlled
+using ``V4L2_CID_CCS_SHADING_CORRECTION``. Additionally, the luminance
+correction level may be changed using
+``V4L2_CID_CCS_LUMINANCE_CORRECTION_LEVEL``, where value 0 indicates no
+correction and 128 indicates correcting the luminance in corners to 10 % less
+than in the centre.
+
+Shading correction needs to be enabled for luminance correction level to have an
+effect.
+
 **Copyright** |copy| 2020 Intel Corporation
diff --git a/include/uapi/linux/ccs.h b/include/uapi/linux/ccs.h
index 6880958ab7b4..445d1789ff44 100644
--- a/include/uapi/linux/ccs.h
+++ b/include/uapi/linux/ccs.h
@@ -12,5 +12,7 @@
 #define V4L2_CID_CCS_ANALOGUE_GAIN_C1		(V4L2_CID_USER_CCS_BASE + 4)
 #define V4L2_CID_CCS_ANALOGUE_LINEAR_GAIN	(V4L2_CID_USER_CCS_BASE + 5)
 #define V4L2_CID_CCS_ANALOGUE_EXPONENTIAL_GAIN	(V4L2_CID_USER_CCS_BASE + 6)
+#define V4L2_CID_CCS_SHADING_CORRECTION		(V4L2_CID_USER_CCS_BASE + 8)
+#define V4L2_CID_CCS_LUMINANCE_CORRECTION_LEVEL	(V4L2_CID_USER_CCS_BASE + 9)
 
 #endif
-- 
2.29.2

[PATCH 13/24] ccs: Add shading correction and luminance correction level controls

[Sakari Ailus]

Add controls for supporting lens shading correction, including colour
shading and luminance correction level.

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

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 7591a52a41a4..12c30fb0f37a 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -756,6 +756,19 @@ static int ccs_set_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_TEST_PATTERN_GREENB:
 		rval = ccs_write(sensor, TEST_DATA_GREENB, ctrl->val);
 
+		break;
+	case V4L2_CID_CCS_SHADING_CORRECTION:
+		rval = ccs_write(sensor, SHADING_CORRECTION_EN,
+				 ctrl->val ? CCS_SHADING_CORRECTION_EN_ENABLE :
+				 0);
+
+		if (!rval && sensor->luminance_level)
+			v4l2_ctrl_activate(sensor->luminance_level, ctrl->val);
+
+		break;
+	case V4L2_CID_CCS_LUMINANCE_CORRECTION_LEVEL:
+		rval = ccs_write(sensor, LUMINANCE_CORRECTION_LEVEL, ctrl->val);
+
 		break;
 	case V4L2_CID_PIXEL_RATE:
 		/* For v4l2_ctrl_s_ctrl_int64() used internally. */
@@ -877,6 +890,39 @@ static int ccs_init_controls(struct ccs_sensor *sensor)
 	}
 	}
 
+	if (CCS_LIM(sensor, SHADING_CORRECTION_CAPABILITY) &
+	    (CCS_SHADING_CORRECTION_CAPABILITY_COLOR_SHADING |
+	     CCS_SHADING_CORRECTION_CAPABILITY_LUMINANCE_CORRECTION)) {
+		const struct v4l2_ctrl_config ctrl_cfg = {
+			.name = "Shading Correction",
+			.type = V4L2_CTRL_TYPE_BOOLEAN,
+			.id = V4L2_CID_CCS_SHADING_CORRECTION,
+			.ops = &ccs_ctrl_ops,
+			.max = 1,
+			.step = 1,
+		};
+
+		v4l2_ctrl_new_custom(&sensor->pixel_array->ctrl_handler,
+				     &ctrl_cfg, NULL);
+	}
+
+	if (CCS_LIM(sensor, SHADING_CORRECTION_CAPABILITY) &
+	    CCS_SHADING_CORRECTION_CAPABILITY_LUMINANCE_CORRECTION) {
+		const struct v4l2_ctrl_config ctrl_cfg = {
+			.name = "Luminance Correction Level",
+			.type = V4L2_CTRL_TYPE_BOOLEAN,
+			.id = V4L2_CID_CCS_LUMINANCE_CORRECTION_LEVEL,
+			.ops = &ccs_ctrl_ops,
+			.max = 255,
+			.step = 1,
+			.def = 128,
+		};
+
+		sensor->luminance_level =
+			v4l2_ctrl_new_custom(&sensor->pixel_array->ctrl_handler,
+					     &ctrl_cfg, NULL);
+	}
+
 	if (CCS_LIM(sensor, DIGITAL_GAIN_CAPABILITY) ==
 	    CCS_DIGITAL_GAIN_CAPABILITY_GLOBAL ||
 	    CCS_LIM(sensor, DIGITAL_GAIN_CAPABILITY) ==
diff --git a/drivers/media/i2c/ccs/ccs.h b/drivers/media/i2c/ccs/ccs.h
index 9fc3333f6c4e..cc33c9ba3165 100644
--- a/drivers/media/i2c/ccs/ccs.h
+++ b/drivers/media/i2c/ccs/ccs.h
@@ -268,6 +268,7 @@ struct ccs_sensor {
 	struct v4l2_ctrl *vblank;
 	struct v4l2_ctrl *hblank;
 	struct v4l2_ctrl *pixel_rate_parray;
+	struct v4l2_ctrl *luminance_level;
 	/* src controls */
 	struct v4l2_ctrl *link_freq;
 	struct v4l2_ctrl *pixel_rate_csi;
-- 
2.29.2

[PATCH 14/24] ccs: Get the endpoint by port rather than any next endpoint

[Sakari Ailus]

Get the first endpoint from port 0 instead of the next one, whatever it
might be. There are no other ports so there's no functional change.

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

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 12c30fb0f37a..11c6de7f55aa 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -3090,7 +3090,8 @@ static int ccs_get_hwconfig(struct ccs_sensor *sensor, struct device *dev)
 	int i;
 	int rval;
 
-	ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
+	ep = fwnode_graph_get_endpoint_by_id(fwnode, 0, 0,
+					     FWNODE_GRAPH_ENDPOINT_NEXT);
 	if (!ep)
 		return -ENODEV;
 
-- 
2.29.2

[PATCH 15/24] ccs: Don't change the I²C address just for software reset

[Sakari Ailus]

The sensor's address was changed before resetting and changing the
address again. Don't do it before reset as it's useless.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs/ccs-core.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 11c6de7f55aa..da7a6bd2c820 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -1552,14 +1552,6 @@ static int ccs_power_on(struct device *dev)
 	 * is found.
 	 */
 
-	if (sensor->hwcfg.i2c_addr_alt) {
-		rval = ccs_change_cci_addr(sensor);
-		if (rval) {
-			dev_err(dev, "cci address change error\n");
-			goto out_cci_addr_fail;
-		}
-	}
-
 	rval = ccs_write(sensor, SOFTWARE_RESET, CCS_SOFTWARE_RESET_ON);
 	if (rval < 0) {
 		dev_err(dev, "software reset failed\n");
-- 
2.29.2

[PATCH 16/24] ccs: Only do software reset if we have no hardware reset

[Sakari Ailus]

The driver always used software reset after the sensor's power sequence
that includes a hardware reset if we have a reset GPIO. Do not use
software reset if we just brought the sensor up from hardware reset state.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs/ccs-core.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index da7a6bd2c820..fdf2e83eeac3 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -1552,10 +1552,12 @@ static int ccs_power_on(struct device *dev)
 	 * is found.
 	 */
 
-	rval = ccs_write(sensor, SOFTWARE_RESET, CCS_SOFTWARE_RESET_ON);
-	if (rval < 0) {
-		dev_err(dev, "software reset failed\n");
-		goto out_cci_addr_fail;
+	if (!sensor->reset && !sensor->xshutdown) {
+		rval = ccs_write(sensor, SOFTWARE_RESET, CCS_SOFTWARE_RESET_ON);
+		if (rval < 0) {
+			dev_err(dev, "software reset failed\n");
+			goto out_cci_addr_fail;
+		}
 	}
 
 	if (sensor->hwcfg.i2c_addr_alt) {
-- 
2.29.2

[PATCH 17/24] ccs: Wait until software reset is done

[Sakari Ailus]

Verify the software reset has been completed until proceeding.

The spec does not guarantee a delay but presumably 100 ms should be
enough.

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

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index fdf2e83eeac3..e1b3c5693e01 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -1553,11 +1553,26 @@ static int ccs_power_on(struct device *dev)
 	 */
 
 	if (!sensor->reset && !sensor->xshutdown) {
+		u8 retry = 100;
+		u32 reset;
+
 		rval = ccs_write(sensor, SOFTWARE_RESET, CCS_SOFTWARE_RESET_ON);
 		if (rval < 0) {
 			dev_err(dev, "software reset failed\n");
 			goto out_cci_addr_fail;
 		}
+
+		do {
+			rval = ccs_read(sensor, SOFTWARE_RESET, &reset);
+			reset = !rval && reset == CCS_SOFTWARE_RESET_OFF;
+			if (reset)
+				break;
+
+			usleep_range(1000, 2000);
+		} while (--retry);
+
+		if (!reset)
+			return -EIO;
 	}
 
 	if (sensor->hwcfg.i2c_addr_alt) {
-- 
2.29.2

[PATCH 18/24] ccs: Hardware requires a delay after starting the clock of lifting reset

[Sakari Ailus]

A CCS compliant device requires a delay before the first I²C transaction
after pulling xshutdown up or starting the external clock. This is what
the driver does. However, if neither is actually there, there's no need
for the delay.

This also has the effect of removing an unnecessary delay on ACPI systems
where ACPI is responsible for the power-up sequence.

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

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index e1b3c5693e01..fae8ceded861 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -1515,7 +1515,6 @@ static int ccs_power_on(struct device *dev)
 	struct ccs_sensor *sensor =
 		container_of(ssd, struct ccs_sensor, ssds[0]);
 	const struct ccs_device *ccsdev = device_get_match_data(dev);
-	unsigned int sleep;
 	int rval;
 
 	rval = regulator_bulk_enable(ARRAY_SIZE(ccs_regulators),
@@ -1525,21 +1524,25 @@ static int ccs_power_on(struct device *dev)
 		return rval;
 	}
 
-	rval = clk_prepare_enable(sensor->ext_clk);
-	if (rval < 0) {
-		dev_dbg(dev, "failed to enable xclk\n");
-		goto out_xclk_fail;
-	}
+	if (sensor->reset || sensor->xshutdown || sensor->ext_clk) {
+		unsigned int sleep;
+
+		rval = clk_prepare_enable(sensor->ext_clk);
+		if (rval < 0) {
+			dev_dbg(dev, "failed to enable xclk\n");
+			goto out_xclk_fail;
+		}
 
-	gpiod_set_value(sensor->reset, 0);
-	gpiod_set_value(sensor->xshutdown, 1);
+		gpiod_set_value(sensor->reset, 0);
+		gpiod_set_value(sensor->xshutdown, 1);
 
-	if (ccsdev->flags & CCS_DEVICE_FLAG_IS_SMIA)
-		sleep = SMIAPP_RESET_DELAY(sensor->hwcfg.ext_clk);
-	else
-		sleep = 5000;
+		if (ccsdev->flags & CCS_DEVICE_FLAG_IS_SMIA)
+			sleep = SMIAPP_RESET_DELAY(sensor->hwcfg.ext_clk);
+		else
+			sleep = 5000;
 
-	usleep_range(sleep, sleep);
+		usleep_range(sleep, sleep);
+	}
 
 	/*
 	 * Failures to respond to the address change command have been noticed.
-- 
2.29.2

[PATCH 19/24] ccs: Add a sanity check for external clock frequency

[Sakari Ailus]

The driver depends on the external clock frequency. Add a sanity check for
the frequency, by returning an error from probe if it's zero.

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

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index fae8ceded861..08fce285f73a 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -3275,6 +3275,11 @@ static int ccs_probe(struct i2c_client *client)
 		return -EINVAL;
 	}
 
+	if (!sensor->hwcfg.ext_clk) {
+		dev_err(&client->dev, "cannot work with xclk frequency 0\n");
+		return -EINVAL;
+	}
+
 	sensor->reset = devm_gpiod_get_optional(&client->dev, "reset",
 						GPIOD_OUT_HIGH);
 	if (IS_ERR(sensor->reset))
-- 
2.29.2

[PATCH 20/24] ccs: Support and default to auto PHY control

[Sakari Ailus]

CCS supports three variants of PHY timing control, auto, UI based and
manual. The driver previously assumed UI based control that requires
updating the link rate to the sensor. Now default to automatic control
instead, and only write the link rate to the sensor in UI mode.

If neither auto or UI control is supported, return an error in probe.

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

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 08fce285f73a..a8f591c95bc2 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -383,15 +383,22 @@ static int ccs_pll_configure(struct ccs_sensor *sensor)
 	if (rval < 0)
 		return rval;
 
-	/* Lane op clock ratio does not apply here. */
-	rval = ccs_write(sensor, REQUESTED_LINK_RATE,
-			 DIV_ROUND_UP(pll->op_bk.sys_clk_freq_hz,
-				      1000000 / 256 / 256) *
-			 (pll->flags & CCS_PLL_FLAG_LANE_SPEED_MODEL ?
-			  sensor->pll.csi2.lanes : 1) <<
-			 (pll->flags & CCS_PLL_FLAG_OP_SYS_DDR ? 1 : 0));
-	if (rval < 0 || sensor->pll.flags & CCS_PLL_FLAG_NO_OP_CLOCKS)
-		return rval;
+	if (!(CCS_LIM(sensor, PHY_CTRL_CAPABILITY) &
+	      CCS_PHY_CTRL_CAPABILITY_AUTO_PHY_CTL)) {
+		/* Lane op clock ratio does not apply here. */
+		rval = ccs_write(sensor, REQUESTED_LINK_RATE,
+				 DIV_ROUND_UP(pll->op_bk.sys_clk_freq_hz,
+					      1000000 / 256 / 256) *
+				 (pll->flags & CCS_PLL_FLAG_LANE_SPEED_MODEL ?
+				  sensor->pll.csi2.lanes : 1) <<
+				 (pll->flags & CCS_PLL_FLAG_OP_SYS_DDR ?
+				  1 : 0));
+		if (rval < 0)
+			return rval;
+	}
+
+	if (sensor->pll.flags & CCS_PLL_FLAG_NO_OP_CLOCKS)
+		return 0;
 
 	rval = ccs_write(sensor, OP_PIX_CLK_DIV, pll->op_bk.pix_clk_div);
 	if (rval < 0)
@@ -1504,6 +1511,28 @@ static int ccs_write_msr_regs(struct ccs_sensor *sensor)
 				   sensor->mdata.num_module_manufacturer_regs);
 }
 
+static int ccs_update_phy_ctrl(struct ccs_sensor *sensor)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd);
+	u8 val;
+
+	if (!sensor->ccs_limits)
+		return 0;
+
+	if (CCS_LIM(sensor, PHY_CTRL_CAPABILITY) &
+	    CCS_PHY_CTRL_CAPABILITY_AUTO_PHY_CTL) {
+		val = CCS_PHY_CTRL_AUTO;
+	} else if (CCS_LIM(sensor, PHY_CTRL_CAPABILITY) &
+		   CCS_PHY_CTRL_CAPABILITY_UI_PHY_CTL) {
+		val = CCS_PHY_CTRL_UI;
+	} else {
+		dev_err(&client->dev, "manual PHY control not supported\n");
+		return -EINVAL;
+	}
+
+	return ccs_write(sensor, PHY_CTRL, val);
+}
+
 static int ccs_power_on(struct device *dev)
 {
 	struct v4l2_subdev *subdev = dev_get_drvdata(dev);
@@ -1620,8 +1649,7 @@ static int ccs_power_on(struct device *dev)
 		goto out_cci_addr_fail;
 	}
 
-	/* DPHY control done by sensor based on requested link rate */
-	rval = ccs_write(sensor, PHY_CTRL, CCS_PHY_CTRL_UI);
+	rval = ccs_update_phy_ctrl(sensor);
 	if (rval < 0)
 		goto out_cci_addr_fail;
 
@@ -3348,6 +3376,10 @@ static int ccs_probe(struct i2c_client *client)
 		goto out_free_ccs_limits;
 	}
 
+	rval = ccs_update_phy_ctrl(sensor);
+	if (rval < 0)
+		goto out_free_ccs_limits;
+
 	/*
 	 * Handle Sensor Module orientation on the board.
 	 *
-- 
2.29.2

[PATCH 21/24] Documentation: Include CCS PLL calculator to CCS driver documentation

[Sakari Ailus]

Include existing CCS PLL calculator kerneldoc documentation to the
documentation build.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/driver-api/media/drivers/ccs/ccs.rst | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/driver-api/media/drivers/ccs/ccs.rst b/Documentation/driver-api/media/drivers/ccs/ccs.rst
index f49e971f2d92..b461c8aa2a16 100644
--- a/Documentation/driver-api/media/drivers/ccs/ccs.rst
+++ b/Documentation/driver-api/media/drivers/ccs/ccs.rst
@@ -79,4 +79,17 @@ definitions:
 		-l drivers/media/i2c/ccs/ccs-limits.c \
 		-c Documentation/driver-api/media/drivers/ccs/ccs-regs.asc
 
+CCS PLL calculator
+==================
+
+The CCS PLL calculator is used to compute the PLL configuration, given sensor's
+capabilities as well as board configuration and user specified configuration. As
+the configuration space that encompasses all these configurations is vast, the
+PLL calculator isn't entirely trivial. Yet it is relatively simple to use for a
+driver.
+
+The PLL model implemented by the PLL calculator corresponds to MIPI CCS 1.1.
+
+.. kernel-doc:: drivers/media/i2c/ccs-pll.h
+
 **Copyright** |copy| 2020 Intel Corporation
-- 
2.29.2

[PATCH 22/24] ccs-pll: Switch from standard integer types to kernel ones

[Sakari Ailus]

The preferred integer types in the kernel are the Linux specific ones,
switch from standard C types to u32 and alike.

The patch has been produced with the following Coccinelle spatch, with few
alignment adjustments:

@@
typedef uint32_t;
typedef u32;
@@
- uint32_t
+ u32

@@
typedef uint16_t;
typedef u16;
@@
- uint16_t
+ u16

@@
typedef uint8_t;
typedef u8;
@@
- uint8_t
+ u8

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs-pll.c | 114 ++++++++++++++++++------------------
 drivers/media/i2c/ccs-pll.h |  86 +++++++++++++--------------
 2 files changed, 100 insertions(+), 100 deletions(-)

diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
index eb7b6f01f623..530085693a56 100644
--- a/drivers/media/i2c/ccs-pll.c
+++ b/drivers/media/i2c/ccs-pll.c
@@ -17,20 +17,20 @@
 #include "ccs-pll.h"
 
 /* Return an even number or one. */
-static inline uint32_t clk_div_even(uint32_t a)
+static inline u32 clk_div_even(u32 a)
 {
-	return max_t(uint32_t, 1, a & ~1);
+	return max_t(u32, 1, a & ~1);
 }
 
 /* Return an even number or one. */
-static inline uint32_t clk_div_even_up(uint32_t a)
+static inline u32 clk_div_even_up(u32 a)
 {
 	if (a == 1)
 		return 1;
 	return (a + 1) & ~1;
 }
 
-static inline uint32_t is_one_or_even(uint32_t a)
+static inline u32 is_one_or_even(u32 a)
 {
 	if (a == 1)
 		return 1;
@@ -40,13 +40,13 @@ static inline uint32_t is_one_or_even(uint32_t a)
 	return 1;
 }
 
-static inline uint32_t one_or_more(uint32_t a)
+static inline u32 one_or_more(u32 a)
 {
 	return a ?: 1;
 }
 
-static int bounds_check(struct device *dev, uint32_t val,
-			uint32_t min, uint32_t max, const char *prefix,
+static int bounds_check(struct device *dev, u32 val,
+			u32 min, u32 max, const char *prefix,
 			char *str)
 {
 	if (val >= min && val <= max)
@@ -138,12 +138,12 @@ static void print_pll(struct device *dev, struct ccs_pll *pll)
 		pll->flags & PLL_FL(OP_PIX_DDR) ? " op-pix-ddr" : "");
 }
 
-static uint32_t op_sys_ddr(uint32_t flags)
+static u32 op_sys_ddr(u32 flags)
 {
 	return flags & CCS_PLL_FLAG_OP_SYS_DDR ? 1 : 0;
 }
 
-static uint32_t op_pix_ddr(uint32_t flags)
+static u32 op_pix_ddr(u32 flags)
 {
 	return flags & CCS_PLL_FLAG_OP_PIX_DDR ? 1 : 0;
 }
@@ -250,8 +250,8 @@ static int check_ext_bounds(struct device *dev, struct ccs_pll *pll)
 static void
 ccs_pll_find_vt_sys_div(struct device *dev, const struct ccs_pll_limits *lim,
 			struct ccs_pll *pll, struct ccs_pll_branch_fr *pll_fr,
-			uint16_t min_vt_div, uint16_t max_vt_div,
-			uint16_t *min_sys_div, uint16_t *max_sys_div)
+			u16 min_vt_div, u16 max_vt_div,
+			u16 *min_sys_div, u16 *max_sys_div)
 {
 	/*
 	 * Find limits for sys_clk_div. Not all values are possible with all
@@ -259,11 +259,11 @@ ccs_pll_find_vt_sys_div(struct device *dev, const struct ccs_pll_limits *lim,
 	 */
 	*min_sys_div = lim->vt_bk.min_sys_clk_div;
 	dev_dbg(dev, "min_sys_div: %u\n", *min_sys_div);
-	*min_sys_div = max_t(uint16_t, *min_sys_div,
+	*min_sys_div = max_t(u16, *min_sys_div,
 			     DIV_ROUND_UP(min_vt_div,
 					  lim->vt_bk.max_pix_clk_div));
 	dev_dbg(dev, "min_sys_div: max_vt_pix_clk_div: %u\n", *min_sys_div);
-	*min_sys_div = max_t(uint16_t, *min_sys_div,
+	*min_sys_div = max_t(u16, *min_sys_div,
 			     pll_fr->pll_op_clk_freq_hz
 			     / lim->vt_bk.max_sys_clk_freq_hz);
 	dev_dbg(dev, "min_sys_div: max_pll_op_clk_freq_hz: %u\n", *min_sys_div);
@@ -272,11 +272,11 @@ ccs_pll_find_vt_sys_div(struct device *dev, const struct ccs_pll_limits *lim,
 
 	*max_sys_div = lim->vt_bk.max_sys_clk_div;
 	dev_dbg(dev, "max_sys_div: %u\n", *max_sys_div);
-	*max_sys_div = min_t(uint16_t, *max_sys_div,
+	*max_sys_div = min_t(u16, *max_sys_div,
 			     DIV_ROUND_UP(max_vt_div,
 					  lim->vt_bk.min_pix_clk_div));
 	dev_dbg(dev, "max_sys_div: min_vt_pix_clk_div: %u\n", *max_sys_div);
-	*max_sys_div = min_t(uint16_t, *max_sys_div,
+	*max_sys_div = min_t(u16, *max_sys_div,
 			     DIV_ROUND_UP(pll_fr->pll_op_clk_freq_hz,
 					  lim->vt_bk.min_pix_clk_freq_hz));
 	dev_dbg(dev, "max_sys_div: min_vt_pix_clk_freq_hz: %u\n", *max_sys_div);
@@ -289,15 +289,15 @@ ccs_pll_find_vt_sys_div(struct device *dev, const struct ccs_pll_limits *lim,
 static inline int
 __ccs_pll_calculate_vt_tree(struct device *dev,
 			    const struct ccs_pll_limits *lim,
-			    struct ccs_pll *pll, uint32_t mul, uint32_t div)
+			    struct ccs_pll *pll, u32 mul, u32 div)
 {
 	const struct ccs_pll_branch_limits_fr *lim_fr = &lim->vt_fr;
 	const struct ccs_pll_branch_limits_bk *lim_bk = &lim->vt_bk;
 	struct ccs_pll_branch_fr *pll_fr = &pll->vt_fr;
 	struct ccs_pll_branch_bk *pll_bk = &pll->vt_bk;
-	uint32_t more_mul;
-	uint16_t best_pix_div = SHRT_MAX >> 1, best_div;
-	uint16_t vt_div, min_sys_div, max_sys_div, sys_div;
+	u32 more_mul;
+	u16 best_pix_div = SHRT_MAX >> 1, best_div;
+	u16 vt_div, min_sys_div, max_sys_div, sys_div;
 
 	pll_fr->pll_ip_clk_freq_hz =
 		pll->ext_clk_freq_hz / pll_fr->pre_pll_clk_div;
@@ -331,7 +331,7 @@ __ccs_pll_calculate_vt_tree(struct device *dev,
 
 	for (sys_div = min_sys_div; sys_div <= max_sys_div;
 	     sys_div += 2 - (sys_div & 1)) {
-		uint16_t pix_div;
+		u16 pix_div;
 
 		if (vt_div % sys_div)
 			continue;
@@ -379,9 +379,9 @@ static int ccs_pll_calculate_vt_tree(struct device *dev,
 {
 	const struct ccs_pll_branch_limits_fr *lim_fr = &lim->vt_fr;
 	struct ccs_pll_branch_fr *pll_fr = &pll->vt_fr;
-	uint16_t min_pre_pll_clk_div = lim_fr->min_pre_pll_clk_div;
-	uint16_t max_pre_pll_clk_div = lim_fr->max_pre_pll_clk_div;
-	uint32_t pre_mul, pre_div;
+	u16 min_pre_pll_clk_div = lim_fr->min_pre_pll_clk_div;
+	u16 max_pre_pll_clk_div = lim_fr->max_pre_pll_clk_div;
+	u32 pre_mul, pre_div;
 
 	pre_div = gcd(pll->pixel_rate_csi,
 		      pll->ext_clk_freq_hz * pll->vt_lanes);
@@ -390,11 +390,11 @@ static int ccs_pll_calculate_vt_tree(struct device *dev,
 
 	/* Make sure PLL input frequency is within limits */
 	max_pre_pll_clk_div =
-		min_t(uint16_t, max_pre_pll_clk_div,
+		min_t(u16, max_pre_pll_clk_div,
 		      DIV_ROUND_UP(pll->ext_clk_freq_hz,
 				   lim_fr->min_pll_ip_clk_freq_hz));
 
-	min_pre_pll_clk_div = max_t(uint16_t, min_pre_pll_clk_div,
+	min_pre_pll_clk_div = max_t(u16, min_pre_pll_clk_div,
 				    pll->ext_clk_freq_hz /
 				    lim_fr->max_pll_ip_clk_freq_hz);
 
@@ -406,7 +406,7 @@ static int ccs_pll_calculate_vt_tree(struct device *dev,
 	     pll_fr->pre_pll_clk_div +=
 		     (pll->flags & CCS_PLL_FLAG_EXT_IP_PLL_DIVIDER) ? 1 :
 		     2 - (pll_fr->pre_pll_clk_div & 1)) {
-		uint32_t mul, div;
+		u32 mul, div;
 		int rval;
 
 		div = gcd(pre_mul * pll_fr->pre_pll_clk_div, pre_div);
@@ -440,13 +440,13 @@ ccs_pll_calculate_vt(struct device *dev, const struct ccs_pll_limits *lim,
 		     const struct ccs_pll_branch_limits_bk *op_lim_bk,
 		     struct ccs_pll *pll, struct ccs_pll_branch_fr *pll_fr,
 		     struct ccs_pll_branch_bk *op_pll_bk, bool cphy,
-		     uint32_t phy_const)
+		     u32 phy_const)
 {
-	uint16_t sys_div;
-	uint16_t best_pix_div = SHRT_MAX >> 1;
-	uint16_t vt_op_binning_div;
-	uint16_t min_vt_div, max_vt_div, vt_div;
-	uint16_t min_sys_div, max_sys_div;
+	u16 sys_div;
+	u16 best_pix_div = SHRT_MAX >> 1;
+	u16 vt_op_binning_div;
+	u16 min_vt_div, max_vt_div, vt_div;
+	u16 min_sys_div, max_sys_div;
 
 	if (pll->flags & CCS_PLL_FLAG_NO_OP_CLOCKS)
 		goto out_calc_pixel_rate;
@@ -500,18 +500,18 @@ ccs_pll_calculate_vt(struct device *dev, const struct ccs_pll_limits *lim,
 
 	/* Find smallest and biggest allowed vt divisor. */
 	dev_dbg(dev, "min_vt_div: %u\n", min_vt_div);
-	min_vt_div = max_t(uint16_t, min_vt_div,
+	min_vt_div = max_t(u16, min_vt_div,
 			   DIV_ROUND_UP(pll_fr->pll_op_clk_freq_hz,
 					lim->vt_bk.max_pix_clk_freq_hz));
 	dev_dbg(dev, "min_vt_div: max_vt_pix_clk_freq_hz: %u\n",
 		min_vt_div);
-	min_vt_div = max_t(uint16_t, min_vt_div, lim->vt_bk.min_pix_clk_div
-						 * lim->vt_bk.min_sys_clk_div);
+	min_vt_div = max_t(u16, min_vt_div, lim->vt_bk.min_pix_clk_div
+					    * lim->vt_bk.min_sys_clk_div);
 	dev_dbg(dev, "min_vt_div: min_vt_clk_div: %u\n", min_vt_div);
 
 	max_vt_div = lim->vt_bk.max_sys_clk_div * lim->vt_bk.max_pix_clk_div;
 	dev_dbg(dev, "max_vt_div: %u\n", max_vt_div);
-	max_vt_div = min_t(uint16_t, max_vt_div,
+	max_vt_div = min_t(u16, max_vt_div,
 			   DIV_ROUND_UP(pll_fr->pll_op_clk_freq_hz,
 				      lim->vt_bk.min_pix_clk_freq_hz));
 	dev_dbg(dev, "max_vt_div: min_vt_pix_clk_freq_hz: %u\n",
@@ -526,12 +526,12 @@ ccs_pll_calculate_vt(struct device *dev, const struct ccs_pll_limits *lim,
 	 * divisor.
 	 */
 	for (vt_div = min_vt_div; vt_div <= max_vt_div; vt_div++) {
-		uint16_t __max_sys_div = vt_div & 1 ? 1 : max_sys_div;
+		u16 __max_sys_div = vt_div & 1 ? 1 : max_sys_div;
 
 		for (sys_div = min_sys_div; sys_div <= __max_sys_div;
 		     sys_div += 2 - (sys_div & 1)) {
-			uint16_t pix_div;
-			uint16_t rounded_div;
+			u16 pix_div;
+			u16 rounded_div;
 
 			pix_div = DIV_ROUND_UP(vt_div, sys_div);
 
@@ -588,9 +588,9 @@ ccs_pll_calculate_op(struct device *dev, const struct ccs_pll_limits *lim,
 		     const struct ccs_pll_branch_limits_fr *op_lim_fr,
 		     const struct ccs_pll_branch_limits_bk *op_lim_bk,
 		     struct ccs_pll *pll, struct ccs_pll_branch_fr *op_pll_fr,
-		     struct ccs_pll_branch_bk *op_pll_bk, uint32_t mul,
-		     uint32_t div, uint32_t op_sys_clk_freq_hz_sdr, uint32_t l,
-		     bool cphy, uint32_t phy_const)
+		     struct ccs_pll_branch_bk *op_pll_bk, u32 mul,
+		     u32 div, u32 op_sys_clk_freq_hz_sdr, u32 l,
+		     bool cphy, u32 phy_const)
 {
 	/*
 	 * Higher multipliers (and divisors) are often required than
@@ -598,9 +598,9 @@ ccs_pll_calculate_op(struct device *dev, const struct ccs_pll_limits *lim,
 	 * There are limits for all values in the clock tree. These
 	 * are the minimum and maximum multiplier for mul.
 	 */
-	uint32_t more_mul_min, more_mul_max;
-	uint32_t more_mul_factor;
-	uint32_t i;
+	u32 more_mul_min, more_mul_max;
+	u32 more_mul_factor;
+	u32 i;
 
 	/*
 	 * Get pre_pll_clk_div so that our pll_op_clk_freq_hz won't be
@@ -614,7 +614,7 @@ ccs_pll_calculate_op(struct device *dev, const struct ccs_pll_limits *lim,
 		more_mul_max);
 	/* Don't go above max pll op frequency. */
 	more_mul_max =
-		min_t(uint32_t,
+		min_t(u32,
 		      more_mul_max,
 		      op_lim_fr->max_pll_op_clk_freq_hz
 		      / (pll->ext_clk_freq_hz /
@@ -706,14 +706,14 @@ int ccs_pll_calculate(struct device *dev, const struct ccs_pll_limits *lim,
 	struct ccs_pll_branch_fr *op_pll_fr;
 	struct ccs_pll_branch_bk *op_pll_bk;
 	bool cphy = pll->bus_type == CCS_PLL_BUS_TYPE_CSI2_CPHY;
-	uint32_t phy_const = cphy ? CPHY_CONST : DPHY_CONST;
-	uint32_t op_sys_clk_freq_hz_sdr;
-	uint16_t min_op_pre_pll_clk_div;
-	uint16_t max_op_pre_pll_clk_div;
-	uint32_t mul, div;
-	uint32_t l = (!pll->op_bits_per_lane ||
-		      pll->op_bits_per_lane >= pll->bits_per_pixel) ? 1 : 2;
-	uint32_t i;
+	u32 phy_const = cphy ? CPHY_CONST : DPHY_CONST;
+	u32 op_sys_clk_freq_hz_sdr;
+	u16 min_op_pre_pll_clk_div;
+	u16 max_op_pre_pll_clk_div;
+	u32 mul, div;
+	u32 l = (!pll->op_bits_per_lane ||
+		 pll->op_bits_per_lane >= pll->bits_per_pixel) ? 1 : 2;
+	u32 i;
 	int rval = -EINVAL;
 
 	if (!(pll->flags & CCS_PLL_FLAG_LANE_SPEED_MODEL)) {
@@ -797,11 +797,11 @@ int ccs_pll_calculate(struct device *dev, const struct ccs_pll_limits *lim,
 	dev_dbg(dev, "min / max op_pre_pll_clk_div: %u / %u\n",
 		op_lim_fr->min_pre_pll_clk_div, op_lim_fr->max_pre_pll_clk_div);
 	max_op_pre_pll_clk_div =
-		min_t(uint16_t, op_lim_fr->max_pre_pll_clk_div,
+		min_t(u16, op_lim_fr->max_pre_pll_clk_div,
 		      clk_div_even(pll->ext_clk_freq_hz /
 				   op_lim_fr->min_pll_ip_clk_freq_hz));
 	min_op_pre_pll_clk_div =
-		max_t(uint16_t, op_lim_fr->min_pre_pll_clk_div,
+		max_t(u16, op_lim_fr->min_pre_pll_clk_div,
 		      clk_div_even_up(
 			      DIV_ROUND_UP(pll->ext_clk_freq_hz,
 					   op_lim_fr->max_pll_ip_clk_freq_hz)));
@@ -815,7 +815,7 @@ int ccs_pll_calculate(struct device *dev, const struct ccs_pll_limits *lim,
 	dev_dbg(dev, "mul %u / div %u\n", mul, div);
 
 	min_op_pre_pll_clk_div =
-		max_t(uint16_t, min_op_pre_pll_clk_div,
+		max_t(u16, min_op_pre_pll_clk_div,
 		      clk_div_even_up(
 			      mul /
 			      one_or_more(
diff --git a/drivers/media/i2c/ccs-pll.h b/drivers/media/i2c/ccs-pll.h
index b97d7ff50ea5..6eb1b1c68e1e 100644
--- a/drivers/media/i2c/ccs-pll.h
+++ b/drivers/media/i2c/ccs-pll.h
@@ -44,10 +44,10 @@
  * @pll_op_clk_freq_hz: PLL output clock frequency
  */
 struct ccs_pll_branch_fr {
-	uint16_t pre_pll_clk_div;
-	uint16_t pll_multiplier;
-	uint32_t pll_ip_clk_freq_hz;
-	uint32_t pll_op_clk_freq_hz;
+	u16 pre_pll_clk_div;
+	u16 pll_multiplier;
+	u32 pll_ip_clk_freq_hz;
+	u32 pll_op_clk_freq_hz;
 };
 
 /**
@@ -61,10 +61,10 @@ struct ccs_pll_branch_fr {
  * @pix_clk_freq_hz: Pixel clock frequency
  */
 struct ccs_pll_branch_bk {
-	uint16_t sys_clk_div;
-	uint16_t pix_clk_div;
-	uint32_t sys_clk_freq_hz;
-	uint32_t pix_clk_freq_hz;
+	u16 sys_clk_div;
+	u16 pix_clk_div;
+	u32 sys_clk_freq_hz;
+	u32 pix_clk_freq_hz;
 };
 
 /**
@@ -97,21 +97,21 @@ struct ccs_pll_branch_bk {
  */
 struct ccs_pll {
 	/* input values */
-	uint8_t bus_type;
-	uint8_t op_lanes;
-	uint8_t vt_lanes;
+	u8 bus_type;
+	u8 op_lanes;
+	u8 vt_lanes;
 	struct {
-		uint8_t lanes;
+		u8 lanes;
 	} csi2;
-	uint8_t binning_horizontal;
-	uint8_t binning_vertical;
-	uint8_t scale_m;
-	uint8_t scale_n;
-	uint8_t bits_per_pixel;
-	uint8_t op_bits_per_lane;
-	uint16_t flags;
-	uint32_t link_freq;
-	uint32_t ext_clk_freq_hz;
+	u8 binning_horizontal;
+	u8 binning_vertical;
+	u8 scale_m;
+	u8 scale_n;
+	u8 bits_per_pixel;
+	u8 op_bits_per_lane;
+	u16 flags;
+	u32 link_freq;
+	u32 ext_clk_freq_hz;
 
 	/* output values */
 	struct ccs_pll_branch_fr vt_fr;
@@ -119,8 +119,8 @@ struct ccs_pll {
 	struct ccs_pll_branch_fr op_fr;
 	struct ccs_pll_branch_bk op_bk;
 
-	uint32_t pixel_rate_csi;
-	uint32_t pixel_rate_pixel_array;
+	u32 pixel_rate_csi;
+	u32 pixel_rate_pixel_array;
 };
 
 /**
@@ -136,14 +136,14 @@ struct ccs_pll {
  * @max_pll_op_clk_freq_hz: Maximum PLL output clock frequency
  */
 struct ccs_pll_branch_limits_fr {
-	uint16_t min_pre_pll_clk_div;
-	uint16_t max_pre_pll_clk_div;
-	uint32_t min_pll_ip_clk_freq_hz;
-	uint32_t max_pll_ip_clk_freq_hz;
-	uint16_t min_pll_multiplier;
-	uint16_t max_pll_multiplier;
-	uint32_t min_pll_op_clk_freq_hz;
-	uint32_t max_pll_op_clk_freq_hz;
+	u16 min_pre_pll_clk_div;
+	u16 max_pre_pll_clk_div;
+	u32 min_pll_ip_clk_freq_hz;
+	u32 max_pll_ip_clk_freq_hz;
+	u16 min_pll_multiplier;
+	u16 max_pll_multiplier;
+	u32 min_pll_op_clk_freq_hz;
+	u32 max_pll_op_clk_freq_hz;
 };
 
 /**
@@ -159,14 +159,14 @@ struct ccs_pll_branch_limits_fr {
  * @max_pix_clk_freq_hz: Maximum pixel clock frequency
  */
 struct ccs_pll_branch_limits_bk {
-	uint16_t min_sys_clk_div;
-	uint16_t max_sys_clk_div;
-	uint32_t min_sys_clk_freq_hz;
-	uint32_t max_sys_clk_freq_hz;
-	uint16_t min_pix_clk_div;
-	uint16_t max_pix_clk_div;
-	uint32_t min_pix_clk_freq_hz;
-	uint32_t max_pix_clk_freq_hz;
+	u16 min_sys_clk_div;
+	u16 max_sys_clk_div;
+	u32 min_sys_clk_freq_hz;
+	u32 max_sys_clk_freq_hz;
+	u16 min_pix_clk_div;
+	u16 max_pix_clk_div;
+	u32 min_pix_clk_freq_hz;
+	u32 max_pix_clk_freq_hz;
 };
 
 /**
@@ -183,8 +183,8 @@ struct ccs_pll_branch_limits_bk {
  */
 struct ccs_pll_limits {
 	/* Strict PLL limits */
-	uint32_t min_ext_clk_freq_hz;
-	uint32_t max_ext_clk_freq_hz;
+	u32 min_ext_clk_freq_hz;
+	u32 max_ext_clk_freq_hz;
 
 	struct ccs_pll_branch_limits_fr vt_fr;
 	struct ccs_pll_branch_limits_bk vt_bk;
@@ -192,8 +192,8 @@ struct ccs_pll_limits {
 	struct ccs_pll_branch_limits_bk op_bk;
 
 	/* Other relevant limits */
-	uint32_t min_line_length_pck_bin;
-	uint32_t min_line_length_pck;
+	u32 min_line_length_pck_bin;
+	u32 min_line_length_pck;
 };
 
 struct device;
-- 
2.29.2

[PATCH 23/24] ccs: Switch from standard integer types to kernel ones

[Sakari Ailus]

The preferred integer types in the kernel are the Linux specific ones,
switch from standard C types to u32 and alike.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ccs/ccs-core.c       |  2 +-
 drivers/media/i2c/ccs/ccs-reg-access.c | 21 ++++++++++-----------
 drivers/media/i2c/ccs/ccs.h            |  6 +++---
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index a8f591c95bc2..15afbb4f5b31 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -1190,7 +1190,7 @@ static void ccs_update_blanking(struct ccs_sensor *sensor)
 {
 	struct v4l2_ctrl *vblank = sensor->vblank;
 	struct v4l2_ctrl *hblank = sensor->hblank;
-	uint16_t min_fll, max_fll, min_llp, max_llp, min_lbp;
+	u16 min_fll, max_fll, min_llp, max_llp, min_lbp;
 	int min, max;
 
 	if (sensor->binning_vertical > 1 || sensor->binning_horizontal > 1) {
diff --git a/drivers/media/i2c/ccs/ccs-reg-access.c b/drivers/media/i2c/ccs/ccs-reg-access.c
index 5f0705952198..25993445f4fe 100644
--- a/drivers/media/i2c/ccs/ccs-reg-access.c
+++ b/drivers/media/i2c/ccs/ccs-reg-access.c
@@ -17,11 +17,10 @@
 #include "ccs.h"
 #include "ccs-limits.h"
 
-static uint32_t float_to_u32_mul_1000000(struct i2c_client *client,
-					 uint32_t phloat)
+static u32 float_to_u32_mul_1000000(struct i2c_client *client, u32 phloat)
 {
-	int32_t exp;
-	uint64_t man;
+	s32 exp;
+	u64 man;
 
 	if (phloat >= 0x80000000) {
 		dev_err(&client->dev, "this is a negative number\n");
@@ -137,11 +136,11 @@ static int ____ccs_read_addr_8only(struct ccs_sensor *sensor, u16 reg,
 unsigned int ccs_reg_width(u32 reg)
 {
 	if (reg & CCS_FL_16BIT)
-		return sizeof(uint16_t);
+		return sizeof(u16);
 	if (reg & CCS_FL_32BIT)
-		return sizeof(uint32_t);
+		return sizeof(u32);
 
-	return sizeof(uint8_t);
+	return sizeof(u8);
 }
 
 static u32 ireal32_to_u32_mul_1000000(struct i2c_client *client, u32 val)
@@ -205,7 +204,7 @@ static int __ccs_read_data(struct ccs_reg *regs, size_t num_regs,
 	size_t i;
 
 	for (i = 0; i < num_regs; i++, regs++) {
-		uint8_t *data;
+		u8 *data;
 
 		if (regs->addr + regs->len < CCS_REG_ADDR(reg) + width)
 			continue;
@@ -216,13 +215,13 @@ static int __ccs_read_data(struct ccs_reg *regs, size_t num_regs,
 		data = &regs->value[CCS_REG_ADDR(reg) - regs->addr];
 
 		switch (width) {
-		case sizeof(uint8_t):
+		case sizeof(u8):
 			*val = *data;
 			break;
-		case sizeof(uint16_t):
+		case sizeof(u16):
 			*val = get_unaligned_be16(data);
 			break;
-		case sizeof(uint32_t):
+		case sizeof(u32):
 			*val = get_unaligned_be32(data);
 			break;
 		default:
diff --git a/drivers/media/i2c/ccs/ccs.h b/drivers/media/i2c/ccs/ccs.h
index cc33c9ba3165..6beac375cc48 100644
--- a/drivers/media/i2c/ccs/ccs.h
+++ b/drivers/media/i2c/ccs/ccs.h
@@ -84,11 +84,11 @@ struct ccs_hwconfig {
 	unsigned short i2c_addr_dfl;	/* Default i2c addr */
 	unsigned short i2c_addr_alt;	/* Alternate i2c addr */
 
-	uint32_t ext_clk;		/* sensor external clk */
+	u32 ext_clk;			/* sensor external clk */
 
 	unsigned int lanes;		/* Number of CSI-2 lanes */
-	uint32_t csi_signalling_mode;	/* CCS_CSI_SIGNALLING_MODE_* */
-	uint64_t *op_sys_clock;
+	u32 csi_signalling_mode;	/* CCS_CSI_SIGNALLING_MODE_* */
+	u64 *op_sys_clock;
 
 	enum ccs_module_board_orient module_board_orient;
 
-- 
2.29.2

[PATCH 24/24] Revert "media: ccs-pll: Fix MODULE_LICENSE"

[Sakari Ailus]

This reverts commit b3c0115e34adcabe12fce8845e24ca6f04c1554e.

As per Documentation/process/license-rules.rst "GPL v2" exists only for
historical reasons and has the same meaning as "GPL". So revert this
patch.

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

diff --git a/drivers/media/i2c/ccs-pll.c b/drivers/media/i2c/ccs-pll.c
index 530085693a56..aeb87ba6fe37 100644
--- a/drivers/media/i2c/ccs-pll.c
+++ b/drivers/media/i2c/ccs-pll.c
@@ -883,4 +883,4 @@ EXPORT_SYMBOL_GPL(ccs_pll_calculate);
 
 MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>");
 MODULE_DESCRIPTION("Generic MIPI CCS/SMIA/SMIA++ PLL calculator");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
-- 
2.29.2

[PATCH v1.1 07/24] v4l: uapi: ccs: Add controls for analogue gain constants

[Sakari Ailus]

Add a V4L2 controls for analogue gai constants required to control
analogue gain. The values are device specific and thus need to be obtained
from the driver.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
since v1:

- Include v4l2-controls.h instead of videodev2.h.

 .../userspace-api/media/drivers/ccs.rst       | 25 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 include/uapi/linux/ccs.h                      | 14 +++++++++++
 3 files changed, 40 insertions(+)
 create mode 100644 include/uapi/linux/ccs.h

diff --git a/Documentation/userspace-api/media/drivers/ccs.rst b/Documentation/userspace-api/media/drivers/ccs.rst
index 00bb3a6288f5..a95d7941533d 100644
--- a/Documentation/userspace-api/media/drivers/ccs.rst
+++ b/Documentation/userspace-api/media/drivers/ccs.rst
@@ -56,4 +56,29 @@ analogue data is never read from the pixel matrix that are outside the
 configured selection rectangle that designates crop. The difference has an
 effect in device timing and likely also in power consumption.
 
+Private controls
+----------------
+
+The MIPI CCS driver implements a number of private controls under
+``V4L2_CID_USER_BASE_CCS`` to control the MIPI CCS compliant camera sensors.
+
+Analogue gain model
+~~~~~~~~~~~~~~~~~~~
+
+The CCS defines an analogue gain model where the gain can be calculated using
+the following formula:
+
+	gain = m0 * x + c0 / (m1 * x + c1)
+
+Either m0 or c0 will be zero. The constants that are device specific, can be
+obtained from the following controls:
+
+	V4L2_CID_CCS_ANALOGUE_GAIN_M0
+	V4L2_CID_CCS_ANALOGUE_GAIN_M1
+	V4L2_CID_CCS_ANALOGUE_GAIN_C0
+	V4L2_CID_CCS_ANALOGUE_GAIN_C1
+
+The analogue gain (``x`` in the formula) is controlled through
+``V4L2_CID_ANALOGUE_GAIN`` in this case.
+
 **Copyright** |copy| 2020 Intel Corporation
diff --git a/MAINTAINERS b/MAINTAINERS
index ab0bf9b8dd72..bd6ee5c746f5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11648,6 +11648,7 @@ F:	Documentation/userspace-api/media/drivers/ccs.rst
 F:	drivers/media/i2c/ccs-pll.c
 F:	drivers/media/i2c/ccs-pll.h
 F:	drivers/media/i2c/ccs/
+F:	include/uapi/linux/ccs.h
 F:	include/uapi/linux/smiapp.h
 
 MIPS
diff --git a/include/uapi/linux/ccs.h b/include/uapi/linux/ccs.h
new file mode 100644
index 000000000000..57515ed7aaab
--- /dev/null
+++ b/include/uapi/linux/ccs.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
+/* Copyright (C) 2020 Intel Corporation */
+
+#ifndef __UAPI_CCS_H__
+#define __UAPI_CCS_H__
+
+#include <linux/v4l2-controls.h>
+
+#define V4L2_CID_CCS_ANALOGUE_GAIN_M0		(V4L2_CID_USER_CCS_BASE + 1)
+#define V4L2_CID_CCS_ANALOGUE_GAIN_C0		(V4L2_CID_USER_CCS_BASE + 2)
+#define V4L2_CID_CCS_ANALOGUE_GAIN_M1		(V4L2_CID_USER_CCS_BASE + 3)
+#define V4L2_CID_CCS_ANALOGUE_GAIN_C1		(V4L2_CID_USER_CCS_BASE + 4)
+
+#endif
-- 
2.29.2

Re: [PATCH 17/24] ccs: Wait until software reset is done

[Mauro Carvalho Chehab]

Em Mon,  7 Dec 2020 23:15:23 +0200
Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:

> Verify the software reset has been completed until proceeding.
> 
> The spec does not guarantee a delay but presumably 100 ms should be
> enough.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/i2c/ccs/ccs-core.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
> index fdf2e83eeac3..e1b3c5693e01 100644
> --- a/drivers/media/i2c/ccs/ccs-core.c
> +++ b/drivers/media/i2c/ccs/ccs-core.c
> @@ -1553,11 +1553,26 @@ static int ccs_power_on(struct device *dev)
>  	 */
>  
>  	if (!sensor->reset && !sensor->xshutdown) {
> +		u8 retry = 100;
> +		u32 reset;
> +
>  		rval = ccs_write(sensor, SOFTWARE_RESET, CCS_SOFTWARE_RESET_ON);
>  		if (rval < 0) {
>  			dev_err(dev, "software reset failed\n");
>  			goto out_cci_addr_fail;
>  		}
> +
> +		do {
> +			rval = ccs_read(sensor, SOFTWARE_RESET, &reset);
> +			reset = !rval && reset == CCS_SOFTWARE_RESET_OFF;
> +			if (reset)
> +				break;
> +
> +			usleep_range(1000, 2000);
> +		} while (--retry);

Hmm... the way it is, the loop will wait for some time between
100ms and 200ms. Based on past experiences with non-deterministic
sleep times, this can be hard to debug if some device would require
to wait for a value between 100ms and 200ms.

So, I would, instead, make the retry time more deterministic, by
using time_is_after_jiffies(), e. g.:

	end = jiffies + msecs_to_jiffies(retry);
	do {
		rval = ccs_read(sensor, SOFTWARE_RESET, &reset);
		reset = !rval && reset == CCS_SOFTWARE_RESET_OFF;
		if (reset)
			break;

		usleep_range(1000, 2000);
	} while (time_is_after_jiffies(end));

In any case, I'm taking this patch, but it seems worth to change
to something like the above on a followup patch.

> +
> +		if (!reset)
> +			return -EIO;
>  	}
>  
>  	if (sensor->hwcfg.i2c_addr_alt) {



Thanks,
Mauro

Re: [PATCH 17/24] ccs: Wait until software reset is done

[Sakari Ailus]

Hi Mauro,

On Tue, Jan 12, 2021 at 05:49:48PM +0100, Mauro Carvalho Chehab wrote:
> Em Mon,  7 Dec 2020 23:15:23 +0200
> Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:
> 
> > Verify the software reset has been completed until proceeding.
> > 
> > The spec does not guarantee a delay but presumably 100 ms should be
> > enough.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/media/i2c/ccs/ccs-core.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
> > index fdf2e83eeac3..e1b3c5693e01 100644
> > --- a/drivers/media/i2c/ccs/ccs-core.c
> > +++ b/drivers/media/i2c/ccs/ccs-core.c
> > @@ -1553,11 +1553,26 @@ static int ccs_power_on(struct device *dev)
> >  	 */
> >  
> >  	if (!sensor->reset && !sensor->xshutdown) {
> > +		u8 retry = 100;
> > +		u32 reset;
> > +
> >  		rval = ccs_write(sensor, SOFTWARE_RESET, CCS_SOFTWARE_RESET_ON);
> >  		if (rval < 0) {
> >  			dev_err(dev, "software reset failed\n");
> >  			goto out_cci_addr_fail;
> >  		}
> > +
> > +		do {
> > +			rval = ccs_read(sensor, SOFTWARE_RESET, &reset);
> > +			reset = !rval && reset == CCS_SOFTWARE_RESET_OFF;
> > +			if (reset)
> > +				break;
> > +
> > +			usleep_range(1000, 2000);
> > +		} while (--retry);
> 
> Hmm... the way it is, the loop will wait for some time between
> 100ms and 200ms. Based on past experiences with non-deterministic
> sleep times, this can be hard to debug if some device would require
> to wait for a value between 100ms and 200ms.
> 
> So, I would, instead, make the retry time more deterministic, by
> using time_is_after_jiffies(), e. g.:
> 
> 	end = jiffies + msecs_to_jiffies(retry);
> 	do {
> 		rval = ccs_read(sensor, SOFTWARE_RESET, &reset);
> 		reset = !rval && reset == CCS_SOFTWARE_RESET_OFF;
> 		if (reset)
> 			break;
> 
> 		usleep_range(1000, 2000);
> 	} while (time_is_after_jiffies(end));
> 
> In any case, I'm taking this patch, but it seems worth to change
> to something like the above on a followup patch.

Thanks for the suggestion. I agree.

I'll address this soon.

-- 
Sakari Ailus