Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH v3 4/4] HID: touchscreen: Add initial support for Himax HID-over-SPI
From: kernel test robot @ 2023-10-19  6:47 UTC (permalink / raw)
  To: Tylor Yang, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, jikos, benjamin.tissoires, linux-input, devicetree,
	linux-kernel
  Cc: oe-kbuild-all, poyuan_chang, jingyliang, hbarnor, wuxy23, luolm1,
	poyu_hung, Tylor Yang
In-Reply-To: <20231017091900.801989-5-tylor_yang@himax.corp-partner.google.com>

Hi Tylor,

kernel test robot noticed the following build errors:

[auto build test ERROR on hid/for-next]
[also build test ERROR on dtor-input/next dtor-input/for-linus robh/for-next linus/master v6.6-rc6 next-20231018]
[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/Tylor-Yang/dt-bindings-input-Introduce-Himax-HID-over-SPI-device/20231017-172156
base:   https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link:    https://lore.kernel.org/r/20231017091900.801989-5-tylor_yang%40himax.corp-partner.google.com
patch subject: [PATCH v3 4/4] HID: touchscreen: Add initial support for Himax HID-over-SPI
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20231019/202310191454.v9qp5FPx-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231019/202310191454.v9qp5FPx-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310191454.v9qp5FPx-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/hid/hx-hid/hx_core.c: In function 'himax_boot_upgrade':
   drivers/hid/hx-hid/hx_core.c:701:14: warning: variable 'fw_load_status' set but not used [-Wunused-but-set-variable]
     701 |         bool fw_load_status = false;
         |              ^~~~~~~~~~~~~~
   drivers/hid/hx-hid/hx_core.c: At top level:
   drivers/hid/hx-hid/hx_core.c:831:6: warning: no previous prototype for 'hx_hid_update' [-Wmissing-prototypes]
     831 | void hx_hid_update(struct work_struct *work)
         |      ^~~~~~~~~~~~~
   drivers/hid/hx-hid/hx_core.c:890:6: warning: no previous prototype for 'himax_report_data_deinit' [-Wmissing-prototypes]
     890 | void himax_report_data_deinit(struct himax_ts_data *ts)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~
   drivers/hid/hx-hid/hx_core.c:940:5: warning: no previous prototype for 'himax_chip_suspend' [-Wmissing-prototypes]
     940 | int himax_chip_suspend(struct himax_ts_data *ts)
         |     ^~~~~~~~~~~~~~~~~~
   drivers/hid/hx-hid/hx_core.c: In function 'himax_chip_suspend':
   drivers/hid/hx-hid/hx_core.c:942:13: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
     942 |         int ret = 0;
         |             ^~~
   drivers/hid/hx-hid/hx_core.c: At top level:
   drivers/hid/hx-hid/hx_core.c:983:5: warning: no previous prototype for 'himax_chip_resume' [-Wmissing-prototypes]
     983 | int himax_chip_resume(struct himax_ts_data *ts)
         |     ^~~~~~~~~~~~~~~~~
   drivers/hid/hx-hid/hx_core.c: In function 'himax_resume':
>> drivers/hid/hx-hid/hx_core.c:1047:21: error: too few arguments to function 'himax_chip_init'
    1047 |                 if (himax_chip_init())
         |                     ^~~~~~~~~~~~~~~
   In file included from drivers/hid/hx-hid/hx_ic_core.h:6,
                    from drivers/hid/hx-hid/hx_core.c:16:
   drivers/hid/hx-hid/hx_core.h:485:5: note: declared here
     485 | int himax_chip_init(struct himax_ts_data *ts);
         |     ^~~~~~~~~~~~~~~
   drivers/hid/hx-hid/hx_core.c: At top level:
   drivers/hid/hx-hid/hx_core.c:1212:6: warning: no previous prototype for 'himax_chip_deinit' [-Wmissing-prototypes]
    1212 | void himax_chip_deinit(struct himax_ts_data *ts)
         |      ^~~~~~~~~~~~~~~~~
   drivers/hid/hx-hid/hx_core.c: In function 'himax_platform_init':
   drivers/hid/hx-hid/hx_core.c:1271:13: warning: variable 'err' set but not used [-Wunused-but-set-variable]
    1271 |         int err = PROBE_FAIL;
         |             ^~~
   drivers/hid/hx-hid/hx_core.c: At top level:
   drivers/hid/hx-hid/hx_core.c:1353:5: warning: no previous prototype for 'himax_spi_drv_probe' [-Wmissing-prototypes]
    1353 | int himax_spi_drv_probe(struct spi_device *spi)
         |     ^~~~~~~~~~~~~~~~~~~


vim +/himax_chip_init +1047 drivers/hid/hx-hid/hx_core.c

66a3d0692ad03f Tylor Yang 2023-10-17  1034  
66a3d0692ad03f Tylor Yang 2023-10-17  1035  int himax_resume(struct device *dev)
66a3d0692ad03f Tylor Yang 2023-10-17  1036  {
66a3d0692ad03f Tylor Yang 2023-10-17  1037  	int ret = 0;
66a3d0692ad03f Tylor Yang 2023-10-17  1038  	struct himax_ts_data *ts = dev_get_drvdata(dev);
66a3d0692ad03f Tylor Yang 2023-10-17  1039  
66a3d0692ad03f Tylor Yang 2023-10-17  1040  	I("enter");
66a3d0692ad03f Tylor Yang 2023-10-17  1041  	/*
66a3d0692ad03f Tylor Yang 2023-10-17  1042  	 *	wait until device resume for TDDI
66a3d0692ad03f Tylor Yang 2023-10-17  1043  	 *	TDDI: Touch and display Driver IC
66a3d0692ad03f Tylor Yang 2023-10-17  1044  	 */
66a3d0692ad03f Tylor Yang 2023-10-17  1045  	if (!ts->initialized) {
66a3d0692ad03f Tylor Yang 2023-10-17  1046  #if !defined(CONFIG_FB)
66a3d0692ad03f Tylor Yang 2023-10-17 @1047  		if (himax_chip_init())
66a3d0692ad03f Tylor Yang 2023-10-17  1048  			return -ECANCELED;
66a3d0692ad03f Tylor Yang 2023-10-17  1049  #else
66a3d0692ad03f Tylor Yang 2023-10-17  1050  		E("init not ready, skip!");
66a3d0692ad03f Tylor Yang 2023-10-17  1051  		return -ECANCELED;
66a3d0692ad03f Tylor Yang 2023-10-17  1052  #endif
66a3d0692ad03f Tylor Yang 2023-10-17  1053  	}
66a3d0692ad03f Tylor Yang 2023-10-17  1054  	ret = himax_chip_resume(ts);
66a3d0692ad03f Tylor Yang 2023-10-17  1055  	if (ret < 0) {
66a3d0692ad03f Tylor Yang 2023-10-17  1056  		E("resume failed!");
66a3d0692ad03f Tylor Yang 2023-10-17  1057  		I("retry resume");
66a3d0692ad03f Tylor Yang 2023-10-17  1058  		schedule_delayed_work(&ts->work_resume_delayed_work,
66a3d0692ad03f Tylor Yang 2023-10-17  1059  				      msecs_to_jiffies(ts->pdata->ic_resume_delay));
66a3d0692ad03f Tylor Yang 2023-10-17  1060  		// I("try int rescue");
66a3d0692ad03f Tylor Yang 2023-10-17  1061  		// himax_int_enable(ts, 1);
66a3d0692ad03f Tylor Yang 2023-10-17  1062  	}
66a3d0692ad03f Tylor Yang 2023-10-17  1063  
66a3d0692ad03f Tylor Yang 2023-10-17  1064  	return ret;
66a3d0692ad03f Tylor Yang 2023-10-17  1065  }
66a3d0692ad03f Tylor Yang 2023-10-17  1066  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* Re: [PATCH v3 4/4] HID: touchscreen: Add initial support for Himax HID-over-SPI
From: kernel test robot @ 2023-10-19  3:51 UTC (permalink / raw)
  To: Tylor Yang, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, jikos, benjamin.tissoires, linux-input, devicetree,
	linux-kernel
  Cc: oe-kbuild-all, poyuan_chang, jingyliang, hbarnor, wuxy23, luolm1,
	poyu_hung, Tylor Yang
In-Reply-To: <20231017091900.801989-5-tylor_yang@himax.corp-partner.google.com>

Hi Tylor,

kernel test robot noticed the following build warnings:

[auto build test WARNING on hid/for-next]
[also build test WARNING on dtor-input/next dtor-input/for-linus robh/for-next linus/master v6.6-rc6 next-20231018]
[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/Tylor-Yang/dt-bindings-input-Introduce-Himax-HID-over-SPI-device/20231017-172156
base:   https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link:    https://lore.kernel.org/r/20231017091900.801989-5-tylor_yang%40himax.corp-partner.google.com
patch subject: [PATCH v3 4/4] HID: touchscreen: Add initial support for Himax HID-over-SPI
config: xtensa-allyesconfig (https://download.01.org/0day-ci/archive/20231019/202310191116.ulMkjBrE-lkp@intel.com/config)
compiler: xtensa-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231019/202310191116.ulMkjBrE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310191116.ulMkjBrE-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/hid/hx-hid/hx_plat.c:19:6: warning: no previous prototype for 'himax_rst_gpio_set' [-Wmissing-prototypes]
      19 | void himax_rst_gpio_set(int pinnum, u8 value)
         |      ^~~~~~~~~~~~~~~~~~
>> drivers/hid/hx-hid/hx_plat.c:309:13: warning: no previous prototype for 'himax_ts_thread' [-Wmissing-prototypes]
     309 | irqreturn_t himax_ts_thread(int irq, void *ptr)
         |             ^~~~~~~~~~~~~~~
>> drivers/hid/hx-hid/hx_plat.c:461:6: warning: no previous prototype for 'hx_check_power_status' [-Wmissing-prototypes]
     461 | void hx_check_power_status(struct work_struct *work)
         |      ^~~~~~~~~~~~~~~~~~~~~
>> drivers/hid/hx-hid/hx_plat.c:473:5: warning: no previous prototype for 'pwr_notifier_callback' [-Wmissing-prototypes]
     473 | int pwr_notifier_callback(struct notifier_block *self,
         |     ^~~~~~~~~~~~~~~~~~~~~
--
   In file included from include/linux/kernel.h:30,
                    from include/linux/cpumask.h:10,
                    from include/linux/mm_types_task.h:14,
                    from include/linux/mm_types.h:5,
                    from include/linux/buildid.h:5,
                    from include/linux/module.h:14,
                    from drivers/hid/hx-hid/hx_core.h:5,
                    from drivers/hid/hx-hid/hx_ic_core.h:6,
                    from drivers/hid/hx-hid/hx_ic_core.c:16:
   drivers/hid/hx-hid/hx_ic_core.c: In function 'hx_mcu_bin_desc_get':
   include/linux/kern_levels.h:5:25: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
       5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
         |                         ^~~~~~
   include/linux/printk.h:427:25: note: in definition of macro 'printk_index_wrap'
     427 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ^~~~
   include/linux/printk.h:508:9: note: in expansion of macro 'printk'
     508 |         printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~
   include/linux/kern_levels.h:12:25: note: in expansion of macro 'KERN_SOH'
      12 | #define KERN_WARNING    KERN_SOH "4"    /* warning conditions */
         |                         ^~~~~~~~
   include/linux/printk.h:508:16: note: in expansion of macro 'KERN_WARNING'
     508 |         printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
         |                ^~~~~~~~~~~~
   drivers/hid/hx-hid/hx_core.h:64:24: note: in expansion of macro 'pr_warn'
      64 | #define W(fmt, arg...) pr_warn("[HXTP][WARNING][%s]: " fmt "\n", __func__, ##arg)
         |                        ^~~~~~~
   drivers/hid/hx-hid/hx_ic_core.c:1595:25: note: in expansion of macro 'W'
    1595 |                         W("hid_table_addr = %d, ts->hxfw->size = %lu!",
         |                         ^
   include/linux/kern_levels.h:5:25: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
       5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
         |                         ^~~~~~
   include/linux/printk.h:427:25: note: in definition of macro 'printk_index_wrap'
     427 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ^~~~
   include/linux/printk.h:508:9: note: in expansion of macro 'printk'
     508 |         printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~
   include/linux/kern_levels.h:12:25: note: in expansion of macro 'KERN_SOH'
      12 | #define KERN_WARNING    KERN_SOH "4"    /* warning conditions */
         |                         ^~~~~~~~
   include/linux/printk.h:508:16: note: in expansion of macro 'KERN_WARNING'
     508 |         printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
         |                ^~~~~~~~~~~~
   drivers/hid/hx-hid/hx_core.h:64:24: note: in expansion of macro 'pr_warn'
      64 | #define W(fmt, arg...) pr_warn("[HXTP][WARNING][%s]: " fmt "\n", __func__, ##arg)
         |                        ^~~~~~~
   drivers/hid/hx-hid/hx_ic_core.c:1597:25: note: in expansion of macro 'W'
    1597 |                         W("hid_rd_desc_addr = %d, rd_len = %d, ts->hxfw->size = %lu!",
         |                         ^
   drivers/hid/hx-hid/hx_ic_core.c: At top level:
>> drivers/hid/hx-hid/hx_ic_core.c:2404:5: warning: no previous prototype for 'himax_zf_part_info' [-Wmissing-prototypes]
    2404 | int himax_zf_part_info(const struct firmware *fw, struct himax_ts_data *ts,
         |     ^~~~~~~~~~~~~~~~~~
>> drivers/hid/hx-hid/hx_ic_core.c:2598:5: warning: no previous prototype for 'himax_mcu_firmware_update_0f' [-Wmissing-prototypes]
    2598 | int himax_mcu_firmware_update_0f(const struct firmware *fw,
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/hid/hx-hid/hx_ic_core.c:2693:5: warning: no previous prototype for 'hx_0f_op_file_dirly' [-Wmissing-prototypes]
    2693 | int hx_0f_op_file_dirly(char *file_name, struct himax_ts_data *ts)
         |     ^~~~~~~~~~~~~~~~~~~
>> drivers/hid/hx-hid/hx_ic_core.c:2746:6: warning: no previous prototype for 'himax_mcu_read_sram_0f' [-Wmissing-prototypes]
    2746 | void himax_mcu_read_sram_0f(struct himax_ts_data *ts,
         |      ^~~~~~~~~~~~~~~~~~~~~~
>> drivers/hid/hx-hid/hx_ic_core.c:2847:6: warning: no previous prototype for 'himax_mcu_read_all_sram' [-Wmissing-prototypes]
    2847 | void himax_mcu_read_all_sram(struct himax_ts_data *ts, u8 *addr,
         |      ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/hid/hx-hid/hx_ic_core.c:2905:6: warning: no previous prototype for 'himax_mcu_firmware_read_0f' [-Wmissing-prototypes]
    2905 | void himax_mcu_firmware_read_0f(struct himax_ts_data *ts,
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~
--
   drivers/hid/hx-hid/hx_core.c: In function 'himax_boot_upgrade':
>> drivers/hid/hx-hid/hx_core.c:701:14: warning: variable 'fw_load_status' set but not used [-Wunused-but-set-variable]
     701 |         bool fw_load_status = false;
         |              ^~~~~~~~~~~~~~
   drivers/hid/hx-hid/hx_core.c: At top level:
>> drivers/hid/hx-hid/hx_core.c:831:6: warning: no previous prototype for 'hx_hid_update' [-Wmissing-prototypes]
     831 | void hx_hid_update(struct work_struct *work)
         |      ^~~~~~~~~~~~~
>> drivers/hid/hx-hid/hx_core.c:890:6: warning: no previous prototype for 'himax_report_data_deinit' [-Wmissing-prototypes]
     890 | void himax_report_data_deinit(struct himax_ts_data *ts)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/hid/hx-hid/hx_core.c:940:5: warning: no previous prototype for 'himax_chip_suspend' [-Wmissing-prototypes]
     940 | int himax_chip_suspend(struct himax_ts_data *ts)
         |     ^~~~~~~~~~~~~~~~~~
   drivers/hid/hx-hid/hx_core.c: In function 'himax_chip_suspend':
>> drivers/hid/hx-hid/hx_core.c:942:13: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
     942 |         int ret = 0;
         |             ^~~
   drivers/hid/hx-hid/hx_core.c: At top level:
>> drivers/hid/hx-hid/hx_core.c:983:5: warning: no previous prototype for 'himax_chip_resume' [-Wmissing-prototypes]
     983 | int himax_chip_resume(struct himax_ts_data *ts)
         |     ^~~~~~~~~~~~~~~~~
>> drivers/hid/hx-hid/hx_core.c:1212:6: warning: no previous prototype for 'himax_chip_deinit' [-Wmissing-prototypes]
    1212 | void himax_chip_deinit(struct himax_ts_data *ts)
         |      ^~~~~~~~~~~~~~~~~
   drivers/hid/hx-hid/hx_core.c: In function 'himax_platform_init':
>> drivers/hid/hx-hid/hx_core.c:1271:13: warning: variable 'err' set but not used [-Wunused-but-set-variable]
    1271 |         int err = PROBE_FAIL;
         |             ^~~
   drivers/hid/hx-hid/hx_core.c: At top level:
>> drivers/hid/hx-hid/hx_core.c:1353:5: warning: no previous prototype for 'himax_spi_drv_probe' [-Wmissing-prototypes]
    1353 | int himax_spi_drv_probe(struct spi_device *spi)
         |     ^~~~~~~~~~~~~~~~~~~
--
   In file included from include/asm-generic/bug.h:22,
                    from ./arch/xtensa/include/generated/asm/bug.h:1,
                    from include/linux/bug.h:5,
                    from include/linux/thread_info.h:13,
                    from include/asm-generic/preempt.h:5,
                    from ./arch/xtensa/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:79,
                    from include/linux/spinlock.h:56,
                    from include/linux/mmzone.h:8,
                    from include/linux/gfp.h:7,
                    from include/linux/slab.h:16,
                    from include/linux/hid.h:19,
                    from drivers/hid/hx-hid/hx_hid.c:16:
   drivers/hid/hx-hid/hx_hid.c: In function 'hx_hid_get_raw_report':
>> drivers/hid/hx-hid/hx_core.h:66:33: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
      66 | #define D(fmt, arg...) pr_debug("[HXTP][DEBUG][%s]: " fmt "\n", __func__, ##arg)
         |                                 ^~~~~~~~~~~~~~~~~~~~~
   include/linux/printk.h:345:21: note: in definition of macro 'pr_fmt'
     345 | #define pr_fmt(fmt) fmt
         |                     ^~~
   include/linux/dynamic_debug.h:248:9: note: in expansion of macro '__dynamic_func_call_cls'
     248 |         __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:250:9: note: in expansion of macro '_dynamic_func_call_cls'
     250 |         _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call'
     269 |         _dynamic_func_call(fmt, __dynamic_pr_debug,             \
         |         ^~~~~~~~~~~~~~~~~~
   include/linux/printk.h:579:9: note: in expansion of macro 'dynamic_pr_debug'
     579 |         dynamic_pr_debug(fmt, ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~
   drivers/hid/hx-hid/hx_core.h:66:24: note: in expansion of macro 'pr_debug'
      66 | #define D(fmt, arg...) pr_debug("[HXTP][DEBUG][%s]: " fmt "\n", __func__, ##arg)
         |                        ^~~~~~~~
   drivers/hid/hx-hid/hx_hid.c:264:9: note: in expansion of macro 'D'
     264 |         D("reportnum:%d, len:%lu, report_type:%d", reportnum, len, report_type);
         |         ^
   drivers/hid/hx-hid/hx_hid.c: In function 'hx_hid_set_raw_report':
>> drivers/hid/hx-hid/hx_core.h:66:33: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
      66 | #define D(fmt, arg...) pr_debug("[HXTP][DEBUG][%s]: " fmt "\n", __func__, ##arg)
         |                                 ^~~~~~~~~~~~~~~~~~~~~
   include/linux/printk.h:345:21: note: in definition of macro 'pr_fmt'
     345 | #define pr_fmt(fmt) fmt
         |                     ^~~
   include/linux/dynamic_debug.h:248:9: note: in expansion of macro '__dynamic_func_call_cls'
     248 |         __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:250:9: note: in expansion of macro '_dynamic_func_call_cls'
     250 |         _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call'
     269 |         _dynamic_func_call(fmt, __dynamic_pr_debug,             \
         |         ^~~~~~~~~~~~~~~~~~
   include/linux/printk.h:579:9: note: in expansion of macro 'dynamic_pr_debug'
     579 |         dynamic_pr_debug(fmt, ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~
   drivers/hid/hx-hid/hx_core.h:66:24: note: in expansion of macro 'pr_debug'
      66 | #define D(fmt, arg...) pr_debug("[HXTP][DEBUG][%s]: " fmt "\n", __func__, ##arg)
         |                        ^~~~~~~~
   drivers/hid/hx-hid/hx_hid.c:437:9: note: in expansion of macro 'D'
     437 |         D("reportnum:%d, len:%lu, report_type:%d", reportnum, len, report_type);
         |         ^
   drivers/hid/hx-hid/hx_hid.c: In function 'hx_raw_request':
>> drivers/hid/hx-hid/hx_core.h:66:33: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
      66 | #define D(fmt, arg...) pr_debug("[HXTP][DEBUG][%s]: " fmt "\n", __func__, ##arg)
         |                                 ^~~~~~~~~~~~~~~~~~~~~
   include/linux/printk.h:345:21: note: in definition of macro 'pr_fmt'
     345 | #define pr_fmt(fmt) fmt
         |                     ^~~
   include/linux/dynamic_debug.h:248:9: note: in expansion of macro '__dynamic_func_call_cls'
     248 |         __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:250:9: note: in expansion of macro '_dynamic_func_call_cls'
     250 |         _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call'
     269 |         _dynamic_func_call(fmt, __dynamic_pr_debug,             \
         |         ^~~~~~~~~~~~~~~~~~
   include/linux/printk.h:579:9: note: in expansion of macro 'dynamic_pr_debug'
     579 |         dynamic_pr_debug(fmt, ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~
   drivers/hid/hx-hid/hx_core.h:66:24: note: in expansion of macro 'pr_debug'
      66 | #define D(fmt, arg...) pr_debug("[HXTP][DEBUG][%s]: " fmt "\n", __func__, ##arg)
         |                        ^~~~~~~~
   drivers/hid/hx-hid/hx_hid.c:621:9: note: in expansion of macro 'D'
     621 |         D("report num %d, len %lu, rtype %d, reqtype %d", reportnum, len, rtype, reqtype);
         |         ^


vim +/himax_rst_gpio_set +19 drivers/hid/hx-hid/hx_plat.c

    18	
  > 19	void himax_rst_gpio_set(int pinnum, u8 value)
    20	{
    21		gpio_direction_output(pinnum, value);
    22	}
    23	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* Re: [PATCH v4 2/5] dt-bindings: mfd: ti,twl: Add clock provider properties
From: Stephen Boyd @ 2023-10-19  1:22 UTC (permalink / raw)
  To: andreas, bcousson, conor+dt, devicetree, dmitry.torokhov,
	krzysztof.kozlowski+dt, lee, linux-clk, linux-input, linux-kernel,
	linux-omap, mturquette, robh+dt, tony
  Cc: Conor Dooley
In-Reply-To: <20230916100515.1650336-3-andreas@kemnade.info>

Quoting Andreas Kemnade (2023-09-16 03:05:12)
> Since these devices provide clock outputs, add the corresponding
> property.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> ---

Acked-by: Stephen Boyd <sboyd@kernel.org>

^ permalink raw reply

* Re: [PATCH v4 4/5] clk: twl: add clock driver for TWL6032
From: Stephen Boyd @ 2023-10-19  1:20 UTC (permalink / raw)
  To: andreas, bcousson, conor+dt, devicetree, dmitry.torokhov,
	krzysztof.kozlowski+dt, lee, linux-clk, linux-input, linux-kernel,
	linux-omap, mturquette, robh+dt, tony
In-Reply-To: <20230916100515.1650336-5-andreas@kemnade.info>

Quoting Andreas Kemnade (2023-09-16 03:05:14)
> The TWL6032 has some clock outputs which are controlled like
> fixed-voltage regulators, in some drivers for these chips
> found in the wild, just the regulator api is abused for controlling
> them, so simply use something similar to the regulator functions.
> Due to a lack of hardware available for testing, leave out the
> TWL6030-specific part of those functions.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---

Did you want me to pick this up in clk tree?

Acked-by: Stephen Boyd <sboyd@kernel.org>

^ permalink raw reply

* Re: [PATCH v5 2/2] input: joystick: driver for Adafruit Seesaw Gamepad
From: kernel test robot @ 2023-10-19  0:10 UTC (permalink / raw)
  To: Anshul Dalal, linux-input, devicetree
  Cc: oe-kbuild-all, Anshul Dalal, Dmitry Torokhov, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Thomas Weißschuh,
	Shuah Khan, linux-kernel-mentees, linux-kernel
In-Reply-To: <20231017034356.1436677-2-anshulusr@gmail.com>

Hi Anshul,

kernel test robot noticed the following build warnings:

[auto build test WARNING on dtor-input/next]
[also build test WARNING on dtor-input/for-linus hid/for-next linus/master v6.6-rc6 next-20231018]
[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/Anshul-Dalal/input-joystick-driver-for-Adafruit-Seesaw-Gamepad/20231017-160635
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link:    https://lore.kernel.org/r/20231017034356.1436677-2-anshulusr%40gmail.com
patch subject: [PATCH v5 2/2] input: joystick: driver for Adafruit Seesaw Gamepad
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20231019/202310190852.BCw4Ry7D-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231019/202310190852.BCw4Ry7D-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310190852.BCw4Ry7D-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/thread_info.h:27,
                    from arch/sparc/include/asm/current.h:15,
                    from include/linux/sched.h:12,
                    from include/linux/delay.h:23,
                    from drivers/input/joystick/adafruit-seesaw.c:17:
   drivers/input/joystick/adafruit-seesaw.c: In function 'seesaw_read_data':
>> include/linux/bitops.h:52:11: warning: array subscript 'long unsigned int[0]' is partly outside array bounds of 'u32[1]' {aka 'unsigned int[1]'} [-Warray-bounds=]
      52 |           __builtin_constant_p(*(const unsigned long *)(addr))) ?       \
         |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/bitops.h:61:41: note: in expansion of macro 'bitop'
      61 | #define test_bit(nr, addr)              bitop(_test_bit, nr, addr)
         |                                         ^~~~~
   drivers/input/joystick/adafruit-seesaw.c:89:27: note: in expansion of macro 'test_bit'
      89 |         data->button_a = !test_bit(BUTTON_A, (long *)&result);
         |                           ^~~~~~~~
   drivers/input/joystick/adafruit-seesaw.c:87:13: note: object 'result' of size 4
      87 |         u32 result = get_unaligned_be32(&read_buf);
         |             ^~~~~~
   In file included from include/linux/bitops.h:34:
   In function 'generic_test_bit',
       inlined from 'seesaw_read_data' at drivers/input/joystick/adafruit-seesaw.c:89:20:
>> include/asm-generic/bitops/generic-non-atomic.h:128:27: warning: array subscript 'long unsigned int[0]' is partly outside array bounds of 'u32[1]' {aka 'unsigned int[1]'} [-Warray-bounds=]
     128 |         return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
         |                       ~~~~^~~~~~~~~~~~~~
   drivers/input/joystick/adafruit-seesaw.c: In function 'seesaw_read_data':
   drivers/input/joystick/adafruit-seesaw.c:87:13: note: object 'result' of size 4
      87 |         u32 result = get_unaligned_be32(&read_buf);
         |             ^~~~~~
   In function 'generic_test_bit',
       inlined from 'seesaw_read_data' at drivers/input/joystick/adafruit-seesaw.c:90:20:
>> include/asm-generic/bitops/generic-non-atomic.h:128:27: warning: array subscript 'long unsigned int[0]' is partly outside array bounds of 'u32[1]' {aka 'unsigned int[1]'} [-Warray-bounds=]
     128 |         return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
         |                       ~~~~^~~~~~~~~~~~~~
   drivers/input/joystick/adafruit-seesaw.c: In function 'seesaw_read_data':
   drivers/input/joystick/adafruit-seesaw.c:87:13: note: object 'result' of size 4
      87 |         u32 result = get_unaligned_be32(&read_buf);
         |             ^~~~~~
   In function 'generic_test_bit',
       inlined from 'seesaw_read_data' at drivers/input/joystick/adafruit-seesaw.c:91:20:
>> include/asm-generic/bitops/generic-non-atomic.h:128:27: warning: array subscript 'long unsigned int[0]' is partly outside array bounds of 'u32[1]' {aka 'unsigned int[1]'} [-Warray-bounds=]
     128 |         return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
         |                       ~~~~^~~~~~~~~~~~~~
   drivers/input/joystick/adafruit-seesaw.c: In function 'seesaw_read_data':
   drivers/input/joystick/adafruit-seesaw.c:87:13: note: object 'result' of size 4
      87 |         u32 result = get_unaligned_be32(&read_buf);
         |             ^~~~~~
   In function 'generic_test_bit',
       inlined from 'seesaw_read_data' at drivers/input/joystick/adafruit-seesaw.c:92:20:
>> include/asm-generic/bitops/generic-non-atomic.h:128:27: warning: array subscript 'long unsigned int[0]' is partly outside array bounds of 'u32[1]' {aka 'unsigned int[1]'} [-Warray-bounds=]
     128 |         return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
         |                       ~~~~^~~~~~~~~~~~~~
   drivers/input/joystick/adafruit-seesaw.c: In function 'seesaw_read_data':
   drivers/input/joystick/adafruit-seesaw.c:87:13: note: object 'result' of size 4
      87 |         u32 result = get_unaligned_be32(&read_buf);
         |             ^~~~~~
   In function 'generic_test_bit',
       inlined from 'seesaw_read_data' at drivers/input/joystick/adafruit-seesaw.c:93:24:
>> include/asm-generic/bitops/generic-non-atomic.h:128:27: warning: array subscript 'long unsigned int[0]' is partly outside array bounds of 'u32[1]' {aka 'unsigned int[1]'} [-Warray-bounds=]
     128 |         return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
         |                       ~~~~^~~~~~~~~~~~~~
   drivers/input/joystick/adafruit-seesaw.c: In function 'seesaw_read_data':
   drivers/input/joystick/adafruit-seesaw.c:87:13: note: object 'result' of size 4
      87 |         u32 result = get_unaligned_be32(&read_buf);
         |             ^~~~~~
   In function 'generic_test_bit',
       inlined from 'seesaw_read_data' at drivers/input/joystick/adafruit-seesaw.c:94:25:
>> include/asm-generic/bitops/generic-non-atomic.h:128:27: warning: array subscript 'long unsigned int[0]' is partly outside array bounds of 'u32[1]' {aka 'unsigned int[1]'} [-Warray-bounds=]
     128 |         return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
         |                       ~~~~^~~~~~~~~~~~~~
   drivers/input/joystick/adafruit-seesaw.c: In function 'seesaw_read_data':
   drivers/input/joystick/adafruit-seesaw.c:87:13: note: object 'result' of size 4
      87 |         u32 result = get_unaligned_be32(&read_buf);
         |             ^~~~~~


vim +52 include/linux/bitops.h

0e862838f29014 Alexander Lobakin 2022-06-24  35  
b03fc1173c0c2b Alexander Lobakin 2022-06-24  36  /*
b03fc1173c0c2b Alexander Lobakin 2022-06-24  37   * Many architecture-specific non-atomic bitops contain inline asm code and due
b03fc1173c0c2b Alexander Lobakin 2022-06-24  38   * to that the compiler can't optimize them to compile-time expressions or
b03fc1173c0c2b Alexander Lobakin 2022-06-24  39   * constants. In contrary, generic_*() helpers are defined in pure C and
b03fc1173c0c2b Alexander Lobakin 2022-06-24  40   * compilers optimize them just well.
b03fc1173c0c2b Alexander Lobakin 2022-06-24  41   * Therefore, to make `unsigned long foo = 0; __set_bit(BAR, &foo)` effectively
b03fc1173c0c2b Alexander Lobakin 2022-06-24  42   * equal to `unsigned long foo = BIT(BAR)`, pick the generic C alternative when
b03fc1173c0c2b Alexander Lobakin 2022-06-24  43   * the arguments can be resolved at compile time. That expression itself is a
b03fc1173c0c2b Alexander Lobakin 2022-06-24  44   * constant and doesn't bring any functional changes to the rest of cases.
b03fc1173c0c2b Alexander Lobakin 2022-06-24  45   * The casts to `uintptr_t` are needed to mitigate `-Waddress` warnings when
b03fc1173c0c2b Alexander Lobakin 2022-06-24  46   * passing a bitmap from .bss or .data (-> `!!addr` is always true).
b03fc1173c0c2b Alexander Lobakin 2022-06-24  47   */
e69eb9c460f128 Alexander Lobakin 2022-06-24  48  #define bitop(op, nr, addr)						\
b03fc1173c0c2b Alexander Lobakin 2022-06-24  49  	((__builtin_constant_p(nr) &&					\
b03fc1173c0c2b Alexander Lobakin 2022-06-24  50  	  __builtin_constant_p((uintptr_t)(addr) != (uintptr_t)NULL) &&	\
b03fc1173c0c2b Alexander Lobakin 2022-06-24  51  	  (uintptr_t)(addr) != (uintptr_t)NULL &&			\
b03fc1173c0c2b Alexander Lobakin 2022-06-24 @52  	  __builtin_constant_p(*(const unsigned long *)(addr))) ?	\
b03fc1173c0c2b Alexander Lobakin 2022-06-24  53  	 const##op(nr, addr) : op(nr, addr))
e69eb9c460f128 Alexander Lobakin 2022-06-24  54  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* Re: [PATCH v2 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
From: kernel test robot @ 2023-10-18 22:17 UTC (permalink / raw)
  To: Caleb Connolly, Dmitry Torokhov, Vincent Huang
  Cc: llvm, oe-kbuild-all, methanal, linux-input, devicetree,
	phone-devel, ~postmarketos/upstreaming, Caleb Connolly
In-Reply-To: <20230929-caleb-rmi4-quirks-v2-7-b227ac498d88@linaro.org>

Hi Caleb,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.6-rc6 next-20231018]
[cannot apply to dtor-input/next dtor-input/for-linus]
[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/Caleb-Connolly/Input-synaptics-rmi4-handle-duplicate-unknown-PDT-entries/20231017-132844
base:   linus/master
patch link:    https://lore.kernel.org/r/20230929-caleb-rmi4-quirks-v2-7-b227ac498d88%40linaro.org
patch subject: [PATCH v2 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20231019/202310190640.pxJQCbWc-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231019/202310190640.pxJQCbWc-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310190640.pxJQCbWc-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/input/rmi4/rmi_driver.c:1223:32: error: incompatible pointer types passing 'struct rmi_device *' to parameter of type 'struct device *' [-Werror,-Wincompatible-pointer-types]
                   retval = rmi_driver_of_probe(rmi_dev, pdata);
                                                ^~~~~~~
   drivers/input/rmi4/rmi_driver.c:1101:54: note: passing argument to parameter 'dev' here
   static inline int rmi_driver_of_probe(struct device *dev,
                                                        ^
   1 error generated.


vim +1223 drivers/input/rmi4/rmi_driver.c

  1199	
  1200	static int rmi_driver_probe(struct device *dev)
  1201	{
  1202		struct rmi_driver *rmi_driver;
  1203		struct rmi_driver_data *data;
  1204		struct rmi_device_platform_data *pdata;
  1205		struct rmi_device *rmi_dev;
  1206		int retval;
  1207	
  1208		rmi_dbg(RMI_DEBUG_CORE, dev, "%s: Starting probe.\n",
  1209				__func__);
  1210	
  1211		if (!rmi_is_physical_device(dev)) {
  1212			rmi_dbg(RMI_DEBUG_CORE, dev, "Not a physical device.\n");
  1213			return -ENODEV;
  1214		}
  1215	
  1216		rmi_dev = to_rmi_device(dev);
  1217		rmi_driver = to_rmi_driver(dev->driver);
  1218		rmi_dev->driver = rmi_driver;
  1219	
  1220		pdata = rmi_get_platform_data(rmi_dev);
  1221	
  1222		if (rmi_dev->xport->dev->of_node) {
> 1223			retval = rmi_driver_of_probe(rmi_dev, pdata);
  1224			if (retval)
  1225				return retval;
  1226		}
  1227	
  1228		data = devm_kzalloc(dev, sizeof(struct rmi_driver_data), GFP_KERNEL);
  1229		if (!data)
  1230			return -ENOMEM;
  1231	
  1232		INIT_LIST_HEAD(&data->function_list);
  1233		data->rmi_dev = rmi_dev;
  1234		dev_set_drvdata(&rmi_dev->dev, data);
  1235	
  1236		/*
  1237		 * Right before a warm boot, the sensor might be in some unusual state,
  1238		 * such as F54 diagnostics, or F34 bootloader mode after a firmware
  1239		 * or configuration update.  In order to clear the sensor to a known
  1240		 * state and/or apply any updates, we issue a initial reset to clear any
  1241		 * previous settings and force it into normal operation.
  1242		 *
  1243		 * We have to do this before actually building the PDT because
  1244		 * the reflash updates (if any) might cause various registers to move
  1245		 * around.
  1246		 *
  1247		 * For a number of reasons, this initial reset may fail to return
  1248		 * within the specified time, but we'll still be able to bring up the
  1249		 * driver normally after that failure.  This occurs most commonly in
  1250		 * a cold boot situation (where then firmware takes longer to come up
  1251		 * than from a warm boot) and the reset_delay_ms in the platform data
  1252		 * has been set too short to accommodate that.  Since the sensor will
  1253		 * eventually come up and be usable, we don't want to just fail here
  1254		 * and leave the customer's device unusable.  So we warn them, and
  1255		 * continue processing.
  1256		 */
  1257		retval = rmi_scan_pdt(rmi_dev, NULL, rmi_initial_reset);
  1258		if (retval < 0)
  1259			dev_warn(dev, "RMI initial reset failed! Continuing in spite of this.\n");
  1260	
  1261		retval = rmi_read(rmi_dev, PDT_PROPERTIES_LOCATION, &data->pdt_props);
  1262		if (retval < 0) {
  1263			/*
  1264			 * we'll print out a warning and continue since
  1265			 * failure to get the PDT properties is not a cause to fail
  1266			 */
  1267			dev_warn(dev, "Could not read PDT properties from %#06x (code %d). Assuming 0x00.\n",
  1268				 PDT_PROPERTIES_LOCATION, retval);
  1269		}
  1270	
  1271		mutex_init(&data->irq_mutex);
  1272		mutex_init(&data->enabled_mutex);
  1273	
  1274		retval = rmi_probe_interrupts(data);
  1275		if (retval)
  1276			goto err;
  1277	
  1278		if (rmi_dev->xport->input) {
  1279			/*
  1280			 * The transport driver already has an input device.
  1281			 * In some cases it is preferable to reuse the transport
  1282			 * devices input device instead of creating a new one here.
  1283			 * One example is some HID touchpads report "pass-through"
  1284			 * button events are not reported by rmi registers.
  1285			 */
  1286			data->input = rmi_dev->xport->input;
  1287		} else {
  1288			data->input = devm_input_allocate_device(dev);
  1289			if (!data->input) {
  1290				dev_err(dev, "%s: Failed to allocate input device.\n",
  1291					__func__);
  1292				retval = -ENOMEM;
  1293				goto err;
  1294			}
  1295			rmi_driver_set_input_params(rmi_dev, data->input);
  1296			data->input->phys = devm_kasprintf(dev, GFP_KERNEL,
  1297							"%s/input0", dev_name(dev));
  1298		}
  1299	
  1300		retval = rmi_init_functions(data);
  1301		if (retval)
  1302			goto err;
  1303	
  1304		retval = rmi_f34_create_sysfs(rmi_dev);
  1305		if (retval)
  1306			goto err;
  1307	
  1308		if (data->input) {
  1309			rmi_driver_set_input_name(rmi_dev, data->input);
  1310			if (!rmi_dev->xport->input) {
  1311				retval = input_register_device(data->input);
  1312				if (retval) {
  1313					dev_err(dev, "%s: Failed to register input device.\n",
  1314						__func__);
  1315					goto err_destroy_functions;
  1316				}
  1317			}
  1318		}
  1319	
  1320		retval = rmi_irq_init(rmi_dev);
  1321		if (retval < 0)
  1322			goto err_destroy_functions;
  1323	
  1324		if (data->f01_container->dev.driver) {
  1325			/* Driver already bound, so enable ATTN now. */
  1326			retval = rmi_enable_sensor(rmi_dev);
  1327			if (retval)
  1328				goto err_disable_irq;
  1329		}
  1330	
  1331		return 0;
  1332	
  1333	err_disable_irq:
  1334		rmi_disable_irq(rmi_dev, false);
  1335	err_destroy_functions:
  1336		rmi_free_function_list(rmi_dev);
  1337	err:
  1338		return retval;
  1339	}
  1340	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* Re: [PATCH] input: gpio-keys - optimize wakeup sequence.
From: Gareth Randall @ 2023-10-18 21:49 UTC (permalink / raw)
  To: abhi1.singh
  Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	SRI-N IT Security
In-Reply-To: <1830198030.734987.1697538855953@mail-kr5-1.mail-kr5.knoxportal-kr-prod-blue.svc.cluster.local>

Dear Mr Singh,

I am not a maintainer but can point out some issues you need to resolve 
with this post.

1. Put the patch in the body of the email and not in an attachment.
2. You need a "Signed-off-by:" line.
3. The email needs to start with a description of the what the patch 
resolves. Don't put "suggested changes".

There are probably other issues as well but I hope this helps you to get 
started. Note that I am not involved in the review process.

Yours,

Gareth

On 17/10/2023 11:34, Abhishek Kumar Singh wrote:
> Dear Mr. Dmitry,
> 
> Greetings!
> 
> 
> 
> This patch is related to optimization in input key event driver of Kernel module.
> 
> Suggested change to avoid the many APIs call chain if there is no key press event triggered.
> 
> 
> 
> There is a call back function gpio_keys_resume() called for every suspend/resume of the device.
> 
> And whenever this function is called, it is reading the status of the key.
> 
> And gpio_keys_resume() API further calls the below chain of API irrespective of key press event.
> 
> 
> 
> APIs call chain:
> 
> static void gpio_keys_report_state(struct gpio_keys_drvdata *ddata)
> 
> static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
> 
> gpiod_get_value_cansleep(bdata->gpiod);
> 
> input_event(input, type, *bdata->code, state);
> 
> input_sync(input);
> 
> 
> 
> 
> Suggested changes to avoid the above APIs call chain if there is no key press event triggered.
> 
> It will save the device computational resources, power resources and optimize the suspend/resume time"
> 
> 
> Please help to review the attached patch and integrate in main line kernel code.
> 
>   
> 
> 
> 
> Thanks and Regards,
> Abhishek Kumar Singh
> Sr. Chief Engineer, Samsung Electronics, Noida-India


^ permalink raw reply

* Re: [PATCH v4 1/4] dt-bindings: input: cirrus,cs40l50: Add initial DT binding
From: James Ogletree @ 2023-10-18 21:44 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: James Ogletree, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Lee Jones, Fred Treven, Ben Bright, linux-input@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <65cc9c43-e776-41bc-adad-1e57c3b24d7f@linaro.org>



> On Oct 18, 2023, at 2:21 PM, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
> On 18/10/2023 19:57, James Ogletree wrote:
>> From: James Ogletree <james.ogletree@cirrus.com>
>> 
>> The CS40L50 is a haptic driver with waveform memory,
>> integrated DSP, and closed-loop algorithms.
>> 
>> Add a YAML DT binding document for this device.
>> 
> 
> This is a friendly reminder during the review process.
> 
> It looks like you received a tag and forgot to add it.
> 
> If you do not know the process, here is a short explanation:
> Please add Acked-by/Reviewed-by/Tested-by tags when posting new
> versions, under or above your Signed-off-by tag. Tag is "received", when
> provided in a message replied to you on the mailing list. Tools like b4
> can help here. However, there's no need to repost patches *only* to add
> the tags. The upstream maintainer will do that for tags received on the
> version they apply.
> 
> https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst*L577__;Iw!!DQ3KfwI!1EWk9UBnRfBQy30s9CXXIfiyzRiXLDvIiZsri22s9tJuRYN-X0PHPgMwZsqkKEq2hSBTrsP1Rj0hTWa4ws8u42Em84kK3mI$ 
> 
> If a tag was not added on purpose, please state why and what changed.
> 
> Best regards,
> Krzysztof

Noted for the future. This was an accidental leave-out.

Best,
James


^ permalink raw reply

* Re: [PATCH v4 1/4] dt-bindings: input: cirrus,cs40l50: Add initial DT binding
From: Krzysztof Kozlowski @ 2023-10-18 19:21 UTC (permalink / raw)
  To: James Ogletree
  Cc: James Ogletree, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Lee Jones, Fred Treven, Ben Bright, linux-input, devicetree,
	linux-kernel
In-Reply-To: <20231018175726.3879955-2-james.ogletree@opensource.cirrus.com>

On 18/10/2023 19:57, James Ogletree wrote:
> From: James Ogletree <james.ogletree@cirrus.com>
> 
> The CS40L50 is a haptic driver with waveform memory,
> integrated DSP, and closed-loop algorithms.
> 
> Add a YAML DT binding document for this device.
> 

This is a friendly reminder during the review process.

It looks like you received a tag and forgot to add it.

If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

If a tag was not added on purpose, please state why and what changed.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
From: Hans de Goede @ 2023-10-18 19:08 UTC (permalink / raw)
  To: Mario Limonciello, Christian König, Shyam Sundar S K,
	Ilpo Järvinen
  Cc: markgross, basavaraj.natikar, jikos, benjamin.tissoires,
	alexander.deucher, Xinhui.Pan, airlied, daniel, Patil.Reddy,
	platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <238f915f-b95f-4d85-ad67-66781f53e75d@amd.com>

Hi,

I was not following this at first, so my apologies for
jumping in in the middle of the thread:


<snip>

>>>>> +static int amd_pmf_gpu_get_cur_state(struct thermal_cooling_device *cooling_dev,
>>>>> +                     unsigned long *state)
>>>>> +{
>>>>> +    struct backlight_device *bd;
>>>>> +
>>>>> +    if (!acpi_video_backlight_use_native())
>>>>> +        return -ENODEV;
>>>>> +
>>>>> +    bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>>>> +    if (!bd)
>>>>> +        return -ENODEV;
>>>>> +
>>>>> +    *state = backlight_get_brightness(bd);
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static int amd_pmf_gpu_get_max_state(struct thermal_cooling_device *cooling_dev,
>>>>> +                     unsigned long *state)
>>>>> +{
>>>>> +    struct backlight_device *bd;
>>>>> +
>>>>> +    if (!acpi_video_backlight_use_native())
>>>>> +        return -ENODEV;
>>>>> +
>>>>> +    bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>>>> +    if (!bd)
>>>>> +        return -ENODEV;
>>>>> +
>>>>> +    if (backlight_is_blank(bd))
>>>>> +        *state = 0;
>>>>> +    else
>>>>> +        *state = bd->props.max_brightness;
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static const struct thermal_cooling_device_ops bd_cooling_ops = {
>>>>> +    .get_max_state = amd_pmf_gpu_get_max_state,
>>>>> +    .get_cur_state = amd_pmf_gpu_get_cur_state,
>>>>> +};

So first of all, good to see that this is using the
thermal_cooling_device APIs now, that is great thank you.

But the whole idea behind using the thermal_cooling_device APIs
is that amdgpu exports the cooling_device itself, rather then have
the AMD PMF code export it. Now the AMD PMF code is still poking
at the backlight_device itself, while the idea was to delegate
this to the GPU driver.

Actually seeing all the acpi_video_backlight_use_native()
checks here, I wonder why only have this work with native backlight
control. One step better would be to add thermal_cooling_device
support to the backlight core in:
drivers/video/backlight/backlight.c

Then it will work with any backlight control provider!



Last but not least this code MUST not call
acpi_video_backlight_use_native()

No code other then native GPU drivers must ever call
acpi_video_backlight_use_native(). This special function
not only checks if the native backlight control is the
one which the detection code in drivers/acpi/video_detect.c
has selected, it also signals to video_detect.c that
native GPU backlight control is available.

So by calling this in the AMD PMF code you are now
telling video_detect.c that native GPU backlight control
is available on all systems where AMD PMF runs.

As I already said I really believe the whole cooling
device should be registered somewhere else. But if you
do end up sticking with this then you MUST replace
the acpi_video_backlight_use_native() calls with:

	if (acpi_video_get_backlight_type() == acpi_backlight_native) {...}

Regards,

Hans




^ permalink raw reply

* Re: [PATCH v2 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
From: kernel test robot @ 2023-10-18 18:14 UTC (permalink / raw)
  To: Caleb Connolly, Dmitry Torokhov, Vincent Huang
  Cc: oe-kbuild-all, methanal, linux-input, devicetree, phone-devel,
	~postmarketos/upstreaming, Caleb Connolly
In-Reply-To: <20230929-caleb-rmi4-quirks-v2-7-b227ac498d88@linaro.org>

Hi Caleb,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.6-rc6 next-20231018]
[cannot apply to dtor-input/next dtor-input/for-linus]
[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/Caleb-Connolly/Input-synaptics-rmi4-handle-duplicate-unknown-PDT-entries/20231017-132844
base:   linus/master
patch link:    https://lore.kernel.org/r/20230929-caleb-rmi4-quirks-v2-7-b227ac498d88%40linaro.org
patch subject: [PATCH v2 7/7] Input: synaptics-rmi4 - support fallback values for PDT descriptor bytes
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20231019/202310190215.hvnjZ9RG-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231019/202310190215.hvnjZ9RG-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310190215.hvnjZ9RG-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/input/rmi4/rmi_driver.c: In function 'rmi_driver_probe':
>> drivers/input/rmi4/rmi_driver.c:1223:46: error: passing argument 1 of 'rmi_driver_of_probe' from incompatible pointer type [-Werror=incompatible-pointer-types]
    1223 |                 retval = rmi_driver_of_probe(rmi_dev, pdata);
         |                                              ^~~~~~~
         |                                              |
         |                                              struct rmi_device *
   drivers/input/rmi4/rmi_driver.c:1101:54: note: expected 'struct device *' but argument is of type 'struct rmi_device *'
    1101 | static inline int rmi_driver_of_probe(struct device *dev,
         |                                       ~~~~~~~~~~~~~~~^~~
   cc1: some warnings being treated as errors


vim +/rmi_driver_of_probe +1223 drivers/input/rmi4/rmi_driver.c

  1199	
  1200	static int rmi_driver_probe(struct device *dev)
  1201	{
  1202		struct rmi_driver *rmi_driver;
  1203		struct rmi_driver_data *data;
  1204		struct rmi_device_platform_data *pdata;
  1205		struct rmi_device *rmi_dev;
  1206		int retval;
  1207	
  1208		rmi_dbg(RMI_DEBUG_CORE, dev, "%s: Starting probe.\n",
  1209				__func__);
  1210	
  1211		if (!rmi_is_physical_device(dev)) {
  1212			rmi_dbg(RMI_DEBUG_CORE, dev, "Not a physical device.\n");
  1213			return -ENODEV;
  1214		}
  1215	
  1216		rmi_dev = to_rmi_device(dev);
  1217		rmi_driver = to_rmi_driver(dev->driver);
  1218		rmi_dev->driver = rmi_driver;
  1219	
  1220		pdata = rmi_get_platform_data(rmi_dev);
  1221	
  1222		if (rmi_dev->xport->dev->of_node) {
> 1223			retval = rmi_driver_of_probe(rmi_dev, pdata);
  1224			if (retval)
  1225				return retval;
  1226		}
  1227	
  1228		data = devm_kzalloc(dev, sizeof(struct rmi_driver_data), GFP_KERNEL);
  1229		if (!data)
  1230			return -ENOMEM;
  1231	
  1232		INIT_LIST_HEAD(&data->function_list);
  1233		data->rmi_dev = rmi_dev;
  1234		dev_set_drvdata(&rmi_dev->dev, data);
  1235	
  1236		/*
  1237		 * Right before a warm boot, the sensor might be in some unusual state,
  1238		 * such as F54 diagnostics, or F34 bootloader mode after a firmware
  1239		 * or configuration update.  In order to clear the sensor to a known
  1240		 * state and/or apply any updates, we issue a initial reset to clear any
  1241		 * previous settings and force it into normal operation.
  1242		 *
  1243		 * We have to do this before actually building the PDT because
  1244		 * the reflash updates (if any) might cause various registers to move
  1245		 * around.
  1246		 *
  1247		 * For a number of reasons, this initial reset may fail to return
  1248		 * within the specified time, but we'll still be able to bring up the
  1249		 * driver normally after that failure.  This occurs most commonly in
  1250		 * a cold boot situation (where then firmware takes longer to come up
  1251		 * than from a warm boot) and the reset_delay_ms in the platform data
  1252		 * has been set too short to accommodate that.  Since the sensor will
  1253		 * eventually come up and be usable, we don't want to just fail here
  1254		 * and leave the customer's device unusable.  So we warn them, and
  1255		 * continue processing.
  1256		 */
  1257		retval = rmi_scan_pdt(rmi_dev, NULL, rmi_initial_reset);
  1258		if (retval < 0)
  1259			dev_warn(dev, "RMI initial reset failed! Continuing in spite of this.\n");
  1260	
  1261		retval = rmi_read(rmi_dev, PDT_PROPERTIES_LOCATION, &data->pdt_props);
  1262		if (retval < 0) {
  1263			/*
  1264			 * we'll print out a warning and continue since
  1265			 * failure to get the PDT properties is not a cause to fail
  1266			 */
  1267			dev_warn(dev, "Could not read PDT properties from %#06x (code %d). Assuming 0x00.\n",
  1268				 PDT_PROPERTIES_LOCATION, retval);
  1269		}
  1270	
  1271		mutex_init(&data->irq_mutex);
  1272		mutex_init(&data->enabled_mutex);
  1273	
  1274		retval = rmi_probe_interrupts(data);
  1275		if (retval)
  1276			goto err;
  1277	
  1278		if (rmi_dev->xport->input) {
  1279			/*
  1280			 * The transport driver already has an input device.
  1281			 * In some cases it is preferable to reuse the transport
  1282			 * devices input device instead of creating a new one here.
  1283			 * One example is some HID touchpads report "pass-through"
  1284			 * button events are not reported by rmi registers.
  1285			 */
  1286			data->input = rmi_dev->xport->input;
  1287		} else {
  1288			data->input = devm_input_allocate_device(dev);
  1289			if (!data->input) {
  1290				dev_err(dev, "%s: Failed to allocate input device.\n",
  1291					__func__);
  1292				retval = -ENOMEM;
  1293				goto err;
  1294			}
  1295			rmi_driver_set_input_params(rmi_dev, data->input);
  1296			data->input->phys = devm_kasprintf(dev, GFP_KERNEL,
  1297							"%s/input0", dev_name(dev));
  1298		}
  1299	
  1300		retval = rmi_init_functions(data);
  1301		if (retval)
  1302			goto err;
  1303	
  1304		retval = rmi_f34_create_sysfs(rmi_dev);
  1305		if (retval)
  1306			goto err;
  1307	
  1308		if (data->input) {
  1309			rmi_driver_set_input_name(rmi_dev, data->input);
  1310			if (!rmi_dev->xport->input) {
  1311				retval = input_register_device(data->input);
  1312				if (retval) {
  1313					dev_err(dev, "%s: Failed to register input device.\n",
  1314						__func__);
  1315					goto err_destroy_functions;
  1316				}
  1317			}
  1318		}
  1319	
  1320		retval = rmi_irq_init(rmi_dev);
  1321		if (retval < 0)
  1322			goto err_destroy_functions;
  1323	
  1324		if (data->f01_container->dev.driver) {
  1325			/* Driver already bound, so enable ATTN now. */
  1326			retval = rmi_enable_sensor(rmi_dev);
  1327			if (retval)
  1328				goto err_disable_irq;
  1329		}
  1330	
  1331		return 0;
  1332	
  1333	err_disable_irq:
  1334		rmi_disable_irq(rmi_dev, false);
  1335	err_destroy_functions:
  1336		rmi_free_function_list(rmi_dev);
  1337	err:
  1338		return retval;
  1339	}
  1340	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* [PATCH v4 3/4] mfd: cs40l50: Add support for CS40L50 core driver
From: James Ogletree @ 2023-10-18 17:57 UTC (permalink / raw)
  Cc: James Ogletree, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Lee Jones, Fred Treven, Ben Bright, linux-input, devicetree,
	linux-kernel
In-Reply-To: <20231018175726.3879955-1-james.ogletree@opensource.cirrus.com>

From: James Ogletree <james.ogletree@cirrus.com>

Introduce support for Cirrus Logic Device CS40L50: a
haptic driver with waveform memory, integrated DSP,
and closed-loop algorithms.

The MFD component registers and initializes the device.

Signed-off-by: James Ogletree <james.ogletree@cirrus.com>
---
 MAINTAINERS                 |   2 +
 drivers/mfd/Kconfig         |  30 +++
 drivers/mfd/Makefile        |   4 +
 drivers/mfd/cs40l50-core.c  | 443 ++++++++++++++++++++++++++++++++++++
 drivers/mfd/cs40l50-i2c.c   |  69 ++++++
 drivers/mfd/cs40l50-spi.c   |  68 ++++++
 include/linux/mfd/cs40l50.h | 198 ++++++++++++++++
 7 files changed, 814 insertions(+)
 create mode 100644 drivers/mfd/cs40l50-core.c
 create mode 100644 drivers/mfd/cs40l50-i2c.c
 create mode 100644 drivers/mfd/cs40l50-spi.c
 create mode 100644 include/linux/mfd/cs40l50.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 57daf77bf550..08e1e9695d49 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4971,7 +4971,9 @@ L:	patches@opensource.cirrus.com
 S:	Supported
 F:	Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
 F:	drivers/input/misc/cirrus*
+F:	drivers/mfd/cs40l*
 F:	include/linux/input/cirrus*
+F:	include/linux/mfd/cs40l*
 
 CIRRUS LOGIC DSP FIRMWARE DRIVER
 M:	Simon Trimmer <simont@opensource.cirrus.com>
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 8b93856de432..a133d04a7859 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2187,6 +2187,36 @@ config MCP_UCB1200_TS
 
 endmenu
 
+config MFD_CS40L50_CORE
+	tristate
+	select MFD_CORE
+	select CS_DSP
+	select REGMAP_IRQ
+
+config MFD_CS40L50_I2C
+	tristate "Cirrus Logic CS40L50 (I2C)"
+	select REGMAP_I2C
+	select MFD_CS40L50_CORE
+	depends on I2C
+	help
+	  Select this to support the Cirrus Logic CS40L50 Haptic
+	  Driver over I2C.
+
+	  This driver can be built as a module. If built as a module it will be
+	  called "cs40l50-i2c".
+
+config MFD_CS40L50_SPI
+	tristate "Cirrus Logic CS40L50 (SPI)"
+	select REGMAP_SPI
+	select MFD_CS40L50_CORE
+	depends on SPI
+	help
+	  Select this to support the Cirrus Logic CS40L50 Haptic
+	  Driver over SPI.
+
+	  This driver can be built as a module. If built as a module it will be
+	  called "cs40l50-spi".
+
 config MFD_VEXPRESS_SYSREG
 	tristate "Versatile Express System Registers"
 	depends on VEXPRESS_CONFIG && GPIOLIB
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 7ed3ef4a698c..3b1a43b3acaf 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -95,6 +95,10 @@ obj-$(CONFIG_MFD_MADERA)	+= madera.o
 obj-$(CONFIG_MFD_MADERA_I2C)	+= madera-i2c.o
 obj-$(CONFIG_MFD_MADERA_SPI)	+= madera-spi.o
 
+obj-$(CONFIG_MFD_CS40L50_CORE)	+= cs40l50-core.o
+obj-$(CONFIG_MFD_CS40L50_I2C)	+= cs40l50-i2c.o
+obj-$(CONFIG_MFD_CS40L50_SPI)	+= cs40l50-spi.o
+
 obj-$(CONFIG_TPS6105X)		+= tps6105x.o
 obj-$(CONFIG_TPS65010)		+= tps65010.o
 obj-$(CONFIG_TPS6507X)		+= tps6507x.o
diff --git a/drivers/mfd/cs40l50-core.c b/drivers/mfd/cs40l50-core.c
new file mode 100644
index 000000000000..f1eadd80203a
--- /dev/null
+++ b/drivers/mfd/cs40l50-core.c
@@ -0,0 +1,443 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CS40L50 Advanced Haptic Driver with waveform memory,
+ * integrated DSP, and closed-loop algorithms
+ *
+ * Copyright 2023 Cirrus Logic, Inc.
+ *
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/cs40l50.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+
+static const struct mfd_cell cs40l50_devs[] = {
+	{
+		.name = "cs40l50-vibra",
+	},
+};
+
+const struct regmap_config cs40l50_regmap = {
+	.reg_bits =		32,
+	.reg_stride =		4,
+	.val_bits =		32,
+	.reg_format_endian =	REGMAP_ENDIAN_BIG,
+	.val_format_endian =	REGMAP_ENDIAN_BIG,
+};
+EXPORT_SYMBOL_GPL(cs40l50_regmap);
+
+static struct regulator_bulk_data cs40l50_supplies[] = {
+	{
+		.supply = "vp",
+	},
+	{
+		.supply = "vio",
+	},
+};
+
+static int cs40l50_handle_f0_est_done(struct cs40l50_private *cs40l50)
+{
+	u32 f_zero;
+	int error;
+
+	error = regmap_read(cs40l50->regmap, CS40L50_F0_ESTIMATION, &f_zero);
+	if (error)
+		return error;
+
+	return regmap_write(cs40l50->regmap, CS40L50_F0_STORED, f_zero);
+}
+
+static int cs40l50_handle_redc_est_done(struct cs40l50_private *cs40l50)
+{
+	int error, fractional, integer, stored;
+	u32 redc;
+
+	error = regmap_read(cs40l50->regmap, CS40L50_RE_EST_STATUS, &redc);
+	if (error)
+		return error;
+
+	error = regmap_write(cs40l50->regmap, CS40L50_REDC_ESTIMATION, redc);
+	if (error)
+		return error;
+
+	/* Convert from Q8.15 to (Q7.17 * 29/240) */
+	integer = ((redc >> 15) & 0xFF) << 17;
+	fractional = (redc & 0x7FFF) * 4;
+	stored = (integer | fractional) * 29 / 240;
+
+	return regmap_write(cs40l50->regmap, CS40L50_REDC_STORED, stored);
+}
+
+static int cs40l50_error_release(struct cs40l50_private *cs40l50)
+{
+	int error;
+
+	error = regmap_write(cs40l50->regmap, CS40L50_ERR_RLS,
+			     CS40L50_GLOBAL_ERR_RLS);
+	if (error)
+		return error;
+
+	return regmap_write(cs40l50->regmap, CS40L50_ERR_RLS, 0);
+}
+
+static int cs40l50_mailbox_read_next(struct cs40l50_private *cs40l50, u32 *val)
+{
+	u32 rd_ptr, wt_ptr;
+	int error;
+
+	error = regmap_read(cs40l50->regmap, CS40L50_MBOX_QUEUE_WT, &wt_ptr);
+	if (error)
+		return error;
+
+	error = regmap_read(cs40l50->regmap, CS40L50_MBOX_QUEUE_RD, &rd_ptr);
+	if (error)
+		return error;
+
+	if (wt_ptr == rd_ptr) {
+		*val = 0;
+		return 0;
+	}
+
+	error = regmap_read(cs40l50->regmap, rd_ptr, val);
+	if (error)
+		return error;
+
+	rd_ptr += sizeof(u32);
+	if (rd_ptr > CS40L50_MBOX_QUEUE_END)
+		rd_ptr = CS40L50_MBOX_QUEUE_BASE;
+
+	return regmap_write(cs40l50->regmap, CS40L50_MBOX_QUEUE_RD, rd_ptr);
+}
+
+static irqreturn_t cs40l50_process_mbox(int irq, void *data)
+{
+	struct cs40l50_private *cs40l50 = data;
+	int error = 0;
+	u32 val;
+
+	mutex_lock(&cs40l50->lock);
+
+	while (!cs40l50_mailbox_read_next(cs40l50, &val)) {
+		switch (val) {
+		case 0:
+			mutex_unlock(&cs40l50->lock);
+			dev_dbg(cs40l50->dev, "Reached end of queue\n");
+			return IRQ_HANDLED;
+		case CS40L50_MBOX_HAPTIC_TRIGGER_GPIO:
+			dev_dbg(cs40l50->dev, "Mailbox: TRIGGER_GPIO\n");
+			break;
+		case CS40L50_MBOX_HAPTIC_TRIGGER_MBOX:
+			dev_dbg(cs40l50->dev, "Mailbox: TRIGGER_MBOX\n");
+			break;
+		case CS40L50_MBOX_HAPTIC_TRIGGER_I2S:
+			dev_dbg(cs40l50->dev, "Mailbox: TRIGGER_I2S\n");
+			break;
+		case CS40L50_MBOX_HAPTIC_COMPLETE_MBOX:
+			dev_dbg(cs40l50->dev, "Mailbox: COMPLETE_MBOX\n");
+			break;
+		case CS40L50_MBOX_HAPTIC_COMPLETE_GPIO:
+			dev_dbg(cs40l50->dev, "Mailbox: COMPLETE_GPIO\n");
+			break;
+		case CS40L50_MBOX_HAPTIC_COMPLETE_I2S:
+			dev_dbg(cs40l50->dev, "Mailbox: COMPLETE_I2S\n");
+			break;
+		case CS40L50_MBOX_F0_EST_START:
+			dev_dbg(cs40l50->dev, "Mailbox: F0_EST_START\n");
+			break;
+		case CS40L50_MBOX_F0_EST_DONE:
+			dev_dbg(cs40l50->dev, "Mailbox: F0_EST_DONE\n");
+			error = cs40l50_handle_f0_est_done(cs40l50);
+			if (error)
+				goto out_mutex;
+			break;
+		case CS40L50_MBOX_REDC_EST_START:
+			dev_dbg(cs40l50->dev, "Mailbox: REDC_EST_START\n");
+			break;
+		case CS40L50_MBOX_REDC_EST_DONE:
+			dev_dbg(cs40l50->dev, "Mailbox: REDC_EST_DONE\n");
+			error = cs40l50_handle_redc_est_done(cs40l50);
+			if (error)
+				goto out_mutex;
+			break;
+		case CS40L50_MBOX_LE_EST_START:
+			dev_dbg(cs40l50->dev, "Mailbox: LE_EST_START\n");
+			break;
+		case CS40L50_MBOX_LE_EST_DONE:
+			dev_dbg(cs40l50->dev, "Mailbox: LE_EST_DONE\n");
+			break;
+		case CS40L50_MBOX_AWAKE:
+			dev_dbg(cs40l50->dev, "Mailbox: AWAKE\n");
+			break;
+		case CS40L50_MBOX_INIT:
+			dev_dbg(cs40l50->dev, "Mailbox: INIT\n");
+			break;
+		case CS40L50_MBOX_ACK:
+			dev_dbg(cs40l50->dev, "Mailbox: ACK\n");
+			break;
+		case CS40L50_MBOX_ERR_EVENT_UNMAPPED:
+			dev_err(cs40l50->dev, "Unmapped event\n");
+			break;
+		case CS40L50_MBOX_ERR_EVENT_MODIFY:
+			dev_err(cs40l50->dev, "Failed to modify event index\n");
+			break;
+		case CS40L50_MBOX_ERR_NULLPTR:
+			dev_err(cs40l50->dev, "Null pointer\n");
+			break;
+		case CS40L50_MBOX_ERR_BRAKING:
+			dev_err(cs40l50->dev, "Braking not in progress\n");
+			break;
+		case CS40L50_MBOX_ERR_INVAL_SRC:
+			dev_err(cs40l50->dev, "Suspend/resume invalid source\n");
+			break;
+		case CS40L50_MBOX_ERR_ENABLE_RANGE:
+			dev_err(cs40l50->dev, "GPIO enable out of range\n");
+			break;
+		case CS40L50_MBOX_ERR_GPIO_UNMAPPED:
+			dev_err(cs40l50->dev, "GPIO not mapped\n");
+			break;
+		case CS40L50_MBOX_ERR_ISR_RANGE:
+			dev_err(cs40l50->dev, "GPIO ISR out of range\n");
+			break;
+		case CS40L50_MBOX_PERMANENT_SHORT:
+			dev_crit(cs40l50->dev, "Permanent short detected\n");
+			break;
+		case CS40L50_MBOX_RUNTIME_SHORT:
+			dev_err(cs40l50->dev, "Runtime short detected\n");
+			error = cs40l50_error_release(cs40l50);
+			if (error)
+				goto out_mutex;
+			break;
+		default:
+			dev_err(cs40l50->dev, "Payload %#X not recognized\n", val);
+			error = -EINVAL;
+			goto out_mutex;
+		}
+	}
+
+	error = -EIO;
+
+out_mutex:
+	mutex_unlock(&cs40l50->lock);
+
+	return IRQ_RETVAL(!error);
+}
+
+static irqreturn_t cs40l50_error(int irq, void *data);
+
+static const struct cs40l50_irq cs40l50_irqs[] = {
+	CS40L50_IRQ(AMP_SHORT,		"Amp short",		error),
+	CS40L50_IRQ(VIRT2_MBOX,		"Mailbox",		process_mbox),
+	CS40L50_IRQ(TEMP_ERR,		"Overtemperature",	error),
+	CS40L50_IRQ(BST_UVP,		"Boost undervoltage",	error),
+	CS40L50_IRQ(BST_SHORT,		"Boost short",		error),
+	CS40L50_IRQ(BST_ILIMIT,		"Boost current limit",	error),
+	CS40L50_IRQ(UVLO_VDDBATT,	"Boost UVLO",		error),
+	CS40L50_IRQ(GLOBAL_ERROR,	"Global",		error),
+};
+
+static irqreturn_t cs40l50_error(int irq, void *data)
+{
+	struct cs40l50_private *cs40l50 = data;
+
+	dev_err(cs40l50->dev, "%s error\n", cs40l50_irqs[irq].name);
+
+	return IRQ_RETVAL(!cs40l50_error_release(cs40l50));
+}
+
+static const struct regmap_irq cs40l50_reg_irqs[] = {
+	CS40L50_REG_IRQ(IRQ1_INT_1,	AMP_SHORT),
+	CS40L50_REG_IRQ(IRQ1_INT_2,	VIRT2_MBOX),
+	CS40L50_REG_IRQ(IRQ1_INT_8,	TEMP_ERR),
+	CS40L50_REG_IRQ(IRQ1_INT_9,	BST_UVP),
+	CS40L50_REG_IRQ(IRQ1_INT_9,	BST_SHORT),
+	CS40L50_REG_IRQ(IRQ1_INT_9,	BST_ILIMIT),
+	CS40L50_REG_IRQ(IRQ1_INT_10,	UVLO_VDDBATT),
+	CS40L50_REG_IRQ(IRQ1_INT_18,	GLOBAL_ERROR),
+};
+
+static struct regmap_irq_chip cs40l50_irq_chip = {
+	.name =			"CS40L50 IRQ Controller",
+
+	.status_base =		CS40L50_IRQ1_INT_1,
+	.mask_base =		CS40L50_IRQ1_MASK_1,
+	.ack_base =		CS40L50_IRQ1_INT_1,
+	.num_regs =		22,
+
+	.irqs =			cs40l50_reg_irqs,
+	.num_irqs =		ARRAY_SIZE(cs40l50_reg_irqs),
+
+	.runtime_pm =		true,
+};
+
+static int cs40l50_irq_init(struct cs40l50_private *cs40l50)
+{
+	struct device *dev = cs40l50->dev;
+	int error, i, irq;
+
+	error = devm_regmap_add_irq_chip(dev, cs40l50->regmap, cs40l50->irq,
+					 IRQF_ONESHOT | IRQF_SHARED, 0,
+					 &cs40l50_irq_chip, &cs40l50->irq_data);
+	if (error)
+		return error;
+
+	for (i = 0; i < ARRAY_SIZE(cs40l50_irqs); i++) {
+		irq = regmap_irq_get_virq(cs40l50->irq_data, cs40l50_irqs[i].irq);
+		if (irq < 0) {
+			dev_err(dev, "Failed getting %s\n", cs40l50_irqs[i].name);
+			return irq;
+		}
+
+		error = devm_request_threaded_irq(dev, irq, NULL,
+						  cs40l50_irqs[i].handler,
+						  IRQF_ONESHOT | IRQF_SHARED,
+						  cs40l50_irqs[i].name, cs40l50);
+		if (error) {
+			dev_err(dev, "Failed requesting %s\n", cs40l50_irqs[i].name);
+			return error;
+		}
+	}
+
+	return 0;
+}
+
+static int cs40l50_part_num_resolve(struct cs40l50_private *cs40l50)
+{
+	struct device *dev = cs40l50->dev;
+	int error;
+
+	error = regmap_read(cs40l50->regmap, CS40L50_DEVID, &cs40l50->devid);
+	if (error)
+		return error;
+
+	if (cs40l50->devid != CS40L50_DEVID_A) {
+		dev_err(dev, "Invalid device ID: %#010X\n", cs40l50->devid);
+		return -EINVAL;
+	}
+
+	error = regmap_read(cs40l50->regmap, CS40L50_REVID, &cs40l50->revid);
+	if (error)
+		return error;
+
+	if (cs40l50->revid != CS40L50_REVID_B0) {
+		dev_err(dev, "Invalid revision: %#04X\n", cs40l50->revid);
+		return -EINVAL;
+	}
+
+	dev_info(dev, "Cirrus Logic CS40L50 revision %02X\n", cs40l50->revid);
+
+	return 0;
+}
+
+int cs40l50_probe(struct cs40l50_private *cs40l50)
+{
+	struct device *dev = cs40l50->dev;
+	int error;
+
+	mutex_init(&cs40l50->lock);
+
+	cs40l50->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(cs40l50->reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(cs40l50->reset_gpio),
+				     "Failed getting reset GPIO\n");
+
+	error = devm_regulator_bulk_get(dev, ARRAY_SIZE(cs40l50_supplies),
+					cs40l50_supplies);
+	if (error)
+		goto err_reset;
+
+	error = regulator_bulk_enable(ARRAY_SIZE(cs40l50_supplies),
+				      cs40l50_supplies);
+	if (error)
+		goto err_reset;
+
+	usleep_range(CS40L50_CP_READY_US, CS40L50_CP_READY_US + 100);
+
+	gpiod_set_value_cansleep(cs40l50->reset_gpio, 1);
+
+	usleep_range(CS40L50_CP_READY_US, CS40L50_CP_READY_US + 1000);
+
+	pm_runtime_set_autosuspend_delay(cs40l50->dev, CS40L50_AUTOSUSPEND_MS);
+	pm_runtime_use_autosuspend(cs40l50->dev);
+	pm_runtime_set_active(cs40l50->dev);
+	pm_runtime_get_noresume(cs40l50->dev);
+	devm_pm_runtime_enable(cs40l50->dev);
+
+	error = cs40l50_part_num_resolve(cs40l50);
+	if (error)
+		goto err_supplies;
+
+	error = cs40l50_irq_init(cs40l50);
+	if (error)
+		goto err_supplies;
+
+	error = devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, cs40l50_devs,
+				     ARRAY_SIZE(cs40l50_devs), NULL, 0, NULL);
+	if (error)
+		goto err_supplies;
+
+	pm_runtime_mark_last_busy(cs40l50->dev);
+	pm_runtime_put_autosuspend(cs40l50->dev);
+
+	return 0;
+
+err_supplies:
+	regulator_bulk_disable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies);
+err_reset:
+	gpiod_set_value_cansleep(cs40l50->reset_gpio, 0);
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(cs40l50_probe);
+
+int cs40l50_remove(struct cs40l50_private *cs40l50)
+{
+	regulator_bulk_disable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies);
+	gpiod_set_value_cansleep(cs40l50->reset_gpio, 1);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cs40l50_remove);
+
+static int cs40l50_runtime_suspend(struct device *dev)
+{
+	struct cs40l50_private *cs40l50 = dev_get_drvdata(dev);
+
+	return regmap_write(cs40l50->regmap, CS40L50_DSP_MBOX, CS40L50_ALLOW_HIBER);
+}
+
+static int cs40l50_runtime_resume(struct device *dev)
+{
+	struct cs40l50_private *cs40l50 = dev_get_drvdata(dev);
+	int error, i;
+	u32 val;
+
+	/* Device NAKs when exiting hibernation, so optionally retry here. */
+	for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) {
+		error = regmap_write(cs40l50->regmap, CS40L50_DSP_MBOX,
+				     CS40L50_PREVENT_HIBER);
+		if (!error)
+			break;
+
+		usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100);
+	}
+
+	for (; i < CS40L50_DSP_TIMEOUT_COUNT; i++) {
+		error = regmap_read(cs40l50->regmap, CS40L50_DSP_MBOX, &val);
+		if (!error && val == 0)
+			return 0;
+
+		usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100);
+	}
+
+	return error ? error : -ETIMEDOUT;
+}
+
+EXPORT_GPL_DEV_PM_OPS(cs40l50_pm_ops) = {
+	RUNTIME_PM_OPS(cs40l50_runtime_suspend, cs40l50_runtime_resume, NULL)
+};
+
+MODULE_DESCRIPTION("CS40L50 Advanced Haptic Driver");
+MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/cs40l50-i2c.c b/drivers/mfd/cs40l50-i2c.c
new file mode 100644
index 000000000000..be1b130eb94b
--- /dev/null
+++ b/drivers/mfd/cs40l50-i2c.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CS40L50 I2C Driver
+ *
+ * Copyright 2023 Cirrus Logic, Inc.
+ *
+ */
+
+#include <linux/i2c.h>
+#include <linux/mfd/cs40l50.h>
+
+static int cs40l50_i2c_probe(struct i2c_client *client,
+			     const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	struct cs40l50_private *cs40l50;
+
+	cs40l50 = devm_kzalloc(dev, sizeof(*cs40l50), GFP_KERNEL);
+	if (!cs40l50)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, cs40l50);
+
+	cs40l50->regmap = devm_regmap_init_i2c(client, &cs40l50_regmap);
+	if (IS_ERR(cs40l50->regmap))
+		return dev_err_probe(cs40l50->dev, PTR_ERR(cs40l50->regmap),
+				     "Failed to initialize register map\n");
+
+	cs40l50->dev = dev;
+	cs40l50->irq = client->irq;
+
+	return cs40l50_probe(cs40l50);
+}
+
+static void cs40l50_i2c_remove(struct i2c_client *client)
+{
+	struct cs40l50_private *cs40l50 = i2c_get_clientdata(client);
+
+	cs40l50_remove(cs40l50);
+}
+
+static const struct i2c_device_id cs40l50_id_i2c[] = {
+	{"cs40l50", 0},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, cs40l50_id_i2c);
+
+static const struct of_device_id cs40l50_of_match[] = {
+	{ .compatible = "cirrus,cs40l50" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, cs40l50_of_match);
+
+static struct i2c_driver cs40l50_i2c_driver = {
+	.driver = {
+		.name = "cs40l50",
+		.of_match_table = cs40l50_of_match,
+		.pm = pm_ptr(&cs40l50_pm_ops),
+	},
+	.id_table = cs40l50_id_i2c,
+	.probe = cs40l50_i2c_probe,
+	.remove = cs40l50_i2c_remove,
+};
+
+module_i2c_driver(cs40l50_i2c_driver);
+
+MODULE_DESCRIPTION("CS40L50 I2C Driver");
+MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/cs40l50-spi.c b/drivers/mfd/cs40l50-spi.c
new file mode 100644
index 000000000000..8311d48efedf
--- /dev/null
+++ b/drivers/mfd/cs40l50-spi.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CS40L50 SPI Driver
+ *
+ * Copyright 2023 Cirrus Logic, Inc.
+ *
+ */
+
+#include <linux/input/cs40l50.h>
+#include <linux/mfd/spi.h>
+
+static int cs40l50_spi_probe(struct spi_device *spi)
+{
+	struct cs40l50_private *cs40l50;
+	struct device *dev = &spi->dev;
+
+	cs40l50 = devm_kzalloc(dev, sizeof(*cs40l50), GFP_KERNEL);
+	if (!cs40l50)
+		return -ENOMEM;
+
+	spi_set_drvdata(spi, cs40l50);
+
+	cs40l50->regmap = devm_regmap_init_spi(spi, &cs40l50_regmap);
+	if (IS_ERR(cs40l50->regmap))
+		return dev_err_probe(cs40l50->dev, PTR_ERR(cs40l50->regmap),
+				     "Failed to initialize register map\n");
+
+	cs40l50->dev = dev;
+	cs40l50->irq = spi->irq;
+
+	return cs40l50_probe(cs40l50);
+}
+
+static void cs40l50_spi_remove(struct spi_device *spi)
+{
+	struct cs40l50_private *cs40l50 = spi_get_drvdata(spi);
+
+	cs40l50_remove(cs40l50);
+}
+
+static const struct spi_device_id cs40l50_id_spi[] = {
+	{"cs40l50", 0},
+	{}
+};
+MODULE_DEVICE_TABLE(spi, cs40l50_id_spi);
+
+static const struct of_device_id cs40l50_of_match[] = {
+	{ .compatible = "cirrus,cs40l50" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, cs40l50_of_match);
+
+static struct spi_driver cs40l50_spi_driver = {
+	.driver = {
+		.name = "cs40l50",
+		.of_match_table = cs40l50_of_match,
+		.pm = pm_ptr(&cs40l50_pm_ops),
+	},
+	.id_table = cs40l50_id_spi,
+	.probe = cs40l50_spi_probe,
+	.remove = cs40l50_spi_remove,
+};
+
+module_spi_driver(cs40l50_spi_driver);
+
+MODULE_DESCRIPTION("CS40L50 SPI Driver");
+MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/cs40l50.h b/include/linux/mfd/cs40l50.h
new file mode 100644
index 000000000000..c625a999a5ae
--- /dev/null
+++ b/include/linux/mfd/cs40l50.h
@@ -0,0 +1,198 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * CS40L50 Advanced Haptic Driver with waveform memory,
+ * integrated DSP, and closed-loop algorithms
+ *
+ * Copyright 2023 Cirrus Logic, Inc.
+ *
+ */
+
+#ifndef __CS40L50_H__
+#define __CS40L50_H__
+
+#include <linux/firmware/cirrus/cs_dsp.h>
+#include <linux/gpio/consumer.h>
+#include <linux/input.h>
+#include <linux/input/cirrus_haptics.h>
+#include <linux/interrupt.h>
+#include <linux/pm.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+/* Power Supply Configuration */
+#define CS40L50_BLOCK_ENABLES2			0x201C
+#define CS40L50_ERR_RLS				0x2034
+#define CS40L50_PWRMGT_CTL			0x2900
+#define CS40L50_BST_LPMODE_SEL			0x3810
+#define CS40L50_DCM_LOW_POWER		0x1
+#define CS40L50_OVERTEMP_WARN		0x4000010
+
+/* Interrupts */
+#define CS40L50_IRQ1_INT_1			0xE010
+#define CS40L50_IRQ1_INT_2			0xE014
+#define CS40L50_IRQ1_INT_8			0xE02C
+#define CS40L50_IRQ1_INT_9			0xE030
+#define CS40L50_IRQ1_INT_10			0xE034
+#define CS40L50_IRQ1_INT_18			0xE054
+#define CS40L50_IRQ1_MASK_1			0xE090
+#define CS40L50_IRQ1_MASK_2			0xE094
+#define CS40L50_IRQ1_MASK_20			0xE0DC
+#define CS40L50_IRQ_MASK_2_OVERRIDE	0xFFDF7FFF
+#define CS40L50_IRQ_MASK_20_OVERRIDE	0x15C01000
+#define CS40L50_AMP_SHORT_MASK		BIT(31)
+#define CS40L50_VIRT2_MBOX_MASK		BIT(21)
+#define CS40L50_TEMP_ERR_MASK		BIT(31)
+#define CS40L50_BST_UVP_MASK		BIT(6)
+#define CS40L50_BST_SHORT_MASK		BIT(7)
+#define CS40L50_BST_ILIMIT_MASK		BIT(8)
+#define CS40L50_UVLO_VDDBATT_MASK	BIT(16)
+#define CS40L50_GLOBAL_ERROR_MASK	BIT(15)
+#define CS40L50_GLOBAL_ERR_RLS		BIT(11)
+#define CS40L50_IRQ(_irq, _name, _hand)		\
+	{					\
+		.irq = CS40L50_ ## _irq ## _IRQ,\
+		.name = _name,			\
+		.handler = cs40l50_ ## _hand,	\
+	}
+#define CS40L50_REG_IRQ(_reg, _irq)					\
+	[CS40L50_ ## _irq ## _IRQ] = {					\
+		.reg_offset = (CS40L50_ ## _reg) - CS40L50_IRQ1_INT_1,	\
+		.mask = CS40L50_ ## _irq ## _MASK			\
+	}
+
+/* Mailbox Inbound Commands */
+#define CS40L50_RAM_BANK_INDEX_START	0x1000000
+#define CS40L50_RTH_INDEX_START		0x1400000
+#define CS40L50_RTH_INDEX_END		0x1400001
+#define CS40L50_ROM_BANK_INDEX_START	0x1800000
+#define CS40L50_ROM_BANK_INDEX_END	0x180001A
+#define CS40L50_PREVENT_HIBER		0x2000003
+#define CS40L50_ALLOW_HIBER		0x2000004
+#define CS40L50_OWT_PUSH		0x3000008
+#define CS40L50_STOP_PLAYBACK		0x5000000
+#define CS40L50_OWT_DELETE		0xD000000
+
+/* Mailbox Outbound Commands */
+#define CS40L50_MBOX_QUEUE_BASE				0x11004
+#define CS40L50_MBOX_QUEUE_END				0x1101C
+#define CS40L50_DSP_MBOX				0x11020
+#define CS40L50_MBOX_QUEUE_WT				0x28042C8
+#define CS40L50_MBOX_QUEUE_RD				0x28042CC
+#define CS40L50_MBOX_HAPTIC_COMPLETE_MBOX	0x1000000
+#define CS40L50_MBOX_HAPTIC_COMPLETE_GPIO	0x1000001
+#define CS40L50_MBOX_HAPTIC_COMPLETE_I2S	0x1000002
+#define CS40L50_MBOX_HAPTIC_TRIGGER_MBOX	0x1000010
+#define CS40L50_MBOX_HAPTIC_TRIGGER_GPIO	0x1000011
+#define CS40L50_MBOX_HAPTIC_TRIGGER_I2S		0x1000012
+#define CS40L50_MBOX_INIT			0x2000000
+#define CS40L50_MBOX_AWAKE			0x2000002
+#define CS40L50_MBOX_F0_EST_START		0x7000011
+#define CS40L50_MBOX_F0_EST_DONE		0x7000021
+#define CS40L50_MBOX_REDC_EST_START		0x7000012
+#define CS40L50_MBOX_REDC_EST_DONE		0x7000022
+#define CS40L50_MBOX_LE_EST_START		0x7000014
+#define CS40L50_MBOX_LE_EST_DONE		0x7000024
+#define CS40L50_MBOX_ACK			0xA000000
+#define CS40L50_MBOX_PANIC			0xC000000
+#define CS40L50_MBOX_WATERMARK			0xD000000
+#define CS40L50_MBOX_ERR_EVENT_UNMAPPED		0xC0004B3
+#define CS40L50_MBOX_ERR_EVENT_MODIFY		0xC0004B4
+#define CS40L50_MBOX_ERR_NULLPTR		0xC0006A5
+#define CS40L50_MBOX_ERR_BRAKING		0xC0006A8
+#define CS40L50_MBOX_ERR_INVAL_SRC		0xC0006AC
+#define CS40L50_MBOX_ERR_ENABLE_RANGE		0xC000836
+#define CS40L50_MBOX_ERR_GPIO_UNMAPPED		0xC000837
+#define CS40L50_MBOX_ERR_ISR_RANGE		0xC000838
+#define CS40L50_MBOX_PERMANENT_SHORT		0xC000C1C
+#define CS40L50_MBOX_RUNTIME_SHORT		0xC000C1D
+
+/* DSP */
+#define CS40L50_DSP1_XMEM_PACKED_0		0x2000000
+#define CS40L50_DSP1_XMEM_UNPACKED32_0		0x2400000
+#define CS40L50_SYS_INFO_ID			0x25E0000
+#define CS40L50_DSP1_XMEM_UNPACKED24_0		0x2800000
+#define CS40L50_RAM_INIT			0x28021DC
+#define CS40L50_POWER_ON_SEQ			0x2804320
+#define CS40L50_OWT_BASE			0x2805C34
+#define CS40L50_NUM_OF_WAVES			0x280CB4C
+#define CS40L50_CORE_BASE			0x2B80000
+#define CS40L50_CCM_CORE_CONTROL		0x2BC1000
+#define CS40L50_DSP1_YMEM_PACKED_0		0x2C00000
+#define CS40L50_DSP1_YMEM_UNPACKED32_0		0x3000000
+#define CS40L50_DSP1_YMEM_UNPACKED24_0		0x3400000
+#define CS40L50_DSP1_PMEM_0			0x3800000
+#define CS40L50_DSP1_PMEM_5114			0x3804FE8
+#define CS40L50_MEM_RDY_HW		0x2
+#define CS40L50_RAM_INIT_FLAG		0x1
+#define CS40L50_CLOCK_DISABLE		0x80
+#define CS40L50_CLOCK_ENABLE		0x281
+#define CS40L50_DSP_POLL_US		1000
+#define CS40L50_DSP_TIMEOUT_COUNT	100
+#define CS40L50_CP_READY_US		2200
+#define CS40L50_PSEQ_SIZE		200
+#define CS40L50_AUTOSUSPEND_MS		2000
+
+/* Firmware */
+#define CS40L50_FW			"cs40l50.wmfw"
+#define CS40L50_WT			"cs40l50.bin"
+
+/* Calibration */
+#define CS40L50_REDC_ESTIMATION		0x2802F7C
+#define CS40L50_F0_ESTIMATION		0x2802F84
+#define CS40L50_F0_STORED		0x2805C00
+#define CS40L50_REDC_STORED		0x2805C04
+#define CS40L50_RE_EST_STATUS		0x3401B40
+
+/* Open wavetable */
+#define CS40L50_OWT_SIZE		0x2805C38
+#define CS40L50_OWT_NEXT		0x2805C3C
+#define CS40L50_NUM_OF_OWT_WAVES	0x2805C40
+
+/* GPIO */
+#define CS40L50_GPIO_BASE		0x2804140
+
+/* Device */
+#define CS40L50_DEVID			0x0
+#define CS40L50_REVID			0x4
+#define CS40L50_DEVID_A		0x40A50
+#define CS40L50_REVID_B0	0xB0
+
+enum cs40l50_irq_list {
+	CS40L50_GLOBAL_ERROR_IRQ,
+	CS40L50_UVLO_VDDBATT_IRQ,
+	CS40L50_BST_ILIMIT_IRQ,
+	CS40L50_BST_SHORT_IRQ,
+	CS40L50_BST_UVP_IRQ,
+	CS40L50_TEMP_ERR_IRQ,
+	CS40L50_VIRT2_MBOX_IRQ,
+	CS40L50_AMP_SHORT_IRQ,
+};
+
+struct cs40l50_irq {
+	const char *name;
+	int irq;
+	irqreturn_t (*handler)(int irq, void *data);
+};
+
+struct cs40l50_private {
+	struct device *dev;
+	struct regmap *regmap;
+	struct cs_dsp dsp;
+	struct mutex lock;
+	struct gpio_desc *reset_gpio;
+	struct regmap_irq_chip_data *irq_data;
+	struct input_dev *input;
+	const struct firmware *wmfw;
+	struct cs_hap haptics;
+	u32 devid;
+	u32 revid;
+	int irq;
+};
+
+int cs40l50_probe(struct cs40l50_private *cs40l50);
+int cs40l50_remove(struct cs40l50_private *cs40l50);
+
+extern const struct regmap_config cs40l50_regmap;
+extern const struct dev_pm_ops cs40l50_pm_ops;
+
+#endif /* __CS40L50_H__ */
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 2/4] Input: cs40l50 - Add cirrus haptics base support
From: James Ogletree @ 2023-10-18 17:57 UTC (permalink / raw)
  Cc: James Ogletree, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Lee Jones, Fred Treven, Ben Bright, linux-input, devicetree,
	linux-kernel
In-Reply-To: <20231018175726.3879955-1-james.ogletree@opensource.cirrus.com>

From: James Ogletree <james.ogletree@cirrus.com>

Introduce the cirrus haptics library which factors out
common haptics operations used by Cirrus Logic Input
drivers.

Signed-off-by: James Ogletree <james.ogletree@cirrus.com>
---
 MAINTAINERS                          |   2 +
 drivers/input/misc/cirrus_haptics.c  | 586 +++++++++++++++++++++++++++
 include/linux/input/cirrus_haptics.h | 121 ++++++
 3 files changed, 709 insertions(+)
 create mode 100644 drivers/input/misc/cirrus_haptics.c
 create mode 100644 include/linux/input/cirrus_haptics.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 28f0ca9324b3..57daf77bf550 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4970,6 +4970,8 @@ M:	Ben Bright <ben.bright@cirrus.com>
 L:	patches@opensource.cirrus.com
 S:	Supported
 F:	Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
+F:	drivers/input/misc/cirrus*
+F:	include/linux/input/cirrus*
 
 CIRRUS LOGIC DSP FIRMWARE DRIVER
 M:	Simon Trimmer <simont@opensource.cirrus.com>
diff --git a/drivers/input/misc/cirrus_haptics.c b/drivers/input/misc/cirrus_haptics.c
new file mode 100644
index 000000000000..7e539cd45167
--- /dev/null
+++ b/drivers/input/misc/cirrus_haptics.c
@@ -0,0 +1,586 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Helper functions for dealing with wavetable
+ * formats and DSP interfaces used by Cirrus
+ * haptic drivers.
+ *
+ * Copyright 2023 Cirrus Logic, Inc.
+ */
+
+#include <linux/firmware/cirrus/cs_dsp.h>
+#include <linux/input.h>
+#include <linux/input/cirrus_haptics.h>
+#include <linux/pm_runtime.h>
+
+static int cs_hap_pseq_init(struct cs_hap *haptics)
+{
+	struct cs_hap_pseq_op *op;
+	int error, i, num_words;
+	u8 operation;
+	u32 *words;
+
+	if (!haptics->dsp.pseq_size || !haptics->dsp.pseq_reg)
+		return 0;
+
+	INIT_LIST_HEAD(&haptics->pseq_head);
+
+	words = kcalloc(haptics->dsp.pseq_size, sizeof(u32), GFP_KERNEL);
+	if (!words)
+		return -ENOMEM;
+
+	error = regmap_bulk_read(haptics->regmap, haptics->dsp.pseq_reg,
+				 words, haptics->dsp.pseq_size);
+	if (error)
+		goto err_free;
+
+	for (i = 0; i < haptics->dsp.pseq_size; i += num_words) {
+		operation = FIELD_GET(PSEQ_OP_MASK, words[i]);
+		switch (operation) {
+		case PSEQ_OP_END:
+		case PSEQ_OP_WRITE_UNLOCK:
+			num_words = PSEQ_OP_END_WORDS;
+			break;
+		case PSEQ_OP_WRITE_ADDR8:
+		case PSEQ_OP_WRITE_H16:
+		case PSEQ_OP_WRITE_L16:
+			num_words = PSEQ_OP_WRITE_X16_WORDS;
+			break;
+		case PSEQ_OP_WRITE_FULL:
+			num_words = PSEQ_OP_WRITE_FULL_WORDS;
+			break;
+		default:
+			error = -EINVAL;
+			dev_err(haptics->dev, "Unsupported op: %u\n", operation);
+			goto err_free;
+		}
+
+		op = devm_kzalloc(haptics->dev, sizeof(*op), GFP_KERNEL);
+		if (!op) {
+			error = -ENOMEM;
+			goto err_free;
+		}
+
+		op->size = num_words * sizeof(u32);
+		memcpy(op->words, &words[i], op->size);
+		op->offset = i * sizeof(u32);
+		op->operation = operation;
+		list_add(&op->list, &haptics->pseq_head);
+
+		if (operation == PSEQ_OP_END)
+			break;
+	}
+
+	if (operation != PSEQ_OP_END)
+		error = -ENOENT;
+
+err_free:
+	kfree(words);
+
+	return error;
+}
+
+static int cs_hap_pseq_find_end(struct cs_hap *haptics,
+				struct cs_hap_pseq_op **op_end)
+{
+	u8 operation = PSEQ_OP_WRITE_FULL;
+	struct cs_hap_pseq_op *op;
+
+	list_for_each_entry(op, &haptics->pseq_head, list) {
+		operation = op->operation;
+		if (operation == PSEQ_OP_END)
+			break;
+	}
+
+	if (operation != PSEQ_OP_END) {
+		dev_err(haptics->dev, "Missing PSEQ list terminator\n");
+		return -ENOENT;
+	}
+
+	*op_end = op;
+
+	return 0;
+}
+
+static struct cs_hap_pseq_op *cs_hap_pseq_find_op(struct cs_hap_pseq_op *match_op,
+						  struct list_head *pseq_head)
+{
+	struct cs_hap_pseq_op *op;
+
+	list_for_each_entry(op, pseq_head, list) {
+		if (op->operation == PSEQ_OP_END)
+			break;
+		if (op->operation != match_op->operation ||
+		    op->words[0] != match_op->words[0])
+			continue;
+		switch (op->operation) {
+		case PSEQ_OP_WRITE_FULL:
+			if (FIELD_GET(GENMASK(23, 8), op->words[1]) ==
+			    FIELD_GET(GENMASK(23, 8), match_op->words[1]))
+				return op;
+			break;
+		case PSEQ_OP_WRITE_H16:
+		case PSEQ_OP_WRITE_L16:
+			if (FIELD_GET(GENMASK(23, 16), op->words[1]) ==
+			    FIELD_GET(GENMASK(23, 16), match_op->words[1]))
+				return op;
+			break;
+		default:
+			break;
+		}
+	}
+
+	return NULL;
+}
+
+int cs_hap_pseq_write(struct cs_hap *haptics, u32 addr,
+		      u32 data, bool update, u8 op_code)
+{
+	struct cs_hap_pseq_op *op, *op_end, *op_new;
+	struct cs_dsp_chunk ch;
+	u32 pseq_bytes;
+	int error;
+
+	op_new = devm_kzalloc(haptics->dev, sizeof(*op_new), GFP_KERNEL);
+	if (!op_new)
+		return -ENOMEM;
+
+	op_new->operation = op_code;
+
+	ch = cs_dsp_chunk((void *) op_new->words,
+			  PSEQ_OP_WRITE_FULL_WORDS * sizeof(u32));
+	cs_dsp_chunk_write(&ch, 8, op_code);
+	switch (op_code) {
+	case PSEQ_OP_WRITE_FULL:
+		cs_dsp_chunk_write(&ch, 32, addr);
+		cs_dsp_chunk_write(&ch, 32, data);
+		break;
+	case PSEQ_OP_WRITE_L16:
+	case PSEQ_OP_WRITE_H16:
+		cs_dsp_chunk_write(&ch, 24, addr);
+		cs_dsp_chunk_write(&ch, 16, data);
+		break;
+	default:
+		error = -EINVAL;
+		goto op_new_free;
+	}
+
+	op_new->size = cs_dsp_chunk_bytes(&ch);
+
+	if (update) {
+		op = cs_hap_pseq_find_op(op_new, &haptics->pseq_head);
+		if (!op) {
+			error = -EINVAL;
+			goto op_new_free;
+		}
+	}
+
+	error = cs_hap_pseq_find_end(haptics, &op_end);
+	if (error)
+		goto op_new_free;
+
+	pseq_bytes = haptics->dsp.pseq_size * sizeof(u32);
+
+	if (pseq_bytes - op_end->offset < op_new->size) {
+		error = -ENOMEM;
+		goto op_new_free;
+	}
+
+	if (update) {
+		op_new->offset = op->offset;
+	} else {
+		op_new->offset = op_end->offset;
+		op_end->offset += op_new->size;
+	}
+
+	error = regmap_raw_write(haptics->regmap, haptics->dsp.pseq_reg +
+				 op_new->offset, op_new->words, op_new->size);
+	if (error)
+		goto op_new_free;
+
+	if (update) {
+		list_replace(&op->list, &op_new->list);
+	} else {
+		error = regmap_raw_write(haptics->regmap, haptics->dsp.pseq_reg +
+					 op_end->offset, op_end->words,
+					 op_end->size);
+		if (error)
+			goto op_new_free;
+
+		list_add(&op_new->list, &haptics->pseq_head);
+	}
+
+	return 0;
+
+op_new_free:
+	devm_kfree(haptics->dev, op_new);
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(cs_hap_pseq_write);
+
+int cs_hap_pseq_multi_write(struct cs_hap *haptics,
+			    const struct reg_sequence *reg_seq,
+			    int num_regs, bool update, u8 op_code)
+{
+	int error, i;
+
+	for (i = 0; i < num_regs; i++) {
+		error = cs_hap_pseq_write(haptics, reg_seq[i].reg,
+					  reg_seq[i].def, update, op_code);
+		if (error)
+			return error;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cs_hap_pseq_multi_write);
+
+static struct cs_hap_effect *cs_hap_find_effect(int id,
+						struct list_head *effect_head)
+{
+	struct cs_hap_effect *effect;
+
+	list_for_each_entry(effect, effect_head, list)
+		if (effect->id == id)
+			return effect;
+
+	return NULL;
+}
+
+static int cs_hap_effect_bank_set(struct cs_hap *haptics,
+				  struct cs_hap_effect *effect,
+				  struct ff_periodic_effect add_effect)
+{
+	s16 bank = add_effect.custom_data[0] & 0xffffu;
+	unsigned int len = add_effect.custom_len;
+
+	if (bank >= WVFRM_BANK_NUM) {
+		dev_err(haptics->dev, "Invalid waveform bank: %d\n", bank);
+		return -EINVAL;
+	}
+
+	effect->bank = len > CUSTOM_DATA_SIZE ? WVFRM_BANK_OWT : bank;
+
+	return 0;
+}
+
+static int cs_hap_effect_mapping_set(struct cs_hap *haptics, u16 button,
+				     struct cs_hap_effect *effect)
+{
+	u32 gpio_num, gpio_edge;
+
+	if (button) {
+		gpio_num = FIELD_GET(BTN_NUM_MASK, button);
+		gpio_edge = FIELD_GET(BTN_EDGE_MASK, button);
+		effect->mapping = haptics->dsp.gpio_base_reg +
+				  (gpio_num * 8) - gpio_edge;
+
+		return regmap_write(haptics->regmap, effect->mapping, button);
+	}
+
+	effect->mapping = GPIO_MAPPING_INVALID;
+
+	return 0;
+}
+
+static int cs_hap_effect_index_set(struct cs_hap *haptics,
+				   struct cs_hap_effect *effect,
+				   struct ff_periodic_effect add_effect)
+{
+	struct cs_hap_effect *owt_effect;
+	u32 base_index, max_index;
+
+	base_index = haptics->banks[effect->bank].base_index;
+	max_index = haptics->banks[effect->bank].max_index;
+
+	effect->index = base_index;
+
+	switch (effect->bank) {
+	case WVFRM_BANK_OWT:
+		list_for_each_entry(owt_effect, &haptics->effect_head, list)
+			if (owt_effect->bank == WVFRM_BANK_OWT)
+				effect->index++;
+		break;
+	case WVFRM_BANK_ROM:
+	case WVFRM_BANK_RAM:
+		effect->index += add_effect.custom_data[1] & 0xffffu;
+		break;
+	default:
+		dev_err(haptics->dev, "Bank not supported: %d\n", effect->bank);
+		return -EINVAL;
+	}
+
+	if (effect->index > max_index || effect->index < base_index) {
+		dev_err(haptics->dev, "Index out of bounds: %u\n", effect->index);
+		return -ENOSPC;
+	}
+
+	return 0;
+}
+
+static int cs_hap_upload_pwle(struct cs_hap *haptics,
+			      struct cs_hap_effect *effect,
+			      struct ff_periodic_effect add_effect)
+{
+	u32 len, wt_offset, wt_size_words;
+	struct cs_hap_pwle_header header;
+	u8 *out_data;
+	int error;
+
+	error = regmap_read(haptics->regmap, haptics->dsp.owt_offset_reg,
+			    &wt_offset);
+	if (error)
+		return error;
+
+	error = regmap_read(haptics->regmap, haptics->dsp.owt_size_reg,
+			    &wt_size_words);
+	if (error)
+		return error;
+
+	len = 2 * add_effect.custom_len;
+
+	if ((wt_size_words * sizeof(u32)) < OWT_HEADER_SIZE + len)
+		return -ENOSPC;
+
+	out_data = kzalloc(OWT_HEADER_SIZE + len, GFP_KERNEL);
+	if (!out_data)
+		return -ENOMEM;
+
+	header.type = add_effect.custom_data[0] == PCM_ID ? OWT_TYPE_PCM :
+							    OWT_TYPE_PWLE;
+	header.offset = OWT_HEADER_SIZE / sizeof(u32);
+	header.data_words = len / sizeof(u32);
+
+	memcpy(out_data, &header, sizeof(header));
+	memcpy(out_data + OWT_HEADER_SIZE, add_effect.custom_data, len);
+
+	error = regmap_bulk_write(haptics->regmap, haptics->dsp.owt_base_reg +
+				  (wt_offset * sizeof(u32)), out_data,
+				  OWT_HEADER_SIZE + len);
+	if (error)
+		goto err_free;
+
+	error = regmap_write(haptics->regmap, haptics->dsp.mailbox_reg,
+			     haptics->dsp.push_owt_cmd);
+
+err_free:
+	kfree(out_data);
+
+	return error;
+}
+
+static void cs_hap_add_worker(struct work_struct *work)
+{
+	struct cs_hap *haptics = container_of(work, struct cs_hap,
+					      add_work);
+	struct ff_effect add_effect = haptics->add_effect;
+	bool is_new = false;
+	struct cs_hap_effect *effect;
+	int error;
+
+	if (haptics->runtime_pm) {
+		error = pm_runtime_resume_and_get(haptics->dev);
+		if (error < 0) {
+			haptics->add_error = error;
+			return;
+		}
+	}
+
+	mutex_lock(&haptics->lock);
+
+	effect = cs_hap_find_effect(add_effect.id, &haptics->effect_head);
+	if (!effect) {
+		effect = kzalloc(sizeof(*effect), GFP_KERNEL);
+		if (!effect) {
+			error = -ENOMEM;
+			goto err_mutex;
+		}
+		effect->id = add_effect.id;
+		is_new = true;
+	}
+
+	error = cs_hap_effect_bank_set(haptics, effect, add_effect.u.periodic);
+	if (error)
+		goto err_free;
+
+	error = cs_hap_effect_index_set(haptics, effect, add_effect.u.periodic);
+	if (error)
+		goto err_free;
+
+	error = cs_hap_effect_mapping_set(haptics, add_effect.trigger.button,
+					  effect);
+	if (error)
+		goto err_free;
+
+	if (effect->bank == WVFRM_BANK_OWT)
+		error = cs_hap_upload_pwle(haptics, effect,
+					   add_effect.u.periodic);
+
+err_free:
+	if (is_new) {
+		if (error)
+			kfree(effect);
+		else
+			list_add(&effect->list, &haptics->effect_head);
+	}
+
+err_mutex:
+	mutex_unlock(&haptics->lock);
+
+	if (haptics->runtime_pm) {
+		pm_runtime_mark_last_busy(haptics->dev);
+		pm_runtime_put_autosuspend(haptics->dev);
+	}
+
+	haptics->add_error = error;
+}
+
+static void cs_hap_erase_worker(struct work_struct *work)
+{
+	struct cs_hap *haptics = container_of(work, struct cs_hap,
+					      erase_work);
+	int error = 0;
+	struct cs_hap_effect *owt_effect, *erase_effect;
+
+	if (haptics->runtime_pm) {
+		error = pm_runtime_resume_and_get(haptics->dev);
+		if (error < 0) {
+			haptics->erase_error = error;
+			return;
+		}
+	}
+
+	mutex_lock(&haptics->lock);
+
+	erase_effect = cs_hap_find_effect(haptics->erase_effect->id,
+					  &haptics->effect_head);
+	if (!erase_effect) {
+		dev_err(haptics->dev, "Effect to erase does not exist\n");
+		error = -EINVAL;
+		goto err_mutex;
+	}
+
+	if (erase_effect->mapping != GPIO_MAPPING_INVALID) {
+		error = regmap_write(haptics->regmap, erase_effect->mapping,
+				     GPIO_DISABLE);
+		if (error)
+			goto err_mutex;
+	}
+
+	if (erase_effect->bank == WVFRM_BANK_OWT) {
+		error = regmap_write(haptics->regmap, haptics->dsp.mailbox_reg,
+				     haptics->dsp.delete_owt_cmd |
+				     erase_effect->index);
+		if (error)
+			goto err_mutex;
+
+		list_for_each_entry(owt_effect, &haptics->effect_head, list)
+			if (owt_effect->bank == WVFRM_BANK_OWT &&
+			    owt_effect->index > erase_effect->index)
+				owt_effect->index--;
+	}
+
+	list_del(&erase_effect->list);
+	kfree(erase_effect);
+
+err_mutex:
+	mutex_unlock(&haptics->lock);
+
+	if (haptics->runtime_pm) {
+		pm_runtime_mark_last_busy(haptics->dev);
+		pm_runtime_put_autosuspend(haptics->dev);
+	}
+
+	haptics->erase_error = error;
+}
+
+static void cs_hap_vibe_start_worker(struct work_struct *work)
+{
+	struct cs_hap *haptics = container_of(work, struct cs_hap,
+					      vibe_start_work);
+	struct cs_hap_effect *effect;
+	int error;
+
+	if (haptics->runtime_pm) {
+		error = pm_runtime_resume_and_get(haptics->dev);
+		if (error < 0) {
+			haptics->start_error = error;
+			return;
+		}
+	}
+
+	mutex_lock(&haptics->lock);
+
+	effect = cs_hap_find_effect(haptics->start_effect->id,
+				    &haptics->effect_head);
+	if (effect) {
+		error = regmap_write(haptics->regmap, haptics->dsp.mailbox_reg,
+				     effect->index);
+	} else {
+		dev_err(haptics->dev, "Effect to start does not exist\n");
+		error = -EINVAL;
+	}
+
+	mutex_unlock(&haptics->lock);
+
+	if (haptics->runtime_pm) {
+		pm_runtime_mark_last_busy(haptics->dev);
+		pm_runtime_put_autosuspend(haptics->dev);
+	}
+
+	haptics->start_error = error;
+}
+
+static void cs_hap_vibe_stop_worker(struct work_struct *work)
+{
+	struct cs_hap *haptics = container_of(work, struct cs_hap,
+					      vibe_stop_work);
+	int error;
+
+	if (haptics->runtime_pm) {
+		error = pm_runtime_resume_and_get(haptics->dev);
+		if (error < 0) {
+			haptics->stop_error = error;
+			return;
+		}
+	}
+
+	mutex_lock(&haptics->lock);
+	error = regmap_write(haptics->regmap, haptics->dsp.mailbox_reg,
+			     haptics->dsp.stop_cmd);
+	mutex_unlock(&haptics->lock);
+
+	if (haptics->runtime_pm) {
+		pm_runtime_mark_last_busy(haptics->dev);
+		pm_runtime_put_autosuspend(haptics->dev);
+	}
+
+	haptics->stop_error = error;
+}
+
+int cs_hap_init(struct cs_hap *haptics)
+{
+	haptics->vibe_wq = alloc_ordered_workqueue("vibe_wq", 0);
+	if (!haptics->vibe_wq)
+		return -ENOMEM;
+
+	mutex_init(&haptics->lock);
+
+	INIT_WORK(&haptics->vibe_start_work, cs_hap_vibe_start_worker);
+	INIT_WORK(&haptics->vibe_stop_work, cs_hap_vibe_stop_worker);
+	INIT_WORK(&haptics->erase_work, cs_hap_erase_worker);
+	INIT_WORK(&haptics->add_work, cs_hap_add_worker);
+
+	return cs_hap_pseq_init(haptics);
+}
+EXPORT_SYMBOL_GPL(cs_hap_init);
+
+void cs_hap_remove(struct cs_hap *haptics)
+{
+	flush_workqueue(haptics->vibe_wq);
+	destroy_workqueue(haptics->vibe_wq);
+}
+EXPORT_SYMBOL_GPL(cs_hap_remove);
+
+MODULE_DESCRIPTION("Cirrus Logic Haptics Support");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/input/cirrus_haptics.h b/include/linux/input/cirrus_haptics.h
new file mode 100644
index 000000000000..42f6afed7944
--- /dev/null
+++ b/include/linux/input/cirrus_haptics.h
@@ -0,0 +1,121 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Helper functions for dealing with wavetable
+ * formats and DSP interfaces used by Cirrus
+ * haptic drivers.
+ *
+ * Copyright 2023 Cirrus Logic, Inc.
+ */
+
+#ifndef __CIRRUS_HAPTICS_H
+#define __CIRRUS_HAPTICS_H
+
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+/* Power-on write sequencer */
+#define PSEQ_OP_MASK			GENMASK(23, 16)
+#define PSEQ_OP_SHIFT			16
+#define PSEQ_OP_WRITE_FULL_WORDS	3
+#define PSEQ_OP_WRITE_X16_WORDS		2
+#define PSEQ_OP_END_WORDS		1
+#define PSEQ_OP_WRITE_FULL		0x00
+#define PSEQ_OP_WRITE_ADDR8		0x02
+#define PSEQ_OP_WRITE_L16		0x04
+#define PSEQ_OP_WRITE_H16		0x05
+#define PSEQ_OP_WRITE_UNLOCK		0xFD
+#define PSEQ_OP_END			0xFF
+
+/* Open wavetable */
+#define OWT_HEADER_SIZE		12
+#define OWT_TYPE_PCM		8
+#define OWT_TYPE_PWLE		12
+#define PCM_ID			0x0
+#define CUSTOM_DATA_SIZE	2
+
+/* GPIO */
+#define BTN_NUM_MASK		GENMASK(14, 12)
+#define BTN_EDGE_MASK		BIT(15)
+#define GPIO_MAPPING_INVALID	0
+#define GPIO_DISABLE		0x1FF
+
+enum cs_hap_bank_type {
+	WVFRM_BANK_RAM,
+	WVFRM_BANK_ROM,
+	WVFRM_BANK_OWT,
+	WVFRM_BANK_NUM,
+};
+
+struct cs_hap_pseq_op {
+	struct list_head list;
+	u32 words[3];
+	u16 offset;
+	u8 operation;
+	u8 size;
+};
+
+struct cs_hap_effect {
+	enum cs_hap_bank_type bank;
+	struct list_head list;
+	u32 mapping;
+	u32 index;
+	int id;
+};
+
+struct cs_hap_pwle_header {
+	u32 type;
+	u32 data_words;
+	u32 offset;
+} __packed;
+
+struct cs_hap_bank {
+	enum cs_hap_bank_type bank;
+	u32 base_index;
+	u32 max_index;
+};
+
+struct cs_hap_dsp {
+	u32 gpio_base_reg;
+	u32 owt_offset_reg;
+	u32 owt_size_reg;
+	u32 owt_base_reg;
+	u32 mailbox_reg;
+	u32 pseq_reg;
+	u32 push_owt_cmd;
+	u32 delete_owt_cmd;
+	u32 stop_cmd;
+	u32 pseq_size;
+};
+
+struct cs_hap {
+	struct regmap *regmap;
+	struct mutex lock;
+	struct device *dev;
+	struct list_head pseq_head;
+	struct cs_hap_bank *banks;
+	struct cs_hap_dsp dsp;
+	struct workqueue_struct *vibe_wq;
+	struct work_struct vibe_start_work;
+	struct work_struct vibe_stop_work;
+	struct work_struct erase_work;
+	struct work_struct add_work;
+	struct ff_effect *start_effect;
+	struct ff_effect *erase_effect;
+	struct ff_effect add_effect;
+	struct list_head effect_head;
+	int erase_error;
+	int start_error;
+	int stop_error;
+	int add_error;
+	bool runtime_pm;
+};
+
+int cs_hap_pseq_write(struct cs_hap *haptics, u32 addr,
+		      u32 data, bool update, u8 op_code);
+int cs_hap_pseq_multi_write(struct cs_hap *haptics,
+			    const struct reg_sequence *reg_seq,
+			    int num_regs, bool update, u8 op_code);
+int cs_hap_init(struct cs_hap *haptics);
+void cs_hap_remove(struct cs_hap *haptics);
+
+#endif
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 4/4] Input: cs40l50 - Add support for the CS40L50 haptic driver
From: James Ogletree @ 2023-10-18 17:57 UTC (permalink / raw)
  Cc: James Ogletree, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Lee Jones, Fred Treven, Ben Bright, linux-input, devicetree,
	linux-kernel
In-Reply-To: <20231018175726.3879955-1-james.ogletree@opensource.cirrus.com>

From: James Ogletree <james.ogletree@cirrus.com>

Introduce support for Cirrus Logic Device CS40L50: a
haptic driver with waveform memory, integrated DSP,
and closed-loop algorithms.

The input driver provides the interface for control of
haptic effects through the device.

Signed-off-by: James Ogletree <james.ogletree@cirrus.com>
---
 MAINTAINERS                        |   1 +
 drivers/input/misc/Kconfig         |  10 +
 drivers/input/misc/Makefile        |   1 +
 drivers/input/misc/cs40l50-vibra.c | 353 +++++++++++++++++++++++++++++
 4 files changed, 365 insertions(+)
 create mode 100644 drivers/input/misc/cs40l50-vibra.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 08e1e9695d49..24a00d8e5c1c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4971,6 +4971,7 @@ L:	patches@opensource.cirrus.com
 S:	Supported
 F:	Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
 F:	drivers/input/misc/cirrus*
+F:	drivers/input/misc/cs40l*
 F:	drivers/mfd/cs40l*
 F:	include/linux/input/cirrus*
 F:	include/linux/mfd/cs40l*
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 9f088900f863..938090648126 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -129,6 +129,16 @@ config INPUT_BMA150
 	  To compile this driver as a module, choose M here: the
 	  module will be called bma150.
 
+config INPUT_CS40L50_VIBRA
+	tristate "CS40L50 Haptic Driver support"
+	depends on MFD_CS40L50_CORE
+	help
+	  Say Y here to enable support for Cirrus Logic's CS40L50
+	  haptic driver.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cs40l50-vibra.
+
 config INPUT_E3X0_BUTTON
 	tristate "NI Ettus Research USRP E3xx Button support."
 	default n
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 6abefc41037b..6b653ed2124f 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_INPUT_CMA3000)		+= cma3000_d0x.o
 obj-$(CONFIG_INPUT_CMA3000_I2C)		+= cma3000_d0x_i2c.o
 obj-$(CONFIG_INPUT_COBALT_BTNS)		+= cobalt_btns.o
 obj-$(CONFIG_INPUT_CPCAP_PWRBUTTON)	+= cpcap-pwrbutton.o
+obj-$(CONFIG_INPUT_CS40L50_VIBRA)	+= cs40l50-vibra.o cirrus_haptics.o
 obj-$(CONFIG_INPUT_DA7280_HAPTICS)	+= da7280.o
 obj-$(CONFIG_INPUT_DA9052_ONKEY)	+= da9052_onkey.o
 obj-$(CONFIG_INPUT_DA9055_ONKEY)	+= da9055_onkey.o
diff --git a/drivers/input/misc/cs40l50-vibra.c b/drivers/input/misc/cs40l50-vibra.c
new file mode 100644
index 000000000000..3b3e4cb10de0
--- /dev/null
+++ b/drivers/input/misc/cs40l50-vibra.c
@@ -0,0 +1,353 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CS40L50 Advanced Haptic Driver with waveform memory,
+ * integrated DSP, and closed-loop algorithms
+ *
+ * Copyright 2023 Cirrus Logic, Inc.
+ *
+ */
+
+#include <linux/firmware/cirrus/wmfw.h>
+#include <linux/mfd/cs40l50.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+
+static int cs40l50_add(struct input_dev *dev,
+		       struct ff_effect *effect,
+		       struct ff_effect *old)
+{
+	struct cs40l50_private *cs40l50 = input_get_drvdata(dev);
+	u32 len = effect->u.periodic.custom_len;
+
+	if (effect->type != FF_PERIODIC || effect->u.periodic.waveform != FF_CUSTOM) {
+		dev_err(cs40l50->dev, "Type (%#X) or waveform (%#X) unsupported\n",
+			effect->type, effect->u.periodic.waveform);
+		return -EINVAL;
+	}
+
+	memcpy(&cs40l50->haptics.add_effect, effect, sizeof(struct ff_effect));
+
+	cs40l50->haptics.add_effect.u.periodic.custom_data = kcalloc(len,
+								     sizeof(s16),
+								     GFP_KERNEL);
+	if (!cs40l50->haptics.add_effect.u.periodic.custom_data)
+		return -ENOMEM;
+
+	if (copy_from_user(cs40l50->haptics.add_effect.u.periodic.custom_data,
+			   effect->u.periodic.custom_data, sizeof(s16) * len)) {
+		cs40l50->haptics.add_error = -EFAULT;
+		goto out_free;
+	}
+
+	queue_work(cs40l50->haptics.vibe_wq, &cs40l50->haptics.add_work);
+	flush_work(&cs40l50->haptics.add_work);
+
+out_free:
+	kfree(cs40l50->haptics.add_effect.u.periodic.custom_data);
+	cs40l50->haptics.add_effect.u.periodic.custom_data = NULL;
+
+	return cs40l50->haptics.add_error;
+}
+
+static int cs40l50_playback(struct input_dev *dev, int effect_id, int val)
+{
+	struct cs40l50_private *cs40l50 = input_get_drvdata(dev);
+
+	if (val > 0) {
+		cs40l50->haptics.start_effect = &dev->ff->effects[effect_id];
+		queue_work(cs40l50->haptics.vibe_wq,
+			   &cs40l50->haptics.vibe_start_work);
+	} else {
+		queue_work(cs40l50->haptics.vibe_wq,
+			   &cs40l50->haptics.vibe_stop_work);
+	}
+
+	return 0;
+}
+
+static int cs40l50_erase(struct input_dev *dev, int effect_id)
+{
+	struct cs40l50_private *cs40l50 = input_get_drvdata(dev);
+
+	cs40l50->haptics.erase_effect = &dev->ff->effects[effect_id];
+
+	queue_work(cs40l50->haptics.vibe_wq, &cs40l50->haptics.erase_work);
+	flush_work(&cs40l50->haptics.erase_work);
+
+	return cs40l50->haptics.erase_error;
+}
+
+static const struct reg_sequence cs40l50_int_vamp_seq[] = {
+	{ CS40L50_BST_LPMODE_SEL,	CS40L50_DCM_LOW_POWER },
+	{ CS40L50_BLOCK_ENABLES2,	CS40L50_OVERTEMP_WARN },
+};
+
+static const struct reg_sequence cs40l50_irq_mask_seq[] = {
+	{ CS40L50_IRQ1_MASK_2,	CS40L50_IRQ_MASK_2_OVERRIDE },
+	{ CS40L50_IRQ1_MASK_20,	CS40L50_IRQ_MASK_20_OVERRIDE },
+};
+
+static int cs40l50_hw_init(struct cs40l50_private *cs40l50)
+{
+	int error;
+
+	error = regmap_multi_reg_write(cs40l50->regmap,
+				       cs40l50_int_vamp_seq,
+				       ARRAY_SIZE(cs40l50_int_vamp_seq));
+	if (error)
+		return error;
+
+	error = cs_hap_pseq_multi_write(&cs40l50->haptics,
+					cs40l50_int_vamp_seq,
+					ARRAY_SIZE(cs40l50_int_vamp_seq),
+					false, PSEQ_OP_WRITE_FULL);
+	if (error)
+		return error;
+
+	error = regmap_multi_reg_write(cs40l50->regmap, cs40l50_irq_mask_seq,
+				       ARRAY_SIZE(cs40l50_irq_mask_seq));
+	if (error)
+		return error;
+
+	return cs_hap_pseq_multi_write(&cs40l50->haptics, cs40l50_irq_mask_seq,
+				       ARRAY_SIZE(cs40l50_irq_mask_seq), false,
+				       PSEQ_OP_WRITE_FULL);
+}
+
+static const struct cs_dsp_client_ops cs40l50_cs_dsp_client_ops;
+
+static const struct cs_dsp_region cs40l50_dsp_regions[] = {
+	{
+		.type = WMFW_HALO_PM_PACKED,
+		.base = CS40L50_DSP1_PMEM_0
+	},
+	{
+		.type = WMFW_HALO_XM_PACKED,
+		.base = CS40L50_DSP1_XMEM_PACKED_0
+	},
+	{
+		.type = WMFW_HALO_YM_PACKED,
+		.base = CS40L50_DSP1_YMEM_PACKED_0
+	},
+	{
+		.type = WMFW_ADSP2_XM,
+		.base = CS40L50_DSP1_XMEM_UNPACKED24_0
+	},
+	{
+		.type = WMFW_ADSP2_YM,
+		.base = CS40L50_DSP1_YMEM_UNPACKED24_0
+	},
+};
+
+static int cs40l50_cs_dsp_init(struct cs40l50_private *cs40l50)
+{
+	cs40l50->dsp.num = 1;
+	cs40l50->dsp.type = WMFW_HALO;
+	cs40l50->dsp.dev = cs40l50->dev;
+	cs40l50->dsp.regmap = cs40l50->regmap;
+	cs40l50->dsp.base = CS40L50_CORE_BASE;
+	cs40l50->dsp.base_sysinfo = CS40L50_SYS_INFO_ID;
+	cs40l50->dsp.mem = cs40l50_dsp_regions;
+	cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions);
+	cs40l50->dsp.no_core_startstop = true;
+	cs40l50->dsp.client_ops = &cs40l50_cs_dsp_client_ops;
+
+	return cs_dsp_halo_init(&cs40l50->dsp);
+}
+
+static struct cs_hap_bank cs40l50_banks[] = {
+	{
+		.bank =		WVFRM_BANK_RAM,
+		.base_index =	CS40L50_RAM_BANK_INDEX_START,
+		.max_index =	CS40L50_RAM_BANK_INDEX_START,
+	},
+	{
+		.bank =		WVFRM_BANK_ROM,
+		.base_index =	CS40L50_ROM_BANK_INDEX_START,
+		.max_index =	CS40L50_ROM_BANK_INDEX_END,
+	},
+	{
+		.bank =		WVFRM_BANK_OWT,
+		.base_index =	CS40L50_RTH_INDEX_START,
+		.max_index =	CS40L50_RTH_INDEX_END,
+	},
+};
+
+static int cs40l50_cs_hap_init(struct cs40l50_private *cs40l50)
+{
+	cs40l50->haptics.regmap = cs40l50->regmap;
+	cs40l50->haptics.dev = cs40l50->dev;
+	cs40l50->haptics.banks = cs40l50_banks;
+	cs40l50->haptics.dsp.gpio_base_reg = CS40L50_GPIO_BASE;
+	cs40l50->haptics.dsp.owt_base_reg = CS40L50_OWT_BASE;
+	cs40l50->haptics.dsp.owt_offset_reg = CS40L50_OWT_NEXT;
+	cs40l50->haptics.dsp.owt_size_reg = CS40L50_OWT_SIZE;
+	cs40l50->haptics.dsp.mailbox_reg = CS40L50_DSP_MBOX;
+	cs40l50->haptics.dsp.pseq_reg = CS40L50_POWER_ON_SEQ;
+	cs40l50->haptics.dsp.push_owt_cmd = CS40L50_OWT_PUSH;
+	cs40l50->haptics.dsp.delete_owt_cmd = CS40L50_OWT_DELETE;
+	cs40l50->haptics.dsp.stop_cmd = CS40L50_STOP_PLAYBACK;
+	cs40l50->haptics.dsp.pseq_size = CS40L50_PSEQ_SIZE;
+	cs40l50->haptics.runtime_pm = true;
+
+	return cs_hap_init(&cs40l50->haptics);
+}
+
+static void cs40l50_upload_wt(const struct firmware *bin, void *context)
+{
+	struct cs40l50_private *cs40l50 = context;
+	u32 nwaves;
+
+	mutex_lock(&cs40l50->lock);
+
+	if (cs40l50->wmfw) {
+		if (regmap_write(cs40l50->regmap, CS40L50_CCM_CORE_CONTROL,
+				 CS40L50_CLOCK_DISABLE))
+			goto err_mutex;
+
+		if (regmap_write(cs40l50->regmap, CS40L50_RAM_INIT,
+				 CS40L50_RAM_INIT_FLAG))
+			goto err_mutex;
+
+		if (regmap_write(cs40l50->regmap, CS40L50_PWRMGT_CTL,
+				 CS40L50_MEM_RDY_HW))
+			goto err_mutex;
+	}
+
+	cs_dsp_power_up(&cs40l50->dsp, cs40l50->wmfw, "cs40l50.wmfw",
+			bin, "cs40l50.bin", "cs40l50");
+
+	if (cs40l50->wmfw) {
+		if (regmap_write(cs40l50->regmap, CS40L50_CCM_CORE_CONTROL,
+				 CS40L50_CLOCK_ENABLE))
+			goto err_mutex;
+	}
+
+	if (regmap_read(cs40l50->regmap, CS40L50_NUM_OF_WAVES, &nwaves))
+		goto err_mutex;
+
+	cs40l50->haptics.banks[WVFRM_BANK_RAM].max_index += (nwaves - 1);
+
+err_mutex:
+	mutex_unlock(&cs40l50->lock);
+	release_firmware(bin);
+	release_firmware(cs40l50->wmfw);
+}
+
+static void cs40l50_upload_patch(const struct firmware *wmfw, void *context)
+{
+	struct cs40l50_private *cs40l50 = context;
+
+	cs40l50->wmfw = wmfw;
+
+	if (request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT, CS40L50_WT,
+				    cs40l50->dev, GFP_KERNEL,
+				    cs40l50, cs40l50_upload_wt))
+		release_firmware(cs40l50->wmfw);
+}
+
+static int cs40l50_input_init(struct cs40l50_private *cs40l50)
+{
+	int error;
+
+	cs40l50->input = devm_input_allocate_device(cs40l50->dev);
+	if (!cs40l50->input)
+		return -ENOMEM;
+
+	cs40l50->input->id.product = cs40l50->devid & 0xFFFF;
+	cs40l50->input->id.version = cs40l50->revid;
+	cs40l50->input->name = "cs40l50_vibra";
+
+	input_set_drvdata(cs40l50->input, cs40l50);
+	input_set_capability(cs40l50->input, EV_FF, FF_PERIODIC);
+	input_set_capability(cs40l50->input, EV_FF, FF_CUSTOM);
+
+	error = input_ff_create(cs40l50->input, FF_MAX_EFFECTS);
+	if (error)
+		return error;
+
+	cs40l50->input->ff->upload = cs40l50_add;
+	cs40l50->input->ff->playback = cs40l50_playback;
+	cs40l50->input->ff->erase = cs40l50_erase;
+
+	INIT_LIST_HEAD(&cs40l50->haptics.effect_head);
+
+	error = input_register_device(cs40l50->input);
+	if (error)
+		goto err_free_dev;
+
+	return cs40l50_cs_hap_init(cs40l50);
+
+err_free_dev:
+	input_free_device(cs40l50->input);
+	return error;
+}
+static int cs40l50_vibra_probe(struct platform_device *pdev)
+{
+	struct cs40l50_private *cs40l50 = dev_get_drvdata(pdev->dev.parent);
+	int error;
+
+	error = cs40l50_input_init(cs40l50);
+	if (error)
+		return error;
+
+	error = cs40l50_hw_init(cs40l50);
+	if (error)
+		goto err_input;
+
+	error = cs40l50_cs_dsp_init(cs40l50);
+	if (error)
+		goto err_input;
+
+	error = request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT,
+					CS40L50_FW, cs40l50->dev,
+					GFP_KERNEL, cs40l50,
+					cs40l50_upload_patch);
+	if (error)
+		goto err_dsp;
+
+	return 0;
+
+err_dsp:
+	cs_dsp_remove(&cs40l50->dsp);
+err_input:
+	input_unregister_device(cs40l50->input);
+	cs_hap_remove(&cs40l50->haptics);
+
+	return error;
+}
+
+static int cs40l50_vibra_remove(struct platform_device *pdev)
+{
+	struct cs40l50_private *cs40l50 = dev_get_drvdata(pdev->dev.parent);
+
+	input_unregister_device(cs40l50->input);
+	cs_hap_remove(&cs40l50->haptics);
+
+	if (cs40l50->dsp.booted)
+		cs_dsp_power_down(&cs40l50->dsp);
+	if (&cs40l50->dsp)
+		cs_dsp_remove(&cs40l50->dsp);
+
+	return 0;
+}
+
+static const struct platform_device_id cs40l50_id_vibra[] = {
+	{"cs40l50-vibra", },
+	{}
+};
+MODULE_DEVICE_TABLE(platform, cs40l50_id_vibra);
+
+static struct platform_driver cs40l50_vibra_driver = {
+	.probe		= cs40l50_vibra_probe,
+	.remove		= cs40l50_vibra_remove,
+	.id_table	= cs40l50_id_vibra,
+	.driver		= {
+		.name	= "cs40l50-vibra",
+	},
+};
+module_platform_driver(cs40l50_vibra_driver);
+
+MODULE_DESCRIPTION("CS40L50 Advanced Haptic Driver");
+MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
+MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 1/4] dt-bindings: input: cirrus,cs40l50: Add initial DT binding
From: James Ogletree @ 2023-10-18 17:57 UTC (permalink / raw)
  Cc: James Ogletree, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Lee Jones, Fred Treven, Ben Bright, linux-input, devicetree,
	linux-kernel
In-Reply-To: <20231018175726.3879955-1-james.ogletree@opensource.cirrus.com>

From: James Ogletree <james.ogletree@cirrus.com>

The CS40L50 is a haptic driver with waveform memory,
integrated DSP, and closed-loop algorithms.

Add a YAML DT binding document for this device.

Signed-off-by: James Ogletree <james.ogletree@cirrus.com>
---
 .../bindings/input/cirrus,cs40l50.yaml        | 70 +++++++++++++++++++
 MAINTAINERS                                   |  8 +++
 2 files changed, 78 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml

diff --git a/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml b/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
new file mode 100644
index 000000000000..6a5bdafed56b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/cirrus,cs40l50.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cirrus Logic CS40L50 Advanced Haptic Driver
+
+maintainers:
+  - James Ogletree <james.ogletree@cirrus.com>
+
+description:
+  CS40L50 is a haptic driver with waveform memory,
+  integrated DSP, and closed-loop algorithms.
+
+properties:
+  compatible:
+    enum:
+      - cirrus,cs40l50
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  va-supply:
+    description: Power supply for internal analog circuits.
+
+  vp-supply:
+    description: Power supply for always-on circuits.
+
+  vio-supply:
+    description: Power supply for digital input/output.
+
+  vamp-supply:
+    description: Power supply for the Class D amplifier.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - reset-gpios
+  - vp-supply
+  - vio-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      haptic-driver@34 {
+        compatible = "cirrus,cs40l50";
+        reg = <0x34>;
+        interrupt-parent = <&gpio>;
+        interrupts = <113 IRQ_TYPE_LEVEL_LOW>;
+        reset-gpios = <&gpio 112 GPIO_ACTIVE_LOW>;
+        vp-supply = <&vreg>;
+        vio-supply = <&vreg>;
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 046ff06ff97f..28f0ca9324b3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4963,6 +4963,14 @@ F:	sound/pci/hda/cs*
 F:	sound/pci/hda/hda_cs_dsp_ctl.*
 F:	sound/soc/codecs/cs*
 
+CIRRUS LOGIC HAPTIC DRIVERS
+M:	James Ogletree <james.ogletree@cirrus.com>
+M:	Fred Treven <fred.treven@cirrus.com>
+M:	Ben Bright <ben.bright@cirrus.com>
+L:	patches@opensource.cirrus.com
+S:	Supported
+F:	Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
+
 CIRRUS LOGIC DSP FIRMWARE DRIVER
 M:	Simon Trimmer <simont@opensource.cirrus.com>
 M:	Charles Keepax <ckeepax@opensource.cirrus.com>
-- 
2.25.1


^ permalink raw reply related

* [PATCH v4 0/4] Add support for CS40L50
From: James Ogletree @ 2023-10-18 17:57 UTC (permalink / raw)
  Cc: James Ogletree, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Lee Jones, Fred Treven, Ben Bright, linux-input, devicetree,
	linux-kernel

From: James Ogletree <james.ogletree@cirrus.com>

This patch series adds support for Cirrus Logic CS40L50, a haptic driver.

While I2S streaming to the device will need to be supported in the future,
no codec driver is included in this submission and therefore this MFD
driver has just one component. A bare bones codec driver can be created
and included if maintainers prefer.

Changes in v4:
- Move from Input to MFD
- Move common Cirrus haptic functions to a library
- Incorporate runtime PM framework
- Coding style related improvements

Changes in v3:
- YAML formatting corrections
- Fix typo in MAINTAINERS
- Use generic node name "haptic-driver"
- Fix probe error code paths
- Use sizeof(*)
- Remove tree reference in MAINTAINERS

Changes in v2:
- Fix checkpatch warnings

James Ogletree (4):
  dt-bindings: input: cirrus,cs40l50: Add initial DT binding
  Input: cs40l50 - Add cirrus haptics base support
  mfd: cs40l50: Add support for CS40L50 core driver
  Input: cs40l50 - Add support for the CS40L50 haptic driver

 .../bindings/input/cirrus,cs40l50.yaml        |  70 +++
 MAINTAINERS                                   |  13 +
 drivers/input/misc/Kconfig                    |  10 +
 drivers/input/misc/Makefile                   |   1 +
 drivers/input/misc/cirrus_haptics.c           | 586 ++++++++++++++++++
 drivers/input/misc/cs40l50-vibra.c            | 353 +++++++++++
 drivers/mfd/Kconfig                           |  30 +
 drivers/mfd/Makefile                          |   4 +
 drivers/mfd/cs40l50-core.c                    | 443 +++++++++++++
 drivers/mfd/cs40l50-i2c.c                     |  69 +++
 drivers/mfd/cs40l50-spi.c                     |  68 ++
 include/linux/input/cirrus_haptics.h          | 121 ++++
 include/linux/mfd/cs40l50.h                   | 198 ++++++
 13 files changed, 1966 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
 create mode 100644 drivers/input/misc/cirrus_haptics.c
 create mode 100644 drivers/input/misc/cs40l50-vibra.c
 create mode 100644 drivers/mfd/cs40l50-core.c
 create mode 100644 drivers/mfd/cs40l50-i2c.c
 create mode 100644 drivers/mfd/cs40l50-spi.c
 create mode 100644 include/linux/input/cirrus_haptics.h
 create mode 100644 include/linux/mfd/cs40l50.h

-- 
2.25.1


^ permalink raw reply

* Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
From: Shyam Sundar S K @ 2023-10-18 17:05 UTC (permalink / raw)
  To: Christian König, Mario Limonciello, Ilpo Järvinen
  Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
	benjamin.tissoires, alexander.deucher, Xinhui.Pan, airlied,
	daniel, Patil.Reddy, platform-driver-x86, linux-input, amd-gfx,
	dri-devel
In-Reply-To: <c8ed2e1e-77b9-459e-b81a-e95db4d22a9b@amd.com>



On 10/18/2023 9:37 PM, Christian König wrote:
> Am 18.10.23 um 17:47 schrieb Mario Limonciello:
>> On 10/18/2023 08:40, Christian König wrote:
>>>
>>>
>>> Am 18.10.23 um 11:28 schrieb Shyam Sundar S K:
>>>>
>>>> On 10/18/2023 2:50 PM, Ilpo Järvinen wrote:
>>>>> On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
>>>>>
>>>>>> In order to provide GPU inputs to TA for the Smart PC solution
>>>>>> to work, we
>>>>>> need to have interface between the PMF driver and the AMDGPU
>>>>>> driver.
>>>>>>
>>>>>> Add the initial code path for get interface from AMDGPU.
>>>>>>
>>>>>> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
>>>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>>>>> ---
>>>>>>   drivers/gpu/drm/amd/amdgpu/Makefile     |   2 +
>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h     |   1 +
>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 138
>>>>>> ++++++++++++++++++++++++
>>>>>>   drivers/platform/x86/amd/pmf/Kconfig    |   1 +
>>>>>>   drivers/platform/x86/amd/pmf/core.c     |   1 +
>>>>>>   drivers/platform/x86/amd/pmf/pmf.h      |   3 +
>>>>>>   drivers/platform/x86/amd/pmf/spc.c      |  13 +++
>>>>>>   drivers/platform/x86/amd/pmf/tee-if.c   |  26 +++++
>>>>>>   include/linux/amd-pmf-io.h              |  35 ++++++
>>>>>>   9 files changed, 220 insertions(+)
>>>>>>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>>>>   create mode 100644 include/linux/amd-pmf-io.h
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
>>>>>> b/drivers/gpu/drm/amd/amdgpu/Makefile
>>>>>> index 384b798a9bad..7fafccefbd7a 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
>>>>>> @@ -86,6 +86,8 @@ amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
>>>>>>   amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
>>>>>> +amdgpu-$(CONFIG_AMD_PMF) += amdgpu_pmf.o
>>>>>> +
>>>>>>   # add asic specific block
>>>>>>   amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o \
>>>>>>       dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>>> index a79d53bdbe13..26ffa1b4fe57 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>>> @@ -50,6 +50,7 @@
>>>>>>   #include <linux/hashtable.h>
>>>>>>   #include <linux/dma-fence.h>
>>>>>>   #include <linux/pci.h>
>>>>>> +#include <linux/amd-pmf-io.h>
>>>>>>   #include <drm/ttm/ttm_bo.h>
>>>>>>   #include <drm/ttm/ttm_placement.h>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>>>> new file mode 100644
>>>>>> index 000000000000..ac981848df50
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>>>> @@ -0,0 +1,138 @@
>>>>>> +/*
>>>>>> + * Copyright 2023 Advanced Micro Devices, Inc.
>>>>>> + *
>>>>>> + * Permission is hereby granted, free of charge, to any person
>>>>>> obtaining a
>>>>>> + * copy of this software and associated documentation files
>>>>>> (the "Software"),
>>>>>> + * to deal in the Software without restriction, including
>>>>>> without limitation
>>>>>> + * the rights to use, copy, modify, merge, publish, distribute,
>>>>>> sublicense,
>>>>>> + * and/or sell copies of the Software, and to permit persons to
>>>>>> whom the
>>>>>> + * Software is furnished to do so, subject to the following
>>>>>> conditions:
>>>>>> + *
>>>>>> + * The above copyright notice and this permission notice shall
>>>>>> be included in
>>>>>> + * all copies or substantial portions of the Software.
>>>>>> + *
>>>>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
>>>>>> KIND, EXPRESS OR
>>>>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>>>>>> MERCHANTABILITY,
>>>>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
>>>>>> EVENT SHALL
>>>>>> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY
>>>>>> CLAIM, DAMAGES OR
>>>>>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>>>>>> OTHERWISE,
>>>>>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR
>>>>>> THE USE OR
>>>>>> + * OTHER DEALINGS IN THE SOFTWARE.
>>>>> This is MIT, right? Add the required SPDX-License-Identifier line
>>>>> for it
>>>>> at the top of the file, thank you.
>>>>>
>>>> all the files in drivers/gpu/drm/amd/amdgpu/* carry the same license
>>>> text. So, have retained it to maintain uniformity.
>>>
>>> Please add the SPDX License Identifier for any file you add.

OK. I thought to keep it same like the other files. I will change this
to SPDX in the next revision.

>>>
>>> Apart from that the whole approach of attaching this directly to
>>> amdgpu looks extremely questionable to me.
>>>
>>
>> What's the long term outlook for things that are needed directly
>> from amdgpu?  Is there going to be more besides the backlight and
>> the display count/type?
> 
> Yeah, that goes into the same direction as my concern.

PMF is an APU only feature and will need inputs from multiple
subsystems (in this case its GPU) to send changing system information
to PMF TA (Trusted Application, running on ASP/PSP) as input.

Its not only about the display count/type and backlight, there are
many others things in pipe like the GPU engine running time,
utilization percentage etc, that guide the TA to make better decisions.

This series is only targeted to build the initial plubming with the
subsystems inside the kernel and later keep adding changes to get more
information from other subsystems.

that is the reason you see that, this patch proposes amd-pmf-io.h as
the interface to talk to other subsystems. For the initial path, I
have opted to get information from SFH and GPU drivers. But this is
meant to grow in future.

> 
>>
>> At least for the display count I suppose one way that it could be
>> "decoupled" from amdgpu is to use drm_for_each_connector_iter to
>> iterate all the connectors and then count the connected ones.
> 
> And what if the number of connected displays change? How is amdgpu
> supposed to signal events like that?

PMF driver polls for the information based on a configurable sampling
frequency.

you can look at amd_pmf_get_gpu_info(), that gets called from
amd_pmf_populate_ta_inputs() in spc.c in this proposed series.

Thanks,
Shyam

> 
> This whole solution doesn't looks well thought through.
> 
> Regards,
> Christian.
> 
>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>>>> + *
>>>>>> + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>>>>> + *
>>>>>> + */
>>>>> Remove the extra empty line at the end of the comment.
>>>>>
>>>> I just took the standard template for all the gpu files. Is that
>>>> OK to
>>>> retain the blank line?
>>>>
>>>> If not, I can remove it in the next version.
>>>>
>>>> Rest all remarks I will address.
>>>>
>>>> Thanks,
>>>> Shyam
>>>>
>>>>>> +
>>>>>> +#include <linux/backlight.h>
>>>>>> +#include "amdgpu.h"
>>>>>> +
>>>>>> +int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
>>>>>> +{
>>>>>> +    struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
>>>>>> +    struct drm_mode_config *mode_config = &drm_dev->mode_config;
>>>>>> +    struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>>>>> +    struct drm_connector_list_iter iter;
>>>>>> +    struct drm_connector *connector;
>>>>>> +    int i = 0;
>>>>>> +
>>>>>> +    /* Reset the count to zero */
>>>>>> +    pmf->display_count = 0;
>>>>>> +    if (!(adev->flags & AMD_IS_APU)) {
>>>>>> +        DRM_ERROR("PMF-AMDGPU interface not supported\n");
>>>>>> +        return -ENODEV;
>>>>>> +    }
>>>>>> +
>>>>>> +    mutex_lock(&mode_config->mutex);
>>>>>> +    drm_connector_list_iter_begin(drm_dev, &iter);
>>>>>> +    drm_for_each_connector_iter(connector, &iter) {
>>>>>> +        if (connector->status == connector_status_connected)
>>>>>> +            pmf->display_count++;
>>>>>> +        if (connector->status != pmf->con_status[i])
>>>>>> +            pmf->con_status[i] = connector->status;
>>>>>> +        if (connector->connector_type != pmf->connector_type[i])
>>>>>> +            pmf->connector_type[i] = connector->connector_type;
>>>>>> +
>>>>>> +        i++;
>>>>>> +        if (i >= MAX_SUPPORTED)
>>>>>> +            break;
>>>>>> +    }
>>>>>> +    drm_connector_list_iter_end(&iter);
>>>>>> +    mutex_unlock(&mode_config->mutex);
>>>>>> +
>>>>>> +    return 0;
>>>>>> +}
>>>>>> +EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
>>>>>> +
>>>>>> +static int amd_pmf_gpu_get_cur_state(struct
>>>>>> thermal_cooling_device *cooling_dev,
>>>>>> +                     unsigned long *state)
>>>>>> +{
>>>>>> +    struct backlight_device *bd;
>>>>>> +
>>>>>> +    if (!acpi_video_backlight_use_native())
>>>>>> +        return -ENODEV;
>>>>>> +
>>>>>> +    bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>>>>> +    if (!bd)
>>>>>> +        return -ENODEV;
>>>>>> +
>>>>>> +    *state = backlight_get_brightness(bd);
>>>>>> +
>>>>>> +    return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static int amd_pmf_gpu_get_max_state(struct
>>>>>> thermal_cooling_device *cooling_dev,
>>>>>> +                     unsigned long *state)
>>>>>> +{
>>>>>> +    struct backlight_device *bd;
>>>>>> +
>>>>>> +    if (!acpi_video_backlight_use_native())
>>>>>> +        return -ENODEV;
>>>>>> +
>>>>>> +    bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>>>>> +    if (!bd)
>>>>>> +        return -ENODEV;
>>>>>> +
>>>>>> +    if (backlight_is_blank(bd))
>>>>>> +        *state = 0;
>>>>>> +    else
>>>>>> +        *state = bd->props.max_brightness;
>>>>>> +
>>>>>> +    return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static const struct thermal_cooling_device_ops bd_cooling_ops = {
>>>>>> +    .get_max_state = amd_pmf_gpu_get_max_state,
>>>>>> +    .get_cur_state = amd_pmf_gpu_get_cur_state,
>>>>>> +};
>>>>>> +
>>>>>> +int amd_pmf_gpu_init(struct amd_gpu_pmf_data *pmf)
>>>>>> +{
>>>>>> +    struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
>>>>>> +    struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>>>>> +
>>>>>> +    if (!(adev->flags & AMD_IS_APU)) {
>>>>>> +        DRM_ERROR("PMF-AMDGPU interface not supported\n");
>>>>>> +        return -ENODEV;
>>>>>> +    }
>>>>>> +
>>>>>> +    if (!acpi_video_backlight_use_native())
>>>>>> +        return -ENODEV;
>>>>>> +
>>>>>> +    pmf->raw_bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>>>>> +    if (!pmf->raw_bd)
>>>>>> +        return -ENODEV;
>>>>>> +
>>>>>> +    pmf->cooling_dev =
>>>>>> thermal_cooling_device_register("pmf_gpu_bd",
>>>>>> +                               pmf, &bd_cooling_ops);
>>>>>> +    if (IS_ERR(pmf->cooling_dev))
>>>>>> +        return -ENODEV;
>>>>>> +
>>>>>> +    return 0;
>>>>>> +}
>>>>>> +EXPORT_SYMBOL_GPL(amd_pmf_gpu_init);
>>>>>> +
>>>>>> +void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf)
>>>>>> +{
>>>>>> + thermal_cooling_device_unregister(pmf->cooling_dev);
>>>>>> +}
>>>>>> +EXPORT_SYMBOL_GPL(amd_pmf_gpu_deinit);
>>>>>> diff --git a/drivers/platform/x86/amd/pmf/Kconfig
>>>>>> b/drivers/platform/x86/amd/pmf/Kconfig
>>>>>> index f246252bddd8..7f430de7af44 100644
>>>>>> --- a/drivers/platform/x86/amd/pmf/Kconfig
>>>>>> +++ b/drivers/platform/x86/amd/pmf/Kconfig
>>>>>> @@ -10,6 +10,7 @@ config AMD_PMF
>>>>>>       depends on AMD_NB
>>>>>>       select ACPI_PLATFORM_PROFILE
>>>>>>       depends on TEE && AMDTEE
>>>>>> +    depends on DRM_AMDGPU
>>>>>>       help
>>>>>>         This driver provides support for the AMD Platform
>>>>>> Management Framework.
>>>>>>         The goal is to enhance end user experience by making AMD
>>>>>> PCs smarter,
>>>>>> diff --git a/drivers/platform/x86/amd/pmf/core.c
>>>>>> b/drivers/platform/x86/amd/pmf/core.c
>>>>>> index 4b8156033fa6..c59ba527ff49 100644
>>>>>> --- a/drivers/platform/x86/amd/pmf/core.c
>>>>>> +++ b/drivers/platform/x86/amd/pmf/core.c
>>>>>> @@ -411,6 +411,7 @@ static int amd_pmf_probe(struct
>>>>>> platform_device *pdev)
>>>>>>       }
>>>>>>       dev->cpu_id = rdev->device;
>>>>>> +    dev->root = rdev;
>>>>>>       err = amd_smn_read(0, AMD_PMF_BASE_ADDR_LO, &val);
>>>>>>       if (err) {
>>>>>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
>>>>>> b/drivers/platform/x86/amd/pmf/pmf.h
>>>>>> index 8712299ad52b..615cd3a31986 100644
>>>>>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>>>>>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>>>>>> @@ -13,6 +13,7 @@
>>>>>>   #include <linux/acpi.h>
>>>>>>   #include <linux/platform_profile.h>
>>>>>> +#include <linux/amd-pmf-io.h>
>>>>>>   #define POLICY_BUF_MAX_SZ        0x4b000
>>>>>>   #define POLICY_SIGN_COOKIE        0x31535024
>>>>>> @@ -228,9 +229,11 @@ struct amd_pmf_dev {
>>>>>>       void *shbuf;
>>>>>>       struct delayed_work pb_work;
>>>>>>       struct pmf_action_table *prev_data;
>>>>>> +    struct amd_gpu_pmf_data gfx_data;
>>>>>>       u64 policy_addr;
>>>>>>       void *policy_base;
>>>>>>       bool smart_pc_enabled;
>>>>>> +    struct pci_dev *root;
>>>>>>   };
>>>>>>   struct apmf_sps_prop_granular {
>>>>>> diff --git a/drivers/platform/x86/amd/pmf/spc.c
>>>>>> b/drivers/platform/x86/amd/pmf/spc.c
>>>>>> index 512e0c66efdc..cf4962ef97c2 100644
>>>>>> --- a/drivers/platform/x86/amd/pmf/spc.c
>>>>>> +++ b/drivers/platform/x86/amd/pmf/spc.c
>>>>>> @@ -44,6 +44,10 @@ void amd_pmf_dump_ta_inputs(struct
>>>>>> amd_pmf_dev *dev, struct ta_pmf_enact_table *
>>>>>>       dev_dbg(dev->dev, "Max C0 Residency : %u\n",
>>>>>> in->ev_info.max_c0residency);
>>>>>>       dev_dbg(dev->dev, "GFX Busy : %u\n", in->ev_info.gfx_busy);
>>>>>>       dev_dbg(dev->dev, "Connected Display Count : %u\n",
>>>>>> in->ev_info.monitor_count);
>>>>>> +    dev_dbg(dev->dev, "Primary Display Type : %s\n",
>>>>>> + drm_get_connector_type_name(in->ev_info.display_type));
>>>>>> +    dev_dbg(dev->dev, "Primary Display State : %s\n",
>>>>>> in->ev_info.display_state ?
>>>>>> +            "Connected" : "disconnected/unknown");
>>>>>>       dev_dbg(dev->dev, "LID State : %s\n",
>>>>>> in->ev_info.lid_state ? "Close" : "Open");
>>>>>>       dev_dbg(dev->dev, "==== TA inputs END ====\n");
>>>>>>   }
>>>>>> @@ -146,6 +150,14 @@ static int amd_pmf_get_slider_info(struct
>>>>>> amd_pmf_dev *dev, struct ta_pmf_enact_
>>>>>>       return 0;
>>>>>>   }
>>>>>> +static void amd_pmf_get_gpu_info(struct amd_pmf_dev *dev,
>>>>>> struct ta_pmf_enact_table *in)
>>>>>> +{
>>>>>> +    amd_pmf_get_gfx_data(&dev->gfx_data);
>>>>>> +    in->ev_info.monitor_count = dev->gfx_data.display_count;
>>>>>> +    in->ev_info.display_type = dev->gfx_data.connector_type[0];
>>>>>> +    in->ev_info.display_state = dev->gfx_data.con_status[0];
>>>>>> +}
>>>>>> +
>>>>>>   void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev,
>>>>>> struct ta_pmf_enact_table *in)
>>>>>>   {
>>>>>>       /* TA side lid open is 1 and close is 0, hence the ! here */
>>>>>> @@ -154,4 +166,5 @@ void amd_pmf_populate_ta_inputs(struct
>>>>>> amd_pmf_dev *dev, struct ta_pmf_enact_tab
>>>>>>       amd_pmf_get_smu_info(dev, in);
>>>>>>       amd_pmf_get_battery_info(dev, in);
>>>>>>       amd_pmf_get_slider_info(dev, in);
>>>>>> +    amd_pmf_get_gpu_info(dev, in);
>>>>>>   }
>>>>>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
>>>>>> b/drivers/platform/x86/amd/pmf/tee-if.c
>>>>>> index 2f5fb8623c20..956e66b78605 100644
>>>>>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>>>>>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>>>>>> @@ -9,6 +9,7 @@
>>>>>>    */
>>>>>>   #include <linux/debugfs.h>
>>>>>> +#include <linux/pci.h>
>>>>>>   #include <linux/tee_drv.h>
>>>>>>   #include <linux/uuid.h>
>>>>>>   #include "pmf.h"
>>>>>> @@ -357,6 +358,19 @@ static int amd_pmf_get_bios_buffer(struct
>>>>>> amd_pmf_dev *dev)
>>>>>>       return amd_pmf_start_policy_engine(dev);
>>>>>>   }
>>>>>> +static int amd_pmf_get_gpu_handle(struct pci_dev *pdev, void
>>>>>> *data)
>>>>>> +{
>>>>>> +    struct amd_pmf_dev *dev = data;
>>>>>> +
>>>>>> +    if (pdev->vendor == PCI_VENDOR_ID_ATI && pdev->devfn == 0) {
>>>>>> +        /* Found the amdgpu handle from the pci root after
>>>>>> walking through the pci bus */
>>>>> If you insist on having this comment, make it a function comment
>>>>> instead
>>>>> (with appropriate modifications into the content of it) but I
>>>>> personally
>>>>> don't find it that useful so it could be just dropped as well, IMO.
>>>>>
>>>>>> +        dev->gfx_data.gpu_dev = pdev;
>>>>>> +        return 1; /* Stop walking */
>>>>>> +    }
>>>>>> +
>>>>>> +    return 0; /* Continue walking */
>>>>>> +}
>>>>>> +
>>>>>>   static int amd_pmf_amdtee_ta_match(struct
>>>>>> tee_ioctl_version_data *ver, const void *data)
>>>>>>   {
>>>>>>       return ver->impl_id == TEE_IMPL_ID_AMDTEE;
>>>>>> @@ -446,6 +460,15 @@ int amd_pmf_init_smart_pc(struct
>>>>>> amd_pmf_dev *dev)
>>>>>>       INIT_DELAYED_WORK(&dev->pb_work, amd_pmf_invoke_cmd);
>>>>>>       amd_pmf_set_dram_addr(dev);
>>>>>>       amd_pmf_get_bios_buffer(dev);
>>>>>> +
>>>>>> +    /* Get amdgpu handle */
>>>>> Useless comment.
>>>>>
>>>>>> + pci_walk_bus(dev->root->bus, amd_pmf_get_gpu_handle, dev);
>>>>>> +    if (!dev->gfx_data.gpu_dev)
>>>>>> +        dev_err(dev->dev, "GPU handle not found!\n");
>>>>>> +
>>>>>> +    if (!amd_pmf_gpu_init(&dev->gfx_data))
>>>>>> +        dev->gfx_data.gpu_dev_en = true;
>>>>>> +
>>>>>>       dev->prev_data = kzalloc(sizeof(*dev->prev_data),
>>>>>> GFP_KERNEL);
>>>>>>       if (!dev->prev_data)
>>>>>>           return -ENOMEM;
>>>>>> @@ -461,5 +484,8 @@ void amd_pmf_deinit_smart_pc(struct
>>>>>> amd_pmf_dev *dev)
>>>>>>       kfree(dev->prev_data);
>>>>>>       kfree(dev->policy_buf);
>>>>>>       cancel_delayed_work_sync(&dev->pb_work);
>>>>>> +    if (dev->gfx_data.gpu_dev_en)
>>>>>> +        amd_pmf_gpu_deinit(&dev->gfx_data);
>>>>>> +    pci_dev_put(dev->gfx_data.gpu_dev);
>>>>>>       amd_pmf_tee_deinit(dev);
>>>>>>   }
>>>>>> diff --git a/include/linux/amd-pmf-io.h
>>>>>> b/include/linux/amd-pmf-io.h
>>>>>> new file mode 100644
>>>>>> index 000000000000..5f79e66a41b3
>>>>>> --- /dev/null
>>>>>> +++ b/include/linux/amd-pmf-io.h
>>>>>> @@ -0,0 +1,35 @@
>>>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>>>> +/*
>>>>>> + * AMD Platform Management Framework Interface
>>>>>> + *
>>>>>> + * Copyright (c) 2023, Advanced Micro Devices, Inc.
>>>>>> + * All Rights Reserved.
>>>>>> + *
>>>>>> + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>>>>> + */
>>>>>> +
>>>>>> +#ifndef AMD_PMF_IO_H
>>>>>> +#define AMD_PMF_IO_H
>>>>>> +
>>>>>> +#include <acpi/video.h>
>>>>>> +#include <drm/drm_connector.h>
>>>>>> +#include <linux/backlight.h>
>>>>>> +#include <linux/thermal.h>
>>>>>> +
>>>>>> +#define MAX_SUPPORTED 4
>>>>>> +
>>>>>> +/* amdgpu */
>>>>> Document the structure properly with kerneldoc instead of an
>>>>> unhelpful
>>>>> comment like above :-). Please also check if you add any other
>>>>> structs
>>>>> into kernel-wide headers that you didn't document yet. Or fields
>>>>> into
>>>>> existing structs.
>>>>>
>>>>>> +struct amd_gpu_pmf_data {
>>>>>> +    struct pci_dev *gpu_dev;
>>>>>> +    struct backlight_device *raw_bd;
>>>>>> +    struct thermal_cooling_device *cooling_dev;
>>>>>> +    enum drm_connector_status con_status[MAX_SUPPORTED];
>>>>>> +    int display_count;
>>>>>> +    int connector_type[MAX_SUPPORTED];
>>>>>> +    bool gpu_dev_en;
>>>>>> +};
>>>>>> +
>>>>>> +int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
>>>>>> +int amd_pmf_gpu_init(struct amd_gpu_pmf_data *pmf);
>>>>>> +void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf);
>>>>>> +#endif
>>>>>>
>>>
>>
> 

^ permalink raw reply

* Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
From: Christian König @ 2023-10-18 16:07 UTC (permalink / raw)
  To: Mario Limonciello, Shyam Sundar S K, Ilpo Järvinen
  Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
	benjamin.tissoires, alexander.deucher, Xinhui.Pan, airlied,
	daniel, Patil.Reddy, platform-driver-x86, linux-input, amd-gfx,
	dri-devel
In-Reply-To: <238f915f-b95f-4d85-ad67-66781f53e75d@amd.com>

Am 18.10.23 um 17:47 schrieb Mario Limonciello:
> On 10/18/2023 08:40, Christian König wrote:
>>
>>
>> Am 18.10.23 um 11:28 schrieb Shyam Sundar S K:
>>>
>>> On 10/18/2023 2:50 PM, Ilpo Järvinen wrote:
>>>> On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
>>>>
>>>>> In order to provide GPU inputs to TA for the Smart PC solution to 
>>>>> work, we
>>>>> need to have interface between the PMF driver and the AMDGPU driver.
>>>>>
>>>>> Add the initial code path for get interface from AMDGPU.
>>>>>
>>>>> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
>>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/Makefile     |   2 +
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h     |   1 +
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 138 
>>>>> ++++++++++++++++++++++++
>>>>>   drivers/platform/x86/amd/pmf/Kconfig    |   1 +
>>>>>   drivers/platform/x86/amd/pmf/core.c     |   1 +
>>>>>   drivers/platform/x86/amd/pmf/pmf.h      |   3 +
>>>>>   drivers/platform/x86/amd/pmf/spc.c      |  13 +++
>>>>>   drivers/platform/x86/amd/pmf/tee-if.c   |  26 +++++
>>>>>   include/linux/amd-pmf-io.h              |  35 ++++++
>>>>>   9 files changed, 220 insertions(+)
>>>>>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>>>   create mode 100644 include/linux/amd-pmf-io.h
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
>>>>> b/drivers/gpu/drm/amd/amdgpu/Makefile
>>>>> index 384b798a9bad..7fafccefbd7a 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
>>>>> @@ -86,6 +86,8 @@ amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
>>>>>   amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
>>>>> +amdgpu-$(CONFIG_AMD_PMF) += amdgpu_pmf.o
>>>>> +
>>>>>   # add asic specific block
>>>>>   amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o \
>>>>>       dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> index a79d53bdbe13..26ffa1b4fe57 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>>> @@ -50,6 +50,7 @@
>>>>>   #include <linux/hashtable.h>
>>>>>   #include <linux/dma-fence.h>
>>>>>   #include <linux/pci.h>
>>>>> +#include <linux/amd-pmf-io.h>
>>>>>   #include <drm/ttm/ttm_bo.h>
>>>>>   #include <drm/ttm/ttm_placement.h>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>>> new file mode 100644
>>>>> index 000000000000..ac981848df50
>>>>> --- /dev/null
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>>> @@ -0,0 +1,138 @@
>>>>> +/*
>>>>> + * Copyright 2023 Advanced Micro Devices, Inc.
>>>>> + *
>>>>> + * Permission is hereby granted, free of charge, to any person 
>>>>> obtaining a
>>>>> + * copy of this software and associated documentation files (the 
>>>>> "Software"),
>>>>> + * to deal in the Software without restriction, including without 
>>>>> limitation
>>>>> + * the rights to use, copy, modify, merge, publish, distribute, 
>>>>> sublicense,
>>>>> + * and/or sell copies of the Software, and to permit persons to 
>>>>> whom the
>>>>> + * Software is furnished to do so, subject to the following 
>>>>> conditions:
>>>>> + *
>>>>> + * The above copyright notice and this permission notice shall be 
>>>>> included in
>>>>> + * all copies or substantial portions of the Software.
>>>>> + *
>>>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY 
>>>>> KIND, EXPRESS OR
>>>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>>>>> MERCHANTABILITY,
>>>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO 
>>>>> EVENT SHALL
>>>>> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, 
>>>>> DAMAGES OR
>>>>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR 
>>>>> OTHERWISE,
>>>>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE 
>>>>> USE OR
>>>>> + * OTHER DEALINGS IN THE SOFTWARE.
>>>> This is MIT, right? Add the required SPDX-License-Identifier line 
>>>> for it
>>>> at the top of the file, thank you.
>>>>
>>> all the files in drivers/gpu/drm/amd/amdgpu/* carry the same license
>>> text. So, have retained it to maintain uniformity.
>>
>> Please add the SPDX License Identifier for any file you add.
>>
>> Apart from that the whole approach of attaching this directly to 
>> amdgpu looks extremely questionable to me.
>>
>
> What's the long term outlook for things that are needed directly from 
> amdgpu?  Is there going to be more besides the backlight and the 
> display count/type?

Yeah, that goes into the same direction as my concern.

>
> At least for the display count I suppose one way that it could be 
> "decoupled" from amdgpu is to use drm_for_each_connector_iter to 
> iterate all the connectors and then count the connected ones.

And what if the number of connected displays change? How is amdgpu 
supposed to signal events like that?

This whole solution doesn't looks well thought through.

Regards,
Christian.

>
>> Regards,
>> Christian.
>>
>>>
>>>>> + *
>>>>> + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>>>> + *
>>>>> + */
>>>> Remove the extra empty line at the end of the comment.
>>>>
>>> I just took the standard template for all the gpu files. Is that OK to
>>> retain the blank line?
>>>
>>> If not, I can remove it in the next version.
>>>
>>> Rest all remarks I will address.
>>>
>>> Thanks,
>>> Shyam
>>>
>>>>> +
>>>>> +#include <linux/backlight.h>
>>>>> +#include "amdgpu.h"
>>>>> +
>>>>> +int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
>>>>> +{
>>>>> +    struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
>>>>> +    struct drm_mode_config *mode_config = &drm_dev->mode_config;
>>>>> +    struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>>>> +    struct drm_connector_list_iter iter;
>>>>> +    struct drm_connector *connector;
>>>>> +    int i = 0;
>>>>> +
>>>>> +    /* Reset the count to zero */
>>>>> +    pmf->display_count = 0;
>>>>> +    if (!(adev->flags & AMD_IS_APU)) {
>>>>> +        DRM_ERROR("PMF-AMDGPU interface not supported\n");
>>>>> +        return -ENODEV;
>>>>> +    }
>>>>> +
>>>>> +    mutex_lock(&mode_config->mutex);
>>>>> +    drm_connector_list_iter_begin(drm_dev, &iter);
>>>>> +    drm_for_each_connector_iter(connector, &iter) {
>>>>> +        if (connector->status == connector_status_connected)
>>>>> +            pmf->display_count++;
>>>>> +        if (connector->status != pmf->con_status[i])
>>>>> +            pmf->con_status[i] = connector->status;
>>>>> +        if (connector->connector_type != pmf->connector_type[i])
>>>>> +            pmf->connector_type[i] = connector->connector_type;
>>>>> +
>>>>> +        i++;
>>>>> +        if (i >= MAX_SUPPORTED)
>>>>> +            break;
>>>>> +    }
>>>>> +    drm_connector_list_iter_end(&iter);
>>>>> +    mutex_unlock(&mode_config->mutex);
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
>>>>> +
>>>>> +static int amd_pmf_gpu_get_cur_state(struct 
>>>>> thermal_cooling_device *cooling_dev,
>>>>> +                     unsigned long *state)
>>>>> +{
>>>>> +    struct backlight_device *bd;
>>>>> +
>>>>> +    if (!acpi_video_backlight_use_native())
>>>>> +        return -ENODEV;
>>>>> +
>>>>> +    bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>>>> +    if (!bd)
>>>>> +        return -ENODEV;
>>>>> +
>>>>> +    *state = backlight_get_brightness(bd);
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static int amd_pmf_gpu_get_max_state(struct 
>>>>> thermal_cooling_device *cooling_dev,
>>>>> +                     unsigned long *state)
>>>>> +{
>>>>> +    struct backlight_device *bd;
>>>>> +
>>>>> +    if (!acpi_video_backlight_use_native())
>>>>> +        return -ENODEV;
>>>>> +
>>>>> +    bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>>>> +    if (!bd)
>>>>> +        return -ENODEV;
>>>>> +
>>>>> +    if (backlight_is_blank(bd))
>>>>> +        *state = 0;
>>>>> +    else
>>>>> +        *state = bd->props.max_brightness;
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static const struct thermal_cooling_device_ops bd_cooling_ops = {
>>>>> +    .get_max_state = amd_pmf_gpu_get_max_state,
>>>>> +    .get_cur_state = amd_pmf_gpu_get_cur_state,
>>>>> +};
>>>>> +
>>>>> +int amd_pmf_gpu_init(struct amd_gpu_pmf_data *pmf)
>>>>> +{
>>>>> +    struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
>>>>> +    struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>>>> +
>>>>> +    if (!(adev->flags & AMD_IS_APU)) {
>>>>> +        DRM_ERROR("PMF-AMDGPU interface not supported\n");
>>>>> +        return -ENODEV;
>>>>> +    }
>>>>> +
>>>>> +    if (!acpi_video_backlight_use_native())
>>>>> +        return -ENODEV;
>>>>> +
>>>>> +    pmf->raw_bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>>>> +    if (!pmf->raw_bd)
>>>>> +        return -ENODEV;
>>>>> +
>>>>> +    pmf->cooling_dev = thermal_cooling_device_register("pmf_gpu_bd",
>>>>> +                               pmf, &bd_cooling_ops);
>>>>> +    if (IS_ERR(pmf->cooling_dev))
>>>>> +        return -ENODEV;
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(amd_pmf_gpu_init);
>>>>> +
>>>>> +void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf)
>>>>> +{
>>>>> + thermal_cooling_device_unregister(pmf->cooling_dev);
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(amd_pmf_gpu_deinit);
>>>>> diff --git a/drivers/platform/x86/amd/pmf/Kconfig 
>>>>> b/drivers/platform/x86/amd/pmf/Kconfig
>>>>> index f246252bddd8..7f430de7af44 100644
>>>>> --- a/drivers/platform/x86/amd/pmf/Kconfig
>>>>> +++ b/drivers/platform/x86/amd/pmf/Kconfig
>>>>> @@ -10,6 +10,7 @@ config AMD_PMF
>>>>>       depends on AMD_NB
>>>>>       select ACPI_PLATFORM_PROFILE
>>>>>       depends on TEE && AMDTEE
>>>>> +    depends on DRM_AMDGPU
>>>>>       help
>>>>>         This driver provides support for the AMD Platform 
>>>>> Management Framework.
>>>>>         The goal is to enhance end user experience by making AMD 
>>>>> PCs smarter,
>>>>> diff --git a/drivers/platform/x86/amd/pmf/core.c 
>>>>> b/drivers/platform/x86/amd/pmf/core.c
>>>>> index 4b8156033fa6..c59ba527ff49 100644
>>>>> --- a/drivers/platform/x86/amd/pmf/core.c
>>>>> +++ b/drivers/platform/x86/amd/pmf/core.c
>>>>> @@ -411,6 +411,7 @@ static int amd_pmf_probe(struct 
>>>>> platform_device *pdev)
>>>>>       }
>>>>>       dev->cpu_id = rdev->device;
>>>>> +    dev->root = rdev;
>>>>>       err = amd_smn_read(0, AMD_PMF_BASE_ADDR_LO, &val);
>>>>>       if (err) {
>>>>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h 
>>>>> b/drivers/platform/x86/amd/pmf/pmf.h
>>>>> index 8712299ad52b..615cd3a31986 100644
>>>>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>>>>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>>>>> @@ -13,6 +13,7 @@
>>>>>   #include <linux/acpi.h>
>>>>>   #include <linux/platform_profile.h>
>>>>> +#include <linux/amd-pmf-io.h>
>>>>>   #define POLICY_BUF_MAX_SZ        0x4b000
>>>>>   #define POLICY_SIGN_COOKIE        0x31535024
>>>>> @@ -228,9 +229,11 @@ struct amd_pmf_dev {
>>>>>       void *shbuf;
>>>>>       struct delayed_work pb_work;
>>>>>       struct pmf_action_table *prev_data;
>>>>> +    struct amd_gpu_pmf_data gfx_data;
>>>>>       u64 policy_addr;
>>>>>       void *policy_base;
>>>>>       bool smart_pc_enabled;
>>>>> +    struct pci_dev *root;
>>>>>   };
>>>>>   struct apmf_sps_prop_granular {
>>>>> diff --git a/drivers/platform/x86/amd/pmf/spc.c 
>>>>> b/drivers/platform/x86/amd/pmf/spc.c
>>>>> index 512e0c66efdc..cf4962ef97c2 100644
>>>>> --- a/drivers/platform/x86/amd/pmf/spc.c
>>>>> +++ b/drivers/platform/x86/amd/pmf/spc.c
>>>>> @@ -44,6 +44,10 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev 
>>>>> *dev, struct ta_pmf_enact_table *
>>>>>       dev_dbg(dev->dev, "Max C0 Residency : %u\n", 
>>>>> in->ev_info.max_c0residency);
>>>>>       dev_dbg(dev->dev, "GFX Busy : %u\n", in->ev_info.gfx_busy);
>>>>>       dev_dbg(dev->dev, "Connected Display Count : %u\n", 
>>>>> in->ev_info.monitor_count);
>>>>> +    dev_dbg(dev->dev, "Primary Display Type : %s\n",
>>>>> + drm_get_connector_type_name(in->ev_info.display_type));
>>>>> +    dev_dbg(dev->dev, "Primary Display State : %s\n", 
>>>>> in->ev_info.display_state ?
>>>>> +            "Connected" : "disconnected/unknown");
>>>>>       dev_dbg(dev->dev, "LID State : %s\n", in->ev_info.lid_state 
>>>>> ? "Close" : "Open");
>>>>>       dev_dbg(dev->dev, "==== TA inputs END ====\n");
>>>>>   }
>>>>> @@ -146,6 +150,14 @@ static int amd_pmf_get_slider_info(struct 
>>>>> amd_pmf_dev *dev, struct ta_pmf_enact_
>>>>>       return 0;
>>>>>   }
>>>>> +static void amd_pmf_get_gpu_info(struct amd_pmf_dev *dev, struct 
>>>>> ta_pmf_enact_table *in)
>>>>> +{
>>>>> +    amd_pmf_get_gfx_data(&dev->gfx_data);
>>>>> +    in->ev_info.monitor_count = dev->gfx_data.display_count;
>>>>> +    in->ev_info.display_type = dev->gfx_data.connector_type[0];
>>>>> +    in->ev_info.display_state = dev->gfx_data.con_status[0];
>>>>> +}
>>>>> +
>>>>>   void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct 
>>>>> ta_pmf_enact_table *in)
>>>>>   {
>>>>>       /* TA side lid open is 1 and close is 0, hence the ! here */
>>>>> @@ -154,4 +166,5 @@ void amd_pmf_populate_ta_inputs(struct 
>>>>> amd_pmf_dev *dev, struct ta_pmf_enact_tab
>>>>>       amd_pmf_get_smu_info(dev, in);
>>>>>       amd_pmf_get_battery_info(dev, in);
>>>>>       amd_pmf_get_slider_info(dev, in);
>>>>> +    amd_pmf_get_gpu_info(dev, in);
>>>>>   }
>>>>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c 
>>>>> b/drivers/platform/x86/amd/pmf/tee-if.c
>>>>> index 2f5fb8623c20..956e66b78605 100644
>>>>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>>>>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>>>>> @@ -9,6 +9,7 @@
>>>>>    */
>>>>>   #include <linux/debugfs.h>
>>>>> +#include <linux/pci.h>
>>>>>   #include <linux/tee_drv.h>
>>>>>   #include <linux/uuid.h>
>>>>>   #include "pmf.h"
>>>>> @@ -357,6 +358,19 @@ static int amd_pmf_get_bios_buffer(struct 
>>>>> amd_pmf_dev *dev)
>>>>>       return amd_pmf_start_policy_engine(dev);
>>>>>   }
>>>>> +static int amd_pmf_get_gpu_handle(struct pci_dev *pdev, void *data)
>>>>> +{
>>>>> +    struct amd_pmf_dev *dev = data;
>>>>> +
>>>>> +    if (pdev->vendor == PCI_VENDOR_ID_ATI && pdev->devfn == 0) {
>>>>> +        /* Found the amdgpu handle from the pci root after 
>>>>> walking through the pci bus */
>>>> If you insist on having this comment, make it a function comment 
>>>> instead
>>>> (with appropriate modifications into the content of it) but I 
>>>> personally
>>>> don't find it that useful so it could be just dropped as well, IMO.
>>>>
>>>>> +        dev->gfx_data.gpu_dev = pdev;
>>>>> +        return 1; /* Stop walking */
>>>>> +    }
>>>>> +
>>>>> +    return 0; /* Continue walking */
>>>>> +}
>>>>> +
>>>>>   static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data 
>>>>> *ver, const void *data)
>>>>>   {
>>>>>       return ver->impl_id == TEE_IMPL_ID_AMDTEE;
>>>>> @@ -446,6 +460,15 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev 
>>>>> *dev)
>>>>>       INIT_DELAYED_WORK(&dev->pb_work, amd_pmf_invoke_cmd);
>>>>>       amd_pmf_set_dram_addr(dev);
>>>>>       amd_pmf_get_bios_buffer(dev);
>>>>> +
>>>>> +    /* Get amdgpu handle */
>>>> Useless comment.
>>>>
>>>>> + pci_walk_bus(dev->root->bus, amd_pmf_get_gpu_handle, dev);
>>>>> +    if (!dev->gfx_data.gpu_dev)
>>>>> +        dev_err(dev->dev, "GPU handle not found!\n");
>>>>> +
>>>>> +    if (!amd_pmf_gpu_init(&dev->gfx_data))
>>>>> +        dev->gfx_data.gpu_dev_en = true;
>>>>> +
>>>>>       dev->prev_data = kzalloc(sizeof(*dev->prev_data), GFP_KERNEL);
>>>>>       if (!dev->prev_data)
>>>>>           return -ENOMEM;
>>>>> @@ -461,5 +484,8 @@ void amd_pmf_deinit_smart_pc(struct 
>>>>> amd_pmf_dev *dev)
>>>>>       kfree(dev->prev_data);
>>>>>       kfree(dev->policy_buf);
>>>>>       cancel_delayed_work_sync(&dev->pb_work);
>>>>> +    if (dev->gfx_data.gpu_dev_en)
>>>>> +        amd_pmf_gpu_deinit(&dev->gfx_data);
>>>>> +    pci_dev_put(dev->gfx_data.gpu_dev);
>>>>>       amd_pmf_tee_deinit(dev);
>>>>>   }
>>>>> diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
>>>>> new file mode 100644
>>>>> index 000000000000..5f79e66a41b3
>>>>> --- /dev/null
>>>>> +++ b/include/linux/amd-pmf-io.h
>>>>> @@ -0,0 +1,35 @@
>>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>>> +/*
>>>>> + * AMD Platform Management Framework Interface
>>>>> + *
>>>>> + * Copyright (c) 2023, Advanced Micro Devices, Inc.
>>>>> + * All Rights Reserved.
>>>>> + *
>>>>> + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>>>> + */
>>>>> +
>>>>> +#ifndef AMD_PMF_IO_H
>>>>> +#define AMD_PMF_IO_H
>>>>> +
>>>>> +#include <acpi/video.h>
>>>>> +#include <drm/drm_connector.h>
>>>>> +#include <linux/backlight.h>
>>>>> +#include <linux/thermal.h>
>>>>> +
>>>>> +#define MAX_SUPPORTED 4
>>>>> +
>>>>> +/* amdgpu */
>>>> Document the structure properly with kerneldoc instead of an unhelpful
>>>> comment like above :-). Please also check if you add any other structs
>>>> into kernel-wide headers that you didn't document yet. Or fields into
>>>> existing structs.
>>>>
>>>>> +struct amd_gpu_pmf_data {
>>>>> +    struct pci_dev *gpu_dev;
>>>>> +    struct backlight_device *raw_bd;
>>>>> +    struct thermal_cooling_device *cooling_dev;
>>>>> +    enum drm_connector_status con_status[MAX_SUPPORTED];
>>>>> +    int display_count;
>>>>> +    int connector_type[MAX_SUPPORTED];
>>>>> +    bool gpu_dev_en;
>>>>> +};
>>>>> +
>>>>> +int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
>>>>> +int amd_pmf_gpu_init(struct amd_gpu_pmf_data *pmf);
>>>>> +void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf);
>>>>> +#endif
>>>>>
>>
>


^ permalink raw reply

* Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
From: Mario Limonciello @ 2023-10-18 15:47 UTC (permalink / raw)
  To: Christian König, Shyam Sundar S K, Ilpo Järvinen
  Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
	benjamin.tissoires, alexander.deucher, Xinhui.Pan, airlied,
	daniel, Patil.Reddy, platform-driver-x86, linux-input, amd-gfx,
	dri-devel
In-Reply-To: <92bba3b3-a3f9-4fab-86c7-4d0ef4c23fcb@amd.com>

On 10/18/2023 08:40, Christian König wrote:
> 
> 
> Am 18.10.23 um 11:28 schrieb Shyam Sundar S K:
>>
>> On 10/18/2023 2:50 PM, Ilpo Järvinen wrote:
>>> On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
>>>
>>>> In order to provide GPU inputs to TA for the Smart PC solution to 
>>>> work, we
>>>> need to have interface between the PMF driver and the AMDGPU driver.
>>>>
>>>> Add the initial code path for get interface from AMDGPU.
>>>>
>>>> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/Makefile     |   2 +
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h     |   1 +
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 138 
>>>> ++++++++++++++++++++++++
>>>>   drivers/platform/x86/amd/pmf/Kconfig    |   1 +
>>>>   drivers/platform/x86/amd/pmf/core.c     |   1 +
>>>>   drivers/platform/x86/amd/pmf/pmf.h      |   3 +
>>>>   drivers/platform/x86/amd/pmf/spc.c      |  13 +++
>>>>   drivers/platform/x86/amd/pmf/tee-if.c   |  26 +++++
>>>>   include/linux/amd-pmf-io.h              |  35 ++++++
>>>>   9 files changed, 220 insertions(+)
>>>>   create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>>   create mode 100644 include/linux/amd-pmf-io.h
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
>>>> b/drivers/gpu/drm/amd/amdgpu/Makefile
>>>> index 384b798a9bad..7fafccefbd7a 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
>>>> @@ -86,6 +86,8 @@ amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
>>>>   amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
>>>> +amdgpu-$(CONFIG_AMD_PMF) += amdgpu_pmf.o
>>>> +
>>>>   # add asic specific block
>>>>   amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o \
>>>>       dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> index a79d53bdbe13..26ffa1b4fe57 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>>> @@ -50,6 +50,7 @@
>>>>   #include <linux/hashtable.h>
>>>>   #include <linux/dma-fence.h>
>>>>   #include <linux/pci.h>
>>>> +#include <linux/amd-pmf-io.h>
>>>>   #include <drm/ttm/ttm_bo.h>
>>>>   #include <drm/ttm/ttm_placement.h>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>> new file mode 100644
>>>> index 000000000000..ac981848df50
>>>> --- /dev/null
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>>>> @@ -0,0 +1,138 @@
>>>> +/*
>>>> + * Copyright 2023 Advanced Micro Devices, Inc.
>>>> + *
>>>> + * Permission is hereby granted, free of charge, to any person 
>>>> obtaining a
>>>> + * copy of this software and associated documentation files (the 
>>>> "Software"),
>>>> + * to deal in the Software without restriction, including without 
>>>> limitation
>>>> + * the rights to use, copy, modify, merge, publish, distribute, 
>>>> sublicense,
>>>> + * and/or sell copies of the Software, and to permit persons to 
>>>> whom the
>>>> + * Software is furnished to do so, subject to the following 
>>>> conditions:
>>>> + *
>>>> + * The above copyright notice and this permission notice shall be 
>>>> included in
>>>> + * all copies or substantial portions of the Software.
>>>> + *
>>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>>>> EXPRESS OR
>>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>>>> MERCHANTABILITY,
>>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO 
>>>> EVENT SHALL
>>>> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, 
>>>> DAMAGES OR
>>>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR 
>>>> OTHERWISE,
>>>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE 
>>>> USE OR
>>>> + * OTHER DEALINGS IN THE SOFTWARE.
>>> This is MIT, right? Add the required SPDX-License-Identifier line for it
>>> at the top of the file, thank you.
>>>
>> all the files in drivers/gpu/drm/amd/amdgpu/* carry the same license
>> text. So, have retained it to maintain uniformity.
> 
> Please add the SPDX License Identifier for any file you add.
> 
> Apart from that the whole approach of attaching this directly to amdgpu 
> looks extremely questionable to me.
> 

What's the long term outlook for things that are needed directly from 
amdgpu?  Is there going to be more besides the backlight and the display 
count/type?

At least for the display count I suppose one way that it could be 
"decoupled" from amdgpu is to use drm_for_each_connector_iter to iterate 
all the connectors and then count the connected ones.

> Regards,
> Christian.
> 
>>
>>>> + *
>>>> + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>>> + *
>>>> + */
>>> Remove the extra empty line at the end of the comment.
>>>
>> I just took the standard template for all the gpu files. Is that OK to
>> retain the blank line?
>>
>> If not, I can remove it in the next version.
>>
>> Rest all remarks I will address.
>>
>> Thanks,
>> Shyam
>>
>>>> +
>>>> +#include <linux/backlight.h>
>>>> +#include "amdgpu.h"
>>>> +
>>>> +int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
>>>> +{
>>>> +    struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
>>>> +    struct drm_mode_config *mode_config = &drm_dev->mode_config;
>>>> +    struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>>> +    struct drm_connector_list_iter iter;
>>>> +    struct drm_connector *connector;
>>>> +    int i = 0;
>>>> +
>>>> +    /* Reset the count to zero */
>>>> +    pmf->display_count = 0;
>>>> +    if (!(adev->flags & AMD_IS_APU)) {
>>>> +        DRM_ERROR("PMF-AMDGPU interface not supported\n");
>>>> +        return -ENODEV;
>>>> +    }
>>>> +
>>>> +    mutex_lock(&mode_config->mutex);
>>>> +    drm_connector_list_iter_begin(drm_dev, &iter);
>>>> +    drm_for_each_connector_iter(connector, &iter) {
>>>> +        if (connector->status == connector_status_connected)
>>>> +            pmf->display_count++;
>>>> +        if (connector->status != pmf->con_status[i])
>>>> +            pmf->con_status[i] = connector->status;
>>>> +        if (connector->connector_type != pmf->connector_type[i])
>>>> +            pmf->connector_type[i] = connector->connector_type;
>>>> +
>>>> +        i++;
>>>> +        if (i >= MAX_SUPPORTED)
>>>> +            break;
>>>> +    }
>>>> +    drm_connector_list_iter_end(&iter);
>>>> +    mutex_unlock(&mode_config->mutex);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
>>>> +
>>>> +static int amd_pmf_gpu_get_cur_state(struct thermal_cooling_device 
>>>> *cooling_dev,
>>>> +                     unsigned long *state)
>>>> +{
>>>> +    struct backlight_device *bd;
>>>> +
>>>> +    if (!acpi_video_backlight_use_native())
>>>> +        return -ENODEV;
>>>> +
>>>> +    bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>>> +    if (!bd)
>>>> +        return -ENODEV;
>>>> +
>>>> +    *state = backlight_get_brightness(bd);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int amd_pmf_gpu_get_max_state(struct thermal_cooling_device 
>>>> *cooling_dev,
>>>> +                     unsigned long *state)
>>>> +{
>>>> +    struct backlight_device *bd;
>>>> +
>>>> +    if (!acpi_video_backlight_use_native())
>>>> +        return -ENODEV;
>>>> +
>>>> +    bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>>> +    if (!bd)
>>>> +        return -ENODEV;
>>>> +
>>>> +    if (backlight_is_blank(bd))
>>>> +        *state = 0;
>>>> +    else
>>>> +        *state = bd->props.max_brightness;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static const struct thermal_cooling_device_ops bd_cooling_ops = {
>>>> +    .get_max_state = amd_pmf_gpu_get_max_state,
>>>> +    .get_cur_state = amd_pmf_gpu_get_cur_state,
>>>> +};
>>>> +
>>>> +int amd_pmf_gpu_init(struct amd_gpu_pmf_data *pmf)
>>>> +{
>>>> +    struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
>>>> +    struct amdgpu_device *adev = drm_to_adev(drm_dev);
>>>> +
>>>> +    if (!(adev->flags & AMD_IS_APU)) {
>>>> +        DRM_ERROR("PMF-AMDGPU interface not supported\n");
>>>> +        return -ENODEV;
>>>> +    }
>>>> +
>>>> +    if (!acpi_video_backlight_use_native())
>>>> +        return -ENODEV;
>>>> +
>>>> +    pmf->raw_bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>>>> +    if (!pmf->raw_bd)
>>>> +        return -ENODEV;
>>>> +
>>>> +    pmf->cooling_dev = thermal_cooling_device_register("pmf_gpu_bd",
>>>> +                               pmf, &bd_cooling_ops);
>>>> +    if (IS_ERR(pmf->cooling_dev))
>>>> +        return -ENODEV;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(amd_pmf_gpu_init);
>>>> +
>>>> +void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf)
>>>> +{
>>>> +    thermal_cooling_device_unregister(pmf->cooling_dev);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(amd_pmf_gpu_deinit);
>>>> diff --git a/drivers/platform/x86/amd/pmf/Kconfig 
>>>> b/drivers/platform/x86/amd/pmf/Kconfig
>>>> index f246252bddd8..7f430de7af44 100644
>>>> --- a/drivers/platform/x86/amd/pmf/Kconfig
>>>> +++ b/drivers/platform/x86/amd/pmf/Kconfig
>>>> @@ -10,6 +10,7 @@ config AMD_PMF
>>>>       depends on AMD_NB
>>>>       select ACPI_PLATFORM_PROFILE
>>>>       depends on TEE && AMDTEE
>>>> +    depends on DRM_AMDGPU
>>>>       help
>>>>         This driver provides support for the AMD Platform Management 
>>>> Framework.
>>>>         The goal is to enhance end user experience by making AMD PCs 
>>>> smarter,
>>>> diff --git a/drivers/platform/x86/amd/pmf/core.c 
>>>> b/drivers/platform/x86/amd/pmf/core.c
>>>> index 4b8156033fa6..c59ba527ff49 100644
>>>> --- a/drivers/platform/x86/amd/pmf/core.c
>>>> +++ b/drivers/platform/x86/amd/pmf/core.c
>>>> @@ -411,6 +411,7 @@ static int amd_pmf_probe(struct platform_device 
>>>> *pdev)
>>>>       }
>>>>       dev->cpu_id = rdev->device;
>>>> +    dev->root = rdev;
>>>>       err = amd_smn_read(0, AMD_PMF_BASE_ADDR_LO, &val);
>>>>       if (err) {
>>>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h 
>>>> b/drivers/platform/x86/amd/pmf/pmf.h
>>>> index 8712299ad52b..615cd3a31986 100644
>>>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>>>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>>>> @@ -13,6 +13,7 @@
>>>>   #include <linux/acpi.h>
>>>>   #include <linux/platform_profile.h>
>>>> +#include <linux/amd-pmf-io.h>
>>>>   #define POLICY_BUF_MAX_SZ        0x4b000
>>>>   #define POLICY_SIGN_COOKIE        0x31535024
>>>> @@ -228,9 +229,11 @@ struct amd_pmf_dev {
>>>>       void *shbuf;
>>>>       struct delayed_work pb_work;
>>>>       struct pmf_action_table *prev_data;
>>>> +    struct amd_gpu_pmf_data gfx_data;
>>>>       u64 policy_addr;
>>>>       void *policy_base;
>>>>       bool smart_pc_enabled;
>>>> +    struct pci_dev *root;
>>>>   };
>>>>   struct apmf_sps_prop_granular {
>>>> diff --git a/drivers/platform/x86/amd/pmf/spc.c 
>>>> b/drivers/platform/x86/amd/pmf/spc.c
>>>> index 512e0c66efdc..cf4962ef97c2 100644
>>>> --- a/drivers/platform/x86/amd/pmf/spc.c
>>>> +++ b/drivers/platform/x86/amd/pmf/spc.c
>>>> @@ -44,6 +44,10 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev 
>>>> *dev, struct ta_pmf_enact_table *
>>>>       dev_dbg(dev->dev, "Max C0 Residency : %u\n", 
>>>> in->ev_info.max_c0residency);
>>>>       dev_dbg(dev->dev, "GFX Busy : %u\n", in->ev_info.gfx_busy);
>>>>       dev_dbg(dev->dev, "Connected Display Count : %u\n", 
>>>> in->ev_info.monitor_count);
>>>> +    dev_dbg(dev->dev, "Primary Display Type : %s\n",
>>>> +        drm_get_connector_type_name(in->ev_info.display_type));
>>>> +    dev_dbg(dev->dev, "Primary Display State : %s\n", 
>>>> in->ev_info.display_state ?
>>>> +            "Connected" : "disconnected/unknown");
>>>>       dev_dbg(dev->dev, "LID State : %s\n", in->ev_info.lid_state ? 
>>>> "Close" : "Open");
>>>>       dev_dbg(dev->dev, "==== TA inputs END ====\n");
>>>>   }
>>>> @@ -146,6 +150,14 @@ static int amd_pmf_get_slider_info(struct 
>>>> amd_pmf_dev *dev, struct ta_pmf_enact_
>>>>       return 0;
>>>>   }
>>>> +static void amd_pmf_get_gpu_info(struct amd_pmf_dev *dev, struct 
>>>> ta_pmf_enact_table *in)
>>>> +{
>>>> +    amd_pmf_get_gfx_data(&dev->gfx_data);
>>>> +    in->ev_info.monitor_count = dev->gfx_data.display_count;
>>>> +    in->ev_info.display_type = dev->gfx_data.connector_type[0];
>>>> +    in->ev_info.display_state = dev->gfx_data.con_status[0];
>>>> +}
>>>> +
>>>>   void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct 
>>>> ta_pmf_enact_table *in)
>>>>   {
>>>>       /* TA side lid open is 1 and close is 0, hence the ! here */
>>>> @@ -154,4 +166,5 @@ void amd_pmf_populate_ta_inputs(struct 
>>>> amd_pmf_dev *dev, struct ta_pmf_enact_tab
>>>>       amd_pmf_get_smu_info(dev, in);
>>>>       amd_pmf_get_battery_info(dev, in);
>>>>       amd_pmf_get_slider_info(dev, in);
>>>> +    amd_pmf_get_gpu_info(dev, in);
>>>>   }
>>>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c 
>>>> b/drivers/platform/x86/amd/pmf/tee-if.c
>>>> index 2f5fb8623c20..956e66b78605 100644
>>>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>>>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>>>> @@ -9,6 +9,7 @@
>>>>    */
>>>>   #include <linux/debugfs.h>
>>>> +#include <linux/pci.h>
>>>>   #include <linux/tee_drv.h>
>>>>   #include <linux/uuid.h>
>>>>   #include "pmf.h"
>>>> @@ -357,6 +358,19 @@ static int amd_pmf_get_bios_buffer(struct 
>>>> amd_pmf_dev *dev)
>>>>       return amd_pmf_start_policy_engine(dev);
>>>>   }
>>>> +static int amd_pmf_get_gpu_handle(struct pci_dev *pdev, void *data)
>>>> +{
>>>> +    struct amd_pmf_dev *dev = data;
>>>> +
>>>> +    if (pdev->vendor == PCI_VENDOR_ID_ATI && pdev->devfn == 0) {
>>>> +        /* Found the amdgpu handle from the pci root after walking 
>>>> through the pci bus */
>>> If you insist on having this comment, make it a function comment instead
>>> (with appropriate modifications into the content of it) but I personally
>>> don't find it that useful so it could be just dropped as well, IMO.
>>>
>>>> +        dev->gfx_data.gpu_dev = pdev;
>>>> +        return 1; /* Stop walking */
>>>> +    }
>>>> +
>>>> +    return 0; /* Continue walking */
>>>> +}
>>>> +
>>>>   static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data 
>>>> *ver, const void *data)
>>>>   {
>>>>       return ver->impl_id == TEE_IMPL_ID_AMDTEE;
>>>> @@ -446,6 +460,15 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
>>>>       INIT_DELAYED_WORK(&dev->pb_work, amd_pmf_invoke_cmd);
>>>>       amd_pmf_set_dram_addr(dev);
>>>>       amd_pmf_get_bios_buffer(dev);
>>>> +
>>>> +    /* Get amdgpu handle */
>>> Useless comment.
>>>
>>>> +    pci_walk_bus(dev->root->bus, amd_pmf_get_gpu_handle, dev);
>>>> +    if (!dev->gfx_data.gpu_dev)
>>>> +        dev_err(dev->dev, "GPU handle not found!\n");
>>>> +
>>>> +    if (!amd_pmf_gpu_init(&dev->gfx_data))
>>>> +        dev->gfx_data.gpu_dev_en = true;
>>>> +
>>>>       dev->prev_data = kzalloc(sizeof(*dev->prev_data), GFP_KERNEL);
>>>>       if (!dev->prev_data)
>>>>           return -ENOMEM;
>>>> @@ -461,5 +484,8 @@ void amd_pmf_deinit_smart_pc(struct amd_pmf_dev 
>>>> *dev)
>>>>       kfree(dev->prev_data);
>>>>       kfree(dev->policy_buf);
>>>>       cancel_delayed_work_sync(&dev->pb_work);
>>>> +    if (dev->gfx_data.gpu_dev_en)
>>>> +        amd_pmf_gpu_deinit(&dev->gfx_data);
>>>> +    pci_dev_put(dev->gfx_data.gpu_dev);
>>>>       amd_pmf_tee_deinit(dev);
>>>>   }
>>>> diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
>>>> new file mode 100644
>>>> index 000000000000..5f79e66a41b3
>>>> --- /dev/null
>>>> +++ b/include/linux/amd-pmf-io.h
>>>> @@ -0,0 +1,35 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>> +/*
>>>> + * AMD Platform Management Framework Interface
>>>> + *
>>>> + * Copyright (c) 2023, Advanced Micro Devices, Inc.
>>>> + * All Rights Reserved.
>>>> + *
>>>> + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>>> + */
>>>> +
>>>> +#ifndef AMD_PMF_IO_H
>>>> +#define AMD_PMF_IO_H
>>>> +
>>>> +#include <acpi/video.h>
>>>> +#include <drm/drm_connector.h>
>>>> +#include <linux/backlight.h>
>>>> +#include <linux/thermal.h>
>>>> +
>>>> +#define MAX_SUPPORTED 4
>>>> +
>>>> +/* amdgpu */
>>> Document the structure properly with kerneldoc instead of an unhelpful
>>> comment like above :-). Please also check if you add any other structs
>>> into kernel-wide headers that you didn't document yet. Or fields into
>>> existing structs.
>>>
>>>> +struct amd_gpu_pmf_data {
>>>> +    struct pci_dev *gpu_dev;
>>>> +    struct backlight_device *raw_bd;
>>>> +    struct thermal_cooling_device *cooling_dev;
>>>> +    enum drm_connector_status con_status[MAX_SUPPORTED];
>>>> +    int display_count;
>>>> +    int connector_type[MAX_SUPPORTED];
>>>> +    bool gpu_dev_en;
>>>> +};
>>>> +
>>>> +int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
>>>> +int amd_pmf_gpu_init(struct amd_gpu_pmf_data *pmf);
>>>> +void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf);
>>>> +#endif
>>>>
> 


^ permalink raw reply

* [PATCH AUTOSEL 4.14 2/7] Input: i8042 - add Fujitsu Lifebook E5411 to i8042 quirk table
From: Sasha Levin @ 2023-10-18 14:16 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Szilard Fabian, Dmitry Torokhov, Sasha Levin, linux-input
In-Reply-To: <20231018141613.1335848-1-sashal@kernel.org>

From: Szilard Fabian <szfabian@bluemarch.art>

[ Upstream commit 80f39e1c27ba9e5a1ea7e68e21c569c9d8e46062 ]

In the initial boot stage the integrated keyboard of Fujitsu Lifebook E5411
refuses to work and it's not possible to type for example a dm-crypt
passphrase without the help of an external keyboard.

i8042.nomux kernel parameter resolves this issue but using that a PS/2
mouse is detected. This input device is unused even when the i2c-hid-acpi
kernel module is blacklisted making the integrated ELAN touchpad
(04F3:308A) not working at all.

Since the integrated touchpad is managed by the i2c_designware input
driver in the Linux kernel and you can't find a PS/2 mouse port on the
computer I think it's safe to not use the PS/2 mouse port at all.

Signed-off-by: Szilard Fabian <szfabian@bluemarch.art>
Link: https://lore.kernel.org/r/20231004011749.101789-1-szfabian@bluemarch.art
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/input/serio/i8042-x86ia64io.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
index c218e107c0c8f..064b20c914e45 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -174,6 +174,14 @@ static const struct dmi_system_id __initconst i8042_dmi_noloop_table[] = {
 			DMI_MATCH(DMI_PRODUCT_VERSION, "M606"),
 		},
 	},
+	{
+		/* Fujitsu Lifebook E5411 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU CLIENT COMPUTING LIMITED"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "LIFEBOOK E5411"),
+		},
+		.driver_data = (void *)(SERIO_QUIRK_NOAUX)
+	},
 	{
 		/* Gigabyte M912 */
 		.matches = {
-- 
2.40.1


^ permalink raw reply related

* [PATCH AUTOSEL 4.14 6/7] Input: xpad - add PXN V900 support
From: Sasha Levin @ 2023-10-18 14:16 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Matthias Berndt, Dmitry Torokhov, Sasha Levin, vi, swyterzone,
	lyude, radon86dev, maxwell.nguyen, matthias.benkmann, linux-input
In-Reply-To: <20231018141613.1335848-1-sashal@kernel.org>

From: Matthias Berndt <matthias_berndt@gmx.de>

[ Upstream commit a65cd7ef5a864bdbbe037267c327786b7759d4c6 ]

Add VID and PID to the xpad_device table to allow driver to use the PXN
V900 steering wheel, which is XTYPE_XBOX360 compatible in xinput mode.

Signed-off-by: Matthias Berndt <matthias_berndt@gmx.de>
Link: https://lore.kernel.org/r/4932699.31r3eYUQgx@fedora
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/input/joystick/xpad.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 9f503c1e74b42..1dbe303354589 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -269,6 +269,7 @@ static const struct xpad_device {
 	{ 0x1038, 0x1430, "SteelSeries Stratus Duo", 0, XTYPE_XBOX360 },
 	{ 0x1038, 0x1431, "SteelSeries Stratus Duo", 0, XTYPE_XBOX360 },
 	{ 0x11c9, 0x55f0, "Nacon GC-100XF", 0, XTYPE_XBOX360 },
+	{ 0x11ff, 0x0511, "PXN V900", 0, XTYPE_XBOX360 },
 	{ 0x1209, 0x2882, "Ardwiino Controller", 0, XTYPE_XBOX360 },
 	{ 0x12ab, 0x0004, "Honey Bee Xbox360 dancepad", MAP_DPAD_TO_BUTTONS, XTYPE_XBOX360 },
 	{ 0x12ab, 0x0301, "PDP AFTERGLOW AX.1", 0, XTYPE_XBOX360 },
@@ -463,6 +464,7 @@ static const struct usb_device_id xpad_table[] = {
 	XPAD_XBOXONE_VENDOR(0x0f0d),		/* Hori Controllers */
 	XPAD_XBOX360_VENDOR(0x1038),		/* SteelSeries Controllers */
 	XPAD_XBOX360_VENDOR(0x11c9),		/* Nacon GC100XF */
+	XPAD_XBOX360_VENDOR(0x11ff),		/* PXN V900 */
 	XPAD_XBOX360_VENDOR(0x1209),		/* Ardwiino Controllers */
 	XPAD_XBOX360_VENDOR(0x12ab),		/* X-Box 360 dance pads */
 	XPAD_XBOX360_VENDOR(0x1430),		/* RedOctane X-Box 360 controllers */
-- 
2.40.1


^ permalink raw reply related

* [PATCH AUTOSEL 5.4 7/7] Input: powermate - fix use-after-free in powermate_config_complete
From: Sasha Levin @ 2023-10-18 14:15 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Javier Carrasco, syzbot+0434ac83f907a1dbdd1e, Dmitry Torokhov,
	Sasha Levin, linux-input
In-Reply-To: <20231018141525.1335533-1-sashal@kernel.org>

From: Javier Carrasco <javier.carrasco.cruz@gmail.com>

[ Upstream commit 5c15c60e7be615f05a45cd905093a54b11f461bc ]

syzbot has found a use-after-free bug [1] in the powermate driver. This
happens when the device is disconnected, which leads to a memory free from
the powermate_device struct.  When an asynchronous control message
completes after the kfree and its callback is invoked, the lock does not
exist anymore and hence the bug.

Use usb_kill_urb() on pm->config to cancel any in-progress requests upon
device disconnection.

[1] https://syzkaller.appspot.com/bug?extid=0434ac83f907a1dbdd1e

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Reported-by: syzbot+0434ac83f907a1dbdd1e@syzkaller.appspotmail.com
Link: https://lore.kernel.org/r/20230916-topic-powermate_use_after_free-v3-1-64412b81a7a2@gmail.com
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/input/misc/powermate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/misc/powermate.c b/drivers/input/misc/powermate.c
index c4e0e1886061f..6b1b95d58e6b5 100644
--- a/drivers/input/misc/powermate.c
+++ b/drivers/input/misc/powermate.c
@@ -425,6 +425,7 @@ static void powermate_disconnect(struct usb_interface *intf)
 		pm->requires_update = 0;
 		usb_kill_urb(pm->irq);
 		input_unregister_device(pm->input);
+		usb_kill_urb(pm->config);
 		usb_free_urb(pm->irq);
 		usb_free_urb(pm->config);
 		powermate_free_buffers(interface_to_usbdev(intf), pm);
-- 
2.40.1


^ permalink raw reply related

* Re: [PATCH v3 01/12] HID: logitech-hidpp: Don't restart IO, instead defer hid_connect() only
From: Benjamin Tissoires @ 2023-10-18 15:17 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Filipe Laíns, Bastien Nocera, Jiri Kosina,
	Benjamin Tissoires, linux-input
In-Reply-To: <20231010102029.111003-2-hdegoede@redhat.com>

Hi Hans,

FWIW, your series looks good, but I came accross a small nitpick here:

On Oct 10 2023, Hans de Goede wrote:
> Restarting IO causes 2 problems:
> 
> 1. Some devices do not like IO being restarted this was addressed in
>    commit 498ba2069035 ("HID: logitech-hidpp: Don't restart communication
>    if not necessary"), but that change has issues of its own and needs to
>    be reverted.
> 
> 2. Restarting IO and specifically calling hid_device_io_stop() causes
>    received packets to be missed, which may cause connect-events to
>    get missed.
> 
> Restarting IO was introduced in commit 91cf9a98ae41 ("HID: logitech-hidpp:
> make .probe usbhid capable") to allow to retrieve the device's name and
> serial number and store these in hdev->name and hdev->uniq before
> connecting any hid subdrivers (hid-input, hidraw) exporting this info
> to userspace.
> 
> But this does not require restarting IO, this merely requires deferring
> calling hid_connect(). Calling hid_hw_start() with a connect-mask of
> 0 makes it skip calling hid_connect(), so hidpp_probe() can simply call
> hid_connect() later without needing to restart IO.
> 
> Remove the stop + restart of IO and instead just call hid_connect() later
> to avoid the issues caused by restarting IO.
> 
> Now that IO is no longer stopped, hid_hw_close() must be called at the end
> of probe() to balance the hid_hw_open() done at the beginning probe().
> 
> This series has been tested on the following devices:
> Logitech Bluetooth Laser Travel Mouse (bluetooth, HID++ 1.0)
> Logitech M720 Triathlon (bluetooth, HID++ 4.5)
> Logitech M720 Triathlon (unifying, HID++ 4.5)
> Logitech K400 Pro (unifying, HID++ 4.1)
> Logitech K270 (eQUAD nano Lite, HID++ 2.0)
> Logitech M185 (eQUAD nano Lite, HID++ 4.5)
> Logitech LX501 keyboard (27 Mhz, HID++ builtin scroll-wheel, HID++ 1.0)
> Logitech M-RAZ105 mouse (27 Mhz, HID++ extra mouse buttons, HID++ 1.0)
> 
> Fixes: 498ba2069035 ("HID: logitech-hidpp: Don't restart communication if not necessary")
> Suggested-by: Benjamin Tissoires <bentiss@kernel.org>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/hid/hid-logitech-hidpp.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index a209d51bd247..aa4f232c4518 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4460,8 +4460,10 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  			 hdev->name);
>  
>  	/*
> -	 * Plain USB connections need to actually call start and open
> -	 * on the transport driver to allow incoming data.
> +	 * First call hid_hw_start(hdev, 0) to allow IO without connecting any
> +	 * hid subdrivers (hid-input, hidraw). This allows retrieving the dev's
> +	 * name and serial number and store these in hdev->name and hdev->uniq,
> +	 * before the hid-input and hidraw drivers expose these to userspace.
>  	 */
>  	ret = hid_hw_start(hdev, will_restart ? 0 : connect_mask);
>  	if (ret) {
> @@ -4519,19 +4521,14 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	flush_work(&hidpp->work);
>  
>  	if (will_restart) {
> -		/* Reset the HID node state */
> -		hid_device_io_stop(hdev);
> -		hid_hw_close(hdev);
> -		hid_hw_stop(hdev);
> -
>  		if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
>  			connect_mask &= ~HID_CONNECT_HIDINPUT;
>  
>  		/* Now export the actual inputs and hidraw nodes to the world */
> -		ret = hid_hw_start(hdev, connect_mask);
> +		ret = hid_connect(hdev, connect_mask);

On plain USB devices, we get a new warning here "io already started".

This is because hid_connect() will call hid_pidff_init() from
drivers/hid/usbhid/hid-pidff.c if connect_mask has the `HID_CONNECT_FF`
bit set.

And hid_pidff_init() blindly calls hid_device_io_start() then
hid_device_io_stop().

It's not a big issue, but still it's a new warning we have to tackly on.

I see 3 possible solutions:
- teach hid_pidff_init() to only start/stop the IOs if it's not already
  done so
- if a device is actually connected through USB, call
  hid_device_io_stop() before hid_connect()
- unconditionally call hid_device_io_stop() before hid_connect()

The latter has my current preference as we won't get biten in the future
if something else decides to change the io state.

However, do you thing it'll be an issue to disable IOs there?
And maybe we should re-enable them after?

If it's fine to simply disable the IOs, we can add an extra patch on top
of this series to fix that warning in the USB case.

As mentioned above, I have tested with the T650, T651 that were likely to
be a problem, and they are working just fine :)

So with that minor fix, we should be able to stage this series.

Thanks again for your hard work

Cheers,
Benjamin

>  		if (ret) {
> -			hid_err(hdev, "%s:hid_hw_start returned error\n", __func__);
> -			goto hid_hw_start_fail;
> +			hid_err(hdev, "%s:hid_connect returned error %d\n", __func__, ret);
> +			goto hid_hw_init_fail;
>  		}
>  	}
>  
> @@ -4543,6 +4540,11 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  				 ret);
>  	}
>  
> +	/*
> +	 * This relies on logi_dj_ll_close() being a no-op so that DJ connection
> +	 * events will still be received.
> +	 */
> +	hid_hw_close(hdev);
>  	return ret;
>  
>  hid_hw_init_fail:
> -- 
> 2.41.0
> 

^ permalink raw reply

* [PATCH AUTOSEL 5.10 02/11] Input: goodix - ensure int GPIO is in input for gpio_count == 1 && gpio_int_idx == 0 case
From: Sasha Levin @ 2023-10-18 14:14 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Hans de Goede, Michael Smith, Dmitry Torokhov, Sasha Levin,
	hadess, linux-input
In-Reply-To: <20231018141455.1335353-1-sashal@kernel.org>

From: Hans de Goede <hdegoede@redhat.com>

[ Upstream commit 423622a90abb243944d1517b9f57db53729e45c4 ]

Add a special case for gpio_count == 1 && gpio_int_idx == 0 to
goodix_add_acpi_gpio_mappings().

It seems that on newer x86/ACPI devices the reset and irq GPIOs are no
longer listed as GPIO resources instead there is only 1 GpioInt resource
and _PS0 does the whole reset sequence for us.

This means that we must call acpi_device_fix_up_power() on these devices
to ensure that the chip is reset before we try to use it.

This part was already fixed in commit 3de93e6ed2df ("Input: goodix - call
acpi_device_fix_up_power() in some cases") by adding a call to
acpi_device_fix_up_power() to the generic "Unexpected ACPI resources"
catch all.

But it turns out that this case on some hw needs some more special
handling. Specifically the firmware may bootup with the IRQ pin in
output mode. The reset sequence from ACPI _PS0 (executed by
acpi_device_fix_up_power()) should put the pin in input mode,
but the GPIO subsystem has cached the direction at bootup, causing
request_irq() to fail due to gpiochip_lock_as_irq() failure:

[    9.119864] Goodix-TS i2c-GDIX1002:00: Unexpected ACPI resources: gpio_count 1, gpio_int_idx 0
[    9.317443] Goodix-TS i2c-GDIX1002:00: ID 911, version: 1060
[    9.321902] input: Goodix Capacitive TouchScreen as /devices/pci0000:00/0000:00:17.0/i2c_designware.4/i2c-5/i2c-GDIX1002:00/input/input8
[    9.327840] gpio gpiochip0: (INT3453:00): gpiochip_lock_as_irq: tried to flag a GPIO set as output for IRQ
[    9.327856] gpio gpiochip0: (INT3453:00): unable to lock HW IRQ 26 for IRQ
[    9.327861] genirq: Failed to request resources for GDIX1002:00 (irq 131) on irqchip intel-gpio
[    9.327912] Goodix-TS i2c-GDIX1002:00: request IRQ failed: -5

Fix this by adding a special case for gpio_count == 1 && gpio_int_idx == 0
which adds an ACPI GPIO lookup table for the int GPIO even though we cannot
use it for reset purposes (as there is no reset GPIO).

Adding the lookup will make the gpiod_int = gpiod_get(..., GPIOD_IN) call
succeed, which will explicitly set the direction to input fixing the issue.

Note this re-uses the acpi_goodix_int_first_gpios[] lookup table, since
there is only 1 GPIO in the ACPI resources the reset entry in that
lookup table will amount to a no-op.

Reported-and-tested-by: Michael Smith <1973.mjsmith@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20231003215144.69527-1-hdegoede@redhat.com
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/input/touchscreen/goodix.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 098115eb80841..53792a1b6ac39 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -820,6 +820,25 @@ static int goodix_add_acpi_gpio_mappings(struct goodix_ts_data *ts)
 		dev_info(dev, "No ACPI GpioInt resource, assuming that the GPIO order is reset, int\n");
 		ts->irq_pin_access_method = IRQ_PIN_ACCESS_ACPI_GPIO;
 		gpio_mapping = acpi_goodix_int_last_gpios;
+	} else if (ts->gpio_count == 1 && ts->gpio_int_idx == 0) {
+		/*
+		 * On newer devices there is only 1 GpioInt resource and _PS0
+		 * does the whole reset sequence for us.
+		 */
+		acpi_device_fix_up_power(ACPI_COMPANION(dev));
+
+		/*
+		 * Before the _PS0 call the int GPIO may have been in output
+		 * mode and the call should have put the int GPIO in input mode,
+		 * but the GPIO subsys cached state may still think it is
+		 * in output mode, causing gpiochip_lock_as_irq() failure.
+		 *
+		 * Add a mapping for the int GPIO to make the
+		 * gpiod_int = gpiod_get(..., GPIOD_IN) call succeed,
+		 * which will explicitly set the direction to input.
+		 */
+		ts->irq_pin_access_method = IRQ_PIN_ACCESS_NONE;
+		gpio_mapping = acpi_goodix_int_first_gpios;
 	} else {
 		dev_warn(dev, "Unexpected ACPI resources: gpio_count %d, gpio_int_idx %d\n",
 			 ts->gpio_count, ts->gpio_int_idx);
-- 
2.40.1


^ permalink raw reply related

* [PATCH AUTOSEL 5.10 01/11] Input: i8042 - add Fujitsu Lifebook E5411 to i8042 quirk table
From: Sasha Levin @ 2023-10-18 14:14 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Szilard Fabian, Dmitry Torokhov, Sasha Levin, wse, hdegoede,
	jdenose, tiwai, linux-input

From: Szilard Fabian <szfabian@bluemarch.art>

[ Upstream commit 80f39e1c27ba9e5a1ea7e68e21c569c9d8e46062 ]

In the initial boot stage the integrated keyboard of Fujitsu Lifebook E5411
refuses to work and it's not possible to type for example a dm-crypt
passphrase without the help of an external keyboard.

i8042.nomux kernel parameter resolves this issue but using that a PS/2
mouse is detected. This input device is unused even when the i2c-hid-acpi
kernel module is blacklisted making the integrated ELAN touchpad
(04F3:308A) not working at all.

Since the integrated touchpad is managed by the i2c_designware input
driver in the Linux kernel and you can't find a PS/2 mouse port on the
computer I think it's safe to not use the PS/2 mouse port at all.

Signed-off-by: Szilard Fabian <szfabian@bluemarch.art>
Link: https://lore.kernel.org/r/20231004011749.101789-1-szfabian@bluemarch.art
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/input/serio/i8042-acpipnpio.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
index 1bd5898abb97d..09528c0a8a34e 100644
--- a/drivers/input/serio/i8042-acpipnpio.h
+++ b/drivers/input/serio/i8042-acpipnpio.h
@@ -609,6 +609,14 @@ static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = {
 		},
 		.driver_data = (void *)(SERIO_QUIRK_NOMUX)
 	},
+	{
+		/* Fujitsu Lifebook E5411 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU CLIENT COMPUTING LIMITED"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "LIFEBOOK E5411"),
+		},
+		.driver_data = (void *)(SERIO_QUIRK_NOAUX)
+	},
 	{
 		/* Gigabyte M912 */
 		.matches = {
-- 
2.40.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox