diff mbox series

[23/33] ARM: dts: exynos: Remove empty camera pinctrl configuration in Odroid X/U3

Message ID 20200830135200.24304-23-krzk@kernel.org
State Under Review
Headers show
Series [01/33] ARM: dts: exynos: Silence i2c-gpio dtschema warning in Galaxy I9100 | expand

Commit Message

Krzysztof Kozlowski Aug. 30, 2020, 1:51 p.m. UTC
The camera's pinctrl configuration is simply empty and not effective.
Remove it to fix dtbs_check warning:

  arch/arm/boot/dts/exynos4412-odroidx.dt.yaml: camera: pinctrl-0: True is not of type 'array'

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 2 --
 1 file changed, 2 deletions(-)

Comments

Marek Szyprowski Aug. 31, 2020, 8:31 a.m. UTC | #1
Hi Krzysztof,

On 30.08.2020 15:51, Krzysztof Kozlowski wrote:
> The camera's pinctrl configuration is simply empty and not effective.
> Remove it to fix dtbs_check warning:
>
>    arch/arm/boot/dts/exynos4412-odroidx.dt.yaml: camera: pinctrl-0: True is not of type 'array'
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

I think that this was intentional to properly enable support for 
mem-2-mem mode in Exynos4-IS (FIMC), but I'm not sure what are the 
default values if no pinctrl properties are provided. Sylwester, could 
you comment?

> ---
>   arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 2 --
>   1 file changed, 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> index a5c1ce1e396c..6d3576e21ffa 100644
> --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
> @@ -136,8 +136,6 @@
>   
>   &camera {
>   	status = "okay";
> -	pinctrl-names = "default";
> -	pinctrl-0 = <>;
>   };
>   
>   &clock {

Best regards
Krzysztof Kozlowski Aug. 31, 2020, 8:38 a.m. UTC | #2
On Mon, 31 Aug 2020 at 10:31, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Hi Krzysztof,
>
> On 30.08.2020 15:51, Krzysztof Kozlowski wrote:
> > The camera's pinctrl configuration is simply empty and not effective.
> > Remove it to fix dtbs_check warning:
> >
> >    arch/arm/boot/dts/exynos4412-odroidx.dt.yaml: camera: pinctrl-0: True is not of type 'array'
> >
> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>
> I think that this was intentional to properly enable support for
> mem-2-mem mode in Exynos4-IS (FIMC), but I'm not sure what are the
> default values if no pinctrl properties are provided. Sylwester, could
> you comment?

Indeed it could be intentional... I see now errors:
[   33.752203] s5p-fimc-md soc:camera: Failed to get pinctrl: -19

I wonder why getting an empty pinctrl is needed... maybe the driver
should accept missing pinctrl?

Best regards,
Krzysztof
Marek Szyprowski Aug. 31, 2020, 8:44 a.m. UTC | #3
Hi Krzysztof,

On 31.08.2020 10:38, Krzysztof Kozlowski wrote:
> On Mon, 31 Aug 2020 at 10:31, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> On 30.08.2020 15:51, Krzysztof Kozlowski wrote:
>>> The camera's pinctrl configuration is simply empty and not effective.
>>> Remove it to fix dtbs_check warning:
>>>
>>>     arch/arm/boot/dts/exynos4412-odroidx.dt.yaml: camera: pinctrl-0: True is not of type 'array'
>>>
>>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>> I think that this was intentional to properly enable support for
>> mem-2-mem mode in Exynos4-IS (FIMC), but I'm not sure what are the
>> default values if no pinctrl properties are provided. Sylwester, could
>> you comment?
> Indeed it could be intentional... I see now errors:
> [   33.752203] s5p-fimc-md soc:camera: Failed to get pinctrl: -19
>
> I wonder why getting an empty pinctrl is needed... maybe the driver
> should accept missing pinctrl?

Please ensure that you have those patches applied:

https://patchwork.kernel.org/patch/11707579/

https://patchwork.kernel.org/patch/11707577/

https://patchwork.kernel.org/patch/11705409/


Best regards
Sylwester Nawrocki Aug. 31, 2020, 10:35 a.m. UTC | #4
Hi,

On 8/31/20 10:38, Krzysztof Kozlowski wrote:
> On Mon, 31 Aug 2020 at 10:31, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>> On 30.08.2020 15:51, Krzysztof Kozlowski wrote:
>>> The camera's pinctrl configuration is simply empty and not effective.
>>> Remove it to fix dtbs_check warning:
>>>
>>>     arch/arm/boot/dts/exynos4412-odroidx.dt.yaml: camera: pinctrl-0: True is not of type 'array'
>>>
>>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>>
>> I think that this was intentional to properly enable support for
>> mem-2-mem mode in Exynos4-IS (FIMC), but I'm not sure what are the
>> default values if no pinctrl properties are provided. Sylwester, could
>> you comment?
> 
> Indeed it could be intentional... I see now errors:
> [   33.752203] s5p-fimc-md soc:camera: Failed to get pinctrl: -19
> 
> I wonder why getting an empty pinctrl is needed... maybe the driver
> should accept missing pinctrl?

It might have been better to have the pinctrl properties optional, as there
might be boards without the image sensor attached and FIMC could still be 
used in memory-to-memory mode, as Marek pointed out. But it seems too late 
now to change that. The binding defines the pinctrl properties as required 
(Documentation/devicetree/bindings/media/samsung-fimc.txt) and we need to
keep them in dtses.

--
Regards,
Sylwester
Krzysztof Kozlowski Aug. 31, 2020, 10:42 a.m. UTC | #5
On Mon, 31 Aug 2020 at 12:35, Sylwester Nawrocki <snawrocki@kernel.org> wrote:
>
> Hi,
>
> On 8/31/20 10:38, Krzysztof Kozlowski wrote:
> > On Mon, 31 Aug 2020 at 10:31, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >> On 30.08.2020 15:51, Krzysztof Kozlowski wrote:
> >>> The camera's pinctrl configuration is simply empty and not effective.
> >>> Remove it to fix dtbs_check warning:
> >>>
> >>>     arch/arm/boot/dts/exynos4412-odroidx.dt.yaml: camera: pinctrl-0: True is not of type 'array'
> >>>
> >>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> >>
> >> I think that this was intentional to properly enable support for
> >> mem-2-mem mode in Exynos4-IS (FIMC), but I'm not sure what are the
> >> default values if no pinctrl properties are provided. Sylwester, could
> >> you comment?
> >
> > Indeed it could be intentional... I see now errors:
> > [   33.752203] s5p-fimc-md soc:camera: Failed to get pinctrl: -19
> >
> > I wonder why getting an empty pinctrl is needed... maybe the driver
> > should accept missing pinctrl?
>
> It might have been better to have the pinctrl properties optional, as there
> might be boards without the image sensor attached and FIMC could still be
> used in memory-to-memory mode, as Marek pointed out. But it seems too late
> now to change that. The binding defines the pinctrl properties as required
> (Documentation/devicetree/bindings/media/samsung-fimc.txt) and we need to
> keep them in dtses.

You can always make a required property optional and it is not a break
of ABI. The other way around would be a break. Why then not changing
the driver to accept optional pinctrl?

Best regards,
Krzysztof
Sylwester Nawrocki Aug. 31, 2020, 1:57 p.m. UTC | #6
On 31.08.2020 12:42, Krzysztof Kozlowski wrote:
> On Mon, 31 Aug 2020 at 12:35, Sylwester Nawrocki <snawrocki@kernel.org> wrote:
>> On 8/31/20 10:38, Krzysztof Kozlowski wrote:
>>> On Mon, 31 Aug 2020 at 10:31, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>>> On 30.08.2020 15:51, Krzysztof Kozlowski wrote:
>>>>> The camera's pinctrl configuration is simply empty and not effective.
>>>>> Remove it to fix dtbs_check warning:
>>>>>
>>>>>     arch/arm/boot/dts/exynos4412-odroidx.dt.yaml: camera: pinctrl-0: True is not of type 'array'
>>>>>
>>>>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>>>>
>>>> I think that this was intentional to properly enable support for
>>>> mem-2-mem mode in Exynos4-IS (FIMC), but I'm not sure what are the
>>>> default values if no pinctrl properties are provided. Sylwester, could
>>>> you comment?
>>>
>>> Indeed it could be intentional... I see now errors:
>>> [   33.752203] s5p-fimc-md soc:camera: Failed to get pinctrl: -19
>>>
>>> I wonder why getting an empty pinctrl is needed... maybe the driver
>>> should accept missing pinctrl?
>>
>> It might have been better to have the pinctrl properties optional, as there
>> might be boards without the image sensor attached and FIMC could still be
>> used in memory-to-memory mode, as Marek pointed out. But it seems too late
>> now to change that. The binding defines the pinctrl properties as required
>> (Documentation/devicetree/bindings/media/samsung-fimc.txt) and we need to
>> keep them in dtses.
> 
> You can always make a required property optional and it is not a break
> of ABI. The other way around would be a break. Why then not changing
> the driver to accept optional pinctrl?

Feel free to send the patch, I would prefer to leave that as is though.
Is it really suddenly a problem to use an empty property? The pinctrl 
bindings allows it.
Krzysztof Kozlowski Aug. 31, 2020, 1:59 p.m. UTC | #7
On Mon, 31 Aug 2020 at 15:57, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>
> On 31.08.2020 12:42, Krzysztof Kozlowski wrote:
> > On Mon, 31 Aug 2020 at 12:35, Sylwester Nawrocki <snawrocki@kernel.org> wrote:
> >> On 8/31/20 10:38, Krzysztof Kozlowski wrote:
> >>> On Mon, 31 Aug 2020 at 10:31, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >>>> On 30.08.2020 15:51, Krzysztof Kozlowski wrote:
> >>>>> The camera's pinctrl configuration is simply empty and not effective.
> >>>>> Remove it to fix dtbs_check warning:
> >>>>>
> >>>>>     arch/arm/boot/dts/exynos4412-odroidx.dt.yaml: camera: pinctrl-0: True is not of type 'array'
> >>>>>
> >>>>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> >>>>
> >>>> I think that this was intentional to properly enable support for
> >>>> mem-2-mem mode in Exynos4-IS (FIMC), but I'm not sure what are the
> >>>> default values if no pinctrl properties are provided. Sylwester, could
> >>>> you comment?
> >>>
> >>> Indeed it could be intentional... I see now errors:
> >>> [   33.752203] s5p-fimc-md soc:camera: Failed to get pinctrl: -19
> >>>
> >>> I wonder why getting an empty pinctrl is needed... maybe the driver
> >>> should accept missing pinctrl?
> >>
> >> It might have been better to have the pinctrl properties optional, as there
> >> might be boards without the image sensor attached and FIMC could still be
> >> used in memory-to-memory mode, as Marek pointed out. But it seems too late
> >> now to change that. The binding defines the pinctrl properties as required
> >> (Documentation/devicetree/bindings/media/samsung-fimc.txt) and we need to
> >> keep them in dtses.
> >
> > You can always make a required property optional and it is not a break
> > of ABI. The other way around would be a break. Why then not changing
> > the driver to accept optional pinctrl?
>
> Feel free to send the patch, I would prefer to leave that as is though.
> Is it really suddenly a problem to use an empty property? The pinctrl
> bindings allows it.

The bindings scream:
arch/arm/boot/dts/exynos4412-odroidx.dt.yaml: camera: pinctrl-0: True
is not of type 'array'
so either we should model it differently (not as empty array), update
the driver to make it optional or change the bindings/dtschema. I'll
see how to fix it properly.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
index a5c1ce1e396c..6d3576e21ffa 100644
--- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -136,8 +136,6 @@ 
 
 &camera {
 	status = "okay";
-	pinctrl-names = "default";
-	pinctrl-0 = <>;
 };
 
 &clock {