[U-Boot] [PATCH 4/7] dm: Expand the uclass for Peripheral Controller Hubs (PCH)

[Simon Glass]

Hi Bin,

On 8 December 2015 at 06:23, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Tue, Dec 1, 2015 at 12:11 PM, Simon Glass <sjg at 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 at 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 at 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