From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Marco Felsch <m.felsch@pengutronix.de>
Cc: thierry.reding@gmail.com, lee.jones@linaro.org,
shawnguo@kernel.org, s.hauer@pengutronix.de, festevam@gmail.com,
linux-imx@nxp.com, Anson.Huang@nxp.com, michal.vokac@ysoft.com,
l.majewski@majess.pl, linux-pwm@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, kernel@pengutronix.de
Subject: Re: [PATCH v2 1/5] pwm: imx27: enable clock unconditional for register access
Date: Sat, 26 Sep 2020 15:48:23 +0200 [thread overview]
Message-ID: <20200926134823.zog3722y3l3ti25x@pengutronix.de> (raw)
In-Reply-To: <20200925155330.32301-2-m.felsch@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]
Hello,
On Fri, Sep 25, 2020 at 05:53:26PM +0200, Marco Felsch wrote:
> The current implementation enables the clock if the current PWM state
> is '!enabled' to ensure the register access and left the clock on if the
> new state is 'enabled'. Further apply calls don't enable the clock since
> they relying on the fact the the clock is already running. Change this
> behaviour since it is not very intuitive.
>
> This commit changes this behaviour. Now the clocks are unconditional
> enabled/disabled before/after the register access. If the PWM should be
> turned on (state.enabled) we enable the clock again and vice versa if
> the PWM should be turned off (!state.enabled).
>
> Therefore I added the enable member to the driver state struct since
> the usage of cstate and pwm_get_state() is a layer violation. I removed
> this violation while on it.
while looking through patch 2 I found something missing here:
You don't initialize .enabled in .probe().
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Marco Felsch <m.felsch@pengutronix.de>
Cc: linux-pwm@vger.kernel.org, michal.vokac@ysoft.com,
kernel@pengutronix.de, Anson.Huang@nxp.com, lee.jones@linaro.org,
s.hauer@pengutronix.de, thierry.reding@gmail.com,
linux-imx@nxp.com, festevam@gmail.com, shawnguo@kernel.org,
l.majewski@majess.pl, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 1/5] pwm: imx27: enable clock unconditional for register access
Date: Sat, 26 Sep 2020 15:48:23 +0200 [thread overview]
Message-ID: <20200926134823.zog3722y3l3ti25x@pengutronix.de> (raw)
In-Reply-To: <20200925155330.32301-2-m.felsch@pengutronix.de>
[-- Attachment #1.1: Type: text/plain, Size: 1162 bytes --]
Hello,
On Fri, Sep 25, 2020 at 05:53:26PM +0200, Marco Felsch wrote:
> The current implementation enables the clock if the current PWM state
> is '!enabled' to ensure the register access and left the clock on if the
> new state is 'enabled'. Further apply calls don't enable the clock since
> they relying on the fact the the clock is already running. Change this
> behaviour since it is not very intuitive.
>
> This commit changes this behaviour. Now the clocks are unconditional
> enabled/disabled before/after the register access. If the PWM should be
> turned on (state.enabled) we enable the clock again and vice versa if
> the PWM should be turned off (!state.enabled).
>
> Therefore I added the enable member to the driver state struct since
> the usage of cstate and pwm_get_state() is a layer violation. I removed
> this violation while on it.
while looking through patch 2 I found something missing here:
You don't initialize .enabled in .probe().
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-09-26 13:48 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-25 15:53 [PATCH v2 0/5] PWM i.MX27 fix disabled state for inverted signals Marco Felsch
2020-09-25 15:53 ` Marco Felsch
2020-09-25 15:53 ` [PATCH v2 1/5] pwm: imx27: enable clock unconditional for register access Marco Felsch
2020-09-25 15:53 ` Marco Felsch
2020-09-26 13:28 ` Uwe Kleine-König
2020-09-26 13:28 ` Uwe Kleine-König
2020-09-26 13:48 ` Uwe Kleine-König [this message]
2020-09-26 13:48 ` Uwe Kleine-König
2020-09-28 5:52 ` Marco Felsch
2020-09-28 5:52 ` Marco Felsch
2020-09-25 15:53 ` [PATCH v2 2/5] pwm: imx27: move constant PWMCR register values into probe Marco Felsch
2020-09-25 15:53 ` Marco Felsch
2020-09-26 13:46 ` Uwe Kleine-König
2020-09-26 13:46 ` Uwe Kleine-König
2020-09-28 5:50 ` Marco Felsch
2020-09-28 5:50 ` Marco Felsch
2020-09-25 15:53 ` [PATCH v2 3/5] pwm: imx27: reset the PWM if it is not running Marco Felsch
2020-09-25 15:53 ` Marco Felsch
2020-09-28 7:30 ` Uwe Kleine-König
2020-09-28 7:30 ` Uwe Kleine-König
2020-09-28 9:29 ` Marco Felsch
2020-09-28 9:29 ` Marco Felsch
2020-09-25 15:53 ` [PATCH v2 4/5] pwm: imx27: fix disable state for inverted PWMs Marco Felsch
2020-09-25 15:53 ` Marco Felsch
2020-09-28 7:47 ` Uwe Kleine-König
2020-09-28 7:47 ` Uwe Kleine-König
2020-09-28 9:52 ` Marco Felsch
2020-09-28 9:52 ` Marco Felsch
2020-09-28 19:06 ` Uwe Kleine-König
2020-09-28 19:06 ` Uwe Kleine-König
2020-09-29 5:23 ` Marco Felsch
2020-09-29 5:23 ` Marco Felsch
2020-09-25 15:53 ` [PATCH v2 5/5] pwm: imx27: wait till the duty cycle is applied Marco Felsch
2020-09-25 15:53 ` Marco Felsch
2020-09-28 8:04 ` Uwe Kleine-König
2020-09-28 8:04 ` Uwe Kleine-König
2020-09-28 9:59 ` Marco Felsch
2020-09-28 9:59 ` Marco Felsch
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200926134823.zog3722y3l3ti25x@pengutronix.de \
--to=u.kleine-koenig@pengutronix.de \
--cc=Anson.Huang@nxp.com \
--cc=festevam@gmail.com \
--cc=kernel@pengutronix.de \
--cc=l.majewski@majess.pl \
--cc=lee.jones@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-imx@nxp.com \
--cc=linux-pwm@vger.kernel.org \
--cc=m.felsch@pengutronix.de \
--cc=michal.vokac@ysoft.com \
--cc=s.hauer@pengutronix.de \
--cc=shawnguo@kernel.org \
--cc=thierry.reding@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.