2014-01-30 10:49:40

by Mohit KUMAR DCG

[permalink] [raw]
Subject: [PATCH V3 4/8] phy: Initialize phy core with subsys_initcall

From: Pratyush Anand <[email protected]>

PCIe RC drivers are initialized with subsys_initcall. Few PCIe drivers
like SPEAr13xx needs phy drivers to be initialized.

Therefore initialize phy core driver with subsys_initcall to avoid
calling of phy_get before phy_class is created.

Signed-off-by: Pratyush Anand <[email protected]>
Cc: Mohit Kumar <[email protected]>
Cc: Kishon Vijay Abraham I <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/phy/phy-core.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 03cf8fb..fa73101 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -685,7 +685,7 @@ static int __init phy_core_init(void)

return 0;
}
-module_init(phy_core_init);
+subsys_initcall(phy_core_init);

static void __exit phy_core_exit(void)
{
--
1.7.0.1


2014-01-30 11:43:53

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH V3 4/8] phy: Initialize phy core with subsys_initcall

Hi,

On Thursday 30 January 2014 04:18 PM, Mohit Kumar wrote:
> From: Pratyush Anand <[email protected]>
>
> PCIe RC drivers are initialized with subsys_initcall. Few PCIe drivers
> like SPEAr13xx needs phy drivers to be initialized.

Instead change PCIe RC drivers to module init. Phy drivers should be loaded
very early otherwise. (Hint: drivers/Makefile).

Thanks
Kishon
>
> Therefore initialize phy core driver with subsys_initcall to avoid
> calling of phy_get before phy_class is created.
>
> Signed-off-by: Pratyush Anand <[email protected]>
> Cc: Mohit Kumar <[email protected]>
> Cc: Kishon Vijay Abraham I <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/phy/phy-core.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 03cf8fb..fa73101 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -685,7 +685,7 @@ static int __init phy_core_init(void)
>
> return 0;
> }
> -module_init(phy_core_init);
> +subsys_initcall(phy_core_init);
>
> static void __exit phy_core_exit(void)
> {
>

2014-01-30 11:52:56

by Pratyush Anand

[permalink] [raw]
Subject: Re: [PATCH V3 4/8] phy: Initialize phy core with subsys_initcall

On Thu, Jan 30, 2014 at 07:43:37PM +0800, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Thursday 30 January 2014 04:18 PM, Mohit Kumar wrote:
> > From: Pratyush Anand <[email protected]>
> >
> > PCIe RC drivers are initialized with subsys_initcall. Few PCIe drivers
> > like SPEAr13xx needs phy drivers to be initialized.
>
> Instead change PCIe RC drivers to module init. Phy drivers should be loaded
> very early otherwise. (Hint: drivers/Makefile).

I think PCIe RC driver can not be made module init. Bjorn can comment
better.

All PCIe card drivers are initialized with module init. RC driver must
have been initialized before any card driver initialization.
Currently, card drivers does not have deferred probe concept, so I am
not sure if keeping RC driver as module init will work always.

By the way, is there any side effect of loading phy driver very early?

Regards
Pratyush
>
> Thanks
> Kishon
> >
> > Therefore initialize phy core driver with subsys_initcall to avoid
> > calling of phy_get before phy_class is created.
> >
> > Signed-off-by: Pratyush Anand <[email protected]>
> > Cc: Mohit Kumar <[email protected]>
> > Cc: Kishon Vijay Abraham I <[email protected]>
> > Cc: Arnd Bergmann <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > ---
> > drivers/phy/phy-core.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> > index 03cf8fb..fa73101 100644
> > --- a/drivers/phy/phy-core.c
> > +++ b/drivers/phy/phy-core.c
> > @@ -685,7 +685,7 @@ static int __init phy_core_init(void)
> >
> > return 0;
> > }
> > -module_init(phy_core_init);
> > +subsys_initcall(phy_core_init);
> >
> > static void __exit phy_core_exit(void)
> > {
> >
>

2014-01-30 12:10:46

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH V3 4/8] phy: Initialize phy core with subsys_initcall

Hi,

On Thursday 30 January 2014 05:22 PM, Pratyush Anand wrote:
> On Thu, Jan 30, 2014 at 07:43:37PM +0800, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Thursday 30 January 2014 04:18 PM, Mohit Kumar wrote:
>>> From: Pratyush Anand <[email protected]>
>>>
>>> PCIe RC drivers are initialized with subsys_initcall. Few PCIe drivers
>>> like SPEAr13xx needs phy drivers to be initialized.
>>
>> Instead change PCIe RC drivers to module init. Phy drivers should be loaded
>> very early otherwise. (Hint: drivers/Makefile).
>
> I think PCIe RC driver can not be made module init. Bjorn can comment
> better.

Why not? I have used it for DRA7xx without any issues (I'll send that one
upstream once the PIPE3 phy part gets clear).
>
> All PCIe card drivers are initialized with module init. RC driver must
> have been initialized before any card driver initialization.
> Currently, card drivers does not have deferred probe concept, so I am
> not sure if keeping RC driver as module init will work always.

the card drivers will anyway be probed only after RC driver comes up no?
>
> By the way, is there any side effect of loading phy driver very early?

I assume you meant 'is there any side effect of using subsys_initcall?', since
phy driver is loaded early anyway.
The answer is no just that module_init is common one and more people prefer to
use module_init. (btw initial versions of phy-core had susbsys_initcall before
it got changed to use module_init)

Thanks
Kishon

2014-01-30 12:21:06

by Pratyush Anand

[permalink] [raw]
Subject: Re: [PATCH V3 4/8] phy: Initialize phy core with subsys_initcall

On Thu, Jan 30, 2014 at 07:52:12PM +0800, Pratyush ANAND wrote:
> On Thu, Jan 30, 2014 at 07:43:37PM +0800, Kishon Vijay Abraham I wrote:
> > Hi,
> >
> > On Thursday 30 January 2014 04:18 PM, Mohit Kumar wrote:
> > > From: Pratyush Anand <[email protected]>
> > >
> > > PCIe RC drivers are initialized with subsys_initcall. Few PCIe drivers
> > > like SPEAr13xx needs phy drivers to be initialized.
> >
> > Instead change PCIe RC drivers to module init. Phy drivers should be loaded
> > very early otherwise. (Hint: drivers/Makefile).

>From hint, you mean that if makefile has pci entry (and hence RC
driver entry) before card drivers entry, then it insures that rc
driver's probe is called before card driver's probe?
I think, yes.
And if yes, then what you say is acceptable :)

Regards
Pratyush
>
> I think PCIe RC driver can not be made module init. Bjorn can comment
> better.
>
> All PCIe card drivers are initialized with module init. RC driver must
> have been initialized before any card driver initialization.
> Currently, card drivers does not have deferred probe concept, so I am
> not sure if keeping RC driver as module init will work always.
>
> By the way, is there any side effect of loading phy driver very early?
>
> Regards
> Pratyush
> >
> > Thanks
> > Kishon
> > >
> > > Therefore initialize phy core driver with subsys_initcall to avoid
> > > calling of phy_get before phy_class is created.
> > >
> > > Signed-off-by: Pratyush Anand <[email protected]>
> > > Cc: Mohit Kumar <[email protected]>
> > > Cc: Kishon Vijay Abraham I <[email protected]>
> > > Cc: Arnd Bergmann <[email protected]>
> > > Cc: [email protected]
> > > Cc: [email protected]
> > > ---
> > > drivers/phy/phy-core.c | 2 +-
> > > 1 files changed, 1 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> > > index 03cf8fb..fa73101 100644
> > > --- a/drivers/phy/phy-core.c
> > > +++ b/drivers/phy/phy-core.c
> > > @@ -685,7 +685,7 @@ static int __init phy_core_init(void)
> > >
> > > return 0;
> > > }
> > > -module_init(phy_core_init);
> > > +subsys_initcall(phy_core_init);
> > >
> > > static void __exit phy_core_exit(void)
> > > {
> > >
> >

2014-01-30 12:25:21

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH V3 4/8] phy: Initialize phy core with subsys_initcall

On Thursday 30 January 2014 05:45 PM, Pratyush Anand wrote:
> On Thu, Jan 30, 2014 at 07:52:12PM +0800, Pratyush ANAND wrote:
>> On Thu, Jan 30, 2014 at 07:43:37PM +0800, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Thursday 30 January 2014 04:18 PM, Mohit Kumar wrote:
>>>> From: Pratyush Anand <[email protected]>
>>>>
>>>> PCIe RC drivers are initialized with subsys_initcall. Few PCIe drivers
>>>> like SPEAr13xx needs phy drivers to be initialized.
>>>
>>> Instead change PCIe RC drivers to module init. Phy drivers should be loaded
>>> very early otherwise. (Hint: drivers/Makefile).
>
> From hint, you mean that if makefile has pci entry (and hence RC
> driver entry) before card drivers entry, then it insures that rc
> driver's probe is called before card driver's probe?

That's right but here I was referring to PHY and PCI.

Thanks
Kishon

