* Re: [PATCH 0/3] mm: split the file's i_mmap tree for NUMA
From: Mateusz Guzik @ 2026-04-16 10:29 UTC (permalink / raw)
To: Huang Shijie
Cc: akpm, viro, brauner, linux-mm, linux-kernel, linux-arm-kernel,
linux-fsdevel, muchun.song, osalvador, linux-trace-kernel,
linux-perf-users, linux-parisc, nvdimm, zhongyuan, fangbaoshun,
yingzhiwei
In-Reply-To: <ad4EvoDcAKE2Sl4+@hsj-2U-Workstation>
On Tue, Apr 14, 2026 at 11:11 AM Huang Shijie <huangsj@hygon.cn> wrote:
>
> On Mon, Apr 13, 2026 at 05:33:21PM +0200, Mateusz Guzik wrote:
> > On Mon, Apr 13, 2026 at 02:20:39PM +0800, Huang Shijie wrote:
> > > In NUMA, there are maybe many NUMA nodes and many CPUs.
> > > For example, a Hygon's server has 12 NUMA nodes, and 384 CPUs.
> > > In the UnixBench tests, there is a test "execl" which tests
> > > the execve system call.
> > >
> > > When we test our server with "./Run -c 384 execl",
> > > the test result is not good enough. The i_mmap locks contended heavily on
> > > "libc.so" and "ld.so". For example, the i_mmap tree for "libc.so" can have
> > > over 6000 VMAs, all the VMAs can be in different NUMA mode.
> > > The insert/remove operations do not run quickly enough.
> > >
> > > patch 1 & patch 2 are try to hide the direct access of i_mmap.
> > > patch 3 splits the i_mmap into sibling trees, and we can get better
> > > performance with this patch set:
> > > we can get 77% performance improvement(10 times average)
> > >
> >
> > To my reading you kept the lock as-is and only distributed the protected
> > state.
> >
> > While I don't doubt the improvement, I'm confident should you take a
> > look at the profile you are going to find this still does not scale with
> > rwsem being one of the problems (there are other global locks, some of
> > which have experimental patches for).
> IMHO, when the number of VMAs in the i_mmap is very large, only optimise the rwsem
> lock does not help too much for our NUMA case.
>
> In our NUMA server, the remote access could be the major issue.
>
I'm confused how this is not supposed to help. You moved your data to
be stored per-domain. With my proposal the lock itself will also get
that treatment.
Modulo the issue of what to do with code wanting to iterate the entire
thing, this is blatantly faster.
>
> >
> > Apart from that this does nothing to help high core systems which are
> > all one node, which imo puts another question mark on this specific
> > proposal.
> Yes, this patch set only focus on the NUMA case.
> The one-node case should use the original i_mmap.
>
> Maybe I can add a new config, CONFIG_SPILT_I_MMAP. The config is disabled
> by default, and enabled when the NUMA node is not one.
>
> >
> > Of course one may question whether a RB tree is the right choice here,
> > it may be the lock-protected cost can go way down with merely a better
> > data structure.
> >
> > Regardless of that, for actual scalability, there will be no way around
> > decentralazing locking around this and partitioning per some core count
> > (not just by numa awareness).
> >
> > Decentralizing locking is definitely possible, but I have not looked
> > into specifics of how problematic it is. Best case scenario it will
> > merely with separate locks. Worst case scenario something needs a fully
> > stabilized state for traversal, in that case another rw lock can be
> Yes.
>
> The traversal may need to hold many locks.
>
The very paragraph you partially quoted answers what to do in that
case: wrap everything with a new rwsem taken for reading when
adding/removing entries and taken for writing when iterating the
entire thing. Then the iteration sticks to one lock.
The new rw lock puts an upper ceiling on scalability of the thing, but
it is way higher than the current state.
Given the extra overhead associated with it one could consider
sticking to one centralized state by default and switching to
distributed state if there is enough contention.
> > slapped around this, creating locking order read lock -> per-subset
> > write lock -- this will suffer scalability due to the read locking, but
> > it will still scale drastically better as apart from that there will be
> > no serialization. In this setting the problematic consumer will write
> > lock the new thing to stabilize the state.
> >
> > So my non-maintainer opinion is that the patchset is not worth it as it
> > fails to address anything for significantly more common and already
> > affected setups.
> This patch set is to reduce the remote access latency for insert/remove VMA
> in NUMA.
>
And I am saying the mmap semaphore is a significant problem already on
high-core no-numa setups. Addressing scalability in that case would
sort out the problem in your setup and to a significantly higher
extent.
> >
> > Have you looked into splitting the lock?
> >
> I ever tried.
>
> But there are two disadvantages:
> 1.) The traversal may need to hold many locks which makes the
> code very horrible.
>
I already above this is avoidable.
> 2.) Even we split the locks. Each lock protects a tree, when the tree becomes
> big enough, the VMA insert/remove will also become slow in NUMA.
> The reason is that the tree has VMAs in different NUMA nodes.
>
This is orthogonal to my proposal. In fact, if one is to pretend this
is never a factor with your patch, I would like to point out it will
remain not a factor if the per-numa struct gets its own lock.
^ permalink raw reply
* Re: [PATCH v2 2/3] pwm: rp1: Add RP1 PWM controller driver
From: Andrea della Porta @ 2026-04-16 10:30 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Andrea della Porta, linux-pwm, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, devicetree,
linux-rpi-kernel, linux-arm-kernel, linux-kernel, Naushir Patuck,
Stanimir Varbanov, mbrugger
In-Reply-To: <adkrHkANCzxO8KUP@monoceros>
Hi Uwe,
On 19:31 Fri 10 Apr , Uwe Kleine-König wrote:
> Hello Andrea,
>
> nice work for a v2!
Thanks!
>
> On Fri, Apr 10, 2026 at 04:09:58PM +0200, Andrea della Porta wrote:
<...snip...>
> > +#define RP1_PWM_GLOBAL_CTRL 0x000
> > +#define RP1_PWM_CHANNEL_CTRL(x) (0x014 + ((x) * 0x10))
> > +#define RP1_PWM_RANGE(x) (0x018 + ((x) * 0x10))
> > +#define RP1_PWM_PHASE(x) (0x01C + ((x) * 0x10))
> > +#define RP1_PWM_DUTY(x) (0x020 + ((x) * 0x10))
> > +
> > +/* 8:FIFO_POP_MASK + 0:Trailing edge M/S modulation */
> > +#define RP1_PWM_CHANNEL_DEFAULT (BIT(8) + BIT(0))
>
> Please add a #define for BIT(8) and then use that and
> FIELD_PREP(RP1_PWM_MODE, RP1_PWM_MODE_SOMENICENAME) to define the
> constant. Also I would define it below the register defines.
Ack.
>
> > +#define RP1_PWM_CHANNEL_ENABLE(x) BIT(x)
> > +#define RP1_PWM_POLARITY BIT(3)
> > +#define RP1_PWM_SET_UPDATE BIT(31)
> > +#define RP1_PWM_MODE_MASK GENMASK(1, 0)
>
> s/_MASK// please
>
> It would be great if the bitfield's names started with the register
> name.
Ack.
>
> > +
> > +#define RP1_PWM_NUM_PWMS 4
> > +
> > +struct rp1_pwm {
> > + struct regmap *regmap;
> > + struct clk *clk;
> > + unsigned long clk_rate;
> > + bool clk_enabled;
> > +};
> > +
> > +struct rp1_pwm_waveform {
> > + u32 period_ticks;
> > + u32 duty_ticks;
> > + bool enabled;
> > + bool inverted_polarity;
> > +};
> > +
> > +static const struct regmap_config rp1_pwm_regmap_config = {
> > + .reg_bits = 32,
> > + .val_bits = 32,
> > + .reg_stride = 4,
> > + .max_register = 0x60,
>
> I'm not a fan of aligning the = in a struct, still more if it fails like
> here. Please consistently align all =s, or even better, use a single
> space before each =. (Same for the struct definitions above, but I won't
> insist.)
Let's use the single space.
>
> > +};
> > +
> > +static void rp1_pwm_apply_config(struct pwm_chip *chip, struct pwm_device *pwm)
> > +{
> > + struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> > + u32 value;
> > +
> > + /* update the changed registers on the next strobe to avoid glitches */
> > + regmap_read(rp1->regmap, RP1_PWM_GLOBAL_CTRL, &value);
> > + value |= RP1_PWM_SET_UPDATE;
> > + regmap_write(rp1->regmap, RP1_PWM_GLOBAL_CTRL, value);
>
> I assume there is a glitch if I update two channels and the old
> configuration of the first channel ends while I'm in the middle of
> configuring the second?
The configuration registers are per-channel but the update flag is global.
I don't have details of the hw insights, my best guess is that anything that
you set in the registers before updating the flag will take effect, so there
should be no glitches.
>
> > +}
> > +
> > +static int rp1_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> > +{
> > + struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> > +
> > + /* init channel to reset defaults */
> > + regmap_write(rp1->regmap, RP1_PWM_CHANNEL_CTRL(pwm->hwpwm), RP1_PWM_CHANNEL_DEFAULT);
> > + return 0;
> > +}
> > +
> > +static int rp1_pwm_round_waveform_tohw(struct pwm_chip *chip,
> > + struct pwm_device *pwm,
> > + const struct pwm_waveform *wf,
> > + void *_wfhw)
> > +{
> > + struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> > + struct rp1_pwm_waveform *wfhw = _wfhw;
> > + u64 clk_rate = rp1->clk_rate;
> > + u64 ticks;
>
> if (!wf->period_length_ns)
> wfhw->enabled = false
> return 0;
>
> > + ticks = mul_u64_u64_div_u64(wf->period_length_ns, clk_rate, NSEC_PER_SEC);
>
> To ensure this doesn't overflow please fail to probe the driver if
> clk_rate > 1 GHz with an explaining comment. (Or alternatively calculate
> the length of period_ticks = U32_MAX and skip the calculation if
> wf->period_length_ns is bigger.)
Ack.
>
> > + if (ticks > U32_MAX)
> > + ticks = U32_MAX;
> > + wfhw->period_ticks = ticks;
>
> What happens if wf->period_length_ns > 0 but ticks == 0?
I've added a check, returning 1 to signal teh round-up, and a minimum tick of 1
in this case.
>
> > + if (wf->duty_offset_ns + wf->duty_length_ns >= wf->period_length_ns) {
>
> The maybe surprising effect here is that in the two cases
>
> wf->duty_offset_ns == wf->period_length_ns and wf->duty_length_ns == 0
>
> and
>
> wf->duty_length_ns == wf->period_length_ns and wf->duty_offset_ns == 0
>
> you're configuring inverted polarity. I doesn't matter technically
> because the result is the same, but for consumers still using pwm_state
> this is irritating. That's why pwm-stm32 uses inverted polarity only if
> also wf->duty_length_ns and wf->duty_offset_ns are non-zero.
Ack.
>
> > + ticks = mul_u64_u64_div_u64(wf->period_length_ns - wf->duty_length_ns,
> > + clk_rate, NSEC_PER_SEC);
>
> The rounding is wrong here. You should pick the biggest duty_length not
> bigger than wf->duty_length_ns, so you have to use
>
> ticks = wfhw->period_ticks - mul_u64_u64_div_u64(wf->duty_length_ns, clk_rate, NSEC_PER_SEC):
>
> . I see this is a hole in the pwmtestperf coverage.
Ack.
>
> > + wfhw->inverted_polarity = true;
> > + } else {
> > + ticks = mul_u64_u64_div_u64(wf->duty_length_ns, clk_rate, NSEC_PER_SEC);
> > + wfhw->inverted_polarity = false;
> > + }
> > +
> > + if (ticks > wfhw->period_ticks)
> > + ticks = wfhw->period_ticks;
>
> You can and should assume that wf->duty_length_ns <=
> wf->period_length_ns. Then the if condition can never become true.
Ack.
>
> > + wfhw->duty_ticks = ticks;
> > +
> > + wfhw->enabled = !!wfhw->duty_ticks;
> > +
> > + return 0;
> > +}
> > +
> > +static int rp1_pwm_round_waveform_fromhw(struct pwm_chip *chip,
> > + struct pwm_device *pwm,
> > + const void *_wfhw,
> > + struct pwm_waveform *wf)
> > +{
> > + struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> > + const struct rp1_pwm_waveform *wfhw = _wfhw;
> > + u64 clk_rate = rp1->clk_rate;
> > + u32 ticks;
> > +
> > + memset(wf, 0, sizeof(*wf));
>
> wf = (struct pwm_waveform){ };
>
> is usually more efficient.
Ack.
>
> > + if (!wfhw->enabled)
> > + return 0;
> > +
> > + wf->period_length_ns = DIV_ROUND_UP_ULL((u64)wfhw->period_ticks * NSEC_PER_SEC, clk_rate);
> > +
> > + if (wfhw->inverted_polarity) {
> > + wf->duty_length_ns = DIV_ROUND_UP_ULL((u64)wfhw->duty_ticks * NSEC_PER_SEC,
> > + clk_rate);
> > + } else {
> > + wf->duty_offset_ns = DIV_ROUND_UP_ULL((u64)wfhw->duty_ticks * NSEC_PER_SEC,
> > + clk_rate);
> > + ticks = wfhw->period_ticks - wfhw->duty_ticks;
> > + wf->duty_length_ns = DIV_ROUND_UP_ULL((u64)ticks * NSEC_PER_SEC, clk_rate);
> > + }
>
> This needs adaption after the rounding issue in tohw is fixed.
Ack.
>
> > + return 0;
> > +}
> > +
> > +static int rp1_pwm_write_waveform(struct pwm_chip *chip,
> > + struct pwm_device *pwm,
> > + const void *_wfhw)
> > +{
> > + struct rp1_pwm *rp1 = pwmchip_get_drvdata(chip);
> > + const struct rp1_pwm_waveform *wfhw = _wfhw;
> > + u32 value;
> > +
> > + /* set period and duty cycle */
> > + regmap_write(rp1->regmap,
> > + RP1_PWM_RANGE(pwm->hwpwm), wfhw->period_ticks);
> > + regmap_write(rp1->regmap,
> > + RP1_PWM_DUTY(pwm->hwpwm), wfhw->duty_ticks);
> > +
> > + /* set polarity */
> > + regmap_read(rp1->regmap, RP1_PWM_CHANNEL_CTRL(pwm->hwpwm), &value);
> > + if (!wfhw->inverted_polarity)
> > + value &= ~RP1_PWM_POLARITY;
> > + else
> > + value |= RP1_PWM_POLARITY;
> > + regmap_write(rp1->regmap, RP1_PWM_CHANNEL_CTRL(pwm->hwpwm), value);
> > +
> > + /* enable/disable */
> > + regmap_read(rp1->regmap, RP1_PWM_GLOBAL_CTRL, &value);
> > + if (wfhw->enabled)
> > + value |= RP1_PWM_CHANNEL_ENABLE(pwm->hwpwm);
> > + else
> > + value &= ~RP1_PWM_CHANNEL_ENABLE(pwm->hwpwm);
> > + regmap_write(rp1->regmap, RP1_PWM_GLOBAL_CTRL, value);
>
> You can exit early if wfhw->enabled is false after clearing the channel
> enable bit.
Ack.
>
> > + rp1_pwm_apply_config(chip, pwm);
> > +
> > + return 0;
> > +}
> > +
<,...snip...>
> > + }
> > +
> > + return 0;
> > +
> > +err_disable_clk:
> > + clk_disable_unprepare(rp1->clk);
> > +
> > + return ret;
> > +}
>
> On remove you miss to balance the call to clk_prepare_enable() (if no
> failed call to clk_prepare_enable() in rp1_pwm_resume() happend).
Since this driver now exports a syscon, it's only builtin (=Y) so
it cannot be unloaded.
I've also avoided the .remove callback via .suppress_bind_attrs.
>
> > +
> > +static int rp1_pwm_suspend(struct device *dev)
> > +{
> > + struct rp1_pwm *rp1 = dev_get_drvdata(dev);
> > +
> > + if (rp1->clk_enabled) {
> > + clk_disable_unprepare(rp1->clk);
> > + rp1->clk_enabled = false;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int rp1_pwm_resume(struct device *dev)
> > +{
> > + struct rp1_pwm *rp1 = dev_get_drvdata(dev);
> > + int ret;
> > +
> > + ret = clk_prepare_enable(rp1->clk);
> > + if (ret) {
> > + dev_err(dev, "Failed to enable clock on resume: %d\n", ret);
>
> Please use %pe for error codes.
Ack.
Best regards,
Andrea
>
> > + return ret;
> > + }
> > +
> > + rp1->clk_enabled = true;
> > +
> > + return 0;
> > +}
>
> Best regards
> Uwe
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: Update HiSilicon PMU driver maintainer to Yushan Wang
From: Yushan Wang @ 2026-04-16 10:10 UTC (permalink / raw)
To: Jonathan Cameron, Yushan Wang, Jie Zhan, Will Deacon,
Mark Rutland, linux-arm-kernel, linux-perf-users
Cc: jic23, Linuxarm
In-Reply-To: <20260416095110.25612-1-Jonathan.Cameron@huawei.com>
On 4/16/2026 5:51 PM, Jonathan Cameron wrote:
> Replace myself with Yushan Wang who is very familiar with the HiSilicon PMU
> drivers.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> MAINTAINERS | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
It's privileged to have your guidance, thanks a lot!
Acked-by: Yushan Wang <wangyushan12@huawei.com>
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d1cc0e12fe1f..8b95a43527fa 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11563,7 +11563,7 @@ F: Documentation/devicetree/bindings/net/hisilicon*.txt
> F: drivers/net/ethernet/hisilicon/
>
> HISILICON PMU DRIVER
> -M: Jonathan Cameron <jonathan.cameron@huawei.com>
> +M: Yushan Wang <wangyushan12@huawei.com>
> S: Supported
> W: http://www.hisilicon.com
> F: Documentation/admin-guide/perf/hisi-pcie-pmu.rst
^ permalink raw reply
* [PATCH] MAINTAINERS: Update HiSilicon PMU driver maintainer to Yushan Wang
From: Jonathan Cameron @ 2026-04-16 9:51 UTC (permalink / raw)
To: Yushan Wang, Jie Zhan, Will Deacon, Mark Rutland,
linux-arm-kernel, linux-perf-users
Cc: linuxarm, jic23
Replace myself with Yushan Wang who is very familiar with the HiSilicon PMU
drivers.
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index d1cc0e12fe1f..8b95a43527fa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11563,7 +11563,7 @@ F: Documentation/devicetree/bindings/net/hisilicon*.txt
F: drivers/net/ethernet/hisilicon/
HISILICON PMU DRIVER
-M: Jonathan Cameron <jonathan.cameron@huawei.com>
+M: Yushan Wang <wangyushan12@huawei.com>
S: Supported
W: http://www.hisilicon.com
F: Documentation/admin-guide/perf/hisi-pcie-pmu.rst
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] arm_pmu: acpi: fix reference leak on failed device registration
From: Mark Rutland @ 2026-04-16 9:50 UTC (permalink / raw)
To: Guangshuo Li
Cc: Johan Hovold, Greg Kroah-Hartman, Will Deacon, Anshuman Khandual,
linux-arm-kernel, linux-perf-users, linux-kernel, stable
In-Reply-To: <CANUHTR9+Z9s3thfKMC5qiLMdYJAo-1sX1g9QiU65OVCbb+mAMQ@mail.gmail.com>
On Thu, Apr 16, 2026 at 04:59:01PM +0800, Guangshuo Li wrote:
> On Thu, 16 Apr 2026 at 15:23, Johan Hovold <johan@kernel.org> wrote:
> > On Thu, Apr 16, 2026 at 06:40:55AM +0200, Greg Kroah-Hartman wrote:
> > > On Wed, Apr 15, 2026 at 07:19:06PM +0100, Mark Rutland wrote:
> > > > Greg, am I missing some functional reason why we can't rework
> > > > device_register() and friends to handle cleanup themselves? I appreciate
> > > > that'll involve churn for some callers, but AFAICT the majority of
> > > > callers don't have the required cleanup.
> > >
> > > Yes, we should fix the platform core code here, this should not be
> > > required to do everywhere as obviously we all got it wrong.
> >
> > It's not just the platform code as this directly reflects the behaviour
> > of device_register() as Mark pointed out.
> >
> > It is indeed an unfortunate quirk of the driver model, but one can argue
> > that having a registration function that frees its argument on errors
> > would be even worse. And even more so when many (or most) users get this
> > right.
> >
> > So if we want to change this, I think we would need to deprecate
> > device_register() in favour of explicit device_initialize() and
> > device_add().
> >
> > That said, most users of platform_device_register() appear to operate
> > on static platform devices which don't even have a release function and
> > would trigger a WARN() if we ever drop the reference (which is arguably
> > worse than leaking a tiny bit of memory).
> >
> > So leaving things as-is is also an option.
> >
> > Johan
>
> I did some more investigation, and it looks like directly changing the
> semantics of the existing API would break code that is already correct
> today.
Evidently this wasn't entirely clear, but when I suggested changing the
semantic, I had implicitly meant that we'd also go and fix up callers to
handle the new semantic.
I agree that whatever we do, we'll have to change some callers, given
that existing callers have inconsistent expectations.
> In particular, there seem to be at least two different kinds of callers:
>
> Callers that already handle the failure path explicitly after
> platform_device_register() fails. For these users, changing
> platform_device_register() itself to drop the reference internally
> would lead to double put / use-after-free issues.
Yes; for those we could drop the explicit cleanup.
As an alternative (as Johan mentioned above), if we deprecated
*_register() in favour of separate *_initialize() and *_add() calls,
then we could require that callers had explicit cleanup. As that cleanup
would more obviously pair with the *_initialize() step, it would be less
surprising than cleaning up for a function that returned an error.
As I mentioned in my other reply to Johan, that might also give options
for how to handle the static platform_device case, e.g. with an
*_uninitialize() function.
> Callers that operate on static struct platform_device objects. Many of
> these do not have a release callback, so blindly dropping the
> reference on failure would trigger a WARN.
>
> Because of this, changing platform_device_register() itself to always
> clean up on failure does not look safe.
I agree that we probably can't have _*register() do all the necessary
cleanup, since callers want different things.
As per Johan's suggestion, and my reply, I suspect the best option
for a consistent API would be to deprecate *_register() in favour of
separate *_initialize() and *_add() calls.
> One possible direction may be to leave platform_device_register()
> unchanged, and instead add new helper APIs for the different cases.
>
> For case (1), I was thinking of a helper like:
>
> platform_device_register_and_put()
>
> The implementation would simply call platform_device_register(), and if
> that fails, call platform_device_put(). Callers converted to this helper
> would then no longer perform their own put on the failure path.
I think that's going to be a source of confusion, because there's no
clear way to name that function. A '_and_put' suffix makes it sound like
it does a put unconditionally, rather than when the *_add() step fails.
Otherwise, I agree that would work for those callers.
> For case (2), I was thinking of a helper like:
>
> platform_device_register_static()
>
> The implementation would first install a no-op release callback when
> pdev->dev.release is not set, and then call
> platform_device_register_and_put(). This would make the failure path
> well-defined for static platform_device users, avoiding the reference
> leak without triggering a WARN.
Something like that might work.
As above, I think my preference would be to have separate
init/add/uninit calls, as that way each of the functions succeeds or
fails atomically, which is more aligned with general conventions.
> If this direction sounds reasonable, I would be happy to work on it and
> send a patch, and I would also be very willing to help with the related
> API conversion work for existing callers.
Fantastic!
I think we should hear what Greg thinks of the options before we start
on that, but it's great to hear that you're willing!
Mark.
^ permalink raw reply
* Re: [PATCH v2 0/9] driver core / pmdomain: Add support for fined grained sync_state
From: Ulf Hansson @ 2026-04-16 9:42 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm,
Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
Jonathan Hunter, Dmitry Baryshkov, linux-arm-kernel, linux-kernel
In-Reply-To: <CAMuHMdWHnANr4R+AW5-xHrm=D4SJLuKVF5mq3PFkbevcTz5qWw@mail.gmail.com>
On Thu, 16 Apr 2026 at 11:15, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Ulf,
>
> On Fri, 10 Apr 2026 at 12:41, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > Since the introduction [1] of the common sync_state support for pmdomains
> > (genpd), we have encountered a lot of various interesting problems. In most
> > cases the new behaviour of genpd triggered some weird platform specific bugs.
> >
> > That said, in LPC in Tokyo me and Saravana hosted a session to walk through the
> > remaining limitations that we have found for genpd's sync state support. In
> > particular, we discussed the problems we have for the so-called onecell power
> > domain providers, where a single provider typically provides multiple
> > independent power domains, all with their own set of consumers.
> >
> > Note that, onecell power domain providers are very common. It's being used by
> > many SoCs/platforms/technologies. To name a few:
> > SCMI, Qualcomm, NXP, Mediatek, Renesas, TI, etc.
> >
> > Anyway, in these cases, the generic sync_state mechanism with fw_devlink isn't
> > fine grained enough, as we end up waiting for all consumers for all power
> > domains before the ->sync_callback gets called for the supplier/provider. In
> > other words, we may end up keeping unused power domains powered-on, for no good
> > reasons.
> >
> > The series intends to fix this problem. Please have a look at the commit
> > messages for more details and help review/test!
>
> Thanks for the update!
>
> At first glance, the only real change compared to v1 seems to be
> the removal of printing
>
> pr_info("%s:%s con=%s\n", __func__, dev_name(dev),
> dev_name(consumer));
>
> Right?
Correct! I forgot to include a version history, sorry.
Besides the removed print, I have just added your tags and updated the
commit message in patch9.
FYI, my plan is to queue this as soon as v7.1-rc1 is available.
Kind regards
Uffe
^ permalink raw reply
* ACPI dump of HP OmniBook 5 14-he0xxx
From: gaming stream @ 2026-04-16 9:42 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
I’ve extracted a full ACPI dump from an HP OmniBook 5 14-he0xxx
running Snapdragon X (X126100, Oryon CPU).
This includes DSDT, all SSDTs, and full raw ACPI tables.
Repository: https://github.com/TheXterminator/hp-omnibook-5-snapdragon-x-acpi.git
Sharing in case it helps with Linux bring-up or device tree work
for Snapdragon X platforms.
^ permalink raw reply
* Re: [PATCH 1/9] dt-bindings: sound: mt2701-afe-pcm: add HDMI audio path clocks
From: Krzysztof Kozlowski @ 2026-04-16 9:38 UTC (permalink / raw)
To: Daniel Golle
Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Matthias Brugger, AngeloGioacchino Del Regno,
Jaroslav Kysela, Takashi Iwai, Cyril Chao, Arnd Bergmann,
Kuninori Morimoto, Nícolas F. R. A. Prado, Eugen Hristev,
linux-sound, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek
In-Reply-To: <50afd83a314cd20c715fb9b0d3bc85fb00f9a6eb.1776265610.git.daniel@makrotopia.org>
On Wed, Apr 15, 2026 at 04:23:27PM +0100, Daniel Golle wrote:
> Document four additional optional clocks feeding the HDMI audio
> output path on MT2701 and MT7623N: the HADDS2 PLL (root of the
There is no MT7623N compatible in this file, so that's confusing. Does
mt7622 have it? If not, then it should be restricted per variant. If
yet, the model name is confusing.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] arm_pmu: acpi: fix reference leak on failed device registration
From: Mark Rutland @ 2026-04-16 9:30 UTC (permalink / raw)
To: Johan Hovold
Cc: Greg Kroah-Hartman, Guangshuo Li, Will Deacon, Anshuman Khandual,
linux-arm-kernel, linux-perf-users, linux-kernel, stable
In-Reply-To: <aeCOdWLaVpH-5w8s@hovoldconsulting.com>
On Thu, Apr 16, 2026 at 09:23:33AM +0200, Johan Hovold wrote:
> On Thu, Apr 16, 2026 at 06:40:55AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Apr 15, 2026 at 07:19:06PM +0100, Mark Rutland wrote:
>
> > > AFAICT you're saying that the reference was taken *within*
> > > platform_device_register(), and then platform_device_register() itself
> > > has failed. I think it's surprising that platform_device_register()
> > > doesn't clean that up itself in the case of an error.
> > >
> > > There are *tonnes* of calls to platform_device_register() throughout the
> > > kernel that don't even bother to check the return value, and many that
> > > just pass the return onto a caller that can't possibly know to call
> > > platform_device_put().
> > >
> > > Code in the same file as platform_device_register() expects it to clean up
> > > after itself, e.g.
> > >
> > > | int platform_add_devices(struct platform_device **devs, int num)
> > > | {
> > > | int i, ret = 0;
> > > |
> > > | for (i = 0; i < num; i++) {
> > > | ret = platform_device_register(devs[i]);
> > > | if (ret) {
> > > | while (--i >= 0)
> > > | platform_device_unregister(devs[i]);
> > > | break;
> > > | }
> > > | }
> > > |
> > > | return ret;
> > > | }
> > >
> > > That's been there since the initial git commit, and back then,
> > > platform_device_register() didn't mention that callers needed to perform
> > > any cleanup.
> > >
> > > I see a comment was added to platform_device_register() in commit:
> > >
> > > 67e532a42cf4 ("driver core: platform: document registration-failure requirement")
> > >
> > > ... and that copied the commend added for device_register() in commit:
> > >
> > > 5739411acbaa ("Driver core: Clarify device cleanup.")
> > >
> > > ... but the potential brokenness is so widespread, and the behaviour is
> > > so surprising, that I'd argue the real but is that device_register()
> > > doesn't clean up in case of error. I don't think it's worth changing
> > > this single instance given the prevalance and churn fixing all of that
> > > would involve.
> > >
> > > I think it would be far better to fix the core driver API such that when
> > > those functions return an error, they've already cleaned up for
> > > themselves.
> > >
> > > Greg, am I missing some functional reason why we can't rework
> > > device_register() and friends to handle cleanup themselves? I appreciate
> > > that'll involve churn for some callers, but AFAICT the majority of
> > > callers don't have the required cleanup.
> >
> > Yes, we should fix the platform core code here, this should not be
> > required to do everywhere as obviously we all got it wrong.
>
> It's not just the platform code as this directly reflects the behaviour
> of device_register() as Mark pointed out.
>
> It is indeed an unfortunate quirk of the driver model, but one can argue
> that having a registration function that frees its argument on errors
> would be even worse. And even more so when many (or most) users get this
> right.
Ah, sorry; I had missed that the _put() step would actually free the
object (and as you explain below, how that won't work for many callers).
> So if we want to change this, I think we would need to deprecate
> device_register() in favour of explicit device_initialize() and
> device_add().
Is is possible to have {platfom_,}device_uninitialize() functions that
does everything except the ->release() call? If we had that, then we'd
be able to have a flow along the lines of:
int some_init_function(void)
{
int err;
platform_device_init(&static_pdev);
err = platform_device_add(&static_pdev))
if (err)
goto out_uninit;
return 0;
out_uninit:
platform_device_uninit(&static_pdev);
return err;
}
... which I think would align with what people generally expect to have
to do.
Those would have to check that only a single reference was held (from
the corresponding _initialize()), and could WARN/fail if more were held.
> That said, most users of platform_device_register() appear to operate
> on static platform devices which don't even have a release function and
> would trigger a WARN() if we ever drop the reference (which is arguably
> worse than leaking a tiny bit of memory).
>
> So leaving things as-is is also an option.
I suspect that might be the best option for now.
Mark.
^ permalink raw reply
* Re: [PATCH v2 0/9] driver core / pmdomain: Add support for fined grained sync_state
From: Geert Uytterhoeven @ 2026-04-16 9:15 UTC (permalink / raw)
To: Ulf Hansson
Cc: Saravana Kannan, Rafael J . Wysocki, Greg Kroah-Hartman, linux-pm,
Sudeep Holla, Cristian Marussi, Kevin Hilman, Stephen Boyd,
Marek Szyprowski, Bjorn Andersson, Abel Vesa, Peng Fan,
Tomi Valkeinen, Maulik Shah, Konrad Dybcio, Thierry Reding,
Jonathan Hunter, Dmitry Baryshkov, linux-arm-kernel, linux-kernel
In-Reply-To: <20260410104058.83748-1-ulf.hansson@linaro.org>
Hi Ulf,
On Fri, 10 Apr 2026 at 12:41, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Since the introduction [1] of the common sync_state support for pmdomains
> (genpd), we have encountered a lot of various interesting problems. In most
> cases the new behaviour of genpd triggered some weird platform specific bugs.
>
> That said, in LPC in Tokyo me and Saravana hosted a session to walk through the
> remaining limitations that we have found for genpd's sync state support. In
> particular, we discussed the problems we have for the so-called onecell power
> domain providers, where a single provider typically provides multiple
> independent power domains, all with their own set of consumers.
>
> Note that, onecell power domain providers are very common. It's being used by
> many SoCs/platforms/technologies. To name a few:
> SCMI, Qualcomm, NXP, Mediatek, Renesas, TI, etc.
>
> Anyway, in these cases, the generic sync_state mechanism with fw_devlink isn't
> fine grained enough, as we end up waiting for all consumers for all power
> domains before the ->sync_callback gets called for the supplier/provider. In
> other words, we may end up keeping unused power domains powered-on, for no good
> reasons.
>
> The series intends to fix this problem. Please have a look at the commit
> messages for more details and help review/test!
Thanks for the update!
At first glance, the only real change compared to v1 seems to be
the removal of printing
pr_info("%s:%s con=%s\n", __func__, dev_name(dev),
dev_name(consumer));
Right?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] crypto: tstmgr - guard xxhash tests
From: Herbert Xu @ 2026-04-16 9:09 UTC (permalink / raw)
To: Hamza Mahfooz
Cc: linux-crypto, David S. Miller, Maxime Coquelin, Alexandre Torgue,
linux-stm32, linux-arm-kernel, linux-kernel, Jeff Barnes,
Paul Monson
In-Reply-To: <aeCmk6LbLFT4Keo2@gondor.apana.org.au>
On Thu, Apr 16, 2026 at 05:06:27PM +0800, Herbert Xu wrote:
>
> So the error is coming from tcrypt. I think that's where the ifdef
> should be added.
On second thought, fips_allowed should not mean that an algorithm
must be present.
So we should change it such that an -ENOENT is not fatal, or at least
when it's called from tcrypt.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] crypto: tstmgr - guard xxhash tests
From: Herbert Xu @ 2026-04-16 9:06 UTC (permalink / raw)
To: Hamza Mahfooz
Cc: linux-crypto, David S. Miller, Maxime Coquelin, Alexandre Torgue,
linux-stm32, linux-arm-kernel, linux-kernel, Jeff Barnes,
Paul Monson
In-Reply-To: <adffSYxKIuaDLZit@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
On Thu, Apr 09, 2026 at 10:18:01AM -0700, Hamza Mahfooz wrote:
>
> alg: hash: failed to allocate transform for xxhash64: -2
> Kernel panic - not syncing: alg: self-tests for xxhash64 (xxhash64) failed in fips mode!
> CPU: 0 PID: 425 Comm: modprobe Not tainted 6.6.130.2-2.azl3 #1
> Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS Hyper-V UEFI Release v4.1 01/08/2026
> Call Trace:
> <TASK>
> dump_stack_lvl+0x4c/0x70
> dump_stack+0x14/0x20
> panic+0x179/0x330
> alg_test+0x678/0x680
> ? __alloc_pages+0x1e2/0x340
> do_test+0x26f8/0x7670 [tcrypt]
So the error is coming from tcrypt. I think that's where the ifdef
should be added.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v2 2/3] dt-bindings: gpio: Add EIO GPIO compatible to gpio-zynq
From: Krzysztof Kozlowski @ 2026-04-16 9:06 UTC (permalink / raw)
To: Michal Simek
Cc: Conor Dooley, Shubhrajyoti Datta, linux-kernel, git,
shubhrajyoti.datta, Srinivas Neeli, Linus Walleij,
Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-gpio, devicetree, linux-arm-kernel
In-Reply-To: <c973f9d4-9bb5-40f4-8f09-72e23f92cd2d@amd.com>
On Thu, Apr 16, 2026 at 07:58:27AM +0200, Michal Simek wrote:
>
>
> On 4/15/26 17:01, Conor Dooley wrote:
> > On Wed, Apr 15, 2026 at 04:26:27PM +0530, Shubhrajyoti Datta wrote:
> > > EIO (Extended IO) is a GPIO block found on xa2ve3288 silicon..
> >
> >
> > Why does the compatible have a "1.0" when it is in silicon?
>
> Sorry not following what the problem is. Yes this is hard block in silicon
> and it is silicon v1.
Writing bindings: compatibles should be specific to device, not some
arbitrary versioning.
OR explain in commit msg. That commit msg clealy suggests code is wrong.
>
> > Why doesn't the compatible contain "xa2ve3288"?
>
> This unit can be used on different silicons too.
That's not what the commit said.
>
> > Why is this device not compatible with existing ones, since
> > gpio-lines-names appears to be the sole difference?
>
> There is no way how to detect gpio width.
Where in the commit msg are the differences explained?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] arm_pmu: acpi: fix reference leak on failed device registration
From: Guangshuo Li @ 2026-04-16 8:59 UTC (permalink / raw)
To: Johan Hovold
Cc: Greg Kroah-Hartman, Mark Rutland, Will Deacon, Anshuman Khandual,
linux-arm-kernel, linux-perf-users, linux-kernel, stable
In-Reply-To: <aeCOdWLaVpH-5w8s@hovoldconsulting.com>
Hi Greg, Mark, Johan,
Thanks for the further comments.
On Thu, 16 Apr 2026 at 15:23, Johan Hovold <johan@kernel.org> wrote:
>
> On Thu, Apr 16, 2026 at 06:40:55AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Apr 15, 2026 at 07:19:06PM +0100, Mark Rutland wrote:
>
> > > AFAICT you're saying that the reference was taken *within*
> > > platform_device_register(), and then platform_device_register() itself
> > > has failed. I think it's surprising that platform_device_register()
> > > doesn't clean that up itself in the case of an error.
> > >
> > > There are *tonnes* of calls to platform_device_register() throughout the
> > > kernel that don't even bother to check the return value, and many that
> > > just pass the return onto a caller that can't possibly know to call
> > > platform_device_put().
> > >
> > > Code in the same file as platform_device_register() expects it to clean up
> > > after itself, e.g.
> > >
> > > | int platform_add_devices(struct platform_device **devs, int num)
> > > | {
> > > | int i, ret = 0;
> > > |
> > > | for (i = 0; i < num; i++) {
> > > | ret = platform_device_register(devs[i]);
> > > | if (ret) {
> > > | while (--i >= 0)
> > > | platform_device_unregister(devs[i]);
> > > | break;
> > > | }
> > > | }
> > > |
> > > | return ret;
> > > | }
> > >
> > > That's been there since the initial git commit, and back then,
> > > platform_device_register() didn't mention that callers needed to perform
> > > any cleanup.
> > >
> > > I see a comment was added to platform_device_register() in commit:
> > >
> > > 67e532a42cf4 ("driver core: platform: document registration-failure requirement")
> > >
> > > ... and that copied the commend added for device_register() in commit:
> > >
> > > 5739411acbaa ("Driver core: Clarify device cleanup.")
> > >
> > > ... but the potential brokenness is so widespread, and the behaviour is
> > > so surprising, that I'd argue the real but is that device_register()
> > > doesn't clean up in case of error. I don't think it's worth changing
> > > this single instance given the prevalance and churn fixing all of that
> > > would involve.
> > >
> > > I think it would be far better to fix the core driver API such that when
> > > those functions return an error, they've already cleaned up for
> > > themselves.
> > >
> > > Greg, am I missing some functional reason why we can't rework
> > > device_register() and friends to handle cleanup themselves? I appreciate
> > > that'll involve churn for some callers, but AFAICT the majority of
> > > callers don't have the required cleanup.
> >
> > Yes, we should fix the platform core code here, this should not be
> > required to do everywhere as obviously we all got it wrong.
>
> It's not just the platform code as this directly reflects the behaviour
> of device_register() as Mark pointed out.
>
> It is indeed an unfortunate quirk of the driver model, but one can argue
> that having a registration function that frees its argument on errors
> would be even worse. And even more so when many (or most) users get this
> right.
>
> So if we want to change this, I think we would need to deprecate
> device_register() in favour of explicit device_initialize() and
> device_add().
>
> That said, most users of platform_device_register() appear to operate
> on static platform devices which don't even have a release function and
> would trigger a WARN() if we ever drop the reference (which is arguably
> worse than leaking a tiny bit of memory).
>
> So leaving things as-is is also an option.
>
> Johan
I did some more investigation, and it looks like directly changing the
semantics of the existing API would break code that is already correct
today.
In particular, there seem to be at least two different kinds of callers:
Callers that already handle the failure path explicitly after
platform_device_register() fails. For these users, changing
platform_device_register() itself to drop the reference internally
would lead to double put / use-after-free issues.
Callers that operate on static struct platform_device objects. Many of
these do not have a release callback, so blindly dropping the
reference on failure would trigger a WARN.
Because of this, changing platform_device_register() itself to always
clean up on failure does not look safe.
One possible direction may be to leave platform_device_register()
unchanged, and instead add new helper APIs for the different cases.
For case (1), I was thinking of a helper like:
platform_device_register_and_put()
The implementation would simply call platform_device_register(), and if
that fails, call platform_device_put(). Callers converted to this helper
would then no longer perform their own put on the failure path.
For case (2), I was thinking of a helper like:
platform_device_register_static()
The implementation would first install a no-op release callback when
pdev->dev.release is not set, and then call
platform_device_register_and_put(). This would make the failure path
well-defined for static platform_device users, avoiding the reference
leak without triggering a WARN.
If this direction sounds reasonable, I would be happy to work on it and
send a patch, and I would also be very willing to help with the related
API conversion work for existing callers.
Thanks,
Guangshuo
^ permalink raw reply
* Re: [PATCH v3 6/6] dt-bindings: soc: mediatek: devapc: Add bindings for MT8196
From: Krzysztof Kozlowski @ 2026-04-16 8:58 UTC (permalink / raw)
To: Xiaoshun Xu
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek, Sirius Wang, Vince-wl Liu,
Project_Global_Chrome_Upstream_Group
In-Reply-To: <20260416031231.2932493-7-xiaoshun.xu@mediatek.com>
On Thu, Apr 16, 2026 at 11:12:09AM +0800, Xiaoshun Xu wrote:
> Extend the devapc device tree bindings to support the MediaTek MT8196
> SoC. This includes:
>
> - Adding "mediatek,mt8196-devapc" to the list of compatible strings.
>
> These changes enable proper configuration and integration of devapc on
> MT8196 platforms, ensuring accurate device matching and resource
> allocation in the device tree.
Same comments. It's really poor commit msg.
Also, subject wrong. Drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 4/6] dt-bindings: soc: mediatek: devapc: Add bindings for MT8189
From: Krzysztof Kozlowski @ 2026-04-16 8:56 UTC (permalink / raw)
To: Xiaoshun Xu
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek, Sirius Wang, Vince-wl Liu,
Project_Global_Chrome_Upstream_Group
In-Reply-To: <20260416031231.2932493-5-xiaoshun.xu@mediatek.com>
On Thu, Apr 16, 2026 at 11:12:07AM +0800, Xiaoshun Xu wrote:
> Extend the devapc device tree bindings to support the MediaTek MT8189
> SoC. This includes:
>
> - Adding "mediatek,mt8189-devapc" to the list of compatible strings.
> - Introducing the "vio-idx-num" property to specify the number of bus
> slaves managed by devapc.
>
> These changes enable proper configuration and integration of devapc on
> MT8189 platforms, ensuring accurate device matching and resource
> allocation in the device tree.
Pointless paragraph. Would you write a commit which does not enable
proper configuration?
>
> Signed-off-by: Xiaoshun Xu <xiaoshun.xu@mediatek.com>
> ---
> .../devicetree/bindings/soc/mediatek/devapc.yaml | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> index 99e2caafeadf..06a096440331 100644
> --- a/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> +++ b/Documentation/devicetree/bindings/soc/mediatek/devapc.yaml
> @@ -14,13 +14,14 @@ description: |
> analysis and countermeasures.
>
> maintainers:
> - - Neal Liu <neal.liu@mediatek.com>
Your commit said what the change is doing. It's pointless because we see
it in the diff. Except that we don't...
> + - Xiaoshun Xu <xiaoshun.xu@mediatek.com>
>
> properties:
> compatible:
> enum:
> - mediatek,mt6779-devapc
> - mediatek,mt8186-devapc
> + - mediatek,mt8189-devapc
>
> reg:
> description: The base address of devapc register bank
> @@ -30,6 +31,10 @@ properties:
> description: A single interrupt specifier
> maxItems: 1
>
> + vio-idx-num:
Nah, compatible defines it. Please follow standard rules for bindings,
see writing-bindings doc.
> + description: Describe the number of bus slaves controlled by devapc
> + $ref: /schemas/types.yaml#/definitions/uint32
> +
> clocks:
> description: Contains module clock source and clock names
> maxItems: 1
> @@ -42,8 +47,6 @@ required:
> - compatible
> - reg
> - interrupts
> - - clocks
> - - clock-names
Why?
This commit explains nothing and makes some random-looking code changes.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v2 0/3] arm64: dts: amlogic: t7: Add UART support and enable Bluetooth on VIM4
From: Ronald Claveau @ 2026-04-16 8:54 UTC (permalink / raw)
To: Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-kernel, linux-amlogic, devicetree, linux-kernel,
Ronald Claveau
This series adds all UART controllers for the Amlogic T7 SoC and enables
the Bluetooth controller on the Khadas VIM4 board.
The T7 SoC ships with six UART controllers (A through F), but only
uart_a was previously described in the device tree.
- Patch 1 adds the pinctrl group for UART C, which is needed to route
its four signals (TX, RX, CTS, RTS) through the correct pads.
- Patch 2 completes the uart_a node (peripheral clock) and
repositions it to respect the ascending reg address order required
by the DT specification. It then adds nodes for UART B through F,
each with their respective peripheral clock.
- Patch 3 enables UART C on the Khadas VIM4 board and attaches the
on-board BCM43438 Bluetooth controller to it, with hardware flow
control, wakeup GPIOs, LPO clock and power supplies.
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
Changes in v2:
- PATCH 1: change underscore to dash in pin node name,
according to Xianwei's feedback.
- PATCH 3: remove clocks and clock-names as already defined in DTSI,
according to Xianwei's feedback.
- Link to v1: https://lore.kernel.org/r/20260415-add-bluetooth-t7-vim4-v1-0-0ba0746cc1d6@aliel.fr
---
Ronald Claveau (3):
arm64: dts: amlogic: t7: Add uart_c pinctrl pins group
arm64: dts: amlogic: t7: Add UART controllers nodes
arm64: dts: amlogic: t7: khadas-vim4: Enable Bluetooth
.../dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts | 21 ++++++-
arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi | 73 +++++++++++++++++++---
2 files changed, 85 insertions(+), 9 deletions(-)
---
base-commit: 6aa9edb4f8266cfb913ee74f5e55116550b5574d
change-id: 20260414-add-bluetooth-t7-vim4-f01e03c4ec2a
Best regards,
--
Ronald Claveau <linux-kernel-dev@aliel.fr>
^ permalink raw reply
* [PATCH v2 2/3] arm64: dts: amlogic: t7: Add UART controllers nodes
From: Ronald Claveau @ 2026-04-16 8:54 UTC (permalink / raw)
To: Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-kernel, linux-amlogic, devicetree, linux-kernel,
Ronald Claveau
In-Reply-To: <20260416-add-bluetooth-t7-vim4-v2-0-9a57098fd055@aliel.fr>
Add device tree nodes for UART B through F (serial@7a000 to
serial@82000), completing the UART controller description for the T7
SoC. Each node includes the peripheral clock.
While at it, move the uart_a node to its correct position in the
bus address order (0x78000) to comply with the DT requirement that
nodes be sorted by their reg address. Complete the
uart_a node with its peripheral clock (CLKID_SYS_UART_A) and the
associated clock-names, matching the vendor default clock assignment,
consistent with the other UART nodes.
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi | 61 +++++++++++++++++++++++++----
1 file changed, 54 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
index 4a55d9641bc9b..81c26b1e3e7a4 100644
--- a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
+++ b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
@@ -577,13 +577,6 @@ gpio_intc: interrupt-controller@4080 {
<10 11 12 13 14 15 16 17 18 19 20 21>;
};
- uart_a: serial@78000 {
- compatible = "amlogic,t7-uart", "amlogic,meson-s4-uart";
- reg = <0x0 0x78000 0x0 0x18>;
- interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
- status = "disabled";
- };
-
gp0: clock-controller@8080 {
compatible = "amlogic,t7-gp0-pll";
reg = <0x0 0x8080 0x0 0x20>;
@@ -713,6 +706,60 @@ pwm_ao_cd: pwm@60000 {
status = "disabled";
};
+ uart_a: serial@78000 {
+ compatible = "amlogic,t7-uart", "amlogic,meson-s4-uart";
+ reg = <0x0 0x78000 0x0 0x18>;
+ interrupts = <GIC_SPI 168 IRQ_TYPE_EDGE_RISING>;
+ clocks = <&xtal>, <&clkc_periphs CLKID_SYS_UART_A>, <&xtal>;
+ clock-names = "xtal", "pclk", "baud";
+ status = "disabled";
+ };
+
+ uart_b: serial@7a000 {
+ compatible = "amlogic,t7-uart", "amlogic,meson-s4-uart";
+ reg = <0x0 0x7a000 0x0 0x18>;
+ interrupts = <GIC_SPI 169 IRQ_TYPE_EDGE_RISING>;
+ clocks = <&xtal>, <&clkc_periphs CLKID_SYS_UART_B>, <&xtal>;
+ clock-names = "xtal", "pclk", "baud";
+ status = "disabled";
+ };
+
+ uart_c: serial@7c000 {
+ compatible = "amlogic,t7-uart", "amlogic,meson-s4-uart";
+ reg = <0x0 0x7c000 0x0 0x18>;
+ interrupts = <GIC_SPI 170 IRQ_TYPE_EDGE_RISING>;
+ clocks = <&xtal>, <&clkc_periphs CLKID_SYS_UART_C>, <&xtal>;
+ clock-names = "xtal", "pclk", "baud";
+ status = "disabled";
+ };
+
+ uart_d: serial@7e000 {
+ compatible = "amlogic,t7-uart", "amlogic,meson-s4-uart";
+ reg = <0x0 0x7e000 0x0 0x18>;
+ interrupts = <GIC_SPI 171 IRQ_TYPE_EDGE_RISING>;
+ clocks = <&xtal>, <&clkc_periphs CLKID_SYS_UART_D>, <&xtal>;
+ clock-names = "xtal", "pclk", "baud";
+ status = "disabled";
+ };
+
+ uart_e: serial@80000 {
+ compatible = "amlogic,t7-uart", "amlogic,meson-s4-uart";
+ reg = <0x0 0x80000 0x0 0x18>;
+ interrupts = <GIC_SPI 172 IRQ_TYPE_EDGE_RISING>;
+ clocks = <&xtal>, <&clkc_periphs CLKID_SYS_UART_E>, <&xtal>;
+ clock-names = "xtal", "pclk", "baud";
+ status = "disabled";
+ };
+
+ uart_f: serial@82000 {
+ compatible = "amlogic,t7-uart", "amlogic,meson-s4-uart";
+ reg = <0x0 0x82000 0x0 0x18>;
+ interrupts = <GIC_SPI 173 IRQ_TYPE_EDGE_RISING>;
+ clocks = <&xtal>, <&clkc_periphs CLKID_SYS_UART_F>, <&xtal>;
+ clock-names = "xtal", "pclk", "baud";
+ status = "disabled";
+ };
+
sd_emmc_a: mmc@88000 {
compatible = "amlogic,t7-mmc", "amlogic,meson-axg-mmc";
reg = <0x0 0x88000 0x0 0x800>;
--
2.49.0
^ permalink raw reply related
* [PATCH v2 3/3] arm64: dts: amlogic: t7: khadas-vim4: Enable Bluetooth
From: Ronald Claveau @ 2026-04-16 8:54 UTC (permalink / raw)
To: Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-kernel, linux-amlogic, devicetree, linux-kernel,
Ronald Claveau
In-Reply-To: <20260416-add-bluetooth-t7-vim4-v2-0-9a57098fd055@aliel.fr>
Enable UART C on the Khadas VIM4 board and attach the BCM43438
compatible Bluetooth controller to it. The node configures the RTS/CTS
hardware flow control, the associated pinmux, the power supplies (vddao_3v3
and vddao_1v8), the 32 kHz LPO clock shared with the wifi32k fixed
clock, and the GPIO lines used for host wakeup, device wakeup and
shutdown.
Remove clocks and clock-names for UART A, as they are defined in DTSI.
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
.../dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts b/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
index 69d6118ba57e7..8ea7ae609fbd5 100644
--- a/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
+++ b/arch/arm64/boot/dts/amlogic/amlogic-t7-a311d2-khadas-vim4.dts
@@ -250,6 +250,23 @@ &sd_emmc_c {
&uart_a {
status = "okay";
- clocks = <&xtal>, <&xtal>, <&xtal>;
- clock-names = "xtal", "pclk", "baud";
+};
+
+&uart_c {
+ status = "okay";
+ pinctrl-0 = <&uart_c_pins>;
+ pinctrl-names = "default";
+ uart-has-rtscts;
+
+ bluetooth {
+ compatible = "brcm,bcm43438-bt";
+ shutdown-gpios = <&gpio GPIOX_17 GPIO_ACTIVE_HIGH>;
+ host-wakeup-gpios = <&gpio GPIOX_18 GPIO_ACTIVE_HIGH>;
+ device-wakeup-gpios = <&gpio GPIOX_19 GPIO_ACTIVE_HIGH>;
+ max-speed = <3000000>;
+ clocks = <&wifi32k>;
+ clock-names = "lpo";
+ vbat-supply = <&vddao_3v3>;
+ vddio-supply = <&vddao_1v8>;
+ };
};
--
2.49.0
^ permalink raw reply related
* [PATCH v2 1/3] arm64: dts: amlogic: t7: Add uart_c pinctrl pins group
From: Ronald Claveau @ 2026-04-16 8:54 UTC (permalink / raw)
To: Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-kernel, linux-amlogic, devicetree, linux-kernel,
Ronald Claveau
In-Reply-To: <20260416-add-bluetooth-t7-vim4-v2-0-9a57098fd055@aliel.fr>
Add the pin multiplexing configuration for UART C (TX, RX, CTS, RTS)
in the T7 SoC pinctrl node, required to route the UART C signals
through the correct pads before enabling the controller.
Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
index 7fe72c94ed623..4a55d9641bc9b 100644
--- a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
+++ b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
@@ -553,6 +553,18 @@ mux {
bias-pull-up;
};
};
+
+ uart_c_pins: uart-c {
+ mux {
+ groups = "uart_c_tx",
+ "uart_c_rx",
+ "uart_c_cts",
+ "uart_c_rts";
+ bias-pull-up;
+ output-high;
+ function = "uart_c";
+ };
+ };
};
gpio_intc: interrupt-controller@4080 {
--
2.49.0
^ permalink raw reply related
* Re: [PATCH net] net: airoha: Fix possible TX queue stall in airoha_qdma_tx_napi_poll()
From: Paolo Abeni @ 2026-04-16 8:44 UTC (permalink / raw)
To: Lorenzo Bianconi, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski
Cc: linux-arm-kernel, linux-mediatek, netdev
In-Reply-To: <20260413-airoha-txq-potential-stall-v1-1-7830363b1543@kernel.org>
On 4/13/26 10:29 AM, Lorenzo Bianconi wrote:
> Since multiple net_device TX queues can share the same hw QDMA TX queue,
> there is no guarantee we have inflight packets queued in hw belonging to a
> net_device TX queue stopped in the xmit path because hw QDMA TX queue
> can be full. In this corner case the net_device TX queue will never be
> re-activated. In order to avoid any potential net_device TX queue stall,
> we need to wake all the net_device TX queues feeding the same hw QDMA TX
> queue in airoha_qdma_tx_napi_poll routine.
>
> Fixes: 23020f0493270 ("net: airoha: Introduce ethernet support for EN7581 SoC")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> drivers/net/ethernet/airoha/airoha_eth.c | 30 ++++++++++++++++++++++++++----
> 1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 9e995094c32a..e7610f36b8e4 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -855,6 +855,19 @@ static int airoha_qdma_init_rx(struct airoha_qdma *qdma)
> return 0;
> }
>
> +static void airoha_qdma_wake_tx_queues(struct airoha_qdma *qdma)
> +{
> + struct airoha_eth *eth = qdma->eth;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(eth->ports); i++) {
> + struct airoha_gdm_port *port = eth->ports[i];
> +
> + if (port && port->qdma == qdma)
> + netif_tx_wake_all_queues(port->dev);
> + }
> +}
> +
> static int airoha_qdma_tx_napi_poll(struct napi_struct *napi, int budget)
> {
> struct airoha_tx_irq_queue *irq_q;
> @@ -931,12 +944,21 @@ static int airoha_qdma_tx_napi_poll(struct napi_struct *napi, int budget)
>
> txq = netdev_get_tx_queue(skb->dev, queue);
> netdev_tx_completed_queue(txq, 1, skb->len);
> - if (netif_tx_queue_stopped(txq) &&
> - q->ndesc - q->queued >= q->free_thr)
> - netif_tx_wake_queue(txq);
> -
> dev_kfree_skb_any(skb);
> }
> +
> + if (q->ndesc - q->queued == q->free_thr) {
Sashiko says:
---
Can this exact equality check cause a permanent TX queue stall?
The previous logic checked if the free space was greater than or equal
to q->free_thr. If the xmit path stops the queue because the free space
drops to exactly q->free_thr, the hardware queue will have exactly
q->free_thr free slots.
When the NAPI poll routine subsequently reaps a completed descriptor,
q->queued is decremented, increasing the free space to q->free_thr + 1.
Since the free space is no longer exactly equal to the threshold, this
condition evaluates to false.
As NAPI continues to reap more descriptors, the free space strictly
increases, meaning the exact equality check will never evaluate to true
and the netdev TX queue will remain permanently stalled.
---
Please, try to triage sashiko comments proactively. Especially on NIC
drivers, validating the AI statements is extremely cumbersome for the
maintainers.
Thanks,
Paolo
^ permalink raw reply
* [PATCH] serial: mxs-auart: Compare the return value of gpiod_get_direction against GPIO_LINE_DIRECTION_IN
From: Nikola Z. Ivanov @ 2026-04-16 8:32 UTC (permalink / raw)
To: gregkh, jirislaby, Frank.Li, s.hauer, kernel, festevam
Cc: linux-kernel, linux-serial, imx, linux-arm-kernel,
Nikola Z. Ivanov
The GPIO_LINE_DIRECTION_* definitions have just recently been exposed to
gpio consumers.h by breaking them out in a separate defs.h file.
Use this to validate the gpio direction instead of the hard-coded literal.
Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com>
---
drivers/tty/serial/mxs-auart.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index cc65c9fb6446..6c6df4d5c21f 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -1519,7 +1519,7 @@ static int mxs_auart_init_gpios(struct mxs_auart_port *s, struct device *dev)
for (i = 0; i < UART_GPIO_MAX; i++) {
gpiod = mctrl_gpio_to_gpiod(s->gpios, i);
- if (gpiod && (gpiod_get_direction(gpiod) == 1))
+ if (gpiod && (gpiod_get_direction(gpiod) == GPIO_LINE_DIRECTION_IN))
s->gpio_irq[i] = gpiod_to_irq(gpiod);
else
s->gpio_irq[i] = -EINVAL;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2 1/8] dt-bindings: mfd: khadas: Add new compatible for Khadas VIM4 MCU
From: Ronald Claveau @ 2026-04-16 8:25 UTC (permalink / raw)
To: Rob Herring
Cc: Neil Armstrong, Lee Jones, Krzysztof Kozlowski, Conor Dooley,
Andi Shyti, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Beniamino Galvani, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
Lukasz Luba, Liam Girdwood, Mark Brown, linux-amlogic, devicetree,
linux-kernel, linux-i2c, linux-arm-kernel, linux-pm
In-Reply-To: <20260415214815.GA602572-robh@kernel.org>
On 4/15/26 11:48 PM, Rob Herring wrote:
> On Fri, Apr 03, 2026 at 06:08:34PM +0200, Ronald Claveau wrote:
>> The Khadas VIM4 MCU register is slightly different
>> from previous boards' MCU.
>> This board also features a switchable power source for its fan.
>>
>> Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
>> ---
>> Documentation/devicetree/bindings/mfd/khadas,mcu.yaml | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml b/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
>> index 084960fd5a1fd..67769ef5d58b1 100644
>> --- a/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
>> @@ -18,6 +18,7 @@ properties:
>> compatible:
>> enum:
>> - khadas,mcu # MCU revision is discoverable
>
> The revision is no longer discoverable as was claimed?
>
The firmware revision is still discoverable, and via the same register,
but the VIM4 MCU has a different register layout (eg: no DEVICE_NO
register). The new compatible is needed to describe a different MCU
variant, not a different revision of the same MCU.
I will remove the comment as it is confusing with new boards.
>> + - khadas,vim4-mcu
>>
>> "#cooling-cells": # Only needed for boards having FAN control feature
>> const: 2
>> @@ -25,6 +26,10 @@ properties:
>> reg:
>> maxItems: 1
>>
>> + fan-supply:
>> + description: Phandle to the regulator that powers the fan.
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> +
>> required:
>> - compatible
>> - reg
>>
>> --
>> 2.49.0
>>
--
Best regards,
Ronald
^ permalink raw reply
* Re: [PATCH v4 0/4] Introduce Allwinner H616 PWM controller
From: Richard GENOUD @ 2026-04-16 8:22 UTC (permalink / raw)
To: Paul Kocialkowski
Cc: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Philipp Zabel, Thomas Petazzoni, John Stultz, Joao Schim,
linux-pwm, devicetree, linux-arm-kernel, linux-sunxi,
linux-kernel
In-Reply-To: <adfe2z2YeBxm_6oR@shepard>
Le 09/04/2026 à 19:16, Paul Kocialkowski a écrit :
> Hi Richard,
>
> On Thu 05 Mar 26, 10:19, Richard Genoud wrote:
>> Allwinner H616 PWM controller is quite different from the A10 one.
>
> As I've mentionned before, this PWM controller is not specific to the H616
> but also appears in other chips, so the name of the driver and registers
> should not mention H616.
>
> After further investigation, I can see multiple versions of this new PWM IP
> being used in different chips, starting with the R40/V40 (sun8iw11) in 2016.
>
> The latest downstream BSP driver has a list of the different generations:
> https://github.com/radxa/allwinner-bsp/blob/cubie-aiot-v1.4.6/drivers/pwm/pwm-sunxi.c#L1901
>
> We have a first generation called v100/v101 for the following chips:
> H616, R328 and R40. A second generation is called v200 and brings slight
> register layout differences for A133, D1/T113-S3 and V851. Subsequent
> iterations (v201-5) are used in more recent chips like A527 and A733 and
> seem register-compatible with v200 (from a quick look).
>
> So what I suggest here is to rename the driver "sun8i-pwm" and eventually add
> a list of generations to the driver and different registers when needed, with
> an appropriate suffix in their name.
>
> But since you're currently only dealing with H616, this work can be done later
> when introducing support for more chips.
ok, I'm fine with that :)
>
>> It can drive 6 PWM channels, and like for the A10, each channel has a
>> bypass that permits to output a clock, bypassing the PWM logic, when
>> enabled.
>>
>> But, the channels are paired 2 by 2, sharing a first set of
>> MUX/prescaler/gate.
>> Then, for each channel, there's another prescaler (that will be bypassed
>> if the bypass is enabled for this channel).
>>
>> It looks like that:
>> _____ ______ ________
>> OSC24M --->| | | | | |
>> APB1 ----->| Mux |--->| Gate |--->| /div_m |-----> PWM_clock_src_xy
>> |_____| |______| |________|
>> ________
>> | |
>> +->| /div_k |---> PWM_clock_x
>> | |________|
>> | ______
>> | | |
>> +-->| Gate |----> PWM_bypass_clock_x
>> | |______|
>> PWM_clock_src_xy -----+ ________
>> | | |
>> +->| /div_k |---> PWM_clock_y
>> | |________|
>> | ______
>> | | |
>> +-->| Gate |----> PWM_bypass_clock_y
>> |______|
>>
>> Where xy can be 0/1, 2/3, 4/5
>>
>> PWM_clock_x/y serve for the PWM purpose.
>> PWM_bypass_clock_x/y serve for the clock-provider purpose.
>> The common clock framework has been used to manage those clocks.
>>
>> This PWM driver serves as a clock-provider for PWM_bypass_clocks.
>> This is needed for example by the embedded AC300 PHY which clock comes
>> from PMW5 pin (PB12).
>>
>> Usually, to get a clock from a PWM driver, we use the pwm-clock driver
>> so that the PWM driver doesn't need to be a clk-provider itself.
>> While this works in most cases, here it just doesn't.
>> That's because the pwm-clock request a period from the PWM driver,
>> without any clue that it actually wants a clock at a specific frequency,
>> and not a PWM signal with duty cycle capability.
>
> From what I understand the pwm-clock driver will either assume a fixed rate
> set in device-tree or deduce the rate from the pwm period. In any case it will
> check that the pwm period (which it cannot change) is the same as the requested
> clock period.
>
> So I agree that pwm-clock is unable to change the clock rate at runtime and will
> just use whatever frequency the pwm is running at (which is typically set
> in the device-tree consumer property).
>
>> So, the PWM driver doesn't know if it can use the bypass or not, it
>> doesn't even have the real accurate frequency information (23809524 Hz
>> instead of 24MHz) because PWM drivers only deal with periods.
>
> I agree that the driver needs to register as a proper clock provider in
> addition to pwm. But what happens if the same PWM clock is requested both from
> the clk side and the pwm side?
The first to request it is the winner :)
The other ones will receive a -EBUSY
In h616_pwm_request() and h616_pwm_of_clk_get(), the channel mode is
checked, and if it's free to use, it's set as either PWM or CLK mode so
that it can't be requested a second time.
>
>> With pwm-clock, we loose a precious information along the way (that we
>> actually want a clock and not a PWM signal).
>> That's ok with simple PWM drivers that don't have multiple input clocks,
>> but in this case, without this information, we can't know for sure which
>> clock to use.
>> And here, for instance, if we ask for a 24MHz clock, pwm-clock will
>> requests 42ns (assigned-clocks doesn't help for that matter). The logic
>> is to select the highest clock (100MHz) with no prescaler and a duty
>> cycle value of 2/4 => we have 25MHz instead of 24MHz.
>> And that's a perfectly fine choice for a PMW, because we still can
>> change the duty cycle in the range [0-4]/4.
>> But obviously for a clock, we don't care about the duty cycle, but more
>> about the clock accuracy.
>>
>> And actually, this PWM is really a PWM AND a real clock when the bypass
>> is set.
>
> Make sense to me.
>
>> This series is based onto v6.19-rc4
>>
>> NB: checkpatch is not happy with patch 2, but it's a false positive.
>> It doesn't detect that PWM_XY_SRC_MUX/GATE/DIV are structures, but as
>> it's more readable like that, I prefer keeping it that way.
>>
>> NB2: for geopolitical reasons, I didn't re-use the old series that Paul
>> was referring to.
>>
>> Changes since v3:
>> - gather Acked-by/Tested-by
>> - fix cast from pointer to integer of different size (kernel test robot
>> with arc platform)
>> - add devm_action for clk_hw_unregister_composite as suggested by Philipp
>> - remove now unused pwm_remove as suggested by Philipp
>>
>> Changes since v2:
>> - use U32_MAX instead of defining UINT32_MAX
>> - add a comment on U32_MAX usage in clk_round_rate()
>> - change clk_table_div_m (use macros)
>> - fix formatting (double space, superfluous comma, extra line feed)
>> - fix the parent clock order
>> - simplify code by using scoped_guard()
>> - add missing const in to_h616_pwm_chip() and rename to
>> h616_pwm_from_chip()
>> - add/remove missing/superflous error messages
>> - rename cnt->period_ticks, duty_cnt->duty_ticks
>> - fix PWM_PERIOD_MAX
>> - add .remove() callback
>> - fix DIV_ROUND_CLOSEST_ULL->DIV_ROUND_UP_ULL
>> - add H616_ prefix
>> - protect _reg in macros
>> - switch to waveforms instead of apply/get_state
>> - shrink struct h616_pwm_channel
>> - rebase on v6.19-rc4
>>
>> Changes since v1:
>> - rebase onto v6.19-rc1
>> - add missing headers
>> - remove MODULE_ALIAS (suggested by Krzysztof)
>> - use sun4i-pwm binding instead of creating a new one (suggested by Krzysztof)
>> - retrieve the parent clocks from the devicetree
>> - switch num_parents to unsigned int
>>
>> Richard Genoud (4):
>> dt-bindings: pwm: allwinner: add h616 pwm compatible
>> pwm: sun50i: Add H616 PWM support
>> arm64: dts: allwinner: h616: add PWM controller
>> MAINTAINERS: Add entry on Allwinner H616 PWM driver
>>
>> .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 19 +-
>> MAINTAINERS | 5 +
>> .../arm64/boot/dts/allwinner/sun50i-h616.dtsi | 47 +
>> drivers/pwm/Kconfig | 12 +
>> drivers/pwm/Makefile | 1 +
>> drivers/pwm/pwm-sun50i-h616.c | 936 ++++++++++++++++++
>> 6 files changed, 1019 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/pwm/pwm-sun50i-h616.c
>>
>>
>> base-commit: 11439c4635edd669ae435eec308f4ab8a0804808
>
Regards,
Richard
^ permalink raw reply
* Re: [PATCH net,v2 1/1] net: stmmac: Update default_an_inband before passing value to phylink_config
From: KhaiWenTan @ 2026-04-16 8:22 UTC (permalink / raw)
To: Paolo Abeni, andrew+netdev, davem, edumazet, kuba,
mcoquelin.stm32, alexandre.torgue, rmk+kernel, maxime.chevallier,
ovidiu.panait.rb, vladimir.oltean
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel,
yoong.siang.song, hong.aun.looi, khai.wen.tan
In-Reply-To: <72d1b0b7-c8df-463e-a2d9-bf5ff04ba33c@redhat.com>
On 4/16/2026 4:12 PM, Paolo Abeni wrote:
> On 4/13/26 4:03 AM, KhaiWenTan wrote:
>> get_interfaces() will update both the plat->phy_interfaces and
>> mdio_bus_data->default_an_inband based on reading a SERDES register. As
>> get_interfaces() will be called after default_an_inband had already been
>> read, dwmac-intel regressed as a result with incorrect default_an_inband
>> value in phylink_config.
>>
>> Therefore, we moved the priv->plat->get_interfaces() to be executed first
>> before assigning mdio_bus_data->default_an_inband to
>> config->default_an_inband to ensure default_an_inband is in correct value.
>>
>> Fixes: d3836052fe09 ("net: stmmac: intel: convert speed_mode_2500() to get_interfaces()")
>> Signed-off-by: KhaiWenTan <khai.wen.tan@linux.intel.com>
> Since Jakub sent the net-next PR and forwarded the trees, this patch
> does not apply anymore. Please rebase and repost. You can retain
> Russell's reviewed-by tag.
>
> Thanks,
>
> Paolo
Thank you Paolo, will be rebasing the patch and update a v3.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox