Messages in this thread | | | Date | Thu, 2 Jul 2009 14:22:08 -0400 | From | Forest Bond <> | Subject | Re: Staging: vt6656 ? |
| |
Hi,
On Thu, Jul 02, 2009 at 07:45:49PM +0200, Bartlomiej Zolnierkiewicz wrote: > On Sunday 28 June 2009 18:47:16 Forest Bond wrote: > > On Sun, Jun 28, 2009 at 05:59:45PM +0200, Bartlomiej Zolnierkiewicz wrote: > > > [ I'll later setup vt665x branch of my misc.git tree, merge your patches, > > > merge all outstanding vt6655 patches from Alexander and investigate a bit > > > more whether merge of vt665x drivers is feasible and what needs to be > > > done if so.. ] > > > > Good. > > The temporary tree is here: > > git://git.kernel.org:/pub/scm/linux/kernel/git/bart/misc.git vt665x > > and I'll happily apply patches to it till Greg digs out from under the > overdue patch queues.. :)
Thanks for doing this, Bartlomiej.
> > FYI, there is a known issue with the drivers as I've submitted them that causes > > lock-ups. Please see the attached message for a suggested fix. > > I think that all netdev_priv() changes should be reverted for now:
I'm happy to defer to you on this. I don't really understand the code, to be frank. However, if those changes are simply reverted, the driver will not compile. I assume that you mean those areas should be removed?
> --- a/drivers/staging/vt6655/wpactl.c > +++ b/drivers/staging/vt6655/wpactl.c > @@ -112,14 +112,17 @@ static void wpadev_setup(struct net_device *dev) > > static int wpa_init_wpadev(PSDevice pDevice) > { > + PSDevice wpadev_priv; > struct net_device *dev = pDevice->dev; > int ret=0; > > - pDevice->wpadev = alloc_netdev(0, "vntwpa", wpadev_setup); > + pDevice->wpadev = alloc_netdev(sizeof(PSDevice), "vntwpa", wpadev_setup); > if (pDevice->wpadev == NULL) > return -ENOMEM; > > - pDevice->wpadev->priv = pDevice; > + wpadev_priv = netdev_priv(pDevice->wpadev); > + *wpadev_priv = *pDevice; > + > memcpy(pDevice->wpadev->dev_addr, dev->dev_addr, U_ETHER_ADDR_LEN); > pDevice->wpadev->base_addr = dev->base_addr; > pDevice->wpadev->irq = dev->irq; > > This will copy the current state of pDevice to newly allocated private part > of ->apdev but later modifications to the original pDevice won't be seen if > we access it through netdev_priv(pDevice->apdev) instead of apdev->priv. > > [ I don't know whether this is a problem currently but it looks suspicious. ]
Agreed. I gave this a best effort, but was not very confident about the result.
Feel free to aggressively rework my changes if it seems appropriate.
Thanks, Forest -- Forest Bond http://www.alittletooquiet.net http://www.pytagsfs.org [unhandled content-type:application/pgp-signature] | |