| Message ID | 20221130152148.2769768-1-u.kleine-koenig@pengutronix.de |
|---|---|
| Headers | show |
| Series | pwm: Allow .get_state to fail | expand |
Hello Andy, On Fri, Dec 09, 2022 at 11:47:54PM +0200, Andy Shevchenko wrote: > On Wed, Nov 30, 2022 at 04:21:37PM +0100, Uwe Kleine-König wrote: > > In v1 Thierry had the concern: > > > > | That raises the question about what to do in these cases. If we return > > | an error, that could potentially throw off consumers. So perhaps the > > | closest would be to return a disabled PWM? Or perhaps it'd be up to the > > | consumer to provide some fallback configuration for invalidly configured > > | or unconfigured PWMs. > > > > .get_state() is only called in pwm_device_request on a pwm_state that a > > consumer might see. Before my series a consumer might have seen a > > partial modified pwm_state (because .get_state() might have modified > > .period, then stumbled and returned silently). The last patch ensures > > that this partial modification isn't given out to the consumer. Instead > > they now see the same as if .get_state wasn't implemented at all. > > I'm wondering why we didn't see a compiler warning about mistyped function > prototypes in some drivers. I don't understand where you expected a warning. Care to elaborate? > P.S. The series is good thing to do, thank you. It's already too late for an ack, the series is already in Thierry's tree. Best regards Uwe
Hello Andy, On Sat, Dec 10, 2022 at 10:57:16PM +0200, Andy Shevchenko wrote: > On Sat, Dec 10, 2022 at 10:18:33AM +0100, Uwe Kleine-König wrote: > > On Fri, Dec 09, 2022 at 11:47:54PM +0200, Andy Shevchenko wrote: > > > On Wed, Nov 30, 2022 at 04:21:37PM +0100, Uwe Kleine-König wrote: > > ... > > > > I'm wondering why we didn't see a compiler warning about mistyped function > > > prototypes in some drivers. > > > > I don't understand where you expected a warning. Care to elaborate? > > intel-lpss.c has the prototype that returns an int. IIRC it was like this Do you mean drivers/mfd/intel-lpss.c? That one doesn't implement a PWM?! And drivers/pwm/pwm-lpss.c is adapted by this series. One of us is confused ... Best regards Uwe
Hello, I forgot about this series and was remembered when I talked to Conor Dooley about how .get_state() should behave in an error case. Compared to (implicit) v1, sent with Message-Id: 20220916151506.298488-1-u.kleine-koenig@pengutronix.de I changed: - Patch #1 which does the prototype change now just adds "return 0" to all implementations and so gets simpler and doesn't change behaviour. The adaptions to the different .get_state() implementations are split out into individual patches to ease review. - One minor inconsistency fixed in "pwm: Handle .get_state() failures" that I noticed while looking into this patch. - I skipped changing sun4i.c as I don't know how to handle the error there. Someone might want to have a look. (That's not ideal, but it's not worse than the same issue before this series.) In v1 Thierry had the concern: | That raises the question about what to do in these cases. If we return | an error, that could potentially throw off consumers. So perhaps the | closest would be to return a disabled PWM? Or perhaps it'd be up to the | consumer to provide some fallback configuration for invalidly configured | or unconfigured PWMs. .get_state() is only called in pwm_device_request on a pwm_state that a consumer might see. Before my series a consumer might have seen a partial modified pwm_state (because .get_state() might have modified .period, then stumbled and returned silently). The last patch ensures that this partial modification isn't given out to the consumer. Instead they now see the same as if .get_state wasn't implemented at all. Best regards Uwe Uwe Kleine-König (11): pwm: Make .get_state() callback return an error code pwm/tracing: Also record trace events for failed API calls drm/bridge: ti-sn65dsi86: Propagate errors in .get_state() to the caller leds: qcom-lpg: Propagate errors in .get_state() to the caller pwm: crc: Propagate errors in .get_state() to the caller pwm: cros-ec: Propagate errors in .get_state() to the caller pwm: imx27: Propagate errors in .get_state() to the caller pwm: mtk-disp: Propagate errors in .get_state() to the caller pwm: rockchip: Propagate errors in .get_state() to the caller pwm: sprd: Propagate errors in .get_state() to the caller pwm: Handle .get_state() failures drivers/gpio/gpio-mvebu.c | 9 ++++++--- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 14 ++++++++------ drivers/leds/rgb/leds-qcom-lpg.c | 14 ++++++++------ drivers/pwm/core.c | 28 +++++++++++++++++---------- drivers/pwm/pwm-atmel.c | 6 ++++-- drivers/pwm/pwm-bcm-iproc.c | 8 +++++--- drivers/pwm/pwm-crc.c | 10 ++++++---- drivers/pwm/pwm-cros-ec.c | 8 +++++--- drivers/pwm/pwm-dwc.c | 6 ++++-- drivers/pwm/pwm-hibvt.c | 6 ++++-- drivers/pwm/pwm-imx-tpm.c | 8 +++++--- drivers/pwm/pwm-imx27.c | 8 +++++--- drivers/pwm/pwm-intel-lgm.c | 6 ++++-- drivers/pwm/pwm-iqs620a.c | 6 ++++-- drivers/pwm/pwm-keembay.c | 6 ++++-- drivers/pwm/pwm-lpss.c | 6 ++++-- drivers/pwm/pwm-meson.c | 8 +++++--- drivers/pwm/pwm-mtk-disp.c | 12 +++++++----- drivers/pwm/pwm-pca9685.c | 8 +++++--- drivers/pwm/pwm-raspberrypi-poe.c | 8 +++++--- drivers/pwm/pwm-rockchip.c | 12 +++++++----- drivers/pwm/pwm-sifive.c | 6 ++++-- drivers/pwm/pwm-sl28cpld.c | 8 +++++--- drivers/pwm/pwm-sprd.c | 8 +++++--- drivers/pwm/pwm-stm32-lp.c | 8 +++++--- drivers/pwm/pwm-sun4i.c | 12 +++++++----- drivers/pwm/pwm-sunplus.c | 6 ++++-- drivers/pwm/pwm-visconti.c | 6 ++++-- drivers/pwm/pwm-xilinx.c | 8 +++++--- include/linux/pwm.h | 4 ++-- include/trace/events/pwm.h | 20 +++++++++---------- 31 files changed, 174 insertions(+), 109 deletions(-) base-commit: 50315945d178eebec4e8e2c50c265767ddb926eb