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 <[email protected]>
Signed-off-by: Dmitry Torokhov <[email protected]>
---
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 <[email protected]>
- * Andrzej Hajda <[email protected]>
- *
- * 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
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 <[email protected]>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
> 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 <[email protected]>
> - * Andrzej Hajda <[email protected]>
> - *
> - * 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 <[email protected]>
--
Tommaso Merciai
Embedded Linux Engineer
[email protected]
__________________________________
Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
[email protected]
http://www.amarulasolutions.com
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/[email protected]/
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 <[email protected]>
> Signed-off-by: Dmitry Torokhov <[email protected]>
> ---
> 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 <[email protected]>
> - * Andrzej Hajda <[email protected]>
> - *
> - * 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__ */
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