usb: gadget: Add UDC driver for tegra XUSB device mode controller
PATCH v2 7/8 usb: gadget: Add UDC driver for tegra XUSB device mode controller
Quoted 1 line · Nagarjuna Kristam
This patch adds UDC driver for tegra XUSB 3.0 device mode controller.
s/tegra/Tegra/
Quoted 1 line
XUSB device mode controller support SS, HS and FS modes
s/support/supports/ and terminate the sentence with a full-stop.
Quoted 18 lines
Based on work by:
Mark Kuo <[email protected]>
Andrew Bresticker <[email protected]>Signed-off-by Nagarjuna Kristam <[email protected]>--- drivers/usb/gadget/udc/Kconfig | 10 + drivers/usb/gadget/udc/Makefile | 1 + drivers/usb/gadget/udc/tegra_xudc.c | 3702 +++++++++++++++++++++++++++++++++++ 3 files changed, 3713 insertions(+) create mode 100644 drivers/usb/gadget/udc/tegra_xudc.cdrivers/usb/gadget/udc/Kconfig +0
-0
drivers/usb/gadget/udc/Kconfig config USB_GADGET_XILINX · line 443
Quoted 5 lines
drivers/usb/gadget/udc/Kconfig +2
-0@@ -439,6 +439,16 @@ config USB_GADGET_XILINX dynamically linked module called "udc-xilinx" and force all gadget drivers to also be dynamically linked. +config USB_TEGRA_XUDC + tristate "NVIDIA Superspeed USB 3.0 Device Controller"
NVIDIA Tegra? Not sure if this is available anywhere else.
drivers/usb/gadget/udc/Kconfig config USB_GADGET_XILINX · line 446
Quoted 3 lines
drivers/usb/gadget/udc/Kconfig +3
-0+ depends on ARCH_TEGRA + help + Enables TEGRA USB 3.0 device mode controller driver.
NVIDIA Tegra here, too.
Quoted 26 lines
drivers/usb/gadget/udc/Kconfig +5
-0+ + Say "y" to link the driver statically, or "m" to build a + dynamically linked module called "tegra_xudc" and force all + gadget drivers to also be dynamically linked. + source "drivers/usb/gadget/udc/aspeed-vhub/Kconfig" #drivers/usb/gadget/udc/Makefile +1
-0@@ -24,6 +24,7 @@ obj-$(CONFIG_USB_BCM63XX_UDC) += bcm63xx_udc.o obj-$(CONFIG_USB_FSL_USB2) += fsl_usb2_udc.o fsl_usb2_udc-y := fsl_udc_core.o fsl_usb2_udc-$(CONFIG_ARCH_MXC) += fsl_mxc_udc.o +obj-$(CONFIG_USB_TEGRA_XUDC) += tegra_xudc.o obj-$(CONFIG_USB_M66592) += m66592-udc.o obj-$(CONFIG_USB_R8A66597) += r8a66597-udc.o obj-$(CONFIG_USB_RENESAS_USB3) += renesas_usb3.odrivers/usb/gadget/udc/tegra_xudc.c new file +0
-0
drivers/usb/gadget/udc/tegra_xudc.c line 3
Quoted 3 lines
drivers/usb/gadget/udc/tegra_xudc.c new file +3
-0@@ -0,0 +1,3702 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* +* NVIDIA XUSB device mode controller
Again, perhaps mention that this is for Tegra.
I didn't find anything that stood out in most of the driver. Below are a couple of things towards the end that I think you should look at.
Generally I thought it was fairly difficult to read. You may want to see if you can improve readability by adding a bit of whitespace where appropriate. For example, try to leave a blank line above and below block elements, such as conditionals or loops. I find that that helps a lot in making the code easier to read.
See below for an example.
[...]
drivers/usb/gadget/udc/tegra_xudc.c line 15
Quoted 12 lines
drivers/usb/gadget/udc/tegra_xudc.c new file +12
-0+static int tegra_xudc_probe (struct platform_device *pdev) +{ +struct tegra_xudc *xudc; +struct resource *res; +unsigned int i; +int err; + + xudc =devm_kzalloc (&pdev->dev,sizeof (*xudc),GFP_ATOMIC ); +if (!xudc) +return -ENOMEM ; + xudc->dev = &pdev->dev; +platform_set_drvdata (pdev, xudc);
This, for example, would be easier to read as:
xudc = devm_kzalloc(&pdev->dev, sizeof(*xudc), GFP_ATOMIC); if (!xudc) return -ENOMEM; platform_set_drvdata(pdev, xudc); xudc->dev = &pdev->dev;
drivers/usb/gadget/udc/tegra_xudc.c line 19
Quoted 4 lines
drivers/usb/gadget/udc/tegra_xudc.c new file +4
-0+ + xudc->soc =of_device_get_match_data (&pdev->dev); +if (!xudc->soc) +return -ENODEV ;
This situation can never happen. The driver is only ever instantiated from device tree, in which case xudc->soc will be a valid pointer to the correct SoC data structure.
Quoted 173 lines
drivers/usb/gadget/udc/tegra_xudc.c new file +173
-0+ + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); + xudc->base =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (xudc->base)) +return PTR_ERR (xudc->base); + xudc->phys_base = res->start; + + res =platform_get_resource (pdev,IORESOURCE_MEM ,1 ); + xudc->fpci =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (xudc->fpci)) +return PTR_ERR (xudc->fpci); + +if (xudc->soc->has_ipfs) { + res =platform_get_resource (pdev,IORESOURCE_MEM ,2 ); + xudc->ipfs =devm_ioremap_resource (&pdev->dev, res); +if (IS_ERR (xudc->ipfs)) +return PTR_ERR (xudc->ipfs); + } + + xudc->irq =platform_get_irq (pdev,0 ); +if (xudc->irq <0 ) { +dev_err (xudc->dev,"failed to get IRQ: %d\n" , + xudc->irq); +return xudc->irq; + } + err =devm_request_irq (&pdev->dev, xudc->irq, tegra_xudc_irq,0 , +dev_name (&pdev->dev), xudc); +if (err <0 ) { +dev_err (xudc->dev,"failed to claim IRQ#%u: %d\n" , xudc->irq, + err); +return err; + } + + xudc->clks =devm_kcalloc (&pdev->dev, xudc->soc->num_clks, +sizeof (*xudc->clks),GFP_KERNEL ); +if (!xudc->clks) +return -ENOMEM ; +for (i =0 ; i < xudc->soc->num_clks; i++) + xudc->clks[i].id = xudc->soc->clock_names[i]; + err =devm_clk_bulk_get (&pdev->dev, xudc->soc->num_clks, + xudc->clks); +if (err) { +dev_err (xudc->dev,"failed to request clks %d\n" , err); +return err; + } + + xudc->supplies =devm_kcalloc (&pdev->dev, xudc->soc->num_supplies, +sizeof (*xudc->supplies),GFP_KERNEL ); +if (!xudc->supplies) +return -ENOMEM ; +for (i =0 ; i < xudc->soc->num_supplies; i++) + xudc->supplies[i].supply = xudc->soc->supply_names[i]; + err =devm_regulator_bulk_get (&pdev->dev, xudc->soc->num_supplies, + xudc->supplies); +if (err) { +dev_err (xudc->dev,"failed to request regulators %d\n" , err); +return err; + } + + xudc->padctl =tegra_xusb_padctl_get (&pdev->dev); +if (IS_ERR (xudc->padctl)) +return PTR_ERR (xudc->padctl); + + err =regulator_bulk_enable (xudc->soc->num_supplies, xudc->supplies); +if (err) { +dev_err (xudc->dev,"failed to enable regulators %d\n" , err); + goto put_padctl; + } + + xudc->usb3_phy =devm_phy_optional_get (&pdev->dev,"usb3" ); +if (IS_ERR (xudc->usb3_phy)) { + err =PTR_ERR (xudc->usb3_phy); +dev_err (xudc->dev,"failed to get usb3 phy: %d\n" , err); + goto disable_regulator; + } + xudc->utmi_phy =devm_phy_optional_get (&pdev->dev,"usb2" ); +if (IS_ERR (xudc->utmi_phy)) { + err =PTR_ERR (xudc->utmi_phy); +dev_err (xudc->dev,"failed to get usb2 phy: %d\n" , err); + goto disable_regulator; + } + + xudc->data_role_extcon =extcon_get_edev_by_phandle (&pdev->dev,0 ); +if (IS_ERR (xudc->data_role_extcon)) { + err =PTR_ERR (xudc->data_role_extcon); +dev_err (xudc->dev,"extcon_get_edev_by_phandle failed %d\n" , + err); + goto disable_regulator; + } + + err =tegra_xudc_powerdomain_init (xudc); +if (err) + goto put_powerdomains; + + err =tegra_xudc_phy_init (xudc); +if (err) + goto put_powerdomains; + + err =tegra_xudc_alloc_event_ring (xudc); +if (err) + goto disable_phy; + + err =tegra_xudc_alloc_eps (xudc); +if (err) + goto free_event_ring; + +spin_lock_init (&xudc->lock); +init_completion (&xudc->disconnect_complete); + +INIT_WORK (&xudc->data_role_work, tegra_xudc_data_role_work); + xudc->data_role_nb.notifier_call = tegra_xudc_data_role_notifier; +extcon_register_notifier (xudc->data_role_extcon,EXTCON_USB , + &xudc->data_role_nb); + +INIT_DELAYED_WORK (&xudc->plc_reset_work, tegra_xudc_plc_reset_work); + +INIT_DELAYED_WORK (&xudc->port_reset_war_work, + tegra_xudc_port_reset_war_work); + +pm_runtime_enable (&pdev->dev); + + xudc->gadget.ops = &tegra_xudc_gadget_ops; + xudc->gadget.ep0 = &xudc->ep[0 ].usb_ep; + xudc->gadget.name ="tegra-xudc" ; + xudc->gadget.max_speed =USB_SPEED_SUPER ; + + err =usb_add_gadget_udc (&pdev->dev, &xudc->gadget); +if (err) { +dev_err (&pdev->dev,"failed to add USB gadget: %d\n" , err); + goto free_eps; + } + +tegra_xudc_update_data_role (xudc); + +return 0 ; + +free_eps: +tegra_xudc_free_eps (xudc); +free_event_ring: +tegra_xudc_free_event_ring (xudc); +disable_phy: +tegra_xudc_phy_exit (xudc); +put_powerdomains: +tegra_xudc_powerdomain_remove (xudc); +disable_regulator: +regulator_bulk_disable (xudc->soc->num_supplies, xudc->supplies); +put_padctl: +tegra_xusb_padctl_put (xudc->padctl); + +return err; +} + +static int tegra_xudc_remove (struct platform_device *pdev) +{ +struct tegra_xudc *xudc =platform_get_drvdata (pdev); + +pm_runtime_get_sync (xudc->dev); + +cancel_delayed_work (&xudc->plc_reset_work); +if (xudc->data_role_extcon) { +extcon_unregister_notifier (xudc->data_role_extcon,EXTCON_USB , + &xudc->data_role_nb); +cancel_work_sync (&xudc->data_role_work); + } +usb_del_gadget_udc (&xudc->gadget); +tegra_xudc_free_eps (xudc); +tegra_xudc_free_event_ring (xudc); +tegra_xudc_powerdomain_remove (xudc); + +regulator_bulk_disable (xudc->soc->num_supplies, xudc->supplies); + +phy_power_off (xudc->utmi_phy); +phy_power_off (xudc->usb3_phy);
drivers/usb/gadget/udc/tegra_xudc.c line 202
Quoted 10 lines
drivers/usb/gadget/udc/tegra_xudc.c new file +10
-0+tegra_xudc_phy_exit (xudc); +pm_runtime_disable (xudc->dev); +pm_runtime_put (xudc->dev); + +tegra_xusb_padctl_put (xudc->padctl); + +return 0 ; +} + +#if IS_ENABLED (CONFIG_PM_SLEEP ) ||IS_ENABLED (CONFIG_PM )
I'd drop these and instead annotate with __maybe_unused. We no longer support building Tegra without PM, so this is mostly academic anyway.
drivers/usb/gadget/udc/tegra_xudc.c line 207
Quoted 5 lines
drivers/usb/gadget/udc/tegra_xudc.c new file +5
-0+static int tegra_xudc_powergate (struct tegra_xudc *xudc) +{ +unsigned long flags; + +dev_info (xudc->dev,"entering ELPG\n" );
There's a couple of dev_info() calls throughout the driver that I think are too noisy. I think most of those should be dev_dbg() so that users aren't bothered with them. In cases where you really want to highlight an error or something, make them dev_err() or dev_warn().
Quoted 17 lines
drivers/usb/gadget/udc/tegra_xudc.c new file +17
-0+spin_lock_irqsave (&xudc->lock, flags); + xudc->powergated = true; + xudc->saved_regs.ctrl =xudc_readl (xudc,CTRL ); + xudc->saved_regs.portpm =xudc_readl (xudc,PORTPM ); +xudc_writel (xudc,0 ,CTRL ); +spin_unlock_irqrestore (&xudc->lock, flags); + +clk_bulk_disable_unprepare (xudc->soc->num_clks, xudc->clks); +regulator_bulk_disable (xudc->soc->num_supplies, xudc->supplies); + +dev_info (xudc->dev,"entering ELPG done\n" ); +return 0 ; +} + +static int tegra_xudc_unpowergate (struct tegra_xudc *xudc) +{ +unsigned long flags;
drivers/usb/gadget/udc/tegra_xudc.c line 234
Quoted 10 lines
drivers/usb/gadget/udc/tegra_xudc.c new file +10
-0+int err; + +dev_info (xudc->dev,"exiting ELPG\n" ); + + err =regulator_bulk_enable (xudc->soc->num_supplies, + xudc->supplies); +if (err <0 ) +return err; + +
Gratuituous blank line.
Quoted 73 lines
drivers/usb/gadget/udc/tegra_xudc.c new file +73
-0+ err =clk_bulk_prepare_enable (xudc->soc->num_clks, xudc->clks); +if (err <0 ) +return err; + +tegra_xudc_fpci_ipfs_init (xudc); +tegra_xudc_device_params_init (xudc); + +tegra_xudc_init_event_ring (xudc); +tegra_xudc_init_eps (xudc); + +xudc_writel (xudc, xudc->saved_regs.portpm,PORTPM ); +xudc_writel (xudc, xudc->saved_regs.ctrl,CTRL ); + +spin_lock_irqsave (&xudc->lock, flags); + xudc->powergated = false; +spin_unlock_irqrestore (&xudc->lock, flags); + +dev_info (xudc->dev,"exiting ELPG done\n" ); +return 0 ; +} +#endif + +#ifdef CONFIG_PM_SLEEP +static int tegra_xudc_suspend (struct device *dev) +{ +struct tegra_xudc *xudc =dev_get_drvdata (dev); +unsigned long flags; + +spin_lock_irqsave (&xudc->lock, flags); + xudc->suspended = true; +spin_unlock_irqrestore (&xudc->lock, flags); + +if (xudc->data_role_extcon) +flush_work (&xudc->data_role_work); + +/* Forcibly disconnect before powergating. */ +tegra_xudc_device_mode_off (xudc); + +if (!pm_runtime_status_suspended (dev)) +tegra_xudc_powergate (xudc); + +pm_runtime_disable (dev); + +return 0 ; +} + +static int tegra_xudc_resume (struct device *dev) +{ +struct tegra_xudc *xudc =dev_get_drvdata (dev); +unsigned long flags; +int err; + + err =tegra_xudc_unpowergate (xudc); +if (err <0 ) +return err; + +spin_lock_irqsave (&xudc->lock, flags); + xudc->suspended = false; +spin_unlock_irqrestore (&xudc->lock, flags); + +tegra_xudc_update_data_role (xudc); + +pm_runtime_enable (dev); + +return 0 ; +} +#endif + +#ifdef CONFIG_PM +static int tegra_xudc_runtime_suspend (struct device *dev) +{ +struct tegra_xudc *xudc =dev_get_drvdata (dev); +
drivers/usb/gadget/udc/tegra_xudc.c line 317
Quoted 10 lines
drivers/usb/gadget/udc/tegra_xudc.c new file +10
-0+ return tegra_xudc_powergate(xudc); +} + +static int tegra_xudc_runtime_resume (struct device *dev) +{ +struct tegra_xudc *xudc =dev_get_drvdata (dev); + +return tegra_xudc_unpowergate (xudc); +} +#endif
__maybe_unused for these as well.
Thierry
Quoted 25 lines
drivers/usb/gadget/udc/tegra_xudc.c new file +22
-0+ +static const struct dev_pm_ops tegra_xudc_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(tegra_xudc_suspend, tegra_xudc_resume) +SET_RUNTIME_PM_OPS (tegra_xudc_runtime_suspend, + tegra_xudc_runtime_resume,NULL ) +}; + +static struct platform_driver tegra_xudc_driver = { + .probe = tegra_xudc_probe, + .remove = tegra_xudc_remove, + .driver = { + .name ="tegra-xudc" , + .pm = &tegra_xudc_pm_ops, + .of_match_table = tegra_xudc_of_match, + }, +}; +module_platform_driver (tegra_xudc_driver); + +MODULE_DESCRIPTION ("NVIDIA Tegra XUSB Device Controller" ); +MODULE_AUTHOR ("Andrew Bresticker <[email protected]>" ); +MODULE_AUTHOR ("Hui Fu" ); +MODULE_LICENSE ("GPL v2" );Signature · 1 line
2.7.4