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 for Android: free password hash cracker in your pocket

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

Message-ID: <ZuAdwpUHQn4fdDjc@kekkonen.localdomain>
Date: Tue, 10 Sep 2024 10:21:54 +0000
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>,
	Jacopo Mondi <jacopo.mondi@...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 Laurent,

On Tue, Sep 10, 2024 at 01:11:37PM +0300, Laurent Pinchart wrote:
> On Tue, Sep 10, 2024 at 12:56:38PM +0300, 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 =).
> 
> For me it proves there's a dire need to document the runtime PM API in a
> way that a human could understand :-)
> 
> > 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 
> > 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.
> 
> Does pm_runtime_set_active() resume the parent ?

No, it simply sets the device's runtime PM status.

> 
> > 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).
> 
> The runtime PM concepts are nice, but the API is wrong in my opinion.
> Instead of being designed to expose the internals of runtime PM, it
> should focus on usability from a driver point of view first.

It's been moving a little bit to that direction, largely with new helper
functions.

I²C devices have been powered on for probe since commit a76e9bd89ae70 .
Relation to runtime PM wasn't considered at the time, apparently.

-- 
Kind regards,

Sakari Ailus