[PATCH 1/2] media: Revert "media: exynos4-is: Add missed check for pinctrl_lookup_state()"

[PATCH 1/2] media: Revert "media: exynos4-is: Add missed check for pinctrl_lookup_state()"

[Sylwester Nawrocki]

The "idle" pinctrl state is optional as documented in the DT binding.
The change introduced by the commit being reverted makes that pinctrl state
mandatory and breaks initialization of the whole media driver, since the
"idle" state is not specified in any mainline dts.

This reverts commit 18ffec750578f7447c288647d7282c7d12b1d969 to fix
the regression.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/media/platform/exynos4-is/media-dev.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 16dd660..9a57523 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -1268,11 +1268,9 @@ static int fimc_md_get_pinctrl(struct fimc_md *fmd)
 	if (IS_ERR(pctl->state_default))
 		return PTR_ERR(pctl->state_default);
 
+	/* PINCTRL_STATE_IDLE is optional */
 	pctl->state_idle = pinctrl_lookup_state(pctl->pinctrl,
 					PINCTRL_STATE_IDLE);
-	if (IS_ERR(pctl->state_idle))
-		return PTR_ERR(pctl->state_idle);
-
 	return 0;
 }
 
-- 
2.7.4

[PATCH 2/2] media: exynos4-is: Simplify the pinctrl code

[Sylwester Nawrocki]

There is no need to request the "idle" pinctrl state in the driver as that
is implemented in the driver core and the pinctrl_pm_* API can be used for
switching between the default and the idle state.

Simplify the pinctrl code to only request and check for the mandatory
"default" pinctrl state.

Switching between the default/idle pinctrl state is not yet implemented
in the driver and this patch doesn't change that.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/media/platform/exynos4-is/media-dev.c | 27 +++++----------------------
 drivers/media/platform/exynos4-is/media-dev.h | 11 -----------
 2 files changed, 5 insertions(+), 33 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
index 9a57523..6df2796 100644
--- a/drivers/media/platform/exynos4-is/media-dev.c
+++ b/drivers/media/platform/exynos4-is/media-dev.c
@@ -19,6 +19,7 @@
 #include <linux/of_platform.h>
 #include <linux/of_device.h>
 #include <linux/of_graph.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/types.h>
