* [PATCH 1/2] backlight/lp855x: Refactor dt parsing code
From: Sean Paul @ 2014-11-26 19:11 UTC (permalink / raw)
To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, milo.kim-l0cyMroinI0
Cc: seanpaul-F7+t8E8rja9g9hUCZPvPmw, Stéphane Marchesin
This patch refactors the dt parsing code to avoid setting platform_data,
instead just setting lp->pdata directly. This facilitates easier
probe deferral since the current scheme would require us to clear out
dev->platform_data before deferring.
Cc: Stéphane Marchesin <marcheu@chromium.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/video/backlight/lp855x_bl.c | 37 ++++++++++++++++++-------------------
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
index 25fb8e3..257b3ba 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -341,8 +341,10 @@ static const struct attribute_group lp855x_attr_group = {
};
#ifdef CONFIG_OF
-static int lp855x_parse_dt(struct device *dev, struct device_node *node)
+static int lp855x_parse_dt(struct lp855x *lp)
{
+ struct device *dev = lp->dev;
+ struct device_node *node = dev->of_node;
struct lp855x_platform_data *pdata;
int rom_length;
@@ -381,12 +383,12 @@ static int lp855x_parse_dt(struct device *dev, struct device_node *node)
pdata->rom_data = &rom[0];
}
- dev->platform_data = pdata;
+ lp->pdata = pdata;
return 0;
}
#else
-static int lp855x_parse_dt(struct device *dev, struct device_node *node)
+static int lp855x_parse_dt(struct lp855x *lp)
{
return -EINVAL;
}
@@ -395,18 +397,8 @@ static int lp855x_parse_dt(struct device *dev, struct device_node *node)
static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
{
struct lp855x *lp;
- struct lp855x_platform_data *pdata = dev_get_platdata(&cl->dev);
- struct device_node *node = cl->dev.of_node;
int ret;
- if (!pdata) {
- ret = lp855x_parse_dt(&cl->dev, node);
- if (ret < 0)
- return ret;
-
- pdata = dev_get_platdata(&cl->dev);
- }
-
if (!i2c_check_functionality(cl->adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
return -EIO;
@@ -414,16 +406,23 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
if (!lp)
return -ENOMEM;
- if (pdata->period_ns > 0)
- lp->mode = PWM_BASED;
- else
- lp->mode = REGISTER_BASED;
-
lp->client = cl;
lp->dev = &cl->dev;
- lp->pdata = pdata;
lp->chipname = id->name;
lp->chip_id = id->driver_data;
+ lp->pdata = dev_get_platdata(&cl->dev);
+
+ if (!lp->pdata) {
+ ret = lp855x_parse_dt(lp);
+ if (ret < 0)
+ return ret;
+ }
+
+ if (lp->pdata->period_ns > 0)
+ lp->mode = PWM_BASED;
+ else
+ lp->mode = REGISTER_BASED;
+
i2c_set_clientdata(cl, lp);
ret = lp855x_configure(lp);
--
2.1.1
^ permalink raw reply related
* [PATCH 2/2] backlight/lp855x: Add supply regulator to lp855x
From: Sean Paul @ 2014-11-26 19:11 UTC (permalink / raw)
To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, milo.kim-l0cyMroinI0
Cc: seanpaul-F7+t8E8rja9g9hUCZPvPmw, Stéphane Marchesin,
Aaron Durbin
In-Reply-To: <1417029064-12460-1-git-send-email-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
This patch adds a supply regulator to the lp855x platform data to facilitate
powering on/off the 3V rail attached to the controller.
Cc: Stéphane Marchesin <marcheu@chromium.org>
Cc: Aaron Durbin <adurbin@chromium.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
.../devicetree/bindings/video/backlight/lp855x.txt | 2 ++
drivers/video/backlight/lp855x_bl.c | 17 +++++++++++++++++
include/linux/platform_data/lp855x.h | 2 ++
3 files changed, 21 insertions(+)
diff --git a/Documentation/devicetree/bindings/video/backlight/lp855x.txt b/Documentation/devicetree/bindings/video/backlight/lp855x.txt
index 96e83a5..0a3ecbc 100644
--- a/Documentation/devicetree/bindings/video/backlight/lp855x.txt
+++ b/Documentation/devicetree/bindings/video/backlight/lp855x.txt
@@ -12,6 +12,7 @@ Optional properties:
- pwm-period: PWM period value. Set only PWM input mode used (u32)
- rom-addr: Register address of ROM area to be updated (u8)
- rom-val: Register value to be updated (u8)
+ - power-supply: Regulator which controls the 3V rail
Example:
@@ -56,6 +57,7 @@ Example:
backlight@2c {
compatible = "ti,lp8557";
reg = <0x2c>;
+ power-supply = <&backlight_vdd>;
dev-ctrl = /bits/ 8 <0x41>;
init-brt = /bits/ 8 <0x0a>;
diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
index 257b3ba..8021009 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -17,6 +17,7 @@
#include <linux/of.h>
#include <linux/platform_data/lp855x.h>
#include <linux/pwm.h>
+#include <linux/regulator/consumer.h>
/* LP8550/1/2/3/6 Registers */
#define LP855X_BRIGHTNESS_CTRL 0x00
@@ -383,6 +384,13 @@ static int lp855x_parse_dt(struct lp855x *lp)
pdata->rom_data = &rom[0];
}
+ pdata->supply = devm_regulator_get(dev, "power");
+ if (IS_ERR(pdata->supply)) {
+ if (PTR_ERR(pdata->supply) = -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ pdata->supply = NULL;
+ }
+
lp->pdata = pdata;
return 0;
@@ -423,6 +431,14 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
else
lp->mode = REGISTER_BASED;
+ if (lp->pdata->supply) {
+ ret = regulator_enable(lp->pdata->supply);
+ if (ret < 0) {
+ dev_err(&cl->dev, "failed to enable supply: %d\n", ret);
+ return ret;
+ }
+ }
+
i2c_set_clientdata(cl, lp);
ret = lp855x_configure(lp);
@@ -454,6 +470,7 @@ static int lp855x_remove(struct i2c_client *cl)
lp->bl->props.brightness = 0;
backlight_update_status(lp->bl);
+ regulator_disable(lp->pdata->supply);
sysfs_remove_group(&lp->dev->kobj, &lp855x_attr_group);
return 0;
diff --git a/include/linux/platform_data/lp855x.h b/include/linux/platform_data/lp855x.h
index 1b2ba24..9c7fd1e 100644
--- a/include/linux/platform_data/lp855x.h
+++ b/include/linux/platform_data/lp855x.h
@@ -136,6 +136,7 @@ struct lp855x_rom_data {
Only valid when mode is PWM_BASED.
* @size_program : total size of lp855x_rom_data
* @rom_data : list of new eeprom/eprom registers
+ * @supply : regulator that supplies 3V input
*/
struct lp855x_platform_data {
const char *name;
@@ -144,6 +145,7 @@ struct lp855x_platform_data {
unsigned int period_ns;
int size_program;
struct lp855x_rom_data *rom_data;
+ struct regulator *supply;
};
#endif
--
2.1.1
^ permalink raw reply related
* [PATCH 1/3] video: fbdev: vt8623fb: suppress build warning
From: Lad, Prabhakar @ 2014-11-26 22:07 UTC (permalink / raw)
To: Tomi Valkeinen, Jean-Christophe Plagniol-Villard
Cc: linux-fbdev, linux-kernel, Lad, Prabhakar
this patch fixes following build warning:
drivers/video/fbdev/vt8623fb.c: In function ‘vt8623_pci_probe’:
drivers/video/fbdev/vt8623fb.c:734:23: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
par->state.vgabase = (void __iomem *) vga_res.start;
^
Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
---
drivers/video/fbdev/vt8623fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
index 5c7cbc6..ea7f056 100644
--- a/drivers/video/fbdev/vt8623fb.c
+++ b/drivers/video/fbdev/vt8623fb.c
@@ -731,7 +731,7 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
pcibios_bus_to_resource(dev->bus, &vga_res, &bus_reg);
- par->state.vgabase = (void __iomem *) vga_res.start;
+ par->state.vgabase = (void __iomem *) (unsigned long) vga_res.start;
/* Find how many physical memory there is on card */
memsize1 = (vga_rseq(par->state.vgabase, 0x34) + 1) >> 1;
--
1.9.1
^ permalink raw reply related
* [PATCH 2/3] video: fbdev: s3fb: suppress build warning
From: Lad, Prabhakar @ 2014-11-26 22:07 UTC (permalink / raw)
To: Tomi Valkeinen, Jean-Christophe Plagniol-Villard
Cc: linux-fbdev, linux-kernel, Lad, Prabhakar
In-Reply-To: <1417039645-5313-1-git-send-email-prabhakar.csengg@gmail.com>
this patch fixes following build warning:
drivers/video/fbdev/s3fb.c: In function ‘s3_pci_probe’:
drivers/video/fbdev/s3fb.c:1185:23: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
par->state.vgabase = (void __iomem *) vga_res.start;
^
Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
---
drivers/video/fbdev/s3fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/s3fb.c b/drivers/video/fbdev/s3fb.c
index c43b969..f0ae61a 100644
--- a/drivers/video/fbdev/s3fb.c
+++ b/drivers/video/fbdev/s3fb.c
@@ -1182,7 +1182,7 @@ static int s3_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
pcibios_bus_to_resource(dev->bus, &vga_res, &bus_reg);
- par->state.vgabase = (void __iomem *) vga_res.start;
+ par->state.vgabase = (void __iomem *) (unsigned long) vga_res.start;
/* Unlock regs */
cr38 = vga_rcrt(par->state.vgabase, 0x38);
--
1.9.1
^ permalink raw reply related
* [PATCH 3/3] video: fbdev: arkfb: suppress build warning
From: Lad, Prabhakar @ 2014-11-26 22:07 UTC (permalink / raw)
To: Tomi Valkeinen, Jean-Christophe Plagniol-Villard
Cc: linux-fbdev, linux-kernel, Lad, Prabhakar
In-Reply-To: <1417039645-5313-1-git-send-email-prabhakar.csengg@gmail.com>
this patch fixes following build warning:
drivers/video/fbdev/arkfb.c: In function ‘ark_pci_probe’:
drivers/video/fbdev/arkfb.c:1019:23: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
par->state.vgabase = (void __iomem *) vga_res.start;
^
Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
---
drivers/video/fbdev/arkfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
index adc4ea2..b305a1e 100644
--- a/drivers/video/fbdev/arkfb.c
+++ b/drivers/video/fbdev/arkfb.c
@@ -1016,7 +1016,7 @@ static int ark_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
pcibios_bus_to_resource(dev->bus, &vga_res, &bus_reg);
- par->state.vgabase = (void __iomem *) vga_res.start;
+ par->state.vgabase = (void __iomem *) (unsigned long) vga_res.start;
/* FIXME get memsize */
regval = vga_rseq(par->state.vgabase, 0x10);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 1/2] backlight/lp855x: Refactor dt parsing code
From: Kim, Milo @ 2014-11-27 1:32 UTC (permalink / raw)
To: Sean Paul, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Stéphane Marchesin, jg1.han-Sze3O3UU22JBDgjK7y7TUQ,
cooloney-Re5JQEeQqe8AvxtiuMwx3w, lee.jones-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1417029064-12460-1-git-send-email-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
(Looping backlight class subsystem maintainers)
Hi Sean,
Thanks for checking this. I'd like to describe why the original code is
preferred.
The original code is copying the values of the lp855x platform data from
the DT in advance of allocating lp855x data.
It guarantees returning quickly in case of invalid platform data.
In other words, no memory allocation of lp855x proceeds if parsing the
DT gets failed.
However, this patch allocates the lp855x first and checking the DT.
Of course, devm_kzalloc() will free allocated memory later but original
code does NOT try to allocate it.
So I'd prefer to keep this way.
Best regards,
Milo
On 11/27/2014 4:11 AM, Sean Paul wrote:
> This patch refactors the dt parsing code to avoid setting platform_data,
> instead just setting lp->pdata directly. This facilitates easier
> probe deferral since the current scheme would require us to clear out
> dev->platform_data before deferring.
>
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
> drivers/video/backlight/lp855x_bl.c | 37 ++++++++++++++++++-------------------
> 1 file changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> index 25fb8e3..257b3ba 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -341,8 +341,10 @@ static const struct attribute_group lp855x_attr_group = {
> };
>
> #ifdef CONFIG_OF
> -static int lp855x_parse_dt(struct device *dev, struct device_node *node)
> +static int lp855x_parse_dt(struct lp855x *lp)
> {
> + struct device *dev = lp->dev;
> + struct device_node *node = dev->of_node;
> struct lp855x_platform_data *pdata;
> int rom_length;
>
> @@ -381,12 +383,12 @@ static int lp855x_parse_dt(struct device *dev, struct device_node *node)
> pdata->rom_data = &rom[0];
> }
>
> - dev->platform_data = pdata;
> + lp->pdata = pdata;
>
> return 0;
> }
> #else
> -static int lp855x_parse_dt(struct device *dev, struct device_node *node)
> +static int lp855x_parse_dt(struct lp855x *lp)
> {
> return -EINVAL;
> }
> @@ -395,18 +397,8 @@ static int lp855x_parse_dt(struct device *dev, struct device_node *node)
> static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
> {
> struct lp855x *lp;
> - struct lp855x_platform_data *pdata = dev_get_platdata(&cl->dev);
> - struct device_node *node = cl->dev.of_node;
> int ret;
>
> - if (!pdata) {
> - ret = lp855x_parse_dt(&cl->dev, node);
> - if (ret < 0)
> - return ret;
> -
> - pdata = dev_get_platdata(&cl->dev);
> - }
> -
> if (!i2c_check_functionality(cl->adapter, I2C_FUNC_SMBUS_I2C_BLOCK))
> return -EIO;
>
> @@ -414,16 +406,23 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
> if (!lp)
> return -ENOMEM;
>
> - if (pdata->period_ns > 0)
> - lp->mode = PWM_BASED;
> - else
> - lp->mode = REGISTER_BASED;
> -
> lp->client = cl;
> lp->dev = &cl->dev;
> - lp->pdata = pdata;
> lp->chipname = id->name;
> lp->chip_id = id->driver_data;
> + lp->pdata = dev_get_platdata(&cl->dev);
> +
> + if (!lp->pdata) {
> + ret = lp855x_parse_dt(lp);
> + if (ret < 0)
> + return ret;
> + }
> +
> + if (lp->pdata->period_ns > 0)
> + lp->mode = PWM_BASED;
> + else
> + lp->mode = REGISTER_BASED;
> +
> i2c_set_clientdata(cl, lp);
>
> ret = lp855x_configure(lp);
>
^ permalink raw reply
* Re: [PATCH 2/2] backlight/lp855x: Add supply regulator to lp855x
From: Kim, Milo @ 2014-11-27 1:32 UTC (permalink / raw)
To: Sean Paul, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Stéphane Marchesin, Aaron Durbin,
jg1.han-Sze3O3UU22JBDgjK7y7TUQ, cooloney-Re5JQEeQqe8AvxtiuMwx3w,
lee.jones-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1417029064-12460-2-git-send-email-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
(Looping backlight class subsystem maintainers)
Hi Sean,
Please see my comments below. Thanks!
On 11/27/2014 4:11 AM, Sean Paul wrote:
> This patch adds a supply regulator to the lp855x platform data to facilitate
> powering on/off the 3V rail attached to the controller.
>
> Cc: Stéphane Marchesin <marcheu@chromium.org>
> Cc: Aaron Durbin <adurbin@chromium.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
> .../devicetree/bindings/video/backlight/lp855x.txt | 2 ++
> drivers/video/backlight/lp855x_bl.c | 17 +++++++++++++++++
> include/linux/platform_data/lp855x.h | 2 ++
> 3 files changed, 21 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/video/backlight/lp855x.txt b/Documentation/devicetree/bindings/video/backlight/lp855x.txt
> index 96e83a5..0a3ecbc 100644
> --- a/Documentation/devicetree/bindings/video/backlight/lp855x.txt
> +++ b/Documentation/devicetree/bindings/video/backlight/lp855x.txt
> @@ -12,6 +12,7 @@ Optional properties:
> - pwm-period: PWM period value. Set only PWM input mode used (u32)
> - rom-addr: Register address of ROM area to be updated (u8)
> - rom-val: Register value to be updated (u8)
> + - power-supply: Regulator which controls the 3V rail
>
> Example:
>
> @@ -56,6 +57,7 @@ Example:
> backlight@2c {
> compatible = "ti,lp8557";
> reg = <0x2c>;
> + power-supply = <&backlight_vdd>;
>
> dev-ctrl = /bits/ 8 <0x41>;
> init-brt = /bits/ 8 <0x0a>;
> diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
> index 257b3ba..8021009 100644
> --- a/drivers/video/backlight/lp855x_bl.c
> +++ b/drivers/video/backlight/lp855x_bl.c
> @@ -17,6 +17,7 @@
> #include <linux/of.h>
> #include <linux/platform_data/lp855x.h>
> #include <linux/pwm.h>
> +#include <linux/regulator/consumer.h>
>
> /* LP8550/1/2/3/6 Registers */
> #define LP855X_BRIGHTNESS_CTRL 0x00
> @@ -383,6 +384,13 @@ static int lp855x_parse_dt(struct lp855x *lp)
> pdata->rom_data = &rom[0];
> }
>
> + pdata->supply = devm_regulator_get(dev, "power");
> + if (IS_ERR(pdata->supply)) {
> + if (PTR_ERR(pdata->supply) = -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + pdata->supply = NULL;
This optional data, 'supply' is set to NULL.
Please refer to my comments below.
> + }
> +
> lp->pdata = pdata;
>
> return 0;
> @@ -423,6 +431,14 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
> else
> lp->mode = REGISTER_BASED;
>
> + if (lp->pdata->supply) {
> + ret = regulator_enable(lp->pdata->supply);
> + if (ret < 0) {
> + dev_err(&cl->dev, "failed to enable supply: %d\n", ret);
> + return ret;
> + }
> + }
> +
> i2c_set_clientdata(cl, lp);
>
> ret = lp855x_configure(lp);
> @@ -454,6 +470,7 @@ static int lp855x_remove(struct i2c_client *cl)
>
> lp->bl->props.brightness = 0;
> backlight_update_status(lp->bl);
> + regulator_disable(lp->pdata->supply);
The 'lp->pdata->supply' is optional, what happens if it is NULL?
This will cause the kernel OOPS inside regulator_disable() API.
int regulator_disable(struct regulator *regulator)
{
struct regulator_dev *rdev = regulator->rdev;
...
}
> sysfs_remove_group(&lp->dev->kobj, &lp855x_attr_group);
>
> return 0;
> diff --git a/include/linux/platform_data/lp855x.h b/include/linux/platform_data/lp855x.h
> index 1b2ba24..9c7fd1e 100644
> --- a/include/linux/platform_data/lp855x.h
> +++ b/include/linux/platform_data/lp855x.h
> @@ -136,6 +136,7 @@ struct lp855x_rom_data {
> Only valid when mode is PWM_BASED.
> * @size_program : total size of lp855x_rom_data
> * @rom_data : list of new eeprom/eprom registers
> + * @supply : regulator that supplies 3V input
> */
> struct lp855x_platform_data {
> const char *name;
> @@ -144,6 +145,7 @@ struct lp855x_platform_data {
> unsigned int period_ns;
> int size_program;
> struct lp855x_rom_data *rom_data;
> + struct regulator *supply;
> };
>
> #endif
>
Best regards,
Milo
^ permalink raw reply
* [PATCH] Converting int usage to bool
From: Quentin Lambert @ 2014-11-27 10:41 UTC (permalink / raw)
To: Thomas Winischhofer
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel
The following semantic patch was used to find
functions and variables declared as int but
used as boolean. The patch resulted in a
significant number of false positive that I
have ignored.
I am not hyper confident about the modification
I made to the functions. Some of them seem to
have quite significant side effects, which is not
necesserarily intended from a boolean function.
In particular both *_rwtest functions write data.
Moreover sisfb_find_host_bridge name may suggest
more than a boolean return.
/* match all explicit boolean functions */
@boolean_function@
identifier fbool;
typedef bool;
@@
bool fbool(...) {
...
}
/* match variables eligible for boolean conversion */
@eligible_var exists@
identifier f, boolean_function.fbool;
local idexpression int x;
identifier xname;
expression e;
position p;
@@
f@p(...) {
...when any
(
x@xname = 1;
|
x@xname = 0;
|
x@xname = (e) ? 0 : 1;
|
x@xname = (e) ? 1 : 0;
|
x@xname = fbool(...);
)
...when any
}
/* match all acceptable complex assignement */
@valid_assign exists@
identifier eligible_var.f, boolean_function.fbool;
local idexpression int eligible_var.x;
expression e;
position p;
@@
f(...) {
...when any
(
x@p = (e) ? 0 : 1;
|
x@p = (e) ? 1 : 0;
|
x@p = fbool(...);
)
...when any
}
/* match any expression where x is used as an int */
@badvar1 exists@
identifier eligible_var.f;
local idexpression int eligible_var.x;
expression e1 != {0, 1}, e2;
position p != {valid_assign.p};
@@
f(...) {
...when any
(
x@p = e1;
|
x++
|
++x
|
x--
|
--x
|
x + e2
|
x - e2
|
e2 - x
|
x & e2
|
x * e2
|
x / e2
|
e2 / x
)
...when any
}
/* match all return statement involving an eligible variable */
@valid_var_return depends on !badvar1 exists@
identifier eligible_var.f;
local idexpression int eligible_var.x;
position p;
@@
f(...) {
...when any
return x@p;
}
/* match all function eligible for boolean conversion */
/* do not match function returning only boolean variable different from the one considered */
@eligible_func exists@
identifier f;
local idexpression int x;
position valid_var_return.p;
@@
int f(...) {
...when any
(
return 1;
|
return 0;
|
return x@p;
)
}
/* match functions returning something else than a bool */
@badfunc1 exists@
identifier eligible_func.f;
expression e != {0, 1};
position p != valid_var_return.p;
@@
int
f(...) {
...when any
return e@p;
}
/* satisfied when eligible_var is variable of eligible_func */
@same_function depends on eligible_var exists@
identifier eligible_func.f;
position eligible_var.p;
@@
int
f@p(...) {
...
}
/* match variable being returned as well as other non boolean variable */
@badvar2 depends on badfunc1 && same_function exists@
local idexpression int eligible_var.x;
identifier eligible_func.f;
@@
int
f(...) {
...when any
return x;
}
@depends on (!eligible_func || eligible_func && !same_function) && !badvar1@
identifier eligible_var.f;
local idexpression int eligible_var.x;
identifier eligible_var.xname;
expression e;
@@
f(...) {
...
(
++ bool xname;
- int xname;
|
++ bool xname = false;
- int xname = 0;
|
++ bool xname = true;
- int xname = 1;
)
<...
(
x - 1;
+ true;
|
x - 0;
+ false;
|
- x = (e) ? 1 : 0;
+ x = (e) ? true : false;
|
- x = (e) ? 0 : 1;
+ x = (e) ? false : true;
)
...>
}
@depends on eligible_func && same_function && !badvar1 && !badvar2@
identifier eligible_func.f;
local idexpression int eligible_var.x;
identifier eligible_var.xname;
expression e;
@@
f(...) {
...
(
++ bool xname;
- int xname;
|
++ bool xname = false;
- int xname = 0;
|
++ bool xname = true;
- int xname = 1;
)
<...
(
x - 1;
+ true;
|
x - 0;
+ false;
|
- x = (e) ? 1 : 0;
+ x = (e) ? true : false;
|
- x = (e) ? 0 : 1;
+ x = (e) ? false : true;
)
...>
}
@depends on !badfunc1@
identifier eligible_func.f;
@@
- int
+ bool
f(...) {
<...
(
- return 1;
+ return true;
|
- return 0;
+ return false;
)
...>
}
Quentin Lambert (1):
video: fbdev: sis: sis_main.c: converting relevant int to bool
drivers/video/fbdev/sis/sis_main.c | 64 ++++++++++++++++++++------------------
1 file changed, 33 insertions(+), 31 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH] video: fbdev: sis: sis_main.c: converting relevant int to bool
From: Quentin Lambert @ 2014-11-27 10:42 UTC (permalink / raw)
To: Thomas Winischhofer
Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel
Convert int variables and functions to boolean when
relevant. The eligible cases were found using Coccinelle.
A simplified version of the semantic patch used to find
candidate is presented below :
@r exists@
identifier f;
local idexpression int x;
identifier xname;
@@
f(...) {
...when any
(
x@xname = 1;
|
x@xname = 0;
)
...when any
}
@bad exists@
identifier r.f;
local idexpression int r.x
expression e1 != {0, 1}, e2;
@@
f(...) {
...when any
(
x = e1;
|
x + e2
)
...when any
}
@depends on !bad@
identifier r.f;
local idexpression int r.x;
identifier r.xname;
@@
f(...) {
...
++ bool xname;
- int xname;
<...
(
x - 1;
+ true;
|
x - -1;
+ false;
)
...>
}
Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
---
drivers/video/fbdev/sis/sis_main.c | 64 ++++++++++++++++++++------------------
1 file changed, 33 insertions(+), 31 deletions(-)
diff --git a/drivers/video/fbdev/sis/sis_main.c b/drivers/video/fbdev/sis/sis_main.c
index e5d11b1..5884846 100644
--- a/drivers/video/fbdev/sis/sis_main.c
+++ b/drivers/video/fbdev/sis/sis_main.c
@@ -108,7 +108,8 @@ sisfb_setdefaultparms(void)
static void sisfb_search_vesamode(unsigned int vesamode, bool quiet)
{
- int i = 0, j = 0;
+ bool j = false;
+ int i = 0;
/* We don't know the hardware specs yet and there is no ivideo */
@@ -137,7 +138,7 @@ static void sisfb_search_vesamode(unsigned int vesamode, bool quiet)
continue;
}
sisfb_mode_idx = i - 1;
- j = 1;
+ j = true;
break;
}
}
@@ -1185,7 +1186,8 @@ sisfb_do_set_var(struct fb_var_screeninfo *var, int isactive, struct fb_info *in
struct sis_video_info *ivideo = (struct sis_video_info *)info->par;
unsigned int htotal = 0, vtotal = 0;
unsigned int drate = 0, hrate = 0;
- int found_mode = 0, ret;
+ bool found_mode = false;
+ int ret;
int old_mode;
u32 pixclock;
@@ -1228,7 +1230,7 @@ sisfb_do_set_var(struct fb_var_screeninfo *var, int isactive, struct fb_info *in
(sisbios_mode[ivideo->sisfb_mode_idx].yres = var->yres) &&
(sisbios_mode[ivideo->sisfb_mode_idx].bpp = var->bits_per_pixel)) {
ivideo->mode_no = sisbios_mode[ivideo->sisfb_mode_idx].mode_no[ivideo->mni];
- found_mode = 1;
+ found_mode = true;
break;
}
ivideo->sisfb_mode_idx++;
@@ -1431,7 +1433,7 @@ sisfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
struct sis_video_info *ivideo = (struct sis_video_info *)info->par;
unsigned int htotal = 0, vtotal = 0, myrateindex = 0;
unsigned int drate = 0, hrate = 0, maxyres;
- int found_mode = 0;
+ bool found_mode = false;
int refresh_rate, search_idx, tidx;
bool recalc_clock = false;
u32 pixclock;
@@ -1466,7 +1468,7 @@ sisfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
(sisbios_mode[search_idx].bpp = var->bits_per_pixel)) {
if((tidx = sisfb_validate_mode(ivideo, search_idx,
ivideo->currentvbflags)) > 0) {
- found_mode = 1;
+ found_mode = true;
search_idx = tidx;
break;
}
@@ -1482,7 +1484,7 @@ sisfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
(var->bits_per_pixel = sisbios_mode[search_idx].bpp) ) {
if((tidx = sisfb_validate_mode(ivideo,search_idx,
ivideo->currentvbflags)) > 0) {
- found_mode = 1;
+ found_mode = true;
search_idx = tidx;
break;
}
@@ -3224,7 +3226,7 @@ sisfb_poh_allocate(struct SIS_HEAP *memheap, u32 size)
{
struct SIS_OH *pohThis;
struct SIS_OH *pohRoot;
- int bAllocated = 0;
+ bool bAllocated = false;
if(size > memheap->max_freesize) {
DPRINTK("sisfb: Can't allocate %dk video memory\n",
@@ -3236,7 +3238,7 @@ sisfb_poh_allocate(struct SIS_HEAP *memheap, u32 size)
while(pohThis != &memheap->oh_free) {
if(size <= pohThis->size) {
- bAllocated = 1;
+ bAllocated = true;
break;
}
pohThis = pohThis->poh_next;
@@ -3299,13 +3301,13 @@ sisfb_poh_free(struct SIS_HEAP *memheap, u32 base)
struct SIS_OH *poh_next;
u32 ulUpper;
u32 ulLower;
- int foundNode = 0;
+ bool foundNode = false;
poh_freed = memheap->oh_used.poh_next;
while(poh_freed != &memheap->oh_used) {
if(poh_freed->offset = base) {
- foundNode = 1;
+ foundNode = true;
break;
}
@@ -3892,7 +3894,7 @@ sisfb_reset_mode(struct sis_video_info *ivideo)
static void
sisfb_handle_command(struct sis_video_info *ivideo, struct sisfb_cmd *sisfb_command)
{
- int mycrt1off;
+ bool mycrt1off;
switch(sisfb_command->sisfb_cmd) {
case SISFB_CMD_GETVBFLAGS:
@@ -3919,7 +3921,7 @@ sisfb_handle_command(struct sis_video_info *ivideo, struct sisfb_cmd *sisfb_comm
sisfb_command->sisfb_result[0] = SISFB_CMD_ERR_NOCRT2;
} else {
sisfb_command->sisfb_result[0] = SISFB_CMD_ERR_OK;
- mycrt1off = sisfb_command->sisfb_arg[0] ? 0 : 1;
+ mycrt1off = sisfb_command->sisfb_arg[0] ? false : true;
if( ((ivideo->currentvbflags & VB_DISPTYPE_CRT1) && mycrt1off) ||
((!(ivideo->currentvbflags & VB_DISPTYPE_CRT1)) && !mycrt1off) ) {
ivideo->sisfb_crt1off = mycrt1off;
@@ -4037,32 +4039,32 @@ static int __init sisfb_setup(char *options)
}
#endif
-static int sisfb_check_rom(void __iomem *rom_base,
+static bool sisfb_check_rom(void __iomem *rom_base,
struct sis_video_info *ivideo)
{
void __iomem *rom;
int romptr;
if((readb(rom_base) != 0x55) || (readb(rom_base + 1) != 0xaa))
- return 0;
+ return false;
romptr = (readb(rom_base + 0x18) | (readb(rom_base + 0x19) << 8));
if(romptr > (0x10000 - 8))
- return 0;
+ return false;
rom = rom_base + romptr;
if((readb(rom) != 'P') || (readb(rom + 1) != 'C') ||
(readb(rom + 2) != 'I') || (readb(rom + 3) != 'R'))
- return 0;
+ return false;
if((readb(rom + 4) | (readb(rom + 5) << 8)) != ivideo->chip_vendor)
- return 0;
+ return false;
if((readb(rom + 6) | (readb(rom + 7) << 8)) != ivideo->chip_id)
- return 0;
+ return false;
- return 1;
+ return true;
}
static unsigned char *sisfb_find_rom(struct pci_dev *pdev)
@@ -4215,7 +4217,7 @@ static const unsigned short SiS_DRAMType[17][5] = {
{0x09,0x08,0x01,0x01,0x00}
};
-static int sisfb_post_300_rwtest(struct sis_video_info *ivideo, int iteration,
+static bool sisfb_post_300_rwtest(struct sis_video_info *ivideo, int iteration,
int buswidth, int PseudoRankCapacity,
int PseudoAdrPinCount, unsigned int mapsize)
{
@@ -4275,10 +4277,10 @@ static int sisfb_post_300_rwtest(struct sis_video_info *ivideo, int iteration,
/* Read data */
if(readw(FBAddr + BankNumHigh + PhysicalAdrHigh) = PhysicalAdrHigh)
- return 1;
+ return true;
}
- return 0;
+ return false;
}
static void sisfb_post_300_ramsize(struct pci_dev *pdev, unsigned int mapsize)
@@ -4539,18 +4541,18 @@ static void sisfb_post_xgi_delay(struct sis_video_info *ivideo, int delay)
}
}
-static int sisfb_find_host_bridge(struct sis_video_info *ivideo,
+static bool sisfb_find_host_bridge(struct sis_video_info *ivideo,
struct pci_dev *mypdev,
unsigned short pcivendor)
{
struct pci_dev *pdev = NULL;
unsigned short temp;
- int ret = 0;
+ bool ret = false;
while((pdev = pci_get_class(PCI_CLASS_BRIDGE_HOST, pdev))) {
temp = pdev->vendor;
if(temp = pcivendor) {
- ret = 1;
+ ret = true;
pci_dev_put(pdev);
break;
}
@@ -4559,7 +4561,7 @@ static int sisfb_find_host_bridge(struct sis_video_info *ivideo,
return ret;
}
-static int sisfb_post_xgi_rwtest(struct sis_video_info *ivideo, int starta,
+static bool sisfb_post_xgi_rwtest(struct sis_video_info *ivideo, int starta,
unsigned int enda, unsigned int mapsize)
{
unsigned int pos;
@@ -4576,18 +4578,18 @@ static int sisfb_post_xgi_rwtest(struct sis_video_info *ivideo, int starta,
sisfb_post_xgi_delay(ivideo, 150);
if(readl(ivideo->video_vbase) != 0)
- return 0;
+ return false;
for(i = starta; i <= enda; i++) {
pos = 1 << i;
if(pos < mapsize) {
if(readl(ivideo->video_vbase + pos) != pos)
- return 0;
+ return false;
} else
- return 0;
+ return false;
}
- return 1;
+ return true;
}
static int sisfb_post_xgi_ramsize(struct sis_video_info *ivideo)
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 1/2] backlight/lp855x: Refactor dt parsing code
From: Sean Paul @ 2014-11-27 12:22 UTC (permalink / raw)
To: Kim, Milo
Cc: Sean Paul, linux-fbdev,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Stéphane Marchesin, Jingoo Han,
cooloney-Re5JQEeQqe8AvxtiuMwx3w, lee.jones-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <54767F37.3020006-l0cyMroinI0@public.gmane.org>
On Wed, Nov 26, 2014 at 5:32 PM, Kim, Milo <milo.kim@ti.com> wrote:
> (Looping backlight class subsystem maintainers)
>
> Hi Sean,
>
> Thanks for checking this. I'd like to describe why the original code is
> preferred.
>
> The original code is copying the values of the lp855x platform data from the
> DT in advance of allocating lp855x data.
> It guarantees returning quickly in case of invalid platform data.
> In other words, no memory allocation of lp855x proceeds if parsing the DT
> gets failed.
> However, this patch allocates the lp855x first and checking the DT.
> Of course, devm_kzalloc() will free allocated memory later but original code
> does NOT try to allocate it.
> So I'd prefer to keep this way.
>
Hi Milo,
Thanks for the quick response. The reason I'm changing the existing
code is that it causes problems in the probe deferral case.
A concrete example: In my follow-on patch, I call devm_regulator_get
in the parse_dt function. Assume that something later on in probe()
returns -EPROBEDEFER. If we didn't have this patch, dev->platform_data
would continue to be set the next time probe() was called. This would
causes us to skip the parse_dt() function (because pdata is non-null
from last time) and we would have an invalid pointer to the regulator
we got the last time.
Sean
> Best regards,
> Milo
>
>
> On 11/27/2014 4:11 AM, Sean Paul wrote:
>>
>> This patch refactors the dt parsing code to avoid setting platform_data,
>> instead just setting lp->pdata directly. This facilitates easier
>> probe deferral since the current scheme would require us to clear out
>> dev->platform_data before deferring.
>>
>> Cc: Stéphane Marchesin <marcheu@chromium.org>
>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>> ---
>> drivers/video/backlight/lp855x_bl.c | 37
>> ++++++++++++++++++-------------------
>> 1 file changed, 18 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/video/backlight/lp855x_bl.c
>> b/drivers/video/backlight/lp855x_bl.c
>> index 25fb8e3..257b3ba 100644
>> --- a/drivers/video/backlight/lp855x_bl.c
>> +++ b/drivers/video/backlight/lp855x_bl.c
>> @@ -341,8 +341,10 @@ static const struct attribute_group lp855x_attr_group
>> = {
>> };
>>
>> #ifdef CONFIG_OF
>> -static int lp855x_parse_dt(struct device *dev, struct device_node *node)
>> +static int lp855x_parse_dt(struct lp855x *lp)
>> {
>> + struct device *dev = lp->dev;
>> + struct device_node *node = dev->of_node;
>> struct lp855x_platform_data *pdata;
>> int rom_length;
>>
>> @@ -381,12 +383,12 @@ static int lp855x_parse_dt(struct device *dev,
>> struct device_node *node)
>> pdata->rom_data = &rom[0];
>> }
>>
>> - dev->platform_data = pdata;
>> + lp->pdata = pdata;
>>
>> return 0;
>> }
>> #else
>> -static int lp855x_parse_dt(struct device *dev, struct device_node *node)
>> +static int lp855x_parse_dt(struct lp855x *lp)
>> {
>> return -EINVAL;
>> }
>> @@ -395,18 +397,8 @@ static int lp855x_parse_dt(struct device *dev, struct
>> device_node *node)
>> static int lp855x_probe(struct i2c_client *cl, const struct
>> i2c_device_id *id)
>> {
>> struct lp855x *lp;
>> - struct lp855x_platform_data *pdata = dev_get_platdata(&cl->dev);
>> - struct device_node *node = cl->dev.of_node;
>> int ret;
>>
>> - if (!pdata) {
>> - ret = lp855x_parse_dt(&cl->dev, node);
>> - if (ret < 0)
>> - return ret;
>> -
>> - pdata = dev_get_platdata(&cl->dev);
>> - }
>> -
>> if (!i2c_check_functionality(cl->adapter,
>> I2C_FUNC_SMBUS_I2C_BLOCK))
>> return -EIO;
>>
>> @@ -414,16 +406,23 @@ static int lp855x_probe(struct i2c_client *cl, const
>> struct i2c_device_id *id)
>> if (!lp)
>> return -ENOMEM;
>>
>> - if (pdata->period_ns > 0)
>> - lp->mode = PWM_BASED;
>> - else
>> - lp->mode = REGISTER_BASED;
>> -
>> lp->client = cl;
>> lp->dev = &cl->dev;
>> - lp->pdata = pdata;
>> lp->chipname = id->name;
>> lp->chip_id = id->driver_data;
>> + lp->pdata = dev_get_platdata(&cl->dev);
>> +
>> + if (!lp->pdata) {
>> + ret = lp855x_parse_dt(lp);
>> + if (ret < 0)
>> + return ret;
>> + }
>> +
>> + if (lp->pdata->period_ns > 0)
>> + lp->mode = PWM_BASED;
>> + else
>> + lp->mode = REGISTER_BASED;
>> +
>> i2c_set_clientdata(cl, lp);
>>
>> ret = lp855x_configure(lp);
>>
>
^ permalink raw reply
* Re: [PATCH 2/2] backlight/lp855x: Add supply regulator to lp855x
From: Sean Paul @ 2014-11-27 12:23 UTC (permalink / raw)
To: Kim, Milo
Cc: Sean Paul, linux-fbdev,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Stéphane Marchesin, Aaron Durbin, Jingoo Han,
cooloney-Re5JQEeQqe8AvxtiuMwx3w, lee.jones-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <54767F43.2060901-l0cyMroinI0@public.gmane.org>
On Wed, Nov 26, 2014 at 5:32 PM, Kim, Milo <milo.kim@ti.com> wrote:
> (Looping backlight class subsystem maintainers)
>
> Hi Sean,
>
> Please see my comments below. Thanks!
>
> On 11/27/2014 4:11 AM, Sean Paul wrote:
>>
>> This patch adds a supply regulator to the lp855x platform data to
>> facilitate
>> powering on/off the 3V rail attached to the controller.
>>
>> Cc: Stéphane Marchesin <marcheu@chromium.org>
>> Cc: Aaron Durbin <adurbin@chromium.org>
>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>> ---
>> .../devicetree/bindings/video/backlight/lp855x.txt | 2 ++
>> drivers/video/backlight/lp855x_bl.c | 17
>> +++++++++++++++++
>> include/linux/platform_data/lp855x.h | 2 ++
>> 3 files changed, 21 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/video/backlight/lp855x.txt
>> b/Documentation/devicetree/bindings/video/backlight/lp855x.txt
>> index 96e83a5..0a3ecbc 100644
>> --- a/Documentation/devicetree/bindings/video/backlight/lp855x.txt
>> +++ b/Documentation/devicetree/bindings/video/backlight/lp855x.txt
>> @@ -12,6 +12,7 @@ Optional properties:
>> - pwm-period: PWM period value. Set only PWM input mode used (u32)
>> - rom-addr: Register address of ROM area to be updated (u8)
>> - rom-val: Register value to be updated (u8)
>> + - power-supply: Regulator which controls the 3V rail
>>
>> Example:
>>
>> @@ -56,6 +57,7 @@ Example:
>> backlight@2c {
>> compatible = "ti,lp8557";
>> reg = <0x2c>;
>> + power-supply = <&backlight_vdd>;
>>
>> dev-ctrl = /bits/ 8 <0x41>;
>> init-brt = /bits/ 8 <0x0a>;
>> diff --git a/drivers/video/backlight/lp855x_bl.c
>> b/drivers/video/backlight/lp855x_bl.c
>> index 257b3ba..8021009 100644
>> --- a/drivers/video/backlight/lp855x_bl.c
>> +++ b/drivers/video/backlight/lp855x_bl.c
>> @@ -17,6 +17,7 @@
>> #include <linux/of.h>
>> #include <linux/platform_data/lp855x.h>
>> #include <linux/pwm.h>
>> +#include <linux/regulator/consumer.h>
>>
>> /* LP8550/1/2/3/6 Registers */
>> #define LP855X_BRIGHTNESS_CTRL 0x00
>> @@ -383,6 +384,13 @@ static int lp855x_parse_dt(struct lp855x *lp)
>> pdata->rom_data = &rom[0];
>> }
>>
>> + pdata->supply = devm_regulator_get(dev, "power");
>> + if (IS_ERR(pdata->supply)) {
>> + if (PTR_ERR(pdata->supply) = -EPROBE_DEFER)
>> + return -EPROBE_DEFER;
>> + pdata->supply = NULL;
>
>
> This optional data, 'supply' is set to NULL.
> Please refer to my comments below.
>
>> + }
>> +
>> lp->pdata = pdata;
>>
>> return 0;
>> @@ -423,6 +431,14 @@ static int lp855x_probe(struct i2c_client *cl, const
>> struct i2c_device_id *id)
>> else
>> lp->mode = REGISTER_BASED;
>>
>> + if (lp->pdata->supply) {
>> + ret = regulator_enable(lp->pdata->supply);
>> + if (ret < 0) {
>> + dev_err(&cl->dev, "failed to enable supply: %d\n",
>> ret);
>> + return ret;
>> + }
>> + }
>> +
>> i2c_set_clientdata(cl, lp);
>>
>> ret = lp855x_configure(lp);
>> @@ -454,6 +470,7 @@ static int lp855x_remove(struct i2c_client *cl)
>>
>> lp->bl->props.brightness = 0;
>> backlight_update_status(lp->bl);
>> + regulator_disable(lp->pdata->supply);
>
>
> The 'lp->pdata->supply' is optional, what happens if it is NULL?
> This will cause the kernel OOPS inside regulator_disable() API.
>
bleh. Thanks for catching this, I'll re-spin and send a new version next week.
Sean
> int regulator_disable(struct regulator *regulator)
> {
> struct regulator_dev *rdev = regulator->rdev;
> ...
> }
>
>> sysfs_remove_group(&lp->dev->kobj, &lp855x_attr_group);
>>
>> return 0;
>> diff --git a/include/linux/platform_data/lp855x.h
>> b/include/linux/platform_data/lp855x.h
>> index 1b2ba24..9c7fd1e 100644
>> --- a/include/linux/platform_data/lp855x.h
>> +++ b/include/linux/platform_data/lp855x.h
>> @@ -136,6 +136,7 @@ struct lp855x_rom_data {
>> Only valid when mode is PWM_BASED.
>> * @size_program : total size of lp855x_rom_data
>> * @rom_data : list of new eeprom/eprom registers
>> + * @supply : regulator that supplies 3V input
>> */
>> struct lp855x_platform_data {
>> const char *name;
>> @@ -144,6 +145,7 @@ struct lp855x_platform_data {
>> unsigned int period_ns;
>> int size_program;
>> struct lp855x_rom_data *rom_data;
>> + struct regulator *supply;
>> };
>>
>> #endif
>>
>
> Best regards,
> Milo
^ permalink raw reply
* Re: [PATCH v7.1 09/19] ASoC: omap-hdmi-audio: Add platform device for OMAP HDMI audio support
From: Mark Brown @ 2014-11-29 11:32 UTC (permalink / raw)
To: Jyri Sarha
Cc: alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi,
liam.r.girdwood, tomi.valkeinen
In-Reply-To: <9a528990bbe116d9bb073f690b835c847b3fa0bf.1415803065.git.jsarha@ti.com>
[-- Attachment #1: Type: text/plain, Size: 698 bytes --]
On Wed, Nov 12, 2014 at 04:41:00PM +0200, Jyri Sarha wrote:
> The platform device should only be registered from OMAPDSS HDMI
> driver. The platform driver registers and unregisters all ASoC
> components needed for OMAP HDMI audio.
So, this basically seems fine but I'm looking at this code and I'm not
really seeing a huge amount of OMAP specificness here. Why is this an
OMAP driver and not something more generic - can't we have other HDMI
controllers offering the same interface? There's a few serial numbers
that would need filing off but since the ASoC driver is basically just
interface matching and not talking to the hardware it looks like it
ought to be usable by more than just OMAP.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH v7.1 00/19] Rework OMAP4+ HDMI audio support
From: Mark Brown @ 2014-11-29 11:59 UTC (permalink / raw)
To: Jyri Sarha
Cc: alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi,
liam.r.girdwood, tomi.valkeinen
In-Reply-To: <cover.1415803064.git.jsarha@ti.com>
[-- Attachment #1: Type: text/plain, Size: 539 bytes --]
On Wed, Nov 12, 2014 at 04:40:51PM +0200, Jyri Sarha wrote:
> The patches are based on:
> git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git for-next
>
> The base, the patches, and couple of additional not-to-be-merged
> omap2plus_defconfig patches can be found here:
> https://github.com/jsarha/linux.git omap-hdmi-audio
I guess I'm OK with these so
Reviewed-by: Mark Brown <broonie@kernel.org>
but like I said in reply to the patch adding the new driver I think
we're going to want to generalize this a bit.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* [PATCH v3 0/3] omapdss: Add video output support for gta04
From: Marek Belisko @ 2014-11-30 13:07 UTC (permalink / raw)
To: linux-arm-kernel
changes from v2:
- rename amplifier-opa362.c to encoder-opa362
- drop inversion handlign from driver
- add ti,invert-polarity to venc node
I also drop devconf1 handling patches because discussion how to
properly implement is not yet finished and this part is independent
from video driver.
changes from v1:
- fix opa362 compilation error
- fix opa362 DT documentation
- move devconf1 definition to omap3.dtsi
Marek Belisko (3):
video: omapdss: Add opa362 driver
Documentation: DT: Add documentation for ti,opa362 bindings
arm: dts: omap3-gta04: Add handling for tv output
.../devicetree/bindings/video/ti,opa362.txt | 38 +++
arch/arm/boot/dts/omap3-gta04.dtsi | 49 ++++
drivers/video/fbdev/omap2/displays-new/Kconfig | 6 +
drivers/video/fbdev/omap2/displays-new/Makefile | 1 +
.../fbdev/omap2/displays-new/encoder-opa362.c | 283 +++++++++++++++++++++
5 files changed, 377 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/ti,opa362.txt
create mode 100644 drivers/video/fbdev/omap2/displays-new/encoder-opa362.c
--
1.9.1
^ permalink raw reply
* [PATCH v3 1/3] video: omapdss: Add opa362 driver
From: Marek Belisko @ 2014-11-30 13:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1417352867-7436-1-git-send-email-marek@goldelico.com>
opa362 is amplifier for video and can be connected to the tvout pads
of the OMAP3. It has one gpio control for enable/disable of the output
(high impedance).
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
Signed-off-by: Marek Belisko <marek@goldelico.com>
---
drivers/video/fbdev/omap2/displays-new/Kconfig | 6 +
drivers/video/fbdev/omap2/displays-new/Makefile | 1 +
.../fbdev/omap2/displays-new/encoder-opa362.c | 283 +++++++++++++++++++++
3 files changed, 290 insertions(+)
create mode 100644 drivers/video/fbdev/omap2/displays-new/encoder-opa362.c
diff --git a/drivers/video/fbdev/omap2/displays-new/Kconfig b/drivers/video/fbdev/omap2/displays-new/Kconfig
index e6cfc38..5747101 100644
--- a/drivers/video/fbdev/omap2/displays-new/Kconfig
+++ b/drivers/video/fbdev/omap2/displays-new/Kconfig
@@ -1,6 +1,12 @@
menu "OMAP Display Device Drivers (new device model)"
depends on OMAP2_DSS
+config DISPLAY_ENCODER_OPA362
+ tristate "OPA362 external analog amplifier"
+ help
+ Driver for OPA362 external analog TV amplifier controlled
+ through a GPIO.
+
config DISPLAY_ENCODER_TFP410
tristate "TFP410 DPI to DVI Encoder"
help
diff --git a/drivers/video/fbdev/omap2/displays-new/Makefile b/drivers/video/fbdev/omap2/displays-new/Makefile
index 0323a8a..9aa176b 100644
--- a/drivers/video/fbdev/omap2/displays-new/Makefile
+++ b/drivers/video/fbdev/omap2/displays-new/Makefile
@@ -1,3 +1,4 @@
+obj-$(CONFIG_DISPLAY_ENCODER_OPA362) += encoder-opa362.o
obj-$(CONFIG_DISPLAY_ENCODER_TFP410) += encoder-tfp410.o
obj-$(CONFIG_DISPLAY_ENCODER_TPD12S015) += encoder-tpd12s015.o
obj-$(CONFIG_DISPLAY_CONNECTOR_DVI) += connector-dvi.o
diff --git a/drivers/video/fbdev/omap2/displays-new/encoder-opa362.c b/drivers/video/fbdev/omap2/displays-new/encoder-opa362.c
new file mode 100644
index 0000000..d2cc649
--- /dev/null
+++ b/drivers/video/fbdev/omap2/displays-new/encoder-opa362.c
@@ -0,0 +1,283 @@
+/*
+ * OPA362 analog video amplifier with output/power control
+ *
+ * Copyright (C) 2014 Golden Delicious Computers
+ * Author: H. Nikolaus Schaller <hns@goldelico.com>
+ *
+ * based on encoder-tfp410
+ *
+ * Copyright (C) 2013 Texas Instruments
+ * Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/gpio.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of_gpio.h>
+
+#include <video/omapdss.h>
+#include <video/omap-panel-data.h>
+
+struct panel_drv_data {
+ struct omap_dss_device dssdev;
+ struct omap_dss_device *in;
+
+ struct gpio_desc *enable_gpio;
+
+ struct omap_video_timings timings;
+};
+
+#define to_panel_data(x) container_of(x, struct panel_drv_data, dssdev)
+
+static int opa362_connect(struct omap_dss_device *dssdev,
+ struct omap_dss_device *dst)
+{
+ struct panel_drv_data *ddata = to_panel_data(dssdev);
+ struct omap_dss_device *in = ddata->in;
+ int r;
+
+ dev_dbg(dssdev->dev, "connect\n");
+
+ if (omapdss_device_is_connected(dssdev))
+ return -EBUSY;
+
+ r = in->ops.atv->connect(in, dssdev);
+ if (r)
+ return r;
+
+ dst->src = dssdev;
+ dssdev->dst = dst;
+
+ return 0;
+}
+
+static void opa362_disconnect(struct omap_dss_device *dssdev,
+ struct omap_dss_device *dst)
+{
+ struct panel_drv_data *ddata = to_panel_data(dssdev);
+ struct omap_dss_device *in = ddata->in;
+
+ dev_dbg(dssdev->dev, "disconnect\n");
+
+ WARN_ON(!omapdss_device_is_connected(dssdev));
+ if (!omapdss_device_is_connected(dssdev))
+ return;
+
+ WARN_ON(dst != dssdev->dst);
+ if (dst != dssdev->dst)
+ return;
+
+ dst->src = NULL;
+ dssdev->dst = NULL;
+
+ in->ops.atv->disconnect(in, &ddata->dssdev);
+}
+
+static int opa362_enable(struct omap_dss_device *dssdev)
+{
+ struct panel_drv_data *ddata = to_panel_data(dssdev);
+ struct omap_dss_device *in = ddata->in;
+ int r;
+
+ dev_dbg(dssdev->dev, "enable\n");
+
+ if (!omapdss_device_is_connected(dssdev))
+ return -ENODEV;
+
+ if (omapdss_device_is_enabled(dssdev))
+ return 0;
+
+ in->ops.atv->set_timings(in, &ddata->timings);
+
+ r = in->ops.atv->enable(in);
+ if (r)
+ return r;
+
+ if (ddata->enable_gpio)
+ gpiod_set_value_cansleep(ddata->enable_gpio, 1);
+
+ dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
+
+ return 0;
+}
+
+static void opa362_disable(struct omap_dss_device *dssdev)
+{
+ struct panel_drv_data *ddata = to_panel_data(dssdev);
+ struct omap_dss_device *in = ddata->in;
+
+ dev_dbg(dssdev->dev, "disable\n");
+
+ if (!omapdss_device_is_enabled(dssdev))
+ return;
+
+ if (ddata->enable_gpio)
+ gpiod_set_value_cansleep(ddata->enable_gpio, 0);
+
+ in->ops.atv->disable(in);
+
+ dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
+}
+
+static void opa362_set_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings)
+{
+ struct panel_drv_data *ddata = to_panel_data(dssdev);
+ struct omap_dss_device *in = ddata->in;
+
+ dev_dbg(dssdev->dev, "set_timings\n");
+
+ ddata->timings = *timings;
+ dssdev->panel.timings = *timings;
+
+ in->ops.atv->set_timings(in, timings);
+}
+
+static void opa362_get_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings)
+{
+ struct panel_drv_data *ddata = to_panel_data(dssdev);
+
+ dev_dbg(dssdev->dev, "get_timings\n");
+
+ *timings = ddata->timings;
+}
+
+static int opa362_check_timings(struct omap_dss_device *dssdev,
+ struct omap_video_timings *timings)
+{
+ struct panel_drv_data *ddata = to_panel_data(dssdev);
+ struct omap_dss_device *in = ddata->in;
+
+ dev_dbg(dssdev->dev, "check_timings\n");
+
+ return in->ops.atv->check_timings(in, timings);
+}
+
+static void opa362_set_type(struct omap_dss_device *dssdev,
+ enum omap_dss_venc_type type)
+{
+ /* we can only drive a COMPOSITE output */
+ WARN_ON(type != OMAP_DSS_VENC_TYPE_COMPOSITE);
+
+}
+
+static const struct omapdss_atv_ops opa362_atv_ops = {
+ .connect = opa362_connect,
+ .disconnect = opa362_disconnect,
+
+ .enable = opa362_enable,
+ .disable = opa362_disable,
+
+ .check_timings = opa362_check_timings,
+ .set_timings = opa362_set_timings,
+ .get_timings = opa362_get_timings,
+
+ .set_type = opa362_set_type,
+};
+
+static int opa362_probe(struct platform_device *pdev)
+{
+ struct device_node *node = pdev->dev.of_node;
+ struct panel_drv_data *ddata;
+ struct omap_dss_device *dssdev, *in;
+ struct gpio_desc *gpio;
+ int r;
+
+ dev_dbg(&pdev->dev, "probe\n");
+
+ if (node = NULL) {
+ dev_err(&pdev->dev, "Unable to find device tree\n");
+ return -EINVAL;
+ }
+
+ ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL);
+ if (!ddata)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, ddata);
+
+ gpio = devm_gpiod_get(&pdev->dev, "enable");
+ if (IS_ERR(gpio)) {
+ if (PTR_ERR(gpio) != -ENOENT)
+ return PTR_ERR(gpio);
+ } else {
+ gpiod_direction_output(gpio, 0);
+ }
+
+ ddata->enable_gpio = gpio;
+
+ in = omapdss_of_find_source_for_first_ep(node);
+ if (IS_ERR(in)) {
+ dev_err(&pdev->dev, "failed to find video source\n");
+ return PTR_ERR(in);
+ }
+
+ ddata->in = in;
+
+ dssdev = &ddata->dssdev;
+ dssdev->ops.atv = &opa362_atv_ops;
+ dssdev->dev = &pdev->dev;
+ dssdev->type = OMAP_DISPLAY_TYPE_VENC;
+ dssdev->output_type = OMAP_DISPLAY_TYPE_VENC;
+ dssdev->owner = THIS_MODULE;
+
+ r = omapdss_register_output(dssdev);
+ if (r) {
+ dev_err(&pdev->dev, "Failed to register output\n");
+ goto err_reg;
+ }
+
+ return 0;
+err_reg:
+ omap_dss_put_device(ddata->in);
+ return r;
+}
+
+static int __exit opa362_remove(struct platform_device *pdev)
+{
+ struct panel_drv_data *ddata = platform_get_drvdata(pdev);
+ struct omap_dss_device *dssdev = &ddata->dssdev;
+ struct omap_dss_device *in = ddata->in;
+
+ omapdss_unregister_output(&ddata->dssdev);
+
+ WARN_ON(omapdss_device_is_enabled(dssdev));
+ if (omapdss_device_is_enabled(dssdev))
+ opa362_disable(dssdev);
+
+ WARN_ON(omapdss_device_is_connected(dssdev));
+ if (omapdss_device_is_connected(dssdev))
+ opa362_disconnect(dssdev, dssdev->dst);
+
+ omap_dss_put_device(in);
+
+ return 0;
+}
+
+static const struct of_device_id opa362_of_match[] = {
+ { .compatible = "omapdss,ti,opa362", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, opa362_of_match);
+
+static struct platform_driver opa362_driver = {
+ .probe = opa362_probe,
+ .remove = __exit_p(opa362_remove),
+ .driver = {
+ .name = "amplifier-opa362",
+ .owner = THIS_MODULE,
+ .of_match_table = opa362_of_match,
+ },
+};
+
+module_platform_driver(opa362_driver);
+
+MODULE_AUTHOR("H. Nikolaus Schaller <hns@goldelico.com>");
+MODULE_DESCRIPTION("OPA362 analog video amplifier with output/power control");
+MODULE_LICENSE("GPL");
--
1.9.1
^ permalink raw reply related
* [PATCH v3 2/3] Documentation: DT: Add documentation for ti,opa362 bindings
From: Marek Belisko @ 2014-11-30 13:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1417352867-7436-1-git-send-email-marek@goldelico.com>
Signed-off-by: Marek Belisko <marek@goldelico.com>
---
.../devicetree/bindings/video/ti,opa362.txt | 38 ++++++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/video/ti,opa362.txt
diff --git a/Documentation/devicetree/bindings/video/ti,opa362.txt b/Documentation/devicetree/bindings/video/ti,opa362.txt
new file mode 100644
index 0000000..4747ef9
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/ti,opa362.txt
@@ -0,0 +1,38 @@
+OPA362 analog video amplifier
+
+Required properties:
+- compatible: "ti,opa362"
+- gpio: enable/disable output gpio
+
+Required node:
+- Video port 0 for opa362 input
+- Video port 1 for opa362 output
+
+Example:
+
+tv_amp: opa362 {
+ compatible = "ti,opa362";
+ gpios = <&gpio1 23 0>; /* GPIO to enable video out amplifier */
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ opa_in: endpoint@0 {
+ remote-endpoint = <&venc_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ opa_out: endpoint@0 {
+ remote-endpoint = <&tv_connector_in>;
+ };
+ };
+ };
+};
+
+
+
--
1.9.1
^ permalink raw reply related
* [PATCH v3 3/3] arm: dts: omap3-gta04: Add handling for tv output
From: Marek Belisko @ 2014-11-30 13:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1417352867-7436-1-git-send-email-marek@goldelico.com>
Add handling for gta04 tv out chain:
venc -> opa362 -> svideo
Use invert-polarity in venc node because opa362
is doing polarity inversion also.
Signed-off-by: Marek Belisko <marek@goldelico.com>
---
arch/arm/boot/dts/omap3-gta04.dtsi | 49 ++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi
index fd34f91..e9f221d 100644
--- a/arch/arm/boot/dts/omap3-gta04.dtsi
+++ b/arch/arm/boot/dts/omap3-gta04.dtsi
@@ -83,6 +83,41 @@
compatible = "usb-nop-xceiv";
reset-gpios = <&gpio6 14 GPIO_ACTIVE_LOW>;
};
+
+ tv0: connector@1 {
+ compatible = "svideo-connector";
+ label = "tv";
+
+ port {
+ tv_connector_in: endpoint {
+ remote-endpoint = <&opa_out>;
+ };
+ };
+ };
+
+ tv_amp: opa362 {
+ compatible = "ti,opa362";
+ gpios = <&gpio1 23 0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ opa_in: endpoint@0 {
+ remote-endpoint = <&venc_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ opa_out: endpoint@0 {
+ remote-endpoint = <&tv_connector_in>;
+ };
+ };
+ };
+ };
};
&omap3_pmx_core {
@@ -396,6 +431,20 @@
};
};
+&venc {
+ status = "okay";
+
+ vdda-supply = <&vdac>;
+
+ port {
+ venc_out: endpoint {
+ remote-endpoint = <&opa_in>;
+ ti,channels = <2>;
+ ti,invert-polarity;
+ };
+ };
+};
+
&gpmc {
ranges = <0 0 0x30000000 0x04>; /* CS0: NAND */
--
1.9.1
^ permalink raw reply related
* RE: [PATCH 0/4] LS1021A: Add dcfb framebuffer driver support.
From: Li.Xiubo @ 2014-12-01 2:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1416824454-28156-1-git-send-email-Li.Xiubo@freescale.com>
Hi,
I'd like to know the status of this patches for internal release needed.
Thanks very much,
BRs
Xiubo
> -----Original Message-----
> From: Xiubo Li [mailto:Li.Xiubo@freescale.com]
> Sent: Monday, November 24, 2014 6:21 PM
> To: plagnioj@jcrosoft.com; tomi.valkeinen@ti.com
> Cc: shawn.guo@linaro.org; linux-fbdev@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Xiubo Li-B47053
> Subject: [PATCH 0/4] LS1021A: Add dcfb framebuffer driver support.
>
> Framebuffer driver for the Freescale SoC Display Controller.
>
> Xiubo Li (4):
> video: fsl-dcfb: Add dcfb framebuffer driver for LS1021A platform
> video: fsl-dcfb: Add devicetree binding support
> ARM: ls1021a: dtsi: Add dt node support for dcfb.
> ARM: ls1021a: dts: Add and enable dt node for dcfb.
>
> .../devicetree/bindings/video/fsl,dcfb.txt | 52 ++
> arch/arm/boot/dts/ls1021a-twr.dts | 26 +
> arch/arm/boot/dts/ls1021a.dtsi | 10 +
> drivers/video/fbdev/Kconfig | 19 +
> drivers/video/fbdev/Makefile | 1 +
> drivers/video/fbdev/fsl-dcfb.c | 811
> +++++++++++++++++++++
> 6 files changed, 919 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/video/fsl,dcfb.txt
> create mode 100644 drivers/video/fbdev/fsl-dcfb.c
>
> --
> 2.1.0.27.g96db324
^ permalink raw reply
* Re: [PATCH v7.1 09/19] ASoC: omap-hdmi-audio: Add platform device for OMAP HDMI audio support
From: Jyri Sarha @ 2014-12-01 7:15 UTC (permalink / raw)
To: Mark Brown
Cc: alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi,
liam.r.girdwood, tomi.valkeinen
In-Reply-To: <20141129113230.GC7712@sirena.org.uk>
On 11/29/2014 01:32 PM, Mark Brown wrote:
> On Wed, Nov 12, 2014 at 04:41:00PM +0200, Jyri Sarha wrote:
>> The platform device should only be registered from OMAPDSS HDMI
>> driver. The platform driver registers and unregisters all ASoC
>> components needed for OMAP HDMI audio.
>
> So, this basically seems fine but I'm looking at this code and I'm not
> really seeing a huge amount of OMAP specificness here. Why is this an
> OMAP driver and not something more generic - can't we have other HDMI
> controllers offering the same interface? There's a few serial numbers
> that would need filing off but since the ASoC driver is basically just
> interface matching and not talking to the hardware it looks like it
> ought to be usable by more than just OMAP.
>
The only reason is that I have not encountered any other audio capable
HDMI device falling in this category (cpu-dai integrated within HDMI
device block). Without at least two devices utilizing this code it
remains questionable whether the code is really generic. At least the
dss_version should be replaced with some explicit data to fill struct
snd_soc_dai_driver. Also struct omap_dss_audio is coming directly from
omap dss code, its contents is generic but I am not sure if the
information within struct snd_aes_iec958 and struct snd_cea_861_aud_if
is enough to configure the HDMI IP in all possible cases.
I can work on making this API look more generic, but before there is
another candidate to use this API it is questionable if the code will
ever be used by any other device.
Best regards,
Jyri
^ permalink raw reply
* Re: [PATCH v7.1 00/19] Rework OMAP4+ HDMI audio support
From: Tomi Valkeinen @ 2014-12-01 9:07 UTC (permalink / raw)
To: Mark Brown, Jyri Sarha
Cc: alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi,
liam.r.girdwood
In-Reply-To: <20141129115957.GE7712@sirena.org.uk>
[-- Attachment #1: Type: text/plain, Size: 910 bytes --]
On 29/11/14 13:59, Mark Brown wrote:
> On Wed, Nov 12, 2014 at 04:40:51PM +0200, Jyri Sarha wrote:
>> The patches are based on:
>> git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git for-next
>>
>> The base, the patches, and couple of additional not-to-be-merged
>> omap2plus_defconfig patches can be found here:
>> https://github.com/jsarha/linux.git omap-hdmi-audio
>
> I guess I'm OK with these so
>
> Reviewed-by: Mark Brown <broonie@kernel.org>
Thanks. And just to be sure, that's "ok, we can merge these in the next
merge window"?
> but like I said in reply to the patch adding the new driver I think
> we're going to want to generalize this a bit.
Yep, if I'm not mistaken I did suggest that to Jyri at some point. We
can continue working on that, but I'd rather not do anything big on that
front before there is some other SoC that has the same setup.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 1/4] video: fsl-dcfb: Add dcfb framebuffer driver for LS1021A platform
From: Alexander Stein @ 2014-12-01 13:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1416824454-28156-2-git-send-email-Li.Xiubo@freescale.com>
On Monday 24 November 2014 18:20:51, Xiubo Li wrote:
> [...]
> +static int fsl_dcfb_init_fbinfo(struct fb_info *info)
> +{
> + struct fsl_dcfb_mfb_info *mfbi = info->par;
> + struct fb_var_screeninfo *var = &info->var;
> + struct fsl_dcfb_fb_private *dcfb = mfbi->parent;
> + struct device_node *np = dcfb->dev->of_node;
> + struct device_node *dnp, *tnp;
> + struct display_timings *timings;
> + int i, ret;
> +
> + dnp = of_parse_phandle(np, "display", 0);
> + if (!dnp) {
> + dev_err(dcfb->dev, "failed to find \"display\" phandle.\n");
> + return -ENODEV;
> + }
> +
> + ret = of_property_read_u32(dnp, "bits-per-pixel",
> + &var->bits_per_pixel);
> + if (ret < 0) {
> + dev_err(dcfb->dev, "failed to get \"bits-per-pixel\" property.\n");
> + goto put_dnp;
> + }
> +
> + timings = of_get_display_timings(dnp);
> + if (!timings) {
> + dev_err(dcfb->dev, "failed to get display timings\n");
> + return -ENODEV;
> + goto put_dnp;
> + }
> +
> + tnp = of_find_node_by_name(dnp, "display-timings");
> + if (!tnp) {
> + dev_err(dcfb->dev, "failed to find \"display-timings\" node\n");
> + return -ENODEV;
> + goto put_dnp;
> + }
> +
> + for (i = 0; i < of_get_child_count(tnp); i++) {
> + struct videomode vm;
> + struct fb_videomode fb_vm;
> +
> + ret = videomode_from_timings(timings, &vm, i);
> + if (ret < 0)
> + goto put_tnp;
> +
> + ret = fb_videomode_from_videomode(&vm, &fb_vm);
> + if (ret < 0)
> + goto put_tnp;
> +
> + fb_add_videomode(&fb_vm, &info->modelist);
> + }
Adding each display timing to the modelist here...
> [...]
> + ret = fsl_dcfb_init_fbinfo(info);
> + if (ret)
> + goto err_cmap;
> +
> + modelist = list_first_entry(&info->modelist,
> + struct fb_modelist, list);
> + fb_videomode_to_var(&info->var, &modelist->mode);
... and just picking the first here, essentially renders the "native-mode" property in device tree as useless, since only the first in that list is picked (which seems to be the last one inserted).
Best regards,
Alexander
--
Dipl.-Inf. Alexander Stein
SYS TEC electronic GmbH
Am Windrad 2
08468 Heinsdorfergrund
Tel.: 03765 38600-1156
Fax: 03765 38600-4100
Email: alexander.stein@systec-electronic.com
Website: www.systec-electronic.com
Managing Director: Dipl.-Phys. Siegmar Schmidt
Commercial registry: Amtsgericht Chemnitz, HRB 28082
^ permalink raw reply
* Re: [PATCH 1/2] backlight: pwm: don't call legacy pwm request for device defined in dt
From: Vladimir Zapolskiy @ 2014-12-01 14:47 UTC (permalink / raw)
To: Thierry Reding; +Cc: linux-fbdev, linux-pwm, Jingoo Han, Bryan Wu, Lee Jones
In-Reply-To: <545CDDF5.70905@mentor.com>
Hello Thierry,
On 07.11.2014 16:57, Vladimir Zapolskiy wrote:
> Thierry,
>
> On 07.11.2014 16:19, Vladimir Zapolskiy wrote:
>> Hi Thierry,
>>
>> On 07.11.2014 15:48, Thierry Reding wrote:
>>> On Sat, Oct 11, 2014 at 04:46:25PM +0300, Vladimir Zapolskiy wrote:
>>>> Platform PWM backlight data provided by board's device tree should be
>>>> complete enough to successfully request a pwm device using pwm_get() API.
>>>>
>>>> Based on initial implementation done by Dmitry Eremin-Solenikov.
>>>>
>>>> Reported-by: Dmitry Eremin-Solenikov <dmitry_eremin@mentor.com>
>>>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
>>>> Cc: Thierry Reding <thierry.reding@gmail.com>
>>>> Cc: Jingoo Han <jg1.han@samsung.com>
>>>> Cc: Bryan Wu <cooloney@gmail.com>
>>>> Cc: Lee Jones <lee.jones@linaro.org>
>>>> ---
>>>> drivers/video/backlight/pwm_bl.c | 14 +++++++-------
>>>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>>
>>> I don't really understand what this is supposed to do. The commit
>>> message doesn't make a very good job of explaining it either.
>>>
>>> Can you describe in more detail what problem this fixes and why it
>>> should be merged?
>>
>> thank you for review.
>>
>> As it is shown by the code this particular change rejects fallback to
>> legacy PWM device request (which itself in turn is fixed in the next
>> commit) for boards with supplied DTS, "pwm-backlight" compatible node
>> and unregistered corresponding PWM device in that node.
>>
>> I don't know if there is a good enough reason to register PWM backlight
>> device connected to some quite arbitrary PWM device, if no PWM device
>> information is given in the "pwm-backlight" compatible node, so I think
>> it makes sense to change the default policy.
>>
>
> also please note that
> Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
> quite fairly describes "pwms" as a required property, but right now this
> statement from the documentation is wrong, it is possible to register
> pwm-backlight device driver (using notorious pwm_request() legacy API)
> connected to some unspecified pwm device.
>
> I don't think that the current registration policy is correct, that's
> why I propose to fix the logic instead of making a documentation update.
>
have you had a chance to check the rationale of the change?
If you accept it, should I make the commit message more verbose?
--
With best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH v7.1 09/19] ASoC: omap-hdmi-audio: Add platform device for OMAP HDMI audio support
From: Mark Brown @ 2014-12-01 19:21 UTC (permalink / raw)
To: Jyri Sarha
Cc: alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi,
liam.r.girdwood, tomi.valkeinen
In-Reply-To: <547C1598.1000509@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1378 bytes --]
On Mon, Dec 01, 2014 at 09:15:36AM +0200, Jyri Sarha wrote:
> The only reason is that I have not encountered any other audio capable HDMI
> device falling in this category (cpu-dai integrated within HDMI device
> block). Without at least two devices utilizing this code it remains
> questionable whether the code is really generic. At least the dss_version
> should be replaced with some explicit data to fill struct
> snd_soc_dai_driver. Also struct omap_dss_audio is coming directly from omap
> dss code, its contents is generic but I am not sure if the information
> within struct snd_aes_iec958 and struct snd_cea_861_aud_if is enough to
> configure the HDMI IP in all possible cases.
It should be no big deal to add extra information if required. Given
how poor the upstreaming state is for HDMI I expect we'd get people
coming out of the woodwork if there were infrastructure, some of the
Exynos stuff looked similar to your situation for example.
> I can work on making this API look more generic, but before there is another
> candidate to use this API it is questionable if the code will ever be used
> by any other device.
Unless there is something obviously terrible about the hardware design
that I'm missing which means nobody else would ever have implemented
something like it I'd expect there will be at least one other system
that can make use of the code.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH v7.1 00/19] Rework OMAP4+ HDMI audio support
From: Mark Brown @ 2014-12-01 19:31 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jyri Sarha, alsa-devel, linux-fbdev, linux-omap, peter.ujfalusi,
liam.r.girdwood
In-Reply-To: <547C2FBA.4030204@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1091 bytes --]
On Mon, Dec 01, 2014 at 11:07:06AM +0200, Tomi Valkeinen wrote:
> On 29/11/14 13:59, Mark Brown wrote:
> > Reviewed-by: Mark Brown <broonie@kernel.org>
> Thanks. And just to be sure, that's "ok, we can merge these in the next
> merge window"?
Yes.
> > but like I said in reply to the patch adding the new driver I think
> > we're going to want to generalize this a bit.
> Yep, if I'm not mistaken I did suggest that to Jyri at some point. We
> can continue working on that, but I'd rather not do anything big on that
> front before there is some other SoC that has the same setup.
I really want to see people making an effort to make code shareable here
- I think one of the reasons nobody is upstreaming any of their code is
that there's nothing generic in place to handle generic tasks so people
just look at their code, think it's too much of a device specific hack
and think they'll get back to looking at it later. If this is a
sensible set of callbacks to have to pass configuration around let's
make that discoverable without requiring people to look through the OMAP
drivers.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* [PATCH v2 2/2] backlight/lp855x: Add supply regulator to lp855x
From: Sean Paul @ 2014-12-01 21:07 UTC (permalink / raw)
To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, milo.kim-l0cyMroinI0
Cc: jg1.han-Sze3O3UU22JBDgjK7y7TUQ, cooloney-Re5JQEeQqe8AvxtiuMwx3w,
lee.jones-QSEj5FYQhm4dnm+yROfE0A, Sean Paul,
Stéphane Marchesin, Aaron Durbin
In-Reply-To: <54767F43.2060901-l0cyMroinI0@public.gmane.org>
This patch adds a supply regulator to the lp855x platform data to facilitate
powering on/off the 3V rail attached to the controller.
Cc: Stéphane Marchesin <marcheu@chromium.org>
Cc: Aaron Durbin <adurbin@chromium.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
Changes in v2:
- Add NULL check in lp855x_remove
.../devicetree/bindings/video/backlight/lp855x.txt | 2 ++
drivers/video/backlight/lp855x_bl.c | 18 ++++++++++++++++++
include/linux/platform_data/lp855x.h | 2 ++
3 files changed, 22 insertions(+)
diff --git a/Documentation/devicetree/bindings/video/backlight/lp855x.txt b/Documentation/devicetree/bindings/video/backlight/lp855x.txt
index 96e83a5..0a3ecbc 100644
--- a/Documentation/devicetree/bindings/video/backlight/lp855x.txt
+++ b/Documentation/devicetree/bindings/video/backlight/lp855x.txt
@@ -12,6 +12,7 @@ Optional properties:
- pwm-period: PWM period value. Set only PWM input mode used (u32)
- rom-addr: Register address of ROM area to be updated (u8)
- rom-val: Register value to be updated (u8)
+ - power-supply: Regulator which controls the 3V rail
Example:
@@ -56,6 +57,7 @@ Example:
backlight@2c {
compatible = "ti,lp8557";
reg = <0x2c>;
+ power-supply = <&backlight_vdd>;
dev-ctrl = /bits/ 8 <0x41>;
init-brt = /bits/ 8 <0x0a>;
diff --git a/drivers/video/backlight/lp855x_bl.c b/drivers/video/backlight/lp855x_bl.c
index 257b3ba..a26d3bb 100644
--- a/drivers/video/backlight/lp855x_bl.c
+++ b/drivers/video/backlight/lp855x_bl.c
@@ -17,6 +17,7 @@
#include <linux/of.h>
#include <linux/platform_data/lp855x.h>
#include <linux/pwm.h>
+#include <linux/regulator/consumer.h>
/* LP8550/1/2/3/6 Registers */
#define LP855X_BRIGHTNESS_CTRL 0x00
@@ -383,6 +384,13 @@ static int lp855x_parse_dt(struct lp855x *lp)
pdata->rom_data = &rom[0];
}
+ pdata->supply = devm_regulator_get(dev, "power");
+ if (IS_ERR(pdata->supply)) {
+ if (PTR_ERR(pdata->supply) = -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ pdata->supply = NULL;
+ }
+
lp->pdata = pdata;
return 0;
@@ -423,6 +431,14 @@ static int lp855x_probe(struct i2c_client *cl, const struct i2c_device_id *id)
else
lp->mode = REGISTER_BASED;
+ if (lp->pdata->supply) {
+ ret = regulator_enable(lp->pdata->supply);
+ if (ret < 0) {
+ dev_err(&cl->dev, "failed to enable supply: %d\n", ret);
+ return ret;
+ }
+ }
+
i2c_set_clientdata(cl, lp);
ret = lp855x_configure(lp);
@@ -454,6 +470,8 @@ static int lp855x_remove(struct i2c_client *cl)
lp->bl->props.brightness = 0;
backlight_update_status(lp->bl);
+ if (lp->pdata->supply)
+ regulator_disable(lp->pdata->supply);
sysfs_remove_group(&lp->dev->kobj, &lp855x_attr_group);
return 0;
diff --git a/include/linux/platform_data/lp855x.h b/include/linux/platform_data/lp855x.h
index 1b2ba24..9c7fd1e 100644
--- a/include/linux/platform_data/lp855x.h
+++ b/include/linux/platform_data/lp855x.h
@@ -136,6 +136,7 @@ struct lp855x_rom_data {
Only valid when mode is PWM_BASED.
* @size_program : total size of lp855x_rom_data
* @rom_data : list of new eeprom/eprom registers
+ * @supply : regulator that supplies 3V input
*/
struct lp855x_platform_data {
const char *name;
@@ -144,6 +145,7 @@ struct lp855x_platform_data {
unsigned int period_ns;
int size_program;
struct lp855x_rom_data *rom_data;
+ struct regulator *supply;
};
#endif
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
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