linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 1/4] pwm: jz4740: Use clocks from TCU driver
@ 2020-03-23 14:24 Paul Cercueil
  2020-03-23 14:24 ` [PATCH v4 2/4] pwm: jz4740: Improve algorithm of clock calculation Paul Cercueil
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Paul Cercueil @ 2020-03-23 14:24 UTC (permalink / raw)
  To: Thierry Reding, Uwe Kleine-König
  Cc: od, linux-pwm, linux-kernel, Paul Cercueil, Mathieu Malaterre,
	Artur Rojek

The ingenic-timer "TCU" driver provides us with clocks, that can be
(un)gated, reparented or reclocked from devicetree, instead of having
these settings hardcoded in this driver.

The new code now uses a clk pointer per PWM (instead of a clk per
pwm-chip before). So the pointer is stored in per-pwm data now.
The calls to arch-specific timer code is replaced with standard
clock API calls to start and stop each channel's clock.

While this driver is devicetree-compatible, it is never (as of now)
probed from devicetree, so this change does not introduce a ABI problem
with current devicetree files.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
Tested-by: Artur Rojek <contact@artur-rojek.eu>
---

Notes:
    v2: This patch is now before the patch introducing regmap, so the code
    	has changed a bit.
    v3: - Use %pe printf specifier to print error
    	- Update commit message
    	- Removed call to jz4740_timer_set_ctrl() in jz4740_pwm_free() which
    	  was reseting the clock's parent.
    v4: No change

 drivers/pwm/Kconfig      |  1 +
 drivers/pwm/pwm-jz4740.c | 53 +++++++++++++++++++++++++++++-----------
 2 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 30190beeb6e9..69f00eb9bd36 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -225,6 +225,7 @@ config PWM_IMX_TPM
 config PWM_JZ4740
 	tristate "Ingenic JZ47xx PWM support"
 	depends on MACH_INGENIC
+	depends on COMMON_CLK
 	help
 	  Generic PWM framework driver for Ingenic JZ47xx based
 	  machines.
diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
index 9d78cc21cb12..ee50ac5fabb6 100644
--- a/drivers/pwm/pwm-jz4740.c
+++ b/drivers/pwm/pwm-jz4740.c
@@ -24,7 +24,6 @@
 
 struct jz4740_pwm_chip {
 	struct pwm_chip chip;
-	struct clk *clk;
 };
 
 static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip *chip)
@@ -34,6 +33,11 @@ static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip *chip)
 
 static int jz4740_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 {
+	struct jz4740_pwm_chip *jz = to_jz4740(chip);
+	struct clk *clk;
+	char clk_name[16];
+	int ret;
+
 	/*
 	 * Timers 0 and 1 are used for system tasks, so they are unavailable
 	 * for use as PWMs.
@@ -41,16 +45,32 @@ static int jz4740_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 	if (pwm->hwpwm < 2)
 		return -EBUSY;
 
-	jz4740_timer_start(pwm->hwpwm);
+	snprintf(clk_name, sizeof(clk_name), "timer%u", pwm->hwpwm);
+
+	clk = clk_get(chip->dev, clk_name);
+	if (IS_ERR(clk)) {
+		if (PTR_ERR(clk) != -EPROBE_DEFER)
+			dev_err(chip->dev, "Failed to get clock: %pe", clk);
+		return PTR_ERR(clk);
+	}
+
+	ret = clk_prepare_enable(clk);
+	if (ret) {
+		clk_put(clk);
+		return ret;
+	}
+
+	pwm_set_chip_data(pwm, clk);
 
 	return 0;
 }
 
 static void jz4740_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
 {
-	jz4740_timer_set_ctrl(pwm->hwpwm, 0);
+	struct clk *clk = pwm_get_chip_data(pwm);
 
-	jz4740_timer_stop(pwm->hwpwm);
+	clk_disable_unprepare(clk);
+	clk_put(clk);
 }
 
 static int jz4740_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
@@ -91,17 +111,22 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 			    const struct pwm_state *state)
 {
 	struct jz4740_pwm_chip *jz4740 = to_jz4740(pwm->chip);
+	struct clk *clk = pwm_get_chip_data(pwm),
+		   *parent_clk = clk_get_parent(clk);
+	unsigned long rate, period, duty;
 	unsigned long long tmp;
-	unsigned long period, duty;
 	unsigned int prescaler = 0;
 	uint16_t ctrl;
+	int err;
 
-	tmp = (unsigned long long)clk_get_rate(jz4740->clk) * state->period;
+	rate = clk_get_rate(parent_clk);
+	tmp = (unsigned long long)rate * state->period;
 	do_div(tmp, 1000000000);
 	period = tmp;
 
 	while (period > 0xffff && prescaler < 6) {
 		period >>= 2;
+		rate >>= 2;
 		++prescaler;
 	}
 
@@ -117,14 +142,18 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	jz4740_pwm_disable(chip, pwm);
 
+	err = clk_set_rate(clk, rate);
+	if (err) {
+		dev_err(chip->dev, "Unable to set rate: %d", err);
+		return err;
+	}
+
 	jz4740_timer_set_count(pwm->hwpwm, 0);
 	jz4740_timer_set_duty(pwm->hwpwm, duty);
 	jz4740_timer_set_period(pwm->hwpwm, period);
 
-	ctrl = JZ_TIMER_CTRL_PRESCALER(prescaler) | JZ_TIMER_CTRL_SRC_EXT |
-		JZ_TIMER_CTRL_PWM_ABBRUPT_SHUTDOWN;
-
-	jz4740_timer_set_ctrl(pwm->hwpwm, ctrl);
+	ctrl = jz4740_timer_get_ctrl(pwm->hwpwm);
+	ctrl |= JZ_TIMER_CTRL_PWM_ABBRUPT_SHUTDOWN;
 
 	switch (state->polarity) {
 	case PWM_POLARITY_NORMAL:
@@ -158,10 +187,6 @@ static int jz4740_pwm_probe(struct platform_device *pdev)
 	if (!jz4740)
 		return -ENOMEM;
 
-	jz4740->clk = devm_clk_get(&pdev->dev, "ext");
-	if (IS_ERR(jz4740->clk))
-		return PTR_ERR(jz4740->clk);
-
 	jz4740->chip.dev = &pdev->dev;
 	jz4740->chip.ops = &jz4740_pwm_ops;
 	jz4740->chip.npwm = NUM_PWM;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v4 2/4] pwm: jz4740: Improve algorithm of clock calculation
  2020-03-23 14:24 [PATCH v4 1/4] pwm: jz4740: Use clocks from TCU driver Paul Cercueil
@ 2020-03-23 14:24 ` Paul Cercueil
  2020-03-23 14:24 ` [PATCH v4 3/4] pwm: jz4740: Obtain regmap from parent node Paul Cercueil
  2020-03-23 14:24 ` [PATCH v4 4/4] pwm: jz4740: Allow selection of PWM channels 0 and 1 Paul Cercueil
  2 siblings, 0 replies; 6+ messages in thread
From: Paul Cercueil @ 2020-03-23 14:24 UTC (permalink / raw)
  To: Thierry Reding, Uwe Kleine-König
  Cc: od, linux-pwm, linux-kernel, Paul Cercueil, Mathieu Malaterre,
	Artur Rojek

The previous algorithm hardcoded details about how the TCU clocks work.
The new algorithm will use clk_round_rate to find the perfect clock rate
for the PWM channel.

This code relies on the fact that clk_round_rate() will always round
down, which is not a valid assumption given by the clk API, but only
happens to be true with the clk drivers used for Ingenic SoCs.

Right now, there is no alternative as the clk API does not have a
round-down function (and won't have one for a while), but if it ever
comes to light, a round-down function should be used instead.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
Tested-by: Artur Rojek <contact@artur-rojek.eu>
---

Notes:
    v4: New patch

 drivers/pwm/pwm-jz4740.c | 44 ++++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
index ee50ac5fabb6..0b41baf40e4a 100644
--- a/drivers/pwm/pwm-jz4740.c
+++ b/drivers/pwm/pwm-jz4740.c
@@ -111,28 +111,42 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 			    const struct pwm_state *state)
 {
 	struct jz4740_pwm_chip *jz4740 = to_jz4740(pwm->chip);
-	struct clk *clk = pwm_get_chip_data(pwm),
-		   *parent_clk = clk_get_parent(clk);
-	unsigned long rate, period, duty;
-	unsigned long long tmp;
-	unsigned int prescaler = 0;
+	struct clk *clk = pwm_get_chip_data(pwm);
+	unsigned long long tmp = 0xffffull * NSEC_PER_SEC;
+	unsigned long period, duty;
 	uint16_t ctrl;
+	long rate;
 	int err;
 
-	rate = clk_get_rate(parent_clk);
-	tmp = (unsigned long long)rate * state->period;
-	do_div(tmp, 1000000000);
-	period = tmp;
+	/*
+	 * Limit the clock to a maximum rate that still gives us a period value
+	 * which fits in 16 bits.
+	 */
+	do_div(tmp, state->period);
 
-	while (period > 0xffff && prescaler < 6) {
-		period >>= 2;
-		rate >>= 2;
-		++prescaler;
+	/*
+	 * /!\ IMPORTANT NOTE:
+	 * -------------------
+	 * This code relies on the fact that clk_round_rate() will always round
+	 * down, which is not a valid assumption given by the clk API, but only
+	 * happens to be true with the clk drivers used for Ingenic SoCs.
+	 *
+	 * Right now, there is no alternative as the clk API does not have a
+	 * round-down function (and won't have one for a while), but if it ever
+	 * comes to light, a round-down function should be used instead.
+	 */
+	rate = clk_round_rate(clk, tmp);
+	if (rate < 0) {
+		dev_err(chip->dev, "Unable to round rate: %ld", rate);
+		return rate;
 	}
 
-	if (prescaler == 6)
-		return -EINVAL;
+	/* Calculate period value */
+	tmp = (unsigned long long)rate * state->period;
+	do_div(tmp, NSEC_PER_SEC);
+	period = (unsigned long)tmp;
 
+	/* Calculate duty value */
 	tmp = (unsigned long long)period * state->duty_cycle;
 	do_div(tmp, state->period);
 	duty = period - tmp;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v4 3/4] pwm: jz4740: Obtain regmap from parent node
  2020-03-23 14:24 [PATCH v4 1/4] pwm: jz4740: Use clocks from TCU driver Paul Cercueil
  2020-03-23 14:24 ` [PATCH v4 2/4] pwm: jz4740: Improve algorithm of clock calculation Paul Cercueil
@ 2020-03-23 14:24 ` Paul Cercueil
  2020-03-30 14:37   ` Thierry Reding
  2020-03-23 14:24 ` [PATCH v4 4/4] pwm: jz4740: Allow selection of PWM channels 0 and 1 Paul Cercueil
  2 siblings, 1 reply; 6+ messages in thread
From: Paul Cercueil @ 2020-03-23 14:24 UTC (permalink / raw)
  To: Thierry Reding, Uwe Kleine-König
  Cc: od, linux-pwm, linux-kernel, Paul Cercueil, Mathieu Malaterre,
	Artur Rojek

The TCU registers are shared between a handful of drivers, accessing
them through the same regmap.

While this driver is devicetree-compatible, it is never (as of now)
probed from devicetree, so this change does not introduce a ABI problem
with current devicetree files.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
Tested-by: Artur Rojek <contact@artur-rojek.eu>
---

Notes:
    v2: This patch is now after the patch introducing TCU clocks, so the code
    	changed a bit.
    v3-v4: No change

 drivers/pwm/Kconfig      |  1 +
 drivers/pwm/pwm-jz4740.c | 67 ++++++++++++++++++++++++++--------------
 2 files changed, 44 insertions(+), 24 deletions(-)

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 69f00eb9bd36..82fb4d39ced7 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -226,6 +226,7 @@ config PWM_JZ4740
 	tristate "Ingenic JZ47xx PWM support"
 	depends on MACH_INGENIC
 	depends on COMMON_CLK
+	select MFD_SYSCON
 	help
 	  Generic PWM framework driver for Ingenic JZ47xx based
 	  machines.
diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
index 0b41baf40e4a..df6906c51b8d 100644
--- a/drivers/pwm/pwm-jz4740.c
+++ b/drivers/pwm/pwm-jz4740.c
@@ -13,17 +13,19 @@
 #include <linux/err.h>
 #include <linux/gpio.h>
 #include <linux/kernel.h>
+#include <linux/mfd/ingenic-tcu.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pwm.h>
-
-#include <asm/mach-jz4740/timer.h>
+#include <linux/regmap.h>
 
 #define NUM_PWM 8
 
 struct jz4740_pwm_chip {
 	struct pwm_chip chip;
+	struct regmap *map;
 };
 
 static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip *chip)
@@ -75,36 +77,39 @@ static void jz4740_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
 
 static int jz4740_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
-	uint32_t ctrl = jz4740_timer_get_ctrl(pwm->pwm);
+	struct jz4740_pwm_chip *jz = to_jz4740(chip);
+
+	/* Enable PWM output */
+	regmap_update_bits(jz->map, TCU_REG_TCSRc(pwm->hwpwm),
+			   TCU_TCSR_PWM_EN, TCU_TCSR_PWM_EN);
 
-	ctrl |= JZ_TIMER_CTRL_PWM_ENABLE;
-	jz4740_timer_set_ctrl(pwm->hwpwm, ctrl);
-	jz4740_timer_enable(pwm->hwpwm);
+	/* Start counter */
+	regmap_write(jz->map, TCU_REG_TESR, BIT(pwm->hwpwm));
 
 	return 0;
 }
 
 static void jz4740_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
-	uint32_t ctrl = jz4740_timer_get_ctrl(pwm->hwpwm);
+	struct jz4740_pwm_chip *jz = to_jz4740(chip);
 
 	/*
 	 * Set duty > period. This trick allows the TCU channels in TCU2 mode to
 	 * properly return to their init level.
 	 */
-	jz4740_timer_set_duty(pwm->hwpwm, 0xffff);
-	jz4740_timer_set_period(pwm->hwpwm, 0x0);
+	regmap_write(jz->map, TCU_REG_TDHRc(pwm->hwpwm), 0xffff);
+	regmap_write(jz->map, TCU_REG_TDFRc(pwm->hwpwm), 0x0);
 
 	/*
 	 * Disable PWM output.
 	 * In TCU2 mode (channel 1/2 on JZ4750+), this must be done before the
 	 * counter is stopped, while in TCU1 mode the order does not matter.
 	 */
-	ctrl &= ~JZ_TIMER_CTRL_PWM_ENABLE;
-	jz4740_timer_set_ctrl(pwm->hwpwm, ctrl);
+	regmap_update_bits(jz->map, TCU_REG_TCSRc(pwm->hwpwm),
+			   TCU_TCSR_PWM_EN, 0);
 
 	/* Stop counter */
-	jz4740_timer_disable(pwm->hwpwm);
+	regmap_write(jz->map, TCU_REG_TECR, BIT(pwm->hwpwm));
 }
 
 static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
@@ -114,7 +119,6 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 	struct clk *clk = pwm_get_chip_data(pwm);
 	unsigned long long tmp = 0xffffull * NSEC_PER_SEC;
 	unsigned long period, duty;
-	uint16_t ctrl;
 	long rate;
 	int err;
 
@@ -162,24 +166,32 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
 		return err;
 	}
 
-	jz4740_timer_set_count(pwm->hwpwm, 0);
-	jz4740_timer_set_duty(pwm->hwpwm, duty);
-	jz4740_timer_set_period(pwm->hwpwm, period);
+	/* Reset counter to 0 */
+	regmap_write(jz4740->map, TCU_REG_TCNTc(pwm->hwpwm), 0);
+
+	/* Set duty */
+	regmap_write(jz4740->map, TCU_REG_TDHRc(pwm->hwpwm), duty);
 
-	ctrl = jz4740_timer_get_ctrl(pwm->hwpwm);
-	ctrl |= JZ_TIMER_CTRL_PWM_ABBRUPT_SHUTDOWN;
+	/* Set period */
+	regmap_write(jz4740->map, TCU_REG_TDFRc(pwm->hwpwm), period);
 
+	/* Set abrupt shutdown */
+	regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm),
+			   TCU_TCSR_PWM_SD, TCU_TCSR_PWM_SD);
+
+	/* Set polarity */
 	switch (state->polarity) {
 	case PWM_POLARITY_NORMAL:
-		ctrl &= ~JZ_TIMER_CTRL_PWM_ACTIVE_LOW;
+		regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm),
+				   TCU_TCSR_PWM_INITL_HIGH, 0);
 		break;
 	case PWM_POLARITY_INVERSED:
