Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1636483

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

Show all headers | View raw

On Fri, May 5, 2017 at 9:00 AM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> 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 ...

Don't be silly. That's *exactly* what cdev_device_add() does.

That's the whole and only point of the whole function: it does *both*
the cdev_add() _and_ the device_add(), and it handles errors sanely
and unwinds things.

So removing the device_add() is what the code should do. It's also
exactly what the commit I pointed you at does (the one that clashed
with your addition of the new ":[c]devs" cases. Let me repeat:

  8dbbf5825181 tpm-chip: utilize new cdev_device_add helper function

So your patch looks like complete garbage, and I think you're confused
about what cdev_device_add() is.

I think you're confusing it with the nasty old "cdev_add()" function
that indeed didn't do a "device_add()", but that's exactly why
"cdev_device_add()" as added, so that you don't have to have that
nasty pattern of having both.

That said, I do think that my merge is missing a "cdev_device_del()"
in the [c]dev if the second cdev_device_add() (on [c]devs) fails.

Although I also considered just saying "maybe if the second device
register fails, we still want to just succeed, and at least leave the
first one available".  Because why shouldn't you let people access at
least the old interface even if the new interface couldn't be set up?

So that's part of why I'd like people to look at that resolutin. But
more importantly it should be tested.

               Linus

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[GIT PULL] Char/Misc driver patches for 4.12-rc1 Greg KH <gregkh@linuxfoundation.org> - 2017-05-05 02:20 +0200
  Re: [GIT PULL] Char/Misc driver patches for 4.12-rc1 Linus Torvalds <torvalds@linux-foundation.org> - 2017-05-05 04:30 +0200
    Re: [GIT PULL] Char/Misc driver patches for 4.12-rc1 James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-05-05 18:10 +0200
      Re: [GIT PULL] Char/Misc driver patches for 4.12-rc1 Greg KH <gregkh@linuxfoundation.org> - 2017-05-05 18:40 +0200
        Re: [GIT PULL] Char/Misc driver patches for 4.12-rc1 James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-05-05 21:50 +0200
        Re: [GIT PULL] Char/Misc driver patches for 4.12-rc1 Linus Torvalds <torvalds@linux-foundation.org> - 2017-05-05 22:10 +0200
          Re: [GIT PULL] Char/Misc driver patches for 4.12-rc1 Stephen Rothwell <sfr@canb.auug.org.au> - 2017-05-06 07:10 +0200
            Re: [GIT PULL] Char/Misc driver patches for 4.12-rc1 Linus Torvalds <torvalds@linux-foundation.org> - 2017-05-06 20:10 +0200
              Re: [GIT PULL] Char/Misc driver patches for 4.12-rc1 James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-05-06 20:20 +0200
      Re: [GIT PULL] Char/Misc driver patches for 4.12-rc1 Linus Torvalds <torvalds@linux-foundation.org> - 2017-05-05 18:40 +0200
        Re: [GIT PULL] Char/Misc driver patches for 4.12-rc1 James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-05-05 18:50 +0200

csiph-web