Re: [PATCH v3 2/2] PCI: microchip: rework reg region handing to support using either instance 1 or 2

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4693 bytes --]

On Thu, Jul 04, 2024 at 04:01:42AM +0800, kernel test robot wrote:
> Hi Conor,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on pci/for-linus]
> [also build test WARNING on robh/for-next linus/master v6.10-rc6]
> [cannot apply to pci/next next-20240703]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Conor-Dooley/dt-bindings-PCI-microchip-pcie-host-fix-reg-properties/20240701-183154
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git for-linus
> patch link:    https://lore.kernel.org/r/20240701-charger-recognize-fd8a65026a89%40spud
> patch subject: [PATCH v3 2/2] PCI: microchip: rework reg region handing to support using either instance 1 or 2
> config: x86_64-randconfig-102-20240703 (https://download.01.org/0day-ci/archive/20240704/202407040339.i1nFFozS-lkp@intel.com/config)
> compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202407040339.i1nFFozS-lkp@intel.com/
> 
> cocci warnings: (new ones prefixed by >>)
> >> drivers/pci/controller/pcie-microchip-host.c:1142:5-11: inconsistent IS_ERR and PTR_ERR on line 1143.

This is a valid warning and was reported against my patch submission
rather than my kernel.org tree where I had put the patch overnight
before submitting - any reason why it only CCed me and the build issue
mailing list? Seems worth sending to the maintainers etc since it is a
Real Bug^tm.

Also, Julia - any chance of an improved error message here? I was
scratching my head saying wtf cos IS_ERR() and PTR_ERR() are the right
things to use here - missing that the inconsistency was the variable
passed to it. Would something like "inconsistent argument passed to IS_ERR()
and PTR_ERR() on line $" be an improvement here?

Cheers,
Conor.

> 
> vim +1142 drivers/pci/controller/pcie-microchip-host.c
> 
>   1118	
>   1119	static int mc_host_probe(struct platform_device *pdev)
>   1120	{
>   1121		struct device *dev = &pdev->dev;
>   1122		void __iomem *apb_base_addr;
>   1123		int ret;
>   1124		u32 val;
>   1125	
>   1126		port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
>   1127		if (!port)
>   1128			return -ENOMEM;
>   1129	
>   1130		port->dev = dev;
>   1131	
>   1132		port->bridge_base_addr = devm_platform_ioremap_resource_byname(pdev, "bridge");
>   1133		port->ctrl_base_addr = devm_platform_ioremap_resource_byname(pdev, "ctrl");
>   1134		if(!IS_ERR(port->bridge_base_addr) && !IS_ERR(port->ctrl_base_addr))
>   1135			goto addrs_set;
>   1136	
>   1137		/*
>   1138		 * The original, incorrect, binding that lumped the control and
>   1139		 * bridge addresses together still needs to be handled by the driver.
>   1140		 */
>   1141		apb_base_addr = devm_platform_ioremap_resource_byname(pdev, "apb");
> > 1142		if (IS_ERR(apb_base_addr))
> > 1143			return dev_err_probe(dev, PTR_ERR(port->bridge_base_addr),
>   1144					     "both legacy apb register and ctrl/bridge regions missing");
>   1145	
>   1146		port->bridge_base_addr = apb_base_addr + MC_PCIE1_BRIDGE_ADDR;
>   1147		port->ctrl_base_addr = apb_base_addr + MC_PCIE1_CTRL_ADDR;
>   1148	
>   1149	addrs_set:
>   1150		mc_disable_interrupts(port);
>   1151	
>   1152		/* Allow enabling MSI by disabling MSI-X */
>   1153		val = readl(port->bridge_base_addr + PCIE_PCI_IRQ_DW0);
>   1154		val &= ~MSIX_CAP_MASK;
>   1155		writel(val, port->bridge_base_addr + PCIE_PCI_IRQ_DW0);
>   1156	
>   1157		/* Pick num vectors from bitfile programmed onto FPGA fabric */
>   1158		val = readl(port->bridge_base_addr + PCIE_PCI_IRQ_DW0);
>   1159		val &= NUM_MSI_MSGS_MASK;
>   1160		val >>= NUM_MSI_MSGS_SHIFT;
>   1161	
>   1162		port->msi.num_vectors = 1 << val;
>   1163	
>   1164		/* Pick vector address from design */
>   1165		port->msi.vector_phy = readl_relaxed(port->bridge_base_addr + IMSI_ADDR);
>   1166	
>   1167		ret = mc_pcie_init_clks(dev);
>   1168		if (ret) {
>   1169			dev_err(dev, "failed to get clock resources, error %d\n", ret);
>   1170			return -ENODEV;
>   1171		}
>   1172	
>   1173		return pci_host_common_probe(pdev);
>   1174	}
>   1175	
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]