[U-Boot,v4,5/8] dm: x86: Add a driver for Intel PCH7

At some point we may need to distinguish between different types of PCHs,
but for existing supported platforms we only need to worry about version 7
and version 9 bridges. Add a driver for the PCH7.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v4:
- Correct BIOS_CTRL address for PCH7

Changes in v3: None
Changes in v2:
- Rename the PCH functions
- Update the get_version() handle to use an enum
- Add a function to obtain the SPI base address
- Add enums for BIOS_CTRL register and bits

 drivers/pch/Makefile |  1 +
 drivers/pch/pch7.c   | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/pch.h        |  8 +++++++
 3 files changed, 70 insertions(+)
 create mode 100644 drivers/pch/pch7.c

Hi Simon,

On Sun, Jan 17, 2016 at 7:44 AM, Simon Glass <sjg@chromium.org> wrote:
> At some point we may need to distinguish between different types of PCHs,
> but for existing supported platforms we only need to worry about version 7
> and version 9 bridges. Add a driver for the PCH7.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v4:
> - Correct BIOS_CTRL address for PCH7
>
> Changes in v3: None
> Changes in v2:
> - Rename the PCH functions
> - Update the get_version() handle to use an enum
> - Add a function to obtain the SPI base address
> - Add enums for BIOS_CTRL register and bits
>
>  drivers/pch/Makefile |  1 +
>  drivers/pch/pch7.c   | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/pch.h        |  8 +++++++
>  3 files changed, 70 insertions(+)
>  create mode 100644 drivers/pch/pch7.c
>
> diff --git a/drivers/pch/Makefile b/drivers/pch/Makefile
> index d69a99c..33aa727 100644
> --- a/drivers/pch/Makefile
> +++ b/drivers/pch/Makefile
> @@ -3,3 +3,4 @@
>  #
>
>  obj-y += pch-uclass.o
> +obj-y += pch7.o
> diff --git a/drivers/pch/pch7.c b/drivers/pch/pch7.c
> new file mode 100644
> index 0000000..ef72422
> --- /dev/null
> +++ b/drivers/pch/pch7.c
> @@ -0,0 +1,61 @@
> +/*
> + * Copyright (C) 2014 Google, Inc
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <pch.h>
> +
> +#define BIOS_CTRL      0xd8
> +
> +static int pch7_get_sbase(struct udevice *dev, ulong *sbasep)
> +{
> +       u32 rcba;
> +
> +       dm_pci_read_config32(dev, PCH_RCBA, &rcba);
> +       /* Bits 31-14 are the base address, 13-1 are reserved, 0 is enable */
> +       rcba = rcba & 0xffffc000;
> +       *sbasep = rcba + 0x3020;
> +
> +       return 0;
> +}
> +
> +static enum pch_version pch7_get_version(struct udevice *dev)
> +{
> +       return PCHV_7;
> +}
> +
> +static int pch7_set_spi_protect(struct udevice *dev, bool protect)
> +{
> +       uint8_t bios_cntl;
> +
> +       /* Adjust the BIOS write protect to dis/allow write commands */
> +       dm_pci_read_config8(dev, BIOS_CTRL, &bios_cntl);
> +       if (protect)
> +               bios_cntl &= ~BIOS_CTRL_BIOSWE;
> +       else
> +               bios_cntl |= BIOS_CTRL_BIOSWE;
> +       dm_pci_write_config8(dev, BIOS_CTRL, bios_cntl);
> +
> +       return 0;
> +}
> +
> +static const struct pch_ops pch7_ops = {
> +       .get_sbase      = pch7_get_sbase,
> +       .get_version    = pch7_get_version,
> +       .set_spi_protect = pch7_set_spi_protect,
> +};
> +
> +static const struct udevice_id pch7_ids[] = {
> +       { .compatible = "intel,pch7" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(pch7_drv) = {
> +       .name           = "intel-pch7",
> +       .id             = UCLASS_PCH,
> +       .of_match       = pch7_ids,
> +       .ops            = &pch7_ops,
> +};
> diff --git a/include/pch.h b/include/pch.h
> index ff26865..dbfa265 100644
> --- a/include/pch.h
> +++ b/include/pch.h
> @@ -14,6 +14,14 @@ enum pch_version {
>         PCHV_9,
>  };
>
> +enum {
> +       PCH_RCBA        = 0xf0,

Normally we don't declare register offset using enum, no?

> +};
> +
> +enum {
> +       BIOS_CTRL_BIOSWE        = BIT(0),

Neither does bit field I think.

> +};
> +
>  /* Operations for the Platform Controller Hub */
>  struct pch_ops {
>         /**
> --

Other than that,

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Regards,
Bin

Hi Bin,

On 17 January 2016 at 23:12, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Sun, Jan 17, 2016 at 7:44 AM, Simon Glass <sjg@chromium.org> wrote:
>> At some point we may need to distinguish between different types of PCHs,
>> but for existing supported platforms we only need to worry about version 7
>> and version 9 bridges. Add a driver for the PCH7.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> Changes in v4:
>> - Correct BIOS_CTRL address for PCH7
>>
>> Changes in v3: None
>> Changes in v2:
>> - Rename the PCH functions
>> - Update the get_version() handle to use an enum
>> - Add a function to obtain the SPI base address
>> - Add enums for BIOS_CTRL register and bits
>>
>>  drivers/pch/Makefile |  1 +
>>  drivers/pch/pch7.c   | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/pch.h        |  8 +++++++
>>  3 files changed, 70 insertions(+)
>>  create mode 100644 drivers/pch/pch7.c
>>
>> diff --git a/drivers/pch/Makefile b/drivers/pch/Makefile
>> index d69a99c..33aa727 100644
>> --- a/drivers/pch/Makefile
>> +++ b/drivers/pch/Makefile
>> @@ -3,3 +3,4 @@
>>  #
>>
>>  obj-y += pch-uclass.o
>> +obj-y += pch7.o
>> diff --git a/drivers/pch/pch7.c b/drivers/pch/pch7.c
>> new file mode 100644
>> index 0000000..ef72422
>> --- /dev/null
>> +++ b/drivers/pch/pch7.c
>> @@ -0,0 +1,61 @@
>> +/*
>> + * Copyright (C) 2014 Google, Inc
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <pch.h>
>> +
>> +#define BIOS_CTRL      0xd8
>> +
>> +static int pch7_get_sbase(struct udevice *dev, ulong *sbasep)
>> +{
>> +       u32 rcba;
>> +
>> +       dm_pci_read_config32(dev, PCH_RCBA, &rcba);
>> +       /* Bits 31-14 are the base address, 13-1 are reserved, 0 is enable */
>> +       rcba = rcba & 0xffffc000;
>> +       *sbasep = rcba + 0x3020;
>> +
>> +       return 0;
>> +}
>> +
>> +static enum pch_version pch7_get_version(struct udevice *dev)
>> +{
>> +       return PCHV_7;
>> +}
>> +
>> +static int pch7_set_spi_protect(struct udevice *dev, bool protect)
>> +{
>> +       uint8_t bios_cntl;
>> +
>> +       /* Adjust the BIOS write protect to dis/allow write commands */
>> +       dm_pci_read_config8(dev, BIOS_CTRL, &bios_cntl);
>> +       if (protect)
>> +               bios_cntl &= ~BIOS_CTRL_BIOSWE;
>> +       else
>> +               bios_cntl |= BIOS_CTRL_BIOSWE;
>> +       dm_pci_write_config8(dev, BIOS_CTRL, bios_cntl);
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct pch_ops pch7_ops = {
>> +       .get_sbase      = pch7_get_sbase,
>> +       .get_version    = pch7_get_version,
>> +       .set_spi_protect = pch7_set_spi_protect,
>> +};
>> +
>> +static const struct udevice_id pch7_ids[] = {
>> +       { .compatible = "intel,pch7" },
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(pch7_drv) = {
>> +       .name           = "intel-pch7",
>> +       .id             = UCLASS_PCH,
>> +       .of_match       = pch7_ids,
>> +       .ops            = &pch7_ops,
>> +};
>> diff --git a/include/pch.h b/include/pch.h
>> index ff26865..dbfa265 100644
>> --- a/include/pch.h
>> +++ b/include/pch.h
>> @@ -14,6 +14,14 @@ enum pch_version {
>>         PCHV_9,
>>  };
>>
>> +enum {
>> +       PCH_RCBA        = 0xf0,
>
> Normally we don't declare register offset using enum, no?

I prefer them as a way to group values and also to make them show up
in the debugger. But if you really don't like it I can change it.

>
>> +};
>> +
>> +enum {
>> +       BIOS_CTRL_BIOSWE        = BIT(0),
>
> Neither does bit field I think.
>
>> +};
>> +
>>  /* Operations for the Platform Controller Hub */
>>  struct pch_ops {
>>         /**
>> --
>
> Other than that,
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

>
> Regards,
> Bin

Regards.
Simon

Hi Simon,

On Tue, Jan 19, 2016 at 11:09 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 17 January 2016 at 23:12, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Simon,
>>
>> On Sun, Jan 17, 2016 at 7:44 AM, Simon Glass <sjg@chromium.org> wrote:
>>> At some point we may need to distinguish between different types of PCHs,
>>> but for existing supported platforms we only need to worry about version 7
>>> and version 9 bridges. Add a driver for the PCH7.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>> Changes in v4:
>>> - Correct BIOS_CTRL address for PCH7
>>>
>>> Changes in v3: None
>>> Changes in v2:
>>> - Rename the PCH functions
>>> - Update the get_version() handle to use an enum
>>> - Add a function to obtain the SPI base address
>>> - Add enums for BIOS_CTRL register and bits
>>>
>>>  drivers/pch/Makefile |  1 +
>>>  drivers/pch/pch7.c   | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  include/pch.h        |  8 +++++++
>>>  3 files changed, 70 insertions(+)
>>>  create mode 100644 drivers/pch/pch7.c
>>>
>>> diff --git a/drivers/pch/Makefile b/drivers/pch/Makefile
>>> index d69a99c..33aa727 100644
>>> --- a/drivers/pch/Makefile
>>> +++ b/drivers/pch/Makefile
>>> @@ -3,3 +3,4 @@
>>>  #
>>>
>>>  obj-y += pch-uclass.o
>>> +obj-y += pch7.o
>>> diff --git a/drivers/pch/pch7.c b/drivers/pch/pch7.c
>>> new file mode 100644
>>> index 0000000..ef72422
>>> --- /dev/null
>>> +++ b/drivers/pch/pch7.c
>>> @@ -0,0 +1,61 @@
>>> +/*
>>> + * Copyright (C) 2014 Google, Inc
>>> + *
>>> + * SPDX-License-Identifier:    GPL-2.0+
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <dm.h>
>>> +#include <pch.h>
>>> +
>>> +#define BIOS_CTRL      0xd8
>>> +
>>> +static int pch7_get_sbase(struct udevice *dev, ulong *sbasep)
>>> +{
>>> +       u32 rcba;
>>> +
>>> +       dm_pci_read_config32(dev, PCH_RCBA, &rcba);
>>> +       /* Bits 31-14 are the base address, 13-1 are reserved, 0 is enable */
>>> +       rcba = rcba & 0xffffc000;
>>> +       *sbasep = rcba + 0x3020;
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static enum pch_version pch7_get_version(struct udevice *dev)
>>> +{
>>> +       return PCHV_7;
>>> +}
>>> +
>>> +static int pch7_set_spi_protect(struct udevice *dev, bool protect)
>>> +{
>>> +       uint8_t bios_cntl;
>>> +
>>> +       /* Adjust the BIOS write protect to dis/allow write commands */
>>> +       dm_pci_read_config8(dev, BIOS_CTRL, &bios_cntl);
>>> +       if (protect)
>>> +               bios_cntl &= ~BIOS_CTRL_BIOSWE;
>>> +       else
>>> +               bios_cntl |= BIOS_CTRL_BIOSWE;
>>> +       dm_pci_write_config8(dev, BIOS_CTRL, bios_cntl);
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static const struct pch_ops pch7_ops = {
>>> +       .get_sbase      = pch7_get_sbase,
>>> +       .get_version    = pch7_get_version,
>>> +       .set_spi_protect = pch7_set_spi_protect,
>>> +};
>>> +
>>> +static const struct udevice_id pch7_ids[] = {
>>> +       { .compatible = "intel,pch7" },
>>> +       { }
>>> +};
>>> +
>>> +U_BOOT_DRIVER(pch7_drv) = {
>>> +       .name           = "intel-pch7",
>>> +       .id             = UCLASS_PCH,
>>> +       .of_match       = pch7_ids,
>>> +       .ops            = &pch7_ops,
>>> +};
>>> diff --git a/include/pch.h b/include/pch.h
>>> index ff26865..dbfa265 100644
>>> --- a/include/pch.h
>>> +++ b/include/pch.h
>>> @@ -14,6 +14,14 @@ enum pch_version {
>>>         PCHV_9,
>>>  };
>>>
>>> +enum {
>>> +       PCH_RCBA        = 0xf0,
>>
>> Normally we don't declare register offset using enum, no?
>
> I prefer them as a way to group values and also to make them show up
> in the debugger. But if you really don't like it I can change it.
>

But not all register offsets are listed here. We still have "#define
BIOS_CTRL 0xd8" in pch7.c. So maybe we should still make it a define.

>>
>>> +};
>>> +
>>> +enum {
>>> +       BIOS_CTRL_BIOSWE        = BIT(0),
>>
>> Neither does bit field I think.
>>
>>> +};
>>> +
>>>  /* Operations for the Platform Controller Hub */
>>>  struct pch_ops {
>>>         /**
>>> --
>>
>> Other than that,
>>
>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>

Regards,
Bin