* Re: [PATCH v9 00/13] firmware: qcom: qseecom: convert to using the TZ allocator
From: Maximilian Luz @ 2024-03-28 16:50 UTC (permalink / raw)
To: Bartosz Golaszewski, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Elliot Berman, Krzysztof Kozlowski, Guru Das Srinagesh,
Andrew Halaney, Alex Elder, Srini Kandagatla, Arnd Bergmann
Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, kernel,
Bartosz Golaszewski
In-Reply-To: <20240325100359.17001-1-brgl@bgdev.pl>
On 3/25/24 11:03 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> SCM calls that take memory buffers as arguments require that they be
> page-aligned, physically continuous and non-cachable. The same
> requirements apply to the buffer used to pass additional arguments to SCM
> calls that take more than 4.
>
> To that end drivers typically use dma_alloc_coherent() to allocate memory
> of suitable format which is slow and inefficient space-wise.
>
> SHM Bridge is a safety mechanism that - once enabled - will only allow
> passing buffers to the TrustZone that have been explicitly marked as
> shared. It improves the overall system safety with SCM calls and is
> required by the upcoming scminvoke functionality.
>
> The end goal of this series is to enable SHM bridge support for those
> architectures that support it but to that end we first need to unify the
> way memory for SCM calls is allocated. This in itself is beneficial as
> the current approach of using dma_alloc_coherent() in most places is quite
> slow.
>
> First let's add a new TZ Memory allocator that allows users to create
> dynamic memory pools of format suitable for sharing with the TrustZone.
> Make it ready for implementing multiple build-time modes.
>
> Convert all relevant drivers to using it. Add separate pools for SCM core
> and for qseecom.
>
> Finally add support for SHM bridge and make it the default mode of
> operation with the generic allocator as fallback for the platforms that
> don't support SHM bridge.
>
> Tested on db410c, RB5, sm8550-qrd. Previous iteration tested also on
> sa8775p-ride and lenovo X13s (please do retest on those platforms if you
> can).
Not sure in which version things changed (I haven't really kept up with
that, sorry), but this version (with the generic allocator selected in
the config) fails reading EFI vars on my Surface Pro X (sc8180x):
[ 2.381020] BUG: scheduling while atomic: mount/258/0x00000002
[ 2.383356] Modules linked in:
[ 2.385669] Preemption disabled at:
[ 2.385672] [<ffff800080f7868c>] qcom_tzmem_alloc+0x7c/0x224
[ 2.390325] CPU: 1 PID: 258 Comm: mount Not tainted 6.8.0-1-surface-dev #2
[ 2.392632] Hardware name: Microsoft Corporation Surface Pro X/Surface Pro X, BIOS 7.620.140 08/11/2023
[ 2.394955] Call trace:
[ 2.397269] dump_backtrace+0x94/0x114
[ 2.399583] show_stack+0x18/0x24
[ 2.401883] dump_stack_lvl+0x48/0x60
[ 2.404181] dump_stack+0x18/0x24
[ 2.406476] __schedule_bug+0x84/0xa0
[ 2.408770] __schedule+0x6f4/0x7fc
[ 2.411051] schedule+0x30/0xf0
[ 2.413323] synchronize_rcu_expedited+0x158/0x1ec
[ 2.415594] lru_cache_disable+0x28/0x74
[ 2.417853] __alloc_contig_migrate_range+0x68/0x210
[ 2.420122] alloc_contig_range+0x140/0x280
[ 2.422384] cma_alloc+0x128/0x404
[ 2.424643] cma_alloc_aligned+0x44/0x6c
[ 2.426881] dma_alloc_contiguous+0x30/0x44
[ 2.429111] __dma_direct_alloc_pages.isra.0+0x60/0x20c
[ 2.431343] dma_direct_alloc+0x6c/0x2ec
[ 2.433569] dma_alloc_attrs+0x80/0xf4
[ 2.435786] qcom_tzmem_pool_add_memory+0x8c/0x178
[ 2.438008] qcom_tzmem_alloc+0xe8/0x224
[ 2.440232] qsee_uefi_get_next_variable+0x78/0x2cc
[ 2.442443] qcuefi_get_next_variable+0x50/0x94
[ 2.444643] efivar_get_next_variable+0x20/0x2c
[ 2.446832] efivar_init+0x8c/0x29c
[ 2.449009] efivarfs_fill_super+0xd4/0xec
[ 2.451182] get_tree_single+0x74/0xbc
[ 2.453349] efivarfs_get_tree+0x18/0x24
[ 2.455513] vfs_get_tree+0x28/0xe8
[ 2.457680] vfs_cmd_create+0x5c/0xf4
[ 2.459840] __do_sys_fsconfig+0x458/0x598
[ 2.461993] __arm64_sys_fsconfig+0x24/0x30
[ 2.464143] invoke_syscall+0x48/0x110
[ 2.466281] el0_svc_common.constprop.0+0x40/0xe0
[ 2.468415] do_el0_svc+0x1c/0x28
[ 2.470546] el0_svc+0x34/0xb4
[ 2.472669] el0t_64_sync_handler+0x120/0x12c
[ 2.474793] el0t_64_sync+0x190/0x194
and subsequently
[ 2.477613] DEBUG_LOCKS_WARN_ON(val > preempt_count())
[ 2.477618] WARNING: CPU: 4 PID: 258 at kernel/sched/core.c:5889 preempt_count_sub+0x90/0xd4
[ 2.478682] Modules linked in:
[ 2.479214] CPU: 4 PID: 258 Comm: mount Tainted: G W 6.8.0-1-surface-dev #2
[ 2.479752] Hardware name: Microsoft Corporation Surface Pro X/Surface Pro X, BIOS 7.620.140 08/11/2023
[ 2.480296] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 2.480839] pc : preempt_count_sub+0x90/0xd4
[ 2.481380] lr : preempt_count_sub+0x90/0xd4
[ 2.481917] sp : ffff8000857cbb00
[ 2.482450] x29: ffff8000857cbb00 x28: ffff8000806b759c x27: 8000000000000005
[ 2.482988] x26: 0000000000000000 x25: ffff0000802cbaa0 x24: ffff0000809228b0
[ 2.483525] x23: 0000000000000000 x22: ffff800082b462f0 x21: 0000000000001000
[ 2.484062] x20: ffff80008363d000 x19: ffff000080922880 x18: fffffffffffc9660
[ 2.484600] x17: 0000000000000020 x16: 0000000000000000 x15: 0000000000000038
[ 2.485137] x14: 0000000000000000 x13: ffff800082649258 x12: 00000000000006db
[ 2.485674] x11: 0000000000000249 x10: ffff8000826fc930 x9 : ffff800082649258
[ 2.486207] x8 : 00000000ffffdfff x7 : ffff8000826f9258 x6 : 0000000000000249
[ 2.486738] x5 : 0000000000000000 x4 : 40000000ffffe249 x3 : 0000000000000000
[ 2.487267] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000841fa300
[ 2.487792] Call trace:
[ 2.488311] preempt_count_sub+0x90/0xd4
[ 2.488831] _raw_spin_unlock_irqrestore+0x1c/0x44
[ 2.489352] qcom_tzmem_alloc+0x1cc/0x224
[ 2.489873] qsee_uefi_get_next_variable+0x78/0x2cc
[ 2.490390] qcuefi_get_next_variable+0x50/0x94
[ 2.490907] efivar_get_next_variable+0x20/0x2c
[ 2.491420] efivar_init+0x8c/0x29c
[ 2.491931] efivarfs_fill_super+0xd4/0xec
[ 2.492440] get_tree_single+0x74/0xbc
[ 2.492948] efivarfs_get_tree+0x18/0x24
[ 2.493453] vfs_get_tree+0x28/0xe8
[ 2.493957] vfs_cmd_create+0x5c/0xf4
[ 2.494459] __do_sys_fsconfig+0x458/0x598
[ 2.494963] __arm64_sys_fsconfig+0x24/0x30
[ 2.495468] invoke_syscall+0x48/0x110
[ 2.495972] el0_svc_common.constprop.0+0x40/0xe0
[ 2.496475] do_el0_svc+0x1c/0x28
[ 2.496976] el0_svc+0x34/0xb4
[ 2.497475] el0t_64_sync_handler+0x120/0x12c
[ 2.497975] el0t_64_sync+0x190/0x194
[ 2.498466] ---[ end trace 0000000000000000 ]---
[ 2.507347] qcom_scm firmware:scm: qseecom: scm call failed with error -22
[ 2.507813] efivars: get_next_variable: status=8000000000000007
If I understand correctly, it enters an atomic section in
qcom_tzmem_alloc() and then tries to schedule somewhere down the line.
So this shouldn't be qseecom specific.
I should probably also say that I'm currently testing this on a patched
v6.8 kernel, so there's a chance that it's my fault. However, as far as
I understand, it enters an atomic section in qcom_tzmem_alloc() and then
later tries to expand the pool memory with dma_alloc_coherent(). Which
AFAIK is allowed to sleep with GFP_KERNEL (and I guess that that's the
issue here).
I've also tried the shmem allocator option, but that seems to get stuck
quite early at boot, before I even have usb-serial access to get any
logs. If I can find some more time, I'll try to see if I can get some
useful output for that.
Best regards,
Max
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 2/3] dmaengine: xilinx: xdma: Fix synchronization issue
From: Lizhi Hou @ 2024-03-28 16:49 UTC (permalink / raw)
To: Miquel Raynal
Cc: Louis Chauvet, Brian Xu, Raj Kumar Rampelli, Vinod Koul,
Michal Simek, Jan Kuliga, dmaengine, linux-arm-kernel,
linux-kernel, stable
In-Reply-To: <20240328012257.4a5955f2@xps-13>
On 3/27/24 17:23, Miquel Raynal wrote:
> Hi Lizhi,
>
>>> @@ -376,6 +378,8 @@ static int xdma_xfer_start(struct xdma_chan *xchan)
>>> return ret;
>>> > xchan->busy = true;
>>> + xchan->stop_requested = false;
>>> + reinit_completion(&xchan->last_interrupt);
>> If stop_requested is true, it should not start another transfer. So I would suggest to add
>>
>> if (xchan->stop_requested)
>>
>> return -ENODEV;
> Maybe -EBUSY in this case?
>
> I thought synchronize() was mandatory in-between. If that's not the
> case then indeed we need to block or error-out if a new transfer
> gets started.
Okay. It looks issue_pending is not expected between terminate_all() and
synchronize().
This check is not needed.
Thanks,
Lizhi
>
>> at the beginning of xdma_xfer_start().
>>
>> xdma_xfer_start() is protected by chan lock.
>>
>>> > return 0;
>>> }
> Thanks,
> Miquèl
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v6 0/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
From: Cristian Marussi @ 2024-03-28 16:48 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Sudeep Holla, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Linus Walleij, Dan Carpenter, linux-arm-kernel, linux-kernel,
devicetree, linux-gpio, Peng Fan, Oleksii Moisieiev
In-Reply-To: <20240323-pinctrl-scmi-v6-0-a895243257c0@nxp.com>
On Sat, Mar 23, 2024 at 08:15:13PM +0800, Peng Fan (OSS) wrote:
> This patchset is a rework from Oleksii's RFC v5 patchset
> https://lore.kernel.org/all/cover.1698353854.git.oleksii_moisieiev@epam.com/
>
Hi,
beside a few more remarks on the series, I tested this again on my setup
with the latest v3.2 changes and it works fine.
Thanks,
Cristian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v6 4/4] pinctrl: Implementation of the generic scmi-pinctrl driver
From: Cristian Marussi @ 2024-03-28 16:46 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Sudeep Holla, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Linus Walleij, Dan Carpenter, linux-arm-kernel, linux-kernel,
devicetree, linux-gpio, Peng Fan, Oleksii Moisieiev
In-Reply-To: <20240323-pinctrl-scmi-v6-4-a895243257c0@nxp.com>
On Sat, Mar 23, 2024 at 08:15:17PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> scmi-pinctrl driver implements pinctrl driver interface and using
> SCMI protocol to redirect messages from pinctrl subsystem SDK to
> SCMI platform firmware, which does the changes in HW.
>
Hi,
> Co-developed-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> MAINTAINERS | 1 +
> drivers/firmware/arm_scmi/pinctrl.c | 1 +
> drivers/pinctrl/Kconfig | 11 +
> drivers/pinctrl/Makefile | 1 +
> drivers/pinctrl/pinctrl-scmi.c | 564 ++++++++++++++++++++++++++++++++++++
> 5 files changed, 578 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4b511a55101c..d8270ac6651a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -21457,6 +21457,7 @@ F: drivers/cpufreq/sc[mp]i-cpufreq.c
> F: drivers/firmware/arm_scmi/
> F: drivers/firmware/arm_scpi.c
> F: drivers/hwmon/scmi-hwmon.c
> +F: drivers/pinctrl/pinctrl-scmi.c
> F: drivers/pmdomain/arm/
> F: drivers/powercap/arm_scmi_powercap.c
> F: drivers/regulator/scmi-regulator.c
> diff --git a/drivers/firmware/arm_scmi/pinctrl.c b/drivers/firmware/arm_scmi/pinctrl.c
> index 87d9b89cab13..0ecefe855432 100644
> --- a/drivers/firmware/arm_scmi/pinctrl.c
> +++ b/drivers/firmware/arm_scmi/pinctrl.c
> @@ -465,6 +465,7 @@ scmi_pinctrl_settings_conf(const struct scmi_protocol_handle *ph,
>
> tx = t->tx.buf;
> tx->identifier = cpu_to_le32(selector);
> + tx->function_id = cpu_to_le32(0xFFFFFFFF);
As already said....does not belong to this patch
> attributes = FIELD_PREP(GENMASK(1, 0), type) |
> FIELD_PREP(GENMASK(9, 2), chunk);
> tx->attributes = cpu_to_le32(attributes);
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index d45657aa986a..4e6f65cf0e76 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -450,6 +450,17 @@ config PINCTRL_ROCKCHIP
> help
> This support pinctrl and GPIO driver for Rockchip SoCs.
>
> +config PINCTRL_SCMI
> + tristate "Pinctrl driver using SCMI protocol interface"
> + depends on ARM_SCMI_PROTOCOL || COMPILE_TEST
> + select PINMUX
> + select GENERIC_PINCONF
> + help
> + This driver provides support for pinctrl which is controlled
> + by firmware that implements the SCMI interface.
> + It uses SCMI Message Protocol to interact with the
> + firmware providing all the pinctrl controls.
> +
[snip]
> +static int pinctrl_scmi_pinconf_group_get(struct pinctrl_dev *pctldev,
> + unsigned int group,
> + unsigned long *config)
> +{
> + int ret;
> + struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
> + enum pin_config_param config_type;
> + enum scmi_pinctrl_conf_type type;
> + u32 config_value;
> +
> + if (!config)
> + return -EINVAL;
> +
> + config_type = pinconf_to_config_param(*config);
> + ret = pinctrl_scmi_map_pinconf_type(config_type, &type);
> + if (ret) {
> + dev_err(pmx->dev, "Error map pinconf_type %d\n", ret);
> + return ret;
> + }
> +
> + ret = pinctrl_ops->settings_get(pmx->ph, group, GROUP_TYPE, type,
> + &config_value);
> + if (ret)
> + return ret;
> +
> + *config = pinconf_to_config_packed(config_type, config_value);
> +
> + return 0;
> +}
> +
> +static const struct pinconf_ops pinctrl_scmi_pinconf_ops = {
> + .is_generic = true,
> + .pin_config_get = pinctrl_scmi_pinconf_get,
> + .pin_config_set = pinctrl_scmi_pinconf_set,
> + .pin_config_group_set = pinctrl_scmi_pinconf_group_set,
> + .pin_config_group_get = pinctrl_scmi_pinconf_group_get,
> + .pin_config_config_dbg_show = pinconf_generic_dump_config,
> +};
> +
> +static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
> + struct pinctrl_desc *desc)
> +{
> + struct pinctrl_pin_desc *pins;
> + unsigned int npins;
> + int ret, i;
> +
> + npins = pinctrl_ops->count_get(pmx->ph, PIN_TYPE);
> + pins = devm_kmalloc_array(pmx->dev, npins, sizeof(*pins), GFP_KERNEL);
> + if (!pins)
> + return -ENOMEM;
> +
This is fine only if npins != 0, because on zero npins
devm_kmalloc_array will return ZERO_SIZE_PTR which is NOT-NULL so I
would add a check for !npins and bail out..or use ZERO_OR_NULL_PTR()
to check the return value...if from the previous pinctrl patch you
had decided to bail out at the protocol layer when (nr_pins == 0) and so
you will never get here, please add a comment above that npins cannot be
zero...
> + for (i = 0; i < npins; i++) {
> + pins[i].number = i;
> + ret = pinctrl_ops->name_get(pmx->ph, i, PIN_TYPE, &pins[i].name);
> + if (ret)
> + return dev_err_probe(pmx->dev, ret,
> + "Can't get name for pin %d", i);
> + }
> +
> + desc->npins = npins;
> + desc->pins = pins;
> + dev_dbg(pmx->dev, "got pins %d", npins);
> +
> + return 0;
> +}
> +
> +static const struct scmi_device_id scmi_id_table[] = {
> + { SCMI_PROTOCOL_PINCTRL, "pinctrl" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(scmi, scmi_id_table);
> +
> +static int scmi_pinctrl_probe(struct scmi_device *sdev)
> +{
> + int ret;
> + struct device *dev = &sdev->dev;
> + struct scmi_pinctrl *pmx;
> + const struct scmi_handle *handle;
> + struct scmi_protocol_handle *ph;
> +
> + if (!sdev || !sdev->handle)
if (!sdev->handle) is enough...
> + return -EINVAL;
> +
> + handle = sdev->handle;
> +
> + pinctrl_ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_PINCTRL,
> + &ph);
> + if (IS_ERR(pinctrl_ops))
> + return PTR_ERR(pinctrl_ops);
> +
> + pmx = devm_kzalloc(dev, sizeof(*pmx), GFP_KERNEL);
> + if (!pmx)
> + return -ENOMEM;
> +
> + pmx->ph = ph;
> +
> + pmx->dev = dev;
> + pmx->pctl_desc.name = DRV_NAME;
> + pmx->pctl_desc.owner = THIS_MODULE;
> + pmx->pctl_desc.pctlops = &pinctrl_scmi_pinctrl_ops;
> + pmx->pctl_desc.pmxops = &pinctrl_scmi_pinmux_ops;
> + pmx->pctl_desc.confops = &pinctrl_scmi_pinconf_ops;
> +
> + ret = pinctrl_scmi_get_pins(pmx, &pmx->pctl_desc);
> + if (ret)
> + return ret;
> +
> + ret = devm_pinctrl_register_and_init(dev, &pmx->pctl_desc, pmx,
> + &pmx->pctldev);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to register pinctrl\n");
> +
> + pmx->nr_functions = pinctrl_scmi_get_functions_count(pmx->pctldev);
> + pmx->functions = devm_kcalloc(dev, pmx->nr_functions,
> + sizeof(*pmx->functions),
> + GFP_KERNEL);
> + if (!pmx->functions)
> + return -ENOMEM;
> +
> + return pinctrl_enable(pmx->pctldev);
> +}
> +
> +static struct scmi_driver scmi_pinctrl_driver = {
> + .name = DRV_NAME,
> + .probe = scmi_pinctrl_probe,
> + .id_table = scmi_id_table,
> +};
> +module_scmi_driver(scmi_pinctrl_driver);
> +
> +MODULE_AUTHOR("Oleksii Moisieiev <oleksii_moisieiev@epam.com>");
> +MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
> +MODULE_DESCRIPTION("ARM SCMI pin controller driver");
> +MODULE_LICENSE("GPL");
>
Thanks,
Cristian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 2/3] dma: xilinx_dpdma: Remove unnecessary use of irqsave/restore
From: Tomi Valkeinen @ 2024-03-28 16:37 UTC (permalink / raw)
To: Sean Anderson
Cc: Michal Simek, linux-kernel, linux-arm-kernel, Laurent Pinchart,
Vinod Koul, dmaengine
In-Reply-To: <d6a8c1c8-258a-447b-b6cd-199f33199388@linux.dev>
On 28/03/2024 17:00, Sean Anderson wrote:
> On 3/27/24 08:27, Tomi Valkeinen wrote:
>> Hi,
>>
>> On 08/03/2024 23:00, Sean Anderson wrote:
>>> xilinx_dpdma_chan_done_irq and xilinx_dpdma_chan_vsync_irq are always
>>> called with IRQs disabled from xilinx_dpdma_irq_handler. Therefore we
>>> don't need to save/restore the IRQ flags.
>>
>> I think this is fine, but a few thoughts:
>>
>> - Is spin_lock clearly faster than the irqsave variant, or is this a pointless optimization? It's safer to just use irqsave variant, instead of making sure the code is always called from the expected contexts.
>
> It's not an optimization. Technically this will save a few instructions,
> but...
>
>> - Is this style documented/recommended anywhere? Going through docs, I only found docs telling to use irqsave when mixing irq and non-irq contexts.
>
> The purpose is mainly to make it clear that this is meant to be called
> in IRQ context. With irqsave, there's an implication that this could be
> called in non-IRQ context, which it never is.
Hmm, I see. Yes, I think that makes sense.
>> - Does this cause issues on PREEMPT_RT?
>
> Why would it?
I was reading locktypes.rst, I started wondering what it means if
spinlocks are changed into sleeping locks. But thinking about it again,
it doesn't matter, as the irq will still be masked when in irq-context.
So:
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Tomi
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 2/2] mailbox: arm_mhuv3: Add driver
From: kernel test robot @ 2024-03-28 16:37 UTC (permalink / raw)
To: Cristian Marussi, linux-kernel, linux-arm-kernel, devicetree
Cc: oe-kbuild-all, sudeep.holla, cristian.marussi, jassisinghbrar,
robh+dt, krzysztof.kozlowski+dt, conor+dt
In-Reply-To: <20240325092808.117510-3-cristian.marussi@arm.com>
Hi Cristian,
kernel test robot noticed the following build warnings:
[auto build test WARNING on soc/for-next]
[also build test WARNING on robh/for-next linus/master v6.9-rc1 next-20240328]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Cristian-Marussi/dt-bindings-mailbox-arm-mhuv3-Add-bindings/20240326-020048
base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link: https://lore.kernel.org/r/20240325092808.117510-3-cristian.marussi%40arm.com
patch subject: [PATCH 2/2] mailbox: arm_mhuv3: Add driver
config: powerpc-randconfig-r131-20240328 (https://download.01.org/0day-ci/archive/20240329/202403290015.tCLXudqC-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 23de3862dce582ce91c1aa914467d982cb1a73b4)
reproduce: (https://download.01.org/0day-ci/archive/20240329/202403290015.tCLXudqC-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403290015.tCLXudqC-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/mailbox/arm_mhuv3.c:565:46: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected struct pdbcw_page *dbcw @@ got struct pdbcw_page [noderef] __iomem * @@
drivers/mailbox/arm_mhuv3.c:565:46: sparse: expected struct pdbcw_page *dbcw
drivers/mailbox/arm_mhuv3.c:565:46: sparse: got struct pdbcw_page [noderef] __iomem *
>> drivers/mailbox/arm_mhuv3.c:568:25: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const volatile [noderef] __iomem *addr @@ got struct pdbcw_int * @@
drivers/mailbox/arm_mhuv3.c:568:25: sparse: expected void const volatile [noderef] __iomem *addr
drivers/mailbox/arm_mhuv3.c:568:25: sparse: got struct pdbcw_int *
>> drivers/mailbox/arm_mhuv3.c:568:25: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void volatile [noderef] __iomem *addr @@ got struct pdbcw_int * @@
drivers/mailbox/arm_mhuv3.c:568:25: sparse: expected void volatile [noderef] __iomem *addr
drivers/mailbox/arm_mhuv3.c:568:25: sparse: got struct pdbcw_int *
drivers/mailbox/arm_mhuv3.c:569:25: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const volatile [noderef] __iomem *addr @@ got struct pdbcw_int * @@
drivers/mailbox/arm_mhuv3.c:569:25: sparse: expected void const volatile [noderef] __iomem *addr
drivers/mailbox/arm_mhuv3.c:569:25: sparse: got struct pdbcw_int *
drivers/mailbox/arm_mhuv3.c:569:25: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void volatile [noderef] __iomem *addr @@ got struct pdbcw_int * @@
drivers/mailbox/arm_mhuv3.c:569:25: sparse: expected void volatile [noderef] __iomem *addr
drivers/mailbox/arm_mhuv3.c:569:25: sparse: got struct pdbcw_int *
>> drivers/mailbox/arm_mhuv3.c:570:25: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const volatile [noderef] __iomem *addr @@ got struct xbcw_ctrl * @@
drivers/mailbox/arm_mhuv3.c:570:25: sparse: expected void const volatile [noderef] __iomem *addr
drivers/mailbox/arm_mhuv3.c:570:25: sparse: got struct xbcw_ctrl *
>> drivers/mailbox/arm_mhuv3.c:570:25: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void volatile [noderef] __iomem *addr @@ got struct xbcw_ctrl * @@
drivers/mailbox/arm_mhuv3.c:570:25: sparse: expected void volatile [noderef] __iomem *addr
drivers/mailbox/arm_mhuv3.c:570:25: sparse: got struct xbcw_ctrl *
>> drivers/mailbox/arm_mhuv3.c:573:46: sparse: sparse: incorrect type in initializer (different address spaces) @@ expected struct mdbcw_page *dbcw @@ got struct mdbcw_page [noderef] __iomem * @@
drivers/mailbox/arm_mhuv3.c:573:46: sparse: expected struct mdbcw_page *dbcw
drivers/mailbox/arm_mhuv3.c:573:46: sparse: got struct mdbcw_page [noderef] __iomem *
>> drivers/mailbox/arm_mhuv3.c:576:25: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void volatile [noderef] __iomem *addr @@ got unsigned int * @@
drivers/mailbox/arm_mhuv3.c:576:25: sparse: expected void volatile [noderef] __iomem *addr
drivers/mailbox/arm_mhuv3.c:576:25: sparse: got unsigned int *
drivers/mailbox/arm_mhuv3.c:577:25: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void volatile [noderef] __iomem *addr @@ got unsigned int * @@
drivers/mailbox/arm_mhuv3.c:577:25: sparse: expected void volatile [noderef] __iomem *addr
drivers/mailbox/arm_mhuv3.c:577:25: sparse: got unsigned int *
drivers/mailbox/arm_mhuv3.c:578:25: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const volatile [noderef] __iomem *addr @@ got struct xbcw_ctrl * @@
drivers/mailbox/arm_mhuv3.c:578:25: sparse: expected void const volatile [noderef] __iomem *addr
drivers/mailbox/arm_mhuv3.c:578:25: sparse: got struct xbcw_ctrl *
drivers/mailbox/arm_mhuv3.c:578:25: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void volatile [noderef] __iomem *addr @@ got struct xbcw_ctrl * @@
drivers/mailbox/arm_mhuv3.c:578:25: sparse: expected void volatile [noderef] __iomem *addr
drivers/mailbox/arm_mhuv3.c:578:25: sparse: got struct xbcw_ctrl *
>> drivers/mailbox/arm_mhuv3.c:360:9: sparse: sparse: dereference of noderef expression
drivers/mailbox/arm_mhuv3.c:370:9: sparse: sparse: dereference of noderef expression
drivers/mailbox/arm_mhuv3.c:373:9: sparse: sparse: dereference of noderef expression
drivers/mailbox/arm_mhuv3.c:639:30: sparse: sparse: dereference of noderef expression
drivers/mailbox/arm_mhuv3.c:659:33: sparse: sparse: dereference of noderef expression
drivers/mailbox/arm_mhuv3.c:688:14: sparse: sparse: dereference of noderef expression
drivers/mailbox/arm_mhuv3.c:700:17: sparse: sparse: dereference of noderef expression
drivers/mailbox/arm_mhuv3.c:719:14: sparse: sparse: dereference of noderef expression
drivers/mailbox/arm_mhuv3.c:732:14: sparse: sparse: dereference of noderef expression
drivers/mailbox/arm_mhuv3.c:789:22: sparse: sparse: dereference of noderef expression
drivers/mailbox/arm_mhuv3.c:795:22: sparse: sparse: dereference of noderef expression
drivers/mailbox/arm_mhuv3.c:796:22: sparse: sparse: dereference of noderef expression
drivers/mailbox/arm_mhuv3.c:802:31: sparse: sparse: dereference of noderef expression
drivers/mailbox/arm_mhuv3.c:806:17: sparse: sparse: dereference of noderef expression
drivers/mailbox/arm_mhuv3.c:1038:17: sparse: sparse: dereference of noderef expression
vim +565 drivers/mailbox/arm_mhuv3.c
558
559 static void mhuv3_dbe_combined_irq_setup(struct mhuv3 *mhu)
560 {
561 int i;
562 struct mhuv3_extension *e = mhu->ext[DBE_EXT];
563
564 if (mhu->frame == PBX_FRAME) {
> 565 struct pdbcw_page *dbcw = mhu->pbx->dbcw;
566
567 for (i = 0; i < e->max_chans; i++) {
> 568 writel_relaxed_bitfield(0x1, &dbcw[i].int_clr, tfr_ack);
569 writel_relaxed_bitfield(0x0, &dbcw[i].int_en, tfr_ack);
> 570 writel_relaxed_bitfield(0x1, &dbcw[i].ctrl, comb_en);
571 }
572 } else {
> 573 struct mdbcw_page *dbcw = mhu->mbx->dbcw;
574
575 for (i = 0; i < e->max_chans; i++) {
> 576 writel_relaxed(0xFFFFFFFF, &dbcw[i].clr);
577 writel_relaxed(0xFFFFFFFF, &dbcw[i].msk_set);
578 writel_relaxed_bitfield(0x1, &dbcw[i].ctrl, comb_en);
579 }
580 }
581 }
582
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] drm/mediatek: Init `ddp_comp` with devm_kcalloc()
From: Douglas Anderson @ 2024-03-28 16:22 UTC (permalink / raw)
To: Chun-Kuang Hu, Philipp Zabel
Cc: Douglas Anderson, AngeloGioacchino Del Regno, CK Hu,
Daniel Vetter, David Airlie, Jason-JH.Lin, Matthias Brugger,
Nathan Lu, dri-devel, linux-arm-kernel, linux-kernel,
linux-mediatek
In the case where `conn_routes` is true we allocate an extra slot in
the `ddp_comp` array but mtk_drm_crtc_create() never seemed to
initialize it in the test case I ran. For me, this caused a later
crash when we looped through the array in mtk_drm_crtc_mode_valid().
This showed up for me when I booted with `slub_debug=FZPUA` which
poisons the memory initially. Without `slub_debug` I couldn't
reproduce, presumably because the later code handles the value being
NULL and in most cases (not guaranteed in all cases) the memory the
allocator returned started out as 0.
It really doesn't hurt to initialize the array with devm_kcalloc()
since the array is small and the overhead of initting a handful of
elements to 0 is small. In general initting memory to zero is a safer
practice and usually it's suggested to only use the non-initting alloc
functions if you really need to.
Let's switch the function to use an allocation function that zeros the
memory. For me, this avoids the crash.
Fixes: 01389b324c97 ("drm/mediatek: Add connector dynamic selection capability")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
I don't have a ton of experience with this driver to know if the fact
that the array item was still uninitialized when
mtk_drm_crtc_mode_valid() ran is the sign of a bug that should be
fixed. However, even if it is a bug and that bug is fixed then zeroing
memory when we allocate is still safer. If it's a bug that this memory
wasn't initialized then please consider this patch a bug report. ;-)
I'll also note that I reproduced this on a downstream 6.1-based
kernel. It appears that only mt8188 uses `conn_routes` and, as far as
I can tell, mt8188 isn't supported upstream yet.
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index a04499c4f9ca..29207b2756c1 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -1009,10 +1009,10 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
mtk_crtc->mmsys_dev = priv->mmsys_dev;
mtk_crtc->ddp_comp_nr = path_len;
- mtk_crtc->ddp_comp = devm_kmalloc_array(dev,
- mtk_crtc->ddp_comp_nr + (conn_routes ? 1 : 0),
- sizeof(*mtk_crtc->ddp_comp),
- GFP_KERNEL);
+ mtk_crtc->ddp_comp = devm_kcalloc(dev,
+ mtk_crtc->ddp_comp_nr + (conn_routes ? 1 : 0),
+ sizeof(*mtk_crtc->ddp_comp),
+ GFP_KERNEL);
if (!mtk_crtc->ddp_comp)
return -ENOMEM;
--
2.44.0.396.g6e790dbe36-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 1/2] dt-bindings: arm64: marvell: add solidrun cn9130 clearfog boards
From: Josua Mayer @ 2024-03-28 16:22 UTC (permalink / raw)
To: Andrew Lunn, Gregory Clement, Sebastian Hesselbarth, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Yazan Shhady, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20240321-cn9130-som-v1-1-711127a409ae@solid-run.com>
Hi all,
Am 21.03.24 um 22:47 schrieb Josua Mayer:
> Add bindings for SolidRun Clearfog boards, using a new SoM based on
> CN9130 SoC.
> The carrier boards are identical to the older Armada 388 based Clearfog
> boards. For consistency the carrier part of compatible strings are
> copied, including the established "-a1" suffix.
>
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
> .../devicetree/bindings/arm/marvell/armada-7k-8k.yaml | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
> index 16d2e132d3d1..36bdfd1bedd9 100644
> --- a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
> @@ -82,4 +82,16 @@ properties:
> - const: marvell,armada-ap807-quad
> - const: marvell,armada-ap807
>
> + - description:
> + SolidRun CN9130 clearfog family single-board computers
> + items:
vvv
> + - enum:
> + - solidrun,clearfog-base-a1
> + - solidrun,clearfog-pro-a1
> + - const: solidrun,clearfog-a1
^^^
After some thought, this no longer makes any sense to me.
Even identical carrier board combined with different SoC
will have different characteristics.
Any reason to repeat armada 388 board compatibles?
Otherwise I may choose only solidrun,cn9130-clearfog-base/pro.
> + - const: solidrun,cn9130-sr-som
> + - const: marvell,cn9130
> + - const: marvell,armada-ap807-quad
> + - const: marvell,armada-ap807
> +
> additionalProperties: true
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 17/23] dt-bindings: media: imx258: Rename to include vendor prefix
From: Luigi311 @ 2024-03-28 16:15 UTC (permalink / raw)
To: Conor Dooley, Kieran Bingham
Cc: Conor Dooley, git, linux-media, dave.stevenson, jacopo.mondi,
mchehab, robh, krzysztof.kozlowski+dt, conor+dt, shawnguo,
s.hauer, kernel, festevam, sakari.ailus, devicetree, imx,
linux-arm-kernel, linux-kernel
In-Reply-To: <20240328-prideful-unopposed-e29fcee74c29@wendy>
On 3/28/24 04:15, Conor Dooley wrote:
> On Thu, Mar 28, 2024 at 08:52:01AM +0000, Kieran Bingham wrote:
>> Quoting git@luigi311.com (2024-03-28 00:57:34)
>>> On 3/27/24 17:47, Conor Dooley wrote:
>>>> On Wed, Mar 27, 2024 at 05:17:03PM -0600, git@luigi311.com wrote:
>>>>> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
>>>>>
>>>>> imx258.yaml doesn't include the vendor prefix of sony, so
>>>>> rename to add it.
>>>>> Update the id entry and MAINTAINERS to match.
>>>>>
>>>>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
>>>>> Acked-by: Conor Dooley <conor.dooley@microchip.com>
>>>>
>>>> This is a v1 with my ack, something has gone awry here. It's also
>>>> missing your signoff. Did you pick up someone else's series?
>>>
>>> Yes, this is a continuation of Dave's work. I contacted him directly,
>>> and he mentioned that he is unable to submit a v2 any time soon and
>>> was open to someone else continuing it in his stead.
>
> Ah okay. Unfortunately I see so many binding patches pass by that I
> sometimes forget about what I already reviewed, and I did not
> remember this one at all.
No worries I'm not surprised since i see constant things submitted
to upstream and v1 was actually sent a year ago so there would be no
shot that i would remember it either.
>
>>> This is my first
>>> time submitting a patch via a mailing list, so I'm not sure if I'm
>>> missing something, but I only added my sign off for anything that
>>> actually included work from my side and not just bringing his patch
>>> forward to this patch series.
>
> Right. The rules are that you need to add it when you send someone's
> work, like chain of custody type of thing.
Ohh i see, ok ill go ahead and add my sign off to all the patches then
>
>> Your cover letter states v2, but the individual patches do not.
>>
>> Add the '-v2' (or, rather, next it will be '-v3') to git format-patch
>> when you save your series and it will add the version to each patch. You
>> can also add '-s' to that command I believe to add your SoB to each
>> patch.
>
> or a rebase will do it with --signoff:
> https://git-scm.com/docs/git-rebase#Documentation/git-rebase.txt---signoff
Perfect thanks for the information you two! Ill be sure to use those
for the next revision.
>
> Cheers,
> Conor.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 01/11] staging: vc04_services: changen strncpy() to strscpy_pad()
From: Arnd Bergmann @ 2024-03-28 16:15 UTC (permalink / raw)
To: Dan Carpenter, Arnd Bergmann
Cc: linux-kernel, Florian Fainelli, Greg Kroah-Hartman,
Broadcom internal kernel review list, linux-rpi-kernel,
linux-arm-kernel, linux-staging
In-Reply-To: <508e4ede-ab78-418d-9aef-f657827b6dd1@moroto.mountain>
On Thu, Mar 28, 2024, at 15:42, Dan Carpenter wrote:
> On Thu, Mar 28, 2024 at 03:04:45PM +0100, Arnd Bergmann wrote:
>> diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
>> index 258aa0e37f55..6ca5797aeae5 100644
>> --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
>> +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
>> @@ -937,8 +937,8 @@ static int create_component(struct vchiq_mmal_instance *instance,
>> /* build component create message */
>> m.h.type = MMAL_MSG_TYPE_COMPONENT_CREATE;
>> m.u.component_create.client_component = component->client_component;
>> - strncpy(m.u.component_create.name, name,
>> - sizeof(m.u.component_create.name));
>> + strscpy_pad(m.u.component_create.name, name,
>> + sizeof(m.u.component_create.name));
>
> You sent this earlier and we already applied it.
Sorry about that. I normally mark patches I send as applied
in the subject but it looks like I lost the annotation
by accident.
> Btw, I just learned there is a new trick to write this when it's just
> sizeof(dest).
>
> strscpy_pad(m.u.component_create.name, name);
Ah, cute.
arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 4/7] initrd: Remove the now superfluous sentinel element from ctl_table array
From: Joel Granados via B4 Relay @ 2024-03-28 15:57 UTC (permalink / raw)
To: Andrew Morton, Muchun Song, Miaohe Lin, Naoya Horiguchi,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
David Howells, Jarkko Sakkinen, Kees Cook, Herbert Xu,
David S. Miller, Jens Axboe, Pavel Begunkov, Atish Patra,
Anup Patel, Will Deacon, Mark Rutland, Paul Walmsley,
Palmer Dabbelt, Albert Ou
Cc: Luis Chamberlain, linux-mm, linux-kernel, linux-fsdevel, apparmor,
linux-security-module, keyrings, linux-crypto, io-uring,
linux-riscv, linux-arm-kernel, Joel Granados
In-Reply-To: <20240328-jag-sysctl_remset_misc-v1-0-47c1463b3af2@samsung.com>
From: Joel Granados <j.granados@samsung.com>
This commit comes at the tail end of a greater effort to remove the
empty elements at the end of the ctl_table arrays (sentinels) which will
reduce the overall build time size of the kernel and run time memory
bloat by ~64 bytes per sentinel (further information Link :
https://lore.kernel.org/all/ZO5Yx5JFogGi%2FcBo@bombadil.infradead.org/)
Remove sentinel from kern_do_mounts_initrd_table.
Signed-off-by: Joel Granados <j.granados@samsung.com>
---
init/do_mounts_initrd.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/init/do_mounts_initrd.c b/init/do_mounts_initrd.c
index 425f4bcf4b77..22c7f41ff642 100644
--- a/init/do_mounts_initrd.c
+++ b/init/do_mounts_initrd.c
@@ -29,7 +29,6 @@ static struct ctl_table kern_do_mounts_initrd_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec,
},
- { }
};
static __init int kernel_do_mounts_initrd_sysctls_init(void)
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 5/7] ipc: Remove the now superfluous sentinel element from ctl_table array
From: Joel Granados via B4 Relay @ 2024-03-28 15:57 UTC (permalink / raw)
To: Andrew Morton, Muchun Song, Miaohe Lin, Naoya Horiguchi,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
David Howells, Jarkko Sakkinen, Kees Cook, Herbert Xu,
David S. Miller, Jens Axboe, Pavel Begunkov, Atish Patra,
Anup Patel, Will Deacon, Mark Rutland, Paul Walmsley,
Palmer Dabbelt, Albert Ou
Cc: Luis Chamberlain, linux-mm, linux-kernel, linux-fsdevel, apparmor,
linux-security-module, keyrings, linux-crypto, io-uring,
linux-riscv, linux-arm-kernel, Joel Granados
In-Reply-To: <20240328-jag-sysctl_remset_misc-v1-0-47c1463b3af2@samsung.com>
From: Joel Granados <j.granados@samsung.com>
This commit comes at the tail end of a greater effort to remove the
empty elements at the end of the ctl_table arrays (sentinels) which will
reduce the overall build time size of the kernel and run time memory
bloat by ~64 bytes per sentinel (further information Link :
https://lore.kernel.org/all/ZO5Yx5JFogGi%2FcBo@bombadil.infradead.org/)
Remove the sentinels from ipc_sysctls and mq_sysctls
Signed-off-by: Joel Granados <j.granados@samsung.com>
---
ipc/ipc_sysctl.c | 1 -
ipc/mq_sysctl.c | 1 -
2 files changed, 2 deletions(-)
diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
index 45cb1dabce29..0867535af96f 100644
--- a/ipc/ipc_sysctl.c
+++ b/ipc/ipc_sysctl.c
@@ -178,7 +178,6 @@ static struct ctl_table ipc_sysctls[] = {
.extra2 = SYSCTL_INT_MAX,
},
#endif
- {}
};
static struct ctl_table_set *set_lookup(struct ctl_table_root *root)
diff --git a/ipc/mq_sysctl.c b/ipc/mq_sysctl.c
index 21fba3a6edaf..22ec532c7fa1 100644
--- a/ipc/mq_sysctl.c
+++ b/ipc/mq_sysctl.c
@@ -64,7 +64,6 @@ static struct ctl_table mq_sysctls[] = {
.extra1 = &msg_maxsize_limit_min,
.extra2 = &msg_maxsize_limit_max,
},
- {}
};
static struct ctl_table_set *set_lookup(struct ctl_table_root *root)
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 3/7] crypto: Remove the now superfluous sentinel element from ctl_table array
From: Joel Granados via B4 Relay @ 2024-03-28 15:57 UTC (permalink / raw)
To: Andrew Morton, Muchun Song, Miaohe Lin, Naoya Horiguchi,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
David Howells, Jarkko Sakkinen, Kees Cook, Herbert Xu,
David S. Miller, Jens Axboe, Pavel Begunkov, Atish Patra,
Anup Patel, Will Deacon, Mark Rutland, Paul Walmsley,
Palmer Dabbelt, Albert Ou
Cc: Luis Chamberlain, linux-mm, linux-kernel, linux-fsdevel, apparmor,
linux-security-module, keyrings, linux-crypto, io-uring,
linux-riscv, linux-arm-kernel, Joel Granados
In-Reply-To: <20240328-jag-sysctl_remset_misc-v1-0-47c1463b3af2@samsung.com>
From: Joel Granados <j.granados@samsung.com>
This commit comes at the tail end of a greater effort to remove the
empty elements at the end of the ctl_table arrays (sentinels) which will
reduce the overall build time size of the kernel and run time memory
bloat by ~64 bytes per sentinel (further information Link :
https://lore.kernel.org/all/ZO5Yx5JFogGi%2FcBo@bombadil.infradead.org/)
Remove sentinel from crypto_sysctl_table
Signed-off-by: Joel Granados <j.granados@samsung.com>
---
crypto/fips.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/crypto/fips.c b/crypto/fips.c
index 92fd506abb21..8a784018ebfc 100644
--- a/crypto/fips.c
+++ b/crypto/fips.c
@@ -63,7 +63,6 @@ static struct ctl_table crypto_sysctl_table[] = {
.mode = 0444,
.proc_handler = proc_dostring
},
- {}
};
static struct ctl_table_header *crypto_sysctls;
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 1/7] memory: Remove the now superfluous sentinel element from ctl_table array
From: Joel Granados via B4 Relay @ 2024-03-28 15:57 UTC (permalink / raw)
To: Andrew Morton, Muchun Song, Miaohe Lin, Naoya Horiguchi,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
David Howells, Jarkko Sakkinen, Kees Cook, Herbert Xu,
David S. Miller, Jens Axboe, Pavel Begunkov, Atish Patra,
Anup Patel, Will Deacon, Mark Rutland, Paul Walmsley,
Palmer Dabbelt, Albert Ou
Cc: Luis Chamberlain, linux-mm, linux-kernel, linux-fsdevel, apparmor,
linux-security-module, keyrings, linux-crypto, io-uring,
linux-riscv, linux-arm-kernel, Joel Granados
In-Reply-To: <20240328-jag-sysctl_remset_misc-v1-0-47c1463b3af2@samsung.com>
From: Joel Granados <j.granados@samsung.com>
This commit comes at the tail end of a greater effort to remove the
empty elements at the end of the ctl_table arrays (sentinels) which will
reduce the overall build time size of the kernel and run time memory
bloat by ~64 bytes per sentinel (further information Link :
https://lore.kernel.org/all/ZO5Yx5JFogGi%2FcBo@bombadil.infradead.org/)
Remove sentinel from all files under mm/ that register a sysctl table.
Signed-off-by: Joel Granados <j.granados@samsung.com>
---
mm/compaction.c | 1 -
mm/hugetlb.c | 1 -
mm/hugetlb_vmemmap.c | 1 -
mm/memory-failure.c | 1 -
mm/oom_kill.c | 1 -
mm/page-writeback.c | 1 -
mm/page_alloc.c | 1 -
7 files changed, 7 deletions(-)
diff --git a/mm/compaction.c b/mm/compaction.c
index 807b58e6eb68..e8a047afca22 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -3345,7 +3345,6 @@ static struct ctl_table vm_compaction[] = {
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE,
},
- { }
};
static int __init kcompactd_init(void)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 23ef240ba48a..7ac5240a197d 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -5045,7 +5045,6 @@ static struct ctl_table hugetlb_table[] = {
.mode = 0644,
.proc_handler = hugetlb_overcommit_handler,
},
- { }
};
static void hugetlb_sysctl_init(void)
diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
index da177e49d956..b9a55322e52c 100644
--- a/mm/hugetlb_vmemmap.c
+++ b/mm/hugetlb_vmemmap.c
@@ -679,7 +679,6 @@ static struct ctl_table hugetlb_vmemmap_sysctls[] = {
.mode = 0644,
.proc_handler = proc_dobool,
},
- { }
};
static int __init hugetlb_vmemmap_init(void)
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 9349948f1abf..6a112f9ecf91 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -141,7 +141,6 @@ static struct ctl_table memory_failure_table[] = {
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE,
},
- { }
};
/*
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 8d6a207c3c59..4d7a0004df2c 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -724,7 +724,6 @@ static struct ctl_table vm_oom_kill_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec,
},
- {}
};
#endif
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 3e19b87049db..fba324e1a010 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2291,7 +2291,6 @@ static struct ctl_table vm_page_writeback_sysctls[] = {
.mode = 0644,
.proc_handler = proc_dointvec_jiffies,
},
- {}
};
#endif
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 14d39f34d336..8b9820620fe3 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6211,7 +6211,6 @@ static struct ctl_table page_alloc_sysctl_table[] = {
.extra2 = SYSCTL_ONE_HUNDRED,
},
#endif
- {}
};
void __init page_alloc_sysctl_init(void)
--
2.43.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 0/7] sysctl: Remove sentinel elements from misc directories
From: Joel Granados via B4 Relay @ 2024-03-28 15:57 UTC (permalink / raw)
To: Andrew Morton, Muchun Song, Miaohe Lin, Naoya Horiguchi,
John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
David Howells, Jarkko Sakkinen, Kees Cook, Herbert Xu,
David S. Miller, Jens Axboe, Pavel Begunkov, Atish Patra,
Anup Patel, Will Deacon, Mark Rutland, Paul Walmsley,
Palmer Dabbelt, Albert Ou
Cc: Luis Chamberlain, linux-mm, linux-kernel, linux-fsdevel, apparmor,
linux-security-module, keyrings, linux-crypto, io-uring,
linux-riscv, linux-arm-kernel, Joel Granados
From: Joel Granados <j.granados@samsung.com>
What?
These commits remove the sentinel element (last empty element) from the
sysctl arrays of all the files under the "mm/", "security/", "ipc/",
"init/", "io_uring/", "drivers/perf/" and "crypto/" directories that
register a sysctl array. The inclusion of [4] to mainline allows the
removal of sentinel elements without behavioral change. This is safe
because the sysctl registration code (register_sysctl() and friends) use
the array size in addition to checking for a sentinel [1].
Why?
By removing the sysctl sentinel elements we avoid kernel bloat as
ctl_table arrays get moved out of kernel/sysctl.c into their own
respective subsystems. This move was started long ago to avoid merge
conflicts; the sentinel removal bit came after Mathew Wilcox suggested
it to avoid bloating the kernel by one element as arrays moved out. This
patchset will reduce the overall build time size of the kernel and run
time memory bloat by about ~64 bytes per declared ctl_table array (more
info here [5]).
When are we done?
There are 4 patchest (25 commits [2]) that are still outstanding to
completely remove the sentinels: files under "net/", files under
"kernel/" dir, misc dirs (this patchset) and the final set that removes
the unneeded check for ->procname == NULL.
Testing:
* Ran sysctl selftests (./tools/testing/selftests/sysctl/sysctl.sh)
* Ran this through 0-day with no errors or warnings
Savings in vmlinux:
A total of 64 bytes per sentinel is saved after removal; I measured in
x86_64 to give an idea of the aggregated savings. The actual savings
will depend on individual kernel configuration.
* bloat-o-meter
- The "yesall" config saves 963 bytes (bloat-o-meter output [6])
- A reduced config [3] saves 452 bytes (bloat-o-meter output [7])
Savings in allocated memory:
None in this set but will occur when the superfluous allocations are
removed from proc_sysctl.c. I include it here for context. The
estimated savings during boot for config [3] are 6272 bytes. See [8]
for how to measure it.
Comments/feedback greatly appreciated
Best
Joel
[1] https://lore.kernel.org/all/20230809105006.1198165-1-j.granados@samsung.com/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/joel.granados/linux.git/tag/?h=sysctl_remove_empty_elem_v5
[3] https://gist.github.com/Joelgranados/feaca7af5537156ca9b73aeaec093171
[4] https://lore.kernel.org/all/ZO5Yx5JFogGi%2FcBo@bombadil.infradead.org/
[5]
Links Related to the ctl_table sentinel removal:
* Good summaries from Luis:
https://lore.kernel.org/all/ZO5Yx5JFogGi%2FcBo@bombadil.infradead.org/
https://lore.kernel.org/all/ZMFizKFkVxUFtSqa@bombadil.infradead.org/
* Patches adjusting sysctl register calls:
https://lore.kernel.org/all/20230302204612.782387-1-mcgrof@kernel.org/
https://lore.kernel.org/all/20230302202826.776286-1-mcgrof@kernel.org/
* Discussions about expectations and approach
https://lore.kernel.org/all/20230321130908.6972-1-frank.li@vivo.com
https://lore.kernel.org/all/20220220060626.15885-1-tangmeng@uniontech.com
[6]
add/remove: 0/0 grow/shrink: 0/16 up/down: 0/-963 (-963)
Function old new delta
setup_mq_sysctls 502 499 -3
yama_sysctl_table 128 64 -64
vm_page_writeback_sysctls 512 448 -64
vm_oom_kill_table 256 192 -64
vm_compaction 320 256 -64
page_alloc_sysctl_table 576 512 -64
mq_sysctls 384 320 -64
memory_failure_table 192 128 -64
loadpin_sysctl_table 128 64 -64
key_sysctls 448 384 -64
kernel_io_uring_disabled_table 192 128 -64
kern_do_mounts_initrd_table 128 64 -64
ipc_sysctls 832 768 -64
hugetlb_vmemmap_sysctls 128 64 -64
hugetlb_table 320 256 -64
apparmor_sysctl_table 256 192 -64
Total: Before=440605433, After=440604470, chg -0.00%
[7]
add/remove: 0/0 grow/shrink: 0/8 up/down: 0/-452 (-452)
Function old new delta
setup_ipc_sysctls 306 302 -4
vm_page_writeback_sysctls 512 448 -64
vm_oom_kill_table 256 192 -64
page_alloc_sysctl_table 384 320 -64
key_sysctls 384 320 -64
kernel_io_uring_disabled_table 192 128 -64
ipc_sysctls 640 576 -64
hugetlb_table 256 192 -64
Total: Before=8523801, After=8523349, chg -0.01%
[8]
To measure the in memory savings apply this on top of this patchset.
"
diff --git i/fs/proc/proc_sysctl.c w/fs/proc/proc_sysctl.c
index 37cde0efee57..896c498600e8 100644
--- i/fs/proc/proc_sysctl.c
+++ w/fs/proc/proc_sysctl.c
@@ -966,6 +966,7 @@ static struct ctl_dir *new_dir(struct ctl_table_set *set,
table[0].procname = new_name;
table[0].mode = S_IFDIR|S_IRUGO|S_IXUGO;
init_header(&new->header, set->dir.header.root, set, node, table, 1);
+ printk("%ld sysctl saved mem kzalloc\n", sizeof(struct ctl_table));
return new;
}
@@ -1189,6 +1190,7 @@ static struct ctl_table_header *new_links(struct ctl_dir *dir, s>
link_name += len;
link++;
}
+ printk("%ld sysctl saved mem kzalloc\n", sizeof(struct ctl_table));
init_header(links, dir->header.root, dir->header.set, node, link_table,
head->ctl_table_size);
links->nreg = nr_entries;
"
and then run the following bash script in the kernel:
accum=0
for n in $(dmesg | grep kzalloc | awk '{print $3}') ; do
accum=$(calc "$accum + $n")
done
echo $accum
Signed-off-by: Joel Granados <j.granados@samsung.com>
--
---
Joel Granados (7):
memory: Remove the now superfluous sentinel element from ctl_table array
security: Remove the now superfluous sentinel element from ctl_table array
crypto: Remove the now superfluous sentinel element from ctl_table array
initrd: Remove the now superfluous sentinel element from ctl_table array
ipc: Remove the now superfluous sentinel element from ctl_table array
io_uring: Remove the now superfluous sentinel elements from ctl_table array
drivers: perf: Remove the now superfluous sentinel elements from ctl_table array
crypto/fips.c | 1 -
drivers/perf/riscv_pmu_sbi.c | 1 -
init/do_mounts_initrd.c | 1 -
io_uring/io_uring.c | 1 -
ipc/ipc_sysctl.c | 1 -
ipc/mq_sysctl.c | 1 -
mm/compaction.c | 1 -
mm/hugetlb.c | 1 -
mm/hugetlb_vmemmap.c | 1 -
mm/memory-failure.c | 1 -
mm/oom_kill.c | 1 -
mm/page-writeback.c | 1 -
mm/page_alloc.c | 1 -
security/apparmor/lsm.c | 1 -
security/keys/sysctl.c | 1 -
security/loadpin/loadpin.c | 1 -
security/yama/yama_lsm.c | 1 -
17 files changed, 17 deletions(-)
---
base-commit: 4cece764965020c22cff7665b18a012006359095
change-id: 20240320-jag-sysctl_remset_misc-a261f5a7ddea
Best regards,
--
Joel Granados <j.granados@samsung.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v6 3/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
From: Cristian Marussi @ 2024-03-28 15:47 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: Sudeep Holla, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Linus Walleij, Dan Carpenter, linux-arm-kernel, linux-kernel,
devicetree, linux-gpio, Peng Fan, Oleksii Moisieiev
In-Reply-To: <20240323-pinctrl-scmi-v6-3-a895243257c0@nxp.com>
On Sat, Mar 23, 2024 at 08:15:16PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> Add basic implementation of the SCMI v3.2 pincontrol protocol.
>
Hi,
a few more comments down below...
> Co-developed-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/firmware/arm_scmi/Makefile | 1 +
> drivers/firmware/arm_scmi/driver.c | 2 +
> drivers/firmware/arm_scmi/pinctrl.c | 921 ++++++++++++++++++++++++++++++++++
> drivers/firmware/arm_scmi/protocols.h | 1 +
> include/linux/scmi_protocol.h | 75 +++
> 5 files changed, 1000 insertions(+)
>
> diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
> index a7bc4796519c..8e3874ff1544 100644
> --- a/drivers/firmware/arm_scmi/Makefile
> +++ b/drivers/firmware/arm_scmi/Makefile
> @@ -11,6 +11,7 @@ scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
> scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
> scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
> scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
> +scmi-protocols-y += pinctrl.o
> scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
>
> obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index 415e6f510057..ac2d4b19727c 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -3142,6 +3142,7 @@ static int __init scmi_driver_init(void)
> scmi_voltage_register();
> scmi_system_register();
> scmi_powercap_register();
> + scmi_pinctrl_register();
>
> return platform_driver_register(&scmi_driver);
> }
> @@ -3159,6 +3160,7 @@ static void __exit scmi_driver_exit(void)
> scmi_voltage_unregister();
> scmi_system_unregister();
> scmi_powercap_unregister();
> + scmi_pinctrl_unregister();
>
> scmi_transports_exit();
>
> diff --git a/drivers/firmware/arm_scmi/pinctrl.c b/drivers/firmware/arm_scmi/pinctrl.c
> new file mode 100644
> index 000000000000..87d9b89cab13
> --- /dev/null
> +++ b/drivers/firmware/arm_scmi/pinctrl.c
> @@ -0,0 +1,921 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * System Control and Management Interface (SCMI) Pinctrl Protocol
> + *
> + * Copyright (C) 2024 EPAM
> + * Copyright 2024 NXP
> + */
> +
> +#include <linux/module.h>
> +#include <linux/scmi_protocol.h>
> +#include <linux/slab.h>
> +
> +#include "common.h"
> +#include "protocols.h"
> +
> +/* Updated only after ALL the mandatory features for that version are merged */
> +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x0
> +
AFAICS, the only missing things are PINCTRL_SET_PERMISSIONS (optional command)
and the multiple-configs on SETTINGS_GET, but this latter is something really
that we have to ask for in the request AND we did not as of now since we dont
need it...so I would say to bump the version to 0x10000 just to avoid needless
warning as soon as a server supporting Pinctrl is met.
> +#define GET_GROUPS_NR(x) le32_get_bits((x), GENMASK(31, 16))
> +#define GET_PINS_NR(x) le32_get_bits((x), GENMASK(15, 0))
> +#define GET_FUNCTIONS_NR(x) le32_get_bits((x), GENMASK(15, 0))
> +
> +#define EXT_NAME_FLAG(x) le32_get_bits((x), BIT(31))
> +#define NUM_ELEMS(x) le32_get_bits((x), GENMASK(15, 0))
> +
> +#define REMAINING(x) le32_get_bits((x), GENMASK(31, 16))
> +#define RETURNED(x) le32_get_bits((x), GENMASK(11, 0))
> +
> +#define CONFIG_FLAG_MASK GENMASK(19, 18)
> +#define SELECTOR_MASK GENMASK(17, 16)
> +#define SKIP_CONFIGS_MASK GENMASK(15, 8)
> +#define CONFIG_TYPE_MASK GENMASK(7, 0)
> +
> +enum scmi_pinctrl_protocol_cmd {
> + PINCTRL_ATTRIBUTES = 0x3,
> + PINCTRL_LIST_ASSOCIATIONS = 0x4,
> + PINCTRL_SETTINGS_GET = 0x5,
> + PINCTRL_SETTINGS_CONFIGURE = 0x6,
> + PINCTRL_REQUEST = 0x7,
> + PINCTRL_RELEASE = 0x8,
> + PINCTRL_NAME_GET = 0x9,
> + PINCTRL_SET_PERMISSIONS = 0xa
> +};
> +
> +struct scmi_msg_settings_conf {
> + __le32 identifier;
> + __le32 function_id;
> + __le32 attributes;
> + __le32 configs[];
> +};
> +
> +struct scmi_msg_settings_get {
> + __le32 identifier;
> + __le32 attributes;
> +};
> +
> +struct scmi_resp_settings_get {
> + __le32 function_selected;
> + __le32 num_configs;
> + __le32 configs[];
> +};
> +
> +struct scmi_msg_pinctrl_protocol_attributes {
> + __le32 attributes_low;
> + __le32 attributes_high;
> +};
> +
> +struct scmi_msg_pinctrl_attributes {
> + __le32 identifier;
> + __le32 flags;
> +};
> +
> +struct scmi_resp_pinctrl_attributes {
> + __le32 attributes;
> + u8 name[SCMI_SHORT_NAME_MAX_SIZE];
> +};
> +
> +struct scmi_msg_pinctrl_list_assoc {
> + __le32 identifier;
> + __le32 flags;
> + __le32 index;
> +};
> +
> +struct scmi_resp_pinctrl_list_assoc {
> + __le32 flags;
> + __le16 array[];
> +};
> +
> +struct scmi_msg_func_set {
> + __le32 identifier;
> + __le32 function_id;
> + __le32 flags;
> +};
> +
As said by Dan...drop this.
> +struct scmi_msg_request {
> + __le32 identifier;
> + __le32 flags;
> +};
> +
> +struct scmi_group_info {
> + char name[SCMI_MAX_STR_SIZE];
> + bool present;
> + u32 *group_pins;
> + u32 nr_pins;
> +};
> +
> +struct scmi_function_info {
> + char name[SCMI_MAX_STR_SIZE];
> + bool present;
> + u32 *groups;
> + u32 nr_groups;
> +};
> +
> +struct scmi_pin_info {
> + char name[SCMI_MAX_STR_SIZE];
> + bool present;
> +};
> +
> +struct scmi_pinctrl_info {
> + u32 version;
> + int nr_groups;
> + int nr_functions;
> + int nr_pins;
> + struct scmi_group_info *groups;
> + struct scmi_function_info *functions;
> + struct scmi_pin_info *pins;
> +};
> +
> +static int scmi_pinctrl_attributes_get(const struct scmi_protocol_handle *ph,
> + struct scmi_pinctrl_info *pi)
> +{
> + int ret;
> + struct scmi_xfer *t;
> + struct scmi_msg_pinctrl_protocol_attributes *attr;
> +
> + ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0, sizeof(*attr),
> + &t);
> + if (ret)
> + return ret;
> +
> + attr = t->rx.buf;
> +
> + ret = ph->xops->do_xfer(ph, t);
> + if (!ret) {
> + pi->nr_functions = GET_FUNCTIONS_NR(attr->attributes_high);
> + pi->nr_groups = GET_GROUPS_NR(attr->attributes_low);
> + pi->nr_pins = GET_PINS_NR(attr->attributes_low);
I was thinking, does make sense to allow a nr_pins == 0 setup to probe
successfully ? Becasuse is legit for the platform to return zero groups or
zero functions BUT zero pins is just useless (spec does not say
anything)
Maybe you could just put a dev_warn() here on if (nr_pins == 0) and bail
out with -EINVAL...
On the other side looking at the zero groups/function case, that is
plausible and handled properly by the driver since a 0-bytes
devm_kcalloc will return ZERO_SIZE_PTR (not NULL) and all the remaining
references to pinfo->groups and pinfo->functions are guarded by a check
on selector >= nr_groups (or >= nr_functions), and by scmi_pinctrl_validate_id()
so the zero grouyps/fuctions scenarios should be safely handled.
> + }
> +
> + ph->xops->xfer_put(ph, t);
> + return ret;
> +}
> +
> +static int scmi_pinctrl_count_get(const struct scmi_protocol_handle *ph,
> + enum scmi_pinctrl_selector_type type)
> +{
> + struct scmi_pinctrl_info *pi = ph->get_priv(ph);
> +
> + switch (type) {
> + case PIN_TYPE:
> + return pi->nr_pins;
> + case GROUP_TYPE:
> + return pi->nr_groups;
> + case FUNCTION_TYPE:
> + return pi->nr_functions;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int scmi_pinctrl_validate_id(const struct scmi_protocol_handle *ph,
> + u32 identifier,
> + enum scmi_pinctrl_selector_type type)
> +{
> + int value;
> +
> + value = scmi_pinctrl_count_get(ph, type);
> + if (value < 0)
> + return value;
> +
> + if (identifier >= value)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int scmi_pinctrl_attributes(const struct scmi_protocol_handle *ph,
> + enum scmi_pinctrl_selector_type type,
> + u32 selector, char *name,
> + u32 *n_elems)
> +{
> + int ret;
> + struct scmi_xfer *t;
> + struct scmi_msg_pinctrl_attributes *tx;
> + struct scmi_resp_pinctrl_attributes *rx;
> + u32 ext_name_flag;
what about a bool
> +
> + if (!name)
> + return -EINVAL;
> +
> + ret = scmi_pinctrl_validate_id(ph, selector, type);
> + if (ret)
> + return ret;
> +
> + ret = ph->xops->xfer_get_init(ph, PINCTRL_ATTRIBUTES, sizeof(*tx),
> + sizeof(*rx), &t);
> + if (ret)
> + return ret;
> +
> + tx = t->tx.buf;
> + rx = t->rx.buf;
> + tx->identifier = cpu_to_le32(selector);
> + tx->flags = cpu_to_le32(type);
> +
> + ret = ph->xops->do_xfer(ph, t);
> + if (!ret) {
> + if (n_elems)
> + *n_elems = NUM_ELEMS(rx->attributes);
> +
> + strscpy(name, rx->name, SCMI_SHORT_NAME_MAX_SIZE);
> +
> + ext_name_flag = EXT_NAME_FLAG(rx->attributes);
> + } else
> + ext_name_flag = 0;
and you dont need this else branch to set ext_name_flag to false, since down
below you will check ext_flag ONLY if !ret, so it will have surely been
set if the do_xfer did not fail.
> +
> + ph->xops->xfer_put(ph, t);
> +
> + /*
> + * If supported overwrite short name with the extended one;
> + * on error just carry on and use already provided short name.
> + */
> + if (!ret && ext_name_flag)
> + ph->hops->extended_name_get(ph, PINCTRL_NAME_GET, selector,
> + (u32 *)&type, name,
> + SCMI_MAX_STR_SIZE);
> + return ret;
> +}
> +
> +struct scmi_pinctrl_ipriv {
> + u32 selector;
> + enum scmi_pinctrl_selector_type type;
> + u32 *array;
> +};
> +
> +static void iter_pinctrl_assoc_prepare_message(void *message,
> + u32 desc_index,
> + const void *priv)
> +{
> + struct scmi_msg_pinctrl_list_assoc *msg = message;
> + const struct scmi_pinctrl_ipriv *p = priv;
> +
> + msg->identifier = cpu_to_le32(p->selector);
> + msg->flags = cpu_to_le32(p->type);
> + /* Set the number of OPPs to be skipped/already read */
OPP ? .. maybe drop this comment that was cut/pasted from somewhere else :D
> + msg->index = cpu_to_le32(desc_index);
> +}
> +
> +static int iter_pinctrl_assoc_update_state(struct scmi_iterator_state *st,
> + const void *response, void *priv)
> +{
> + const struct scmi_resp_pinctrl_list_assoc *r = response;
> +
> + st->num_returned = RETURNED(r->flags);
> + st->num_remaining = REMAINING(r->flags);
> +
> + return 0;
> +}
> +
> +static int
> +iter_pinctrl_assoc_process_response(const struct scmi_protocol_handle *ph,
> + const void *response,
> + struct scmi_iterator_state *st, void *priv)
> +{
> + const struct scmi_resp_pinctrl_list_assoc *r = response;
> + struct scmi_pinctrl_ipriv *p = priv;
> +
> + p->array[st->desc_index + st->loop_idx] =
> + le16_to_cpu(r->array[st->loop_idx]);
> +
> + return 0;
> +}
> +
> +static int scmi_pinctrl_list_associations(const struct scmi_protocol_handle *ph,
> + u32 selector,
> + enum scmi_pinctrl_selector_type type,
> + u16 size, u32 *array)
> +{
> + int ret;
> + void *iter;
> + struct scmi_iterator_ops ops = {
> + .prepare_message = iter_pinctrl_assoc_prepare_message,
> + .update_state = iter_pinctrl_assoc_update_state,
> + .process_response = iter_pinctrl_assoc_process_response,
> + };
> + struct scmi_pinctrl_ipriv ipriv = {
> + .selector = selector,
> + .type = type,
> + .array = array,
> + };
> +
> + if (!array || !size || type == PIN_TYPE)
> + return -EINVAL;
> +
> + ret = scmi_pinctrl_validate_id(ph, selector, type);
> + if (ret)
> + return ret;
> +
> + iter = ph->hops->iter_response_init(ph, &ops, size,
> + PINCTRL_LIST_ASSOCIATIONS,
> + sizeof(struct scmi_msg_pinctrl_list_assoc),
> + &ipriv);
> +
> + if (IS_ERR(iter))
> + return PTR_ERR(iter);
> +
> + return ph->hops->iter_response_run(iter);
> +}
> +
> +struct scmi_settings_get_ipriv {
> + u32 selector;
> + enum scmi_pinctrl_selector_type type;
> + u32 flag;
> + enum scmi_pinctrl_conf_type *config_types;
> + u32 *config_values;
> +};
> +
> +static void
> +iter_pinctrl_settings_get_prepare_message(void *message, u32 desc_index,
> + const void *priv)
> +{
> + struct scmi_msg_settings_get *msg = message;
> + const struct scmi_settings_get_ipriv *p = priv;
> + u32 attributes;
> +
> + attributes = FIELD_PREP(CONFIG_FLAG_MASK, p->flag) |
> + FIELD_PREP(SELECTOR_MASK, p->type);
> +
> + if (p->flag == 1)
A boolean like .get_all would be more clear..see down below why you dont need
a flag 0|1|2
> + attributes |= FIELD_PREP(SKIP_CONFIGS_MASK, desc_index);
> + else if (!p->flag)
> + attributes |= FIELD_PREP(CONFIG_TYPE_MASK, p->config_types[0]);
> +
> + msg->attributes = cpu_to_le32(attributes);
> + msg->identifier = cpu_to_le32(p->selector);
> +}
> +
> +static int
> +iter_pinctrl_settings_get_update_state(struct scmi_iterator_state *st,
> + const void *response, void *priv)
> +{
> + const struct scmi_resp_settings_get *r = response;
> + struct scmi_settings_get_ipriv *p = priv;
> +
> + if (p->flag == 1) {
Ditto... see below the explanation
> + st->num_returned = le32_get_bits(r->num_configs, GENMASK(7, 0));
> + st->num_remaining = le32_get_bits(r->num_configs,
> + GENMASK(31, 24));
> + } else {
> + st->num_returned = 1;
> + st->num_remaining = 0;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +iter_pinctrl_settings_get_process_response(const struct scmi_protocol_handle *ph,
> + const void *response,
> + struct scmi_iterator_state *st,
> + void *priv)
> +{
> + const struct scmi_resp_settings_get *r = response;
> + struct scmi_settings_get_ipriv *p = priv;
> +
> + if (!p->flag) {
> + if (p->config_types[0] !=
> + le32_get_bits(r->configs[st->loop_idx * 2], GENMASK(7, 0)))
> + return -EINVAL;
> + } else if (p->flag == 1) {
> + p->config_types[st->desc_index + st->loop_idx] =
> + le32_get_bits(r->configs[st->loop_idx * 2],
> + GENMASK(7, 0));
> + } else if (p->flag == 2) {
> + return 0;
> + }
Unneeded...see down below for explanation
> +
> + p->config_values[st->desc_index + st->loop_idx] =
> + le32_to_cpu(r->configs[st->loop_idx * 2 + 1]);
> +
> + return 0;
> +}
> +
> +static int
> +scmi_pinctrl_settings_get(const struct scmi_protocol_handle *ph, u32 selector,
> + enum scmi_pinctrl_selector_type type,
> + enum scmi_pinctrl_conf_type config_type,
> + u32 *config_value)
> +{
> + int ret;
> + void *iter;
> + struct scmi_iterator_ops ops = {
> + .prepare_message = iter_pinctrl_settings_get_prepare_message,
> + .update_state = iter_pinctrl_settings_get_update_state,
> + .process_response = iter_pinctrl_settings_get_process_response,
> + };
> + struct scmi_settings_get_ipriv ipriv = {
> + .selector = selector,
> + .type = type,
> + .flag = 0,
> + .config_types = &config_type,
> + .config_values = config_value,
> + };
> +
So this function is used to retrieve configs; as of now, just one, then
it could be extended to fetch all the configs, and for this it uses the
iterators helpers, BUT it is not and will not be used to just fetch the
selected_function with flag_2 (even though is always provided), since in
that case you wont get back a multi-part SCMI response and so there is no
need to use iterators...
IOW... no need here to handle flag_2 scenario and as a consequence I would
change the ipriv flag to be be a boolean .get_all, like it was, since it is
more readable (and so you wont need to add any comment..)
In the future could make sense to add here also a *selected_function output
param to this function since you will always get it back for free when
retrieving configs ... BUT for now is just not needed really...no users
for this case till now...
...when the time will come that we will need a function_selected_get to
be issued without retrieveing also the configs I would add a distinct
routine that crafts properly a SETTINGS_GET with flag_2 without worrying
about multi-part responses (and with no need for iterators support)
Trying to handle all in here just complicates stuff...
> + if (!config_value || type == FUNCTION_TYPE)
> + return -EINVAL;
> +
> + ret = scmi_pinctrl_validate_id(ph, selector, type);
> + if (ret)
> + return ret;
> +
> + iter = ph->hops->iter_response_init(ph, &ops, 1, PINCTRL_SETTINGS_GET,
> + sizeof(struct scmi_msg_settings_get),
> + &ipriv);
> +
> + if (IS_ERR(iter))
> + return PTR_ERR(iter);
> +
> + return ph->hops->iter_response_run(iter);
> +}
> +
> +static int
> +scmi_pinctrl_settings_conf(const struct scmi_protocol_handle *ph,
> + u32 selector,
> + enum scmi_pinctrl_selector_type type,
> + u32 nr_configs,
> + enum scmi_pinctrl_conf_type *config_type,
> + u32 *config_value)
> +{
> + struct scmi_xfer *t;
> + struct scmi_msg_settings_conf *tx;
> + u32 attributes;
> + int ret, i;
> + u32 configs_in_chunk, conf_num = 0;
> + u32 chunk;
> + int max_msg_size = ph->hops->get_max_msg_size(ph);
> +
> + if (!config_type || !config_value || type == FUNCTION_TYPE)
> + return -EINVAL;
> +
> + ret = scmi_pinctrl_validate_id(ph, selector, type);
> + if (ret)
> + return ret;
> +
> + configs_in_chunk = (max_msg_size - sizeof(*tx)) / (sizeof(__le32) * 2);
> + while (conf_num < nr_configs) {
> + chunk = (nr_configs - conf_num > configs_in_chunk) ?
> + configs_in_chunk : nr_configs - conf_num;
> +
> + ret = ph->xops->xfer_get_init(ph, PINCTRL_SETTINGS_CONFIGURE,
> + sizeof(*tx) +
> + chunk * 2 * sizeof(__le32),
> + 0, &t);
> + if (ret)
> + return ret;
for consistency I would
break;
like below and you will exit always from the last return ret;
> +
> + tx = t->tx.buf;
> + tx->identifier = cpu_to_le32(selector);
> + attributes = FIELD_PREP(GENMASK(1, 0), type) |
> + FIELD_PREP(GENMASK(9, 2), chunk);
> + tx->attributes = cpu_to_le32(attributes);
> +
> + for (i = 0; i < chunk; i++) {
> + tx->configs[i * 2] =
> + cpu_to_le32(config_type[conf_num + i]);
> + tx->configs[i * 2 + 1] =
> + cpu_to_le32(config_value[conf_num + i]);
> + }
> +
> + ret = ph->xops->do_xfer(ph, t);
> +
> + ph->xops->xfer_put(ph, t);
> +
> + if (ret)
> + break;
> +
> + conf_num += chunk;
> + }
> +
> + return ret;
> +}
> +
> +static int scmi_pinctrl_function_select(const struct scmi_protocol_handle *ph,
> + u32 group,
> + enum scmi_pinctrl_selector_type type,
> + u32 function_id)
> +{
> + int ret;
> + struct scmi_xfer *t;
> + struct scmi_msg_settings_conf *tx;
> + u32 attributes;
> +
> + ret = scmi_pinctrl_validate_id(ph, group, type);
> + if (ret)
> + return ret;
> +
> + ret = ph->xops->xfer_get_init(ph, PINCTRL_SETTINGS_CONFIGURE,
> + sizeof(*tx), 0, &t);
> + if (ret)
> + return ret;
> +
> + tx = t->tx.buf;
> + tx->identifier = cpu_to_le32(group);
> + tx->function_id = cpu_to_le32(function_id);
> + attributes = FIELD_PREP(GENMASK(1, 0), type) | BIT(10);
> + tx->attributes = cpu_to_le32(attributes);
> +
> + ret = ph->xops->do_xfer(ph, t);
> + ph->xops->xfer_put(ph, t);
> +
> + return ret;
> +}
> +
> +static int scmi_pinctrl_request(const struct scmi_protocol_handle *ph,
> + u32 identifier,
> + enum scmi_pinctrl_selector_type type)
> +{
> + int ret;
> + struct scmi_xfer *t;
> + struct scmi_msg_request *tx;
> +
> + if (type == FUNCTION_TYPE)
> + return -EINVAL;
> +
> + ret = scmi_pinctrl_validate_id(ph, identifier, type);
> + if (ret)
> + return ret;
> +
> + ret = ph->xops->xfer_get_init(ph, PINCTRL_REQUEST, sizeof(*tx), 0, &t);
> + if (ret)
> + return ret;
> +
> + tx = t->tx.buf;
> + tx->identifier = cpu_to_le32(identifier);
> + tx->flags = cpu_to_le32(type);
> +
> + ret = ph->xops->do_xfer(ph, t);
> + ph->xops->xfer_put(ph, t);
> +
> + return ret;
> +}
> +
..this function ...
> +static int scmi_pinctrl_pin_request(const struct scmi_protocol_handle *ph,
> + u32 pin)
> +{
> + return scmi_pinctrl_request(ph, pin, PIN_TYPE);
> +}
> +
> +static int scmi_pinctrl_free(const struct scmi_protocol_handle *ph,
> + u32 identifier,
> + enum scmi_pinctrl_selector_type type)
> +{
> + int ret;
> + struct scmi_xfer *t;
> + struct scmi_msg_request *tx;
> +
> + if (type == FUNCTION_TYPE)
> + return -EINVAL;
> +
> + ret = scmi_pinctrl_validate_id(ph, identifier, type);
> + if (ret)
> + return ret;
> +
> + ret = ph->xops->xfer_get_init(ph, PINCTRL_RELEASE, sizeof(*tx), 0, &t);
> + if (ret)
> + return ret;
> +
> + tx = t->tx.buf;
> + tx->identifier = cpu_to_le32(identifier);
> + tx->flags = cpu_to_le32(type);
> +
> + ret = ph->xops->do_xfer(ph, t);
> + ph->xops->xfer_put(ph, t);
> +
> + return ret;
> +}
> +
...and this are completely identical, beside the used command msg_id...please make
it a common workhorse function by adding a param for the command...
> +static int scmi_pinctrl_pin_free(const struct scmi_protocol_handle *ph, u32 pin)
> +{
> + return scmi_pinctrl_free(ph, pin, PIN_TYPE);
> +}
> +
...and convert these _request/_free functions into a pair odf simple wrapper invoking
the common workhorse...
> +static int scmi_pinctrl_get_group_info(const struct scmi_protocol_handle *ph,
> + u32 selector,
> + struct scmi_group_info *group)
> +{
> + int ret;
> +
> + if (!group)
> + return -EINVAL;
> +
> + ret = scmi_pinctrl_attributes(ph, GROUP_TYPE, selector,
> + group->name,
> + &group->nr_pins);
> + if (ret)
> + return ret;
> +
> + if (!group->nr_pins) {
> + dev_err(ph->dev, "Group %d has 0 elements", selector);
> + return -ENODATA;
> + }
> +
> + group->group_pins = kmalloc_array(group->nr_pins,
> + sizeof(*group->group_pins),
> + GFP_KERNEL);
> + if (!group->group_pins)
> + return -ENOMEM;
> +
> + ret = scmi_pinctrl_list_associations(ph, selector, GROUP_TYPE,
> + group->nr_pins, group->group_pins);
> + if (ret) {
> + kfree(group->group_pins);
> + return ret;
> + }
> +
> + group->present = true;
> + return 0;
> +}
> +
> +static int scmi_pinctrl_get_group_name(const struct scmi_protocol_handle *ph,
> + u32 selector, const char **name)
> +{
> + struct scmi_pinctrl_info *pi = ph->get_priv(ph);
> +
> + if (!name)
> + return -EINVAL;
> +
> + if (selector >= pi->nr_groups)
> + return -EINVAL;
> +
> + if (!pi->groups[selector].present) {
> + int ret;
> +
> + ret = scmi_pinctrl_get_group_info(ph, selector,
> + &pi->groups[selector]);
> + if (ret)
> + return ret;
> + }
> +
> + *name = pi->groups[selector].name;
> +
> + return 0;
> +}
> +
> +static int scmi_pinctrl_group_pins_get(const struct scmi_protocol_handle *ph,
> + u32 selector, const u32 **pins,
> + u32 *nr_pins)
> +{
> + struct scmi_pinctrl_info *pi = ph->get_priv(ph);
> +
> + if (!pins || !nr_pins)
> + return -EINVAL;
> +
> + if (selector >= pi->nr_groups)
> + return -EINVAL;
> +
> + if (!pi->groups[selector].present) {
> + int ret;
> +
> + ret = scmi_pinctrl_get_group_info(ph, selector,
> + &pi->groups[selector]);
> + if (ret)
> + return ret;
> + }
> +
> + *pins = pi->groups[selector].group_pins;
> + *nr_pins = pi->groups[selector].nr_pins;
> +
> + return 0;
> +}
> +
> +static int scmi_pinctrl_get_function_info(const struct scmi_protocol_handle *ph,
> + u32 selector,
> + struct scmi_function_info *func)
> +{
> + int ret;
> +
> + if (!func)
> + return -EINVAL;
> +
> + ret = scmi_pinctrl_attributes(ph, FUNCTION_TYPE, selector,
> + func->name,
> + &func->nr_groups);
> + if (ret)
> + return ret;
> +
> + if (!func->nr_groups) {
> + dev_err(ph->dev, "Function %d has 0 elements", selector);
> + return -ENODATA;
> + }
> +
> + func->groups = kmalloc_array(func->nr_groups, sizeof(*func->groups),
> + GFP_KERNEL);
> + if (!func->groups)
> + return -ENOMEM;
> +
> + ret = scmi_pinctrl_list_associations(ph, selector, FUNCTION_TYPE,
> + func->nr_groups, func->groups);
> + if (ret) {
> + kfree(func->groups);
> + return ret;
> + }
> +
> + func->present = true;
> + return 0;
> +}
> +
> +static int scmi_pinctrl_get_function_name(const struct scmi_protocol_handle *ph,
> + u32 selector, const char **name)
> +{
> + struct scmi_pinctrl_info *pi = ph->get_priv(ph);
> +
> + if (!name)
> + return -EINVAL;
> +
> + if (selector >= pi->nr_functions)
> + return -EINVAL;
> +
> + if (!pi->functions[selector].present) {
> + int ret;
> +
> + ret = scmi_pinctrl_get_function_info(ph, selector,
> + &pi->functions[selector]);
> + if (ret)
> + return ret;
> + }
> +
> + *name = pi->functions[selector].name;
> + return 0;
> +}
> +
> +static int
> +scmi_pinctrl_function_groups_get(const struct scmi_protocol_handle *ph,
> + u32 selector, u32 *nr_groups,
> + const u32 **groups)
> +{
> + struct scmi_pinctrl_info *pi = ph->get_priv(ph);
> +
> + if (!groups || !nr_groups)
> + return -EINVAL;
> +
> + if (selector >= pi->nr_functions)
> + return -EINVAL;
> +
> + if (!pi->functions[selector].present) {
> + int ret;
> +
> + ret = scmi_pinctrl_get_function_info(ph, selector,
> + &pi->functions[selector]);
> + if (ret)
> + return ret;
> + }
> +
> + *groups = pi->functions[selector].groups;
> + *nr_groups = pi->functions[selector].nr_groups;
> +
> + return 0;
> +}
> +
> +static int scmi_pinctrl_mux_set(const struct scmi_protocol_handle *ph,
> + u32 selector, u32 group)
> +{
> + return scmi_pinctrl_function_select(ph, group, GROUP_TYPE, selector);
> +}
> +
> +static int scmi_pinctrl_get_pin_info(const struct scmi_protocol_handle *ph,
> + u32 selector, struct scmi_pin_info *pin)
> +{
> + int ret;
> +
> + if (!pin)
> + return -EINVAL;
> +
> + ret = scmi_pinctrl_attributes(ph, PIN_TYPE, selector,
> + pin->name, NULL);
> + if (ret)
> + return ret;
> +
> + pin->present = true;
> + return 0;
> +}
> +
> +static int scmi_pinctrl_get_pin_name(const struct scmi_protocol_handle *ph,
> + u32 selector, const char **name)
> +{
> + struct scmi_pinctrl_info *pi = ph->get_priv(ph);
> +
> + if (!name)
> + return -EINVAL;
> +
> + if (selector >= pi->nr_pins)
> + return -EINVAL;
> +
> + if (!pi->pins[selector].present) {
> + int ret;
> +
> + ret = scmi_pinctrl_get_pin_info(ph, selector,
> + &pi->pins[selector]);
> + if (ret)
> + return ret;
> + }
> +
> + *name = pi->pins[selector].name;
> +
> + return 0;
> +}
> +
> +static int scmi_pinctrl_name_get(const struct scmi_protocol_handle *ph,
> + u32 selector,
> + enum scmi_pinctrl_selector_type type,
> + const char **name)
> +{
> + switch (type) {
> + case PIN_TYPE:
> + return scmi_pinctrl_get_pin_name(ph, selector, name);
> + case GROUP_TYPE:
> + return scmi_pinctrl_get_group_name(ph, selector, name);
> + case FUNCTION_TYPE:
> + return scmi_pinctrl_get_function_name(ph, selector, name);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static const struct scmi_pinctrl_proto_ops pinctrl_proto_ops = {
> + .count_get = scmi_pinctrl_count_get,
> + .name_get = scmi_pinctrl_name_get,
> + .group_pins_get = scmi_pinctrl_group_pins_get,
> + .function_groups_get = scmi_pinctrl_function_groups_get,
> + .mux_set = scmi_pinctrl_mux_set,
> + .settings_get = scmi_pinctrl_settings_get,
> + .settings_conf = scmi_pinctrl_settings_conf,
> + .pin_request = scmi_pinctrl_pin_request,
> + .pin_free = scmi_pinctrl_pin_free,
> +};
> +
> +static int scmi_pinctrl_protocol_init(const struct scmi_protocol_handle *ph)
> +{
> + int ret;
> + u32 version;
> + struct scmi_pinctrl_info *pinfo;
> +
> + ret = ph->xops->version_get(ph, &version);
> + if (ret)
> + return ret;
> +
> + dev_dbg(ph->dev, "Pinctrl Version %d.%d\n",
> + PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
> +
> + pinfo = devm_kzalloc(ph->dev, sizeof(*pinfo), GFP_KERNEL);
> + if (!pinfo)
> + return -ENOMEM;
> +
> + ret = scmi_pinctrl_attributes_get(ph, pinfo);
> + if (ret)
> + return ret;
..as a I was saying is nr_pins == 0 the scmi_pinctrl_attributes_get
could return -EINVAL here and bail out....not sure that a running setup
with zero pins has any values (even for testing...) BUT, as said above,
I wuld certainly add a dev_warn in scmi_pinctrl_attributes_get() when
nr_pins == 0
Thanks,
Cristian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 19/20] virt: geniezone: Add tracing support for hyp call and vcpu exit_reason
From: Steven Rostedt @ 2024-03-28 15:40 UTC (permalink / raw)
To: Yi-De Wu
Cc: Yingshiuan Pan, Ze-Yu Wang, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Corbet, Catalin Marinas, Wihl Deacon,
Masami Hiramatsu, Mathieu Desnoyers, Richard Cochran,
Matthias Brugger, AngeloGioacchino Del Regno, devicetree,
linux-kernel, linux-doc, linux-arm-kernel, linux-trace-kernel,
netdev, linux-mediatek, David Bradil, Trilok Soni, Jade Shih,
Ivan Tseng, My Chuang, Shawn Hsiao, PeiLun Suei, Liju Chen,
Willix Yeh, Kevenny Hsieh
In-Reply-To: <20231228105147.13752-20-yi-de.wu@mediatek.com>
On Thu, 28 Dec 2023 18:51:46 +0800
Yi-De Wu <yi-de.wu@mediatek.com> wrote:
> Add tracepoints for hypervisor calls and VCPU exit reasons in GenieZone
> driver. It aids performance debugging by providing more information
> about hypervisor operations and VCPU behavior.
>
> Command Usage:
> echo geniezone:* >> /sys/kernel/tracing/set_event
> echo 1 > /sys/kernel/tracing/tracing_on
> echo 0 > /sys/kernel/tracing/tracing_on
> cat /sys/kernel/tracing/trace
>
> For example:
> crosvm_vcpu0-4838 [004] ..... 76053.536034: mtk_hypcall_enter: id=0xbb001005
> crosvm_vcpu0-4838 [004] ..... 76053.540039: mtk_hypcall_leave: id=0xbb001005 invalid=0
> crosvm_vcpu0-4838 [004] ..... 76053.540040: mtk_vcpu_exit: vcpu exit_reason=0x92920003
Cleaning out patchwork, I noticed this patch.
You can make the above more informative by having it output:
crosvm_vcpu0-4838 [004] ..... 76053.540040: mtk_vcpu_exit: vcpu exit_reason=IRQ
>
> This example tracks a hypervisor function call by an ID (`0xbb001005`)
> from initiation to termination, which is supported (invalid=0). A vCPU
> exit is triggered by an Interrupt Request (IRQ) (exit reason: 0x92920003).
>
> /* VM exit reason */
> enum {
> GZVM_EXIT_UNKNOWN = 0x92920000,
> GZVM_EXIT_MMIO = 0x92920001,
> GZVM_EXIT_HYPERCALL = 0x92920002,
> GZVM_EXIT_IRQ = 0x92920003,
> GZVM_EXIT_EXCEPTION = 0x92920004,
> GZVM_EXIT_DEBUG = 0x92920005,
> GZVM_EXIT_FAIL_ENTRY = 0x92920006,
> GZVM_EXIT_INTERNAL_ERROR = 0x92920007,
> GZVM_EXIT_SYSTEM_EVENT = 0x92920008,
> GZVM_EXIT_SHUTDOWN = 0x92920009,
> GZVM_EXIT_GZ = 0x9292000a,
> };
>
> Signed-off-by: Liju-clr Chen <liju-clr.chen@mediatek.com>
> Signed-off-by: Yi-De Wu <yi-de.wu@mediatek.com>
> ---
> arch/arm64/geniezone/vm.c | 5 +++
> drivers/virt/geniezone/gzvm_vcpu.c | 3 ++
> include/trace/events/geniezone.h | 54 ++++++++++++++++++++++++++++++
> 3 files changed, 62 insertions(+)
> create mode 100644 include/trace/events/geniezone.h
>
> diff --git a/arch/arm64/geniezone/vm.c b/arch/arm64/geniezone/vm.c
> index a9d264bbb3b1..5667643251b5 100644
> --- a/arch/arm64/geniezone/vm.c
> +++ b/arch/arm64/geniezone/vm.c
> @@ -7,6 +7,8 @@
> #include <linux/err.h>
> #include <linux/uaccess.h>
>
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/geniezone.h>
> #include <linux/gzvm.h>
> #include <linux/gzvm_drv.h>
> #include "gzvm_arch_common.h"
> @@ -33,7 +35,10 @@ int gzvm_hypcall_wrapper(unsigned long a0, unsigned long a1,
> unsigned long a6, unsigned long a7,
> struct arm_smccc_res *res)
> {
> + trace_mtk_hypcall_enter(a0);
> arm_smccc_hvc(a0, a1, a2, a3, a4, a5, a6, a7, res);
> + trace_mtk_hypcall_leave(a0, (res->a0 != ERR_NOT_SUPPORTED) ? 0 : 1);
> +
> return gzvm_err_to_errno(res->a0);
> }
>
> diff --git a/drivers/virt/geniezone/gzvm_vcpu.c b/drivers/virt/geniezone/gzvm_vcpu.c
> index 86c690749277..138ec064596b 100644
> --- a/drivers/virt/geniezone/gzvm_vcpu.c
> +++ b/drivers/virt/geniezone/gzvm_vcpu.c
> @@ -10,6 +10,8 @@
> #include <linux/mm.h>
> #include <linux/platform_device.h>
> #include <linux/slab.h>
> +
> +#include <trace/events/geniezone.h>
> #include <linux/gzvm_drv.h>
>
> /* maximum size needed for holding an integer */
> @@ -103,6 +105,7 @@ static long gzvm_vcpu_run(struct gzvm_vcpu *vcpu, void __user *argp)
>
> while (!need_userspace && !signal_pending(current)) {
> gzvm_arch_vcpu_run(vcpu, &exit_reason);
> + trace_mtk_vcpu_exit(exit_reason);
>
> switch (exit_reason) {
> case GZVM_EXIT_MMIO:
> diff --git a/include/trace/events/geniezone.h b/include/trace/events/geniezone.h
> new file mode 100644
> index 000000000000..1fa44f9c4b3c
> --- /dev/null
> +++ b/include/trace/events/geniezone.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2023 MediaTek Inc.
> + */
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM geniezone
> +
> +#define _TRACE_GENIEZONE_H
> +
> +#include <linux/tracepoint.h>
#define GZVM_EXIT_REASONS \
EM(UNKNOWN) \
EM(MMIO) \
EM(HYPERCALL) \
EM(IRQ) \
EM(EXCEPTION) \
EM(DEBUG) \
EM(FAIL_ENTRY) \
EM(INTERNAL_ERROR) \
EM(SYSTEM_EVENT) \
EM(SHUTDOWN) \
EMe(GZ)
#undef EM
#undef EMe
#define EM(a) TRACE_DEFINE_ENUM(GZVM_EXIT_##a);
#define EMe(a) TRACE_DEFINE_ENUM(GZVM_EXIT_##a);
GZVM_EXIT_REASONS
#undef EM
#undef EMe
#define EM(a) { GZVM_EXIT_##a, #a },
#define EMe(a) { GZVM_EXIT_##a, #a }
> +
> +TRACE_EVENT(mtk_hypcall_enter,
> + TP_PROTO(unsigned long id),
> +
> + TP_ARGS(id),
> +
> + TP_STRUCT__entry(__field(unsigned long, id)),
> +
> + TP_fast_assign(__entry->id = id;),
> +
> + TP_printk("id=0x%lx", __entry->id)
> +);
> +
> +TRACE_EVENT(mtk_hypcall_leave,
> + TP_PROTO(unsigned long id, unsigned long invalid),
> +
> + TP_ARGS(id, invalid),
> +
> + TP_STRUCT__entry(__field(unsigned long, id)
> + __field(unsigned long, invalid)
> + ),
> +
> + TP_fast_assign(__entry->id = id;
> + __entry->invalid = invalid;
> + ),
> +
> + TP_printk("id=0x%lx invalid=%lu", __entry->id, __entry->invalid)
> +);
> +
> +TRACE_EVENT(mtk_vcpu_exit,
> + TP_PROTO(unsigned long exit_reason),
> +
> + TP_ARGS(exit_reason),
> +
> + TP_STRUCT__entry(__field(unsigned long, exit_reason)),
> +
> + TP_fast_assign(__entry->exit_reason = exit_reason;),
> +
> + TP_printk("vcpu exit_reason=0x%lx", __entry->exit_reason)
TP_printk("vcpu exit_reason=0x%lx",
__print_symbolic(__entry->exit_reason, GZVM_EXIT_REASONS))
And instead of having the cryptic enum values printed, you will have human
readable reasons.
-- Steve
> +);
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] regset: use kvzalloc() for regset_get_alloc()
From: Doug Anderson @ 2024-03-28 15:36 UTC (permalink / raw)
To: Catalin Marinas
Cc: Alexander Viro, Christian Brauner, Andrew Morton, Mark Brown,
Will Deacon, Dave Martin, Oleg Nesterov, linux-arm-kernel,
Matthew Wilcox, Eric Biederman, Jan Kara, Kees Cook,
linux-fsdevel, linux-kernel, linux-mm
In-Reply-To: <ZgWNtmcyZOMZR1Fi@arm.com>
Hi,
On Thu, Mar 28, 2024 at 8:33 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> > I'm not trying to be a pest here, so if this is on someone's todo list
> > and they'll get to it eventually then feel free to tell me to go away
> > and I'll snooze this for another few months. I just want to make sure
> > it's not forgotten.
> >
> > I've been assuming that someone like Al Viro or Christian Brauner
> > would land this patch eventually and I know Al responded rather
> > quickly to my v1 [2]. I think all of Al's issues were resolved by Mark
> > Brown's patch [1] (which has landed in the arm64 tree) and my updating
> > of the patch description in v2. I see that Al and Christian are
> > flagged as maintainers of "fs/binfmt_elf.c" which is one of the two
> > files I'm touching, so that's mostly why I was assuming they would
> > land it.
> >
> > ...but I realize that perhaps my assumptions are wrong and this needs
> > to go through a different maintainer. In this case (if I'm reading it
> > correctly) Al and Christian are listed because the file is under "fs"
> > even though this isn't _really_ much of a filesystem-related patch.
> > Perhaps this needs to go through something like Andrew Morton's tree
> > since he often picks up patches that have nowhere else to land? If
> > someone else has suggestions, I'm all ears. I'm also happy to repost
> > this patch in case it helps with a maintainer applying it.
>
> FWIW, for this patch:
>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Thanks!
> Yeah, normally Al or Christian would take it but with their ack we can
> also take it through the arm64 tree (or Andrew can pick it up through
> the mm tree).
OK, let's see what folks say.
> With Mark's fix, I assume this is no longer urgent, cc stable material,
> but rather something nice in the future to reduce the risk of allocation
> failure on this path.
It's not quite as urgent as before Mark's fix, which gets rid of the
order 7 allocation. ...but an unnecessary order 5 allocation is still
nothing to sneeze at. I'd let others make the decision about whether
to CC stable, but I'll at least advocate backporting it to all the
kernel trees I'm directly involved in.
-Doug
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] regset: use kvzalloc() for regset_get_alloc()
From: Catalin Marinas @ 2024-03-28 15:33 UTC (permalink / raw)
To: Doug Anderson
Cc: Alexander Viro, Christian Brauner, Andrew Morton, Mark Brown,
Will Deacon, Dave Martin, Oleg Nesterov, linux-arm-kernel,
Matthew Wilcox, Eric Biederman, Jan Kara, Kees Cook,
linux-fsdevel, linux-kernel, linux-mm
In-Reply-To: <CAD=FV=U82H41q3sKxZK_i1ffaQuqwFo98MLiPhSo=mY8SWLJcA@mail.gmail.com>
On Thu, Mar 28, 2024 at 07:16:37AM -0700, Doug Anderson wrote:
> On Mon, Feb 26, 2024 at 3:55 PM Doug Anderson <dianders@chromium.org> wrote:
> > On Mon, Feb 5, 2024 at 9:27 AM Douglas Anderson <dianders@chromium.org> wrote:
> > > While browsing through ChromeOS crash reports, I found one with an
> > > allocation failure that looked like this:
> > >
> > > chrome: page allocation failure: order:7,
> > > mode:0x40dc0(GFP_KERNEL|__GFP_COMP|__GFP_ZERO),
> > > nodemask=(null),cpuset=urgent,mems_allowed=0
> > > CPU: 7 PID: 3295 Comm: chrome Not tainted
> > > 5.15.133-20574-g8044615ac35c #1 (HASH:1162 1)
> > > Hardware name: Google Lazor (rev3 - 8) with KB Backlight (DT)
> > > Call trace:
> > > ...
> > > warn_alloc+0x104/0x174
> > > __alloc_pages+0x5f0/0x6e4
> > > kmalloc_order+0x44/0x98
> > > kmalloc_order_trace+0x34/0x124
> > > __kmalloc+0x228/0x36c
> > > __regset_get+0x68/0xcc
> > > regset_get_alloc+0x1c/0x28
> > > elf_core_dump+0x3d8/0xd8c
> > > do_coredump+0xeb8/0x1378
> > > get_signal+0x14c/0x804
> > > ...
> > >
> > > An order 7 allocation is (1 << 7) contiguous pages, or 512K. It's not
> > > a surprise that this allocation failed on a system that's been running
> > > for a while.
> > >
> > > More digging showed that it was fairly easy to see the order 7
> > > allocation by just sending a SIGQUIT to chrome (or other processes) to
> > > generate a core dump. The actual amount being allocated was 279,584
> > > bytes and it was for "core_note_type" NT_ARM_SVE.
> > >
> > > There was quite a bit of discussion [1] on the mailing lists in
> > > response to my v1 patch attempting to switch to vmalloc. The overall
> > > conclusion was that we could likely reduce the 279,584 byte allocation
> > > by quite a bit and Mark Brown has sent a patch to that effect [2].
> > > However even with the 279,584 byte allocation gone there are still
> > > 65,552 byte allocations. These are just barely more than the 65,536
> > > bytes and thus would require an order 5 allocation.
> > >
> > > An order 5 allocation is still something to avoid unless necessary and
> > > nothing needs the memory here to be contiguous. Change the allocation
> > > to kvzalloc() which should still be efficient for small allocations
> > > but doesn't force the memory subsystem to work hard (and maybe fail)
> > > at getting a large contiguous chunk.
> > >
> > > [1] https://lore.kernel.org/r/20240201171159.1.Id9ad163b60d21c9e56c2d686b0cc9083a8ba7924@changeid
> > > [2] https://lore.kernel.org/r/20240203-arm64-sve-ptrace-regset-size-v1-1-2c3ba1386b9e@kernel.org
> > >
> > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > > ---
> > >
> > > Changes in v2:
> > > - Use kvzalloc() instead of vmalloc().
> > > - Update description based on v1 discussion.
> > >
> > > fs/binfmt_elf.c | 2 +-
> > > kernel/regset.c | 6 +++---
> > > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > Just wanted to check in to see if there's anything else that I need to
> > do here. Mark's patch to avoid the order 7 allocations [1] has landed,
> > but we still want this kvzalloc() because the order 5 allocations
> > can't really be avoided. I'm happy to sit tight for longer but just
> > wanted to make sure it was clear that we still want my patch _in
> > addition_ to Mark's patch and to see if there was anything else you
> > needed me to do.
> >
> > Thanks!
> >
> > [1] https://lore.kernel.org/r/20240213-arm64-sve-ptrace-regset-size-v2-1-c7600ca74b9b@kernel.org
>
> I'm not trying to be a pest here, so if this is on someone's todo list
> and they'll get to it eventually then feel free to tell me to go away
> and I'll snooze this for another few months. I just want to make sure
> it's not forgotten.
>
> I've been assuming that someone like Al Viro or Christian Brauner
> would land this patch eventually and I know Al responded rather
> quickly to my v1 [2]. I think all of Al's issues were resolved by Mark
> Brown's patch [1] (which has landed in the arm64 tree) and my updating
> of the patch description in v2. I see that Al and Christian are
> flagged as maintainers of "fs/binfmt_elf.c" which is one of the two
> files I'm touching, so that's mostly why I was assuming they would
> land it.
>
> ...but I realize that perhaps my assumptions are wrong and this needs
> to go through a different maintainer. In this case (if I'm reading it
> correctly) Al and Christian are listed because the file is under "fs"
> even though this isn't _really_ much of a filesystem-related patch.
> Perhaps this needs to go through something like Andrew Morton's tree
> since he often picks up patches that have nowhere else to land? If
> someone else has suggestions, I'm all ears. I'm also happy to repost
> this patch in case it helps with a maintainer applying it.
FWIW, for this patch:
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Yeah, normally Al or Christian would take it but with their ack we can
also take it through the arm64 tree (or Andrew can pick it up through
the mm tree).
With Mark's fix, I assume this is no longer urgent, cc stable material,
but rather something nice in the future to reduce the risk of allocation
failure on this path.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 1/1] dt-bindings: net: dwmac: Document STM32 property st,ext-phyclk
From: Rob Herring @ 2024-03-28 15:25 UTC (permalink / raw)
To: Christophe Roullier
Cc: linux-arm-kernel, Marek Vasut, Mark Brown, Jose Abreu,
Eric Dumazet, linux-kernel, Richard Cochran, Jakub Kicinski,
Rob Herring, linux-stm32, devicetree, Liam Girdwood,
David S . Miller, Maxime Coquelin, Conor Dooley,
Krzysztof Kozlowski, Alexandre Torgue, netdev, Paolo Abeni
In-Reply-To: <20240328140803.324141-2-christophe.roullier@foss.st.com>
On Thu, 28 Mar 2024 15:08:03 +0100, Christophe Roullier wrote:
> The Linux kernel dwmac-stm32 driver currently supports three DT
> properties used to configure whether PHY clock are generated by
> the MAC or supplied to the MAC from the PHY.
>
> Originally there were two properties, st,eth-clk-sel and
> st,eth-ref-clk-sel, each used to configure MAC clocking in
> different bus mode and for different MAC clock frequency.
> Since it is possible to determine the MAC 'eth-ck' clock
> frequency from the clock subsystem and PHY bus mode from
> the 'phy-mode' property, two disparate DT properties are
> no longer required to configure MAC clocking.
>
> Linux kernel commit 1bb694e20839 ("net: ethernet: stmmac: simplify phy modes management for stm32")
> introduced a third, unified, property st,ext-phyclk. This property
> covers both use cases of st,eth-clk-sel and st,eth-ref-clk-sel DT
> properties, as well as a new use case for 25 MHz clock generated
> by the MAC.
>
> The third property st,ext-phyclk is so far undocumented,
> document it.
>
> Below table summarizes the clock requirement and clock sources for
> supported PHY interface modes.
> __________________________________________________________________________
> |PHY_MODE | Normal | PHY wo crystal| PHY wo crystal |No 125Mhz from PHY|
> | | | 25MHz | 50MHz | |
>
> ---------------------------------------------------------------------------
> | MII | - | eth-ck | n/a | n/a |
> | | | st,ext-phyclk | | |
>
> ---------------------------------------------------------------------------
> | GMII | - | eth-ck | n/a | n/a |
> | | | st,ext-phyclk | | |
>
> ---------------------------------------------------------------------------
> | RGMII | - | eth-ck | n/a | eth-ck |
> | | | st,ext-phyclk | | st,eth-clk-sel or|
> | | | | | st,ext-phyclk |
>
> ---------------------------------------------------------------------------
> | RMII | - | eth-ck | eth-ck | n/a |
> | | | st,ext-phyclk | st,eth-ref-clk-sel | |
> | | | | or st,ext-phyclk | |
>
> ---------------------------------------------------------------------------
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
> ---
> Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 7 +++++++
> 1 file changed, 7 insertions(+)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
./Documentation/devicetree/bindings/net/stm32-dwmac.yaml:86:5: [warning] wrong indentation: expected 2 but found 4 (indentation)
./Documentation/devicetree/bindings/net/stm32-dwmac.yaml:92:3: [error] syntax error: expected <block end>, but found '<block mapping start>' (syntax)
dtschema/dtc warnings/errors:
make[2]: *** Deleting file 'Documentation/devicetree/bindings/net/stm32-dwmac.example.dts'
Documentation/devicetree/bindings/net/stm32-dwmac.yaml:92:3: did not find expected key
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/net/stm32-dwmac.example.dts] Error 1
make[2]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/net/stm32-dwmac.yaml:92:3: did not find expected key
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/stm32-dwmac.yaml: ignoring, error parsing file
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240328140803.324141-2-christophe.roullier@foss.st.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] ARM: dts: imx7s-warp: Pass OV2680 link-frequencies
From: Fabio Estevam @ 2024-03-28 15:19 UTC (permalink / raw)
To: shawnguo
Cc: sakari.ailus, hdegoede, robh, krzysztof.kozlowski+dt, conor+dt,
linux-arm-kernel, devicetree, Fabio Estevam, stable
From: Fabio Estevam <festevam@denx.de>
Since commit 63b0cd30b78e ("media: ov2680: Add bus-cfg / endpoint
property verification") the ov2680 no longer probes on a imx7s-warp7:
ov2680 1-0036: error -EINVAL: supported link freq 330000000 not found
ov2680 1-0036: probe with driver ov2680 failed with error -22
Fix it by passing the required 'link-frequencies' property as
recommended by:
https://www.kernel.org/doc/html/v6.9-rc1/driver-api/media/camera-sensor.html#handling-clocks
Cc: stable@vger.kernel.org
Fixes: 63b0cd30b78e ("media: ov2680: Add bus-cfg / endpoint property verification")
Signed-off-by: Fabio Estevam <festevam@denx.de>
---
arch/arm/boot/dts/nxp/imx/imx7s-warp.dts | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/nxp/imx/imx7s-warp.dts b/arch/arm/boot/dts/nxp/imx/imx7s-warp.dts
index ba7231b364bb..7bab113ca6da 100644
--- a/arch/arm/boot/dts/nxp/imx/imx7s-warp.dts
+++ b/arch/arm/boot/dts/nxp/imx/imx7s-warp.dts
@@ -210,6 +210,7 @@ ov2680_to_mipi: endpoint {
remote-endpoint = <&mipi_from_sensor>;
clock-lanes = <0>;
data-lanes = <1>;
+ link-frequencies = /bits/ 64 <330000000>;
};
};
};
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v1 1/2] dt-bindings: thermal: amlogic: add support for A1 thermal sensor
From: neil.armstrong @ 2024-03-28 14:07 UTC (permalink / raw)
To: Dmitry Rokosov, jbrunet, mturquette, khilman, martin.blumenstingl,
glaroque, rafael, daniel.lezcano, rui.zhang, lukasz.luba, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: kernel, rockosov, linux-amlogic, linux-pm, linux-kernel,
devicetree, linux-arm-kernel
In-Reply-To: <20240328133802.15651-2-ddrokosov@salutedevices.com>
Hi,
On 28/03/2024 14:37, Dmitry Rokosov wrote:
> Provide right compatible properties for Amlogic A1 Thermal Sensor
> controller. A1 family supports only one thermal node - CPU thermal
> sensor.
>
> Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
> ---
> .../bindings/thermal/amlogic,thermal.yaml | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
> index 20f8f9b3b971..0e7f6568d385 100644
> --- a/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
> +++ b/Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml
> @@ -13,11 +13,15 @@ description: Binding for Amlogic Thermal
>
> properties:
> compatible:
> - items:
> - - enum:
> - - amlogic,g12a-cpu-thermal
> - - amlogic,g12a-ddr-thermal
> - - const: amlogic,g12a-thermal
> + oneOf:
> + - items:
> + - enum:
> + - amlogic,g12a-cpu-thermal
> + - amlogic,g12a-ddr-thermal
> + - const: amlogic,g12a-thermal
> + - items:
> + - const: amlogic,a1-cpu-thermal
> + - const: amlogic,a1-thermal
In this case you can just use "amlogic,a1-cpu-thermal" or "amlogic,a1-thermal", no need for a fallback.
Thanks,
Neil
>
> reg:
> maxItems: 1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [External] Re: [PATCH bpf-next v2 1/9] bpf: tracing: add support to record and check the accessed args
From: Steven Rostedt @ 2024-03-28 15:13 UTC (permalink / raw)
To: 梦龙董
Cc: Alexei Starovoitov, Jiri Olsa, Andrii Nakryiko,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Eddy Z,
Song Liu, Yonghong Song, John Fastabend, KP Singh,
Stanislav Fomichev, Hao Luo, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, David S. Miller,
David Ahern, Dave Hansen, X86 ML, Mathieu Desnoyers,
Quentin Monnet, bpf, linux-arm-kernel, LKML, linux-riscv,
linux-s390, Network Development, linux-trace-kernel,
open list:KERNEL SELFTEST FRAMEWORK, linux-stm32
In-Reply-To: <CALz3k9jG5Jrqw=BGjt05yMkEF-1u909GbBYrV-02W0dQtm6KQQ@mail.gmail.com>
On Thu, 28 Mar 2024 22:43:46 +0800
梦龙董 <dongmenglong.8@bytedance.com> wrote:
> I have done a simple benchmark on creating 1000
> trampolines. It is slow, quite slow, which consume up to
> 60s. We can't do it this way.
>
> Now, I have a bad idea. How about we introduce
> a "dynamic trampoline"? The basic logic of it can be:
>
> """
> save regs
> bpfs = trampoline_lookup_ip(ip)
> fentry = bpfs->fentries
> while fentry:
> fentry(ctx)
> fentry = fentry->next
>
> call origin
> save return value
>
> fexit = bpfs->fexits
> while fexit:
> fexit(ctx)
> fexit = fexit->next
>
> xxxxxx
> """
>
> And we lookup the "bpfs" by the function ip in a hash map
> in trampoline_lookup_ip. The type of "bpfs" is:
>
> struct bpf_array {
> struct bpf_prog *fentries;
> struct bpf_prog *fexits;
> struct bpf_prog *modify_returns;
> }
>
> When we need to attach the bpf progA to function A/B/C,
> we only need to create the bpf_arrayA, bpf_arrayB, bpf_arrayC
> and add the progA to them, and insert them to the hash map
> "direct_call_bpfs", and attach the "dynamic trampoline" to
> A/B/C. If bpf_arrayA exist, just add progA to the tail of
> bpf_arrayA->fentries. When we need to attach progB to
> B/C, just add progB to bpf_arrayB->fentries and
> bpf_arrayB->fentries.
>
> Compared to the trampoline, extra overhead is introduced
> by the hash lookuping.
>
> I have not begun to code yet, and I am not sure the overhead is
> acceptable. Considering that we also need to do hash lookup
> by the function in kprobe_multi, maybe the overhead is
> acceptable?
Sounds like you are just recreating the function management that ftrace
has. It also can add thousands of trampolines very quickly, because it does
it in batches. It takes special synchronization steps to attach to fentry.
ftrace (and I believe multi-kprobes) updates all the attachments for each
step, so the synchronization needed is only done once.
If you really want to have thousands of functions, why not just register it
with ftrace itself. It will give you the arguments via the ftrace_regs
structure. Can't you just register a program as the callback?
It will probably make your accounting much easier, and just let ftrace
handle the fentry logic. That's what it was made to do.
-- Steve
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 1/1] dt-bindings: net: dwmac: Document STM32 property st,ext-phyclk
From: Christophe ROULLIER @ 2024-03-28 15:07 UTC (permalink / raw)
To: Marek Vasut, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maxime Coquelin, Alexandre Torgue, Richard Cochran, Jose Abreu,
Liam Girdwood, Mark Brown
Cc: netdev, devicetree, linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <480d4064-b553-4005-ad98-499a862703ff@denx.de>
On 3/28/24 15:19, Marek Vasut wrote:
> On 3/28/24 3:08 PM, Christophe Roullier wrote:
>
> [...]
>
>> | RMII | - | eth-ck | eth-ck | n/a |
>> | | | st,ext-phyclk | st,eth-ref-clk-sel
>> | |
>> | | | | or st,ext-phyclk
>> | |
>>
>> ---------------------------------------------------------------------------
>>
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
>> ---
>> Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>> b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>> index fc8c96b08d7dc..b35eae80ed6ac 100644
>> --- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>> @@ -82,6 +82,13 @@ properties:
>> Should be phandle/offset pair. The phandle to the syscon node
>> which
>> encompases the glue register, and the offset of the control
>> register
>> +st,ext-phyclk:
>
> Don't you need two spaces in front of the 'st,' here ?
Sorry, that's right.
>
>> + description:
>> + set this property in RMII mode when you have PHY without
>> crystal 50MHz and want to
>> + select RCC clock instead of ETH_REF_CLK. OR in RGMII mode when
>> you want to select
>> + RCC clock instead of ETH_CLK125.
>> + type: boolean
>> +
>
> With that fixed:
>
> Reviewed-by: Marek Vasut <marex@denx.de>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RESEND PATCH v4 3/3] perf/marvell : Odyssey LLC-TAD performance monitor
From: kernel test robot @ 2024-03-28 15:01 UTC (permalink / raw)
To: Gowthami Thiagarajan, will, mark.rutland, linux-arm-kernel,
linux-kernel
Cc: oe-kbuild-all, sgoutham, bbhushan2, gcherian,
Gowthami Thiagarajan
In-Reply-To: <20240327071832.1556576-4-gthiagarajan@marvell.com>
Hi Gowthami,
kernel test robot noticed the following build warnings:
[auto build test WARNING on soc/for-next]
[also build test WARNING on linus/master v6.9-rc1 next-20240328]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Gowthami-Thiagarajan/perf-marvell-Refactor-to-extract-platform-data/20240327-152242
base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link: https://lore.kernel.org/r/20240327071832.1556576-4-gthiagarajan%40marvell.com
patch subject: [RESEND PATCH v4 3/3] perf/marvell : Odyssey LLC-TAD performance monitor
config: alpha-randconfig-r071-20240328 (https://download.01.org/0day-ci/archive/20240328/202403282246.ybWRrXj7-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240328/202403282246.ybWRrXj7-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403282246.ybWRrXj7-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/perf/marvell_cn10k_tad_pmu.c:416:34: warning: 'tad_pmu_v2_data' defined but not used [-Wunused-const-variable=]
416 | static const struct tad_pmu_data tad_pmu_v2_data = {
| ^~~~~~~~~~~~~~~
>> drivers/perf/marvell_cn10k_tad_pmu.c:412:34: warning: 'tad_pmu_data' defined but not used [-Wunused-const-variable=]
412 | static const struct tad_pmu_data tad_pmu_data = {
| ^~~~~~~~~~~~
vim +/tad_pmu_data +412 drivers/perf/marvell_cn10k_tad_pmu.c
411
> 412 static const struct tad_pmu_data tad_pmu_data = {
413 .id = TAD_PMU_V1,
414 };
415
> 416 static const struct tad_pmu_data tad_pmu_v2_data = {
417 .id = TAD_PMU_V2,
418 };
419
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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