Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_PHUB driver to 2.6.35

[Greg KH]

On Thu, Aug 19, 2010 at 09:25:03PM +0900, Masayuki Ohtake wrote:
> Please find <MASA>

If someone takes the time to review your code and ask questions, it is
considered common courtesy to at least answer them all and not ignore
some of them.  Please do so.

> > >  drivers/misc/Kconfig    |    9 +
> > >  drivers/misc/Makefile   |    1 +
> > >  drivers/misc/pch_phub.c |  722 +++++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 732 insertions(+), 0 deletions(-)
> > >  create mode 100755 drivers/misc/pch_phub.c
> >
> > You forgot to add documentation for your sysfs files in
> > Documentation/ABI/ which is a requiremend when you add new ones.
> 
> <MASA>
> Which folder should we put stable/ or testing/ ?

Which do you feel it should be in?

> > > +};
> > > +
> > > +/* SROM SPEC for MAC address assignment offset */
> > > +static const int pch_phub_mac_offset[ETH_ALEN] = {0x3, 0x2, 0x1, 0x0, 0xb, 0xa};
> > > +
> > > +static DEFINE_MUTEX(pch_phub_mutex);
> > > +static struct pch_phub_reg pch_phub_reg;
> >
> > So you can only have one of these devices in a system at the same time?
> > What happens when a machine ships with two of them?
> 
> <MASA>
> I can't understand the above questioin meaning.
> Give me more information, please.
> What's does the above "these devices" mean?

The device the driver is controlling.  What happens when this driver
runs on a system that has 2 of these devices?  You need to be able to
handle multiple devices, and that doesn't happen with a single variable,
right?  Please dynamically allocate it and make the lock associated to
the actual device, not the whole driver, if possible.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/