[PATCH v2] coresight: replicator: Use module_platform_driver

[PATCH v2] coresight: replicator: Use module_platform_driver

[Vaishali Thakkar]

Use module_platform_driver for drivers whose init and exit functions
only register and unregister, respectively.

Signed-off-by: Vaishali Thakkar <vthakkar1994@gmail.com>
---
Changes since v1:
	- As per Mathieu's suggestion, remove the comment and
          code of Coccinelle
	- Also, add blank line before module_platform_driver
---
 drivers/hwtracing/coresight/coresight-replicator.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
index 7974b7c..9c18bb5 100644
--- a/drivers/hwtracing/coresight/coresight-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-replicator.c
@@ -184,17 +184,7 @@ static struct platform_driver replicator_driver = {
 	},
 };
 
-static int __init replicator_init(void)
-{
-	return platform_driver_register(&replicator_driver);
-}
-module_init(replicator_init);
-
-static void __exit replicator_exit(void)
-{
-	platform_driver_unregister(&replicator_driver);
-}
-module_exit(replicator_exit);
+module_platform_driver(replicator_driver);
 
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("CoreSight Replicator driver");
-- 
1.9.1

Re: [PATCH v2] coresight: replicator: Use module_platform_driver

[Paul Bolle]

On vr, 2015-07-10 at 01:29 +0530, Vaishali Thakkar wrote:
> --- a/drivers/hwtracing/coresight/coresight-replicator.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator.c

> -static int __init replicator_init(void)
> -{
> -	return platform_driver_register(&replicator_driver);
> -}
> -module_init(replicator_init);
> -
> -static void __exit replicator_exit(void)
> -{
> -	platform_driver_unregister(&replicator_driver);
> -}
> -module_exit(replicator_exit);
> +module_platform_driver(replicator_driver);

coresight-replicator.o is built if CONFIG_CORESIGHT_LINKS_AND_SINKS is
defined. CORESIGHT_LINKS_AND_SINKS is a bool symbol. It depends on
CORESIGHT, which is also a bool symbol. CORESIGHT is a top level symbol,
available on arm and arm64.

I think coresight-replicator.o can only be built-in. So I suggest to use
builtin_platform_driver() instead.

Thanks,


Paul Bolle

Re: [PATCH v2] coresight: replicator: Use module_platform_driver

[Vaishali Thakkar]

On Fri, Jul 10, 2015 at 1:49 AM, Paul Bolle <pebolle@tiscali.nl> wrote:
>
> On vr, 2015-07-10 at 01:29 +0530, Vaishali Thakkar wrote:
> > --- a/drivers/hwtracing/coresight/coresight-replicator.c
> > +++ b/drivers/hwtracing/coresight/coresight-replicator.c
>
> > -static int __init replicator_init(void)
> > -{
> > -     return platform_driver_register(&replicator_driver);
> > -}
> > -module_init(replicator_init);
> > -
> > -static void __exit replicator_exit(void)
> > -{
> > -     platform_driver_unregister(&replicator_driver);
> > -}
> > -module_exit(replicator_exit);
> > +module_platform_driver(replicator_driver);
>
> coresight-replicator.o is built if CONFIG_CORESIGHT_LINKS_AND_SINKS is
> defined. CORESIGHT_LINKS_AND_SINKS is a bool symbol. It depends on
> CORESIGHT, which is also a bool symbol. CORESIGHT is a top level symbol,
> available on arm and arm64.
>
> I think coresight-replicator.o can only be built-in. So I suggest to use
> builtin_platform_driver() instead.
>

I thought about this solution before sending this patch. But I was not
sure about it. Thanks for the explanation. I will send v3 with this
change.

Can I add Suggested By: Paul Bolle <pebolle@tiscali.nl>


>
> Thanks,
>
>
> Paul Bolle




-- 
Vaishali

Re: [PATCH v2] coresight: replicator: Use module_platform_driver

[Paul Bolle]

On vr, 2015-07-10 at 08:53 +0530, Vaishali Thakkar wrote:
> I thought about this solution before sending this patch. But I was not
> sure about it. Thanks for the explanation. I will send v3 with this
> change.
> 
> Can I add Suggested By: Paul Bolle <pebolle@tiscali.nl>

That should be "Suggested-by:". The net effect would be that, if my
suggestion turns out to be unwise, fan mail will also hit my INBOX,
right? Anyhow, fine with me.

By the way, there's more module specific stuff in
drivers/hwtracing/coresight/. And there's no tristate symbol to be found
in its Kconfig file. So I'd guess there are a few other cleanups
possible too, if someone cared enough to have a closer look at that.


Paul Bolle

Re: [PATCH v2] coresight: replicator: Use module_platform_driver

[Vaishali Thakkar]

On Fri, Jul 10, 2015 at 2:30 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
> On vr, 2015-07-10 at 08:53 +0530, Vaishali Thakkar wrote:
>> I thought about this solution before sending this patch. But I was not
>> sure about it. Thanks for the explanation. I will send v3 with this
>> change.
>>
>> Can I add Suggested By: Paul Bolle <pebolle@tiscali.nl>
>
> That should be "Suggested-by:". The net effect would be that, if my
> suggestion turns out to be unwise, fan mail will also hit my INBOX,
> right? Anyhow, fine with me.

Ok. Thanks.

> By the way, there's more module specific stuff in
> drivers/hwtracing/coresight/. And there's no tristate symbol to be found
> in its Kconfig file. So I'd guess there are a few other cleanups
> possible too, if someone cared enough to have a closer look at that.
>

Yes. It seems that introducing something like builtin_amba_driver()
can be useful for files which are using module_amba_driver now . But I'm
not sure if Mathieu is ok with it or not? If it seems useful to him, then I
can go for it.

>
> Paul Bolle



-- 
Vaishali

Re: [PATCH v2] coresight: replicator: Use module_platform_driver

[Mathieu Poirier]

On 10 July 2015 at 05:47, Vaishali Thakkar <vthakkar1994@gmail.com> wrote:
> On Fri, Jul 10, 2015 at 2:30 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
>> On vr, 2015-07-10 at 08:53 +0530, Vaishali Thakkar wrote:
>>> I thought about this solution before sending this patch. But I was not
>>> sure about it. Thanks for the explanation. I will send v3 with this
>>> change.
>>>
>>> Can I add Suggested By: Paul Bolle <pebolle@tiscali.nl>
>>
>> That should be "Suggested-by:". The net effect would be that, if my
>> suggestion turns out to be unwise, fan mail will also hit my INBOX,
>> right? Anyhow, fine with me.
>
> Ok. Thanks.
>
>> By the way, there's more module specific stuff in
>> drivers/hwtracing/coresight/. And there's no tristate symbol to be found
>> in its Kconfig file. So I'd guess there are a few other cleanups
>> possible too, if someone cared enough to have a closer look at that.
>>
>
> Yes. It seems that introducing something like builtin_amba_driver()
> can be useful for files which are using module_amba_driver now . But I'm
> not sure if Mathieu is ok with it or not? If it seems useful to him, then I
> can go for it.

The ETB drivers could use a "module_amba_driver()"...

>
>>
>> Paul Bolle
>
>
>
> --
> Vaishali

Re: [PATCH v2] coresight: replicator: Use module_platform_driver

[Vaishali Thakkar]

On Fri, Jul 10, 2015 at 8:00 PM, Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
> On 10 July 2015 at 05:47, Vaishali Thakkar <vthakkar1994@gmail.com> wrote:
>> On Fri, Jul 10, 2015 at 2:30 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
>>> On vr, 2015-07-10 at 08:53 +0530, Vaishali Thakkar wrote:
>>>> I thought about this solution before sending this patch. But I was not
>>>> sure about it. Thanks for the explanation. I will send v3 with this
>>>> change.
>>>>
>>>> Can I add Suggested By: Paul Bolle <pebolle@tiscali.nl>
>>>
>>> That should be "Suggested-by:". The net effect would be that, if my
>>> suggestion turns out to be unwise, fan mail will also hit my INBOX,
>>> right? Anyhow, fine with me.
>>
>> Ok. Thanks.
>>
>>> By the way, there's more module specific stuff in
>>> drivers/hwtracing/coresight/. And there's no tristate symbol to be found
>>> in its Kconfig file. So I'd guess there are a few other cleanups
>>> possible too, if someone cared enough to have a closer look at that.
>>>
>>
>> Yes. It seems that introducing something like builtin_amba_driver()
>> can be useful for files which are using module_amba_driver now . But I'm
>> not sure if Mathieu is ok with it or not? If it seems useful to him, then I
>> can go for it.
>
> The ETB drivers could use a "module_amba_driver()"...

Why? Is there any specific reason behind this?
How about other drivers?? Will it be beneficial to introduce
builtin_amba_driver() for the others?

Thank You.

>>
>>>
>>> Paul Bolle
>>
>>
>>
>> --
>> Vaishali



-- 
Vaishali

Re: [PATCH v2] coresight: replicator: Use module_platform_driver

[Mathieu Poirier]

On 10 July 2015 at 09:51, Vaishali Thakkar <vthakkar1994@gmail.com> wrote:
> On Fri, Jul 10, 2015 at 8:00 PM, Mathieu Poirier
> <mathieu.poirier@linaro.org> wrote:
>> On 10 July 2015 at 05:47, Vaishali Thakkar <vthakkar1994@gmail.com> wrote:
>>> On Fri, Jul 10, 2015 at 2:30 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
>>>> On vr, 2015-07-10 at 08:53 +0530, Vaishali Thakkar wrote:
>>>>> I thought about this solution before sending this patch. But I was not
>>>>> sure about it. Thanks for the explanation. I will send v3 with this
>>>>> change.
>>>>>
>>>>> Can I add Suggested By: Paul Bolle <pebolle@tiscali.nl>
>>>>
>>>> That should be "Suggested-by:". The net effect would be that, if my
>>>> suggestion turns out to be unwise, fan mail will also hit my INBOX,
>>>> right? Anyhow, fine with me.
>>>
>>> Ok. Thanks.
>>>
>>>> By the way, there's more module specific stuff in
>>>> drivers/hwtracing/coresight/. And there's no tristate symbol to be found
>>>> in its Kconfig file. So I'd guess there are a few other cleanups
>>>> possible too, if someone cared enough to have a closer look at that.
>>>>
>>>
>>> Yes. It seems that introducing something like builtin_amba_driver()
>>> can be useful for files which are using module_amba_driver now . But I'm
>>> not sure if Mathieu is ok with it or not? If it seems useful to him, then I
>>> can go for it.
>>
>> The ETB drivers could use a "module_amba_driver()"...
>
> Why? Is there any specific reason behind this?
> How about other drivers?? Will it be beneficial to introduce
> builtin_amba_driver() for the others?

All the other drivers (aside from the replicator) have been moved to
"module_amba_driver()" to avoid boilerplate code.  The only one that
was forgotten is the ETB.  A fix for ETM3x is already part of the 4.2
cycle.

As for builtin_amba_driver(), that will be up to Russell to decide.
Other than not calling the second half of the module_driver() macro, I
don't see what else it could do.

>
> Thank You.
>
>>>
>>>>
>>>> Paul Bolle
>>>
>>>
>>>
>>> --
>>> Vaishali
>
>
>
> --
> Vaishali

Re: [PATCH v2] coresight: replicator: Use module_platform_driver

[Vaishali Thakkar]

On Mon, Jul 13, 2015 at 9:01 PM, Mathieu Poirier
<mathieu.poirier@linaro.org> wrote:
>
> On 10 July 2015 at 09:51, Vaishali Thakkar <vthakkar1994@gmail.com> wrote:
> > On Fri, Jul 10, 2015 at 8:00 PM, Mathieu Poirier
> > <mathieu.poirier@linaro.org> wrote:
> >> On 10 July 2015 at 05:47, Vaishali Thakkar <vthakkar1994@gmail.com> wrote:
> >>> On Fri, Jul 10, 2015 at 2:30 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
> >>>> On vr, 2015-07-10 at 08:53 +0530, Vaishali Thakkar wrote:
> >>>>> I thought about this solution before sending this patch. But I was not
> >>>>> sure about it. Thanks for the explanation. I will send v3 with this
> >>>>> change.
> >>>>>
> >>>>> Can I add Suggested By: Paul Bolle <pebolle@tiscali.nl>
> >>>>
> >>>> That should be "Suggested-by:". The net effect would be that, if my
> >>>> suggestion turns out to be unwise, fan mail will also hit my INBOX,
> >>>> right? Anyhow, fine with me.
> >>>
> >>> Ok. Thanks.
> >>>
> >>>> By the way, there's more module specific stuff in
> >>>> drivers/hwtracing/coresight/. And there's no tristate symbol to be found
> >>>> in its Kconfig file. So I'd guess there are a few other cleanups
> >>>> possible too, if someone cared enough to have a closer look at that.
> >>>>
> >>>
> >>> Yes. It seems that introducing something like builtin_amba_driver()
> >>> can be useful for files which are using module_amba_driver now . But I'm
> >>> not sure if Mathieu is ok with it or not? If it seems useful to him, then I
> >>> can go for it.
> >>
> >> The ETB drivers could use a "module_amba_driver()"...
> >
> > Why? Is there any specific reason behind this?
> > How about other drivers?? Will it be beneficial to introduce
> > builtin_amba_driver() for the others?
>
> All the other drivers (aside from the replicator) have been moved to
> "module_amba_driver()" to avoid boilerplate code.  The only one that
> was forgotten is the ETB.  A fix for ETM3x is already part of the 4.2
> cycle.
>

