Re: [PATCHv8 07/15] cec: add HDMI CEC framework
On Tue, Aug 18, 2015 at 11:00:20AM +0100, Russell King - ARM Linux wrote:
On Tue, Aug 18, 2015 at 10:26:32AM +0200, Hans Verkuil wrote:
+ /* Part 2: Initialize and register the character device */
+ cdev_init(cecdev-cdev, cec_devnode_fops);
+ cecdev-cdev.owner = owner;
+
+ ret = cdev_add(cecdev-cdev, MKDEV(MAJOR(cec_dev_t), cecdev-minor),
+ 1);
+ if (ret 0) {
+ pr_err(%s: cdev_add failed\n, __func__);
+ goto error;
+ }
+
+ /* Part 3: Register the cec device */
+ cecdev-dev.bus = cec_bus_type;
+ cecdev-dev.devt = MKDEV(MAJOR(cec_dev_t), cecdev-minor);
+ cecdev-dev.release = cec_devnode_release;
+ if (cecdev-parent)
+ cecdev-dev.parent = cecdev-parent;
+ dev_set_name(cecdev-dev, cec%d, cecdev-minor);
+ ret = device_register(cecdev-dev);
It's worth pointing out that you can greatly simplify the lifetime
handling (you don't need to get and put cecdev-dev) if you make
the cdev a child of the cecdev-dev.
If you grep for kobj.parent in drivers/ you'll see many drivers are
doing this.
cecdev-cdev.kobj.parent = cecdev-dev.kobj;
but you will need to call device_initialize() on cecdev-dev first,
and use device_add() here.
This is basically a requirement if one embeds both device and a cdev
into the same structure. Trying to do get/put in the driver is racy,
you need to let framework know (by setting cdve's parent to the device
structure).
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 04/15] HID: add HDMI CEC specific keycodes
On Tue, Jun 30, 2015 at 09:36:49AM +0200, Hans Verkuil wrote: On 06/29/15 21:25, Dmitry Torokhov wrote: On Mon, Jun 29, 2015 at 12:14:49PM +0200, Hans Verkuil wrote: From: Kamil Debski ka...@wypas.org Add HDMI CEC specific keycodes to the keycodes definition. Signed-off-by: Kamil Debski ka...@wypas.org Signed-off-by: Hans Verkuil hans.verk...@cisco.com Could you please describe the intended use for these keycodes for people who do not live and breathe CEC specs? Do you want this as comments in the patch or as the commit description? In the patch would be preferable. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 04/15] HID: add HDMI CEC specific keycodes
On Mon, Jun 29, 2015 at 12:14:49PM +0200, Hans Verkuil wrote:
From: Kamil Debski ka...@wypas.org
Add HDMI CEC specific keycodes to the keycodes definition.
Signed-off-by: Kamil Debski ka...@wypas.org
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
Could you please describe the intended use for these keycodes for people
who do not live and breathe CEC specs?
Thanks!
---
include/uapi/linux/input.h | 12
1 file changed, 12 insertions(+)
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index 731417c..7430a3f 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -752,6 +752,18 @@ struct input_keymap_entry {
#define KEY_KBDINPUTASSIST_ACCEPT0x264
#define KEY_KBDINPUTASSIST_CANCEL0x265
+#define KEY_RIGHT_UP 0x266
+#define KEY_RIGHT_DOWN 0x267
+#define KEY_LEFT_UP 0x268
+#define KEY_LEFT_DOWN0x269
+
+#define KEY_NEXT_FAVORITE0x270
+#define KEY_STOP_RECORD 0x271
+#define KEY_PAUSE_RECORD 0x272
+#define KEY_VOD 0x273
+#define KEY_UNMUTE 0x274
+#define KEY_DVB 0x275
+
#define BTN_TRIGGER_HAPPY0x2c0
#define BTN_TRIGGER_HAPPY1 0x2c0
#define BTN_TRIGGER_HAPPY2 0x2c1
--
2.1.4
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 05/15] input.h: add BUS_CEC type
On Mon, Jun 29, 2015 at 12:14:50PM +0200, Hans Verkuil wrote:
Inputs can come in over the HDMI CEC bus, so add a new type for this.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
Acked-by: Dmitry Torokhov dmitry.torok...@gmail.com
---
include/uapi/linux/input.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index 7430a3f..0af80b2 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -984,6 +984,7 @@ struct input_keymap_entry {
#define BUS_GSC 0x1A
#define BUS_ATARI0x1B
#define BUS_SPI 0x1C
+#define BUS_CEC 0x1D
/*
* MT_TOOL types
--
2.1.4
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] Input: s3c2410_ts: fix S3C_ADC dependency
On Mon, Mar 02, 2015 at 01:35:54PM +0100, Arnd Bergmann wrote: S3C_ADC is only available on machines that don't do ARCH_MULTIPLATFORM, so changing the 'select' into 'depends on' here helps us move to ARCH_MULTIPLATFORM without introducing regressions. Signed-off-by: Arnd Bergmann a...@arndb.de Acked-by: Dmitry Torokhov dmitry.torok...@gmail.com --- drivers/input/touchscreen/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig index 58917525126e..3b4cd9ab1af0 100644 --- a/drivers/input/touchscreen/Kconfig +++ b/drivers/input/touchscreen/Kconfig @@ -326,7 +326,7 @@ config TOUCHSCREEN_ILI210X config TOUCHSCREEN_S3C2410 tristate Samsung S3C2410/generic touchscreen input driver depends on ARCH_S3C24XX || SAMSUNG_DEV_TS - select S3C_ADC + depends on S3C_ADC help Say Y here if you have the s3c2410 touchscreen. -- 2.1.0.rc2 -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/10] iio: exynos-adc: add experimental touchscreen support
Hi Arnd,
On Mon, Mar 02, 2015 at 01:35:59PM +0100, Arnd Bergmann wrote:
This adds support for the touchscreen on Samsung s3c64xx.
The driver is completely untested but shows roughly how
it could be done, following the example of the at91 driver.
compared to the old plat-samsung/adc driver, there is
no support for prioritizing ts over other clients, nor
for oversampling. From my reading of the code, the
priorities didn't actually have any effect at all, but
the oversampling might be needed.
Verifying this driver is the main issue that is currently
holding up multiplatform support for s3c64xx, so any help
in testing is very much appreciated.
The current version uses the IS_REACHABLE() that is
going to be introduced in the linux-media tree, please
comment this out for testing.
Signed-off-by: Arnd Bergmann a...@arndb.de
Acked-by: Dmitry Torokhov dmitry.torok...@gmail.com
---
.../devicetree/bindings/arm/samsung/exynos-adc.txt | 3 +
drivers/iio/adc/exynos_adc.c | 222
-
2 files changed, 218 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
index f46ca9a316a2..ccaaec6014bd 100644
--- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
@@ -47,6 +47,9 @@ Required properties:
- samsung,syscon-phandle Contains the PMU system controller node
(To access the ADC_PHY register on
Exynos5250/5420/5800/3250)
+Optional properties:
+- has-touchscreen: If present, indicates that a touchscreen is
+ connected an usable.
Note: child nodes can be added for auto probing from device tree.
diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index 3a2dbb3b4926..75cd381a8181 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -35,6 +35,7 @@
#include linux/regulator/consumer.h
#include linux/of_platform.h
#include linux/err.h
+#include linux/input.h
#include linux/iio/iio.h
#include linux/iio/machine.h
@@ -42,12 +43,18 @@
#include linux/mfd/syscon.h
#include linux/regmap.h
+#include linux/platform_data/touchscreen-s3c2410.h
+
/* S3C/EXYNOS4412/5250 ADC_V1 registers definitions */
#define ADC_V1_CON(x)((x) + 0x00)
+#define ADC_V1_TSC(x)((x) + 0x04)
#define ADC_V1_DLY(x)((x) + 0x08)
#define ADC_V1_DATX(x) ((x) + 0x0C)
+#define ADC_V1_DATY(x) ((x) + 0x10)
+#define ADC_V1_UPDN(x) ((x) + 0x14)
#define ADC_V1_INTCLR(x) ((x) + 0x18)
#define ADC_V1_MUX(x)((x) + 0x1c)
+#define ADC_V1_CLRINTPNDNUP(x) ((x) + 0x20)
/* S3C2410 ADC registers definitions */
#define ADC_S3C2410_MUX(x) ((x) + 0x18)
@@ -71,6 +78,30 @@
#define ADC_S3C2410_DATX_MASK0x3FF
#define ADC_S3C2416_CON_RES_SEL (1u 3)
+/* touch screen always uses channel 0 */
+#define ADC_S3C2410_MUX_TS 0
+
+/* ADCTSC Register Bits */
+#define ADC_S3C2443_TSC_UD_SEN (1u 8)
+#define ADC_S3C2410_TSC_YM_SEN (1u 7)
+#define ADC_S3C2410_TSC_YP_SEN (1u 6)
+#define ADC_S3C2410_TSC_XM_SEN (1u 5)
+#define ADC_S3C2410_TSC_XP_SEN (1u 4)
+#define ADC_S3C2410_TSC_PULL_UP_DISABLE (1u 3)
+#define ADC_S3C2410_TSC_AUTO_PST (1u 2)
+#define ADC_S3C2410_TSC_XY_PST(x)(((x) 0x3) 0)
+
+#define ADC_TSC_WAIT4INT (ADC_S3C2410_TSC_YM_SEN | \
+ ADC_S3C2410_TSC_YP_SEN | \
+ ADC_S3C2410_TSC_XP_SEN | \
+ ADC_S3C2410_TSC_XY_PST(3))
+
+#define ADC_TSC_AUTOPST (ADC_S3C2410_TSC_YM_SEN | \
+ ADC_S3C2410_TSC_YP_SEN | \
+ ADC_S3C2410_TSC_XP_SEN | \
+ ADC_S3C2410_TSC_AUTO_PST | \
+ ADC_S3C2410_TSC_XY_PST(0))
+
/* Bit definitions for ADC_V2 */
#define ADC_V2_CON1_SOFT_RESET (1u 2)
@@ -88,7 +119,9 @@
/* Bit definitions common for ADC_V1 and ADC_V2 */
#define ADC_CON_EN_START (1u 0)
#define ADC_CON_EN_START_MASK(0x3 0)
+#define ADC_DATX_PRESSED (1u 15)
#define ADC_DATX_MASK0xFFF
+#define ADC_DATY_MASK0xFFF
#define EXYNOS_ADC_TIMEOUT (msecs_to_jiffies(100))
@@ -98,17 +131,24 @@
struct exynos_adc {
struct exynos_adc_data *data;
struct device *dev;
+ struct input_dev*input;
void __iomem*regs;
struct regmap *pmu_map;
struct clk *clk;
struct clk *sclk;
unsigned intirq;
+ unsigned inttsirq;
+ unsigned intdelay;
struct regulator*vdd
Re: [PATCH 00/10] ARM: s3c64xx multiplatform, help needed
On Mon, Mar 02, 2015 at 01:35:53PM +0100, Arnd Bergmann wrote: Hi everyone, I've had these patches in a private git tree for a while, and have finally gotten around to clean them up some more for submission. Hopefully we can get all of it merged into 4.1. I've done this to the best of my knowledge, but some parts are a bit tricky, so I expect that there are bugs. The trickiest part is the touchscreen driver. I've sent it out for review last year already, but I have not found anybody who could test it, and it's basically a blind rewrite of an existing driver, so it's unlikely that I got it all right. I think Vasily had access to hardware with s3c2410_ts at some point... The other parts may actually work, but it is possible that I made a mistake with the ASoC driver, the sparseirq support or something else. Does anyone still have access to the hardware? I'm particularly interested in seeing this patch set get tested on smartq and on smdk6410, which have the majority of the hardware. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc: dw_mmc: exynos: remove incorrect __exit_p()
dw_mci_pltfm_remove() is not (nor should it be) marked as __exit,
so we should not be using __exit_p() wrapper with it.
Signed-off-by: Dmitry Torokhov dmitry.torok...@gmail.com
---
drivers/mmc/host/dw_mmc-exynos.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 12a5eaa..fe32948 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -422,7 +422,7 @@ static const struct dev_pm_ops dw_mci_exynos_pmops = {
static struct platform_driver dw_mci_exynos_pltfm_driver = {
.probe = dw_mci_exynos_probe,
- .remove = __exit_p(dw_mci_pltfm_remove),
+ .remove = dw_mci_pltfm_remove,
.driver = {
.name = dwmmc_exynos,
.of_match_table = dw_mci_exynos_match,
--
2.2.0.rc0.207.ga3a616c
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/3] Input: add regulator haptic driver
HI Jaewon,
On Wed, Dec 17, 2014 at 12:35:06PM +0900, Jaewon Kim wrote:
This patch adds support for haptic driver controlled by
voltage of regulator. And this driver support for
Force Feedback interface from input framework
Signed-off-by: Jaewon Kim jaewon02@samsung.com
Signed-off-by: Hyunhee Kim hyunhee@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
Tested-by: Chanwoo Choi cw00.c...@samsung.com
Reviewed-by: Chanwoo Choi cw00.c...@samsung.com
Reviewed-by: Pankaj Dubey pankaj.du...@samsung.com
Does the driver still work if you apply the patch below on top of yours?
Thanks.
--
Dmitry
Input: regulator-haptics - misc changes
From: Dmitry Torokhov dmitry.torok...@gmail.com
Signed-off-by: Dmitry Torokhov dmitry.torok...@gmail.com
---
drivers/input/misc/Kconfig|4 -
drivers/input/misc/regulator-haptic.c | 164 -
2 files changed, 100 insertions(+), 68 deletions(-)
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index e5e556d..0b652c5 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -395,11 +395,11 @@ config INPUT_CM109
called cm109.
config INPUT_REGULATOR_HAPTIC
- tristate regulator haptics support
+ tristate Regulator haptics support
select INPUT_FF_MEMLESS
help
This option enables device driver support for the haptic controlled
- by regulator. This driver supports ff-memless interface
+ by a regulator. This driver supports ff-memless interface
from input framework.
To compile this driver as a module, choose M here: the
diff --git a/drivers/input/misc/regulator-haptic.c
b/drivers/input/misc/regulator-haptic.c
index 16f5ec8..9426221 100644
--- a/drivers/input/misc/regulator-haptic.c
+++ b/drivers/input/misc/regulator-haptic.c
@@ -28,55 +28,78 @@ struct regulator_haptic {
struct work_struct work;
struct mutex mutex;
- bool enabled;
- bool suspend_state;
+ bool active;
+ bool suspended;
+
unsigned int max_volt;
unsigned int min_volt;
- unsigned int intensity;
unsigned int magnitude;
};
-static void regulator_haptic_enable(struct regulator_haptic *haptic, bool
state)
+static int regulator_haptic_toggle(struct regulator_haptic *haptic, bool on)
{
int error;
- if (haptic-enabled == state)
- return;
+ if (haptic-active != on) {
+
+ error = on ? regulator_enable(haptic-regulator) :
+regulator_disable(haptic-regulator);
+ if (error) {
+ dev_err(haptic-dev,
+ failed to switch regulator %s: %d\n,
+ on ? on : off, error);
+ return error;
+ }
+
+ haptic-active = on;
+ }
- error = state ? regulator_enable(haptic-regulator) :
- regulator_disable(haptic-regulator);
+ return 0;
+}
+
+static int regulator_haptic_set_voltage(struct regulator_haptic *haptic,
+unsigned int magnitude)
+{
+ u64 volt_mag_multi;
+ unsigned int intensity;
+ int error;
+
+ volt_mag_multi = (u64)(haptic-max_volt - haptic-min_volt) * magnitude;
+ intensity = (unsigned int)(volt_mag_multi MAX_MAGNITUDE_SHIFT);
+
+ error = regulator_set_voltage(haptic-regulator,
+ intensity + haptic-min_volt,
+ haptic-max_volt);
if (error) {
- dev_err(haptic-dev, cannot enable regulator\n);
- return;
+ dev_err(haptic-dev, cannot set regulator voltage to %d: %d\n,
+ intensity + haptic-min_volt, error);
+ return error;
}
- haptic-enabled = state;
+ return 0;
}
static void regulator_haptic_work(struct work_struct *work)
{
struct regulator_haptic *haptic = container_of(work,
struct regulator_haptic, work);
+ unsigned int magnitude;
int error;
mutex_lock(haptic-mutex);
- if (haptic-suspend_state)
- goto err;
+ if (haptic-suspended)
+ goto out;
- error = regulator_set_voltage(haptic-regulator,
- haptic-intensity + haptic-min_volt, haptic-max_volt);
- if (error) {
- dev_err(haptic-dev, cannot set regulator voltage\n);
- goto err;
- }
+ magnitude = ACCESS_ONCE(haptic-magnitude);
- if (haptic-magnitude)
- regulator_haptic_enable(haptic, true);
- else
- regulator_haptic_enable(haptic, false);
+ error = regulator_haptic_set_voltage(haptic, magnitude);
+ if (error)
+ goto out;
-err:
+ regulator_haptic_toggle(haptic
Re: [PATCH v7 1/3] Input: add regulator haptic driver
On Thursday, December 18, 2014 10:17:42 AM Jaewon Kim wrote: Hi Dmity, 2014년 12월 18일 07:06에 Dmitry Torokhov 이(가) 쓴 글: HI Jaewon, On Wed, Dec 17, 2014 at 12:35:06PM +0900, Jaewon Kim wrote: This patch adds support for haptic driver controlled by voltage of regulator. And this driver support for Force Feedback interface from input framework Signed-off-by: Jaewon Kim jaewon02@samsung.com Signed-off-by: Hyunhee Kim hyunhee@samsung.com Acked-by: Kyungmin Park kyungmin.p...@samsung.com Tested-by: Chanwoo Choi cw00.c...@samsung.com Reviewed-by: Chanwoo Choi cw00.c...@samsung.com Reviewed-by: Pankaj Dubey pankaj.du...@samsung.com Does the driver still work if you apply the patch below on top of yours? Thanks. Yes, there is no problem to apply this one first. Thank you. Then I'll fold it into yours and queue the driver for the next release. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] cpufreq: exynos5440: protect call to dev_pm_opp_get_opp_count with RCU lock
dev_pm_opp_get_opp_count() must be called with RCU lock held. Signed-off-by: Dmitry Torokhov d...@chromium.org --- Again, not tested... drivers/cpufreq/exynos5440-cpufreq.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/cpufreq/exynos5440-cpufreq.c b/drivers/cpufreq/exynos5440-cpufreq.c index 21a90ed..588b9ee 100644 --- a/drivers/cpufreq/exynos5440-cpufreq.c +++ b/drivers/cpufreq/exynos5440-cpufreq.c @@ -373,7 +373,11 @@ static int exynos_cpufreq_probe(struct platform_device *pdev) failed to init cpufreq table: %d\n, ret); goto err_free_opp; } + + rcu_read_lock(); dvfs_info-freq_count = dev_pm_opp_get_opp_count(dvfs_info-dev); + rcu_read_unlock(); + exynos_sort_descend_freq_table(); if (of_property_read_u32(np, clock-latency, dvfs_info-latency)) -- 2.2.0.rc0.207.ga3a616c -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/2] Input: add regulator haptic driver
On Tue, Dec 16, 2014 at 10:09:25AM +0900, Jaewon Kim wrote:
Hi Dmitry,
2014년 12월 14일 04:56에 Dmitry Torokhov 이(가) 쓴 글:
Hi Jaewon,
On Fri, Dec 12, 2014 at 07:32:28PM +0900, Jaewon Kim wrote:
...
+static int __maybe_unused regulator_haptic_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct regulator_haptic *haptic = platform_get_drvdata(pdev);
+
+ mutex_lock(haptic-mutex);
+ if (haptic-enabled) {
+ regulator_haptic_enable(haptic, false);
+ haptic-suspend_state = true;
Why do we only set suspend_state if an effect was playing? I think we
should always indicate that the device is suspended so that we do not
try to start playing another effect - while it is true that normally
effects are played by request from userspace which should be frozen by
now, it is theoretically possible to trigger an effect from kernel as
well.
This variable name seems to make you confuse.
I used this variable to restore the old state.
When kernel is entering suspend state while the motor is vibrating,
I store vibrating state for vibrate again after escape suspend state.
I will change variable name to suspend_restore.
And prevent to start playing effect when kernel entering suspend state.
You do not need to save if haptic was playing or not - on resume, if
haptic-magnitude != 0 you need to restart playing, otherwise leave it
off.
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/2] Input: add regulator haptic driver
Hi Jaewon,
On Fri, Dec 12, 2014 at 07:32:28PM +0900, Jaewon Kim wrote:
This patch adds support for haptic driver controlled by
voltage of regulator. And this driver support for
Force Feedback interface from input framework
Signed-off-by: Jaewon Kim jaewon02@samsung.com
Signed-off-by: Hyunhee Kim hyunhee@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
Tested-by: Chanwoo Choi cw00.c...@samsung.com
Reviewed-by: Chanwoo Choi cw00.c...@samsung.com
Reviewed-by: Pankaj Dubey pankaj.du...@samsung.com
---
.../devicetree/bindings/input/regulator-haptic.txt | 21 ++
drivers/input/misc/Kconfig | 11 +
drivers/input/misc/Makefile|1 +
drivers/input/misc/regulator-haptic.c | 259
include/linux/input/regulator-haptic.h | 31 +++
5 files changed, 323 insertions(+)
create mode 100644
Documentation/devicetree/bindings/input/regulator-haptic.txt
create mode 100644 drivers/input/misc/regulator-haptic.c
create mode 100644 include/linux/input/regulator-haptic.h
diff --git a/Documentation/devicetree/bindings/input/regulator-haptic.txt
b/Documentation/devicetree/bindings/input/regulator-haptic.txt
new file mode 100644
index 000..3ed1c7e
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/regulator-haptic.txt
@@ -0,0 +1,21 @@
+* Regulator Haptic Device Tree Bindings
+
+Required Properties:
+ - compatible : Should be regulator-haptic
+ - haptic-supply : Power supply to the haptic motor.
+ [*] refer Documentation/devicetree/bindings/regulator/regulator.txt
+
+ - max-microvolt : The maximum voltage value supplied to the haptic motor.
+ [The unit of the voltage is a micro]
+
+ - min-microvolt : The minimum voltage value supplied to the haptic motor.
+ [The unit of the voltage is a micro]
+
+Example:
+
+ haptics {
+ compatible = regulator-haptic;
+ haptic-supply = motor_regulator;
+ max-microvolt = 270;
+ min-microvolt = 110;
+ };
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 23297ab..e5e556d 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -394,6 +394,17 @@ config INPUT_CM109
To compile this driver as a module, choose M here: the module will be
called cm109.
+config INPUT_REGULATOR_HAPTIC
+ tristate regulator haptics support
+ select INPUT_FF_MEMLESS
+ help
+ This option enables device driver support for the haptic controlled
+ by regulator. This driver supports ff-memless interface
+ from input framework.
+
+ To compile this driver as a module, choose M here: the
+ module will be called regulator-haptic.
+
config INPUT_RETU_PWRBUTTON
tristate Retu Power button Driver
depends on MFD_RETU
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 19c7603..1f135af 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY) += pmic8xxx-pwrkey.o
obj-$(CONFIG_INPUT_POWERMATE)+= powermate.o
obj-$(CONFIG_INPUT_PWM_BEEPER) += pwm-beeper.o
obj-$(CONFIG_INPUT_RB532_BUTTON) += rb532_button.o
+obj-$(CONFIG_INPUT_REGULATOR_HAPTIC) += regulator-haptic.o
obj-$(CONFIG_INPUT_RETU_PWRBUTTON) += retu-pwrbutton.o
obj-$(CONFIG_INPUT_GPIO_ROTARY_ENCODER) += rotary_encoder.o
obj-$(CONFIG_INPUT_SGI_BTNS) += sgi_btns.o
diff --git a/drivers/input/misc/regulator-haptic.c
b/drivers/input/misc/regulator-haptic.c
new file mode 100644
index 000..2fa94bc
--- /dev/null
+++ b/drivers/input/misc/regulator-haptic.c
@@ -0,0 +1,259 @@
+/*
+ * Regulator haptic driver
+ *
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ * Author: Jaewon Kim jaewon02@samsung.com
+ * Author: Hyunhee Kim hyunhee@samsung.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/input.h
+#include linux/input/regulator-haptic.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include linux/regulator/consumer.h
+#include linux/slab.h
+
+#define MAX_MAGNITUDE_SHIFT 16
+
+struct regulator_haptic {
+ struct device *dev;
+ struct input_dev *input_dev;
+ struct regulator *regulator;
+
+ struct work_struct work;
+ struct mutex mutex;
+
+ bool enabled;
+ bool suspend_state;
+ unsigned int max_volt;
+ unsigned int min_volt;
+ unsigned int intensity;
+ unsigned int magnitude;
+};
+
+static void regulator_haptic_enable(struct regulator_haptic *haptic, bool
state)
+{
+ int error;
+
+ if (haptic-enabled ==
Re: [PATCH v4 1/2] Input: add regulator haptic driver
Hi Jaewon,
On Mon, Dec 01, 2014 at 11:11:12AM +0900, Jaewon Kim wrote:
This patch adds support for haptic driver controlled by
voltage of regulator. And this driver support for
Force Feedback interface from input framework
Signed-off-by: Jaewon Kim jaewon02@samsung.com
Signed-off-by: Hyunhee Kim hyunhee@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
Tested-by: Chanwoo Choi cw00.c...@samsung.com
Reviewed-by: Chanwoo Choi cw00.c...@samsung.com
Reviewed-by: Pankaj Dubey pankaj.du...@samsung.com
---
.../devicetree/bindings/input/regulator-haptic.txt | 21 ++
drivers/input/misc/Kconfig | 11 +
drivers/input/misc/Makefile|1 +
drivers/input/misc/regulator-haptic.c | 247
include/linux/input/regulator-haptic.h | 31 +++
5 files changed, 311 insertions(+)
create mode 100644
Documentation/devicetree/bindings/input/regulator-haptic.txt
create mode 100644 drivers/input/misc/regulator-haptic.c
create mode 100644 include/linux/input/regulator-haptic.h
diff --git a/Documentation/devicetree/bindings/input/regulator-haptic.txt
b/Documentation/devicetree/bindings/input/regulator-haptic.txt
new file mode 100644
index 000..3ed1c7e
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/regulator-haptic.txt
@@ -0,0 +1,21 @@
+* Regulator Haptic Device Tree Bindings
+
+Required Properties:
+ - compatible : Should be regulator-haptic
+ - haptic-supply : Power supply to the haptic motor.
+ [*] refer Documentation/devicetree/bindings/regulator/regulator.txt
+
+ - max-microvolt : The maximum voltage value supplied to the haptic motor.
+ [The unit of the voltage is a micro]
+
+ - min-microvolt : The minimum voltage value supplied to the haptic motor.
+ [The unit of the voltage is a micro]
+
+Example:
+
+ haptics {
+ compatible = regulator-haptic;
+ haptic-supply = motor_regulator;
+ max-microvolt = 270;
+ min-microvolt = 110;
+ };
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 23297ab..e5e556d 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -394,6 +394,17 @@ config INPUT_CM109
To compile this driver as a module, choose M here: the module will be
called cm109.
+config INPUT_REGULATOR_HAPTIC
+ tristate regulator haptics support
+ select INPUT_FF_MEMLESS
+ help
+ This option enables device driver support for the haptic controlled
+ by regulator. This driver supports ff-memless interface
+ from input framework.
+
+ To compile this driver as a module, choose M here: the
+ module will be called regulator-haptic.
+
config INPUT_RETU_PWRBUTTON
tristate Retu Power button Driver
depends on MFD_RETU
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 19c7603..1f135af 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY) += pmic8xxx-pwrkey.o
obj-$(CONFIG_INPUT_POWERMATE)+= powermate.o
obj-$(CONFIG_INPUT_PWM_BEEPER) += pwm-beeper.o
obj-$(CONFIG_INPUT_RB532_BUTTON) += rb532_button.o
+obj-$(CONFIG_INPUT_REGULATOR_HAPTIC) += regulator-haptic.o
obj-$(CONFIG_INPUT_RETU_PWRBUTTON) += retu-pwrbutton.o
obj-$(CONFIG_INPUT_GPIO_ROTARY_ENCODER) += rotary_encoder.o
obj-$(CONFIG_INPUT_SGI_BTNS) += sgi_btns.o
diff --git a/drivers/input/misc/regulator-haptic.c
b/drivers/input/misc/regulator-haptic.c
new file mode 100644
index 000..6bc8e45
--- /dev/null
+++ b/drivers/input/misc/regulator-haptic.c
@@ -0,0 +1,247 @@
+/*
+ * Regulator haptic driver
+ *
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ * Author: Jaewon Kim jaewon02@samsung.com
+ * Author: Hyunhee Kim hyunhee@samsung.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/input.h
+#include linux/input/regulator-haptic.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include linux/regulator/consumer.h
+#include linux/slab.h
+
+#define MAX_MAGNITUDE_SHIFT 16
+
+struct regulator_haptic {
+ struct device *dev;
+ struct input_dev *input_dev;
+ struct regulator *regulator;
+ struct work_struct work;
+
+ bool enabled;
+ bool suspend_state;
+ unsigned int max_volt;
+ unsigned int min_volt;
+ unsigned int intensity;
+ unsigned int magnitude;
+};
+
+static void regulator_haptic_enable(struct regulator_haptic *haptic, bool
state)
+{
+ int error;
+
+ if (haptic-enabled == state)
+ return;
+
Re: [PATCH 1/3] PM / Domains: Initial PM clock support for genpd
On Wed, Nov 19, 2014 at 03:00:36PM +0100, Ulf Hansson wrote:
It's quite common for PM domains to use PM clocks. Typically from SOC
specific code, the per device PM clock list is created and
pm_clk_suspend|resume() are invoked to handle clock gating/ungating.
A step towards consolidation is to integrate PM clock support into
genpd, which is what this patch does.
In this initial step, the calls to the pm_clk_suspend|resume() are
handled within genpd, but the per device PM clock list still needs to
be created from SOC specific code. It seems reasonable to have gendp to
handle that as well, but that left to future patches to address.
It's not every users of genpd that are keen on using PM clocks thus we
need to provide this a configuration option for genpd. Therefore let's
add flag field in the genpd struct to keep this information and define
a new PM_DOMAIN_PM_CLK bit can then be set at initialization.
Signed-off-by: Ulf Hansson ulf.hans...@linaro.org
---
drivers/base/power/domain.c | 7 +++
include/linux/pm_domain.h | 3 +++
2 files changed, 10 insertions(+)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 3989eb6..42e328c 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -12,6 +12,7 @@
#include linux/pm_runtime.h
#include linux/pm_domain.h
#include linux/pm_qos.h
+#include linux/pm_clock.h
#include linux/slab.h
#include linux/err.h
#include linux/sched.h
@@ -1948,6 +1949,12 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
genpd-domain.ops.complete = pm_genpd_complete;
genpd-dev_ops.save_state = pm_genpd_default_save_state;
genpd-dev_ops.restore_state = pm_genpd_default_restore_state;
+
+ if (genpd-flags PM_DOMAIN_PM_CLK) {
+ genpd-dev_ops.stop = pm_clk_suspend;
+ genpd-dev_ops.start = pm_clk_suspend;
The 2nd one is wrong.
+ }
+
mutex_lock(gpd_list_lock);
list_add(genpd-gpd_list_node, gpd_list);
mutex_unlock(gpd_list_lock);
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 9d254e2..44c6931 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -14,6 +14,7 @@
#include linux/pm.h
#include linux/err.h
#include linux/of.h
+#include linux/bitops.h
#include linux/notifier.h
#include linux/cpuidle.h
@@ -76,6 +77,8 @@ struct generic_pm_domain {
struct device *dev);
void (*detach_dev)(struct generic_pm_domain *domain,
struct device *dev);
+ unsigned int flags; /* Bit field of configs for genpd */
+#define PM_DOMAIN_PM_CLK BIT(0) /* PM domain use PM clk */
s/use/uses ?
Are you planning on adding a separate flag for collecting clocks from OF
on attach/detach?
};
static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
--
1.9.1
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Power on the PM domain right after attach completes
On Tue, Nov 18, 2014 at 11:13:28AM -0500, Alan Stern wrote: On Mon, 17 Nov 2014, Dmitry Torokhov wrote: However, is it allowed to call pm_runtime_get_sync() on devices that didn't issue pm_runtime_enable()? Yes. But the bus has to issue pm_runtime_enable() before probing the driver, because the driver will expect runtime PM to work properly while its probe routine runs. For example, the probe routine might want to leave the device in a runtime-suspended state. It can't do that if the device isn't enabled for runtime PM. That means that runtime PM will be enabled for all devices on given bus while up till now drivers were deciding if their devices should be runtime-pm-managed or not. I do not think we are quite ready for this. It's up to both the bus _and_ the driver to make this decision. If a driver is completely runtime-PM-unaware then it will never decrement the device's usage counter (which was incremented when the bus called _get_noresume()), and therefore the device will never be runtime-suspended. OK. Another question then: pm_runtime_get_noresume() does literally this: atomic_inc(dev-power.usage_count); So who is responsible for actually waking up parent device and/or power domain? Is it simply missing because we did not really have PM domains before? Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Power on the PM domain right after attach completes
On Tue, Nov 18, 2014 at 12:44:22PM -0500, Alan Stern wrote: On Tue, 18 Nov 2014, Dmitry Torokhov wrote: OK. Another question then: pm_runtime_get_noresume() does literally this: atomic_inc(dev-power.usage_count); So who is responsible for actually waking up parent device and/or power domain? Is it simply missing because we did not really have PM domains before? Ths bus is responsible for making sure that all the standard resources are available -- that is, all the resources that would be needed by a normal device on that bus. Anything beyond that (such as special-purpose clocks) has to be set up by the driver. Thus the bus would insure that the device was powered, its parent was resumed, and the usual clock inputs were enabled. And of course, one mechanism for doing this is to runtime-resume the power domain. This does not sound like anything bus-specific. Can we move powering on the domain before probing into the driver core, similarly to the default pin selection by pinctrl? I do not see why we want to have every individual bus to implement this. I guess right now we can't because we assign the domain to device in individual bus's probe function, but I do not think this is proper place for that either: bus and pm domain are orthogonal concepts IMO. Often the bus doesn't have to do anything special to resume the device's parent. This is because the device gets probed when it is registered, which happens when it is discovered, and the discovery is done by the parent's driver as part of its normal activity. Since the parent has to be powered up to carry out this normal activity, nothing more is needed. I think this only true for buses that support discovery. (Of course, devices can get probed at other times too, not just when they are discovered. For example, a device might get probed when a driver module is loaded, and that can occur at any time. So in general it might indeed be necessary for the bus to wake up the parent before calling the driver's probe routine.) Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Power on the PM domain right after attach completes
On Tue, Nov 18, 2014 at 09:14:56PM +0100, Rafael J. Wysocki wrote: On Tuesday, November 18, 2014 09:55:15 AM Dmitry Torokhov wrote: On Tue, Nov 18, 2014 at 12:44:22PM -0500, Alan Stern wrote: On Tue, 18 Nov 2014, Dmitry Torokhov wrote: OK. Another question then: pm_runtime_get_noresume() does literally this: atomic_inc(dev-power.usage_count); So who is responsible for actually waking up parent device and/or power domain? Is it simply missing because we did not really have PM domains before? Ths bus is responsible for making sure that all the standard resources are available -- that is, all the resources that would be needed by a normal device on that bus. Anything beyond that (such as special-purpose clocks) has to be set up by the driver. Thus the bus would insure that the device was powered, its parent was resumed, and the usual clock inputs were enabled. And of course, one mechanism for doing this is to runtime-resume the power domain. This does not sound like anything bus-specific. Can we move powering on the domain before probing into the driver core, similarly to the default pin selection by pinctrl? We could do that for genpd if devices were added to domains before registering (those devices). Otherwise, there's no guarantee that all has been set up yet. Note that this would only be the case for genpd, not for the ACPI PM domain in particular, for example. The reason why is that the ACPI PM domain cannot be used along with bus types that provide non-trivial PM callbacks, so pretty much the bus type's -probe needs to decide whether or not to use it. In genpd code there is a notion of providers that match devices and domains. Can we do the same for ACPI and stuff all that knowledge into it's provider? IOW why ACPI is that special? -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Power on the PM domain right after attach completes
On Tue, Nov 18, 2014 at 10:17:46PM +0100, Rafael J. Wysocki wrote: On Tuesday, November 18, 2014 10:03:18 PM Rafael J. Wysocki wrote: On Tuesday, November 18, 2014 12:04:38 PM Dmitry Torokhov wrote: On Tue, Nov 18, 2014 at 09:14:56PM +0100, Rafael J. Wysocki wrote: On Tuesday, November 18, 2014 09:55:15 AM Dmitry Torokhov wrote: On Tue, Nov 18, 2014 at 12:44:22PM -0500, Alan Stern wrote: On Tue, 18 Nov 2014, Dmitry Torokhov wrote: OK. Another question then: pm_runtime_get_noresume() does literally this: atomic_inc(dev-power.usage_count); So who is responsible for actually waking up parent device and/or power domain? Is it simply missing because we did not really have PM domains before? Ths bus is responsible for making sure that all the standard resources are available -- that is, all the resources that would be needed by a normal device on that bus. Anything beyond that (such as special-purpose clocks) has to be set up by the driver. Thus the bus would insure that the device was powered, its parent was resumed, and the usual clock inputs were enabled. And of course, one mechanism for doing this is to runtime-resume the power domain. This does not sound like anything bus-specific. Can we move powering on the domain before probing into the driver core, similarly to the default pin selection by pinctrl? We could do that for genpd if devices were added to domains before registering (those devices). Otherwise, there's no guarantee that all has been set up yet. Note that this would only be the case for genpd, not for the ACPI PM domain in particular, for example. The reason why is that the ACPI PM domain cannot be used along with bus types that provide non-trivial PM callbacks, so pretty much the bus type's -probe needs to decide whether or not to use it. In genpd code there is a notion of providers that match devices and domains. Can we do the same for ACPI and stuff all that knowledge into it's provider? It is in ACPI like that too, but not in the form of the ACPI PM domain. IOW why ACPI is that special? The ACPI PM domain is there specifically for bus types that don't provide non-trivial PM callbacks to avoid duplication of code (if it didn't exist, all of the bus types in question would need to provide callbacks with optional ACPI handling in them). That's all about it. And there are bus types that provide non-trivial PM callbacks *and* use ACPI in them, like PCI, and that is more interleaved with the native PM in there. For those bus types we can't add devices to the ACPI PM domain just because they have ACPI companion objects. I'm not really sure why it is important here, though. We're talking about genpd, aren't we? I just wanted to indicate that the PM domains concept is not only about handling power domains and not all of its use cases can be shoehorned into the same scheme. And by the way, things worked just fine for the ACPI PM domain before commit 46420dd73b80 (PM / Domains: Add APIs to attach/detach a PM domain for a device) which put the ACPI PM domain and genpd into one bag, which was a mistake, because they are different things. Can we maybe settle on the naming then so that we do not mix them up in the future? For me PM domain is group of devices that share certain power constraints so that they have to be powered up and down together. Is this definition is not correct (for genpd at least)? And what is the proper definition for ACPI PM domain? Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Power on the PM domain right after attach completes
On Tue, Nov 18, 2014 at 10:58:17PM +0100, Rafael J. Wysocki wrote: On Tuesday, November 18, 2014 01:02:29 PM Dmitry Torokhov wrote: On Tue, Nov 18, 2014 at 10:17:46PM +0100, Rafael J. Wysocki wrote: On Tuesday, November 18, 2014 10:03:18 PM Rafael J. Wysocki wrote: On Tuesday, November 18, 2014 12:04:38 PM Dmitry Torokhov wrote: On Tue, Nov 18, 2014 at 09:14:56PM +0100, Rafael J. Wysocki wrote: On Tuesday, November 18, 2014 09:55:15 AM Dmitry Torokhov wrote: On Tue, Nov 18, 2014 at 12:44:22PM -0500, Alan Stern wrote: On Tue, 18 Nov 2014, Dmitry Torokhov wrote: OK. Another question then: pm_runtime_get_noresume() does literally this: atomic_inc(dev-power.usage_count); So who is responsible for actually waking up parent device and/or power domain? Is it simply missing because we did not really have PM domains before? Ths bus is responsible for making sure that all the standard resources are available -- that is, all the resources that would be needed by a normal device on that bus. Anything beyond that (such as special-purpose clocks) has to be set up by the driver. Thus the bus would insure that the device was powered, its parent was resumed, and the usual clock inputs were enabled. And of course, one mechanism for doing this is to runtime-resume the power domain. This does not sound like anything bus-specific. Can we move powering on the domain before probing into the driver core, similarly to the default pin selection by pinctrl? We could do that for genpd if devices were added to domains before registering (those devices). Otherwise, there's no guarantee that all has been set up yet. Note that this would only be the case for genpd, not for the ACPI PM domain in particular, for example. The reason why is that the ACPI PM domain cannot be used along with bus types that provide non-trivial PM callbacks, so pretty much the bus type's -probe needs to decide whether or not to use it. In genpd code there is a notion of providers that match devices and domains. Can we do the same for ACPI and stuff all that knowledge into it's provider? It is in ACPI like that too, but not in the form of the ACPI PM domain. IOW why ACPI is that special? The ACPI PM domain is there specifically for bus types that don't provide non-trivial PM callbacks to avoid duplication of code (if it didn't exist, all of the bus types in question would need to provide callbacks with optional ACPI handling in them). That's all about it. And there are bus types that provide non-trivial PM callbacks *and* use ACPI in them, like PCI, and that is more interleaved with the native PM in there. For those bus types we can't add devices to the ACPI PM domain just because they have ACPI companion objects. I'm not really sure why it is important here, though. We're talking about genpd, aren't we? I just wanted to indicate that the PM domains concept is not only about handling power domains and not all of its use cases can be shoehorned into the same scheme. And by the way, things worked just fine for the ACPI PM domain before commit 46420dd73b80 (PM / Domains: Add APIs to attach/detach a PM domain for a device) which put the ACPI PM domain and genpd into one bag, which was a mistake, because they are different things. Can we maybe settle on the naming then so that we do not mix them up in the future? For me PM domain is group of devices that share certain power constraints so that they have to be powered up and down together. Is this definition is not correct (for genpd at least)? It is correct for genpd, it isn't correct for the ACPI PM domain. And what is the proper definition for ACPI PM domain? I agree that the terminology is (somewhat?) confusing. From the code perspective, using a PM domain object is a way to provide PM callbacks that will be executed for a subset of devices instead of or in addition to the bus type (or class or device type) callbacks. Of course, that applies to proper power domains in particular, but it can also apply to broader sets of devices. In the ACPI PM domain case this covers devices with ACPI power management support (or more precisely, devices with ACPI companion objects that can provide PM support). In this context the word domain means as much as area of control (which is a proper dictionary definition of it AFAICS). genpd is all about proper power domains, like you said. OK, thank you for explaining this. Up until now I
Re: [PATCH] PM / Domains: Power on the PM domain right after attach completes
On Mon, Nov 17, 2014 at 11:06 AM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 17 Nov 2014, Kevin Hilman wrote: Ulf Hansson ulf.hans...@linaro.org writes: The amba bus, amba drivers and a vast amount of platform drivers which enables runtime PM, don't invoke a pm_runtime_get_sync() while probing their devices. Instead, once they have turned on their PM resourses during -probe() and are ready to handle I/O, these invokes pm_runtime_set_active() to synchronize its state towards the runtime PM core. From a runtime PM point of view this behavior is perfectly acceptable, In the context of PM domains that can be dynamically powered on/off, I'm not so sure it's perfectly acceptable anymore. Why doesn't the bus do a _get_sync() instead of a _get_noresume() followed by a _set_active(). By using the _get_noresume() you're bypassing the paths that would bring up your PM domain. This probably comes about because devices that are part of a power domain need special treatment. Before the driver's probe routine gets called, the bus ought to resume the entire power domain. For that, some sort of _get_sync() is needed. For devices that aren't part of a power domain, things are simpler. The bus does _get_noresume() to make sure the device won't be runtime suspended while the probe routine is running. It doesn't do _get_sync(), because that would end up calling the driver's runtime_resume routine before the driver was bound to the device. (The bus could prevent that from happening by taking special precautions, like PCI does, but in general it's a nuisance.) That's why I think we need some new call that would mean make sure the device is powered which would properly handle power domain and bus, but ignore all driver stuff since it may not be initialized yet. And similar call for asking to put device and maybe domain in powered down state in case probing failed. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Power on the PM domain right after attach completes
On Mon, Nov 17, 2014 at 02:54:53PM -0500, Alan Stern wrote: On Mon, 17 Nov 2014, Dmitry Torokhov wrote: For devices that aren't part of a power domain, things are simpler. The bus does _get_noresume() to make sure the device won't be runtime suspended while the probe routine is running. It doesn't do _get_sync(), because that would end up calling the driver's runtime_resume routine before the driver was bound to the device. (The bus could prevent that from happening by taking special precautions, like PCI does, but in general it's a nuisance.) That's why I think we need some new call that would mean make sure the device is powered which would properly handle power domain and bus, but ignore all driver stuff since it may not be initialized yet. And similar call for asking to put device and maybe domain in powered down state in case probing failed. I can't imagine how such a call would work. The PM core invokes the subsystem's runtime_suspend/resume callback, and then the subsystem's routine is responsible for invoking the driver's callback (or _not_ invoking it, in this case). Thus, the PM core has no way to tell the subsystem's callback not to invoke the driver's routine, and adding a new runtime PM call wouldn't change that. You'd have to add a new pair of callbacks instead, which IMO would be a tremendous waste. Furthermore, the subsystem already _knows_ when the driver gets probed, because probing works in the same sort of way: The subsystem's probe routine gets invoked, and it is responsible for invoking the driver's probe routine. Therefore the PM core doesn't _need_ to provide this extra information to the subsystem. Rather, the subsystem just needs to keep track of the information it already has available. You are missing concept of power domains in this picture. True, subsystem knows when it probes but power domain does not. Subsystem has no knowledge of power domain (devices in the same subsystem can come from different domains). We need to have either subsystem or device core to indicate to power management core that we do not need full runtime resume, but rather a partial one since driver is not ready yet. We would not need new callbacks here I think, we just need to be able to select appropriate set of callbacks, depending on the binding state. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Power on the PM domain right after attach completes
On Mon, Nov 17, 2014 at 03:49:14PM -0500, Alan Stern wrote:
On Mon, 17 Nov 2014, Dmitry Torokhov wrote:
On Mon, Nov 17, 2014 at 02:54:53PM -0500, Alan Stern wrote:
On Mon, 17 Nov 2014, Dmitry Torokhov wrote:
For devices that aren't part of a power domain, things are simpler.
The bus does _get_noresume() to make sure the device won't be runtime
suspended while the probe routine is running. It doesn't do
_get_sync(), because that would end up calling the driver's
runtime_resume routine before the driver was bound to the device.
(The
bus could prevent that from happening by taking special precautions,
like PCI does, but in general it's a nuisance.)
That's why I think we need some new call that would mean make sure the
device is powered which would properly handle power domain and bus, but
ignore all driver stuff since it may not be initialized yet. And similar
call for asking to put device and maybe domain in powered down state in
case probing failed.
I can't imagine how such a call would work.
The PM core invokes the subsystem's runtime_suspend/resume callback,
and then the subsystem's routine is responsible for invoking the
driver's callback (or _not_ invoking it, in this case).
Thus, the PM core has no way to tell the subsystem's callback not to
invoke the driver's routine, and adding a new runtime PM call wouldn't
change that. You'd have to add a new pair of callbacks instead, which
IMO would be a tremendous waste.
Furthermore, the subsystem already _knows_ when the driver gets probed,
because probing works in the same sort of way: The subsystem's probe
routine gets invoked, and it is responsible for invoking the driver's
probe routine. Therefore the PM core doesn't _need_ to provide this
extra information to the subsystem. Rather, the subsystem just needs
to keep track of the information it already has available.
You are missing concept of power domains in this picture. True,
subsystem knows when it probes but power domain does not. Subsystem has
no knowledge of power domain (devices in the same subsystem can come
from different domains).
Okay, I take your point.
We need to have either subsystem or device core to indicate to power
management core that we do not need full runtime resume, but rather a
partial one since driver is not ready yet.
We would not need new callbacks here I think, we just need to be able to
select appropriate set of callbacks, depending on the binding state.
When the runtime PM core invokes a power domain's callback routine,
what does the domain's routine usually do? Does it go ahead and invoke
the driver's callback? Or does it try to invoke the subsystem's
callback?
Obviously this depends on how the power domain code is written. But
suppose every power domain would always use the same strategy as the PM
core: Invoke the subsystem's callback if there is one; otherwise invoke
the driver's callback.
Then there wouldn't be a problem. Even when a runtime-resume went via
the power domain, the subsystem would still be able to protect the
not-yet-bound driver from being called.
(... Unless the subsystem itself was incapable of doing this the right
way. But subsystems can be fixed.)
The genpd code currently starts by powering on the domain (if it is not
on already) and then does device restore which is:
/**
* pm_genpd_default_restore_state - Default PM domians restore device state.
* @dev: Device to handle.
*/
static int pm_genpd_default_restore_state(struct device *dev)
{
int (*cb)(struct device *__dev);
cb = dev_gpd_data(dev)-ops.restore_state;
if (cb)
return cb(dev);
if (dev-type dev-type-pm)
cb = dev-type-pm-runtime_resume;
else if (dev-class dev-class-pm)
cb = dev-class-pm-runtime_resume;
else if (dev-bus dev-bus-pm)
cb = dev-bus-pm-runtime_resume;
else
cb = NULL;
if (!cb dev-driver dev-driver-pm)
cb = dev-driver-pm-runtime_resume;
return cb ? cb(dev) : 0;
}
So I guess bus (or class or type) can take care of it. Except buses
usually call pm_generic_runtime_resume() which ends up fetching driver's
callbacks. Maybe pm_generic_runtime_*() need be a bit smarter?
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Power on the PM domain right after attach completes
On Mon, Nov 17, 2014 at 04:44:56PM -0500, Alan Stern wrote:
On Mon, 17 Nov 2014, Dmitry Torokhov wrote:
When the runtime PM core invokes a power domain's callback routine,
what does the domain's routine usually do? Does it go ahead and invoke
the driver's callback? Or does it try to invoke the subsystem's
callback?
Obviously this depends on how the power domain code is written. But
suppose every power domain would always use the same strategy as the PM
core: Invoke the subsystem's callback if there is one; otherwise invoke
the driver's callback.
Then there wouldn't be a problem. Even when a runtime-resume went via
the power domain, the subsystem would still be able to protect the
not-yet-bound driver from being called.
(... Unless the subsystem itself was incapable of doing this the right
way. But subsystems can be fixed.)
The genpd code currently starts by powering on the domain (if it is not
on already) and then does device restore which is:
/**
* pm_genpd_default_restore_state - Default PM domians restore device
state.
* @dev: Device to handle.
*/
static int pm_genpd_default_restore_state(struct device *dev)
{
int (*cb)(struct device *__dev);
cb = dev_gpd_data(dev)-ops.restore_state;
if (cb)
return cb(dev);
if (dev-type dev-type-pm)
cb = dev-type-pm-runtime_resume;
else if (dev-class dev-class-pm)
cb = dev-class-pm-runtime_resume;
else if (dev-bus dev-bus-pm)
cb = dev-bus-pm-runtime_resume;
else
cb = NULL;
if (!cb dev-driver dev-driver-pm)
cb = dev-driver-pm-runtime_resume;
return cb ? cb(dev) : 0;
}
So I guess bus (or class or type) can take care of it.
The bus could. I don't think the class or type knows when a driver is
being probed.
Except buses
usually call pm_generic_runtime_resume() which ends up fetching driver's
callbacks. Maybe pm_generic_runtime_*() need be a bit smarter?
No, the bus subsystem needs to be smarter. It shouldn't call
pm_generic_runtime_resume() if the driver hasn't been probed yet, or if
the driver has already been unbound from the device.
But that code wold be exactly the same for all buses, right? So why
can't pm_generic_runtime_resume() be smarter?
However, is it allowed to call pm_runtime_get_sync() on devices that
didn't issue pm_runtime_enable()?
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Power on the PM domain right after attach completes
On Mon, Nov 17, 2014 at 05:12:35PM -0500, Alan Stern wrote: On Mon, 17 Nov 2014, Dmitry Torokhov wrote: Except buses usually call pm_generic_runtime_resume() which ends up fetching driver's callbacks. Maybe pm_generic_runtime_*() need be a bit smarter? No, the bus subsystem needs to be smarter. It shouldn't call pm_generic_runtime_resume() if the driver hasn't been probed yet, or if the driver has already been unbound from the device. But that code wold be exactly the same for all buses, right? So why can't pm_generic_runtime_resume() be smarter? It would not be the same for all buses. Each bus will have its own way of recognizing whether or not a driver has been probed (i.e., by checking some field in the bus-specific part of the device structure). However, is it allowed to call pm_runtime_get_sync() on devices that didn't issue pm_runtime_enable()? Yes. But the bus has to issue pm_runtime_enable() before probing the driver, because the driver will expect runtime PM to work properly while its probe routine runs. For example, the probe routine might want to leave the device in a runtime-suspended state. It can't do that if the device isn't enabled for runtime PM. That means that runtime PM will be enabled for all devices on given bus while up till now drivers were deciding if their devices should be runtime-pm-managed or not. I do not think we are quite ready for this. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Power on the PM domain right after attach completes
On Tue, Nov 18, 2014 at 12:28:26AM +0100, Rafael J. Wysocki wrote: On Monday, November 17, 2014 02:17:00 PM Dmitry Torokhov wrote: On Mon, Nov 17, 2014 at 05:12:35PM -0500, Alan Stern wrote: On Mon, 17 Nov 2014, Dmitry Torokhov wrote: Except buses usually call pm_generic_runtime_resume() which ends up fetching driver's callbacks. Maybe pm_generic_runtime_*() need be a bit smarter? No, the bus subsystem needs to be smarter. It shouldn't call pm_generic_runtime_resume() if the driver hasn't been probed yet, or if the driver has already been unbound from the device. But that code wold be exactly the same for all buses, right? So why can't pm_generic_runtime_resume() be smarter? It would not be the same for all buses. Each bus will have its own way of recognizing whether or not a driver has been probed (i.e., by checking some field in the bus-specific part of the device structure). However, is it allowed to call pm_runtime_get_sync() on devices that didn't issue pm_runtime_enable()? Yes. But the bus has to issue pm_runtime_enable() before probing the driver, because the driver will expect runtime PM to work properly while its probe routine runs. For example, the probe routine might want to leave the device in a runtime-suspended state. It can't do that if the device isn't enabled for runtime PM. That means that runtime PM will be enabled for all devices on given bus while up till now drivers were deciding if their devices should be runtime-pm-managed or not. That's not the case for PCI drivers. I do not think we are quite ready for this. We have to do that if power domains are in use, however, because if at least one device in a power domain in enabled for runtime PM, that will affect the other devices in that domain. We could make a rule to keep a domail always up if at least one device in it has runtime PM disabled, but that is equivalent to enabling runtime PM for that device, powering the domain up and bumping up the device's usage counter. What will driver will see if it tries to check pm_runtime_active()? Would not it get unexpected result if the driver did not call pm_runtime_enable() on it's device? -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] exynos4-is: fix error handling of irq_of_parse_and_map
Return value of irq_of_parse_and_map() is unsigned int, with 0
indicating failure, so testing for negative result never works.
Signed-off-by: Dmitry Torokhov d...@chromium.org
---
Not tested, found by casual code inspection.
drivers/media/platform/exynos4-is/fimc-is.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/exynos4-is/fimc-is.c
b/drivers/media/platform/exynos4-is/fimc-is.c
index 94c6b47..0fdca86 100644
--- a/drivers/media/platform/exynos4-is/fimc-is.c
+++ b/drivers/media/platform/exynos4-is/fimc-is.c
@@ -814,9 +814,9 @@ static int fimc_is_probe(struct platform_device *pdev)
return -ENOMEM;
is-irq = irq_of_parse_and_map(dev-of_node, 0);
- if (is-irq 0) {
+ if (!is-irq) {
dev_err(dev, no irq found\n);
- return is-irq;
+ return -EINVAL;
}
ret = fimc_is_get_clocks(is);
--
2.1.0.rc2.206.gedb03e5
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Fix initial default state of the need_restore flag
On Mon, Nov 10, 2014 at 04:18:50PM +0100, Ulf Hansson wrote: [...] I guess we do need it for 3.18, but when we are talking about 3.19, before we make any more changes can we outline how power domains are supposed to work? 1. How do we attach a device to power domain? Right now it seems that individual buses are responsible for attaching their devices to power domains. Should we move it into driver core (device_pm_add?) so that device starts its life in its proper power domain? That was the initial approach. To my understanding, Rafael's primary reason for not accepting that was that it's not common, but it's platform-specific. http://marc.info/?l=linux-pmm=140243462304669w=2 I am not sure I agree with Rafael there: - when we are talking about the latest incarnation of power domains it is not really platform specific anymore (as it was when we were dealing with ACPI only case); - I do not see why sirincling platform specific code around i2c, spi, etc bus code (which is not platform-specific) is OK, but a no-no for the driver ocre. Now, even if we would reconsider doing as you propose, what would the actual benefit be? Obviously, we would get less amount of code to maintain, which is one reason, but are there more? I think so - you would have complete picture of your power domain, including data exposed in debugfs, etc. 2. When do we power up the devices (and the domains)? Right now devices in ACPI power domain are powered when they are attached to the power domain (which coincides with probing), but generic power domains do not do that. Can we add a separate API to explicitly power up the device (and its domain if it is powered down) and do it again, either in device core or in individual buses. This way, regardless of runtime PM or not, we will have devices powered on when driver tries to bind to them. If binding fails we can power down the device. Isn't that exactly what I implemented in [1], what am I missing? Hm, OK. I guess the title of the patch series thrown me off because as far as I am concerned it is not a race, we simply were not powering the domain properly for probing. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Fix initial default state of the need_restore flag
On November 10, 2014 11:39:31 AM PST, Mark Brown broo...@kernel.org wrote: On Mon, Nov 10, 2014 at 10:32:24AM -0800, Dmitry Torokhov wrote: - I do not see why sirincling platform specific code around i2c, spi, etc bus code (which is not platform-specific) is OK, but a no-no for the driver ocre. sirincling? Sorry, I was going for sprinkling but failed spectacularly. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Fix initial default state of the need_restore flag
On Fri, Nov 07, 2014 at 11:47:53AM -0800, Kevin Hilman wrote: Ulf Hansson ulf.hans...@linaro.org writes: The initial state of the device's need_restore flag should'nt depend on the current state of the PM domain. For example it should be perfectly valid to attach an inactive device to a powered PM domain. The pm_genpd_dev_need_restore() API allow us to update the need_restore flag to somewhat cope with such scenarios. Typically that should have been done from drivers/buses -probe() since it's those that put the requirements on the value of the need_restore flag. Until recently, the Exynos SOCs were the only user of the pm_genpd_dev_need_restore() API, though invoking it from a centralized location while adding devices to their PM domains. Due to that Exynos now have swithed to the generic OF-based PM domain look-up, it's no longer possible to invoke the API from a centralized location. The reason is because devices are now added to their PM domains during the probe sequence. Commit ARM: exynos: Move to generic PM domain DT bindings did the switch for Exynos to the generic OF-based PM domain look-up, but it also removed the call to pm_genpd_dev_need_restore(). This caused a regression for some of the Exynos drivers. To handle things more properly in the generic PM domain, let's change the default initial value of the need_restore flag to reflect that the state is unknown. As soon as some of the runtime PM callbacks gets invoked, update the initial value accordingly. Moreover, since the generic PM domain is verifying that all device's are both runtime PM enabled and suspended, using pm_runtime_suspended() while pm_genpd_poweroff() is invoked from the scheduled work, we can be sure of that the PM domain won't be powering off while having active devices. Do note that, the generic PM domain can still only know about active devices which has been activated through invoking its runtime PM resume callback. In other words, buses/drivers using pm_runtime_set_active() during -probe() will still suffer from a race condition, potentially probing a device without having its PM domain being powered. That issue will have to be solved using a different approach. This a log from the boot regression for Exynos5, which is being fixed in this patch. [ cut here ] WARNING: CPU: 0 PID: 308 at ../drivers/clk/clk.c:851 clk_disable+0x24/0x30() Modules linked in: CPU: 0 PID: 308 Comm: kworker/0:1 Not tainted 3.18.0-rc3-00569-gbd9449f-dirty #10 Workqueue: pm pm_runtime_work [c0013c64] (unwind_backtrace) from [c0010dec] (show_stack+0x10/0x14) [c0010dec] (show_stack) from [c03ee4cc] (dump_stack+0x70/0xbc) [c03ee4cc] (dump_stack) from [c0020d34] (warn_slowpath_common+0x64/0x88) [c0020d34] (warn_slowpath_common) from [c0020d74] (warn_slowpath_null+0x1c/0x24) [c0020d74] (warn_slowpath_null) from [c03107b0] (clk_disable+0x24/0x30) [c03107b0] (clk_disable) from [c02cc834] (gsc_runtime_suspend+0x128/0x160) [c02cc834] (gsc_runtime_suspend) from [c0249024] (pm_generic_runtime_suspend+0x2c/0x38) [c0249024] (pm_generic_runtime_suspend) from [c024f44c] (pm_genpd_default_save_state+0x2c/0x8c) [c024f44c] (pm_genpd_default_save_state) from [c024ff2c] (pm_genpd_poweroff+0x224/0x3ec) [c024ff2c] (pm_genpd_poweroff) from [c02501b4] (pm_genpd_runtime_suspend+0x9c/0xcc) [c02501b4] (pm_genpd_runtime_suspend) from [c024a4f8] (__rpm_callback+0x2c/0x60) [c024a4f8] (__rpm_callback) from [c024a54c] (rpm_callback+0x20/0x74) [c024a54c] (rpm_callback) from [c024a930] (rpm_suspend+0xd4/0x43c) [c024a930] (rpm_suspend) from [c024bbcc] (pm_runtime_work+0x80/0x90) [c024bbcc] (pm_runtime_work) from [c0032a9c] (process_one_work+0x12c/0x314) [c0032a9c] (process_one_work) from [c0032cf4] (worker_thread+0x3c/0x4b0) [c0032cf4] (worker_thread) from [c003747c] (kthread+0xcc/0xe8) [c003747c] (kthread) from [c000e738] (ret_from_fork+0x14/0x3c) ---[ end trace 40cd58bcd6988f12 ]--- Fixes: a4a8c2c4962bb655 (ARM: exynos: Move to generic PM domain DT bindings) Reported-by: Sylwester Nawrocki s.nawro...@samsung.com Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Reviewed-by: Kevin Hilman khil...@linaro.org And looks like we need this for v3.18-rc since it does fix the regression. I guess we do need it for 3.18, but when we are talking about 3.19, before we make any more changes can we outline how power domains are supposed to work? 1. How do we attach a device to power domain? Right now it seems that individual buses are responsible for attaching their devices to power domains. Should we move it into driver core (device_pm_add?) so that device starts its life in its proper power domain? 2. When do we power up the devices (and the domains)? Right now devices in ACPI power domain are powered when they are attached to the power domain (which coincides with probing), but generic
Re: [PATCH 03/21] thermal: of: Extend of-thermal.c to provide number of non critical trip points
Hi Lukasz,
On Fri, Nov 7, 2014 at 2:05 AM, Lukasz Majewski l.majew...@samsung.com wrote:
Hi Eduardo,
On Thu, Oct 09, 2014 at 06:38:39PM +0200, Lukasz Majewski wrote:
This patch extends the of-thermal.c to provide information about
number of available non critical (i.e. non HW) trip points in the
system.
Signed-off-by: Lukasz Majewski l.majew...@samsung.com
---
drivers/thermal/of-thermal.c | 12
drivers/thermal/thermal_core.h | 5 +
2 files changed, 17 insertions(+)
diff --git a/drivers/thermal/of-thermal.c
b/drivers/thermal/of-thermal.c index 23c8d6c..cd74e64 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -128,6 +128,18 @@ int of_thermal_is_trip_en(struct
thermal_zone_device *tz, int trip) return 1;
}
+int of_thermal_get_non_crit_ntrips(struct thermal_zone_device *tz)
+{
+ struct __thermal_zone *data = tz-devdata;
+ int i;
+
+ for (i = 0; i data-ntrips; i++)
+ if (data-trips[i].type != THERMAL_TRIP_CRITICAL)
+ continue;
+
+ return --i;
+}
+
I am not against this addition. But looks like we start to spread some
specific APIs that may not be used to other drivers.
Why do you think that this is a specific API? In the thermal OF we can
define trip point as active, passive, hot and critical.
With the first three we can handle them and properly react. For the last
one SoC's PMU will power down the board.
Do you know if any board (e.g. from TI) is NOT supposed to shut down
when critical temperature is passed?
The real problem here is the accessibility to __thermal_trip and
__thermal_bind arrays.
Use case:
In the Exynos driver we do need to initialize TMU registers with
threshold temperatures.
Is this indeed plural or you want to program the next trip point as your
alarm temperature? If so, there was another patch floating around adding
set_trips() callback that would get passed in the low and high trip points for
the current temperature reading and you can use that data to program
alarms. Rockchip thermal driver uses it.
https://lkml.org/lkml/2014/6/27/76
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] PM / Domains: Fix race conditions during boot
On Tue, Nov 04, 2014 at 09:54:19AM +0100, Ulf Hansson wrote: [...] Generally, there are two or even three levels of runtime PM handling, driver, (possibly) bus type and (possibly) PM domain (and multiple levels of these are possible in principle). All of them have to be initialized at different times. Quite arguably, the PM domain and/or bus type runtime PM handling should be initialized even before registerind the device or during device registration. Doing that later may be too late. When the device has been registered, runtime PM should work to an extent allowing the driver to access the device and configure it further after calling pm_runtime_resume(). Of course, if -probe() is to call pm_runtime_resume() for this purpose, it must take the fact that the driver's own -runtime_resume() may be called as a result of this into account. That's why I'm asking whether or not the core should call pm_runtime_resume() before calling really_probe() in a followup branch of this thread. I am reading the other thread, let's see. The driver's own runtime PM handling must be initialized in the driver and the only place suitable for that is -probe(). However, it needs to be done *before* the driver's own -runtime_resume() or -runtime_suspend() callback is executed. If that is done properly, it should be possible to cover both the CONFIG_PM_RUNTIME set/unset cases in that code. And I wouldn't recommend anyone to do the runtime PM initialization in -runtime_resume() (when it is called for the first time), as that would be error prone and fragile. Great! That's means we are at least aligned on this topic. :-) The AMBA bus and some of its drivers a good example of how this has been implemented: driver/amba/bus.c drivers/mmc/host/mmci.c drivers/spi/spi-pl022.c This conclusion I have made from this is: - Using pm_runtime_get_sync() during the -probe() path to explicitly power up a PM domain, is not suitable as the _common_ solution to solve the race condition. It certainly may work for some scenarios, but not for those I am looking at. I think, however, that it might work if the core calls pm_runtime_get_sync() from driver_probe_device(). Currently this won't work. That's because the buses' -probe() are invoked in this path and they are doing the attachment of the device to its PM domain. In other words, we can't power up the PM domain using pm_runtime_get_sync(), until the device has been attached to its PM domain. Right? I think this is one of the issues that we have there. Why do we conflate probing and placing the device into a power domain? The latter should happen when we register the device. The fact that a device was probed and has a driver bound or not bound to it should have no bearing on whether the device is member of power domain or not. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] PM / Domains: Fix race conditions during boot
On Tue, Nov 04, 2014 at 06:01:44PM +0100, Ulf Hansson wrote: Devices that are created while discoverable buses are being probed can't be attached to a PM domain before the probing is done, because those simply doesn't exist. Honestly, I'm not sure what you're talking about. Devices on a discoverable* bus (say PCI) are added when the *controller* is probed, not when *they* are probed. Okay, so maybe discoverable buses isn't the proper term. You very much need to have a struct device registered to be able to call really_probe() for it. Yes. But my point is that the struct device may be created dynamically at some point in time. And that is fine. This is how mmc/sd/sdio cards are handled. We don't have the information about the card and thus not the struct device of it, until we have detected it. Maybe we could at that point try to add the device to its PM domain? Well, I think we need to first define what PM domain they will fall into. Do they have to be in one? The concept of power domain, as far as I understand it, is needed when we need to form relations going outside the standard parent/child relationship. Here we have a controller and then one or more cards in it. To be able to use card you need to power up parent and you can not power down parent until all children are powered down. But in any case, device discovery and binding them to drivers are 2 separate steps. You have a PCI bus. You enumerate it - new PCI devices are created. Some of them may be put in a certain power domain. You then bind drivers to PCI devices (not strictly 'then' but we could implement driver core to postpone binding until current round of enumeration is complete) - and you discover USB controller and maybe i2c controller. Then you bind drivers to them which causes enumeration of the new bus and new devices are created. But in all these cases enumeration and creation of new 'struct device's, and driver binding are logically separate steps. Now, I haven't yet seen a demand for such a cases, but it seems wrong to not consider them. The current solution cover these. Oh dear. Please rethink this. Currently, dev_pm_domain_attach() is being invoked at the point when a SDIO card has been found. From sdio_add_func(). How would that be solved? It is probably the one place where we currently doing it correctly (form logical point of view, not implementation wise - implementation wise in sdio we add device to power domain after calling device_add() which means that probe() could have been called before we added the new device to any power domain). Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Change prototype for the -attach_dev() callback
On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote: Rafael J. Wysocki r...@rjwysocki.net writes: On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote: Convert the prototype to return and int. This is just an initial step, needed to support error handling. Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Acked-by: Kevin Hilman khil...@linaro.org This patch is intended as fix for 3.18 rc[n]. Why? There are other SOC specific patches around that adds genpd support and which implements the -attach_dev() callback. To prevent having an atomic patch during the next release cycle, let's change the prototype now instead. Further patches will add the actual error handling in genpd and these can then be reviewed and tested thoroughly. So we have no users of -attach_dev at the moment, right? Not in mainline, but there are a couple getting ready to hit -next, so we wanted to fix this before they arrive so that adding the error handling will be easier. BTW, while we are at it, can we also pass the domain itself to attach_dev() and detach_dev()? If anything it helps with debugging (you can print domain name from the callbacks). Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM / Domains: Change prototype for the -attach_dev() callback
On Wed, Nov 05, 2014 at 08:43:29AM +0100, Geert Uytterhoeven wrote: On Wed, Nov 5, 2014 at 2:33 AM, Dmitry Torokhov dmitry.torok...@gmail.com wrote: On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote: Rafael J. Wysocki r...@rjwysocki.net writes: On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote: Convert the prototype to return and int. This is just an initial step, needed to support error handling. Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Acked-by: Kevin Hilman khil...@linaro.org This patch is intended as fix for 3.18 rc[n]. Why? There are other SOC specific patches around that adds genpd support and which implements the -attach_dev() callback. To prevent having an atomic patch during the next release cycle, let's change the prototype now instead. Further patches will add the actual error handling in genpd and these can then be reviewed and tested thoroughly. So we have no users of -attach_dev at the moment, right? Not in mainline, but there are a couple getting ready to hit -next, so we wanted to fix this before they arrive so that adding the error handling will be easier. BTW, while we are at it, can we also pass the domain itself to attach_dev() and detach_dev()? If anything it helps with debugging (you can print domain name from the callbacks). You can use dev-pm_domain, which is already set. Note that this is no longer the case after Ulf's [PATCH 3/4] PM / Domains: Improve error handling while adding/removing devices! Right, but I'd rather not poke in dev structure directly if I can help it. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/9] drivercore / platform: Keep PM domain powered during -probe()
On Thu, Oct 30, 2014 at 01:47:27PM -0700, Kevin Hilman wrote: Ulf Hansson ulf.hans...@linaro.org writes: To sucessfully probe some devices their corresponding PM domains may need to be powered. Isn't that what pm_runtime_get*() is supposed to be doing? Why isn't that working? Also, I do not understand why we placing device into a power domain only when we probe it. Why if I unbind device from its driver (or do not have a driver for it) it disappears from its power domain? To me power domain and having driver bound to a device are 2 orthogonal concepts. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] input: cros_ec_keyb: Add of match table
On Fri, Sep 19, 2014 at 10:08:13AM +0200, Sjoerd Simons wrote:
To enable the cros_ec_keyb driver to be auto-loaded when build as
module add an of match table (and export it) to match the modalias
information passed on to userspace as the Cros EC MFD driver registers
the MFD subdevices with an of_compatibility string.
Signed-off-by: Sjoerd Simons sjoerd.sim...@collabora.co.uk
Reviewed-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
Applied, thank you.
---
Changes in v2: Fixed some indentation issues
drivers/input/keyboard/cros_ec_keyb.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/input/keyboard/cros_ec_keyb.c
b/drivers/input/keyboard/cros_ec_keyb.c
index 791781a..e1903ec 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -342,10 +342,19 @@ static int cros_ec_keyb_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(cros_ec_keyb_pm_ops, NULL, cros_ec_keyb_resume);
+#ifdef CONFIG_OF
+static const struct of_device_id cros_ec_keyb_of_match[] = {
+ { .compatible = google,cros-ec-keyb },
+ {},
+};
+MODULE_DEVICE_TABLE(of, cros_ec_keyb_of_match);
+#endif
+
static struct platform_driver cros_ec_keyb_driver = {
.probe = cros_ec_keyb_probe,
.driver = {
.name = cros-ec-keyb,
+ .of_match_table = of_match_ptr(cros_ec_keyb_of_match),
.pm = cros_ec_keyb_pm_ops,
},
};
--
2.1.0
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Input: atmel_mxt_ts - fix merge in DT documentation
On Thu, Sep 11, 2014 at 03:52:46PM +0100, Nick Dyer wrote: On 15/08/14 17:13, Stephen Warren wrote: Any comments on this? I would really appreciate if you can expand on how this DT property is supposed to be used so I can re-spin the atmel support patch for Peach boards. The below patch improves the documentation for the gpio-property. That patch makes sense, and is a nice description, Acked-by: Stephen Warren swar...@nvidia.com Hi Dmitry- Something went a bit wrong in merging f5940231a - there's a bit of repeated text that's been introduced. Signed-off-by: Nick Dyer nick.d...@itdev.co.uk Hmm, not sure how I messed this up, but I will queue the patch for the next push. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/3] ARM: dts: Add Peach Pit dts entry for Atmel touchpad
On Wed, Sep 10, 2014 at 01:31:29PM +0200, Javier Martinez Canillas wrote:
From: Sjoerd Simons sjoerd.sim...@collabora.co.uk
The Peach Pit board has an Atmel maXTouch trackpad device.
Add the needed Device Tree nodes to support it.
This Device Tree change is based on the Chrome OS 3.8 tree
but adapted to use the mainline Atmel maXTouch DT binding.
Signed-off-by: Sjoerd Simons sjoerd.sim...@collabora.co.uk
Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk
Reviewed-by: Dmitry Torokhov dmitry.torok...@gmail.com
---
Changes since v3:
- Use KEY_RESERVED even for not reserved pins with a GPIO not hooked.
Suggested by Nick Dyer.
- Add a comment to specify the maXTouch chip version so is more clear.
Suggested by Nick Dyer.
- Omit trailing omit KEY_RESERVED after the GPIO pins. Suggested by Nick
Dyer.
Changes since v2:
- Add spaces around '=' on properties. Suggested by Andreas Faerber.
Changes since v1:
- Change trackpad IRQ pad function from 0x0 (GPIO input) to 0xf (GPIO IRQ).
suggested by Tomasz Figa.
- Remove BTN_TOOL_* from linux,gpio-keymap property since those are set
by input mt core if INPUT_MT_POINTER is set. Suggested by Nick Dyer.
- Use correct values for linux,gpio-keymap property. Suggested by Nick
Dyer.
- Remove support for Peach Pi board since it uses a different Atmel touchpad
that requires an Atmel object protocol (T100) not supported by the driver.
- Use IRQ type constants from dt-bindings/interrupt-controller/irq.h
instead
of magic numbers. Suggested by Andreas Faerber.
arch/arm/boot/dts/exynos5420-peach-pit.dts | 29 +
1 file changed, 29 insertions(+)
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts
b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index f247709..ad56d4c 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -507,6 +507,28 @@
};
};
+hsi2c_8 {
+ status = okay;
+ clock-frequency = 333000;
+
+ /* Atmel mXT336S */
+ trackpad@4b {
+ compatible = atmel,maxtouch;
+ reg = 0x4b;
+ interrupt-parent = gpx1;
+ interrupts = 1 IRQ_TYPE_EDGE_FALLING;
+ wakeup-source;
+ pinctrl-names = default;
+ pinctrl-0 = trackpad_irq;
+ linux,gpio-keymap = KEY_RESERVED
+ KEY_RESERVED
+ KEY_RESERVED /* GPIO0 */
+ KEY_RESERVED /* GPIO1 */
+ KEY_RESERVED /* GPIO2 */
+ BTN_LEFT; /* GPIO3 */
Seems like a single space sneaked between the semicolon and the tab.
Maybe whoever applies could squash it.
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/3] ARM: dts: Add Peach Pi dts entry for Atmel touchpad
On Wed, Sep 10, 2014 at 01:31:30PM +0200, Javier Martinez Canillas wrote: The Peach Pi board has an Atmel maXTouch trackpad device. Add the needed Device Tree nodes to support it. This Device Tree change is based on the Chrome OS 3.8 tree but adapted to use the mainline Atmel maXTouch DT binding. Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk This one looks fine to me as well. Reviewed-by: Dmitry Torokhov dmitry.torok...@gmail.com Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/7] Input: cros_ec_keyb: Optimize ghosting algorithm.
On Mon, Aug 25, 2014 at 03:40:08PM +0200, Javier Martinez Canillas wrote: From: Todd Broch tbr...@chromium.org Previous algorithm was a bit conservative and complicating with respect to identifying key ghosting. This CL uses the bitops hamming weight function (hweight8) to count the number of matching rows for colM colN. If that number is 1 ghosting is present. Additionally it removes NULL keys and our one virtual keypress KEY_BATTERY from consideration as these inputs are never physical keypresses. Signed-off-by: Todd Broch tbr...@chromium.org Reviewed-by: Vincent Palatin vpala...@chromium.org Reviewed-by: Luigi Semenzato semenz...@chromium.org Tested-by: Andreas Färber afaer...@suse.de Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk Applied, thank you. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/7] Second batch of cleanups for cros_ec
On Mon, Aug 25, 2014 at 03:40:01PM +0200, Javier Martinez Canillas wrote: This is a second batch of cleanups patches for the mfd cros_ec driver and its subdevices drivers. The first batch of cleanups was posted by Doug Anderson [0] and have already been merged. The patches were picked from the ChromeOS 3.8 kernel and after these no cleanups patches for cros_ec are left, only commits that add cros ec support not yet available in mainline. This is a second version of the patch series that addresses issues pointed out by Doug Anderson and Lee Jones on v1 [1]. There is almost no functionality added on this series but the idea is to reduce the delta between the mainline drivers and the ones in the downstream Chrome OS 3.8 kernel so the missing functionality can be added on top once these cleanups patches are merged. The missing functionlity currently in mainline is: - Chrome OS Embedded Controller userspace device interface - Chrome OS Embedded Controller Low Pin Count (LPC) inteface - Access to vboot context stored on a block device - Access to vboot context stored on EC's nvram The patches in this series are authored by different people (all on cc) and consist of the following: Andrew Bresticker (3): mfd: cros_ec: stop calling -cmd_xfer() directly mfd: cros_ec: move locking into cros_ec_cmd_xfer mfd: cros_ec: wait for completion of commands that return IN_PROGRESS Derek Basehore (1): i2c: i2c-cros-ec-tunnel: Set retries to 3 Doug Anderson (1): mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC Todd Broch (2): mfd: cros_ec: Instantiate sub-devices from device tree Input: cros_ec_keyb: Optimize ghosting algorithm. drivers/i2c/busses/i2c-cros-ec-tunnel.c | 5 +- drivers/input/keyboard/cros_ec_keyb.c | 94 ++--- drivers/mfd/cros_ec.c | 78 --- drivers/mfd/cros_ec_spi.c | 20 --- include/linux/mfd/cros_ec.h | 24 ++--- 5 files changed, 141 insertions(+), 80 deletions(-) Patches #1, #2, #6 and #7 do not depend of others so they can be merged independently but patches #3, #4 and #5 have to be merged in that specific order since they depend on the previous one. #7 does not apply to my tree (I guess it depends on the 1st batch which I expect will go through MFD tree?). Maybe you could rebase it on top of my next so it can be applied sooner? Or it really needs parts of patchset #1? Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/7] Second batch of cleanups for cros_ec
On Mon, Aug 25, 2014 at 07:28:01PM +0200, Javier Martinez Canillas wrote: Hello Dmitry, On 08/25/2014 07:05 PM, Dmitry Torokhov wrote: Patches #1, #2, #6 and #7 do not depend of others so they can be merged independently but patches #3, #4 and #5 have to be merged in that specific order since they depend on the previous one. #7 does not apply to my tree (I guess it depends on the 1st batch which I expect will go through MFD tree?). Maybe you could rebase it on top of my next so it can be applied sooner? Or it really needs parts of patchset #1? The first batch sent by Doug did indeed touch this driver and the patches were merged through the MFD tree for 3.17 as you said. I see that your next branch is based on 3.16-rc6 and that is why it does not apply cleanly. I guess you will rebase your next branch for 3.18 on top of 3.17-rc1 anyways which will fix this issue? If not please let me know and I can of course re-spin the patch so it applies cleanly on top of 3.16-rc6. I normally merge with mainline around -rc3 so please ping me around that time if you do not see the patch in my tree. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] Input: atmel_mxt_ts - Get IRQ edge/level flags on DT booting
On Fri, Aug 08, 2014 at 03:07:33PM +0100, Nick Dyer wrote: On 07/08/14 08:44, Javier Martinez Canillas wrote: The Atmel maXTouch driver assumed that the IRQ type flags will always be passed using platform data but this is not true when booting using Device Trees. In these setups the interrupt type was ignored by the driver when requesting an IRQ. This means that it will fail if a machine specified other type than IRQ_TYPE_NONE. The right approach is to get the IRQ flags that was parsed by OF from the interrupt Device Tree propery. Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk I'm happy for this to go in if Dmitry will accept it. It does seem to be a quirk of some platforms that it is necessary, but it's only one line. I'd rather not as it masks the deeper platform issue. There might be other drovers also expecting platform/OF code set up interrupt triggers and working/not working by chance. Can we figure out why the platform in question needs this change? Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Input: atmel_mxt_ts - Get IRQ edge/level flags on DT booting
On Thu, Aug 07, 2014 at 09:49:49AM +0200, Javier Martinez Canillas wrote: Hello Dmitry, On 08/07/2014 08:09 AM, Dmitry Torokhov wrote: irq_of_parse_and_map() already sets up IRQ trigger type based on DT data, by calling irq_create_of_mapping() which in turn calls irq_set_irq_type(). Right but somehow when the IRQ is actually requested the type is overwritten by the value passed to request_threaded_irq() and interrupts are not being generated by the device without this patch. Do you think that this is a bug in the interrupt-parent irqchip driver or the IRQ core? I'm not that familiar with the IRQ subsystem. No, this is clearly driver fault - it smashed previously done setup with new flags. Thanks a lot for the clarification. That was my understanding as well but wanted to be sure. Actually, I take this back. In mainline everything as it should: if pdata does not specify particular trigger the flags requested end up being IRQF_ONESHOT, which should preserve trigger bits previously set up by the board or OF code. In Chrome kernel we have: /* Default to falling edge if no platform data provided */ irqflags = data-pdata ? data-pdata-irqflags : IRQF_TRIGGER_FALLING; error = request_threaded_irq(client-irq, NULL, mxt_interrupt, irqflags | IRQF_ONESHOT, client-name, data); which I believe should go away. If it is needed on ACPI systems we need to figure out how to do things we can do with OF there. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Input: s3c2410_ts: Move to clk_prepare_enable/clk_disable_unprepare
Hi Vasily, On Wed, Jul 09, 2014 at 12:13:41PM +0300, Vasily Khoruzhick wrote: On 8 July 2014 18:00:49 Dmitry Torokhov wrote: Hi Dmitry, - clk_disable(ts.clock); + clk_disable_unprepare(ts.clock); Do we really need to unprepare on suspend? Why simply disabling is not enough here? You're right, disabling should be enough here. I'll resend a patch after testing on a hardware. I ended up cutting out suspend/resume parts and applying. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] iio: exynos-adc: add experimental touchscreen support
) {
+ info-ts_x = readl(ADC_V1_DATX(info-regs));
+ info-ts_y = readl(ADC_V1_DATY(info-regs));
+ writel(ADC_TSC_WAIT4INT | ADC_S3C2443_TSC_UD_SEN,
ADC_V1_TSC(info-regs));
+ } else {
+ info-value = readl(ADC_V1_DATX(info-regs)) ADC_DATX_MASK;
+ }
/* clear irq */
if (info-data-clear_irq)
@@ -406,6 +463,46 @@ static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
return IRQ_HANDLED;
}
+/*
+ * Here we (ab)use a threaded interrupt handler to stay running
+ * for as long as the touchscreen remains pressed, we report
+ * a new event with the latest data and then sleep until the
+ * next timer tick. This mirrors the behavior of the old
+ * driver, with much less code.
+ */
+static irqreturn_t exynos_ts_isr(int irq, void *dev_id)
+{
+ struct exynos_adc *info = dev_id;
+ struct iio_dev *dev = dev_get_drvdata(info-dev);
+ u32 x, y;
+ bool pressed;
+ int ret;
+
+ while (info-input-users) {
+ ret = exynos_read_s3c64xx_ts(dev, x, y);
+ if (ret == -ETIMEDOUT)
+ break;
+
+ pressed = x y ADC_DATX_PRESSED;
+ if (!pressed) {
+ input_report_key(info-input, BTN_TOUCH, 0);
+ input_sync(info-input);
+ break;
+ }
+
+ input_report_abs(info-input, ABS_X, x ADC_DATX_MASK);
+ input_report_abs(info-input, ABS_Y, y ADC_DATY_MASK);
+ input_report_key(info-input, BTN_TOUCH, 1);
+ input_sync(info-input);
+
+ msleep(1);
+ };
+
+ writel(0, ADC_V1_CLRINTPNDNUP(info-regs));
+
+ return IRQ_HANDLED;
+}
+
static int exynos_adc_reg_access(struct iio_dev *indio_dev,
unsigned reg, unsigned writeval,
unsigned *readval)
@@ -457,12 +554,66 @@ static int exynos_adc_remove_devices(struct device
*dev, void *c)
return 0;
}
+static int exynos_adc_ts_open(struct input_dev *dev)
+{
+ struct exynos_adc *info = input_get_drvdata(dev);
+
+ enable_irq(info-tsirq);
+
+ return 0;
+}
+
+static void exynos_adc_ts_close(struct input_dev *dev)
+{
+ struct exynos_adc *info = input_get_drvdata(dev);
+
+ disable_irq(info-tsirq);
+}
+
+static int exynos_adc_ts_init(struct exynos_adc *info)
+{
+ int ret;
+
+ if (info-tsirq = 0)
+ return -ENODEV;
+
+ info-input = input_allocate_device();
+ if (!info-input)
+ return -ENOMEM;
+
+ info-input-evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+ info-input-keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+
+ input_set_abs_params(info-input, ABS_X, 0, 0x3FF, 0, 0);
+ input_set_abs_params(info-input, ABS_Y, 0, 0x3FF, 0, 0);
+
+ info-input-name = S3C24xx TouchScreen;
+ info-input-id.bustype = BUS_HOST;
+ info-input-open = exynos_adc_ts_open;
+ info-input-close = exynos_adc_ts_close;
+
+ input_set_drvdata(info-input, info);
+
+ ret = input_register_device(info-input);
+ if (ret)
+ input_free_device(info-input);
+
+ disable_irq(info-tsirq);
+ ret = request_threaded_irq(info-tsirq, NULL, exynos_ts_isr,
+0, touchscreen, info);
+ if (ret)
+ input_unregister_device(info-input);
+
+ return ret;
+}
+
static int exynos_adc_probe(struct platform_device *pdev)
{
struct exynos_adc *info = NULL;
struct device_node *np = pdev-dev.of_node;
struct iio_dev *indio_dev = NULL;
struct resource *mem;
+ bool has_ts = false;
int ret = -ENODEV;
int irq;
@@ -498,8 +649,14 @@ static int exynos_adc_probe(struct platform_device *pdev)
dev_err(pdev-dev, no irq resource?\n);
return irq;
}
-
info-irq = irq;
+
+ irq = platform_get_irq(pdev, 1);
+ if (irq == -EPROBE_DEFER)
+ return irq;
+
+ info-tsirq = irq;
+
info-dev = pdev-dev;
init_completion(info-completion);
@@ -565,6 +722,15 @@ static int exynos_adc_probe(struct platform_device *pdev)
if (info-data-init_hw)
info-data-init_hw(info);
+ /* leave out any TS related code if unreachable */
+ if (IS_BUILTIN(CONFIG_INPUT) ||
+ (IS_MODULE(CONFIG_INPUT) config_enabled(MODULE)))
This is ugly... We need IS_SUBSYSTEM_AVAILABLE() wrapper for this...
Anyway,
Acked-by: Dmitry Torokhov dmitry.torok...@gmail.com
+ has_ts = of_property_read_bool(pdev-dev.of_node,
has-touchscreen);
+ if (has_ts)
+ ret = exynos_adc_ts_init(info);
+ if (ret)
+ goto err_iio;
+
ret = of_platform_populate(np, exynos_adc_match, NULL, indio_dev-dev);
if (ret 0) {
dev_err(pdev-dev, failed adding child nodes
Re: [PATCH 2/2] iio: exynos-adc: add experimental touchscreen support
On Mon, Jul 21, 2014 at 12:23:58PM +0200, Arnd Bergmann wrote:
On Sunday 20 July 2014 13:28:42 Dmitry Torokhov wrote:
On Sun, Jul 20, 2014 at 02:51:37PM +0100, Jonathan Cameron wrote:
+
+do {
+ret =exynos_read_s3c64xx_ts(dev, NULL, x, y,
IIO_CHAN_INFO_RAW);
= exynos
+if (ret == -ETIMEDOUT)
+break;
+
+pressed = x y ADC_DATX_PRESSED;
+if (!pressed)
+break;
+
+input_report_abs(info-input, ABS_X, x ADC_DATX_MASK);
+input_report_abs(info-input, ABS_Y, y ADC_DATX_MASK);
+input_report_key(info-input, BTN_TOUCH, 1);
+input_sync(info-input);
+
+msleep(1);
+} while (1);
It would be nice to actually close the device even if someone is
touching screen. Please implement open/close methods and have them set a
flag that you would check here.
Ok. I think it's even better to move the request_irq() into the open function,
which will avoid the flag and defer the error handling into the actual
opening,
as well as syncing the running irq with the close function.
I do not quite like acquiring resources needed in open. I think drivers should
do all resource acquisition in probe() and leave open()/close() to
activate/quiesce devices.
+/* data from s3c2410_ts driver */
+info-input-name = S3C24xx TouchScreen;
+info-input-id.bustype = BUS_HOST;
+info-input-id.vendor = 0xDEAD;
+info-input-id.product = 0xBEEF;
You do not need to fill these entries with fake data.
Ok, I wondered about this, but didn't want to change too much from
the old driver (I changed the version number).
+info-input-id.version = 0x0200;
Do I need this?
Not really.
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] iio: exynos-adc: add experimental touchscreen support
On Mon, Jul 21, 2014 at 05:11:27PM +0200, Arnd Bergmann wrote:
On Monday 21 July 2014 07:44:42 Dmitry Torokhov wrote:
It would be nice to actually close the device even if someone is
touching screen. Please implement open/close methods and have them set a
flag that you would check here.
Ok. I think it's even better to move the request_irq() into the open
function,
which will avoid the flag and defer the error handling into the actual
opening,
as well as syncing the running irq with the close function.
I do not quite like acquiring resources needed in open. I think drivers
should
do all resource acquisition in probe() and leave open()/close() to
activate/quiesce devices.
Ok, I'll move it back then. I'm not sure what I'm supposed to do
in open/close then. Isn't it enough to check info-input-users
like this?
static irqreturn_t exynos_ts_isr(int irq, void *dev_id)
{
struct exynos_adc *info = dev_id;
struct iio_dev *dev = dev_get_drvdata(info-dev);
u32 x, y;
bool pressed;
int ret;
while (info-input-users) {
ret = exynos_read_s3c64xx_ts(dev, x, y);
if (ret == -ETIMEDOUT)
break;
pressed = x y ADC_DATX_PRESSED;
if (!pressed) {
input_report_key(info-input, BTN_TOUCH, 0);
input_sync(info-input);
break;
}
input_report_abs(info-input, ABS_X, x ADC_DATX_MASK);
input_report_abs(info-input, ABS_Y, y ADC_DATY_MASK);
input_report_key(info-input, BTN_TOUCH, 1);
input_sync(info-input);
msleep(1);
};
writel(0, ADC_V1_CLRINTPNDNUP(info-regs));
return IRQ_HANDLED;
}
I could do enable_irq()/disable_irq(), but that leaves a small
race at startup where we request the irq line and then immediately
disable it again.
I think the above (or a separate flag in driver structure) coupled with
enable/disable IRQ is fine - input core can deal with calls to input_report_*
on devices that have been properly allocated but have not been registered yet.
drivers/input/keyboard/samsung-keypad.c does similar handling.
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] iio: exynos-adc: add experimental touchscreen support
On Sun, Jul 20, 2014 at 02:51:37PM +0100, Jonathan Cameron wrote:
On 20/07/14 14:49, Jonathan Cameron wrote:
On 18/07/14 20:29, Arnd Bergmann wrote:
This adds support for the touchscreen on Samsung s3c64xx.
The driver is completely untested but shows roughly how
it could be done, following the example of the at91 driver.
Hi Arnd,
Cc'd linux-input and Dmitry.
Open questions include:
- compared to the old plat-samsung/adc driver, there is
no support for prioritizing ts over other clients, nor
for oversampling. From my reading of the code, the
priorities didn't actually have any effect at all, but
the oversampling might be needed. Maybe the original
authors have some insight.
- I simply register the input device from the adc driver
itself, as the at91 code does. The driver also supports
sub-nodes, but I don't understand how they are meant
to be used, so using those might be better.
So, the alternative you are (I think) referring to is to use
the buffered in kernel client interface. That way a separate
touch screen driver can use the output channels provided by IIO
in much the same way you might use a regulator or similar.
Note that whilst this is similar to the simple polled interface
used for things like the iio_hwmon driver, the data flow is
quite different (clearly the polled interfce would be
inappropriate here).
Whilst we've discussed it in the past for touch screen drivers
like this, usually the hardware isn't generic enough to be
of any real use if not being used as a touch screen. As such
it's often simpler to just have the support directly in the
driver (as you've observed the at91 driver does this).
Whilst the interface has been there a while, it's not really had
all that much use. The original target was the simpler case
of 3D accelerometer where we have a generic iio to input
bridge driver. Time constraints meant that I haven't yet actually
formally submitted the input side of this. Whilst there are lots
of other things that can use this interface, right now nothing
actually does so.
- The new exynos_read_s3c64xx_ts() function is intentionally
very similar to the existing exynos_read_raw() functions.
It should probably be changed, either by merging the two
into one, or by simplifying the exynos_read_s3c64xx_ts()
function. This depends a bit on the answers to the questions
above.
I'd be tempted to not bother keeping them that similar. It's
not a generic IIO channel so simplify it where possible.
- We probably need to add support for platform_data as well,
I've skipped this so far.
- Is anybody able to debug this driver on real hardware?
While it's possible that it actually works, it's more
likely that I made a few subtle mistakes.
Signed-off-by: Arnd Bergmann a...@arndb.de
Looks pretty good to me. A few symantic bits and pieces and
one bug spotted. Short and sweet.
diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
index e1b74828f413..4329bf3c3326 100644
--- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
@@ -41,6 +41,10 @@ Required properties:
and compatible ADC block)
- vdd-supplyVDD input supply.
+Optional properties:
+- has-touchscreen:If present, indicates that a touchscreen is
+connected an usable.
+
Note: child nodes can be added for auto probing from device tree.
Example: adding device info in dtsi file
diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index 5f95638513d2..cf1d9f3e2492 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -34,6 +34,7 @@
#include linux/regulator/consumer.h
#include linux/of_platform.h
#include linux/err.h
+#include linux/input.h
Might want to make the input side optional at compile time...
I supose the existing parts are unlikely to be used much in headless
devices, but you never know. Maybe we just leave this until someone
shouts they want to be able to avoid compiling it in.
#include linux/iio/iio.h
#include linux/iio/machine.h
@@ -103,6 +104,7 @@
/* Bit definitions common for ADC_V1 and ADC_V2 */
#define ADC_CON_EN_START(1u 0)
+#define ADC_DATX_PRESSED(1u 15)
#define ADC_DATX_MASK0xFFF
#define EXYNOS_ADC_TIMEOUT(msecs_to_jiffies(100))
@@ -110,16 +112,20 @@
struct exynos_adc {
struct exynos_adc_data*data;
struct device*dev;
+struct input_dev*input;
void __iomem*regs;
void __iomem*enable_reg;
struct clk*clk;
struct clk*sclk;
unsigned intirq;
+unsigned inttsirq;
struct regulator*vdd;
struct completioncompletion;
+bool
Re: [PATCH] Input: s3c2410_ts: Move to clk_prepare_enable/clk_disable_unprepare
On Mon, Jun 30, 2014 at 10:09:37PM +0300, Vasily Khoruzhick wrote:
Use clk_prepare_enable/clk_disable_unprepare to make the driver
work properly with common clock framework.
Signed-off-by: Vasily Khoruzhick anars...@gmail.com
---
drivers/input/touchscreen/s3c2410_ts.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/input/touchscreen/s3c2410_ts.c
b/drivers/input/touchscreen/s3c2410_ts.c
index 19cb247..c0e0baa 100644
--- a/drivers/input/touchscreen/s3c2410_ts.c
+++ b/drivers/input/touchscreen/s3c2410_ts.c
@@ -264,7 +264,7 @@ static int s3c2410ts_probe(struct platform_device *pdev)
return -ENOENT;
}
- clk_enable(ts.clock);
+ clk_prepare_enable(ts.clock);
dev_dbg(dev, got and enabled clocks\n);
ts.irq_tc = ret = platform_get_irq(pdev, 0);
@@ -369,7 +369,7 @@ static int s3c2410ts_remove(struct platform_device *pdev)
free_irq(ts.irq_tc, ts.input);
del_timer_sync(touch_timer);
- clk_disable(ts.clock);
+ clk_disable_unprepare(ts.clock);
clk_put(ts.clock);
input_unregister_device(ts.input);
@@ -383,7 +383,7 @@ static int s3c2410ts_suspend(struct device *dev)
{
writel(TSC_SLEEP, ts.io + S3C2410_ADCTSC);
disable_irq(ts.irq_tc);
- clk_disable(ts.clock);
+ clk_disable_unprepare(ts.clock);
Do we really need to unprepare on suspend? Why simply disabling is not
enough here?
Thanks.
return 0;
}
@@ -393,7 +393,7 @@ static int s3c2410ts_resume(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
struct s3c2410_ts_mach_info *info = dev_get_platdata(pdev-dev);
- clk_enable(ts.clock);
+ clk_prepare_enable(ts.clock);
enable_irq(ts.irq_tc);
/* Initialise registers */
--
2.0.0
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 07/10] mfd: cros_ec: cleanup: Remove EC wrapper functions
On Fri, Jun 27, 2014 at 5:31 AM, Wolfram Sang w...@the-dreams.de wrote: On Wed, Jun 18, 2014 at 11:14:04AM -0700, Doug Anderson wrote: From: Bill Richardson wfric...@chromium.org Remove the three wrapper functions that talk to the EC without passing all the desired arguments and just use the underlying communication function that passes everything in a struct intead. This is internal code refactoring only. Nothing should change. Signed-off-by: Bill Richardson wfric...@chromium.org Signed-off-by: Doug Anderson diand...@chromium.org Acked-by: Lee Jones lee.jo...@linaro.org Reviewed-by: Simon Glass s...@chromium.org For the I2C part: Acked-by: Wolfram Sang w...@the-dreams.de I'm good with input bits as well. Acked-by: Dmitry Torokhov dmitry.torok...@gmail.com Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] regulator: fixed: update to devm_* API
Hi Manish,
On Tue, Jan 28, 2014 at 08:42:00AM +0530, Manish Badarkhe wrote:
Update the code to use devm_* API so that driver core will manage
resources.
Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
Changes since V1:
1. Updated driver to use devm_kzalloc to kstrdup.
2. Updated commit message.
Not tested on any board.
:100644 100644 5ea64b9... e9763a4... Mdrivers/regulator/fixed.c
drivers/regulator/fixed.c | 42 --
1 file changed, 12 insertions(+), 30 deletions(-)
diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 5ea64b9..e9763a4 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -132,15 +132,15 @@ static int reg_fixed_voltage_probe(struct
platform_device *pdev)
GFP_KERNEL);
if (drvdata == NULL) {
dev_err(pdev-dev, Failed to allocate device data\n);
- ret = -ENOMEM;
- goto err;
+ return -ENOMEM;
}
- drvdata-desc.name = kstrdup(config-supply_name, GFP_KERNEL);
+ drvdata-desc.name = devm_kzalloc(pdev-dev,
+ strlen(config-supply_name) + 1,
+ GFP_KERNEL);
if (drvdata-desc.name == NULL) {
dev_err(pdev-dev, Failed to allocate supply name\n);
- ret = -ENOMEM;
- goto err;
+ return -ENOMEM;
}
Umm, I am fairly certain that devm_kzalloc() can't be used as a
substitute for kstrdup, at least not without accompanying memcpy.
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] regulator: fixed: Use devm_regulator_register
Hi Manish,
On Sat, Jan 25, 2014 at 11:35:54PM +0530, Manish Badarkhe wrote:
Use devm_regulator_register instead of regulator_register
which simplifies the code.
... and also breaks the driver: now you are freeing desc-name and
desc-supply_name while regulator structures are still alive and can be
referenced from its sysfs attributes, etc.
Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
:100644 100644 5ea64b9... 6d32341... Mdrivers/regulator/fixed.c
drivers/regulator/fixed.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 5ea64b9..6d32341 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -186,7 +186,8 @@ static int reg_fixed_voltage_probe(struct platform_device
*pdev)
cfg.driver_data = drvdata;
cfg.of_node = pdev-dev.of_node;
- drvdata-dev = regulator_register(drvdata-desc, cfg);
+ drvdata-dev = devm_regulator_register(pdev-dev, drvdata-desc,
+cfg);
if (IS_ERR(drvdata-dev)) {
ret = PTR_ERR(drvdata-dev);
dev_err(pdev-dev, Failed to register regulator: %d\n, ret);
@@ -212,7 +213,6 @@ static int reg_fixed_voltage_remove(struct
platform_device *pdev)
{
struct fixed_voltage_data *drvdata = platform_get_drvdata(pdev);
- regulator_unregister(drvdata-dev);
kfree(drvdata-desc.supply_name);
kfree(drvdata-desc.name);
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] max8925_power: Use IS_ENABLED(CONFIG_OF) for DT code.
On Sun, Jan 26, 2014 at 07:31:50PM +0530, Manish Badarkhe wrote:
Hi Tomasz,
Thank you for your review comments.
On Sun, Jan 26, 2014 at 6:52 PM, Tomasz Figa tomasz.f...@gmail.com wrote:
Hi Manish,
On 26.01.2014 08:15, Manish Badarkhe wrote:
Instead of #if define CONFIG_OF use IS_ENABLED(CONFIG_OF)
option for DT code to avoid if-deffery in code.
Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
:100644 100644 b4513f2... d353fbc... M drivers/power/max8925_power.c
drivers/power/max8925_power.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
index b4513f2..d353fbc 100644
--- a/drivers/power/max8925_power.c
+++ b/drivers/power/max8925_power.c
@@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct
max8925_power_info *info)
return 0;
}
-#ifdef CONFIG_OF
static struct max8925_power_pdata *
max8925_power_dt_init(struct platform_device *pdev)
{
@@ -468,13 +467,6 @@ max8925_power_dt_init(struct platform_device *pdev)
return pdata;
}
-#else
-static struct max8925_power_pdata *
-max8925_power_dt_init(struct platform_device *pdev)
-{
- return pdev-dev.platform_data;
-}
-#endif
static int max8925_power_probe(struct platform_device *pdev)
{
@@ -483,7 +475,11 @@ static int max8925_power_probe(struct platform_device
*pdev)
struct max8925_power_info *info;
int ret;
- pdata = max8925_power_dt_init(pdev);
+ if (IS_ENABLED(CONFIG_OF))
+ pdata = max8925_power_dt_init(pdev);
+ else
+ pdata = pdev-dev.platform_data;
+
This does not look much better than before this patch. Instead of
if-deffery outside function bodies you are adding iffery (if there is
such a word) inside a function.
What about adding
if (!IS_ENABLED(CONFIG_OF))
return pdev-dev.platform_data;
on top of max8925_power_dt_init() instead or maybe also renaming this
function to max8925_get_pdata()?
Okay, I will rename function max8925_power_dt_init() to
max8925_get_pdata().
As you suggested, in the body of this function will add a logic to
retrieve data in case
of DT and non-DT platforms.
Should we not always favor platform-supplied data regardless of
CONFIG_OF state and fall back to DT (firmware) supplied data if platform
data is absent? This way one can tweak kernel behavior without needing
to change firmware.
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] max8925_power: Use IS_ENABLED(CONFIG_OF) for DT code.
On Mon, Jan 27, 2014 at 02:31:59AM +0400, Dmitry Eremin-Solenikov wrote:
On Mon, Jan 27, 2014 at 1:49 AM, Tomasz Figa tomasz.f...@gmail.com wrote:
On 26.01.2014 22:45, Dmitry Torokhov wrote:
On Sun, Jan 26, 2014 at 07:31:50PM +0530, Manish Badarkhe wrote:
Hi Tomasz,
Thank you for your review comments.
On Sun, Jan 26, 2014 at 6:52 PM, Tomasz Figa tomasz.f...@gmail.com
wrote:
Hi Manish,
On 26.01.2014 08:15, Manish Badarkhe wrote:
Instead of #if define CONFIG_OF use IS_ENABLED(CONFIG_OF)
option for DT code to avoid if-deffery in code.
Signed-off-by: Manish Badarkhe badarkhe.man...@gmail.com
---
:100644 100644 b4513f2... d353fbc... M drivers/power/max8925_power.c
drivers/power/max8925_power.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/power/max8925_power.c
b/drivers/power/max8925_power.c
index b4513f2..d353fbc 100644
--- a/drivers/power/max8925_power.c
+++ b/drivers/power/max8925_power.c
@@ -427,7 +427,6 @@ static int max8925_deinit_charger(struct
max8925_power_info *info)
return 0;
}
-#ifdef CONFIG_OF
static struct max8925_power_pdata *
max8925_power_dt_init(struct platform_device *pdev)
{
@@ -468,13 +467,6 @@ max8925_power_dt_init(struct platform_device
*pdev)
return pdata;
}
-#else
-static struct max8925_power_pdata *
-max8925_power_dt_init(struct platform_device *pdev)
-{
- return pdev-dev.platform_data;
-}
-#endif
static int max8925_power_probe(struct platform_device *pdev)
{
@@ -483,7 +475,11 @@ static int max8925_power_probe(struct
platform_device *pdev)
struct max8925_power_info *info;
int ret;
- pdata = max8925_power_dt_init(pdev);
+ if (IS_ENABLED(CONFIG_OF))
+ pdata = max8925_power_dt_init(pdev);
+ else
+ pdata = pdev-dev.platform_data;
+
This does not look much better than before this patch. Instead of
if-deffery outside function bodies you are adding iffery (if there is
such a word) inside a function.
What about adding
if (!IS_ENABLED(CONFIG_OF))
return pdev-dev.platform_data;
on top of max8925_power_dt_init() instead or maybe also renaming this
function to max8925_get_pdata()?
Okay, I will rename function max8925_power_dt_init() to
max8925_get_pdata().
As you suggested, in the body of this function will add a logic to
retrieve data in case
of DT and non-DT platforms.
Should we not always favor platform-supplied data regardless of
CONFIG_OF state and fall back to DT (firmware) supplied data if platform
data is absent? This way one can tweak kernel behavior without needing
to change firmware.
I guess we should, but apparently this was not the original behavior before
this patch, so I'm not sure if we should be squashing such semantic change
with this simple refactor.
Hmm. Judging from the code, max8925_power_dt_init() function follows exactly
opposite strategy - it uses platform_data if of_node is not
populated/available.
So (if dt_init will compile with CONFIG_OF disabled) one can always
use _dt_init()
function to retrieve pdata.
Right, and I question whether this is good behavior or if it should be
corrected.
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device
Hi Joe, On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote: Emitting an OOM message isn't necessary after input_allocate_device as there's a generic OOM and a dump_stack already done. No, please don't. The kzalloc may get changed in the future to not dump stack (that was added originally because not everyone was handling OOM properly, right?), input core might get changed to use something else than kzalloc, etc, etc. The majority of errors use dev_err so we also get idea what device failed (if there are several), and more. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device
On Thu, Oct 24, 2013 at 11:45:39AM -0700, Joe Perches wrote: On Thu, 2013-10-24 at 11:37 -0700, Dmitry Torokhov wrote: Hi Joe, On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote: Emitting an OOM message isn't necessary after input_allocate_device as there's a generic OOM and a dump_stack already done. No, please don't. The kzalloc may get changed in the future to not dump stack (that was added originally because not everyone was handling OOM properly, right?), input core might get changed to use something else than kzalloc, etc, etc. The majority of errors use dev_err so we also get idea what device failed (if there are several), and more. I think that's not valuable as input_allocate_device already has dozens of locations that don't emit a specific OOM and centralizing the location for any generic message would work anyway. Not having diagnostic messages in some of the drivers is hardly a justification to remove them from everywhere else. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Input: gpio-keys - update to devm_* API
On Wed, Sep 18, 2013 at 12:41:11AM +0530, Manish Badarkhe wrote: Hi Dmitry, On Wed, Sep 18, 2013 at 12:22 AM, Dmitry Torokhov dmitry.torok...@gmail.com wrote: Hi Manish, On Sun, Sep 15, 2013 at 01:22:23AM +0530, Manish Badarkhe wrote: Update the code to use devm_* API so that driver core will manage resources. And the benefit of this would be...? There are still resources that are managed in traditional way and I really dislike mixing the 2 styles. I can see applying patch that converts the driver to use devm_ for all its resources and gets rid of the remove() method altogether, but I am not sure how beneficial this kind of changes are for existing drivers. IMO devm_ makes us to manage resources properly without much care about freeing it ( as devm_handles freeing automatically). Are the resources released improperly in the current version of the driver? IOW I do not see the point in applying this patch as it does not make the driver materially better. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Input: gpio-keys - update to devm_* API
Hi Manish, On Sun, Sep 15, 2013 at 01:22:23AM +0530, Manish Badarkhe wrote: Update the code to use devm_* API so that driver core will manage resources. And the benefit of this would be...? There are still resources that are managed in traditional way and I really dislike mixing the 2 styles. I can see applying patch that converts the driver to use devm_ for all its resources and gets rid of the remove() method altogether, but I am not sure how beneficial this kind of changes are for existing drivers. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] input: touchscreen: atmel_mxt_ts: Add Device Tree support
Hi Tomasz, On Thu, Apr 04, 2013 at 06:14:24PM +0200, Tomasz Figa wrote: This series is an attempt to add Device Tree support to Atmel maXtouch touchscreen driver. First patch adds support for VDD voltage regulator to get operating voltage using regulator API instead of a driver specific field in platform data. Second patch implements Device Tree bindings for the driver and adds respective documentation. Tested on Universal C210 board. Please coordinate this effort with Nick Dyer nick.d...@itdev.co.uk, Daniel Kurtz djku...@chromium.org and Benson Leung ble...@chromium.org as there is a big update to the atmel_mxt_ts in works and I would prefer not to disturb it. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] input: keyboard: gpio-keys: Try to parse IRQ from device tree
Hi Tomasz, On Wed, Oct 03, 2012 at 01:20:00PM +0200, Tomasz Figa wrote: On modern platforms using device tree and non-legacy IRQ domains there is usually no way to perform direct translation between GPIO and IRQ, because the IRQ of interest is not mapped yet into sparse IRQ namespace. This patch modifies the gpio_keys driver to parse IRQ from device tree and use gpio_to_irq only as a fallback. This means that this change would need to be applied to every driver that currently maps gpio to IRQ. Why can't gpio_to_irq() be fixed instead? Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] input: samsung-keypad: add clk_prepare and clk_unprepare
On Wed, Oct 03, 2012 at 08:31:52AM +0900, Thomas Abraham wrote:
Add calls to clk_prepare and clk_unprepare as required by commom clock
framework.
Signed-off-by: Thomas Abraham thomas.abra...@linaro.org
Applied, thank you Thomas.
---
drivers/input/keyboard/samsung-keypad.c | 11 ++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/drivers/input/keyboard/samsung-keypad.c
b/drivers/input/keyboard/samsung-keypad.c
index 277e26d..9d7a111 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -431,6 +431,12 @@ static int __devinit samsung_keypad_probe(struct
platform_device *pdev)
goto err_unmap_base;
}
+ error = clk_prepare(keypad-clk);
+ if (error) {
+ dev_err(pdev-dev, keypad clock prepare failed\n);
+ goto err_put_clk;
+ }
+
keypad-input_dev = input_dev;
keypad-pdev = pdev;
keypad-row_shift = row_shift;
@@ -461,7 +467,7 @@ static int __devinit samsung_keypad_probe(struct
platform_device *pdev)
keypad-keycodes, input_dev);
if (error) {
dev_err(pdev-dev, failed to build keymap\n);
- goto err_put_clk;
+ goto err_unprepare_clk;
}
input_set_capability(input_dev, EV_MSC, MSC_SCAN);
@@ -503,6 +509,8 @@ err_free_irq:
pm_runtime_disable(pdev-dev);
device_init_wakeup(pdev-dev, 0);
platform_set_drvdata(pdev, NULL);
+err_unprepare_clk:
+ clk_unprepare(keypad-clk);
err_put_clk:
clk_put(keypad-clk);
samsung_keypad_dt_gpio_free(keypad);
@@ -531,6 +539,7 @@ static int __devexit samsung_keypad_remove(struct
platform_device *pdev)
*/
free_irq(keypad-irq, keypad);
+ clk_unprepare(keypad-clk);
clk_put(keypad-clk);
samsung_keypad_dt_gpio_free(keypad);
--
1.7.4.1
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] input: samsung-keypad: Add HAVE_SAMSUNG_KEYPAD config option
Hi Thomas, On Tue, Sep 06, 2011 at 07:25:16PM +0530, Thomas Abraham wrote: Samsung keyboard driver could be used with platforms using device tree. So the inclusion of samsung keyboard driver cannot be based on SAMSUNG_DEV_KEYPAD. A new config option HAVE_SAMSUNG_KEYPAD is added which device tree based platforms should use to include samsung keyboard driver. I am sorry, I do not follow... What is the difference between SAMSUNG_DEV_KEYPAD and HAVE_SAMSUNG_KEYPAD? They look exactly the same. Thanks, Signed-off-by: Thomas Abraham thomas.abra...@linaro.org --- drivers/input/keyboard/Kconfig |9 - 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig index b4dee9d..370fb18 100644 --- a/drivers/input/keyboard/Kconfig +++ b/drivers/input/keyboard/Kconfig @@ -423,9 +423,16 @@ config KEYBOARD_PMIC8XXX To compile this driver as a module, choose M here: the module will be called pmic8xxx-keypad. +config HAVE_SAMSUNG_KEYPAD + bool + help + This will include Samsung Keypad controller driver support. If you + want to include Samsung Keypad support for any machine, kindly + select this in the respective mach-/Kconfig file. + config KEYBOARD_SAMSUNG tristate Samsung keypad support - depends on SAMSUNG_DEV_KEYPAD + depends on SAMSUNG_DEV_KEYPAD || HAVE_SAMSUNG_KEYPAD help Say Y here if you want to use the Samsung keypad. -- 1.6.6.rc2 -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] input: samsung-keypad: Add HAVE_SAMSUNG_KEYPAD config option
On Wed, Sep 07, 2011 at 11:22:48AM -0700, Dmitry Torokhov wrote:
Hi Thomas,
On Tue, Sep 06, 2011 at 07:25:16PM +0530, Thomas Abraham wrote:
Samsung keyboard driver could be used with platforms using device tree.
So the inclusion of samsung keyboard driver cannot be based on
SAMSUNG_DEV_KEYPAD. A new config option HAVE_SAMSUNG_KEYPAD is added
which device tree based platforms should use to include samsung keyboard
driver.
I am sorry, I do not follow... What is the difference between
SAMSUNG_DEV_KEYPAD and HAVE_SAMSUNG_KEYPAD? They look exactly the same.
BTW, should we do something like below?
Thanks.
--
Dmitry
Input: samsung-keypad - enable compiling on other platforms
From: Dmitry Torokhov dmitry.torok...@gmail.com
There is nothing in keypad platform definitions that requires
the driver be complied on Samsung platform only, so let's move them
out of the platform subdirectory and relax the dependencies.
Signed-off-by: Dmitry Torokhov d...@mail.ru
---
arch/arm/plat-samsung/include/plat/keypad.h | 27 +
drivers/input/keyboard/Kconfig |5 ++-
drivers/input/keyboard/samsung-keypad.c |2 +
include/linux/input/samsung-keypad.h| 43 +++
4 files changed, 48 insertions(+), 29 deletions(-)
create mode 100644 include/linux/input/samsung-keypad.h
diff --git a/arch/arm/plat-samsung/include/plat/keypad.h
b/arch/arm/plat-samsung/include/plat/keypad.h
index b59a648..8fddee3 100644
--- a/arch/arm/plat-samsung/include/plat/keypad.h
+++ b/arch/arm/plat-samsung/include/plat/keypad.h
@@ -13,32 +13,7 @@
#ifndef __PLAT_SAMSUNG_KEYPAD_H
#define __PLAT_SAMSUNG_KEYPAD_H
-#include linux/input/matrix_keypad.h
-
-#define SAMSUNG_MAX_ROWS 8
-#define SAMSUNG_MAX_COLS 8
-
-/**
- * struct samsung_keypad_platdata - Platform device data for Samsung Keypad.
- * @keymap_data: pointer to matrix_keymap_data.
- * @rows: number of keypad row supported.
- * @cols: number of keypad col supported.
- * @no_autorepeat: disable key autorepeat.
- * @wakeup: controls whether the device should be set up as wakeup source.
- * @cfg_gpio: configure the GPIO.
- *
- * Initialisation data specific to either the machine or the platform
- * for the device driver to use or call-back when configuring gpio.
- */
-struct samsung_keypad_platdata {
- const struct matrix_keymap_data *keymap_data;
- unsigned int rows;
- unsigned int cols;
- bool no_autorepeat;
- bool wakeup;
-
- void (*cfg_gpio)(unsigned int rows, unsigned int cols);
-};
+#include linux/input/samsung_keypad.h
/**
* samsung_keypad_set_platdata - Set platform data for Samsung Keypad device.
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index de846f8..db1b221 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -434,9 +434,10 @@ config KEYBOARD_PMIC8XXX
config KEYBOARD_SAMSUNG
tristate Samsung keypad support
- depends on SAMSUNG_DEV_KEYPAD
+ depends on HAVE_CLK
help
- Say Y here if you want to use the Samsung keypad.
+ Say Y here if you want to use the keypad on your Samsung mobile
+ device.
To compile this driver as a module, choose M here: the
module will be called samsung-keypad.
diff --git a/drivers/input/keyboard/samsung-keypad.c
b/drivers/input/keyboard/samsung-keypad.c
index d244fdf..1a2b755 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -22,7 +22,7 @@
#include linux/platform_device.h
#include linux/slab.h
#include linux/sched.h
-#include plat/keypad.h
+#include linux/input/samsung-keypad.h
#define SAMSUNG_KEYIFCON 0x00
#define SAMSUNG_KEYIFSTSCLR0x04
diff --git a/include/linux/input/samsung-keypad.h
b/include/linux/input/samsung-keypad.h
new file mode 100644
index 000..f25619b
--- /dev/null
+++ b/include/linux/input/samsung-keypad.h
@@ -0,0 +1,43 @@
+/*
+ * Samsung Keypad platform data definitions
+ *
+ * Copyright (C) 2010 Samsung Electronics Co.Ltd
+ * Author: Joonyoung Shim jy0922.s...@samsung.com
+ *
+ * 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; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#ifndef __SAMSUNG_KEYPAD_H
+#define __SAMSUNG_KEYPAD_H
+
+#include linux/input/matrix_keypad.h
+
+#define SAMSUNG_MAX_ROWS 8
+#define SAMSUNG_MAX_COLS 8
+
+/**
+ * struct samsung_keypad_platdata - Platform device data for Samsung Keypad.
+ * @keymap_data: pointer to matrix_keymap_data.
+ * @rows: number of keypad row supported.
+ * @cols: number of keypad col supported.
+ * @no_autorepeat: disable key autorepeat.
+ * @wakeup: controls whether the device should be set up as wakeup source.
+ * @cfg_gpio: configure
Re: [PATCH v6 3/3] input: samsung-keypad - Add samsung keypad driver
On Mon, Aug 02, 2010 at 03:30:57PM +0900, Kukjin Kim wrote: Joonyoung Shim wrote: On 8/2/2010 2:12 PM, Kukjin Kim wrote: Joonyoung Shim wrote: On 8/2/2010 12:38 PM, Kukjin Kim wrote: Joonyoung Shim wrote: Hi, I came back from vacation. Do you have any feedback? If ok, i want to go to input tree or samsung tree these added sched.h including. I applied the patch adding samsung-keypad.c and the platform data arch/arm/plat-samsung/include/plat/keypad.h and I expect that patches to actually enable keypad will go through appropriate platform tree(s). OK, the rest will go through samsung platform tree Ok..will apply. Kukjin, could you remove keypad.h on the commit ARM: SAMSUNG: Add keypad device support of the samsung platform tree. The keypad.h file was included on input tree. (http://git.kernel.org/?p=linux/kernel/git/dtor/input.git;a=commit;h=0fffed27f92d9d7 a34de9fe017b7082b5958bb93) You mean in plat-samsung/dev-keypad.c? If remove inclusion plat/keypad.h in there, following build error happens. No. I mean to remove arch/arm/plat-samsung/include/plat/keypad.h file on samsung platform tree, it was applied on input tree already. If needs, i can repost. If it is not small change like to remove a file in a patch, I think should be re- submitted. Will drop your previous patch in my tree. arch/arm/plat-samsung/dev-keypad.c:41: warning: 'struct samsung_keypad_platdata' declared inside parameter list arch/arm/plat-samsung/dev-keypad.c:41: warning: its scope is only this definition or declaration, which is probably not what you want arch/arm/plat-samsung/dev-keypad.c: In function 'samsung_keypad_set_platdata': arch/arm/plat-samsung/dev-keypad.c:50: error: invalid application of 'sizeof' to incomplete type 'struct samsung_keypad_platdata' arch/arm/plat-samsung/dev-keypad.c:54: error: dereferencing pointer to incomplete type arch/arm/plat-samsung/dev-keypad.c:55: error: dereferencing pointer to incomplete type arch/arm/plat-samsung/dev-keypad.c:55: error: 'samsung_keypad_cfg_gpio' undeclared (first use in this function) arch/arm/plat-samsung/dev-keypad.c:55: error: (Each undeclared identifier is reported only once arch/arm/plat-samsung/dev-keypad.c:55: error: for each function it appears in.) make[1]: *** [arch/arm/plat-samsung/dev-keypad.o] Error 1 As you know, released 35 today. So we have no much time for this merge window. I'm not sure about merge rule, but I think the compile problem can be solved by merge to 36 of input tree and samsung platform tree. Hmm...actually, already informed about that on linux-next. If it's just build problem, I or Dmitry can fix it. However, this is not just that. I cannot understand how your 'plat/keypad.h' was included to both of patch. Please see the prior mail history. The keypad.h file is included only at the 1/3 patch in the original patch set, but Dmitry applied the 3/3 patch modified with keypad.h. The basic problem is that each patches be applied on two tree. It makes compile errors at the one tree lacking keypad.h file because both tree share keypad.h file. There are two solutions, First, all patches go to one tree. Second, we wait for merging to 36 of both tree then apply platform patches. And I'm not sure, should be 'plat/keypad.h' in which tree... Hi Dmitry, How can/should I do for it?...Which way is better to us? :-) Joonyoung, Let's wait for Dmitry's reply. I will holding drop your previous patch in my tree till that time. Kgene, I will be sending pull request to Linus in the next day or so, so it will have plat/keypad.h as it is in my tree. Once you merge (with his or mine) you have an option to patching the plat/keypad.h with hooks that I removed, although I would prefer if you changed the way you initialize your devices since having the these hooks defeats the purpose of having config pointer in platform data. Another option would be to have a separate include file for the hooks, one that drivers/input/keyboard/samsung-keypad.c does not use. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: S3C64XX: Add s3c64xx support to touchscreen driver
On Mon, Jun 28, 2010 at 11:11:11PM +0200, Maurus Cuelenaere wrote: Op 28-06-10 20:30, Dmitry Torokhov schreef: On Fri, May 07, 2010 at 01:40:30AM +0100, Ben Dooks wrote: On Thu, May 06, 2010 at 11:50:29AM +0200, Maurus Cuelenaere wrote: Add support for the s3c64xx touchscreen to the s3c2410_ts driver, ensuring it compiles and supports the extra features such as the interrupt generation. Signed-off-by: Maurus Cuelenaere mcuelena...@gmail.com Acked-by: Ben Dooks ben-li...@fluff.org Hmm, it looks like I slept on th epatch for too long. Is it still needed? Could you please rebase it against 2.6.35-rc*? Thanks! This was superseded by Input: s3c24xx_ts - Add FEAT for Samsung touchscreen support which is already in Linus' tree AFAICR. So you can drop this. Ah, great. Thank you for letting me know. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] input: samsung-keypad - Add samsung keypad driver
On Mon, Jun 28, 2010 at 05:39:39PM +0900, Joonyoung Shim wrote: Hi, Dmitry. On 6/25/2010 5:30 PM, Dmitry Torokhov wrote: Hi Joonyoung, On Mon, Jun 21, 2010 at 03:26:45PM +0900, Joonyoung Shim wrote: This patch adds support for keypad driver running on Samsung cpus. This driver is tested on GONI and Aquila board using S5PC110 cpu. Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com Following my conversation with Thomas Gleixner reagrding long playing threaded interrupt handlers I tried to convert your driver to use this concept. The idea is to keep polling within IRQ thread context instead of using additional work/timer structures to simplify code and ensure race-free shutdown/unbind. I think it was based on v4 of your driver and I'd appreciate if you could give it a try. Thank you. I have tested your patch on my v5 patchset and it is working after some fixing. I will post v6 patchset appling your patch. Cool, thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 3/3] input: samsung-keypad - Add samsung keypad driver
Hi Joonyoung,
On Mon, Jun 21, 2010 at 03:26:45PM +0900, Joonyoung Shim wrote:
This patch adds support for keypad driver running on Samsung cpus. This
driver is tested on GONI and Aquila board using S5PC110 cpu.
Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
Following my conversation with Thomas Gleixner reagrding long playing
threaded interrupt handlers I tried to convert your driver to use this
concept. The idea is to keep polling within IRQ thread context instead
of using additional work/timer structures to simplify code and ensure
race-free shutdown/unbind.
I think it was based on v4 of your driver and I'd appreciate if you could
give it a try.
Thank you.
--
Dmitry
Input: samsung-keypad - updates
From: Dmitry Torokhov dmitry.torok...@gmail.com
Signed-off-by: Dmitry Torokhov d...@mail.ru
---
arch/arm/plat-samsung/include/plat/keypad.h | 21 -
arch/arm/plat-samsung/include/plat/regs-keypad.h | 49 ---
drivers/input/keyboard/samsung-keypad.c | 334 ++
3 files changed, 215 insertions(+), 189 deletions(-)
delete mode 100644 arch/arm/plat-samsung/include/plat/regs-keypad.h
diff --git a/arch/arm/plat-samsung/include/plat/keypad.h
b/arch/arm/plat-samsung/include/plat/keypad.h
index 04ba600..0a35784 100644
--- a/arch/arm/plat-samsung/include/plat/keypad.h
+++ b/arch/arm/plat-samsung/include/plat/keypad.h
@@ -34,24 +34,11 @@
*/
struct samsung_keypad_platdata {
const struct matrix_keymap_data *keymap_data;
- unsigned introws;
- unsigned intcols;
- unsigned intrep:1;
+ unsigned int rows;
+ unsigned int cols;
+ bool rep;
- void(*cfg_gpio)(unsigned int rows, unsigned int cols);
+ void (*cfg_gpio)(unsigned int rows, unsigned int cols);
};
-/**
- * samsung_keypad_set_platdata - Set platform data for Samsung Keypad device.
- * @pd: Platform data to register to device.
- *
- * Register the given platform data for use with Samsung Keypad device.
- * The call will copy the platform data, so the board definitions can
- * make the structure itself __initdata.
- */
-extern void samsung_keypad_set_platdata(struct samsung_keypad_platdata *pd);
-
-/* defined by architecture to configure gpio. */
-extern void samsung_keypad_cfg_gpio(unsigned int rows, unsigned int cols);
-
#endif /* __PLAT_SAMSUNG_KEYPAD_H */
diff --git a/arch/arm/plat-samsung/include/plat/regs-keypad.h
b/arch/arm/plat-samsung/include/plat/regs-keypad.h
deleted file mode 100644
index e4688f0..000
--- a/arch/arm/plat-samsung/include/plat/regs-keypad.h
+++ /dev/null
@@ -1,49 +0,0 @@
-/*
- * linux/arch/arm/plat-samsung/include/plat/regs-keypad.h
- *
- * Copyright (C) 2010 Samsung Electronics Co.Ltd
- * Author: Joonyoung Shim jy0922.s...@samsung.com
- *
- * 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; either version 2 of the License, or (at your
- * option) any later version.
- *
- */
-
-#ifndef __SAMSUNG_KEYPAD_H__
-#define __SAMSUNG_KEYPAD_H__
-
-#define SAMSUNG_KEYIFCON 0x00
-#define SAMSUNG_KEYIFSTSCLR0x04
-#define SAMSUNG_KEYIFCOL 0x08
-#define SAMSUNG_KEYIFROW 0x0c
-#define SAMSUNG_KEYIFFC0x10
-
-/* SAMSUNG_KEYIFCON */
-#define SAMSUNG_KEYIFCON_INT_F_EN (1 0)
-#define SAMSUNG_KEYIFCON_INT_R_EN (1 1)
-#define SAMSUNG_KEYIFCON_DF_EN (1 2)
-#define SAMSUNG_KEYIFCON_FC_EN (1 3)
-#define SAMSUNG_KEYIFCON_WAKEUPEN (1 4)
-
-/* SAMSUNG_KEYIFSTSCLR */
-#define SAMSUNG_KEYIFSTSCLR_P_INT_MASK (0xff 0)
-#define SAMSUNG_KEYIFSTSCLR_R_INT_MASK (0xff 8)
-#define SAMSUNG_KEYIFSTSCLR_R_INT_OFFSET 8
-#define S5PV210_KEYIFSTSCLR_P_INT_MASK (0x3fff 0)
-#define S5PV210_KEYIFSTSCLR_R_INT_MASK (0x3fff 16)
-#define S5PV210_KEYIFSTSCLR_R_INT_OFFSET 16
-
-/* SAMSUNG_KEYIFCOL */
-#define SAMSUNG_KEYIFCOL_MASK (0xff 0)
-#define S5PV210_KEYIFCOLEN_MASK(0xff 8)
-
-/* SAMSUNG_KEYIFROW */
-#define SAMSUNG_KEYIFROW_MASK (0xff 0)
-#define S5PV210_KEYIFROW_MASK (0x3fff 0)
-
-/* SAMSUNG_KEYIFFC */
-#define SAMSUNG_KEYIFFC_MASK (0x3ff 0)
-
-#endif /* __SAMSUNG_KEYPAD_H__ */
diff --git a/drivers/input/keyboard/samsung-keypad.c
b/drivers/input/keyboard/samsung-keypad.c
index 244a3b6..c243fc5 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -1,5 +1,5 @@
/*
- * samsung-keypad.c -- Samsung keypad driver
+ * Samsung keypad driver
*
* Copyright (C) 2010 Samsung Electronics Co.Ltd
* Author: Joonyoung Shim jy0922.s...@samsung.com
@@ -21,8
Re: [PATCH v2 1/5] ARM: SAMSUNG: Add keypad device support
On Sun, May 30, 2010 at 05:42:37AM +0200, Marek Vasut wrote:
Dne Ne 30. května 2010 05:06:20 Joonyoung Shim napsal(a):
+ */
+struct samsung_kp_platdata {
+ const struct matrix_keymap_data *keymap_data;
+ unsigned introws;
+ unsigned intcols;
+ unsigned intrep;
I don't know, maybe using uint32_t here? On ARM, it doesn't matter so far as
int
will be always 32bit, but maybe we should just type the variables well ?
Guys, what do you think ?
I think unsigned int is fine, we do not care about particular size, just
need big enough. I'd change 'rep' to be a boolean though.
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/1] TOUCHSCREEN: S3C24XX touchscreen driver from Arnaud Patard.
On Thu, Nov 19, 2009 at 01:52:36PM +, Daniel Silverstone wrote: On Thu, Nov 19, 2009 at 11:34:40AM +, Ben Dooks wrote: + input_report_key(ts.input, BTN_TOUCH, 1); + input_report_abs(ts.input, ABS_PRESSURE, 1); No fake pressure events please, BTN_TOUCH should be enough. I'd have to check, IIRC tslib needs these to function properly. Indeed it does -- otherwise it won't work. Yes you could try going around and patching TSLIB but so many people use it that it is principle-of-least-surprise to produce pressure events. + ts.input-keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH); + input_set_abs_params(ts.input, ABS_X, 0, 0x3FF, 0, 0); + input_set_abs_params(ts.input, ABS_Y, 0, 0x3FF, 0, 0); + input_set_abs_params(ts.input, ABS_PRESSURE, 0, 1, 0, 0); Drop ABS_PRESSURE. ok, see above. The same applies here -- claim ABS_PRESSURE or tslib won't operate with the touchscreen. While it is tempting to be 100% exactly correct to what the hardware reports (which is only TOUCH not PRESSURE) it is preferable to work with the software which the majority of people use -- namely tslib. I would strongly argue against removing the ABS_PRESSURE stuff personally, despite it being essentially a lie. And I would strongly argue that you just need to update your tslib, especially given the fact that this issue was dealt with there about a year ago. And if you really need that fix to be in released (read 'tagged') version of tslib - please speak to its maintainer. Why everyone thinks that it is a good idea to pile workarounds for software issues in the kernel but updating the other parts of software stack is a big no-no? -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 1/1] TOUCHSCREEN: S3C24XX touchscreen driver from Arnaud Patard.
Hi Ben, On Wed, Nov 18, 2009 at 11:29:40PM +, Ben Dooks wrote: From: Arnaud Patard arnaud.pat...@rtp-net.org S3C24XX touchscreen driver, originally written by Arnaud Patard and other contributors. This driver is the version from the Simtec Electronics tree, and as such is the best place to start and thus proposed to be merged into the linux input system. Thank you for the patch. First thing first - do we have an agreement from all Samsung folks and other interested parties that _this_ is the driver? Because it's like 3rd implementation that came across... The driver has had substantial testing as well as a number of tidying up passes done by Ben Dooks, as noted: - added kernel-doc comments to most of the routines - removed old code from pre adc framework days - updated device probe code to use platform id list matching - cleaned up debug, since printk() now has timestamp feature - ensure code uses dev_() reporting macros where necessary Signed-off-by: Arnaud Patard arnaud.pat...@rtp-net.org Signed-off-by: Simtec Linux Team li...@simtec.co.uk Signed-off-by: Ben Dooks b...@simtec.co.uk --- drivers/input/touchscreen/Kconfig | 18 + drivers/input/touchscreen/Makefile |1 drivers/input/touchscreen/s3c2410_ts.c | 464 + 3 files changed, 483 insertions(+) Index: b/drivers/input/touchscreen/Kconfig === --- a/drivers/input/touchscreen/Kconfig 2009-11-09 22:28:23.0 + +++ b/drivers/input/touchscreen/Kconfig 2009-11-10 10:38:44.0 + @@ -133,6 +133,24 @@ config TOUCHSCREEN_FUJITSU To compile this driver as a module, choose M here: the module will be called fujitsu-ts. +config TOUCHSCREEN_S3C2410 + tristate Samsung S3C2410 touchscreen input driver + depends on ARCH_S3C2410 INPUT INPUT_TOUCHSCREEN INPUT INPUT_TOUCHSCREEN are superfluous. + select SERIO I don't think you need SERIO + help + Say Y here if you have the s3c2410 touchscreen. + + If unsure, say N. + + To compile this driver as a module, choose M here: the + module will be called s3c2410_ts. + +config TOUCHSCREEN_S3C2410_DEBUG + boolean Samsung S3C2410 touchscreen debug messages + depends on TOUCHSCREEN_S3C2410 + help + Select this if you want debug messages Where is this used? + config TOUCHSCREEN_GUNZE tristate Gunze AHL-51S touchscreen select SERIO Index: b/drivers/input/touchscreen/Makefile === --- a/drivers/input/touchscreen/Makefile 2009-11-09 22:28:23.0 + +++ b/drivers/input/touchscreen/Makefile 2009-11-10 10:38:44.0 + @@ -26,6 +26,7 @@ obj-$(CONFIG_TOUCHSCREEN_HP7XX) += jorn obj-$(CONFIG_TOUCHSCREEN_HTCPEN) += htcpen.o obj-$(CONFIG_TOUCHSCREEN_USB_COMPOSITE) += usbtouchscreen.o obj-$(CONFIG_TOUCHSCREEN_PENMOUNT) += penmount.o +obj-$(CONFIG_TOUCHSCREEN_S3C2410)+= s3c2410_ts.o obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213) += touchit213.o obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT) += touchright.o obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN) += touchwin.o Index: b/drivers/input/touchscreen/s3c2410_ts.c === --- /dev/null 1970-01-01 00:00:00.0 + +++ b/drivers/input/touchscreen/s3c2410_ts.c 2009-11-10 10:47:38.0 + @@ -0,0 +1,464 @@ +/* drivers/input/touchscreen/s3c2410_ts.c + * + * Samsung S3C24XX touchscreen driver + * + * This program is free software; you can redistribute it and/or modify + * it under the term of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * 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. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Copyright 2004 Arnaud Patard arnaud.pat...@rtp-net.org + * Copyright 2008 Ben Dooks ben-li...@fluff.org + * Copyright 2009 Simtec Electronics li...@simtec.co.uk + * + * Additional work by Herbert Pötzl herb...@13thfloor.at and + * Harald Welte lafo...@openmoko.org + */ + +#include linux/errno.h +#include linux/kernel.h +#include linux/module.h +#include linux/slab.h +#include linux/gpio.h +#include linux/input.h +#include linux/init.h +#include linux/serio.h No serio in sight. +#include linux/delay.h +#include linux/interrupt.h +#include
