* [PATCH net-next] bcm87xx: Add MODULE_DEVICE_TABLE
@ 2013-09-01 21:33 Ben Hutchings
2013-09-03 17:32 ` David Daney
0 siblings, 1 reply; 6+ messages in thread
From: Ben Hutchings @ 2013-09-01 21:33 UTC (permalink / raw)
To: David Miller; +Cc: David Daney, netdev
[-- Attachment #1: Type: text/plain, Size: 705 bytes --]
bcm87xx currently isn't auto-loaded if built as a module.
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
Compile-tested only.
Ben.
drivers/net/phy/bcm87xx.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/phy/bcm87xx.c b/drivers/net/phy/bcm87xx.c
index 7997895..f5b44ff 100644
--- a/drivers/net/phy/bcm87xx.c
+++ b/drivers/net/phy/bcm87xx.c
@@ -230,4 +230,11 @@ static void __exit bcm87xx_exit(void)
}
module_exit(bcm87xx_exit);
+static struct mdio_device_id __maybe_unused bcm87xx_tbl[] = {
+ { PHY_ID_BCM8706, 0xffffffff },
+ { PHY_ID_BCM8727, 0xffffffff },
+ { }
+};
+
MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(mdio, bcm87xx_tbl);
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] bcm87xx: Add MODULE_DEVICE_TABLE
2013-09-01 21:33 [PATCH net-next] bcm87xx: Add MODULE_DEVICE_TABLE Ben Hutchings
@ 2013-09-03 17:32 ` David Daney
2013-09-03 18:53 ` Ben Hutchings
0 siblings, 1 reply; 6+ messages in thread
From: David Daney @ 2013-09-03 17:32 UTC (permalink / raw)
To: Ben Hutchings, David Miller; +Cc: David Daney, netdev
On 09/01/2013 02:33 PM, Ben Hutchings wrote:
> bcm87xx currently isn't auto-loaded if built as a module.
>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> Compile-tested only.
Then how do you know that it does anything sensible?
Other than that, it seems plausible.
David Daney
>
> Ben.
>
> drivers/net/phy/bcm87xx.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/phy/bcm87xx.c b/drivers/net/phy/bcm87xx.c
> index 7997895..f5b44ff 100644
> --- a/drivers/net/phy/bcm87xx.c
> +++ b/drivers/net/phy/bcm87xx.c
> @@ -230,4 +230,11 @@ static void __exit bcm87xx_exit(void)
> }
> module_exit(bcm87xx_exit);
>
> +static struct mdio_device_id __maybe_unused bcm87xx_tbl[] = {
> + { PHY_ID_BCM8706, 0xffffffff },
> + { PHY_ID_BCM8727, 0xffffffff },
> + { }
> +};
> +
> MODULE_LICENSE("GPL");
> +MODULE_DEVICE_TABLE(mdio, bcm87xx_tbl);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] bcm87xx: Add MODULE_DEVICE_TABLE
2013-09-03 17:32 ` David Daney
@ 2013-09-03 18:53 ` Ben Hutchings
2013-09-03 19:13 ` David Daney
0 siblings, 1 reply; 6+ messages in thread
From: Ben Hutchings @ 2013-09-03 18:53 UTC (permalink / raw)
To: David Daney; +Cc: David Miller, David Daney, netdev
On Tue, Sep 03, 2013 at 10:32:02AM -0700, David Daney wrote:
> On 09/01/2013 02:33 PM, Ben Hutchings wrote:
> >bcm87xx currently isn't auto-loaded if built as a module.
> >
> >Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> >---
> >Compile-tested only.
>
> Then how do you know that it does anything sensible?
This is generally required in modular PHY drivers. I was hoping you'd
be able to say whether that it's useful or necessary for this one.
> Other than that, it seems plausible.
>
> David Daney
Thanks.
Ben.
> >
> >Ben.
> >
> > drivers/net/phy/bcm87xx.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> >diff --git a/drivers/net/phy/bcm87xx.c b/drivers/net/phy/bcm87xx.c
> >index 7997895..f5b44ff 100644
> >--- a/drivers/net/phy/bcm87xx.c
> >+++ b/drivers/net/phy/bcm87xx.c
> >@@ -230,4 +230,11 @@ static void __exit bcm87xx_exit(void)
> > }
> > module_exit(bcm87xx_exit);
> >
> >+static struct mdio_device_id __maybe_unused bcm87xx_tbl[] = {
> >+ { PHY_ID_BCM8706, 0xffffffff },
> >+ { PHY_ID_BCM8727, 0xffffffff },
> >+ { }
> >+};
> >+
> > MODULE_LICENSE("GPL");
> >+MODULE_DEVICE_TABLE(mdio, bcm87xx_tbl);
> >
>
--
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
- Albert Camus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] bcm87xx: Add MODULE_DEVICE_TABLE
2013-09-03 18:53 ` Ben Hutchings
@ 2013-09-03 19:13 ` David Daney
2013-09-03 19:28 ` David Daney
0 siblings, 1 reply; 6+ messages in thread
From: David Daney @ 2013-09-03 19:13 UTC (permalink / raw)
To: Ben Hutchings, David Miller; +Cc: David Daney, David Daney, netdev
On 09/03/2013 11:53 AM, Ben Hutchings wrote:
> On Tue, Sep 03, 2013 at 10:32:02AM -0700, David Daney wrote:
>> On 09/01/2013 02:33 PM, Ben Hutchings wrote:
>>> bcm87xx currently isn't auto-loaded if built as a module.
>>>
>>> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
>>> ---
>>> Compile-tested only.
>>
>> Then how do you know that it does anything sensible?
>
> This is generally required in modular PHY drivers. I was hoping you'd
> be able to say whether that it's useful or necessary for this one.
OK. I just tested the patch, and it is not sufficient to get the module
loaded automatically.
My configuration also successfully uses drivers/net/phy/marvell.ko, so I
think the modules.alias file is properly generated, and the auto-loading
is working for some PHY drivers, but bcm87xx isn't detected.
I may take a look later this week, but as the patch stands, I don't
think it should be merged.
David Daney
>
>> Other than that, it seems plausible.
>>
>> David Daney
>
> Thanks.
>
> Ben.
>
>>>
>>> Ben.
>>>
>>> drivers/net/phy/bcm87xx.c | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/net/phy/bcm87xx.c b/drivers/net/phy/bcm87xx.c
>>> index 7997895..f5b44ff 100644
>>> --- a/drivers/net/phy/bcm87xx.c
>>> +++ b/drivers/net/phy/bcm87xx.c
>>> @@ -230,4 +230,11 @@ static void __exit bcm87xx_exit(void)
>>> }
>>> module_exit(bcm87xx_exit);
>>>
>>> +static struct mdio_device_id __maybe_unused bcm87xx_tbl[] = {
>>> + { PHY_ID_BCM8706, 0xffffffff },
>>> + { PHY_ID_BCM8727, 0xffffffff },
>>> + { }
>>> +};
>>> +
>>> MODULE_LICENSE("GPL");
>>> +MODULE_DEVICE_TABLE(mdio, bcm87xx_tbl);
>>>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] bcm87xx: Add MODULE_DEVICE_TABLE
2013-09-03 19:13 ` David Daney
@ 2013-09-03 19:28 ` David Daney
2013-09-03 20:53 ` Ben Hutchings
0 siblings, 1 reply; 6+ messages in thread
From: David Daney @ 2013-09-03 19:28 UTC (permalink / raw)
To: Ben Hutchings, David Miller; +Cc: David Daney, David Daney, netdev
On 09/03/2013 12:13 PM, David Daney wrote:
> On 09/03/2013 11:53 AM, Ben Hutchings wrote:
>> On Tue, Sep 03, 2013 at 10:32:02AM -0700, David Daney wrote:
>>> On 09/01/2013 02:33 PM, Ben Hutchings wrote:
>>>> bcm87xx currently isn't auto-loaded if built as a module.
>>>>
>>>> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
>>>> ---
>>>> Compile-tested only.
>>>
>>> Then how do you know that it does anything sensible?
>>
>> This is generally required in modular PHY drivers. I was hoping you'd
>> be able to say whether that it's useful or necessary for this one.
>
>
> OK. I just tested the patch, and it is not sufficient to get the module
> loaded automatically.
>
The problem is that get_phy_c45_ids() sets the phy_id that is passed to
request_module() to zero, so it will never match anything.
We need to think about how this should work for 802.3-c45 PHYs.
Most c54 PHYs are conceptually composed of several pieces each with its
own set of identifiers. Which of these should be used? Will something
break if we start supplying a C22 phy_id for these devices?
David Daney
> My configuration also successfully uses drivers/net/phy/marvell.ko, so I
> think the modules.alias file is properly generated, and the auto-loading
> is working for some PHY drivers, but bcm87xx isn't detected.
>
> I may take a look later this week, but as the patch stands, I don't
> think it should be merged.
>
> David Daney
>
>
>>
>>> Other than that, it seems plausible.
>>>
>>> David Daney
>>
>> Thanks.
>>
>> Ben.
>>
>>>>
>>>> Ben.
>>>>
>>>> drivers/net/phy/bcm87xx.c | 7 +++++++
>>>> 1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/net/phy/bcm87xx.c b/drivers/net/phy/bcm87xx.c
>>>> index 7997895..f5b44ff 100644
>>>> --- a/drivers/net/phy/bcm87xx.c
>>>> +++ b/drivers/net/phy/bcm87xx.c
>>>> @@ -230,4 +230,11 @@ static void __exit bcm87xx_exit(void)
>>>> }
>>>> module_exit(bcm87xx_exit);
>>>>
>>>> +static struct mdio_device_id __maybe_unused bcm87xx_tbl[] = {
>>>> + { PHY_ID_BCM8706, 0xffffffff },
>>>> + { PHY_ID_BCM8727, 0xffffffff },
>>>> + { }
>>>> +};
>>>> +
>>>> MODULE_LICENSE("GPL");
>>>> +MODULE_DEVICE_TABLE(mdio, bcm87xx_tbl);
>>>>
>>>
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] bcm87xx: Add MODULE_DEVICE_TABLE
2013-09-03 19:28 ` David Daney
@ 2013-09-03 20:53 ` Ben Hutchings
0 siblings, 0 replies; 6+ messages in thread
From: Ben Hutchings @ 2013-09-03 20:53 UTC (permalink / raw)
To: David Daney; +Cc: David Miller, David Daney, David Daney, netdev
On Tue, Sep 03, 2013 at 12:28:50PM -0700, David Daney wrote:
> On 09/03/2013 12:13 PM, David Daney wrote:
> >On 09/03/2013 11:53 AM, Ben Hutchings wrote:
> >>On Tue, Sep 03, 2013 at 10:32:02AM -0700, David Daney wrote:
> >>>On 09/01/2013 02:33 PM, Ben Hutchings wrote:
> >>>>bcm87xx currently isn't auto-loaded if built as a module.
> >>>>
> >>>>Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> >>>>---
> >>>>Compile-tested only.
> >>>
> >>>Then how do you know that it does anything sensible?
> >>
> >>This is generally required in modular PHY drivers. I was hoping you'd
> >>be able to say whether that it's useful or necessary for this one.
> >
> >
> >OK. I just tested the patch, and it is not sufficient to get the module
> >loaded automatically.
> >
>
> The problem is that get_phy_c45_ids() sets the phy_id that is passed
> to request_module() to zero, so it will never match anything.
Right. Now I see that phy_driver::phy_id{,_mask} are actually not
used for this driver because phy_driver::match_phy_device overrides
those.
> We need to think about how this should work for 802.3-c45 PHYs.
>
> Most c54 PHYs are conceptually composed of several pieces each with
> its own set of identifiers.
Yes, I know that but I wasn't sure how libphy deals with them.
> Which of these should be used? Will
> something break if we start supplying a C22 phy_id for these
> devices?
But which one? Maybe it would make sense to generate a modalias for
the device with all MMDs listed:
mdio:<mmd><id>...
where <mmd> is the MMD number converted to a letter (a-z, A-F) and
<id> is the device ID as a bitstring, and they're repeated for all
present MMDs. A module would then have modaliases that match on
any particular MMD's ID, e.g. this module would have the aliases:
mdio:*e00000001010000111011110111000001*
mdio:*e00000001010000111011111111110000*
(I'm assuming that a driver wouldn't need to match on multiple
MMD IDs, which would make the device table structure and
modpost logic more complicated.)
Maybe there's a simpler way to do it, but I don't know what.
Ben.
--
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
- Albert Camus
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-09-03 20:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-01 21:33 [PATCH net-next] bcm87xx: Add MODULE_DEVICE_TABLE Ben Hutchings
2013-09-03 17:32 ` David Daney
2013-09-03 18:53 ` Ben Hutchings
2013-09-03 19:13 ` David Daney
2013-09-03 19:28 ` David Daney
2013-09-03 20:53 ` Ben Hutchings
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).