.get_state() can return an error indication. Make use of it to propagate
failing hardware accesses.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Link: https://lore.kernel.org/r/20221130152148.2769768-8-u.kleine-koenig@pengutronix.de
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
.get_state() might fail in some cases. To make it possible that a driver
signals such a failure change the prototype of .get_state() to return an
error code.
This patch was created using coccinelle and the following semantic patch:
@p1@
identifier getstatefunc;
identifier driver;
@@
struct pwm_ops driver = {
...,
.get_state = getstatefunc
,...
};
@p2@
identifier p1.getstatefunc;
identifier chip, pwm, state;
@@
-void
+int
getstatefunc(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_state *state)
{
...
- return;
+ return 0;
...
}
plus the actual change of the prototype in include/linux/pwm.h (plus some
manual fixing of indentions and empty lines).
So for now all drivers return success unconditionally. They are adapted
in the following patches to make the changes easier reviewable.
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Acked-by: Douglas Anderson <dianders@chromium.org>
Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Link: https://lore.kernel.org/r/20221130152148.2769768-2-u.kleine-koenig@pengutronix.de
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
This allows to drop the platform_driver's remove function. This is the
only user of driver data so this can go away, too.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
With the previous commit there is no need for the lowlevel driver any
more to specify it it uses two or three cells. So simplify accordingly.
The only non-trival change affects the pwm-rockchip driver: It used to only
support three cells if the hardware supports polarity. Now the default
number depends on the device tree which has to match hardware anyhow
(and if it doesn't the error is just a bit delayed as a PWM handle with
an inverted setting is catched when pwm_apply_state() is called).
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
Since commit 5e5da1e9fb ("pwm: ab8500: Explicitly allocate pwm chip
base dynamically") all drivers use dynamic ID allocation explicitly. New
drivers are supposed to do the same, so remove support for driver
specified base IDs and drop all assignments in the low-level drivers.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
The second parameter of do_div is an u32 and NSEC_PER_SEC * prescale
overflows this for bigger periods. Assuming the usual pwm input clk rate
of 66 MHz this happens starting at requested period > 606060 ns.
Splitting the division into two operations doesn't loose any precision.
It doesn't need to be feared that c / NSEC_PER_SEC doesn't fit into the
unsigned long variable "duty_cycles" because in this case the assignment
above to period_cycles would already have been overflowing as
period >= duty_cycle and then the calculation is moot anyhow.
Fixes: aef1a3799b ("pwm: imx27: Fix rounding behavior")
Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
Tested-by: Johannes Pointner <johannes.pointner@br-automation.com>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
dev_err_probe() can reduce code size, uniform error handling and record the
defer probe reason etc., use it to simplify the code.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
Since the PWM framework is switching struct pwm_state.period's
datatype to u64, prepare for this transition by using
DIV_ROUND_UP_ULL to handle a 64-bit dividend.
Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
To not trigger the warnings provided by CONFIG_PWM_DEBUG
- use up-rounding in .get_state()
- don't divide by the result of a division
- don't use the rounded counter value for the period length to calculate
the counter value for the duty cycle
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
There is nothing in use from of_device.h, remove it.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
Up to now the .probe() function didn't enable clocks and relied on the
core to call the .get_state() callback to have the clock running. The
latter enabled the needed clocks and kept them running if the PWM wass
enabled.
This only works correctly if the .get_state() callback is called exactly
once and this single call happens before unused clocks are disabled by
the clk core.
The former wasn't true for a short period while commit 01ccf903ed
("pwm: Let pwm_get_state() return the last implemented state") applied
and not reverted yet and might become wrong in the future.
The latter isn't true any more since commit cfc4c189bc ("pwm: Read
initial hardware state at request time") which results in a running PWM
being stopped at boot time if for example the consumer lives in a kernel
module that is only loaded after the clk core disabled unused clocks.
So ensure .probe() is left with the clocks on if the PWM is running and
.get_state() disables everything it enabled.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
The .remove() callback is not supposed to modify hardware state. This is
in the responsibility of the PWM consumer.
After the PWM was disabled the clocks are off (apart from a bug that is
fixed in the next patch), so unbinding the driver either stops the PWM
(which it should not) or disables already disabled clocks yielding
warnings from the clk core.
So just drop the call to disable the clocks. (Which BTW was also in the
wrong order because the call makes the PWM unfunctional and so should
have come only after pwmchip_remove()).
Fixes: 9f4c8f9607 ("pwm: imx: Add ipg clock operation")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
pwm_imx27_clk_prepare_enable() took a pointer to a struct pwm_chip just
to convert it to a struct pwm_imx27_chip pointer while all callers
already have the latter. Ditto for pwm_imx27_clk_disable_unprepare().
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
pwm_imx27_apply() enables the clocks if the previous PWM state was
disabled. Given that the clocks are supposed to be left on iff the PWM
is running, the decision to disable the clocks at the end of the
function must not depend on the previous state.
Without this fix the enable count of the two affected clocks increases
by one whenever ->apply() changes from one disabled state to another.
Fixes: bd88d319ab ("pwm: imx27: Unconditionally write state to hardware")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
For defer probe error, no need to output error message which
will cause confusion.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
The i.MX driver currently uses a shortcut and doesn't write all of the
state through to the hardware when the PWM is disabled. This causes an
inconsistent state to be read back by consumers with the result of them
malfunctioning.
Fix this by always writing the full state through to the hardware
registers so that the correct state can always be read back.
Tested-by: Michal Vokáč <michal.vokac@ysoft.com>
Tested-by: Adam Ford <aford173@gmail.com>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
The hardware register containing the duty cycle value cannot be accessed
when the PWM is disabled. This causes the ->get_state() callback to read
back a duty cycle value of 0, which can confuse consumer drivers.
Tested-by: Michal Vokáč <michal.vokac@ysoft.com>
Tested-by: Adam Ford <aford173@gmail.com>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
It is surprising for a PWM consumer when the variable holding the
requested state is modified by pwm_apply_state(). Consider for example a
driver doing:
#define PERIOD 5000000
#define DUTY_LITTLE 10
...
struct pwm_state state = {
.period = PERIOD,
.duty_cycle = DUTY_LITTLE,
.polarity = PWM_POLARITY_NORMAL,
.enabled = true,
};
pwm_apply_state(mypwm, &state);
...
state.duty_cycle = PERIOD / 2;
pwm_apply_state(mypwm, &state);
For sure the second call to pwm_apply_state() should still have
state.period = PERIOD and not something the hardware driver chose for a
reason that doesn't necessarily apply to the second call.
So declare the state argument as a pointer to a const type and adapt all
drivers' .apply callbacks.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
Use the new helper devm_platform_ioremap_resource() which wraps the
platform_get_resource() and devm_ioremap_resource() together, to
simplify the code.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
Currently the function pwm_imx27_get_state() of enables the clocks once
unconditionally at the start, a second time if the PWM is enabled and
disables unconditionally at the end.
Simplify that to enable once at the start and disable conditionally at
the end.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
"ret" only holds zero and negative error codes. It needs to be signed
for the error handling to work.
Fixes: 9f4c8f9607 ("pwm: imx: Add ipg clock operation")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
The two PWM implementations called v1 (for i.MX1 and i.MX21) and v2 (for
i.MX27 and later) have nothing in common apart from needing two clocks
named "per" and "ipg" and being integrated in a SoC named i.MX.
So split the file containing the two disjunct drivers into two files and
two complete separate drivers.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
[thierry.reding@gmail.com: fix a modular build issue]
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
Uwe Kleine-König
Uwe Kleine-König
Anson Huang
Guru Das Srinagesh
Thierry Reding
Dan Carpenter