* [PATCH 1/4] media: i2c: s5k6a3: switch to using gpiod API
@ 2022-11-15 22:11 Dmitry Torokhov
2022-11-15 22:11 ` [PATCH 2/4] media: i2c: s5k5baf: " Dmitry Torokhov
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2022-11-15 22:11 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sylwester Nawrocki, Andrzej Hajda, Linus Walleij, linux-kernel,
linux-media
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.38.1.431.g37b22c650d-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/4] media: i2c: s5k5baf: switch to using gpiod API
2022-11-15 22:11 [PATCH 1/4] media: i2c: s5k6a3: switch to using gpiod API Dmitry Torokhov
@ 2022-11-15 22:11 ` Dmitry Torokhov
2022-11-17 10:54 ` Tommaso Merciai
2022-11-15 22:11 ` [PATCH 3/4] media: i2c: s5c73m3: remove support for platform data Dmitry Torokhov
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Dmitry Torokhov @ 2022-11-15 22:11 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sylwester Nawrocki, Andrzej Hajda, Linus Walleij, linux-kernel,
linux-media
This patch switches the driver away from legacy gpio/of_gpio API to
gpiod API, and removes use of of_get_named_gpio_flags() which I want to
make private to gpiolib.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/media/i2c/s5k5baf.c | 64 +++++++++++--------------------------
1 file changed, 18 insertions(+), 46 deletions(-)
diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
index 5c2253ab3b6f..960fbf6428ea 100644
--- a/drivers/media/i2c/s5k5baf.c
+++ b/drivers/media/i2c/s5k5baf.c
@@ -13,11 +13,10 @@
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/firmware.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
#include <linux/media.h>
#include <linux/module.h>
-#include <linux/of_gpio.h>
#include <linux/of_graph.h>
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
@@ -228,11 +227,6 @@ static const char * const s5k5baf_supply_names[] = {
};
#define S5K5BAF_NUM_SUPPLIES ARRAY_SIZE(s5k5baf_supply_names)
-struct s5k5baf_gpio {
- int gpio;
- int level;
-};
-
enum s5k5baf_gpio_id {
STBY,
RSET,
@@ -284,7 +278,7 @@ struct s5k5baf_fw {
};
struct s5k5baf {
- struct s5k5baf_gpio gpios[NUM_GPIOS];
+ struct gpio_desc *gpios[NUM_GPIOS];
enum v4l2_mbus_type bus_type;
u8 nlanes;
struct regulator_bulk_data supplies[S5K5BAF_NUM_SUPPLIES];
@@ -936,16 +930,12 @@ static void s5k5baf_hw_set_test_pattern(struct s5k5baf *state, int id)
static void s5k5baf_gpio_assert(struct s5k5baf *state, int id)
{
- struct s5k5baf_gpio *gpio = &state->gpios[id];
-
- gpio_set_value(gpio->gpio, gpio->level);
+ gpiod_set_value_cansleep(state->gpios[id], 1);
}
static void s5k5baf_gpio_deassert(struct s5k5baf *state, int id)
{
- struct s5k5baf_gpio *gpio = &state->gpios[id];
-
- gpio_set_value(gpio->gpio, !gpio->level);
+ gpiod_set_value_cansleep(state->gpios[id], 0);
}
static int s5k5baf_power_on(struct s5k5baf *state)
@@ -1799,44 +1789,30 @@ static const struct v4l2_subdev_ops s5k5baf_subdev_ops = {
static int s5k5baf_configure_gpios(struct s5k5baf *state)
{
- static const char * const name[] = { "S5K5BAF_STBY", "S5K5BAF_RST" };
+ static const char * const name[] = { "stbyn", "rstn" };
+ static const char * const label[] = { "S5K5BAF_STBY", "S5K5BAF_RST" };
struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
- struct s5k5baf_gpio *g = state->gpios;
+ struct gpio_desc *gpio;
int ret, i;
for (i = 0; i < NUM_GPIOS; ++i) {
- int flags = GPIOF_DIR_OUT;
- if (g[i].level)
- flags |= GPIOF_INIT_HIGH;
- ret = devm_gpio_request_one(&c->dev, g[i].gpio, flags, name[i]);
- if (ret < 0) {
- v4l2_err(c, "failed to request gpio %s\n", name[i]);
+ gpio = devm_gpiod_get(&c->dev, name[i], GPIOD_OUT_HIGH);
+ ret = PTR_ERR_OR_ZERO(gpio);
+ if (ret) {
+ v4l2_err(c, "failed to request gpio %s: %d\n",
+ name[i], ret);
return ret;
}
- }
- return 0;
-}
-
-static int s5k5baf_parse_gpios(struct s5k5baf_gpio *gpios, struct device *dev)
-{
- static const char * const names[] = {
- "stbyn-gpios",
- "rstn-gpios",
- };
- struct device_node *node = dev->of_node;
- enum of_gpio_flags flags;
- int ret, i;
- for (i = 0; i < NUM_GPIOS; ++i) {
- ret = of_get_named_gpio_flags(node, names[i], 0, &flags);
- if (ret < 0) {
- dev_err(dev, "no %s GPIO pin provided\n", names[i]);
+ ret = gpiod_set_consumer_name(gpio, label[i]);
+ if (ret) {
+ v4l2_err(c, "failed to set up name for gpio %s: %d\n",
+ name[i], ret);
return ret;
}
- gpios[i].gpio = ret;
- gpios[i].level = !(flags & OF_GPIO_ACTIVE_LOW);
- }
+ state->gpios[i] = gpio;
+ }
return 0;
}
@@ -1860,10 +1836,6 @@ static int s5k5baf_parse_device_node(struct s5k5baf *state, struct device *dev)
state->mclk_frequency);
}
- ret = s5k5baf_parse_gpios(state->gpios, dev);
- if (ret < 0)
- return ret;
-
node_ep = of_graph_get_next_endpoint(node, NULL);
if (!node_ep) {
dev_err(dev, "no endpoint defined at node %pOF\n", node);
--
2.38.1.431.g37b22c650d-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/4] media: i2c: s5c73m3: remove support for platform data
2022-11-15 22:11 [PATCH 1/4] media: i2c: s5k6a3: switch to using gpiod API Dmitry Torokhov
2022-11-15 22:11 ` [PATCH 2/4] media: i2c: s5k5baf: " Dmitry Torokhov
@ 2022-11-15 22:11 ` Dmitry Torokhov
2022-11-17 11:05 ` Tommaso Merciai
2022-11-28 8:39 ` Hans Verkuil
2022-11-15 22:11 ` [PATCH 4/4] media: i2c: s5c73m3: switch to using gpiod API Dmitry Torokhov
` (2 subsequent siblings)
4 siblings, 2 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2022-11-15 22:11 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sylwester Nawrocki, Andrzej Hajda, Linus Walleij, linux-kernel,
linux-media
There are no existing users of s5c73m3_platform_data in the tree, and
new users shoudl either be using device tree, ACPI, or static device
properties, so let's remove it from the driver.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/media/i2c/s5c73m3/s5c73m3-core.c | 19 ++------
drivers/media/i2c/s5c73m3/s5c73m3-ctrls.c | 2 -
drivers/media/i2c/s5c73m3/s5c73m3.h | 6 ++-
include/media/i2c/s5c73m3.h | 56 -----------------------
4 files changed, 9 insertions(+), 74 deletions(-)
delete mode 100644 include/media/i2c/s5c73m3.h
diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
index d96ba58ce1e5..561c1a1583ac 100644
--- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
+++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
@@ -27,7 +27,6 @@
#include <media/v4l2-device.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-mediabus.h>
-#include <media/i2c/s5c73m3.h>
#include <media/v4l2-fwnode.h>
#include "s5c73m3.h"
@@ -1592,26 +1591,16 @@ static int s5c73m3_parse_gpios(struct s5c73m3 *state)
return 0;
}
-static int s5c73m3_get_platform_data(struct s5c73m3 *state)
+static int s5c73m3_get_dt_data(struct s5c73m3 *state)
{
struct device *dev = &state->i2c_client->dev;
- const struct s5c73m3_platform_data *pdata = dev->platform_data;
struct device_node *node = dev->of_node;
struct device_node *node_ep;
struct v4l2_fwnode_endpoint ep = { .bus_type = 0 };
int ret;
- if (!node) {
- if (!pdata) {
- dev_err(dev, "Platform data not specified\n");
- return -EINVAL;
- }
-
- state->mclk_frequency = pdata->mclk_frequency;
- state->gpio[STBY] = pdata->gpio_stby;
- state->gpio[RSET] = pdata->gpio_reset;
- return 0;
- }
+ if (!node)
+ return -EINVAL;
state->clock = devm_clk_get(dev, S5C73M3_CLK_NAME);
if (IS_ERR(state->clock))
@@ -1666,7 +1655,7 @@ static int s5c73m3_probe(struct i2c_client *client)
return -ENOMEM;
state->i2c_client = client;
- ret = s5c73m3_get_platform_data(state);
+ ret = s5c73m3_get_dt_data(state);
if (ret < 0)
return ret;
diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-ctrls.c b/drivers/media/i2c/s5c73m3/s5c73m3-ctrls.c
index 141ad0ba7f5a..1c8103670fa2 100644
--- a/drivers/media/i2c/s5c73m3/s5c73m3-ctrls.c
+++ b/drivers/media/i2c/s5c73m3/s5c73m3-ctrls.c
@@ -10,7 +10,6 @@
#include <linux/sizes.h>
#include <linux/delay.h>
#include <linux/firmware.h>
-#include <linux/gpio.h>
#include <linux/i2c.h>
#include <linux/init.h>
#include <linux/media.h>
@@ -24,7 +23,6 @@
#include <media/v4l2-device.h>
#include <media/v4l2-subdev.h>
#include <media/v4l2-mediabus.h>
-#include <media/i2c/s5c73m3.h>
#include "s5c73m3.h"
diff --git a/drivers/media/i2c/s5c73m3/s5c73m3.h b/drivers/media/i2c/s5c73m3/s5c73m3.h
index c3fcfdd3ea66..d68528898249 100644
--- a/drivers/media/i2c/s5c73m3/s5c73m3.h
+++ b/drivers/media/i2c/s5c73m3/s5c73m3.h
@@ -15,7 +15,6 @@
#include <media/v4l2-common.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-subdev.h>
-#include <media/i2c/s5c73m3.h>
#define DRIVER_NAME "S5C73M3"
@@ -357,6 +356,11 @@ enum s5c73m3_gpio_id {
GPIO_NUM,
};
+struct s5c73m3_gpio {
+ int gpio;
+ int level;
+};
+
enum s5c73m3_resolution_types {
RES_ISP,
RES_JPEG,
diff --git a/include/media/i2c/s5c73m3.h b/include/media/i2c/s5c73m3.h
deleted file mode 100644
index a51f1025ba1c..000000000000
--- a/include/media/i2c/s5c73m3.h
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- * Samsung LSI S5C73M3 8M pixel camera driver
- *
- * Copyright (C) 2012, Samsung Electronics, Co., Ltd.
- * Sylwester Nawrocki <s.nawrocki@samsung.com>
- * Andrzej Hajda <a.hajda@samsung.com>
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * version 2 as published by the Free Software Foundation.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-#ifndef MEDIA_S5C73M3__
-#define MEDIA_S5C73M3__
-
-#include <linux/videodev2.h>
-#include <media/v4l2-mediabus.h>
-
-/**
- * struct s5c73m3_gpio - data structure describing a GPIO
- * @gpio: GPIO number
- * @level: indicates active state of the @gpio
- */
-struct s5c73m3_gpio {
- int gpio;
- int level;
-};
-
-/**
- * struct s5c73m3_platform_data - s5c73m3 driver platform data
- * @mclk_frequency: sensor's master clock frequency in Hz
- * @gpio_reset: GPIO driving RESET pin
- * @gpio_stby: GPIO driving STBY pin
- * @bus_type: bus type
- * @nlanes: maximum number of MIPI-CSI lanes used
- * @horiz_flip: default horizontal image flip value, non zero to enable
- * @vert_flip: default vertical image flip value, non zero to enable
- */
-
-struct s5c73m3_platform_data {
- unsigned long mclk_frequency;
-
- struct s5c73m3_gpio gpio_reset;
- struct s5c73m3_gpio gpio_stby;
-
- enum v4l2_mbus_type bus_type;
- u8 nlanes;
- u8 horiz_flip;
- u8 vert_flip;
-};
-
-#endif /* MEDIA_S5C73M3__ */
--
2.38.1.431.g37b22c650d-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/4] media: i2c: s5c73m3: switch to using gpiod API
2022-11-15 22:11 [PATCH 1/4] media: i2c: s5k6a3: switch to using gpiod API Dmitry Torokhov
2022-11-15 22:11 ` [PATCH 2/4] media: i2c: s5k5baf: " Dmitry Torokhov
2022-11-15 22:11 ` [PATCH 3/4] media: i2c: s5c73m3: remove support for platform data Dmitry Torokhov
@ 2022-11-15 22:11 ` Dmitry Torokhov
2022-11-17 9:06 ` Linus Walleij
2022-11-17 11:06 ` Tommaso Merciai
2022-11-17 9:04 ` [PATCH 1/4] media: i2c: s5k6a3: " Linus Walleij
2022-11-17 9:11 ` Tommaso Merciai
4 siblings, 2 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2022-11-15 22:11 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sylwester Nawrocki, Andrzej Hajda, Linus Walleij, linux-kernel,
linux-media
This patch switches the driver away from legacy gpio/of_gpio API to
gpiod API, and removes use of of_get_named_gpio_flags() which I want to
make private to gpiolib.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/media/i2c/s5c73m3/s5c73m3-core.c | 63 +++++++-----------------
drivers/media/i2c/s5c73m3/s5c73m3.h | 7 +--
2 files changed, 20 insertions(+), 50 deletions(-)
diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
index 561c1a1583ac..f1e073ed5f99 100644
--- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
+++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
@@ -10,12 +10,11 @@
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/firmware.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
#include <linux/init.h>
#include <linux/media.h>
#include <linux/module.h>
-#include <linux/of_gpio.h>
#include <linux/of_graph.h>
#include <linux/regulator/consumer.h>
#include <linux/sizes.h>
@@ -1348,20 +1347,20 @@ static int s5c73m3_oif_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
static int s5c73m3_gpio_set_value(struct s5c73m3 *priv, int id, u32 val)
{
- if (!gpio_is_valid(priv->gpio[id].gpio))
+ if (!priv->gpio[id])
return 0;
- gpio_set_value(priv->gpio[id].gpio, !!val);
+ gpiod_set_value_cansleep(priv->gpio[id], val);
return 1;
}
static int s5c73m3_gpio_assert(struct s5c73m3 *priv, int id)
{
- return s5c73m3_gpio_set_value(priv, id, priv->gpio[id].level);
+ return s5c73m3_gpio_set_value(priv, id, 1);
}
static int s5c73m3_gpio_deassert(struct s5c73m3 *priv, int id)
{
- return s5c73m3_gpio_set_value(priv, id, !priv->gpio[id].level);
+ return s5c73m3_gpio_set_value(priv, id, 0);
}
static int __s5c73m3_power_on(struct s5c73m3 *state)
@@ -1544,49 +1543,29 @@ static const struct v4l2_subdev_ops oif_subdev_ops = {
static int s5c73m3_configure_gpios(struct s5c73m3 *state)
{
- static const char * const gpio_names[] = {
- "S5C73M3_STBY", "S5C73M3_RST"
- };
+ static const char * const name[] = { "standby", "xshutdown" };
+ static const char * const label[] = { "S5C73M3_STBY", "S5C73M3_RST" };
struct i2c_client *c = state->i2c_client;
- struct s5c73m3_gpio *g = state->gpio;
+ struct gpio_desc *gpio;
int ret, i;
for (i = 0; i < GPIO_NUM; ++i) {
- unsigned int flags = GPIOF_DIR_OUT;
- if (g[i].level)
- flags |= GPIOF_INIT_HIGH;
- ret = devm_gpio_request_one(&c->dev, g[i].gpio, flags,
- gpio_names[i]);
+ gpio = devm_gpiod_get(&c->dev, name[i], GPIOD_OUT_HIGH);
+ ret = PTR_ERR_OR_ZERO(gpio);
if (ret) {
- v4l2_err(c, "failed to request gpio %s\n",
- gpio_names[i]);
+ v4l2_err(c, "failed to request gpio %s: %d\n",
+ name[i], ret);
return ret;
}
- }
- return 0;
-}
-
-static int s5c73m3_parse_gpios(struct s5c73m3 *state)
-{
- static const char * const prop_names[] = {
- "standby-gpios", "xshutdown-gpios",
- };
- struct device *dev = &state->i2c_client->dev;
- struct device_node *node = dev->of_node;
- int ret, i;
- for (i = 0; i < GPIO_NUM; ++i) {
- enum of_gpio_flags of_flags;
-
- ret = of_get_named_gpio_flags(node, prop_names[i],
- 0, &of_flags);
- if (ret < 0) {
- dev_err(dev, "failed to parse %s DT property\n",
- prop_names[i]);
- return -EINVAL;
+ ret = gpiod_set_consumer_name(gpio, label[i]);
+ if (ret) {
+ v4l2_err(c, "failed to set up name for gpio %s: %d\n",
+ name[i], ret);
+ return ret;
}
- state->gpio[i].gpio = ret;
- state->gpio[i].level = !(of_flags & OF_GPIO_ACTIVE_LOW);
+
+ state->gpio[i] = gpio;
}
return 0;
}
@@ -1613,10 +1592,6 @@ static int s5c73m3_get_dt_data(struct s5c73m3 *state)
state->mclk_frequency);
}
- ret = s5c73m3_parse_gpios(state);
- if (ret < 0)
- return -EINVAL;
-
node_ep = of_graph_get_next_endpoint(node, NULL);
if (!node_ep) {
dev_warn(dev, "no endpoint defined for node: %pOF\n", node);
diff --git a/drivers/media/i2c/s5c73m3/s5c73m3.h b/drivers/media/i2c/s5c73m3/s5c73m3.h
index d68528898249..9887d03fcdeb 100644
--- a/drivers/media/i2c/s5c73m3/s5c73m3.h
+++ b/drivers/media/i2c/s5c73m3/s5c73m3.h
@@ -356,11 +356,6 @@ enum s5c73m3_gpio_id {
GPIO_NUM,
};
-struct s5c73m3_gpio {
- int gpio;
- int level;
-};
-
enum s5c73m3_resolution_types {
RES_ISP,
RES_JPEG,
@@ -387,7 +382,7 @@ struct s5c73m3 {
u32 i2c_read_address;
struct regulator_bulk_data supplies[S5C73M3_MAX_SUPPLIES];
- struct s5c73m3_gpio gpio[GPIO_NUM];
+ struct gpio_desc *gpio[GPIO_NUM];
struct clk *clock;
--
2.38.1.431.g37b22c650d-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] media: i2c: s5k6a3: switch to using gpiod API
2022-11-15 22:11 [PATCH 1/4] media: i2c: s5k6a3: switch to using gpiod API Dmitry Torokhov
` (2 preceding siblings ...)
2022-11-15 22:11 ` [PATCH 4/4] media: i2c: s5c73m3: switch to using gpiod API Dmitry Torokhov
@ 2022-11-17 9:04 ` Linus Walleij
2022-11-17 9:11 ` Tommaso Merciai
4 siblings, 0 replies; 15+ messages in thread
From: Linus Walleij @ 2022-11-17 9:04 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mauro Carvalho Chehab, Sylwester Nawrocki, Andrzej Hajda,
linux-kernel, linux-media
On Tue, Nov 15, 2022 at 11:11 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>
> - sensor->gpio_reset = -EINVAL;
> - sensor->clock = ERR_PTR(-EINVAL);
Looks unrelated but makes sense.
> sensor->dev = dev;
>
> sensor->clock = devm_clk_get(sensor->dev, S5K6A3_CLK_NAME);
Given that it is initialized unconditionally two lines down :P
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] media: i2c: s5c73m3: switch to using gpiod API
2022-11-15 22:11 ` [PATCH 4/4] media: i2c: s5c73m3: switch to using gpiod API Dmitry Torokhov
@ 2022-11-17 9:06 ` Linus Walleij
2022-11-26 10:34 ` Hans Verkuil
2022-11-17 11:06 ` Tommaso Merciai
1 sibling, 1 reply; 15+ messages in thread
From: Linus Walleij @ 2022-11-17 9:06 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mauro Carvalho Chehab, Sylwester Nawrocki, Andrzej Hajda,
linux-kernel, linux-media
On Tue, Nov 15, 2022 at 11:11 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 use of of_get_named_gpio_flags() which I want to
> make private to gpiolib.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] media: i2c: s5k6a3: switch to using gpiod API
2022-11-15 22:11 [PATCH 1/4] media: i2c: s5k6a3: switch to using gpiod API Dmitry Torokhov
` (3 preceding siblings ...)
2022-11-17 9:04 ` [PATCH 1/4] media: i2c: s5k6a3: " Linus Walleij
@ 2022-11-17 9:11 ` Tommaso Merciai
4 siblings, 0 replies; 15+ messages in thread
From: Tommaso Merciai @ 2022-11-17 9:11 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mauro Carvalho Chehab, Sylwester Nawrocki, Andrzej Hajda,
Linus Walleij, linux-kernel, linux-media
Hi Dmitry,
On Tue, Nov 15, 2022 at 02:11:42PM -0800, 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>
> ---
> 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;
Patch looks good to me!
Reviewed-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
For the future I think would be nice use "reset-gpios" name in dts.
Then call:
devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
But this is a todo :)
Regards,
Tommaso
>
> - 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.38.1.431.g37b22c650d-goog
>
--
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] media: i2c: s5k5baf: switch to using gpiod API
2022-11-15 22:11 ` [PATCH 2/4] media: i2c: s5k5baf: " Dmitry Torokhov
@ 2022-11-17 10:54 ` Tommaso Merciai
0 siblings, 0 replies; 15+ messages in thread
From: Tommaso Merciai @ 2022-11-17 10:54 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mauro Carvalho Chehab, Sylwester Nawrocki, Andrzej Hajda,
Linus Walleij, linux-kernel, linux-media
Hi Dmitry,
On Tue, Nov 15, 2022 at 02:11:43PM -0800, Dmitry Torokhov wrote:
> This patch switches the driver away from legacy gpio/of_gpio API to
> gpiod API, and removes use of of_get_named_gpio_flags() which I want to
> make private to gpiolib.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/media/i2c/s5k5baf.c | 64 +++++++++++--------------------------
> 1 file changed, 18 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
> index 5c2253ab3b6f..960fbf6428ea 100644
> --- a/drivers/media/i2c/s5k5baf.c
> +++ b/drivers/media/i2c/s5k5baf.c
> @@ -13,11 +13,10 @@
> #include <linux/clk.h>
> #include <linux/delay.h>
> #include <linux/firmware.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/i2c.h>
> #include <linux/media.h>
> #include <linux/module.h>
> -#include <linux/of_gpio.h>
> #include <linux/of_graph.h>
> #include <linux/regulator/consumer.h>
> #include <linux/slab.h>
> @@ -228,11 +227,6 @@ static const char * const s5k5baf_supply_names[] = {
> };
> #define S5K5BAF_NUM_SUPPLIES ARRAY_SIZE(s5k5baf_supply_names)
>
> -struct s5k5baf_gpio {
> - int gpio;
> - int level;
> -};
> -
> enum s5k5baf_gpio_id {
> STBY,
> RSET,
> @@ -284,7 +278,7 @@ struct s5k5baf_fw {
> };
>
> struct s5k5baf {
> - struct s5k5baf_gpio gpios[NUM_GPIOS];
> + struct gpio_desc *gpios[NUM_GPIOS];
> enum v4l2_mbus_type bus_type;
> u8 nlanes;
> struct regulator_bulk_data supplies[S5K5BAF_NUM_SUPPLIES];
> @@ -936,16 +930,12 @@ static void s5k5baf_hw_set_test_pattern(struct s5k5baf *state, int id)
>
> static void s5k5baf_gpio_assert(struct s5k5baf *state, int id)
> {
> - struct s5k5baf_gpio *gpio = &state->gpios[id];
> -
> - gpio_set_value(gpio->gpio, gpio->level);
> + gpiod_set_value_cansleep(state->gpios[id], 1);
> }
>
> static void s5k5baf_gpio_deassert(struct s5k5baf *state, int id)
> {
> - struct s5k5baf_gpio *gpio = &state->gpios[id];
> -
> - gpio_set_value(gpio->gpio, !gpio->level);
> + gpiod_set_value_cansleep(state->gpios[id], 0);
> }
>
> static int s5k5baf_power_on(struct s5k5baf *state)
> @@ -1799,44 +1789,30 @@ static const struct v4l2_subdev_ops s5k5baf_subdev_ops = {
>
> static int s5k5baf_configure_gpios(struct s5k5baf *state)
> {
> - static const char * const name[] = { "S5K5BAF_STBY", "S5K5BAF_RST" };
> + static const char * const name[] = { "stbyn", "rstn" };
> + static const char * const label[] = { "S5K5BAF_STBY", "S5K5BAF_RST" };
> struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
> - struct s5k5baf_gpio *g = state->gpios;
> + struct gpio_desc *gpio;
> int ret, i;
>
> for (i = 0; i < NUM_GPIOS; ++i) {
> - int flags = GPIOF_DIR_OUT;
> - if (g[i].level)
> - flags |= GPIOF_INIT_HIGH;
> - ret = devm_gpio_request_one(&c->dev, g[i].gpio, flags, name[i]);
> - if (ret < 0) {
> - v4l2_err(c, "failed to request gpio %s\n", name[i]);
> + gpio = devm_gpiod_get(&c->dev, name[i], GPIOD_OUT_HIGH);
> + ret = PTR_ERR_OR_ZERO(gpio);
> + if (ret) {
> + v4l2_err(c, "failed to request gpio %s: %d\n",
> + name[i], ret);
> return ret;
> }
> - }
> - return 0;
> -}
> -
> -static int s5k5baf_parse_gpios(struct s5k5baf_gpio *gpios, struct device *dev)
> -{
> - static const char * const names[] = {
> - "stbyn-gpios",
> - "rstn-gpios",
> - };
> - struct device_node *node = dev->of_node;
> - enum of_gpio_flags flags;
> - int ret, i;
>
> - for (i = 0; i < NUM_GPIOS; ++i) {
> - ret = of_get_named_gpio_flags(node, names[i], 0, &flags);
> - if (ret < 0) {
> - dev_err(dev, "no %s GPIO pin provided\n", names[i]);
> + ret = gpiod_set_consumer_name(gpio, label[i]);
> + if (ret) {
> + v4l2_err(c, "failed to set up name for gpio %s: %d\n",
> + name[i], ret);
> return ret;
> }
> - gpios[i].gpio = ret;
> - gpios[i].level = !(flags & OF_GPIO_ACTIVE_LOW);
> - }
>
> + state->gpios[i] = gpio;
> + }
> return 0;
> }
>
> @@ -1860,10 +1836,6 @@ static int s5k5baf_parse_device_node(struct s5k5baf *state, struct device *dev)
> state->mclk_frequency);
> }
>
> - ret = s5k5baf_parse_gpios(state->gpios, dev);
> - if (ret < 0)
> - return ret;
> -
> node_ep = of_graph_get_next_endpoint(node, NULL);
> if (!node_ep) {
> dev_err(dev, "no endpoint defined at node %pOF\n", node);
> --
> 2.38.1.431.g37b22c650d-goog
>
Looks good to me.
Reviewed-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
Regards,
Tommaso
--
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] media: i2c: s5c73m3: remove support for platform data
2022-11-15 22:11 ` [PATCH 3/4] media: i2c: s5c73m3: remove support for platform data Dmitry Torokhov
@ 2022-11-17 11:05 ` Tommaso Merciai
2022-11-28 8:39 ` Hans Verkuil
1 sibling, 0 replies; 15+ messages in thread
From: Tommaso Merciai @ 2022-11-17 11:05 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mauro Carvalho Chehab, Sylwester Nawrocki, Andrzej Hajda,
Linus Walleij, linux-kernel, linux-media
On Tue, Nov 15, 2022 at 02:11:44PM -0800, Dmitry Torokhov wrote:
> There are no existing users of s5c73m3_platform_data in the tree, and
> new users shoudl either be using device tree, ACPI, or static device
> properties, so let's remove it from the driver.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/media/i2c/s5c73m3/s5c73m3-core.c | 19 ++------
> drivers/media/i2c/s5c73m3/s5c73m3-ctrls.c | 2 -
> drivers/media/i2c/s5c73m3/s5c73m3.h | 6 ++-
> include/media/i2c/s5c73m3.h | 56 -----------------------
> 4 files changed, 9 insertions(+), 74 deletions(-)
> delete mode 100644 include/media/i2c/s5c73m3.h
>
> diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> index d96ba58ce1e5..561c1a1583ac 100644
> --- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> +++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> @@ -27,7 +27,6 @@
> #include <media/v4l2-device.h>
> #include <media/v4l2-subdev.h>
> #include <media/v4l2-mediabus.h>
> -#include <media/i2c/s5c73m3.h>
> #include <media/v4l2-fwnode.h>
>
> #include "s5c73m3.h"
> @@ -1592,26 +1591,16 @@ static int s5c73m3_parse_gpios(struct s5c73m3 *state)
> return 0;
> }
>
> -static int s5c73m3_get_platform_data(struct s5c73m3 *state)
> +static int s5c73m3_get_dt_data(struct s5c73m3 *state)
> {
> struct device *dev = &state->i2c_client->dev;
> - const struct s5c73m3_platform_data *pdata = dev->platform_data;
> struct device_node *node = dev->of_node;
> struct device_node *node_ep;
> struct v4l2_fwnode_endpoint ep = { .bus_type = 0 };
> int ret;
>
> - if (!node) {
> - if (!pdata) {
> - dev_err(dev, "Platform data not specified\n");
> - return -EINVAL;
> - }
> -
> - state->mclk_frequency = pdata->mclk_frequency;
> - state->gpio[STBY] = pdata->gpio_stby;
> - state->gpio[RSET] = pdata->gpio_reset;
> - return 0;
> - }
> + if (!node)
> + return -EINVAL;
>
> state->clock = devm_clk_get(dev, S5C73M3_CLK_NAME);
> if (IS_ERR(state->clock))
> @@ -1666,7 +1655,7 @@ static int s5c73m3_probe(struct i2c_client *client)
> return -ENOMEM;
>
> state->i2c_client = client;
> - ret = s5c73m3_get_platform_data(state);
> + ret = s5c73m3_get_dt_data(state);
> if (ret < 0)
> return ret;
>
> diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-ctrls.c b/drivers/media/i2c/s5c73m3/s5c73m3-ctrls.c
> index 141ad0ba7f5a..1c8103670fa2 100644
> --- a/drivers/media/i2c/s5c73m3/s5c73m3-ctrls.c
> +++ b/drivers/media/i2c/s5c73m3/s5c73m3-ctrls.c
> @@ -10,7 +10,6 @@
> #include <linux/sizes.h>
> #include <linux/delay.h>
> #include <linux/firmware.h>
> -#include <linux/gpio.h>
> #include <linux/i2c.h>
> #include <linux/init.h>
> #include <linux/media.h>
> @@ -24,7 +23,6 @@
> #include <media/v4l2-device.h>
> #include <media/v4l2-subdev.h>
> #include <media/v4l2-mediabus.h>
> -#include <media/i2c/s5c73m3.h>
>
> #include "s5c73m3.h"
>
> diff --git a/drivers/media/i2c/s5c73m3/s5c73m3.h b/drivers/media/i2c/s5c73m3/s5c73m3.h
> index c3fcfdd3ea66..d68528898249 100644
> --- a/drivers/media/i2c/s5c73m3/s5c73m3.h
> +++ b/drivers/media/i2c/s5c73m3/s5c73m3.h
> @@ -15,7 +15,6 @@
> #include <media/v4l2-common.h>
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-subdev.h>
> -#include <media/i2c/s5c73m3.h>
>
> #define DRIVER_NAME "S5C73M3"
>
> @@ -357,6 +356,11 @@ enum s5c73m3_gpio_id {
> GPIO_NUM,
> };
>
> +struct s5c73m3_gpio {
> + int gpio;
> + int level;
> +};
> +
> enum s5c73m3_resolution_types {
> RES_ISP,
> RES_JPEG,
> diff --git a/include/media/i2c/s5c73m3.h b/include/media/i2c/s5c73m3.h
> deleted file mode 100644
> index a51f1025ba1c..000000000000
> --- a/include/media/i2c/s5c73m3.h
> +++ /dev/null
> @@ -1,56 +0,0 @@
> -/*
> - * Samsung LSI S5C73M3 8M pixel camera driver
> - *
> - * Copyright (C) 2012, Samsung Electronics, Co., Ltd.
> - * Sylwester Nawrocki <s.nawrocki@samsung.com>
> - * Andrzej Hajda <a.hajda@samsung.com>
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License
> - * version 2 as published by the Free Software Foundation.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - */
> -#ifndef MEDIA_S5C73M3__
> -#define MEDIA_S5C73M3__
> -
> -#include <linux/videodev2.h>
> -#include <media/v4l2-mediabus.h>
> -
> -/**
> - * struct s5c73m3_gpio - data structure describing a GPIO
> - * @gpio: GPIO number
> - * @level: indicates active state of the @gpio
> - */
> -struct s5c73m3_gpio {
> - int gpio;
> - int level;
> -};
> -
> -/**
> - * struct s5c73m3_platform_data - s5c73m3 driver platform data
> - * @mclk_frequency: sensor's master clock frequency in Hz
> - * @gpio_reset: GPIO driving RESET pin
> - * @gpio_stby: GPIO driving STBY pin
> - * @bus_type: bus type
> - * @nlanes: maximum number of MIPI-CSI lanes used
> - * @horiz_flip: default horizontal image flip value, non zero to enable
> - * @vert_flip: default vertical image flip value, non zero to enable
> - */
> -
> -struct s5c73m3_platform_data {
> - unsigned long mclk_frequency;
> -
> - struct s5c73m3_gpio gpio_reset;
> - struct s5c73m3_gpio gpio_stby;
> -
> - enum v4l2_mbus_type bus_type;
> - u8 nlanes;
> - u8 horiz_flip;
> - u8 vert_flip;
> -};
> -
> -#endif /* MEDIA_S5C73M3__ */
> --
> 2.38.1.431.g37b22c650d-goog
>
Looks good to me.
Reviewed-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
--
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] media: i2c: s5c73m3: switch to using gpiod API
2022-11-15 22:11 ` [PATCH 4/4] media: i2c: s5c73m3: switch to using gpiod API Dmitry Torokhov
2022-11-17 9:06 ` Linus Walleij
@ 2022-11-17 11:06 ` Tommaso Merciai
1 sibling, 0 replies; 15+ messages in thread
From: Tommaso Merciai @ 2022-11-17 11:06 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mauro Carvalho Chehab, Sylwester Nawrocki, Andrzej Hajda,
Linus Walleij, linux-kernel, linux-media
On Tue, Nov 15, 2022 at 02:11:45PM -0800, Dmitry Torokhov wrote:
> This patch switches the driver away from legacy gpio/of_gpio API to
> gpiod API, and removes use of of_get_named_gpio_flags() which I want to
> make private to gpiolib.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/media/i2c/s5c73m3/s5c73m3-core.c | 63 +++++++-----------------
> drivers/media/i2c/s5c73m3/s5c73m3.h | 7 +--
> 2 files changed, 20 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> index 561c1a1583ac..f1e073ed5f99 100644
> --- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> +++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> @@ -10,12 +10,11 @@
> #include <linux/clk.h>
> #include <linux/delay.h>
> #include <linux/firmware.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/i2c.h>
> #include <linux/init.h>
> #include <linux/media.h>
> #include <linux/module.h>
> -#include <linux/of_gpio.h>
> #include <linux/of_graph.h>
> #include <linux/regulator/consumer.h>
> #include <linux/sizes.h>
> @@ -1348,20 +1347,20 @@ static int s5c73m3_oif_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
>
> static int s5c73m3_gpio_set_value(struct s5c73m3 *priv, int id, u32 val)
> {
> - if (!gpio_is_valid(priv->gpio[id].gpio))
> + if (!priv->gpio[id])
> return 0;
> - gpio_set_value(priv->gpio[id].gpio, !!val);
> + gpiod_set_value_cansleep(priv->gpio[id], val);
> return 1;
> }
>
> static int s5c73m3_gpio_assert(struct s5c73m3 *priv, int id)
> {
> - return s5c73m3_gpio_set_value(priv, id, priv->gpio[id].level);
> + return s5c73m3_gpio_set_value(priv, id, 1);
> }
>
> static int s5c73m3_gpio_deassert(struct s5c73m3 *priv, int id)
> {
> - return s5c73m3_gpio_set_value(priv, id, !priv->gpio[id].level);
> + return s5c73m3_gpio_set_value(priv, id, 0);
> }
>
> static int __s5c73m3_power_on(struct s5c73m3 *state)
> @@ -1544,49 +1543,29 @@ static const struct v4l2_subdev_ops oif_subdev_ops = {
>
> static int s5c73m3_configure_gpios(struct s5c73m3 *state)
> {
> - static const char * const gpio_names[] = {
> - "S5C73M3_STBY", "S5C73M3_RST"
> - };
> + static const char * const name[] = { "standby", "xshutdown" };
> + static const char * const label[] = { "S5C73M3_STBY", "S5C73M3_RST" };
> struct i2c_client *c = state->i2c_client;
> - struct s5c73m3_gpio *g = state->gpio;
> + struct gpio_desc *gpio;
> int ret, i;
>
> for (i = 0; i < GPIO_NUM; ++i) {
> - unsigned int flags = GPIOF_DIR_OUT;
> - if (g[i].level)
> - flags |= GPIOF_INIT_HIGH;
> - ret = devm_gpio_request_one(&c->dev, g[i].gpio, flags,
> - gpio_names[i]);
> + gpio = devm_gpiod_get(&c->dev, name[i], GPIOD_OUT_HIGH);
> + ret = PTR_ERR_OR_ZERO(gpio);
> if (ret) {
> - v4l2_err(c, "failed to request gpio %s\n",
> - gpio_names[i]);
> + v4l2_err(c, "failed to request gpio %s: %d\n",
> + name[i], ret);
> return ret;
> }
> - }
> - return 0;
> -}
> -
> -static int s5c73m3_parse_gpios(struct s5c73m3 *state)
> -{
> - static const char * const prop_names[] = {
> - "standby-gpios", "xshutdown-gpios",
> - };
> - struct device *dev = &state->i2c_client->dev;
> - struct device_node *node = dev->of_node;
> - int ret, i;
>
> - for (i = 0; i < GPIO_NUM; ++i) {
> - enum of_gpio_flags of_flags;
> -
> - ret = of_get_named_gpio_flags(node, prop_names[i],
> - 0, &of_flags);
> - if (ret < 0) {
> - dev_err(dev, "failed to parse %s DT property\n",
> - prop_names[i]);
> - return -EINVAL;
> + ret = gpiod_set_consumer_name(gpio, label[i]);
> + if (ret) {
> + v4l2_err(c, "failed to set up name for gpio %s: %d\n",
> + name[i], ret);
> + return ret;
> }
> - state->gpio[i].gpio = ret;
> - state->gpio[i].level = !(of_flags & OF_GPIO_ACTIVE_LOW);
> +
> + state->gpio[i] = gpio;
> }
> return 0;
> }
> @@ -1613,10 +1592,6 @@ static int s5c73m3_get_dt_data(struct s5c73m3 *state)
> state->mclk_frequency);
> }
>
> - ret = s5c73m3_parse_gpios(state);
> - if (ret < 0)
> - return -EINVAL;
> -
> node_ep = of_graph_get_next_endpoint(node, NULL);
> if (!node_ep) {
> dev_warn(dev, "no endpoint defined for node: %pOF\n", node);
> diff --git a/drivers/media/i2c/s5c73m3/s5c73m3.h b/drivers/media/i2c/s5c73m3/s5c73m3.h
> index d68528898249..9887d03fcdeb 100644
> --- a/drivers/media/i2c/s5c73m3/s5c73m3.h
> +++ b/drivers/media/i2c/s5c73m3/s5c73m3.h
> @@ -356,11 +356,6 @@ enum s5c73m3_gpio_id {
> GPIO_NUM,
> };
>
> -struct s5c73m3_gpio {
> - int gpio;
> - int level;
> -};
> -
> enum s5c73m3_resolution_types {
> RES_ISP,
> RES_JPEG,
> @@ -387,7 +382,7 @@ struct s5c73m3 {
> u32 i2c_read_address;
>
> struct regulator_bulk_data supplies[S5C73M3_MAX_SUPPLIES];
> - struct s5c73m3_gpio gpio[GPIO_NUM];
> + struct gpio_desc *gpio[GPIO_NUM];
>
> struct clk *clock;
>
> --
> 2.38.1.431.g37b22c650d-goog
>
Looks good to me.
Reviewed-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
--
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@amarulasolutions.com
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@amarulasolutions.com
www.amarulasolutions.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] media: i2c: s5c73m3: switch to using gpiod API
2022-11-17 9:06 ` Linus Walleij
@ 2022-11-26 10:34 ` Hans Verkuil
2022-11-26 10:39 ` Hans Verkuil
2022-11-26 21:49 ` Linus Walleij
0 siblings, 2 replies; 15+ messages in thread
From: Hans Verkuil @ 2022-11-26 10:34 UTC (permalink / raw)
To: Linus Walleij, Dmitry Torokhov
Cc: Mauro Carvalho Chehab, Sylwester Nawrocki, Andrzej Hajda,
linux-kernel, linux-media
Hi Linus,
On 17/11/2022 10:06, Linus Walleij wrote:
> On Tue, Nov 15, 2022 at 11:11 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 use of of_get_named_gpio_flags() which I want to
>> make private to gpiolib.
>>
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
There are now two patches that do this switch to using the gpiod API:
this one, and one from you:
https://patchwork.linuxtv.org/project/linux-media/patch/20221108100604.1500909-1-linus.walleij@linaro.org/
Any preference?
Regards,
Hans
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] media: i2c: s5c73m3: switch to using gpiod API
2022-11-26 10:34 ` Hans Verkuil
@ 2022-11-26 10:39 ` Hans Verkuil
2022-11-26 21:49 ` Linus Walleij
1 sibling, 0 replies; 15+ messages in thread
From: Hans Verkuil @ 2022-11-26 10:39 UTC (permalink / raw)
To: Linus Walleij, Dmitry Torokhov
Cc: Mauro Carvalho Chehab, Sylwester Nawrocki, Andrzej Hajda,
linux-kernel, linux-media
On 26/11/2022 11:34, Hans Verkuil wrote:
> Hi Linus,
>
> On 17/11/2022 10:06, Linus Walleij wrote:
>> On Tue, Nov 15, 2022 at 11:11 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 use of of_get_named_gpio_flags() which I want to
>>> make private to gpiolib.
>>>
>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> There are now two patches that do this switch to using the gpiod API:
> this one, and one from you:
>
> https://patchwork.linuxtv.org/project/linux-media/patch/20221108100604.1500909-1-linus.walleij@linaro.org/
And a third one as well:
https://patchwork.linuxtv.org/project/linux-media/patch/20221116194307.164543-1-mairacanal@riseup.net/
>
> Any preference?
>
> Regards,
>
> Hans
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/4] media: i2c: s5c73m3: switch to using gpiod API
2022-11-26 10:34 ` Hans Verkuil
2022-11-26 10:39 ` Hans Verkuil
@ 2022-11-26 21:49 ` Linus Walleij
1 sibling, 0 replies; 15+ messages in thread
From: Linus Walleij @ 2022-11-26 21:49 UTC (permalink / raw)
To: Hans Verkuil
Cc: Dmitry Torokhov, Mauro Carvalho Chehab, Sylwester Nawrocki,
Andrzej Hajda, linux-kernel, linux-media
On Sat, Nov 26, 2022 at 11:34 AM Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 17/11/2022 10:06, Linus Walleij wrote:
> > On Tue, Nov 15, 2022 at 11:11 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 use of of_get_named_gpio_flags() which I want to
> >> make private to gpiolib.
> >>
> >> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> There are now two patches that do this switch to using the gpiod API:
> this one, and one from you:
Ooops we stepped on each others' toes again. Dmitry & I really like
to fix these drivers :D sorry about that.
> https://patchwork.linuxtv.org/project/linux-media/patch/20221108100604.1500909-1-linus.walleij@linaro.org/
>
> Any preference?
Not really, take the one you like best, they look functionally equivalent to me,
just slightly different stylistic choices.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] media: i2c: s5c73m3: remove support for platform data
2022-11-15 22:11 ` [PATCH 3/4] media: i2c: s5c73m3: remove support for platform data Dmitry Torokhov
2022-11-17 11:05 ` Tommaso Merciai
@ 2022-11-28 8:39 ` Hans Verkuil
2022-11-28 19:50 ` Dmitry Torokhov
1 sibling, 1 reply; 15+ messages in thread
From: Hans Verkuil @ 2022-11-28 8:39 UTC (permalink / raw)
To: Dmitry Torokhov, Mauro Carvalho Chehab
Cc: Sylwester Nawrocki, Andrzej Hajda, Linus Walleij, linux-kernel,
linux-media
Hi Dmitry,
I received multiple patches for this driver that switch it to using the gpiod
API. I chose to go with Linus' patch [1]. Primarily because that one was
reviewed/acked by more people.
That means also that this patch that removes the platform data no longer applies
cleanly.
Can you post a v2 of this patch that sits on top of [1]?
Thanks!
Hans
[1]: https://patchwork.linuxtv.org/project/linux-media/patch/20221108100604.1500909-1-linus.walleij@linaro.org/
On 15/11/2022 23:11, Dmitry Torokhov wrote:
> There are no existing users of s5c73m3_platform_data in the tree, and
> new users shoudl either be using device tree, ACPI, or static device
> properties, so let's remove it from the driver.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/media/i2c/s5c73m3/s5c73m3-core.c | 19 ++------
> drivers/media/i2c/s5c73m3/s5c73m3-ctrls.c | 2 -
> drivers/media/i2c/s5c73m3/s5c73m3.h | 6 ++-
> include/media/i2c/s5c73m3.h | 56 -----------------------
> 4 files changed, 9 insertions(+), 74 deletions(-)
> delete mode 100644 include/media/i2c/s5c73m3.h
>
> diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> index d96ba58ce1e5..561c1a1583ac 100644
> --- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> +++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> @@ -27,7 +27,6 @@
> #include <media/v4l2-device.h>
> #include <media/v4l2-subdev.h>
> #include <media/v4l2-mediabus.h>
> -#include <media/i2c/s5c73m3.h>
> #include <media/v4l2-fwnode.h>
>
> #include "s5c73m3.h"
> @@ -1592,26 +1591,16 @@ static int s5c73m3_parse_gpios(struct s5c73m3 *state)
> return 0;
> }
>
> -static int s5c73m3_get_platform_data(struct s5c73m3 *state)
> +static int s5c73m3_get_dt_data(struct s5c73m3 *state)
> {
> struct device *dev = &state->i2c_client->dev;
> - const struct s5c73m3_platform_data *pdata = dev->platform_data;
> struct device_node *node = dev->of_node;
> struct device_node *node_ep;
> struct v4l2_fwnode_endpoint ep = { .bus_type = 0 };
> int ret;
>
> - if (!node) {
> - if (!pdata) {
> - dev_err(dev, "Platform data not specified\n");
> - return -EINVAL;
> - }
> -
> - state->mclk_frequency = pdata->mclk_frequency;
> - state->gpio[STBY] = pdata->gpio_stby;
> - state->gpio[RSET] = pdata->gpio_reset;
> - return 0;
> - }
> + if (!node)
> + return -EINVAL;
>
> state->clock = devm_clk_get(dev, S5C73M3_CLK_NAME);
> if (IS_ERR(state->clock))
> @@ -1666,7 +1655,7 @@ static int s5c73m3_probe(struct i2c_client *client)
> return -ENOMEM;
>
> state->i2c_client = client;
> - ret = s5c73m3_get_platform_data(state);
> + ret = s5c73m3_get_dt_data(state);
> if (ret < 0)
> return ret;
>
> diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-ctrls.c b/drivers/media/i2c/s5c73m3/s5c73m3-ctrls.c
> index 141ad0ba7f5a..1c8103670fa2 100644
> --- a/drivers/media/i2c/s5c73m3/s5c73m3-ctrls.c
> +++ b/drivers/media/i2c/s5c73m3/s5c73m3-ctrls.c
> @@ -10,7 +10,6 @@
> #include <linux/sizes.h>
> #include <linux/delay.h>
> #include <linux/firmware.h>
> -#include <linux/gpio.h>
> #include <linux/i2c.h>
> #include <linux/init.h>
> #include <linux/media.h>
> @@ -24,7 +23,6 @@
> #include <media/v4l2-device.h>
> #include <media/v4l2-subdev.h>
> #include <media/v4l2-mediabus.h>
> -#include <media/i2c/s5c73m3.h>
>
> #include "s5c73m3.h"
>
> diff --git a/drivers/media/i2c/s5c73m3/s5c73m3.h b/drivers/media/i2c/s5c73m3/s5c73m3.h
> index c3fcfdd3ea66..d68528898249 100644
> --- a/drivers/media/i2c/s5c73m3/s5c73m3.h
> +++ b/drivers/media/i2c/s5c73m3/s5c73m3.h
> @@ -15,7 +15,6 @@
> #include <media/v4l2-common.h>
> #include <media/v4l2-ctrls.h>
> #include <media/v4l2-subdev.h>
> -#include <media/i2c/s5c73m3.h>
>
> #define DRIVER_NAME "S5C73M3"
>
> @@ -357,6 +356,11 @@ enum s5c73m3_gpio_id {
> GPIO_NUM,
> };
>
> +struct s5c73m3_gpio {
> + int gpio;
> + int level;
> +};
> +
> enum s5c73m3_resolution_types {
> RES_ISP,
> RES_JPEG,
> diff --git a/include/media/i2c/s5c73m3.h b/include/media/i2c/s5c73m3.h
> deleted file mode 100644
> index a51f1025ba1c..000000000000
> --- a/include/media/i2c/s5c73m3.h
> +++ /dev/null
> @@ -1,56 +0,0 @@
> -/*
> - * Samsung LSI S5C73M3 8M pixel camera driver
> - *
> - * Copyright (C) 2012, Samsung Electronics, Co., Ltd.
> - * Sylwester Nawrocki <s.nawrocki@samsung.com>
> - * Andrzej Hajda <a.hajda@samsung.com>
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License
> - * version 2 as published by the Free Software Foundation.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - */
> -#ifndef MEDIA_S5C73M3__
> -#define MEDIA_S5C73M3__
> -
> -#include <linux/videodev2.h>
> -#include <media/v4l2-mediabus.h>
> -
> -/**
> - * struct s5c73m3_gpio - data structure describing a GPIO
> - * @gpio: GPIO number
> - * @level: indicates active state of the @gpio
> - */
> -struct s5c73m3_gpio {
> - int gpio;
> - int level;
> -};
> -
> -/**
> - * struct s5c73m3_platform_data - s5c73m3 driver platform data
> - * @mclk_frequency: sensor's master clock frequency in Hz
> - * @gpio_reset: GPIO driving RESET pin
> - * @gpio_stby: GPIO driving STBY pin
> - * @bus_type: bus type
> - * @nlanes: maximum number of MIPI-CSI lanes used
> - * @horiz_flip: default horizontal image flip value, non zero to enable
> - * @vert_flip: default vertical image flip value, non zero to enable
> - */
> -
> -struct s5c73m3_platform_data {
> - unsigned long mclk_frequency;
> -
> - struct s5c73m3_gpio gpio_reset;
> - struct s5c73m3_gpio gpio_stby;
> -
> - enum v4l2_mbus_type bus_type;
> - u8 nlanes;
> - u8 horiz_flip;
> - u8 vert_flip;
> -};
> -
> -#endif /* MEDIA_S5C73M3__ */
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] media: i2c: s5c73m3: remove support for platform data
2022-11-28 8:39 ` Hans Verkuil
@ 2022-11-28 19:50 ` Dmitry Torokhov
0 siblings, 0 replies; 15+ messages in thread
From: Dmitry Torokhov @ 2022-11-28 19:50 UTC (permalink / raw)
To: Hans Verkuil
Cc: Mauro Carvalho Chehab, Sylwester Nawrocki, Andrzej Hajda,
Linus Walleij, linux-kernel, linux-media
Hi Hans,
On Mon, Nov 28, 2022 at 09:39:04AM +0100, Hans Verkuil wrote:
> Hi Dmitry,
>
> I received multiple patches for this driver that switch it to using the gpiod
> API. I chose to go with Linus' patch [1]. Primarily because that one was
> reviewed/acked by more people.
>
> That means also that this patch that removes the platform data no longer applies
> cleanly.
>
> Can you post a v2 of this patch that sits on top of [1]?
Sure, I just sent out the v2.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-11-28 19:50 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-15 22:11 [PATCH 1/4] media: i2c: s5k6a3: switch to using gpiod API Dmitry Torokhov
2022-11-15 22:11 ` [PATCH 2/4] media: i2c: s5k5baf: " Dmitry Torokhov
2022-11-17 10:54 ` Tommaso Merciai
2022-11-15 22:11 ` [PATCH 3/4] media: i2c: s5c73m3: remove support for platform data Dmitry Torokhov
2022-11-17 11:05 ` Tommaso Merciai
2022-11-28 8:39 ` Hans Verkuil
2022-11-28 19:50 ` Dmitry Torokhov
2022-11-15 22:11 ` [PATCH 4/4] media: i2c: s5c73m3: switch to using gpiod API Dmitry Torokhov
2022-11-17 9:06 ` Linus Walleij
2022-11-26 10:34 ` Hans Verkuil
2022-11-26 10:39 ` Hans Verkuil
2022-11-26 21:49 ` Linus Walleij
2022-11-17 11:06 ` Tommaso Merciai
2022-11-17 9:04 ` [PATCH 1/4] media: i2c: s5k6a3: " Linus Walleij
2022-11-17 9:11 ` Tommaso Merciai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).