-		ctrl |= JZ_TIMER_CTRL_PWM_ACTIVE_LOW;
+		regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm),
+				   TCU_TCSR_PWM_INITL_HIGH,
+				   TCU_TCSR_PWM_INITL_HIGH);
 		break;
 	}
 
-	jz4740_timer_set_ctrl(pwm->hwpwm, ctrl);
-
 	if (state->enabled)
 		jz4740_pwm_enable(chip, pwm);
 
@@ -196,12 +208,19 @@ static const struct pwm_ops jz4740_pwm_ops = {
 static int jz4740_pwm_probe(struct platform_device *pdev)
 {
 	struct jz4740_pwm_chip *jz4740;
+	struct device *dev = &pdev->dev;
 
-	jz4740 = devm_kzalloc(&pdev->dev, sizeof(*jz4740), GFP_KERNEL);
+	jz4740 = devm_kzalloc(dev, sizeof(*jz4740), GFP_KERNEL);
 	if (!jz4740)
 		return -ENOMEM;
 
-	jz4740->chip.dev = &pdev->dev;
+	jz4740->map = device_node_to_regmap(dev->parent->of_node);
+	if (!jz4740->map) {
+		dev_err(dev, "regmap not found\n");
+		return -EINVAL;
+	}
+
+	jz4740->chip.dev = dev;
 	jz4740->chip.ops = &jz4740_pwm_ops;
 	jz4740->chip.npwm = NUM_PWM;
 	jz4740->chip.base = -1;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v4 4/4] pwm: jz4740: Allow selection of PWM channels 0 and 1
  2020-03-23 14:24 [PATCH v4 1/4] pwm: jz4740: Use clocks from TCU driver Paul Cercueil
  2020-03-23 14:24 ` [PATCH v4 2/4] pwm: jz4740: Improve algorithm of clock calculation Paul Cercueil
  2020-03-23 14:24 ` [PATCH v4 3/4] pwm: jz4740: Obtain regmap from parent node Paul Cercueil
@ 2020-03-23 14:24 ` Paul Cercueil
  2 siblings, 0 replies; 6+ messages in thread
From: Paul Cercueil @ 2020-03-23 14:24 UTC (permalink / raw)
  To: Thierry Reding, Uwe Kleine-König
  Cc: od, linux-pwm, linux-kernel, Paul Cercueil, Mathieu Malaterre,
	Artur Rojek

The TCU channels 0 and 1 were previously reserved for system tasks, and
thus unavailable for PWM.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Tested-by: Mathieu Malaterre <malat@debian.org>
Tested-by: Artur Rojek <contact@artur-rojek.eu>
---

Notes:
    v2-v4: No change

 drivers/pwm/pwm-jz4740.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
index df6906c51b8d..c12c43eb2506 100644
--- a/drivers/pwm/pwm-jz4740.c
+++ b/drivers/pwm/pwm-jz4740.c
@@ -33,6 +33,19 @@ static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip *chip)
 	return container_of(chip, struct jz4740_pwm_chip, chip);
 }
 
+static bool jz4740_pwm_can_use_chn(struct jz4740_pwm_chip *jz,
+				   unsigned int channel)
+{
+	/* Enable all TCU channels for PWM use by default except channels 0/1 */
+	u32 pwm_channels_mask = GENMASK(NUM_PWM - 1, 2);
+
+	device_property_read_u32(jz->chip.dev->parent,
+				 "ingenic,pwm-channels-mask",
+				 &pwm_channels_mask);
+
+	return !!(pwm_channels_mask & BIT(channel));
+}
+
 static int jz4740_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 {
 	struct jz4740_pwm_chip *jz = to_jz4740(chip);
@@ -40,11 +53,7 @@ static int jz4740_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 	char clk_name[16];
 	int ret;
 
-	/*
-	 * Timers 0 and 1 are used for system tasks, so they are unavailable
-	 * for use as PWMs.
-	 */
-	if (pwm->hwpwm < 2)
+	if (!jz4740_pwm_can_use_chn(jz, pwm->hwpwm))
 		return -EBUSY;
 
 	snprintf(clk_name, sizeof(clk_name), "timer%u", pwm->hwpwm);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 3/4] pwm: jz4740: Obtain regmap from parent node
  2020-03-23 14:24 ` [PATCH v4 3/4] pwm: jz4740: Obtain regmap from parent node Paul Cercueil
@ 2020-03-30 14:37   ` Thierry Reding
  2020-03-30 16:41     ` Paul Cercueil
  0 siblings, 1 reply; 6+ messages in thread
From: Thierry Reding @ 2020-03-30 14:37 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Uwe Kleine-König, od, linux-pwm, linux-kernel,
	Mathieu Malaterre, Artur Rojek

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

On Mon, Mar 23, 2020 at 03:24:20PM +0100, Paul Cercueil wrote:
[...]
> diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
[...]
> @@ -196,12 +208,19 @@ static const struct pwm_ops jz4740_pwm_ops = {
>  static int jz4740_pwm_probe(struct platform_device *pdev)
>  {
>  	struct jz4740_pwm_chip *jz4740;
> +	struct device *dev = &pdev->dev;
>  
> -	jz4740 = devm_kzalloc(&pdev->dev, sizeof(*jz4740), GFP_KERNEL);
> +	jz4740 = devm_kzalloc(dev, sizeof(*jz4740), GFP_KERNEL);
>  	if (!jz4740)
>  		return -ENOMEM;
>  
> -	jz4740->chip.dev = &pdev->dev;
> +	jz4740->map = device_node_to_regmap(dev->parent->of_node);
> +	if (!jz4740->map) {

This seems wrong. According to the code, device_node_to_regmap() returns
an ERR_PTR()-encoded error code on failure, so I think this should be:

	if (IS_ERR(jz4740->map)) {
		...
		return PTR_ERR(jz4740->map);
	}

No need to resend for that, I can take care of that when applying. Let
me know if that doesn't work.

Thierry

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 3/4] pwm: jz4740: Obtain regmap from parent node
  2020-03-30 14:37   ` Thierry Reding
@ 2020-03-30 16:41     ` Paul Cercueil
  0 siblings, 0 replies; 6+ messages in thread
From: Paul Cercueil @ 2020-03-30 16:41 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Uwe Kleine-König, od, linux-pwm, linux-kernel,
	Mathieu Malaterre, Artur Rojek

Hi Thierry,

Le lun. 30 mars 2020 à 16:37, Thierry Reding 
<thierry.reding@gmail.com> a écrit :
> On Mon, Mar 23, 2020 at 03:24:20PM +0100, Paul Cercueil wrote:
> [...]
>>  diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
> [...]
>>  @@ -196,12 +208,19 @@ static const struct pwm_ops jz4740_pwm_ops = {
>>   static int jz4740_pwm_probe(struct platform_device *pdev)
>>   {
>>   	struct jz4740_pwm_chip *jz4740;
>>  +	struct device *dev = &pdev->dev;
>> 
>>  -	jz4740 = devm_kzalloc(&pdev->dev, sizeof(*jz4740), GFP_KERNEL);
>>  +	jz4740 = devm_kzalloc(dev, sizeof(*jz4740), GFP_KERNEL);
>>   	if (!jz4740)
>>   		return -ENOMEM;
>> 
>>  -	jz4740->chip.dev = &pdev->dev;
>>  +	jz4740->map = device_node_to_regmap(dev->parent->of_node);
>>  +	if (!jz4740->map) {
> 
> This seems wrong. According to the code, device_node_to_regmap() 
> returns
> an ERR_PTR()-encoded error code on failure, so I think this should be:
> 
> 	if (IS_ERR(jz4740->map)) {
> 		...
> 		return PTR_ERR(jz4740->map);
> 	}
> 
> No need to resend for that, I can take care of that when applying. Let
> me know if that doesn't work.

Yes, that works for me. Good catch.

Thanks,
-Paul

> 
> Thierry



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-03-30 16:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-23 14:24 [PATCH v4 1/4] pwm: jz4740: Use clocks from TCU driver Paul Cercueil
2020-03-23 14:24 ` [PATCH v4 2/4] pwm: jz4740: Improve algorithm of clock calculation Paul Cercueil
2020-03-23 14:24 ` [PATCH v4 3/4] pwm: jz4740: Obtain regmap from parent node Paul Cercueil
2020-03-30 14:37   ` Thierry Reding
2020-03-30 16:41     ` Paul Cercueil
2020-03-23 14:24 ` [PATCH v4 4/4] pwm: jz4740: Allow selection of PWM channels 0 and 1 Paul Cercueil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).