[U-Boot] [PATCH v2 09/13] pci: Use common functions to read/write config

[Bin Meng]

Hi Simon,

On Sun, Nov 22, 2015 at 7:28 AM, Simon Glass <sjg at chromium.org> wrote:
> Currently we use switch() and access PCI configuration via several
> functions, one for each data size. Adjust the code to use generic functions,
> where the data size is a parameter.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> Changes in v2:
> - Add back the leading 0 in the printf() statements
>
>  common/cmd_pci.c | 49 +++++++++++++++----------------------------------
>  1 file changed, 15 insertions(+), 34 deletions(-)
>
> diff --git a/common/cmd_pci.c b/common/cmd_pci.c
> index 7b83c1d..51e1f1c 100644
> --- a/common/cmd_pci.c
> +++ b/common/cmd_pci.c
> @@ -325,7 +325,8 @@ static pci_dev_t get_pci_dev(char *name)
>         return PCI_BDF(bdfs[0], bdfs[1], bdfs[2]);
>  }
>
> -static int pci_cfg_display(pci_dev_t bdf, ulong addr, ulong size, ulong length)
> +static int pci_cfg_display(pci_dev_t bdf, ulong addr, enum pci_size_t size,

Here parameter 'size' has been changed from 1/2/4 to type enum
pci_size_t whose valid number is 0/1/2.

> +                          ulong length)
>  {
>  #define DISP_LINE_LEN  16
>         ulong i, nbytes, linebytes;

The following two lines will cause divide by zero exception when size is 0.

335         if (length == 0)
336                 length = 0x40 / size; /* Standard PCI configuration space */

> @@ -339,23 +340,13 @@ static int pci_cfg_display(pci_dev_t bdf, ulong addr, ulong size, ulong length)
>          */
>         nbytes = length * size;

And here nbytes is not correct too.

>         do {
> -               uint    val4;
> -               ushort  val2;
> -               u_char  val1;
> -
>                 printf("%08lx:", addr);
>                 linebytes = (nbytes>DISP_LINE_LEN)?DISP_LINE_LEN:nbytes;
>                 for (i=0; i<linebytes; i+= size) {
> -                       if (size == 4) {
> -                               pci_read_config_dword(bdf, addr, &val4);
> -                               printf(" %08x", val4);
> -                       } else if (size == 2) {
> -                               pci_read_config_word(bdf, addr, &val2);
> -                               printf(" %04x", val2);
> -                       } else {
> -                               pci_read_config_byte(bdf, addr, &val1);
> -                               printf(" %02x", val1);
> -                       }
> +                       unsigned long val;
> +
> +                       val = pci_read_config(bdf, addr, size);
> +                       printf(" %0*lx", pci_field_width(size), val);
>                         addr += size;
>                 }
>                 printf("\n");
> @@ -385,32 +376,20 @@ static int pci_cfg_write (pci_dev_t bdf, ulong addr, ulong size, ulong value)
>         return 0;
>  }
>
> -static int
> -pci_cfg_modify (pci_dev_t bdf, ulong addr, ulong size, ulong value, int incrflag)
> +static int pci_cfg_modify(pci_dev_t bdf, ulong addr, enum pci_size_t size,
> +                         ulong value, int incrflag)
>  {
>         ulong   i;
>         int     nbytes;
> -       uint    val4;
> -       ushort  val2;
> -       u_char  val1;
> +       ulong val;
>
>         /* Print the address, followed by value.  Then accept input for
>          * the next value.  A non-converted value exits.
>          */
>         do {
>                 printf("%08lx:", addr);
> -               if (size == 4) {
> -                       pci_read_config_dword(bdf, addr, &val4);
> -                       printf(" %08x", val4);
> -               }
> -               else if (size == 2) {
> -                       pci_read_config_word(bdf, addr, &val2);
> -                       printf(" %04x", val2);
> -               }
> -               else {
> -                       pci_read_config_byte(bdf, addr, &val1);
> -                       printf(" %02x", val1);
> -               }
> +               val = pci_read_config(bdf, addr, size);
> +               printf(" %0*lx", pci_field_width(size), val);
>
>                 nbytes = cli_readline(" ? ");
>                 if (nbytes == 0 || (nbytes == 1 && console_buffer[0] == '-')) {
> @@ -456,7 +435,8 @@ pci_cfg_modify (pci_dev_t bdf, ulong addr, ulong size, ulong value, int incrflag
>   */
>  static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  {
> -       ulong addr = 0, value = 0, size = 0;
> +       ulong addr = 0, value = 0, cmd_size = 0;
> +       enum pci_size_t size;
>         int busnum = 0;
>         pci_dev_t bdf = 0;
>         char cmd = 's';
> @@ -471,7 +451,8 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>         case 'm':               /* modify */
>         case 'w':               /* write */
>                 /* Check for a size specification. */
> -               size = cmd_get_data_size(argv[1], 4);
> +               cmd_size = cmd_get_data_size(argv[1], 4);
> +               size = (cmd_size == 4) ? PCI_SIZE_32 : size - 1;

This is wrong, should be:

size = (cmd_size == 4) ? PCI_SIZE_32 : cmd_size - 1;

>                 if (argc > 3)
>                         addr = simple_strtoul(argv[3], NULL, 16);
>                 if (argc > 4)
> --

Regards,
Bin