> I think, yes.
> And if yes, then what you say is acceptable :)
>
> Regards
> Pratyush
>>
>> I think PCIe RC driver can not be made module init. Bjorn can comment
>> better.
>>
>> All PCIe card drivers are initialized with module init. RC driver must
>> have been initialized before any card driver initialization.
>> Currently, card drivers does not have deferred probe concept, so I am
>> not sure if keeping RC driver as module init will work always.
>>
>> By the way, is there any side effect of loading phy driver very early?
>>
>> Regards
>> Pratyush
>>>
>>> Thanks
>>> Kishon
>>>>
>>>> Therefore initialize phy core driver with subsys_initcall to avoid
>>>> calling of phy_get before phy_class is created.
>>>>
>>>> Signed-off-by: Pratyush Anand <[email protected]>
>>>> Cc: Mohit Kumar <[email protected]>
>>>> Cc: Kishon Vijay Abraham I <[email protected]>
>>>> Cc: Arnd Bergmann <[email protected]>
>>>> Cc: [email protected]
>>>> Cc: [email protected]
>>>> ---
>>>> drivers/phy/phy-core.c | 2 +-
>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>>>> index 03cf8fb..fa73101 100644
>>>> --- a/drivers/phy/phy-core.c
>>>> +++ b/drivers/phy/phy-core.c
>>>> @@ -685,7 +685,7 @@ static int __init phy_core_init(void)
>>>>
>>>> return 0;
>>>> }
>>>> -module_init(phy_core_init);
>>>> +subsys_initcall(phy_core_init);
>>>>
>>>> static void __exit phy_core_exit(void)
>>>> {
>>>>
>>>

2014-01-30 12:45:23

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH V3 4/8] phy: Initialize phy core with subsys_initcall

On Thursday 30 January 2014, Pratyush Anand wrote:
> On Thu, Jan 30, 2014 at 07:43:37PM +0800, Kishon Vijay Abraham I wrote:
> > Hi,
> >
> > On Thursday 30 January 2014 04:18 PM, Mohit Kumar wrote:
> > > From: Pratyush Anand <[email protected]>
> > >
> > > PCIe RC drivers are initialized with subsys_initcall. Few PCIe drivers
> > > like SPEAr13xx needs phy drivers to be initialized.
> >
> > Instead change PCIe RC drivers to module init. Phy drivers should be loaded
> > very early otherwise. (Hint: drivers/Makefile).
>
> I think PCIe RC driver can not be made module init. Bjorn can comment
> better.

I don't think there is any problem here: the PCI devices will only appear
after the PCIe root bus has been probed. All drivers using the regular
pci_driver framework should work fine even if they are loaded before the
device is found. There are a handful of drivers using 'pci_get_device'
rather than pci_register_driver, and those will break. As far as I can
tell, those drivers are all x86 specific, and you should not worry about
them.

Having the PHY driver get initialized after the PCI root driver should
also work, but it requires correct handling of -EPROBE_DEFER: if phy_get
returns this error, the PCI driver must silently return the same error
from its probe() function so it will get called again at a later time
(after some other devices have been probed successfully).

Arnd

2014-01-31 03:48:19

by Pratyush Anand

[permalink] [raw]
Subject: Re: [PATCH V3 4/8] phy: Initialize phy core with subsys_initcall

On Thu, Jan 30, 2014 at 08:44:58PM +0800, Arnd Bergmann wrote:
> On Thursday 30 January 2014, Pratyush Anand wrote:
> > On Thu, Jan 30, 2014 at 07:43:37PM +0800, Kishon Vijay Abraham I wrote:
> > > Hi,
> > >
> > > On Thursday 30 January 2014 04:18 PM, Mohit Kumar wrote:
> > > > From: Pratyush Anand <[email protected]>
> > > >
> > > > PCIe RC drivers are initialized with subsys_initcall. Few PCIe drivers
> > > > like SPEAr13xx needs phy drivers to be initialized.
> > >
> > > Instead change PCIe RC drivers to module init. Phy drivers should be loaded
> > > very early otherwise. (Hint: drivers/Makefile).
> >
> > I think PCIe RC driver can not be made module init. Bjorn can comment
> > better.
>
> I don't think there is any problem here: the PCI devices will only appear
> after the PCIe root bus has been probed. All drivers using the regular
> pci_driver framework should work fine even if they are loaded before the
> device is found. There are a handful of drivers using 'pci_get_device'
> rather than pci_register_driver, and those will break. As far as I can
> tell, those drivers are all x86 specific, and you should not worry about
> them.
>
> Having the PHY driver get initialized after the PCI root driver should
> also work, but it requires correct handling of -EPROBE_DEFER: if phy_get

I had issue with phy-core driver getting initialized after pcie rc
driver. I found a kernel crash, as devm_phy_get was called before
phy_class was created. I think this too need to be fixed, we should
not see a crash.

Anyway, I will keep spear phy and rc driver both with module_init and
-EPROBE_DEFER implementation.

Regards
Pratyush
> returns this error, the PCI driver must silently return the same error
> from its probe() function so it will get called again at a later time
> (after some other devices have been probed successfully).
>
> Arnd

2014-01-31 15:26:06

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH V3 4/8] phy: Initialize phy core with subsys_initcall

On Friday 31 January 2014, Pratyush Anand wrote:
> > Having the PHY driver get initialized after the PCI root driver should
> > also work, but it requires correct handling of -EPROBE_DEFER: if phy_get
>
> I had issue with phy-core driver getting initialized after pcie rc
> driver. I found a kernel crash, as devm_phy_get was called before
> phy_class was created. I think this too need to be fixed, we should
> not see a crash.

Yes, agreed. This should be trivial to do though.

> Anyway, I will keep spear phy and rc driver both with module_init and
> -EPROBE_DEFER implementation.

Ok.

Arnd