Re: [PATCH 3/3] get_maintainer: add patch-only pattern matching type
On Wed, Sep 27, 2023 at 3:14 PM Greg KH wrote: > > On Wed, Sep 27, 2023 at 03:19:16AM +, Justin Stitt wrote: > > Note that folks really shouldn't be using get_maintainer on tree files > > anyways [1]. > > That's not true, Linus and I use it on a daily basis this way, it's part > of our normal workflow, AND the workflow of the kernel security team. > > So please don't take that valid use-case away from us. Fair. I'm on the side of keeping the "K:'' behavior the way it is and that's why I'm proposing adding "D:" to provide a more granular content matching type operating strictly on patches. It's purely opt-in. The patch I linked mentioned steering folks away from using tree files but not necessarily removing the behavior. > > thanks, > > greg k-h Thanks Justin
Re: [PATCH 3/3] get_maintainer: add patch-only pattern matching type
On Wed, Sep 27, 2023 at 03:19:16AM +, Justin Stitt wrote: > Note that folks really shouldn't be using get_maintainer on tree files > anyways [1]. That's not true, Linus and I use it on a daily basis this way, it's part of our normal workflow, AND the workflow of the kernel security team. So please don't take that valid use-case away from us. thanks, greg k-h
Re: [PATCH v3 1/1] scripts: Add add-maintainer.py
On Tue, Sep 26, 2023 at 05:32:10PM +0530, Pavan Kondeti wrote:
> On Sat, Aug 26, 2023 at 01:07:42AM -0700, Guru Das Srinagesh wrote:
> > +def gather_maintainers_of_file(patch_file):
> > +all_entities_of_patch = dict()
> > +
> > +# Run get_maintainer.pl on patch file
> > +logging.info("GET: Patch: {}".format(os.path.basename(patch_file)))
> > +cmd = ['scripts/get_maintainer.pl']
> > +cmd.extend([patch_file])
> > +
> > +try:
> > +p = subprocess.run(cmd, stdout=subprocess.PIPE, check=True)
> > +except:
> > +sys.exit(1)
> > +
> > +logging.debug("\n{}".format(p.stdout.decode()))
> > +
> > +entries = p.stdout.decode().splitlines()
> > +
> > +maintainers = []
> > +lists = []
> > +others = []
> > +
> > +for entry in entries:
> > +entity = entry.split('(')[0].strip()
> > +if any(role in entry for role in ["maintainer", "reviewer"]):
> > +maintainers.append(entity)
> > +elif "list" in entry:
> > +lists.append(entity)
> > +else:
> > +others.append(entity)
> > +
> > +all_entities_of_patch["maintainers"] = set(maintainers)
> > +all_entities_of_patch["lists"] = set(lists)
> > +all_entities_of_patch["others"] = set(others)
> > +
> > +return all_entities_of_patch
> > +
>
> FYI, there are couple of issues found while playing around.
>
> - Some entries in MAINTAINERS could be "supporter"
> - When names contain ("company"), the script fails to extract name.
>
> Thanks,
> Pavan
>
> diff --git a/scripts/add-maintainer.py b/scripts/add-maintainer.py
> index 5a5cc9482b06..6aa5e7941172 100755
> --- a/scripts/add-maintainer.py
> +++ b/scripts/add-maintainer.py
> @@ -29,8 +29,8 @@ def gather_maintainers_of_file(patch_file):
> others = []
>
> for entry in entries:
> -entity = entry.split('(')[0].strip()
> -if any(role in entry for role in ["maintainer", "reviewer"]):
> +entity = entry.rsplit('(', 1)[0].strip()
> +if any(role in entry for role in ["maintainer", "reviewer",
> "supporter"]):
> maintainers.append(entity)
> elif "list" in entry:
> lists.append(entity)
>
>
The %s/split/rsplit trades one bug for another :-( , pls ignore the
diff, when entries have multiple braces "()" , the script does not work
as epxected.
Thanks,
Pavan
Re: [PATCH v3 1/1] scripts: Add add-maintainer.py
On Mon, Aug 28, 2023 at 10:21:32AM +0200, Krzysztof Kozlowski wrote: > On 26/08/2023 10:07, Guru Das Srinagesh wrote: > > This script runs get_maintainer.py on a given patch file (or multiple > > patch files) and adds its output to the patch file in place with the > > appropriate email headers "To: " or "Cc: " as the case may be. These new > > headers are added after the "From: " line in the patch. > > > > Currently, for a single patch, maintainers and reviewers are added as > > "To: ", mailing lists and all other roles are added as "Cc: ". > > > > For a series of patches, however, a set-union scheme is employed in > > order to solve the all-too-common problem of ending up sending only > > subsets of a patch series to some lists, which results in important > > pieces of context such as the cover letter (or other patches in the > > series) being dropped from those lists. This scheme is as follows: > > > > - Create set-union of all maintainers and reviewers from all patches and > > use this to do the following per patch: > > - add only that specific patch's maintainers and reviewers as "To: " > > - add the other maintainers and reviewers from the other patches as "Cc: " > > > > - Create set-union of all mailing lists corresponding to all patches and > > add this to all patches as "Cc: " > > > > - Create set-union of all other roles corresponding to all patches and > > add this to all patches as "Cc: " > > > > Please note that patch files that don't have any "Maintainer"s or > > "Reviewers" explicitly listed in their `get_maintainer.pl` output will > > So before you will ignoring the reviewers, right? One more reason to not > get it right... > > > not have any "To: " entries added to them; developers are expected to > > manually make edits to the added entries in such cases to convert some > > "Cc: " entries to "To: " as desired. > > > > The script is quiet by default (only prints errors) and its verbosity > > can be adjusted via an optional parameter. > > > > Signed-off-by: Guru Das Srinagesh > > --- > > MAINTAINERS | 5 ++ > > scripts/add-maintainer.py | 164 ++ > > 2 files changed, 169 insertions(+) > > create mode 100755 scripts/add-maintainer.py > > > > I do not see the benefits of this script. For me - it's unnecessarily > more complicated instead of my simple bash function which makes > everything one command less than here. > One more thing to maintain. Thanks for your bash script. I slightly modified it to keep maintainers in To and rest in Cc. git send-email --dry-run --to="$(scripts/get_maintainer.pl --no-multiline --separator=, --no-r --no-l --no-git --no-roles --no-rolestats --no-git-fallback *.patch)" --cc="$(scripts/get_maintainer.pl --no-multiline --separator=, --no-m --no-git --no-roles --no-rolestats --no-git-fallback *.patch)" *.patch > > I don't see the benefits of this for newcomers, either. They should use > b4. b4 can do much, much more, so anyone creating his workflow should > start from b4, not from this script. The ROI from b4 is huge. Totally agree that one should definitely consider b4 for kernel patch submissions. I really liked b4 appending a unique "change-id" across patch-versions. This is the single most feature I miss out from gerrit where all revisions of a patch are glued with a common change-id. If everyone uses, a common change-id for all versions of series, it is super easy to dig into archives. Thanks, Pavan Thanks, Pavan
Re: [PATCH 2/3] get_maintainer: run perltidy
On Wed, 2023-09-27 at 03:19 +, Justin Stitt wrote: > I'm a first time contributor to get_maintainer.pl and the formatting is > suspicious. I am not sure if there is a particular reason it is the way > it is but I let my editor format it and submitted the diff here in this > patch. Capital NACK. Completely unnecessary and adds no value.
[PATCH 3/3] get_maintainer: add patch-only pattern matching type
Add the "D:" type which behaves the same as "K:" but will only match
content present in a patch file.
To illustrate:
Imagine this entry in MAINTAINERS:
NEW REPUBLIC
M: Han Solo
W: https://www.jointheresistance.org
D: \bstrncpy\b
Our maintainer, Han, will only be added to the recipients if a patch
file is passed to get_maintainer (like what b4 does):
$ ./scripts/get_maintainer.pl 0004-some-change.patch
If the above patch has a `strncpy` present in the subject, commit log or
diff then Han will be to/cc'd.
However, in the event of a file from the tree given like:
$ ./scripts/get_maintainer.pl ./lib/string.c
Han will not be noisily to/cc'd (like a K: type would in this
circumstance)
Note that folks really shouldn't be using get_maintainer on tree files
anyways [1].
[1]: https://lore.kernel.org/all/20230726151515.1650519-1-k...@kernel.org/
---
scripts/get_maintainer.pl | 9 +
1 file changed, 9 insertions(+)
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index e679eac96793..f290bf0948f1 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -309,6 +309,7 @@ if ( $tree && !top_of_kernel_tree($lk_path) ) {
my @typevalue = ();
my %keyword_hash;
+my %patch_keyword_hash;
my @mfiles = ();
my @self_test_info = ();
@@ -339,6 +340,9 @@ sub read_maintainer_file {
elsif ( $type eq "K" ) {
$keyword_hash{@typevalue} = $value;
}
+elsif ( $type eq "D" ) {
+$patch_keyword_hash{@typevalue} = $value;
+}
push( @typevalue, "$type:$value" );
}
elsif ( !( /^\s*$/ || /^\s*\#/ ) ) {
@@ -591,6 +595,11 @@ foreach my $file (@ARGV) {
push( @keyword_tvi, $line );
}
}
+foreach my $line ( keys %patch_keyword_hash ) {
+if ($patch_line =~
m/${patch_prefix}$patch_keyword_hash{$line}/x ) {
+push( @keyword_tvi, $line );
+}
+}
}
}
close($patch);
--
2.42.0.582.g8ccd20d70d-goog
Re: [PATCH 1/3] MAINTAINERS: add documentation for D:
On Wed, 2023-09-27 at 03:19 +, Justin Stitt wrote: > Document what "D:" does. > > This is more or less the same as what "K:" does but only works for patch > files. Nack. I'd rather just add a !$file test to K: patterns.
[PATCH 1/3] MAINTAINERS: add documentation for D:
Document what "D:" does. This is more or less the same as what "K:" does but only works for patch files. See [3/3] for more info and an illustrative example. --- MAINTAINERS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index b19995690904..de68d2c0cf29 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -59,6 +59,9 @@ Descriptions of section entries and preferred order matches patches or files that contain one or more of the words printk, pr_info or pr_err One regex pattern per line. Multiple K: lines acceptable. + D: *Content regex* (perl extended) pattern match patches only. + Usage same as K:. + Maintainers List -- 2.42.0.582.g8ccd20d70d-goog
Re: [PATCH 1/3] MAINTAINERS: add documentation for D:
On Wed, Sep 27, 2023 at 12:27 PM Joe Perches wrote: > > On Wed, 2023-09-27 at 03:19 +, Justin Stitt wrote: > > Document what "D:" does. > > > > This is more or less the same as what "K:" does but only works for patch > > files. > > Nack. I'd rather just add a !$file test to K: patterns. Are there no legitimate use cases for K:'s current behavior to warrant keeping it around? >
[PATCH 0/3] get_maintainer: add patch-only keyword matching
This series aims to add "D:" which behaves exactly the same as "K:" but works only on patch files. The goal of this is to reduce noise when folks use get_maintainer on tree files as opposed to patches. This use case should be steered away from [1] but "D:" should help maintainers reduce noise in their inboxes regardless, especially when matching omnipresent keywords like [2]. In the event of [2] Kees would be to/cc'd from folks running get_maintainer on _any_ file containing "__counted_by". The number of these files is rising and I fear for his inbox as his goal, as I understand it, is to simply monitor the introduction of new __counted_by annotations to ensure accurate semantics. See [3/3] for an illustrative example. This series also includes a formatting pass over get_maintainer because I personally found it difficult to parse with the human eye. [1]: https://lore.kernel.org/all/20230726151515.1650519-1-k...@kernel.org/ [2]: https://lore.kernel.org/all/20230925172037.work.853-k...@kernel.org/ Signed-off-by: Justin Stitt --- Justin Stitt (3): MAINTAINERS: add documentation for D: get_maintainer: run perltidy get_maintainer: add patch-only pattern matching type MAINTAINERS |3 + scripts/get_maintainer.pl | 3334 +++-- 2 files changed, 1718 insertions(+), 1619 deletions(-) --- base-commit: 6465e260f48790807eef06b583b38ca9789b6072 change-id: 20230926-get_maintainer_add_d-07424a814e72 Best regards, -- Justin Stitt
Re: [PATCH v5 01/18] cgroup/misc: Add per resource callbacks for CSS events
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?
Thanks
Haitao
Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem
On Tue, Sep 26, 2023 at 10:29:29PM +0200, Konrad Dybcio wrote:
> On 26.09.2023 22:27, Stephan Gerhold wrote:
> > On Tue, Sep 26, 2023 at 10:18:22PM +0200, Konrad Dybcio wrote:
> >> On 26.09.2023 22:09, Stephan Gerhold wrote:
> >>> On Tue, Sep 26, 2023 at 09:56:12PM +0200, Konrad Dybcio wrote:
> [...]
>
> >>> +
> >>> &blsp_i2c2 {
> >>> status = "okay";
> >>>
> >>> @@ -243,6 +258,13 @@ &gpu {
> >>> status = "okay";
> >>> };
> >>>
> >>> +&lpass {
> >>> + dai-link@3 {
> >>> + reg = ;
> >>> + qcom,playback-sd-lines = <1>;
> >>> + };
> >>> +};
> >> Is that not status = reserved?
> >>
> >
> > Correct. This is here to simplify switching to the modem-bypass audio
> > routing if someone does not need (or want) the modem. The direct audio
> > path with the LPASS drivers tends to be more reliable and configurable
> > (especially wrt bit formats, sampling rates, latency, channels etc).
> > I know that at some point this helped someone who tried to use an old
> > phone as some kind of portable musical instrument / synthesizer.
> >
> > It's not too obvious that these definitions would be needed when making
> > those changes (because devices using the standard SD lines (i.e <0>) do
> > not need it). If you forget about this you get non-functional audio with
> > no error or any hint what could be wrong.
> >
> > To simplify switching between the different audio routing options, the
> > lk2nd bootloader actually has an option to do this transformation in the
> > DTB automagically during boot. It's sort of like a DTB overlay that
> > disables the QDSP6 audio path and enables this node instead. The DAI
> > links are also adjusted where necessary.
> >
> > Do you think a comment would help here?
> I'd say a comment would be necessary here :D
>
> >>>
> >>> No problem, I will try to add something simple.
> >>>
> While I understand this use-case, I believe this is better suited
> for an actual DTBO or something entirely kept inside lk2nd.
> Otherwise this looks very confusing to an outside reader.
>
> >>>
> >>> Translating from the QDSP6 audio setup to the LPASS one is mostly simple
> >>> but not entirely trivial (especially the patching needed for the DAI
> >>> links). Main blocker for DTBOs is that you can only add or change, but
> >>> AFAIK there is no mechanism to _delete_ or fully recreate nodes.
> >> Correct.
> >>
> >>>
> >>> I guess I could maybe derive this from the QDSP6 definitions using
> >>> custom magic code, but the code complexity for that is much higher than
> >>> adding these nodes here for completeness.
> >> I hate to be the bearer of bad news, but this is probably more
> >> of a "do we reasonably want this upstream" type question, as
> >> you'll probably get some grumpy emails about upstream not caring
> >> about what's outside the mainline tree..
> >>
> >
> > IMHO this is hardly a "downstream" type of situation. The whole point of
> > the magic in lk2nd is to make the life for the mainline code base easier
> > and simpler, while still allowing the "bypass modem audio" feature with
> > an unmodified kernel.
> >
> > Before implementing it like this I used to have this directly in the
> > kernel tree, by duplicating each device into a normal XYZ.dts and
> > "XYZ-no-modem.dts". If you apply this to the total of ~45 different
> > MSM8916/MSM8939 DTBs I would like to get upstream this options gets
> > extremely ugly. :'D
> Maybe a kernel module parameter could be somehow useful here?
>
Maybe, but ultimately this affecs multiple modules that do not
necessarily know about each other. You need to enable the &lpass node
but also adjust the DAI links in the &sound node to refer to the lpass
DAIs instead of q6afe/q6routing. Implementing this in the kernel would
be complexity everyone would have to carry, while in lk2nd it's local to
the devices that can actually make use of it.
With the device tree we have the flexibility to adjust it based on
device-specifics (RAM size, display panels, broken CPU cores, ...).
I think it's perfectly fine if the bootloader makes good use of this
capability to keep Linux code simple and focused.
Thanks,
Stephan
Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem
On 26.09.2023 22:27, Stephan Gerhold wrote:
> On Tue, Sep 26, 2023 at 10:18:22PM +0200, Konrad Dybcio wrote:
>> On 26.09.2023 22:09, Stephan Gerhold wrote:
>>> On Tue, Sep 26, 2023 at 09:56:12PM +0200, Konrad Dybcio wrote:
[...]
>>> +
>>> &blsp_i2c2 {
>>> status = "okay";
>>>
>>> @@ -243,6 +258,13 @@ &gpu {
>>> status = "okay";
>>> };
>>>
>>> +&lpass {
>>> + dai-link@3 {
>>> + reg = ;
>>> + qcom,playback-sd-lines = <1>;
>>> + };
>>> +};
>> Is that not status = reserved?
>>
>
> Correct. This is here to simplify switching to the modem-bypass audio
> routing if someone does not need (or want) the modem. The direct audio
> path with the LPASS drivers tends to be more reliable and configurable
> (especially wrt bit formats, sampling rates, latency, channels etc).
> I know that at some point this helped someone who tried to use an old
> phone as some kind of portable musical instrument / synthesizer.
>
> It's not too obvious that these definitions would be needed when making
> those changes (because devices using the standard SD lines (i.e <0>) do
> not need it). If you forget about this you get non-functional audio with
> no error or any hint what could be wrong.
>
> To simplify switching between the different audio routing options, the
> lk2nd bootloader actually has an option to do this transformation in the
> DTB automagically during boot. It's sort of like a DTB overlay that
> disables the QDSP6 audio path and enables this node instead. The DAI
> links are also adjusted where necessary.
>
> Do you think a comment would help here?
I'd say a comment would be necessary here :D
>>>
>>> No problem, I will try to add something simple.
>>>
While I understand this use-case, I believe this is better suited
for an actual DTBO or something entirely kept inside lk2nd.
Otherwise this looks very confusing to an outside reader.
>>>
>>> Translating from the QDSP6 audio setup to the LPASS one is mostly simple
>>> but not entirely trivial (especially the patching needed for the DAI
>>> links). Main blocker for DTBOs is that you can only add or change, but
>>> AFAIK there is no mechanism to _delete_ or fully recreate nodes.
>> Correct.
>>
>>>
>>> I guess I could maybe derive this from the QDSP6 definitions using
>>> custom magic code, but the code complexity for that is much higher than
>>> adding these nodes here for completeness.
>> I hate to be the bearer of bad news, but this is probably more
>> of a "do we reasonably want this upstream" type question, as
>> you'll probably get some grumpy emails about upstream not caring
>> about what's outside the mainline tree..
>>
>
> IMHO this is hardly a "downstream" type of situation. The whole point of
> the magic in lk2nd is to make the life for the mainline code base easier
> and simpler, while still allowing the "bypass modem audio" feature with
> an unmodified kernel.
>
> Before implementing it like this I used to have this directly in the
> kernel tree, by duplicating each device into a normal XYZ.dts and
> "XYZ-no-modem.dts". If you apply this to the total of ~45 different
> MSM8916/MSM8939 DTBs I would like to get upstream this options gets
> extremely ugly. :'D
Maybe a kernel module parameter could be somehow useful here?
Konrad
Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem
On Tue, Sep 26, 2023 at 10:18:22PM +0200, Konrad Dybcio wrote:
> On 26.09.2023 22:09, Stephan Gerhold wrote:
> > On Tue, Sep 26, 2023 at 09:56:12PM +0200, Konrad Dybcio wrote:
> >> [...]
> >>
> > +
> > &blsp_i2c2 {
> > status = "okay";
> >
> > @@ -243,6 +258,13 @@ &gpu {
> > status = "okay";
> > };
> >
> > +&lpass {
> > + dai-link@3 {
> > + reg = ;
> > + qcom,playback-sd-lines = <1>;
> > + };
> > +};
> Is that not status = reserved?
>
> >>>
> >>> Correct. This is here to simplify switching to the modem-bypass audio
> >>> routing if someone does not need (or want) the modem. The direct audio
> >>> path with the LPASS drivers tends to be more reliable and configurable
> >>> (especially wrt bit formats, sampling rates, latency, channels etc).
> >>> I know that at some point this helped someone who tried to use an old
> >>> phone as some kind of portable musical instrument / synthesizer.
> >>>
> >>> It's not too obvious that these definitions would be needed when making
> >>> those changes (because devices using the standard SD lines (i.e <0>) do
> >>> not need it). If you forget about this you get non-functional audio with
> >>> no error or any hint what could be wrong.
> >>>
> >>> To simplify switching between the different audio routing options, the
> >>> lk2nd bootloader actually has an option to do this transformation in the
> >>> DTB automagically during boot. It's sort of like a DTB overlay that
> >>> disables the QDSP6 audio path and enables this node instead. The DAI
> >>> links are also adjusted where necessary.
> >>>
> >>> Do you think a comment would help here?
> >> I'd say a comment would be necessary here :D
> >>
> >
> > No problem, I will try to add something simple.
> >
> >> While I understand this use-case, I believe this is better suited
> >> for an actual DTBO or something entirely kept inside lk2nd.
> >> Otherwise this looks very confusing to an outside reader.
> >>
> >
> > Translating from the QDSP6 audio setup to the LPASS one is mostly simple
> > but not entirely trivial (especially the patching needed for the DAI
> > links). Main blocker for DTBOs is that you can only add or change, but
> > AFAIK there is no mechanism to _delete_ or fully recreate nodes.
> Correct.
>
> >
> > I guess I could maybe derive this from the QDSP6 definitions using
> > custom magic code, but the code complexity for that is much higher than
> > adding these nodes here for completeness.
> I hate to be the bearer of bad news, but this is probably more
> of a "do we reasonably want this upstream" type question, as
> you'll probably get some grumpy emails about upstream not caring
> about what's outside the mainline tree..
>
IMHO this is hardly a "downstream" type of situation. The whole point of
the magic in lk2nd is to make the life for the mainline code base easier
and simpler, while still allowing the "bypass modem audio" feature with
an unmodified kernel.
Before implementing it like this I used to have this directly in the
kernel tree, by duplicating each device into a normal XYZ.dts and
"XYZ-no-modem.dts". If you apply this to the total of ~45 different
MSM8916/MSM8939 DTBs I would like to get upstream this options gets
extremely ugly. :'D
Thanks,
Stephan
Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem
On 26.09.2023 22:09, Stephan Gerhold wrote:
> On Tue, Sep 26, 2023 at 09:56:12PM +0200, Konrad Dybcio wrote:
>> [...]
>>
> +
> &blsp_i2c2 {
> status = "okay";
>
> @@ -243,6 +258,13 @@ &gpu {
> status = "okay";
> };
>
> +&lpass {
> + dai-link@3 {
> + reg = ;
> + qcom,playback-sd-lines = <1>;
> + };
> +};
Is that not status = reserved?
>>>
>>> Correct. This is here to simplify switching to the modem-bypass audio
>>> routing if someone does not need (or want) the modem. The direct audio
>>> path with the LPASS drivers tends to be more reliable and configurable
>>> (especially wrt bit formats, sampling rates, latency, channels etc).
>>> I know that at some point this helped someone who tried to use an old
>>> phone as some kind of portable musical instrument / synthesizer.
>>>
>>> It's not too obvious that these definitions would be needed when making
>>> those changes (because devices using the standard SD lines (i.e <0>) do
>>> not need it). If you forget about this you get non-functional audio with
>>> no error or any hint what could be wrong.
>>>
>>> To simplify switching between the different audio routing options, the
>>> lk2nd bootloader actually has an option to do this transformation in the
>>> DTB automagically during boot. It's sort of like a DTB overlay that
>>> disables the QDSP6 audio path and enables this node instead. The DAI
>>> links are also adjusted where necessary.
>>>
>>> Do you think a comment would help here?
>> I'd say a comment would be necessary here :D
>>
>
> No problem, I will try to add something simple.
>
>> While I understand this use-case, I believe this is better suited
>> for an actual DTBO or something entirely kept inside lk2nd.
>> Otherwise this looks very confusing to an outside reader.
>>
>
> Translating from the QDSP6 audio setup to the LPASS one is mostly simple
> but not entirely trivial (especially the patching needed for the DAI
> links). Main blocker for DTBOs is that you can only add or change, but
> AFAIK there is no mechanism to _delete_ or fully recreate nodes.
Correct.
>
> I guess I could maybe derive this from the QDSP6 definitions using
> custom magic code, but the code complexity for that is much higher than
> adding these nodes here for completeness.
I hate to be the bearer of bad news, but this is probably more
of a "do we reasonably want this upstream" type question, as
you'll probably get some grumpy emails about upstream not caring
about what's outside the mainline tree..
>
> Let me try to add some comment first.
Please try to explicitly explain the reasoning of why one would
want this change and what are the drawbacks etc.
Konrad
Re: [PATCH 03/13] arm64: dts: qcom: msm8916: Add common msm8916-modem-qdsp6.dtsi
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!
> [...]
>
> >>> +&lpass_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 = <&sound_dai_primary &sound_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.
I think the overhead of these template notes is absolutely negligible
compared to all the (potentially) unused SoC nodes we have. :D
Thanks,
Stephan
Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem
On Tue, Sep 26, 2023 at 09:56:12PM +0200, Konrad Dybcio wrote:
> [...]
>
> >>> +
> >>> &blsp_i2c2 {
> >>> status = "okay";
> >>>
> >>> @@ -243,6 +258,13 @@ &gpu {
> >>> status = "okay";
> >>> };
> >>>
> >>> +&lpass {
> >>> + dai-link@3 {
> >>> + reg = ;
> >>> + qcom,playback-sd-lines = <1>;
> >>> + };
> >>> +};
> >> Is that not status = reserved?
> >>
> >
> > Correct. This is here to simplify switching to the modem-bypass audio
> > routing if someone does not need (or want) the modem. The direct audio
> > path with the LPASS drivers tends to be more reliable and configurable
> > (especially wrt bit formats, sampling rates, latency, channels etc).
> > I know that at some point this helped someone who tried to use an old
> > phone as some kind of portable musical instrument / synthesizer.
> >
> > It's not too obvious that these definitions would be needed when making
> > those changes (because devices using the standard SD lines (i.e <0>) do
> > not need it). If you forget about this you get non-functional audio with
> > no error or any hint what could be wrong.
> >
> > To simplify switching between the different audio routing options, the
> > lk2nd bootloader actually has an option to do this transformation in the
> > DTB automagically during boot. It's sort of like a DTB overlay that
> > disables the QDSP6 audio path and enables this node instead. The DAI
> > links are also adjusted where necessary.
> >
> > Do you think a comment would help here?
> I'd say a comment would be necessary here :D
>
No problem, I will try to add something simple.
> While I understand this use-case, I believe this is better suited
> for an actual DTBO or something entirely kept inside lk2nd.
> Otherwise this looks very confusing to an outside reader.
>
Translating from the QDSP6 audio setup to the LPASS one is mostly simple
but not entirely trivial (especially the patching needed for the DAI
links). Main blocker for DTBOs is that you can only add or change, but
AFAIK there is no mechanism to _delete_ or fully recreate nodes.
I guess I could maybe derive this from the QDSP6 definitions using
custom magic code, but the code complexity for that is much higher than
adding these nodes here for completeness.
Let me try to add some comment first.
Thanks,
Stephan
Re: [PATCH 05/13] arm64: dts: qcom: msm8916-samsung-serranove: Add sound and modem
On Tue, Sep 26, 2023 at 09:57:51PM +0200, Konrad Dybcio wrote:
> [...]
>
> >>>
> >>> +&sound {
> >>> + status = "okay";
> >>> + audio-routing =
> >>> + "AMIC1", "MIC BIAS External1",
> >>> + "AMIC2", "MIC BIAS Internal2",
> >>> + "AMIC3", "MIC BIAS External1";
> >>> +};
> >> I *think* we should be able to harmlessly enable &audio globally?
> >>
> >
> > What about boards that do not have/use audio at all? (see
> > msm8916-ufi.dtsi). We don't even want to load the kernel modules on
> > those.
> Is it really an issue other than losing a few kb of memory?
>
Well, the msm8916-ufi.dtsi boards (basically USB modem/WiFi sticks)
have 512 MiB of RAM, with 85 MiB reserved for modem firmware, plus more
for TZ, HYP etc etc. That's not too much :D
> >
> > IMO the SoC dtsi should always be minimal by default. This also
> > guarantees that you don't run into trouble because of half- or
> > incorrectly configured components during early bring-up, especially if
> > you don't have serial and are hoping to get the device booting far
> > enough to debug it.
> Generally I'd agree, but if the audio machine driver cannot NOP
> successfully without a topology configuration, that's a problem.
>
I think it will effectively fail to probe because there are no DAI links
and no "model". I guess you could consider this to be a NOP but it's
confusing and takes away the attention from the actual errors in dmesg.
I would say it's disabled by default for the same reason that WiFi,
SDHCI, UFS etc etc are disabled by default. They simply don't do
anything useful without additional configuration & hardware support.
Thanks,
Stephan
Re: [PATCH 03/13] arm64: dts: qcom: msm8916: Add common msm8916-modem-qdsp6.dtsi
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.
[...]
>>> +&lpass_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
[...]
>>> + 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 = <&sound_dai_primary &sound_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?
Konrad
Re: [PATCH 05/13] arm64: dts: qcom: msm8916-samsung-serranove: Add sound and modem
[...]
>>>
>>> +&sound {
>>> + status = "okay";
>>> + audio-routing =
>>> + "AMIC1", "MIC BIAS External1",
>>> + "AMIC2", "MIC BIAS Internal2",
>>> + "AMIC3", "MIC BIAS External1";
>>> +};
>> I *think* we should be able to harmlessly enable &audio globally?
>>
>
> What about boards that do not have/use audio at all? (see
> msm8916-ufi.dtsi). We don't even want to load the kernel modules on
> those.
Is it really an issue other than losing a few kb of memory?
>
> IMO the SoC dtsi should always be minimal by default. This also
> guarantees that you don't run into trouble because of half- or
> incorrectly configured components during early bring-up, especially if
> you don't have serial and are hoping to get the device booting far
> enough to debug it.
Generally I'd agree, but if the audio machine driver cannot NOP
successfully without a topology configuration, that's a problem.
Konrad
Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem
[...]
>>> +
>>> &blsp_i2c2 {
>>> status = "okay";
>>>
>>> @@ -243,6 +258,13 @@ &gpu {
>>> status = "okay";
>>> };
>>>
>>> +&lpass {
>>> + dai-link@3 {
>>> + reg = ;
>>> + qcom,playback-sd-lines = <1>;
>>> + };
>>> +};
>> Is that not status = reserved?
>>
>
> Correct. This is here to simplify switching to the modem-bypass audio
> routing if someone does not need (or want) the modem. The direct audio
> path with the LPASS drivers tends to be more reliable and configurable
> (especially wrt bit formats, sampling rates, latency, channels etc).
> I know that at some point this helped someone who tried to use an old
> phone as some kind of portable musical instrument / synthesizer.
>
> It's not too obvious that these definitions would be needed when making
> those changes (because devices using the standard SD lines (i.e <0>) do
> not need it). If you forget about this you get non-functional audio with
> no error or any hint what could be wrong.
>
> To simplify switching between the different audio routing options, the
> lk2nd bootloader actually has an option to do this transformation in the
> DTB automagically during boot. It's sort of like a DTB overlay that
> disables the QDSP6 audio path and enables this node instead. The DAI
> links are also adjusted where necessary.
>
> Do you think a comment would help here?
I'd say a comment would be necessary here :D
While I understand this use-case, I believe this is better suited
for an actual DTBO or something entirely kept inside lk2nd.
Otherwise this looks very confusing to an outside reader.
Konrad
Re: [PATCH 13/13] arm64: dts: qcom: msm8939-samsung-a7: Add sound and modem
On Tue, Sep 26, 2023 at 09:04:47PM +0200, Konrad Dybcio wrote:
> On 26.09.2023 18:51, Stephan Gerhold wrote:
> > From: "Lin, Meng-Bo"
> >
> > Enable sound and modem for the Samsung A7. The setup is similar to most
> > MSM8916 devices, i.e.:
> >
> > - QDSP6 audio
> > - Earpiece/headphones/microphones via digital/analog codec in
> >MSM8916/PM8916
> > - WWAN Internet via BAM-DMUX
> >
> > except for the same differences as the MSM8916-based Samsung A2015
> > devices:
> >
> > - NXP TFA9895 codec for speaker on Quaternary MI2S
> > - Samsung-specific audio jack detection (not supported yet)
> >
> > Signed-off-by: "Lin, Meng-Bo"
> > [Stephan: Add consistent commit message]
> > Signed-off-by: Stephan Gerhold
> > ---
> [...]
>
> >
> > +&lpass {
> > + dai-link@3 {
> > + reg = ;
> > + qcom,playback-sd-lines = <1>;
> > + };
> > +};
> reserved hw?
>
My previous reply for reference:
https://lore.kernel.org/linux-arm-msm/zrmwdro9ham4b...@gerhold.net/
Thanks,
Stephan
Re: [PATCH 12/13] arm64: dts: qcom: msm8916-samsung-j5: Add sound and modem
On Tue, Sep 26, 2023 at 09:04:22PM +0200, Konrad Dybcio wrote:
> On 26.09.2023 18:51, Stephan Gerhold wrote:
> > From: "Lin, Meng-Bo"
> >
> > Enable sound and modem for the Samsung J5 smartphones. The setup is
> > similar to most MSM8916 devices, i.e.:
> >
> > - QDSP6 audio
> > - Speaker/earpiece/headphones/microphones via digital/analog codec
> >in MSM8916/PM8916
> > - WWAN Internet via BAM-DMUX
> >
> > except:
> >
> > - There is no secondary microphone, so a different "model" is used to
> >differentiate that in the UCM configuration.
> > - Samsung-specific audio jack detection (not supported yet)
> >
> > Co-developed-by: Markuss Broks
> > Signed-off-by: Markuss Broks
> > Signed-off-by: "Lin, Meng-Bo"
> > [Stephan: Add consistent commit message]
> > Signed-off-by: Stephan Gerhold
> > ---
> > arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi | 15
> > +++
> > arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts | 4
> > 2 files changed, 19 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
> > b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
> > index fe59be3505fe..2caa820b0c26 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
> > @@ -1,6 +1,8 @@
> > // SPDX-License-Identifier: GPL-2.0-only
> >
> > #include "msm8916-pm8916.dtsi"
> > +#include "msm8916-modem-qdsp6.dtsi"
> > +
> > #include
> > #include
> > #include
> > @@ -135,6 +137,10 @@ &blsp_uart2 {
> > status = "okay";
> > };
> >
> > +&mpss_mem {
> > + reg = <0x0 0x8680 0x0 0x580>;
> > +};
> > +
> > &pm8916_resin {
> > status = "okay";
> > linux,code = ;
> > @@ -154,6 +160,15 @@ &sdhc_2 {
> > cd-gpios = <&tlmm 38 GPIO_ACTIVE_LOW>;
> > };
> >
> > +&sound {
> > + model = "msm8916-1mic";
> That's.. vague.. Is that intended?
>
msm8916-modem-qdsp6.dtsi defines model = "msm8916" by default since we
have a standard UCM configuration that configures the typical audio
mixer setup when all outputs/inputs are connected to the digital/analog
codec of the SoC/PMIC. "msm8916-1mic" is exactly the same just with the
SecondaryMic disabled.
Unfortunately these names are effectively limited to 15 chars
(everything after will be cut off), so there is extremely limited
potential for more expressive names. :(
> I also noticed only now that random patches have status
> at random places in the property tree.. Removing the disablement
> would aid that as well (wink wink)!
>
Hm, I think I could do the status = "okay" in msm8916-modem-qdsp6.dtsi.
I don't think it should be enabled by default in the SoC dtsi but there
it would probably be fine. :)
Thanks,
Stephan
Re: [PATCH 09/13] arm64: dts: qcom: msm8916-longcheer-l8150: Add sound and modem
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?
>
> [...]
>
> > reserved-memory {
> > + /delete-node/ mpss@8680;
> > /delete-node/ wcnss;
> delete by label, please
>
I would say the same as on PATCH 07/13 here:
https://lore.kernel.org/linux-arm-msm/zrmye1heiuno5...@gerhold.net/
Thanks,
Stephan
Re: [PATCH 11/13] arm64: dts: qcom: msm8916-samsung-gt5: Add sound and modem
On Tue, Sep 26, 2023 at 09:03:14PM +0200, Konrad Dybcio wrote:
> On 26.09.2023 18:51, Stephan Gerhold wrote:
> > From: Jasper Korten
> >
> > Enable sound and modem for the Samsung Galaxy Tab A 2015 tablets.
> > The setup is similar to most MSM8916 devices, i.e.:
> >
> > - QDSP6 audio
> > - Headphones/microphones via digital/analog codec in
> >MSM8916/PM8916. Earpiece exists on samsung-gt58 only.
> > - WWAN Internet via BAM-DMUX
> >
> > except:
> >
> > - gt510: Stereo Maxim MAX98357A codecs for speaker on Quaternary MI2S
> > - gt58: Mono NXP TFA9895 codec for speaker on Quaternary MI2S
> >- For some reason connected to GPIOs where no hardware I2C
> > controller is available -> need to use i2c-gpio
> > - Samsung-specific audio jack detection (not supported yet)
> >
> > Signed-off-by: Jasper Korten
> > Co-developed-by: Siddharth Manthan
> > Signed-off-by: Siddharth Manthan
> > Co-developed-by: Nikita Travkin
> > Signed-off-by: Nikita Travkin
> > [Stephan: Add consistent commit message]
> > Signed-off-by: Stephan Gerhold
> > ---
> > .../boot/dts/qcom/msm8916-samsung-gt5-common.dtsi | 36 ++
> > arch/arm64/boot/dts/qcom/msm8916-samsung-gt510.dts | 23
> > arch/arm64/boot/dts/qcom/msm8916-samsung-gt58.dts | 43
> > ++
> > 3 files changed, 102 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi
> > b/arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi
> > index 6a16eb5ce07b..396853fcece5 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi
> > @@ -3,9 +3,12 @@
> > /dts-v1/;
> >
> > #include "msm8916-pm8916.dtsi"
> > +#include "msm8916-modem-qdsp6.dtsi"
> > +
> > #include
> > #include
> > #include
> > +#include
> >
> > / {
> > aliases {
> > @@ -116,6 +119,17 @@ &blsp_uart2 {
> > status = "okay";
> > };
> >
> > +&lpass {
> > + dai-link@3 {
> > + reg = ;
> > + qcom,playback-sd-lines = <1>;
> > + };
> > +};
> status = reserved?
>
For reference:
https://lore.kernel.org/linux-arm-msm/zrmwdro9ham4b...@gerhold.net/
> [...]
> >
> > + i2c-amplifier {
> > + compatible = "i2c-gpio";
> > + sda-gpios = <&tlmm 55 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> > + scl-gpios = <&tlmm 56 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> non-msm8916 files have a space around the OR operator, hm
>
Hm I can add a space if you think it looks better. :D
Thanks,
Stephan
Re: [PATCH 07/13] arm64: dts: qcom: msm8916-alcatel-idol347: Add sound and modem
On Tue, Sep 26, 2023 at 08:58:12PM +0200, Konrad Dybcio wrote:
> On 26.09.2023 18:51, Stephan Gerhold wrote:
> > From: Vincent Knecht
> >
> > Enable sound and modem for the Alcatel Idol 3 (4.7"). The setup is
> > similar to most MSM8916 devices, i.e.:
> >
> > - QDSP6 audio
> > - Microphones via digital/analog codec in MSM8916/PM8916
> > - WWAN Internet via BAM-DMUX
> >
> > except:
> >
> > - Stereo NXP TFA9890 codecs for speakers on Quaternary MI2S
> >- These are also used as earpieces at the top/bottom.
> > - Asahi Kasei AK4375 headphone codec on Secondary MI2S
> > -> Primary MI2S is not used for playback
> >
> > Signed-off-by: Vincent Knecht
> > [Stephan: minor cleanup, add consistent commit message]
> > Signed-off-by: Stephan Gerhold
> > ---
> > There are some trivial conflicts unless
> > https://lore.kernel.org/linux-arm-msm/20230921-msm8916-rmem-fixups-v1-3-34d2b6e72...@gerhold.net/
> > is applied first. But given that there are important fixups for the
> > dynamic reserved memory changes in that series it should preferably
> > get applied before this one anyway.
> > ---
> > .../boot/dts/qcom/msm8916-alcatel-idol347.dts | 164
> > +
> > 1 file changed, 164 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
> > b/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
> > index fade93c55299..ef5fc9289754 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
> > +++ b/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
> > @@ -3,6 +3,8 @@
> > /dts-v1/;
> >
> > #include "msm8916-pm8916.dtsi"
> > +#include "msm8916-modem-qdsp6.dtsi"
> > +
> > #include
> > #include
> > #include
> > @@ -22,6 +24,19 @@ chosen {
> > stdout-path = "serial0";
> > };
> >
> > + reserved-memory {
> > + /delete-node/ reserved@8668;
> > + /delete-node/ rmtfs@8670;
> Deleting with a label reference is strongly preferred to avoid
> mistakes.
>
I would say the opposite applies here. The deletions are based on the
assumption that the nodes are at the address that are listed here. If
you would move rmtfs somewhere else the adjustments made here must be
re-evaulated.
/delete-node/ throws an error if the referenced name does not exist,
so it's exactly the indication we need if someone makes changes to the
original node in the SoC dtsi.
Note that this is different from property assignments, i.e.
/ {
reserved-memory {
rmtfs@8670 {
status = "disabled";
};
};
};
instead of
&rmtfs {
status = "disabled";
};
because here there would not be an error if the node is renamed.
> [...]
>
> >
> > +&q6afedai {
> > + dai@18 {
> > + reg = ;
> > + qcom,sd-lines = <0>;
> > + };
> > + dai@22 {
> Missing newline above
>
Thanks, will fix this!
>
> > +
> > +&sound_dai_primary {
> > + status = "disabled";
> > +};
> > +
> Hm, gives me an idea to sprinkle a bit more /omit-if-no-ref/ in
> patch 3..
>
(See reply in patch 3, /omit-if-no-ref/ sadly only works for phandle
references...)
Thanks,
Stephan
Re: [PATCH 05/13] arm64: dts: qcom: msm8916-samsung-serranove: Add sound and modem
On Tue, Sep 26, 2023 at 08:55:14PM +0200, Konrad Dybcio wrote:
> On 26.09.2023 18:51, Stephan Gerhold wrote:
> > Enable sound and modem for the Samsung S4 Mini Value Edition. The setup
> > is similar to most MSM8916 devices, i.e.:
> >
> > - QDSP6 audio
> > - Speaker/earpiece/headphones/microphones via digital/analog codec in
> >MSM8916/PM8916
> > - WWAN Internet via BAM-DMUX
> >
> > except:
> >
> > - Samsung-specific audio jack detection (not supported yet)
> >
> > Signed-off-by: Stephan Gerhold
> > ---
> > arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts | 14 ++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
> > b/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
> > index 68da2a2d3077..5f33aa0ad7b5 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
> > +++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
> > @@ -6,6 +6,8 @@
> > /dts-v1/;
> >
> > #include "msm8916-pm8916.dtsi"
> > +#include "msm8916-modem-qdsp6.dtsi"
> > +
> > #include
> > #include
> > #include
> > @@ -319,6 +321,10 @@ &blsp_uart2 {
> > status = "okay";
> > };
> >
> > +&mpss_mem {
> > + reg = <0x0 0x8680 0x0 0x5a0>;
> > +};
> > +
> > &pm8916_resin {
> > status = "okay";
> > linux,code = ;
> > @@ -350,6 +356,14 @@ &sdhc_2 {
> > no-1-8-v;
> > };
> >
> > +&sound {
> > + status = "okay";
> > + audio-routing =
> > + "AMIC1", "MIC BIAS External1",
> > + "AMIC2", "MIC BIAS Internal2",
> > + "AMIC3", "MIC BIAS External1";
> > +};
> I *think* we should be able to harmlessly enable &audio globally?
>
What about boards that do not have/use audio at all? (see
msm8916-ufi.dtsi). We don't even want to load the kernel modules on
those.
IMO the SoC dtsi should always be minimal by default. This also
guarantees that you don't run into trouble because of half- or
incorrectly configured components during early bring-up, especially if
you don't have serial and are hoping to get the device booting far
enough to debug it.
Thanks,
Stephan
Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem
On Tue, Sep 26, 2023 at 08:54:29PM +0200, Konrad Dybcio wrote:
> On 26.09.2023 18:51, Stephan Gerhold wrote:
> > Enable sound and modem for the Samsung A2015 based devices (A3, A5, E5,
> > E7, Grand Max). The setup is similar to most MSM8916 devices, i.e.:
> >
> > - QDSP6 audio
> > - Earpiece/headphones/microphones via digital/analog codec in
> >MSM8916/PM8916
> > - WWAN Internet via BAM-DMUX
> >
> > except:
> >
> > - NXP TFA9895 codec for speaker on Quaternary MI2S
> > - Samsung-specific audio jack detection (not supported yet)
> >
> > [Lin: Add e2015 and grandmax]
> > Co-developed-by: "Lin, Meng-Bo"
> > Signed-off-by: "Lin, Meng-Bo"
> > Signed-off-by: Stephan Gerhold
> > ---
> > .../dts/qcom/msm8916-samsung-a2015-common.dtsi | 55
> > ++
> > .../dts/qcom/msm8916-samsung-e2015-common.dtsi | 4 ++
> > .../boot/dts/qcom/msm8916-samsung-grandmax.dts | 4 ++
> > 3 files changed, 63 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> > b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> > index 0b29132b74e1..f71b18d89bf9 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> > @@ -1,10 +1,13 @@
> > // SPDX-License-Identifier: GPL-2.0-only
> >
> > #include "msm8916-pm8916.dtsi"
> > +#include "msm8916-modem-qdsp6.dtsi"
> > +
> > #include
> > #include
> > #include
> > #include
> > +#include
> >
> > / {
> > aliases {
> > @@ -196,6 +199,18 @@ vibrator: vibrator {
> > };
> > };
> >
> > +&blsp_i2c1 {
> > + status = "okay";
> > +
> > + speaker_codec: audio-codec@34 {
> > + compatible = "nxp,tfa9895";
> > + reg = <0x34>;
> > + vddd-supply = <&pm8916_l5>;
> > + sound-name-prefix = "Speaker";
> > + #sound-dai-cells = <0>;
> > + };
> > +};
> > +
> > &blsp_i2c2 {
> > status = "okay";
> >
> > @@ -243,6 +258,13 @@ &gpu {
> > status = "okay";
> > };
> >
> > +&lpass {
> > + dai-link@3 {
> > + reg = ;
> > + qcom,playback-sd-lines = <1>;
> > + };
> > +};
> Is that not status = reserved?
>
Correct. This is here to simplify switching to the modem-bypass audio
routing if someone does not need (or want) the modem. The direct audio
path with the LPASS drivers tends to be more reliable and configurable
(especially wrt bit formats, sampling rates, latency, channels etc).
I know that at some point this helped someone who tried to use an old
phone as some kind of portable musical instrument / synthesizer.
It's not too obvious that these definitions would be needed when making
those changes (because devices using the standard SD lines (i.e <0>) do
not need it). If you forget about this you get non-functional audio with
no error or any hint what could be wrong.
To simplify switching between the different audio routing options, the
lk2nd bootloader actually has an option to do this transformation in the
DTB automagically during boot. It's sort of like a DTB overlay that
disables the QDSP6 audio path and enables this node instead. The DAI
links are also adjusted where necessary.
Do you think a comment would help here?
Thanks,
Stephan
Re: [PATCH 02/13] arm64: dts: qcom: msm8916/39: Add QDSP6
On Tue, Sep 26, 2023 at 09:05:24PM +0200, Konrad Dybcio wrote: > On 26.09.2023 20:54, Stephan Gerhold wrote: > > On Tue, Sep 26, 2023 at 08:46:36PM +0200, Konrad Dybcio wrote: > >> On 26.09.2023 18:51, Stephan Gerhold wrote: > >>> MSM8916 and MSM8939 do not have a dedicated ADSP. Instead, the audio > >>> services via APR are also implemented by the modem DSP. Audio can be > >>> either routed via the modem DSP (necessary for voice call audio etc) > >>> or directly sent to the LPASS hardware (currently used by DB410c). > >>> Bypassing QDSP6 audio is only possible with special firmware > >>> (on DB410c) or when the modem DSP is completely disabled. > >>> > >>> Add the typical nodes for QDSP6 audio to msm8916.dtsi and msm8939.dtsi. > >>> The apr node is disabled by default to avoid changing behavior for > >>> devices like DB410c that use the bypassed audio path. > >>> > >>> Signed-off-by: Stephan Gerhold > >>> --- > >> I'm generally grumpy with regards to multi-soc changes that > >> have no need to be multi-soc.. > >> > > > > Well it's 100% the same diff so reviewing it separately doesn't really > > make sense IMHO. When I do "msm8916/39" patches these are generally the > > changes where strictly speaking there is no need to duplicate at all. > > It could go into a common include between both. We just haven't found > > a good solution/agreement yet how sharing SoC components could work. > My bottom line is that, somebody trying to track down an issue on > one may need to unnecessarily resolve 2 merge conflicts when reverting :/ > I mean you could easily discard the changes in the other .dtsi. Probably a single shell command if one knows enough "Git-fu". Stephan
Re: [PATCH v2 5/7] dt-bindings: pinctrl: qcom,sc7280: Allow gpio-reserved-ranges
On Tue Sep 19, 2023 at 2:45 PM CEST, Luca Weiss wrote: > Allow the gpio-reserved-ranges property on SC7280 TLMM. > > Acked-by: Linus Walleij > Acked-by: Krzysztof Kozlowski > Signed-off-by: Luca Weiss Hi Linus, the rest of this series is merged so would be great if you could pick up this patch (as you wrote in v1) :) Regards Luca > --- > Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml | 4 > 1 file changed, 4 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml > b/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml > index 368d44ff5468..c8735ab97e40 100644 > --- a/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml > +++ b/Documentation/devicetree/bindings/pinctrl/qcom,sc7280-pinctrl.yaml > @@ -41,6 +41,10 @@ properties: >gpio-ranges: > maxItems: 1 > > + gpio-reserved-ranges: > +minItems: 1 > +maxItems: 88 > + >gpio-line-names: > maxItems: 175 >
Re: [PATCH 03/13] arm64: dts: qcom: msm8916: Add common msm8916-modem-qdsp6.dtsi
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.
> > arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi | 163
> > ++
> > 1 file changed, 163 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi
> > b/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi
> > new file mode 100644
> > index ..ddd74d428406
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi
> > @@ -0,0 +1,163 @@
> > +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
> > +/*
> > + * msm8916-modem-qdsp6.dtsi describes the typical modem setup on MSM8916
> > devices
> > + * (or similar SoCs) with audio routed via the QDSP6 services provided by
> > the
> > + * modem firmware. The digital/analog codec in the SoC/PMIC is used by
> > default,
> > + * but boards can define additional codecs using the templates for
> > Secondary and
> > + * Quaternary MI2S.
> > + */
> > +
> > +#include
> > +#include
> > +
> > +&apr {
> > + status = "okay";
> > +};
> > +
> > +&bam_dmux {
> > + status = "okay";
> > +};
> > +
> > +&bam_dmux_dma {
> > + status = "okay";
> > +};
> > +
> > +&lpass {
> > + status = "reserved"; /* Controlled by QDSP6 */
> > +};
> > +
> > +&lpass_codec {
> > + status = "okay";
> > +};
> Any reason for it to stay disabled?
>
You mean in msm8916.dtsi? 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).
This include is a bit more "opinionated", to reduce duplication for
the most common setup. But it's separate and optional to use. The SoC
dtsi is included by everyone.
> > +
> > +&mba_mem {
> > + status = "okay";
> > +};
> > +
> > +&mpss {
> > + status = "okay";
> > +};
> > +
> > +&mpss_mem {
> > + status = "okay";
> > +};
> > +
> > +&pm8916_codec {
> > + status = "okay";
> > +};
> Ditto
>
Same as above.
> > + multimedia1-dai-link {
> > + link-name = "MultiMedia1";
> Newline before last property and subnodes, please
>
Thanks, will change this!
>
> > + 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 = <&sound_dai_primary &sound_dai_secondary>;
But this doesn't exist so /omit-if-no-ref/ cannot be used here.
Thanks,
Stephan
Re: [PATCH 02/13] arm64: dts: qcom: msm8916/39: Add QDSP6
On 26.09.2023 20:54, Stephan Gerhold wrote: > On Tue, Sep 26, 2023 at 08:46:36PM +0200, Konrad Dybcio wrote: >> On 26.09.2023 18:51, Stephan Gerhold wrote: >>> MSM8916 and MSM8939 do not have a dedicated ADSP. Instead, the audio >>> services via APR are also implemented by the modem DSP. Audio can be >>> either routed via the modem DSP (necessary for voice call audio etc) >>> or directly sent to the LPASS hardware (currently used by DB410c). >>> Bypassing QDSP6 audio is only possible with special firmware >>> (on DB410c) or when the modem DSP is completely disabled. >>> >>> Add the typical nodes for QDSP6 audio to msm8916.dtsi and msm8939.dtsi. >>> The apr node is disabled by default to avoid changing behavior for >>> devices like DB410c that use the bypassed audio path. >>> >>> Signed-off-by: Stephan Gerhold >>> --- >> I'm generally grumpy with regards to multi-soc changes that >> have no need to be multi-soc.. >> > > Well it's 100% the same diff so reviewing it separately doesn't really > make sense IMHO. When I do "msm8916/39" patches these are generally the > changes where strictly speaking there is no need to duplicate at all. > It could go into a common include between both. We just haven't found > a good solution/agreement yet how sharing SoC components could work. My bottom line is that, somebody trying to track down an issue on one may need to unnecessarily resolve 2 merge conflicts when reverting :/ Konrad
Re: [PATCH 13/13] arm64: dts: qcom: msm8939-samsung-a7: Add sound and modem
On 26.09.2023 18:51, Stephan Gerhold wrote:
> From: "Lin, Meng-Bo"
>
> Enable sound and modem for the Samsung A7. The setup is similar to most
> MSM8916 devices, i.e.:
>
> - QDSP6 audio
> - Earpiece/headphones/microphones via digital/analog codec in
>MSM8916/PM8916
> - WWAN Internet via BAM-DMUX
>
> except for the same differences as the MSM8916-based Samsung A2015
> devices:
>
> - NXP TFA9895 codec for speaker on Quaternary MI2S
> - Samsung-specific audio jack detection (not supported yet)
>
> Signed-off-by: "Lin, Meng-Bo"
> [Stephan: Add consistent commit message]
> Signed-off-by: Stephan Gerhold
> ---
[...]
>
> +&lpass {
> + dai-link@3 {
> + reg = ;
> + qcom,playback-sd-lines = <1>;
> + };
> +};
reserved hw?
Konrad
Re: [PATCH 12/13] arm64: dts: qcom: msm8916-samsung-j5: Add sound and modem
On 26.09.2023 18:51, Stephan Gerhold wrote:
> From: "Lin, Meng-Bo"
>
> Enable sound and modem for the Samsung J5 smartphones. The setup is
> similar to most MSM8916 devices, i.e.:
>
> - QDSP6 audio
> - Speaker/earpiece/headphones/microphones via digital/analog codec
>in MSM8916/PM8916
> - WWAN Internet via BAM-DMUX
>
> except:
>
> - There is no secondary microphone, so a different "model" is used to
>differentiate that in the UCM configuration.
> - Samsung-specific audio jack detection (not supported yet)
>
> Co-developed-by: Markuss Broks
> Signed-off-by: Markuss Broks
> Signed-off-by: "Lin, Meng-Bo"
> [Stephan: Add consistent commit message]
> Signed-off-by: Stephan Gerhold
> ---
> arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi | 15 +++
> arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts | 4
> 2 files changed, 19 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
> b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
> index fe59be3505fe..2caa820b0c26 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
> @@ -1,6 +1,8 @@
> // SPDX-License-Identifier: GPL-2.0-only
>
> #include "msm8916-pm8916.dtsi"
> +#include "msm8916-modem-qdsp6.dtsi"
> +
> #include
> #include
> #include
> @@ -135,6 +137,10 @@ &blsp_uart2 {
> status = "okay";
> };
>
> +&mpss_mem {
> + reg = <0x0 0x8680 0x0 0x580>;
> +};
> +
> &pm8916_resin {
> status = "okay";
> linux,code = ;
> @@ -154,6 +160,15 @@ &sdhc_2 {
> cd-gpios = <&tlmm 38 GPIO_ACTIVE_LOW>;
> };
>
> +&sound {
> + model = "msm8916-1mic";
That's.. vague.. Is that intended?
I also noticed only now that random patches have status
at random places in the property tree.. Removing the disablement
would aid that as well (wink wink)!
Konrad
Re: [PATCH 11/13] arm64: dts: qcom: msm8916-samsung-gt5: Add sound and modem
On 26.09.2023 18:51, Stephan Gerhold wrote:
> From: Jasper Korten
>
> Enable sound and modem for the Samsung Galaxy Tab A 2015 tablets.
> The setup is similar to most MSM8916 devices, i.e.:
>
> - QDSP6 audio
> - Headphones/microphones via digital/analog codec in
>MSM8916/PM8916. Earpiece exists on samsung-gt58 only.
> - WWAN Internet via BAM-DMUX
>
> except:
>
> - gt510: Stereo Maxim MAX98357A codecs for speaker on Quaternary MI2S
> - gt58: Mono NXP TFA9895 codec for speaker on Quaternary MI2S
>- For some reason connected to GPIOs where no hardware I2C
> controller is available -> need to use i2c-gpio
> - Samsung-specific audio jack detection (not supported yet)
>
> Signed-off-by: Jasper Korten
> Co-developed-by: Siddharth Manthan
> Signed-off-by: Siddharth Manthan
> Co-developed-by: Nikita Travkin
> Signed-off-by: Nikita Travkin
> [Stephan: Add consistent commit message]
> Signed-off-by: Stephan Gerhold
> ---
> .../boot/dts/qcom/msm8916-samsung-gt5-common.dtsi | 36 ++
> arch/arm64/boot/dts/qcom/msm8916-samsung-gt510.dts | 23
> arch/arm64/boot/dts/qcom/msm8916-samsung-gt58.dts | 43
> ++
> 3 files changed, 102 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi
> b/arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi
> index 6a16eb5ce07b..396853fcece5 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi
> @@ -3,9 +3,12 @@
> /dts-v1/;
>
> #include "msm8916-pm8916.dtsi"
> +#include "msm8916-modem-qdsp6.dtsi"
> +
> #include
> #include
> #include
> +#include
>
> / {
> aliases {
> @@ -116,6 +119,17 @@ &blsp_uart2 {
> status = "okay";
> };
>
> +&lpass {
> + dai-link@3 {
> + reg = ;
> + qcom,playback-sd-lines = <1>;
> + };
> +};
status = reserved?
[...]
>
> + i2c-amplifier {
> + compatible = "i2c-gpio";
> + sda-gpios = <&tlmm 55 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> + scl-gpios = <&tlmm 56 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
non-msm8916 files have a space around the OR operator, hm
Konrad
Re: [PATCH 10/13] arm64: dts: qcom: msm8916-longcheer-l8910: Add sound and modem
On 26.09.2023 18:51, Stephan Gerhold wrote: > From: Jonathan Albrieux > > Enable sound and modem for the Longcheer L8910 (BQ Aquaris X5). > The setup is similar to most MSM8916 devices, i.e.: > > - QDSP6 audio > - Earpiece/headphones/microphones via digital/analog codec in >MSM8916/PM8916 > - Audio jack detection via analog codec in PM8916 > - WWAN Internet via BAM-DMUX > > except: > > - Awinic AW8738 connected to HPH_R (headphones) output of the analog >codec. Note that unlike for wingtech-wt88047 there is no analog >switch that would allow disabling output via the headphone jack >when the speaker is enabled. > > Signed-off-by: Jonathan Albrieux > Co-developed-by: Stephan Gerhold > Signed-off-by: Stephan Gerhold > --- Reviewed-by: Konrad Dybcio Konrad
Re: [PATCH 09/13] arm64: dts: qcom: msm8916-longcheer-l8150: Add sound and modem
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?
[...]
> reserved-memory {
> + /delete-node/ mpss@8680;
> /delete-node/ wcnss;
delete by label, please
Konrad
Re: [PATCH 08/13] arm64: dts: qcom: msm8916-asus-z00l: Add sound and modem
On 26.09.2023 18:51, Stephan Gerhold wrote: > From: "J.R. Divya Antony" > > Enable sound and modem for the ASUS Zenfone 2 Laser. The setup is > similar to most MSM8916 devices, i.e.: > > - QDSP6 audio > - Speakear/earpiece/headphones/microphones via digital/analog codec >in MSM8916/PM8916 > - Audio jack detection via analog codec in PM8916 > - WWAN Internet via BAM-DMUX > > Signed-off-by: J.R. Divya Antony > [Stephan: rebase and simplify, add consistent commit message] > Signed-off-by: Stephan Gerhold > --- Reviewed-by: Konrad Dybcio Konrad
Re: [PATCH 07/13] arm64: dts: qcom: msm8916-alcatel-idol347: Add sound and modem
On 26.09.2023 18:51, Stephan Gerhold wrote:
> From: Vincent Knecht
>
> Enable sound and modem for the Alcatel Idol 3 (4.7"). The setup is
> similar to most MSM8916 devices, i.e.:
>
> - QDSP6 audio
> - Microphones via digital/analog codec in MSM8916/PM8916
> - WWAN Internet via BAM-DMUX
>
> except:
>
> - Stereo NXP TFA9890 codecs for speakers on Quaternary MI2S
>- These are also used as earpieces at the top/bottom.
> - Asahi Kasei AK4375 headphone codec on Secondary MI2S
> -> Primary MI2S is not used for playback
>
> Signed-off-by: Vincent Knecht
> [Stephan: minor cleanup, add consistent commit message]
> Signed-off-by: Stephan Gerhold
> ---
> There are some trivial conflicts unless
> https://lore.kernel.org/linux-arm-msm/20230921-msm8916-rmem-fixups-v1-3-34d2b6e72...@gerhold.net/
> is applied first. But given that there are important fixups for the
> dynamic reserved memory changes in that series it should preferably
> get applied before this one anyway.
> ---
> .../boot/dts/qcom/msm8916-alcatel-idol347.dts | 164
> +
> 1 file changed, 164 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
> b/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
> index fade93c55299..ef5fc9289754 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
> +++ b/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
> @@ -3,6 +3,8 @@
> /dts-v1/;
>
> #include "msm8916-pm8916.dtsi"
> +#include "msm8916-modem-qdsp6.dtsi"
> +
> #include
> #include
> #include
> @@ -22,6 +24,19 @@ chosen {
> stdout-path = "serial0";
> };
>
> + reserved-memory {
> + /delete-node/ reserved@8668;
> + /delete-node/ rmtfs@8670;
Deleting with a label reference is strongly preferred to avoid
mistakes.
[...]
>
> +&q6afedai {
> + dai@18 {
> + reg = ;
> + qcom,sd-lines = <0>;
> + };
> + dai@22 {
Missing newline above
[...]
> +
> +&sound_dai_primary {
> + status = "disabled";
> +};
> +
Hm, gives me an idea to sprinkle a bit more /omit-if-no-ref/ in
patch 3..
Konrad
Re: [PATCH 06/13] arm64: dts: qcom: msm8916-wingtech-wt88047: Add sound and modem
On 26.09.2023 18:51, Stephan Gerhold wrote:
> Enable sound and modem for the Xiaomi Redmi 2. The setup
> is similar to most MSM8916 devices, i.e.:
>
> - QDSP6 audio
> - Earpiece/headphones/microphones via digital/analog codec in
>MSM8916/PM8916
> - Audio jack detection via analog codec in PM8916
> - WWAN Internet via BAM-DMUX
>
> except:
>
> - Speaker amplifier is connected to HPH_R (headphones) output of the
>analog codec. There is a separate analog switch that allows disabling
>playback via the headphone jack.
>
> Signed-off-by: Stephan Gerhold
> ---
> .../boot/dts/qcom/msm8916-wingtech-wt88047.dts | 76
> ++
> 1 file changed, 76 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916-wingtech-wt88047.dts
> b/arch/arm64/boot/dts/qcom/msm8916-wingtech-wt88047.dts
> index 419f35c1fc92..600c225a2568 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916-wingtech-wt88047.dts
> +++ b/arch/arm64/boot/dts/qcom/msm8916-wingtech-wt88047.dts
> @@ -6,6 +6,8 @@
> /dts-v1/;
>
> #include "msm8916-pm8916.dtsi"
> +#include "msm8916-modem-qdsp6.dtsi"
> +
> #include
> #include
> #include
> @@ -25,6 +27,28 @@ chosen {
> stdout-path = "serial0";
> };
>
> + speaker_amp: audio-amplifier {
> + compatible = "simple-audio-amplifier";
> + enable-gpios = <&tlmm 117 GPIO_ACTIVE_HIGH>;
> + sound-name-prefix = "Speaker Amp";
> + pinctrl-0 = <&speaker_amp_default>;
> + pinctrl-names = "default";
> + };
> +
> + /*
> + * This seems to be actually an analog switch that either routes audio
> + * to the headphone jack or nowhere. Given that we need to enable a GPIO
> + * to get sound on headphones, modelling it as simple-audio-amplifier
> + * works just fine.
> + */
Funny phones, as always
Reviewed-by: Konrad Dybcio
Konrad
Re: [PATCH 02/13] arm64: dts: qcom: msm8916/39: Add QDSP6
On Tue, Sep 26, 2023 at 08:46:36PM +0200, Konrad Dybcio wrote: > On 26.09.2023 18:51, Stephan Gerhold wrote: > > MSM8916 and MSM8939 do not have a dedicated ADSP. Instead, the audio > > services via APR are also implemented by the modem DSP. Audio can be > > either routed via the modem DSP (necessary for voice call audio etc) > > or directly sent to the LPASS hardware (currently used by DB410c). > > Bypassing QDSP6 audio is only possible with special firmware > > (on DB410c) or when the modem DSP is completely disabled. > > > > Add the typical nodes for QDSP6 audio to msm8916.dtsi and msm8939.dtsi. > > The apr node is disabled by default to avoid changing behavior for > > devices like DB410c that use the bypassed audio path. > > > > Signed-off-by: Stephan Gerhold > > --- > I'm generally grumpy with regards to multi-soc changes that > have no need to be multi-soc.. > Well it's 100% the same diff so reviewing it separately doesn't really make sense IMHO. When I do "msm8916/39" patches these are generally the changes where strictly speaking there is no need to duplicate at all. It could go into a common include between both. We just haven't found a good solution/agreement yet how sharing SoC components could work. Thanks, Stephan
Re: [PATCH 05/13] arm64: dts: qcom: msm8916-samsung-serranove: Add sound and modem
On 26.09.2023 18:51, Stephan Gerhold wrote:
> Enable sound and modem for the Samsung S4 Mini Value Edition. The setup
> is similar to most MSM8916 devices, i.e.:
>
> - QDSP6 audio
> - Speaker/earpiece/headphones/microphones via digital/analog codec in
>MSM8916/PM8916
> - WWAN Internet via BAM-DMUX
>
> except:
>
> - Samsung-specific audio jack detection (not supported yet)
>
> Signed-off-by: Stephan Gerhold
> ---
> arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
> b/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
> index 68da2a2d3077..5f33aa0ad7b5 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
> +++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
> @@ -6,6 +6,8 @@
> /dts-v1/;
>
> #include "msm8916-pm8916.dtsi"
> +#include "msm8916-modem-qdsp6.dtsi"
> +
> #include
> #include
> #include
> @@ -319,6 +321,10 @@ &blsp_uart2 {
> status = "okay";
> };
>
> +&mpss_mem {
> + reg = <0x0 0x8680 0x0 0x5a0>;
> +};
> +
> &pm8916_resin {
> status = "okay";
> linux,code = ;
> @@ -350,6 +356,14 @@ &sdhc_2 {
> no-1-8-v;
> };
>
> +&sound {
> + status = "okay";
> + audio-routing =
> + "AMIC1", "MIC BIAS External1",
> + "AMIC2", "MIC BIAS Internal2",
> + "AMIC3", "MIC BIAS External1";
> +};
I *think* we should be able to harmlessly enable &audio globally?
Konrad
Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem
On 26.09.2023 18:51, Stephan Gerhold wrote:
> Enable sound and modem for the Samsung A2015 based devices (A3, A5, E5,
> E7, Grand Max). The setup is similar to most MSM8916 devices, i.e.:
>
> - QDSP6 audio
> - Earpiece/headphones/microphones via digital/analog codec in
>MSM8916/PM8916
> - WWAN Internet via BAM-DMUX
>
> except:
>
> - NXP TFA9895 codec for speaker on Quaternary MI2S
> - Samsung-specific audio jack detection (not supported yet)
>
> [Lin: Add e2015 and grandmax]
> Co-developed-by: "Lin, Meng-Bo"
> Signed-off-by: "Lin, Meng-Bo"
> Signed-off-by: Stephan Gerhold
> ---
> .../dts/qcom/msm8916-samsung-a2015-common.dtsi | 55
> ++
> .../dts/qcom/msm8916-samsung-e2015-common.dtsi | 4 ++
> .../boot/dts/qcom/msm8916-samsung-grandmax.dts | 4 ++
> 3 files changed, 63 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> index 0b29132b74e1..f71b18d89bf9 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
> @@ -1,10 +1,13 @@
> // SPDX-License-Identifier: GPL-2.0-only
>
> #include "msm8916-pm8916.dtsi"
> +#include "msm8916-modem-qdsp6.dtsi"
> +
> #include
> #include
> #include
> #include
> +#include
>
> / {
> aliases {
> @@ -196,6 +199,18 @@ vibrator: vibrator {
> };
> };
>
> +&blsp_i2c1 {
> + status = "okay";
> +
> + speaker_codec: audio-codec@34 {
> + compatible = "nxp,tfa9895";
> + reg = <0x34>;
> + vddd-supply = <&pm8916_l5>;
> + sound-name-prefix = "Speaker";
> + #sound-dai-cells = <0>;
> + };
> +};
> +
> &blsp_i2c2 {
> status = "okay";
>
> @@ -243,6 +258,13 @@ &gpu {
> status = "okay";
> };
>
> +&lpass {
> + dai-link@3 {
> + reg = ;
> + qcom,playback-sd-lines = <1>;
> + };
> +};
Is that not status = reserved?
Konrad
Re: [PATCH v1 1/2] ACPI: NFIT: Fix memory leak, and local use of devm_*()
On 9/26/23 11:45, 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.
>
> Fixes: eaf961536e16 ("libnvdimm, nfit: add interleave-set state-tracking
> infrastructure")
> Reported-by: Andy Shevchenko
> Signed-off-by: Michal Wilczynski
Reviewed-by: Dave Jiang
> ---
> drivers/acpi/nfit/core.c | 23 +++
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index f0e6738ae3c9..78f0f855c4de 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -2262,6 +2262,7 @@ static int acpi_nfit_init_interleave_set(struct
> acpi_nfit_desc *acpi_desc,
> u16 nr = ndr_desc->num_mappings;
> struct nfit_set_info2 *info2;
> struct nfit_set_info *info;
> + int err = 0;
> int i;
>
> nd_set = devm_kzalloc(dev, sizeof(*nd_set), GFP_KERNEL);
> @@ -2269,13 +2270,15 @@ static int acpi_nfit_init_interleave_set(struct
> acpi_nfit_desc *acpi_desc,
> return -ENOMEM;
> import_guid(&nd_set->type_guid, spa->range_guid);
>
> - info = devm_kcalloc(dev, nr, sizeof(*info), GFP_KERNEL);
> + info = kcalloc(nr, sizeof(*info), GFP_KERNEL);
> if (!info)
> return -ENOMEM;
>
> - info2 = devm_kcalloc(dev, nr, sizeof(*info2), GFP_KERNEL);
> - if (!info2)
> - return -ENOMEM;
> + info2 = kcalloc(nr, sizeof(*info2), GFP_KERNEL);
> + if (!info2) {
> + err = -ENOMEM;
> + goto free_info;
> + }
>
> for (i = 0; i < nr; i++) {
> struct nd_mapping_desc *mapping = &ndr_desc->mapping[i];
> @@ -2289,7 +2292,8 @@ static int acpi_nfit_init_interleave_set(struct
> acpi_nfit_desc *acpi_desc,
>
> if (!memdev || !nfit_mem->dcr) {
> dev_err(dev, "%s: failed to find DCR\n", __func__);
> - return -ENODEV;
> + err = -ENODEV;
> + goto free_info2;
> }
>
> map->region_offset = memdev->region_offset;
> @@ -2337,10 +2341,13 @@ static int acpi_nfit_init_interleave_set(struct
> acpi_nfit_desc *acpi_desc,
> }
>
> ndr_desc->nd_set = nd_set;
> - devm_kfree(dev, info);
> - devm_kfree(dev, info2);
>
> - return 0;
> +free_info2:
> + kfree(info2);
> +free_info:
> + kfree(info);
> +
> + return err;
> }
>
> static int ars_get_cap(struct acpi_nfit_desc *acpi_desc,
Re: [PATCH v1 2/2] ACPI: NFIT: Use modern scope based rollback
On 9/26/23 11:45, 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.
>
> Suggested-by: Dave Jiang
> Suggested-by: Andy Shevchenko
> Signed-off-by: Michal Wilczynski
Reviewed-by: Dave Jiang
> ---
> drivers/acpi/nfit/core.c | 32 ++--
> 1 file changed, 10 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 78f0f855c4de..079bd663495f 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -2257,29 +2257,23 @@ static int acpi_nfit_init_interleave_set(struct
> acpi_nfit_desc *acpi_desc,
> struct nd_region_desc *ndr_desc,
> struct acpi_nfit_system_address *spa)
> {
> + u16 nr = ndr_desc->num_mappings;
> + struct nfit_set_info2 *info2 __free(kfree) =
> + kcalloc(nr, sizeof(*info2), GFP_KERNEL);
> + struct nfit_set_info *info __free(kfree) =
> + kcalloc(nr, sizeof(*info), GFP_KERNEL);
> struct device *dev = acpi_desc->dev;
> struct nd_interleave_set *nd_set;
> - u16 nr = ndr_desc->num_mappings;
> - struct nfit_set_info2 *info2;
> - struct nfit_set_info *info;
> - int err = 0;
> int i;
>
> + if (!info || !info2)
> + return -ENOMEM;
> +
> nd_set = devm_kzalloc(dev, sizeof(*nd_set), GFP_KERNEL);
> if (!nd_set)
> return -ENOMEM;
> import_guid(&nd_set->type_guid, spa->range_guid);
>
> - info = kcalloc(nr, sizeof(*info), GFP_KERNEL);
> - if (!info)
> - return -ENOMEM;
> -
> - info2 = kcalloc(nr, sizeof(*info2), GFP_KERNEL);
> - if (!info2) {
> - err = -ENOMEM;
> - goto free_info;
> - }
> -
> for (i = 0; i < nr; i++) {
> struct nd_mapping_desc *mapping = &ndr_desc->mapping[i];
> struct nvdimm *nvdimm = mapping->nvdimm;
> @@ -2292,8 +2286,7 @@ static int acpi_nfit_init_interleave_set(struct
> acpi_nfit_desc *acpi_desc,
>
> if (!memdev || !nfit_mem->dcr) {
> dev_err(dev, "%s: failed to find DCR\n", __func__);
> - err = -ENODEV;
> - goto free_info2;
> + return -ENODEV;
> }
>
> map->region_offset = memdev->region_offset;
> @@ -2342,12 +2335,7 @@ static int acpi_nfit_init_interleave_set(struct
> acpi_nfit_desc *acpi_desc,
>
> ndr_desc->nd_set = nd_set;
>
> -free_info2:
> - kfree(info2);
> -free_info:
> - kfree(info);
> -
> - return err;
> + return 0;
> }
>
> static int ars_get_cap(struct acpi_nfit_desc *acpi_desc,
Re: [PATCH 03/13] arm64: dts: qcom: msm8916: Add common msm8916-modem-qdsp6.dtsi
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..
> arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi | 163
> ++
> 1 file changed, 163 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi
> b/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi
> new file mode 100644
> index ..ddd74d428406
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi
> @@ -0,0 +1,163 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
> +/*
> + * msm8916-modem-qdsp6.dtsi describes the typical modem setup on MSM8916
> devices
> + * (or similar SoCs) with audio routed via the QDSP6 services provided by the
> + * modem firmware. The digital/analog codec in the SoC/PMIC is used by
> default,
> + * but boards can define additional codecs using the templates for Secondary
> and
> + * Quaternary MI2S.
> + */
> +
> +#include
> +#include
> +
> +&apr {
> + status = "okay";
> +};
> +
> +&bam_dmux {
> + status = "okay";
> +};
> +
> +&bam_dmux_dma {
> + status = "okay";
> +};
> +
> +&lpass {
> + status = "reserved"; /* Controlled by QDSP6 */
> +};
> +
> +&lpass_codec {
> + status = "okay";
> +};
Any reason for it to stay disabled?
> +
> +&mba_mem {
> + status = "okay";
> +};
> +
> +&mpss {
> + status = "okay";
> +};
> +
> +&mpss_mem {
> + status = "okay";
> +};
> +
> +&pm8916_codec {
> + status = "okay";
> +};
Ditto
[...]
> + multimedia1-dai-link {
> + link-name = "MultiMedia1";
Newline before last property and subnodes, please
[...]
> + 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/?
Konrad
Re: [PATCH 02/13] arm64: dts: qcom: msm8916/39: Add QDSP6
On 26.09.2023 18:51, Stephan Gerhold wrote: > MSM8916 and MSM8939 do not have a dedicated ADSP. Instead, the audio > services via APR are also implemented by the modem DSP. Audio can be > either routed via the modem DSP (necessary for voice call audio etc) > or directly sent to the LPASS hardware (currently used by DB410c). > Bypassing QDSP6 audio is only possible with special firmware > (on DB410c) or when the modem DSP is completely disabled. > > Add the typical nodes for QDSP6 audio to msm8916.dtsi and msm8939.dtsi. > The apr node is disabled by default to avoid changing behavior for > devices like DB410c that use the bypassed audio path. > > Signed-off-by: Stephan Gerhold > --- I'm generally grumpy with regards to multi-soc changes that have no need to be multi-soc.. Konrad
[PATCH v1 1/2] ACPI: NFIT: Fix memory leak, and local use of devm_*()
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.
Fixes: eaf961536e16 ("libnvdimm, nfit: add interleave-set state-tracking
infrastructure")
Reported-by: Andy Shevchenko
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 23 +++
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index f0e6738ae3c9..78f0f855c4de 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -2262,6 +2262,7 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
u16 nr = ndr_desc->num_mappings;
struct nfit_set_info2 *info2;
struct nfit_set_info *info;
+ int err = 0;
int i;
nd_set = devm_kzalloc(dev, sizeof(*nd_set), GFP_KERNEL);
@@ -2269,13 +2270,15 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
return -ENOMEM;
import_guid(&nd_set->type_guid, spa->range_guid);
- info = devm_kcalloc(dev, nr, sizeof(*info), GFP_KERNEL);
+ info = kcalloc(nr, sizeof(*info), GFP_KERNEL);
if (!info)
return -ENOMEM;
- info2 = devm_kcalloc(dev, nr, sizeof(*info2), GFP_KERNEL);
- if (!info2)
- return -ENOMEM;
+ info2 = kcalloc(nr, sizeof(*info2), GFP_KERNEL);
+ if (!info2) {
+ err = -ENOMEM;
+ goto free_info;
+ }
for (i = 0; i < nr; i++) {
struct nd_mapping_desc *mapping = &ndr_desc->mapping[i];
@@ -2289,7 +2292,8 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
if (!memdev || !nfit_mem->dcr) {
dev_err(dev, "%s: failed to find DCR\n", __func__);
- return -ENODEV;
+ err = -ENODEV;
+ goto free_info2;
}
map->region_offset = memdev->region_offset;
@@ -2337,10 +2341,13 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
}
ndr_desc->nd_set = nd_set;
- devm_kfree(dev, info);
- devm_kfree(dev, info2);
- return 0;
+free_info2:
+ kfree(info2);
+free_info:
+ kfree(info);
+
+ return err;
}
static int ars_get_cap(struct acpi_nfit_desc *acpi_desc,
--
2.41.0
[PATCH v1 2/2] ACPI: NFIT: Use modern scope based rollback
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.
Suggested-by: Dave Jiang
Suggested-by: Andy Shevchenko
Signed-off-by: Michal Wilczynski
---
drivers/acpi/nfit/core.c | 32 ++--
1 file changed, 10 insertions(+), 22 deletions(-)
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 78f0f855c4de..079bd663495f 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -2257,29 +2257,23 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
struct nd_region_desc *ndr_desc,
struct acpi_nfit_system_address *spa)
{
+ u16 nr = ndr_desc->num_mappings;
+ struct nfit_set_info2 *info2 __free(kfree) =
+ kcalloc(nr, sizeof(*info2), GFP_KERNEL);
+ struct nfit_set_info *info __free(kfree) =
+ kcalloc(nr, sizeof(*info), GFP_KERNEL);
struct device *dev = acpi_desc->dev;
struct nd_interleave_set *nd_set;
- u16 nr = ndr_desc->num_mappings;
- struct nfit_set_info2 *info2;
- struct nfit_set_info *info;
- int err = 0;
int i;
+ if (!info || !info2)
+ return -ENOMEM;
+
nd_set = devm_kzalloc(dev, sizeof(*nd_set), GFP_KERNEL);
if (!nd_set)
return -ENOMEM;
import_guid(&nd_set->type_guid, spa->range_guid);
- info = kcalloc(nr, sizeof(*info), GFP_KERNEL);
- if (!info)
- return -ENOMEM;
-
- info2 = kcalloc(nr, sizeof(*info2), GFP_KERNEL);
- if (!info2) {
- err = -ENOMEM;
- goto free_info;
- }
-
for (i = 0; i < nr; i++) {
struct nd_mapping_desc *mapping = &ndr_desc->mapping[i];
struct nvdimm *nvdimm = mapping->nvdimm;
@@ -2292,8 +2286,7 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
if (!memdev || !nfit_mem->dcr) {
dev_err(dev, "%s: failed to find DCR\n", __func__);
- err = -ENODEV;
- goto free_info2;
+ return -ENODEV;
}
map->region_offset = memdev->region_offset;
@@ -2342,12 +2335,7 @@ static int acpi_nfit_init_interleave_set(struct
acpi_nfit_desc *acpi_desc,
ndr_desc->nd_set = nd_set;
-free_info2:
- kfree(info2);
-free_info:
- kfree(info);
-
- return err;
+ return 0;
}
static int ars_get_cap(struct acpi_nfit_desc *acpi_desc,
--
2.41.0
[PATCH v1 0/2] Fix memory leak and move to modern scope based rollback
In acpi_nfit_init_interleave_set() there is a memory leak + improper use of devm_*() family of functions for local memory allocations. This patch series provides two commits - one is meant as a bug fix, and could potentially be backported, and the other one improves old style rollback with scope based, similar to C++ RAII [1]. Link: https://lwn.net/Articles/934679/ [1] Michal Wilczynski (2): ACPI: NFIT: Fix memory leak, and local use of devm_*() ACPI: NFIT: Use modern scope based rollback drivers/acpi/nfit/core.c | 21 - 1 file changed, 8 insertions(+), 13 deletions(-) -- 2.41.0
Re: [PATCH 01/13] arm64: dts: qcom: msm8939: Add BAM-DMUX WWAN
On 26.09.2023 18:51, Stephan Gerhold wrote: > From: Vincent Knecht > > BAM DMUX is used as the network interface to the modem. This is copied > as-is from msm8916.dtsi. > > Signed-off-by: Vincent Knecht > Signed-off-by: Stephan Gerhold > --- Reviewed-by: Konrad Dybcio Konrad
Re: SPDX: Appletalk FW license in the kernel
On 9/26/23 1:34 AM, Christoph Hellwig wrote: On Fri, Sep 15, 2023 at 09:39:05AM -0400, Prarit Bhargava wrote: Is there anyone you know of that we could approach to determine a proper SPDX License for these files? Answering this question generally, even though it sounds like it wasn't needed for this particular situation: YES! If you find a license in the kernel that does not match a license already on the SPDX License List and want to submit the license for inclusion on the SPDX License List (which, if accepted, means the license will get an SPDX id assigned), please follow this process: https://github.com/spdx/license-list-XML/blob/main/DOCS/request-new-license.md By the way, people on the linux-spdx list may be interested to know that Fedora has adopted the use of SPDX license ids in the license field of Fedora package metadata. There has been close collaboration between the two projects, which has resulted in 95 licenses or exceptions added to the SPDX License List so far. I think this is a great thing (even if a lot of work) as it is making the SPDX License List more reflective of the reality of open source software licensing (including all the variations on old permissive licenses). Jilayne
[PATCH 02/13] arm64: dts: qcom: msm8916/39: Add QDSP6
MSM8916 and MSM8939 do not have a dedicated ADSP. Instead, the audio
services via APR are also implemented by the modem DSP. Audio can be
either routed via the modem DSP (necessary for voice call audio etc)
or directly sent to the LPASS hardware (currently used by DB410c).
Bypassing QDSP6 audio is only possible with special firmware
(on DB410c) or when the modem DSP is completely disabled.
Add the typical nodes for QDSP6 audio to msm8916.dtsi and msm8939.dtsi.
The apr node is disabled by default to avoid changing behavior for
devices like DB410c that use the bypassed audio path.
Signed-off-by: Stephan Gerhold
---
arch/arm64/boot/dts/qcom/msm8916.dtsi | 49 +++
arch/arm64/boot/dts/qcom/msm8939.dtsi | 49 +++
2 files changed, 98 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi
b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 4f799b536a92..e8a14dd7e7c2 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
/ {
@@ -1989,6 +1990,54 @@ smd-edge {
label = "hexagon";
+ apr: apr {
+ compatible = "qcom,apr-v2";
+ qcom,smd-channels = "apr_audio_svc";
+ qcom,domain = ;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+
+ q6core: service@3 {
+ compatible = "qcom,q6core";
+ reg = ;
+ };
+
+ q6afe: service@4 {
+ compatible = "qcom,q6afe";
+ reg = ;
+
+ q6afedai: dais {
+ compatible =
"qcom,q6afe-dais";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #sound-dai-cells = <1>;
+ };
+ };
+
+ q6asm: service@7 {
+ compatible = "qcom,q6asm";
+ reg = ;
+
+ q6asmdai: dais {
+ compatible =
"qcom,q6asm-dais";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #sound-dai-cells = <1>;
+ };
+ };
+
+ q6adm: service@8 {
+ compatible = "qcom,q6adm";
+ reg = ;
+
+ q6routing: routing {
+ compatible =
"qcom,q6adm-routing";
+ #sound-dai-cells = <0>;
+ };
+ };
+ };
+
fastrpc {
compatible = "qcom,fastrpc";
qcom,smd-channels =
"fastrpcsmd-apps-dsp";
diff --git a/arch/arm64/boot/dts/qcom/msm8939.dtsi
b/arch/arm64/boot/dts/qcom/msm8939.dtsi
index 65c68e0e88d5..95610a32750a 100644
--- a/arch/arm64/boot/dts/qcom/msm8939.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8939.dtsi
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
/ {
@@ -1615,6 +1616,54 @@ smd-edge {
qcom,remote-pid = <1>;
label = "hexagon";
+
+ apr: apr {
+ compatible = "qcom,apr-v2";
+ qcom,smd-channels = "apr_audio_svc";
+ qcom,domain = ;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+
+ q6core: service@3 {
+ compatible = "qcom,q6core";
+
[PATCH 13/13] arm64: dts: qcom: msm8939-samsung-a7: Add sound and modem
From: "Lin, Meng-Bo"
Enable sound and modem for the Samsung A7. The setup is similar to most
MSM8916 devices, i.e.:
- QDSP6 audio
- Earpiece/headphones/microphones via digital/analog codec in
MSM8916/PM8916
- WWAN Internet via BAM-DMUX
except for the same differences as the MSM8916-based Samsung A2015
devices:
- NXP TFA9895 codec for speaker on Quaternary MI2S
- Samsung-specific audio jack detection (not supported yet)
Signed-off-by: "Lin, Meng-Bo"
[Stephan: Add consistent commit message]
Signed-off-by: Stephan Gerhold
---
arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts | 54 +
1 file changed, 54 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts
b/arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts
index fccd8fec8b8f..4951b3e265d2 100644
--- a/arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts
+++ b/arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts
@@ -3,10 +3,12 @@
/dts-v1/;
#include "msm8939-pm8916.dtsi"
+#include "msm8916-modem-qdsp6.dtsi"
#include
#include
#include
+#include
/ {
model = "Samsung Galaxy A7 (2015)";
@@ -287,6 +289,18 @@ muic: extcon@25 {
};
};
+&blsp_i2c2 {
+ status = "okay";
+
+ speaker_codec: audio-codec@34 {
+ compatible = "nxp,tfa9895";
+ reg = <0x34>;
+ vddd-supply = <&pm8916_l5>;
+ sound-name-prefix = "Speaker";
+ #sound-dai-cells = <0>;
+ };
+};
+
&blsp_i2c5 {
status = "okay";
@@ -309,6 +323,17 @@ &blsp_uart2 {
status = "okay";
};
+&lpass {
+ dai-link@3 {
+ reg = ;
+ qcom,playback-sd-lines = <1>;
+ };
+};
+
+&mpss_mem {
+ reg = <0x0 0x8680 0x0 0x580>;
+};
+
&pm8916_resin {
linux,code = ;
status = "okay";
@@ -321,6 +346,13 @@ pm8916_l17: l17 {
};
};
+&q6afedai {
+ dai@22 {
+ reg = ;
+ qcom,sd-lines = <1>;
+ };
+};
+
&sdhc_1 {
status = "okay";
};
@@ -335,6 +367,28 @@ &sdhc_2 {
status = "okay";
};
+&sound {
+ model = "samsung-a2015";
+ audio-routing =
+ "AMIC1", "MIC BIAS External1",
+ "AMIC2", "MIC BIAS Internal2",
+ "AMIC3", "MIC BIAS External1";
+
+ pinctrl-0 = <&cdc_pdm_default &sec_mi2s_default>;
+ pinctrl-1 = <&cdc_pdm_sleep &sec_mi2s_sleep>;
+ pinctrl-names = "default", "sleep";
+
+ status = "okay";
+};
+
+&sound_dai_quaternary {
+ status = "okay";
+
+ codec {
+ sound-dai = <&speaker_codec>;
+ };
+};
+
&usb {
extcon = <&muic>, <&muic>;
status = "okay";
--
2.42.0
[PATCH 09/13] arm64: dts: qcom: msm8916-longcheer-l8150: Add sound and modem
From: Nikita Travkin
Enable sound and modem for the Longcheer L8150 (e.g. Wileyfox Swift).
The setup is similar to most MSM8916 devices, i.e.:
- QDSP6 audio
- Speaker/earpiece/headphones/microphones via digital/analog codec
in MSM8916/PM8916
- Audio jack detection via analog codec in PM8916
- WWAN Internet via BAM-DMUX
except:
- The mpss firmware region must be relocated to a different address.
This is because the wcnss firmware is not relocatable for some
reason. The mpss firmware is too large to avoid overlap with wcnss
when placed at the default address (0x8680).
Surprisingly the vendor kernel does not handle this. The firmware
regions end up overlapping there and somehow this does not explode.
We try to handle this more safely by relocating the mpss region to
the first higher address that is working correctly: 0x8e80.
Signed-off-by: Nikita Travkin
Co-developed-by: Stephan Gerhold
Signed-off-by: Stephan Gerhold
---
.../boot/dts/qcom/msm8916-longcheer-l8150.dts | 32 --
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts
b/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts
index 47d1c5cb13f4..78f08254b287 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts
@@ -3,6 +3,8 @@
/dts-v1/;
#include "msm8916-pm8916.dtsi"
+#include "msm8916-modem-qdsp6.dtsi"
+
#include
#include
#include
@@ -25,17 +27,26 @@ chosen {
/*
* For some reason, the signed wcnss firmware is not relocatable.
-* It must be loaded at 0x8b60. All other firmware is relocatable,
-* so place wcnss at the fixed address and then all other firmware
-* regions will be automatically allocated at a fitting place.
+* It must be loaded at 0x8b60. Unfortunately, this also means that
+* mpss_mem does not fit when loaded to the typical address at
0x8680.
+*
+* Load wcnss_mem to the fixed address and relocate mpss_mem to the next
+* working higher address. For some reason the modem firmware does not
+* boot when placed at 0x8a80 to 0x8e80.
*/
reserved-memory {
+ /delete-node/ mpss@8680;
/delete-node/ wcnss;
wcnss_mem: wcnss@8b60 {
reg = <0x0 0x8b60 0x0 0x60>;
no-map;
};
+
+ mpss_mem: mpss@8e80 {
+ reg = <0x0 0x8e80 0x0 0x500>;
+ no-map;
+ };
};
gpio-keys {
@@ -225,6 +236,13 @@ &blsp_uart2 {
status = "okay";
};
+&pm8916_codec {
+ qcom,micbias-lvl = <2800>;
+ qcom,mbhc-vthreshold-low = <75 150 237 450 500>;
+ qcom,mbhc-vthreshold-high = <75 150 237 450 500>;
+ qcom,hphl-jack-type-normally-open;
+};
+
&pm8916_resin {
status = "okay";
linux,code = ;
@@ -254,6 +272,14 @@ &sdhc_2 {
non-removable;
};
+&sound {
+ status = "okay";
+ audio-routing =
+ "AMIC1", "MIC BIAS Internal1",
+ "AMIC2", "MIC BIAS Internal2",
+ "AMIC3", "MIC BIAS Internal3";
+};
+
&usb {
status = "okay";
dr_mode = "peripheral";
--
2.42.0
[PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem
Enable sound and modem for the Samsung A2015 based devices (A3, A5, E5,
E7, Grand Max). The setup is similar to most MSM8916 devices, i.e.:
- QDSP6 audio
- Earpiece/headphones/microphones via digital/analog codec in
MSM8916/PM8916
- WWAN Internet via BAM-DMUX
except:
- NXP TFA9895 codec for speaker on Quaternary MI2S
- Samsung-specific audio jack detection (not supported yet)
[Lin: Add e2015 and grandmax]
Co-developed-by: "Lin, Meng-Bo"
Signed-off-by: "Lin, Meng-Bo"
Signed-off-by: Stephan Gerhold
---
.../dts/qcom/msm8916-samsung-a2015-common.dtsi | 55 ++
.../dts/qcom/msm8916-samsung-e2015-common.dtsi | 4 ++
.../boot/dts/qcom/msm8916-samsung-grandmax.dts | 4 ++
3 files changed, 63 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
index 0b29132b74e1..f71b18d89bf9 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
@@ -1,10 +1,13 @@
// SPDX-License-Identifier: GPL-2.0-only
#include "msm8916-pm8916.dtsi"
+#include "msm8916-modem-qdsp6.dtsi"
+
#include
#include
#include
#include
+#include
/ {
aliases {
@@ -196,6 +199,18 @@ vibrator: vibrator {
};
};
+&blsp_i2c1 {
+ status = "okay";
+
+ speaker_codec: audio-codec@34 {
+ compatible = "nxp,tfa9895";
+ reg = <0x34>;
+ vddd-supply = <&pm8916_l5>;
+ sound-name-prefix = "Speaker";
+ #sound-dai-cells = <0>;
+ };
+};
+
&blsp_i2c2 {
status = "okay";
@@ -243,6 +258,13 @@ &gpu {
status = "okay";
};
+&lpass {
+ dai-link@3 {
+ reg = ;
+ qcom,playback-sd-lines = <1>;
+ };
+};
+
&mdss {
status = "okay";
};
@@ -253,6 +275,10 @@ &mdss_dsi0 {
pinctrl-1 = <&mdss_sleep>;
};
+&mpss_mem {
+ reg = <0x0 0x8680 0x0 0x540>;
+};
+
&pm8916_resin {
status = "okay";
linux,code = ;
@@ -265,6 +291,13 @@ pm8916_l17: l17 {
};
};
+&q6afedai {
+ dai@22 {
+ reg = ;
+ qcom,sd-lines = <1>;
+ };
+};
+
&sdhc_1 {
status = "okay";
};
@@ -279,6 +312,28 @@ &sdhc_2 {
cd-gpios = <&tlmm 38 GPIO_ACTIVE_LOW>;
};
+&sound {
+ status = "okay";
+
+ model = "samsung-a2015";
+ audio-routing =
+ "AMIC1", "MIC BIAS External1",
+ "AMIC2", "MIC BIAS Internal2",
+ "AMIC3", "MIC BIAS External1";
+
+ pinctrl-0 = <&cdc_pdm_default &sec_mi2s_default>;
+ pinctrl-1 = <&cdc_pdm_sleep &sec_mi2s_sleep>;
+ pinctrl-names = "default", "sleep";
+};
+
+&sound_dai_quaternary {
+ status = "okay";
+
+ codec {
+ sound-dai = <&speaker_codec>;
+ };
+};
+
&usb {
status = "okay";
extcon = <&muic>, <&muic>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-e2015-common.dtsi
b/arch/arm64/boot/dts/qcom/msm8916-samsung-e2015-common.dtsi
index 0824ab041d80..3c49dac92d2d 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-e2015-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-e2015-common.dtsi
@@ -65,6 +65,10 @@ accelerometer@1d {
};
};
+&mpss_mem {
+ reg = <0x0 0x8680 0x0 0x5a0>;
+};
+
®_motor_vdd {
regulator-min-microvolt = <330>;
regulator-max-microvolt = <330>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-grandmax.dts
b/arch/arm64/boot/dts/qcom/msm8916-samsung-grandmax.dts
index 3f145dde4059..5882b3a593b8 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-grandmax.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-grandmax.dts
@@ -49,6 +49,10 @@ ®_touch_key {
status = "disabled";
};
+&sound {
+ model = "samsung-gmax"; /* No secondary microphone */
+};
+
&tlmm {
gpio_leds_default: gpio-led-default-state {
pins = "gpio60";
--
2.42.0
[PATCH 10/13] arm64: dts: qcom: msm8916-longcheer-l8910: Add sound and modem
From: Jonathan Albrieux
Enable sound and modem for the Longcheer L8910 (BQ Aquaris X5).
The setup is similar to most MSM8916 devices, i.e.:
- QDSP6 audio
- Earpiece/headphones/microphones via digital/analog codec in
MSM8916/PM8916
- Audio jack detection via analog codec in PM8916
- WWAN Internet via BAM-DMUX
except:
- Awinic AW8738 connected to HPH_R (headphones) output of the analog
codec. Note that unlike for wingtech-wt88047 there is no analog
switch that would allow disabling output via the headphone jack
when the speaker is enabled.
Signed-off-by: Jonathan Albrieux
Co-developed-by: Stephan Gerhold
Signed-off-by: Stephan Gerhold
---
.../boot/dts/qcom/msm8916-longcheer-l8910.dts | 54 ++
1 file changed, 54 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8910.dts
b/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8910.dts
index 41cadb906b98..c0dc9a3bbac4 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8910.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8910.dts
@@ -3,6 +3,8 @@
/dts-v1/;
#include "msm8916-pm8916.dtsi"
+#include "msm8916-modem-qdsp6.dtsi"
+
#include
#include
#include
@@ -22,6 +24,16 @@ chosen {
stdout-path = "serial0";
};
+ speaker_amp: audio-amplifier {
+ compatible = "awinic,aw8738";
+ mode-gpios = <&tlmm 114 GPIO_ACTIVE_HIGH>;
+ awinic,mode = <5>;
+ sound-name-prefix = "Speaker Amp";
+
+ pinctrl-0 = <&spk_ext_pa_default>;
+ pinctrl-names = "default";
+ };
+
flash-led-controller {
compatible = "ocs,ocp8110";
enable-gpios = <&tlmm 49 GPIO_ACTIVE_HIGH>;
@@ -107,6 +119,17 @@ &blsp_uart2 {
status = "okay";
};
+&mpss_mem {
+ reg = <0x0 0x8680 0x0 0x500>;
+};
+
+&pm8916_codec {
+ qcom,micbias-lvl = <2800>;
+ qcom,mbhc-vthreshold-low = <75 100 120 180 500>;
+ qcom,mbhc-vthreshold-high = <75 100 120 180 500>;
+ qcom,hphl-jack-type-normally-open;
+};
+
&pm8916_resin {
status = "okay";
linux,code = ;
@@ -137,6 +160,30 @@ &sdhc_2 {
cd-gpios = <&tlmm 38 GPIO_ACTIVE_LOW>;
};
+&sound {
+ status = "okay";
+
+ /*
+* Provide widgets/pin-switches to allow enabling speaker separately.
+* The hardware does not provide a way to disable the output via the
+* headphone jack when the speaker is enabled.
+*/
+ model = "bq-paella";
+ widgets =
+ "Speaker", "Speaker",
+ "Headphone", "Headphones";
+ pin-switches = "Speaker";
+ audio-routing =
+ "Speaker", "Speaker Amp OUT",
+ "Speaker Amp IN", "HPH_R",
+ "Headphones", "HPH_L",
+ "Headphones", "HPH_R",
+ "AMIC1", "MIC BIAS External1",
+ "AMIC2", "MIC BIAS Internal2",
+ "AMIC3", "MIC BIAS External1";
+ aux-devs = <&speaker_amp>;
+};
+
&usb {
status = "okay";
extcon = <&usb_id>, <&usb_id>;
@@ -205,6 +252,13 @@ sdc2_cd_default: sdc2-cd-default-state {
bias-disable;
};
+ spk_ext_pa_default: spk-ext-pa-default-state {
+ pins = "gpio114";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
usb_id_default: usb-id-default-state {
pins = "gpio110";
function = "gpio";
--
2.42.0
[PATCH 07/13] arm64: dts: qcom: msm8916-alcatel-idol347: Add sound and modem
From: Vincent Knecht
Enable sound and modem for the Alcatel Idol 3 (4.7"). The setup is
similar to most MSM8916 devices, i.e.:
- QDSP6 audio
- Microphones via digital/analog codec in MSM8916/PM8916
- WWAN Internet via BAM-DMUX
except:
- Stereo NXP TFA9890 codecs for speakers on Quaternary MI2S
- These are also used as earpieces at the top/bottom.
- Asahi Kasei AK4375 headphone codec on Secondary MI2S
-> Primary MI2S is not used for playback
Signed-off-by: Vincent Knecht
[Stephan: minor cleanup, add consistent commit message]
Signed-off-by: Stephan Gerhold
---
There are some trivial conflicts unless
https://lore.kernel.org/linux-arm-msm/20230921-msm8916-rmem-fixups-v1-3-34d2b6e72...@gerhold.net/
is applied first. But given that there are important fixups for the
dynamic reserved memory changes in that series it should preferably
get applied before this one anyway.
---
.../boot/dts/qcom/msm8916-alcatel-idol347.dts | 164 +
1 file changed, 164 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
b/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
index fade93c55299..ef5fc9289754 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-alcatel-idol347.dts
@@ -3,6 +3,8 @@
/dts-v1/;
#include "msm8916-pm8916.dtsi"
+#include "msm8916-modem-qdsp6.dtsi"
+
#include
#include
#include
@@ -22,6 +24,19 @@ chosen {
stdout-path = "serial0";
};
+ reserved-memory {
+ /delete-node/ reserved@8668;
+ /delete-node/ rmtfs@8670;
+
+ rmtfs: rmtfs@8668 {
+ compatible = "qcom,rmtfs-mem";
+ reg = <0x0 0x8668 0x0 0x16>;
+ no-map;
+
+ qcom,client-id = <1>;
+ };
+ };
+
gpio-keys {
compatible = "gpio-keys";
@@ -50,6 +65,17 @@ led-0 {
};
};
+ reg_headphones_avdd: regulator-headphones-avdd {
+ compatible = "regulator-fixed";
+ regulator-name = "headphones_avdd";
+ regulator-min-microvolt = <180>;
+ regulator-max-microvolt = <180>;
+ gpio = <&tlmm 121 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ pinctrl-0 = <&headphones_avdd_default>;
+ pinctrl-names = "default";
+ };
+
usb_id: usb-id {
compatible = "linux,extcon-usb-gpio";
id-gpios = <&tlmm 69 GPIO_ACTIVE_HIGH>;
@@ -58,6 +84,43 @@ usb_id: usb-id {
};
};
+&blsp_i2c3 {
+ status = "okay";
+
+ headphones: audio-codec@10 {
+ compatible = "asahi-kasei,ak4375";
+ reg = <0x10>;
+ avdd-supply = <®_headphones_avdd>;
+ tvdd-supply = <&pm8916_l6>;
+ pdn-gpios = <&tlmm 114 GPIO_ACTIVE_HIGH>;
+ pinctrl-0 = <&headphones_pdn_default>;
+ pinctrl-names = "default";
+ #sound-dai-cells = <0>;
+ };
+
+ speaker_codec_top: audio-codec@34 {
+ compatible = "nxp,tfa9897";
+ reg = <0x34>;
+ vddd-supply = <&pm8916_l6>;
+ rcv-gpios = <&tlmm 50 GPIO_ACTIVE_HIGH>;
+ pinctrl-0 = <&speaker_top_default>;
+ pinctrl-names = "default";
+ sound-name-prefix = "Speaker Top";
+ #sound-dai-cells = <0>;
+ };
+
+ speaker_codec_bottom: audio-codec@36 {
+ compatible = "nxp,tfa9897";
+ reg = <0x36>;
+ vddd-supply = <&pm8916_l6>;
+ rcv-gpios = <&tlmm 111 GPIO_ACTIVE_HIGH>;
+ pinctrl-0 = <&speaker_bottom_default>;
+ pinctrl-names = "default";
+ sound-name-prefix = "Speaker Bottom";
+ #sound-dai-cells = <0>;
+ };
+};
+
&blsp_i2c4 {
status = "okay";
@@ -153,6 +216,18 @@ &blsp_uart2 {
status = "okay";
};
+&mpss_mem {
+ reg = <0x0 0x8680 0x0 0x500>;
+};
+
+&pm8916_codec {
+ qcom,micbias1-ext-cap;
+ qcom,micbias-lvl = <2800>;
+ qcom,mbhc-vthreshold-low = <75 100 120 180 500>;
+ qcom,mbhc-vthreshold-high = <75 100 120 180 500>;
+ qcom,hphl-jack-type-normally-open;
+};
+
&pm8916_resin {
status = "okay";
linux,code = ;
@@ -169,6 +244,17 @@ &pm8916_vib {
status = "okay";
};
+&q6afedai {
+ dai@18 {
+ reg = ;
+ qcom,sd-lines = <0>;
+ };
+ dai@22 {
+ reg = ;
+ qcom,sd-lines = <0>;
+ };
+};
+
&sdhc_1 {
status = "okay";
};
@@ -183,6 +269,47 @@ &sdhc_2 {
cd-gpios = <&tlmm 38 GPIO_ACTIVE_LOW>;
};
+&sound {
+ status = "okay";
+
+ /* Add pin switches for speakers to allow disabling them individually */
+ model = "alcate
[PATCH 05/13] arm64: dts: qcom: msm8916-samsung-serranove: Add sound and modem
Enable sound and modem for the Samsung S4 Mini Value Edition. The setup
is similar to most MSM8916 devices, i.e.:
- QDSP6 audio
- Speaker/earpiece/headphones/microphones via digital/analog codec in
MSM8916/PM8916
- WWAN Internet via BAM-DMUX
except:
- Samsung-specific audio jack detection (not supported yet)
Signed-off-by: Stephan Gerhold
---
arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
b/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
index 68da2a2d3077..5f33aa0ad7b5 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-serranove.dts
@@ -6,6 +6,8 @@
/dts-v1/;
#include "msm8916-pm8916.dtsi"
+#include "msm8916-modem-qdsp6.dtsi"
+
#include
#include
#include
@@ -319,6 +321,10 @@ &blsp_uart2 {
status = "okay";
};
+&mpss_mem {
+ reg = <0x0 0x8680 0x0 0x5a0>;
+};
+
&pm8916_resin {
status = "okay";
linux,code = ;
@@ -350,6 +356,14 @@ &sdhc_2 {
no-1-8-v;
};
+&sound {
+ status = "okay";
+ audio-routing =
+ "AMIC1", "MIC BIAS External1",
+ "AMIC2", "MIC BIAS Internal2",
+ "AMIC3", "MIC BIAS External1";
+};
+
&usb {
status = "okay";
extcon = <&muic>, <&muic>;
--
2.42.0
[PATCH 11/13] arm64: dts: qcom: msm8916-samsung-gt5: Add sound and modem
From: Jasper Korten
Enable sound and modem for the Samsung Galaxy Tab A 2015 tablets.
The setup is similar to most MSM8916 devices, i.e.:
- QDSP6 audio
- Headphones/microphones via digital/analog codec in
MSM8916/PM8916. Earpiece exists on samsung-gt58 only.
- WWAN Internet via BAM-DMUX
except:
- gt510: Stereo Maxim MAX98357A codecs for speaker on Quaternary MI2S
- gt58: Mono NXP TFA9895 codec for speaker on Quaternary MI2S
- For some reason connected to GPIOs where no hardware I2C
controller is available -> need to use i2c-gpio
- Samsung-specific audio jack detection (not supported yet)
Signed-off-by: Jasper Korten
Co-developed-by: Siddharth Manthan
Signed-off-by: Siddharth Manthan
Co-developed-by: Nikita Travkin
Signed-off-by: Nikita Travkin
[Stephan: Add consistent commit message]
Signed-off-by: Stephan Gerhold
---
.../boot/dts/qcom/msm8916-samsung-gt5-common.dtsi | 36 ++
arch/arm64/boot/dts/qcom/msm8916-samsung-gt510.dts | 23
arch/arm64/boot/dts/qcom/msm8916-samsung-gt58.dts | 43 ++
3 files changed, 102 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi
b/arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi
index 6a16eb5ce07b..396853fcece5 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-gt5-common.dtsi
@@ -3,9 +3,12 @@
/dts-v1/;
#include "msm8916-pm8916.dtsi"
+#include "msm8916-modem-qdsp6.dtsi"
+
#include
#include
#include
+#include
/ {
aliases {
@@ -116,6 +119,17 @@ &blsp_uart2 {
status = "okay";
};
+&lpass {
+ dai-link@3 {
+ reg = ;
+ qcom,playback-sd-lines = <1>;
+ };
+};
+
+&mpss_mem {
+ reg = <0x0 0x8680 0x0 0x540>;
+};
+
&pm8916_resin {
linux,code = ;
status = "okay";
@@ -133,6 +147,13 @@ &pm8916_usbin {
status = "okay";
};
+&q6afedai {
+ dai@22 {
+ reg = ;
+ qcom,sd-lines = <1>;
+ };
+};
+
&sdhc_1 {
status = "okay";
};
@@ -147,6 +168,21 @@ &sdhc_2 {
status = "okay";
};
+&sound {
+ audio-routing =
+ "AMIC1", "MIC BIAS External1",
+ "AMIC2", "MIC BIAS Internal2",
+ "AMIC3", "MIC BIAS External1";
+};
+
+&sound_dai_quaternary {
+ status = "okay";
+
+ codec {
+ sound-dai = <&speaker_codec>;
+ };
+};
+
&usb {
dr_mode = "peripheral";
extcon = <&pm8916_usbin>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-gt510.dts
b/arch/arm64/boot/dts/qcom/msm8916-samsung-gt510.dts
index c3f1acc55078..f393e9efa72c 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-gt510.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-gt510.dts
@@ -9,6 +9,14 @@ / {
compatible = "samsung,gt510", "qcom,msm8916";
chassis-type = "tablet";
+ speaker_codec: audio-codec {
+ compatible = "maxim,max98357a";
+ sdmode-gpios = <&tlmm 55 GPIO_ACTIVE_HIGH>;
+ #sound-dai-cells = <0>;
+ pinctrl-0 = <&audio_sdmode_default>;
+ pinctrl-names = "default";
+ };
+
clk_pwm: pwm {
compatible = "clk-pwm";
#pwm-cells = <2>;
@@ -146,7 +154,22 @@ &mdss_dsi0_out {
remote-endpoint = <&panel_in>;
};
+&sound {
+ model = "samsung-gt510";
+ pinctrl-0 = <&cdc_pdm_default &sec_mi2s_default>;
+ pinctrl-1 = <&cdc_pdm_sleep &sec_mi2s_sleep>;
+ pinctrl-names = "default", "sleep";
+ status = "okay";
+};
+
&tlmm {
+ audio_sdmode_default: audio-sdmode-default-state {
+ pins = "gpio55";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
buckbooster_en_default: buckbooster-en-default-state {
pins = "gpio51";
function = "gpio";
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-gt58.dts
b/arch/arm64/boot/dts/qcom/msm8916-samsung-gt58.dts
index 998625abd409..3f2165556986 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-gt58.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-gt58.dts
@@ -35,6 +35,26 @@ reg_vdd_tsp: regulator-vdd-tsp {
pinctrl-names = "default";
};
+ i2c-amplifier {
+ compatible = "i2c-gpio";
+ sda-gpios = <&tlmm 55 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+ scl-gpios = <&tlmm 56 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+
+ pinctrl-0 = <&_i2c_default>;
+ pinctrl-names = "default";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ speaker_codec: audio-codec@34 {
+ compatible = "nxp,tfa9895";
+ reg = <0x34>;
+ vddd-supply = <&pm8916_l5>;
+ sound-name-prefix = "Speaker
[PATCH 12/13] arm64: dts: qcom: msm8916-samsung-j5: Add sound and modem
From: "Lin, Meng-Bo"
Enable sound and modem for the Samsung J5 smartphones. The setup is
similar to most MSM8916 devices, i.e.:
- QDSP6 audio
- Speaker/earpiece/headphones/microphones via digital/analog codec
in MSM8916/PM8916
- WWAN Internet via BAM-DMUX
except:
- There is no secondary microphone, so a different "model" is used to
differentiate that in the UCM configuration.
- Samsung-specific audio jack detection (not supported yet)
Co-developed-by: Markuss Broks
Signed-off-by: Markuss Broks
Signed-off-by: "Lin, Meng-Bo"
[Stephan: Add consistent commit message]
Signed-off-by: Stephan Gerhold
---
arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi | 15 +++
arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts | 4
2 files changed, 19 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
index fe59be3505fe..2caa820b0c26 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5-common.dtsi
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: GPL-2.0-only
#include "msm8916-pm8916.dtsi"
+#include "msm8916-modem-qdsp6.dtsi"
+
#include
#include
#include
@@ -135,6 +137,10 @@ &blsp_uart2 {
status = "okay";
};
+&mpss_mem {
+ reg = <0x0 0x8680 0x0 0x580>;
+};
+
&pm8916_resin {
status = "okay";
linux,code = ;
@@ -154,6 +160,15 @@ &sdhc_2 {
cd-gpios = <&tlmm 38 GPIO_ACTIVE_LOW>;
};
+&sound {
+ model = "msm8916-1mic";
+ audio-routing =
+ "AMIC1", "MIC BIAS External1",
+ "AMIC2", "MIC BIAS Internal2",
+ "AMIC3", "MIC BIAS External1";
+ status = "okay";
+};
+
&usb {
extcon = <&muic>, <&muic>;
status = "okay";
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts
b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts
index 58c2f5a70e78..ba8650971d6a 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts
@@ -19,6 +19,10 @@ &blsp_i2c5 {
status = "disabled";
};
+&pm8916_codec {
+ qcom,micbias1-ext-cap;
+};
+
&touchscreen {
/* FIXME: Missing sm5703-mfd driver to power up vdd-supply */
};
--
2.42.0
[PATCH 03/13] arm64: dts: qcom: msm8916: Add common msm8916-modem-qdsp6.dtsi
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
---
arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi | 163 ++
1 file changed, 163 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi
b/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi
new file mode 100644
index ..ddd74d428406
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi
@@ -0,0 +1,163 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+/*
+ * msm8916-modem-qdsp6.dtsi describes the typical modem setup on MSM8916
devices
+ * (or similar SoCs) with audio routed via the QDSP6 services provided by the
+ * modem firmware. The digital/analog codec in the SoC/PMIC is used by default,
+ * but boards can define additional codecs using the templates for Secondary
and
+ * Quaternary MI2S.
+ */
+
+#include
+#include
+
+&apr {
+ status = "okay";
+};
+
+&bam_dmux {
+ status = "okay";
+};
+
+&bam_dmux_dma {
+ status = "okay";
+};
+
+&lpass {
+ status = "reserved"; /* Controlled by QDSP6 */
+};
+
+&lpass_codec {
+ status = "okay";
+};
+
+&mba_mem {
+ status = "okay";
+};
+
+&mpss {
+ status = "okay";
+};
+
+&mpss_mem {
+ status = "okay";
+};
+
+&pm8916_codec {
+ status = "okay";
+};
+
+&q6afedai {
+ dai@16 {
+ reg = ;
+ qcom,sd-lines = <0 1>;
+ };
+ dai@20 {
+ reg = ;
+ qcom,sd-lines = <0 1>;
+ };
+};
+
+&q6asmdai {
+ dai@0 {
+ reg = <0>;
+ direction = ;
+ };
+ dai@1 {
+ reg = <1>;
+ direction = ;
+ };
+ dai@2 {
+ reg = <2>;
+ direction = ;
+ };
+ dai@3 {
+ reg = <3>;
+ direction = ;
+ is-compress-dai;
+ };
+};
+
+&sound {
+ compatible = "qcom,msm8916-qdsp6-sndcard";
+ model = "msm8916";
+
+ pinctrl-0 = <&cdc_pdm_default>;
+ pinctrl-1 = <&cdc_pdm_sleep>;
+ pinctrl-names = "default", "sleep";
+
+ multimedia1-dai-link {
+ link-name = "MultiMedia1";
+ cpu {
+ sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA1>;
+ };
+ };
+
+ multimedia2-dai-link {
+ link-name = "MultiMedia2";
+ cpu {
+ sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA2>;
+ };
+ };
+
+ multimedia3-dai-link {
+ link-name = "MultiMedia3";
+ cpu {
+ sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA3>;
+ };
+ };
+
+ multimedia4-dai-link {
+ link-name = "MultiMedia4";
+ cpu {
+ sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA4>;
+ };
+ };
+
+ sound_dai_primary: mi2s-primary-dai-link {
+ link-name = "Primary MI2S";
+ cpu {
+ sound-dai = <&q6afedai PRIMARY_MI2S_RX>;
+ };
+ platform {
+ sound-dai = <&q6routing>;
+ };
+ codec {
+ sound-dai = <&lpass_codec 0>, <&pm8916_codec 0>;
+ };
+ };
+
+ sound_dai_secondary: mi2s-secondary-dai-link {
+ link-name = "Secondary MI2S";
+ status = "disabled"; /* Needs extra codec configuration */
+ cpu {
+ sound-dai = <&q6afedai SECONDARY_MI2S_RX>;
+ };
+ platform {
+ sound-dai = <&q6routing>;
+ };
+ };
+
+ sound_dai_tertiary: mi2s-tertiary-dai-link {
+ link-name = "Tertiary MI2S";
+ cpu {
+ sound-dai = <&q6afedai TERTIARY_MI2S_TX>;
+ };
+ platform {
+ sound-dai = <&q6routing>;
+ };
+ codec {
+ sound-dai = <&lpass_codec 1>, <&pm8916_codec 1>;
+ };
+ };
+
+ sound_dai_quaternary: mi2s-quaternary-dai-link {
+ link-name = "Quaternary MI2S";
+ status = "disabled"; /* Needs extra codec configuration */
+ cpu {
+ sound-dai = <&q6afedai QUATERNARY_MI2S_RX>;
+ };
+ platfor
[PATCH 08/13] arm64: dts: qcom: msm8916-asus-z00l: Add sound and modem
From: "J.R. Divya Antony"
Enable sound and modem for the ASUS Zenfone 2 Laser. The setup is
similar to most MSM8916 devices, i.e.:
- QDSP6 audio
- Speakear/earpiece/headphones/microphones via digital/analog codec
in MSM8916/PM8916
- Audio jack detection via analog codec in PM8916
- WWAN Internet via BAM-DMUX
Signed-off-by: J.R. Divya Antony
[Stephan: rebase and simplify, add consistent commit message]
Signed-off-by: Stephan Gerhold
---
arch/arm64/boot/dts/qcom/msm8916-asus-z00l.dts | 22 ++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-asus-z00l.dts
b/arch/arm64/boot/dts/qcom/msm8916-asus-z00l.dts
index a8be6ff66893..ff8eb0a94795 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-asus-z00l.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-asus-z00l.dts
@@ -3,6 +3,8 @@
/dts-v1/;
#include "msm8916-pm8916.dtsi"
+#include "msm8916-modem-qdsp6.dtsi"
+
#include
#include
#include
@@ -130,6 +132,18 @@ &blsp_uart2 {
status = "okay";
};
+&mpss_mem {
+ reg = <0x0 0x8680 0x0 0x550>;
+};
+
+&pm8916_codec {
+ qcom,micbias-lvl = <2800>;
+ qcom,mbhc-vthreshold-low = <75 150 237 450 500>;
+ qcom,mbhc-vthreshold-high = <75 150 237 450 500>;
+ qcom,micbias1-ext-cap;
+ qcom,hphl-jack-type-normally-open;
+};
+
&pm8916_rpm_regulators {
pm8916_l17: l17 {
regulator-min-microvolt = <285>;
@@ -151,6 +165,14 @@ &sdhc_2 {
cd-gpios = <&tlmm 38 GPIO_ACTIVE_LOW>;
};
+&sound {
+ status = "okay";
+ audio-routing =
+ "AMIC1", "MIC BIAS External1",
+ "AMIC2", "MIC BIAS Internal2",
+ "AMIC3", "MIC BIAS External1";
+};
+
&usb {
status = "okay";
extcon = <&usb_id>, <&usb_id>;
--
2.42.0
[PATCH 06/13] arm64: dts: qcom: msm8916-wingtech-wt88047: Add sound and modem
Enable sound and modem for the Xiaomi Redmi 2. The setup
is similar to most MSM8916 devices, i.e.:
- QDSP6 audio
- Earpiece/headphones/microphones via digital/analog codec in
MSM8916/PM8916
- Audio jack detection via analog codec in PM8916
- WWAN Internet via BAM-DMUX
except:
- Speaker amplifier is connected to HPH_R (headphones) output of the
analog codec. There is a separate analog switch that allows disabling
playback via the headphone jack.
Signed-off-by: Stephan Gerhold
---
.../boot/dts/qcom/msm8916-wingtech-wt88047.dts | 76 ++
1 file changed, 76 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-wingtech-wt88047.dts
b/arch/arm64/boot/dts/qcom/msm8916-wingtech-wt88047.dts
index 419f35c1fc92..600c225a2568 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-wingtech-wt88047.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-wingtech-wt88047.dts
@@ -6,6 +6,8 @@
/dts-v1/;
#include "msm8916-pm8916.dtsi"
+#include "msm8916-modem-qdsp6.dtsi"
+
#include
#include
#include
@@ -25,6 +27,28 @@ chosen {
stdout-path = "serial0";
};
+ speaker_amp: audio-amplifier {
+ compatible = "simple-audio-amplifier";
+ enable-gpios = <&tlmm 117 GPIO_ACTIVE_HIGH>;
+ sound-name-prefix = "Speaker Amp";
+ pinctrl-0 = <&speaker_amp_default>;
+ pinctrl-names = "default";
+ };
+
+ /*
+* This seems to be actually an analog switch that either routes audio
+* to the headphone jack or nowhere. Given that we need to enable a GPIO
+* to get sound on headphones, modelling it as simple-audio-amplifier
+* works just fine.
+*/
+ headphones_switch: audio-switch {
+ compatible = "simple-audio-amplifier";
+ enable-gpios = <&tlmm 8 GPIO_ACTIVE_HIGH>;
+ sound-name-prefix = "Headphones Switch";
+ pinctrl-0 = <&headphones_switch_default>;
+ pinctrl-names = "default";
+ };
+
flash-led-controller {
compatible = "ocs,ocp8110";
enable-gpios = <&tlmm 31 GPIO_ACTIVE_HIGH>;
@@ -146,6 +170,18 @@ &blsp_uart2 {
status = "okay";
};
+&mpss_mem {
+ reg = <0x0 0x8680 0x0 0x510>;
+};
+
+&pm8916_codec {
+ qcom,micbias1-ext-cap;
+ qcom,micbias-lvl = <2800>;
+ qcom,mbhc-vthreshold-low = <75 100 120 180 500>;
+ qcom,mbhc-vthreshold-high = <75 100 120 180 500>;
+ qcom,hphl-jack-type-normally-open;
+};
+
&pm8916_resin {
status = "okay";
linux,code = ;
@@ -180,6 +216,32 @@ &sdhc_2 {
non-removable;
};
+&sound {
+ status = "okay";
+
+ /*
+* Provide widgets/pin-switches to allow enabling speaker and headphones
+* separately. Both are routed via the HPH_L/HPH_R pins of the codec.
+*/
+ model = "wt88047";
+ widgets =
+ "Speaker", "Speaker",
+ "Headphone", "Headphones";
+ pin-switches = "Speaker", "Headphones";
+ audio-routing =
+ "Speaker", "Speaker Amp OUTL",
+ "Speaker", "Speaker Amp OUTR",
+ "Speaker Amp INL", "HPH_R",
+ "Speaker Amp INR", "HPH_R",
+ "Headphones", "Headphones Switch OUTL",
+ "Headphones", "Headphones Switch OUTR",
+ "Headphones Switch INL", "HPH_L",
+ "Headphones Switch INR", "HPH_R",
+ "AMIC1", "MIC BIAS External1",
+ "AMIC2", "MIC BIAS Internal2";
+ aux-devs = <&speaker_amp>, <&headphones_switch>;
+};
+
&usb {
status = "okay";
extcon = <&usb_id>, <&usb_id>;
@@ -226,6 +288,13 @@ gpio_keys_default: gpio-keys-default-state {
bias-pull-up;
};
+ headphones_switch_default: headphones-switch-default-state {
+ pins = "gpio8";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
imu_default: imu-default-state {
pins = "gpio115";
function = "gpio";
@@ -234,6 +303,13 @@ imu_default: imu-default-state {
bias-disable;
};
+ speaker_amp_default: speaker-amp-default-state {
+ pins = "gpio117";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
touchscreen_default: touchscreen-default-state {
touchscreen-pins {
pins = "gpio13";
--
2.42.0
[PATCH 01/13] arm64: dts: qcom: msm8939: Add BAM-DMUX WWAN
From: Vincent Knecht
BAM DMUX is used as the network interface to the modem. This is copied
as-is from msm8916.dtsi.
Signed-off-by: Vincent Knecht
Signed-off-by: Stephan Gerhold
---
arch/arm64/boot/dts/qcom/msm8939.dtsi | 30 ++
1 file changed, 30 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8939.dtsi
b/arch/arm64/boot/dts/qcom/msm8939.dtsi
index 324b5d26db40..65c68e0e88d5 100644
--- a/arch/arm64/boot/dts/qcom/msm8939.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8939.dtsi
@@ -1537,6 +1537,20 @@ spmi_bus: spmi@200f000 {
#interrupt-cells = <4>;
};
+ bam_dmux_dma: dma-controller@4044000 {
+ compatible = "qcom,bam-v1.7.0";
+ reg = <0x04044000 0x19000>;
+ interrupts = ;
+ #dma-cells = <1>;
+ qcom,ee = <0>;
+
+ num-channels = <6>;
+ qcom,num-ees = <1>;
+ qcom,powered-remotely;
+
+ status = "disabled";
+ };
+
mpss: remoteproc@408 {
compatible = "qcom,msm8916-mss-pil";
reg = <0x0408 0x100>, <0x0402 0x040>;
@@ -1569,6 +1583,22 @@ mpss: remoteproc@408 {
qcom,halt-regs = <&tcsr 0x18000 0x19000 0x1a000>;
status = "disabled";
+ bam_dmux: bam-dmux {
+ compatible = "qcom,bam-dmux";
+
+ interrupt-parent = <&hexagon_smsm>;
+ interrupts = <1 IRQ_TYPE_EDGE_BOTH>, <11
IRQ_TYPE_EDGE_BOTH>;
+ interrupt-names = "pc", "pc-ack";
+
+ qcom,smem-states = <&apps_smsm 1>, <&apps_smsm
11>;
+ qcom,smem-state-names = "pc", "pc-ack";
+
+ dmas = <&bam_dmux_dma 4>, <&bam_dmux_dma 5>;
+ dma-names = "tx", "rx";
+
+ status = "disabled";
+ };
+
mba {
memory-region = <&mba_mem>;
};
--
2.42.0
[PATCH 00/13] arm64: dts: qcom: msm8916/39: Enable sound and modem with QDSP6
Enable sound and modem on most of the MSM8916/MSM8939 smartphones/tablets supported upstream by: - Adding the BAM-DMUX DT nodes to msm8939.dtsi for WWAN Internet - Adding the QDSP6 DT nodes to both msm8916.dtsi and msm8939.dtsi. This is needed because audio must be routed through the QDSP6 services provided by the modem firmware when the modem is active. - Setting up the sound/codec related nodes for all the devices. Most of the sound/modem setup is very similar on all MSM8916/MSM8939 devices because the device-specific details are abstracted by the modem firmware. Reduce duplication by adding "msm8916-modem-qdsp6.dtsi" which contains most of the common definitions. The board-specific DT part is limited to extra codecs or board-specific sound setup. With this patch set, the following functionality works on most MSM8916/MSM8939 devices supported upstream: - Sound: Speaker/earpiece/headphones/microphones - Modem: Calls, SMS, WWAN Internet (e.g. with ModemManager) And with extra pending patches also: - Voice call audio - GPS These patches have been contributed by different people and have been used/tested in postmarketOS for several years. Until now they had to wait for other changes to be upstreamed (QDSP6 audio support for MSM8916, dynamic reserved memory, ...). Signed-off-by: Stephan Gerhold --- J.R. Divya Antony (1): arm64: dts: qcom: msm8916-asus-z00l: Add sound and modem Jasper Korten (1): arm64: dts: qcom: msm8916-samsung-gt5: Add sound and modem Jonathan Albrieux (1): arm64: dts: qcom: msm8916-longcheer-l8910: Add sound and modem Lin, Meng-Bo (2): arm64: dts: qcom: msm8916-samsung-j5: Add sound and modem arm64: dts: qcom: msm8939-samsung-a7: Add sound and modem Nikita Travkin (1): arm64: dts: qcom: msm8916-longcheer-l8150: Add sound and modem Stephan Gerhold (5): arm64: dts: qcom: msm8916/39: Add QDSP6 arm64: dts: qcom: msm8916: Add common msm8916-modem-qdsp6.dtsi arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem arm64: dts: qcom: msm8916-samsung-serranove: Add sound and modem arm64: dts: qcom: msm8916-wingtech-wt88047: Add sound and modem Vincent Knecht (2): arm64: dts: qcom: msm8939: Add BAM-DMUX WWAN arm64: dts: qcom: msm8916-alcatel-idol347: Add sound and modem .../boot/dts/qcom/msm8916-alcatel-idol347.dts | 164 + arch/arm64/boot/dts/qcom/msm8916-asus-z00l.dts | 22 +++ .../boot/dts/qcom/msm8916-longcheer-l8150.dts | 32 +++- .../boot/dts/qcom/msm8916-longcheer-l8910.dts | 54 +++ arch/arm64/boot/dts/qcom/msm8916-modem-qdsp6.dtsi | 163 .../dts/qcom/msm8916-samsung-a2015-common.dtsi | 55 +++ .../dts/qcom/msm8916-samsung-e2015-common.dtsi | 4 + .../boot/dts/qcom/msm8916-samsung-grandmax.dts | 4 + .../boot/dts/qcom/msm8916-samsung-gt5-common.dtsi | 36 + arch/arm64/boot/dts/qcom/msm8916-samsung-gt510.dts | 23 +++ arch/arm64/boot/dts/qcom/msm8916-samsung-gt58.dts | 43 ++ .../boot/dts/qcom/msm8916-samsung-j5-common.dtsi | 15 ++ arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts| 4 + .../boot/dts/qcom/msm8916-samsung-serranove.dts| 14 ++ .../boot/dts/qcom/msm8916-wingtech-wt88047.dts | 76 ++ arch/arm64/boot/dts/qcom/msm8916.dtsi | 49 ++ arch/arm64/boot/dts/qcom/msm8939-samsung-a7.dts| 54 +++ arch/arm64/boot/dts/qcom/msm8939.dtsi | 79 ++ 18 files changed, 888 insertions(+), 3 deletions(-) --- change-id: 20230922-msm8916-modem-0d8b6c8abf76 Best regards, -- Stephan Gerhold
[PATCH] dt-bindings: remoteproc: mtk,scp: Add missing additionalProperties on child node schemas
Just as unevaluatedProperties or additionalProperties are required at the top level of schemas, they should (and will) also be required for child node schemas. That ensures only documented properties are present for any node. Signed-off-by: Rob Herring --- Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml index 895415772d1d..24422fd56e83 100644 --- a/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml +++ b/Documentation/devicetree/bindings/remoteproc/mtk,scp.yaml @@ -91,6 +91,7 @@ allOf: additionalProperties: type: object + additionalProperties: false description: Subnodes of the SCP represent rpmsg devices. The names of the devices are not important. The properties of these nodes are defined by the -- 2.40.1
Re: [PATCH] [v2] ieee802154: ca8210: Fix a potential UAF in ca8210_probe
Hi,
On Tue, Sep 26, 2023 at 4:29 AM Stefan Schmidt
wrote:
>
> Hello.
>
> On 26.09.23 10:02, Miquel Raynal wrote:
> > Hi Dinghao,
> >
> > dinghao@zju.edu.cn wrote on Tue, 26 Sep 2023 11:22:44 +0800:
> >
> >> If of_clk_add_provider() fails in ca8210_register_ext_clock(),
> >> it calls clk_unregister() to release priv->clk and returns an
> >> error. However, the caller ca8210_probe() then calls ca8210_remove(),
> >> where priv->clk is freed again in ca8210_unregister_ext_clock(). In
> >> this case, a use-after-free may happen in the second time we call
> >> clk_unregister().
> >>
> >> Fix this by removing the first clk_unregister(). Also, priv->clk could
> >> be an error code on failure of clk_register_fixed_rate(). Use
> >> IS_ERR_OR_NULL to catch this case in ca8210_unregister_ext_clock().
> >>
> >> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
> >
> > Missing Cc stable, this needs to be backported.
> >
> >> Signed-off-by: Dinghao Liu
> >> ---
> >>
> >> Changelog:
> >>
> >> v2: -Remove the first clk_unregister() instead of nulling priv->clk.
> >> ---
> >> drivers/net/ieee802154/ca8210.c | 3 +--
> >> 1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/ieee802154/ca8210.c
> >> b/drivers/net/ieee802154/ca8210.c
> >> index aebb19f1b3a4..b35c6f59bd1a 100644
> >> --- a/drivers/net/ieee802154/ca8210.c
> >> +++ b/drivers/net/ieee802154/ca8210.c
> >> @@ -2759,7 +2759,6 @@ static int ca8210_register_ext_clock(struct
> >> spi_device *spi)
> >> }
> >> ret = of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
> >> if (ret) {
> >> -clk_unregister(priv->clk);
> >> dev_crit(
> >> &spi->dev,
> >> "Failed to register external clock as clock
> >> provider\n"
> >
> > I was hoping you would simplify this function a bit more.
> >
> >> @@ -2780,7 +2779,7 @@ static void ca8210_unregister_ext_clock(struct
> >> spi_device *spi)
> >> {
> >> struct ca8210_priv *priv = spi_get_drvdata(spi);
> >>
> >> -if (!priv->clk)
> >> +if (IS_ERR_OR_NULL(priv->clk))
> >> return
> >>
> >> of_clk_del_provider(spi->dev.of_node);
> >
> > Alex, Stefan, who handles wpan and wpan/next this release?
>
> IIRC it would be me for wpan and Alex for wpan-next.
That's okay for me.
- Alex
[PATCH] 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
---
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;
}
-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;
list_for_each_entry(file, &tr->events, list) {
if (file->system != dir)
continue;
- __remove_filter(file);
+ i += __remove_filter(file);
}
+ 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;
+
remove_filter_string(system->filter);
filter = system->filter;
system->filter = NULL;
--
2.25.1
RE: [PATCH v4] vmscan: add trace events for lru_gen
>>>On Mon, Sep 25, 2023 at 10:20?PM Jaewon Kim wrote:
As the legacy lru provides, the lru_gen needs some trace events for
debugging.
This commit introduces 2 trace events.
trace_mm_vmscan_lru_gen_scan
trace_mm_vmscan_lru_gen_evict
Each event is similar to the following legacy events.
trace_mm_vmscan_lru_isolate,
trace_mm_vmscan_lru_shrink_[in]active
>>>
>>>We should just reuse trace_mm_vmscan_lru_isolate and
>>>trace_mm_vmscan_lru_shrink_inactive instead of adding new tracepoints.
>>>
>>>To reuse trace_mm_vmscan_lru_isolate, we'd just need to append two new
>>>names to LRU_NAMES.
>>>
>>>The naming of trace_mm_vmscan_lru_shrink_inactive might seem confusing
>>>but it's how MGLRU maintains the compatibility, e.g., the existing
>>>active/inactive counters in /proc/vmstat.
>>
>>
>>Hello
>>
>>Actually I had tried to reuse them. But some value was not that compatible.
>>Let me try that way again.
>>
>>>
>
>Hello Yu Zhao
>
>Could you look into what I tried below? I reused the legacy trace events as
>you recommened.
>
>For the nr_scanned for trace_mm_vmscan_lru_shrink_inactive, I just used the
>scanned returned from isolate_folios.
>I thought this is right as scan_folios also uses its isolated.
> __count_vm_events(PGSCAN_ANON + type, isolated);
>But I guess the scanned in scan_folios is actually the one used in
>shrink_inactive_list
please ignore nr_scanned thing above I just misread the code.
This is an example, I think it works well.
mm_vmscan_lru_isolate: isolate_mode=0 classzone=2 order=0 nr_requested=4096
nr_scanned=64 nr_skipped=0 nr_taken=64 lru=inactive_file
mm_vmscan_lru_shrink_inactive: nid=0 nr_scanned=64 nr_reclaimed=63 nr_dirty=0
nr_writeback=0 nr_congested=0 nr_immediate=0 nr_activate_anon=0
nr_activate_file=1 nr_ref_keep=0 nr_unmap_fail=0 priority=2
flags=RECLAIM_WB_FILE|RECLAIM_WB_ASYNC
>
>I tested this on both 0 and 7 of /sys/kernel/mm/lru_gen/enabled
>
>
>diff --git a/mm/vmscan.c b/mm/vmscan.c
>index a4e44f1c97c1..b61a0156559c 100644
>--- a/mm/vmscan.c
>+++ b/mm/vmscan.c
>@@ -4328,6 +4328,7 @@ static int scan_folios(struct lruvec *lruvec, struct
>scan_control *sc,
>int sorted = 0;
>int scanned = 0;
>int isolated = 0;
>+ int skipped = 0;
>int remaining = MAX_LRU_BATCH;
>struct lru_gen_folio *lrugen = &lruvec->lrugen;
>struct mem_cgroup *memcg = lruvec_memcg(lruvec);
>@@ -4341,7 +4342,7 @@ static int scan_folios(struct lruvec *lruvec, struct
>scan_control *sc,
>
>for (i = MAX_NR_ZONES; i > 0; i--) {
>LIST_HEAD(moved);
>- int skipped = 0;
>+ int skipped_zone = 0;
>int zone = (sc->reclaim_idx + i) % MAX_NR_ZONES;
>struct list_head *head = &lrugen->folios[gen][type][zone];
>
>@@ -4363,16 +4364,17 @@ static int scan_folios(struct lruvec *lruvec, struct
>scan_control *sc,
>isolated += delta;
>} else {
>list_move(&folio->lru, &moved);
>- skipped += delta;
>+ skipped_zone += delta;
>}
>
>- if (!--remaining || max(isolated, skipped) >=
>MIN_LRU_BATCH)
>+ if (!--remaining || max(isolated, skipped_zone) >=
>MIN_LRU_BATCH)
>break;
>}
>
>- if (skipped) {
>+ if (skipped_zone) {
>list_splice(&moved, head);
>- __count_zid_vm_events(PGSCAN_SKIP, zone, skipped);
>+ __count_zid_vm_events(PGSCAN_SKIP, zone, skipped_zone);
>+ skipped += skipped_zone;
>}
>
>if (!remaining || isolated >= MIN_LRU_BATCH)
>@@ -4387,6 +4389,9 @@ static int scan_folios(struct lruvec *lruvec, struct
>scan_control *sc,
>__count_memcg_events(memcg, item, isolated);
>__count_memcg_events(memcg, PGREFILL, sorted);
>__count_vm_events(PGSCAN_ANON + type, isolated);
>+ trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, MAX_LRU_BATCH,
>+ scanned, skipped, isolated,
>+ type ? LRU_INACTIVE_FILE :
>LRU_INACTIVE_ANON);
>
>/*
> * There might not be eligible folios due to reclaim_idx. Check the
>@@ -4517,6 +4522,9 @@ static int evict_folios(struct lruvec *lruvec, struct
>scan_control *sc, int swap
> retry:
>reclaimed = shrink_folio_list(&list, pgdat, sc, &stat, false);
>sc->nr_reclaimed += reclaimed;
>+ trace_mm_vmscan_lru_shrink_inactive(pgdat->node_id,
>+ scanned, reclaimed, &stat, sc->priority,
>+ type ? LRU_INACTIVE_FILE : LRU_INACTIVE_ANON);
>
>list_for_each_entry_safe_reverse(folio, next, &list, lru) {
>if (!foli
Re: SPDX: Appletalk FW license in the kernel
On 9/26/23 04:02, Greg KH wrote: On Tue, Sep 26, 2023 at 12:34:03AM -0700, Christoph Hellwig wrote: On Fri, Sep 15, 2023 at 09:39:05AM -0400, Prarit Bhargava wrote: To be clear, I am not asking for their removal, however, I do think a better license should be issued for these files. The files were trivially modified in 2006. It could be that the code in question is now unused and it is just easier to remove them. Is there anyone you know of that we could approach to determine a proper SPDX License for these files? The code contains firmware that is downloaded to the device. The proper thing would be to convert them to separate binary files in the linux-firmware packages. But given that the driver has seen nothing but tree wide cleanups since the dawn of git I suspect there is no maintainer and probably no user left. The best might be to indeed just remove it and see if anyone screams, in which case we could bring it back after doing the above. We should just remove them for now, I have no objection to that at all. Want me to send the patch? Yes, that would be appreciated. Thanks :) P. thanks, greg k-h
Re: [PATCH v5 01/18] cgroup/misc: Add per resource callbacks for CSS events
On Tue Sep 26, 2023 at 4:10 PM EEST, Jarkko Sakkinen wrote:
> On Tue Sep 26, 2023 at 6:04 AM EEST, Haitao Huang wrote:
> > Hi Jarkko
> >
> > On Mon, 25 Sep 2023 12:09:21 -0500, Jarkko Sakkinen
> > wrote:
> >
> > > On Sat Sep 23, 2023 at 6:06 AM EEST, Haitao Huang wrote:
> > >> From: Kristen Carlson Accardi
> > >>
> > >> The misc cgroup controller (subsystem) currently does not perform
> > >> resource type specific action for Cgroups Subsystem State (CSS) events:
> > >> the 'css_alloc' event when a cgroup is created and the 'css_free' event
> > >> when a cgroup is destroyed, or in event of user writing the max value to
> > >> the misc.max file to set the usage limit of a specific resource
> > >> [admin-guide/cgroup-v2.rst, 5-9. Misc].
> > >>
> > >> Define callbacks for those events and allow resource providers to
> > >> register the callbacks per resource type as needed. This will be
> > >> utilized later by the EPC misc cgroup support implemented in the SGX
> > >> driver:
> > >> - On css_alloc, allocate and initialize necessary structures for EPC
> > >> reclaiming, e.g., LRU list, work queue, etc.
> > >> - On css_free, cleanup and free those structures created in alloc.
> > >> - On max_write, trigger EPC reclaiming if the new limit is at or below
> > >> current usage.
> > >>
> > >> Signed-off-by: Kristen Carlson Accardi
> > >> Signed-off-by: Haitao Huang
> > >> ---
> > >> V5:
> > >> - Remove prefixes from the callback names (tj)
> > >> - Update commit message (Jarkko)
> > >>
> > >> V4:
> > >> - Moved this to the front of the series.
> > >> - Applies on cgroup/for-6.6 with the overflow fix for misc.
> > >>
> > >> V3:
> > >> - Removed the released() callback
> > >> ---
> > >> include/linux/misc_cgroup.h | 5 +
> > >> kernel/cgroup/misc.c| 32 +---
> > >> 2 files changed, 34 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/include/linux/misc_cgroup.h b/include/linux/misc_cgroup.h
> > >> index e799b1f8d05b..96a88822815a 100644
> > >> --- a/include/linux/misc_cgroup.h
> > >> +++ b/include/linux/misc_cgroup.h
> > >> @@ -37,6 +37,11 @@ struct misc_res {
> > >> u64 max;
> > >> atomic64_t usage;
> > >> atomic64_t events;
> > >> +
> > >> +/* per resource callback ops */
> > >> +int (*alloc)(struct misc_cg *cg);
> > >> +void (*free)(struct misc_cg *cg);
> > >> +void (*max_write)(struct misc_cg *cg);
> > >> };
> > >>
> > >> /**
> > >> diff --git a/kernel/cgroup/misc.c b/kernel/cgroup/misc.c
> > >> index 79a3717a5803..62c9198dee21 100644
> > >> --- a/kernel/cgroup/misc.c
> > >> +++ b/kernel/cgroup/misc.c
> > >> @@ -276,10 +276,13 @@ static ssize_t misc_cg_max_write(struct
> > >> kernfs_open_file *of, char *buf,
> > >>
> > >> cg = css_misc(of_css(of));
> > >>
> > >> -if (READ_ONCE(misc_res_capacity[type]))
> > >> +if (READ_ONCE(misc_res_capacity[type])) {
> > >> WRITE_ONCE(cg->res[type].max, max);
> > >> -else
> > >> +if (cg->res[type].max_write)
> > >> +cg->res[type].max_write(cg);
> > >> +} else {
> > >> ret = -EINVAL;
> > >> +}
> > >>
> > >> return ret ? ret : nbytes;
> > >> }
> > >> @@ -383,23 +386,39 @@ static struct cftype misc_cg_files[] = {
> > >> static struct cgroup_subsys_state *
> > >> misc_cg_alloc(struct cgroup_subsys_state *parent_css)
> > >> {
> > >> +struct misc_cg *parent_cg;
> > >> enum misc_res_type i;
> > >> struct misc_cg *cg;
> > >> +int ret;
> > >>
> > >> if (!parent_css) {
> > >> cg = &root_cg;
> > >> +parent_cg = &root_cg;
> > >> } else {
> > >> cg = kzalloc(sizeof(*cg), GFP_KERNEL);
> > >> if (!cg)
> > >> return ERR_PTR(-ENOMEM);
> > >> +parent_cg = css_misc(parent_css);
> > >> }
> > >>
> > >> for (i = 0; i < MISC_CG_RES_TYPES; i++) {
> > >> WRITE_ONCE(cg->res[i].max, MAX_NUM);
> > >> atomic64_set(&cg->res[i].usage, 0);
> > >> +if (parent_cg->res[i].alloc) {
> > >> +ret = parent_cg->res[i].alloc(cg);
> > >> +if (ret)
> > >> +goto alloc_err;
> > >> +}
> > >> }
> > >>
> > >> return &cg->css;
> > >> +
> > >> +alloc_err:
> > >> +for (i = 0; i < MISC_CG_RES_TYPES; i++)
> > >> +if (parent_cg->res[i].free)
> > >> +cg->res[i].free(cg);
> > >> +kfree(cg);
> > >> +return ERR_PTR(ret);
> > >> }
> > >>
> > >> /**
> > >> @@ -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_c
Re: [PATCH v5 01/18] cgroup/misc: Add per resource callbacks for CSS events
On Tue Sep 26, 2023 at 6:04 AM EEST, Haitao Huang wrote:
> Hi Jarkko
>
> On Mon, 25 Sep 2023 12:09:21 -0500, Jarkko Sakkinen
> wrote:
>
> > On Sat Sep 23, 2023 at 6:06 AM EEST, Haitao Huang wrote:
> >> From: Kristen Carlson Accardi
> >>
> >> The misc cgroup controller (subsystem) currently does not perform
> >> resource type specific action for Cgroups Subsystem State (CSS) events:
> >> the 'css_alloc' event when a cgroup is created and the 'css_free' event
> >> when a cgroup is destroyed, or in event of user writing the max value to
> >> the misc.max file to set the usage limit of a specific resource
> >> [admin-guide/cgroup-v2.rst, 5-9. Misc].
> >>
> >> Define callbacks for those events and allow resource providers to
> >> register the callbacks per resource type as needed. This will be
> >> utilized later by the EPC misc cgroup support implemented in the SGX
> >> driver:
> >> - On css_alloc, allocate and initialize necessary structures for EPC
> >> reclaiming, e.g., LRU list, work queue, etc.
> >> - On css_free, cleanup and free those structures created in alloc.
> >> - On max_write, trigger EPC reclaiming if the new limit is at or below
> >> current usage.
> >>
> >> Signed-off-by: Kristen Carlson Accardi
> >> Signed-off-by: Haitao Huang
> >> ---
> >> V5:
> >> - Remove prefixes from the callback names (tj)
> >> - Update commit message (Jarkko)
> >>
> >> V4:
> >> - Moved this to the front of the series.
> >> - Applies on cgroup/for-6.6 with the overflow fix for misc.
> >>
> >> V3:
> >> - Removed the released() callback
> >> ---
> >> include/linux/misc_cgroup.h | 5 +
> >> kernel/cgroup/misc.c| 32 +---
> >> 2 files changed, 34 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/include/linux/misc_cgroup.h b/include/linux/misc_cgroup.h
> >> index e799b1f8d05b..96a88822815a 100644
> >> --- a/include/linux/misc_cgroup.h
> >> +++ b/include/linux/misc_cgroup.h
> >> @@ -37,6 +37,11 @@ struct misc_res {
> >>u64 max;
> >>atomic64_t usage;
> >>atomic64_t events;
> >> +
> >> + /* per resource callback ops */
> >> + int (*alloc)(struct misc_cg *cg);
> >> + void (*free)(struct misc_cg *cg);
> >> + void (*max_write)(struct misc_cg *cg);
> >> };
> >>
> >> /**
> >> diff --git a/kernel/cgroup/misc.c b/kernel/cgroup/misc.c
> >> index 79a3717a5803..62c9198dee21 100644
> >> --- a/kernel/cgroup/misc.c
> >> +++ b/kernel/cgroup/misc.c
> >> @@ -276,10 +276,13 @@ static ssize_t misc_cg_max_write(struct
> >> kernfs_open_file *of, char *buf,
> >>
> >>cg = css_misc(of_css(of));
> >>
> >> - if (READ_ONCE(misc_res_capacity[type]))
> >> + if (READ_ONCE(misc_res_capacity[type])) {
> >>WRITE_ONCE(cg->res[type].max, max);
> >> - else
> >> + if (cg->res[type].max_write)
> >> + cg->res[type].max_write(cg);
> >> + } else {
> >>ret = -EINVAL;
> >> + }
> >>
> >>return ret ? ret : nbytes;
> >> }
> >> @@ -383,23 +386,39 @@ static struct cftype misc_cg_files[] = {
> >> static struct cgroup_subsys_state *
> >> misc_cg_alloc(struct cgroup_subsys_state *parent_css)
> >> {
> >> + struct misc_cg *parent_cg;
> >>enum misc_res_type i;
> >>struct misc_cg *cg;
> >> + int ret;
> >>
> >>if (!parent_css) {
> >>cg = &root_cg;
> >> + parent_cg = &root_cg;
> >>} else {
> >>cg = kzalloc(sizeof(*cg), GFP_KERNEL);
> >>if (!cg)
> >>return ERR_PTR(-ENOMEM);
> >> + parent_cg = css_misc(parent_css);
> >>}
> >>
> >>for (i = 0; i < MISC_CG_RES_TYPES; i++) {
> >>WRITE_ONCE(cg->res[i].max, MAX_NUM);
> >>atomic64_set(&cg->res[i].usage, 0);
> >> + if (parent_cg->res[i].alloc) {
> >> + ret = parent_cg->res[i].alloc(cg);
> >> + if (ret)
> >> + goto alloc_err;
> >> + }
> >>}
> >>
> >>return &cg->css;
> >> +
> >> +alloc_err:
> >> + for (i = 0; i < MISC_CG_RES_TYPES; i++)
> >> + if (parent_cg->res[i].free)
> >> + cg->res[i].free(cg);
> >> + kfree(cg);
> >> + return ERR_PTR(ret);
> >> }
> >>
> >> /**
> >> @@ -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
>
Re: [PATCH] [v2] ieee802154: ca8210: Fix a potential UAF in ca8210_probe
> Missing Cc stable, this needs to be backported.
I will cc stable (sta...@vger.kernel.org) for the next version, thanks!
> > diff --git a/drivers/net/ieee802154/ca8210.c
> > b/drivers/net/ieee802154/ca8210.c
> > index aebb19f1b3a4..b35c6f59bd1a 100644
> > --- a/drivers/net/ieee802154/ca8210.c
> > +++ b/drivers/net/ieee802154/ca8210.c
> > @@ -2759,7 +2759,6 @@ static int ca8210_register_ext_clock(struct
> > spi_device *spi)
> > }
> > ret = of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
> > if (ret) {
> > - clk_unregister(priv->clk);
> > dev_crit(
> > &spi->dev,
> > "Failed to register external clock as clock provider\n"
>
> I was hoping you would simplify this function a bit more.
I understand. In the next patch version, I will just return
of_clk_add_provider().
>
> > @@ -2780,7 +2779,7 @@ static void ca8210_unregister_ext_clock(struct
> > spi_device *spi)
> > {
> > struct ca8210_priv *priv = spi_get_drvdata(spi);
> >
> > - if (!priv->clk)
> > + if (IS_ERR_OR_NULL(priv->clk))
> > return
> >
> > of_clk_del_provider(spi->dev.of_node);
>
> Alex, Stefan, who handles wpan and wpan/next this release?
>
Is there any problem I need to handle in the next patch?
Regards,
Dinghao
Re: [PATCH v3 1/1] scripts: Add add-maintainer.py
On Sat, Aug 26, 2023 at 01:07:42AM -0700, Guru Das Srinagesh wrote:
> +def gather_maintainers_of_file(patch_file):
> +all_entities_of_patch = dict()
> +
> +# Run get_maintainer.pl on patch file
> +logging.info("GET: Patch: {}".format(os.path.basename(patch_file)))
> +cmd = ['scripts/get_maintainer.pl']
> +cmd.extend([patch_file])
> +
> +try:
> +p = subprocess.run(cmd, stdout=subprocess.PIPE, check=True)
> +except:
> +sys.exit(1)
> +
> +logging.debug("\n{}".format(p.stdout.decode()))
> +
> +entries = p.stdout.decode().splitlines()
> +
> +maintainers = []
> +lists = []
> +others = []
> +
> +for entry in entries:
> +entity = entry.split('(')[0].strip()
> +if any(role in entry for role in ["maintainer", "reviewer"]):
> +maintainers.append(entity)
> +elif "list" in entry:
> +lists.append(entity)
> +else:
> +others.append(entity)
> +
> +all_entities_of_patch["maintainers"] = set(maintainers)
> +all_entities_of_patch["lists"] = set(lists)
> +all_entities_of_patch["others"] = set(others)
> +
> +return all_entities_of_patch
> +
FYI, there are couple of issues found while playing around.
- Some entries in MAINTAINERS could be "supporter"
- When names contain ("company"), the script fails to extract name.
Thanks,
Pavan
diff --git a/scripts/add-maintainer.py b/scripts/add-maintainer.py
index 5a5cc9482b06..6aa5e7941172 100755
--- a/scripts/add-maintainer.py
+++ b/scripts/add-maintainer.py
@@ -29,8 +29,8 @@ def gather_maintainers_of_file(patch_file):
others = []
for entry in entries:
-entity = entry.split('(')[0].strip()
-if any(role in entry for role in ["maintainer", "reviewer"]):
+entity = entry.rsplit('(', 1)[0].strip()
+if any(role in entry for role in ["maintainer", "reviewer",
"supporter"]):
maintainers.append(entity)
elif "list" in entry:
lists.append(entity)
Thanks,
Pavan
Re: [PATCH v3 10/13] arch: make execmem setup available regardless of CONFIG_MODULES
Hi Arnd,
On Tue, Sep 26, 2023 at 09:33:48AM +0200, Arnd Bergmann wrote:
> On Mon, Sep 18, 2023, at 09:29, Mike Rapoport wrote:
> > index a42e4cd11db2..c0b536e398b4 100644
> > --- a/arch/arm/mm/init.c
> > +++ b/arch/arm/mm/init.c
> > +#ifdef CONFIG_XIP_KERNEL
> > +/*
> > + * The XIP kernel text is mapped in the module area for modules and
> > + * some other stuff to work without any indirect relocations.
> > + * MODULES_VADDR is redefined here and not in asm/memory.h to avoid
> > + * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned
> > on/off.
> > + */
> > +#undef MODULES_VADDR
> > +#define MODULES_VADDR (((unsigned long)_exiprom + ~PMD_MASK) &
> > PMD_MASK)
> > +#endif
> > +
> > +#if defined(CONFIG_MMU) && defined(CONFIG_EXECMEM)
> > +static struct execmem_params execmem_params __ro_after_init = {
> > + .ranges = {
> > + [EXECMEM_DEFAULT] = {
> > + .start = MODULES_VADDR,
> > + .end = MODULES_END,
> > + .alignment = 1,
> > + },
>
> This causes a randconfig build failure for me on linux-next now:
>
> arch/arm/mm/init.c:499:25: error: initializer element is not constant
> 499 | #define MODULES_VADDR (((unsigned long)_exiprom + ~PMD_MASK) &
> PMD_MASK)
> | ^
> arch/arm/mm/init.c:506:34: note: in expansion of macro 'MODULES_VADDR'
> 506 | .start = MODULES_VADDR,
> | ^
> arch/arm/mm/init.c:499:25: note: (near initialization for
> 'execmem_params.ranges[0].start')
> 499 | #define MODULES_VADDR (((unsigned long)_exiprom + ~PMD_MASK) &
> PMD_MASK)
> | ^
> arch/arm/mm/init.c:506:34: note: in expansion of macro 'MODULES_VADDR'
> 506 | .start = MODULES_VADDR,
> | ^
>
> I have not done any analysis on the issue so far, I hope
> you can see the problem directly. See
> https://pastebin.com/raw/xVqAyakH for a .config that runs into
> this problem with gcc-13.2.0.
The first patch that breaks XIP build is rather patch 04/13, currently
commit 52a34d45419f ("mm/execmem, arch: convert remaining overrides of
module_alloc to execmem") in mm.git/mm-unstable.
The hunk below is a fix for that and the attached patch is the updated
version of 835bc9685f45 ("arch: make execmem setup available regardless of
CONFIG_MODULES")
Andrew, please let me know if you'd like to me to resend these differently.
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index 2c7651a2d84c..096cc1ead635 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -38,8 +38,6 @@
static struct execmem_params execmem_params __ro_after_init = {
.ranges = {
[EXECMEM_DEFAULT] = {
- .start = MODULES_VADDR,
- .end = MODULES_END,
.alignment = 1,
},
},
@@ -49,6 +47,8 @@ struct execmem_params __init *execmem_arch_params(void)
{
struct execmem_range *r = &execmem_params.ranges[EXECMEM_DEFAULT];
+ r->start = MODULES_VADDR;
+ r->end = MODULES_END;
r->pgprot = PAGE_KERNEL_EXEC;
if (IS_ENABLED(CONFIG_ARM_MODULE_PLTS)) {
>
> Arnd
--
Sincerely yours,
Mike.
>From a2dae5a88d172d54e7f074799a286faedd2cdb6a Mon Sep 17 00:00:00 2001
From: "Mike Rapoport (IBM)"
Date: Wed, 31 May 2023 14:58:24 +0300
Subject: [PATCH] arch: make execmem setup available regardless of
CONFIG_MODULES
execmem does not depend on modules, on the contrary modules use
execmem.
To make execmem available when CONFIG_MODULES=n, for instance for
kprobes, split execmem_params initialization out from
arch/kernel/module.c and compile it when CONFIG_EXECMEM=y
Signed-off-by: Mike Rapoport (IBM)
---
arch/arm/kernel/module.c | 38 --
arch/arm/mm/init.c | 38 ++
arch/arm64/kernel/module.c | 130
arch/arm64/mm/init.c | 132 +
arch/loongarch/kernel/module.c | 18 -
arch/loongarch/mm/init.c | 20 +
arch/mips/kernel/module.c | 19 -
arch/mips/mm/init.c| 20 +
arch/parisc/kernel/module.c| 17 -
arch/parisc/mm/init.c | 22 +-
arch/powerpc/kernel/module.c | 60 ---
arch/powerpc/mm/mem.c | 62
arch/riscv/kernel/module.c | 39 --
arch/riscv/mm/init.c | 39 ++
arch/s390/kernel/module.c | 25 ---
arch/s390/mm/init.c| 28 +++
arch/sparc/kernel/module.c | 23 --
arch/sparc/mm/Makefile | 2 +
arch/sparc/mm/execmem.c| 25 +++
arch/x86/kernel/module.c | 27 ---
arch/x86/mm/init.c | 29
21 files changed, 416 insertions(+), 397 deletions(-)
create mode 100644 arch/sparc/mm/exec
Re: [PATCH] [v2] ieee802154: ca8210: Fix a potential UAF in ca8210_probe
Hello.
On 26.09.23 10:02, Miquel Raynal wrote:
Hi Dinghao,
dinghao@zju.edu.cn wrote on Tue, 26 Sep 2023 11:22:44 +0800:
If of_clk_add_provider() fails in ca8210_register_ext_clock(),
it calls clk_unregister() to release priv->clk and returns an
error. However, the caller ca8210_probe() then calls ca8210_remove(),
where priv->clk is freed again in ca8210_unregister_ext_clock(). In
this case, a use-after-free may happen in the second time we call
clk_unregister().
Fix this by removing the first clk_unregister(). Also, priv->clk could
be an error code on failure of clk_register_fixed_rate(). Use
IS_ERR_OR_NULL to catch this case in ca8210_unregister_ext_clock().
Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Missing Cc stable, this needs to be backported.
Signed-off-by: Dinghao Liu
---
Changelog:
v2: -Remove the first clk_unregister() instead of nulling priv->clk.
---
drivers/net/ieee802154/ca8210.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index aebb19f1b3a4..b35c6f59bd1a 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -2759,7 +2759,6 @@ static int ca8210_register_ext_clock(struct spi_device
*spi)
}
ret = of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
if (ret) {
- clk_unregister(priv->clk);
dev_crit(
&spi->dev,
"Failed to register external clock as clock provider\n"
I was hoping you would simplify this function a bit more.
@@ -2780,7 +2779,7 @@ static void ca8210_unregister_ext_clock(struct spi_device
*spi)
{
struct ca8210_priv *priv = spi_get_drvdata(spi);
- if (!priv->clk)
+ if (IS_ERR_OR_NULL(priv->clk))
return
of_clk_del_provider(spi->dev.of_node);
Alex, Stefan, who handles wpan and wpan/next this release?
IIRC it would be me for wpan and Alex for wpan-next.
regards
Stefan Schmidt
Re: [PATCH v3 02/13] mm: introduce execmem_text_alloc() and execmem_free()
On Sat, Sep 23, 2023 at 03:36:01PM -0700, Song Liu wrote:
> On Sat, Sep 23, 2023 at 8:39 AM Mike Rapoport wrote:
> >
> > On Thu, Sep 21, 2023 at 03:34:18PM -0700, Song Liu wrote:
> > > On Mon, Sep 18, 2023 at 12:30 AM Mike Rapoport wrote:
> > > >
> > >
> > > [...]
> > >
> > > > diff --git a/arch/s390/kernel/module.c b/arch/s390/kernel/module.c
> > > > index 42215f9404af..db5561d0c233 100644
> > > > --- a/arch/s390/kernel/module.c
> > > > +++ b/arch/s390/kernel/module.c
> > > > @@ -21,6 +21,7 @@
> > > > #include
> > > > #include
> > > > #include
> > > > +#include
> > > > #include
> > > > #include
> > > > #include
> > > > @@ -76,7 +77,7 @@ void *module_alloc(unsigned long size)
> > > > #ifdef CONFIG_FUNCTION_TRACER
> > > > void module_arch_cleanup(struct module *mod)
> > > > {
> > > > - module_memfree(mod->arch.trampolines_start);
> > > > + execmem_free(mod->arch.trampolines_start);
> > > > }
> > > > #endif
> > > >
> > > > @@ -510,7 +511,7 @@ static int
> > > > module_alloc_ftrace_hotpatch_trampolines(struct module *me,
> > > >
> > > > size = FTRACE_HOTPATCH_TRAMPOLINES_SIZE(s->sh_size);
> > > > numpages = DIV_ROUND_UP(size, PAGE_SIZE);
> > > > - start = module_alloc(numpages * PAGE_SIZE);
> > > > + start = execmem_text_alloc(EXECMEM_FTRACE, numpages *
> > > > PAGE_SIZE);
> > >
> > > This should be EXECMEM_MODULE_TEXT?
> >
> > This is an ftrace trampoline, so I think it should be FTRACE type of
> > allocation.
>
> Yeah, I was aware of the ftrace trampoline. My point was, ftrace trampoline
> doesn't seem to have any special requirements. Therefore, it is probably not
> necessary to have a separate type just for it.
Since ftrace trampolines are currently used only on s390 and x86 which
enforce the same range for all executable allocations there are no special
requirements indeed. But I think that explicitly marking these allocations
as FTRACE makes it clearer what are they used for and I don't see downsides
to having a type for FTRACE.
> AFAICT, kprobe, ftrace, and BPF (JIT and trampoline) can share the same
> execmem_type. We may need some work for some archs, but nothing is
> fundamentally different among these.
Using the same type for all generated code implies that all types of the
generated code must live in the same range and I don't think we want to
impose this limitation on architectures.
For example, RISC-V deliberately added a range for BPF code to allow
relative addressing, see commit 7f3631e88ee6 ("riscv, bpf: Provide RISC-V
specific JIT image alloc/free").
> Thanks,
> Song
--
Sincerely yours,
Mike.
Re: SPDX: Appletalk FW license in the kernel
On Tue, Sep 26, 2023 at 12:34:03AM -0700, Christoph Hellwig wrote: > On Fri, Sep 15, 2023 at 09:39:05AM -0400, Prarit Bhargava wrote: > > To be clear, I am not asking for their removal, however, I do think a better > > license should be issued for these files. The files were trivially modified > > in 2006. It could be that the code in question is now unused and it is just > > easier to remove them. > > > > Is there anyone you know of that we could approach to determine a proper > > SPDX License for these files? > > The code contains firmware that is downloaded to the device. The proper > thing would be to convert them to separate binary files in the > linux-firmware packages. But given that the driver has seen nothing > but tree wide cleanups since the dawn of git I suspect there is no > maintainer and probably no user left. The best might be to indeed just > remove it and see if anyone screams, in which case we could bring it > back after doing the above. > We should just remove them for now, I have no objection to that at all. Want me to send the patch? thanks, greg k-h
Re: [PATCH] [v2] ieee802154: ca8210: Fix a potential UAF in ca8210_probe
Hi Dinghao,
dinghao@zju.edu.cn wrote on Tue, 26 Sep 2023 11:22:44 +0800:
> If of_clk_add_provider() fails in ca8210_register_ext_clock(),
> it calls clk_unregister() to release priv->clk and returns an
> error. However, the caller ca8210_probe() then calls ca8210_remove(),
> where priv->clk is freed again in ca8210_unregister_ext_clock(). In
> this case, a use-after-free may happen in the second time we call
> clk_unregister().
>
> Fix this by removing the first clk_unregister(). Also, priv->clk could
> be an error code on failure of clk_register_fixed_rate(). Use
> IS_ERR_OR_NULL to catch this case in ca8210_unregister_ext_clock().
>
> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Missing Cc stable, this needs to be backported.
> Signed-off-by: Dinghao Liu
> ---
>
> Changelog:
>
> v2: -Remove the first clk_unregister() instead of nulling priv->clk.
> ---
> drivers/net/ieee802154/ca8210.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> index aebb19f1b3a4..b35c6f59bd1a 100644
> --- a/drivers/net/ieee802154/ca8210.c
> +++ b/drivers/net/ieee802154/ca8210.c
> @@ -2759,7 +2759,6 @@ static int ca8210_register_ext_clock(struct spi_device
> *spi)
> }
> ret = of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
> if (ret) {
> - clk_unregister(priv->clk);
> dev_crit(
> &spi->dev,
> "Failed to register external clock as clock provider\n"
I was hoping you would simplify this function a bit more.
> @@ -2780,7 +2779,7 @@ static void ca8210_unregister_ext_clock(struct
> spi_device *spi)
> {
> struct ca8210_priv *priv = spi_get_drvdata(spi);
>
> - if (!priv->clk)
> + if (IS_ERR_OR_NULL(priv->clk))
> return
>
> of_clk_del_provider(spi->dev.of_node);
Alex, Stefan, who handles wpan and wpan/next this release?
Thanks,
Miquèl
Re: [PATCH v3 10/13] arch: make execmem setup available regardless of CONFIG_MODULES
On Mon, Sep 18, 2023, at 09:29, Mike Rapoport wrote:
> index a42e4cd11db2..c0b536e398b4 100644
> --- a/arch/arm/mm/init.c
> +++ b/arch/arm/mm/init.c
> +#ifdef CONFIG_XIP_KERNEL
> +/*
> + * The XIP kernel text is mapped in the module area for modules and
> + * some other stuff to work without any indirect relocations.
> + * MODULES_VADDR is redefined here and not in asm/memory.h to avoid
> + * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned
> on/off.
> + */
> +#undef MODULES_VADDR
> +#define MODULES_VADDR(((unsigned long)_exiprom + ~PMD_MASK) &
> PMD_MASK)
> +#endif
> +
> +#if defined(CONFIG_MMU) && defined(CONFIG_EXECMEM)
> +static struct execmem_params execmem_params __ro_after_init = {
> + .ranges = {
> + [EXECMEM_DEFAULT] = {
> + .start = MODULES_VADDR,
> + .end = MODULES_END,
> + .alignment = 1,
> + },
This causes a randconfig build failure for me on linux-next now:
arch/arm/mm/init.c:499:25: error: initializer element is not constant
499 | #define MODULES_VADDR (((unsigned long)_exiprom + ~PMD_MASK) &
PMD_MASK)
| ^
arch/arm/mm/init.c:506:34: note: in expansion of macro 'MODULES_VADDR'
506 | .start = MODULES_VADDR,
| ^
arch/arm/mm/init.c:499:25: note: (near initialization for
'execmem_params.ranges[0].start')
499 | #define MODULES_VADDR (((unsigned long)_exiprom + ~PMD_MASK) &
PMD_MASK)
| ^
arch/arm/mm/init.c:506:34: note: in expansion of macro 'MODULES_VADDR'
506 | .start = MODULES_VADDR,
| ^
I have not done any analysis on the issue so far, I hope
you can see the problem directly. See
https://pastebin.com/raw/xVqAyakH for a .config that runs into
this problem with gcc-13.2.0.
Arnd
Re: SPDX: Appletalk FW license in the kernel
On Fri, Sep 15, 2023 at 09:39:05AM -0400, Prarit Bhargava wrote: > To be clear, I am not asking for their removal, however, I do think a better > license should be issued for these files. The files were trivially modified > in 2006. It could be that the code in question is now unused and it is just > easier to remove them. > > Is there anyone you know of that we could approach to determine a proper > SPDX License for these files? The code contains firmware that is downloaded to the device. The proper thing would be to convert them to separate binary files in the linux-firmware packages. But given that the driver has seen nothing but tree wide cleanups since the dawn of git I suspect there is no maintainer and probably no user left. The best might be to indeed just remove it and see if anyone screams, in which case we could bring it back after doing the above.
RE: [PATCH v4] vmscan: add trace events for lru_gen
>>On Mon, Sep 25, 2023 at 10:20?PM Jaewon Kim wrote:
>>>
>>> As the legacy lru provides, the lru_gen needs some trace events for
>>> debugging.
>>>
>>> This commit introduces 2 trace events.
>>> trace_mm_vmscan_lru_gen_scan
>>> trace_mm_vmscan_lru_gen_evict
>>>
>>> Each event is similar to the following legacy events.
>>> trace_mm_vmscan_lru_isolate,
>>> trace_mm_vmscan_lru_shrink_[in]active
>>
>>We should just reuse trace_mm_vmscan_lru_isolate and
>>trace_mm_vmscan_lru_shrink_inactive instead of adding new tracepoints.
>>
>>To reuse trace_mm_vmscan_lru_isolate, we'd just need to append two new
>>names to LRU_NAMES.
>>
>>The naming of trace_mm_vmscan_lru_shrink_inactive might seem confusing
>>but it's how MGLRU maintains the compatibility, e.g., the existing
>>active/inactive counters in /proc/vmstat.
>
>
>Hello
>
>Actually I had tried to reuse them. But some value was not that compatible.
>Let me try that way again.
>
>>
Hello Yu Zhao
Could you look into what I tried below? I reused the legacy trace events as you
recommened.
For the nr_scanned for trace_mm_vmscan_lru_shrink_inactive, I just used the
scanned returned from isolate_folios.
I thought this is right as scan_folios also uses its isolated.
__count_vm_events(PGSCAN_ANON + type, isolated);
But I guess the scanned in scan_folios is actually the one used in
shrink_inactive_list
I tested this on both 0 and 7 of /sys/kernel/mm/lru_gen/enabled
diff --git a/mm/vmscan.c b/mm/vmscan.c
index a4e44f1c97c1..b61a0156559c 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4328,6 +4328,7 @@ static int scan_folios(struct lruvec *lruvec, struct
scan_control *sc,
int sorted = 0;
int scanned = 0;
int isolated = 0;
+ int skipped = 0;
int remaining = MAX_LRU_BATCH;
struct lru_gen_folio *lrugen = &lruvec->lrugen;
struct mem_cgroup *memcg = lruvec_memcg(lruvec);
@@ -4341,7 +4342,7 @@ static int scan_folios(struct lruvec *lruvec, struct
scan_control *sc,
for (i = MAX_NR_ZONES; i > 0; i--) {
LIST_HEAD(moved);
- int skipped = 0;
+ int skipped_zone = 0;
int zone = (sc->reclaim_idx + i) % MAX_NR_ZONES;
struct list_head *head = &lrugen->folios[gen][type][zone];
@@ -4363,16 +4364,17 @@ static int scan_folios(struct lruvec *lruvec, struct
scan_control *sc,
isolated += delta;
} else {
list_move(&folio->lru, &moved);
- skipped += delta;
+ skipped_zone += delta;
}
- if (!--remaining || max(isolated, skipped) >=
MIN_LRU_BATCH)
+ if (!--remaining || max(isolated, skipped_zone) >=
MIN_LRU_BATCH)
break;
}
- if (skipped) {
+ if (skipped_zone) {
list_splice(&moved, head);
- __count_zid_vm_events(PGSCAN_SKIP, zone, skipped);
+ __count_zid_vm_events(PGSCAN_SKIP, zone, skipped_zone);
+ skipped += skipped_zone;
}
if (!remaining || isolated >= MIN_LRU_BATCH)
@@ -4387,6 +4389,9 @@ static int scan_folios(struct lruvec *lruvec, struct
scan_control *sc,
__count_memcg_events(memcg, item, isolated);
__count_memcg_events(memcg, PGREFILL, sorted);
__count_vm_events(PGSCAN_ANON + type, isolated);
+ trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, MAX_LRU_BATCH,
+ scanned, skipped, isolated,
+ type ? LRU_INACTIVE_FILE :
LRU_INACTIVE_ANON);
/*
* There might not be eligible folios due to reclaim_idx. Check the
@@ -4517,6 +4522,9 @@ static int evict_folios(struct lruvec *lruvec, struct
scan_control *sc, int swap
retry:
reclaimed = shrink_folio_list(&list, pgdat, sc, &stat, false);
sc->nr_reclaimed += reclaimed;
+ trace_mm_vmscan_lru_shrink_inactive(pgdat->node_id,
+ scanned, reclaimed, &stat, sc->priority,
+ type ? LRU_INACTIVE_FILE : LRU_INACTIVE_ANON);
list_for_each_entry_safe_reverse(folio, next, &list, lru) {
if (!folio_evictable(folio)) {
>>> Here's an example
>>> mm_vmscan_lru_gen_scan: classzone=2 order=0 nr_requested=4096
>>> nr_scanned=64 nr_skipped=0 nr_taken=64 lru=anon
>>> mm_vmscan_lru_gen_evict: nid=0 nr_reclaimed=64 nr_dirty=0 nr_writeback=0
>>> nr_congested=0 nr_immediate=0 nr_activate_anon=0 nr_activate_file=0
>>> nr_ref_keep=0 nr_unmap_fail=0 priority=2
>>> flags=RECLAIM_WB_ANON|RECLAIM_WB_ASYNC
>>> mm_vmscan_lru_gen_scan: classzone=1 order=0 nr_requested=4096
>>> nr_scanned=64 nr_skipped=0 nr_taken=64 lru=file
>>> mm_vmscan_lru_gen_evict: nid=0 nr_reclaimed=64 nr_dirty=0 nr_writebac
Re: droid4 -- weird behaviour when attempting to use usb host
* Pavel Machek [230925 14:36]: > Hi! > > I'm having some fun with usb host. Good news is it works with > externally powered hub... after a while. I get some error messages > about inability to go host mode, but with enough patience it > eventually does enter host mode and I see my keyboard/mouse. > > And usually in that process, one of my cpu cores disappear. top no > longer shows 2 cores, and I was wondering for a while if d4 is > single-core system. It is not, my two cores are back after reboot. > > That's with 6.1.9 kernel from leste. Ideas how to debug this would be > welcome. (Do you use usb host?) You are using a "proper" non-standard usb micro-b cable that grounds the id pin, right? If not, try with one of those as it allows the hardware to do what it's supposed to do. And presumably you don't have a hacked usb hub that feeds back the vbus to your phone, right? If you have, that should not be used as the pmic can feed vbus. Regards, Tony
