Skip to contentSkip to conversation

sh: maple: fix empty port handling

PATCH In review Florian Fuchs 2 participants 2 messages May 15 – Oct 6 SuperH

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 sh: maple: fix empty port handling

Fix empty Maple port handling so the bus works with fewer than four populated ports and devices can be hot-plugged at runtime.

Cast the response code to s8 so 0xff is handled as MAPLE_RESPONSE_NONE, continue scanning after empty ports, and process devinfo replies for empty unit-0 ports.

---
v2->v3: No functional change. Condense commit message and add Tested-by,
Acked-by tags.
v1->v2: Don't break hot-plugging of new devices to empty ports.
Explicitly cast the response code to s8 to use the existing logic, that relies on signed chars. In the case of 0xff (all 1s, DCs TIMEOUT response) that was interpreted as 255 unknown response code, which blocked the maple bus, as busy was never reset. Add condition to maple_response_devinfo() to process the devinfo for currently empty ports.

v2: https://lore.kernel.org/all/[email protected]/
v1: https://lore.kernel.org/all/[email protected]/

 drivers/sh/maple/maple.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)
drivers/sh/maple/maple.c +6 -8
index 5585f220e495..8e7bb42aef07 100644
@@ -445,7 +445,7 @@ static int setup_maple_commands(struct device *device, void *ignored)
 /* VBLANK bottom half - implemented via workqueue */
 static void maple_vblank_handler(struct work_struct *work)
 {
-	int x, locking;
+	int x;
 	struct maple_device *mdev;
 
 	if (!maple_dma_done())
@@ -474,13 +474,10 @@ static void maple_vblank_handler(struct work_struct *work)
 				if (!mdev)
 					break;
 				atomic_set(&mdev->busy, 1);
-				locking = maple_add_packet(mdev, 0,
+				maple_add_packet(mdev, 0,
 					MAPLE_COMMAND_DEVINFO, 0, NULL);
-				if (!locking)
-					break;
-				}
 			}
-
+		}
 		maple_pnp_time = jiffies + MAPLE_PNP_INTERVAL;
 	}
 
@@ -578,7 +575,8 @@ static void maple_response_devinfo(struct maple_device *mdev,
 				   char *recvbuf)
 {
 	char submask;
-	if (!started || (scanning == 2) || !fullscan) {
+	if (!started || (scanning == 2) || !fullscan ||
+	    ((mdev->unit == 0) && empty[mdev->port])) {
 		if ((mdev->unit == 0) && (checked[mdev->port] == false)) {
 			checked[mdev->port] = true;
 			maple_attach_driver(mdev);
@@ -644,7 +642,7 @@ static void maple_dma_handler(struct work_struct *work)
 			recvbuf = mq->recvbuf->buf;
 			__flush_invalidate_region(sh_cacheop_vaddr(recvbuf),
 					0x400);
-			code = recvbuf[0];
+			code = (s8)recvbuf[0];
 			kfree(mq->sendbuf);
 			list_del_init(&mq->list);
 			switch (code) {

base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f

Signature · 1 line
2.43.0
#1 lore raw

Hi Florian,

Quoted 2 lines · Florian Fuchs

Fix empty Maple port handling so the bus works with fewer than four populated ports and devices can be hot-plugged at runtime.

The description doesn't really add much information to what the title says. It would be more helpful if the description briefly described the actual bug and how it was fixed.

Adrian

Quoted 74 lines

Cast the response code to s8 so 0xff is handled as MAPLE_RESPONSE_NONE, continue scanning after empty ports, and process devinfo replies for empty unit-0 ports.

---
v2->v3: No functional change. Condense commit message and add Tested-by,
Acked-by tags.
v1->v2: Don't break hot-plugging of new devices to empty ports.
Explicitly cast the response code to s8 to use the existing logic, that relies on signed chars. In the case of 0xff (all 1s, DCs TIMEOUT response) that was interpreted as 255 unknown response code, which blocked the maple bus, as busy was never reset. Add condition to maple_response_devinfo() to process the devinfo for currently empty ports.

v2: https://lore.kernel.org/all/[email protected]/
v1: https://lore.kernel.org/all/[email protected]/

 drivers/sh/maple/maple.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)
drivers/sh/maple/maple.c +6 -8
index 5585f220e495..8e7bb42aef07 100644
@@ -445,7 +445,7 @@ static int setup_maple_commands(struct device *device, void *ignored)
 /* VBLANK bottom half - implemented via workqueue */
 static void maple_vblank_handler(struct work_struct *work)
 {
-	int x, locking;
+	int x;
 	struct maple_device *mdev;
 
 	if (!maple_dma_done())
@@ -474,13 +474,10 @@ static void maple_vblank_handler(struct work_struct *work)
 				if (!mdev)
 					break;
 				atomic_set(&mdev->busy, 1);
-				locking = maple_add_packet(mdev, 0,
+				maple_add_packet(mdev, 0,
 					MAPLE_COMMAND_DEVINFO, 0, NULL);
-				if (!locking)
-					break;
-				}
 			}
-
+		}
 		maple_pnp_time = jiffies + MAPLE_PNP_INTERVAL;
 	}
 
@@ -578,7 +575,8 @@ static void maple_response_devinfo(struct maple_device *mdev,
 				   char *recvbuf)
 {
 	char submask;
-	if (!started || (scanning == 2) || !fullscan) {
+	if (!started || (scanning == 2) || !fullscan ||
+	    ((mdev->unit == 0) && empty[mdev->port])) {
 		if ((mdev->unit == 0) && (checked[mdev->port] == false)) {
 			checked[mdev->port] = true;
 			maple_attach_driver(mdev);
@@ -644,7 +642,7 @@ static void maple_dma_handler(struct work_struct *work)
 			recvbuf = mq->recvbuf->buf;
 			__flush_invalidate_region(sh_cacheop_vaddr(recvbuf),
 					0x400);
-			code = recvbuf[0];
+			code = (s8)recvbuf[0];
 			kfree(mq->sendbuf);
 			list_del_init(&mq->list);
 			switch (code) {

base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f

Signature · 4 lines
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913