Message ID | f103baa439a64b517e6c8cb51c30e81987dfedb3.1501173271.git.digetx@gmail.com |
---|---|
State | Superseded |
Headers | show |
On Thu, Jul 27, 2017 at 07:46:07PM +0300, Dmitry Osipenko wrote: > From: Thierry Reding <treding@nvidia.com> > > All of these Tegra SoC generations have a ChipIdea UDC IP block that can > be used for device mode communication with a host. Implement rudimentary > support that doesn't allow switching between host and device modes. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > [digetx@gmail.com: rebased patches and added DMA alignment quirk for Tegra20] > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > Acked-by: Peter Chen <peter.chen@nxp.com> This works on my Asus TF300T (Tegra3 device). Minor nit below. Tested-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> (on tf300t) [...] > diff --git a/drivers/usb/chipidea/ci_hdrc_tegra.c b/drivers/usb/chipidea/ci_hdrc_tegra.c > new file mode 100644 > index 000000000000..7814500e71fa > --- /dev/null > +++ b/drivers/usb/chipidea/ci_hdrc_tegra.c > @@ -0,0 +1,160 @@ [...] > +struct tegra_udc_soc_info { > + bool supports_unaligned_dma; > +}; [...] > +static const struct tegra_udc_soc_info tegra20_udc_soc_info = { > + .supports_unaligned_dma = false, > +}; > + > +static const struct tegra_udc_soc_info tegra30_udc_soc_info = { > + .supports_unaligned_dma = true, > +}; > + > +static const struct tegra_udc_soc_info tegra114_udc_soc_info = { > + .supports_unaligned_dma = true, > +}; > + > +static const struct tegra_udc_soc_info tegra124_udc_soc_info = { > + .supports_unaligned_dma = true, > +}; > + > +static const struct of_device_id tegra_udc_of_match[] = { > + { > + .compatible = "nvidia,tegra20-udc", > + .data = &tegra20_udc_soc_info, > + }, { > + .compatible = "nvidia,tegra30-udc", > + .data = &tegra30_udc_soc_info, > + }, { > + .compatible = "nvidia,tegra114-udc", > + .data = &tegra114_udc_soc_info, > + }, { > + .compatible = "nvidia,tegra124-udc", > + .data = &tegra124_udc_soc_info, > + }, { > + /* sentinel */ > + } > +}; > +MODULE_DEVICE_TABLE(of, tegra_udc_of_match); [...] > +static int tegra_udc_probe(struct platform_device *pdev) > +{ [...] > + /* setup device specific quirks */ > + soc = id->data; > + > + if (!soc->supports_unaligned_dma) > + udc->data.flags |= CI_HDRC_REQUIRES_ALIGNED_DMA; [...] Some lines would be saved if this was just: udc->data.flags |= id->data; Best Regards, Michał Mirosław -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 28.07.2017 01:32, Michał Mirosław wrote: > On Thu, Jul 27, 2017 at 07:46:07PM +0300, Dmitry Osipenko wrote: >> From: Thierry Reding <treding@nvidia.com> >> >> All of these Tegra SoC generations have a ChipIdea UDC IP block that can >> be used for device mode communication with a host. Implement rudimentary >> support that doesn't allow switching between host and device modes. >> >> Signed-off-by: Thierry Reding <treding@nvidia.com> >> [digetx@gmail.com: rebased patches and added DMA alignment quirk for Tegra20] >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> Acked-by: Peter Chen <peter.chen@nxp.com> > > This works on my Asus TF300T (Tegra3 device). Minor nit below. > > Tested-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> (on tf300t) > Nice, thank you! > [...] >> diff --git a/drivers/usb/chipidea/ci_hdrc_tegra.c b/drivers/usb/chipidea/ci_hdrc_tegra.c >> new file mode 100644 >> index 000000000000..7814500e71fa >> --- /dev/null >> +++ b/drivers/usb/chipidea/ci_hdrc_tegra.c >> @@ -0,0 +1,160 @@ > [...] >> +struct tegra_udc_soc_info { >> + bool supports_unaligned_dma; >> +}; > [...] >> +static const struct tegra_udc_soc_info tegra20_udc_soc_info = { >> + .supports_unaligned_dma = false, >> +}; >> + >> +static const struct tegra_udc_soc_info tegra30_udc_soc_info = { >> + .supports_unaligned_dma = true, >> +}; >> + >> +static const struct tegra_udc_soc_info tegra114_udc_soc_info = { >> + .supports_unaligned_dma = true, >> +}; >> + >> +static const struct tegra_udc_soc_info tegra124_udc_soc_info = { >> + .supports_unaligned_dma = true, >> +}; >> + >> +static const struct of_device_id tegra_udc_of_match[] = { >> + { >> + .compatible = "nvidia,tegra20-udc", >> + .data = &tegra20_udc_soc_info, >> + }, { >> + .compatible = "nvidia,tegra30-udc", >> + .data = &tegra30_udc_soc_info, >> + }, { >> + .compatible = "nvidia,tegra114-udc", >> + .data = &tegra114_udc_soc_info, >> + }, { >> + .compatible = "nvidia,tegra124-udc", >> + .data = &tegra124_udc_soc_info, >> + }, { >> + /* sentinel */ >> + } >> +}; >> +MODULE_DEVICE_TABLE(of, tegra_udc_of_match); > [...] >> +static int tegra_udc_probe(struct platform_device *pdev) >> +{ > [...] >> + /* setup device specific quirks */ >> + soc = id->data; >> + >> + if (!soc->supports_unaligned_dma) >> + udc->data.flags |= CI_HDRC_REQUIRES_ALIGNED_DMA; > [...] > > Some lines would be saved if this was just: > > udc->data.flags |= id->data; > I think a verbose approach is more preferable and it also more future proof since we may extend soc_info with other stuff later.
On Fri, Jul 28, 2017 at 01:00:32PM +0300, Dmitry Osipenko wrote: > On 28.07.2017 01:32, Michał Mirosław wrote: > > On Thu, Jul 27, 2017 at 07:46:07PM +0300, Dmitry Osipenko wrote: > >> From: Thierry Reding <treding@nvidia.com> > >> > >> All of these Tegra SoC generations have a ChipIdea UDC IP block that can > >> be used for device mode communication with a host. Implement rudimentary > >> support that doesn't allow switching between host and device modes. > >> > >> Signed-off-by: Thierry Reding <treding@nvidia.com> > >> [digetx@gmail.com: rebased patches and added DMA alignment quirk for Tegra20] > >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > >> Acked-by: Peter Chen <peter.chen@nxp.com> > > > > This works on my Asus TF300T (Tegra3 device). Minor nit below. > > > > Tested-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> (on tf300t) > > > > Nice, thank you! > > > [...] > >> diff --git a/drivers/usb/chipidea/ci_hdrc_tegra.c b/drivers/usb/chipidea/ci_hdrc_tegra.c > >> new file mode 100644 > >> index 000000000000..7814500e71fa > >> --- /dev/null > >> +++ b/drivers/usb/chipidea/ci_hdrc_tegra.c > >> @@ -0,0 +1,160 @@ > > [...] > >> +struct tegra_udc_soc_info { > >> + bool supports_unaligned_dma; > >> +}; > > [...] > >> +static const struct tegra_udc_soc_info tegra20_udc_soc_info = { > >> + .supports_unaligned_dma = false, > >> +}; > >> + > >> +static const struct tegra_udc_soc_info tegra30_udc_soc_info = { > >> + .supports_unaligned_dma = true, > >> +}; > >> + > >> +static const struct tegra_udc_soc_info tegra114_udc_soc_info = { > >> + .supports_unaligned_dma = true, > >> +}; > >> + > >> +static const struct tegra_udc_soc_info tegra124_udc_soc_info = { > >> + .supports_unaligned_dma = true, > >> +}; > >> + > >> +static const struct of_device_id tegra_udc_of_match[] = { > >> + { > >> + .compatible = "nvidia,tegra20-udc", > >> + .data = &tegra20_udc_soc_info, > >> + }, { > >> + .compatible = "nvidia,tegra30-udc", > >> + .data = &tegra30_udc_soc_info, > >> + }, { > >> + .compatible = "nvidia,tegra114-udc", > >> + .data = &tegra114_udc_soc_info, > >> + }, { > >> + .compatible = "nvidia,tegra124-udc", > >> + .data = &tegra124_udc_soc_info, > >> + }, { > >> + /* sentinel */ > >> + } > >> +}; > >> +MODULE_DEVICE_TABLE(of, tegra_udc_of_match); > > [...] > >> +static int tegra_udc_probe(struct platform_device *pdev) > >> +{ > > [...] > >> + /* setup device specific quirks */ > >> + soc = id->data; > >> + > >> + if (!soc->supports_unaligned_dma) > >> + udc->data.flags |= CI_HDRC_REQUIRES_ALIGNED_DMA; > > [...] > > > > Some lines would be saved if this was just: > > > > udc->data.flags |= id->data; > > > > I think a verbose approach is more preferable and it also more future proof > since we may extend soc_info with other stuff later. I would just leave that to be done when it's needed. :-) Or you could just have udc_flags in struct tegra_udc_soc_info, so that it would not need to be translated - again saving code, bugs and future developers' time. Best Regards, Michał Mirosław -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 28, 2017 at 05:51:16PM +0200, Michał Mirosław wrote: > On Fri, Jul 28, 2017 at 01:00:32PM +0300, Dmitry Osipenko wrote: > > On 28.07.2017 01:32, Michał Mirosław wrote: > > > On Thu, Jul 27, 2017 at 07:46:07PM +0300, Dmitry Osipenko wrote: > > >> From: Thierry Reding <treding@nvidia.com> > > >> > > >> All of these Tegra SoC generations have a ChipIdea UDC IP block that can > > >> be used for device mode communication with a host. Implement rudimentary > > >> support that doesn't allow switching between host and device modes. > > >> > > >> Signed-off-by: Thierry Reding <treding@nvidia.com> > > >> [digetx@gmail.com: rebased patches and added DMA alignment quirk for Tegra20] > > >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > > >> Acked-by: Peter Chen <peter.chen@nxp.com> > > > > > > This works on my Asus TF300T (Tegra3 device). Minor nit below. > > > > > > Tested-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> (on tf300t) > > > > > > > Nice, thank you! > > > > > [...] > > >> diff --git a/drivers/usb/chipidea/ci_hdrc_tegra.c b/drivers/usb/chipidea/ci_hdrc_tegra.c > > >> new file mode 100644 > > >> index 000000000000..7814500e71fa > > >> --- /dev/null > > >> +++ b/drivers/usb/chipidea/ci_hdrc_tegra.c > > >> @@ -0,0 +1,160 @@ > > > [...] > > >> +struct tegra_udc_soc_info { > > >> + bool supports_unaligned_dma; > > >> +}; > > > [...] > > >> +static const struct tegra_udc_soc_info tegra20_udc_soc_info = { > > >> + .supports_unaligned_dma = false, > > >> +}; > > >> + > > >> +static const struct tegra_udc_soc_info tegra30_udc_soc_info = { > > >> + .supports_unaligned_dma = true, > > >> +}; > > >> + > > >> +static const struct tegra_udc_soc_info tegra114_udc_soc_info = { > > >> + .supports_unaligned_dma = true, > > >> +}; > > >> + > > >> +static const struct tegra_udc_soc_info tegra124_udc_soc_info = { > > >> + .supports_unaligned_dma = true, > > >> +}; > > >> + > > >> +static const struct of_device_id tegra_udc_of_match[] = { > > >> + { > > >> + .compatible = "nvidia,tegra20-udc", > > >> + .data = &tegra20_udc_soc_info, > > >> + }, { > > >> + .compatible = "nvidia,tegra30-udc", > > >> + .data = &tegra30_udc_soc_info, > > >> + }, { > > >> + .compatible = "nvidia,tegra114-udc", > > >> + .data = &tegra114_udc_soc_info, > > >> + }, { > > >> + .compatible = "nvidia,tegra124-udc", > > >> + .data = &tegra124_udc_soc_info, > > >> + }, { > > >> + /* sentinel */ > > >> + } > > >> +}; > > >> +MODULE_DEVICE_TABLE(of, tegra_udc_of_match); > > > [...] > > >> +static int tegra_udc_probe(struct platform_device *pdev) > > >> +{ > > > [...] > > >> + /* setup device specific quirks */ > > >> + soc = id->data; > > >> + > > >> + if (!soc->supports_unaligned_dma) > > >> + udc->data.flags |= CI_HDRC_REQUIRES_ALIGNED_DMA; > > > [...] > > > > > > Some lines would be saved if this was just: > > > > > > udc->data.flags |= id->data; > > > I prefer this way too, and add CI_HDRC_REQUIRES_ALIGNED_DMA as platform flags for necessary SoCs. Peter > > > > I think a verbose approach is more preferable and it also more future proof > > since we may extend soc_info with other stuff later. > > I would just leave that to be done when it's needed. :-) > > Or you could just have udc_flags in struct tegra_udc_soc_info, so > that it would not need to be translated - again saving code, bugs > and future developers' time. > > Best Regards, > Michał Mirosław > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 31.07.2017 04:14, Peter Chen wrote: > On Fri, Jul 28, 2017 at 05:51:16PM +0200, Michał Mirosław wrote: >> On Fri, Jul 28, 2017 at 01:00:32PM +0300, Dmitry Osipenko wrote: >>> On 28.07.2017 01:32, Michał Mirosław wrote: >>>> On Thu, Jul 27, 2017 at 07:46:07PM +0300, Dmitry Osipenko wrote: >>>>> From: Thierry Reding <treding@nvidia.com> >>>>> >>>>> All of these Tegra SoC generations have a ChipIdea UDC IP block that can >>>>> be used for device mode communication with a host. Implement rudimentary >>>>> support that doesn't allow switching between host and device modes. >>>>> >>>>> Signed-off-by: Thierry Reding <treding@nvidia.com> >>>>> [digetx@gmail.com: rebased patches and added DMA alignment quirk for Tegra20] >>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >>>>> Acked-by: Peter Chen <peter.chen@nxp.com> >>>> >>>> This works on my Asus TF300T (Tegra3 device). Minor nit below. >>>> >>>> Tested-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> (on tf300t) >>>> >>> >>> Nice, thank you! >>> >>>> [...] >>>>> diff --git a/drivers/usb/chipidea/ci_hdrc_tegra.c b/drivers/usb/chipidea/ci_hdrc_tegra.c >>>>> new file mode 100644 >>>>> index 000000000000..7814500e71fa >>>>> --- /dev/null >>>>> +++ b/drivers/usb/chipidea/ci_hdrc_tegra.c >>>>> @@ -0,0 +1,160 @@ >>>> [...] >>>>> +struct tegra_udc_soc_info { >>>>> + bool supports_unaligned_dma; >>>>> +}; >>>> [...] >>>>> +static const struct tegra_udc_soc_info tegra20_udc_soc_info = { >>>>> + .supports_unaligned_dma = false, >>>>> +}; >>>>> + >>>>> +static const struct tegra_udc_soc_info tegra30_udc_soc_info = { >>>>> + .supports_unaligned_dma = true, >>>>> +}; >>>>> + >>>>> +static const struct tegra_udc_soc_info tegra114_udc_soc_info = { >>>>> + .supports_unaligned_dma = true, >>>>> +}; >>>>> + >>>>> +static const struct tegra_udc_soc_info tegra124_udc_soc_info = { >>>>> + .supports_unaligned_dma = true, >>>>> +}; >>>>> + >>>>> +static const struct of_device_id tegra_udc_of_match[] = { >>>>> + { >>>>> + .compatible = "nvidia,tegra20-udc", >>>>> + .data = &tegra20_udc_soc_info, >>>>> + }, { >>>>> + .compatible = "nvidia,tegra30-udc", >>>>> + .data = &tegra30_udc_soc_info, >>>>> + }, { >>>>> + .compatible = "nvidia,tegra114-udc", >>>>> + .data = &tegra114_udc_soc_info, >>>>> + }, { >>>>> + .compatible = "nvidia,tegra124-udc", >>>>> + .data = &tegra124_udc_soc_info, >>>>> + }, { >>>>> + /* sentinel */ >>>>> + } >>>>> +}; >>>>> +MODULE_DEVICE_TABLE(of, tegra_udc_of_match); >>>> [...] >>>>> +static int tegra_udc_probe(struct platform_device *pdev) >>>>> +{ >>>> [...] >>>>> + /* setup device specific quirks */ >>>>> + soc = id->data; >>>>> + >>>>> + if (!soc->supports_unaligned_dma) >>>>> + udc->data.flags |= CI_HDRC_REQUIRES_ALIGNED_DMA; >>>> [...] >>>> >>>> Some lines would be saved if this was just: >>>> >>>> udc->data.flags |= id->data; >>>> > > I prefer this way too, and add CI_HDRC_REQUIRES_ALIGNED_DMA as > platform flags for necessary SoCs. > Okay, please let me know if there are any other nitpicks.
diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile index 39fca5715ed3..ddcbddf8361a 100644 --- a/drivers/usb/chipidea/Makefile +++ b/drivers/usb/chipidea/Makefile @@ -15,3 +15,4 @@ obj-$(CONFIG_USB_CHIPIDEA) += ci_hdrc_zevio.o obj-$(CONFIG_USB_CHIPIDEA_PCI) += ci_hdrc_pci.o obj-$(CONFIG_USB_CHIPIDEA_OF) += usbmisc_imx.o ci_hdrc_imx.o +obj-$(CONFIG_USB_CHIPIDEA_OF) += ci_hdrc_tegra.o diff --git a/drivers/usb/chipidea/ci_hdrc_tegra.c b/drivers/usb/chipidea/ci_hdrc_tegra.c new file mode 100644 index 000000000000..7814500e71fa --- /dev/null +++ b/drivers/usb/chipidea/ci_hdrc_tegra.c @@ -0,0 +1,160 @@ +/* + * Copyright (c) 2016, NVIDIA Corporation + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + */ + +#include <linux/clk.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/reset.h> + +#include <linux/usb/chipidea.h> + +#include "ci.h" + +struct tegra_udc_soc_info { + bool supports_unaligned_dma; +}; + +struct tegra_udc { + struct ci_hdrc_platform_data data; + struct platform_device *dev; + + struct usb_phy *phy; + struct clk *clk; +}; + +static const struct tegra_udc_soc_info tegra20_udc_soc_info = { + .supports_unaligned_dma = false, +}; + +static const struct tegra_udc_soc_info tegra30_udc_soc_info = { + .supports_unaligned_dma = true, +}; + +static const struct tegra_udc_soc_info tegra114_udc_soc_info = { + .supports_unaligned_dma = true, +}; + +static const struct tegra_udc_soc_info tegra124_udc_soc_info = { + .supports_unaligned_dma = true, +}; + +static const struct of_device_id tegra_udc_of_match[] = { + { + .compatible = "nvidia,tegra20-udc", + .data = &tegra20_udc_soc_info, + }, { + .compatible = "nvidia,tegra30-udc", + .data = &tegra30_udc_soc_info, + }, { + .compatible = "nvidia,tegra114-udc", + .data = &tegra114_udc_soc_info, + }, { + .compatible = "nvidia,tegra124-udc", + .data = &tegra124_udc_soc_info, + }, { + /* sentinel */ + } +}; +MODULE_DEVICE_TABLE(of, tegra_udc_of_match); + +static int tegra_udc_probe(struct platform_device *pdev) +{ + const struct tegra_udc_soc_info *soc; + const struct of_device_id *id; + struct tegra_udc *udc; + int err; + + udc = devm_kzalloc(&pdev->dev, sizeof(*udc), GFP_KERNEL); + if (!udc) + return -ENOMEM; + + id = of_match_node(tegra_udc_of_match, pdev->dev.of_node); + if (!id) + return -ENODEV; + + udc->phy = devm_usb_get_phy_by_phandle(&pdev->dev, "nvidia,phy", 0); + if (IS_ERR(udc->phy)) { + err = PTR_ERR(udc->phy); + dev_err(&pdev->dev, "failed to get PHY: %d\n", err); + return err; + } + + udc->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(udc->clk)) { + err = PTR_ERR(udc->clk); + dev_err(&pdev->dev, "failed to get clock: %d\n", err); + return err; + } + + err = clk_prepare_enable(udc->clk); + if (err < 0) { + dev_err(&pdev->dev, "failed to enable clock: %d\n", err); + return err; + } + + /* setup and register ChipIdea HDRC device */ + udc->data.name = "tegra-udc"; + udc->data.capoffset = DEF_CAPOFFSET; + udc->data.flags = 0; + udc->data.usb_phy = udc->phy; + + /* setup device specific quirks */ + soc = id->data; + + if (!soc->supports_unaligned_dma) + udc->data.flags |= CI_HDRC_REQUIRES_ALIGNED_DMA; + + /* + * Tegra's USB PHY driver doesn't implement optional phy_init() + * hook, so we have to power on UDC controller before ChipIdea + * driver initialization kicks in. + */ + usb_phy_set_suspend(udc->phy, 0); + + udc->dev = ci_hdrc_add_device(&pdev->dev, pdev->resource, + pdev->num_resources, &udc->data); + if (IS_ERR(udc->dev)) { + err = PTR_ERR(udc->dev); + dev_err(&pdev->dev, "failed to add HDRC device: %d\n", err); + goto power_off; + } + + platform_set_drvdata(pdev, udc); + + return 0; + +power_off: + usb_phy_set_suspend(udc->phy, 1); + clk_disable_unprepare(udc->clk); + return err; +} + +static int tegra_udc_remove(struct platform_device *pdev) +{ + struct tegra_udc *udc = platform_get_drvdata(pdev); + + usb_phy_set_suspend(udc->phy, 1); + clk_disable_unprepare(udc->clk); + + return 0; +} + +static struct platform_driver tegra_udc_driver = { + .driver = { + .name = "tegra-udc", + .of_match_table = tegra_udc_of_match, + }, + .probe = tegra_udc_probe, + .remove = tegra_udc_remove, +}; +module_platform_driver(tegra_udc_driver); + +MODULE_DESCRIPTION("NVIDIA Tegra USB device mode driver"); +MODULE_AUTHOR("Thierry Reding <treding@nvidia.com>"); +MODULE_ALIAS("platform:tegra-udc"); +MODULE_LICENSE("GPL v2");