| Message ID | 1448943086-1079-5-git-send-email-sjg@chromium.org |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Simon, On Tue, Dec 1, 2015 at 12:11 PM, Simon Glass <sjg@chromium.org> wrote: > A Peripheral Controller Hub is an Intel concept - it is like the peripherals I believe the name is Platform Controller Hub. > on an SoC and is often in a separate chip from the CPU. Even when it is not > it is addressed and used differently. The chip is typically found on the "Even when it is not" (a separate chip) "it is addressed and used differently"? I feel it should be "it is addressed and used the same'? > first PCI device. This indicates b.d.f = 0.0.0, but for registers like RCBA, SPI base, those are actually on the LPC device (b.d.f = 0.1f.0). Maybe we can say: the chip is typically found on the first PCI bus and integrates multiple devices? > > We have a very simple uclass to support PCHs. Add a few operations, such as > setting up the devices on the PCH and finding the SPI controller base > address. Also move it into drivers/pch/ since we will be adding a few PCH > drivers. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > arch/x86/lib/Makefile | 1 - > drivers/Makefile | 1 + > drivers/pch/Makefile | 5 +++ > {arch/x86/lib => drivers/pch}/pch-uclass.c | 32 +++++++++++++++ > include/pch.h | 66 ++++++++++++++++++++++++++++++ > 5 files changed, 104 insertions(+), 1 deletion(-) > create mode 100644 drivers/pch/Makefile > rename {arch/x86/lib => drivers/pch}/pch-uclass.c (53%) > create mode 100644 include/pch.h > > diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile > index cd5ecb6..43792bc 100644 > --- a/arch/x86/lib/Makefile > +++ b/arch/x86/lib/Makefile > @@ -24,7 +24,6 @@ obj-$(CONFIG_I8254_TIMER) += i8254.o > ifndef CONFIG_DM_PCI > obj-$(CONFIG_PCI) += pci_type1.o > endif > -obj-y += pch-uclass.o > obj-y += pirq_routing.o > obj-y += relocate.o > obj-y += physmem.o > diff --git a/drivers/Makefile b/drivers/Makefile > index c9031f2..acc6af9 100644 > --- a/drivers/Makefile > +++ b/drivers/Makefile > @@ -51,6 +51,7 @@ obj-y += hwmon/ > obj-y += misc/ > obj-y += pcmcia/ > obj-y += dfu/ > +obj-$(CONFIG_X86) += pch/ > obj-y += rtc/ > obj-y += sound/ > obj-y += timer/ > diff --git a/drivers/pch/Makefile b/drivers/pch/Makefile > new file mode 100644 > index 0000000..d69a99c > --- /dev/null > +++ b/drivers/pch/Makefile > @@ -0,0 +1,5 @@ > +# > +# SPDX-License-Identifier: GPL-2.0+ > +# > + > +obj-y += pch-uclass.o > diff --git a/arch/x86/lib/pch-uclass.c b/drivers/pch/pch-uclass.c > similarity index 53% > rename from arch/x86/lib/pch-uclass.c > rename to drivers/pch/pch-uclass.c > index 20dfa81..09a0107 100644 > --- a/arch/x86/lib/pch-uclass.c > +++ b/drivers/pch/pch-uclass.c > @@ -7,10 +7,42 @@ > > #include <common.h> > #include <dm.h> > +#include <pch.h> > #include <dm/root.h> > > DECLARE_GLOBAL_DATA_PTR; > > +int pch_init(struct udevice *dev) > +{ > + struct pch_ops *ops = pch_get_ops(dev); > + > + if (!ops->init) > + return -ENOSYS; > + > + return ops->init(dev); > +} > + > +int pch_get_sbase(struct udevice *dev, ulong *sbasep) > +{ > + struct pch_ops *ops = pch_get_ops(dev); > + > + *sbasep = 0; > + if (!ops->get_sbase) > + return -ENOSYS; > + > + return ops->get_sbase(dev, sbasep); > +} > + > +int pch_get_version(struct udevice *dev) > +{ > + struct pch_ops *ops = pch_get_ops(dev); > + > + if (!ops->get_version) > + return -ENOSYS; > + > + return ops->get_version(dev); > +} > + > static int pch_uclass_post_bind(struct udevice *bus) > { > /* > diff --git a/include/pch.h b/include/pch.h > new file mode 100644 > index 0000000..98bb5f2 > --- /dev/null > +++ b/include/pch.h > @@ -0,0 +1,66 @@ > +/* > + * Copyright (c) 2015 Google, Inc > + * Written by Simon Glass <sjg@chromium.org> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#ifndef __pch_h > +#define __pch_h > + > +struct pch_ops { > + /** > + * init() - set up the PCH devices > + * > + * This makes sure that all the devices are ready for use. They are > + * not actually started, just set up so that they can be probed. > + */ > + int (*init)(struct udevice *dev); Do we need create such an init op? Should this be done in the driver's probe routine? > + > + /** > + * get_sbase() - get the address of SBASE SBASE -> SPI base > + * > + * @dev: PCH device to check > + * @sbasep: Returns address of SBASE if available, else 0 > + * @return 0 if OK, -ve on error (e.g. there is no SBASE) > + */ > + int (*get_sbase)(struct udevice *dev, ulong *sbasep); > + > + /** > + * get_version() - get the PCH version (e.g. 7 or 9) Can we create an enum for 7 and 9? > + * > + * @return version, or -1 if unknown > + */ > + int (*get_version)(struct udevice *dev); > +}; > + > +#define pch_get_ops(dev) ((struct pch_ops *)(dev)->driver->ops) > + > +/** > + * pch_init() - init a PCH > + * > + * This makes sure that all the devices are ready for use. They are > + * not actually started, just set up so that they can be probed. > + * > + * @dev: PCH device to init > + * @return 0 if OK, -ve on error > + */ > +int pch_init(struct udevice *dev); > + > +/** > + * pch_get_sbase() - get the address of SBASE > + * > + * @dev: PCH device to check > + * @sbasep: Returns address of SBASE if available, else 0 > + * @return 0 if OK, -ve on error (e.g. there is no SBASE) > + */ > +int pch_get_sbase(struct udevice *dev, ulong *sbasep); > + > +/** > + * pch_get_version() - get the PCH version (e.g. 7 or 9) > + * > + * @return version, or -ve if unknown/error > + */ > +int pch_get_version(struct udevice *dev); > + > +#endif > -- Regards, Bin
Hi Simon, On Tue, Dec 8, 2015 at 9:23 PM, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Tue, Dec 1, 2015 at 12:11 PM, Simon Glass <sjg@chromium.org> wrote: >> A Peripheral Controller Hub is an Intel concept - it is like the peripherals > > I believe the name is Platform Controller Hub. > >> on an SoC and is often in a separate chip from the CPU. Even when it is not >> it is addressed and used differently. The chip is typically found on the > > "Even when it is not" (a separate chip) "it is addressed and used > differently"? I feel it should be "it is addressed and used the same'? > >> first PCI device. > > This indicates b.d.f = 0.0.0, but for registers like RCBA, SPI base, > those are actually on the LPC device (b.d.f = 0.1f.0). Maybe we can > say: the chip is typically found on the first PCI bus and integrates > multiple devices? > >> >> We have a very simple uclass to support PCHs. Add a few operations, such as >> setting up the devices on the PCH and finding the SPI controller base >> address. Also move it into drivers/pch/ since we will be adding a few PCH >> drivers. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> --- >> >> arch/x86/lib/Makefile | 1 - >> drivers/Makefile | 1 + >> drivers/pch/Makefile | 5 +++ >> {arch/x86/lib => drivers/pch}/pch-uclass.c | 32 +++++++++++++++ >> include/pch.h | 66 ++++++++++++++++++++++++++++++ >> 5 files changed, 104 insertions(+), 1 deletion(-) >> create mode 100644 drivers/pch/Makefile >> rename {arch/x86/lib => drivers/pch}/pch-uclass.c (53%) >> create mode 100644 include/pch.h >> >> diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile >> index cd5ecb6..43792bc 100644 >> --- a/arch/x86/lib/Makefile >> +++ b/arch/x86/lib/Makefile >> @@ -24,7 +24,6 @@ obj-$(CONFIG_I8254_TIMER) += i8254.o >> ifndef CONFIG_DM_PCI >> obj-$(CONFIG_PCI) += pci_type1.o >> endif >> -obj-y += pch-uclass.o >> obj-y += pirq_routing.o >> obj-y += relocate.o >> obj-y += physmem.o >> diff --git a/drivers/Makefile b/drivers/Makefile >> index c9031f2..acc6af9 100644 >> --- a/drivers/Makefile >> +++ b/drivers/Makefile >> @@ -51,6 +51,7 @@ obj-y += hwmon/ >> obj-y += misc/ >> obj-y += pcmcia/ >> obj-y += dfu/ >> +obj-$(CONFIG_X86) += pch/ >> obj-y += rtc/ >> obj-y += sound/ >> obj-y += timer/ >> diff --git a/drivers/pch/Makefile b/drivers/pch/Makefile >> new file mode 100644 >> index 0000000..d69a99c >> --- /dev/null >> +++ b/drivers/pch/Makefile >> @@ -0,0 +1,5 @@ >> +# >> +# SPDX-License-Identifier: GPL-2.0+ >> +# >> + >> +obj-y += pch-uclass.o >> diff --git a/arch/x86/lib/pch-uclass.c b/drivers/pch/pch-uclass.c >> similarity index 53% >> rename from arch/x86/lib/pch-uclass.c >> rename to drivers/pch/pch-uclass.c >> index 20dfa81..09a0107 100644 >> --- a/arch/x86/lib/pch-uclass.c >> +++ b/drivers/pch/pch-uclass.c >> @@ -7,10 +7,42 @@ >> >> #include <common.h> >> #include <dm.h> >> +#include <pch.h> >> #include <dm/root.h> >> >> DECLARE_GLOBAL_DATA_PTR; >> >> +int pch_init(struct udevice *dev) >> +{ >> + struct pch_ops *ops = pch_get_ops(dev); >> + >> + if (!ops->init) >> + return -ENOSYS; >> + >> + return ops->init(dev); >> +} >> + >> +int pch_get_sbase(struct udevice *dev, ulong *sbasep) >> +{ >> + struct pch_ops *ops = pch_get_ops(dev); >> + >> + *sbasep = 0; >> + if (!ops->get_sbase) >> + return -ENOSYS; >> + >> + return ops->get_sbase(dev, sbasep); >> +} >> + >> +int pch_get_version(struct udevice *dev) >> +{ >> + struct pch_ops *ops = pch_get_ops(dev); >> + >> + if (!ops->get_version) >> + return -ENOSYS; >> + >> + return ops->get_version(dev); >> +} >> + >> static int pch_uclass_post_bind(struct udevice *bus) >> { >> /* >> diff --git a/include/pch.h b/include/pch.h >> new file mode 100644 >> index 0000000..98bb5f2 >> --- /dev/null >> +++ b/include/pch.h >> @@ -0,0 +1,66 @@ >> +/* >> + * Copyright (c) 2015 Google, Inc >> + * Written by Simon Glass <sjg@chromium.org> >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#ifndef __pch_h >> +#define __pch_h >> + >> +struct pch_ops { >> + /** >> + * init() - set up the PCH devices >> + * >> + * This makes sure that all the devices are ready for use. They are >> + * not actually started, just set up so that they can be probed. >> + */ >> + int (*init)(struct udevice *dev); > > Do we need create such an init op? Should this be done in the driver's > probe routine? > >> + >> + /** >> + * get_sbase() - get the address of SBASE > > SBASE -> SPI base > >> + * >> + * @dev: PCH device to check >> + * @sbasep: Returns address of SBASE if available, else 0 >> + * @return 0 if OK, -ve on error (e.g. there is no SBASE) >> + */ >> + int (*get_sbase)(struct udevice *dev, ulong *sbasep); >> + >> + /** >> + * get_version() - get the PCH version (e.g. 7 or 9) > > Can we create an enum for 7 and 9? > >> + * >> + * @return version, or -1 if unknown >> + */ >> + int (*get_version)(struct udevice *dev); >> +}; I feel we should create an op for getting RCBA as well? For example, the IRQ router driver needs it. >> + >> +#define pch_get_ops(dev) ((struct pch_ops *)(dev)->driver->ops) >> + >> +/** >> + * pch_init() - init a PCH >> + * >> + * This makes sure that all the devices are ready for use. They are >> + * not actually started, just set up so that they can be probed. >> + * >> + * @dev: PCH device to init >> + * @return 0 if OK, -ve on error >> + */ >> +int pch_init(struct udevice *dev); >> + >> +/** >> + * pch_get_sbase() - get the address of SBASE >> + * >> + * @dev: PCH device to check >> + * @sbasep: Returns address of SBASE if available, else 0 >> + * @return 0 if OK, -ve on error (e.g. there is no SBASE) >> + */ >> +int pch_get_sbase(struct udevice *dev, ulong *sbasep); >> + >> +/** >> + * pch_get_version() - get the PCH version (e.g. 7 or 9) >> + * >> + * @return version, or -ve if unknown/error >> + */ >> +int pch_get_version(struct udevice *dev); >> + >> +#endif >> -- Regards, Bin
Hi Bin, On 8 December 2015 at 06:23, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Tue, Dec 1, 2015 at 12:11 PM, Simon Glass <sjg@chromium.org> wrote: >> A Peripheral Controller Hub is an Intel concept - it is like the peripherals > > I believe the name is Platform Controller Hub. > >> on an SoC and is often in a separate chip from the CPU. Even when it is not >> it is addressed and used differently. The chip is typically found on the > > "Even when it is not" (a separate chip) "it is addressed and used > differently"? I feel it should be "it is addressed and used the same'? > >> first PCI device. > > This indicates b.d.f = 0.0.0, but for registers like RCBA, SPI base, > those are actually on the LPC device (b.d.f = 0.1f.0). Maybe we can > say: the chip is typically found on the first PCI bus and integrates > multiple devices? > >> >> We have a very simple uclass to support PCHs. Add a few operations, such as >> setting up the devices on the PCH and finding the SPI controller base >> address. Also move it into drivers/pch/ since we will be adding a few PCH >> drivers. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> --- >> >> arch/x86/lib/Makefile | 1 - >> drivers/Makefile | 1 + >> drivers/pch/Makefile | 5 +++ >> {arch/x86/lib => drivers/pch}/pch-uclass.c | 32 +++++++++++++++ >> include/pch.h | 66 ++++++++++++++++++++++++++++++ >> 5 files changed, 104 insertions(+), 1 deletion(-) >> create mode 100644 drivers/pch/Makefile >> rename {arch/x86/lib => drivers/pch}/pch-uclass.c (53%) >> create mode 100644 include/pch.h >> >> diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile >> index cd5ecb6..43792bc 100644 >> --- a/arch/x86/lib/Makefile >> +++ b/arch/x86/lib/Makefile >> @@ -24,7 +24,6 @@ obj-$(CONFIG_I8254_TIMER) += i8254.o >> ifndef CONFIG_DM_PCI >> obj-$(CONFIG_PCI) += pci_type1.o >> endif >> -obj-y += pch-uclass.o >> obj-y += pirq_routing.o >> obj-y += relocate.o >> obj-y += physmem.o >> diff --git a/drivers/Makefile b/drivers/Makefile >> index c9031f2..acc6af9 100644 >> --- a/drivers/Makefile >> +++ b/drivers/Makefile >> @@ -51,6 +51,7 @@ obj-y += hwmon/ >> obj-y += misc/ >> obj-y += pcmcia/ >> obj-y += dfu/ >> +obj-$(CONFIG_X86) += pch/ >> obj-y += rtc/ >> obj-y += sound/ >> obj-y += timer/ >> diff --git a/drivers/pch/Makefile b/drivers/pch/Makefile >> new file mode 100644 >> index 0000000..d69a99c >> --- /dev/null >> +++ b/drivers/pch/Makefile >> @@ -0,0 +1,5 @@ >> +# >> +# SPDX-License-Identifier: GPL-2.0+ >> +# >> + >> +obj-y += pch-uclass.o >> diff --git a/arch/x86/lib/pch-uclass.c b/drivers/pch/pch-uclass.c >> similarity index 53% >> rename from arch/x86/lib/pch-uclass.c >> rename to drivers/pch/pch-uclass.c >> index 20dfa81..09a0107 100644 >> --- a/arch/x86/lib/pch-uclass.c >> +++ b/drivers/pch/pch-uclass.c >> @@ -7,10 +7,42 @@ >> >> #include <common.h> >> #include <dm.h> >> +#include <pch.h> >> #include <dm/root.h> >> >> DECLARE_GLOBAL_DATA_PTR; >> >> +int pch_init(struct udevice *dev) >> +{ >> + struct pch_ops *ops = pch_get_ops(dev); >> + >> + if (!ops->init) >> + return -ENOSYS; >> + >> + return ops->init(dev); >> +} >> + >> +int pch_get_sbase(struct udevice *dev, ulong *sbasep) >> +{ >> + struct pch_ops *ops = pch_get_ops(dev); >> + >> + *sbasep = 0; >> + if (!ops->get_sbase) >> + return -ENOSYS; >> + >> + return ops->get_sbase(dev, sbasep); >> +} >> + >> +int pch_get_version(struct udevice *dev) >> +{ >> + struct pch_ops *ops = pch_get_ops(dev); >> + >> + if (!ops->get_version) >> + return -ENOSYS; >> + >> + return ops->get_version(dev); >> +} >> + >> static int pch_uclass_post_bind(struct udevice *bus) >> { >> /* >> diff --git a/include/pch.h b/include/pch.h >> new file mode 100644 >> index 0000000..98bb5f2 >> --- /dev/null >> +++ b/include/pch.h >> @@ -0,0 +1,66 @@ >> +/* >> + * Copyright (c) 2015 Google, Inc >> + * Written by Simon Glass <sjg@chromium.org> >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#ifndef __pch_h >> +#define __pch_h >> + >> +struct pch_ops { >> + /** >> + * init() - set up the PCH devices >> + * >> + * This makes sure that all the devices are ready for use. They are >> + * not actually started, just set up so that they can be probed. >> + */ >> + int (*init)(struct udevice *dev); > > Do we need create such an init op? Should this be done in the driver's > probe routine? The PCH is modelled in ivybridge as the device at address 0,0,0. I have found that we need to do the init in two stages, so this is the reason for the init() method. However, I am still working on refactoring and simplifying the code. So it is possible that at some point I will figure out how to remove this. But for now I cannot see how. > >> + >> + /** >> + * get_sbase() - get the address of SBASE > > SBASE -> SPI base > >> + * >> + * @dev: PCH device to check >> + * @sbasep: Returns address of SBASE if available, else 0 >> + * @return 0 if OK, -ve on error (e.g. there is no SBASE) >> + */ >> + int (*get_sbase)(struct udevice *dev, ulong *sbasep); >> + >> + /** >> + * get_version() - get the PCH version (e.g. 7 or 9) > > Can we create an enum for 7 and 9? > >> + * >> + * @return version, or -1 if unknown >> + */ >> + int (*get_version)(struct udevice *dev); >> +}; >> + >> +#define pch_get_ops(dev) ((struct pch_ops *)(dev)->driver->ops) >> + >> +/** >> + * pch_init() - init a PCH >> + * >> + * This makes sure that all the devices are ready for use. They are >> + * not actually started, just set up so that they can be probed. >> + * >> + * @dev: PCH device to init >> + * @return 0 if OK, -ve on error >> + */ >> +int pch_init(struct udevice *dev); >> + >> +/** >> + * pch_get_sbase() - get the address of SBASE >> + * >> + * @dev: PCH device to check >> + * @sbasep: Returns address of SBASE if available, else 0 >> + * @return 0 if OK, -ve on error (e.g. there is no SBASE) >> + */ >> +int pch_get_sbase(struct udevice *dev, ulong *sbasep); >> + >> +/** >> + * pch_get_version() - get the PCH version (e.g. 7 or 9) >> + * >> + * @return version, or -ve if unknown/error >> + */ >> +int pch_get_version(struct udevice *dev); >> + >> +#endif >> -- > > Regards, > Bin Regards, Simon
Hi Simon, On Thu, Dec 17, 2015 at 12:09 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 8 December 2015 at 06:23, Bin Meng <bmeng.cn@gmail.com> wrote: >> Hi Simon, >> >> On Tue, Dec 1, 2015 at 12:11 PM, Simon Glass <sjg@chromium.org> wrote: >>> A Peripheral Controller Hub is an Intel concept - it is like the peripherals >> >> I believe the name is Platform Controller Hub. >> >>> on an SoC and is often in a separate chip from the CPU. Even when it is not >>> it is addressed and used differently. The chip is typically found on the >> >> "Even when it is not" (a separate chip) "it is addressed and used >> differently"? I feel it should be "it is addressed and used the same'? >> >>> first PCI device. >> >> This indicates b.d.f = 0.0.0, but for registers like RCBA, SPI base, >> those are actually on the LPC device (b.d.f = 0.1f.0). Maybe we can >> say: the chip is typically found on the first PCI bus and integrates >> multiple devices? >> >>> >>> We have a very simple uclass to support PCHs. Add a few operations, such as >>> setting up the devices on the PCH and finding the SPI controller base >>> address. Also move it into drivers/pch/ since we will be adding a few PCH >>> drivers. >>> >>> Signed-off-by: Simon Glass <sjg@chromium.org> >>> --- >>> >>> arch/x86/lib/Makefile | 1 - >>> drivers/Makefile | 1 + >>> drivers/pch/Makefile | 5 +++ >>> {arch/x86/lib => drivers/pch}/pch-uclass.c | 32 +++++++++++++++ >>> include/pch.h | 66 ++++++++++++++++++++++++++++++ >>> 5 files changed, 104 insertions(+), 1 deletion(-) >>> create mode 100644 drivers/pch/Makefile >>> rename {arch/x86/lib => drivers/pch}/pch-uclass.c (53%) >>> create mode 100644 include/pch.h >>> >>> diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile >>> index cd5ecb6..43792bc 100644 >>> --- a/arch/x86/lib/Makefile >>> +++ b/arch/x86/lib/Makefile >>> @@ -24,7 +24,6 @@ obj-$(CONFIG_I8254_TIMER) += i8254.o >>> ifndef CONFIG_DM_PCI >>> obj-$(CONFIG_PCI) += pci_type1.o >>> endif >>> -obj-y += pch-uclass.o >>> obj-y += pirq_routing.o >>> obj-y += relocate.o >>> obj-y += physmem.o >>> diff --git a/drivers/Makefile b/drivers/Makefile >>> index c9031f2..acc6af9 100644 >>> --- a/drivers/Makefile >>> +++ b/drivers/Makefile >>> @@ -51,6 +51,7 @@ obj-y += hwmon/ >>> obj-y += misc/ >>> obj-y += pcmcia/ >>> obj-y += dfu/ >>> +obj-$(CONFIG_X86) += pch/ >>> obj-y += rtc/ >>> obj-y += sound/ >>> obj-y += timer/ >>> diff --git a/drivers/pch/Makefile b/drivers/pch/Makefile >>> new file mode 100644 >>> index 0000000..d69a99c >>> --- /dev/null >>> +++ b/drivers/pch/Makefile >>> @@ -0,0 +1,5 @@ >>> +# >>> +# SPDX-License-Identifier: GPL-2.0+ >>> +# >>> + >>> +obj-y += pch-uclass.o >>> diff --git a/arch/x86/lib/pch-uclass.c b/drivers/pch/pch-uclass.c >>> similarity index 53% >>> rename from arch/x86/lib/pch-uclass.c >>> rename to drivers/pch/pch-uclass.c >>> index 20dfa81..09a0107 100644 >>> --- a/arch/x86/lib/pch-uclass.c >>> +++ b/drivers/pch/pch-uclass.c >>> @@ -7,10 +7,42 @@ >>> >>> #include <common.h> >>> #include <dm.h> >>> +#include <pch.h> >>> #include <dm/root.h> >>> >>> DECLARE_GLOBAL_DATA_PTR; >>> >>> +int pch_init(struct udevice *dev) >>> +{ >>> + struct pch_ops *ops = pch_get_ops(dev); >>> + >>> + if (!ops->init) >>> + return -ENOSYS; >>> + >>> + return ops->init(dev); >>> +} >>> + >>> +int pch_get_sbase(struct udevice *dev, ulong *sbasep) >>> +{ >>> + struct pch_ops *ops = pch_get_ops(dev); >>> + >>> + *sbasep = 0; >>> + if (!ops->get_sbase) >>> + return -ENOSYS; >>> + >>> + return ops->get_sbase(dev, sbasep); >>> +} >>> + >>> +int pch_get_version(struct udevice *dev) >>> +{ >>> + struct pch_ops *ops = pch_get_ops(dev); >>> + >>> + if (!ops->get_version) >>> + return -ENOSYS; >>> + >>> + return ops->get_version(dev); >>> +} >>> + >>> static int pch_uclass_post_bind(struct udevice *bus) >>> { >>> /* >>> diff --git a/include/pch.h b/include/pch.h >>> new file mode 100644 >>> index 0000000..98bb5f2 >>> --- /dev/null >>> +++ b/include/pch.h >>> @@ -0,0 +1,66 @@ >>> +/* >>> + * Copyright (c) 2015 Google, Inc >>> + * Written by Simon Glass <sjg@chromium.org> >>> + * >>> + * SPDX-License-Identifier: GPL-2.0+ >>> + */ >>> + >>> +#ifndef __pch_h >>> +#define __pch_h >>> + >>> +struct pch_ops { >>> + /** >>> + * init() - set up the PCH devices >>> + * >>> + * This makes sure that all the devices are ready for use. They are >>> + * not actually started, just set up so that they can be probed. >>> + */ >>> + int (*init)(struct udevice *dev); >> >> Do we need create such an init op? Should this be done in the driver's >> probe routine? > > The PCH is modelled in ivybridge as the device at address 0,0,0. I > have found that we need to do the init in two stages, so this is the > reason for the init() method. However, I am still working on > refactoring and simplifying the code. So it is possible that at some > point I will figure out how to remove this. But for now I cannot see > how. > Does if (gd->flags & GD_FLG_RELOC) not help? [snip] Regards, Bin
Hi Bin, On 17 December 2015 at 03:09, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Thu, Dec 17, 2015 at 12:09 PM, Simon Glass <sjg@chromium.org> wrote: >> Hi Bin, >> >> On 8 December 2015 at 06:23, Bin Meng <bmeng.cn@gmail.com> wrote: >>> Hi Simon, >>> >>> On Tue, Dec 1, 2015 at 12:11 PM, Simon Glass <sjg@chromium.org> wrote: >>>> A Peripheral Controller Hub is an Intel concept - it is like the peripherals >>> >>> I believe the name is Platform Controller Hub. >>> >>>> on an SoC and is often in a separate chip from the CPU. Even when it is not >>>> it is addressed and used differently. The chip is typically found on the >>> >>> "Even when it is not" (a separate chip) "it is addressed and used >>> differently"? I feel it should be "it is addressed and used the same'? >>> >>>> first PCI device. >>> >>> This indicates b.d.f = 0.0.0, but for registers like RCBA, SPI base, >>> those are actually on the LPC device (b.d.f = 0.1f.0). Maybe we can >>> say: the chip is typically found on the first PCI bus and integrates >>> multiple devices? >>> >>>> >>>> We have a very simple uclass to support PCHs. Add a few operations, such as >>>> setting up the devices on the PCH and finding the SPI controller base >>>> address. Also move it into drivers/pch/ since we will be adding a few PCH >>>> drivers. >>>> >>>> Signed-off-by: Simon Glass <sjg@chromium.org> >>>> --- >>>> >>>> arch/x86/lib/Makefile | 1 - >>>> drivers/Makefile | 1 + >>>> drivers/pch/Makefile | 5 +++ >>>> {arch/x86/lib => drivers/pch}/pch-uclass.c | 32 +++++++++++++++ >>>> include/pch.h | 66 ++++++++++++++++++++++++++++++ >>>> 5 files changed, 104 insertions(+), 1 deletion(-) >>>> create mode 100644 drivers/pch/Makefile >>>> rename {arch/x86/lib => drivers/pch}/pch-uclass.c (53%) >>>> create mode 100644 include/pch.h >>>> >>>> diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile >>>> index cd5ecb6..43792bc 100644 >>>> --- a/arch/x86/lib/Makefile >>>> +++ b/arch/x86/lib/Makefile >>>> @@ -24,7 +24,6 @@ obj-$(CONFIG_I8254_TIMER) += i8254.o >>>> ifndef CONFIG_DM_PCI >>>> obj-$(CONFIG_PCI) += pci_type1.o >>>> endif >>>> -obj-y += pch-uclass.o >>>> obj-y += pirq_routing.o >>>> obj-y += relocate.o >>>> obj-y += physmem.o >>>> diff --git a/drivers/Makefile b/drivers/Makefile >>>> index c9031f2..acc6af9 100644 >>>> --- a/drivers/Makefile >>>> +++ b/drivers/Makefile >>>> @@ -51,6 +51,7 @@ obj-y += hwmon/ >>>> obj-y += misc/ >>>> obj-y += pcmcia/ >>>> obj-y += dfu/ >>>> +obj-$(CONFIG_X86) += pch/ >>>> obj-y += rtc/ >>>> obj-y += sound/ >>>> obj-y += timer/ >>>> diff --git a/drivers/pch/Makefile b/drivers/pch/Makefile >>>> new file mode 100644 >>>> index 0000000..d69a99c >>>> --- /dev/null >>>> +++ b/drivers/pch/Makefile >>>> @@ -0,0 +1,5 @@ >>>> +# >>>> +# SPDX-License-Identifier: GPL-2.0+ >>>> +# >>>> + >>>> +obj-y += pch-uclass.o >>>> diff --git a/arch/x86/lib/pch-uclass.c b/drivers/pch/pch-uclass.c >>>> similarity index 53% >>>> rename from arch/x86/lib/pch-uclass.c >>>> rename to drivers/pch/pch-uclass.c >>>> index 20dfa81..09a0107 100644 >>>> --- a/arch/x86/lib/pch-uclass.c >>>> +++ b/drivers/pch/pch-uclass.c >>>> @@ -7,10 +7,42 @@ >>>> >>>> #include <common.h> >>>> #include <dm.h> >>>> +#include <pch.h> >>>> #include <dm/root.h> >>>> >>>> DECLARE_GLOBAL_DATA_PTR; >>>> >>>> +int pch_init(struct udevice *dev) >>>> +{ >>>> + struct pch_ops *ops = pch_get_ops(dev); >>>> + >>>> + if (!ops->init) >>>> + return -ENOSYS; >>>> + >>>> + return ops->init(dev); >>>> +} >>>> + >>>> +int pch_get_sbase(struct udevice *dev, ulong *sbasep) >>>> +{ >>>> + struct pch_ops *ops = pch_get_ops(dev); >>>> + >>>> + *sbasep = 0; >>>> + if (!ops->get_sbase) >>>> + return -ENOSYS; >>>> + >>>> + return ops->get_sbase(dev, sbasep); >>>> +} >>>> + >>>> +int pch_get_version(struct udevice *dev) >>>> +{ >>>> + struct pch_ops *ops = pch_get_ops(dev); >>>> + >>>> + if (!ops->get_version) >>>> + return -ENOSYS; >>>> + >>>> + return ops->get_version(dev); >>>> +} >>>> + >>>> static int pch_uclass_post_bind(struct udevice *bus) >>>> { >>>> /* >>>> diff --git a/include/pch.h b/include/pch.h >>>> new file mode 100644 >>>> index 0000000..98bb5f2 >>>> --- /dev/null >>>> +++ b/include/pch.h >>>> @@ -0,0 +1,66 @@ >>>> +/* >>>> + * Copyright (c) 2015 Google, Inc >>>> + * Written by Simon Glass <sjg@chromium.org> >>>> + * >>>> + * SPDX-License-Identifier: GPL-2.0+ >>>> + */ >>>> + >>>> +#ifndef __pch_h >>>> +#define __pch_h >>>> + >>>> +struct pch_ops { >>>> + /** >>>> + * init() - set up the PCH devices >>>> + * >>>> + * This makes sure that all the devices are ready for use. They are >>>> + * not actually started, just set up so that they can be probed. >>>> + */ >>>> + int (*init)(struct udevice *dev); >>> >>> Do we need create such an init op? Should this be done in the driver's >>> probe routine? >> >> The PCH is modelled in ivybridge as the device at address 0,0,0. I >> have found that we need to do the init in two stages, so this is the >> reason for the init() method. However, I am still working on >> refactoring and simplifying the code. So it is possible that at some >> point I will figure out how to remove this. But for now I cannot see >> how. This comment is incorrect - it is the northbridge at is at 0,0,0. The PCH is 0,1f,0. >> > > Does if (gd->flags & GD_FLG_RELOC) not help? I already use that, but no it does not help. That said, I do hope to remove it. It's very difficult to move everything at once and it would make the code difficult to review also. Regards, Simon
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile index cd5ecb6..43792bc 100644 --- a/arch/x86/lib/Makefile +++ b/arch/x86/lib/Makefile @@ -24,7 +24,6 @@ obj-$(CONFIG_I8254_TIMER) += i8254.o ifndef CONFIG_DM_PCI obj-$(CONFIG_PCI) += pci_type1.o endif -obj-y += pch-uclass.o obj-y += pirq_routing.o obj-y += relocate.o obj-y += physmem.o diff --git a/drivers/Makefile b/drivers/Makefile index c9031f2..acc6af9 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -51,6 +51,7 @@ obj-y += hwmon/ obj-y += misc/ obj-y += pcmcia/ obj-y += dfu/ +obj-$(CONFIG_X86) += pch/ obj-y += rtc/ obj-y += sound/ obj-y += timer/ diff --git a/drivers/pch/Makefile b/drivers/pch/Makefile new file mode 100644 index 0000000..d69a99c --- /dev/null +++ b/drivers/pch/Makefile @@ -0,0 +1,5 @@ +# +# SPDX-License-Identifier: GPL-2.0+ +# + +obj-y += pch-uclass.o diff --git a/arch/x86/lib/pch-uclass.c b/drivers/pch/pch-uclass.c similarity index 53% rename from arch/x86/lib/pch-uclass.c rename to drivers/pch/pch-uclass.c index 20dfa81..09a0107 100644 --- a/arch/x86/lib/pch-uclass.c +++ b/drivers/pch/pch-uclass.c @@ -7,10 +7,42 @@ #include <common.h> #include <dm.h> +#include <pch.h> #include <dm/root.h> DECLARE_GLOBAL_DATA_PTR; +int pch_init(struct udevice *dev) +{ + struct pch_ops *ops = pch_get_ops(dev); + + if (!ops->init) + return -ENOSYS; + + return ops->init(dev); +} + +int pch_get_sbase(struct udevice *dev, ulong *sbasep) +{ + struct pch_ops *ops = pch_get_ops(dev); + + *sbasep = 0; + if (!ops->get_sbase) + return -ENOSYS; + + return ops->get_sbase(dev, sbasep); +} + +int pch_get_version(struct udevice *dev) +{ + struct pch_ops *ops = pch_get_ops(dev); + + if (!ops->get_version) + return -ENOSYS; + + return ops->get_version(dev); +} + static int pch_uclass_post_bind(struct udevice *bus) { /* diff --git a/include/pch.h b/include/pch.h new file mode 100644 index 0000000..98bb5f2 --- /dev/null +++ b/include/pch.h @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2015 Google, Inc + * Written by Simon Glass <sjg@chromium.org> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef __pch_h +#define __pch_h + +struct pch_ops { + /** + * init() - set up the PCH devices + * + * This makes sure that all the devices are ready for use. They are + * not actually started, just set up so that they can be probed. + */ + int (*init)(struct udevice *dev); + + /** + * get_sbase() - get the address of SBASE + * + * @dev: PCH device to check + * @sbasep: Returns address of SBASE if available, else 0 + * @return 0 if OK, -ve on error (e.g. there is no SBASE) + */ + int (*get_sbase)(struct udevice *dev, ulong *sbasep); + + /** + * get_version() - get the PCH version (e.g. 7 or 9) + * + * @return version, or -1 if unknown + */ + int (*get_version)(struct udevice *dev); +}; + +#define pch_get_ops(dev) ((struct pch_ops *)(dev)->driver->ops) + +/** + * pch_init() - init a PCH + * + * This makes sure that all the devices are ready for use. They are + * not actually started, just set up so that they can be probed. + * + * @dev: PCH device to init + * @return 0 if OK, -ve on error + */ +int pch_init(struct udevice *dev); + +/** + * pch_get_sbase() - get the address of SBASE + * + * @dev: PCH device to check + * @sbasep: Returns address of SBASE if available, else 0 + * @return 0 if OK, -ve on error (e.g. there is no SBASE) + */ +int pch_get_sbase(struct udevice *dev, ulong *sbasep); + +/** + * pch_get_version() - get the PCH version (e.g. 7 or 9) + * + * @return version, or -ve if unknown/error + */ +int pch_get_version(struct udevice *dev); + +#endif
A Peripheral Controller Hub is an Intel concept - it is like the peripherals on an SoC and is often in a separate chip from the CPU. Even when it is not it is addressed and used differently. The chip is typically found on the first PCI device. We have a very simple uclass to support PCHs. Add a few operations, such as setting up the devices on the PCH and finding the SPI controller base address. Also move it into drivers/pch/ since we will be adding a few PCH drivers. Signed-off-by: Simon Glass <sjg@chromium.org> --- arch/x86/lib/Makefile | 1 - drivers/Makefile | 1 + drivers/pch/Makefile | 5 +++ {arch/x86/lib => drivers/pch}/pch-uclass.c | 32 +++++++++++++++ include/pch.h | 66 ++++++++++++++++++++++++++++++ 5 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 drivers/pch/Makefile rename {arch/x86/lib => drivers/pch}/pch-uclass.c (53%) create mode 100644 include/pch.h