* Re: [PATCH v3] video: Versatile Express DVI output driver
From: Pawel Moll @ 2012-11-19 13:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1351612732.6199.4.camel@hornet>
Hi Arnd, Olof,
One of the important parts of the VE update is the display muxer
driver...
On Tue, 2012-10-30 at 15:58 +0000, Pawel Moll wrote:
> Hello again Florian,
>
> On Tue, 2012-10-16 at 17:30 +0100, Pawel Moll wrote:
> > Versatile Express' DVI video output can be connected to one the three
> > sources - motherboard's CLCD controller or a video signal generated
> > by one of the daughterboards.
> >
> > This driver configures the muxer FPGA so the output displays content
> > of one of the framebuffers in the system (0 by default, can be changed
> > by user writing to the "fb" sysfs attribute of the muxfpga device).
> >
> > It will also set up the display formatter mode and keep it up
> > to date with mode changes requested by the user (eg. with fbset
> > tool).
> >
> > Signed-off-by: Pawel Moll <pawel.moll@arm.com>
>
> It's just a polite a friendly nag regarding this patch - it's been 2
> weeks since I posted this version (and 1.5 month since the first one)...
>
> Does it look good enough or completely wrong?
I've asked the framebuffer people 3 times about this but got no comment
whatsoever:
https://patchwork.kernel.org/patch/1473091/
https://patchwork.kernel.org/patch/1601781/
so I assume there is no problem with merging this code ;-) Therefore,
would you be so kind to pull this single patch? Without it the 3.8
kernel will not be able to show anything on VE's display...
The following changes since commit f4a75d2eb7b1e2206094b901be09adb31ba63681:
Linux 3.7-rc6 (2012-11-16 17:42:40 -0800)
are available in the git repository at:
git://git.linaro.org/people/pawelmoll/linux.git vexpress-drivers2
for you to fetch changes up to 357e174056d44c6614afec6c5ace1c2aea2ef0bb:
video: Versatile Express DVI output driver (2012-11-19 11:12:40 +0000)
This is the very last thing for 3.8 merge window from me.
Thanks and regards
Paweł
^ permalink raw reply
* RE: [PATCH 2/2] video: exynos-mipi-dsi: Adding DT support to exynos mipi driver
From: Inki Dae @ 2012-11-19 12:31 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1352547285-12302-3-git-send-email-shaik.ameer@samsung.com>
Hi,
Please, post your dts and dtsi file you used also. I think this patch should
be separated into SoC and Board-specific parts. And below is quick review.
Thanks,
Inki Dae
> -----Original Message-----
> From: Shaik Ameer Basha [mailto:shaik.ameer@samsung.com]
> Sent: Saturday, November 10, 2012 8:35 PM
> To: linux-fbdev@vger.kernel.org
> Cc: inki.dae@samsung.com; dh09.lee@samsung.com; FlorianSchandinat@gmx.de;
> s.nawrocki@samsung.com; kgene.kim@samsung.com
> Subject: [PATCH 2/2] video: exynos-mipi-dsi: Adding DT support to exynos
> mipi driver
>
> This patch adds the DT support for the exynos mipi-dsi driver.
> for DT support mipi device node should supply the following
> information to the mipi-dsi driver.
> 1] dsim_config information
> 2] d-phy setting information
> 3] lcd poweron, reset information
> 4] fb_videomode information
>
> Change-Id: I93005636a7825b0c5ef4832dd17a2809d0aeda1d
> Signed-off-by: Shaik Ameer Basha <shaik.ameer@samsung.com>
> ---
> .../devicetree/bindings/video/exynos/mipi-dsi.txt | 185 +++++++
> drivers/video/exynos/exynos_mipi_dsi.c | 573
> +++++++++++++++++++-
> include/video/exynos_mipi_dsim.h | 27 +
> 3 files changed, 765 insertions(+), 20 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/video/exynos/mipi-
> dsi.txt
>
> diff --git a/Documentation/devicetree/bindings/video/exynos/mipi-dsi.txt
> b/Documentation/devicetree/bindings/video/exynos/mipi-dsi.txt
> new file mode 100644
> index 0000000..6445eac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/exynos/mipi-dsi.txt
> @@ -0,0 +1,185 @@
> +* Samsung Exynos MIPI-DSI bindings
> +
> +Properties for MIPI-DSI node ::
> +===============> +- compatible: should be "samsung,exynos5-mipi"
> +
Make sure mipi type. "samsung, exynos5250-mipi-dsi"
> +- reg: should contain mipi-dsi physical address location and length.
> +
> +- interrupts: should contain mipi-dsi interrupt number
> +
> +- enabled: Describes whether MIPI DSI got enabled in uboot
> +
Maybe board-specific.
> +- mipi-lcd: phandle to lcd specific information. It can be anything
> + specific to lcd driver.
> +
Ditto.
> +- mipi-phy: phandle to D-PHY node.
> +
> +- mipi-config: subnode for mipi config information
> + - auto_flush: enable or disable Auto flush of MD FIFO using VSYNC
> pulse
> + - eot_disable: enable or disable EoT packet in HS mode
> + - auto_vertical_cnt: specifies auto vertical count mode.
> + In Video mode, the vertical line transition uses line
> counter
> + configured by VSA, VBP, and Vertical resolution. If this bit
> is
> + set to '1', the line counter does not use VSA and VBP
> registers.
> + In command mode, this property is ignored.
> + - hse: set horizontal sync event mode.
> + In VSYNC pulse and Vporch area, MIPI DSI master transfers
> only
> + HSYNC start packet to MIPI DSI slave at MIPI DSI spec1.1r02.
> + This bit transfers HSYNC end packet in VSYNC pulse and
> Vporch
> + area. In command mode, this property is ignored.
> + - hfp: specifies HFP disable mode.
> + If this property is set, DSI master ignores HFP area in
> + VIDEO mode. In command mode, this property is ignored.
> + - hbp: specifies HBP disable mode.
> + If this property is set, DSI master ignores HBP area in
> + VIDEO mode. In command mode, this property is ignored.
> + - hsa: specifies HSA disable mode.
> + If this property is set, DSI master ignores HSA area in
> + VIDEO mode. In command mode, this property is ignored.
> + - cmd_allow: specifies the number of horizontal lines, where
> command
> + packet transmission is allowed after Stable VFP period.
> + - e_interface: specifies interface to be used.(CPU or RGB interface)
> + - e_virtual_ch: specifies virtual channel number that main or
> + sub display uses.
> + - e_pixel_format: specifies pixel stream format for main or sub
> display.
> + - e_burst_mode: selects Burst mode in Video mode.
> + In Non-burst mode, RGB data area is filled with RGB data
> + and NULL packets, according to input bandwidth of RGB
> interface.
> + In Burst mode, RGB data area is filled with RGB data only.
> + - e_no_data_lane: specifies data lane count to be used by Master.
> + - e_byte_clk: select byte clock source. (it must be
> DSIM_PLL_OUT_DIV8)
> + DSIM_EXT_CLK_DIV8 and DSIM_EXT_CLK_BYPASSS are not
supported.
> + - pll_stable_time: specifies the PLL Timer for stability of the
> + generated clock(System clock cycle base). If the timer value
> + goes to 0x00000000, the clock stable bit of status and
> interrupt
> + register is set.
> + - esc_clk: specifies escape clock frequency for getting the escape
> clock
> + prescaler value.
> + - stop_holding_cnt: specifies the interval value between
> transmitting
> + read packet(or write "set_tear_on" command) and BTA request.
> + after transmitting read packet or write "set_tear_on"
> command,
> + BTA requests to D-PHY automatically. this counter value
> + specifies the interval between them.
> + - bta_timeout: specifies the timer for BTA.
> + this register specifies time out from BTA request to change
> + the direction with respect to Tx escape clock.
> + - rx_timeout: specifies the timer for LP Rx mode timeout.
> + this register specifies time out on how long RxValid
> deasserts,
> + after RxLpdt asserts with respect to Tx escape clock.
> + - RxValid specifies Rx data valid indicator.
> + - RxLpdt specifies an indicator that D-PHY is under RxLpdt
> mode.
> + - RxValid and RxLpdt specifies signal from D-PHY.
> +
> +- display-mode: timing and resolution information of the panel used. As
> per
> + current exynos mipi-dsi driver need, only some fields from
> + struct fb_videomode are defined in this node.
> + - xres: horizontal resolution (active frame width)
> + - yres: vertical resolution (active frame height).
> + - left_margin: Horizontal Back Porch (Number of PIXCLK pulses
> between
> + HSYNC signal and the first valid pixel data.
> + - right_margin: Horizontal Front Porch (Number of PIXCLK between
> + last valid pixel data in the line and the next HSYNC pulse).
> + - upper_margin: Vertical Back Porch (Number of lines (HSYNC pulses)
> + from when a VSYNC signal is asserted and the first valid
> line).
> + - lower_margin: Vertical Front Porch (Number of lines (HSYNC pulses)
> + between last valid line of the frame and the next VSYNC
> Pulse).
> + - hsync_len: Hsync pulse width (Number of PIXCLK pulses when a
> HSYNC
> + signal is active).
> + - vsync_len: Vsync pulse width (Number of HSYNC pulses when a VSYNC
> + signal is active).
> +
All things above are specific to board.
> +Properties for D-PHY node ::
> +==============> + Instead of passing D-PHY related callbacks as part of platform data,
> +we can pass the phy nodes to the mipi driver. Depending on the type of
> PHY
> +settings, we can implement multiple PHY node types and corresponding
> +enable/disable/reset callbacks in the driver itself. Currently we support
> +only one type of PHY node.
> +
> +D-PHY node type1:
> +------------------
> +- compatible: "samsung,exynos-mipi-phy-type1"
> +- reg_enable_dphy: should contain physical address location of
> + D-PHY enable register
> +- mask_enable_dphy: should contain the mask for D-PHY enable register
> +- reg_reset_dsim: should contain physical address location of
> + D-PHY DSIM reset register
> +- mask_reset_dsim: should contain the mask for D-PHY DSIM reset register
> +
> +MIPI-LCD node ::
> +========> + Apart from the following three properties, driver specific
> +properties can be sent through this node. The following example sends
> +some more properties for driver's use.
> +
> +- lcd-name: name of the device to use with this device
> +- id: id of device to be registered (default -1 in case not specified)
> +- bus-id: bus id for identifing connected bus and this bus id should be
> + same as id of mipi_dsim_device (default -1 incase not specified)
> +
> +Example:
> +--------
> + mipi_lcd: mipi-lcd@toshiba {
> + lcd-name = "tc358764";
> + id = <0>;
> + enabled = <1>;
> + reset-delay = <120>;
> + power-on-delay = <25>;
> + power-off-delay = <200>;
> + gpio-poweron = <&gpx1 5 0 0 0>;
> + };
> +
Board-specific.
> + mipi_dsim_phy: mipi-phy@exynos5250 {
> + compatible = "samsung-exynos,mipi-phy-type1";
> + reg_enable_dphy = <0x10040714>;
> + mask_enable_dphy = <0x00000001>;
> + reg_reset_dsim = <0x10040714>;
> + mask_reset_dsim = <0x00000004>;
> + };
> +
SoC-specific.
> + mipi {
> + compatible = "samsung,exynos-mipi";
mipi-dsi {
compatible = "samsung, exynos5250-mipi-dsi";
> + reg = <0x14500000 0x10000>;
> + interrupts = <0 82 0>;
> +
SoC-specific.
> + mipi-lcd = <&mipi_lcd>;
Board-specific.
> + mipi-phy = <&mipi_dsim_phy>;
SoC-specific.
> + enabled = <0>;
> +
Board-specific.
> + mipi-config {
> + e_interface = <1>;
> + e_pixel_format = <7>;
> + auto_flush = <0>;
> + eot_disable = <0>;
> + auto_vertical_cnt = <0>;
> + hse = <0>;
> + hfp = <0>;
> + hbp = <0>;
> + hsa = <0>;
> + e_no_data_lane = <3>;
> + e_byte_clk = <0>;
> + e_burst_mode = <3>;
> + p = <3>;
> + m = <115>;
> + s = <1>;
> + pll_stable_time = <500>;
> + esc_clk = <400000>;
> + stop_holding_cnt =<0x0f>;
> + bta_timeout = <0xff>;
> + rx_timeout = <0xffff>;
> + e_virtual_ch = <0>;
> + cmd_allow = <0xf>;
> + };
> +
> + panel-info {
> + left_margin = <0x4>;
> + right_margin = <0x4>;
> + upper_margin = <0x4>;
> + lower_margin = <0x4>;
> + hsync_len = <0x4>;
> + vsync_len = <0x4>;
> + xres = <1280>;
> + yres = <800>;
> + };
All things above are specific to board also.
> + };
> diff --git a/drivers/video/exynos/exynos_mipi_dsi.c
> b/drivers/video/exynos/exynos_mipi_dsi.c
> index ae20bc3..667857b 100755
> --- a/drivers/video/exynos/exynos_mipi_dsi.c
> +++ b/drivers/video/exynos/exynos_mipi_dsi.c
> @@ -32,6 +32,7 @@
> #include <linux/notifier.h>
> #include <linux/regulator/consumer.h>
> #include <linux/pm_runtime.h>
> +#include <linux/lcd.h>
>
> #include <video/exynos_mipi_dsim.h>
>
> @@ -43,6 +44,8 @@
> struct mipi_dsim_ddi {
> int bus_id;
> struct list_head list;
> + struct device_node *ofnode_dsim_lcd_dev;
> + struct device_node *ofnode_dsim_dphy;
> struct mipi_dsim_lcd_device *dsim_lcd_dev;
> struct mipi_dsim_lcd_driver *dsim_lcd_drv;
> };
> @@ -181,6 +184,36 @@ static int exynos_mipi_dsi_blank_mode(struct
> mipi_dsim_device *dsim, int power)
> return 0;
> }
>
> +struct mipi_dsim_ddi *exynos_mipi_dsi_find_lcd_driver(
> + struct mipi_dsim_lcd_device *lcd_dev)
> +{
> + struct mipi_dsim_ddi *dsim_ddi, *next;
> + struct mipi_dsim_lcd_driver *lcd_drv;
> +
> + mutex_lock(&mipi_dsim_lock);
> +
> + list_for_each_entry_safe(dsim_ddi, next, &dsim_ddi_list, list) {
> + if (!dsim_ddi)
> + goto out;
> +
> + lcd_drv = dsim_ddi->dsim_lcd_drv;
> + if (!lcd_drv)
> + continue;
> +
> + if ((strcmp(lcd_dev->name, lcd_drv->name)) = 0) {
> + mutex_unlock(&mipi_dsim_lock);
> + return dsim_ddi;
> + }
> +
> + list_del(&dsim_ddi->list);
> + kfree(dsim_ddi);
> + }
> +
> +out:
> + mutex_unlock(&mipi_dsim_lock);
> + return NULL;
> +}
> +
> int exynos_mipi_dsi_register_lcd_device(struct mipi_dsim_lcd_device
> *lcd_dev)
> {
> struct mipi_dsim_ddi *dsim_ddi;
> @@ -190,13 +223,17 @@ int exynos_mipi_dsi_register_lcd_device(struct
> mipi_dsim_lcd_device *lcd_dev)
> return -EFAULT;
> }
>
> - dsim_ddi = kzalloc(sizeof(struct mipi_dsim_ddi), GFP_KERNEL);
> + dsim_ddi = exynos_mipi_dsi_find_lcd_driver(lcd_dev);
> if (!dsim_ddi) {
> - pr_err("failed to allocate dsim_ddi object.\n");
> - return -ENOMEM;
> + dsim_ddi = kzalloc(sizeof(struct mipi_dsim_ddi),
GFP_KERNEL);
> + if (!dsim_ddi) {
> + pr_err("failed to allocate dsim_ddi object.\n");
> + return -ENOMEM;
> + }
> }
>
> dsim_ddi->dsim_lcd_dev = lcd_dev;
> + dsim_ddi->bus_id = lcd_dev->bus_id;
>
> mutex_lock(&mipi_dsim_lock);
> list_add_tail(&dsim_ddi->list, &dsim_ddi_list);
> @@ -253,11 +290,26 @@ int exynos_mipi_dsi_register_lcd_driver(struct
> mipi_dsim_lcd_driver *lcd_drv)
>
> dsim_ddi = exynos_mipi_dsi_find_lcd_device(lcd_drv);
> if (!dsim_ddi) {
> - pr_err("mipi_dsim_ddi object not found.\n");
> - return -EFAULT;
> - }
> + /*
> + * If driver specific device is not registered then create a
> + * dsim_ddi object, fill the driver information and add to
> the
> + * end of the dsim_ddi_list list
> + */
> + dsim_ddi = kzalloc(sizeof(struct mipi_dsim_ddi),
GFP_KERNEL);
> + if (!dsim_ddi) {
> + pr_err("failed to allocate dsim_ddi object.\n");
> + return -ENOMEM;
> + }
> +
> + dsim_ddi->dsim_lcd_drv = lcd_drv;
>
> - dsim_ddi->dsim_lcd_drv = lcd_drv;
> + mutex_lock(&mipi_dsim_lock);
> + list_add_tail(&dsim_ddi->list, &dsim_ddi_list);
> + mutex_unlock(&mipi_dsim_lock);
> +
> + } else {
> + dsim_ddi->dsim_lcd_drv = lcd_drv;
> + }
>
> pr_info("registered panel driver(%s) to mipi-dsi driver.\n",
> lcd_drv->name);
> @@ -329,6 +381,372 @@ static struct mipi_dsim_master_ops master_ops = {
> .set_blank_mode = exynos_mipi_dsi_blank_mode,
> };
>
> +struct device_node *exynos_mipi_find_ofnode_dsim_phy(
> + struct platform_device *pdev)
> +{
> + struct device_node *dn, *dn_dphy;
> + const __be32 *prop;
> +
> + dn = pdev->dev.of_node;
> + prop = of_get_property(dn, "mipi-phy", NULL);
> + if (NULL = prop) {
> + dev_err(&pdev->dev, "Could not find property mipi-phy\n");
> + return NULL;
> + }
> +
> + dn_dphy = of_find_node_by_phandle(be32_to_cpup(prop));
> + if (NULL = dn_dphy) {
> + dev_err(&pdev->dev, "Could not find node\n");
> + return NULL;
> + }
> +
> + return dn_dphy;
> +}
> +
> +struct device_node *exynos_mipi_find_ofnode_lcd_device(
> + struct platform_device *pdev)
> +{
> + struct device_node *dn, *dn_lcd_panel;
> + const __be32 *prop;
> +
> + dn = pdev->dev.of_node;
> + prop = of_get_property(dn, "mipi-lcd", NULL);
> + if (NULL = prop) {
> + dev_err(&pdev->dev, "could not find property mipi-lcd\n");
> + return NULL;
> + }
> +
> + dn_lcd_panel = of_find_node_by_phandle(be32_to_cpup(prop));
> + if (NULL = dn_lcd_panel) {
> + dev_err(&pdev->dev, "could not find node\n");
> + return NULL;
> + }
> +
> + return dn_lcd_panel;
> +}
> +
> +static void exynos_mipi_dsim_enable_d_phy_type1(
> + struct platform_device *pdev,
> + bool enable)
> +{
> + struct mipi_dsim_device *dsim = (struct mipi_dsim_device *)
> + platform_get_drvdata(pdev);
> + struct mipi_dsim_phy_config_type1 *dphy_cfg_type1 > +
&dsim->dsim_phy_config->phy_cfg_type1;
> + u32 reg_enable;
> +
> + reg_enable = __raw_readl(dphy_cfg_type1->reg_enable_dphy);
> + reg_enable &= ~(dphy_cfg_type1->ctrlbit_enable_dphy);
> +
> + if (enable)
> + reg_enable |= dphy_cfg_type1->ctrlbit_enable_dphy;
> +
> + __raw_writel(reg_enable, dphy_cfg_type1->reg_enable_dphy);
> +}
> +
> +static void exynos_mipi_dsim_reset_type1(
> + struct platform_device *pdev,
> + bool enable)
> +{
> + struct mipi_dsim_device *dsim = (struct mipi_dsim_device *)
> + platform_get_drvdata(pdev);
> + struct mipi_dsim_phy_config_type1 *dphy_cfg_type1 > +
&dsim->dsim_phy_config->phy_cfg_type1;
> + u32 reg_reset;
> +
> + reg_reset = __raw_readl(dphy_cfg_type1->reg_reset_dsim);
> + reg_reset &= ~(dphy_cfg_type1->ctrlbit_reset_dsim);
> +
> + if (enable)
> + reg_reset |= dphy_cfg_type1->ctrlbit_reset_dsim;
> +
> + __raw_writel(reg_reset, dphy_cfg_type1->reg_reset_dsim);
> +}
> +
> +static int exynos_mipi_dsim_phy_init_type1(
> + struct platform_device *pdev,
> + bool on_off)
> +{
> + exynos_mipi_dsim_enable_d_phy_type1(pdev, on_off);
> + exynos_mipi_dsim_reset_type1(pdev, on_off);
> + return 0;
> +}
> +
> +static int parse_u32_property(struct device_node *np, char *name,
> + u32 *result)
> +{
> + if (of_property_read_u32(np, name, result)) {
> + pr_err("not able to find property: %s\n", name);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int exynos_mipi_parse_ofnode_dsim_phy_type1(
> + struct platform_device *pdev,
> + struct mipi_dsim_phy_config_type1 *dphy_cfg_type1,
> + struct device_node *np)
> +{
> + struct mipi_dsim_device *dsim = (struct mipi_dsim_device *)
> + platform_get_drvdata(pdev);
> + u32 paddr_phy_enable, paddr_dsim_reset;
> + int ret = 0;
> +
> + ret |= parse_u32_property(np, "reg_enable_dphy", &paddr_phy_enable);
> + ret |= parse_u32_property(np, "reg_reset_dsim", &paddr_dsim_reset);
> + ret |= parse_u32_property(np, "mask_enable_dphy",
> +
&dphy_cfg_type1->ctrlbit_enable_dphy);
> + ret |= parse_u32_property(np, "mask_reset_dsim",
> +
&dphy_cfg_type1->ctrlbit_reset_dsim);
> +
> + if (ret) {
> + pr_err("%s: error reading phy node properties\n", __func__);
> + return -EINVAL;
> + }
> +
> + dphy_cfg_type1->reg_enable_dphy = ioremap(paddr_phy_enable, SZ_4);
> + if (!dphy_cfg_type1->reg_enable_dphy)
> + return -EINVAL;
> +
> + dphy_cfg_type1->reg_reset_dsim = ioremap(paddr_dsim_reset, SZ_4);
> + if (!dphy_cfg_type1->reg_reset_dsim)
> + goto err_ioremap;
> +
> + dsim->pd->phy_enable = exynos_mipi_dsim_phy_init_type1;
> +
> + return 0;
> +
> +err_ioremap:
> + iounmap(dphy_cfg_type1->reg_enable_dphy);
> + return -EINVAL;
> +}
> +
> +static struct mipi_dsim_phy_config *exynos_mipi_parse_ofnode_dsim_phy(
> + struct platform_device *pdev,
> + struct device_node *np)
> +{
> + struct mipi_dsim_phy_config *mipi_dphy_config;
> + const char *compatible;
> +
> + mipi_dphy_config = devm_kzalloc(&pdev->dev,
> + sizeof(struct mipi_dsim_phy_config), GFP_KERNEL);
> + if (!mipi_dphy_config) {
> + dev_err(&pdev->dev,
> + "failed to allocate mipi_dsim_phy_config
object.\n");
> + return NULL;
> + }
> +
> + if (of_property_read_string(np, "compatible", &compatible)) {
> + dev_err(&pdev->dev, "compatible property not found");
> + return NULL;
> + }
> +
> + /* try to find the phy node type from compatible string */
> + if (!strcmp(compatible, "samsung-exynos,mipi-phy-type1"))
> + mipi_dphy_config->type = MIPI_DSIM_PHY_CONFIG_TYPE1;
> + else
> + mipi_dphy_config->type = -1;
> +
> + /* parse the phy node as per its type */
> + switch (mipi_dphy_config->type) {
> + case MIPI_DSIM_PHY_CONFIG_TYPE1:
> + if (exynos_mipi_parse_ofnode_dsim_phy_type1(
> + pdev, &mipi_dphy_config->phy_cfg_type1, np))
> + return NULL;
> + break;
> + default:
> + dev_err(&pdev->dev, "mipi phy - unknown type");
> + return NULL;
> + }
> +
> + return mipi_dphy_config;
> +}
> +
> +static struct mipi_dsim_lcd_device *exynos_mipi_parse_ofnode_lcd(
> + struct platform_device *pdev, struct device_node *np)
> +{
> + struct mipi_dsim_lcd_device *active_mipi_dsim_lcd_device;
> + struct lcd_platform_data *active_lcd_platform_data;
> + const char *lcd_name;
> +
> + active_mipi_dsim_lcd_device = devm_kzalloc(&pdev->dev,
> + sizeof(struct mipi_dsim_lcd_device), GFP_KERNEL);
> + if (!active_mipi_dsim_lcd_device) {
> + dev_err(&pdev->dev,
> + "failed to allocate active_mipi_dsim_lcd_device object.\n");
> + return NULL;
> + }
> +
> + if (of_property_read_string(np, "lcd-name", &lcd_name)) {
> + dev_err(&pdev->dev, "lcd name property not found");
> + return NULL;
> + }
> +
> + active_mipi_dsim_lcd_device->name = (char *)lcd_name;
> +
> + if (of_property_read_u32(np, "id", &active_mipi_dsim_lcd_device-
> >id))
> + active_mipi_dsim_lcd_device->id = -1;
> +
> + if (of_property_read_u32(np, "bus-id",
> +
&active_mipi_dsim_lcd_device->bus_id))
> + active_mipi_dsim_lcd_device->bus_id = -1;
> +
> + active_lcd_platform_data = devm_kzalloc(&pdev->dev,
> + sizeof(struct lcd_platform_data),
GFP_KERNEL);
> + if (!active_lcd_platform_data) {
> + dev_err(&pdev->dev,
> + "failed to allocate active_lcd_platform_data object.\n");
> + return NULL;
> + }
> +
> + /* store the lcd node pointer for futher use in lcd driver */
> + active_lcd_platform_data->pdata = (void *) np;
> + active_mipi_dsim_lcd_device->platform_data > + (void *)active_lcd_platform_data;
> +
> + return active_mipi_dsim_lcd_device;
> +}
> +
> +static int exynos_mipi_parse_ofnode_config(struct platform_device *pdev,
> + struct device_node *np, struct mipi_dsim_config
*dsim_config)
> +{
> + unsigned int u32Val;
> +
> + if (parse_u32_property(np, "e_interface", &u32Val))
> + return -EINVAL;
> + dsim_config->e_interface = (enum mipi_dsim_interface_type)u32Val;
> +
> + if (parse_u32_property(np, "e_pixel_format", &u32Val))
> + return -EINVAL;
> + dsim_config->e_pixel_format = (enum mipi_dsim_pixel_format)u32Val;
> +
> + if (parse_u32_property(np, "auto_flush", &u32Val))
> + return -EINVAL;
> + dsim_config->auto_flush = (unsigned char)u32Val;
> +
> + if (parse_u32_property(np, "eot_disable", &u32Val))
> + return -EINVAL;
> + dsim_config->eot_disable = (unsigned char)u32Val;
> +
> + if (parse_u32_property(np, "auto_vertical_cnt", &u32Val))
> + return -EINVAL;
> + dsim_config->auto_vertical_cnt = (unsigned char)u32Val;
> +
> + if (parse_u32_property(np, "hse", &u32Val))
> + return -EINVAL;
> + dsim_config->hse = (unsigned char)u32Val;
> +
> + if (parse_u32_property(np, "hfp", &u32Val))
> + return -EINVAL;
> + dsim_config->hfp = (unsigned char)u32Val;
> +
> + if (parse_u32_property(np, "hbp", &u32Val))
> + return -EINVAL;
> + dsim_config->hbp = (unsigned char)u32Val;
> +
> + if (parse_u32_property(np, "hsa", &u32Val))
> + return -EINVAL;
> + dsim_config->hsa = (unsigned char)u32Val;
> +
> + if (parse_u32_property(np, "e_no_data_lane", &u32Val))
> + return -EINVAL;
> + dsim_config->e_no_data_lane = (enum
> mipi_dsim_no_of_data_lane)u32Val;
> +
> + if (parse_u32_property(np, "e_byte_clk", &u32Val))
> + return -EINVAL;
> + dsim_config->e_byte_clk = (enum mipi_dsim_byte_clk_src)u32Val;
> +
> + if (parse_u32_property(np, "e_burst_mode", &u32Val))
> + return -EINVAL;
> + dsim_config->e_burst_mode = (enum mipi_dsim_burst_mode_type)u32Val;
> +
> + if (parse_u32_property(np, "p", &u32Val))
> + return -EINVAL;
> + dsim_config->p = (unsigned char)u32Val;
> +
> + if (parse_u32_property(np, "m", &u32Val))
> + return -EINVAL;
> + dsim_config->m = (unsigned short)u32Val;
> +
> + if (parse_u32_property(np, "s", &u32Val))
> + return -EINVAL;
> + dsim_config->s = (unsigned char)u32Val;
> +
> + if (parse_u32_property(np, "pll_stable_time", &u32Val))
> + return -EINVAL;
> + dsim_config->pll_stable_time = (unsigned int)u32Val;
> +
> + if (parse_u32_property(np, "esc_clk", &u32Val))
> + return -EINVAL;
> + dsim_config->esc_clk = (unsigned long)u32Val;
> +
> + if (parse_u32_property(np, "stop_holding_cnt", &u32Val))
> + return -EINVAL;
> + dsim_config->stop_holding_cnt = (unsigned short)u32Val;
> +
> + if (parse_u32_property(np, "bta_timeout", &u32Val))
> + return -EINVAL;
> + dsim_config->bta_timeout = (unsigned char)u32Val;
> +
> + if (parse_u32_property(np, "rx_timeout", &u32Val))
> + return -EINVAL;
> + dsim_config->rx_timeout = (unsigned short)u32Val;
> +
> + if (parse_u32_property(np, "e_virtual_ch", &u32Val))
> + return -EINVAL;
> + dsim_config->e_virtual_ch = (enum mipi_dsim_virtual_ch_no)u32Val;
> +
> + if (parse_u32_property(np, "cmd_allow", &u32Val))
> + return -EINVAL;
> + dsim_config->cmd_allow = (unsigned char)u32Val;
> +
> + return 0;
> +}
> +
> +static int exynos_mipi_parse_ofnode_panel_info(struct platform_device
> *pdev,
> + struct device_node *np, struct fb_videomode *vm)
> +{
> + int ret = 0;
> +
> + ret |= parse_u32_property(np, "left_margin", &vm->left_margin);
> + ret |= parse_u32_property(np, "right_margin", &vm->right_margin);
> + ret |= parse_u32_property(np, "upper_margin", &vm->upper_margin);
> + ret |= parse_u32_property(np, "lower_margin", &vm->lower_margin);
> + ret |= parse_u32_property(np, "hsync_len", &vm->hsync_len);
> + ret |= parse_u32_property(np, "vsync_len", &vm->vsync_len);
> + ret |= parse_u32_property(np, "xres", &vm->xres);
> + ret |= parse_u32_property(np, "yres", &vm->yres);
> + if (ret)
> + pr_err("%s: error reading fb_videomodeproperties\n",
> __func__);
> +
> + return ret;
> +}
> +
> +static int exynos_mipi_parse_ofnode(struct platform_device *pdev,
> + struct mipi_dsim_config *dsim_config, struct fb_videomode
> *panel_info)
> +{
> + struct device_node *np_dsim_config, *np_panel_info;
> + struct device_node *np = pdev->dev.of_node;
> +
> + np_dsim_config = of_find_node_by_name(np, "mipi-config");
> + if (!np_dsim_config)
> + return -EINVAL;
> +
> + if (exynos_mipi_parse_ofnode_config(pdev, np_dsim_config,
> dsim_config))
> + return -EINVAL;
> +
> + np_panel_info = of_find_node_by_name(np, "display-mode");
> + if (!np_panel_info)
> + return -EINVAL;
> +
> + if (exynos_mipi_parse_ofnode_panel_info(pdev,
> + np_panel_info, panel_info))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static int exynos_mipi_dsi_probe(struct platform_device *pdev)
> {
> struct resource *res;
> @@ -336,6 +754,12 @@ static int exynos_mipi_dsi_probe(struct
> platform_device *pdev)
> struct mipi_dsim_config *dsim_config;
> struct mipi_dsim_platform_data *dsim_pd;
> struct mipi_dsim_ddi *dsim_ddi;
> + struct device_node *ofnode_lcd = NULL;
> + struct device_node *ofnode_dphy = NULL;
> + struct mipi_dsim_lcd_device *active_mipi_dsim_lcd_device = NULL;
> + struct mipi_dsim_phy_config *mipi_dphy_config;
> + struct fb_videomode *panel_info;
> + unsigned int u32Val;
> int ret = -EINVAL;
>
> dsim = devm_kzalloc(&pdev->dev,
> @@ -345,23 +769,87 @@ static int exynos_mipi_dsi_probe(struct
> platform_device *pdev)
> return -ENOMEM;
> }
>
> + if (pdev->dev.of_node) {
> + ofnode_lcd = exynos_mipi_find_ofnode_lcd_device(pdev);
> + if (!ofnode_lcd)
> + return -EINVAL;
> +
> + active_mipi_dsim_lcd_device > + exynos_mipi_parse_ofnode_lcd(pdev,
ofnode_lcd);
> +
> + if (NULL = active_mipi_dsim_lcd_device)
> + return -EINVAL;
> +
> + if (NULL = exynos_mipi_dsi_find_lcd_driver
> + (active_mipi_dsim_lcd_device))
> + return -ENXIO;
> +
> + exynos_mipi_dsi_register_lcd_device(
> +
active_mipi_dsim_lcd_device);
> + }
> +
> dsim->pd = to_dsim_plat(pdev);
> dsim->dev = &pdev->dev;
> dsim->id = pdev->id;
>
> - /* get mipi_dsim_platform_data. */
> - dsim_pd = (struct mipi_dsim_platform_data *)dsim->pd;
> - if (dsim_pd = NULL) {
> - dev_err(&pdev->dev, "failed to get platform data for
> dsim.\n");
> - return -EFAULT;
> - }
> - /* get mipi_dsim_config. */
> - dsim_config = dsim_pd->dsim_config;
> - if (dsim_config = NULL) {
> - dev_err(&pdev->dev, "failed to get dsim config data.\n");
> - return -EFAULT;
> - }
> + if (pdev->dev.of_node) {
> + dsim_config = devm_kzalloc(&pdev->dev,
> + sizeof(struct mipi_dsim_config), GFP_KERNEL);
> + if (!dsim_config) {
> + dev_err(&pdev->dev,
> + "failed to allocate dsim_config object.\n");
> + return -ENOMEM;
> + }
>
> + panel_info = devm_kzalloc(&pdev->dev,
> + sizeof(struct fb_videomode), GFP_KERNEL);
> + if (!panel_info) {
> + dev_err(&pdev->dev,
> + "failed to allocate fb_videomode
object.\n");
> + return -ENOMEM;
> + }
> +
> + /* parse the mipi of_node for dism_config and panel info. */
> + if (exynos_mipi_parse_ofnode(pdev, dsim_config, panel_info))
> {
> + dev_err(&pdev->dev,
> + "failed to read mipi-config,
display-mode\n");
> + return -EINVAL;
> + }
> +
> + dsim_pd = devm_kzalloc(&pdev->dev,
> + sizeof(struct mipi_dsim_platform_data), GFP_KERNEL);
> + if (!dsim_pd) {
> + dev_err(&pdev->dev,
> + "failed to allocate
mipi_dsim_platform_data\n");
> + return -ENOMEM;
> + }
> +
> + if (of_property_read_u32(pdev->dev.of_node, "enabled",
> &u32Val))
> + dev_err(&pdev->dev, "enabled property not found\n");
> + else
> + dsim_pd->enabled = !(!u32Val);
> +
> + dsim_pd->lcd_panel_info = (void *)panel_info;
> + dsim_pd->dsim_config = dsim_config;
> + dsim->pd = dsim_pd;
> +
> + } else {
> + /* get mipi_dsim_platform_data. */
> + dsim_pd = (struct mipi_dsim_platform_data *)dsim->pd;
> + if (dsim_pd = NULL) {
> + dev_err(&pdev->dev,
> + "failed to get platform data for dsim.\n");
> + return -EFAULT;
> + }
> +
> + /* get mipi_dsim_config. */
> + dsim_config = dsim_pd->dsim_config;
> + if (dsim_config = NULL) {
> + dev_err(&pdev->dev,
> + "failed to get dsim config data.\n");
> + return -EFAULT;
> + }
> + }
> dsim->dsim_config = dsim_config;
> dsim->master_ops = &master_ops;
>
> @@ -394,11 +882,21 @@ static int exynos_mipi_dsi_probe(struct
> platform_device *pdev)
> mutex_init(&dsim->lock);
>
> /* bind lcd ddi matched with panel name. */
> - dsim_ddi = exynos_mipi_dsi_bind_lcd_ddi(dsim, dsim_pd-
> >lcd_panel_name);
> + if (pdev->dev.of_node) {
> + dsim_ddi = exynos_mipi_dsi_bind_lcd_ddi(dsim,
> + active_mipi_dsim_lcd_device->name);
> + } else {
> + dsim_ddi = exynos_mipi_dsi_bind_lcd_ddi(dsim,
> + dsim_pd->lcd_panel_name);
> + }
> +
> if (!dsim_ddi) {
> dev_err(&pdev->dev, "mipi_dsim_ddi object not found.\n");
> ret = -ENXIO;
> goto cleanup_clk;
> + } else if (pdev->dev.of_node) {
> + dsim_ddi->ofnode_dsim_lcd_dev = ofnode_lcd;
> + dsim_ddi->ofnode_dsim_dphy = ofnode_dphy;
> }
>
> dsim->irq = platform_get_irq(pdev, 0);
> @@ -412,6 +910,21 @@ static int exynos_mipi_dsi_probe(struct
> platform_device *pdev)
> init_completion(&dsim_rd_comp);
> platform_set_drvdata(pdev, dsim);
>
> + /* update dsim phy config node */
> + if (pdev->dev.of_node) {
> + ofnode_dphy = exynos_mipi_find_ofnode_dsim_phy(pdev);
> + if (!ofnode_dphy)
> + return -EINVAL;
> +
> + mipi_dphy_config = exynos_mipi_parse_ofnode_dsim_phy(pdev,
> +
ofnode_dphy);
> +
> + if (NULL = mipi_dphy_config)
> + return -EINVAL;
> +
> + dsim->dsim_phy_config = mipi_dphy_config;
> + }
> +
> ret = devm_request_irq(&pdev->dev, dsim->irq,
> exynos_mipi_dsi_interrupt_handler,
> IRQF_SHARED, dev_name(&pdev->dev), dsim);
> @@ -557,13 +1070,33 @@ static const struct dev_pm_ops
> exynos_mipi_dsi_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(exynos_mipi_dsi_suspend,
> exynos_mipi_dsi_resume)
> };
>
> +static struct platform_device_id exynos_mipi_driver_ids[] = {
> + {
> + .name = "exynos-mipi",
> + .driver_data = NULL,
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(platform, exynos_mipi_driver_ids);
> +
> +static const struct of_device_id exynos_mipi_match[] = {
> + {
> + .compatible = "samsung,exynos5-mipi",
> + .data = NULL,
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, exynos_mipi_match);
> +
> static struct platform_driver exynos_mipi_dsi_driver = {
> .probe = exynos_mipi_dsi_probe,
> .remove = __devexit_p(exynos_mipi_dsi_remove),
> + .id_table = exynos_mipi_driver_ids,
> .driver = {
> .name = "exynos-mipi-dsim",
> .owner = THIS_MODULE,
> .pm = &exynos_mipi_dsi_pm_ops,
> + .of_match_table = exynos_mipi_match,
> },
> };
>
> diff --git a/include/video/exynos_mipi_dsim.h
> b/include/video/exynos_mipi_dsim.h
> index 772c770..6d9b01d 100644
> --- a/include/video/exynos_mipi_dsim.h
> +++ b/include/video/exynos_mipi_dsim.h
> @@ -230,6 +230,7 @@ struct mipi_dsim_device {
> struct mipi_dsim_master_ops *master_ops;
> struct mipi_dsim_lcd_device *dsim_lcd_dev;
> struct mipi_dsim_lcd_driver *dsim_lcd_drv;
> + struct mipi_dsim_phy_config *dsim_phy_config;
>
> unsigned int state;
> unsigned int data_lane;
> @@ -295,6 +296,32 @@ struct mipi_dsim_master_ops {
> };
>
> /*
> + * phy node structure for mipi-dsim.
> + *
> + * @reg_enable_dphy : base address to memory mapped D-PHY enable
> register
> + * @ctrlbit_enable_dphy : control bit for enabling D-PHY
> + * @reg_reset_dsim : base address to memory mapped DSIM reset register
> + * @ctrlbit_reset_dsim : control bit for resetting DSIM
> + */
> +struct mipi_dsim_phy_config_type1 {
> + void __iomem *reg_enable_dphy;
> + int ctrlbit_enable_dphy;
> + void __iomem *reg_reset_dsim;
> + int ctrlbit_reset_dsim;
> +};
> +
> +enum mipi_dsim_phy_config_type {
> + MIPI_DSIM_PHY_CONFIG_TYPE1,
> +};
> +
> +struct mipi_dsim_phy_config {
> + enum mipi_dsim_phy_config_type type;
> + union {
> + struct mipi_dsim_phy_config_type1 phy_cfg_type1;
> + };
> +};
> +
> +/*
> * device structure for mipi-dsi based lcd panel.
> *
> * @name: name of the device to use with this device, or an
> --
> 1.7.0.4
^ permalink raw reply
* Re: [GIT PULL] omapdss fixes for 3.7-rc
From: Tomi Valkeinen @ 2012-11-19 12:04 UTC (permalink / raw)
To: Florian Tobias Schandinat, linux-omap@vger.kernel.org,
linux-fbdev
In-Reply-To: <50978C61.8080907@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2793 bytes --]
Hi Florian,
I have an additional crash bug fix for omapdss. I don't think you've
pulled the original request below yet, so here's an updated one:
Tomi
The following changes since commit ddffeb8c4d0331609ef2581d84de4d763607bd37:
Linux 3.7-rc1 (2012-10-14 14:41:04 -0700)
are available in the git repository at:
git://gitorious.org/linux-omap-dss2/linux.git tags/omapdss-for-3.7-rc7
for you to fetch changes up to c415187b689842e8bb85135c070c822c2505f805:
OMAPFB: Fix possible null pointer dereferencing (2012-11-19 10:41:50 +0200)
----------------------------------------------------------------
omapdss fixes for 3.7-rc7
omapdss fixes for three crash bugs and one missing mutex unlock.
----------------------------------------------------------------
Laurent Pinchart (1):
omapdss: dss: Fix clocks on OMAP363x
Tomi Valkeinen (1):
OMAPDSS: DSI: fix dsi_get_dsidev_from_id()
Tushar Behera (1):
OMAPFB: Fix possible null pointer dereferencing
Wei Yongjun (1):
OMAPDSS: HDMI: fix missing unlock on error in hdmi_dump_regs()
drivers/video/omap2/dss/dsi.c | 13 +++++++++++--
drivers/video/omap2/dss/dss.c | 4 ++--
drivers/video/omap2/dss/hdmi.c | 4 +++-
drivers/video/omap2/omapfb/omapfb-ioctl.c | 2 +-
4 files changed, 17 insertions(+), 6 deletions(-)
On 2012-11-05 11:52, Tomi Valkeinen wrote:
> Hi Florian,
>
> Here are a few omapdss fixes, two fixing crasher bugs and the third
> fixes a missing unlock. These are based on 3.7-rc1. I can rebase on
> top of something else if you prefer that.
>
> Tomi
>
>
> The following changes since commit
> ddffeb8c4d0331609ef2581d84de4d763607bd37:
>
> Linux 3.7-rc1 (2012-10-14 14:41:04 -0700)
>
> are available in the git repository at:
>
> git://gitorious.org/linux-omap-dss2/linux.git
> tags/omapdss-for-3.7-rc5
>
> for you to fetch changes up to
> dffc70ade1d13edd186f542718c4d78a31d92fb8:
>
> OMAPDSS: HDMI: fix missing unlock on error in hdmi_dump_regs()
> (2012-10-26 08:45:43 +0300)
>
> ----------------------------------------------------------------
> omapdss fixes for 3.7-rc5
>
> omapdss fixes for two crashes bugs and one missing mutex unlock.
>
> ----------------------------------------------------------------
> Laurent Pinchart (1): omapdss: dss: Fix clocks on OMAP363x
>
> Tomi Valkeinen (1): OMAPDSS: DSI: fix dsi_get_dsidev_from_id()
>
> Wei Yongjun (1): OMAPDSS: HDMI: fix missing unlock on error in
> hdmi_dump_regs()
>
> drivers/video/omap2/dss/dsi.c | 13 +++++++++++--
> drivers/video/omap2/dss/dss.c | 4 ++--
> drivers/video/omap2/dss/hdmi.c | 4 +++- 3 files changed, 16
> insertions(+), 5 deletions(-)
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* Re: [RFC] fbdev: arm has __raw I/O accessors, use them in fb.h
From: Tomi Valkeinen @ 2012-11-19 10:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121119094606.GF3290@n2100.arm.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 2183 bytes --]
On 2012-11-19 11:46, Russell King - ARM Linux wrote:
> On Mon, Nov 19, 2012 at 11:36:24AM +0200, Tomi Valkeinen wrote:
>> Probably not. I can't say anything to that matter, but I wonder if this
>> patch is just going around the problem that we get sparse warnings when
>> falling into the else ifdef block in fb.h.
>>
>> The macros in the else block are defined as:
>>
>> #define fb_readb(addr) (*(volatile u8 *) (addr))
>>
>> And fb code passes a pointer to __iomem. So shouldn't the cast be to
>> (volatile u8 __iomem *)?
>
> No, because sparse won't let you directly dereference an __iomem pointer.
>
> Directly dereferencing an __iomem pointer is wrong, always has been. It's
> marked with __iomem _because_ it's a separate cookied address space from
> the virtual address space.
>
> This is one of the situations which has been left because the warning is
> correct - and this is one of those situations where silencing them via
> casts et.al. is the wrong thing to do. The right thing to do is to leave
> the warning in place.
>
> This is one of the hardest things that I seem to get people to understand:
> if the code is wrong then we _should_ get a warning for it and we should
> definitely not paper over the warning.
Yes, you're right. Well, I'm not very experienced with handling
different endiannesses, but my analysis:
fb.h uses either __raw_read/write functions or (*(volatile u32 *)
(addr)) and (*(volatile u32 *) (addr) = (b)) for read/write.
__raw_read/write are defined in arch/arm/include/asm/io.h and are the
same for BE and LE. I made a test c-file with two functions, one using
raw_read style assembly, other using pointer dereference. I compiled it
with -mlittle-endian and -mbig-endian, and the resulting assembly was
identical for both, and the assembly for both functions were the same.
So if I didn't make any mistakes above, using __raw_read/write instead
of the direct pointer dereference results in identical operation for
both big and little endian arms. So if big-endian fb reads/writes is
correct now, it should be correct with raw_read/write also.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* Re: [RFC] fbdev: arm has __raw I/O accessors, use them in fb.h
From: Russell King - ARM Linux @ 2012-11-19 9:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50A9FD98.1060105@ti.com>
On Mon, Nov 19, 2012 at 11:36:24AM +0200, Tomi Valkeinen wrote:
> Probably not. I can't say anything to that matter, but I wonder if this
> patch is just going around the problem that we get sparse warnings when
> falling into the else ifdef block in fb.h.
>
> The macros in the else block are defined as:
>
> #define fb_readb(addr) (*(volatile u8 *) (addr))
>
> And fb code passes a pointer to __iomem. So shouldn't the cast be to
> (volatile u8 __iomem *)?
No, because sparse won't let you directly dereference an __iomem pointer.
Directly dereferencing an __iomem pointer is wrong, always has been. It's
marked with __iomem _because_ it's a separate cookied address space from
the virtual address space.
This is one of the situations which has been left because the warning is
correct - and this is one of those situations where silencing them via
casts et.al. is the wrong thing to do. The right thing to do is to leave
the warning in place.
This is one of the hardest things that I seem to get people to understand:
if the code is wrong then we _should_ get a warning for it and we should
definitely not paper over the warning.
^ permalink raw reply
* Re: [RFC] fbdev: arm has __raw I/O accessors, use them in fb.h
From: Tomi Valkeinen @ 2012-11-19 9:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121119091504.GB3290@n2100.arm.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1192 bytes --]
On 2012-11-19 11:15, Russell King - ARM Linux wrote:
> On Mon, Nov 19, 2012 at 10:51:08AM +0530, Archit Taneja wrote:
>> On Friday 16 November 2012 10:14 PM, H Hartley Sweeten wrote:
>>> On Friday, November 16, 2012 2:16 AM, Archit Taneja wrote:
>>>>
>>>> This removes the sparse warnings on arm platforms:
>>>>
>>>> warning: cast removes address space of expression
>>>>
>>>> Signed-off-by: Archit Taneja <archit@ti.com>
>>>
>>> I submitted the same patch around early March 2012. So FWIW:
>>>
>>> Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>
>>
>> Thanks.
>>
>> Florian,
>>
>> Could you queue this for 3.8 merge window?
>
> Actually no. Has anyone checked whether this has any impact for the BE
> ARM platforms?
Probably not. I can't say anything to that matter, but I wonder if this
patch is just going around the problem that we get sparse warnings when
falling into the else ifdef block in fb.h.
The macros in the else block are defined as:
#define fb_readb(addr) (*(volatile u8 *) (addr))
And fb code passes a pointer to __iomem. So shouldn't the cast be to
(volatile u8 __iomem *)?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]
^ permalink raw reply
* Re: [RFC] fbdev: arm has __raw I/O accessors, use them in fb.h
From: Russell King - ARM Linux @ 2012-11-19 9:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <50A9C1C4.5070907@ti.com>
On Mon, Nov 19, 2012 at 10:51:08AM +0530, Archit Taneja wrote:
> On Friday 16 November 2012 10:14 PM, H Hartley Sweeten wrote:
>> On Friday, November 16, 2012 2:16 AM, Archit Taneja wrote:
>>>
>>> This removes the sparse warnings on arm platforms:
>>>
>>> warning: cast removes address space of expression
>>>
>>> Signed-off-by: Archit Taneja <archit@ti.com>
>>
>> I submitted the same patch around early March 2012. So FWIW:
>>
>> Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>
>
> Thanks.
>
> Florian,
>
> Could you queue this for 3.8 merge window?
Actually no. Has anyone checked whether this has any impact for the BE
ARM platforms?
^ permalink raw reply
* Re: [PATCH] video: Kconfig: Specify the SoCs that make use of FB_IMX
From: Sascha Hauer @ 2012-11-19 8:33 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1353182050-13338-1-git-send-email-festevam@gmail.com>
On Sat, Nov 17, 2012 at 05:54:10PM -0200, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> FB_IMX is the framebuffer driver used by MX25 and MX27 processors.
>
> Pass this information to the Kconfig text to make it clear.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> drivers/video/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 0cff083..f0c53b2 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -364,7 +364,7 @@ config FB_SA1100
> Y here.
>
> config FB_IMX
> - tristate "Freescale i.MX LCD support"
> + tristate "Freescale i.MX25/27 LCD support"
i.MX1 and i.MX21 use this driver aswell.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [RFC] fbdev: arm has __raw I/O accessors, use them in fb.h
From: Archit Taneja @ 2012-11-19 5:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ADE657CA350FB648AAC2C43247A983F00206B2644F3F@AUSP01VMBX24.collaborationhost.net>
On Friday 16 November 2012 10:14 PM, H Hartley Sweeten wrote:
> On Friday, November 16, 2012 2:16 AM, Archit Taneja wrote:
>>
>> This removes the sparse warnings on arm platforms:
>>
>> warning: cast removes address space of expression
>>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>
> I submitted the same patch around early March 2012. So FWIW:
>
> Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Thanks.
Florian,
Could you queue this for 3.8 merge window?
Regards,
Archit
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Anton Vorontsov @ 2012-11-19 2:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8058793.mIXgB85Mso@percival>
On Mon, Nov 19, 2012 at 11:29:05AM +0900, Alex Courbot wrote:
> On Saturday 17 November 2012 19:38:50 Anton Vorontsov wrote:
> > > +++ b/drivers/power/power_seq/Kconfig
> > > @@ -0,0 +1,2 @@
> > > +config POWER_SEQ
> > > + tristate
> >
> > This really needs a proper Kconfig description and a help text, shortly
> > describing the purpose of the subsystem.
>
> Does it? The current state makes power seqs automatically selected by drivers
> that need it, and they do not appear in the configuration menu. If we add a
> description they will then become a visible item, so the logic would be to
> make it user-selectable.
Ah. OK, then yes, there is no need for the short description, but the help
text would a good idea anyway.
Thanks,
Anton.
^ permalink raw reply
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-11-19 2:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121117113849.GA5228@lizard>
On Saturday 17 November 2012 19:38:50 Anton Vorontsov wrote:
> > +++ b/drivers/power/power_seq/Kconfig
> > @@ -0,0 +1,2 @@
> > +config POWER_SEQ
> > + tristate
>
> This really needs a proper Kconfig description and a help text, shortly
> describing the purpose of the subsystem.
Does it? The current state makes power seqs automatically selected by drivers
that need it, and they do not appear in the configuration menu. If we add a
description they will then become a visible item, so the logic would be to
make it user-selectable.
If that approach is preferred, I will also have to change pwm-backlight so
that it compiles without power sequences. Not that I mind, but I liked the
idea of not adding yet-another-option to the config menu.
Fixed the code according to all your other comments, thanks!
Alex.
^ permalink raw reply
* Re: video: pull request for branch "cfa-10049-3.7-oled-driver"
From: Thomas Petazzoni @ 2012-11-18 16:35 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1352993527-9113-1-git-send-email-maxime.ripard@free-electrons.com>
Andrew, Greg,
On Thu, 15 Nov 2012 16:32:06 +0100, Maxime Ripard wrote:
> Since these patches have been around for a few months without any comment,
> please pull the following branch.
We have been sending the below code for 2-3 months without reply or
comments from the fbdev maintainer other than "I will look at it some
day". It is a simple 400 lines driver, so nothing really horrible
to review.
We already contacted Greg directly about this situation, who suggested
resending the driver, which we did. Now we sent a formal pull request
last week, and still no answer. Would it be possible to get this driver
merged for 3.8 ?
Of course, we're willing to rework, fix and modify the driver as
needed, if comments are being made.
Thanks a lot,
Thomas
> The following changes since commit 6f0c0580b70c89094b3422ba81118c7b959c7556:
>
> Linux 3.7-rc2 (2012-10-20 12:11:32 -0700)
>
> are available in the git repository at:
>
> https://github.com/crystalfontz/cfa_10036_kernel.git cfa-10049-3.7-oled-driver
>
> for you to fetch changes up to 353b74485bd80950b4beea1497fa234d3277e4e8:
>
> video: Add support for the Solomon SSD1307 OLED Controller (2012-10-22 12:35:02 +0200)
>
> ----------------------------------------------------------------
> Maxime Ripard (1):
> video: Add support for the Solomon SSD1307 OLED Controller
>
> .../devicetree/bindings/video/ssd1307fb.txt | 24 ++
> drivers/video/Kconfig | 13 +
> drivers/video/Makefile | 1 +
> drivers/video/ssd1307fb.c | 407 ++++++++++++++++++++
> 4 files changed, 445 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/video/ssd1307fb.txt
> create mode 100644 drivers/video/ssd1307fb.c
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* [PATCH] video: Kconfig: Specify the SoCs that make use of FB_IMX
From: Fabio Estevam @ 2012-11-17 19:54 UTC (permalink / raw)
To: linux-fbdev
From: Fabio Estevam <fabio.estevam@freescale.com>
FB_IMX is the framebuffer driver used by MX25 and MX27 processors.
Pass this information to the Kconfig text to make it clear.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/video/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 0cff083..f0c53b2 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -364,7 +364,7 @@ config FB_SA1100
Y here.
config FB_IMX
- tristate "Freescale i.MX LCD support"
+ tristate "Freescale i.MX25/27 LCD support"
depends on FB && IMX_HAVE_PLATFORM_IMX_FB
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Anton Vorontsov @ 2012-11-17 11:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1353149747-31871-2-git-send-email-acourbot@nvidia.com>
On Sat, Nov 17, 2012 at 07:55:45PM +0900, Alexandre Courbot wrote:
> Some device drivers (e.g. panel or backlights) need to follow precise
> sequences for powering on and off, involving GPIOs, regulators, PWMs
> and other power-related resources, with a defined powering order and
> delays to respect between steps. These sequences are device-specific,
> and do not belong to a particular driver - therefore they have been
> performed by board-specific hook functions so far.
>
> With the advent of the device tree and of ARM kernels that are not
> board-tied, we cannot rely on these board-specific hooks anymore but
> need a way to implement these sequences in a portable manner. This patch
> introduces a simple interpreter that can execute such power sequences
> encoded either as platform data or within the device tree. It also
> introduces first support for regulator, GPIO and PWM resources.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> Reviewed-by: Stephen Warren <swarren@wwwdotorg.org>
> Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> ---
This looks almost perfect!
Just a few final notes, again mostly about the build system bits.
[...]
> diff --git a/drivers/power/power_seq/Kconfig b/drivers/power/power_seq/Kconfig
> new file mode 100644
> index 0000000..0ece819
> --- /dev/null
> +++ b/drivers/power/power_seq/Kconfig
> @@ -0,0 +1,2 @@
> +config POWER_SEQ
> + tristate
This really needs a proper Kconfig description and a help text, shortly
describing the purpose of the subsystem.
> diff --git a/drivers/power/power_seq/Makefile b/drivers/power/power_seq/Makefile
> new file mode 100644
> index 0000000..964b91e
> --- /dev/null
> +++ b/drivers/power/power_seq/Makefile
> @@ -0,0 +1,2 @@
> +obj-$(CONFIG_POWER_SEQ) += power_seq.o
> +power_seq-y := core.o delay.o regulator.o gpio.o pwm.o
> diff --git a/drivers/power/power_seq/core.c b/drivers/power/power_seq/core.c
> new file mode 100644
> index 0000000..d51b9b8
> --- /dev/null
> +++ b/drivers/power/power_seq/core.c
> @@ -0,0 +1,362 @@
> +/*
> + * core.c - power sequence interpreter for platform devices and device tree
We usually don't write file names in the comments.
> + *
> + * Author: Alexandre Courbot <acourbot@nvidia.com>
> + *
[...]
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(power_seq_run);
> +
> +/* defined in power_seq_*.c files */
Why not place the decls into the _priv.h file?.. I understand that this
might be a temporary stuff until we have something better for ops
registration, but still, I believe it belongs to the header file.
> +extern const struct power_seq_res_ops power_seq_delay_ops;
> +extern const struct power_seq_res_ops power_seq_gpio_ops;
> +extern const struct power_seq_res_ops power_seq_regulator_ops;
> +extern const struct power_seq_res_ops power_seq_pwm_ops;
> +
> +static const struct power_seq_res_ops *power_seq_ops[POWER_SEQ_NUM_TYPES] = {
> + [POWER_SEQ_DELAY] = &power_seq_delay_ops,
> + [POWER_SEQ_REGULATOR] = &power_seq_regulator_ops,
> + [POWER_SEQ_PWM] = &power_seq_gpio_ops,
> + [POWER_SEQ_GPIO] = &power_seq_pwm_ops,
> +};
> +
> +MODULE_AUTHOR("Alexandre Courbot <acourbot@nvidia.com>");
> +MODULE_DESCRIPTION("Runtime Interpreted Power Sequences");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/power/power_seq/delay.c b/drivers/power/power_seq/delay.c
> new file mode 100644
> index 0000000..de6810b
> --- /dev/null
> +++ b/drivers/power/power_seq/delay.c
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright (c) 2012 NVIDIA Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + */
> +
> +#include "power_seq_priv.h"
> +#include <linux/delay.h>
Things should not depend on _priv.h's includes. I.e. I see this file uses
of_ routines, so it should include <linux/of.h>. <linux/delay.h> for
udelay_range(), etc.
Also, usually we place "private" headers at the very end, not at the
beginning.
> +
> +#ifdef CONFIG_OF
> +static int of_power_seq_parse_delay(struct device_node *node,
> + struct power_seq *seq,
> + unsigned int step_nbr,
> + struct power_seq_resource *res)
> +{
> + struct power_seq_step *step = &seq->steps[step_nbr];
> + int err;
> +
> + err = of_property_read_u32(node, "delay",
> + &step->delay.delay);
> + if (err < 0)
> + power_seq_err(seq, step_nbr, "error reading delay property\n");
> +
> + return err;
> +}
> +#else
> +#define of_power_seq_parse_delay NULL
> +#endif
[...]
> +#define power_seq_err(seq, step_nbr, format, ...) \
> + dev_err(seq->set->dev, "%s[%d]: " format, seq->id, step_nbr, \
> + ##__VA_ARGS__);
> +
> +#ifdef CONFIG_OF
> +int of_power_seq_parse_enable_properties(struct device_node *node,
> + struct power_seq *seq,
> + unsigned int step_nbr, bool *enable);
Um, I believe you don't need CONFIG_OF here. (If it's about 'struct
device_node', then as I see it in of.h, the header always declares it,
even for the !OF case.)
> +#endif
> +
> +/**
[...]
> +++ b/drivers/power/power_seq/regulator.c
> @@ -0,0 +1,87 @@
> +/*
> + * Copyright (c) 2012 NVIDIA Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + */
> +
> +#include "power_seq_priv.h"
> +
> +#ifdef CONFIG_REGULATOR
Would be really great if you could get rid of the #ifdefs in the .c files.
To do so, in the makefile you could write something like this:
power_seq-$(CONFIG_REGULATOR) += regulator.o
And in the header file (as explained above), you'd have
#ifdef CONFIG_REGULATOR
#define ...REGULATOR_OPS &power_seq_regulator_ops
#else
#define ...REGULATOR_OPS NULL
#endif
Or something along these lines...
Thanks,
Anton.
^ permalink raw reply
* [PATCHv9 3/3] Take maintainership of power sequences
From: Alexandre Courbot @ 2012-11-17 10:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1353149747-31871-1-git-send-email-acourbot@nvidia.com>
Add entry for power sequences into MAINTAINERS with all the needed
contact and SCM info.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Acked-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
---
MAINTAINERS | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index bb0b27d..e9098f5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5745,6 +5745,16 @@ F: fs/timerfd.c
F: include/linux/timer*
F: kernel/*timer*
+POWER SEQUENCES
+M: Alexandre Courbot <acourbot@nvidia.com>
+S: Supported
+W: https://github.com/Gnurou/linux
+T: git git://github.com/Gnurou/linux.git power-seq/for-next
+F: Documentation/devicetree/bindings/power/power_seq.txt
+F: Documentation/power/power_seq.txt
+F: include/linux/power_seq.h
+F: drivers/power/power_seq/
+
POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS
M: Anton Vorontsov <cbou@mail.ru>
M: David Woodhouse <dwmw2@infradead.org>
--
1.8.0
^ permalink raw reply related
* [PATCHv9 2/3] pwm_backlight: use power sequences
From: Alexandre Courbot @ 2012-11-17 10:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1353149747-31871-1-git-send-email-acourbot@nvidia.com>
Make use of the power sequences specified in the device tree or platform
data to control how the backlight is powered on and off.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Stephen Warren <swarren@wwwdotorg.org>
Tested-by: Stephen Warren <swarren@nvidia.com>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
---
.../bindings/video/backlight/pwm-backlight.txt | 63 +++++++-
drivers/video/backlight/Kconfig | 1 +
drivers/video/backlight/pwm_bl.c | 160 ++++++++++++++++-----
include/linux/pwm_backlight.h | 18 ++-
4 files changed, 202 insertions(+), 40 deletions(-)
diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
index 1e4fc72..b20e98e 100644
--- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
@@ -13,16 +13,73 @@ Required properties:
Optional properties:
- pwm-names: a list of names for the PWM devices specified in the
- "pwms" property (see PWM binding[0])
+ "pwms" property (see PWM binding[0]).
+ - power-sequences: Power sequences (see Power sequences[1]) used to bring the
+ backlight on and off. If this property is present, then two power
+ sequences named "power-on" and "power-off" must be defined to control how
+ the backlight is to be powered on and off. These sequences must reference
+ the PWM specified in the pwms property by its name, and can also reference
+ other resources supported by the power sequences mechanism
[0]: Documentation/devicetree/bindings/pwm/pwm.txt
+[1]: Documentation/devicetree/bindings/power/power_seq.txt
Example:
backlight {
compatible = "pwm-backlight";
- pwms = <&pwm 0 5000000>;
-
brightness-levels = <0 4 8 16 32 64 128 255>;
default-brightness-level = <6>;
+ low-threshold-brightness = <50>;
+
+ /* resources used by the power sequences */
+ pwms = <&pwm 0 5000000>;
+ pwm-names = "backlight";
+ power-supply = <&backlight_reg>;
+
+ power-sequences {
+ power-on {
+ step0 {
+ type = "regulator";
+ id = "power";
+ enable;
+ };
+ step1 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step2 {
+ type = "pwm";
+ id = "backlight";
+ enable;
+ };
+ step3 {
+ type = "gpio";
+ gpio = <&gpio 28 0>;
+ value = <1>;
+ };
+ };
+
+ power-off {
+ step0 {
+ type = "gpio";
+ gpio = <&gpio 28 0>;
+ value = <0>;
+ };
+ step1 {
+ type = "pwm";
+ id = "backlight";
+ disable;
+ };
+ step2 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step3 {
+ type = "regulator";
+ id = "power";
+ disable;
+ };
+ };
+ };
};
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 765a945..a6b0640 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -240,6 +240,7 @@ config BACKLIGHT_CARILLO_RANCH
config BACKLIGHT_PWM
tristate "Generic PWM based Backlight Driver"
depends on PWM
+ select POWER_SEQ
help
If you have a LCD backlight adjustable by PWM, say Y to enable
this driver.
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 069983c..cfc0780 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -27,6 +27,12 @@ struct pwm_bl_data {
unsigned int period;
unsigned int lth_brightness;
unsigned int *levels;
+ bool enabled;
+ struct power_seq_set power_seqs;
+ struct power_seq *power_on_seq;
+ struct power_seq *power_off_seq;
+
+ /* Legacy callbacks */
int (*notify)(struct device *,
int brightness);
void (*notify_after)(struct device *,
@@ -35,6 +41,51 @@ struct pwm_bl_data {
void (*exit)(struct device *);
};
+static void pwm_backlight_on(struct backlight_device *bl)
+{
+ struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
+ int ret;
+
+ if (pb->enabled)
+ return;
+
+ if (pb->power_on_seq) {
+ ret = power_seq_run(pb->power_on_seq);
+ if (ret < 0) {
+ dev_err(&bl->dev, "cannot run power on sequence\n");
+ return;
+ }
+ } else {
+ /* legacy framework */
+ pwm_enable(pb->pwm);
+ }
+
+ pb->enabled = true;
+}
+
+static void pwm_backlight_off(struct backlight_device *bl)
+{
+ struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
+ int ret;
+
+ if (!pb->enabled)
+ return;
+
+ if (pb->power_off_seq) {
+ ret = power_seq_run(pb->power_off_seq);
+ if (ret < 0) {
+ dev_err(&bl->dev, "cannot run power off sequence\n");
+ return;
+ }
+ } else {
+ /* legacy framework */
+ pwm_config(pb->pwm, 0, pb->period);
+ pwm_disable(pb->pwm);
+ }
+
+ pb->enabled = false;
+}
+
static int pwm_backlight_update_status(struct backlight_device *bl)
{
struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
@@ -51,8 +102,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
brightness = pb->notify(pb->dev, brightness);
if (brightness = 0) {
- pwm_config(pb->pwm, 0, pb->period);
- pwm_disable(pb->pwm);
+ pwm_backlight_off(bl);
} else {
int duty_cycle;
@@ -66,7 +116,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
duty_cycle = pb->lth_brightness +
(duty_cycle * (pb->period - pb->lth_brightness) / max);
pwm_config(pb->pwm, duty_cycle, pb->period);
- pwm_enable(pb->pwm);
+ pwm_backlight_on(bl);
}
if (pb->notify_after)
@@ -145,11 +195,10 @@ static int pwm_backlight_parse_dt(struct device *dev,
data->max_brightness--;
}
- /*
- * TODO: Most users of this driver use a number of GPIOs to control
- * backlight power. Support for specifying these needs to be
- * added.
- */
+ /* read power sequences */
+ data->power_seqs = devm_of_parse_power_seq_set(dev);
+ if (IS_ERR(data->power_seqs))
+ return PTR_ERR(data->power_seqs);
return 0;
}
@@ -172,6 +221,7 @@ static int pwm_backlight_probe(struct platform_device *pdev)
{
struct platform_pwm_backlight_data *data = pdev->dev.platform_data;
struct platform_pwm_backlight_data defdata;
+ struct power_seq_resource *res;
struct backlight_properties props;
struct backlight_device *bl;
struct pwm_bl_data *pb;
@@ -180,7 +230,9 @@ static int pwm_backlight_probe(struct platform_device *pdev)
if (!data) {
ret = pwm_backlight_parse_dt(&pdev->dev, &defdata);
- if (ret < 0) {
+ if (ret = -EPROBE_DEFER) {
+ return ret;
+ } else if (ret < 0) {
dev_err(&pdev->dev, "failed to find platform data\n");
return ret;
}
@@ -201,6 +253,68 @@ static int pwm_backlight_probe(struct platform_device *pdev)
goto err_alloc;
}
+ if (data->power_seqs) {
+ /* use power sequences */
+ struct power_seq_set *seqs = &pb->power_seqs;
+
+ power_seq_set_init(seqs, &pdev->dev);
+ power_seq_set_add_sequences(seqs, data->power_seqs);
+
+ /* Check that the required sequences are here */
+ pb->power_on_seq = power_seq_lookup(seqs, "power-on");
+ if (!pb->power_on_seq) {
+ dev_err(&pdev->dev, "missing power-on sequence\n");
+ return -EINVAL;
+ }
+ pb->power_off_seq = power_seq_lookup(seqs, "power-off");
+ if (!pb->power_off_seq) {
+ dev_err(&pdev->dev, "missing power-off sequence\n");
+ return -EINVAL;
+ }
+
+ /* we must have exactly one PWM resource for this driver */
+ power_seq_for_each_resource(res, seqs) {
+ if (res->type != POWER_SEQ_PWM)
+ continue;
+ if (pb->pwm) {
+ dev_err(&pdev->dev, "more than one PWM used\n");
+ return -EINVAL;
+ }
+ /* keep the pwm at hand */
+ pb->pwm = res->pwm.pwm;
+ }
+ /* from here we should have a PWM */
+ if (!pb->pwm) {
+ dev_err(&pdev->dev, "no PWM defined!\n");
+ return -EINVAL;
+ }
+ } else {
+ /* use legacy interface */
+ pb->pwm = devm_pwm_get(&pdev->dev, NULL);
+ if (IS_ERR(pb->pwm)) {
+ dev_err(&pdev->dev,
+ "unable to request PWM, trying legacy API\n");
+
+ pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");
+ if (IS_ERR(pb->pwm)) {
+ dev_err(&pdev->dev,
+ "unable to request legacy PWM\n");
+ ret = PTR_ERR(pb->pwm);
+ goto err_alloc;
+ }
+ }
+
+ dev_dbg(&pdev->dev, "got pwm for backlight\n");
+
+ /*
+ * The DT case will set the pwm_period_ns field to 0 and store
+ * the period, parsed from the DT, in the PWM device. For the
+ * non-DT case, set the period from platform data.
+ */
+ if (data->pwm_period_ns > 0)
+ pwm_set_period(pb->pwm, data->pwm_period_ns);
+ }
+
if (data->levels) {
max = data->levels[data->max_brightness];
pb->levels = data->levels;
@@ -213,28 +327,6 @@ static int pwm_backlight_probe(struct platform_device *pdev)
pb->exit = data->exit;
pb->dev = &pdev->dev;
- pb->pwm = devm_pwm_get(&pdev->dev, NULL);
- if (IS_ERR(pb->pwm)) {
- dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
-
- pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");
- if (IS_ERR(pb->pwm)) {
- dev_err(&pdev->dev, "unable to request legacy PWM\n");
- ret = PTR_ERR(pb->pwm);
- goto err_alloc;
- }
- }
-
- dev_dbg(&pdev->dev, "got pwm for backlight\n");
-
- /*
- * The DT case will set the pwm_period_ns field to 0 and store the
- * period, parsed from the DT, in the PWM device. For the non-DT case,
- * set the period from platform data.
- */
- if (data->pwm_period_ns > 0)
- pwm_set_period(pb->pwm, data->pwm_period_ns);
-
pb->period = pwm_get_period(pb->pwm);
pb->lth_brightness = data->lth_brightness * (pb->period / max);
@@ -267,8 +359,7 @@ static int pwm_backlight_remove(struct platform_device *pdev)
struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
backlight_device_unregister(bl);
- pwm_config(pb->pwm, 0, pb->period);
- pwm_disable(pb->pwm);
+ pwm_backlight_off(bl);
if (pb->exit)
pb->exit(&pdev->dev);
return 0;
@@ -282,8 +373,7 @@ static int pwm_backlight_suspend(struct device *dev)
if (pb->notify)
pb->notify(pb->dev, 0);
- pwm_config(pb->pwm, 0, pb->period);
- pwm_disable(pb->pwm);
+ pwm_backlight_off(bl);
if (pb->notify_after)
pb->notify_after(pb->dev, 0);
return 0;
diff --git a/include/linux/pwm_backlight.h b/include/linux/pwm_backlight.h
index 56f4a86..0dcec1d 100644
--- a/include/linux/pwm_backlight.h
+++ b/include/linux/pwm_backlight.h
@@ -5,14 +5,28 @@
#define __LINUX_PWM_BACKLIGHT_H
#include <linux/backlight.h>
+#include <linux/power_seq.h>
struct platform_pwm_backlight_data {
- int pwm_id;
unsigned int max_brightness;
unsigned int dft_brightness;
unsigned int lth_brightness;
- unsigned int pwm_period_ns;
unsigned int *levels;
+ /*
+ * New interface using power sequences. Must include exactly
+ * two power sequences named 'power-on' and 'power-off'. If NULL,
+ * the legacy interface is used.
+ */
+ struct platform_power_seq_set *power_seqs;
+
+ /*
+ * Legacy interface - use power sequences instead!
+ *
+ * pwm_id and pwm_period_ns need only be specified
+ * if get_pwm(dev, NULL) would return NULL.
+ */
+ int pwm_id;
+ unsigned int pwm_period_ns;
int (*init)(struct device *dev);
int (*notify)(struct device *dev, int brightness);
void (*notify_after)(struct device *dev, int brightness);
--
1.8.0
^ permalink raw reply related
* [PATCHv9 1/3] Runtime Interpreted Power Sequences
From: Alexandre Courbot @ 2012-11-17 10:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1353149747-31871-1-git-send-email-acourbot@nvidia.com>
Some device drivers (e.g. panel or backlights) need to follow precise
sequences for powering on and off, involving GPIOs, regulators, PWMs
and other power-related resources, with a defined powering order and
delays to respect between steps. These sequences are device-specific,
and do not belong to a particular driver - therefore they have been
performed by board-specific hook functions so far.
With the advent of the device tree and of ARM kernels that are not
board-tied, we cannot rely on these board-specific hooks anymore but
need a way to implement these sequences in a portable manner. This patch
introduces a simple interpreter that can execute such power sequences
encoded either as platform data or within the device tree. It also
introduces first support for regulator, GPIO and PWM resources.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Stephen Warren <swarren@wwwdotorg.org>
Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
Tested-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
---
.../devicetree/bindings/power/power_seq.txt | 121 +++++++
Documentation/power/power_seq.txt | 253 ++++++++++++++
drivers/power/Kconfig | 1 +
drivers/power/Makefile | 1 +
drivers/power/power_seq/Kconfig | 2 +
drivers/power/power_seq/Makefile | 2 +
drivers/power/power_seq/core.c | 362 +++++++++++++++++++++
drivers/power/power_seq/delay.c | 66 ++++
drivers/power/power_seq/gpio.c | 95 ++++++
drivers/power/power_seq/power_seq_priv.h | 56 ++++
drivers/power/power_seq/pwm.c | 85 +++++
drivers/power/power_seq/regulator.c | 87 +++++
include/linux/power_seq.h | 203 ++++++++++++
13 files changed, 1334 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/power_seq.txt
create mode 100644 Documentation/power/power_seq.txt
create mode 100644 drivers/power/power_seq/Kconfig
create mode 100644 drivers/power/power_seq/Makefile
create mode 100644 drivers/power/power_seq/core.c
create mode 100644 drivers/power/power_seq/delay.c
create mode 100644 drivers/power/power_seq/gpio.c
create mode 100644 drivers/power/power_seq/power_seq_priv.h
create mode 100644 drivers/power/power_seq/pwm.c
create mode 100644 drivers/power/power_seq/regulator.c
create mode 100644 include/linux/power_seq.h
diff --git a/Documentation/devicetree/bindings/power/power_seq.txt b/Documentation/devicetree/bindings/power/power_seq.txt
new file mode 100644
index 0000000..7880a6c
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/power_seq.txt
@@ -0,0 +1,121 @@
+Runtime Interpreted Power Sequences
+=================+
+Power sequences are sequential descriptions of actions to be performed on
+power-related resources. Having these descriptions in a well-defined data format
+allows us to take much of the board- or device- specific power control code out
+of the kernel and place it into the device tree instead, making kernels less
+board-dependant.
+
+A device typically makes use of multiple power sequences, for different purposes
+such as powering on and off. All the power sequences of a given device are
+grouped into a set. In the device tree, this set is a sub-node of the device
+node named "power-sequences".
+
+Power Sequences Structure
+-------------------------
+Every device that makes use of power sequences must have a "power-sequences"
+node into which individual power sequences are declared as sub-nodes. The name
+of the node becomes the name of the sequence within the power sequences
+framework.
+
+Similarly, each power sequence declares its steps as sub-nodes of itself. Steps
+must be named sequentially, with the first step named step0, the second step1,
+etc. Failure to follow this rule will result in a parsing error.
+
+Power Sequences Steps
+---------------------
+Steps of a sequence describe an action to be performed on a resource. They
+always include a "type" property which indicates what kind of resource this
+step works on. Depending on the resource type, additional properties are defined
+to control the action to be performed.
+
+"delay" type required properties:
+ - delay: delay to wait (in microseconds)
+
+"regulator" type required properties:
+ - id: name of the regulator to use.
+ - enable / disable: one of these two empty properties must be present to
+ enable or disable the resource
+
+"pwm" type required properties:
+ - id: name of the PWM to use.
+ - enable / disable: one of these two empty properties must be present to
+ enable or disable the resource
+
+"gpio" type required properties:
+ - gpio: phandle of the GPIO to use.
+ - value: value this GPIO should take. Must be 0 or 1.
+
+Example
+-------
+Here are example sequences declared within a backlight device that use all the
+supported resources types:
+
+ backlight {
+ compatible = "pwm-backlight";
+ ...
+
+ /* resources used by the power sequences */
+ pwms = <&pwm 2 5000000>;
+ pwm-names = "backlight";
+ power-supply = <&backlight_reg>;
+
+ power-sequences {
+ power-on {
+ step0 {
+ type = "regulator";
+ id = "power";
+ enable;
+ };
+ step1 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step2 {
+ type = "pwm";
+ id = "backlight";
+ enable;
+ };
+ step3 {
+ type = "gpio";
+ gpio = <&gpio 28 0>;
+ value = <1>;
+ };
+ };
+
+ power-off {
+ step0 {
+ type = "gpio";
+ gpio = <&gpio 28 0>;
+ value = <0>;
+ };
+ step1 {
+ type = "pwm";
+ id = "backlight";
+ disable;
+ };
+ step2 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step3 {
+ type = "regulator";
+ id = "power";
+ disable;
+ };
+ };
+ };
+ };
+
+The first part lists the PWM and regulator resources used by the sequences.
+These resources will be requested on behalf of the backlight device when the
+sequences are built and are declared according to their own bindings (for
+instance, regulators and pwms are resolved by name - note though that name
+declaration is done differently by the two frameworks).
+
+After the resources declaration, two sequences follow for powering the backlight
+on and off. Their names are specified by the pwm-backlight device bindings. Once
+the sequences are built by calling devm_of_parse_power_seq_set() on the
+backlight device, they can be added to a set using
+power_seq_set_add_sequences().
diff --git a/Documentation/power/power_seq.txt b/Documentation/power/power_seq.txt
new file mode 100644
index 0000000..8be0570
--- /dev/null
+++ b/Documentation/power/power_seq.txt
@@ -0,0 +1,253 @@
+Runtime Interpreted Power Sequences
+=================+
+Problem
+-------
+Very commonly, boards need the help of out-of-driver code to turn some of their
+devices on and off. For instance, SoC boards might use a GPIO (abstracted to a
+regulator or not) to control the power supply of a backlight. The GPIO that
+should be used, however, as well as the exact power sequence that may also
+involve other resources, is board-dependent and thus unknown to the driver.
+
+This was previously addressed by having hooks in the device's platform data that
+are called whenever the state of the device might need a power status change.
+This approach, however, introduces board-dependant code into the kernel and is
+not compatible with the device tree.
+
+The Runtime Interpreted Power Sequences (or power sequences for short) aim at
+turning this code into platform data or device tree nodes. Power sequences are
+described using a simple format and run by a lightweight interpreter whenever
+needed. This allows device drivers to work without power callbacks and makes the
+kernel less board-dependant.
+
+What are Power Sequences?
+-------------------------
+A power sequence is an array of sequential steps describing an action to be
+performed on a resource. The supported resources and actions operations are:
+- delay (just wait for a given number of microseconds)
+- GPIO (set to 0 or 1)
+- regulator (enable or disable)
+- PWM (enable or disable)
+
+When a power sequence is run, its steps is executed one after the other until
+one step fails or the end of the sequence is reached.
+
+Power sequences are named, and grouped into "sets" which contain all the
+sequences of a device as well as the resources they use.
+
+Power sequences can be declared as platform data or in the device tree.
+
+Platform Data Format
+--------------------
+All relevant data structures for declaring power sequences are located in
+include/linux/power_seq.h.
+
+The platform data for a device may include an instance of platform_power_seq_set
+which references all the power sequences used for a device. The power sequences
+reference resources in their steps, and setup the union member that corresponds
+to the resource's type. Resources, similarly, have a union which relevant member
+depends on their type.
+
+Note that the only "platform data" per se here is platform_power_seq_set. Other
+structures (power_seq and power_seq_resource) will be used at runtime and thus
+*must* survive initialization, so do not declare them with the __initdata
+attribute.
+
+The following example should make it clear how the platform data for power
+sequences is defined. It declares two power sequences named "power-on" and
+"power-off" for a backlight device. The "power-on" sequence enables the "power"
+regulator of the device, waits for 10ms, and then enables PWM "backlight" and
+set GPIO 28 to 1. "power-off" does the opposite.
+
+struct power_seq_resource reg_res = {
+ .type = POWER_SEQ_REGULATOR,
+ .regulator.id = "power",
+};
+
+struct power_seq_resource gpio_res = {
+ .type = POWER_SEQ_GPIO,
+ .gpio.gpio = 28,
+};
+
+struct power_seq_resource pwm_res = {
+ .type = POWER_SEQ_PWM,
+ .pwm.id = "backlight",
+};
+
+struct power_seq_resource delay_res = {
+ .type = POWER_SEQ_DELAY,
+};
+
+struct power_seq power_on_seq = {
+ .id = "power-on",
+ .num_steps = 4,
+ .steps = {
+ {
+ .resource = ®_res,
+ .regulator.enable = true,
+ }, {
+ .resource = &delay_res,
+ .delay.delay = 10000,
+ }, {
+ .resource = &pwm_res,
+ .pwm.enable = true,
+ }, {
+ .resource = &gpio_res,
+ .gpio.value = 1,
+ },
+ },
+};
+
+struct power_seq power_off_seq = {
+ .id = "power-off",
+ .num_steps = 4,
+ .steps = {
+ {
+ .resource = &gpio_res,
+ .gpio.value = 0,
+ }, {
+ .resource = &pwm_res,
+ .pwm.enable = false,
+ }, {
+ .resource = &delay_res,
+ .delay.delay = 10000,
+ }, {
+ .resource = ®_res,
+ .regulator.enable = false,
+ },
+ },
+};
+
+struct platform_power_seq_set backlight_power_seqs __initdata = {
+ .num_seqs = 2,
+ .seqs = {
+ &power_on_seq,
+ &power_off_seq,
+ },
+};
+
+"backlight_power_seqs" can then be passed to power_seq_set_add_sequences() in
+order to add the sequences to a set and allocate all the necessary resources.
+More on this later in this document.
+
+Device Tree
+-----------
+Power sequences can also be encoded as device tree nodes. The following
+properties and nodes are equivalent to the platform data defined previously:
+
+pwms = <&pwm 2 5000000>;
+pwm-names = "backlight";
+power-supply = <&vdd_bl_reg>;
+
+power-sequences {
+ power-on {
+ step0 {
+ type = "regulator";
+ id = "power";
+ enable;
+ };
+ step1 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step2 {
+ type = "pwm";
+ id = "backlight";
+ enable;
+ };
+ step3 {
+ type = "gpio";
+ gpio = <&gpio 28 0>;
+ value = <1>;
+ };
+ };
+
+ power-off {
+ step0 {
+ type = "gpio";
+ gpio = <&gpio 28 0>;
+ value = <0>;
+ };
+ step1 {
+ type = "pwm";
+ id = "backlight";
+ disable;
+ };
+ step2 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step3 {
+ type = "regulator";
+ id = "power";
+ disable;
+ };
+ };
+};
+
+See Documentation/devicetree/bindings/power/power_seq.txt for the complete
+syntax of the DT bindings.
+
+Use by Drivers and Resources Management
+---------------------------------------
+Power sequences make use of resources that must be properly allocated and
+managed. The power_seq_set structure manages the sequences and resources for a
+particular device. A driver willing to use power sequences will thus declare one
+instance of power_seq_set per device and initialize it at probe time:
+
+struct my_device_data {
+ struct device *dev;
+ ...
+ struct power_set_set power_seqs;
+ ...
+};
+
+power_seq_set_init(&my_device->power_seqs, my_device->dev);
+
+The power_seq_set_add_sequence() and power_seq_set_add_sequences() functions are
+then used to add one or several sequences to a set. These functions will also
+allocate all the resources used by the sequence(s) and make sure they are ready
+to be run. All resources are allocated through devm and will thus be freed when
+the set's device is removed.
+
+ int power_seq_set_add_sequence(struct power_seq_set *set,
+ struct power_seq *seq);
+ int power_seq_set_add_sequences(struct power_seq_set *set,
+ struct platform_power_seq_set *seqs);
+
+Power sequences added to a set can then be resolved by their name using
+power_seq_lookup():
+
+ struct power_seq *power_seq_lookup(struct power_seq_set *seqs,
+ const char *id);
+
+power_seq_lookup() returns a ready-to-run pointer to the power sequence which
+name matches the id parameter.
+
+A retrieved power sequence can then be executed by power_seq_run:
+
+ int power_seq_run(struct power_seq *seq);
+
+It returns 0 if the sequence has successfully been run, or an error code if a
+problem occurred.
+
+Sometimes, you may want to browse the list of resources allocated for the
+sequences of a device, for instance to ensure that a resource of a given type is
+present. The power_seq_for_each_resource() macro does this:
+
+ power_seq_for_each_resource(pos, seqs)
+
+Here "pos" will be a pointer to a struct power_seq_resource. This structure
+contains the type of the resource, the information used for identifying it, and
+the resolved resource itself.
+
+Finally, users of the device tree can obtain a platform_power_seq_set structure
+built from the device's node using devm_of_parse_power_seq_set:
+
+ struct platform_power_seq_set *devm_of_parse_power_seq_set(struct device *dev);
+
+The power sequences must be declared under a "power-sequences" node directly
+declared under the device's node. Detailed syntax contained in Documentation/devicetree/bindings/power/power_seq.txt. As the function name
+states, all memory is allocated through devm. The returned
+platform_power_seq_set can be freed after being added to a set, but the
+sequences themselves must be preserved until they are freed by devm.
\ No newline at end of file
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 49a8939..f20d449 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -338,3 +338,4 @@ config AB8500_BATTERY_THERM_ON_BATCTRL
endif # POWER_SUPPLY
source "drivers/power/avs/Kconfig"
+source "drivers/power/power_seq/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index b949cf8..883ad4d 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -49,3 +49,4 @@ obj-$(CONFIG_CHARGER_MAX8997) += max8997_charger.o
obj-$(CONFIG_CHARGER_MAX8998) += max8998_charger.o
obj-$(CONFIG_POWER_AVS) += avs/
obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o
+obj-$(CONFIG_POWER_SEQ) += power_seq/
diff --git a/drivers/power/power_seq/Kconfig b/drivers/power/power_seq/Kconfig
new file mode 100644
index 0000000..0ece819
--- /dev/null
+++ b/drivers/power/power_seq/Kconfig
@@ -0,0 +1,2 @@
+config POWER_SEQ
+ tristate
diff --git a/drivers/power/power_seq/Makefile b/drivers/power/power_seq/Makefile
new file mode 100644
index 0000000..964b91e
--- /dev/null
+++ b/drivers/power/power_seq/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_POWER_SEQ) += power_seq.o
+power_seq-y := core.o delay.o regulator.o gpio.o pwm.o
diff --git a/drivers/power/power_seq/core.c b/drivers/power/power_seq/core.c
new file mode 100644
index 0000000..d51b9b8
--- /dev/null
+++ b/drivers/power/power_seq/core.c
@@ -0,0 +1,362 @@
+/*
+ * core.c - power sequence interpreter for platform devices and device tree
+ *
+ * Author: Alexandre Courbot <acourbot@nvidia.com>
+ *
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include <linux/power_seq.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/device.h>
+#include <linux/of.h>
+
+#include "power_seq_priv.h"
+
+static const struct power_seq_res_ops *power_seq_ops[POWER_SEQ_NUM_TYPES];
+
+#ifdef CONFIG_OF
+int of_power_seq_parse_enable_properties(struct device_node *node,
+ struct power_seq *seq,
+ unsigned int step_nbr, bool *enable)
+{
+ if (of_find_property(node, "enable", NULL)) {
+ *enable = true;
+ } else if (of_find_property(node, "disable", NULL)) {
+ *enable = false;
+ } else {
+ power_seq_err(seq, step_nbr,
+ "missing enable or disable property\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int of_power_seq_parse_step(struct device *dev,
+ struct device_node *node,
+ struct power_seq *seq,
+ unsigned int step_nbr,
+ struct list_head *resources)
+{
+ struct power_seq_step *step = &seq->steps[step_nbr];
+ struct power_seq_resource res, *res2;
+ const char *type;
+ unsigned int i;
+ int err;
+
+ err = of_property_read_string(node, "type", &type);
+ if (err < 0) {
+ power_seq_err(seq, step_nbr, "cannot read type property\n");
+ return err;
+ }
+ for (i = 0; i < POWER_SEQ_NUM_TYPES; i++) {
+ if (power_seq_ops[i]->name = NULL)
+ continue;
+ if (!strcmp(type, power_seq_ops[i]->name))
+ break;
+ }
+ if (i >= POWER_SEQ_NUM_TYPES) {
+ power_seq_err(seq, step_nbr, "unknown type %s\n", type);
+ return -EINVAL;
+ }
+ memset(&res, 0, sizeof(res));
+ res.type = i;
+ err = power_seq_ops[res.type]->of_parse(node, seq, step_nbr, &res);
+ if (err < 0)
+ return err;
+
+ /* Use the same instance of the resource if met before */
+ list_for_each_entry(res2, resources, list) {
+ if (res.type = res2->type &&
+ power_seq_ops[res.type]->res_compare(&res, res2))
+ break;
+ }
+ /* Resource never met before, create it */
+ if (&res2->list = resources) {
+ res2 = devm_kzalloc(dev, sizeof(*res2), GFP_KERNEL);
+ if (!res2)
+ return -ENOMEM;
+ memcpy(res2, &res, sizeof(res));
+ list_add_tail(&res2->list, resources);
+ }
+ step->resource = res2;
+
+ return 0;
+}
+
+static struct power_seq *of_parse_power_seq(struct device *dev,
+ struct device_node *node,
+ struct list_head *resources)
+{
+ struct device_node *child = NULL;
+ struct power_seq *pseq;
+ unsigned int sz;
+ int num_steps;
+ int err;
+
+ if (!node)
+ return ERR_PTR(-EINVAL);
+
+ num_steps = of_get_child_count(node);
+ sz = sizeof(*pseq) + sizeof(pseq->steps[0]) * num_steps;
+ pseq = devm_kzalloc(dev, sz, GFP_KERNEL);
+ if (!pseq)
+ return ERR_PTR(-ENOMEM);
+ pseq->id = node->name;
+ pseq->num_steps = num_steps;
+
+ for_each_child_of_node(node, child) {
+ unsigned int pos;
+
+ /* Check that the name's format is correct and within bounds */
+ if (strncmp("step", child->name, 4)) {
+ err = -EINVAL;
+ goto parse_error;
+ }
+
+ err = kstrtouint(child->name + 4, 10, &pos);
+ if (err < 0)
+ goto parse_error;
+
+ /* Invalid step index or step already parsed? */
+ if (pos >= num_steps || pseq->steps[pos].resource != NULL) {
+ err = -EINVAL;
+ goto parse_error;
+ }
+
+ err = of_power_seq_parse_step(dev, child, pseq, pos, resources);
+ if (err)
+ return ERR_PTR(err);
+ }
+
+ return pseq;
+
+parse_error:
+ dev_err(dev, "%s: invalid power step name %s!\n", pseq->id,
+ child->name);
+ return ERR_PTR(err);
+}
+
+/**
+ * devm_of_parse_power_seq_set - build a power_seq_set from the device tree
+ * @dev: Device to parse the power sequences of
+ *
+ * Sequences must be contained into a subnode named "power-sequences" of the
+ * device root node.
+ *
+ * Memory for the sequence is allocated using devm_kzalloc on dev. The returned
+ * platform_power_seq_set can be freed by devm_kfree after the sequences have
+ * been added, but the sequences themselves must be preserved.
+ *
+ * Returns the built set on success, or an error code in case of failure.
+ */
+struct platform_power_seq_set *devm_of_parse_power_seq_set(struct device *dev)
+{
+ struct platform_power_seq_set *set;
+ struct device_node *root = dev->of_node;
+ struct device_node *seq;
+ struct list_head resources;
+ unsigned int sz;
+ int n;
+
+ if (!root)
+ return NULL;
+
+ root = of_find_node_by_name(root, "power-sequences");
+ if (!root)
+ return NULL;
+
+ n = of_get_child_count(root);
+ sz = sizeof(*set) + sizeof(struct power_seq *) * n;
+ set = devm_kzalloc(dev, sz, GFP_KERNEL);
+ if (!set)
+ return ERR_PTR(-ENOMEM);
+ set->num_seqs = n;
+
+ n = 0;
+ INIT_LIST_HEAD(&resources);
+ for_each_child_of_node(root, seq) {
+ struct power_seq *pseq;
+
+ pseq = of_parse_power_seq(dev, seq, &resources);
+ if (IS_ERR(pseq))
+ return (void *)pseq;
+
+ set->seqs[n++] = pseq;
+ }
+
+ return set;
+}
+EXPORT_SYMBOL_GPL(devm_of_parse_power_seq_set);
+#endif /* CONFIG_OF */
+
+/**
+ * power_seq_set_init - initialize a power_seq_set
+ * @set: Set to initialize
+ * @dev: Device this set is going to belong to
+ */
+void power_seq_set_init(struct power_seq_set *set, struct device *dev)
+{
+ set->dev = dev;
+ INIT_LIST_HEAD(&set->resources);
+ INIT_LIST_HEAD(&set->seqs);
+}
+EXPORT_SYMBOL_GPL(power_seq_set_init);
+
+/**
+ * power_seq_add_sequence - add a power sequence to a set
+ * @set: Set to add the sequence to
+ * @seq: Sequence to add
+ *
+ * This step will check that all the resources used by the sequence are
+ * allocated. If they are not, an attempt to allocate them is made. This
+ * operation can fail and and return an error code.
+ *
+ * Returns 0 on success, error code if a resource initialization failed.
+ */
+int power_seq_add_sequence(struct power_seq_set *set, struct power_seq *seq)
+{
+ struct power_seq_resource *res;
+ unsigned int i;
+ int err;
+
+ for (i = 0; i < seq->num_steps; i++) {
+ struct power_seq_step *step = &seq->steps[i];
+ struct power_seq_resource *step_res = step->resource;
+
+ /* support for resource type not compiled in? */
+ if (power_seq_ops[step_res->type]->name = NULL) {
+ power_seq_err(seq, i, "unsupported resource type\n");
+ return -EINVAL;
+ }
+
+ /* resource already allocated from a previous step ? */
+ list_for_each_entry(res, &set->resources, list) {
+ if (res = step_res)
+ break;
+ }
+ /* resource not allocated yet, allocate and add it */
+ if (&res->list = &set->resources) {
+ err = power_seq_ops[step_res->type]->res_alloc(set->dev,
+ step_res);
+ if (err)
+ return err;
+ list_add_tail(&step->resource->list, &set->resources);
+ }
+ }
+
+ list_add_tail(&seq->list, &set->seqs);
+ seq->set = set;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(power_seq_add_sequence);
+
+/**
+ * power_seq_add_sequences - add power sequences defined as platform data
+ * @set: Set to add the sequences to
+ * @seqs: Sequences to add
+ *
+ * See power_seq_add_sequence for more details.
+ *
+ * Returns 0 on success, error code if a resource initialization failed.
+ */
+int power_seq_set_add_sequences(struct power_seq_set *set,
+ struct platform_power_seq_set *seqs)
+{
+ unsigned int i;
+ int ret;
+
+ for (i = 0; i < seqs->num_seqs; i++) {
+ ret = power_seq_add_sequence(set, seqs->seqs[i]);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(power_seq_set_add_sequences);
+
+/**
+ * power_seq_lookup - Lookup a power sequence by name from a set
+ * @seqs: The set to look in
+ * @id: Name to look after
+ *
+ * Returns a matching power sequence if it exists, NULL if it does not.
+ */
+struct power_seq *power_seq_lookup(struct power_seq_set *set, const char *id)
+{
+ struct power_seq *seq;
+
+ list_for_each_entry(seq, &set->seqs, list) {
+ if (!strcmp(seq->id, id))
+ return seq;
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(power_seq_lookup);
+
+/**
+ * power_seq_run() - run a power sequence
+ * @seq: The power sequence to run
+ *
+ * Returns 0 on success, error code in case of failure.
+ */
+int power_seq_run(struct power_seq *seq)
+{
+ unsigned int i;
+ int err;
+
+ if (!seq)
+ return 0;
+
+ if (!seq->set) {
+ pr_err("cannot run a sequence not added to a set");
+ return -EINVAL;
+ }
+
+ for (i = 0; i < seq->num_steps; i++) {
+ unsigned int type = seq->steps[i].resource->type;
+
+ err = power_seq_ops[type]->step_run(&seq->steps[i]);
+ if (err) {
+ power_seq_err(seq, i,
+ "error %d while running power sequence step\n",
+ err);
+ return err;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(power_seq_run);
+
+/* defined in power_seq_*.c files */
+extern const struct power_seq_res_ops power_seq_delay_ops;
+extern const struct power_seq_res_ops power_seq_gpio_ops;
+extern const struct power_seq_res_ops power_seq_regulator_ops;
+extern const struct power_seq_res_ops power_seq_pwm_ops;
+
+static const struct power_seq_res_ops *power_seq_ops[POWER_SEQ_NUM_TYPES] = {
+ [POWER_SEQ_DELAY] = &power_seq_delay_ops,
+ [POWER_SEQ_REGULATOR] = &power_seq_regulator_ops,
+ [POWER_SEQ_PWM] = &power_seq_gpio_ops,
+ [POWER_SEQ_GPIO] = &power_seq_pwm_ops,
+};
+
+MODULE_AUTHOR("Alexandre Courbot <acourbot@nvidia.com>");
+MODULE_DESCRIPTION("Runtime Interpreted Power Sequences");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/power/power_seq/delay.c b/drivers/power/power_seq/delay.c
new file mode 100644
index 0000000..de6810b
--- /dev/null
+++ b/drivers/power/power_seq/delay.c
@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include "power_seq_priv.h"
+#include <linux/delay.h>
+
+#ifdef CONFIG_OF
+static int of_power_seq_parse_delay(struct device_node *node,
+ struct power_seq *seq,
+ unsigned int step_nbr,
+ struct power_seq_resource *res)
+{
+ struct power_seq_step *step = &seq->steps[step_nbr];
+ int err;
+
+ err = of_property_read_u32(node, "delay",
+ &step->delay.delay);
+ if (err < 0)
+ power_seq_err(seq, step_nbr, "error reading delay property\n");
+
+ return err;
+}
+#else
+#define of_power_seq_parse_delay NULL
+#endif
+
+static bool power_seq_res_compare_delay(struct power_seq_resource *res,
+ struct power_seq_resource *res2)
+{
+ /* Delay resources are just here to hold the type of steps, so they are
+ * all equivalent. */
+ return true;
+}
+
+static int power_seq_res_alloc_delay(struct device *dev,
+ struct power_seq_resource *res)
+{
+ return 0;
+}
+
+static int power_seq_step_run_delay(struct power_seq_step *step)
+{
+ usleep_range(step->delay.delay,
+ step->delay.delay + 1000);
+
+ return 0;
+}
+
+const struct power_seq_res_ops power_seq_delay_ops = {
+ .name = "delay",
+ .of_parse = of_power_seq_parse_delay,
+ .step_run = power_seq_step_run_delay,
+ .res_compare = power_seq_res_compare_delay,
+ .res_alloc = power_seq_res_alloc_delay,
+};
diff --git a/drivers/power/power_seq/gpio.c b/drivers/power/power_seq/gpio.c
new file mode 100644
index 0000000..0cd143a
--- /dev/null
+++ b/drivers/power/power_seq/gpio.c
@@ -0,0 +1,95 @@
+/*
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include "power_seq_priv.h"
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+
+#ifdef CONFIG_OF
+static int of_power_seq_parse_gpio(struct device_node *node,
+ struct power_seq *seq,
+ unsigned int step_nbr,
+ struct power_seq_resource *res)
+{
+ struct power_seq_step *step = &seq->steps[step_nbr];
+ int gpio;
+ int err;
+
+ gpio = of_get_named_gpio(node, "gpio", 0);
+ if (gpio < 0) {
+ power_seq_err(seq, step_nbr, "error reading gpio property\n");
+ return gpio;
+ }
+ res->gpio.gpio = gpio;
+
+ err = of_property_read_u32(node, "value", &step->gpio.value);
+ if (err < 0) {
+ power_seq_err(seq, step_nbr, "error reading value property\n");
+ } else if (step->gpio.value < 0 || step->gpio.value > 1) {
+ power_seq_err(seq, step_nbr,
+ "value out of range (must be 0 or 1)\n");
+ err = -EINVAL;
+ }
+
+ return err;
+}
+#else
+#define of_power_seq_parse_gpio NULL
+#endif
+
+static bool power_seq_res_compare_gpio(struct power_seq_resource *res,
+ struct power_seq_resource *res2)
+{
+ return res->gpio.gpio = res2->gpio.gpio;
+}
+
+static int power_seq_res_alloc_gpio(struct device *dev,
+ struct power_seq_resource *res)
+{
+ int err;
+
+ err = devm_gpio_request(dev, res->gpio.gpio, dev_name(dev));
+ if (err) {
+ dev_err(dev, "cannot get gpio %d\n", res->gpio.gpio);
+ return err;
+ }
+
+ return 0;
+}
+
+static int power_seq_step_run_gpio(struct power_seq_step *step)
+{
+ struct power_seq_resource *res = step->resource;
+
+ /* set the GPIO direction at first use */
+ if (!res->gpio.is_set) {
+ int err = gpio_direction_output(res->gpio.gpio,
+ step->gpio.value);
+ if (err)
+ return err;
+ res->gpio.is_set = true;
+ } else {
+ gpio_set_value_cansleep(res->gpio.gpio, step->gpio.value);
+ }
+
+ return 0;
+}
+
+const struct power_seq_res_ops power_seq_gpio_ops = {
+ .name = "gpio",
+ .of_parse = of_power_seq_parse_gpio,
+ .step_run = power_seq_step_run_gpio,
+ .res_compare = power_seq_res_compare_gpio,
+ .res_alloc = power_seq_res_alloc_gpio,
+};
diff --git a/drivers/power/power_seq/power_seq_priv.h b/drivers/power/power_seq/power_seq_priv.h
new file mode 100644
index 0000000..fa56e21
--- /dev/null
+++ b/drivers/power/power_seq/power_seq_priv.h
@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#ifndef POWER_SEQ_PRIV_H
+#define POWER_SEQ_PRIV_H
+
+#include <linux/device.h>
+#include <linux/power_seq.h>
+#include <linux/of.h>
+
+#define power_seq_err(seq, step_nbr, format, ...) \
+ dev_err(seq->set->dev, "%s[%d]: " format, seq->id, step_nbr, \
+ ##__VA_ARGS__);
+
+#ifdef CONFIG_OF
+int of_power_seq_parse_enable_properties(struct device_node *node,
+ struct power_seq *seq,
+ unsigned int step_nbr, bool *enable);
+#endif
+
+/**
+ * struct power_seq_res_ops - operators for power sequences resources
+ * @name: Name of the resource type. Set to null when a resource
+ * type support is not compiled in
+ * @of_parse: Parse a step for this kind of resource from a device
+ * tree node. The result of parsing must be written into
+ * step step_nbr of seq
+ * @step_run: Run a step for this kind of resource
+ * @res_compare: Return true if the resource used by the resource is the
+ * same as the one referenced by the step, false otherwise.
+ * @res_alloc: Resolve and allocate a resource. Return error code if
+ * the resource cannot be allocated, 0 otherwise
+ */
+struct power_seq_res_ops {
+ const char *name;
+ int (*of_parse)(struct device_node *node, struct power_seq *seq,
+ unsigned int step_nbr, struct power_seq_resource *res);
+ int (*step_run)(struct power_seq_step *step);
+ bool (*res_compare)(struct power_seq_resource *res,
+ struct power_seq_resource *res2);
+ int (*res_alloc)(struct device *dev,
+ struct power_seq_resource *res);
+};
+
+#endif
diff --git a/drivers/power/power_seq/pwm.c b/drivers/power/power_seq/pwm.c
new file mode 100644
index 0000000..a74b768
--- /dev/null
+++ b/drivers/power/power_seq/pwm.c
@@ -0,0 +1,85 @@
+/*
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include "power_seq_priv.h"
+
+#ifdef CONFIG_PWM
+
+#include <linux/pwm.h>
+
+#ifdef CONFIG_OF
+static int of_power_seq_parse_pwm(struct device_node *node,
+ struct power_seq *seq,
+ unsigned int step_nbr,
+ struct power_seq_resource *res)
+{
+ struct power_seq_step *step = &seq->steps[step_nbr];
+ int err;
+
+ err = of_property_read_string(node, "id", &res->pwm.id);
+ if (err) {
+ power_seq_err(seq, step_nbr, "error reading id property\n");
+ return err;
+ }
+
+ err = of_power_seq_parse_enable_properties(node, seq, step_nbr,
+ &step->pwm.enable);
+ return err;
+}
+#else
+#define of_power_seq_parse_pwm NULL
+#endif
+
+static bool power_seq_res_compare_pwm(struct power_seq_resource *res,
+ struct power_seq_resource *res2)
+{
+ return !strcmp(res->pwm.id, res2->pwm.id);
+}
+
+static int power_seq_res_alloc_pwm(struct device *dev,
+ struct power_seq_resource *res)
+{
+ res->pwm.pwm = devm_pwm_get(dev, res->pwm.id);
+ if (IS_ERR(res->pwm.pwm)) {
+ dev_err(dev, "cannot get pwm \"%s\"\n", res->pwm.id);
+ return PTR_ERR(res->pwm.pwm);
+ }
+
+ return 0;
+}
+
+static int power_seq_step_run_pwm(struct power_seq_step *step)
+{
+ if (step->pwm.enable) {
+ return pwm_enable(step->resource->pwm.pwm);
+ } else {
+ pwm_disable(step->resource->pwm.pwm);
+ return 0;
+ }
+}
+
+const struct power_seq_res_ops power_seq_pwm_ops = {
+ .name = "pwm",
+ .of_parse = of_power_seq_parse_pwm,
+ .step_run = power_seq_step_run_pwm,
+ .res_compare = power_seq_res_compare_pwm,
+ .res_alloc = power_seq_res_alloc_pwm,
+};
+
+#else
+
+const struct power_seq_res_ops power_seq_pwm_ops = {
+};
+
+#endif
diff --git a/drivers/power/power_seq/regulator.c b/drivers/power/power_seq/regulator.c
new file mode 100644
index 0000000..4663dbc
--- /dev/null
+++ b/drivers/power/power_seq/regulator.c
@@ -0,0 +1,87 @@
+/*
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include "power_seq_priv.h"
+
+#ifdef CONFIG_REGULATOR
+
+#include <linux/err.h>
+#include <linux/regulator/consumer.h>
+
+#ifdef CONFIG_OF
+static int of_power_seq_parse_regulator(struct device_node *node,
+ struct power_seq *seq,
+ unsigned int step_nbr,
+ struct power_seq_resource *res)
+{
+ struct power_seq_step *step = &seq->steps[step_nbr];
+ int err;
+
+ err = of_property_read_string(node, "id",
+ &res->regulator.id);
+ if (err) {
+ power_seq_err(seq, step_nbr, "error reading id property\n");
+ return err;
+ }
+
+ err = of_power_seq_parse_enable_properties(node, seq, step_nbr,
+ &step->regulator.enable);
+ return err;
+}
+#else
+#define of_power_seq_parse_regulator NULL
+#endif
+
+static bool
+power_seq_res_compare_regulator(struct power_seq_resource *res,
+ struct power_seq_resource *res2)
+{
+ return !strcmp(res->regulator.id, res2->regulator.id);
+}
+
+static int power_seq_res_alloc_regulator(struct device *dev,
+ struct power_seq_resource *res)
+{
+ res->regulator.regulator = devm_regulator_get(dev, res->regulator.id);
+ if (IS_ERR(res->regulator.regulator)) {
+ dev_err(dev, "cannot get regulator \"%s\"\n",
+ res->regulator.id);
+ return PTR_ERR(res->regulator.regulator);
+ }
+
+ return 0;
+}
+
+static int power_seq_step_run_regulator(struct power_seq_step *step)
+{
+ if (step->regulator.enable)
+ return regulator_enable(step->resource->regulator.regulator);
+ else
+ return regulator_disable(step->resource->regulator.regulator);
+}
+
+const struct power_seq_res_ops power_seq_regulator_ops = {
+ .name = "regulator",
+ .of_parse = of_power_seq_parse_regulator,
+ .step_run = power_seq_step_run_regulator,
+ .res_compare = power_seq_res_compare_regulator,
+ .res_alloc = power_seq_res_alloc_regulator,
+};
+
+#else
+
+const struct power_seq_res_ops power_seq_regulator_ops = {
+};
+
+#endif
diff --git a/include/linux/power_seq.h b/include/linux/power_seq.h
new file mode 100644
index 0000000..21b95b6
--- /dev/null
+++ b/include/linux/power_seq.h
@@ -0,0 +1,203 @@
+/*
+ * power_seq.h
+ *
+ * Simple interpreter for power sequences defined as platform data or device
+ * tree properties.
+ *
+ * Power sequences are designed to replace the callbacks typically used in
+ * board-specific files that implement board- or device- specific power
+ * sequences (such as those of backlights). A power sequence is an array of
+ * steps referencing resources (regulators, GPIOs, PWMs, ...) with an action to
+ * perform on them. By having the power sequences interpreted, it becomes
+ * possible to describe them in the device tree and thus to remove
+ * board-specific files from the kernel.
+ *
+ * See Documentation/power/power_seqs.txt for detailed information.
+ *
+ * Author: Alexandre Courbot <acourbot@nvidia.com>
+ *
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#ifndef __LINUX_POWER_SEQ_H
+#define __LINUX_POWER_SEQ_H
+
+#include <linux/types.h>
+#include <linux/list.h>
+
+struct device;
+struct regulator;
+struct pwm_device;
+
+/**
+ * The different kinds of resources that can be controlled by the sequences
+ */
+enum power_seq_res_type {
+ POWER_SEQ_DELAY,
+ POWER_SEQ_REGULATOR,
+ POWER_SEQ_PWM,
+ POWER_SEQ_GPIO,
+ POWER_SEQ_NUM_TYPES,
+};
+
+/**
+ * struct power_seq_regulator_resource
+ * @id: name of the regulator
+ * @regulator: resolved regulator. Written during resource resolution.
+ */
+struct power_seq_regulator_resource {
+ const char *id;
+ struct regulator *regulator;
+};
+
+/**
+ * struct power_seq_pwm_resource
+ * @id: name of the PWM
+ * @regulator: resolved PWM. Written during resource resolution.
+ */
+struct power_seq_pwm_resource {
+ const char *id;
+ struct pwm_device *pwm;
+};
+
+/**
+ * struct power_seq_gpio_resource
+ * @gpio: number of the GPIO
+ * @is_set: track GPIO state to set its direction at first use
+ */
+struct power_seq_gpio_resource {
+ int gpio;
+ bool is_set;
+};
+
+/**
+ * struct power_seq_resource - resource used by power sequences
+ * @type: type of the resource. This decides which member of the union is
+ * used for this resource
+ * @list: link resources together in power_seq_set
+ * @regulator: used if @type = POWER_SEQ_REGULATOR
+ * @pwm: used if @type = POWER_SEQ_PWM
+ * @gpio: used if @type = POWER_SEQ_GPIO
+ */
+struct power_seq_resource {
+ enum power_seq_res_type type;
+ struct list_head list;
+ union {
+ struct power_seq_regulator_resource regulator;
+ struct power_seq_pwm_resource pwm;
+ struct power_seq_gpio_resource gpio;
+ };
+};
+#define power_seq_for_each_resource(pos, set) \
+ list_for_each_entry(pos, &(set)->resources, list)
+
+/**
+ * struct power_seq_delay_step - action data for delay steps
+ * @delay: amount of time to wait, in microseconds
+ */
+struct power_seq_delay_step {
+ unsigned int delay;
+};
+
+/**
+ * struct power_seq_regulator_step - platform data for regulator steps
+ * @enable: whether to enable or disable the regulator during this step
+ */
+struct power_seq_regulator_step {
+ bool enable;
+};
+
+/**
+ * struct power_seq_pwm_step - action data for PWM steps
+ * @enable: whether to enable or disable the PWM during this step
+ */
+struct power_seq_pwm_step {
+ bool enable;
+};
+
+/**
+ * struct power_seq_gpio_step - action data for GPIO steps
+ * @enable: whether to enable or disable the GPIO during this step
+ */
+struct power_seq_gpio_step {
+ int value;
+};
+
+/**
+ * struct power_seq_step - data for power sequences steps
+ * @resource: resource used by this step
+ * @delay: used if resource->type = POWER_SEQ_DELAY
+ * @regulator: used if resource->type = POWER_SEQ_REGULATOR
+ * @pwm: used if resource->type = POWER_SEQ_PWN
+ * @gpio: used if resource->type = POWER_SEQ_GPIO
+ */
+struct power_seq_step {
+ struct power_seq_resource *resource;
+ union {
+ struct power_seq_delay_step delay;
+ struct power_seq_regulator_step regulator;
+ struct power_seq_pwm_step pwm;
+ struct power_seq_gpio_step gpio;
+ };
+};
+
+struct power_seq_set;
+
+/**
+ * struct power_seq - single power sequence
+ * @id: name of this sequence
+ * @list: link sequences together in power_seq_set. Leave as-is
+ * @set: set this sequence belongs to. Written when added to a set
+ * @num_steps: number of steps in the sequence
+ * @steps: array of steps that make the sequence
+ */
+struct power_seq {
+ const char *id;
+ struct list_head list;
+ struct power_seq_set *set;
+ unsigned int num_steps;
+ struct power_seq_step steps[];
+};
+
+/**
+ * struct power_seq_set - power sequences and resources used by a device
+ * @dev: device this set belongs to
+ * @resources: list of resources used by power sequences
+ * @seqs: list of power sequences
+ */
+struct power_seq_set {
+ struct device *dev;
+ struct list_head resources;
+ struct list_head seqs;
+};
+
+/**
+ * struct platform_power_seq_set - define power sequences as platform data
+ * @num_seqs: number of sequences defined
+ * @seqs: array of num_seqs power sequences
+ */
+struct platform_power_seq_set {
+ unsigned int num_seqs;
+ struct power_seq *seqs[];
+};
+
+struct platform_power_seq_set *devm_of_parse_power_seq_set(struct device *dev);
+void power_seq_set_init(struct power_seq_set *set, struct device *dev);
+int power_seq_set_add_sequence(struct power_seq_set *set,
+ struct power_seq *seq);
+int power_seq_set_add_sequences(struct power_seq_set *set,
+ struct platform_power_seq_set *seqs);
+struct power_seq *power_seq_lookup(struct power_seq_set *seqs, const char *id);
+int power_seq_run(struct power_seq *seq);
+
+#endif
--
1.8.0
^ permalink raw reply related
* [PATCHv9 0/3] Runtime Interpreted Power Sequences
From: Alexandre Courbot @ 2012-11-17 10:55 UTC (permalink / raw)
To: linux-arm-kernel
Apologies for sending two patchsets in two days - the main purpose
of this new revision is to add the linux-arm-kernel list to the
audience. A few suggestions from v8 have also been added.
Changelog from v8:
- Compile resource support into different compilation units
- Check that resource support is compiled in when resolving sequences
- Now compilable as a module
- Renamed source files to avoid repeated power_seq in their path
- Add linux-arm-kernel list to audience as suggested by Mark Rutland
Alexandre Courbot (3):
Runtime Interpreted Power Sequences
pwm_backlight: use power sequences
Take maintainership of power sequences
.../devicetree/bindings/power/power_seq.txt | 121 +++++++
.../bindings/video/backlight/pwm-backlight.txt | 63 +++-
Documentation/power/power_seq.txt | 253 ++++++++++++++
MAINTAINERS | 10 +
drivers/power/Kconfig | 1 +
drivers/power/Makefile | 1 +
drivers/power/power_seq/Kconfig | 2 +
drivers/power/power_seq/Makefile | 2 +
drivers/power/power_seq/core.c | 362 +++++++++++++++++++++
drivers/power/power_seq/delay.c | 66 ++++
drivers/power/power_seq/gpio.c | 95 ++++++
drivers/power/power_seq/power_seq_priv.h | 56 ++++
drivers/power/power_seq/pwm.c | 85 +++++
drivers/power/power_seq/regulator.c | 87 +++++
drivers/video/backlight/Kconfig | 1 +
drivers/video/backlight/pwm_bl.c | 160 +++++++--
include/linux/power_seq.h | 203 ++++++++++++
include/linux/pwm_backlight.h | 18 +-
18 files changed, 1546 insertions(+), 40 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/power_seq.txt
create mode 100644 Documentation/power/power_seq.txt
create mode 100644 drivers/power/power_seq/Kconfig
create mode 100644 drivers/power/power_seq/Makefile
create mode 100644 drivers/power/power_seq/core.c
create mode 100644 drivers/power/power_seq/delay.c
create mode 100644 drivers/power/power_seq/gpio.c
create mode 100644 drivers/power/power_seq/power_seq_priv.h
create mode 100644 drivers/power/power_seq/pwm.c
create mode 100644 drivers/power/power_seq/regulator.c
create mode 100644 include/linux/power_seq.h
--
1.8.0
^ permalink raw reply
* Re: [PATCH v8 1/3] Runtime Interpreted Power Sequences
From: Alexandre Courbot @ 2012-11-17 10:12 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Stephen Warren, Thierry Reding, Mark Zhang, Grant Likely,
Rob Herring, Mark Brown, David Woodhouse, Arnd Bergmann,
Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-pm@vger.kernel.org
In-Reply-To: <20121116122540.GA2198@lizard>
On Fri, Nov 16, 2012 at 9:25 PM, Anton Vorontsov <cbouatmailru@gmail.com> wrote:
>> The POWER_SEQ_*_TYPE macros are defined in the C files. It's the
>> simplest way to initialize this table, and the code inside these C
>> files is short and simple enough that I thought I would be forgiven.
>> :)
>
> I think in the header file you could just write
>
> extern ..... power_seq_delay_type;
> #ifndef ...
> #define power_seq_delay_type NULL
> #endif
>
> And then, in the .c file:
>
> static const struct power_seq_res_ops power_seq_ops[POWER_SEQ_NUM_TYPES] = {
> [POWER_SEQ_DELAY] = power_seq_delay_type,
> ...
> };
>
> And then you can stop including the .c files directly, and link them
> instead.
I have done something similar, but that avoids exporting resource ops
into the header file. I am still not satisfied with it though - I'd
rather have one init function per resource file that changes the
pointer of the ops table from NULL to its own set of functions. That
way the core could live without knowing anything about the different
resources, and that would be nicer. Unfortunately modules only support
one init function, so this is not doable here.
>> >nit: one variable declaration per line.
>>
>> Fair enough - but is that a convention? checkpatch.pl was happy with these.
>
> It's not a rule, although there is a small passage about it in CodingStyle
> file when it describes commenting style. Though, some folks choose to
> ignore this suggestion quite frequently, especially if variables have no
> comments.
>
> Often, the multiple declarations per line are used to hide ugly functions
> w/ tons of variables, or just to confuse the reader for the fun of it.
>
> There are exceptions, of course. E.g.
>
> int i, j, k; /* Our iterators. */
>
> Is perfectly fine.
>
> But
> int i, err;
>
> At least to me seems weird, this stuff is logically disjunct.
>
> Only human can truly differentiate when the stuff "looks better" together
> or apart, so that's why checkpatch doesn't complain. Personally, I use
> this rule of thumb: when in doubt, use just one declaration per line, it
> is always OK. :)
That totally makes sense - thanks for the precision.
Prepping another patchset with audience extended to linux-arm-kernel
as Mark suggested.
Alex.
^ permalink raw reply
* Re: [PATCH v8 3/3] Take maintainership of power sequences
From: Alexandre Courbot @ 2012-11-17 6:41 UTC (permalink / raw)
To: Stephen Warren
Cc: Anton Vorontsov, Stephen Warren, Thierry Reding, Mark Zhang,
Grant Likely, Rob Herring, Mark Brown, David Woodhouse,
Arnd Bergmann,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Leela Krishna Amudala,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
Linux Kernel Mailing List,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <50A6733F.7020101-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
On Sat, Nov 17, 2012 at 2:09 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> Acked-by: Stephen Warren <swarren@nvidia.com>
Thanks!
>> +POWER SEQUENCES
>> +M: Alexandre Courbot <acourbot@nvidia.com>
>> +S: Maintained
>
> Given you're presumably working on this on NVIDIA's time, perhaps make
> that "Supported" not "Maintained"?
Absolutely.
Alex.
^ permalink raw reply
* Re: [PATCH v8 1/3] Runtime Interpreted Power Sequences
From: Alexandre Courbot @ 2012-11-17 4:04 UTC (permalink / raw)
To: Mark Rutland
Cc: Anton Vorontsov, Stephen Warren, Thierry Reding, Mark Zhang,
Grant Likely, rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org,
Mark Brown, David Woodhouse, Arnd Bergmann,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Leela Krishna Amudala,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20121116103511.GB16084-NuALmloUBlrZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org>
Hi Mark,
On Fri, Nov 16, 2012 at 7:35 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> Given there are several ARM platforms that may have an interest in this, please
> consider posting this to the ARM mailing list:
> linux-arm-kernel@lists.infradead.org.
That's right. New revision on the way.
>> +Similarly, each power sequence declares its steps as sub-nodes of itself. Steps
>> +must be named sequentially, with the first step named step0, the second step1,
>> +etc. Failure to follow this rule will result in a parsing error.
>
> Could we not encode the step number in the unit-address? i.e. step@N rather than
> stepN.
That was the way I did it initially, but it has been pointed out that
doing so would require to have #address-cells and #size-cells in every
power sequence, as well as a "reg" property in every step (see
https://lkml.org/lkml/2012/7/31/454 ). Although I'd prefer to use the
@ notation too (and neither dtc nor the kernel complained when I did
it), I tend to think the current solution is less burdensome than
having these redundant properties.
>> +"gpio" type required properties:
>> + - gpio: phandle of the GPIO to use.
>> + - value: value this GPIO should take. Must be 0 or 1.
>
> Is there any reason for id to be a name rather than a phandle? It seems
> inconsistent with the gpio case.
That's another long story. But to make it short, I'd like to make it
possible for power sequences to be referenced and shared between
devices of the same type (as everybody knows, copy/pasting is bad). If
we use phandles in steps, the power sequence becomes tied to the
referred resources and thus cannot be shared with another instance of
the same device. On the other hand, using an identifier that is
resolved at runtime (through e.g. regulator_get(device *, char *)
leverages the existing frameworks and makes things more flexible.
GPIO is currently the exception. It is the only framework for which
you cannot currently resolve a resource from a device and an
identifier. So at the moment we have to use a phandle - but we are
also working with Linus Walleij to provide an alternative GPIO API
that will be more like what we have for regulators/pinctrl/PWM/etc.
Another problem with phandles is that some of the functions that
resolve them are not publicly exported (i.e. AFAIK there is no public
function that returns a regulator from a phandle - the only to obtain
one is through regulator_get)
> I also see from the example below that the gpio property is not just a phandle,
> as it has the gpio-specifier appended. Is there a better way of describing this
> format in the documentation?
This is already clearly be defined in
Documentation/devicetree/bindings/gpio/, isn't it?
Thanks,
Alex.
^ permalink raw reply
* [PATCH v2] backlight: Add of_find_backlight_by_node() function
From: Thierry Reding @ 2012-11-16 22:29 UTC (permalink / raw)
To: Andrew Morton
Cc: Jingoo Han, Grant Likely, Richard Purdie,
Florian Tobias Schandinat, linux-fbdev, devicetree-discuss,
linux-kernel
This function finds the struct backlight_device for a given device tree
node. A dummy function is provided so that it safely compiles out if OF
support is disabled.
Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
Reviewed-by: Grant Likely <grant.likely@secretlab.ca>
Acked-by: Jingoo Han <jg1.han@samsung.com>
---
Changes in v2:
- use #ifdef CONFIG_OF instead of #if IS_ENABLED(CONFIG_OF)
- add kerneldoc to of_find_backlight_by_node() function
drivers/video/backlight/backlight.c | 29 +++++++++++++++++++++++++++++
include/linux/backlight.h | 10 ++++++++++
2 files changed, 39 insertions(+)
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 297db2f..345f666 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -370,6 +370,35 @@ void backlight_device_unregister(struct backlight_device *bd)
}
EXPORT_SYMBOL(backlight_device_unregister);
+#ifdef CONFIG_OF
+static int of_parent_match(struct device *dev, void *data)
+{
+ return dev->parent && dev->parent->of_node = data;
+}
+
+/**
+ * of_find_backlight_by_node() - find backlight device by device-tree node
+ * @node: device-tree node of the backlight device
+ *
+ * Returns a pointer to the backlight device corresponding to the given DT
+ * node or NULL if no such backlight device exists or if the device hasn't
+ * been probed yet.
+ *
+ * This function obtains a reference on the backlight device and it is the
+ * caller's responsibility to drop the reference by calling put_device() on
+ * the backlight device's .dev field.
+ */
+struct backlight_device *of_find_backlight_by_node(struct device_node *node)
+{
+ struct device *dev;
+
+ dev = class_find_device(backlight_class, NULL, node, of_parent_match);
+
+ return dev ? to_backlight_device(dev) : NULL;
+}
+EXPORT_SYMBOL(of_find_backlight_by_node);
+#endif
+
static void __exit backlight_class_exit(void)
{
class_destroy(backlight_class);
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 5ffc6dd..da9a082 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -134,4 +134,14 @@ struct generic_bl_info {
void (*kick_battery)(void);
};
+#ifdef CONFIG_OF
+struct backlight_device *of_find_backlight_by_node(struct device_node *node);
+#else
+static inline struct backlight_device *
+of_find_backlight_by_node(struct device_node *node)
+{
+ return NULL;
+}
+#endif
+
#endif
--
1.8.0
^ permalink raw reply related
* Re: [PATCH] backlight: Add of_find_backlight_by_node() function
From: Thierry Reding @ 2012-11-16 21:34 UTC (permalink / raw)
To: Andrew Morton
Cc: Richard Purdie, Florian Tobias Schandinat, linux-fbdev,
devicetree-discuss, linux-kernel
In-Reply-To: <20121116131645.c979837c.akpm@linux-foundation.org>
[-- Attachment #1: Type: text/plain, Size: 3055 bytes --]
On Fri, Nov 16, 2012 at 01:16:45PM -0800, Andrew Morton wrote:
> On Fri, 9 Nov 2012 15:04:38 +0100
> Thierry Reding <thierry.reding@avionic-design.de> wrote:
>
> > This function finds the struct backlight_device for a given device tree
> > node. A dummy function is provided so that it safely compiles out if OF
> > support is disabled.
> >
> > Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> > ---
> > drivers/video/backlight/backlight.c | 17 +++++++++++++++++
> > include/linux/backlight.h | 10 ++++++++++
> > 2 files changed, 27 insertions(+)
> >
> > diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> > index 297db2f..0d1ed4f 100644
> > --- a/drivers/video/backlight/backlight.c
> > +++ b/drivers/video/backlight/backlight.c
> > @@ -370,6 +370,23 @@ void backlight_device_unregister(struct backlight_device *bd)
> > }
> > EXPORT_SYMBOL(backlight_device_unregister);
> >
> > +#if IS_ENABLED(CONFIG_OF)
>
> Using IS_ENABLED() was odd. We'll never support CONFIG_OF=m, so can't
> we use plain old "#ifdef CONFIG_OF" here?
>
> --- a/drivers/video/backlight/backlight.c~backlight-add-of_find_backlight_by_node-function-fix
> +++ a/drivers/video/backlight/backlight.c
> @@ -370,7 +370,7 @@ void backlight_device_unregister(struct
> }
> EXPORT_SYMBOL(backlight_device_unregister);
>
> -#if IS_ENABLED(CONFIG_OF)
> +#ifdef CONFIG_OF
> static int of_parent_match(struct device *dev, void *data)
> {
> return dev->parent && dev->parent->of_node == data;
> --- a/include/linux/backlight.h~backlight-add-of_find_backlight_by_node-function-fix
> +++ a/include/linux/backlight.h
> @@ -134,7 +134,7 @@ struct generic_bl_info {
> void (*kick_battery)(void);
> };
>
> -#if IS_ENABLED(CONFIG_OF)
> +#ifdef CONFIG_OF
> struct backlight_device *of_find_backlight_by_node(struct device_node *node);
> #else
> static inline struct backlight_device *
> _
Yes, that should work.
> > +static int of_parent_match(struct device *dev, void *data)
> > +{
> > + return dev->parent && dev->parent->of_node == data;
> > +}
> > +
> > +struct backlight_device *of_find_backlight_by_node(struct device_node *node)
> > +{
> > + struct device *dev;
> > +
> > + dev = class_find_device(backlight_class, NULL, node, of_parent_match);
> > +
> > + return dev ? to_backlight_device(dev) : NULL;
> > +}
> > +EXPORT_SYMBOL(of_find_backlight_by_node);
>
> It's a global, exported-to-modules function. We should document such
> major interfaces. Unless they are dead trivial, but I don't think this
> one is that simple. The semantics of the return value could be
> explained, and callers should be told that of_find_backlight_by_node()
> took a ref on the returned device, and that they need to run
> put_device(retval->dev), if retval was not NULL.
>
> And anything else which might be useful.
Yes, I should have thought about that. I'll add some proper kerneldoc
and repost.
Thanks for reviewing,
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] backlight: Add of_find_backlight_by_node() function
From: Andrew Morton @ 2012-11-16 21:16 UTC (permalink / raw)
To: Thierry Reding
Cc: Richard Purdie, Florian Tobias Schandinat, linux-fbdev,
devicetree-discuss, linux-kernel
In-Reply-To: <1352469878-4532-1-git-send-email-thierry.reding@avionic-design.de>
On Fri, 9 Nov 2012 15:04:38 +0100
Thierry Reding <thierry.reding@avionic-design.de> wrote:
> This function finds the struct backlight_device for a given device tree
> node. A dummy function is provided so that it safely compiles out if OF
> support is disabled.
>
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> ---
> drivers/video/backlight/backlight.c | 17 +++++++++++++++++
> include/linux/backlight.h | 10 ++++++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index 297db2f..0d1ed4f 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -370,6 +370,23 @@ void backlight_device_unregister(struct backlight_device *bd)
> }
> EXPORT_SYMBOL(backlight_device_unregister);
>
> +#if IS_ENABLED(CONFIG_OF)
Using IS_ENABLED() was odd. We'll never support CONFIG_OF=m, so can't
we use plain old "#ifdef CONFIG_OF" here?
--- a/drivers/video/backlight/backlight.c~backlight-add-of_find_backlight_by_node-function-fix
+++ a/drivers/video/backlight/backlight.c
@@ -370,7 +370,7 @@ void backlight_device_unregister(struct
}
EXPORT_SYMBOL(backlight_device_unregister);
-#if IS_ENABLED(CONFIG_OF)
+#ifdef CONFIG_OF
static int of_parent_match(struct device *dev, void *data)
{
return dev->parent && dev->parent->of_node = data;
--- a/include/linux/backlight.h~backlight-add-of_find_backlight_by_node-function-fix
+++ a/include/linux/backlight.h
@@ -134,7 +134,7 @@ struct generic_bl_info {
void (*kick_battery)(void);
};
-#if IS_ENABLED(CONFIG_OF)
+#ifdef CONFIG_OF
struct backlight_device *of_find_backlight_by_node(struct device_node *node);
#else
static inline struct backlight_device *
_
> +static int of_parent_match(struct device *dev, void *data)
> +{
> + return dev->parent && dev->parent->of_node = data;
> +}
> +
> +struct backlight_device *of_find_backlight_by_node(struct device_node *node)
> +{
> + struct device *dev;
> +
> + dev = class_find_device(backlight_class, NULL, node, of_parent_match);
> +
> + return dev ? to_backlight_device(dev) : NULL;
> +}
> +EXPORT_SYMBOL(of_find_backlight_by_node);
It's a global, exported-to-modules function. We should document such
major interfaces. Unless they are dead trivial, but I don't think this
one is that simple. The semantics of the return value could be
explained, and callers should be told that of_find_backlight_by_node()
took a ref on the returned device, and that they need to run
put_device(retval->dev), if retval was not NULL.
And anything else which might be useful.
^ permalink raw reply
* Re: [PATCH] backlight: Add of_find_backlight_by_node() function
From: Thierry Reding @ 2012-11-16 20:56 UTC (permalink / raw)
To: Andrew Morton
Cc: Jingoo Han, 'Richard Purdie',
'Florian Tobias Schandinat', linux-fbdev,
devicetree-discuss, linux-kernel
In-Reply-To: <001201cdc30f$65a17be0$30e473a0$%han@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 1802 bytes --]
On Thu, Nov 15, 2012 at 05:58:35PM +0900, Jingoo Han wrote:
> On Thursday, November 15, 2012 3:52 PM Thierry Reding wrote
> > On Thu, Nov 15, 2012 at 10:30:11AM +0900, Jingoo Han wrote:
> > > On Friday, November 09, 2012 11:05 PM Thierry Reding wrote
> > > >
> > > > This function finds the struct backlight_device for a given device tree
> > > > node. A dummy function is provided so that it safely compiles out if OF
> > > > support is disabled.
> > > >
> > > > Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> > >
> > > CC'ed Andrew Morton
> >
> > Yes, the backlight subsystem isn't very well maintained, so I should
> > have added Andrew in the first place. Thanks.
> >
> > >
> > > Hi Thierry Reding,
> > >
> > > The patch itself looks good.
> > > Could you explain when this API is used?
> > > Thank you.
> >
> > I use this for the upcoming Tegra DRM driver in order to hook up the
> > backlight with the DRM driver via DT to allow switching off the
> > backlight when the corresponding DRM output is switched of using DPMS.
> > Basically what you have is something like this in the device tree:
> >
> > display {
> > ...
> >
> > backlight = <&backlight>;
> >
> > ...
> > }
> >
> > Then you call something along these lines:
> >
> > np = of_parse_phandle(display, "backlight", 0);
> > if (np) {
> > backlight = of_find_backlight_by_node(np);
> > of_node_put(np);
> > }
> >
> > And then use the standard backlight API on the returned pointer.
>
> OK, I see how this API can be called.
> AS you mentioned, it will allow Tegra DRM driver to use
> the backlight driver.
>
> Acked-by: Jingoo Han <jg1.han@samsung.com>
Andrew,
any chance we could still get this in for the 3.8 merge window?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 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