Re: [BUG, bisect, PATCH 04/10] tpm: move the PPI attributes to character device directory.

2015-11-06 Thread Jeremiah Mahler
Jarkko,

On Wed, Nov 04, 2015 at 10:17:05AM -0800, Jeremiah Mahler wrote:
> Jarkko, all,
> 
> On Fri, Oct 16, 2015 at 09:40:23PM +0300, Jarkko Sakkinen wrote:
> > Moved PPI attributes to the character device directory. This aligns with
> > the sysfs guidelines and makes them race free because they are created
> > atomically with the character device as part of device_register().The
> > character device and the sysfs attributes appear at the same time to the
> > user space.
> > 
> > As part of this change we enable PPI attributes also for TPM 2.0
> > devices. In order to retain backwards compatibility with TPM 1.x
> > devices, a symlink is created to the platform device directory.
> > 
> > Signed-off-by: Jarkko Sakkinen 
> > Reviewed-by: Jason Gunthorpe 
> > Tested-by: Mimi Zohar  (on TPM 1.2)
> > Tested-by: Chris J Arges 
> > Tested-by: Colin Ian King 
> > ---
> >  drivers/char/tpm/tpm-chip.c | 24 
> >  drivers/char/tpm/tpm.h  | 17 ++---
> >  drivers/char/tpm/tpm_ppi.c  | 34 +++---
> >  3 files changed, 33 insertions(+), 42 deletions(-)
> > 
[...]
> > @@ -225,10 +220,20 @@ int tpm_chip_register(struct tpm_chip *chip)
> > if (rc)
> > return rc;
> >  
> > +   tpm_add_ppi(chip);
> > +
> > rc = tpm_dev_add_device(chip);
> > if (rc)
> > goto out_err;
> >  
> > +   if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> > +   rc = __compat_only_sysfs_link_entry_to_kobj(>pdev->kobj,
> > +   >dev.kobj,
> > +   "ppi");
> > +   if (rc)
> > +   goto out_err;
> > +   }
> > +

This new call to __compat_only_sysfs_link_entry_to_kobj fails on an Acer
C720 due to a failed call to kernfs_find_and_get (discussed in a second
message).

[...]
> 
> The commit for this patch (9b774d5cf2db4) present in the latest
> linux-next (20151101+) breaks suspend/resume on an Acer C720 Chromebook.
> The computer will successfully suspend but when a resume is attempted
> a blank screen is displayed for a few seconds and then it reboots.
> 
> -- 
> - Jeremiah Mahler

-- 
- Jeremiah Mahler
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [tpmdd-devel] [BUG, bisect, PATCH 04/10] tpm: move the PPI attributes to character device directory.

2015-11-06 Thread Jarkko Sakkinen
On Thu, Nov 05, 2015 at 10:17:55AM -0800, Jeremiah Mahler wrote:
> Jarkko,
> 
> On Thu, Nov 05, 2015 at 07:46:30PM +0200, Jarkko Sakkinen wrote:
> > On Thu, Nov 05, 2015 at 08:47:58AM -0800, Jeremiah Mahler wrote:
> > > Jarkko,
> > > 
> > > On Thu, Nov 05, 2015 at 01:05:45PM +0200, Jarkko Sakkinen wrote:
> > > > On Thu, Nov 05, 2015 at 11:22:55AM +0200, Jarkko Sakkinen wrote:
> > > > > On Wed, Nov 04, 2015 at 10:17:05AM -0800, Jeremiah Mahler wrote:
> > > > > > Jarkko, all,
> > > > > > 
> > > [...]
> > > > > > 
> > > > > > The commit for this patch (9b774d5cf2db4) present in the latest
> > I have the same laptop at hand and would like to setup exactly the same
> > environment.
> > 
> > /Jarkko
> 
> The only difference is that yours is a C720P and mine is a C720.
> Hopefully they are similar enough.

The only difference is AFAIK touch screen. I had the laptop lost but
found it today. I'm going to work on this during the weekend with stable
Debian and see if I can reproduce it.

> -- 
> - Jeremiah Mahler

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [BUG, bisect, PATCH 04/10] tpm: move the PPI attributes to character device directory.

2015-11-06 Thread Jeremiah Mahler
Jarkko,

On Wed, Nov 04, 2015 at 10:17:05AM -0800, Jeremiah Mahler wrote:
> Jarkko, all,
> 
> On Fri, Oct 16, 2015 at 09:40:23PM +0300, Jarkko Sakkinen wrote:
> > Moved PPI attributes to the character device directory. This aligns with
> > the sysfs guidelines and makes them race free because they are created
> > atomically with the character device as part of device_register().The
> > character device and the sysfs attributes appear at the same time to the
> > user space.
> > 
> > As part of this change we enable PPI attributes also for TPM 2.0
> > devices. In order to retain backwards compatibility with TPM 1.x
> > devices, a symlink is created to the platform device directory.
> > 
> > Signed-off-by: Jarkko Sakkinen 
> > Reviewed-by: Jason Gunthorpe 
> > Tested-by: Mimi Zohar  (on TPM 1.2)
> > Tested-by: Chris J Arges 
> > Tested-by: Colin Ian King 
> > ---
> >  drivers/char/tpm/tpm-chip.c | 24 
> >  drivers/char/tpm/tpm.h  | 17 ++---
> >  drivers/char/tpm/tpm_ppi.c  | 34 +++---
> >  3 files changed, 33 insertions(+), 42 deletions(-)
> > 
[...]
> > @@ -225,10 +220,20 @@ int tpm_chip_register(struct tpm_chip *chip)
> > if (rc)
> > return rc;
> >  
> > +   tpm_add_ppi(chip);
> > +
> > rc = tpm_dev_add_device(chip);
> > if (rc)
> > goto out_err;
> >  
> > +   if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> > +   rc = __compat_only_sysfs_link_entry_to_kobj(>pdev->kobj,
> > +   >dev.kobj,
> > +   "ppi");
> > +   if (rc)
> > +   goto out_err;
> > +   }
> > +

This new call to __compat_only_sysfs_link_entry_to_kobj fails on an Acer
C720 due to a failed call to kernfs_find_and_get (discussed in a second
message).

[...]
> 
> The commit for this patch (9b774d5cf2db4) present in the latest
> linux-next (20151101+) breaks suspend/resume on an Acer C720 Chromebook.
> The computer will successfully suspend but when a resume is attempted
> a blank screen is displayed for a few seconds and then it reboots.
> 
> -- 
> - Jeremiah Mahler

-- 
- Jeremiah Mahler
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [tpmdd-devel] [BUG, bisect, PATCH 04/10] tpm: move the PPI attributes to character device directory.

2015-11-06 Thread Jarkko Sakkinen
On Thu, Nov 05, 2015 at 10:17:55AM -0800, Jeremiah Mahler wrote:
> Jarkko,
> 
> On Thu, Nov 05, 2015 at 07:46:30PM +0200, Jarkko Sakkinen wrote:
> > On Thu, Nov 05, 2015 at 08:47:58AM -0800, Jeremiah Mahler wrote:
> > > Jarkko,
> > > 
> > > On Thu, Nov 05, 2015 at 01:05:45PM +0200, Jarkko Sakkinen wrote:
> > > > On Thu, Nov 05, 2015 at 11:22:55AM +0200, Jarkko Sakkinen wrote:
> > > > > On Wed, Nov 04, 2015 at 10:17:05AM -0800, Jeremiah Mahler wrote:
> > > > > > Jarkko, all,
> > > > > > 
> > > [...]
> > > > > > 
> > > > > > The commit for this patch (9b774d5cf2db4) present in the latest
> > I have the same laptop at hand and would like to setup exactly the same
> > environment.
> > 
> > /Jarkko
> 
> The only difference is that yours is a C720P and mine is a C720.
> Hopefully they are similar enough.

The only difference is AFAIK touch screen. I had the laptop lost but
found it today. I'm going to work on this during the weekend with stable
Debian and see if I can reproduce it.

> -- 
> - Jeremiah Mahler

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [tpmdd-devel] [BUG, bisect, PATCH 04/10] tpm: move the PPI attributes to character device directory.

2015-11-05 Thread Jeremiah Mahler
Jarkko,

On Thu, Nov 05, 2015 at 07:46:30PM +0200, Jarkko Sakkinen wrote:
> On Thu, Nov 05, 2015 at 08:47:58AM -0800, Jeremiah Mahler wrote:
> > Jarkko,
> > 
> > On Thu, Nov 05, 2015 at 01:05:45PM +0200, Jarkko Sakkinen wrote:
> > > On Thu, Nov 05, 2015 at 11:22:55AM +0200, Jarkko Sakkinen wrote:
> > > > On Wed, Nov 04, 2015 at 10:17:05AM -0800, Jeremiah Mahler wrote:
> > > > > Jarkko, all,
> > > > > 
> > [...]
> > > > > 
> > > > > The commit for this patch (9b774d5cf2db4) present in the latest
> > > > > linux-next (20151101+) breaks suspend/resume on an Acer C720 
> > > > > Chromebook.
> > > > > The computer will successfully suspend but when a resume is attempted
> > > > > a blank screen is displayed for a few seconds and then it reboots.
> > > > 
> > > > I think I have this same model at home (have to check). If it is, I can
> > > > try to reproduce it today when I get back to home.
> > > 
> > > Yup, it's C720P!
> > > 
> > > > What kind of environment are you running?
> > > > Are you able to acquire kernel logs?
> > > 
> > > Both of these questions still apply. Even if I create the same
> > > environment, your logs might have something useful embedded.
> > > 
> > > Thank you for reporting this issue! I'll try to find a way to reproduce
> > > and fix it at latest for -rc2.
> > > 
> > > > > -- 
> > > > > - Jeremiah Mahler
> > > 
> > > /Jarkko
> > 
> > I have attached the full dmesg.  The following snippets looked
> > particularly interesting.
> 
> I'll look at the logs with time tomorrow. What kind of distribution you
> have in your system? Can you also send your kernel config?
> 
I'm running Debian unstable, not sure of the exact version.
Attached is my current .config.

> I have the same laptop at hand and would like to setup exactly the same
> environment.
> 
> /Jarkko

The only difference is that yours is a C720P and mine is a C720.
Hopefully they are similar enough.

-- 
- Jeremiah Mahler
#
# Automatically generated file; DO NOT EDIT.
# Linux/x86 4.3.0-rc4 Kernel Configuration
#
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_PERF_EVENTS_INTEL_UNCORE=y
CONFIG_OUTPUT_FORMAT="elf64-x86-64"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_HAVE_LATENCYTOP_SUPPORT=y
CONFIG_MMU=y
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_HAVE_INTEL_TXT=y
CONFIG_X86_64_SMP=y
CONFIG_ARCH_HWEIGHT_CFLAGS="-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx 
-fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 
-fcall-saved-r11"
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_PGTABLE_LEVELS=4
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y

#
# General setup
#
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
# CONFIG_COMPILE_TEST is not set
CONFIG_LOCALVERSION=""
# CONFIG_LOCALVERSION_AUTO is not set
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_HAVE_KERNEL_LZ4=y
# CONFIG_KERNEL_GZIP is not set
# CONFIG_KERNEL_BZIP2 is not set
# CONFIG_KERNEL_LZMA is not set
CONFIG_KERNEL_XZ=y
# CONFIG_KERNEL_LZO is not set
# CONFIG_KERNEL_LZ4 is not set
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_CROSS_MEMORY_ATTACH=y
CONFIG_FHANDLE=y
CONFIG_USELIB=y
CONFIG_AUDIT=y
CONFIG_HAVE_ARCH_AUDITSYSCALL=y
CONFIG_AUDITSYSCALL=y
CONFIG_AUDIT_WATCH=y
CONFIG_AUDIT_TREE=y

#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_DOMAIN_HIERARCHY=y
CONFIG_GENERIC_MSI_IRQ=y
CONFIG_GENERIC_MSI_IRQ_DOMAIN=y
# CONFIG_IRQ_DOMAIN_DEBUG is not set
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y

#
# Timers subsystem
#
CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ_COMMON=y
# CONFIG_HZ_PERIODIC is not set
CONFIG_NO_HZ_IDLE=y
# CONFIG_NO_HZ_FULL is not set
# CONFIG_NO_HZ is not set
CONFIG_HIGH_RES_TIMERS=y

#
# 

Re: [tpmdd-devel] [BUG, bisect, PATCH 04/10] tpm: move the PPI attributes to character device directory.

2015-11-05 Thread Jarkko Sakkinen
On Thu, Nov 05, 2015 at 08:47:58AM -0800, Jeremiah Mahler wrote:
> Jarkko,
> 
> On Thu, Nov 05, 2015 at 01:05:45PM +0200, Jarkko Sakkinen wrote:
> > On Thu, Nov 05, 2015 at 11:22:55AM +0200, Jarkko Sakkinen wrote:
> > > On Wed, Nov 04, 2015 at 10:17:05AM -0800, Jeremiah Mahler wrote:
> > > > Jarkko, all,
> > > > 
> [...]
> > > > 
> > > > The commit for this patch (9b774d5cf2db4) present in the latest
> > > > linux-next (20151101+) breaks suspend/resume on an Acer C720 Chromebook.
> > > > The computer will successfully suspend but when a resume is attempted
> > > > a blank screen is displayed for a few seconds and then it reboots.
> > > 
> > > I think I have this same model at home (have to check). If it is, I can
> > > try to reproduce it today when I get back to home.
> > 
> > Yup, it's C720P!
> > 
> > > What kind of environment are you running?
> > > Are you able to acquire kernel logs?
> > 
> > Both of these questions still apply. Even if I create the same
> > environment, your logs might have something useful embedded.
> > 
> > Thank you for reporting this issue! I'll try to find a way to reproduce
> > and fix it at latest for -rc2.
> > 
> > > > -- 
> > > > - Jeremiah Mahler
> > 
> > /Jarkko
> 
> I have attached the full dmesg.  The following snippets looked
> particularly interesting.

I'll look at the logs with time tomorrow. What kind of distribution you
have in your system? Can you also send your kernel config?

I have the same laptop at hand and would like to setup exactly the same
environment.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [tpmdd-devel] [BUG, bisect, PATCH 04/10] tpm: move the PPI attributes to character device directory.

2015-11-05 Thread Jeremiah Mahler
Jarkko,

On Thu, Nov 05, 2015 at 01:05:45PM +0200, Jarkko Sakkinen wrote:
> On Thu, Nov 05, 2015 at 11:22:55AM +0200, Jarkko Sakkinen wrote:
> > On Wed, Nov 04, 2015 at 10:17:05AM -0800, Jeremiah Mahler wrote:
> > > Jarkko, all,
> > > 
[...]
> > > 
> > > The commit for this patch (9b774d5cf2db4) present in the latest
> > > linux-next (20151101+) breaks suspend/resume on an Acer C720 Chromebook.
> > > The computer will successfully suspend but when a resume is attempted
> > > a blank screen is displayed for a few seconds and then it reboots.
> > 
> > I think I have this same model at home (have to check). If it is, I can
> > try to reproduce it today when I get back to home.
> 
> Yup, it's C720P!
> 
> > What kind of environment are you running?
> > Are you able to acquire kernel logs?
> 
> Both of these questions still apply. Even if I create the same
> environment, your logs might have something useful embedded.
> 
> Thank you for reporting this issue! I'll try to find a way to reproduce
> and fix it at latest for -rc2.
> 
> > > -- 
> > > - Jeremiah Mahler
> 
> /Jarkko

I have attached the full dmesg.  The following snippets looked
particularly interesting.

  ...
  [2.423070] input: PC Speaker as /devices/platform/pcspkr/input/input10
  [2.453071] [ cut here ]
  [2.453078] WARNING: CPU: 0 PID: 237 at kernel/irq/manage.c:1379 
__free_irq+0x90/0x1e0()
  [2.453079] Trying to free already-free IRQ 6
  [2.453110] Modules linked in: evdev serio_raw pcspkr cfg80211 sg i915 
lpc_ich mfd_core i2c_i801 battery ac snd_hda_codec_realtek ath3k 
snd_hda_codec_generic i2c_algo_bit btusb drm_kms_helper btrtl btbcm btintel 
snd_hda_intel bluetooth drm snd_hda_codec snd_hwdep snd_hda_core video snd_pcm 
rfkill tpm_tis(+) snd_timer tpm snd button i2c_designware_pci 
i2c_designware_core shpchp i2c_core soundcore autofs4 ext4 crc16 mbcache jbd2 
sd_mod ahci libahci fan sdhci_acpi sdhci libata mmc_core xhci_pci scsi_mod 
xhci_hcd thermal usbcore usb_common
  [2.453113] CPU: 0 PID: 237 Comm: systemd-udevd Tainted: GW   
