Re: [GIT PULL] Char/Misc driver patches for 4.12-rc1

[Greg KH <gregkh@linuxfoundation.org>]

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