Re: [PATCH v5 1/2] TTY: Add TTY slave enumeration support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Jan 26, 2013 at 03:12:07AM +0000, Zheng, Lv wrote:
> > > +static void tty_slave_release(struct device *dev) {
> > > +	struct tty_slave *tts = to_tty_slave(dev);
> > > +
> > > +	kfree(tts);
> > > +	/* Test code to see if slave device get released */
> > > +	BUG();
> > > +}
> > 
> > You send me a patch like this and expect me to apply it when it's obvious you
> > didn't even test it out?  That's the most _basic_ test for any dynamic type of
> > device (add it and remove it and make sure the kernel still works.)  And
> > obviously you didn't do it.
> > 
> > I need a really good reason why to take you seriously now.
> 
> It's actually my fault.
> I'm lack of release testability then.
> Now I can do this exactly.
> According to internal reviewers' comments, it should be RFC patch to
> see if any objections there in the community.

Don't put code like this in RFC code either, that shows you didn't even
test it out, and want to waste other people's time.

And note, I almost _never_ read RFC patches, so you aren't going to get
a review from me for it, sorry, I have too many other things to do.

> I'll complete the test and refresh the patches if you think the
> functionality is interesting and useful.  Please be noted the device
> tree guys can also get benefit from such tty target enumeration
> mechanism.

No, they can use what we have today, don't reinvent the wheel.

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


[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux