* Re: [PATCH v5 30/46] regulator: pwm: retrieve correct voltage
From: Mark Brown @ 2016-03-30 21:24 UTC (permalink / raw)
To: Boris Brezillon
Cc: Thierry Reding, linux-pwm-u79uwXL29TY76Z2rM5mHXA, Mike Turquette,
Stephen Boyd, linux-clk-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood,
Kamil Debski, lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Jean Delvare,
Guenter Roeck, Dmitry Torokhov,
linux-input-u79uwXL29TY76Z2rM5mHXA, Bryan Wu, Richard Purdie,
Jacek Anaszewski, linux-leds-u79uwXL29TY76Z2rM5mHXA,
Maxime Ripard, Chen-Yu Tsai, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
Joachim Eastwood, Thomas Petazzoni, Heiko Stuebner,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jingoo Han, Lee
In-Reply-To: <1459368249-13241-31-git-send-email-boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 451 bytes --]
On Wed, Mar 30, 2016 at 10:03:53PM +0200, Boris Brezillon wrote:
> The continuous PWM voltage regulator is caching the voltage value in
> the ->volt_uV field. While most of the time this value should reflect the
> real voltage, sometime it can be sightly different if the PWM device
> rounded the set_duty_cycle request.
> Moreover, this value is not valid until someone has modified the regulator
> output.
Acked-by: Mark Brown <broonie@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept
From: Stephen Boyd @ 2016-03-30 21:55 UTC (permalink / raw)
To: Boris Brezillon
Cc: Thierry Reding, linux-pwm-u79uwXL29TY76Z2rM5mHXA, Mike Turquette,
linux-clk-u79uwXL29TY76Z2rM5mHXA, Mark Brown, Liam Girdwood,
Kamil Debski, lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Jean Delvare,
Guenter Roeck, Dmitry Torokhov,
linux-input-u79uwXL29TY76Z2rM5mHXA, Bryan Wu, Richard Purdie,
Jacek Anaszewski, linux-leds-u79uwXL29TY76Z2rM5mHXA,
Maxime Ripard, Chen-Yu Tsai, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
Joachim Eastwood, Thomas Petazzoni, Heiko Stuebner,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jingoo Han
In-Reply-To: <1459368249-13241-6-git-send-email-boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On 03/30, Boris Brezillon wrote:
> @@ -74,6 +74,23 @@ enum pwm_polarity {
> PWM_POLARITY_INVERSED,
> };
>
> +/**
> + * struct pwm_args - PWM arguments
> + * @period: reference period
> + * @polarity: reference polarity
> + *
> + * This structure describe board-dependent arguments attached to a PWM
s/describe/describes/
> + * device. Those arguments are usually retrieved from the PWM lookup table or
> + * DT definition.
> + * This should not be confused with the PWM state: PWM args not representing
s/not representing/don't represent/ ?
> + * the current PWM state, but the configuration the PWM user plan to use
s/plan/plans/
> + * on this PWM device.
> + */
> +struct pwm_args {
> + unsigned int period;
> + enum pwm_polarity polarity;
> +};
> +
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v5 07/46] clk: pwm: use pwm_get_args() where appropriate
From: Stephen Boyd @ 2016-03-30 21:58 UTC (permalink / raw)
To: Boris Brezillon
Cc: Thierry Reding, linux-pwm-u79uwXL29TY76Z2rM5mHXA, Mike Turquette,
linux-clk-u79uwXL29TY76Z2rM5mHXA, Mark Brown, Liam Girdwood,
Kamil Debski, lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Jean Delvare,
Guenter Roeck, Dmitry Torokhov,
linux-input-u79uwXL29TY76Z2rM5mHXA, Bryan Wu, Richard Purdie,
Jacek Anaszewski, linux-leds-u79uwXL29TY76Z2rM5mHXA,
Maxime Ripard, Chen-Yu Tsai, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
Joachim Eastwood, Thomas Petazzoni, Heiko Stuebner,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jingoo Han
In-Reply-To: <1459368249-13241-8-git-send-email-boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On 03/30, Boris Brezillon wrote:
> The PWM framework has clarified the concept of reference PWM config
> (the platform dependent config retrieved from the DT or the PWM
> lookup table) and real PWM state.
>
> Use pwm_get_args() when the PWM user wants to retrieve this reference
> config and not the current state.
>
> This is part of the rework allowing the PWM framework to support
> hardware readout and expose real PWM state even when the PWM has
> just been requested (before the user calls pwm_config/enable/disable()).
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
Acked-by: Stephen Boyd <sboyd@codeaurora.org>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v5 34/46] clk: pwm: switch to the atomic API
From: Stephen Boyd @ 2016-03-30 22:01 UTC (permalink / raw)
To: Boris Brezillon
Cc: Thierry Reding, linux-pwm, Mike Turquette, linux-clk, Mark Brown,
Liam Girdwood, Kamil Debski, lm-sensors, Jean Delvare,
Guenter Roeck, Dmitry Torokhov, linux-input, Bryan Wu,
Richard Purdie, Jacek Anaszewski, linux-leds, Maxime Ripard,
Chen-Yu Tsai, linux-sunxi, Joachim Eastwood, Thomas Petazzoni,
Heiko Stuebner, linux-rockchip, Jingoo Han, Lee
In-Reply-To: <1459368249-13241-35-git-send-email-boris.brezillon@free-electrons.com>
On 03/30, Boris Brezillon wrote:
> diff --git a/drivers/clk/clk-pwm.c b/drivers/clk/clk-pwm.c
> index ebcd738..49ec5b1 100644
> --- a/drivers/clk/clk-pwm.c
> +++ b/drivers/clk/clk-pwm.c
> @@ -28,15 +28,29 @@ static inline struct clk_pwm *to_clk_pwm(struct clk_hw *hw)
> static int clk_pwm_prepare(struct clk_hw *hw)
> {
> struct clk_pwm *clk_pwm = to_clk_pwm(hw);
> + struct pwm_state pstate;
>
> - return pwm_enable(clk_pwm->pwm);
> + pwm_get_state(clk_pwm->pwm, &pstate);
> + if (pstate.enabled)
> + return 0;
> +
> + pstate.enabled = true;
> +
> + return pwm_apply_state(clk_pwm->pwm, &pstate);
This doesn't seem atomic anymore if we're checking the state and
then not calling apply_state if it's already enabled. But I
assume this doesn't matter because we "own" the pwm here?
Otherwise I would think this would be unconditional apply state
and duplicates would be ignored in the pwm framework.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v5 08/46] hwmon: pwm-fan: use pwm_get_args() where appropriate
From: Guenter Roeck @ 2016-03-30 22:52 UTC (permalink / raw)
To: Boris Brezillon
Cc: Thierry Reding, linux-pwm, Mike Turquette, Stephen Boyd,
linux-clk, Mark Brown, Liam Girdwood, Kamil Debski, lm-sensors,
Jean Delvare, Dmitry Torokhov, linux-input, Bryan Wu,
Richard Purdie, Jacek Anaszewski, linux-leds, Maxime Ripard,
Chen-Yu Tsai, linux-sunxi, Joachim Eastwood, Thomas Petazzoni,
Heiko Stuebner, linux-rockchip, Jingoo Han, Lee
In-Reply-To: <1459368249-13241-9-git-send-email-boris.brezillon@free-electrons.com>
On Wed, Mar 30, 2016 at 10:03:31PM +0200, Boris Brezillon wrote:
> The PWM framework has clarified the concept of reference PWM config
> (the platform dependent config retrieved from the DT or the PWM
> lookup table) and real PWM state.
>
> Use pwm_get_args() when the PWM user wants to retrieve this reference
> config and not the current state.
>
> This is part of the rework allowing the PWM framework to support
> hardware readout and expose real PWM state even when the PWM has
> just been requested (before the user calls pwm_config/enable/disable()).
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> drivers/hwmon/pwm-fan.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index 3e23003..82c5656 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -40,15 +40,18 @@ struct pwm_fan_ctx {
>
> static int __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm)
> {
> + struct pwm_args pargs = { };
Hi Boris,
I guess I am missing some context; sorry for that. Unfortunately,
I did not easily find an explanation, so please bear with me.
Two questions: Why do we need a local copy of struct pwm_args instead
of a pointer to it ? If it can change while being used, isn't it
inconsistent anyway ?
Also, assuming the local copy is necessary, why initialize pargs ?
After all, pwm_get_args() just overwrites it.
Thanks,
Guenter
> unsigned long duty;
> int ret = 0;
>
> + pwm_get_args(ctx->pwm, &pargs);
> +
> mutex_lock(&ctx->lock);
> if (ctx->pwm_value = pwm)
> goto exit_set_pwm_err;
>
> - duty = DIV_ROUND_UP(pwm * (ctx->pwm->period - 1), MAX_PWM);
> - ret = pwm_config(ctx->pwm, duty, ctx->pwm->period);
> + duty = DIV_ROUND_UP(pwm * (pargs.period - 1), MAX_PWM);
> + ret = pwm_config(ctx->pwm, duty, pargs.period);
> if (ret)
> goto exit_set_pwm_err;
>
> @@ -214,6 +217,7 @@ static int pwm_fan_of_get_cooling_data(struct device *dev,
> static int pwm_fan_probe(struct platform_device *pdev)
> {
> struct thermal_cooling_device *cdev;
> + struct pwm_args pargs = { };
> struct pwm_fan_ctx *ctx;
> struct device *hwmon;
> int duty_cycle;
> @@ -234,10 +238,11 @@ static int pwm_fan_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, ctx);
>
> /* Set duty cycle to maximum allowed */
> - duty_cycle = ctx->pwm->period - 1;
> + pwm_get_args(ctx->pwm, &pargs);
> + duty_cycle = pargs.period - 1;
> ctx->pwm_value = MAX_PWM;
>
> - ret = pwm_config(ctx->pwm, duty_cycle, ctx->pwm->period);
> + ret = pwm_config(ctx->pwm, duty_cycle, pargs.period);
> if (ret) {
> dev_err(&pdev->dev, "Failed to configure PWM\n");
> return ret;
> @@ -303,14 +308,16 @@ static int pwm_fan_suspend(struct device *dev)
> static int pwm_fan_resume(struct device *dev)
> {
> struct pwm_fan_ctx *ctx = dev_get_drvdata(dev);
> + struct pwm_args pargs = { };
> unsigned long duty;
> int ret;
>
> if (ctx->pwm_value = 0)
> return 0;
>
> - duty = DIV_ROUND_UP(ctx->pwm_value * (ctx->pwm->period - 1), MAX_PWM);
> - ret = pwm_config(ctx->pwm, duty, ctx->pwm->period);
> + pwm_get_args(ctx->pwm, &pargs);
> + duty = DIV_ROUND_UP(ctx->pwm_value * (pargs.period - 1), MAX_PWM);
> + ret = pwm_config(ctx->pwm, duty, pargs.period);
> if (ret)
> return ret;
> return pwm_enable(ctx->pwm);
> --
> 2.5.0
>
^ permalink raw reply
* [PATCH] fbdev: sh_mipi_dsi: remove driver
From: Simon Horman @ 2016-03-31 1:09 UTC (permalink / raw)
To: linux-fbdev
Remove the sh_mipi_dsi driver as it appears to be unused since
9a9863987bf7 ("ARM: shmobile: Remove legacy SoC code for SH-Mobile AG5").
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
Based on v4.6-rc1
---
drivers/video/Kconfig | 4 -
drivers/video/fbdev/Kconfig | 1 -
drivers/video/fbdev/Makefile | 1 -
drivers/video/fbdev/sh_mipi_dsi.c | 587 --------------------------------------
include/video/sh_mipi_dsi.h | 59 ----
5 files changed, 652 deletions(-)
delete mode 100644 drivers/video/fbdev/sh_mipi_dsi.c
delete mode 100644 include/video/sh_mipi_dsi.h
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index e0606c01e8ac..3c20af999893 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -8,10 +8,6 @@ menu "Graphics support"
config HAVE_FB_ATMEL
bool
-config SH_MIPI_DSI
- tristate
- depends on (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
-
config SH_LCD_MIPI_DSI
bool
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 983280e8d93f..7d991cb9e78e 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -1993,7 +1993,6 @@ config FB_SH_MOBILE_LCDC
select FB_SYS_FOPS
select FB_DEFERRED_IO
select FB_BACKLIGHT
- select SH_MIPI_DSI if SH_LCD_MIPI_DSI
---help---
Frame buffer driver for the on-chip SH-Mobile LCD controller.
diff --git a/drivers/video/fbdev/Makefile b/drivers/video/fbdev/Makefile
index 65fb15075c8f..f6731867dd26 100644
--- a/drivers/video/fbdev/Makefile
+++ b/drivers/video/fbdev/Makefile
@@ -117,7 +117,6 @@ obj-$(CONFIG_FB_SM501) += sm501fb.o
obj-$(CONFIG_FB_UDL) += udlfb.o
obj-$(CONFIG_FB_SMSCUFX) += smscufx.o
obj-$(CONFIG_FB_XILINX) += xilinxfb.o
-obj-$(CONFIG_SH_MIPI_DSI) += sh_mipi_dsi.o
obj-$(CONFIG_FB_SH_MOBILE_MERAM) += sh_mobile_meram.o
obj-$(CONFIG_FB_SH_MOBILE_LCDC) += sh_mobile_lcdcfb.o
obj-$(CONFIG_FB_OMAP) += omap/
diff --git a/drivers/video/fbdev/sh_mipi_dsi.c b/drivers/video/fbdev/sh_mipi_dsi.c
deleted file mode 100644
index 8f6e8ff620d4..000000000000
--- a/drivers/video/fbdev/sh_mipi_dsi.c
+++ /dev/null
@@ -1,587 +0,0 @@
-/*
- * Renesas SH-mobile MIPI DSI support
- *
- * Copyright (C) 2010 Guennadi Liakhovetski <g.liakhovetski@gmx.de>
- *
- * This is free software; you can redistribute it and/or modify
- * it under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- */
-
-#include <linux/bitmap.h>
-#include <linux/clk.h>
-#include <linux/delay.h>
-#include <linux/init.h>
-#include <linux/io.h>
-#include <linux/platform_device.h>
-#include <linux/pm_runtime.h>
-#include <linux/slab.h>
-#include <linux/string.h>
-#include <linux/types.h>
-#include <linux/module.h>
-
-#include <video/mipi_display.h>
-#include <video/sh_mipi_dsi.h>
-#include <video/sh_mobile_lcdc.h>
-
-#include "sh_mobile_lcdcfb.h"
-
-#define SYSCTRL 0x0000
-#define SYSCONF 0x0004
-#define TIMSET 0x0008
-#define RESREQSET0 0x0018
-#define RESREQSET1 0x001c
-#define HSTTOVSET 0x0020
-#define LPRTOVSET 0x0024
-#define TATOVSET 0x0028
-#define PRTOVSET 0x002c
-#define DSICTRL 0x0030
-#define DSIINTE 0x0060
-#define PHYCTRL 0x0070
-
-/* relative to linkbase */
-#define DTCTR 0x0000
-#define VMCTR1 0x0020
-#define VMCTR2 0x0024
-#define VMLEN1 0x0028
-#define VMLEN2 0x002c
-#define CMTSRTREQ 0x0070
-#define CMTSRTCTR 0x00d0
-
-/* E.g., sh7372 has 2 MIPI-DSIs - one for each LCDC */
-#define MAX_SH_MIPI_DSI 2
-
-struct sh_mipi {
- struct sh_mobile_lcdc_entity entity;
-
- void __iomem *base;
- void __iomem *linkbase;
- struct clk *dsit_clk;
- struct platform_device *pdev;
-};
-
-#define to_sh_mipi(e) container_of(e, struct sh_mipi, entity)
-
-static struct sh_mipi *mipi_dsi[MAX_SH_MIPI_DSI];
-
-/* Protect the above array */
-static DEFINE_MUTEX(array_lock);
-
-static struct sh_mipi *sh_mipi_by_handle(int handle)
-{
- if (handle >= ARRAY_SIZE(mipi_dsi) || handle < 0)
- return NULL;
-
- return mipi_dsi[handle];
-}
-
-static int sh_mipi_send_short(struct sh_mipi *mipi, u8 dsi_cmd,
- u8 cmd, u8 param)
-{
- u32 data = (dsi_cmd << 24) | (cmd << 16) | (param << 8);
- int cnt = 100;
-
- /* transmit a short packet to LCD panel */
- iowrite32(1 | data, mipi->linkbase + CMTSRTCTR);
- iowrite32(1, mipi->linkbase + CMTSRTREQ);
-
- while ((ioread32(mipi->linkbase + CMTSRTREQ) & 1) && --cnt)
- udelay(1);
-
- return cnt ? 0 : -ETIMEDOUT;
-}
-
-#define LCD_CHAN2MIPI(c) ((c) < LCDC_CHAN_MAINLCD || (c) > LCDC_CHAN_SUBLCD ? \
- -EINVAL : (c) - 1)
-
-static int sh_mipi_dcs(int handle, u8 cmd)
-{
- struct sh_mipi *mipi = sh_mipi_by_handle(LCD_CHAN2MIPI(handle));
- if (!mipi)
- return -ENODEV;
- return sh_mipi_send_short(mipi, MIPI_DSI_DCS_SHORT_WRITE, cmd, 0);
-}
-
-static int sh_mipi_dcs_param(int handle, u8 cmd, u8 param)
-{
- struct sh_mipi *mipi = sh_mipi_by_handle(LCD_CHAN2MIPI(handle));
- if (!mipi)
- return -ENODEV;
- return sh_mipi_send_short(mipi, MIPI_DSI_DCS_SHORT_WRITE_PARAM, cmd,
- param);
-}
-
-static void sh_mipi_dsi_enable(struct sh_mipi *mipi, bool enable)
-{
- /*
- * enable LCDC data tx, transition to LPS after completion of each HS
- * packet
- */
- iowrite32(0x00000002 | enable, mipi->linkbase + DTCTR);
-}
-
-static void sh_mipi_shutdown(struct platform_device *pdev)
-{
- struct sh_mipi *mipi = to_sh_mipi(platform_get_drvdata(pdev));
-
- sh_mipi_dsi_enable(mipi, false);
-}
-
-static int sh_mipi_setup(struct sh_mipi *mipi, const struct fb_videomode *mode)
-{
- void __iomem *base = mipi->base;
- struct sh_mipi_dsi_info *pdata = mipi->pdev->dev.platform_data;
- u32 pctype, datatype, pixfmt, linelength, vmctr2;
- u32 tmp, top, bottom, delay, div;
- int bpp;
-
- /*
- * Select data format. MIPI DSI is not hot-pluggable, so, we just use
- * the default videomode. If this ever becomes a problem, We'll have to
- * move this to mipi_display_on() above and use info->var.xres
- */
- switch (pdata->data_format) {
- case MIPI_RGB888:
- pctype = 0;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_24;
- pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
- linelength = mode->xres * 3;
- break;
- case MIPI_RGB565:
- pctype = 1;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_16;
- pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
- linelength = mode->xres * 2;
- break;
- case MIPI_RGB666_LP:
- pctype = 2;
- datatype = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
- pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
- linelength = mode->xres * 3;
- break;
- case MIPI_RGB666:
- pctype = 3;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_18;
- pixfmt = MIPI_DCS_PIXEL_FMT_18BIT;
- linelength = (mode->xres * 18 + 7) / 8;
- break;
- case MIPI_BGR888:
- pctype = 8;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_24;
- pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
- linelength = mode->xres * 3;
- break;
- case MIPI_BGR565:
- pctype = 9;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_16;
- pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
- linelength = mode->xres * 2;
- break;
- case MIPI_BGR666_LP:
- pctype = 0xa;
- datatype = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
- pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
- linelength = mode->xres * 3;
- break;
- case MIPI_BGR666:
- pctype = 0xb;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_18;
- pixfmt = MIPI_DCS_PIXEL_FMT_18BIT;
- linelength = (mode->xres * 18 + 7) / 8;
- break;
- case MIPI_YUYV:
- pctype = 4;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16;
- pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
- linelength = mode->xres * 2;
- break;
- case MIPI_UYVY:
- pctype = 5;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16;
- pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
- linelength = mode->xres * 2;
- break;
- case MIPI_YUV420_L:
- pctype = 6;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12;
- pixfmt = MIPI_DCS_PIXEL_FMT_12BIT;
- linelength = (mode->xres * 12 + 7) / 8;
- break;
- case MIPI_YUV420:
- pctype = 7;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12;
- pixfmt = MIPI_DCS_PIXEL_FMT_12BIT;
- /* Length of U/V line */
- linelength = (mode->xres + 1) / 2;
- break;
- default:
- return -EINVAL;
- }
-
- if (!pdata->lane)
- return -EINVAL;
-
- /* reset DSI link */
- iowrite32(0x00000001, base + SYSCTRL);
- /* Hold reset for 100 cycles of the slowest of bus, HS byte and LP clock */
- udelay(50);
- iowrite32(0x00000000, base + SYSCTRL);
-
- /* setup DSI link */
-
- /*
- * T_wakeup = 0x7000
- * T_hs-trail = 3
- * T_hs-prepare = 3
- * T_clk-trail = 3
- * T_clk-prepare = 2
- */
- iowrite32(0x70003332, base + TIMSET);
- /* no responses requested */
- iowrite32(0x00000000, base + RESREQSET0);
- /* request response to packets of type 0x28 */
- iowrite32(0x00000100, base + RESREQSET1);
- /* High-speed transmission timeout, default 0xffffffff */
- iowrite32(0x0fffffff, base + HSTTOVSET);
- /* LP reception timeout, default 0xffffffff */
- iowrite32(0x0fffffff, base + LPRTOVSET);
- /* Turn-around timeout, default 0xffffffff */
- iowrite32(0x0fffffff, base + TATOVSET);
- /* Peripheral reset timeout, default 0xffffffff */
- iowrite32(0x0fffffff, base + PRTOVSET);
- /* Interrupts not used, disable all */
- iowrite32(0, base + DSIINTE);
- /* DSI-Tx bias on */
- iowrite32(0x00000001, base + PHYCTRL);
- udelay(200);
- /* Deassert resets, power on */
- iowrite32(0x03070001 | pdata->phyctrl, base + PHYCTRL);
-
- /*
- * Default = ULPS enable |
- * Contention detection enabled |
- * EoT packet transmission enable |
- * CRC check enable |
- * ECC check enable
- */
- bitmap_fill((unsigned long *)&tmp, pdata->lane);
- tmp |= 0x00003700;
- iowrite32(tmp, base + SYSCONF);
-
- /* setup l-bridge */
-
- /*
- * Enable transmission of all packets,
- * transmit LPS after each HS packet completion
- */
- iowrite32(0x00000006, mipi->linkbase + DTCTR);
- /* VSYNC width = 2 (<< 17) */
- iowrite32((mode->vsync_len << pdata->vsynw_offset) |
- (pdata->clksrc << 16) | (pctype << 12) | datatype,
- mipi->linkbase + VMCTR1);
-
- /*
- * Non-burst mode with sync pulses: VSE and HSE are output,
- * HSA period allowed, no commands in LP
- */
- vmctr2 = 0;
- if (pdata->flags & SH_MIPI_DSI_VSEE)
- vmctr2 |= 1 << 23;
- if (pdata->flags & SH_MIPI_DSI_HSEE)
- vmctr2 |= 1 << 22;
- if (pdata->flags & SH_MIPI_DSI_HSAE)
- vmctr2 |= 1 << 21;
- if (pdata->flags & SH_MIPI_DSI_BL2E)
- vmctr2 |= 1 << 17;
- if (pdata->flags & SH_MIPI_DSI_HSABM)
- vmctr2 |= 1 << 5;
- if (pdata->flags & SH_MIPI_DSI_HBPBM)
- vmctr2 |= 1 << 4;
- if (pdata->flags & SH_MIPI_DSI_HFPBM)
- vmctr2 |= 1 << 3;
- iowrite32(vmctr2, mipi->linkbase + VMCTR2);
-
- /*
- * VMLEN1 = RGBLEN | HSALEN
- *
- * see
- * Video mode - Blanking Packet setting
- */
- top = linelength << 16; /* RGBLEN */
- bottom = 0x00000001;
- if (pdata->flags & SH_MIPI_DSI_HSABM) /* HSALEN */
- bottom = (pdata->lane * mode->hsync_len) - 10;
- iowrite32(top | bottom , mipi->linkbase + VMLEN1);
-
- /*
- * VMLEN2 = HBPLEN | HFPLEN
- *
- * see
- * Video mode - Blanking Packet setting
- */
- top = 0x00010000;
- bottom = 0x00000001;
- delay = 0;
-
- div = 1; /* HSbyteCLK is calculation base
- * HS4divCLK = HSbyteCLK/2
- * HS6divCLK is not supported for now */
- if (pdata->flags & SH_MIPI_DSI_HS4divCLK)
- div = 2;
-
- if (pdata->flags & SH_MIPI_DSI_HFPBM) { /* HBPLEN */
- top = mode->hsync_len + mode->left_margin;
- top = ((pdata->lane * top / div) - 10) << 16;
- }
- if (pdata->flags & SH_MIPI_DSI_HBPBM) { /* HFPLEN */
- bottom = mode->right_margin;
- bottom = (pdata->lane * bottom / div) - 12;
- }
-
- bpp = linelength / mode->xres; /* byte / pixel */
- if ((pdata->lane / div) > bpp) {
- tmp = mode->xres / bpp; /* output cycle */
- tmp = mode->xres - tmp; /* (input - output) cycle */
- delay = (pdata->lane * tmp);
- }
-
- iowrite32(top | (bottom + delay) , mipi->linkbase + VMLEN2);
-
- msleep(5);
-
- /* setup LCD panel */
-
- /* cf. drivers/video/omap/lcd_mipid.c */
- sh_mipi_dcs(pdata->channel, MIPI_DCS_EXIT_SLEEP_MODE);
- msleep(120);
- /*
- * [7] - Page Address Mode
- * [6] - Column Address Mode
- * [5] - Page / Column Address Mode
- * [4] - Display Device Line Refresh Order
- * [3] - RGB/BGR Order
- * [2] - Display Data Latch Data Order
- * [1] - Flip Horizontal
- * [0] - Flip Vertical
- */
- sh_mipi_dcs_param(pdata->channel, MIPI_DCS_SET_ADDRESS_MODE, 0x00);
- /* cf. set_data_lines() */
- sh_mipi_dcs_param(pdata->channel, MIPI_DCS_SET_PIXEL_FORMAT,
- pixfmt << 4);
- sh_mipi_dcs(pdata->channel, MIPI_DCS_SET_DISPLAY_ON);
-
- /* Enable timeout counters */
- iowrite32(0x00000f00, base + DSICTRL);
-
- return 0;
-}
-
-static int mipi_display_on(struct sh_mobile_lcdc_entity *entity)
-{
- struct sh_mipi *mipi = to_sh_mipi(entity);
- struct sh_mipi_dsi_info *pdata = mipi->pdev->dev.platform_data;
- int ret;
-
- pm_runtime_get_sync(&mipi->pdev->dev);
-
- ret = pdata->set_dot_clock(mipi->pdev, mipi->base, 1);
- if (ret < 0)
- goto mipi_display_on_fail1;
-
- ret = sh_mipi_setup(mipi, &entity->def_mode);
- if (ret < 0)
- goto mipi_display_on_fail2;
-
- sh_mipi_dsi_enable(mipi, true);
-
- return SH_MOBILE_LCDC_DISPLAY_CONNECTED;
-
-mipi_display_on_fail1:
- pm_runtime_put_sync(&mipi->pdev->dev);
-mipi_display_on_fail2:
- pdata->set_dot_clock(mipi->pdev, mipi->base, 0);
-
- return ret;
-}
-
-static void mipi_display_off(struct sh_mobile_lcdc_entity *entity)
-{
- struct sh_mipi *mipi = to_sh_mipi(entity);
- struct sh_mipi_dsi_info *pdata = mipi->pdev->dev.platform_data;
-
- sh_mipi_dsi_enable(mipi, false);
-
- pdata->set_dot_clock(mipi->pdev, mipi->base, 0);
-
- pm_runtime_put_sync(&mipi->pdev->dev);
-}
-
-static const struct sh_mobile_lcdc_entity_ops mipi_ops = {
- .display_on = mipi_display_on,
- .display_off = mipi_display_off,
-};
-
-static int __init sh_mipi_probe(struct platform_device *pdev)
-{
- struct sh_mipi *mipi;
- struct sh_mipi_dsi_info *pdata = pdev->dev.platform_data;
- struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- struct resource *res2 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- unsigned long rate, f_current;
- int idx = pdev->id, ret;
-
- if (!res || !res2 || idx >= ARRAY_SIZE(mipi_dsi) || !pdata)
- return -ENODEV;
-
- if (!pdata->set_dot_clock)
- return -EINVAL;
-
- mutex_lock(&array_lock);
- if (idx < 0)
- for (idx = 0; idx < ARRAY_SIZE(mipi_dsi) && mipi_dsi[idx]; idx++)
- ;
-
- if (idx = ARRAY_SIZE(mipi_dsi)) {
- ret = -EBUSY;
- goto efindslot;
- }
-
- mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);
- if (!mipi) {
- ret = -ENOMEM;
- goto ealloc;
- }
-
- mipi->entity.owner = THIS_MODULE;
- mipi->entity.ops = &mipi_ops;
-
- if (!request_mem_region(res->start, resource_size(res), pdev->name)) {
- dev_err(&pdev->dev, "MIPI register region already claimed\n");
- ret = -EBUSY;
- goto ereqreg;
- }
-
- mipi->base = ioremap(res->start, resource_size(res));
- if (!mipi->base) {
- ret = -ENOMEM;
- goto emap;
- }
-
- if (!request_mem_region(res2->start, resource_size(res2), pdev->name)) {
- dev_err(&pdev->dev, "MIPI register region 2 already claimed\n");
- ret = -EBUSY;
- goto ereqreg2;
- }
-
- mipi->linkbase = ioremap(res2->start, resource_size(res2));
- if (!mipi->linkbase) {
- ret = -ENOMEM;
- goto emap2;
- }
-
- mipi->pdev = pdev;
-
- mipi->dsit_clk = clk_get(&pdev->dev, "dsit_clk");
- if (IS_ERR(mipi->dsit_clk)) {
- ret = PTR_ERR(mipi->dsit_clk);
- goto eclktget;
- }
-
- f_current = clk_get_rate(mipi->dsit_clk);
- /* 80MHz required by the datasheet */
- rate = clk_round_rate(mipi->dsit_clk, 80000000);
- if (rate > 0 && rate != f_current)
- ret = clk_set_rate(mipi->dsit_clk, rate);
- else
- ret = rate;
- if (ret < 0)
- goto esettrate;
-
- dev_dbg(&pdev->dev, "DSI-T clk %lu -> %lu\n", f_current, rate);
-
- ret = clk_enable(mipi->dsit_clk);
- if (ret < 0)
- goto eclkton;
-
- mipi_dsi[idx] = mipi;
-
- pm_runtime_enable(&pdev->dev);
- pm_runtime_resume(&pdev->dev);
-
- mutex_unlock(&array_lock);
- platform_set_drvdata(pdev, &mipi->entity);
-
- return 0;
-
-eclkton:
-esettrate:
- clk_put(mipi->dsit_clk);
-eclktget:
- iounmap(mipi->linkbase);
-emap2:
- release_mem_region(res2->start, resource_size(res2));
-ereqreg2:
- iounmap(mipi->base);
-emap:
- release_mem_region(res->start, resource_size(res));
-ereqreg:
- kfree(mipi);
-ealloc:
-efindslot:
- mutex_unlock(&array_lock);
-
- return ret;
-}
-
-static int sh_mipi_remove(struct platform_device *pdev)
-{
- struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- struct resource *res2 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- struct sh_mipi *mipi = to_sh_mipi(platform_get_drvdata(pdev));
- int i, ret;
-
- mutex_lock(&array_lock);
-
- for (i = 0; i < ARRAY_SIZE(mipi_dsi) && mipi_dsi[i] != mipi; i++)
- ;
-
- if (i = ARRAY_SIZE(mipi_dsi)) {
- ret = -EINVAL;
- } else {
- ret = 0;
- mipi_dsi[i] = NULL;
- }
-
- mutex_unlock(&array_lock);
-
- if (ret < 0)
- return ret;
-
- pm_runtime_disable(&pdev->dev);
- clk_disable(mipi->dsit_clk);
- clk_put(mipi->dsit_clk);
-
- iounmap(mipi->linkbase);
- if (res2)
- release_mem_region(res2->start, resource_size(res2));
- iounmap(mipi->base);
- if (res)
- release_mem_region(res->start, resource_size(res));
- kfree(mipi);
-
- return 0;
-}
-
-static struct platform_driver sh_mipi_driver = {
- .remove = sh_mipi_remove,
- .shutdown = sh_mipi_shutdown,
- .driver = {
- .name = "sh-mipi-dsi",
- },
-};
-
-module_platform_driver_probe(sh_mipi_driver, sh_mipi_probe);
-
-MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>");
-MODULE_DESCRIPTION("SuperH / ARM-shmobile MIPI DSI driver");
-MODULE_LICENSE("GPL v2");
diff --git a/include/video/sh_mipi_dsi.h b/include/video/sh_mipi_dsi.h
deleted file mode 100644
index a01f197e6ac1..000000000000
--- a/include/video/sh_mipi_dsi.h
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
- * Public SH-mobile MIPI DSI header
- *
- * Copyright (C) 2010 Guennadi Liakhovetski <g.liakhovetski@gmx.de>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-#ifndef VIDEO_SH_MIPI_DSI_H
-#define VIDEO_SH_MIPI_DSI_H
-
-enum sh_mipi_dsi_data_fmt {
- MIPI_RGB888,
- MIPI_RGB565,
- MIPI_RGB666_LP,
- MIPI_RGB666,
- MIPI_BGR888,
- MIPI_BGR565,
- MIPI_BGR666_LP,
- MIPI_BGR666,
- MIPI_YUYV,
- MIPI_UYVY,
- MIPI_YUV420_L,
- MIPI_YUV420,
-};
-
-#define SH_MIPI_DSI_HSABM (1 << 0)
-#define SH_MIPI_DSI_HBPBM (1 << 1)
-#define SH_MIPI_DSI_HFPBM (1 << 2)
-#define SH_MIPI_DSI_BL2E (1 << 3)
-#define SH_MIPI_DSI_VSEE (1 << 4)
-#define SH_MIPI_DSI_HSEE (1 << 5)
-#define SH_MIPI_DSI_HSAE (1 << 6)
-
-#define SH_MIPI_DSI_HSbyteCLK (1 << 24)
-#define SH_MIPI_DSI_HS6divCLK (1 << 25)
-#define SH_MIPI_DSI_HS4divCLK (1 << 26)
-
-#define SH_MIPI_DSI_SYNC_PULSES_MODE (SH_MIPI_DSI_VSEE | \
- SH_MIPI_DSI_HSEE | \
- SH_MIPI_DSI_HSAE)
-#define SH_MIPI_DSI_SYNC_EVENTS_MODE (0)
-#define SH_MIPI_DSI_SYNC_BURST_MODE (SH_MIPI_DSI_BL2E)
-
-struct sh_mipi_dsi_info {
- enum sh_mipi_dsi_data_fmt data_format;
- int channel;
- int lane;
- unsigned long flags;
- u32 clksrc;
- u32 phyctrl; /* for extra setting */
- unsigned int vsynw_offset;
- int (*set_dot_clock)(struct platform_device *pdev,
- void __iomem *base,
- int enable);
-};
-
-#endif
--
2.1.4
^ permalink raw reply related
* Re: [PATCH] omapfb: Fix regulator API abuse in dss.c and hdmi4.c
From: Tomi Valkeinen @ 2016-03-31 6:30 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1459355376-28507-1-git-send-email-broonie@kernel.org>
[-- Attachment #1.1: Type: text/plain, Size: 2042 bytes --]
Hi Mark,
On 30/03/16 19:29, Mark Brown wrote:
> The voltage changing code in this driver is broken and should be
> removed. The driver sets a single, exact voltage on probe. Unless
> there is a very good reason for this (which should be documented in
> comments) constraints like this need to be set via the machine
> constraints, voltage setting in a driver is expected to be used in cases
> where the voltage varies at runtime.
>
> In addition client drivers should almost never be calling
> regulator_can_set_voltage(), if the device needs to set a voltage it
> needs to set the voltage and the regulator core will handle the case
> where the regulator is fixed voltage. If the driver can skip setting
> the voltage it should just never set the voltage.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> drivers/video/fbdev/omap2/omapfb/dss/dsi.c | 9 ---------
> drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c | 9 ---------
> 2 files changed, 18 deletions(-)
>
> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dsi.c b/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
> index 0eec073b3919..cfd0e3d5f36a 100644
> --- a/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/dsi.c
> @@ -1180,15 +1180,6 @@ static int dsi_regulator_init(struct platform_device *dsidev)
> return PTR_ERR(vdds_dsi);
> }
>
> - if (regulator_can_change_voltage(vdds_dsi)) {
> - r = regulator_set_voltage(vdds_dsi, 1800000, 1800000);
> - if (r) {
> - devm_regulator_put(vdds_dsi);
> - DSSERR("can't set the DSI regulator voltage\n");
> - return r;
> - }
> - }
> -
This code did fix an issue, see 02b7a32083b9930543663720758de249b4f6a2a3.
Now, even at the time when I wrote that fix, it did feel a bit odd to
me. I have to say I don't remember the discussion that led to the patch,
perhaps it was something along "yes, the driver should not need to do
that, but for the time being do it".
So where are these "machine constraints" defined?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v5 34/46] clk: pwm: switch to the atomic API
From: Boris Brezillon @ 2016-03-31 6:57 UTC (permalink / raw)
To: Stephen Boyd
Cc: Thierry Reding, linux-pwm-u79uwXL29TY76Z2rM5mHXA, Mike Turquette,
linux-clk-u79uwXL29TY76Z2rM5mHXA, Mark Brown, Liam Girdwood,
Kamil Debski, lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Jean Delvare,
Guenter Roeck, Dmitry Torokhov,
linux-input-u79uwXL29TY76Z2rM5mHXA, Bryan Wu, Richard Purdie,
Jacek Anaszewski, linux-leds-u79uwXL29TY76Z2rM5mHXA,
Maxime Ripard, Chen-Yu Tsai, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
Joachim Eastwood, Thomas Petazzoni, Heiko Stuebner,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jingoo Han,
Lee Jones <lee.jo>
In-Reply-To: <20160330220149.GU18567-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Hi Stephen,
On Wed, 30 Mar 2016 15:01:49 -0700
Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 03/30, Boris Brezillon wrote:
> > diff --git a/drivers/clk/clk-pwm.c b/drivers/clk/clk-pwm.c
> > index ebcd738..49ec5b1 100644
> > --- a/drivers/clk/clk-pwm.c
> > +++ b/drivers/clk/clk-pwm.c
> > @@ -28,15 +28,29 @@ static inline struct clk_pwm *to_clk_pwm(struct clk_hw *hw)
> > static int clk_pwm_prepare(struct clk_hw *hw)
> > {
> > struct clk_pwm *clk_pwm = to_clk_pwm(hw);
> > + struct pwm_state pstate;
> >
> > - return pwm_enable(clk_pwm->pwm);
> > + pwm_get_state(clk_pwm->pwm, &pstate);
> > + if (pstate.enabled)
> > + return 0;
> > +
> > + pstate.enabled = true;
> > +
> > + return pwm_apply_state(clk_pwm->pwm, &pstate);
>
> This doesn't seem atomic anymore if we're checking the state and
> then not calling apply_state if it's already enabled. But I
> assume this doesn't matter because we "own" the pwm here?
Yep. Actually it's not atomic in term of concurrency (maybe the
'atomic' word is not appropriate here). Atomicity is here referring to
the fact that we're now providing all the PWM parameters in the same
request instead of splitting it in pwm_config() + pwm_enable/disable()
calls.
Concurrent accesses still have to be controlled by the PWM user (which
is already the case for this driver, thanks to the locking
infrastructure in the CCF).
> Otherwise I would think this would be unconditional apply state
> and duplicates would be ignored in the pwm framework.
>
Yep, I'll remove the if (pstate.enabled) branch.
Thanks for your review.
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v5 08/46] hwmon: pwm-fan: use pwm_get_args() where appropriate
From: Boris Brezillon @ 2016-03-31 7:07 UTC (permalink / raw)
To: Guenter Roeck
Cc: Milo Kim, Kamil Debski, Heiko Stuebner, linux-doc, David Airlie,
Mike Turquette, linux-fbdev, dri-devel, linux-kernel, linux-sunxi,
Thierry Reding, Alexandre Belloni, Daniel Vetter, Lee Jones,
linux-clk, linux-leds, Krzysztof Kozlowski, linux-samsung-soc,
Alexander Shiyan, Jonathan Corbet, Robert Jarzmik, lm-sensors,
linux-rockchip, Chen-Yu Tsai, Tomi Valkeinen
In-Reply-To: <20160330225244.GA24044@roeck-us.net>
Hi Guenter,
On Wed, 30 Mar 2016 15:52:44 -0700
Guenter Roeck <linux@roeck-us.net> wrote:
> On Wed, Mar 30, 2016 at 10:03:31PM +0200, Boris Brezillon wrote:
> > The PWM framework has clarified the concept of reference PWM config
> > (the platform dependent config retrieved from the DT or the PWM
> > lookup table) and real PWM state.
> >
> > Use pwm_get_args() when the PWM user wants to retrieve this reference
> > config and not the current state.
> >
> > This is part of the rework allowing the PWM framework to support
> > hardware readout and expose real PWM state even when the PWM has
> > just been requested (before the user calls pwm_config/enable/disable()).
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> > drivers/hwmon/pwm-fan.c | 19 +++++++++++++------
> > 1 file changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> > index 3e23003..82c5656 100644
> > --- a/drivers/hwmon/pwm-fan.c
> > +++ b/drivers/hwmon/pwm-fan.c
> > @@ -40,15 +40,18 @@ struct pwm_fan_ctx {
> >
> > static int __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm)
> > {
> > + struct pwm_args pargs = { };
>
> Hi Boris,
>
> I guess I am missing some context; sorry for that. Unfortunately,
> I did not easily find an explanation, so please bear with me.
>
> Two questions: Why do we need a local copy of struct pwm_args instead
> of a pointer to it ? If it can change while being used, isn't it
> inconsistent anyway ?
It cannot change after pwm_get() is called. For the reason behind
prototype: I just followed the Thierry's proposal, but I'm perfectly
fine returning a const struct pwm_args pointer intead of passing
pwm_args as a parameter.
Thierry, what's your opinion?
>
> Also, assuming the local copy is necessary, why initialize pargs ?
> After all, pwm_get_args() just overwrites it.
It's a leftover from a previous version where pwm_get_args was
implemented this way:
static inline void pwm_get_args(pwm, args)
{
if (pwm)
*args = pwm->args
}
and this implementation was generating a lot of 'uninitialized
variable' warnings.
I just decided to drop the 'if (pwm)' test, because, IMO, this
should be checked way before calling pwm_get_args() is called.
Anyway, apparently I forgot to modify a few patches after this
modification.
Thanks for the review.
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept
From: Boris Brezillon @ 2016-03-31 7:09 UTC (permalink / raw)
To: Stephen Boyd
Cc: Thierry Reding, linux-pwm-u79uwXL29TY76Z2rM5mHXA, Mike Turquette,
linux-clk-u79uwXL29TY76Z2rM5mHXA, Mark Brown, Liam Girdwood,
Kamil Debski, lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Jean Delvare,
Guenter Roeck, Dmitry Torokhov,
linux-input-u79uwXL29TY76Z2rM5mHXA, Bryan Wu, Richard Purdie,
Jacek Anaszewski, linux-leds-u79uwXL29TY76Z2rM5mHXA,
Maxime Ripard, Chen-Yu Tsai, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
Joachim Eastwood, Thomas Petazzoni, Heiko Stuebner,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jingoo Han,
Lee Jones <lee.jo>
In-Reply-To: <20160330215510.GS18567-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On Wed, 30 Mar 2016 14:55:10 -0700
Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 03/30, Boris Brezillon wrote:
> > @@ -74,6 +74,23 @@ enum pwm_polarity {
> > PWM_POLARITY_INVERSED,
> > };
> >
> > +/**
> > + * struct pwm_args - PWM arguments
> > + * @period: reference period
> > + * @polarity: reference polarity
> > + *
> > + * This structure describe board-dependent arguments attached to a PWM
>
> s/describe/describes/
>
> > + * device. Those arguments are usually retrieved from the PWM lookup table or
> > + * DT definition.
> > + * This should not be confused with the PWM state: PWM args not representing
>
> s/not representing/don't represent/ ?
>
> > + * the current PWM state, but the configuration the PWM user plan to use
>
> s/plan/plans/
>
> > + * on this PWM device.
> > + */
> > +struct pwm_args {
> > + unsigned int period;
> > + enum pwm_polarity polarity;
> > +};
> > +
>
Will fix these errors.
Thanks,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v5 10/46] leds: pwm: use pwm_get_args() where appropriate
From: Jacek Anaszewski @ 2016-03-31 7:13 UTC (permalink / raw)
To: Boris Brezillon
Cc: Thierry Reding, linux-pwm, Mike Turquette, Stephen Boyd,
linux-clk, Mark Brown, Liam Girdwood, Kamil Debski, lm-sensors,
Jean Delvare, Guenter Roeck, Dmitry Torokhov, linux-input,
Bryan Wu, Richard Purdie, linux-leds, Maxime Ripard, Chen-Yu Tsai,
linux-sunxi, Joachim Eastwood, Thomas Petazzoni, Heiko Stuebner,
linux-rockchip, Jingoo Han, Lee Jones <lee.jon>
In-Reply-To: <1459368249-13241-11-git-send-email-boris.brezillon@free-electrons.com>
Hi Boris,
On 03/30/2016 10:03 PM, Boris Brezillon wrote:
> The PWM framework has clarified the concept of reference PWM config
> (the platform dependent config retrieved from the DT or the PWM
> lookup table) and real PWM state.
>
> Use pwm_get_args() when the PWM user wants to retrieve this reference
> config and not the current state.
>
> This is part of the rework allowing the PWM framework to support
> hardware readout and expose real PWM state even when the PWM has
> just been requested (before the user calls pwm_config/enable/disable()).
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> drivers/leds/leds-pwm.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
> index 4783bac..b48231c 100644
> --- a/drivers/leds/leds-pwm.c
> +++ b/drivers/leds/leds-pwm.c
> @@ -91,6 +91,7 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
> struct led_pwm *led, struct device_node *child)
> {
> struct led_pwm_data *led_data = &priv->leds[priv->num_leds];
> + struct pwm_args pargs = { };
> int ret;
>
> led_data->active_low = led->active_low;
> @@ -117,7 +118,8 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
> else
> led_data->cdev.brightness_set_blocking = led_pwm_set_blocking;
>
> - led_data->period = pwm_get_period(led_data->pwm);
> + pwm_get_args(led_data->pwm, &pargs);
> + led_data->period = pargs.period;
> if (!led_data->period && (led->pwm_period_ns > 0))
> led_data->period = led->pwm_period_ns;
>
>
Acked-by: Jacek Anaszewski <j.anaszewski@samsung.com>
--
Best regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH v5 05/46] pwm: introduce the pwm_args concept
From: Boris Brezillon @ 2016-03-31 7:57 UTC (permalink / raw)
To: Stephen Boyd
Cc: Thierry Reding, linux-pwm, Mike Turquette, linux-clk, Mark Brown,
Liam Girdwood, Kamil Debski, lm-sensors, Jean Delvare,
Guenter Roeck, Dmitry Torokhov, linux-input, Bryan Wu,
Richard Purdie, Jacek Anaszewski, linux-leds, Maxime Ripard,
Chen-Yu Tsai, linux-sunxi, Joachim Eastwood, Thomas Petazzoni,
Heiko Stuebner, linux-rockchip, Jingoo Han, Lee
In-Reply-To: <20160330215510.GS18567@codeaurora.org>
On Wed, 30 Mar 2016 14:55:10 -0700
Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 03/30, Boris Brezillon wrote:
> > @@ -74,6 +74,23 @@ enum pwm_polarity {
> > PWM_POLARITY_INVERSED,
> > };
> >
> > +/**
> > + * struct pwm_args - PWM arguments
> > + * @period: reference period
> > + * @polarity: reference polarity
> > + *
> > + * This structure describe board-dependent arguments attached to a PWM
>
> s/describe/describes/
>
> > + * device. Those arguments are usually retrieved from the PWM lookup table or
> > + * DT definition.
> > + * This should not be confused with the PWM state: PWM args not representing
>
> s/not representing/don't represent/ ?
Yes, I meant "are not representing", but "don't represent" is fine.
>
> > + * the current PWM state, but the configuration the PWM user plan to use
>
> s/plan/plans/
>
> > + * on this PWM device.
> > + */
> > +struct pwm_args {
> > + unsigned int period;
> > + enum pwm_polarity polarity;
> > +};
> > +
>
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v5 26/46] pwm: sun4i: implement hardware readout
From: Alexandre Belloni @ 2016-03-31 8:00 UTC (permalink / raw)
To: Boris Brezillon
Cc: Thierry Reding, linux-pwm-u79uwXL29TY76Z2rM5mHXA, Mike Turquette,
Stephen Boyd, linux-clk-u79uwXL29TY76Z2rM5mHXA, Mark Brown,
Liam Girdwood, Kamil Debski, lm-sensors-GZX6beZjE8VD60Wz+7aTrA,
Jean Delvare, Guenter Roeck, Dmitry Torokhov,
linux-input-u79uwXL29TY76Z2rM5mHXA, Bryan Wu, Richard Purdie,
Jacek Anaszewski, linux-leds-u79uwXL29TY76Z2rM5mHXA,
Maxime Ripard, Chen-Yu Tsai, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
Joachim Eastwood, Thomas Petazzoni, Heiko Stuebner,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jingoo
In-Reply-To: <1459368249-13241-27-git-send-email-boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On 30/03/2016 at 22:03:49 +0200, Boris Brezillon wrote :
> Implement ->get_state() instead of only initializing the polarity in
> the probe function.
>
> This implementation also takes care of keeping the PWM clk enabled if at
> least one of the PWM exported by the PWM chip is already enabled, which
> should prevent glitches.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
> drivers/pwm/pwm-sun4i.c | 74 ++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 55 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> index 03a99a5..34cb296 100644
> --- a/drivers/pwm/pwm-sun4i.c
> +++ b/drivers/pwm/pwm-sun4i.c
> @@ -252,11 +252,65 @@ static void sun4i_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> clk_disable_unprepare(sun4i_pwm->clk);
> }
>
> +static void sun4i_pwm_get_state(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + struct pwm_state *pstate)
> +{
> + struct sun4i_pwm_chip *sun4i_pwm = to_sun4i_pwm_chip(chip);
> + unsigned int clk_rate = clk_get_rate(sun4i_pwm->clk);
> + int prescaler, prescalerid;
> + int ret;
> + u32 val;
> +
> + ret = clk_prepare_enable(sun4i_pwm->clk);
> + if (ret) {
> + dev_err(chip->dev, "Failed to enable PWM clock");
> + return;
> + }
> +
> + val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> + if (val & BIT_CH(PWM_ACT_STATE, pwm->hwpwm))
> + pstate->polarity = PWM_POLARITY_INVERSED;
> + else
> + pstate->polarity = PWM_POLARITY_NORMAL;
> +
> + if ((val & BIT_CH(PWM_EN, pwm->hwpwm)) &&
> + (val & BIT_CH(PWM_CLK_GATING, pwm->hwpwm)))
> + pstate->enabled = true;
> + else
> + pstate->enabled = false;
> +
> + pstate->period = 0;
> + pstate->duty_cycle = 0;
> + prescalerid = (val >> (PWMCH_OFFSET * pwm->hwpwm)) & PWM_PRESCAL_MASK;
> + prescaler = prescaler_table[prescalerid];
> + if (prescaler) {
> + u64 timens;
> +
> + clk_rate /= prescaler;
> +
> + val = sun4i_pwm_readl(sun4i_pwm, PWM_CH_PRD(pwm->hwpwm));
> +
> + timens = ((val >> 16) & PWM_PRD_MASK) + 1;
> + timens *= NSEC_PER_SEC;
> + do_div(timens, clk_rate);
> + pstate->period = timens;
> +
> + timens = val & PWM_DTY_MASK;
> + timens *= NSEC_PER_SEC;
> + do_div(timens, clk_rate);
> + pstate->duty_cycle = timens;
> + }
> +
> + clk_disable_unprepare(sun4i_pwm->clk);
> +}
> +
> static const struct pwm_ops sun4i_pwm_ops = {
> .config = sun4i_pwm_config,
> .set_polarity = sun4i_pwm_set_polarity,
> .enable = sun4i_pwm_enable,
> .disable = sun4i_pwm_disable,
> + .get_state = sun4i_pwm_get_state,
> .owner = THIS_MODULE,
> };
>
> @@ -307,8 +361,7 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
> {
> struct sun4i_pwm_chip *pwm;
> struct resource *res;
> - u32 val;
> - int i, ret;
> + int ret;
> const struct of_device_id *match;
>
> match = of_match_device(sun4i_pwm_dt_ids, &pdev->dev);
> @@ -345,24 +398,7 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, pwm);
>
> - ret = clk_prepare_enable(pwm->clk);
> - if (ret) {
> - dev_err(&pdev->dev, "failed to enable PWM clock\n");
> - goto clk_error;
> - }
> -
> - val = sun4i_pwm_readl(pwm, PWM_CTRL_REG);
> - for (i = 0; i < pwm->chip.npwm; i++)
> - if (!(val & BIT_CH(PWM_ACT_STATE, i)))
> - pwm_set_polarity(&pwm->chip.pwms[i],
> - PWM_POLARITY_INVERSED);
> - clk_disable_unprepare(pwm->clk);
> -
> return 0;
> -
> -clk_error:
> - pwmchip_remove(&pwm->chip);
> - return ret;
> }
>
> static int sun4i_pwm_remove(struct platform_device *pdev)
> --
> 2.5.0
>
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH] fbdev: sh_mipi_dsi: remove driver
From: Geert Uytterhoeven @ 2016-03-31 8:33 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1459386550-4660-1-git-send-email-horms+renesas@verge.net.au>
On Thu, Mar 31, 2016 at 3:09 AM, Simon Horman
<horms+renesas@verge.net.au> wrote:
> Remove the sh_mipi_dsi driver as it appears to be unused since
> 9a9863987bf7 ("ARM: shmobile: Remove legacy SoC code for SH-Mobile AG5").
I believe that should be much older commit c0bb9b3027690e10cf8f ("ARCH: ARM:
shmobile: Remove ag5evm board support").
SH-Mobile AG5 only defined its MSTP clock, not the platform device.
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] omapfb: Fix regulator API abuse in dss.c and hdmi4.c
From: Mark Brown @ 2016-03-31 16:49 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1459355376-28507-1-git-send-email-broonie@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 941 bytes --]
On Thu, Mar 31, 2016 at 09:30:11AM +0300, Tomi Valkeinen wrote:
> This code did fix an issue, see 02b7a32083b9930543663720758de249b4f6a2a3.
That change isn't sensible, especially the _can_change_voltage() like I
said in the commit log.
> Now, even at the time when I wrote that fix, it did feel a bit odd to
> me. I have to say I don't remember the discussion that led to the patch,
> perhaps it was something along "yes, the driver should not need to do
> that, but for the time being do it".
That wasn't a discusion I was involved in, a quick google suggests it
might've been off-list.
> So where are these "machine constraints" defined?
They're the DT. Your machine constraints just seem broken and need to
be fixed, most likely whoever wrote the constraints for the platform
completely failed to understand the purpose of constraints and filled in
the maximum range of voltages that the regulator in use on the board can
support.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH] omapfb: Fix regulator API abuse in dss.c and hdmi4.c
From: Tomi Valkeinen @ 2016-03-31 17:11 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1459355376-28507-1-git-send-email-broonie@kernel.org>
[-- Attachment #1.1: Type: text/plain, Size: 2195 bytes --]
On 31/03/16 19:49, Mark Brown wrote:
> On Thu, Mar 31, 2016 at 09:30:11AM +0300, Tomi Valkeinen wrote:
>
>> This code did fix an issue, see 02b7a32083b9930543663720758de249b4f6a2a3.
>
> That change isn't sensible, especially the _can_change_voltage() like I
> said in the commit log.
I may remember wrong, but I think regulator_set_voltage() failed if
regulator_can_change_voltage() returned false. So I ended up having the
'if' there. But I may remember wrong, or maybe it's been changed since.
>> Now, even at the time when I wrote that fix, it did feel a bit odd to
>> me. I have to say I don't remember the discussion that led to the patch,
>> perhaps it was something along "yes, the driver should not need to do
>> that, but for the time being do it".
>
> That wasn't a discusion I was involved in, a quick google suggests it
> might've been off-list.
That's possible. With quick googling, this may have longer history than
the patch I sent.
>> So where are these "machine constraints" defined?
>
> They're the DT. Your machine constraints just seem broken and need to
> be fixed, most likely whoever wrote the constraints for the platform
> completely failed to understand the purpose of constraints and filled in
> the maximum range of voltages that the regulator in use on the board can
> support.
Ok.
Tero, Tony, with a quick look, for example omap5-board-common.dtsi
defines ldo4_reg having range from 1.5 to 1.8. That should be changed to
1.8 only, right? ldo1_reg has a range too, I'm not familiar with that
LDO's use, but it's for camera so my guess is that it should be 1.8 too.
I can have a more thorough look tomorrow, and do a test run on omap5
uevm (with Mark's patch).
I wonder why we have the same code in hdmi4. Again with a quick look,
omap4 boards seem to use vdac for hdmi, and vdac doesn't have any
constraints in twl6030.dtsi, so I presume it's a fixed-voltage.
Anyway, I'm happy to apply this patch (and we need similar for hdmi5,
and also for omapdrm), we just need to do any necessary fixes to the
.dts first.
Although strictly speaking, I guess that's breaking backward
compatibility...
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v5 32/46] pwm: deprecate pwm_config(), pwm_enable() and pwm_disable()
From: Dmitry Torokhov @ 2016-03-31 17:38 UTC (permalink / raw)
To: Boris Brezillon
Cc: Thierry Reding, linux-pwm, Mike Turquette, Stephen Boyd,
linux-clk, Mark Brown, Liam Girdwood, Kamil Debski, lm-sensors,
Jean Delvare, Guenter Roeck, linux-input, Bryan Wu,
Richard Purdie, Jacek Anaszewski, linux-leds, Maxime Ripard,
Chen-Yu Tsai, linux-sunxi, Joachim Eastwood, Thomas Petazzoni,
Heiko Stuebner, linux-rockchip, Jingoo Han,
Lee Jones <lee.jo>
In-Reply-To: <1459368249-13241-33-git-send-email-boris.brezillon@free-electrons.com>
Hi Boris,
On Wed, Mar 30, 2016 at 10:03:55PM +0200, Boris Brezillon wrote:
> Prefix those function as deprecated to encourage all existing users to
> switch to pwm_apply_state().
Why not keep at least some of them as wrappers where we do not need to
chnage several parameters at once? It is much easier to have a driver
do:
error = pwm_enable(pwm);
if (error)
...
rather than declaring the state variable, fectch it, adjust and then
apply.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> include/linux/pwm.h | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index 4aad4eb..9bac10f 100644
> --- a/include/linux/pwm.h
> +++ b/include/linux/pwm.h
> @@ -225,8 +225,8 @@ int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state);
> *
> * Returns: 0 on success or a negative error code on failure.
> */
> -static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
> - int period_ns)
> +static inline int __deprecated pwm_config(struct pwm_device *pwm, int duty_ns,
> + int period_ns)
> {
> struct pwm_state pstate;
>
> @@ -252,8 +252,8 @@ static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
> *
> * Returns: 0 on success or a negative error code on failure.
> */
> -static inline int pwm_set_polarity(struct pwm_device *pwm,
> - enum pwm_polarity polarity)
> +static inline int __deprecated pwm_set_polarity(struct pwm_device *pwm,
> + enum pwm_polarity polarity)
> {
> struct pwm_state pstate;
>
> @@ -284,7 +284,7 @@ static inline int pwm_set_polarity(struct pwm_device *pwm,
> *
> * Returns: 0 on success or a negative error code on failure.
> */
> -static inline int pwm_enable(struct pwm_device *pwm)
> +static inline int __deprecated pwm_enable(struct pwm_device *pwm)
> {
> struct pwm_state pstate;
>
> @@ -303,7 +303,7 @@ static inline int pwm_enable(struct pwm_device *pwm)
> * pwm_disable() - stop a PWM output toggling
> * @pwm: PWM device
> */
> -static inline void pwm_disable(struct pwm_device *pwm)
> +static inline void __deprecated pwm_disable(struct pwm_device *pwm)
> {
> struct pwm_state pstate;
>
> @@ -360,24 +360,24 @@ static inline int pwm_apply_state(struct pwm_device *pwm,
> return -ENOTSUPP;
> }
>
> -static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
> - int period_ns)
> +static inline int __deprecated pwm_config(struct pwm_device *pwm, int duty_ns,
> + int period_ns)
> {
> return -EINVAL;
> }
>
> -static inline int pwm_set_polarity(struct pwm_device *pwm,
> - enum pwm_polarity polarity)
> +static inline int __deprecated pwm_set_polarity(struct pwm_device *pwm,
> + enum pwm_polarity polarity)
> {
> return -ENOTSUPP;
> }
>
> -static inline int pwm_enable(struct pwm_device *pwm)
> +static inline int __deprecated pwm_enable(struct pwm_device *pwm)
> {
> return -EINVAL;
> }
>
> -static inline void pwm_disable(struct pwm_device *pwm)
> +static inline void __deprecated pwm_disable(struct pwm_device *pwm)
> {
> }
>
> --
> 2.5.0
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH] omapfb: Fix regulator API abuse in dss.c and hdmi4.c
From: Mark Brown @ 2016-03-31 17:39 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1459355376-28507-1-git-send-email-broonie@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]
On Thu, Mar 31, 2016 at 08:11:08PM +0300, Tomi Valkeinen wrote:
> On 31/03/16 19:49, Mark Brown wrote:
> > On Thu, Mar 31, 2016 at 09:30:11AM +0300, Tomi Valkeinen wrote:
> >> This code did fix an issue, see 02b7a32083b9930543663720758de249b4f6a2a3.
> > That change isn't sensible, especially the _can_change_voltage() like I
> > said in the commit log.
> I may remember wrong, but I think regulator_set_voltage() failed if
> regulator_can_change_voltage() returned false. So I ended up having the
> 'if' there. But I may remember wrong, or maybe it's been changed since.
That's not been the case since 2012 but your change was written in
2014...
> I wonder why we have the same code in hdmi4. Again with a quick look,
> omap4 boards seem to use vdac for hdmi, and vdac doesn't have any
> constraints in twl6030.dtsi, so I presume it's a fixed-voltage.
Yes, if no range is specified the regulator API won't touch the set
voltage.
> Anyway, I'm happy to apply this patch (and we need similar for hdmi5,
> and also for omapdrm), we just need to do any necessary fixes to the
> .dts first.
> Although strictly speaking, I guess that's breaking backward
> compatibility...
Will anyone notice?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH v5 36/46] input: misc: max77693: switch to the atomic API
From: Dmitry Torokhov @ 2016-03-31 17:48 UTC (permalink / raw)
To: Boris Brezillon
Cc: Thierry Reding, linux-pwm, Mike Turquette, Stephen Boyd,
linux-clk, Mark Brown, Liam Girdwood, Kamil Debski, lm-sensors,
Jean Delvare, Guenter Roeck, linux-input, Bryan Wu,
Richard Purdie, Jacek Anaszewski, linux-leds, Maxime Ripard,
Chen-Yu Tsai, linux-sunxi, Joachim Eastwood, Thomas Petazzoni,
Heiko Stuebner, linux-rockchip, Jingoo Han,
Lee Jones <lee.jo>
In-Reply-To: <1459368249-13241-37-git-send-email-boris.brezillon@free-electrons.com>
Hi Boris,
On Wed, Mar 30, 2016 at 10:03:59PM +0200, Boris Brezillon wrote:
> pwm_config/enable/disable() have been deprecated and should be replaced
> by pwm_apply_state().
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> drivers/input/misc/max77693-haptic.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/input/misc/max77693-haptic.c b/drivers/input/misc/max77693-haptic.c
> index cf6aac0..aef7dc4 100644
> --- a/drivers/input/misc/max77693-haptic.c
> +++ b/drivers/input/misc/max77693-haptic.c
> @@ -70,13 +70,16 @@ struct max77693_haptic {
>
> static int max77693_haptic_set_duty_cycle(struct max77693_haptic *haptic)
> {
> + struct pwm_state pstate;
> struct pwm_args pargs = { };
> - int delta;
> int error;
>
> pwm_get_args(haptic->pwm_dev, &pargs);
> - delta = (pargs.period + haptic->pwm_duty) / 2;
> - error = pwm_config(haptic->pwm_dev, delta, pargs.period);
> + pwm_get_state(haptic->pwm_dev, &pstate);
> +
> + pstate.period = pargs.period;
> + pstate.duty_cycle = (pargs.period + haptic->pwm_duty) / 2;
> + error = pwm_apply_state(haptic->pwm_dev, &pstate);
This does not make sense with regard to the atomic API. If you look in
max77693_haptic_play_work(), right after calling
max77693_haptic_set_duty_cycle() we either try to enable or disable the
pwm. When switching to this new API we should combine both actions.
> if (error) {
> dev_err(haptic->dev, "failed to configure pwm: %d\n", error);
> return error;
> @@ -161,12 +164,16 @@ static int max77693_haptic_lowsys(struct max77693_haptic *haptic, bool enable)
>
> static void max77693_haptic_enable(struct max77693_haptic *haptic)
> {
> + struct pwm_state pstate;
> int error;
>
> if (haptic->enabled)
> return;
>
> - error = pwm_enable(haptic->pwm_dev);
> + pwm_get_state(haptic->pwm_dev, &pstate);
> + pstate.enabled = true;
> +
> + error = pwm_apply_state(haptic->pwm_dev, &pstate);
As I mentioned I'd rather we did not deprecate pwm_enable() and
pwm_disable() (and maybe others), as it forces us to add unnecessary
boilerplate code to the drivers.
> if (error) {
> dev_err(haptic->dev,
> "failed to enable haptic pwm device: %d\n", error);
> @@ -188,11 +195,13 @@ static void max77693_haptic_enable(struct max77693_haptic *haptic)
> err_enable_config:
> max77693_haptic_lowsys(haptic, false);
> err_enable_lowsys:
> - pwm_disable(haptic->pwm_dev);
> + pstate.enabled = false;
> + pwm_apply_state(haptic->pwm_dev, &pstate);
> }
>
> static void max77693_haptic_disable(struct max77693_haptic *haptic)
> {
> + struct pwm_state pstate;
> int error;
>
> if (!haptic->enabled)
> @@ -206,7 +215,9 @@ static void max77693_haptic_disable(struct max77693_haptic *haptic)
> if (error)
> goto err_disable_lowsys;
>
> - pwm_disable(haptic->pwm_dev);
> + pwm_get_state(haptic->pwm_dev, &pstate);
> + pstate.enabled = false;
> + pwm_apply_state(haptic->pwm_dev, &pstate);
Same here.
> haptic->enabled = false;
>
> return;
> --
> 2.5.0
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v5 32/46] pwm: deprecate pwm_config(), pwm_enable() and pwm_disable()
From: Boris Brezillon @ 2016-03-31 18:54 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Thierry Reding, linux-pwm-u79uwXL29TY76Z2rM5mHXA, Mike Turquette,
Stephen Boyd, linux-clk-u79uwXL29TY76Z2rM5mHXA, Mark Brown,
Liam Girdwood, Kamil Debski, lm-sensors-GZX6beZjE8VD60Wz+7aTrA,
Jean Delvare, Guenter Roeck, linux-input-u79uwXL29TY76Z2rM5mHXA,
Bryan Wu, Richard Purdie, Jacek Anaszewski,
linux-leds-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard, Chen-Yu Tsai,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Joachim Eastwood,
Thomas Petazzoni, Heiko Stuebner,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jingoo Han,
Lee Jones
In-Reply-To: <20160331173858.GB39098@dtor-ws>
Hi Dmitry,
On Thu, 31 Mar 2016 10:38:58 -0700
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> Hi Boris,
>
> On Wed, Mar 30, 2016 at 10:03:55PM +0200, Boris Brezillon wrote:
> > Prefix those function as deprecated to encourage all existing users to
> > switch to pwm_apply_state().
>
> Why not keep at least some of them as wrappers where we do not need to
> chnage several parameters at once? It is much easier to have a driver
> do:
>
> error = pwm_enable(pwm);
> if (error)
> ...
>
> rather than declaring the state variable, fectch it, adjust and then
> apply.
True. Actually deprecating the non-atomic API was not my primary goal.
Thierry would you mind if we keep both APIs around?
>
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> > include/linux/pwm.h | 24 ++++++++++++------------
> > 1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> > index 4aad4eb..9bac10f 100644
> > --- a/include/linux/pwm.h
> > +++ b/include/linux/pwm.h
> > @@ -225,8 +225,8 @@ int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state);
> > *
> > * Returns: 0 on success or a negative error code on failure.
> > */
> > -static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
> > - int period_ns)
> > +static inline int __deprecated pwm_config(struct pwm_device *pwm, int duty_ns,
> > + int period_ns)
> > {
> > struct pwm_state pstate;
> >
> > @@ -252,8 +252,8 @@ static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
> > *
> > * Returns: 0 on success or a negative error code on failure.
> > */
> > -static inline int pwm_set_polarity(struct pwm_device *pwm,
> > - enum pwm_polarity polarity)
> > +static inline int __deprecated pwm_set_polarity(struct pwm_device *pwm,
> > + enum pwm_polarity polarity)
> > {
> > struct pwm_state pstate;
> >
> > @@ -284,7 +284,7 @@ static inline int pwm_set_polarity(struct pwm_device *pwm,
> > *
> > * Returns: 0 on success or a negative error code on failure.
> > */
> > -static inline int pwm_enable(struct pwm_device *pwm)
> > +static inline int __deprecated pwm_enable(struct pwm_device *pwm)
> > {
> > struct pwm_state pstate;
> >
> > @@ -303,7 +303,7 @@ static inline int pwm_enable(struct pwm_device *pwm)
> > * pwm_disable() - stop a PWM output toggling
> > * @pwm: PWM device
> > */
> > -static inline void pwm_disable(struct pwm_device *pwm)
> > +static inline void __deprecated pwm_disable(struct pwm_device *pwm)
> > {
> > struct pwm_state pstate;
> >
> > @@ -360,24 +360,24 @@ static inline int pwm_apply_state(struct pwm_device *pwm,
> > return -ENOTSUPP;
> > }
> >
> > -static inline int pwm_config(struct pwm_device *pwm, int duty_ns,
> > - int period_ns)
> > +static inline int __deprecated pwm_config(struct pwm_device *pwm, int duty_ns,
> > + int period_ns)
> > {
> > return -EINVAL;
> > }
> >
> > -static inline int pwm_set_polarity(struct pwm_device *pwm,
> > - enum pwm_polarity polarity)
> > +static inline int __deprecated pwm_set_polarity(struct pwm_device *pwm,
> > + enum pwm_polarity polarity)
> > {
> > return -ENOTSUPP;
> > }
> >
> > -static inline int pwm_enable(struct pwm_device *pwm)
> > +static inline int __deprecated pwm_enable(struct pwm_device *pwm)
> > {
> > return -EINVAL;
> > }
> >
> > -static inline void pwm_disable(struct pwm_device *pwm)
> > +static inline void __deprecated pwm_disable(struct pwm_device *pwm)
> > {
> > }
> >
> > --
> > 2.5.0
> >
>
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v5 36/46] input: misc: max77693: switch to the atomic API
From: Boris Brezillon @ 2016-03-31 18:57 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Thierry Reding, linux-pwm-u79uwXL29TY76Z2rM5mHXA, Mike Turquette,
Stephen Boyd, linux-clk-u79uwXL29TY76Z2rM5mHXA, Mark Brown,
Liam Girdwood, Kamil Debski, lm-sensors-GZX6beZjE8VD60Wz+7aTrA,
Jean Delvare, Guenter Roeck, linux-input-u79uwXL29TY76Z2rM5mHXA,
Bryan Wu, Richard Purdie, Jacek Anaszewski,
linux-leds-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard, Chen-Yu Tsai,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Joachim Eastwood,
Thomas Petazzoni, Heiko Stuebner,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jingoo Han,
Lee Jones
In-Reply-To: <20160331174801.GC39098@dtor-ws>
On Thu, 31 Mar 2016 10:48:01 -0700
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> Hi Boris,
>
> On Wed, Mar 30, 2016 at 10:03:59PM +0200, Boris Brezillon wrote:
> > pwm_config/enable/disable() have been deprecated and should be replaced
> > by pwm_apply_state().
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> > drivers/input/misc/max77693-haptic.c | 23 +++++++++++++++++------
> > 1 file changed, 17 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/input/misc/max77693-haptic.c b/drivers/input/misc/max77693-haptic.c
> > index cf6aac0..aef7dc4 100644
> > --- a/drivers/input/misc/max77693-haptic.c
> > +++ b/drivers/input/misc/max77693-haptic.c
> > @@ -70,13 +70,16 @@ struct max77693_haptic {
> >
> > static int max77693_haptic_set_duty_cycle(struct max77693_haptic *haptic)
> > {
> > + struct pwm_state pstate;
> > struct pwm_args pargs = { };
> > - int delta;
> > int error;
> >
> > pwm_get_args(haptic->pwm_dev, &pargs);
> > - delta = (pargs.period + haptic->pwm_duty) / 2;
> > - error = pwm_config(haptic->pwm_dev, delta, pargs.period);
> > + pwm_get_state(haptic->pwm_dev, &pstate);
> > +
> > + pstate.period = pargs.period;
> > + pstate.duty_cycle = (pargs.period + haptic->pwm_duty) / 2;
> > + error = pwm_apply_state(haptic->pwm_dev, &pstate);
>
> This does not make sense with regard to the atomic API. If you look in
> max77693_haptic_play_work(), right after calling
> max77693_haptic_set_duty_cycle() we either try to enable or disable the
> pwm. When switching to this new API we should combine both actions.
True. I'll address that, unless Thierry is fine keeping the non-atomic
API, in which case I'll just drop patches 32 to 46.
>
> > if (error) {
> > dev_err(haptic->dev, "failed to configure pwm: %d\n", error);
> > return error;
> > @@ -161,12 +164,16 @@ static int max77693_haptic_lowsys(struct max77693_haptic *haptic, bool enable)
> >
> > static void max77693_haptic_enable(struct max77693_haptic *haptic)
> > {
> > + struct pwm_state pstate;
> > int error;
> >
> > if (haptic->enabled)
> > return;
> >
> > - error = pwm_enable(haptic->pwm_dev);
> > + pwm_get_state(haptic->pwm_dev, &pstate);
> > + pstate.enabled = true;
> > +
> > + error = pwm_apply_state(haptic->pwm_dev, &pstate);
>
> As I mentioned I'd rather we did not deprecate pwm_enable() and
> pwm_disable() (and maybe others), as it forces us to add unnecessary
> boilerplate code to the drivers.
>
> > if (error) {
> > dev_err(haptic->dev,
> > "failed to enable haptic pwm device: %d\n", error);
> > @@ -188,11 +195,13 @@ static void max77693_haptic_enable(struct max77693_haptic *haptic)
> > err_enable_config:
> > max77693_haptic_lowsys(haptic, false);
> > err_enable_lowsys:
> > - pwm_disable(haptic->pwm_dev);
> > + pstate.enabled = false;
> > + pwm_apply_state(haptic->pwm_dev, &pstate);
> > }
> >
> > static void max77693_haptic_disable(struct max77693_haptic *haptic)
> > {
> > + struct pwm_state pstate;
> > int error;
> >
> > if (!haptic->enabled)
> > @@ -206,7 +215,9 @@ static void max77693_haptic_disable(struct max77693_haptic *haptic)
> > if (error)
> > goto err_disable_lowsys;
> >
> > - pwm_disable(haptic->pwm_dev);
> > + pwm_get_state(haptic->pwm_dev, &pstate);
> > + pstate.enabled = false;
> > + pwm_apply_state(haptic->pwm_dev, &pstate);
>
> Same here.
>
> > haptic->enabled = false;
> >
> > return;
> > --
> > 2.5.0
> >
>
> Thanks.
>
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* RE: [PATCH v5 35/46] hwmon: pwm-fan: switch to the atomic API
From: Kamil Debski @ 2016-04-01 8:29 UTC (permalink / raw)
To: 'Boris Brezillon', 'Thierry Reding', linux-pwm
Cc: 'Mike Turquette', 'Stephen Boyd', linux-clk,
'Mark Brown', 'Liam Girdwood', lm-sensors,
'Jean Delvare', 'Guenter Roeck',
'Dmitry Torokhov', linux-input, 'Bryan Wu',
'Richard Purdie', 'Jacek Anaszewski', linux-leds,
'Maxime Ripard', 'Chen-Yu Tsai', linux-sunxi,
'Joachim Eastwood', 'Thomas Petazzoni',
'Heiko Stuebner', linux-rockchip, 'Jingoo Han',
'Lee Jones', linux-fbdev@
In-Reply-To: <1459368249-13241-36-git-send-email-boris.brezillon@free-electrons.com>
Hi Boris,
From: Boris Brezillon [mailto:boris.brezillon@free-electrons.com]
Sent: Wednesday, March 30, 2016 10:04 PM
Subject: [PATCH v5 35/46] hwmon: pwm-fan: switch to the atomic API
>
> pwm_config/enable/disable() have been deprecated and should be replaced
> by pwm_apply_state().
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Acked-by: Kamil Debski <k.debski@samsung.com>
Best wishes,
--
Kamil Debski
Samsung R&D Institute Poland
^ permalink raw reply
* RE: [PATCH v5 08/46] hwmon: pwm-fan: use pwm_get_args() where appropriate
From: Kamil Debski @ 2016-04-01 8:29 UTC (permalink / raw)
To: 'Boris Brezillon', 'Thierry Reding', linux-pwm
Cc: 'Milo Kim', linux-fbdev, 'Heiko Stuebner',
linux-doc, 'David Airlie', 'Mike Turquette',
'Krzysztof Kozlowski', dri-devel, linux-kernel,
linux-sunxi, 'Alexandre Belloni', 'Daniel Vetter',
'Lee Jones', linux-clk, linux-leds, linux-samsung-soc,
'Alexander Shiyan', 'Jonathan Corbet',
'Robert Jarzmik', lm-sensors, linux-rockchip,
'Chen-Yu Tsai', 'Tomi Valkeinen', linux-input,
'Jean-Christophe
In-Reply-To: <1459368249-13241-9-git-send-email-boris.brezillon@free-electrons.com>
Hi Boris,
From: Boris Brezillon [mailto:boris.brezillon@free-electrons.com]
Sent: Wednesday, March 30, 2016 10:04 PM
Subject: [PATCH v5 08/46] hwmon: pwm-fan: use pwm_get_args() where
appropriate
>
> The PWM framework has clarified the concept of reference PWM config (the
> platform dependent config retrieved from the DT or the PWM lookup table)
> and real PWM state.
>
> Use pwm_get_args() when the PWM user wants to retrieve this reference
> config and not the current state.
>
> This is part of the rework allowing the PWM framework to support hardware
> readout and expose real PWM state even when the PWM has just been
> requested (before the user calls pwm_config/enable/disable()).
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Acked-by: Kamil Debski <k.debski@samsung.com>
Best wishes,
--
Kamil Debski
Samsung R&D Institute Poland
^ permalink raw reply
* [PATCH v2] fbdev: sh_mipi_dsi: remove driver
From: Simon Horman @ 2016-04-04 1:48 UTC (permalink / raw)
To: linux-fbdev
Remove the sh_mipi_dsi driver as it appears to be unused since
c0bb9b302769 ("ARCH: ARM: shmobile: Remove ag5evm board support").
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
Based on v4.6-rc1
v2
* Update patch referred to in changelog
---
drivers/video/Kconfig | 4 -
drivers/video/fbdev/Kconfig | 1 -
drivers/video/fbdev/Makefile | 1 -
drivers/video/fbdev/sh_mipi_dsi.c | 587 --------------------------------------
include/video/sh_mipi_dsi.h | 59 ----
5 files changed, 652 deletions(-)
delete mode 100644 drivers/video/fbdev/sh_mipi_dsi.c
delete mode 100644 include/video/sh_mipi_dsi.h
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index e0606c01e8ac..3c20af999893 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -8,10 +8,6 @@ menu "Graphics support"
config HAVE_FB_ATMEL
bool
-config SH_MIPI_DSI
- tristate
- depends on (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
-
config SH_LCD_MIPI_DSI
bool
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 983280e8d93f..7d991cb9e78e 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -1993,7 +1993,6 @@ config FB_SH_MOBILE_LCDC
select FB_SYS_FOPS
select FB_DEFERRED_IO
select FB_BACKLIGHT
- select SH_MIPI_DSI if SH_LCD_MIPI_DSI
---help---
Frame buffer driver for the on-chip SH-Mobile LCD controller.
diff --git a/drivers/video/fbdev/Makefile b/drivers/video/fbdev/Makefile
index 65fb15075c8f..f6731867dd26 100644
--- a/drivers/video/fbdev/Makefile
+++ b/drivers/video/fbdev/Makefile
@@ -117,7 +117,6 @@ obj-$(CONFIG_FB_SM501) += sm501fb.o
obj-$(CONFIG_FB_UDL) += udlfb.o
obj-$(CONFIG_FB_SMSCUFX) += smscufx.o
obj-$(CONFIG_FB_XILINX) += xilinxfb.o
-obj-$(CONFIG_SH_MIPI_DSI) += sh_mipi_dsi.o
obj-$(CONFIG_FB_SH_MOBILE_MERAM) += sh_mobile_meram.o
obj-$(CONFIG_FB_SH_MOBILE_LCDC) += sh_mobile_lcdcfb.o
obj-$(CONFIG_FB_OMAP) += omap/
diff --git a/drivers/video/fbdev/sh_mipi_dsi.c b/drivers/video/fbdev/sh_mipi_dsi.c
deleted file mode 100644
index 8f6e8ff620d4..000000000000
--- a/drivers/video/fbdev/sh_mipi_dsi.c
+++ /dev/null
@@ -1,587 +0,0 @@
-/*
- * Renesas SH-mobile MIPI DSI support
- *
- * Copyright (C) 2010 Guennadi Liakhovetski <g.liakhovetski@gmx.de>
- *
- * This is free software; you can redistribute it and/or modify
- * it under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- */
-
-#include <linux/bitmap.h>
-#include <linux/clk.h>
-#include <linux/delay.h>
-#include <linux/init.h>
-#include <linux/io.h>
-#include <linux/platform_device.h>
-#include <linux/pm_runtime.h>
-#include <linux/slab.h>
-#include <linux/string.h>
-#include <linux/types.h>
-#include <linux/module.h>
-
-#include <video/mipi_display.h>
-#include <video/sh_mipi_dsi.h>
-#include <video/sh_mobile_lcdc.h>
-
-#include "sh_mobile_lcdcfb.h"
-
-#define SYSCTRL 0x0000
-#define SYSCONF 0x0004
-#define TIMSET 0x0008
-#define RESREQSET0 0x0018
-#define RESREQSET1 0x001c
-#define HSTTOVSET 0x0020
-#define LPRTOVSET 0x0024
-#define TATOVSET 0x0028
-#define PRTOVSET 0x002c
-#define DSICTRL 0x0030
-#define DSIINTE 0x0060
-#define PHYCTRL 0x0070
-
-/* relative to linkbase */
-#define DTCTR 0x0000
-#define VMCTR1 0x0020
-#define VMCTR2 0x0024
-#define VMLEN1 0x0028
-#define VMLEN2 0x002c
-#define CMTSRTREQ 0x0070
-#define CMTSRTCTR 0x00d0
-
-/* E.g., sh7372 has 2 MIPI-DSIs - one for each LCDC */
-#define MAX_SH_MIPI_DSI 2
-
-struct sh_mipi {
- struct sh_mobile_lcdc_entity entity;
-
- void __iomem *base;
- void __iomem *linkbase;
- struct clk *dsit_clk;
- struct platform_device *pdev;
-};
-
-#define to_sh_mipi(e) container_of(e, struct sh_mipi, entity)
-
-static struct sh_mipi *mipi_dsi[MAX_SH_MIPI_DSI];
-
-/* Protect the above array */
-static DEFINE_MUTEX(array_lock);
-
-static struct sh_mipi *sh_mipi_by_handle(int handle)
-{
- if (handle >= ARRAY_SIZE(mipi_dsi) || handle < 0)
- return NULL;
-
- return mipi_dsi[handle];
-}
-
-static int sh_mipi_send_short(struct sh_mipi *mipi, u8 dsi_cmd,
- u8 cmd, u8 param)
-{
- u32 data = (dsi_cmd << 24) | (cmd << 16) | (param << 8);
- int cnt = 100;
-
- /* transmit a short packet to LCD panel */
- iowrite32(1 | data, mipi->linkbase + CMTSRTCTR);
- iowrite32(1, mipi->linkbase + CMTSRTREQ);
-
- while ((ioread32(mipi->linkbase + CMTSRTREQ) & 1) && --cnt)
- udelay(1);
-
- return cnt ? 0 : -ETIMEDOUT;
-}
-
-#define LCD_CHAN2MIPI(c) ((c) < LCDC_CHAN_MAINLCD || (c) > LCDC_CHAN_SUBLCD ? \
- -EINVAL : (c) - 1)
-
-static int sh_mipi_dcs(int handle, u8 cmd)
-{
- struct sh_mipi *mipi = sh_mipi_by_handle(LCD_CHAN2MIPI(handle));
- if (!mipi)
- return -ENODEV;
- return sh_mipi_send_short(mipi, MIPI_DSI_DCS_SHORT_WRITE, cmd, 0);
-}
-
-static int sh_mipi_dcs_param(int handle, u8 cmd, u8 param)
-{
- struct sh_mipi *mipi = sh_mipi_by_handle(LCD_CHAN2MIPI(handle));
- if (!mipi)
- return -ENODEV;
- return sh_mipi_send_short(mipi, MIPI_DSI_DCS_SHORT_WRITE_PARAM, cmd,
- param);
-}
-
-static void sh_mipi_dsi_enable(struct sh_mipi *mipi, bool enable)
-{
- /*
- * enable LCDC data tx, transition to LPS after completion of each HS
- * packet
- */
- iowrite32(0x00000002 | enable, mipi->linkbase + DTCTR);
-}
-
-static void sh_mipi_shutdown(struct platform_device *pdev)
-{
- struct sh_mipi *mipi = to_sh_mipi(platform_get_drvdata(pdev));
-
- sh_mipi_dsi_enable(mipi, false);
-}
-
-static int sh_mipi_setup(struct sh_mipi *mipi, const struct fb_videomode *mode)
-{
- void __iomem *base = mipi->base;
- struct sh_mipi_dsi_info *pdata = mipi->pdev->dev.platform_data;
- u32 pctype, datatype, pixfmt, linelength, vmctr2;
- u32 tmp, top, bottom, delay, div;
- int bpp;
-
- /*
- * Select data format. MIPI DSI is not hot-pluggable, so, we just use
- * the default videomode. If this ever becomes a problem, We'll have to
- * move this to mipi_display_on() above and use info->var.xres
- */
- switch (pdata->data_format) {
- case MIPI_RGB888:
- pctype = 0;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_24;
- pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
- linelength = mode->xres * 3;
- break;
- case MIPI_RGB565:
- pctype = 1;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_16;
- pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
- linelength = mode->xres * 2;
- break;
- case MIPI_RGB666_LP:
- pctype = 2;
- datatype = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
- pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
- linelength = mode->xres * 3;
- break;
- case MIPI_RGB666:
- pctype = 3;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_18;
- pixfmt = MIPI_DCS_PIXEL_FMT_18BIT;
- linelength = (mode->xres * 18 + 7) / 8;
- break;
- case MIPI_BGR888:
- pctype = 8;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_24;
- pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
- linelength = mode->xres * 3;
- break;
- case MIPI_BGR565:
- pctype = 9;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_16;
- pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
- linelength = mode->xres * 2;
- break;
- case MIPI_BGR666_LP:
- pctype = 0xa;
- datatype = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
- pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
- linelength = mode->xres * 3;
- break;
- case MIPI_BGR666:
- pctype = 0xb;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_18;
- pixfmt = MIPI_DCS_PIXEL_FMT_18BIT;
- linelength = (mode->xres * 18 + 7) / 8;
- break;
- case MIPI_YUYV:
- pctype = 4;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16;
- pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
- linelength = mode->xres * 2;
- break;
- case MIPI_UYVY:
- pctype = 5;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16;
- pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
- linelength = mode->xres * 2;
- break;
- case MIPI_YUV420_L:
- pctype = 6;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12;
- pixfmt = MIPI_DCS_PIXEL_FMT_12BIT;
- linelength = (mode->xres * 12 + 7) / 8;
- break;
- case MIPI_YUV420:
- pctype = 7;
- datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12;
- pixfmt = MIPI_DCS_PIXEL_FMT_12BIT;
- /* Length of U/V line */
- linelength = (mode->xres + 1) / 2;
- break;
- default:
- return -EINVAL;
- }
-
- if (!pdata->lane)
- return -EINVAL;
-
- /* reset DSI link */
- iowrite32(0x00000001, base + SYSCTRL);
- /* Hold reset for 100 cycles of the slowest of bus, HS byte and LP clock */
- udelay(50);
- iowrite32(0x00000000, base + SYSCTRL);
-
- /* setup DSI link */
-
- /*
- * T_wakeup = 0x7000
- * T_hs-trail = 3
- * T_hs-prepare = 3
- * T_clk-trail = 3
- * T_clk-prepare = 2
- */
- iowrite32(0x70003332, base + TIMSET);
- /* no responses requested */
- iowrite32(0x00000000, base + RESREQSET0);
- /* request response to packets of type 0x28 */
- iowrite32(0x00000100, base + RESREQSET1);
- /* High-speed transmission timeout, default 0xffffffff */
- iowrite32(0x0fffffff, base + HSTTOVSET);
- /* LP reception timeout, default 0xffffffff */
- iowrite32(0x0fffffff, base + LPRTOVSET);
- /* Turn-around timeout, default 0xffffffff */
- iowrite32(0x0fffffff, base + TATOVSET);
- /* Peripheral reset timeout, default 0xffffffff */
- iowrite32(0x0fffffff, base + PRTOVSET);
- /* Interrupts not used, disable all */
- iowrite32(0, base + DSIINTE);
- /* DSI-Tx bias on */
- iowrite32(0x00000001, base + PHYCTRL);
- udelay(200);
- /* Deassert resets, power on */
- iowrite32(0x03070001 | pdata->phyctrl, base + PHYCTRL);
-
- /*
- * Default = ULPS enable |
- * Contention detection enabled |
- * EoT packet transmission enable |
- * CRC check enable |
- * ECC check enable
- */
- bitmap_fill((unsigned long *)&tmp, pdata->lane);
- tmp |= 0x00003700;
- iowrite32(tmp, base + SYSCONF);
-
- /* setup l-bridge */
-
- /*
- * Enable transmission of all packets,
- * transmit LPS after each HS packet completion
- */
- iowrite32(0x00000006, mipi->linkbase + DTCTR);
- /* VSYNC width = 2 (<< 17) */
- iowrite32((mode->vsync_len << pdata->vsynw_offset) |
- (pdata->clksrc << 16) | (pctype << 12) | datatype,
- mipi->linkbase + VMCTR1);
-
- /*
- * Non-burst mode with sync pulses: VSE and HSE are output,
- * HSA period allowed, no commands in LP
- */
- vmctr2 = 0;
- if (pdata->flags & SH_MIPI_DSI_VSEE)
- vmctr2 |= 1 << 23;
- if (pdata->flags & SH_MIPI_DSI_HSEE)
- vmctr2 |= 1 << 22;
- if (pdata->flags & SH_MIPI_DSI_HSAE)
- vmctr2 |= 1 << 21;
- if (pdata->flags & SH_MIPI_DSI_BL2E)
- vmctr2 |= 1 << 17;
- if (pdata->flags & SH_MIPI_DSI_HSABM)
- vmctr2 |= 1 << 5;
- if (pdata->flags & SH_MIPI_DSI_HBPBM)
- vmctr2 |= 1 << 4;
- if (pdata->flags & SH_MIPI_DSI_HFPBM)
- vmctr2 |= 1 << 3;
- iowrite32(vmctr2, mipi->linkbase + VMCTR2);
-
- /*
- * VMLEN1 = RGBLEN | HSALEN
- *
- * see
- * Video mode - Blanking Packet setting
- */
- top = linelength << 16; /* RGBLEN */
- bottom = 0x00000001;
- if (pdata->flags & SH_MIPI_DSI_HSABM) /* HSALEN */
- bottom = (pdata->lane * mode->hsync_len) - 10;
- iowrite32(top | bottom , mipi->linkbase + VMLEN1);
-
- /*
- * VMLEN2 = HBPLEN | HFPLEN
- *
- * see
- * Video mode - Blanking Packet setting
- */
- top = 0x00010000;
- bottom = 0x00000001;
- delay = 0;
-
- div = 1; /* HSbyteCLK is calculation base
- * HS4divCLK = HSbyteCLK/2
- * HS6divCLK is not supported for now */
- if (pdata->flags & SH_MIPI_DSI_HS4divCLK)
- div = 2;
-
- if (pdata->flags & SH_MIPI_DSI_HFPBM) { /* HBPLEN */
- top = mode->hsync_len + mode->left_margin;
- top = ((pdata->lane * top / div) - 10) << 16;
- }
- if (pdata->flags & SH_MIPI_DSI_HBPBM) { /* HFPLEN */
- bottom = mode->right_margin;
- bottom = (pdata->lane * bottom / div) - 12;
- }
-
- bpp = linelength / mode->xres; /* byte / pixel */
- if ((pdata->lane / div) > bpp) {
- tmp = mode->xres / bpp; /* output cycle */
- tmp = mode->xres - tmp; /* (input - output) cycle */
- delay = (pdata->lane * tmp);
- }
-
- iowrite32(top | (bottom + delay) , mipi->linkbase + VMLEN2);
-
- msleep(5);
-
- /* setup LCD panel */
-
- /* cf. drivers/video/omap/lcd_mipid.c */
- sh_mipi_dcs(pdata->channel, MIPI_DCS_EXIT_SLEEP_MODE);
- msleep(120);
- /*
- * [7] - Page Address Mode
- * [6] - Column Address Mode
- * [5] - Page / Column Address Mode
- * [4] - Display Device Line Refresh Order
- * [3] - RGB/BGR Order
- * [2] - Display Data Latch Data Order
- * [1] - Flip Horizontal
- * [0] - Flip Vertical
- */
- sh_mipi_dcs_param(pdata->channel, MIPI_DCS_SET_ADDRESS_MODE, 0x00);
- /* cf. set_data_lines() */
- sh_mipi_dcs_param(pdata->channel, MIPI_DCS_SET_PIXEL_FORMAT,
- pixfmt << 4);
- sh_mipi_dcs(pdata->channel, MIPI_DCS_SET_DISPLAY_ON);
-
- /* Enable timeout counters */
- iowrite32(0x00000f00, base + DSICTRL);
-
- return 0;
-}
-
-static int mipi_display_on(struct sh_mobile_lcdc_entity *entity)
-{
- struct sh_mipi *mipi = to_sh_mipi(entity);
- struct sh_mipi_dsi_info *pdata = mipi->pdev->dev.platform_data;
- int ret;
-
- pm_runtime_get_sync(&mipi->pdev->dev);
-
- ret = pdata->set_dot_clock(mipi->pdev, mipi->base, 1);
- if (ret < 0)
- goto mipi_display_on_fail1;
-
- ret = sh_mipi_setup(mipi, &entity->def_mode);
- if (ret < 0)
- goto mipi_display_on_fail2;
-
- sh_mipi_dsi_enable(mipi, true);
-
- return SH_MOBILE_LCDC_DISPLAY_CONNECTED;
-
-mipi_display_on_fail1:
- pm_runtime_put_sync(&mipi->pdev->dev);
-mipi_display_on_fail2:
- pdata->set_dot_clock(mipi->pdev, mipi->base, 0);
-
- return ret;
-}
-
-static void mipi_display_off(struct sh_mobile_lcdc_entity *entity)
-{
- struct sh_mipi *mipi = to_sh_mipi(entity);
- struct sh_mipi_dsi_info *pdata = mipi->pdev->dev.platform_data;
-
- sh_mipi_dsi_enable(mipi, false);
-
- pdata->set_dot_clock(mipi->pdev, mipi->base, 0);
-
- pm_runtime_put_sync(&mipi->pdev->dev);
-}
-
-static const struct sh_mobile_lcdc_entity_ops mipi_ops = {
- .display_on = mipi_display_on,
- .display_off = mipi_display_off,
-};
-
-static int __init sh_mipi_probe(struct platform_device *pdev)
-{
- struct sh_mipi *mipi;
- struct sh_mipi_dsi_info *pdata = pdev->dev.platform_data;
- struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- struct resource *res2 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- unsigned long rate, f_current;
- int idx = pdev->id, ret;
-
- if (!res || !res2 || idx >= ARRAY_SIZE(mipi_dsi) || !pdata)
- return -ENODEV;
-
- if (!pdata->set_dot_clock)
- return -EINVAL;
-
- mutex_lock(&array_lock);
- if (idx < 0)
- for (idx = 0; idx < ARRAY_SIZE(mipi_dsi) && mipi_dsi[idx]; idx++)
- ;
-
- if (idx = ARRAY_SIZE(mipi_dsi)) {
- ret = -EBUSY;
- goto efindslot;
- }
-
- mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);
- if (!mipi) {
- ret = -ENOMEM;
- goto ealloc;
- }
-
- mipi->entity.owner = THIS_MODULE;
- mipi->entity.ops = &mipi_ops;
-
- if (!request_mem_region(res->start, resource_size(res), pdev->name)) {
- dev_err(&pdev->dev, "MIPI register region already claimed\n");
- ret = -EBUSY;
- goto ereqreg;
- }
-
- mipi->base = ioremap(res->start, resource_size(res));
- if (!mipi->base) {
- ret = -ENOMEM;
- goto emap;
- }
-
- if (!request_mem_region(res2->start, resource_size(res2), pdev->name)) {
- dev_err(&pdev->dev, "MIPI register region 2 already claimed\n");
- ret = -EBUSY;
- goto ereqreg2;
- }
-
- mipi->linkbase = ioremap(res2->start, resource_size(res2));
- if (!mipi->linkbase) {
- ret = -ENOMEM;
- goto emap2;
- }
-
- mipi->pdev = pdev;
-
- mipi->dsit_clk = clk_get(&pdev->dev, "dsit_clk");
- if (IS_ERR(mipi->dsit_clk)) {
- ret = PTR_ERR(mipi->dsit_clk);
- goto eclktget;
- }
-
- f_current = clk_get_rate(mipi->dsit_clk);
- /* 80MHz required by the datasheet */
- rate = clk_round_rate(mipi->dsit_clk, 80000000);
- if (rate > 0 && rate != f_current)
- ret = clk_set_rate(mipi->dsit_clk, rate);
- else
- ret = rate;
- if (ret < 0)
- goto esettrate;
-
- dev_dbg(&pdev->dev, "DSI-T clk %lu -> %lu\n", f_current, rate);
-
- ret = clk_enable(mipi->dsit_clk);
- if (ret < 0)
- goto eclkton;
-
- mipi_dsi[idx] = mipi;
-
- pm_runtime_enable(&pdev->dev);
- pm_runtime_resume(&pdev->dev);
-
- mutex_unlock(&array_lock);
- platform_set_drvdata(pdev, &mipi->entity);
-
- return 0;
-
-eclkton:
-esettrate:
- clk_put(mipi->dsit_clk);
-eclktget:
- iounmap(mipi->linkbase);
-emap2:
- release_mem_region(res2->start, resource_size(res2));
-ereqreg2:
- iounmap(mipi->base);
-emap:
- release_mem_region(res->start, resource_size(res));
-ereqreg:
- kfree(mipi);
-ealloc:
-efindslot:
- mutex_unlock(&array_lock);
-
- return ret;
-}
-
-static int sh_mipi_remove(struct platform_device *pdev)
-{
- struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- struct resource *res2 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- struct sh_mipi *mipi = to_sh_mipi(platform_get_drvdata(pdev));
- int i, ret;
-
- mutex_lock(&array_lock);
-
- for (i = 0; i < ARRAY_SIZE(mipi_dsi) && mipi_dsi[i] != mipi; i++)
- ;
-
- if (i = ARRAY_SIZE(mipi_dsi)) {
- ret = -EINVAL;
- } else {
- ret = 0;
- mipi_dsi[i] = NULL;
- }
-
- mutex_unlock(&array_lock);
-
- if (ret < 0)
- return ret;
-
- pm_runtime_disable(&pdev->dev);
- clk_disable(mipi->dsit_clk);
- clk_put(mipi->dsit_clk);
-
- iounmap(mipi->linkbase);
- if (res2)
- release_mem_region(res2->start, resource_size(res2));
- iounmap(mipi->base);
- if (res)
- release_mem_region(res->start, resource_size(res));
- kfree(mipi);
-
- return 0;
-}
-
-static struct platform_driver sh_mipi_driver = {
- .remove = sh_mipi_remove,
- .shutdown = sh_mipi_shutdown,
- .driver = {
- .name = "sh-mipi-dsi",
- },
-};
-
-module_platform_driver_probe(sh_mipi_driver, sh_mipi_probe);
-
-MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>");
-MODULE_DESCRIPTION("SuperH / ARM-shmobile MIPI DSI driver");
-MODULE_LICENSE("GPL v2");
diff --git a/include/video/sh_mipi_dsi.h b/include/video/sh_mipi_dsi.h
deleted file mode 100644
index a01f197e6ac1..000000000000
--- a/include/video/sh_mipi_dsi.h
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
- * Public SH-mobile MIPI DSI header
- *
- * Copyright (C) 2010 Guennadi Liakhovetski <g.liakhovetski@gmx.de>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-#ifndef VIDEO_SH_MIPI_DSI_H
-#define VIDEO_SH_MIPI_DSI_H
-
-enum sh_mipi_dsi_data_fmt {
- MIPI_RGB888,
- MIPI_RGB565,
- MIPI_RGB666_LP,
- MIPI_RGB666,
- MIPI_BGR888,
- MIPI_BGR565,
- MIPI_BGR666_LP,
- MIPI_BGR666,
- MIPI_YUYV,
- MIPI_UYVY,
- MIPI_YUV420_L,
- MIPI_YUV420,
-};
-
-#define SH_MIPI_DSI_HSABM (1 << 0)
-#define SH_MIPI_DSI_HBPBM (1 << 1)
-#define SH_MIPI_DSI_HFPBM (1 << 2)
-#define SH_MIPI_DSI_BL2E (1 << 3)
-#define SH_MIPI_DSI_VSEE (1 << 4)
-#define SH_MIPI_DSI_HSEE (1 << 5)
-#define SH_MIPI_DSI_HSAE (1 << 6)
-
-#define SH_MIPI_DSI_HSbyteCLK (1 << 24)
-#define SH_MIPI_DSI_HS6divCLK (1 << 25)
-#define SH_MIPI_DSI_HS4divCLK (1 << 26)
-
-#define SH_MIPI_DSI_SYNC_PULSES_MODE (SH_MIPI_DSI_VSEE | \
- SH_MIPI_DSI_HSEE | \
- SH_MIPI_DSI_HSAE)
-#define SH_MIPI_DSI_SYNC_EVENTS_MODE (0)
-#define SH_MIPI_DSI_SYNC_BURST_MODE (SH_MIPI_DSI_BL2E)
-
-struct sh_mipi_dsi_info {
- enum sh_mipi_dsi_data_fmt data_format;
- int channel;
- int lane;
- unsigned long flags;
- u32 clksrc;
- u32 phyctrl; /* for extra setting */
- unsigned int vsynw_offset;
- int (*set_dot_clock)(struct platform_device *pdev,
- void __iomem *base,
- int enable);
-};
-
-#endif
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v5 08/46] hwmon: pwm-fan: use pwm_get_args() where appropriate
From: Thierry Reding @ 2016-04-04 15:20 UTC (permalink / raw)
To: Boris Brezillon
Cc: Guenter Roeck, linux-pwm-u79uwXL29TY76Z2rM5mHXA, Mike Turquette,
Stephen Boyd, linux-clk-u79uwXL29TY76Z2rM5mHXA, Mark Brown,
Liam Girdwood, Kamil Debski, lm-sensors-GZX6beZjE8VD60Wz+7aTrA,
Jean Delvare, Dmitry Torokhov, linux-input-u79uwXL29TY76Z2rM5mHXA,
Bryan Wu, Richard Purdie, Jacek Anaszewski,
linux-leds-u79uwXL29TY76Z2rM5mHXA, Maxime Ripard, Chen-Yu Tsai,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Joachim Eastwood,
Thomas Petazzoni, Heiko Stuebner,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jingoo Han,
Lee Jones <lee.jo>
In-Reply-To: <20160331090709.5c3bdf70@bbrezillon>
[-- Attachment #1: Type: text/plain, Size: 2717 bytes --]
On Thu, Mar 31, 2016 at 09:07:09AM +0200, Boris Brezillon wrote:
> Hi Guenter,
>
> On Wed, 30 Mar 2016 15:52:44 -0700
> Guenter Roeck <linux@roeck-us.net> wrote:
>
> > On Wed, Mar 30, 2016 at 10:03:31PM +0200, Boris Brezillon wrote:
> > > The PWM framework has clarified the concept of reference PWM config
> > > (the platform dependent config retrieved from the DT or the PWM
> > > lookup table) and real PWM state.
> > >
> > > Use pwm_get_args() when the PWM user wants to retrieve this reference
> > > config and not the current state.
> > >
> > > This is part of the rework allowing the PWM framework to support
> > > hardware readout and expose real PWM state even when the PWM has
> > > just been requested (before the user calls pwm_config/enable/disable()).
> > >
> > > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > > ---
> > > drivers/hwmon/pwm-fan.c | 19 +++++++++++++------
> > > 1 file changed, 13 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> > > index 3e23003..82c5656 100644
> > > --- a/drivers/hwmon/pwm-fan.c
> > > +++ b/drivers/hwmon/pwm-fan.c
> > > @@ -40,15 +40,18 @@ struct pwm_fan_ctx {
> > >
> > > static int __set_pwm(struct pwm_fan_ctx *ctx, unsigned long pwm)
> > > {
> > > + struct pwm_args pargs = { };
> >
> > Hi Boris,
> >
> > I guess I am missing some context; sorry for that. Unfortunately,
> > I did not easily find an explanation, so please bear with me.
> >
> > Two questions: Why do we need a local copy of struct pwm_args instead
> > of a pointer to it ? If it can change while being used, isn't it
> > inconsistent anyway ?
>
> It cannot change after pwm_get() is called. For the reason behind
> prototype: I just followed the Thierry's proposal, but I'm perfectly
> fine returning a const struct pwm_args pointer intead of passing
> pwm_args as a parameter.
>
> Thierry, what's your opinion?
I do prefer the current variant because it is more consistent with the
new atomic API, even if not strictly necessary because of the immutable
data.
> > Also, assuming the local copy is necessary, why initialize pargs ?
> > After all, pwm_get_args() just overwrites it.
>
> It's a leftover from a previous version where pwm_get_args was
> implemented this way:
>
> static inline void pwm_get_args(pwm, args)
> {
> if (pwm)
> *args = pwm->args
> }
>
> and this implementation was generating a lot of 'uninitialized
> variable' warnings.
>
> I just decided to drop the 'if (pwm)' test, because, IMO, this
> should be checked way before calling pwm_get_args() is called.
Sounds fine to me.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox