Re: [PATCH v2 1/1] PCI: pciehp: Skip DLLSC handling if DPC is triggered

[Sinan Kaya]

On 3/17/2021 4:02 PM, Kuppuswamy, Sathyanarayanan wrote:
> My point is, there is no race in OS handlers (pciehp_ist() vs
> pcie_do_recovery())
>  However, Sinan wrote in
>> 2018 that one of the issues with hotplug versus DPC is that pciehp
>> may turn off slot power and thereby foil DPC recovery.  (Power off =
>> cold reset, whereas DPC recovery = warm reset.)  This can occur
>> as well if DPC is handled by firmware. 

It has been a while...

If I remember correctly, there is no race condition if the platform
handles DPC and HP interrupts on the same MSI vector.

If HP and DPC interrupts are handled as MSI-x interrupts, these can
fire out of order and can cause problems for each one.

I hope it helps.

Re: Issues with "PCI/LINK: Report degraded links via link bandwidth notification"

[Sinan Kaya]

On 1/28/2021 6:39 PM, Bjorn Helgaas wrote:
> AFAICT, this thread petered out with no resolution.
> 
> If the bandwidth change notifications are important to somebody,
> please speak up, preferably with a patch that makes the notifications
> disabled by default and adds a parameter to enable them (or some other
> strategy that makes sense).
> 
> I think these are potentially useful, so I don't really want to just
> revert them, but if nobody thinks these are important enough to fix,
> that's a possibility.

Hide behind debug or expert option by default? or even mark it as BROKEN
until someone fixes it?

Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call

[Sinan Kaya]

On 10/12/2020 1:13 AM, Kuppuswamy, Sathyanarayanan wrote:
> Hi Sinan,
> 
> On 9/28/20 11:32 AM, Kuppuswamy, Sathyanarayanan wrote:
>>
>>
>> On 9/28/20 11:25 AM, Sinan Kaya wrote:
>>> On 9/28/2020 2:02 PM, Sinan Kaya wrote:
>>>> Since there is no state restoration for FATAL errors, I am wondering
>>>> whether
>>>> calls to ->error_detected(), ->mmio_enabled() and ->slot_reset() are
>>>> required?
>>>
>>> I also would like to ask someone closer to the spec language double
>>> check this.
>>>
>>> When we recover the link at the end of the DPC handler, what is the
>>> expected state of the endpoint?
>>>
>>> Is it a some kind of a reset like secondary bus reset? (I assumed this
>>>   one)
>> I think it will be in reset state.
>>>
>>> Undefined?
>>>
>>> or just plain link recovery with everything else as intact as it used
>>> to be?
>>>
>>
> 
> Please check the following version. It should fix most of the reset issues
> properly.
> 
> https://lore.kernel.org/linux-pci/5c5bca0bdb958e456176fe6ede10ba8f838fbafc.1602263264.git.sathyanarayanan.kuppusw...@linux.intel.com/T/#t
> 
> 

Thanks, good stuff.

Re: [PATCH v4 1/2] PCI/ERR: Call pci_bus_reset() before calling ->slot_reset() callback

[Sinan Kaya]

On 10/12/2020 1:03 AM, sathyanarayanan.nkuppusw...@gmail.com wrote:
> From: Kuppuswamy Sathyanarayanan 
> 
> Currently if report_error_detected() or report_mmio_enabled()
> functions requests PCI_ERS_RESULT_NEED_RESET, current
> pcie_do_recovery() implementation does not do the requested
> explicit device reset, but instead just calls the
> report_slot_reset() on all affected devices. Notifying about the
> reset via report_slot_reset() without doing the actual device
> reset is incorrect. So call pci_bus_reset() before triggering
> ->slot_reset() callback.
> 
> Signed-off-by: Kuppuswamy Sathyanarayanan 
> 
> ---
>  drivers/pci/pcie/err.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
> index c543f419d8f9..067c58728b88 100644
> --- a/drivers/pci/pcie/err.c
> +++ b/drivers/pci/pcie/err.c
> @@ -181,11 +181,7 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
>   }
>  
>   if (status == PCI_ERS_RESULT_NEED_RESET) {
> - /*
> -  * TODO: Should call platform-specific
> -  * functions to reset slot before calling
> -  * drivers' slot_reset callbacks?
> -  */
> + pci_reset_bus(dev);
>   status = PCI_ERS_RESULT_RECOVERED;
>   pci_dbg(dev, "broadcast slot_reset message\n");
>   pci_walk_bus(bus, report_slot_reset, );
> 

Reviewed-by: Sinan Kaya 

Re: [PATCH v4 2/2] PCI/ERR: Split the fatal and non-fatal error recovery handling

[Sinan Kaya]

On 10/12/2020 1:03 AM, sathyanarayanan.nkuppusw...@gmail.com wrote:
> From: Kuppuswamy Sathyanarayanan 
> 
> Commit bdb5ac85777d ("PCI/ERR: Handle fatal error recovery")
> merged fatal and non-fatal error recovery paths, and also made
> recovery code depend on hotplug handler for "remove affected
> device + rescan" support. But this change also complicated the
> error recovery path and which in turn led to the following
> issues.
> 
> 1. We depend on hotplug handler for removing the affected
> devices/drivers on DLLSC LINK down event (on DPC event
> trigger) and DPC handler for handling the error recovery. Since
> both handlers operate on same set of affected devices, it leads
> to race condition, which in turn leads to  NULL pointer
> exceptions or error recovery failures.You can find more details
> about this issue in following link.
> 
> https://lore.kernel.org/linux-pci/20201007113158.48933-1-haifeng.z...@intel.com/T/#t
> 
> 2. For non-hotplug capable devices fatal (DPC) error recovery
> is currently broken. Current fatal error recovery implementation
> relies on PCIe hotplug (pciehp) handler for detaching and
> re-enumerating the affected devices/drivers. So when dealing with
> non-hotplug capable devices, recovery code does not restore the state
> of the affected devices correctly. You can find more details about
> this issue in the following links.
> 
> https://lore.kernel.org/linux-pci/20200527083130.4137-1-zhiqiang@nxp.com/
> https://lore.kernel.org/linux-pci/12115.1588207324@famine/
> https://lore.kernel.org/linux-pci/0e6f89cd6b9e4a72293cc90fafe93487d7c2d295.158584.git.sathyanarayanan.kuppusw...@linux.intel.com/
> 
> In order to fix the above two issues, we should stop relying on hotplug
> handler for cleaning the affected devices/drivers and let error recovery
> handler own this functionality. So this patch reverts Commit bdb5ac85777d
> ("PCI/ERR: Handle fatal error recovery") and re-introduce the  "remove
> affected device + rescan"  functionality in fatal error recovery handler.
> 
> Also holding pci_lock_rescan_remove() will prevent the race between hotplug
> and DPC handler.
> 
> Fixes: bdb5ac85777d ("PCI/ERR: Handle fatal error recovery")
> Signed-off-by: Kuppuswamy Sathyanarayanan 
> 
> ---
>  Documentation/PCI/pci-error-recovery.rst | 47 ++--
>  drivers/pci/pcie/err.c   | 71 +++-
>  2 files changed, 87 insertions(+), 31 deletions(-)

I'm not sure about locks involved but I do like the concept.
Current fatal error handling is best effort.

There is no way to recover if link is down by the time we
reach to fatal error handling routine.

This change will make the solution more reliable.

Reviewed-by: Sinan Kaya 

Re: [RESEND PATCH v3 1/1] PCI/ERR: don't clobber status after reset_link()

[Sinan Kaya]

On 10/10/2020 6:16 PM, Hedi Berriche wrote:
> Commit 6d2c89441571 ("PCI/ERR: Update error status after reset_link()")
> broke pcie_do_recovery(): updating status after reset_link() has the ill
> side effect of causing recovery to fail if the error status is
> PCI_ERS_RESULT_CAN_RECOVER or PCI_ERS_RESULT_NEED_RESET as the following
> code will *never* run in the case of a successful reset_link()
> 
>177 if (status == PCI_ERS_RESULT_CAN_RECOVER) {
>...
>181 }
> 
>183 if (status == PCI_ERS_RESULT_NEED_RESET) {
>...
>192 }
> 
> For instance in the case of PCI_ERS_RESULT_NEED_RESET we end up not
> calling ->slot_reset() (because we skip report_slot_reset()) thus
> breaking driver (re)initialisation.
> 
> Don't clobber status with the return value of reset_link(); set status
> to PCI_ERS_RESULT_RECOVERED, in case of successful link reset, if and
> only if the initial value of error status is PCI_ERS_RESULT_DISCONNECT
> or PCI_ERS_RESULT_NO_AER_DRIVER.
> 
> Fixes: 6d2c89441571 ("PCI/ERR: Update error status after reset_link()")
> Signed-off-by: Hedi Berriche 
> Cc: Russ Anderson 
> Cc: Kuppuswamy Sathyanarayanan 
> Cc: Bjorn Helgaas 
> Cc: Ashok Raj 
> Cc: Joerg Roedel 
> 
> Cc: sta...@kernel.org # v5.7+
> ---
>  drivers/pci/pcie/err.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
> index c543f419d8f9..2730826cfd8a 100644
> --- a/drivers/pci/pcie/err.c
> +++ b/drivers/pci/pcie/err.c
> @@ -165,10 +165,13 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
>   pci_dbg(dev, "broadcast error_detected message\n");
>   if (state == pci_channel_io_frozen) {
>   pci_walk_bus(bus, report_frozen_detected, );
> - status = reset_link(dev);
> - if (status != PCI_ERS_RESULT_RECOVERED) {
> + if (reset_link(dev) != PCI_ERS_RESULT_RECOVERED) {
>   pci_warn(dev, "link reset failed\n");
>   goto failed;
> + } else {
> + if (status == PCI_ERS_RESULT_DISCONNECT ||
> + status == PCI_ERS_RESULT_NO_AER_DRIVER)
> + status = PCI_ERS_RESULT_RECOVERED;
>   }
>   } else {
>   pci_walk_bus(bus, report_normal_detected, );
> 

Reviewed-by: Sinan Kaya 

Re: [PATCH v6 4/5] PCI: only return true when dev io state is really changed

[Sinan Kaya]

On 9/30/2020 3:05 AM, Ethan Zhao wrote:
> When uncorrectable error happens, AER driver and DPC driver interrupt
> handlers likely call
> 
>pcie_do_recovery()
>->pci_walk_bus()
>  ->report_frozen_detected()
> 
> with pci_channel_io_frozen the same time.

We need some more data on this. If DPC is supported by HW, errors
should be triggered by DPC not AER.

If I remember right, there is a register that tells which AER errors
should be handled by DPC.

Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call

[Sinan Kaya]

On 9/28/2020 2:02 PM, Sinan Kaya wrote:
> Since there is no state restoration for FATAL errors, I am wondering
> whether
> calls to ->error_detected(), ->mmio_enabled() and ->slot_reset() are
> required?

I also would like to ask someone closer to the spec language double
check this.

When we recover the link at the end of the DPC handler, what is the
expected state of the endpoint?

Is it a some kind of a reset like secondary bus reset? (I assumed this
 one)

Undefined?

or just plain link recovery with everything else as intact as it used
to be?

Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call

[Sinan Kaya]

On 9/28/2020 1:15 PM, Kuppuswamy, Sathyanarayanan wrote:
> Since there is no state restoration for FATAL errors, I am wondering
> whether
> calls to ->error_detected(), ->mmio_enabled() and ->slot_reset() are
> required?

Good question,

Initially when we started, we were trying to handle both NON_FATAL and
FATAL errors in DPC.

We have seen value in unifying AER's callback mechanism with DPC.
It looks like this no longer applies for DPC.

Some drivers want these indication to stop outgoing DMA/timers so that
system can recover quickly.

There is value in calling them with existing AER based design.

I agree it doesn't apply here anymore if we are going to remove the
device driver. Maybe, you should stop calling pcie_do_recovery() in DPC
as well.

> 
> Let me know your comments about following pseudo code.
> 
> if (fatal error & hotplug_supported)
>    do nothing // if fatal triggered by DPC, clear DPC state.
> 
> if (fatal error & no-hotplug)
>   perform slot_reset and renumerate affected devices.

LGTM,

I apologize for calling this slot_reset but slot_reset in err.c code is
for post recovery callback to endpoint drivers. Let's not use this term
here anymore to not confuse ourselves.

remove device + rescan similar to how hotplug remove + hotplug insertion
notifications does eventually.

All of this to be done in DPC driver without any err.c involvement.

Bjorn,

What do you think? Is this a good direction?

Sinan

Re: [PATCH 2/5 V2] PCI: pciehp: check and wait port status out of DPC before handling DLLSC and PDC

[Sinan Kaya]

On 9/28/2020 7:10 AM, Sinan Kaya wrote:
> On 9/27/2020 10:01 PM, Zhao, Haifeng wrote:
>> Sinan,
>>I explained the reason why locks don't protect this case in the patch 
>> description part. 
>> Write side and read side hold different semaphore and mutex.
>>
> I have been thinking about it some time but is there any reason why we
> have to handle all port AER/DPC/HP events in different threads?
> 
> Can we go to single threaded event loop for all port drivers events?
> 
> This will require some refactoring but it wlll eliminate the lock
> nightmares we are having.
> 
> This means no sleeping. All sleeps need to happen outside of the loop.
> 
> I wanted to see what you all are thinking about this.
> 
> It might become a performance problem if the system is
> continuously observing a hotplug/aer/dpc events.
> 
> I always think that these should be rare events.

If restructuring would be too costly, the preferred solution should be
to fix the locks in hotplug driver rather than throwing there a random
wait call.

Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call

[Sinan Kaya]

On 9/28/2020 7:17 AM, Sinan Kaya wrote:
> This should remove/rescan logic should be inside DPC's slot_reset()
> function BTW. Not here.

Correct function name is dpc_handler().

I hope I did not create confusion with slot_reset() that gets called for
each driver post recovery.

Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call

[Sinan Kaya]

On 9/27/2020 10:43 PM, Kuppuswamy, Sathyanarayanan wrote:
>> 2. no bus reset on NON_FATAL error through AER driver path.
>> This already tells me that you need to split your change into
>> multiple patches.
>>
>> Let's talk about this too. bus reset should be triggered via
>> AER driver before informing the recovery.
> But as per error recovery documentation, any call to
> ->error_detected() or ->mmio_enabled() can request
> PCI_ERS_RESULT_NEED_RESET. So we need to add code
> to do the actual reset before calling ->slot_reset()
> callback. So call to pci_reset_bus() fixes this
> issue.
> 
>  if (status == PCI_ERS_RESULT_NEED_RESET) {
> +    pci_reset_bus(dev);

This part seems to make sense as you already highlighted there is
a TO-DO in the code.

This is an independent change that deserves its own patch.

Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call

[Sinan Kaya]

On 9/27/2020 10:43 PM, Kuppuswamy, Sathyanarayanan wrote:
> FATAL + no-hotplug - In this case, link will still be reseted. But
> currently driver state is not properly restored. So I attempted
> to restore it using pci_reset_bus().
> 
>  status = reset_link(dev);
> -    if (status != PCI_ERS_RESULT_RECOVERED) {
> +    if (status == PCI_ERS_RESULT_RECOVERED) {
> +    status = PCI_ERS_RESULT_NEED_RESET;
> 
> ...
> 
>  if (status == PCI_ERS_RESULT_NEED_RESET) {
>  /*
> - * TODO: Should call platform-specific
> - * functions to reset slot before calling
> - * drivers' slot_reset callbacks?
> + * TODO: Optimize the call to pci_reset_bus()
> + *
> + * There are two components to pci_reset_bus().
> + *
> + * 1. Do platform specific slot/bus reset.
> + * 2. Save/Restore all devices in the bus.
> + *
> + * For hotplug capable devices and fatal errors,
> + * device is already in reset state due to link
> + * reset. So repeating platform specific slot/bus
> + * reset via pci_reset_bus() call is redundant. So
> + * can optimize this logic and conditionally call
> + * pci_reset_bus().
>   */
> +    pci_reset_bus(dev);

I think we have to go to remove/rescan for this case as you also
mentioned above. There is no state to save. All BAR assignments
are gone. Entire device programming is also lost.

I don't think pci_reset_bus() can recover from this situation safely.
It will make things worse by saving/restoring the hardware default
state.

This should remove/rescan logic should be inside DPC's slot_reset()
function BTW. Not here.

Re: [PATCH 2/5 V2] PCI: pciehp: check and wait port status out of DPC before handling DLLSC and PDC

[Sinan Kaya]

On 9/27/2020 10:01 PM, Zhao, Haifeng wrote:
> Sinan,
>I explained the reason why locks don't protect this case in the patch 
> description part. 
> Write side and read side hold different semaphore and mutex.
> 

I have been thinking about it some time but is there any reason why we
have to handle all port AER/DPC/HP events in different threads?

Can we go to single threaded event loop for all port drivers events?

This will require some refactoring but it wlll eliminate the lock
nightmares we are having.

This means no sleeping. All sleeps need to happen outside of the loop.

I wanted to see what you all are thinking about this.

It might become a performance problem if the system is
continuously observing a hotplug/aer/dpc events.

I always think that these should be rare events.

Re: [PATCH 2/5 V2] PCI: pciehp: check and wait port status out of DPC before handling DLLSC and PDC

[Sinan Kaya]

On 9/26/2020 11:28 PM, Ethan Zhao wrote:
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c 
> b/drivers/pci/hotplug/pciehp_hpc.c
> index 53433b37e181..6f271160f18d 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -710,8 +710,10 @@ static irqreturn_t pciehp_ist(int irq, void *dev_id)
>   down_read(>reset_lock);
>   if (events & DISABLE_SLOT)
>   pciehp_handle_disable_request(ctrl);
> - else if (events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC))
> + else if (events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC)) {
> + pci_wait_port_outdpc(pdev);
>   pciehp_handle_presence_or_link_change(ctrl, events);
> + }
>   up_read(>reset_lock);

This looks like a hack TBH.

Lukas, Keith;

What is your take on this?
Why is device lock not protecting this situation?

Is there a lock missing in hotplug driver?

Sinan

Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call

[Sinan Kaya]

On 9/25/2020 2:16 PM, Kuppuswamy, Sathyanarayanan wrote:
>> One approach is to share the restore code between hotplug driver and
>> DPC driver.
>>
>> If this is a too involved change, DPC driver should restore state
>> when hotplug is not supported.
> Yes. we can add a condition for hotplug capability check.

Now that I think about this more...

This won't work. Link is brought down automatically by the DPC hardware.
Therefore, all software state is lost. Restore won't help here.

The only solution I can see is to force driver disconnect and rescan.

Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call

[Sinan Kaya]

On 9/25/2020 2:16 PM, Kuppuswamy, Sathyanarayanan wrote:
>>
>> If this is a too involved change, DPC driver should restore state
>> when hotplug is not supported.
> Yes. we can add a condition for hotplug capability check.
>>
>> DPC driver should be self-sufficient by itself.
>>

Sounds good.

>>> Also for non-fatal errors, if reset is requested then we still need
>>> some kind of bus reset call here
>>
>> DPC should handle both fatal and non-fatal cases
> Currently DPC is only triggered for FATAL errors.
>  and cause a bus reset

Thanks for the heads up.
This seems to have changed since I looked at the DPC code.

>> in hardware already before triggering an interrupt.
> Error recovery is not triggered only DPC driver. AER also uses the
> same error recovery code. If DPC is not supported, then we still need
> reset logic.

It sounds like we are cross-talking two issues.

1. no state restore on DPC after FATAL error.
Let's fix this.

2. no bus reset on NON_FATAL error through AER driver path.
This already tells me that you need to split your change into
multiple patches.

Let's talk about this too. bus reset should be triggered via
AER driver before informing the recovery.

if (status == PCI_ERS_RESULT_NEED_RESET) {
/*
 * TODO: Should call platform-specific
 * functions to reset slot before calling
 * drivers' slot_reset callbacks?
 */
status = PCI_ERS_RESULT_RECOVERED;
pci_dbg(dev, "broadcast slot_reset message\n");
pci_walk_bus(bus, report_slot_reset, );
}

Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call

[Sinan Kaya]

On 9/25/2020 1:11 PM, Kuppuswamy, Sathyanarayanan wrote:
>> Why? Isn't DPC slot reset enough?
> It will do the reset at hardware level. But driver state is not
> cleaned up. So doing bus reset will restore both driver and
> hardware states.

I really don't like this. If hotplug driver is restoring the state
and DPC driver is not; let's fix the DPC driver rather than causing
two resets and hope for the best.

One approach is to share the restore code between hotplug driver and
DPC driver.

If this is a too involved change, DPC driver should restore state
when hotplug is not supported.

DPC driver should be self-sufficient by itself.

> Also for non-fatal errors, if reset is requested then we still need
> some kind of bus reset call here.

DPC should handle both fatal and non-fatal cases and cause a bus reset
in hardware already before triggering an interrupt.

I disagree that you need an additional reset on top of DPC reset.
Isn't one reset enough?

What will the second reset provide that first reset won't provide?

I see that you are trying to do the second reset only because second
reset restores state.

That looks like a short-term fix only to explode on the next iteration.

Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call

[Sinan Kaya]

On 9/25/2020 1:11 AM, Kuppuswamy, Sathyanarayanan wrote:
> 
> 
> On 9/24/20 1:52 PM, Sinan Kaya wrote:
>> On 9/24/2020 12:06 AM, Kuppuswamy, Sathyanarayanan wrote:

>>
>> So, this is a matter of moving the save/restore logic from the hotplug
>> driver into common code so that DPC slot reset takes advantage of it?
> We are not moving it out of hotplug path. But fixing it in this code path.
> With this fix, we will not depend on hotplug driver to restore the state.

Any possibility of unification?


[snip]
>>
>>> To fix above issues, use PCI_ERS_RESULT_NEED_RESET as error state after
>>> successful reset_link() operation. This will ensure ->slot_reset() be
>>> called after reset_link() operation for fatal errors.
>>
>> You lost me here. Why do we want to do secondary bus reset on top of
>> DPC reset?
> For non-hotplug capable slots, when reset (PCI_ERS_RESULT_NEED_RESET) is
> requested, we want to reset it before calling ->slot_reset() callback.

Why? Isn't DPC slot reset enough?
What will bus reset do that DPC slot reset won't do?

I can understand calling bus reset if DPC is not supported.
I don't understand the requirement to do double reset.

Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call

[Sinan Kaya]

On 9/24/2020 12:06 AM, Kuppuswamy, Sathyanarayanan wrote:
> For problem description, please check the following details
> 
> Current pcie_do_recovery() implementation has following two issues:
> 
> 1. Fatal (DPC) error recovery is currently broken for non-hotplug
> capable devices. Current fatal error recovery implementation relies
> on PCIe hotplug (pciehp) handler for detaching and re-enumerating
> the affected devices/drivers. pciehp handler listens for DLLSC state
> changes and handles device/driver detachment on DLLSC_LINK_DOWN event
> and re-enumeration on DLLSC_LINK_UP event. So when dealing with
> non-hotplug capable devices, recovery code does not restore the state
> of the affected devices correctly. Correct implementation should
> restore the device state and call report_slot_reset() function after
> resetting the link to restore the state of the device/driver.
> 

So, this is a matter of moving the save/restore logic from the hotplug
driver into common code so that DPC slot reset takes advantage of it?

If that's direction we are going, this is fine change IMO.

> You can find fatal non-hotplug related issues reported in following links:
> 
> https://lore.kernel.org/linux-pci/20200527083130.4137-1-zhiqiang@nxp.com/
> 
> https://lore.kernel.org/linux-pci/12115.1588207324@famine/
> https://lore.kernel.org/linux-pci/0e6f89cd6b9e4a72293cc90fafe93487d7c2d295.158584.git.sathyanarayanan.kuppusw...@linux.intel.com/
> 
> 
> 2. For non-fatal errors if report_error_detected() or
> report_mmio_enabled() functions requests PCI_ERS_RESULT_NEED_RESET then
> current pcie_do_recovery() implementation does not do the requested
> explicit device reset, instead just calls the report_slot_reset() on all
> affected devices. Notifying about the reset via report_slot_reset()
> without doing the actual device reset is incorrect.
> 

This makes sens too. There seems to be an ordering issue per your
description.

> To fix above issues, use PCI_ERS_RESULT_NEED_RESET as error state after
> successful reset_link() operation. This will ensure ->slot_reset() be
> called after reset_link() operation for fatal errors. 

You lost me here. Why do we want to do secondary bus reset on top of
DPC reset?

> Also call
> pci_bus_reset() to do slot/bus reset() before triggering device specific
> ->slot_reset() callback. Also, using pci_bus_reset() will restore the state
> of the devices after performing the reset operation.
> 
> Even though using pci_bus_reset() will do redundant reset operation after
> ->reset_link() for fatal errors, it should should affect the functional
> behavior.

Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call

[Sinan Kaya]

On 9/23/2020 10:51 PM, Kuppuswamy, Sathyanarayanan wrote:
>>
>> I see. Can I assume that your system supports DPC?
>> DPC is supposed to recover the link via dpc_reset_link().
> Yes. But the affected device/drivers cleanup during error recovery
> is handled by hotplug handler. So we are facing issue when dealing
> with non hotplug capable ports.

This is confusing.

Why would hotplug driver be involved unless port supports hotplug and
the link goes down? You said that DLLSC is only supported on hotplug
capable ports.

Need a better description of symptoms and what triggers hotplug driver
to activate.

Can you expand this a little bit?

Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call

[Sinan Kaya]

On 9/23/2020 10:04 PM, Kuppuswamy, Sathyanarayanan wrote:
>> AFAIK, DLLSC is a requirement not optional. Why is this not supported by
>> non-hotplug ports?
> Its required for hotplug capable ports. Please check PCIe spec v5.0 sec
> 6.7.3.3.
> 
> The Data Link Layer State Changed event provides an indication that the
> state of
> the Data Link Layer Link Active bit in the Link Status Register has
> changed.
> Support for Data Link Layer State Changed events and software
> notification of these
> events are required for hot-plug capable Downstream Ports.

I see. Can I assume that your system supports DPC?
DPC is supposed to recover the link via dpc_reset_link().

Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call

[Sinan Kaya]

On 9/22/2020 7:44 PM, Kuppuswamy, Sathyanarayanan wrote:
>> here does the restore happen here?  I.e., what function does this?
> 
> DLLSC link down event will remove affected devices/drivers. And link up
> event
> will re-create all devices.
> 
> on DLLSC link down event
> ->pciehp_ist()
>   ->pciehp_handle_presence_or_link_change()
>     ->pciehp_disable_slot()
>   ->__pciehp_disable_slot()
>     ->remove_board()
>   ->pciehp_unconfigure_device()
> 
> on DLLSC link up event
> ->pciehp_ist()
>   ->pciehp_handle_presence_or_link_change()
>     ->pciehp_enable_slot()
>   ->__pciehp_enable_slot()
>     ->board_added()
>   ->pciehp_configure_device()

AFAIK, DLLSC is a requirement not optional. Why is this not supported by
non-hotplug ports?

Re: [PATCH] pci: pcie: AER: Fix logging of Correctable errors

[Sinan Kaya]

On 6/18/2020 11:55 AM, Matt Jolly wrote:

> + pci_warn(dev, "  device [%04x:%04x] error 
> status/mask=%08x/%08x\n",
> + dev->vendor, dev->device,
> + info->status, info->mask);
> + } else {



> + pci_err(dev, "  device [%04x:%04x] error 
> status/mask=%08x/%08x\n",
> + dev->vendor, dev->device,
> + info->status, info->mask);


Function pointers for pci_warn vs. pci_err ?

This looks like a lot of copy/paste.

Re: [RFC PATCH] PCI: Remove End-End TLP as PASID dependency

[Sinan Kaya]

On 6/10/2020 4:00 AM, Zhangfei Gao wrote:
>> Why not set the eetlp_prefix_path bit from a PCI quirk?  Unlike the stall
>> problem from the other thread, this one looks like a simple design
>> mistake
>> that can be fixed easily in future iterations of the platform: just set
>> the "End-End TLP Prefix Supported" bit in the Device Capability 2
>> Register
>> of all bridges.
> Yes, we can still set eetlp_prefix_path bit from a PCI quirk.

I agree. Vendor specific quirk should be the way to go here if it is not
compliant with the spec.

Re: [PATCH] dmaengine: qcom_hidma: use true,false for bool variable

[Sinan Kaya]

On 5/4/2020 7:34 AM, Jason Yan wrote:
> Fix the following coccicheck warning:
> 
> drivers/dma/qcom/hidma.c:553:1-17: WARNING: Assignment of 0/1 to bool
> variable
> 
> Signed-off-by: Jason Yan 
> ---
>  drivers/dma/qcom/hidma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c
> index 87490e125bc3..0a6d3ea08c78 100644
> --- a/drivers/dma/qcom/hidma.c
> +++ b/drivers/dma/qcom/hidma.c
> @@ -550,7 +550,7 @@ static void hidma_free_chan_resources(struct dma_chan 
> *dmach)
>   kfree(mdesc);
>   }
>  
> - mchan->allocated = 0;
> + mchan->allocated = false;
>   spin_unlock_irqrestore(>lock, irqflags);
>  }

Acked By: Sinan Kaya 

Re: [PATCH] dmaengine: qcom_hidma: Simplify error handling path in hidma_probe

[Sinan Kaya]

On 4/28/2020 1:21 PM, Dan Carpenter wrote:
> What I meant to say here was:
> 
>   if (msi) {
>   rc = hidma_request_msi(dmadev, pdev);
>   if (rc)
>   msi = false;
> 
> Otherwise we end up checking freeing the msi in the error handling
> code when we did not take it.
> 
> Hopefully, that clears things up?

Yes, that works. However, I'd rather use a different flag for this
in order not to mix the meaning of msi capability vs. msi allocation
failure.

Re: [PATCH] dmaengine: qcom_hidma: Simplify error handling path in hidma_probe

[Sinan Kaya]

On 4/28/2020 8:54 AM, Dan Carpenter wrote:
>> @@ -897,7 +897,6 @@ static int hidma_probe(struct platform_device *pdev)
>>  if (msi)
> ^^^
> This test doesn't work.  It will call free hidma_free_msis() if the
> hidma_request_msi() call fails.  We should do:
> 
>   if (msi) {
>   rc = hidma_request_msi(dmadev, pdev);
>   msi = false;
>   }
> 
>   if (!msi) {
>   hidma_ll_setup_irq(dmadev->lldev, false);
>   rc = devm_request_irq(>dev, chirq, hidma_chirq_handler,
> 0, "qcom-hidma", dmadev->lldev);
>   if (rc)
>   goto uninit;
>   }
> 
> 

Let me clarify how this works. MSI capability is not present on all
platforms. Therefore, this is detected by an ACPI/DTS parameter called
HIDMA_MSI_CAP.

msi = hidma_test_capability(>dev, HIDMA_MSI_CAP);

Therefore,

1. Code will request MSI capability if it is present.
2. Code will fallback to plain IRQ, if MSI allocation also fails.

I hope this helps.

We need both #1 and #2 to be supported.

Re: [PATCH v2 2/2] PCI: pciehp: Prevent deadlock on disconnect

[Sinan Kaya]

On 8/19/2019 4:56 AM, Mika Westerberg wrote:
>> There are PCI controllers that won't report presence detect correctly,
>> but still report link active.
> If that's the case then pciehp_card_present() returns false so we call
> pciehp_check_link_active() which should work with those controllers.
> 
> What I'm missing here?
> 

You are right. I thought we'd somehow prematurely leave the function.
That's not the case.

Re: [PATCH v2 2/2] PCI: pciehp: Prevent deadlock on disconnect

[Sinan Kaya]

On 8/12/2019 10:31 AM, Mika Westerberg wrote:
> +int pciehp_card_present_or_link_active(struct controller *ctrl)
>  {
> - return pciehp_card_present(ctrl) || pciehp_check_link_active(ctrl);
> + int ret;
> +
> + ret = pciehp_card_present(ctrl);
> + if (ret)
> + return ret;
> +
> + return pciehp_check_link_active(ctrl);

The semantics of this function changed here. Before it was checking for
either presence detect bit or link active bit. Now, it is looking to
have both set.

There are PCI controllers that won't report presence detect correctly,
but still report link active.

I think you want

if (ret < 0)
return ret;

here to match the previous behavior while still handling device removal.

Re: [PATCH v3 04/24] dmaengine: qcom_hidma: Remove call to memset after dmam_alloc_coherent

[Sinan Kaya]

On 7/16/2019 7:35 AM, Robin Murphy wrote:
> On 15/07/2019 16:17, Sinan Kaya wrote:
>> On 7/15/2019 1:43 AM, Fuqian Huang wrote:
>>> Should I rewrite the commit log? Just mention that dma_alloc_coherent
>>> has already
>>> zeroed the memory and not to reference the commit?
>>
>> I'd like to hear from Robin Murphy that arm smmu driver follows this as
>> well.
> 
> I'd be lying if I said it did.
> 
> ...but only because that's never been part of the SMMU driver's
> responsibility either way. The iommu-dma layer however, and thus the
> respective arm64 iommu_dma_ops, has always zeroed allocations right from
> its inception.
> 
> 518a2f1925c3 was just cleaning up the last of the stragglers which
> *weren't* already clearing buffers anyway, such that we could formalise
> that behaviour into the API.

Thanks for confirming the behavior for arm64 arch.

Acked-by: Sinan Kaya 

Re: [PATCH] ARM64/irqchip: Make ACPI_IORT depend on PCI again

[Sinan Kaya]

On 7/16/2019 12:04 AM, Sasha Levin wrote:
> ACPI_IORT lost it's explicit dependency on PCI in c6bb8f89fa6df
> ("ARM64/irqchip: Update ACPI_IORT symbol selection logic") where the
> author has relied on the general dependency of ACPI on PCI.
> 
> However, that dependency was finally removed in 5d32a66541c4 ("PCI/ACPI:
> Allow ACPI to be built without CONFIG_PCI set") and now ACPI_IORT breaks
> when we try and build it without PCI support.
> 
> This patch brings back the explicit dependency of ACPI_IORT on PCI.
> 
> Fixes: 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI 
> set")
> Cc: sta...@kernel.org
> Signed-off-by: Sasha Levin 

Do you have more detail on what really is broken without this patch?

It should be possible to build IORT table without PCI.

Re: [PATCH v3 04/24] dmaengine: qcom_hidma: Remove call to memset after dmam_alloc_coherent

[Sinan Kaya]

On 7/15/2019 1:43 AM, Fuqian Huang wrote:
> Should I rewrite the commit log? Just mention that dma_alloc_coherent
> has already
> zeroed the memory and not to reference the commit?

I'd like to hear from Robin Murphy that arm smmu driver follows this as
well.

Re: [PATCH v3 04/24] dmaengine: qcom_hidma: Remove call to memset after dmam_alloc_coherent

[Sinan Kaya]

On 7/14/2019 11:17 PM, Fuqian Huang wrote:
> In commit 518a2f1925c3
> ("dma-mapping: zero memory returned from dma_alloc_*"),
> dma_alloc_coherent has already zeroed the memory.
> So memset is not needed.
> 
> Signed-off-by: Fuqian Huang 

I don't see SWIO or ARM64 IOMMU drivers getting impacted by
the mentioned change above (518a2f1925c3).

 arch/alpha/kernel/pci_iommu.c| 2 +-
 arch/arc/mm/dma.c| 2 +-
 arch/c6x/mm/dma-coherent.c   | 5 -
 arch/m68k/kernel/dma.c   | 2 +-
 arch/microblaze/mm/consistent.c  | 2 +-
 arch/openrisc/kernel/dma.c   | 2 +-
 arch/parisc/kernel/pci-dma.c | 4 ++--
 arch/s390/pci/pci_dma.c  | 2 +-
 arch/sparc/kernel/ioport.c   | 2 +-
 arch/sparc/mm/io-unit.c  | 2 +-
 arch/sparc/mm/iommu.c| 2 +-
 arch/xtensa/kernel/pci-dma.c | 2 +-
 drivers/misc/mic/host/mic_boot.c | 2 +-
 kernel/dma/virt.c| 2 +-
 14 files changed, 18 insertions(+), 15 deletions(-)

How does this new behavior apply globally?

Re: [PATCH 6/6] dma: qcom: hidma: no need to check return value of debugfs_create functions

[Sinan Kaya]

On 6/12/2019 11:39 AM, Greg Kroah-Hartman wrote:
>> Interesting. Wouldn't debugfs_create_file() blow up if dir is NULL
>> for some reason?
> It will create a file in the root of debugfs.  But how will that happen?
> debugfs_create_dir() can not return NULL.

I see.

> 
>> +debugfs_create_file("stats", S_IRUGO, dir, chan,
>> +_chan_fops);
>>
>> Note that code ignores the return value of hidma_debug_init();
>> It was just trying to do clean up on debugfs failure by calling
>>
>>  debugfs_remove_recursive(dmadev->debugfs);
> Is that a problem?

I just wanted to double check. You probably want to remove the return
value on debugfs_create_file() to prevent others from doing the same
thing.

Acked-by: Sinan Kaya 

Re: [PATCH 6/6] dma: qcom: hidma: no need to check return value of debugfs_create functions

[Sinan Kaya]

On 6/12/2019 8:25 AM, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic should
> never do something different based on this.
> 
> Also, because there is no need to save the file dentry, remove the
> variables that were saving them as they were never even being used once
> set.
> 
> Cc: Sinan Kaya 
> Cc: Andy Gross 
> Cc: David Brown 
> Cc: Dan Williams 
> Cc: Vinod Koul 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: dmaeng...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman 

Interesting. Wouldn't debugfs_create_file() blow up if dir is NULL
for some reason?


+   debugfs_create_file("stats", S_IRUGO, dir, chan,
+   _chan_fops);

Note that code ignores the return value of hidma_debug_init();
It was just trying to do clean up on debugfs failure by calling

debugfs_remove_recursive(dmadev->debugfs);

Re: [PATCH] PCI: Add link_change error handler and vfio-pci user

[Sinan Kaya]

On 4/29/2019 10:51 AM, Alex Williamson wrote:

So where do we go from here?  I agree that dmesg is not necessarily a
great choice for these sorts of events and if they went somewhere else,
maybe I wouldn't have the same concerns about them generating user
confusion or contributing to DoS vectors from userspace drivers.  As it
is though, we have known cases where benign events generate confusing
logging messages, which seems like a regression.  Drivers didn't ask
for a link_change handler, but nor did they ask that the link state to
their device be monitored so closely.  Maybe this not only needs some
sort of change to the logging mechanism, but also an opt-in by the
driver if they don't expect runtime link changes.  Thanks,


Is there anyway to detect autonomous hardware management support and
not report link state changes in that situation?

I thought there were some capability bits for these.

Re: [PATCH v5 0/5] init: Do not select DEBUG_KERNEL by default

[Sinan Kaya]

On 4/14/2019 11:55 PM, Kees Cook wrote:

Thanks! This looks good to me. For the series:

Reviewed-by: Kees Cook

Andrew, can you take these from lkml, or should the series get resent
directly to you? I think you might be the best to carry this?


Where do we stand on this?

Anything for me to do, here?

[PATCH v5 3/5] mips: Replace CONFIG_DEBUG_KERNEL with CONFIG_DEBUG_MISC

[Sinan Kaya]

CONFIG_DEBUG_KERNEL should not impact code generation. Use the newly
defined CONFIG_DEBUG_MISC instead to keep the current code.

Signed-off-by: Sinan Kaya 
Reviewed-by: Josh Triplett 
---
 arch/mips/kernel/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 8d1dc6c71173..9fc8fadb8418 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -563,7 +563,7 @@ static void __init bootmem_init(void)
offset = __pa_symbol(_text) - __pa_symbol(VMLINUX_LOAD_ADDRESS);
memblock_free(__pa_symbol(VMLINUX_LOAD_ADDRESS), offset);
 
-#if defined(CONFIG_DEBUG_KERNEL) && defined(CONFIG_DEBUG_INFO)
+#if defined(CONFIG_DEBUG_MISC) && defined(CONFIG_DEBUG_INFO)
/*
 * This information is necessary when debugging the kernel
 * But is a security vulnerability otherwise!
-- 
2.21.0

[PATCH v5 4/5] xtensa: Replace CONFIG_DEBUG_KERNEL with CONFIG_DEBUG_MISC

[Sinan Kaya]

CONFIG_DEBUG_KERNEL should not impact code generation. Use the newly
defined CONFIG_DEBUG_MISC instead to keep the current code.

Signed-off-by: Sinan Kaya 
Reviewed-by: Josh Triplett 
---
 arch/xtensa/include/asm/irqflags.h | 2 +-
 arch/xtensa/kernel/smp.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/xtensa/include/asm/irqflags.h 
b/arch/xtensa/include/asm/irqflags.h
index 9b5e8526afe5..12890681587b 100644
--- a/arch/xtensa/include/asm/irqflags.h
+++ b/arch/xtensa/include/asm/irqflags.h
@@ -27,7 +27,7 @@ static inline unsigned long arch_local_irq_save(void)
 {
unsigned long flags;
 #if XTENSA_FAKE_NMI
-#if defined(CONFIG_DEBUG_KERNEL) && (LOCKLEVEL | TOPLEVEL) >= XCHAL_DEBUGLEVEL
+#if defined(CONFIG_DEBUG_MISC) && (LOCKLEVEL | TOPLEVEL) >= XCHAL_DEBUGLEVEL
unsigned long tmp;
 
asm volatile("rsr   %0, ps\t\n"
diff --git a/arch/xtensa/kernel/smp.c b/arch/xtensa/kernel/smp.c
index 3699d6d3e479..83b244ce61ee 100644
--- a/arch/xtensa/kernel/smp.c
+++ b/arch/xtensa/kernel/smp.c
@@ -126,7 +126,7 @@ void secondary_start_kernel(void)
 
init_mmu();
 
-#ifdef CONFIG_DEBUG_KERNEL
+#ifdef CONFIG_DEBUG_MISC
if (boot_secondary_processors == 0) {
pr_debug("%s: boot_secondary_processors:%d; Hanging cpu:%d\n",
__func__, boot_secondary_processors, cpu);
-- 
2.21.0

[PATCH v5 0/5] init: Do not select DEBUG_KERNEL by default

[Sinan Kaya]

CONFIG_DEBUG_KERNEL has been designed to just enable Kconfig options.
Kernel code generatoin should not depend on CONFIG_DEBUG_KERNEL.

Proposed alternative plan: let's add a new symbol, something like
DEBUG_MISC ("Miscellaneous debug code that should be under a more
specific debug option but isn't"), make it depend on DEBUG_KERNEL and be
"default DEBUG_KERNEL" but allow itself to be turned off, and then
mechanically change the small handful of "#ifdef CONFIG_DEBUG_KERNEL" to
"#ifdef CONFIG_DEBUG_MISC".

Diff from v4:
- collect reviewed-by
- collect acked-by
- fix nit on 1/5

Sinan Kaya (5):
  init: Introduce DEBUG_MISC option
  powerpc: Replace CONFIG_DEBUG_KERNEL with CONFIG_DEBUG_MISC
  mips: Replace CONFIG_DEBUG_KERNEL with CONFIG_DEBUG_MISC
  xtensa: Replace CONFIG_DEBUG_KERNEL with CONFIG_DEBUG_MISC
  net: Replace CONFIG_DEBUG_KERNEL with CONFIG_DEBUG_MISC

 arch/mips/kernel/setup.c   | 2 +-
 arch/powerpc/kernel/sysfs.c| 8 
 arch/xtensa/include/asm/irqflags.h | 2 +-
 arch/xtensa/kernel/smp.c   | 2 +-
 lib/Kconfig.debug  | 9 +
 net/netfilter/core.c   | 2 +-
 6 files changed, 17 insertions(+), 8 deletions(-)

-- 
2.21.0

[PATCH v5 1/5] init: Introduce DEBUG_MISC option

[Sinan Kaya]

Introduce DEBUG_MISC ("Miscellaneous debug code that should be under a more
specific debug option but isn't"), make it depend on DEBUG_KERNEL and be
"default DEBUG_KERNEL" but allow itself to be turned off, and then
mechanically change the small handful of "#ifdef CONFIG_DEBUG_KERNEL" to
"#ifdef CONFIG_DEBUG_MISC".

Signed-off-by: Sinan Kaya 
Reviewed-by: Josh Triplett 
---
 lib/Kconfig.debug | 9 +
 1 file changed, 9 insertions(+)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 0d9e81779e37..0103a092ce3d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -438,6 +438,15 @@ config DEBUG_KERNEL
  Say Y here if you are developing drivers or trying to debug and
  identify kernel problems.
 
+config DEBUG_MISC
+   bool "Miscellaneous debug code"
+   default DEBUG_KERNEL
+   depends on DEBUG_KERNEL
+   help
+ Say Y here if you need to enable miscellaneous debug code that should
+ be under a more specific debug option but isn't.
+
+
 menu "Memory Debugging"
 
 source "mm/Kconfig.debug"
-- 
2.21.0

[PATCH v5 2/5] powerpc: Replace CONFIG_DEBUG_KERNEL with CONFIG_DEBUG_MISC

[Sinan Kaya]

CONFIG_DEBUG_KERNEL should not impact code generation. Use the newly
defined CONFIG_DEBUG_MISC instead to keep the current code.

Signed-off-by: Sinan Kaya 
Reviewed-by: Josh Triplett 
---
 arch/powerpc/kernel/sysfs.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index e8e93c2c7d03..7a1708875d27 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -610,7 +610,7 @@ SYSFS_PMCSETUP(pa6t_pmc2, SPRN_PA6T_PMC2);
 SYSFS_PMCSETUP(pa6t_pmc3, SPRN_PA6T_PMC3);
 SYSFS_PMCSETUP(pa6t_pmc4, SPRN_PA6T_PMC4);
 SYSFS_PMCSETUP(pa6t_pmc5, SPRN_PA6T_PMC5);
-#ifdef CONFIG_DEBUG_KERNEL
+#ifdef CONFIG_DEBUG_MISC
 SYSFS_SPRSETUP(hid0, SPRN_HID0);
 SYSFS_SPRSETUP(hid1, SPRN_HID1);
 SYSFS_SPRSETUP(hid4, SPRN_HID4);
@@ -639,7 +639,7 @@ SYSFS_SPRSETUP(tsr0, SPRN_PA6T_TSR0);
 SYSFS_SPRSETUP(tsr1, SPRN_PA6T_TSR1);
 SYSFS_SPRSETUP(tsr2, SPRN_PA6T_TSR2);
 SYSFS_SPRSETUP(tsr3, SPRN_PA6T_TSR3);
-#endif /* CONFIG_DEBUG_KERNEL */
+#endif /* CONFIG_DEBUG_MISC */
 #endif /* HAS_PPC_PMC_PA6T */
 
 #ifdef HAS_PPC_PMC_IBM
@@ -680,7 +680,7 @@ static struct device_attribute pa6t_attrs[] = {
__ATTR(pmc3, 0600, show_pa6t_pmc3, store_pa6t_pmc3),
__ATTR(pmc4, 0600, show_pa6t_pmc4, store_pa6t_pmc4),
__ATTR(pmc5, 0600, show_pa6t_pmc5, store_pa6t_pmc5),
-#ifdef CONFIG_DEBUG_KERNEL
+#ifdef CONFIG_DEBUG_MISC
__ATTR(hid0, 0600, show_hid0, store_hid0),
__ATTR(hid1, 0600, show_hid1, store_hid1),
__ATTR(hid4, 0600, show_hid4, store_hid4),
@@ -709,7 +709,7 @@ static struct device_attribute pa6t_attrs[] = {
__ATTR(tsr1, 0600, show_tsr1, store_tsr1),
__ATTR(tsr2, 0600, show_tsr2, store_tsr2),
__ATTR(tsr3, 0600, show_tsr3, store_tsr3),
-#endif /* CONFIG_DEBUG_KERNEL */
+#endif /* CONFIG_DEBUG_MISC */
 };
 #endif /* HAS_PPC_PMC_PA6T */
 #endif /* HAS_PPC_PMC_CLASSIC */
-- 
2.21.0

Re: [PATCH v4 0/5] init: Do not select DEBUG_KERNEL by default

[Sinan Kaya]

On 4/12/2019 12:05 AM, Josh Triplett wrote:

Can you point to the typo?

I did, in my response to the patch itself:
s/Miscellaneous/miscellaneous/ in the new option's description, since it
isn't at the start of a sentence.



Thanks, your emails arrived out of order. I got them now.

Re: [PATCH v4 0/5] init: Do not select DEBUG_KERNEL by default

[Sinan Kaya]

On 4/11/2019 11:02 PM, Josh Triplett wrote:

I noticed one minor typo in patch 1/5, with that fixed, for the whole
series:


Can you point to the typo?

[PATCH v4 4/5] xtensa: Replace CONFIG_DEBUG_KERNEL with CONFIG_DEBUG_MISC

[Sinan Kaya]

CONFIG_DEBUG_KERNEL should not impact code generation. Use the newly
defined CONFIG_DEBUG_MISC instead to keep the current code.

Signed-off-by: Sinan Kaya 
---
 arch/xtensa/include/asm/irqflags.h | 2 +-
 arch/xtensa/kernel/smp.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/xtensa/include/asm/irqflags.h 
b/arch/xtensa/include/asm/irqflags.h
index 9b5e8526afe5..12890681587b 100644
--- a/arch/xtensa/include/asm/irqflags.h
+++ b/arch/xtensa/include/asm/irqflags.h
@@ -27,7 +27,7 @@ static inline unsigned long arch_local_irq_save(void)
 {
unsigned long flags;
 #if XTENSA_FAKE_NMI
-#if defined(CONFIG_DEBUG_KERNEL) && (LOCKLEVEL | TOPLEVEL) >= XCHAL_DEBUGLEVEL
+#if defined(CONFIG_DEBUG_MISC) && (LOCKLEVEL | TOPLEVEL) >= XCHAL_DEBUGLEVEL
unsigned long tmp;
 
asm volatile("rsr   %0, ps\t\n"
diff --git a/arch/xtensa/kernel/smp.c b/arch/xtensa/kernel/smp.c
index 3699d6d3e479..83b244ce61ee 100644
--- a/arch/xtensa/kernel/smp.c
+++ b/arch/xtensa/kernel/smp.c
@@ -126,7 +126,7 @@ void secondary_start_kernel(void)
 
init_mmu();
 
-#ifdef CONFIG_DEBUG_KERNEL
+#ifdef CONFIG_DEBUG_MISC
if (boot_secondary_processors == 0) {
pr_debug("%s: boot_secondary_processors:%d; Hanging cpu:%d\n",
__func__, boot_secondary_processors, cpu);
-- 
2.21.0

[PATCH v4 3/5] mips: Replace CONFIG_DEBUG_KERNEL with CONFIG_DEBUG_MISC

[Sinan Kaya]

CONFIG_DEBUG_KERNEL should not impact code generation. Use the newly
defined CONFIG_DEBUG_MISC instead to keep the current code.

Signed-off-by: Sinan Kaya 
---
 arch/mips/kernel/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 8d1dc6c71173..9fc8fadb8418 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -563,7 +563,7 @@ static void __init bootmem_init(void)
offset = __pa_symbol(_text) - __pa_symbol(VMLINUX_LOAD_ADDRESS);
memblock_free(__pa_symbol(VMLINUX_LOAD_ADDRESS), offset);
 
-#if defined(CONFIG_DEBUG_KERNEL) && defined(CONFIG_DEBUG_INFO)
+#if defined(CONFIG_DEBUG_MISC) && defined(CONFIG_DEBUG_INFO)
/*
 * This information is necessary when debugging the kernel
 * But is a security vulnerability otherwise!
-- 
2.21.0

[PATCH v4 1/5] init: Introduce DEBUG_MISC option

[Sinan Kaya]

Introduce DEBUG_MISC ("Miscellaneous debug code that should be under a more
specific debug option but isn't"), make it depend on DEBUG_KERNEL and be
"default DEBUG_KERNEL" but allow itself to be turned off, and then
mechanically change the small handful of "#ifdef CONFIG_DEBUG_KERNEL" to
"#ifdef CONFIG_DEBUG_MISC".

Signed-off-by: Sinan Kaya 
---
 lib/Kconfig.debug | 9 +
 1 file changed, 9 insertions(+)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 0d9e81779e37..2f80ebaa6d9a 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -438,6 +438,15 @@ config DEBUG_KERNEL
  Say Y here if you are developing drivers or trying to debug and
  identify kernel problems.
 
+config DEBUG_MISC
+   bool "Miscellaneous debug code"
+   default DEBUG_KERNEL
+   depends on DEBUG_KERNEL
+   help
+ Say Y here if you need to enable Miscellaneous debug code that should
+ be under a more specific debug option but isn't.
+
+
 menu "Memory Debugging"
 
 source "mm/Kconfig.debug"
-- 
2.21.0

[PATCH v4 0/5] init: Do not select DEBUG_KERNEL by default

[Sinan Kaya]

CONFIG_DEBUG_KERNEL has been designed to just enable Kconfig options.
Kernel code generatoin should not depend on CONFIG_DEBUG_KERNEL.

Proposed alternative plan: let's add a new symbol, something like
DEBUG_MISC ("Miscellaneous debug code that should be under a more
specific debug option but isn't"), make it depend on DEBUG_KERNEL and be
"default DEBUG_KERNEL" but allow itself to be turned off, and then
mechanically change the small handful of "#ifdef CONFIG_DEBUG_KERNEL" to
"#ifdef CONFIG_DEBUG_MISC".

Sinan Kaya (5):
  init: Introduce DEBUG_MISC option
  powerpc: Replace CONFIG_DEBUG_KERNEL with CONFIG_DEBUG_MISC
  mips: Replace CONFIG_DEBUG_KERNEL with CONFIG_DEBUG_MISC
  xtensa: Replace CONFIG_DEBUG_KERNEL with CONFIG_DEBUG_MISC
  net: Replace CONFIG_DEBUG_KERNEL with CONFIG_DEBUG_MISC

 arch/mips/kernel/setup.c   | 2 +-
 arch/powerpc/kernel/sysfs.c| 8 
 arch/xtensa/include/asm/irqflags.h | 2 +-
 arch/xtensa/kernel/smp.c   | 2 +-
 lib/Kconfig.debug  | 9 +
 net/netfilter/core.c   | 2 +-
 6 files changed, 17 insertions(+), 8 deletions(-)

-- 
2.21.0

Re: [PATCH v2] init: Do not select DEBUG_KERNEL by default

[Sinan Kaya]

On 4/11/2019 6:21 PM, Kees Cook wrote:

Proposed alternative plan: let's add a new symbol, something like
DEBUG_MISC ("Miscellaneous debug code that should be under a more
specific debug option but isn't"), make it depend on DEBUG_KERNEL and be
"default DEBUG_KERNEL" but allow itself to be turned off, and then
mechanically change the small handful of "#ifdef CONFIG_DEBUG_KERNEL" to
"#ifdef CONFIG_DEBUG_MISC".

Does that sound like an appropriately rapid solution for this bug?

Sure, that sounds fine to me. Sinan can you take care of that for v4?


Sure, let me work on this.

Re: [PATCH v3] init: Do not select DEBUG_KERNEL by default

[Sinan Kaya]

On 4/11/2019 1:48 AM, Masahiro Yamada wrote:

I was going by what Kconfig tells me

Symbol: KALLSYMS_ALL [=n]
   Depends on: DEBUG_KERNEL [=n] && KALLSYMS [=y]

Lots of features have 'depends on DEBUG_KERNEL'.
What is special about KALLSYMS_ALL here?


I had to do some learning about KALLSYSM_ALL. Based on my limited
searching, KALLSYMS_ALL allows you to locate the symbol location
at runtime from the kernel.

Without KALLSYM_ALL, you can only locate the kernel code only. With
KALLSYMS_ALL, you can locate the symbols for any data structure
including kernel modules.

I'm not a KALLSYMS person but based on my search, I'd consider
KALLSYMS_ALL a debug feature as it is today.

kernel/kallsyms.c:  else if (IS_ENABLED(CONFIG_KALLSYMS_ALL))
kernel/livepatch/Kconfig:   depends on KALLSYMS_ALL
kernel/module.c:#ifdef CONFIG_KALLSYMS_ALL
kernel/module.c:#ifndef CONFIG_KALLSYMS_ALL

lib/Kconfig.debug:  select KALLSYMS_ALL
config LOCKDEP
bool
depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
select KALLSYMS
select KALLSYMS_ALL

lib/Kconfig.debug:  select KALLSYMS_ALL
config LATENCYTOP
bool "Latency measuring infrastructure"
select KALLSYMS
select KALLSYMS_ALL

scripts/link-vmlinux.sh:if [ -n "${CONFIG_KALLSYMS_ALL}" ]; then

Re: [PATCH v3] init: Do not select DEBUG_KERNEL by default

[Sinan Kaya]

On 4/11/2019 1:31 AM, Masahiro Yamada wrote:

t looks like CONFIG_KALLSYMS_ALL is the only feature that
requires CONFIG_DEBUG_KERNEL.

Which part of KALLSYMS_ALL code requires CONFIG_DEBUG_KERNEL?



I was going by what Kconfig tells me

Symbol: KALLSYMS_ALL [=n]
 Depends on: DEBUG_KERNEL [=n] && KALLSYMS [=y]

Re: [PATCH v2] init: Do not select DEBUG_KERNEL by default

[Sinan Kaya]

On 4/10/2019 11:02 PM, Josh Triplett wrote:

Then let's fix*that*, and get checkpatch to help enforce it in the future. 
EXPERT doesn't affect code generation, and neither should this.


I think we have to do both. We need to go after the users as well as
solve the immediate problem per this patch.

As Mathieu identified, CONFIG_DEBUG_KERNEL is being used all over the
place and getting subsystem owners to remove let alone add a check
to checkpatch is just going to take time.

Please let us know if you are OK with this plan.

[PATCH v3] init: Do not select DEBUG_KERNEL by default

[Sinan Kaya]

We can't seem to have a kernel with CONFIG_EXPERT set but
CONFIG_DEBUG_KERNEL unset these days.

While some of the features under the CONFIG_EXPERT require
CONFIG_DEBUG_KERNEL, it doesn't apply for all features.

It looks like CONFIG_KALLSYMS_ALL is the only feature that
requires CONFIG_DEBUG_KERNEL.

Select CONFIG_EXPERT when CONFIG_DEBUG_KERNEL is chosen but
you can still choose CONFIG_EXPERT without CONFIG_DEBUG_KERNEL.

Signed-off-by: Sinan Kaya 
Reviewed-by: Kees Cook 
---
 init/Kconfig  | 2 --
 lib/Kconfig.debug | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 4592bf7997c0..37e10a8391a3 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1206,8 +1206,6 @@ config BPF
 
 menuconfig EXPERT
bool "Configure standard kernel features (expert users)"
-   # Unhide debug options, to make the on-by-default options visible
-   select DEBUG_KERNEL
help
  This option allows certain base kernel options and settings
   to be disabled or tweaked. This is for specialized
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 0d9e81779e37..9fbf3499ec8d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -434,6 +434,7 @@ config MAGIC_SYSRQ_SERIAL
 
 config DEBUG_KERNEL
bool "Kernel debugging"
+   default EXPERT
help
  Say Y here if you are developing drivers or trying to debug and
  identify kernel problems.
-- 
2.21.0

[PATCH v2] init: Do not select DEBUG_KERNEL by default

[Sinan Kaya]

We can't seem to have a kernel with CONFIG_EXPERT set but
CONFIG_DEBUG_KERNEL unset these days.

While some of the features under the CONFIG_EXPERT require
CONFIG_DEBUG_KERNEL, it doesn't apply for all features.

It looks like CONFIG_KALLSYMS_ALL is the only feature that
requires CONFIG_DEBUG_KERNEL.

Select CONFIG_EXPERT when CONFIG_DEBUG is chosen but you can
still choose CONFIG_EXPERT without CONFIG_DEBUG.

Signed-off-by: Sinan Kaya 
---
 init/Kconfig  | 2 --
 lib/Kconfig.debug | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 4592bf7997c0..37e10a8391a3 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1206,8 +1206,6 @@ config BPF
 
 menuconfig EXPERT
bool "Configure standard kernel features (expert users)"
-   # Unhide debug options, to make the on-by-default options visible
-   select DEBUG_KERNEL
help
  This option allows certain base kernel options and settings
   to be disabled or tweaked. This is for specialized
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 0d9e81779e37..9fbf3499ec8d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -434,6 +434,7 @@ config MAGIC_SYSRQ_SERIAL
 
 config DEBUG_KERNEL
bool "Kernel debugging"
+   default EXPERT
help
  Say Y here if you are developing drivers or trying to debug and
  identify kernel problems.
-- 
2.21.0

Re: [PATCH v1] init: Do not select DEBUG_KERNEL by default

[Sinan Kaya]

On 4/10/2019 6:28 PM, Kees Cook wrote:

diff --git a/init/Kconfig b/init/Kconfig
index c9386a365eea..7ce4a60ab3e9 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1188,8 +1188,6 @@ config BPF

  menuconfig EXPERT
 bool "Configure standard kernel features (expert users)"
-   # Unhide debug options, to make the on-by-default options visible
-   select DEBUG_KERNEL
 help
   This option allows certain base kernel options and settings
to be disabled or tweaked. This is for specialized
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index d4df5b24d75e..6a9bc118b64a 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -433,6 +433,7 @@ config MAGIC_SYSRQ_SERIAL

  config DEBUG_KERNEL
 bool "Kernel debugging"
+   default EXPERT
 help
   Say Y here if you are developing drivers or trying to debug and
   identify kernel problems.


Got it. Thanks. I'll test and respin v2.

Re: [PATCH v1] init: Do not select DEBUG_KERNEL by default

[Sinan Kaya]

On 4/10/2019 6:21 PM, Kees Cook wrote:

I can go after individual enables if you agree assuming Mathieu will
go after the changes in the other email. Let me know otherwise.

How about you split it, but make DEBUG_KERNEL be "default EXPERT" that
way enabling EXPERT will enable DEBUG_KERNEL still in the default
case?


Sorry, can you explain what you mean by split?

Do you mean move the things I need out of EXPERT? or something else?

Re: [PATCH v1] init: Do not select DEBUG_KERNEL by default

[Sinan Kaya]

On 4/10/2019 6:04 PM, Kees Cook wrote:


I don't want any of the debug features in my kernel but still
need all the expert features. My kernel is considered a production
kernel. I don't really want to ship all the good debug enables.


Production kernels enable it. e.g. Ubuntu:
$ grep '\bCONFIG_DEBUG_KERNEL\b' /boot/config-$(uname -r)
CONFIG_DEBUG_KERNEL=y



It makes sense for a general purpose operating system. It doesn't apply
to my limited case where I'm very concerned about image size.


I don't see the relationship between CONFIG_DEBUG and CONFIG_EXPERT
as none of the features except KALLSYMS depend on it. If there was
a compile time dependency, I'd say move it to the things that need
it as this patch suggests.


CONFIG_DEBUG_KERNEL mainly only enables the visibility of various
other options. I can only find two instances of it controlling a
"default", and one is overridden by CONFIG_SMP on alpha. :)

$ git grep -B2 'default.*DEBUG_KERNEL'
arch/alpha/Kconfig.debug-config MATHEMU
arch/alpha/Kconfig.debug-   tristate "Kernel FP software
completion" if DEBUG_KERNEL && !SMP
arch/alpha/Kconfig.debug:   default y if !DEBUG_KERNEL || SMP
--
kernel/trace/Kconfig-menuconfig FTRACE
kernel/trace/Kconfig-   bool "Tracers"
kernel/trace/Kconfig:   default y if DEBUG_KERNEL


If the idea is to just show, nothing should happen based on
DEBUG_KERNEL, right?

No default selection as in FTRACE, no c/S file changes in the code
path as Mathieu identified.

I can go after individual enables if you agree assuming Mathieu will
go after the changes in the other email. Let me know otherwise.



What do you see enabled that CONFIG_DEBUG_KERNEL enables that you don't want?

Re: [PATCH v1] init: Do not select DEBUG_KERNEL by default

[Sinan Kaya]

On 4/10/2019 5:45 PM, Kees Cook wrote:

On Wed, Apr 10, 2019 at 2:26 PM Sinan Kaya  wrote:


We can't seem to have a kernel with CONFIG_EXPERT set but
CONFIG_DEBUG_KERNEL unset these days.

While some of the features under the CONFIG_EXPERT require
CONFIG_DEBUG_KERNEL, it doesn't apply for all features.

The meaning of CONFIG_EXPERT and CONFIG_DEBUG_KERNEL has been
mixed here.


I don't agree: the point of EXPERT is to show _everything_, which
means DEBUG_KERNEL should be selected to show those options as well. I
think this is fine as-is. What is the problem you want to solve?

I think of it as low (nothing selected) medium (DEBUG_KERNEL) and high
(EXPERT and DEBUG_KERNEL). So EXPERT enables DEBUG_KERNEL too.



Sure, let's see if there is a better option.

I don't want any of the debug features in my kernel but still
need all the expert features. My kernel is considered a production
kernel. I don't really want to ship all the good debug enables.

On the other hand, I need the features under CONFIG_EXPERT to have
a functional system.

Let's take "multiple users" as an example.

What's the point of having a kernel without multiple users? :)

I don't see the relationship between CONFIG_DEBUG and CONFIG_EXPERT
as none of the features except KALLSYMS depend on it. If there was
a compile time dependency, I'd say move it to the things that need
it as this patch suggests.

P.S. I found a circular dependency now. I can respin the patch based
on feedback.

[PATCH v1] init: Do not select DEBUG_KERNEL by default

[Sinan Kaya]

We can't seem to have a kernel with CONFIG_EXPERT set but
CONFIG_DEBUG_KERNEL unset these days.

While some of the features under the CONFIG_EXPERT require
CONFIG_DEBUG_KERNEL, it doesn't apply for all features.

The meaning of CONFIG_EXPERT and CONFIG_DEBUG_KERNEL has been
mixed here.

It looks like CONFIG_KALLSYMS_ALL is the only feature that
requires CONFIG_DEBUG_KERNEL. Move the CONFIG_DEBUG_KERNEL
selection to where it really is needed.

Signed-off-by: Sinan Kaya 
---
 init/Kconfig | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 4592bf7997c0..0a6346feae8d 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1206,8 +1206,6 @@ config BPF
 
 menuconfig EXPERT
bool "Configure standard kernel features (expert users)"
-   # Unhide debug options, to make the on-by-default options visible
-   select DEBUG_KERNEL
help
  This option allows certain base kernel options and settings
   to be disabled or tweaked. This is for specialized
@@ -1473,7 +1471,8 @@ config KALLSYMS
 
 config KALLSYMS_ALL
bool "Include all symbols in kallsyms"
-   depends on DEBUG_KERNEL && KALLSYMS
+   select DEBUG_KERNEL
+   depends on KALLSYMS
help
   Normally kallsyms only contains the symbols of functions for nicer
   OOPS messages and backtraces (i.e., symbols from the text and 
inittext
-- 
2.21.0

Re: [PATCH v2] PCI: Fix "try" semantics of bus and slot reset

[Sinan Kaya]

On 2/18/2019 2:46 PM, Alex Williamson wrote:

The commit referenced below introduced device locking around save and
restore of state for each device during a PCI bus "try" reset, making
it decidely non-"try" and prone to deadlock in the event that a device
is already locked.  Restore __pci_reset_bus() and __pci_reset_slot()
to their advertised locking semantics by pushing the save and restore
functions into the branch where the entire tree is already locked.
Extend the helper function names with "_locked" and update the comment
to reflect this calling requirement.

Fixes: b014e96d1abb ("PCI: Protect pci_error_handlers->reset_notify() usage with 
device_lock()")
Signed-off-by: Alex Williamson 
---


Reviewed-by: Sinan Kaya 

Re: [PATCH v1 1/2] PCI: ATS: Add function to check ATS page aligned request status.

[Sinan Kaya]

On 2/11/2019 2:15 PM, Raj, Ashok wrote:

It seems rather odd we have to check for ATS version.

I always assumed unspecified bits (Reserved) must be 0. We only check
this if ATS is enabled, and this particular bit wasn't given away for another
feature.

Is it really required to check for ATS version before consuming this?


Reading again, it looks like version check is not necessary since it
is implied by the presence of this bit per this paragraph.

Page Aligned Request – If Set, indicates the Untranslated Address is 
always aligned to a 4096 byte boundary.  Setting this bit is 
recommended.  This bit permits software to distinguish between 
implementations compatible with earlier version of this specification 
that permitted a requester to supply anything in bits [11:2].

Re: [PATCH v1 1/2] PCI: ATS: Add function to check ATS page aligned request status.

[Sinan Kaya]

On 2/8/2019 8:02 PM, sathyanarayanan kuppuswamy wrote:

This means that you should probably have some kind of version check
here.


There is no version field in ATS v1.0 spec. Also, If I follow the 
history log in PCI spec, I think ATS if first added at v1.2. Please 
correct me if I am wrong.


v1.2 was incorporated into PCIe spec at that time. However, the ATS spec
is old and there could be some HW that could claim to be ATS compatible.
I know AMD GPUs declare ATS capability.

See this ECN

https://composter.com.ua/documents/ats_r1.1_26Jan09.pdf

You need to validate the version field from ATS capability header to be
1 before reading this register.

See Table 5-1:  ATS Extended Capability Header

Re: [PATCH v1 1/2] PCI: ATS: Add function to check ATS page aligned request status.

[Sinan Kaya]

On 2/7/2019 5:16 PM, sathyanarayanan kuppuswamy wrote:

If I remember this right, aligned request is only supported on ATS v1.1
but not supported on v1.0.

Its added in v1.1.


This means that you should probably have some kind of version check
here.

Re: [PATCH v1 1/2] PCI: ATS: Add function to check ATS page aligned request status.

[Sinan Kaya]

On 2/7/2019 1:41 PM, sathyanarayanan.kuppusw...@linux.intel.com wrote:

+ * As per PCI spec, If page aligned request bit is set, it indicates
+ * the untranslated address is always aligned to a 4096 byte boundary.
+ */
+int pci_ats_page_aligned(struct pci_dev *pdev)
+{
+   u16 cap;
+
+   if (!pdev->ats_cap)
+   return 0;
+
+   pci_read_config_word(pdev, pdev->ats_cap + PCI_ATS_CAP, );


If I remember this right, aligned request is only supported on ATS v1.1
but not supported on v1.0.

Can you please check the spec?

Re: PCI extended tags regression: 3ware 9650SE-2LP RAID controller not working on AMD Ryzen system

[Sinan Kaya]

On 2/1/2019 4:52 PM, Bjorn Helgaas wrote:

See https://bugzilla.kernel.org/show_bug.cgi?id=202425.

Robert bisected a problem with a 3ware 9650SE-2LP controller to
60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported") and
verified that reverting it solves the problem.

We have found issues in the past where Root Ports don't handle
extended tags correctly and have added quirks to avoid use of them:

   1b30dfd376e2 ("PCI: Mark Broadcom HT1100 and HT2000 Root Port Extended Tags as 
broken")
   62ce94a7a5a5 ("PCI: Mark Broadcom HT2100 Root Port Extended Tags as broken")

I don't know yet what Root Ports Robert has, but maybe we need a
similar quirk?

I'm pretty sure Root Ports that don't handle extended tags are out of
spec, so it would be ideal if we could get the manufacturer to provide
an erratum as documentation.



I agree. The issue needs to be directed to AMD. PCI spec says it is
safe to enable this option. I also have not heart of a single complaint
from Intel systems.



Bjorn

[for next][PATCH v3 2/2] platform/x86: Fix unmet dependency warning for SAMSUNG_Q10

[Sinan Kaya]

Add BACKLIGHT_LCD_SUPPORT for SAMSUNG_Q10 to fix the
warning: unmet direct dependencies detected for BACKLIGHT_CLASS_DEVICE.

SAMSUNG_Q10 selects BACKLIGHT_CLASS_DEVICE but BACKLIGHT_CLASS_DEVICE
depends on BACKLIGHT_LCD_SUPPORT.

Copy BACKLIGHT_LCD_SUPPORT dependency into SAMSUNG_Q10 to fix:

WARNING: unmet direct dependencies detected for BACKLIGHT_CLASS_DEVICE
  Depends on [n]: HAS_IOMEM [=y] && BACKLIGHT_LCD_SUPPORT [=n]
  Selected by [y]:
  - SAMSUNG_Q10 [=y] && X86 [=y] && X86_PLATFORM_DEVICES [=y] && ACPI [=y]

Signed-off-by: Sinan Kaya 
---
 drivers/platform/x86/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 11810591840d..b5e9db85e881 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1129,6 +1129,7 @@ config INTEL_OAKTRAIL
 config SAMSUNG_Q10
tristate "Samsung Q10 Extras"
depends on ACPI
+   depends on BACKLIGHT_LCD_SUPPORT
select BACKLIGHT_CLASS_DEVICE
---help---
  This driver provides support for backlight control on Samsung Q10
-- 
2.19.0

[for next][PATCH v3 1/2] platform/x86: Fix unmet dependency warning for ACPI_CMPC

[Sinan Kaya]

Add BACKLIGHT_LCD_SUPPORT for ACPI_CMPC to fix the
warning: unmet direct dependencies detected for BACKLIGHT_CLASS_DEVICE.

ACPI_CMPC selects BACKLIGHT_CLASS_DEVICE but BACKLIGHT_CLASS_DEVICE
depends on BACKLIGHT_LCD_SUPPORT.

Copy BACKLIGHT_LCD_SUPPORT dependency into ACPI_CMPC to fix

WARNING: unmet direct dependencies detected for BACKLIGHT_CLASS_DEVICE
  Depends on [n]: HAS_IOMEM [=y] && BACKLIGHT_LCD_SUPPORT [=n]
  Selected by [y]:
  - ACPI_CMPC [=y] && X86 [=y] && X86_PLATFORM_DEVICES [=y] && ACPI [=y] && 
INPUT [=y] && (RFKILL [=n] || RFKILL [=n]=n)

Signed-off-by: Sinan Kaya 
---
 drivers/platform/x86/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 5e2109c54c7c..11810591840d 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -905,6 +905,7 @@ config TOSHIBA_WMI
 config ACPI_CMPC
tristate "CMPC Laptop Extras"
depends on ACPI && INPUT
+   depends on BACKLIGHT_LCD_SUPPORT
depends on RFKILL || RFKILL=n
select BACKLIGHT_CLASS_DEVICE
help
-- 
2.19.0

[for next][PATCH v2 2/2] platform/x86: Fix unmet dependency warning for SAMSUNG_Q10

[Sinan Kaya]

Add BACKLIGHT_LCD_SUPPORT for SAMSUNG_Q10 to fix the
warning: unmet direct dependencies detected for BACKLIGHT_CLASS_DEVICE.

SAMSUNG_Q10 selects BACKLIGHT_CLASS_DEVICE but BACKLIGHT_CLASS_DEVICE
depends on BACKLIGHT_LCD_SUPPORT.

Copy BACKLIGHT_LCD_SUPPORT dependency into SAMSUNG_Q10 to fix:

WARNING: unmet direct dependencies detected for BACKLIGHT_CLASS_DEVICE
  Depends on [n]: HAS_IOMEM [=y] && BACKLIGHT_LCD_SUPPORT [=n]
  Selected by [y]:
  - SAMSUNG_Q10 [=y] && X86 [=y] && X86_PLATFORM_DEVICES [=y] && ACPI [=y]

Signed-off-by: Sinan Kaya 
---
 drivers/platform/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index b84c2c5b6684..129e37c296a7 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1127,7 +1127,7 @@ config INTEL_OAKTRAIL
 
 config SAMSUNG_Q10
tristate "Samsung Q10 Extras"
-   depends on ACPI
+   depends on ACPI && BACKLIGHT_LCD_SUPPORT
select BACKLIGHT_CLASS_DEVICE
---help---
  This driver provides support for backlight control on Samsung Q10
-- 
2.19.0

[for next][PATCH v2 1/2] platform/x86: Fix unmet dependency warning for ACPI_CMPC

[Sinan Kaya]

Add BACKLIGHT_LCD_SUPPORT for ACPI_CMPC to fix the
warning: unmet direct dependencies detected for BACKLIGHT_CLASS_DEVICE.

ACPI_CMPC selects BACKLIGHT_CLASS_DEVICE but BACKLIGHT_CLASS_DEVICE
depends on BACKLIGHT_LCD_SUPPORT.

Copy BACKLIGHT_LCD_SUPPORT dependency into ACPI_CMPC to fix

WARNING: unmet direct dependencies detected for BACKLIGHT_CLASS_DEVICE
  Depends on [n]: HAS_IOMEM [=y] && BACKLIGHT_LCD_SUPPORT [=n]
  Selected by [y]:
  - ACPI_CMPC [=y] && X86 [=y] && X86_PLATFORM_DEVICES [=y] && ACPI [=y] && 
INPUT [=y] && (RFKILL [=n] || RFKILL [=n]=n)

Signed-off-by: Sinan Kaya 
---
 drivers/platform/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 5e2109c54c7c..b84c2c5b6684 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -904,7 +904,7 @@ config TOSHIBA_WMI
 
 config ACPI_CMPC
tristate "CMPC Laptop Extras"
-   depends on ACPI && INPUT
+   depends on ACPI && INPUT && BACKLIGHT_LCD_SUPPORT
depends on RFKILL || RFKILL=n
select BACKLIGHT_CLASS_DEVICE
help
-- 
2.19.0

Re: [for next][PATCH 1/2] platform/x86: Fix unmet dependency warning for ACPI_CMPC

[Sinan Kaya]

On 1/24/2019 5:52 AM, Rafael J. Wysocki wrote:

Andy/Darren, these two seem to be for you, but I can take them too as
related to ACPI tagentially, so please let me know.


I'll post V2 to fix a "fat-finger" in summary. I was hoping to receive a
feedback until now.

I'll get the V2 out.

Re: [for next][PATCH 1/2] mfd: Fix unmet dependency warning for MFD_TPS68470

[Sinan Kaya]

On 1/24/2019 5:51 AM, Rafael J. Wysocki wrote:

Is anyone taking this or should I?


Nobody replied to this yet. I was hoping this series to go through acpi
tree like the rest of the other fixes.

Re: [for next][PATCH 2/2] x86/Kconfig: Select PCI_LOCKLESS_CONFIG if PCI is enabled

[Sinan Kaya]

On 1/24/2019 5:51 AM, Rafael J. Wysocki wrote:



Is anyone taking this or should I?





This got applied:

https://git.kernel.org/tip/625210cfa6c0c26ea422f655bf68288176f174e6

[tip:x86/urgent] x86/Kconfig: Select PCI_LOCKLESS_CONFIG if PCI is enabled

[tip-bot for Sinan Kaya]

Commit-ID:  625210cfa6c0c26ea422f655bf68288176f174e6
Gitweb: https://git.kernel.org/tip/625210cfa6c0c26ea422f655bf68288176f174e6
Author: Sinan Kaya 
AuthorDate: Mon, 21 Jan 2019 23:19:58 +
Committer:  Borislav Petkov 
CommitDate: Tue, 22 Jan 2019 17:06:28 +0100

x86/Kconfig: Select PCI_LOCKLESS_CONFIG if PCI is enabled

After commit

  5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")

dependencies on CONFIG_PCI that previously were satisfied implicitly
through dependencies on CONFIG_ACPI have to be specified directly.

PCI_LOCKLESS_CONFIG depends on PCI but this dependency has not been
mentioned in the Kconfig so add an explicit dependency here and fix

  WARNING: unmet direct dependencies detected for PCI_LOCKLESS_CONFIG
Depends on [n]: PCI [=n]
Selected by [y]:
- X86 [=y]

Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")
Signed-off-by: Sinan Kaya 
Signed-off-by: Borislav Petkov 
Cc: "H. Peter Anvin" 
Cc: Ingo Molnar 
Cc: Thomas Gleixner 
Cc: linux-a...@vger.kernel.org
Cc: x86-ml 
Link: https://lkml.kernel.org/r/20190121231958.28255-2-ok...@kernel.org
---
 arch/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 4b4a7f32b68e..26387c7bf305 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -198,7 +198,7 @@ config X86
select IRQ_FORCED_THREADING
select NEED_SG_DMA_LENGTH
select PCI_DOMAINS  if PCI
-   select PCI_LOCKLESS_CONFIG
+   select PCI_LOCKLESS_CONFIG  if PCI
select PERF_EVENTS
select RTC_LIB
select RTC_MC146818_LIB

Re: [for next][PATCH] iwlwifi: Fix unmet dependency error for IWLWIFI_LEDS

[Sinan Kaya]

On 1/22/2019 11:17 AM, Kalle Valo wrote:

Sinan Kaya  writes:


On 1/22/2019 5:15 AM, Luciano Coelho wrote:

On Mon, 2019-01-21 at 23:31 +, Sinan Kaya wrote:

There is an unresolved dependency as follows:

IWLWIFI_LEDS selects MAC80211_LEDS.
MAC80211_LEDS depends on MAC80211.

It is possible to choose MAC80211_LEDS (y) but not choose MAC80211
(n)

WARNING: unmet direct dependencies detected for MAC80211_LEDS
Depends on [n]: NET [=y] && WIRELESS [=y] && MAC80211 [=n] &&
LEDS_CLASS [=y]
Selected by [y]:
- IWLWIFI_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] &&
WLAN_VENDOR_INTEL [=y] && IWLWIFI [=y] && (LEDS_CLASS [=y]=y ||
LEDS_CLASS [=y]=IWLWIFI [=y])

Move the MAC80211 dependency into IWLWIFI_LEDS so that we avoid this
configuration.

Signed-off-by: Sinan Kaya 
---


Thanks for your patch! But we already have another patch to fix this
issued queued for 5.0-rc4 (it's currently in wireless-drivers.git):

https://patchwork.kernel.org/patch/10762079/


Is it possible to queue this up soon? There is an effort to clean up
linux-next against randconfig failures and this issue showed up there.


It already should be in linux-next:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git/commit/?id=ec5aecc0b227f5509d25853537f989ca303e2be1

But it's not in Linus' tree, yet.



Thanks, let me grab a recent linux-next tag.

Re: [for next][PATCH] iwlwifi: Fix unmet dependency error for IWLWIFI_LEDS

[Sinan Kaya]

On 1/22/2019 5:15 AM, Luciano Coelho wrote:

On Mon, 2019-01-21 at 23:31 +, Sinan Kaya wrote:

There is an unresolved dependency as follows:

IWLWIFI_LEDS selects MAC80211_LEDS.
MAC80211_LEDS depends on MAC80211.

It is possible to choose MAC80211_LEDS (y) but not choose MAC80211
(n)

WARNING: unmet direct dependencies detected for MAC80211_LEDS
   Depends on [n]: NET [=y] && WIRELESS [=y] && MAC80211 [=n] &&
LEDS_CLASS [=y]
   Selected by [y]:
   - IWLWIFI_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] &&
WLAN_VENDOR_INTEL [=y] && IWLWIFI [=y] && (LEDS_CLASS [=y]=y ||
LEDS_CLASS [=y]=IWLWIFI [=y])

Move the MAC80211 dependency into IWLWIFI_LEDS so that we avoid this
configuration.

Signed-off-by: Sinan Kaya 
---


Thanks for your patch! But we already have another patch to fix this
issued queued for 5.0-rc4 (it's currently in wireless-drivers.git):

https://patchwork.kernel.org/patch/10762079/


Is it possible to queue this up soon? There is an effort to clean up
linux-next against randconfig failures and this issue showed up there.



--
Cheers,
Luca.

Re: [for next][PATCH 2/2] x86/Kconfig: Select PCI_LOCKLESS_CONFIG if PCI is enabled

[Sinan Kaya]

On 1/22/19, Borislav Petkov  wrote:
> On Mon, Jan 21, 2019 at 11:19:58PM +0000, Sinan Kaya wrote:
>> After 'commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without
>> CONFIG_PCI set")' dependencies on CONFIG_PCI that previously were
>> satisfied implicitly through dependencies on CONFIG_ACPI have to be
>> specified directly.
>>
>> PCI_LOCKLESS_CONFIG depends on PCI but this dependency has not been
>> mentioned in the Kconfig.
>>
>> Add an explicit dependency here.
>>
>> Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI
>> set")
>> Signed-off-by: Sinan Kaya 
>> ---
>>  arch/x86/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 568f339595ed..0519da6f8ee4 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -199,7 +199,7 @@ config X86
>>  select IRQ_FORCED_THREADING
>>  select NEED_SG_DMA_LENGTH
>>  select PCI_DOMAINS  if PCI
>> -select PCI_LOCKLESS_CONFIG
>> +select PCI_LOCKLESS_CONFIG  if PCI
>>  select PERF_EVENTS
>>  select RTC_LIB
>>  select RTC_MC146818_LIB
>> --
>
> AFAICT, this is not really fixing a random config build issue but only
> correcting the dependency, right?
>

This is fixing a warning found by randconfig on this thread
'linux-next: Tree for Jan 16 (PCI config warning?)'

> --
> Regards/Gruss,
> Boris.
>
> Good mailing practices for 400: avoid top-posting and trim the reply.
>

[for next][PATCH] iwlwifi: Fix unmet dependency error for IWLWIFI_LEDS

[Sinan Kaya]

There is an unresolved dependency as follows:

IWLWIFI_LEDS selects MAC80211_LEDS.
MAC80211_LEDS depends on MAC80211.

It is possible to choose MAC80211_LEDS (y) but not choose MAC80211 (n)

WARNING: unmet direct dependencies detected for MAC80211_LEDS
  Depends on [n]: NET [=y] && WIRELESS [=y] && MAC80211 [=n] && LEDS_CLASS [=y]
  Selected by [y]:
  - IWLWIFI_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] && WLAN_VENDOR_INTEL [=y] 
&& IWLWIFI [=y] && (LEDS_CLASS [=y]=y || LEDS_CLASS [=y]=IWLWIFI [=y])

Move the MAC80211 dependency into IWLWIFI_LEDS so that we avoid this
configuration.

Signed-off-by: Sinan Kaya 
---
 drivers/net/wireless/intel/iwlwifi/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/Kconfig 
b/drivers/net/wireless/intel/iwlwifi/Kconfig
index 491ca3c8b43c..74f0d0bbed34 100644
--- a/drivers/net/wireless/intel/iwlwifi/Kconfig
+++ b/drivers/net/wireless/intel/iwlwifi/Kconfig
@@ -46,7 +46,7 @@ if IWLWIFI
 
 config IWLWIFI_LEDS
bool
-   depends on LEDS_CLASS=y || LEDS_CLASS=IWLWIFI
+   depends on MAC80211 && (LEDS_CLASS=y || LEDS_CLASS=IWLWIFI)
select LEDS_TRIGGERS
select MAC80211_LEDS
default y
-- 
2.19.0

[for next][PATCH 1/2] mfd: Fix unmet dependency warning for MFD_TPS68470

[Sinan Kaya]

After 'commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without
CONFIG_PCI set")' dependencies on CONFIG_PCI that previously were
satisfied implicitly through dependencies on CONFIG_ACPI have to be
specified directly.

WARNING: unmet direct dependencies detected for I2C_DESIGNWARE_PLATFORM
  Depends on [n]: I2C [=y] && HAS_IOMEM [=y] && (ACPI [=y] && COMMON_CLK [=n] 
|| !ACPI [=y])
  Selected by [y]:
  - MFD_TPS68470 [=y] && HAS_IOMEM [=y] && ACPI [=y] && I2C [=y]=y

MFD_TPS68470 is an ACPI only device and selects I2C_DESIGNWARE_PLATFORM.
I2C_DESIGNWARE_PLATFORM does not have any configuration today for ACPI
support without CONFIG_PCI set.

For sake of a quick fix this introduces a new mandatory dependency to the
driver which may survive without it. Otherwise we need to revisit the
driver architecture to address this properly.

Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")
Signed-off-by: Sinan Kaya 
---
 drivers/mfd/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index f461460a2aeb..76f9909cf396 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1419,7 +1419,7 @@ config MFD_TPS65217
 
 config MFD_TPS68470
bool "TI TPS68470 Power Management / LED chips"
-   depends on ACPI && I2C=y
+   depends on ACPI && PCI && I2C=y
select MFD_CORE
select REGMAP_I2C
select I2C_DESIGNWARE_PLATFORM
-- 
2.19.0

[for next][PATCH 2/2] x86/Kconfig: Select PCI_LOCKLESS_CONFIG if PCI is enabled

[Sinan Kaya]

After 'commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without
CONFIG_PCI set")' dependencies on CONFIG_PCI that previously were
satisfied implicitly through dependencies on CONFIG_ACPI have to be
specified directly.

PCI_LOCKLESS_CONFIG depends on PCI but this dependency has not been
mentioned in the Kconfig.

Add an explicit dependency here.

Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")
Signed-off-by: Sinan Kaya 
---
 arch/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 568f339595ed..0519da6f8ee4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -199,7 +199,7 @@ config X86
select IRQ_FORCED_THREADING
select NEED_SG_DMA_LENGTH
select PCI_DOMAINS  if PCI
-   select PCI_LOCKLESS_CONFIG
+   select PCI_LOCKLESS_CONFIG  if PCI
select PERF_EVENTS
select RTC_LIB
select RTC_MC146818_LIB
-- 
2.19.0

Re: [for-next][PATCH] x86/Kconfig: Select PCI_LOCKLESS_CONFIG if PCI is enabled

[Sinan Kaya]

On 1/18/2019 6:45 AM, Rafael J. Wysocki wrote:

On Thu, Jan 17, 2019 at 11:10 PM Borislav Petkov  wrote:


On Thu, Jan 17, 2019 at 11:05:43PM +0100, Rafael J. Wysocki wrote:

I have patches for intel_ips and intel_pmc_ipc queued up (will be
pushed for -rc3), plus some others.


Yeah, I saw the patchset and applied some of them locally so that I be
able to do randconfig builds. Do you have a branch somewhere which I can
merge locally for testing?


You can pull the tag I've just pushed to Linus:
https://lore.kernel.org/lkml/CAJZ5v0jiK=zLP4cUsw=y9ea7PLHHgy=xshashfhtgydjh+k...@mail.gmail.com/T/#u



Here is a result of my randconfig analysis using next-20190116.

There are two issues that are related to my change:
1. x86/Kconfig: Select PCI_LOCKLESS_CONFIG if PCI is enabled
2. mfd: Fix unmet dependency warning for MFD_TPS68470

I'll post v2 for this patch to pick up the above commit #2.

and three unrelated changes as follows:
1. platform/x86: Fix unmet dependency warning for SAMSUNG_Q10
2. platform/x86: Fix unmet dependency warning for ACPI_CMPC
3. iwlwifi: Fix unmet dependency warning for MAC80211_LEDS

I'm posting these unrelated changes independently.

Re: [for next][PATCH 1/2] platform/x86: Fix unmet dependency warning for ACPI_CMPC

[Sinan Kaya]

On 1/21/2019 5:46 PM, Sinan Kaya wrote:

ACPI_CMPC selects BACKLIGHT_LCD_SUPPORT but BACKLIGHT_LCD_SUPPORT
depends on BACKLIGHT_LCD_SUPPORT.


This should have been:

ACPI_CMPC selects BACKLIGHT_CLASS_DEVICE but BACKLIGHT_CLASS_DEVICE
depends on BACKLIGHT_LCD_SUPPORT.

[for next][PATCH 1/2] platform/x86: Fix unmet dependency warning for ACPI_CMPC

[Sinan Kaya]

Add BACKLIGHT_LCD_SUPPORT for ACPI_CMPC to fix the
warning: unmet direct dependencies detected for BACKLIGHT_CLASS_DEVICE.

ACPI_CMPC selects BACKLIGHT_LCD_SUPPORT but BACKLIGHT_LCD_SUPPORT
depends on BACKLIGHT_LCD_SUPPORT.

Copy this dependency into ACPI_CMPC.

Signed-off-by: Sinan Kaya 
---
 drivers/platform/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 5e2109c54c7c..b84c2c5b6684 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -904,7 +904,7 @@ config TOSHIBA_WMI
 
 config ACPI_CMPC
tristate "CMPC Laptop Extras"
-   depends on ACPI && INPUT
+   depends on ACPI && INPUT && BACKLIGHT_LCD_SUPPORT
depends on RFKILL || RFKILL=n
select BACKLIGHT_CLASS_DEVICE
help
-- 
2.19.0

[for next][PATCH 2/2] platform/x86: Fix unmet dependency warning for SAMSUNG_Q10

[Sinan Kaya]

Add BACKLIGHT_LCD_SUPPORT for SAMSUNG_Q10 to fix the
warning: unmet direct dependencies detected for BACKLIGHT_CLASS_DEVICE.

SAMSUNG_Q10 selects BACKLIGHT_LCD_SUPPORT but BACKLIGHT_LCD_SUPPORT
depends on BACKLIGHT_LCD_SUPPORT.

Copy this dependency into SAMSUNG_Q10.

Signed-off-by: Sinan Kaya 
---
 drivers/platform/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index b84c2c5b6684..129e37c296a7 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1127,7 +1127,7 @@ config INTEL_OAKTRAIL
 
 config SAMSUNG_Q10
tristate "Samsung Q10 Extras"
-   depends on ACPI
+   depends on ACPI && BACKLIGHT_LCD_SUPPORT
select BACKLIGHT_CLASS_DEVICE
---help---
  This driver provides support for backlight control on Samsung Q10
-- 
2.19.0

Re: [for-next][PATCH] x86/Kconfig: Select PCI_LOCKLESS_CONFIG if PCI is enabled

[Sinan Kaya]

On 1/17/2019 5:09 PM, Borislav Petkov wrote:

On Thu, Jan 17, 2019 at 11:05:43PM +0100, Rafael J. Wysocki wrote:

I have patches for intel_ips and intel_pmc_ipc queued up (will be
pushed for -rc3), plus some others.


Yeah, I saw the patchset and applied some of them locally so that I be
able to do randconfig builds. Do you have a branch somewhere which I can
merge locally for testing?



You can use this tag (next-20190116) for local test.

Re: [for-next][PATCH] x86/Kconfig: Select PCI_LOCKLESS_CONFIG if PCI is enabled

[Sinan Kaya]

On 1/17/2019 11:37 AM, Borislav Petkov wrote:

Also, I see a lot of build failures when doing randconfig builds for the
stuff in drivers/platform/x86/Kconfig. Is someone picking those up too?


Can you share the build failures you are seeing?

Re: [for-next][PATCH] x86/Kconfig: Select PCI_LOCKLESS_CONFIG if PCI is enabled

[Sinan Kaya]

On 1/17/2019 11:37 AM, Borislav Petkov wrote:

On Thu, Jan 17, 2019 at 04:17:22PM +, Sinan Kaya wrote:

After 'commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without
CONFIG_PCI set")' dependencies on CONFIG_PCI that previously were
satisfied implicitly through dependencies on CONFIG_ACPI have to be
specified directly.

PCI_LOCKLESS_CONFIG depends on PCI but this dependency has not been
mentioned in the Kconfig.

Add an explicit dependency here.

Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")
Signed-off-by: Sinan Kaya 
---
  arch/x86/Kconfig | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 568f339595ed..0519da6f8ee4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -199,7 +199,7 @@ config X86
select IRQ_FORCED_THREADING
select NEED_SG_DMA_LENGTH
select PCI_DOMAINS  if PCI
-   select PCI_LOCKLESS_CONFIG
+   select PCI_LOCKLESS_CONFIG  if PCI
select PERF_EVENTS
select RTC_LIB
select RTC_MC146818_LIB
--


Are there any more arch/x86/ fixes for the 5d32a66541c4 fallout floating
around?

If so, pls merge them all together into a single patch so that we're
done with this once and for all.

Also, I see a lot of build failures when doing randconfig builds for the
stuff in drivers/platform/x86/Kconfig. Is someone picking those up too?

If not, I'd take 'em too in a single patch, if Darren and Andy are fine
with it.


I'm only looking at PCI and ACPI related changes to be honest. The rest is
out of my expertise area.

I started a run of 1000 randconfig runs in the meantime. I'll collect the
results soon and follow up if there is something I can help.



Thx.

[for-next][PATCH] x86/Kconfig: Select PCI_LOCKLESS_CONFIG if PCI is enabled

[Sinan Kaya]

After 'commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without
CONFIG_PCI set")' dependencies on CONFIG_PCI that previously were
satisfied implicitly through dependencies on CONFIG_ACPI have to be
specified directly.

PCI_LOCKLESS_CONFIG depends on PCI but this dependency has not been
mentioned in the Kconfig.

Add an explicit dependency here.

Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")
Signed-off-by: Sinan Kaya 
---
 arch/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 568f339595ed..0519da6f8ee4 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -199,7 +199,7 @@ config X86
select IRQ_FORCED_THREADING
select NEED_SG_DMA_LENGTH
select PCI_DOMAINS  if PCI
-   select PCI_LOCKLESS_CONFIG
+   select PCI_LOCKLESS_CONFIG  if PCI
select PERF_EVENTS
select RTC_LIB
select RTC_MC146818_LIB
-- 
2.19.0

Re: linux-next: Tree for Jan 16 (PCI config warning?)

[Sinan Kaya]

Hi,

On 1/16/2019 12:00 PM, Randy Dunlap wrote:

On 1/15/19 10:38 PM, Stephen Rothwell wrote:

Hi all,

Changes since 20190115:



on x86_64:

when CONFIG_PCI is not enabled (via randconfig):

WARNING: unmet direct dependencies detected for PCI_LOCKLESS_CONFIG
   Depends on [n]: PCI [=n]
   Selected by [y]:
   - X86 [=y]




It looks like we'll have to continue peeling the onion some more time.
Can you please attach the kernel configuration file used?

Can I assume that this tree includes Rafael's latest pm pull?

Sinan

Re: linux-next: Fixes tags need some work in the pm tree

[Sinan Kaya]

On 1/15/2019 3:55 PM, Stephen Rothwell wrote:

[I am experimenting with checking the Fixes tags in commits in linux-next.
Please let me know if you think I am being too strict.]

Hi Rafael,

Commits

   62b33d57c534 ("drivers: thermal: int340x_thermal: Make PCI dependency 
explicit")
   cd793ab22a93 ("x86/intel/lpss: Make PCI dependency explicit")
   42ac19e7b81e ("ACPI: EC: Look for ECDT EC after calling acpi_load_tables()")
   6c29b81b5695 ("platform/x86: apple-gmux: Make PCI dependency explicit")
   34783dc0182a ("platform/x86: intel_pmc: Make PCI dependency explicit")
   704658d1d3ae ("platform/x86: intel_ips: make PCI dependency explicit")
   5df37f3a1aa9 ("vga-switcheroo: make PCI dependency explicit")
   da1df6ee4296 ("ata: pata_acpi: Make PCI dependency explicit")
   ce97a22a596b ("ACPI / LPSS: Make PCI dependency explicit")

Have malformed Fixes tags:

There should be double quotes around the commit subject.



Interesting, can you add this to the checkpatch.pl script so that it doesn't
happen again?

[tip:x86/urgent] x86/intel/lpss: Make PCI dependency explicit

[tip-bot for Sinan Kaya]

Commit-ID:  5962dd22f0ff6f7d72fff974b3c637d52586643e
Gitweb: https://git.kernel.org/tip/5962dd22f0ff6f7d72fff974b3c637d52586643e
Author: Sinan Kaya 
AuthorDate: Wed, 2 Jan 2019 18:10:37 +
Committer:  Borislav Petkov 
CommitDate: Fri, 11 Jan 2019 19:32:22 +0100

x86/intel/lpss: Make PCI dependency explicit

After commit

  5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")

dependencies on CONFIG_PCI that previously were satisfied implicitly
through dependencies on CONFIG_ACPI have to be specified directly. LPSS
code relies on PCI infrastructure but this dependency has not been
explicitly called out so do that.

Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")
Signed-off-by: Sinan Kaya 
Signed-off-by: Borislav Petkov 
Cc: "H. Peter Anvin" 
Cc: linux-a...@vger.kernel.org
Cc: x86-ml 
Link: https://lkml.kernel.org/r/20190102181038.4418-11-ok...@kernel.org
---
 arch/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 15af091611e2..4b4a7f32b68e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -617,7 +617,7 @@ config X86_INTEL_QUARK
 
 config X86_INTEL_LPSS
bool "Intel Low Power Subsystem Support"
-   depends on X86 && ACPI
+   depends on X86 && ACPI && PCI
select COMMON_CLK
select PINCTRL
select IOSF_MBI

Re: [PATCH] drm: Require PCI for selecting VGA_ARB.

[Sinan Kaya]

On 1/8/19, Paul Menzel  wrote:
> Dear Maarten,
>
>
> Thank you very much for the quick response.
>
> On 01/08/19 16:37, Maarten Lankhorst wrote:
>> Op 08-01-2019 om 16:07 schreef Paul Menzel:
>
>>> Building Linux 5.0-rc1 fails with the errors below. Please find the
>>> configuration file attached.
>>>
>>> ```
>>> $ make -j120
>>> […]
>>> drivers/gpu/vga/vgaarb.c: In function ‘__vga_tryget’:
>>> drivers/gpu/vga/vgaarb.c:286:14: error: ‘PCI_VGA_STATE_CHANGE_DECODES’
>>> undeclared (first use in this function); did you mean
>>> ‘PCI_SUBTRACTIVE_DECODE’?
>>>  flags |= PCI_VGA_STATE_CHANGE_DECODES;
>>>   ^~~~
>>>   PCI_SUBTRACTIVE_DECODE
>>> drivers/gpu/vga/vgaarb.c:286:14: note: each undeclared identifier is
>>> reported only once for each function it appears in
>>>   CC [M]  net/netfilter/xt_realm.o
>>>   CC  drivers/hid/hid-cherry.o
>>> drivers/gpu/vga/vga_switcheroo.c: In function
>>> ‘vga_switcheroo_runtime_suspend’:
>>> drivers/gpu/vga/vga_switcheroo.c:1053:2: error: implicit declaration of
>>> function ‘pci_bus_set_current_state’; did you mean ‘__set_current_state’?
>>> [-Werror=implicit-function-declaration]
>>>   pci_bus_set_current_state(pdev->bus, PCI_D3cold);
>>>   ^
>>>   __set_current_state
>>> drivers/gpu/vga/vgaarb.c:291:13: error: ‘PCI_VGA_STATE_CHANGE_BRIDGE’
>>> undeclared (first use in this function); did you mean
>>> ‘PCI_VGA_STATE_CHANGE_DECODES’?
>>> flags |= PCI_VGA_STATE_CHANGE_BRIDGE;
>>>  ^~~
>>>  PCI_VGA_STATE_CHANGE_DECODES
>>>   CC  fs/reiserfs/dir.o
>>>   LD [M]  net/tipc/tipc.o
>>> drivers/gpu/vga/vga_switcheroo.c: In function
>>> ‘vga_switcheroo_runtime_resume’:
>>> drivers/gpu/vga/vga_switcheroo.c:1067:2: error: implicit declaration of
>>> function ‘pci_wakeup_bus’; did you mean ‘__wake_up_bit’?
>>> [-Werror=implicit-function-declaration]
>>>   pci_wakeup_bus(pdev->bus);
>>>   ^~
>>>   __wake_up_bit
>>> drivers/gpu/vga/vgaarb.c:293:3: error: implicit declaration of function
>>> ‘pci_set_vga_state’; did you mean ‘pci_set_power_state’?
>>> [-Werror=implicit-function-declaration]
>>>pci_set_vga_state(conflict->pdev, false, pci_bits, flags);
>>>^
>>>pci_set_power_state
>>>   CC  fs/read_write.o
>>>   CC  drivers/hid/hid-chicony.o
>>>   CC  drivers/hid/hid-cypress.o
>>> drivers/gpu/vga/vgaarb.c: In function ‘vga_arb_device_init’:
>>> drivers/gpu/vga/vgaarb.c:1495:25: error: ‘pci_bus_type’ undeclared (first
>>> use in this function); did you mean ‘pci_pcie_type’?
>>>   bus_register_notifier(_bus_type, _notifier);
>>>  ^~~~
>>>  pci_pcie_type
>>> cc1: some warnings being treated as errors
>>> make[3]: *** [scripts/Makefile.build:277: drivers/gpu/vga/vgaarb.o] Error
>>> 1
>>> make[3]: *** Waiting for unfinished jobs
>>> […]
>>> ```
>
>> WARNING: unmet direct dependencies detected for VGA_ARB
>>   Depends on [n]: HAS_IOMEM [=y] && PCI [=n] && !S390
>>   Selected by [y]:
>>   - VGA_SWITCHEROO [=y] && HAS_IOMEM [=y] && X86 [=y] && ACPI [=y]
>>
>> So I guess you need to enable PCI, probably not a common config you're
>> using. :)
>>
>> Especially since you selected EXPERT.
>
> We have the attached defconfig, which is then integrated using
> `make olddefconfig`.
>
>> Oh well, apply this with git am --scissors?
>> -8<
>> When configuring the kernel without PCI we can still enable VGA
>> switcheroo,
>> which is not possible because VGA_ARB cannot be selected.
>>
>> Remove this by depending on PCI for !S390.
>>
>> Reported-by: Paul Menzel 
>> Signed-off-by: Maarten Lankhorst 
>> ---
>> diff --git a/drivers/gpu/vga/Kconfig b/drivers/gpu/vga/Kconfig
>> index b677e5d524e6..ef5671475870 100644
>> --- a/drivers/gpu/vga/Kconfig
>> +++ b/drivers/gpu/vga/Kconfig
>> @@ -21,6 +21,7 @@ config VGA_SWITCHEROO
>>  bool "Laptop Hybrid Graphics - GPU switching support"
>>  depends on X86
>>  depends on ACPI
>> +depends on (PCI && !S390) # For VGA_ARB
>>  select VGA_ARB
>>  help
>>Many laptops released in 2008/9/10 have two GPUs with a multiplexer
>
> Is this an effect of commit eb01d42a (PCI: consolidate PCI config entry in
> drivers/pci) as the `default y` is missing now?
>

See this :

https://patchwork.kernel.org/patch/10749209/

This change is about to go in.

>
> Kind regards,
>
> Paul
>

Re: [PATCH v6 07/11] drivers: thermal: int3406_thermal: Make PCI dependency explicit

[Sinan Kaya]

On Tue, Jan 8, 2019 at 6:58 AM Zhang Rui  wrote:
>
> On 一, 2019-01-07 at 12:19 +0100, Rafael J. Wysocki wrote:
> > On Sat, Jan 5, 2019 at 11:06 AM Sinan Kaya  wrote:
> > >
> > >
> > > After 'commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built
> > > without
> > > CONFIG_PCI set")' dependencies on CONFIG_PCI that previously were
> > > satisfied implicitly through dependencies on CONFIG_ACPI have to be
> > > specified directly. Need CONFIG_PCI to be set in order to be able
> > > to use
> > > this driver.
> > >
> > > Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without
> > > CONFIG_PCI set")
> > > Signed-off-by: Sinan Kaya 
> > > ---
> > >  drivers/thermal/intel/int340x_thermal/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/thermal/intel/int340x_thermal/Kconfig
> > > b/drivers/thermal/intel/int340x_thermal/Kconfig
> > > index 0582bd12a239..fba1976d5f8d 100644
> > > --- a/drivers/thermal/intel/int340x_thermal/Kconfig
> > > +++ b/drivers/thermal/intel/int340x_thermal/Kconfig
> > > @@ -31,7 +31,7 @@ if INT340X_THERMAL
> > >
> > >  config INT3406_THERMAL
> > > tristate "ACPI INT3406 display thermal driver"
> > > -   depends on ACPI_VIDEO
> > > +   depends on ACPI_VIDEO && PCI
> > > help
> > >   The display thermal device represents the LED/LCD display
> > > panel
> > >   that may or may not include touch support. The main
> > > function of
> > > --
> > Rui, any objections here?  And for the [11/11]?
>
> CONFIG_INT3406_THERMAL depends on CONFIG_INT340X_THERMAL, so IMO, patch
> 11 is sufficient.

Yes, we can drop this patch. I found the issues in reverse order. Can
we get an ACK for 11/11?

>
> thanks,
> rui

Re: [PATCH v6 08/11] ASoC: Intel: atom: Make PCI dependency explicit

[Sinan Kaya]

On Mon, Jan 7, 2019 at 7:19 AM Mark Brown  wrote:
>
> On Mon, Jan 07, 2019 at 12:15:35PM +0100, Rafael J. Wysocki wrote:
> > On Sat, Jan 5, 2019 at 11:06 AM Sinan Kaya  wrote:
>
> > > Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without 
> > > CONFIG_PCI set")
> > > Signed-off-by: Sinan Kaya 
>
> > Sinan, I thought you received an ACK from Pierre-Louis on this one, didn't 
> > you?
>
> Yes.  I've also applied it already and it's *still* being sent without
> either a cover letter or the rest of the series :(

There is a cover letter but it is being sent to Linux-acpi and Linux-next only.

https://patchwork.kernel.org/cover/10749203/

This means that you are not on either of these mailing lists.

>
> Sinan:
>
> Please do not submit new versions of already applied patches, please
> submit incremental updates to the existing code.  Modifying existing
> commits creates problems for other users building on top of those
> commits so it's best practice to only change pubished git commits if
> absolutely essential.

I apologize. I didn't realize that you applied this already. I'll drop
this patch
in case I need to submit a new revision.

Re: [PATCH v6 08/11] ASoC: Intel: atom: Make PCI dependency explicit

[Sinan Kaya]

On Mon, Jan 7, 2019 at 6:15 AM Rafael J. Wysocki  wrote:
>
> On Sat, Jan 5, 2019 at 11:06 AM Sinan Kaya  wrote:
> >
> > After 'commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without
> > CONFIG_PCI set")' dependencies on CONFIG_PCI that previously were
> > satisfied implicitly through dependencies on CONFIG_ACPI have to be
> > specified directly. This code relies on IOSF_MBI and IOSF_MBI depends
> > on PCI. For this reason, add a direct dependency on CONFIG_PCI to the
> > IOSF_MBI driver.
> >
> > Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI 
> > set")
> > Signed-off-by: Sinan Kaya 
>
> Sinan, I thought you received an ACK from Pierre-Louis on this one, didn't 
> you?
>

Yes, there was an ack assuming that I fixed the ia64 kconfig. Since I
didn't, I took it back.
Later ia64 failure was fixed by another patchset and Pierre-Louis said
"looks good to me". I didn't
assume it was an ack unless explicitly stated.

It might be good if Pierre-Louis acked this version instead.

> > ---
> >  sound/soc/intel/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
> > index 99a62ba409df..bd9fd2035c55 100644
> > --- a/sound/soc/intel/Kconfig
> > +++ b/sound/soc/intel/Kconfig
> > @@ -91,7 +91,7 @@ config SND_SST_ATOM_HIFI2_PLATFORM_PCI
> >  config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
> > tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms"
> > default ACPI
> > -   depends on X86 && ACPI
> > +   depends on X86 && ACPI && PCI
> > select SND_SST_IPC_ACPI
> > select SND_SST_ATOM_HIFI2_PLATFORM
> > select SND_SOC_ACPI_INTEL_MATCH
> > --
>
> Mark, assuming that the ACK above was given here, do you want me to
> take this patch or do you want to take care of it yourself?

[PATCH v6 11/11] drivers: thermal: int340x_thermal: Make PCI dependency explicit

[Sinan Kaya]

After 'commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without
CONFIG_PCI set")' dependencies on CONFIG_PCI that previously were
satisfied implicitly through dependencies on CONFIG_ACPI have to be
specified directly. IOSF_CORE depends on PCI. For this reason, add a
direct dependency on CONFIG_PCI.

Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")
Signed-off-by: Sinan Kaya 
---
 drivers/thermal/intel/int340x_thermal/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/intel/int340x_thermal/Kconfig 
b/drivers/thermal/intel/int340x_thermal/Kconfig
index fba1976d5f8d..2acddf8fb314 100644
--- a/drivers/thermal/intel/int340x_thermal/Kconfig
+++ b/drivers/thermal/intel/int340x_thermal/Kconfig
@@ -4,7 +4,7 @@
 
 config INT340X_THERMAL
tristate "ACPI INT340X thermal drivers"
-   depends on X86 && ACPI
+   depends on X86 && ACPI && PCI
select THERMAL_GOV_USER_SPACE
select ACPI_THERMAL_REL
select ACPI_FAN
-- 
2.19.0

[PATCH v6 06/11] platform/x86: apple-gmux: Make PCI dependency explicit

[Sinan Kaya]

After 'commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without
CONFIG_PCI set")' dependencies on CONFIG_PCI that previously were
satisfied implicitly through dependencies on CONFIG_ACPI have to be
specified directly. This driver depends on the PCI infrastructure but
the dependency has not been explicitly called out.

Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")
Signed-off-by: Sinan Kaya 
Reviewed-by: Andy Shevchenko 
Acked-by: Andy Shevchenko 
---
 drivers/platform/x86/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 7afb96cb1cd6..5e2109c54c7c 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1135,7 +1135,7 @@ config SAMSUNG_Q10
 
 config APPLE_GMUX
tristate "Apple Gmux Driver"
-   depends on ACPI
+   depends on ACPI && PCI
depends on PNP
depends on BACKLIGHT_CLASS_DEVICE
depends on BACKLIGHT_APPLE=n || BACKLIGHT_APPLE
-- 
2.19.0

[PATCH v6 01/11] ACPI / LPSS: Make PCI dependency explicit

[Sinan Kaya]

After 'commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without
CONFIG_PCI set")', it is possible to build ACPI without any PCI support.
This code depends on PCI. Compile only when PCI is present.

Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")
Signed-off-by: Sinan Kaya 
---
 drivers/acpi/Makefile   | 3 ++-
 drivers/acpi/internal.h | 4 
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 7c6afc111d76..bb857421c2e8 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -41,7 +41,8 @@ acpi-y+= ec.o
 acpi-$(CONFIG_ACPI_DOCK)   += dock.o
 acpi-$(CONFIG_PCI) += pci_root.o pci_link.o pci_irq.o
 obj-$(CONFIG_ACPI_MCFG)+= pci_mcfg.o
-acpi-y += acpi_lpss.o acpi_apd.o
+acpi-$(CONFIG_PCI) += acpi_lpss.o
+acpi-y += acpi_apd.o
 acpi-y += acpi_platform.o
 acpi-y += acpi_pnp.o
 acpi-$(CONFIG_ARM_AMBA)+= acpi_amba.o
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 7e6952edb5b0..6a9e1fb8913a 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -81,7 +81,11 @@ void acpi_debugfs_init(void);
 #else
 static inline void acpi_debugfs_init(void) { return; }
 #endif
+#ifdef CONFIG_PCI
 void acpi_lpss_init(void);
+#else
+static inline void acpi_lpss_init(void) {}
+#endif
 
 void acpi_apd_init(void);
 
-- 
2.19.0

[PATCH v6 03/11] vga-switcheroo: make PCI dependency explicit

[Sinan Kaya]

This driver depends on the PCI infrastructure but the dependency has not
been explicitly called out.

Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")
Signed-off-by: Sinan Kaya 
Reviewed-by: Lukas Wunner 
Acked-by: Daniel Vetter 
---
 drivers/gpu/vga/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/vga/Kconfig b/drivers/gpu/vga/Kconfig
index b677e5d524e6..d5f1d8e1c6f8 100644
--- a/drivers/gpu/vga/Kconfig
+++ b/drivers/gpu/vga/Kconfig
@@ -21,6 +21,7 @@ config VGA_SWITCHEROO
bool "Laptop Hybrid Graphics - GPU switching support"
depends on X86
depends on ACPI
+   depends on PCI
select VGA_ARB
help
  Many laptops released in 2008/9/10 have two GPUs with a multiplexer
-- 
2.19.0

[PATCH v6 08/11] ASoC: Intel: atom: Make PCI dependency explicit

[Sinan Kaya]

After 'commit 5d32a66541c4 ("PCI/ACPI: Allow ACPI to be built without
CONFIG_PCI set")' dependencies on CONFIG_PCI that previously were
satisfied implicitly through dependencies on CONFIG_ACPI have to be
specified directly. This code relies on IOSF_MBI and IOSF_MBI depends
on PCI. For this reason, add a direct dependency on CONFIG_PCI to the
IOSF_MBI driver.

Fixes: 5d32a66541c46 ("PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set")
Signed-off-by: Sinan Kaya 
---
 sound/soc/intel/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index 99a62ba409df..bd9fd2035c55 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -91,7 +91,7 @@ config SND_SST_ATOM_HIFI2_PLATFORM_PCI
 config SND_SST_ATOM_HIFI2_PLATFORM_ACPI
tristate "ACPI HiFi2 (Baytrail, Cherrytrail) Platforms"
default ACPI
-   depends on X86 && ACPI
+   depends on X86 && ACPI && PCI
select SND_SST_IPC_ACPI
select SND_SST_ATOM_HIFI2_PLATFORM
select SND_SOC_ACPI_INTEL_MATCH
-- 
2.19.0