[flashrom] [PATCH] add support for Toshiba Satellite P25

Idwer Vollering vidwer at gmail.com
Sat Apr 23 01:47:22 CEST 2011


2011/4/22 Brandon Dowdy <brandonrd7 at gmail.com>:
> This patch

looks like a mess, the way it is currently indented (I'm blaming the
gobby suite).

> adds support for the Toshiba Satellite P25 series.

Yay, more support for laptops.

Do you have any proof that it (probe and read) works on all versions
of the Satellite P25 ?

> board_enable.c has been reconfigured to fix compilation errors.
No, this is not the idea of integrating a patch. When you submit a
patch, it should compile.

This is how this patch should look like, comments are inline:

> Index: board_enable.c
> ===================================================================
> --- board_enable.c (revision 1288)
> +++ board_enable.c (working copy)
> @@ -24,6 +24,7 @@
>  * Contains the board specific flash enables.
>  */
>
> +#include <unistd.h>
> #include <string.h>
> #include "flash.h"
> #include "programmer.h"
> @@ -1859,6 +1860,142 @@
> #endif
>
> /*
> + * Only for the Toshiba Satellite P25-S520 (not tested, but possibly for other P25 Series laptops as well)

Contradictio in terminis: "only" and "possibly (..) others (..) as well".

> + */
> +
> +static int wait_uc_writebuf_empty(uint16_t port)
> +{
> +    int count;
> +
> +    for (count = 0; count < 50; count++)
> +    {
> +        /* quit if OBF bit clear */
> +        if (!(INB(port + 4) & 2))
> +        { /* when you're executing more than one line of code inside an if block, you need to surround the code inside an if block with accolades: {} */

Remove the comment about the if block.

> +            msg_pinfo("EC write buffer is already empty.\n");
> +            return 0;
> +        }
> +        usleep(10000);
> +    }
> +    msg_perr("wait_uc_writebuf_empty returned false or EC write buffer not empty.\n");
> +    return -1;
> +}
> +
> +static int uc_send_command(uint16_t port, uint8_t command)
> +{
> +       if (wait_uc_writebuf_empty(port) != 0)
> +               return -1;
> +       OUTB(command, port + 4);
> +       return 0;
> +}
> +
> +static int uc_send_data(uint16_t port, uint8_t command)
> +{
> +       if (wait_uc_writebuf_empty(port) != 0)
> +               return -1;
> +       OUTB(command, port);
> +       return 0;
> +}
> +
> +static int compal_os_takeover_fan(void) // only for Toshiba P25-S520

Can probably be used for other hardware: remove the comment.

> +{
> +    if(uc_send_command(0x60, 0x59) != 0) // system state notification
> +    {

Maybe add a #define KBDC 0x60 ?

> +        msg_perr("Unable to send System State Notification.\n");
> +        return -1;
> +    }
> +
> +    if(uc_send_data(0x60, 0xA8) != 0) // disable EC fan control

#define KBDC 0x60

> +    {
> +        msg_perr("Unable to disable fan control by the EC.\n");
> +        return -1;
> +    }

Add a newline here..

> +    if(uc_send_command(0x60, 0x49) != 0) // thermal state notification

#define KBDC 0x60

> +    {
> +        msg_perr("Unable to send Thermal State Notification.\n");
> +        return -1;
> +    }

and here.

> +    if(uc_send_data(0x60, 0xA2) != 0) // set fans to level 2

#define KBDC 0x60

Level 2 means that the fan is set to high speed ?

> +    {
> +        msg_perr("Unable to set fans to high speed.\n");
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +static int compal_pc87591_flash_enable(void) // only for Toshiba P25-S520

No, can be re-used.

> +{
> +    int i;
> +    unsigned int flashcontrolbase;
> +
> +    /* select logical device 0xf: shared BIOS and flash control */
> +    sio_write(0x4C, 0x7, 0xF);
> +    sio_write(0x4C, 0x30, 0x1); /* activate processor access */
> +
> +    /* get I/O base address */
> +    flashcontrolbase = sio_read(0x4C, 0x61) | (sio_read(0x4C, 0x60) << 8);

#define KBDC 0x60

> +    msg_pinfo("flash control base address is 0x%x\n", flashcontrolbase);
> +    for(i = 0; i < 16; i++)
> +        OUTB(i * 0x10, flashcontrolbase+8); /* remove protection of flash segment i */

For readability: OUTB(i * 0x10, flashcontrolbase + 8);

> +
> +    if(uc_send_command(0x60, 0x59) != 0) // system state notification
> +    {

#define KBDC 0x60

> +        msg_perr("Unable to send System State Notification.\n");
> +        return -1;
> +    }
> +
> +    if(uc_send_data(0x60, 0xF2) != 0) // enter flash mode

#define KBDC 0x60

> +    {
> +        msg_perr("Unable to put the EC into flash mode.\n");
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +static int intel_ich_gpio25_lower(void)
> +{
> +       return intel_ich_gpio_set(25, 0);
> +}
> +
> +static void toshiba_flashmode_exit(void * cmdptr) // only for Toshiba P25-S520

Can likely be re-used.

> +{
> +       if(uc_send_command(0x60, 0x49) != 0) // thermal state notification

#define KBDC 0x60

> +       {
> +           msg_perr("Unable to send Thermal State Notification.\n");
> +           return;
> +       }
> +
> +       if(uc_send_data(0x60, 0xA1) != 0) // set fans back to level 1

#define KBDC 0x60

> +       {
> +           msg_perr("Unable to set fans back to level 1.\n");

"level 1" (and "level 2") are too vague.

> +           return;
> +       }
> +}
> +
> +static int board_toshiba_satellite_s520(void) // Toshiba P25-S520 laptop

Drop the comment, as it is obvious where it is going to be used for.

> +{
> +    if(compal_os_takeover_fan() != 0)
> +    {
> +        msg_perr("compal_os_takeover_fan() is not returning 0.\n");
> +        return -1;
> +    }
> +
> +    if(compal_pc87591_flash_enable() != 0)
> +    {
> +        msg_perr("compal_pc87591_flash_enable() is not returning 0.\n");
> +        return -1;
> +    }
> +
> +    if (intel_ich_gpio25_lower() != 0)
> + return -1;
> +
> +    register_shutdown(toshiba_flashmode_exit, NULL);
> +    buses_supported = CHIP_BUSTYPE_PARALLEL;
> +    return 0;
> +}
> +
> +/*
>  * Below is the list of boards which need a special "board enable" code in
>  * flashrom before their ROM chip can be accessed/written to.
>  *
> @@ -2000,6 +2137,7 @@
> {0x1106, 0x3038, 0x0925, 0x1234,  0x1106, 0x3058, 0x15DD, 0x7609, NULL,          NULL,         NULL,          "Soyo",        "SY-7VCA",               0,   OK, via_apollo_gpo0_lower},
> {0x1106, 0x3038, 0x0925, 0x1234,  0x1106, 0x0596, 0x1106,      0, NULL,          NULL,         NULL,          "Tekram",      "P6Pro-A5",              256, OK, NULL},
> {0x1106, 0x3123, 0x1106, 0x3123,  0x1106, 0x3059, 0x1106, 0x4161, NULL,          NULL,         NULL,          "Termtek",     "TK-3370 (Rev:2.5B)",    0,   OK, w836xx_memw_enable_4e},
> + {0x8086, 0x24d0, 0,      0,       0x8086, 0x24d3, 0x1179, 0xff00, NULL,          NULL,         NULL,          "COMPAL",      "BTQ00",                 0,   NT, board_toshiba_satellite_s520},
> {0x8086, 0x1076, 0x8086, 0x1176,  0x1106, 0x3059, 0x10f1, 0x2498, NULL,          NULL,         NULL,          "Tyan",        "S2498 (Tomcat K7M)",    0,   OK, w836xx_memw_enable_2e},
> {0x1106, 0x0259, 0x1106, 0xAA07,  0x1106, 0x3227, 0x1106, 0xAA07, NULL,          NULL,         NULL,          "VIA",         "EPIA EK",               0,   NT, via_vt823x_gpio9_raise},
> {0x1106, 0x3177, 0x1106, 0xAA01,  0x1106, 0x3123, 0x1106, 0xAA01, NULL,          NULL,         NULL,          "VIA",         "EPIA M/MII/...",        0,   OK, via_vt823x_gpio15_raise},

> Thanks to roysjosh, twice11, idwer and others who I forgotten for reverse
> engineering the board enable and helping me and helping me fix the problems
> that can be major.
> If it hasn't been for those people, that laptop shouldn't have been
> supported by flashrom. :)

Where are the lines that change the (C)opyright ?

>
> Signed-off-by: Brandon Dowdy <brandonrd7 at gmail.com>

Signing off for code that you haven't written ? Oh wow.

Idwer

>
> _______________________________________________
> flashrom mailing list
> flashrom at flashrom.org
> http://www.flashrom.org/mailman/listinfo/flashrom
>




More information about the flashrom mailing list