* [PATCH] drivers/staging/media/atomisp: replace legacy GPIO APIs in atomisp_gmin_platform
@ 2024-12-03 9:14 Song Chen
2024-12-03 10:13 ` Hans de Goede
0 siblings, 1 reply; 3+ messages in thread
From: Song Chen @ 2024-12-03 9:14 UTC (permalink / raw)
To: andy, hdegoede, mchehab, sakari.ailus, gregkh, bergh.jonathan
Cc: arnd, linux-kernel, linux-media, linux-staging, Song Chen
In atomisp_gmin_platform, gpio0 and gpio1 have moved to descriptor-based
GPIO APIs, but v1p8_gpio and v2p8_gpio still remain calling legacy ones,
such as gpio_request.
This patch replaces old with new, also removes including gpio.h.
Signed-off-by: Song Chen <chensong_2000@189.cn>
---
.../media/atomisp/pci/atomisp_gmin_platform.c | 63 ++++++++-----------
1 file changed, 25 insertions(+), 38 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
index e176483df301..7ff548ac3171 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
@@ -11,7 +11,6 @@
#include <linux/mfd/intel_soc_pmic.h>
#include <linux/regulator/consumer.h>
#include <linux/gpio/consumer.h>
-#include <linux/gpio.h>
#include <linux/platform_device.h>
#include "../../include/linux/atomisp_platform.h"
#include "../../include/linux/atomisp_gmin_platform.h"
@@ -85,8 +84,8 @@ struct gmin_subdev {
bool v2p8_on;
bool v1p2_on;
- int v1p8_gpio;
- int v2p8_gpio;
+ struct gpio_desc *v1p8_gpiod;
+ struct gpio_desc *v2p8_gpiod;
u8 pwm_i2c_addr;
@@ -548,23 +547,6 @@ static int gmin_subdev_add(struct gmin_subdev *gs)
else
dev_info(dev, "will handle gpio1 via ACPI\n");
- /*
- * Those are used only when there is an external regulator apart
- * from the PMIC that would be providing power supply, like on the
- * two cases below:
- *
- * The ECS E7 board drives camera 2.8v from an external regulator
- * instead of the PMIC. There's a gmin_CamV2P8 config variable
- * that specifies the GPIO to handle this particular case,
- * but this needs a broader architecture for handling camera power.
- *
- * The CHT RVP board drives camera 1.8v from an* external regulator
- * instead of the PMIC just like ECS E7 board.
- */
-
- gs->v1p8_gpio = gmin_get_var_int(dev, true, "V1P8GPIO", -1);
- gs->v2p8_gpio = gmin_get_var_int(dev, true, "V2P8GPIO", -1);
-
/*
* FIXME:
*
@@ -830,21 +812,23 @@ static int gmin_v1p2_ctrl(struct v4l2_subdev *subdev, int on)
static int gmin_v1p8_ctrl(struct v4l2_subdev *subdev, int on)
{
struct gmin_subdev *gs = find_gmin_subdev(subdev);
+ struct i2c_client *client = v4l2_get_subdevdata(gs->subdev);
+ struct device *dev = &client->dev;
int ret;
int value;
int reg;
+ int v1p8_gpio;
if (!gs || gs->v1p8_on == on)
return 0;
- if (gs->v1p8_gpio >= 0) {
- pr_info("atomisp_gmin_platform: 1.8v power on GPIO %d\n",
- gs->v1p8_gpio);
- ret = gpio_request(gs->v1p8_gpio, "camera_v1p8_en");
- if (!ret)
- ret = gpio_direction_output(gs->v1p8_gpio, 0);
- if (ret)
+ v1p8_gpio = gmin_get_var_int(dev, true, "V1P8GPIO", -1);
+ if (v1p8_gpio >= 0) {
+ gs->v1p8_gpiod = gpiod_get_index(dev, "camera_v1p8", v1p8_gpio, GPIOD_ASIS);
+ if (IS_ERR(gs->v1p8_gpiod))
pr_err("V1P8 GPIO initialization failed\n");
+ else
+ gpiod_direction_output(gs->v1p8_gpiod, 0);
}
gs->v1p8_on = on;
@@ -861,8 +845,8 @@ static int gmin_v1p8_ctrl(struct v4l2_subdev *subdev, int on)
goto out; /* Still needed */
}
- if (gs->v1p8_gpio >= 0)
- gpio_set_value(gs->v1p8_gpio, on);
+ if (v1p8_gpio >= 0)
+ gpiod_set_value(gs->v1p8_gpiod, on);
if (gs->v1p8_reg) {
regulator_set_voltage(gs->v1p8_reg, 1800000, 1800000);
@@ -911,21 +895,24 @@ static int gmin_v1p8_ctrl(struct v4l2_subdev *subdev, int on)
static int gmin_v2p8_ctrl(struct v4l2_subdev *subdev, int on)
{
struct gmin_subdev *gs = find_gmin_subdev(subdev);
+ struct i2c_client *client = v4l2_get_subdevdata(gs->subdev);
+ struct device *dev = &client->dev;
int ret;
int value;
int reg;
+ int v2p8_gpio;
if (WARN_ON(!gs))
return -ENODEV;
- if (gs->v2p8_gpio >= 0) {
- pr_info("atomisp_gmin_platform: 2.8v power on GPIO %d\n",
- gs->v2p8_gpio);
- ret = gpio_request(gs->v2p8_gpio, "camera_v2p8");
- if (!ret)
- ret = gpio_direction_output(gs->v2p8_gpio, 0);
- if (ret)
+ v2p8_gpio = gmin_get_var_int(dev, true, "V2P8GPIO", -1);
+ if (v2p8_gpio >= 0) {
+ pr_info("atomisp_gmin_platform: 2.8v power on GPIO %d\n", v2p8_gpio);
+ gs->v2p8_gpiod = gpiod_get_index(dev, "camera_v2p8", v2p8_gpio, GPIOD_ASIS);
+ if (IS_ERR(gs->v2p8_gpiod))
pr_err("V2P8 GPIO initialization failed\n");
+ else
+ gpiod_direction_output(gs->v2p8_gpiod, 0);
}
if (gs->v2p8_on == on)
@@ -944,8 +931,8 @@ static int gmin_v2p8_ctrl(struct v4l2_subdev *subdev, int on)
goto out; /* Still needed */
}
- if (gs->v2p8_gpio >= 0)
- gpio_set_value(gs->v2p8_gpio, on);
+ if (v2p8_gpio >= 0)
+ gpiod_set_value(gs->v2p8_gpiod, on);
if (gs->v2p8_reg) {
regulator_set_voltage(gs->v2p8_reg, 2900000, 2900000);
--
2.25.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drivers/staging/media/atomisp: replace legacy GPIO APIs in atomisp_gmin_platform
2024-12-03 9:14 [PATCH] drivers/staging/media/atomisp: replace legacy GPIO APIs in atomisp_gmin_platform Song Chen
@ 2024-12-03 10:13 ` Hans de Goede
2024-12-03 12:46 ` Song Chen
0 siblings, 1 reply; 3+ messages in thread
From: Hans de Goede @ 2024-12-03 10:13 UTC (permalink / raw)
To: Song Chen, andy, mchehab, sakari.ailus, gregkh, bergh.jonathan
Cc: arnd, linux-kernel, linux-media, linux-staging
Hi,
On 3-Dec-24 10:14 AM, Song Chen wrote:
> In atomisp_gmin_platform, gpio0 and gpio1 have moved to descriptor-based
> GPIO APIs, but v1p8_gpio and v2p8_gpio still remain calling legacy ones,
> such as gpio_request.
>
> This patch replaces old with new, also removes including gpio.h.
>
> Signed-off-by: Song Chen <chensong_2000@189.cn>
Thank you for your patch, this is already addresses by this patch
which I plan to merge soon:
https://lore.kernel.org/linux-media/20241107221134.596149-1-hdegoede@redhat.com/
Regards,
Hans
> ---
> .../media/atomisp/pci/atomisp_gmin_platform.c | 63 ++++++++-----------
> 1 file changed, 25 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> index e176483df301..7ff548ac3171 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
> @@ -11,7 +11,6 @@
> #include <linux/mfd/intel_soc_pmic.h>
> #include <linux/regulator/consumer.h>
> #include <linux/gpio/consumer.h>
> -#include <linux/gpio.h>
> #include <linux/platform_device.h>
> #include "../../include/linux/atomisp_platform.h"
> #include "../../include/linux/atomisp_gmin_platform.h"
> @@ -85,8 +84,8 @@ struct gmin_subdev {
> bool v2p8_on;
> bool v1p2_on;
>
> - int v1p8_gpio;
> - int v2p8_gpio;
> + struct gpio_desc *v1p8_gpiod;
> + struct gpio_desc *v2p8_gpiod;
>
> u8 pwm_i2c_addr;
>
> @@ -548,23 +547,6 @@ static int gmin_subdev_add(struct gmin_subdev *gs)
> else
> dev_info(dev, "will handle gpio1 via ACPI\n");
>
> - /*
> - * Those are used only when there is an external regulator apart
> - * from the PMIC that would be providing power supply, like on the
> - * two cases below:
> - *
> - * The ECS E7 board drives camera 2.8v from an external regulator
> - * instead of the PMIC. There's a gmin_CamV2P8 config variable
> - * that specifies the GPIO to handle this particular case,
> - * but this needs a broader architecture for handling camera power.
> - *
> - * The CHT RVP board drives camera 1.8v from an* external regulator
> - * instead of the PMIC just like ECS E7 board.
> - */
> -
> - gs->v1p8_gpio = gmin_get_var_int(dev, true, "V1P8GPIO", -1);
> - gs->v2p8_gpio = gmin_get_var_int(dev, true, "V2P8GPIO", -1);
> -
> /*
> * FIXME:
> *
> @@ -830,21 +812,23 @@ static int gmin_v1p2_ctrl(struct v4l2_subdev *subdev, int on)
> static int gmin_v1p8_ctrl(struct v4l2_subdev *subdev, int on)
> {
> struct gmin_subdev *gs = find_gmin_subdev(subdev);
> + struct i2c_client *client = v4l2_get_subdevdata(gs->subdev);
> + struct device *dev = &client->dev;
> int ret;
> int value;
> int reg;
> + int v1p8_gpio;
>
> if (!gs || gs->v1p8_on == on)
> return 0;
>
> - if (gs->v1p8_gpio >= 0) {
> - pr_info("atomisp_gmin_platform: 1.8v power on GPIO %d\n",
> - gs->v1p8_gpio);
> - ret = gpio_request(gs->v1p8_gpio, "camera_v1p8_en");
> - if (!ret)
> - ret = gpio_direction_output(gs->v1p8_gpio, 0);
> - if (ret)
> + v1p8_gpio = gmin_get_var_int(dev, true, "V1P8GPIO", -1);
> + if (v1p8_gpio >= 0) {
> + gs->v1p8_gpiod = gpiod_get_index(dev, "camera_v1p8", v1p8_gpio, GPIOD_ASIS);
> + if (IS_ERR(gs->v1p8_gpiod))
> pr_err("V1P8 GPIO initialization failed\n");
> + else
> + gpiod_direction_output(gs->v1p8_gpiod, 0);
> }
>
> gs->v1p8_on = on;
> @@ -861,8 +845,8 @@ static int gmin_v1p8_ctrl(struct v4l2_subdev *subdev, int on)
> goto out; /* Still needed */
> }
>
> - if (gs->v1p8_gpio >= 0)
> - gpio_set_value(gs->v1p8_gpio, on);
> + if (v1p8_gpio >= 0)
> + gpiod_set_value(gs->v1p8_gpiod, on);
>
> if (gs->v1p8_reg) {
> regulator_set_voltage(gs->v1p8_reg, 1800000, 1800000);
> @@ -911,21 +895,24 @@ static int gmin_v1p8_ctrl(struct v4l2_subdev *subdev, int on)
> static int gmin_v2p8_ctrl(struct v4l2_subdev *subdev, int on)
> {
> struct gmin_subdev *gs = find_gmin_subdev(subdev);
> + struct i2c_client *client = v4l2_get_subdevdata(gs->subdev);
> + struct device *dev = &client->dev;
> int ret;
> int value;
> int reg;
> + int v2p8_gpio;
>
> if (WARN_ON(!gs))
> return -ENODEV;
>
> - if (gs->v2p8_gpio >= 0) {
> - pr_info("atomisp_gmin_platform: 2.8v power on GPIO %d\n",
> - gs->v2p8_gpio);
> - ret = gpio_request(gs->v2p8_gpio, "camera_v2p8");
> - if (!ret)
> - ret = gpio_direction_output(gs->v2p8_gpio, 0);
> - if (ret)
> + v2p8_gpio = gmin_get_var_int(dev, true, "V2P8GPIO", -1);
> + if (v2p8_gpio >= 0) {
> + pr_info("atomisp_gmin_platform: 2.8v power on GPIO %d\n", v2p8_gpio);
> + gs->v2p8_gpiod = gpiod_get_index(dev, "camera_v2p8", v2p8_gpio, GPIOD_ASIS);
> + if (IS_ERR(gs->v2p8_gpiod))
> pr_err("V2P8 GPIO initialization failed\n");
> + else
> + gpiod_direction_output(gs->v2p8_gpiod, 0);
> }
>
> if (gs->v2p8_on == on)
> @@ -944,8 +931,8 @@ static int gmin_v2p8_ctrl(struct v4l2_subdev *subdev, int on)
> goto out; /* Still needed */
> }
>
> - if (gs->v2p8_gpio >= 0)
> - gpio_set_value(gs->v2p8_gpio, on);
> + if (v2p8_gpio >= 0)
> + gpiod_set_value(gs->v2p8_gpiod, on);
>
> if (gs->v2p8_reg) {
> regulator_set_voltage(gs->v2p8_reg, 2900000, 2900000);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drivers/staging/media/atomisp: replace legacy GPIO APIs in atomisp_gmin_platform
2024-12-03 10:13 ` Hans de Goede
@ 2024-12-03 12:46 ` Song Chen
0 siblings, 0 replies; 3+ messages in thread
From: Song Chen @ 2024-12-03 12:46 UTC (permalink / raw)
To: Hans de Goede, andy, mchehab, sakari.ailus, gregkh, bergh.jonathan
Cc: arnd, linux-kernel, linux-media, linux-staging
oh, good to know, many thanks.
Song
在 2024/12/3 18:13, Hans de Goede 写道:
> Hi,
>
> On 3-Dec-24 10:14 AM, Song Chen wrote:
>> In atomisp_gmin_platform, gpio0 and gpio1 have moved to descriptor-based
>> GPIO APIs, but v1p8_gpio and v2p8_gpio still remain calling legacy ones,
>> such as gpio_request.
>>
>> This patch replaces old with new, also removes including gpio.h.
>>
>> Signed-off-by: Song Chen <chensong_2000@189.cn>
>
> Thank you for your patch, this is already addresses by this patch
> which I plan to merge soon:
>
> https://lore.kernel.org/linux-media/20241107221134.596149-1-hdegoede@redhat.com/
>
> Regards,
>
> Hans
>
>
>> ---
>> .../media/atomisp/pci/atomisp_gmin_platform.c | 63 ++++++++-----------
>> 1 file changed, 25 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
>> index e176483df301..7ff548ac3171 100644
>> --- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
>> +++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c
>> @@ -11,7 +11,6 @@
>> #include <linux/mfd/intel_soc_pmic.h>
>> #include <linux/regulator/consumer.h>
>> #include <linux/gpio/consumer.h>
>> -#include <linux/gpio.h>
>> #include <linux/platform_device.h>
>> #include "../../include/linux/atomisp_platform.h"
>> #include "../../include/linux/atomisp_gmin_platform.h"
>> @@ -85,8 +84,8 @@ struct gmin_subdev {
>> bool v2p8_on;
>> bool v1p2_on;
>>
>> - int v1p8_gpio;
>> - int v2p8_gpio;
>> + struct gpio_desc *v1p8_gpiod;
>> + struct gpio_desc *v2p8_gpiod;
>>
>> u8 pwm_i2c_addr;
>>
>> @@ -548,23 +547,6 @@ static int gmin_subdev_add(struct gmin_subdev *gs)
>> else
>> dev_info(dev, "will handle gpio1 via ACPI\n");
>>
>> - /*
>> - * Those are used only when there is an external regulator apart
>> - * from the PMIC that would be providing power supply, like on the
>> - * two cases below:
>> - *
>> - * The ECS E7 board drives camera 2.8v from an external regulator
>> - * instead of the PMIC. There's a gmin_CamV2P8 config variable
>> - * that specifies the GPIO to handle this particular case,
>> - * but this needs a broader architecture for handling camera power.
>> - *
>> - * The CHT RVP board drives camera 1.8v from an* external regulator
>> - * instead of the PMIC just like ECS E7 board.
>> - */
>> -
>> - gs->v1p8_gpio = gmin_get_var_int(dev, true, "V1P8GPIO", -1);
>> - gs->v2p8_gpio = gmin_get_var_int(dev, true, "V2P8GPIO", -1);
>> -
>> /*
>> * FIXME:
>> *
>> @@ -830,21 +812,23 @@ static int gmin_v1p2_ctrl(struct v4l2_subdev *subdev, int on)
>> static int gmin_v1p8_ctrl(struct v4l2_subdev *subdev, int on)
>> {
>> struct gmin_subdev *gs = find_gmin_subdev(subdev);
>> + struct i2c_client *client = v4l2_get_subdevdata(gs->subdev);
>> + struct device *dev = &client->dev;
>> int ret;
>> int value;
>> int reg;
>> + int v1p8_gpio;
>>
>> if (!gs || gs->v1p8_on == on)
>> return 0;
>>
>> - if (gs->v1p8_gpio >= 0) {
>> - pr_info("atomisp_gmin_platform: 1.8v power on GPIO %d\n",
>> - gs->v1p8_gpio);
>> - ret = gpio_request(gs->v1p8_gpio, "camera_v1p8_en");
>> - if (!ret)
>> - ret = gpio_direction_output(gs->v1p8_gpio, 0);
>> - if (ret)
>> + v1p8_gpio = gmin_get_var_int(dev, true, "V1P8GPIO", -1);
>> + if (v1p8_gpio >= 0) {
>> + gs->v1p8_gpiod = gpiod_get_index(dev, "camera_v1p8", v1p8_gpio, GPIOD_ASIS);
>> + if (IS_ERR(gs->v1p8_gpiod))
>> pr_err("V1P8 GPIO initialization failed\n");
>> + else
>> + gpiod_direction_output(gs->v1p8_gpiod, 0);
>> }
>>
>> gs->v1p8_on = on;
>> @@ -861,8 +845,8 @@ static int gmin_v1p8_ctrl(struct v4l2_subdev *subdev, int on)
>> goto out; /* Still needed */
>> }
>>
>> - if (gs->v1p8_gpio >= 0)
>> - gpio_set_value(gs->v1p8_gpio, on);
>> + if (v1p8_gpio >= 0)
>> + gpiod_set_value(gs->v1p8_gpiod, on);
>>
>> if (gs->v1p8_reg) {
>> regulator_set_voltage(gs->v1p8_reg, 1800000, 1800000);
>> @@ -911,21 +895,24 @@ static int gmin_v1p8_ctrl(struct v4l2_subdev *subdev, int on)
>> static int gmin_v2p8_ctrl(struct v4l2_subdev *subdev, int on)
>> {
>> struct gmin_subdev *gs = find_gmin_subdev(subdev);
>> + struct i2c_client *client = v4l2_get_subdevdata(gs->subdev);
>> + struct device *dev = &client->dev;
>> int ret;
>> int value;
>> int reg;
>> + int v2p8_gpio;
>>
>> if (WARN_ON(!gs))
>> return -ENODEV;
>>
>> - if (gs->v2p8_gpio >= 0) {
>> - pr_info("atomisp_gmin_platform: 2.8v power on GPIO %d\n",
>> - gs->v2p8_gpio);
>> - ret = gpio_request(gs->v2p8_gpio, "camera_v2p8");
>> - if (!ret)
>> - ret = gpio_direction_output(gs->v2p8_gpio, 0);
>> - if (ret)
>> + v2p8_gpio = gmin_get_var_int(dev, true, "V2P8GPIO", -1);
>> + if (v2p8_gpio >= 0) {
>> + pr_info("atomisp_gmin_platform: 2.8v power on GPIO %d\n", v2p8_gpio);
>> + gs->v2p8_gpiod = gpiod_get_index(dev, "camera_v2p8", v2p8_gpio, GPIOD_ASIS);
>> + if (IS_ERR(gs->v2p8_gpiod))
>> pr_err("V2P8 GPIO initialization failed\n");
>> + else
>> + gpiod_direction_output(gs->v2p8_gpiod, 0);
>> }
>>
>> if (gs->v2p8_on == on)
>> @@ -944,8 +931,8 @@ static int gmin_v2p8_ctrl(struct v4l2_subdev *subdev, int on)
>> goto out; /* Still needed */
>> }
>>
>> - if (gs->v2p8_gpio >= 0)
>> - gpio_set_value(gs->v2p8_gpio, on);
>> + if (v2p8_gpio >= 0)
>> + gpiod_set_value(gs->v2p8_gpiod, on);
>>
>> if (gs->v2p8_reg) {
>> regulator_set_voltage(gs->v2p8_reg, 2900000, 2900000);
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-12-03 12:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-12-03 9:14 [PATCH] drivers/staging/media/atomisp: replace legacy GPIO APIs in atomisp_gmin_platform Song Chen
2024-12-03 10:13 ` Hans de Goede
2024-12-03 12:46 ` Song Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®