Re: [PATCH 1/7] pwm: jz4740: Obtain regmap from parent node
From: Paul Cercueil
Date: Fri Aug 09 2019 - 13:05:06 EST
Hi Uwe,
Le ven. 9 août 2019 à 18:51, Uwe =?iso-8859-1?q?Kleine-K=F6nig?=
<u.kleine-koenig@xxxxxxxxxxxxxx> a écrit :
On Fri, Aug 09, 2019 at 02:30:25PM +0200, Paul Cercueil wrote:
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.
If you adapt the binding also update the binding doc please.
It's already updated, in mips-next, so it'll be in the next -rc1.
Signed-off-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
Tested-by: Mathieu Malaterre <malat@xxxxxxxxxx>
Tested-by: Artur Rojek <contact@xxxxxxxxxxxxxx>
nitpick: put your S-o-b line after the other tags you added.
---
drivers/pwm/Kconfig | 1 +
drivers/pwm/pwm-jz4740.c | 80
++++++++++++++++++++++++++--------------
2 files changed, 53 insertions(+), 28 deletions(-)
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index a7e57516959e..cc4df0ec978a 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
+ 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 f901e8a0d33d..7aea5e0c6e18 100644
--- a/drivers/pwm/pwm-jz4740.c
+++ b/drivers/pwm/pwm-jz4740.c
@@ -8,18 +8,20 @@
#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 clk *clk;
+ struct regmap *map;
};
static inline struct jz4740_pwm_chip *to_jz4740(struct pwm_chip
*chip)
@@ -29,6 +31,8 @@ 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);
+
/*
* Timers 0 and 1 are used for system tasks, so they are
unavailable
* for use as PWMs.
@@ -36,50 +40,53 @@ static int jz4740_pwm_request(struct pwm_chip
*chip, struct pwm_device *pwm)
if (pwm->hwpwm < 2)
return -EBUSY;
- jz4740_timer_start(pwm->hwpwm);
+ regmap_write(jz->map, TCU_REG_TSCR, BIT(pwm->hwpwm));
jz4740_timer_start does
writel(BIT(timer), jz4740_timer_base + JZ_REG_TIMER_STOP_CLEAR);
with
#define JZ_REG_TIMER_STOP_CLEAR 0x2C
and
#define TCU_REG_TSCR 0x3c
I wonder why the offsets are different.
The offset are different because the base is different.
return 0;
}
static void jz4740_pwm_free(struct pwm_chip *chip, struct
pwm_device *pwm)
{
- jz4740_timer_set_ctrl(pwm->hwpwm, 0);
+ struct jz4740_pwm_chip *jz = to_jz4740(chip);
- jz4740_timer_stop(pwm->hwpwm);
+ regmap_write(jz->map, TCU_REG_TSCR, BIT(pwm->hwpwm));
jz4740_timer_set_ctrl writes to offset (((pwm->hwpwm) * 0x10) + 0x3C)
and jz4740_timer_stop to offset 0x1c. The regmap_write doesn't do both
of them but instead writes to offset 0x3c.
I guess it should have been TCU_REG_TSSR ("Timer Stop Set Register") and
I didn't notice because the next patch drops this write anyway.
I'll do as you suggested in your other reply and swap the two patches if
it makes things easier, it'll get rid of this write.
So this doesn't seem to be a 1:1 conversion. This either needs fixing,
splitting into several patches or a better commit log.
Stopping my review here.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König
|
Industrial Linux Solutions |
http://www.pengutronix.de/ |