Linux Input/HID development
 help / color / mirror / Atom feed
* RE: must hold the driver_input_lock in hid_debug_rdesc_show
From: He, Bo @ 2019-03-20  1:50 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: benjamin.tissoires@redhat.com, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org, Zhang, Jun, Zhang, Yanmin
In-Reply-To: <nycvar.YFH.7.76.1903191541410.19912@cbobk.fhfr.pm>

thanks, without the patch we can reproduce with the way in 10 hours Suspend/Resume test, with the test, we can't reproduce for 30 hours.

-----Original Message-----
From: Jiri Kosina <jikos@kernel.org> 
Sent: Tuesday, March 19, 2019 10:42 PM
To: He, Bo <bo.he@intel.com>
Cc: benjamin.tissoires@redhat.com; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Zhang, Jun <jun.zhang@intel.com>; Zhang, Yanmin <yanmin.zhang@intel.com>
Subject: Re: must hold the driver_input_lock in hid_debug_rdesc_show

On Thu, 14 Mar 2019, He, Bo wrote:

> we see the below kernel panic logs when run suspend resume test with 
> usb mouse and usb keyboard connected.
> 
> the scenario is the userspace call the hid_debug_rdesc_show to dump 
> the input device while the device is removed. the patch hold the 
> driver_input_lock to avoid the race.
> 
> [ 5381.757295] selinux: SELinux:  Could not stat
> /sys/devices/pci0000:00/0000:00:15.0/usb1/1-2/1-2:1.0/0003:03F0:0325.0320/input/input960/input960::scrolllock:
> No such file or directory.
> [ 5382.636498] BUG: unable to handle kernel paging request at 0000000783316040
> [ 5382.651950] CPU: 1 PID: 1512 Comm: getevent Tainted: G     U     O 4.19.20-quilt-2e5dc0ac-00029-gc455a447dd55 #1
> [ 5382.663797] RIP: 0010:hid_dump_device+0x9b/0x160 [ 5382.758853] 
> Call Trace:
> [ 5382.761581]  hid_debug_rdesc_show+0x72/0x1d0 [ 5382.766343]  
> seq_read+0xe0/0x410 [ 5382.769941]  full_proxy_read+0x5f/0x90 [ 
> 5382.774121]  __vfs_read+0x3a/0x170 [ 5382.788392]  
> vfs_read+0xa0/0x150 [ 5382.791984]  ksys_read+0x58/0xc0 [ 5382.801404]  
> __x64_sys_read+0x1a/0x20 [ 5382.805483]  do_syscall_64+0x55/0x110 [ 
> 5382.809559]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> Signed-off-by: he, bo <bo.he@intel.com>
> Signed-off-by: "Zhang, Jun" <jun.zhang@intel.com>

I rewrote the changelog to explain the situation a bit more clearly, and applied. Thanks,

--
Jiri Kosina
SUSE Labs

^ permalink raw reply

* [PATCH] HID: logitech: Handle 0 scroll events for the m560
From: Peter Hutterer @ 2019-03-19 22:48 UTC (permalink / raw)
  To: linux-input
  Cc: Benjamin Tissoires, Aimo Metsälä, nlopezcasad,
	Jiri Kosina, linux-kernel

hidpp_scroll_counter_handle_scroll() doesn't expect a 0-value scroll event, it
gets interpreted as a negative scroll direction event. This can cause scroll
direction resets and thus broken scrolling.

Reported-and-tested-by: Aimo Metsälä <aimetsal@outlook.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
 drivers/hid/hid-logitech-hidpp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 15ed6177a7a3..f040c8a7f9a9 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -2608,8 +2608,9 @@ static int m560_raw_event(struct hid_device *hdev, u8 *data, int size)
 		input_report_rel(mydata->input, REL_Y, v);
 
 		v = hid_snto32(data[6], 8);
-		hidpp_scroll_counter_handle_scroll(
-				&hidpp->vertical_wheel_counter, v);
+		if (v != 0)
+			hidpp_scroll_counter_handle_scroll(
+					&hidpp->vertical_wheel_counter, v);
 
 		input_sync(mydata->input);
 	}
-- 
2.20.1

^ permalink raw reply related

* [PATCH 1/1] hid: hid-sensor-custom: simplify getting .driver_data
From: Wolfram Sang @ 2019-03-19 16:36 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-renesas-soc, Wolfram Sang, Jiri Kosina, Jonathan Cameron,
	Srinivas Pandruvada, Benjamin Tissoires, linux-input, linux-iio

We should get 'driver_data' from 'struct device' directly. Going via
platform_device is an unneeded step back and forth.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Build tested only. buildbot is happy.

 drivers/hid/hid-sensor-custom.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
index bb012bc032e0..462e653a7bbb 100644
--- a/drivers/hid/hid-sensor-custom.c
+++ b/drivers/hid/hid-sensor-custom.c
@@ -157,8 +157,7 @@ static int usage_id_cmp(const void *p1, const void *p2)
 static ssize_t enable_sensor_show(struct device *dev,
 				  struct device_attribute *attr, char *buf)
 {
-	struct platform_device *pdev = to_platform_device(dev);
-	struct hid_sensor_custom *sensor_inst = platform_get_drvdata(pdev);
+	struct hid_sensor_custom *sensor_inst = dev_get_drvdata(dev);
 
 	return sprintf(buf, "%d\n", sensor_inst->enable);
 }
@@ -237,8 +236,7 @@ static ssize_t enable_sensor_store(struct device *dev,
 				   struct device_attribute *attr,
 				   const char *buf, size_t count)
 {
-	struct platform_device *pdev = to_platform_device(dev);
-	struct hid_sensor_custom *sensor_inst = platform_get_drvdata(pdev);
+	struct hid_sensor_custom *sensor_inst = dev_get_drvdata(dev);
 	int value;
 	int ret = -EINVAL;
 
@@ -283,8 +281,7 @@ static const struct attribute_group enable_sensor_attr_group = {
 static ssize_t show_value(struct device *dev, struct device_attribute *attr,
 			  char *buf)
 {
-	struct platform_device *pdev = to_platform_device(dev);
-	struct hid_sensor_custom *sensor_inst = platform_get_drvdata(pdev);
+	struct hid_sensor_custom *sensor_inst = dev_get_drvdata(dev);
 	struct hid_sensor_hub_attribute_info *attribute;
 	int index, usage, field_index;
 	char name[HID_CUSTOM_NAME_LENGTH];
@@ -392,8 +389,7 @@ static ssize_t show_value(struct device *dev, struct device_attribute *attr,
 static ssize_t store_value(struct device *dev, struct device_attribute *attr,
 			   const char *buf, size_t count)
 {
-	struct platform_device *pdev = to_platform_device(dev);
-	struct hid_sensor_custom *sensor_inst = platform_get_drvdata(pdev);
+	struct hid_sensor_custom *sensor_inst = dev_get_drvdata(dev);
 	int index, field_index, usage;
 	char name[HID_CUSTOM_NAME_LENGTH];
 	int value;
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH v2 2/2] Input: add Apple SPI keyboard and trackpad driver.
From: Andy Shevchenko @ 2019-03-19 14:59 UTC (permalink / raw)
  To: Life is hard, and then you die
  Cc: Dmitry Torokhov, Henrik Rydberg, Lukas Wunner, Federico Lorenzi,
	linux-input, linux-kernel
In-Reply-To: <20190227072958.GA10349@innovation.ch>

On Tue, Feb 26, 2019 at 11:29:58PM -0800, Life is hard, and then you die wrote:
> On Tue, Feb 26, 2019 at 11:20:59AM +0200, Andy Shevchenko wrote:
> > On Thu, Feb 21, 2019 at 02:56:09AM -0800, Ronald Tschalär wrote:

> > > +config KEYBOARD_APPLESPI
> > > +	tristate "Apple SPI keyboard and trackpad"
> > 
> > > +	depends on ACPI && SPI && EFI
> > 
> > I would rather want to see separate line for SPI...
> > 
> > > +	depends on X86 || COMPILE_TEST
> > 
> > ...like here
> > 
> > 	depends on SPI
> 
> Sure. Generally, what is the criteria/rule here for splitting
> conjunctions into separate 'depends'?

Rule of common sense.

For example UEFI and ACPI may have some relations, SPI and ACPI kinda
orthogonal.

> > + #define DEV(applespi)           (&(applespi)->spi->dev)

> > > +	if (memcmp(applespi->tx_status, status_ok, APPLESPI_STATUS_SIZE)) {
> > 
> > > +		dev_warn(DEV(applespi), "Error writing to device: %*ph\n",
> > 
> > Hmm... DEV() is too generic name for custom macro. And frankly I don't think
> > it's good to have in the first place.
> 
> Yeah, I've been having trouble coming up with a better (but still
> succinct) name - CORE_DEV()? RAW_DEV()? DEV_OF()? However, because
> this expression is used in many places throughout the driver (mostly,
> but not only, for logging statements) I feel like it's good to factor
> it out. But I'll defer to your .

Please remove this macro for good. Otherwise big subsystems / drivers usually do something like

#define foo_err(...)	dev_err(...)
...

Don't know if it would help here, the driver is standalone and not so big.

> > > +static void
> > > +applespi_remap_fn_key(struct keyboard_protocol *keyboard_protocol)
> > > +{
> > > +	unsigned char tmp;
> > 
> > > +	unsigned long *modifiers =
> > > +			(unsigned long *)&keyboard_protocol->modifiers;
> > 
> > I would leave it on one online despite checkpatch warning (also, instead of
> > (unsigned long *) the (void *) might be used as a small trick).
> > 
> > > +
> > > +	if (!fnremap || fnremap > ARRAY_SIZE(applespi_controlcodes) ||
> > > +	    !applespi_controlcodes[fnremap - 1])
> > > +		return;
> > > +
> > > +	tmp = keyboard_protocol->fn_pressed;
> > > +	keyboard_protocol->fn_pressed = test_bit(fnremap - 1, modifiers);
> > > +	if (tmp)
> > 
> > > +		__set_bit(fnremap - 1, modifiers);
> > > +	else
> > > +		__clear_bit(fnremap - 1, modifiers);
> > 
> > Oh, this is not good. modifiers should be really unsigned long bounary,
> > otherwise it is potential overflow.
> > 
> > Best to fix is to define them as unsigned long in the first place.
> 
> Can't do that directly, because keyboard_protocol->modifiers is a
> field in the data received from the device, i.e. defined by that
> protocol. Instead I could make a copy of the modifiers and pass that
> around separately (i.e. in addition to the keyboard_protocol struct).
> 
> However, the implied size assertions here would basically still apply:
> 
>   MAX_MODIFIERS == sizeof(keyboard_protocol->modifiers) * 8
>   ARRAY_SIZE(applespi_controlcodes) == sizeof(keyboard_protocol->modifiers) * 8
> 
> (hmm, MAX_MODIFIERS is really redundant - getting rid of it...)
> 
> Would using compiletime_assert()'s be an acceptable alternate approach
> here? It would serve to both document the size constraint and to
> protect against overflow due to an error in some future edit. E.g.
> 
>  applespi_remap_fn_key(struct keyboard_protocol *keyboard_protocol)
>  {
>         unsigned char tmp;
>         unsigned long *modifiers = (void *)&keyboard_protocol->modifiers;
> +
> +       compiletime_assert(ARRAY_SIZE(applespi_controlcodes) ==
> +                          sizeof_field(struct keyboard_protocol, modifiers) * 8,
> +                          "applespi_controlcodes has wrong number of entries");
>  
>         if (!fnremap || fnremap > ARRAY_SIZE(applespi_controlcodes) ||
>             !applespi_controlcodes[fnremap - 1])
>  		return;
>  
>  	tmp = keyboard_protocol->fn_pressed;
>  	keyboard_protocol->fn_pressed = test_bit(fnremap - 1, modifiers);
>  	if (tmp)
>  
>  		__set_bit(fnremap - 1, modifiers);
>  	else
>  		__clear_bit(fnremap - 1, modifiers);
>  }

Perhaps, simple

	__set_bit(b, x)		->	x |= BIT(b);
	__clear_bit(b, x)	->	x &= ~BIT(b);

?

> > > +}

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: must hold the driver_input_lock in hid_debug_rdesc_show
From: Jiri Kosina @ 2019-03-19 14:42 UTC (permalink / raw)
  To: He, Bo
  Cc: benjamin.tissoires@redhat.com, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org, Zhang, Jun, Zhang, Yanmin
In-Reply-To: <CD6925E8781EFD4D8E11882D20FC406D52A6168C@SHSMSX104.ccr.corp.intel.com>

On Thu, 14 Mar 2019, He, Bo wrote:

> we see the below kernel panic logs when run suspend resume test with
> usb mouse and usb keyboard connected.
> 
> the scenario is the userspace call the hid_debug_rdesc_show to dump
> the input device while the device is removed. the patch
> hold the driver_input_lock to avoid the race.
> 
> [ 5381.757295] selinux: SELinux:  Could not stat
> /sys/devices/pci0000:00/0000:00:15.0/usb1/1-2/1-2:1.0/0003:03F0:0325.0320/input/input960/input960::scrolllock:
> No such file or directory.
> [ 5382.636498] BUG: unable to handle kernel paging request at 0000000783316040
> [ 5382.651950] CPU: 1 PID: 1512 Comm: getevent Tainted: G     U     O 4.19.20-quilt-2e5dc0ac-00029-gc455a447dd55 #1
> [ 5382.663797] RIP: 0010:hid_dump_device+0x9b/0x160
> [ 5382.758853] Call Trace:
> [ 5382.761581]  hid_debug_rdesc_show+0x72/0x1d0
> [ 5382.766343]  seq_read+0xe0/0x410
> [ 5382.769941]  full_proxy_read+0x5f/0x90
> [ 5382.774121]  __vfs_read+0x3a/0x170
> [ 5382.788392]  vfs_read+0xa0/0x150
> [ 5382.791984]  ksys_read+0x58/0xc0
> [ 5382.801404]  __x64_sys_read+0x1a/0x20
> [ 5382.805483]  do_syscall_64+0x55/0x110
> [ 5382.809559]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> Signed-off-by: he, bo <bo.he@intel.com>
> Signed-off-by: "Zhang, Jun" <jun.zhang@intel.com>

I rewrote the changelog to explain the situation a bit more clearly, and 
applied. Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH 2/6] input: da9063_onkey: remove platform_data support
From: Simon Horman @ 2019-03-19 12:49 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-kernel, linux-renesas-soc, Lee Jones, Mark Brown,
	Support Opensource, Dmitry Torokhov, linux-input
In-Reply-To: <20190318154759.21978-3-wsa+renesas@sang-engineering.com>

On Mon, Mar 18, 2019 at 04:47:54PM +0100, Wolfram Sang wrote:
> There are no in-kernel users anymore, so remove this outdated interface.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> ---
>  drivers/input/misc/da9063_onkey.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/input/misc/da9063_onkey.c b/drivers/input/misc/da9063_onkey.c
> index 3e9c353d82ef..e3a273c74123 100644
> --- a/drivers/input/misc/da9063_onkey.c
> +++ b/drivers/input/misc/da9063_onkey.c
> @@ -22,7 +22,6 @@
>  #include <linux/regmap.h>
>  #include <linux/of.h>
>  #include <linux/mfd/da9063/core.h>
> -#include <linux/mfd/da9063/pdata.h>
>  #include <linux/mfd/da9063/registers.h>
>  #include <linux/mfd/da9062/core.h>
>  #include <linux/mfd/da9062/registers.h>
> @@ -201,8 +200,6 @@ static void da9063_cancel_poll(void *data)
>  
>  static int da9063_onkey_probe(struct platform_device *pdev)
>  {
> -	struct da9063 *da9063 = dev_get_drvdata(pdev->dev.parent);
> -	struct da9063_pdata *pdata = dev_get_platdata(da9063->dev);
>  	struct da9063_onkey *onkey;
>  	const struct of_device_id *match;
>  	int irq;
> @@ -229,12 +226,8 @@ static int da9063_onkey_probe(struct platform_device *pdev)
>  		return -ENXIO;
>  	}
>  
> -	if (pdata)
> -		onkey->key_power = pdata->key_power;
> -	else
> -		onkey->key_power =
> -			!of_property_read_bool(pdev->dev.of_node,
> -					       "dlg,disable-key-power");
> +	onkey->key_power = !of_property_read_bool(pdev->dev.of_node,
> +						  "dlg,disable-key-power");
>  
>  	onkey->input = devm_input_allocate_device(&pdev->dev);
>  	if (!onkey->input) {
> -- 
> 2.11.0
> 

^ permalink raw reply

* Re: [PATCH 3/4] input: da9063_onkey: convert header to SPDX
From: Simon Horman @ 2019-03-19 12:09 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-kernel, linux-renesas-soc, Lee Jones, Mark Brown,
	Support Opensource, Dmitry Torokhov, linux-input
In-Reply-To: <20190318155728.28365-4-wsa+renesas@sang-engineering.com>

On Mon, Mar 18, 2019 at 04:57:26PM +0100, Wolfram Sang wrote:
> Covnert the header of the source file to SPDX.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> ---
>  drivers/input/misc/da9063_onkey.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/input/misc/da9063_onkey.c b/drivers/input/misc/da9063_onkey.c
> index 3e9c353d82ef..7b8ae905a6fb 100644
> --- a/drivers/input/misc/da9063_onkey.c
> +++ b/drivers/input/misc/da9063_onkey.c
> @@ -1,16 +1,7 @@
> +// SPDX-License-Identifier: GPL-2.0+
>  /*
>   * OnKey device driver for DA9063, DA9062 and DA9061 PMICs
>   * Copyright (C) 2015  Dialog Semiconductor Ltd.
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License
> - * as published by the Free Software Foundation; either version 2
> - * of the License, or (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
>   */
>  
>  #include <linux/module.h>
> -- 
> 2.11.0
> 

^ permalink raw reply

* Re: [PATCH v2 00/10] HID: intel-ish-hid: Clean up external interfaces
From: Jiri Kosina @ 2019-03-19 10:59 UTC (permalink / raw)
  To: Srinivas Pandruvada; +Cc: benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <20190318191428.6527-1-srinivas.pandruvada@linux.intel.com>

On Mon, 18 Mar 2019, Srinivas Pandruvada wrote:

> NOT FOR kernel v5.1.
> 
> v2
> - Rebased on the top of
> "HID: intel-ish: enable raw interface to HID devices on ISH"
> which Jiri already applied.
> - Also exported devc pointer for DMA

I've already reviewed v1 before, so I just went through these highlighted 
changes above, and applied.

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH v3] hid: logitech: check the return value of create_singlethread_workqueue
From: Jiri Kosina @ 2019-03-19 10:48 UTC (permalink / raw)
  To: Kangjie Lu; +Cc: pakki001, Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20190314052402.31448-1-kjlu@umn.edu>

On Thu, 14 Mar 2019, Kangjie Lu wrote:

> create_singlethread_workqueue may fail and return NULL. The fix
> checks if it is NULL to avoid NULL pointer dereference.
> Also, the fix moves the call of create_singlethread_workqueue
> earlier to avoid resource-release issues.
> 
> --
> V3: do not introduce memory leaks.
> 
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>

The signoff has to be in the commit log, not in the "notes" area. I've 
fixed that and applied.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* [PATCH v2 10/10] HID: intel-ish-hid: Add interface function for PCI device pointer
From: Srinivas Pandruvada @ 2019-03-18 19:14 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190318191428.6527-1-srinivas.pandruvada@linux.intel.com>

Instead of directly accessing PCI device poitner via struct ishtp_cl,
create interface function for same. This is required for DMA transfer.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ishtp/bus.c | 13 +++++++++++++
 include/linux/intel-ish-client-if.h   |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 66a485a41481..fb8ca12955b4 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -855,6 +855,19 @@ struct device *ishtp_device(struct ishtp_cl_device *device)
 }
 EXPORT_SYMBOL(ishtp_device);
 
+/**
+ * ishtp_get_pci_device() - Return PCI device dev pointer
+ * This interface is used to return PCI device pointer
+ * from ishtp_cl_device instance.
+ *
+ * Return: device *.
+ */
+struct device *ishtp_get_pci_device(struct ishtp_cl_device *device)
+{
+	return device->ishtp_dev->devc;
+}
+EXPORT_SYMBOL(ishtp_get_pci_device);
+
 /**
  * ishtp_trace_callback() - Return trace callback
  *
diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
index e98bfbb1e07e..16255c2ca2f4 100644
--- a/include/linux/intel-ish-client-if.h
+++ b/include/linux/intel-ish-client-if.h
@@ -77,6 +77,8 @@ int ishtp_register_event_cb(struct ishtp_cl_device *device,
 struct device *ishtp_device(struct ishtp_cl_device *cl_device);
 /* Trace interface for clients */
 void *ishtp_trace_callback(struct ishtp_cl_device *cl_device);
+/* Get device pointer of PCI device for DMA acces */
+struct device *ishtp_get_pci_device(struct ishtp_cl_device *cl_device);
 
 struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device);
 void ishtp_cl_free(struct ishtp_cl *cl);
-- 
2.17.2

^ permalink raw reply related

* [PATCH v2 09/10] HID: intel-ish-hid: Use the new interface functions in HID ish client
From: Srinivas Pandruvada @ 2019-03-18 19:14 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190318191428.6527-1-srinivas.pandruvada@linux.intel.com>

Only include intel-ish-client-if.h, which has all interfaces required to
implment ISHTP client. There is no longer any direct field access from
core ISHTP only include files.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ishtp-hid-client.c | 68 +++++++++-----------
 drivers/hid/intel-ish-hid/ishtp-hid.c        |  2 -
 drivers/hid/intel-ish-hid/ishtp-hid.h        |  6 +-
 3 files changed, 35 insertions(+), 41 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index 4afb39713213..56777a43e69c 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -17,8 +17,6 @@
 #include <linux/hid.h>
 #include <linux/intel-ish-client-if.h>
 #include <linux/sched.h>
-#include "ishtp/ishtp-dev.h"
-#include "ishtp/client.h"
 #include "ishtp-hid.h"
 
 /* Rx ring buffer pool size */
@@ -40,7 +38,7 @@ static void report_bad_packet(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
 			      size_t cur_pos,  size_t payload_len)
 {
 	struct hostif_msg *recv_msg = recv_buf;
-	struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+	struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
 
 	dev_err(cl_data_to_dev(client_data), "[hid-ish]: BAD packet %02X\n"
 		"total_bad=%u cur_pos=%u\n"
@@ -77,7 +75,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
 	struct report_list *reports_list;
 	char *reports;
 	size_t report_len;
-	struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+	struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
 	int curr_hid_dev = client_data->cur_hid_dev;
 	struct ishtp_hid_data *hid_data = NULL;
 	struct hid_device *hid = NULL;
@@ -93,7 +91,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
 				(unsigned int)data_len,
 				(unsigned int)sizeof(struct hostif_msg_hdr));
 			++client_data->bad_recv_cnt;
-			ish_hw_reset(hid_ishtp_cl->dev);
+			ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
 			break;
 		}
 
@@ -106,7 +104,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
 			++client_data->bad_recv_cnt;
 			report_bad_packet(hid_ishtp_cl, recv_msg, cur_pos,
 					  payload_len);
-			ish_hw_reset(hid_ishtp_cl->dev);
+			ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
 			break;
 		}
 
@@ -121,7 +119,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
 				report_bad_packet(hid_ishtp_cl, recv_msg,
 						  cur_pos,
 						  payload_len);
-				ish_hw_reset(hid_ishtp_cl->dev);
+				ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
 				break;
 			}
 			client_data->hid_dev_count = (unsigned int)*payload;
@@ -170,7 +168,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
 				report_bad_packet(hid_ishtp_cl, recv_msg,
 						  cur_pos,
 						  payload_len);
-				ish_hw_reset(hid_ishtp_cl->dev);
+				ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
 				break;
 			}
 			if (!client_data->hid_descr[curr_hid_dev])
@@ -195,7 +193,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
 				report_bad_packet(hid_ishtp_cl, recv_msg,
 						  cur_pos,
 						  payload_len);
-				ish_hw_reset(hid_ishtp_cl->dev);
+				ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
 				break;
 			}
 			if (!client_data->report_descr[curr_hid_dev])
@@ -313,7 +311,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
 			++client_data->bad_recv_cnt;
 			report_bad_packet(hid_ishtp_cl, recv_msg, cur_pos,
 					  payload_len);
-			ish_hw_reset(hid_ishtp_cl->dev);
+			ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
 			break;
 
 		}
@@ -493,7 +491,7 @@ int ishtp_hid_link_ready_wait(struct ishtp_cl_data *client_data)
 static int ishtp_enum_enum_devices(struct ishtp_cl *hid_ishtp_cl)
 {
 	struct hostif_msg msg;
-	struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+	struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
 	int retry_count;
 	int rv;
 
@@ -530,7 +528,7 @@ static int ishtp_enum_enum_devices(struct ishtp_cl *hid_ishtp_cl)
 	}
 
 	client_data->num_hid_devices = client_data->hid_dev_count;
-	dev_info(&hid_ishtp_cl->device->dev,
+	dev_info(ishtp_device(client_data->cl_device),
 		"[hid-ish]: enum_devices_done OK, num_hid_devices=%d\n",
 		client_data->num_hid_devices);
 
@@ -549,7 +547,7 @@ static int ishtp_enum_enum_devices(struct ishtp_cl *hid_ishtp_cl)
 static int ishtp_get_hid_descriptor(struct ishtp_cl *hid_ishtp_cl, int index)
 {
 	struct hostif_msg msg;
-	struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+	struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
 	int rv;
 
 	/* Get HID descriptor */
@@ -596,7 +594,7 @@ static int ishtp_get_report_descriptor(struct ishtp_cl *hid_ishtp_cl,
 				       int index)
 {
 	struct hostif_msg msg;
-	struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+	struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
 	int rv;
 
 	/* Get report descriptor */
@@ -644,7 +642,7 @@ static int ishtp_get_report_descriptor(struct ishtp_cl *hid_ishtp_cl,
 static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
 {
 	struct ishtp_device *dev;
-	struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+	struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
 	struct ishtp_fw_client *fw_client;
 	int i;
 	int rv;
@@ -661,11 +659,11 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
 
 	client_data->init_done = 0;
 
-	dev = hid_ishtp_cl->dev;
+	dev = ishtp_get_ishtp_device(hid_ishtp_cl);
 
 	/* Connect to FW client */
-	hid_ishtp_cl->rx_ring_size = HID_CL_RX_RING_SIZE;
-	hid_ishtp_cl->tx_ring_size = HID_CL_TX_RING_SIZE;
+	ishtp_set_tx_ring_size(hid_ishtp_cl, HID_CL_TX_RING_SIZE);
+	ishtp_set_rx_ring_size(hid_ishtp_cl, HID_CL_RX_RING_SIZE);
 
 	fw_client = ishtp_fw_cl_get_client(dev, &hid_ishtp_guid);
 	if (!fw_client) {
@@ -673,9 +671,9 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
 			"ish client uuid not found\n");
 		return -ENOENT;
 	}
-
-	hid_ishtp_cl->fw_client_id = fw_client->client_id;
-	hid_ishtp_cl->state = ISHTP_CL_CONNECTING;
+	ishtp_cl_set_fw_client_id(hid_ishtp_cl,
+				  ishtp_get_fw_client_id(fw_client));
+	ishtp_set_connection_state(hid_ishtp_cl, ISHTP_CL_CONNECTING);
 
 	rv = ishtp_cl_connect(hid_ishtp_cl);
 	if (rv) {
@@ -687,7 +685,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
 	hid_ishtp_trace(client_data,  "%s client connected\n", __func__);
 
 	/* Register read callback */
-	ishtp_register_event_cb(hid_ishtp_cl->device, ish_cl_event_cb);
+	ishtp_register_event_cb(client_data->cl_device, ish_cl_event_cb);
 
 	rv = ishtp_enum_enum_devices(hid_ishtp_cl);
 	if (rv)
@@ -725,7 +723,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
 	return 0;
 
 err_cl_disconnect:
-	hid_ishtp_cl->state = ISHTP_CL_DISCONNECTING;
+	ishtp_set_connection_state(hid_ishtp_cl, ISHTP_CL_DISCONNECTING);
 	ishtp_cl_disconnect(hid_ishtp_cl);
 err_cl_unlink:
 	ishtp_cl_unlink(hid_ishtp_cl);
@@ -762,7 +760,7 @@ static void hid_ishtp_cl_reset_handler(struct work_struct *work)
 
 	hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
 			hid_ishtp_cl);
-	dev_dbg(&cl_device->dev, "%s\n", __func__);
+	dev_dbg(ishtp_device(client_data->cl_device), "%s\n", __func__);
 
 	hid_ishtp_cl_deinit(hid_ishtp_cl);
 
@@ -771,7 +769,7 @@ static void hid_ishtp_cl_reset_handler(struct work_struct *work)
 		return;
 
 	ishtp_set_drvdata(cl_device, hid_ishtp_cl);
-	hid_ishtp_cl->client_data = client_data;
+	ishtp_set_client_data(hid_ishtp_cl, client_data);
 	client_data->hid_ishtp_cl = hid_ishtp_cl;
 
 	client_data->num_hid_devices = 0;
@@ -789,7 +787,7 @@ static void hid_ishtp_cl_reset_handler(struct work_struct *work)
 	}
 }
 
-void (*hid_print_trace)(void *dev, const char *format, ...);
+void (*hid_print_trace)(void *unused, const char *format, ...);
 
 /**
  * hid_ishtp_cl_probe() - ISHTP client driver probe
@@ -819,7 +817,7 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
 		return -ENOMEM;
 
 	ishtp_set_drvdata(cl_device, hid_ishtp_cl);
-	hid_ishtp_cl->client_data = client_data;
+	ishtp_set_client_data(hid_ishtp_cl, client_data);
 	client_data->hid_ishtp_cl = hid_ishtp_cl;
 	client_data->cl_device = cl_device;
 
@@ -851,13 +849,13 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
 static int hid_ishtp_cl_remove(struct ishtp_cl_device *cl_device)
 {
 	struct ishtp_cl *hid_ishtp_cl = ishtp_get_drvdata(cl_device);
-	struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+	struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
 
 	hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
 			hid_ishtp_cl);
 
 	dev_dbg(ishtp_device(cl_device), "%s\n", __func__);
-	hid_ishtp_cl->state = ISHTP_CL_DISCONNECTING;
+	ishtp_set_connection_state(hid_ishtp_cl, ISHTP_CL_DISCONNECTING);
 	ishtp_cl_disconnect(hid_ishtp_cl);
 	ishtp_put_device(cl_device);
 	ishtp_hid_remove(client_data);
@@ -881,7 +879,7 @@ static int hid_ishtp_cl_remove(struct ishtp_cl_device *cl_device)
 static int hid_ishtp_cl_reset(struct ishtp_cl_device *cl_device)
 {
 	struct ishtp_cl *hid_ishtp_cl = ishtp_get_drvdata(cl_device);
-	struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+	struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
 
 	hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
 			hid_ishtp_cl);
@@ -891,8 +889,6 @@ static int hid_ishtp_cl_reset(struct ishtp_cl_device *cl_device)
 	return 0;
 }
 
-#define to_ishtp_cl_device(d) container_of(d, struct ishtp_cl_device, dev)
-
 /**
  * hid_ishtp_cl_suspend() - ISHTP client driver suspend
  * @device:	device instance
@@ -903,9 +899,9 @@ static int hid_ishtp_cl_reset(struct ishtp_cl_device *cl_device)
  */
 static int hid_ishtp_cl_suspend(struct device *device)
 {
-	struct ishtp_cl_device *cl_device = to_ishtp_cl_device(device);
+	struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
 	struct ishtp_cl *hid_ishtp_cl = ishtp_get_drvdata(cl_device);
-	struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+	struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
 
 	hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
 			hid_ishtp_cl);
@@ -924,9 +920,9 @@ static int hid_ishtp_cl_suspend(struct device *device)
  */
 static int hid_ishtp_cl_resume(struct device *device)
 {
-	struct ishtp_cl_device *cl_device = to_ishtp_cl_device(device);
+	struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
 	struct ishtp_cl *hid_ishtp_cl = ishtp_get_drvdata(cl_device);
-	struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+	struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
 
 	hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
 			hid_ishtp_cl);
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid.c b/drivers/hid/intel-ish-hid/ishtp-hid.c
index 95a5b87d9105..62c03561adaa 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid.c
@@ -16,7 +16,6 @@
 #include <linux/hid.h>
 #include <linux/intel-ish-client-if.h>
 #include <uapi/linux/input.h>
-#include "ishtp/client.h"
 #include "ishtp-hid.h"
 
 /**
@@ -154,7 +153,6 @@ static void ishtp_hid_request(struct hid_device *hid, struct hid_report *rep,
 static int ishtp_wait_for_response(struct hid_device *hid)
 {
 	struct ishtp_hid_data *hid_data =  hid->driver_data;
-	struct ishtp_cl_data *client_data = hid_data->client_data;
 	int rv;
 
 	hid_ishtp_trace(client_data,  "%s hid %p\n", __func__, hid);
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid.h b/drivers/hid/intel-ish-hid/ishtp-hid.h
index 8af44e855a49..e27d3d6c1920 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid.h
+++ b/drivers/hid/intel-ish-hid/ishtp-hid.h
@@ -24,9 +24,9 @@
 #define	IS_RESPONSE	0x80
 
 /* Used to dump to Linux trace buffer, if enabled */
-#define hid_ishtp_trace(client, ...)   \
-	client->cl_device->ishtp_dev->print_log(\
-		client->cl_device->ishtp_dev, __VA_ARGS__)
+extern void (*hid_print_trace)(void *unused, const char *format, ...);
+#define hid_ishtp_trace(client, ...) \
+		(hid_print_trace)(NULL, __VA_ARGS__)
 
 /* ISH Transport protocol (ISHTP in short) GUID */
 static const guid_t hid_ishtp_guid =
-- 
2.17.2

^ permalink raw reply related

* [PATCH v2 08/10] HID: intel-ish-hid: Move functions related to bus and device
From: Srinivas Pandruvada @ 2019-03-18 19:14 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190318191428.6527-1-srinivas.pandruvada@linux.intel.com>

Move function idefinitions related to bus and device to common header file.
Also create new function to get fw client id and move ish_hw_reset() from
inline to exported function.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ishtp/bus.c       | 26 +++++++++++++++++++++
 drivers/hid/intel-ish-hid/ishtp/bus.h       | 11 ---------
 drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h |  5 ----
 include/linux/intel-ish-client-if.h         | 12 ++++++++++
 4 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 2ca65864192f..66a485a41481 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -170,6 +170,19 @@ struct ishtp_fw_client *ishtp_fw_cl_get_client(struct ishtp_device *dev,
 }
 EXPORT_SYMBOL(ishtp_fw_cl_get_client);
 
+/**
+ * ishtp_get_fw_client_id() - Get fw client id
+ *
+ * This interface is used to reset HW get FW client id.
+ *
+ * Return: firmware client id.
+ */
+int ishtp_get_fw_client_id(struct ishtp_fw_client *fw_client)
+{
+	return fw_client->client_id;
+}
+EXPORT_SYMBOL(ishtp_get_fw_client_id);
+
 /**
  * ishtp_fw_cl_by_id() - return index to fw_clients for client_id
  * @dev: the ishtp device structure
@@ -855,6 +868,19 @@ void *ishtp_trace_callback(struct ishtp_cl_device *cl_device)
 }
 EXPORT_SYMBOL(ishtp_trace_callback);
 
+/**
+ * ish_hw_reset() - Call HW reset IPC callback
+ *
+ * This interface is used to reset HW in case of error.
+ *
+ * Return: value from IPC hw_reset callback
+ */
+int ish_hw_reset(struct ishtp_device *dev)
+{
+	return dev->ops->hw_reset(dev);
+}
+EXPORT_SYMBOL(ish_hw_reset);
+
 /**
  * ishtp_bus_register() - Function to register bus
  *
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.h b/drivers/hid/intel-ish-hid/ishtp/bus.h
index 4aed195719de..93d516f5a853 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.h
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.h
@@ -80,16 +80,5 @@ void	ishtp_recv(struct ishtp_device *dev);
 void	ishtp_reset_handler(struct ishtp_device *dev);
 void	ishtp_reset_compl_handler(struct ishtp_device *dev);
 
-void	ishtp_put_device(struct ishtp_cl_device *);
-void	ishtp_get_device(struct ishtp_cl_device *);
-
-void	ishtp_set_drvdata(struct ishtp_cl_device *cl_device, void *data);
-void	*ishtp_get_drvdata(struct ishtp_cl_device *cl_device);
-
-int	ishtp_register_event_cb(struct ishtp_cl_device *device,
-				void (*read_cb)(struct ishtp_cl_device *));
 int	ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const guid_t *cuuid);
-struct	ishtp_fw_client *ishtp_fw_cl_get_client(struct ishtp_device *dev,
-						const guid_t *uuid);
-
 #endif /* _LINUX_ISHTP_CL_BUS_H */
diff --git a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
index e0a320e67a41..3cfef084b9fc 100644
--- a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
+++ b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
@@ -238,11 +238,6 @@ static inline int ish_ipc_reset(struct ishtp_device *dev)
 	return dev->ops->ipc_reset(dev);
 }
 
-static inline int ish_hw_reset(struct ishtp_device *dev)
-{
-	return dev->ops->hw_reset(dev);
-}
-
 /* Exported function */
 void	ishtp_device_init(struct ishtp_device *dev);
 int	ishtp_start(struct ishtp_device *dev);
diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
index 526e3048e09f..e98bfbb1e07e 100644
--- a/include/linux/intel-ish-client-if.h
+++ b/include/linux/intel-ish-client-if.h
@@ -9,7 +9,9 @@
 #define _INTEL_ISH_CLIENT_IF_H_
 
 struct ishtp_cl_device;
+struct ishtp_device;
 struct ishtp_cl;
+struct ishtp_fw_client;
 
 /* Client state */
 enum cl_state {
@@ -95,4 +97,14 @@ void ishtp_set_rx_ring_size(struct ishtp_cl *cl, int size);
 void ishtp_set_connection_state(struct ishtp_cl *cl, int state);
 void ishtp_cl_set_fw_client_id(struct ishtp_cl *cl, int fw_client_id);
 
+void ishtp_put_device(struct ishtp_cl_device *cl_dev);
+void ishtp_get_device(struct ishtp_cl_device *cl_dev);
+void ishtp_set_drvdata(struct ishtp_cl_device *cl_device, void *data);
+void *ishtp_get_drvdata(struct ishtp_cl_device *cl_device);
+int ishtp_register_event_cb(struct ishtp_cl_device *device,
+				void (*read_cb)(struct ishtp_cl_device *));
+struct	ishtp_fw_client *ishtp_fw_cl_get_client(struct ishtp_device *dev,
+						const guid_t *uuid);
+int ishtp_get_fw_client_id(struct ishtp_fw_client *fw_client);
+int ish_hw_reset(struct ishtp_device *dev);
 #endif /* _INTEL_ISH_CLIENT_IF_H_ */
-- 
2.17.2

^ permalink raw reply related

* [PATCH v2 07/10] HID: intel-ish-hid: Add interface functions for struct ishtp_cl
From: Srinivas Pandruvada @ 2019-03-18 19:14 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190318191428.6527-1-srinivas.pandruvada@linux.intel.com>

Instead of directly accessing members of struct ishtp_cl, create interface
functions to access them.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ishtp/client.c | 42 ++++++++++++++++++++++++
 include/linux/intel-ish-client-if.h      |  7 ++++
 2 files changed, 49 insertions(+)

diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c b/drivers/hid/intel-ish-hid/ishtp/client.c
index 657b46dcefa6..b7ac5e3b1e82 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.c
+++ b/drivers/hid/intel-ish-hid/ishtp/client.c
@@ -1063,3 +1063,45 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device *dev, void *msg,
 eoi:
 	return;
 }
+
+void *ishtp_get_client_data(struct ishtp_cl *cl)
+{
+	return cl->client_data;
+}
+EXPORT_SYMBOL(ishtp_get_client_data);
+
+void ishtp_set_client_data(struct ishtp_cl *cl, void *data)
+{
+	cl->client_data = data;
+}
+EXPORT_SYMBOL(ishtp_set_client_data);
+
+struct ishtp_device *ishtp_get_ishtp_device(struct ishtp_cl *cl)
+{
+	return cl->dev;
+}
+EXPORT_SYMBOL(ishtp_get_ishtp_device);
+
+void ishtp_set_tx_ring_size(struct ishtp_cl *cl, int size)
+{
+	cl->tx_ring_size = size;
+}
+EXPORT_SYMBOL(ishtp_set_tx_ring_size);
+
+void ishtp_set_rx_ring_size(struct ishtp_cl *cl, int size)
+{
+	cl->rx_ring_size = size;
+}
+EXPORT_SYMBOL(ishtp_set_rx_ring_size);
+
+void ishtp_set_connection_state(struct ishtp_cl *cl, int state)
+{
+	cl->state = state;
+}
+EXPORT_SYMBOL(ishtp_set_connection_state);
+
+void ishtp_cl_set_fw_client_id(struct ishtp_cl *cl, int fw_client_id)
+{
+	cl->fw_client_id = fw_client_id;
+}
+EXPORT_SYMBOL(ishtp_cl_set_fw_client_id);
diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
index 7ce172f656f8..526e3048e09f 100644
--- a/include/linux/intel-ish-client-if.h
+++ b/include/linux/intel-ish-client-if.h
@@ -87,5 +87,12 @@ int ishtp_cl_flush_queues(struct ishtp_cl *cl);
 int ishtp_cl_io_rb_recycle(struct ishtp_cl_rb *rb);
 bool ishtp_cl_tx_empty(struct ishtp_cl *cl);
 struct ishtp_cl_rb *ishtp_cl_rx_get_rb(struct ishtp_cl *cl);
+void *ishtp_get_client_data(struct ishtp_cl *cl);
+void ishtp_set_client_data(struct ishtp_cl *cl, void *data);
+struct ishtp_device *ishtp_get_ishtp_device(struct ishtp_cl *cl);
+void ishtp_set_tx_ring_size(struct ishtp_cl *cl, int size);
+void ishtp_set_rx_ring_size(struct ishtp_cl *cl, int size);
+void ishtp_set_connection_state(struct ishtp_cl *cl, int state);
+void ishtp_cl_set_fw_client_id(struct ishtp_cl *cl, int fw_client_id);
 
 #endif /* _INTEL_ISH_CLIENT_IF_H_ */
-- 
2.17.2

^ permalink raw reply related

* [PATCH v2 06/10] HID: intel-ish-hid: Move the common functions from client.h
From: Srinivas Pandruvada @ 2019-03-18 19:14 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190318191428.6527-1-srinivas.pandruvada@linux.intel.com>

Move the interface functions in client.h to common include. These are
already abstracted well to use as is. Also move any associated structures
used by these functions.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ishtp/client.h    | 24 -----------
 drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h | 26 -----------
 include/linux/intel-ish-client-if.h         | 48 +++++++++++++++++++++
 3 files changed, 48 insertions(+), 50 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ishtp/client.h b/drivers/hid/intel-ish-hid/ishtp/client.h
index afa8b1f521d0..6ed00947d6bc 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.h
+++ b/drivers/hid/intel-ish-hid/ishtp/client.h
@@ -19,15 +19,6 @@
 #include <linux/types.h>
 #include "ishtp-dev.h"
 
-/* Client state */
-enum cl_state {
-	ISHTP_CL_INITIALIZING = 0,
-	ISHTP_CL_CONNECTING,
-	ISHTP_CL_CONNECTED,
-	ISHTP_CL_DISCONNECTING,
-	ISHTP_CL_DISCONNECTED
-};
-
 /* Tx and Rx ring size */
 #define	CL_DEF_RX_RING_SIZE	2
 #define	CL_DEF_TX_RING_SIZE	2
@@ -169,19 +160,4 @@ static inline bool ishtp_cl_cmp_id(const struct ishtp_cl *cl1,
 		(cl1->fw_client_id == cl2->fw_client_id);
 }
 
-/* exported functions from ISHTP under client management scope */
-struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device);
-void ishtp_cl_free(struct ishtp_cl *cl);
-int ishtp_cl_link(struct ishtp_cl *cl);
-void ishtp_cl_unlink(struct ishtp_cl *cl);
-int ishtp_cl_disconnect(struct ishtp_cl *cl);
-int ishtp_cl_connect(struct ishtp_cl *cl);
-int ishtp_cl_send(struct ishtp_cl *cl, uint8_t *buf, size_t length);
-int ishtp_cl_flush_queues(struct ishtp_cl *cl);
-
-/* exported functions from ISHTP client buffer management scope */
-int ishtp_cl_io_rb_recycle(struct ishtp_cl_rb *rb);
-bool ishtp_cl_tx_empty(struct ishtp_cl *cl);
-struct ishtp_cl_rb *ishtp_cl_rx_get_rb(struct ishtp_cl *cl);
-
 #endif /* _ISHTP_CLIENT_H_ */
diff --git a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
index e54ce1ef27dd..e0a320e67a41 100644
--- a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
+++ b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h
@@ -79,32 +79,6 @@ struct ishtp_fw_client {
 	uint8_t client_id;
 };
 
-/**
- * struct ishtp_msg_data - ISHTP message data struct
- * @size:	Size of data in the *data
- * @data:	Pointer to data
- */
-struct ishtp_msg_data {
-	uint32_t size;
-	unsigned char *data;
-};
-
-/*
- * struct ishtp_cl_rb - request block structure
- * @list:	Link to list members
- * @cl:		ISHTP client instance
- * @buffer:	message header
- * @buf_idx:	Index into buffer
- * @read_time:	 unused at this time
- */
-struct ishtp_cl_rb {
-	struct list_head list;
-	struct ishtp_cl *cl;
-	struct ishtp_msg_data buffer;
-	unsigned long buf_idx;
-	unsigned long read_time;
-};
-
 /*
  * Control info for IPC messages ISHTP/IPC sending FIFO -
  * list with inline data buffer
diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
index abc0b8122f07..7ce172f656f8 100644
--- a/include/linux/intel-ish-client-if.h
+++ b/include/linux/intel-ish-client-if.h
@@ -9,6 +9,16 @@
 #define _INTEL_ISH_CLIENT_IF_H_
 
 struct ishtp_cl_device;
+struct ishtp_cl;
+
+/* Client state */
+enum cl_state {
+	ISHTP_CL_INITIALIZING = 0,
+	ISHTP_CL_CONNECTING,
+	ISHTP_CL_CONNECTED,
+	ISHTP_CL_DISCONNECTING,
+	ISHTP_CL_DISCONNECTED
+};
 
 /**
  * struct ishtp_cl_device - ISHTP device handle
@@ -29,6 +39,32 @@ struct ishtp_cl_driver {
 	const struct dev_pm_ops *pm;
 };
 
+/**
+ * struct ishtp_msg_data - ISHTP message data struct
+ * @size:	Size of data in the *data
+ * @data:	Pointer to data
+ */
+struct ishtp_msg_data {
+	uint32_t size;
+	unsigned char *data;
+};
+
+/*
+ * struct ishtp_cl_rb - request block structure
+ * @list:	Link to list members
+ * @cl:		ISHTP client instance
+ * @buffer:	message header
+ * @buf_idx:	Index into buffer
+ * @read_time:	 unused at this time
+ */
+struct ishtp_cl_rb {
+	struct list_head list;
+	struct ishtp_cl *cl;
+	struct ishtp_msg_data buffer;
+	unsigned long buf_idx;
+	unsigned long read_time;
+};
+
 int ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
 			     struct module *owner);
 void ishtp_cl_driver_unregister(struct ishtp_cl_driver *driver);
@@ -40,4 +76,16 @@ struct device *ishtp_device(struct ishtp_cl_device *cl_device);
 /* Trace interface for clients */
 void *ishtp_trace_callback(struct ishtp_cl_device *cl_device);
 
+struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device);
+void ishtp_cl_free(struct ishtp_cl *cl);
+int ishtp_cl_link(struct ishtp_cl *cl);
+void ishtp_cl_unlink(struct ishtp_cl *cl);
+int ishtp_cl_disconnect(struct ishtp_cl *cl);
+int ishtp_cl_connect(struct ishtp_cl *cl);
+int ishtp_cl_send(struct ishtp_cl *cl, uint8_t *buf, size_t length);
+int ishtp_cl_flush_queues(struct ishtp_cl *cl);
+int ishtp_cl_io_rb_recycle(struct ishtp_cl_rb *rb);
+bool ishtp_cl_tx_empty(struct ishtp_cl *cl);
+struct ishtp_cl_rb *ishtp_cl_rx_get_rb(struct ishtp_cl *cl);
+
 #endif /* _INTEL_ISH_CLIENT_IF_H_ */
-- 
2.17.2

^ permalink raw reply related

* [PATCH v2 05/10] HID: intel-ish-hid: Store ishtp_cl_device instance in device
From: Srinivas Pandruvada @ 2019-03-18 19:14 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190318191428.6527-1-srinivas.pandruvada@linux.intel.com>

Store ishtp_cl_device pointer in device struct private data. In this
way we can get ishtp_cl_device * from device struct pointer.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ishtp/bus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 95a97534fcda..2ca65864192f 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -466,6 +466,7 @@ static struct ishtp_cl_device *ishtp_bus_add_device(struct ishtp_device *dev,
 	}
 
 	ishtp_device_ready = true;
+	dev_set_drvdata(&device->dev, device);
 
 	return device;
 }
-- 
2.17.2

^ permalink raw reply related

* [PATCH v2 04/10] HID: intel-ish-hid: Move driver registry functions
From: Srinivas Pandruvada @ 2019-03-18 19:14 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190318191428.6527-1-srinivas.pandruvada@linux.intel.com>

Move the driver registry with the ishtp bus to the common interface
file, which clients can include.
Also rename __ishtp_cl_driver_register() to ishtp_cl_driver_register()
and removed define for ishtp_cl_driver_register.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ishtp-hid-client.c |  2 +-
 drivers/hid/intel-ish-hid/ishtp/bus.c        |  8 +++---
 drivers/hid/intel-ish-hid/ishtp/bus.h        | 27 +-------------------
 include/linux/intel-ish-client-if.h          | 25 ++++++++++++++++++
 4 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index 94a00ffeb09b..4afb39713213 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -953,7 +953,7 @@ static int __init ish_hid_init(void)
 	int	rv;
 
 	/* Register ISHTP client device driver with ISHTP Bus */
-	rv = ishtp_cl_driver_register(&hid_ishtp_cl_driver);
+	rv = ishtp_cl_driver_register(&hid_ishtp_cl_driver, THIS_MODULE);
 
 	return rv;
 
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 308853eab91c..95a97534fcda 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -485,7 +485,7 @@ static void ishtp_bus_remove_device(struct ishtp_cl_device *device)
 }
 
 /**
- * __ishtp_cl_driver_register() - Client driver register
+ * ishtp_cl_driver_register() - Client driver register
  * @driver:	the client driver instance
  * @owner:	Owner of this driver module
  *
@@ -494,8 +494,8 @@ static void ishtp_bus_remove_device(struct ishtp_cl_device *device)
  *
  * Return: Return value of driver_register or -ENODEV if not ready
  */
-int __ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
-			       struct module *owner)
+int ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
+			     struct module *owner)
 {
 	int err;
 
@@ -512,7 +512,7 @@ int __ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
 
 	return 0;
 }
-EXPORT_SYMBOL(__ishtp_cl_driver_register);
+EXPORT_SYMBOL(ishtp_cl_driver_register);
 
 /**
  * ishtp_cl_driver_unregister() - Client driver unregister
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.h b/drivers/hid/intel-ish-hid/ishtp/bus.h
index c96e7fb42f01..4aed195719de 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.h
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.h
@@ -17,6 +17,7 @@
 
 #include <linux/device.h>
 #include <linux/mod_devicetable.h>
+#include <linux/intel-ish-client-if.h>
 
 struct ishtp_cl;
 struct ishtp_cl_device;
@@ -52,26 +53,6 @@ struct ishtp_cl_device {
 	void (*event_cb)(struct ishtp_cl_device *device);
 };
 
-/**
- * struct ishtp_cl_device - ISHTP device handle
- * @driver:	driver instance on a bus
- * @name:	Name of the device for probe
- * @probe:	driver callback for device probe
- * @remove:	driver callback on device removal
- *
- * Client drivers defines to get probed/removed for ISHTP client device.
- */
-struct ishtp_cl_driver {
-	struct device_driver driver;
-	const char *name;
-	const guid_t *guid;
-	int (*probe)(struct ishtp_cl_device *dev);
-	int (*remove)(struct ishtp_cl_device *dev);
-	int (*reset)(struct ishtp_cl_device *dev);
-	const struct dev_pm_ops *pm;
-};
-
-
 int	ishtp_bus_new_client(struct ishtp_device *dev);
 void	ishtp_remove_all_clients(struct ishtp_device *dev);
 int	ishtp_cl_device_bind(struct ishtp_cl *cl);
@@ -105,12 +86,6 @@ void	ishtp_get_device(struct ishtp_cl_device *);
 void	ishtp_set_drvdata(struct ishtp_cl_device *cl_device, void *data);
 void	*ishtp_get_drvdata(struct ishtp_cl_device *cl_device);
 
-int	__ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
-				   struct module *owner);
-#define ishtp_cl_driver_register(driver)		\
-	__ishtp_cl_driver_register(driver, THIS_MODULE)
-void	ishtp_cl_driver_unregister(struct ishtp_cl_driver *driver);
-
 int	ishtp_register_event_cb(struct ishtp_cl_device *device,
 				void (*read_cb)(struct ishtp_cl_device *));
 int	ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const guid_t *cuuid);
diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
index 11e285172735..abc0b8122f07 100644
--- a/include/linux/intel-ish-client-if.h
+++ b/include/linux/intel-ish-client-if.h
@@ -10,6 +10,31 @@
 
 struct ishtp_cl_device;
 
+/**
+ * struct ishtp_cl_device - ISHTP device handle
+ * @driver:	driver instance on a bus
+ * @name:	Name of the device for probe
+ * @probe:	driver callback for device probe
+ * @remove:	driver callback on device removal
+ *
+ * Client drivers defines to get probed/removed for ISHTP client device.
+ */
+struct ishtp_cl_driver {
+	struct device_driver driver;
+	const char *name;
+	const guid_t *guid;
+	int (*probe)(struct ishtp_cl_device *dev);
+	int (*remove)(struct ishtp_cl_device *dev);
+	int (*reset)(struct ishtp_cl_device *dev);
+	const struct dev_pm_ops *pm;
+};
+
+int ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
+			     struct module *owner);
+void ishtp_cl_driver_unregister(struct ishtp_cl_driver *driver);
+int ishtp_register_event_cb(struct ishtp_cl_device *device,
+			    void (*read_cb)(struct ishtp_cl_device *));
+
 /* Get the device * from ishtp device instance */
 struct device *ishtp_device(struct ishtp_cl_device *cl_device);
 /* Trace interface for clients */
-- 
2.17.2

^ permalink raw reply related

* [PATCH v2 03/10] HID: intel-ish-hid: Simplify ishtp_cl_link()
From: Srinivas Pandruvada @ 2019-03-18 19:14 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190318191428.6527-1-srinivas.pandruvada@linux.intel.com>

All callers will only use ISHTP_HOST_CLIENT_ID_ANY, so get rid of
option to pass this additional id.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ishtp-hid-client.c |  2 +-
 drivers/hid/intel-ish-hid/ishtp/client.c     | 14 ++++----------
 drivers/hid/intel-ish-hid/ishtp/client.h     |  2 +-
 3 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index 17d5bd4f52c1..94a00ffeb09b 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -652,7 +652,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
 	dev_dbg(cl_data_to_dev(client_data), "%s\n", __func__);
 	hid_ishtp_trace(client_data,  "%s reset flag: %d\n", __func__, reset);
 
-	rv = ishtp_cl_link(hid_ishtp_cl, ISHTP_HOST_CLIENT_ID_ANY);
+	rv = ishtp_cl_link(hid_ishtp_cl);
 	if (rv) {
 		dev_err(cl_data_to_dev(client_data),
 			"ishtp_cl_link failed\n");
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c b/drivers/hid/intel-ish-hid/ishtp/client.c
index 760e48a368a8..657b46dcefa6 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.c
+++ b/drivers/hid/intel-ish-hid/ishtp/client.c
@@ -168,9 +168,6 @@ EXPORT_SYMBOL(ishtp_cl_free);
 /**
  * ishtp_cl_link() - Reserve a host id and link the client instance
  * @cl: client device instance
- * @id: host client id to use. It can be ISHTP_HOST_CLIENT_ID_ANY if any
- *	id from the available can be used
- *
  *
  * This allocates a single bit in the hostmap. This function will make sure
  * that not many client sessions are opened at the same time. Once allocated
@@ -179,11 +176,11 @@ EXPORT_SYMBOL(ishtp_cl_free);
  *
  * Return: 0 or error code on failure
  */
-int ishtp_cl_link(struct ishtp_cl *cl, int id)
+int ishtp_cl_link(struct ishtp_cl *cl)
 {
 	struct ishtp_device *dev;
-	unsigned long	flags, flags_cl;
-	int	ret = 0;
+	unsigned long flags, flags_cl;
+	int id, ret = 0;
 
 	if (WARN_ON(!cl || !cl->dev))
 		return -EINVAL;
@@ -197,10 +194,7 @@ int ishtp_cl_link(struct ishtp_cl *cl, int id)
 		goto unlock_dev;
 	}
 
-	/* If Id is not assigned get one*/
-	if (id == ISHTP_HOST_CLIENT_ID_ANY)
-		id = find_first_zero_bit(dev->host_clients_map,
-			ISHTP_CLIENTS_MAX);
+	id = find_first_zero_bit(dev->host_clients_map, ISHTP_CLIENTS_MAX);
 
 	if (id >= ISHTP_CLIENTS_MAX) {
 		spin_unlock_irqrestore(&dev->device_lock, flags);
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.h b/drivers/hid/intel-ish-hid/ishtp/client.h
index 992625891a6c..afa8b1f521d0 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.h
+++ b/drivers/hid/intel-ish-hid/ishtp/client.h
@@ -172,7 +172,7 @@ static inline bool ishtp_cl_cmp_id(const struct ishtp_cl *cl1,
 /* exported functions from ISHTP under client management scope */
 struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device);
 void ishtp_cl_free(struct ishtp_cl *cl);
-int ishtp_cl_link(struct ishtp_cl *cl, int id);
+int ishtp_cl_link(struct ishtp_cl *cl);
 void ishtp_cl_unlink(struct ishtp_cl *cl);
 int ishtp_cl_disconnect(struct ishtp_cl *cl);
 int ishtp_cl_connect(struct ishtp_cl *cl);
-- 
2.17.2

^ permalink raw reply related

* [PATCH v2 02/10] HID: intel-ish-hid: Hide members of struct ishtp_cl_device
From: Srinivas Pandruvada @ 2019-03-18 19:14 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20190318191428.6527-1-srinivas.pandruvada@linux.intel.com>

ISH clients don't need to access any field of struct ishtp_cl_device. To
avoid this create an interface functions instead where it is required.
In the case of ishtp_cl_allocate(), modify the parameters so that the
clients don't have to dereference.
Clients can also use tracing, here a new interface is added to get the
common trace function pointer, instead of direct call.
The new interface functions defined in one external header file, named
intel-ish-client-if.h. This is the only header files all ISHTP clients
must include.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ishtp-hid-client.c | 56 +++++++++++---------
 drivers/hid/intel-ish-hid/ishtp-hid.c        |  4 +-
 drivers/hid/intel-ish-hid/ishtp-hid.h        |  2 +-
 drivers/hid/intel-ish-hid/ishtp/bus.c        | 27 ++++++++++
 drivers/hid/intel-ish-hid/ishtp/client.c     |  4 +-
 drivers/hid/intel-ish-hid/ishtp/client.h     |  2 +-
 include/linux/intel-ish-client-if.h          | 18 +++++++
 7 files changed, 84 insertions(+), 29 deletions(-)
 create mode 100644 include/linux/intel-ish-client-if.h

diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index b86290611a5f..17d5bd4f52c1 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -15,6 +15,7 @@
 
 #include <linux/module.h>
 #include <linux/hid.h>
+#include <linux/intel-ish-client-if.h>
 #include <linux/sched.h>
 #include "ishtp/ishtp-dev.h"
 #include "ishtp/client.h"
@@ -24,6 +25,8 @@
 #define HID_CL_RX_RING_SIZE	32
 #define HID_CL_TX_RING_SIZE	16
 
+#define cl_data_to_dev(client_data) ishtp_device(client_data->cl_device)
+
 /**
  * report_bad_packets() - Report bad packets
  * @hid_ishtp_cl:	Client instance to get stats
@@ -39,7 +42,7 @@ static void report_bad_packet(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
 	struct hostif_msg *recv_msg = recv_buf;
 	struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
 
-	dev_err(&client_data->cl_device->dev, "[hid-ish]: BAD packet %02X\n"
+	dev_err(cl_data_to_dev(client_data), "[hid-ish]: BAD packet %02X\n"
 		"total_bad=%u cur_pos=%u\n"
 		"[%02X %02X %02X %02X]\n"
 		"payload_len=%u\n"
@@ -85,7 +88,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
 
 	do {
 		if (cur_pos + sizeof(struct hostif_msg) > total_len) {
-			dev_err(&client_data->cl_device->dev,
+			dev_err(cl_data_to_dev(client_data),
 				"[hid-ish]: error, received %u which is less than data header %u\n",
 				(unsigned int)data_len,
 				(unsigned int)sizeof(struct hostif_msg_hdr));
@@ -124,12 +127,12 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
 			client_data->hid_dev_count = (unsigned int)*payload;
 			if (!client_data->hid_devices)
 				client_data->hid_devices = devm_kcalloc(
-						&client_data->cl_device->dev,
+						cl_data_to_dev(client_data),
 						client_data->hid_dev_count,
 						sizeof(struct device_info),
 						GFP_KERNEL);
 			if (!client_data->hid_devices) {
-				dev_err(&client_data->cl_device->dev,
+				dev_err(cl_data_to_dev(client_data),
 				"Mem alloc failed for hid device info\n");
 				wake_up_interruptible(&client_data->init_wait);
 				break;
@@ -137,7 +140,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
 			for (i = 0; i < client_data->hid_dev_count; ++i) {
 				if (1 + sizeof(struct device_info) * i >=
 						payload_len) {
-					dev_err(&client_data->cl_device->dev,
+					dev_err(cl_data_to_dev(client_data),
 						"[hid-ish]: [ENUM_DEVICES]: content size %zu is bigger than payload_len %zu\n",
 						1 + sizeof(struct device_info)
 						* i, payload_len);
@@ -172,7 +175,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
 			}
 			if (!client_data->hid_descr[curr_hid_dev])
 				client_data->hid_descr[curr_hid_dev] =
-				devm_kmalloc(&client_data->cl_device->dev,
+				devm_kmalloc(cl_data_to_dev(client_data),
 					     payload_len, GFP_KERNEL);
 			if (client_data->hid_descr[curr_hid_dev]) {
 				memcpy(client_data->hid_descr[curr_hid_dev],
@@ -197,7 +200,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf,
 			}
 			if (!client_data->report_descr[curr_hid_dev])
 				client_data->report_descr[curr_hid_dev] =
-				devm_kmalloc(&client_data->cl_device->dev,
+				devm_kmalloc(cl_data_to_dev(client_data),
 					     payload_len, GFP_KERNEL);
 			if (client_data->report_descr[curr_hid_dev])  {
 				memcpy(client_data->report_descr[curr_hid_dev],
@@ -516,12 +519,12 @@ static int ishtp_enum_enum_devices(struct ishtp_cl *hid_ishtp_cl)
 					   sizeof(struct hostif_msg));
 	}
 	if (!client_data->enum_devices_done) {
-		dev_err(&client_data->cl_device->dev,
+		dev_err(cl_data_to_dev(client_data),
 			"[hid-ish]: timed out waiting for enum_devices\n");
 		return -ETIMEDOUT;
 	}
 	if (!client_data->hid_devices) {
-		dev_err(&client_data->cl_device->dev,
+		dev_err(cl_data_to_dev(client_data),
 			"[hid-ish]: failed to allocate HID dev structures\n");
 		return -ENOMEM;
 	}
@@ -564,13 +567,13 @@ static int ishtp_get_hid_descriptor(struct ishtp_cl *hid_ishtp_cl, int index)
 						 client_data->hid_descr_done,
 						 3 * HZ);
 		if (!client_data->hid_descr_done) {
-			dev_err(&client_data->cl_device->dev,
+			dev_err(cl_data_to_dev(client_data),
 				"[hid-ish]: timed out for hid_descr_done\n");
 			return -EIO;
 		}
 
 		if (!client_data->hid_descr[index]) {
-			dev_err(&client_data->cl_device->dev,
+			dev_err(cl_data_to_dev(client_data),
 				"[hid-ish]: allocation HID desc fail\n");
 			return -ENOMEM;
 		}
@@ -611,12 +614,12 @@ static int ishtp_get_report_descriptor(struct ishtp_cl *hid_ishtp_cl,
 					 client_data->report_descr_done,
 					 3 * HZ);
 	if (!client_data->report_descr_done) {
-		dev_err(&client_data->cl_device->dev,
+		dev_err(cl_data_to_dev(client_data),
 				"[hid-ish]: timed out for report descr\n");
 		return -EIO;
 	}
 	if (!client_data->report_descr[index]) {
-		dev_err(&client_data->cl_device->dev,
+		dev_err(cl_data_to_dev(client_data),
 			"[hid-ish]: failed to alloc report descr\n");
 		return -ENOMEM;
 	}
@@ -646,12 +649,12 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
 	int i;
 	int rv;
 
-	dev_dbg(&client_data->cl_device->dev, "%s\n", __func__);
+	dev_dbg(cl_data_to_dev(client_data), "%s\n", __func__);
 	hid_ishtp_trace(client_data,  "%s reset flag: %d\n", __func__, reset);
 
 	rv = ishtp_cl_link(hid_ishtp_cl, ISHTP_HOST_CLIENT_ID_ANY);
 	if (rv) {
-		dev_err(&client_data->cl_device->dev,
+		dev_err(cl_data_to_dev(client_data),
 			"ishtp_cl_link failed\n");
 		return	-ENOMEM;
 	}
@@ -666,7 +669,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
 
 	fw_client = ishtp_fw_cl_get_client(dev, &hid_ishtp_guid);
 	if (!fw_client) {
-		dev_err(&client_data->cl_device->dev,
+		dev_err(cl_data_to_dev(client_data),
 			"ish client uuid not found\n");
 		return -ENOENT;
 	}
@@ -676,7 +679,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
 
 	rv = ishtp_cl_connect(hid_ishtp_cl);
 	if (rv) {
-		dev_err(&client_data->cl_device->dev,
+		dev_err(cl_data_to_dev(client_data),
 			"client connect fail\n");
 		goto err_cl_unlink;
 	}
@@ -707,7 +710,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl, int reset)
 		if (!reset) {
 			rv = ishtp_hid_probe(i, client_data);
 			if (rv) {
-				dev_err(&client_data->cl_device->dev,
+				dev_err(cl_data_to_dev(client_data),
 				"[hid-ish]: HID probe for #%u failed: %d\n",
 				i, rv);
 				goto err_cl_disconnect;
@@ -763,7 +766,7 @@ static void hid_ishtp_cl_reset_handler(struct work_struct *work)
 
 	hid_ishtp_cl_deinit(hid_ishtp_cl);
 
-	hid_ishtp_cl = ishtp_cl_allocate(cl_device->ishtp_dev);
+	hid_ishtp_cl = ishtp_cl_allocate(cl_device);
 	if (!hid_ishtp_cl)
 		return;
 
@@ -777,15 +780,17 @@ static void hid_ishtp_cl_reset_handler(struct work_struct *work)
 		rv = hid_ishtp_cl_init(hid_ishtp_cl, 1);
 		if (!rv)
 			break;
-		dev_err(&client_data->cl_device->dev, "Retry reset init\n");
+		dev_err(cl_data_to_dev(client_data), "Retry reset init\n");
 	}
 	if (rv) {
-		dev_err(&client_data->cl_device->dev, "Reset Failed\n");
+		dev_err(cl_data_to_dev(client_data), "Reset Failed\n");
 		hid_ishtp_trace(client_data, "%s Failed hid_ishtp_cl %p\n",
 				__func__, hid_ishtp_cl);
 	}
 }
 
+void (*hid_print_trace)(void *dev, const char *format, ...);
+
 /**
  * hid_ishtp_cl_probe() - ISHTP client driver probe
  * @cl_device:		ISHTP client device instance
@@ -803,12 +808,13 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
 	if (!cl_device)
 		return	-ENODEV;
 
-	client_data = devm_kzalloc(&cl_device->dev, sizeof(*client_data),
+	client_data = devm_kzalloc(ishtp_device(cl_device),
+				   sizeof(*client_data),
 				   GFP_KERNEL);
 	if (!client_data)
 		return -ENOMEM;
 
-	hid_ishtp_cl = ishtp_cl_allocate(cl_device->ishtp_dev);
+	hid_ishtp_cl = ishtp_cl_allocate(cl_device);
 	if (!hid_ishtp_cl)
 		return -ENOMEM;
 
@@ -822,6 +828,8 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
 
 	INIT_WORK(&client_data->work, hid_ishtp_cl_reset_handler);
 
+	hid_print_trace = ishtp_trace_callback(cl_device);
+
 	rv = hid_ishtp_cl_init(hid_ishtp_cl, 0);
 	if (rv) {
 		ishtp_cl_free(hid_ishtp_cl);
@@ -848,7 +856,7 @@ static int hid_ishtp_cl_remove(struct ishtp_cl_device *cl_device)
 	hid_ishtp_trace(client_data, "%s hid_ishtp_cl %p\n", __func__,
 			hid_ishtp_cl);
 
-	dev_dbg(&cl_device->dev, "%s\n", __func__);
+	dev_dbg(ishtp_device(cl_device), "%s\n", __func__);
 	hid_ishtp_cl->state = ISHTP_CL_DISCONNECTING;
 	ishtp_cl_disconnect(hid_ishtp_cl);
 	ishtp_put_device(cl_device);
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid.c b/drivers/hid/intel-ish-hid/ishtp-hid.c
index 5c7e127b0483..95a5b87d9105 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid.c
@@ -14,6 +14,7 @@
  */
 
 #include <linux/hid.h>
+#include <linux/intel-ish-client-if.h>
 #include <uapi/linux/input.h>
 #include "ishtp/client.h"
 #include "ishtp-hid.h"
@@ -241,7 +242,8 @@ int ishtp_hid_probe(unsigned int cur_hid_dev,
 
 	hid->ll_driver = &ishtp_hid_ll_driver;
 	hid->bus = BUS_INTEL_ISHTP;
-	hid->dev.parent = &client_data->cl_device->dev;
+	hid->dev.parent = ishtp_device(client_data->cl_device);
+
 	hid->version = le16_to_cpu(ISH_HID_VERSION);
 	hid->vendor = le16_to_cpu(client_data->hid_devices[cur_hid_dev].vid);
 	hid->product = le16_to_cpu(client_data->hid_devices[cur_hid_dev].pid);
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid.h b/drivers/hid/intel-ish-hid/ishtp-hid.h
index 40663639e43b..8af44e855a49 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid.h
+++ b/drivers/hid/intel-ish-hid/ishtp-hid.h
@@ -24,7 +24,7 @@
 #define	IS_RESPONSE	0x80
 
 /* Used to dump to Linux trace buffer, if enabled */
-#define hid_ishtp_trace(client, ...)	\
+#define hid_ishtp_trace(client, ...)   \
 	client->cl_device->ishtp_dev->print_log(\
 		client->cl_device->ishtp_dev, __VA_ARGS__)
 
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 6348fee8aadc..308853eab91c 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -827,6 +827,33 @@ int ishtp_use_dma_transfer(void)
 	return ishtp_use_dma;
 }
 
+/**
+ * ishtp_device() - Return device pointer
+ *
+ * This interface is used to return device pointer from ishtp_cl_device
+ * instance.
+ *
+ * Return: device *.
+ */
+struct device *ishtp_device(struct ishtp_cl_device *device)
+{
+	return &device->dev;
+}
+EXPORT_SYMBOL(ishtp_device);
+
+/**
+ * ishtp_trace_callback() - Return trace callback
+ *
+ * This interface is used to return trace callback function pointer.
+ *
+ * Return: void *.
+ */
+void *ishtp_trace_callback(struct ishtp_cl_device *cl_device)
+{
+	return cl_device->ishtp_dev->print_log;
+}
+EXPORT_SYMBOL(ishtp_trace_callback);
+
 /**
  * ishtp_bus_register() - Function to register bus
  *
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c b/drivers/hid/intel-ish-hid/ishtp/client.c
index faeccdb1475b..760e48a368a8 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.c
+++ b/drivers/hid/intel-ish-hid/ishtp/client.c
@@ -126,7 +126,7 @@ static void ishtp_cl_init(struct ishtp_cl *cl, struct ishtp_device *dev)
  *
  * Return: The allocated client instance or NULL on failure
  */
-struct ishtp_cl *ishtp_cl_allocate(struct ishtp_device *dev)
+struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device)
 {
 	struct ishtp_cl *cl;
 
@@ -134,7 +134,7 @@ struct ishtp_cl *ishtp_cl_allocate(struct ishtp_device *dev)
 	if (!cl)
 		return NULL;
 
-	ishtp_cl_init(cl, dev);
+	ishtp_cl_init(cl, cl_device->ishtp_dev);
 	return cl;
 }
 EXPORT_SYMBOL(ishtp_cl_allocate);
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.h b/drivers/hid/intel-ish-hid/ishtp/client.h
index e0df3eb611e6..992625891a6c 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.h
+++ b/drivers/hid/intel-ish-hid/ishtp/client.h
@@ -170,7 +170,7 @@ static inline bool ishtp_cl_cmp_id(const struct ishtp_cl *cl1,
 }
 
 /* exported functions from ISHTP under client management scope */
-struct ishtp_cl	*ishtp_cl_allocate(struct ishtp_device *dev);
+struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device);
 void ishtp_cl_free(struct ishtp_cl *cl);
 int ishtp_cl_link(struct ishtp_cl *cl, int id);
 void ishtp_cl_unlink(struct ishtp_cl *cl);
diff --git a/include/linux/intel-ish-client-if.h b/include/linux/intel-ish-client-if.h
new file mode 100644
index 000000000000..11e285172735
--- /dev/null
+++ b/include/linux/intel-ish-client-if.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Intel ISH client Interface definitions
+ *
+ * Copyright (c) 2019, Intel Corporation.
+ */
+
+#ifndef _INTEL_ISH_CLIENT_IF_H_
+#define _INTEL_ISH_CLIENT_IF_H_
+
+struct ishtp_cl_device;
+
+/* Get the device * from ishtp device instance */
+struct device *ishtp_device(struct ishtp_cl_device *cl_device);
+/* Trace interface for clients */
+void *ishtp_trace_callback(struct ishtp_cl_device *cl_device);
+
+#endif /* _INTEL_ISH_CLIENT_IF_H_ */
-- 
2.17.2

^ permalink raw reply related

* [PATCH v2 01/10] HID: intel-ish-hid: Add match callback to ishtp bus type
From: Srinivas Pandruvada @ 2019-03-18 19:14 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Hong Liu
In-Reply-To: <20190318191428.6527-1-srinivas.pandruvada@linux.intel.com>

From: Hong Liu <hong.liu@intel.com>

Currently we depend on the guid check in ishtp_cl_driver.probe to match
the device and driver. However Linux device core first calls the match()
callback to decide the matching of driver and device, and then does some
preparation before calling the driver probe function. If we return error
in the driver probe, it needs to tear down all the preparation work and
retry with next driver.

Adding the match callback can avoid the unnecessary entry into unmatched
driver probe function for ishtp clients reported by FW.

Signed-off-by: Hong Liu <hong.liu@intel.com>
---
 drivers/hid/intel-ish-hid/ishtp-hid-client.c |  5 +----
 drivers/hid/intel-ish-hid/ishtp/bus.c        | 21 ++++++++++++++++++++
 drivers/hid/intel-ish-hid/ishtp/bus.h        |  1 +
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index 58773a3b5150..b86290611a5f 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -803,10 +803,6 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
 	if (!cl_device)
 		return	-ENODEV;
 
-	if (!guid_equal(&hid_ishtp_guid,
-			&cl_device->fw_client->props.protocol_name))
-		return	-ENODEV;
-
 	client_data = devm_kzalloc(&cl_device->dev, sizeof(*client_data),
 				   GFP_KERNEL);
 	if (!client_data)
@@ -937,6 +933,7 @@ static const struct dev_pm_ops hid_ishtp_pm_ops = {
 
 static struct ishtp_cl_driver	hid_ishtp_cl_driver = {
 	.name = "ish-hid",
+	.guid = &hid_ishtp_guid,
 	.probe = hid_ishtp_cl_probe,
 	.remove = hid_ishtp_cl_remove,
 	.reset = hid_ishtp_cl_reset,
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index d5f4b6438d86..6348fee8aadc 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -219,6 +219,26 @@ static int ishtp_cl_device_probe(struct device *dev)
 	return driver->probe(device);
 }
 
+/**
+ * ishtp_cl_bus_match() - Bus match() callback
+ * @dev: the device structure
+ * @drv: the driver structure
+ *
+ * This is a bus match callback, called when a new ishtp_cl_device is
+ * registered during ishtp bus client enumeration. Use the guid_t in
+ * drv and dev to decide whether they match or not.
+ *
+ * Return: 1 if dev & drv matches, 0 otherwise.
+ */
+static int ishtp_cl_bus_match(struct device *dev, struct device_driver *drv)
+{
+	struct ishtp_cl_device *device = to_ishtp_cl_device(dev);
+	struct ishtp_cl_driver *driver = to_ishtp_cl_driver(drv);
+
+	return guid_equal(driver->guid,
+			  &device->fw_client->props.protocol_name);
+}
+
 /**
  * ishtp_cl_device_remove() - Bus remove() callback
  * @dev: the device structure
@@ -372,6 +392,7 @@ static struct bus_type ishtp_cl_bus_type = {
 	.name		= "ishtp",
 	.dev_groups	= ishtp_cl_dev_groups,
 	.probe		= ishtp_cl_device_probe,
+	.match		= ishtp_cl_bus_match,
 	.remove		= ishtp_cl_device_remove,
 	.pm		= &ishtp_cl_bus_dev_pm_ops,
 	.uevent		= ishtp_cl_uevent,
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.h b/drivers/hid/intel-ish-hid/ishtp/bus.h
index 4cf7ad586c37..c96e7fb42f01 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.h
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.h
@@ -64,6 +64,7 @@ struct ishtp_cl_device {
 struct ishtp_cl_driver {
 	struct device_driver driver;
 	const char *name;
+	const guid_t *guid;
 	int (*probe)(struct ishtp_cl_device *dev);
 	int (*remove)(struct ishtp_cl_device *dev);
 	int (*reset)(struct ishtp_cl_device *dev);
-- 
2.17.2

^ permalink raw reply related

* [PATCH v2 00/10] HID: intel-ish-hid: Clean up external interfaces
From: Srinivas Pandruvada @ 2019-03-18 19:14 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Srinivas Pandruvada

NOT FOR kernel v5.1.

v2
- Rebased on the top of
"HID: intel-ish: enable raw interface to HID devices on ISH"
which Jiri already applied.
- Also exported devc pointer for DMA

No functional changes are expected with this series. I am posting this now
because of usage of ISH in ChromeOS Embedded Controller.
https://lkml.org/lkml/2019/2/24/26
I want to make sure that API is restricted before more development and posting
there.

Currently only one ISH client is using ISH transport. But it is changing now
with the development of other clients using ISH transport. Some of these
clients which are targeted for Linux based OS only laptops, are not using
HID to export sensors. As more clients are getting developed it is important
that the external interface for ISH transport only allows what clients need.
Currently the header files used by clients "client.h" and "ishtp-dev.h"
include other ISH header files. Also clients access fields from structure
which also has other fields which are only used by ISH transort.

So this series introduces one header file "linux/intel-ish-client-if.h".
This header files doesn't include any other ISH transport header files.
There are interface functions defined so that clients never have to directly
access any ISH transort structures.
Also clients don't have to match there GUID in probe. They will be only
probbed if their GUID matches, which is passed as driver registry. 


Hong Liu (1):
  HID: intel-ish-hid: Add match callback to ishtp bus type

Srinivas Pandruvada (9):
  HID: intel-ish-hid: Hide members of struct ishtp_cl_device
  HID: intel-ish-hid: Simplify ishtp_cl_link()
  HID: intel-ish-hid: Move driver registry functions
  HID: intel-ish-hid: Store ishtp_cl_device instance in device
  HID: intel-ish-hid: Move the common functions from client.h
  HID: intel-ish-hid: Add interface functions for struct ishtp_cl
  HID: intel-ish-hid: Move functions related to bus and device
  HID: intel-ish-hid: Use the new interface functions in HID ish client
  HID: intel-ish-hid: Add interface function for PCI device pointer

 drivers/hid/intel-ish-hid/ishtp-hid-client.c | 131 ++++++++++---------
 drivers/hid/intel-ish-hid/ishtp-hid.c        |   6 +-
 drivers/hid/intel-ish-hid/ishtp-hid.h        |   6 +-
 drivers/hid/intel-ish-hid/ishtp/bus.c        |  96 +++++++++++++-
 drivers/hid/intel-ish-hid/ishtp/bus.h        |  37 +-----
 drivers/hid/intel-ish-hid/ishtp/client.c     |  60 +++++++--
 drivers/hid/intel-ish-hid/ishtp/client.h     |  24 ----
 drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h  |  31 -----
 include/linux/intel-ish-client-if.h          | 112 ++++++++++++++++
 9 files changed, 325 insertions(+), 178 deletions(-)
 create mode 100644 include/linux/intel-ish-client-if.h

-- 
2.17.2

^ permalink raw reply

* Re: [RESEND PATCH v6 00/11] mfd: add support for max77650 PMIC
From: Bartosz Golaszewski @ 2019-03-18 17:44 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
	Jacek Anaszewski, Pavel Machek, Lee Jones, Sebastian Reichel,
	Liam Girdwood, Greg Kroah-Hartman
  Cc: Linux Kernel Mailing List, open list:GPIO SUBSYSTEM, devicetree,
	Linux Input, Linux LED Subsystem, Linux PM list,
	Bartosz Golaszewski
In-Reply-To: <20190318174040.17899-1-brgl@bgdev.pl>

pon., 18 mar 2019 o 18:40 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a):
>
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> This series missed the last merge window so resending rebased on top
> of v5.1-rc1 with no functional changes.
>
> ---
>
> This series adds support for max77650 ultra low-power PMIC. It provides
> the core mfd driver and a set of five sub-drivers for the regulator,
> power supply, gpio, leds and input subsystems.
>
> Patches 1-4 add the DT binding documents. Patch 5 documents mfd_add_devices().
> Patches 6-10 add all drivers. Last patch adds a MAINTAINERS entry for this
> device.
>
> The regulator part is already upstream.
>
> v1 -> v2:
> =========
>
> General:
> - use C++ style comments for the SPDX license identifier and the
>   copyright header
> - s/MODULE_LICENSE("GPL")/MODULE_LICENSE("GPL v2")/
> - lookup the virtual interrupt numbers in the MFD driver, setup
>   resources for child devices and use platform_get_irq_byname()
>   in sub-drivers
> - picked up review tags
> - use devm_request_any_context_irq() for interrupt requests
>
> LEDs:
> - changed the max77650_leds_ prefix to max77650_led_
> - drop the max77650_leds structure as the only field it held was the
>   regmap pointer, move said pointer to struct max77650_led
> - change the driver name to "max77650-led"
> - drop the last return value check and return the result of
>   regmap_write() directly
> - change the labeling scheme to one consistent with other LED drivers
>
> ONKEY:
> - drop the key reporting helper and call the input functions directly
>   from interrupt handlers
> - rename the rv local variable to error
> - drop parent device asignment
>
> Regulator:
> - drop the unnecessary init_data lookup from the driver code
> - drop unnecessary include
>
> Charger:
> - disable the charger on driver remove
> - change the power supply type to POWER_SUPPLY_TYPE_USB
>
> GPIO:
> - drop interrupt support until we have correct implementation of hierarchical
>   irqs in gpiolib
>
> v2 -> v3:
> =========
>
> General:
> - dropped regulator patches as they're already in Mark Brown's branch
>
> LED:
> - fix the compatible string in the DT binding example
> - use the max_brightness property
> - use a common prefix ("MAX77650_LED") for all defines in the driver
>
> MFD:
> - add the MODULE_DEVICE_TABLE()
> - add a sentinel to the of_device_id array
> - constify the pointers to irq names
> - use an enum instead of defines for interrupt indexes
>
> v3 -> v4:
> =========
>
> GPIO:
> - as discussed with Linus Walleij: the gpio-controller is now part of
>   the core mfd module (we don't spawn a sub-node anymore), the binding
>   document for GPIO has been dropped, the GPIO properties have been
>   defined in the binding document for the mfd core, the interrupt
>   functionality has been reintroduced with the irq directly passed from
>   the mfd part
> - due to the above changes the Reviewed-by tag from Linus was dropped
>
> v4 -> v5:
> =========
>
> General:
> - add a patch documenting mfd_add_devices()
>
> MFD:
> - pass the regmap irq_chip irq domain to mfd over mfd_add_devices so that
>   the hw interrupts from resources can be correctly mapped to virtual irqs
> - remove the enum listing cell indexes
> - extend Kconfig help
> - add a link to the programming manual
> - use REGMAP_IRQ_REG() for regmap interrupts (except for GPI which has
>   is composed of two hw interrupts for rising and falling edge)
> - add error messages in probe
> - use PLATFORM_DEVID_NONE constant in devm_mfd_add_devices()
> - set irq_base to 0 in regmap_add_irq_chip() as other users to, it's only
>   relevant if it's > 0
>
> Charger:
> - use non-maxim specific property names for minimum input voltage and current
>   limit
> - code shrink by using the enable/disable charger helpers everywhere
> - use more descriptive names for constants
>
> Onkey:
> - use EV_SW event type for slide mode
>
> LED:
> - remove stray " from Kconfig help
>
> v5 -> v6:
> =========
>
> MFD:
> - remove stray spaces in the binding document
> - rename the example dt node
> - remove unnecessary interrupt-parent property from the bindings
>
> LED:
> - add a missing dependency on LEDS_CLASS to Kconfig
>
> Onkey:
> - use boolean for the slide button property
>
> Charger:
> - fix the property names in DT example
> - make constants even more readable
>
> Bartosz Golaszewski (11):
>   dt-bindings: mfd: add DT bindings for max77650
>   dt-bindings: power: supply: add DT bindings for max77650
>   dt-bindings: leds: add DT bindings for max77650
>   dt-bindings: input: add DT bindings for max77650
>   mfd: core: document mfd_add_devices()
>   mfd: max77650: new core mfd driver
>   power: supply: max77650: add support for battery charger
>   gpio: max77650: add GPIO support
>   leds: max77650: add LEDs support
>   input: max77650: add onkey support
>   MAINTAINERS: add an entry for max77650 mfd driver
>
>  .../bindings/input/max77650-onkey.txt         |  26 ++
>  .../bindings/leds/leds-max77650.txt           |  57 +++
>  .../devicetree/bindings/mfd/max77650.txt      |  46 +++
>  .../power/supply/max77650-charger.txt         |  27 ++
>  MAINTAINERS                                   |  14 +
>  drivers/gpio/Kconfig                          |   7 +
>  drivers/gpio/Makefile                         |   1 +
>  drivers/gpio/gpio-max77650.c                  | 190 +++++++++
>  drivers/input/misc/Kconfig                    |   9 +
>  drivers/input/misc/Makefile                   |   1 +
>  drivers/input/misc/max77650-onkey.c           | 121 ++++++
>  drivers/leds/Kconfig                          |   6 +
>  drivers/leds/Makefile                         |   1 +
>  drivers/leds/leds-max77650.c                  | 147 +++++++
>  drivers/mfd/Kconfig                           |  14 +
>  drivers/mfd/Makefile                          |   1 +
>  drivers/mfd/max77650.c                        | 234 +++++++++++
>  drivers/mfd/mfd-core.c                        |  14 +
>  drivers/power/supply/Kconfig                  |   7 +
>  drivers/power/supply/Makefile                 |   1 +
>  drivers/power/supply/max77650-charger.c       | 366 ++++++++++++++++++
>  include/linux/mfd/max77650.h                  |  59 +++
>  22 files changed, 1349 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/max77650-onkey.txt
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-max77650.txt
>  create mode 100644 Documentation/devicetree/bindings/mfd/max77650.txt
>  create mode 100644 Documentation/devicetree/bindings/power/supply/max77650-charger.txt
>  create mode 100644 drivers/gpio/gpio-max77650.c
>  create mode 100644 drivers/input/misc/max77650-onkey.c
>  create mode 100644 drivers/leds/leds-max77650.c
>  create mode 100644 drivers/mfd/max77650.c
>  create mode 100644 drivers/power/supply/max77650-charger.c
>  create mode 100644 include/linux/mfd/max77650.h
>
> --
> 2.20.1
>

Sorry for spamming - I got a strange error from git send-email:

Died at /<snip!>libexec/git-core/git-send-email line 1542.

Bart

^ permalink raw reply

* [RESEND PATCH v6 11/11] MAINTAINERS: add an entry for max77650 mfd driver
From: Bartosz Golaszewski @ 2019-03-18 17:42 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
	Jacek Anaszewski, Pavel Machek, Lee Jones, Sebastian Reichel,
	Liam Girdwood, Greg Kroah-Hartman
  Cc: linux-kernel, linux-gpio, devicetree, linux-input, linux-leds,
	linux-pm, Bartosz Golaszewski
In-Reply-To: <20190318174228.18194-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

I plan on extending this set of drivers so add myself as maintainer.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 MAINTAINERS | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index e17ebf70b548..6f19949111d5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9397,6 +9397,20 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/sound/max9860.txt
 F:	sound/soc/codecs/max9860.*
 
+MAXIM MAX77650 PMIC MFD DRIVER
+M:	Bartosz Golaszewski <bgolaszewski@baylibre.com>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/*/*max77650.txt
+F:	Documentation/devicetree/bindings/*/max77650*.txt
+F:	include/linux/mfd/max77650.h
+F:	drivers/mfd/max77650.c
+F:	drivers/regulator/max77650-regulator.c
+F:	drivers/power/supply/max77650-charger.c
+F:	drivers/input/misc/max77650-onkey.c
+F:	drivers/leds/leds-max77650.c
+F:	drivers/gpio/gpio-max77650.c
+
 MAXIM MAX77802 PMIC REGULATOR DEVICE DRIVER
 M:	Javier Martinez Canillas <javier@dowhile0.org>
 L:	linux-kernel@vger.kernel.org
-- 
2.20.1

^ permalink raw reply related

* [RESEND PATCH v6 10/11] input: max77650: add onkey support
From: Bartosz Golaszewski @ 2019-03-18 17:42 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
	Jacek Anaszewski, Pavel Machek, Lee Jones, Sebastian Reichel,
	Liam Girdwood, Greg Kroah-Hartman
  Cc: linux-kernel, linux-gpio, devicetree, linux-input, linux-leds,
	linux-pm, Bartosz Golaszewski
In-Reply-To: <20190318174228.18194-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Add support for the push- and slide-button events for max77650.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/misc/Kconfig          |   9 +++
 drivers/input/misc/Makefile         |   1 +
 drivers/input/misc/max77650-onkey.c | 121 ++++++++++++++++++++++++++++
 3 files changed, 131 insertions(+)
 create mode 100644 drivers/input/misc/max77650-onkey.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index e15ed1bb8558..85bc675eecd3 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -190,6 +190,15 @@ config INPUT_M68K_BEEP
 	tristate "M68k Beeper support"
 	depends on M68K
 
+config INPUT_MAX77650_ONKEY
+	tristate "Maxim MAX77650 ONKEY support"
+	depends on MFD_MAX77650
+	help
+	  Support the ONKEY of the MAX77650 PMIC as an input device.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called max77650-onkey.
+
 config INPUT_MAX77693_HAPTIC
 	tristate "MAXIM MAX77693/MAX77843 haptic controller support"
 	depends on (MFD_MAX77693 || MFD_MAX77843) && PWM
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index b936c5b1d4ac..ffd72161c79b 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_INPUT_IXP4XX_BEEPER)	+= ixp4xx-beeper.o
 obj-$(CONFIG_INPUT_KEYSPAN_REMOTE)	+= keyspan_remote.o
 obj-$(CONFIG_INPUT_KXTJ9)		+= kxtj9.o
 obj-$(CONFIG_INPUT_M68K_BEEP)		+= m68kspkr.o
+obj-$(CONFIG_INPUT_MAX77650_ONKEY)	+= max77650-onkey.o
 obj-$(CONFIG_INPUT_MAX77693_HAPTIC)	+= max77693-haptic.o
 obj-$(CONFIG_INPUT_MAX8925_ONKEY)	+= max8925_onkey.o
 obj-$(CONFIG_INPUT_MAX8997_HAPTIC)	+= max8997_haptic.o
diff --git a/drivers/input/misc/max77650-onkey.c b/drivers/input/misc/max77650-onkey.c
new file mode 100644
index 000000000000..fbf6caab7217
--- /dev/null
+++ b/drivers/input/misc/max77650-onkey.c
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (C) 2018 BayLibre SAS
+// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
+//
+// ONKEY driver for MAXIM 77650/77651 charger/power-supply.
+
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/max77650.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define MAX77650_ONKEY_MODE_MASK	BIT(3)
+#define MAX77650_ONKEY_MODE_PUSH	0x00
+#define MAX77650_ONKEY_MODE_SLIDE	BIT(3)
+
+struct max77650_onkey {
+	struct input_dev *input;
+	unsigned int code;
+};
+
+static irqreturn_t max77650_onkey_falling(int irq, void *data)
+{
+	struct max77650_onkey *onkey = data;
+
+	input_report_key(onkey->input, onkey->code, 0);
+	input_sync(onkey->input);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t max77650_onkey_rising(int irq, void *data)
+{
+	struct max77650_onkey *onkey = data;
+
+	input_report_key(onkey->input, onkey->code, 1);
+	input_sync(onkey->input);
+
+	return IRQ_HANDLED;
+}
+
+static int max77650_onkey_probe(struct platform_device *pdev)
+{
+	int irq_r, irq_f, error, mode;
+	struct max77650_onkey *onkey;
+	struct device *dev, *parent;
+	struct regmap *map;
+	unsigned int type;
+
+	dev = &pdev->dev;
+	parent = dev->parent;
+
+	map = dev_get_regmap(parent, NULL);
+	if (!map)
+		return -ENODEV;
+
+	onkey = devm_kzalloc(dev, sizeof(*onkey), GFP_KERNEL);
+	if (!onkey)
+		return -ENOMEM;
+
+	error = device_property_read_u32(dev, "linux,code", &onkey->code);
+	if (error)
+		onkey->code = KEY_POWER;
+
+	if (device_property_read_bool(dev, "maxim,onkey-slide")) {
+		mode = MAX77650_ONKEY_MODE_SLIDE;
+		type = EV_SW;
+	} else {
+		mode = MAX77650_ONKEY_MODE_PUSH;
+		type = EV_KEY;
+	}
+
+	error = regmap_update_bits(map, MAX77650_REG_CNFG_GLBL,
+				   MAX77650_ONKEY_MODE_MASK, mode);
+	if (error)
+		return error;
+
+	irq_f = platform_get_irq_byname(pdev, "nEN_F");
+	if (irq_f < 0)
+		return irq_f;
+
+	irq_r = platform_get_irq_byname(pdev, "nEN_R");
+	if (irq_r < 0)
+		return irq_r;
+
+	onkey->input = devm_input_allocate_device(dev);
+	if (!onkey->input)
+		return -ENOMEM;
+
+	onkey->input->name = "max77650_onkey";
+	onkey->input->phys = "max77650_onkey/input0";
+	onkey->input->id.bustype = BUS_I2C;
+	input_set_capability(onkey->input, type, onkey->code);
+
+	error = devm_request_any_context_irq(dev, irq_f, max77650_onkey_falling,
+					     IRQF_ONESHOT, "onkey-down", onkey);
+	if (error < 0)
+		return error;
+
+	error = devm_request_any_context_irq(dev, irq_r, max77650_onkey_rising,
+					     IRQF_ONESHOT, "onkey-up", onkey);
+	if (error < 0)
+		return error;
+
+	return input_register_device(onkey->input);
+}
+
+static struct platform_driver max77650_onkey_driver = {
+	.driver = {
+		.name = "max77650-onkey",
+	},
+	.probe = max77650_onkey_probe,
+};
+module_platform_driver(max77650_onkey_driver);
+
+MODULE_DESCRIPTION("MAXIM 77650/77651 ONKEY driver");
+MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.20.1

^ permalink raw reply related

* [RESEND PATCH v6 09/11] leds: max77650: add LEDs support
From: Bartosz Golaszewski @ 2019-03-18 17:42 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
	Jacek Anaszewski, Pavel Machek, Lee Jones, Sebastian Reichel,
	Liam Girdwood, Greg Kroah-Hartman
  Cc: linux-kernel, linux-gpio, devicetree, linux-input, linux-leds,
	linux-pm, Bartosz Golaszewski
In-Reply-To: <20190318174228.18194-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

This adds basic support for LEDs for the max77650 PMIC. The device has
three current sinks for driving LEDs.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
---
 drivers/leds/Kconfig         |   6 ++
 drivers/leds/Makefile        |   1 +
 drivers/leds/leds-max77650.c | 147 +++++++++++++++++++++++++++++++++++
 3 files changed, 154 insertions(+)
 create mode 100644 drivers/leds/leds-max77650.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index a72f97fca57b..d8c70cc6a714 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -608,6 +608,12 @@ config LEDS_TLC591XX
 	  This option enables support for Texas Instruments TLC59108
 	  and TLC59116 LED controllers.
 
+config LEDS_MAX77650
+	tristate "LED support for Maxim MAX77650 PMIC"
+	depends on LEDS_CLASS && MFD_MAX77650
+	help
+	  LEDs driver for MAX77650 family of PMICs from Maxim Integrated.
+
 config LEDS_MAX77693
 	tristate "LED support for MAX77693 Flash"
 	depends on LEDS_CLASS_FLASH
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 4c1b0054f379..f48b2404dbb7 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_LEDS_MC13783)		+= leds-mc13783.o
 obj-$(CONFIG_LEDS_NS2)			+= leds-ns2.o
 obj-$(CONFIG_LEDS_NETXBIG)		+= leds-netxbig.o
 obj-$(CONFIG_LEDS_ASIC3)		+= leds-asic3.o
+obj-$(CONFIG_LEDS_MAX77650)		+= leds-max77650.o
 obj-$(CONFIG_LEDS_MAX77693)		+= leds-max77693.o
 obj-$(CONFIG_LEDS_MAX8997)		+= leds-max8997.o
 obj-$(CONFIG_LEDS_LM355x)		+= leds-lm355x.o
diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
new file mode 100644
index 000000000000..6b74ce9cac12
--- /dev/null
+++ b/drivers/leds/leds-max77650.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (C) 2018 BayLibre SAS
+// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
+//
+// LED driver for MAXIM 77650/77651 charger/power-supply.
+
+#include <linux/i2c.h>
+#include <linux/leds.h>
+#include <linux/mfd/max77650.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define MAX77650_LED_NUM_LEDS		3
+
+#define MAX77650_LED_A_BASE		0x40
+#define MAX77650_LED_B_BASE		0x43
+
+#define MAX77650_LED_BR_MASK		GENMASK(4, 0)
+#define MAX77650_LED_EN_MASK		GENMASK(7, 6)
+
+#define MAX77650_LED_MAX_BRIGHTNESS	MAX77650_LED_BR_MASK
+
+/* Enable EN_LED_MSTR. */
+#define MAX77650_LED_TOP_DEFAULT	BIT(0)
+
+#define MAX77650_LED_ENABLE		GENMASK(7, 6)
+#define MAX77650_LED_DISABLE		0x00
+
+#define MAX77650_LED_A_DEFAULT		MAX77650_LED_DISABLE
+/* 100% on duty */
+#define MAX77650_LED_B_DEFAULT		GENMASK(3, 0)
+
+struct max77650_led {
+	struct led_classdev cdev;
+	struct regmap *map;
+	unsigned int regA;
+	unsigned int regB;
+};
+
+static struct max77650_led *max77650_to_led(struct led_classdev *cdev)
+{
+	return container_of(cdev, struct max77650_led, cdev);
+}
+
+static int max77650_led_brightness_set(struct led_classdev *cdev,
+				       enum led_brightness brightness)
+{
+	struct max77650_led *led = max77650_to_led(cdev);
+	int val, mask;
+
+	mask = MAX77650_LED_BR_MASK | MAX77650_LED_EN_MASK;
+
+	if (brightness == LED_OFF)
+		val = MAX77650_LED_DISABLE;
+	else
+		val = MAX77650_LED_ENABLE | brightness;
+
+	return regmap_update_bits(led->map, led->regA, mask, val);
+}
+
+static int max77650_led_probe(struct platform_device *pdev)
+{
+	struct device_node *of_node, *child;
+	struct max77650_led *leds, *led;
+	struct device *parent;
+	struct device *dev;
+	struct regmap *map;
+	const char *label;
+	int rv, num_leds;
+	u32 reg;
+
+	dev = &pdev->dev;
+	parent = dev->parent;
+	of_node = dev->of_node;
+
+	if (!of_node)
+		return -ENODEV;
+
+	leds = devm_kcalloc(dev, sizeof(*leds),
+			    MAX77650_LED_NUM_LEDS, GFP_KERNEL);
+	if (!leds)
+		return -ENOMEM;
+
+	map = dev_get_regmap(dev->parent, NULL);
+	if (!map)
+		return -ENODEV;
+
+	num_leds = of_get_child_count(of_node);
+	if (!num_leds || num_leds > MAX77650_LED_NUM_LEDS)
+		return -ENODEV;
+
+	for_each_child_of_node(of_node, child) {
+		rv = of_property_read_u32(child, "reg", &reg);
+		if (rv || reg >= MAX77650_LED_NUM_LEDS)
+			return -EINVAL;
+
+		led = &leds[reg];
+		led->map = map;
+		led->regA = MAX77650_LED_A_BASE + reg;
+		led->regB = MAX77650_LED_B_BASE + reg;
+		led->cdev.brightness_set_blocking = max77650_led_brightness_set;
+		led->cdev.max_brightness = MAX77650_LED_MAX_BRIGHTNESS;
+
+		label = of_get_property(child, "label", NULL);
+		if (!label) {
+			led->cdev.name = "max77650::";
+		} else {
+			led->cdev.name = devm_kasprintf(dev, GFP_KERNEL,
+							"max77650:%s", label);
+			if (!led->cdev.name)
+				return -ENOMEM;
+		}
+
+		of_property_read_string(child, "linux,default-trigger",
+					&led->cdev.default_trigger);
+
+		rv = devm_of_led_classdev_register(dev, child, &led->cdev);
+		if (rv)
+			return rv;
+
+		rv = regmap_write(map, led->regA, MAX77650_LED_A_DEFAULT);
+		if (rv)
+			return rv;
+
+		rv = regmap_write(map, led->regB, MAX77650_LED_B_DEFAULT);
+		if (rv)
+			return rv;
+	}
+
+	return regmap_write(map,
+			    MAX77650_REG_CNFG_LED_TOP,
+			    MAX77650_LED_TOP_DEFAULT);
+}
+
+static struct platform_driver max77650_led_driver = {
+	.driver = {
+		.name = "max77650-led",
+	},
+	.probe = max77650_led_probe,
+};
+module_platform_driver(max77650_led_driver);
+
+MODULE_DESCRIPTION("MAXIM 77650/77651 LED driver");
+MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.20.1

^ permalink raw reply related

* [RESEND PATCH v6 08/11] gpio: max77650: add GPIO support
From: Bartosz Golaszewski @ 2019-03-18 17:42 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Linus Walleij, Dmitry Torokhov,
	Jacek Anaszewski, Pavel Machek, Lee Jones, Sebastian Reichel,
	Liam Girdwood, Greg Kroah-Hartman
  Cc: linux-kernel, linux-gpio, devicetree, linux-input, linux-leds,
	linux-pm, Bartosz Golaszewski
In-Reply-To: <20190318174228.18194-1-brgl@bgdev.pl>

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Add GPIO support for max77650 mfd device. This PMIC exposes a single
GPIO line.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/Kconfig         |   7 ++
 drivers/gpio/Makefile        |   1 +
 drivers/gpio/gpio-max77650.c | 190 +++++++++++++++++++++++++++++++++++
 3 files changed, 198 insertions(+)
 create mode 100644 drivers/gpio/gpio-max77650.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 3f50526a771f..c4f912104440 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1112,6 +1112,13 @@ config GPIO_MAX77620
 	  driver also provides interrupt support for each of the gpios.
 	  Say yes here to enable the max77620 to be used as gpio controller.
 
+config GPIO_MAX77650
+	tristate "Maxim MAX77650/77651 GPIO support"
+	depends on MFD_MAX77650
+	help
+	  GPIO driver for MAX77650/77651 PMIC from Maxim Semiconductor.
+	  These chips have a single pin that can be configured as GPIO.
+
 config GPIO_MSIC
 	bool "Intel MSIC mixed signal gpio support"
 	depends on (X86 || COMPILE_TEST) && MFD_INTEL_MSIC
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 54d55274b93a..075722d8317d 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_GPIO_MAX7300)	+= gpio-max7300.o
 obj-$(CONFIG_GPIO_MAX7301)	+= gpio-max7301.o
 obj-$(CONFIG_GPIO_MAX732X)	+= gpio-max732x.o
 obj-$(CONFIG_GPIO_MAX77620)	+= gpio-max77620.o
+obj-$(CONFIG_GPIO_MAX77650)	+= gpio-max77650.o
 obj-$(CONFIG_GPIO_MB86S7X)	+= gpio-mb86s7x.o
 obj-$(CONFIG_GPIO_MENZ127)	+= gpio-menz127.o
 obj-$(CONFIG_GPIO_MERRIFIELD)	+= gpio-merrifield.o
diff --git a/drivers/gpio/gpio-max77650.c b/drivers/gpio/gpio-max77650.c
new file mode 100644
index 000000000000..3f03f4e8956c
--- /dev/null
+++ b/drivers/gpio/gpio-max77650.c
@@ -0,0 +1,190 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (C) 2018 BayLibre SAS
+// Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
+//
+// GPIO driver for MAXIM 77650/77651 charger/power-supply.
+
+#include <linux/gpio/driver.h>
+#include <linux/i2c.h>
+#include <linux/mfd/max77650.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define MAX77650_GPIO_DIR_MASK		BIT(0)
+#define MAX77650_GPIO_INVAL_MASK	BIT(1)
+#define MAX77650_GPIO_DRV_MASK		BIT(2)
+#define MAX77650_GPIO_OUTVAL_MASK	BIT(3)
+#define MAX77650_GPIO_DEBOUNCE_MASK	BIT(4)
+
+#define MAX77650_GPIO_DIR_OUT		0x00
+#define MAX77650_GPIO_DIR_IN		BIT(0)
+#define MAX77650_GPIO_OUT_LOW		0x00
+#define MAX77650_GPIO_OUT_HIGH		BIT(3)
+#define MAX77650_GPIO_DRV_OPEN_DRAIN	0x00
+#define MAX77650_GPIO_DRV_PUSH_PULL	BIT(2)
+#define MAX77650_GPIO_DEBOUNCE		BIT(4)
+
+#define MAX77650_GPIO_DIR_BITS(_reg) \
+		((_reg) & MAX77650_GPIO_DIR_MASK)
+#define MAX77650_GPIO_INVAL_BITS(_reg) \
+		(((_reg) & MAX77650_GPIO_INVAL_MASK) >> 1)
+
+struct max77650_gpio_chip {
+	struct regmap *map;
+	struct gpio_chip gc;
+	int irq;
+};
+
+static int max77650_gpio_direction_input(struct gpio_chip *gc,
+					 unsigned int offset)
+{
+	struct max77650_gpio_chip *chip = gpiochip_get_data(gc);
+
+	return regmap_update_bits(chip->map,
+				  MAX77650_REG_CNFG_GPIO,
+				  MAX77650_GPIO_DIR_MASK,
+				  MAX77650_GPIO_DIR_IN);
+}
+
+static int max77650_gpio_direction_output(struct gpio_chip *gc,
+					  unsigned int offset, int value)
+{
+	struct max77650_gpio_chip *chip = gpiochip_get_data(gc);
+	int mask, regval;
+
+	mask = MAX77650_GPIO_DIR_MASK | MAX77650_GPIO_OUTVAL_MASK;
+	regval = value ? MAX77650_GPIO_OUT_HIGH : MAX77650_GPIO_OUT_LOW;
+	regval |= MAX77650_GPIO_DIR_OUT;
+
+	return regmap_update_bits(chip->map,
+				  MAX77650_REG_CNFG_GPIO, mask, regval);
+}
+
+static void max77650_gpio_set_value(struct gpio_chip *gc,
+				    unsigned int offset, int value)
+{
+	struct max77650_gpio_chip *chip = gpiochip_get_data(gc);
+	int rv, regval;
+
+	regval = value ? MAX77650_GPIO_OUT_HIGH : MAX77650_GPIO_OUT_LOW;
+
+	rv = regmap_update_bits(chip->map, MAX77650_REG_CNFG_GPIO,
+				MAX77650_GPIO_OUTVAL_MASK, regval);
+	if (rv)
+		dev_err(gc->parent, "cannot set GPIO value: %d\n", rv);
+}
+
+static int max77650_gpio_get_value(struct gpio_chip *gc,
+				   unsigned int offset)
+{
+	struct max77650_gpio_chip *chip = gpiochip_get_data(gc);
+	unsigned int val;
+	int rv;
+
+	rv = regmap_read(chip->map, MAX77650_REG_CNFG_GPIO, &val);
+	if (rv)
+		return rv;
+
+	return MAX77650_GPIO_INVAL_BITS(val);
+}
+
+static int max77650_gpio_get_direction(struct gpio_chip *gc,
+				       unsigned int offset)
+{
+	struct max77650_gpio_chip *chip = gpiochip_get_data(gc);
+	unsigned int val;
+	int rv;
+
+	rv = regmap_read(chip->map, MAX77650_REG_CNFG_GPIO, &val);
+	if (rv)
+		return rv;
+
+	return MAX77650_GPIO_DIR_BITS(val);
+}
+
+static int max77650_gpio_set_config(struct gpio_chip *gc,
+				    unsigned int offset, unsigned long cfg)
+{
+	struct max77650_gpio_chip *chip = gpiochip_get_data(gc);
+
+	switch (pinconf_to_config_param(cfg)) {
+	case PIN_CONFIG_DRIVE_OPEN_DRAIN:
+		return regmap_update_bits(chip->map,
+					  MAX77650_REG_CNFG_GPIO,
+					  MAX77650_GPIO_DRV_MASK,
+					  MAX77650_GPIO_DRV_OPEN_DRAIN);
+	case PIN_CONFIG_DRIVE_PUSH_PULL:
+		return regmap_update_bits(chip->map,
+					  MAX77650_REG_CNFG_GPIO,
+					  MAX77650_GPIO_DRV_MASK,
+					  MAX77650_GPIO_DRV_PUSH_PULL);
+	case PIN_CONFIG_INPUT_DEBOUNCE:
+		return regmap_update_bits(chip->map,
+					  MAX77650_REG_CNFG_GPIO,
+					  MAX77650_GPIO_DEBOUNCE_MASK,
+					  MAX77650_GPIO_DEBOUNCE);
+	default:
+		return -ENOTSUPP;
+	}
+}
+
+static int max77650_gpio_to_irq(struct gpio_chip *gc, unsigned int offset)
+{
+	struct max77650_gpio_chip *chip = gpiochip_get_data(gc);
+
+	return chip->irq;
+}
+
+static int max77650_gpio_probe(struct platform_device *pdev)
+{
+	struct max77650_gpio_chip *chip;
+	struct device *dev, *parent;
+	struct i2c_client *i2c;
+
+	dev = &pdev->dev;
+	parent = dev->parent;
+	i2c = to_i2c_client(parent);
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->map = dev_get_regmap(parent, NULL);
+	if (!chip->map)
+		return -ENODEV;
+
+	chip->irq = platform_get_irq_byname(pdev, "GPI");
+	if (chip->irq < 0)
+		return chip->irq;
+
+	chip->gc.base = -1;
+	chip->gc.ngpio = 1;
+	chip->gc.label = i2c->name;
+	chip->gc.parent = dev;
+	chip->gc.owner = THIS_MODULE;
+	chip->gc.can_sleep = true;
+
+	chip->gc.direction_input = max77650_gpio_direction_input;
+	chip->gc.direction_output = max77650_gpio_direction_output;
+	chip->gc.set = max77650_gpio_set_value;
+	chip->gc.get = max77650_gpio_get_value;
+	chip->gc.get_direction = max77650_gpio_get_direction;
+	chip->gc.set_config = max77650_gpio_set_config;
+	chip->gc.to_irq = max77650_gpio_to_irq;
+
+	return devm_gpiochip_add_data(dev, &chip->gc, chip);
+}
+
+static struct platform_driver max77650_gpio_driver = {
+	.driver = {
+		.name = "max77650-gpio",
+	},
+	.probe = max77650_gpio_probe,
+};
+module_platform_driver(max77650_gpio_driver);
+
+MODULE_DESCRIPTION("MAXIM 77650/77651 GPIO driver");
+MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.20.1

^ permalink raw reply related


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