pwm: atmel-hlcdc: add at91sam9x5 and sama5d3 errata handling

PATCH Superseded Boris Brezillon 2 participants 3 messages Nov 19, 2014 – Dec 4, 2014

Subsystems (2)

Arm/Microchip (AT91) SoC support PWM

lore

Share & tools

lore

Boris Brezillon

author

2014-11-19 14:33 UTC #0 lore raw

PATCH v2 1/1 pwm: atmel-hlcdc: add at91sam9x5 and sama5d3 errata handling

at91sam9x5 has an errata forbidding the use of slow clk as a clk source and sama5d3 SoCs has another errata forbidding the use of div1 prescaler.

Take both of these erratas into account.

Signed-off-by Boris Brezillon <[email protected]>

---
Hi Thierry,

I've addressed the "erratas stored in of_device_id data" part, but still haven't modified the compatible strings of the HLCDC subdevices.

Please let me know if you really want to handle erratas through pwm compatibles instead of parent device compatibles.

Regards,

Boris

Changes since v1:
- use data field in of_device_id to attach erratas to an IP revision

 drivers/pwm/pwm-atmel-hlcdc.c | 50 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 5 deletions(-)

drivers/pwm/pwm-atmel-hlcdc.c +45 -5

index eaf8b12..e7c4400 100644
@@ -32,10 +32,16 @@
 #define ATMEL_HLCDC_PWMPS_MAX		0x6
 #define ATMEL_HLCDC_PWMPS(x)		((x) & ATMEL_HLCDC_PWMPS_MASK)
 
+struct atmel_hlcdc_pwm_erratas {
+	bool slow_clk_errata;
+	bool div1_clk_errata;
+};
+
 struct atmel_hlcdc_pwm {
 	struct pwm_chip chip;
 	struct atmel_hlcdc *hlcdc;
 	struct clk *cur_clk;
+	const struct atmel_hlcdc_pwm_erratas *erratas;
 };
 
 static inline struct atmel_hlcdc_pwm *to_atmel_hlcdc_pwm(struct pwm_chip *chip)

@@ -56,20 +62,29 @@ static int atmel_hlcdc_pwm_config(struct pwm_chip *c,
 	u32 pwmcfg;
 	int pres;
 
-	clk_freq = clk_get_rate(new_clk);
-	clk_period_ns = (u64)NSEC_PER_SEC * 256;
-	do_div(clk_period_ns, clk_freq);
+	if (!chip->erratas || !chip->erratas->slow_clk_errata) {
+		clk_freq = clk_get_rate(new_clk);
+		clk_period_ns = (u64)NSEC_PER_SEC * 256;
+		do_div(clk_period_ns, clk_freq);
+	}
 
-	if (clk_period_ns > period_ns) {
+	/* Errata: cannot use slow clk on some IP revisions */
+	if ((chip->erratas && chip->erratas->slow_clk_errata) ||
+	    clk_period_ns > period_ns) {
 		new_clk = hlcdc->sys_clk;
 		clk_freq = clk_get_rate(new_clk);
 		clk_period_ns = (u64)NSEC_PER_SEC * 256;
 		do_div(clk_period_ns, clk_freq);
 	}
 
-	for (pres = 0; pres <= ATMEL_HLCDC_PWMPS_MAX; pres++)
+	for (pres = 0; pres <= ATMEL_HLCDC_PWMPS_MAX; pres++) {
+		/* Errata: cannot divide by 1 on some IP revisions */
+		if (!pres && chip->erratas && chip->erratas->div1_clk_errata)
+			continue;
+
 		if ((clk_period_ns << pres) >= period_ns)
 			break;
+	}
 
 	if (pres > ATMEL_HLCDC_PWMPS_MAX)
 		return -EINVAL;

@@ -187,8 +202,29 @@ static const struct pwm_ops atmel_hlcdc_pwm_ops = {
 	.owner = THIS_MODULE,
 };
 
+const struct atmel_hlcdc_pwm_erratas atmel_hlcdc_pwm_at91sam9x5_erratas = {
+	.slow_clk_errata = true,
+};
+
+const struct atmel_hlcdc_pwm_erratas atmel_hlcdc_pwm_sama5d3_erratas = {
+	.div1_clk_errata = true,
+};
+
+static const struct of_device_id atmel_hlcdc_dt_ids[] = {
+	{
+		.compatible = "atmel,at91sam9x5-hlcdc",
+		.data = &atmel_hlcdc_pwm_at91sam9x5_erratas,
+	},
+	{
+		.compatible = "atmel,sama5d3-hlcdc",
+		.data = &atmel_hlcdc_pwm_sama5d3_erratas,
+	},
+	{ /* sentinel */ },
+};
+
 static int atmel_hlcdc_pwm_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *match;
 	struct device *dev = &pdev->dev;
 	struct atmel_hlcdc_pwm *chip;
 	struct atmel_hlcdc *hlcdc;

@@ -204,6 +240,10 @@ static int atmel_hlcdc_pwm_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	match = of_match_node(atmel_hlcdc_dt_ids, dev->parent->of_node);
+	if (match)
+		chip->erratas = match->data;
+
 	chip->hlcdc = hlcdc;
 	chip->chip.ops = &atmel_hlcdc_pwm_ops;
 	chip->chip.dev = dev;

Signature · 1 line

1.9.1

Thierry Reding

2014-12-04 13:12 UTC #1 lore raw

PATCH v2 1/1 pwm: atmel-hlcdc: add at91sam9x5 and sama5d3 errata handling

Quoted 17 lines · Boris Brezillon

at91sam9x5 has an errata forbidding the use of slow clk as a clk source and sama5d3 SoCs has another errata forbidding the use of div1 prescaler.
Take both of these erratas into account.
Signed-off-by Boris Brezillon <[email protected]>
---
Hi Thierry,
I've addressed the "erratas stored in of_device_id data" part, but still haven't modified the compatible strings of the HLCDC subdevices.
Please let me know if you really want to handle erratas through pwm compatibles instead of parent device compatibles.
Regards,

Quoted 7 lines

Boris
Changes since v1:
- use data field in of_device_id to attach erratas to an IP revision
 drivers/pwm/pwm-atmel-hlcdc.c | 50 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 5 deletions(-)

I applied this, but this was really much more difficult than I would've wanted. Since the MFD driver hasn't been merged into Linus' tree yet I wasn't able to actually build test this driver at all without manually pulling in the patches that add the MFD support. I went through this trouble this time because it's what we had agreed upon, but for the record, next time I'll request a stable branch that I can pull into the PWM tree to resolve this kind of dependency. Or patches will have to wait until the next merge window. I don't want to have to jump through hoops just to make sure the code in my tree actually compiles.

Oh, and it's good that I do compile tests because...

Quoted 1 line

drivers/pwm/pwm-atmel-hlcdc.c +0 -0

[...]

drivers/pwm/pwm-atmel-hlcdc.c static const struct pwm_ops atmel_hlcdc_pwm_ops = { · line 211

Quoted 7 lines

drivers/pwm/pwm-atmel-hlcdc.c +7 -0

+const struct atmel_hlcdc_pwm_erratas atmel_hlcdc_pwm_at91sam9x5_erratas = {
+	.slow_clk_errata = true,
+};
+
+const struct atmel_hlcdc_pwm_erratas atmel_hlcdc_pwm_sama5d3_erratas = {
+	.div1_clk_errata = true,
+};

... these actually should be static.

Also I took the liberty of substituting "erratum" for "errata" and "errata" for "erratas".

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141204/fb2eedf3/attachment-0001.sig>

Boris Brezillon

author

2014-12-04 13:29 UTC #2 lore raw

PATCH v2 1/1 pwm: atmel-hlcdc: add at91sam9x5 and sama5d3 errata handling

Hi Thierry

Quoted 26 lines · On Thu, 4 Dec 2014 14:12:31 +0100 Thierry Reding <[email protected]>…

Quoted 17 lines · Boris Brezillon
at91sam9x5 has an errata forbidding the use of slow clk as a clk source and sama5d3 SoCs has another errata forbidding the use of div1 prescaler.
Take both of these erratas into account.
Signed-off-by Boris Brezillon <[email protected]>
---
Hi Thierry,
I've addressed the "erratas stored in of_device_id data" part, but still haven't modified the compatible strings of the HLCDC subdevices.
Please let me know if you really want to handle erratas through pwm compatibles instead of parent device compatibles.
Regards,
Quoted 7 lines
Boris
Changes since v1:
- use data field in of_device_id to attach erratas to an IP revision
 drivers/pwm/pwm-atmel-hlcdc.c | 50 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 5 deletions(-)

Quoted 9 lines

I applied this, but this was really much more difficult than I would've wanted. Since the MFD driver hasn't been merged into Linus' tree yet I wasn't able to actually build test this driver at all without manually pulling in the patches that add the MFD support. I went through this trouble this time because it's what we had agreed upon, but for the record, next time I'll request a stable branch that I can pull into the PWM tree to resolve this kind of dependency. Or patches will have to wait until the next merge window. I don't want to have to jump through hoops just to make sure the code in my tree actually compiles.

Thanks for doing that.
Just for my understanding, am I the one responsible for providing this stable branch, or were you expecting Lee to provide it ?

Quoted 15 lines

Oh, and it's good that I do compile tests because...
Quoted 1 line
drivers/pwm/pwm-atmel-hlcdc.c +0 -0
[...]
drivers/pwm/pwm-atmel-hlcdc.c static const struct pwm_ops atmel_hlcdc_pwm_ops = { · line 211
Quoted 7 lines
drivers/pwm/pwm-atmel-hlcdc.c +7 -0
+const struct atmel_hlcdc_pwm_erratas atmel_hlcdc_pwm_at91sam9x5_erratas = {
+	.slow_clk_errata = true,
+};
+
+const struct atmel_hlcdc_pwm_erratas atmel_hlcdc_pwm_sama5d3_erratas = {
+	.div1_clk_errata = true,
+};
... these actually should be static.

Quoted 2 lines

Also I took the liberty of substituting "erratum" for "errata" and "errata" for "erratas".

Indeed, thanks for fixing those problems.

Best Regards,

Boris

Signature · 3 lines

Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com