On Sunday 10 September 2006 16:10, Zephaniah E. Hull wrote:
> Take two, with most of the items people commented about addressed.
>
Hi Zephaniah,
I have couple more comments/requests:
>
> +
> + if (gs_down) {
> + input_report_abs(dev2, ABS_X, gx);
> + input_report_abs(dev2, ABS_Y, gy);
> + }
> + input_report_abs(dev2, ABS_PRESSURE, gz);
> +
> + if (pt_down) {
> + input_report_abs(dev, ABS_X, px);
> + input_report_abs(dev, ABS_Y, py);
> + }
> +
> + input_sync(dev);
Please add input_sync(dev2);
> +}
> +
> +static psmouse_ret_t olpc_process_byte(struct psmouse *psmouse, struct pt_regs *regs)
> +{
> + struct olpc_data *priv = psmouse->private;
> + psmouse_ret_t ret = PSMOUSE_BAD_DATA;
> +
> + if ((psmouse->packet[0] & priv->i->mask0) != priv->i->byte0) {
> + ret = PSMOUSE_BAD_DATA;
It looks like you can kill "ret = PSMOUSE_BAD_DATA" assignments since you
initialize ret with it.
> + goto out;
> + }
> +
> + /* Bytes 2 - 9 should have 0 in the highest bit */
> + if (psmouse->pktcnt >= 2 && psmouse->pktcnt <= 9 &&
> + (psmouse->packet[psmouse->pktcnt - 1] & 0x80)) {
> + ret = PSMOUSE_BAD_DATA;
> + goto out;
> + }
I'd like to have standard identation throughout the driver (and input
sybsystem in general).
> +
> +#ifndef _OLPC_H
> +#define _OLPC_H
> +
> +int olpc_detect(struct psmouse *psmouse, int set_properties);
> +int olpc_init(struct psmouse *psmouse);
> +
> +struct olpc_model_info {
> + unsigned char signature[3];
> + unsigned char byte0, mask0;
> + unsigned char flags;
> +};
Hard TABs for identation please.
> +
> +struct olpc_data {
> + struct input_dev *dev2; /* Relative device */
> + char name[32]; /* Name */
> + char phys[32]; /* Phys */
> + const struct olpc_model_info *i;/* Info */
> +};
> +
> +
> +#endif
> diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
> index 8bc9f51..20060b0 100644
> --- a/drivers/input/mouse/psmouse-base.c
> +++ b/drivers/input/mouse/psmouse-base.c
> @@ -26,6 +26,7 @@
> #include "synaptics.h"
> #include "logips2pp.h"
> #include "alps.h"
> +#include "olpc.h"
> #include "lifebook.h"
> #include "trackpoint.h"
>
> @@ -616,6 +617,15 @@ static int psmouse_extensions(struct psm
> */
> max_proto = PSMOUSE_IMEX;
> }
> + ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
Do we have to do 2nd reset here? Plus logic seems a bit fuzzy here -
if ALPS is detected but initizliztion fails it will start OLPC detection
which is probably not what you wanted...
> + if (olpc_detect(psmouse, set_properties) == 0) {
> + if (!set_properties || olpc_init(psmouse) == 0)
> + return PSMOUSE_OLPC;
> +/*
> + * Init failed, try basic relative protocols
> + */
> + max_proto = PSMOUSE_IMEX;
> + }
> }
>
> if (max_proto > PSMOUSE_IMEX && genius_detect(psmouse, set_properties) == 0)
> @@ -726,6 +736,13 @@ static struct psmouse_protocol psmouse_p
> .detect = trackpoint_detect,
> },
> {
> + .type = PSMOUSE_OLPC,
> + .name = "OLPC",
> + .alias = "olpc",
> + .maxproto = 1,
Do not set maxproto on speciality protocols. It is meant to limit highest
version of standard protocols to be probed/used by a device.
--
Dmitry
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
