Re: Re: [PATCH v4 4/4] PM: Devfreq: Add Exynos5-bus devfreq driver for Exynos5250

Re: Re: [PATCH v4 4/4] PM: Devfreq: Add Exynos5-bus devfreq driver for Exynos5250

[MyungJoo Ham]

> Hi,
> 
> > Sorry for being late.
> >
> > One concern is that I cannot apply Patch 1/4 directly as the .dts file isn't
> > available to me.
> > Do you intend to apply that patch to another tree and apply the other three
> > to devfreq tree?
> I have been merging the devfreq tree into Kgene's tree for my testing. I am not
> sure how this should get in, any suggestions ?

I'd suggest to send Patch 1/4 to Kgene and get it applied first if that patch is preliminary to the other patchset. If it is not, we can go seperated and independently.

> >
> > Another is the location of exynos_ppmu.h.
> > Who is going to refer exynos_ppmu.h?
> > If Exynos devfreq drivers are the only files to refer, wouldn't it be better
> > located at drivers/devfreq/exynos/ ?
> >
> > Or do you intend to let someone else (drivers located not in
> > drivers/devfreq) access exynos-ppmu?
> > (If so, who's going to be? and that means we are going to have
> > include/linux/devfreq/, I don't think it'd be appropriate to populate
> > include/linux with device driver specific headers)
> In an earlier comment on this patch Olof suggested I place it in
> include/linux/. It is going to be
> used only by Exynos and I will move it into drivers/devfreq/exynos as
> per your suggestion.

If it is to be used by Exynos non-devfreq drivers (e.g., Exynos ADC driver in drivers/iio/...), it should be stting on include/linux/devfreq/. If it is to be used by Exynos devfreq drivers only, it'd be better located at drivers/devfreq/exynos/. It appears to be still compatible with Olof's suggestion:

(quoting Olof) "Local defines that are used in just one C file can either go in that file, or in a header file that sits next to it instead of in the shared directory."



> >
> >
> >
> > And, there are errors (we do not have __devinit/__devexit anymore):
> >
> >   CC      drivers/devfreq/exynos/exynos5_bus.o
> > drivers/devfreq/exynos/exynos5_bus.c:315:18: error: expected =, ,, ;, asm or
> > __attribute__ before int
> > drivers/devfreq/exynos/exynos5_bus.c:430:18: error: expected =, ,, ;, asm or
> > __attribute__ before int
> > drivers/devfreq/exynos/exynos5_bus.c:462:11: error:
> > exynos5_busfreq_int_probe undeclared here (not in a function)
> > drivers/devfreq/exynos/exynos5_bus.c:463:2: error: implicit declaration of
> > function __devexit_p [-Werror=implicit-function-declaration]
> > drivers/devfreq/exynos/exynos5_bus.c:463:24: error:
> > exynos5_busfreq_int_remove undeclared here (not in a function)
> > drivers/devfreq/exynos/exynos5_bus.c:235:35: warning:
> > exynos5_devfreq_int_profile defined but not used [-Wunused-variable]
> > drivers/devfreq/exynos/exynos5_bus.c:243:12: warning:
> > exynos5250_init_int_tables defined but not used [-Wunused-function]
> > drivers/devfreq/exynos/exynos5_bus.c:259:12: warning:
> > exynos5_busfreq_int_pm_notifier_event defined but not used
> > [-Wunused-function]
> > cc1: some warnings being treated as errors
> I will fix these and re-post.
> >
> >
> >
> > Cheers,
> > MyungJoo.
> >
> Abhilash
> 

Re: Re: [PATCH v4 4/4] PM: Devfreq: Add Exynos5-bus devfreq driver for Exynos5250

[Abhilash Kesavan]

Hi,

On Mon, Feb 4, 2013 at 1:49 PM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
>> Hi,
>>
>> > Sorry for being late.
>> >
>> > One concern is that I cannot apply Patch 1/4 directly as the .dts file isn't
>> > available to me.
>> > Do you intend to apply that patch to another tree and apply the other three
>> > to devfreq tree?
>> I have been merging the devfreq tree into Kgene's tree for my testing. I am not
>> sure how this should get in, any suggestions ?
>
> I'd suggest to send Patch 1/4 to Kgene and get it applied first if that patch is preliminary to the other patchset. If it is not, we can go seperated and independently.
Thanks for the suggestion. Kgene is a recipient on patches 1 and 2. I
am hoping he'll pick it up soon. However, the devfreq patches are not
dependent on them and can be applied independently.
>
>> >
>> > Another is the location of exynos_ppmu.h.
>> > Who is going to refer exynos_ppmu.h?
>> > If Exynos devfreq drivers are the only files to refer, wouldn't it be better
>> > located at drivers/devfreq/exynos/ ?
>> >
>> > Or do you intend to let someone else (drivers located not in
>> > drivers/devfreq) access exynos-ppmu?
>> > (If so, who's going to be? and that means we are going to have
>> > include/linux/devfreq/, I don't think it'd be appropriate to populate
>> > include/linux with device driver specific headers)
>> In an earlier comment on this patch Olof suggested I place it in
>> include/linux/. It is going to be
>> used only by Exynos and I will move it into drivers/devfreq/exynos as
>> per your suggestion.
>
> If it is to be used by Exynos non-devfreq drivers (e.g., Exynos ADC driver in drivers/iio/...), it should be stting on include/linux/devfreq/. If it is to be used by Exynos devfreq drivers only, it'd be better located at drivers/devfreq/exynos/. It appears to be still compatible with Olof's suggestion:
>
> (quoting Olof) "Local defines that are used in just one C file can either go in that file, or in a header file that sits next to it instead of in the shared directory."
Understood, updated patch 4/4 in a bit.
>
>

 Abhilash

RE: Re: [PATCH v4 4/4] PM: Devfreq: Add Exynos5-bus devfreq driver for Exynos5250

[Kukjin Kim]

Abhilash Kesavan wrote:

> >> > One concern is that I cannot apply Patch 1/4 directly as the .dts
> file isn't
> >> > available to me.
> >> > Do you intend to apply that patch to another tree and apply the other
> three
> >> > to devfreq tree?
> >> I have been merging the devfreq tree into Kgene's tree for my testing.
> I am not
> >> sure how this should get in, any suggestions ?
> >
> > I'd suggest to send Patch 1/4 to Kgene and get it applied first if that
> patch is preliminary to the other patchset. If it is not, we can go
> seperated and independently.

> Thanks for the suggestion. Kgene is a recipient on patches 1 and 2. I
> am hoping he'll pick it up soon. However, the devfreq patches are not
> dependent on them and can be applied independently.

Just note, as I commented, arch side patches should be re-worked and
current devfreq in mainline won't cause any problems without the arch side.
And I discussed with Abhilash about that, he will finish arch side re-
posting next time.

Thanks.

- Kukjin