Re: [PATCH v8 2/7] PCI: Introduce shpchp_is_native()

[Bjorn Helgaas <helgaas@kernel.org>]

On Fri, Jun 01, 2018 at 12:27:10PM +0300, Mika Westerberg wrote:
> On Thu, May 31, 2018 at 11:55:56AM -0500, Bjorn Helgaas wrote:
> > On Thu, May 31, 2018 at 04:51:17PM +0300, Mika Westerberg wrote:
> > > On Thu, May 31, 2018 at 08:12:02AM -0500, Bjorn Helgaas wrote:
> > > > Maybe I'm reading your patches wrong.  It looks to me like shpchp will
> > > > claim GOLAM_7450, which means shpchp will register slots, program the
> > > > SHPC, handle hotplug interrupts, etc.
> > > > 
> > > > But since shpchp_is_native() returns false, acpiphp thinks *it* should
> > > > handle hotplug.  For example, I think that given some ACPI
> > > > prerequisites (_EJ0/_RMV/etc), both will call pci_hp_register():
> > > > 
> > > >   shpc_probe
> > > >     is_shpc_capable             # true for GOLAM_7450
> > > >     init_slots
> > > >       pci_hp_register
> > > > 
> > > >   acpi_pci_add_slots
> > > >     acpiphp_enumerate_slots
> > > >       acpi_walk_namespace(..., acpiphp_add_context)
> > > >         acpiphp_add_context
> > > >           hotplug_is_native     # false for GOLAM_7450
> > > >           acpiphp_register_hotplug_slot
> > > >             pci_hp_register
> > > > 
> > > > It is true that the same situation occurred before your patches, since
> > > > acpiphp_add_context() only checked pciehp_is_native().  In fact, with
> > > > the existing code, shpchp and acpiphp could both try to manage *any*
> > > > SHPC, not just GOLAM_7450.
> > > > 
> > > > I think the current series fixes 99% of that problem and it seems like
> > > > we should try to do that last 1% at the same time so the SHPC code
> > > > makes more sense.
> > > 
> > > Would the following fix the last 1% for you? Applies on top of this
> > > patch.
> > 
> > Yes, that's exactly what I was looking for!  Thanks!
> 
> Great. Do you want me to update this patch accordingly or will you do
> that yourself?

No need, I squashed it in already.