| Message ID | 20250410200202.2974062-3-superm1@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | AMD Zen debugging documentation | expand |
On Thu, Apr 10, 2025 at 03:02:00PM -0500, Mario Limonciello wrote: > From: Mario Limonciello <mario.limonciello@amd.com> > > SB800_PIIX4_FCH_PM_ADDR is used to indicate the base address for the > FCH PM registers. Multiple drivers may need this base address, so > move it to a common header location and rename accordingly. > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > arch/x86/include/asm/amd_node.h | 2 ++ > drivers/i2c/busses/i2c-piix4.c | 12 ++++++------ > 2 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/include/asm/amd_node.h b/arch/x86/include/asm/amd_node.h > index 23fe617898a8f..f4993201834ea 100644 > --- a/arch/x86/include/asm/amd_node.h > +++ b/arch/x86/include/asm/amd_node.h > @@ -19,6 +19,8 @@ > > #include <linux/pci.h> > > +#define FCH_PM_BASE 0xFED80300 Is that even related to amd_node? Or should it be in some x86...platform.h header?
On 4/11/25 06:49, Borislav Petkov wrote: > On Thu, Apr 10, 2025 at 03:02:00PM -0500, Mario Limonciello wrote: >> From: Mario Limonciello <mario.limonciello@amd.com> >> >> SB800_PIIX4_FCH_PM_ADDR is used to indicate the base address for the >> FCH PM registers. Multiple drivers may need this base address, so >> move it to a common header location and rename accordingly. >> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- >> arch/x86/include/asm/amd_node.h | 2 ++ >> drivers/i2c/busses/i2c-piix4.c | 12 ++++++------ >> 2 files changed, 8 insertions(+), 6 deletions(-) >> >> diff --git a/arch/x86/include/asm/amd_node.h b/arch/x86/include/asm/amd_node.h >> index 23fe617898a8f..f4993201834ea 100644 >> --- a/arch/x86/include/asm/amd_node.h >> +++ b/arch/x86/include/asm/amd_node.h >> @@ -19,6 +19,8 @@ >> >> #include <linux/pci.h> >> >> +#define FCH_PM_BASE 0xFED80300 > > Is that even related to amd_node? > > Or should it be in some x86...platform.h header? > I was aiming for a header that we would conceivably use in all these places anyway. Can you suggest a more fitting existing header? A new one felt too heavy for a single register define.
On Fri, Apr 11, 2025 at 07:09:56AM -0500, Mario Limonciello wrote: > I was aiming for a header that we would conceivably use in all these places > anyway. > > Can you suggest a more fitting existing header? A new one felt too heavy > for a single register define. No, the logic is: put it in the *right* header. Not in the "whatever-works" header. So you can easily add a arch/x86/include/asm/platform.h header which contains exactly platform stuff. And FCH sounds like a platform thing to me. Or at least southbridge or whatever that thing is called now. It certainly ain't part of the CPU so platform should be more fitting. Unless someone has a better idea...
Hi Mario, kernel test robot noticed the following build errors: [auto build test ERROR on tip/x86/core] [also build test ERROR on andi-shyti/i2c/i2c-host tip/master linus/master v6.15-rc1 next-20250411] [cannot apply to bp/for-next tip/auto-latest] [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/Mario-Limonciello/Documentation-Add-AMD-Zen-debugging-document/20250411-040641 base: tip/x86/core patch link: https://lore.kernel.org/r/20250410200202.2974062-3-superm1%40kernel.org patch subject: [PATCH v3 2/4] i2c: piix4: Move SB800_PIIX4_FCH_PM_ADDR definition to amd_node.h config: arm-randconfig-001-20250412 (https://download.01.org/0day-ci/archive/20250412/202504120432.0F8lOF3k-lkp@intel.com/config) compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250412/202504120432.0F8lOF3k-lkp@intel.com/reproduce) 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/202504120432.0F8lOF3k-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/i2c/busses/i2c-piix4.c:24:10: fatal error: 'asm/amd_node.h' file not found 24 | #include <asm/amd_node.h> | ^~~~~~~~~~~~~~~~ 1 error generated. vim +24 drivers/i2c/busses/i2c-piix4.c 23 > 24 #include <asm/amd_node.h> 25 #include <linux/module.h> 26 #include <linux/moduleparam.h> 27 #include <linux/pci.h> 28 #include <linux/kernel.h> 29 #include <linux/delay.h> 30 #include <linux/stddef.h> 31 #include <linux/ioport.h> 32 #include <linux/i2c.h> 33 #include <linux/i2c-smbus.h> 34 #include <linux/slab.h> 35 #include <linux/dmi.h> 36 #include <linux/acpi.h> 37 #include <linux/io.h> 38
Hi Mario, kernel test robot noticed the following build errors: [auto build test ERROR on tip/x86/core] [also build test ERROR on andi-shyti/i2c/i2c-host tip/master linus/master v6.15-rc1 next-20250411] [cannot apply to bp/for-next tip/auto-latest] [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/Mario-Limonciello/Documentation-Add-AMD-Zen-debugging-document/20250411-040641 base: tip/x86/core patch link: https://lore.kernel.org/r/20250410200202.2974062-3-superm1%40kernel.org patch subject: [PATCH v3 2/4] i2c: piix4: Move SB800_PIIX4_FCH_PM_ADDR definition to amd_node.h config: arm64-randconfig-003-20250412 (https://download.01.org/0day-ci/archive/20250412/202504120558.sq3IpWdH-lkp@intel.com/config) compiler: aarch64-linux-gcc (GCC) 8.5.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250412/202504120558.sq3IpWdH-lkp@intel.com/reproduce) 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/202504120558.sq3IpWdH-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/i2c/busses/i2c-piix4.c:24:10: fatal error: asm/amd_node.h: No such file or directory #include <asm/amd_node.h> ^~~~~~~~~~~~~~~~ compilation terminated. Kconfig warnings: (for reference only) WARNING: unmet direct dependencies detected for FB_IOMEM_HELPERS Depends on [n]: HAS_IOMEM [=y] && FB_CORE [=n] Selected by [m]: - DRM_XE_DISPLAY [=y] && HAS_IOMEM [=y] && DRM [=y] && DRM_XE [=m] && DRM_XE [=m]=m [=m] && HAS_IOPORT [=y] vim +24 drivers/i2c/busses/i2c-piix4.c 23 > 24 #include <asm/amd_node.h> 25 #include <linux/module.h> 26 #include <linux/moduleparam.h> 27 #include <linux/pci.h> 28 #include <linux/kernel.h> 29 #include <linux/delay.h> 30 #include <linux/stddef.h> 31 #include <linux/ioport.h> 32 #include <linux/i2c.h> 33 #include <linux/i2c-smbus.h> 34 #include <linux/slab.h> 35 #include <linux/dmi.h> 36 #include <linux/acpi.h> 37 #include <linux/io.h> 38
* Borislav Petkov <bp@alien8.de> wrote: > On Fri, Apr 11, 2025 at 07:09:56AM -0500, Mario Limonciello wrote: > > I was aiming for a header that we would conceivably use in all these places > > anyway. > > > > Can you suggest a more fitting existing header? A new one felt too heavy > > for a single register define. > > No, the logic is: put it in the *right* header. Not in the "whatever-works" > header. Yeah, it's the Linux kernel equivalent of: 'if you touch it, you own it', a.k.a. 'no good deed goes unpunished'. ;-) > So you can easily add a > > arch/x86/include/asm/platform.h > > header which contains exactly platform stuff. And FCH sounds like a platform > thing to me. Or at least southbridge or whatever that thing is called now. It > certainly ain't part of the CPU so platform should be more fitting. > > Unless someone has a better idea... Yeah, so I think we can create a brand new <asm/amd_sb.h> header or so, because it's an AMD SB800 southbridge chipset register? We already have <asm/amd_nb.h>. 'platform' might be a bit too generic and fungible I think: often the northbridge and the CPU is considered part of a 'platform' too. Thanks, Ingo
On 4/12/25 14:44, Ingo Molnar wrote: > > * Borislav Petkov <bp@alien8.de> wrote: > >> On Fri, Apr 11, 2025 at 07:09:56AM -0500, Mario Limonciello wrote: >>> I was aiming for a header that we would conceivably use in all these places >>> anyway. >>> >>> Can you suggest a more fitting existing header? A new one felt too heavy >>> for a single register define. >> >> No, the logic is: put it in the *right* header. Not in the "whatever-works" >> header. > > Yeah, it's the Linux kernel equivalent of: 'if you touch it, you own it', > a.k.a. 'no good deed goes unpunished'. ;-) > Ya. >> So you can easily add a >> >> arch/x86/include/asm/platform.h >> >> header which contains exactly platform stuff. And FCH sounds like a platform >> thing to me. Or at least southbridge or whatever that thing is called now. It >> certainly ain't part of the CPU so platform should be more fitting. >> >> Unless someone has a better idea... > > Yeah, so I think we can create a brand new <asm/amd_sb.h> header or so, > because it's an AMD SB800 southbridge chipset register? We already have > <asm/amd_nb.h>. > > 'platform' might be a bit too generic and fungible I think: often the > northbridge and the CPU is considered part of a 'platform' too. > > Thanks, > > Ingo SB800 is pre-Zen stuff. It's "before my time" - I guess that's the precursor to FCH being in the SoC but has the same functionality. So I'm thinking <asm/amd_fch.h>.
* Mario Limonciello <superm1@kernel.org> wrote: > SB800 is pre-Zen stuff. It's "before my time" - I guess that's the > precursor to FCH being in the SoC but has the same functionality. > > So I'm thinking <asm/amd_fch.h>. I went by the SB800_PIIX4_FCH_PM_ADDR name, which is a misnomer these days? But yeah, <asm/amd_fch.h> sounds good to me too. Boris? and ... I'm sure you knew this was coming, but we should probably move *all* basic FCH_PM definitions into that header, such as SB800_PIIX4_FCH_PM_SIZE, and rename it to FCH_PM_SIZE or so? Thanks, Ingo
On 4/12/25 15:15, Ingo Molnar wrote: > > * Mario Limonciello <superm1@kernel.org> wrote: > >> SB800 is pre-Zen stuff. It's "before my time" - I guess that's the >> precursor to FCH being in the SoC but has the same functionality. >> >> So I'm thinking <asm/amd_fch.h>. > > I went by the SB800_PIIX4_FCH_PM_ADDR name, which is a misnomer these > days? > > But yeah, <asm/amd_fch.h> sounds good to me too. OK. > Boris? > > and ... I'm sure you knew this was coming, but we should probably move > *all* basic FCH_PM definitions into that header, such as > SB800_PIIX4_FCH_PM_SIZE, and rename it to FCH_PM_SIZE or so? I'll double check how it's actually used against the documentation to see if this makes sense. If it's only mapping a subset of registers for the PIIX4 driver use bringing the definition out to a header used by other drivers that might need a larger or smaller subset to be mapped might not make sense.
* Mario Limonciello <superm1@kernel.org> wrote: > > and ... I'm sure you knew this was coming, but we should probably > > move *all* basic FCH_PM definitions into that header, such as > > SB800_PIIX4_FCH_PM_SIZE, and rename it to FCH_PM_SIZE or so? > > I'll double check how it's actually used against the documentation to > see if this makes sense. > > If it's only mapping a subset of registers for the PIIX4 driver use > bringing the definition out to a header used by other drivers that > might need a larger or smaller subset to be mapped might not make > sense. Sounds good to me! Ingo
On April 12, 2025 10:15:27 PM GMT+02:00, Ingo Molnar <mingo@kernel.org> wrote: > >* Mario Limonciello <superm1@kernel.org> wrote: > >> SB800 is pre-Zen stuff. It's "before my time" - I guess that's the >> precursor to FCH being in the SoC but has the same functionality. >> >> So I'm thinking <asm/amd_fch.h>. > >I went by the SB800_PIIX4_FCH_PM_ADDR name, which is a misnomer these >days? > >But yeah, <asm/amd_fch.h> sounds good to me too. Boris? I was aiming more for a header which contains non-CPU defines - i.e., platform. But the FCH is only one part of that platform. But let's start with amd/fch.h - "amd/" subpath element would allow us to trivially put other headers there too - and see where it gets us. We can (and will) always refactor later if needed... Thx.
* Borislav Petkov <bp@alien8.de> wrote: > On April 12, 2025 10:15:27 PM GMT+02:00, Ingo Molnar <mingo@kernel.org> wrote: > > > >* Mario Limonciello <superm1@kernel.org> wrote: > > > >> SB800 is pre-Zen stuff. It's "before my time" - I guess that's the > >> precursor to FCH being in the SoC but has the same functionality. > >> > >> So I'm thinking <asm/amd_fch.h>. > > > >I went by the SB800_PIIX4_FCH_PM_ADDR name, which is a misnomer these > >days? > > > >But yeah, <asm/amd_fch.h> sounds good to me too. Boris? > > I was aiming more for a header which contains non-CPU defines - i.e., > platform. But the FCH is only one part of that platform. But let's > start with amd/fch.h - "amd/" subpath element would allow us to > trivially put other headers there too - and see where it gets us. We > can (and will) always refactor later if needed... Yeah, agreed on opening the <asm/amd/> namespace for this. Thanks, Ingo
* Ingo Molnar <mingo@kernel.org> wrote: > > * Borislav Petkov <bp@alien8.de> wrote: > > > I was aiming more for a header which contains non-CPU defines - > > i.e., platform. But the FCH is only one part of that platform. But > > let's start with amd/fch.h - "amd/" subpath element would allow us > > to trivially put other headers there too - and see where it gets > > us. We can (and will) always refactor later if needed... > > Yeah, agreed on opening the <asm/amd/> namespace for this. Here's a tree that establishes <asm/amd/> and moves existing headers there: git://git.kernel.org/pub/scm/linux/kernel/git/mingo/tip.git WIP.x86/platform Mario, could you base your series on top of this tree? Thanks, Ingo ===============> Ingo Molnar (6): x86/platform/amd: Move the <asm/amd-ibs.h> header to <asm/amd/ibs.h> x86/platform/amd: Add standard header guards to <asm/amd/ibs.h> x86/platform/amd: Move the <asm/amd_nb.h> header to <asm/amd/nb.h> x86/platform/amd: Move the <asm/amd_hsmp.h> header to <asm/amd/hsmp.h> x86/platform/amd: Clean up the <asm/amd/hsmp.h> header guards a bit x86/platform/amd: Move the <asm/amd_node.h> header to <asm/amd/node.h> Documentation/userspace-api/ioctl/ioctl-number.rst | 2 +- MAINTAINERS | 6 +++--- arch/x86/events/amd/ibs.c | 2 +- arch/x86/include/asm/{amd_hsmp.h => amd/hsmp.h} | 4 ++-- arch/x86/include/asm/{amd-ibs.h => amd/ibs.h} | 5 +++++ arch/x86/include/asm/{amd_nb.h => amd/nb.h} | 2 +- arch/x86/include/asm/{amd_node.h => amd/node.h} | 0 arch/x86/kernel/amd_gart_64.c | 2 +- arch/x86/kernel/amd_nb.c | 2 +- arch/x86/kernel/amd_node.c | 2 +- arch/x86/kernel/aperture_64.c | 2 +- arch/x86/kernel/cpu/cacheinfo.c | 2 +- arch/x86/kernel/cpu/mce/inject.c | 2 +- arch/x86/mm/amdtopology.c | 2 +- arch/x86/mm/numa.c | 2 +- arch/x86/pci/amd_bus.c | 2 +- arch/x86/pci/fixup.c | 2 +- drivers/char/agp/amd64-agp.c | 2 +- drivers/edac/amd64_edac.c | 4 ++-- drivers/hwmon/k10temp.c | 2 +- drivers/platform/x86/amd/hsmp/acpi.c | 4 ++-- drivers/platform/x86/amd/hsmp/hsmp.c | 2 +- drivers/platform/x86/amd/hsmp/plat.c | 4 ++-- drivers/platform/x86/amd/pmc/mp1_stb.c | 2 +- drivers/platform/x86/amd/pmc/pmc.c | 2 +- drivers/platform/x86/amd/pmf/core.c | 2 +- drivers/pnp/quirks.c | 2 +- drivers/ras/amd/atl/internal.h | 4 ++-- sound/soc/amd/acp/acp-rembrandt.c | 2 +- sound/soc/amd/acp/acp63.c | 2 +- sound/soc/amd/acp/acp70.c | 2 +- sound/soc/sof/amd/acp.c | 2 +- tools/perf/check-headers.sh | 2 +- tools/perf/util/amd-sample-raw.c | 2 +- 34 files changed, 44 insertions(+), 39 deletions(-) rename arch/x86/include/asm/{amd_hsmp.h => amd/hsmp.h} (91%) rename arch/x86/include/asm/{amd-ibs.h => amd/ibs.h} (98%) rename arch/x86/include/asm/{amd_nb.h => amd/nb.h} (98%) rename arch/x86/include/asm/{amd_node.h => amd/node.h} (100%)
On 4/13/25 03:44, Ingo Molnar wrote: > > * Ingo Molnar <mingo@kernel.org> wrote: > >> >> * Borislav Petkov <bp@alien8.de> wrote: >> >>> I was aiming more for a header which contains non-CPU defines - >>> i.e., platform. But the FCH is only one part of that platform. But >>> let's start with amd/fch.h - "amd/" subpath element would allow us >>> to trivially put other headers there too - and see where it gets >>> us. We can (and will) always refactor later if needed... >> >> Yeah, agreed on opening the <asm/amd/> namespace for this. > > Here's a tree that establishes <asm/amd/> and moves existing headers > there: > > git://git.kernel.org/pub/scm/linux/kernel/git/mingo/tip.git WIP.x86/platform > > Mario, could you base your series on top of this tree? > Sure. One problem that I notice though is that by using <asm/amd/fch.h> that drivers/i2c/busses/i2c-piix4.c has some compile failures on non-x86. This is the same problem that lkp robot raised for v3 when it was on <asm/amd_node.h>: https://lore.kernel.org/linux-doc/20250410200202.2974062-1-superm1@kernel.org/T/#mc33abfcdc434c85fbc94d03498759db631efcf53 What's the preferred way to handle it? The three ways that immediately jump out to me: 1) Add '#if CONFIG_X86' around all related code. 2) Move applicable code to drivers/i2c/busses/i2c-amd-fch.c (similar to how we have i2c-amd-asf-plat.c) but modify drivers/i2c/busses/Makefile to only compile it for x86. 3) Idea two but also add a new Kconfig for CONFIG_I2C_AMD_FCH that depends on CONFIG_X86. I am /leaning/ on the refactor with idea 3. > Thanks, > > Ingo > > ===============> > Ingo Molnar (6): > x86/platform/amd: Move the <asm/amd-ibs.h> header to <asm/amd/ibs.h> > x86/platform/amd: Add standard header guards to <asm/amd/ibs.h> > x86/platform/amd: Move the <asm/amd_nb.h> header to <asm/amd/nb.h> > x86/platform/amd: Move the <asm/amd_hsmp.h> header to <asm/amd/hsmp.h> > x86/platform/amd: Clean up the <asm/amd/hsmp.h> header guards a bit > x86/platform/amd: Move the <asm/amd_node.h> header to <asm/amd/node.h> > > Documentation/userspace-api/ioctl/ioctl-number.rst | 2 +- > MAINTAINERS | 6 +++--- > arch/x86/events/amd/ibs.c | 2 +- > arch/x86/include/asm/{amd_hsmp.h => amd/hsmp.h} | 4 ++-- > arch/x86/include/asm/{amd-ibs.h => amd/ibs.h} | 5 +++++ > arch/x86/include/asm/{amd_nb.h => amd/nb.h} | 2 +- > arch/x86/include/asm/{amd_node.h => amd/node.h} | 0 > arch/x86/kernel/amd_gart_64.c | 2 +- > arch/x86/kernel/amd_nb.c | 2 +- > arch/x86/kernel/amd_node.c | 2 +- > arch/x86/kernel/aperture_64.c | 2 +- > arch/x86/kernel/cpu/cacheinfo.c | 2 +- > arch/x86/kernel/cpu/mce/inject.c | 2 +- > arch/x86/mm/amdtopology.c | 2 +- > arch/x86/mm/numa.c | 2 +- > arch/x86/pci/amd_bus.c | 2 +- > arch/x86/pci/fixup.c | 2 +- > drivers/char/agp/amd64-agp.c | 2 +- > drivers/edac/amd64_edac.c | 4 ++-- > drivers/hwmon/k10temp.c | 2 +- > drivers/platform/x86/amd/hsmp/acpi.c | 4 ++-- > drivers/platform/x86/amd/hsmp/hsmp.c | 2 +- > drivers/platform/x86/amd/hsmp/plat.c | 4 ++-- > drivers/platform/x86/amd/pmc/mp1_stb.c | 2 +- > drivers/platform/x86/amd/pmc/pmc.c | 2 +- > drivers/platform/x86/amd/pmf/core.c | 2 +- > drivers/pnp/quirks.c | 2 +- > drivers/ras/amd/atl/internal.h | 4 ++-- > sound/soc/amd/acp/acp-rembrandt.c | 2 +- > sound/soc/amd/acp/acp63.c | 2 +- > sound/soc/amd/acp/acp70.c | 2 +- > sound/soc/sof/amd/acp.c | 2 +- > tools/perf/check-headers.sh | 2 +- > tools/perf/util/amd-sample-raw.c | 2 +- > 34 files changed, 44 insertions(+), 39 deletions(-) > rename arch/x86/include/asm/{amd_hsmp.h => amd/hsmp.h} (91%) > rename arch/x86/include/asm/{amd-ibs.h => amd/ibs.h} (98%) > rename arch/x86/include/asm/{amd_nb.h => amd/nb.h} (98%) > rename arch/x86/include/asm/{amd_node.h => amd/node.h} (100%) >
* Mario Limonciello <superm1@kernel.org> wrote: > > > On 4/13/25 03:44, Ingo Molnar wrote: > > > > * Ingo Molnar <mingo@kernel.org> wrote: > > > > > > > > * Borislav Petkov <bp@alien8.de> wrote: > > > > > > > I was aiming more for a header which contains non-CPU defines - > > > > i.e., platform. But the FCH is only one part of that platform. But > > > > let's start with amd/fch.h - "amd/" subpath element would allow us > > > > to trivially put other headers there too - and see where it gets > > > > us. We can (and will) always refactor later if needed... > > > > > > Yeah, agreed on opening the <asm/amd/> namespace for this. > > > > Here's a tree that establishes <asm/amd/> and moves existing headers > > there: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/mingo/tip.git WIP.x86/platform > > > > Mario, could you base your series on top of this tree? > > > > Sure. > > One problem that I notice though is that by using <asm/amd/fch.h> that > drivers/i2c/busses/i2c-piix4.c has some compile failures on non-x86. Hm, should these I2C drivers even be built on non-x86 systems? > 1) Add '#if CONFIG_X86' around all related code. > > 2) Move applicable code to drivers/i2c/busses/i2c-amd-fch.c (similar to how > we have i2c-amd-asf-plat.c) but modify drivers/i2c/busses/Makefile to only > compile it for x86. > > 3) Idea two but also add a new Kconfig for CONFIG_I2C_AMD_FCH that depends > on CONFIG_X86. > > I am /leaning/ on the refactor with idea 3. I'd go for something like the patch below. There's X86 dependencies for other I2C drivers as well, so it's not unprecedented. Thanks, Ingo ==============> drivers/i2c/busses/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 83c88c79afe2..bbbd6240fa6e 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -200,7 +200,7 @@ config I2C_ISMT config I2C_PIIX4 tristate "Intel PIIX4 and compatible (ATI/AMD/Serverworks/Broadcom/SMSC)" - depends on PCI && HAS_IOPORT + depends on PCI && HAS_IOPORT && X86 select I2C_SMBUS help If you say yes to this option, support will be included for the Intel
diff --git a/arch/x86/include/asm/amd_node.h b/arch/x86/include/asm/amd_node.h index 23fe617898a8f..f4993201834ea 100644 --- a/arch/x86/include/asm/amd_node.h +++ b/arch/x86/include/asm/amd_node.h @@ -19,6 +19,8 @@ #include <linux/pci.h> +#define FCH_PM_BASE 0xFED80300 + #define MAX_AMD_NUM_NODES 8 #define AMD_NODE0_PCI_SLOT 0x18 diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c index dd75916157f05..7c895001c5e8f 100644 --- a/drivers/i2c/busses/i2c-piix4.c +++ b/drivers/i2c/busses/i2c-piix4.c @@ -21,6 +21,7 @@ an i2c_algorithm to access them. */ +#include <asm/amd_node.h> #include <linux/module.h> #include <linux/moduleparam.h> #include <linux/pci.h> @@ -85,7 +86,6 @@ #define SB800_PIIX4_PORT_IDX_MASK_KERNCZ 0x18 #define SB800_PIIX4_PORT_IDX_SHIFT_KERNCZ 3 -#define SB800_PIIX4_FCH_PM_ADDR 0xFED80300 #define SB800_PIIX4_FCH_PM_SIZE 8 #define SB800_ASF_ACPI_PATH "\\_SB.ASFC" @@ -162,19 +162,19 @@ int piix4_sb800_region_request(struct device *dev, struct sb800_mmio_cfg *mmio_c if (mmio_cfg->use_mmio) { void __iomem *addr; - if (!request_mem_region_muxed(SB800_PIIX4_FCH_PM_ADDR, + if (!request_mem_region_muxed(FCH_PM_BASE, SB800_PIIX4_FCH_PM_SIZE, "sb800_piix4_smb")) { dev_err(dev, "SMBus base address memory region 0x%x already in use.\n", - SB800_PIIX4_FCH_PM_ADDR); + FCH_PM_BASE); return -EBUSY; } - addr = ioremap(SB800_PIIX4_FCH_PM_ADDR, + addr = ioremap(FCH_PM_BASE, SB800_PIIX4_FCH_PM_SIZE); if (!addr) { - release_mem_region(SB800_PIIX4_FCH_PM_ADDR, + release_mem_region(FCH_PM_BASE, SB800_PIIX4_FCH_PM_SIZE); dev_err(dev, "SMBus base address mapping failed.\n"); return -ENOMEM; @@ -201,7 +201,7 @@ void piix4_sb800_region_release(struct device *dev, struct sb800_mmio_cfg *mmio_ { if (mmio_cfg->use_mmio) { iounmap(mmio_cfg->addr); - release_mem_region(SB800_PIIX4_FCH_PM_ADDR, + release_mem_region(FCH_PM_BASE, SB800_PIIX4_FCH_PM_SIZE); return; }