* Re: [PATCH v4 3/4] PCI: tegra: Add Tegra264 support
From: Thierry Reding @ 2026-04-07 9:38 UTC (permalink / raw)
To: Manivannan Sadhasivam
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,
linux-pci, devicetree, linux-tegra, linux-kernel,
linux-arm-kernel, Thierry Reding, Manikanta Maddireddy
In-Reply-To: <iaoee5r5e2w52fap7ex23wdikbuvpjpesinedgjkehsedszhzo@64yoo2avmxle>
[-- Attachment #1: Type: text/plain, Size: 12894 bytes --]
On Thu, Apr 02, 2026 at 11:02:02PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Apr 02, 2026 at 04:27:37PM +0200, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > Add a driver for the PCIe controller found on NVIDIA Tegra264 SoCs. The
> > driver is very small, with its main purpose being to set up the address
> > translation registers and then creating a standard PCI host using ECAM.
> >
> > Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
>
> What is the rationale for adding a new driver? Can't you reuse the existing one?
> If so, that should be mentioned in the description.
Which existing one? Tegra PCI controllers for previou generations
(Tegra194 and Tegra234) were DesignWare IP, but Tegra264 is an internal
IP, so the programming is entirely different. I'll add something to that
effect to the commit message.
> > diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> > index 5aaed8ac6e44..6ead04f7bd6e 100644
> > --- a/drivers/pci/controller/Kconfig
> > +++ b/drivers/pci/controller/Kconfig
> > @@ -254,7 +254,15 @@ config PCI_TEGRA
> > select IRQ_MSI_LIB
> > help
> > Say Y here if you want support for the PCIe host controller found
> > - on NVIDIA Tegra SoCs.
> > + on NVIDIA Tegra SoCs (Tegra20 through Tegra186).
> > +
> > +config PCIE_TEGRA264
> > + bool "NVIDIA Tegra264 PCIe controller"
>
> This driver seems to be using external MSI controller. So it can be built as a
> module. Also, you have the remove() callback for some reason.
Okay, I can turn this into a tristate symbol.
> > + depends on ARCH_TEGRA || COMPILE_TEST
> > + depends on PCI_MSI
>
> Why?
I suppose it's not necessary in the sense of it being a build
dependency. At runtime, however, the root complex is not useful if PCI
MSI is not enabled. We can drop this dependency and rely on .config to
have it enabled as needed.
> > diff --git a/drivers/pci/controller/pcie-tegra264.c b/drivers/pci/controller/pcie-tegra264.c
> > new file mode 100644
> > index 000000000000..3ce1ad971bdb
> > --- /dev/null
> > +++ b/drivers/pci/controller/pcie-tegra264.c
[...]
> > +struct tegra264_pcie {
> > + struct device *dev;
> > + bool link_up;
>
> Keep bool types at the end to avoid holes.
Done.
> > +static int tegra264_pcie_parse_dt(struct tegra264_pcie *pcie)
> > +{
> > + int err;
> > +
> > + pcie->wake_gpio = devm_gpiod_get_optional(pcie->dev, "nvidia,pex-wake",
>
> You should switch to standard 'wake-gpios' property.
Will do.
> > + GPIOD_IN);
> > + if (IS_ERR(pcie->wake_gpio))
> > + return PTR_ERR(pcie->wake_gpio);
> > +
> > + if (pcie->wake_gpio) {
>
> Since you are bailing out above, you don't need this check.
I think we still want to have this check to handle the case of optional
wake GPIOs. Not all controllers may have this wired up and
devm_gpiod_get_optional() will return NULL (not an ERR_PTR()-encoded
error) if the wake-gpios property is missing.
> > +static void tegra264_pcie_bpmp_set_rp_state(struct tegra264_pcie *pcie)
>
> I don't think this function name is self explanatory. Looks like it is turning
> off the PCIe controller, so how about tegra264_pcie_power_off()?
Agreed. The name is a relic from when this was potentially being used to
toggle on and off the controller. But it's only used for disabling, so
tegra264_pcie__power_off() sounds much better.
> > +{
> > + struct tegra_bpmp_message msg = {};
> > + struct mrq_pcie_request req = {};
> > + int err;
> > +
> > + req.cmd = CMD_PCIE_RP_CONTROLLER_OFF;
> > + req.rp_ctrlr_off.rp_controller = pcie->ctl_id;
> > +
> > + msg.mrq = MRQ_PCIE;
> > + msg.tx.data = &req;
> > + msg.tx.size = sizeof(req);
> > +
> > + err = tegra_bpmp_transfer(pcie->bpmp, &msg);
> > + if (err)
> > + dev_info(pcie->dev, "failed to turn off PCIe #%u: %pe\n",
>
> Why not dev_err()?
>
> > + pcie->ctl_id, ERR_PTR(err));
> > +
> > + if (msg.rx.ret)
> > + dev_info(pcie->dev, "failed to turn off PCIe #%u: %d\n",
>
> Same here.
These are not fatal errors and are safe to ignore. dev_err() seemed too
strong for this. They also really shouldn't happen. Though I now realize
that's a bad argument, or rather, actually an argument for making them
dev_err() so that they do stand out if they really should happen.
>
> > + pcie->ctl_id, msg.rx.ret);
> > +}
> > +
> > +static void tegra264_pcie_icc_set(struct tegra264_pcie *pcie)
> > +{
> > + u32 value, speed, width, bw;
> > + int err;
> > +
> > + value = readw(pcie->ecam + XTL_RC_PCIE_CFG_LINK_STATUS);
> > + speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, value);
> > + width = FIELD_GET(PCI_EXP_LNKSTA_NLW, value);
> > +
> > + bw = width * (PCIE_SPEED2MBS_ENC(speed) / BITS_PER_BYTE);
> > + value = MBps_to_icc(bw);
>
> So this becomes, 'width * (PCIE_SPEED2MBS_ENC(speed) / 8) * 1000 / 8'. But don't
> you want, 'width * (PCIE_SPEED2MBS_ENC(speed)) * 1000 / 8'?
This is M*B*ps_to_icc(), not M*b*ps_to_icc(), so we do in fact get the
latter. I almost fell for this as well because I got confused by some of
these macros being all-caps and other times the case actually mattering.
> > + err = icc_set_bw(pcie->icc_path, bw, bw);
> > + if (err < 0)
> > + dev_err(pcie->dev,
> > + "failed to request bandwidth (%u MBps): %pe\n",
> > + bw, ERR_PTR(err));
>
> So you don't want to error out if this fails?
No. This is not a fatal error and the system will continue to work,
albeit perhaps at suboptimal performance. Given that Ethernet and mass
storage are connected to these, a failure to set the bandwidth and
erroring out here may leave the system unusable, but continuing on would
let the system boot and update firmware, kernel or whatever to recover.
I'll add a comment explaining this.
[...]
> > +static void tegra264_pcie_init(struct tegra264_pcie *pcie)
> > +{
> > + enum pci_bus_speed speed;
> > + unsigned int i;
> > + u32 value;
> > +
> > + /* bring the link out of reset */
>
> s/link/controller or endpoint?
This controls the PERST# signal, so I guess "endpoint" would be more
correct.
> > + value = readl(pcie->xtl + XTL_RC_MGMT_PERST_CONTROL);
> > + value |= XTL_RC_MGMT_PERST_CONTROL_PERST_O_N;
> > + writel(value, pcie->xtl + XTL_RC_MGMT_PERST_CONTROL);
> > +
> > + if (!tegra_is_silicon()) {
>
> This looks like some pre-silicon validation thing. Do you really want it to be
> present in the upstream driver?
At this point there is silicon for this chip, but we've been trying to
get some of the pre-silicon code merged upstream as well because
occasionally people will want to run upstream on simulation, even after
silicon is available. At other times we may want to reuse these drivers
on future chips during pre-silicon validation.
Obviously there needs to be a balance. We don't want to have excessive
amounts of code specifically for pre-silicon validation, but in
relatively simple cases like this it is useful.
>
> > + dev_info(pcie->dev,
> > + "skipping link state for PCIe #%u in simulation\n",
> > + pcie->ctl_id);
> > + pcie->link_up = true;
> > + return;
> > + }
> > +
> > + for (i = 0; i < PCIE_LINK_WAIT_MAX_RETRIES; i++) {
> > + if (tegra264_pcie_link_up(pcie, NULL))
> > + break;
> > +
> > + usleep_range(PCIE_LINK_WAIT_US_MIN, PCIE_LINK_WAIT_US_MAX);
> > + }
> > +
> > + if (tegra264_pcie_link_up(pcie, &speed)) {
>
> Why are you doing it for the second time?
It's just a last-resort check to see if it's really not come up after
the retries. Also, in this call we're actually interested in retrieving
the detected link speed.
>
> > + /* Per PCIe r5.0, 6.6.1 wait for 100ms after DLL up */
>
> No need of this comment.
Fair enough. This was perhaps more useful in earlier versions of the
patch before the line below used the standardize wait time.
[...]
> > +static int tegra264_pcie_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct pci_host_bridge *bridge;
> > + struct tegra264_pcie *pcie;
> > + struct resource_entry *bus;
> > + struct resource *res;
> > + int err;
> > +
> > + bridge = devm_pci_alloc_host_bridge(dev, sizeof(struct tegra264_pcie));
> > + if (!bridge)
> > + return dev_err_probe(dev, -ENOMEM,
> > + "failed to allocate host bridge\n");
> > +
> > + pcie = pci_host_bridge_priv(bridge);
> > + platform_set_drvdata(pdev, pcie);
> > + pcie->bridge = bridge;
> > + pcie->dev = dev;
> > +
> > + err = pinctrl_pm_select_default_state(dev);
>
> I questioned this before:
> https://lore.kernel.org/linux-pci/o5sxxdikdjwd76zsedvkpsl54nw6wrhopwsflt43y5st67mrub@uuw3yfjfqthd/
I'll remove this. Looks like we should be fine with just relying on the
default state being set by the pinctrl core. We might need to move it
into the resume callback.
> > + if (err < 0)
> > + return dev_err_probe(dev, err,
> > + "failed to configure sideband pins\n");
> > +
> > + err = tegra264_pcie_parse_dt(pcie);
> > + if (err < 0)
> > + return dev_err_probe(dev, err, "failed to parse device tree");
> > +
> > + pcie->xal = devm_platform_ioremap_resource_byname(pdev, "xal");
> > + if (IS_ERR(pcie->xal))
> > + return dev_err_probe(dev, PTR_ERR(pcie->xal),
> > + "failed to map XAL memory\n");
> > +
> > + pcie->xtl = devm_platform_ioremap_resource_byname(pdev, "xtl-pri");
> > + if (IS_ERR(pcie->xtl))
> > + return dev_err_probe(dev, PTR_ERR(pcie->xtl),
> > + "failed to map XTL-PRI memory\n");
> > +
> > + bus = resource_list_first_type(&bridge->windows, IORESOURCE_BUS);
> > + if (!bus)
> > + return dev_err_probe(dev, -ENODEV,
> > + "failed to get bus resources\n");
> > +
> > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ecam");
> > + if (!res)
> > + return dev_err_probe(dev, -ENXIO,
> > + "failed to get ECAM resource\n");
> > +
> > + pcie->icc_path = devm_of_icc_get(&pdev->dev, "write");
> > + if (IS_ERR(pcie->icc_path))
> > + return dev_err_probe(&pdev->dev, PTR_ERR(pcie->icc_path),
> > + "failed to get ICC");
> > +
> > + /*
> > + * Parse BPMP property only for silicon, as interaction with BPMP is
> > + * not needed for other platforms.
> > + */
> > + if (tegra_is_silicon()) {
> > + pcie->bpmp = tegra_bpmp_get_with_id(dev, &pcie->ctl_id);
> > + if (IS_ERR(pcie->bpmp))
> > + return dev_err_probe(dev, PTR_ERR(pcie->bpmp),
> > + "failed to get BPMP\n");
> > + }
> > +
>
> pm_runtime_set_active()
>
> > + pm_runtime_enable(dev);
>
> devm_pm_runtime_enable()?
Looks like I can even use devm_pm_runtime_set_active_enabled() to
combine the two.
>
> > + pm_runtime_get_sync(dev);
> > +
> > + /* sanity check that programmed ranges match what's in DT */
> > + if (!tegra264_pcie_check_ranges(pdev)) {
> > + err = -EINVAL;
> > + goto put_pm;
> > + }
> > +
> > + pcie->cfg = pci_ecam_create(dev, res, bus->res, &pci_generic_ecam_ops);
> > + if (IS_ERR(pcie->cfg)) {
> > + err = dev_err_probe(dev, PTR_ERR(pcie->cfg),
> > + "failed to create ECAM\n");
> > + goto put_pm;
> > + }
> > +
> > + bridge->ops = (struct pci_ops *)&pci_generic_ecam_ops.pci_ops;
> > + bridge->sysdata = pcie->cfg;
> > + pcie->ecam = pcie->cfg->win;
> > +
> > + tegra264_pcie_init(pcie);
> > +
> > + if (!pcie->link_up)
> > + goto free;
>
> goto free_ecam;
It's not clear to me, but are you suggesting to rename the existing
"free" label to "free_ecam"? I can do that.
> > + err = pci_host_probe(bridge);
> > + if (err < 0) {
> > + dev_err(dev, "failed to register host: %pe\n", ERR_PTR(err));
>
> dev_err_probe()
Okay.
>
> > + goto free;
> > + }
> > +
> > + return err;
>
> return 0;
Done.
[...]
> > +static int tegra264_pcie_resume_noirq(struct device *dev)
> > +{
> > + struct tegra264_pcie *pcie = dev_get_drvdata(dev);
> > + int err;
> > +
> > + 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;
> > +
> > + tegra264_pcie_init(pcie);
> > +
>
> Why do you need init() here without deinit() in tegra264_pcie_suspend_noirq()?
That's because when we come out of suspend the link may have gone down
again, so we need to take the endpoint out of reset to retrigger the
link training. I think we could possibly explicitly clear that PERST_O_N
bit in the PERST_CONTROL register in a new tegra264_pcie_deinit() to
mirror what tegra264_pcie_init() does, but it's automatically done by
firmware anyway, so not needed.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 1/1] PCI: tegra194: fix min() signedness when capping ASPM L1 entrance latency
From: Manikanta Maddireddy @ 2026-04-07 14:57 UTC (permalink / raw)
To: bhelgaas, lpieralisi, kwilczynski, mani, robh, krzk+dt, conor+dt,
thierry.reding, jonathanh, kishon, arnd, gregkh, Frank.Li, den,
hongxing.zhu, jingoohan1, vidyas, cassel, 18255117159
Cc: linux-pci, linux-tegra, linux-kernel, Manikanta Maddireddy,
kernel test robot
The DT property "aspm-l1-entry-delay-ns" is converted to microseconds,
then encoded for the L1 entrance latency register field as ilog2(us) + 1,
clamped to the hardware maximum of 7.
ilog2() returns int type, while the upper bound is 7U (unsigned int).
The min() macro is implemented with __careful_cmp(), which rejects mixed
signed and unsigned operands at compile time via BUILD_BUG_ON_MSG in
minmax.h; that check trips on this pair, notably when building with W=1.
This combination fails to build (e.g. parisc allyesconfig, GCC 15, as
reported by the 0-day bot).
Use min_t(u32, ilog2(us) + 1U, 7U) so both sides of the comparison are
unsigned and consistent with aspm_l1_enter_lat.
Fixes: 4a44cd65c9dd ("PCI: tegra194: Use aspm-l1-entry-delay-ns DT property for L1 entrance latency")
Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202604051407.AODe3ddZ-lkp@intel.com/
---
drivers/pci/controller/dwc/pcie-tegra194.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 393f75ce3df3..93d3452ac117 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1147,7 +1147,7 @@ static int tegra_pcie_dw_parse_dt(struct tegra_pcie_dw *pcie)
if (!ret) {
u32 us = max(val / 1000, 1U);
- pcie->aspm_l1_enter_lat = min(ilog2(us) + 1, 7U);
+ pcie->aspm_l1_enter_lat = min_t(u32, ilog2(us) + 1U, 7U);
}
ret = of_property_read_u32(np, "num-lanes", &pcie->num_lanes);
--
2.34.1
^ permalink raw reply related
* [PATCH 0/2] ASoC: tegra210: simplify byte map handling in ADX and AMX
From: Piyush Patle @ 2026-04-07 17:03 UTC (permalink / raw)
To: Mark Brown
Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Thierry Reding,
Jonathan Hunter, Sheetal, Kuninori Morimoto, linux-sound,
linux-tegra, linux-kernel
The Tegra210 ADX and AMX drivers both keep their "Byte Map N" ALSA
control state as a byte-packed u32 map[] array along with a separate
byte_mask[] bitmap. This is because the control range exposed to
userspace is [0, 256], where 256 is the "disabled" sentinel and
does not fit in a byte, so the two arrays have to be cross-checked
on every get()/put().
This series stores each slot as a u16 holding the user-visible
value directly, turning get_byte_map() into a direct return and
put_byte_map() into a compare-and-store. The hardware-facing packed
RAM word and the IN_BYTE_EN / OUT_BYTE_EN enable masks are computed
on the fly inside each write_map_ram() callback, which is the only
place that needs to know the hardware layout. The byte_mask[] field
is dropped from both driver state structs.
There is no userspace-visible ABI change. Control declarations,
ranges, initial values and handling of out-of-range writes is
preserved by treating values outside [0, 255] as disabled (256),
matching previous behavior. As a side effect each patch also fixes
a latent bug in put_byte_map() where an enabled-to-enabled value
change was not persisted.
The packed RAM word construction is also updated to ensure the shift
operates on a u32 value, avoiding potential undefined behavior due
to signed integer promotion.
Addresses TODO comments left in tegra210_{adx,amx}_get_byte_map().
Patch 1/2: ASoC: tegra210_adx: simplify byte map get/put logic
Patch 2/2: ASoC: tegra210_amx: simplify byte map get/put logic
--
2.34.1
^ permalink raw reply
* [PATCH 1/2] ASoC: tegra210_adx: simplify byte map get/put logic
From: Piyush Patle @ 2026-04-07 17:03 UTC (permalink / raw)
To: Mark Brown
Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Thierry Reding,
Jonathan Hunter, Sheetal, Kuninori Morimoto, linux-sound,
linux-tegra, linux-kernel
In-Reply-To: <20260407170308.100238-1-piyushpatle228@gmail.com>
The byte-map controls ("Byte Map N") already expose a value range of
[0, 256] to userspace via SOC_SINGLE_EXT(), where 256 is the
"disabled" sentinel. The driver stored this state as a byte-packed
u32 map[] array plus a separate byte_mask[] bitmap tracking which
slots were enabled, because 256 does not fit in a byte. As a result
get_byte_map() had to consult byte_mask[] to decide whether to
report the stored byte or 256, and put_byte_map() had to keep the
two arrays in sync on every write.
Store each slot as a u16 holding the control value directly
(0..255 enabled, 256 disabled). This is the native representation
for what userspace already sees, so get_byte_map() becomes a direct
return and put_byte_map() becomes a compare-and-store. The
hardware-facing packed RAM word and the IN_BYTE_EN mask are now
derived on the fly inside tegra210_adx_write_map_ram() from the
slot array, which is the only place that needs to know about the
hardware layout. This also lets us drop the byte_mask field from
struct tegra210_adx.
Slots are initialised to 256 in probe() so the default reported
value stays "disabled", matching previous behaviour. Values written
from userspace that fall outside [0, 255] are clamped to 256
("disabled") exactly as before -- no userspace-visible change.
As a side effect this also fixes a latent bug in the previous
put_byte_map(): because it compared the enable mask rather than the
stored byte, changing a slot from one enabled value to another
enabled value (e.g. 42 -> 99) would early-return without persisting
the new value.
Also fix a potential undefined behavior when constructing the packed
RAM word by ensuring the shift operates on a u32 value.
Addresses TODO left in tegra210_adx_get_byte_map().
Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
---
sound/soc/tegra/tegra210_adx.c | 80 ++++++++++++++++------------------
sound/soc/tegra/tegra210_adx.h | 5 ++-
2 files changed, 40 insertions(+), 45 deletions(-)
diff --git a/sound/soc/tegra/tegra210_adx.c b/sound/soc/tegra/tegra210_adx.c
index 95875c75ddf8..67948459e884 100644
--- a/sound/soc/tegra/tegra210_adx.c
+++ b/sound/soc/tegra/tegra210_adx.c
@@ -47,6 +47,7 @@ static const struct reg_default tegra264_adx_reg_defaults[] = {
static void tegra210_adx_write_map_ram(struct tegra210_adx *adx)
{
+ unsigned int byte_mask[TEGRA264_ADX_BYTE_MASK_COUNT] = { 0 };
int i;
regmap_write(adx->regmap, TEGRA210_ADX_CFG_RAM_CTRL +
@@ -55,15 +56,28 @@ static void tegra210_adx_write_map_ram(struct tegra210_adx *adx)
TEGRA210_ADX_CFG_RAM_CTRL_ADDR_INIT_EN |
TEGRA210_ADX_CFG_RAM_CTRL_RW_WRITE);
- for (i = 0; i < adx->soc_data->ram_depth; i++)
+ for (i = 0; i < adx->soc_data->ram_depth; i++) {
+ u32 word = 0;
+ int b;
+
+ for (b = 0; b < 4; b++) {
+ unsigned int slot = i * 4 + b;
+ u16 val = adx->map[slot];
+
+ if (val >= 256)
+ continue;
+
+ word |= (u32)val << (b * 8);
+ byte_mask[slot / 32] |= 1U << (slot % 32);
+ }
regmap_write(adx->regmap, TEGRA210_ADX_CFG_RAM_DATA +
- adx->soc_data->cya_offset,
- adx->map[i]);
+ adx->soc_data->cya_offset, word);
+ }
for (i = 0; i < adx->soc_data->byte_mask_size; i++)
regmap_write(adx->regmap,
TEGRA210_ADX_IN_BYTE_EN0 + (i * TEGRA210_ADX_AUDIOCIF_CH_STRIDE),
- adx->byte_mask[i]);
+ byte_mask[i]);
}
static int tegra210_adx_startup(struct snd_pcm_substream *substream,
@@ -188,27 +202,10 @@ static int tegra210_adx_get_byte_map(struct snd_kcontrol *kcontrol,
{
struct snd_soc_component *cmpnt = snd_kcontrol_chip(kcontrol);
struct tegra210_adx *adx = snd_soc_component_get_drvdata(cmpnt);
- struct soc_mixer_control *mc;
- unsigned char *bytes_map = (unsigned char *)adx->map;
- int enabled;
+ struct soc_mixer_control *mc =
+ (struct soc_mixer_control *)kcontrol->private_value;
- mc = (struct soc_mixer_control *)kcontrol->private_value;
- enabled = adx->byte_mask[mc->reg / 32] & (1 << (mc->reg % 32));
-
- /*
- * TODO: Simplify this logic to just return from bytes_map[]
- *
- * Presently below is required since bytes_map[] is
- * tightly packed and cannot store the control value of 256.
- * Byte mask state is used to know if 256 needs to be returned.
- * Note that for control value of 256, the put() call stores 0
- * in the bytes_map[] and disables the corresponding bit in
- * byte_mask[].
- */
- if (enabled)
- ucontrol->value.integer.value[0] = bytes_map[mc->reg];
- else
- ucontrol->value.integer.value[0] = 256;
+ ucontrol->value.integer.value[0] = adx->map[mc->reg];
return 0;
}
@@ -218,23 +215,22 @@ static int tegra210_adx_put_byte_map(struct snd_kcontrol *kcontrol,
{
struct snd_soc_component *cmpnt = snd_kcontrol_chip(kcontrol);
struct tegra210_adx *adx = snd_soc_component_get_drvdata(cmpnt);
- unsigned char *bytes_map = (unsigned char *)adx->map;
- int value = ucontrol->value.integer.value[0];
struct soc_mixer_control *mc =
(struct soc_mixer_control *)kcontrol->private_value;
- unsigned int mask_val = adx->byte_mask[mc->reg / 32];
+ unsigned int value = ucontrol->value.integer.value[0];
- if (value >= 0 && value <= 255)
- mask_val |= (1 << (mc->reg % 32));
- else
- mask_val &= ~(1 << (mc->reg % 32));
+ /*
+ * Match the previous behaviour: any value outside [0, 255] is
+ * treated as the "disabled" sentinel (256). Negative values from
+ * userspace fold in through the unsigned cast and are caught here.
+ */
+ if (value > 255)
+ value = 256;
- if (mask_val == adx->byte_mask[mc->reg / 32])
+ if (adx->map[mc->reg] == value)
return 0;
- /* Update byte map and slot */
- bytes_map[mc->reg] = value % 256;
- adx->byte_mask[mc->reg / 32] = mask_val;
+ adx->map[mc->reg] = value;
return 1;
}
@@ -675,7 +671,7 @@ static int tegra210_adx_platform_probe(struct platform_device *pdev)
const struct of_device_id *match;
struct tegra210_adx_soc_data *soc_data;
void __iomem *regs;
- int err;
+ int err, i;
adx = devm_kzalloc(dev, sizeof(*adx), GFP_KERNEL);
if (!adx)
@@ -700,16 +696,14 @@ static int tegra210_adx_platform_probe(struct platform_device *pdev)
regcache_cache_only(adx->regmap, true);
- adx->map = devm_kzalloc(dev, soc_data->ram_depth * sizeof(*adx->map),
- GFP_KERNEL);
+ adx->map = devm_kcalloc(dev, soc_data->ram_depth * 4,
+ sizeof(*adx->map), GFP_KERNEL);
if (!adx->map)
return -ENOMEM;
- adx->byte_mask = devm_kzalloc(dev,
- soc_data->byte_mask_size * sizeof(*adx->byte_mask),
- GFP_KERNEL);
- if (!adx->byte_mask)
- return -ENOMEM;
+ /* Initialize all byte map slots as disabled (value 256). */
+ for (i = 0; i < soc_data->ram_depth * 4; i++)
+ adx->map[i] = 256;
tegra210_adx_dais[TEGRA_ADX_IN_DAI_ID].playback.channels_max =
adx->soc_data->max_ch;
diff --git a/sound/soc/tegra/tegra210_adx.h b/sound/soc/tegra/tegra210_adx.h
index 176a4e40de0a..afe95e45458f 100644
--- a/sound/soc/tegra/tegra210_adx.h
+++ b/sound/soc/tegra/tegra210_adx.h
@@ -8,6 +8,8 @@
#ifndef __TEGRA210_ADX_H__
#define __TEGRA210_ADX_H__
+#include <linux/types.h>
+
/* Register offsets from TEGRA210_ADX*_BASE */
#define TEGRA210_ADX_RX_STATUS 0x0c
#define TEGRA210_ADX_RX_INT_STATUS 0x10
@@ -88,8 +90,7 @@ struct tegra210_adx_soc_data {
struct tegra210_adx {
struct regmap *regmap;
- unsigned int *map;
- unsigned int *byte_mask;
+ u16 *map;
const struct tegra210_adx_soc_data *soc_data;
};
--
2.34.1
^ permalink raw reply related
* [PATCH 2/2] ASoC: tegra210_amx: simplify byte map get/put logic
From: Piyush Patle @ 2026-04-07 17:03 UTC (permalink / raw)
To: Mark Brown
Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Thierry Reding,
Jonathan Hunter, Sheetal, Kuninori Morimoto, linux-sound,
linux-tegra, linux-kernel
In-Reply-To: <20260407170308.100238-1-piyushpatle228@gmail.com>
The byte-map controls ("Byte Map N") already expose a value range of
[0, 256] to userspace via SOC_SINGLE_EXT(), where 256 is the
"disabled" sentinel. The driver stored this state as a byte-packed
u32 map[] array plus a separate byte_mask[] bitmap tracking which
slots were enabled, because 256 does not fit in a byte. As a result
get_byte_map() had to consult byte_mask[] to decide whether to
report the stored byte or 256, and put_byte_map() had to keep the
two arrays in sync on every write.
Store each slot as a u16 holding the control value directly
(0..255 enabled, 256 disabled). This is the native representation
for what userspace already sees, so get_byte_map() becomes a direct
return and put_byte_map() becomes a compare-and-store. The
hardware-facing packed RAM word and the OUT_BYTE_EN mask are now
derived on the fly inside tegra210_amx_write_map_ram() from the
slot array, which is the only place that needs to know about the
hardware layout. This also lets us drop the byte_mask field from
struct tegra210_amx.
Slots are initialised to 256 in probe() so the default reported
value stays "disabled", matching previous behaviour. Values written
from userspace that fall outside [0, 255] are clamped to 256
("disabled") exactly as before -- no userspace-visible change.
As a side effect this also fixes a latent bug in the previous
put_byte_map(): because it compared the enable mask rather than the
stored byte, changing a slot from one enabled value to another
enabled value (e.g. 42 -> 99) would early-return without persisting
the new value.
Also fix a potential undefined behavior when constructing the packed
RAM word by ensuring the shift operates on a u32 value.
Addresses TODO left in tegra210_amx_get_byte_map().
Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
---
sound/soc/tegra/tegra210_amx.c | 77 ++++++++++++++++------------------
sound/soc/tegra/tegra210_amx.h | 5 ++-
2 files changed, 38 insertions(+), 44 deletions(-)
diff --git a/sound/soc/tegra/tegra210_amx.c b/sound/soc/tegra/tegra210_amx.c
index bfda82505298..4dd158e6e974 100644
--- a/sound/soc/tegra/tegra210_amx.c
+++ b/sound/soc/tegra/tegra210_amx.c
@@ -60,6 +60,7 @@ static const struct reg_default tegra264_amx_reg_defaults[] = {
static void tegra210_amx_write_map_ram(struct tegra210_amx *amx)
{
+ unsigned int byte_mask[TEGRA264_AMX_BYTE_MASK_COUNT] = { 0 };
int i;
regmap_write(amx->regmap, TEGRA210_AMX_CFG_RAM_CTRL + amx->soc_data->reg_offset,
@@ -67,14 +68,28 @@ static void tegra210_amx_write_map_ram(struct tegra210_amx *amx)
TEGRA210_AMX_CFG_RAM_CTRL_ADDR_INIT_EN |
TEGRA210_AMX_CFG_RAM_CTRL_RW_WRITE);
- for (i = 0; i < amx->soc_data->ram_depth; i++)
+ for (i = 0; i < amx->soc_data->ram_depth; i++) {
+ u32 word = 0;
+ int b;
+
+ for (b = 0; b < 4; b++) {
+ unsigned int slot = i * 4 + b;
+ u16 val = amx->map[slot];
+
+ if (val >= 256)
+ continue;
+
+ word |= (u32)val << (b * 8);
+ byte_mask[slot / 32] |= 1U << (slot % 32);
+ }
regmap_write(amx->regmap, TEGRA210_AMX_CFG_RAM_DATA + amx->soc_data->reg_offset,
- amx->map[i]);
+ word);
+ }
for (i = 0; i < amx->soc_data->byte_mask_size; i++)
regmap_write(amx->regmap,
TEGRA210_AMX_OUT_BYTE_EN0 + (i * TEGRA210_AMX_AUDIOCIF_CH_STRIDE),
- amx->byte_mask[i]);
+ byte_mask[i]);
}
static int tegra210_amx_startup(struct snd_pcm_substream *substream,
@@ -212,26 +227,8 @@ static int tegra210_amx_get_byte_map(struct snd_kcontrol *kcontrol,
struct soc_mixer_control *mc =
(struct soc_mixer_control *)kcontrol->private_value;
struct tegra210_amx *amx = snd_soc_component_get_drvdata(cmpnt);
- unsigned char *bytes_map = (unsigned char *)amx->map;
- int reg = mc->reg;
- int enabled;
- enabled = amx->byte_mask[reg / 32] & (1 << (reg % 32));
-
- /*
- * TODO: Simplify this logic to just return from bytes_map[]
- *
- * Presently below is required since bytes_map[] is
- * tightly packed and cannot store the control value of 256.
- * Byte mask state is used to know if 256 needs to be returned.
- * Note that for control value of 256, the put() call stores 0
- * in the bytes_map[] and disables the corresponding bit in
- * byte_mask[].
- */
- if (enabled)
- ucontrol->value.integer.value[0] = bytes_map[reg];
- else
- ucontrol->value.integer.value[0] = 256;
+ ucontrol->value.integer.value[0] = amx->map[mc->reg];
return 0;
}
@@ -243,22 +240,20 @@ static int tegra210_amx_put_byte_map(struct snd_kcontrol *kcontrol,
(struct soc_mixer_control *)kcontrol->private_value;
struct snd_soc_component *cmpnt = snd_kcontrol_chip(kcontrol);
struct tegra210_amx *amx = snd_soc_component_get_drvdata(cmpnt);
- unsigned char *bytes_map = (unsigned char *)amx->map;
- int reg = mc->reg;
- int value = ucontrol->value.integer.value[0];
- unsigned int mask_val = amx->byte_mask[reg / 32];
+ unsigned int value = ucontrol->value.integer.value[0];
- if (value >= 0 && value <= 255)
- mask_val |= (1 << (reg % 32));
- else
- mask_val &= ~(1 << (reg % 32));
+ /*
+ * Match the previous behaviour: any value outside [0, 255] is
+ * treated as the "disabled" sentinel (256). Negative values from
+ * userspace fold in through the unsigned cast and are caught here.
+ */
+ if (value > 255)
+ value = 256;
- if (mask_val == amx->byte_mask[reg / 32])
+ if (amx->map[mc->reg] == value)
return 0;
- /* Update byte map and slot */
- bytes_map[reg] = value % 256;
- amx->byte_mask[reg / 32] = mask_val;
+ amx->map[mc->reg] = value;
return 1;
}
@@ -727,7 +722,7 @@ static int tegra210_amx_platform_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct tegra210_amx *amx;
void __iomem *regs;
- int err;
+ int err, i;
amx = devm_kzalloc(dev, sizeof(*amx), GFP_KERNEL);
if (!amx)
@@ -750,16 +745,14 @@ static int tegra210_amx_platform_probe(struct platform_device *pdev)
regcache_cache_only(amx->regmap, true);
- amx->map = devm_kzalloc(dev, amx->soc_data->ram_depth * sizeof(*amx->map),
- GFP_KERNEL);
+ amx->map = devm_kcalloc(dev, amx->soc_data->ram_depth * 4,
+ sizeof(*amx->map), GFP_KERNEL);
if (!amx->map)
return -ENOMEM;
- amx->byte_mask = devm_kzalloc(dev,
- amx->soc_data->byte_mask_size * sizeof(*amx->byte_mask),
- GFP_KERNEL);
- if (!amx->byte_mask)
- return -ENOMEM;
+ /* Initialize all byte map slots as disabled (value 256). */
+ for (i = 0; i < amx->soc_data->ram_depth * 4; i++)
+ amx->map[i] = 256;
tegra210_amx_dais[TEGRA_AMX_OUT_DAI_ID].capture.channels_max =
amx->soc_data->max_ch;
diff --git a/sound/soc/tegra/tegra210_amx.h b/sound/soc/tegra/tegra210_amx.h
index 50a237b197ba..6df9ab0fe220 100644
--- a/sound/soc/tegra/tegra210_amx.h
+++ b/sound/soc/tegra/tegra210_amx.h
@@ -8,6 +8,8 @@
#ifndef __TEGRA210_AMX_H__
#define __TEGRA210_AMX_H__
+#include <linux/types.h>
+
/* Register offsets from TEGRA210_AMX*_BASE */
#define TEGRA210_AMX_RX_STATUS 0x0c
#define TEGRA210_AMX_RX_INT_STATUS 0x10
@@ -105,8 +107,7 @@ struct tegra210_amx_soc_data {
struct tegra210_amx {
const struct tegra210_amx_soc_data *soc_data;
- unsigned int *map;
- unsigned int *byte_mask;
+ u16 *map;
struct regmap *regmap;
};
--
2.34.1
^ permalink raw reply related
* [PATCH] gpio: tegra: fix irq_release_resources calling enable instead of disable
From: Samasth Norway Ananda @ 2026-04-07 21:02 UTC (permalink / raw)
To: linusw, brgl, thierry.reding, jonathanh
Cc: linux-gpio, linux-tegra, linux-kernel, samasth.norway.ananda
tegra_gpio_irq_release_resources() erroneously calls tegra_gpio_enable()
instead of tegra_gpio_disable(). When IRQ resources are released, the
GPIO configuration bit (CNF) should be cleared to deconfigure the pin as
a GPIO. Leaving it enabled wastes power and can cause unexpected behavior
if the pin is later reused for an alternate function via pinctrl.
Fixes: 66fecef5bde0 ("gpio: tegra: Convert to gpio_irq_chip")
Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
---
drivers/gpio/gpio-tegra.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
index 15a5762a82c2..b14052fe64ac 100644
--- a/drivers/gpio/gpio-tegra.c
+++ b/drivers/gpio/gpio-tegra.c
@@ -595,7 +595,7 @@ static void tegra_gpio_irq_release_resources(struct irq_data *d)
struct tegra_gpio_info *tgi = gpiochip_get_data(chip);
gpiochip_relres_irq(chip, d->hwirq);
- tegra_gpio_enable(tgi, d->hwirq);
+ tegra_gpio_disable(tgi, d->hwirq);
}
static void tegra_gpio_irq_print_chip(struct irq_data *d, struct seq_file *s)
--
2.50.1
^ permalink raw reply related
* Re: [PATCH 1/1] PCI: tegra194: fix min() signedness when capping ASPM L1 entrance latency
From: Manikanta Maddireddy @ 2026-04-08 5:57 UTC (permalink / raw)
To: mani
Cc: linux-pci, linux-tegra, linux-kernel, kernel test robot, robh,
krzk+dt, conor+dt, thierry.reding, jonathanh, kishon, arnd,
gregkh, Frank.Li, den, hongxing.zhu, jingoohan1, vidyas, bhelgaas,
lpieralisi, kwilczynski, cassel, 18255117159
In-Reply-To: <20260407145749.130753-1-mmaddireddy@nvidia.com>
Hi Mani,
I added W=1 argument to make command to reproduce the issue, but I am
not able reproduce the issue at my end. I made this change based on the
issue description in
https://lore.kernel.org/oe-kbuild-all/202604051407.AODe3ddZ-lkp@intel.com/,
but I couldn't verify the fix since build is working fine with or
without this change.
Thanks,
Manikanta
On 07/04/26 8:27 pm, Manikanta Maddireddy wrote:
> The DT property "aspm-l1-entry-delay-ns" is converted to microseconds,
> then encoded for the L1 entrance latency register field as ilog2(us) + 1,
> clamped to the hardware maximum of 7.
>
> ilog2() returns int type, while the upper bound is 7U (unsigned int).
> The min() macro is implemented with __careful_cmp(), which rejects mixed
> signed and unsigned operands at compile time via BUILD_BUG_ON_MSG in
> minmax.h; that check trips on this pair, notably when building with W=1.
>
> This combination fails to build (e.g. parisc allyesconfig, GCC 15, as
> reported by the 0-day bot).
>
> Use min_t(u32, ilog2(us) + 1U, 7U) so both sides of the comparison are
> unsigned and consistent with aspm_l1_enter_lat.
>
> Fixes: 4a44cd65c9dd ("PCI: tegra194: Use aspm-l1-entry-delay-ns DT property for L1 entrance latency")
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202604051407.AODe3ddZ-lkp@intel.com/
> ---
> drivers/pci/controller/dwc/pcie-tegra194.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 393f75ce3df3..93d3452ac117 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -1147,7 +1147,7 @@ static int tegra_pcie_dw_parse_dt(struct tegra_pcie_dw *pcie)
> if (!ret) {
> u32 us = max(val / 1000, 1U);
>
> - pcie->aspm_l1_enter_lat = min(ilog2(us) + 1, 7U);
> + pcie->aspm_l1_enter_lat = min_t(u32, ilog2(us) + 1U, 7U);
> }
>
> ret = of_property_read_u32(np, "num-lanes", &pcie->num_lanes);
--
nvpublic
^ permalink raw reply
* Re: [PATCH] iommu: Always fill in gather when unmapping
From: Jon Hunter @ 2026-04-08 8:42 UTC (permalink / raw)
To: Jason Gunthorpe, Robin Murphy
Cc: Alexandre Ghiti, AngeloGioacchino Del Regno, Albert Ou, asahi,
Baolin Wang, iommu, Janne Grunau, Jernej Skrabec, Joerg Roedel,
Jean-Philippe Brucker, linux-arm-kernel, linux-mediatek,
linux-riscv, linux-sunxi, Matthias Brugger, Neal Gompa,
Orson Zhai, Palmer Dabbelt, Paul Walmsley, Samuel Holland,
Sven Peter, virtualization, Chen-Yu Tsai, Will Deacon, Yong Wu,
Chunyan Zhang, Lu Baolu, Janusz Krzysztofik, Joerg Roedel,
patches, Samiullah Khawaja, stable, Vasant Hegde,
linux-tegra@vger.kernel.org
In-Reply-To: <20260402225121.GI310919@nvidia.com>
Hi Robin, Jason,
On 02/04/2026 23:51, Jason Gunthorpe wrote:
> On Thu, Apr 02, 2026 at 07:11:13PM +0100, Robin Murphy wrote:
>>>> @@ -2714,6 +2714,10 @@ static size_t __iommu_unmap(struct iommu_domain *domain,
>>>> pr_debug("unmapped: iova 0x%lx size 0x%zx\n",
>>>> iova, unmapped_page);
>>>> + /* If the driver itself isn't using the gather, mark it used */
>>>> + if (iotlb_gather->end <= iotlb_gather->start)
>>>> + iommu_iotlb_gather_add_range(&iotlb_gather, iova, unmapped_page);
>>>
>>> The gathers can be joined across unmaps and now we are inviting subtly
>>> ill-formed gathers as only the first unmap will get included.
>
>>> We do have error cases where the gather is legitimately empty, and
>>> this would squash that, it probably needs to check unmapped_page for 0
>>> too, at least.
>>
>> Maybe try looking at the rest of the code around these lines...
>
> Okay, well lets do this one, do you want to send it since it is your
> idea?
Any update on this? Boot is still broken on a couple of our boards.
Thanks
Jon
--
nvpublic
^ permalink raw reply
* Re: [PATCH 1/1] PCI: tegra194: fix min() signedness when capping ASPM L1 entrance latency
From: David Laight @ 2026-04-08 11:58 UTC (permalink / raw)
To: Manikanta Maddireddy
Cc: bhelgaas, lpieralisi, kwilczynski, mani, robh, krzk+dt, conor+dt,
thierry.reding, jonathanh, kishon, arnd, gregkh, Frank.Li, den,
hongxing.zhu, jingoohan1, vidyas, cassel, 18255117159, linux-pci,
linux-tegra, linux-kernel, kernel test robot
In-Reply-To: <20260407145749.130753-1-mmaddireddy@nvidia.com>
On Tue, 7 Apr 2026 20:27:49 +0530
Manikanta Maddireddy <mmaddireddy@nvidia.com> wrote:
> The DT property "aspm-l1-entry-delay-ns" is converted to microseconds,
> then encoded for the L1 entrance latency register field as ilog2(us) + 1,
> clamped to the hardware maximum of 7.
>
> ilog2() returns int type, while the upper bound is 7U (unsigned int).
> The min() macro is implemented with __careful_cmp(), which rejects mixed
> signed and unsigned operands at compile time via BUILD_BUG_ON_MSG in
> minmax.h; that check trips on this pair, notably when building with W=1.
>
> This combination fails to build (e.g. parisc allyesconfig, GCC 15, as
> reported by the 0-day bot).
>
> Use min_t(u32, ilog2(us) + 1U, 7U) so both sides of the comparison are
> unsigned and consistent with aspm_l1_enter_lat.
Adding 1U (rather than 1) is enough to make everything signed.
Alternatively change the 7U to 7 and it will all be fine regardless of
whether ilog2() returns a signed or unsigned result.
Remember min_t(u32, x, y) is min((u32)x, (u32)y) and you wouldn't put in
casts like that for any other arithmetic operation.
Note that for the compile to fail there has to be a code path where
ilog2(us) isn't known to generate a non-negative value.
ilog2(us) (probably) ends up as 'fls(us) - 1'. If that is implemented using a
compiler builtin (because there is a single instruction) then gcc knows that
the input can't be zero (from the max()), so knows that fls() can't return 0
(which it does for 0), so knows it is never negative and the checks in min()
pass.
parisc may be one of the architectures that ends up with a real function
for fls() so the compiler doesn't know the result of ilog2() is
non-negative.
Just delete the U.
David
>
> Fixes: 4a44cd65c9dd ("PCI: tegra194: Use aspm-l1-entry-delay-ns DT property for L1 entrance latency")
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202604051407.AODe3ddZ-lkp@intel.com/
> ---
> drivers/pci/controller/dwc/pcie-tegra194.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 393f75ce3df3..93d3452ac117 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -1147,7 +1147,7 @@ static int tegra_pcie_dw_parse_dt(struct tegra_pcie_dw *pcie)
> if (!ret) {
> u32 us = max(val / 1000, 1U);
>
> - pcie->aspm_l1_enter_lat = min(ilog2(us) + 1, 7U);
> + pcie->aspm_l1_enter_lat = min_t(u32, ilog2(us) + 1U, 7U);
> }
>
> ret = of_property_read_u32(np, "num-lanes", &pcie->num_lanes);
^ permalink raw reply
* Re: [PATCH 0/3] slab: support memoryless nodes with sheaves
From: Jon Hunter @ 2026-04-08 13:04 UTC (permalink / raw)
To: Vlastimil Babka (SUSE), Ming Lei
Cc: Harry Yoo, Hao Li, Andrew Morton, Christoph Lameter,
David Rientjes, Roman Gushchin, linux-mm, linux-kernel,
linux-tegra@vger.kernel.org
In-Reply-To: <8ab58ecb-1fc1-42a1-b67a-c3107de2ece4@kernel.org>
Hi Vlastimil,
On 11/03/2026 17:22, Vlastimil Babka (SUSE) wrote:
> On 3/11/26 10:49, Ming Lei wrote:
>> On Wed, Mar 11, 2026 at 09:25:54AM +0100, Vlastimil Babka (SUSE) wrote:
>>> This is the draft patch from [1] turned into a proper series with
>>> incremental changes. It's based on v7.0-rc3. It's too intrusive for a
>>> 7.0 hotfix, so we'll only be able to fix/reduce the regression in 7.1. I
>>> hope it's acceptable given it's a non-standard configuration, 7.0 is not
>>> a LTS, and it's a perf regression, not functionality.
>>>
>>> Ming can you please retest this on top of v7.0-rc3, which already has
>>> fb1091febd66 ("mm/slab: allow sheaf refill if blocking is not
>>> allowed"). Separate data point for v7.0-rc3 could be also useful.
>>>
>>> [1] https://lore.kernel.org/all/c6a01f7e-c6eb-454b-9b9e-734526dd659d@kernel.org/
>>>
>>> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
>>> ---
>>> Vlastimil Babka (SUSE) (3):
>>> slab: decouple pointer to barn from kmem_cache_node
>>> slab: create barns for online memoryless nodes
>>> slab: free remote objects to sheaves on memoryless nodes
>>
>> Hi Vlastimil and Guys,
>>
>> I re-run the test case used in https://lore.kernel.org/all/aZ0SbIqaIkwoW2mB@fedora/
>>
>> - v6.19-rc5: 34M
>>
>> - 815c8e35511d Merge branch 'slab/for-7.0/sheaves' into slab/for-next: 13M
>>
>> - v7.0-rc3: 13M
>
> Thanks, that's in line with your previous testing of "mm/slab: allow sheaf
> refill if blocking is not allowed" making no difference here. At least we
> just learned it helps other benchmarks :)
>
>> - v7.0-rc3 + the three patches: 24M
>
> OK. So now it might be really the total per-cpu caching capacity difference.
I have also observed a performance regresssion for Linux v7.0-rc for
some graphics related tests we run. I bisected to ...
# first bad commit: [e47c897a29491ade20b27612fdd3107c39a07357] slab: add
sheaves to most caches
I came across Ming's report and hence, found this series. I have also
tested the 3 patches in this series and it did appear to help with one
test, but overall I am still seeing a ~25% performance regression (the
tests are taking about 25% longer to run). I am not the owner or author
of these specific tests and I have not dived into see exactly what is
taking longer, but I just know they are taking longer to run.
Anyway, I have not seen any recent updates on this, and so I am not sure
if there are any other updates or what the current status of this is?
If there are any more patches available I will be happy to test.
Thanks!
Jon
--
nvpublic
^ permalink raw reply
* Re: [PATCH v4 1/7] dt-bindings: pwm: Document Tegra264 controller
From: Rob Herring (Arm) @ 2026-04-08 13:27 UTC (permalink / raw)
To: Mikko Perttunen
Cc: Thierry Reding, Uwe Kleine-König, Krzysztof Kozlowski,
devicetree, linux-kernel, linux-tegra, Conor Dooley, linux-pwm,
Thierry Reding, Jonathan Hunter
In-Reply-To: <20260331-t264-pwm-v4-1-c041659677cf@nvidia.com>
On Tue, 31 Mar 2026 11:12:13 +0900, Mikko Perttunen wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Add a new compatible string for the PWM controller found on Tegra264.
> The controller is similar to earlier generations but not compatible
> with them.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> [mperttunen: Drop extra Tegra194 compatible string]
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
> Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH] iommu: Always fill in gather when unmapping
From: Jon Hunter @ 2026-04-08 13:34 UTC (permalink / raw)
To: Jason Gunthorpe, Robin Murphy
Cc: Alexandre Ghiti, AngeloGioacchino Del Regno, Albert Ou, asahi,
Baolin Wang, iommu, Janne Grunau, Jernej Skrabec, Joerg Roedel,
Jean-Philippe Brucker, linux-arm-kernel, linux-mediatek,
linux-riscv, linux-sunxi, Matthias Brugger, Neal Gompa,
Orson Zhai, Palmer Dabbelt, Paul Walmsley, Samuel Holland,
Sven Peter, virtualization, Chen-Yu Tsai, Will Deacon, Yong Wu,
Chunyan Zhang, Lu Baolu, Janusz Krzysztofik, Joerg Roedel,
patches, Samiullah Khawaja, stable, Vasant Hegde,
linux-tegra@vger.kernel.org
In-Reply-To: <94576121-d4ff-47fd-9ff8-2a00ff4c5c2a@nvidia.com>
On 08/04/2026 09:42, Jon Hunter wrote:
> Hi Robin, Jason,
>
> On 02/04/2026 23:51, Jason Gunthorpe wrote:
>> On Thu, Apr 02, 2026 at 07:11:13PM +0100, Robin Murphy wrote:
>>>>> @@ -2714,6 +2714,10 @@ static size_t __iommu_unmap(struct
>>>>> iommu_domain *domain,
>>>>> pr_debug("unmapped: iova 0x%lx size 0x%zx\n",
>>>>> iova, unmapped_page);
>>>>> + /* If the driver itself isn't using the gather, mark it
>>>>> used */
>>>>> + if (iotlb_gather->end <= iotlb_gather->start)
>>>>> + iommu_iotlb_gather_add_range(&iotlb_gather, iova,
>>>>> unmapped_page);
>>>>
>>>> The gathers can be joined across unmaps and now we are inviting subtly
>>>> ill-formed gathers as only the first unmap will get included.
>>
>>>> We do have error cases where the gather is legitimately empty, and
>>>> this would squash that, it probably needs to check unmapped_page for 0
>>>> too, at least.
>>>
>>> Maybe try looking at the rest of the code around these lines...
>>
>> Okay, well lets do this one, do you want to send it since it is your
>> idea?
>
>
> Any update on this? Boot is still broken on a couple of our boards.
I just noticed that this is now broken on mainline as well as -next. Can
we get a fix in place before v7.0 is released?
Thanks
Jon
--
nvpublic
^ permalink raw reply
* [PATCH] spi: tegra210-quad: Fix false positive WARN on interrupt timeout with transfer complete
From: Breno Leitao @ 2026-04-08 13:45 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Laxman Dewangan, Mark Brown
Cc: linux-tegra, linux-spi, linux-kernel, song, kernel-team,
Breno Leitao
The WARN_ON_ONCE/WARN_ON fired unconditionally on any completion
timeout, including the recoverable case where the interrupt was lost but
the hardware actually finished the transfer. This produced a noisy splat
with a full call trace even though the driver successfully recovered via
tegra_qspi_handle_timeout().
Since tegra210 uses threaded interrupts, the transfer completion can be
signaled before the interrupt fires, making this false positive case
common in practice.
Almost all the hosts I sysadmin in my fleet produce the following splat:
WARNING: CPU: 47 PID: 844 at drivers/spi/spi-tegra210-quad.c:1226 tegra_qspi_transfer_one_message+0x8a4/0xba8
....
tegra-qspi NVDA1513:00: QSPI interrupt timeout, but transfer complete
Move WARN_ON_ONCE/WARN_ON to fire only on real unrecoverable timeouts,
i.e., when tegra_qspi_handle_timeout() confirms the hardware did NOT
complete. This makes the warning actionable instead of just polluting
the metrics.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
drivers/spi/spi-tegra210-quad.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index 7cca5578eba31..db28dd556484b 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -1223,7 +1223,7 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
(&tqspi->xfer_completion,
QSPI_DMA_TIMEOUT);
- if (WARN_ON_ONCE(ret == 0)) {
+ if (ret == 0) {
/*
* Check if hardware completed the transfer
* even though interrupt was lost or delayed.
@@ -1232,6 +1232,7 @@ static int tegra_qspi_combined_seq_xfer(struct tegra_qspi *tqspi,
ret = tegra_qspi_handle_timeout(tqspi);
if (ret < 0) {
/* Real timeout - clean up and fail */
+ WARN_ON_ONCE(1);
dev_err(tqspi->dev, "transfer timeout\n");
/* Abort transfer by resetting pio/dma bit */
@@ -1340,7 +1341,7 @@ static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
ret = wait_for_completion_timeout(&tqspi->xfer_completion,
QSPI_DMA_TIMEOUT);
- if (WARN_ON(ret == 0)) {
+ if (ret == 0) {
/*
* Check if hardware completed the transfer even though
* interrupt was lost or delayed. If so, process the
@@ -1349,6 +1350,7 @@ static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
ret = tegra_qspi_handle_timeout(tqspi);
if (ret < 0) {
/* Real timeout - clean up and fail */
+ WARN_ON(1);
dev_err(tqspi->dev, "transfer timeout\n");
if (tqspi->is_curr_dma_xfer)
---
base-commit: f3e6330d7fe42b204af05a2dbc68b379e0ad179e
change-id: 20260408-tegra_warn-cecd2ca3a61b
Best regards,
--
Breno Leitao <leitao@debian.org>
^ permalink raw reply related
* Re: [PATCH 0/3] slab: support memoryless nodes with sheaves
From: Hao Li @ 2026-04-08 14:06 UTC (permalink / raw)
To: Jon Hunter
Cc: Vlastimil Babka (SUSE), Ming Lei, Harry Yoo, Andrew Morton,
Christoph Lameter, David Rientjes, Roman Gushchin, linux-mm,
linux-kernel, linux-tegra@vger.kernel.org
In-Reply-To: <e7e4ea66-49cc-4996-a638-25f6cb08a42d@nvidia.com>
On Wed, Apr 08, 2026 at 02:04:54PM +0100, Jon Hunter wrote:
> Hi Vlastimil,
>
> On 11/03/2026 17:22, Vlastimil Babka (SUSE) wrote:
> > On 3/11/26 10:49, Ming Lei wrote:
> > > On Wed, Mar 11, 2026 at 09:25:54AM +0100, Vlastimil Babka (SUSE) wrote:
> > > > This is the draft patch from [1] turned into a proper series with
> > > > incremental changes. It's based on v7.0-rc3. It's too intrusive for a
> > > > 7.0 hotfix, so we'll only be able to fix/reduce the regression in 7.1. I
> > > > hope it's acceptable given it's a non-standard configuration, 7.0 is not
> > > > a LTS, and it's a perf regression, not functionality.
> > > >
> > > > Ming can you please retest this on top of v7.0-rc3, which already has
> > > > fb1091febd66 ("mm/slab: allow sheaf refill if blocking is not
> > > > allowed"). Separate data point for v7.0-rc3 could be also useful.
> > > >
> > > > [1] https://lore.kernel.org/all/c6a01f7e-c6eb-454b-9b9e-734526dd659d@kernel.org/
> > > >
> > > > Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> > > > ---
> > > > Vlastimil Babka (SUSE) (3):
> > > > slab: decouple pointer to barn from kmem_cache_node
> > > > slab: create barns for online memoryless nodes
> > > > slab: free remote objects to sheaves on memoryless nodes
> > >
> > > Hi Vlastimil and Guys,
> > >
> > > I re-run the test case used in https://lore.kernel.org/all/aZ0SbIqaIkwoW2mB@fedora/
> > >
> > > - v6.19-rc5: 34M
> > >
> > > - 815c8e35511d Merge branch 'slab/for-7.0/sheaves' into slab/for-next: 13M
> > >
> > > - v7.0-rc3: 13M
> >
> > Thanks, that's in line with your previous testing of "mm/slab: allow sheaf
> > refill if blocking is not allowed" making no difference here. At least we
> > just learned it helps other benchmarks :)
> >
> > > - v7.0-rc3 + the three patches: 24M
> >
> > OK. So now it might be really the total per-cpu caching capacity difference.
>
>
> I have also observed a performance regresssion for Linux v7.0-rc for some
> graphics related tests we run. I bisected to ...
>
> # first bad commit: [e47c897a29491ade20b27612fdd3107c39a07357] slab: add
> sheaves to most caches
Hi, Jon
Thanks for the reporting.
This first bad commit is surprising. In theory, this commit seems couldn't hurt
performance.
Could you possibly manually switch commits to verify this bad commit again,
without using git bisect?
>
> I came across Ming's report and hence, found this series. I have also tested
> the 3 patches in this series and it did appear to help with one test, but
> overall I am still seeing a ~25% performance regression (the tests are
> taking about 25% longer to run). I am not the owner or author of these
> specific tests and I have not dived into see exactly what is taking longer,
> but I just know they are taking longer to run.
>
> Anyway, I have not seen any recent updates on this, and so I am not sure if
> there are any other updates or what the current status of this is?
>
> If there are any more patches available I will be happy to test.
>
> Thanks!
> Jon
>
> --
> nvpublic
--
Thanks,
Hao
^ permalink raw reply
* Re: [PATCH 2/2] ASoC: tegra210_amx: simplify byte map get/put logic
From: Sheetal . @ 2026-04-08 14:08 UTC (permalink / raw)
To: Piyush Patle, Mark Brown
Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Thierry Reding,
Jonathan Hunter, Kuninori Morimoto, linux-sound, linux-tegra,
linux-kernel
In-Reply-To: <20260407170308.100238-3-piyushpatle228@gmail.com>
On 07-04-2026 22:33, Piyush Patle wrote:
> External email: Use caution opening links or attachments
>
>
> The byte-map controls ("Byte Map N") already expose a value range of
> [0, 256] to userspace via SOC_SINGLE_EXT(), where 256 is the
> "disabled" sentinel. The driver stored this state as a byte-packed
> u32 map[] array plus a separate byte_mask[] bitmap tracking which
> slots were enabled, because 256 does not fit in a byte. As a result
> get_byte_map() had to consult byte_mask[] to decide whether to
> report the stored byte or 256, and put_byte_map() had to keep the
> two arrays in sync on every write.
>
> Store each slot as a u16 holding the control value directly
> (0..255 enabled, 256 disabled). This is the native representation
> for what userspace already sees, so get_byte_map() becomes a direct
> return and put_byte_map() becomes a compare-and-store. The
> hardware-facing packed RAM word and the OUT_BYTE_EN mask are now
> derived on the fly inside tegra210_amx_write_map_ram() from the
> slot array, which is the only place that needs to know about the
> hardware layout. This also lets us drop the byte_mask field from
> struct tegra210_amx.
>
> Slots are initialised to 256 in probe() so the default reported
> value stays "disabled", matching previous behaviour. Values written
> from userspace that fall outside [0, 255] are clamped to 256
> ("disabled") exactly as before -- no userspace-visible change.
>
> As a side effect this also fixes a latent bug in the previous
> put_byte_map(): because it compared the enable mask rather than the
> stored byte, changing a slot from one enabled value to another
> enabled value (e.g. 42 -> 99) would early-return without persisting
> the new value.
>
> Also fix a potential undefined behavior when constructing the packed
> RAM word by ensuring the shift operates on a u32 value.
>
> Addresses TODO left in tegra210_amx_get_byte_map().
>
> Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
> ---
> sound/soc/tegra/tegra210_amx.c | 77 ++++++++++++++++------------------
> sound/soc/tegra/tegra210_amx.h | 5 ++-
> 2 files changed, 38 insertions(+), 44 deletions(-)
>
> diff --git a/sound/soc/tegra/tegra210_amx.c b/sound/soc/tegra/tegra210_amx.c
> index bfda82505298..4dd158e6e974 100644
> --- a/sound/soc/tegra/tegra210_amx.c
> +++ b/sound/soc/tegra/tegra210_amx.c
> @@ -60,6 +60,7 @@ static const struct reg_default tegra264_amx_reg_defaults[] = {
>
> static void tegra210_amx_write_map_ram(struct tegra210_amx *amx)
> {
> + unsigned int byte_mask[TEGRA264_AMX_BYTE_MASK_COUNT] = { 0 };
byte_mask[] is sized to the chip-specific TEGRA264_AMX_BYTE_MASK_COUNT,
but the map array in probe() is already dynamically sized from
soc_data. Since soc_data->byte_mask_size is available here, kcalloc()
would be consistent and avoid coupling to a specific SoC variant's constant.
> int i;
>
> regmap_write(amx->regmap, TEGRA210_AMX_CFG_RAM_CTRL + amx->soc_data->reg_offset,
> @@ -67,14 +68,28 @@ static void tegra210_amx_write_map_ram(struct tegra210_amx *amx)
> TEGRA210_AMX_CFG_RAM_CTRL_ADDR_INIT_EN |
> TEGRA210_AMX_CFG_RAM_CTRL_RW_WRITE);
>
> - for (i = 0; i < amx->soc_data->ram_depth; i++)
> + for (i = 0; i < amx->soc_data->ram_depth; i++) {
> + u32 word = 0;
> + int b;
> +
> + for (b = 0; b < 4; b++) {
> + unsigned int slot = i * 4 + b;
> + u16 val = amx->map[slot];
> +
> + if (val >= 256)
> + continue;
> +
> + word |= (u32)val << (b * 8);
The literal '4' (bytes per RAM word) and '8' (bits per byte) are magic
numbers scattered through the code here and in probe function. Please
consider defining:
#define TEGRA_AMX_SLOTS_PER_WORD 4
and using BITS_PER_BYTE from <linux/bits.h> for the shift.
> + byte_mask[slot / 32] |= 1U << (slot % 32);
> + }
> regmap_write(amx->regmap, TEGRA210_AMX_CFG_RAM_DATA + amx->soc_data->reg_offset,
> - amx->map[i]);
> + word);
> + }
>
> for (i = 0; i < amx->soc_data->byte_mask_size; i++)
> regmap_write(amx->regmap,
> TEGRA210_AMX_OUT_BYTE_EN0 + (i * TEGRA210_AMX_AUDIOCIF_CH_STRIDE),
> - amx->byte_mask[i]);
> + byte_mask[i]);
> }
>
> static int tegra210_amx_startup(struct snd_pcm_substream *substream,
> @@ -212,26 +227,8 @@ static int tegra210_amx_get_byte_map(struct snd_kcontrol *kcontrol,
> struct soc_mixer_control *mc =
> (struct soc_mixer_control *)kcontrol->private_value;
> struct tegra210_amx *amx = snd_soc_component_get_drvdata(cmpnt);
> - unsigned char *bytes_map = (unsigned char *)amx->map;
> - int reg = mc->reg;
> - int enabled;
>
> - enabled = amx->byte_mask[reg / 32] & (1 << (reg % 32));
> -
> - /*
> - * TODO: Simplify this logic to just return from bytes_map[]
> - *
> - * Presently below is required since bytes_map[] is
> - * tightly packed and cannot store the control value of 256.
> - * Byte mask state is used to know if 256 needs to be returned.
> - * Note that for control value of 256, the put() call stores 0
> - * in the bytes_map[] and disables the corresponding bit in
> - * byte_mask[].
> - */
> - if (enabled)
> - ucontrol->value.integer.value[0] = bytes_map[reg];
> - else
> - ucontrol->value.integer.value[0] = 256;
> + ucontrol->value.integer.value[0] = amx->map[mc->reg];
>
> return 0;
> }
> @@ -243,22 +240,20 @@ static int tegra210_amx_put_byte_map(struct snd_kcontrol *kcontrol,
> (struct soc_mixer_control *)kcontrol->private_value;
> struct snd_soc_component *cmpnt = snd_kcontrol_chip(kcontrol);
> struct tegra210_amx *amx = snd_soc_component_get_drvdata(cmpnt);
> - unsigned char *bytes_map = (unsigned char *)amx->map;
> - int reg = mc->reg;
> - int value = ucontrol->value.integer.value[0];
> - unsigned int mask_val = amx->byte_mask[reg / 32];
> + unsigned int value = ucontrol->value.integer.value[0];
>
> - if (value >= 0 && value <= 255)
> - mask_val |= (1 << (reg % 32));
> - else
> - mask_val &= ~(1 << (reg % 32));
> + /*
> + * Match the previous behaviour: any value outside [0, 255] is
> + * treated as the "disabled" sentinel (256). Negative values from
> + * userspace fold in through the unsigned cast and are caught here.
> + */
> + if (value > 255)
> + value = 256;
>
> - if (mask_val == amx->byte_mask[reg / 32])
> + if (amx->map[mc->reg] == value)
> return 0;
>
> - /* Update byte map and slot */
> - bytes_map[reg] = value % 256;
> - amx->byte_mask[reg / 32] = mask_val;
> + amx->map[mc->reg] = value;
>
> return 1;
> }
> @@ -727,7 +722,7 @@ static int tegra210_amx_platform_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct tegra210_amx *amx;
> void __iomem *regs;
> - int err;
> + int err, i;
>
> amx = devm_kzalloc(dev, sizeof(*amx), GFP_KERNEL);
> if (!amx)
> @@ -750,16 +745,14 @@ static int tegra210_amx_platform_probe(struct platform_device *pdev)
>
> regcache_cache_only(amx->regmap, true);
>
> - amx->map = devm_kzalloc(dev, amx->soc_data->ram_depth * sizeof(*amx->map),
> - GFP_KERNEL);
> + amx->map = devm_kcalloc(dev, amx->soc_data->ram_depth * 4,
> + sizeof(*amx->map), GFP_KERNEL);
> if (!amx->map)
> return -ENOMEM;
>
> - amx->byte_mask = devm_kzalloc(dev,
> - amx->soc_data->byte_mask_size * sizeof(*amx->byte_mask),
> - GFP_KERNEL);
> - if (!amx->byte_mask)
> - return -ENOMEM;
> + /* Initialize all byte map slots as disabled (value 256). */
> + for (i = 0; i < amx->soc_data->ram_depth * 4; i++)
> + amx->map[i] = 256;
>
> tegra210_amx_dais[TEGRA_AMX_OUT_DAI_ID].capture.channels_max =
> amx->soc_data->max_ch;
> diff --git a/sound/soc/tegra/tegra210_amx.h b/sound/soc/tegra/tegra210_amx.h
> index 50a237b197ba..6df9ab0fe220 100644
> --- a/sound/soc/tegra/tegra210_amx.h
> +++ b/sound/soc/tegra/tegra210_amx.h
> @@ -8,6 +8,8 @@
> #ifndef __TEGRA210_AMX_H__
> #define __TEGRA210_AMX_H__
>
> +#include <linux/types.h>
> +
> /* Register offsets from TEGRA210_AMX*_BASE */
> #define TEGRA210_AMX_RX_STATUS 0x0c
> #define TEGRA210_AMX_RX_INT_STATUS 0x10
> @@ -105,8 +107,7 @@ struct tegra210_amx_soc_data {
>
> struct tegra210_amx {
> const struct tegra210_amx_soc_data *soc_data;
> - unsigned int *map;
> - unsigned int *byte_mask;
> + u16 *map;
> struct regmap *regmap;
> };
>
> --
> 2.34.1
>
Same comments apply to the ADX patch (patch 1/2) as well.
Thanks,
Sheetal
^ permalink raw reply
* Re: [PATCH 0/3] slab: support memoryless nodes with sheaves
From: Harry Yoo (Oracle) @ 2026-04-08 14:31 UTC (permalink / raw)
To: Jon Hunter
Cc: Vlastimil Babka (SUSE), Ming Lei, Hao Li, Andrew Morton,
Christoph Lameter, David Rientjes, Roman Gushchin, linux-mm,
linux-kernel, linux-tegra@vger.kernel.org
In-Reply-To: <e7e4ea66-49cc-4996-a638-25f6cb08a42d@nvidia.com>
On Wed, Apr 08, 2026 at 02:04:54PM +0100, Jon Hunter wrote:
> Hi Vlastimil,
Hi Jon,
> On 11/03/2026 17:22, Vlastimil Babka (SUSE) wrote:
> > On 3/11/26 10:49, Ming Lei wrote:
> > > On Wed, Mar 11, 2026 at 09:25:54AM +0100, Vlastimil Babka (SUSE) wrote:
> > > > This is the draft patch from [1] turned into a proper series with
> > > > incremental changes. It's based on v7.0-rc3. It's too intrusive for a
> > > > 7.0 hotfix, so we'll only be able to fix/reduce the regression in 7.1. I
> > > > hope it's acceptable given it's a non-standard configuration, 7.0 is not
> > > > a LTS, and it's a perf regression, not functionality.
> > > >
> > > > Ming can you please retest this on top of v7.0-rc3, which already has
> > > > fb1091febd66 ("mm/slab: allow sheaf refill if blocking is not
> > > > allowed"). Separate data point for v7.0-rc3 could be also useful.
> > > >
> > > > [1] https://lore.kernel.org/all/c6a01f7e-c6eb-454b-9b9e-734526dd659d@kernel.org/
> > > >
> > > > Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> > > > ---
> > > > Vlastimil Babka (SUSE) (3):
> > > > slab: decouple pointer to barn from kmem_cache_node
> > > > slab: create barns for online memoryless nodes
> > > > slab: free remote objects to sheaves on memoryless nodes
> > >
> > > Hi Vlastimil and Guys,
> > >
> > > I re-run the test case used in https://lore.kernel.org/all/aZ0SbIqaIkwoW2mB@fedora/
> > >
> > > - v6.19-rc5: 34M
> > >
> > > - 815c8e35511d Merge branch 'slab/for-7.0/sheaves' into slab/for-next: 13M
> > >
> > > - v7.0-rc3: 13M
> >
> > Thanks, that's in line with your previous testing of "mm/slab: allow sheaf
> > refill if blocking is not allowed" making no difference here. At least we
> > just learned it helps other benchmarks :)
> >
> > > - v7.0-rc3 + the three patches: 24M
> >
> > OK. So now it might be really the total per-cpu caching capacity difference.
>
> I have also observed a performance regresssion for Linux v7.0-rc for some
> graphics related tests we run. I bisected to ...
>
> # first bad commit: [e47c897a29491ade20b27612fdd3107c39a07357] slab: add
> sheaves to most caches
>
> I came across Ming's report and hence, found this series. I have also tested
> the 3 patches in this series and it did appear to help with one test, but
> overall I am still seeing a ~25% performance regression (the tests are
> taking about 25% longer to run). I am not the owner or author of these
> specific tests and I have not dived into see exactly what is taking longer,
> but I just know they are taking longer to run.
>
> Anyway, I have not seen any recent updates on this, and so I am not sure if
> there are any other updates or what the current status of this is?
As far as I remember we didn't get to fully recovering the performance
yet. Interestingly even when most of allocations go through the fastpath
it didn't fully recover [1].
[1] https://lore.kernel.org/all/abI9DKxuwl_4Gasj@hyeyoo
I was suspecting it's probably because of:
- false sharing on something (sheaves, obj metadata, etc.), or
- suboptimal NUMA placement, or
- something outside slab involved
But I don't have enough data to back up any of these theories yet.
> If there are any more patches available I will be happy to test.
Thanks!
Before diving deeper, could you please share the NUMA topology from
`numactl -H` on your machine?
It's probably a NUMA machine? (and hopefully not memoryless ones!)
--
Cheers,
Harry / Hyeonggon
^ permalink raw reply
* [PATCH] iommu: Ensure .iotlb_sync is called correctly
From: Robin Murphy @ 2026-04-08 14:40 UTC (permalink / raw)
To: joro, will; +Cc: jgg, linux, iommu, linux-tegra, Jon Hunter
Many drivers have no reason to use the iotlb_gather mechanism, but do
still depend on .iotlb_sync being called to properly complete an unmap.
Since the core code is now relying on the gather to detect when there
is legitimately something to sync, it should also take care of encoding
a successful unmap when the driver does not touch the gather itself.
Fixes: 90c5def10bea ("iommu: Do not call drivers for empty gathers")
Reported-by: Jon Hunter <jonathanh@nvidia.com>
Closes: https://lore.kernel.org/r/8800a38b-8515-4bbe-af15-0dae81274bf7@nvidia.com
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/iommu/iommu.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 50718ab810a4..ee83850c7060 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2717,6 +2717,12 @@ static size_t __iommu_unmap(struct iommu_domain *domain,
pr_debug("unmapped: iova 0x%lx size 0x%zx\n",
iova, unmapped_page);
+ /*
+ * If the driver itself isn't using the gather, make sure
+ * it looks non-empty so iotlb_sync will still be called.
+ */
+ if (iotlb_gather->start >= iotlb_gather->end)
+ iommu_iotlb_gather_add_range(iotlb_gather, iova, size);
iova += unmapped_page;
unmapped += unmapped_page;
--
2.39.2.101.g768bb238c484.dirty
^ permalink raw reply related
* Re: [PATCH] iommu: Ensure .iotlb_sync is called correctly
From: Jason Gunthorpe @ 2026-04-08 16:28 UTC (permalink / raw)
To: Robin Murphy; +Cc: joro, will, linux, iommu, linux-tegra, Jon Hunter
In-Reply-To: <8982e3412563cf91e106d59228dfb6115024c75e.1775659257.git.robin.murphy@arm.com>
On Wed, Apr 08, 2026 at 03:40:57PM +0100, Robin Murphy wrote:
> Many drivers have no reason to use the iotlb_gather mechanism, but do
> still depend on .iotlb_sync being called to properly complete an unmap.
> Since the core code is now relying on the gather to detect when there
> is legitimately something to sync, it should also take care of encoding
> a successful unmap when the driver does not touch the gather itself.
>
> Fixes: 90c5def10bea ("iommu: Do not call drivers for empty gathers")
> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> Closes: https://lore.kernel.org/r/8800a38b-8515-4bbe-af15-0dae81274bf7@nvidia.com
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
> drivers/iommu/iommu.c | 6 ++++++
> 1 file changed, 6 insertions(+)
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
^ permalink raw reply
* [PATCH v2 0/2] ASoC: tegra210: simplify byte map handling in ADX and AMX
From: Piyush Patle @ 2026-04-08 17:08 UTC (permalink / raw)
To: Mark Brown
Cc: Sheetal, Jonathan Hunter, Thierry Reding, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai, Kuninori Morimoto, linux-sound,
linux-tegra, linux-kernel
In-Reply-To: <20260407170308.100238-3-piyushpatle228@gmail.com>
The Tegra210 ADX and AMX drivers both keep their "Byte Map N" ALSA
control state as a byte-packed u32 map[] array along with a separate
byte_mask[] bitmap. This is because the control range exposed to
userspace is [0, 256], where 256 is the "disabled" sentinel and
does not fit in a byte, so the two arrays have to be cross-checked
on every get()/put().
This series stores each slot as a u16 holding the user-visible
value directly, turning get_byte_map() into a direct return and
put_byte_map() into a compare-and-store. The hardware-facing packed
RAM word and the IN_BYTE_EN / OUT_BYTE_EN enable masks are computed
on the fly inside each write_map_ram() callback, which is the only
place that needs to know the hardware layout. The byte_mask[] field
is dropped from both driver state structs.
There is no userspace-visible ABI change. Control declarations,
ranges, initial values and handling of out-of-range writes is
preserved by treating values outside [0, 255] as disabled (256),
matching previous behavior. As a side effect each patch also fixes
a latent bug in put_byte_map() where an enabled-to-enabled value
change was not persisted.
The packed RAM word construction is also updated to ensure the shift
operates on a u32 value, avoiding potential undefined behavior due
to signed integer promotion.
Addresses TODO comments left in tegra210_{adx,amx}_get_byte_map().
Changes since v1:
- Allocate byte_mask[] dynamically using kcalloc() based on
soc_data->byte_mask_size instead of fixed-size arrays
- Propagate -ENOMEM from write_map_ram() to callers
- Replace magic numbers with TEGRA_{ADX,AMX}_SLOTS_PER_WORD
- Use BITS_PER_BYTE and BITS_PER_TYPE() instead of literal shifts
- Add <linux/bits.h> and <linux/slab.h> includes
Patch 1/2: ASoC: tegra210_adx: simplify byte map get/put logic
Patch 2/2: ASoC: tegra210_amx: simplify byte map get/put logic
--
2.34.1
^ permalink raw reply
* [PATCH v2 1/2] ASoC: tegra210_adx: simplify byte map get/put logic
From: Piyush Patle @ 2026-04-08 17:08 UTC (permalink / raw)
To: Mark Brown
Cc: Sheetal, Jonathan Hunter, Thierry Reding, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai, Kuninori Morimoto, linux-sound,
linux-tegra, linux-kernel
In-Reply-To: <20260408170818.70322-1-piyushpatle228@gmail.com>
The byte-map controls ("Byte Map N") already expose a value range of
[0, 256] to userspace via SOC_SINGLE_EXT(), where 256 is the
"disabled" sentinel. The driver stored this state as a byte-packed
u32 map[] array plus a separate byte_mask[] bitmap tracking which
slots were enabled, because 256 does not fit in a byte. As a result
get_byte_map() had to consult byte_mask[] to decide whether to
report the stored byte or 256, and put_byte_map() had to keep the
two arrays in sync on every write.
Store each slot as a u16 holding the control value directly
(0..255 enabled, 256 disabled). This is the native representation
for what userspace already sees, so get_byte_map() becomes a direct
return and put_byte_map() becomes a compare-and-store. The
hardware-facing packed RAM word and the IN_BYTE_EN mask are now
derived on the fly inside tegra210_adx_write_map_ram() from the
slot array, which is the only place that needs to know about the
hardware layout.
The byte_mask scratch buffer is allocated dynamically using
kcalloc() based on soc_data->byte_mask_size, removing dependency
on SoC-specific constants. The byte_mask field is dropped from
struct tegra210_adx.
A new TEGRA_ADX_SLOTS_PER_WORD constant replaces the literal '4'
used for byte slots per RAM word, and BITS_PER_BYTE /
BITS_PER_TYPE() from <linux/bits.h> replace the literal '8' and
'32' shifts.
Slots are initialised to 256 in probe() so the default reported
value stays "disabled", matching previous behaviour. Values written
from userspace that fall outside [0, 255] are clamped to 256
("disabled") exactly as before -- no userspace-visible change.
As a side effect this also fixes a latent bug in the previous
put_byte_map(): because it compared the enable mask rather than the
stored byte, changing a slot from one enabled value to another
enabled value (e.g. 42 -> 99) would early-return without persisting
the new value, and the next CFG_RAM flush would still program the
old byte. The new implementation compares the stored value itself,
so this case is now handled correctly.
Addresses TODO left in tegra210_adx_get_byte_map().
Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
---
Changes since v1:
- Allocate byte_mask[] dynamically using kcalloc()
- Propagate -ENOMEM from write_map_ram()
- Replace magic numbers with TEGRA_ADX_SLOTS_PER_WORD
- Use BITS_PER_BYTE and BITS_PER_TYPE()
- Add <linux/bits.h> and <linux/slab.h>
sound/soc/tegra/tegra210_adx.c | 99 ++++++++++++++++++----------------
sound/soc/tegra/tegra210_adx.h | 6 ++-
2 files changed, 56 insertions(+), 49 deletions(-)
diff --git a/sound/soc/tegra/tegra210_adx.c b/sound/soc/tegra/tegra210_adx.c
index 95875c75ddf8..2a2f7e9e90ca 100644
--- a/sound/soc/tegra/tegra210_adx.c
+++ b/sound/soc/tegra/tegra210_adx.c
@@ -4,6 +4,7 @@
//
// tegra210_adx.c - Tegra210 ADX driver
+#include <linux/bits.h>
#include <linux/clk.h>
#include <linux/device.h>
#include <linux/io.h>
@@ -13,6 +14,7 @@
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/regmap.h>
+#include <linux/slab.h>
#include <sound/core.h>
#include <sound/pcm.h>
#include <sound/pcm_params.h>
@@ -45,25 +47,49 @@ static const struct reg_default tegra264_adx_reg_defaults[] = {
{ TEGRA264_ADX_CFG_RAM_CTRL, 0x00004000},
};
-static void tegra210_adx_write_map_ram(struct tegra210_adx *adx)
+static int tegra210_adx_write_map_ram(struct tegra210_adx *adx)
{
+ const unsigned int bits_per_mask = BITS_PER_TYPE(*adx->map) * BITS_PER_BYTE;
+ unsigned int *byte_mask;
int i;
+ byte_mask = kcalloc(adx->soc_data->byte_mask_size, sizeof(*byte_mask),
+ GFP_KERNEL);
+ if (!byte_mask)
+ return -ENOMEM;
+
regmap_write(adx->regmap, TEGRA210_ADX_CFG_RAM_CTRL +
adx->soc_data->cya_offset,
TEGRA210_ADX_CFG_RAM_CTRL_SEQ_ACCESS_EN |
TEGRA210_ADX_CFG_RAM_CTRL_ADDR_INIT_EN |
TEGRA210_ADX_CFG_RAM_CTRL_RW_WRITE);
- for (i = 0; i < adx->soc_data->ram_depth; i++)
+ for (i = 0; i < adx->soc_data->ram_depth; i++) {
+ u32 word = 0;
+ int b;
+
+ for (b = 0; b < TEGRA_ADX_SLOTS_PER_WORD; b++) {
+ unsigned int slot = i * TEGRA_ADX_SLOTS_PER_WORD + b;
+ u16 val = adx->map[slot];
+
+ if (val >= 256)
+ continue;
+
+ word |= (u32)val << (b * BITS_PER_BYTE);
+ byte_mask[slot / bits_per_mask] |= 1U << (slot % bits_per_mask);
+ }
regmap_write(adx->regmap, TEGRA210_ADX_CFG_RAM_DATA +
- adx->soc_data->cya_offset,
- adx->map[i]);
+ adx->soc_data->cya_offset, word);
+ }
for (i = 0; i < adx->soc_data->byte_mask_size; i++)
regmap_write(adx->regmap,
TEGRA210_ADX_IN_BYTE_EN0 + (i * TEGRA210_ADX_AUDIOCIF_CH_STRIDE),
- adx->byte_mask[i]);
+ byte_mask[i]);
+
+ kfree(byte_mask);
+
+ return 0;
}
static int tegra210_adx_startup(struct snd_pcm_substream *substream,
@@ -118,9 +144,7 @@ static int tegra210_adx_runtime_resume(struct device *dev)
regcache_cache_only(adx->regmap, false);
regcache_sync(adx->regmap);
- tegra210_adx_write_map_ram(adx);
-
- return 0;
+ return tegra210_adx_write_map_ram(adx);
}
static int tegra210_adx_set_audio_cif(struct snd_soc_dai *dai,
@@ -188,27 +212,10 @@ static int tegra210_adx_get_byte_map(struct snd_kcontrol *kcontrol,
{
struct snd_soc_component *cmpnt = snd_kcontrol_chip(kcontrol);
struct tegra210_adx *adx = snd_soc_component_get_drvdata(cmpnt);
- struct soc_mixer_control *mc;
- unsigned char *bytes_map = (unsigned char *)adx->map;
- int enabled;
-
- mc = (struct soc_mixer_control *)kcontrol->private_value;
- enabled = adx->byte_mask[mc->reg / 32] & (1 << (mc->reg % 32));
+ struct soc_mixer_control *mc =
+ (struct soc_mixer_control *)kcontrol->private_value;
- /*
- * TODO: Simplify this logic to just return from bytes_map[]
- *
- * Presently below is required since bytes_map[] is
- * tightly packed and cannot store the control value of 256.
- * Byte mask state is used to know if 256 needs to be returned.
- * Note that for control value of 256, the put() call stores 0
- * in the bytes_map[] and disables the corresponding bit in
- * byte_mask[].
- */
- if (enabled)
- ucontrol->value.integer.value[0] = bytes_map[mc->reg];
- else
- ucontrol->value.integer.value[0] = 256;
+ ucontrol->value.integer.value[0] = adx->map[mc->reg];
return 0;
}
@@ -218,23 +225,22 @@ static int tegra210_adx_put_byte_map(struct snd_kcontrol *kcontrol,
{
struct snd_soc_component *cmpnt = snd_kcontrol_chip(kcontrol);
struct tegra210_adx *adx = snd_soc_component_get_drvdata(cmpnt);
- unsigned char *bytes_map = (unsigned char *)adx->map;
- int value = ucontrol->value.integer.value[0];
struct soc_mixer_control *mc =
(struct soc_mixer_control *)kcontrol->private_value;
- unsigned int mask_val = adx->byte_mask[mc->reg / 32];
+ unsigned int value = ucontrol->value.integer.value[0];
- if (value >= 0 && value <= 255)
- mask_val |= (1 << (mc->reg % 32));
- else
- mask_val &= ~(1 << (mc->reg % 32));
+ /*
+ * Match the previous behaviour: any value outside [0, 255] is
+ * treated as the "disabled" sentinel (256). Negative values from
+ * userspace fold in through the unsigned cast and are caught here.
+ */
+ if (value > 255)
+ value = 256;
- if (mask_val == adx->byte_mask[mc->reg / 32])
+ if (adx->map[mc->reg] == value)
return 0;
- /* Update byte map and slot */
- bytes_map[mc->reg] = value % 256;
- adx->byte_mask[mc->reg / 32] = mask_val;
+ adx->map[mc->reg] = value;
return 1;
}
@@ -675,7 +681,7 @@ static int tegra210_adx_platform_probe(struct platform_device *pdev)
const struct of_device_id *match;
struct tegra210_adx_soc_data *soc_data;
void __iomem *regs;
- int err;
+ int err, i;
adx = devm_kzalloc(dev, sizeof(*adx), GFP_KERNEL);
if (!adx)
@@ -700,16 +706,15 @@ static int tegra210_adx_platform_probe(struct platform_device *pdev)
regcache_cache_only(adx->regmap, true);
- adx->map = devm_kzalloc(dev, soc_data->ram_depth * sizeof(*adx->map),
- GFP_KERNEL);
+ adx->map = devm_kcalloc(dev,
+ soc_data->ram_depth * TEGRA_ADX_SLOTS_PER_WORD,
+ sizeof(*adx->map), GFP_KERNEL);
if (!adx->map)
return -ENOMEM;
- adx->byte_mask = devm_kzalloc(dev,
- soc_data->byte_mask_size * sizeof(*adx->byte_mask),
- GFP_KERNEL);
- if (!adx->byte_mask)
- return -ENOMEM;
+ /* Initialise all byte map slots as disabled (value 256). */
+ for (i = 0; i < soc_data->ram_depth * TEGRA_ADX_SLOTS_PER_WORD; i++)
+ adx->map[i] = 256;
tegra210_adx_dais[TEGRA_ADX_IN_DAI_ID].playback.channels_max =
adx->soc_data->max_ch;
diff --git a/sound/soc/tegra/tegra210_adx.h b/sound/soc/tegra/tegra210_adx.h
index 176a4e40de0a..7ea623b4183b 100644
--- a/sound/soc/tegra/tegra210_adx.h
+++ b/sound/soc/tegra/tegra210_adx.h
@@ -8,6 +8,8 @@
#ifndef __TEGRA210_ADX_H__
#define __TEGRA210_ADX_H__
+#include <linux/types.h>
+
/* Register offsets from TEGRA210_ADX*_BASE */
#define TEGRA210_ADX_RX_STATUS 0x0c
#define TEGRA210_ADX_RX_INT_STATUS 0x10
@@ -61,6 +63,7 @@
#define TEGRA210_ADX_SOFT_RESET_SOFT_DEFAULT (0 << TEGRA210_ADX_SOFT_RESET_SOFT_RESET_SHIFT)
#define TEGRA210_ADX_AUDIOCIF_CH_STRIDE 4
+#define TEGRA_ADX_SLOTS_PER_WORD 4
#define TEGRA210_ADX_RAM_DEPTH 16
#define TEGRA210_ADX_MAP_STREAM_NUMBER_SHIFT 6
#define TEGRA210_ADX_MAP_WORD_NUMBER_SHIFT 2
@@ -88,8 +91,7 @@ struct tegra210_adx_soc_data {
struct tegra210_adx {
struct regmap *regmap;
- unsigned int *map;
- unsigned int *byte_mask;
+ u16 *map;
const struct tegra210_adx_soc_data *soc_data;
};
--
2.34.1
^ permalink raw reply related
* [PATCH v2 2/2] ASoC: tegra210_amx: simplify byte map get/put logic
From: Piyush Patle @ 2026-04-08 17:08 UTC (permalink / raw)
To: Mark Brown
Cc: Sheetal, Jonathan Hunter, Thierry Reding, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai, Kuninori Morimoto, linux-sound,
linux-tegra, linux-kernel
In-Reply-To: <20260408170818.70322-1-piyushpatle228@gmail.com>
The byte-map controls ("Byte Map N") already expose a value range of
[0, 256] to userspace via SOC_SINGLE_EXT(), where 256 is the
"disabled" sentinel. The driver stored this state as a byte-packed
u32 map[] array plus a separate byte_mask[] bitmap tracking which
slots were enabled, because 256 does not fit in a byte. As a result
get_byte_map() had to consult byte_mask[] to decide whether to
report the stored byte or 256, and put_byte_map() had to keep the
two arrays in sync on every write.
Store each slot as a u16 holding the control value directly
(0..255 enabled, 256 disabled). This is the native representation
for what userspace already sees, so get_byte_map() becomes a direct
return and put_byte_map() becomes a compare-and-store. The
hardware-facing packed RAM word and the OUT_BYTE_EN mask are now
derived on the fly inside tegra210_amx_write_map_ram() from the
slot array, which is the only place that needs to know about the
hardware layout.
The byte_mask scratch buffer is allocated dynamically using
kcalloc() based on soc_data->byte_mask_size, removing dependency
on SoC-specific constants. The byte_mask field is dropped from
struct tegra210_amx.
A new TEGRA_AMX_SLOTS_PER_WORD constant replaces the literal '4'
used for byte slots per RAM word, and BITS_PER_BYTE /
BITS_PER_TYPE() from <linux/bits.h> replace the literal '8' and
'32' shifts.
Slots are initialised to 256 in probe() so the default reported
value stays "disabled", matching previous behaviour. Values written
from userspace that fall outside [0, 255] are clamped to 256
("disabled") exactly as before -- no userspace-visible change.
As a side effect this also fixes a latent bug in the previous
put_byte_map(): because it compared the enable mask rather than the
stored byte, changing a slot from one enabled value to another
enabled value (e.g. 42 -> 99) would early-return without persisting
the new value, and the next CFG_RAM flush would still program the
old byte. The new implementation compares the stored value itself,
so this case is now handled correctly.
Addresses TODO left in tegra210_amx_get_byte_map().
Signed-off-by: Piyush Patle <piyushpatle228@gmail.com>
---
Changes since v1:
- Allocate byte_mask[] dynamically using kcalloc()
- Propagate -ENOMEM from write_map_ram()
- Replace magic numbers with TEGRA_AMX_SLOTS_PER_WORD
- Use BITS_PER_BYTE and BITS_PER_TYPE()
- Add <linux/bits.h> and <linux/slab.h>
sound/soc/tegra/tegra210_amx.c | 96 ++++++++++++++++++----------------
sound/soc/tegra/tegra210_amx.h | 6 ++-
2 files changed, 54 insertions(+), 48 deletions(-)
diff --git a/sound/soc/tegra/tegra210_amx.c b/sound/soc/tegra/tegra210_amx.c
index bfda82505298..bc808fabe171 100644
--- a/sound/soc/tegra/tegra210_amx.c
+++ b/sound/soc/tegra/tegra210_amx.c
@@ -4,6 +4,7 @@
//
// tegra210_amx.c - Tegra210 AMX driver
+#include <linux/bits.h>
#include <linux/clk.h>
#include <linux/device.h>
#include <linux/io.h>
@@ -12,6 +13,7 @@
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/regmap.h>
+#include <linux/slab.h>
#include <sound/core.h>
#include <sound/pcm.h>
#include <sound/pcm_params.h>
@@ -58,23 +60,48 @@ static const struct reg_default tegra264_amx_reg_defaults[] = {
{ TEGRA264_AMX_CFG_RAM_CTRL, 0x00004000},
};
-static void tegra210_amx_write_map_ram(struct tegra210_amx *amx)
+static int tegra210_amx_write_map_ram(struct tegra210_amx *amx)
{
+ const unsigned int bits_per_mask = BITS_PER_TYPE(*amx->map) * BITS_PER_BYTE;
+ unsigned int *byte_mask;
int i;
+ byte_mask = kcalloc(amx->soc_data->byte_mask_size, sizeof(*byte_mask),
+ GFP_KERNEL);
+ if (!byte_mask)
+ return -ENOMEM;
+
regmap_write(amx->regmap, TEGRA210_AMX_CFG_RAM_CTRL + amx->soc_data->reg_offset,
TEGRA210_AMX_CFG_RAM_CTRL_SEQ_ACCESS_EN |
TEGRA210_AMX_CFG_RAM_CTRL_ADDR_INIT_EN |
TEGRA210_AMX_CFG_RAM_CTRL_RW_WRITE);
- for (i = 0; i < amx->soc_data->ram_depth; i++)
+ for (i = 0; i < amx->soc_data->ram_depth; i++) {
+ u32 word = 0;
+ int b;
+
+ for (b = 0; b < TEGRA_AMX_SLOTS_PER_WORD; b++) {
+ unsigned int slot = i * TEGRA_AMX_SLOTS_PER_WORD + b;
+ u16 val = amx->map[slot];
+
+ if (val >= 256)
+ continue;
+
+ word |= (u32)val << (b * BITS_PER_BYTE);
+ byte_mask[slot / bits_per_mask] |= 1U << (slot % bits_per_mask);
+ }
regmap_write(amx->regmap, TEGRA210_AMX_CFG_RAM_DATA + amx->soc_data->reg_offset,
- amx->map[i]);
+ word);
+ }
for (i = 0; i < amx->soc_data->byte_mask_size; i++)
regmap_write(amx->regmap,
TEGRA210_AMX_OUT_BYTE_EN0 + (i * TEGRA210_AMX_AUDIOCIF_CH_STRIDE),
- amx->byte_mask[i]);
+ byte_mask[i]);
+
+ kfree(byte_mask);
+
+ return 0;
}
static int tegra210_amx_startup(struct snd_pcm_substream *substream,
@@ -134,9 +161,7 @@ static int tegra210_amx_runtime_resume(struct device *dev)
TEGRA210_AMX_CTRL_RX_DEP_MASK,
TEGRA210_AMX_WAIT_ON_ANY << TEGRA210_AMX_CTRL_RX_DEP_SHIFT);
- tegra210_amx_write_map_ram(amx);
-
- return 0;
+ return tegra210_amx_write_map_ram(amx);
}
static int tegra210_amx_set_audio_cif(struct snd_soc_dai *dai,
@@ -212,26 +237,8 @@ static int tegra210_amx_get_byte_map(struct snd_kcontrol *kcontrol,
struct soc_mixer_control *mc =
(struct soc_mixer_control *)kcontrol->private_value;
struct tegra210_amx *amx = snd_soc_component_get_drvdata(cmpnt);
- unsigned char *bytes_map = (unsigned char *)amx->map;
- int reg = mc->reg;
- int enabled;
-
- enabled = amx->byte_mask[reg / 32] & (1 << (reg % 32));
- /*
- * TODO: Simplify this logic to just return from bytes_map[]
- *
- * Presently below is required since bytes_map[] is
- * tightly packed and cannot store the control value of 256.
- * Byte mask state is used to know if 256 needs to be returned.
- * Note that for control value of 256, the put() call stores 0
- * in the bytes_map[] and disables the corresponding bit in
- * byte_mask[].
- */
- if (enabled)
- ucontrol->value.integer.value[0] = bytes_map[reg];
- else
- ucontrol->value.integer.value[0] = 256;
+ ucontrol->value.integer.value[0] = amx->map[mc->reg];
return 0;
}
@@ -243,22 +250,20 @@ static int tegra210_amx_put_byte_map(struct snd_kcontrol *kcontrol,
(struct soc_mixer_control *)kcontrol->private_value;
struct snd_soc_component *cmpnt = snd_kcontrol_chip(kcontrol);
struct tegra210_amx *amx = snd_soc_component_get_drvdata(cmpnt);
- unsigned char *bytes_map = (unsigned char *)amx->map;
- int reg = mc->reg;
- int value = ucontrol->value.integer.value[0];
- unsigned int mask_val = amx->byte_mask[reg / 32];
+ unsigned int value = ucontrol->value.integer.value[0];
- if (value >= 0 && value <= 255)
- mask_val |= (1 << (reg % 32));
- else
- mask_val &= ~(1 << (reg % 32));
+ /*
+ * Match the previous behaviour: any value outside [0, 255] is
+ * treated as the "disabled" sentinel (256). Negative values from
+ * userspace fold in through the unsigned cast and are caught here.
+ */
+ if (value > 255)
+ value = 256;
- if (mask_val == amx->byte_mask[reg / 32])
+ if (amx->map[mc->reg] == value)
return 0;
- /* Update byte map and slot */
- bytes_map[reg] = value % 256;
- amx->byte_mask[reg / 32] = mask_val;
+ amx->map[mc->reg] = value;
return 1;
}
@@ -727,7 +732,7 @@ static int tegra210_amx_platform_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct tegra210_amx *amx;
void __iomem *regs;
- int err;
+ int err, i;
amx = devm_kzalloc(dev, sizeof(*amx), GFP_KERNEL);
if (!amx)
@@ -750,16 +755,15 @@ static int tegra210_amx_platform_probe(struct platform_device *pdev)
regcache_cache_only(amx->regmap, true);
- amx->map = devm_kzalloc(dev, amx->soc_data->ram_depth * sizeof(*amx->map),
- GFP_KERNEL);
+ amx->map = devm_kcalloc(dev,
+ amx->soc_data->ram_depth * TEGRA_AMX_SLOTS_PER_WORD,
+ sizeof(*amx->map), GFP_KERNEL);
if (!amx->map)
return -ENOMEM;
- amx->byte_mask = devm_kzalloc(dev,
- amx->soc_data->byte_mask_size * sizeof(*amx->byte_mask),
- GFP_KERNEL);
- if (!amx->byte_mask)
- return -ENOMEM;
+ /* Initialise all byte map slots as disabled (value 256). */
+ for (i = 0; i < amx->soc_data->ram_depth * TEGRA_AMX_SLOTS_PER_WORD; i++)
+ amx->map[i] = 256;
tegra210_amx_dais[TEGRA_AMX_OUT_DAI_ID].capture.channels_max =
amx->soc_data->max_ch;
diff --git a/sound/soc/tegra/tegra210_amx.h b/sound/soc/tegra/tegra210_amx.h
index 50a237b197ba..2c125a191a1a 100644
--- a/sound/soc/tegra/tegra210_amx.h
+++ b/sound/soc/tegra/tegra210_amx.h
@@ -8,6 +8,8 @@
#ifndef __TEGRA210_AMX_H__
#define __TEGRA210_AMX_H__
+#include <linux/types.h>
+
/* Register offsets from TEGRA210_AMX*_BASE */
#define TEGRA210_AMX_RX_STATUS 0x0c
#define TEGRA210_AMX_RX_INT_STATUS 0x10
@@ -73,6 +75,7 @@
#define TEGRA210_AMX_SOFT_RESET_SOFT_RESET_MASK TEGRA210_AMX_SOFT_RESET_SOFT_EN
#define TEGRA210_AMX_AUDIOCIF_CH_STRIDE 4
+#define TEGRA_AMX_SLOTS_PER_WORD 4
#define TEGRA210_AMX_RAM_DEPTH 16
#define TEGRA210_AMX_MAP_STREAM_NUM_SHIFT 6
#define TEGRA210_AMX_MAP_WORD_NUM_SHIFT 2
@@ -105,8 +108,7 @@ struct tegra210_amx_soc_data {
struct tegra210_amx {
const struct tegra210_amx_soc_data *soc_data;
- unsigned int *map;
- unsigned int *byte_mask;
+ u16 *map;
struct regmap *regmap;
};
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] iommu: Ensure .iotlb_sync is called correctly
From: Jon Hunter @ 2026-04-08 17:28 UTC (permalink / raw)
To: Robin Murphy, joro, will; +Cc: jgg, linux, iommu, linux-tegra
In-Reply-To: <8982e3412563cf91e106d59228dfb6115024c75e.1775659257.git.robin.murphy@arm.com>
On 08/04/2026 15:40, Robin Murphy wrote:
> Many drivers have no reason to use the iotlb_gather mechanism, but do
> still depend on .iotlb_sync being called to properly complete an unmap.
> Since the core code is now relying on the gather to detect when there
> is legitimately something to sync, it should also take care of encoding
> a successful unmap when the driver does not touch the gather itself.
>
> Fixes: 90c5def10bea ("iommu: Do not call drivers for empty gathers")
> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> Closes: https://lore.kernel.org/r/8800a38b-8515-4bbe-af15-0dae81274bf7@nvidia.com
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
> drivers/iommu/iommu.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 50718ab810a4..ee83850c7060 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -2717,6 +2717,12 @@ static size_t __iommu_unmap(struct iommu_domain *domain,
>
> pr_debug("unmapped: iova 0x%lx size 0x%zx\n",
> iova, unmapped_page);
> + /*
> + * If the driver itself isn't using the gather, make sure
> + * it looks non-empty so iotlb_sync will still be called.
> + */
> + if (iotlb_gather->start >= iotlb_gather->end)
> + iommu_iotlb_gather_add_range(iotlb_gather, iova, size);
>
> iova += unmapped_page;
> unmapped += unmapped_page;
Tested-by: Jon Hunter <jonathanh@nvidia.com>
Thanks!
Jon
--
nvpublic
^ permalink raw reply
* Re: [PATCH v2 1/2] ASoC: tegra210_adx: simplify byte map get/put logic
From: Jon Hunter @ 2026-04-08 17:38 UTC (permalink / raw)
To: Piyush Patle, Mark Brown
Cc: Sheetal, Thierry Reding, Liam Girdwood, Jaroslav Kysela,
Takashi Iwai, Kuninori Morimoto, linux-sound, linux-tegra,
linux-kernel
In-Reply-To: <20260408170818.70322-2-piyushpatle228@gmail.com>
On 08/04/2026 18:08, Piyush Patle wrote:
> The byte-map controls ("Byte Map N") already expose a value range of
> [0, 256] to userspace via SOC_SINGLE_EXT(), where 256 is the
> "disabled" sentinel. The driver stored this state as a byte-packed
> u32 map[] array plus a separate byte_mask[] bitmap tracking which
> slots were enabled, because 256 does not fit in a byte. As a result
> get_byte_map() had to consult byte_mask[] to decide whether to
> report the stored byte or 256, and put_byte_map() had to keep the
> two arrays in sync on every write.
>
> Store each slot as a u16 holding the control value directly
> (0..255 enabled, 256 disabled). This is the native representation
> for what userspace already sees, so get_byte_map() becomes a direct
> return and put_byte_map() becomes a compare-and-store. The
> hardware-facing packed RAM word and the IN_BYTE_EN mask are now
> derived on the fly inside tegra210_adx_write_map_ram() from the
> slot array, which is the only place that needs to know about the
> hardware layout.
>
> The byte_mask scratch buffer is allocated dynamically using
> kcalloc() based on soc_data->byte_mask_size, removing dependency
> on SoC-specific constants. The byte_mask field is dropped from
> struct tegra210_adx.
So this was already the case. However ...
> -static void tegra210_adx_write_map_ram(struct tegra210_adx *adx)
> +static int tegra210_adx_write_map_ram(struct tegra210_adx *adx)
> {
> + const unsigned int bits_per_mask = BITS_PER_TYPE(*adx->map) * BITS_PER_BYTE;
> + unsigned int *byte_mask;
> int i;
>
> + byte_mask = kcalloc(adx->soc_data->byte_mask_size, sizeof(*byte_mask),
> + GFP_KERNEL);
> + if (!byte_mask)
> + return -ENOMEM;
> +
Now you are allocating this everytime this function is called (which
happens on RPM resume) instead of ...
> @@ -700,16 +706,15 @@ static int tegra210_adx_platform_probe(struct platform_device *pdev)
>
> regcache_cache_only(adx->regmap, true);
>
> - adx->map = devm_kzalloc(dev, soc_data->ram_depth * sizeof(*adx->map),
> - GFP_KERNEL);
> + adx->map = devm_kcalloc(dev,
> + soc_data->ram_depth * TEGRA_ADX_SLOTS_PER_WORD,
> + sizeof(*adx->map), GFP_KERNEL);
> if (!adx->map)
> return -ENOMEM;
>
> - adx->byte_mask = devm_kzalloc(dev,
> - soc_data->byte_mask_size * sizeof(*adx->byte_mask),
> - GFP_KERNEL);
> - if (!adx->byte_mask)
> - return -ENOMEM;
... here in the probe function, which makes more sense. IOW I am not
sure why you have changed this.
Jon
--
nvpublic
^ permalink raw reply
* Re: [PATCH v2 0/2] ASoC: tegra210: simplify byte map handling in ADX and AMX
From: Mark Brown @ 2026-04-08 17:41 UTC (permalink / raw)
To: Piyush Patle
Cc: Sheetal, Jonathan Hunter, Thierry Reding, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai, Kuninori Morimoto, linux-sound,
linux-tegra, linux-kernel
In-Reply-To: <20260408170818.70322-1-piyushpatle228@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 716 bytes --]
On Wed, Apr 08, 2026 at 10:38:16PM +0530, Piyush Patle wrote:
> The Tegra210 ADX and AMX drivers both keep their "Byte Map N" ALSA
> control state as a byte-packed u32 map[] array along with a separate
> byte_mask[] bitmap. This is because the control range exposed to
> userspace is [0, 256], where 256 is the "disabled" sentinel and
> does not fit in a byte, so the two arrays have to be cross-checked
> on every get()/put().
Please don't send new patches in reply to old patches or serieses, this
makes it harder for both people and tools to understand what is going
on - it can bury things in mailboxes and make it difficult to keep track
of what current patches are, both for the new patches and the old ones.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/2] ASoC: tegra210_adx: simplify byte map get/put logic
From: Mark Brown @ 2026-04-08 17:47 UTC (permalink / raw)
To: Piyush Patle
Cc: Sheetal, Jonathan Hunter, Thierry Reding, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai, Kuninori Morimoto, linux-sound,
linux-tegra, linux-kernel
In-Reply-To: <20260408170818.70322-2-piyushpatle228@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1128 bytes --]
On Wed, Apr 08, 2026 at 10:38:17PM +0530, Piyush Patle wrote:
> +static int tegra210_adx_write_map_ram(struct tegra210_adx *adx)
> {
> + const unsigned int bits_per_mask = BITS_PER_TYPE(*adx->map) * BITS_PER_BYTE;
Why are we multiplying by BITS_PER_BYTE here? We've got a number of
bits already from BITS_PER_TYPE().
> + for (i = 0; i < adx->soc_data->ram_depth; i++) {
> + u32 word = 0;
> + int b;
> +
> + for (b = 0; b < TEGRA_ADX_SLOTS_PER_WORD; b++) {
> + unsigned int slot = i * TEGRA_ADX_SLOTS_PER_WORD + b;
> + u16 val = adx->map[slot];
> +
> + if (val >= 256)
> + continue;
> +
> + word |= (u32)val << (b * BITS_PER_BYTE);
> + byte_mask[slot / bits_per_mask] |= 1U << (slot % bits_per_mask);
How big can bits_per_mask get?
> @@ -118,9 +144,7 @@ static int tegra210_adx_runtime_resume(struct device *dev)
> regcache_cache_only(adx->regmap, false);
> regcache_sync(adx->regmap);
>
> - tegra210_adx_write_map_ram(adx);
> -
> - return 0;
> + return tegra210_adx_write_map_ram(adx);
We need to unwind at least the regcache_cache_only() above if resume
fails.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ 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