| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 10 Aug 2020 16:23:34 -0700
From: Randy Dunlap <rdunlap@...radead.org>
To: Sandeep Singh <Sandeep.Singh@....com>, jikos@...nel.org,
benjamin.tissoires@...hat.com, linux-kernel@...r.kernel.org,
linux-input@...r.kernel.org, srinivas.pandruvada@...ux.intel.com,
jic23@...nel.org, linux-iio@...r.kernel.org, hdegoede@...hat.com,
Nehal-bakulchandra.Shah@....com, andy.shevchenko@...il.com,
mail@...hard-neumann.de, m.felsch@...gutronix.de
Cc: Shyam-sundar.S-k@....com
Subject: Re: [PATCH v7 1/4] SFH: Add maintainers and documentation for AMD SFH
based on HID framework
On 8/10/20 2:30 PM, Sandeep Singh wrote:
> From: Sandeep Singh <sandeep.singh@....com>
>
> Add Maintainers for AMD SFH(SENSOR FUSION HUB) Solution and work flow
> document.
>
> Signed-off-by: Nehal Shah <Nehal-bakulchandra.Shah@....com>
> Signed-off-by: Sandeep Singh <sandeep.singh@....com>
Hi,
Did you build the documentation?
Oh, I think you would need to add amd-sfh-hid to
Documentation/hid/index.rst first in order to build the documentation.
That change should be part of your patch set.
So please do that and then run "make htmldocs" and look for warnings or errors.
More below:
> ---
> Documentation/hid/amd-sfh-hid.rst | 153 ++++++++++++++++++++++++++++++
> MAINTAINERS | 8 ++
> 2 files changed, 161 insertions(+)
> create mode 100644 Documentation/hid/amd-sfh-hid.rst
>
> diff --git a/Documentation/hid/amd-sfh-hid.rst b/Documentation/hid/amd-sfh-hid.rst
> new file mode 100644
> index 000000000000..0ee9003013f2
> --- /dev/null
> +++ b/Documentation/hid/amd-sfh-hid.rst
> @@ -0,0 +1,153 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +AMD Sensor Fusion Hub:-
> +======================
Underlines (like above) should be at least as long as the heading line,
but heading lines don't need endings on them like ':' or '-', so please
remove those ending characters on all heading lines.
> +AMD Sensor Fusion Hub is part of a SOC starting from Ryzen based platforms.
an SOC
> +The solution is working well on several OEM products. AMD SFH uses HID over PCIe bus.
I would drop that first sentence above.
> +In terms of architecture it resembles ISH .However the major difference is all
resembles ISH. However
> +The hid reports are generated as part of kernel driver.
the HID
> +
> +Block Diagram:-
> +=============
> + -------------------------------
> + | HID User Space Applications |
> + -------------------------------
> +---------------------------------------------
> + ---------------------------------
> + | HID Core |
> + ---------------------------------
> +
> + ---------------------------------
> + | AMD HID Transport |
> + ---------------------------------
> +
> + --------------------------------
> + | AMD HID Client |
> + | with HID Report Generator|
> + --------------------------------
> +
> + --------------------------------
> + | AMD MP2 PCIe Driver |
> + --------------------------------
> +---------------------------------------------
> + -------------------------------
> + | SFH MP2 Processor |
> + --------------------------------
> +
> +
> +AMD HID Transport Layer :-
> +***************************
> +AMD SFH transport is also implemented as a bus. Each client application executing in the AMD MP2
> +is registered as a device on this bus. MP2 which is an ARM® Cortex-M4 core based co-processor to
Incomplete sentence here: MP2 ....
> +x86. The driver, which binds each device (AMD SFH HID driver) identifies the device type and
> +registers with the hid core. Transport drivers attach a constant "struct hid_ll_driver" object with
HID
> +each device. Once a device is registered with HID core, the callbacks provided via this struct are
> +used by HID core to communicate with the device. AMD HID Transport driver implements the synchronouscalls.
implements synchronous calls.
> +
> +AMD HID Client Layer:-
> +**********************
> +This layer is responsible to implement HID request and descriptors. As firmware is OS agnostic,
> +HID client layer fills the HID request structure and descriptors. HID client layer is in complex
is complex
> +in nature as it is interface between MP2 PCIe driver and HID. HID client layer initialized
initializes
I would drop "in nature".
> +the MP2 PCIe driver and holds the instance of MP2 driver.It identified the number of sensors
driver. It identifies
> +connected using MP2-PCIe driver and based on that allocate the DRAM address for each and every
allocates
> +sensor and pass it to MP2-PCIe driver.On enumeration of each sensor, client layer fills out the HID
passes driver. On
> +Descriptor structure and HID input report structure. HID Feature report structure can be optional.
structure is optional.
> +The report descriptor structure varies sensor to sensor. Now on enumeration client layer does
> +two major things
things:
(at least for punctuation; I don't know if that's sufficient for ReST markup.)
> +1. Register the HID sensor client to virtual bus (Platform driver) and bind it.
> +2. Probes the AMD HID transport driver. Which in turns register device to the core.
driver, which in turn registers
> +
> +AMD MP2 PCIe layer:-
> +********************
> +MP2 PCIe Layer is responsible for making all transaction with the firmware over PCIe.
transactions
> +The connection establishment between firmware and PCIe happens here.
> +
> +The communication between X86 and MP2 is spilt into three parts.
split
> +1. Command Transfer => C2P Mailbox Register are used
Registers are used
> +2. Data Transfer => DRAM
> +3. Supported sensor info => P2C Register
> +
> +Commands are sent to MP2 using C2P Mail Box registers. These C2P registers are common between x86
Mailbox C2P registers X86
Use Mailbox and X86 as above for consistency.
> +and MP2. Writing into C2P Message register generate interrupt to MP2. The client layer allocates
> +the physical memory and send the same to MP2 for data transfer. MP2 firmware uses HUBIF interface
What is HUBIF interface? do we care?
> +to access DRAM memory. For Firmware always write minimum 32 bytes into DRAM.So it is expected that
firmware DRAM. So
> +driver shall allocate minimum 32 bytes DRAM space.
> +
> +Enumeration and Probing flow:-
> +*****************************
> + HID AMD AMD AMD -PCIe MP2
> + Core Transport Client layer layer FW
> + | | | | |
> + | | | on Boot Driver Loaded |
> + | | | | |
> + | | | MP2-PCIe Int |
> + | | | | |
> + | | |---Get Number of sensors-> | |
> + | | | Read P2C |
> + | | | Register |
> + | | | | |
> + | | | Loop(for No of Sensors) | |
> + | | |----------------------| | |
> + | | | Create HID Descriptor| | |
> + | | | Create Input report | | |
> + | | | Descriptor Map | | |
> + | | | the MP2 FW Index to | | |
> + | | | HID Index | | |
> + | | | Allocate the DRAM | Enable |
> + | | | address | Sensors |
> + | | |----------------------| | |
> + | | HID transport| | Enable |
> + | |<--Probe------| |---Sensor CMD--> |
> + | | Create the | | |
> + | | HID device | | |
> + | | (MFD) | | |
> + | | by Populating| | |
> + | | the HID | | |
> + | | ll_driver | | |
> + | HID | | | |
> + | add | | | |
> + |Device | | | |
> + |<------------- | | | |
> +
> +
> +Data Flow from Application to the AMD SFH Driver:-
> +*************************************************
> +
> + | | | | |
> +Get | | | | |
> +Input | | | | |
> +Report | | | | |
> +---> | | | | |
> + |HID_req | | | |
> + |get_report | | | |
> + |------------->| | | |
> + | | HID_get_input| | |
> + | | report | | |
> + | |------------->|------------------------| | |
> + | | | Read the DRAM data for| | |
> + | | | requested sensor and | | |
> + | | | create the HID input | | |
> + | | | report | | |
> + | | |------------------------| | |
> + | |Data received | | |
> + | | in HID report| | |
> + To |<-------------|<-------------| | |
> +Applications | | | |
> +<-------| | | | |
> +
> +
> +Data Flow from AMD SFH Driver to Application:-
> +**********************************************
> + | | | | |
> + | | |------------------------| | |
> + | | |Periodically Read | | |
> + | | |the data for all | | |
> + | | |enumerated sensors | | |
> + | | |from the dram and create| | |
> + | | | HID Input reports | | |
> + | | |------------------------| | |
> + | |HID Input | | |
> + | |Input report | | |
> + <----submit to Application-----| | |
> + | | | | |
thanks.
--
~Randy
Powered by blists - more mailing lists