Re: [PATCH 3/4] arm64: dts: qcom: sm8250: camss: Add downstream camera pin definitions

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Sat 09 Apr 11:45 CDT 2022, Bryan O'Donoghue wrote:

> There are a number of pin mux descriptions for the CAMSS block which are
> defined downstream.
> 
> Bring in the definitions now. Subsequent changes will optionally activate
> those muxes for different sm8250 based boards.
> 

I'm not able to spot where these are referenced, are they not used or am
I just failing the search today?

I would prefer that we don't sprinkle pinctrl states that potentially
might be used at a later time...

> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8250.dtsi | 471 +++++++++++++++++++++++++++
>  1 file changed, 471 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 3d6c617de045..91ed079edbf7 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -3687,6 +3687,477 @@ tlmm: pinctrl@f100000 {
>  			gpio-ranges = <&tlmm 0 0 181>;
>  			wakeup-parent = <&pdc>;
>  
> +			cam_sensor_mclk0_active: cam_sensor_mclk0_active {

Please don't use '_' in the node name.

> +				/* MCLK0 */

I would prefer that we don't inherit the mux/config split upstream.

You can either do:

cam-sensor-mclk0-active-state {
	pins = ;
	function = ;
	bias...
};

or:

cam-sensor-mclk0-active-state {
	mclk0 {
		pins = ;
		function = ;
		bias...
	};
};

And depending on how this state is actually used you could lump together
the configuration of several pins - with different properties - in the
same state node.

Regards,
Bjorn