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
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@nvidia.com --- 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)
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; }
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
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
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 - resets: Must contain an entry for each entry in reset-names. See ../reset/reset.txt for details. - reset-names: Must include the following entries:
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
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@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.
Yes that is fine with me.
Jon
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; }
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
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
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
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);
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 --- 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,
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?
Acked-by: Linus Walleij linus.walleij@linaro.org
Yours, Linus Walleij
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
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.
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
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.
Thanks, Mark.
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
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@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.
Yes, indeed. Thanks. Will fix.
Cheers Jon
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)
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
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
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"; + };
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
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")
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)
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
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
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
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>; + }; }; };
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
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
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
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
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
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>;
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
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
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
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(+)
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"; };
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
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
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
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
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?
Yours, Linus Walleij
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
On Thu, Jun 23, 2016 at 10:04 AM, Thierry Reding thierry.reding@gmail.com 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@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.
OK use this: https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-pinctrl.git/log/?h...
I merged that into my devel branch too.
Yours, Linus Walleij
dri-devel@lists.freedesktop.org
-
Jon Hunter -
Linus Walleij -
Mark Rutland -
Thierry Reding