4.3.0-rc4+ #271
  [2.453114] Hardware name: Acer Peppy, BIOS  04/30/2014
  [2.453117]  817288a1 812c37a7 880035a6fad0 
8106d8bd
  [2.453119]  880100421800 880035a6fb20 880100421800 
0006
  [2.453121]  8801004218d8 8106d93c 8171e8c8 
8820
  [2.453122] Call Trace:
  [2.453127]  [] ? dump_stack+0x40/0x59
  [2.453132]  [] ? warn_slowpath_common+0x7d/0xb0
  [2.453135]  [] ? warn_slowpath_fmt+0x4c/0x50
  [2.453140]  [] ? tpm_chip_register+0x142/0x190 [tpm]
  [2.453142]  [] ? __free_irq+0x90/0x1e0
  [2.453144]  [] ? free_irq+0x45/0xb0
  [2.453148]  [] ? release_nodes+0xf5/0x1c0
  [2.453152]  [] ? driver_probe_device+0xcd/0x480
  [2.453155]  [] ? __driver_attach+0x7b/0x80
  [2.453158]  [] ? driver_probe_device+0x480/0x480
  [2.453161]  [] ? bus_for_each_dev+0x5a/0x90
  [2.453164]  [] ? bus_add_driver+0x1df/0x270
  [2.453166]  [] ? 0xa00d1000
  [2.453169]  [] ? driver_register+0x57/0xc0
  [2.453173]  [] ? init_tis+0x25/0x1000 [tpm_tis]
  [2.453178]  [] ? free_pcppages_bulk+0xc9/0x450
  [2.453179]  [] ? 0xa00d1000
  [2.453182]  [] ? do_one_initcall+0xb2/0x200
  [2.453185]  [] ? do_init_module+0x5b/0x1dc
  [2.453188]  [] ? load_module+0x2197/0x27a0
  [2.453190]  [] ? __symbol_put+0x30/0x30
  [2.453194]  [] ? SyS_finit_module+0x90/0xc0
  [2.453198]  [] ? entry_SYSCALL_64_fastpath+0x16/0x75
  [2.453200] ---[ end trace 0835dfb15879b441 ]---
  [2.453212] tpm_tis: probe of 00:08 failed with error -2
  [2.456012] tpm_inf_pnp 00:08: Found TPM with ID IFX0102
  [2.544707] usb 1-4: new full-speed USB device number 4 using xhci_hcd
  [2.571036] __add_probed_i2c_device failed to register device 1-4a
  ...

Notice the "tpm_tis: probe of ... failed with error".  With a working
kernel this probe succeeds.  However, the probe fails, which causes it
to try unload the module which produces the "free already-free IRQ"
trace.  The IRQ message is a secondary issue.  Interrupts were disabled
due to a firmware bug and it is trying to free them anyway.

  ...
  [2.244779] tpm_tis 00:08: [Firmware Bug]: TPM interrupt not working, 
polling instead
  ...

So the patch changed something which causes the probe to fail.  I
haven't figured out what that is yet.

-- 
- Jeremiah Mahler
[0.00] Initializing cgroup subsys cpuset
[0.00] Initializing cgroup subsys cpu
[0.00] Initializing cgroup subsys cpuacct
[0.00] Linux version 4.3.0-rc4+ (jeri@newt) (gcc version 5.2.1 20151010 
(Debian 5.2.1-22) ) #271 SMP Wed Nov 4 10:02:50 PST 2015
[0.00] Command line: BOOT_IMAGE=/boot/vmlinuz-4.3.0-rc4+ 
root=UUID=e5fd0a8b-e57d-428c-831e-3aff30526800 ro quiet
[0.00] x86/fpu: Supporting XSAVE feature 0x01: 'x87 floating 

Re: [tpmdd-devel] [BUG, bisect, PATCH 04/10] tpm: move the PPI attributes to character device directory.

