All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] pinctrl: mcp23s08: Rename and change function that wraps regmap_update_bits()
@ 2023-03-24 14:01 Uwe Kleine-König
  2023-03-24 14:01 ` [PATCH 2/2] pinctrl: mcp23s08: Implement gpio bulk functions Uwe Kleine-König
  0 siblings, 1 reply; 4+ messages in thread
From: Uwe Kleine-König @ 2023-03-24 14:01 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-gpio, kernel

The semantic of mcp_set_mask() was surprising to me when I first read
that driver. So it was unexpected that in the call

	mcp_set_mask(mcp, MCP_OLAT, mask, value);

value was a bool. Make the function a thinner wrapper around
regmap_update_bits() and rename it to also have a similar name.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 5f356edfd0fd..79a41d418482 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -144,10 +144,9 @@ static int mcp_write(struct mcp23s08 *mcp, unsigned int reg, unsigned int val)
 	return regmap_write(mcp->regmap, reg << mcp->reg_shift, val);
 }
 
-static int mcp_set_mask(struct mcp23s08 *mcp, unsigned int reg,
-		       unsigned int mask, bool enabled)
+static int mcp_update_bits(struct mcp23s08 *mcp, unsigned int reg,
+			   unsigned int mask, unsigned int val)
 {
-	u16 val  = enabled ? 0xffff : 0x0000;
 	return regmap_update_bits(mcp->regmap, reg << mcp->reg_shift,
 				  mask, val);
 }
@@ -156,7 +155,7 @@ static int mcp_set_bit(struct mcp23s08 *mcp, unsigned int reg,
 		       unsigned int pin, bool enabled)
 {
 	u16 mask = BIT(pin);
-	return mcp_set_mask(mcp, reg, mask, enabled);
+	return mcp_update_bits(mcp, reg, mask, enabled ? mask : 0);
 }
 
 static const struct pinctrl_pin_desc mcp23x08_pins[] = {
@@ -310,7 +309,7 @@ static int mcp23s08_get(struct gpio_chip *chip, unsigned offset)
 
 static int __mcp23s08_set(struct mcp23s08 *mcp, unsigned mask, bool value)
 {
-	return mcp_set_mask(mcp, MCP_OLAT, mask, value);
+	return mcp_update_bits(mcp, MCP_OLAT, mask, value ? mask : 0);
 }
 
 static void mcp23s08_set(struct gpio_chip *chip, unsigned offset, int value)
@@ -333,7 +332,7 @@ mcp23s08_direction_output(struct gpio_chip *chip, unsigned offset, int value)
 	mutex_lock(&mcp->lock);
 	status = __mcp23s08_set(mcp, mask, value);
 	if (status == 0) {
-		status = mcp_set_mask(mcp, MCP_IODIR, mask, false);
+		status = mcp_update_bits(mcp, MCP_IODIR, mask, 0);
 	}
 	mutex_unlock(&mcp->lock);
 	return status;

base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH 2/2] pinctrl: mcp23s08: Implement gpio bulk functions
  2023-03-24 14:01 [PATCH 1/2] pinctrl: mcp23s08: Rename and change function that wraps regmap_update_bits() Uwe Kleine-König
@ 2023-03-24 14:01 ` Uwe Kleine-König
  2023-03-24 17:05   ` kernel test robot
  0 siblings, 1 reply; 4+ messages in thread
From: Uwe Kleine-König @ 2023-03-24 14:01 UTC (permalink / raw)
  To: Linus Walleij; +Cc: linux-gpio, kernel

To speed up some usecases implement reading and writing several IO lines
at once.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pinctrl/pinctrl-mcp23s08.c | 35 ++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-mcp23s08.c b/drivers/pinctrl/pinctrl-mcp23s08.c
index 79a41d418482..a447c77c7ebb 100644
--- a/drivers/pinctrl/pinctrl-mcp23s08.c
+++ b/drivers/pinctrl/pinctrl-mcp23s08.c
@@ -307,6 +307,28 @@ static int mcp23s08_get(struct gpio_chip *chip, unsigned offset)
 	return status;
 }
 
+static int mcp23s08_get_multiple(struct gpio_chip *chip,
+				 unsigned long *mask, unsigned long *bits)
+{
+	struct mcp23s08 *mcp = gpiochip_get_data(chip);
+	unsigned int status;
+	int ret;
+
+	mutex_lock(&mcp->lock);
+
+	/* REVISIT reading this clears any IRQ ... */
+	ret = mcp_read(mcp, MCP_GPIO, &status);
+	if (ret < 0)
+		status = 0;
+	else {
+		mcp->cached_gpio = status;
+		*bits = status;
+	}
+
+	mutex_unlock(&mcp->lock);
+	return ret;
+}
+
 static int __mcp23s08_set(struct mcp23s08 *mcp, unsigned mask, bool value)
 {
 	return mcp_update_bits(mcp, MCP_OLAT, mask, value ? mask : 0);
@@ -322,6 +344,17 @@ static void mcp23s08_set(struct gpio_chip *chip, unsigned offset, int value)
 	mutex_unlock(&mcp->lock);
 }
 
+static void mcp23s08_set_multiple(struct gpio_chip *chip,
+				  unsigned long *mask, unsigned long *bits)
+{
+	struct mcp23s08	*mcp = gpiochip_get_data(chip);
+	int ret;
+
+	mutex_lock(&mcp->lock);
+	mcp_update_bits(mcp, MCP_OLAT, *mask, *bits);
+	mutex_unlock(&mcp->lock);
+}
+
 static int
 mcp23s08_direction_output(struct gpio_chip *chip, unsigned offset, int value)
 {
@@ -546,8 +579,10 @@ int mcp23s08_probe_one(struct mcp23s08 *mcp, struct device *dev,
 
 	mcp->chip.direction_input = mcp23s08_direction_input;
 	mcp->chip.get = mcp23s08_get;
+	mcp->chip.get_multiple = mcp23s08_get_multiple;
 	mcp->chip.direction_output = mcp23s08_direction_output;
 	mcp->chip.set = mcp23s08_set;
+	mcp->chip.set_multiple = mcp23s08_set_multiple;
 
 	mcp->chip.base = base;
 	mcp->chip.can_sleep = true;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] pinctrl: mcp23s08: Implement gpio bulk functions
  2023-03-24 14:01 ` [PATCH 2/2] pinctrl: mcp23s08: Implement gpio bulk functions Uwe Kleine-König
@ 2023-03-24 17:05   ` kernel test robot
  2023-03-24 17:48     ` Uwe Kleine-König
  0 siblings, 1 reply; 4+ messages in thread
From: kernel test robot @ 2023-03-24 17:05 UTC (permalink / raw)
  To: Uwe Kleine-König, Linus Walleij; +Cc: oe-kbuild-all, linux-gpio, kernel

Hi Uwe,

I love your patch! Perhaps something to improve:

[auto build test WARNING on fe15c26ee26efa11741a7b632e9f23b01aca4cc6]

url:    https://github.com/intel-lab-lkp/linux/commits/Uwe-Kleine-K-nig/pinctrl-mcp23s08-Implement-gpio-bulk-functions/20230324-220530
base:   fe15c26ee26efa11741a7b632e9f23b01aca4cc6
patch link:    https://lore.kernel.org/r/20230324140148.479125-2-u.kleine-koenig%40pengutronix.de
patch subject: [PATCH 2/2] pinctrl: mcp23s08: Implement gpio bulk functions
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230325/202303250001.YzK908so-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/0e7980f9275391601f384ff6fe0c4713114f0f0b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Uwe-Kleine-K-nig/pinctrl-mcp23s08-Implement-gpio-bulk-functions/20230324-220530
        git checkout 0e7980f9275391601f384ff6fe0c4713114f0f0b
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303250001.YzK908so-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/pinctrl/pinctrl-mcp23s08.c: In function 'mcp23s08_set_multiple':
>> drivers/pinctrl/pinctrl-mcp23s08.c:351:13: warning: unused variable 'ret' [-Wunused-variable]
     351 |         int ret;
         |             ^~~


vim +/ret +351 drivers/pinctrl/pinctrl-mcp23s08.c

   346	
   347	static void mcp23s08_set_multiple(struct gpio_chip *chip,
   348					  unsigned long *mask, unsigned long *bits)
   349	{
   350		struct mcp23s08	*mcp = gpiochip_get_data(chip);
 > 351		int ret;
   352	
   353		mutex_lock(&mcp->lock);
   354		mcp_update_bits(mcp, MCP_OLAT, *mask, *bits);
   355		mutex_unlock(&mcp->lock);
   356	}
   357	

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

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] pinctrl: mcp23s08: Implement gpio bulk functions
  2023-03-24 17:05   ` kernel test robot
@ 2023-03-24 17:48     ` Uwe Kleine-König
  0 siblings, 0 replies; 4+ messages in thread
From: Uwe Kleine-König @ 2023-03-24 17:48 UTC (permalink / raw)
  To: kernel test robot; +Cc: Linus Walleij, linux-gpio, kernel, oe-kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2162 bytes --]

On Sat, Mar 25, 2023 at 01:05:58AM +0800, kernel test robot wrote:
> Hi Uwe,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on fe15c26ee26efa11741a7b632e9f23b01aca4cc6]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Uwe-Kleine-K-nig/pinctrl-mcp23s08-Implement-gpio-bulk-functions/20230324-220530
> base:   fe15c26ee26efa11741a7b632e9f23b01aca4cc6
> patch link:    https://lore.kernel.org/r/20230324140148.479125-2-u.kleine-koenig%40pengutronix.de
> patch subject: [PATCH 2/2] pinctrl: mcp23s08: Implement gpio bulk functions
> config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230325/202303250001.YzK908so-lkp@intel.com/config)
> compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
> reproduce (this is a W=1 build):
>         # https://github.com/intel-lab-lkp/linux/commit/0e7980f9275391601f384ff6fe0c4713114f0f0b
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Uwe-Kleine-K-nig/pinctrl-mcp23s08-Implement-gpio-bulk-functions/20230324-220530
>         git checkout 0e7980f9275391601f384ff6fe0c4713114f0f0b
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         make W=1 O=build_dir ARCH=x86_64 olddefconfig
>         make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/
> 
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202303250001.YzK908so-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>    drivers/pinctrl/pinctrl-mcp23s08.c: In function 'mcp23s08_set_multiple':
> >> drivers/pinctrl/pinctrl-mcp23s08.c:351:13: warning: unused variable 'ret' [-Wunused-variable]
>      351 |         int ret;
>          |             ^~~

FTR: I noticed that myself and there is already a v2 on the list that is
older than this bot report.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-03-24 17:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-24 14:01 [PATCH 1/2] pinctrl: mcp23s08: Rename and change function that wraps regmap_update_bits() Uwe Kleine-König
2023-03-24 14:01 ` [PATCH 2/2] pinctrl: mcp23s08: Implement gpio bulk functions Uwe Kleine-König
2023-03-24 17:05   ` kernel test robot
2023-03-24 17:48     ` Uwe Kleine-König

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.