| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20210210093333.twjnglabtcbecoku@gilmour>
Date: Wed, 10 Feb 2021 10:33:33 +0100
From: Maxime Ripard <maxime@...no.tech>
To: 班涛 <fengzheng923@...il.com>
Cc: thierry.reding@...il.com,
Uwe Kleine-König
<u.kleine-koenig@...gutronix.de>, wens@...e.org,
linux-kernel@...r.kernel.org, linux-pwm@...r.kernel.org
Subject: Re: [PATCH v2] pwm: sunxi: Add Allwinner SoC PWM controller driver
On Sat, Feb 06, 2021 at 09:27:30PM +0800, 班涛 wrote:
> Maxime Ripard <maxime@...no.tech> 于2021年2月6日周六 上午12:12写道：
>
> > Hi,
> >
> > On Thu, Feb 04, 2021 at 11:47:34AM +0800, 班涛 wrote:
> > > Maxime Ripard <maxime@...no.tech> 于2021年2月3日周三 下午11:46写道：
> > >
> > > > Hi,
> > > >
> > > > On Wed, Feb 03, 2021 at 08:53:17PM +0800, Ban Tao wrote:
> > > > > From: Ban Tao <fengzheng923@...il.com>
> > > > >
> > > > > 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 <fengzheng923@...il.com>
> > > >
> > > > 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@...r.kernel.org
> > > > > S: Maintained
> > > > > F: drivers/staging/media/sunxi/cedrus/
> > > > >
> > > > > +ALLWINNER PWM DRIVER
> > > > > +M: Ban Tao <fengzheng923@...il.com>
> > > > > +L: linux-pwm@...r.kernel.org
> > > > > +S: Maintained
> > > > > +F: drivers/pwm/pwm-sun50i.c
> > > > > +
> > > > > ALPHA PORT
> > > > > M: Richard Henderson <rth@...ddle.net>
> > > > > M: Ivan Kokshaysky <ink@...assic.park.msu.ru>
> > > > > 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
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists