Re: [meta-intel] [PATCH V3 1/1] formfactor: detect USB HID keyboard and touch screen

2016-06-16 Thread Saul Wold 
On Mon, 2016-06-13 at 10:00 -0700, Jianxun Zhang wrote:
> > 
> > On Jun 13, 2016, at 8:25 AM, Saul Wold  wrote:
> > 
> > On Fri, 2016-06-10 at 13:17 -0700, Jianxun Zhang wrote:
> > > 
> > > The new machconfig probes USB keyboard and touch screen, and
> > > then sets HAVE_* variables according to detection.
> > > 
> > > Detectable devices:
> > > USB HID keyboards (Generic Desktop)
> > > USB HID touch screens (Digitizer)
> > > 
> > > Note:
> > > The intention is to have a way to provide initial formfactor
> > > settings in a boot procedure. That means supported keyboard
> > > and touch screen must be connected before machconfig runs.
> > > Any new connection or disconnection won't be detected until
> > > machconfig is executed again.
> > > 
> > > Limitation:
> > > There could be some USB HID devices presents more than one
> > > usage in a single descriptor. We will add support once such
> > > device emerges.
> > > 
> > > Some platforms may have _virtual_ devices provided by BIOS.
> > > It will cause false detection when they are presented as
> > > types we supported. We can add black list logic when it
> > > becomes a big concern.
> > > 
> > > Fixes [YOCTO #9205]
> > > 
> > > Signed-off-by: Jianxun Zhang 
> > > ---
> > > Tom & Saul,
> > > V3 puts entire logic in machconfig into a new if block, so
> > > that detection will be skipped when any error occurs at the
> > > first step. Same basic test is done as before.
> > > 
> > > Let me know if this version makes more sense.
> > > 
> > > Thanks
> > > 
> > >  .../recipes-bsp/formfactor/formfactor/machconfig   | 37
> > > ++
> > >  .../recipes-bsp/formfactor/formfactor_0.0.bbappend |  1 +
> > >  2 files changed, 38 insertions(+)
> > >  create mode 100644 common/recipes-
> > > bsp/formfactor/formfactor/machconfig
> > >  create mode 100644 common/recipes-
> > > bsp/formfactor/formfactor_0.0.bbappend
> > > 
> > > diff --git a/common/recipes-bsp/formfactor/formfactor/machconfig
> > > b/common/recipes-bsp/formfactor/formfactor/machconfig
> > > new file mode 100644
> > > index 000..22d3112
> > > --- /dev/null
> > > +++ b/common/recipes-bsp/formfactor/formfactor/machconfig
> > > @@ -0,0 +1,37 @@
> > > +# Note: super user permission is required to run usbhid-dump
> > > +# successfully.
> > > +
> > > +# HEX keys are according to USB HID spec and USB HID usage table
> > > +# We could add more keys as needed in the future.
> > > +
> > > +# It may not be very accurate. Here we only look for first two
> > > lines
> > > +# of a descriptor section. Example:
> > > +#
> > > +# 001:003:000:DESCRIPTOR 1460501386.337809
> > > +#  05 01 09 02 A1 01 09 01 A1 00 05 09 19 01 29 03
> > > +#  15 00 25 01 95 03 75 01 81 02 .. .. .. .. .. ..
> > > +#  .. .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
> > > +#
> > > +# By doing so we elimiate false matches when HEX keys are in the
> > > lines
> > > +# in the middle of whole descriptor section.
> > > +
> > > +if USBHID_DUMP_OUTPUT=$(usbhid-dump -e descriptor
> > > 2>/dev/null|grep
> > > -A1 DESCRIPTOR); then
> > Jianxun, 
> > 
> > I think you missed the intent of the if test, while this could work
> > it's not clear from this line why the "if" succeeds or fails.  Both
> > Tom
> > and I suggested checking for the existence of usbhid-dump first,
> > that
> > way it's clear why the rest of the code is not used.
> > 
> Saul,
> I understand you want to check usbhid-dump’ s existence first, but
> then I realize I should skip whole detection for any other errors too
> (a bigger issue in V1&2). That’s why I combined errors into single
> command in my mind. The logic (well, implicitly) covers that error
> case already so I don’t need another check. What we missed when cmd
> doesn’t exist, is the error code which I thought we don’t really
> care.
> 
> I will revise it with a check of cmd existence first, just let you
> know my thoughts behind this V3.
> 
Jianxun,

While I understand the logic, I think it's would be cleaner to have the
actual test separated out that way it's obvious why the script is
bailing out.

We look forward to v4.

Thanks
Sau!

> Thanks
> 
> > 
> > Thanks
> >    Sau!
> > 
> > > 
> > > +# checker for generic USB HID keyboard
> > > +USBHID_KBD_CMD="grep -E '^ 05 01 09 06'"
> > > +
> > > +# checker for touch screen
> > > +USBHID_TS_CMD="grep -E '^ 05 0D 09 04'"
> > > +
> > > +if echo "$USBHID_DUMP_OUTPUT"|eval $USBHID_TS_CMD
> > > &>/dev/null;
> > > then
> > > + HAVE_TOUCHSCREEN=1
> > > +fi
> > > +
> > > +if echo "$USBHID_DUMP_OUTPUT"|eval $USBHID_KBD_CMD
> > > &>/dev/null;
> > > then
> > > + HAVE_KEYBOARD=1
> > > +else
> > > + # config script in OE will set HAVE_KEYBOARD=1
> > > + # if we don't set any value. We have to explicitly
> > > + # tell it when keyboard is not detected.
> > > + HAVE_KEYBOARD=0
> > > +fi
> > > +fi
> > > diff --git a/common/recipes-
> > > bsp/formfactor/formfactor_0.0.bbappend
> > >

Re: [meta-intel] [PATCH V3 1/1] formfactor: detect USB HID keyboard and touch screen

2016-06-13 Thread Jianxun Zhang 

> On Jun 13, 2016, at 8:25 AM, Saul Wold  wrote:
> 
> On Fri, 2016-06-10 at 13:17 -0700, Jianxun Zhang wrote:
>> The new machconfig probes USB keyboard and touch screen, and
>> then sets HAVE_* variables according to detection.
>> 
>> Detectable devices:
>> USB HID keyboards (Generic Desktop)
>> USB HID touch screens (Digitizer)
>> 
>> Note:
>> The intention is to have a way to provide initial formfactor
>> settings in a boot procedure. That means supported keyboard
>> and touch screen must be connected before machconfig runs.
>> Any new connection or disconnection won't be detected until
>> machconfig is executed again.
>> 
>> Limitation:
>> There could be some USB HID devices presents more than one
>> usage in a single descriptor. We will add support once such
>> device emerges.
>> 
>> Some platforms may have _virtual_ devices provided by BIOS.
>> It will cause false detection when they are presented as
>> types we supported. We can add black list logic when it
>> becomes a big concern.
>> 
>> Fixes [YOCTO #9205]
>> 
>> Signed-off-by: Jianxun Zhang 
>> ---
>> Tom & Saul,
>> V3 puts entire logic in machconfig into a new if block, so
>> that detection will be skipped when any error occurs at the
>> first step. Same basic test is done as before.
>> 
>> Let me know if this version makes more sense.
>> 
>> Thanks
>> 
>>  .../recipes-bsp/formfactor/formfactor/machconfig   | 37
>> ++
>>  .../recipes-bsp/formfactor/formfactor_0.0.bbappend |  1 +
>>  2 files changed, 38 insertions(+)
>>  create mode 100644 common/recipes-
>> bsp/formfactor/formfactor/machconfig
>>  create mode 100644 common/recipes-
>> bsp/formfactor/formfactor_0.0.bbappend
>> 
>> diff --git a/common/recipes-bsp/formfactor/formfactor/machconfig
>> b/common/recipes-bsp/formfactor/formfactor/machconfig
>> new file mode 100644
>> index 000..22d3112
>> --- /dev/null
>> +++ b/common/recipes-bsp/formfactor/formfactor/machconfig
>> @@ -0,0 +1,37 @@
>> +# Note: super user permission is required to run usbhid-dump
>> +# successfully.
>> +
>> +# HEX keys are according to USB HID spec and USB HID usage table
>> +# We could add more keys as needed in the future.
>> +
>> +# It may not be very accurate. Here we only look for first two lines
>> +# of a descriptor section. Example:
>> +#
>> +# 001:003:000:DESCRIPTOR 1460501386.337809
>> +#  05 01 09 02 A1 01 09 01 A1 00 05 09 19 01 29 03
>> +#  15 00 25 01 95 03 75 01 81 02 .. .. .. .. .. ..
>> +#  .. .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
>> +#
>> +# By doing so we elimiate false matches when HEX keys are in the
>> lines
>> +# in the middle of whole descriptor section.
>> +
>> +if USBHID_DUMP_OUTPUT=$(usbhid-dump -e descriptor 2>/dev/null|grep
>> -A1 DESCRIPTOR); then
> Jianxun, 
> 
> I think you missed the intent of the if test, while this could work
> it's not clear from this line why the "if" succeeds or fails.  Both Tom
> and I suggested checking for the existence of usbhid-dump first, that
> way it's clear why the rest of the code is not used.
> 

Saul,
I understand you want to check usbhid-dump’ s existence first, but then I 
realize I should skip whole detection for any other errors too (a bigger issue 
in V1&2). That’s why I combined errors into single command in my mind. The 
logic (well, implicitly) covers that error case already so I don’t need another 
check. What we missed when cmd doesn’t exist, is the error code which I thought 
we don’t really care.

I will revise it with a check of cmd existence first, just let you know my 
thoughts behind this V3.

Thanks

> Thanks
>Sau!
> 
>> +# checker for generic USB HID keyboard
>> +USBHID_KBD_CMD="grep -E '^ 05 01 09 06'"
>> +
>> +# checker for touch screen
>> +USBHID_TS_CMD="grep -E '^ 05 0D 09 04'"
>> +
>> +if echo "$USBHID_DUMP_OUTPUT"|eval $USBHID_TS_CMD &>/dev/null;
>> then
>> +HAVE_TOUCHSCREEN=1
>> +fi
>> +
>> +if echo "$USBHID_DUMP_OUTPUT"|eval $USBHID_KBD_CMD &>/dev/null;
>> then
>> +HAVE_KEYBOARD=1
>> +else
>> +# config script in OE will set HAVE_KEYBOARD=1
>> +# if we don't set any value. We have to explicitly
>> +# tell it when keyboard is not detected.
>> +HAVE_KEYBOARD=0
>> +fi
>> +fi
>> diff --git a/common/recipes-bsp/formfactor/formfactor_0.0.bbappend
>> b/common/recipes-bsp/formfactor/formfactor_0.0.bbappend
>> new file mode 100644
>> index 000..72d991c
>> --- /dev/null
>> +++ b/common/recipes-bsp/formfactor/formfactor_0.0.bbappend
>> @@ -0,0 +1 @@
>> +FILESEXTRAPATHS_prepend := "${THISDIR}/${PN}:"

-- 
___
meta-intel mailing list
meta-intel@yoctoproject.org
https://lists.yoctoproject.org/listinfo/meta-intel

Re: [meta-intel] [PATCH V3 1/1] formfactor: detect USB HID keyboard and touch screen

2016-06-13 Thread Saul Wold 
On Fri, 2016-06-10 at 13:17 -0700, Jianxun Zhang wrote:
> The new machconfig probes USB keyboard and touch screen, and
> then sets HAVE_* variables according to detection.
> 
> Detectable devices:
> USB HID keyboards (Generic Desktop)
> USB HID touch screens (Digitizer)
> 
> Note:
> The intention is to have a way to provide initial formfactor
> settings in a boot procedure. That means supported keyboard
> and touch screen must be connected before machconfig runs.
> Any new connection or disconnection won't be detected until
> machconfig is executed again.
> 
> Limitation:
> There could be some USB HID devices presents more than one
> usage in a single descriptor. We will add support once such
> device emerges.
> 
> Some platforms may have _virtual_ devices provided by BIOS.
> It will cause false detection when they are presented as
> types we supported. We can add black list logic when it
> becomes a big concern.
> 
> Fixes [YOCTO #9205]
> 
> Signed-off-by: Jianxun Zhang 
> ---
> Tom & Saul,
> V3 puts entire logic in machconfig into a new if block, so
> that detection will be skipped when any error occurs at the
> first step. Same basic test is done as before.
> 
> Let me know if this version makes more sense.
> 
> Thanks
> 
>  .../recipes-bsp/formfactor/formfactor/machconfig   | 37
> ++
>  .../recipes-bsp/formfactor/formfactor_0.0.bbappend |  1 +
>  2 files changed, 38 insertions(+)
>  create mode 100644 common/recipes-
> bsp/formfactor/formfactor/machconfig
>  create mode 100644 common/recipes-
> bsp/formfactor/formfactor_0.0.bbappend
> 
> diff --git a/common/recipes-bsp/formfactor/formfactor/machconfig
> b/common/recipes-bsp/formfactor/formfactor/machconfig
> new file mode 100644
> index 000..22d3112
> --- /dev/null
> +++ b/common/recipes-bsp/formfactor/formfactor/machconfig
> @@ -0,0 +1,37 @@
> +# Note: super user permission is required to run usbhid-dump
> +# successfully.
> +
> +# HEX keys are according to USB HID spec and USB HID usage table
> +# We could add more keys as needed in the future.
> +
> +# It may not be very accurate. Here we only look for first two lines
> +# of a descriptor section. Example:
> +#
> +# 001:003:000:DESCRIPTOR 1460501386.337809
> +#  05 01 09 02 A1 01 09 01 A1 00 05 09 19 01 29 03
> +#  15 00 25 01 95 03 75 01 81 02 .. .. .. .. .. ..
> +#  .. .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
> +#
> +# By doing so we elimiate false matches when HEX keys are in the
> lines
> +# in the middle of whole descriptor section.
> +
> +if USBHID_DUMP_OUTPUT=$(usbhid-dump -e descriptor 2>/dev/null|grep
> -A1 DESCRIPTOR); then
Jianxun, 

I think you missed the intent of the if test, while this could work
it's not clear from this line why the "if" succeeds or fails.  Both Tom
and I suggested checking for the existence of usbhid-dump first, that
way it's clear why the rest of the code is not used.

Thanks
   Sau!

> +# checker for generic USB HID keyboard
> +USBHID_KBD_CMD="grep -E '^ 05 01 09 06'"
> +
> +# checker for touch screen
> +USBHID_TS_CMD="grep -E '^ 05 0D 09 04'"
> +
> +if echo "$USBHID_DUMP_OUTPUT"|eval $USBHID_TS_CMD &>/dev/null;
> then
> + HAVE_TOUCHSCREEN=1
> +fi
> +
> +if echo "$USBHID_DUMP_OUTPUT"|eval $USBHID_KBD_CMD &>/dev/null;
> then
> + HAVE_KEYBOARD=1
> +else
> + # config script in OE will set HAVE_KEYBOARD=1
> + # if we don't set any value. We have to explicitly
> + # tell it when keyboard is not detected.
> + HAVE_KEYBOARD=0
> +fi
> +fi
> diff --git a/common/recipes-bsp/formfactor/formfactor_0.0.bbappend
> b/common/recipes-bsp/formfactor/formfactor_0.0.bbappend
> new file mode 100644
> index 000..72d991c
> --- /dev/null
> +++ b/common/recipes-bsp/formfactor/formfactor_0.0.bbappend
> @@ -0,0 +1 @@
> +FILESEXTRAPATHS_prepend := "${THISDIR}/${PN}:"
-- 
___
meta-intel mailing list
meta-intel@yoctoproject.org
https://lists.yoctoproject.org/listinfo/meta-intel

[meta-intel] [PATCH V3 1/1] formfactor: detect USB HID keyboard and touch screen

2016-06-10 Thread Jianxun Zhang 
The new machconfig probes USB keyboard and touch screen, and
then sets HAVE_* variables according to detection.

Detectable devices:
USB HID keyboards (Generic Desktop)
USB HID touch screens (Digitizer)

Note:
The intention is to have a way to provide initial formfactor
settings in a boot procedure. That means supported keyboard
and touch screen must be connected before machconfig runs.
Any new connection or disconnection won't be detected until
machconfig is executed again.

Limitation:
There could be some USB HID devices presents more than one
usage in a single descriptor. We will add support once such
device emerges.

Some platforms may have _virtual_ devices provided by BIOS.
It will cause false detection when they are presented as
types we supported. We can add black list logic when it
becomes a big concern.

Fixes [YOCTO #9205]

Signed-off-by: Jianxun Zhang 
---
Tom & Saul,
V3 puts entire logic in machconfig into a new if block, so
that detection will be skipped when any error occurs at the
first step. Same basic test is done as before.

Let me know if this version makes more sense.

Thanks

 .../recipes-bsp/formfactor/formfactor/machconfig   | 37 ++
 .../recipes-bsp/formfactor/formfactor_0.0.bbappend |  1 +
 2 files changed, 38 insertions(+)
 create mode 100644 common/recipes-bsp/formfactor/formfactor/machconfig
 create mode 100644 common/recipes-bsp/formfactor/formfactor_0.0.bbappend

diff --git a/common/recipes-bsp/formfactor/formfactor/machconfig 
b/common/recipes-bsp/formfactor/formfactor/machconfig
new file mode 100644
index 000..22d3112
--- /dev/null
+++ b/common/recipes-bsp/formfactor/formfactor/machconfig
@@ -0,0 +1,37 @@
+# Note: super user permission is required to run usbhid-dump
+# successfully.
+
+# HEX keys are according to USB HID spec and USB HID usage table
+# We could add more keys as needed in the future.
+
+# It may not be very accurate. Here we only look for first two lines
+# of a descriptor section. Example:
+#
+# 001:003:000:DESCRIPTOR 1460501386.337809
+#  05 01 09 02 A1 01 09 01 A1 00 05 09 19 01 29 03
+#  15 00 25 01 95 03 75 01 81 02 .. .. .. .. .. ..
+#  .. .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
+#
+# By doing so we elimiate false matches when HEX keys are in the lines
+# in the middle of whole descriptor section.
+
+if USBHID_DUMP_OUTPUT=$(usbhid-dump -e descriptor 2>/dev/null|grep -A1 
DESCRIPTOR); then
+# checker for generic USB HID keyboard
+USBHID_KBD_CMD="grep -E '^ 05 01 09 06'"
+
+# checker for touch screen
+USBHID_TS_CMD="grep -E '^ 05 0D 09 04'"
+
+if echo "$USBHID_DUMP_OUTPUT"|eval $USBHID_TS_CMD &>/dev/null; then
+   HAVE_TOUCHSCREEN=1
+fi
+
+if echo "$USBHID_DUMP_OUTPUT"|eval $USBHID_KBD_CMD &>/dev/null; then
+   HAVE_KEYBOARD=1
+else
+   # config script in OE will set HAVE_KEYBOARD=1
+   # if we don't set any value. We have to explicitly
+   # tell it when keyboard is not detected.
+   HAVE_KEYBOARD=0
+fi
+fi
diff --git a/common/recipes-bsp/formfactor/formfactor_0.0.bbappend 
b/common/recipes-bsp/formfactor/formfactor_0.0.bbappend
new file mode 100644
index 000..72d991c
--- /dev/null
+++ b/common/recipes-bsp/formfactor/formfactor_0.0.bbappend
@@ -0,0 +1 @@
+FILESEXTRAPATHS_prepend := "${THISDIR}/${PN}:"
-- 
2.7.4

-- 
___
meta-intel mailing list
meta-intel@yoctoproject.org
https://lists.yoctoproject.org/listinfo/meta-intel