[RFC PATCH 00/13] Add support for Tegra DPAUX pinctrl

[RFC PATCH 00/13] Add support for Tegra DPAUX pinctrl

[Jon Hunter]

The Display Port Auxiliary (DPAUX) channel pads can be shared with an
internal I2C controller. Add pinctrl support for these pads so that the
I2C controller can request and use these pads.

Jon Hunter (13):
  drm/tegra: Clean-up if probing DPAUX fails
  drm/tegra: Add helper functions for setting up DPAUX pads
  dt-bindings: drm/tegra: Update DPAUX documentation
  drm/tegra: Add sor-safe clock for DPAUX on Tegra210
  drm/tegra: Prepare DPAUX for supporting generic PM domains
  pinctrl: pinconf: Add generic helper function for freeing mappings
  dt-bindings: i2c: Add support for 'i2c-bus' subnode
  i2c: core: Add support for 'i2c-bus' subnode
  dt-bindings: drm/tegra: Add DPAUX pinctrl documentation
  drm/tegra: Add pinctrl support for DPAUX
  arm64: tegra: Add SOR power-domain node
  arm64: tegra: Add sor-safe clock to DPAUX binding
  arm64: tegra: Add DPAUX pinctrl bindings

 .../display/tegra/nvidia,tegra20-host1x.txt        |  11 +-
 Documentation/devicetree/bindings/i2c/i2c.txt      |   8 +
 .../pinctrl/nvidia,tegra124-dpaux-padctl.txt       |  60 +++++
 arch/arm64/boot/dts/nvidia/tegra210.dtsi           |  78 ++++++-
 drivers/gpu/drm/tegra/Kconfig                      |   1 +
 drivers/gpu/drm/tegra/dpaux.c                      | 255 +++++++++++++++++----
 drivers/i2c/i2c-core.c                             |  11 +-
 drivers/pinctrl/pinconf-generic.c                  |   8 +
 include/linux/pinctrl/pinconf-generic.h            |   2 +
 9 files changed, 380 insertions(+), 54 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-dpaux-padctl.txt

-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC PATCH 01/13] drm/tegra: Clean-up if probing DPAUX fails

[Jon Hunter]

If the probing of the DPAUX fails, then clocks are left enabled and the
DPAUX reset de-asserted. Add code to perform the necessary clean-up on
probe failure by disabling clocks and asserting the reset.

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/gpu/drm/tegra/dpaux.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index b24a0f14821a..0874a7e5b37b 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -321,28 +321,30 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
 	if (IS_ERR(dpaux->clk_parent)) {
 		dev_err(&pdev->dev, "failed to get parent clock: %ld\n",
 			PTR_ERR(dpaux->clk_parent));
-		return PTR_ERR(dpaux->clk_parent);
+		err = PTR_ERR(dpaux->clk_parent);
+		goto assert_reset;
 	}
 
 	err = clk_prepare_enable(dpaux->clk_parent);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to enable parent clock: %d\n",
 			err);
-		return err;
+		goto assert_reset;
 	}
 
 	err = clk_set_rate(dpaux->clk_parent, 270000000);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to set clock to 270 MHz: %d\n",
 			err);
