Re: [PATCH] drivers/misc/cxl: Avoid unnecessary error message

[Michael Ellerman <mpe@ellerman.id.au>]

Gavin Shan <gwshan@linux.vnet.ibm.com> writes:

> On Wed, Feb 08, 2017 at 10:39:55AM +1100, Andrew Donnellan wrote:
>>On 08/02/17 10:21, Gavin Shan wrote:
>>>On Tue, Feb 07, 2017 at 10:12:48PM +1100, Michael Ellerman wrote:
>>>>Andrew Donnellan <andrew.donnellan@au1.ibm.com> writes:
>>>
>>>.../...
>>>
>>>>>The effect of this patch is to copy the memory resources from the *real*
>>>>>PHB to the vPHB, as given through the device tree. It shouldn't have any
>>>>>practical effect other than squashing this message.
>>>>
>>>>It sounds a bit backward to me. If we don't need the resources then
>>>>why have them?
>>>>
>>>>If we have code that thinks that's an error, than maybe that's what
>>>>needs fixing, or special casing for the vPHB?
>>>>
>>>
>>>Yeah, vPHB is a special case. There are basically two stages in PCI enumeration:
>>>probing and then resource assignment. vPHB is different from *real* PHB as the
>>>resource assignment is skipped on it. So vPHB doesn't need any resources to be
>>>populated. However, there is a check in probing stage and it's where the warning
>>>message comes from.
>>>
>>>   drivers/misc/cxl/vphb.c::cxl_pci_vphb_add()
>>>   arch/powerpc/kernel/pci-common.c::pcibios_scan_phb()
>>>                                     pcibios_setup_phb_resources()
>>>
>>>   static void pcibios_setup_phb_resources(struct pci_controller *hose,
>>>                                        struct list_head *resources)
>>>   {
>>>       :
>>>       for (i = 0; i < 3; ++i) {
>>>            res = &hose->mem_resources[i];
>>>            if (!res->flags) {
>>>                if (i == 0)
>>>                    printk(KERN_ERR "PCI: Memory resource 0 not set for "
>>>                        "host bridge %s (domain %d)\n",
>>>                        hose->dn->full_name, hose->global_number);
>>>                continue;
>>>            }
>>>       :
>>>   }
>>>
>>>Alternatively, we can replace prink(KERN_ERR) with pr_debug(). It's going to
>>>affect all PHBs including the real ones. Andrew and Michael, what do you think? :-)
>>
>>In what other circumstances do we get this error printed on real PHBs?
>>
>
> When hose->mem_resources[0] isn't built from PHB's device-tree node. It means
> the device-tree node's "ranges" isn't populated correctly by loader and it
> should be very rare ... I never saw it before.

Sounds to me like we could probably just drop the warning. Or make it pr_devel().

cheers