RE: [PATCH v10 1/6] drivers/platform/x86/p2sb: New Primary to Sideband bridge support driver for Intel SOC's

-----Original Message-----
From: linux-gpio-owner@vger.kernel.org [mailto:linux-gpio-
owner@vger.kernel.org] On Behalf Of Andy Shevchenko
Sent: Friday, November 11, 2016 12:07 AM
To: Tan, Jui Nee <redacted>; mika.westerberg@linux.intel.com;
heikki.krogerus@linux.intel.com; tglx@linutronix.de; dvhart@infradead.org;
mingo@redhat.com; hpa@zytor.com; x86@kernel.org; ptyser@xes-inc.com;
lee.jones@linaro.org; linus.walleij@linaro.org
Cc: linux-gpio@vger.kernel.org; platform-driver-x86@vger.kernel.org;
linux-kernel@vger.kernel.org; Yong, Jonathan [off-list ref];
Yu, Ong Hock [off-list ref]; Luck, Tony [off-list ref];
Wan Mohamad, Wan Ahmad Zainie [off-list ref];
Sun, Yunying [off-list ref]
Subject: Re: [PATCH v10 1/6] drivers/platform/x86/p2sb: New Primary to
Sideband bridge support driver for Intel SOC's

On Thu, 2016-11-10 at 17:00 +0800, Tan Jui Nee wrote:

quoted

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

There is already one and at least one more user coming which require
an access to Primary to Sideband bridge (P2SB) in order to get IO or
MMIO bar hidden by BIOS.
Create a driver to access P2SB for x86 devices.

Signed-off-by: Yong, Jonathan <redacted>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

quoted

+int p2sb_bar(struct pci_dev *pdev, unsigned int devfn,
+	struct resource *res)
+{
+	u32 base_addr;
+	u64 base64_addr;
+	unsigned long flags;
+

quoted

+	if (!res)
+		return -EINVAL;

I don't remember the details, one version was quite changed, so, I think
these lines are not needed anymore.

Noted, these lines will be removed in next patch version (v12).

quoted

+	/* Get IO or MMIO BAR */
+	pci_bus_read_config_dword(pdev->bus, devfn, SBREG_BAR,
&base_addr);
+	if ((base_addr & PCI_BASE_ADDRESS_SPACE) ==
PCI_BASE_ADDRESS_SPACE_IO) {
+		flags = IORESOURCE_IO;
+		base64_addr = base_addr & PCI_BASE_ADDRESS_IO_MASK;
+	} else {
+		flags = IORESOURCE_MEM;
+		base64_addr = base_addr & PCI_BASE_ADDRESS_MEM_MASK;
+		if (base_addr & PCI_BASE_ADDRESS_MEM_TYPE_64) {
+			flags |= IORESOURCE_MEM_64;

quoted

+			pci_bus_read_config_dword(pdev->bus, devfn,
+				SBREG_BAR + 4, &base_addr);

Fix indentation.

Thanks for pointing that out. I will fix that in next patch version (v12). 

quoted

+			base64_addr |= (u64)base_addr << 32;
+		}
+	}
+
+	/* Hide the P2SB device */
+	pci_bus_write_config_byte(pdev->bus, devfn, SBREG_HIDE,
0x01);
+
+	spin_unlock(&p2sb_spinlock);
+

quoted

+	/* User provides prefilled resources */

Not anymore as far I as I can see. You just return here the result.

Current version is returning status of p2sb_bar function, i.e., 0 on success or appropriate errno value on error. Perhaps you could share the reason of return the result instead of status. 

quoted

+	res->start = (resource_size_t)base64_addr;
+	res->flags = flags;

--
Andy Shevchenko [off-list ref]
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org More majordomo info at
http://vger.kernel.org/majordomo-info.html