| Message ID | 1414497280-3126-20-git-send-email-ray.huang@amd.com |
|---|---|
| State | Not Applicable |
| Headers | show |
Hi, almost there... On Tue, Oct 28, 2014 at 07:54:40PM +0800, Huang Rui wrote: > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 8b94ad5..b08a2f9 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -699,6 +699,7 @@ struct dwc3_scratchpad_array { > * @rx_detect_poll_quirk: set if we enable rx_detect to polling lfps quirk > * @dis_u3_susphy_quirk: set if we disable usb3 suspend phy > * @dis_u2_susphy_quirk: set if we disable usb2 suspend phy > + * @amd_nl_plat: set if we use AMD NL platform however, as I mentioned before, the core shouldn't have to know that it's running on an AMD platform. We already support several different platforms (OMAP5, AM437x, DRA7xx, Exynos5, Exynos7, Qcom, Merrifield, Baytrail, Braswell, HAPS PCIe, and STiH407) and none of them get their $my_awesome_platform flag in dwc3, why should AMD be any different ? This is the only part of $subject that I cannot accept because it would mean we would be giving AMD a special treatment when there shouldn't be any, for anybody.
On Tue, Oct 28, 2014 at 08:38:56AM -0500, Felipe Balbi wrote: > Hi, > > almost there... > > On Tue, Oct 28, 2014 at 07:54:40PM +0800, Huang Rui wrote: > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > > index 8b94ad5..b08a2f9 100644 > > --- a/drivers/usb/dwc3/core.h > > +++ b/drivers/usb/dwc3/core.h > > @@ -699,6 +699,7 @@ struct dwc3_scratchpad_array { > > * @rx_detect_poll_quirk: set if we enable rx_detect to polling lfps quirk > > * @dis_u3_susphy_quirk: set if we disable usb3 suspend phy > > * @dis_u2_susphy_quirk: set if we disable usb2 suspend phy > > + * @amd_nl_plat: set if we use AMD NL platform > > however, as I mentioned before, the core shouldn't have to know that > it's running on an AMD platform. We already support several different > platforms (OMAP5, AM437x, DRA7xx, Exynos5, Exynos7, Qcom, Merrifield, > Baytrail, Braswell, HAPS PCIe, and STiH407) and none of them get their > $my_awesome_platform flag in dwc3, why should AMD be any different ? > > This is the only part of $subject that I cannot accept because it would > mean we would be giving AMD a special treatment when there shouldn't be > any, for anybody. > That's because I used this flag to enable below quirks on AMD NL FPGA board, and FPGA flag only can be detected on core. Can I set disable_scramble_quirk, dis_u3_susphy_quirk, and dis_u2_susphy_quirk for all the FPGA platforms? if (dwc->amd_nl_plat && dwc->is_fpga) { dwc->disable_scramble_quirk = true; dwc->dis_u3_susphy_quirk = true; dwc->dis_u2_susphy_quirk = true; } Thanks, Rui -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Felipe, Paul, On Tue, Oct 28, 2014 at 10:35:37PM +0800, Huang Rui wrote: > On Tue, Oct 28, 2014 at 08:38:56AM -0500, Felipe Balbi wrote: <snip> > > > > however, as I mentioned before, the core shouldn't have to know that > > it's running on an AMD platform. We already support several different > > platforms (OMAP5, AM437x, DRA7xx, Exynos5, Exynos7, Qcom, Merrifield, > > Baytrail, Braswell, HAPS PCIe, and STiH407) and none of them get their > > $my_awesome_platform flag in dwc3, why should AMD be any different ? > > > > This is the only part of $subject that I cannot accept because it would > > mean we would be giving AMD a special treatment when there shouldn't be > > any, for anybody. > > > > That's because I used this flag to enable below quirks on AMD NL FPGA > board, and FPGA flag only can be detected on core. Can I set > disable_scramble_quirk, dis_u3_susphy_quirk, and dis_u2_susphy_quirk > for all the FPGA platforms? > > if (dwc->amd_nl_plat && dwc->is_fpga) { > dwc->disable_scramble_quirk = true; > dwc->dis_u3_susphy_quirk = true; > dwc->dis_u2_susphy_quirk = true; > } > I confirmed with HW designer, these three quirks only will be needed on FPGA board. And these should *not* be used on non-FPGA board, as you known. So I would like to use below conditions on dwc3 core. When I set these quirk flags in pci glue layer, then core can filter them by is_fpga flag to support both on FPGA and SoC. Is there any concern? If that, I should remove WARN_ONCE at disable_scramble flag. if (dwc->disable_scramble_quirk && dwc->is_fpga) {..} if (dwc->dis_u2_susphy_quirk && dwc->is_fpga) {..} if (dwc->dis_u3_susphy_quirk && dwc->is_fpga) {..} Thanks, Rui -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 29, 2014 at 05:13:43PM +0800, Huang Rui wrote: > Hi Felipe, Paul, > > On Tue, Oct 28, 2014 at 10:35:37PM +0800, Huang Rui wrote: > > On Tue, Oct 28, 2014 at 08:38:56AM -0500, Felipe Balbi wrote: > > <snip> > > > > > > > however, as I mentioned before, the core shouldn't have to know that > > > it's running on an AMD platform. We already support several different > > > platforms (OMAP5, AM437x, DRA7xx, Exynos5, Exynos7, Qcom, Merrifield, > > > Baytrail, Braswell, HAPS PCIe, and STiH407) and none of them get their > > > $my_awesome_platform flag in dwc3, why should AMD be any different ? > > > > > > This is the only part of $subject that I cannot accept because it would > > > mean we would be giving AMD a special treatment when there shouldn't be > > > any, for anybody. > > > > > > > That's because I used this flag to enable below quirks on AMD NL FPGA > > board, and FPGA flag only can be detected on core. Can I set > > disable_scramble_quirk, dis_u3_susphy_quirk, and dis_u2_susphy_quirk > > for all the FPGA platforms? > > > > if (dwc->amd_nl_plat && dwc->is_fpga) { > > dwc->disable_scramble_quirk = true; > > dwc->dis_u3_susphy_quirk = true; > > dwc->dis_u2_susphy_quirk = true; > > } > > > > I confirmed with HW designer, these three quirks only will be needed > on FPGA board. And these should *not* be used on non-FPGA board, as you > known. > > So I would like to use below conditions on dwc3 core. When I set these > quirk flags in pci glue layer, then core can filter them by is_fpga > flag to support both on FPGA and SoC. Is there any concern? If that, I > should remove WARN_ONCE at disable_scramble flag. > > if (dwc->disable_scramble_quirk && dwc->is_fpga) {..} > > if (dwc->dis_u2_susphy_quirk && dwc->is_fpga) {..} > > if (dwc->dis_u3_susphy_quirk && dwc->is_fpga) {..} the problem is that somebody might need this on non-FPGA. Currently, only AMD needs these and only on FPGA, but you never know. I guess we can add it like this for now and once we have a real AMD product, we drop FPGA support from AMD. cheers
On Wed, Oct 29, 2014 at 09:11:46AM -0500, Felipe Balbi wrote: > On Wed, Oct 29, 2014 at 05:13:43PM +0800, Huang Rui wrote: > > Hi Felipe, Paul, > > > > On Tue, Oct 28, 2014 at 10:35:37PM +0800, Huang Rui wrote: > > > On Tue, Oct 28, 2014 at 08:38:56AM -0500, Felipe Balbi wrote: > > > > <snip> > > > > > > > > > > however, as I mentioned before, the core shouldn't have to know that > > > > it's running on an AMD platform. We already support several different > > > > platforms (OMAP5, AM437x, DRA7xx, Exynos5, Exynos7, Qcom, Merrifield, > > > > Baytrail, Braswell, HAPS PCIe, and STiH407) and none of them get their > > > > $my_awesome_platform flag in dwc3, why should AMD be any different ? > > > > > > > > This is the only part of $subject that I cannot accept because it would > > > > mean we would be giving AMD a special treatment when there shouldn't be > > > > any, for anybody. > > > > > > > > > > That's because I used this flag to enable below quirks on AMD NL FPGA > > > board, and FPGA flag only can be detected on core. Can I set > > > disable_scramble_quirk, dis_u3_susphy_quirk, and dis_u2_susphy_quirk > > > for all the FPGA platforms? > > > > > > if (dwc->amd_nl_plat && dwc->is_fpga) { > > > dwc->disable_scramble_quirk = true; > > > dwc->dis_u3_susphy_quirk = true; > > > dwc->dis_u2_susphy_quirk = true; > > > } > > > > > > > I confirmed with HW designer, these three quirks only will be needed > > on FPGA board. And these should *not* be used on non-FPGA board, as you > > known. > > > > So I would like to use below conditions on dwc3 core. When I set these > > quirk flags in pci glue layer, then core can filter them by is_fpga > > flag to support both on FPGA and SoC. Is there any concern? If that, I > > should remove WARN_ONCE at disable_scramble flag. > > > > if (dwc->disable_scramble_quirk && dwc->is_fpga) {..} > > > > if (dwc->dis_u2_susphy_quirk && dwc->is_fpga) {..} > > > > if (dwc->dis_u3_susphy_quirk && dwc->is_fpga) {..} > > the problem is that somebody might need this on non-FPGA. Currently, > only AMD needs these and only on FPGA, but you never know. > > I guess we can add it like this for now and once we have a real AMD > product, we drop FPGA support from AMD. > OK, agree. Then I comments below WARN_ONCE, OK? /* FIXME it should be used after AMD NL product taps out */ #if 0 WARN_ONCE(dwc->disable_scramble_quirk && !dwc->is_fpga, "disable_scramble cannot be used on non-FPGA builds\n"); #endif Thanks, Rui -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 29, 2014 at 10:33:19PM +0800, Huang Rui wrote: > On Wed, Oct 29, 2014 at 09:11:46AM -0500, Felipe Balbi wrote: > > On Wed, Oct 29, 2014 at 05:13:43PM +0800, Huang Rui wrote: > > > Hi Felipe, Paul, > > > > > > On Tue, Oct 28, 2014 at 10:35:37PM +0800, Huang Rui wrote: > > > > On Tue, Oct 28, 2014 at 08:38:56AM -0500, Felipe Balbi wrote: > > > > > > <snip> > > > > > > > > > > > > > however, as I mentioned before, the core shouldn't have to know that > > > > > it's running on an AMD platform. We already support several different > > > > > platforms (OMAP5, AM437x, DRA7xx, Exynos5, Exynos7, Qcom, Merrifield, > > > > > Baytrail, Braswell, HAPS PCIe, and STiH407) and none of them get their > > > > > $my_awesome_platform flag in dwc3, why should AMD be any different ? > > > > > > > > > > This is the only part of $subject that I cannot accept because it would > > > > > mean we would be giving AMD a special treatment when there shouldn't be > > > > > any, for anybody. > > > > > > > > > > > > > That's because I used this flag to enable below quirks on AMD NL FPGA > > > > board, and FPGA flag only can be detected on core. Can I set > > > > disable_scramble_quirk, dis_u3_susphy_quirk, and dis_u2_susphy_quirk > > > > for all the FPGA platforms? > > > > > > > > if (dwc->amd_nl_plat && dwc->is_fpga) { > > > > dwc->disable_scramble_quirk = true; > > > > dwc->dis_u3_susphy_quirk = true; > > > > dwc->dis_u2_susphy_quirk = true; > > > > } > > > > > > > > > > I confirmed with HW designer, these three quirks only will be needed > > > on FPGA board. And these should *not* be used on non-FPGA board, as you > > > known. > > > > > > So I would like to use below conditions on dwc3 core. When I set these > > > quirk flags in pci glue layer, then core can filter them by is_fpga > > > flag to support both on FPGA and SoC. Is there any concern? If that, I > > > should remove WARN_ONCE at disable_scramble flag. > > > > > > if (dwc->disable_scramble_quirk && dwc->is_fpga) {..} > > > > > > if (dwc->dis_u2_susphy_quirk && dwc->is_fpga) {..} > > > > > > if (dwc->dis_u3_susphy_quirk && dwc->is_fpga) {..} > > > > the problem is that somebody might need this on non-FPGA. Currently, > > only AMD needs these and only on FPGA, but you never know. > > > > I guess we can add it like this for now and once we have a real AMD > > product, we drop FPGA support from AMD. > > > > OK, agree. > > Then I comments below WARN_ONCE, OK? > > /* FIXME it should be used after AMD NL product taps out */ > #if 0 > WARN_ONCE(dwc->disable_scramble_quirk && !dwc->is_fpga, > "disable_scramble cannot be used on non-FPGA builds\n"); > #endif just remove it, we don't like commented out code.
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 33a8f3c..7ab867b 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -526,6 +526,12 @@ static int dwc3_core_init(struct dwc3 *dwc) dwc->is_fpga = true; } + if (dwc->amd_nl_plat && dwc->is_fpga) { + dwc->disable_scramble_quirk = true; + dwc->dis_u3_susphy_quirk = true; + dwc->dis_u2_susphy_quirk = true; + } + WARN_ONCE(dwc->disable_scramble_quirk && !dwc->is_fpga, "disable_scramble cannot be used on non-FPGA builds\n"); @@ -817,6 +823,8 @@ static int dwc3_probe(struct platform_device *pdev) "snps,dis_u3_susphy_quirk"); dwc->dis_u2_susphy_quirk = of_property_read_bool(node, "snps,dis_u2_susphy_quirk"); + dwc->amd_nl_plat = of_property_read_bool(node, + "snps,amd_nl_plat"); dwc->tx_deemph_quirk = of_property_read_bool(node, "snps,tx_deemph_quirk"); @@ -841,6 +849,7 @@ static int dwc3_probe(struct platform_device *pdev) dwc->rx_detect_poll_quirk = pdata->rx_detect_poll_quirk; dwc->dis_u3_susphy_quirk = pdata->dis_u3_susphy_quirk; dwc->dis_u2_susphy_quirk = pdata->dis_u2_susphy_quirk; + dwc->amd_nl_plat = pdata->amd_nl_plat; dwc->tx_deemph_quirk = pdata->tx_deemph_quirk; if (pdata->tx_deemph) diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 8b94ad5..b08a2f9 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -699,6 +699,7 @@ struct dwc3_scratchpad_array { * @rx_detect_poll_quirk: set if we enable rx_detect to polling lfps quirk * @dis_u3_susphy_quirk: set if we disable usb3 suspend phy * @dis_u2_susphy_quirk: set if we disable usb2 suspend phy + * @amd_nl_plat: set if we use AMD NL platform * @tx_deemph_quirk: set if we enable Tx deemphasis quirk * @tx_deemph: Tx deemphasis value * 0 - -6dB de-emphasis @@ -822,6 +823,7 @@ struct dwc3 { unsigned rx_detect_poll_quirk:1; unsigned dis_u3_susphy_quirk:1; unsigned dis_u2_susphy_quirk:1; + unsigned amd_nl_plat:1; unsigned tx_deemph_quirk:1; unsigned tx_deemph:2; diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c index ada975f..3af9b47 100644 --- a/drivers/usb/dwc3/dwc3-pci.c +++ b/drivers/usb/dwc3/dwc3-pci.c @@ -145,6 +145,24 @@ static int dwc3_pci_probe(struct pci_dev *pci, res[1].name = "dwc_usb3"; res[1].flags = IORESOURCE_IRQ; + if (pci->vendor == PCI_VENDOR_ID_AMD && + pci->device == PCI_DEVICE_ID_AMD_NL_USB) { + dwc3_pdata.has_lpm_erratum = true; + dwc3_pdata.lpm_nyet_thres = 0xf; + + dwc3_pdata.u2exit_lfps_quirk = true; + dwc3_pdata.u2ss_inp3_quirk = true; + dwc3_pdata.req_p1p2p3_quirk = true; + dwc3_pdata.del_p1p2p3_quirk = true; + dwc3_pdata.del_phy_power_chg_quirk = true; + dwc3_pdata.lfps_filter_quirk = true; + dwc3_pdata.rx_detect_poll_quirk = true; + dwc3_pdata.amd_nl_plat = true; + + dwc3_pdata.tx_deemph_quirk = true; + dwc3_pdata.tx_deemph = 1; + } + ret = platform_device_add_resources(dwc3, res, ARRAY_SIZE(res)); if (ret) { dev_err(dev, "couldn't add resources to dwc3 device\n"); @@ -194,6 +212,7 @@ static const struct pci_device_id dwc3_pci_id_table[] = { { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BSW), }, { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT), }, { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRFLD), }, + { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_NL_USB), }, { } /* Terminating Entry */ }; MODULE_DEVICE_TABLE(pci, dwc3_pci_id_table); diff --git a/drivers/usb/dwc3/platform_data.h b/drivers/usb/dwc3/platform_data.h index 1cfd384..c2c3511 100644 --- a/drivers/usb/dwc3/platform_data.h +++ b/drivers/usb/dwc3/platform_data.h @@ -37,6 +37,7 @@ struct dwc3_platform_data { unsigned rx_detect_poll_quirk:1; unsigned dis_u3_susphy_quirk:1; unsigned dis_u2_susphy_quirk:1; + unsigned amd_nl_plat:1; unsigned tx_deemph_quirk:1; unsigned tx_deemph:2;
This patch adds support for AMD Nolan (NL) FPGA and SoC platform. Cc: Jason Chang <jason.chang@amd.com> Signed-off-by: Huang Rui <ray.huang@amd.com> --- drivers/usb/dwc3/core.c | 9 +++++++++ drivers/usb/dwc3/core.h | 2 ++ drivers/usb/dwc3/dwc3-pci.c | 19 +++++++++++++++++++ drivers/usb/dwc3/platform_data.h | 1 + 4 files changed, 31 insertions(+)