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 <gre...@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
