Re: How to write "modern" pinctrl DT node

["Jonathan Neuschäfer" <j.neuschaefer@gmx.net>]

[-- Attachment #1: Type: text/plain, Size: 2688 bytes --]

Hi,

On Tue, Jun 11, 2019 at 06:12:30PM +0200, Marc Gonzalez wrote:
> Hello,
> 
> I'm working with a device (TSIF0) which apparently drives 4 pins:
> (Or maybe 5... it seems gpio40 might be associated with TSIF0 as well.)
> 
> https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm8998-pinctrl.dtsi?h=LE.UM.1.3.r3.25#n2258
> 
> I'll copy the downstream DT nodes here for discussion:
> 
> 		tsif0_signals_active: tsif0_signals_active {
> 			tsif1_clk {
> 				pins = "gpio89"; /* TSIF0 CLK */
> 				function = "tsif1_clk";
> 			};
> 			tsif1_en {
> 				pins = "gpio90"; /* TSIF0 Enable */
> 				function = "tsif1_en";
> 			};
> 			tsif1_data {
> 				pins = "gpio91"; /* TSIF0 DATA */
> 				function = "tsif1_data";
> 			};
> 			signals_cfg {
> 				pins = "gpio89", "gpio90", "gpio91";
> 				drive_strength = <2>;	/* 2 mA */
> 				bias-pull-down;		/* pull down */
> 			};
> 		};
> 
> 		/* sync signal is only used if configured to mode-2 */
> 		tsif0_sync_active: tsif0_sync_active {
> 			tsif1_sync {
> 				pins = "gpio9";	/* TSIF0 SYNC */
> 				function = "tsif1_sync";
> 				drive_strength = <2>;	/* 2 mA */
> 				bias-pull-down;		/* pull down */
> 			};
> 		};

Looking at the upstream binding[1], I think you can almost use the above
snippet as-is, except for the drive_strength property which is spelled
drive-strength.

> Can I rewrite the first node as:
> 
> 	tsif0_default {
> 		pins = "gpio89", "gpio90", "gpio91"; /* clk, enable, data */
> 		function = "is_this_just_a_label?"; /* Can I just leave it out? */
> 		drive-strength = <2>;
> 		bias-pull-down;
> 	}

The function property is not just an arbitrary label, it needs to be one
of the values defined in the binding. (... which makes because the
driver needs to know *which* hardware function to mux to a given pin.)

> Is this enough information to configure the 3 pins? Probably not...

No, I think you need the separate nodes in order to define the separate
functions of the different pins. There isn't just one function called
"tsif1" that you could apply to all pins.

> Can I merge pin 9 in the above node, since it has the same
> "hardware properties" (drive_strength and bias_direction) ?

Merge what? &tsif0_signals_active/signals_cfg with
&tsif0_sync_active/tsif1_sync?

That depends on how &tsif0_signals_active and &tsif0_sync_active are
used. Are they always used at the same time? Then I think so.
Or are they different states of the TSIF interface that are configured
at different times?


Greetings,
Jonathan Neuschäfer


[1]: Documentation/devicetree/bindings/pinctrl/qcom%2Cmsm8998-pinctrl.txt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]