Re: [PATCH v5 11/18] x86/sgx: store unreclaimable pages in LRU lists
On Wed, 27 Sep 2023 06:57:18 -0500, Huang, Kai wrote: On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote: From: Sean Christopherson When an OOM event occurs, all pages associated with an enclave will need to be freed, including pages that are not currently tracked by the cgroup LRU lists. What are "cgroup LRU lists"? Will reword it. At them moment there is only one global sgx_epc_lru_lists. Add a new "unreclaimable" list to the sgx_epc_lru_lists struct and update the "sgx_record/drop_epc_pages()" functions for adding/removing VA and SECS pages to/from this "unreclaimable" list. Sorry I don't follow the logic between the two paragraphs. What is the exact problem? How does the new "unreclaimable" list solve the problem? Currently they are not tracked in a list managed by the ksgxd (future cgroup worker). So add a list to track them. Thanks Haitao
Re: [PATCH v5 11/18] x86/sgx: store unreclaimable pages in LRU lists
On Thu, 28 Sep 2023 04:41:33 -0500, Huang, Kai wrote:
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -746,6 +746,7 @@ void sgx_encl_release(struct kref *ref)
xa_destroy(>page_array);
if (!encl->secs_child_cnt && encl->secs.epc_page) {
+ sgx_drop_epc_page(encl->secs.epc_page);
sgx_encl_free_epc_page(encl->secs.epc_page);
encl->secs.epc_page = NULL;
}
The "record" of SECS/VA pages should be done together with this. I see
no
reason why the "record" and "drop" are separated into different patches.
"record" of SECS/VA pages are done in this patch. Before nothing done in
"record" for them because no tracking LRU lists for them. Now they are
tracked.
Thanks
Haitao
Re: [PATCH v5 08/18] x86/sgx: Use a list to track to-be-reclaimed pages
On Thu, 28 Sep 2023 04:28:34 -0500, Huang, Kai wrote:
On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote:
@@ -314,18 +313,22 @@ static void sgx_reclaim_pages(void)
if (kref_get_unless_zero(_page->encl->refcount) != 0) {
sgx_epc_page_set_state(epc_page,
SGX_EPC_PAGE_RECLAIM_IN_PROGRESS);
- chunk[cnt++] = epc_page;
+ list_move_tail(_page->list, );
} else {
- /* The owner is freeing the page. No need to add the
-* page back to the list of reclaimable pages.
+ /* The owner is freeing the page, remove it from the
+* LRU list
*/
Please fix comment style.
Sure
sgx_epc_page_reset_state(epc_page);
+ list_del_init(_page->list);
Is this still needed? It seems list_del_init() has been done when the
EPC page
is taken off from the global active list?
Good catch. I'll remove it.
Thanks
Haitao
Re: [PATCH v5 06/18] x86/sgx: Introduce EPC page states
On Wed, 27 Sep 2023 05:28:36 -0500, Huang, Kai wrote: On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote: Use the lower 3 bits in the flags field of sgx_epc_page struct to track EPC states in its life cycle and define an enum for possible states. More state(s) will be added later. This patch does more than what the changelog claims to do. AFAICT it does below: 1) Use the lower 3 bits to track EPC page status 2) Rename SGX_EPC_PAGE_RECLAIMER_TRACKED to SGX_EPC_PAGE_RERCLAIMABLE 3) Introduce a new state SGX_EPC_PAGE_UNRECLAIMABLE 4) Track SECS and VA pages as SGX_EPC_PAGE_UNRECLAIMABLE The changelog only says 1) IIUC. I don't quite get why you would view 3) as a separate item from 1). In my view, 4) is not done as long as there is not separate list to track it. Maybe I should make it clear the "states" vs "tracking". States are just bits in the flags, "tracking" is done using the lists by ksgxd/cgroup. And this patch is really about "states" Would that clarify the intention of the patch? If we really want to do all these in one patch, then the changelog should at least mention the justification of all of them. But I don't see why 3) and 4) need to be done here. Instead, IMHO they should be done in a separate patch, and do it after the unreclaimable list is introduced (or you need to bring that patch forward). For instance, ... [snip] + + /* Page is in use but tracked in an unreclaimable LRU list. These are + * only reclaimable when the whole enclave is OOM killed or the enclave +* is released, e.g., VA, SECS pages +* Becomes NOT_TRACKED after sgx_drop_epc() +*/ + SGX_EPC_PAGE_UNRECLAIMABLE = 3, ... We even don't have the unreclaimable LRU list yet. It's odd to have this comment here. Yeah, I should take out the mentioning of the LRUs from definitions of the states. Thanks Haitao
Re: [PATCH v4 2/2] dax/kmem: allow kmem to add memory with memmap_on_memory
On 9/29/23 2:00 AM, Vishal Verma wrote:
> Large amounts of memory managed by the kmem driver may come in via CXL,
> and it is often desirable to have the memmap for this memory on the new
> memory itself.
>
> Enroll kmem-managed memory for memmap_on_memory semantics if the dax
> region originates via CXL. For non-CXL dax regions, retain the existing
> default behavior of hot adding without memmap_on_memory semantics.
>
Are we not looking at doing altmap space for CXL DAX regions? Last discussion
around
this was suggesting we look at doing this via altmap reservation so that
we get contigous space for device memory enabling us to map them
via 1G direct mapping entries?
> Add a sysfs override under the dax device to control this behavior and
> override either default.
>
> Cc: Andrew Morton
> Cc: David Hildenbrand
> Cc: Michal Hocko
> Cc: Oscar Salvador
> Cc: Dan Williams
> Cc: Dave Jiang
> Cc: Dave Hansen
> Cc: Huang Ying
> Reviewed-by: Jonathan Cameron
> Signed-off-by: Vishal Verma
> ---
> drivers/dax/bus.h | 1 +
> drivers/dax/dax-private.h | 1 +
> drivers/dax/bus.c | 38 ++
> drivers/dax/cxl.c | 1 +
> drivers/dax/hmem/hmem.c | 1 +
> drivers/dax/kmem.c| 8 +++-
> drivers/dax/pmem.c| 1 +
> 7 files changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dax/bus.h b/drivers/dax/bus.h
> index 1ccd23360124..cbbf64443098 100644
> --- a/drivers/dax/bus.h
> +++ b/drivers/dax/bus.h
> @@ -23,6 +23,7 @@ struct dev_dax_data {
> struct dev_pagemap *pgmap;
> resource_size_t size;
> int id;
> + bool memmap_on_memory;
> };
>
> struct dev_dax *devm_create_dev_dax(struct dev_dax_data *data);
> diff --git a/drivers/dax/dax-private.h b/drivers/dax/dax-private.h
> index 27cf2d79..446617b73aea 100644
> --- a/drivers/dax/dax-private.h
> +++ b/drivers/dax/dax-private.h
> @@ -70,6 +70,7 @@ struct dev_dax {
> struct ida ida;
> struct device dev;
> struct dev_pagemap *pgmap;
> + bool memmap_on_memory;
> int nr_range;
> struct dev_dax_range {
> unsigned long pgoff;
> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> index 0ee96e6fc426..81351eb69884 100644
> --- a/drivers/dax/bus.c
> +++ b/drivers/dax/bus.c
> @@ -367,6 +367,7 @@ static ssize_t create_store(struct device *dev, struct
> device_attribute *attr,
> .dax_region = dax_region,
> .size = 0,
> .id = -1,
> + .memmap_on_memory = false,
> };
> struct dev_dax *dev_dax = devm_create_dev_dax();
>
> @@ -1269,6 +1270,40 @@ static ssize_t numa_node_show(struct device *dev,
> }
> static DEVICE_ATTR_RO(numa_node);
>
> +static ssize_t memmap_on_memory_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct dev_dax *dev_dax = to_dev_dax(dev);
> +
> + return sprintf(buf, "%d\n", dev_dax->memmap_on_memory);
> +}
> +
> +static ssize_t memmap_on_memory_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct dev_dax *dev_dax = to_dev_dax(dev);
> + struct dax_region *dax_region = dev_dax->region;
> + ssize_t rc;
> + bool val;
> +
> + rc = kstrtobool(buf, );
> + if (rc)
> + return rc;
> +
> + device_lock(dax_region->dev);
> + if (!dax_region->dev->driver) {
> + device_unlock(dax_region->dev);
> + return -ENXIO;
> + }
> +
> + dev_dax->memmap_on_memory = val;
> +
> + device_unlock(dax_region->dev);
> + return rc == 0 ? len : rc;
> +}
> +static DEVICE_ATTR_RW(memmap_on_memory);
> +
> static umode_t dev_dax_visible(struct kobject *kobj, struct attribute *a,
> int n)
> {
> struct device *dev = container_of(kobj, struct device, kobj);
> @@ -1295,6 +1330,7 @@ static struct attribute *dev_dax_attributes[] = {
> _attr_align.attr,
> _attr_resource.attr,
> _attr_numa_node.attr,
> + _attr_memmap_on_memory.attr,
> NULL,
> };
>
> @@ -1400,6 +1436,8 @@ struct dev_dax *devm_create_dev_dax(struct dev_dax_data
> *data)
> dev_dax->align = dax_region->align;
> ida_init(_dax->ida);
>
> + dev_dax->memmap_on_memory = data->memmap_on_memory;
> +
> inode = dax_inode(dax_dev);
> dev->devt = inode->i_rdev;
> dev->bus = _bus_type;
> diff --git a/drivers/dax/cxl.c b/drivers/dax/cxl.c
> index 8bc9d04034d6..c696837ab23c 100644
> --- a/drivers/dax/cxl.c
> +++ b/drivers/dax/cxl.c
> @@ -26,6 +26,7 @@ static int cxl_dax_region_probe(struct device *dev)
> .dax_region = dax_region,
> .id = -1,
> .size = range_len(_dax->hpa_range),
> + .memmap_on_memory = true,
> };
>
> return
Re: [PATCH v5 01/18] cgroup/misc: Add per resource callbacks for CSS events
On Tue Oct 3, 2023 at 1:47 AM EEST, Jarkko Sakkinen wrote:
> On Wed Sep 27, 2023 at 4:56 AM EEST, Haitao Huang wrote:
> > On Tue, 26 Sep 2023 08:13:18 -0500, Jarkko Sakkinen
> > wrote:
> >
> > ...
> > >> > >> /**
> > >> > >> @@ -410,7 +429,14 @@ misc_cg_alloc(struct cgroup_subsys_state
> > >> > >> *parent_css)
> > >> > >> */
> > >> > >> static void misc_cg_free(struct cgroup_subsys_state *css)
> > >> > >> {
> > >> > >> - kfree(css_misc(css));
> > >> > >> + struct misc_cg *cg = css_misc(css);
> > >> > >> + enum misc_res_type i;
> > >> > >> +
> > >> > >> + for (i = 0; i < MISC_CG_RES_TYPES; i++)
> > >> > >> + if (cg->res[i].free)
> > >> > >> + cg->res[i].free(cg);
> > >> > >> +
> > >> > >> + kfree(cg);
> > >> > >> }
> > >> > >>
> > >> > >> /* Cgroup controller callbacks */
> > >> > >> --
> > >> > >> 2.25.1
> > >> > >
> > >> > > Since the only existing client feature requires all callbacks,
> > >> should
> > >> > > this not have that as an invariant?
> > >> > >
> > >> > > I.e. it might be better to fail unless *all* ops are non-nil (e.g.
> > >> to
> > >> > > catch issues in the kernel code).
> > >> > >
> > >> >
> > >> > These callbacks are chained from cgroup_subsys, and they are defined
> > >> > separately so it'd be better follow the same pattern. Or change
> > >> together
> > >> > with cgroup_subsys if we want to do that. Reasonable?
> > >>
> > >> I noticed this one later:
> > >>
> > >> It would better to create a separate ops struct and declare the instance
> > >> as const at minimum.
> > >>
> > >> Then there is no need for dynamic assigment of ops and all that is in
> > >> rodata. This is improves both security and also allows static analysis
> > >> bit better.
> > >>
> > >> Now you have to dynamically trace the struct instance, e.g. in case of
> > >> a bug. If this one done, it would be already in the vmlinux.
> > >I.e. then in the driver you can have static const struct declaration
> > > with *all* pointers pre-assigned.
> > >
> > > Not sure if cgroups follows this or not but it is *objectively*
> > > better. Previous work is not always best possible work...
> > >
> >
> > IIUC, like vm_ops field in vma structs. Although function pointers in
> > vm_ops are assigned statically, but you still need dynamically assign
> > vm_ops for each instance of vma.
> >
> > So the code will look like this:
> >
> > if (parent_cg->res[i].misc_ops && parent_cg->res[i].misc_ops->alloc)
> > {
> > ...
> > }
> >
> > I don't see this is the pattern used in cgroups and no strong opinion
> > either way.
> >
> > TJ, do you have preference on this?
>
> I do have strong opinion on this. In the client side we want as much
> things declared statically as we can because it gives more tools for
> statical analysis.
>
> I don't want to see dynamic assignments in the SGX driver, when they
> are not actually needed, no matter things are done in cgroups.
I.e. I don't really even care what crazy things cgroups subsystem
might do or not do. It's not my problem.
All I care is that we *do not* have any use for assigning those
pointers at run-time. So do whatever you want with cgroups side
as long as this is not the case.
BR, Jarkko
Re: [PATCH v5 01/18] cgroup/misc: Add per resource callbacks for CSS events
On Wed Sep 27, 2023 at 4:56 AM EEST, Haitao Huang wrote:
> On Tue, 26 Sep 2023 08:13:18 -0500, Jarkko Sakkinen
> wrote:
>
> ...
> >> > >> /**
> >> > >> @@ -410,7 +429,14 @@ misc_cg_alloc(struct cgroup_subsys_state
> >> > >> *parent_css)
> >> > >> */
> >> > >> static void misc_cg_free(struct cgroup_subsys_state *css)
> >> > >> {
> >> > >> - kfree(css_misc(css));
> >> > >> + struct misc_cg *cg = css_misc(css);
> >> > >> + enum misc_res_type i;
> >> > >> +
> >> > >> + for (i = 0; i < MISC_CG_RES_TYPES; i++)
> >> > >> + if (cg->res[i].free)
> >> > >> + cg->res[i].free(cg);
> >> > >> +
> >> > >> + kfree(cg);
> >> > >> }
> >> > >>
> >> > >> /* Cgroup controller callbacks */
> >> > >> --
> >> > >> 2.25.1
> >> > >
> >> > > Since the only existing client feature requires all callbacks,
> >> should
> >> > > this not have that as an invariant?
> >> > >
> >> > > I.e. it might be better to fail unless *all* ops are non-nil (e.g.
> >> to
> >> > > catch issues in the kernel code).
> >> > >
> >> >
> >> > These callbacks are chained from cgroup_subsys, and they are defined
> >> > separately so it'd be better follow the same pattern. Or change
> >> together
> >> > with cgroup_subsys if we want to do that. Reasonable?
> >>
> >> I noticed this one later:
> >>
> >> It would better to create a separate ops struct and declare the instance
> >> as const at minimum.
> >>
> >> Then there is no need for dynamic assigment of ops and all that is in
> >> rodata. This is improves both security and also allows static analysis
> >> bit better.
> >>
> >> Now you have to dynamically trace the struct instance, e.g. in case of
> >> a bug. If this one done, it would be already in the vmlinux.
> >I.e. then in the driver you can have static const struct declaration
> > with *all* pointers pre-assigned.
> >
> > Not sure if cgroups follows this or not but it is *objectively*
> > better. Previous work is not always best possible work...
> >
>
> IIUC, like vm_ops field in vma structs. Although function pointers in
> vm_ops are assigned statically, but you still need dynamically assign
> vm_ops for each instance of vma.
>
> So the code will look like this:
>
> if (parent_cg->res[i].misc_ops && parent_cg->res[i].misc_ops->alloc)
> {
> ...
> }
>
> I don't see this is the pattern used in cgroups and no strong opinion
> either way.
>
> TJ, do you have preference on this?
I do have strong opinion on this. In the client side we want as much
things declared statically as we can because it gives more tools for
statical analysis.
I don't want to see dynamic assignments in the SGX driver, when they
are not actually needed, no matter things are done in cgroups.
> Thanks
> Haitao
BR, Jarkko
Re: [PATCH v2 1/2] dt-bindings: i2c: qcom-cci: Document SC7280 compatible
On Mon, Oct 02, 2023 at 08:55:30AM +0200, Luca Weiss wrote: > Document the compatible for the CCI block found on SC7280 SoC. > > Reviewed-by: Bryan O'Donoghue > Signed-off-by: Luca Weiss Applied to for-next, thanks! signature.asc Description: PGP signature
Re: [GIT PULL] NVDIMM Fixes for 6.6-rc5
The pull request you sent on Mon, 2 Oct 2023 10:25:42 -0700: > git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git > tags/libnvdimm-fixes-6.6-rc5 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/a9c2be4f3730961fdda03d226d783e444babe6f2 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [PATCH v1 1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed()
On 10/2/2023 3:54 PM, Andy Shevchenko wrote:
> The acpi_evaluate_dsm_typed() provides a way to check the type of the
> object evaluated by _DSM call. Use it instead of open coded variant.
>
> Signed-off-by: Andy Shevchenko
> ---
> drivers/acpi/nfit/core.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index f96bf32cd368..280da408c02c 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -1737,9 +1737,8 @@ __weak void nfit_intel_shutdown_status(struct nfit_mem
> *nfit_mem)
> if ((nfit_mem->dsm_mask & (1 << func)) == 0)
> return;
>
> - out_obj = acpi_evaluate_dsm(handle, guid, revid, func, _obj);
> - if (!out_obj || out_obj->type != ACPI_TYPE_BUFFER
> - || out_obj->buffer.length < sizeof(smart)) {
> + out_obj = acpi_evaluate_dsm_typed(handle, guid, revid, func, _obj,
> ACPI_TYPE_BUFFER);
This line is 90 characters long, wouldn't it be better to split it ?
> + if (!out_obj || out_obj->buffer.length < sizeof(smart)) {
> dev_dbg(dev->parent, "%s: failed to retrieve initial health\n",
> dev_name(dev));
While at it maybe fix alignment ? :-)
> ACPI_FREE(out_obj);
Just nitpicks, functionally code seems correct to me.
Reviewed-by: Michal Wilczynski
[GIT PULL] NVDIMM Fixes for 6.6-rc5
Hi Linus, please pull from: git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git tags/libnvdimm-fixes-6.6-rc5 ...to receive a small fix for libnvdimm correcting the calculation of idt size in the NFIT code. It has appeared in -next for a few days with no reported issues. --- The following changes since commit 6465e260f48790807eef06b583b38ca9789b6072: Linux 6.6-rc3 (2023-09-24 14:31:13 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git tags/libnvdimm-fixes-6.6-rc5 for you to fetch changes up to 33908660e814203e996f6e775d033c5c32fcf9a7: ACPI: NFIT: Fix incorrect calculation of idt size (2023-09-25 12:25:30 -0700) libnvdimm fixes for v6.6-rc5 - Fix incorrect calculation of idt size in NFIT Yu Liao (1): ACPI: NFIT: Fix incorrect calculation of idt size drivers/acpi/nfit/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Re: [PATCH 1/2] dt-bindings: mfd: qcom,spmi-pmic: Update gpio example
On Mon, Oct 02, 2023 at 08:40:10AM +0200, Luca Weiss wrote:
> On Sat Sep 30, 2023 at 5:06 PM CEST, Krzysztof Kozlowski wrote:
> > On 29/09/2023 10:17, Luca Weiss wrote:
> > > As per commit ea25d61b448a ("arm64: dts: qcom: Use plural _gpios node
> > > label for PMIC gpios") all dts files now use the plural _gpios instead
> > > of the singular _gpio as label. Update the schema example also to match.
> > >
> > > Signed-off-by: Luca Weiss
> > > ---
> > > Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> > > b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> > > index 55e931ba5b47..e4842e1fbd65 100644
> > > --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> > > +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> > > @@ -245,7 +245,7 @@ examples:
> > > #address-cells = <1>;
> > > #size-cells = <0>;
> > >
> > > -pmi8998_gpio: gpio@c000 {
> > > +pmi8998_gpios: gpio@c000
> >
> > This does no make sense... you update label only here, but not in any
> > user of it which proves that label is not used. If it is not used, it
> > should be dropped, not changed...
>
> Okay, I will drop the label instead of updating it in v2.
Or just drop the patch and skip the trivial changes. If you want to fix
unused labels, fix it for the whole subsystem (mfd) or treewide.
Rob
Re: [PATCH v2 0/5] params: harden string ops and allocatio ops
On Mon, Oct 02, 2023 at 03:48:51PM +0300, Andy Shevchenko wrote: > A couple of patches are for get the string ops, used in the module, > slightly harden. On top a few cleanups. > > Since the main part is rather hardening, I think the Kees' tree is > the best fit for the series, but I'm open for another option(s). > > Changelog v2: > - dropped the s*printf() --> sysfs_emit() conversion as it revealed > an issue, i.e. reuse getters with non-page-aligned pointer, which > would be addressed separately > - added cover letter and clarified the possible route for the series > (Luis) > > Andy Shevchenko (5): > params: Introduce the param_unknown_fn type > params: Do not go over the limit when getting the string length > params: Use size_add() for kmalloc() > params: Sort headers > params: Fix multi-line comment style Seems like a nice bit of clean-up. Reviewed-by: Kees Cook -- Kees Cook
Re: [PATCH v2 2/5] params: Do not go over the limit when getting the string length
On Mon, Oct 02, 2023 at 03:48:53PM +0300, Andy Shevchenko wrote:
> We can use strnlen() even on early stages and it prevents from
> going over the string boundaries in case it's already too long.
It makes sense to avoid calling strlen() multiple times. I don't have
much opinion one way or another about using strnlen() here, since we
know the string will be terminated.
-Kees
>
> Signed-off-by: Andy Shevchenko
> ---
> kernel/params.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/params.c b/kernel/params.c
> index 626fa8265932..f8e3c4139854 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -260,7 +260,10 @@ EXPORT_SYMBOL_GPL(param_set_uint_minmax);
>
> int param_set_charp(const char *val, const struct kernel_param *kp)
> {
> - if (strlen(val) > 1024) {
> + size_t len, maxlen = 1024;
> +
> + len = strnlen(val, maxlen + 1);
> + if (len == maxlen + 1) {
> pr_err("%s: string parameter too long\n", kp->name);
> return -ENOSPC;
> }
> @@ -270,7 +273,7 @@ int param_set_charp(const char *val, const struct
> kernel_param *kp)
> /* This is a hack. We can't kmalloc in early boot, and we
>* don't need to; this mangled commandline is preserved. */
> if (slab_is_available()) {
> - *(char **)kp->arg = kmalloc_parameter(strlen(val)+1);
> + *(char **)kp->arg = kmalloc_parameter(len + 1);
> if (!*(char **)kp->arg)
> return -ENOMEM;
> strcpy(*(char **)kp->arg, val);
> @@ -508,7 +511,7 @@ int param_set_copystring(const char *val, const struct
> kernel_param *kp)
> {
> const struct kparam_string *kps = kp->str;
>
> - if (strlen(val)+1 > kps->maxlen) {
> + if (strnlen(val, kps->maxlen) == kps->maxlen) {
> pr_err("%s: string doesn't fit in %u chars.\n",
> kp->name, kps->maxlen-1);
> return -ENOSPC;
> --
> 2.40.0.1.gaa8946217a0b
>
--
Kees Cook
[PATCH v6 2/2] leds: add ktd202x driver
This commit adds support for Kinetic KTD2026/7 RGB/White LED driver.
Signed-off-by: André Apitzsch
---
drivers/leds/rgb/Kconfig| 13 +
drivers/leds/rgb/Makefile | 1 +
drivers/leds/rgb/leds-ktd202x.c | 625
3 files changed, 639 insertions(+)
diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig
index 183bccc06cf3..a6a21f564673 100644
--- a/drivers/leds/rgb/Kconfig
+++ b/drivers/leds/rgb/Kconfig
@@ -14,6 +14,19 @@ config LEDS_GROUP_MULTICOLOR
To compile this driver as a module, choose M here: the module
will be called leds-group-multicolor.
+config LEDS_KTD202X
+ tristate "LED support for KTD202x Chips"
+ depends on I2C
+ depends on OF
+ select REGMAP_I2C
+ help
+ This option enables support for the Kinetic KTD2026/KTD2027
+ RGB/White LED driver found in different BQ mobile phones.
+ It is a 3 or 4 channel LED driver programmed via an I2C interface.
+
+ To compile this driver as a module, choose M here: the module
+ will be called leds-ktd202x.
+
config LEDS_PWM_MULTICOLOR
tristate "PWM driven multi-color LED Support"
depends on PWM
diff --git a/drivers/leds/rgb/Makefile b/drivers/leds/rgb/Makefile
index c11cc56384e7..243f31e4d70d 100644
--- a/drivers/leds/rgb/Makefile
+++ b/drivers/leds/rgb/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_LEDS_GROUP_MULTICOLOR)+= leds-group-multicolor.o
+obj-$(CONFIG_LEDS_KTD202X) += leds-ktd202x.o
obj-$(CONFIG_LEDS_PWM_MULTICOLOR) += leds-pwm-multicolor.o
obj-$(CONFIG_LEDS_QCOM_LPG)+= leds-qcom-lpg.o
obj-$(CONFIG_LEDS_MT6370_RGB) += leds-mt6370-rgb.o
diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
new file mode 100644
index ..514965795a10
--- /dev/null
+++ b/drivers/leds/rgb/leds-ktd202x.c
@@ -0,0 +1,625 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Kinetic KTD2026/7 RGB/White LED driver with I2C interface
+ *
+ * Copyright 2023 André Apitzsch
+ *
+ * Datasheet: https://www.kinet-ic.com/uploads/KTD2026-7-04h.pdf
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define KTD2026_NUM_LEDS 3
+#define KTD2027_NUM_LEDS 4
+#define KTD202X_MAX_LEDS 4
+
+/* Register bank */
+#define KTD202X_REG_RESET_CONTROL 0x00
+#define KTD202X_REG_FLASH_PERIOD 0x01
+#define KTD202X_REG_PWM1_TIMER 0x02
+#define KTD202X_REG_PWM2_TIMER 0x03
+#define KTD202X_REG_CHANNEL_CTRL 0x04
+#define KTD202X_REG_TRISE_FALL 0x05
+#define KTD202X_REG_LED_IOUT(x)(0x06 + (x))
+
+/* Register 0 */
+#define KTD202X_TIMER_SLOT_CONTROL_TSLOT1 0x00
+#define KTD202X_TIMER_SLOT_CONTROL_TSLOT2 0x01
+#define KTD202X_TIMER_SLOT_CONTROL_TSLOT3 0x02
+#define KTD202X_TIMER_SLOT_CONTROL_TSLOT4 0x03
+#define KTD202X_RSTR_RESET 0x07
+
+#define KTD202X_ENABLE_CTRL_WAKE 0x00 /* SCL High & SDA High */
+#define KTD202X_ENABLE_CTRL_SLEEP 0x08 /* SCL High & SDA Toggling */
+
+#define KTD202X_TRISE_FALL_SCALE_NORMAL0x00
+#define KTD202X_TRISE_FALL_SCALE_SLOW_X2 0x20
+#define KTD202X_TRISE_FALL_SCALE_SLOW_X4 0x40
+#define KTD202X_TRISE_FALL_SCALE_FAST_X8 0x60
+
+/* Register 1 */
+#define KTD202X_FLASH_PERIOD_256_MS_LOG_RAMP 0x00
+
+/* Register 2-3 */
+#define KTD202X_FLASH_ON_TIME_0_4_PERCENT 0x01
+
+/* Register 4 */
+#define KTD202X_CHANNEL_CTRL_MASK(x) (BIT(2 * (x)) | BIT(2 * (x) + 1))
+#define KTD202X_CHANNEL_CTRL_OFF 0x00
+#define KTD202X_CHANNEL_CTRL_ON(x) BIT(2 * (x))
+#define KTD202X_CHANNEL_CTRL_PWM1(x) BIT(2 * (x) + 1)
+#define KTD202X_CHANNEL_CTRL_PWM2(x) (BIT(2 * (x)) | BIT(2 * (x) + 1))
+
+/* Register 5 */
+#define KTD202X_RAMP_TIMES_2_MS0x00
+
+/* Register 6-9 */
+#define KTD202X_LED_CURRENT_10_mA 0x4f
+
+#define KTD202X_FLASH_PERIOD_MIN_MS 256
+#define KTD202X_FLASH_PERIOD_STEP_MS 128
+#define KTD202X_FLASH_PERIOD_MAX_STEPS 126
+#define KTD202X_FLASH_ON_MAX 256
+
+#define KTD202X_MAX_BRIGHTNESS 192
+
+static const struct reg_default ktd202x_reg_defaults[] = {
+ { KTD202X_REG_RESET_CONTROL, KTD202X_TIMER_SLOT_CONTROL_TSLOT1 |
+ KTD202X_ENABLE_CTRL_WAKE | KTD202X_TRISE_FALL_SCALE_NORMAL },
+ { KTD202X_REG_FLASH_PERIOD, KTD202X_FLASH_PERIOD_256_MS_LOG_RAMP },
+ { KTD202X_REG_PWM1_TIMER, KTD202X_FLASH_ON_TIME_0_4_PERCENT },
+ { KTD202X_REG_PWM2_TIMER, KTD202X_FLASH_ON_TIME_0_4_PERCENT },
+ { KTD202X_REG_CHANNEL_CTRL, KTD202X_CHANNEL_CTRL_OFF },
+ { KTD202X_REG_TRISE_FALL, KTD202X_RAMP_TIMES_2_MS },
+ { KTD202X_REG_LED_IOUT(0), KTD202X_LED_CURRENT_10_mA },
+ { KTD202X_REG_LED_IOUT(1), KTD202X_LED_CURRENT_10_mA },
+ { KTD202X_REG_LED_IOUT(2), KTD202X_LED_CURRENT_10_mA },
+ { KTD202X_REG_LED_IOUT(3),
[PATCH v6 0/2] leds: Add a driver for KTD202x
Add the binding description and the corresponding driver for
the Kinetic KTD2026 and KTD2027.
Signed-off-by: André Apitzsch
---
Changes in v6:
- Remove un-needed inits
- Narrow scope of variables
- Release of_node references on early exit
- Pass child node to dev_err() in ktd202x_setup_led_rgb()
- Link to v5:
https://lore.kernel.org/r/20231001-ktd202x-v5-0-f544a1d05...@apitzsch.eu
Changes in v5:
- Restructure brightness_set() + add comments to it to be easier understandable
- Add some line breaks + remove little line-wraps to improve readability
- Move parts of add_led() to setup_led_{rgb,single}()
- Move mutex_init() to the end of probe to omit gotos
- Fix grammar
- Set initial intensity to max brightness to avoid LED staying off when
brightness is changed after switching to timer trigger, because of zero
intensity
- Link to v4:
https://lore.kernel.org/r/20230923-ktd202x-v4-0-14f724f6d...@apitzsch.eu
Changes in v4:
- Annotate struct ktd202x with __counted_by
- Link to v3:
https://lore.kernel.org/r/20230906-ktd202x-v3-0-7fcb91c65...@apitzsch.eu
Changes in v3:
- Add r-b to bindings patch
- Replace .probe_new by .probe
- Link to v2:
https://lore.kernel.org/r/20230901-ktd202x-v2-0-3cb8b0ca0...@apitzsch.eu
Changes in v2:
- Make binding description filename match compatible
- Address comments by Lee Jones
- Extend driver description in Kconfig
- Add copyright + link to datasheet
- Add unit to definition/variable names, where needed
- Define magic numbers
- Remove forward declaration of 'struct ktd202x'
- Remove superfluous comments
- Get rid of struct ktd202x_info
- Join ktd202x_chip_init() with ktd202x_chip_enable()
- Return the error on ktd202x_chip_disable()
- Remove unreachable case from chip_in_use()
- Rename ktd202x_brightness_set() argument from num_colors to num_channels
- Forward errors received in ktd202x_brightness_set()
- Remove variable for 'num_channels = 1'
- Add some explanations to blink time calculation
- Remove unneeded lcdev from ktd202x_blink_*_set()
- Add define for max brightness and replace deprecated LED_FULL by it
- Move setting led_classdev.brightness to ktd202x_brightness_*_set()
- Move mutex_lock inside ktd202x_blink_set()
- Add comment that 'color' property is optional (allow EINVAL)
- Replace escaped double quotes by single quotes
- Avoid overloading variable 'color'
- Do not lock during probe
- Remove usage of 'of_match_ptr'
- Document interrupt and pull-up supply, like done for aw2013[1]
- Fix error in num_steps calculation
- Link to v1:
https://lore.kernel.org/r/20230618-ktd202x-v1-0-fc182fefa...@apitzsch.eu
[1]
https://lore.kernel.org/linux-leds/20230815-aw2013-vio-v3-0-2505296b0...@gerhold.net/
---
André Apitzsch (2):
dt-bindings: leds: Add Kinetic KTD2026/2027 LED
leds: add ktd202x driver
.../devicetree/bindings/leds/kinetic,ktd202x.yaml | 171 ++
drivers/leds/rgb/Kconfig | 13 +
drivers/leds/rgb/Makefile | 1 +
drivers/leds/rgb/leds-ktd202x.c| 625 +
4 files changed, 810 insertions(+)
---
base-commit: 165adeea3617ea22dc49f8880474ebf3a98b696d
change-id: 20230618-ktd202x-546b2a7d240b
Best regards,
--
André Apitzsch
[PATCH v6 1/2] dt-bindings: leds: Add Kinetic KTD2026/2027 LED
Document Kinetic KTD2026/2027 LED driver devicetree bindings.
Reviewed-by: Krzysztof Kozlowski
Signed-off-by: André Apitzsch
---
.../devicetree/bindings/leds/kinetic,ktd202x.yaml | 171 +
1 file changed, 171 insertions(+)
diff --git a/Documentation/devicetree/bindings/leds/kinetic,ktd202x.yaml
b/Documentation/devicetree/bindings/leds/kinetic,ktd202x.yaml
new file mode 100644
index ..832c030a5acf
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/kinetic,ktd202x.yaml
@@ -0,0 +1,171 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/kinetic,ktd202x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Kinetic KTD2026/7 RGB/White LED Driver
+
+maintainers:
+ - André Apitzsch
+
+description: |
+ The KTD2026/7 is a RGB/White LED driver with I2C interface.
+
+ The data sheet can be found at:
+https://www.kinet-ic.com/uploads/KTD2026-7-04h.pdf
+
+properties:
+ compatible:
+enum:
+ - kinetic,ktd2026
+ - kinetic,ktd2027
+
+ reg:
+maxItems: 1
+
+ vin-supply:
+description: Regulator providing power to the "VIN" pin.
+
+ vio-supply:
+description: Regulator providing power for pull-up of the I/O lines.
+ Note that this regulator does not directly connect to KTD2026, but is
+ needed for the correct operation of the status ("ST") and I2C lines.
+
+ "#address-cells":
+const: 1
+
+ "#size-cells":
+const: 0
+
+ multi-led:
+type: object
+$ref: leds-class-multicolor.yaml#
+unevaluatedProperties: false
+
+properties:
+ "#address-cells":
+const: 1
+
+ "#size-cells":
+const: 0
+
+patternProperties:
+ "^led@[0-3]$":
+type: object
+$ref: common.yaml#
+unevaluatedProperties: false
+
+properties:
+ reg:
+description: Index of the LED.
+minimum: 0
+maximum: 3
+
+required:
+ - reg
+ - color
+
+required:
+ - "#address-cells"
+ - "#size-cells"
+
+patternProperties:
+ "^led@[0-3]$":
+type: object
+$ref: common.yaml#
+unevaluatedProperties: false
+
+properties:
+ reg:
+description: Index of the LED.
+minimum: 0
+maximum: 3
+
+required:
+ - reg
+
+required:
+ - compatible
+ - reg
+ - "#address-cells"
+ - "#size-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+#include
+
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+led-controller@30 {
+compatible = "kinetic,ktd2026";
+reg = <0x30>;
+#address-cells = <1>;
+#size-cells = <0>;
+
+vin-supply = <_l17>;
+vio-supply = <_l6>;
+
+led@0 {
+reg = <0>;
+function = LED_FUNCTION_STATUS;
+color = ;
+};
+
+led@1 {
+reg = <1>;
+function = LED_FUNCTION_STATUS;
+color = ;
+};
+
+led@2 {
+reg = <2>;
+function = LED_FUNCTION_STATUS;
+color = ;
+};
+};
+};
+ - |
+#include
+
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+led-controller@30 {
+compatible = "kinetic,ktd2026";
+reg = <0x30>;
+#address-cells = <1>;
+#size-cells = <0>;
+
+vin-supply = <_l17>;
+vio-supply = <_l6>;
+
+multi-led {
+color = ;
+function = LED_FUNCTION_STATUS;
+
+#address-cells = <1>;
+#size-cells = <0>;
+
+led@0 {
+reg = <0>;
+color = ;
+};
+
+led@1 {
+reg = <1>;
+color = ;
+};
+
+led@2 {
+reg = <2>;
+color = ;
+};
+};
+};
+};
--
2.42.0
Re: [PATCH 03/13] arm64: dts: qcom: msm8916: Add common msm8916-modem-qdsp6.dtsi
On Mon, Oct 02, 2023 at 11:59:21AM +0200, Konrad Dybcio wrote:
>
>
> On 9/26/23 22:17, Stephan Gerhold wrote:
> > On Tue, Sep 26, 2023 at 10:01:21PM +0200, Konrad Dybcio wrote:
> > > On 26.09.2023 21:06, Stephan Gerhold wrote:
> > > > On Tue, Sep 26, 2023 at 08:49:24PM +0200, Konrad Dybcio wrote:
> > > > > On 26.09.2023 18:51, Stephan Gerhold wrote:
> > > > > > Most MSM8916/MSM8939 devices use very similar setups for the modem,
> > > > > > because most of the device-specific details are abstracted by the
> > > > > > modem
> > > > > > firmware. There are several definitions (status switches, DAI links
> > > > > > etc) that will be exactly the same for every board.
> > > > > >
> > > > > > Introduce a common msm8916-modem-qdsp6.dtsi include that can be
> > > > > > used to
> > > > > > simplify enabling the modem for such devices. By default the
> > > > > > digital/analog codec in the SoC/PMIC is used, but boards can define
> > > > > > additional codecs using the templates for Secondary and Quaternary
> > > > > > MI2S.
> > > > > >
> > > > > > Signed-off-by: Stephan Gerhold
> > > > > > ---
> > > > > I'd rather see at least one usage so that you aren't introducing
> > > > > effectively non-compiled code..
> > > > >
> > > >
> > > > There are 10 usages in the rest of the patch series.
> > > > Is that enough? :D
> > > >
> > > > IMHO it doesn't make sense to squash this with one of the device
> > > > patches, especially considering several of them are primarily authored
> > > > by others.
> > > I see..
> > >
> > > Well, I guess I don't have better counter-arguments, but please
> > > consider this the next time around.
> > >
> >
> > Will do!
> >
> > > [...]
> > >
> > > > > > +_codec {
> > > > > > + status = "okay";
> > > > > > +};
> > > > > Any reason for it to stay disabled?
> > > > >
> > > >
> > > > You mean in msm8916.dtsi?
> > > Yes
> > >
> > > > For the SoC dtsi we don't make assumptions
> > > > what devices use or not. There could be devices that ignore the internal
> > > > codec entirely. If there is nothing connected to the codec lpass_codec
> > > > should not be enabled (e.g. the msm8916-ufi.dtsi devices).
> > > See my reply to patch 5
> > >
> > > [...]
> > >
> >
> > Let's continue discussing that there I guess. :D
> >
> > > > > > + sound_dai_secondary: mi2s-secondary-dai-link {
> > > > > > + link-name = "Secondary MI2S";
> > > > > > + status = "disabled"; /* Needs extra codec configuration
> > > > > > */
> > > > > Hmm.. Potential good user of /omit-if-no-ref/?
> > > > >
> > > >
> > > > AFAICT /omit-if-no-ref/ is for phandle references only. Basically it
> > > > would only work if you would somewhere reference the phandle:
> > > >
> > > > list-of-sound-dais = <_dai_primary _dai_secondary>;
> > > >
> > > > But this doesn't exist so /omit-if-no-ref/ cannot be used here.
> > > Ahh right, this is the one we don't reference.. Too bad,
> > > would be a nice fit :/
> > >
> > > I only see one usage of it though (patch 7), perhaps it could
> > > be kept local to that one?
> > >
> >
> > This patch series just contains the initial set of
> > msm8916-modem-qdsp6.dtsi users (for devices which are already upstream).
> > We probably have like 20 more that still need to be upstreamed. :D
> >
> > sound_dai_secondary is fairly rare, but there is at least one more user
> > that will probably end up upstream soon.
> 2 users don't sound particularly great in a devicetree included by 20 other
> non-users
>
> > I think the overhead of these template notes is absolutely negligible
> > compared to all the (potentially) unused SoC nodes we have. :D
> Yes, however the unused SoC nodes are mostly standardized and could be used
> as-they-are on a vast majority of devices
>
To be fair we're talking about 152 bytes difference here, in a DTB that
is like 60,000 bytes total. But I can't think of enough compelling
arguments for my "template node" approach, so I will try to rework this
in v2. Let's see if I can get rid of the unused nodes without too much
mess. :)
Thanks,
Stephan
Re: [PATCH v1 1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed()
On 10/2/23 06:54, Andy Shevchenko wrote:
> The acpi_evaluate_dsm_typed() provides a way to check the type of the
> object evaluated by _DSM call. Use it instead of open coded variant.
>
> Signed-off-by: Andy Shevchenko
Reviewed-by: Dave Jiang
> ---
> drivers/acpi/nfit/core.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index f96bf32cd368..280da408c02c 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -1737,9 +1737,8 @@ __weak void nfit_intel_shutdown_status(struct nfit_mem
> *nfit_mem)
> if ((nfit_mem->dsm_mask & (1 << func)) == 0)
> return;
>
> - out_obj = acpi_evaluate_dsm(handle, guid, revid, func, _obj);
> - if (!out_obj || out_obj->type != ACPI_TYPE_BUFFER
> - || out_obj->buffer.length < sizeof(smart)) {
> + out_obj = acpi_evaluate_dsm_typed(handle, guid, revid, func, _obj,
> ACPI_TYPE_BUFFER);
> + if (!out_obj || out_obj->buffer.length < sizeof(smart)) {
> dev_dbg(dev->parent, "%s: failed to retrieve initial health\n",
> dev_name(dev));
> ACPI_FREE(out_obj);
Re: [PATCH v5 00/12] tracing: fprobe: rethook: Use ftrace_regs instead of pt_regs
On Sat, 30 Sep 2023 18:14:35 +0900 Masami Hiramatsu (Google) wrote: > On Fri, 29 Sep 2023 17:12:07 -0700 > Alexei Starovoitov wrote: > > > On Thu, Sep 28, 2023 at 6:21 PM Masami Hiramatsu > > wrote: > > > > > > > > > Thus, what I need is to make fprobe to use function-graph tracer's shadow > > > stack and trampoline instead of rethook. This may need to generalize its > > > interface so that we can share it between fprobe and function-graph > > > tracer, > > > but we don't need to involve rethook and kretprobes anymore. > > > > ... > > > > > And need to add patches > > > > > > - Introduce a generized function exit hook interface for ftrace. > > > - Replace rethook in fprobe with the function exit hook interface. > > > > you mean that rethook will be removed after that? > > No, it is too late. rethook is deeply integrated with kretprobe. > So when we remove the kretprobe, rethook will be removed too. > (fprobe and kretprobe provides similar functionality, so we can > move to fprobe) > > Even though, objpool(*) itself might be kept for some other use > cases. As far as I can see, ftrace_ret_stack can not provide a context > local storage between entry -> exit callbacks. (so this feature must > be dropped from fprobe) > > (*) > https://lore.kernel.org/all/20230905015255.81545-1-wuqiang.m...@bytedance.com/ Oops, I rechecked the performance of objpool with prctl loop by perf stat -a -I 1 --interval-count 4 -e syscalls:sys_enter_prctl And I found that with objpool, fprobe performance is the same as function-graph! noprobe kretprobe fprobe function-graph T1 107067628506402 1047588711249096 T2 28698960209725432256792322586848 T4 56634397415006754504271444932685 T8 114910972 852115229156007890068034 T16 228519966 169212249 181582171 181181211 T32 448049923 330408645 361074536 356221873 T64 623779515 450932779 499909030 495516920 objpool consumes more memory than current freelist (because it is just a list with counter) but that is limited. Usual use-case (per-probe node size is 128, # of cpu: 8) one probe will allocate 22KB. (100 probes will need 2.2MB) This is comparable to function graph ret-stack. So now I'm reconsidering the strategy. I might better to keep using rethook, but without ugly pt_regs casts. (e.g. use different trampoline if rethook user requires ftrace_regs) Sorry for confusing the direction. Thank you, -- Masami Hiramatsu (Google)
[PATCH v2] trace: tracing_event_filter: fast path when no subsystem filters
From: Nicholas Lowell
If there are no filters in the event subsystem, then there's no
reason to continue and hit the potentially time consuming
tracepoint_synchronize_unregister function. This should give
a speed up for initial disabling/configuring
Signed-off-by: Nicholas Lowell
---
Notes:
v2: switch from needless counting to boolean
WARN_ON_ONCE if no preds but filter exists
kernel/trace/trace_events_filter.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c
b/kernel/trace/trace_events_filter.c
index 33264e510d16..baa108f88032 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1317,22 +1317,29 @@ void free_event_filter(struct event_filter *filter)
__free_filter(filter);
}
-static inline void __remove_filter(struct trace_event_file *file)
+static inline bool __remove_filter(struct trace_event_file *file)
{
filter_disable(file);
+ if (!file->filter)
+ return false;
+
remove_filter_string(file->filter);
+ return true;
}
-static void filter_free_subsystem_preds(struct trace_subsystem_dir *dir,
+static bool filter_free_subsystem_preds(struct trace_subsystem_dir *dir,
struct trace_array *tr)
{
struct trace_event_file *file;
+ bool do_sync = false;
list_for_each_entry(file, >events, list) {
if (file->system != dir)
continue;
- __remove_filter(file);
+ if (__remove_filter(file))
+ do_sync = true;
}
+ return do_sync;
}
static inline void __free_subsystem_filter(struct trace_event_file *file)
@@ -2411,7 +2418,12 @@ int apply_subsystem_event_filter(struct
trace_subsystem_dir *dir,
}
if (!strcmp(strstrip(filter_string), "0")) {
- filter_free_subsystem_preds(dir, tr);
+ /* If nothing was freed, we do not need to sync */
+ if (!filter_free_subsystem_preds(dir, tr)) {
+ if(!(WARN_ON_ONCE(system->filter)))
+ goto out_unlock;
+ }
+
remove_filter_string(system->filter);
filter = system->filter;
system->filter = NULL;
--
2.25.1
Re: [PATCH v5] scripts/link-vmlinux.sh: Add alias to duplicate symbols for kallsyms
Hi. Le mercredi 27 septembre 2023, 20:35:16 EEST Alessandro Carminati (Red Hat) a écrit : > It is not uncommon for drivers or modules related to similar peripherals > to have symbols with the exact same name. > While this is not a problem for the kernel's binary itself, it becomes an > issue when attempting to trace or probe specific functions using > infrastructure like ftrace or kprobe. > > The tracing subsystem relies on the `nm -n vmlinux` output, which provides > symbol information from the kernel's ELF binary. However, when multiple > symbols share the same name, the standard nm output does not differentiate > between them. This can lead to confusion and difficulty when trying to > probe the intended symbol. > > ~ # cat /proc/kallsyms | grep " name_show" > 8c4f76d0 t name_show > 8c9cccb0 t name_show > 8cb0ac20 t name_show > 8cc728c0 t name_show > 8ce0efd0 t name_show > 8ce126c0 t name_show > 8ce1dd20 t name_show > 8ce24e70 t name_show > 8d1104c0 t name_show > 8d1fe480 t name_show > > kas_alias addresses this challenge by enhancing symbol names with > meaningful suffixes generated from the source file and line number > during the kernel build process. > These newly generated aliases provide tracers with the ability to > comprehend the symbols they are interacting with when utilizing the > ftracefs interface. > This approach may also allow for the probing by name of previously > inaccessible symbols. > > ~ # cat /proc/kallsyms | grep gic_mask_irq > d15671e505ac t gic_mask_irq > d15671e505ac t gic_mask_irq@drivers_irqchip_irq_gic_c_167 > d15671e532a4 t gic_mask_irq > d15671e532a4 t gic_mask_irq@drivers_irqchip_irq_gic_v3_c_407 > ~ # > > Changes from v1: > - Integrated changes requested by Masami to exclude symbols with prefixes > "_cfi" and "_pfx". > - Introduced a small framework to handle patterns that need to be excluded > from the alias production. > - Excluded other symbols using the framework. > - Introduced the ability to discriminate between text and data symbols. > - Added two new config symbols in this version: CONFIG_KALLSYMS_ALIAS_DATA, > which allows data for data, and CONFIG_KALLSYMS_ALIAS_DATA_ALL, which > excludes all filters and provides an alias for each duplicated symbol. > > https://lore.kernel.org/all/20230711151925.1092080-1-alessandro.carminati@gm > ail.com/ > > Changes from v2: > - Alias tags are created by querying DWARF information from the vmlinux. > - The filename + line number is normalized and appended to the original > name. > - The tag begins with '@' to indicate the symbol source. > - Not a change, but worth mentioning, since the alias is added to the > existing list, the old duplicated name is preserved, and the livepatch > way of dealing with duplicates is maintained. > - Acknowledging the existence of scenarios where inlined functions > declared in header files may result in multiple copies due to compiler > behavior, though it is not actionable as it does not pose an operational > issue. > - Highlighting a single exception where the same name refers to different > functions: the case of "compat_binfmt_elf.c," which directly includes > "binfmt_elf.c" producing identical function copies in two separate > modules. > > https://lore.kernel.org/all/20230714150326.1152359-1-alessandro.carminati@gm > ail.com/ > > Changes from v3: > - kas_alias was rewritten in Python to create a more concise and > maintainable codebase. > - The previous automation process used by kas_alias to locate the vmlinux > and the addr2line has been replaced with an explicit command-line switch > for specifying these requirements. > - addr2line has been added into the main Makefile. > - A new command-line switch has been introduced, enabling users to extend > the alias to global data names. > > https://lore.kernel.org/all/20230828080423.3539686-1-alessandro.carminati@gm > ail.com/ > > Changes from v4: > - Fixed the O= build issue > - The tool halts execution upon encountering major issues, thereby ensuring > the pipeline is interrupted. > - A cmdline option to specify the source directory added. > - Minor code adjusments. > - Tested on mips32 and i386 Thank you for sending this new version! I only found nits and I think we are near its merge! > https://lore.kernel.org/all/20230919193948.465340-1-alessandro.carminati@gma > il.com/ > > NOTE: > About the symbols name duplication that happens as consequence of the > inclusion compat_binfmt_elf.c does, it is evident that this corner is > inherently challenging the addr2line approach. > Attempting to conceal this limitation would be counterproductive. > > compat_binfmt_elf.c includes directly binfmt_elf.c, addr2line can't help > but report all functions and data declared by that file, coming from > binfmt_elf.c. > > My position is that, rather than producing a more complicated pipeline > to
Re: [PATCH] trace: tracing_event_filter: fast path when no subsystem filters
On Mon, 2 Oct 2023 09:57:34 -0400
Nick Lowell wrote:
> >
> > The above looks awkward. What about:
> >
> > if (!file->filter)
> > return 0;
> >
> > remove_filter_string(file->filter);
> > return 1;
> >
> > ?
> >
> > Or better yet:
> >
> > if (!file->filter)
> > return false;
> >
> > remove_filter_string(file->filter);
> > return true;
> >
> >
> Is it safe to assume you would like the function's return type to change
> from int to bool if I go with option 2?
Yes.
>
>
> > and ...
> >
> > > }
> > >
> > > -static void filter_free_subsystem_preds(struct trace_subsystem_dir *dir,
> > > +static int filter_free_subsystem_preds(struct trace_subsystem_dir *dir,
> > > struct trace_array *tr)
> > > {
> > > struct trace_event_file *file;
> > > + int i = 0;
> >
> > We don't really need a counter. It's either do the synchronization or
> > we don't.
> >
> > bool do_sync = false;
> >
> > >
> > > list_for_each_entry(file, >events, list) {
> > > if (file->system != dir)
> > > continue;
> > > - __remove_filter(file);
> > > + i += __remove_filter(file);
> >
> > if (remove_filter(file))
> > do_sync = true;
> >
> > > }
> >
> > return do_sync;
> >
> >
> Going to assume the same here--that return type should change from int to
> bool.
>
Correct.
>
> > > + return i;
> > > }
> > >
> > > static inline void __free_subsystem_filter(struct trace_event_file
> > *file)
> > > @@ -2411,7 +2418,9 @@ int apply_subsystem_event_filter(struct
> > trace_subsystem_dir *dir,
> > > }
> > >
> > > if (!strcmp(strstrip(filter_string), "0")) {
> > > - filter_free_subsystem_preds(dir, tr);
> > > + if (filter_free_subsystem_preds(dir, tr) == 0)
> > > + goto out_unlock;
> > > +
> >
> > /* If nothing was freed, we do not need to sync */
> > if (!filter_free_subsystem_preds(dir, tr))
> > goto out_unlock;
> >
> > And yes, add the comment.
> >
> > And actually, in that block with the goto out_unlock, we should have:
> >
> > if (!filter_free_subsystem_preds(dir, tr)) {
> > if (!(WARN_ON_ONCE(system->filter))
> > goto out_unlock;
> > }
> >
> >
> Can you explain why the WARN_ON_ONCE should be in a conditional.
>
> Don't we still want the original conditional to cause the goto regardless?
>
Because if it exists, we still want to free it and do the synchronization,
and set it to NULL. In other words, it means we missed something and need
to revert back to the original behavior.
The WARN_ON_ONCE() documents that we never expect that to happen, and if it
does, it means we have a bug.
-- Steve
>
>
> if (!filter_free_subsystem_preds(dir, tr)) {
> WARN_ON_ONCE(system->filter);
> goto out_unlock;
> }
>
[PATCH v2 2/3] arm64: dts: qcom: sc7280: Move video-firmware to chrome-common
If the video-firmware node is present, the venus driver assumes we're on
a system that doesn't use TZ for starting venus, like on ChromeOS
devices.
Move the video-firmware node to chrome-common.dtsi so we can use venus
on a non-ChromeOS devices.
At the same time also disable the venus node by default in the dtsi,
like it's done on other SoCs.
Reviewed-by: Bryan O'Donoghue
Signed-off-by: Luca Weiss
---
arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 8
arch/arm64/boot/dts/qcom/sc7280.dtsi | 6 ++
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
index 5d462ae14ba1..cd491e4d 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
@@ -104,6 +104,14 @@ {
dma-coherent;
};
+ {
+ status = "okay";
+
+ video-firmware {
+ iommus = <_smmu 0x21a2 0x0>;
+ };
+};
+
{
status = "okay";
};
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 66f1eb83cca7..fa53f54d4675 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3740,6 +3740,8 @@ venus: video-codec@aa0 {
<_smmu 0x2184 0x20>;
memory-region = <_mem>;
+ status = "disabled";
+
video-decoder {
compatible = "venus-decoder";
};
@@ -3748,10 +3750,6 @@ video-encoder {
compatible = "venus-encoder";
};
- video-firmware {
- iommus = <_smmu 0x21a2 0x0>;
- };
-
venus_opp_table: opp-table {
compatible = "operating-points-v2";
--
2.42.0
[PATCH v2 3/3] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable venus node
Enable the venus node so that the video encoder/decoder will start
working.
Reviewed-by: Konrad Dybcio
Reviewed-by: Bryan O'Donoghue
Signed-off-by: Luca Weiss
---
arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 5 +
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
index 2de0b8c26c35..d29f10f822c9 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
@@ -665,3 +665,8 @@ _1_qmpphy {
status = "okay";
};
+
+ {
+ firmware-name = "qcom/qcm6490/fairphone5/venus.mbn";
+ status = "okay";
+};
--
2.42.0
[PATCH v2 1/3] media: venus: core: Set up secure memory ranges for SC7280
Not all SC7280 devices ship with ChromeOS firmware. Other devices need
PAS for image authentication. That requires the predefined virtual
address ranges to be passed via scm calls. Define them to enable Venus
on non-CrOS SC7280 devices.
Reviewed-by: Konrad Dybcio
Reviewed-by: Bryan O'Donoghue
Signed-off-by: Luca Weiss
---
drivers/media/platform/qcom/venus/core.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/media/platform/qcom/venus/core.c
b/drivers/media/platform/qcom/venus/core.c
index 054b8e74ba4f..5c6baa0f4d45 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -881,6 +881,10 @@ static const struct venus_resources sc7280_res = {
.vmem_size = 0,
.vmem_addr = 0,
.dma_mask = 0xe000 - 1,
+ .cp_start = 0,
+ .cp_size = 0x2580,
+ .cp_nonpixel_start = 0x100,
+ .cp_nonpixel_size = 0x2480,
.fwname = "qcom/vpu-2.0/venus.mbn",
};
--
2.42.0
[PATCH v2 0/3] Enable venus on Fairphone 5 / non-ChromeOS sc7280 venus support
Devices with Qualcomm firmware (compared to ChromeOS firmware) need some changes in the venus driver and dts layout so that venus can initialize. Do these changes, similar to sc7180. Signed-off-by: Luca Weiss --- Changes in v2: - Reword commit message 2/3 to be clearer (Konrad) - Link to v1: https://lore.kernel.org/r/20230929-sc7280-venus-pas-v1-0-9c6738cf1...@fairphone.com --- Luca Weiss (3): media: venus: core: Set up secure memory ranges for SC7280 arm64: dts: qcom: sc7280: Move video-firmware to chrome-common arm64: dts: qcom: qcm6490-fairphone-fp5: Enable venus node arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 5 + arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 8 arch/arm64/boot/dts/qcom/sc7280.dtsi | 6 ++ drivers/media/platform/qcom/venus/core.c | 4 4 files changed, 19 insertions(+), 4 deletions(-) --- base-commit: df964ce9ef9fea10cf131bf6bad8658fde7956f6 change-id: 20230929-sc7280-venus-pas-ea9630525753 Best regards, -- Luca Weiss
Re: [PATCH] trace: tracing_event_filter: fast path when no subsystem filters
Sending again in plain text mode.
Thanks for the great feedback! Hopefully my inline comments/questions
aren't garbled.
On Sat, Sep 30, 2023 at 4:04 AM Steven Rostedt wrote:
>
> On Tue, 26 Sep 2023 10:20:58 -0400
> Nicholas Lowell wrote:
>
> > From: Nicholas Lowell
> >
> > If there are no filters in the event subsystem, then there's no
> > reason to continue and hit the potentially time consuming
> > tracepoint_synchronize_unregister function. This should give
> > a speed up for initial disabling/configuring
> >
> > Signed-off-by: Nicholas Lowell
> > ---
> > kernel/trace/trace_events_filter.c | 19 ++-
> > 1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/kernel/trace/trace_events_filter.c
> > b/kernel/trace/trace_events_filter.c
> > index 33264e510d16..93653d37a132 100644
> > --- a/kernel/trace/trace_events_filter.c
> > +++ b/kernel/trace/trace_events_filter.c
> > @@ -1317,22 +1317,29 @@ void free_event_filter(struct event_filter *filter)
> > __free_filter(filter);
> > }
> >
> > -static inline void __remove_filter(struct trace_event_file *file)
> > +static inline int __remove_filter(struct trace_event_file *file)
> > {
> > filter_disable(file);
> > - remove_filter_string(file->filter);
> > + if (file->filter)
> > + remove_filter_string(file->filter);
> > + else
> > + return 0;
> > +
> > + return 1;
>
> The above looks awkward. What about:
>
> if (!file->filter)
> return 0;
>
> remove_filter_string(file->filter);
> return 1;
>
> ?
>
> Or better yet:
>
> if (!file->filter)
> return false;
>
> remove_filter_string(file->filter);
> return true;
>
Is it safe to assume you would like the function's return type to
change from int to bool if I go with option 2?
> and ...
>
> > }
> >
> > -static void filter_free_subsystem_preds(struct trace_subsystem_dir *dir,
> > +static int filter_free_subsystem_preds(struct trace_subsystem_dir *dir,
> > struct trace_array *tr)
> > {
> > struct trace_event_file *file;
> > + int i = 0;
>
> We don't really need a counter. It's either do the synchronization or
> we don't.
>
> bool do_sync = false;
>
> >
> > list_for_each_entry(file, >events, list) {
> > if (file->system != dir)
> > continue;
> > - __remove_filter(file);
> > + i += __remove_filter(file);
>
> if (remove_filter(file))
> do_sync = true;
>
> > }
>
> return do_sync;
>
Going to assume the same here--that return type should change from int to bool.
> > + return i;
> > }
> >
> > static inline void __free_subsystem_filter(struct trace_event_file *file)
> > @@ -2411,7 +2418,9 @@ int apply_subsystem_event_filter(struct
> > trace_subsystem_dir *dir,
> > }
> >
> > if (!strcmp(strstrip(filter_string), "0")) {
> > - filter_free_subsystem_preds(dir, tr);
> > + if (filter_free_subsystem_preds(dir, tr) == 0)
> > + goto out_unlock;
> > +
>
> /* If nothing was freed, we do not need to sync */
> if (!filter_free_subsystem_preds(dir, tr))
> goto out_unlock;
>
> And yes, add the comment.
>
> And actually, in that block with the goto out_unlock, we should have:
>
> if (!filter_free_subsystem_preds(dir, tr)) {
> if (!(WARN_ON_ONCE(system->filter))
> goto out_unlock;
> }
>
Can you explain why the WARN_ON_ONCE should be in a conditional?
Don't we still want the original conditional to cause the goto regardless?
if (!filter_free_subsystem_preds(dir, tr)) {
WARN_ON_ONCE(system->filter);
goto out_unlock;
}
> If there were no preds, ideally there would be no subsystem filter. But
> if that's not the case, we need to warn about that and then continue.
>
> -- Steve
>
> > remove_filter_string(system->filter);
> > filter = system->filter;
> > system->filter = NULL;
>
[PATCH v2 1/2] ASoC: dt-bindings: awinic,aw88395: Remove reset-gpios from AW88261
The AW88261 chip doesn't have a reset GPIO, so disallow providing reset-gpios. At the same time also don't keep reset-gpios required for AW88395. This is both because the Linux driver has it optional, and it also simplifies the bindings by not introducing another conditional. Signed-off-by: Luca Weiss --- .../devicetree/bindings/sound/awinic,aw88395.yaml| 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml b/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml index b977d3de87cb..5d5ebc72b721 100644 --- a/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml +++ b/Documentation/devicetree/bindings/sound/awinic,aw88395.yaml @@ -14,9 +14,6 @@ description: digital Smart K audio amplifier with an integrated 10.25V smart boost convert. -allOf: - - $ref: dai-common.yaml# - properties: compatible: enum: @@ -49,9 +46,20 @@ required: - compatible - reg - '#sound-dai-cells' - - reset-gpios - awinic,audio-channel +allOf: + - $ref: dai-common.yaml# + - if: + properties: +compatible: + contains: +enum: + - awinic,aw88261 +then: + properties: +reset-gpios: false + unevaluatedProperties: false examples: -- 2.42.0
[PATCH v2 2/2] ASoC: codecs: aw88261: Remove non-existing reset gpio
According to the AW88261 datasheet (V1.1) and device schematics I have
access to, there is no reset gpio present on the AW88261. Remove it.
Signed-off-by: Luca Weiss
---
sound/soc/codecs/aw88261.c | 15 ---
1 file changed, 15 deletions(-)
diff --git a/sound/soc/codecs/aw88261.c b/sound/soc/codecs/aw88261.c
index 45eaf931a69c..e7683f70c2ef 100644
--- a/sound/soc/codecs/aw88261.c
+++ b/sound/soc/codecs/aw88261.c
@@ -10,7 +10,6 @@
#include
#include
-#include
#include
#include
#include "aw88261.h"
@@ -1175,14 +1174,6 @@ static const struct snd_soc_component_driver
soc_codec_dev_aw88261 = {
.remove = aw88261_codec_remove,
};
-static void aw88261_hw_reset(struct aw88261 *aw88261)
-{
- gpiod_set_value_cansleep(aw88261->reset_gpio, 0);
- usleep_range(AW88261_1000_US, AW88261_1000_US + 10);
- gpiod_set_value_cansleep(aw88261->reset_gpio, 1);
- usleep_range(AW88261_1000_US, AW88261_1000_US + 10);
-}
-
static void aw88261_parse_channel_dt(struct aw88261 *aw88261)
{
struct aw_device *aw_dev = aw88261->aw_pa;
@@ -1254,12 +1245,6 @@ static int aw88261_i2c_probe(struct i2c_client *i2c)
i2c_set_clientdata(i2c, aw88261);
- aw88261->reset_gpio = devm_gpiod_get_optional(>dev, "reset",
GPIOD_OUT_LOW);
- if (IS_ERR(aw88261->reset_gpio))
- dev_info(>dev, "reset gpio not defined\n");
- else
- aw88261_hw_reset(aw88261);
-
aw88261->regmap = devm_regmap_init_i2c(i2c, _remap_config);
if (IS_ERR(aw88261->regmap)) {
ret = PTR_ERR(aw88261->regmap);
--
2.42.0
[PATCH v2 0/2] Remove reset GPIO for AW88261
The AW88261 chip doesn't have a reset gpio, so remove it from the bindings and from the driver. Signed-off-by: Luca Weiss --- Changes in v2: - Include dt-bindings change - Link to v1: https://lore.kernel.org/r/20230929-aw88261-reset-v1-1-fcbce194a...@fairphone.com --- Luca Weiss (2): ASoC: dt-bindings: awinic,aw88395: Remove reset-gpios from AW88261 ASoC: codecs: aw88261: Remove non-existing reset gpio .../devicetree/bindings/sound/awinic,aw88395.yaml| 16 sound/soc/codecs/aw88261.c | 15 --- 2 files changed, 12 insertions(+), 19 deletions(-) --- base-commit: df964ce9ef9fea10cf131bf6bad8658fde7956f6 change-id: 20230929-aw88261-reset-7e00d9e25952 Best regards, -- Luca Weiss
Re: [PATCH v2 2/2] arm64: dts: qcom: pm7250b: Use correct node name for gpios
On 02/10/2023 09:00, Luca Weiss wrote: > Use gpio@ instead of pinctrl@ as that's the name expected by the > qcom,spmi-pmic.yaml schema. Update it to fix dt validation. > > Signed-off-by: Luca Weiss Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
[PATCH v1 1/1] ACPI: NFIT: Switch to use acpi_evaluate_dsm_typed()
The acpi_evaluate_dsm_typed() provides a way to check the type of the
object evaluated by _DSM call. Use it instead of open coded variant.
Signed-off-by: Andy Shevchenko
---
drivers/acpi/nfit/core.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index f96bf32cd368..280da408c02c 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -1737,9 +1737,8 @@ __weak void nfit_intel_shutdown_status(struct nfit_mem
*nfit_mem)
if ((nfit_mem->dsm_mask & (1 << func)) == 0)
return;
- out_obj = acpi_evaluate_dsm(handle, guid, revid, func, _obj);
- if (!out_obj || out_obj->type != ACPI_TYPE_BUFFER
- || out_obj->buffer.length < sizeof(smart)) {
+ out_obj = acpi_evaluate_dsm_typed(handle, guid, revid, func, _obj,
ACPI_TYPE_BUFFER);
+ if (!out_obj || out_obj->buffer.length < sizeof(smart)) {
dev_dbg(dev->parent, "%s: failed to retrieve initial health\n",
dev_name(dev));
ACPI_FREE(out_obj);
--
2.40.0.1.gaa8946217a0b
Re: [PATCH v2 1/2] dt-bindings: mfd: qcom,spmi-pmic: Drop unused labels from examples
On 02/10/2023 09:00, Luca Weiss wrote:
> There's not much point in having unused labels in the binding example,
> so drop them.
>
> This patch was originally motivated by ea25d61b448a ("arm64: dts: qcom:
> Use plural _gpios node label for PMIC gpios") updating all dts files to
> use the plural _gpios label instead of the singular _gpio as label but
> this example wasn't updated. But since we should just drop the label
> alltogether, do that.
>
> Signed-off-by: Luca Weiss
Acked-by: Krzysztof Kozlowski
Best regards,
Krzysztof
[RFC PATCH] tracing: change syscall number type in struct syscall_trace_*
linux-rt-devel tree contains a patch that adds an extra member to struct
trace_entry. This causes the offset of args field in struct
trace_event_raw_sys_enter be different from the one in struct
syscall_trace_enter:
struct trace_event_raw_sys_enter {
struct trace_entry ent; /* 012 */
/* XXX last struct has 3 bytes of padding */
/* XXX 4 bytes hole, try to pack */
long int id; /*16 8 */
long unsigned int args[6]; /*2448 */
/* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */
char __data[]; /*72 0 */
/* size: 72, cachelines: 2, members: 4 */
/* sum members: 68, holes: 1, sum holes: 4 */
/* paddings: 1, sum paddings: 3 */
/* last cacheline: 8 bytes */
};
struct syscall_trace_enter {
struct trace_entry ent; /* 012 */
/* XXX last struct has 3 bytes of padding */
intnr; /*12 4 */
long unsigned int args[]; /*16 0 */
/* size: 16, cachelines: 1, members: 3 */
/* paddings: 1, sum paddings: 3 */
/* last cacheline: 16 bytes */
};
This, in turn, causes perf_event_set_bpf_prog() fail while running bpf
test_profiler testcase because max_ctx_offset is calculated based on the
former struct, while off on the latter:
10488 if (is_tracepoint || is_syscall_tp) {
10489 int off = trace_event_get_offsets(event->tp_event);
10490
10491 if (prog->aux->max_ctx_offset > off)
10492 return -EACCES;
10493 }
This patch changes the type of nr member in syscall_trace_* structs to
be long so that "args" offset is equal to that in struct
trace_event_raw_sys_enter.
Signed-off-by: Artem Savkov
---
kernel/trace/trace.h | 4 ++--
kernel/trace/trace_syscalls.c | 7 ---
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 77debe53f07cf..cd1d24df85364 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -135,13 +135,13 @@ enum trace_type {
*/
struct syscall_trace_enter {
struct trace_entry ent;
- int nr;
+ longnr;
unsigned long args[];
};
struct syscall_trace_exit {
struct trace_entry ent;
- int nr;
+ longnr;
longret;
};
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index de753403cdafb..c26939119f2e4 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -101,7 +101,7 @@ find_syscall_meta(unsigned long syscall)
return NULL;
}
-static struct syscall_metadata *syscall_nr_to_meta(int nr)
+static struct syscall_metadata *syscall_nr_to_meta(long nr)
{
if (IS_ENABLED(CONFIG_HAVE_SPARSE_SYSCALL_NR))
return xa_load(_metadata_sparse, (unsigned long)nr);
@@ -132,7 +132,8 @@ print_syscall_enter(struct trace_iterator *iter, int flags,
struct trace_entry *ent = iter->ent;
struct syscall_trace_enter *trace;
struct syscall_metadata *entry;
- int i, syscall;
+ int i;
+ long syscall;
trace = (typeof(trace))ent;
syscall = trace->nr;
@@ -177,7 +178,7 @@ print_syscall_exit(struct trace_iterator *iter, int flags,
struct trace_seq *s = >seq;
struct trace_entry *ent = iter->ent;
struct syscall_trace_exit *trace;
- int syscall;
+ long syscall;
struct syscall_metadata *entry;
trace = (typeof(trace))ent;
--
2.41.0
Re: [PATCH v2 1/2] dt-bindings: i2c: qcom-cci: Document SC7280 compatible
On 02/10/2023 08:55, Luca Weiss wrote: > Document the compatible for the CCI block found on SC7280 SoC. > > Reviewed-by: Bryan O'Donoghue > Signed-off-by: Luca Weiss > --- Thanks, looks good now. Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable UFS
On 10/2/23 14:30, Luca Weiss wrote: Enable the UFS phy and controller so that we can access the internal storage of the phone. At the same time we need to bump the minimum voltage used for UFS VCC, otherwise it doesn't initialize properly. The 2.952V is taken from the vcc-voltage-level property downstream. See also the following link for more information about the VCCQ/VCCQ2: https://gerrit-public.fairphone.software/plugins/gitiles/kernel/msm-extra/devicetree/+/1590a3739e7dc29d2597307881553236d492f188/fp5/yupik-idp-pm7250b.dtsi#207 Signed-off-by: Luca Weiss --- Depends on: https://lore.kernel.org/linux-arm-msm/20230927081858.15961-1-quic_nitir...@quicinc.com/ --- Changes in v2: - Constrain UFS voltage to only 2.952V - Link to v1: https://lore.kernel.org/r/20230929-fp5-ufs-v1-1-122941e28...@fairphone.com --- Reviewed-by: Konrad Dybcio Konrad
[PATCH] riscv: ftrace: Fix to pass correct ftrace_regs to ftrace_func_t functions
From: Masami Hiramatsu (Google)
Since ftrace_func_t requires to pass 'struct ftrace_regs *' as the 4th
argument even if FTRACE_OPS_FL_SAVE_REGS is not set, ftrace_caller must
pass 'struct ftrace_regs *', which is a partial pt_regs, on the stack
to the ftrace_func_t functions, so that the ftrace_func_t functions can
access some partial registers.
Fix to allocate 'struct ftrace_regs' (which has the same size of 'struct
pt_regs') on the stack and save partial (argument) registers on it
instead of reduced size custom data structure.
Fixes: afc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of MCOUNT")
Signed-off-by: Masami Hiramatsu (Google)
---
arch/riscv/kernel/mcount-dyn.S | 65 +---
1 file changed, 28 insertions(+), 37 deletions(-)
diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
index 669b8697aa38..84963680eff4 100644
--- a/arch/riscv/kernel/mcount-dyn.S
+++ b/arch/riscv/kernel/mcount-dyn.S
@@ -14,46 +14,37 @@
.text
#define FENTRY_RA_OFFSET 8
-#define ABI_SIZE_ON_STACK 80
-#define ABI_A0 0
-#define ABI_A1 8
-#define ABI_A2 16
-#define ABI_A3 24
-#define ABI_A4 32
-#define ABI_A5 40
-#define ABI_A6 48
-#define ABI_A7 56
-#define ABI_T0 64
-#define ABI_RA 72
.macro SAVE_ABI
- addisp, sp, -ABI_SIZE_ON_STACK
-
- REG_S a0, ABI_A0(sp)
- REG_S a1, ABI_A1(sp)
- REG_S a2, ABI_A2(sp)
- REG_S a3, ABI_A3(sp)
- REG_S a4, ABI_A4(sp)
- REG_S a5, ABI_A5(sp)
- REG_S a6, ABI_A6(sp)
- REG_S a7, ABI_A7(sp)
- REG_S t0, ABI_T0(sp)
- REG_S ra, ABI_RA(sp)
+ addisp, sp, -PT_SIZE_ON_STACK
+
+ /* Save t0 as epc for ftrace_regs_get_instruction_pointer() */
+ REG_S t0, PT_EPC(sp)
+ REG_S a0, PT_A0(sp)
+ REG_S a1, PT_A1(sp)
+ REG_S a2, PT_A2(sp)
+ REG_S a3, PT_A3(sp)
+ REG_S a4, PT_A4(sp)
+ REG_S a5, PT_A5(sp)
+ REG_S a6, PT_A6(sp)
+ REG_S a7, PT_A7(sp)
+ REG_S t0, PT_T0(sp)
+ REG_S ra, PT_RA(sp)
.endm
.macro RESTORE_ABI
- REG_L a0, ABI_A0(sp)
- REG_L a1, ABI_A1(sp)
- REG_L a2, ABI_A2(sp)
- REG_L a3, ABI_A3(sp)
- REG_L a4, ABI_A4(sp)
- REG_L a5, ABI_A5(sp)
- REG_L a6, ABI_A6(sp)
- REG_L a7, ABI_A7(sp)
- REG_L t0, ABI_T0(sp)
- REG_L ra, ABI_RA(sp)
-
- addisp, sp, ABI_SIZE_ON_STACK
+ REG_L a0, PT_A0(sp)
+ REG_L a1, PT_A1(sp)
+ REG_L a2, PT_A2(sp)
+ REG_L a3, PT_A3(sp)
+ REG_L a4, PT_A4(sp)
+ REG_L a5, PT_A5(sp)
+ REG_L a6, PT_A6(sp)
+ REG_L a7, PT_A7(sp)
+ REG_L t0, PT_T0(sp)
+ REG_L ra, PT_RA(sp)
+
+ addisp, sp, PT_SIZE_ON_STACK
.endm
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
@@ -96,8 +87,8 @@ ftrace_call:
callftrace_stub
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
- addia0, sp, ABI_RA
- REG_L a1, ABI_T0(sp)
+ addia0, sp, PT_RA
+ REG_L a1, PT_T0(sp)
addia1, a1, -FENTRY_RA_OFFSET
#ifdef HAVE_FUNCTION_GRAPH_FP_TEST
mv a2, s0
[PATCH v2 4/5] params: Sort headers
Sort the headers in alphabetic order in order to ease the maintenance for this part. Signed-off-by: Andy Shevchenko --- kernel/params.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kernel/params.c b/kernel/params.c index c3a029fe183d..eb55b32399b4 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -3,18 +3,18 @@ Copyright (C) 2001 Rusty Russell. */ +#include +#include +#include +#include #include #include -#include -#include #include #include -#include -#include #include -#include -#include #include +#include +#include #ifdef CONFIG_SYSFS /* Protects all built-in parameters, modules use their own param_lock */ -- 2.40.0.1.gaa8946217a0b
[PATCH v2 1/5] params: Introduce the param_unknown_fn type
Introduce a new type for the callback to parse an unknown argument.
This unifies function prototypes which takes that as a parameter.
Signed-off-by: Andy Shevchenko
---
include/linux/moduleparam.h | 6 +++---
kernel/params.c | 8 ++--
2 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 4fa9726bc328..bfb85fd13e1f 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -385,6 +385,8 @@ extern bool parameq(const char *name1, const char *name2);
*/
extern bool parameqn(const char *name1, const char *name2, size_t n);
+typedef int (*parse_unknown_fn)(char *param, char *val, const char *doing,
void *arg);
+
/* Called on module insert or kernel boot */
extern char *parse_args(const char *name,
char *args,
@@ -392,9 +394,7 @@ extern char *parse_args(const char *name,
unsigned num,
s16 level_min,
s16 level_max,
- void *arg,
- int (*unknown)(char *param, char *val,
-const char *doing, void *arg));
+ void *arg, parse_unknown_fn unknown);
/* Called by module remove. */
#ifdef CONFIG_SYSFS
diff --git a/kernel/params.c b/kernel/params.c
index 2d4a0564697e..626fa8265932 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -120,9 +120,7 @@ static int parse_one(char *param,
unsigned num_params,
s16 min_level,
s16 max_level,
-void *arg,
-int (*handle_unknown)(char *param, char *val,
-const char *doing, void *arg))
+void *arg, parse_unknown_fn handle_unknown)
{
unsigned int i;
int err;
@@ -165,9 +163,7 @@ char *parse_args(const char *doing,
unsigned num,
s16 min_level,
s16 max_level,
-void *arg,
-int (*unknown)(char *param, char *val,
- const char *doing, void *arg))
+void *arg, parse_unknown_fn unknown)
{
char *param, *val, *err = NULL;
--
2.40.0.1.gaa8946217a0b
[PATCH v2 3/5] params: Use size_add() for kmalloc()
Prevent allocations from integer overflow by using size_add().
Signed-off-by: Andy Shevchenko
---
kernel/params.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/params.c b/kernel/params.c
index f8e3c4139854..c3a029fe183d 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -48,7 +49,7 @@ static void *kmalloc_parameter(unsigned int size)
{
struct kmalloced_param *p;
- p = kmalloc(sizeof(*p) + size, GFP_KERNEL);
+ p = kmalloc(size_add(sizeof(*p), size), GFP_KERNEL);
if (!p)
return NULL;
--
2.40.0.1.gaa8946217a0b
[PATCH v2 0/5] params: harden string ops and allocatio ops
A couple of patches are for get the string ops, used in the module, slightly harden. On top a few cleanups. Since the main part is rather hardening, I think the Kees' tree is the best fit for the series, but I'm open for another option(s). Changelog v2: - dropped the s*printf() --> sysfs_emit() conversion as it revealed an issue, i.e. reuse getters with non-page-aligned pointer, which would be addressed separately - added cover letter and clarified the possible route for the series (Luis) Andy Shevchenko (5): params: Introduce the param_unknown_fn type params: Do not go over the limit when getting the string length params: Use size_add() for kmalloc() params: Sort headers params: Fix multi-line comment style include/linux/moduleparam.h | 6 ++--- kernel/params.c | 52 - 2 files changed, 31 insertions(+), 27 deletions(-) -- 2.40.0.1.gaa8946217a0b
[PATCH v2 2/5] params: Do not go over the limit when getting the string length
We can use strnlen() even on early stages and it prevents from
going over the string boundaries in case it's already too long.
Signed-off-by: Andy Shevchenko
---
kernel/params.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/kernel/params.c b/kernel/params.c
index 626fa8265932..f8e3c4139854 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -260,7 +260,10 @@ EXPORT_SYMBOL_GPL(param_set_uint_minmax);
int param_set_charp(const char *val, const struct kernel_param *kp)
{
- if (strlen(val) > 1024) {
+ size_t len, maxlen = 1024;
+
+ len = strnlen(val, maxlen + 1);
+ if (len == maxlen + 1) {
pr_err("%s: string parameter too long\n", kp->name);
return -ENOSPC;
}
@@ -270,7 +273,7 @@ int param_set_charp(const char *val, const struct
kernel_param *kp)
/* This is a hack. We can't kmalloc in early boot, and we
* don't need to; this mangled commandline is preserved. */
if (slab_is_available()) {
- *(char **)kp->arg = kmalloc_parameter(strlen(val)+1);
+ *(char **)kp->arg = kmalloc_parameter(len + 1);
if (!*(char **)kp->arg)
return -ENOMEM;
strcpy(*(char **)kp->arg, val);
@@ -508,7 +511,7 @@ int param_set_copystring(const char *val, const struct
kernel_param *kp)
{
const struct kparam_string *kps = kp->str;
- if (strlen(val)+1 > kps->maxlen) {
+ if (strnlen(val, kps->maxlen) == kps->maxlen) {
pr_err("%s: string doesn't fit in %u chars.\n",
kp->name, kps->maxlen-1);
return -ENOSPC;
--
2.40.0.1.gaa8946217a0b
[PATCH v2 5/5] params: Fix multi-line comment style
The multi-line comment style in the file is rather arbitrary.
Make it follow the standard one.
Signed-off-by: Andy Shevchenko
---
kernel/params.c | 20
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/kernel/params.c b/kernel/params.c
index eb55b32399b4..2e447f8ae183 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -1,8 +1,8 @@
// SPDX-License-Identifier: GPL-2.0-or-later
-/* Helpers for initial module or kernel cmdline parsing
- Copyright (C) 2001 Rusty Russell.
-
-*/
+/*
+ * Helpers for initial module or kernel cmdline parsing
+ * Copyright (C) 2001 Rusty Russell.
+ */
#include
#include
#include
@@ -271,8 +271,10 @@ int param_set_charp(const char *val, const struct
kernel_param *kp)
maybe_kfree_parameter(*(char **)kp->arg);
- /* This is a hack. We can't kmalloc in early boot, and we
-* don't need to; this mangled commandline is preserved. */
+ /*
+* This is a hack. We can't kmalloc() in early boot, and we
+* don't need to; this mangled commandline is preserved.
+*/
if (slab_is_available()) {
*(char **)kp->arg = kmalloc_parameter(len + 1);
if (!*(char **)kp->arg)
@@ -743,8 +745,10 @@ void module_param_sysfs_remove(struct module *mod)
{
if (mod->mkobj.mp) {
sysfs_remove_group(>mkobj.kobj, >mkobj.mp->grp);
- /* We are positive that no one is using any param
-* attrs at this point. Deallocate immediately. */
+ /*
+* We are positive that no one is using any param
+* attrs at this point. Deallocate immediately.
+*/
free_module_param_attrs(>mkobj);
}
}
--
2.40.0.1.gaa8946217a0b
[PATCH v2] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable UFS
Enable the UFS phy and controller so that we can access the internal
storage of the phone.
At the same time we need to bump the minimum voltage used for UFS VCC,
otherwise it doesn't initialize properly. The 2.952V is taken from the
vcc-voltage-level property downstream.
See also the following link for more information about the VCCQ/VCCQ2:
https://gerrit-public.fairphone.software/plugins/gitiles/kernel/msm-extra/devicetree/+/1590a3739e7dc29d2597307881553236d492f188/fp5/yupik-idp-pm7250b.dtsi#207
Signed-off-by: Luca Weiss
---
Depends on:
https://lore.kernel.org/linux-arm-msm/20230927081858.15961-1-quic_nitir...@quicinc.com/
---
Changes in v2:
- Constrain UFS voltage to only 2.952V
- Link to v1:
https://lore.kernel.org/r/20230929-fp5-ufs-v1-1-122941e28...@fairphone.com
---
arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 27 --
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
index 2de0b8c26c35..762c5db29520 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
@@ -182,8 +182,9 @@ vreg_l6b: ldo6 {
};
vreg_l7b: ldo7 {
- regulator-min-microvolt = <240>;
- regulator-max-microvolt = <3544000>;
+ /* Constrained for UFS VCC, at least until UFS driver
scales voltage */
+ regulator-min-microvolt = <2952000>;
+ regulator-max-microvolt = <2952000>;
regulator-initial-mode = ;
};
@@ -632,6 +633,28 @@ bluetooth: bluetooth {
};
};
+_mem_hc {
+ reset-gpios = < 175 GPIO_ACTIVE_LOW>;
+
+ vcc-supply = <_l7b>;
+ vcc-max-microamp = <80>;
+ /*
+* Technically l9b enables an eLDO (supplied by s1b) which then powers
+* VCCQ2 of the UFS.
+*/
+ vccq-supply = <_l9b>;
+ vccq-max-microamp = <90>;
+
+ status = "okay";
+};
+
+_mem_phy {
+ vdda-phy-supply = <_l10c>;
+ vdda-pll-supply = <_l6b>;
+
+ status = "okay";
+};
+
_1 {
status = "okay";
};
---
base-commit: d85348daa4407216e47198ed35a43a66883edab6
change-id: 20230929-fp5-ufs-e2c0e21a0142
Best regards,
--
Luca Weiss
[PATCH v3 2/7] arm: Remove now superfluous sentinel elem from ctl_table arrays
From: Joel Granados
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%2f...@bombadil.infradead.org/)
Removed the sentinel as well as the explicit size from ctl_isa_vars. The
size is redundant as the initialization sets it. Changed
insn_emulation->sysctl from a 2 element array of struct ctl_table to a
simple struct. This has no consequence for the sysctl registration as it
is forwarded as a pointer. Removed sentinel from sve_defatul_vl_table,
sme_default_vl_table, tagged_addr_sysctl_table and
armv8_pmu_sysctl_table.
This removal is safe because register_sysctl_sz and register_sysctl use
the array size in addition to checking for the sentinel.
Signed-off-by: Joel Granados
---
arch/arm/kernel/isa.c| 4 ++--
arch/arm64/kernel/armv8_deprecated.c | 8 +++-
arch/arm64/kernel/fpsimd.c | 2 --
arch/arm64/kernel/process.c | 1 -
drivers/perf/arm_pmuv3.c | 1 -
5 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/arch/arm/kernel/isa.c b/arch/arm/kernel/isa.c
index 20218876bef2..905b1b191546 100644
--- a/arch/arm/kernel/isa.c
+++ b/arch/arm/kernel/isa.c
@@ -16,7 +16,7 @@
static unsigned int isa_membase, isa_portbase, isa_portshift;
-static struct ctl_table ctl_isa_vars[4] = {
+static struct ctl_table ctl_isa_vars[] = {
{
.procname = "membase",
.data = _membase,
@@ -35,7 +35,7 @@ static struct ctl_table ctl_isa_vars[4] = {
.maxlen = sizeof(isa_portshift),
.mode = 0444,
.proc_handler = proc_dointvec,
- }, {}
+ },
};
static struct ctl_table_header *isa_sysctl_header;
diff --git a/arch/arm64/kernel/armv8_deprecated.c
b/arch/arm64/kernel/armv8_deprecated.c
index e459cfd33711..dd6ce86d4332 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -52,10 +52,8 @@ struct insn_emulation {
int min;
int max;
- /*
-* sysctl for this emulation + a sentinal entry.
-*/
- struct ctl_table sysctl[2];
+ /* sysctl for this emulation */
+ struct ctl_table sysctl;
};
#define ARM_OPCODE_CONDTEST_FAIL 0
@@ -558,7 +556,7 @@ static void __init register_insn_emulation(struct
insn_emulation *insn)
update_insn_emulation_mode(insn, INSN_UNDEF);
if (insn->status != INSN_UNAVAILABLE) {
- sysctl = >sysctl[0];
+ sysctl = >sysctl;
sysctl->mode = 0644;
sysctl->maxlen = sizeof(int);
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 91e44ac7150f..9afd0eb0cf88 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -589,7 +589,6 @@ static struct ctl_table sve_default_vl_table[] = {
.proc_handler = vec_proc_do_default_vl,
.extra1 = _info[ARM64_VEC_SVE],
},
- { }
};
static int __init sve_sysctl_init(void)
@@ -613,7 +612,6 @@ static struct ctl_table sme_default_vl_table[] = {
.proc_handler = vec_proc_do_default_vl,
.extra1 = _info[ARM64_VEC_SME],
},
- { }
};
static int __init sme_sysctl_init(void)
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 0fcc4eb1a7ab..610e13c3d41b 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -724,7 +724,6 @@ static struct ctl_table tagged_addr_sysctl_table[] = {
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE,
},
- { }
};
static int __init tagged_addr_init(void)
diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
index 8fcaa26f0f8a..c0307b9181c3 100644
--- a/drivers/perf/arm_pmuv3.c
+++ b/drivers/perf/arm_pmuv3.c
@@ -1175,7 +1175,6 @@ static struct ctl_table armv8_pmu_sysctl_table[] = {
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE,
},
- { }
};
static void armv8_pmu_register_sysctl_table(void)
--
2.30.2
[PATCH v3 7/7] c-sky: Remove now superfluous sentinel element from ctl_talbe array
From: Joel Granados
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%2f...@bombadil.infradead.org/)
Remove sentinel from alignment_tbl ctl_table array. This removal is safe
because register_sysctl_init implicitly uses ARRAY_SIZE() in addition to
checking for the sentinel.
Acked-by: Guo Ren
Signed-off-by: Joel Granados
---
arch/csky/abiv1/alignment.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/csky/abiv1/alignment.c b/arch/csky/abiv1/alignment.c
index b60259daed1b..e5b8b4b2109a 100644
--- a/arch/csky/abiv1/alignment.c
+++ b/arch/csky/abiv1/alignment.c
@@ -329,7 +329,6 @@ static struct ctl_table alignment_tbl[5] = {
.mode = 0666,
.proc_handler = _dointvec
},
- {}
};
static int __init csky_alignment_init(void)
--
2.30.2
[PATCH v3 0/7] sysctl: Remove sentinel elements from arch
From: Joel Granados
What?
These commits remove the sentinel element (last empty element) from the
sysctl arrays of all the files under the "arch/" directory that use a
sysctl array for registration. The merging of the preparation patches
(in https://lore.kernel.org/all/zo5yx5jfoggi%2f...@bombadil.infradead.org/)
to mainline allows us to just remove sentinel elements without changing
behavior. This is now safe because the sysctl registration code
(register_sysctl() and friends) use the array size in addition to
checking for a sentinel ([1] for more info).
These commits are part of a bigger set (bigger patchset here
https://github.com/Joelgranados/linux/tree/tag/sysctl_remove_empty_elem_V4)
that remove the ctl_table sentinel. The idea is to make the review
process easier by chunking the 52 commits into manageable pieces. By
sending out one chunk at a time, they can be reviewed separately without
noise from parallel sets. After the "arch/" commits in this set are
reviewed, I will continue with drivers/*, fs/*, kernel/*, net/* and
miscellaneous. The final set will remove the unneeded check for
->procname == NULL.
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. I
have consolidated some links that shed light on the history of this
effort [2].
V2:
* Added clarification both in the commit messages and the coverletter as
to why this patch is safe to apply.
* Added {Acked,Reviewed,Tested}-by from list
* Link to v1:
https://lore.kernel.org/r/20230906-jag-sysctl_remove_empty_elem_arch-v1-0-3935d4854...@samsung.com
V3:
* Removed the ia64 patch to avoid conflicts with the ia64 removal
* Rebased onto v6.6-rc4
* Kept/added the trailing comma for the ctl_table arrays. This was a comment
that we received "drivers/*" patch set.
* Link to v2:
https://lore.kernel.org/r/20230913-jag-sysctl_remove_empty_elem_arch-v2-0-d1bd13a29...@samsung.com
Testing:
* Ran sysctl selftests (./tools/testing/selftests/sysctl/sysctl.sh)
* Ran this through 0-day with no errors or warnings
Size saving after removing all sentinels:
A consequence of eventually removing all the sentinels (64 bytes per
sentinel) is the bytes we save. These are *not* numbers that we will
get after this patch set; these are the numbers that we will get after
removing all the sentinels. I included them here because they are
relevant and to get an idea of just how much memory we are talking
about.
* bloat-o-meter:
- The "yesall" configuration results save 9158 bytes (bloat-o-meter
output here
https://lore.kernel.org/all/20230621091000.424843-1-j.grana...@samsung.com/)
- The "tiny" config + CONFIG_SYSCTL save 1215 bytes (bloat-o-meter
output here
https://lore.kernel.org/all/20230809105006.1198165-1-j.grana...@samsung.com/)
* memory usage:
we save some bytes in main memory as well. In my testing kernel
I measured a difference of 7296 bytes. I include the way to
measure in [3]
Size saving after this patchset:
Here I give the values that I measured for the architecture that I'm
running (x86_64). This can give an approximation of how many bytes are
saved for each arch. I won't publish for all the archs because I don't
have access to all of them.
* bloat-o-meter
- The "yesall" config saves 192 bytes (bloat-o-meter output [4])
- The "tiny" config saves 64 bytes (bloat-o-meter output [5])
* memory usage:
In this case there were no bytes saved. To measure it comment the
printk in `new_dir` and uncomment the if conditional in `new_links`
[3].
Comments/feedback greatly appreciated
Best
Joel
[1]
https://lore.kernel.org/all/20230809105006.1198165-1-j.grana...@samsung.com/
[2]
Links Related to the ctl_table sentinel removal:
* Good summary from Luis sent with the "pull request" for the
preparation patches.
https://lore.kernel.org/all/zo5yx5jfoggi%2f...@bombadil.infradead.org/
* Another very good summary from Luis.
https://lore.kernel.org/all/zmfizkfkvxuft...@bombadil.infradead.org/
* This is a patch set that replaces register_sysctl_table with register_sysctl
https://lore.kernel.org/all/20230302204612.782387-1-mcg...@kernel.org/
* Patch set to deprecate register_sysctl_paths()
https://lore.kernel.org/all/20230302202826.776286-1-mcg...@kernel.org/
* Here there is an explicit expectation for the removal of the sentinel element.
https://lore.kernel.org/all/20230321130908.6972-1-frank...@vivo.com
* The "ARRAY_SIZE" approach was mentioned (proposed?) in this thread
[PATCH v3 6/7] powerpc: Remove now superfluous sentinel element from ctl_table arrays
From: Joel Granados
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%2f...@bombadil.infradead.org/)
Remove sentinel from powersave_nap_ctl_table and nmi_wd_lpm_factor_ctl_table.
This removal is safe because register_sysctl implicitly uses ARRAY_SIZE()
in addition to checking for the sentinel.
Signed-off-by: Joel Granados
---
arch/powerpc/kernel/idle.c| 1 -
arch/powerpc/platforms/pseries/mobility.c | 1 -
2 files changed, 2 deletions(-)
diff --git a/arch/powerpc/kernel/idle.c b/arch/powerpc/kernel/idle.c
index b1c0418b25c8..30b56c67fa61 100644
--- a/arch/powerpc/kernel/idle.c
+++ b/arch/powerpc/kernel/idle.c
@@ -105,7 +105,6 @@ static struct ctl_table powersave_nap_ctl_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec,
},
- {}
};
static int __init
diff --git a/arch/powerpc/platforms/pseries/mobility.c
b/arch/powerpc/platforms/pseries/mobility.c
index 0161226d8fec..1798f0f14d58 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -61,7 +61,6 @@ static struct ctl_table nmi_wd_lpm_factor_ctl_table[] = {
.mode = 0644,
.proc_handler = proc_douintvec_minmax,
},
- {}
};
static int __init register_nmi_wd_lpm_factor_sysctl(void)
--
2.30.2
[PATCH v3 3/7] arch/x86: Remove now superfluous sentinel elem from ctl_table arrays
From: Joel Granados
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%2f...@bombadil.infradead.org/)
Remove sentinel element from sld_sysctl and itmt_kern_table. This
removal is safe because register_sysctl_init and register_sysctl
implicitly use the array size in addition to checking for the sentinel.
Reviewed-by: Ingo Molnar
Acked-by: Dave Hansen # for x86
Signed-off-by: Joel Granados
---
arch/x86/kernel/cpu/intel.c | 1 -
arch/x86/kernel/itmt.c | 1 -
2 files changed, 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index be4045628fd3..d9bb53f49a4d 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -1016,7 +1016,6 @@ static struct ctl_table sld_sysctls[] = {
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE,
},
- {}
};
static int __init sld_mitigate_sysctl_init(void)
diff --git a/arch/x86/kernel/itmt.c b/arch/x86/kernel/itmt.c
index ee4fe8cdb857..9a7c03d47861 100644
--- a/arch/x86/kernel/itmt.c
+++ b/arch/x86/kernel/itmt.c
@@ -74,7 +74,6 @@ static struct ctl_table itmt_kern_table[] = {
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE,
},
- {}
};
static struct ctl_table_header *itmt_sysctl_header;
--
2.30.2
[PATCH v3 1/7] S390: Remove now superfluous sentinel elem from ctl_table arrays
From: Joel Granados
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%2f...@bombadil.infradead.org/)
Remove the sentinel element from appldata_table, s390dbf_table,
topology_ctl_table, cmm_table and page_table_sysctl. Reduced the memory
allocation in appldata_register_ops by 1 effectively removing the
sentinel from ops->ctl_table.
This removal is safe because register_sysctl_sz and register_sysctl use
the array size in addition to checking for the sentinel.
Tested-by: Alexander Gordeev
Acked-by: Heiko Carstens
Signed-off-by: Joel Granados
---
arch/s390/appldata/appldata_base.c | 4 +---
arch/s390/kernel/debug.c | 1 -
arch/s390/kernel/topology.c| 1 -
arch/s390/mm/cmm.c | 1 -
arch/s390/mm/pgalloc.c | 1 -
5 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/s390/appldata/appldata_base.c
b/arch/s390/appldata/appldata_base.c
index 3b0994625652..c2978cb03b36 100644
--- a/arch/s390/appldata/appldata_base.c
+++ b/arch/s390/appldata/appldata_base.c
@@ -63,7 +63,6 @@ static struct ctl_table appldata_table[] = {
.mode = S_IRUGO | S_IWUSR,
.proc_handler = appldata_interval_handler,
},
- { },
};
/*
@@ -351,8 +350,7 @@ int appldata_register_ops(struct appldata_ops *ops)
if (ops->size > APPLDATA_MAX_REC_SIZE)
return -EINVAL;
- /* The last entry must be an empty one */
- ops->ctl_table = kcalloc(2, sizeof(struct ctl_table), GFP_KERNEL);
+ ops->ctl_table = kcalloc(1, sizeof(struct ctl_table), GFP_KERNEL);
if (!ops->ctl_table)
return -ENOMEM;
diff --git a/arch/s390/kernel/debug.c b/arch/s390/kernel/debug.c
index a85e0c3e7027..85328a0ef3b6 100644
--- a/arch/s390/kernel/debug.c
+++ b/arch/s390/kernel/debug.c
@@ -978,7 +978,6 @@ static struct ctl_table s390dbf_table[] = {
.mode = S_IRUGO | S_IWUSR,
.proc_handler = s390dbf_procactive,
},
- { }
};
static struct ctl_table_header *s390dbf_sysctl_header;
diff --git a/arch/s390/kernel/topology.c b/arch/s390/kernel/topology.c
index 68adf1de..be8467b25953 100644
--- a/arch/s390/kernel/topology.c
+++ b/arch/s390/kernel/topology.c
@@ -636,7 +636,6 @@ static struct ctl_table topology_ctl_table[] = {
.mode = 0644,
.proc_handler = topology_ctl_handler,
},
- { },
};
static int __init topology_init(void)
diff --git a/arch/s390/mm/cmm.c b/arch/s390/mm/cmm.c
index f47515313226..f8b13f247646 100644
--- a/arch/s390/mm/cmm.c
+++ b/arch/s390/mm/cmm.c
@@ -332,7 +332,6 @@ static struct ctl_table cmm_table[] = {
.mode = 0644,
.proc_handler = cmm_timeout_handler,
},
- { }
};
#ifdef CONFIG_CMM_IUCV
diff --git a/arch/s390/mm/pgalloc.c b/arch/s390/mm/pgalloc.c
index 07fc660a24aa..75e1039f2ec5 100644
--- a/arch/s390/mm/pgalloc.c
+++ b/arch/s390/mm/pgalloc.c
@@ -30,7 +30,6 @@ static struct ctl_table page_table_sysctl[] = {
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE,
},
- { }
};
static int __init page_table_register_sysctl(void)
--
2.30.2
[PATCH v3 4/7] x86/vdso: Remove now superfluous sentinel element from ctl_table array
From: Joel Granados
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%2f...@bombadil.infradead.org/)
Remove sentinel element from abi_table2. This removal is safe because
register_sysctl implicitly uses ARRAY_SIZE() in addition to checking for
the sentinel.
Signed-off-by: Joel Granados
---
arch/x86/entry/vdso/vdso32-setup.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/x86/entry/vdso/vdso32-setup.c
b/arch/x86/entry/vdso/vdso32-setup.c
index f3b3cacbcbb0..76e4e74f35b5 100644
--- a/arch/x86/entry/vdso/vdso32-setup.c
+++ b/arch/x86/entry/vdso/vdso32-setup.c
@@ -67,7 +67,6 @@ static struct ctl_table abi_table2[] = {
.extra1 = SYSCTL_ZERO,
.extra2 = SYSCTL_ONE,
},
- {}
};
static __init int ia32_binfmt_init(void)
--
2.30.2
[PATCH v3 5/7] riscv: Remove now superfluous sentinel element from ctl_table array
From: Joel Granados
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%2f...@bombadil.infradead.org/)
Remove sentinel element from riscv_v_default_vstate_table. This removal
is safe because register_sysctl implicitly uses ARRAY_SIZE() in addition
to checking for the sentinel.
Signed-off-by: Joel Granados
---
arch/riscv/kernel/vector.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
index 8d92fb6c522c..578b6292487e 100644
--- a/arch/riscv/kernel/vector.c
+++ b/arch/riscv/kernel/vector.c
@@ -255,7 +255,6 @@ static struct ctl_table riscv_v_default_vstate_table[] = {
.mode = 0644,
.proc_handler = proc_dobool,
},
- { }
};
static int __init riscv_v_sysctl_init(void)
--
2.30.2
Re: [PATCH v2 1/2] dt-bindings: i2c: qcom-cci: Document SC7280 compatible
On Mon, Oct 02, 2023 at 08:55:30AM +0200, Luca Weiss wrote: > Document the compatible for the CCI block found on SC7280 SoC. > > Reviewed-by: Bryan O'Donoghue > Signed-off-by: Luca Weiss Acked-by: Conor Dooley Thanks, Conor. > --- > Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml > b/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml > index 042d4dc636ee..8386cfe21532 100644 > --- a/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml > +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml > @@ -25,6 +25,7 @@ properties: > >- items: >- enum: > + - qcom,sc7280-cci >- qcom,sdm845-cci >- qcom,sm6350-cci >- qcom,sm8250-cci > @@ -159,6 +160,7 @@ allOf: > compatible: >contains: > enum: > + - qcom,sc7280-cci >- qcom,sm8250-cci >- qcom,sm8450-cci > then: > > -- > 2.42.0 > signature.asc Description: PGP signature
Re: [PATCH v2 1/2] dt-bindings: mfd: qcom,spmi-pmic: Drop unused labels from examples
On Mon, Oct 02, 2023 at 09:00:11AM +0200, Luca Weiss wrote:
> There's not much point in having unused labels in the binding example,
> so drop them.
>
> This patch was originally motivated by ea25d61b448a ("arm64: dts: qcom:
> Use plural _gpios node label for PMIC gpios") updating all dts files to
> use the plural _gpios label instead of the singular _gpio as label but
> this example wasn't updated. But since we should just drop the label
> alltogether, do that.
>
> Signed-off-by: Luca Weiss
Acked-by: Conor Dooley
Thanks,
Conor.
> ---
> Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> index 55e931ba5b47..9fa568603930 100644
> --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> @@ -239,13 +239,13 @@ examples:
> interrupt-controller;
> #interrupt-cells = <4>;
>
> -pmi8998_lsid0: pmic@2 {
> +pmic@2 {
> compatible = "qcom,pmi8998", "qcom,spmi-pmic";
> reg = <0x2 SPMI_USID>;
> #address-cells = <1>;
> #size-cells = <0>;
>
> -pmi8998_gpio: gpio@c000 {
> +gpio@c000 {
> compatible = "qcom,pmi8998-gpio", "qcom,spmi-gpio";
> reg = <0xc000>;
> gpio-controller;
> @@ -330,7 +330,7 @@ examples:
> };
> };
>
> -pm6150_gpio: gpio@c000 {
> +gpio@c000 {
> compatible = "qcom,pm6150-gpio", "qcom,spmi-gpio";
> reg = <0xc000>;
> gpio-controller;
>
> --
> 2.42.0
>
signature.asc
Description: PGP signature
Re: [PATCH v5 09/18] x86/sgx: Store struct sgx_encl when allocating new VA pages
On Fri, 2023-09-29 at 10:06 -0500, Haitao Huang wrote:
> On Wed, 27 Sep 2023 16:21:19 -0500, Huang, Kai wrote:
>
> > On Wed, 2023-09-27 at 10:35 -0500, Haitao Huang wrote:
> > > > > +
> > > > > + /* Possible owner types */
> > > > > + union {
> > > > > + struct sgx_encl_page *encl_page;
> > > > > + struct sgx_encl *encl;
> > > > > + };
> > > >
> > > > Sadly for virtual EPC page the owner is set to the 'sgx_vepc'
> > > instance it
> > > > belongs to.
> > > >
> > > > Given how sgx_{alloc|free}_epc_page() arbitrarily uses encl_page,>
> > > perhaps we
> > > > should do below?
> > > >
> > > > union {
> > > > struct sgx_encl_page *encl_page;
> > > > struct sgx_encl *encl;
> > > > struct sgx_vepc *vepc;
> > > > void *owner;
> > > > };
> > > >
> > > > And in sgx_{alloc|free}_epc_page() we can use 'owner' instead.
> > > >
> > >
> > > As I mentioned in cover letter and change log in 11/18, this series does
> > > not track virtual EPC.
> >
> > It's not about how does the cover letter says. We cannot ignore the
> > fact that
> > currently virtual EPC uses owner too.
> >
> > But given the virtual EPC code currently doesn't use the owner, I can
> > live with
> > not having the 'vepc' member in the union now.
>
> Ah, I forgot even though we don't use the owner field assigned by vepc, it
> is still passed into sgx_alloc/free_epc_page().
>
> Will add back "void* owner" and use it for assignment inside
> sgx_alloc/free_epc_page().
>
>
And also sgx_setup_epc_section().
Re: [PATCH 09/13] arm64: dts: qcom: msm8916-longcheer-l8150: Add sound and modem
On 9/30/23 18:59, Stephan Gerhold wrote: On Tue, Sep 26, 2023 at 08:59:52PM +0200, Konrad Dybcio wrote: On 26.09.2023 18:51, Stephan Gerhold wrote: From: Nikita Travkin Enable sound and modem for the Longcheer L8150 (e.g. Wileyfox Swift). e.g. -> i.e., or is that thing sold under many labels? Yes, "e.g." is indeed correct here. AFAIK the MSM8916-based Android One devices (aka "google-seed") are also based on the Longcheer L8150. They are available under names like "Cherry Mobile One G1", "i-mobile IQ II", and "General Mobile 4G". They are also covered by this device tree. Oh wow.. That's surely fun Konrad
Re: [PATCH 03/13] arm64: dts: qcom: msm8916: Add common msm8916-modem-qdsp6.dtsi
On 9/26/23 22:17, Stephan Gerhold wrote:
On Tue, Sep 26, 2023 at 10:01:21PM +0200, Konrad Dybcio wrote:
On 26.09.2023 21:06, Stephan Gerhold wrote:
On Tue, Sep 26, 2023 at 08:49:24PM +0200, Konrad Dybcio wrote:
On 26.09.2023 18:51, Stephan Gerhold wrote:
Most MSM8916/MSM8939 devices use very similar setups for the modem,
because most of the device-specific details are abstracted by the modem
firmware. There are several definitions (status switches, DAI links
etc) that will be exactly the same for every board.
Introduce a common msm8916-modem-qdsp6.dtsi include that can be used to
simplify enabling the modem for such devices. By default the
digital/analog codec in the SoC/PMIC is used, but boards can define
additional codecs using the templates for Secondary and Quaternary
MI2S.
Signed-off-by: Stephan Gerhold
---
I'd rather see at least one usage so that you aren't introducing
effectively non-compiled code..
There are 10 usages in the rest of the patch series.
Is that enough? :D
IMHO it doesn't make sense to squash this with one of the device
patches, especially considering several of them are primarily authored
by others.
I see..
Well, I guess I don't have better counter-arguments, but please
consider this the next time around.
Will do!
[...]
+_codec {
+ status = "okay";
+};
Any reason for it to stay disabled?
You mean in msm8916.dtsi?
Yes
For the SoC dtsi we don't make assumptions
what devices use or not. There could be devices that ignore the internal
codec entirely. If there is nothing connected to the codec lpass_codec
should not be enabled (e.g. the msm8916-ufi.dtsi devices).
See my reply to patch 5
[...]
Let's continue discussing that there I guess. :D
+ sound_dai_secondary: mi2s-secondary-dai-link {
+ link-name = "Secondary MI2S";
+ status = "disabled"; /* Needs extra codec configuration */
Hmm.. Potential good user of /omit-if-no-ref/?
AFAICT /omit-if-no-ref/ is for phandle references only. Basically it
would only work if you would somewhere reference the phandle:
list-of-sound-dais = <_dai_primary _dai_secondary>;
But this doesn't exist so /omit-if-no-ref/ cannot be used here.
Ahh right, this is the one we don't reference.. Too bad,
would be a nice fit :/
I only see one usage of it though (patch 7), perhaps it could
be kept local to that one?
This patch series just contains the initial set of
msm8916-modem-qdsp6.dtsi users (for devices which are already upstream).
We probably have like 20 more that still need to be upstreamed. :D
sound_dai_secondary is fairly rare, but there is at least one more user
that will probably end up upstream soon.
2 users don't sound particularly great in a devicetree included by 20
other non-users
I think the overhead of these template notes is absolutely negligible
compared to all the (potentially) unused SoC nodes we have. :D
Yes, however the unused SoC nodes are mostly standardized and could be
used as-they-are on a vast majority of devices
Konrad
Re: [PATCH v1 2/2] ACPI: NFIT: Use modern scope based rollback
On Tue, Sep 26, 2023 at 09:45:20PM +0300, Michal Wilczynski wrote: > Change rollback in acpi_nfit_init_interleave_set() to use modern scope > based attribute __free(). This is similar to C++ RAII and is a preferred > way for handling local memory allocations. LGTM, Reviewed-by: Andy Shevchenko -- With Best Regards, Andy Shevchenko
Re: [PATCH v1 1/2] ACPI: NFIT: Fix memory leak, and local use of devm_*()
On Tue, Sep 26, 2023 at 09:45:19PM +0300, Michal Wilczynski wrote: > devm_*() family of functions purpose is managing memory attached to a > device. So in general it should only be used for allocations that should > last for the whole lifecycle of the device. This is not the case for > acpi_nfit_init_interleave_set(). There are two allocations that are only > used locally in this function. What's more - if the function exits on > error path memory is never freed. It's still attached to dev and would > be freed on device detach, so this leak could be called a 'local leak'. > > Fix this by switching from devm_kcalloc() to kcalloc(), and adding > proper rollback. LGTM, Reviewed-by: Andy Shevchenko -- With Best Regards, Andy Shevchenko
Re: [PATCH v4 1/2] mm/memory_hotplug: split memmap_on_memory requests across memblocks
+
+static int __ref try_remove_memory(u64 start, u64 size)
+{
+ int rc, nid = NUMA_NO_NODE;
+
+ BUG_ON(check_hotplug_memory_range(start, size));
+
+ /*
+* All memory blocks must be offlined before removing memory. Check
+* whether all memory blocks in question are offline and return error
+* if this is not the case.
+*
+* While at it, determine the nid. Note that if we'd have mixed nodes,
+* we'd only try to offline the last determined one -- which is good
+* enough for the cases we care about.
+*/
+ rc = walk_memory_blocks(start, size, , check_memblock_offlined_cb);
+ if (rc)
+ return rc;
+
+ /*
+* For memmap_on_memory, the altmaps could have been added on
+* a per-memblock basis. Loop through the entire range if so,
+* and remove each memblock and its altmap.
+*/
+ if (mhp_memmap_on_memory()) {
+ unsigned long memblock_size = memory_block_size_bytes();
+ u64 cur_start;
+
+ for (cur_start = start; cur_start < start + size;
+cur_start += memblock_size)
+ __try_remove_memory(nid, cur_start, memblock_size);
+ } else {
+ __try_remove_memory(nid, start, size);
+ }
+
return 0;
}
Why is the firmware, memblock and nid handling not kept in this outer
function?
We really shouldn't be doing per memory block what needs to be done per
memblock: remove_memory_block_devices() and arch_remove_memory().
--
Cheers,
David / dhildenb
Re: [PATCH v4 2/2] dax/kmem: allow kmem to add memory with memmap_on_memory
On 28.09.23 22:30, Vishal Verma wrote: Large amounts of memory managed by the kmem driver may come in via CXL, and it is often desirable to have the memmap for this memory on the new memory itself. Enroll kmem-managed memory for memmap_on_memory semantics if the dax region originates via CXL. For non-CXL dax regions, retain the existing default behavior of hot adding without memmap_on_memory semantics. Add a sysfs override under the dax device to control this behavior and override either default. Cc: Andrew Morton Cc: David Hildenbrand Cc: Michal Hocko Cc: Oscar Salvador Cc: Dan Williams Cc: Dave Jiang Cc: Dave Hansen Cc: Huang Ying Reviewed-by: Jonathan Cameron Signed-off-by: Vishal Verma --- Reviewed-by: David Hildenbrand -- Cheers, David / dhildenb
Re: [PATCH] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable UFS
On 10/2/23 09:02, Luca Weiss wrote: On Fri Sep 29, 2023 at 3:12 PM CEST, Konrad Dybcio wrote: On 29.09.2023 11:52, Luca Weiss wrote: Enable the UFS phy and controller so that we can access the internal storage of the phone. At the same time we need to bump the minimum voltage used for UFS VCC, otherwise it doesn't initialize properly. The new range is taken from the vcc-voltage-level property downstream. See also the following link for more information about the VCCQ/VCCQ2: https://gerrit-public.fairphone.software/plugins/gitiles/kernel/msm-extra/devicetree/+/1590a3739e7dc29d2597307881553236d492f188/fp5/yupik-idp-pm7250b.dtsi#207 Signed-off-by: Luca Weiss --- I'm not 100% convinced about the regulator range change. For sure with the original voltage range the UFS fails to initialize, but looking at downstream kernel during runtime (debugfs) we see the VCC voltage switches between 2.4V (idle?) and 2.952V (active?). But even with this change in mainline the regulator would always stay at 2.504V which is for sure lower than the downstream operating voltage of 2.952V. Behavior wise I don't see a difference between ~2.5V and ~2.9V. Should I just constrain the regulator here to min=max=2.952V? Or just say it's okay as-is? Depends on: https://lore.kernel.org/linux-arm-msm/20230927081858.15961-1-quic_nitir...@quicinc.com/ --- There's a little funny hack inside the driver #if defined(CONFIG_SCSI_UFSHCD_QTI) if (vreg->low_voltage_sup && !vreg->low_voltage_active && on) min_uV = vreg->max_uV; #endif so, when the ufs is in use, it's pinned to vmax Hi Konrad, Are you implying I *should* or *should not* pin the voltage range to 2.952V-2.952V for mainline? Neither, voltage scaling should be implemented :P But for now, pinning it to 2.952 const is the right temporary solution, as having working UFS is generally better than one that can only idle in a stable manner :D Konrad
Re: [PATCH] arm64: dts: qcom: qcm6490-fairphone-fp5: Enable UFS
On Fri Sep 29, 2023 at 3:12 PM CEST, Konrad Dybcio wrote: > On 29.09.2023 11:52, Luca Weiss wrote: > > Enable the UFS phy and controller so that we can access the internal > > storage of the phone. > > > > At the same time we need to bump the minimum voltage used for UFS VCC, > > otherwise it doesn't initialize properly. The new range is taken from > > the vcc-voltage-level property downstream. > > > > See also the following link for more information about the VCCQ/VCCQ2: > > https://gerrit-public.fairphone.software/plugins/gitiles/kernel/msm-extra/devicetree/+/1590a3739e7dc29d2597307881553236d492f188/fp5/yupik-idp-pm7250b.dtsi#207 > > > > Signed-off-by: Luca Weiss > > --- > > I'm not 100% convinced about the regulator range change. For sure with > > the original voltage range the UFS fails to initialize, but looking at > > downstream kernel during runtime (debugfs) we see the VCC voltage > > switches between 2.4V (idle?) and 2.952V (active?). But even with this > > change in mainline the regulator would always stay at 2.504V which is > > for sure lower than the downstream operating voltage of 2.952V. Behavior > > wise I don't see a difference between ~2.5V and ~2.9V. > > > > Should I just constrain the regulator here to min=max=2.952V? Or just > > say it's okay as-is? > > > > Depends on: > > https://lore.kernel.org/linux-arm-msm/20230927081858.15961-1-quic_nitir...@quicinc.com/ > > --- > There's a little funny hack inside the driver > > #if defined(CONFIG_SCSI_UFSHCD_QTI) > if (vreg->low_voltage_sup && > !vreg->low_voltage_active && on) > min_uV = vreg->max_uV; > #endif > > so, when the ufs is in use, it's pinned to vmax Hi Konrad, Are you implying I *should* or *should not* pin the voltage range to 2.952V-2.952V for mainline? Regards Luca > > Konrad
[PATCH v2 2/2] arm64: dts: qcom: pm7250b: Use correct node name for gpios
Use gpio@ instead of pinctrl@ as that's the name expected by the
qcom,spmi-pmic.yaml schema. Update it to fix dt validation.
Signed-off-by: Luca Weiss
---
arch/arm64/boot/dts/qcom/pm7250b.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
index df0afe82f250..3bf7cf5d1700 100644
--- a/arch/arm64/boot/dts/qcom/pm7250b.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm7250b.dtsi
@@ -148,7 +148,7 @@ pm7250b_adc_tm: adc-tm@3500 {
status = "disabled";
};
- pm7250b_gpios: pinctrl@c000 {
+ pm7250b_gpios: gpio@c000 {
compatible = "qcom,pm7250b-gpio", "qcom,spmi-gpio";
reg = <0xc000>;
gpio-controller;
--
2.42.0
[PATCH v2 1/2] dt-bindings: mfd: qcom,spmi-pmic: Drop unused labels from examples
There's not much point in having unused labels in the binding example,
so drop them.
This patch was originally motivated by ea25d61b448a ("arm64: dts: qcom:
Use plural _gpios node label for PMIC gpios") updating all dts files to
use the plural _gpios label instead of the singular _gpio as label but
this example wasn't updated. But since we should just drop the label
alltogether, do that.
Signed-off-by: Luca Weiss
---
Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
index 55e931ba5b47..9fa568603930 100644
--- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
+++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
@@ -239,13 +239,13 @@ examples:
interrupt-controller;
#interrupt-cells = <4>;
-pmi8998_lsid0: pmic@2 {
+pmic@2 {
compatible = "qcom,pmi8998", "qcom,spmi-pmic";
reg = <0x2 SPMI_USID>;
#address-cells = <1>;
#size-cells = <0>;
-pmi8998_gpio: gpio@c000 {
+gpio@c000 {
compatible = "qcom,pmi8998-gpio", "qcom,spmi-gpio";
reg = <0xc000>;
gpio-controller;
@@ -330,7 +330,7 @@ examples:
};
};
-pm6150_gpio: gpio@c000 {
+gpio@c000 {
compatible = "qcom,pm6150-gpio", "qcom,spmi-gpio";
reg = <0xc000>;
gpio-controller;
--
2.42.0
[PATCH v2 0/2] Small updates / fixups for PMIC spmi-gpio
Update the schema to use plural _gpios label in the example. And fix a dtbs_check warning in pm7250b.dtsi. Signed-off-by: Luca Weiss --- Changes in v2: - Remove labels instead of updating them in documentation patch - Link to v1: https://lore.kernel.org/r/20230929-pm7250b-gpio-fixup-v1-0-ef68543c1...@fairphone.com --- Luca Weiss (2): dt-bindings: mfd: qcom,spmi-pmic: Drop unused labels from examples arm64: dts: qcom: pm7250b: Use correct node name for gpios Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 6 +++--- arch/arm64/boot/dts/qcom/pm7250b.dtsi | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) --- base-commit: df964ce9ef9fea10cf131bf6bad8658fde7956f6 change-id: 20230929-pm7250b-gpio-fixup-f407ee98425a Best regards, -- Luca Weiss
[PATCH v2 1/2] dt-bindings: i2c: qcom-cci: Document SC7280 compatible
Document the compatible for the CCI block found on SC7280 SoC. Reviewed-by: Bryan O'Donoghue Signed-off-by: Luca Weiss --- Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml b/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml index 042d4dc636ee..8386cfe21532 100644 --- a/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml @@ -25,6 +25,7 @@ properties: - items: - enum: + - qcom,sc7280-cci - qcom,sdm845-cci - qcom,sm6350-cci - qcom,sm8250-cci @@ -159,6 +160,7 @@ allOf: compatible: contains: enum: + - qcom,sc7280-cci - qcom,sm8250-cci - qcom,sm8450-cci then: -- 2.42.0
[PATCH v2 2/2] arm64: dts: qcom: sc7280: Add Camera Control Interface busses
Add the CCI busses found on sc7280 and their pinctrl states.
Reviewed-by: Bryan O'Donoghue
Signed-off-by: Luca Weiss
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 136 +++
1 file changed, 136 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 66f1eb83cca7..65550de2e4ff 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3793,6 +3793,86 @@ videocc: clock-controller@aaf {
#power-domain-cells = <1>;
};
+ cci0: cci@ac4a000 {
+ compatible = "qcom,sc7280-cci", "qcom,msm8996-cci";
+ reg = <0 0x0ac4a000 0 0x1000>;
+ interrupts = ;
+ power-domains = < CAM_CC_TITAN_TOP_GDSC>;
+
+ clocks = < CAM_CC_CAMNOC_AXI_CLK>,
+< CAM_CC_SLOW_AHB_CLK_SRC>,
+< CAM_CC_CPAS_AHB_CLK>,
+< CAM_CC_CCI_0_CLK>,
+< CAM_CC_CCI_0_CLK_SRC>;
+ clock-names = "camnoc_axi",
+ "slow_ahb_src",
+ "cpas_ahb",
+ "cci",
+ "cci_src";
+ pinctrl-0 = <_default _default>;
+ pinctrl-1 = <_sleep _sleep>;
+ pinctrl-names = "default", "sleep";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ status = "disabled";
+
+ cci0_i2c0: i2c-bus@0 {
+ reg = <0>;
+ clock-frequency = <100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ cci0_i2c1: i2c-bus@1 {
+ reg = <1>;
+ clock-frequency = <100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+
+ cci1: cci@ac4b000 {
+ compatible = "qcom,sc7280-cci", "qcom,msm8996-cci";
+ reg = <0 0x0ac4b000 0 0x1000>;
+ interrupts = ;
+ power-domains = < CAM_CC_TITAN_TOP_GDSC>;
+
+ clocks = < CAM_CC_CAMNOC_AXI_CLK>,
+< CAM_CC_SLOW_AHB_CLK_SRC>,
+< CAM_CC_CPAS_AHB_CLK>,
+< CAM_CC_CCI_1_CLK>,
+< CAM_CC_CCI_1_CLK_SRC>;
+ clock-names = "camnoc_axi",
+ "slow_ahb_src",
+ "cpas_ahb",
+ "cci",
+ "cci_src";
+ pinctrl-0 = <_default _default>;
+ pinctrl-1 = <_sleep _sleep>;
+ pinctrl-names = "default", "sleep";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ status = "disabled";
+
+ cci1_i2c0: i2c-bus@0 {
+ reg = <0>;
+ clock-frequency = <100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ cci1_i2c1: i2c-bus@1 {
+ reg = <1>;
+ clock-frequency = <100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+
camcc: clock-controller@ad0 {
compatible = "qcom,sc7280-camcc";
reg = <0 0x0ad0 0 0x1>;
@@ -4298,6 +4378,62 @@ tlmm: pinctrl@f10 {
gpio-ranges = < 0 0 175>;
wakeup-parent = <>;
+ cci0_default: cci0-default-state {
+ pins = "gpio69", "gpio70";
+ function = "cci_i2c";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ cci0_sleep: cci0-sleep-state {
+ pins = "gpio69", "gpio70";
+ function = "cci_i2c";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+
+ cci1_default: cci1-default-state {
+
[PATCH v2 0/2] Add CCI support for SC7280
Add the dts nodes for the camera control interface found on the SC7280 SoC. And then enable the CCI nodes in the Fairphone 5 dts. Signed-off-by: Luca Weiss --- Changes in v2: - Add missing clock constraints in bindings - Drop enabling cci nodes on fairphone-fp5 - Link to v1: https://lore.kernel.org/r/20230929-sc7280-cci-v1-0-16c7d386f...@fairphone.com --- Luca Weiss (2): dt-bindings: i2c: qcom-cci: Document SC7280 compatible arm64: dts: qcom: sc7280: Add Camera Control Interface busses .../devicetree/bindings/i2c/qcom,i2c-cci.yaml | 2 + arch/arm64/boot/dts/qcom/sc7280.dtsi | 136 + 2 files changed, 138 insertions(+) --- base-commit: c858197a69efe69e1607f4854af42ec338e54e96 change-id: 20230929-sc7280-cci-4690ef8da107 Best regards, -- Luca Weiss
Re: [PATCH 1/2] dt-bindings: mfd: qcom,spmi-pmic: Update gpio example
On Sat Sep 30, 2023 at 5:06 PM CEST, Krzysztof Kozlowski wrote:
> On 29/09/2023 10:17, Luca Weiss wrote:
> > As per commit ea25d61b448a ("arm64: dts: qcom: Use plural _gpios node
> > label for PMIC gpios") all dts files now use the plural _gpios instead
> > of the singular _gpio as label. Update the schema example also to match.
> >
> > Signed-off-by: Luca Weiss
> > ---
> > Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> > b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> > index 55e931ba5b47..e4842e1fbd65 100644
> > --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> > @@ -245,7 +245,7 @@ examples:
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > -pmi8998_gpio: gpio@c000 {
> > +pmi8998_gpios: gpio@c000
>
> This does no make sense... you update label only here, but not in any
> user of it which proves that label is not used. If it is not used, it
> should be dropped, not changed...
Okay, I will drop the label instead of updating it in v2.
Regards
Luca
>
> Best regards,
> Krzysztof
Re: [PATCH v5 2/2] leds: add ktd202x driver
Am Sonntag, dem 01.10.2023 um 22:46 +0200 schrieb Christophe JAILLET:
> Le 01/10/2023 à 18:56, André Apitzsch a écrit :
> > Hi Christophe,
> >
> > Am Sonntag, dem 01.10.2023 um 17:15 +0200 schrieb Christophe
> > JAILLET:
> > > Le 01/10/2023 à 15:52, André Apitzsch a écrit :
> > > > This commit adds support for Kinetic KTD2026/7 RGB/White LED
> > > > driver.
> > > >
> > > > Signed-off-by: André Apitzsch
> > > >
> > >
> > > ...
> > >
> > > > +static int ktd202x_setup_led_rgb(struct ktd202x *chip, struct
> > > > device_node *np,
> > > > + struct ktd202x_led *led,
> > > > struct
> > > > led_init_data *init_data)
> > > > +{
> > > > + struct led_classdev *cdev;
> > > > + struct device_node *child;
> > > > + struct mc_subled *info;
> > > > + int num_channels;
> > > > + int i = 0;
> > > > + u32 reg;
> > > > + int ret;
> > > > +
> > > > + num_channels = of_get_available_child_count(np);
> > > > + if (!num_channels || num_channels > chip->num_leds)
> > > > + return -EINVAL;
> > > > +
> > > > + info = devm_kcalloc(chip->dev, num_channels,
> > > > sizeof(*info),
> > > > GFP_KERNEL);
> > > > + if (!info)
> > > > + return -ENOMEM;
> > > > +
> > > > + for_each_available_child_of_node(np, child) {
> > > > + u32 mono_color = 0;
> > >
> > > Un-needed init.
> > > And, why is it defined here, while reg is defined out-side the
> > > loop?
> >
> > I'll move it out-side the loop (without initialization).
> >
> > >
> > > > +
> > > > + ret = of_property_read_u32(child, "reg", );
> > > > + if (ret != 0 || reg >= chip->num_leds) {
> > > > + dev_err(chip->dev, "invalid 'reg' of
> > > > %pOFn\n", np);
> > >
> > > Mossing of_node_put(np);?
> >
> > It shouldn't be needed here if handled in the calling function,
> > right?
>
> How can the caller do this?
>
> The goal of this of_node_put() is to release a reference taken by the
> for_each_available_child_of_node() loop, in case of early exit.
>
> The caller can't know if np needs to be released or not. An error
> code
> is returned either if an error occurs within the for_each loop, or if
> devm_led_classdev_multicolor_register_ext() fails.
>
> More over, in your case the caller is ktd202x_add_led().
> From there either ktd202x_setup_led_rgb() or
> ktd202x_setup_led_single()
> is called.
>
> ktd202x_setup_led_single() does not take any reference to np.
> But if it fails, of_node_put() would still be called.
>
>
Hello Christophe,
It seems I misunderstood when of_node_put() is used. Thanks for the
explanation.
While checking the usage of of_node_put(), I noticed that dev_err()
(and of_node_put()) should take "child" and not "np", here.
André
> > >
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + ret = of_property_read_u32(child, "color",
> > > > _color);
> > > > + if (ret < 0 && ret != -EINVAL) {
> > > > + dev_err(chip->dev, "failed to parse
> > > > 'color'
> > > > of %pOF\n", np);
> > >
> > > Mossing of_node_put(np);?
> > >
> > > > + return ret;
> > > > + }
> > > > +
> > > > + info[i].color_index = mono_color;
> > > > + info[i].channel = reg;
> > > > + info[i].intensity = KTD202X_MAX_BRIGHTNESS;
> > > > + i++;
> > > > + }
> > > > +
> > > > + led->mcdev.subled_info = info;
> > > > + led->mcdev.num_colors = num_channels;
> > > > +
> > > > + cdev = >mcdev.led_cdev;
> > > > + cdev->brightness_set_blocking =
> > > > ktd202x_brightness_mc_set;
> > > > + cdev->blink_set = ktd202x_blink_mc_set;
> > > > +
> > > > + return devm_led_classdev_multicolor_register_ext(chip-
> > > > >dev,
> > > > >mcdev, init_data);
> > > > +}
> > > > +
> > > > +static int ktd202x_setup_led_single(struct ktd202x *chip,
> > > > struct
> > > > device_node *np,
> > > > + struct ktd202x_led *led,
> > > > struct
> > > > led_init_data *init_data)
> > > > +{
> > > > + struct led_classdev *cdev;
> > > > + u32 reg;
> > > > + int ret;
> > > > +
> > > > + ret = of_property_read_u32(np, "reg", );
> > > > + if (ret != 0 || reg >= chip->num_leds) {
> > > > + dev_err(chip->dev, "invalid 'reg' of %pOFn\n",
> > > > np);
> > > > + return -EINVAL;
> > > > + }
> > > > + led->index = reg;
> > > > +
> > > > + cdev = >cdev;
> > > > + cdev->brightness_set_blocking =
> > > > ktd202x_brightness_single_set;
> > > > + cdev->blink_set = ktd202x_blink_single_set;
> > > > +
> > > > + return devm_led_classdev_register_ext(chip->dev,
> > > > > cdev, init_data);
> > > > +}
> > > > +
> > > > +static int ktd202x_add_led(struct ktd202x *chip, struct
> > > > device_node *np, unsigned