2015-11-05 Thread Jarkko Sakkinen
On Thu, Nov 05, 2015 at 11:22:55AM +0200, Jarkko Sakkinen wrote:
> On Wed, Nov 04, 2015 at 10:17:05AM -0800, Jeremiah Mahler wrote:
> > Jarkko, all,
> > 
> > On Fri, Oct 16, 2015 at 09:40:23PM +0300, Jarkko Sakkinen wrote:
> > > Moved PPI attributes to the character device directory. This aligns with
> > > the sysfs guidelines and makes them race free because they are created
> > > atomically with the character device as part of device_register().The
> > > character device and the sysfs attributes appear at the same time to the
> > > user space.
> > > 
> > > As part of this change we enable PPI attributes also for TPM 2.0
> > > devices. In order to retain backwards compatibility with TPM 1.x
> > > devices, a symlink is created to the platform device directory.
> > > 
> > > Signed-off-by: Jarkko Sakkinen 
> > > Reviewed-by: Jason Gunthorpe 
> > > Tested-by: Mimi Zohar  (on TPM 1.2)
> > > Tested-by: Chris J Arges 
> > > Tested-by: Colin Ian King 
> > > ---
> > >  drivers/char/tpm/tpm-chip.c | 24 
> > >  drivers/char/tpm/tpm.h  | 17 ++---
> > >  drivers/char/tpm/tpm_ppi.c  | 34 +++---
> > >  3 files changed, 33 insertions(+), 42 deletions(-)
> > > 
> > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > > index 1082d4b..f26b0ae 100644
> > > --- a/drivers/char/tpm/tpm-chip.c
> > > +++ b/drivers/char/tpm/tpm-chip.c
> > > @@ -119,6 +119,9 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
> > >   chip->dev.class = tpm_class;
> > >   chip->dev.release = tpm_dev_release;
> > >   chip->dev.parent = chip->pdev;
> > > +#ifdef CONFIG_ACPI
> > > + chip->dev.groups = chip->groups;
> > > +#endif
> > >  
> > >   if (chip->dev_num == 0)
> > >   chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
> > > @@ -182,12 +185,6 @@ static int tpm1_chip_register(struct tpm_chip *chip)
> > >   if (rc)
> > >   return rc;
> > >  
> > > - rc = tpm_add_ppi(chip);
> > > - if (rc) {
> > > - tpm_sysfs_del_device(chip);
> > > - return rc;
> > > - }
> > > -
> > >   chip->bios_dir = tpm_bios_log_setup(chip->devname);
> > >  
> > >   return 0;
> > > @@ -201,8 +198,6 @@ static void tpm1_chip_unregister(struct tpm_chip 
> > > *chip)
> > >   if (chip->bios_dir)
> > >   tpm_bios_log_teardown(chip->bios_dir);
> > >  
> > > - tpm_remove_ppi(chip);
> > > -
> > >   tpm_sysfs_del_device(chip);
> > >  }
> > >  
> > > @@ -225,10 +220,20 @@ int tpm_chip_register(struct tpm_chip *chip)
> > >   if (rc)
> > >   return rc;
> > >  
> > > + tpm_add_ppi(chip);
> > > +
> > >   rc = tpm_dev_add_device(chip);
> > >   if (rc)
> > >   goto out_err;
> > >  
> > > + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> > > + rc = __compat_only_sysfs_link_entry_to_kobj(>pdev->kobj,
> > > + >dev.kobj,
> > > + "ppi");
> > > + if (rc)
> > > + goto out_err;
> > > + }
> > > +
> > >   /* Make the chip available. */
> > >   spin_lock(_lock);
> > >   list_add_rcu(>list, _chip_list);
> > > @@ -263,6 +268,9 @@ void tpm_chip_unregister(struct tpm_chip *chip)
> > >   spin_unlock(_lock);
> > >   synchronize_rcu();
> > >  
> > > + if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> > > + sysfs_remove_link(>pdev->kobj, "ppi");
> > > +
> > >   tpm1_chip_unregister(chip);
> > >   tpm_dev_del_device(chip);
> > >  }
> > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > > index 39be5ac..36ceb71 100644
> > > --- a/drivers/char/tpm/tpm.h
> > > +++ b/drivers/char/tpm/tpm.h
> > > @@ -158,8 +158,7 @@ struct tpm_vendor_specific {
> > >  
> > >  enum tpm_chip_flags {
> > >   TPM_CHIP_FLAG_REGISTERED= BIT(0),
> > > - TPM_CHIP_FLAG_PPI   = BIT(1),
> > > - TPM_CHIP_FLAG_TPM2  = BIT(2),
> > > + TPM_CHIP_FLAG_TPM2  = BIT(1),
> > >  };
> > >  
> > >  struct tpm_chip {
> > > @@ -182,6 +181,8 @@ struct tpm_chip {
> > >   struct dentry **bios_dir;
> > >  
> > >  #ifdef CONFIG_ACPI
> > > + const struct attribute_group *groups[2];
> > > + unsigned int groups_cnt;
> > >   acpi_handle acpi_dev_handle;
> > >   char ppi_version[TPM_PPI_VERSION_LEN + 1];
> > >  #endif /* CONFIG_ACPI */
> > > @@ -189,7 +190,7 @@ struct tpm_chip {
> > >   struct list_head list;
> > >  };
> > >  
> > > -#define to_tpm_chip(n) container_of(n, struct tpm_chip, vendor)
> > > +#define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
> > >  
> > >  static inline void tpm_chip_put(struct tpm_chip *chip)
> > >  {
> > > @@ -419,15 +420,9 @@ void tpm_sysfs_del_device(struct tpm_chip *chip);
> > >  int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
> > >  
> > >  #ifdef CONFIG_ACPI
> > > -extern int tpm_add_ppi(struct tpm_chip *chip);
> > > -extern void tpm_remove_ppi(struct tpm_chip *chip);
> > > +extern void tpm_add_ppi(struct tpm_chip *chip);
> > >  #else
> > > -static inline int 

Re: [BUG, bisect, PATCH 04/10] tpm: move the PPI attributes to character device directory.

2015-11-05 Thread Jarkko Sakkinen
On Wed, Nov 04, 2015 at 10:17:05AM -0800, Jeremiah Mahler wrote:
> Jarkko, all,
> 
> On Fri, Oct 16, 2015 at 09:40:23PM +0300, Jarkko Sakkinen wrote:
> > Moved PPI attributes to the character device directory. This aligns with
> > the sysfs guidelines and makes them race free because they are created
> > atomically with the character device as part of device_register().The
> > character device and the sysfs attributes appear at the same time to the
> > user space.
> > 
> > As part of this change we enable PPI attributes also for TPM 2.0
> > devices. In order to retain backwards compatibility with TPM 1.x
> > devices, a symlink is created to the platform device directory.
> > 
> > Signed-off-by: Jarkko Sakkinen 
> > Reviewed-by: Jason Gunthorpe 
> > Tested-by: Mimi Zohar  (on TPM 1.2)
> > Tested-by: Chris J Arges 
> > Tested-by: Colin Ian King 
> > ---
> >  drivers/char/tpm/tpm-chip.c | 24 
> >  drivers/char/tpm/tpm.h  | 17 ++---
> >  drivers/char/tpm/tpm_ppi.c  | 34 +++---
> >  3 files changed, 33 insertions(+), 42 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > index 1082d4b..f26b0ae 100644
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -119,6 +119,9 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
> > chip->dev.class = tpm_class;
> > chip->dev.release = tpm_dev_release;
> > chip->dev.parent = chip->pdev;
> > +#ifdef CONFIG_ACPI
> > +   chip->dev.groups = chip->groups;
> > +#endif
> >  
> > if (chip->dev_num == 0)
> > chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
> > @@ -182,12 +185,6 @@ static int tpm1_chip_register(struct tpm_chip *chip)
> > if (rc)
> > return rc;
> >  
> > -   rc = tpm_add_ppi(chip);
> > -   if (rc) {
> > -   tpm_sysfs_del_device(chip);
> > -   return rc;
> > -   }
> > -
> > chip->bios_dir = tpm_bios_log_setup(chip->devname);
> >  
> > return 0;
> > @@ -201,8 +198,6 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
> > if (chip->bios_dir)
> > tpm_bios_log_teardown(chip->bios_dir);
> >  
> > -   tpm_remove_ppi(chip);
> > -
> > tpm_sysfs_del_device(chip);
> >  }
> >  
> > @@ -225,10 +220,20 @@ int tpm_chip_register(struct tpm_chip *chip)
> > if (rc)
> > return rc;
> >  
> > +   tpm_add_ppi(chip);
> > +
> > rc = tpm_dev_add_device(chip);
> > if (rc)
> > goto out_err;
> >  
> > +   if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> > +   rc = __compat_only_sysfs_link_entry_to_kobj(>pdev->kobj,
> > +   >dev.kobj,
> > +   "ppi");
> > +   if (rc)
> > +   goto out_err;
> > +   }
> > +
> > /* Make the chip available. */
> > spin_lock(_lock);
> > list_add_rcu(>list, _chip_list);
> > @@ -263,6 +268,9 @@ void tpm_chip_unregister(struct tpm_chip *chip)
> > spin_unlock(_lock);
> > synchronize_rcu();
> >  
> > +   if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> > +   sysfs_remove_link(>pdev->kobj, "ppi");
> > +
> > tpm1_chip_unregister(chip);
> > tpm_dev_del_device(chip);
> >  }
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > index 39be5ac..36ceb71 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -158,8 +158,7 @@ struct tpm_vendor_specific {
> >  
> >  enum tpm_chip_flags {
> > TPM_CHIP_FLAG_REGISTERED= BIT(0),
> > -   TPM_CHIP_FLAG_PPI   = BIT(1),
> > -   TPM_CHIP_FLAG_TPM2  = BIT(2),
> > +   TPM_CHIP_FLAG_TPM2  = BIT(1),
> >  };
> >  
> >  struct tpm_chip {
> > @@ -182,6 +181,8 @@ struct tpm_chip {
> > struct dentry **bios_dir;
> >  
> >  #ifdef CONFIG_ACPI
> > +   const struct attribute_group *groups[2];
> > +   unsigned int groups_cnt;
> > acpi_handle acpi_dev_handle;
> > char ppi_version[TPM_PPI_VERSION_LEN + 1];
> >  #endif /* CONFIG_ACPI */
> > @@ -189,7 +190,7 @@ struct tpm_chip {
> > struct list_head list;
> >  };
> >  
> > -#define to_tpm_chip(n) container_of(n, struct tpm_chip, vendor)
> > +#define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
> >  
> >  static inline void tpm_chip_put(struct tpm_chip *chip)
> >  {
> > @@ -419,15 +420,9 @@ void tpm_sysfs_del_device(struct tpm_chip *chip);
> >  int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
> >  
> >  #ifdef CONFIG_ACPI
> > -extern int tpm_add_ppi(struct tpm_chip *chip);
> > -extern void tpm_remove_ppi(struct tpm_chip *chip);
> > +extern void tpm_add_ppi(struct tpm_chip *chip);
> >  #else
> > -static inline int tpm_add_ppi(struct tpm_chip *chip)
> > -{
> > -   return 0;
> > -}
> > -
> > -static inline void tpm_remove_ppi(struct tpm_chip *chip)
> > +static inline void tpm_add_ppi(struct tpm_chip *chip)
> >  {
> >  }
> >  #endif
> > diff --git 

Re: [BUG, bisect, PATCH 04/10] tpm: move the PPI attributes to character device directory.

2015-11-05 Thread Jarkko Sakkinen
On Wed, Nov 04, 2015 at 10:17:05AM -0800, Jeremiah Mahler wrote:
> Jarkko, all,
> 
> On Fri, Oct 16, 2015 at 09:40:23PM +0300, Jarkko Sakkinen wrote:
> > Moved PPI attributes to the character device directory. This aligns with
> > the sysfs guidelines and makes them race free because they are created
> > atomically with the character device as part of device_register().The
> > character device and the sysfs attributes appear at the same time to the
> > user space.
> > 
> > As part of this change we enable PPI attributes also for TPM 2.0
> > devices. In order to retain backwards compatibility with TPM 1.x
> > devices, a symlink is created to the platform device directory.
> > 
> > Signed-off-by: Jarkko Sakkinen 
> > Reviewed-by: Jason Gunthorpe 
> > Tested-by: Mimi Zohar  (on TPM 1.2)
> > Tested-by: Chris J Arges 
> > Tested-by: Colin Ian King 
> > ---
> >  drivers/char/tpm/tpm-chip.c | 24 
> >  drivers/char/tpm/tpm.h  | 17 ++---
> >  drivers/char/tpm/tpm_ppi.c  | 34 +++---
> >  3 files changed, 33 insertions(+), 42 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > index 1082d4b..f26b0ae 100644
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -119,6 +119,9 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
> > chip->dev.class = tpm_class;
> > chip->dev.release = tpm_dev_release;
> > chip->dev.parent = chip->pdev;
> > +#ifdef CONFIG_ACPI
> > +   chip->dev.groups = chip->groups;
> > +#endif
> >  
> > if (chip->dev_num == 0)
> > chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
> > @@ -182,12 +185,6 @@ static int tpm1_chip_register(struct tpm_chip *chip)
> > if (rc)
> > return rc;
> >  
> > -   rc = tpm_add_ppi(chip);
> > -   if (rc) {
> > -   tpm_sysfs_del_device(chip);
> > -   return rc;
> > -   }
> > -
> > chip->bios_dir = tpm_bios_log_setup(chip->devname);
> >  
> > return 0;
> > @@ -201,8 +198,6 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
> > if (chip->bios_dir)
> > tpm_bios_log_teardown(chip->bios_dir);
> >  
> > -   tpm_remove_ppi(chip);
> > -
> > tpm_sysfs_del_device(chip);
> >  }
> >  
> > @@ -225,10 +220,20 @@ int tpm_chip_register(struct tpm_chip *chip)
> > if (rc)
> > return rc;
> >  
> > +   tpm_add_ppi(chip);
> > +
> > rc = tpm_dev_add_device(chip);
> > if (rc)
> > goto out_err;
> >  
> > +   if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> > +   rc = __compat_only_sysfs_link_entry_to_kobj(>pdev->kobj,
> > +   >dev.kobj,
> > +   "ppi");
> > +   if (rc)
> > +   goto out_err;
> > +   }
> > +
> > /* Make the chip available. */
> > spin_lock(_lock);
> > list_add_rcu(>list, _chip_list);
> > @@ -263,6 +268,9 @@ void tpm_chip_unregister(struct tpm_chip *chip)
> > spin_unlock(_lock);
> > synchronize_rcu();
> >  
> > +   if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> > +   sysfs_remove_link(>pdev->kobj, "ppi");
> > +
> > tpm1_chip_unregister(chip);
> > tpm_dev_del_device(chip);
> >  }
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > index 39be5ac..36ceb71 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -158,8 +158,7 @@ struct tpm_vendor_specific {
> >  
> >  enum tpm_chip_flags {
> > TPM_CHIP_FLAG_REGISTERED= BIT(0),
> > -   TPM_CHIP_FLAG_PPI   = BIT(1),
> > -   TPM_CHIP_FLAG_TPM2  = BIT(2),
> > +   TPM_CHIP_FLAG_TPM2  = BIT(1),
> >  };
> >  
> >  struct tpm_chip {
> > @@ -182,6 +181,8 @@ struct tpm_chip {
> > struct dentry **bios_dir;
> >  
> >  #ifdef CONFIG_ACPI
> > +   const struct attribute_group *groups[2];
> > +   unsigned int groups_cnt;
> > acpi_handle acpi_dev_handle;
> > char ppi_version[TPM_PPI_VERSION_LEN + 1];
> >  #endif /* CONFIG_ACPI */
> > @@ -189,7 +190,7 @@ struct tpm_chip {
> > struct list_head list;
> >  };
> >  
> > -#define to_tpm_chip(n) container_of(n, struct tpm_chip, vendor)
> > +#define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
> >  
> >  static inline void tpm_chip_put(struct tpm_chip *chip)
> >  {
> > @@ -419,15 +420,9 @@ void tpm_sysfs_del_device(struct tpm_chip *chip);
> >  int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
> >  
> >  #ifdef CONFIG_ACPI
> > -extern int tpm_add_ppi(struct tpm_chip *chip);
> > -extern void tpm_remove_ppi(struct tpm_chip *chip);
> > +extern void tpm_add_ppi(struct tpm_chip *chip);
> >  #else
> > -static inline int tpm_add_ppi(struct tpm_chip *chip)
> > -{
> > -   return 0;
> > -}
> > -
> > -static 

Re: [tpmdd-devel] [BUG, bisect, PATCH 04/10] tpm: move the PPI attributes to character device directory.

2015-11-05 Thread Jarkko Sakkinen
On Thu, Nov 05, 2015 at 11:22:55AM +0200, Jarkko Sakkinen wrote:
> On Wed, Nov 04, 2015 at 10:17:05AM -0800, Jeremiah Mahler wrote:
> > Jarkko, all,
> > 
> > On Fri, Oct 16, 2015 at 09:40:23PM +0300, Jarkko Sakkinen wrote:
> > > Moved PPI attributes to the character device directory. This aligns with
> > > the sysfs guidelines and makes them race free because they are created
> > > atomically with the character device as part of device_register().The
> > > character device and the sysfs attributes appear at the same time to the
> > > user space.
> > > 
> > > As part of this change we enable PPI attributes also for TPM 2.0
> > > devices. In order to retain backwards compatibility with TPM 1.x
> > > devices, a symlink is created to the platform device directory.
> > > 
> > > Signed-off-by: Jarkko Sakkinen 
> > > Reviewed-by: Jason Gunthorpe 
> > > Tested-by: Mimi Zohar  (on TPM 1.2)
> > > Tested-by: Chris J Arges 
> > > Tested-by: Colin Ian King 
> > > ---
> > >  drivers/char/tpm/tpm-chip.c | 24 
> > >  drivers/char/tpm/tpm.h  | 17 ++---
> > >  drivers/char/tpm/tpm_ppi.c  | 34 +++---
> > >  3 files changed, 33 insertions(+), 42 deletions(-)
> > > 
> > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > > index 1082d4b..f26b0ae 100644
> > > --- a/drivers/char/tpm/tpm-chip.c
> > > +++ b/drivers/char/tpm/tpm-chip.c
> > > @@ -119,6 +119,9 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
> > >   chip->dev.class = tpm_class;
> > >   chip->dev.release = tpm_dev_release;
> > >   chip->dev.parent = chip->pdev;
> > > +#ifdef CONFIG_ACPI
> > > + chip->dev.groups = chip->groups;
> > > +#endif
> > >  
> > >   if (chip->dev_num == 0)
> > >   chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
> > > @@ -182,12 +185,6 @@ static int tpm1_chip_register(struct tpm_chip *chip)
> > >   if (rc)
> > >   return rc;
> > >  
> > > - rc = tpm_add_ppi(chip);
> > > - if (rc) {
> > > - tpm_sysfs_del_device(chip);
> > > - return rc;
> > > - }
> > > -
> > >   chip->bios_dir = tpm_bios_log_setup(chip->devname);
> > >  
> > >   return 0;
> > > @@ -201,8 +198,6 @@ static void tpm1_chip_unregister(struct tpm_chip 
> > > *chip)
> > >   if (chip->bios_dir)
> > >   tpm_bios_log_teardown(chip->bios_dir);
> > >  
> > > - tpm_remove_ppi(chip);
> > > -
> > >   tpm_sysfs_del_device(chip);
> > >  }
> > >  
> > > @@ -225,10 +220,20 @@ int tpm_chip_register(struct tpm_chip *chip)
> > >   if (rc)
> > >   return rc;
> > >  
> > > + tpm_add_ppi(chip);
> > > +
> > >   rc = tpm_dev_add_device(chip);
> > >   if (rc)
> > >   goto out_err;
> > >  
> > > + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> > > + rc = __compat_only_sysfs_link_entry_to_kobj(>pdev->kobj,
> > > + >dev.kobj,
> > > + "ppi");
> > > + if (rc)
> > > + goto out_err;
> > > + }
> > > +
> > >   /* Make the chip available. */
> > >   spin_lock(_lock);
> > >   list_add_rcu(>list, _chip_list);
> > > @@ -263,6 +268,9 @@ void tpm_chip_unregister(struct tpm_chip *chip)
> > >   spin_unlock(_lock);
> > >   synchronize_rcu();
> > >  
> > > + if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> > > + sysfs_remove_link(>pdev->kobj, "ppi");
> > > +
> > >   tpm1_chip_unregister(chip);
> > >   tpm_dev_del_device(chip);
> > >  }
> > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > > index 39be5ac..36ceb71 100644
> > > --- a/drivers/char/tpm/tpm.h
> > > +++ b/drivers/char/tpm/tpm.h
> > > @@ -158,8 +158,7 @@ struct tpm_vendor_specific {
> > >  
> > >  enum tpm_chip_flags {
> > >   TPM_CHIP_FLAG_REGISTERED= BIT(0),
> > > - TPM_CHIP_FLAG_PPI   = BIT(1),
> > > - TPM_CHIP_FLAG_TPM2  = BIT(2),
> > > + TPM_CHIP_FLAG_TPM2  = BIT(1),
> > >  };
> > >  
> > >  struct tpm_chip {
> > > @@ -182,6 +181,8 @@ struct tpm_chip {
> > >   struct dentry **bios_dir;
> > >  
> > >  #ifdef CONFIG_ACPI
> > > + const struct attribute_group *groups[2];
> > > + unsigned int groups_cnt;
> > >   acpi_handle acpi_dev_handle;
> > >   char ppi_version[TPM_PPI_VERSION_LEN + 1];
> > >  #endif /* CONFIG_ACPI */
> > > @@ -189,7 +190,7 @@ struct tpm_chip {
> > >   struct list_head list;
> > >  };
> > >  
> > > -#define to_tpm_chip(n) container_of(n, struct tpm_chip, vendor)
> > > +#define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
> > >  
> > >  static inline void tpm_chip_put(struct tpm_chip *chip)
> > >  {
> > > @@ -419,15 +420,9 @@ void tpm_sysfs_del_device(struct tpm_chip *chip);
> > >  int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
> > >  
> > >  #ifdef CONFIG_ACPI
> > > -extern int tpm_add_ppi(struct tpm_chip *chip);
> > > 

Re: [tpmdd-devel] [BUG, bisect, PATCH 04/10] tpm: move the PPI attributes to character device directory.

2015-11-05 Thread Jeremiah Mahler
Jarkko,

On Thu, Nov 05, 2015 at 01:05:45PM +0200, Jarkko Sakkinen wrote:
> On Thu, Nov 05, 2015 at 11:22:55AM +0200, Jarkko Sakkinen wrote:
> > On Wed, Nov 04, 2015 at 10:17:05AM -0800, Jeremiah Mahler wrote:
> > > Jarkko, all,
> > > 
[...]
> > > 
> > > The commit for this patch (9b774d5cf2db4) present in the latest
> > > linux-next (20151101+) breaks suspend/resume on an Acer C720 Chromebook.
> > > The computer will successfully suspend but when a resume is attempted
> > > a blank screen is displayed for a few seconds and then it reboots.
> > 
> > I think I have this same model at home (have to check). If it is, I can
> > try to reproduce it today when I get back to home.
> 
> Yup, it's C720P!
> 
> > What kind of environment are you running?
> > Are you able to acquire kernel logs?
> 
> Both of these questions still apply. Even if I create the same
> environment, your logs might have something useful embedded.
> 
> Thank you for reporting this issue! I'll try to find a way to reproduce
> and fix it at latest for -rc2.
> 
> > > -- 
> > > - Jeremiah Mahler
> 
> /Jarkko

I have attached the full dmesg.  The following snippets looked
particularly interesting.

  ...
  [2.423070] input: PC Speaker as /devices/platform/pcspkr/input/input10
  [2.453071] [ cut here ]
  [2.453078] WARNING: CPU: 0 PID: 237 at kernel/irq/manage.c:1379 
__free_irq+0x90/0x1e0()
  [2.453079] Trying to free already-free IRQ 6
  [2.453110] Modules linked in: evdev serio_raw pcspkr cfg80211 sg i915 
lpc_ich mfd_core i2c_i801 battery ac snd_hda_codec_realtek ath3k 
snd_hda_codec_generic i2c_algo_bit btusb drm_kms_helper btrtl btbcm btintel 
snd_hda_intel bluetooth drm snd_hda_codec snd_hwdep snd_hda_core video snd_pcm 
rfkill tpm_tis(+) snd_timer tpm snd button i2c_designware_pci 
i2c_designware_core shpchp i2c_core soundcore autofs4 ext4 crc16 mbcache jbd2 
sd_mod ahci libahci fan sdhci_acpi sdhci libata mmc_core xhci_pci scsi_mod 
xhci_hcd thermal usbcore usb_common
  [2.453113] CPU: 0 PID: 237 Comm: systemd-udevd Tainted: GW   
4.3.0-rc4+ #271
  [2.453114] Hardware name: Acer Peppy, BIOS  04/30/2014
  [2.453117]  817288a1 812c37a7 880035a6fad0 
8106d8bd
  [2.453119]  880100421800 880035a6fb20 880100421800 
0006
  [2.453121]  8801004218d8 8106d93c 8171e8c8 
8820
  [2.453122] Call Trace:
  [2.453127]  [] ? dump_stack+0x40/0x59
  [2.453132]  [] ? warn_slowpath_common+0x7d/0xb0
  [2.453135]  [] ? warn_slowpath_fmt+0x4c/0x50
  [2.453140]  [] ? tpm_chip_register+0x142/0x190 [tpm]
  [2.453142]  [] ? __free_irq+0x90/0x1e0
  [2.453144]  [] ? free_irq+0x45/0xb0
  [2.453148]  [] ? release_nodes+0xf5/0x1c0
  [2.453152]  [] ? driver_probe_device+0xcd/0x480
  [2.453155]  [] ? __driver_attach+0x7b/0x80
  [2.453158]  [] ? driver_probe_device+0x480/0x480
  [2.453161]  [] ? bus_for_each_dev+0x5a/0x90
  [2.453164]  [] ? bus_add_driver+0x1df/0x270
  [2.453166]  [] ? 0xa00d1000
  [2.453169]  [] ? driver_register+0x57/0xc0
  [2.453173]  [] ? init_tis+0x25/0x1000 [tpm_tis]
  [2.453178]  [] ? free_pcppages_bulk+0xc9/0x450
  [2.453179]  [] ? 0xa00d1000
  [2.453182]  [] ? do_one_initcall+0xb2/0x200
  [2.453185]  [] ? do_init_module+0x5b/0x1dc
  [2.453188]  [] ? load_module+0x2197/0x27a0
  [2.453190]  [] ? __symbol_put+0x30/0x30
  [2.453194]  [] ? SyS_finit_module+0x90/0xc0
  [2.453198]  [] ? entry_SYSCALL_64_fastpath+0x16/0x75
  [2.453200] ---[ end trace 0835dfb15879b441 ]---
  [2.453212] tpm_tis: probe of 00:08 failed with error -2
  [2.456012] tpm_inf_pnp 00:08: Found TPM with ID IFX0102
  [2.544707] usb 1-4: new full-speed USB device number 4 using xhci_hcd
  [2.571036] __add_probed_i2c_device failed to register device 1-4a
  ...

Notice the "tpm_tis: probe of ... failed with error".  With a working
kernel this probe succeeds.  However, the probe fails, which causes it
to try unload the module which produces the "free already-free IRQ"
trace.  The IRQ message is a secondary issue.  Interrupts were disabled
due to a firmware bug and it is trying to free them anyway.

  ...
  [2.244779] tpm_tis 00:08: [Firmware Bug]: TPM interrupt not working, 
polling instead
  ...

So the patch changed something which causes the probe to fail.  I
haven't figured out what that is yet.

-- 
- Jeremiah Mahler
[0.00] Initializing cgroup subsys cpuset
[0.00] Initializing cgroup subsys cpu
[0.00] Initializing cgroup subsys cpuacct
[0.00] Linux version 4.3.0-rc4+ (jeri@newt) (gcc version 5.2.1 20151010 
(Debian 5.2.1-22) ) #271 SMP Wed Nov 4 10:02:50 PST 2015
[0.00] Command line: BOOT_IMAGE=/boot/vmlinuz-4.3.0-rc4+ 
root=UUID=e5fd0a8b-e57d-428c-831e-3aff30526800 ro quiet
[0.00] x86/fpu: Supporting XSAVE feature 0x01: 'x87 floating 

Re: [tpmdd-devel] [BUG, bisect, PATCH 04/10] tpm: move the PPI attributes to character device directory.

2015-11-05 Thread Jarkko Sakkinen
On Thu, Nov 05, 2015 at 08:47:58AM -0800, Jeremiah Mahler wrote:
> Jarkko,
> 
> On Thu, Nov 05, 2015 at 01:05:45PM +0200, Jarkko Sakkinen wrote:
> > On Thu, Nov 05, 2015 at 11:22:55AM +0200, Jarkko Sakkinen wrote:
> > > On Wed, Nov 04, 2015 at 10:17:05AM -0800, Jeremiah Mahler wrote:
> > > > Jarkko, all,
> > > > 
> [...]
> > > > 
> > > > The commit for this patch (9b774d5cf2db4) present in the latest
> > > > linux-next (20151101+) breaks suspend/resume on an Acer C720 Chromebook.
> > > > The computer will successfully suspend but when a resume is attempted
> > > > a blank screen is displayed for a few seconds and then it reboots.
> > > 
> > > I think I have this same model at home (have to check). If it is, I can
> > > try to reproduce it today when I get back to home.
> > 
> > Yup, it's C720P!
> > 
> > > What kind of environment are you running?
> > > Are you able to acquire kernel logs?
> > 
> > Both of these questions still apply. Even if I create the same
> > environment, your logs might have something useful embedded.
> > 
> > Thank you for reporting this issue! I'll try to find a way to reproduce
> > and fix it at latest for -rc2.
> > 
> > > > -- 
> > > > - Jeremiah Mahler
> > 
> > /Jarkko
> 
> I have attached the full dmesg.  The following snippets looked
> particularly interesting.

I'll look at the logs with time tomorrow. What kind of distribution you
have in your system? Can you also send your kernel config?

I have the same laptop at hand and would like to setup exactly the same
environment.

/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [tpmdd-devel] [BUG, bisect, PATCH 04/10] tpm: move the PPI attributes to character device directory.

2015-11-05 Thread Jeremiah Mahler
Jarkko,

On Thu, Nov 05, 2015 at 07:46:30PM +0200, Jarkko Sakkinen wrote:
> On Thu, Nov 05, 2015 at 08:47:58AM -0800, Jeremiah Mahler wrote:
> > Jarkko,
> > 
> > On Thu, Nov 05, 2015 at 01:05:45PM +0200, Jarkko Sakkinen wrote:
> > > On Thu, Nov 05, 2015 at 11:22:55AM +0200, Jarkko Sakkinen wrote:
> > > > On Wed, Nov 04, 2015 at 10:17:05AM -0800, Jeremiah Mahler wrote:
> > > > > Jarkko, all,
> > > > > 
> > [...]
> > > > > 
> > > > > The commit for this patch (9b774d5cf2db4) present in the latest
> > > > > linux-next (20151101+) breaks suspend/resume on an Acer C720 
> > > > > Chromebook.
> > > > > The computer will successfully suspend but when a resume is attempted
> > > > > a blank screen is displayed for a few seconds and then it reboots.
> > > > 
> > > > I think I have this same model at home (have to check). If it is, I can
> > > > try to reproduce it today when I get back to home.
> > > 
> > > Yup, it's C720P!
> > > 
> > > > What kind of environment are you running?
> > > > Are you able to acquire kernel logs?
> > > 
> > > Both of these questions still apply. Even if I create the same
> > > environment, your logs might have something useful embedded.
> > > 
> > > Thank you for reporting this issue! I'll try to find a way to reproduce
> > > and fix it at latest for -rc2.
> > > 
> > > > > -- 
> > > > > - Jeremiah Mahler
> > > 
> > > /Jarkko
> > 
> > I have attached the full dmesg.  The following snippets looked
> > particularly interesting.
> 
> I'll look at the logs with time tomorrow. What kind of distribution you
> have in your system? Can you also send your kernel config?
> 
I'm running Debian unstable, not sure of the exact version.
Attached is my current .config.

> I have the same laptop at hand and would like to setup exactly the same
> environment.
> 
> /Jarkko

The only difference is that yours is a C720P and mine is a C720.
Hopefully they are similar enough.

-- 
- Jeremiah Mahler
#
# Automatically generated file; DO NOT EDIT.
# Linux/x86 4.3.0-rc4 Kernel Configuration
#
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_PERF_EVENTS_INTEL_UNCORE=y
CONFIG_OUTPUT_FORMAT="elf64-x86-64"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_HAVE_LATENCYTOP_SUPPORT=y
CONFIG_MMU=y
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_HAVE_INTEL_TXT=y
CONFIG_X86_64_SMP=y
CONFIG_ARCH_HWEIGHT_CFLAGS="-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx 
-fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 
-fcall-saved-r11"
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_PGTABLE_LEVELS=4
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y

#
# General setup
#
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
# CONFIG_COMPILE_TEST is not set
CONFIG_LOCALVERSION=""
# CONFIG_LOCALVERSION_AUTO is not set
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_HAVE_KERNEL_LZ4=y
# CONFIG_KERNEL_GZIP is not set
# CONFIG_KERNEL_BZIP2 is not set
# CONFIG_KERNEL_LZMA is not set
CONFIG_KERNEL_XZ=y
# CONFIG_KERNEL_LZO is not set
# CONFIG_KERNEL_LZ4 is not set
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_CROSS_MEMORY_ATTACH=y
CONFIG_FHANDLE=y
CONFIG_USELIB=y
CONFIG_AUDIT=y
CONFIG_HAVE_ARCH_AUDITSYSCALL=y
CONFIG_AUDITSYSCALL=y
CONFIG_AUDIT_WATCH=y
CONFIG_AUDIT_TREE=y

#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_DOMAIN_HIERARCHY=y
CONFIG_GENERIC_MSI_IRQ=y
CONFIG_GENERIC_MSI_IRQ_DOMAIN=y
# CONFIG_IRQ_DOMAIN_DEBUG is not set
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y

#
# Timers subsystem
#
CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ_COMMON=y
# CONFIG_HZ_PERIODIC is not set
CONFIG_NO_HZ_IDLE=y
# CONFIG_NO_HZ_FULL is not set
# CONFIG_NO_HZ is not set
CONFIG_HIGH_RES_TIMERS=y

#
# 

Re: [BUG, bisect, PATCH 04/10] tpm: move the PPI attributes to character device directory.

2015-11-04 Thread Jeremiah Mahler
Jarkko, all,

On Fri, Oct 16, 2015 at 09:40:23PM +0300, Jarkko Sakkinen wrote:
> Moved PPI attributes to the character device directory. This aligns with
> the sysfs guidelines and makes them race free because they are created
> atomically with the character device as part of device_register().The
> character device and the sysfs attributes appear at the same time to the
> user space.
> 
> As part of this change we enable PPI attributes also for TPM 2.0
> devices. In order to retain backwards compatibility with TPM 1.x
> devices, a symlink is created to the platform device directory.
> 
> Signed-off-by: Jarkko Sakkinen 
> Reviewed-by: Jason Gunthorpe 
> Tested-by: Mimi Zohar  (on TPM 1.2)
> Tested-by: Chris J Arges 
> Tested-by: Colin Ian King 
> ---
>  drivers/char/tpm/tpm-chip.c | 24 
>  drivers/char/tpm/tpm.h  | 17 ++---
>  drivers/char/tpm/tpm_ppi.c  | 34 +++---
>  3 files changed, 33 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 1082d4b..f26b0ae 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -119,6 +119,9 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
>   chip->dev.class = tpm_class;
>   chip->dev.release = tpm_dev_release;
>   chip->dev.parent = chip->pdev;
> +#ifdef CONFIG_ACPI
> + chip->dev.groups = chip->groups;
> +#endif
>  
>   if (chip->dev_num == 0)
>   chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
> @@ -182,12 +185,6 @@ static int tpm1_chip_register(struct tpm_chip *chip)
>   if (rc)
>   return rc;
>  
> - rc = tpm_add_ppi(chip);
> - if (rc) {
> - tpm_sysfs_del_device(chip);
> - return rc;
> - }
> -
>   chip->bios_dir = tpm_bios_log_setup(chip->devname);
>  
>   return 0;
> @@ -201,8 +198,6 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
>   if (chip->bios_dir)
>   tpm_bios_log_teardown(chip->bios_dir);
>  
> - tpm_remove_ppi(chip);
> -
>   tpm_sysfs_del_device(chip);
>  }
>  
> @@ -225,10 +220,20 @@ int tpm_chip_register(struct tpm_chip *chip)
>   if (rc)
>   return rc;
>  
> + tpm_add_ppi(chip);
> +
>   rc = tpm_dev_add_device(chip);
>   if (rc)
>   goto out_err;
>  
> + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> + rc = __compat_only_sysfs_link_entry_to_kobj(>pdev->kobj,
> + >dev.kobj,
> + "ppi");
> + if (rc)
> + goto out_err;
> + }
> +
>   /* Make the chip available. */
>   spin_lock(_lock);
>   list_add_rcu(>list, _chip_list);
> @@ -263,6 +268,9 @@ void tpm_chip_unregister(struct tpm_chip *chip)
>   spin_unlock(_lock);
>   synchronize_rcu();
>  
> + if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> + sysfs_remove_link(>pdev->kobj, "ppi");
> +
>   tpm1_chip_unregister(chip);
>   tpm_dev_del_device(chip);
>  }
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 39be5ac..36ceb71 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -158,8 +158,7 @@ struct tpm_vendor_specific {
>  
>  enum tpm_chip_flags {
>   TPM_CHIP_FLAG_REGISTERED= BIT(0),
> - TPM_CHIP_FLAG_PPI   = BIT(1),
> - TPM_CHIP_FLAG_TPM2  = BIT(2),
> + TPM_CHIP_FLAG_TPM2  = BIT(1),
>  };
>  
>  struct tpm_chip {
> @@ -182,6 +181,8 @@ struct tpm_chip {
>   struct dentry **bios_dir;
>  
>  #ifdef CONFIG_ACPI
> + const struct attribute_group *groups[2];
> + unsigned int groups_cnt;
>   acpi_handle acpi_dev_handle;
>   char ppi_version[TPM_PPI_VERSION_LEN + 1];
>  #endif /* CONFIG_ACPI */
> @@ -189,7 +190,7 @@ struct tpm_chip {
>   struct list_head list;
>  };
>  
> -#define to_tpm_chip(n) container_of(n, struct tpm_chip, vendor)
> +#define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
>  
>  static inline void tpm_chip_put(struct tpm_chip *chip)
>  {
> @@ -419,15 +420,9 @@ void tpm_sysfs_del_device(struct tpm_chip *chip);
>  int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
>  
>  #ifdef CONFIG_ACPI
> -extern int tpm_add_ppi(struct tpm_chip *chip);
> -extern void tpm_remove_ppi(struct tpm_chip *chip);
> +extern void tpm_add_ppi(struct tpm_chip *chip);
>  #else
> -static inline int tpm_add_ppi(struct tpm_chip *chip)
> -{
> - return 0;
> -}
> -
> -static inline void tpm_remove_ppi(struct tpm_chip *chip)
> +static inline void tpm_add_ppi(struct tpm_chip *chip)
>  {
>  }
>  #endif
> diff --git a/drivers/char/tpm/tpm_ppi.c b/drivers/char/tpm/tpm_ppi.c
> index 6ca9b5d..692a2c6 100644
> --- a/drivers/char/tpm/tpm_ppi.c
> +++ b/drivers/char/tpm/tpm_ppi.c
> @@ -53,7 +53,7 @@ tpm_eval_dsm(acpi_handle ppi_handle, int func, 
> acpi_object_type type,

Re: [BUG, bisect, PATCH 04/10] tpm: move the PPI attributes to character device directory.

2015-11-04 Thread Jeremiah Mahler
Jarkko, all,

On Fri, Oct 16, 2015 at 09:40:23PM +0300, Jarkko Sakkinen wrote:
> Moved PPI attributes to the character device directory. This aligns with
> the sysfs guidelines and makes them race free because they are created
> atomically with the character device as part of device_register().The
> character device and the sysfs attributes appear at the same time to the
> user space.
> 
> As part of this change we enable PPI attributes also for TPM 2.0
> devices. In order to retain backwards compatibility with TPM 1.x
> devices, a symlink is created to the platform device directory.
> 
> Signed-off-by: Jarkko Sakkinen 
> Reviewed-by: Jason Gunthorpe 
> Tested-by: Mimi Zohar  (on TPM 1.2)
> Tested-by: Chris J Arges 
> Tested-by: Colin Ian King 
> ---
>  drivers/char/tpm/tpm-chip.c | 24 
>  drivers/char/tpm/tpm.h  | 17 ++---
>  drivers/char/tpm/tpm_ppi.c  | 34 +++---
>  3 files changed, 33 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 1082d4b..f26b0ae 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -119,6 +119,9 @@ struct tpm_chip *tpmm_chip_alloc(struct device *dev,
>   chip->dev.class = tpm_class;
>   chip->dev.release = tpm_dev_release;
>   chip->dev.parent = chip->pdev;
> +#ifdef CONFIG_ACPI
> + chip->dev.groups = chip->groups;
> +#endif
>  
>   if (chip->dev_num == 0)
>   chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
> @@ -182,12 +185,6 @@ static int tpm1_chip_register(struct tpm_chip *chip)
>   if (rc)
>   return rc;
>  
> - rc = tpm_add_ppi(chip);
> - if (rc) {
> - tpm_sysfs_del_device(chip);
> - return rc;
> - }
> -
>   chip->bios_dir = tpm_bios_log_setup(chip->devname);
>  
>   return 0;
> @@ -201,8 +198,6 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
>   if (chip->bios_dir)
>   tpm_bios_log_teardown(chip->bios_dir);
>  
> - tpm_remove_ppi(chip);
> -
>   tpm_sysfs_del_device(chip);
>  }
>  
> @@ -225,10 +220,20 @@ int tpm_chip_register(struct tpm_chip *chip)
>   if (rc)
>   return rc;
>  
> + tpm_add_ppi(chip);
> +
>   rc = tpm_dev_add_device(chip);
>   if (rc)
>   goto out_err;
>  
> + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> + rc = __compat_only_sysfs_link_entry_to_kobj(>pdev->kobj,
> + >dev.kobj,
> + "ppi");
> + if (rc)
> + goto out_err;
> + }
> +
>   /* Make the chip available. */
>   spin_lock(_lock);
>   list_add_rcu(>list, _chip_list);
> @@ -263,6 +268,9 @@ void tpm_chip_unregister(struct tpm_chip *chip)
>   spin_unlock(_lock);
>   synchronize_rcu();
>  
> + if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> + sysfs_remove_link(>pdev->kobj, "ppi");
> +
>   tpm1_chip_unregister(chip);
>   tpm_dev_del_device(chip);
>  }
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 39be5ac..36ceb71 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -158,8 +158,7 @@ struct tpm_vendor_specific {
>  
>  enum tpm_chip_flags {
>   TPM_CHIP_FLAG_REGISTERED= BIT(0),
> - TPM_CHIP_FLAG_PPI   = BIT(1),
> - TPM_CHIP_FLAG_TPM2  = BIT(2),
> + TPM_CHIP_FLAG_TPM2  = BIT(1),
>  };
>  
>  struct tpm_chip {
> @@ -182,6 +181,8 @@ struct tpm_chip {
>   struct dentry **bios_dir;
>  
>  #ifdef CONFIG_ACPI
> + const struct attribute_group *groups[2];
> + unsigned int groups_cnt;
>   acpi_handle acpi_dev_handle;
>   char ppi_version[TPM_PPI_VERSION_LEN + 1];
>  #endif /* CONFIG_ACPI */
> @@ -189,7 +190,7 @@ struct tpm_chip {
>   struct list_head list;
>  };
>  
> -#define to_tpm_chip(n) container_of(n, struct tpm_chip, vendor)
> +#define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
>  
>  static inline void tpm_chip_put(struct tpm_chip *chip)
>  {
> @@ -419,15 +420,9 @@ void tpm_sysfs_del_device(struct tpm_chip *chip);
>  int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
>  
>  #ifdef CONFIG_ACPI
> -extern int tpm_add_ppi(struct tpm_chip *chip);
> -extern void tpm_remove_ppi(struct tpm_chip *chip);
> +extern void tpm_add_ppi(struct tpm_chip *chip);
>  #else
> -static inline int tpm_add_ppi(struct tpm_chip *chip)
> -{
> - return 0;
> -}
> -
> -static inline void tpm_remove_ppi(struct tpm_chip *chip)
> +static inline void tpm_add_ppi(struct tpm_chip *chip)
>  {
>  }
>  #endif
> diff --git a/drivers/char/tpm/tpm_ppi.c b/drivers/char/tpm/tpm_ppi.c
> index 6ca9b5d..692a2c6 100644
> ---