[PATCH v2 0/4] tegra: Some patches to improve MIPI d-phy calibration

[PATCH v2 0/4] tegra: Some patches to improve MIPI d-phy calibration

[Sean Paul]

A few patches to make mipi d-phy calibration match the procedure in the TRM,
as well as making sure the mipi-cal clk has the correct parent.

Sean Paul (4):
  clk: tegra: Move mipi-cal initialization into clk-tegra1[12]4.c
  host1x: MIPI: Preserve the contents of MIPI_CAL_CTRL
  host1x: MIPI: Include clock lanes in MIPI calibrate
  host1x: MIPI: Set MIPI_CAL_BIAS_PAD_CFG1 register

 drivers/clk/tegra/clk-tegra-periph.c |   1 -
 drivers/clk/tegra/clk-tegra114.c     |   5 ++
 drivers/clk/tegra/clk-tegra124.c     |   5 ++
 drivers/gpu/host1x/mipi.c            | 119 ++++++++++++++++++++++++++---------
 4 files changed, 101 insertions(+), 29 deletions(-)

-- 
2.0.0

[PATCH v2 1/4] clk: tegra: Move mipi-cal initialization into clk-tegra1[12]4.c

[Sean Paul]

This patch moves the mipi-cal gate registration down into the SoC
specific files to reflect the different in parent between them.

Without this change, MIPI calibration will fail on K1 devices if
the 72MHz clock is off.

Signed-off-by: Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Changes in v2:
	- Added this patch in favor of the patch that did this in the mipi driver

 drivers/clk/tegra/clk-tegra-periph.c | 1 -
 drivers/clk/tegra/clk-tegra114.c     | 5 +++++
 drivers/clk/tegra/clk-tegra124.c     | 5 +++++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
index 37f32c4..97c0eb4 100644
--- a/drivers/clk/tegra/clk-tegra-periph.c
+++ b/drivers/clk/tegra/clk-tegra-periph.c
@@ -524,7 +524,6 @@ static struct tegra_periph_init_data gate_clks[] = {
 	GATE("hda2hdmi", "clk_m", 128, TEGRA_PERIPH_ON_APB, tegra_clk_hda2hdmi, 0),
 	GATE("bsea", "clk_m", 62, 0, tegra_clk_bsea, 0),
 	GATE("bsev", "clk_m", 63, 0, tegra_clk_bsev, 0),
-	GATE("mipi-cal", "clk_m", 56, 0, tegra_clk_mipi_cal, 0),
 	GATE("usbd", "clk_m", 22, 0, tegra_clk_usbd, 0),
 	GATE("usb2", "clk_m", 58, 0, tegra_clk_usb2, 0),
 	GATE("usb3", "clk_m", 59, 0, tegra_clk_usb3, 0),
diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c
index f760f31..5a1fa41 100644
--- a/drivers/clk/tegra/clk-tegra114.c
+++ b/drivers/clk/tegra/clk-tegra114.c
@@ -1238,6 +1238,11 @@ static __init void tegra114_periph_clk_init(void __iomem *clk_base,
 		clks[data->clk_id] = clk;
 	}
 
+	clk = tegra_clk_register_periph_gate("mipi-cal", "clk_m", 0, clk_base,
+				0, TEGRA114_CLK_MIPI_CAL,
+				periph_clk_enb_refcnt);
+	clks[TEGRA114_CLK_MIPI_CAL] = clk;
+
 	tegra_periph_clk_init(clk_base, pmc_base, tegra114_clks,
 				&pll_p_params);
 }
diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
index 9525c68..0c3c892 100644
--- a/drivers/clk/tegra/clk-tegra124.c
+++ b/drivers/clk/tegra/clk-tegra124.c
@@ -1141,6 +1141,11 @@ static __init void tegra124_periph_clk_init(void __iomem *clk_base,
 	clk_register_clkdev(clk, "cml1", NULL);
 	clks[TEGRA124_CLK_CML1] = clk;
 
+	clk = tegra_clk_register_periph_gate("mipi-cal", "clk72mhz", 0,
+				clk_base, 0, TEGRA124_CLK_MIPI_CAL,
+				periph_clk_enb_refcnt);
+	clks[TEGRA124_CLK_MIPI_CAL] = clk;
+
 	tegra_periph_clk_init(clk_base, pmc_base, tegra124_clks, &pll_p_params);
 }
 
-- 
2.0.0

[PATCH v2 2/4] host1x: MIPI: Preserve the contents of MIPI_CAL_CTRL

[Sean Paul]

By paving the CTRL reg value, the current code changes
MIPI_CAL_PRESCALE ("Auto-cal calibration step prescale")
from 1us to 0.1us (val=0). In the description for phy's noise
filter (MIPI_CAL_NOISE_FLT), the TRM states that if the value
of the prescale is 0 (or 0.1us), the filter should be set
between 2-5. However, the current code sets it to 0.

For now, let's keep the prescale and filter values as-is, which
is most likely the power-on-reset values of 0x2 and 0xa, respectively.

Signed-off-by: Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Changes in v2:
	- Improved the commit description

 drivers/gpu/host1x/mipi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/host1x/mipi.c b/drivers/gpu/host1x/mipi.c
index 9882ea1..0f3b77e 100644
--- a/drivers/gpu/host1x/mipi.c
+++ b/drivers/gpu/host1x/mipi.c
@@ -208,7 +208,9 @@ int tegra_mipi_calibrate(struct tegra_mipi_device *device)
 		tegra_mipi_writel(device->mipi, value, modules[i].reg);
 	}
 
-	tegra_mipi_writel(device->mipi, MIPI_CAL_CTRL_START, MIPI_CAL_CTRL);
+	value = tegra_mipi_readl(device->mipi, MIPI_CAL_CTRL);
+	value |= MIPI_CAL_CTRL_START;
+	tegra_mipi_writel(device->mipi, value, MIPI_CAL_CTRL);
 
 	err = tegra_mipi_wait(device->mipi);
 
-- 
2.0.0

[PATCH v2 3/4] host1x: MIPI: Include clock lanes in MIPI calibrate

[Sean Paul]

When calibrating the MIPI phy on tegra 124 compatible devices,
also include the clock lanes in the calibration.

Signed-off-by: Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Changes in v2:
	- Don't try to calibrate clock lane on t114

 drivers/gpu/host1x/mipi.c | 111 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 84 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/host1x/mipi.c b/drivers/gpu/host1x/mipi.c
index 0f3b77e..e017f7f 100644
--- a/drivers/gpu/host1x/mipi.c
+++ b/drivers/gpu/host1x/mipi.c
@@ -49,10 +49,18 @@
 #define MIPI_CAL_CONFIG_DSIC		0x10
 #define MIPI_CAL_CONFIG_DSID		0x11
 
+#define MIPI_CAL_CONFIG_DSIAB_CLK	0x19
+#define MIPI_CAL_CONFIG_DSICD_CLK	0x1a
+#define MIPI_CAL_CONFIG_CSIAB_CLK	0x1b
+#define MIPI_CAL_CONFIG_CSICD_CLK	0x1c
+#define MIPI_CAL_CONFIG_CSIE_CLK	0x1d
+
 #define MIPI_CAL_CONFIG_SELECT		(1 << 21)
 #define MIPI_CAL_CONFIG_HSPDOS(x)	(((x) & 0x1f) << 16)
 #define MIPI_CAL_CONFIG_HSPUOS(x)	(((x) & 0x1f) <<  8)
 #define MIPI_CAL_CONFIG_TERMOS(x)	(((x) & 0x1f) <<  0)
+#define MIPI_CAL_CONFIG_HSCLKPDOSD(x)	(((x) & 0x1f) << 8)
+#define MIPI_CAL_CONFIG_HSCLKPUOSD(x)	(((x) & 0x1f) <<  0)
 
 #define MIPI_CAL_BIAS_PAD_CFG0		0x16
 #define MIPI_CAL_BIAS_PAD_PDVCLAMP	(1 << 1)
@@ -63,18 +71,15 @@
 #define MIPI_CAL_BIAS_PAD_CFG2		0x18
 #define MIPI_CAL_BIAS_PAD_PDVREG	(1 << 1)
 
-static const struct module {
-	unsigned long reg;
-} modules[] = {
-	{ .reg = MIPI_CAL_CONFIG_CSIA },
-	{ .reg = MIPI_CAL_CONFIG_CSIB },
-	{ .reg = MIPI_CAL_CONFIG_CSIC },
-	{ .reg = MIPI_CAL_CONFIG_CSID },
-	{ .reg = MIPI_CAL_CONFIG_CSIE },
-	{ .reg = MIPI_CAL_CONFIG_DSIA },
-	{ .reg = MIPI_CAL_CONFIG_DSIB },
-	{ .reg = MIPI_CAL_CONFIG_DSIC },
-	{ .reg = MIPI_CAL_CONFIG_DSID },
+struct calibration_regs {
+	unsigned long data;
+	unsigned long clk;
+};
+
+struct tegra_mipi_config {
+	bool calibrate_clk_lane;
+	int num_pads;
+	const struct calibration_regs *regs;
 };
 
 struct tegra_mipi {
@@ -87,6 +92,7 @@ struct tegra_mipi_device {
 	struct platform_device *pdev;
 	struct tegra_mipi *mipi;
 	struct device *device;
+	const struct tegra_mipi_config *config;
 	unsigned long pads;
 };
 
@@ -102,9 +108,48 @@ static inline void tegra_mipi_writel(struct tegra_mipi *mipi,
 	writel(value, mipi->regs + (reg << 2));
 }
 
+static const struct calibration_regs tegra114_mipi_calibration_regs[] = {
+	{ .data = MIPI_CAL_CONFIG_CSIA },
+	{ .data = MIPI_CAL_CONFIG_CSIB },
+	{ .data = MIPI_CAL_CONFIG_CSIC },
+	{ .data = MIPI_CAL_CONFIG_CSID },
+	{ .data = MIPI_CAL_CONFIG_CSIE },
+	{ .data = MIPI_CAL_CONFIG_DSIA },
+	{ .data = MIPI_CAL_CONFIG_DSIB },
+	{ .data = MIPI_CAL_CONFIG_DSIC },
+	{ .data = MIPI_CAL_CONFIG_DSID },
+};
+static const struct tegra_mipi_config tegra114_mipi_config = {
+	.calibrate_clk_lane = false,
+	.regs = tegra114_mipi_calibration_regs,
+	.num_pads = ARRAY_SIZE(tegra114_mipi_calibration_regs),
+};
+
+static const struct calibration_regs tegra124_mipi_calibration_regs[] = {
+	{ .data = MIPI_CAL_CONFIG_CSIA, .clk = MIPI_CAL_CONFIG_CSIAB_CLK },
+	{ .data = MIPI_CAL_CONFIG_CSIB, .clk = MIPI_CAL_CONFIG_CSIAB_CLK },
+	{ .data = MIPI_CAL_CONFIG_CSIC, .clk = MIPI_CAL_CONFIG_CSICD_CLK },
+	{ .data = MIPI_CAL_CONFIG_CSID, .clk = MIPI_CAL_CONFIG_CSICD_CLK },
+	{ .data = MIPI_CAL_CONFIG_CSIE, .clk = MIPI_CAL_CONFIG_CSIE_CLK },
+	{ .data = MIPI_CAL_CONFIG_DSIA, .clk = MIPI_CAL_CONFIG_DSIAB_CLK },
+	{ .data = MIPI_CAL_CONFIG_DSIB, .clk = MIPI_CAL_CONFIG_DSIAB_CLK },
+};
+static const struct tegra_mipi_config tegra124_mipi_config = {
+	.calibrate_clk_lane = true,
+	.regs = tegra124_mipi_calibration_regs,
+	.num_pads = ARRAY_SIZE(tegra124_mipi_calibration_regs),
+};
+
+static struct of_device_id tegra_mipi_of_match[] = {
+	{ .compatible = "nvidia,tegra114-mipi", .data = &tegra114_mipi_config },
+	{ .compatible = "nvidia,tegra124-mipi", .data = &tegra124_mipi_config },
+	{ },
+};
+
 struct tegra_mipi_device *tegra_mipi_request(struct device *device)
 {
 	struct device_node *np = device->of_node;
+	const struct of_device_id *match;
 	struct tegra_mipi_device *dev;
 	struct of_phandle_args args;
 	int err;
@@ -117,19 +162,23 @@ struct tegra_mipi_device *tegra_mipi_request(struct device *device)
 
 	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
 	if (!dev) {
-		of_node_put(args.np);
 		err = -ENOMEM;
 		goto out;
 	}
 
 	dev->pdev = of_find_device_by_node(args.np);
 	if (!dev->pdev) {
-		of_node_put(args.np);
 		err = -ENODEV;
-		goto free;
+		goto node_put;
 	}
 
-	of_node_put(args.np);
+	match = of_match_node(tegra_mipi_of_match, dev->pdev->dev.of_node);
+	if (!match) {
+		err = -ENODEV;
+		goto node_put;
+	}
+
+	dev->config = match->data;
 
 	dev->mipi = platform_get_drvdata(dev->pdev);
 	if (!dev->mipi) {
@@ -137,6 +186,8 @@ struct tegra_mipi_device *tegra_mipi_request(struct device *device)
 		goto pdev_put;
 	}
 
+	of_node_put(args.np);
+
 	dev->pads = args.args[0];
 	dev->device = device;
 
@@ -144,7 +195,8 @@ struct tegra_mipi_device *tegra_mipi_request(struct device *device)
 
 pdev_put:
 	platform_device_put(dev->pdev);
-free:
+node_put:
+	of_node_put(args.np);
 	kfree(dev);
 out:
 	return ERR_PTR(err);
@@ -177,7 +229,8 @@ static int tegra_mipi_wait(struct tegra_mipi *mipi)
 
 int tegra_mipi_calibrate(struct tegra_mipi_device *device)
 {
-	unsigned long value;
+	const struct tegra_mipi_config *cfg = device->config;
+	unsigned long value, clk_value;
 	unsigned int i;
 	int err;
 
@@ -196,16 +249,25 @@ int tegra_mipi_calibrate(struct tegra_mipi_device *device)
 	value &= ~MIPI_CAL_BIAS_PAD_PDVREG;
 	tegra_mipi_writel(device->mipi, value, MIPI_CAL_BIAS_PAD_CFG2);
 
-	for (i = 0; i < ARRAY_SIZE(modules); i++) {
-		if (device->pads & BIT(i))
+	for (i = 0; i < cfg->num_pads; i++) {
+		if (device->pads & BIT(i)) {
 			value = MIPI_CAL_CONFIG_SELECT |
 				MIPI_CAL_CONFIG_HSPDOS(0) |
 				MIPI_CAL_CONFIG_HSPUOS(4) |
 				MIPI_CAL_CONFIG_TERMOS(5);
-		else
+			clk_value = MIPI_CAL_CONFIG_SELECT |
+				    MIPI_CAL_CONFIG_HSCLKPDOSD(0) |
+				    MIPI_CAL_CONFIG_HSCLKPUOSD(4);
+		} else {
 			value = 0;
+			clk_value = 0;
+		}
+
+		tegra_mipi_writel(device->mipi, value, cfg->regs[i].data);
 
-		tegra_mipi_writel(device->mipi, value, modules[i].reg);
+		if (cfg->calibrate_clk_lane)
+			tegra_mipi_writel(device->mipi, clk_value,
+					cfg->regs[i].clk);
 	}
 
 	value = tegra_mipi_readl(device->mipi, MIPI_CAL_CTRL);
@@ -262,11 +324,6 @@ static int tegra_mipi_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct of_device_id tegra_mipi_of_match[] = {
-	{ .compatible = "nvidia,tegra114-mipi", },
-	{ },
-};
-
 struct platform_driver tegra_mipi_driver = {
 	.driver = {
 		.name = "tegra-mipi",
-- 
2.0.0

[PATCH v2 4/4] host1x: MIPI: Set MIPI_CAL_BIAS_PAD_CFG1 register

[Sean Paul]

During calibration, sets the "internal reference level for drive
pull-down" to the value specified in the tegra TRM.

Signed-off-by: Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Changes in v2:
	- Added field definition for PAD_DRV_DN_REF

 drivers/gpu/host1x/mipi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/host1x/mipi.c b/drivers/gpu/host1x/mipi.c
index e017f7f..c7d6819 100644
--- a/drivers/gpu/host1x/mipi.c
+++ b/drivers/gpu/host1x/mipi.c
@@ -67,6 +67,7 @@
 #define MIPI_CAL_BIAS_PAD_E_VCLAMP_REF	(1 << 0)
 
 #define MIPI_CAL_BIAS_PAD_CFG1		0x17
+#define MIPI_CAL_BIAS_PAD_DRV_DN_REF(x) (((x) & 0x7) << 16)
 
 #define MIPI_CAL_BIAS_PAD_CFG2		0x18
 #define MIPI_CAL_BIAS_PAD_PDVREG	(1 << 1)
@@ -245,6 +246,9 @@ int tegra_mipi_calibrate(struct tegra_mipi_device *device)
 	value |= MIPI_CAL_BIAS_PAD_E_VCLAMP_REF;
 	tegra_mipi_writel(device->mipi, value, MIPI_CAL_BIAS_PAD_CFG0);
 
+	tegra_mipi_writel(device->mipi, MIPI_CAL_BIAS_PAD_DRV_DN_REF(2),
+			MIPI_CAL_BIAS_PAD_CFG1);
+
 	value = tegra_mipi_readl(device->mipi, MIPI_CAL_BIAS_PAD_CFG2);
 	value &= ~MIPI_CAL_BIAS_PAD_PDVREG;
 	tegra_mipi_writel(device->mipi, value, MIPI_CAL_BIAS_PAD_CFG2);
-- 
2.0.0

Re: [PATCH v2 0/4] tegra: Some patches to improve MIPI d-phy calibration

[Stephen Warren]

On 09/10/2014 08:52 AM, Sean Paul wrote:
> A few patches to make mipi d-phy calibration match the procedure in the TRM,
> as well as making sure the mipi-cal clk has the correct parent.

Peter De Schrijver is the drivers/clk/tegra maintainer; explicitly CCing 
him just in case he didn't noticed the series.

Re: [PATCH v2 1/4] clk: tegra: Move mipi-cal initialization into clk-tegra1[12]4.c

[Stephen Warren]

On 09/10/2014 08:52 AM, Sean Paul wrote:
> This patch moves the mipi-cal gate registration down into the SoC
> specific files to reflect the different in parent between them.
>
> Without this change, MIPI calibration will fail on K1 devices if
> the 72MHz clock is off.

This isn't a problem with this patch per se, but I notice that what's 
removed from clk-tegra-periph.c is data in a table, whereas open-coded 
calls are added to clk-tegra*.c. It'd be nice if the open-coded calls in 
clk-tegra*.c could be converted to a table (simply with SoC-specific 
data) and processed by the same function. Peter, would that make sense?

Re: [PATCH v2 1/4] clk: tegra: Move mipi-cal initialization into clk-tegra1[12]4.c

[Peter De Schrijver]

On Wed, Sep 10, 2014 at 06:17:44PM +0200, Stephen Warren wrote:
> On 09/10/2014 08:52 AM, Sean Paul wrote:
> > This patch moves the mipi-cal gate registration down into the SoC
> > specific files to reflect the different in parent between them.
> >
> > Without this change, MIPI calibration will fail on K1 devices if
> > the 72MHz clock is off.
> 
> This isn't a problem with this patch per se, but I notice that what's 
> removed from clk-tegra-periph.c is data in a table, whereas open-coded 
> calls are added to clk-tegra*.c. It'd be nice if the open-coded calls in 
> clk-tegra*.c could be converted to a table (simply with SoC-specific 
> data) and processed by the same function. Peter, would that make sense?

Maybe yes. I will look into this. For this patch, it would make more sense
to keep the entry in clk-tegra-periph.c as it can be used by both Tegra124
and Tegra132.

Cheers,

Peter.

Re: [PATCH v2 1/4] clk: tegra: Move mipi-cal initialization into clk-tegra1[12]4.c

[Sean Paul]

On Thu, Sep 18, 2014 at 7:26 AM, Peter De Schrijver
<pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> On Wed, Sep 10, 2014 at 06:17:44PM +0200, Stephen Warren wrote:
>> On 09/10/2014 08:52 AM, Sean Paul wrote:
>> > This patch moves the mipi-cal gate registration down into the SoC
>> > specific files to reflect the different in parent between them.
>> >
>> > Without this change, MIPI calibration will fail on K1 devices if
>> > the 72MHz clock is off.
>>
>> This isn't a problem with this patch per se, but I notice that what's
>> removed from clk-tegra-periph.c is data in a table, whereas open-coded
>> calls are added to clk-tegra*.c. It'd be nice if the open-coded calls in
>> clk-tegra*.c could be converted to a table (simply with SoC-specific
>> data) and processed by the same function. Peter, would that make sense?
>
> Maybe yes. I will look into this. For this patch, it would make more sense
> to keep the entry in clk-tegra-periph.c as it can be used by both Tegra124
> and Tegra132.
>

How do you suggest we switch the parent based on 124 vs 132 if it
remains in clk-tegra-periph.c?

Sean



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

Re: [PATCH v2 1/4] clk: tegra: Move mipi-cal initialization into clk-tegra1[12]4.c

[Peter De Schrijver]

On Thu, Sep 18, 2014 at 08:59:53PM +0200, Sean Paul wrote:
> On Thu, Sep 18, 2014 at 7:26 AM, Peter De Schrijver
> <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > On Wed, Sep 10, 2014 at 06:17:44PM +0200, Stephen Warren wrote:
> >> On 09/10/2014 08:52 AM, Sean Paul wrote:
> >> > This patch moves the mipi-cal gate registration down into the SoC
> >> > specific files to reflect the different in parent between them.
> >> >
> >> > Without this change, MIPI calibration will fail on K1 devices if
> >> > the 72MHz clock is off.
> >>
> >> This isn't a problem with this patch per se, but I notice that what's
> >> removed from clk-tegra-periph.c is data in a table, whereas open-coded
> >> calls are added to clk-tegra*.c. It'd be nice if the open-coded calls in
> >> clk-tegra*.c could be converted to a table (simply with SoC-specific
> >> data) and processed by the same function. Peter, would that make sense?
> >
> > Maybe yes. I will look into this. For this patch, it would make more sense
> > to keep the entry in clk-tegra-periph.c as it can be used by both Tegra124
> > and Tegra132.
> >
> 
> How do you suggest we switch the parent based on 124 vs 132 if it
> remains in clk-tegra-periph.c?
> 

I would change the parent of mipi-cal in clk-tegra-periph.c to clk72mhz and
use a different mechanism for Tegra114.

Cheers,

Peter.

Re: [PATCH v2 1/4] clk: tegra: Move mipi-cal initialization into clk-tegra1[12]4.c

[Sean Paul]

On Fri, Sep 19, 2014 at 7:28 AM, Peter De Schrijver
<pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> On Thu, Sep 18, 2014 at 08:59:53PM +0200, Sean Paul wrote:
>> On Thu, Sep 18, 2014 at 7:26 AM, Peter De Schrijver
>> <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> > On Wed, Sep 10, 2014 at 06:17:44PM +0200, Stephen Warren wrote:
>> >> On 09/10/2014 08:52 AM, Sean Paul wrote:
>> >> > This patch moves the mipi-cal gate registration down into the SoC
>> >> > specific files to reflect the different in parent between them.
>> >> >
>> >> > Without this change, MIPI calibration will fail on K1 devices if
>> >> > the 72MHz clock is off.
>> >>
>> >> This isn't a problem with this patch per se, but I notice that what's
>> >> removed from clk-tegra-periph.c is data in a table, whereas open-coded
>> >> calls are added to clk-tegra*.c. It'd be nice if the open-coded calls in
>> >> clk-tegra*.c could be converted to a table (simply with SoC-specific
>> >> data) and processed by the same function. Peter, would that make sense?
>> >
>> > Maybe yes. I will look into this. For this patch, it would make more sense
>> > to keep the entry in clk-tegra-periph.c as it can be used by both Tegra124
>> > and Tegra132.
>> >
>>
>> How do you suggest we switch the parent based on 124 vs 132 if it
>> remains in clk-tegra-periph.c?
>>
>
> I would change the parent of mipi-cal in clk-tegra-periph.c to clk72mhz and
> use a different mechanism for Tegra114.
>

Hi Peter,
Sorry for the delay, I'm just getting back to this now.

I'm still unclear on how you propose we alter mipi-cal in tegra114.
AFAICT, we can't re-parent it since it's a gate clock. Can you please
be a little more specific (and forgive my ignorance wrt this driver,
it's still new to me)?

Sean



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

Re: [PATCH v2 2/4] host1x: MIPI: Preserve the contents of MIPI_CAL_CTRL

[Thierry Reding]

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

On Wed, Sep 10, 2014 at 10:52:03AM -0400, Sean Paul wrote:
> By paving the CTRL reg value, the current code changes
> MIPI_CAL_PRESCALE ("Auto-cal calibration step prescale")
> from 1us to 0.1us (val=0). In the description for phy's noise
> filter (MIPI_CAL_NOISE_FLT), the TRM states that if the value
> of the prescale is 0 (or 0.1us), the filter should be set
> between 2-5. However, the current code sets it to 0.
> 
> For now, let's keep the prescale and filter values as-is, which
> is most likely the power-on-reset values of 0x2 and 0xa, respectively.
> 
> Signed-off-by: Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
> Changes in v2:
> 	- Improved the commit description

Applied, thanks. I did reformat the commit description to take advantage
of the full 72 columns.

Thierry

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

Re: [PATCH v2 1/4] clk: tegra: Move mipi-cal initialization into clk-tegra1[12]4.c

[Sean Paul]

On Wed, Oct 1, 2014 at 11:38 AM, Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> On Fri, Sep 19, 2014 at 7:28 AM, Peter De Schrijver
> <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> On Thu, Sep 18, 2014 at 08:59:53PM +0200, Sean Paul wrote:
>>> On Thu, Sep 18, 2014 at 7:26 AM, Peter De Schrijver
>>> <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>>> > On Wed, Sep 10, 2014 at 06:17:44PM +0200, Stephen Warren wrote:
>>> >> On 09/10/2014 08:52 AM, Sean Paul wrote:
>>> >> > This patch moves the mipi-cal gate registration down into the SoC
>>> >> > specific files to reflect the different in parent between them.
>>> >> >
>>> >> > Without this change, MIPI calibration will fail on K1 devices if
>>> >> > the 72MHz clock is off.
>>> >>
>>> >> This isn't a problem with this patch per se, but I notice that what's
>>> >> removed from clk-tegra-periph.c is data in a table, whereas open-coded
>>> >> calls are added to clk-tegra*.c. It'd be nice if the open-coded calls in
>>> >> clk-tegra*.c could be converted to a table (simply with SoC-specific
>>> >> data) and processed by the same function. Peter, would that make sense?
>>> >
>>> > Maybe yes. I will look into this. For this patch, it would make more sense
>>> > to keep the entry in clk-tegra-periph.c as it can be used by both Tegra124
>>> > and Tegra132.
>>> >
>>>
>>> How do you suggest we switch the parent based on 124 vs 132 if it
>>> remains in clk-tegra-periph.c?
>>>
>>
>> I would change the parent of mipi-cal in clk-tegra-periph.c to clk72mhz and
>> use a different mechanism for Tegra114.
>>
>
> Hi Peter,
> Sorry for the delay, I'm just getting back to this now.
>
> I'm still unclear on how you propose we alter mipi-cal in tegra114.
> AFAICT, we can't re-parent it since it's a gate clock. Can you please
> be a little more specific (and forgive my ignorance wrt this driver,
> it's still new to me)?
>

Ping. Can I get some advice on this?

I've also got 2 other outstanding tegra clock patches that have not
received a response:
- clk: tegra124: Add init data for dsi lp clocks
- clk/tegra: Fix mux typo, s/pll_m/clk_m/

Sean


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

Re: [PATCH v2 1/4] clk: tegra: Move mipi-cal initialization into clk-tegra1[12]4.c

[Thierry Reding]

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

On Tue, Oct 07, 2014 at 01:07:29PM -0700, Sean Paul wrote:
> On Wed, Oct 1, 2014 at 11:38 AM, Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > On Fri, Sep 19, 2014 at 7:28 AM, Peter De Schrijver
> > <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> >> On Thu, Sep 18, 2014 at 08:59:53PM +0200, Sean Paul wrote:
> >>> On Thu, Sep 18, 2014 at 7:26 AM, Peter De Schrijver
> >>> <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> >>> > On Wed, Sep 10, 2014 at 06:17:44PM +0200, Stephen Warren wrote:
> >>> >> On 09/10/2014 08:52 AM, Sean Paul wrote:
> >>> >> > This patch moves the mipi-cal gate registration down into the SoC
> >>> >> > specific files to reflect the different in parent between them.
> >>> >> >
> >>> >> > Without this change, MIPI calibration will fail on K1 devices if
> >>> >> > the 72MHz clock is off.
> >>> >>
> >>> >> This isn't a problem with this patch per se, but I notice that what's
> >>> >> removed from clk-tegra-periph.c is data in a table, whereas open-coded
> >>> >> calls are added to clk-tegra*.c. It'd be nice if the open-coded calls in
> >>> >> clk-tegra*.c could be converted to a table (simply with SoC-specific
> >>> >> data) and processed by the same function. Peter, would that make sense?
> >>> >
> >>> > Maybe yes. I will look into this. For this patch, it would make more sense
> >>> > to keep the entry in clk-tegra-periph.c as it can be used by both Tegra124
> >>> > and Tegra132.
> >>> >
> >>>
> >>> How do you suggest we switch the parent based on 124 vs 132 if it
> >>> remains in clk-tegra-periph.c?
> >>>
> >>
> >> I would change the parent of mipi-cal in clk-tegra-periph.c to clk72mhz and
> >> use a different mechanism for Tegra114.
> >>
> >
> > Hi Peter,
> > Sorry for the delay, I'm just getting back to this now.
> >
> > I'm still unclear on how you propose we alter mipi-cal in tegra114.
> > AFAICT, we can't re-parent it since it's a gate clock. Can you please
> > be a little more specific (and forgive my ignorance wrt this driver,
> > it's still new to me)?
> >
> 
> Ping. Can I get some advice on this?

I don't see a way how we can make this work with a table that wouldn't
involve adding a lot more code. Given that we need to separate the table
based on SoC generation we need to add code to both Tegra114 and
Tegra124 to register a table of generation-specific clocks.

We do something like that already for Tegra114 using the
tegra_periph_clk_list table, but initializing the clocks is done using
an open-coded loop. Perhaps one possibility would be to implement a
generic function that takes a table of tegra_periph_init_data structs
and a count. Then we can add per-generation clocks to such a table and
only need to modify the code once.

But again, this would be adding much more code than this patch does, so
perhaps we can address it when we start needing more generation-specific
clocks.

Thierry

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

Re: [PATCH v2 1/4] clk: tegra: Move mipi-cal initialization into clk-tegra1[12]4.c

[Peter De Schrijver]

On Tue, Oct 07, 2014 at 01:07:29PM -0700, Sean Paul wrote:
> On Wed, Oct 1, 2014 at 11:38 AM, Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > On Fri, Sep 19, 2014 at 7:28 AM, Peter De Schrijver
> > <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> >> On Thu, Sep 18, 2014 at 08:59:53PM +0200, Sean Paul wrote:
> >>> On Thu, Sep 18, 2014 at 7:26 AM, Peter De Schrijver
> >>> <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> >>> > On Wed, Sep 10, 2014 at 06:17:44PM +0200, Stephen Warren wrote:
> >>> >> On 09/10/2014 08:52 AM, Sean Paul wrote:
> >>> >> > This patch moves the mipi-cal gate registration down into the SoC
> >>> >> > specific files to reflect the different in parent between them.
> >>> >> >
> >>> >> > Without this change, MIPI calibration will fail on K1 devices if
> >>> >> > the 72MHz clock is off.
> >>> >>
> >>> >> This isn't a problem with this patch per se, but I notice that what's
> >>> >> removed from clk-tegra-periph.c is data in a table, whereas open-coded
> >>> >> calls are added to clk-tegra*.c. It'd be nice if the open-coded calls in
> >>> >> clk-tegra*.c could be converted to a table (simply with SoC-specific
> >>> >> data) and processed by the same function. Peter, would that make sense?
> >>> >
> >>> > Maybe yes. I will look into this. For this patch, it would make more sense
> >>> > to keep the entry in clk-tegra-periph.c as it can be used by both Tegra124
> >>> > and Tegra132.
> >>> >
> >>>
> >>> How do you suggest we switch the parent based on 124 vs 132 if it
> >>> remains in clk-tegra-periph.c?
> >>>
> >>
> >> I would change the parent of mipi-cal in clk-tegra-periph.c to clk72mhz and
> >> use a different mechanism for Tegra114.
> >>
> >
> > Hi Peter,
> > Sorry for the delay, I'm just getting back to this now.
> >
> > I'm still unclear on how you propose we alter mipi-cal in tegra114.
> > AFAICT, we can't re-parent it since it's a gate clock. Can you please
> > be a little more specific (and forgive my ignorance wrt this driver,
> > it's still new to me)?
> >
> 
> Ping. Can I get some advice on this?
> 

Sorry about the late reply. But you could do something like:


diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
index 37f32c4..a9a3b69 100644
--- a/drivers/clk/tegra/clk-tegra-periph.c
+++ b/drivers/clk/tegra/clk-tegra-periph.c
@@ -524,7 +524,7 @@ static struct tegra_periph_init_data gate_clks[] = {
 	GATE("hda2hdmi", "clk_m", 128, TEGRA_PERIPH_ON_APB, tegra_clk_hda2hdmi, 0),
 	GATE("bsea", "clk_m", 62, 0, tegra_clk_bsea, 0),
 	GATE("bsev", "clk_m", 63, 0, tegra_clk_bsev, 0),
-	GATE("mipi-cal", "clk_m", 56, 0, tegra_clk_mipi_cal, 0),
+	GATE("mipi-cal", "clk72mhz", 56, 0, tegra_clk_mipi_cal, 0),
 	GATE("usbd", "clk_m", 22, 0, tegra_clk_usbd, 0),
 	GATE("usb2", "clk_m", 58, 0, tegra_clk_usb2, 0),
 	GATE("usb3", "clk_m", 59, 0, tegra_clk_usb3, 0),
diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c
index f760f31..8b44a8f 100644
--- a/drivers/clk/tegra/clk-tegra114.c
+++ b/drivers/clk/tegra/clk-tegra114.c
@@ -720,7 +720,6 @@ static struct tegra_clk tegra114_clks[tegra_clk_max] __initdata = {
 	[tegra_clk_csi] = { .dt_id = TEGRA114_CLK_CSI, .present = true },
 	[tegra_clk_i2c2] = { .dt_id = TEGRA114_CLK_I2C2, .present = true },
 	[tegra_clk_uartc] = { .dt_id = TEGRA114_CLK_UARTC, .present = true },
-	[tegra_clk_mipi_cal] = { .dt_id = TEGRA114_CLK_MIPI_CAL, .present = true },
 	[tegra_clk_emc] = { .dt_id = TEGRA114_CLK_EMC, .present = true },
 	[tegra_clk_usb2] = { .dt_id = TEGRA114_CLK_USB2, .present = true },
 	[tegra_clk_usb3] = { .dt_id = TEGRA114_CLK_USB3, .present = true },
@@ -1230,6 +1229,11 @@ static __init void tegra114_periph_clk_init(void __iomem *clk_base,
 			       clk_base + CLK_SOURCE_EMC,
 			       29, 3, 0, NULL);
 
+	clk = tegra_clk_register_periph_gate("mipi-cal", "clk_m", 0, clk_base,
+					CLK_SET_RATE_PARENT, 56,
+					periph_clk_enb_refcnt);
+	clks[TEGRA114_CLK_MIPI_CAL] = clk;
+
 	for (i = 0; i < ARRAY_SIZE(tegra_periph_clk_list); i++) {
 		data = &tegra_periph_clk_list[i];
 		clk = tegra_clk_register_periph(data->name,

The advantage would be that this also solves the problem for Tegra132 and
hopefully for future chips. So Tegra114 would be the odd ball out which is
handled manually.

> I've also got 2 other outstanding tegra clock patches that have not
> received a response:
> - clk: tegra124: Add init data for dsi lp clocks
> - clk/tegra: Fix mux typo, s/pll_m/clk_m/
> 

I will have a look.

Cheers,

Peter.

Re: [PATCH v2 1/4] clk: tegra: Move mipi-cal initialization into clk-tegra1[12]4.c

[Sean Paul]

On Wed, Oct 8, 2014 at 8:10 AM, Peter De Schrijver
<pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> On Tue, Oct 07, 2014 at 01:07:29PM -0700, Sean Paul wrote:
>> On Wed, Oct 1, 2014 at 11:38 AM, Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>> > On Fri, Sep 19, 2014 at 7:28 AM, Peter De Schrijver
>> > <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> >> On Thu, Sep 18, 2014 at 08:59:53PM +0200, Sean Paul wrote:
>> >>> On Thu, Sep 18, 2014 at 7:26 AM, Peter De Schrijver
>> >>> <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> >>> > On Wed, Sep 10, 2014 at 06:17:44PM +0200, Stephen Warren wrote:
>> >>> >> On 09/10/2014 08:52 AM, Sean Paul wrote:
>> >>> >> > This patch moves the mipi-cal gate registration down into the SoC
>> >>> >> > specific files to reflect the different in parent between them.
>> >>> >> >
>> >>> >> > Without this change, MIPI calibration will fail on K1 devices if
>> >>> >> > the 72MHz clock is off.
>> >>> >>
>> >>> >> This isn't a problem with this patch per se, but I notice that what's
>> >>> >> removed from clk-tegra-periph.c is data in a table, whereas open-coded
>> >>> >> calls are added to clk-tegra*.c. It'd be nice if the open-coded calls in
>> >>> >> clk-tegra*.c could be converted to a table (simply with SoC-specific
>> >>> >> data) and processed by the same function. Peter, would that make sense?
>> >>> >
>> >>> > Maybe yes. I will look into this. For this patch, it would make more sense
>> >>> > to keep the entry in clk-tegra-periph.c as it can be used by both Tegra124
>> >>> > and Tegra132.
>> >>> >
>> >>>
>> >>> How do you suggest we switch the parent based on 124 vs 132 if it
>> >>> remains in clk-tegra-periph.c?
>> >>>
>> >>
>> >> I would change the parent of mipi-cal in clk-tegra-periph.c to clk72mhz and
>> >> use a different mechanism for Tegra114.
>> >>
>> >
>> > Hi Peter,
>> > Sorry for the delay, I'm just getting back to this now.
>> >
>> > I'm still unclear on how you propose we alter mipi-cal in tegra114.
>> > AFAICT, we can't re-parent it since it's a gate clock. Can you please
>> > be a little more specific (and forgive my ignorance wrt this driver,
>> > it's still new to me)?
>> >
>>
>> Ping. Can I get some advice on this?
>>
>
> Sorry about the late reply. But you could do something like:
>
>
> diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c
> index 37f32c4..a9a3b69 100644
> --- a/drivers/clk/tegra/clk-tegra-periph.c
> +++ b/drivers/clk/tegra/clk-tegra-periph.c
> @@ -524,7 +524,7 @@ static struct tegra_periph_init_data gate_clks[] = {
>         GATE("hda2hdmi", "clk_m", 128, TEGRA_PERIPH_ON_APB, tegra_clk_hda2hdmi, 0),
>         GATE("bsea", "clk_m", 62, 0, tegra_clk_bsea, 0),
>         GATE("bsev", "clk_m", 63, 0, tegra_clk_bsev, 0),
> -       GATE("mipi-cal", "clk_m", 56, 0, tegra_clk_mipi_cal, 0),
> +       GATE("mipi-cal", "clk72mhz", 56, 0, tegra_clk_mipi_cal, 0),
>         GATE("usbd", "clk_m", 22, 0, tegra_clk_usbd, 0),
>         GATE("usb2", "clk_m", 58, 0, tegra_clk_usb2, 0),
>         GATE("usb3", "clk_m", 59, 0, tegra_clk_usb3, 0),
> diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c
> index f760f31..8b44a8f 100644
> --- a/drivers/clk/tegra/clk-tegra114.c
> +++ b/drivers/clk/tegra/clk-tegra114.c
> @@ -720,7 +720,6 @@ static struct tegra_clk tegra114_clks[tegra_clk_max] __initdata = {
>         [tegra_clk_csi] = { .dt_id = TEGRA114_CLK_CSI, .present = true },
>         [tegra_clk_i2c2] = { .dt_id = TEGRA114_CLK_I2C2, .present = true },
>         [tegra_clk_uartc] = { .dt_id = TEGRA114_CLK_UARTC, .present = true },
> -       [tegra_clk_mipi_cal] = { .dt_id = TEGRA114_CLK_MIPI_CAL, .present = true },
>         [tegra_clk_emc] = { .dt_id = TEGRA114_CLK_EMC, .present = true },
>         [tegra_clk_usb2] = { .dt_id = TEGRA114_CLK_USB2, .present = true },
>         [tegra_clk_usb3] = { .dt_id = TEGRA114_CLK_USB3, .present = true },
> @@ -1230,6 +1229,11 @@ static __init void tegra114_periph_clk_init(void __iomem *clk_base,
>                                clk_base + CLK_SOURCE_EMC,
>                                29, 3, 0, NULL);
>
> +       clk = tegra_clk_register_periph_gate("mipi-cal", "clk_m", 0, clk_base,
> +                                       CLK_SET_RATE_PARENT, 56,
> +                                       periph_clk_enb_refcnt);
> +       clks[TEGRA114_CLK_MIPI_CAL] = clk;
> +
>         for (i = 0; i < ARRAY_SIZE(tegra_periph_clk_list); i++) {
>                 data = &tegra_periph_clk_list[i];
>                 clk = tegra_clk_register_periph(data->name,
>
> The advantage would be that this also solves the problem for Tegra132 and
> hopefully for future chips. So Tegra114 would be the odd ball out which is
> handled manually.
>

Indeed, this does seem like a better solution. I hadn't considered
doing it this way.

Would you mind posting a proper patch for it, so that I can pick it up
in the Chromium tree?

Thanks,

Sean



>> I've also got 2 other outstanding tegra clock patches that have not
>> received a response:
>> - clk: tegra124: Add init data for dsi lp clocks
>> - clk/tegra: Fix mux typo, s/pll_m/clk_m/
>>
>
> I will have a look.
>
> Cheers,
>
> Peter.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/4] clk: tegra: Move mipi-cal initialization into clk-tegra1[12]4.c

[Peter De Schrijver]

On Wed, Oct 08, 2014 at 09:19:53AM -0700, Sean Paul wrote:
> >
> 
> Indeed, this does seem like a better solution. I hadn't considered
> doing it this way.
> 
> Would you mind posting a proper patch for it, so that I can pick it up
> in the Chromium tree?
> 
> Thanks,
> 

Ok. Will do.

Cheers,

Peter.