16.12.2020 09:08, Peter Chen пишет:
> On 20-12-15 23:21:10, Dmitry Osipenko wrote:
>> From: Peter Geis <pgwipeout@xxxxxxxxx>
>>
>> struct tegra_usb_soc_info {
>> unsigned long flags;
>> + unsigned int txfifothresh;
>> + enum usb_dr_mode dr_mode;
>> +};
>> +
>> +static const struct tegra_usb_soc_info tegra20_ehci_soc_info = {
>> + .flags = CI_HDRC_REQUIRES_ALIGNED_DMA |
>> + CI_HDRC_OVERRIDE_PHY_CONTROL,
>> + .dr_mode = USB_DR_MODE_HOST,
>> + .txfifothresh = 10,
>> +};
>> +
>> +static const struct tegra_usb_soc_info tegra30_ehci_soc_info = {
>> + .flags = CI_HDRC_REQUIRES_ALIGNED_DMA |
>> + CI_HDRC_OVERRIDE_PHY_CONTROL
>> + .dr_mode = USB_DR_MODE_HOST,
>> + .txfifothresh = 16,
>> };
>>
>> static const struct tegra_usb_soc_info tegra_udc_soc_info = {
>> - .flags = CI_HDRC_REQUIRES_ALIGNED_DMA,
>> + .flags = CI_HDRC_REQUIRES_ALIGNED_DMA |
>> + CI_HDRC_OVERRIDE_PHY_CONTROL,
>> + .dr_mode = USB_DR_MODE_UNKNOWN,
>> };
>
> What's the usage for this dr_mode? Does these three SoCs only supports
> single mode, it usually sets dr_mode at board dts file to indicate
> USB role for this board.
All Tegra SoCs have three USB controllers. Only one of these three
controllers supports peripheral / OTG modes, the other two controllers
are fixed to the host mode and device trees do not specify the dr_mode
for the host controllers.
Hence we need to enforce the dr_mode=host for the host-mode controllers.
For UDC the default mode is "unknown" because actual mode comes from a
board's device tree.
>> static const struct of_device_id tegra_usb_of_match[] = {
>> {
>> + .compatible = "nvidia,tegra20-ehci",
>> + .data = &tegra20_ehci_soc_info,
>> + }, {
>> + .compatible = "nvidia,tegra30-ehci",
>> + .data = &tegra30_ehci_soc_info,
>> + }, {
>> .compatible = "nvidia,tegra20-udc",
>> .data = &tegra_udc_soc_info,
>
> Your compatible indicates this controller is single USB role, is it
> on purpose?
Yes, the "tegra20/30-ehci" controllers are fixed to the host-mode in
hardware.
...
>> +static int tegra_usb_internal_port_reset(struct ehci_hcd *ehci,
>> + u32 __iomem *portsc_reg,
>> + unsigned long *flags)
>> +{
>> + u32 saved_usbintr, temp;
>> + unsigned int i, tries;
>> + int retval = 0;
>> +
>> + saved_usbintr = ehci_readl(ehci, &ehci->regs->intr_enable);
>> + /* disable USB interrupt */
>> + ehci_writel(ehci, 0, &ehci->regs->intr_enable);
>> + spin_unlock_irqrestore(&ehci->lock, *flags);
>> +
>> + /*
>> + * Here we have to do Port Reset at most twice for
>> + * Port Enable bit to be set.
>> + */
>> + for (i = 0; i < 2; i++) {
>> + temp = ehci_readl(ehci, portsc_reg);
>> + temp |= PORT_RESET;
>> + ehci_writel(ehci, temp, portsc_reg);
>> + mdelay(10);
>
> Needs accurate delay? How about usleep_range?
To be hones I don't know for sure whether hub_control() could be used
from interrupt context. This mdelay is borrowed from the older
ehci-tegra driver.
Are you suggesting that it should be safe to sleep here?
...
>> static int tegra_usb_probe(struct platform_device *pdev)
>> {
>> const struct tegra_usb_soc_info *soc;
>> @@ -83,24 +292,49 @@ static int tegra_usb_probe(struct platform_device *pdev)
>> return err;
>> }
>>
>> + if (device_property_present(&pdev->dev, "nvidia,needs-double-reset"))
>> + usb->needs_double_reset = true;
>> +
>> + err = tegra_usb_reset_controller(&pdev->dev);
>> + if (err) {
>> + dev_err(&pdev->dev, "failed to reset controller: %d\n", err);
>> + goto fail_power_off;
>> + }
>> +
>> + /*
>> + * USB controller registers shan't be touched before PHY is
>
> %s/shan't/shouldn't
ok
...
>> static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable)
>> {
>> struct ehci_hcd *ehci = hcd_to_ehci(hcd);
>> @@ -160,14 +166,14 @@ static int host_start(struct ci_hdrc *ci)
>> pinctrl_select_state(ci->platdata->pctl,
>> ci->platdata->pins_host);
>>
>> + ci->hcd = hcd;
>> +
>> ret = usb_add_hcd(hcd, 0, 0);
>> if (ret) {
>> goto disable_reg;
>> } else {
>> struct usb_otg *otg = &ci->otg;
>>
>> - ci->hcd = hcd;
>> -
>
> Why this changed?
The ci->hcd is used by tegra_usb_notify_event() to handle reset event
and the reset happens once usb_add_hcd() is invoked. Hence we need to
pre-assign the hcd pointer, otherwise there will be a NULL dereference.
