mbox series

[v5,0/4] SFH: Add Support for AMD Sensor Fusion Hub

Message ID 1590759730-32494-1-git-send-email-Sandeep.Singh@amd.com (mailing list archive)
Headers show
Series SFH: Add Support for AMD Sensor Fusion Hub | expand

Message

Sandeep Singh May 29, 2020, 1:42 p.m. UTC
From: Sandeep Singh <sandeep.singh@amd.com>

AMD SFH(Sensor Fusion Hub) is HID based driver.SFH FW
is part of MP2 processor (MP2 which is an ARM® Cortex-M4
core based co-processor to x86) and it runs on MP2 where
in driver resides on X86.The driver functionalities are
divided  into three parts:-

1: amd-mp2-pcie:-       This module will communicate with MP2 FW and
                        provide that data into DRAM.
2: Client driver :-     This part for driver will use dram data and
                        convert that data into HID format based on
                        HID reports.
3: Transport driver :-  This part of driver will communicate with
                        HID core. Communication between devices and
                        HID core is mostly done via HID reports

In terms of architecture it is much more reassembles like
ISH(Intel Integrated Sensor Hub). However the major difference
is all the hid reports are generated as part of kernel driver.
AMD SFH driver taken reference from ISH in terms of
design and functionalities at fewer location.

AMD sensor fusion Hub is part of a SOC 17h family based platforms.
The solution is working well on several OEM products.
AMD SFH uses HID over PCIe bus.

Sandeep Singh (4):
  SFH: Add maintainers and documentation for AMD SFH based on HID
    framework
  SFH: PCI driver to add support of AMD sensor fusion Hub using HID
    framework
  SFH: Transport Driver to add support of AMD Sensor Fusion Hub (SFH)
  SFH: Create HID report to Enable support of AMD sensor fusion Hub
    (SFH)

Changes since v1:
        -Fix auto build test warnings
        -Fix warnings captured using smatch
        -Changes suggested by Dan Carpenter

Links of the review comments for v1:
        [1] https://patchwork.kernel.org/patch/11325163/
        [2] https://patchwork.kernel.org/patch/11325167/
        [3] https://patchwork.kernel.org/patch/11325171/
        [4] https://patchwork.kernel.org/patch/11325187/


Changes since v2:
        -Debugfs divided into another patch
        -Fix some cosmetic changes
        -Fix for review comments
         Reported and Suggested by:-  Srinivas Pandruvada

Links of the review comments for v2:
        [1] https://patchwork.kernel.org/patch/11355491/
        [2] https://patchwork.kernel.org/patch/11355495/
        [3] https://patchwork.kernel.org/patch/11355499/
        [4] https://patchwork.kernel.org/patch/11355503/


Changes since v3:
	-removed debugfs suggested by - Benjamin Tissoires

Links of the review comments for v3:
	[1] https://lkml.org/lkml/2020/2/11/1256
	[2] https://lkml.org/lkml/2020/2/11/1257
	[3] https://lkml.org/lkml/2020/2/11/1258
	[4] https://lkml.org/lkml/2020/2/11/1259
	[5] https://lkml.org/lkml/2020/2/11/1260 


Changes since v4:
	- Rework done based on review comments by - Andy Shevchenko
	- changes done based on suggested by -Richard Neumann

Links of the review comments for v4:
	[1] https://lkml.org/lkml/2020/2/26/1360
	[2] https://lkml.org/lkml/2020/2/26/1361
	[3] https://lkml.org/lkml/2020/2/26/1362
	[4] https://lkml.org/lkml/2020/2/26/1363
	[5] https://lkml.org/lkml/2020/2/27/1


Sandeep Singh (4):
  SFH: Add maintainers and documentation for AMD SFH based      on HID
    framework
  SFH: PCI driver to add support of AMD sensor fusion Hub using HID
    framework
  SFH: Transport Driver to add support of AMD Sensor Fusion Hub (SFH
  SFH: Create HID report to Enable support of AMD sensor fusion Hub
    (SFH)

 Documentation/hid/amd-sfh-hid.rst                  | 160 +++++
 MAINTAINERS                                        |   8 +
 drivers/hid/Kconfig                                |   2 +
 drivers/hid/Makefile                               |   1 +
 drivers/hid/amd-sfh-hid/Kconfig                    |  21 +
 drivers/hid/amd-sfh-hid/Makefile                   |  16 +
 drivers/hid/amd-sfh-hid/amd_mp2_pcie.c             | 196 +++++++
 drivers/hid/amd-sfh-hid/amd_mp2_pcie.h             | 137 +++++
 drivers/hid/amd-sfh-hid/amdsfh-hid-client.c        | 256 ++++++++
 drivers/hid/amd-sfh-hid/amdsfh-hid.c               | 179 ++++++
 drivers/hid/amd-sfh-hid/amdsfh-hid.h               |  84 +++
 .../hid_descriptor/amd_sfh_hid_descriptor.c        | 251 ++++++++
 .../hid_descriptor/amd_sfh_hid_descriptor.h        | 125 ++++
 .../hid_descriptor/amd_sfh_hid_report_descriptor.h | 642 +++++++++++++++++++++
 14 files changed, 2078 insertions(+)
 create mode 100644 Documentation/hid/amd-sfh-hid.rst
 create mode 100644 drivers/hid/amd-sfh-hid/Kconfig
 create mode 100644 drivers/hid/amd-sfh-hid/Makefile
 create mode 100644 drivers/hid/amd-sfh-hid/amd_mp2_pcie.c
 create mode 100644 drivers/hid/amd-sfh-hid/amd_mp2_pcie.h
 create mode 100644 drivers/hid/amd-sfh-hid/amdsfh-hid-client.c
 create mode 100644 drivers/hid/amd-sfh-hid/amdsfh-hid.c
 create mode 100644 drivers/hid/amd-sfh-hid/amdsfh-hid.h
 create mode 100644 drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_descriptor.c
 create mode 100644 drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_descriptor.h
 create mode 100644 drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_descriptor.h

Comments

Andy Shevchenko May 29, 2020, 2:21 p.m. UTC | #1
On Fri, May 29, 2020 at 4:42 PM Sandeep Singh <Sandeep.Singh@amd.com> wrote:
>
> From: Sandeep Singh <sandeep.singh@amd.com>
>
> AMD SFH(Sensor Fusion Hub) is HID based driver.SFH FW
> is part of MP2 processor (MP2 which is an ARM® Cortex-M4
> core based co-processor to x86) and it runs on MP2 where
> in driver resides on X86.The driver functionalities are
> divided  into three parts:-
>
> 1: amd-mp2-pcie:-       This module will communicate with MP2 FW and
>                         provide that data into DRAM.
> 2: Client driver :-     This part for driver will use dram data and
>                         convert that data into HID format based on
>                         HID reports.
> 3: Transport driver :-  This part of driver will communicate with
>                         HID core. Communication between devices and
>                         HID core is mostly done via HID reports
>
> In terms of architecture it is much more reassembles like
> ISH(Intel Integrated Sensor Hub). However the major difference
> is all the hid reports are generated as part of kernel driver.
> AMD SFH driver taken reference from ISH in terms of
> design and functionalities at fewer location.
>
> AMD sensor fusion Hub is part of a SOC 17h family based platforms.
> The solution is working well on several OEM products.
> AMD SFH uses HID over PCIe bus.
>

I can't believe this is fifth version.
Guys, you have to do much more work on these drivers.

> Sandeep Singh (4):
>   SFH: Add maintainers and documentation for AMD SFH based on HID
>     framework
>   SFH: PCI driver to add support of AMD sensor fusion Hub using HID
>     framework
>   SFH: Transport Driver to add support of AMD Sensor Fusion Hub (SFH)
>   SFH: Create HID report to Enable support of AMD sensor fusion Hub
>     (SFH)
>
> Changes since v1:
>         -Fix auto build test warnings
>         -Fix warnings captured using smatch
>         -Changes suggested by Dan Carpenter
>
> Links of the review comments for v1:
>         [1] https://patchwork.kernel.org/patch/11325163/
>         [2] https://patchwork.kernel.org/patch/11325167/
>         [3] https://patchwork.kernel.org/patch/11325171/
>         [4] https://patchwork.kernel.org/patch/11325187/
>
>
> Changes since v2:
>         -Debugfs divided into another patch
>         -Fix some cosmetic changes
>         -Fix for review comments
>          Reported and Suggested by:-  Srinivas Pandruvada
>
> Links of the review comments for v2:
>         [1] https://patchwork.kernel.org/patch/11355491/
>         [2] https://patchwork.kernel.org/patch/11355495/
>         [3] https://patchwork.kernel.org/patch/11355499/
>         [4] https://patchwork.kernel.org/patch/11355503/
>
>
> Changes since v3:
>         -removed debugfs suggested by - Benjamin Tissoires
>
> Links of the review comments for v3:
>         [1] https://lkml.org/lkml/2020/2/11/1256
>         [2] https://lkml.org/lkml/2020/2/11/1257
>         [3] https://lkml.org/lkml/2020/2/11/1258
>         [4] https://lkml.org/lkml/2020/2/11/1259
>         [5] https://lkml.org/lkml/2020/2/11/1260
>
>
> Changes since v4:
>         - Rework done based on review comments by - Andy Shevchenko
>         - changes done based on suggested by -Richard Neumann
>
> Links of the review comments for v4:
>         [1] https://lkml.org/lkml/2020/2/26/1360
>         [2] https://lkml.org/lkml/2020/2/26/1361
>         [3] https://lkml.org/lkml/2020/2/26/1362
>         [4] https://lkml.org/lkml/2020/2/26/1363
>         [5] https://lkml.org/lkml/2020/2/27/1
>
>
> Sandeep Singh (4):
>   SFH: Add maintainers and documentation for AMD SFH based      on HID
>     framework
>   SFH: PCI driver to add support of AMD sensor fusion Hub using HID
>     framework
>   SFH: Transport Driver to add support of AMD Sensor Fusion Hub (SFH
>   SFH: Create HID report to Enable support of AMD sensor fusion Hub
>     (SFH)
>
>  Documentation/hid/amd-sfh-hid.rst                  | 160 +++++
>  MAINTAINERS                                        |   8 +
>  drivers/hid/Kconfig                                |   2 +
>  drivers/hid/Makefile                               |   1 +
>  drivers/hid/amd-sfh-hid/Kconfig                    |  21 +
>  drivers/hid/amd-sfh-hid/Makefile                   |  16 +
>  drivers/hid/amd-sfh-hid/amd_mp2_pcie.c             | 196 +++++++
>  drivers/hid/amd-sfh-hid/amd_mp2_pcie.h             | 137 +++++
>  drivers/hid/amd-sfh-hid/amdsfh-hid-client.c        | 256 ++++++++
>  drivers/hid/amd-sfh-hid/amdsfh-hid.c               | 179 ++++++
>  drivers/hid/amd-sfh-hid/amdsfh-hid.h               |  84 +++
>  .../hid_descriptor/amd_sfh_hid_descriptor.c        | 251 ++++++++
>  .../hid_descriptor/amd_sfh_hid_descriptor.h        | 125 ++++
>  .../hid_descriptor/amd_sfh_hid_report_descriptor.h | 642 +++++++++++++++++++++
>  14 files changed, 2078 insertions(+)
>  create mode 100644 Documentation/hid/amd-sfh-hid.rst
>  create mode 100644 drivers/hid/amd-sfh-hid/Kconfig
>  create mode 100644 drivers/hid/amd-sfh-hid/Makefile
>  create mode 100644 drivers/hid/amd-sfh-hid/amd_mp2_pcie.c
>  create mode 100644 drivers/hid/amd-sfh-hid/amd_mp2_pcie.h
>  create mode 100644 drivers/hid/amd-sfh-hid/amdsfh-hid-client.c
>  create mode 100644 drivers/hid/amd-sfh-hid/amdsfh-hid.c
>  create mode 100644 drivers/hid/amd-sfh-hid/amdsfh-hid.h
>  create mode 100644 drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_descriptor.c
>  create mode 100644 drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_descriptor.h
>  create mode 100644 drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_descriptor.h
>
> --
> 2.7.4
>
Marco Felsch July 28, 2020, 2:58 p.m. UTC | #2
Hi,

On 20-05-29 17:21, Andy Shevchenko wrote:
> On Fri, May 29, 2020 at 4:42 PM Sandeep Singh <Sandeep.Singh@amd.com> wrote:
> >
> > From: Sandeep Singh <sandeep.singh@amd.com>
> >
> > AMD SFH(Sensor Fusion Hub) is HID based driver.SFH FW
> > is part of MP2 processor (MP2 which is an ARM® Cortex-M4
> > core based co-processor to x86) and it runs on MP2 where
> > in driver resides on X86.The driver functionalities are
> > divided  into three parts:-
> >
> > 1: amd-mp2-pcie:-       This module will communicate with MP2 FW and
> >                         provide that data into DRAM.
> > 2: Client driver :-     This part for driver will use dram data and
> >                         convert that data into HID format based on
> >                         HID reports.
> > 3: Transport driver :-  This part of driver will communicate with
> >                         HID core. Communication between devices and
> >                         HID core is mostly done via HID reports
> >
> > In terms of architecture it is much more reassembles like
> > ISH(Intel Integrated Sensor Hub). However the major difference
> > is all the hid reports are generated as part of kernel driver.
> > AMD SFH driver taken reference from ISH in terms of
> > design and functionalities at fewer location.
> >
> > AMD sensor fusion Hub is part of a SOC 17h family based platforms.
> > The solution is working well on several OEM products.
> > AMD SFH uses HID over PCIe bus.
> >
> 
> I can't believe this is fifth version.
> Guys, you have to do much more work on these drivers.

Is there any active development addressing Andy's comments? Else can we
support you to bring these patches mainline?

Regards,
  Marco
Richard Neumann July 28, 2020, 3:05 p.m. UTC | #3
Hi,

Am Dienstag, den 28.07.2020, 16:58 +0200 schrieb Marco Felsch:
> Hi,
> 
> On 20-05-29 17:21, Andy Shevchenko wrote:
> > On Fri, May 29, 2020 at 4:42 PM Sandeep Singh <
> > Sandeep.Singh@amd.com> wrote:
> > > From: Sandeep Singh <sandeep.singh@amd.com>
> > > 
> > > AMD SFH(Sensor Fusion Hub) is HID based driver.SFH FW
> > > is part of MP2 processor (MP2 which is an ARM® Cortex-M4
> > > core based co-processor to x86) and it runs on MP2 where
> > > in driver resides on X86.The driver functionalities are
> > > divided  into three parts:-
> > > 
> > > 1: amd-mp2-pcie:-       This module will communicate with MP2 FW
> > > and
> > >                         provide that data into DRAM.
> > > 2: Client driver :-     This part for driver will use dram data
> > > and
> > >                         convert that data into HID format based
> > > on
> > >                         HID reports.
> > > 3: Transport driver :-  This part of driver will communicate with
> > >                         HID core. Communication between devices
> > > and
> > >                         HID core is mostly done via HID reports
> > > 
> > > In terms of architecture it is much more reassembles like
> > > ISH(Intel Integrated Sensor Hub). However the major difference
> > > is all the hid reports are generated as part of kernel driver.
> > > AMD SFH driver taken reference from ISH in terms of
> > > design and functionalities at fewer location.
> > > 
> > > AMD sensor fusion Hub is part of a SOC 17h family based
> > > platforms.
> > > The solution is working well on several OEM products.
> > > AMD SFH uses HID over PCIe bus.
> > > 
> > 
> > I can't believe this is fifth version.
> > Guys, you have to do much more work on these drivers.
> 
> Is there any active development addressing Andy's comments? Else can
> we
> support you to bring these patches mainline?
> 
> Regards,
>   Marco

I did some major refactoring¹ on the v4 patch series and am currently
providing it for Arch Linux as a DKMS package in the AUR².
A teensy bit of my changes seems to have influenced the v5 version but
I actually did some major changes, like strictly separating the HID,
platform and PCI driver parts.
I did not submit my changes for upstream so far, since I do not want to
undermine AMD's work on this nor cause competing driver
implementations.

Sincerely,

Richard

[1] https://github.com/conqp/linux/tree/amd-sfh-hid/drivers/hid/amd-sfh-hid
[2] https://aur.archlinux.org/packages/amd-sfh-hid-dkms/
Marco Felsch July 28, 2020, 3:20 p.m. UTC | #4
Hi Richard,

On 20-07-28 17:05, Richard Neumann wrote:
> Hi,
> 
> Am Dienstag, den 28.07.2020, 16:58 +0200 schrieb Marco Felsch:
> > Hi,
> > 
> > On 20-05-29 17:21, Andy Shevchenko wrote:
> > > On Fri, May 29, 2020 at 4:42 PM Sandeep Singh <
> > > Sandeep.Singh@amd.com> wrote:
> > > > From: Sandeep Singh <sandeep.singh@amd.com>
> > > > 

...

> > > I can't believe this is fifth version.
> > > Guys, you have to do much more work on these drivers.
> > 
> > Is there any active development addressing Andy's comments? Else can
> > we
> > support you to bring these patches mainline?
> > 
> > Regards,
> >   Marco
> 
> I did some major refactoring¹ on the v4 patch series and am currently
> providing it for Arch Linux as a DKMS package in the AUR².
> A teensy bit of my changes seems to have influenced the v5 version but
> I actually did some major changes, like strictly separating the HID,
> platform and PCI driver parts.
> I did not submit my changes for upstream so far, since I do not want to
> undermine AMD's work on this nor cause competing driver
> implementations.
> 
> Sincerely,
> 
> Richard
> 
> [1] https://github.com/conqp/linux/tree/amd-sfh-hid/drivers/hid/amd-sfh-hid
> [2] https://aur.archlinux.org/packages/amd-sfh-hid-dkms/

Thanks for the links :) We should get this work done to improve user
experience. Let's wait for the AMD developers comments and then we can
discuss the further steps :)

Regards,
  Marco