From: Jean Delvare <jdelvare@suse.de>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>,
Lee Jones <lee.jones@linaro.org>,
Tan Jui Nee <jui.nee.tan@intel.com>,
Jim Quinlan <james.quinlan@broadcom.com>,
Jonathan Yong <jonathan.yong@intel.com>,
Bjorn Helgaas <bhelgaas@google.com>,
linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org,
linux-pci@vger.kernel.org, Peter Tyser <ptyser@xes-inc.com>,
hdegoede@redhat.com, henning.schild@siemens.com
Subject: Re: [PATCH v1 7/7] i2c: i801: convert to use common P2SB accessor
Date: Wed, 10 Mar 2021 15:51:45 +0100 [thread overview]
Message-ID: <20210310155145.513a7165@endymion> (raw)
In-Reply-To: <20210308122020.57071-8-andriy.shevchenko@linux.intel.com>
Hi Andy,
On Mon, 8 Mar 2021 14:20:20 +0200, Andy Shevchenko wrote:
> Since we have a common P2SB accessor in tree we may use it instead of
> open coded variants.
>
> Replace custom code by pci_p2sb_bar() call.
I like the idea. Just two things...
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/i2c/busses/Kconfig | 1 +
> drivers/i2c/busses/i2c-i801.c | 40 ++++++++---------------------------
> drivers/pci/pci-p2sb.c | 6 ++++++
> 3 files changed, 16 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 05ebf7546e3f..ffd3007f888c 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -101,6 +101,7 @@ config I2C_HIX5HD2
> config I2C_I801
> tristate "Intel 82801 (ICH/PCH)"
> depends on PCI
> + select PCI_P2SB if X86
> select CHECK_SIGNATURE if X86 && DMI
> select I2C_SMBUS
> help
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index 4acee6f9e5a3..23b43de9786a 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -90,6 +90,7 @@
> #include <linux/interrupt.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> +#include <linux/pci-p2sb.h>
> #include <linux/kernel.h>
> #include <linux/stddef.h>
> #include <linux/delay.h>
> @@ -136,7 +137,6 @@
> #define TCOBASE 0x050
> #define TCOCTL 0x054
>
> -#define SBREG_BAR 0x10
> #define SBREG_SMBCTRL 0xc6000c
> #define SBREG_SMBCTRL_DNV 0xcf000c
>
> @@ -1524,52 +1524,30 @@ static const struct itco_wdt_platform_data spt_tco_platform_data = {
> .version = 4,
> };
>
> -static DEFINE_SPINLOCK(p2sb_spinlock);
> -
> static struct platform_device *
> i801_add_tco_spt(struct i801_priv *priv, struct pci_dev *pci_dev,
> struct resource *tco_res)
> {
> struct resource *res;
> unsigned int devfn;
> - u64 base64_addr;
> - u32 base_addr;
> - u8 hidden;
> + int ret;
>
> /*
> * We must access the NO_REBOOT bit over the Primary to Sideband
> - * bridge (P2SB). The BIOS prevents the P2SB device from being
> - * enumerated by the PCI subsystem, so we need to unhide/hide it
> - * to lookup the P2SB BAR.
> + * bridge (P2SB).
> */
> - spin_lock(&p2sb_spinlock);
>
> devfn = PCI_DEVFN(PCI_SLOT(pci_dev->devfn), 1);
>
> - /* Unhide the P2SB device, if it is hidden */
> - pci_bus_read_config_byte(pci_dev->bus, devfn, 0xe1, &hidden);
> - if (hidden)
> - pci_bus_write_config_byte(pci_dev->bus, devfn, 0xe1, 0x0);
> -
> - pci_bus_read_config_dword(pci_dev->bus, devfn, SBREG_BAR, &base_addr);
> - base64_addr = base_addr & 0xfffffff0;
> -
> - pci_bus_read_config_dword(pci_dev->bus, devfn, SBREG_BAR + 0x4, &base_addr);
> - base64_addr |= (u64)base_addr << 32;
> -
> - /* Hide the P2SB device, if it was hidden before */
> - if (hidden)
> - pci_bus_write_config_byte(pci_dev->bus, devfn, 0xe1, hidden);
> - spin_unlock(&p2sb_spinlock);
> -
> res = &tco_res[1];
> + ret = pci_p2sb_bar(pci_dev, devfn, res);
> + if (ret)
> + return ERR_PTR(ret);
> +
> if (pci_dev->device == PCI_DEVICE_ID_INTEL_DNV_SMBUS)
> - res->start = (resource_size_t)base64_addr + SBREG_SMBCTRL_DNV;
> + res->start += SBREG_SMBCTRL_DNV;
> else
> - res->start = (resource_size_t)base64_addr + SBREG_SMBCTRL;
> -
> - res->end = res->start + 3;
> - res->flags = IORESOURCE_MEM;
> + res->start += SBREG_SMBCTRL;
I can't see why you no longer set res->end and res->flags here. I can
imagine that pci_p2sb_bar() may have set the flags for us, but not that
->end is still correct after you fixed up ->start. Am I missing
something?
>
> return platform_device_register_resndata(&pci_dev->dev, "iTCO_wdt", -1,
> tco_res, 2, &spt_tco_platform_data,
> diff --git a/drivers/pci/pci-p2sb.c b/drivers/pci/pci-p2sb.c
> index 68d7dad48cdb..7f6bc7d4482a 100644
> --- a/drivers/pci/pci-p2sb.c
> +++ b/drivers/pci/pci-p2sb.c
> @@ -22,6 +22,12 @@
>
> static const struct x86_cpu_id p2sb_cpu_ids[] = {
> X86_MATCH_INTEL_FAM6_MODEL(ATOM_GOLDMONT, PCI_DEVFN(13, 0)),
> + X86_MATCH_INTEL_FAM6_MODEL(ATOM_GOLDMONT_D, PCI_DEVFN(31, 1)),
> + X86_MATCH_INTEL_FAM6_MODEL(ATOM_SILVERMONT_D, PCI_DEVFN(31, 1)),
> + X86_MATCH_INTEL_FAM6_MODEL(KABYLAKE, PCI_DEVFN(31, 1)),
> + X86_MATCH_INTEL_FAM6_MODEL(KABYLAKE_L, PCI_DEVFN(31, 1)),
> + X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE, PCI_DEVFN(31, 1)),
> + X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_L, PCI_DEVFN(31, 1)),
> {}
> };
>
Any reason why this is added in this patch instead of [3/7] (PCI: New
Primary to Sideband (P2SB) bridge support library)?
--
Jean Delvare
SUSE L3 Support
next prev parent reply other threads:[~2021-03-10 14:52 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-08 12:20 [rfc, PATCH v1 0/7] PCI: introduce p2sb helper Andy Shevchenko
2021-03-08 12:20 ` [PATCH v1 1/7] PCI: Introduce pci_bus_*() printing macros when device is not available Andy Shevchenko
2021-03-10 14:57 ` Jean Delvare
2021-03-08 12:20 ` [PATCH v1 2/7] PCI: Convert __pci_read_base() to __pci_bus_read_base() Andy Shevchenko
2021-03-10 17:14 ` Christoph Hellwig
2021-03-08 12:20 ` [PATCH v1 3/7] PCI: New Primary to Sideband (P2SB) bridge support library Andy Shevchenko
2021-03-08 18:52 ` Bjorn Helgaas
2021-03-08 19:16 ` Andy Shevchenko
2021-03-09 1:42 ` Bjorn Helgaas
2021-03-09 8:42 ` Henning Schild
2021-04-01 15:45 ` Andy Shevchenko
2021-04-01 16:42 ` Bjorn Helgaas
2021-04-01 18:23 ` Andy Shevchenko
2021-04-01 18:44 ` Bjorn Helgaas
2021-07-12 12:15 ` Andy Shevchenko
2021-11-26 15:10 ` Andy Shevchenko
2021-04-02 13:09 ` Enrico Weigelt, metux IT consult
2021-04-06 13:40 ` Henning Schild
2021-07-12 12:11 ` Andy Shevchenko
2021-11-26 15:38 ` Andy Shevchenko
2021-11-29 21:07 ` Bjorn Helgaas
2021-12-08 17:51 ` Andy Shevchenko
2021-03-13 9:45 ` Henning Schild
2021-04-01 15:43 ` Andy Shevchenko
2021-04-01 18:06 ` Mika Westerberg
2021-04-01 18:22 ` Andy Shevchenko
2021-04-01 18:32 ` Mika Westerberg
2021-07-12 12:13 ` Andy Shevchenko
2021-03-08 12:20 ` [PATCH v1 4/7] mfd: lpc_ich: Factor out lpc_ich_enable_spi_write() Andy Shevchenko
2021-03-10 10:29 ` Lee Jones
2021-03-08 12:20 ` [PATCH v1 5/7] mfd: lpc_ich: Switch to generic pci_p2sb_bar() Andy Shevchenko
2021-03-10 10:35 ` Lee Jones
2021-03-10 12:05 ` Andy Shevchenko
2021-03-10 12:57 ` Lee Jones
2021-03-08 12:20 ` [PATCH v1 6/7] mfd: lpc_ich: Add support for pinctrl in non-ACPI system Andy Shevchenko
2021-03-10 10:27 ` Lee Jones
2021-04-12 16:01 ` Henning Schild
2021-04-12 16:40 ` Henning Schild
2021-04-12 16:59 ` Andy Shevchenko
2021-04-12 17:16 ` Henning Schild
2021-04-12 17:34 ` Andy Shevchenko
2021-04-13 6:47 ` Henning Schild
2021-04-12 16:51 ` Andy Shevchenko
2021-04-12 17:27 ` Henning Schild
2021-04-12 17:41 ` Andy Shevchenko
2021-03-08 12:20 ` [PATCH v1 7/7] i2c: i801: convert to use common P2SB accessor Andy Shevchenko
2021-03-10 14:51 ` Jean Delvare [this message]
2021-12-21 15:08 ` Andy Shevchenko
2021-03-13 9:25 ` [rfc, PATCH v1 0/7] PCI: introduce p2sb helper Henning Schild
2021-06-10 9:02 ` Henning Schild
2021-06-10 10:14 ` Andy Shevchenko
2021-06-10 13:48 ` Henning Schild
2021-06-10 14:04 ` Andy Shevchenko
2021-11-26 15:43 ` Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210310155145.513a7165@endymion \
--to=jdelvare@suse.de \
--cc=andriy.shevchenko@linux.intel.com \
--cc=bhelgaas@google.com \
--cc=hdegoede@redhat.com \
--cc=henning.schild@siemens.com \
--cc=james.quinlan@broadcom.com \
--cc=jonathan.yong@intel.com \
--cc=jui.nee.tan@intel.com \
--cc=lee.jones@linaro.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=ptyser@xes-inc.com \
--cc=wsa+renesas@sang-engineering.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).