media: rtl28xxu: fix regression in linux-next

A patch addressing a new USB core warning in rtl28xxu ended up causing a regression. As this was caught before the patch was committed it was superseded by a v2. Unfortunately, the first erroneous version ended up being applied to the media tree anyway.

This reverts the first version of the patch and resends the second version.

Mauro and Hans, I've reported this issue to you both repeatedly over the course of three weeks but you never replied to any of mails. I'm not sure what went wrong, but I would assume that regressions would be handled with a bit more urgency. Are you not receiving my mails or was this perhaps a case of it not being clear who of you should act on the reports?

2021-05-24: v1 posted

	https://lore.kernel.org/r/[email protected]

2021-05-31: Request not apply v1

https://lore.kernel.org/r/[email protected]

2021-05-31: v2 posted

	https://lore.kernel.org/r/[email protected]

2021-06-02: v1 applied to media tree and a request to drop it was sent
in response to the automatic notification mail

2021-06-07: Second request to drop v1 with a question of how best to
proceed to fix the regression otherwise

https://lore.kernel.org/r/[email protected]

Johan

Johan Hovold (2):
Revert "media: rtl28xxu: fix zero-length control request"
media: rtl28xxu: fix zero-length control request

 drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

2.31.1

This reverts commit 25d5ce3a606a1eb23a9265d615a92a876ff9cb5f.

The patch in question causes a regression and was superseded by a second version. Unfortunately, the first revision ended up being applied instead of the correct one.

---
 drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index 83705730e37e..0cbdb95f8d35 100644
@@ -612,9 +612,8 @@ static int rtl28xxu_read_config(struct dvb_usb_device *d)
 static int rtl28xxu_identify_state(struct dvb_usb_device *d, const char **name)
 {
 	struct rtl28xxu_dev *dev = d_to_priv(d);
-	u8 buf[1];
 	int ret;
-	struct rtl28xxu_req req_demod_i2c = {0x0020, CMD_I2C_DA_RD, 1, buf};
+	struct rtl28xxu_req req_demod_i2c = {0x0020, CMD_I2C_DA_RD, 0, NULL};
 
 	dev_dbg(&d->intf->dev, "\n");
 

2.31.1

The direction of the pipe argument must match the request-type direction bit or control requests may fail depending on the host-controller-driver implementation.

Control transfers without a data stage are treated as OUT requests by the USB stack and should be using usb_sndctrlpipe(). Failing to do so will now trigger a warning.

The driver uses a zero-length i2c-read request for type detection so update the control-request code to use usb_sndctrlpipe() in this case.

Note that actually trying to read the i2c register in question does not work as the register might not exist (e.g. depending on the demodulator) as reported by Eero Lehtinen <[email protected]>.

---
 drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

index 0cbdb95f8d35..795a012d4020 100644
@@ -37,7 +37,16 @@ static int rtl28xxu_ctrl_msg(struct dvb_usb_device *d, struct rtl28xxu_req *req)
 	} else {
 		/* read */
 		requesttype = (USB_TYPE_VENDOR | USB_DIR_IN);
-		pipe = usb_rcvctrlpipe(d->udev, 0);
+
+		/*
+		 * Zero-length transfers must use usb_sndctrlpipe() and
+		 * rtl28xxu_identify_state() uses a zero-length i2c read
+		 * command to determine the chip type.
+		 */
+		if (req->size)
+			pipe = usb_rcvctrlpipe(d->udev, 0);
+		else
+			pipe = usb_sndctrlpipe(d->udev, 0);
 	}
 
 	ret = usb_control_msg(d->udev, pipe, 0, requesttype, req->value,

2.31.1