Hi Simon,
On Tue, Dec 8, 2015 at 9:23 PM, Bin Meng <bmeng...@gmail.com> wrote:
> Hi Simon,
>
> On Tue, Dec 1, 2015 at 12:11 PM, Simon Glass <s...@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 <s...@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 <s...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