I see. Ok.

>
> As for builtin_amba_driver(), that will be up to Russell to decide.
> Other than not calling the second half of the module_driver() macro, I
> don't see what else it could do.

I think there was a good conversation between Paul Gortmaker and other
developers when he first introduced the idea of adding such macro
(builtin_platform_driver). His commit explains that idea in a good way too.
But yes I believe that it is a matter of taste. And at the end of the day it is
upon maintainers whether such change is good for their drivers or not.
Anyways I will be happy to work on this thing if Russell or you decides
to go for builtin_amba_driver.

Thank You.

> >
> > Thank You.
> >
> >>>
> >>>>
> >>>> Paul Bolle
> >>>
> >>>
> >>>
> >>> --
> >>> Vaishali
> >
> >
> >
> > --
> > Vaishali




-- 
Vaishali

Re: [PATCH v2] coresight: replicator: Use module_platform_driver

[Mathieu Poirier]

On 14 July 2015 at 05:49, Vaishali Thakkar <vthakkar1994@gmail.com> wrote:
> On Mon, Jul 13, 2015 at 9:01 PM, Mathieu Poirier
> <mathieu.poirier@linaro.org> wrote:
>>
>> On 10 July 2015 at 09:51, Vaishali Thakkar <vthakkar1994@gmail.com> wrote:
>> > On Fri, Jul 10, 2015 at 8:00 PM, Mathieu Poirier
>> > <mathieu.poirier@linaro.org> wrote:
>> >> On 10 July 2015 at 05:47, Vaishali Thakkar <vthakkar1994@gmail.com> wrote:
>> >>> On Fri, Jul 10, 2015 at 2:30 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
>> >>>> On vr, 2015-07-10 at 08:53 +0530, Vaishali Thakkar wrote:
>> >>>>> I thought about this solution before sending this patch. But I was not
>> >>>>> sure about it. Thanks for the explanation. I will send v3 with this
>> >>>>> change.
>> >>>>>
>> >>>>> Can I add Suggested By: Paul Bolle <pebolle@tiscali.nl>
>> >>>>
>> >>>> That should be "Suggested-by:". The net effect would be that, if my
>> >>>> suggestion turns out to be unwise, fan mail will also hit my INBOX,
>> >>>> right? Anyhow, fine with me.
>> >>>
>> >>> Ok. Thanks.
>> >>>
>> >>>> By the way, there's more module specific stuff in
>> >>>> drivers/hwtracing/coresight/. And there's no tristate symbol to be found
>> >>>> in its Kconfig file. So I'd guess there are a few other cleanups
>> >>>> possible too, if someone cared enough to have a closer look at that.
>> >>>>
>> >>>
>> >>> Yes. It seems that introducing something like builtin_amba_driver()
>> >>> can be useful for files which are using module_amba_driver now . But I'm
>> >>> not sure if Mathieu is ok with it or not? If it seems useful to him, then I
>> >>> can go for it.
>> >>
>> >> The ETB drivers could use a "module_amba_driver()"...
>> >
>> > Why? Is there any specific reason behind this?
>> > How about other drivers?? Will it be beneficial to introduce
>> > builtin_amba_driver() for the others?
>>
>> All the other drivers (aside from the replicator) have been moved to
>> "module_amba_driver()" to avoid boilerplate code.  The only one that
>> was forgotten is the ETB.  A fix for ETM3x is already part of the 4.2
>> cycle.
>>
>
> I see. Ok.
>
>>
>> As for builtin_amba_driver(), that will be up to Russell to decide.
>> Other than not calling the second half of the module_driver() macro, I
>> don't see what else it could do.
>
> I think there was a good conversation between Paul Gortmaker and other
> developers when he first introduced the idea of adding such macro
> (builtin_platform_driver). His commit explains that idea in a good way too.
> But yes I believe that it is a matter of taste. And at the end of the day it is
> upon maintainers whether such change is good for their drivers or not.
> Anyways I will be happy to work on this thing if Russell or you decides
> to go for builtin_amba_driver.

I personally think there is better (and more interesting) work to do
in the kernel for someone who wants to learn.  The staging directory
is full of drivers begging to be upstreamed.  Most of them even have a
tally of things to do before they can be taken out of staging.  On top
of things their original author will likely welcome help to work on
them.