| Message ID | 1625043642-29822-5-git-send-email-wcheng@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Re-introduce TX FIFO resize for larger EP bursting | expand |
Hi Wesley,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on usb/usb-testing]
[also build test ERROR on robh/for-next v5.13 next-20210630]
[cannot apply to balbi-usb/testing/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Wesley-Cheng/Re-introduce-TX-FIFO-resize-for-larger-EP-bursting/20210630-170142
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: arm64-randconfig-r015-20210630 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 8d21d5472501460933e78aead04cf59579025ba4)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/253494fa42973a9e2cfaa4a9e8735126c2da6d74
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Wesley-Cheng/Re-introduce-TX-FIFO-resize-for-larger-EP-bursting/20210630-170142
git checkout 253494fa42973a9e2cfaa4a9e8735126c2da6d74
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "of_add_property" [drivers/usb/dwc3/dwc3-qcom.ko] undefined!
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Wed, Jun 30, 2021 at 02:00:41AM -0700, Wesley Cheng wrote: > In order to take advantage of the TX fifo resizing logic, manually add > these properties to the DWC3 child node by default. This will allow > the DWC3 gadget to resize the TX fifos for the IN endpoints, which > help with performance. > > Signed-off-by: Wesley Cheng <wcheng@codeaurora.org> > --- > drivers/usb/dwc3/dwc3-qcom.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c > index 49e6ca9..cec4f4a 100644 > --- a/drivers/usb/dwc3/dwc3-qcom.c > +++ b/drivers/usb/dwc3/dwc3-qcom.c > @@ -640,6 +640,25 @@ static int dwc3_qcom_acpi_register_core(struct platform_device *pdev) > return ret; > } > > +#ifdef CONFIG_OF > +static void dwc3_qcom_add_dt_props(struct device *dev, struct device_node *np) > +{ > + struct property *prop; > + int ret; > + > + prop = devm_kzalloc(dev, sizeof(*prop), GFP_KERNEL); > + if (prop) { > + prop->name = "tx-fifo-resize"; > + ret = of_add_property(np, prop); > + if (ret < 0) > + dev_info(dev, "unable to add tx-fifo-resize prop\n"); Is that really an "informational" error? :( > + } So if you can not allocate memory, you just fail quietly? Are you sure that is ok? Please properly handle errors. greg k-h
On 7/1/2021 10:06 PM, Greg KH wrote: > On Wed, Jun 30, 2021 at 02:00:41AM -0700, Wesley Cheng wrote: >> In order to take advantage of the TX fifo resizing logic, manually add >> these properties to the DWC3 child node by default. This will allow >> the DWC3 gadget to resize the TX fifos for the IN endpoints, which >> help with performance. >> >> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org> >> --- >> drivers/usb/dwc3/dwc3-qcom.c | 21 +++++++++++++++++++++ >> 1 file changed, 21 insertions(+) >> >> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c >> index 49e6ca9..cec4f4a 100644 >> --- a/drivers/usb/dwc3/dwc3-qcom.c >> +++ b/drivers/usb/dwc3/dwc3-qcom.c >> @@ -640,6 +640,25 @@ static int dwc3_qcom_acpi_register_core(struct platform_device *pdev) >> return ret; >> } >> >> +#ifdef CONFIG_OF >> +static void dwc3_qcom_add_dt_props(struct device *dev, struct device_node *np) >> +{ >> + struct property *prop; >> + int ret; >> + >> + prop = devm_kzalloc(dev, sizeof(*prop), GFP_KERNEL); >> + if (prop) { >> + prop->name = "tx-fifo-resize"; >> + ret = of_add_property(np, prop); >> + if (ret < 0) >> + dev_info(dev, "unable to add tx-fifo-resize prop\n"); Hi Greg, > > Is that really an "informational" error? :( > > We can remove it. If anything we can always check the sysfs if the property is present or not. >> + } > > So if you can not allocate memory, you just fail quietly? Are you sure > that is ok? > > Please properly handle errors. > OK, will handle the case where we can't allocate memory for the property. Thanks Wesley Cheng
On Fri, Jul 02, 2021 at 01:10:13AM -0700, Wesley Cheng wrote: > > > On 7/1/2021 10:06 PM, Greg KH wrote: > > On Wed, Jun 30, 2021 at 02:00:41AM -0700, Wesley Cheng wrote: > >> In order to take advantage of the TX fifo resizing logic, manually add > >> these properties to the DWC3 child node by default. This will allow > >> the DWC3 gadget to resize the TX fifos for the IN endpoints, which > >> help with performance. > >> > >> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org> > >> --- > >> drivers/usb/dwc3/dwc3-qcom.c | 21 +++++++++++++++++++++ > >> 1 file changed, 21 insertions(+) > >> > >> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c > >> index 49e6ca9..cec4f4a 100644 > >> --- a/drivers/usb/dwc3/dwc3-qcom.c > >> +++ b/drivers/usb/dwc3/dwc3-qcom.c > >> @@ -640,6 +640,25 @@ static int dwc3_qcom_acpi_register_core(struct platform_device *pdev) > >> return ret; > >> } > >> > >> +#ifdef CONFIG_OF > >> +static void dwc3_qcom_add_dt_props(struct device *dev, struct device_node *np) > >> +{ > >> + struct property *prop; > >> + int ret; > >> + > >> + prop = devm_kzalloc(dev, sizeof(*prop), GFP_KERNEL); > >> + if (prop) { > >> + prop->name = "tx-fifo-resize"; > >> + ret = of_add_property(np, prop); > >> + if (ret < 0) > >> + dev_info(dev, "unable to add tx-fifo-resize prop\n"); > > Hi Greg, > > > > Is that really an "informational" error? :( > > > > > We can remove it. If anything we can always check the sysfs if the > property is present or not. Sorry for being vague. That message should be dev_err(), and not dev_info(), right? > >> + } > > > > So if you can not allocate memory, you just fail quietly? Are you sure > > that is ok? > > > > Please properly handle errors. > > > OK, will handle the case where we can't allocate memory for the property. And the case for when of_add_property() fails. thanks, greg k-h
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c index 49e6ca9..cec4f4a 100644 --- a/drivers/usb/dwc3/dwc3-qcom.c +++ b/drivers/usb/dwc3/dwc3-qcom.c @@ -640,6 +640,25 @@ static int dwc3_qcom_acpi_register_core(struct platform_device *pdev) return ret; } +#ifdef CONFIG_OF +static void dwc3_qcom_add_dt_props(struct device *dev, struct device_node *np) +{ + struct property *prop; + int ret; + + prop = devm_kzalloc(dev, sizeof(*prop), GFP_KERNEL); + if (prop) { + prop->name = "tx-fifo-resize"; + ret = of_add_property(np, prop); + if (ret < 0) + dev_info(dev, "unable to add tx-fifo-resize prop\n"); + } +} +#else +static void dwc3_qcom_add_dt_props(struct device *dev, struct device_node *np) +{ } +#endif + static int dwc3_qcom_of_register_core(struct platform_device *pdev) { struct dwc3_qcom *qcom = platform_get_drvdata(pdev); @@ -653,6 +672,8 @@ static int dwc3_qcom_of_register_core(struct platform_device *pdev) return -ENODEV; } + dwc3_qcom_add_dt_props(dev, dwc3_np); + ret = of_platform_populate(np, NULL, NULL, dev); if (ret) { dev_err(dev, "failed to register dwc3 core - %d\n", ret);
In order to take advantage of the TX fifo resizing logic, manually add these properties to the DWC3 child node by default. This will allow the DWC3 gadget to resize the TX fifos for the IN endpoints, which help with performance. Signed-off-by: Wesley Cheng <wcheng@codeaurora.org> --- drivers/usb/dwc3/dwc3-qcom.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)