RE: [PATCH 5/7] input: cyapa: add proximity and interrupt sysfs interfaces support
Dmitry,
Thanks for the review.
The purpose of adding the proximity interface is:
1. User can apply the use of the proximity function as they want.
Some productions may need the proximity function, but some may not.
And for product, the requirements may be also different in the different
usage time.
so through this interface, user can apply their specific strategy to use the
proximity function as required instead of the default behaver.
The purpose of adding the interrupt interface is:
1. For temporary disable the trackpad device, user can disable the trackpad
device to report data through the interrupt interface.
And keep the trackpad device scanning when the data report is disabled, so
when re-enable the report data of the device,
the trackpad device can be re-enabled immediately without the disturb
re-initialize processing.
It may helpful in firmware debugging.
Thanks,
Dudley
> -Original Message-
> From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
> Sent: 2015?6?12? 19:10
> To: Dudley Du
> Cc: mark.rutl...@arm.com; robh...@kernel.org; rydb...@euromail.se;
> ble...@google.com; jmmah...@gmail.com; devicet...@vger.kernel.org;
> linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 5/7] input: cyapa: add proximity and interrupt sysfs
> interfaces
> support
>
> Hi Dudley,
>
> On Fri, Jun 12, 2015 at 02:56:36PM +0800, Dudley Du wrote:
> > Add proximity and interrupt control interfaces in sysfs device node.
> > The proximity interface is used to disable/enable proximity function in
> > device.
> > The interrupt interface is used to disbale/enable interrupt from device.
>
> Please explain why you feel that these sysfs interfaces are needed. Why
> would one want to disable interrupt for Gen6 devices? And why do we
> need to enable/disable proximity function? I'd expect kernel provide the
> data and clients to decide if they want to use it or not...
>
> Thanks.
>
> > TEST=test on Chromebook.
> >
> > Signed-off-by: Dudley Du
> > ---
> > drivers/input/mouse/cyapa.c | 101
> +++
> > drivers/input/mouse/cyapa.h | 4 ++
> > drivers/input/mouse/cyapa_gen3.c | 1 +
> > drivers/input/mouse/cyapa_gen5.c | 2 +
> > drivers/input/mouse/cyapa_gen6.c | 14 ++
> > 5 files changed, 122 insertions(+)
> >
> > diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
> > index faead4d..86f2263 100644
> > --- a/drivers/input/mouse/cyapa.c
> > +++ b/drivers/input/mouse/cyapa.c
> > @@ -594,6 +594,7 @@ static int cyapa_initialize(struct cyapa *cyapa)
> >
> > cyapa->state = CYAPA_STATE_NO_DEVICE;
> > cyapa->gen = CYAPA_GEN_UNKNOWN;
> > +cyapa->interrupt = true;
> > mutex_init(>state_sync_lock);
> >
> > /*
> > @@ -1217,12 +1218,110 @@ static ssize_t cyapa_show_mode(struct device
> *dev,
> > return size;
> > }
> >
> > +static ssize_t cyapa_show_interrupt(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > +struct cyapa *cyapa = dev_get_drvdata(dev);
> > +int size;
> > +int error;
> > +
> > +error = mutex_lock_interruptible(>state_sync_lock);
> > +if (error)
> > +return error;
> > +
> > +size = scnprintf(buf, PAGE_SIZE, "%d\n", cyapa->interrupt ? 1 : 0);
> > +
> > +mutex_unlock(>state_sync_lock);
> > +return size;
> > +}
> > +
> > +static ssize_t cyapa_interrupt_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > +struct cyapa *cyapa = dev_get_drvdata(dev);
> > +u16 value;
> > +int error;
> > +
> > +if (cyapa->gen < CYAPA_GEN6)
> > +return -EOPNOTSUPP;
> > +
> > +if (kstrtou16(buf, 10, )) {
> > +dev_err(dev, "invalid interrupt set parameter\n");
> > +return -EINVAL;
> > +}
> > +
> > +error = mutex_lock_interruptible(>state_sync_lock);
> > +if (error)
> > +return error;
> > +
> > +if (cyapa->operational)
> > +error = cyapa->ops->set_interrupt(cyapa,
> > + value ? true : false);
> > +else
> > +error = -EBUSY; /* Still running in bootloader mode. */
> > +
> > +mutex_unlock(>state_sync_lock);
> > +return error < 0 ? error : count;
> > +}
> > +
> > +static ssize_t cyapa_show_proximity(struct device *dev,
> > + struct device_attribute *attr, char *buf)
> > +{
> > +struct cyapa *cyapa = dev_get_drvdata(dev);
> > +int size;
> > +int error;
> > +
> >
RE: [PATCH 5/7] input: cyapa: add proximity and interrupt sysfs interfaces support
Dmitry,
Thanks for the review.
The purpose of adding the proximity interface is:
1. User can apply the use of the proximity function as they want.
Some productions may need the proximity function, but some may not.
And for product, the requirements may be also different in the different
usage time.
so through this interface, user can apply their specific strategy to use the
proximity function as required instead of the default behaver.
The purpose of adding the interrupt interface is:
1. For temporary disable the trackpad device, user can disable the trackpad
device to report data through the interrupt interface.
And keep the trackpad device scanning when the data report is disabled, so
when re-enable the report data of the device,
the trackpad device can be re-enabled immediately without the disturb
re-initialize processing.
It may helpful in firmware debugging.
Thanks,
Dudley
-Original Message-
From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
Sent: 2015?6?12? 19:10
To: Dudley Du
Cc: mark.rutl...@arm.com; robh...@kernel.org; rydb...@euromail.se;
ble...@google.com; jmmah...@gmail.com; devicet...@vger.kernel.org;
linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/7] input: cyapa: add proximity and interrupt sysfs
interfaces
support
Hi Dudley,
On Fri, Jun 12, 2015 at 02:56:36PM +0800, Dudley Du wrote:
Add proximity and interrupt control interfaces in sysfs device node.
The proximity interface is used to disable/enable proximity function in
device.
The interrupt interface is used to disbale/enable interrupt from device.
Please explain why you feel that these sysfs interfaces are needed. Why
would one want to disable interrupt for Gen6 devices? And why do we
need to enable/disable proximity function? I'd expect kernel provide the
data and clients to decide if they want to use it or not...
Thanks.
TEST=test on Chromebook.
Signed-off-by: Dudley Du d...@cypress.com
---
drivers/input/mouse/cyapa.c | 101
+++
drivers/input/mouse/cyapa.h | 4 ++
drivers/input/mouse/cyapa_gen3.c | 1 +
drivers/input/mouse/cyapa_gen5.c | 2 +
drivers/input/mouse/cyapa_gen6.c | 14 ++
5 files changed, 122 insertions(+)
diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
index faead4d..86f2263 100644
--- a/drivers/input/mouse/cyapa.c
+++ b/drivers/input/mouse/cyapa.c
@@ -594,6 +594,7 @@ static int cyapa_initialize(struct cyapa *cyapa)
cyapa-state = CYAPA_STATE_NO_DEVICE;
cyapa-gen = CYAPA_GEN_UNKNOWN;
+cyapa-interrupt = true;
mutex_init(cyapa-state_sync_lock);
/*
@@ -1217,12 +1218,110 @@ static ssize_t cyapa_show_mode(struct device
*dev,
return size;
}
+static ssize_t cyapa_show_interrupt(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+struct cyapa *cyapa = dev_get_drvdata(dev);
+int size;
+int error;
+
+error = mutex_lock_interruptible(cyapa-state_sync_lock);
+if (error)
+return error;
+
+size = scnprintf(buf, PAGE_SIZE, %d\n, cyapa-interrupt ? 1 : 0);
+
+mutex_unlock(cyapa-state_sync_lock);
+return size;
+}
+
+static ssize_t cyapa_interrupt_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+struct cyapa *cyapa = dev_get_drvdata(dev);
+u16 value;
+int error;
+
+if (cyapa-gen CYAPA_GEN6)
+return -EOPNOTSUPP;
+
+if (kstrtou16(buf, 10, value)) {
+dev_err(dev, invalid interrupt set parameter\n);
+return -EINVAL;
+}
+
+error = mutex_lock_interruptible(cyapa-state_sync_lock);
+if (error)
+return error;
+
+if (cyapa-operational)
+error = cyapa-ops-set_interrupt(cyapa,
+ value ? true : false);
+else
+error = -EBUSY; /* Still running in bootloader mode. */
+
+mutex_unlock(cyapa-state_sync_lock);
+return error 0 ? error : count;
+}
+
+static ssize_t cyapa_show_proximity(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+struct cyapa *cyapa = dev_get_drvdata(dev);
+int size;
+int error;
+
+error = mutex_lock_interruptible(cyapa-state_sync_lock);
+if (error)
+return error;
+
+size = scnprintf(buf, PAGE_SIZE, %d\n, cyapa-proximity ? 1 : 0);
+
+mutex_unlock(cyapa-state_sync_lock);
+return size;
+}
+
+static ssize_t cyapa_proximity_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+struct cyapa *cyapa = dev_get_drvdata(dev);
+u16 value;
+int error;
+
+if (cyapa-gen CYAPA_GEN5)
+return -EOPNOTSUPP;
+
+if (kstrtou16(buf, 10, value)) {
+dev_err(dev, invalid set value of proximity\n);
+return -EINVAL;
+}
+
+error = mutex_lock_interruptible(cyapa-state_sync_lock);
+if (error)
+return error;
+
+if (cyapa-operational)
+error = cyapa-ops-set_proximity(cyapa,
+ value ? true : false);
+else
+error
Re: [PATCH 5/7] input: cyapa: add proximity and interrupt sysfs interfaces support
Hi Dudley,
On Fri, Jun 12, 2015 at 02:56:36PM +0800, Dudley Du wrote:
> Add proximity and interrupt control interfaces in sysfs device node.
> The proximity interface is used to disable/enable proximity function in
> device.
> The interrupt interface is used to disbale/enable interrupt from device.
Please explain why you feel that these sysfs interfaces are needed. Why
would one want to disable interrupt for Gen6 devices? And why do we
need to enable/disable proximity function? I'd expect kernel provide the
data and clients to decide if they want to use it or not...
Thanks.
> TEST=test on Chromebook.
>
> Signed-off-by: Dudley Du
> ---
> drivers/input/mouse/cyapa.c | 101
> +++
> drivers/input/mouse/cyapa.h | 4 ++
> drivers/input/mouse/cyapa_gen3.c | 1 +
> drivers/input/mouse/cyapa_gen5.c | 2 +
> drivers/input/mouse/cyapa_gen6.c | 14 ++
> 5 files changed, 122 insertions(+)
>
> diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
> index faead4d..86f2263 100644
> --- a/drivers/input/mouse/cyapa.c
> +++ b/drivers/input/mouse/cyapa.c
> @@ -594,6 +594,7 @@ static int cyapa_initialize(struct cyapa *cyapa)
>
> cyapa->state = CYAPA_STATE_NO_DEVICE;
> cyapa->gen = CYAPA_GEN_UNKNOWN;
> + cyapa->interrupt = true;
> mutex_init(>state_sync_lock);
>
> /*
> @@ -1217,12 +1218,110 @@ static ssize_t cyapa_show_mode(struct device *dev,
> return size;
> }
>
> +static ssize_t cyapa_show_interrupt(struct device *dev,
> +struct device_attribute *attr, char *buf)
> +{
> + struct cyapa *cyapa = dev_get_drvdata(dev);
> + int size;
> + int error;
> +
> + error = mutex_lock_interruptible(>state_sync_lock);
> + if (error)
> + return error;
> +
> + size = scnprintf(buf, PAGE_SIZE, "%d\n", cyapa->interrupt ? 1 : 0);
> +
> + mutex_unlock(>state_sync_lock);
> + return size;
> +}
> +
> +static ssize_t cyapa_interrupt_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct cyapa *cyapa = dev_get_drvdata(dev);
> + u16 value;
> + int error;
> +
> + if (cyapa->gen < CYAPA_GEN6)
> + return -EOPNOTSUPP;
> +
> + if (kstrtou16(buf, 10, )) {
> + dev_err(dev, "invalid interrupt set parameter\n");
> + return -EINVAL;
> + }
> +
> + error = mutex_lock_interruptible(>state_sync_lock);
> + if (error)
> + return error;
> +
> + if (cyapa->operational)
> + error = cyapa->ops->set_interrupt(cyapa,
> + value ? true : false);
> + else
> + error = -EBUSY; /* Still running in bootloader mode. */
> +
> + mutex_unlock(>state_sync_lock);
> + return error < 0 ? error : count;
> +}
> +
> +static ssize_t cyapa_show_proximity(struct device *dev,
> +struct device_attribute *attr, char *buf)
> +{
> + struct cyapa *cyapa = dev_get_drvdata(dev);
> + int size;
> + int error;
> +
> + error = mutex_lock_interruptible(>state_sync_lock);
> + if (error)
> + return error;
> +
> + size = scnprintf(buf, PAGE_SIZE, "%d\n", cyapa->proximity ? 1 : 0);
> +
> + mutex_unlock(>state_sync_lock);
> + return size;
> +}
> +
> +static ssize_t cyapa_proximity_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct cyapa *cyapa = dev_get_drvdata(dev);
> + u16 value;
> + int error;
> +
> + if (cyapa->gen < CYAPA_GEN5)
> + return -EOPNOTSUPP;
> +
> + if (kstrtou16(buf, 10, )) {
> + dev_err(dev, "invalid set value of proximity\n");
> + return -EINVAL;
> + }
> +
> + error = mutex_lock_interruptible(>state_sync_lock);
> + if (error)
> + return error;
> +
> + if (cyapa->operational)
> + error = cyapa->ops->set_proximity(cyapa,
> + value ? true : false);
> + else
> + error = -EBUSY; /* Still running in bootloader mode. */
> +
> + mutex_unlock(>state_sync_lock);
> + return error < 0 ? error : count;
> +}
> +
> static DEVICE_ATTR(firmware_version, S_IRUGO, cyapa_show_fm_ver, NULL);
> static DEVICE_ATTR(product_id, S_IRUGO, cyapa_show_product_id, NULL);
> static DEVICE_ATTR(update_fw, S_IWUSR, NULL, cyapa_update_fw_store);
> static DEVICE_ATTR(baseline, S_IRUGO, cyapa_show_baseline, NULL);
> static DEVICE_ATTR(calibrate, S_IWUSR, NULL, cyapa_calibrate_store);
> static DEVICE_ATTR(mode, S_IRUGO, cyapa_show_mode, NULL);
> +static DEVICE_ATTR(interrupt, S_IRUGO | S_IWUSR,
> +cyapa_show_interrupt, cyapa_interrupt_store);
> +static
Re: [PATCH 5/7] input: cyapa: add proximity and interrupt sysfs interfaces support
Hi Dudley,
On Fri, Jun 12, 2015 at 02:56:36PM +0800, Dudley Du wrote:
Add proximity and interrupt control interfaces in sysfs device node.
The proximity interface is used to disable/enable proximity function in
device.
The interrupt interface is used to disbale/enable interrupt from device.
Please explain why you feel that these sysfs interfaces are needed. Why
would one want to disable interrupt for Gen6 devices? And why do we
need to enable/disable proximity function? I'd expect kernel provide the
data and clients to decide if they want to use it or not...
Thanks.
TEST=test on Chromebook.
Signed-off-by: Dudley Du d...@cypress.com
---
drivers/input/mouse/cyapa.c | 101
+++
drivers/input/mouse/cyapa.h | 4 ++
drivers/input/mouse/cyapa_gen3.c | 1 +
drivers/input/mouse/cyapa_gen5.c | 2 +
drivers/input/mouse/cyapa_gen6.c | 14 ++
5 files changed, 122 insertions(+)
diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
index faead4d..86f2263 100644
--- a/drivers/input/mouse/cyapa.c
+++ b/drivers/input/mouse/cyapa.c
@@ -594,6 +594,7 @@ static int cyapa_initialize(struct cyapa *cyapa)
cyapa-state = CYAPA_STATE_NO_DEVICE;
cyapa-gen = CYAPA_GEN_UNKNOWN;
+ cyapa-interrupt = true;
mutex_init(cyapa-state_sync_lock);
/*
@@ -1217,12 +1218,110 @@ static ssize_t cyapa_show_mode(struct device *dev,
return size;
}
+static ssize_t cyapa_show_interrupt(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+ struct cyapa *cyapa = dev_get_drvdata(dev);
+ int size;
+ int error;
+
+ error = mutex_lock_interruptible(cyapa-state_sync_lock);
+ if (error)
+ return error;
+
+ size = scnprintf(buf, PAGE_SIZE, %d\n, cyapa-interrupt ? 1 : 0);
+
+ mutex_unlock(cyapa-state_sync_lock);
+ return size;
+}
+
+static ssize_t cyapa_interrupt_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct cyapa *cyapa = dev_get_drvdata(dev);
+ u16 value;
+ int error;
+
+ if (cyapa-gen CYAPA_GEN6)
+ return -EOPNOTSUPP;
+
+ if (kstrtou16(buf, 10, value)) {
+ dev_err(dev, invalid interrupt set parameter\n);
+ return -EINVAL;
+ }
+
+ error = mutex_lock_interruptible(cyapa-state_sync_lock);
+ if (error)
+ return error;
+
+ if (cyapa-operational)
+ error = cyapa-ops-set_interrupt(cyapa,
+ value ? true : false);
+ else
+ error = -EBUSY; /* Still running in bootloader mode. */
+
+ mutex_unlock(cyapa-state_sync_lock);
+ return error 0 ? error : count;
+}
+
+static ssize_t cyapa_show_proximity(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+ struct cyapa *cyapa = dev_get_drvdata(dev);
+ int size;
+ int error;
+
+ error = mutex_lock_interruptible(cyapa-state_sync_lock);
+ if (error)
+ return error;
+
+ size = scnprintf(buf, PAGE_SIZE, %d\n, cyapa-proximity ? 1 : 0);
+
+ mutex_unlock(cyapa-state_sync_lock);
+ return size;
+}
+
+static ssize_t cyapa_proximity_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct cyapa *cyapa = dev_get_drvdata(dev);
+ u16 value;
+ int error;
+
+ if (cyapa-gen CYAPA_GEN5)
+ return -EOPNOTSUPP;
+
+ if (kstrtou16(buf, 10, value)) {
+ dev_err(dev, invalid set value of proximity\n);
+ return -EINVAL;
+ }
+
+ error = mutex_lock_interruptible(cyapa-state_sync_lock);
+ if (error)
+ return error;
+
+ if (cyapa-operational)
+ error = cyapa-ops-set_proximity(cyapa,
+ value ? true : false);
+ else
+ error = -EBUSY; /* Still running in bootloader mode. */
+
+ mutex_unlock(cyapa-state_sync_lock);
+ return error 0 ? error : count;
+}
+
static DEVICE_ATTR(firmware_version, S_IRUGO, cyapa_show_fm_ver, NULL);
static DEVICE_ATTR(product_id, S_IRUGO, cyapa_show_product_id, NULL);
static DEVICE_ATTR(update_fw, S_IWUSR, NULL, cyapa_update_fw_store);
static DEVICE_ATTR(baseline, S_IRUGO, cyapa_show_baseline, NULL);
static DEVICE_ATTR(calibrate, S_IWUSR, NULL, cyapa_calibrate_store);
static DEVICE_ATTR(mode, S_IRUGO, cyapa_show_mode, NULL);
+static DEVICE_ATTR(interrupt, S_IRUGO | S_IWUSR,
+cyapa_show_interrupt, cyapa_interrupt_store);
+static DEVICE_ATTR(proximity, S_IRUGO | S_IWUSR,
+cyapa_show_proximity,
