[PATCH 1/2] ARM: dts: exynos: correct s5k6a3 reset polarity on Midas family

[PATCH 1/2] ARM: dts: exynos: correct s5k6a3 reset polarity on Midas family

[Dmitry Torokhov]

According to s5k6a3 driver code, the reset line for the chip appears to
be active low. This also matches the typical polarity of reset lines in
general. Let's fix it up as having correct polarity in DTS is important
when the driver will be switched over to gpiod API.

Fixes: b4fec64758ab ("ARM: dts: Add camera device nodes for Exynos4412 TRATS2 board")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 arch/arm/boot/dts/exynos4412-midas.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos4412-midas.dtsi b/arch/arm/boot/dts/exynos4412-midas.dtsi
index b967397a46c5..8e1c19a8ad06 100644
--- a/arch/arm/boot/dts/exynos4412-midas.dtsi
+++ b/arch/arm/boot/dts/exynos4412-midas.dtsi
@@ -586,7 +586,7 @@ image-sensor@10 {
 		clocks = <&camera 1>;
 		clock-names = "extclk";
 		samsung,camclk-out = <1>;
-		gpios = <&gpm1 6 GPIO_ACTIVE_HIGH>;
+		gpios = <&gpm1 6 GPIO_ACTIVE_LOW>;
 
 		port {
 			is_s5k6a3_ep: endpoint {
-- 
2.37.2.789.g6183377224-goog

[PATCH 2/2] media: i2c: s5k6a3: switch to using gpiod API

[Dmitry Torokhov]

This patch switches the driver away from legacy gpio/of_gpio API to
gpiod API, and removes one of the last uses of of_get_gpio_flags().

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/media/i2c/s5k6a3.c | 30 +++++++++++-------------------
 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/media/i2c/s5k6a3.c b/drivers/media/i2c/s5k6a3.c
index a4efd6d10b43..ef6673b10580 100644
--- a/drivers/media/i2c/s5k6a3.c
+++ b/drivers/media/i2c/s5k6a3.c
@@ -9,12 +9,12 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/err.h>
 #include <linux/errno.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/of_gpio.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
@@ -59,7 +59,7 @@ struct s5k6a3 {
 	struct v4l2_subdev subdev;
 	struct media_pad pad;
 	struct regulator_bulk_data supplies[S5K6A3_NUM_SUPPLIES];
-	int gpio_reset;
+	struct gpio_desc *gpio_reset;
 	struct mutex lock;
 	struct v4l2_mbus_framefmt format;
 	struct clk *clock;
@@ -216,11 +216,11 @@ static int __s5k6a3_power_on(struct s5k6a3 *sensor)
 			goto error_clk;
 	}
 
-	gpio_set_value(sensor->gpio_reset, 1);
+	gpiod_set_value_cansleep(sensor->gpio_reset, 0);
 	usleep_range(600, 800);
-	gpio_set_value(sensor->gpio_reset, 0);
+	gpiod_set_value_cansleep(sensor->gpio_reset, 1);
 	usleep_range(600, 800);
-	gpio_set_value(sensor->gpio_reset, 1);
+	gpiod_set_value_cansleep(sensor->gpio_reset, 0);
 
 	/* Delay needed for the sensor initialization */
 	msleep(20);
@@ -240,7 +240,7 @@ static int __s5k6a3_power_off(struct s5k6a3 *sensor)
 {
 	int i;
 
-	gpio_set_value(sensor->gpio_reset, 0);
+	gpiod_set_value_cansleep(sensor->gpio_reset, 1);
 
 	for (i = S5K6A3_NUM_SUPPLIES - 1; i >= 0; i--)
 		regulator_disable(sensor->supplies[i].consumer);
@@ -285,32 +285,24 @@ static int s5k6a3_probe(struct i2c_client *client)
 	struct device *dev = &client->dev;
 	struct s5k6a3 *sensor;
 	struct v4l2_subdev *sd;
-	int gpio, i, ret;
+	int i, ret;
 
 	sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
 	if (!sensor)
 		return -ENOMEM;
 
 	mutex_init(&sensor->lock);
-	sensor->gpio_reset = -EINVAL;
-	sensor->clock = ERR_PTR(-EINVAL);
 	sensor->dev = dev;
 
 	sensor->clock = devm_clk_get(sensor->dev, S5K6A3_CLK_NAME);
 	if (IS_ERR(sensor->clock))
 		return PTR_ERR(sensor->clock);
 
-	gpio = of_get_gpio_flags(dev->of_node, 0, NULL);
-	if (!gpio_is_valid(gpio))
-		return gpio;
-
-	ret = devm_gpio_request_one(dev, gpio, GPIOF_OUT_INIT_LOW,
-						S5K6A3_DRV_NAME);
-	if (ret < 0)
+	sensor->gpio_reset = devm_gpiod_get(dev, NULL, GPIOD_OUT_HIGH);
+	ret = PTR_ERR_OR_ZERO(sensor->gpio_reset);
+	if (ret)
 		return ret;
 
-	sensor->gpio_reset = gpio;
-
 	if (of_property_read_u32(dev->of_node, "clock-frequency",
 				 &sensor->clock_frequency)) {
 		sensor->clock_frequency = S5K6A3_DEFAULT_CLK_FREQ;
-- 
2.37.2.789.g6183377224-goog

Re: [PATCH 1/2] ARM: dts: exynos: correct s5k6a3 reset polarity on Midas family

[Linus Walleij]

On Tue, Sep 13, 2022 at 6:41 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> According to s5k6a3 driver code, the reset line for the chip appears to
> be active low. This also matches the typical polarity of reset lines in
> general. Let's fix it up as having correct polarity in DTS is important
> when the driver will be switched over to gpiod API.
>
> Fixes: b4fec64758ab ("ARM: dts: Add camera device nodes for Exynos4412 TRATS2 board")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Excellent catch!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 2/2] media: i2c: s5k6a3: switch to using gpiod API

[Linus Walleij]

On Tue, Sep 13, 2022 at 6:41 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> This patch switches the driver away from legacy gpio/of_gpio API to
> gpiod API, and removes one of the last uses of of_get_gpio_flags().
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: (subset) [PATCH 1/2] ARM: dts: exynos: correct s5k6a3 reset polarity on Midas family

[Krzysztof Kozlowski]

On Tue, 13 Sep 2022 09:41:03 -0700, Dmitry Torokhov wrote:
> According to s5k6a3 driver code, the reset line for the chip appears to
> be active low. This also matches the typical polarity of reset lines in
> general. Let's fix it up as having correct polarity in DTS is important
> when the driver will be switched over to gpiod API.
> 
> 

Applied, thanks!

[1/2] ARM: dts: exynos: correct s5k6a3 reset polarity on Midas family
      https://git.kernel.org/krzk/linux/c/f539422ddaff0680dd1d4ad94df11be4703ccc93

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Re: [PATCH 2/2] media: i2c: s5k6a3: switch to using gpiod API

[Marek Szyprowski]

Hi,

On 13.09.2022 18:41, Dmitry Torokhov wrote:
> This patch switches the driver away from legacy gpio/of_gpio API to
> gpiod API, and removes one of the last uses of of_get_gpio_flags().
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

Thanks!

> ---
>   drivers/media/i2c/s5k6a3.c | 30 +++++++++++-------------------
>   1 file changed, 11 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/media/i2c/s5k6a3.c b/drivers/media/i2c/s5k6a3.c
> index a4efd6d10b43..ef6673b10580 100644
> --- a/drivers/media/i2c/s5k6a3.c
> +++ b/drivers/media/i2c/s5k6a3.c
> @@ -9,12 +9,12 @@
>   #include <linux/clk.h>
>   #include <linux/delay.h>
>   #include <linux/device.h>
> +#include <linux/err.h>
>   #include <linux/errno.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>   #include <linux/i2c.h>
>   #include <linux/kernel.h>
>   #include <linux/module.h>
> -#include <linux/of_gpio.h>
>   #include <linux/pm_runtime.h>
>   #include <linux/regulator/consumer.h>
>   #include <linux/slab.h>
> @@ -59,7 +59,7 @@ struct s5k6a3 {
>   	struct v4l2_subdev subdev;
>   	struct media_pad pad;
>   	struct regulator_bulk_data supplies[S5K6A3_NUM_SUPPLIES];
> -	int gpio_reset;
> +	struct gpio_desc *gpio_reset;
>   	struct mutex lock;
>   	struct v4l2_mbus_framefmt format;
>   	struct clk *clock;
> @@ -216,11 +216,11 @@ static int __s5k6a3_power_on(struct s5k6a3 *sensor)
>   			goto error_clk;
>   	}
>   
> -	gpio_set_value(sensor->gpio_reset, 1);
> +	gpiod_set_value_cansleep(sensor->gpio_reset, 0);
>   	usleep_range(600, 800);
> -	gpio_set_value(sensor->gpio_reset, 0);
> +	gpiod_set_value_cansleep(sensor->gpio_reset, 1);
>   	usleep_range(600, 800);
> -	gpio_set_value(sensor->gpio_reset, 1);
> +	gpiod_set_value_cansleep(sensor->gpio_reset, 0);
>   
>   	/* Delay needed for the sensor initialization */
>   	msleep(20);
> @@ -240,7 +240,7 @@ static int __s5k6a3_power_off(struct s5k6a3 *sensor)
>   {
>   	int i;
>   
> -	gpio_set_value(sensor->gpio_reset, 0);
> +	gpiod_set_value_cansleep(sensor->gpio_reset, 1);
>   
>   	for (i = S5K6A3_NUM_SUPPLIES - 1; i >= 0; i--)
>   		regulator_disable(sensor->supplies[i].consumer);
> @@ -285,32 +285,24 @@ static int s5k6a3_probe(struct i2c_client *client)
>   	struct device *dev = &client->dev;
>   	struct s5k6a3 *sensor;
>   	struct v4l2_subdev *sd;
> -	int gpio, i, ret;
> +	int i, ret;
>   
>   	sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
>   	if (!sensor)
>   		return -ENOMEM;
>   
>   	mutex_init(&sensor->lock);
> -	sensor->gpio_reset = -EINVAL;
> -	sensor->clock = ERR_PTR(-EINVAL);
>   	sensor->dev = dev;
>   
>   	sensor->clock = devm_clk_get(sensor->dev, S5K6A3_CLK_NAME);
>   	if (IS_ERR(sensor->clock))
>   		return PTR_ERR(sensor->clock);
>   
> -	gpio = of_get_gpio_flags(dev->of_node, 0, NULL);
> -	if (!gpio_is_valid(gpio))
> -		return gpio;
> -
> -	ret = devm_gpio_request_one(dev, gpio, GPIOF_OUT_INIT_LOW,
> -						S5K6A3_DRV_NAME);
> -	if (ret < 0)
> +	sensor->gpio_reset = devm_gpiod_get(dev, NULL, GPIOD_OUT_HIGH);
> +	ret = PTR_ERR_OR_ZERO(sensor->gpio_reset);
> +	if (ret)
>   		return ret;
>   
> -	sensor->gpio_reset = gpio;
> -
>   	if (of_property_read_u32(dev->of_node, "clock-frequency",
>   				 &sensor->clock_frequency)) {
>   		sensor->clock_frequency = S5K6A3_DEFAULT_CLK_FREQ;

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland