Re: [PATCH] HID: mcp2221: Fix GPIO output handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 29 Nov 2022, Bjorn Helgaas wrote:
[+cc Takayuki, thanks for your report!]

On Wed, Nov 04, 2020 at 11:02:23PM +0100, Lars Povlsen wrote:
The mcp2221 driver GPIO output handling has has several issues.

* A wrong value is used for the GPIO direction.

* Wrong offsets are calculated for some GPIO set value/set direction
  operations, when offset is larger than 0.

This has been fixed by introducing proper manifest constants for the
direction encoding, and using 'offsetof' when calculating GPIO
register offsets.

The updated driver has been tested with the Sparx5 pcb134/pcb135
board, which has the mcp2221 device with several (output) GPIO's.

Signed-off-by: Lars Povlsen <lars.povlsen@xxxxxxxxxxxxx>
---
 drivers/hid/hid-mcp2221.c | 48 +++++++++++++++++++++++++++++++--------
 1 file changed, 39 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index 0d27ccb55dd9..4211b9839209 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c

+/* MCP GPIO get command layout */
+struct mcp_get_gpio {
+	u8 cmd;
+	u8 dummy;
+	struct {
+		u8 direction;
+		u8 value;
+	} gpio[MCP_NGPIO];
+} __packed;

This bug report: https://bugzilla.kernel.org/show_bug.cgi?id=216736
suggests that direction and value may be reversed here.

Mentioning here in case nobody actively monitors the bugzilla.

Bjorn


I can confirm that the order is reversed when compared to both the data sheet
and what I can observe in the hid reports.
The patch from the linked bugzilla bug fixes the issue for me.

--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -74,8 +74,8 @@ struct mcp_get_gpio {
 	u8 cmd;
 	u8 dummy;
 	struct {
-		u8 direction;
 		u8 value;
+		u8 direction;
 	} gpio[MCP_NGPIO];
 } __packed;

Tested-by: Sven Zühlsdorf <sven.zuehlsdorf@xxxxxxxx>

[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux