Intel Merrifield SoC uses these endpoints for tracing and they shouldn't
be used for normal transfers, we need to skip them.

• 1 High BW Bulk IN (IN#1) (RTIT)
• 1 1KB BW Bulk IN (IN#8) + 1 1KB BW Bulk OUT (Run Control) (OUT#8)

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/usb/dwc3/dwc3-pci.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 052852f80146..9b73dfd34823 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -148,11 +148,20 @@ static const struct property_entry dwc3_pci_intel_byt_properties[] = {
 	{}
 };
 
+/*
+ * Intel Merrifield uses these endpoints for tracing and they shouldn't be used
+ * for normal transfers, we need to skip them.
+ * • 1 High BW Bulk IN (IN#1) (RTIT)
+ * • 1 1KB BW Bulk IN (IN#8) + 1 1KB BW Bulk OUT (Run Control) (OUT#8)
+ */
+static const u8 dwc3_pci_mrfld_reserved_endpoints[] = { 3, 16, 17 };
+
 static const struct property_entry dwc3_pci_mrfld_properties[] = {
 	PROPERTY_ENTRY_STRING("dr_mode", "otg"),
 	PROPERTY_ENTRY_STRING("linux,extcon-name", "mrfld_bcove_pwrsrc"),
 	PROPERTY_ENTRY_BOOL("snps,dis_u3_susphy_quirk"),
 	PROPERTY_ENTRY_BOOL("snps,dis_u2_susphy_quirk"),
+	PROPERTY_ENTRY_U8_ARRAY("snps,reserved-endpoints", dwc3_pci_mrfld_reserved_endpoints),
 	PROPERTY_ENTRY_BOOL("snps,usb2-gadget-lpm-disable"),
 	PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
 	{}
-- 
2.43.0.rc1.1336.g36b5255a03ac

On Thu, Jan 16, 2025, Andy Shevchenko wrote:
> Intel Merrifield SoC uses these endpoints for tracing and they shouldn't
> be used for normal transfers, we need to skip them.
> 
> • 1 High BW Bulk IN (IN#1) (RTIT)
> • 1 1KB BW Bulk IN (IN#8) + 1 1KB BW Bulk OUT (Run Control) (OUT#8)
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/usb/dwc3/dwc3-pci.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
> index 052852f80146..9b73dfd34823 100644
> --- a/drivers/usb/dwc3/dwc3-pci.c
> +++ b/drivers/usb/dwc3/dwc3-pci.c
> @@ -148,11 +148,20 @@ static const struct property_entry dwc3_pci_intel_byt_properties[] = {
>  	{}
>  };
>  
> +/*
> + * Intel Merrifield uses these endpoints for tracing and they shouldn't be used
> + * for normal transfers, we need to skip them.
> + * • 1 High BW Bulk IN (IN#1) (RTIT)
> + * • 1 1KB BW Bulk IN (IN#8) + 1 1KB BW Bulk OUT (Run Control) (OUT#8)

Please use regular bullet character and list the endpoint per line.

BR,
Thinh

> + */
> +static const u8 dwc3_pci_mrfld_reserved_endpoints[] = { 3, 16, 17 };
> +
>  static const struct property_entry dwc3_pci_mrfld_properties[] = {
>  	PROPERTY_ENTRY_STRING("dr_mode", "otg"),
>  	PROPERTY_ENTRY_STRING("linux,extcon-name", "mrfld_bcove_pwrsrc"),
>  	PROPERTY_ENTRY_BOOL("snps,dis_u3_susphy_quirk"),
>  	PROPERTY_ENTRY_BOOL("snps,dis_u2_susphy_quirk"),
> +	PROPERTY_ENTRY_U8_ARRAY("snps,reserved-endpoints", dwc3_pci_mrfld_reserved_endpoints),
>  	PROPERTY_ENTRY_BOOL("snps,usb2-gadget-lpm-disable"),
>  	PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
>  	{}
> -- 
> 2.43.0.rc1.1336.g36b5255a03ac
> 

On Thu, Jan 16, 2025 at 11:39:42PM +0000, Thinh Nguyen wrote:
> On Thu, Jan 16, 2025, Andy Shevchenko wrote:

...

> > + * Intel Merrifield uses these endpoints for tracing and they shouldn't be used
> > + * for normal transfers, we need to skip them.
> > + * • 1 High BW Bulk IN (IN#1) (RTIT)
> > + * • 1 1KB BW Bulk IN (IN#8) + 1 1KB BW Bulk OUT (Run Control) (OUT#8)
> 
> Please use regular bullet character and list the endpoint per line.

Which is...?

To my curiosity, what's wrong with the above?

-- 
With Best Regards,
Andy Shevchenko

On Fri, Jan 17, 2025, Andy Shevchenko wrote:
> On Thu, Jan 16, 2025 at 11:39:42PM +0000, Thinh Nguyen wrote:
> > On Thu, Jan 16, 2025, Andy Shevchenko wrote:
> 
> ...
> 
> > > + * Intel Merrifield uses these endpoints for tracing and they shouldn't be used
> > > + * for normal transfers, we need to skip them.
> > > + * • 1 High BW Bulk IN (IN#1) (RTIT)
> > > + * • 1 1KB BW Bulk IN (IN#8) + 1 1KB BW Bulk OUT (Run Control) (OUT#8)
> > 
> > Please use regular bullet character and list the endpoint per line.
> 
> Which is...?
> 
> To my curiosity, what's wrong with the above?
> 

Please use a character that we can find on the keyboard (- or * for
example).

And why would you want to list them like this:

	* Endpoint A
	* Endpoint B + Endpoint C

As oppose to:

	* Endpoint A
	* Endpoint B
	* Endpoint C

BR,
Thinh

On Tue, Jan 21, 2025 at 11:46:17PM +0000, Thinh Nguyen wrote:
> On Fri, Jan 17, 2025, Andy Shevchenko wrote:
> > On Thu, Jan 16, 2025 at 11:39:42PM +0000, Thinh Nguyen wrote:
> > > On Thu, Jan 16, 2025, Andy Shevchenko wrote:

...

> > > > + * Intel Merrifield uses these endpoints for tracing and they shouldn't be used
> > > > + * for normal transfers, we need to skip them.
> > > > + * • 1 High BW Bulk IN (IN#1) (RTIT)
> > > > + * • 1 1KB BW Bulk IN (IN#8) + 1 1KB BW Bulk OUT (Run Control) (OUT#8)
> > > 
> > > Please use regular bullet character and list the endpoint per line.
> > 
> > Which is...?
> > 
> > To my curiosity, what's wrong with the above?
> 
> Please use a character that we can find on the keyboard (- or * for
> example).

Hmm... We can find all characters on keyboard by using standard approach of
typing Unicode ones. I'm not sure why this is a problem. Linux kernel is UTF-8
ready project (from source tree point of view), at least I haven't found any
limitations in the documentation.

Note, this is _not_ a kernel-doc style to which you may refer when pointing out
to the how lists should be represented.

But it's not big deal for me to change the • character.

> And why would you want to list them like this:
> 
> 	* Endpoint A
> 	* Endpoint B + Endpoint C

Because:
1) they are logically connected;
2) the above is the exact citation from the specification and I would like to
keep it that way.

> As oppose to:
> 
> 	* Endpoint A
> 	* Endpoint B
> 	* Endpoint C

-- 
With Best Regards,
Andy Shevchenko

On Wed, Jan 22, 2025, Andy Shevchenko wrote:
> On Tue, Jan 21, 2025 at 11:46:17PM +0000, Thinh Nguyen wrote:
> > On Fri, Jan 17, 2025, Andy Shevchenko wrote:
> > > On Thu, Jan 16, 2025 at 11:39:42PM +0000, Thinh Nguyen wrote:
> > > > On Thu, Jan 16, 2025, Andy Shevchenko wrote:
> 
> ...
> 
> > > > > + * Intel Merrifield uses these endpoints for tracing and they shouldn't be used
> > > > > + * for normal transfers, we need to skip them.
> > > > > + * • 1 High BW Bulk IN (IN#1) (RTIT)
> > > > > + * • 1 1KB BW Bulk IN (IN#8) + 1 1KB BW Bulk OUT (Run Control) (OUT#8)
> > > > 
> > > > Please use regular bullet character and list the endpoint per line.
> > > 
> > > Which is...?
> > > 
> > > To my curiosity, what's wrong with the above?
> > 
> > Please use a character that we can find on the keyboard (- or * for
> > example).
> 
> Hmm... We can find all characters on keyboard by using standard approach of
> typing Unicode ones. I'm not sure why this is a problem. Linux kernel is UTF-8
> ready project (from source tree point of view), at least I haven't found any
> limitations in the documentation.
> 
> Note, this is _not_ a kernel-doc style to which you may refer when pointing out

I'm not requesting this out of any kernel-doc style. It's just a
personal preference and consistency in dwc3. If it's not too difficult,
please use "-". But if you must insist, future lists would need to be
consistent to this new unicode style. Then I would need to ask others to
use the new Unicode one. Typically typing * doesn't automatically
convert to • unless you edit using Word, and so I prefer something I and
others can easily find on the keyboard.

> to the how lists should be represented.
> 
> But it's not big deal for me to change the • character.
> 
> > And why would you want to list them like this:
> > 
> > 	* Endpoint A
> > 	* Endpoint B + Endpoint C
> 
> Because:
> 1) they are logically connected;
> 2) the above is the exact citation from the specification and I would like to
> keep it that way.
> 
> > As oppose to:
> > 
> > 	* Endpoint A
> > 	* Endpoint B
> > 	* Endpoint C
> 

If you prefer to keep the snippet of your vendor specification intact,
we can instead document this fully in the commit message and note the
EBC feature. Remove these comments here.

Thanks,
Thinh

On Tue, Jan 28, 2025 at 02:21:40AM +0000, Thinh Nguyen wrote:
> On Wed, Jan 22, 2025, Andy Shevchenko wrote:
> > On Tue, Jan 21, 2025 at 11:46:17PM +0000, Thinh Nguyen wrote:
> > > On Fri, Jan 17, 2025, Andy Shevchenko wrote:
> > > > On Thu, Jan 16, 2025 at 11:39:42PM +0000, Thinh Nguyen wrote:
> > > > > On Thu, Jan 16, 2025, Andy Shevchenko wrote:

...

> > > > > > + * Intel Merrifield uses these endpoints for tracing and they shouldn't be used
> > > > > > + * for normal transfers, we need to skip them.
> > > > > > + * • 1 High BW Bulk IN (IN#1) (RTIT)
> > > > > > + * • 1 1KB BW Bulk IN (IN#8) + 1 1KB BW Bulk OUT (Run Control) (OUT#8)
> > > > > 
> > > > > Please use regular bullet character and list the endpoint per line.
> > > > 
> > > > Which is...?
> > > > 
> > > > To my curiosity, what's wrong with the above?
> > > 
> > > Please use a character that we can find on the keyboard (- or * for
> > > example).
> > 
> > Hmm... We can find all characters on keyboard by using standard approach of
> > typing Unicode ones. I'm not sure why this is a problem. Linux kernel is UTF-8
> > ready project (from source tree point of view), at least I haven't found any
> > limitations in the documentation.
> > 
> > Note, this is _not_ a kernel-doc style to which you may refer when pointing out
> 
> I'm not requesting this out of any kernel-doc style. It's just a
> personal preference and consistency in dwc3. If it's not too difficult,
> please use "-".

As I said...

> But if you must insist, future lists would need to be
> consistent to this new unicode style. Then I would need to ask others to
> use the new Unicode one. Typically typing * doesn't automatically
> convert to • unless you edit using Word, and so I prefer something I and
> others can easily find on the keyboard.
> 
> > to the how lists should be represented.
> > 
> > But it's not big deal for me to change the • character.

...not a big deal to me, I will change as requested.

> > > And why would you want to list them like this:
> > > 
> > > 	* Endpoint A
> > > 	* Endpoint B + Endpoint C
> > 
> > Because:
> > 1) they are logically connected;
> > 2) the above is the exact citation from the specification and I would like to
> > keep it that way.
> > 
> > > As oppose to:
> > > 
> > > 	* Endpoint A
> > > 	* Endpoint B
> > > 	* Endpoint C
> 
> If you prefer to keep the snippet of your vendor specification intact,
> we can instead document this fully in the commit message and note the
> EBC feature. Remove these comments here.

I prefer to have a comment to explain magic numbers. I just want it to be
as closer as possible to the specification wording.

-- 
With Best Regards,
Andy Shevchenko

On Tue, Jan 28, 2025, Andy Shevchenko wrote:
> On Tue, Jan 28, 2025 at 02:21:40AM +0000, Thinh Nguyen wrote:
> > 
> > If you prefer to keep the snippet of your vendor specification intact,
> > we can instead document this fully in the commit message and note the
> > EBC feature. Remove these comments here.
> 
> I prefer to have a comment to explain magic numbers. I just want it to be
> as closer as possible to the specification wording.
> 

Ok. That's fine.

BR,
Thinh

On Tue, Jan 21, 2025, Thinh Nguyen wrote:
> On Fri, Jan 17, 2025, Andy Shevchenko wrote:
> > On Thu, Jan 16, 2025 at 11:39:42PM +0000, Thinh Nguyen wrote:
> > > On Thu, Jan 16, 2025, Andy Shevchenko wrote:
> > 
> > ...
> > 
> > > > + * Intel Merrifield uses these endpoints for tracing and they shouldn't be used
> > > > + * for normal transfers, we need to skip them.
> > > > + * • 1 High BW Bulk IN (IN#1) (RTIT)
> > > > + * • 1 1KB BW Bulk IN (IN#8) + 1 1KB BW Bulk OUT (Run Control) (OUT#8)
> > > 
> > > Please use regular bullet character and list the endpoint per line.
> > 
> > Which is...?
> > 
> > To my curiosity, what's wrong with the above?
> > 
> 
> Please use a character that we can find on the keyboard (- or * for
> example).
> 
> And why would you want to list them like this:
> 
> 	* Endpoint A
> 	* Endpoint B + Endpoint C
> 
> As oppose to:
> 
> 	* Endpoint A
> 	* Endpoint B
> 	* Endpoint C
> 

Also, please fix the $subject and replace "endpoints ep[18]{in,out}" to
just "reserved endpoints".

BR,
Thinh

On Tue, Jan 21, 2025 at 11:51:51PM +0000, Thinh Nguyen wrote:
> On Tue, Jan 21, 2025, Thinh Nguyen wrote:

...

> Also, please fix the $subject and replace "endpoints ep[18]{in,out}" to
> just "reserved endpoints".

Sure.

Thank you for the review.

-- 
With Best Regards,
Andy Shevchenko