-		return err;
+		goto disable_parent_clk;
 	}
 
 	dpaux->vdd = devm_regulator_get(&pdev->dev, "vdd");
 	if (IS_ERR(dpaux->vdd)) {
 		dev_err(&pdev->dev, "failed to get VDD supply: %ld\n",
 			PTR_ERR(dpaux->vdd));
-		return PTR_ERR(dpaux->vdd);
+		err = PTR_ERR(dpaux->vdd);
+		goto disable_parent_clk;
 	}
 
 	err = devm_request_irq(dpaux->dev, dpaux->irq, tegra_dpaux_irq, 0,
@@ -350,7 +352,7 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
 	if (err < 0) {
 		dev_err(dpaux->dev, "failed to request IRQ#%u: %d\n",
 			dpaux->irq, err);
-		return err;
+		goto disable_parent_clk;
 	}
 
 	disable_irq(dpaux->irq);
@@ -360,7 +362,7 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
 
 	err = drm_dp_aux_register(&dpaux->aux);
 	if (err < 0)
-		return err;
+		goto disable_parent_clk;
 
 	/*
 	 * Assume that by default the DPAUX/I2C pads will be used for HDMI,
@@ -393,6 +395,14 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, dpaux);
 
 	return 0;
+
+disable_parent_clk:
+	clk_disable_unprepare(dpaux->clk_parent);
+assert_reset:
+	reset_control_assert(dpaux->rst);
+	clk_disable_unprepare(dpaux->clk);
+
+	return err;
 }
 
 static int tegra_dpaux_remove(struct platform_device *pdev)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH 03/13] dt-bindings: drm/tegra: Update DPAUX documentation

[Jon Hunter]

Update the DPAUX compatibility string information for Tegra124, Tegra132
and Tegra210. For Tegra210 an additional clock, 'sor-safe' is also
required for DPAUX and so add this clock information as well.

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 .../devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt    | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
index a3bd8c050c4e..361a472eac4b 100644
--- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
+++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
@@ -226,9 +226,9 @@ of the following host1x client modules:
   - nvidia,dpaux: phandle to a DispayPort AUX interface
 
 - dpaux: DisplayPort AUX interface
-  - compatible: For Tegra124, must contain "nvidia,tegra124-dpaux".  Otherwise,
-    must contain '"nvidia,<chip>-dpaux", "nvidia,tegra124-dpaux"', where
-    <chip> is tegra132.
+  - compatible : Should contain one of the following:
+    - "nvidia,tegra124-dpaux": for Tegra124 and Tegra132
+    - "nvidia,tegra210-dpaux": for Tegra210
   - reg: Physical base address and length of the controller's registers.
   - interrupts: The interrupt outputs from the controller.
   - clocks: Must contain an entry for each entry in clock-names.
@@ -236,6 +236,7 @@ of the following host1x client modules:
   - clock-names: Must include the following entries:
     - dpaux: clock input for the DPAUX hardware
     - parent: reference clock
+    - sor-safe: additional clock input for the DPAUX hardware on Tegra210
   - resets: Must contain an entry for each entry in reset-names.
     See ../reset/reset.txt for details.
   - reset-names: Must include the following entries:
-- 
2.1.4

Re: [RFC PATCH 03/13] dt-bindings: drm/tegra: Update DPAUX documentation

[Thierry Reding]

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

On Fri, Jun 17, 2016 at 01:03:37PM +0100, Jon Hunter wrote:
> Update the DPAUX compatibility string information for Tegra124, Tegra132
> and Tegra210. For Tegra210 an additional clock, 'sor-safe' is also
> required for DPAUX and so add this clock information as well.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  .../devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt    | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> index a3bd8c050c4e..361a472eac4b 100644
> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> @@ -226,9 +226,9 @@ of the following host1x client modules:
>    - nvidia,dpaux: phandle to a DispayPort AUX interface
>  
>  - dpaux: DisplayPort AUX interface
> -  - compatible: For Tegra124, must contain "nvidia,tegra124-dpaux".  Otherwise,
> -    must contain '"nvidia,<chip>-dpaux", "nvidia,tegra124-dpaux"', where
> -    <chip> is tegra132.
> +  - compatible : Should contain one of the following:
> +    - "nvidia,tegra124-dpaux": for Tegra124 and Tegra132
> +    - "nvidia,tegra210-dpaux": for Tegra210
>    - reg: Physical base address and length of the controller's registers.
>    - interrupts: The interrupt outputs from the controller.
>    - clocks: Must contain an entry for each entry in clock-names.
> @@ -236,6 +236,7 @@ of the following host1x client modules:
>    - clock-names: Must include the following entries:
>      - dpaux: clock input for the DPAUX hardware
>      - parent: reference clock
> +    - sor-safe: additional clock input for the DPAUX hardware on Tegra210

It might be worth calling this simply "safe" because that's the
contextual role of the clock, whereas sor-safe is the system name of the
clock.

Thierry

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

Re: [RFC PATCH 03/13] dt-bindings: drm/tegra: Update DPAUX documentation

[Jon Hunter]

On 17/06/16 17:13, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Jun 17, 2016 at 01:03:37PM +0100, Jon Hunter wrote:
>> Update the DPAUX compatibility string information for Tegra124, Tegra132
>> and Tegra210. For Tegra210 an additional clock, 'sor-safe' is also
>> required for DPAUX and so add this clock information as well.
>>
>> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>>  .../devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt    | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>> index a3bd8c050c4e..361a472eac4b 100644
>> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>> @@ -226,9 +226,9 @@ of the following host1x client modules:
>>    - nvidia,dpaux: phandle to a DispayPort AUX interface
>>  
>>  - dpaux: DisplayPort AUX interface
>> -  - compatible: For Tegra124, must contain "nvidia,tegra124-dpaux".  Otherwise,
>> -    must contain '"nvidia,<chip>-dpaux", "nvidia,tegra124-dpaux"', where
>> -    <chip> is tegra132.
>> +  - compatible : Should contain one of the following:
>> +    - "nvidia,tegra124-dpaux": for Tegra124 and Tegra132
>> +    - "nvidia,tegra210-dpaux": for Tegra210
>>    - reg: Physical base address and length of the controller's registers.
>>    - interrupts: The interrupt outputs from the controller.
>>    - clocks: Must contain an entry for each entry in clock-names.
>> @@ -236,6 +236,7 @@ of the following host1x client modules:
>>    - clock-names: Must include the following entries:
>>      - dpaux: clock input for the DPAUX hardware
>>      - parent: reference clock
>> +    - sor-safe: additional clock input for the DPAUX hardware on Tegra210
> 
> It might be worth calling this simply "safe" because that's the
> contextual role of the clock, whereas sor-safe is the system name of the
> clock.

Yes that is fine with me.

Jon

-- 
nvpublic

[RFC PATCH 06/13] pinctrl: pinconf: Add generic helper function for freeing mappings

[Jon Hunter]

The pinconf-generic.h file exposes functions for creating generic mappings
but it does not expose a function for freeing the mappings. Add a function
for freeing generic mappings.

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/pinctrl/pinconf-generic.c       | 8 ++++++++
 include/linux/pinctrl/pinconf-generic.h | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index 34b601b06764..5020ae534479 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -395,4 +395,12 @@ exit:
 }
 EXPORT_SYMBOL_GPL(pinconf_generic_dt_node_to_map);
 
+void pinconf_generic_dt_free_map(struct pinctrl_dev *pctldev,
+				 struct pinctrl_map *map,
+				 unsigned num_maps)
+{
+	pinctrl_utils_free_map(pctldev, map, num_maps);
+}
+EXPORT_SYMBOL_GPL(pinconf_generic_dt_free_map);
+
 #endif
diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
index d921afd5f109..12343caa114e 100644
--- a/include/linux/pinctrl/pinconf-generic.h
+++ b/include/linux/pinctrl/pinconf-generic.h
@@ -175,6 +175,8 @@ int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 int pinconf_generic_dt_node_to_map(struct pinctrl_dev *pctldev,
 		struct device_node *np_config, struct pinctrl_map **map,
 		unsigned *num_maps, enum pinctrl_map_type type);
+void pinconf_generic_dt_free_map(struct pinctrl_dev *pctldev,
+		struct pinctrl_map *map, unsigned num_maps);
 
 static inline int pinconf_generic_dt_node_to_map_group(
 		struct pinctrl_dev *pctldev, struct device_node *np_config,
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 06/13] pinctrl: pinconf: Add generic helper function for freeing mappings

[Linus Walleij]

On Fri, Jun 17, 2016 at 2:03 PM, Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:

> The pinconf-generic.h file exposes functions for creating generic mappings
> but it does not expose a function for freeing the mappings. Add a function
> for freeing generic mappings.
>
> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Looks reasonable. Can I apply it in isolation or do you need to keep
it in your series with my ACK?

Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 06/13] pinctrl: pinconf: Add generic helper function for freeing mappings

[Jon Hunter]

On 18/06/16 10:04, Linus Walleij wrote:
> On Fri, Jun 17, 2016 at 2:03 PM, Jon Hunter <jonathanh@nvidia.com> wrote:
> 
>> The pinconf-generic.h file exposes functions for creating generic mappings
>> but it does not expose a function for freeing the mappings. Add a function
>> for freeing generic mappings.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> 
> Looks reasonable. Can I apply it in isolation or do you need to keep
> it in your series with my ACK?

There is definitely a build dependency on this patch for the DPAUX
driver. Thierry was suggesting (in response to patch 00/13) that he take
this through the drm/tegra branch if you are ok with that?

> Acked-by: Linus Walleij <linus.walleij@linaro.org>

Thanks.
Jon

-- 
nvpublic
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC PATCH 13/13] arm64: tegra: Add DPAUX pinctrl bindings

[Jon Hunter]

Add the DPAUX pinctrl states for the DPAUX nodes defining all three
possible states of "aux", "i2c" and "off". Also add the 'i2c-bus'
node for the DPAUX nodes so that the I2C driver core does not attempt
to parse the pinctrl state nodes.

Populate the nodes for the pinctrl clients of the DPAUX pin controller.
There are two clients for each DPAUX instance, namely the SOR and one of
the I2C adapters. The SOR clients may used the DPAUX pins in either AUX
or I2C modes and so for these devices we don't define any of the generic
pinctrl states (default, idle, etc) because the SOR driver will directly
set the state needed. For I2C clients only the I2C mode is used and so
we can simplify matters by using the generic pinctrl states for default
and idle.

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi | 48 ++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 78bcc87b627d..c4ae0215291a 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -37,6 +37,23 @@
 			reset-names = "dpaux";
 			power-domains = <&pd_sor>;
 			status = "disabled";
+
+			state_dpaux1_aux: pinmux_aux {
+				groups = "dpaux-io";
+				function = "aux";
+			};
+
+			state_dpaux1_i2c: pinmux_i2c {
+				groups = "dpaux-io";
+				function = "i2c";
+			};
+
+			state_dpaux1_off: pinmux_off {
+				groups = "dpaux-io";
+				function = "off";
+			};
+
+			i2c-bus { };
 		};
 
 		vi@54080000 {
@@ -156,6 +173,10 @@
 			clock-names = "sor", "parent", "dp", "safe";
 			resets = <&tegra_car 182>;
 			reset-names = "sor";
+			pinctrl-0 = <&state_dpaux_aux>;
+			pinctrl-1 = <&state_dpaux_i2c>;
+			pinctrl-2 = <&state_dpaux_off>;
+			pinctrl-names = "aux", "i2c", "off";
 			power-domains = <&pd_sor>;
 			status = "disabled";
 		};
@@ -171,6 +192,10 @@
 			clock-names = "sor", "parent", "dp", "safe";
 			resets = <&tegra_car 183>;
 			reset-names = "sor";
+			pinctrl-0 = <&state_dpaux1_aux>;
+			pinctrl-1 = <&state_dpaux1_i2c>;
+			pinctrl-2 = <&state_dpaux1_off>;
+			pinctrl-names = "aux", "i2c", "off";
 			power-domains = <&pd_sor>;
 			status = "disabled";
 		};
@@ -187,6 +212,23 @@
 			reset-names = "dpaux";
 			power-domains = <&pd_sor>;
 			status = "disabled";
+
+			state_dpaux_aux: pinmux_aux {
+				groups = "dpaux-io";
+				function = "aux";
+			};
+
+			state_dpaux_i2c: pinmux_i2c {
+				groups = "dpaux-io";
+				function = "i2c";
+			};
+
+			state_dpaux_off: pinmux_off {
+				groups = "dpaux-io";
+				function = "off";
+			};
+
+			i2c-bus { };
 		};
 
 		isp@54600000 {
@@ -484,6 +526,9 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 26>, <&apbdma 26>;
 		dma-names = "rx", "tx";
+		pinctrl-0 = <&state_dpaux1_i2c>;
+		pinctrl-1 = <&state_dpaux1_off>;
+		pinctrl-names = "default", "idle";
 		status = "disabled";
 	};
 
@@ -514,6 +559,9 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 30>, <&apbdma 30>;
 		dma-names = "rx", "tx";
+		pinctrl-0 = <&state_dpaux_i2c>;
+		pinctrl-1 = <&state_dpaux_off>;
+		pinctrl-names = "default", "idle";
 		status = "disabled";
 	};
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 13/13] arm64: tegra: Add DPAUX pinctrl bindings

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1109 bytes --]

On Fri, Jun 17, 2016 at 01:03:47PM +0100, Jon Hunter wrote:
> Add the DPAUX pinctrl states for the DPAUX nodes defining all three
> possible states of "aux", "i2c" and "off". Also add the 'i2c-bus'
> node for the DPAUX nodes so that the I2C driver core does not attempt
> to parse the pinctrl state nodes.
> 
> Populate the nodes for the pinctrl clients of the DPAUX pin controller.
> There are two clients for each DPAUX instance, namely the SOR and one of
> the I2C adapters. The SOR clients may used the DPAUX pins in either AUX
> or I2C modes and so for these devices we don't define any of the generic
> pinctrl states (default, idle, etc) because the SOR driver will directly
> set the state needed. For I2C clients only the I2C mode is used and so
> we can simplify matters by using the generic pinctrl states for default
> and idle.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  arch/arm64/boot/dts/nvidia/tegra210.dtsi | 48 ++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)

Looks good to me. Thanks for driving this to conclusion.

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 13/13] arm64: tegra: Add DPAUX pinctrl bindings

[Linus Walleij]

On Fri, Jun 17, 2016 at 2:03 PM, Jon Hunter <jonathanh@nvidia.com> wrote:

> Add the DPAUX pinctrl states for the DPAUX nodes defining all three
> possible states of "aux", "i2c" and "off". Also add the 'i2c-bus'
> node for the DPAUX nodes so that the I2C driver core does not attempt
> to parse the pinctrl state nodes.
>
> Populate the nodes for the pinctrl clients of the DPAUX pin controller.
> There are two clients for each DPAUX instance, namely the SOR and one of
> the I2C adapters. The SOR clients may used the DPAUX pins in either AUX
> or I2C modes and so for these devices we don't define any of the generic
> pinctrl states (default, idle, etc) because the SOR driver will directly
> set the state needed. For I2C clients only the I2C mode is used and so
> we can simplify matters by using the generic pinctrl states for default
> and idle.
>
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours
Linus Walleij

Re: [RFC PATCH 00/13] Add support for Tegra DPAUX pinctrl

[Thierry Reding]

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

On Fri, Jun 17, 2016 at 01:03:34PM +0100, Jon Hunter wrote:
> The Display Port Auxiliary (DPAUX) channel pads can be shared with an
> internal I2C controller. Add pinctrl support for these pads so that the
> I2C controller can request and use these pads.
> 
> Jon Hunter (13):
>   drm/tegra: Clean-up if probing DPAUX fails
>   drm/tegra: Add helper functions for setting up DPAUX pads
>   dt-bindings: drm/tegra: Update DPAUX documentation
>   drm/tegra: Add sor-safe clock for DPAUX on Tegra210
>   drm/tegra: Prepare DPAUX for supporting generic PM domains
>   pinctrl: pinconf: Add generic helper function for freeing mappings
>   dt-bindings: i2c: Add support for 'i2c-bus' subnode
>   i2c: core: Add support for 'i2c-bus' subnode
>   dt-bindings: drm/tegra: Add DPAUX pinctrl documentation
>   drm/tegra: Add pinctrl support for DPAUX
>   arm64: tegra: Add SOR power-domain node
>   arm64: tegra: Add sor-safe clock to DPAUX binding
>   arm64: tegra: Add DPAUX pinctrl bindings

There aren't really any hard dependencies between all these patches,
right? I think the worst case would be if the arm64 DTS changes get
merged before the I2C core changes (i2c-bus node support), then the
I2C core will complain about the pinmux nodes, but that wouldn't be
fatal, or have any bad side-effects, right?

If so, I think it would be fine if the I2C changes went through the
I2C tree. It might be nicer to have the I2C changes in a separate branch
that could be pulled into the Tegra tree so that we can get everything
ready there and avoid the warnings.

Wolfram, if you agree I can apply the I2C patches (binding + core) to a
stable branch and send out a pull request? That is, once Jon's addressed
any comments and you are onboard with the change.

Thierry

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

Re: [RFC PATCH 00/13] Add support for Tegra DPAUX pinctrl

[Thierry Reding]

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

On Fri, Jun 17, 2016 at 06:56:11PM +0200, Thierry Reding wrote:
> On Fri, Jun 17, 2016 at 01:03:34PM +0100, Jon Hunter wrote:
> > The Display Port Auxiliary (DPAUX) channel pads can be shared with an
> > internal I2C controller. Add pinctrl support for these pads so that the
> > I2C controller can request and use these pads.
> > 
> > Jon Hunter (13):
> >   drm/tegra: Clean-up if probing DPAUX fails
> >   drm/tegra: Add helper functions for setting up DPAUX pads
> >   dt-bindings: drm/tegra: Update DPAUX documentation
> >   drm/tegra: Add sor-safe clock for DPAUX on Tegra210
> >   drm/tegra: Prepare DPAUX for supporting generic PM domains
> >   pinctrl: pinconf: Add generic helper function for freeing mappings
> >   dt-bindings: i2c: Add support for 'i2c-bus' subnode
> >   i2c: core: Add support for 'i2c-bus' subnode
> >   dt-bindings: drm/tegra: Add DPAUX pinctrl documentation
> >   drm/tegra: Add pinctrl support for DPAUX
> >   arm64: tegra: Add SOR power-domain node
> >   arm64: tegra: Add sor-safe clock to DPAUX binding
> >   arm64: tegra: Add DPAUX pinctrl bindings
> 
> There aren't really any hard dependencies between all these patches,
> right? I think the worst case would be if the arm64 DTS changes get
> merged before the I2C core changes (i2c-bus node support), then the
> I2C core will complain about the pinmux nodes, but that wouldn't be
> fatal, or have any bad side-effects, right?

Oh wait... there's the pinctrl helper function that is a build-
dependency. Linus, would you be okay if I took that through the
drm-tegra tree along with the DPAUX driver change, and provide a
stable branch for you to resolve conflicts against if needed?

Thierry

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

Re: [RFC PATCH 00/13] Add support for Tegra DPAUX pinctrl

[Linus Walleij]

On Fri, Jun 17, 2016 at 6:58 PM, Thierry Reding
<thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Oh wait... there's the pinctrl helper function that is a build-
> dependency. Linus, would you be okay if I took that through the
> drm-tegra tree along with the DPAUX driver change, and provide a
> stable branch for you to resolve conflicts against if needed?

I have already applied it, but I can rebase and pull that patch out
on a separate immutable branch and then  merge that branch
into my devel and then you can pull it too.

Would that work?

Yours,
Linus Walleij

Re: [RFC PATCH 00/13] Add support for Tegra DPAUX pinctrl

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 683 bytes --]

On Thu, Jun 23, 2016 at 09:49:20AM +0200, Linus Walleij wrote:
> On Fri, Jun 17, 2016 at 6:58 PM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> 
> > Oh wait... there's the pinctrl helper function that is a build-
> > dependency. Linus, would you be okay if I took that through the
> > drm-tegra tree along with the DPAUX driver change, and provide a
> > stable branch for you to resolve conflicts against if needed?
> 
> I have already applied it, but I can rebase and pull that patch out
> on a separate immutable branch and then  merge that branch
> into my devel and then you can pull it too.
> 
> Would that work?

Yes, that would be just fine.

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 00/13] Add support for Tegra DPAUX pinctrl

[Linus Walleij]

On Thu, Jun 23, 2016 at 10:04 AM, Thierry Reding
<thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, Jun 23, 2016 at 09:49:20AM +0200, Linus Walleij wrote:
>> On Fri, Jun 17, 2016 at 6:58 PM, Thierry Reding
>> <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>> > Oh wait... there's the pinctrl helper function that is a build-
>> > dependency. Linus, would you be okay if I took that through the
>> > drm-tegra tree along with the DPAUX driver change, and provide a
>> > stable branch for you to resolve conflicts against if needed?
>>
>> I have already applied it, but I can rebase and pull that patch out
>> on a separate immutable branch and then  merge that branch
>> into my devel and then you can pull it too.
>>
>> Would that work?
>
> Yes, that would be just fine.

OK use this:
https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel-dt-free-map

I merged that into my devel branch too.

Yours,
Linus Walleij

[RFC PATCH 02/13] drm/tegra: Add helper functions for setting up DPAUX pads

[Jon Hunter]

In preparation for adding pinctrl support for the DPAUX pads, add
helpers functions for configuring the pads and controlling the power
for the pads.

Please note that although a simple if-statement could be used instead
of a case statement for configuring the pads as there are only two
possible modes, a case statement is used because when integrating with
the pinctrl framework, we need to be able to handle invalid modes that
could be passed.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/gpu/drm/tegra/dpaux.c | 75 ++++++++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index 0874a7e5b37b..aa3a037fcd3b 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -267,6 +267,45 @@ static irqreturn_t tegra_dpaux_irq(int irq, void *data)
 	return ret;
 }
 
+static void tegra_dpaux_powerdown(struct tegra_dpaux *dpaux, bool enable)
+{
+	u32 value = tegra_dpaux_readl(dpaux, DPAUX_HYBRID_SPARE);
+
+	if (enable)
+		value |= DPAUX_HYBRID_SPARE_PAD_POWER_DOWN;
+	else
+		value &= ~DPAUX_HYBRID_SPARE_PAD_POWER_DOWN;
+
+	tegra_dpaux_writel(dpaux, value, DPAUX_HYBRID_SPARE);
+}
+
+static int tegra_dpaux_config(struct tegra_dpaux *dpaux, int function)
+{
+	u32 value;
+
+	switch (function) {
+	case DPAUX_HYBRID_PADCTL_MODE_AUX:
+		value = DPAUX_HYBRID_PADCTL_AUX_CMH(2) |
+			DPAUX_HYBRID_PADCTL_AUX_DRVZ(4) |
+			DPAUX_HYBRID_PADCTL_AUX_DRVI(0x18) |
+			DPAUX_HYBRID_PADCTL_AUX_INPUT_RCV |
+			DPAUX_HYBRID_PADCTL_MODE_AUX;
+		break;
+	case DPAUX_HYBRID_PADCTL_MODE_I2C:
+		value = DPAUX_HYBRID_PADCTL_I2C_SDA_INPUT_RCV |
+			DPAUX_HYBRID_PADCTL_I2C_SCL_INPUT_RCV |
+			DPAUX_HYBRID_PADCTL_MODE_I2C;
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+
+	tegra_dpaux_writel(dpaux, value, DPAUX_HYBRID_PADCTL);
+	tegra_dpaux_powerdown(dpaux, false);
+
+	return 0;
+}
+
 static int tegra_dpaux_probe(struct platform_device *pdev)
 {
 	struct tegra_dpaux *dpaux;
@@ -372,15 +411,9 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
 	 * is no possibility to perform the I2C mode configuration in the
 	 * HDMI path.
 	 */
-	value = tegra_dpaux_readl(dpaux, DPAUX_HYBRID_SPARE);
-	value &= ~DPAUX_HYBRID_SPARE_PAD_POWER_DOWN;
-	tegra_dpaux_writel(dpaux, value, DPAUX_HYBRID_SPARE);
-
-	value = tegra_dpaux_readl(dpaux, DPAUX_HYBRID_PADCTL);
-	value = DPAUX_HYBRID_PADCTL_I2C_SDA_INPUT_RCV |
-		DPAUX_HYBRID_PADCTL_I2C_SCL_INPUT_RCV |
-		DPAUX_HYBRID_PADCTL_MODE_I2C;
-	tegra_dpaux_writel(dpaux, value, DPAUX_HYBRID_PADCTL);
+	err = tegra_dpaux_config(dpaux, DPAUX_HYBRID_PADCTL_MODE_I2C);
+	if (err < 0)
+		return err;
 
 	/* enable and clear all interrupts */
 	value = DPAUX_INTR_AUX_DONE | DPAUX_INTR_IRQ_EVENT |
@@ -408,12 +441,9 @@ assert_reset:
 static int tegra_dpaux_remove(struct platform_device *pdev)
 {
 	struct tegra_dpaux *dpaux = platform_get_drvdata(pdev);
-	u32 value;
 
 	/* make sure pads are powered down when not in use */
-	value = tegra_dpaux_readl(dpaux, DPAUX_HYBRID_SPARE);
-	value |= DPAUX_HYBRID_SPARE_PAD_POWER_DOWN;
-	tegra_dpaux_writel(dpaux, value, DPAUX_HYBRID_SPARE);
+	tegra_dpaux_powerdown(dpaux, true);
 
 	drm_dp_aux_unregister(&dpaux->aux);
 
@@ -538,30 +568,15 @@ enum drm_connector_status drm_dp_aux_detect(struct drm_dp_aux *aux)
 int drm_dp_aux_enable(struct drm_dp_aux *aux)
 {
 	struct tegra_dpaux *dpaux = to_dpaux(aux);
-	u32 value;
-
-	value = DPAUX_HYBRID_PADCTL_AUX_CMH(2) |
-		DPAUX_HYBRID_PADCTL_AUX_DRVZ(4) |
-		DPAUX_HYBRID_PADCTL_AUX_DRVI(0x18) |
-		DPAUX_HYBRID_PADCTL_AUX_INPUT_RCV |
-		DPAUX_HYBRID_PADCTL_MODE_AUX;
-	tegra_dpaux_writel(dpaux, value, DPAUX_HYBRID_PADCTL);
-
-	value = tegra_dpaux_readl(dpaux, DPAUX_HYBRID_SPARE);
-	value &= ~DPAUX_HYBRID_SPARE_PAD_POWER_DOWN;
-	tegra_dpaux_writel(dpaux, value, DPAUX_HYBRID_SPARE);
 
-	return 0;
+	return tegra_dpaux_config(dpaux, DPAUX_HYBRID_PADCTL_MODE_AUX);
 }
 
 int drm_dp_aux_disable(struct drm_dp_aux *aux)
 {
 	struct tegra_dpaux *dpaux = to_dpaux(aux);
-	u32 value;
 
-	value = tegra_dpaux_readl(dpaux, DPAUX_HYBRID_SPARE);
-	value |= DPAUX_HYBRID_SPARE_PAD_POWER_DOWN;
-	tegra_dpaux_writel(dpaux, value, DPAUX_HYBRID_SPARE);
+	tegra_dpaux_powerdown(dpaux, true);
 
 	return 0;
 }
-- 
2.1.4

Re: [RFC PATCH 02/13] drm/tegra: Add helper functions for setting up DPAUX pads

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1973 bytes --]

On Fri, Jun 17, 2016 at 01:03:36PM +0100, Jon Hunter wrote:
> In preparation for adding pinctrl support for the DPAUX pads, add
> helpers functions for configuring the pads and controlling the power
> for the pads.
> 
> Please note that although a simple if-statement could be used instead
> of a case statement for configuring the pads as there are only two
> possible modes, a case statement is used because when integrating with
> the pinctrl framework, we need to be able to handle invalid modes that
> could be passed.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/dpaux.c | 75 ++++++++++++++++++++++++++-----------------
>  1 file changed, 45 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
> index 0874a7e5b37b..aa3a037fcd3b 100644
> --- a/drivers/gpu/drm/tegra/dpaux.c
> +++ b/drivers/gpu/drm/tegra/dpaux.c
> @@ -267,6 +267,45 @@ static irqreturn_t tegra_dpaux_irq(int irq, void *data)
>  	return ret;
>  }
>  
> +static void tegra_dpaux_powerdown(struct tegra_dpaux *dpaux, bool enable)
> +{
> +	u32 value = tegra_dpaux_readl(dpaux, DPAUX_HYBRID_SPARE);
> +
> +	if (enable)
> +		value |= DPAUX_HYBRID_SPARE_PAD_POWER_DOWN;
> +	else
> +		value &= ~DPAUX_HYBRID_SPARE_PAD_POWER_DOWN;
> +
> +	tegra_dpaux_writel(dpaux, value, DPAUX_HYBRID_SPARE);
> +}

I'd like for this to be two functions without the boolean parameter. The
reason is that without looking at the implementation there's no way to
understand what the meaning of true and false is. If instead you call
this:

	static void tegra_dpaux_pad_power_down(struct tegra_dpaux *dpaux)
	{
		...
	}

and

	static void tegra_dpaux_pad_power_up(struct tegra_dpaux *dpaux)
	{
		...
	}

you can easily deduce from the name what's going on.

> +static int tegra_dpaux_config(struct tegra_dpaux *dpaux, int function)

Can function not be unsigned?

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 02/13] drm/tegra: Add helper functions for setting up DPAUX pads

[Jon Hunter]

On 17/06/16 17:11, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Jun 17, 2016 at 01:03:36PM +0100, Jon Hunter wrote:
>> In preparation for adding pinctrl support for the DPAUX pads, add
>> helpers functions for configuring the pads and controlling the power
>> for the pads.
>>
>> Please note that although a simple if-statement could be used instead
>> of a case statement for configuring the pads as there are only two
>> possible modes, a case statement is used because when integrating with
>> the pinctrl framework, we need to be able to handle invalid modes that
>> could be passed.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  drivers/gpu/drm/tegra/dpaux.c | 75 ++++++++++++++++++++++++++-----------------
>>  1 file changed, 45 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
>> index 0874a7e5b37b..aa3a037fcd3b 100644
>> --- a/drivers/gpu/drm/tegra/dpaux.c
>> +++ b/drivers/gpu/drm/tegra/dpaux.c
>> @@ -267,6 +267,45 @@ static irqreturn_t tegra_dpaux_irq(int irq, void *data)
>>  	return ret;
>>  }
>>  
>> +static void tegra_dpaux_powerdown(struct tegra_dpaux *dpaux, bool enable)
>> +{
>> +	u32 value = tegra_dpaux_readl(dpaux, DPAUX_HYBRID_SPARE);
>> +
>> +	if (enable)
>> +		value |= DPAUX_HYBRID_SPARE_PAD_POWER_DOWN;
>> +	else
>> +		value &= ~DPAUX_HYBRID_SPARE_PAD_POWER_DOWN;
>> +
>> +	tegra_dpaux_writel(dpaux, value, DPAUX_HYBRID_SPARE);
>> +}
> 
> I'd like for this to be two functions without the boolean parameter. The
> reason is that without looking at the implementation there's no way to
> understand what the meaning of true and false is. If instead you call
> this:
> 
> 	static void tegra_dpaux_pad_power_down(struct tegra_dpaux *dpaux)
> 	{
> 		...
> 	}
> 
> and
> 
> 	static void tegra_dpaux_pad_power_up(struct tegra_dpaux *dpaux)
> 	{
> 		...
> 	}
> 
> you can easily deduce from the name what's going on.

OK, fine with me. I was not sure if it was obvious or not.

>> +static int tegra_dpaux_config(struct tegra_dpaux *dpaux, int function)
> 
> Can function not be unsigned?

Yes it can and in fact should be. I had thought that it needed to be
signed to match with the pinctrl-ops but on closer inspection that is
not the case.

Jon

-- 
nvpublic
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC PATCH 04/13] drm/tegra: Add sor-safe clock for DPAUX on Tegra210

[Jon Hunter]

For Tegra210 the 'sor-safe' clock needs to be enabled when using DPAUX.
Add support to the DPAUX driver for enabling this clock on Tegra210.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/gpu/drm/tegra/dpaux.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index aa3a037fcd3b..d696a7e45935 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -37,6 +37,7 @@ struct tegra_dpaux {
 
 	struct reset_control *rst;
 	struct clk *clk_parent;
+	struct clk *clk_sor;
 	struct clk *clk;
 
 	struct regulator *vdd;
@@ -340,18 +341,37 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
 		return PTR_ERR(dpaux->rst);
 	}
 
+	if (of_device_is_compatible(pdev->dev.of_node,
+				    "nvidia,tegra210-dpaux")) {
+		dpaux->clk_sor = devm_clk_get(&pdev->dev, "sor-safe");
+		if (IS_ERR(dpaux->clk_sor)) {
+			dev_err(&pdev->dev,
+				"failed to get sor-safe clock: %ld\n",
+				PTR_ERR(dpaux->clk_sor));
+			return PTR_ERR(dpaux->clk_sor);
+		}
+
+		err = clk_prepare_enable(dpaux->clk_sor);
+		if (err < 0) {
+			dev_err(&pdev->dev,
+				"failed to enable sor-safe clock: %d\n", err);
+			return err;
+		}
+	}
+
 	dpaux->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(dpaux->clk)) {
 		dev_err(&pdev->dev, "failed to get module clock: %ld\n",
 			PTR_ERR(dpaux->clk));
-		return PTR_ERR(dpaux->clk);
+		err = PTR_ERR(dpaux->clk);
+		goto disable_sor_clk;
 	}
 
 	err = clk_prepare_enable(dpaux->clk);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to enable module clock: %d\n",
 			err);
-		return err;
+		goto disable_sor_clk;
 	}
 
 	reset_control_deassert(dpaux->rst);
@@ -434,6 +454,9 @@ disable_parent_clk:
 assert_reset:
 	reset_control_assert(dpaux->rst);
 	clk_disable_unprepare(dpaux->clk);
+disable_sor_clk:
+	if (dpaux->clk_sor)
+		clk_disable_unprepare(dpaux->clk_sor);
 
 	return err;
 }
@@ -456,6 +479,8 @@ static int tegra_dpaux_remove(struct platform_device *pdev)
 	clk_disable_unprepare(dpaux->clk_parent);
 	reset_control_assert(dpaux->rst);
 	clk_disable_unprepare(dpaux->clk);
+	if (dpaux->clk_sor)
+		clk_disable_unprepare(dpaux->clk_sor);
 
 	return 0;
 }
-- 
2.1.4

Re: [RFC PATCH 04/13] drm/tegra: Add sor-safe clock for DPAUX on Tegra210

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2929 bytes --]

On Fri, Jun 17, 2016 at 01:03:38PM +0100, Jon Hunter wrote:
> For Tegra210 the 'sor-safe' clock needs to be enabled when using DPAUX.
> Add support to the DPAUX driver for enabling this clock on Tegra210.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/dpaux.c | 29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
> index aa3a037fcd3b..d696a7e45935 100644
> --- a/drivers/gpu/drm/tegra/dpaux.c
> +++ b/drivers/gpu/drm/tegra/dpaux.c
> @@ -37,6 +37,7 @@ struct tegra_dpaux {
>  
>  	struct reset_control *rst;
>  	struct clk *clk_parent;
> +	struct clk *clk_sor;

Can we call this "clk_safe", please? On one hand that mirrors the name
of the clock in the binding and on the other hand it avoids confusion
with the real SOR clock.

>  	struct clk *clk;
>  
>  	struct regulator *vdd;
> @@ -340,18 +341,37 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
>  		return PTR_ERR(dpaux->rst);
>  	}
>  
> +	if (of_device_is_compatible(pdev->dev.of_node,
> +				    "nvidia,tegra210-dpaux")) {
> +		dpaux->clk_sor = devm_clk_get(&pdev->dev, "sor-safe");
> +		if (IS_ERR(dpaux->clk_sor)) {
> +			dev_err(&pdev->dev,
> +				"failed to get sor-safe clock: %ld\n",
> +				PTR_ERR(dpaux->clk_sor));
> +			return PTR_ERR(dpaux->clk_sor);
> +		}
> +
> +		err = clk_prepare_enable(dpaux->clk_sor);
> +		if (err < 0) {
> +			dev_err(&pdev->dev,
> +				"failed to enable sor-safe clock: %d\n", err);
> +			return err;
> +		}
> +	}

Please make this part of a struct tegra_dpaux_soc, so that we don't have
to check the compatible string again here. This could look like:

	struct tegra_dpaux_soc {
		bool needs_safe_clock;
	};

	static const struct tegra_dpaux_soc tegra124_dpaux_soc = {
		.needs_safe_clock = false,
	};

	static const struct tegra_dpaux_soc tegra210_dpaux_soc = {
		.needs_safe_clock = true,
	};

	...

	static const struct of_device_id tegra_dpaux_of_match[] = {
		{ .compatible = "nvidia,tegra210-dpaux", .data = &tegra210_dpaux_soc },
		{ .compatible = "nvidia,tegra124-dpaux", .data = &tegra124_dpaux_soc },
		{ },
	};

> @@ -434,6 +454,9 @@ disable_parent_clk:
>  assert_reset:
>  	reset_control_assert(dpaux->rst);
>  	clk_disable_unprepare(dpaux->clk);
> +disable_sor_clk:
> +	if (dpaux->clk_sor)
> +		clk_disable_unprepare(dpaux->clk_sor);

You can drop the extra check here, since the common clock framework
ignores NULL or ERR_PTR() pointers.

>  
>  	return err;
>  }
> @@ -456,6 +479,8 @@ static int tegra_dpaux_remove(struct platform_device *pdev)
>  	clk_disable_unprepare(dpaux->clk_parent);
>  	reset_control_assert(dpaux->rst);
>  	clk_disable_unprepare(dpaux->clk);
> +	if (dpaux->clk_sor)
> +		clk_disable_unprepare(dpaux->clk_sor);

Same here.

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 04/13] drm/tegra: Add sor-safe clock for DPAUX on Tegra210

[Jon Hunter]

On 17/06/16 17:18, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Jun 17, 2016 at 01:03:38PM +0100, Jon Hunter wrote:
>> For Tegra210 the 'sor-safe' clock needs to be enabled when using DPAUX.
>> Add support to the DPAUX driver for enabling this clock on Tegra210.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  drivers/gpu/drm/tegra/dpaux.c | 29 +++++++++++++++++++++++++++--
>>  1 file changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
>> index aa3a037fcd3b..d696a7e45935 100644
>> --- a/drivers/gpu/drm/tegra/dpaux.c
>> +++ b/drivers/gpu/drm/tegra/dpaux.c
>> @@ -37,6 +37,7 @@ struct tegra_dpaux {
>>  
>>  	struct reset_control *rst;
>>  	struct clk *clk_parent;
>> +	struct clk *clk_sor;
> 
> Can we call this "clk_safe", please? On one hand that mirrors the name
> of the clock in the binding and on the other hand it avoids confusion
> with the real SOR clock.

OK.

>>  	struct clk *clk;
>>  
>>  	struct regulator *vdd;
>> @@ -340,18 +341,37 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
>>  		return PTR_ERR(dpaux->rst);
>>  	}
>>  
>> +	if (of_device_is_compatible(pdev->dev.of_node,
>> +				    "nvidia,tegra210-dpaux")) {
>> +		dpaux->clk_sor = devm_clk_get(&pdev->dev, "sor-safe");
>> +		if (IS_ERR(dpaux->clk_sor)) {
>> +			dev_err(&pdev->dev,
>> +				"failed to get sor-safe clock: %ld\n",
>> +				PTR_ERR(dpaux->clk_sor));
>> +			return PTR_ERR(dpaux->clk_sor);
>> +		}
>> +
>> +		err = clk_prepare_enable(dpaux->clk_sor);
>> +		if (err < 0) {
>> +			dev_err(&pdev->dev,
>> +				"failed to enable sor-safe clock: %d\n", err);
>> +			return err;
>> +		}
>> +	}
> 
> Please make this part of a struct tegra_dpaux_soc, so that we don't have
> to check the compatible string again here. This could look like:
> 
> 	struct tegra_dpaux_soc {
> 		bool needs_safe_clock;
> 	};
> 
> 	static const struct tegra_dpaux_soc tegra124_dpaux_soc = {
> 		.needs_safe_clock = false,
> 	};
> 
> 	static const struct tegra_dpaux_soc tegra210_dpaux_soc = {
> 		.needs_safe_clock = true,
> 	};
> 
> 	...
> 
> 	static const struct of_device_id tegra_dpaux_of_match[] = {
> 		{ .compatible = "nvidia,tegra210-dpaux", .data = &tegra210_dpaux_soc },
> 		{ .compatible = "nvidia,tegra124-dpaux", .data = &tegra124_dpaux_soc },
> 		{ },
> 	};

OK. I wonder if we should call it 'has_safe_clock' because this clock
does not exist for tegra124 AFAICT. #bikeshed ;-)

>> @@ -434,6 +454,9 @@ disable_parent_clk:
>>  assert_reset:
>>  	reset_control_assert(dpaux->rst);
>>  	clk_disable_unprepare(dpaux->clk);
>> +disable_sor_clk:
>> +	if (dpaux->clk_sor)
>> +		clk_disable_unprepare(dpaux->clk_sor);
> 
> You can drop the extra check here, since the common clock framework
> ignores NULL or ERR_PTR() pointers.

OK.

>>  
>>  	return err;
>>  }
>> @@ -456,6 +479,8 @@ static int tegra_dpaux_remove(struct platform_device *pdev)
>>  	clk_disable_unprepare(dpaux->clk_parent);
>>  	reset_control_assert(dpaux->rst);
>>  	clk_disable_unprepare(dpaux->clk);
>> +	if (dpaux->clk_sor)
>> +		clk_disable_unprepare(dpaux->clk_sor);
> 
> Same here.

OK.

Cheers
Jon

-- 
nvpublic
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 04/13] drm/tegra: Add sor-safe clock for DPAUX on Tegra210

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1748 bytes --]

On Mon, Jun 20, 2016 at 09:43:42AM +0100, Jon Hunter wrote:
> On 17/06/16 17:18, Thierry Reding wrote:
> > On Fri, Jun 17, 2016 at 01:03:38PM +0100, Jon Hunter wrote:
> >> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
[...]
> >> +	if (of_device_is_compatible(pdev->dev.of_node,
> >> +				    "nvidia,tegra210-dpaux")) {
> >> +		dpaux->clk_sor = devm_clk_get(&pdev->dev, "sor-safe");
> >> +		if (IS_ERR(dpaux->clk_sor)) {
> >> +			dev_err(&pdev->dev,
> >> +				"failed to get sor-safe clock: %ld\n",
> >> +				PTR_ERR(dpaux->clk_sor));
> >> +			return PTR_ERR(dpaux->clk_sor);
> >> +		}
> >> +
> >> +		err = clk_prepare_enable(dpaux->clk_sor);
> >> +		if (err < 0) {
> >> +			dev_err(&pdev->dev,
> >> +				"failed to enable sor-safe clock: %d\n", err);
> >> +			return err;
> >> +		}
> >> +	}
> > 
> > Please make this part of a struct tegra_dpaux_soc, so that we don't have
> > to check the compatible string again here. This could look like:
> > 
> > 	struct tegra_dpaux_soc {
> > 		bool needs_safe_clock;
> > 	};
> > 
> > 	static const struct tegra_dpaux_soc tegra124_dpaux_soc = {
> > 		.needs_safe_clock = false,
> > 	};
> > 
> > 	static const struct tegra_dpaux_soc tegra210_dpaux_soc = {
> > 		.needs_safe_clock = true,
> > 	};
> > 
> > 	...
> > 
> > 	static const struct of_device_id tegra_dpaux_of_match[] = {
> > 		{ .compatible = "nvidia,tegra210-dpaux", .data = &tegra210_dpaux_soc },
> > 		{ .compatible = "nvidia,tegra124-dpaux", .data = &tegra124_dpaux_soc },
> > 		{ },
> > 	};
> 
> OK. I wonder if we should call it 'has_safe_clock' because this clock
> does not exist for tegra124 AFAICT. #bikeshed ;-)

has_safe_clock is fine with me, too.

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC PATCH 05/13] drm/tegra: Prepare DPAUX for supporting generic PM domains

[Jon Hunter]

To utilise the DPAUX on Tegra, the SOR power partition must be enabled.
Now that Tegra supports the generic PM domain framework we manage the
SOR power partition via this framework for DPAUX. However, the sequence
for gating/ungating the SOR power partition requires that the DPAUX
reset is asserted/de-asserted at the time the SOR power partition is
gated/ungated, respectively. Now that the reset control core assumes
that resets are exclusive, the Tegra generic PM domain code and the
DPAUX driver cannot request the same reset unless we mark the resets as
shared. Sharing resets we will not work in this case because we cannot
guarantee that the reset is asserted/de-asserted at the appropriate
time. Therefore, given that the Tegra generic PM domain code will handle
the DPAUX reset, do not request the reset in the DPAUX driver if the
DPAUX device has a PM domain associated.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/gpu/drm/tegra/dpaux.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index d696a7e45935..289bb064ca1e 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -334,11 +334,14 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
 		return -ENXIO;
 	}
 
-	dpaux->rst = devm_reset_control_get(&pdev->dev, "dpaux");
-	if (IS_ERR(dpaux->rst)) {
-		dev_err(&pdev->dev, "failed to get reset control: %ld\n",
-			PTR_ERR(dpaux->rst));
-		return PTR_ERR(dpaux->rst);
+	if (!pdev->dev.pm_domain) {
+		dpaux->rst = devm_reset_control_get(&pdev->dev, "dpaux");
+		if (IS_ERR(dpaux->rst)) {
+			dev_err(&pdev->dev,
+				"failed to get reset control: %ld\n",
+				PTR_ERR(dpaux->rst));
+			return PTR_ERR(dpaux->rst);
+		}
 	}
 
 	if (of_device_is_compatible(pdev->dev.of_node,
@@ -374,7 +377,8 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
 		goto disable_sor_clk;
 	}
 
-	reset_control_deassert(dpaux->rst);
+	if (dpaux->rst)
+		reset_control_deassert(dpaux->rst);
 
 	dpaux->clk_parent = devm_clk_get(&pdev->dev, "parent");
 	if (IS_ERR(dpaux->clk_parent)) {
@@ -452,7 +456,8 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
 disable_parent_clk:
 	clk_disable_unprepare(dpaux->clk_parent);
 assert_reset:
-	reset_control_assert(dpaux->rst);
+	if (dpaux->rst)
+		reset_control_assert(dpaux->rst);
 	clk_disable_unprepare(dpaux->clk);
 disable_sor_clk:
 	if (dpaux->clk_sor)
@@ -477,7 +482,8 @@ static int tegra_dpaux_remove(struct platform_device *pdev)
 	cancel_work_sync(&dpaux->work);
 
 	clk_disable_unprepare(dpaux->clk_parent);
-	reset_control_assert(dpaux->rst);
+	if (dpaux->rst)
+		reset_control_assert(dpaux->rst);
 	clk_disable_unprepare(dpaux->clk);
 	if (dpaux->clk_sor)
 		clk_disable_unprepare(dpaux->clk_sor);
-- 
2.1.4

[RFC PATCH 07/13] dt-bindings: i2c: Add support for 'i2c-bus' subnode

[Jon Hunter]

The I2C driver core for boards using device-tree assumes any subnode of
an I2C adapter in the device-tree blob as being a I2C slave device.
Although this makes complete sense, some I2C adapters may have subnodes
which are not I2C slaves but subnodes presenting other features. For
example some Tegra devices have an I2C interface which may share its
pins with other devices and to share these pins subnodes for
representing these pins so they have be shared via the pinctrl framework
are needed.

To allow I2C adapters to have non-I2C specific subnodes in device-tree
that are not parsed by the I2C driver core by adding support for a
'i2c-bus' subnode where I2C slaves can be placed. If the 'i2c-bus'
subnode is present then all I2C slaves must be placed under this subnode.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 Documentation/devicetree/bindings/i2c/i2c.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index f31b2ad1552b..ed56b08c7e6e 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -32,6 +32,14 @@ wants to support one of the below features, it should adapt the bindings below.
 - clock-frequency
 	frequency of bus clock in Hz.
 
+- i2c-bus
+	For I2C adapters that have child nodes that are a mixture of both I2C
+	devices and non-I2C devices (such as a pin controller), the 'i2c-bus'
+	subnode can be used for populating I2C devices to prevent the I2C core
+	from attempting to add any non-i2c nodes as I2C devices. If 'i2c-bus'
+	subnode is present then all I2C slaves must be added under this
+	subnode.
+
 - i2c-scl-falling-time-ns
 	Number of nanoseconds the SCL signal takes to fall; t(f) in the I2C
 	specification.
-- 
2.1.4

Re: [RFC PATCH 07/13] dt-bindings: i2c: Add support for 'i2c-bus' subnode

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1933 bytes --]

On Fri, Jun 17, 2016 at 01:03:41PM +0100, Jon Hunter wrote:
> The I2C driver core for boards using device-tree assumes any subnode of
> an I2C adapter in the device-tree blob as being a I2C slave device.
> Although this makes complete sense, some I2C adapters may have subnodes
> which are not I2C slaves but subnodes presenting other features. For
> example some Tegra devices have an I2C interface which may share its
> pins with other devices and to share these pins subnodes for
> representing these pins so they have be shared via the pinctrl framework
> are needed.
> 
> To allow I2C adapters to have non-I2C specific subnodes in device-tree
> that are not parsed by the I2C driver core by adding support for a
> 'i2c-bus' subnode where I2C slaves can be placed. If the 'i2c-bus'
> subnode is present then all I2C slaves must be placed under this subnode.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  Documentation/devicetree/bindings/i2c/i2c.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)

I think this makes a lot of sense, so:

Acked-by: Thierry Reding <treding@nvidia.com>

One minor inconsistency below...

> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
> index f31b2ad1552b..ed56b08c7e6e 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> @@ -32,6 +32,14 @@ wants to support one of the below features, it should adapt the bindings below.
>  - clock-frequency
>  	frequency of bus clock in Hz.
>  
> +- i2c-bus
> +	For I2C adapters that have child nodes that are a mixture of both I2C
> +	devices and non-I2C devices (such as a pin controller), the 'i2c-bus'
> +	subnode can be used for populating I2C devices to prevent the I2C core
> +	from attempting to add any non-i2c nodes as I2C devices. If 'i2c-bus'

"non-I2C"

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 07/13] dt-bindings: i2c: Add support for 'i2c-bus' subnode

[Mark Rutland]

On Fri, Jun 17, 2016 at 01:03:41PM +0100, Jon Hunter wrote:
> The I2C driver core for boards using device-tree assumes any subnode of
> an I2C adapter in the device-tree blob as being a I2C slave device.
> Although this makes complete sense, some I2C adapters may have subnodes
> which are not I2C slaves but subnodes presenting other features. For
> example some Tegra devices have an I2C interface which may share its
> pins with other devices and to share these pins subnodes for
> representing these pins so they have be shared via the pinctrl framework
> are needed.
> 
> To allow I2C adapters to have non-I2C specific subnodes in device-tree
> that are not parsed by the I2C driver core by adding support for a
> 'i2c-bus' subnode where I2C slaves can be placed. If the 'i2c-bus'
> subnode is present then all I2C slaves must be placed under this subnode.
> 
> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/i2c/i2c.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
> index f31b2ad1552b..ed56b08c7e6e 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> @@ -32,6 +32,14 @@ wants to support one of the below features, it should adapt the bindings below.
>  - clock-frequency
>  	frequency of bus clock in Hz.
>  
> +- i2c-bus
> +	For I2C adapters that have child nodes that are a mixture of both I2C
> +	devices and non-I2C devices (such as a pin controller), the 'i2c-bus'
> +	subnode can be used for populating I2C devices to prevent the I2C core
> +	from attempting to add any non-i2c nodes as I2C devices. If 'i2c-bus'
> +	subnode is present then all I2C slaves must be added under this
> +	subnode.

The general idea seems sound.

It would be good if we could remove the mention of the I2C core,
something like:

  - i2c-bus
	For I2C adapters that have child nodes that are a mixture of both I2C
	devices and non-I2C devices (such as a pin controller), the 'i2c-bus'
	subnode can be used for populating I2C devices. If an 'i2c-bus'
	subnode is present, only subnodes of this will be considered as
	I2C slaves.

How are #address-cells and #size-cells handled in this case? I assume
that they should live under the i2c-bus subnode, which should be called
out.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 07/13] dt-bindings: i2c: Add support for 'i2c-bus' subnode

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2728 bytes --]

On Fri, Jun 17, 2016 at 05:30:54PM +0100, Mark Rutland wrote:
> On Fri, Jun 17, 2016 at 01:03:41PM +0100, Jon Hunter wrote:
> > The I2C driver core for boards using device-tree assumes any subnode of
> > an I2C adapter in the device-tree blob as being a I2C slave device.
> > Although this makes complete sense, some I2C adapters may have subnodes
> > which are not I2C slaves but subnodes presenting other features. For
> > example some Tegra devices have an I2C interface which may share its
> > pins with other devices and to share these pins subnodes for
> > representing these pins so they have be shared via the pinctrl framework
> > are needed.
> > 
> > To allow I2C adapters to have non-I2C specific subnodes in device-tree
> > that are not parsed by the I2C driver core by adding support for a
> > 'i2c-bus' subnode where I2C slaves can be placed. If the 'i2c-bus'
> > subnode is present then all I2C slaves must be placed under this subnode.
> > 
> > Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> > ---
> >  Documentation/devicetree/bindings/i2c/i2c.txt | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
> > index f31b2ad1552b..ed56b08c7e6e 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> > @@ -32,6 +32,14 @@ wants to support one of the below features, it should adapt the bindings below.
> >  - clock-frequency
> >  	frequency of bus clock in Hz.
> >  
> > +- i2c-bus
> > +	For I2C adapters that have child nodes that are a mixture of both I2C
> > +	devices and non-I2C devices (such as a pin controller), the 'i2c-bus'
> > +	subnode can be used for populating I2C devices to prevent the I2C core
> > +	from attempting to add any non-i2c nodes as I2C devices. If 'i2c-bus'
> > +	subnode is present then all I2C slaves must be added under this
> > +	subnode.
> 
> The general idea seems sound.
> 
> It would be good if we could remove the mention of the I2C core,
> something like:
> 
>   - i2c-bus
> 	For I2C adapters that have child nodes that are a mixture of both I2C
> 	devices and non-I2C devices (such as a pin controller), the 'i2c-bus'
> 	subnode can be used for populating I2C devices. If an 'i2c-bus'
> 	subnode is present, only subnodes of this will be considered as
> 	I2C slaves.
> 
> How are #address-cells and #size-cells handled in this case? I assume
> that they should live under the i2c-bus subnode, which should be called
> out.

Good catch. Yes, I think the i2c-bus subnode would be the right place
for #address-cells and #size-cells.

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 07/13] dt-bindings: i2c: Add support for 'i2c-bus' subnode

[Jon Hunter]

On 17/06/16 17:45, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Jun 17, 2016 at 05:30:54PM +0100, Mark Rutland wrote:
>> On Fri, Jun 17, 2016 at 01:03:41PM +0100, Jon Hunter wrote:
>>> The I2C driver core for boards using device-tree assumes any subnode of
>>> an I2C adapter in the device-tree blob as being a I2C slave device.
>>> Although this makes complete sense, some I2C adapters may have subnodes
>>> which are not I2C slaves but subnodes presenting other features. For
>>> example some Tegra devices have an I2C interface which may share its
>>> pins with other devices and to share these pins subnodes for
>>> representing these pins so they have be shared via the pinctrl framework
>>> are needed.
>>>
>>> To allow I2C adapters to have non-I2C specific subnodes in device-tree
>>> that are not parsed by the I2C driver core by adding support for a
>>> 'i2c-bus' subnode where I2C slaves can be placed. If the 'i2c-bus'
>>> subnode is present then all I2C slaves must be placed under this subnode.
>>>
>>> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>  Documentation/devicetree/bindings/i2c/i2c.txt | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
>>> index f31b2ad1552b..ed56b08c7e6e 100644
>>> --- a/Documentation/devicetree/bindings/i2c/i2c.txt
>>> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
>>> @@ -32,6 +32,14 @@ wants to support one of the below features, it should adapt the bindings below.
>>>  - clock-frequency
>>>  	frequency of bus clock in Hz.
>>>  
>>> +- i2c-bus
>>> +	For I2C adapters that have child nodes that are a mixture of both I2C
>>> +	devices and non-I2C devices (such as a pin controller), the 'i2c-bus'
>>> +	subnode can be used for populating I2C devices to prevent the I2C core
>>> +	from attempting to add any non-i2c nodes as I2C devices. If 'i2c-bus'
>>> +	subnode is present then all I2C slaves must be added under this
>>> +	subnode.
>>
>> The general idea seems sound.
>>
>> It would be good if we could remove the mention of the I2C core,
>> something like:
>>
>>   - i2c-bus
>> 	For I2C adapters that have child nodes that are a mixture of both I2C
>> 	devices and non-I2C devices (such as a pin controller), the 'i2c-bus'
>> 	subnode can be used for populating I2C devices. If an 'i2c-bus'
>> 	subnode is present, only subnodes of this will be considered as
>> 	I2C slaves.
>>
>> How are #address-cells and #size-cells handled in this case? I assume
>> that they should live under the i2c-bus subnode, which should be called
>> out.
> 
> Good catch. Yes, I think the i2c-bus subnode would be the right place
> for #address-cells and #size-cells.

Yes, indeed. Thanks. Will fix.

Cheers
Jon

-- 
nvpublic

[RFC PATCH 08/13] i2c: core: Add support for 'i2c-bus' subnode

[Jon Hunter]

If the 'i2c-bus' device-tree node is present for an I2C adapter then
parse this subnode for I2C slaves.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/i2c/i2c-core.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 952d2f0c02c5..f552d97bad32 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1452,7 +1452,7 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
 
 static void of_i2c_register_devices(struct i2c_adapter *adap)
 {
-	struct device_node *node;
+	struct device_node *bus, *node;
 
 	/* Only register child devices if the adapter has a node pointer set */
 	if (!adap->dev.of_node)
@@ -1460,11 +1460,18 @@ static void of_i2c_register_devices(struct i2c_adapter *adap)
 
 	dev_dbg(&adap->dev, "of_i2c: walking child nodes\n");
 
-	for_each_available_child_of_node(adap->dev.of_node, node) {
+	bus = of_get_child_by_name(adap->dev.of_node, "i2c-bus");
+	if (!bus)
+		bus = adap->dev.of_node;
+
+	for_each_available_child_of_node(bus, node) {
 		if (of_node_test_and_set_flag(node, OF_POPULATED))
 			continue;
 		of_i2c_register_device(adap, node);
 	}
+
+	if (bus != adap->dev.of_node)
+		of_node_put(bus);
 }
 
 static int of_dev_node_match(struct device *dev, void *data)
-- 
2.1.4

Re: [RFC PATCH 08/13] i2c: core: Add support for 'i2c-bus' subnode

[Thierry Reding]

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

On Fri, Jun 17, 2016 at 01:03:42PM +0100, Jon Hunter wrote:
> If the 'i2c-bus' device-tree node is present for an I2C adapter then
> parse this subnode for I2C slaves.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  drivers/i2c/i2c-core.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 952d2f0c02c5..f552d97bad32 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -1452,7 +1452,7 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
>  
>  static void of_i2c_register_devices(struct i2c_adapter *adap)
>  {
> -	struct device_node *node;
> +	struct device_node *bus, *node;
>  
>  	/* Only register child devices if the adapter has a node pointer set */
>  	if (!adap->dev.of_node)
> @@ -1460,11 +1460,18 @@ static void of_i2c_register_devices(struct i2c_adapter *adap)
>  
>  	dev_dbg(&adap->dev, "of_i2c: walking child nodes\n");
>  
> -	for_each_available_child_of_node(adap->dev.of_node, node) {
> +	bus = of_get_child_by_name(adap->dev.of_node, "i2c-bus");
> +	if (!bus)
> +		bus = adap->dev.of_node;

Maybe
		bus = of_node_get(adap->dev.of_node);

here...

> +
> +	for_each_available_child_of_node(bus, node) {
>  		if (of_node_test_and_set_flag(node, OF_POPULATED))
>  			continue;
>  		of_i2c_register_device(adap, node);
>  	}
> +
> +	if (bus != adap->dev.of_node)
> +		of_node_put(bus);

... and drop the extra check here?

Thierry

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

Re: [RFC PATCH 08/13] i2c: core: Add support for 'i2c-bus' subnode

[Jon Hunter]

On 17/06/16 17:24, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Jun 17, 2016 at 01:03:42PM +0100, Jon Hunter wrote:
>> If the 'i2c-bus' device-tree node is present for an I2C adapter then
>> parse this subnode for I2C slaves.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  drivers/i2c/i2c-core.c | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
>> index 952d2f0c02c5..f552d97bad32 100644
>> --- a/drivers/i2c/i2c-core.c
>> +++ b/drivers/i2c/i2c-core.c
>> @@ -1452,7 +1452,7 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
>>  
>>  static void of_i2c_register_devices(struct i2c_adapter *adap)
>>  {
>> -	struct device_node *node;
>> +	struct device_node *bus, *node;
>>  
>>  	/* Only register child devices if the adapter has a node pointer set */
>>  	if (!adap->dev.of_node)
>> @@ -1460,11 +1460,18 @@ static void of_i2c_register_devices(struct i2c_adapter *adap)
>>  
>>  	dev_dbg(&adap->dev, "of_i2c: walking child nodes\n");
>>  
>> -	for_each_available_child_of_node(adap->dev.of_node, node) {
>> +	bus = of_get_child_by_name(adap->dev.of_node, "i2c-bus");
>> +	if (!bus)
>> +		bus = adap->dev.of_node;
> 
> Maybe
> 		bus = of_node_get(adap->dev.of_node);
> 
> here...
> 
>> +
>> +	for_each_available_child_of_node(bus, node) {
>>  		if (of_node_test_and_set_flag(node, OF_POPULATED))
>>  			continue;
>>  		of_i2c_register_device(adap, node);
>>  	}
>> +
>> +	if (bus != adap->dev.of_node)
>> +		of_node_put(bus);
> 
> ... and drop the extra check here?

Yes makes sense.

Jon

-- 
nvpublic
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC PATCH 09/13] dt-bindings: drm/tegra: Add DPAUX pinctrl documentation

[Jon Hunter]

On Tegra124, Tegra132 and Tegra210 devices the pads used by the Display
Port Auxiliary (DPAUX) channel are multiplexed such that they can also
be used by one of the internal i2c controllers. Note that this is
different from i2c-over-AUX supported by the DPAUX controller. The
register that configures these pads is part of the DPAUX controllers
register set and so a pinctrl driver is being added for the DPAUX device
to share these pads. Add the device-tree binding documentation for the
DPAUX pad controller.

Please note that although the "off" function for the DPAUX pads is not
technically a pin-mux setting but more of a pin-conf setting it is
simpler to expose these as a function so that the user can simply select
either "aux", "i2c" or "off" as the current function/mode.

Update the main DPAUX binding documentation to reference the DPAUX pad
controller binding document and add the 'i2c-bus' subnode. The 'i2c-bus'
subnode is used for populating I2C slaves for the DPAUX device so that
the I2C driver core does not attempt to add the DPAUX pad controller
nodes as I2C slaves.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 .../display/tegra/nvidia,tegra20-host1x.txt        |  4 ++
 .../pinctrl/nvidia,tegra124-dpaux-padctl.txt       | 60 ++++++++++++++++++++++
 2 files changed, 64 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-dpaux-padctl.txt

diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
index 361a472eac4b..6759554b7b8f 100644
--- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
+++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
@@ -242,6 +242,10 @@ of the following host1x client modules:
   - reset-names: Must include the following entries:
     - dpaux
   - vdd-supply: phandle of a supply that powers the DisplayPort link
+  - i2c-bus: Subnode where I2C slave devices should be listed.
+
+  See ../pinctrl/nvidia,tegra124-dpaux-padctl.txt for information
+  regarding the DPAUX pad controller bindings.
 
 Example:
 
diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-dpaux-padctl.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-dpaux-padctl.txt
new file mode 100644
index 000000000000..3be0ced01680
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-dpaux-padctl.txt
@@ -0,0 +1,60 @@
+Device tree binding for NVIDIA Tegra DPAUX pad controller
+========================================================
+
+The Tegra Display Port Auxiliary (DPAUX) pad controller manages two pins
+which can be assigned to either the DPAUX channel or to an I2C
+controller.
+
+This document defines the device-specific binding for the DPAUX pad
+controller. Refer to pinctrl-bindings.txt in this directory for generic
+information about pin controller device tree bindings. Please refer to
+the binding document ../display/tegra/nvidia,tegra20-host1x.txt for more
+details on the DPAUX binding.
+
+Pin muxing:
+-----------
+
+Child nodes contain the pinmux configurations following the conventions
+from the pinctrl-bindings.txt document.
+
+Since only three configurations are possible, only three child nodes are
+needed to describe the pin mux'ing options for the DPAUX pads.
+Furthermore, given that the pad functions are only applicable to a
+single set of pads, the child nodes do not need to describe the pads the
+functions are being applied to.
+
+Required properties:
+- groups: Must be "dpaux-io"
+- function: Must be either "aux", "i2c" or "off".
+
+Example:
+--------
+
+	dpaux@545c0000 {
+		...
+
+		state_dpaux_aux: pinmux_aux {
+			groups = "dpaux-io";
+			function = "aux";
+		};
+
+		state_dpaux_i2c: pinmux_i2c {
+			groups = "dpaux-io";
+			function = "i2c";
+		};
+
+		state_dpaux_off: pinmux_off {
+			groups = "dpaux-io";
+			function = "off";
+		};
+	};
+
+	...
+
+	i2c@7000d100 {
+		...
+		pinctrl-0 = <&state_dpaux_i2c>;
+		pinctrl-1 = <&state_dpaux_off>;
+		pinctrl-names = "default", "idle";
+		status = "disabled";
+	};
-- 
2.1.4

Re: [RFC PATCH 09/13] dt-bindings: drm/tegra: Add DPAUX pinctrl documentation

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 4404 bytes --]

On Fri, Jun 17, 2016 at 01:03:43PM +0100, Jon Hunter wrote:
> On Tegra124, Tegra132 and Tegra210 devices the pads used by the Display
> Port Auxiliary (DPAUX) channel are multiplexed such that they can also
> be used by one of the internal i2c controllers. Note that this is
> different from i2c-over-AUX supported by the DPAUX controller. The
> register that configures these pads is part of the DPAUX controllers
> register set and so a pinctrl driver is being added for the DPAUX device
> to share these pads. Add the device-tree binding documentation for the
> DPAUX pad controller.

There are a couple more "i2c" vs. "I2C" in the above. Please use the
latter consistently.

Also the subject line should be something different. drm is a linuxism
and hence shouldn't be used anywhere near DT bindings.

> Please note that although the "off" function for the DPAUX pads is not
> technically a pin-mux setting but more of a pin-conf setting it is
> simpler to expose these as a function so that the user can simply select
> either "aux", "i2c" or "off" as the current function/mode.
> 
> Update the main DPAUX binding documentation to reference the DPAUX pad
> controller binding document and add the 'i2c-bus' subnode. The 'i2c-bus'
> subnode is used for populating I2C slaves for the DPAUX device so that
> the I2C driver core does not attempt to add the DPAUX pad controller
> nodes as I2C slaves.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  .../display/tegra/nvidia,tegra20-host1x.txt        |  4 ++
>  .../pinctrl/nvidia,tegra124-dpaux-padctl.txt       | 60 ++++++++++++++++++++++
>  2 files changed, 64 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-dpaux-padctl.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> index 361a472eac4b..6759554b7b8f 100644
> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> @@ -242,6 +242,10 @@ of the following host1x client modules:
>    - reset-names: Must include the following entries:
>      - dpaux
>    - vdd-supply: phandle of a supply that powers the DisplayPort link
> +  - i2c-bus: Subnode where I2C slave devices should be listed.

Should we perhaps say at this point that the subnode should always be
added, oven if empty? Otherwise we'll still run into a conflict with the
pinmux nodes.

> +
> +  See ../pinctrl/nvidia,tegra124-dpaux-padctl.txt for information
> +  regarding the DPAUX pad controller bindings.
>  
>  Example:
>  
> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-dpaux-padctl.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-dpaux-padctl.txt
> new file mode 100644
> index 000000000000..3be0ced01680
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-dpaux-padctl.txt
> @@ -0,0 +1,60 @@
> +Device tree binding for NVIDIA Tegra DPAUX pad controller
> +========================================================
> +
> +The Tegra Display Port Auxiliary (DPAUX) pad controller manages two pins
> +which can be assigned to either the DPAUX channel or to an I2C
> +controller.
> +
> +This document defines the device-specific binding for the DPAUX pad
> +controller. Refer to pinctrl-bindings.txt in this directory for generic
> +information about pin controller device tree bindings. Please refer to
> +the binding document ../display/tegra/nvidia,tegra20-host1x.txt for more
> +details on the DPAUX binding.
> +
> +Pin muxing:
> +-----------
> +
> +Child nodes contain the pinmux configurations following the conventions
> +from the pinctrl-bindings.txt document.
> +
> +Since only three configurations are possible, only three child nodes are
> +needed to describe the pin mux'ing options for the DPAUX pads.
> +Furthermore, given that the pad functions are only applicable to a
> +single set of pads, the child nodes do not need to describe the pads the
> +functions are being applied to.
> +
> +Required properties:
> +- groups: Must be "dpaux-io"

Above you say that we don't need to describe the pads, but then the
groups property does describe the pads. Why?

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 09/13] dt-bindings: drm/tegra: Add DPAUX pinctrl documentation

[Jon Hunter]

On 17/06/16 17:31, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Jun 17, 2016 at 01:03:43PM +0100, Jon Hunter wrote:
>> On Tegra124, Tegra132 and Tegra210 devices the pads used by the Display
>> Port Auxiliary (DPAUX) channel are multiplexed such that they can also
>> be used by one of the internal i2c controllers. Note that this is
>> different from i2c-over-AUX supported by the DPAUX controller. The
>> register that configures these pads is part of the DPAUX controllers
>> register set and so a pinctrl driver is being added for the DPAUX device
>> to share these pads. Add the device-tree binding documentation for the
>> DPAUX pad controller.
> 
> There are a couple more "i2c" vs. "I2C" in the above. Please use the
> latter consistently.
> 
> Also the subject line should be something different. drm is a linuxism
> and hence shouldn't be used anywhere near DT bindings.

OK.

>> Please note that although the "off" function for the DPAUX pads is not
>> technically a pin-mux setting but more of a pin-conf setting it is
>> simpler to expose these as a function so that the user can simply select
>> either "aux", "i2c" or "off" as the current function/mode.
>>
>> Update the main DPAUX binding documentation to reference the DPAUX pad
>> controller binding document and add the 'i2c-bus' subnode. The 'i2c-bus'
>> subnode is used for populating I2C slaves for the DPAUX device so that
>> the I2C driver core does not attempt to add the DPAUX pad controller
>> nodes as I2C slaves.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  .../display/tegra/nvidia,tegra20-host1x.txt        |  4 ++
>>  .../pinctrl/nvidia,tegra124-dpaux-padctl.txt       | 60 ++++++++++++++++++++++
>>  2 files changed, 64 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-dpaux-padctl.txt
>>
>> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>> index 361a472eac4b..6759554b7b8f 100644
>> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>> @@ -242,6 +242,10 @@ of the following host1x client modules:
>>    - reset-names: Must include the following entries:
>>      - dpaux
>>    - vdd-supply: phandle of a supply that powers the DisplayPort link
>> +  - i2c-bus: Subnode where I2C slave devices should be listed.
> 
> Should we perhaps say at this point that the subnode should always be
> added, oven if empty? Otherwise we'll still run into a conflict with the
> pinmux nodes.

Yes, indeed.

>> +
>> +  See ../pinctrl/nvidia,tegra124-dpaux-padctl.txt for information
>> +  regarding the DPAUX pad controller bindings.
>>  
>>  Example:
>>  
>> diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-dpaux-padctl.txt b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-dpaux-padctl.txt
>> new file mode 100644
>> index 000000000000..3be0ced01680
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-dpaux-padctl.txt
>> @@ -0,0 +1,60 @@
>> +Device tree binding for NVIDIA Tegra DPAUX pad controller
>> +========================================================
>> +
>> +The Tegra Display Port Auxiliary (DPAUX) pad controller manages two pins
>> +which can be assigned to either the DPAUX channel or to an I2C
>> +controller.
>> +
>> +This document defines the device-specific binding for the DPAUX pad
>> +controller. Refer to pinctrl-bindings.txt in this directory for generic
>> +information about pin controller device tree bindings. Please refer to
>> +the binding document ../display/tegra/nvidia,tegra20-host1x.txt for more
>> +details on the DPAUX binding.
>> +
>> +Pin muxing:
>> +-----------
>> +
>> +Child nodes contain the pinmux configurations following the conventions
>> +from the pinctrl-bindings.txt document.
>> +
>> +Since only three configurations are possible, only three child nodes are
>> +needed to describe the pin mux'ing options for the DPAUX pads.
>> +Furthermore, given that the pad functions are only applicable to a
>> +single set of pads, the child nodes do not need to describe the pads the
>> +functions are being applied to.
>> +
>> +Required properties:
>> +- groups: Must be "dpaux-io"
> 
> Above you say that we don't need to describe the pads, but then the
> groups property does describe the pads. Why?

I will re-word that. The individual pads/pins themselves are not needed
as we can represent this as a group because we only can mux the pins on
a group basis. However, although there is only one group we need to has
this in the binding, otherwise the pads will not be allocated by the
pinctrl core for the client and this would allow another client to grab
the same pins. We had a similar issue with xusb before [0]. We debated
whether this should be fixed in the core, but ended up fixing in the
xusb pinctrl driver.

Cheers
Jon

[0] 8480c2e7b048 ("pinctrl: tegra-xusb: Fix allocation of pins")

-- 
nvpublic
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC PATCH 10/13] drm/tegra: Add pinctrl support for DPAUX

[Jon Hunter]

The DPAUX pins are shared with an internal I2C controller. To allow
these pins to be muxed to the I2C controller, register a pinctrl device
for the DPAUX device. Make Tegra DRM support dependent on PINCTRL to
avoid any compilation issues.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/gpu/drm/tegra/Kconfig |   1 +
 drivers/gpu/drm/tegra/dpaux.c | 117 ++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 115 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig
index 63ebb154b9b5..d34937a96f94 100644
--- a/drivers/gpu/drm/tegra/Kconfig
+++ b/drivers/gpu/drm/tegra/Kconfig
@@ -4,6 +4,7 @@ config DRM_TEGRA
 	depends on COMMON_CLK
 	depends on DRM
 	depends on RESET_CONTROLLER
+	depends on PINCTRL
 	select DRM_KMS_HELPER
 	select DRM_MIPI_DSI
 	select DRM_PANEL
diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index 289bb064ca1e..391273652fc8 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -12,6 +12,9 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/of_gpio.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
 #include <linux/platform_device.h>
 #include <linux/reset.h>
 #include <linux/regulator/consumer.h>
@@ -45,6 +48,9 @@ struct tegra_dpaux {
 	struct completion complete;
 	struct work_struct work;
 	struct list_head list;
+
+	struct pinctrl_dev *pinctrl;
+	struct pinctrl_desc desc;
 };
 
 static inline struct tegra_dpaux *to_dpaux(struct drm_dp_aux *aux)
@@ -268,6 +274,80 @@ static irqreturn_t tegra_dpaux_irq(int irq, void *data)
 	return ret;
 }
 
+static const struct pinctrl_pin_desc tegra_dpaux_pins[] = {
+	PINCTRL_PIN(0, "DP_AUX_CHx_P"),
+	PINCTRL_PIN(1, "DP_AUX_CHx_N"),
+};
+
+static const unsigned tegra_dpaux_pin_numbers[] = { 0, 1 };
+
+static const char * const tegra_dpaux_groups[] = {
+	"dpaux-io",
+};
+
+static const char * const tegra_dpaux_functions[] = {
+	"aux",
+	"i2c",
+	"off",
+};
+
+static int tegra_dpaux_get_groups_count(struct pinctrl_dev *pinctrl)
+{
+	return ARRAY_SIZE(tegra_dpaux_groups);
+}
+
+static const char *tegra_dpaux_get_group_name(struct pinctrl_dev *pinctrl,
+					      unsigned int group)
+{
+	return tegra_dpaux_groups[group];
+}
+
+static int tegra_dpaux_get_group_pins(struct pinctrl_dev *pinctrl,
+				      unsigned group, const unsigned **pins,
+				      unsigned *num_pins)
+{
+	*pins = tegra_dpaux_pin_numbers;
+	*num_pins = ARRAY_SIZE(tegra_dpaux_pin_numbers);
+
+	return 0;
+}
+
+enum tegra_dpaux_functions {
+	DPAUX_PADCTL_FUNC_AUX,
+	DPAUX_PADCTL_FUNC_I2C,
+	DPAUX_PADCTL_FUNC_OFF,
+};
+
+static const struct pinctrl_ops tegra_dpaux_pinctrl_ops = {
+	.get_groups_count = tegra_dpaux_get_groups_count,
+	.get_group_name = tegra_dpaux_get_group_name,
+	.get_group_pins = tegra_dpaux_get_group_pins,
+	.dt_node_to_map = pinconf_generic_dt_node_to_map_group,
+	.dt_free_map = pinconf_generic_dt_free_map,
+};
+
+static int tegra_dpaux_get_functions_count(struct pinctrl_dev *pinctrl)
+{
+	return ARRAY_SIZE(tegra_dpaux_functions);
+}
+
+static const char *tegra_dpaux_get_function_name(struct pinctrl_dev *pinctrl,
+						 unsigned int function)
+{
+	return tegra_dpaux_functions[function];
+}
+
+static int tegra_dpaux_get_function_groups(struct pinctrl_dev *pinctrl,
+					   unsigned int function,
+					   const char * const **groups,
+					   unsigned * const num_groups)
+{
+	*num_groups = ARRAY_SIZE(tegra_dpaux_groups);
+	*groups = tegra_dpaux_groups;
+
+	return 0;
+}
+
 static void tegra_dpaux_powerdown(struct tegra_dpaux *dpaux, bool enable)
 {
 	u32 value = tegra_dpaux_readl(dpaux, DPAUX_HYBRID_SPARE);
@@ -285,18 +365,21 @@ static int tegra_dpaux_config(struct tegra_dpaux *dpaux, int function)
 	u32 value;
 
 	switch (function) {
-	case DPAUX_HYBRID_PADCTL_MODE_AUX:
+	case DPAUX_PADCTL_FUNC_AUX:
 		value = DPAUX_HYBRID_PADCTL_AUX_CMH(2) |
 			DPAUX_HYBRID_PADCTL_AUX_DRVZ(4) |
 			DPAUX_HYBRID_PADCTL_AUX_DRVI(0x18) |
 			DPAUX_HYBRID_PADCTL_AUX_INPUT_RCV |
 			DPAUX_HYBRID_PADCTL_MODE_AUX;
 		break;
-	case DPAUX_HYBRID_PADCTL_MODE_I2C:
+	case DPAUX_PADCTL_FUNC_I2C:
 		value = DPAUX_HYBRID_PADCTL_I2C_SDA_INPUT_RCV |
 			DPAUX_HYBRID_PADCTL_I2C_SCL_INPUT_RCV |
 			DPAUX_HYBRID_PADCTL_MODE_I2C;
 		break;
+	case DPAUX_PADCTL_FUNC_OFF:
+		tegra_dpaux_powerdown(dpaux, true);
+		return 0;
 	default:
 		return -ENOTSUPP;
 	}
@@ -307,6 +390,21 @@ static int tegra_dpaux_config(struct tegra_dpaux *dpaux, int function)
 	return 0;
 }
 
+static int tegra_dpaux_set_mux(struct pinctrl_dev *pinctrl,
+			       unsigned int function, unsigned int group)
+{
+	struct tegra_dpaux *dpaux = pinctrl_dev_get_drvdata(pinctrl);
+
+	return tegra_dpaux_config(dpaux, function);
+}
+
+static const struct pinmux_ops tegra_dpaux_pinmux_ops = {
+	.get_functions_count = tegra_dpaux_get_functions_count,
+	.get_function_name = tegra_dpaux_get_function_name,
+	.get_function_groups = tegra_dpaux_get_function_groups,
+	.set_mux = tegra_dpaux_set_mux,
+};
+
 static int tegra_dpaux_probe(struct platform_device *pdev)
 {
 	struct tegra_dpaux *dpaux;
@@ -439,6 +537,19 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
 	if (err < 0)
 		return err;
 
+	dpaux->desc.name = dev_name(&pdev->dev);
+	dpaux->desc.pins = tegra_dpaux_pins;
+	dpaux->desc.npins = ARRAY_SIZE(tegra_dpaux_pins);
+	dpaux->desc.pctlops = &tegra_dpaux_pinctrl_ops;
+	dpaux->desc.pmxops = &tegra_dpaux_pinmux_ops;
+	dpaux->desc.owner = THIS_MODULE;
+
+	dpaux->pinctrl = pinctrl_register(&dpaux->desc, &pdev->dev, dpaux);
+	if (!dpaux->pinctrl) {
+		dev_err(&pdev->dev, "failed to register pincontrol\n");
+		return -ENODEV;
+	}
+
 	/* enable and clear all interrupts */
 	value = DPAUX_INTR_AUX_DONE | DPAUX_INTR_IRQ_EVENT |
 		DPAUX_INTR_UNPLUG_EVENT | DPAUX_INTR_PLUG_EVENT;
@@ -600,7 +711,7 @@ int drm_dp_aux_enable(struct drm_dp_aux *aux)
 {
 	struct tegra_dpaux *dpaux = to_dpaux(aux);
 
-	return tegra_dpaux_config(dpaux, DPAUX_HYBRID_PADCTL_MODE_AUX);
+	return tegra_dpaux_config(dpaux, DPAUX_PADCTL_FUNC_AUX);
 }
 
 int drm_dp_aux_disable(struct drm_dp_aux *aux)
-- 
2.1.4

Re: [RFC PATCH 10/13] drm/tegra: Add pinctrl support for DPAUX

[Jon Hunter]

On 17/06/16 13:03, Jon Hunter wrote:
> The DPAUX pins are shared with an internal I2C controller. To allow
> these pins to be muxed to the I2C controller, register a pinctrl device
> for the DPAUX device. Make Tegra DRM support dependent on PINCTRL to
> avoid any compilation issues.

I forgot to add a comment here to say that this is based upon work from
Thierry Reding and give credit for that.

Cheers
Jon

-- 
nvpublic

Re: [RFC PATCH 10/13] drm/tegra: Add pinctrl support for DPAUX

[Thierry Reding]

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

On Fri, Jun 17, 2016 at 01:03:44PM +0100, Jon Hunter wrote:
> The DPAUX pins are shared with an internal I2C controller. To allow
> these pins to be muxed to the I2C controller, register a pinctrl device
> for the DPAUX device. Make Tegra DRM support dependent on PINCTRL to
> avoid any compilation issues.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/Kconfig |   1 +
>  drivers/gpu/drm/tegra/dpaux.c | 117 ++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 115 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig
> index 63ebb154b9b5..d34937a96f94 100644
> --- a/drivers/gpu/drm/tegra/Kconfig
> +++ b/drivers/gpu/drm/tegra/Kconfig
> @@ -4,6 +4,7 @@ config DRM_TEGRA
>  	depends on COMMON_CLK
>  	depends on DRM
>  	depends on RESET_CONTROLLER
> +	depends on PINCTRL

Could we instead make the code optional? I don't care much about pulling
in the extra dependency (for Tegra we always enable PINCTRL anyway), but
I worry that somebody may end up searching for DRM_TEGRA and not find it
because PINCTRL happens to be disabled in they .config.

> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
[...]
> @@ -439,6 +537,19 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		return err;
>  
> +	dpaux->desc.name = dev_name(&pdev->dev);
> +	dpaux->desc.pins = tegra_dpaux_pins;
> +	dpaux->desc.npins = ARRAY_SIZE(tegra_dpaux_pins);
> +	dpaux->desc.pctlops = &tegra_dpaux_pinctrl_ops;
> +	dpaux->desc.pmxops = &tegra_dpaux_pinmux_ops;
> +	dpaux->desc.owner = THIS_MODULE;
> +
> +	dpaux->pinctrl = pinctrl_register(&dpaux->desc, &pdev->dev, dpaux);
> +	if (!dpaux->pinctrl) {
> +		dev_err(&pdev->dev, "failed to register pincontrol\n");
> +		return -ENODEV;
> +	}

Did you mean to use the devm_ variant here? Because I don't see a
pinctrl_unregister() in tegra_dpaux_remove().

Thierry

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

Re: [RFC PATCH 10/13] drm/tegra: Add pinctrl support for DPAUX

[Jon Hunter]

On 17/06/16 17:37, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Jun 17, 2016 at 01:03:44PM +0100, Jon Hunter wrote:
>> The DPAUX pins are shared with an internal I2C controller. To allow
>> these pins to be muxed to the I2C controller, register a pinctrl device
>> for the DPAUX device. Make Tegra DRM support dependent on PINCTRL to
>> avoid any compilation issues.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  drivers/gpu/drm/tegra/Kconfig |   1 +
>>  drivers/gpu/drm/tegra/dpaux.c | 117 ++++++++++++++++++++++++++++++++++++++++--
>>  2 files changed, 115 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig
>> index 63ebb154b9b5..d34937a96f94 100644
>> --- a/drivers/gpu/drm/tegra/Kconfig
>> +++ b/drivers/gpu/drm/tegra/Kconfig
>> @@ -4,6 +4,7 @@ config DRM_TEGRA
>>  	depends on COMMON_CLK
>>  	depends on DRM
>>  	depends on RESET_CONTROLLER
>> +	depends on PINCTRL
> 
> Could we instead make the code optional? I don't care much about pulling
> in the extra dependency (for Tegra we always enable PINCTRL anyway), but
> I worry that somebody may end up searching for DRM_TEGRA and not find it
> because PINCTRL happens to be disabled in they .config.

Yes we could if you don't mind the #ifdef in the source file. The
alternative here would be to select PINCTRL and like you said this
should always be the case as Tegra selects in anyway.

>> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
> [...]
>> @@ -439,6 +537,19 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
>>  	if (err < 0)
>>  		return err;
>>  
>> +	dpaux->desc.name = dev_name(&pdev->dev);
>> +	dpaux->desc.pins = tegra_dpaux_pins;
>> +	dpaux->desc.npins = ARRAY_SIZE(tegra_dpaux_pins);
>> +	dpaux->desc.pctlops = &tegra_dpaux_pinctrl_ops;
>> +	dpaux->desc.pmxops = &tegra_dpaux_pinmux_ops;
>> +	dpaux->desc.owner = THIS_MODULE;
>> +
>> +	dpaux->pinctrl = pinctrl_register(&dpaux->desc, &pdev->dev, dpaux);
>> +	if (!dpaux->pinctrl) {
>> +		dev_err(&pdev->dev, "failed to register pincontrol\n");
>> +		return -ENODEV;
>> +	}
> 
> Did you mean to use the devm_ variant here? Because I don't see a
> pinctrl_unregister() in tegra_dpaux_remove().

Yes good catch. Will fix.

Jon

-- 
nvpublic
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC PATCH 11/13] arm64: tegra: Add SOR power-domain node

[Jon Hunter]

Add node for SOR power-domain for Tegra210 and populate the SOR
power-domain phandle for SOR and DPAUX nodes that are dependent
on this power-domain.

Please note that although neither the SOR or DPAUX drivers currently
support runtime power-management, by populating the power-domain node
the SOR power-domain will be turned on before probing SOR or DPAUX
devices and kept on while the devices are bound.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index ebf44f4059f8..94f780b43037 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -34,6 +34,7 @@
 			clock-names = "dpaux", "parent";
 			resets = <&tegra_car 207>;
 			reset-names = "dpaux";
+			power-domains = <&pd_sor>;
 			status = "disabled";
 		};
 
@@ -154,6 +155,7 @@
 			clock-names = "sor", "parent", "dp", "safe";
 			resets = <&tegra_car 182>;
 			reset-names = "sor";
+			power-domains = <&pd_sor>;
 			status = "disabled";
 		};
 
@@ -168,6 +170,7 @@
 			clock-names = "sor", "parent", "dp", "safe";
 			resets = <&tegra_car 183>;
 			reset-names = "sor";
+			power-domains = <&pd_sor>;
 			status = "disabled";
 		};
 
@@ -180,6 +183,7 @@
 			clock-names = "dpaux", "parent";
 			resets = <&tegra_car 181>;
 			reset-names = "dpaux";
+			power-domains = <&pd_sor>;
 			status = "disabled";
 		};
 
@@ -592,6 +596,20 @@
 				resets = <&tegra_car 198>;
 				#power-domain-cells = <0>;
 			};
+
+			pd_sor: sor {
+				clocks = <&tegra_car TEGRA210_CLK_SOR0>,
+					 <&tegra_car TEGRA210_CLK_DSIA>,
+					 <&tegra_car TEGRA210_CLK_DSIB>,
+					 <&tegra_car TEGRA210_CLK_MIPI_CAL>,
+					 <&tegra_car TEGRA210_CLK_DPAUX>;
+				resets = <&tegra_car TEGRA210_CLK_SOR0>,
+					 <&tegra_car TEGRA210_CLK_DSIA>,
+					 <&tegra_car TEGRA210_CLK_DSIB>,
+					 <&tegra_car TEGRA210_CLK_DPAUX>,
+					 <&tegra_car TEGRA210_CLK_MIPI_CAL>;
+				#power-domain-cells = <0>;
+			};
 		};
 	};
 
-- 
2.1.4

Re: [RFC PATCH 11/13] arm64: tegra: Add SOR power-domain node

[Thierry Reding]

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

On Fri, Jun 17, 2016 at 01:03:45PM +0100, Jon Hunter wrote:
> Add node for SOR power-domain for Tegra210 and populate the SOR
> power-domain phandle for SOR and DPAUX nodes that are dependent
> on this power-domain.
> 
> Please note that although neither the SOR or DPAUX drivers currently
> support runtime power-management, by populating the power-domain node
> the SOR power-domain will be turned on before probing SOR or DPAUX
> devices and kept on while the devices are bound.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  arch/arm64/boot/dts/nvidia/tegra210.dtsi | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)

I've got patches queued in my drm-tegra tree to add support for runtime
PM for the SOR driver. Will that in some way clash with this work if
merged in parallel?

Thierry

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

Re: [RFC PATCH 11/13] arm64: tegra: Add SOR power-domain node

[Jon Hunter]

On 17/06/16 17:42, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Jun 17, 2016 at 01:03:45PM +0100, Jon Hunter wrote:
>> Add node for SOR power-domain for Tegra210 and populate the SOR
>> power-domain phandle for SOR and DPAUX nodes that are dependent
>> on this power-domain.
>>
>> Please note that although neither the SOR or DPAUX drivers currently
>> support runtime power-management, by populating the power-domain node
>> the SOR power-domain will be turned on before probing SOR or DPAUX
>> devices and kept on while the devices are bound.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  arch/arm64/boot/dts/nvidia/tegra210.dtsi | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
> 
> I've got patches queued in my drm-tegra tree to add support for runtime
> PM for the SOR driver. Will that in some way clash with this work if
> merged in parallel?

Yes I had seen those. I was wondering if I should include them here but
opted not too (as it was becoming a massive series). It should be fine
to enable RPM later, you just need to be aware that the partition could
turn off on a RPM put so you just need to ensure that any register
context is saved and restored.

By the way, one item I am not certain about is the relationship between
the HDMI device and the SOR device. If the HDMI is in the SOR partition
it should also have the SOR power-domain populated, but it just uses the
SOR, then the SOR driver should handle this.

Cheers
Jon

-- 
nvpublic
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 11/13] arm64: tegra: Add SOR power-domain node

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2166 bytes --]

On Mon, Jun 20, 2016 at 10:18:42AM +0100, Jon Hunter wrote:
> 
> On 17/06/16 17:42, Thierry Reding wrote:
> > * PGP Signed by an unknown key
> > 
> > On Fri, Jun 17, 2016 at 01:03:45PM +0100, Jon Hunter wrote:
> >> Add node for SOR power-domain for Tegra210 and populate the SOR
> >> power-domain phandle for SOR and DPAUX nodes that are dependent
> >> on this power-domain.
> >>
> >> Please note that although neither the SOR or DPAUX drivers currently
> >> support runtime power-management, by populating the power-domain node
> >> the SOR power-domain will be turned on before probing SOR or DPAUX
> >> devices and kept on while the devices are bound.
> >>
> >> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> >> ---
> >>  arch/arm64/boot/dts/nvidia/tegra210.dtsi | 18 ++++++++++++++++++
> >>  1 file changed, 18 insertions(+)
> > 
> > I've got patches queued in my drm-tegra tree to add support for runtime
> > PM for the SOR driver. Will that in some way clash with this work if
> > merged in parallel?
> 
> Yes I had seen those. I was wondering if I should include them here but
> opted not too (as it was becoming a massive series). It should be fine
> to enable RPM later, you just need to be aware that the partition could
> turn off on a RPM put so you just need to ensure that any register
> context is saved and restored.

This doesn't matter much for Tegra DRM because we always program the
entire set of registers after pm_runtime_get(), which already resets
the IP block anyway.

> By the way, one item I am not certain about is the relationship between
> the HDMI device and the SOR device. If the HDMI is in the SOR partition
> it should also have the SOR power-domain populated, but it just uses the
> SOR, then the SOR driver should handle this.

On Tegra210 the HDMI output is driven by the SOR, and that's implemented
by the SOR driver, too, so the partition setup should be correct
already.

On Tegra124 and earlier the HDMI output is driven by a separate IP block
that's not at all related to SOR, and I haven't found any evidence that
it would be in the SOR power partition either.

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 11/13] arm64: tegra: Add SOR power-domain node

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2463 bytes --]

On Fri, Jun 17, 2016 at 01:03:45PM +0100, Jon Hunter wrote:
> Add node for SOR power-domain for Tegra210 and populate the SOR
> power-domain phandle for SOR and DPAUX nodes that are dependent
> on this power-domain.
> 
> Please note that although neither the SOR or DPAUX drivers currently
> support runtime power-management, by populating the power-domain node
> the SOR power-domain will be turned on before probing SOR or DPAUX
> devices and kept on while the devices are bound.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  arch/arm64/boot/dts/nvidia/tegra210.dtsi | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> index ebf44f4059f8..94f780b43037 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> @@ -34,6 +34,7 @@
>  			clock-names = "dpaux", "parent";
>  			resets = <&tegra_car 207>;
>  			reset-names = "dpaux";
> +			power-domains = <&pd_sor>;
>  			status = "disabled";
>  		};
>  
> @@ -154,6 +155,7 @@
>  			clock-names = "sor", "parent", "dp", "safe";
>  			resets = <&tegra_car 182>;
>  			reset-names = "sor";
> +			power-domains = <&pd_sor>;
>  			status = "disabled";
>  		};
>  
> @@ -168,6 +170,7 @@
>  			clock-names = "sor", "parent", "dp", "safe";
>  			resets = <&tegra_car 183>;
>  			reset-names = "sor";
> +			power-domains = <&pd_sor>;
>  			status = "disabled";
>  		};
>  
> @@ -180,6 +183,7 @@
>  			clock-names = "dpaux", "parent";
>  			resets = <&tegra_car 181>;
>  			reset-names = "dpaux";
> +			power-domains = <&pd_sor>;
>  			status = "disabled";
>  		};
>  
> @@ -592,6 +596,20 @@
>  				resets = <&tegra_car 198>;
>  				#power-domain-cells = <0>;
>  			};
> +
> +			pd_sor: sor {
> +				clocks = <&tegra_car TEGRA210_CLK_SOR0>,
> +					 <&tegra_car TEGRA210_CLK_DSIA>,
> +					 <&tegra_car TEGRA210_CLK_DSIB>,
> +					 <&tegra_car TEGRA210_CLK_MIPI_CAL>,

Does this mean that all of these clocks will be running while the SOR
partition is enabled? Seems like a waste because we rarely need MIPI_CAL
and DSIA and DSIB are completely unused on boards that for example have
only an HDMI output.

I vaguely remember the power domain driver only making sure these are
enabled during the partition power transitions, so perhaps my concerns
aren't justified?

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 11/13] arm64: tegra: Add SOR power-domain node

[Jon Hunter]

On 17/06/16 17:44, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Jun 17, 2016 at 01:03:45PM +0100, Jon Hunter wrote:
>> Add node for SOR power-domain for Tegra210 and populate the SOR
>> power-domain phandle for SOR and DPAUX nodes that are dependent
>> on this power-domain.
>>
>> Please note that although neither the SOR or DPAUX drivers currently
>> support runtime power-management, by populating the power-domain node
>> the SOR power-domain will be turned on before probing SOR or DPAUX
>> devices and kept on while the devices are bound.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  arch/arm64/boot/dts/nvidia/tegra210.dtsi | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>> index ebf44f4059f8..94f780b43037 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>> @@ -34,6 +34,7 @@
>>  			clock-names = "dpaux", "parent";
>>  			resets = <&tegra_car 207>;
>>  			reset-names = "dpaux";
>> +			power-domains = <&pd_sor>;
>>  			status = "disabled";
>>  		};
>>  
>> @@ -154,6 +155,7 @@
>>  			clock-names = "sor", "parent", "dp", "safe";
>>  			resets = <&tegra_car 182>;
>>  			reset-names = "sor";
>> +			power-domains = <&pd_sor>;
>>  			status = "disabled";
>>  		};
>>  
>> @@ -168,6 +170,7 @@
>>  			clock-names = "sor", "parent", "dp", "safe";
>>  			resets = <&tegra_car 183>;
>>  			reset-names = "sor";
>> +			power-domains = <&pd_sor>;
>>  			status = "disabled";
>>  		};
>>  
>> @@ -180,6 +183,7 @@
>>  			clock-names = "dpaux", "parent";
>>  			resets = <&tegra_car 181>;
>>  			reset-names = "dpaux";
>> +			power-domains = <&pd_sor>;
>>  			status = "disabled";
>>  		};
>>  
>> @@ -592,6 +596,20 @@
>>  				resets = <&tegra_car 198>;
>>  				#power-domain-cells = <0>;
>>  			};
>> +
>> +			pd_sor: sor {
>> +				clocks = <&tegra_car TEGRA210_CLK_SOR0>,
>> +					 <&tegra_car TEGRA210_CLK_DSIA>,
>> +					 <&tegra_car TEGRA210_CLK_DSIB>,
>> +					 <&tegra_car TEGRA210_CLK_MIPI_CAL>,
> 
> Does this mean that all of these clocks will be running while the SOR
> partition is enabled? Seems like a waste because we rarely need MIPI_CAL
> and DSIA and DSIB are completely unused on boards that for example have
> only an HDMI output.
> 
> I vaguely remember the power domain driver only making sure these are
> enabled during the partition power transitions, so perhaps my concerns
> aren't justified?

Right, these are only enabled during the transition of the partition. So
the SOR driver still needs to enable the clocks it needs.

Cheers
Jon

-- 
nvpublic
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC PATCH 12/13] arm64: tegra: Add sor-safe clock to DPAUX binding

[Jon Hunter]

Populate the 'sor-safe' clock for DPAUX devices on Tegra210 that require
this clock for operation. Update the compatability string for the DPAUX
instance at address 0x545c0000 to be "nvidia,tegra210-dpaux" to ensure
that the 'sor-safe' clock is enabled for this device.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra210.dtsi | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 94f780b43037..78bcc87b627d 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -30,8 +30,9 @@
 			reg = <0x0 0x54040000 0x0 0x00040000>;
 			interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA210_CLK_DPAUX1>,
-				 <&tegra_car TEGRA210_CLK_PLL_DP>;
-			clock-names = "dpaux", "parent";
+				 <&tegra_car TEGRA210_CLK_PLL_DP>,
+				 <&tegra_car TEGRA210_CLK_SOR_SAFE>;
+			clock-names = "dpaux", "parent", "sor-safe";
 			resets = <&tegra_car 207>;
 			reset-names = "dpaux";
 			power-domains = <&pd_sor>;
@@ -175,12 +176,13 @@
 		};
 
 		dpaux: dpaux@545c0000 {
-			compatible = "nvidia,tegra124-dpaux";
+			compatible = "nvidia,tegra210-dpaux";
 			reg = <0x0 0x545c0000 0x0 0x00040000>;
 			interrupts = <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&tegra_car TEGRA210_CLK_DPAUX>,
-				 <&tegra_car TEGRA210_CLK_PLL_DP>;
-			clock-names = "dpaux", "parent";
+				 <&tegra_car TEGRA210_CLK_PLL_DP>,
+				 <&tegra_car TEGRA210_CLK_SOR_SAFE>;
+			clock-names = "dpaux", "parent", "sor-safe";
 			resets = <&tegra_car 181>;
 			reset-names = "dpaux";
 			power-domains = <&pd_sor>;
-- 
2.1.4

Re: [RFC PATCH 12/13] arm64: tegra: Add sor-safe clock to DPAUX binding

[Thierry Reding]

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

On Fri, Jun 17, 2016 at 01:03:46PM +0100, Jon Hunter wrote:
> Populate the 'sor-safe' clock for DPAUX devices on Tegra210 that require
> this clock for operation. Update the compatability string for the DPAUX
> instance at address 0x545c0000 to be "nvidia,tegra210-dpaux" to ensure
> that the 'sor-safe' clock is enabled for this device.

Does the second DPAUX need this, too? I have a vague recollection that
they were both slightly different.

Thierry

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

Re: [RFC PATCH 12/13] arm64: tegra: Add sor-safe clock to DPAUX binding

[Jon Hunter]

On 17/06/16 17:47, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Jun 17, 2016 at 01:03:46PM +0100, Jon Hunter wrote:
>> Populate the 'sor-safe' clock for DPAUX devices on Tegra210 that require
>> this clock for operation. Update the compatability string for the DPAUX
>> instance at address 0x545c0000 to be "nvidia,tegra210-dpaux" to ensure
>> that the 'sor-safe' clock is enabled for this device.
> 
> Does the second DPAUX need this, too? I have a vague recollection that
> they were both slightly different.

I have assumed so, but I am checking with the h/w folks on this. Right
now the TRM only describes the procedure for configuring the DPAUX pads
for i2c6. I am also asking about sharing the DPAUX1 pads with i2c4.

Cheers
Jon

-- 
nvpublic
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 12/13] arm64: tegra: Add sor-safe clock to DPAUX binding

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1042 bytes --]

On Mon, Jun 20, 2016 at 10:23:38AM +0100, Jon Hunter wrote:
> 
> On 17/06/16 17:47, Thierry Reding wrote:
> > * PGP Signed by an unknown key
> > 
> > On Fri, Jun 17, 2016 at 01:03:46PM +0100, Jon Hunter wrote:
> >> Populate the 'sor-safe' clock for DPAUX devices on Tegra210 that require
> >> this clock for operation. Update the compatability string for the DPAUX
> >> instance at address 0x545c0000 to be "nvidia,tegra210-dpaux" to ensure
> >> that the 'sor-safe' clock is enabled for this device.
> > 
> > Does the second DPAUX need this, too? I have a vague recollection that
> > they were both slightly different.
> 
> I have assumed so, but I am checking with the h/w folks on this. Right
> now the TRM only describes the procedure for configuring the DPAUX pads
> for i2c6. I am also asking about sharing the DPAUX1 pads with i2c4.

Yes, last time I looked it wasn't documented anywhere with which I2C
controller the other DPAUX shared its pads.

It'd be good to get all of that documented in the TRM.

Thierry

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel