NetBSD-Bugs archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: kern/48960: ichlpcib maps I/O space from ACPI BAR which ichsmb wants
The following reply was made to PR kern/48960; it has been noted by GNATS.
From: SAITOH Masanobu <msaitoh%execsw.org@localhost>
To: gnats-bugs%NetBSD.org@localhost, kern-bug-people%netbsd.org@localhost,
gnats-admin%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost
Cc: msaitoh%execsw.org@localhost
Subject: Re: kern/48960: ichlpcib maps I/O space from ACPI BAR which ichsmb
wants
Date: Sat, 20 Dec 2014 12:13:29 +0900
On 2014/07/03 4:50, Taylor R Campbell wrote:
>> Number: 48960
>> Category: kern
>> Synopsis: ichlpcib maps I/O space from ACPI BAR which ichsmb wants
>> Confidential: no
>> Severity: serious
>> Priority: medium
>> Responsible: kern-bug-people
>> State: open
>> Class: sw-bug
>> Submitter-Id: net
>> Arrival-Date: Wed Jul 02 19:50:00 +0000 2014
>> Originator: Taylor R Campbell <campbell+netbsd%mumble.net@localhost>
>> Release: NetBSD-current as of 2014-07-02
>> Organization:
>> Environment:
> Architecture: x86_64
> Machine: amd64
>> Description:
>
> ichlpcib(4) maps the space in PCI register 0x40, ACPI PMBASE,
> for acpipmtimer, tcotimer, and speedstep support. ichsmb(4)
> tries to map space which is sometimes in the middle of this
> region.
>
>> How-To-Repeat:
>
> Boot a system with ichlpcib and ichsmb. Observe that ichsmb0
> fails to map its I/O space because ichlpcib0 already mapped
> it.
>
>> Fix:
>
> matt@ and jakllsch@ say it is wrong for ichlpcib(4) to map the
> ACPI PMBASE region, which it does for acpipmtimer, tcotimer,
> and speedstep support.
>
> I imagine the Right Thing is to attach these only if ACPI is
> disabled, and to write proper ACPI drivers under sys/dev/acpi
> for them. Disabling the ad hoc drivers until proper ACPI
> drivers are written is probably not a good idea, however, and I
> don't have time or the datasheets or hardware necessary to port
> the ad hoc drivers to the ACPI subsystem.
>
How about the following patch?
This fixes a bug that ichlpcib(4) maps I/O area incorrectly.
- The LPCIB_PCI_PMBASE and LPCIB_PCI_GPIO register are alike PCI BAR but not
completely compatible with it. The PMBASE and GPIO registers define the
base address and the type but not describe the size. The size is fixed
to 128bytes. So use pci_mapreg_submap().
- Make pci_mapreg_submap() extern again.
- Fix the calculation of the map size in pci_mapreg_submap().
Index: sys/dev/pci/pcivar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/pcivar.h,v
retrieving revision 1.100
diff -u -p -r1.100 pcivar.h
--- sys/dev/pci/pcivar.h 16 Oct 2014 12:31:23 -0000 1.100
+++ sys/dev/pci/pcivar.h 19 Dec 2014 17:08:14 -0000
@@ -269,6 +269,10 @@ int pci_mapreg_info(pci_chipset_tag_t, p
int pci_mapreg_map(const struct pci_attach_args *, int, pcireg_t, int,
bus_space_tag_t *, bus_space_handle_t *, bus_addr_t *,
bus_size_t *);
+int pci_mapreg_submap(const struct pci_attach_args *, int, pcireg_t, int,
+ bus_size_t, bus_size_t, bus_space_tag_t *, bus_space_handle_t *,
+ bus_addr_t *, bus_size_t *);
+
int pci_find_rom(const struct pci_attach_args *, bus_space_tag_t,
bus_space_handle_t, bus_size_t,
Index: sys/dev/pci/pci_map.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/pci_map.c,v
retrieving revision 1.31
diff -u -p -r1.31 pci_map.c
--- sys/dev/pci/pci_map.c 16 Oct 2014 12:31:23 -0000 1.31
+++ sys/dev/pci/pci_map.c 19 Dec 2014 17:08:14 -0000
@@ -43,10 +43,6 @@ __KERNEL_RCSID(0, "$NetBSD: pci_map.c,v
#include <dev/pci/pcireg.h>
#include <dev/pci/pcivar.h>
-static int pci_mapreg_submap(const struct pci_attach_args *, int, pcireg_t, int,
- bus_size_t, bus_size_t, bus_space_tag_t *, bus_space_handle_t *,
- bus_addr_t *, bus_size_t *);
-
static int
pci_io_find(pci_chipset_tag_t pc, pcitag_t tag, int reg, pcireg_t type,
bus_addr_t *basep, bus_size_t *sizep, int *flagsp)
@@ -282,7 +278,7 @@ pci_mapreg_map(const struct pci_attach_a
handlep, basep, sizep);
}
-static int
+int
pci_mapreg_submap(const struct pci_attach_args *pa, int reg, pcireg_t type,
int busflags, bus_size_t maxsize, bus_size_t offset, bus_space_tag_t *tagp,
bus_space_handle_t *handlep, bus_addr_t *basep, bus_size_t *sizep)
@@ -324,10 +320,10 @@ pci_mapreg_submap(const struct pci_attac
* pci_mapreg_map.
*/
- maxsize = (maxsize && offset) ? maxsize : size;
+ maxsize = (maxsize != 0) ? maxsize : size;
base += offset;
- if ((maxsize < size && offset + maxsize <= size) || offset != 0)
+ if ((size < maxsize) || (size < (offset + maxsize)))
return 1;
if (bus_space_map(tag, base, maxsize, busflags | flags, &handle))
Index: sys/dev/ic/i82801lpcreg.h
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/i82801lpcreg.h,v
retrieving revision 1.11
diff -u -p -r1.11 i82801lpcreg.h
--- sys/dev/ic/i82801lpcreg.h 23 Jul 2010 02:23:58 -0000 1.11
+++ sys/dev/ic/i82801lpcreg.h 19 Dec 2014 17:08:14 -0000
@@ -40,6 +40,7 @@
* PCI configuration registers
*/
#define LPCIB_PCI_PMBASE 0x40
+#define LPCIB_PCI_PM_SIZE 0x00000080
#define LPCIB_PCI_ACPI_CNTL 0x44
# define LPCIB_PCI_ACPI_CNTL_EN (1 << 4)
/* GPIO config registers ICH6+ */
@@ -51,6 +52,7 @@
#define LPCIB_PCI_TCO_CNTL 0x54
/* GPIO config registers ICH0-ICH5 */
#define LPCIB_PCI_GPIO_BASE 0x58
+#define LPCIB_PCI_GPIO_SIZE 0x00000080
#define LPCIB_PCI_GPIO_CNTL 0x5c
#define LPCIB_PCI_GPIO_CNTL_EN (1 << 4)
#define LPCIB_PCI_PIRQA_ROUT 0x60
Index: sys/arch/x86/pci/ichlpcib.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/pci/ichlpcib.c,v
retrieving revision 1.44
diff -u -p -r1.44 ichlpcib.c
--- sys/arch/x86/pci/ichlpcib.c 15 Dec 2014 13:29:42 -0000 1.44
+++ sys/arch/x86/pci/ichlpcib.c 19 Dec 2014 17:08:14 -0000
@@ -321,9 +321,14 @@ lpcibattach(device_t parent, device_t se
* Part of our I/O registers are used as ACPI PM regs.
* Since our ACPI subsystem accesses the I/O space directly so far,
* we do not have to bother bus_space I/O map confliction.
+ *
+ * The PMBASE register is alike PCI BAR but not completely compatible
+ * with it. The PMBASE define the base address and the type but
+ * not describe the size.
*/
- if (pci_mapreg_map(pa, LPCIB_PCI_PMBASE, PCI_MAPREG_TYPE_IO, 0,
- &sc->sc_iot, &sc->sc_ioh, NULL, &sc->sc_iosize)) {
+ if (pci_mapreg_submap(pa, LPCIB_PCI_PMBASE, PCI_MAPREG_TYPE_IO, 0,
+ LPCIB_PCI_PM_SIZE, 0, &sc->sc_iot, &sc->sc_ioh, NULL,
+ &sc->sc_iosize)) {
aprint_error_dev(self, "can't map power management i/o space\n");
return;
}
@@ -1057,6 +1062,7 @@ lpcib_gpio_configure(device_t self)
pcireg_t gpio_cntl;
uint32_t use, io, bit;
int pin, shift, base_reg, cntl_reg, reg;
+ int rv;
/* this implies ICH >= 6, and thus different mapreg */
if (sc->sc_has_rcba) {
@@ -1073,11 +1079,16 @@ lpcib_gpio_configure(device_t self)
/* Is GPIO enabled? */
if ((gpio_cntl & LPCIB_PCI_GPIO_CNTL_EN) == 0)
return;
-
- if (pci_mapreg_map(&sc->sc_pa, base_reg, PCI_MAPREG_TYPE_IO, 0,
- &sc->sc_gpio_iot, &sc->sc_gpio_ioh,
- NULL, &sc->sc_gpio_ios)) {
- aprint_error_dev(self, "can't map general purpose i/o space\n");
+ /*
+ * The GPIO_BASE register is alike PCI BAR but not completely
+ * compatible with it. The PMBASE define the base address and the type
+ * but not describe the size.
+ */
+ rv = pci_mapreg_submap(&sc->sc_pa, base_reg, PCI_MAPREG_TYPE_IO, 0,
+ LPCIB_PCI_GPIO_SIZE, 0, &sc->sc_gpio_iot, &sc->sc_gpio_ioh,
+ NULL, &sc->sc_gpio_ios);
+ if (rv != 0) {
+ aprint_error_dev(self, "can't map general purpose i/o space(rv = %d)\n", rv);
return;
}
--
-----------------------------------------------
SAITOH Masanobu (msaitoh%execsw.org@localhost
msaitoh%netbsd.org@localhost)
Home |
Main Index |
Thread Index |
Old Index