[PATCH 1/7] staging: atomisp: Fix calling efivar_entry_get() with unaligned arguments

[PATCH 1/7] staging: atomisp: Fix calling efivar_entry_get() with unaligned arguments

[Hans de Goede]

efivar_entry_get has certain alignment requirements and the atomisp
platform code was not honoring these, causing an oops by triggering the
WARN_ON in arch/x86/platform/efi/efi_64.c: virt_to_phys_or_null_size().

This commit fixes this by using the members of the efivar struct embedded
in the efivar_entry struct we kzalloc as arguments to efivar_entry_get(),
which is how all the other callers of efivar_entry_get() do this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../atomisp/platform/intel-mid/atomisp_gmin_platform.c  | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
index 5b4506a71126..104fea2f8697 100644
--- a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
@@ -623,9 +623,7 @@ int gmin_get_config_var(struct device *dev, const char *var, char *out, size_t *
 	char var8[CFG_VAR_NAME_MAX];
 	efi_char16_t var16[CFG_VAR_NAME_MAX];
 	struct efivar_entry *ev;
-	u32 efiattr_dummy;
 	int i, j, ret;
-	unsigned long efilen;
 
         if (dev && ACPI_COMPANION(dev))
                 dev = &ACPI_COMPANION(dev)->dev;
@@ -684,15 +682,18 @@ int gmin_get_config_var(struct device *dev, const char *var, char *out, size_t *
 		return -ENOMEM;
 	memcpy(&ev->var.VariableName, var16, sizeof(var16));
 	ev->var.VendorGuid = GMIN_CFG_VAR_EFI_GUID;
+	ev->var.DataSize = *out_len;
 
-	efilen = *out_len;
-	ret = efivar_entry_get(ev, &efiattr_dummy, &efilen, out);
+	ret = efivar_entry_get(ev, &ev->var.Attributes,
+			       &ev->var.DataSize, ev->var.Data);
+	if (ret == 0) {
+		memcpy(out, ev->var.Data, ev->var.DataSize);
+		*out_len = ev->var.DataSize;
+	} else {
+		dev_warn(dev, "Failed to find gmin variable %s\n", var8);
+	}
 
 	kfree(ev);
-	*out_len = efilen;
-
-	if (ret)
- 		dev_warn(dev, "Failed to find gmin variable %s\n", var8);
 
 	return ret;
 }
-- 
2.13.0

[PATCH 2/7] staging: atomisp: Do not call dev_warn with a NULL device

[Hans de Goede]

Do not call dev_warn with a NULL device, this silence the following 2
warnings:

[   14.392194] (NULL device *): Failed to find gmin variable gmin_V2P8GPIO
[   14.392257] (NULL device *): Failed to find gmin variable gmin_V1P8GPIO

We could switch to using pr_warn for dev == NULL instead, but as comments
in the source indicate, the check for these 2 special gmin variables with
a NULL device is a workaround for 2 specific evaluation boards, so
completely silencing the missing warning for these actually is a good
thing.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
index 104fea2f8697..3fea81ea5dbd 100644
--- a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c
@@ -689,7 +689,7 @@ int gmin_get_config_var(struct device *dev, const char *var, char *out, size_t *
 	if (ret == 0) {
 		memcpy(out, ev->var.Data, ev->var.DataSize);
 		*out_len = ev->var.DataSize;
-	} else {
+	} else if (dev) {
 		dev_warn(dev, "Failed to find gmin variable %s\n", var8);
 	}
 
-- 
2.13.0

[PATCH 3/7] staging: atomisp: Set step to 0 for mt9m114 menu control

[Hans de Goede]

menu controls are not allowed to have a step size, set step to 0 to
fix an oops from the WARN_ON in v4l2_ctrl_new_custom() triggering
because of this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/i2c/mt9m114.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/i2c/mt9m114.c b/drivers/staging/media/atomisp/i2c/mt9m114.c
index ced175c268d1..3fa915313e53 100644
--- a/drivers/staging/media/atomisp/i2c/mt9m114.c
+++ b/drivers/staging/media/atomisp/i2c/mt9m114.c
@@ -1499,7 +1499,7 @@ static struct v4l2_ctrl_config mt9m114_controls[] = {
 	 .type = V4L2_CTRL_TYPE_MENU,
 	 .min = 0,
 	 .max = 3,
-	 .step = 1,
+	 .step = 0,
 	 .def = 1,
 	 .flags = 0,
 	 },
-- 
2.13.0

[PATCH 4/7] staging: atomisp: Add INT0310 ACPI id to gc0310 driver

[Hans de Goede]

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/i2c/gc0310.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/atomisp/i2c/gc0310.c b/drivers/staging/media/atomisp/i2c/gc0310.c
index 1ec616a15086..350fd7fd5b86 100644
--- a/drivers/staging/media/atomisp/i2c/gc0310.c
+++ b/drivers/staging/media/atomisp/i2c/gc0310.c
@@ -1455,6 +1455,7 @@ static int gc0310_probe(struct i2c_client *client,
 
 static struct acpi_device_id gc0310_acpi_match[] = {
 	{"XXGC0310"},
+	{"INT0310"},
 	{},
 };
 
-- 
2.13.0

[PATCH 5/7] staging: atomisp: Add OVTI2680 ACPI id to ov2680 driver

[Hans de Goede]

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/i2c/ov2680.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/atomisp/i2c/ov2680.c b/drivers/staging/media/atomisp/i2c/ov2680.c
index 566091035c64..449aa2aa276f 100644
--- a/drivers/staging/media/atomisp/i2c/ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/ov2680.c
@@ -1521,6 +1521,7 @@ static int ov2680_probe(struct i2c_client *client,
 
 static struct acpi_device_id ov2680_acpi_match[] = {
 	{"XXOV2680"},
+	{"OVTI2680"},
 	{},
 };
 MODULE_DEVICE_TABLE(acpi, ov2680_acpi_match);
-- 
2.13.0

[PATCH 6/7] staging: atomisp: Ignore errors from second gpio in ov2680 driver

[Hans de Goede]

As the existing comment in the driver indicates the sensor has only 1 pin,
but some boards may have 2 gpios defined and we toggle both as we we don't
know which one is the right one. However if the ACPI resources table
defines only 1 gpio (as expected) the gpio1_ctrl call will always fail,
causing the probing of the driver to file.

This commit ignore the return value of the gpio1_ctrl call, fixing this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/i2c/ov2680.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/ov2680.c b/drivers/staging/media/atomisp/i2c/ov2680.c
index 449aa2aa276f..6dd466558701 100644
--- a/drivers/staging/media/atomisp/i2c/ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/ov2680.c
@@ -885,11 +885,12 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
 	if (flag) {
 		ret = dev->platform_data->gpio0_ctrl(sd, 1);
 		usleep_range(10000, 15000);
-		ret |= dev->platform_data->gpio1_ctrl(sd, 1);
+		/* Ignore return from second gpio, it may not be there */
+		dev->platform_data->gpio1_ctrl(sd, 1);
 		usleep_range(10000, 15000);
 	} else {
-		ret = dev->platform_data->gpio1_ctrl(sd, 0);
-		ret |= dev->platform_data->gpio0_ctrl(sd, 0);
+		dev->platform_data->gpio1_ctrl(sd, 0);
+		ret = dev->platform_data->gpio0_ctrl(sd, 0);
 	}
 	return ret;
 }
-- 
2.13.0

[PATCH 7/7] staging: atomisp: Make ov2680 driver less chatty

[Hans de Goede]

There is no reason for all this printk spamming and certainly
not at an error log level.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/i2c/ov2680.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/ov2680.c b/drivers/staging/media/atomisp/i2c/ov2680.c
index 6dd466558701..3cabfe54c669 100644
--- a/drivers/staging/media/atomisp/i2c/ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/ov2680.c
@@ -1191,9 +1191,8 @@ static int ov2680_detect(struct i2c_client *client)
 					OV2680_SC_CMMN_SUB_ID, &high);
 	revision = (u8) high & 0x0f;
 
-	dev_err(&client->dev, "sensor_revision id  = 0x%x\n", id);
-	dev_err(&client->dev, "detect ov2680 success\n");
-	dev_err(&client->dev, "################5##########\n");
+	dev_info(&client->dev, "sensor_revision id = 0x%x\n", id);
+
 	return 0;
 }
 
@@ -1448,8 +1447,6 @@ static int ov2680_probe(struct i2c_client *client,
 	void *pdata;
 	unsigned int i;
 
-	printk("++++ov2680_probe++++\n");
-	dev_info(&client->dev, "++++ov2680_probe++++\n");
 	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
 	if (!dev) {
 		dev_err(&client->dev, "out of memory\n");
-- 
2.13.0

Re: [PATCH 2/7] staging: atomisp: Do not call dev_warn with a NULL device

[Andy Shevchenko]

On Sun, May 28, 2017 at 3:31 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Do not call dev_warn with a NULL device, this silence the following 2
> warnings:
>
> [   14.392194] (NULL device *): Failed to find gmin variable gmin_V2P8GPIO
> [   14.392257] (NULL device *): Failed to find gmin variable gmin_V1P8GPIO
>
> We could switch to using pr_warn for dev == NULL instead, but as comments
> in the source indicate, the check for these 2 special gmin variables with
> a NULL device is a workaround for 2 specific evaluation boards, so
> completely silencing the missing warning for these actually is a good
> thing.

Perhaps removing all code related explicitly to Gmin is a right thing to do.


-- 
With Best Regards,
Andy Shevchenko