On 11/18/2016 12:18 PM, Christian Lamparter wrote: > On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote: >> On Thu, Nov 17, 2016 at 04:35:10PM +0100, Stefan Wahren wrote: >>> Hi John, >>> >>> Am 17.11.2016 um 00:47 schrieb John Youn: >>>> Add the "snps,ahb-burst" binding and read it in. >>>> >>>> This property controls which burst type to perform on the AHB bus as a >>>> master in internal DMA mode. This overrides the legacy param value, >>>> which we need to keep around for now since several platforms use it. >>>> >>>> Some platforms may see better or worse performance based on this >>>> value. The HAPS platform is one example where all INCRx have worse >>>> performance than INCR. >>>> >>>> Other platforms (such as the Canyonlands board) report that the default >>>> value causes system hangs. >>>> >>>> Signed-off-by: John Youn <[email protected]> >>>> Cc: Christian Lamparter <[email protected]> >>>> --- >>>> Documentation/devicetree/bindings/usb/dwc2.txt | 2 + >>>> drivers/usb/dwc2/core.h | 9 +++++ >>>> drivers/usb/dwc2/params.c | 56 >>>> ++++++++++++++++++++++++++ >>>> 3 files changed, 67 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt >>>> b/Documentation/devicetree/bindings/usb/dwc2.txt >>>> index 6c7c2bce..9e7b4b4 100644 >>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt >>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt >>> >>> according to Documentation/devicetree/bindings/submitting-patches.txt >>> this change should be a separate patch. >>> >>>> @@ -26,6 +26,8 @@ Optional properties: >>>> Refer to phy/phy-bindings.txt for generic phy consumer properties >>>> - dr_mode: shall be one of "host", "peripheral" and "otg" >>>> Refer to usb/generic.txt >>>> +- snps,ahb-burst: specifies the ahb burst length. Valid arguments are: >>>> + "SINGLE", "INCR", "INCR4", "INCR8", "INCR16". Defaults to "INCR4". >>> >>> This doesn't apply in case of the bcm2835. I would prefer this option is >>> ignored in that case with a dev_warn("snps,ahb-burst is not supported on >>> this platform"). >> >> Also, perhaps you should allow that the compatible string can define the >> default. >> > I hoped you would say that :). > > I've attached a patch (on top of John Youn changes) that does > just that for the amcc,dwc-otg. I put the GAHBCFG_HBSTLEN_INCR > value into the .data, if that's a problem, I can certainly > respin the patch and put it in a dedicated struct. > > Regards > > Christian > --- > From 4c31a029dde714828810b1c3e61a5b1412ac939a Mon Sep 17 00:00:00 2001 > From: Christian Lamparter <[email protected]> > Date: Fri, 18 Nov 2016 21:03:19 +0100 > Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg > > This patch adds a of_device_id table which can be used by > existing devices to supply a ahb-burst value for the platform > without having to add a "snps,ahb-burst" entry to the dts. > > Note: Adding new devices to this table is discouraged. > please consider adding the "snps,ahb-burst" property > with the correct configuration to your device tree > file instead. > > Signed-off-by: Christian Lamparter <[email protected]> > --- > drivers/usb/dwc2/params.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c > index e0fc9aa..51be266 100644 > --- a/drivers/usb/dwc2/params.c > +++ b/drivers/usb/dwc2/params.c > @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = { > [GAHBCFG_HBSTLEN_INCR16] = "INCR16", > }; > > +/* > + * This table provides AHB burst configuration for existing > + * device tree bindings that work poorly with the default setting. > + * > + * Note: Adding new devices to this table is discouraged. > + * please consider adding the "snps,ahb-burst" property > + * with the correct configuration to your device tree > + * file instead. > + */ > +static const struct of_device_id dwc2_compat_ahb_bursts[] = { > + { > + .compatible = "amcc,dwc-otg", > + .data = (void *) GAHBCFG_HBSTLEN_INCR16, > + }, > +}; > + > static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg) > { > struct device_node *node = hsotg->dev->of_node; > @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct > dwc2_hsotg *hsotg) > ret = device_property_read_string(hsotg->dev, > "snps,ahb-burst", &str); > if (ret < 0) { > + const struct of_device_id *match; > + > + match = of_match_node(dwc2_compat_ahb_bursts, node); > + if (match) > + ret = (int)match->data; > + > return ret; > } else if (of_device_is_compatible(node, "brcm,bcm2835-usb")) { > dev_warn(hsotg->dev, >
Hi Christian, I'd prefer if you use the binding which requires no extra code in dwc2. Regards, John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
