Re: [PATCH 2/2] cpuidle: governors: Move the files to the upper directory

From: Daniel Lezcano
Date: Fri Oct 21 2016 - 09:25:33 EST


On Fri, Oct 21, 2016 at 03:22:18PM +0200, Rafael J. Wysocki wrote:
> On Fri, Oct 21, 2016 at 3:09 PM, Daniel Lezcano
> <daniel.lezcano@xxxxxxxxxx> wrote:
> > On Fri, Oct 21, 2016 at 02:47:22PM +0200, Rafael J. Wysocki wrote:
> >> On Wed, Oct 5, 2016 at 9:33 AM, Daniel Lezcano
> >> <daniel.lezcano@xxxxxxxxxx> wrote:
> >> > Currently the different governors are stored in the subdir
> >> > 'governors'. That is not a problem.
> >> >
> >> > However, that forces to declare some private structure in the
> >> > include/linux/cpuidle.h header because these governor files
> >> > don't have access to the private 'cpuidle.h' located in
> >> > drivers/cpuidle.
> >> >
> >> > Instead of having the governors in the separate directory, move
> >> > them along with the drivers and prefix them with 'governor-',
> >> > that allows to do a proper cleanup in the cpuidle headers.
> >>
> >> While I'm not particularly against this change, I'm sort of wondering
> >> about the reason.
> >>
> >> What in particular would be wrong with doing
> >>
> >> #include "../cpuidle.h"
> >>
> >> in a governor .c file?
> >
> > Hi Rafael,
> >
> > there is nothing wrong by doing this relative inclusion. It is an alternative
> > to the proposed patch. I personally don't like relative inclusion but it is
> > a matter of taste and I am perfectly fine to resend the patch by just moving
> > the structure to the private header and change the inclusion.
> >
> > On the other side, the cpufreq susbsytem has all the governors along with the
> > drivers in the same directory, so perhaps it makes sense to have a similar files
> > organization.
> >
> > Actually, I'm fine with both approaches. Up to you to decide.
>
> I'm thinking let's keep the code where it is in case people depend on
> the current location somehow (ie. have patches out of the tree or
> similar). We can still move it later if need be.

Ok, I will resend the patch [2/2] by moving the structure from the exported header
to the private header and add the relative inclusion path.

Thanks.

-- Daniel