Hi,
Felipe Balbi wrote:
> Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes:
>
>> DWC_usb32 supports dual-lane at different transfer rate. This patch
>> initializes the controller to use the maximum support transfer rate
>> describes by the dwc3 device property of lane count and lane speed
>> mantissa in Gbps.
> Perhaps this should read as:
>
> "DWC_usb32 supports dual-lane at different transfer rate. This
> patch initializes the controller to use the maximum supported
> transfer rate described by the dwc3 device property of lane
> count and lane speed mantissa in Gbps."
Ok.
>> @@ -1424,6 +1428,38 @@ static void dwc3_check_params(struct dwc3 *dwc)
>>
>> break;
>> }
>> +
>> + switch (dwc->maximum_lsm) {
>> + case 5:
>> + break;
>> + case 10:
>> + if (dwc->maximum_speed == USB_SPEED_SUPER)
>> + dev_err(dev, "invalid maximum_lsm parameter %d\n",
>> + dwc->maximum_lsm);
>> + /* Fall Through */
>> + default:
>> + if (dwc->maximum_speed == USB_SPEED_SUPER)
>> + dwc->maximum_lsm = 5;
>> + else if (dwc->maximum_speed > USB_SPEED_SUPER)
>> + dwc->maximum_lsm = 10;
>> + break;
>> + }
>> +
>> + switch (dwc->maximum_lanes) {
>> + case 1:
>> + case 2:
>> + break;
>> + default:
>> + if (dwc->maximum_lanes > 2)
>> + dev_err(dev, "invalid number of lanes %d\n",
>> + dwc->maximum_lanes);
>> +
>> + if (dwc3_is_usb32(dwc) &&
>> + dwc->maximum_speed == USB_SPEED_SUPER_PLUS)
>> + dwc->maximum_lanes = 2;
>> + else
>> + dwc->maximum_lanes = 1;
>> + }
>> }
>>
>> static int dwc3_probe(struct platform_device *pdev)
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 7fde3c7da543..8e729d4cd5bd 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -376,6 +376,8 @@
>> #define DWC3_GUCTL2_RST_ACTBITLATER BIT(14)
>>
>> /* Device Configuration Register */
>> +#define DWC3_DCFG_NUMLANES(n) (((n) & 0x3) << 30) /* DWC_usb32 only */
>> +
>> #define DWC3_DCFG_DEVADDR(addr) ((addr) << 3)
>> #define DWC3_DCFG_DEVADDR_MASK DWC3_DCFG_DEVADDR(0x7f)
>>
>> @@ -449,6 +451,8 @@
>> #define DWC3_DEVTEN_USBRSTEN BIT(1)
>> #define DWC3_DEVTEN_DISCONNEVTEN BIT(0)
>>
>> +#define DWC3_DSTS_CONNLANES(n) (((n) >> 30) & 0x3) /* DWC_usb32 only */
>> +
>> /* Device Status Register */
>> #define DWC3_DSTS_DCNRD BIT(29)
>>
>> @@ -946,6 +950,8 @@ struct dwc3_scratchpad_array {
>> * @nr_scratch: number of scratch buffers
>> * @u1u2: only used on revisions <1.83a for workaround
>> * @maximum_speed: maximum speed requested (mainly for testing purposes)
>> + * @maximum_lsm: maximum lane speed mantissa in Gbps
>> + * @maximum_lanes: maximum lane count
>> * @ip: controller's ID
>> * @revision: controller's version of an IP
>> * @version_type: VERSIONTYPE register contents, a sub release of a revision
>> @@ -973,6 +979,7 @@ struct dwc3_scratchpad_array {
>> * @ep0state: state of endpoint zero
>> * @link_state: link state
>> * @speed: device speed (super, high, full, low)
>> + * @lane_count: number of connected lanes
>> * @hwparams: copy of hwparams registers
>> * @root: debugfs root folder pointer
>> * @regset: debugfs pointer to regdump file
>> @@ -1100,6 +1107,8 @@ struct dwc3 {
>> u32 nr_scratch;
>> u32 u1u2;
>> u32 maximum_speed;
>> + u8 maximum_lsm;
>> + u8 maximum_lanes;
>>
>> u32 ip;
>>
>> @@ -1159,6 +1168,7 @@ struct dwc3 {
>> u8 u1pel;
>>
>> u8 speed;
>> + u8 lane_count;
>>
>> u8 num_eps;
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index a6d562e208a9..c31144af3261 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2183,6 +2183,53 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
>> spin_unlock_irqrestore(&dwc->lock, flags);
>> }
>>
>> +static void dwc3_gadget_set_sublink_attr(struct usb_gadget *g,
>> + unsigned int lane_count,
>> + unsigned int lsm)
>> +{
>> + struct dwc3 *dwc = gadget_to_dwc(g);
>> + unsigned int lanes;
>> + unsigned long flags;
>> + u32 reg;
>> +
>> + spin_lock_irqsave(&dwc->lock, flags);
>> + if (dwc->maximum_speed <= USB_SPEED_SUPER) {
>> + /* Fall back to maximum speed supported by HW */
>> + spin_unlock_irqrestore(&dwc->lock, flags);
>> + dwc3_gadget_set_speed(g, dwc->maximum_speed);
>> + spin_lock_irqsave(&dwc->lock, flags);
> it looks like we should extract a __dwc3_gadget_set_speed() to avoid the
> possible race here:
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index a9aba716bf80..e317b696029e 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2118,14 +2118,11 @@ static void dwc3_gadget_config_params(struct usb_gadget *g,
> cpu_to_le16(DWC3_DEFAULT_U2_DEV_EXIT_LAT);
> }
>
> -static void dwc3_gadget_set_speed(struct usb_gadget *g,
> +static void __dwc3_gadget_set_speed(struct dwc3 *dwc,
> enum usb_device_speed speed)
> {
> - struct dwc3 *dwc = gadget_to_dwc(g);
> - unsigned long flags;
> u32 reg;
>
> - spin_lock_irqsave(&dwc->lock, flags);
> reg = dwc3_readl(dwc->regs, DWC3_DCFG);
> reg &= ~(DWC3_DCFG_SPEED_MASK);
>
> @@ -2175,7 +2172,16 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
> }
> }
> dwc3_writel(dwc->regs, DWC3_DCFG, reg);
> +}
> +
> +static void dwc3_gadget_set_speed(struct usb_gadget *g,
> + enum usb_device_speed speed)
> +{
> + struct dwc3 *dwc = gadget_to_dwc(g);
> + unsigned long flags;
>
> + spin_lock_irqsave(&dwc->lock, flags);
> + __dwc3_gadget_set_speed(dwc, speed);
> spin_unlock_irqrestore(&dwc->lock, flags);
> }
>
> Then your patch would look like:
>
> static void dwc3_gadget_set_sublink_attr(struct usb_gadget *g,
> unsigned int lane_count,
> unsigned int lsm)
> {
> struct dwc3 *dwc = gadget_to_dwc(g);
> unsigned int lanes;
> unsigned long flags;
> u32 reg;
>
> spin_lock_irqsave(&dwc->lock, flags);
> if (dwc->maximum_speed <= USB_SPEED_SUPER) {
> /* Fall back to maximum speed supported by HW */
> __dwc3_gadget_set_speed(dwc, dwc->maximum_speed);
> goto done;
> }
>
> [...]
> }
Ok. I'll revise this.
Thanks!
Thinh
