Hi all, on a quick glance, I noticed, that the sensor masks (*_EN) are no longer derived from their indices: amd_sfh_pcie.h: enum sensor_idx { accel_idx = 0, gyro_idx = 1, mag_idx = 2, als_idx = 19 }; amd_sfh_pcie.c: #define ACEL_EN BIT(1) #define GYRO_EN BIT(2) #define MAGNO_EN BIT(3) #define ALS_EN BIT(19) Why were all except ALS_EN shifted by one? Is this intentional or a bug? I have received an email by a user, suggesting that this change breaks correct sensors detection when compared to activestatus. Can you confirm this? Also I wonder why the structures "enum desc_type", "struct _hid_report_descriptor" and "struct _hid_device_descriptor" have been introduced in amd_sfh_hid_desc.h as they aren't used anywhere in the code. Kind regards, Richard Am Samstag, den 10.10.2020, 01:31 +0530 schrieb Sandeep Singh: > From: Sandeep Singh <sandeep.singh@xxxxxxx> > > AMD SFH (Sensor Fusion Hub) is a solution running on MP2 > (which is ARM core connected to x86 for processing sensor data). > AMD SFH uses HID over PCI bus to form the HID descriptors and > talks to HID clients like the monitor-sensor/iio-proxy. MP2 which > is exposed as a PCI device to the x86, uses mailboxes to talk to > MP2 firmware to send/receive commands. > > Co-developed-by: Nehal Shah <Nehal-bakulchandra.Shah@xxxxxxx> > Signed-off-by: Nehal Shah <Nehal-bakulchandra.Shah@xxxxxxx> > Signed-off-by: Sandeep Singh <sandeep.singh@xxxxxxx> > --- > drivers/hid/Kconfig | 2 + > drivers/hid/Makefile | 2 + > drivers/hid/amd-sfh-hid/Kconfig | 18 +++ > drivers/hid/amd-sfh-hid/Makefile | 13 +++ > drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 152 > +++++++++++++++++++++++++ > drivers/hid/amd-sfh-hid/amd_sfh_pcie.h | 79 +++++++++++++ > 6 files changed, 266 insertions(+) > 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_sfh_pcie.c > create mode 100644 drivers/hid/amd-sfh-hid/amd_sfh_pcie.h > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > index 05315b434276..1b5f5e9b8d0d 100644 > --- a/drivers/hid/Kconfig > +++ b/drivers/hid/Kconfig > @@ -1174,4 +1174,6 @@ source "drivers/hid/i2c-hid/Kconfig" > > source "drivers/hid/intel-ish-hid/Kconfig" > > +source "drivers/hid/amd-sfh-hid/Kconfig" > + > endmenu > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > index d8ea4b8c95af..7d8ca4e34572 100644 > --- a/drivers/hid/Makefile > +++ b/drivers/hid/Makefile > @@ -141,3 +141,5 @@ obj-$(CONFIG_I2C_HID) += i2c-hid/ > > obj-$(CONFIG_INTEL_ISH_HID) += intel-ish-hid/ > obj-$(INTEL_ISH_FIRMWARE_DOWNLOADER) += intel-ish-hid/ > + > +obj-$(CONFIG_AMD_SFH_HID) += amd-sfh-hid/ > diff --git a/drivers/hid/amd-sfh-hid/Kconfig b/drivers/hid/amd-sfh- > hid/Kconfig > new file mode 100644 > index 000000000000..db069a83e9a2 > --- /dev/null > +++ b/drivers/hid/amd-sfh-hid/Kconfig > @@ -0,0 +1,18 @@ > +# SPDX-License-Identifier: GPL-2.0-or-later > +menu "AMD SFH HID Support" > + depends on X86_64 || COMPILE_TEST > + depends on PCI > + depends on HID > + > +config AMD_SFH_HID > + tristate "AMD Sensor Fusion Hub" > + help > + If you say yes to this option, support will be included for > the > + AMD Sensor Fusion Hub. > + This driver will enable sensors functionality on AMD > platforms > + starting from 17h family of RYZEN parts. > + > + This driver can also be built as a module. If so, the > module will > + be called amd-sfh. > + Say Y or M here if you want to support AMD SFH. If unsure, > say N. > +endmenu > diff --git a/drivers/hid/amd-sfh-hid/Makefile b/drivers/hid/amd-sfh- > hid/Makefile > new file mode 100644 > index 000000000000..35e704da5612 > --- /dev/null > +++ b/drivers/hid/amd-sfh-hid/Makefile > @@ -0,0 +1,13 @@ > +# SPDX-License-Identifier: GPL-2.0-or-later > +# > +# Makefile - AMD SFH HID drivers > +# Copyright (c) 2019-2020, Advanced Micro Devices, Inc. > +# > +# > +obj-$(CONFIG_AMD_SFH_HID) += amd_sfh.o > +amd_sfh-objs := amd_sfh_hid.o > +amd_sfh-objs += amd_sfh_client.o > +amd_sfh-objs += amd_sfh_pcie.o > +amd_sfh-objs += hid_descriptor/amd_sfh_hid_desc.o > + > +ccflags-y += -I $(srctree)/$(src)/ > diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c > b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c > new file mode 100644 > index 000000000000..9c5eb442e1a6 > --- /dev/null > +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c > @@ -0,0 +1,152 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * AMD MP2 PCIe communication driver > + * Copyright 2020 Advanced Micro Devices, Inc. > + * > + * Authors: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx> > + * Sandeep Singh <Sandeep.singh@xxxxxxx> > + */ > + > +#include <linux/bitops.h> > +#include <linux/delay.h> > +#include <linux/dma-mapping.h> > +#include <linux/interrupt.h> > +#include <linux/io-64-nonatomic-lo-hi.h> > +#include <linux/module.h> > +#include <linux/slab.h> > + > +#include "amd_sfh_pcie.h" > + > +#define DRIVER_NAME "pcie_mp2_amd" > +#define DRIVER_DESC "AMD(R) PCIe MP2 Communication Driver" > + > +#define ACEL_EN BIT(1) > +#define GYRO_EN BIT(2) > +#define MAGNO_EN BIT(3) > +#define ALS_EN BIT(19) > + > +void amd_start_sensor(struct amd_mp2_dev *privdata, struct > amd_mp2_sensor_info info) > +{ > + union sfh_cmd_param cmd_param; > + union sfh_cmd_base cmd_base; > + > + /* fill up command register */ > + memset(&cmd_base, 0, sizeof(cmd_base)); > + cmd_base.s.cmd_id = ENABLE_SENSOR; > + cmd_base.s.period = info.period; > + cmd_base.s.sensor_id = info.sensor_idx; > + > + /* fill up command param register */ > + memset(&cmd_param, 0, sizeof(cmd_param)); > + cmd_param.s.buf_layout = 1; > + cmd_param.s.buf_length = 16; > + > + writeq(info.phys_address, privdata->mmio + AMD_C2P_MSG2); > + writel(cmd_param.ul, privdata->mmio + AMD_C2P_MSG1); > + writel(cmd_base.ul, privdata->mmio + AMD_C2P_MSG0); > +} > + > +void amd_stop_sensor(struct amd_mp2_dev *privdata, u16 sensor_idx) > +{ > + union sfh_cmd_base cmd_base; > + > + /* fill up command register */ > + memset(&cmd_base, 0, sizeof(cmd_base)); > + cmd_base.s.cmd_id = DISABLE_SENSOR; > + cmd_base.s.period = 0; > + cmd_base.s.sensor_id = sensor_idx; > + > + writeq(0x0, privdata->mmio + AMD_C2P_MSG2); > + writel(cmd_base.ul, privdata->mmio + AMD_C2P_MSG0); > +} > + > +void amd_stop_all_sensors(struct amd_mp2_dev *privdata) > +{ > + union sfh_cmd_base cmd_base; > + > + /* fill up command register */ > + memset(&cmd_base, 0, sizeof(cmd_base)); > + cmd_base.s.cmd_id = STOP_ALL_SENSORS; > + cmd_base.s.period = 0; > + cmd_base.s.sensor_id = 0; > + > + writel(cmd_base.ul, privdata->mmio + AMD_C2P_MSG0); > +} > + > +int amd_mp2_get_sensor_num(struct amd_mp2_dev *privdata, u8 > *sensor_id) > +{ > + int activestatus, num_of_sensors = 0; > + > + privdata->activecontrolstatus = readl(privdata->mmio + > AMD_P2C_MSG3); > + activestatus = privdata->activecontrolstatus >> 4; > + if (ACEL_EN & activestatus) > + sensor_id[num_of_sensors++] = accel_idx; > + > + if (GYRO_EN & activestatus) > + sensor_id[num_of_sensors++] = gyro_idx; > + > + if (MAGNO_EN & activestatus) > + sensor_id[num_of_sensors++] = mag_idx; > + > + if (ALS_EN & activestatus) > + sensor_id[num_of_sensors++] = als_idx; > + > + return num_of_sensors; > +} > + > +static void amd_mp2_pci_remove(void *privdata) > +{ > + amd_sfh_hid_client_deinit(privdata); > + amd_stop_all_sensors(privdata); > +} > + > +static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct > pci_device_id *id) > +{ > + struct amd_mp2_dev *privdata; > + int rc; > + > + privdata = devm_kzalloc(&pdev->dev, sizeof(*privdata), > GFP_KERNEL); > + if (!privdata) > + return -ENOMEM; > + > + privdata->pdev = pdev; > + pci_set_drvdata(pdev, privdata); > + rc = pcim_enable_device(pdev); > + if (rc) > + return rc; > + > + rc = pcim_iomap_regions(pdev, BIT(2), DRIVER_NAME); > + if (rc) > + return rc; > + > + privdata->mmio = pcim_iomap_table(pdev)[2]; > + pci_set_master(pdev); > + rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(64)); > + if (rc) { > + rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32)); > + return rc; > + } > + rc = devm_add_action_or_reset(&pdev->dev, amd_mp2_pci_remove, > privdata); > + if (rc) > + return rc; > + > + return amd_sfh_hid_client_init(privdata); > +} > + > +static const struct pci_device_id amd_mp2_pci_tbl[] = { > + { PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_MP2) }, > + { } > +}; > +MODULE_DEVICE_TABLE(pci, amd_mp2_pci_tbl); > + > +static struct pci_driver amd_mp2_pci_driver = { > + .name = DRIVER_NAME, > + .id_table = amd_mp2_pci_tbl, > + .probe = amd_mp2_pci_probe, > +}; > +module_pci_driver(amd_mp2_pci_driver); > + > +MODULE_DESCRIPTION(DRIVER_DESC); > +MODULE_LICENSE("Dual BSD/GPL"); > +MODULE_AUTHOR("Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>"); > +MODULE_AUTHOR("Sandeep Singh <Sandeep.singh@xxxxxxx>"); > diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h > b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h > new file mode 100644 > index 000000000000..e8be94f935b7 > --- /dev/null > +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h > @@ -0,0 +1,79 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * AMD MP2 PCIe communication driver > + * Copyright 2020 Advanced Micro Devices, Inc. > + * Authors: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx> > + * Sandeep Singh <Sandeep.singh@xxxxxxx> > + */ > + > +#ifndef PCIE_MP2_AMD_H > +#define PCIE_MP2_AMD_H > + > +#include <linux/pci.h> > + > +#define PCI_DEVICE_ID_AMD_MP2 0x15E4 > + > +#define ENABLE_SENSOR 1 > +#define DISABLE_SENSOR 2 > +#define STOP_ALL_SENSORS 8 > + > +/* MP2 C2P Message Registers */ > +#define AMD_C2P_MSG0 0x10500 > +#define AMD_C2P_MSG1 0x10504 > +#define AMD_C2P_MSG2 0x10508 > + > +/* MP2 P2C Message Registers */ > +#define AMD_P2C_MSG3 0x1068C /* Supported Sensors info */ > + > +/* SFH Command register */ > +union sfh_cmd_base { > + u32 ul; > + struct { > + u32 cmd_id : 8; > + u32 sensor_id : 8; > + u32 period : 16; > + } s; > +}; > + > +union sfh_cmd_param { > + u32 ul; > + struct { > + u32 buf_layout : 2; > + u32 buf_length : 6; > + u32 rsvd : 24; > + } s; > +}; > + > +struct sfh_cmd_reg { > + union sfh_cmd_base cmd_base; > + union sfh_cmd_param cmd_param; > + phys_addr_t phys_addr; > +}; > + > +enum sensor_idx { > + accel_idx = 0, > + gyro_idx = 1, > + mag_idx = 2, > + als_idx = 19 > +}; > + > +struct amd_mp2_dev { > + struct pci_dev *pdev; > + struct amdtp_cl_data *cl_data; > + void __iomem *mmio; > + u32 activecontrolstatus; > +}; > + > +struct amd_mp2_sensor_info { > + u8 sensor_idx; > + u32 period; > + phys_addr_t phys_address; > +}; > + > +void amd_start_sensor(struct amd_mp2_dev *privdata, struct > amd_mp2_sensor_info info); > +void amd_stop_sensor(struct amd_mp2_dev *privdata, u16 sensor_idx); > +void amd_stop_all_sensors(struct amd_mp2_dev *privdata); > +int amd_mp2_get_sensor_num(struct amd_mp2_dev *privdata, u8 > *sensor_id); > +int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata); > +int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata); > +#endif