Message ID | 20220929094321.770125-9-quan@os.amperecomputing.com |
---|---|
State | New |
Headers | show |
Series | Add Ampere's Altra SMPro MFD and its child drivers | expand |
Hi Quan,
I love your patch! Perhaps something to improve:
[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on groeck-staging/hwmon-next lee-mfd/for-mfd-next linus/master v6.0-rc7 next-20220930]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Quan-Nguyen/Add-Ampere-s-Altra-SMPro-MFD-and-its-child-drivers/20220929-174756
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 3aa12610b481f99b5e4e3f801ff7f9b7629e4ecf
config: hexagon-randconfig-r041-20221001
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/b5237c2e863cc2719cae133dcc218bb1cc165514
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Quan-Nguyen/Add-Ampere-s-Altra-SMPro-MFD-and-its-child-drivers/20220929-174756
git checkout b5237c2e863cc2719cae133dcc218bb1cc165514
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/hwmon/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/hwmon/smpro-hwmon.c:378:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
default:
^
drivers/hwmon/smpro-hwmon.c:378:2: note: insert 'break;' to avoid fall-through
default:
^
break;
1 warning generated.
vim +378 drivers/hwmon/smpro-hwmon.c
afdad0428cacb9 Quan Nguyen 2022-09-29 359
afdad0428cacb9 Quan Nguyen 2022-09-29 360 static umode_t smpro_is_visible(const void *data, enum hwmon_sensor_types type,
afdad0428cacb9 Quan Nguyen 2022-09-29 361 u32 attr, int channel)
afdad0428cacb9 Quan Nguyen 2022-09-29 362 {
afdad0428cacb9 Quan Nguyen 2022-09-29 363 const struct smpro_hwmon *hwmon = data;
afdad0428cacb9 Quan Nguyen 2022-09-29 364 unsigned int value;
afdad0428cacb9 Quan Nguyen 2022-09-29 365 int ret;
afdad0428cacb9 Quan Nguyen 2022-09-29 366
afdad0428cacb9 Quan Nguyen 2022-09-29 367 switch (type) {
afdad0428cacb9 Quan Nguyen 2022-09-29 368 case hwmon_temp:
afdad0428cacb9 Quan Nguyen 2022-09-29 369 switch (attr) {
afdad0428cacb9 Quan Nguyen 2022-09-29 370 case hwmon_temp_input:
afdad0428cacb9 Quan Nguyen 2022-09-29 371 case hwmon_temp_label:
afdad0428cacb9 Quan Nguyen 2022-09-29 372 case hwmon_temp_crit:
afdad0428cacb9 Quan Nguyen 2022-09-29 373 ret = regmap_read(hwmon->regmap, temperature[channel].reg, &value);
afdad0428cacb9 Quan Nguyen 2022-09-29 374 if (ret || value == 0xFFFF)
afdad0428cacb9 Quan Nguyen 2022-09-29 375 return 0;
afdad0428cacb9 Quan Nguyen 2022-09-29 376 break;
afdad0428cacb9 Quan Nguyen 2022-09-29 377 }
afdad0428cacb9 Quan Nguyen 2022-09-29 @378 default:
afdad0428cacb9 Quan Nguyen 2022-09-29 379 break;
afdad0428cacb9 Quan Nguyen 2022-09-29 380 }
afdad0428cacb9 Quan Nguyen 2022-09-29 381
afdad0428cacb9 Quan Nguyen 2022-09-29 382 return 0444;
afdad0428cacb9 Quan Nguyen 2022-09-29 383 }
afdad0428cacb9 Quan Nguyen 2022-09-29 384
On Thu, 29 Sep 2022, Quan Nguyen wrote: > Adds Multi-function devices driver for SMpro co-processor found on the > Mt.Jade hardware reference platform with Ampere's Altra processor family. > > Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com> > --- > > Changes in v9: > + Rename smpro-mfd.c to smpro-core.c [Lee Jones] > + Fix include files in alphabetical order [Lee Jones] > + Add defines to avoid using magic numbers [Lee Jones] > + Return -EINVAL if device does not match [Lee Jones] > + Remove unneccessary comment [Lee Jones] > + Refactor error path handling in smpro_core_read/write() [Quan] > > Changes in v8: > + None > > Changes in v7: > + Smpro-mfd now significant changes in compare with simple-mfd-i2c > driver, remove the copyright note about simple-mfd-i2c [Quan] > + Install bus->read/write() to handle multiple types of bus > access. [Quan] > + Update license to MODULE_LICENSE("GPL") [Quan] > + Add others minor refactor the code [Quan] > > Changes in v6: > + Update license part to reflect that this driver is clone from > simple-mfd-i2c driver [Quan] > > Changes in v5: > + Dropped the use of simple-mfd-i2c driver [Quan] > + Introduced drivers/mfd/smpro-mfd.c driver to instantiate > sub-devices. This is to avoid DT nodes without resource issue [Quan] > + Revised commit message [Quan] > > Changes in v4: > + Add "depends on I2C" to fix build issue found by kernel test > robot [Guenter] > > Changes in v3: > + None > > Changes in v2: > + Used 'struct of_device_id's .data attribute [Lee Jones] > > drivers/mfd/Kconfig | 12 ++++ > drivers/mfd/Makefile | 1 + > drivers/mfd/smpro-core.c | 138 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 151 insertions(+) > create mode 100644 drivers/mfd/smpro-core.c Applied, thanks.
On 24/10/2022 19:39, Lee Jones wrote: > On Thu, 29 Sep 2022, Quan Nguyen wrote: > >> Adds Multi-function devices driver for SMpro co-processor found on the >> Mt.Jade hardware reference platform with Ampere's Altra processor family. >> >> Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com> >> --- >> >> Changes in v9: >> + Rename smpro-mfd.c to smpro-core.c [Lee Jones] >> + Fix include files in alphabetical order [Lee Jones] >> + Add defines to avoid using magic numbers [Lee Jones] >> + Return -EINVAL if device does not match [Lee Jones] >> + Remove unneccessary comment [Lee Jones] >> + Refactor error path handling in smpro_core_read/write() [Quan] >> >> Changes in v8: >> + None >> >> Changes in v7: >> + Smpro-mfd now significant changes in compare with simple-mfd-i2c >> driver, remove the copyright note about simple-mfd-i2c [Quan] >> + Install bus->read/write() to handle multiple types of bus >> access. [Quan] >> + Update license to MODULE_LICENSE("GPL") [Quan] >> + Add others minor refactor the code [Quan] >> >> Changes in v6: >> + Update license part to reflect that this driver is clone from >> simple-mfd-i2c driver [Quan] >> >> Changes in v5: >> + Dropped the use of simple-mfd-i2c driver [Quan] >> + Introduced drivers/mfd/smpro-mfd.c driver to instantiate >> sub-devices. This is to avoid DT nodes without resource issue [Quan] >> + Revised commit message [Quan] >> >> Changes in v4: >> + Add "depends on I2C" to fix build issue found by kernel test >> robot [Guenter] >> >> Changes in v3: >> + None >> >> Changes in v2: >> + Used 'struct of_device_id's .data attribute [Lee Jones] >> >> drivers/mfd/Kconfig | 12 ++++ >> drivers/mfd/Makefile | 1 + >> drivers/mfd/smpro-core.c | 138 +++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 151 insertions(+) >> create mode 100644 drivers/mfd/smpro-core.c > > Applied, thanks. > Thank you for picking the patch. - Quan
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 8b93856de432..9e77f4762999 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -77,6 +77,18 @@ config MFD_AS3711 help Support for the AS3711 PMIC from AMS +config MFD_SMPRO + tristate "Ampere Computing SMpro core driver" + depends on I2C + select MFD_CORE + select REGMAP_I2C + help + Say yes here to enable SMpro driver support for Ampere's Altra + processor family. + + Ampere's Altra SMpro exposes an I2C regmap interface that can + be accessed by child devices. + config MFD_AS3722 tristate "ams AS3722 Power Management IC" select MFD_CORE diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index 7ed3ef4a698c..9387c3ddab4e 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -271,6 +271,7 @@ obj-$(CONFIG_MFD_QCOM_PM8008) += qcom-pm8008.o obj-$(CONFIG_SGI_MFD_IOC3) += ioc3.o obj-$(CONFIG_MFD_SIMPLE_MFD_I2C) += simple-mfd-i2c.o +obj-$(CONFIG_MFD_SMPRO) += smpro-core.o obj-$(CONFIG_MFD_INTEL_M10_BMC) += intel-m10-bmc.o obj-$(CONFIG_MFD_ATC260X) += atc260x-core.o diff --git a/drivers/mfd/smpro-core.c b/drivers/mfd/smpro-core.c new file mode 100644 index 000000000000..d7729cf70378 --- /dev/null +++ b/drivers/mfd/smpro-core.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Ampere Altra Family SMPro core driver + * Copyright (c) 2022, Ampere Computing LLC + */ + +#include <linux/i2c.h> +#include <linux/kernel.h> +#include <linux/mfd/core.h> +#include <linux/module.h> +#include <linux/of_platform.h> +#include <linux/regmap.h> + +/* Identification Registers */ +#define MANUFACTURER_ID_REG 0x02 +#define AMPERE_MANUFACTURER_ID 0xCD3A + +#define CORE_CE_ERR_DATA 0x82 +#define CORE_UE_ERR_DATA 0x85 +#define MEM_CE_ERR_DATA 0x92 +#define MEM_UE_ERR_DATA 0x95 +#define PCIE_CE_ERR_DATA 0xC2 +#define PCIE_UE_ERR_DATA 0xC5 +#define OTHER_CE_ERR_DATA 0xD2 +#define OTHER_UE_ERR_DATA 0xDA + +static int smpro_core_write(void *context, const void *data, size_t count) +{ + struct device *dev = context; + struct i2c_client *i2c = to_i2c_client(dev); + int ret; + + ret = i2c_master_send(i2c, data, count); + if (unlikely(ret != count)) + return (ret < 0) ? ret : -EIO; + + return 0; +} + +static int smpro_core_read(void *context, const void *reg, size_t reg_size, + void *val, size_t val_size) +{ + struct device *dev = context; + struct i2c_client *i2c = to_i2c_client(dev); + struct i2c_msg xfer[2]; + unsigned char buf[2]; + int ret; + + xfer[0].addr = i2c->addr; + xfer[0].flags = 0; + + buf[0] = *(u8 *)reg; + buf[1] = val_size; + xfer[0].len = 2; + xfer[0].buf = buf; + + xfer[1].addr = i2c->addr; + xfer[1].flags = I2C_M_RD; + xfer[1].len = val_size; + xfer[1].buf = val; + + ret = i2c_transfer(i2c->adapter, xfer, 2); + if (unlikely(ret != 2)) + return (ret < 0) ? ret : -EIO; + + return 0; +} + +static const struct regmap_bus smpro_regmap_bus = { + .read = smpro_core_read, + .write = smpro_core_write, + .val_format_endian_default = REGMAP_ENDIAN_BIG, +}; + +static bool smpro_core_readable_noinc_reg(struct device *dev, unsigned int reg) +{ + return (reg == CORE_CE_ERR_DATA || reg == CORE_UE_ERR_DATA || + reg == MEM_CE_ERR_DATA || reg == MEM_UE_ERR_DATA || + reg == PCIE_CE_ERR_DATA || reg == PCIE_UE_ERR_DATA || + reg == OTHER_CE_ERR_DATA || reg == OTHER_UE_ERR_DATA); +} + +static const struct regmap_config smpro_regmap_config = { + .reg_bits = 8, + .val_bits = 16, + .readable_noinc_reg = smpro_core_readable_noinc_reg, +}; + +static const struct mfd_cell smpro_devs[] = { + MFD_CELL_NAME("smpro-hwmon"), + MFD_CELL_NAME("smpro-errmon"), + MFD_CELL_NAME("smpro-misc"), +}; + +static int smpro_core_probe(struct i2c_client *i2c) +{ + const struct regmap_config *config; + struct regmap *regmap; + unsigned int val; + int ret; + + config = device_get_match_data(&i2c->dev); + if (!config) + return -EINVAL; + + regmap = devm_regmap_init(&i2c->dev, &smpro_regmap_bus, &i2c->dev, config); + if (IS_ERR(regmap)) + return PTR_ERR(regmap); + + ret = regmap_read(regmap, MANUFACTURER_ID_REG, &val); + if (ret) + return ret; + + if (val != AMPERE_MANUFACTURER_ID) + return -ENODEV; + + return devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, + smpro_devs, ARRAY_SIZE(smpro_devs), NULL, 0, NULL); +} + +static const struct of_device_id smpro_core_of_match[] = { + { .compatible = "ampere,smpro", .data = &smpro_regmap_config }, + {} +}; +MODULE_DEVICE_TABLE(of, smpro_core_of_match); + +static struct i2c_driver smpro_core_driver = { + .probe_new = smpro_core_probe, + .driver = { + .name = "smpro-core", + .of_match_table = smpro_core_of_match, + }, +}; +module_i2c_driver(smpro_core_driver); + +MODULE_AUTHOR("Quan Nguyen <quan@os.amperecomputing.com>"); +MODULE_DESCRIPTION("SMPRO CORE - I2C driver"); +MODULE_LICENSE("GPL");
Adds Multi-function devices driver for SMpro co-processor found on the Mt.Jade hardware reference platform with Ampere's Altra processor family. Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com> --- Changes in v9: + Rename smpro-mfd.c to smpro-core.c [Lee Jones] + Fix include files in alphabetical order [Lee Jones] + Add defines to avoid using magic numbers [Lee Jones] + Return -EINVAL if device does not match [Lee Jones] + Remove unneccessary comment [Lee Jones] + Refactor error path handling in smpro_core_read/write() [Quan] Changes in v8: + None Changes in v7: + Smpro-mfd now significant changes in compare with simple-mfd-i2c driver, remove the copyright note about simple-mfd-i2c [Quan] + Install bus->read/write() to handle multiple types of bus access. [Quan] + Update license to MODULE_LICENSE("GPL") [Quan] + Add others minor refactor the code [Quan] Changes in v6: + Update license part to reflect that this driver is clone from simple-mfd-i2c driver [Quan] Changes in v5: + Dropped the use of simple-mfd-i2c driver [Quan] + Introduced drivers/mfd/smpro-mfd.c driver to instantiate sub-devices. This is to avoid DT nodes without resource issue [Quan] + Revised commit message [Quan] Changes in v4: + Add "depends on I2C" to fix build issue found by kernel test robot [Guenter] Changes in v3: + None Changes in v2: + Used 'struct of_device_id's .data attribute [Lee Jones] drivers/mfd/Kconfig | 12 ++++ drivers/mfd/Makefile | 1 + drivers/mfd/smpro-core.c | 138 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 151 insertions(+) create mode 100644 drivers/mfd/smpro-core.c