On 4/28/2021 1:53 PM, Bjorn Helgaas wrote: > [+cc Amjad, Philipp: possible issue with 557acb3d2cd9 ("reset: make > shared pulsed reset controls re-triggerable") below; report at > https://lore.kernel.org/linux-ide/20210428200058.GA366202@bjorn-Precision-5520/] > > On Wed, Apr 28, 2021 at 04:34:00PM -0400, Jim Quinlan wrote: >> On Wed, Apr 28, 2021 at 4:01 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: >>> On Fri, Mar 12, 2021 at 03:45:53PM -0500, Jim Quinlan wrote: >>>> v5 -- Improved (I hope) commit description (Bjorn). >>>> -- Rnamed error labels (Krzyszt). >>>> -- Fixed typos. >>>> >>>> v4 -- does not rely on a pending commit, unlike v3. >>>> >>>> v3 -- discard commit from v2; instead rely on the new function >>>> reset_control_rearm provided in a recent commit [1] applied >>>> to reset/next. >>>> -- New commit to correct pcie-brcmstb.c usage of a reset controller >>>> to use reset/rearm verses deassert/assert. >>>> >>>> v2 -- refactor rescal-reset driver to implement assert/deassert rather than >>>> reset because the reset call only fires once per lifetime and we need >>>> to reset after every resume from S2 or S3. >>>> -- Split the use of "ahci" and "rescal" controllers in separate fields >>>> to keep things simple. >>>> >>>> v1 -- original >>>> >>>> Jim Quinlan (2): >>>> ata: ahci_brcm: Fix use of BCM7216 reset controller >>>> PCI: brcmstb: Use reset/rearm instead of deassert/assert >>>> >>>> drivers/ata/ahci_brcm.c | 46 +++++++++++++-------------- >>>> drivers/pci/controller/pcie-brcmstb.c | 19 +++++++---- >>>> 2 files changed, 36 insertions(+), 29 deletions(-) >>> >>> Tripped over these errors while build testing with the .config below. >>> This is on the pci/brcmstb branch from >>> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git >>> >>> Dropping the pci/brcmstb branch while we get this figured out. This will >>> remove the following commits: >>> >>> a24fd1d6469f ("PCI: brcmstb: Use reset/rearm instead of deassert/assert") >>> 92b9cb55a9b6 ("ata: ahci_brcm: Fix use of BCM7216 reset controller") >>> b5d9209d5083 ("PCI: brcmstb: Fix error return code in brcm_pcie_probe()") >> >> Hi Bjorn, >> >> I believe the problem is that the commit >> >> 557acb3d2cd9c82de19f944f6cc967a347735385 >> "reset: make shared pulsed reset controls re-triggerable" >> >> defined reset_control_rearm() for the CONFIG_RESET_CONTROLLER=y case >> but forgot to define an empty function for the unset case. Your test >> .config has this CONFIG unset. >> >> Would you like me to resubmit this with an additional commit that >> fixes this? > > The fix could be a patch along those lines, or it could be a Kconfig > change that makes this config impossible. I didn't look deeper to see > what makes sense. But I don't think the fix should be "manually avoid > this configuration." > > It looks like 557acb3d2cd9 ("reset: make shared pulsed reset controls > re-triggerable") appeared in v5.11, so if a patch is the right thing, > it should probably be marked for stable ("v5.11+"). All of the other reset controller API have inline stubs when CONFIG_RESET_CONTROLLER is disabled, not having one for reset_control_rearm() seems like an oversight (easy to make since if you introduce the API you obviously needed it and did not consider the case where it may not be available). I agree this would be stable material. -- Florian