* [PATCH 1/3] pwm: jz4740: Drop dependency on MACH_INGENIC
@ 2020-04-13 12:14 Paul Cercueil
2020-04-13 12:14 ` [PATCH 2/3] pwm: jz4740: Make PWM start with the active part Paul Cercueil
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Paul Cercueil @ 2020-04-13 12:14 UTC (permalink / raw)
To: Thierry Reding, Uwe Kleine-König
Cc: od, linux-pwm, linux-kernel, Paul Cercueil
Depending on MACH_INGENIC prevent us from creating a generic kernel that
works on more than one MIPS board. Instead, we just depend on MIPS being
set.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
drivers/pwm/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index eebbc917ac97..7814e5b2cad7 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -234,7 +234,7 @@ config PWM_IMX_TPM
config PWM_JZ4740
tristate "Ingenic JZ47xx PWM support"
- depends on MACH_INGENIC
+ depends on MIPS
depends on COMMON_CLK
select MFD_SYSCON
help
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] pwm: jz4740: Make PWM start with the active part
2020-04-13 12:14 [PATCH 1/3] pwm: jz4740: Drop dependency on MACH_INGENIC Paul Cercueil
@ 2020-04-13 12:14 ` Paul Cercueil
2020-05-24 17:35 ` Uwe Kleine-König
2020-04-13 12:14 ` [PATCH 3/3] pwm: jz4740: Add support for the JZ4725B Paul Cercueil
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Paul Cercueil @ 2020-04-13 12:14 UTC (permalink / raw)
To: Thierry Reding, Uwe Kleine-König
Cc: od, linux-pwm, linux-kernel, Paul Cercueil
The PWM would previously always start with the inactive part.
To counter that, the common trick is to use an inverted duty as the
real duty (as in, 'period - duty'), and invert the polarity when the
PWM is enabled.
However, for some reason the driver was already configuring the hardware
for an inverted duty, so inverting it again means we do configure the
hardware with the actual duty value.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
drivers/pwm/pwm-jz4740.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
index 3cd5c054ad9a..f566f9d248d6 100644
--- a/drivers/pwm/pwm-jz4740.c
+++ b/drivers/pwm/pwm-jz4740.c
@@ -6,7 +6,6 @@
* Limitations:
* - The .apply callback doesn't complete the currently running period before
* reconfiguring the hardware.
- * - Each period starts with the inactive part.
*/
#include <linux/clk.h>
@@ -163,7 +162,7 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
/* Calculate duty value */
tmp = (unsigned long long)period * state->duty_cycle;
do_div(tmp, state->period);
- duty = period - tmp;
+ duty = (unsigned long)tmp;
if (duty >= period)
duty = period - 1;
@@ -190,17 +189,13 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
TCU_TCSR_PWM_SD, TCU_TCSR_PWM_SD);
/* Set polarity */
- switch (state->polarity) {
- case PWM_POLARITY_NORMAL:
+ if ((state->polarity != PWM_POLARITY_INVERSED) ^ state->enabled)
regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm),
TCU_TCSR_PWM_INITL_HIGH, 0);
- break;
- case PWM_POLARITY_INVERSED:
+ else
regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm),
TCU_TCSR_PWM_INITL_HIGH,
TCU_TCSR_PWM_INITL_HIGH);
- break;
- }
if (state->enabled)
jz4740_pwm_enable(chip, pwm);
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] pwm: jz4740: Add support for the JZ4725B
2020-04-13 12:14 [PATCH 1/3] pwm: jz4740: Drop dependency on MACH_INGENIC Paul Cercueil
2020-04-13 12:14 ` [PATCH 2/3] pwm: jz4740: Make PWM start with the active part Paul Cercueil
@ 2020-04-13 12:14 ` Paul Cercueil
2020-05-24 17:37 ` Uwe Kleine-König
2020-05-24 16:16 ` [PATCH 1/3] pwm: jz4740: Drop dependency on MACH_INGENIC Paul Cercueil
2020-05-24 17:03 ` Uwe Kleine-König
3 siblings, 1 reply; 9+ messages in thread
From: Paul Cercueil @ 2020-04-13 12:14 UTC (permalink / raw)
To: Thierry Reding, Uwe Kleine-König
Cc: od, linux-pwm, linux-kernel, Paul Cercueil
The PWM hardware in the JZ4725B works the same as in the JZ4740, but has
only six channels available.
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
Notes:
I did not add documentation for the new jz4725b-pwm compatible
string on purpose. The reason is that the documentation file
for the Timer/Counter Unit (TCU) of Ingenic SoCs will be
completely rewritten from .txt to YAML in a separate patchset.
drivers/pwm/pwm-jz4740.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
index f566f9d248d6..bb27934fb6c2 100644
--- a/drivers/pwm/pwm-jz4740.c
+++ b/drivers/pwm/pwm-jz4740.c
@@ -22,6 +22,10 @@
#define NUM_PWM 8
+struct soc_info {
+ unsigned int num_pwms;
+};
+
struct jz4740_pwm_chip {
struct pwm_chip chip;
struct regmap *map;
@@ -36,7 +40,7 @@ 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);
+ u32 pwm_channels_mask = GENMASK(jz->chip.npwm - 1, 2);
device_property_read_u32(jz->chip.dev->parent,
"ingenic,pwm-channels-mask",
@@ -214,6 +218,7 @@ static int jz4740_pwm_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct jz4740_pwm_chip *jz4740;
+ const struct soc_info *info = device_get_match_data(dev);
jz4740 = devm_kzalloc(dev, sizeof(*jz4740), GFP_KERNEL);
if (!jz4740)
@@ -227,8 +232,8 @@ static int jz4740_pwm_probe(struct platform_device *pdev)
jz4740->chip.dev = dev;
jz4740->chip.ops = &jz4740_pwm_ops;
- jz4740->chip.npwm = NUM_PWM;
jz4740->chip.base = -1;
+ jz4740->chip.npwm = info ? info->num_pwms : NUM_PWM;
jz4740->chip.of_xlate = of_pwm_xlate_with_flags;
jz4740->chip.of_pwm_n_cells = 3;
@@ -244,9 +249,18 @@ static int jz4740_pwm_remove(struct platform_device *pdev)
return pwmchip_remove(&jz4740->chip);
}
+static const struct soc_info __maybe_unused jz4740_soc_info = {
+ .num_pwms = 8,
+};
+
+static const struct soc_info __maybe_unused jz4725b_soc_info = {
+ .num_pwms = 6,
+};
+
#ifdef CONFIG_OF
static const struct of_device_id jz4740_pwm_dt_ids[] = {
- { .compatible = "ingenic,jz4740-pwm", },
+ { .compatible = "ingenic,jz4740-pwm", .data = &jz4740_soc_info },
+ { .compatible = "ingenic,jz4725b-pwm", .data = &jz4725b_soc_info },
{},
};
MODULE_DEVICE_TABLE(of, jz4740_pwm_dt_ids);
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] pwm: jz4740: Drop dependency on MACH_INGENIC
2020-04-13 12:14 [PATCH 1/3] pwm: jz4740: Drop dependency on MACH_INGENIC Paul Cercueil
2020-04-13 12:14 ` [PATCH 2/3] pwm: jz4740: Make PWM start with the active part Paul Cercueil
2020-04-13 12:14 ` [PATCH 3/3] pwm: jz4740: Add support for the JZ4725B Paul Cercueil
@ 2020-05-24 16:16 ` Paul Cercueil
2020-05-24 17:03 ` Uwe Kleine-König
3 siblings, 0 replies; 9+ messages in thread
From: Paul Cercueil @ 2020-05-24 16:16 UTC (permalink / raw)
To: Thierry Reding, Uwe Kleine-König; +Cc: od, linux-pwm, linux-kernel
Hi,
Any feedback on this patchset?
Cheers,
-Paul
Le lun. 13 avril 2020 à 14:14, Paul Cercueil <paul@crapouillou.net> a
écrit :
> Depending on MACH_INGENIC prevent us from creating a generic kernel
> that
> works on more than one MIPS board. Instead, we just depend on MIPS
> being
> set.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> drivers/pwm/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index eebbc917ac97..7814e5b2cad7 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -234,7 +234,7 @@ config PWM_IMX_TPM
>
> config PWM_JZ4740
> tristate "Ingenic JZ47xx PWM support"
> - depends on MACH_INGENIC
> + depends on MIPS
> depends on COMMON_CLK
> select MFD_SYSCON
> help
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] pwm: jz4740: Drop dependency on MACH_INGENIC
2020-04-13 12:14 [PATCH 1/3] pwm: jz4740: Drop dependency on MACH_INGENIC Paul Cercueil
` (2 preceding siblings ...)
2020-05-24 16:16 ` [PATCH 1/3] pwm: jz4740: Drop dependency on MACH_INGENIC Paul Cercueil
@ 2020-05-24 17:03 ` Uwe Kleine-König
3 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2020-05-24 17:03 UTC (permalink / raw)
To: Paul Cercueil; +Cc: Thierry Reding, od, linux-pwm, linux-kernel
On Mon, Apr 13, 2020 at 02:14:43PM +0200, Paul Cercueil wrote:
> Depending on MACH_INGENIC prevent us from creating a generic kernel that
> works on more than one MIPS board. Instead, we just depend on MIPS being
> set.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> drivers/pwm/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index eebbc917ac97..7814e5b2cad7 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -234,7 +234,7 @@ config PWM_IMX_TPM
>
> config PWM_JZ4740
> tristate "Ingenic JZ47xx PWM support"
> - depends on MACH_INGENIC
> + depends on MIPS
Looks good to me.
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] pwm: jz4740: Make PWM start with the active part
2020-04-13 12:14 ` [PATCH 2/3] pwm: jz4740: Make PWM start with the active part Paul Cercueil
@ 2020-05-24 17:35 ` Uwe Kleine-König
0 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2020-05-24 17:35 UTC (permalink / raw)
To: Paul Cercueil; +Cc: Thierry Reding, od, linux-pwm, linux-kernel
On Mon, Apr 13, 2020 at 02:14:44PM +0200, Paul Cercueil wrote:
> The PWM would previously always start with the inactive part.
> To counter that, the common trick is to use an inverted duty as the
> real duty (as in, 'period - duty'), and invert the polarity when the
> PWM is enabled.
>
> However, for some reason the driver was already configuring the hardware
> for an inverted duty, so inverting it again means we do configure the
> hardware with the actual duty value.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
> drivers/pwm/pwm-jz4740.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
> index 3cd5c054ad9a..f566f9d248d6 100644
> --- a/drivers/pwm/pwm-jz4740.c
> +++ b/drivers/pwm/pwm-jz4740.c
> @@ -6,7 +6,6 @@
> * Limitations:
> * - The .apply callback doesn't complete the currently running period before
> * reconfiguring the hardware.
> - * - Each period starts with the inactive part.
> */
>
> #include <linux/clk.h>
> @@ -163,7 +162,7 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
Side note: in the calculation above the driver is loosing precision as
the calculation for the duty counter uses the (already rounded) stuff
for the period counter.
The currently used calculation is (somewhat simplified syntax):
period = (rate * requested_period) // NSEC_PER_SEC;
duty = (period * requested_duty) // requested_period
The output then is an actual period of
period * NSEC_PER_SEC / rate
and the duty cycle is
duty * NSEC_PER_SEC / rate
.
Consider the following:
rate = 23856100
requested_period = 20959
requested_duty = 16000
With the currently implemented algorithm we get:
period = 499
duty = 380
yielding a real period of 20917.08 ns (ok) and a duty cycle of 15928.84
ns. With duty = 381 we'd get 15970.758 ns which is better.
To reach that we'd need:
/* Calculate duty value */
tmp = (unsigned long long)rate * state->duty_cycle;
do_div(tmp, NSEC_PER_SEC);
duty = tmp
> /* Calculate duty value */
> tmp = (unsigned long long)period * state->duty_cycle;
> do_div(tmp, state->period);
> - duty = period - tmp;
> + duty = (unsigned long)tmp;
That cast is unnecessary.
>
> if (duty >= period)
> duty = period - 1;
> @@ -190,17 +189,13 @@ static int jz4740_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> TCU_TCSR_PWM_SD, TCU_TCSR_PWM_SD);
>
> /* Set polarity */
> - switch (state->polarity) {
> - case PWM_POLARITY_NORMAL:
> + if ((state->polarity != PWM_POLARITY_INVERSED) ^ state->enabled)
This needs to be more complicated than before as a disabled PWM needs
the other setting to yield its inactive level?
Maybe worth a comment?
> regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm),
> TCU_TCSR_PWM_INITL_HIGH, 0);
> - break;
> - case PWM_POLARITY_INVERSED:
> + else
> regmap_update_bits(jz4740->map, TCU_REG_TCSRc(pwm->hwpwm),
> TCU_TCSR_PWM_INITL_HIGH,
> TCU_TCSR_PWM_INITL_HIGH);
> - break;
> - }
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] pwm: jz4740: Add support for the JZ4725B
2020-04-13 12:14 ` [PATCH 3/3] pwm: jz4740: Add support for the JZ4725B Paul Cercueil
@ 2020-05-24 17:37 ` Uwe Kleine-König
2020-05-24 17:51 ` Paul Cercueil
0 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2020-05-24 17:37 UTC (permalink / raw)
To: Paul Cercueil; +Cc: Thierry Reding, od, linux-pwm, linux-kernel
On Mon, Apr 13, 2020 at 02:14:45PM +0200, Paul Cercueil wrote:
> The PWM hardware in the JZ4725B works the same as in the JZ4740, but has
> only six channels available.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>
> Notes:
> I did not add documentation for the new jz4725b-pwm compatible
> string on purpose. The reason is that the documentation file
> for the Timer/Counter Unit (TCU) of Ingenic SoCs will be
> completely rewritten from .txt to YAML in a separate patchset.
>
> drivers/pwm/pwm-jz4740.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
> index f566f9d248d6..bb27934fb6c2 100644
> --- a/drivers/pwm/pwm-jz4740.c
> +++ b/drivers/pwm/pwm-jz4740.c
> @@ -22,6 +22,10 @@
>
> #define NUM_PWM 8
>
> +struct soc_info {
> + unsigned int num_pwms;
> +};
> +
> struct jz4740_pwm_chip {
> struct pwm_chip chip;
> struct regmap *map;
> @@ -36,7 +40,7 @@ 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);
> + u32 pwm_channels_mask = GENMASK(jz->chip.npwm - 1, 2);
>
> device_property_read_u32(jz->chip.dev->parent,
> "ingenic,pwm-channels-mask",
> @@ -214,6 +218,7 @@ static int jz4740_pwm_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct jz4740_pwm_chip *jz4740;
> + const struct soc_info *info = device_get_match_data(dev);
>
> jz4740 = devm_kzalloc(dev, sizeof(*jz4740), GFP_KERNEL);
> if (!jz4740)
> @@ -227,8 +232,8 @@ static int jz4740_pwm_probe(struct platform_device *pdev)
>
> jz4740->chip.dev = dev;
> jz4740->chip.ops = &jz4740_pwm_ops;
> - jz4740->chip.npwm = NUM_PWM;
> jz4740->chip.base = -1;
> + jz4740->chip.npwm = info ? info->num_pwms : NUM_PWM;
Can info be actually NULL? I don't think so, so you can just use
info->num_pwms here and drop the definition of NUM_PWM.
> jz4740->chip.of_xlate = of_pwm_xlate_with_flags;
> jz4740->chip.of_pwm_n_cells = 3;
>
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] pwm: jz4740: Add support for the JZ4725B
2020-05-24 17:37 ` Uwe Kleine-König
@ 2020-05-24 17:51 ` Paul Cercueil
2020-05-24 18:56 ` Uwe Kleine-König
0 siblings, 1 reply; 9+ messages in thread
From: Paul Cercueil @ 2020-05-24 17:51 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: Thierry Reding, od, linux-pwm, linux-kernel
Hi Uwe,
Le dim. 24 mai 2020 à 19:37, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> a écrit :
> On Mon, Apr 13, 2020 at 02:14:45PM +0200, Paul Cercueil wrote:
>> The PWM hardware in the JZ4725B works the same as in the JZ4740,
>> but has
>> only six channels available.
>>
>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>> ---
>>
>> Notes:
>> I did not add documentation for the new jz4725b-pwm compatible
>> string on purpose. The reason is that the documentation file
>> for the Timer/Counter Unit (TCU) of Ingenic SoCs will be
>> completely rewritten from .txt to YAML in a separate patchset.
>>
>> drivers/pwm/pwm-jz4740.c | 20 +++++++++++++++++---
>> 1 file changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pwm/pwm-jz4740.c b/drivers/pwm/pwm-jz4740.c
>> index f566f9d248d6..bb27934fb6c2 100644
>> --- a/drivers/pwm/pwm-jz4740.c
>> +++ b/drivers/pwm/pwm-jz4740.c
>> @@ -22,6 +22,10 @@
>>
>> #define NUM_PWM 8
>>
>> +struct soc_info {
>> + unsigned int num_pwms;
>> +};
>> +
>> struct jz4740_pwm_chip {
>> struct pwm_chip chip;
>> struct regmap *map;
>> @@ -36,7 +40,7 @@ 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);
>> + u32 pwm_channels_mask = GENMASK(jz->chip.npwm - 1, 2);
>>
>> device_property_read_u32(jz->chip.dev->parent,
>> "ingenic,pwm-channels-mask",
>> @@ -214,6 +218,7 @@ static int jz4740_pwm_probe(struct
>> platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> struct jz4740_pwm_chip *jz4740;
>> + const struct soc_info *info = device_get_match_data(dev);
>>
>> jz4740 = devm_kzalloc(dev, sizeof(*jz4740), GFP_KERNEL);
>> if (!jz4740)
>> @@ -227,8 +232,8 @@ static int jz4740_pwm_probe(struct
>> platform_device *pdev)
>>
>> jz4740->chip.dev = dev;
>> jz4740->chip.ops = &jz4740_pwm_ops;
>> - jz4740->chip.npwm = NUM_PWM;
>> jz4740->chip.base = -1;
>> + jz4740->chip.npwm = info ? info->num_pwms : NUM_PWM;
>
> Can info be actually NULL? I don't think so, so you can just use
> info->num_pwms here and drop the definition of NUM_PWM.
In *theory* it can be NULL if the kernel is configured without
CONFIG_OF, which will never happen on any board supported by this
driver. I can add a dependency on CONFIG_OF in V2, then use
info->num_pwms there.
cheers,
-Paul
>> jz4740->chip.of_xlate = of_pwm_xlate_with_flags;
>> jz4740->chip.of_pwm_n_cells = 3;
>>
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König
> |
> Industrial Linux Solutions |
> https://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] pwm: jz4740: Add support for the JZ4725B
2020-05-24 17:51 ` Paul Cercueil
@ 2020-05-24 18:56 ` Uwe Kleine-König
0 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2020-05-24 18:56 UTC (permalink / raw)
To: Paul Cercueil; +Cc: Thierry Reding, od, linux-pwm, linux-kernel
Hello Paul,
On Sun, May 24, 2020 at 07:51:23PM +0200, Paul Cercueil wrote:
> Le dim. 24 mai 2020 à 19:37, Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> a écrit :
> > On Mon, Apr 13, 2020 at 02:14:45PM +0200, Paul Cercueil wrote:
> > > @@ -214,6 +218,7 @@ static int jz4740_pwm_probe(struct
> > > platform_device *pdev)
> > > {
> > > struct device *dev = &pdev->dev;
> > > struct jz4740_pwm_chip *jz4740;
> > > + const struct soc_info *info = device_get_match_data(dev);
> > >
> > > jz4740 = devm_kzalloc(dev, sizeof(*jz4740), GFP_KERNEL);
> > > if (!jz4740)
> > > @@ -227,8 +232,8 @@ static int jz4740_pwm_probe(struct
> > > platform_device *pdev)
> > >
> > > jz4740->chip.dev = dev;
> > > jz4740->chip.ops = &jz4740_pwm_ops;
> > > - jz4740->chip.npwm = NUM_PWM;
> > > jz4740->chip.base = -1;
> > > + jz4740->chip.npwm = info ? info->num_pwms : NUM_PWM;
> >
> > Can info be actually NULL? I don't think so, so you can just use
> > info->num_pwms here and drop the definition of NUM_PWM.
>
> In *theory* it can be NULL if the kernel is configured without CONFIG_OF,
> which will never happen on any board supported by this driver. I can add a
> dependency on CONFIG_OF in V2, then use info->num_pwms there.
I'd prefer just something like:
if (!info)
return -EINVAL;
instead of a dependency on CONFIG_OF.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-05-24 18:56 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-13 12:14 [PATCH 1/3] pwm: jz4740: Drop dependency on MACH_INGENIC Paul Cercueil
2020-04-13 12:14 ` [PATCH 2/3] pwm: jz4740: Make PWM start with the active part Paul Cercueil
2020-05-24 17:35 ` Uwe Kleine-König
2020-04-13 12:14 ` [PATCH 3/3] pwm: jz4740: Add support for the JZ4725B Paul Cercueil
2020-05-24 17:37 ` Uwe Kleine-König
2020-05-24 17:51 ` Paul Cercueil
2020-05-24 18:56 ` Uwe Kleine-König
2020-05-24 16:16 ` [PATCH 1/3] pwm: jz4740: Drop dependency on MACH_INGENIC Paul Cercueil
2020-05-24 17:03 ` Uwe Kleine-König
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).