* Re: [PATCH] media: rtl28xxu: add type-detection instrumentation @ 2021-05-30 12:23 Eero Lehtinen 2021-05-30 13:54 ` Johan Hovold 0 siblings, 1 reply; 12+ messages in thread From: Eero Lehtinen @ 2021-05-30 12:23 UTC (permalink / raw) To: linux-usb Hi, I tested Johans latest patch and my dvb-t stick can find the channel list but not to tune to them. It uses the mxl5005s driver again with repeating mxl5005s I2C write failed messages: [ 23.276076] mxl5005s I2C reset failed [ 23.296082] mxl5005s I2C write failed [ 23.316041] mxl5005s I2C write failed [ 23.336061] mxl5005s I2C write failed [ 23.336100] usb 1-1: Frontend requested software zigzag, but didn't set the frequency step size [ 23.356096] mxl5005s I2C reset failed [ 23.376226] mxl5005s I2C write failed [ 23.396084] mxl5005s I2C write failed [ 23.416082] mxl5005s I2C write failed ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] media: rtl28xxu: add type-detection instrumentation 2021-05-30 12:23 [PATCH] media: rtl28xxu: add type-detection instrumentation Eero Lehtinen @ 2021-05-30 13:54 ` Johan Hovold 2021-05-30 15:57 ` Eero Lehtinen 0 siblings, 1 reply; 12+ messages in thread From: Johan Hovold @ 2021-05-30 13:54 UTC (permalink / raw) To: Eero Lehtinen; +Cc: linux-usb, Alan Stern On Sun, May 30, 2021 at 03:23:36PM +0300, Eero Lehtinen wrote: > Hi, > > I tested Johans latest patch and my dvb-t stick can find the channel > list but not to tune to them. It uses the mxl5005s driver again with > repeating mxl5005s I2C write failed messages: > > [ 23.276076] mxl5005s I2C reset failed > [ 23.296082] mxl5005s I2C write failed > [ 23.316041] mxl5005s I2C write failed > [ 23.336061] mxl5005s I2C write failed > [ 23.336100] usb 1-1: Frontend requested software zigzag, but didn't > set the frequency step size > [ 23.356096] mxl5005s I2C reset failed > [ 23.376226] mxl5005s I2C write failed > [ 23.396084] mxl5005s I2C write failed > [ 23.416082] mxl5005s I2C write failed It was just an instrumentation patch to gather more information. Can you post the logs from when probing/using the device with that patch in place? Specifically, look for the "rtl28xxu_identify_state" entries, but please include the full log in case there are more hints in there. Also, please keep me and Alan on CC (along with the list) so that we get your replies directly. The list can be a bit slow at forwarding at times. Johan ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] media: rtl28xxu: add type-detection instrumentation 2021-05-30 13:54 ` Johan Hovold @ 2021-05-30 15:57 ` Eero Lehtinen 2021-05-30 18:02 ` Johan Hovold 0 siblings, 1 reply; 12+ messages in thread From: Eero Lehtinen @ 2021-05-30 15:57 UTC (permalink / raw) To: Johan Hovold; +Cc: linux-usb, Alan Stern cat /sys/kernel/debug/dynamic_debug/control | grep rtl28xxu.c drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1791 [dvb_usb_rtl28xxu]rtl2832u_rc_query =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1694 [dvb_usb_rtl28xxu]rtl2831u_rc_query =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1616 [dvb_usb_rtl28xxu]rtl28xxu_frontend_ctrl =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1592 [dvb_usb_rtl28xxu]rtl28xxu_frontend_ctrl =_ "fe=%d onoff=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1570 [dvb_usb_rtl28xxu]rtl2832u_power_ctrl =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1520 [dvb_usb_rtl28xxu]rtl2832u_power_ctrl =_ "onoff=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1512 [dvb_usb_rtl28xxu]rtl2831u_power_ctrl =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1490 [dvb_usb_rtl28xxu]rtl2831u_power_ctrl =_ "WR SYS0=%02x GPIO_OUT_VAL=%02x\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1471 [dvb_usb_rtl28xxu]rtl2831u_power_ctrl =_ "RD SYS0=%02x GPIO_OUT_VAL=%02x\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1459 [dvb_usb_rtl28xxu]rtl2831u_power_ctrl =_ "onoff=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1450 [dvb_usb_rtl28xxu]rtl28xxu_init =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1425 [dvb_usb_rtl28xxu]rtl28xxu_init =_ "\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1403 [dvb_usb_rtl28xxu]rtl28xxu_tuner_detach =_ "\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1382 [dvb_usb_rtl28xxu]rtl2832u_tuner_attach =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1377 [dvb_usb_rtl28xxu]rtl2832u_tuner_attach =_ "no SDR for tuner=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1187 [dvb_usb_rtl28xxu]rtl2832u_tuner_attach =_ "\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1152 [dvb_usb_rtl28xxu]rtl2831u_tuner_attach =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1122 [dvb_usb_rtl28xxu]rtl2831u_tuner_attach =_ "\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1070 [dvb_usb_rtl28xxu]rtl28xxu_frontend_detach =_ "\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:1042 [dvb_usb_rtl28xxu]rtl2832u_frontend_attach =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:894 [dvb_usb_rtl28xxu]rtl2832u_frontend_attach =_ "\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:869 [dvb_usb_rtl28xxu]rtl2832u_frontend_callback =_ "component=%d cmd=%d arg=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:842 [dvb_usb_rtl28xxu]rtl2832u_tua9001_tuner_callback =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:809 [dvb_usb_rtl28xxu]rtl2832u_tua9001_tuner_callback =_ "cmd=%d arg=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:799 [dvb_usb_rtl28xxu]rtl2832u_fc0012_tuner_callback =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:774 [dvb_usb_rtl28xxu]rtl2832u_fc0012_tuner_callback =_ "cmd=%d arg=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:729 [dvb_usb_rtl28xxu]rtl2831u_frontend_attach =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:686 [dvb_usb_rtl28xxu]rtl2831u_frontend_attach =_ "\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:648 [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:640 [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "chip_id=%u\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:620 [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:598 [dvb_usb_rtl28xxu]rtl2832u_read_config =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:584 [dvb_usb_rtl28xxu]rtl2832u_read_config =_ "Si2168 found\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:575 [dvb_usb_rtl28xxu]rtl2832u_read_config =_ "CXD2837ER found\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:568 [dvb_usb_rtl28xxu]rtl2832u_read_config =_ "MN88473 found\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:561 [dvb_usb_rtl28xxu]rtl2832u_read_config =_ "MN88472 found\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:527 [dvb_usb_rtl28xxu]rtl2832u_read_config =_ "tuner=%s\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:378 [dvb_usb_rtl28xxu]rtl2832u_read_config =_ "\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:346 [dvb_usb_rtl28xxu]rtl2831u_read_config =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:342 [dvb_usb_rtl28xxu]rtl2831u_read_config =_ "tuner=%s\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:280 [dvb_usb_rtl28xxu]rtl2831u_read_config =_ "\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:59 [dvb_usb_rtl28xxu]rtl28xxu_ctrl_msg =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:45 [dvb_usb_rtl28xxu]rtl28xxu_ctrl_msg =_ "%s: %02x %02x %02x %02x %02x %02x %02x %02x %s %*ph\012" On Sun, May 30, 2021 at 4:54 PM Johan Hovold <johan@kernel.org> wrote: > > On Sun, May 30, 2021 at 03:23:36PM +0300, Eero Lehtinen wrote: > > Hi, > > > > I tested Johans latest patch and my dvb-t stick can find the channel > > list but not to tune to them. It uses the mxl5005s driver again with > > repeating mxl5005s I2C write failed messages: > > > > [ 23.276076] mxl5005s I2C reset failed > > [ 23.296082] mxl5005s I2C write failed > > [ 23.316041] mxl5005s I2C write failed > > [ 23.336061] mxl5005s I2C write failed > > [ 23.336100] usb 1-1: Frontend requested software zigzag, but didn't > > set the frequency step size > > [ 23.356096] mxl5005s I2C reset failed > > [ 23.376226] mxl5005s I2C write failed > > [ 23.396084] mxl5005s I2C write failed > > [ 23.416082] mxl5005s I2C write failed > > It was just an instrumentation patch to gather more information. Can you > post the logs from when probing/using the device with that patch in > place? > > Specifically, look for the "rtl28xxu_identify_state" entries, but please > include the full log in case there are more hints in there. > > Also, please keep me and Alan on CC (along with the list) so that we get > your replies directly. The list can be a bit slow at forwarding at > times. > > Johan ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] media: rtl28xxu: add type-detection instrumentation 2021-05-30 15:57 ` Eero Lehtinen @ 2021-05-30 18:02 ` Johan Hovold 2021-05-30 18:58 ` Eero Lehtinen 0 siblings, 1 reply; 12+ messages in thread From: Johan Hovold @ 2021-05-30 18:02 UTC (permalink / raw) To: Eero Lehtinen; +Cc: linux-usb, Alan Stern [ Please avoid top-posting; instead reply in-line as I do below. ] On Sun, May 30, 2021 at 06:57:12PM +0300, Eero Lehtinen wrote: > cat /sys/kernel/debug/dynamic_debug/control | grep rtl28xxu.c > drivers/media/usb/dvb-usb-v2/rtl28xxu.c:640 > [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "chip_id=%u\012" The dev_info() added by the instrumentation patch (included again below) should show up in dmesg if you applied it and rebuilt and installed the driver correctly. You can enable dynamic debugging as well to determine if the chip type is detected differently (with and without the zero-length control transfer patch), for example: echo module dvb_usb_rtl28xxu +p > /sys/kernel/debug/dynamic_debug/control but it won't log the return value from the i2c read in question. > On Sun, May 30, 2021 at 4:54 PM Johan Hovold <johan@kernel.org> wrote: > > > > On Sun, May 30, 2021 at 03:23:36PM +0300, Eero Lehtinen wrote: > > > Hi, > > > > > > I tested Johans latest patch and my dvb-t stick can find the channel > > > list but not to tune to them. It uses the mxl5005s driver again with > > > repeating mxl5005s I2C write failed messages: > > > > > > [ 23.276076] mxl5005s I2C reset failed > > It was just an instrumentation patch to gather more information. Can you > > post the logs from when probing/using the device with that patch in > > place? > > > > Specifically, look for the "rtl28xxu_identify_state" entries, but please > > include the full log in case there are more hints in there. > > > > Also, please keep me and Alan on CC (along with the list) so that we get > > your replies directly. The list can be a bit slow at forwarding at > > times. Johan From eda5deca4cbdebe21718bb13f76b8eed0673f9be Mon Sep 17 00:00:00 2001 From: Johan Hovold <johan@kernel.org> Date: Mon, 24 May 2021 10:55:19 +0200 Subject: [PATCH] media: rtl28xxu: add type-detection instrumentation Signed-off-by: Johan Hovold <johan@kernel.org> --- drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c index 97ed17a141bb..21e565603108 100644 --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c @@ -612,8 +612,10 @@ 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, 0, NULL}; + struct rtl28xxu_req req_demod_i2c2 = {0x0020, CMD_I2C_DA_RD, 1, buf}; dev_dbg(&d->intf->dev, "\n"); @@ -622,6 +624,11 @@ static int rtl28xxu_identify_state(struct dvb_usb_device *d, const char **name) * by old RTL2831U. */ ret = rtl28xxu_ctrl_msg(d, &req_demod_i2c); + dev_info(&d->intf->dev, "%s - ret1 = %d\n", __func__, ret); + + ret = rtl28xxu_ctrl_msg(d, &req_demod_i2c2); + dev_info(&d->intf->dev, "%s - ret2 = %d\n", __func__, ret); + if (ret == -EPIPE) { dev->chip_id = CHIP_ID_RTL2831U; } else if (ret == 0) { -- 2.31.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] media: rtl28xxu: add type-detection instrumentation 2021-05-30 18:02 ` Johan Hovold @ 2021-05-30 18:58 ` Eero Lehtinen 2021-05-31 6:46 ` Johan Hovold 0 siblings, 1 reply; 12+ messages in thread From: Eero Lehtinen @ 2021-05-30 18:58 UTC (permalink / raw) To: Johan Hovold; +Cc: linux-usb, Alan Stern Hi, I used dev_dbg instead of dev_info and got: drivers/media/usb/dvb-usb-v2/rtl28xxu.c:648 [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "failed=%d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:640 [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "chip_id=%u\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:630 [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "%s - ret2 = %d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:627 [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "%s - ret1 = %d\012" drivers/media/usb/dvb-usb-v2/rtl28xxu.c:620 [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "\012" dev_info does not show up with dmesg or dynamic debug. Should the kernel use the CXD2837ER driver and not the mxl5005s driver like it does with this patch. On Sun, May 30, 2021 at 9:02 PM Johan Hovold <johan@kernel.org> wrote: > > [ Please avoid top-posting; instead reply in-line as I do below. ] > > On Sun, May 30, 2021 at 06:57:12PM +0300, Eero Lehtinen wrote: > > cat /sys/kernel/debug/dynamic_debug/control | grep rtl28xxu.c > > > drivers/media/usb/dvb-usb-v2/rtl28xxu.c:640 > > [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "chip_id=%u\012" > > The dev_info() added by the instrumentation patch (included again below) > should show up in dmesg if you applied it and rebuilt and installed the > driver correctly. > > You can enable dynamic debugging as well to determine if the chip type > is detected differently (with and without the zero-length control > transfer patch), for example: > > echo module dvb_usb_rtl28xxu +p > /sys/kernel/debug/dynamic_debug/control > > but it won't log the return value from the i2c read in question. > > > On Sun, May 30, 2021 at 4:54 PM Johan Hovold <johan@kernel.org> wrote: > > > > > > On Sun, May 30, 2021 at 03:23:36PM +0300, Eero Lehtinen wrote: > > > > Hi, > > > > > > > > I tested Johans latest patch and my dvb-t stick can find the channel > > > > list but not to tune to them. It uses the mxl5005s driver again with > > > > repeating mxl5005s I2C write failed messages: > > > > > > > > [ 23.276076] mxl5005s I2C reset failed > > > > It was just an instrumentation patch to gather more information. Can you > > > post the logs from when probing/using the device with that patch in > > > place? > > > > > > Specifically, look for the "rtl28xxu_identify_state" entries, but please > > > include the full log in case there are more hints in there. > > > > > > Also, please keep me and Alan on CC (along with the list) so that we get > > > your replies directly. The list can be a bit slow at forwarding at > > > times. > > Johan > > > From eda5deca4cbdebe21718bb13f76b8eed0673f9be Mon Sep 17 00:00:00 2001 > From: Johan Hovold <johan@kernel.org> > Date: Mon, 24 May 2021 10:55:19 +0200 > Subject: [PATCH] media: rtl28xxu: add type-detection instrumentation > > Signed-off-by: Johan Hovold <johan@kernel.org> > --- > drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c > index 97ed17a141bb..21e565603108 100644 > --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c > +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c > @@ -612,8 +612,10 @@ 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, 0, NULL}; > + struct rtl28xxu_req req_demod_i2c2 = {0x0020, CMD_I2C_DA_RD, 1, buf}; > > dev_dbg(&d->intf->dev, "\n"); > > @@ -622,6 +624,11 @@ static int rtl28xxu_identify_state(struct dvb_usb_device *d, const char **name) > * by old RTL2831U. > */ > ret = rtl28xxu_ctrl_msg(d, &req_demod_i2c); > + dev_info(&d->intf->dev, "%s - ret1 = %d\n", __func__, ret); > + > + ret = rtl28xxu_ctrl_msg(d, &req_demod_i2c2); > + dev_info(&d->intf->dev, "%s - ret2 = %d\n", __func__, ret); > + > if (ret == -EPIPE) { > dev->chip_id = CHIP_ID_RTL2831U; > } else if (ret == 0) { > -- > 2.31.1 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] media: rtl28xxu: add type-detection instrumentation 2021-05-30 18:58 ` Eero Lehtinen @ 2021-05-31 6:46 ` Johan Hovold [not found] ` <CAHS3B0Ez+eKSgrCEnW2ccpBCHc_gJ_Cs3abS_DAYXRAAjNYeTA@mail.gmail.com> 0 siblings, 1 reply; 12+ messages in thread From: Johan Hovold @ 2021-05-31 6:46 UTC (permalink / raw) To: Eero Lehtinen; +Cc: linux-usb, Alan Stern [ Again, please don't top post. See http://en.wikipedia.org/wiki/Top_post ] On Sun, May 30, 2021 at 09:58:26PM +0300, Eero Lehtinen wrote: > Hi, > > I used dev_dbg instead of dev_info and got: > drivers/media/usb/dvb-usb-v2/rtl28xxu.c:648 > [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "failed=%d\012" > drivers/media/usb/dvb-usb-v2/rtl28xxu.c:640 > [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "chip_id=%u\012" > drivers/media/usb/dvb-usb-v2/rtl28xxu.c:630 > [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "%s - ret2 = %d\012" > drivers/media/usb/dvb-usb-v2/rtl28xxu.c:627 > [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "%s - ret1 = %d\012" > drivers/media/usb/dvb-usb-v2/rtl28xxu.c:620 > [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "\012" > > dev_info does not show up with dmesg or dynamic debug. Odd. Just use dev_err() since that shows up in your logs. > Should the kernel use the CXD2837ER driver and not the mxl5005s driver > like it does with this patch. Yes, something like that could be the problem here. Did it use CXD2837ER before the patch? I still need to see the return values for those transfer to determine how best to address this so could you try again with dev_err()? Johan ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <CAHS3B0Ez+eKSgrCEnW2ccpBCHc_gJ_Cs3abS_DAYXRAAjNYeTA@mail.gmail.com>]
* Re: [PATCH] media: rtl28xxu: add type-detection instrumentation [not found] ` <CAHS3B0Ez+eKSgrCEnW2ccpBCHc_gJ_Cs3abS_DAYXRAAjNYeTA@mail.gmail.com> @ 2021-05-31 7:52 ` Johan Hovold 2021-05-31 8:42 ` Johan Hovold 0 siblings, 1 reply; 12+ messages in thread From: Johan Hovold @ 2021-05-31 7:52 UTC (permalink / raw) To: Eero Lehtinen Cc: Alan Stern, Antti Palosaari, Hans Verkuil, Mauro Carvalho Chehab, linux-usb [ Resending with linux-usb on CC...] [ Adding back linux-usb and Alan on CC. Also adding Antti, Hans and Mauro. Eero, please make sure to keep everyone on CC. ] Quick summary: A recent attempt to suppress a new warning in USB core: https://lore.kernel.org/r/20210524110920.24599-4-johan@kernel.org caused the chip type detection to fail in Eero's system, and we're trying to determine how best to handle this. Hans, could you hold off applying the above until this has been resolved? On Mon, May 31, 2021 at 09:58:09AM +0300, Eero Lehtinen wrote: > Hi, > > I found dev_info messages from /var/log/messages. > > May 30 18:41:19 optipc kernel: [ 3.143433] dvb_usb_rtl28xxu > 1-1:1.0: rtl28xxu_identify_state - ret1 = 0 > May 30 18:41:19 optipc kernel: [ 3.147688] dvb_usb_rtl28xxu > 1-1:1.0: rtl28xxu_identify_state - ret2 = -32 Ok, thanks. So this explains how things go wrong. ret = rtl28xxu_ctrl_msg(d, &req_demod_i2c); if (ret == -EPIPE) { dev->chip_id = CHIP_ID_RTL2831U; } else if (ret == 0) { dev->chip_id = CHIP_ID_RTL2832U; The chip used to be identified as RTL2832U but after my change it is now detected as RTL2831U and the driver uses a separate implementation with different hardcoded defaults. Commit d0f232e823af ("[media] rtl28xxu: add heuristic to detect chip type") added this code and claimed that the i2c register in question would only be found on newer RTL2832U models. Yet, actually reading the register returns an error in your setup. So, something is fishy here. Has anyone verified that the chip-type detection works as expected for older RTL2831U? > Gmail allows only top posting. Even with the web interface you can expand the quoted text and reply inline. > Hi, > > Yes, a working kernel uses the CXD2837ER driver. It is recognized as > follows at boot. You must switch to CXD2837ER at boot time so that > applications pick it: > #!/bin/bash > if [ ! -e /dev/dvb/adapter0/frontend99 ] then > mv /dev/dvb/adapter0/frontend0 /dev/dvb/adapter0/frontend99 > mv /dev/dvb/adapter0/frontend1 /dev/dvb/adapter0/frontend0 > fi > > [ 3.426235] dvbdev: DVB: registering new adapter (Astrometa DVB-T2) > [ 3.431121] i2c i2c-10: Added multiplexed i2c bus 11 > [ 3.431125] rtl2832 10-0010: Realtek RTL2832 successfully attached > [ 3.432648] i2c i2c-10: cxd2841er_attach(): I2C adapter > 000000000397a340 SLVX addr 6e SLVT addr 6c > [ 3.441178] i2c i2c-10: cxd2841er_attach(): attaching CXD2837ER > DVB-C/T/T2 frontend > [ 3.441180] i2c i2c-10: cxd2841er_attach(): chip ID 0xb1 OK. > [ 3.441182] usb 1-1: DVB: registering adapter 0 frontend 0 (Realtek > RTL2832 (DVB-T))... > [ 3.441218] usb 1-1: DVB: registering adapter 0 frontend 1 (Sony > CXD2837ER DVB-T/T2/C demodulator)... > [ 3.443994] r820t 11-003a: creating new instance > [ 3.457197] r820t 11-003a: Rafael Micro r820t successfully identified > [ 3.457210] r820t 11-003a: attaching existing instance > [ 3.512185] r820t 11-003a: Rafael Micro r820t successfully identified > [ 3.526915] rtl2832_sdr rtl2832_sdr.1.auto: Registered as swradio0 > [ 3.526917] rtl2832_sdr rtl2832_sdr.1.auto: Realtek RTL2832 SDR attached > [ 3.526918] rtl2832_sdr rtl2832_sdr.1.auto: SDR API is still > slightly experimental and functionality changes may follow > [ 3.542192] Registered IR keymap rc-astrometa-t2hybrid > [ 3.542205] rc rc0: Astrometa DVB-T2 as > /devices/pci0000:00/0000:00:01.3/0000:02:00.0/usb1/1-1/rc/rc0 > [ 3.542226] input: Astrometa DVB-T2 as > /devices/pci0000:00/0000:00:01.3/0000:02:00.0/usb1/1-1/rc/rc0/input21 > [ 3.542271] usb 1-1: dvb_usb_v2: schedule remote query interval to 200 msecs > [ 3.562187] usb 1-1: dvb_usb_v2: 'Astrometa DVB-T2' successfully > initialized and connected > [ 3.562230] usbcore: registered new interface driver dvb_usb_rtl28xxu > On Mon, May 31, 2021 at 9:46 AM Johan Hovold <johan@kernel.org> wrote: > > > > [ Again, please don't top post. See > > http://en.wikipedia.org/wiki/Top_post ] > > > > On Sun, May 30, 2021 at 09:58:26PM +0300, Eero Lehtinen wrote: > > > Hi, > > > > > > I used dev_dbg instead of dev_info and got: > > > drivers/media/usb/dvb-usb-v2/rtl28xxu.c:648 > > > [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "failed=%d\012" > > > drivers/media/usb/dvb-usb-v2/rtl28xxu.c:640 > > > [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "chip_id=%u\012" > > > drivers/media/usb/dvb-usb-v2/rtl28xxu.c:630 > > > [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "%s - ret2 = %d\012" > > > drivers/media/usb/dvb-usb-v2/rtl28xxu.c:627 > > > [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "%s - ret1 = %d\012" > > > drivers/media/usb/dvb-usb-v2/rtl28xxu.c:620 > > > [dvb_usb_rtl28xxu]rtl28xxu_identify_state =_ "\012" > > > > > > dev_info does not show up with dmesg or dynamic debug. > > > > Odd. Just use dev_err() since that shows up in your logs. > > > > > Should the kernel use the CXD2837ER driver and not the mxl5005s driver > > > like it does with this patch. > > > > Yes, something like that could be the problem here. Did it use CXD2837ER > > before the patch? > > > > I still need to see the return values for those transfer to determine > > how best to address this so could you try again with dev_err()? Johan ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] media: rtl28xxu: add type-detection instrumentation 2021-05-31 7:52 ` Johan Hovold @ 2021-05-31 8:42 ` Johan Hovold 2021-05-31 9:08 ` Eero Lehtinen 0 siblings, 1 reply; 12+ messages in thread From: Johan Hovold @ 2021-05-31 8:42 UTC (permalink / raw) To: Eero Lehtinen Cc: Alan Stern, Antti Palosaari, Hans Verkuil, Mauro Carvalho Chehab, linux-usb On Mon, May 31, 2021 at 09:52:18AM +0200, Johan Hovold wrote: > On Mon, May 31, 2021 at 09:58:09AM +0300, Eero Lehtinen wrote: > > Hi, > > > > I found dev_info messages from /var/log/messages. > > > > May 30 18:41:19 optipc kernel: [ 3.143433] dvb_usb_rtl28xxu > > 1-1:1.0: rtl28xxu_identify_state - ret1 = 0 > > May 30 18:41:19 optipc kernel: [ 3.147688] dvb_usb_rtl28xxu > > 1-1:1.0: rtl28xxu_identify_state - ret2 = -32 > > Ok, thanks. So this explains how things go wrong. > > ret = rtl28xxu_ctrl_msg(d, &req_demod_i2c); > if (ret == -EPIPE) { > dev->chip_id = CHIP_ID_RTL2831U; > } else if (ret == 0) { > dev->chip_id = CHIP_ID_RTL2832U; > > The chip used to be identified as RTL2832U but after my change it is > now detected as RTL2831U and the driver uses a separate implementation > with different hardcoded defaults. > > Commit d0f232e823af ("[media] rtl28xxu: add heuristic to detect chip > type") added this code and claimed that the i2c register in question > would only be found on newer RTL2832U models. Yet, actually reading the > register returns an error in your setup. > > So, something is fishy here. Has anyone verified that the chip-type > detection works as expected for older RTL2831U? Ok, the driver just wants to know if the i2c-read vendor request exists, and actually reading the register will not work since the register may not even exist (e.g. depending on the demodulator). So it seems we need to keep this zero-length read request and only update the pipe argument to suppress the new WARN() in usb_submit_urb(). Eero, could you try applying the below on top of -next and confirm that it suppresses the warning without messing up the type detection? Johan From 2cec8fa000152bcb997dd7aeeb0917ebf744a7bd Mon Sep 17 00:00:00 2001 From: Johan Hovold <johan@kernel.org> Date: Mon, 24 May 2021 10:55:19 +0200 Subject: [PATCH v2] media: rtl28xxu: fix zero-length control request 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 <debiangamer2@gmail.com>. Reported-by: syzbot+faf11bbadc5a372564da@syzkaller.appspotmail.com Reported-by: Eero Lehtinen <debiangamer2@gmail.com> Fixes: d0f232e823af ("[media] rtl28xxu: add heuristic to detect chip type") Cc: stable@vger.kernel.org # 4.0 Cc: Antti Palosaari <crope@iki.fi> Signed-off-by: Johan Hovold <johan@kernel.org> --- drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c index 97ed17a141bb..a6124472cb06 100644 --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c @@ -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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] media: rtl28xxu: add type-detection instrumentation 2021-05-31 8:42 ` Johan Hovold @ 2021-05-31 9:08 ` Eero Lehtinen 2021-05-31 9:13 ` Johan Hovold 0 siblings, 1 reply; 12+ messages in thread From: Eero Lehtinen @ 2021-05-31 9:08 UTC (permalink / raw) To: Johan Hovold Cc: Alan Stern, Antti Palosaari, Hans Verkuil, Mauro Carvalho Chehab, linux-usb On Mon, May 31, 2021 at 11:42 AM Johan Hovold <johan@kernel.org> wrote: > > On Mon, May 31, 2021 at 09:52:18AM +0200, Johan Hovold wrote: > > > On Mon, May 31, 2021 at 09:58:09AM +0300, Eero Lehtinen wrote: > > > Hi, > > > > > > I found dev_info messages from /var/log/messages. > > > > > > May 30 18:41:19 optipc kernel: [ 3.143433] dvb_usb_rtl28xxu > > > 1-1:1.0: rtl28xxu_identify_state - ret1 = 0 > > > May 30 18:41:19 optipc kernel: [ 3.147688] dvb_usb_rtl28xxu > > > 1-1:1.0: rtl28xxu_identify_state - ret2 = -32 > > > > Ok, thanks. So this explains how things go wrong. > > > > ret = rtl28xxu_ctrl_msg(d, &req_demod_i2c); > > if (ret == -EPIPE) { > > dev->chip_id = CHIP_ID_RTL2831U; > > } else if (ret == 0) { > > dev->chip_id = CHIP_ID_RTL2832U; > > > > The chip used to be identified as RTL2832U but after my change it is > > now detected as RTL2831U and the driver uses a separate implementation > > with different hardcoded defaults. > > > > Commit d0f232e823af ("[media] rtl28xxu: add heuristic to detect chip > > type") added this code and claimed that the i2c register in question > > would only be found on newer RTL2832U models. Yet, actually reading the > > register returns an error in your setup. > > > > So, something is fishy here. Has anyone verified that the chip-type > > detection works as expected for older RTL2831U? > > Ok, the driver just wants to know if the i2c-read vendor request exists, > and actually reading the register will not work since the register may > not even exist (e.g. depending on the demodulator). > > So it seems we need to keep this zero-length read request and only > update the pipe argument to suppress the new WARN() in > usb_submit_urb(). > > Eero, could you try applying the below on top of -next and confirm that > it suppresses the warning without messing up the type detection? > > Johan > > > From 2cec8fa000152bcb997dd7aeeb0917ebf744a7bd Mon Sep 17 00:00:00 2001 > From: Johan Hovold <johan@kernel.org> > Date: Mon, 24 May 2021 10:55:19 +0200 > Subject: [PATCH v2] media: rtl28xxu: fix zero-length control request > > 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 <debiangamer2@gmail.com>. > > Reported-by: syzbot+faf11bbadc5a372564da@syzkaller.appspotmail.com > Reported-by: Eero Lehtinen <debiangamer2@gmail.com> > Fixes: d0f232e823af ("[media] rtl28xxu: add heuristic to detect chip type") > Cc: stable@vger.kernel.org # 4.0 > Cc: Antti Palosaari <crope@iki.fi> > Signed-off-by: Johan Hovold <johan@kernel.org> > --- > drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c > index 97ed17a141bb..a6124472cb06 100644 > --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c > +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c > @@ -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 > Hi, I confirm that it suppresses the warning without messing up the type detection. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] media: rtl28xxu: add type-detection instrumentation 2021-05-31 9:08 ` Eero Lehtinen @ 2021-05-31 9:13 ` Johan Hovold 2021-06-02 11:01 ` Antti Palosaari 0 siblings, 1 reply; 12+ messages in thread From: Johan Hovold @ 2021-05-31 9:13 UTC (permalink / raw) To: Eero Lehtinen Cc: Alan Stern, Antti Palosaari, Hans Verkuil, Mauro Carvalho Chehab, linux-usb On Mon, May 31, 2021 at 12:08:20PM +0300, Eero Lehtinen wrote: > On Mon, May 31, 2021 at 11:42 AM Johan Hovold <johan@kernel.org> wrote: > > Ok, the driver just wants to know if the i2c-read vendor request exists, > > and actually reading the register will not work since the register may > > not even exist (e.g. depending on the demodulator). > > > > So it seems we need to keep this zero-length read request and only > > update the pipe argument to suppress the new WARN() in > > usb_submit_urb(). > > > > Eero, could you try applying the below on top of -next and confirm that > > it suppresses the warning without messing up the type detection? > > From 2cec8fa000152bcb997dd7aeeb0917ebf744a7bd Mon Sep 17 00:00:00 2001 > > From: Johan Hovold <johan@kernel.org> > > Date: Mon, 24 May 2021 10:55:19 +0200 > > Subject: [PATCH v2] media: rtl28xxu: fix zero-length control request > > > > 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 <debiangamer2@gmail.com>. > > > > Reported-by: syzbot+faf11bbadc5a372564da@syzkaller.appspotmail.com > > Reported-by: Eero Lehtinen <debiangamer2@gmail.com> > > Fixes: d0f232e823af ("[media] rtl28xxu: add heuristic to detect chip type") > > Cc: stable@vger.kernel.org # 4.0 > > Cc: Antti Palosaari <crope@iki.fi> > > Signed-off-by: Johan Hovold <johan@kernel.org> > > --- > > drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c > > index 97ed17a141bb..a6124472cb06 100644 > > --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c > > +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c > > @@ -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 > I confirm that it suppresses the warning without messing up the type > detection. Thanks for confirming. Is it ok if I add also a tested-by tag for you to the commit message when I send this to the media people? Johan ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] media: rtl28xxu: add type-detection instrumentation 2021-05-31 9:13 ` Johan Hovold @ 2021-06-02 11:01 ` Antti Palosaari 2021-06-02 12:33 ` Johan Hovold 0 siblings, 1 reply; 12+ messages in thread From: Antti Palosaari @ 2021-06-02 11:01 UTC (permalink / raw) To: Johan Hovold, Eero Lehtinen Cc: Alan Stern, Hans Verkuil, Mauro Carvalho Chehab, linux-usb On 5/31/21 12:13 PM, Johan Hovold wrote: > On Mon, May 31, 2021 at 12:08:20PM +0300, Eero Lehtinen wrote: >> On Mon, May 31, 2021 at 11:42 AM Johan Hovold <johan@kernel.org> wrote: > >>> Ok, the driver just wants to know if the i2c-read vendor request exists, >>> and actually reading the register will not work since the register may >>> not even exist (e.g. depending on the demodulator). >>> >>> So it seems we need to keep this zero-length read request and only >>> update the pipe argument to suppress the new WARN() in >>> usb_submit_urb(). >>> >>> Eero, could you try applying the below on top of -next and confirm that >>> it suppresses the warning without messing up the type detection? > >>> From 2cec8fa000152bcb997dd7aeeb0917ebf744a7bd Mon Sep 17 00:00:00 2001 >>> From: Johan Hovold <johan@kernel.org> >>> Date: Mon, 24 May 2021 10:55:19 +0200 >>> Subject: [PATCH v2] media: rtl28xxu: fix zero-length control request >>> >>> 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 <debiangamer2@gmail.com>. >>> >>> Reported-by: syzbot+faf11bbadc5a372564da@syzkaller.appspotmail.com >>> Reported-by: Eero Lehtinen <debiangamer2@gmail.com> >>> Fixes: d0f232e823af ("[media] rtl28xxu: add heuristic to detect chip type") >>> Cc: stable@vger.kernel.org # 4.0 >>> Cc: Antti Palosaari <crope@iki.fi> >>> Signed-off-by: Johan Hovold <johan@kernel.org> >>> --- >>> drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 11 ++++++++++- >>> 1 file changed, 10 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c >>> index 97ed17a141bb..a6124472cb06 100644 >>> --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c >>> +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c >>> @@ -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 > >> I confirm that it suppresses the warning without messing up the type >> detection. > > Thanks for confirming. Is it ok if I add also a tested-by tag for you to > the commit message when I send this to the media people? I can also confirm it works for both rtl2831u and rtl2832u. regards Antti ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] media: rtl28xxu: add type-detection instrumentation 2021-06-02 11:01 ` Antti Palosaari @ 2021-06-02 12:33 ` Johan Hovold 0 siblings, 0 replies; 12+ messages in thread From: Johan Hovold @ 2021-06-02 12:33 UTC (permalink / raw) To: Antti Palosaari Cc: Eero Lehtinen, Alan Stern, Hans Verkuil, Mauro Carvalho Chehab, linux-usb On Wed, Jun 02, 2021 at 02:01:02PM +0300, Antti Palosaari wrote: > On 5/31/21 12:13 PM, Johan Hovold wrote: > > On Mon, May 31, 2021 at 12:08:20PM +0300, Eero Lehtinen wrote: > >> On Mon, May 31, 2021 at 11:42 AM Johan Hovold <johan@kernel.org> wrote: > >>> From 2cec8fa000152bcb997dd7aeeb0917ebf744a7bd Mon Sep 17 00:00:00 2001 > >>> From: Johan Hovold <johan@kernel.org> > >>> Date: Mon, 24 May 2021 10:55:19 +0200 > >>> Subject: [PATCH v2] media: rtl28xxu: fix zero-length control request > >>> > >>> 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 <debiangamer2@gmail.com>. > >>> > >>> Reported-by: syzbot+faf11bbadc5a372564da@syzkaller.appspotmail.com > >>> Reported-by: Eero Lehtinen <debiangamer2@gmail.com> > >>> Fixes: d0f232e823af ("[media] rtl28xxu: add heuristic to detect chip type") > >>> Cc: stable@vger.kernel.org # 4.0 > >>> Cc: Antti Palosaari <crope@iki.fi> > >>> Signed-off-by: Johan Hovold <johan@kernel.org> > >>> --- > >>> drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 11 ++++++++++- > >>> 1 file changed, 10 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c > >>> index 97ed17a141bb..a6124472cb06 100644 > >>> --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c > >>> +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c > >>> @@ -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 > > > >> I confirm that it suppresses the warning without messing up the type > >> detection. > > > > Thanks for confirming. Is it ok if I add also a tested-by tag for you to > > the commit message when I send this to the media people? > > I can also confirm it works for both rtl2831u and rtl2832u. Thanks for testing, Antti. Johan ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-06-02 12:33 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-30 12:23 [PATCH] media: rtl28xxu: add type-detection instrumentation Eero Lehtinen
2021-05-30 13:54 ` Johan Hovold
2021-05-30 15:57 ` Eero Lehtinen
2021-05-30 18:02 ` Johan Hovold
2021-05-30 18:58 ` Eero Lehtinen
2021-05-31 6:46 ` Johan Hovold
[not found] ` <CAHS3B0Ez+eKSgrCEnW2ccpBCHc_gJ_Cs3abS_DAYXRAAjNYeTA@mail.gmail.com>
2021-05-31 7:52 ` Johan Hovold
2021-05-31 8:42 ` Johan Hovold
2021-05-31 9:08 ` Eero Lehtinen
2021-05-31 9:13 ` Johan Hovold
2021-06-02 11:01 ` Antti Palosaari
2021-06-02 12:33 ` Johan Hovold
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.