Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1419974 > unrolled thread
| Started by | Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> |
|---|---|
| First post | 2016-06-11 14:20 +0200 |
| Last post | 2016-06-20 16:30 +0200 |
| Articles | 8 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH 0/6] Intel Integrated Sensor Hub Support (ISH) Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> - 2016-06-11 14:20 +0200
Re: [PATCH 0/6] Intel Integrated Sensor Hub Support (ISH) Grant Likely <grant.likely@secretlab.ca> - 2016-06-16 21:20 +0200
Re: [PATCH 2/6] hid: intel_ish-hid: ISH Transport layer Jiri Kosina <jikos@kernel.org> - 2016-06-17 22:50 +0200
Re: [PATCH 2/6] hid: intel_ish-hid: ISH Transport layer Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> - 2016-06-17 23:10 +0200
Re: [PATCH 2/6] hid: intel_ish-hid: ISH Transport layer Jiri Kosina <jikos@kernel.org> - 2016-06-17 22:50 +0200
Re: [PATCH 2/6] hid: intel_ish-hid: ISH Transport layer Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> - 2016-06-17 23:20 +0200
Re: [PATCH 2/6] hid: intel_ish-hid: ISH Transport layer Jiri Kosina <jikos@kernel.org> - 2016-06-20 11:40 +0200
Re: [PATCH 2/6] hid: intel_ish-hid: ISH Transport layer One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-06-20 16:30 +0200
| From | Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> |
|---|---|
| Date | 2016-06-11 14:20 +0200 |
| Subject | [PATCH 0/6] Intel Integrated Sensor Hub Support (ISH) |
| Message-ID | <rIQ3n-2Yh-3@gated-at.bofh.it> |
Starting from Cherrytrail, multiple generation of Intel processors offers on package sensor hub. Several recent tablets, 2-in-1 convertible laptops are using ISH instead of external sensor hubs. This resulted in lack of support of sensor function like device rotation and auto backlight adjustment. In addition, depending on the OEM implementation, support of ISH is required to support low power sleep states. The support of ISH on Linux platforms is not new. Android platforms with Intel SoCs had this support for a while submitted by Daniel Drubin. This patcheset is reusing most of those changes with clean up and removing Android platform specific changes. The user mode ABI is still same as external sensor hubs using Linux IIO. So existing user mode software should still work. This series primarily brings in new HID transport used in ISH. Thanks to the community members who tested RFC patches and provided feedback. For users testing on Linux distributions using IIO sensor proxy, a short term work around is required till we have debugged this issue. In systemd unit file iio-sensor-proxy.service In the section "[Unit]" add After=multi-user.target Daniel Drubin (3): hid: intel_ish-hid: ISH Transport layer hid: intel-ish-hid: ipc layer hid: intel-ish-hid: ISH HID client driver Srinivas Pandruvada (3): Documentation: hid: Intel ISH HID document iio: hid-sensors: use asynchronous resume hid: hid-sensor-hub: Add ISH quirk Documentation/hid/intel-ish-hid.txt | 417 ++++++++ drivers/hid/Kconfig | 2 + drivers/hid/Makefile | 2 + drivers/hid/hid-sensor-hub.c | 4 + drivers/hid/intel-ish-hid/Kconfig | 27 + drivers/hid/intel-ish-hid/Makefile | 20 + drivers/hid/intel-ish-hid/ipc/hw-ish-regs.h | 220 ++++ drivers/hid/intel-ish-hid/ipc/hw-ish.h | 70 ++ drivers/hid/intel-ish-hid/ipc/ipc.c | 720 +++++++++++++ drivers/hid/intel-ish-hid/ipc/pci-ish.c | 305 ++++++ drivers/hid/intel-ish-hid/ipc/utils.h | 64 ++ drivers/hid/intel-ish-hid/ishtp-hid-client.c | 922 ++++++++++++++++ drivers/hid/intel-ish-hid/ishtp-hid.c | 231 ++++ drivers/hid/intel-ish-hid/ishtp-hid.h | 182 ++++ drivers/hid/intel-ish-hid/ishtp/bus.c | 774 ++++++++++++++ drivers/hid/intel-ish-hid/ishtp/bus.h | 105 ++ drivers/hid/intel-ish-hid/ishtp/client.c | 1129 ++++++++++++++++++++ drivers/hid/intel-ish-hid/ishtp/client.h | 194 ++++ drivers/hid/intel-ish-hid/ishtp/dma-if.c | 178 +++ drivers/hid/intel-ish-hid/ishtp/hbm.c | 911 ++++++++++++++++ drivers/hid/intel-ish-hid/ishtp/hbm.h | 319 ++++++ drivers/hid/intel-ish-hid/ishtp/init.c | 94 ++ drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h | 280 +++++ .../iio/common/hid-sensors/hid-sensor-trigger.c | 21 +- include/linux/hid-sensor-hub.h | 1 + include/trace/events/intel_ish.h | 30 + include/uapi/linux/input.h | 1 + 27 files changed, 7222 insertions(+), 1 deletion(-) create mode 100644 Documentation/hid/intel-ish-hid.txt create mode 100644 drivers/hid/intel-ish-hid/Kconfig create mode 100644 drivers/hid/intel-ish-hid/Makefile create mode 100644 drivers/hid/intel-ish-hid/ipc/hw-ish-regs.h create mode 100644 drivers/hid/intel-ish-hid/ipc/hw-ish.h create mode 100644 drivers/hid/intel-ish-hid/ipc/ipc.c create mode 100644 drivers/hid/intel-ish-hid/ipc/pci-ish.c create mode 100644 drivers/hid/intel-ish-hid/ipc/utils.h create mode 100644 drivers/hid/intel-ish-hid/ishtp-hid-client.c create mode 100644 drivers/hid/intel-ish-hid/ishtp-hid.c create mode 100644 drivers/hid/intel-ish-hid/ishtp-hid.h create mode 100644 drivers/hid/intel-ish-hid/ishtp/bus.c create mode 100644 drivers/hid/intel-ish-hid/ishtp/bus.h create mode 100644 drivers/hid/intel-ish-hid/ishtp/client.c create mode 100644 drivers/hid/intel-ish-hid/ishtp/client.h create mode 100644 drivers/hid/intel-ish-hid/ishtp/dma-if.c create mode 100644 drivers/hid/intel-ish-hid/ishtp/hbm.c create mode 100644 drivers/hid/intel-ish-hid/ishtp/hbm.h create mode 100644 drivers/hid/intel-ish-hid/ishtp/init.c create mode 100644 drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h create mode 100644 include/trace/events/intel_ish.h -- 2.5.5
[toc] | [next] | [standalone]
| From | Grant Likely <grant.likely@secretlab.ca> |
|---|---|
| Date | 2016-06-16 21:20 +0200 |
| Message-ID | <rKKZA-3Qd-11@gated-at.bofh.it> |
| In reply to | #1419974 |
On Sat, Jun 11, 2016 at 1:13 PM, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > Starting from Cherrytrail, multiple generation of Intel processors offers > on package sensor hub. Several recent tablets, 2-in-1 convertible laptops > are using ISH instead of external sensor hubs. This resulted in lack of > support of sensor function like device rotation and auto backlight > adjustment. > In addition, depending on the OEM implementation, support of ISH is required > to support low power sleep states. > > The support of ISH on Linux platforms is not new. Android platforms with > Intel SoCs had this support for a while submitted by Daniel Drubin. > This patcheset is reusing most of those changes with clean up and > removing Android platform specific changes. > > The user mode ABI is still same as external sensor hubs using Linux > IIO. So existing user mode software should still work. > This series primarily brings in new HID transport used in ISH. > > Thanks to the community members who tested RFC patches and provided > feedback. > > For users testing on Linux distributions using IIO sensor proxy, > a short term work around is required till we have debugged this issue. > In systemd unit file iio-sensor-proxy.service > In the section "[Unit]" add > After=multi-user.target Tested-by: Grant Likely <grant.likely@hpe.com> There is still a fiddly problem on my laptop where the sensor data doesn't always start streaming, but otherwise this version works for me. g. > > Daniel Drubin (3): > hid: intel_ish-hid: ISH Transport layer > hid: intel-ish-hid: ipc layer > hid: intel-ish-hid: ISH HID client driver > > Srinivas Pandruvada (3): > Documentation: hid: Intel ISH HID document > iio: hid-sensors: use asynchronous resume > hid: hid-sensor-hub: Add ISH quirk > > Documentation/hid/intel-ish-hid.txt | 417 ++++++++ > drivers/hid/Kconfig | 2 + > drivers/hid/Makefile | 2 + > drivers/hid/hid-sensor-hub.c | 4 + > drivers/hid/intel-ish-hid/Kconfig | 27 + > drivers/hid/intel-ish-hid/Makefile | 20 + > drivers/hid/intel-ish-hid/ipc/hw-ish-regs.h | 220 ++++ > drivers/hid/intel-ish-hid/ipc/hw-ish.h | 70 ++ > drivers/hid/intel-ish-hid/ipc/ipc.c | 720 +++++++++++++ > drivers/hid/intel-ish-hid/ipc/pci-ish.c | 305 ++++++ > drivers/hid/intel-ish-hid/ipc/utils.h | 64 ++ > drivers/hid/intel-ish-hid/ishtp-hid-client.c | 922 ++++++++++++++++ > drivers/hid/intel-ish-hid/ishtp-hid.c | 231 ++++ > drivers/hid/intel-ish-hid/ishtp-hid.h | 182 ++++ > drivers/hid/intel-ish-hid/ishtp/bus.c | 774 ++++++++++++++ > drivers/hid/intel-ish-hid/ishtp/bus.h | 105 ++ > drivers/hid/intel-ish-hid/ishtp/client.c | 1129 ++++++++++++++++++++ > drivers/hid/intel-ish-hid/ishtp/client.h | 194 ++++ > drivers/hid/intel-ish-hid/ishtp/dma-if.c | 178 +++ > drivers/hid/intel-ish-hid/ishtp/hbm.c | 911 ++++++++++++++++ > drivers/hid/intel-ish-hid/ishtp/hbm.h | 319 ++++++ > drivers/hid/intel-ish-hid/ishtp/init.c | 94 ++ > drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h | 280 +++++ > .../iio/common/hid-sensors/hid-sensor-trigger.c | 21 +- > include/linux/hid-sensor-hub.h | 1 + > include/trace/events/intel_ish.h | 30 + > include/uapi/linux/input.h | 1 + > 27 files changed, 7222 insertions(+), 1 deletion(-) > create mode 100644 Documentation/hid/intel-ish-hid.txt > create mode 100644 drivers/hid/intel-ish-hid/Kconfig > create mode 100644 drivers/hid/intel-ish-hid/Makefile > create mode 100644 drivers/hid/intel-ish-hid/ipc/hw-ish-regs.h > create mode 100644 drivers/hid/intel-ish-hid/ipc/hw-ish.h > create mode 100644 drivers/hid/intel-ish-hid/ipc/ipc.c > create mode 100644 drivers/hid/intel-ish-hid/ipc/pci-ish.c > create mode 100644 drivers/hid/intel-ish-hid/ipc/utils.h > create mode 100644 drivers/hid/intel-ish-hid/ishtp-hid-client.c > create mode 100644 drivers/hid/intel-ish-hid/ishtp-hid.c > create mode 100644 drivers/hid/intel-ish-hid/ishtp-hid.h > create mode 100644 drivers/hid/intel-ish-hid/ishtp/bus.c > create mode 100644 drivers/hid/intel-ish-hid/ishtp/bus.h > create mode 100644 drivers/hid/intel-ish-hid/ishtp/client.c > create mode 100644 drivers/hid/intel-ish-hid/ishtp/client.h > create mode 100644 drivers/hid/intel-ish-hid/ishtp/dma-if.c > create mode 100644 drivers/hid/intel-ish-hid/ishtp/hbm.c > create mode 100644 drivers/hid/intel-ish-hid/ishtp/hbm.h > create mode 100644 drivers/hid/intel-ish-hid/ishtp/init.c > create mode 100644 drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h > create mode 100644 include/trace/events/intel_ish.h > > -- > 2.5.5 >
[toc] | [prev] | [next] | [standalone]
| From | Jiri Kosina <jikos@kernel.org> |
|---|---|
| Date | 2016-06-17 22:50 +0200 |
| Subject | Re: [PATCH 2/6] hid: intel_ish-hid: ISH Transport layer |
| Message-ID | <rL8Sd-2Y1-9@gated-at.bofh.it> |
| In reply to | #1419974 |
On Fri, 17 Jun 2016, Jiri Kosina wrote:
> > +struct ishtp_cl_device *ishtp_bus_add_device(struct ishtp_device *dev,
> > + uuid_le uuid, char *name)
> > +{
>
> Should be static.
Actually, going deeper into the code and trying to untangle all the
dependencies, there are quite a few more in other ipc.c, hid-client.c,
etc. Please fix that globally in the next iteration.
Thanks,
--
Jiri Kosina
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> |
|---|---|
| Date | 2016-06-17 23:10 +0200 |
| Subject | Re: [PATCH 2/6] hid: intel_ish-hid: ISH Transport layer |
| Message-ID | <rL9bz-3jz-1@gated-at.bofh.it> |
| In reply to | #1425464 |
On Fri, 2016-06-17 at 22:45 +0200, Jiri Kosina wrote:
> On Fri, 17 Jun 2016, Jiri Kosina wrote:
>
> > > +struct ishtp_cl_device *ishtp_bus_add_device(struct ishtp_device
> *dev,
> > > + uuid_le uuid, char
> *name)
> > > +{
> >
> > Should be static.
>
> Actually, going deeper into the code and trying to untangle all the
> dependencies, there are quite a few more in other ipc.c, hid-
> client.c,
> etc. Please fix that globally in the next iteration.
>
Great. I ran sparse and identified few more. I will fix this is new
iteration.
One thing I am still wondering is that the current ISH model is built
in only. Some distros configure CONFIG_HID as module. So in Kconfig in
drivers/hid/intel-ish-hid/, I need to add "select HID".
What do you think about this?
Unless you are in middle of review, I want to go ahead and send v2.
Thanks,
Srinivas
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>
[toc] | [prev] | [next] | [standalone]
| From | Jiri Kosina <jikos@kernel.org> |
|---|---|
| Date | 2016-06-17 22:50 +0200 |
| Subject | Re: [PATCH 2/6] hid: intel_ish-hid: ISH Transport layer |
| Message-ID | <rL8Sd-2Y1-11@gated-at.bofh.it> |
| In reply to | #1419974 |
On Sat, 11 Jun 2016, Srinivas Pandruvada wrote:
[ ... snip ... ]
> diff --git a/drivers/hid/intel-ish-hid/Kconfig b/drivers/hid/intel-ish-hid/Kconfig
> new file mode 100644
> index 0000000..8914f3b
> --- /dev/null
> +++ b/drivers/hid/intel-ish-hid/Kconfig
> @@ -0,0 +1,22 @@
> +menu "Intel ISH HID support"
> + depends on X86 && PCI
> +
> +config INTEL_ISH_HID_TRANSPORT
> + bool
> + default n
> +
> +config INTEL_ISH_HID
> + bool "Intel Integrated Sensor Hub"
Why can't the transport driver be built as a module?
[ ... snip ... ]
> +/**
> + * ishtp_bus_add_device() - Function to create device on bus
> + *
> + * @dev: ishtp device
> + * @uuid: uuid of the client
> + * @name: Name of the client
> + *
> + * Allocate ISHTP bus client device, attach it to uuid
> + * and register with ISHTP bus.
> + */
> +struct ishtp_cl_device *ishtp_bus_add_device(struct ishtp_device *dev,
> + uuid_le uuid, char *name)
> +{
Should be static.
[ ... snip ... ]
> +/**
> + * ishtp_bus_remove_device() - Function to relase device on bus
> + *
> + * @device: client device instance
> + *
> + * This is a counterpart of ishtp_bus_add_device.
> + * Device is unregistered.
> + * the device structure is freed in 'ishtp_cl_dev_release' function
> + * Called only during error in pci driver init path.
> + */
> +void ishtp_bus_remove_device(struct ishtp_cl_device *device)
> +{
Should be static.
[ ... snip ... ]
> +/*
> + * ishtp_hbm_dma_xfer_ack - receive ack for ISHTP-over-DMA client message
> + *
> + * Constraint:
> + * First implementation is one ISHTP message per DMA transfer
> + */
> +void ishtp_hbm_dma_xfer_ack(struct ishtp_device *dev,
Should be static.
[ ... snip ... ]
> +/* ishtp_hbm_dma_xfer - receive ISHTP-over-DMA client message */
> +void ishtp_hbm_dma_xfer(struct ishtp_device *dev,
> + struct dma_xfer_hbm *dma_xfer)
Should be static.
--
Jiri Kosina
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> |
|---|---|
| Date | 2016-06-17 23:20 +0200 |
| Subject | Re: [PATCH 2/6] hid: intel_ish-hid: ISH Transport layer |
| Message-ID | <rL9lf-3mT-21@gated-at.bofh.it> |
| In reply to | #1425466 |
On Fri, 2016-06-17 at 22:43 +0200, Jiri Kosina wrote:
> On Sat, 11 Jun 2016, Srinivas Pandruvada wrote:
>
> [ ... snip ... ]
> > diff --git a/drivers/hid/intel-ish-hid/Kconfig b/drivers/hid/intel-
> ish-hid/Kconfig
> > new file mode 100644
> > index 0000000..8914f3b
> > --- /dev/null
> > +++ b/drivers/hid/intel-ish-hid/Kconfig
> > @@ -0,0 +1,22 @@
> > +menu "Intel ISH HID support"
> > + depends on X86 && PCI
> > +
> > +config INTEL_ISH_HID_TRANSPORT
> > + bool
> > + default n
> > +
> > +config INTEL_ISH_HID
> > + bool "Intel Integrated Sensor Hub"
>
> Why can't the transport driver be built as a module?
In current use case for PM, we don't want anyone to unload and
complain.
But if this is a strong requirement, I will change this to a module.
Thanks,
Srinivas
>
> [ ... snip ... ]
> > +/**
> > + * ishtp_bus_add_device() - Function to create device on bus
> > + *
> > + * @dev: ishtp device
> > + * @uuid: uuid of the client
> > + * @name: Name of the client
> > + *
> > + * Allocate ISHTP bus client device, attach it to uuid
> > + * and register with ISHTP bus.
> > + */
> > +struct ishtp_cl_device *ishtp_bus_add_device(struct ishtp_device
> *dev,
> > + uuid_le uuid, char
> *name)
> > +{
>
> Should be static.
>
> [ ... snip ... ]
> > +/**
> > + * ishtp_bus_remove_device() - Function to relase device on bus
> > + *
> > + * @device: client device instance
> > + *
> > + * This is a counterpart of ishtp_bus_add_device.
> > + * Device is unregistered.
> > + * the device structure is freed in 'ishtp_cl_dev_release'
> function
> > + * Called only during error in pci driver init path.
> > + */
> > +void ishtp_bus_remove_device(struct ishtp_cl_device *device)
> > +{
>
> Should be static.
>
> [ ... snip ... ]
> > +/*
> > + * ishtp_hbm_dma_xfer_ack - receive ack for ISHTP-over-DMA client
> message
> > + *
> > + * Constraint:
> > + * First implementation is one ISHTP message per DMA transfer
> > + */
> > +void ishtp_hbm_dma_xfer_ack(struct ishtp_device *dev,
>
> Should be static.
>
> [ ... snip ... ]
> > +/* ishtp_hbm_dma_xfer - receive ISHTP-over-DMA client message */
> > +void ishtp_hbm_dma_xfer(struct ishtp_device *dev,
> > + struct dma_xfer_hbm *dma_xfer)
>
> Should be static.
>
> --
> Jiri Kosina
> SUSE Labs
>
[toc] | [prev] | [next] | [standalone]
| From | Jiri Kosina <jikos@kernel.org> |
|---|---|
| Date | 2016-06-20 11:40 +0200 |
| Subject | Re: [PATCH 2/6] hid: intel_ish-hid: ISH Transport layer |
| Message-ID | <rM3Qu-6sw-53@gated-at.bofh.it> |
| In reply to | #1425489 |
On Fri, 17 Jun 2016, Srinivas Pandruvada wrote: > > > +config INTEL_ISH_HID_TRANSPORT > > > + bool > > > + default n > > > + > > > +config INTEL_ISH_HID > > > + bool "Intel Integrated Sensor Hub" > > > > Why can't the transport driver be built as a module? > In current use case for PM, we don't want anyone to unload and > complain. Sorry, I don't understand this explanation, could you please elaborate? Thanks, -- Jiri Kosina SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> |
|---|---|
| Date | 2016-06-20 16:30 +0200 |
| Subject | Re: [PATCH 2/6] hid: intel_ish-hid: ISH Transport layer |
| Message-ID | <rM8n8-Wx-11@gated-at.bofh.it> |
| In reply to | #1426420 |
On Mon, 20 Jun 2016 11:29:28 +0200 (CEST) Jiri Kosina <jikos@kernel.org> wrote: > On Fri, 17 Jun 2016, Srinivas Pandruvada wrote: > > > > > +config INTEL_ISH_HID_TRANSPORT > > > > + bool > > > > + default n > > > > + > > > > +config INTEL_ISH_HID > > > > + bool "Intel Integrated Sensor Hub" > > > > > > Why can't the transport driver be built as a module? > > In current use case for PM, we don't want anyone to unload and > > complain. > > Sorry, I don't understand this explanation, could you please elaborate? If the driver isn't loaded your machine doesn't do power management. There are a small set of drivers that need to be loaded to get pm even if not using that device. ISH is one of them, and unlike the others not sucked into a standard configuration. If it's going to get loaded due to the presence of the PCI identifiers on any normal distribution then it's in the same category as graphics and ADSP based audio, both of which can be modules and the only real world impact is they get loaded a second or two later during boot. Alan
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web