On Sun, Mar 28, 2021 at 06:07:10PM +0200, Daniel Lezcano wrote:

On 28/03/2021 13:24, Greg KH wrote:

quoted

On Sun, Mar 28, 2021 at 01:11:30PM +0200, Daniel Lezcano wrote:

quoted

Hi Greg,

On 28/03/2021 08:50, Greg KH wrote:

[ ... ]

quoted

quoted

quoted

And any reason why you are not using "real" struct devices in this
subsystem?  You seem to be rolling your own infrastructure for no good
reason.  I imagine you want sysfs support next, right?

Actually, the framework is on top of powercap, so it has de facto the
sysfs support. On the other side, the dtpm backends are tied with the
device they manage.

So why are they not a "real" device in the driver model?  It looks like
you almost are wanting all of that functionality and are having to
implement it "by hand" instead.

I'm sorry I misunderstanding your point. dtpm is the backend for the
powercap subsystem which is the generic subsystem to do power limitation.

We have:

struct dtpm_cpu {
	struct dtpm dtmp;
	...
}

struct dtpm {
	struct powercap powecap;
};

struct powercap {
	struct device dev;
};

Oh nice.  So you can not use a kref here at all as you already have a
reference counted device controling your structure.  You can not have 2
references trying to control the same structure, that way lies madness
and bugs :(

So why are you trying to add a kref here as the structure already has
support for proper lifetimes?

Right, I'll revisit that part. Thanks for the review.

I've a branch which is pulled by Rafael [1]. These parts are already
merged in the dtpm/next branch but not yet in Rafael's tree.

I would recommend fixing that up if you can rebase it.  If not, you need
to revert it and start over.  I'll be glad to review it if you cc: me on
the patches.

I think a rebase is possible but I would like to avoid that. Would be a
patch on top of the dtpm/next acceptable given your flow with Android ?

This has nothing to do with the Android kernel workflow, sorry.  I am
concerned about proper kernel development and keeping bugs out of it.

thanks,

greg k-h