@@ -1254,26 +1255,6 @@ static ssize_t fimc_md_sysfs_store(struct device *dev,
 static DEVICE_ATTR(subdev_conf_mode, S_IWUSR | S_IRUGO,
 		   fimc_md_sysfs_show, fimc_md_sysfs_store);
 
-static int fimc_md_get_pinctrl(struct fimc_md *fmd)
-{
-	struct device *dev = &fmd->pdev->dev;
-	struct fimc_pinctrl *pctl = &fmd->pinctl;
-
-	pctl->pinctrl = devm_pinctrl_get(dev);
-	if (IS_ERR(pctl->pinctrl))
-		return PTR_ERR(pctl->pinctrl);
-
-	pctl->state_default = pinctrl_lookup_state(pctl->pinctrl,
-					PINCTRL_STATE_DEFAULT);
-	if (IS_ERR(pctl->state_default))
-		return PTR_ERR(pctl->state_default);
-
-	/* PINCTRL_STATE_IDLE is optional */
-	pctl->state_idle = pinctrl_lookup_state(pctl->pinctrl,
-					PINCTRL_STATE_IDLE);
-	return 0;
-}
-
 static int cam_clk_prepare(struct clk_hw *hw)
 {
 	struct cam_clk *camclk = to_cam_clk(hw);
@@ -1429,6 +1410,7 @@ static int fimc_md_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct v4l2_device *v4l2_dev;
+	struct pinctrl *pinctrl;
 	struct fimc_md *fmd;
 	int ret;
 
@@ -1465,8 +1447,9 @@ static int fimc_md_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_v4l2dev;
 
-	ret = fimc_md_get_pinctrl(fmd);
-	if (ret < 0) {
+	pinctrl = devm_pinctrl_get(dev);
+	if (IS_ERR(pinctrl)) {
+		ret = PTR_ERR(pinctrl);
 		if (ret != EPROBE_DEFER)
 			dev_err(dev, "Failed to get pinctrl: %d\n", ret);
 		goto err_clk;
diff --git a/drivers/media/platform/exynos4-is/media-dev.h b/drivers/media/platform/exynos4-is/media-dev.h
index 4b8f9ac..9447faf 100644
--- a/drivers/media/platform/exynos4-is/media-dev.h
+++ b/drivers/media/platform/exynos4-is/media-dev.h
@@ -27,8 +27,6 @@
 #define FIMC_IS_OF_NODE_NAME	"fimc-is"
 #define CSIS_OF_NODE_NAME	"csis"
 
-#define PINCTRL_STATE_IDLE	"idle"
-
 #define FIMC_MAX_SENSORS	4
 #define FIMC_MAX_CAMCLKS	2
 #define DEFAULT_SENSOR_CLK_FREQ	24000000U
@@ -109,9 +107,6 @@ struct cam_clk {
  * @media_dev: top level media device
  * @v4l2_dev: top level v4l2_device holding up the subdevs
  * @pdev: platform device this media device is hooked up into
- * @pinctrl: camera port pinctrl handle
- * @state_default: pinctrl default state handle
- * @state_idle: pinctrl idle state handle
  * @cam_clk_provider: CAMCLK clock provider structure
  * @user_subdev_api: true if subdevs are not configured by the host driver
  * @slock: spinlock protecting @sensor array
@@ -131,12 +126,6 @@ struct fimc_md {
 	struct v4l2_device v4l2_dev;
 	struct platform_device *pdev;
 
-	struct fimc_pinctrl {
-		struct pinctrl *pinctrl;
-		struct pinctrl_state *state_default;
-		struct pinctrl_state *state_idle;
-	} pinctl;
-
 	struct cam_clk_provider {
 		struct clk *clks[FIMC_MAX_CAMCLKS];
 		struct clk_onecell_data clk_data;
-- 
2.7.4

Re: [PATCH 1/2] media: Revert "media: exynos4-is: Add missed check for pinctrl_lookup_state()"

[Hans Verkuil]

Hi Sylwester,

Can you rebase this patch series on top of the media_tree master?

This series didn't apply anymore.

Thanks!

	Hans

On 10/08/2020 17:32, Sylwester Nawrocki wrote:
> The "idle" pinctrl state is optional as documented in the DT binding.
> The change introduced by the commit being reverted makes that pinctrl state
> mandatory and breaks initialization of the whole media driver, since the
> "idle" state is not specified in any mainline dts.
> 
> This reverts commit 18ffec750578f7447c288647d7282c7d12b1d969 to fix
> the regression.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  drivers/media/platform/exynos4-is/media-dev.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> index 16dd660..9a57523 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -1268,11 +1268,9 @@ static int fimc_md_get_pinctrl(struct fimc_md *fmd)
>  	if (IS_ERR(pctl->state_default))
>  		return PTR_ERR(pctl->state_default);
>  
> +	/* PINCTRL_STATE_IDLE is optional */
>  	pctl->state_idle = pinctrl_lookup_state(pctl->pinctrl,
>  					PINCTRL_STATE_IDLE);
> -	if (IS_ERR(pctl->state_idle))
> -		return PTR_ERR(pctl->state_idle);
> -
>  	return 0;
>  }
>  
> 

Re: [PATCH 1/2] media: Revert "media: exynos4-is: Add missed check for pinctrl_lookup_state()"

[Hans Verkuil]

On 19/08/2020 15:15, Hans Verkuil wrote:
> Hi Sylwester,
> 
> Can you rebase this patch series on top of the media_tree master?
> 
> This series didn't apply anymore.

Never mind, it was a conflict between this patch and this one:

https://patchwork.linuxtv.org/project/linux-media/patch/20200807083548.204360-2-dwlsalmeida@gmail.com/

I'll fix this up myself.

Regards,

	Hans

> 
> Thanks!
> 
> 	Hans
> 
> On 10/08/2020 17:32, Sylwester Nawrocki wrote:
>> The "idle" pinctrl state is optional as documented in the DT binding.
>> The change introduced by the commit being reverted makes that pinctrl state
>> mandatory and breaks initialization of the whole media driver, since the
>> "idle" state is not specified in any mainline dts.
>>
>> This reverts commit 18ffec750578f7447c288647d7282c7d12b1d969 to fix
>> the regression.
>>
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> ---
>>  drivers/media/platform/exynos4-is/media-dev.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
>> index 16dd660..9a57523 100644
>> --- a/drivers/media/platform/exynos4-is/media-dev.c
>> +++ b/drivers/media/platform/exynos4-is/media-dev.c
>> @@ -1268,11 +1268,9 @@ static int fimc_md_get_pinctrl(struct fimc_md *fmd)
>>  	if (IS_ERR(pctl->state_default))
>>  		return PTR_ERR(pctl->state_default);
>>  
>> +	/* PINCTRL_STATE_IDLE is optional */
>>  	pctl->state_idle = pinctrl_lookup_state(pctl->pinctrl,
>>  					PINCTRL_STATE_IDLE);
>> -	if (IS_ERR(pctl->state_idle))
>> -		return PTR_ERR(pctl->state_idle);
>> -
>>  	return 0;
>>  }
>>  
>>
>