linux-kernel - Re: [PATCH v4 3/4] media: raspberrypi: Add support for RP1-CFE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <csfjh3rdhdieemasmfjmvuy4uaypvbct7y7vm2zogdgmglsc56@u7yzkyrbuthz>
Date: Tue, 10 Sep 2024 12:18:50 +0200
From: Jacopo Mondi <jacopo.mondi@...asonboard.com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Cc: Jacopo Mondi <jacopo.mondi@...asonboard.com>, 
	Sakari Ailus <sakari.ailus@...ux.intel.com>, Laurent Pinchart <laurent.pinchart@...asonboard.com>, 
	Mauro Carvalho Chehab <mchehab@...nel.org>, Raspberry Pi Kernel Maintenance <kernel-list@...pberrypi.com>, 
	Rob Herring <robh+dt@...nel.org>, Krzysztof Kozlowski <krzk@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, Florian Fainelli <florian.fainelli@...adcom.com>, 
	Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>, oe-kbuild-all@...ts.linux.dev, linux-media@...r.kernel.org, 
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org, linux-rpi-kernel@...ts.infradead.org, 
	linux-arm-kernel@...ts.infradead.org, Naushir Patuck <naush@...pberrypi.com>, 
	Kieran Bingham <kieran.bingham@...asonboard.com>, kernel test robot <lkp@...el.com>
Subject: Re: [PATCH v4 3/4] media: raspberrypi: Add support for RP1-CFE

Hi Tomi

On Tue, Sep 10, 2024 at 12:56:38PM GMT, Tomi Valkeinen wrote:
> On 10/09/2024 12:19, Jacopo Mondi wrote:
>
> > However, I think this current patch is correct (assuming the above
> > reasoning on i2c sensor drivers is correct) and doesn't require
> > CONFIG_PM, so I would be tempted to keep this version.
>
> I think the existence of this discussion alone proves my point that we
> should only support PM-case, unless !PM is a requirement =).
>
> But if you do want to keep !PM:
>
> Is there a reason why not mark the device as active with
> pm_runtime_set_active() after calling pispbe_runtime_resume and before

cargo-cult ?

> accessing the device? That feels like the most logical way to use the
> function, and it would be right regardless whether the core will enable the
> parents before probe() or not.

Possibly more accurate, but there's no guarantee it's correct. The
peripheral might have requirements on the clock or power rails
enablement order and some might be managed by the parent. I know we're
talking hypothesis but my point is that there's not correctness
guarantee we can enforce unless the parent is powered up when the
device probes ?

Anyway, I'll defer the call to the group: either keep the patch as it
is right now on the list, or go full runtime_pm. I understand there is
no reason to care about !CONFIG_PM but somehow I feel "bad" in listing
it as a dependency if the peripheral can actually work without it.
Maybe I should just ignore that feeling ?


>
> And not related to the BE or CFE drivers, but it strikes me odd that to
> support PM and !PM we need to play with these tricks. I think the core
> should just do the right thing if the driver does pm_runtime_get_sync() even
> with !PM (although maybe the time has passed to be able to do that).

I wish that was the case.

>
>  Tomi
>