* duplicate requests on host side while streaming via uvcvideo gadget
From: Michael Grzeschik @ 2024-02-05 23:40 UTC (permalink / raw)
To: Thinh Nguyen; +Cc: linux-kernel, linux-usb
[-- Attachment #1: Type: text/plain, Size: 7219 bytes --]
Hi Thinh
I found some strange situation while streaming via uvc-gadget to some
usb host. It happens when some requests are missed due to higher load on
the gadget machine. In some cases some requests will reach the host
twice. In my special case, I added the following changes [1] for the
host and gadget side.
When applying the patches you will find all requests marked as errors on
the host and gadget side reported. However, the odd thing is that the
error counter on the host side will rise higher than the number of
requests we have actually marked as errornous on the gadget side. You
check the number of errors found on the host by looking in the
statistics and compare it with the numer of requests that are actually
marked with UVC_STREAM_ERR.
[1] applied hunks:
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index 6bdcf3bdd62a9..63515fc949880 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -91,6 +91,8 @@ struct uvc_video {
struct work_struct pump;
struct workqueue_struct *async_wq;
+ int errorcount;
+
/* Frame parameters */
u8 bpp;
u32 fcc;
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index eb1f7cee4a0af..f45dd53fde7ef 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -35,6 +35,12 @@ uvc_video_encode_header(struct uvc_video *video, struct uvc_buffer *buf,
data[1] = UVC_STREAM_EOH | video->fid;
+ if (video->queue.flags & UVC_QUEUE_DROP_INCOMPLETE) {
+ video->errorcount++;
+ printk("dropping frame! %d\n", video->errorcount);
+ data[1] |= UVC_STREAM_ERR;
+ }
+
if (video->queue.buf_used == 0 && ts.tv_sec) {
/* dwClockFrequency is 48 MHz */
u32 pts = ((u64)ts.tv_sec * USEC_PER_SEC + ts.tv_nsec / NSEC_PER_USEC) * 48;
@@ -47,6 +47,8 @@ uvc_video_encode_header(struct uvc_video *video, struct uvc_buffer *buf,
data[1] |= UVC_STREAM_PTS;
put_unaligned_le32(pts, &data[pos]);
+ if (data[1] & UVC_STREAM_ERR)
+ trace_printk("PTS: %u\n", data[pos]);
pos += 4;
}
@@ -60,6 +62,10 @@ uvc_video_encode_header(struct uvc_video *video, struct uvc_buffer *buf,
data[1] |= UVC_STREAM_SCR;
put_unaligned_le32(stc, &data[pos]);
put_unaligned_le16(sof, &data[pos+4]);
+ if (data[1] & UVC_STREAM_ERR) {
+ trace_printk("SCR: %u\n", data[pos]);
+ trace_printk("SCR: %hu\n", data[pos+4]);
+ }
pos += 6;
}
@@ -731,6 +734,7 @@ int uvcg_video_enable(struct uvc_video *video)
struct usb_gadget *gadget = cdev->gadget;
int ret;
+ video->errorcount = 0;
if (video->ep == NULL) {
uvcg_info(&video->uvc->func,
"Video enable failed, device is uninitialized.\n");
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 28dde08ec6c5d..40eafe43d1888 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -916,8 +916,24 @@ static void uvc_video_stats_decode(struct uvc_streaming *stream,
if (len <= header_size)
stream->stats.frame.nb_empty++;
- if (data[1] & UVC_STREAM_ERR)
+ if (data[1] & UVC_STREAM_ERR) {
stream->stats.frame.nb_errors++;
+ printk("error on uvc package!\n");
+ if (data[1] & UVC_STREAM_PTS) {
+ printk("PTS: %u\n", data[2]);
+ if (data[1] & UVC_STREAM_SCR) {
+ printk("SCR: %u\n", data[6]);
+ printk("SCR: %hu\n", data[10]);
+ }
+ } else {
+ if (data[1] & UVC_STREAM_SCR) {
+ printk("SCR: %u\n", data[2]);
+ printk("SCR: %hu\n", data[6]);
+ }
+ }
+
+
+ }
}
-- Host:
[ 1769.213387] error on uvc package!
[ 1769.213396] PTS: 16
[ 1769.213400] SCR: 64
[ 1769.213402] SCR: 229
[ 1769.461386] error on uvc package!
[ 1769.461394] PTS: 96
[ 1769.461398] SCR: 80
[ 1769.461401] SCR: 33
[ 1769.461405] error on uvc package! <- duplicate
[ 1769.461408] PTS: 96
[ 1769.461410] SCR: 80
[ 1769.461413] SCR: 33
[ 1769.657405] error on uvc package!
[ 1769.657442] PTS: 224
[ 1769.657449] SCR: 64
[ 1769.657453] SCR: 81
[ 1769.657460] error on uvc package! <- duplicate
[ 1769.657465] PTS: 224
[ 1769.657470] SCR: 64
[ 1769.657476] SCR: 81
[ 1779.525368] error on uvc package!
[ 1779.525374] PTS: 128
[ 1779.525378] SCR: 224
[ 1779.525380] SCR: 157
[ 1784.637359] error on uvc package!
[ 1784.637367] PTS: 0
[ 1784.637371] SCR: 208
[ 1784.637374] SCR: 89
[ 1784.825357] error on uvc package!
[ 1784.825394] PTS: 224
[ 1784.825401] SCR: 192
[ 1784.825406] SCR: 63
[ 1784.841362] error on uvc package!
[ 1784.841394] PTS: 144
[ 1784.841403] SCR: 48
[ 1784.841410] SCR: 186
[ 1784.841418] error on uvc package! <- duplicate
[ 1784.841424] PTS: 144
[ 1784.841430] SCR: 48
[ 1784.841436] SCR: 186
host$ grep errors /sys/kernel/debug/usb/uvcvideo/*/stats
/sys/kernel/debug/usb/uvcvideo/4-81-1/stats:errors: 10
-- Gadget:
[ 126.826517] dropping frame! 1
[ 126.829658] PTS: 16
[ 126.831761] SCR: 64
[ 126.833858] SCR: 229
[ 127.090069] dropping frame! 2
[ 127.093059] PTS: 96
[ 127.095164] SCR: 80
[ 127.097261] SCR: 33
[ 127.288045] dropping frame! 3
[ 127.291041] PTS: 224
[ 127.293233] SCR: 64
[ 127.295330] SCR: 81
[ 137.153499] dropping frame! 4
[ 137.156494] PTS: 128
[ 137.158687] SCR: 224
[ 137.160871] SCR: 157
[ 142.265135] dropping frame! 5
[ 142.268131] PTS: 0
[ 142.270148] SCR: 208
[ 142.272332] SCR: 89
[ 142.453636] dropping frame! 6
[ 142.456634] PTS: 224
[ 142.458825] SCR: 192
[ 142.461009] SCR: 63
[ 142.469131] dropping frame! 7
[ 142.472118] PTS: 144
[ 142.474310] SCR: 48
[ 142.476407] SCR: 186
Now I am totally unsure what could cause such error, but would expect
the issue to be somewhere in the gadget driver and the mapped trb memory
content. For the uvc_video layer, I compared and tested the enqueued
request list for duplicates but could not find any. I also reverted all
recent patches that changed request handling in the past year. I still
find these request duplicates on the host side show up.
Any Ideas?
Regards,
Michael
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [PATCH v3 3/3] usb: typec: ucsi: Get PD revision for partner
From: Abhishek Pandit-Subedi @ 2024-02-05 22:05 UTC (permalink / raw)
To: Prashant Malani
Cc: Heikki Krogerus, linux-usb, jthies, Andy Shevchenko,
Bjorn Andersson, Dmitry Baryshkov, Fabrice Gasnier,
Greg Kroah-Hartman, Hans de Goede, Neil Armstrong,
Rajaram Regupathy, Saranya Gopal, linux-kernel
In-Reply-To: <CACeCKaeVtU3ckmGU932d-pPn=eOnt6KjAavNY3rSOUgrJNriDg@mail.gmail.com>
Hi Heikki,
Friendly ping to review this patch (I see you added Reviewed-by to the
other two in this series).
Thanks,
Abhishek
On Fri, Jan 26, 2024 at 12:25 PM Prashant Malani <pmalani@chromium.org> wrote:
>
> Hi Abhishek,
>
> On Fri, Jan 26, 2024 at 10:39 AM Abhishek Pandit-Subedi
> <abhishekpandit@chromium.org> wrote:
> >
> > PD major revision for the port partner is described in
> > GET_CONNECTOR_CAPABILITY and is only valid on UCSI 2.0 and newer. Update
> > the pd_revision on the partner if the UCSI version is 2.0 or newer.
> >
> > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > ---
> > $ cat /sys/class/typec/port2-partner/usb_power_delivery_revision
> > 3.0
> >
> > (no changes since v2)
> >
> > Changes in v2:
> > - Formatting changes and update macro to use brackets.
> > - Fix incorrect guard condition when checking connector capability.
> >
> > drivers/usb/typec/ucsi/ucsi.c | 23 +++++++++++++++++++++++
> > drivers/usb/typec/ucsi/ucsi.h | 3 +++
> > 2 files changed, 26 insertions(+)
> >
> > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> > index a35056ee3e96..2b7983d2fdae 100644
> > --- a/drivers/usb/typec/ucsi/ucsi.c
> > +++ b/drivers/usb/typec/ucsi/ucsi.c
> > @@ -782,6 +782,7 @@ static int ucsi_register_partner(struct ucsi_connector *con)
> > }
> >
> > desc.usb_pd = pwr_opmode == UCSI_CONSTAT_PWR_OPMODE_PD;
> > + desc.pd_revision = UCSI_CONCAP_FLAG_PARTNER_PD_MAJOR_REV_AS_BCD(con->cap.flags);
> >
> > partner = typec_register_partner(con->port, &desc);
> > if (IS_ERR(partner)) {
> > @@ -856,6 +857,27 @@ static void ucsi_partner_change(struct ucsi_connector *con)
> > con->num, u_role);
> > }
> >
> > +static int ucsi_check_connector_capability(struct ucsi_connector *con)
> > +{
> > + u64 command;
> > + int ret;
> > +
> > + if (!con->partner || !IS_MIN_VERSION_2_0(con->ucsi))
>
> I'll reiterate my comment from a previous version, since this series
> has been revv-ed a few
> times since and it may have gotten lost; no need to respond to it if
> you don't want to,
> since I believe we left it to the maintainer(s) to decide [1]:
>
> This macro is unnecessary. Since the version is in BCD format and we
> already have the
> macros for versions, just a simple comparison is enough:
> if (!con-partner || con->ucsi->version < UCSI_VERSION_2_0)
> return 0;
>
> I'll add that Patch 1 of this series [2] is also using the same style
> for comparing version numbers.
>
> > + return 0;
> > +
> > + command = UCSI_GET_CONNECTOR_CAPABILITY | UCSI_CONNECTOR_NUMBER(con->num);
> > + ret = ucsi_send_command(con->ucsi, command, &con->cap, sizeof(con->cap));
> > + if (ret < 0) {
> > + dev_err(con->ucsi->dev, "GET_CONNECTOR_CAPABILITY failed (%d)\n", ret);
>
> nit: I know this is being done elsewhere in this file, but we should
> avoid putting error
> numbers in parentheses [3]. Perhaps something for a separate cleanup patch.
>
> [1] https://lore.kernel.org/linux-usb/CANFp7mXP=aN8bQi4akKKcoMZE8RaCBuFnwTa5hbp0MZvZe0hYQ@mail.gmail.com/
> [2] https://lore.kernel.org/linux-usb/20240126103859.v3.1.Iacf5570a66b82b73ef03daa6557e2fc0db10266a@changeid/
> [3] https://www.kernel.org/doc/html/latest/process/coding-style.html#printing-kernel-messages
^ permalink raw reply
* Re: [PATCH v13 50/53] ALSA: usb-audio: Allow for rediscovery of connected USB SND devices
From: Wesley Cheng @ 2024-02-05 21:27 UTC (permalink / raw)
To: Amadeusz Sławiński, srinivas.kandagatla, mathias.nyman,
perex, conor+dt, corbet, lgirdwood, andersson,
krzysztof.kozlowski+dt, gregkh, Thinh.Nguyen, broonie, bgoswami,
tiwai, robh+dt, konrad.dybcio
Cc: linux-kernel, devicetree, linux-sound, linux-usb, linux-arm-msm,
linux-doc, alsa-devel
In-Reply-To: <aaa76d7a-4299-4e1c-83f1-cbbea763927f@linux.intel.com>
Hi Amadeusz,
On 2/5/2024 1:01 AM, Amadeusz Sławiński wrote:
> On 2/3/2024 3:36 AM, Wesley Cheng wrote:
>> In case of notifying SND platform drivers of connection events, some of
>> these use cases, such as offloading, require an ASoC USB backend
>> device to
>> be initialized before the events can be handled. If the USB backend
>> device
>> has not yet been probed, this leads to missing initial USB audio device
>> connection events.
>>
>> Expose an API that traverses the usb_chip array for connected devices,
>> and
>> to call the respective connection callback registered to the SND platform
>> driver.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>> ---
>> sound/usb/card.c | 19 +++++++++++++++++++
>> sound/usb/card.h | 2 ++
>> sound/usb/qcom/qc_audio_offload.c | 2 ++
>> 3 files changed, 23 insertions(+)
>>
>> diff --git a/sound/usb/card.c b/sound/usb/card.c
>> index 11b827b7a2a5..995b2df676ab 100644
>> --- a/sound/usb/card.c
>> +++ b/sound/usb/card.c
>> @@ -202,6 +202,25 @@ struct snd_usb_stream
>> *snd_usb_find_suppported_substream(int card_idx,
>> }
>> EXPORT_SYMBOL_GPL(snd_usb_find_suppported_substream);
>> +/*
>> + * in case the platform driver was not ready at the time of USB SND
>> + * device connect, expose an API to discover all connected USB devices
>> + * so it can populate any dependent resources/structures.
>> + */
>> +void snd_usb_rediscover_devices(void)
>> +{
>> + int i;
>> +
>> + mutex_lock(®ister_mutex);
>> + for (i = 0; i < SNDRV_CARDS; i++) {
>> + if (usb_chip[i])
>> + if (platform_ops && platform_ops->connect_cb)
>> + platform_ops->connect_cb(usb_chip[i]);
>
> if inside if, it can just be && or maybe move callback check before
> mutex lock and just return early if it is not present?
>
Thanks for pointing that out. Makes sense, and will update it.
Thanks
Wesley Cheng
^ permalink raw reply
* Re: [PATCH v13 20/53] ASoC: Add SOC USB APIs for adding an USB backend
From: Wesley Cheng @ 2024-02-05 21:26 UTC (permalink / raw)
To: Amadeusz Sławiński, srinivas.kandagatla, mathias.nyman,
perex, conor+dt, corbet, lgirdwood, andersson,
krzysztof.kozlowski+dt, gregkh, Thinh.Nguyen, broonie, bgoswami,
tiwai, robh+dt, konrad.dybcio
Cc: linux-kernel, devicetree, linux-sound, linux-usb, linux-arm-msm,
linux-doc, alsa-devel
In-Reply-To: <2abb6c0b-ea66-4649-b205-bafe49340aee@linux.intel.com>
Hi Amadeusz,
On 2/5/2024 12:20 AM, Amadeusz Sławiński wrote:
> On 2/3/2024 3:36 AM, Wesley Cheng wrote:
>> Some platforms may have support for offloading USB audio devices to a
>> dedicated audio DSP. Introduce a set of APIs that allow for
>> management of
>> USB sound card and PCM devices enumerated by the USB SND class driver.
>> This allows for the ASoC components to be aware of what USB devices are
>> available for offloading.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>> ---
>
> ...
>
>> +
>> +/**
>> + * snd_soc_usb_add_port() - Add a USB backend port
>> + * @dev: USB backend device
>> + * @priv: private data
>> + * @connection_cb: connection status callback
>> + *
>> + * Register a USB backend device to the SND USB SOC framework.
>> Memory is
>> + * allocated as part of the USB backend device.
>> + *
>> + */
>> +int snd_soc_usb_add_port(struct snd_soc_usb *usb)
>> +{
>> +
>> +
>
> Cosmetic, but why is there white space between start of function and
> body of function?
>
Thanks for catching this. Will fix it.
Thanks
Wesley Cheng
^ permalink raw reply
* [syzbot] Monthly usb report (Feb 2024)
From: syzbot @ 2024-02-05 20:59 UTC (permalink / raw)
To: linux-kernel, linux-usb, syzkaller-bugs
Hello usb maintainers/developers,
This is a 31-day syzbot report for the usb subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/usb
During the period, 0 new issues were detected and 0 were fixed.
In total, 59 issues are still open and 332 have been fixed so far.
Some of the still happening issues:
Ref Crashes Repro Title
<1> 3116 Yes WARNING in firmware_fallback_sysfs
https://syzkaller.appspot.com/bug?extid=95f2e2439b97575ec3c0
<2> 949 Yes WARNING in implement
https://syzkaller.appspot.com/bug?extid=38e7237add3712479d65
<3> 864 Yes general protection fault in ir_raw_event_store_with_filter
https://syzkaller.appspot.com/bug?extid=34008406ee9a31b13c73
<4> 862 Yes INFO: task hung in usb_get_descriptor (2)
https://syzkaller.appspot.com/bug?extid=e8db9d9e65feff8fa471
<5> 567 Yes INFO: task hung in hub_port_init (3)
https://syzkaller.appspot.com/bug?extid=b6f11035e572f08bc20f
<6> 460 Yes INFO: task hung in usbdev_open (2)
https://syzkaller.appspot.com/bug?extid=b73659f5bb96fac34820
<7> 387 Yes INFO: task hung in r871xu_dev_remove
https://syzkaller.appspot.com/bug?extid=f39c1dad0b7db49ca4a8
<8> 304 Yes KASAN: use-after-free Read in v4l2_fh_init
https://syzkaller.appspot.com/bug?extid=c025d34b8eaa54c571b8
<9> 253 No INFO: task hung in hub_event (3)
https://syzkaller.appspot.com/bug?extid=a7edecbf389d11a369d4
<10> 247 Yes INFO: task hung in get_bMaxPacketSize0
https://syzkaller.appspot.com/bug?extid=f7ac46d91cf13b4591a4
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders
To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem
You may send multiple commands in a single email message.
^ permalink raw reply
* Re: [PATCH v3] usb: gadget: ncm: Avoid dropping datagrams of properly parsed NTBs
From: Maciej Żenczykowski @ 2024-02-05 19:33 UTC (permalink / raw)
To: Krishna Kurapati
Cc: Greg Kroah-Hartman, Hardik Gajjar, linux-usb, linux-kernel,
quic_ppratap, quic_wcheng, quic_jackp, stable
In-Reply-To: <20240205074650.200304-1-quic_kriskura@quicinc.com>
On Sun, Feb 4, 2024 at 11:47 PM Krishna Kurapati
<quic_kriskura@quicinc.com> wrote:
>
> It is observed sometimes when tethering is used over NCM with Windows 11
> as host, at some instances, the gadget_giveback has one byte appended at
> the end of a proper NTB. When the NTB is parsed, unwrap call looks for
> any leftover bytes in SKB provided by u_ether and if there are any pending
> bytes, it treats them as a separate NTB and parses it. But in case the
> second NTB (as per unwrap call) is faulty/corrupt, all the datagrams that
> were parsed properly in the first NTB and saved in rx_list are dropped.
>
> Adding a few custom traces showed the following:
> [002] d..1 7828.532866: dwc3_gadget_giveback: ep1out:
> req 000000003868811a length 1025/16384 zsI ==> 0
> [002] d..1 7828.532867: ncm_unwrap_ntb: K: ncm_unwrap_ntb toprocess: 1025
> [002] d..1 7828.532867: ncm_unwrap_ntb: K: ncm_unwrap_ntb nth: 1751999342
> [002] d..1 7828.532868: ncm_unwrap_ntb: K: ncm_unwrap_ntb seq: 0xce67
> [002] d..1 7828.532868: ncm_unwrap_ntb: K: ncm_unwrap_ntb blk_len: 0x400
> [002] d..1 7828.532868: ncm_unwrap_ntb: K: ncm_unwrap_ntb ndp_len: 0x10
> [002] d..1 7828.532869: ncm_unwrap_ntb: K: Parsed NTB with 1 frames
>
> In this case, the giveback is of 1025 bytes and block length is 1024.
> The rest 1 byte (which is 0x00) won't be parsed resulting in drop of
> all datagrams in rx_list.
>
> Same is case with packets of size 2048:
> [002] d..1 7828.557948: dwc3_gadget_giveback: ep1out:
> req 0000000011dfd96e length 2049/16384 zsI ==> 0
> [002] d..1 7828.557949: ncm_unwrap_ntb: K: ncm_unwrap_ntb nth: 1751999342
> [002] d..1 7828.557950: ncm_unwrap_ntb: K: ncm_unwrap_ntb blk_len: 0x800
>
> Lecroy shows one byte coming in extra confirming that the byte is coming
> in from PC:
>
> Transfer 2959 - Bytes Transferred(1025) Timestamp((18.524 843 590)
> - Transaction 8391 - Data(1025 bytes) Timestamp(18.524 843 590)
> --- Packet 4063861
> Data(1024 bytes)
> Duration(2.117us) Idle(14.700ns) Timestamp(18.524 843 590)
> --- Packet 4063863
> Data(1 byte)
> Duration(66.160ns) Time(282.000ns) Timestamp(18.524 845 722)
>
> According to Windows driver, no ZLP is needed if wBlockLength is non-zero,
> because the non-zero wBlockLength has already told the function side the
> size of transfer to be expected. However, there are in-market NCM devices
> that rely on ZLP as long as the wBlockLength is multiple of wMaxPacketSize.
> To deal with such devices, it pads an extra 0 at end so the transfer is no
> longer multiple of wMaxPacketSize.
>
> Cc: <stable@vger.kernel.org>
> Fixes: 9f6ce4240a2b ("usb: gadget: f_ncm.c added")
> Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
> ---
> Link to v2:
> https://lore.kernel.org/all/20240131150332.1326523-1-quic_kriskura@quicinc.com/
>
> Changes in v2:
> Added check to see if the padded byte is 0x00.
>
> Changes in v3:
> Removed wMaxPacketSize check from v2.
>
> drivers/usb/gadget/function/f_ncm.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
> index ca5d5f564998..e2a059cfda2c 100644
> --- a/drivers/usb/gadget/function/f_ncm.c
> +++ b/drivers/usb/gadget/function/f_ncm.c
> @@ -1338,7 +1338,15 @@ static int ncm_unwrap_ntb(struct gether *port,
> "Parsed NTB with %d frames\n", dgram_counter);
>
> to_process -= block_len;
> - if (to_process != 0) {
> +
> + /*
> + * Windows NCM driver avoids USB ZLPs by adding a 1-byte
> + * zero pad as needed.
> + */
> + if (to_process == 1 &&
> + (*(unsigned char *)(ntb_ptr + block_len) == 0x00)) {
> + to_process--;
> + } else if (to_process > 0) {
> ntb_ptr = (unsigned char *)(ntb_ptr + block_len);
> goto parse_ntb;
> }
> --
> 2.34.1
>
Reviewed-by: Maciej Żenczykowski <maze@google.com>
Let's get this fix out.
Greg, there's further code cleanups (here and elsewhere) I'll send
once this is merged.
I don't want to annoy Krishna further ;-)
^ permalink raw reply
* Re: [PATCH v2 5/5] usb: dwc3-am62: add workaround for Errata i2409
From: Andrew Davis @ 2024-02-05 18:07 UTC (permalink / raw)
To: Roger Quadros, Thinh.Nguyen
Cc: gregkh, r-gunasekaran, b-liu, nm, srk, linux-usb, linux-kernel,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
In-Reply-To: <20240205141221.56076-6-rogerq@kernel.org>
On 2/5/24 8:12 AM, Roger Quadros wrote:
> All AM62 devices have Errata i2409 [1] due to which
> USB2 PHY may lock up due to short suspend.
>
> Workaround involves setting bit 5 and 4 PLL_REG12
> in PHY2 register space after USB controller is brought
> out of LPSC reset but before controller initialization.
>
> Handle this workaround.
>
> [1] - https://www.ti.com/lit/er/sprz487d/sprz487d.pdf
>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
>
> Notes:
> Changelog:
>
> v2:
> - don't add phy read/write helpers or add phy to private data
>
> v1: https://lore.kernel.org/all/20240201121220.5523-5-rogerq@kernel.org/
>
> drivers/usb/dwc3/dwc3-am62.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c
> index af1ce934e7fb..5ae5c3087b0f 100644
> --- a/drivers/usb/dwc3/dwc3-am62.c
> +++ b/drivers/usb/dwc3/dwc3-am62.c
> @@ -101,6 +101,11 @@
> #define PHY_CORE_VOLTAGE_MASK BIT(31)
> #define PHY_PLL_REFCLK_MASK GENMASK(3, 0)
>
> +/* USB PHY2 register offsets */
> +#define USB_PHY_PLL_REG12 0x130
> +#define USB_PHY_PLL_LDO_REF_EN BIT(5)
> +#define USB_PHY_PLL_LDO_REF_EN_EN BIT(4)
> +
> #define DWC3_AM62_AUTOSUSPEND_DELAY 100
>
> struct dwc3_am62 {
> @@ -184,8 +189,9 @@ static int dwc3_ti_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct device_node *node = pdev->dev.of_node;
> struct dwc3_am62 *am62;
> - int i, ret;
> unsigned long rate;
> + void __iomem *phy;
> + int i, ret;
> u32 reg;
>
> am62 = devm_kzalloc(dev, sizeof(*am62), GFP_KERNEL);
> @@ -201,6 +207,12 @@ static int dwc3_ti_probe(struct platform_device *pdev)
> return PTR_ERR(am62->usbss);
> }
>
> + phy = devm_platform_ioremap_resource(pdev, 1);
> + if (IS_ERR(phy)) {
> + dev_err(dev, "can't map PHY IOMEM resource. Won't apply i2409 fix.\n");
> + phy = NULL;
> + }
Why not move this down to where you use it below, then just have
it be an if/else with the work around in the if (!IS_ERR(phy))
and the warning in the else.
Andrew
> +
> am62->usb2_refclk = devm_clk_get(dev, "ref");
> if (IS_ERR(am62->usb2_refclk)) {
> dev_err(dev, "can't get usb2_refclk\n");
> @@ -227,6 +239,13 @@ static int dwc3_ti_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> + /* Workaround Errata i2409 */
> + if (phy) {
> + reg = readl(phy + USB_PHY_PLL_REG12);
> + reg |= USB_PHY_PLL_LDO_REF_EN | USB_PHY_PLL_LDO_REF_EN_EN;
> + writel(reg, phy + USB_PHY_PLL_REG12);
> + }
> +
> /* VBUS divider select */
> am62->vbus_divider = device_property_read_bool(dev, "ti,vbus-divider");
> reg = dwc3_ti_readl(am62, USBSS_PHY_CONFIG);
^ permalink raw reply
* Re: [PATCH 4/4] usb: typec: tcpci: add support to set connector orientation
From: Marco Felsch @ 2024-02-05 17:32 UTC (permalink / raw)
To: Guenter Roeck
Cc: Dr. David Alan Gilbert, gregkh, robh+dt, krzysztof.kozlowski+dt,
conor+dt, heikki.krogerus, linux-usb, devicetree, linux-kernel,
kernel
In-Reply-To: <8cffdf64-d0ee-4e20-8c43-d3010ddf9839@roeck-us.net>
On 24-02-05, Guenter Roeck wrote:
> On 2/5/24 08:54, Marco Felsch wrote:
> > Hi David,
> >
> > On 24-02-05, Dr. David Alan Gilbert wrote:
> > > * Marco Felsch (m.felsch@pengutronix.de) wrote:
> > > > This add the support to set the optional connector orientation bit which
> > > > is part of the optional CONFIG_STANDARD_OUTPUT register 0x18 [1]. This
> > > > allows system designers to connect the tcpc orientation pin directly to
> > > > the 2:1 ss-mux.
> > > >
> > > > [1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf
> > > >
> > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > > ---
> > > > drivers/usb/typec/tcpm/tcpci.c | 43 ++++++++++++++++++++++++++++++++++
> > > > include/linux/usb/tcpci.h | 8 +++++++
> > > > 2 files changed, 51 insertions(+)
> > > >
> > > > diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> > > > index 7118551827f6..7ce9d4923bc7 100644
> > > > --- a/drivers/usb/typec/tcpm/tcpci.c
> > > > +++ b/drivers/usb/typec/tcpm/tcpci.c
> > > > @@ -67,6 +67,18 @@ static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
> > > > return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u16));
> > > > }
> > > > +static bool tcpci_check_std_output_cap(struct regmap *regmap, u8 mask)
> > > > +{
> > > > + unsigned int reg;
> > > > + int ret;
> > > > +
> > > > + ret = regmap_read(regmap, TCPC_STD_OUTPUT_CAP, ®);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > +
> > > > + return (reg & mask) == mask;
> > > > +}
> > > > +
> > > > static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
> > > > {
> > > > struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > > > @@ -301,6 +313,27 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
> > > > TCPC_TCPC_CTRL_ORIENTATION : 0);
> > > > }
> > > > +static int tcpci_set_orientation(struct tcpc_dev *tcpc,
> > > > + enum typec_orientation orientation)
> > > > +{
> > > > + struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > > > + unsigned int reg;
> > > > +
> > > > + switch (orientation) {
> > > > + case TYPEC_ORIENTATION_NONE:
> > > > + /* We can't put a single output into high impedance */
> > >
> > > Is that intended to be a fallthrough? If so I guess it needs
> > > marking as such with a
> > > fallthrough;
> >
> > You need to add it if there is code in between. Since there is no code,
> > just this comment, it shouldn't be necessary.
> >
>
> Still, I think it would be desirable here to clarify that this
> is not a lost return but intentionally sets
> TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL.
Sure, I will adapt it once the complete patchset is reviewed.
Regards,
Marco
> Guenter
>
> > Regards,
> > Marco
> >
> > >
> > > Dave
> > >
> > > > + case TYPEC_ORIENTATION_NORMAL:
> > > > + reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL;
> > > > + break;
> > > > + case TYPEC_ORIENTATION_REVERSE:
> > > > + reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED;
> > > > + break;
> > > > + }
> > > > +
> > > > + return regmap_update_bits(tcpci->regmap, TCPC_CONFIG_STD_OUTPUT,
> > > > + TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK, reg);
> > > > +}
> > > > +
> > > > static void tcpci_set_partner_usb_comm_capable(struct tcpc_dev *tcpc, bool capable)
> > > > {
> > > > struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > > > @@ -808,6 +841,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
> > > > if (tcpci->data->vbus_vsafe0v)
> > > > tcpci->tcpc.is_vbus_vsafe0v = tcpci_is_vbus_vsafe0v;
> > > > + if (tcpci->data->set_orientation)
> > > > + tcpci->tcpc.set_orientation = tcpci_set_orientation;
> > > > +
> > > > err = tcpci_parse_config(tcpci);
> > > > if (err < 0)
> > > > return ERR_PTR(err);
> > > > @@ -851,6 +887,13 @@ static int tcpci_probe(struct i2c_client *client)
> > > > if (err < 0)
> > > > return err;
> > > > + err = tcpci_check_std_output_cap(chip->data.regmap,
> > > > + TCPC_STD_OUTPUT_CAP_ORIENTATION);
> > > > + if (err < 0)
> > > > + return err;
> > > > +
> > > > + chip->data.set_orientation = err;
> > > > +
> > > > chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
> > > > if (IS_ERR(chip->tcpci))
> > > > return PTR_ERR(chip->tcpci);
> > > > diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
> > > > index 467e8045e9f8..f2bfb4250366 100644
> > > > --- a/include/linux/usb/tcpci.h
> > > > +++ b/include/linux/usb/tcpci.h
> > > > @@ -47,6 +47,9 @@
> > > > #define TCPC_SINK_FAST_ROLE_SWAP BIT(0)
> > > > #define TCPC_CONFIG_STD_OUTPUT 0x18
> > > > +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK BIT(0)
> > > > +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL 0
> > > > +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED 1
> > > > #define TCPC_TCPC_CTRL 0x19
> > > > #define TCPC_TCPC_CTRL_ORIENTATION BIT(0)
> > > > @@ -127,6 +130,7 @@
> > > > #define TCPC_DEV_CAP_2 0x26
> > > > #define TCPC_STD_INPUT_CAP 0x28
> > > > #define TCPC_STD_OUTPUT_CAP 0x29
> > > > +#define TCPC_STD_OUTPUT_CAP_ORIENTATION BIT(0)
> > > > #define TCPC_MSG_HDR_INFO 0x2e
> > > > #define TCPC_MSG_HDR_INFO_DATA_ROLE BIT(3)
> > > > @@ -198,12 +202,16 @@ struct tcpci;
> > > > * Chip level drivers are expected to check for contaminant and call
> > > > * tcpm_clean_port when the port is clean to put the port back into
> > > > * toggling state.
> > > > + * @set_orientation:
> > > > + * Optional; Enable setting the connector orientation
> > > > + * CONFIG_STANDARD_OUTPUT (0x18) bit0.
> > > > */
> > > > struct tcpci_data {
> > > > struct regmap *regmap;
> > > > unsigned char TX_BUF_BYTE_x_hidden:1;
> > > > unsigned char auto_discharge_disconnect:1;
> > > > unsigned char vbus_vsafe0v:1;
> > > > + unsigned char set_orientation:1;
> > > > int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
> > > > int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
> > > > --
> > > > 2.39.2
> > > >
> > > >
> > > --
> > > -----Open up your eyes, open up your mind, open up your code -------
> > > / Dr. David Alan Gilbert | Running GNU/Linux | Happy \
> > > \ dave @ treblig.org | | In Hex /
> > > \ _________________________|_____ http://www.treblig.org |_______/
> > >
>
>
^ permalink raw reply
* Re: [PATCH 4/4] usb: typec: tcpci: add support to set connector orientation
From: Guenter Roeck @ 2024-02-05 17:11 UTC (permalink / raw)
To: Marco Felsch, Dr. David Alan Gilbert
Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, conor+dt,
heikki.krogerus, linux-usb, devicetree, linux-kernel, kernel
In-Reply-To: <20240205165420.kyujim2takwswzmw@pengutronix.de>
On 2/5/24 08:54, Marco Felsch wrote:
> Hi David,
>
> On 24-02-05, Dr. David Alan Gilbert wrote:
>> * Marco Felsch (m.felsch@pengutronix.de) wrote:
>>> This add the support to set the optional connector orientation bit which
>>> is part of the optional CONFIG_STANDARD_OUTPUT register 0x18 [1]. This
>>> allows system designers to connect the tcpc orientation pin directly to
>>> the 2:1 ss-mux.
>>>
>>> [1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf
>>>
>>> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
>>> ---
>>> drivers/usb/typec/tcpm/tcpci.c | 43 ++++++++++++++++++++++++++++++++++
>>> include/linux/usb/tcpci.h | 8 +++++++
>>> 2 files changed, 51 insertions(+)
>>>
>>> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
>>> index 7118551827f6..7ce9d4923bc7 100644
>>> --- a/drivers/usb/typec/tcpm/tcpci.c
>>> +++ b/drivers/usb/typec/tcpm/tcpci.c
>>> @@ -67,6 +67,18 @@ static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
>>> return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u16));
>>> }
>>>
>>> +static bool tcpci_check_std_output_cap(struct regmap *regmap, u8 mask)
>>> +{
>>> + unsigned int reg;
>>> + int ret;
>>> +
>>> + ret = regmap_read(regmap, TCPC_STD_OUTPUT_CAP, ®);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + return (reg & mask) == mask;
>>> +}
>>> +
>>> static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
>>> {
>>> struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
>>> @@ -301,6 +313,27 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
>>> TCPC_TCPC_CTRL_ORIENTATION : 0);
>>> }
>>>
>>> +static int tcpci_set_orientation(struct tcpc_dev *tcpc,
>>> + enum typec_orientation orientation)
>>> +{
>>> + struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
>>> + unsigned int reg;
>>> +
>>> + switch (orientation) {
>>> + case TYPEC_ORIENTATION_NONE:
>>> + /* We can't put a single output into high impedance */
>>
>> Is that intended to be a fallthrough? If so I guess it needs
>> marking as such with a
>> fallthrough;
>
> You need to add it if there is code in between. Since there is no code,
> just this comment, it shouldn't be necessary.
>
Still, I think it would be desirable here to clarify that this
is not a lost return but intentionally sets
TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL.
Guenter
> Regards,
> Marco
>
>>
>> Dave
>>
>>> + case TYPEC_ORIENTATION_NORMAL:
>>> + reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL;
>>> + break;
>>> + case TYPEC_ORIENTATION_REVERSE:
>>> + reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED;
>>> + break;
>>> + }
>>> +
>>> + return regmap_update_bits(tcpci->regmap, TCPC_CONFIG_STD_OUTPUT,
>>> + TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK, reg);
>>> +}
>>> +
>>> static void tcpci_set_partner_usb_comm_capable(struct tcpc_dev *tcpc, bool capable)
>>> {
>>> struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
>>> @@ -808,6 +841,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
>>> if (tcpci->data->vbus_vsafe0v)
>>> tcpci->tcpc.is_vbus_vsafe0v = tcpci_is_vbus_vsafe0v;
>>>
>>> + if (tcpci->data->set_orientation)
>>> + tcpci->tcpc.set_orientation = tcpci_set_orientation;
>>> +
>>> err = tcpci_parse_config(tcpci);
>>> if (err < 0)
>>> return ERR_PTR(err);
>>> @@ -851,6 +887,13 @@ static int tcpci_probe(struct i2c_client *client)
>>> if (err < 0)
>>> return err;
>>>
>>> + err = tcpci_check_std_output_cap(chip->data.regmap,
>>> + TCPC_STD_OUTPUT_CAP_ORIENTATION);
>>> + if (err < 0)
>>> + return err;
>>> +
>>> + chip->data.set_orientation = err;
>>> +
>>> chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
>>> if (IS_ERR(chip->tcpci))
>>> return PTR_ERR(chip->tcpci);
>>> diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
>>> index 467e8045e9f8..f2bfb4250366 100644
>>> --- a/include/linux/usb/tcpci.h
>>> +++ b/include/linux/usb/tcpci.h
>>> @@ -47,6 +47,9 @@
>>> #define TCPC_SINK_FAST_ROLE_SWAP BIT(0)
>>>
>>> #define TCPC_CONFIG_STD_OUTPUT 0x18
>>> +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK BIT(0)
>>> +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL 0
>>> +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED 1
>>>
>>> #define TCPC_TCPC_CTRL 0x19
>>> #define TCPC_TCPC_CTRL_ORIENTATION BIT(0)
>>> @@ -127,6 +130,7 @@
>>> #define TCPC_DEV_CAP_2 0x26
>>> #define TCPC_STD_INPUT_CAP 0x28
>>> #define TCPC_STD_OUTPUT_CAP 0x29
>>> +#define TCPC_STD_OUTPUT_CAP_ORIENTATION BIT(0)
>>>
>>> #define TCPC_MSG_HDR_INFO 0x2e
>>> #define TCPC_MSG_HDR_INFO_DATA_ROLE BIT(3)
>>> @@ -198,12 +202,16 @@ struct tcpci;
>>> * Chip level drivers are expected to check for contaminant and call
>>> * tcpm_clean_port when the port is clean to put the port back into
>>> * toggling state.
>>> + * @set_orientation:
>>> + * Optional; Enable setting the connector orientation
>>> + * CONFIG_STANDARD_OUTPUT (0x18) bit0.
>>> */
>>> struct tcpci_data {
>>> struct regmap *regmap;
>>> unsigned char TX_BUF_BYTE_x_hidden:1;
>>> unsigned char auto_discharge_disconnect:1;
>>> unsigned char vbus_vsafe0v:1;
>>> + unsigned char set_orientation:1;
>>>
>>> int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
>>> int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
>>> --
>>> 2.39.2
>>>
>>>
>> --
>> -----Open up your eyes, open up your mind, open up your code -------
>> / Dr. David Alan Gilbert | Running GNU/Linux | Happy \
>> \ dave @ treblig.org | | In Hex /
>> \ _________________________|_____ http://www.treblig.org |_______/
>>
^ permalink raw reply
* Re: [PATCH 4/4] usb: typec: tcpci: add support to set connector orientation
From: Dr. David Alan Gilbert @ 2024-02-05 16:51 UTC (permalink / raw)
To: Marco Felsch
Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux,
heikki.krogerus, linux-usb, devicetree, linux-kernel, kernel
In-Reply-To: <20240205164316.805408-5-m.felsch@pengutronix.de>
* Marco Felsch (m.felsch@pengutronix.de) wrote:
> This add the support to set the optional connector orientation bit which
> is part of the optional CONFIG_STANDARD_OUTPUT register 0x18 [1]. This
> allows system designers to connect the tcpc orientation pin directly to
> the 2:1 ss-mux.
>
> [1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> drivers/usb/typec/tcpm/tcpci.c | 43 ++++++++++++++++++++++++++++++++++
> include/linux/usb/tcpci.h | 8 +++++++
> 2 files changed, 51 insertions(+)
>
> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index 7118551827f6..7ce9d4923bc7 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c
> @@ -67,6 +67,18 @@ static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
> return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u16));
> }
>
> +static bool tcpci_check_std_output_cap(struct regmap *regmap, u8 mask)
> +{
> + unsigned int reg;
> + int ret;
> +
> + ret = regmap_read(regmap, TCPC_STD_OUTPUT_CAP, ®);
> + if (ret < 0)
> + return ret;
> +
> + return (reg & mask) == mask;
> +}
> +
> static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
> {
> struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> @@ -301,6 +313,27 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
> TCPC_TCPC_CTRL_ORIENTATION : 0);
> }
>
> +static int tcpci_set_orientation(struct tcpc_dev *tcpc,
> + enum typec_orientation orientation)
> +{
> + struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> + unsigned int reg;
> +
> + switch (orientation) {
> + case TYPEC_ORIENTATION_NONE:
> + /* We can't put a single output into high impedance */
Is that intended to be a fallthrough? If so I guess it needs
marking as such with a
fallthrough;
Dave
> + case TYPEC_ORIENTATION_NORMAL:
> + reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL;
> + break;
> + case TYPEC_ORIENTATION_REVERSE:
> + reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED;
> + break;
> + }
> +
> + return regmap_update_bits(tcpci->regmap, TCPC_CONFIG_STD_OUTPUT,
> + TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK, reg);
> +}
> +
> static void tcpci_set_partner_usb_comm_capable(struct tcpc_dev *tcpc, bool capable)
> {
> struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> @@ -808,6 +841,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
> if (tcpci->data->vbus_vsafe0v)
> tcpci->tcpc.is_vbus_vsafe0v = tcpci_is_vbus_vsafe0v;
>
> + if (tcpci->data->set_orientation)
> + tcpci->tcpc.set_orientation = tcpci_set_orientation;
> +
> err = tcpci_parse_config(tcpci);
> if (err < 0)
> return ERR_PTR(err);
> @@ -851,6 +887,13 @@ static int tcpci_probe(struct i2c_client *client)
> if (err < 0)
> return err;
>
> + err = tcpci_check_std_output_cap(chip->data.regmap,
> + TCPC_STD_OUTPUT_CAP_ORIENTATION);
> + if (err < 0)
> + return err;
> +
> + chip->data.set_orientation = err;
> +
> chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
> if (IS_ERR(chip->tcpci))
> return PTR_ERR(chip->tcpci);
> diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
> index 467e8045e9f8..f2bfb4250366 100644
> --- a/include/linux/usb/tcpci.h
> +++ b/include/linux/usb/tcpci.h
> @@ -47,6 +47,9 @@
> #define TCPC_SINK_FAST_ROLE_SWAP BIT(0)
>
> #define TCPC_CONFIG_STD_OUTPUT 0x18
> +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK BIT(0)
> +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL 0
> +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED 1
>
> #define TCPC_TCPC_CTRL 0x19
> #define TCPC_TCPC_CTRL_ORIENTATION BIT(0)
> @@ -127,6 +130,7 @@
> #define TCPC_DEV_CAP_2 0x26
> #define TCPC_STD_INPUT_CAP 0x28
> #define TCPC_STD_OUTPUT_CAP 0x29
> +#define TCPC_STD_OUTPUT_CAP_ORIENTATION BIT(0)
>
> #define TCPC_MSG_HDR_INFO 0x2e
> #define TCPC_MSG_HDR_INFO_DATA_ROLE BIT(3)
> @@ -198,12 +202,16 @@ struct tcpci;
> * Chip level drivers are expected to check for contaminant and call
> * tcpm_clean_port when the port is clean to put the port back into
> * toggling state.
> + * @set_orientation:
> + * Optional; Enable setting the connector orientation
> + * CONFIG_STANDARD_OUTPUT (0x18) bit0.
> */
> struct tcpci_data {
> struct regmap *regmap;
> unsigned char TX_BUF_BYTE_x_hidden:1;
> unsigned char auto_discharge_disconnect:1;
> unsigned char vbus_vsafe0v:1;
> + unsigned char set_orientation:1;
>
> int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
> int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
> --
> 2.39.2
>
>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
^ permalink raw reply
* Re: [PATCH 4/4] usb: typec: tcpci: add support to set connector orientation
From: Marco Felsch @ 2024-02-05 16:54 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: gregkh, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux,
heikki.krogerus, linux-usb, devicetree, linux-kernel, kernel
In-Reply-To: <ZcESKqRTsGNZMMX1@gallifrey>
Hi David,
On 24-02-05, Dr. David Alan Gilbert wrote:
> * Marco Felsch (m.felsch@pengutronix.de) wrote:
> > This add the support to set the optional connector orientation bit which
> > is part of the optional CONFIG_STANDARD_OUTPUT register 0x18 [1]. This
> > allows system designers to connect the tcpc orientation pin directly to
> > the 2:1 ss-mux.
> >
> > [1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf
> >
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> > drivers/usb/typec/tcpm/tcpci.c | 43 ++++++++++++++++++++++++++++++++++
> > include/linux/usb/tcpci.h | 8 +++++++
> > 2 files changed, 51 insertions(+)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> > index 7118551827f6..7ce9d4923bc7 100644
> > --- a/drivers/usb/typec/tcpm/tcpci.c
> > +++ b/drivers/usb/typec/tcpm/tcpci.c
> > @@ -67,6 +67,18 @@ static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
> > return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u16));
> > }
> >
> > +static bool tcpci_check_std_output_cap(struct regmap *regmap, u8 mask)
> > +{
> > + unsigned int reg;
> > + int ret;
> > +
> > + ret = regmap_read(regmap, TCPC_STD_OUTPUT_CAP, ®);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return (reg & mask) == mask;
> > +}
> > +
> > static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
> > {
> > struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > @@ -301,6 +313,27 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
> > TCPC_TCPC_CTRL_ORIENTATION : 0);
> > }
> >
> > +static int tcpci_set_orientation(struct tcpc_dev *tcpc,
> > + enum typec_orientation orientation)
> > +{
> > + struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > + unsigned int reg;
> > +
> > + switch (orientation) {
> > + case TYPEC_ORIENTATION_NONE:
> > + /* We can't put a single output into high impedance */
>
> Is that intended to be a fallthrough? If so I guess it needs
> marking as such with a
> fallthrough;
You need to add it if there is code in between. Since there is no code,
just this comment, it shouldn't be necessary.
Regards,
Marco
>
> Dave
>
> > + case TYPEC_ORIENTATION_NORMAL:
> > + reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL;
> > + break;
> > + case TYPEC_ORIENTATION_REVERSE:
> > + reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED;
> > + break;
> > + }
> > +
> > + return regmap_update_bits(tcpci->regmap, TCPC_CONFIG_STD_OUTPUT,
> > + TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK, reg);
> > +}
> > +
> > static void tcpci_set_partner_usb_comm_capable(struct tcpc_dev *tcpc, bool capable)
> > {
> > struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > @@ -808,6 +841,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
> > if (tcpci->data->vbus_vsafe0v)
> > tcpci->tcpc.is_vbus_vsafe0v = tcpci_is_vbus_vsafe0v;
> >
> > + if (tcpci->data->set_orientation)
> > + tcpci->tcpc.set_orientation = tcpci_set_orientation;
> > +
> > err = tcpci_parse_config(tcpci);
> > if (err < 0)
> > return ERR_PTR(err);
> > @@ -851,6 +887,13 @@ static int tcpci_probe(struct i2c_client *client)
> > if (err < 0)
> > return err;
> >
> > + err = tcpci_check_std_output_cap(chip->data.regmap,
> > + TCPC_STD_OUTPUT_CAP_ORIENTATION);
> > + if (err < 0)
> > + return err;
> > +
> > + chip->data.set_orientation = err;
> > +
> > chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
> > if (IS_ERR(chip->tcpci))
> > return PTR_ERR(chip->tcpci);
> > diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
> > index 467e8045e9f8..f2bfb4250366 100644
> > --- a/include/linux/usb/tcpci.h
> > +++ b/include/linux/usb/tcpci.h
> > @@ -47,6 +47,9 @@
> > #define TCPC_SINK_FAST_ROLE_SWAP BIT(0)
> >
> > #define TCPC_CONFIG_STD_OUTPUT 0x18
> > +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK BIT(0)
> > +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL 0
> > +#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED 1
> >
> > #define TCPC_TCPC_CTRL 0x19
> > #define TCPC_TCPC_CTRL_ORIENTATION BIT(0)
> > @@ -127,6 +130,7 @@
> > #define TCPC_DEV_CAP_2 0x26
> > #define TCPC_STD_INPUT_CAP 0x28
> > #define TCPC_STD_OUTPUT_CAP 0x29
> > +#define TCPC_STD_OUTPUT_CAP_ORIENTATION BIT(0)
> >
> > #define TCPC_MSG_HDR_INFO 0x2e
> > #define TCPC_MSG_HDR_INFO_DATA_ROLE BIT(3)
> > @@ -198,12 +202,16 @@ struct tcpci;
> > * Chip level drivers are expected to check for contaminant and call
> > * tcpm_clean_port when the port is clean to put the port back into
> > * toggling state.
> > + * @set_orientation:
> > + * Optional; Enable setting the connector orientation
> > + * CONFIG_STANDARD_OUTPUT (0x18) bit0.
> > */
> > struct tcpci_data {
> > struct regmap *regmap;
> > unsigned char TX_BUF_BYTE_x_hidden:1;
> > unsigned char auto_discharge_disconnect:1;
> > unsigned char vbus_vsafe0v:1;
> > + unsigned char set_orientation:1;
> >
> > int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
> > int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
> > --
> > 2.39.2
> >
> >
> --
> -----Open up your eyes, open up your mind, open up your code -------
> / Dr. David Alan Gilbert | Running GNU/Linux | Happy \
> \ dave @ treblig.org | | In Hex /
> \ _________________________|_____ http://www.treblig.org |_______/
>
^ permalink raw reply
* [PATCH 3/4] usb: typec: tcpm: add support to set tcpc connector orientatition
From: Marco Felsch @ 2024-02-05 16:43 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux,
heikki.krogerus
Cc: linux-usb, devicetree, linux-kernel, kernel
In-Reply-To: <20240205164316.805408-1-m.felsch@pengutronix.de>
This adds the support to set the connector orientation value
accordingly. This is part of the optional CONFIG_STANDARD_OUTPUT
register 0x18, specified within the USB port controller spsicification
rev. 2.0 [1].
[1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
drivers/usb/typec/tcpm/tcpm.c | 6 ++++++
include/linux/usb/tcpm.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 5945e3a2b0f7..85ca26687324 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -1099,6 +1099,12 @@ static int tcpm_set_roles(struct tcpm_port *port, bool attached,
if (ret < 0)
return ret;
+ if (port->tcpc->set_orientation) {
+ ret = port->tcpc->set_orientation(port->tcpc, orientation);
+ if (ret < 0)
+ return ret;
+ }
+
port->pwr_role = role;
port->data_role = data;
typec_set_data_role(port->typec_port, data);
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index 65fac5e1f317..93b681ff3ef9 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -133,6 +133,8 @@ struct tcpc_dev {
enum typec_cc_status *cc2);
int (*set_polarity)(struct tcpc_dev *dev,
enum typec_cc_polarity polarity);
+ int (*set_orientation)(struct tcpc_dev *dev,
+ enum typec_orientation orientation);
int (*set_vconn)(struct tcpc_dev *dev, bool on);
int (*set_vbus)(struct tcpc_dev *dev, bool on, bool charge);
int (*set_current_limit)(struct tcpc_dev *dev, u32 max_ma, u32 mv);
--
2.39.2
^ permalink raw reply related
* [PATCH 4/4] usb: typec: tcpci: add support to set connector orientation
From: Marco Felsch @ 2024-02-05 16:43 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux,
heikki.krogerus
Cc: linux-usb, devicetree, linux-kernel, kernel
In-Reply-To: <20240205164316.805408-1-m.felsch@pengutronix.de>
This add the support to set the optional connector orientation bit which
is part of the optional CONFIG_STANDARD_OUTPUT register 0x18 [1]. This
allows system designers to connect the tcpc orientation pin directly to
the 2:1 ss-mux.
[1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
drivers/usb/typec/tcpm/tcpci.c | 43 ++++++++++++++++++++++++++++++++++
include/linux/usb/tcpci.h | 8 +++++++
2 files changed, 51 insertions(+)
diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index 7118551827f6..7ce9d4923bc7 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -67,6 +67,18 @@ static int tcpci_write16(struct tcpci *tcpci, unsigned int reg, u16 val)
return regmap_raw_write(tcpci->regmap, reg, &val, sizeof(u16));
}
+static bool tcpci_check_std_output_cap(struct regmap *regmap, u8 mask)
+{
+ unsigned int reg;
+ int ret;
+
+ ret = regmap_read(regmap, TCPC_STD_OUTPUT_CAP, ®);
+ if (ret < 0)
+ return ret;
+
+ return (reg & mask) == mask;
+}
+
static int tcpci_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
{
struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
@@ -301,6 +313,27 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
TCPC_TCPC_CTRL_ORIENTATION : 0);
}
+static int tcpci_set_orientation(struct tcpc_dev *tcpc,
+ enum typec_orientation orientation)
+{
+ struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
+ unsigned int reg;
+
+ switch (orientation) {
+ case TYPEC_ORIENTATION_NONE:
+ /* We can't put a single output into high impedance */
+ case TYPEC_ORIENTATION_NORMAL:
+ reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL;
+ break;
+ case TYPEC_ORIENTATION_REVERSE:
+ reg = TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED;
+ break;
+ }
+
+ return regmap_update_bits(tcpci->regmap, TCPC_CONFIG_STD_OUTPUT,
+ TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK, reg);
+}
+
static void tcpci_set_partner_usb_comm_capable(struct tcpc_dev *tcpc, bool capable)
{
struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
@@ -808,6 +841,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
if (tcpci->data->vbus_vsafe0v)
tcpci->tcpc.is_vbus_vsafe0v = tcpci_is_vbus_vsafe0v;
+ if (tcpci->data->set_orientation)
+ tcpci->tcpc.set_orientation = tcpci_set_orientation;
+
err = tcpci_parse_config(tcpci);
if (err < 0)
return ERR_PTR(err);
@@ -851,6 +887,13 @@ static int tcpci_probe(struct i2c_client *client)
if (err < 0)
return err;
+ err = tcpci_check_std_output_cap(chip->data.regmap,
+ TCPC_STD_OUTPUT_CAP_ORIENTATION);
+ if (err < 0)
+ return err;
+
+ chip->data.set_orientation = err;
+
chip->tcpci = tcpci_register_port(&client->dev, &chip->data);
if (IS_ERR(chip->tcpci))
return PTR_ERR(chip->tcpci);
diff --git a/include/linux/usb/tcpci.h b/include/linux/usb/tcpci.h
index 467e8045e9f8..f2bfb4250366 100644
--- a/include/linux/usb/tcpci.h
+++ b/include/linux/usb/tcpci.h
@@ -47,6 +47,9 @@
#define TCPC_SINK_FAST_ROLE_SWAP BIT(0)
#define TCPC_CONFIG_STD_OUTPUT 0x18
+#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_MASK BIT(0)
+#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_NORMAL 0
+#define TCPC_CONFIG_STD_OUTPUT_ORIENTATION_FLIPPED 1
#define TCPC_TCPC_CTRL 0x19
#define TCPC_TCPC_CTRL_ORIENTATION BIT(0)
@@ -127,6 +130,7 @@
#define TCPC_DEV_CAP_2 0x26
#define TCPC_STD_INPUT_CAP 0x28
#define TCPC_STD_OUTPUT_CAP 0x29
+#define TCPC_STD_OUTPUT_CAP_ORIENTATION BIT(0)
#define TCPC_MSG_HDR_INFO 0x2e
#define TCPC_MSG_HDR_INFO_DATA_ROLE BIT(3)
@@ -198,12 +202,16 @@ struct tcpci;
* Chip level drivers are expected to check for contaminant and call
* tcpm_clean_port when the port is clean to put the port back into
* toggling state.
+ * @set_orientation:
+ * Optional; Enable setting the connector orientation
+ * CONFIG_STANDARD_OUTPUT (0x18) bit0.
*/
struct tcpci_data {
struct regmap *regmap;
unsigned char TX_BUF_BYTE_x_hidden:1;
unsigned char auto_discharge_disconnect:1;
unsigned char vbus_vsafe0v:1;
+ unsigned char set_orientation:1;
int (*init)(struct tcpci *tcpci, struct tcpci_data *data);
int (*set_vconn)(struct tcpci *tcpci, struct tcpci_data *data,
--
2.39.2
^ permalink raw reply related
* [PATCH 1/4] dt-bindings: usb: typec-tcpci: add tcpci compatible binding
From: Marco Felsch @ 2024-02-05 16:43 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux,
heikki.krogerus
Cc: linux-usb, devicetree, linux-kernel, kernel
In-Reply-To: <20240205164316.805408-1-m.felsch@pengutronix.de>
This binding descripes the generic TCPCI specification [1]. So add the
generic binding support since which can be used if an different TCPC is
used compatible which is compatible to [1].
[1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml b/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml
index eaedb4cc6b6c..7bd7bbbac9e0 100644
--- a/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml
+++ b/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml
@@ -11,7 +11,9 @@ maintainers:
properties:
compatible:
- const: nxp,ptn5110
+ enum:
+ - nxp,ptn5110
+ - tcpci
reg:
maxItems: 1
--
2.39.2
^ permalink raw reply related
* [PATCH 0/4] USB-C TCPM Orientation Support
From: Marco Felsch @ 2024-02-05 16:43 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux,
heikki.krogerus
Cc: linux-usb, devicetree, linux-kernel, kernel
Hi,
this adds the support to controll the optional connector-orientation
available on some TCPC from the TCPM.
I used an custom board with OnSemi FUSB307B TCPC which is spec [1]
compatible but albeit the spec [1] says that this pin is controlled by
the TCPC if 'TCPC_CONTROL.DebugAccessoryControl = 0' it isn't at least
for this device.
I'm unsure if the usb tcpci spec has an copy'n'paste failure since
'TCPC_CONTROL.DebugAccessoryControl' shouldn't control the state of the
'connector orientation' pin or if the OnSemi FUSB307B has an HW bug.
Because on my device the 'TCPC_CONTROL.DebugAccessoryControl' is set to
0 but the register wasn't updated automatically.
Regards,
Marco
[1] https://www.usb.org/sites/default/files/documents/usb-port_controller_specification_rev2.0_v1.0_0.pdf
Marco Felsch (4):
dt-bindings: usb: typec-tcpci: add tcpci compatible binding
usb: typec: tcpci: add generic tcpci compatible
usb: typec: tcpm: add support to set tcpc connector orientatition
usb: typec: tcpci: add support to set connector orientation
.../devicetree/bindings/usb/nxp,ptn5110.yaml | 4 +-
drivers/usb/typec/tcpm/tcpci.c | 44 +++++++++++++++++++
drivers/usb/typec/tcpm/tcpm.c | 6 +++
include/linux/usb/tcpci.h | 8 ++++
include/linux/usb/tcpm.h | 2 +
5 files changed, 63 insertions(+), 1 deletion(-)
--
2.39.2
^ permalink raw reply
* [PATCH 2/4] usb: typec: tcpci: add generic tcpci compatible
From: Marco Felsch @ 2024-02-05 16:43 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, conor+dt, linux,
heikki.krogerus
Cc: linux-usb, devicetree, linux-kernel, kernel
In-Reply-To: <20240205164316.805408-1-m.felsch@pengutronix.de>
The driver already support the tcpci binding for the i2c_device_id so
add the support for the of_device_id too.
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
drivers/usb/typec/tcpm/tcpci.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index 0ee3e6e29bb1..7118551827f6 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -889,6 +889,7 @@ MODULE_DEVICE_TABLE(i2c, tcpci_id);
#ifdef CONFIG_OF
static const struct of_device_id tcpci_of_match[] = {
{ .compatible = "nxp,ptn5110", },
+ { .compatible = "tcpci", },
{},
};
MODULE_DEVICE_TABLE(of, tcpci_of_match);
--
2.39.2
^ permalink raw reply related
* Re: usb: gadget: dwc2: RK3308: Transmission to EP OUT stalls at larger packet size
From: Pavel Hofman @ 2024-02-05 16:20 UTC (permalink / raw)
To: Minas Harutyunyan; +Cc: linux-usb@vger.kernel.org
In-Reply-To: <0efd145e-eacd-412d-a937-7c7a91790de7@synopsys.com>
Dne 05. 02. 24 v 17:05 Minas Harutyunyan napsal(a):
> Hi Pavel,
>
> On 2/5/24 19:52, Pavel Hofman wrote:
>>
>>
>> Dne 05. 02. 24 v 15:35 Pavel Hofman napsal(a):
>>>
>>>>
>>> It really looks like some DMA performance issue. Stream 980 bytes/
>>> 250us (bInterval=2) is bitperfect, no dropped packets. While 24
>>> bytes/125us (bInterval=1) gets stuck. IIUC the DMA is not capable of
>>> copying packets every 125us. Please is there any chance to tweak the
>>> performance to handle the 125us packets reliably?
>>>
>>
>> I tried increasing f_uac2 req_number/UAC2_DEF_REQ_NUM from 2 to 8 and
>> streaming seems to run stable at 125us microframes now (in both
>> directions simultaneously). Please is there any other gadget tweak which
>> could potentially reduce the risk of dropped packets? Something like
>> using plain DMA instead of desc DMA (no idea :-) )...
>>
>> Thanks a lot for your expert opinion.
>
>
> Yes, it's mandatory to increase f_uac2_req_number, at least 4. Obviously
> 2 is not enough for descriptor list and main cause of BNA interrupt.
Interestingly, increasing req_number from 8 to 16 did not seem to have
any effect on frequency of the BNAs:
[ 2444.657558] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 2444.659406] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 2444.683667] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 2444.723693] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 2444.725426] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 2444.763671] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 2444.783599] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 2444.795679] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 2444.833475] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 2444.839453] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 2444.917478] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 2444.923416] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 2444.935450] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
> Another suggestion to change DMA mode from DDMA to BDMA as I suggested
> in previous email.
Please what is the technical difference? I wonder why BDMA made it so
worse than DDMA. Thanks!
^ permalink raw reply
* Re: usb: gadget: dwc2: RK3308: Transmission to EP OUT stalls at larger packet size
From: Pavel Hofman @ 2024-02-05 16:12 UTC (permalink / raw)
To: Minas Harutyunyan; +Cc: linux-usb@vger.kernel.org
In-Reply-To: <fb6f2c71-0362-0f17-bd86-725f772710b4@synopsys.com>
Dne 05. 02. 24 v 16:40 Minas Harutyunyan napsal(a):
> Hi Pavel,
>
> On 2/5/24 18:35, Pavel Hofman wrote:
>>
>>>
>> It really looks like some DMA performance issue. Stream 980 bytes/ 250us
>> (bInterval=2) is bitperfect, no dropped packets. While 24 bytes/125us
>> (bInterval=1) gets stuck. IIUC the DMA is not capable of copying packets
>> every 125us. Please is there any chance to tweak the performance to
>> handle the 125us packets reliably?
>>
Minas, thanks a lot!
> Quick analysis.
> 1. In dmesg seen BNA (buffer not available) interrupt which mean driver
> not prepare DMA descriptor on time.
This is still listed, even with the req_num=8 and no dropouts:
[ 1851.090554] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 1851.108563] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 1851.158427] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 1851.166529] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 1851.188525] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
[ 1851.190529] dwc2 ff400000.usb: dwc2_hsotg_epint: BNA interrupt
Please is there any way to improve? Interesting that it produces no
dropouts.
> 2. For a try please disable debug printing from g_audio:
> [ 2080.107701] u_audio_iso_complete: iso_complete status(-61) 0/1000
That helped too, thanks.
> 3. Please try run device in Buffer DMA instead of Descriptor DMA:
> p->g_dma_desc = hw->dma_desc_enable;
> replace to:
> p->g_dma_desc = 0;
>
Actually this made the transfer worse. With req_num=8 and dma_desc
enabled transfer clean, with dma_desc disabled many dropouts, with
req_num=2 no transfer at all.
With regards,
Pavel.
^ permalink raw reply
* Re: usb: gadget: dwc2: RK3308: Transmission to EP OUT stalls at larger packet size
From: Minas Harutyunyan @ 2024-02-05 16:05 UTC (permalink / raw)
To: Pavel Hofman; +Cc: linux-usb@vger.kernel.org
In-Reply-To: <4c198e2b-72fe-f21c-77a0-7c011ace3c6d@ivitera.com>
Hi Pavel,
On 2/5/24 19:52, Pavel Hofman wrote:
>
>
> Dne 05. 02. 24 v 15:35 Pavel Hofman napsal(a):
>>
>>>
>> It really looks like some DMA performance issue. Stream 980 bytes/
>> 250us (bInterval=2) is bitperfect, no dropped packets. While 24
>> bytes/125us (bInterval=1) gets stuck. IIUC the DMA is not capable of
>> copying packets every 125us. Please is there any chance to tweak the
>> performance to handle the 125us packets reliably?
>>
>
> I tried increasing f_uac2 req_number/UAC2_DEF_REQ_NUM from 2 to 8 and
> streaming seems to run stable at 125us microframes now (in both
> directions simultaneously). Please is there any other gadget tweak which
> could potentially reduce the risk of dropped packets? Something like
> using plain DMA instead of desc DMA (no idea :-) )...
>
> Thanks a lot for your expert opinion.
Yes, it's mandatory to increase f_uac2_req_number, at least 4. Obviously
2 is not enough for descriptor list and main cause of BNA interrupt.
Another suggestion to change DMA mode from DDMA to BDMA as I suggested
in previous email.
Thanks,
Minas
^ permalink raw reply
* Re: usb: gadget: dwc2: RK3308: Transmission to EP OUT stalls at larger packet size
From: Pavel Hofman @ 2024-02-05 15:52 UTC (permalink / raw)
To: Minas Harutyunyan; +Cc: linux-usb@vger.kernel.org
In-Reply-To: <758d6e5d-d5b4-2bcc-bd51-fb7b49356532@ivitera.com>
Dne 05. 02. 24 v 15:35 Pavel Hofman napsal(a):
>
> Dne 05. 02. 24 v 14:43 Pavel Hofman napsal(a):
>> Hi Minas,
>>
>> I am having issues with dwc2 gadget on RK3308 (Rock Pi S). Kernel 6.6.2.
>>
>> When lightly loading EP OUT with UAC2, EP OUT works OK. When
>> increasing the packet size (960 bytes in the iso packet), the data
>> delivery stops completely. Method u_audio.c:u_audio_iso_complete stops
>> being called, it's like as if the incoming stream reception got stuck.
>>
>> Only the function f_uac2 is used on the gadget.
>>
>> Debug log from gadget.c + u_audio.c (logging all calls of
>> u_audio_iso_complete including !req->status) is attached. Starts of
>> playback on the host are marked with this logs in the dump:
>>
>> g_audio gadget.0: start capture with rate 192000
>>
>>
>>
>> root@rock-pi-s:/sys/kernel/debug/usb/ff400000.usb# cat hw_params
>> op_mode : 0
>> arch : 2
>> dma_desc_enable : 1
>> enable_dynamic_fifo : 1
>> en_multiple_tx_fifo : 1
>> rx_fifo_size : 1024
>> host_nperio_tx_fifo_size : 0
>> dev_nperio_tx_fifo_size : 16
>> host_perio_tx_fifo_size : 0
>> nperio_tx_q_depth : 4
>> host_perio_tx_q_depth : 4
>> dev_token_q_depth : 8
>> max_transfer_size : 524287
>> max_packet_count : 1023
>> host_channels : 9
>> hs_phy_type : 1
>> fs_phy_type : 0
>> i2c_enable : 0
>> num_dev_ep : 9
>> num_dev_perio_in_ep : 0
>> total_fifo_size : 972
>> power_optimized : 1
>> utmi_phy_data_width : 1
>> snpsid : 0x4f54310a
>> dev_ep_dirs : 0x6664
>>
>>
>> root@rock-pi-s:/sys/kernel/debug/usb/ff400000.usb# cat params
>> otg_caps.hnp_support : 0
>> otg_caps.srp_support : 0
>> otg_caps.otg_rev : 0
>> dma_desc_enable : 0
>> dma_desc_fs_enable : 0
>> speed : 0
>> enable_dynamic_fifo : 1
>> en_multiple_tx_fifo : 1
>> host_rx_fifo_size : 525
>> host_nperio_tx_fifo_size : 128
>> host_perio_tx_fifo_size : 256
>> max_transfer_size : 524287
>> max_packet_count : 1023
>> host_channels : 0
>> phy_type : 1
>> phy_utmi_width : 16
>> phy_ulpi_ddr : 0
>> phy_ulpi_ext_vbus : 0
>> i2c_enable : 0
>> ipg_isoc_en : 0
>> ulpi_fs_ls : 0
>> host_support_fs_ls_low_power : 0
>> host_ls_low_power_phy_clk : 0
>> activate_stm_fs_transceiver : 0
>> activate_stm_id_vb_detection : 0
>> ts_dline : 0
>> reload_ctl : 1
>> ahbcfg : 0xe
>> uframe_sched : 1
>> external_id_pin_ctl : 0
>> power_down : 0
>> lpm : 0
>> lpm_clock_gating : 0
>> besl : 0
>> hird_threshold_en : 0
>> hird_threshold : 4
>> service_interval : 0
>> host_dma : 0
>> g_dma : 1
>> g_dma_desc : 1
>> g_rx_fifo_size : 310
>> g_np_tx_fifo_size : 16
>> g_tx_fifo_size[0] : 0
>> g_tx_fifo_size[1] : 256
>> g_tx_fifo_size[2] : 128
>> g_tx_fifo_size[3] : 128
>> g_tx_fifo_size[4] : 64
>> g_tx_fifo_size[5] : 32
>> g_tx_fifo_size[6] : 16
>> g_tx_fifo_size[7] : 0
>> g_tx_fifo_size[8] : 0
>> g_tx_fifo_size[9] : 0
>> g_tx_fifo_size[10] : 0
>> g_tx_fifo_size[11] : 0
>> g_tx_fifo_size[12] : 0
>> g_tx_fifo_size[13] : 0
>> g_tx_fifo_size[14] : 0
>> g_tx_fifo_size[15] : 0
>>
>>
>> root@rock-pi-s:/sys/kernel/debug/usb/ff400000.usb# cat regdump
>> GOTGCTL = 0x000d0000
>> GOTGINT = 0x00000000
>> GAHBCFG = 0x0000002f
>> GUSBCFG = 0x4000140f
>> GRSTCTL = 0x40000000
>> GINTSTS = 0x0438c0ba
>> GINTMSK = 0xd88c3c44
>> GRXSTSR = 0x51643c02
>> GRXFSIZ = 0x00000136
>> GNPTXFSIZ = 0x00100136
>> GNPTXSTS = 0x00080010
>> GI2CCTL = 0x00000000
>> GPVNDCTL = 0x00000000
>> GGPIO = 0x00000000
>> GUID = 0x32000001
>> GSNPSID = 0x4f54310a
>> GHWCFG1 = 0x00006664
>> GHWCFG2 = 0x228e2450
>> GHWCFG3 = 0x03cc90e8
>> GHWCFG4 = 0xdbf04030
>> GLPMCFG = 0x00000000
>> GPWRDN = 0x00600010
>> GDFIFOCFG = 0x03b603cc
>> ADPCTL = 0x00000000
>> HPTXFSIZ = 0x00000000
>> DPTXFSIZN(1) = 0x01000146
>> DPTXFSIZN(2) = 0x00800246
>> DPTXFSIZN(3) = 0x008002c6
>> DPTXFSIZN(4) = 0x00400346
>> DPTXFSIZN(5) = 0x00200386
>> DPTXFSIZN(6) = 0x001003a6
>> DPTXFSIZN(7) = 0x00200386
>> DPTXFSIZN(8) = 0x001003a6
>> DPTXFSIZN(9) = 0x01000146
>> DPTXFSIZN(10) = 0x00800246
>> DPTXFSIZN(11) = 0x008002c6
>> DPTXFSIZN(12) = 0x00400346
>> DPTXFSIZN(13) = 0x00200386
>> DPTXFSIZN(14) = 0x001003a6
>> DPTXFSIZN(15) = 0x00200386
>> DCFG = 0x008402f0
>> DCTL = 0x00000000
>> DSTS = 0x001ef900
>> DIEPMSK = 0x0000220f
>> DOEPMSK = 0x0000023f
>> DAINT = 0x02140000
>> DAINTMSK = 0x0005000b
>> DTKNQR1 = 0x00000000
>> DTKNQR2 = 0x00000000
>> DTKNQR3 = 0x0c100020
>> DTKNQR4 = 0x00000000
>> DVBUSDIS = 0x00000b8f
>> DVBUSPULSE = 0x000002c6
>> DIEPCTL(0) = 0x00028000
>> DIEPCTL(1) = 0x018c8006
>> DIEPCTL(2) = 0x00000400
>> DIEPCTL(3) = 0x01448004
>> DIEPCTL(4) = 0x00000400
>> DIEPCTL(5) = 0x004603e8
>> DIEPCTL(6) = 0x00000400
>> DIEPCTL(7) = 0x00000400
>> DIEPCTL(8) = 0x00000400
>> DIEPCTL(9) = 0x00000400
>> DIEPCTL(10) = 0x00000400
>> DIEPCTL(11) = 0x00000400
>> DIEPCTL(12) = 0x00000400
>> DIEPCTL(13) = 0x00000400
>> DIEPCTL(14) = 0x00000400
>> DIEPCTL(15) = 0x00000400
>> DOEPCTL(0) = 0x80028000
>> DOEPCTL(1) = 0x00000400
>> DOEPCTL(2) = 0x800483e8
>> DOEPCTL(3) = 0x00000400
>> DOEPCTL(4) = 0x00000400
>> DOEPCTL(5) = 0x00000400
>> DOEPCTL(6) = 0x00000400
>> DOEPCTL(7) = 0x00000400
>> DOEPCTL(8) = 0x00000400
>> DOEPCTL(9) = 0x00000400
>> DOEPCTL(10) = 0x00000400
>> DOEPCTL(11) = 0x00000400
>> DOEPCTL(12) = 0x00000400
>> DOEPCTL(13) = 0x00000400
>> DOEPCTL(14) = 0x00000400
>> DOEPCTL(15) = 0x00000400
>> DIEPINT(0) = 0x00000090
>> DIEPINT(1) = 0x00000090
>> DIEPINT(2) = 0x000002a0
>> DIEPINT(3) = 0x00000090
>> DIEPINT(4) = 0x000002a0
>> DIEPINT(5) = 0x000000c0
>> DIEPINT(6) = 0x000002a0
>> DIEPINT(7) = 0x00000080
>> DIEPINT(8) = 0x00000080
>> DIEPINT(9) = 0x00000080
>> DIEPINT(10) = 0x000002a0
>> DIEPINT(11) = 0x000002a0
>> DIEPINT(12) = 0x000002a0
>> DIEPINT(13) = 0x000002a0
>> DIEPINT(14) = 0x000002a0
>> DIEPINT(15) = 0x000002a0
>> DOEPINT(0) = 0x00002000
>> DOEPINT(1) = 0x00000000
>> DOEPINT(2) = 0x00002010
>> DOEPINT(3) = 0x00000000
>> DOEPINT(4) = 0x00000020
>> DOEPINT(5) = 0x00000000
>> DOEPINT(6) = 0x00000000
>> DOEPINT(7) = 0x00000000
>> DOEPINT(8) = 0x00000000
>> DOEPINT(9) = 0x00000220
>> DOEPINT(10) = 0x00000000
>> DOEPINT(11) = 0x00000000
>> DOEPINT(12) = 0x00000000
>> DOEPINT(13) = 0x00000000
>> DOEPINT(14) = 0x00000000
>> DOEPINT(15) = 0x00000000
>> DIEPTSIZ(0) = 0x00080052
>> DIEPTSIZ(1) = 0x00000000
>> DIEPTSIZ(2) = 0x00000000
>> DIEPTSIZ(3) = 0x11d81ef7
>> DIEPTSIZ(4) = 0x00000000
>> DIEPTSIZ(5) = 0x00000000
>> DIEPTSIZ(6) = 0x00000000
>> DIEPTSIZ(7) = 0x00000000
>> DIEPTSIZ(8) = 0x00000000
>> DIEPTSIZ(9) = 0x00000000
>> DIEPTSIZ(10) = 0x00000000
>> DIEPTSIZ(11) = 0x00000000
>> DIEPTSIZ(12) = 0x00000000
>> DIEPTSIZ(13) = 0x00000000
>> DIEPTSIZ(14) = 0x00000000
>> DIEPTSIZ(15) = 0x00000000
>> DOEPTSIZ(0) = 0x2000005e
>> DOEPTSIZ(1) = 0x00000000
>> DOEPTSIZ(2) = 0x1f27b8c0
>> DOEPTSIZ(3) = 0x00000000
>> DOEPTSIZ(4) = 0x00000000
>> DOEPTSIZ(5) = 0x00000000
>> DOEPTSIZ(6) = 0x00000000
>> DOEPTSIZ(7) = 0x00000000
>> DOEPTSIZ(8) = 0x00000000
>> DOEPTSIZ(9) = 0x00000000
>> DOEPTSIZ(10) = 0x00000000
>> DOEPTSIZ(11) = 0x00000000
>> DOEPTSIZ(12) = 0x00000000
>> DOEPTSIZ(13) = 0x00000000
>> DOEPTSIZ(14) = 0x00000000
>> DOEPTSIZ(15) = 0x00000000
>> DIEPDMA(0) = 0x0e693000
>> DIEPDMA(1) = 0x86711010
>> DIEPDMA(2) = 0x92b8b5a4
>> DIEPDMA(3) = 0x1ba02008
>> DIEPDMA(4) = 0x92b8b5a4
>> DIEPDMA(5) = 0x2fae0710
>> DIEPDMA(6) = 0x92b8b5a4
>> DIEPDMA(7) = 0x43c7ba7f
>> DIEPDMA(8) = 0x0cc64000
>> DIEPDMA(9) = 0x57447266
>> DIEPDMA(10) = 0x92b8b5a4
>> DIEPDMA(11) = 0x92b8b5a4
>> DIEPDMA(12) = 0x92b8b5a4
>> DIEPDMA(13) = 0x92b8b5a4
>> DIEPDMA(14) = 0x92b8b5a4
>> DIEPDMA(15) = 0x92b8b5a4
>> DOEPDMA(0) = 0x1ba01000
>> DOEPDMA(1) = 0xa8800c26
>> DOEPDMA(2) = 0x1ba01008
>> DOEPDMA(3) = 0xa8800c26
>> DOEPDMA(4) = 0x6db46af8
>> DOEPDMA(5) = 0xa8800c26
>> DOEPDMA(6) = 0x1291dfd8
>> DOEPDMA(7) = 0xa8800c26
>> DOEPDMA(8) = 0x82d9ef90
>> DOEPDMA(9) = 0x0309d816
>> DOEPDMA(10) = 0xa8800c26
>> DOEPDMA(11) = 0xa8800c26
>> DOEPDMA(12) = 0xa8800c26
>> DOEPDMA(13) = 0xa8800c26
>> DOEPDMA(14) = 0xa8800c26
>> DOEPDMA(15) = 0xa8800c26
>> DTXFSTS(0) = 0x00000010
>> DTXFSTS(1) = 0x00000010
>> DTXFSTS(2) = 0x00000010
>> DTXFSTS(3) = 0x00000020
>> DTXFSTS(4) = 0x00000010
>> DTXFSTS(5) = 0x00000100
>> DTXFSTS(6) = 0x00000010
>> DTXFSTS(7) = 0x00000010
>> DTXFSTS(8) = 0x00000010
>> DTXFSTS(9) = 0x00000010
>> DTXFSTS(10) = 0x00000010
>> DTXFSTS(11) = 0x00000010hofman
>> DTXFSTS(12) = 0x00000010
>> DTXFSTS(13) = 0x00000010
>> DTXFSTS(14) = 0x00000010
>> DTXFSTS(15) = 0x00000010
>> PCGCTL = 0x00000000
>> HCFG = 0x008402f0
>> HFIR = 0x00000b8f
>> HFNUM = 0x0b1803df
>> HPTXSTS = 0x00080100
>> HAINT = 0x00000002
>> HAINTMSK = 0x00000007
>> HFLBADDR = 0x00000000
>> HPRT0 = 0x00000000
>> HCCHAR(0) = 0x018c8006
>> HCCHAR(1) = 0x00000400
>> HCCHAR(2) = 0x800483e8
>> HCCHAR(3) = 0x00000400
>> HCCHAR(4) = 0x00000400
>> HCCHAR(5) = 0x00000400
>> HCCHAR(6) = 0x00000400
>> HCCHAR(7) = 0x00000400
>> HCCHAR(8) = 0x00000400
>> HCCHAR(9) = 0x00000400
>> HCCHAR(10) = 0x00000400
>> HCCHAR(11) = 0x00000400
>> HCCHAR(12) = 0x00000400
>> HCCHAR(13) = 0x00000400
>> HCCHAR(14) = 0x00000400
>> HCCHAR(15) = 0x00000400
>> HCSPLT(0) = 0x00000000
>> HCSPLT(1) = 0x00000000
>> HCSPLT(2) = 0x00000000
>> HCSPLT(3) = 0x00000000
>> HCSPLT(4) = 0x00000000
>> HCSPLT(5) = 0x00000000
>> HCSPLT(6) = 0x00000000
>> HCSPLT(7) = 0x00000000
>> HCSPLT(8) = 0x00000000
>> HCSPLT(9) = 0x00000000
>> HCSPLT(10) = 0x00000000
>> HCSPLT(11) = 0x00000000
>> HCSPLT(12) = 0x00000000
>> HCSPLT(13) = 0x00000000
>> HCSPLT(14) = 0x00000000
>> HCSPLT(15) = 0x00000000
>> HCINT(0) = 0x00000010
>> HCINT(1) = 0x00000000
>> HCINT(2) = 0x00002010
>> HCINT(3) = 0x00000000
>> HCINT(4) = 0x00000020
>> HCINT(5) = 0x00000000
>> HCINT(6) = 0x00000000
>> HCINT(7) = 0x00000000
>> HCINT(8) = 0x00000000
>> HCINT(9) = 0x00000220
>> HCINT(10) = 0x00000000
>> HCINT(11) = 0x00000000
>> HCINT(12) = 0x00000000
>> HCINT(13) = 0x00000000
>> HCINT(14) = 0x00000000
>> HCINT(15) = 0x00000000
>> HCINTMSK(0) = 0x0000020f
>> HCINTMSK(1) = 0x00000000
>> HCINTMSK(2) = 0x00000000
>> HCINTMSK(3) = 0x00000000
>> HCINTMSK(4) = 0x00000000
>> HCINTMSK(5) = 0x00000000
>> HCINTMSK(6) = 0x00000000
>> HCINTMSK(7) = 0x00000000
>> HCINTMSK(8) = 0x00000000
>> HCINTMSK(9) = 0x00000000
>> HCINTMSK(10) = 0x00000000
>> HCINTMSK(11) = 0x00000000
>> HCINTMSK(12) = 0x00000000
>> HCINTMSK(13) = 0x00000000
>> HCINTMSK(14) = 0x00000000
>> HCINTMSK(15) = 0x00000000
>> HCTSIZ(0) = 0x00000000
>> HCTSIZ(1) = 0x00000000
>> HCTSIZ(2) = 0x1f27b8c0
>> HCTSIZ(3) = 0x00000000
>> HCTSIZ(4) = 0x00000000
>> HCTSIZ(5) = 0x00000000
>> HCTSIZ(6) = 0x00000000
>> HCTSIZ(7) = 0x00000000
>> HCTSIZ(8) = 0x00000000
>> HCTSIZ(9) = 0x00000000
>> HCTSIZ(10) = 0x00000000
>> HCTSIZ(11) = 0x00000000
>> HCTSIZ(12) = 0x00000000
>> HCTSIZ(13) = 0x00000000
>> HCTSIZ(14) = 0x00000000
>> HCTSIZ(15) = 0x00000000
>> HCDMA(0) = 0x1ba01000
>> HCDMA(1) = 0xa8800c26
>> HCDMA(2) = 0x1ba01008
>> HCDMA(3) = 0xa8800c26
>> HCDMA(4) = 0x6db46af8
>> HCDMA(5) = 0xa8800c26
>> HCDMA(6) = 0x1291dfd8
>> HCDMA(7) = 0xa8800c26
>> HCDMA(8) = 0x82d9ef90
>> HCDMA(9) = 0x0309d816
>> HCDMA(10) = 0xa8800c26
>> HCDMA(11) = 0xa8800c26
>> HCDMA(12) = 0xa8800c26
>> HCDMA(13) = 0xa8800c26
>> HCDMA(14) = 0xa8800c26
>> HCDMA(15) = 0xa8800c26
>> HCDMAB(0) = 0x069e9ff0
>> HCDMAB(1) = 0xa8800c26
>> HCDMAB(2) = 0x05eaf3c0
>> HCDMAB(3) = 0xa8800c26
>> HCDMAB(4) = 0x1f0d193f
>> HCDMAB(5) = 0xa8800c26
>> HCDMAB(6) = 0x7f72548c
>> HCDMAB(7) = 0xa8800c26
>> HCDMAB(8) = 0x7795e924
>> HCDMAB(9) = 0xc36c019a
>> HCDMAB(10) = 0xa8800c26
>> HCDMAB(11) = 0xa8800c26
>> HCDMAB(12) = 0xa8800c26
>> HCDMAB(13) = 0xa8800c26
>> HCDMAB(14) = 0xa8800c26
>> HCDMAB(15) = 0xa8800c26
>>
> It really looks like some DMA performance issue. Stream 980 bytes/ 250us
> (bInterval=2) is bitperfect, no dropped packets. While 24 bytes/125us
> (bInterval=1) gets stuck. IIUC the DMA is not capable of copying packets
> every 125us. Please is there any chance to tweak the performance to
> handle the 125us packets reliably?
>
I tried increasing f_uac2 req_number/UAC2_DEF_REQ_NUM from 2 to 8 and
streaming seems to run stable at 125us microframes now (in both
directions simultaneously). Please is there any other gadget tweak which
could potentially reduce the risk of dropped packets? Something like
using plain DMA instead of desc DMA (no idea :-) )...
Thanks a lot for your expert opinion.
^ permalink raw reply
* Re: usb: gadget: dwc2: RK3308: Transmission to EP OUT stalls at larger packet size
From: Minas Harutyunyan @ 2024-02-05 15:40 UTC (permalink / raw)
To: Pavel Hofman; +Cc: linux-usb@vger.kernel.org
In-Reply-To: <758d6e5d-d5b4-2bcc-bd51-fb7b49356532@ivitera.com>
Hi Pavel,
On 2/5/24 18:35, Pavel Hofman wrote:
>
>>
> It really looks like some DMA performance issue. Stream 980 bytes/ 250us
> (bInterval=2) is bitperfect, no dropped packets. While 24 bytes/125us
> (bInterval=1) gets stuck. IIUC the DMA is not capable of copying packets
> every 125us. Please is there any chance to tweak the performance to
> handle the 125us packets reliably?
>
> Thanks a lot!
Quick analysis.
1. In dmesg seen BNA (buffer not available) interrupt which mean driver
not prepare DMA descriptor on time.
2. For a try please disable debug printing from g_audio:
[ 2080.107701] u_audio_iso_complete: iso_complete status(-61) 0/1000
3. Please try run device in Buffer DMA instead of Descriptor DMA:
p->g_dma_desc = hw->dma_desc_enable;
replace to:
p->g_dma_desc = 0;
Thanks,
Minas
^ permalink raw reply
* Re: Linux warning `usb: port power management may be unreliable` on Dell XPS 13 9360
From: Mathias Nyman @ 2024-02-05 15:15 UTC (permalink / raw)
To: Paul Menzel, Greg Kroah-Hartman; +Cc: linux-usb, LKML, Hans de Goede
In-Reply-To: <b97d07bf-da27-4576-bed6-fd63e3e0b569@molgen.mpg.de>
On 4.2.2024 10.45, Paul Menzel wrote:
> Dear Linux folks,
>
>
> On the Dell XPS 13 9360, Linux warns:
>
> usb: port power management may be unreliable
Is this a new issue, regression?
Was the firmware recently updated?
>
> $ lsusb -t
> /: Bus 001.Port 001: Dev 001, Class=root_hub, Driver=xhci_hcd/12p, 480M
> |__ Port 003: Dev 002, If 0, Class=Wireless, Driver=[none], 12M
> |__ Port 003: Dev 002, If 1, Class=Wireless, Driver=[none], 12M
> |__ Port 004: Dev 003, If 0, Class=Human Interface Device, Driver=usbhid, 12M
> |__ Port 005: Dev 004, If 0, Class=Video, Driver=uvcvideo, 480M
> |__ Port 005: Dev 004, If 1, Class=Video, Driver=uvcvideo, 480M
> /: Bus 002.Port 001: Dev 001, Class=root_hub, Driver=xhci_hcd/6p, 5000M
>
> Enabling dynamic debug with `usbcore.dyndbg=+p` – `dyndbg="file port.c +p"` did not work¹ – the additional messages are:
>
> [ 1.149417] usb usb2-port1: peered to usb1-port1
> [ 1.150123] usb usb2-port2: peered to usb1-port2
> [ 1.150916] usb usb2-port3: peered to usb1-port6
> [ 1.151621] usb: failed to peer usb2-port4 and usb1-port6 by location (usb2-port4:none) (usb1-port6:usb2-port3)
> [ 1.151634] usb usb2-port4: failed to peer to usb1-port6 (-16)
> [ 1.151642] usb: port power management may be unreliable
> [ 1.152314] usb: failed to peer usb2-port5 and usb1-port6 by location (usb2-port5:none) (usb1-port6:usb2-port3)
> [ 1.152325] usb usb2-port5: failed to peer to usb1-port6 (-16)
> [ 1.153020] usb: failed to peer usb2-port6 and usb1-port6 by location (usb2-port6:none) (usb1-port6:usb2-port3)
> [ 1.153031] usb usb2-port6: failed to peer to usb1-port6 (-16)
> [ 1.153079] usb usb2: port-1 no _DSM function 5
> [ 1.153096] usb usb2: port-2 no _DSM function 5
> [ 1.153111] usb usb2: port-3 no _DSM function 5
> [ 1.153124] usb usb2: port-4 no _DSM function 5
> [ 1.153137] usb usb2: port-5 no _DSM function 5
> [ 1.153151] usb usb2: port-6 no _DSM function 5
> [ 1.166521] usb usb1-port3: status 0101 change 0001
> [ 1.166555] usb usb1-port4: status 0101 change 0001
> [ 1.166584] usb usb1-port5: status 0101 change 0001
> [ 1.270442] usb usb1-port3: status 0101, change 0000, 12 Mb/s
> [ 1.362751] usb usb2: bus auto-suspend, wakeup 1
These are all related to reading values from firmware ACPI tables.
The "failed to peer portx-porty.." messages are because driver can't match which
HS USB 2 and SS USB3 ports are in the same physical connector based on info
read from firmware ACPI _PLD entries
_DSM function 5 is related to port link power management.
Both cases mostly impact power management, but might affects something
else. Haven't looked at it in detail.
ACPI table dump could show more info, especially the DSDT table
>
> So the problematic ports do not show up in `lsusb`, do they?
>
> Please find the output of `dmesg` attached.
dmesg shows that "usb2" is the SuperSpeed USB 3 roothub.
It's suspended as no SuperSpeed devices are connected to it.
Do USB 3 devices work normally on this machine?
Thanks
Mathias
^ permalink raw reply
* [usb:usb-testing] BUILD SUCCESS ed5551279c9100aff6adf337d809057a7532b6f7
From: kernel test robot @ 2024-02-05 14:35 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-usb
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
branch HEAD: ed5551279c9100aff6adf337d809057a7532b6f7 Merge 6.8-rc3 into usb-next
elapsed time: 1447m
configs tested: 237
configs skipped: 4
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc
alpha allyesconfig gcc
alpha defconfig gcc
arc alldefconfig gcc
arc allmodconfig gcc
arc allnoconfig gcc
arc allyesconfig gcc
arc axs103_defconfig gcc
arc axs103_smp_defconfig gcc
arc defconfig gcc
arc hsdk_defconfig gcc
arc randconfig-001-20240205 gcc
arc randconfig-002-20240205 gcc
arc tb10x_defconfig gcc
arc vdk_hs38_defconfig gcc
arm allmodconfig gcc
arm allnoconfig clang
arm allyesconfig gcc
arm at91_dt_defconfig clang
arm defconfig clang
arm ep93xx_defconfig clang
arm imx_v6_v7_defconfig clang
arm mmp2_defconfig gcc
arm moxart_defconfig gcc
arm multi_v4t_defconfig clang
arm mv78xx0_defconfig clang
arm randconfig-001-20240205 clang
arm randconfig-002-20240205 gcc
arm randconfig-003-20240205 clang
arm randconfig-004-20240205 clang
arm s5pv210_defconfig gcc
arm spear6xx_defconfig clang
arm stm32_defconfig gcc
arm64 allmodconfig clang
arm64 allnoconfig gcc
arm64 defconfig gcc
arm64 randconfig-001-20240205 gcc
arm64 randconfig-002-20240205 gcc
arm64 randconfig-003-20240205 gcc
arm64 randconfig-004-20240205 gcc
csky allmodconfig gcc
csky allnoconfig gcc
csky allyesconfig gcc
csky defconfig gcc
csky randconfig-001-20240205 gcc
csky randconfig-002-20240205 gcc
hexagon allmodconfig clang
hexagon allnoconfig clang
hexagon allyesconfig clang
hexagon defconfig clang
hexagon randconfig-001-20240205 clang
hexagon randconfig-002-20240205 clang
i386 allmodconfig gcc
i386 allnoconfig gcc
i386 allyesconfig gcc
i386 buildonly-randconfig-001-20240205 clang
i386 buildonly-randconfig-002-20240205 clang
i386 buildonly-randconfig-003-20240205 clang
i386 buildonly-randconfig-004-20240205 clang
i386 buildonly-randconfig-005-20240205 clang
i386 buildonly-randconfig-006-20240205 clang
i386 defconfig clang
i386 randconfig-001-20240205 clang
i386 randconfig-002-20240205 clang
i386 randconfig-003-20240205 clang
i386 randconfig-004-20240205 gcc
i386 randconfig-005-20240205 clang
i386 randconfig-006-20240205 gcc
i386 randconfig-011-20240205 gcc
i386 randconfig-012-20240205 clang
i386 randconfig-013-20240205 gcc
i386 randconfig-014-20240205 gcc
i386 randconfig-015-20240205 clang
i386 randconfig-016-20240205 gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch allyesconfig gcc
loongarch defconfig gcc
loongarch randconfig-001-20240205 gcc
loongarch randconfig-002-20240205 gcc
m68k allmodconfig gcc
m68k allnoconfig gcc
m68k allyesconfig gcc
m68k amiga_defconfig gcc
m68k atari_defconfig gcc
m68k defconfig gcc
m68k m5249evb_defconfig gcc
microblaze allmodconfig gcc
microblaze allnoconfig gcc
microblaze allyesconfig gcc
microblaze defconfig gcc
mips allmodconfig gcc
mips allnoconfig gcc
mips allyesconfig gcc
mips cobalt_defconfig gcc
mips gcw0_defconfig clang
mips loongson3_defconfig gcc
mips rm200_defconfig gcc
nios2 3c120_defconfig gcc
nios2 allmodconfig gcc
nios2 allnoconfig gcc
nios2 allyesconfig gcc
nios2 defconfig gcc
nios2 randconfig-001-20240205 gcc
nios2 randconfig-002-20240205 gcc
openrisc alldefconfig gcc
openrisc allmodconfig gcc
openrisc allnoconfig gcc
openrisc allyesconfig gcc
openrisc defconfig gcc
parisc allmodconfig gcc
parisc allnoconfig gcc
parisc allyesconfig gcc
parisc defconfig gcc
parisc randconfig-001-20240205 gcc
parisc randconfig-002-20240205 gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc allyesconfig clang
powerpc fsp2_defconfig gcc
powerpc holly_defconfig clang
powerpc icon_defconfig gcc
powerpc iss476-smp_defconfig gcc
powerpc maple_defconfig clang
powerpc microwatt_defconfig gcc
powerpc ppa8548_defconfig gcc
powerpc randconfig-001-20240205 clang
powerpc randconfig-002-20240205 clang
powerpc randconfig-003-20240205 gcc
powerpc socrates_defconfig gcc
powerpc64 randconfig-001-20240205 clang
powerpc64 randconfig-002-20240205 clang
powerpc64 randconfig-003-20240205 clang
riscv allmodconfig clang
riscv allnoconfig gcc
riscv allyesconfig clang
riscv defconfig clang
riscv randconfig-001-20240205 clang
riscv randconfig-002-20240205 gcc
s390 allmodconfig clang
s390 allnoconfig clang
s390 allyesconfig gcc
s390 defconfig clang
s390 randconfig-001-20240205 clang
s390 randconfig-002-20240205 gcc
sh allmodconfig gcc
sh allnoconfig gcc
sh allyesconfig gcc
sh defconfig gcc
sh dreamcast_defconfig gcc
sh landisk_defconfig gcc
sh polaris_defconfig gcc
sh randconfig-001-20240205 gcc
sh randconfig-002-20240205 gcc
sh se7206_defconfig gcc
sh se7705_defconfig gcc
sh se7750_defconfig gcc
sh sh2007_defconfig gcc
sh sh7710voipgw_defconfig gcc
sh urquell_defconfig gcc
sparc allmodconfig gcc
sparc allnoconfig gcc
sparc allyesconfig gcc
sparc defconfig gcc
sparc64 allmodconfig gcc
sparc64 allyesconfig gcc
sparc64 defconfig gcc
sparc64 randconfig-001-20240205 gcc
sparc64 randconfig-002-20240205 gcc
um allmodconfig clang
um allnoconfig clang
um allyesconfig gcc
um defconfig clang
um randconfig-001-20240205 clang
um randconfig-002-20240205 gcc
um x86_64_defconfig clang
x86_64 allnoconfig clang
x86_64 allyesconfig clang
x86_64 buildonly-randconfig-001-20240204 clang
x86_64 buildonly-randconfig-001-20240205 clang
x86_64 buildonly-randconfig-002-20240204 clang
x86_64 buildonly-randconfig-002-20240205 clang
x86_64 buildonly-randconfig-003-20240204 gcc
x86_64 buildonly-randconfig-003-20240205 gcc
x86_64 buildonly-randconfig-004-20240204 gcc
x86_64 buildonly-randconfig-004-20240205 gcc
x86_64 buildonly-randconfig-005-20240204 clang
x86_64 buildonly-randconfig-005-20240205 gcc
x86_64 buildonly-randconfig-006-20240204 gcc
x86_64 buildonly-randconfig-006-20240205 gcc
x86_64 defconfig gcc
x86_64 randconfig-001-20240204 gcc
x86_64 randconfig-001-20240205 gcc
x86_64 randconfig-002-20240204 gcc
x86_64 randconfig-002-20240205 gcc
x86_64 randconfig-003-20240204 clang
x86_64 randconfig-003-20240205 clang
x86_64 randconfig-004-20240204 clang
x86_64 randconfig-004-20240205 gcc
x86_64 randconfig-005-20240204 gcc
x86_64 randconfig-005-20240205 gcc
x86_64 randconfig-006-20240204 gcc
x86_64 randconfig-006-20240205 gcc
x86_64 randconfig-011-20240204 gcc
x86_64 randconfig-011-20240205 gcc
x86_64 randconfig-012-20240204 gcc
x86_64 randconfig-012-20240205 gcc
x86_64 randconfig-013-20240204 gcc
x86_64 randconfig-013-20240205 clang
x86_64 randconfig-014-20240204 gcc
x86_64 randconfig-014-20240205 clang
x86_64 randconfig-015-20240204 clang
x86_64 randconfig-015-20240205 gcc
x86_64 randconfig-016-20240204 clang
x86_64 randconfig-016-20240205 clang
x86_64 randconfig-071-20240204 gcc
x86_64 randconfig-071-20240205 clang
x86_64 randconfig-072-20240204 gcc
x86_64 randconfig-072-20240205 gcc
x86_64 randconfig-073-20240204 gcc
x86_64 randconfig-073-20240205 clang
x86_64 randconfig-074-20240204 clang
x86_64 randconfig-074-20240205 clang
x86_64 randconfig-075-20240204 clang
x86_64 randconfig-075-20240205 gcc
x86_64 randconfig-076-20240204 gcc
x86_64 randconfig-076-20240205 clang
x86_64 rhel-8.3-bpf gcc
x86_64 rhel-8.3-func gcc
x86_64 rhel-8.3-rust clang
x86_64 rhel-8.3 gcc
xtensa allnoconfig gcc
xtensa allyesconfig gcc
xtensa cadence_csp_defconfig gcc
xtensa randconfig-001-20240205 gcc
xtensa randconfig-002-20240205 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: usb: gadget: dwc2: RK3308: Transmission to EP OUT stalls at larger packet size
From: Pavel Hofman @ 2024-02-05 14:35 UTC (permalink / raw)
To: Minas Harutyunyan; +Cc: linux-usb@vger.kernel.org
In-Reply-To: <91811ad2-991e-bd34-b3ec-2b92229bdd8b@ivitera.com>
Dne 05. 02. 24 v 14:43 Pavel Hofman napsal(a):
> Hi Minas,
>
> I am having issues with dwc2 gadget on RK3308 (Rock Pi S). Kernel 6.6.2.
>
> When lightly loading EP OUT with UAC2, EP OUT works OK. When increasing
> the packet size (960 bytes in the iso packet), the data delivery stops
> completely. Method u_audio.c:u_audio_iso_complete stops being called,
> it's like as if the incoming stream reception got stuck.
>
> Only the function f_uac2 is used on the gadget.
>
> Debug log from gadget.c + u_audio.c (logging all calls of
> u_audio_iso_complete including !req->status) is attached. Starts of
> playback on the host are marked with this logs in the dump:
>
> g_audio gadget.0: start capture with rate 192000
>
>
> Other debug files are below.
>
> The SoC is quite weak (A35, 1GHz), it may be that it's not capable of
> full HS iso bandwidth. But maybe it's just some suboptimal configuration
> of DMA/DWC2.
>
> Of course I am ready to provide any other debug info.
>
> Thank you very much for any help.
>
> With regards,
>
> Pavel.
>
>
> This is EPs overview from the host + packetsize:
>
> Playback:
> Status: Running
> Interface = 1
> Altset = 1
> Packet Size = 1000
> Momentary freq = 192000 Hz (0x18.0000)
> Feedback Format = 16.16
> Interface 1
> Altset 1
> Format: S32_LE
> Channels: 10
> Endpoint: 0x02 (2 OUT) (ASYNC)
> Rates: 192000
> Data packet interval: 125 us
> Bits: 32
> Channel map: FL FR FC LFE RL RR FLC FRC RC SL
> Sync Endpoint: 0x83 (3 IN)
> Sync EP Interface: 1
> Sync EP Altset: 1
> Implicit Feedback Mode: No
>
> Capture:
> Status: Stop
> Interface 2
> Altset 1
> Format: S32_LE
> Channels: 10
> Endpoint: 0x85 (5 IN) (ASYNC)
> Rates: 192000
> Data packet interval: 125 us
> Bits: 32
> Channel map: FL FR FC LFE RL RR FLC FRC RC SL
>
>
>
>
> root@rock-pi-s:/sys/kernel/debug/usb/ff400000.usb# cat ep2out
> Endpoint index 2, named ep2out, dir out:
> DIEPCTL=0x00000400, DOEPCTL=0x800483e8
> DIEPDMA=0x92b8b5a4, DOEPDMA=0x1ba01008
> DIEPINT=0x000002a0, DOEPINT=0x00002010
> DIEPTSIZ=0x00000000, DOEPTSIZ=0x1f27b8c0
>
> mps 1000
> total_data=0
> request list (00000000f7fd142e,00000000900d6fbc):
> req 0000000074dc3bc0: 1000 bytes @0000000074c79a73, 0 done, res -115
> req 00000000980b129b: 1000 bytes @000000006ef8394b, 0 done, res -115
>
>
> RXFIFO is large enough for one full HS iso packet (we calculated it in
> the past to be around 304 bytes minimum). FIFOs fit the 972 bytes of
> total_fifo_size:
>
> root@rock-pi-s:/sys/kernel/debug/usb/ff400000.usb# cat fifo
> Non-periodic FIFOs:
> RXFIFO: Size 310
> NPTXFIFO: Size 16, Start 0x00000136
>
> Periodic TXFIFOs:
> DPTXFIFO 1: Size 256, Start 0x00000146
> DPTXFIFO 2: Size 128, Start 0x00000246
> DPTXFIFO 3: Size 128, Start 0x000002c6
> DPTXFIFO 4: Size 64, Start 0x00000346
> DPTXFIFO 5: Size 32, Start 0x00000386
> DPTXFIFO 6: Size 16, Start 0x000003a6
>
>
> root@rock-pi-s:/sys/kernel/debug/usb/ff400000.usb# cat hw_params
> op_mode : 0
> arch : 2
> dma_desc_enable : 1
> enable_dynamic_fifo : 1
> en_multiple_tx_fifo : 1
> rx_fifo_size : 1024
> host_nperio_tx_fifo_size : 0
> dev_nperio_tx_fifo_size : 16
> host_perio_tx_fifo_size : 0
> nperio_tx_q_depth : 4
> host_perio_tx_q_depth : 4
> dev_token_q_depth : 8
> max_transfer_size : 524287
> max_packet_count : 1023
> host_channels : 9
> hs_phy_type : 1
> fs_phy_type : 0
> i2c_enable : 0
> num_dev_ep : 9
> num_dev_perio_in_ep : 0
> total_fifo_size : 972
> power_optimized : 1
> utmi_phy_data_width : 1
> snpsid : 0x4f54310a
> dev_ep_dirs : 0x6664
>
>
> root@rock-pi-s:/sys/kernel/debug/usb/ff400000.usb# cat params
> otg_caps.hnp_support : 0
> otg_caps.srp_support : 0
> otg_caps.otg_rev : 0
> dma_desc_enable : 0
> dma_desc_fs_enable : 0
> speed : 0
> enable_dynamic_fifo : 1
> en_multiple_tx_fifo : 1
> host_rx_fifo_size : 525
> host_nperio_tx_fifo_size : 128
> host_perio_tx_fifo_size : 256
> max_transfer_size : 524287
> max_packet_count : 1023
> host_channels : 0
> phy_type : 1
> phy_utmi_width : 16
> phy_ulpi_ddr : 0
> phy_ulpi_ext_vbus : 0
> i2c_enable : 0
> ipg_isoc_en : 0
> ulpi_fs_ls : 0
> host_support_fs_ls_low_power : 0
> host_ls_low_power_phy_clk : 0
> activate_stm_fs_transceiver : 0
> activate_stm_id_vb_detection : 0
> ts_dline : 0
> reload_ctl : 1
> ahbcfg : 0xe
> uframe_sched : 1
> external_id_pin_ctl : 0
> power_down : 0
> lpm : 0
> lpm_clock_gating : 0
> besl : 0
> hird_threshold_en : 0
> hird_threshold : 4
> service_interval : 0
> host_dma : 0
> g_dma : 1
> g_dma_desc : 1
> g_rx_fifo_size : 310
> g_np_tx_fifo_size : 16
> g_tx_fifo_size[0] : 0
> g_tx_fifo_size[1] : 256
> g_tx_fifo_size[2] : 128
> g_tx_fifo_size[3] : 128
> g_tx_fifo_size[4] : 64
> g_tx_fifo_size[5] : 32
> g_tx_fifo_size[6] : 16
> g_tx_fifo_size[7] : 0
> g_tx_fifo_size[8] : 0
> g_tx_fifo_size[9] : 0
> g_tx_fifo_size[10] : 0
> g_tx_fifo_size[11] : 0
> g_tx_fifo_size[12] : 0
> g_tx_fifo_size[13] : 0
> g_tx_fifo_size[14] : 0
> g_tx_fifo_size[15] : 0
>
>
> root@rock-pi-s:/sys/kernel/debug/usb/ff400000.usb# cat regdump
> GOTGCTL = 0x000d0000
> GOTGINT = 0x00000000
> GAHBCFG = 0x0000002f
> GUSBCFG = 0x4000140f
> GRSTCTL = 0x40000000
> GINTSTS = 0x0438c0ba
> GINTMSK = 0xd88c3c44
> GRXSTSR = 0x51643c02
> GRXFSIZ = 0x00000136
> GNPTXFSIZ = 0x00100136
> GNPTXSTS = 0x00080010
> GI2CCTL = 0x00000000
> GPVNDCTL = 0x00000000
> GGPIO = 0x00000000
> GUID = 0x32000001
> GSNPSID = 0x4f54310a
> GHWCFG1 = 0x00006664
> GHWCFG2 = 0x228e2450
> GHWCFG3 = 0x03cc90e8
> GHWCFG4 = 0xdbf04030
> GLPMCFG = 0x00000000
> GPWRDN = 0x00600010
> GDFIFOCFG = 0x03b603cc
> ADPCTL = 0x00000000
> HPTXFSIZ = 0x00000000
> DPTXFSIZN(1) = 0x01000146
> DPTXFSIZN(2) = 0x00800246
> DPTXFSIZN(3) = 0x008002c6
> DPTXFSIZN(4) = 0x00400346
> DPTXFSIZN(5) = 0x00200386
> DPTXFSIZN(6) = 0x001003a6
> DPTXFSIZN(7) = 0x00200386
> DPTXFSIZN(8) = 0x001003a6
> DPTXFSIZN(9) = 0x01000146
> DPTXFSIZN(10) = 0x00800246
> DPTXFSIZN(11) = 0x008002c6
> DPTXFSIZN(12) = 0x00400346
> DPTXFSIZN(13) = 0x00200386
> DPTXFSIZN(14) = 0x001003a6
> DPTXFSIZN(15) = 0x00200386
> DCFG = 0x008402f0
> DCTL = 0x00000000
> DSTS = 0x001ef900
> DIEPMSK = 0x0000220f
> DOEPMSK = 0x0000023f
> DAINT = 0x02140000
> DAINTMSK = 0x0005000b
> DTKNQR1 = 0x00000000
> DTKNQR2 = 0x00000000
> DTKNQR3 = 0x0c100020
> DTKNQR4 = 0x00000000
> DVBUSDIS = 0x00000b8f
> DVBUSPULSE = 0x000002c6
> DIEPCTL(0) = 0x00028000
> DIEPCTL(1) = 0x018c8006
> DIEPCTL(2) = 0x00000400
> DIEPCTL(3) = 0x01448004
> DIEPCTL(4) = 0x00000400
> DIEPCTL(5) = 0x004603e8
> DIEPCTL(6) = 0x00000400
> DIEPCTL(7) = 0x00000400
> DIEPCTL(8) = 0x00000400
> DIEPCTL(9) = 0x00000400
> DIEPCTL(10) = 0x00000400
> DIEPCTL(11) = 0x00000400
> DIEPCTL(12) = 0x00000400
> DIEPCTL(13) = 0x00000400
> DIEPCTL(14) = 0x00000400
> DIEPCTL(15) = 0x00000400
> DOEPCTL(0) = 0x80028000
> DOEPCTL(1) = 0x00000400
> DOEPCTL(2) = 0x800483e8
> DOEPCTL(3) = 0x00000400
> DOEPCTL(4) = 0x00000400
> DOEPCTL(5) = 0x00000400
> DOEPCTL(6) = 0x00000400
> DOEPCTL(7) = 0x00000400
> DOEPCTL(8) = 0x00000400
> DOEPCTL(9) = 0x00000400
> DOEPCTL(10) = 0x00000400
> DOEPCTL(11) = 0x00000400
> DOEPCTL(12) = 0x00000400
> DOEPCTL(13) = 0x00000400
> DOEPCTL(14) = 0x00000400
> DOEPCTL(15) = 0x00000400
> DIEPINT(0) = 0x00000090
> DIEPINT(1) = 0x00000090
> DIEPINT(2) = 0x000002a0
> DIEPINT(3) = 0x00000090
> DIEPINT(4) = 0x000002a0
> DIEPINT(5) = 0x000000c0
> DIEPINT(6) = 0x000002a0
> DIEPINT(7) = 0x00000080
> DIEPINT(8) = 0x00000080
> DIEPINT(9) = 0x00000080
> DIEPINT(10) = 0x000002a0
> DIEPINT(11) = 0x000002a0
> DIEPINT(12) = 0x000002a0
> DIEPINT(13) = 0x000002a0
> DIEPINT(14) = 0x000002a0
> DIEPINT(15) = 0x000002a0
> DOEPINT(0) = 0x00002000
> DOEPINT(1) = 0x00000000
> DOEPINT(2) = 0x00002010
> DOEPINT(3) = 0x00000000
> DOEPINT(4) = 0x00000020
> DOEPINT(5) = 0x00000000
> DOEPINT(6) = 0x00000000
> DOEPINT(7) = 0x00000000
> DOEPINT(8) = 0x00000000
> DOEPINT(9) = 0x00000220
> DOEPINT(10) = 0x00000000
> DOEPINT(11) = 0x00000000
> DOEPINT(12) = 0x00000000
> DOEPINT(13) = 0x00000000
> DOEPINT(14) = 0x00000000
> DOEPINT(15) = 0x00000000
> DIEPTSIZ(0) = 0x00080052
> DIEPTSIZ(1) = 0x00000000
> DIEPTSIZ(2) = 0x00000000
> DIEPTSIZ(3) = 0x11d81ef7
> DIEPTSIZ(4) = 0x00000000
> DIEPTSIZ(5) = 0x00000000
> DIEPTSIZ(6) = 0x00000000
> DIEPTSIZ(7) = 0x00000000
> DIEPTSIZ(8) = 0x00000000
> DIEPTSIZ(9) = 0x00000000
> DIEPTSIZ(10) = 0x00000000
> DIEPTSIZ(11) = 0x00000000
> DIEPTSIZ(12) = 0x00000000
> DIEPTSIZ(13) = 0x00000000
> DIEPTSIZ(14) = 0x00000000
> DIEPTSIZ(15) = 0x00000000
> DOEPTSIZ(0) = 0x2000005e
> DOEPTSIZ(1) = 0x00000000
> DOEPTSIZ(2) = 0x1f27b8c0
> DOEPTSIZ(3) = 0x00000000
> DOEPTSIZ(4) = 0x00000000
> DOEPTSIZ(5) = 0x00000000
> DOEPTSIZ(6) = 0x00000000
> DOEPTSIZ(7) = 0x00000000
> DOEPTSIZ(8) = 0x00000000
> DOEPTSIZ(9) = 0x00000000
> DOEPTSIZ(10) = 0x00000000
> DOEPTSIZ(11) = 0x00000000
> DOEPTSIZ(12) = 0x00000000
> DOEPTSIZ(13) = 0x00000000
> DOEPTSIZ(14) = 0x00000000
> DOEPTSIZ(15) = 0x00000000
> DIEPDMA(0) = 0x0e693000
> DIEPDMA(1) = 0x86711010
> DIEPDMA(2) = 0x92b8b5a4
> DIEPDMA(3) = 0x1ba02008
> DIEPDMA(4) = 0x92b8b5a4
> DIEPDMA(5) = 0x2fae0710
> DIEPDMA(6) = 0x92b8b5a4
> DIEPDMA(7) = 0x43c7ba7f
> DIEPDMA(8) = 0x0cc64000
> DIEPDMA(9) = 0x57447266
> DIEPDMA(10) = 0x92b8b5a4
> DIEPDMA(11) = 0x92b8b5a4
> DIEPDMA(12) = 0x92b8b5a4
> DIEPDMA(13) = 0x92b8b5a4
> DIEPDMA(14) = 0x92b8b5a4
> DIEPDMA(15) = 0x92b8b5a4
> DOEPDMA(0) = 0x1ba01000
> DOEPDMA(1) = 0xa8800c26
> DOEPDMA(2) = 0x1ba01008
> DOEPDMA(3) = 0xa8800c26
> DOEPDMA(4) = 0x6db46af8
> DOEPDMA(5) = 0xa8800c26
> DOEPDMA(6) = 0x1291dfd8
> DOEPDMA(7) = 0xa8800c26
> DOEPDMA(8) = 0x82d9ef90
> DOEPDMA(9) = 0x0309d816
> DOEPDMA(10) = 0xa8800c26
> DOEPDMA(11) = 0xa8800c26
> DOEPDMA(12) = 0xa8800c26
> DOEPDMA(13) = 0xa8800c26
> DOEPDMA(14) = 0xa8800c26
> DOEPDMA(15) = 0xa8800c26
> DTXFSTS(0) = 0x00000010
> DTXFSTS(1) = 0x00000010
> DTXFSTS(2) = 0x00000010
> DTXFSTS(3) = 0x00000020
> DTXFSTS(4) = 0x00000010
> DTXFSTS(5) = 0x00000100
> DTXFSTS(6) = 0x00000010
> DTXFSTS(7) = 0x00000010
> DTXFSTS(8) = 0x00000010
> DTXFSTS(9) = 0x00000010
> DTXFSTS(10) = 0x00000010
> DTXFSTS(11) = 0x00000010hofman
> DTXFSTS(12) = 0x00000010
> DTXFSTS(13) = 0x00000010
> DTXFSTS(14) = 0x00000010
> DTXFSTS(15) = 0x00000010
> PCGCTL = 0x00000000
> HCFG = 0x008402f0
> HFIR = 0x00000b8f
> HFNUM = 0x0b1803df
> HPTXSTS = 0x00080100
> HAINT = 0x00000002
> HAINTMSK = 0x00000007
> HFLBADDR = 0x00000000
> HPRT0 = 0x00000000
> HCCHAR(0) = 0x018c8006
> HCCHAR(1) = 0x00000400
> HCCHAR(2) = 0x800483e8
> HCCHAR(3) = 0x00000400
> HCCHAR(4) = 0x00000400
> HCCHAR(5) = 0x00000400
> HCCHAR(6) = 0x00000400
> HCCHAR(7) = 0x00000400
> HCCHAR(8) = 0x00000400
> HCCHAR(9) = 0x00000400
> HCCHAR(10) = 0x00000400
> HCCHAR(11) = 0x00000400
> HCCHAR(12) = 0x00000400
> HCCHAR(13) = 0x00000400
> HCCHAR(14) = 0x00000400
> HCCHAR(15) = 0x00000400
> HCSPLT(0) = 0x00000000
> HCSPLT(1) = 0x00000000
> HCSPLT(2) = 0x00000000
> HCSPLT(3) = 0x00000000
> HCSPLT(4) = 0x00000000
> HCSPLT(5) = 0x00000000
> HCSPLT(6) = 0x00000000
> HCSPLT(7) = 0x00000000
> HCSPLT(8) = 0x00000000
> HCSPLT(9) = 0x00000000
> HCSPLT(10) = 0x00000000
> HCSPLT(11) = 0x00000000
> HCSPLT(12) = 0x00000000
> HCSPLT(13) = 0x00000000
> HCSPLT(14) = 0x00000000
> HCSPLT(15) = 0x00000000
> HCINT(0) = 0x00000010
> HCINT(1) = 0x00000000
> HCINT(2) = 0x00002010
> HCINT(3) = 0x00000000
> HCINT(4) = 0x00000020
> HCINT(5) = 0x00000000
> HCINT(6) = 0x00000000
> HCINT(7) = 0x00000000
> HCINT(8) = 0x00000000
> HCINT(9) = 0x00000220
> HCINT(10) = 0x00000000
> HCINT(11) = 0x00000000
> HCINT(12) = 0x00000000
> HCINT(13) = 0x00000000
> HCINT(14) = 0x00000000
> HCINT(15) = 0x00000000
> HCINTMSK(0) = 0x0000020f
> HCINTMSK(1) = 0x00000000
> HCINTMSK(2) = 0x00000000
> HCINTMSK(3) = 0x00000000
> HCINTMSK(4) = 0x00000000
> HCINTMSK(5) = 0x00000000
> HCINTMSK(6) = 0x00000000
> HCINTMSK(7) = 0x00000000
> HCINTMSK(8) = 0x00000000
> HCINTMSK(9) = 0x00000000
> HCINTMSK(10) = 0x00000000
> HCINTMSK(11) = 0x00000000
> HCINTMSK(12) = 0x00000000
> HCINTMSK(13) = 0x00000000
> HCINTMSK(14) = 0x00000000
> HCINTMSK(15) = 0x00000000
> HCTSIZ(0) = 0x00000000
> HCTSIZ(1) = 0x00000000
> HCTSIZ(2) = 0x1f27b8c0
> HCTSIZ(3) = 0x00000000
> HCTSIZ(4) = 0x00000000
> HCTSIZ(5) = 0x00000000
> HCTSIZ(6) = 0x00000000
> HCTSIZ(7) = 0x00000000
> HCTSIZ(8) = 0x00000000
> HCTSIZ(9) = 0x00000000
> HCTSIZ(10) = 0x00000000
> HCTSIZ(11) = 0x00000000
> HCTSIZ(12) = 0x00000000
> HCTSIZ(13) = 0x00000000
> HCTSIZ(14) = 0x00000000
> HCTSIZ(15) = 0x00000000
> HCDMA(0) = 0x1ba01000
> HCDMA(1) = 0xa8800c26
> HCDMA(2) = 0x1ba01008
> HCDMA(3) = 0xa8800c26
> HCDMA(4) = 0x6db46af8
> HCDMA(5) = 0xa8800c26
> HCDMA(6) = 0x1291dfd8
> HCDMA(7) = 0xa8800c26
> HCDMA(8) = 0x82d9ef90
> HCDMA(9) = 0x0309d816
> HCDMA(10) = 0xa8800c26
> HCDMA(11) = 0xa8800c26
> HCDMA(12) = 0xa8800c26
> HCDMA(13) = 0xa8800c26
> HCDMA(14) = 0xa8800c26
> HCDMA(15) = 0xa8800c26
> HCDMAB(0) = 0x069e9ff0
> HCDMAB(1) = 0xa8800c26
> HCDMAB(2) = 0x05eaf3c0
> HCDMAB(3) = 0xa8800c26
> HCDMAB(4) = 0x1f0d193f
> HCDMAB(5) = 0xa8800c26
> HCDMAB(6) = 0x7f72548c
> HCDMAB(7) = 0xa8800c26
> HCDMAB(8) = 0x7795e924
> HCDMAB(9) = 0xc36c019a
> HCDMAB(10) = 0xa8800c26
> HCDMAB(11) = 0xa8800c26
> HCDMAB(12) = 0xa8800c26
> HCDMAB(13) = 0xa8800c26
> HCDMAB(14) = 0xa8800c26
> HCDMAB(15) = 0xa8800c26
>
It really looks like some DMA performance issue. Stream 980 bytes/ 250us
(bInterval=2) is bitperfect, no dropped packets. While 24 bytes/125us
(bInterval=1) gets stuck. IIUC the DMA is not capable of copying packets
every 125us. Please is there any chance to tweak the performance to
handle the 125us packets reliably?
Thanks a lot!
^ permalink raw reply
* [PATCH v2 5/5] usb: dwc3-am62: add workaround for Errata i2409
From: Roger Quadros @ 2024-02-05 14:12 UTC (permalink / raw)
To: Thinh.Nguyen
Cc: gregkh, r-gunasekaran, b-liu, afd, nm, srk, linux-usb,
linux-kernel, Roger Quadros, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
In-Reply-To: <20240205141221.56076-1-rogerq@kernel.org>
All AM62 devices have Errata i2409 [1] due to which
USB2 PHY may lock up due to short suspend.
Workaround involves setting bit 5 and 4 PLL_REG12
in PHY2 register space after USB controller is brought
out of LPSC reset but before controller initialization.
Handle this workaround.
[1] - https://www.ti.com/lit/er/sprz487d/sprz487d.pdf
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Conor Dooley <conor+dt@kernel.org>
Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
Notes:
Changelog:
v2:
- don't add phy read/write helpers or add phy to private data
v1: https://lore.kernel.org/all/20240201121220.5523-5-rogerq@kernel.org/
drivers/usb/dwc3/dwc3-am62.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c
index af1ce934e7fb..5ae5c3087b0f 100644
--- a/drivers/usb/dwc3/dwc3-am62.c
+++ b/drivers/usb/dwc3/dwc3-am62.c
@@ -101,6 +101,11 @@
#define PHY_CORE_VOLTAGE_MASK BIT(31)
#define PHY_PLL_REFCLK_MASK GENMASK(3, 0)
+/* USB PHY2 register offsets */
+#define USB_PHY_PLL_REG12 0x130
+#define USB_PHY_PLL_LDO_REF_EN BIT(5)
+#define USB_PHY_PLL_LDO_REF_EN_EN BIT(4)
+
#define DWC3_AM62_AUTOSUSPEND_DELAY 100
struct dwc3_am62 {
@@ -184,8 +189,9 @@ static int dwc3_ti_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct device_node *node = pdev->dev.of_node;
struct dwc3_am62 *am62;
- int i, ret;
unsigned long rate;
+ void __iomem *phy;
+ int i, ret;
u32 reg;
am62 = devm_kzalloc(dev, sizeof(*am62), GFP_KERNEL);
@@ -201,6 +207,12 @@ static int dwc3_ti_probe(struct platform_device *pdev)
return PTR_ERR(am62->usbss);
}
+ phy = devm_platform_ioremap_resource(pdev, 1);
+ if (IS_ERR(phy)) {
+ dev_err(dev, "can't map PHY IOMEM resource. Won't apply i2409 fix.\n");
+ phy = NULL;
+ }
+
am62->usb2_refclk = devm_clk_get(dev, "ref");
if (IS_ERR(am62->usb2_refclk)) {
dev_err(dev, "can't get usb2_refclk\n");
@@ -227,6 +239,13 @@ static int dwc3_ti_probe(struct platform_device *pdev)
if (ret)
return ret;
+ /* Workaround Errata i2409 */
+ if (phy) {
+ reg = readl(phy + USB_PHY_PLL_REG12);
+ reg |= USB_PHY_PLL_LDO_REF_EN | USB_PHY_PLL_LDO_REF_EN_EN;
+ writel(reg, phy + USB_PHY_PLL_REG12);
+ }
+
/* VBUS divider select */
am62->vbus_divider = device_property_read_bool(dev, "ti,vbus-divider");
reg = dwc3_ti_readl(am62, USBSS_PHY_CONFIG);
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox