Messages in this thread | | | Date | Fri, 5 May 2017 09:38:46 -0700 | From | Greg KH <> | Subject | Re: [GIT PULL] Char/Misc driver patches for 4.12-rc1 |
| |
On Fri, May 05, 2017 at 09:00:06AM -0700, James Bottomley wrote: > On Thu, 2017-05-04 at 19:28 -0700, Linus Torvalds wrote: > > On Thu, May 4, 2017 at 5:18 PM, Greg KH <gregkh@linuxfoundation.org> > > wrote: > > > > > > Here is the big set of new char/misc driver drivers and features > > > for 4.12-rc1. > > > > Ugh. I'm not particularly happy with the conflicts I got and my > > resolutions there-of. > > Yes, we really should have done this via a postmerge tree. We've had > so little cause to use them recently, I suspect everyone's forgotten > how.
Huh? You could have pulled in my tree into this one, or I could have done that for you, my trees are not rebased at all, and they get used this way every other release or so for this very reason.
> > James, Jarkko, you need to look at that tpm merge of mine. And James, > > double-check my osd_uld thing too. > > I'm not going to defend the earlier coding, but you've lost the real > device_add() calls in the merge, meaning the tpm devices don't actually > get made visible at all. I suspect assuming device_add() is done by > cdev_device_add() because of the name is going to be our next anti > -pattern, so you're at least ahead of the game ...
It's not an anti-pattern at all, it is ment to fix the bugs you, and others, keep making :)
> @@ -272,24 +272,30 @@ EXPORT_SYMBOL_GPL(tpmm_chip_alloc); > static int tpm_add_char_device(struct tpm_chip *chip) > { > int rc; > + const char *errstr; > + struct device *errdev = &chip->dev; > > rc = cdev_device_add(&chip->cdev, &chip->dev); > if (rc) { > - dev_err(&chip->dev, > - "unable to cdev_device_add() %s, major %d, minor %d, err=%d\n", > - dev_name(&chip->dev), MAJOR(chip->dev.devt), > - MINOR(chip->dev.devt), rc); > - return rc; > + errstr = "cdev_device_add for main device"; > + goto error1; > + } > + rc = device_add(&chip->dev);
Not to pile on, but as Linus said, this is totally wrong. Did you test it? chip->dev is already registered at this point in time...
thanks,
greg k-h
| |