On Sat, Feb 06, 2021 at 09:27:30PM +0800, 班涛 wrote: > Maxime Ripard 于2021年2月6日周六 上午12:12写道： > > > Hi, > > > > On Thu, Feb 04, 2021 at 11:47:34AM +0800, 班涛 wrote: > > > Maxime Ripard 于2021年2月3日周三 下午11:46写道： > > > > > > > Hi, > > > > > > > > On Wed, Feb 03, 2021 at 08:53:17PM +0800, Ban Tao wrote: > > > > > From: Ban Tao > > > > > > > > > > The Allwinner R818, A133, R329, V536 and V833 has a new PWM > > controller > > > > > IP compared to the older Allwinner SoCs. > > > > > > > > > > Signed-off-by: Ban Tao > > > > > > > > Thanks for your patch. There's a bunch of warnings reported by > > > > checkpatch --strict, they should be addressed. > > > > > > > > > --- > > > > > v1->v2: > > > > > 1.delete unnecessary code. > > > > > 2.using a named define for some constants. > > > > > 3.Add comment in sun50i_pwm_config function. > > > > > 4.using dev_err_probe() for error handling. > > > > > 5.disable the clock after pwmchip_remove(). > > > > > --- > > > > > MAINTAINERS | 6 + > > > > > drivers/pwm/Kconfig | 11 ++ > > > > > drivers/pwm/Makefile | 1 + > > > > > drivers/pwm/pwm-sun50i.c | 348 > > +++++++++++++++++++++++++++++++++++++++ > > > > > 4 files changed, 366 insertions(+) > > > > > create mode 100644 drivers/pwm/pwm-sun50i.c > > > > > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > > > index e73636b75f29..d33cf1b69b43 100644 > > > > > --- a/MAINTAINERS > > > > > +++ b/MAINTAINERS > > > > > @@ -737,6 +737,12 @@ L: linux-media@vger.kernel.org > > > > > S: Maintained > > > > > F: drivers/staging/media/sunxi/cedrus/ > > > > > > > > > > +ALLWINNER PWM DRIVER > > > > > +M: Ban Tao > > > > > +L: linux-pwm@vger.kernel.org > > > > > +S: Maintained > > > > > +F: drivers/pwm/pwm-sun50i.c > > > > > + > > > > > ALPHA PORT > > > > > M: Richard Henderson > > > > > M: Ivan Kokshaysky > > > > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > > > > > index 9a4f66ae8070..17635a8f2ed3 100644 > > > > > --- a/drivers/pwm/Kconfig > > > > > +++ b/drivers/pwm/Kconfig > > > > > @@ -552,6 +552,17 @@ config PWM_SUN4I > > > > > To compile this driver as a module, choose M here: the module > > > > > will be called pwm-sun4i. > > > > > > > > > > +config PWM_SUN50I > > > > > + tristate "Allwinner enhanced PWM support" > > > > > + depends on ARCH_SUNXI || COMPILE_TEST > > > > > + depends on HAS_IOMEM && COMMON_CLK > > > > > + help > > > > > + Enhanced PWM framework driver for Allwinner R818, A133, R329, > > > > > + V536 and V833 SoCs. > > > > > + > > > > > + To compile this driver as a module, choose M here: the module > > > > > + will be called pwm-sun50i. > > > > > + > > > > > > > > Even though it's unfortunate, there's a bunch of other SoCs part of the > > > > sun50i family that are supported by the sun4i driver. > > > > > > > > Which SoC introduced that new design? It's usually the name we pick up > > > > then. > > > > > > > > > > In fact, some SoCs has not been supported by the sun4i driver, such as > > v833, > > > v536, r818, a133 and r329. > > > v833 and v536 are part of the sun8i family, but r818, a133 and r329 are > > > part of the sun50i family. > > > > Indeed, I missed that sorry. > > > > > So,I'm confused, how do choose the name of this driver? > > > > Just go for the earliest SoC that introduced that design. What was the > > first SoC to use it? > > > > The V536 SOC first used this design, so, we should use the name > "sun8i-pwm.c"? sun8i-pwm would be too generic, but sun8i-v536-pwm would be great then > > > > > > +static const struct sun50i_pwm_data sun8i_pwm_data_c9 = { > > > > > + .npwm = 9, > > > > > +}; > > > > > + > > > > > +static const struct sun50i_pwm_data sun50i_pwm_data_c16 = { > > > > > + .npwm = 16, > > > > > +}; > > > > > + > > > > > +static const struct of_device_id sun50i_pwm_dt_ids[] = { > > > > > + { > > > > > + .compatible = "allwinner,sun8i-v833-pwm", > > > > > + .data = &sun8i_pwm_data_c9, > > > > > + }, { > > > > > + .compatible = "allwinner,sun8i-v536-pwm", > > > > > + .data = &sun8i_pwm_data_c9, > > > > > + }, { > > > > > + .compatible = "allwinner,sun50i-r818-pwm", > > > > > + .data = &sun50i_pwm_data_c16, > > > > > + }, { > > > > > + .compatible = "allwinner,sun50i-a133-pwm", > > > > > + .data = &sun50i_pwm_data_c16, > > > > > + }, { > > > > > + .compatible = "allwinner,sun50i-r329-pwm", > > > > > + .data = &sun8i_pwm_data_c9, > > > > > + }, { > > > > > + /* sentinel */ > > > > > + }, > > > > > +}; > > > > > +MODULE_DEVICE_TABLE(of, sun50i_pwm_dt_ids); > > > > > > > > What are the differences between all these SoCs? If there's none > > between > > > > the v833, v536 and R329, and between the r818 and the A133, you should > > > > use the same compatible. > > > > > > Compared with the sun4i driver, this patch is a completely different PWM > > IP > > > controller. > > > > Sure, I didn't mean to compare it to sun4i. What I meant was that as far > > as these struct goes, the A133 and the R818 look to have the same PWM > > controller. Just like the v833, v536 and R329. > > > > If the PWM controllers are indeed identical across those SoCs, you can > > just use two compatibles, one for the PWM with 9 channels (again, the > > earliest SoC among the V833, v536 and r329), and one for the PWM with 16 > > channels. > > > Ok i see what you mean. > static const struct of_device_id sun50i_pwm_dt_ids[] = { > { > .compatible = "allwinner,sun8i-v536-pwm", > .data = &sun8i_pwm_data_c9, > }, { > .compatible = "allwinner,sun50i-r818-pwm", > .data = &sun50i_pwm_data_c16, > }, { > /* sentinel */ > }, > }; > is this OK? Just write two SoC for the " compatible "？ Yes, this is perfect :) Maxime