* Re: [PATCH 00/13] treewide: replace linux/gpio.h
From: Bartosz Golaszewski @ 2026-06-30 7:53 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-gpio, Arnd Bergmann, Bartosz Golaszewski, Andrew Lunn,
Sebastian Hesselbarth, Gregory Clement, Frank Li, Robert Jarzmik,
Krzysztof Kozlowski, Greg Ungerer, Thomas Bogendoerfer,
Hauke Mehrtens, Rafał Miłecki, Yoshinori Sato,
John Paul Adrian Glaubitz, Linus Walleij, Dmitry Torokhov,
Jakub Kicinski, Paolo Abeni, Dominik Brodowski, linux-kernel,
linux-arm-kernel, linux-samsung-soc, patches, linux-m68k,
linux-mips, linux-sh, linux-input, linux-media, netdev,
linux-sunxi, linux-phy, linux-rockchip, linux-sound
In-Reply-To: <20260629132633.1300009-1-arnd@kernel.org>
On Mon, 29 Jun 2026 15:26:20 +0200, Arnd Bergmann <arnd@kernel.org> said:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The linux/gpio.h header used to be the global definition for the gpio
> interfaces, with 1100 users back in linux-3.17. In linux-7.2, only about
> 130 of those remain, so this series cleans out the rest.
>
> In each subsystem, we can replace the header either with
> linux/gpio/consumer.h for users of the modern gpio descriptor interface,
> or linux/gpio/legacy.h for the few remaining users of the old number
> based interface.
>
> All patches in this series can get applied independently, so my
> preference would be for each subsystem maintainer to apply these
> directly, with the rest going into the gpio tree at some point.
>
> The final patch here obviously needs to wait for all the others
> to get merged first.
>
> Arnd
Thanks for doing this Arnd!
For the series:
Acked-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH v14 0/7] Provide support for Trigger Generation Unit
From: Songwei.Chai @ 2026-06-30 8:01 UTC (permalink / raw)
To: Suzuki K Poulose, Greg KH, Jie Gan
Cc: andersson, alexander.shishkin, mike.leach, konrad.dybcio,
james.clark, krzk+dt, conor+dt, linux-kernel, linux-arm-kernel,
linux-arm-msm, coresight, devicetree
In-Reply-To: <ce9a2121-8f7e-4ac2-8795-5ee602966e74@arm.com>
On 6/29/2026 6:44 PM, Suzuki K Poulose wrote:
> Hello,
>
> On 29/06/2026 11:17, Songwei.Chai wrote:
>>
>> On 6/29/2026 12:22 PM, Greg KH wrote:
>>> On Mon, Jun 29, 2026 at 11:03:33AM +0800, Songwei.Chai wrote:
>>>> Hi Greg & Alexander,
>>>>
>>>> Apologies for interrupting again.
>>>>
>>>> As the TGU hardware plays an important role in Qualcomm tracing
>>>> design, I
>>>> would greatly appreciate it if you could kindly take some time to
>>>> review
>>>> this at your earliest convenience.
>>> The merge window _just_ closed, please give us a chance to catch up.
>>>
>>> Also, why us? Surely you have other reviewers for this code, right?
>>
>> Hi Greg,
>>
>> Understood, thanks for letting us know.
>>
>> Regarding your question: since this introduces a new
>> drivers/hwtracing/ qcom directory, there is no existing maintainer
>> for it.
>> Given your scope (and Alexander's), we believe you are the most
>> relevant reviewers.
>>
>> The reason for creating the qcom directory is as follows:
>>
>> /We previously tried to upstream this driver under drivers/hwtracing/
>> coresight,/
>> /but it was not accepted as it is considered Qualcomm-specific and
>> not tightly/
>> /coupled with the CoreSight subsystem. Based on this feedback, we are
>
> Some clarification here: This device is not CoreSight so we denied
> keeping this under drivers/hwtracing/coresight/ - Not because it is
> Qualcomm specific. We have TPDM, TPDA, TnoC devices under the coresight
> subsystem, which are all Qualcomm specific for e.g.
>
> That said, there are other drivers in drivers/hwtracing/ which I usually
> merge and push to Greg, after some reviews/acks from the respective
> people (e.g., PTT HiSilicon PCIe Tune and Trace).
>
> But, your proposal was that there were other maintainers for your new
> subtree and you were going to push this via ,linux-arm-msm ? to which I
> didn't have any objections.
>
> That said, I am fine with pushing this to Greg via the CoreSight pull
> requests (similar to Hisilicon PTT driver), but would need someone to
> Maintain/Review the driver (with entries in MAINTAINERS, similar to
> PTT).
>
>
> Thoughts ?
Hi Suzuki,
Thank you for your constructive feedback in helping us move this patch
forward.
As the owner of this driver, together with Jie Gan (who has extensive
review experience), we will be responsible for the maintenance and
review going forward.
The MAINTAINERS update will be included in the next TGU release.
Feel free to share any additional comments.
Thanks,
Songwei
>
> Kind regards
> Suzuki
>
>
>
>> exploring/
>> /a dedicated drivers/hwtracing/qcom directory, similar to intel_th,
>> to better/
>> /support this and future Qualcomm hwtracing drivers./
>>
>> More details can be found in “[PATCH v14 0/7] -- Why we are proposing
>> this”.
>>
>> Thanks,
>> Songwei
>>
>>>
>>> thanks,
>>>
>>> greg k-h
>
^ permalink raw reply
* Re: [PATCH v7 3/4] PCI: tegra: Add Tegra264 support
From: Manivannan Sadhasivam @ 2026-06-30 8:03 UTC (permalink / raw)
To: Thierry Reding
Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thierry Reding,
Jonathan Hunter, Karthikeyan Mitran, Hou Zhiqiang,
Thomas Petazzoni, Pali Rohár, Michal Simek, Kevin Xie,
Aksh Garg, linux-pci, devicetree, linux-tegra, linux-kernel,
linux-arm-kernel, Thierry Reding, Manikanta Maddireddy
In-Reply-To: <aju3jokcWR5DzPrv@orome>
On Wed, Jun 24, 2026 at 02:35:04PM +0200, Thierry Reding wrote:
[...]
> > So not hotplug support? Also, you do not want the driver to error out? I'm
> > wondering what's the use then?
>
> Hotplug is supported via pciehp. We skip probing the host bridge if no
> link was detected because there's simply nothing attached to the port,
> otherwise the link would've come up.
>
> pcie->link_up is slightly misleading because it actually means something
> along the lines of "link could be up at some point", either during probe
> or after some hotplug event later on. It is only ever false if there's
> no link during probe and hotplug isn't supported at all.
>
Ok. But if you skip pci_host_probe() then Root Port won't be enumerated at all.
I think that would give a false indication to the user. Typically, Root Port
would get enumerated during probe even if there are no devices atttached and
once the device gets attached, pciehp will enumerate the device.
> > > +
> > > + err = pci_host_probe(bridge);
> > > + if (err < 0) {
> > > + dev_err_probe(dev, err, "failed to register host\n");
> > > + goto free_ecam;
> > > + }
> > > +
> > > + return 0;
> > > +
> > > +free_ecam:
> >
> > Nit: Prefix 'err' for the labels.
>
> I don't see any benefit of adding a prefix. Seems pretty redundant, but
> I also don't feel too strongly about it, so I can add it.
>
People tend to use goto labels to do skip some steps also. So if the error
conditions are prefixed with 'err_' it helps to differentiate between them.
> > > + pci_ecam_free(pcie->cfg);
> > > +put_pm:
> > > + pm_runtime_put_sync(dev);
> > > +put_bpmp:
> > > + tegra_bpmp_put(pcie->bpmp);
> > > +
> > > + return err;
> > > +}
> > > +
> > > +static void tegra264_pcie_remove(struct platform_device *pdev)
> > > +{
> > > + struct tegra264_pcie *pcie = platform_get_drvdata(pdev);
> > > +
> > > + /*
> > > + * If we undo tegra264_pcie_init() then link goes down and need
> > > + * controller reset to bring up the link again. Remove intention is
> > > + * to clean up the root bridge and re-enumerate during bind.
> >
> > But the controller will be consuming power even if PCIe is not used. Do you
> > really want that? Can't tegra264_pcie_init() handle the initialization? I'm
> > wondering how tegra264_pcie_deinit() in tegra264_pcie_suspend() works then.
>
> I had to clarify this with the PCI team and they indicated that
> tegra264_pcie_deinit() is actually useless and maybe even harmful. The
> reason is that there's a processor on these boards (BPMP) that takes
> care of power sequencing and it will automatically take the PCI links
> to L2 on suspend and assert PERST#.
>
Then why are you calling tegra264_pcie_deinit() in tegra264_pcie_suspend()? If
tegra264_pcie_deinit() is harmful, then calling it during suspend should also
be, right?
Or tegra264_pcie_deinit() has to be paired with BPMP doing its own power
sequencing?
Not a big deal, but it just feels weird to see suspend() and remove() doing
different things.
> Another reason why we don't want to reset the entire controller is that
> it is already set up during early boot by UEFI and the kernel driver
> does not redo the entire initialization.
>
So tegra264_pcie_init() is not the full initialization? If so, is it sufficient
during resume()?
> So yes, I think a little bit of power consumption is the compromise that
> we will have to live with. In the bigger picture it's probably not going
> to be noticeable in most cases, and given that these are embedded
> platforms we'll likely see fixed configurations most of the time and the
> case where we remove the PCIe host controller will not be common.
>
Fair enough.
> > > + */
> > > + pci_lock_rescan_remove();
> > > + pci_stop_root_bus(pcie->bridge->bus);
> > > + pci_remove_root_bus(pcie->bridge->bus);
> > > + pci_unlock_rescan_remove();
> > > +
> > > + pm_runtime_put_sync(&pdev->dev);
> > > + tegra_bpmp_put(pcie->bpmp);
> > > + pci_ecam_free(pcie->cfg);
> > > +}
> > > +
> > > +static int tegra264_pcie_suspend(struct device *dev)
> > > +{
> > > + struct tegra264_pcie *pcie = dev_get_drvdata(dev);
> > > + int err;
> > > +
> > > + tegra264_pcie_deinit(pcie);
> > > +
> > > + if (pcie->wake_gpio && device_may_wakeup(dev)) {
> > > + err = enable_irq_wake(pcie->wake_irq);
> > > + if (err < 0)
> > > + dev_err(dev, "failed to enable wake IRQ: %pe\n",
> > > + ERR_PTR(err));
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int tegra264_pcie_resume(struct device *dev)
> > > +{
> > > + struct tegra264_pcie *pcie = dev_get_drvdata(dev);
> > > + int err;
> > > +
> > > + err = pinctrl_pm_select_default_state(dev);
> > > + if (err < 0)
> > > + dev_err(dev, "failed to configure sideband pins: %pe\n",
> > > + ERR_PTR(err));
> >
> > Please remind me if you justified this manual pinctrl handling before.
>
> This is just regular pinctrl PM boilerplate. There's plenty of other
> drivers where we do this, too. We want this because some of the pins
> get configured to non-default states on boot/resume, so doing this
> here ensures they are muxed correctly.
>
But pinctrl core should already be doing it for you, no?
> > > +
> > > + if (pcie->wake_gpio && device_may_wakeup(dev)) {
> > > + err = disable_irq_wake(pcie->wake_irq);
> > > + if (err < 0)
> > > + dev_err(dev, "failed to disable wake IRQ: %pe\n",
> > > + ERR_PTR(err));
> > > + }
> > > +
> > > + if (pcie->link_up == false)
> > > + return 0;
> >
> > How is this possible? If 'pcie->link_up' was 'false' during probe(), then it is
> > going to stay until tegra264_pcie_init() is called below.
>
> Yes, this keeps confusing me, too. The purpose of this is to skip
> initialization if we've already determined during probe that there is
> never going to be a link.
But you are calling tegra264_pcie_deinit() during tegra264_pcie_suspend()
unconditionally. So even if 'pcie->link_up' was false, the controller needs to
be initialized (atleast partially), right? Because, you are calling
tegra264_pcie_init() during probe() before 'pcie->link_up' check.
> link_up will be false if and only if there was
> no link during probe and we don't expect there ever will be a link
> because there is no hotplug support.
>
But above you said that this controller supports hotplug. Which one of the
statement is true?
> Maybe a different name for link_up could help here? maybe_link_up
> perhaps? I don't know if that's any clearer, but I also couldn't come up
> with a better name.
>
> Or maybe we should split this into two booleans, since we're essentially
> trying to use one boolean to track a tristate. What we want to know is
> if a link is truly up and if the controller should be kept powered for
> the case where hotplug is supported.
>
> I suppose we could do:
>
> bool link_up; /* track the link state */
> bool supports_hotplug; /* track whether port is hotpluggable */
>
Based on what criteria you'll set 'supports_hotplug' flag?
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* RE: [PATCH v2 1/4] dt-bindings: connector: Add fsl,aud-io-slot binding
From: Chancel Liu (OSS) @ 2026-06-30 8:06 UTC (permalink / raw)
To: Rob Herring, Chancel Liu (OSS)
Cc: krzk+dt@kernel.org, conor+dt@kernel.org, Frank Li,
s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20260629133253.GA2593312-robh@kernel.org>
> > From: Chancel Liu <chancel.liu@nxp.com>
> >
> > The NXP AUD-IO slot represents a physically present I/O connector on
> > the base board. It acts as a nexus that exposes a constrained set of
> > I/O resources, such as GPIOs, clocks and interrupts, through fixed
> > electrical wiring. All actual hardware providers reside on the base
> > board. The connector node only defines index-based mappings to those
> > providers.
> >
> > This connector type is present on i.MX95 19x19 EVK and i.MX952 EVK,
> > where it is used to attach the IMX-AUD-IO audio expansion card[1]. The
> > same add-on board can be reused across different base boards that
> > carry this connector.
> >
> > [1]https://www.nxp.com/part/IMX-AUD-IO
> >
> > Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
> > ---
> > .../bindings/connector/fsl,aud-io-slot.yaml | 113 ++++++++++++++++++
> > 1 file changed, 113 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/connector/fsl,aud-io-slot.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/connector/fsl,aud-io-slot.yaml
> > b/Documentation/devicetree/bindings/connector/fsl,aud-io-slot.yaml
> > new file mode 100644
> > index 000000000000..5085574d221b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/connector/fsl,aud-io-slot.yaml
> > @@ -0,0 +1,113 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/connector/fsl,aud-io-slot.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP AUD-IO Slot
> > +
> > +maintainers:
> > + - Frank Li <Frank.li@nxp.com>
> > + - Chancel Liu <chancel.liu@nxp.com>
> > +
> > +description:
> > + The NXP AUD-IO slot represents a physically present I/O connector
> > +on
> > + the base board. It acts as a nexus that exposes a constrained set
> > +of
> > + I/O resources, such as GPIOs, clocks and interrupts, through fixed
> > + electrical wiring. All actual hardware providers reside on the base
> > + board. The connector node only defines index-based mappings to
> > +those
> > + providers. This connector type is present on i.MX95 19x19 EVK and
> > + i.MX952 EVK, where it is used to attach the IMX-AUD-IO expansion
> card.
> > +
> > +properties:
> > + compatible:
> > + oneOf:
> > + - items:
> > + - enum:
> > + - fsl,imx952-evk-aud-io
> > + - const: fsl,imx95-19x19-evk-aud-io
> > + - const: fsl,imx95-19x19-evk-aud-io
> > +
> > + gpio-controller: true
> > +
> > + '#gpio-cells':
> > + const: 2
> > +
> > + gpio-map:
> > + minItems: 1
> > + maxItems: 32
>
> You don't know how many GPIOs are on the connector?
>
Understood. I used a loose upper bound here, which is wrong. I will
constrain gpio-map to the exact number of GPIOs on the connector.
> > +
> > + gpio-map-mask:
> > + items:
> > + - const: 0xffff
> > + - const: 0x0
> > +
> > + gpio-map-pass-thru:
> > + items:
> > + - const: 0x0
> > + - const: 0x1
> > +
> > + '#clock-cells':
> > + const: 1
> > +
> > + clock-map:
> > + minItems: 1
> > + maxItems: 16
>
> You don't know how many clocks are on the connector?
>
Same here.
> > +
> > + clock-map-mask:
> > + items:
> > + - const: 0xff
>
> > +
> > + clock-map-pass-thru: true
>
> The purpose of this property (for GPIO) was to pass thru flag cells which
> are standardized. That's not the case for clocks.
>
Agreed. I will drop clock-map-pass-thru.
> Anyways, these properties need to be defined in dtschema first.
>
> Rob
Yes. As noted in the cover letter, this series depends on Miquel
Raynal's clock nexus binding/core support:
https://lore.kernel.org/all/20260327-schneider-v7-0-rc1-crypto-v1-10-5e6ff7853994@bootlin.com/
This series is intended to be applied only after the clock nexus binding
and core support are available.
Regards,
Chancel Liu
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: kaanapali: fix traceNoC probe issue
From: Leo Yan @ 2026-06-30 8:10 UTC (permalink / raw)
To: Jie Gan
Cc: Suzuki K Poulose, Mike Leach, James Clark, Konrad Dybcio,
Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Tingwei Zhang, Jingyi Wang, Abel Vesa,
Yuanfang Zhang, linux-arm-msm, devicetree, linux-kernel,
coresight, linux-arm-kernel
In-Reply-To: <adb45638-1787-45cd-b4fd-d957323cc608@oss.qualcomm.com>
Hi Jie,
On Tue, Jun 30, 2026 at 09:03:52AM +0800, Jie Gan wrote:
[...]
> > - How can you guarantee that a interconnect TraceNoC will never
> > require ATID in the future?
> From a hardware perspective, there is no fundamental difference between an
> itnoc and an AG TraceNoC. They use the same TraceNoC hardware implementation
> and share the same AMBA bus type. The distinction is purely functional: an
> itnoc is used for local trace aggregation within a subsystem, whereas an AG
> TraceNoC serves as the top-level aggregation point for the SoC.
I'm still not convinced that adding "arm,primecell-periphid" is the
right approach.
From the description above, I'd expect either the hardware to expose
bits in a register to distinguish these two module types, or as I
suggested earlier, to use a DT property to indicate the module type (or
whether ATID is required).
Or have you tried to detect the last tnoc on a path and allocate ID for
it? (You can retrieve csdev->path).
Thanks,
Leo
^ permalink raw reply
* Re: [PATCH v7 3/4] PCI: tegra: Add Tegra264 support
From: Manikanta Maddireddy @ 2026-06-30 8:21 UTC (permalink / raw)
To: Manivannan Sadhasivam, Thierry Reding
Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Thierry Reding,
Jonathan Hunter, Karthikeyan Mitran, Hou Zhiqiang,
Thomas Petazzoni, Pali Rohár, Michal Simek, Kevin Xie,
Aksh Garg, linux-pci, devicetree, linux-tegra, linux-kernel,
linux-arm-kernel, Thierry Reding
In-Reply-To: <23rcfdnhnjhdlhiw6eclxap2tk6j5ni7qkfsd3fkfmucvjemie@fvf4j5earp47>
On 30/06/26 1:33 pm, Manivannan Sadhasivam wrote:
>>>> +static void tegra264_pcie_remove(struct platform_device *pdev)
>>>> +{
>>>> + struct tegra264_pcie *pcie = platform_get_drvdata(pdev);
>>>> +
>>>> + /*
>>>> + * If we undo tegra264_pcie_init() then link goes down and need
>>>> + * controller reset to bring up the link again. Remove intention is
>>>> + * to clean up the root bridge and re-enumerate during bind.
>>> But the controller will be consuming power even if PCIe is not used. Do you
>>> really want that? Can't tegra264_pcie_init() handle the initialization? I'm
>>> wondering how tegra264_pcie_deinit() in tegra264_pcie_suspend() works then.
>> I had to clarify this with the PCI team and they indicated that
>> tegra264_pcie_deinit() is actually useless and maybe even harmful. The
>> reason is that there's a processor on these boards (BPMP) that takes
>> care of power sequencing and it will automatically take the PCI links
>> to L2 on suspend and assert PERST#.
>>
> Then why are you calling tegra264_pcie_deinit() in tegra264_pcie_suspend()? If
> tegra264_pcie_deinit() is harmful, then calling it during suspend should also
> be, right?
>
> Or tegra264_pcie_deinit() has to be paired with BPMP doing its own power
> sequencing?
>
> Not a big deal, but it just feels weird to see suspend() and remove() doing
> different things.
tegra264_pcie_deinit() should be removed in tegra264_pcie_suspend(),
BPMP-FW takes care of L2+assert PERST# sequence during suspend.
- Manikanta
--
nvpublic
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: kaanapali: fix traceNoC probe issue
From: Suzuki K Poulose @ 2026-06-30 8:21 UTC (permalink / raw)
To: Jie Gan, Leo Yan, Mike Leach, James Clark
Cc: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Tingwei Zhang, Jingyi Wang,
Abel Vesa, Yuanfang Zhang, linux-arm-msm, devicetree,
linux-kernel, coresight, linux-arm-kernel
In-Reply-To: <adb45638-1787-45cd-b4fd-d957323cc608@oss.qualcomm.com>
On 30/06/2026 02:03, Jie Gan wrote:
>
>
> On 6/29/2026 10:28 PM, Leo Yan wrote:
>> On Mon, Jun 29, 2026 at 10:08:17AM +0800, Jie Gan wrote:
>>
>> [...]
>>
>>> Can I fix the issue by adding "arm,primecell-periphid" property. That's
>>> would be the best temp solution as it avoids breaking the original
>>> design of
>>> both the TraceNoC AMBA driver and interconnect TraceNoC platform driver.
>>
>> Before proceeding with the "arm,primecell-periphid" property, could you
>> clarify a bit:
>>
>> - For an interconnect TraceNoC, what would be the consequence of
>> enabling ATID? Would it simply be a no-op, or are there any side
>> effects? Or is the concern that the trace IDs could be exhausted?
>>
>
> TPDM0(or ATB source) -> interconnect TraceNoC0 -> Aggregator TraceNoc ->
> sink
> TPDM1(or ATB source) -> interconnect TraceNoC1 -> Aggregator TraceNoc ->
> sink
>
> We only have one Aggregator TraceNoC and many interconnect TraceNoC
> devices for one platform. All interconnect TraceNoC devices are
> connected to Aggregator TraceNoC devices in the topology, so the itnoc
> doesnt need an ATID.
>
> That's the design purpose from hardware perspective.
>
>
>> - How can you guarantee that a interconnect TraceNoC will never
>> require ATID in the future?
>>
>
> The interconnect TraceNoC is primarily introduced to reduce routing
> complexity in the hardware design. It is typically deployed as an
> intermediate TraceNoC that connects to an Aggregator TraceNoC (AG
> TraceNoC).
You can always distinguish one from the other by checking the
"compatibles" or even add a custom data field to the of_device_id
table for the platform driver. Personally, I think it is better to
keep things away from AMBA framework, when we get everything from
platform driver.
Cheers
Suzuki
>
> For example, a modem subsystem may contain many TPDM devices. Directly
> connecting every TPDM to the AG TraceNoC would result in significant
> wiring complexity. Instead, an itnoc is placed within the modem
> subsystem to locally aggregate the TPDM connections. All TPDMs first
> connect to the itnoc, and the itnoc then connects to the system-level AG
> TraceNoC.
>
> From a hardware perspective, there is no fundamental difference between
> an itnoc and an AG TraceNoC. They use the same TraceNoC hardware
> implementation and share the same AMBA bus type. The distinction is
> purely functional: an itnoc is used for local trace aggregation within a
> subsystem, whereas an AG TraceNoC serves as the top-level aggregation
> point for the SoC.
>
> Thanks,
> Jie
>
>>> The TraceNoC device here must be treated as an AMBA device and I am
>>> continuing to investigate the issue with our hardware team.
>>
>>> We aim to fix it from hardware perspetive for existing platforms if
>>> possible
>>> and ensure it is fixed in future platforms.
>>
>> I'm concerned that all of use end up repeatedly fixing similar issues
>> whenever hardware configurations change or modules are reused in
>> different topologies.
>>
>> For example, if future platforms may require ATID support for an
>> interconnect TraceNoC, then the issue will pop up again.
>>
>> Thanks,
>> Leo
>
^ permalink raw reply
* Re: [PATCH v10 0/9] perf cs-etm: Support thread stack and callchain
From: Leo Yan @ 2026-06-30 8:23 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, John Garry, Will Deacon, James Clark,
Mike Leach, Suzuki K Poulose, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Ian Rogers, Adrian Hunter, Al Grant, Paschalis Mpeis,
Amir Ayupov, linux-arm-kernel, coresight, linux-perf-users,
Leo Yan
In-Reply-To: <akMPoLNBbAEyNU64@google.com>
Hi Namhyung,
On Mon, Jun 29, 2026 at 05:36:48PM -0700, Namhyung Kim wrote:
[...]
> Will you send a new version or want to merge this? It seems there are
> some remaining comments from Sashiko.
I prefer to merge this series.
Sashiko reported several critical issues in the common code, they are on
my to-do list.
Thanks,
Leo
^ permalink raw reply
* Re: [PATCH RFC v5 05/12] clk: zte: Add Clock registration infrastructure.
From: Philipp Zabel @ 2026-06-30 8:27 UTC (permalink / raw)
To: Stefan Dösinger, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brian Masney
Cc: linux-clk, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20260628-zx29clk-v5-5-79ff044e4192@gmail.com>
On So, 2026-06-28 at 22:59 +0300, Stefan Dösinger wrote:
> The next patches will implement the regmap clocks and PLL driver. The
> actual hardware specific clock listing will live in a separate module.
>
> Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
>
> ---
>
> Version 5:
>
> *) Pass the static clk data instead of calling get_match_data to prepare
> for operating as an MFD child.
I think the MFD driver is unnecessary overhead. Can't you just keep the
reset controllers as auxdev and use of_platform_populate() to create
devices for clock-controller child nodes such as syscon-reboot?
> *) Don't use devm_kzalloc to allocate the auxiliary_device
> structure. I guess Sashiko is right, and that's what "Because once the
> device is placed on the bus the parent driver can not tell what other
> code may have a reference to this data" is trying to dell me.
Not using devm_kzalloc() for the auxdev is correct, but it looks like
you could simplify its creation a lot by just using
devm_auxiliary_device_create().
regards
Philipp
^ permalink raw reply
* [PATCH v2 5/9] ARM: VDSO: Respect COMPAT_32BIT_TIME
From: Thomas Weißschuh @ 2026-06-30 7:38 UTC (permalink / raw)
To: Andy Lutomirski, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Russell King, Catalin Marinas,
Will Deacon, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Thomas Bogendoerfer,
Vincenzo Frascino, John Stultz, Stephen Boyd, David S. Miller,
Andreas Larsson
Cc: Thomas Weißschuh, linux-kernel, linux-arm-kernel,
linuxppc-dev, linux-mips, Arnd Bergmann, linux-api, sparclinux
In-Reply-To: <20260630-vdso-compat_32bit_time-v2-0-520d194640dd@linutronix.de>
If CONFIG_COMPAT_32BIT_TIME is disabled then the vDSO should not
provide any 32-bit time related functionality. This is the intended
effect of the kconfig option and also the fallback system calls would
also not be implemented.
Currently the kconfig option does not affect the gettimeofday() syscall,
so also keep that in the vDSO.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
arch/arm/vdso/vdso.lds.S | 2 ++
arch/arm/vdso/vgettimeofday.c | 14 ++++++++------
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/arm/vdso/vdso.lds.S b/arch/arm/vdso/vdso.lds.S
index 74d8d8bc8a40..e61038c0195a 100644
--- a/arch/arm/vdso/vdso.lds.S
+++ b/arch/arm/vdso/vdso.lds.S
@@ -70,9 +70,11 @@ VERSION
{
LINUX_2.6 {
global:
+#ifdef CONFIG_COMPAT_32BIT_TIME
__vdso_clock_gettime;
__vdso_gettimeofday;
__vdso_clock_getres;
+#endif /* CONFIG_COMPAT_32BIT_TIME */
__vdso_clock_gettime64;
__vdso_clock_getres_time64;
local: *;
diff --git a/arch/arm/vdso/vgettimeofday.c b/arch/arm/vdso/vgettimeofday.c
index f7a2f5dc2fdc..3eebeddbfd18 100644
--- a/arch/arm/vdso/vgettimeofday.c
+++ b/arch/arm/vdso/vgettimeofday.c
@@ -10,16 +10,17 @@
#include <asm/unwind.h>
#include <vdso/gettime.h>
+#ifdef CONFIG_COMPAT_32BIT_TIME
int __vdso_clock_gettime(clockid_t clock,
struct old_timespec32 *ts)
{
return __cvdso_clock_gettime32(clock, ts);
}
-int __vdso_clock_gettime64(clockid_t clock,
- struct __kernel_timespec *ts)
+int __vdso_clock_getres(clockid_t clock_id,
+ struct old_timespec32 *res)
{
- return __cvdso_clock_gettime(clock, ts);
+ return __cvdso_clock_getres_time32(clock_id, res);
}
int __vdso_gettimeofday(struct __kernel_old_timeval *tv,
@@ -27,11 +28,12 @@ int __vdso_gettimeofday(struct __kernel_old_timeval *tv,
{
return __cvdso_gettimeofday(tv, tz);
}
+#endif /* CONFIG_COMPAT_32BIT_TIME */
-int __vdso_clock_getres(clockid_t clock_id,
- struct old_timespec32 *res)
+int __vdso_clock_gettime64(clockid_t clock,
+ struct __kernel_timespec *ts)
{
- return __cvdso_clock_getres_time32(clock_id, res);
+ return __cvdso_clock_gettime(clock, ts);
}
int __vdso_clock_getres_time64(clockid_t clock_id, struct __kernel_timespec *res)
--
2.55.0
^ permalink raw reply related
* Re: [PATCH v2 2/2] arm64: dts: qcom: kaanapali: fix traceNoC probe issue
From: Jie Gan @ 2026-06-30 8:42 UTC (permalink / raw)
To: Leo Yan, Suzuki K Poulose, Mike Leach, James Clark
Cc: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Tingwei Zhang, Jingyi Wang,
Abel Vesa, Yuanfang Zhang, linux-arm-msm, devicetree,
linux-kernel, coresight, linux-arm-kernel
In-Reply-To: <20260630081021.GD1812158@e132581.arm.com>
Hi Leo,
On 6/30/2026 4:10 PM, Leo Yan wrote:
> Hi Jie,
>
> On Tue, Jun 30, 2026 at 09:03:52AM +0800, Jie Gan wrote:
>
> [...]
>
>>> - How can you guarantee that a interconnect TraceNoC will never
>>> require ATID in the future?
>
>> From a hardware perspective, there is no fundamental difference between an
>> itnoc and an AG TraceNoC. They use the same TraceNoC hardware implementation
>> and share the same AMBA bus type. The distinction is purely functional: an
>> itnoc is used for local trace aggregation within a subsystem, whereas an AG
>> TraceNoC serves as the top-level aggregation point for the SoC.
>
> I'm still not convinced that adding "arm,primecell-periphid" is the
> right approach.
>
I agree we shouldn't need to add arm,primecell-periphid for the AMBA
bus, as the hardware provides the necessary registers to read the
peripheral ID. I used it as a temporary workaround to resolve the issue,
but I believe that solution is not correct.
> From the description above, I'd expect either the hardware to expose
> bits in a register to distinguish these two module types, or as I
> suggested earlier, to use a DT property to indicate the module type (or
> whether ATID is required).
>
I wanna distinguish the aggregator traceNoC and interconnect traceNoC,
even probe with platform driver, but the existing compatible is too
specific to the interconnect traceNoC device(coresight-itnoc), that's
why I didnt try the DT property proposal.
> Or have you tried to detect the last tnoc on a path and allocate ID for
> it? (You can retrieve csdev->path).
As Suzuki mentioned in the other thread, I think it would be better to
add separate compatibles in the of_match_table to distinguish between
Aggregator TraceNoC and Interconnect TraceNoC when probing with the
platform driver. This would allow us to allocate an ATID only for
Aggregator TraceNoC during probe, which is consistent with our original
design.
Thanks,
Jie
>
> Thanks,
> Leo
^ permalink raw reply
* Re: [PATCH RFC v5 11/12] reset: zte: Add a zx297520v3 reset driver
From: Philipp Zabel @ 2026-06-30 8:45 UTC (permalink / raw)
To: Stefan Dösinger, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brian Masney
Cc: linux-clk, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20260628-zx29clk-v5-11-79ff044e4192@gmail.com>
On So, 2026-06-28 at 22:59 +0300, Stefan Dösinger wrote:
> This drives the MFD child devices created by the zx297520v3-crm driver
> as well as the aux device created by the zx297520v3-lspclk driver.
>
> Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
>
> ---
>
> v5:
> Make top and matrix MFD children instead of aux devices
Why? What is the difference between top/matrix and lsp here?
Couldn't you just keep all three as aux devices and remove the
platform_device boilerplate half of the driver?
[...]
> diff --git a/drivers/reset/reset-zte-zx297520v3.c b/drivers/reset/reset-zte-zx297520v3.c
> new file mode 100644
> index 000000000000..8ef434904230
> --- /dev/null
> +++ b/drivers/reset/reset-zte-zx297520v3.c
> @@ -0,0 +1,274 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2026 Stefan Dösinger
> + */
> +#include <dt-bindings/reset/zte,zx297520v3-reset.h>
> +#include <linux/reset-controller.h>
> +#include <linux/platform_device.h>
> +#include <linux/auxiliary_bus.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
> +#include <linux/iopoll.h>
Looks like this is not needed anymore.
> +/* Most devices on the zx297520v3 SoC have one reset bit per clock line. As a rule of thumb, the
> + * lower bit disconnects the device from the bus, similarly to turning off PCLK - registers read 0
> + * or hang indefinitely. Unlike PCLK, this reset may have a lingering effect after deasserting.
> + * E.g. timers will be disabled, but retain their counter value.
> + *
> + * The other bit resets the actual device registers.
> + *
> + * For some devices, e.g. GMAC, the reset bits behave in the same way: They disconnect the device
> + * and registers will have their default state after deasserting. For devices that have both reset
> + * bits, both need to be deasserted for the device to function.
> + */
> +struct zte_reset_reg {
> + u32 mask;
> + u16 reg;
> +};
> +
> +struct zte_reset_info {
> + const struct zte_reset_reg *resets;
> + unsigned int num;
> +};
> +
> +struct zte_reset {
> + struct reset_controller_dev rcdev;
> + struct regmap *map;
> + const struct zte_reset_reg *resets;
> +};
> +
> +static inline struct zte_reset *to_zte_reset(struct reset_controller_dev *rcdev)
> +{
> + return container_of(rcdev, struct zte_reset, rcdev);
> +}
> +
> +static int zx29_rst_assert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct zte_reset *rst = to_zte_reset(rcdev);
> +
> + return regmap_clear_bits(rst->map, rst->resets[id].reg, rst->resets[id].mask);
> +}
> +
> +static int zx29_rst_deassert(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct zte_reset *rst = to_zte_reset(rcdev);
> +
> + return regmap_set_bits(rst->map, rst->resets[id].reg, rst->resets[id].mask);
> +}
> +
> +static int zx29_rst_status(struct reset_controller_dev *rcdev, unsigned long id)
> +{
> + struct zte_reset *rst = to_zte_reset(rcdev);
> + int res;
> +
> + res = regmap_test_bits(rst->map, rst->resets[id].reg, rst->resets[id].mask);
The correct thing to do here would be to only check the reset bit.
This happens to work anyway because we always set reset and isolation
bits together, but maybe this warrants a comment.
I assume the registers just read back the value that was set.
> + if (res < 0)
> + return res;
> +
> + return !res;
> +}
> +
[...]
> +static int reset_zx297520v3_common_probe(struct device *dev,
> + struct device_node *of_node,
> + const struct zte_reset_info *drv_info)
> +{
> + struct zte_reset *rst;
> +
> + rst = devm_kzalloc(dev, sizeof(*rst), GFP_KERNEL);
> + if (!rst)
> + return -ENOMEM;
> +
> + rst->resets = drv_info->resets;
> + rst->rcdev.owner = THIS_MODULE;
> + rst->rcdev.nr_resets = drv_info->num;
> + rst->rcdev.ops = &zx29_rst_ops;
> + rst->rcdev.of_node = of_node;
> + rst->rcdev.dev = dev;
> +
> + rst->map = device_node_to_regmap(of_node);
> + if (IS_ERR(rst->map))
> + return dev_err_probe(dev, PTR_ERR(rst->map), "Cannot get parent syscon regmap\n");
> +
> + return devm_reset_controller_register(dev, &rst->rcdev);
> +
Unnecessary blank line.
> +}
> +
> +static int reset_zx297520v3_aux_probe(struct auxiliary_device *adev,
> + const struct auxiliary_device_id *id)
> +{
> + return reset_zx297520v3_common_probe(&adev->dev, adev->dev.of_node,
> + (const struct zte_reset_info *)id->driver_data);
> +}
> +
> +static int reset_zx297520v3_top_probe(struct platform_device *pdev)
> +{
> + return reset_zx297520v3_common_probe(&pdev->dev, pdev->dev.parent->of_node,
> + &zx297520v3_top_info);
> +}
> +
> +static struct platform_driver reset_zx297520v3_top = {
> + .probe = reset_zx297520v3_top_probe,
> + .driver = {
> + .name = "zx297520v3-toprst",
> + },
> +};
> +
> +static int reset_zx297520v3_matrix_probe(struct platform_device *pdev)
> +{
> + return reset_zx297520v3_common_probe(&pdev->dev, pdev->dev.parent->of_node,
> + &zx297520v3_matrix_info);
> +}
> +
> +static struct platform_driver reset_zx297520v3_matrix = {
> + .probe = reset_zx297520v3_matrix_probe,
> + .driver = {
> + .name = "zx297520v3-matrixrst",
> + },
> +};
> +
> +static const struct auxiliary_device_id reset_zx297520v3_ids[] = {
> + {
> + .name = "clk_zte.zx297520v3_lsprst",
> + .driver_data = (kernel_ulong_t)&zx297520v3_lsp_info,
> + },
> + { },
> +};
> +MODULE_DEVICE_TABLE(auxiliary, reset_zx297520v3_ids);
> +
> +static struct auxiliary_driver reset_zx297520v3_auxdrv = {
> + .name = "zx297520v3_lsp_reset",
> + .id_table = reset_zx297520v3_ids,
> + .probe = reset_zx297520v3_aux_probe,
> +};
> +
> +static struct platform_driver * const reset_zx297520v3_mfddrv[] = {
> + &reset_zx297520v3_top,
> + &reset_zx297520v3_matrix,
> +};
> +
> +static int __init reset_zx297520v3_init(void)
> +{
> + int res;
> +
> + res = auxiliary_driver_register(&reset_zx297520v3_auxdrv);
> + if (res)
> + return res;
> +
> + res = platform_register_drivers(reset_zx297520v3_mfddrv,
> + ARRAY_SIZE(reset_zx297520v3_mfddrv));
> + if (res)
> + auxiliary_driver_unregister(&reset_zx297520v3_auxdrv);
> +
> + return res;
> +}
> +
> +static void __exit reset_zx297520v3_exit(void)
> +{
> + platform_unregister_drivers(reset_zx297520v3_mfddrv,
> + ARRAY_SIZE(reset_zx297520v3_mfddrv));
> + auxiliary_driver_unregister(&reset_zx297520v3_auxdrv);
> +}
> +
> +module_init(reset_zx297520v3_init);
> +module_exit(reset_zx297520v3_exit);
That's too much boilerplate given I don't understand the benefit of
using platform_device for top/matrix resets yet.
regards
Philipp
^ permalink raw reply
* Re: [PATCH v2 0/5] netfilter: nf_flow_table_path: L2 bridge offload
From: Pablo Neira Ayuso @ 2026-06-30 8:45 UTC (permalink / raw)
To: Daniel Pawlik
Cc: netfilter-devel, netdev, fw, phil, davem, edumazet, kuba, pabeni,
horms, andrew+netdev, razor, idosch, matthias.bgg,
angelogioacchino.delregno, bridge, coreteam, linux-mediatek,
linux-arm-kernel, rchen14b, lorenzo
In-Reply-To: <20260630065735.3341614-1-pawlik.dan@gmail.com>
Hi,
On Tue, Jun 30, 2026 at 08:57:30AM +0200, Daniel Pawlik wrote:
> This series adds L2 bridge offload support to nft_flow_offload, allowing
> bridged IPv4/IPv6 flows to be accelerated by the flowtable fast path
> without requiring L3 routing.
>
> Background
> ----------
> Hardware flow offload engines (e.g. MediaTek PPE) can accelerate bridged
> traffic but require that nft_flow_offload detect and handle bridged flows
> differently from routed ones: no routing table lookup, MAC addresses from
> the Ethernet header, and VLAN context pre-populated from the bridge port.
>
> v2: Fix missing Returns: tags in kernel-doc comments for the three new
> bridge helpers (br_fdb_has_forwarding_entry_rcu,
> br_vlan_get_offload_info_rcu, br_vlan_is_enabled_rcu).
>
> Patches
> -------
> 1/5 net: export __dev_fill_forward_path
> Refactors dev_fill_forward_path() to expose __dev_fill_forward_path()
> which accepts a caller-supplied net_device_path_ctx, needed to
> pre-populate VLAN state before the forward path walk.
>
> 2/5 net: bridge: add flow offload helpers
> Adds br_fdb_has_forwarding_entry_rcu(), br_vlan_get_offload_info_rcu()
> and br_vlan_is_enabled_rcu() to expose bridge state to nft_flow_offload
> without requiring inclusion of net/bridge/br_private.h.
>
> 3/5 netfilter: nf_flow_table_path: add L2 bridge offload
> Core of the series. Adds nft_flow_offload_is_bridging() detection,
> nft_flow_route_bridging() which avoids nf_route() (fails for
> bridged-only subnets), MAC/VLAN pre-population for bridged flows,
> and a dst leak fix. nft_flow_route() becomes a thin dispatcher.
>
> 4/5 netfilter: nf_flow_table_path: handle DEV_PATH_MTK_WDMA in path info
> Fixes zero-source-MAC in PPE entries when a bridged flow traverses
> MT7996/MT7915 WiFi WDMA hardware.
>
> 5/5 netfilter: nf_flow_table_path: add VLAN passthrough support
> Records VLAN encap info for passthrough-mode bridge ports so hardware
> offload entries include the correct VLAN tag.
>
> Rebase note
> -----------
> Originally developed against OpenWrt pending-6.18 patches by Ryan Chen
> <rchen14b@gmail.com> and Bo-Cun Chen <bc-bocun.chen@mediatek.com>.
> Rebased to current upstream: path discovery infrastructure moved to
> nf_flow_table_path.c in commit 93d7a7ed0734 ("netfilter: flowtable: move
> path discovery infrastructure to its own file"), so all netfilter changes
> now land in that file rather than nft_flow_offload.c.
>
> How to enable bridge offload
> -----------------------------
> 1. Load kmod-br-netfilter so that bridged IP traffic traverses the
> netfilter forward chain.
>
> 2. Enable netfilter hooks on the bridge:
> echo 1 > /sys/class/net/<br>/bridge/nf_call_iptables
> echo 1 > /sys/class/net/<br>/bridge/nf_call_ip6tables
This requires br_netfilter which is a no go.
Sorry, but we should really target at the native nf_conntrack_bridge
support.
> 3. Register bridge member interfaces in the nft flowtable:
> table inet filter {
> flowtable f {
> hook ingress priority filter
> devices = { eth0, wlan0 }
> }
> chain forward {
> type filter hook forward priority filter
> meta l4proto { tcp, udp } flow add @f
> }
> }
Yes, but br_netfilter makes no sense for nftables.
br_netfilter was made to fill gap at the time ebtables was lagging a
lot behind iptables in terms of features. And getting ebtables on pair
with iptables in functionality was not feasible either, because it
required many new extensions that were specific of the bridge family,
which probably was not a big deal, but it also required to get
the ebtables command line tool on pair with iptables userspace, which
has received more development attention/effort that the bridge tool.
All of this does not stand true anymore with nftables, where the
bridge family capabilities are at pair with the inet families.
I am looking now at the native flowtable bridge support, I will get
back to you with updates.
^ permalink raw reply
* Re: [PATCH RFC v5 05/12] clk: zte: Add Clock registration infrastructure.
From: Stefan Dösinger @ 2026-06-30 8:53 UTC (permalink / raw)
To: Philipp Zabel
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Brian Masney, linux-clk, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <c59fab242716c80250a66707d7ccaaf243a85aac.camel@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 754 bytes --]
Hi Philipp,
> Am 30.06.2026 um 11:27 schrieb Philipp Zabel <p.zabel@pengutronix.de>:
>
> I think the MFD driver is unnecessary overhead. Can't you just keep the
> reset controllers as auxdev and use of_platform_populate() to create
> devices for clock-controller child nodes such as syscon-reboot?
MFD for top and matrix was the suggestion of Conor:
https://lore.kernel.org/linux-arm-kernel/20260618-fantasy-estimate-6c52edbc6890@spud/
To quote:
> I think aux bus makes perfect sense when you have a clock/reset
> controller, but once you start expanding past that and you have reboot
> or hwmon or hwspinlock then mfd starts to make sense.
I can go either way. To me aux vs mfd seems like a distinction without a difference.
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v3] iommu/arm-smmu: Use pm_runtime in fault handlers
From: Prakash Gupta @ 2026-06-30 9:02 UTC (permalink / raw)
To: Will Deacon, Robin Murphy, Joerg Roedel, Rob Clark, Connor Abbott
Cc: linux-arm-msm, linux-arm-kernel, iommu, linux-kernel,
Akhil P Oommen, Pranjal Shrivastava, Pratyush Brahma,
Prakash Gupta
Commit d4a44f0750bb ("iommu/arm-smmu: Invoke pm_runtime across the driver")
enabled pm_runtime for the arm-smmu device. On systems where the SMMU
sits in a power domain, all register accesses must be done while the
device is runtime active to avoid unclocked register reads and
potential NoC errors.
So far, this has not been an issue for most SMMU clients because
stall-on-fault is enabled by default. While a translation fault is
being handled, the SMMU stalls further translations for that context
bank, so the fault handler would not race with a powered-down SMMU.
Adreno SMMU now disables stall-on-fault in the presence of fault
storms to avoid saturating SMMU resources and hanging the GMU. With
stall-on-fault disabled, the SMMU can generate faults while its power
domain may no longer be enabled, which makes unclocked accesses to
fault-status registers in the SMMU fault handlers possible.
Guard the context and global fault handlers with
arm_smmu_rpm_get_if_active() and arm_smmu_rpm_put() so that all SMMU
fault register accesses are done with the SMMU powered. If the SMMU is
not runtime active, the fault can be safely ignored as
arm_smmu_device_reset() clears fault registers on resume.
Additionally, disable fault reporting in arm_smmu_runtime_suspend()
before powering down. pm_runtime_get_if_active() returns 0 during
RPM_SUSPENDING, so without this, level-triggered fault interrupts would
cause an interrupt storm while the device is being suspended.
arm_smmu_device_reset() re-enables fault reporting on resume.
Fixes: b13044092c1e ("drm/msm: Temporarily disable stall-on-fault after a page fault")
Co-developed-by: Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>
Signed-off-by: Pratyush Brahma <pratyush.brahma@oss.qualcomm.com>
Signed-off-by: Prakash Gupta <prakash.gupta@oss.qualcomm.com>
---
Changes in v3:
- Add arm_smmu_rpm_get_if_active() wrapper that returns 1 when pm_runtime
is disabled, ensuring fault handlers work on non-pm_runtime systems
- Disable fault reporting in arm_smmu_runtime_suspend() before powering
down to prevent interrupt storms during RPM_SUSPENDING state
- Use pm_runtime_put_autosuspend() in arm_smmu_rpm_put() instead of
private __pm_runtime_put_autosuspend()
- Link to v2: https://patch.msgid.link/20260313-smmu-rpm-v2-1-8c2236b402b0@oss.qualcomm.com
Changes in v2:
- Switched from arm_smmu_rpm_get()/arm_smmu_rpm_put() wrappers to
pm_runtime_get_if_active()/pm_runtime_put_autosuspend() APIs
- Added support for smmu->impl->global_fault callback in global fault handler
- Remove threaded irq context fault restriction to allow modifying stall
mode for adreno smmu
- Link to v1: https://patch.msgid.link/20260127-smmu-rpm-v1-1-2ef2f4c85305@oss.qualcomm.com
---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 92 +++++++++++++++++++++++++----------
1 file changed, 67 insertions(+), 25 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 0bd21d206eb3..045389e89484 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -79,11 +79,16 @@ static inline int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
static inline void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
{
- if (pm_runtime_enabled(smmu->dev)) {
- pm_runtime_mark_last_busy(smmu->dev);
- __pm_runtime_put_autosuspend(smmu->dev);
+ if (pm_runtime_enabled(smmu->dev))
+ pm_runtime_put_autosuspend(smmu->dev);
+}
- }
+static inline int arm_smmu_rpm_get_if_active(struct arm_smmu_device *smmu)
+{
+ if (!pm_runtime_enabled(smmu->dev))
+ return 1;
+
+ return pm_runtime_get_if_active(smmu->dev);
}
static void arm_smmu_rpm_use_autosuspend(struct arm_smmu_device *smmu)
@@ -462,10 +467,20 @@ static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
int idx = smmu_domain->cfg.cbndx;
int ret;
+ if (!arm_smmu_rpm_get_if_active(smmu))
+ return IRQ_NONE;
+
+ if (smmu->impl && smmu->impl->context_fault) {
+ ret = smmu->impl->context_fault(irq, dev);
+ goto out_power_off;
+ }
+
arm_smmu_read_context_fault_info(smmu, idx, &cfi);
- if (!(cfi.fsr & ARM_SMMU_CB_FSR_FAULT))
- return IRQ_NONE;
+ if (!(cfi.fsr & ARM_SMMU_CB_FSR_FAULT)) {
+ ret = IRQ_NONE;
+ goto out_power_off;
+ }
ret = report_iommu_fault(&smmu_domain->domain, NULL, cfi.iova,
cfi.fsynr & ARM_SMMU_CB_FSYNR0_WNR ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ);
@@ -480,7 +495,12 @@ static irqreturn_t arm_smmu_context_fault(int irq, void *dev)
ret == -EAGAIN ? 0 : ARM_SMMU_RESUME_TERMINATE);
}
- return IRQ_HANDLED;
+ ret = IRQ_HANDLED;
+
+out_power_off:
+ arm_smmu_rpm_put(smmu);
+
+ return ret;
}
static irqreturn_t arm_smmu_global_fault(int irq, void *dev)
@@ -489,14 +509,25 @@ static irqreturn_t arm_smmu_global_fault(int irq, void *dev)
struct arm_smmu_device *smmu = dev;
static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL,
DEFAULT_RATELIMIT_BURST);
+ int ret;
+
+ if (!arm_smmu_rpm_get_if_active(smmu))
+ return IRQ_NONE;
+
+ if (smmu->impl && smmu->impl->global_fault) {
+ ret = smmu->impl->global_fault(irq, dev);
+ goto out_power_off;
+ }
gfsr = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sGFSR);
gfsynr0 = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sGFSYNR0);
gfsynr1 = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sGFSYNR1);
gfsynr2 = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sGFSYNR2);
- if (!gfsr)
- return IRQ_NONE;
+ if (!gfsr) {
+ ret = IRQ_NONE;
+ goto out_power_off;
+ }
if (__ratelimit(&rs)) {
if (IS_ENABLED(CONFIG_ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT) &&
@@ -513,7 +544,11 @@ static irqreturn_t arm_smmu_global_fault(int irq, void *dev)
}
arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_sGFSR, gfsr);
- return IRQ_HANDLED;
+ ret = IRQ_HANDLED;
+
+out_power_off:
+ arm_smmu_rpm_put(smmu);
+ return ret;
}
static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
@@ -683,7 +718,6 @@ static int arm_smmu_init_domain_context(struct arm_smmu_domain *smmu_domain,
enum io_pgtable_fmt fmt;
struct iommu_domain *domain = &smmu_domain->domain;
struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
- irqreturn_t (*context_fault)(int irq, void *dev);
mutex_lock(&smmu_domain->init_mutex);
if (smmu_domain->smmu)
@@ -850,19 +884,14 @@ static int arm_smmu_init_domain_context(struct arm_smmu_domain *smmu_domain,
*/
irq = smmu->irqs[cfg->irptndx];
- if (smmu->impl && smmu->impl->context_fault)
- context_fault = smmu->impl->context_fault;
- else
- context_fault = arm_smmu_context_fault;
-
if (smmu->impl && smmu->impl->context_fault_needs_threaded_irq)
ret = devm_request_threaded_irq(smmu->dev, irq, NULL,
- context_fault,
+ arm_smmu_context_fault,
IRQF_ONESHOT | IRQF_SHARED,
"arm-smmu-context-fault",
smmu_domain);
else
- ret = devm_request_irq(smmu->dev, irq, context_fault, IRQF_SHARED,
+ ret = devm_request_irq(smmu->dev, irq, arm_smmu_context_fault, IRQF_SHARED,
"arm-smmu-context-fault", smmu_domain);
if (ret < 0) {
@@ -2125,7 +2154,6 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
int num_irqs, i, err;
u32 global_irqs, pmu_irqs;
- irqreturn_t (*global_fault)(int irq, void *dev);
smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
if (!smmu) {
@@ -2205,18 +2233,13 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
smmu->num_context_irqs = smmu->num_context_banks;
}
- if (smmu->impl && smmu->impl->global_fault)
- global_fault = smmu->impl->global_fault;
- else
- global_fault = arm_smmu_global_fault;
-
for (i = 0; i < global_irqs; i++) {
int irq = platform_get_irq(pdev, i);
if (irq < 0)
return irq;
- err = devm_request_irq(dev, irq, global_fault, IRQF_SHARED,
+ err = devm_request_irq(dev, irq, arm_smmu_global_fault, IRQF_SHARED,
"arm-smmu global fault", smmu);
if (err)
return dev_err_probe(dev, err,
@@ -2306,6 +2329,25 @@ static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
{
struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+ int i;
+ u32 reg;
+
+ /*
+ * Disable fault reporting before powering down to prevent unclocked
+ * register accesses in the fault handlers if an interrupt races with
+ * the suspend callback (e.g. device in RPM_SUSPENDING state).
+ * arm_smmu_device_reset() re-enables fault reporting on resume.
+ */
+ reg = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sCR0);
+ reg &= ~(ARM_SMMU_sCR0_GFRE | ARM_SMMU_sCR0_GFIE |
+ ARM_SMMU_sCR0_GCFGFRE | ARM_SMMU_sCR0_GCFGFIE);
+ arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_sCR0, reg);
+
+ for (i = 0; i < smmu->num_context_banks; i++) {
+ reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_SCTLR);
+ reg &= ~(ARM_SMMU_SCTLR_CFIE | ARM_SMMU_SCTLR_CFRE);
+ arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_SCTLR, reg);
+ }
clk_bulk_disable(smmu->num_clks, smmu->clks);
---
base-commit: ba3e43a9e601636f5edb54e259a74f96ca3b8fd8
change-id: 20251208-smmu-rpm-8bd67db93dca
Best regards,
--
Prakash Gupta <prakash.gupta@oss.qualcomm.com>
^ permalink raw reply related
* [PATCH 0/9] firmware: arm_scmi: Fix SCMI core cleanup paths
From: Sudeep Holla @ 2026-06-30 9:05 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel
Cc: Cristian Marussi, Sudeep Holla, Sudeep Holla, Sashiko
This series fixes a set of SCMI core and mailbox transport lifetime issues
found around device creation, channel setup failure and teardown paths.
They were mostly found by Sashiko[1] when reviewing some other series.
Posting these separately as the issues or the fixes are not related to [2]
Here is the attempt to fix them, some of them can be dropped if it is too
theoretical. I have addressed all the issues that made sense to me at the
time of reading the report.
The fixes tighten ownership of OF node references, make transport devices
reachable by explicit teardown without exposing them to normal SCMI driver
binding, and ensure partially initialized channels are unwound consistently on
probe/setup failures. The series also fixes races around requested-device
notifier teardown and RCU-protected protocol lookups.
The mailbox transport fixes are split by the feature that introduced the
leaking channel: one for unidirectional mailbox channels and one for P2A
completion channels.
Summary:
- Fix OF node reference ownership for generated SCMI devices.
- Allow explicit teardown lookup of internal transport devices.
- Clean up TX/RX channels when SCMI channel setup fails midway.
- Fix SCMI device lifetime handling around child lookup and bus ID reuse.
- Free transport resources when IDR insertion fails after channel setup.
- Unregister requested-device notifier before active protocol IDR teardown.
- Unwind mailbox channels on TX receiver and P2A receiver setup failures.
- Protect requested-device protocol lookup with RCU.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
[1] https://sashiko.dev/#/patchset/20260525-acpi_scmi_pcc-v2-0-4f38938d08d8@arm.com
[2] https://patch.msgid.link/20260525-acpi_scmi_pcc-v2-0-4f38938d08d8@arm.com
---
Sudeep Holla (9):
firmware: arm_scmi: Fix OF node reference handling
firmware: arm_scmi: Fix transport device teardown lookup
firmware: arm_scmi: Clean up channels on setup failure
firmware: arm_scmi: Fix SCMI device destroy lifetimes
firmware: arm_scmi: Free transport channel on IDR failure
firmware: arm_scmi: Unregister device notifier before IDR teardown
firmware: arm_scmi: Unwind TX receiver mailbox setup failure
firmware: arm_scmi: Unwind P2A receiver mailbox setup failure
firmware: arm_scmi: Protect device request lookup with RCU
drivers/firmware/arm_scmi/bus.c | 50 +++++++++++++++++---------
drivers/firmware/arm_scmi/common.h | 2 ++
drivers/firmware/arm_scmi/driver.c | 21 ++++++-----
drivers/firmware/arm_scmi/transports/mailbox.c | 12 +++++--
4 files changed, 57 insertions(+), 28 deletions(-)
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260629-scmi_core_fixes-da3cd753b4ea
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH 1/9] firmware: arm_scmi: Fix OF node reference handling
From: Sudeep Holla @ 2026-06-30 9:05 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel; +Cc: Cristian Marussi, Sudeep Holla, Sudeep Holla
In-Reply-To: <20260630-scmi_core_fixes-v1-0-f932c1e51992@kernel.org>
SCMI devices store the DT node in dev.of_node through
device_set_node(), but that helper only assigns the fwnode and
of_node pointers without taking an OF node reference.
Take a reference when assigning the node and release it from the
SCMI device release path. With the device owning that reference,
remove the separate channel-side get/put pair from the core driver.
Fixes: 96da4a99ce50 ("firmware: arm_scmi: Set fwnode for the scmi_device")
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/bus.c | 3 ++-
drivers/firmware/arm_scmi/driver.c | 4 ----
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index 793be9eabaed..f643a1f0e282 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -395,6 +395,7 @@ static void scmi_device_release(struct device *dev)
{
struct scmi_device *scmi_dev = to_scmi_dev(dev);
+ of_node_put(dev->of_node);
kfree_const(scmi_dev->name);
kfree(scmi_dev);
}
@@ -465,7 +466,7 @@ __scmi_device_create(struct device_node *np, struct device *parent,
scmi_dev->id = id;
scmi_dev->protocol_id = protocol;
scmi_dev->dev.parent = parent;
- device_set_node(&scmi_dev->dev, of_fwnode_handle(np));
+ device_set_node(&scmi_dev->dev, of_fwnode_handle(of_node_get(np)));
scmi_dev->dev.bus = &scmi_bus_type;
scmi_dev->dev.release = scmi_device_release;
dev_set_name(&scmi_dev->dev, "scmi_dev.%d", id);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 3e0d975ec94c..b9245238e293 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2778,13 +2778,11 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
devm_kfree(info->dev, cinfo);
return -EINVAL;
}
- of_node_get(of_node);
cinfo->id = prot_id;
cinfo->dev = &tdev->dev;
ret = info->desc->ops->chan_setup(cinfo, info->dev, tx);
if (ret) {
- of_node_put(of_node);
scmi_device_destroy(info->dev, prot_id, name);
devm_kfree(info->dev, cinfo);
return ret;
@@ -2807,7 +2805,6 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
"unable to allocate SCMI idr slot err %d\n", ret);
/* Destroy channel and device only if created by this call. */
if (tdev) {
- of_node_put(of_node);
scmi_device_destroy(info->dev, prot_id, name);
devm_kfree(info->dev, cinfo);
}
@@ -2892,7 +2889,6 @@ static int scmi_chan_destroy(int id, void *p, void *idr)
struct scmi_info *info = handle_to_scmi_info(cinfo->handle);
struct scmi_device *sdev = to_scmi_dev(cinfo->dev);
- of_node_put(cinfo->dev->of_node);
scmi_device_destroy(info->dev, id, sdev->name);
cinfo->dev = NULL;
}
--
2.43.0
^ permalink raw reply related
* [PATCH 6/9] firmware: arm_scmi: Unregister device notifier before IDR teardown
From: Sudeep Holla @ 2026-06-30 9:06 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel
Cc: Cristian Marussi, Sudeep Holla, Sashiko, Sudeep Holla
In-Reply-To: <20260630-scmi_core_fixes-v1-0-f932c1e51992@kernel.org>
The requested-devices notifier looks up protocol fwnodes from the
active_protocols IDR. During remove, unregister the notifier before
releasing and destroying active_protocols so no notifier callback can race
with the IDR teardown.
Keep the bus notifier registered until after the protocol state is torn
down, matching the existing remove ordering for SCMI bus users.
Fixes: 53b8c25df708 ("firmware: arm_scmi: Add common notifier helpers")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/driver.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 4b369b003003..6df0fe055d64 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -3402,6 +3402,9 @@ static void scmi_remove(struct platform_device *pdev)
scmi_notification_exit(&info->handle);
+ blocking_notifier_chain_unregister(&scmi_requested_devices_nh,
+ &info->dev_req_nb);
+
mutex_lock(&info->protocols_mtx);
idr_destroy(&info->protocols);
mutex_unlock(&info->protocols_mtx);
@@ -3410,8 +3413,6 @@ static void scmi_remove(struct platform_device *pdev)
of_node_put(child);
idr_destroy(&info->active_protocols);
- blocking_notifier_chain_unregister(&scmi_requested_devices_nh,
- &info->dev_req_nb);
bus_unregister_notifier(&scmi_bus_type, &info->bus_nb);
/* Safe to free channels since no more users */
--
2.43.0
^ permalink raw reply related
* [PATCH 7/9] firmware: arm_scmi: Unwind TX receiver mailbox setup failure
From: Sudeep Holla @ 2026-06-30 9:06 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel
Cc: Cristian Marussi, Sudeep Holla, Sashiko, Sudeep Holla
In-Reply-To: <20260630-scmi_core_fixes-v1-0-f932c1e51992@kernel.org>
mailbox_chan_setup() can request an additional unidirectional TX
receiver channel after successfully acquiring the primary channel. If
that second request fails, the function returns immediately and leaves
the primary channel allocated.
Unwind the primary mailbox channel before returning the error so probe
deferral or other setup failures do not leave the channel busy for later
probe attempts.
Fixes: 9f68ff79ec2c ("firmware: arm_scmi: Add support for unidirectional mailbox channels")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/transports/mailbox.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/arm_scmi/transports/mailbox.c b/drivers/firmware/arm_scmi/transports/mailbox.c
index ae0f67e6cc45..44d45ce838e5 100644
--- a/drivers/firmware/arm_scmi/transports/mailbox.c
+++ b/drivers/firmware/arm_scmi/transports/mailbox.c
@@ -225,9 +225,10 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
smbox->chan_receiver = mbox_request_channel(cl, a2p_rx_chan);
if (IS_ERR(smbox->chan_receiver)) {
ret = PTR_ERR(smbox->chan_receiver);
+ smbox->chan_receiver = NULL;
if (ret != -EPROBE_DEFER)
dev_err(cdev, "failed to request SCMI Tx Receiver mailbox\n");
- return ret;
+ goto err_free_chan;
}
}
@@ -246,6 +247,10 @@ static int mailbox_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
mutex_init(&smbox->chan_lock);
return 0;
+
+err_free_chan:
+ mbox_free_channel(smbox->chan);
+ return ret;
}
static int mailbox_chan_free(int id, void *p, void *data)
--
2.43.0
^ permalink raw reply related
* [PATCH 9/9] firmware: arm_scmi: Protect device request lookup with RCU
From: Sudeep Holla @ 2026-06-30 9:06 UTC (permalink / raw)
To: arm-scmi, linux-arm-kernel
Cc: Cristian Marussi, Sudeep Holla, Sashiko, Sudeep Holla
In-Reply-To: <20260630-scmi_core_fixes-v1-0-f932c1e51992@kernel.org>
The SCMI device request notifier looks up protocol OF nodes from the
active_protocols IDR. The IDR lookup can run concurrently with protocol
activation while probe is still registering protocols and creating their
SCMI devices.
Wrap the lookup in an RCU read-side critical section as required by the
IDR API for lockless readers.
Fixes: 53b8c25df708 ("firmware: arm_scmi: Add common notifier helpers")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/firmware/arm_scmi/driver.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 6df0fe055d64..a575e661f1e2 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -33,6 +33,7 @@
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/processor.h>
+#include <linux/rcupdate.h>
#include <linux/refcount.h>
#include <linux/slab.h>
#include <linux/xarray.h>
@@ -2957,7 +2958,9 @@ static int scmi_device_request_notifier(struct notifier_block *nb,
struct scmi_device_id *id_table = data;
struct scmi_info *info = req_nb_to_scmi_info(nb);
+ rcu_read_lock();
np = idr_find(&info->active_protocols, id_table->protocol_id);
+ rcu_read_unlock();
if (!np)
return NOTIFY_DONE;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH RFC v8 01/24] mm: Introduce kpkeys
From: Kevin Brodsky @ 2026-06-30 9:11 UTC (permalink / raw)
To: David Hildenbrand (Arm), Linus Walleij
Cc: linux-hardening, Andrew Morton, Andy Lutomirski, Catalin Marinas,
Dave Hansen, Ira Weiny, Jann Horn, Jeff Xu, Joey Gouly, Kees Cook,
Marc Zyngier, Mark Brown, Matthew Wilcox, Maxwell Bland,
Mike Rapoport (IBM), Peter Zijlstra, Pierre Langlois,
Quentin Perret, Rick Edgecombe, Ryan Roberts, Vlastimil Babka,
Will Deacon, Yang Shi, Yeoreum Yun, linux-arm-kernel, linux-mm,
x86, Lorenzo Stoakes, Thomas Gleixner
In-Reply-To: <9b11b0c0-dfc7-45ba-934e-19cd053e5635@kernel.org>
On 22/06/2026 20:38, David Hildenbrand (Arm) wrote:
> On 6/18/26 15:22, Linus Walleij wrote:
>> On Tue, Jun 16, 2026 at 5:19 PM David Hildenbrand (Arm)
>> <david@kernel.org> wrote:
>>
>>> Looking at this, and wondering about "why do we get registers involved in this
>>> API" I would probably have an interface like:
>>>
>>> arch_kpkeys_enter_context()
>>> arch_kpkeys_leave_context()
>>>
>>> Whereby you return a "struct kpkeys_state" or sth like that.
>> This is close to what I was looking for as well.
> Cool :)
That's probably what I was looking for too, without knowing it!
>> enter/leave makes the code look more like generic entry.
>>
>> Passing some kind of state cookie around is inevitable in
>> this design and IIUC Kevin argued that it would be inefficient
>> (another level of abstraction) as opposed to just hammering
>> in the context we want, where we want it.
I must have misunderstood what you meant in your previous comments, I
don't have a concern with performance here.
>>
>> But I think the compiler will optimize that out by constant
>> propagation if the backing architecture implementation is
>> simple.
> Yes, it should be wrapped somehow. I don't think this will be a problem
> performance-wise that cannot be solved differently.
>
> Returning an u64 ("register") here is really arm64-sepcific and should be
> abstracted.
Agreed. All this looks very much like the new lazy MMU API, and doing
something along the same lines seems perfectly reasonable.
- Kevin
^ permalink raw reply
* [RESEND PATCH v3] coresight: etm-perf: Fix reference count leak in etm_setup_aux
From: Ma Ke @ 2026-06-30 9:11 UTC (permalink / raw)
To: suzuki.poulose, mike.leach, james.clark, leo.yan,
alexander.shishkin, mathieu.poirier
Cc: coresight, linux-arm-kernel, linux-kernel, akpm, Ma Ke, stable
bus_find_device() returns a device with its reference count
incremented. When a user-selected sink is obtained through
coresight_get_sink_by_id(), etm_setup_aux() keeps using the returned
sink while building the path and allocating the sink buffer.
Therefore the lookup reference must remain valid while etm_setup_aux()
is still using the sink, otherwise the sink could be removed under the
caller. Drop the lookup reference on the common exit path, after
etm_setup_aux() no longer directly uses the user-selected sink.
The CoreSight path code takes the references it needs for built paths,
so the initial lookup reference from coresight_get_sink_by_id() is no
longer needed after setup_aux finishes.
Found by code review.
Signed-off-by: Ma Ke <make_ruc2021@163.com>
Cc: stable@vger.kernel.org
Fixes: 0e6c20517596 ("coresight: etm-perf: Allow an event to use different sinks")
---
Changes in v3:
- do not drop the lookup reference in coresight_get_sink_by_id(), as
that would return a sink pointer without keeping the device reference
while etm_setup_aux() is still using it.
- dropped the lookup reference in etm_setup_aux on the common exit path,
as suggested by Suzuki.
- updated the commit message to describe why the reference is kept
until etm_setup_aux() finishes using the sink.
Changes in v2:
- modified the patch as suggestions.
---
drivers/hwtracing/coresight/coresight-etm-perf.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index 09b21a711a87..a25985743b7f 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -509,6 +509,11 @@ static void *etm_setup_aux(struct perf_event *event, void **pages,
goto err;
out:
+ if (user_sink) {
+ put_device(&user_sink->dev);
+ user_sink = NULL;
+ }
+
return event_data;
err:
--
2.43.0
^ permalink raw reply related
* Re: [PATCH RFC v8 01/24] mm: Introduce kpkeys
From: Kevin Brodsky @ 2026-06-30 9:13 UTC (permalink / raw)
To: David Hildenbrand (Arm), linux-hardening
Cc: Andrew Morton, Andy Lutomirski, Catalin Marinas, Dave Hansen,
Ira Weiny, Jann Horn, Jeff Xu, Joey Gouly, Kees Cook,
Linus Walleij, Marc Zyngier, Mark Brown, Matthew Wilcox,
Maxwell Bland, Mike Rapoport (IBM), Peter Zijlstra,
Pierre Langlois, Quentin Perret, Rick Edgecombe, Ryan Roberts,
Vlastimil Babka, Will Deacon, Yang Shi, Yeoreum Yun,
linux-arm-kernel, linux-mm, x86, Lorenzo Stoakes, Thomas Gleixner
In-Reply-To: <70f4dcdf-c45f-47d3-91df-a7897bd86ff4@kernel.org>
On 16/06/2026 17:19, David Hildenbrand (Arm) wrote:
> On 5/26/26 13:15, Kevin Brodsky wrote:
>> kpkeys is a simple framework to enable the use of protection keys
>> (pkeys) to harden the kernel itself. This patch introduces the basic
>> API in <linux/kpkeys.h>: a couple of functions to set and restore
>> the pkey register and macros to define guard objects.
>>
>> kpkeys introduces a new concept on top of pkeys: the kpkeys context.
>> Each context is associated to a set of permissions for the pkeys
>> managed by the kpkeys framework. kpkeys_set_context(ctx) sets those
>> permissions according to ctx, and returns the original pkey
>> register, to be later restored by kpkeys_restore_pkey_reg(). To
>> start with, only KPKEYS_CTX_DEFAULT is available, which is meant to
>> grant RW access to KPKEYS_PKEY_DEFAULT (i.e. all memory since this
>> is the only available pkey for now).
>>
>> Because each architecture implementing pkeys uses a different
>> representation for the pkey register, and may reserve certain pkeys
>> for specific uses, support for kpkeys must be explicitly indicated
>> by selecting ARCH_HAS_KPKEYS and defining the following functions in
>> <asm/kpkeys.h>, in addition to the macros provided in
>> <asm-generic/kpkeys.h>:
>>
>> - arch_kpkeys_set_context()
>> - arch_kpkeys_restore_pkey_reg()
> Looking at this, and wondering about "why do we get registers involved in this
> API" I would probably have an interface like:
>
> arch_kpkeys_enter_context()
> arch_kpkeys_leave_context()
>
> Whereby you return a "struct kpkeys_state" or sth like that.
>
> You could either let the architecture define what's in the state, or
> alternatively store some generic data in there as well.
>
> struct kpkeys_state {
> bool entered_context;
> struct arch_pkey_state arch;
> };
>
> Maybe the "entered_context" or however you would want to call it could avoid the
> KPKEYS_PKEY_REG_INVAL (which confuses me ;) )?
Yep that would do the trick. And would make Sashiko happier too, using a
magic register value isn't great ;)
> But the KPKEYS_PKEY_REG_INVAL usage confuses me. I understand the
> KPKEYS_GUARD_COND + kpkeys_restore_pkey_reg() one, but not the one where
> arch_kpkeys_set_context() would return that value.
Right, that's used in a follow-up series to protect struct cred, so that
unnecessary switches are avoided in case of nesting [1]. I wonder if I
shouldn't fold that patch into this one. I don't think nesting is likely
to occur in this series, but the extra branch probably doesn't add much
cost either (it's easily predicted).
[1]
https://lore.kernel.org/linux-mm/20250815090000.2182450-2-kevin.brodsky@arm.com/
>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>> ---
>> include/asm-generic/kpkeys.h | 17 ++++++
>> include/linux/kpkeys.h | 122 +++++++++++++++++++++++++++++++++++++++++++
>> mm/Kconfig | 2 +
>> 3 files changed, 141 insertions(+)
>>
>> diff --git a/include/asm-generic/kpkeys.h b/include/asm-generic/kpkeys.h
>> new file mode 100644
>> index 000000000000..ab819f157d6a
>> --- /dev/null
>> +++ b/include/asm-generic/kpkeys.h
>> @@ -0,0 +1,17 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +#ifndef __ASM_GENERIC_KPKEYS_H
>> +#define __ASM_GENERIC_KPKEYS_H
>> +
>> +#ifndef KPKEYS_PKEY_DEFAULT
>> +#define KPKEYS_PKEY_DEFAULT 0
>> +#endif
> Do we currently expect an architecture to overwrite this? How does this interact
> with KPKEYS_CTX_DEFAULT?
That's a fair point, pkey 0 being the default is pretty much hardcoded
and I don't see that ever changing.
The value isn't coupled to that of KPKEYS_CTX_DEFAULT, which is purely
symbolic.
> Nobody in this patch uses it, so maybe it should be added where actually needed.
Agreed, the ifdefery can be removed.
>> [...]
>>
>> +/**
>> + * kpkeys_set_context() - switch kpkeys context
>> + * @ctx: the context to switch to
>> + *
>> + * Switches to specified kpkeys context. @ctx must be a compile-time
>> + * constant. The arch-specific pkey register will be updated accordingly, and
>> + * the original value returned.
> Are these arch details and registers relevant? Ideally, we'd keep it very simple
> here ...
>
>> + *
>> + * Return: the original pkey register value if the register was written to, or
>> + * KPKEYS_PKEY_REG_INVAL otherwise (no write to the register was
>> + * required).
> ... and here. Not sure if any caller cares about these details. Again, with some
> abstract state we could maybe handle that internally.
>
> "Return: the pkey state to pass to kpkeys_restore_pkey_reg" (or however that
> function will be called)
Yep agreed.
>> + */
>> +static __always_inline u64 kpkeys_set_context(int ctx)
>> +{
>> + BUILD_BUG_ON_MSG(!__builtin_constant_p(ctx),
>> + "kpkeys_set_context() only takes constant values");
>> + BUILD_BUG_ON_MSG(ctx < KPKEYS_CTX_MIN || ctx > KPKEYS_CTX_MAX,
>> + "Invalid value passed to kpkeys_set_context()");
>> +
>> + return arch_kpkeys_set_context(ctx);
>> +}
>> +
>> +/**
>> + * kpkeys_restore_pkey_reg() - restores a pkey register value
>> + * @pkey_reg: the pkey register value to restore
>> + *
>> + * This function is meant to be passed the value returned by
>> + * kpkeys_set_context(), in order to restore the pkey register to its original
>> + * value (thus restoring the original kpkeys context).
>> + */
>> +static __always_inline void kpkeys_restore_pkey_reg(u64 pkey_reg)
>> +{
>> + if (pkey_reg != KPKEYS_PKEY_REG_INVAL)
>> + arch_kpkeys_restore_pkey_reg(pkey_reg);
>> +}
>> +
>> +static inline bool kpkeys_enabled(void)
> Is the enabled vs. supported intentional?
That's a fair point. It is intentional for
kpkeys_hardened_pgtables*_enabled() in patch 11: on arm64,
arch_supports_kpkeys*() always return true if POE is detected, while
kpkeys_hardened_pgtables*_enabled() also require
CONFIG_KPKEYS_HARDENED_PGTABLES=y.
For kpkeys_enabled() the condition is CONFIG_ARCH_HAS_KPKEYS=y, which is
always true if we support POE. So it would be reasonable to rename it to
kpkeys_supported() (and maybe more intuitive, since it doesn't imply any
functional change).
Thanks for the very useful suggestions and sorry for the late replies!
- Kevin
>> +{
>> + return arch_supports_kpkeys();
>> +}
>> +
>
>
^ permalink raw reply
* Re: [PATCH RFC v8 02/24] set_memory: Introduce set_memory_pkey() stub
From: Kevin Brodsky @ 2026-06-30 9:14 UTC (permalink / raw)
To: David Hildenbrand (Arm), linux-hardening
Cc: Andrew Morton, Andy Lutomirski, Catalin Marinas, Dave Hansen,
Ira Weiny, Jann Horn, Jeff Xu, Joey Gouly, Kees Cook,
Linus Walleij, Marc Zyngier, Mark Brown, Matthew Wilcox,
Maxwell Bland, Mike Rapoport (IBM), Peter Zijlstra,
Pierre Langlois, Quentin Perret, Rick Edgecombe, Ryan Roberts,
Vlastimil Babka, Will Deacon, Yang Shi, Yeoreum Yun,
linux-arm-kernel, linux-mm, x86, Lorenzo Stoakes, Thomas Gleixner
In-Reply-To: <e026ab6b-aee2-48e7-9bb5-1735d7a23859@kernel.org>
On 16/06/2026 17:41, David Hildenbrand (Arm) wrote:
> On 5/26/26 13:15, Kevin Brodsky wrote:
>> Introduce a new function, set_memory_pkey(), which sets the
>> protection key (pkey) of pages in the specified linear mapping
>> range. Architectures implementing kernel pkeys (kpkeys) must
>> provide a suitable implementation; an empty stub is added as
>> fallback.
>>
>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>> ---
>> include/linux/set_memory.h | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/include/linux/set_memory.h b/include/linux/set_memory.h
>> index 3030d9245f5a..7b3a8bfde3c6 100644
>> --- a/include/linux/set_memory.h
>> +++ b/include/linux/set_memory.h
>> @@ -84,4 +84,11 @@ static inline int set_memory_decrypted(unsigned long addr, int numpages)
>> }
>> #endif /* CONFIG_ARCH_HAS_MEM_ENCRYPT */
>>
>> +#ifndef CONFIG_ARCH_HAS_KPKEYS
>> +static inline int set_memory_pkey(unsigned long addr, int numpages, int pkey)
>> +{
>> + return 0;
>> +}
>> +#endif
>> +
>> #endif /* _LINUX_SET_MEMORY_H_ */
>>
> This patch looks rather odd, given that this is just a stub that won't be used
> before patch #20.
>
> And there, it's only used from arm64 code? So why do we need the common-code stub?
It is primarily used in patch 12, the generic kpkeys page table
allocator, so we do need a stub.
The ordering might be a little confusing indeed, but I tried to follow
the following order: 1. kpkeys/generic, 2. kpkeys/arm64, 3.
kpkeys_hardened_pgtables/generic, 4. kpkeys_hardened_pgtables/arm64.
Happy to reorder if you have other preferences.
- Kevin
^ permalink raw reply
* Re: [PATCH 09/27] ASoC: codecs: idt821034: Use guard() for mutex locks
From: Bui Duc Phuc @ 2026-06-30 9:14 UTC (permalink / raw)
To: Herve Codina
Cc: Mark Brown, Takashi Iwai, Nick Li, Support Opensource,
Liam Girdwood, Jaroslav Kysela, Srinivas Kandagatla,
Charles Keepax, Richard Fitzgerald, Matthias Brugger,
AngeloGioacchino Del Regno, Shenghao Ding, Kevin Lu, Baojun Xu,
Sen Wang, Oder Chiou, Linus Walleij, Kuninori Morimoto,
u.kleine-koenig, Zhang Yi, Marco Crivellari, Kees Cook,
HyeongJun An, Arnd Bergmann, Qianfeng Rong, linux-sound,
linux-kernel, patches, linux-mediatek, linux-arm-msm,
linux-arm-kernel
In-Reply-To: <20260630092821.650c30ec@bootlin.com>
On Tue, Jun 30, 2026 at 2:28 PM Herve Codina <herve.codina@bootlin.com> wrote:
>
> Hi,
>
> On Tue, 30 Jun 2026 13:34:31 +0700
> phucduc.bui@gmail.com wrote:
>
> > From: bui duc phuc <phucduc.bui@gmail.com>
> >
> > Clean up the code using guard() for mutex locks.
> > Merely code refactoring, and no behavior change.
> >
> > Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> > ---
> > sound/soc/codecs/idt821034.c | 121 +++++++++++++++--------------------
> > 1 file changed, 51 insertions(+), 70 deletions(-)
> >
> > diff --git a/sound/soc/codecs/idt821034.c b/sound/soc/codecs/idt821034.c
> > index 084090ccef77..078de6c9c395 100644
> > --- a/sound/soc/codecs/idt821034.c
> > +++ b/sound/soc/codecs/idt821034.c
> > @@ -6,6 +6,7 @@
> > //
> > // Author: Herve Codina <herve.codina@bootlin.com>
> >
> > +#include <linux/cleanup.h>
> > #include <linux/bitrev.h>
> > #include <linux/gpio/driver.h>
> > #include <linux/module.h>
>
> Alphabetic order. Move <linux/cleanup.h> after <linux/bitrev.h>.
>
> ...
>
> > @@ -456,7 +457,7 @@ static int idt821034_kctrl_gain_put(struct snd_kcontrol *kcontrol,
> >
> > ch = IDT821034_ID_GET_CHAN(mc->reg);
> >
> > - mutex_lock(&idt821034->mutex);
> > + guard(mutex)(&idt821034->mutex);
> >
> > if (IDT821034_ID_IS_OUT(mc->reg)) {
> > amp = &idt821034->amps.ch[ch].amp_out;
> > @@ -466,21 +467,18 @@ static int idt821034_kctrl_gain_put(struct snd_kcontrol *kcontrol,
> > gain_type = IDT821034_GAIN_TX;
> > }
> >
> > - if (amp->gain == val) {
> > - ret = 0;
> > - goto end;
> > - }
> > + if (amp->gain == val)
> > + return 0;
> >
> > if (!amp->is_muted) {
> > ret = idt821034_set_gain_channel(idt821034, ch, gain_type, val);
> > if (ret)
> > - goto end;
> > + return ret;
> > }
> >
> > amp->gain = val;
> > ret = 1; /* The value changed */
> > -end:
> > - mutex_unlock(&idt821034->mutex);
> > +
> > return ret;
>
> Instead of
> ret = 1; /* The value changed */
> return ret;
>
> Call directly
> return 1; /* The value changed */
>
> > }
>
> ...
> > @@ -521,7 +519,7 @@ static int idt821034_kctrl_mute_put(struct snd_kcontrol *kcontrol,
> > ch = IDT821034_ID_GET_CHAN(id);
> > is_mute = !ucontrol->value.integer.value[0];
> >
> > - mutex_lock(&idt821034->mutex);
> > + guard(mutex)(&idt821034->mutex);
> >
> > if (IDT821034_ID_IS_OUT(id)) {
> > amp = &idt821034->amps.ch[ch].amp_out;
> > @@ -531,20 +529,17 @@ static int idt821034_kctrl_mute_put(struct snd_kcontrol *kcontrol,
> > gain_type = IDT821034_GAIN_TX;
> > }
> >
> > - if (amp->is_muted == is_mute) {
> > - ret = 0;
> > - goto end;
> > - }
> > + if (amp->is_muted == is_mute)
> > + return 0;
> >
> > ret = idt821034_set_gain_channel(idt821034, ch, gain_type,
> > is_mute ? 0 : amp->gain);
> > if (ret)
> > - goto end;
> > + return ret;
> >
> > amp->is_muted = is_mute;
> > ret = 1; /* The value changed */
> > -end:
> > - mutex_unlock(&idt821034->mutex);
> > +
> > return ret;
>
> Instead of
> ret = 1; /* The value changed */
> return ret;
>
> Call directly
> return 1; /* The value changed */
>
> > }
> >
> > @@ -629,7 +624,7 @@ static int idt821034_power_event(struct snd_soc_dapm_widget *w,
> > ch = IDT821034_ID_GET_CHAN(id);
> > mask = IDT821034_ID_IS_OUT(id) ? IDT821034_CONF_PWRUP_RX : IDT821034_CONF_PWRUP_TX;
> >
> > - mutex_lock(&idt821034->mutex);
> > + guard(mutex)(&idt821034->mutex);
> >
> > power = idt821034_get_channel_power(idt821034, ch);
> > if (SND_SOC_DAPM_EVENT_ON(event))
> > @@ -638,8 +633,6 @@ static int idt821034_power_event(struct snd_soc_dapm_widget *w,
> > power &= ~mask;
> > ret = idt821034_set_channel_power(idt821034, ch, power);
> >
> > - mutex_unlock(&idt821034->mutex);
> > -
> > return ret;
>
> Instead of
> ret = idt821034_set_channel_power(idt821034, ch, power);
> return ret;
>
> return directly:
>
> return idt821034_set_channel_power(idt821034, ch, power);
>
> and remove the 'ret' variable (no more used)
>
> > }
> >
>
> ...
> > @@ -771,7 +764,7 @@ static int idt821034_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> > u8 conf;
> > int ret;
> >
> > - mutex_lock(&idt821034->mutex);
> > + guard(mutex)(&idt821034->mutex);
> >
> > conf = idt821034_get_codec_conf(idt821034);
> >
> > @@ -785,12 +778,10 @@ static int idt821034_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> > default:
> > dev_err(dai->dev, "Unsupported DAI format 0x%x\n",
> > fmt & SND_SOC_DAIFMT_FORMAT_MASK);
> > - ret = -EINVAL;
> > - goto end;
> > + return -EINVAL;
> > }
> > ret = idt821034_set_codec_conf(idt821034, conf);
> > -end:
> > - mutex_unlock(&idt821034->mutex);
> > +
> > return ret;
>
> Instead of
> ret = idt821034_set_codec_conf(idt821034, conf);
> return ret;
>
> return directly:
> return idt821034_set_codec_conf(idt821034, conf);
>
> and remove the 'ret' variable.
>
> > }
> >
> > @@ -802,7 +793,7 @@ static int idt821034_dai_hw_params(struct snd_pcm_substream *substream,
> > u8 conf;
> > int ret;
> >
> > - mutex_lock(&idt821034->mutex);
> > + guard(mutex)(&idt821034->mutex);
> >
> > conf = idt821034_get_codec_conf(idt821034);
> >
> > @@ -816,12 +807,10 @@ static int idt821034_dai_hw_params(struct snd_pcm_substream *substream,
> > default:
> > dev_err(dai->dev, "Unsupported PCM format 0x%x\n",
> > params_format(params));
> > - ret = -EINVAL;
> > - goto end;
> > + return -EINVAL;
> > }
> > ret = idt821034_set_codec_conf(idt821034, conf);
> > -end:
> > - mutex_unlock(&idt821034->mutex);
> > +
> > return ret;
>
> Idem here:
> return idt821034_set_codec_conf(idt821034, conf);
>
> and remove 'ret'.
>
>
> > }
> >
> > @@ -897,11 +886,11 @@ static int idt821034_reset_audio(struct idt821034 *idt821034)
> > int ret;
> > u8 i;
> >
> > - mutex_lock(&idt821034->mutex);
> > + guard(mutex)(&idt821034->mutex);
> >
> > ret = idt821034_set_codec_conf(idt821034, 0);
> > if (ret)
> > - goto end;
> > + return ret;
> >
> > for (i = 0; i < IDT821034_NB_CHANNEL; i++) {
> > idt821034->amps.ch[i].amp_out.gain = IDT821034_GAIN_OUT_INIT_RAW;
> > @@ -909,23 +898,22 @@ static int idt821034_reset_audio(struct idt821034 *idt821034)
> > ret = idt821034_set_gain_channel(idt821034, i, IDT821034_GAIN_RX,
> > idt821034->amps.ch[i].amp_out.gain);
> > if (ret)
> > - goto end;
> > + return ret;
> >
> > idt821034->amps.ch[i].amp_in.gain = IDT821034_GAIN_IN_INIT_RAW;
> > idt821034->amps.ch[i].amp_in.is_muted = false;
> > ret = idt821034_set_gain_channel(idt821034, i, IDT821034_GAIN_TX,
> > idt821034->amps.ch[i].amp_in.gain);
> > if (ret)
> > - goto end;
> > + return ret;
> >
> > ret = idt821034_set_channel_power(idt821034, i, 0);
> > if (ret)
> > - goto end;
> > + return ret;
> > }
> >
> > ret = 0;
> > -end:
> > - mutex_unlock(&idt821034->mutex);
> > +
> > return ret;
>
> Instead of
> ret = 0;
> return ret;
>
> return directly
> return 0;
>
> > }
> >
> ...
>
> >
> > @@ -1079,23 +1061,22 @@ static int idt821034_reset_gpio(struct idt821034 *idt821034)
> > int ret;
> > u8 i;
> >
> > - mutex_lock(&idt821034->mutex);
> > + guard(mutex)(&idt821034->mutex);
> >
> > /* IO0 and IO1 as input for all channels and output IO set to 0 */
> > for (i = 0; i < IDT821034_NB_CHANNEL; i++) {
> > ret = idt821034_set_slic_conf(idt821034, i,
> > IDT821034_SLIC_IO1_IN | IDT821034_SLIC_IO0_IN);
> > if (ret)
> > - goto end;
> > + return ret;
> >
> > ret = idt821034_write_slic_raw(idt821034, i, 0);
> > if (ret)
> > - goto end;
> > + return ret;
> >
> > }
> > ret = 0;
> > -end:
> > - mutex_unlock(&idt821034->mutex);
> > +
> > return ret;
>
> return 0;
>
> > }
> >
>
> Best regards,
> Hervé
>
Hi Hervé,
Thank you for the very detailed review.
I hadn't realized I had missed so many issues.
I'll fix them all and include the updates in v2.
Best regards,
Phuc
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox