All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Onur Özkan" <work@onurozkan.dev>
To: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Boris Brezillon <boris.brezillon@collabora.com>,
	Mark Brown <broonie@kernel.org>,
	daniel.almeida@collabora.com, aliceryhl@google.com,
	dakr@kernel.org, airlied@gmail.com, simona@ffwll.ch,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	lgirdwood@gmail.com, ojeda@kernel.org,
	rust-for-linux@vger.kernel.org
Subject: Re: [PATCH v2 1/1] drm/tyr: make SRAM supply optional like panthor
Date: Fri, 13 Feb 2026 15:59:37 +0300	[thread overview]
Message-ID: <20260213155937.6af75786@nimda> (raw)
In-Reply-To: <aY8V8xlTv0UAdyQz@e142607>

Hi Liviu,

Thank you for reviewing this.

On Fri, 13 Feb 2026 12:15:47 +0000
Liviu Dudau <Liviu.Dudau@arm.com> wrote:

> On Thu, Feb 12, 2026 at 09:30:10PM +0300, Onur Özkan wrote:
> > On Thu, 12 Feb 2026 14:51:34 +0100
> > Boris Brezillon <boris.brezillon@collabora.com> wrote:
> > 
> > > On Thu, 12 Feb 2026 13:13:31 +0000
> > > Mark Brown <broonie@kernel.org> wrote:
> > > 
> > > > On Thu, Feb 12, 2026 at 01:46:01PM +0100, Boris Brezillon wrote:
> > > > > Mark Brown <broonie@kernel.org> wrote:  
> > > > 
> > > > > > The panthor driver is buggy here and should be fixed, the
> > > > > > driver should treat the supply as mandatory and let the
> > > > > > system integration work out how it's actually made
> > > > > > available.  
> > > > 
> > > > > > Trying to open code this just breaks the error handling.  
> > > > 
> > > > > Maybe, but the thing is, the DT bindings have been accepted
> > > > > already, and it's not something we can easily change. What we
> > > > > can do is make this sram-supply mandatory for new
> > > > > compatibles, but we can't force it on older/existing SoCs
> > > > > without breaking backward-DT compat.  
> > > > 
> > > > In practice you can because we do sub in a dummy regulator for
> > > > missing supplies, it produces a warning but works fine.  If we
> > > > didn't do this it'd be basically impossible to add regulator
> > > > support to anything at any point after the original merge which
> > > > is clearly not reasonable.
> > > 
> > > Okay, I guess we need to fix panthor then...
> > > 
> > 
> > That + updating the log to something like "sram-supply is missing in
> > the DT" would be quite better I think. It would make the issue more
> > obvious and convey that the DT file is expected to configure that
> > field explicitly. With the current log message, not many people will
> > understand the problem at a glance.
> > 
> > As for the bug I described in this patch, we can proceed with the
> > alternative solution (updating the DT file) that I mentioned in the
> > Zulip thread (the link is included in the patch). Which is this
> > simple diff:
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dtsi
> > b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dtsi
> > index dafad29f9854..a30339fd2c10 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588s-orangepi-5.dtsi
> > @@ -177,6 +177,7 @@ &gmac1_rgmii_clk
> > 
> >  &gpu {
> >         mali-supply = <&vdd_gpu_s0>;
> > +       sram-supply = <&vdd_gpu_mem_s0>;
> >         status = "okay";
> >  };
> > 
> > @@ -537,7 +538,7 @@ rk806_dvs3_null: dvs3-null-pins {
> >                 };
> > 
> >                 regulators {
> > -                       vdd_gpu_s0: dcdc-reg1 {
> > +                       vdd_gpu_s0: vdd_gpu_mem_s0: dcdc-reg1 {
> 
> You don't need to define a new label, using the same supply for
> mali-supply and sram-supply should be fine.
>

That also works but I never seen that usage (sram-supply =
<&vdd_gpu_s0>) anywhere in the tree and I wanted to follow the
current standard.
 
> >                                 regulator-name = "vdd_gpu_s0";
> >                                 regulator-boot-on;
> >                                 regulator-min-microvolt = <550000>;
> > 
> > Note that this only fixes the issue for the Orange Pi 5. If we want
> > to go further, the same approach should be applied to many other
> > boards as well. I can generate a list of the DT files (using a
> > simple Python script) that need this update over the weekend.
> 
> Yes, please, but bias the script towards using the same regulator as
> mali-supply.
> 
> > 
> > If we want to go even further and fix all DT files to properly
> > include sram-supply we could also enforce that DT files do not omit
> > sram-supply in the future. I am not sure this is strictly necessary
> > but it also doesn't seem consistent to leave things as they are.
> > Right now, some DT files include sram-supply even when there is no
> > separate SRAM rail, while others do not. As a result, some boards
> > will continue to print that annoying log message.
> > 
> > It's not very clear which approach is best.
> 
> I'm in favor of the proposal here, where we make sram-supply
> mandatory for non-"mt8196-mali" SoCs and we patch the DTs to add the
> sram-supply for those.
> 

Cool! 

Regards,
Onur

> Best regards,
> Liviu
> 
> 


  parent reply	other threads:[~2026-02-13 12:59 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-12 10:05 [PATCH v2 0/1] drm/tyr: make SRAM supply optional like panthor Onur Özkan
2026-02-12 10:05 ` [PATCH v2 1/1] " Onur Özkan
2026-02-12 11:34   ` Mark Brown
2026-02-12 12:16     ` Onur Özkan
2026-02-12 12:21       ` Mark Brown
2026-02-12 12:46         ` Boris Brezillon
2026-02-12 13:13           ` Mark Brown
2026-02-12 13:51             ` Boris Brezillon
2026-02-12 18:30               ` Onur Özkan
2026-02-13 12:15                 ` Liviu Dudau
2026-02-13 12:42                   ` Boris Brezillon
2026-02-13 12:59                   ` Onur Özkan [this message]
2026-02-13 10:57         ` Liviu Dudau
2026-02-13 15:54           ` Mark Brown
2026-02-12 12:22       ` Boris Brezillon
2026-02-12 13:10         ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260213155937.6af75786@nimda \
    --to=work@onurozkan.dev \
    --cc=Liviu.Dudau@arm.com \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=boris.brezillon@collabora.com \
    --cc=broonie@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.