On Fri, Oct 28, 2022 at 12:37:15PM +0200, gregkh@xxxxxxxxxxxxxxxxxxx wrote: > On Fri, Oct 28, 2022 at 11:50:47AM +0200, Thierry Reding wrote: > > On Fri, Oct 28, 2022 at 08:04:54AM +0200, gregkh@xxxxxxxxxxxxxxxxxxx wrote: > > > On Fri, Oct 28, 2022 at 05:36:41AM +0000, Jim Lin wrote: > > > > On Thu, 2022-10-27 at 15:58 +0200, Greg KH wrote: > > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > > > > On Thu, Oct 27, 2022 at 09:31:25PM +0800, Jim Lin wrote: > > > > > > Add EXPORT_SYMBOL_GPL on xhci_hub_control() for other driver module > > > > > > to invoke and avoid linking error. > > > > > > > > > > What other driver module? > > > > > > > > > > There is no user here :( > > > > > > > > > > confused, > > > > > > > > > > > > > In arch/arm/configs/multi_v7_defconfig > > > > It defines > > > > CONFIG_USB_XHCI_TEGRA=m > > > > > > > > If I don't add EXPORT_SYMBOL_GPL on xhci_hub_control() > > > > , I will get compile/linking error like > > > > > > > > ERROR: modpost: "xhci_hub_control" [drivers/usb/host/xhci-tegra.ko] > > > > undefined! > > > > > > > > if patch > > > > "[PATCH v5,3/3] xhci: tegra: USB2 pad power controls" > > > > > > > > https://patchwork.kernel.org/project/linux-usb/patch/20221027133127.27592-4-jilin@xxxxxxxxxx/ > > > > is added in xhci-tegra.c to invoke xhci_hub_control() > > > > > > > > Should I integrate this patch with [PATCH v5,3/3] as one? > > > > > > Yes, do not add something that is not needed for that specific commit, > > > otherwise it causes reviewers to be confused. > > > > Other subsystem maintainers prefer core changes to be split from driver > > changes, so this type of split is commonly encountered elsewhere. > > > > Obviously, since this is your turf you get to make the rules. I'm just > > trying to say that this kind of advice can be confusing for contributors > > because when they then sent driver and code changes mixed for their next > > submission, the subsystem maintainer might tell them otherwise. > > Sure, but if you do split it up like this, DOCUMENT WHY THE EXPORT IS > NEEDED. That didn't happen here so I had no idea why this was even an > issue. > > And yes, I am very sensitive to this, we have had LOTS of people trying > to export xhci symbols in the past few years for no in-kernel users, > despite us constantly telling them that this is not allowed. It > happened again, just yesterday: > https://lore.kernel.org/r/20221027004050.4192111-1-albertccwang@xxxxxxxxxx > > And at first glance, I assumed this was much the same as there was no > description of why this was needed at all. Agreed. I suppose this could've been spelled out more explicitly in the cover letter or in patch 1. Jim, please make sure to describe this dependency explicitly in v6. Thierry
Attachment:
signature.asc
Description: PGP signature
