| Message ID | 20221104154916.35231-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [v1,1/1] platform/x86: p2sb: Don't fail if unknown CPU is found | expand |
[Public] > -----Original Message----- > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Sent: Friday, November 4, 2022 10:49 > To: Hans de Goede <hdegoede@redhat.com>; Andy Shevchenko > <andriy.shevchenko@linux.intel.com>; platform-driver- > x86@vger.kernel.org; linux-kernel@vger.kernel.org > Cc: Mark Gross <markgross@kernel.org>; Jarkko Nikula > <jarkko.nikula@linux.intel.com> > Subject: [PATCH v1 1/1] platform/x86: p2sb: Don't fail if unknown CPU is > found > > We have accessing P2SB from a very few places for quite known hardware. > > When a new SoC appears in intel-family.h it's not obvious that it needs > to be added to p2sb.c as well. Instead, provide default BDF and refactor > p2sb_get_devfn() to always succeed. If in the future we would need to > exclude something, we may add a list of unsupported IDs. > > Without this change the iTCO on Intel Commet Lake SoCs became Isn't it "Comet Lake"? > unavailable: > > i801_smbus 0000:00:1f.4: failed to create iTCO device > > Fixes: 5c7b9167ddf8 ("i2c: i801: convert to use common P2SB accessor") > Reported-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/platform/x86/p2sb.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/platform/x86/p2sb.c b/drivers/platform/x86/p2sb.c > index 384d0962ae93..1cf2471d54dd 100644 > --- a/drivers/platform/x86/p2sb.c > +++ b/drivers/platform/x86/p2sb.c > @@ -19,26 +19,23 @@ > #define P2SBC 0xe0 > #define P2SBC_HIDE BIT(8) > > +#define P2SB_DEVFN_DEFAULT PCI_DEVFN(31, 1) > + > 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)), > {} > }; > > static int p2sb_get_devfn(unsigned int *devfn) > { > + unsigned int fn = P2SB_DEVFN_DEFAULT; > const struct x86_cpu_id *id; > > id = x86_match_cpu(p2sb_cpu_ids); > - if (!id) > - return -ENODEV; > + if (id) > + fn = (unsigned int)id->driver_data; > > - *devfn = (unsigned int)id->driver_data; > + *devfn = fn; > return 0; > } > > -- > 2.35.1
On Fri, Nov 04, 2022 at 03:58:22PM +0000, Limonciello, Mario wrote: > > -----Original Message----- > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Sent: Friday, November 4, 2022 10:49 ... > > We have accessing P2SB from a very few places for quite known hardware. > > > > When a new SoC appears in intel-family.h it's not obvious that it needs > > to be added to p2sb.c as well. Instead, provide default BDF and refactor > > p2sb_get_devfn() to always succeed. If in the future we would need to > > exclude something, we may add a list of unsupported IDs. > > > > Without this change the iTCO on Intel Commet Lake SoCs became > > Isn't it "Comet Lake"? Right, thanks! In any case I would like to have Jarkko's Tested-by before proceeding with this. Hence v2 is warranted to appear. > > unavailable: > > > > i801_smbus 0000:00:1f.4: failed to create iTCO device
Hi On 11/4/22 19:22, Andy Shevchenko wrote: > On Fri, Nov 04, 2022 at 03:58:22PM +0000, Limonciello, Mario wrote: >> Isn't it "Comet Lake"? > > Right, thanks! > > In any case I would like to have Jarkko's Tested-by before proceeding with > this. Hence v2 is warranted to appear. > Yes, iTCO registration works again with this patch and is functioning, i.e. able to reset the machine. Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Hi, On 11/4/22 16:49, Andy Shevchenko wrote: > We have accessing P2SB from a very few places for quite known hardware. > > When a new SoC appears in intel-family.h it's not obvious that it needs > to be added to p2sb.c as well. Instead, provide default BDF and refactor > p2sb_get_devfn() to always succeed. If in the future we would need to > exclude something, we may add a list of unsupported IDs. > > Without this change the iTCO on Intel Commet Lake SoCs became unavailable: > > i801_smbus 0000:00:1f.4: failed to create iTCO device > > Fixes: 5c7b9167ddf8 ("i2c: i801: convert to use common P2SB accessor") > Reported-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Thanks. I've merged this now, with the Commet Lake spelling error fixed and with Jarko's Tested-by added. I'll include this in my upcoming fixes pull-req to Linus. Regards, Hans > --- > drivers/platform/x86/p2sb.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/platform/x86/p2sb.c b/drivers/platform/x86/p2sb.c > index 384d0962ae93..1cf2471d54dd 100644 > --- a/drivers/platform/x86/p2sb.c > +++ b/drivers/platform/x86/p2sb.c > @@ -19,26 +19,23 @@ > #define P2SBC 0xe0 > #define P2SBC_HIDE BIT(8) > > +#define P2SB_DEVFN_DEFAULT PCI_DEVFN(31, 1) > + > 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)), > {} > }; > > static int p2sb_get_devfn(unsigned int *devfn) > { > + unsigned int fn = P2SB_DEVFN_DEFAULT; > const struct x86_cpu_id *id; > > id = x86_match_cpu(p2sb_cpu_ids); > - if (!id) > - return -ENODEV; > + if (id) > + fn = (unsigned int)id->driver_data; > > - *devfn = (unsigned int)id->driver_data; > + *devfn = fn; > return 0; > } >
diff --git a/drivers/platform/x86/p2sb.c b/drivers/platform/x86/p2sb.c index 384d0962ae93..1cf2471d54dd 100644 --- a/drivers/platform/x86/p2sb.c +++ b/drivers/platform/x86/p2sb.c @@ -19,26 +19,23 @@ #define P2SBC 0xe0 #define P2SBC_HIDE BIT(8) +#define P2SB_DEVFN_DEFAULT PCI_DEVFN(31, 1) + 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)), {} }; static int p2sb_get_devfn(unsigned int *devfn) { + unsigned int fn = P2SB_DEVFN_DEFAULT; const struct x86_cpu_id *id; id = x86_match_cpu(p2sb_cpu_ids); - if (!id) - return -ENODEV; + if (id) + fn = (unsigned int)id->driver_data; - *devfn = (unsigned int)id->driver_data; + *devfn = fn; return 0; }
We have accessing P2SB from a very few places for quite known hardware. When a new SoC appears in intel-family.h it's not obvious that it needs to be added to p2sb.c as well. Instead, provide default BDF and refactor p2sb_get_devfn() to always succeed. If in the future we would need to exclude something, we may add a list of unsupported IDs. Without this change the iTCO on Intel Commet Lake SoCs became unavailable: i801_smbus 0000:00:1f.4: failed to create iTCO device Fixes: 5c7b9167ddf8 ("i2c: i801: convert to use common P2SB accessor") Reported-by: Jarkko Nikula <jarkko.nikula@linux.intel.com> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/platform/x86/p2sb.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)