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

[Bin Meng]

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?

> +
> +       /**
> +        * 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