Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH 1/2] Input: allocate keycode for Fn + R
From: Gergo Koteles @ 2024-02-20  3:39 UTC (permalink / raw)
  To: Ike Panhc, Hans de Goede, Ilpo Järvinen, Dmitry Torokhov,
	Philipp Jungkamp
  Cc: platform-driver-x86, linux-kernel, linux-input, Gergo Koteles
In-Reply-To: <cover.1708399689.git.soyer@irl.hu>

Newer Lenovo Yogas and Legions with 60Hz/90Hz displays send a wmi event
when Fn + R is pressed. This is intended for use to adjust refresh rate.

But there is no silkscreen print on the R key to indicate this,
so add a KEY_FN_R keycode define to use for this key-combo.

Signed-off-by: Gergo Koteles <soyer@irl.hu>
---
 include/uapi/linux/input-event-codes.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 022a520e31fc..ba40af25943a 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -547,6 +547,7 @@
 #define KEY_FN_S		0x1e3
 #define KEY_FN_B		0x1e4
 #define KEY_FN_RIGHT_SHIFT	0x1e5
+#define KEY_FN_R		0x1e6
 
 #define KEY_BRL_DOT1		0x1f1
 #define KEY_BRL_DOT2		0x1f2
-- 
2.43.2


^ permalink raw reply related

* [PATCH 0/2] map Fn + R key on newer Yogas and Legions
From: Gergo Koteles @ 2024-02-20  3:39 UTC (permalink / raw)
  To: Ike Panhc, Hans de Goede, Ilpo Järvinen, Dmitry Torokhov,
	Philipp Jungkamp
  Cc: platform-driver-x86, linux-kernel, linux-input, Gergo Koteles

Hi All,

This patch series adds a new KEY_FN_R input event code and map the
Fn + R key to it in the ideapad-laptop driver.

It affects two WMI keycodes and I couldn't try the 0x0a, but I couldn't 
find any indication that the refresh rate toggle should not be Fn + R.

Regards,
Gergo

Gergo Koteles (2):
  Input: allocate keycode for Fn + R
  platform/x86: ideapad-laptop: map Fn + R key to KEY_FN_R

 drivers/platform/x86/ideapad-laptop.c  | 4 ++--
 include/uapi/linux/input-event-codes.h | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)


base-commit: b401b621758e46812da61fa58a67c3fd8d91de0d
prerequisite-patch-id: 66a1ec71f181c6468a8226430af7ee917f40138a
-- 
2.43.2


^ permalink raw reply

* Re: [PATCH v8 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
From: kernel test robot @ 2024-02-20  3:28 UTC (permalink / raw)
  To: Kamel Bouhara, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Henrik Rydberg, linux-input, linux-kernel,
	devicetree, Marco Felsch, Jeff LaBundy
  Cc: llvm, oe-kbuild-all, catalin.popescu, mark.satterthwaite,
	Thomas Petazzoni, Gregory Clement, bsp-development.geo,
	Kamel Bouhara
In-Reply-To: <20240219101221.129750-4-kamel.bouhara@bootlin.com>

Hi Kamel,

kernel test robot noticed the following build errors:

[auto build test ERROR on dtor-input/next]
[also build test ERROR on dtor-input/for-linus robh/for-next krzk-dt/for-next linus/master v6.8-rc5 next-20240219]
[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/Kamel-Bouhara/dt-bindings-vendor-prefixes-Add-TouchNetix-AS/20240219-181550
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link:    https://lore.kernel.org/r/20240219101221.129750-4-kamel.bouhara%40bootlin.com
patch subject: [PATCH v8 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
config: arm64-randconfig-002-20240220 (https://download.01.org/0day-ci/archive/20240220/202402201157.BKo97uWl-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 36adfec155de366d722f2bac8ff9162289dcf06c)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240220/202402201157.BKo97uWl-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/202402201157.BKo97uWl-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/input/touchscreen/touchnetix_axiom.c:332:18: error: use of undeclared identifier 'slot'; did you mean 'sget'?
     332 |                 target->index, slot, target->present,
         |                                ^~~~
         |                                sget
   include/linux/dev_printk.h:163:47: note: expanded from macro 'dev_dbg'
     163 |                 dev_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__); \
         |                                                             ^
   include/linux/dev_printk.h:129:34: note: expanded from macro 'dev_printk'
     129 |                 _dev_printk(level, dev, fmt, ##__VA_ARGS__);            \
         |                                                ^
   include/linux/fs.h:2272:21: note: 'sget' declared here
    2272 | struct super_block *sget(struct file_system_type *type,
         |                     ^
   1 error generated.


vim +332 drivers/input/touchscreen/touchnetix_axiom.c

   289	
   290	/*
   291	 * Support function to axiom_process_u41_report.
   292	 * Generates input-subsystem events for every target.
   293	 * After calling this function the caller shall issue
   294	 * a Sync to the input sub-system.
   295	 */
   296	static bool axiom_process_u41_report_target(struct axiom_data *ts,
   297						    struct axiom_target_report *target)
   298	{
   299		struct input_dev *input_dev = ts->input_dev;
   300		struct axiom_u41_target *target_prev_state;
   301		enum axiom_target_state current_state;
   302		int id;
   303	
   304		/* Verify the target index */
   305		if (target->index >= AXIOM_U41_MAX_TARGETS) {
   306			dev_err(ts->dev, "Invalid target index! %u\n", target->index);
   307			return false;
   308		}
   309	
   310		target_prev_state = &ts->targets[target->index];
   311	
   312		current_state = AXIOM_TARGET_STATE_NOT_PRESENT;
   313	
   314		if (target->present) {
   315			if (target->z >= 0)
   316				current_state = AXIOM_TARGET_STATE_TOUCHING;
   317			else if (target->z > AXIOM_PROX_LEVEL && target->z < 0)
   318				current_state = AXIOM_TARGET_STATE_HOVER;
   319			else if (target->z == AXIOM_PROX_LEVEL)
   320				current_state = AXIOM_TARGET_STATE_PROX;
   321		}
   322	
   323		if (target_prev_state->state == current_state &&
   324		    target_prev_state->x == target->x &&
   325		    target_prev_state->y == target->y &&
   326		    target_prev_state->z == target->z)
   327			return false;
   328	
   329		id = target->index;
   330	
   331		dev_dbg(ts->dev, "U41 Target T%u, slot:%u present:%u, x:%u, y:%u, z:%d\n",
 > 332			target->index, slot, target->present,
   333			target->x, target->y, target->z);
   334	
   335		switch (current_state) {
   336		case AXIOM_TARGET_STATE_NOT_PRESENT:
   337		case AXIOM_TARGET_STATE_PROX:
   338			if (!target_prev_state->insert)
   339				break;
   340			target_prev_state->insert = false;
   341	
   342			if (!id)
   343				input_report_key(input_dev, BTN_TOUCH, 0);
   344	
   345			input_mt_report_slot_inactive(input_dev);
   346			/*
   347			 * make sure the previous coordinates are
   348			 * all off screen when the finger comes back
   349			 */
   350			target->x = 65535;
   351			target->y = 65535;
   352			target->z = AXIOM_PROX_LEVEL;
   353			break;
   354		case AXIOM_TARGET_STATE_HOVER:
   355		case AXIOM_TARGET_STATE_TOUCHING:
   356			target_prev_state->insert = true;
   357			input_report_abs(input_dev, ABS_MT_TRACKING_ID, id);
   358			input_report_abs(input_dev, ABS_MT_POSITION_X, target->x);
   359			input_report_abs(input_dev, ABS_MT_POSITION_Y, target->y);
   360	
   361			if (current_state == AXIOM_TARGET_STATE_TOUCHING) {
   362				input_report_abs(input_dev, ABS_MT_DISTANCE, 0);
   363				input_report_abs(input_dev, ABS_DISTANCE, 0);
   364				input_report_abs(input_dev, ABS_MT_PRESSURE, target->z);
   365				input_report_abs(input_dev, ABS_PRESSURE, target->z);
   366			} else {
   367				input_report_abs(input_dev, ABS_MT_DISTANCE, -target->z);
   368				input_report_abs(input_dev, ABS_DISTANCE, -target->z);
   369				input_report_abs(input_dev, ABS_MT_PRESSURE, 0);
   370				input_report_abs(input_dev, ABS_PRESSURE, 0);
   371			}
   372	
   373			if (!id)
   374				input_report_key(input_dev, BTN_TOUCH, (current_state ==
   375						 AXIOM_TARGET_STATE_TOUCHING));
   376			break;
   377		default:
   378			break;
   379		}
   380	
   381		target_prev_state->state = current_state;
   382		target_prev_state->x = target->x;
   383		target_prev_state->y = target->y;
   384		target_prev_state->z = target->z;
   385	
   386		return true;
   387	}
   388	

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

^ permalink raw reply

* Re: [PATCH v8 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
From: kernel test robot @ 2024-02-20  0:20 UTC (permalink / raw)
  To: Kamel Bouhara, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Henrik Rydberg, linux-input, linux-kernel,
	devicetree, Marco Felsch, Jeff LaBundy
  Cc: oe-kbuild-all, catalin.popescu, mark.satterthwaite,
	Thomas Petazzoni, Gregory Clement, bsp-development.geo,
	Kamel Bouhara
In-Reply-To: <20240219101221.129750-4-kamel.bouhara@bootlin.com>

Hi Kamel,

kernel test robot noticed the following build errors:

[auto build test ERROR on dtor-input/next]
[also build test ERROR on dtor-input/for-linus robh/for-next krzk-dt/for-next linus/master v6.8-rc5 next-20240219]
[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/Kamel-Bouhara/dt-bindings-vendor-prefixes-Add-TouchNetix-AS/20240219-181550
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link:    https://lore.kernel.org/r/20240219101221.129750-4-kamel.bouhara%40bootlin.com
patch subject: [PATCH v8 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
config: i386-buildonly-randconfig-002-20240220 (https://download.01.org/0day-ci/archive/20240220/202402200849.ABf6sZnr-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240220/202402200849.ABf6sZnr-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/202402200849.ABf6sZnr-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/device.h:15:0,
                    from drivers/input/touchscreen/touchnetix_axiom.c:17:
   drivers/input/touchscreen/touchnetix_axiom.c: In function 'axiom_process_u41_report_target':
>> drivers/input/touchscreen/touchnetix_axiom.c:332:18: error: 'slot' undeclared (first use in this function); did you mean 'sget'?
      target->index, slot, target->present,
                     ^
   include/linux/dev_printk.h:129:34: note: in definition of macro 'dev_printk'
      _dev_printk(level, dev, fmt, ##__VA_ARGS__);  \
                                     ^~~~~~~~~~~
   drivers/input/touchscreen/touchnetix_axiom.c:331:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(ts->dev, "U41 Target T%u, slot:%u present:%u, x:%u, y:%u, z:%d\n",
     ^~~~~~~
   drivers/input/touchscreen/touchnetix_axiom.c:332:18: note: each undeclared identifier is reported only once for each function it appears in
      target->index, slot, target->present,
                     ^
   include/linux/dev_printk.h:129:34: note: in definition of macro 'dev_printk'
      _dev_printk(level, dev, fmt, ##__VA_ARGS__);  \
                                     ^~~~~~~~~~~
   drivers/input/touchscreen/touchnetix_axiom.c:331:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(ts->dev, "U41 Target T%u, slot:%u present:%u, x:%u, y:%u, z:%d\n",
     ^~~~~~~


vim +332 drivers/input/touchscreen/touchnetix_axiom.c

   289	
   290	/*
   291	 * Support function to axiom_process_u41_report.
   292	 * Generates input-subsystem events for every target.
   293	 * After calling this function the caller shall issue
   294	 * a Sync to the input sub-system.
   295	 */
   296	static bool axiom_process_u41_report_target(struct axiom_data *ts,
   297						    struct axiom_target_report *target)
   298	{
   299		struct input_dev *input_dev = ts->input_dev;
   300		struct axiom_u41_target *target_prev_state;
   301		enum axiom_target_state current_state;
   302		int id;
   303	
   304		/* Verify the target index */
   305		if (target->index >= AXIOM_U41_MAX_TARGETS) {
   306			dev_err(ts->dev, "Invalid target index! %u\n", target->index);
   307			return false;
   308		}
   309	
   310		target_prev_state = &ts->targets[target->index];
   311	
   312		current_state = AXIOM_TARGET_STATE_NOT_PRESENT;
   313	
   314		if (target->present) {
   315			if (target->z >= 0)
   316				current_state = AXIOM_TARGET_STATE_TOUCHING;
   317			else if (target->z > AXIOM_PROX_LEVEL && target->z < 0)
   318				current_state = AXIOM_TARGET_STATE_HOVER;
   319			else if (target->z == AXIOM_PROX_LEVEL)
   320				current_state = AXIOM_TARGET_STATE_PROX;
   321		}
   322	
   323		if (target_prev_state->state == current_state &&
   324		    target_prev_state->x == target->x &&
   325		    target_prev_state->y == target->y &&
   326		    target_prev_state->z == target->z)
   327			return false;
   328	
   329		id = target->index;
   330	
   331		dev_dbg(ts->dev, "U41 Target T%u, slot:%u present:%u, x:%u, y:%u, z:%d\n",
 > 332			target->index, slot, target->present,
   333			target->x, target->y, target->z);
   334	
   335		switch (current_state) {
   336		case AXIOM_TARGET_STATE_NOT_PRESENT:
   337		case AXIOM_TARGET_STATE_PROX:
   338			if (!target_prev_state->insert)
   339				break;
   340			target_prev_state->insert = false;
   341	
   342			if (!id)
   343				input_report_key(input_dev, BTN_TOUCH, 0);
   344	
   345			input_mt_report_slot_inactive(input_dev);
   346			/*
   347			 * make sure the previous coordinates are
   348			 * all off screen when the finger comes back
   349			 */
   350			target->x = 65535;
   351			target->y = 65535;
   352			target->z = AXIOM_PROX_LEVEL;
   353			break;
   354		case AXIOM_TARGET_STATE_HOVER:
   355		case AXIOM_TARGET_STATE_TOUCHING:
   356			target_prev_state->insert = true;
   357			input_report_abs(input_dev, ABS_MT_TRACKING_ID, id);
   358			input_report_abs(input_dev, ABS_MT_POSITION_X, target->x);
   359			input_report_abs(input_dev, ABS_MT_POSITION_Y, target->y);
   360	
   361			if (current_state == AXIOM_TARGET_STATE_TOUCHING) {
   362				input_report_abs(input_dev, ABS_MT_DISTANCE, 0);
   363				input_report_abs(input_dev, ABS_DISTANCE, 0);
   364				input_report_abs(input_dev, ABS_MT_PRESSURE, target->z);
   365				input_report_abs(input_dev, ABS_PRESSURE, target->z);
   366			} else {
   367				input_report_abs(input_dev, ABS_MT_DISTANCE, -target->z);
   368				input_report_abs(input_dev, ABS_DISTANCE, -target->z);
   369				input_report_abs(input_dev, ABS_MT_PRESSURE, 0);
   370				input_report_abs(input_dev, ABS_PRESSURE, 0);
   371			}
   372	
   373			if (!id)
   374				input_report_key(input_dev, BTN_TOUCH, (current_state ==
   375						 AXIOM_TARGET_STATE_TOUCHING));
   376			break;
   377		default:
   378			break;
   379		}
   380	
   381		target_prev_state->state = current_state;
   382		target_prev_state->x = target->x;
   383		target_prev_state->y = target->y;
   384		target_prev_state->z = target->z;
   385	
   386		return true;
   387	}
   388	

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

^ permalink raw reply

* Re: [PATCH 00/10] backlight: Replace struct fb_info in interfaces
From: Lee Jones @ 2024-02-19 15:02 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Daniel Thompson, jingoohan1, deller, javierm, dri-devel,
	linux-fbdev, linux-input, linux-pwm
In-Reply-To: <288a480c-74e9-49dd-a58d-294792771ea6@suse.de>

On Thu, 15 Feb 2024, Thomas Zimmermann wrote:

> Hi
> 
> Am 15.02.24 um 13:13 schrieb Daniel Thompson:
> > On Mon, Feb 12, 2024 at 05:16:33PM +0100, Thomas Zimmermann wrote:
> > > Backlight drivers implement struct backlight_ops.check_fb, which
> > > uses struct fb_info in its interface. Replace the callback with one
> > > the does not use fb_info.
> > > 
> > > In DRM, we have several drivers that implement backlight support. By
> > > including <linux/backlight.h> these drivers depend on <linux/fb.h>.
> > > At the same time, fbdev is deprecated for new drivers and likely to
> > > be replaced on many systems.
> > > 
> > > This patchset is part of a larger effort to implement the backlight
> > > code without depending on fbdev.
> > > 
> > > Patch 1 makes the backlight core match backlight and framebuffer
> > > devices via struct fb_info.bl_dev. Patches 2 to 9 then go through
> > > drivers and remove unnecessary implementations of check_fb. Finally,
> > > patch 10 replaces the check_fb hook with controls_device, which
> > > uses the framebuffer's Linux device instead of the framebuffer.
> > I won't reply individually but I also took a look at the patches for
> > the combo devices and it all looked good to me from a backlight
> > point of view.
> > 
> > However I don't want to drop Reviewed-by: on them since it risks those
> > bit being mistaken for an ack and merged ahead of the patch 1...
> 
> Thanks for reviewing. Unless someone objects, my intention is to merge
> everything via the drm-misc, so all patches should go in at once. I do have
> a lot more patches that untangle backlight and fbdev almost completely, but
> most of these changes are in the actual graphics drivers rather than the
> backlight core code. So hopefully everything can go through the DRM tree; or
> maybe the fbdev tree.

This is only acceptable if the maintainers of those trees can provide me
with a pull-request to a succinct (_only_ these patches) immutable
branch.  If this is not possible, then I should like to merge the set
through the Backlight tree and I can provide everyone else with said PR.

-- 
Lee Jones [李琼斯]

^ permalink raw reply

* Re: [PATCH v8 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
From: Kamel Bouhara @ 2024-02-19 12:49 UTC (permalink / raw)
  To: SHUKLA Mamta Ramendra
  Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Henrik Rydberg, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	Marco Felsch, Jeff LaBundy, POPESCU Catalin,
	mark.satterthwaite@touchnetix.com, Thomas Petazzoni,
	Gregory Clement, GEO-CHHER-bsp-development
In-Reply-To: <e374d585-5acc-bbe0-ee1d-65f7abdb8f8d@leica-geosystems.com>

Le Mon, Feb 19, 2024 at 10:58:22AM +0000, SHUKLA Mamta Ramendra a écrit :
>
>
> Hello Kamel,

Hello,

[...]

> > +
> > +/*
> > + * Support function to axiom_process_u41_report.
> > + * Generates input-subsystem events for every target.
> > + * After calling this function the caller shall issue
> > + * a Sync to the input sub-system.
> > + */
> > +static bool axiom_process_u41_report_target(struct axiom_data *ts,
> > +                                           struct axiom_target_report *target)
> > +{
> > +       struct input_dev *input_dev = ts->input_dev;
> > +       struct axiom_u41_target *target_prev_state;
> > +       enum axiom_target_state current_state;
> > +       int id;
> > +
> > +       /* Verify the target index */
> > +       if (target->index >= AXIOM_U41_MAX_TARGETS) {
> > +               dev_err(ts->dev, "Invalid target index! %u\n", target->index);
> > +               return false;
> > +       }
> > +
> > +       target_prev_state = &ts->targets[target->index];
> > +
> > +       current_state = AXIOM_TARGET_STATE_NOT_PRESENT;
> > +
> > +       if (target->present) {
> > +               if (target->z >= 0)
> > +                       current_state = AXIOM_TARGET_STATE_TOUCHING;
> > +               else if (target->z > AXIOM_PROX_LEVEL && target->z < 0)
> > +                       current_state = AXIOM_TARGET_STATE_HOVER;
> > +               else if (target->z == AXIOM_PROX_LEVEL)
> > +                       current_state = AXIOM_TARGET_STATE_PROX;
> > +       }
> > +
> > +       if (target_prev_state->state == current_state &&
> > +           target_prev_state->x == target->x &&
> > +           target_prev_state->y == target->y &&
> > +           target_prev_state->z == target->z)
> > +               return false;
> > +
> > +       id = target->index;
> > +
> > +       dev_dbg(ts->dev, "U41 Target T%u, slot:%u present:%u, x:%u, y:%u, z:%d\n",
> > +               target->index, slot, target->present,
> > +               target->x, target->y, target->z);
>
> This doesn't compile. You forgot to replace slot  with id.
>
>
> -       dev_dbg(ts->dev, "U41 Target T%u, slot:%u present:%u, x:%u,
> y:%u, z:%d\n",
> -               target->index, slot, target->present,
> +       dev_dbg(ts->dev, "U41 Target T%u, id:%u present:%u, x:%u, y:%u,
> z:%d\n",
> +               target->index, id, target->present,
>

I don't know how I could missed this one :/ !

Fixed thanks !

--
Kamel Bouhara, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH v8 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
From: SHUKLA Mamta Ramendra @ 2024-02-19 10:58 UTC (permalink / raw)
  To: Kamel Bouhara, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Henrik Rydberg, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	Marco Felsch, Jeff LaBundy
  Cc: POPESCU Catalin, mark.satterthwaite@touchnetix.com,
	Thomas Petazzoni, Gregory Clement, GEO-CHHER-bsp-development
In-Reply-To: <20240219101221.129750-4-kamel.bouhara@bootlin.com>



Hello Kamel,

Thanks for v8.

On 19.02.24 11:12, Kamel Bouhara wrote:

> Add a new driver for the TouchNetix's axiom family of
> touchscreen controllers. This driver only supports i2c
> and can be later adapted for SPI and USB support.
> 
> Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
> ---
>   MAINTAINERS                                  |   1 +
>   drivers/input/touchscreen/Kconfig            |  12 +
>   drivers/input/touchscreen/Makefile           |   1 +
>   drivers/input/touchscreen/touchnetix_axiom.c | 669 +++++++++++++++++++
>   4 files changed, 683 insertions(+)
>   create mode 100644 drivers/input/touchscreen/touchnetix_axiom.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4b3ed2f22b89..de5fad8f317a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22262,6 +22262,7 @@ M:      Kamel Bouhara <kamel.bouhara@bootlin.com>
>   L:     linux-input@vger.kernel.org
>   S:     Maintained
>   F:     Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml
> +F:     drivers/input/touchscreen/touchnetix_axiom.c
> 
>   TPM DEVICE DRIVER
>   M:     Peter Huewe <peterhuewe@gmx.de>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index e3e2324547b9..f36bee8d8696 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -803,6 +803,18 @@ config TOUCHSCREEN_MIGOR
>            To compile this driver as a module, choose M here: the
>            module will be called migor_ts.
> 
> +config TOUCHSCREEN_TOUCHNETIX_AXIOM
> +       tristate "TouchNetix AXIOM based touchscreen controllers"
> +       depends on I2C
> +       help
> +         Say Y here if you have a axiom touchscreen connected to
> +         your system.
> +
> +         If unsure, say N.
> +
> +         To compile this driver as a module, choose M here: the
> +         module will be called axiom.
> +
>   config TOUCHSCREEN_TOUCHRIGHT
>          tristate "Touchright serial touchscreen"
>          select SERIO
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 62bd24f3ac8e..8e32a2df5e18 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -88,6 +88,7 @@ obj-$(CONFIG_TOUCHSCREEN_SUR40)               += sur40.o
>   obj-$(CONFIG_TOUCHSCREEN_SURFACE3_SPI) += surface3_spi.o
>   obj-$(CONFIG_TOUCHSCREEN_TI_AM335X_TSC)        += ti_am335x_tsc.o
>   obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213)   += touchit213.o
> +obj-$(CONFIG_TOUCHSCREEN_TOUCHNETIX_AXIOM)     += touchnetix_axiom.o
>   obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT)   += touchright.o
>   obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN)     += touchwin.o
>   obj-$(CONFIG_TOUCHSCREEN_TS4800)       += ts4800-ts.o
> diff --git a/drivers/input/touchscreen/touchnetix_axiom.c b/drivers/input/touchscreen/touchnetix_axiom.c
> new file mode 100644
> index 000000000000..f7d1b23a4baf
> --- /dev/null
> +++ b/drivers/input/touchscreen/touchnetix_axiom.c
> @@ -0,0 +1,669 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * TouchNetix axiom Touchscreen Driver
> + *
> + * Copyright (C) 2020-2023 TouchNetix Ltd.
> + *
> + * Author(s): Bart Prescott <bartp@baasheep.co.uk>
> + *            Pedro Torruella <pedro.torruella@touchnetix.com>
> + *            Mark Satterthwaite <mark.satterthwaite@touchnetix.com>
> + *            Hannah Rossiter <hannah.rossiter@touchnetix.com>
> + *            Kamel Bouhara <kamel.bouhara@bootlin.com>
> + *
> + */
> +#include <linux/bitfield.h>
> +#include <linux/crc16.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/input/mt.h>
> +#include <linux/input/touchscreen.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/regmap.h>
> +
> +#include <asm/unaligned.h>
> +#define AXIOM_PROX_LEVEL               -128
> +#define AXIOM_DMA_OPS_DELAY_USEC       250
> +#define AXIOM_STARTUP_TIME_MS          110
> +/*
> + * Register group u31 has 2 pages for usage table entries.
> + */
> +#define AXIOM_U31_MAX_USAGES           0xff
> +#define AXIOM_U31_BYTES_PER_USAGE      6
> +#define AXIOM_U31_PAGE0_LENGTH         0x0C
> +#define AXIOM_U31_BOOTMODE_MASK                BIT(7)
> +#define AXIOM_U31_DEVID_MASK           GENMASK(14, 0)
> +
> +#define AXIOM_MAX_REPORT_LEN           0x7f
> +
> +#define AXIOM_CMD_HEADER_READ_MASK     BIT(15)
> +#define AXIOM_U41_MAX_TARGETS          10
> +
> +#define AXIOM_U46_AUX_CHANNELS         4
> +#define AXIOM_U46_AUX_MASK             GENMASK(11, 0)
> +
> +#define AXIOM_COMMS_MAX_USAGE_PAGES    3
> +#define AXIOM_COMMS_PAGE_SIZE          256
> +#define AXIOM_COMMS_REPORT_LEN_MASK    GENMASK(6, 0)
> +
> +#define AXIOM_REPORT_USAGE_ID          0x34
> +#define AXIOM_DEVINFO_USAGE_ID         0x31
> +#define AXIOM_USAGE_2HB_REPORT_ID      0x01
> +#define AXIOM_USAGE_2AUX_REPORT_ID     0x46
> +#define AXIOM_USAGE_2DCTS_REPORT_ID    0x41
> +
> +#define AXIOM_PAGE_OFFSET_MASK         GENMASK(6, 0)
> +
> +struct axiom_devinfo {
> +       __le16 device_id;
> +       u8 fw_minor;
> +       u8 fw_major;
> +       u8 fw_info_extra;
> +       u8 tcp_revision;
> +       u8 bootloader_fw_minor;
> +       u8 bootloader_fw_major;
> +       __le16 jedec_id;
> +       u8 num_usages;
> +} __packed;
> +
> +/*
> + * Describes parameters of a specific usage, essentially a single element of
> + * the "Usage Table"
> + */
> +struct axiom_usage_entry {
> +       u8 id;
> +       u8 is_report;
> +       u8 start_page;
> +       u8 num_pages;
> +};
> +
> +/*
> + * Represents state of a touch or target when detected prior to a touch (eg.
> + * hover or proximity events).
> + */
> +enum axiom_target_state {
> +       AXIOM_TARGET_STATE_NOT_PRESENT = 0,
> +       AXIOM_TARGET_STATE_PROX = 1,
> +       AXIOM_TARGET_STATE_HOVER = 2,
> +       AXIOM_TARGET_STATE_TOUCHING = 3,
> +};
> +
> +struct axiom_u41_target {
> +       enum axiom_target_state state;
> +       u16 x;
> +       u16 y;
> +       s8 z;
> +       bool insert;
> +       bool touch;
> +};
> +
> +struct axiom_target_report {
> +       u8 index;
> +       u8 present;
> +       u16 x;
> +       u16 y;
> +       s8 z;
> +};
> +
> +struct axiom_cmd_header {
> +       __le16 target_address;
> +       __le16 length;
> +} __packed;
> +
> +struct axiom_data {
> +       struct axiom_devinfo devinfo;
> +       struct device *dev;
> +       struct gpio_desc *reset_gpio;
> +       struct i2c_client *client;
> +       struct input_dev *input_dev;
> +       u32 max_report_len;
> +       u8 rx_buf[AXIOM_COMMS_MAX_USAGE_PAGES * AXIOM_COMMS_PAGE_SIZE];
> +       struct axiom_u41_target targets[AXIOM_U41_MAX_TARGETS];
> +       struct axiom_usage_entry usage_table[AXIOM_U31_MAX_USAGES];
> +       bool usage_table_populated;
> +       struct regulator *vdda;
> +       struct regulator *vddi;
> +       struct regmap *regmap;
> +       struct touchscreen_properties   prop;
> +};
> +
> +static const struct regmap_config axiom_i2c_regmap_config = {
> +       .reg_bits = 32,
> +       .reg_format_endian = REGMAP_ENDIAN_LITTLE,
> +       .val_bits = 8,
> +       .val_format_endian = REGMAP_ENDIAN_LITTLE,
> +};
> +
> +/*
> + * axiom devices are typically configured to report touches at a rate
> + * of 100Hz (10ms) for systems that require polling for reports.
> + * When reports are polled, it will be expected to occasionally
> + * observe the overflow bit being set in the reports.
> + * This indicates that reports are not being read fast enough.
> + */
> +#define POLL_INTERVAL_DEFAULT_MS 10
> +
> +/* Translate usage/page/offset triplet into physical address. */
> +static u16 axiom_usage_to_target_address(struct axiom_data *ts, u8 usage, u8 page,
> +                                        char offset)
> +{
> +       /* At the moment the convention is that u31 is always at physical address 0x0 */
> +       if (!ts->usage_table_populated) {
> +               if (usage == AXIOM_DEVINFO_USAGE_ID)
> +                       return ((page << 8) + offset);
> +               else
> +                       return 0xffff;
> +       }
> +
> +       if (page >= ts->usage_table[usage].num_pages) {
> +               dev_err(ts->dev, "Invalid usage table! usage: u%02x, page: %02x, offset: %02x\n",
> +                       usage, page, offset);
> +               return 0xffff;
> +       }
> +
> +       return ((ts->usage_table[usage].start_page + page) << 8) + offset;
> +}
> +
> +static int axiom_read(struct axiom_data *ts, u8 usage, u8 page, void *buf, u16 len)
> +{
> +       struct axiom_cmd_header cmd_header;
> +       u32 preamble;
> +       int ret;
> +
> +       cmd_header.target_address = cpu_to_le16(axiom_usage_to_target_address(ts, usage, page, 0));
> +       cmd_header.length = cpu_to_le16(len | AXIOM_CMD_HEADER_READ_MASK);
> +
> +       preamble = get_unaligned_le32(&cmd_header);
> +
> +       ret = regmap_write(ts->regmap, preamble, 0);
> +       if (ret) {
> +               dev_err(ts->dev, "failed to write preamble, error %d\n", ret);
> +               return ret;
> +       }
> +
> +       ret = regmap_raw_read(ts->regmap, 0, buf, len);
> +       if (ret) {
> +               dev_err(ts->dev, "failed to read target address %04x, error %d\n",
> +                       cmd_header.target_address, ret);
> +               return ret;
> +       }
> +
> +       /* Wait device's DMA operations */
> +       usleep_range(AXIOM_DMA_OPS_DELAY_USEC, AXIOM_DMA_OPS_DELAY_USEC + 50);
> +
> +       return 0;
> +}
> +
> +/*
> + * One of the main purposes for reading the usage table is to identify
> + * which usages reside at which target address.
> + * When performing subsequent reads or writes to AXIOM, the target address
> + * is used to specify which usage is being accessed.
> + * Consider the following discovery code which will build up the usage table.
> + */
> +static u32 axiom_populate_usage_table(struct axiom_data *ts)
> +{
> +       struct axiom_usage_entry *usage_table;
> +       u8 *rx_data = ts->rx_buf;
> +       u32 max_report_len = 0;
> +       u32 usage_id;
> +       int error;
> +
> +       usage_table = ts->usage_table;
> +
> +       /* Read the second page of usage u31 to get the usage table */
> +       error = axiom_read(ts, AXIOM_DEVINFO_USAGE_ID, 1, rx_data,
> +                          (AXIOM_U31_BYTES_PER_USAGE * ts->devinfo.num_usages));
> +
> +       if (error)
> +               return error;
> +
> +       for (usage_id = 0; usage_id < ts->devinfo.num_usages; usage_id++) {
> +               u16 offset = (usage_id * AXIOM_U31_BYTES_PER_USAGE);
> +               u8 id = rx_data[offset + 0];
> +               u8 start_page = rx_data[offset + 1];
> +               u8 num_pages = rx_data[offset + 2];
> +               u32 max_offset = ((rx_data[offset + 3] & AXIOM_PAGE_OFFSET_MASK) + 1) * 2;
> +
> +               usage_table[id].is_report = !num_pages;
> +
> +               /* Store the entry into the usage table */
> +               usage_table[id].id = id;
> +               usage_table[id].start_page = start_page;
> +               usage_table[id].num_pages = num_pages;
> +
> +               dev_dbg(ts->dev, "Usage u%02x Info: %*ph\n", id, AXIOM_U31_BYTES_PER_USAGE,
> +                       &rx_data[offset]);
> +
> +               /* Identify the max report length the module will receive */
> +               if (usage_table[id].is_report && max_offset > max_report_len)
> +                       max_report_len = max_offset;
> +       }
> +
> +       ts->usage_table_populated = true;
> +
> +       return max_report_len;
> +}
> +
> +static int axiom_discover(struct axiom_data *ts)
> +{
> +       int error;
> +
> +       /*
> +        * Fetch the first page of usage u31 to get the
> +        * device information and the number of usages
> +        */
> +       error = axiom_read(ts, AXIOM_DEVINFO_USAGE_ID, 0, &ts->devinfo, AXIOM_U31_PAGE0_LENGTH);
> +       if (error)
> +               return error;
> +
> +       dev_dbg(ts->dev, "  Boot Mode      : %s\n",
> +               FIELD_GET(AXIOM_U31_BOOTMODE_MASK,
> +                         le16_to_cpu(ts->devinfo.device_id)) ? "BLP" : "TCP");
> +       dev_dbg(ts->dev, "  Device ID      : %04lx\n",
> +               FIELD_GET(AXIOM_U31_DEVID_MASK, le16_to_cpu(ts->devinfo.device_id)));
> +       dev_dbg(ts->dev, "  Firmware Rev   : %02x.%02x\n", ts->devinfo.fw_major,
> +               ts->devinfo.fw_minor);
> +       dev_dbg(ts->dev, "  Bootloader Rev : %02x.%02x\n", ts->devinfo.bootloader_fw_major,
> +               ts->devinfo.bootloader_fw_minor);
> +       dev_dbg(ts->dev, "  FW Extra Info  : %04x\n", ts->devinfo.fw_info_extra);
> +       dev_dbg(ts->dev, "  Silicon        : %04x\n", le16_to_cpu(ts->devinfo.jedec_id));
> +       dev_dbg(ts->dev, "  Number usages        : %04x\n", ts->devinfo.num_usages);
> +
> +       ts->max_report_len = axiom_populate_usage_table(ts);
> +       if (!ts->max_report_len || !ts->devinfo.num_usages ||
> +           ts->max_report_len > AXIOM_MAX_REPORT_LEN) {
> +               dev_err(ts->dev, "Invalid report length or usages number");
> +               return -EINVAL;
> +       }
> +
> +       dev_dbg(ts->dev, "Max Report Length: %u\n", ts->max_report_len);
> +
> +       return 0;
> +}
> +
> +/*
> + * Support function to axiom_process_u41_report.
> + * Generates input-subsystem events for every target.
> + * After calling this function the caller shall issue
> + * a Sync to the input sub-system.
> + */
> +static bool axiom_process_u41_report_target(struct axiom_data *ts,
> +                                           struct axiom_target_report *target)
> +{
> +       struct input_dev *input_dev = ts->input_dev;
> +       struct axiom_u41_target *target_prev_state;
> +       enum axiom_target_state current_state;
> +       int id;
> +
> +       /* Verify the target index */
> +       if (target->index >= AXIOM_U41_MAX_TARGETS) {
> +               dev_err(ts->dev, "Invalid target index! %u\n", target->index);
> +               return false;
> +       }
> +
> +       target_prev_state = &ts->targets[target->index];
> +
> +       current_state = AXIOM_TARGET_STATE_NOT_PRESENT;
> +
> +       if (target->present) {
> +               if (target->z >= 0)
> +                       current_state = AXIOM_TARGET_STATE_TOUCHING;
> +               else if (target->z > AXIOM_PROX_LEVEL && target->z < 0)
> +                       current_state = AXIOM_TARGET_STATE_HOVER;
> +               else if (target->z == AXIOM_PROX_LEVEL)
> +                       current_state = AXIOM_TARGET_STATE_PROX;
> +       }
> +
> +       if (target_prev_state->state == current_state &&
> +           target_prev_state->x == target->x &&
> +           target_prev_state->y == target->y &&
> +           target_prev_state->z == target->z)
> +               return false;
> +
> +       id = target->index;
> +
> +       dev_dbg(ts->dev, "U41 Target T%u, slot:%u present:%u, x:%u, y:%u, z:%d\n",
> +               target->index, slot, target->present,
> +               target->x, target->y, target->z);

This doesn't compile. You forgot to replace slot  with id.


-       dev_dbg(ts->dev, "U41 Target T%u, slot:%u present:%u, x:%u, 
y:%u, z:%d\n",
-               target->index, slot, target->present,
+       dev_dbg(ts->dev, "U41 Target T%u, id:%u present:%u, x:%u, y:%u, 
z:%d\n",
+               target->index, id, target->present,

> +       switch (current_state) {
> +       case AXIOM_TARGET_STATE_NOT_PRESENT:
> +       case AXIOM_TARGET_STATE_PROX:
> +               if (!target_prev_state->insert)
> +                       break;
> +               target_prev_state->insert = false;
> +
> +               if (!id)
> +                       input_report_key(input_dev, BTN_TOUCH, 0);
> +
> +               input_mt_report_slot_inactive(input_dev);
> +               /*
> +                * make sure the previous coordinates are
> +                * all off screen when the finger comes back
> +                */
> +               target->x = 65535;
> +               target->y = 65535;
> +               target->z = AXIOM_PROX_LEVEL;
> +               break;
> +       case AXIOM_TARGET_STATE_HOVER:
> +       case AXIOM_TARGET_STATE_TOUCHING:
> +               target_prev_state->insert = true;
> +               input_report_abs(input_dev, ABS_MT_TRACKING_ID, id);
> +               input_report_abs(input_dev, ABS_MT_POSITION_X, target->x);
> +               input_report_abs(input_dev, ABS_MT_POSITION_Y, target->y);
> +
> +               if (current_state == AXIOM_TARGET_STATE_TOUCHING) {
> +                       input_report_abs(input_dev, ABS_MT_DISTANCE, 0);
> +                       input_report_abs(input_dev, ABS_DISTANCE, 0);
> +                       input_report_abs(input_dev, ABS_MT_PRESSURE, target->z);
> +                       input_report_abs(input_dev, ABS_PRESSURE, target->z);
> +               } else {
> +                       input_report_abs(input_dev, ABS_MT_DISTANCE, -target->z);
> +                       input_report_abs(input_dev, ABS_DISTANCE, -target->z);
> +                       input_report_abs(input_dev, ABS_MT_PRESSURE, 0);
> +                       input_report_abs(input_dev, ABS_PRESSURE, 0);
> +               }
> +
> +               if (!id)
> +                       input_report_key(input_dev, BTN_TOUCH, (current_state ==
> +                                        AXIOM_TARGET_STATE_TOUCHING));
> +               break;
> +       default:
> +               break;
> +       }
> +
> +       target_prev_state->state = current_state;
> +       target_prev_state->x = target->x;
> +       target_prev_state->y = target->y;
> +       target_prev_state->z = target->z;
> +
> +       return true;
> +}
> +
> +/*
> + * U41 is the output report of the 2D CTS and contains the status of targets
> + * (including contacts and pre-contacts) along with their X,Y,Z values.
> + * When a target has been removed (no longer detected),
> + * the corresponding X,Y,Z values will be zeroed.
> + */
> +static bool axiom_process_u41_report(struct axiom_data *ts, u8 *rx_buf)
> +{
> +       struct axiom_target_report target;
> +       bool update_done = false;
> +       u16 target_status;
> +       int i;
> +
> +       target_status = get_unaligned_le16(rx_buf + 1);
> +
> +       for (i = 0; i < AXIOM_U41_MAX_TARGETS; i++) {
> +               u8 *target_step = &rx_buf[i * 4];
> +
> +               target.index = i;
> +               input_mt_slot(ts->input_dev, i);
> +               input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, true);
> +               target.present = ((target_status & (1 << i)) != 0) ? 1 : 0;
> +               target.x = get_unaligned_le16(target_step + 3);
> +               target.y = get_unaligned_le16(target_step + 5);
> +               target.z = (s8)(rx_buf[i + 43]);
> +               touchscreen_report_pos(ts->input_dev, &ts->prop, target.x, target.y, true);
> +               update_done |= axiom_process_u41_report_target(ts, &target);
> +       }
> +
> +       return update_done;
> +}
> +
> +/*
> + * U46 report contains a low level measurement data generated by the capacitive
> + * displacement sensor (CDS) algorithms from the auxiliary channels.
> + * This information is useful when tuning multi-press to assess mechanical
> + * consistency in the unit's construction.
> + */
> +static void axiom_process_u46_report(struct axiom_data *ts, u8 *rx_buf)
> +{
> +       struct input_dev *input_dev = ts->input_dev;
> +       u32 event_value;
> +       u16 aux_value;
> +       int i;
> +
> +       for (i = 0; i < AXIOM_U46_AUX_CHANNELS; i++) {
> +               u8 *target_step = &rx_buf[i * 2];
> +
> +               input_mt_slot(input_dev, i);
> +               input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, true);
> +               aux_value = get_unaligned_le16(target_step + 1) & AXIOM_U46_AUX_MASK;
> +               event_value = (i << 16) | (aux_value);
> +               input_event(input_dev, EV_MSC, MSC_RAW, event_value);
> +       }
> +}
> +
> +/*
> + * Validates the crc and demultiplexes the axiom reports to the appropriate
> + * report handler
> + */
> +static int axiom_handle_events(struct axiom_data *ts)
> +{
> +       struct input_dev *input_dev = ts->input_dev;
> +       u8 *report_data = ts->rx_buf;
> +       struct device *dev = ts->dev;
> +       u16 crc_report;
> +       u8 *crc_bytes;
> +       u16 crc_calc;
> +       int error;
> +       u8 len;
> +
> +       error = axiom_read(ts, AXIOM_REPORT_USAGE_ID, 0, report_data, ts->max_report_len);
> +       if (error)
> +               return error;
> +
> +       len = (report_data[0] & AXIOM_COMMS_REPORT_LEN_MASK) << 1;
> +       if (len <= 2) {
> +               dev_err(dev, "Zero length report discarded.\n");
> +               return -ENODATA;
> +       }
> +
> +       /* Validate the report CRC */
> +       crc_bytes = &report_data[len];
> +
> +       crc_report = get_unaligned_le16(crc_bytes - 2);
> +       /* Length is in 16 bit words and remove the size of the CRC16 itself */
> +       crc_calc = crc16(0, report_data, (len - 2));
> +
> +       if (crc_calc != crc_report) {
> +               dev_err(dev,
> +                       "CRC mismatch! Expected: %#x, Calculated CRC: %#x.\n",
> +                       crc_report, crc_calc);
> +               return -EINVAL;
> +       }
> +
> +       switch (report_data[1]) {
> +       case AXIOM_USAGE_2DCTS_REPORT_ID:
> +               if (axiom_process_u41_report(ts, &report_data[1])) {
> +                       input_mt_sync_frame(input_dev);
> +                       input_sync(input_dev);
> +               }
> +               break;
> +
> +       case AXIOM_USAGE_2AUX_REPORT_ID:
> +               /* This is an aux report (force) */
> +               axiom_process_u46_report(ts, &report_data[1]);
> +               input_mt_sync(input_dev);
> +               input_sync(input_dev);
> +               break;
> +
> +       case AXIOM_USAGE_2HB_REPORT_ID:
> +               /* This is a heartbeat report */
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +static void axiom_i2c_poll(struct input_dev *input_dev)
> +{
> +       struct axiom_data *ts = input_get_drvdata(input_dev);
> +
> +       axiom_handle_events(ts);
> +}
> +
> +static irqreturn_t axiom_irq(int irq, void *dev_id)
> +{
> +       struct axiom_data *ts = dev_id;
> +
> +       axiom_handle_events(ts);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static void axiom_reset(struct gpio_desc *reset_gpio)
> +{
> +       gpiod_set_value_cansleep(reset_gpio, 1);
> +       usleep_range(1000, 2000);
> +       gpiod_set_value_cansleep(reset_gpio, 0);
> +       msleep(AXIOM_STARTUP_TIME_MS);
> +}
> +
> +static int axiom_i2c_probe(struct i2c_client *client)
> +{
> +       struct device *dev = &client->dev;
> +       struct input_dev *input_dev;
> +       struct axiom_data *ts;
> +       u32 poll_interval;
> +       int target;
> +       int error;
> +
> +       ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> +       if (!ts)
> +               return -ENOMEM;
> +
> +       i2c_set_clientdata(client, ts);
> +       ts->client = client;
> +       ts->dev = dev;
> +
> +       ts->regmap = devm_regmap_init_i2c(client, &axiom_i2c_regmap_config);
> +       error = PTR_ERR_OR_ZERO(ts->regmap);
> +       if (error) {
> +               dev_err(dev, "Failed to initialize regmap: %d\n", error);
> +               return error;
> +       }
> +
> +       ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +       if (IS_ERR(ts->reset_gpio))
> +               return dev_err_probe(dev, PTR_ERR(ts->reset_gpio), "failed to get reset GPIO\n");
> +
> +       if (ts->reset_gpio)
> +               axiom_reset(ts->reset_gpio);
> +
> +       ts->vddi = devm_regulator_get_optional(dev, "vddi");
> +       if (!IS_ERR(ts->vddi)) {
> +               error = devm_regulator_get_enable(dev, "vddi");
> +               if (error)
> +                       return dev_err_probe(&client->dev, error,
> +                                            "Failed to enable vddi regulator\n");
> +       }
> +
> +       ts->vdda = devm_regulator_get_optional(dev, "vdda");
> +       if (!IS_ERR(ts->vdda)) {
> +               error = devm_regulator_get_enable(dev, "vdda");
> +               if (error)
> +                       return dev_err_probe(&client->dev, error,
> +                                            "Failed to enable vdda regulator\n");
> +               msleep(AXIOM_STARTUP_TIME_MS);
> +       }
> +
> +       error = axiom_discover(ts);
> +       if (error)
> +               return dev_err_probe(dev, error, "Failed touchscreen discover\n");
> +
> +       input_dev = devm_input_allocate_device(ts->dev);
> +       if (!input_dev)
> +               return -ENOMEM;
> +
> +       input_dev->name = "TouchNetix axiom Touchscreen";
> +       input_dev->phys = "input/axiom_ts";
> +
> +       input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0, 65535, 0, 0);
> +       input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0, 65535, 0, 0);
> +       input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE, 0, MT_TOOL_MAX, 0, 0);
> +       input_set_abs_params(input_dev, ABS_MT_DISTANCE, 0, 127, 0, 0);
> +       input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 127, 0, 0);
> +
> +       touchscreen_parse_properties(input_dev, true, &ts->prop);
> +
> +       /* Registers the axiom device as a touchscreen instead of a mouse pointer */
> +       error = input_mt_init_slots(input_dev, AXIOM_U41_MAX_TARGETS, INPUT_MT_DIRECT);
> +       if (error)
> +               return error;
> +
> +       /* Enables the raw data for up to 4 force channels to be sent to the input subsystem */
> +       set_bit(EV_REL, input_dev->evbit);
> +       set_bit(EV_MSC, input_dev->evbit);
> +       /* Declare that we support "RAW" Miscellaneous events */
> +       set_bit(MSC_RAW, input_dev->mscbit);
> +
> +       ts->input_dev = input_dev;
> +       input_set_drvdata(ts->input_dev, ts);
> +
> +       /* Ensure that all reports are initialised to not be present. */
> +       for (target = 0; target < AXIOM_U41_MAX_TARGETS; target++)
> +               ts->targets[target].state = AXIOM_TARGET_STATE_NOT_PRESENT;
> +
> +       error = devm_request_threaded_irq(dev, client->irq, NULL,
> +                                         axiom_irq, IRQF_ONESHOT, dev_name(dev), ts);
> +       if (error) {
> +               dev_info(dev, "Request irq failed, falling back to polling mode");
> +
> +               error = input_setup_polling(input_dev, axiom_i2c_poll);
> +               if (error)
> +                       return dev_err_probe(ts->dev, error, "Unable to set up polling mode\n");
> +
> +               if (!device_property_read_u32(ts->dev, "poll-interval", &poll_interval))
> +                       input_set_poll_interval(input_dev, poll_interval);
> +               else
> +                       input_set_poll_interval(input_dev, POLL_INTERVAL_DEFAULT_MS);
> +       }
> +
> +       error = input_register_device(input_dev);
> +       if (error)
> +               return dev_err_probe(ts->dev, error,
> +                                    "Could not register with Input Sub-system.\n");
> +
> +       return 0;
> +}
> +
> +static const struct i2c_device_id axiom_i2c_id_table[] = {
> +       { "ax54a" },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(i2c, axiom_i2c_id_table);
> +
> +static const struct of_device_id axiom_i2c_of_match[] = {
> +       { .compatible = "touchnetix,ax54a", },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, axiom_i2c_of_match);
> +
> +static struct i2c_driver axiom_i2c_driver = {
> +       .driver = {
> +                  .name = "axiom",
> +                  .of_match_table = axiom_i2c_of_match,
> +       },
> +       .id_table = axiom_i2c_id_table,
> +       .probe = axiom_i2c_probe,
> +};
> +module_i2c_driver(axiom_i2c_driver);
> +
> +MODULE_AUTHOR("Bart Prescott <bartp@baasheep.co.uk>");
> +MODULE_AUTHOR("Pedro Torruella <pedro.torruella@touchnetix.com>");
> +MODULE_AUTHOR("Mark Satterthwaite <mark.satterthwaite@touchnetix.com>");
> +MODULE_AUTHOR("Hannah Rossiter <hannah.rossiter@touchnetix.com>");
> +MODULE_AUTHOR("Kamel Bouhara <kamel.bouhara@bootlin.com>");
> +MODULE_DESCRIPTION("TouchNetix axiom touchscreen I2C bus driver");
> +MODULE_LICENSE("GPL");
> --
> 2.25.1
> 

^ permalink raw reply

* [PATCH v8 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
From: Kamel Bouhara @ 2024-02-19 10:12 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Henrik Rydberg, linux-input, linux-kernel, devicetree,
	Marco Felsch, Jeff LaBundy
  Cc: catalin.popescu, mark.satterthwaite, Thomas Petazzoni,
	Gregory Clement, bsp-development.geo, Kamel Bouhara
In-Reply-To: <20240219101221.129750-1-kamel.bouhara@bootlin.com>

Add a new driver for the TouchNetix's axiom family of
touchscreen controllers. This driver only supports i2c
and can be later adapted for SPI and USB support.

Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
---
 MAINTAINERS                                  |   1 +
 drivers/input/touchscreen/Kconfig            |  12 +
 drivers/input/touchscreen/Makefile           |   1 +
 drivers/input/touchscreen/touchnetix_axiom.c | 669 +++++++++++++++++++
 4 files changed, 683 insertions(+)
 create mode 100644 drivers/input/touchscreen/touchnetix_axiom.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 4b3ed2f22b89..de5fad8f317a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22262,6 +22262,7 @@ M:	Kamel Bouhara <kamel.bouhara@bootlin.com>
 L:	linux-input@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml
+F:	drivers/input/touchscreen/touchnetix_axiom.c
 
 TPM DEVICE DRIVER
 M:	Peter Huewe <peterhuewe@gmx.de>
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index e3e2324547b9..f36bee8d8696 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -803,6 +803,18 @@ config TOUCHSCREEN_MIGOR
 	  To compile this driver as a module, choose M here: the
 	  module will be called migor_ts.
 
+config TOUCHSCREEN_TOUCHNETIX_AXIOM
+	tristate "TouchNetix AXIOM based touchscreen controllers"
+	depends on I2C
+	help
+	  Say Y here if you have a axiom touchscreen connected to
+	  your system.
+
+	  If unsure, say N.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called axiom.
+
 config TOUCHSCREEN_TOUCHRIGHT
 	tristate "Touchright serial touchscreen"
 	select SERIO
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 62bd24f3ac8e..8e32a2df5e18 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -88,6 +88,7 @@ obj-$(CONFIG_TOUCHSCREEN_SUR40)		+= sur40.o
 obj-$(CONFIG_TOUCHSCREEN_SURFACE3_SPI)	+= surface3_spi.o
 obj-$(CONFIG_TOUCHSCREEN_TI_AM335X_TSC)	+= ti_am335x_tsc.o
 obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213)	+= touchit213.o
+obj-$(CONFIG_TOUCHSCREEN_TOUCHNETIX_AXIOM)	+= touchnetix_axiom.o
 obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT)	+= touchright.o
 obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN)	+= touchwin.o
 obj-$(CONFIG_TOUCHSCREEN_TS4800)	+= ts4800-ts.o
diff --git a/drivers/input/touchscreen/touchnetix_axiom.c b/drivers/input/touchscreen/touchnetix_axiom.c
new file mode 100644
index 000000000000..f7d1b23a4baf
--- /dev/null
+++ b/drivers/input/touchscreen/touchnetix_axiom.c
@@ -0,0 +1,669 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * TouchNetix axiom Touchscreen Driver
+ *
+ * Copyright (C) 2020-2023 TouchNetix Ltd.
+ *
+ * Author(s): Bart Prescott <bartp@baasheep.co.uk>
+ *            Pedro Torruella <pedro.torruella@touchnetix.com>
+ *            Mark Satterthwaite <mark.satterthwaite@touchnetix.com>
+ *            Hannah Rossiter <hannah.rossiter@touchnetix.com>
+ *            Kamel Bouhara <kamel.bouhara@bootlin.com>
+ *
+ */
+#include <linux/bitfield.h>
+#include <linux/crc16.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/regmap.h>
+
+#include <asm/unaligned.h>
+#define AXIOM_PROX_LEVEL		-128
+#define AXIOM_DMA_OPS_DELAY_USEC	250
+#define AXIOM_STARTUP_TIME_MS		110
+/*
+ * Register group u31 has 2 pages for usage table entries.
+ */
+#define AXIOM_U31_MAX_USAGES		0xff
+#define AXIOM_U31_BYTES_PER_USAGE	6
+#define AXIOM_U31_PAGE0_LENGTH		0x0C
+#define AXIOM_U31_BOOTMODE_MASK		BIT(7)
+#define AXIOM_U31_DEVID_MASK		GENMASK(14, 0)
+
+#define AXIOM_MAX_REPORT_LEN		0x7f
+
+#define AXIOM_CMD_HEADER_READ_MASK	BIT(15)
+#define AXIOM_U41_MAX_TARGETS		10
+
+#define AXIOM_U46_AUX_CHANNELS		4
+#define AXIOM_U46_AUX_MASK		GENMASK(11, 0)
+
+#define AXIOM_COMMS_MAX_USAGE_PAGES	3
+#define AXIOM_COMMS_PAGE_SIZE		256
+#define AXIOM_COMMS_REPORT_LEN_MASK	GENMASK(6, 0)
+
+#define AXIOM_REPORT_USAGE_ID		0x34
+#define AXIOM_DEVINFO_USAGE_ID		0x31
+#define AXIOM_USAGE_2HB_REPORT_ID	0x01
+#define AXIOM_USAGE_2AUX_REPORT_ID	0x46
+#define AXIOM_USAGE_2DCTS_REPORT_ID	0x41
+
+#define AXIOM_PAGE_OFFSET_MASK		GENMASK(6, 0)
+
+struct axiom_devinfo {
+	__le16 device_id;
+	u8 fw_minor;
+	u8 fw_major;
+	u8 fw_info_extra;
+	u8 tcp_revision;
+	u8 bootloader_fw_minor;
+	u8 bootloader_fw_major;
+	__le16 jedec_id;
+	u8 num_usages;
+} __packed;
+
+/*
+ * Describes parameters of a specific usage, essentially a single element of
+ * the "Usage Table"
+ */
+struct axiom_usage_entry {
+	u8 id;
+	u8 is_report;
+	u8 start_page;
+	u8 num_pages;
+};
+
+/*
+ * Represents state of a touch or target when detected prior to a touch (eg.
+ * hover or proximity events).
+ */
+enum axiom_target_state {
+	AXIOM_TARGET_STATE_NOT_PRESENT = 0,
+	AXIOM_TARGET_STATE_PROX = 1,
+	AXIOM_TARGET_STATE_HOVER = 2,
+	AXIOM_TARGET_STATE_TOUCHING = 3,
+};
+
+struct axiom_u41_target {
+	enum axiom_target_state state;
+	u16 x;
+	u16 y;
+	s8 z;
+	bool insert;
+	bool touch;
+};
+
+struct axiom_target_report {
+	u8 index;
+	u8 present;
+	u16 x;
+	u16 y;
+	s8 z;
+};
+
+struct axiom_cmd_header {
+	__le16 target_address;
+	__le16 length;
+} __packed;
+
+struct axiom_data {
+	struct axiom_devinfo devinfo;
+	struct device *dev;
+	struct gpio_desc *reset_gpio;
+	struct i2c_client *client;
+	struct input_dev *input_dev;
+	u32 max_report_len;
+	u8 rx_buf[AXIOM_COMMS_MAX_USAGE_PAGES * AXIOM_COMMS_PAGE_SIZE];
+	struct axiom_u41_target targets[AXIOM_U41_MAX_TARGETS];
+	struct axiom_usage_entry usage_table[AXIOM_U31_MAX_USAGES];
+	bool usage_table_populated;
+	struct regulator *vdda;
+	struct regulator *vddi;
+	struct regmap *regmap;
+	struct touchscreen_properties	prop;
+};
+
+static const struct regmap_config axiom_i2c_regmap_config = {
+	.reg_bits = 32,
+	.reg_format_endian = REGMAP_ENDIAN_LITTLE,
+	.val_bits = 8,
+	.val_format_endian = REGMAP_ENDIAN_LITTLE,
+};
+
+/*
+ * axiom devices are typically configured to report touches at a rate
+ * of 100Hz (10ms) for systems that require polling for reports.
+ * When reports are polled, it will be expected to occasionally
+ * observe the overflow bit being set in the reports.
+ * This indicates that reports are not being read fast enough.
+ */
+#define POLL_INTERVAL_DEFAULT_MS 10
+
+/* Translate usage/page/offset triplet into physical address. */
+static u16 axiom_usage_to_target_address(struct axiom_data *ts, u8 usage, u8 page,
+					 char offset)
+{
+	/* At the moment the convention is that u31 is always at physical address 0x0 */
+	if (!ts->usage_table_populated) {
+		if (usage == AXIOM_DEVINFO_USAGE_ID)
+			return ((page << 8) + offset);
+		else
+			return 0xffff;
+	}
+
+	if (page >= ts->usage_table[usage].num_pages) {
+		dev_err(ts->dev, "Invalid usage table! usage: u%02x, page: %02x, offset: %02x\n",
+			usage, page, offset);
+		return 0xffff;
+	}
+
+	return ((ts->usage_table[usage].start_page + page) << 8) + offset;
+}
+
+static int axiom_read(struct axiom_data *ts, u8 usage, u8 page, void *buf, u16 len)
+{
+	struct axiom_cmd_header cmd_header;
+	u32 preamble;
+	int ret;
+
+	cmd_header.target_address = cpu_to_le16(axiom_usage_to_target_address(ts, usage, page, 0));
+	cmd_header.length = cpu_to_le16(len | AXIOM_CMD_HEADER_READ_MASK);
+
+	preamble = get_unaligned_le32(&cmd_header);
+
+	ret = regmap_write(ts->regmap, preamble, 0);
+	if (ret) {
+		dev_err(ts->dev, "failed to write preamble, error %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_raw_read(ts->regmap, 0, buf, len);
+	if (ret) {
+		dev_err(ts->dev, "failed to read target address %04x, error %d\n",
+			cmd_header.target_address, ret);
+		return ret;
+	}
+
+	/* Wait device's DMA operations */
+	usleep_range(AXIOM_DMA_OPS_DELAY_USEC, AXIOM_DMA_OPS_DELAY_USEC + 50);
+
+	return 0;
+}
+
+/*
+ * One of the main purposes for reading the usage table is to identify
+ * which usages reside at which target address.
+ * When performing subsequent reads or writes to AXIOM, the target address
+ * is used to specify which usage is being accessed.
+ * Consider the following discovery code which will build up the usage table.
+ */
+static u32 axiom_populate_usage_table(struct axiom_data *ts)
+{
+	struct axiom_usage_entry *usage_table;
+	u8 *rx_data = ts->rx_buf;
+	u32 max_report_len = 0;
+	u32 usage_id;
+	int error;
+
+	usage_table = ts->usage_table;
+
+	/* Read the second page of usage u31 to get the usage table */
+	error = axiom_read(ts, AXIOM_DEVINFO_USAGE_ID, 1, rx_data,
+			   (AXIOM_U31_BYTES_PER_USAGE * ts->devinfo.num_usages));
+
+	if (error)
+		return error;
+
+	for (usage_id = 0; usage_id < ts->devinfo.num_usages; usage_id++) {
+		u16 offset = (usage_id * AXIOM_U31_BYTES_PER_USAGE);
+		u8 id = rx_data[offset + 0];
+		u8 start_page = rx_data[offset + 1];
+		u8 num_pages = rx_data[offset + 2];
+		u32 max_offset = ((rx_data[offset + 3] & AXIOM_PAGE_OFFSET_MASK) + 1) * 2;
+
+		usage_table[id].is_report = !num_pages;
+
+		/* Store the entry into the usage table */
+		usage_table[id].id = id;
+		usage_table[id].start_page = start_page;
+		usage_table[id].num_pages = num_pages;
+
+		dev_dbg(ts->dev, "Usage u%02x Info: %*ph\n", id, AXIOM_U31_BYTES_PER_USAGE,
+			&rx_data[offset]);
+
+		/* Identify the max report length the module will receive */
+		if (usage_table[id].is_report && max_offset > max_report_len)
+			max_report_len = max_offset;
+	}
+
+	ts->usage_table_populated = true;
+
+	return max_report_len;
+}
+
+static int axiom_discover(struct axiom_data *ts)
+{
+	int error;
+
+	/*
+	 * Fetch the first page of usage u31 to get the
+	 * device information and the number of usages
+	 */
+	error = axiom_read(ts, AXIOM_DEVINFO_USAGE_ID, 0, &ts->devinfo, AXIOM_U31_PAGE0_LENGTH);
+	if (error)
+		return error;
+
+	dev_dbg(ts->dev, "  Boot Mode      : %s\n",
+		FIELD_GET(AXIOM_U31_BOOTMODE_MASK,
+			  le16_to_cpu(ts->devinfo.device_id)) ? "BLP" : "TCP");
+	dev_dbg(ts->dev, "  Device ID      : %04lx\n",
+		FIELD_GET(AXIOM_U31_DEVID_MASK, le16_to_cpu(ts->devinfo.device_id)));
+	dev_dbg(ts->dev, "  Firmware Rev   : %02x.%02x\n", ts->devinfo.fw_major,
+		ts->devinfo.fw_minor);
+	dev_dbg(ts->dev, "  Bootloader Rev : %02x.%02x\n", ts->devinfo.bootloader_fw_major,
+		ts->devinfo.bootloader_fw_minor);
+	dev_dbg(ts->dev, "  FW Extra Info  : %04x\n", ts->devinfo.fw_info_extra);
+	dev_dbg(ts->dev, "  Silicon        : %04x\n", le16_to_cpu(ts->devinfo.jedec_id));
+	dev_dbg(ts->dev, "  Number usages        : %04x\n", ts->devinfo.num_usages);
+
+	ts->max_report_len = axiom_populate_usage_table(ts);
+	if (!ts->max_report_len || !ts->devinfo.num_usages ||
+	    ts->max_report_len > AXIOM_MAX_REPORT_LEN) {
+		dev_err(ts->dev, "Invalid report length or usages number");
+		return -EINVAL;
+	}
+
+	dev_dbg(ts->dev, "Max Report Length: %u\n", ts->max_report_len);
+
+	return 0;
+}
+
+/*
+ * Support function to axiom_process_u41_report.
+ * Generates input-subsystem events for every target.
+ * After calling this function the caller shall issue
+ * a Sync to the input sub-system.
+ */
+static bool axiom_process_u41_report_target(struct axiom_data *ts,
+					    struct axiom_target_report *target)
+{
+	struct input_dev *input_dev = ts->input_dev;
+	struct axiom_u41_target *target_prev_state;
+	enum axiom_target_state current_state;
+	int id;
+
+	/* Verify the target index */
+	if (target->index >= AXIOM_U41_MAX_TARGETS) {
+		dev_err(ts->dev, "Invalid target index! %u\n", target->index);
+		return false;
+	}
+
+	target_prev_state = &ts->targets[target->index];
+
+	current_state = AXIOM_TARGET_STATE_NOT_PRESENT;
+
+	if (target->present) {
+		if (target->z >= 0)
+			current_state = AXIOM_TARGET_STATE_TOUCHING;
+		else if (target->z > AXIOM_PROX_LEVEL && target->z < 0)
+			current_state = AXIOM_TARGET_STATE_HOVER;
+		else if (target->z == AXIOM_PROX_LEVEL)
+			current_state = AXIOM_TARGET_STATE_PROX;
+	}
+
+	if (target_prev_state->state == current_state &&
+	    target_prev_state->x == target->x &&
+	    target_prev_state->y == target->y &&
+	    target_prev_state->z == target->z)
+		return false;
+
+	id = target->index;
+
+	dev_dbg(ts->dev, "U41 Target T%u, slot:%u present:%u, x:%u, y:%u, z:%d\n",
+		target->index, slot, target->present,
+		target->x, target->y, target->z);
+
+	switch (current_state) {
+	case AXIOM_TARGET_STATE_NOT_PRESENT:
+	case AXIOM_TARGET_STATE_PROX:
+		if (!target_prev_state->insert)
+			break;
+		target_prev_state->insert = false;
+
+		if (!id)
+			input_report_key(input_dev, BTN_TOUCH, 0);
+
+		input_mt_report_slot_inactive(input_dev);
+		/*
+		 * make sure the previous coordinates are
+		 * all off screen when the finger comes back
+		 */
+		target->x = 65535;
+		target->y = 65535;
+		target->z = AXIOM_PROX_LEVEL;
+		break;
+	case AXIOM_TARGET_STATE_HOVER:
+	case AXIOM_TARGET_STATE_TOUCHING:
+		target_prev_state->insert = true;
+		input_report_abs(input_dev, ABS_MT_TRACKING_ID, id);
+		input_report_abs(input_dev, ABS_MT_POSITION_X, target->x);
+		input_report_abs(input_dev, ABS_MT_POSITION_Y, target->y);
+
+		if (current_state == AXIOM_TARGET_STATE_TOUCHING) {
+			input_report_abs(input_dev, ABS_MT_DISTANCE, 0);
+			input_report_abs(input_dev, ABS_DISTANCE, 0);
+			input_report_abs(input_dev, ABS_MT_PRESSURE, target->z);
+			input_report_abs(input_dev, ABS_PRESSURE, target->z);
+		} else {
+			input_report_abs(input_dev, ABS_MT_DISTANCE, -target->z);
+			input_report_abs(input_dev, ABS_DISTANCE, -target->z);
+			input_report_abs(input_dev, ABS_MT_PRESSURE, 0);
+			input_report_abs(input_dev, ABS_PRESSURE, 0);
+		}
+
+		if (!id)
+			input_report_key(input_dev, BTN_TOUCH, (current_state ==
+					 AXIOM_TARGET_STATE_TOUCHING));
+		break;
+	default:
+		break;
+	}
+
+	target_prev_state->state = current_state;
+	target_prev_state->x = target->x;
+	target_prev_state->y = target->y;
+	target_prev_state->z = target->z;
+
+	return true;
+}
+
+/*
+ * U41 is the output report of the 2D CTS and contains the status of targets
+ * (including contacts and pre-contacts) along with their X,Y,Z values.
+ * When a target has been removed (no longer detected),
+ * the corresponding X,Y,Z values will be zeroed.
+ */
+static bool axiom_process_u41_report(struct axiom_data *ts, u8 *rx_buf)
+{
+	struct axiom_target_report target;
+	bool update_done = false;
+	u16 target_status;
+	int i;
+
+	target_status = get_unaligned_le16(rx_buf + 1);
+
+	for (i = 0; i < AXIOM_U41_MAX_TARGETS; i++) {
+		u8 *target_step = &rx_buf[i * 4];
+
+		target.index = i;
+		input_mt_slot(ts->input_dev, i);
+		input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, true);
+		target.present = ((target_status & (1 << i)) != 0) ? 1 : 0;
+		target.x = get_unaligned_le16(target_step + 3);
+		target.y = get_unaligned_le16(target_step + 5);
+		target.z = (s8)(rx_buf[i + 43]);
+		touchscreen_report_pos(ts->input_dev, &ts->prop, target.x, target.y, true);
+		update_done |= axiom_process_u41_report_target(ts, &target);
+	}
+
+	return update_done;
+}
+
+/*
+ * U46 report contains a low level measurement data generated by the capacitive
+ * displacement sensor (CDS) algorithms from the auxiliary channels.
+ * This information is useful when tuning multi-press to assess mechanical
+ * consistency in the unit's construction.
+ */
+static void axiom_process_u46_report(struct axiom_data *ts, u8 *rx_buf)
+{
+	struct input_dev *input_dev = ts->input_dev;
+	u32 event_value;
+	u16 aux_value;
+	int i;
+
+	for (i = 0; i < AXIOM_U46_AUX_CHANNELS; i++) {
+		u8 *target_step = &rx_buf[i * 2];
+
+		input_mt_slot(input_dev, i);
+		input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, true);
+		aux_value = get_unaligned_le16(target_step + 1) & AXIOM_U46_AUX_MASK;
+		event_value = (i << 16) | (aux_value);
+		input_event(input_dev, EV_MSC, MSC_RAW, event_value);
+	}
+}
+
+/*
+ * Validates the crc and demultiplexes the axiom reports to the appropriate
+ * report handler
+ */
+static int axiom_handle_events(struct axiom_data *ts)
+{
+	struct input_dev *input_dev = ts->input_dev;
+	u8 *report_data = ts->rx_buf;
+	struct device *dev = ts->dev;
+	u16 crc_report;
+	u8 *crc_bytes;
+	u16 crc_calc;
+	int error;
+	u8 len;
+
+	error = axiom_read(ts, AXIOM_REPORT_USAGE_ID, 0, report_data, ts->max_report_len);
+	if (error)
+		return error;
+
+	len = (report_data[0] & AXIOM_COMMS_REPORT_LEN_MASK) << 1;
+	if (len <= 2) {
+		dev_err(dev, "Zero length report discarded.\n");
+		return -ENODATA;
+	}
+
+	/* Validate the report CRC */
+	crc_bytes = &report_data[len];
+
+	crc_report = get_unaligned_le16(crc_bytes - 2);
+	/* Length is in 16 bit words and remove the size of the CRC16 itself */
+	crc_calc = crc16(0, report_data, (len - 2));
+
+	if (crc_calc != crc_report) {
+		dev_err(dev,
+			"CRC mismatch! Expected: %#x, Calculated CRC: %#x.\n",
+			crc_report, crc_calc);
+		return -EINVAL;
+	}
+
+	switch (report_data[1]) {
+	case AXIOM_USAGE_2DCTS_REPORT_ID:
+		if (axiom_process_u41_report(ts, &report_data[1])) {
+			input_mt_sync_frame(input_dev);
+			input_sync(input_dev);
+		}
+		break;
+
+	case AXIOM_USAGE_2AUX_REPORT_ID:
+		/* This is an aux report (force) */
+		axiom_process_u46_report(ts, &report_data[1]);
+		input_mt_sync(input_dev);
+		input_sync(input_dev);
+		break;
+
+	case AXIOM_USAGE_2HB_REPORT_ID:
+		/* This is a heartbeat report */
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void axiom_i2c_poll(struct input_dev *input_dev)
+{
+	struct axiom_data *ts = input_get_drvdata(input_dev);
+
+	axiom_handle_events(ts);
+}
+
+static irqreturn_t axiom_irq(int irq, void *dev_id)
+{
+	struct axiom_data *ts = dev_id;
+
+	axiom_handle_events(ts);
+
+	return IRQ_HANDLED;
+}
+
+static void axiom_reset(struct gpio_desc *reset_gpio)
+{
+	gpiod_set_value_cansleep(reset_gpio, 1);
+	usleep_range(1000, 2000);
+	gpiod_set_value_cansleep(reset_gpio, 0);
+	msleep(AXIOM_STARTUP_TIME_MS);
+}
+
+static int axiom_i2c_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct input_dev *input_dev;
+	struct axiom_data *ts;
+	u32 poll_interval;
+	int target;
+	int error;
+
+	ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
+	if (!ts)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, ts);
+	ts->client = client;
+	ts->dev = dev;
+
+	ts->regmap = devm_regmap_init_i2c(client, &axiom_i2c_regmap_config);
+	error = PTR_ERR_OR_ZERO(ts->regmap);
+	if (error) {
+		dev_err(dev, "Failed to initialize regmap: %d\n", error);
+		return error;
+	}
+
+	ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(ts->reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(ts->reset_gpio), "failed to get reset GPIO\n");
+
+	if (ts->reset_gpio)
+		axiom_reset(ts->reset_gpio);
+
+	ts->vddi = devm_regulator_get_optional(dev, "vddi");
+	if (!IS_ERR(ts->vddi)) {
+		error = devm_regulator_get_enable(dev, "vddi");
+		if (error)
+			return dev_err_probe(&client->dev, error,
+					     "Failed to enable vddi regulator\n");
+	}
+
+	ts->vdda = devm_regulator_get_optional(dev, "vdda");
+	if (!IS_ERR(ts->vdda)) {
+		error = devm_regulator_get_enable(dev, "vdda");
+		if (error)
+			return dev_err_probe(&client->dev, error,
+					     "Failed to enable vdda regulator\n");
+		msleep(AXIOM_STARTUP_TIME_MS);
+	}
+
+	error = axiom_discover(ts);
+	if (error)
+		return dev_err_probe(dev, error, "Failed touchscreen discover\n");
+
+	input_dev = devm_input_allocate_device(ts->dev);
+	if (!input_dev)
+		return -ENOMEM;
+
+	input_dev->name = "TouchNetix axiom Touchscreen";
+	input_dev->phys = "input/axiom_ts";
+
+	input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0, 65535, 0, 0);
+	input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0, 65535, 0, 0);
+	input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE, 0, MT_TOOL_MAX, 0, 0);
+	input_set_abs_params(input_dev, ABS_MT_DISTANCE, 0, 127, 0, 0);
+	input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 127, 0, 0);
+
+	touchscreen_parse_properties(input_dev, true, &ts->prop);
+
+	/* Registers the axiom device as a touchscreen instead of a mouse pointer */
+	error = input_mt_init_slots(input_dev, AXIOM_U41_MAX_TARGETS, INPUT_MT_DIRECT);
+	if (error)
+		return error;
+
+	/* Enables the raw data for up to 4 force channels to be sent to the input subsystem */
+	set_bit(EV_REL, input_dev->evbit);
+	set_bit(EV_MSC, input_dev->evbit);
+	/* Declare that we support "RAW" Miscellaneous events */
+	set_bit(MSC_RAW, input_dev->mscbit);
+
+	ts->input_dev = input_dev;
+	input_set_drvdata(ts->input_dev, ts);
+
+	/* Ensure that all reports are initialised to not be present. */
+	for (target = 0; target < AXIOM_U41_MAX_TARGETS; target++)
+		ts->targets[target].state = AXIOM_TARGET_STATE_NOT_PRESENT;
+
+	error = devm_request_threaded_irq(dev, client->irq, NULL,
+					  axiom_irq, IRQF_ONESHOT, dev_name(dev), ts);
+	if (error) {
+		dev_info(dev, "Request irq failed, falling back to polling mode");
+
+		error = input_setup_polling(input_dev, axiom_i2c_poll);
+		if (error)
+			return dev_err_probe(ts->dev, error, "Unable to set up polling mode\n");
+
+		if (!device_property_read_u32(ts->dev, "poll-interval", &poll_interval))
+			input_set_poll_interval(input_dev, poll_interval);
+		else
+			input_set_poll_interval(input_dev, POLL_INTERVAL_DEFAULT_MS);
+	}
+
+	error = input_register_device(input_dev);
+	if (error)
+		return dev_err_probe(ts->dev, error,
+				     "Could not register with Input Sub-system.\n");
+
+	return 0;
+}
+
+static const struct i2c_device_id axiom_i2c_id_table[] = {
+	{ "ax54a" },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, axiom_i2c_id_table);
+
+static const struct of_device_id axiom_i2c_of_match[] = {
+	{ .compatible = "touchnetix,ax54a", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, axiom_i2c_of_match);
+
+static struct i2c_driver axiom_i2c_driver = {
+	.driver = {
+		   .name = "axiom",
+		   .of_match_table = axiom_i2c_of_match,
+	},
+	.id_table = axiom_i2c_id_table,
+	.probe = axiom_i2c_probe,
+};
+module_i2c_driver(axiom_i2c_driver);
+
+MODULE_AUTHOR("Bart Prescott <bartp@baasheep.co.uk>");
+MODULE_AUTHOR("Pedro Torruella <pedro.torruella@touchnetix.com>");
+MODULE_AUTHOR("Mark Satterthwaite <mark.satterthwaite@touchnetix.com>");
+MODULE_AUTHOR("Hannah Rossiter <hannah.rossiter@touchnetix.com>");
+MODULE_AUTHOR("Kamel Bouhara <kamel.bouhara@bootlin.com>");
+MODULE_DESCRIPTION("TouchNetix axiom touchscreen I2C bus driver");
+MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related

* [PATCH v8 2/3] dt-bindings: input: Add TouchNetix axiom touchscreen
From: Kamel Bouhara @ 2024-02-19 10:12 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Henrik Rydberg, linux-input, linux-kernel, devicetree,
	Marco Felsch, Jeff LaBundy
  Cc: catalin.popescu, mark.satterthwaite, Thomas Petazzoni,
	Gregory Clement, bsp-development.geo, Kamel Bouhara,
	Krzysztof Kozlowski
In-Reply-To: <20240219101221.129750-1-kamel.bouhara@bootlin.com>

Add the TouchNetix axiom I2C touchscreen device tree bindings
documentation.

Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../input/touchscreen/touchnetix,ax54a.yaml   | 62 +++++++++++++++++++
 MAINTAINERS                                   |  6 ++
 2 files changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml

diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml
new file mode 100644
index 000000000000..66229a4d6f15
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/touchnetix,ax54a.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TouchNetix Axiom series touchscreen controller
+
+maintainers:
+  - Kamel Bouhara <kamel.bouhara@bootlin.com>
+
+allOf:
+  - $ref: /schemas/input/touchscreen/touchscreen.yaml#
+  - $ref: /schemas/input/input.yaml#
+
+properties:
+  compatible:
+    const: touchnetix,ax54a
+
+  reg:
+    const: 0x66
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  vdda-supply:
+    description: Analog power supply regulator on VDDA pin
+
+  vddi-supply:
+    description: I/O power supply regulator on VDDI pin
+
+required:
+  - compatible
+  - reg
+  - vdda-supply
+  - vddi-supply
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      touchscreen@66 {
+        compatible = "touchnetix,ax54a";
+        reg = <0x66>;
+        interrupt-parent = <&gpio2>;
+        interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
+        reset-gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>;
+        vdda-supply = <&vdda_reg>;
+        vddi-supply = <&vddi_reg>;
+        poll-interval = <20>;
+      };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index c4828ab15f1a..4b3ed2f22b89 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22257,6 +22257,12 @@ L:	platform-driver-x86@vger.kernel.org
 S:	Maintained
 F:	drivers/platform/x86/toshiba-wmi.c

+TOUCHNETIX AXIOM I2C TOUCHSCREEN DRIVER
+M:	Kamel Bouhara <kamel.bouhara@bootlin.com>
+L:	linux-input@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml
+
 TPM DEVICE DRIVER
 M:	Peter Huewe <peterhuewe@gmx.de>
 M:	Jarkko Sakkinen <jarkko@kernel.org>
--
2.25.1


^ permalink raw reply related

* [PATCH v8 1/3] dt-bindings: vendor-prefixes: Add TouchNetix AS
From: Kamel Bouhara @ 2024-02-19 10:12 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Henrik Rydberg, linux-input, linux-kernel, devicetree,
	Marco Felsch, Jeff LaBundy
  Cc: catalin.popescu, mark.satterthwaite, Thomas Petazzoni,
	Gregory Clement, bsp-development.geo, Kamel Bouhara,
	Krzysztof Kozlowski
In-Reply-To: <20240219101221.129750-1-kamel.bouhara@bootlin.com>

Add vendor prefix for TouchNetix AS (https://www.touchnetix.com/products/).

Signed-off-by: Kamel Bouhara <kamel.bouhara@bootlin.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 1a0dc04f1db4..88954a286107 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1444,6 +1444,8 @@ patternProperties:
     description: Toradex AG
   "^toshiba,.*":
     description: Toshiba Corporation
+  "^touchnetix,.*":
+    description: TouchNetix AS
   "^toumaz,.*":
     description: Toumaz
   "^tpk,.*":
-- 
2.25.1


^ permalink raw reply related

* [PATCH v8 0/3] Input: Add TouchNetix axiom touchscreen driver
From: Kamel Bouhara @ 2024-02-19 10:12 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Henrik Rydberg, linux-input, linux-kernel, devicetree,
	Marco Felsch, Jeff LaBundy
  Cc: catalin.popescu, mark.satterthwaite, Thomas Petazzoni,
	Gregory Clement, bsp-development.geo, Kamel Bouhara

Add a new driver for the TouchNetix's axiom family of touchscreen
controller. This driver only support i2c and can be later adapted for
SPI and USB support.

---
Changes in v2:
 - Add device tree binding documentation
 - Move core functions in axiom_i2c as we only care about i2c support now
 - Use static function when required
 - Use syntax dev_err_probe()
 - Add an hardware based reset

Changes in v3:
 - Remove irq-gpios property in dt-binding
 - Use a generic node name
 - Fix issues reported in https://lore.kernel.org/oe-kbuild-all/202310100300.oAC2M62R-lkp@intel.com/

Changes in v4:
 - Cleanup unused headers and macros
 - Use standard kernel type
 - Namespace structures and functions
 - Use packed struct when possible to avoid bitfield operators
 - Fix missing break when address is found in axiom_populate_target_address()
 - Split reads in two steps for the reports, first length then report
   itself so we only read required bytes
 - Get poll-interval from devicetree
 - Add VDDI/VDDA regulators
 - Add a startup delay of 110 ms required after VDDA/VDDI is applied
 - Remove axiom_i2c_write() as it is no more used

Changes in v5:
 - Fix wrong message constructed in axiom_i2c_read
 - Delay required between i2c reads is >= 250us
 - Do not split report reading in two phases as we'll
   have to wait 500us
 - Use lower-case in properties names
 - Make regulators properties are required in dt-binding
 - Fix bug report: https://lore.kernel.org/lkml/202312051457.y3N1q3sZ-lkp@intel.com/
 - Fix bug report: https://lore.kernel.org/lkml/6f8e3b64-5b21-4a50-8680-063ef7a93bdb@suswa.mountain/

Changes in v6:
 - Fix missing unevaluatedProperties.in dt-binding
 - Use __le16 to correctly deal with device endianness
 - Use standart kernel types s/char/u8/
 - Use regmap api as driver might support spi later
 - Use get_unaligned_le16() for the sake of clarity
 - Use devm_regulator_enable_optional()

Changes in v7:
 - Remove startup time from dt-binding
 - Fix usage table not correctly populated

Changes in v8:
 - Fix missing call to input_report_slot_state()
 - Fix issue reported in https://lore.kernel.org/oe-kbuild-all/202402020623.8T1Ah513-lkp@intel.com/

Kamel Bouhara (3):
  dt-bindings: vendor-prefixes: Add TouchNetix AS
  dt-bindings: input: Add TouchNetix axiom touchscreen
  Input: Add TouchNetix axiom i2c touchscreen driver

 .../input/touchscreen/touchnetix,ax54a.yaml   |  62 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   7 +
 drivers/input/touchscreen/Kconfig             |  12 +
 drivers/input/touchscreen/Makefile            |   1 +
 drivers/input/touchscreen/touchnetix_axiom.c  | 669 ++++++++++++++++++
 6 files changed, 753 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/touchnetix,ax54a.yaml
 create mode 100644 drivers/input/touchscreen/touchnetix_axiom.c

--
2.25.1


^ permalink raw reply

* Re: [PATCH v7 0/4] Input: support overlay objects on touchscreens
From: Javier Carrasco @ 2024-02-19  8:12 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Henrik Rydberg, Bastian Hecht, Michael Riesch, Jeff LaBundy
  Cc: linux-kernel, linux-input, devicetree, Rob Herring
In-Reply-To: <20240119-feature-ts_virtobj_patch-v7-0-eda70985808f@wolfvision.net>

On 19.01.24 08:43, Javier Carrasco wrote:
> Some touchscreens are shipped with a physical layer on top of them where
> a number of buttons and a resized touchscreen surface might be available.
> 
> In order to generate proper key events by overlay buttons and adjust the
> touch events to a clipped surface, this series offers a documented,
> device-tree-based solution by means of helper functions.
> An implementation for a specific touchscreen driver is also included.
> 
> The functions in touch-overlay provide a simple workflow to acquire
> physical objects from the device tree, map them into a list and generate
> events according to the object descriptions.
> 
> This feature has been tested with a JT240MHQS-E3 display, which consists
> of an st1624 as the base touchscreen and an overlay with two buttons and
> a frame that clips its effective surface mounted on it.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
> ---
> Changes in v7:
> - General: return to a single input device implementation.
> - touchscreen bindings: segment instead of button in the label
>   description.
> - touch-overlay.c: define button-specific data inside segment struct.
> - touch-overlay.c: remove fwnode_property_present() and check return
>   value of fwnode_property_read_u32() in touch_overlay_get_segment().
> - touch-overlay.c: simplify return path in touch_overlay_map().
> - Link to v6: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v6-0-d8a605975153@wolfvision.net
> 
> Changes in v6:
> - General: use a single list to manage a single type of object.
> - General: swap patches to have bindings preceding the code.
> - touch-overlay.c: minor code-sytle fixes.
> - Link to v5: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v5-0-ff6b5c4db693@wolfvision.net
> 
> Changes in v5:
> - touchscreen bindings: move overlay common properties to a $def entry (Rob Herring)
> - st1232 bindings: move overlays to the existing example instead of
>   making a new one (Rob Herring) 
> - Link to v4: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v4-0-5c6c0fc1eed6@wolfvision.net
> 
> Changes in v4:
> - General: rename "touchscreen" to "touch" to include other consumers.
> - PATCH 1/4: move touch-overlay feature to input core.
> - PATCH 1/4, 3/4: set key caps and report key events without consumer's
>   intervention.
> - PATCH 2/4: add missing 'required' field with the required properties.
> - Link to v3: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v3-0-b4fb7fc4bab7@wolfvision.net
> 
> Changes in v3:
> - General: rename "virtobj" and "virtual" to "overlay"
> - PATCH 1/4: Make feature bool instead of tristate (selected by
>   supported touchscreens)
> - Link to v2: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v2-0-f68a6bfe7a0f@wolfvision.net
> 
> Changes in v2:
> - PATCH 1/4: remove preprocessor directives (the module is selected by
>   the drivers that support the feature). Typo in the commit message.
> - PATCH 2/4: more detailed documentation. Images and examples were added.
> - PATCH 3/4: select ts-virtobj automatically.
> - Link to v1: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v1-0-5ae5e81bc264@wolfvision.net
> 
> ---
> Javier Carrasco (4):
>       dt-bindings: touchscreen: add touch-overlay property
>       Input: touch-overlay - Add touchscreen overlay handling
>       dt-bindings: input: touchscreen: st1232: add touch-overlay example
>       Input: st1232 - add touch overlays handling
> 
>  .../input/touchscreen/sitronix,st1232.yaml         |  29 +++
>  .../bindings/input/touchscreen/touchscreen.yaml    | 119 ++++++++++
>  MAINTAINERS                                        |   7 +
>  drivers/input/Makefile                             |   2 +-
>  drivers/input/touch-overlay.c                      | 250 +++++++++++++++++++++
>  drivers/input/touchscreen/st1232.c                 |  48 ++--
>  include/linux/input/touch-overlay.h                |  22 ++
>  7 files changed, 462 insertions(+), 15 deletions(-)
> ---
> base-commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a
> change-id: 20230510-feature-ts_virtobj_patch-e267540aae74
> 
> Best regards,

Gentle ping.

Best regards,
Javier Carrasco


^ permalink raw reply

* Re: [RFC PATCH v2 1/6] dt-bindings: mfd: add entry for Marvell 88PM886 PMIC
From: Krzysztof Kozlowski @ 2024-02-19  7:29 UTC (permalink / raw)
  To: Karel Balej, Rob Herring
  Cc: Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley, Lee Jones,
	Liam Girdwood, Mark Brown, linux-input, devicetree, linux-kernel,
	Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <CZ8ATPJTOA3U.2QJSJY50EX2YH@gimli.ms.mff.cuni.cz>

On 18/02/2024 16:10, Karel Balej wrote:
> Rob Herring, 2024-02-15T08:20:52-06:00:
>>>  .../bindings/mfd/marvell,88pm88x.yaml         | 74 +++++++++++++++++++
>>
>> Filename should match the compatible.
>>
>> In general, drop the 'x' wildcard.
> 
> By "in general", do you mean for the drivers code also?

No, not driver. The rules for wildcard, that they are discouraged, are
DT binding rules.

> 
> As I have mentioned in the commit message for the driver, the other
> device is very similar and if the support for it was ever to be added
> (which I personally currently have no interest in), I believe it would
> make sense to extend this driver. Is it then still prefered to call it
> all just 88pm886 now?

Extend the driver, it's unrelated. Binding still should be named like
compatible, because that extension might never happen.

> 
>>> +properties:
>>> +  compatible:
>>> +    const: marvell,88pm886-a1
> 
> So the file should be called marvell,88pm886-a1.yaml, correct? Again, is
> it prefered to call it like this even if the other revision could
> eventually be added (again, I am not interested in that right now

If you already add two devices, flexible name would be fine. But you do
not add it now and you might never add, so keep the filename=compatible.
It is fine if it has also other compatibles later. We already accepted
many bindings like that.


Best regards,
Krzysztof


^ permalink raw reply

* RE: [PATCH] HID: Intel-ish-hid: Ishtp: Fix sensor reads after ACPI S3 suspend
From: Xu, Even @ 2024-02-19  2:11 UTC (permalink / raw)
  To: Jiri Kosina, srinivas pandruvada
  Cc: benjamin.tissoires@redhat.com, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <nycvar.YFH.7.76.2402131131250.21798@cbobk.fhfr.pm>

Thanks Jiri and Srinivas!

Best Regards,
Even Xu

-----Original Message-----
From: Jiri Kosina <jikos@kernel.org> 
Sent: Tuesday, February 13, 2024 6:32 PM
To: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Xu, Even <even.xu@intel.com>; benjamin.tissoires@redhat.com; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] HID: Intel-ish-hid: Ishtp: Fix sensor reads after ACPI S3 suspend

On Fri, 9 Feb 2024, srinivas pandruvada wrote:

> > Fixes: f645a90e8ff7 ("HID: intel-ish-hid: ishtp-hid-client: use 
> > helper functions for connection")
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218442
> > Signed-off-by: Even Xu <even.xu@intel.com>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> 
> Hi Jiri,
> 
> This regression is introduced with 6.8-rc1, so need a pull request for 
> this rc cycle.

Right; now queued in hid.git#for-6.8/upstream-fixes. Thanks,

--
Jiri Kosina
SUSE Labs


^ permalink raw reply

* Re: [RFC PATCH v2 1/6] dt-bindings: mfd: add entry for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-02-18 15:10 UTC (permalink / raw)
  To: Rob Herring
  Cc: Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley, Lee Jones,
	Liam Girdwood, Mark Brown, linux-input, devicetree, linux-kernel,
	Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <20240215142052.GA4180777-robh@kernel.org>

Rob Herring, 2024-02-15T08:20:52-06:00:
> >  .../bindings/mfd/marvell,88pm88x.yaml         | 74 +++++++++++++++++++
>
> Filename should match the compatible.
>
> In general, drop the 'x' wildcard.

By "in general", do you mean for the drivers code also?

As I have mentioned in the commit message for the driver, the other
device is very similar and if the support for it was ever to be added
(which I personally currently have no interest in), I believe it would
make sense to extend this driver. Is it then still prefered to call it
all just 88pm886 now?

> > +properties:
> > +  compatible:
> > +    const: marvell,88pm886-a1

So the file should be called marvell,88pm886-a1.yaml, correct? Again, is
it prefered to call it like this even if the other revision could
eventually be added (again, I am not interested in that right now
personally)? I mean, if I was implementing support for both revisions
right now, it would make sense to name it just marvell,88pm886.yaml, no?

Thank you, kind regards,
K. B.

^ permalink raw reply

* [syzbot] Monthly input report (Feb 2024)
From: syzbot @ 2024-02-17 20:23 UTC (permalink / raw)
  To: linux-input, linux-kernel, syzkaller-bugs

Hello input maintainers/developers,

This is a 31-day syzbot report for the input subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/input

During the period, 1 new issues were detected and 0 were fixed.
In total, 13 issues are still open and 52 have been fixed so far.

Some of the still happening issues:

Ref Crashes Repro Title
<1> 4346    Yes   WARNING in input_mt_init_slots
                  https://syzkaller.appspot.com/bug?extid=0122fa359a69694395d5
<2> 394     No    possible deadlock in evdev_pass_values (2)
                  https://syzkaller.appspot.com/bug?extid=13d3cb2a3dc61e6092f5
<3> 175     Yes   INFO: task hung in uhid_char_release
                  https://syzkaller.appspot.com/bug?extid=8fe2d362af0e1cba8735
<4> 154     Yes   WARNING in cm109_urb_irq_callback/usb_submit_urb
                  https://syzkaller.appspot.com/bug?extid=2d6d691af5ab4b7e66df
<5> 2       Yes   WARNING in input_unregister_device (2)
                  https://syzkaller.appspot.com/bug?extid=617f4ccb03b9869f6494

---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders

To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem

You may send multiple commands in a single email message.

^ permalink raw reply

* Re: [PATCH RFC bpf-next v2 02/10] bpf/helpers: introduce sleepable timers
From: Toke Høiland-Jørgensen @ 2024-02-17 13:42 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Martin KaFai Lau, bpf, linux-kernel, linux-input, linux-doc,
	linux-kselftest, Alexei Starovoitov, Daniel Borkmann,
	John Fastabend, Andrii Nakryiko, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Shuah Khan
In-Reply-To: <fckhc367l6eha2gpftixhzjdsmo2jts5p6ir6ukx2q5xndsbhf@btzjwvuamcv4>

Benjamin Tissoires <bentiss@kernel.org> writes:

> On Feb 16 2024, Toke Høiland-Jørgensen wrote:
>> Benjamin Tissoires <bentiss@kernel.org> writes:
>> 
>> > On Feb 15 2024, Martin KaFai Lau wrote:
>> >> On 2/14/24 9:18 AM, Benjamin Tissoires wrote:
>> >> > +static void bpf_timer_work_cb(struct work_struct *work)
>> >> > +{
>> >> > +	struct bpf_hrtimer *t = container_of(work, struct bpf_hrtimer, work);
>> >> > +	struct bpf_map *map = t->map;
>> >> > +	void *value = t->value;
>> >> > +	bpf_callback_t callback_fn;
>> >> > +	void *key;
>> >> > +	u32 idx;
>> >> > +
>> >> > +	BTF_TYPE_EMIT(struct bpf_timer);
>> >> > +
>> >> > +	rcu_read_lock();
>> >> > +	callback_fn = rcu_dereference(t->sleepable_cb_fn);
>> >> > +	rcu_read_unlock();
>> >> 
>> >> I took a very brief look at patch 2. One thing that may worth to ask here,
>> >> the rcu_read_unlock() seems to be done too early. It is protecting the
>> >> t->sleepable_cb_fn (?), so should it be done after finished using the
>> >> callback_fn?
>> >
>> > Probably :)
>> >
>> > TBH, everytime I work with RCUs I spent countless hours trying to
>> > re-understand everything, and in this case I'm currently in the "let's
>> > make it work" process than fixing concurrency issues.
>> > I still gave it a shot in case it solves my issue, but no, I still have
>> > the crash.
>> >
>> > But given that callback_fn might sleep, isn't it an issue to keep the
>> > RCU_reader lock so long? (we don't seem to call synchronize_rcu() so it
>> > might be fine, but I'd like the confirmation from someone else).
>> 
>> You're right, it isn't. From the RCU/checklist.rst doc:
>> 
>> 13.	Unlike most flavors of RCU, it *is* permissible to block in an
>> 	SRCU read-side critical section (demarked by srcu_read_lock()
>> 	and srcu_read_unlock()), hence the "SRCU": "sleepable RCU".
>> 	Please note that if you don't need to sleep in read-side critical
>> 	sections, you should be using RCU rather than SRCU, because RCU
>> 	is almost always faster and easier to use than is SRCU.
>> 
>> So we can't use the regular RCU protection for the callback in this
>> usage. We'll need to either convert it to SRCU, or add another
>> protection mechanism to make sure the callback function is not freed
>> from under us (like a refcnt). I suspect the latter may be simpler (from
>> reading the rest of that documentation around SRCU.
>
> Currently I'm thinking at also incrementing the ->prog held in the
> bpf_hrtimer which should prevent the callback to be freed, if I'm not wrong.
> Then I should be able to just release the rcu_read_unlock before calling
> the actual callback. And then put the ref on ->prog once done.
>
> But to be able to do that I might need to protect ->prog with an RCU
> too.

Hmm, bpf_timer_set_callback() already increments the bpf refcnt; so it's
a matter of ensuring that bpf_timer_cancel() and
bpf_timer_cancel_and_free() wait for the callback to complete even in
the workqueue case. The current 'hrtimer_running' percpu global var is
not going to cut it for that, so I guess some other kind of locking will
be needed? Not really sure what would be appropriate here, a refcnt, or
maybe a full mutex?

I am not actually sure the RCU protection of the callback field itself
is that important given all the other protections that make sure the
callback has exited before cancelling? As long as we add another such
protection I think it can just be a READ_ONCE() for getting the cb
pointer?

>> >> A high level design question. The intention of the new
>> >> bpf_timer_set_sleepable_cb() kfunc is actually to delay work to a workqueue.
>> >> It is useful to delay work from the bpf_timer_cb and it may also useful to
>> >> delay work from other bpf running context (e.g. the networking hooks like
>> >> "tc"). The bpf_timer_set_sleepable_cb() seems to be unnecessary forcing
>> >> delay-work must be done in a bpf_timer_cb.
>> >
>> > Basically I'm just a monkey here. I've been told that I should use
>> > bpf_timer[0]. But my implementation is not finished, as Alexei mentioned
>> > that we should bypass hrtimer if I'm not wrong [1].
>> 
>> I don't think getting rid of the hrtimer in favour of
>> schedule_delayed_work() makes any sense. schedule_delayed_work() does
>> exactly the same as you're doing in this version of the patch: it
>> schedules a timer callback, and calls queue_work() from inside that
>> timer callback. It just uses "regular" timers instead of hrtimers. So I
>> don't think there's any performance benefit from using that facility; on
>> the contrary, it would require extra logic to handle cancellation etc;
>> might as well just re-use the existing hrtimer-based callback logic we
>> already have, and do a schedule_work() from the hrtimer callback like
>> you're doing now.
>
> I agree that we can nicely emulate delayed_timer with the current patch
> series. However, if I understand Alexei's idea (and Martin's) there are
> cases where we just want schedule_work(), without any timer involved.
> That makes a weird timer (with a delay always equal to 0), but it would
> allow to satisfy those latency issues.
>
> So (and this also answers your second email today) I'm thinking at:
> - have multiple flags to control the timer (with dedicated timer_cb
>   kernel functions):
>   - BPF_F_TIMER_HRTIMER (default)
>   - BPF_F_TIMER_WORKER (no timer, just workqueue)
>   - BPF_F_TIMER_DELAYED_WORKER (hrtimer + workqueue, or actual
>     delayed_work, but that's re-implementing stuffs)

I don't think the "delayed" bit needs to be a property of the timer; the
context in which the timer is executed (softirq vs workqueue) is,
because that has consequences for how the callback is verified (it would
be neat if we could know the flag at verification time, but since we
can't we need the pairing with the _set_sleepable_cb()).

But the same timer could be used both as an immediate and a delayed
callback during its lifetime; so I think this should rather be governed
by a flag to bpf_timer_start(). In fact, the patch I linked earlier[0]
does just that, adding a BPF_TIMER_IMMEDIATE flag to bpf_timer_start().
I.e., keep the hrtimer allocated at all times, but skip going through it
if that flag is set.

An alternative could also be to just special-case a zero timeout in
bpf_timer_start(); I don't actually recall why I went with the flag
instead when I wrote that patch...

-Toke

[0] https://git.kernel.org/pub/scm/linux/kernel/git/toke/linux.git/commit/?h=xdp-queueing-08&id=54bc201a358d1ac6ebfe900099315bbd0a76e862


^ permalink raw reply

* Re: [syzbot] [usb?] [input?] WARNING in input_unregister_device (2)
From: syzbot @ 2024-02-17 12:01 UTC (permalink / raw)
  To: gregkh, linux-input, linux-kernel, linux-usb, rafael,
	syzkaller-bugs
In-Reply-To: <0000000000004b841a060e876595@google.com>

syzbot has found a reproducer for the following issue on:

HEAD commit:    c1ca10ceffbb Merge tag 'scsi-fixes' of git://git.kernel.or..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=15159df0180000
kernel config:  https://syzkaller.appspot.com/x/.config?x=1d7c92dd8d5c7a1e
dashboard link: https://syzkaller.appspot.com/bug?extid=617f4ccb03b9869f6494
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=11f415fc180000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17ecf362180000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/842b77f831b0/disk-c1ca10ce.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/057da06d8a4b/vmlinux-c1ca10ce.xz
kernel image: https://storage.googleapis.com/syzbot-assets/d4382274a169/bzImage-c1ca10ce.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+617f4ccb03b9869f6494@syzkaller.appspotmail.com

input: HID 045e:07da as /devices/platform/dummy_hcd.0/usb1/1-1/1-1:0.0/0003:045E:07DA.0060/input/input100
microsoft 0003:045E:07DA.0060: input,hidraw0: USB HID v0.00 Device [HID 045e:07da] on usb-dummy_hcd.0-1/input0
usb 1-1: USB disconnect, device number 97
------------[ cut here ]------------
add_uevent_var: buffer size too small
WARNING: CPU: 0 PID: 782 at lib/kobject_uevent.c:671 add_uevent_var+0x2d5/0x450 lib/kobject_uevent.c:671
Modules linked in:
CPU: 0 PID: 782 Comm: kworker/0:2 Not tainted 6.8.0-rc4-syzkaller-00331-gc1ca10ceffbb #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/25/2024
Workqueue: usb_hub_wq hub_event
RIP: 0010:add_uevent_var+0x2d5/0x450 lib/kobject_uevent.c:671
Code: 0f b6 04 27 84 c0 0f 85 28 01 00 00 48 8b 44 24 18 01 18 31 c0 eb 2b e8 09 07 39 f6 90 48 c7 c7 60 e3 c5 8c e8 0c 12 fd f5 90 <0f> 0b 90 90 b8 f4 ff ff ff 49 bc 00 00 00 00 00 fc ff df 4c 8b 6c
RSP: 0018:ffffc90003d3ef20 EFLAGS: 00010246
RAX: 7e434c7b33a2b100 RBX: 00000000000007f5 RCX: ffff88801fd91dc0
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000000
RBP: ffffc90003d3f050 R08: ffffffff81577992 R09: 1ffff920007a7d84
R10: dffffc0000000000 R11: fffff520007a7d85 R12: 000000000000000b
R13: 000000000000000b R14: 1ffff1100fc77443 R15: 1ffff1100fc77543
FS:  0000000000000000(0000) GS:ffff8880b9400000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fc10ead20f0 CR3: 000000002db96000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 kobject_uevent_env+0x520/0x8f0 lib/kobject_uevent.c:588
 device_del+0x812/0xa30 drivers/base/core.c:3832
 input_unregister_device+0xa3/0x100 drivers/input/input.c:2440
 hidinput_disconnect+0x26c/0x300 drivers/hid/hid-input.c:2388
 hid_disconnect drivers/hid/hid-core.c:2280 [inline]
 hid_hw_stop+0x75/0x1e0 drivers/hid/hid-core.c:2329
 ms_remove+0x23/0xa0 drivers/hid/hid-microsoft.c:409
 hid_device_remove+0x225/0x370
 device_remove drivers/base/dd.c:567 [inline]
 __device_release_driver drivers/base/dd.c:1272 [inline]
 device_release_driver_internal+0x4a9/0x7c0 drivers/base/dd.c:1295
 bus_remove_device+0x34f/0x420 drivers/base/bus.c:574
 device_del+0x580/0xa30 drivers/base/core.c:3814
 hid_remove_device drivers/hid/hid-core.c:2867 [inline]
 hid_destroy_device+0x68/0x100 drivers/hid/hid-core.c:2887
 usbhid_disconnect+0x9e/0xc0 drivers/hid/usbhid/hid-core.c:1456
 usb_unbind_interface+0x1d4/0x850 drivers/usb/core/driver.c:461
 device_remove drivers/base/dd.c:569 [inline]
 __device_release_driver drivers/base/dd.c:1272 [inline]
 device_release_driver_internal+0x503/0x7c0 drivers/base/dd.c:1295
 bus_remove_device+0x34f/0x420 drivers/base/bus.c:574
 device_del+0x580/0xa30 drivers/base/core.c:3814
 usb_disable_device+0x3bf/0x850 drivers/usb/core/message.c:1416
 usb_disconnect+0x340/0x950 drivers/usb/core/hub.c:2267
 hub_port_connect drivers/usb/core/hub.c:5323 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:5623 [inline]
 port_event drivers/usb/core/hub.c:5783 [inline]
 hub_event+0x1e62/0x50f0 drivers/usb/core/hub.c:5865
 process_one_work kernel/workqueue.c:2633 [inline]
 process_scheduled_works+0x913/0x1420 kernel/workqueue.c:2706
 worker_thread+0xa5f/0x1000 kernel/workqueue.c:2787
 kthread+0x2ef/0x390 kernel/kthread.c:388
 ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:242
 </TASK>


---
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

^ permalink raw reply

* Fnlock for Lenovo TrackPoint Keyboard II in USB mode
From: Rodrigo Aguilera @ 2024-02-17  9:38 UTC (permalink / raw)
  To: linux-input; +Cc: jkosina, flokli, ValdikSS, benjamin.tissoires

First time for me posting in a kernel mailing list, I was kindly
instructed by ValdikSS on how to do so.

I have a TrackPoint Keyboard II that was working for me normally in
the 5.18 kernel (debian). Then I updated directly to kernel 6.5 and I
started having problems with the middle click pasting text when
scrolling and the fnlock button not changing the F1-12 functions nor
changing the led. I updated to 6.6.8 and the middle click problem is
gone but since the 6.5 update I also lost the ability to change fnlock
with fn+esc. Do you experience the same?

What I currently do is change between android and windows mode on the
keyboard physical switch. Those have different fnlock initial default
state. windows mode locks F1-F12 keys and android locks media keys
like volume up/down. Before the update everything worked fine.

I also found discussions regarding a patched firmware
https://lore.kernel.org/all/?q=trackpoint

So maybe it is not related.

I tried to force pre-5.19 behaviour by using `modprobe -r hid-lenovo`
as ValdikSS suggested but that doesn't recover the fnlock
functionality and I lose scrolling with the trackpoint.

I will summarize the quirks of each mode:

* USB wireless connection and switch in android mode (what I currently use)

-- Fnlock stays locked with the multimedia functions. Esc key led off.

-- Fn has no effect so I can't input F1-F12. That is maybe the reason
why fn+esc doesn't change fnlock

* USB wireless connection and switch in windows mode

-- Fnlock led is on so F1-F12 work and can't be changed

-- Fn + F1-F12 executes multimedia functions

I don't use this mode because I use the multimedia more than F1-F12
but I switch to it manually when some program needs the keys

* Bluetooth connection in windows mode

-- Fnlock can be changed with fn+esc

-- Key combinations like shift+volumeUP don't work while they do in
the USB connection

-- I can't use the keyboard until the bluetooth is up so I have to
input my full disk encryption password with another keyboard or switch
to USB mode.

* Bluetooth connection in windows mode

-- Same as previous mode but fnlock can't be changed


Is there a workaround for any of the issues?

Thanks

^ permalink raw reply

* [PATCH v3] Input: WinWing Orion2 throttle support
From: Ivan Gorinov @ 2024-02-17  5:54 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input, linux-kernel

WinWing Orion2 throttle works with Linux out of box,
but the kernel sees only 16 of 47 buttons on the throttle base.

This module enables all buttons, and also adds LED controls.

Button numbers 0 .. 63 on Orion2 are reserved for throttle grip;
the throttle base buttons have numbers 64 .. 110.

Linux kernel HID subsystem only supports up to 80 buttons.

Remap throttle base buttons to numbers 32 .. 78,
reserving only numbers 0 .. 31 for buttons on the grip handle.

Changes since v2:
   - Fixed automatic line wraps added by mail client

Changes since v1:
   - Fixed formatting of descriptor byte array;
   - Using product codes of Winwing grips in config.

Signed-off-by: Ivan Gorinov <ivan.gorinov@jobyaviation.com>
---
 drivers/hid/Kconfig       |  16 +++
 drivers/hid/Makefile      |   1 +
 drivers/hid/hid-winwing.c | 229 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 246 insertions(+)
 create mode 100644 drivers/hid/hid-winwing.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4c682c650704..08446c89eff6 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1236,6 +1236,22 @@ config HID_WIIMOTE
 	To compile this driver as a module, choose M here: the
 	module will be called hid-wiimote.
 
+config HID_WINWING
+	tristate "WinWing Orion2 throttle support"
+	depends on USB_HID
+	depends on NEW_LEDS
+	depends on LEDS_CLASS
+	help
+	  Support for WinWing Orion2 throttle base with the following grips:
+
+	    * TGRIP-16EX
+	    * TGRIP-18
+
+	  This driver enables all buttons and switches on the throttle base.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called hid-winwing.
+
 config HID_XINMO
 	tristate "Xin-Mo non-fully compliant devices"
 	help
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 082a728eac60..ce71b53ea6c5 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -150,6 +150,7 @@ wacom-objs			:= wacom_wac.o wacom_sys.o
 obj-$(CONFIG_HID_WACOM)		+= wacom.o
 obj-$(CONFIG_HID_WALTOP)	+= hid-waltop.o
 obj-$(CONFIG_HID_WIIMOTE)	+= hid-wiimote.o
+obj-$(CONFIG_HID_WINWING)	+= hid-winwing.o
 obj-$(CONFIG_HID_SENSOR_HUB)	+= hid-sensor-hub.o
 obj-$(CONFIG_HID_SENSOR_CUSTOM_SENSOR)	+= hid-sensor-custom.o
 
diff --git a/drivers/hid/hid-winwing.c b/drivers/hid/hid-winwing.c
new file mode 100644
index 000000000000..d895c82a541d
--- /dev/null
+++ b/drivers/hid/hid-winwing.c
@@ -0,0 +1,229 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * HID driver for WinWing Orion 2 throttle
+ *
+ * Copyright (c) 2023 Ivan Gorinov
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/hidraw.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+
+#define MAX_REPORT 16
+
+struct winwing_led {
+	struct led_classdev cdev;
+	struct hid_device *hdev;
+	int number;
+};
+
+struct winwing_led_info {
+	int number;
+	int max_brightness;
+	const char *led_name;
+};
+
+static struct winwing_led_info led_info[3] = {
+	{ 0, 255, "backlight" },
+	{ 1, 1, "a-a" },
+	{ 2, 1, "a-g" },
+};
+
+struct winwing_drv_data {
+	struct hid_device *hdev;
+	__u8 *report_buf;
+	struct mutex lock;
+	unsigned int num_leds;
+	struct winwing_led leds[];
+};
+
+static int winwing_led_write(struct led_classdev *cdev,
+		enum led_brightness br)
+{
+	struct winwing_led *led = (struct winwing_led *) cdev;
+	struct winwing_drv_data *data = hid_get_drvdata(led->hdev);
+	__u8 *buf = data->report_buf;
+	int ret;
+
+	mutex_lock(&data->lock);
+
+	buf[0] = 0x02;
+	buf[1] = 0x60;
+	buf[2] = 0xbe;
+	buf[3] = 0x00;
+	buf[4] = 0x00;
+	buf[5] = 0x03;
+	buf[6] = 0x49;
+	buf[7] = led->number;
+	buf[8] = br;
+	buf[9] = 0x00;
+	buf[10] = 0;
+	buf[11] = 0;
+	buf[12] = 0;
+	buf[13] = 0;
+
+	ret = hid_hw_output_report(led->hdev, buf, 14);
+
+	mutex_unlock(&data->lock);
+
+	return ret;
+}
+
+static int winwing_init_led(struct hid_device *hdev,
+		struct input_dev *input)
+{
+	struct winwing_drv_data *data;
+	struct winwing_led *led;
+	int ret;
+	int i;
+
+	size_t data_size = struct_size(data, leds, 3);
+
+	data = devm_kzalloc(&hdev->dev, data_size, GFP_KERNEL);
+
+	if (!data)
+		return -ENOMEM;
+
+	data->report_buf = devm_kmalloc(&hdev->dev, MAX_REPORT, GFP_KERNEL);
+
+	if (!data->report_buf)
+		return -ENOMEM;
+
+	for (i = 0; i < 3; i += 1) {
+		struct winwing_led_info *info = &led_info[i];
+
+		led = &data->leds[i];
+		led->hdev = hdev;
+		led->number = info->number;
+		led->cdev.max_brightness = info->max_brightness;
+		led->cdev.brightness_set_blocking = winwing_led_write;
+		led->cdev.flags = LED_HW_PLUGGABLE;
+		led->cdev.name = devm_kasprintf(&hdev->dev, GFP_KERNEL,
+						"%s::%s",
+						dev_name(&input->dev),
+						info->led_name);
+
+		ret = devm_led_classdev_register(&hdev->dev, &led->cdev);
+		if (ret)
+			return ret;
+	}
+
+	hid_set_drvdata(hdev, data);
+
+	return ret;
+}
+
+static int winwing_probe(struct hid_device *hdev,
+		const struct hid_device_id *id)
+{
+	unsigned int minor;
+	int ret;
+
+	ret = hid_parse(hdev);
+	if (ret) {
+		hid_err(hdev, "parse failed\n");
+		return ret;
+	}
+
+	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+	if (ret) {
+		hid_err(hdev, "hw start failed\n");
+		return ret;
+	}
+
+	minor = ((struct hidraw *) hdev->hidraw)->minor;
+
+	return 0;
+}
+
+static int winwing_input_configured(struct hid_device *hdev,
+		struct hid_input *hidinput)
+{
+	int ret;
+
+	ret = winwing_init_led(hdev, hidinput->input);
+
+	if (ret)
+		hid_err(hdev, "led init failed\n");
+
+	return ret;
+}
+
+static __u8 original_rdesc_buttons[] = {
+	0x05, 0x09, 0x19, 0x01, 0x29, 0x6F,
+	0x15, 0x00, 0x25, 0x01, 0x35, 0x00,
+	0x45, 0x01, 0x75, 0x01, 0x95, 0x6F,
+	0x81, 0x02, 0x75, 0x01, 0x95, 0x01,
+	0x81, 0x01
+};
+
+/*
+ * HID report descriptor shows 111 buttons, which exceeds maximum
+ * number of buttons (80) supported by Linux kernel HID subsystem.
+ *
+ * This module skips numbers 32-63, unused on some throttle grips.
+ */
+
+static __u8 *winwing_report_fixup(struct hid_device *hdev, __u8 *rdesc,
+		unsigned int *rsize)
+{
+	int sig_length = sizeof(original_rdesc_buttons);
+	int unused_button_numbers = 32;
+
+	if (*rsize < 34)
+		return rdesc;
+
+	if (memcmp(rdesc + 8, original_rdesc_buttons, sig_length) == 0) {
+
+		/* Usage Maximum */
+		rdesc[13] -= unused_button_numbers;
+
+		/*  Report Count for buttons */
+		rdesc[25] -= unused_button_numbers;
+
+		/*  Report Count for padding [HID1_11, 6.2.2.9] */
+		rdesc[31] += unused_button_numbers;
+
+		hid_info(hdev, "winwing descriptor fixed\n");
+	}
+
+	return rdesc;
+}
+
+static int winwing_raw_event(struct hid_device *hdev,
+		struct hid_report *report, u8 *raw_data, int size)
+{
+	if (size >= 15) {
+		/* Skip buttons 32 .. 63 */
+		memmove(raw_data + 5, raw_data + 9, 6);
+
+		/* Clear the padding */
+		memset(raw_data + 11, 0, 4);
+	}
+
+	return 0;
+}
+
+static const struct hid_device_id winwing_devices[] = {
+	{ HID_USB_DEVICE(0x4098, 0xbe62) },  /* TGRIP-18 */
+	{ HID_USB_DEVICE(0x4098, 0xbe68) },  /* TGRIP-16EX */
+	{}
+};
+
+MODULE_DEVICE_TABLE(hid, winwing_devices);
+
+static struct hid_driver winwing_driver = {
+	.name = "winwing",
+	.id_table = winwing_devices,
+	.probe = winwing_probe,
+	.input_configured = winwing_input_configured,
+	.report_fixup = winwing_report_fixup,
+	.raw_event = winwing_raw_event,
+};
+module_hid_driver(winwing_driver);
+
+MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH v7 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50
From: James Ogletree @ 2024-02-16 23:41 UTC (permalink / raw)
  To: Mark Brown
  Cc: James Ogletree, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones, Jeff LaBundy,
	open list:CIRRUS LOGIC HAPTIC DRIVERS,
	linux-sound@vger.kernel.org, linux-input@vger.kernel.org,
	devicetree@vger.kernel.org
In-Reply-To: <2e22f00a-fbf4-4e6d-8aed-dc78f423c735@sirena.org.uk>

Hi Mark,

Acknowledged on anything unmentioned.

> On Feb 13, 2024, at 1:55 PM, Mark Brown <broonie@kernel.org> wrote:
> 
> On Mon, Feb 12, 2024 at 05:31:11PM +0000, James Ogletree wrote:
> 
>> + switch (clk_src) {
>> + case CS40L50_PLL_REFCLK_BCLK:
>> + ret = cs40l50_get_clk_config(codec->sysclk_rate, &clk_cfg);
>> + if (ret)
>> + return ret;
>> + break;
> 
> We appear to have a set_sysclk() operation but this is saying the sysclk
> is BCLK?  Should the driver be using the bclk_ratio() interface rather
> than set_sysclk(), especially given that the device only appears to
> support either 32.768kHz with no audio or 48kHz and a rather restrictive
> set of multiples of that for the clock?

Yes, I will use the set_bclk_ratio callback in the next version.

> 
>> + case CS40L50_PLL_REFCLK_MCLK:
>> + clk_cfg = CS40L50_PLL_CLK_CFG_32768;
>> + break;
> 
> MCLK is always 32.768kHz?

When we leave audio mode, we need to go back to a constant 32.768kHz clock.

Best,
James


^ permalink raw reply

* Re: [PATCH RFC bpf-next v2 02/10] bpf/helpers: introduce sleepable timers
From: Kui-Feng Lee @ 2024-02-16 16:58 UTC (permalink / raw)
  To: Benjamin Tissoires, Alexei Starovoitov, Daniel Borkmann,
	John Fastabend, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, KP Singh,
	Stanislav Fomichev, Hao Luo, Jiri Olsa, Jiri Kosina,
	Benjamin Tissoires, Jonathan Corbet, Shuah Khan
  Cc: bpf, linux-kernel, linux-input, linux-doc, linux-kselftest
In-Reply-To: <20240214-hid-bpf-sleepable-v2-2-5756b054724d@kernel.org>



On 2/14/24 09:18, Benjamin Tissoires wrote:
> +static void bpf_timer_work_cb(struct work_struct *work)
> +{
> + struct bpf_hrtimer *t = container_of(work, struct bpf_hrtimer, work);
> + struct bpf_map *map = t->map;
> + void *value = t->value;
> + bpf_callback_t callback_fn;
> + void *key;
> + u32 idx;
> +
> + BTF_TYPE_EMIT(struct bpf_timer);
> +
> + rcu_read_lock();
> + callback_fn = rcu_dereference(t->sleepable_cb_fn);
> + rcu_read_unlock();
> + if (!callback_fn)
> + return;
> +
> + /* FIXME: do we need any locking? */
> + if (map->map_type == BPF_MAP_TYPE_ARRAY) {
> + struct bpf_array *array = container_of(map, struct bpf_array, map);
> +
> + /* compute the key */
> + idx = ((char *)value - array->value) / array->elem_size;
> + key = &idx;
> + } else { /* hash or lru */
> + key = value - round_up(map->key_size, 8);
> + }
> +
> + /* FIXME: this crashes the system with
> + * BUG: kernel NULL pointer dereference, address: 000000000000000b
> + */
> + /* callback_fn((u64)(long)map, (u64)(long)key, (u64)(long)value, 0, 0); */
> + /* The verifier checked that return value is zero. */
> +}
> +
>   static DEFINE_PER_CPU(struct bpf_hrtimer *, hrtimer_running);
>   
>   static enum hrtimer_restart bpf_timer_cb(struct hrtimer *hrtimer)
>   {
>   	struct bpf_hrtimer *t = container_of(hrtimer, struct bpf_hrtimer, timer);
> + bpf_callback_t callback_fn, sleepable_cb_fn;
>   	struct bpf_map *map = t->map;
>   	void *value = t->value;
> - bpf_callback_t callback_fn;
>   	void *key;
>   	u32 idx;
>   
>   	BTF_TYPE_EMIT(struct bpf_timer);
> + sleepable_cb_fn = rcu_dereference_check(t->sleepable_cb_fn, 
> rcu_read_lock_bh_held());
> + if (sleepable_cb_fn) {
> + schedule_work(&t->work);
It seems nothing to stop the timer from being free here, right?

You should have a way to make sure the timer & programs here is
still alive when the work is running. For example, it can be flags
to indicate the work is scheduled to prevent the timer from releasing,
and indicate the timer should be free when returning from the callback.

> + goto out;
> + }
> +
>   	callback_fn = rcu_dereference_check(t->callback_fn, rcu_read_lock_bh_held());
>   	if (!callback_fn)

^ permalink raw reply

* [PATCH v5] hid-ft260: Add serial driver
From: Christina Quast @ 2024-02-16 15:03 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: linux-i2c, linux-input, linux-kernel, equinox, Christina Quast,
	Daniel Beer, Michael Zaidman

Adds the serial driver for FT260 USB HID devices, providing direct and
simplified access to UART functionality without the need for FT260 HID
report format knowledge.

This chip implements an UART and I2C interface, but only the latter was
previously supported with a kernel driver. For the UART interface, only
FTDI example code using hidraw from userspace was available.

This commit adds a serial interface /dev/ttyFTx (FT as in FT260), which
implements tty serial driver ops, facilitating baudrate configuration,
data transmission and reception, termios settings.

Signed-off-by: Christina Quast <contact@christina-quast.de>
Co-developed-by: Daniel Beer <daniel.beer@igorinstitute.com>
Signed-off-by: Daniel Beer <daniel.beer@igorinstitute.com>
Co-developed-by: Michael Zaidman <michael.zaidman@gmail.com>
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
Adds the serial driver for FT260 USB HID devices, providing direct and
simplified access to UART functionality without the need for FT260 HID
report format knowledge.

This chip implements an UART and I2C interface, but only the latter was
previously supported with a kernel driver. For the UART interface, only
FTDI example code using hidraw from userspace was available.

This commit adds a serial interface /dev/ttyFTx (FT as in FT260), which
implements tty serial driver ops, facilitating baudrate configuration,
data transmission and reception, termios settings.
---
V1 -> V2: Adressed review comments, added power saving mode quirk
V2 -> V3: Added return 0 in ft260_i2c_probe function
V3 -> V4:
 - Adressed review comments
 - Added get_icount
 - Fixed tty port lifetime bug
V4 -> V5:
  Added and squashed patches from Michael Zaidman:
  [PATCH v1 2/19] hid-ft260: fix Wformat warning
  [PATCH v1 3/19] hid-ft260: fix i2c driver regression in ft260_raw_event
  [PATCH v1 4/19] hid-ft260: remove dead code in ft260_uart_receive_chars
  [PATCH v1 5/19] hid-ft260: fix unprotected write_buf concurrent access
  [PATCH v1 6/19] hid-ft260: uart: enable wakeup workaround
  [PATCH v1 7/19] hid-ft260: depend wakeup workaround activation on uart baud rate
  [PATCH v1 8/19] hid-ft260: depend wakeup workaround activation on eeprom config
  [PATCH v1 9/19] hid-ft260: uart: wakeup device early to not lose rx data
  [PATCH v1 10/19] hid-ft260: uart: do not configure baud rate twice
  [PATCH v1 11/19] hid-ft260: uart: do not disable wakeup workaround twice
  [PATCH v1 12/19] hid-ft260: uart: use kfifo_avail for fifo write room check
  [PATCH v1 13/19] hid-ft260: improve usb interface type detection logic
  [PATCH v1 14/19] hid-ft260: uart: cleanup and refactoring
  [PATCH v1 15/19] hid-ft260: uart: remove FIXME for wake-up workaround
  [PATCH v1 16/19] hid-ft260: uart: suppress unhandled report 0xb1 dmesg
  [PATCH v1 17/19] hid-ft260: uart: arm wake-up timer unconditionally on tty session start
  [PATCH v1 18/19] hid-ft260: uart: fix rx data loss after device reopening
  [PATCH v1 19/19] hid-ft260: uart: improve write performance
---
 drivers/hid/hid-ft260.c | 901 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 835 insertions(+), 66 deletions(-)

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 333341e80b0e..f90ee96990b2 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * hid-ft260.c - FTDI FT260 USB HID to I2C host bridge
+ * FTDI FT260 USB HID to I2C/UART host bridge
  *
  * Copyright (c) 2021, Michael Zaidman <michaelz@xsightlabs.com>
  *
@@ -13,6 +13,12 @@
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/usb.h>
+#include <linux/serial.h>
+#include <linux/serial_core.h>
+#include <linux/kfifo.h>
+#include <linux/tty_flip.h>
+#include <linux/minmax.h>
+#include <asm/unaligned.h>
 
 #ifdef DEBUG
 static int ft260_debug = 1;
@@ -28,8 +34,12 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages");
 			pr_info("%s: " format, __func__, ##arg);	  \
 	} while (0)
 
-#define FT260_REPORT_MAX_LENGTH (64)
-#define FT260_I2C_DATA_REPORT_ID(len) (FT260_I2C_REPORT_MIN + (len - 1) / 4)
+#define FT260_REPORT_MAX_LEN (64)
+#define FT260_DATA_REPORT_ID(min, len) (min + (len - 1) / 4)
+#define FT260_I2C_DATA_REPORT_ID(len) \
+		FT260_DATA_REPORT_ID(FT260_I2C_REPORT_MIN, len)
+#define FT260_UART_DATA_REPORT_ID(len) \
+		FT260_DATA_REPORT_ID(FT260_UART_REPORT_MIN, len)
 
 #define FT260_WAKEUP_NEEDED_AFTER_MS (4800) /* 5s minus 200ms margin */
 
@@ -43,9 +53,10 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages");
  * or optoe limit the i2c reads to 128 bytes. To not block other drivers out
  * of I2C for potentially troublesome amounts of time, we select the maximum
  * read payload length to be 180 bytes.
-*/
+ */
 #define FT260_RD_DATA_MAX (180)
-#define FT260_WR_DATA_MAX (60)
+#define FT260_WR_I2C_DATA_MAX (60)
+#define FT260_WR_UART_DATA_MAX (62)
 
 /*
  * Device interface configuration.
@@ -79,9 +90,10 @@ enum {
 	FT260_I2C_REPORT_MAX		= 0xDE,
 	FT260_GPIO			= 0xB0,
 	FT260_UART_INTERRUPT_STATUS	= 0xB1,
-	FT260_UART_STATUS		= 0xE0,
+	FT260_UART_SETTINGS		= 0xE0,
 	FT260_UART_RI_DCD_STATUS	= 0xE1,
-	FT260_UART_REPORT		= 0xF0,
+	FT260_UART_REPORT_MIN		= 0xF0,
+	FT260_UART_REPORT_MAX		= 0xFE,
 };
 
 /* Feature Out */
@@ -132,6 +144,13 @@ enum {
 	FT260_FLAG_START_STOP_REPEATED	= 0x07,
 };
 
+/* USB interface type values */
+enum {
+	FT260_IFACE_NONE,
+	FT260_IFACE_I2C,
+	FT260_IFACE_UART
+};
+
 #define FT260_SET_REQUEST_VALUE(report_id) ((FT260_FEATURE << 8) | report_id)
 
 /* Feature In reports */
@@ -171,6 +190,18 @@ struct ft260_get_i2c_status_report {
 	u8 reserved;
 } __packed;
 
+struct ft260_get_uart_settings_report {
+	u8 report;		/* FT260_UART_SETTINGS */
+	u8 flow_ctrl;		/* 0 - OFF; 1 - RTS_CTS, 2 - DTR_DSR, */
+				/* 3 - XON_XOFF, 4 - No flow control */
+	/* The baudrate field is unaligned */
+	__le32 baudrate;	/* little endian, 9600 = 0x2580, 19200 = 0x4B00 */
+	u8 data_bit;		/* 7 or 8 */
+	u8 parity;		/* 0: no parity, 1: odd, 2: even, 3: high, 4: low */
+	u8 stop_bit;		/* 0: one stop bit, 2: 2 stop bits */
+	u8 breaking;		/* 0: no break */
+} __packed;
+
 /* Feature Out reports */
 
 struct ft260_set_system_clock_report {
@@ -210,7 +241,7 @@ struct ft260_i2c_write_request_report {
 	u8 address;		/* 7-bit I2C address */
 	u8 flag;		/* I2C transaction condition */
 	u8 length;		/* data payload length */
-	u8 data[FT260_WR_DATA_MAX]; /* data payload */
+	u8 data[FT260_WR_I2C_DATA_MAX]; /* data payload */
 } __packed;
 
 struct ft260_i2c_read_request_report {
@@ -220,12 +251,65 @@ struct ft260_i2c_read_request_report {
 	__le16 length;		/* data payload length */
 } __packed;
 
-struct ft260_i2c_input_report {
-	u8 report;		/* FT260_I2C_REPORT */
+struct ft260_input_report {
+	u8 report;		/* FT260_I2C_REPORT or FT260_UART_REPORT */
 	u8 length;		/* data payload length */
 	u8 data[2];		/* data payload */
 } __packed;
 
+/* UART reports */
+
+struct ft260_uart_write_request_report {
+	u8 report;		/* FT260_UART_REPORT */
+	u8 length;		/* data payload length */
+	u8 data[FT260_WR_UART_DATA_MAX]; /* data payload */
+} __packed;
+
+struct ft260_configure_uart_request_report {
+	u8 report;		/* FT260_SYSTEM_SETTINGS */
+	u8 request;		/* FT260_SET_UART_CONFIG */
+	u8 flow_ctrl;		/* 0: OFF, 1: RTS_CTS, 2: DTR_DSR */
+				/* 3: XON_XOFF, 4: No flow ctrl */
+	/* The baudrate field is unaligned */
+	__le32 baudrate;	/* little endian, 9600 = 0x2580, 19200 = 0x4B00 */
+	u8 data_bit;		/* 7 or 8 */
+	u8 parity;		/* 0: no parity, 1: odd, 2: even, 3: high, 4: low */
+	u8 stop_bit;		/* 0: one stop bit, 2: 2 stop bits */
+	u8 breaking;		/* 0: no break */
+} __packed;
+
+/* UART interface configuration */
+enum {
+	FT260_CFG_FLOW_CTRL_OFF		= 0x00,
+	FT260_CFG_FLOW_CTRL_RTS_CTS	= 0x01,
+	FT260_CFG_FLOW_CTRL_DTR_DSR	= 0x02,
+	FT260_CFG_FLOW_CTRL_XON_XOFF	= 0x03,
+	FT260_CFG_FLOW_CTRL_NONE	= 0x04,
+
+	FT260_CFG_DATA_BITS_7		= 0x07,
+	FT260_CFG_DATA_BITS_8		= 0x08,
+
+	FT260_CFG_PAR_NO		= 0x00,
+	FT260_CFG_PAR_ODD		= 0x01,
+	FT260_CFG_PAR_EVEN		= 0x02,
+	FT260_CFG_PAR_HIGH		= 0x03,
+	FT260_CFG_PAR_LOW		= 0x04,
+
+	FT260_CFG_STOP_ONE_BIT		= 0x00,
+	FT260_CFG_STOP_TWO_BIT		= 0x02,
+
+	FT260_CFG_BREAKING_NO		= 0x00,
+	FT260_CFG_BEAKING_YES		= 0x01,
+
+	FT260_CFG_BAUD_MIN		= 1200,
+	FT260_CFG_BAUD_MAX		= 12000000,
+};
+
+#define FT260_UART_EN_PW_SAVE_BAUD	(4800)
+
+#define UART_COUNT_MAX (4) /* Number of supported UARTs */
+#define XMIT_FIFO_SIZE (PAGE_SIZE)
+
 static const struct hid_device_id ft260_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY,
 			 USB_DEVICE_ID_FT260) },
@@ -236,9 +320,22 @@ MODULE_DEVICE_TABLE(hid, ft260_devices);
 struct ft260_device {
 	struct i2c_adapter adap;
 	struct hid_device *hdev;
+	int iface_type;
+	struct list_head device_list;
+	struct tty_port port;
+	/* tty port index */
+	unsigned int index;
+	struct kfifo xmit_fifo;
+	spinlock_t xmit_fifo_lock;
+	struct uart_icount icount;
+	struct timer_list wakeup_timer;
+	struct work_struct wakeup_work;
+	bool reschedule_work;
+	bool power_saving_en;
 	struct completion wait;
 	struct mutex lock;
-	u8 write_buf[FT260_REPORT_MAX_LENGTH];
+	u8 i2c_wr_buf[FT260_REPORT_MAX_LEN];
+	u8 uart_wr_buf[FT260_REPORT_MAX_LEN];
 	unsigned long need_wakeup_at;
 	u8 *read_buf;
 	u16 read_idx;
@@ -247,8 +344,7 @@ struct ft260_device {
 };
 
 static int ft260_hid_feature_report_get(struct hid_device *hdev,
-					unsigned char report_id, u8 *data,
-					size_t len)
+					u8 report_id, u8 *data, size_t len)
 {
 	u8 *buf;
 	int ret;
@@ -377,8 +473,6 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev,
 
 	ret = ft260_hid_output_report(hdev, data, len);
 	if (ret < 0) {
-		hid_err(hdev, "%s: failed to start transfer, ret %d\n",
-			__func__, ret);
 		ft260_i2c_reset(hdev);
 		return ret;
 	}
@@ -420,7 +514,7 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data,
 	int ret, wr_len, idx = 0;
 	struct hid_device *hdev = dev->hdev;
 	struct ft260_i2c_write_request_report *rep =
-		(struct ft260_i2c_write_request_report *)dev->write_buf;
+		(struct ft260_i2c_write_request_report *)dev->i2c_wr_buf;
 
 	if (len < 1)
 		return -EINVAL;
@@ -428,12 +522,12 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data,
 	rep->flag = FT260_FLAG_START;
 
 	do {
-		if (len <= FT260_WR_DATA_MAX) {
+		if (len <= FT260_WR_I2C_DATA_MAX) {
 			wr_len = len;
 			if (flag == FT260_FLAG_START_STOP)
 				rep->flag |= FT260_FLAG_STOP;
 		} else {
-			wr_len = FT260_WR_DATA_MAX;
+			wr_len = FT260_WR_I2C_DATA_MAX;
 		}
 
 		rep->report = FT260_I2C_DATA_REPORT_ID(wr_len);
@@ -469,7 +563,7 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd,
 	int len = 4;
 
 	struct ft260_i2c_write_request_report *rep =
-		(struct ft260_i2c_write_request_report *)dev->write_buf;
+		(struct ft260_i2c_write_request_report *)dev->i2c_wr_buf;
 
 	if (data_len >= sizeof(rep->data))
 		return -EINVAL;
@@ -489,6 +583,8 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd,
 		  rep->report, addr, cmd, rep->length, len);
 
 	ret = ft260_hid_output_report_check_status(dev, (u8 *)rep, len);
+	if (ret < 0)
+		hid_err(dev->hdev, "%s: failed with %d\n", __func__, ret);
 
 	return ret;
 }
@@ -592,8 +688,7 @@ static int ft260_i2c_write_read(struct ft260_device *dev, struct i2c_msg *msgs)
 		else
 			read_off = *msgs[0].buf;
 
-		pr_info("%s: off %#x rlen %d wlen %d\n", __func__,
-			read_off, rd_len, wr_len);
+		ft260_dbg("off %#x rlen %d wlen %d\n", read_off, rd_len, wr_len);
 	}
 
 	ret = ft260_i2c_write(dev, addr, msgs[0].buf, wr_len,
@@ -782,7 +877,7 @@ static int ft260_get_system_config(struct hid_device *hdev,
 	return 0;
 }
 
-static int ft260_is_interface_enabled(struct hid_device *hdev)
+static int ft260_get_interface_type(struct hid_device *hdev, struct ft260_device *dev)
 {
 	struct ft260_get_system_status_report cfg;
 	struct usb_interface *usbif = to_usb_interface(hdev->dev.parent);
@@ -799,21 +894,25 @@ static int ft260_is_interface_enabled(struct hid_device *hdev)
 	ft260_dbg("i2c_enable: 0x%02x\n", cfg.i2c_enable);
 	ft260_dbg("uart_mode:  0x%02x\n", cfg.uart_mode);
 
+	dev->power_saving_en = cfg.power_saving_en;
+
 	switch (cfg.chip_mode) {
 	case FT260_MODE_ALL:
 	case FT260_MODE_BOTH:
 		if (interface == 1)
-			hid_info(hdev, "uart interface is not supported\n");
+			ret = FT260_IFACE_UART;
 		else
-			ret = 1;
+			ret = FT260_IFACE_I2C;
 		break;
 	case FT260_MODE_UART:
-		hid_info(hdev, "uart interface is not supported\n");
+		ret = FT260_IFACE_UART;
 		break;
 	case FT260_MODE_I2C:
-		ret = 1;
+		ret = FT260_IFACE_I2C;
 		break;
 	}
+
+	dev->iface_type = ret;
 	return ret;
 }
 
@@ -957,51 +1056,518 @@ static const struct attribute_group ft260_attr_group = {
 	}
 };
 
-static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
+static DEFINE_MUTEX(ft260_uart_list_lock);
+static LIST_HEAD(ft260_uart_device_list);
+
+static void ft260_uart_wakeup(struct ft260_device *dev);
+
+static int ft260_get_uart_settings(struct hid_device *hdev,
+				   struct ft260_get_uart_settings_report *cfg)
 {
+	int ret;
+	int len = sizeof(struct ft260_get_uart_settings_report);
+
+	ret = ft260_hid_feature_report_get(hdev, FT260_UART_SETTINGS,
+					   (u8 *)cfg, len);
+	if (ret < 0) {
+		hid_err(hdev, "failed to retrieve uart settings\n");
+		return ret;
+	}
+	return 0;
+}
+
+static void ft260_uart_wakeup_workaraund_enable(struct ft260_device *port,
+						bool enable)
+{
+	if (port->power_saving_en) {
+		port->reschedule_work = enable;
+		ft260_dbg("%s wakeup workaround",
+			  enable ? "activate" : "deactivate");
+	}
+}
+
+static struct ft260_device *ft260_dev_by_index(int index)
+{
+	struct ft260_device *port;
+
+	list_for_each_entry(port, &ft260_uart_device_list, device_list) {
+		if (index == port->index)
+			return port;
+	}
+	return NULL;
+}
+
+static int ft260_uart_add_port(struct ft260_device *port)
+{
+	int index = 0, ret = 0;
 	struct ft260_device *dev;
-	struct ft260_get_chip_version_report version;
-	int ret;
 
-	if (!hid_is_usb(hdev))
-		return -EINVAL;
-
-	dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
-	if (!dev)
+	spin_lock_init(&port->xmit_fifo_lock);
+	if (kfifo_alloc(&port->xmit_fifo, XMIT_FIFO_SIZE, GFP_KERNEL))
 		return -ENOMEM;
 
-	ret = hid_parse(hdev);
-	if (ret) {
-		hid_err(hdev, "failed to parse HID\n");
-		return ret;
+	mutex_lock(&ft260_uart_list_lock);
+	list_for_each_entry(dev, &ft260_uart_device_list, device_list) {
+		if (dev->index != index)
+			break;
+		index++;
 	}
 
-	ret = hid_hw_start(hdev, 0);
-	if (ret) {
-		hid_err(hdev, "failed to start HID HW\n");
-		return ret;
-	}
+	port->index = index;
+	list_add(&port->device_list, &ft260_uart_device_list);
+	mutex_unlock(&ft260_uart_list_lock);
+
+	return ret;
+}
+
+static void ft260_uart_port_put(struct ft260_device *port)
+{
+	tty_port_put(&port->port);
+}
+
+static void ft260_uart_port_remove(struct ft260_device *port)
+{
+	timer_delete_sync(&port->wakeup_timer);
+
+	mutex_lock(&ft260_uart_list_lock);
+	list_del(&port->device_list);
+	mutex_unlock(&ft260_uart_list_lock);
+
+	spin_lock(&port->xmit_fifo_lock);
+	kfifo_free(&port->xmit_fifo);
+	spin_unlock(&port->xmit_fifo_lock);
+
+	mutex_lock(&port->port.mutex);
+	tty_port_tty_hangup(&port->port, false);
+	mutex_unlock(&port->port.mutex);
+
+	ft260_uart_port_put(port);
+}
+
+static struct ft260_device *ft260_uart_port_get(int index)
+{
+	struct ft260_device *port;
+
+	if (index >= UART_COUNT_MAX)
+		return NULL;
+
+	mutex_lock(&ft260_uart_list_lock);
+	port = ft260_dev_by_index(index);
+	if (port)
+		tty_port_get(&port->port);
+	mutex_unlock(&ft260_uart_list_lock);
+
+	return port;
+}
+
+static int ft260_uart_open(struct tty_struct *tty, struct file *filp)
+{
+	int ret;
+	struct ft260_device *port = tty->driver_data;
+
+	ret = tty_port_open(&port->port, tty, filp);
+
+	return ret;
+}
+
+static void ft260_uart_close(struct tty_struct *tty, struct file *filp)
+{
+	struct ft260_device *port = tty->driver_data;
+
+	tty_port_close(&port->port, tty, filp);
+}
+
+static void ft260_uart_hangup(struct tty_struct *tty)
+{
+	struct ft260_device *port = tty->driver_data;
 
-	ret = hid_hw_open(hdev);
-	if (ret) {
-		hid_err(hdev, "failed to open HID HW\n");
-		goto err_hid_stop;
+	tty_port_hangup(&port->port);
+}
+
+static int ft260_uart_transmit_chars(struct ft260_device *port)
+{
+	struct hid_device *hdev = port->hdev;
+	struct kfifo *xmit = &port->xmit_fifo;
+	struct tty_struct *tty;
+	struct ft260_uart_write_request_report *rep;
+	int len, data_len, ret = 0;
+
+	tty = tty_port_tty_get(&port->port);
+
+	data_len = kfifo_len(xmit);
+	if (!tty || !data_len) {
+		ret = -EINVAL;
+		goto tty_out;
 	}
 
-	ret = ft260_hid_feature_report_get(hdev, FT260_CHIP_VERSION,
-					   (u8 *)&version, sizeof(version));
+	rep = (struct ft260_uart_write_request_report *)port->uart_wr_buf;
+
+	do {
+		len = min(data_len, FT260_WR_UART_DATA_MAX);
+
+		rep->report = FT260_UART_DATA_REPORT_ID(len);
+		rep->length = len;
+
+		len = kfifo_out_spinlocked(xmit, rep->data, len, &port->xmit_fifo_lock);
+
+		ret = ft260_hid_output_report(hdev, (u8 *)rep, len + 2);
+		if (ret < 0)
+			goto tty_out;
+
+		data_len -= len;
+		port->icount.tx += len;
+	} while (data_len > 0);
+
+	ret = 0;
+
+tty_out:
+	tty_kref_put(tty);
+	return ret;
+}
+
+static int ft260_uart_receive_chars(struct ft260_device *port, u8 *data, u8 length)
+{
+	int ret;
+
+	ret = tty_insert_flip_string(&port->port, data, length);
+	if (ret != length)
+		ft260_dbg("%d char not inserted to flip buf\n", length - ret);
+
+	port->icount.rx += ret;
+
+	if (ret)
+		tty_flip_buffer_push(&port->port);
+
+	return ret;
+}
+
+static ssize_t ft260_uart_write(struct tty_struct *tty, const u8 *buf, size_t cnt)
+{
+	struct ft260_device *port = tty->driver_data;
+	int len, ret, diff;
+
+	len = kfifo_in_spinlocked(&port->xmit_fifo, buf, cnt, &port->xmit_fifo_lock);
+	ft260_dbg("count: %lu, len: %d", cnt, len);
+
+	ret = ft260_uart_transmit_chars(port);
 	if (ret < 0) {
-		hid_err(hdev, "failed to retrieve chip version\n");
-		goto err_hid_close;
+		ft260_dbg("failed to transmit %d\n", ret);
+		return 0;
 	}
 
-	hid_info(hdev, "chip code: %02x%02x %02x%02x\n",
-		 version.chip_code[0], version.chip_code[1],
-		 version.chip_code[2], version.chip_code[3]);
+	ret = kfifo_len(&port->xmit_fifo);
+	if (ret > 0) {
+		diff = len - ret;
+		ft260_dbg("failed to send %d out of %d bytes\n", diff, len);
+		return diff;
+	}
+
+	return len;
+}
+
+static unsigned int ft260_uart_write_room(struct tty_struct *tty)
+{
+	struct ft260_device *port = tty->driver_data;
+
+	return kfifo_avail(&port->xmit_fifo);
+}
+
+static unsigned int ft260_uart_chars_in_buffer(struct tty_struct *tty)
+{
+	struct ft260_device *port = tty->driver_data;
+
+	return kfifo_len(&port->xmit_fifo);
+}
+
+static int ft260_uart_change_speed(struct ft260_device *port,
+				   struct ktermios *termios,
+				    struct ktermios *old)
+{
+	struct hid_device *hdev = port->hdev;
+	unsigned int baud;
+	struct ft260_configure_uart_request_report req;
+	bool wakeup_workaraund = false;
+	int ret;
+
+	memset(&req, 0, sizeof(req));
+
+	req.report = FT260_SYSTEM_SETTINGS;
+	req.request = FT260_SET_UART_CONFIG;
+
+	switch (termios->c_cflag & CSIZE) {
+	case CS7:
+		req.data_bit = FT260_CFG_DATA_BITS_7;
+		break;
+	case CS5:
+	case CS6:
+		hid_err(hdev, "invalid data bit size, setting a default\n");
+		req.data_bit = FT260_CFG_DATA_BITS_8;
+		termios->c_cflag &= ~CSIZE;
+		termios->c_cflag |= CS8;
+		break;
+	default:
+	case CS8:
+		req.data_bit = FT260_CFG_DATA_BITS_8;
+		break;
+	}
+
+	req.stop_bit = (termios->c_cflag & CSTOPB) ?
+		FT260_CFG_STOP_TWO_BIT : FT260_CFG_STOP_ONE_BIT;
+
+	if (termios->c_cflag & PARENB) {
+		req.parity = (termios->c_cflag & PARODD) ?
+			FT260_CFG_PAR_ODD : FT260_CFG_PAR_EVEN;
+	} else {
+		req.parity = FT260_CFG_PAR_NO;
+	}
+
+	baud = tty_termios_baud_rate(termios);
+	if (baud == 0 || baud < FT260_CFG_BAUD_MIN || baud > FT260_CFG_BAUD_MAX) {
+		struct tty_struct *tty = tty_port_tty_get(&port->port);
+
+		hid_err(hdev, "invalid baud rate %d\n", baud);
+		baud = 9600;
+		tty_encode_baud_rate(tty, baud, baud);
+		tty_kref_put(tty);
+	}
+
+	if (baud > FT260_UART_EN_PW_SAVE_BAUD)
+		wakeup_workaraund = true;
+
+	ft260_uart_wakeup_workaraund_enable(port, wakeup_workaraund);
+
+	put_unaligned_le32(cpu_to_le32(baud), &req.baudrate);
+
+	if (termios->c_cflag & CRTSCTS)
+		req.flow_ctrl = FT260_CFG_FLOW_CTRL_RTS_CTS;
+	else
+		req.flow_ctrl = FT260_CFG_FLOW_CTRL_OFF;
+
+	ft260_dbg("configured termios: flow control: %d, baudrate: %d, ",
+		  req.flow_ctrl, baud);
+	ft260_dbg("data_bit: %d, parity: %d, stop_bit: %d, breaking: %d\n",
+		  req.data_bit, req.parity,
+		  req.stop_bit, req.breaking);
+
+	req.flow_ctrl = FT260_CFG_FLOW_CTRL_NONE;
+	req.breaking = FT260_CFG_BREAKING_NO;
+
+	ret = ft260_hid_feature_report_set(hdev, (u8 *)&req, sizeof(req));
+	if (ret < 0)
+		hid_err(hdev, "failed to change termios: %d\n", ret);
+
+	return ret;
+}
+
+static int ft260_uart_get_icount(struct tty_struct *tty,
+		struct serial_icounter_struct *icount)
+{
+	struct ft260_device *port = tty->driver_data;
+
+	memcpy(icount, &port->icount, sizeof(struct uart_icount));
+
+	return 0;
+}
+
+static void ft260_uart_set_termios(struct tty_struct *tty,
+		const struct ktermios *old_termios)
+{
+	struct ft260_device *port = tty->driver_data;
+
+	ft260_uart_change_speed(port, &tty->termios, NULL);
+}
+
+static int ft260_uart_install(struct tty_driver *driver, struct tty_struct *tty)
+{
+	int idx = tty->index;
+	struct ft260_device *port = ft260_uart_port_get(idx);
+	int ret = tty_standard_install(driver, tty);
+
+	if (ret == 0)
+		/* This is the ref ft260_uart_port get provided */
+		tty->driver_data = port;
+	else
+		ft260_uart_port_put(port);
+
+	return ret;
+}
+
+static void ft260_uart_cleanup(struct tty_struct *tty)
+{
+	struct ft260_device *port = tty->driver_data;
+
+	tty->driver_data = NULL;	/* Bug trap */
+	ft260_uart_port_put(port);
+}
+
+static int ft260_uart_proc_show(struct seq_file *m, void *v)
+{
+	int i;
+
+	seq_printf(m, "ft260 info:1.0 driver%s%s revision:%s\n", "", "", "");
+
+	for (i = 0; i < UART_COUNT_MAX; i++) {
+		struct ft260_device *port = ft260_uart_port_get(i);
+
+		if (port) {
+			seq_printf(m, "%d: uart:FT260", i);
+			if (capable(CAP_SYS_ADMIN)) {
+				seq_printf(m, " tx:%d rx:%d",
+						port->icount.tx, port->icount.rx);
+				if (port->icount.frame)
+					seq_printf(m, " fe:%d",
+							port->icount.frame);
+				if (port->icount.parity)
+					seq_printf(m, " pe:%d",
+							port->icount.parity);
+				if (port->icount.brk)
+					seq_printf(m, " brk:%d",
+							port->icount.brk);
+				if (port->icount.overrun)
+					seq_printf(m, " oe:%d",
+							port->icount.overrun);
+				if (port->icount.cts)
+					seq_printf(m, " cts:%d",
+							port->icount.cts);
+				if (port->icount.dsr)
+					seq_printf(m, " dsr:%d",
+							port->icount.dsr);
+				if (port->icount.rng)
+					seq_printf(m, " rng:%d",
+							port->icount.rng);
+				if (port->icount.dcd)
+					seq_printf(m, " dcd:%d",
+							port->icount.dcd);
+			}
+			ft260_uart_port_put(port);
+			seq_putc(m, '\n');
+		}
+	}
+	return 0;
+}
+
+static const struct tty_operations ft260_uart_ops = {
+	.open			= ft260_uart_open,
+	.close			= ft260_uart_close,
+	.write			= ft260_uart_write,
+	.write_room		= ft260_uart_write_room,
+	.chars_in_buffer	= ft260_uart_chars_in_buffer,
+	.set_termios		= ft260_uart_set_termios,
+	.hangup			= ft260_uart_hangup,
+	.install		= ft260_uart_install,
+	.cleanup		= ft260_uart_cleanup,
+	.proc_show		= ft260_uart_proc_show,
+	.get_icount		= ft260_uart_get_icount,
+};
+
+/*
+ * The FT260 has a "power saving mode" that causes the device to switch
+ * to a 30 kHz oscillator if there's no activity for 5 seconds.
+ * Unfortunately, this mode can only be disabled by reprogramming
+ * internal fuses, which requires an additional programming voltage.
+ *
+ * One effect of this mode is to cause data loss on an Rx line at baud
+ * rates higher than 4800 after being idle for longer than 5 seconds.
+ * We work around this by sending a dummy report at least once per 4.8
+ * seconds if the UART is in use.
+ */
+static void ft260_uart_start_wakeup(struct timer_list *t)
+{
+	struct ft260_device *dev =
+		container_of(t, struct ft260_device, wakeup_timer);
+
+	if (dev->reschedule_work) {
+		schedule_work(&dev->wakeup_work);
+		mod_timer(&dev->wakeup_timer, jiffies +
+			msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS));
+	}
+}
+
+static void ft260_uart_wakeup(struct ft260_device *dev)
+{
+	struct ft260_get_chip_version_report ver;
+	int ret;
+
+	if (dev->reschedule_work) {
+		ret = ft260_hid_feature_report_get(dev->hdev, FT260_CHIP_VERSION,
+						   (u8 *)&ver, sizeof(ver));
+		if (ret < 0)
+			hid_err(dev->hdev, "%s: failed with %d\n", __func__, ret);
+	}
+}
+
+static void ft260_uart_do_wakeup(struct work_struct *work)
+{
+	struct ft260_device *dev =
+		container_of(work, struct ft260_device, wakeup_work);
+
+	ft260_uart_wakeup(dev);
+}
+
+static void ft260_uart_port_shutdown(struct tty_port *tport)
+{
+	struct ft260_device *port =
+		container_of(tport, struct ft260_device, port);
+
+	ft260_uart_wakeup_workaraund_enable(port, false);
+}
+
+static int ft260_uart_port_activate(struct tty_port *tport, struct tty_struct *tty)
+{
+	int ret;
+	int baudrate;
+	struct ft260_get_uart_settings_report cfg;
+	struct ft260_device *port = container_of(tport, struct ft260_device, port);
+
+	set_bit(TTY_IO_ERROR, &tty->flags);
+
+	spin_lock(&port->xmit_fifo_lock);
+	kfifo_reset(&port->xmit_fifo);
+	spin_unlock(&port->xmit_fifo_lock);
+
+	clear_bit(TTY_IO_ERROR, &tty->flags);
+
+	/*
+	 * The port setting may remain intact after session termination.
+	 * Then, when reopening the port without configuring the port
+	 * setting, we need to retrieve the baud rate from the device to
+	 * reactivate the wakeup workaround if needed.
+	 */
+	ret = ft260_get_uart_settings(port->hdev, &cfg);
+	if (ret)
+		return ret;
+
+	baudrate = get_unaligned_le32(&cfg.baudrate);
+	if (baudrate > FT260_UART_EN_PW_SAVE_BAUD)
+		ft260_uart_wakeup_workaraund_enable(port, true);
+
+	ft260_dbg("configured baudrate = %d", baudrate);
+
+	mod_timer(&port->wakeup_timer, jiffies +
+		  msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS));
+
+	return 0;
+}
+
+static void ft260_uart_port_destroy(struct tty_port *tport)
+{
+	struct ft260_device *port =
+		container_of(tport, struct ft260_device, port);
+
+	kfree(port);
+}
+
+static const struct tty_port_operations ft260_uart_port_ops = {
+	.shutdown = ft260_uart_port_shutdown,
+	.activate = ft260_uart_port_activate,
+	.destruct = ft260_uart_port_destroy,
+};
+
+static struct tty_driver *ft260_tty_driver;
 
-	ret = ft260_is_interface_enabled(hdev);
-	if (ret <= 0)
-		goto err_hid_close;
+static int ft260_i2c_probe(struct hid_device *hdev, struct ft260_device *dev)
+{
+	int ret;
 
 	hid_info(hdev, "USB HID v%x.%02x Device [%s] on %s\n",
 		hdev->version >> 8, hdev->version & 0xff, hdev->name,
@@ -1028,7 +1594,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	ret = i2c_add_adapter(&dev->adap);
 	if (ret) {
 		hid_err(hdev, "failed to add i2c adapter\n");
-		goto err_hid_close;
+		return ret;
 	}
 
 	ret = sysfs_create_group(&hdev->dev.kobj, &ft260_attr_group);
@@ -1036,15 +1602,144 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		hid_err(hdev, "failed to create sysfs attrs\n");
 		goto err_i2c_free;
 	}
-
 	return 0;
 
 err_i2c_free:
 	i2c_del_adapter(&dev->adap);
+	return ret;
+}
+
+static int ft260_uart_probe(struct hid_device *hdev, struct ft260_device *dev)
+{
+	struct ft260_configure_uart_request_report req;
+	int ret;
+	struct device *devt;
+
+	INIT_WORK(&dev->wakeup_work, ft260_uart_do_wakeup);
+	// FIXME: are all kfifo access secured by lock? with irq or not?
+	ft260_uart_wakeup_workaraund_enable(dev, true);
+	/* Work not started at this point */
+	timer_setup(&dev->wakeup_timer, ft260_uart_start_wakeup, 0);
+
+	tty_port_init(&dev->port);
+	dev->port.ops = &ft260_uart_port_ops;
+
+	ret = ft260_uart_add_port(dev);
+	if (ret) {
+		hid_err(hdev, "failed to add port\n");
+		return ret;
+	}
+	devt = tty_port_register_device_attr(&dev->port,
+					     ft260_tty_driver,
+					     dev->index, &hdev->dev,
+					     dev, NULL);
+	if (IS_ERR(devt)) {
+		hid_err(hdev, "failed to register tty port\n");
+		ret = PTR_ERR(devt);
+		goto err_register_tty;
+	}
+	hid_info(hdev, "registering device /dev/%s%d\n",
+		ft260_tty_driver->name, dev->index);
+
+	/* Configure UART to 9600n8 */
+	req.report	= FT260_SYSTEM_SETTINGS;
+	req.request	= FT260_SET_UART_CONFIG;
+	req.flow_ctrl	= FT260_CFG_FLOW_CTRL_NONE;
+	put_unaligned_le32(cpu_to_le32(9600), &req.baudrate);
+	req.data_bit	= FT260_CFG_DATA_BITS_8;
+	req.parity	= FT260_CFG_PAR_NO;
+	req.stop_bit	= FT260_CFG_STOP_ONE_BIT;
+	req.breaking	= FT260_CFG_BREAKING_NO;
+
+	ret = ft260_hid_feature_report_set(hdev, (u8 *)&req, sizeof(req));
+	if (ret < 0) {
+		hid_err(hdev, "failed to configure uart: %d\n", ret);
+		goto err_hid_report;
+	}
+
+	return 0;
+
+err_hid_report:
+	tty_port_unregister_device(&dev->port, ft260_tty_driver, dev->index);
+err_register_tty:
+	ft260_uart_port_remove(dev);
+	return ret;
+}
+
+static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+	struct ft260_device *dev;
+	struct ft260_get_chip_version_report version;
+	int ret;
+
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+	/*
+	 * We cannot use devm_kzalloc here because the port has to survive
+	 * until destroy function call.
+	 */
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev) {
+		ret = -ENOMEM;
+		goto alloc_fail;
+	}
+	hid_set_drvdata(hdev, dev);
+
+	ret = hid_parse(hdev);
+	if (ret) {
+		hid_err(hdev, "failed to parse HID\n");
+		goto hid_fail;
+	}
+
+	ret = hid_hw_start(hdev, 0);
+	if (ret) {
+		hid_err(hdev, "failed to start HID HW\n");
+		goto hid_fail;
+	}
+
+	ret = hid_hw_open(hdev);
+	if (ret) {
+		hid_err(hdev, "failed to open HID HW\n");
+		goto err_hid_stop;
+	}
+
+	ret = ft260_hid_feature_report_get(hdev, FT260_CHIP_VERSION,
+					   (u8 *)&version, sizeof(version));
+	if (ret < 0) {
+		hid_err(hdev, "failed to retrieve chip version\n");
+		goto err_hid_close;
+	}
+
+	hid_info(hdev, "chip code: %02x%02x %02x%02x\n",
+		 version.chip_code[0], version.chip_code[1],
+		 version.chip_code[2], version.chip_code[3]);
+
+	ret = ft260_get_interface_type(hdev, dev);
+	if (ret <= FT260_IFACE_NONE)
+		goto err_hid_close;
+
+	hid_set_drvdata(hdev, dev);
+	dev->hdev = hdev;
+
+	mutex_init(&dev->lock);
+	init_completion(&dev->wait);
+
+	if (ret == FT260_IFACE_I2C)
+		ret = ft260_i2c_probe(hdev, dev);
+	else
+		ret = ft260_uart_probe(hdev, dev);
+	if (ret)
+		goto err_hid_close;
+
+	return 0;
+
 err_hid_close:
 	hid_hw_close(hdev);
 err_hid_stop:
 	hid_hw_stop(hdev);
+hid_fail:
+	kfree(dev);
+alloc_fail:
 	return ret;
 }
 
@@ -1055,8 +1750,17 @@ static void ft260_remove(struct hid_device *hdev)
 	if (!dev)
 		return;
 
-	sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
-	i2c_del_adapter(&dev->adap);
+	if (dev->iface_type == FT260_IFACE_UART) {
+		cancel_work_sync(&dev->wakeup_work);
+		tty_port_unregister_device(&dev->port, ft260_tty_driver,
+					   dev->index);
+		ft260_uart_port_remove(dev);
+		/* dev is still needed, so we will free it in _destroy func */
+	} else {
+		sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
+		i2c_del_adapter(&dev->adap);
+		kfree(dev);
+	}
 
 	hid_hw_close(hdev);
 	hid_hw_stop(hdev);
@@ -1066,7 +1770,7 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
 			   u8 *data, int size)
 {
 	struct ft260_device *dev = hid_get_drvdata(hdev);
-	struct ft260_i2c_input_report *xfer = (void *)data;
+	struct ft260_input_report *xfer = (void *)data;
 
 	if (xfer->report >= FT260_I2C_REPORT_MIN &&
 	    xfer->report <= FT260_I2C_REPORT_MAX) {
@@ -1087,9 +1791,19 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
 		if (dev->read_idx == dev->read_len)
 			complete(&dev->wait);
 
-	} else {
-		hid_err(hdev, "unhandled report %#02x\n", xfer->report);
+		return 0;
+
+	} else if (xfer->length > FT260_RD_DATA_MAX) {
+		hid_err(hdev, "received data too long (%d)\n", xfer->length);
+		return -EBADR;
+	} else if (xfer->report >= FT260_UART_REPORT_MIN &&
+		   xfer->report <= FT260_UART_REPORT_MAX) {
+		return ft260_uart_receive_chars(dev, xfer->data, xfer->length);
+	} else if (xfer->report == FT260_UART_INTERRUPT_STATUS) {
+		return 0;
 	}
+	hid_err(hdev, "unhandled report %#02x\n", xfer->report);
+
 	return 0;
 }
 
@@ -1101,7 +1815,62 @@ static struct hid_driver ft260_driver = {
 	.raw_event	= ft260_raw_event,
 };
 
-module_hid_driver(ft260_driver);
-MODULE_DESCRIPTION("FTDI FT260 USB HID to I2C host bridge");
+static int __init ft260_driver_init(void)
+{
+	int ret;
+
+	ft260_tty_driver = tty_alloc_driver(UART_COUNT_MAX,
+		TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV);
+	if (IS_ERR(ft260_tty_driver)) {
+		pr_err("tty_alloc_driver failed: %d\n",
+			(int)PTR_ERR(ft260_tty_driver));
+		return PTR_ERR(ft260_tty_driver);
+	}
+
+	ft260_tty_driver->driver_name = "ft260_ser";
+	ft260_tty_driver->name = "ttyFT";
+	ft260_tty_driver->major = 0;
+	ft260_tty_driver->minor_start = 0;
+	ft260_tty_driver->type = TTY_DRIVER_TYPE_SERIAL;
+	ft260_tty_driver->subtype = SERIAL_TYPE_NORMAL;
+	ft260_tty_driver->init_termios = tty_std_termios;
+	ft260_tty_driver->init_termios.c_cflag = B9600 | CS8 | CREAD | HUPCL | CLOCAL;
+	ft260_tty_driver->init_termios.c_ispeed = 9600;
+	ft260_tty_driver->init_termios.c_ospeed = 9600;
+	tty_set_operations(ft260_tty_driver, &ft260_uart_ops);
+
+	ret = tty_register_driver(ft260_tty_driver);
+	if (ret) {
+		pr_err("tty_register_driver failed: %d\n", ret);
+		goto err_reg_driver;
+	}
+
+	ret = hid_register_driver(&ft260_driver);
+	if (ret) {
+		pr_err("hid_register_driver failed: %d\n", ret);
+		goto err_reg_hid;
+	}
+
+	return 0;
+
+err_reg_hid:
+	tty_unregister_driver(ft260_tty_driver);
+err_reg_driver:
+	tty_driver_kref_put(ft260_tty_driver);
+
+	return ret;
+}
+
+static void __exit ft260_driver_exit(void)
+{
+	hid_unregister_driver(&ft260_driver);
+	tty_unregister_driver(ft260_tty_driver);
+	tty_driver_kref_put(ft260_tty_driver);
+}
+
+module_init(ft260_driver_init);
+module_exit(ft260_driver_exit);
+
+MODULE_DESCRIPTION("FTDI FT260 USB HID to I2C host bridge and TTY driver");
 MODULE_AUTHOR("Michael Zaidman <michael.zaidman@gmail.com>");
 MODULE_LICENSE("GPL v2");

---
base-commit: 54be6c6c5ae8e0d93a6c4641cb7528eb0b6ba478
change-id: 20240216-ft260_review5-c750c39d9674

Best regards,
-- 
Christina Quast <contact@christina-quast.de>


^ permalink raw reply related

* Re: [PATCH RFC bpf-next v2 02/10] bpf/helpers: introduce sleepable timers
From: Benjamin Tissoires @ 2024-02-16 14:58 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Martin KaFai Lau, bpf, linux-kernel, linux-input, linux-doc,
	linux-kselftest, Alexei Starovoitov, Daniel Borkmann,
	John Fastabend, Andrii Nakryiko, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Shuah Khan
In-Reply-To: <87eddccx1q.fsf@toke.dk>

On Feb 16 2024, Toke Høiland-Jørgensen wrote:
> Benjamin Tissoires <bentiss@kernel.org> writes:
> 
> > On Feb 15 2024, Martin KaFai Lau wrote:
> >> On 2/14/24 9:18 AM, Benjamin Tissoires wrote:
> >> > +static void bpf_timer_work_cb(struct work_struct *work)
> >> > +{
> >> > +	struct bpf_hrtimer *t = container_of(work, struct bpf_hrtimer, work);
> >> > +	struct bpf_map *map = t->map;
> >> > +	void *value = t->value;
> >> > +	bpf_callback_t callback_fn;
> >> > +	void *key;
> >> > +	u32 idx;
> >> > +
> >> > +	BTF_TYPE_EMIT(struct bpf_timer);
> >> > +
> >> > +	rcu_read_lock();
> >> > +	callback_fn = rcu_dereference(t->sleepable_cb_fn);
> >> > +	rcu_read_unlock();
> >> 
> >> I took a very brief look at patch 2. One thing that may worth to ask here,
> >> the rcu_read_unlock() seems to be done too early. It is protecting the
> >> t->sleepable_cb_fn (?), so should it be done after finished using the
> >> callback_fn?
> >
> > Probably :)
> >
> > TBH, everytime I work with RCUs I spent countless hours trying to
> > re-understand everything, and in this case I'm currently in the "let's
> > make it work" process than fixing concurrency issues.
> > I still gave it a shot in case it solves my issue, but no, I still have
> > the crash.
> >
> > But given that callback_fn might sleep, isn't it an issue to keep the
> > RCU_reader lock so long? (we don't seem to call synchronize_rcu() so it
> > might be fine, but I'd like the confirmation from someone else).
> 
> You're right, it isn't. From the RCU/checklist.rst doc:
> 
> 13.	Unlike most flavors of RCU, it *is* permissible to block in an
> 	SRCU read-side critical section (demarked by srcu_read_lock()
> 	and srcu_read_unlock()), hence the "SRCU": "sleepable RCU".
> 	Please note that if you don't need to sleep in read-side critical
> 	sections, you should be using RCU rather than SRCU, because RCU
> 	is almost always faster and easier to use than is SRCU.
> 
> So we can't use the regular RCU protection for the callback in this
> usage. We'll need to either convert it to SRCU, or add another
> protection mechanism to make sure the callback function is not freed
> from under us (like a refcnt). I suspect the latter may be simpler (from
> reading the rest of that documentation around SRCU.

Currently I'm thinking at also incrementing the ->prog held in the
bpf_hrtimer which should prevent the callback to be freed, if I'm not wrong.
Then I should be able to just release the rcu_read_unlock before calling
the actual callback. And then put the ref on ->prog once done.

But to be able to do that I might need to protect ->prog with an RCU
too.

> 
> >> A high level design question. The intention of the new
> >> bpf_timer_set_sleepable_cb() kfunc is actually to delay work to a workqueue.
> >> It is useful to delay work from the bpf_timer_cb and it may also useful to
> >> delay work from other bpf running context (e.g. the networking hooks like
> >> "tc"). The bpf_timer_set_sleepable_cb() seems to be unnecessary forcing
> >> delay-work must be done in a bpf_timer_cb.
> >
> > Basically I'm just a monkey here. I've been told that I should use
> > bpf_timer[0]. But my implementation is not finished, as Alexei mentioned
> > that we should bypass hrtimer if I'm not wrong [1].
> 
> I don't think getting rid of the hrtimer in favour of
> schedule_delayed_work() makes any sense. schedule_delayed_work() does
> exactly the same as you're doing in this version of the patch: it
> schedules a timer callback, and calls queue_work() from inside that
> timer callback. It just uses "regular" timers instead of hrtimers. So I
> don't think there's any performance benefit from using that facility; on
> the contrary, it would require extra logic to handle cancellation etc;
> might as well just re-use the existing hrtimer-based callback logic we
> already have, and do a schedule_work() from the hrtimer callback like
> you're doing now.

I agree that we can nicely emulate delayed_timer with the current patch
series. However, if I understand Alexei's idea (and Martin's) there are
cases where we just want schedule_work(), without any timer involved.
That makes a weird timer (with a delay always equal to 0), but it would
allow to satisfy those latency issues.

So (and this also answers your second email today) I'm thinking at:
- have multiple flags to control the timer (with dedicated timer_cb
  kernel functions):
  - BPF_F_TIMER_HRTIMER (default)
  - BPF_F_TIMER_WORKER (no timer, just workqueue)
  - BPF_F_TIMER_DELAYED_WORKER (hrtimer + workqueue, or actual
    delayed_work, but that's re-implementing stuffs)
- have bpf_timer_set_callback() and bpf_timer_set_sleepable_cb() strictly
  equivalent in terms of processing, but the latter just instructs the
  verifier that the callback can be sleepable (so calling
  bpf_timer_set_callback() on a BPF_F_TIMER_DELAYED_WORKER is fine as
  long as the target callback is using non sleepable kfuncs).
- ensure that bpf_timer_set_sleepable_cb() is invalid when called with
  a non sleepable timer flag.
- ensure that when the bpf_timer has no hrtimer we also return -EINVAL
  when a delay is given in bpf_timer_start().

Actually, BPF_F_TIMER_DELAYED_WORKER could be just a combination of
(BPF_F_TIMER_HRTIMER | BPF_F_TIMER_WORKER) which would reflect the
reality of how things are implemented.

Thinking through this a little bit more, maybe we should have
BPF_F_TIMER_SLEEPABLE instead of BPF_F_TIMER_WORKER. We can still have
BPF_F_TIMER_SLEEPABLE_BH when WQ_BH gets merged. _SLEEPABLE allows to
not bind the flag to the implementation...

Cheers,
Benjamin

^ permalink raw reply

* Re: [PATCH RFC bpf-next v2 02/10] bpf/helpers: introduce sleepable timers
From: Toke Høiland-Jørgensen @ 2024-02-16 14:20 UTC (permalink / raw)
  To: Benjamin Tissoires, Alexei Starovoitov, Daniel Borkmann,
	John Fastabend, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, KP Singh,
	Stanislav Fomichev, Hao Luo, Jiri Olsa, Jiri Kosina,
	Benjamin Tissoires, Jonathan Corbet, Shuah Khan
  Cc: bpf, linux-kernel, linux-input, linux-doc, linux-kselftest,
	Benjamin Tissoires
In-Reply-To: <20240214-hid-bpf-sleepable-v2-2-5756b054724d@kernel.org>

Just one comment on one of your FIXMEs:

> +	rcu_assign_pointer(t->sleepable_cb_fn, NULL);
> +	/* FIXME: probably do something about the SLEEPABLE flag */

I guess we should store the flag in the timer struct somewhere, and then
restrict the set_callback() functions so that the regular variant only
works without the flag set, and the _sleepable version only works if the
flag is set? Otherwise we may end up calling a non-sleepable function in
sleepable context or vise versa...

-Toke


^ permalink raw reply


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