Commit 7c32f470f4f6a0fdc6944cefcd22f288e59a0ae2

Authored by Vitaly Bordug
Committed by David S. Miller
1 parent cdcc520d7b

PHY fixed driver: rework release path and update phy_id notation

device_bind_driver() error code returning has been fixed.  release()
function has been written, so that to free resources in correct way; the
release path is now clean.

Before the rework, it used to cause
 Device 'fixed@100:1' does not have a release() function, it is broken
 and must be fixed.
 BUG: at drivers/base/core.c:104 device_release()

 Call Trace:
  [<ffffffff802ec380>] kobject_cleanup+0x53/0x7e
  [<ffffffff802ec3ab>] kobject_release+0x0/0x9
  [<ffffffff802ecf3f>] kref_put+0x74/0x81
  [<ffffffff8035493b>] fixed_mdio_register_device+0x230/0x265
  [<ffffffff80564d31>] fixed_init+0x1f/0x35
  [<ffffffff802071a4>] init+0x147/0x2fb
  [<ffffffff80223b6e>] schedule_tail+0x36/0x92
  [<ffffffff8020a678>] child_rip+0xa/0x12
  [<ffffffff80311714>] acpi_ds_init_one_object+0x0/0x83
  [<ffffffff8020705d>] init+0x0/0x2fb
  [<ffffffff8020a66e>] child_rip+0x0/0x12

Also changed the notation of the fixed phy definition on
mdio bus to the form of <speed>+<duplex> to make it able to be used by
gianfar and ucc_geth that define phy_id strictly as "%d:%d" and cleaned up
the whitespace issues.

Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Jeff Garzik <jeff@garzik.org>

Showing 3 changed files with 207 additions and 155 deletions Side-by-side Diff

drivers/net/phy/Kconfig
... ... @@ -76,5 +76,19 @@
76 76 bool "Emulation for 100M Fdx fixed PHY behavior"
77 77 depends on FIXED_PHY
78 78  
  79 +config FIXED_MII_1000_FDX
  80 + bool "Emulation for 1000M Fdx fixed PHY behavior"
  81 + depends on FIXED_PHY
  82 +
  83 +config FIXED_MII_AMNT
  84 + int "Number of emulated PHYs to allocate "
  85 + depends on FIXED_PHY
  86 + default "1"
  87 + ---help---
  88 + Sometimes it is required to have several independent emulated
  89 + PHYs on the bus (in case of multi-eth but phy-less HW for instance).
  90 + This control will have specified number allocated for each fixed
  91 + PHY type enabled.
  92 +
79 93 endif # PHYLIB
drivers/net/phy/fixed.c
... ... @@ -30,53 +30,31 @@
30 30 #include <linux/mii.h>
31 31 #include <linux/ethtool.h>
32 32 #include <linux/phy.h>
  33 +#include <linux/phy_fixed.h>
33 34  
34 35 #include <asm/io.h>
35 36 #include <asm/irq.h>
36 37 #include <asm/uaccess.h>
37 38  
38   -#define MII_REGS_NUM 7
  39 +/* we need to track the allocated pointers in order to free them on exit */
  40 +static struct fixed_info *fixed_phy_ptrs[CONFIG_FIXED_MII_AMNT*MAX_PHY_AMNT];
39 41  
40   -/*
41   - The idea is to emulate normal phy behavior by responding with
42   - pre-defined values to mii BMCR read, so that read_status hook could
43   - take all the needed info.
44   -*/
45   -
46   -struct fixed_phy_status {
47   - u8 link;
48   - u16 speed;
49   - u8 duplex;
50   -};
51   -
52 42 /*-----------------------------------------------------------------------------
53   - * Private information hoder for mii_bus
54   - *-----------------------------------------------------------------------------*/
55   -struct fixed_info {
56   - u16 *regs;
57   - u8 regs_num;
58   - struct fixed_phy_status phy_status;
59   - struct phy_device *phydev; /* pointer to the container */
60   - /* link & speed cb */
61   - int(*link_update)(struct net_device*, struct fixed_phy_status*);
62   -
63   -};
64   -
65   -/*-----------------------------------------------------------------------------
66 43 * If something weird is required to be done with link/speed,
67 44 * network driver is able to assign a function to implement this.
68 45 * May be useful for PHY's that need to be software-driven.
69 46 *-----------------------------------------------------------------------------*/
70   -int fixed_mdio_set_link_update(struct phy_device* phydev,
71   - int(*link_update)(struct net_device*, struct fixed_phy_status*))
  47 +int fixed_mdio_set_link_update(struct phy_device *phydev,
  48 + int (*link_update) (struct net_device *,
  49 + struct fixed_phy_status *))
72 50 {
73 51 struct fixed_info *fixed;
74 52  
75   - if(link_update == NULL)
  53 + if (link_update == NULL)
76 54 return -EINVAL;
77 55  
78   - if(phydev) {
79   - if(phydev->bus) {
  56 + if (phydev) {
  57 + if (phydev->bus) {
80 58 fixed = phydev->bus->priv;
81 59 fixed->link_update = link_update;
82 60 return 0;
83 61  
84 62  
85 63  
86 64  
87 65  
88 66  
89 67  
90 68  
91 69  
92 70  
93 71  
94 72  
... ... @@ -84,54 +62,64 @@
84 62 }
85 63 return -EINVAL;
86 64 }
  65 +
87 66 EXPORT_SYMBOL(fixed_mdio_set_link_update);
88 67  
  68 +struct fixed_info *fixed_mdio_get_phydev (int phydev_ind)
  69 +{
  70 + if (phydev_ind >= MAX_PHY_AMNT)
  71 + return NULL;
  72 + return fixed_phy_ptrs[phydev_ind];
  73 +}
  74 +
  75 +EXPORT_SYMBOL(fixed_mdio_get_phydev);
  76 +
89 77 /*-----------------------------------------------------------------------------
90 78 * This is used for updating internal mii regs from the status
91 79 *-----------------------------------------------------------------------------*/
92   -#if defined(CONFIG_FIXED_MII_100_FDX) || defined(CONFIG_FIXED_MII_10_FDX)
  80 +#if defined(CONFIG_FIXED_MII_100_FDX) || defined(CONFIG_FIXED_MII_10_FDX) || defined(CONFIG_FIXED_MII_1000_FDX)
93 81 static int fixed_mdio_update_regs(struct fixed_info *fixed)
94 82 {
95 83 u16 *regs = fixed->regs;
96 84 u16 bmsr = 0;
97 85 u16 bmcr = 0;
98 86  
99   - if(!regs) {
  87 + if (!regs) {
100 88 printk(KERN_ERR "%s: regs not set up", __FUNCTION__);
101 89 return -EINVAL;
102 90 }
103 91  
104   - if(fixed->phy_status.link)
  92 + if (fixed->phy_status.link)
105 93 bmsr |= BMSR_LSTATUS;
106 94  
107   - if(fixed->phy_status.duplex) {
  95 + if (fixed->phy_status.duplex) {
108 96 bmcr |= BMCR_FULLDPLX;
109 97  
110   - switch ( fixed->phy_status.speed ) {
  98 + switch (fixed->phy_status.speed) {
111 99 case 100:
112 100 bmsr |= BMSR_100FULL;
113 101 bmcr |= BMCR_SPEED100;
114   - break;
  102 + break;
115 103  
116 104 case 10:
117 105 bmsr |= BMSR_10FULL;
118   - break;
  106 + break;
119 107 }
120 108 } else {
121   - switch ( fixed->phy_status.speed ) {
  109 + switch (fixed->phy_status.speed) {
122 110 case 100:
123 111 bmsr |= BMSR_100HALF;
124 112 bmcr |= BMCR_SPEED100;
125   - break;
  113 + break;
126 114  
127 115 case 10:
128 116 bmsr |= BMSR_100HALF;
129   - break;
  117 + break;
130 118 }
131 119 }
132 120  
133   - regs[MII_BMCR] = bmcr;
134   - regs[MII_BMSR] = bmsr | 0x800; /*we are always capable of 10 hdx*/
  121 + regs[MII_BMCR] = bmcr;
  122 + regs[MII_BMSR] = bmsr | 0x800; /*we are always capable of 10 hdx */
135 123  
136 124 return 0;
137 125 }
138 126  
139 127  
140 128  
141 129  
142 130  
... ... @@ -141,29 +129,30 @@
141 129 struct fixed_info *fixed = bus->priv;
142 130  
143 131 /* if user has registered link update callback, use it */
144   - if(fixed->phydev)
145   - if(fixed->phydev->attached_dev) {
146   - if(fixed->link_update) {
  132 + if (fixed->phydev)
  133 + if (fixed->phydev->attached_dev) {
  134 + if (fixed->link_update) {
147 135 fixed->link_update(fixed->phydev->attached_dev,
148   - &fixed->phy_status);
  136 + &fixed->phy_status);
149 137 fixed_mdio_update_regs(fixed);
150 138 }
151   - }
  139 + }
152 140  
153 141 if ((unsigned int)location >= fixed->regs_num)
154 142 return -1;
155 143 return fixed->regs[location];
156 144 }
157 145  
158   -static int fixed_mii_write(struct mii_bus *bus, int phy_id, int location, u16 val)
  146 +static int fixed_mii_write(struct mii_bus *bus, int phy_id, int location,
  147 + u16 val)
159 148 {
160   - /* do nothing for now*/
  149 + /* do nothing for now */
161 150 return 0;
162 151 }
163 152  
164 153 static int fixed_mii_reset(struct mii_bus *bus)
165 154 {
166   - /*nothing here - no way/need to reset it*/
  155 + /*nothing here - no way/need to reset it */
167 156 return 0;
168 157 }
169 158 #endif
... ... @@ -171,8 +160,8 @@
171 160 static int fixed_config_aneg(struct phy_device *phydev)
172 161 {
173 162 /* :TODO:03/13/2006 09:45:37 PM::
174   - The full autoneg funcionality can be emulated,
175   - but no need to have anything here for now
  163 + The full autoneg funcionality can be emulated,
  164 + but no need to have anything here for now
176 165 */
177 166 return 0;
178 167 }
179 168  
180 169  
181 170  
182 171  
183 172  
184 173  
185 174  
186 175  
187 176  
188 177  
189 178  
... ... @@ -182,59 +171,79 @@
182 171 * match will never return true...
183 172 *-----------------------------------------------------------------------------*/
184 173 static struct phy_driver fixed_mdio_driver = {
185   - .name = "Fixed PHY",
186   - .features = PHY_BASIC_FEATURES,
187   - .config_aneg = fixed_config_aneg,
188   - .read_status = genphy_read_status,
189   - .driver = { .owner = THIS_MODULE,},
  174 + .name = "Fixed PHY",
  175 +#ifdef CONFIG_FIXED_MII_1000_FDX
  176 + .features = PHY_GBIT_FEATURES,
  177 +#else
  178 + .features = PHY_BASIC_FEATURES,
  179 +#endif
  180 + .config_aneg = fixed_config_aneg,
  181 + .read_status = genphy_read_status,
  182 + .driver = { .owner = THIS_MODULE, },
190 183 };
191 184  
  185 +static void fixed_mdio_release(struct device *dev)
  186 +{
  187 + struct phy_device *phydev = container_of(dev, struct phy_device, dev);
  188 + struct mii_bus *bus = phydev->bus;
  189 + struct fixed_info *fixed = bus->priv;
  190 +
  191 + kfree(phydev);
  192 + kfree(bus->dev);
  193 + kfree(bus);
  194 + kfree(fixed->regs);
  195 + kfree(fixed);
  196 +}
  197 +
192 198 /*-----------------------------------------------------------------------------
193 199 * This func is used to create all the necessary stuff, bind
194 200 * the fixed phy driver and register all it on the mdio_bus_type.
195   - * speed is either 10 or 100, duplex is boolean.
  201 + * speed is either 10 or 100 or 1000, duplex is boolean.
196 202 * number is used to create multiple fixed PHYs, so that several devices can
197 203 * utilize them simultaneously.
  204 + *
  205 + * The device on mdio bus will look like [bus_id]:[phy_id],
  206 + * bus_id = number
  207 + * phy_id = speed+duplex.
198 208 *-----------------------------------------------------------------------------*/
199   -#if defined(CONFIG_FIXED_MII_100_FDX) || defined(CONFIG_FIXED_MII_10_FDX)
200   -static int fixed_mdio_register_device(int number, int speed, int duplex)
  209 +#if defined(CONFIG_FIXED_MII_100_FDX) || defined(CONFIG_FIXED_MII_10_FDX) || defined(CONFIG_FIXED_MII_1000_FDX)
  210 +struct fixed_info *fixed_mdio_register_device(
  211 + int bus_id, int speed, int duplex, u8 phy_id)
201 212 {
202 213 struct mii_bus *new_bus;
203 214 struct fixed_info *fixed;
204 215 struct phy_device *phydev;
205   - int err = 0;
  216 + int err;
206 217  
207   - struct device* dev = kzalloc(sizeof(struct device), GFP_KERNEL);
  218 + struct device *dev = kzalloc(sizeof(struct device), GFP_KERNEL);
208 219  
209   - if (NULL == dev)
210   - return -ENOMEM;
  220 + if (dev == NULL)
  221 + goto err_dev_alloc;
211 222  
212 223 new_bus = kzalloc(sizeof(struct mii_bus), GFP_KERNEL);
213 224  
214   - if (NULL == new_bus) {
215   - kfree(dev);
216   - return -ENOMEM;
217   - }
  225 + if (new_bus == NULL)
  226 + goto err_bus_alloc;
  227 +
218 228 fixed = kzalloc(sizeof(struct fixed_info), GFP_KERNEL);
219 229  
220   - if (NULL == fixed) {
221   - kfree(dev);
222   - kfree(new_bus);
223   - return -ENOMEM;
224   - }
  230 + if (fixed == NULL)
  231 + goto err_fixed_alloc;
225 232  
226   - fixed->regs = kzalloc(MII_REGS_NUM*sizeof(int), GFP_KERNEL);
  233 + fixed->regs = kzalloc(MII_REGS_NUM * sizeof(int), GFP_KERNEL);
  234 + if (NULL == fixed->regs)
  235 + goto err_fixed_regs_alloc;
  236 +
227 237 fixed->regs_num = MII_REGS_NUM;
228 238 fixed->phy_status.speed = speed;
229 239 fixed->phy_status.duplex = duplex;
230 240 fixed->phy_status.link = 1;
231 241  
232   - new_bus->name = "Fixed MII Bus",
233   - new_bus->read = &fixed_mii_read,
234   - new_bus->write = &fixed_mii_write,
235   - new_bus->reset = &fixed_mii_reset,
236   -
237   - /*set up workspace*/
  242 + new_bus->name = "Fixed MII Bus";
  243 + new_bus->read = &fixed_mii_read;
  244 + new_bus->write = &fixed_mii_write;
  245 + new_bus->reset = &fixed_mii_reset;
  246 + /*set up workspace */
238 247 fixed_mdio_update_regs(fixed);
239 248 new_bus->priv = fixed;
240 249  
241 250  
242 251  
243 252  
244 253  
245 254  
246 255  
247 256  
248 257  
249 258  
250 259  
251 260  
252 261  
253 262  
254 263  
255 264  
256 265  
257 266  
258 267  
259 268  
260 269  
261 270  
262 271  
263 272  
... ... @@ -243,119 +252,110 @@
243 252  
244 253 /* create phy_device and register it on the mdio bus */
245 254 phydev = phy_device_create(new_bus, 0, 0);
  255 + if (phydev == NULL)
  256 + goto err_phy_dev_create;
246 257  
247 258 /*
248   - Put the phydev pointer into the fixed pack so that bus read/write code could
249   - be able to access for instance attached netdev. Well it doesn't have to do
250   - so, only in case of utilizing user-specified link-update...
  259 + * Put the phydev pointer into the fixed pack so that bus read/write
  260 + * code could be able to access for instance attached netdev. Well it
  261 + * doesn't have to do so, only in case of utilizing user-specified
  262 + * link-update...
251 263 */
  264 +
252 265 fixed->phydev = phydev;
  266 + phydev->speed = speed;
  267 + phydev->duplex = duplex;
253 268  
254   - if(NULL == phydev) {
255   - err = -ENOMEM;
256   - goto device_create_fail;
257   - }
258   -
259 269 phydev->irq = PHY_IGNORE_INTERRUPT;
260 270 phydev->dev.bus = &mdio_bus_type;
261 271  
262   - if(number)
263   - snprintf(phydev->dev.bus_id, BUS_ID_SIZE,
264   - "fixed_%d@%d:%d", number, speed, duplex);
265   - else
266   - snprintf(phydev->dev.bus_id, BUS_ID_SIZE,
267   - "fixed@%d:%d", speed, duplex);
  272 + snprintf(phydev->dev.bus_id, BUS_ID_SIZE,
  273 + PHY_ID_FMT, bus_id, phy_id);
  274 +
268 275 phydev->bus = new_bus;
269 276  
  277 + phydev->dev.driver = &fixed_mdio_driver.driver;
  278 + phydev->dev.release = fixed_mdio_release;
  279 + err = phydev->dev.driver->probe(&phydev->dev);
  280 + if (err < 0) {
  281 + printk(KERN_ERR "Phy %s: problems with fixed driver\n",
  282 + phydev->dev.bus_id);
  283 + goto err_out;
  284 + }
270 285 err = device_register(&phydev->dev);
271   - if(err) {
  286 + if (err) {
272 287 printk(KERN_ERR "Phy %s failed to register\n",
273   - phydev->dev.bus_id);
274   - goto bus_register_fail;
  288 + phydev->dev.bus_id);
  289 + goto err_out;
275 290 }
  291 + //phydev->state = PHY_RUNNING; /* make phy go up quick, but in 10Mbit/HDX
  292 + return fixed;
276 293  
277   - /*
278   - the mdio bus has phy_id match... In order not to do it
279   - artificially, we are binding the driver here by hand;
280   - it will be the same for all the fixed phys anyway.
281   - */
282   - phydev->dev.driver = &fixed_mdio_driver.driver;
283   -
284   - err = phydev->dev.driver->probe(&phydev->dev);
285   - if(err < 0) {
286   - printk(KERN_ERR "Phy %s: problems with fixed driver\n",phydev->dev.bus_id);
287   - goto probe_fail;
288   - }
289   -
290   - err = device_bind_driver(&phydev->dev);
291   - if (err)
292   - goto probe_fail;
293   -
294   - return 0;
295   -
296   -probe_fail:
297   - device_unregister(&phydev->dev);
298   -bus_register_fail:
  294 +err_out:
299 295 kfree(phydev);
300   -device_create_fail:
301   - kfree(dev);
302   - kfree(new_bus);
  296 +err_phy_dev_create:
  297 + kfree(fixed->regs);
  298 +err_fixed_regs_alloc:
303 299 kfree(fixed);
  300 +err_fixed_alloc:
  301 + kfree(new_bus);
  302 +err_bus_alloc:
  303 + kfree(dev);
  304 +err_dev_alloc:
304 305  
305   - return err;
  306 + return NULL;
  307 +
306 308 }
307 309 #endif
308 310  
309   -
310 311 MODULE_DESCRIPTION("Fixed PHY device & driver for PAL");
311 312 MODULE_AUTHOR("Vitaly Bordug");
312 313 MODULE_LICENSE("GPL");
313 314  
314 315 static int __init fixed_init(void)
315 316 {
316   -#if 0
317   - int ret;
318   - int duplex = 0;
319   -#endif
320   -
321   - /* register on the bus... Not expected to be matched with anything there... */
  317 + int cnt = 0;
  318 + int i;
  319 +/* register on the bus... Not expected to be matched
  320 + * with anything there...
  321 + *
  322 + */
322 323 phy_driver_register(&fixed_mdio_driver);
323 324  
324   - /* So let the fun begin...
325   - We will create several mdio devices here, and will bound the upper
326   - driver to them.
327   -
328   - Then the external software can lookup the phy bus by searching
329   - fixed@speed:duplex, e.g. fixed@100:1, to be connected to the
330   - virtual 100M Fdx phy.
331   -
332   - In case several virtual PHYs required, the bus_id will be in form
333   - fixed_<num>@<speed>:<duplex>, which make it able even to define
334   - driver-specific link control callback, if for instance PHY is completely
335   - SW-driven.
336   -
337   - */
338   -
339   -#ifdef CONFIG_FIXED_MII_DUPLEX
340   -#if 0
341   - duplex = 1;
  325 +/* We will create several mdio devices here, and will bound the upper
  326 + * driver to them.
  327 + *
  328 + * Then the external software can lookup the phy bus by searching
  329 + * for 0:101, to be connected to the virtual 100M Fdx phy.
  330 + *
  331 + * In case several virtual PHYs required, the bus_id will be in form
  332 + * [num]:[duplex]+[speed], which make it able even to define
  333 + * driver-specific link control callback, if for instance PHY is
  334 + * completely SW-driven.
  335 + */
  336 + for (i=1; i <= CONFIG_FIXED_MII_AMNT; i++) {
  337 +#ifdef CONFIG_FIXED_MII_1000_FDX
  338 + fixed_phy_ptrs[cnt++] = fixed_mdio_register_device(0, 1000, 1, i);
342 339 #endif
343   -#endif
344   -
345 340 #ifdef CONFIG_FIXED_MII_100_FDX
346   - fixed_mdio_register_device(0, 100, 1);
  341 + fixed_phy_ptrs[cnt++] = fixed_mdio_register_device(1, 100, 1, i);
347 342 #endif
348   -
349 343 #ifdef CONFIG_FIXED_MII_10_FDX
350   - fixed_mdio_register_device(0, 10, 1);
  344 + fixed_phy_ptrs[cnt++] = fixed_mdio_register_device(2, 10, 1, i);
351 345 #endif
  346 + }
  347 +
352 348 return 0;
353 349 }
354 350  
355 351 static void __exit fixed_exit(void)
356 352 {
  353 + int i;
  354 +
357 355 phy_driver_unregister(&fixed_mdio_driver);
358   - /* :WARNING:02/18/2006 04:32:40 AM:: Cleanup all the created stuff */
  356 + for (i=0; i < MAX_PHY_AMNT; i++)
  357 + if ( fixed_phy_ptrs[i] )
  358 + device_unregister(&fixed_phy_ptrs[i]->phydev->dev);
359 359 }
360 360  
361 361 module_init(fixed_init);
include/linux/phy_fixed.h
  1 +#ifndef __PHY_FIXED_H
  2 +#define __PHY_FIXED_H
  3 +
  4 +#define MII_REGS_NUM 29
  5 +
  6 +/* max number of virtual phy stuff */
  7 +#define MAX_PHY_AMNT 10
  8 +/*
  9 + The idea is to emulate normal phy behavior by responding with
  10 + pre-defined values to mii BMCR read, so that read_status hook could
  11 + take all the needed info.
  12 +*/
  13 +
  14 +struct fixed_phy_status {
  15 + u8 link;
  16 + u16 speed;
  17 + u8 duplex;
  18 +};
  19 +
  20 +/*-----------------------------------------------------------------------------
  21 + * Private information hoder for mii_bus
  22 + *-----------------------------------------------------------------------------*/
  23 +struct fixed_info {
  24 + u16 *regs;
  25 + u8 regs_num;
  26 + struct fixed_phy_status phy_status;
  27 + struct phy_device *phydev; /* pointer to the container */
  28 + /* link & speed cb */
  29 + int (*link_update) (struct net_device *, struct fixed_phy_status *);
  30 +
  31 +};
  32 +
  33 +
  34 +int fixed_mdio_set_link_update(struct phy_device *,
  35 + int (*link_update) (struct net_device *, struct fixed_phy_status *));
  36 +struct fixed_info *fixed_mdio_get_phydev (int phydev_ind);
  37 +
  38 +#endif /* __PHY_FIXED_H */