[PATCH 1/5] rayer_spi: Improve support for different pinouts
Create a list of programmer types with names. This list could be listed with flashrom -L in follow-up patches. Handle a bit in status register that is inverted, this will be used in different future programmer types. I did trivial change after receiving the Tested-by. Local variable storage is declared static and bit offset in register is uint8_t. Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Tested-by: Maksim Kuleshov <mmcx@mail.ru> Acked-by: Kyösti Mälkki <kyosti.malkki@gmail.com> --- rayer_spi.c | 106 ++++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 61 insertions(+), 45 deletions(-) diff --git a/rayer_spi.c b/rayer_spi.c index b312610..cbbd5ce 100644 --- a/rayer_spi.c +++ b/rayer_spi.c @@ -37,21 +37,48 @@ #include "programmer.h" #include "hwaccess.h" -enum rayer_type { - TYPE_RAYER, - TYPE_XILINX_DLC5, -}; - /* We have two sets of pins, out and in. The numbers for both sets are * independent and are bitshift values, not real pin numbers. * Default settings are for the RayeR hardware. */ -/* Pins for master->slave direction */ -static int rayer_cs_bit = 5; -static int rayer_sck_bit = 6; -static int rayer_mosi_bit = 7; -/* Pins for slave->master direction */ -static int rayer_miso_bit = 6; + +struct rayer_programmer { + const char * type; + const enum test_state status; + const char * description; + const void * private; +}; + +struct rayer_pinout { + uint8_t cs_bit; + uint8_t sck_bit; + uint8_t mosi_bit; + uint8_t miso_bit; + void (*preinit)(void *); + int (*shutdown)(void *); +}; + +static struct rayer_pinout rayer_spipgm = { + .cs_bit = 5, + .sck_bit = 6, + .mosi_bit = 7, + .miso_bit = 6, +}; + +static struct rayer_pinout xilinx_dlc5 = { + .cs_bit = 2, + .sck_bit = 1, + .mosi_bit = 0, + .miso_bit = 4, +}; + +static struct rayer_programmer rayer_spi_types[] = { + {"rayer", NT, "RayeR SPIPGM", &rayer_spipgm}, + {"xilinx", NT, "Xilinx Parallel Cable III (DLC 5)", &xilinx_dlc5}, + {0}, +}; + +static struct rayer_pinout *pinout = NULL; static uint16_t lpt_iobase; @@ -60,22 +87,22 @@ static uint8_t lpt_outbyte; static void rayer_bitbang_set_cs(int val) { - lpt_outbyte &= ~(1 << rayer_cs_bit); - lpt_outbyte |= (val << rayer_cs_bit); + lpt_outbyte &= ~(1 << pinout->cs_bit); + lpt_outbyte |= (val << pinout->cs_bit); OUTB(lpt_outbyte, lpt_iobase); } static void rayer_bitbang_set_sck(int val) { - lpt_outbyte &= ~(1 << rayer_sck_bit); - lpt_outbyte |= (val << rayer_sck_bit); + lpt_outbyte &= ~(1 << pinout->sck_bit); + lpt_outbyte |= (val << pinout->sck_bit); OUTB(lpt_outbyte, lpt_iobase); } static void rayer_bitbang_set_mosi(int val) { - lpt_outbyte &= ~(1 << rayer_mosi_bit); - lpt_outbyte |= (val << rayer_mosi_bit); + lpt_outbyte &= ~(1 << pinout->mosi_bit); + lpt_outbyte |= (val << pinout->mosi_bit); OUTB(lpt_outbyte, lpt_iobase); } @@ -83,8 +110,8 @@ static int rayer_bitbang_get_miso(void) { uint8_t tmp; - tmp = INB(lpt_iobase + 1); - tmp = (tmp >> rayer_miso_bit) & 0x1; + tmp = INB(lpt_iobase + 1) ^ 0x80; // bit.7 inverted + tmp = (tmp >> pinout->miso_bit) & 0x1; return tmp; } @@ -99,8 +126,8 @@ static const struct bitbang_spi_master bitbang_spi_master_rayer = { int rayer_spi_init(void) { + struct rayer_programmer *prog = rayer_spi_types; char *arg = NULL; - enum rayer_type rayer_type = TYPE_RAYER; /* Non-default port requested? */ arg = extract_programmer_param("iobase"); @@ -138,36 +165,20 @@ int rayer_spi_init(void) arg = extract_programmer_param("type"); if (arg) { - if (!strcasecmp(arg, "rayer")) { - rayer_type = TYPE_RAYER; - } else if (!strcasecmp(arg, "xilinx")) { - rayer_type = TYPE_XILINX_DLC5; - } else { + for (; prog->type; ++prog) { + if (! strcasecmp (arg, prog->type)) { + break; + } + } + if(! prog->type) { msg_perr("Error: Invalid device type specified.\n"); free(arg); return 1; } + free(arg); } - free(arg); - switch (rayer_type) { - case TYPE_RAYER: - msg_pdbg("Using RayeR SPIPGM pinout.\n"); - /* Bits for master->slave direction */ - rayer_cs_bit = 5; - rayer_sck_bit = 6; - rayer_mosi_bit = 7; - /* Bits for slave->master direction */ - rayer_miso_bit = 6; - break; - case TYPE_XILINX_DLC5: - msg_pdbg("Using Xilinx Parallel Cable III (DLC 5) pinout.\n"); - /* Bits for master->slave direction */ - rayer_cs_bit = 2; - rayer_sck_bit = 1; - rayer_mosi_bit = 0; - /* Bits for slave->master direction */ - rayer_miso_bit = 4; - } + msg_pinfo("Using %s pinout.\n", prog->description); + pinout = (struct rayer_pinout *) prog->private; if (rget_io_perms()) return 1; @@ -175,6 +186,11 @@ int rayer_spi_init(void) /* Get the initial value before writing to any line. */ lpt_outbyte = INB(lpt_iobase); + if (pinout->shutdown) + register_shutdown(pinout->shutdown, (void*)pinout); + if (pinout->preinit) + pinout->preinit(pinout); + if (bitbang_spi_init(&bitbang_spi_master_rayer)) return 1; -- 1.8.1.1
Pin 6 on LPT controls a pulldown on MISO/TDO signal. Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Acked-by: Kyösti Mälkki <kyosti.malkki@gmail.com> --- rayer_spi.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/rayer_spi.c b/rayer_spi.c index cbbd5ce..28e8c9c 100644 --- a/rayer_spi.c +++ b/rayer_spi.c @@ -72,9 +72,22 @@ static struct rayer_pinout xilinx_dlc5 = { .miso_bit = 4, }; +static void dlc5b_preinit(void *); +static int dlc5b_shutdown(void *); + +static struct rayer_pinout xilinx_dlc5b = { + .cs_bit = 2, + .sck_bit = 1, + .mosi_bit = 0, + .miso_bit = 4, + .preinit = dlc5b_preinit, + .shutdown = dlc5b_shutdown, +}; + static struct rayer_programmer rayer_spi_types[] = { {"rayer", NT, "RayeR SPIPGM", &rayer_spipgm}, {"xilinx", NT, "Xilinx Parallel Cable III (DLC 5)", &xilinx_dlc5}, + {"dlc-5b", NT, "Xilinx Parallel Cable III (DLC 5) (buffered)", &xilinx_dlc5b}, {0}, }; @@ -197,6 +210,21 @@ int rayer_spi_init(void) return 0; } +static void dlc5b_preinit(void * data) { + msg_pdbg("dlc5b_preinit\n"); + /* Assert pin 6 to receive MISO. */ + lpt_outbyte |= (1<<4); + OUTB(lpt_outbyte, lpt_iobase); +} + +static int dlc5b_shutdown(void * data) { + msg_pdbg("dlc5b_shutdown\n"); + /* De-assert pin 6 to force MISO low. */ + lpt_outbyte &= ~(1<<4); + OUTB(lpt_outbyte, lpt_iobase); + return 0; +} + #else #error PCI port I/O access is not supported on this architecture yet. #endif -- 1.8.1.1
From: "mmcx@mail.ru" <mmcx@mail.ru> Renamed type to byteblastermv after receiving Tested-by. There is ByteBlasterII product that is only almost compatible. Signed-off-by: Maksim Kuleshov <mmcx@mail.ru> Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Tested-by: Maksim Kuleshov <mmcx@mail.ru> Acked-by: Kyösti Mälkki <kyosti.malkki@gmail.com> --- rayer_spi.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/rayer_spi.c b/rayer_spi.c index 28e8c9c..28336c6 100644 --- a/rayer_spi.c +++ b/rayer_spi.c @@ -84,10 +84,23 @@ static struct rayer_pinout xilinx_dlc5b = { .shutdown = dlc5b_shutdown, }; +static void byteblaster_preinit(void *); +static int byteblaster_shutdown(void *); + +static struct rayer_pinout altera_byteblastermv = { + .cs_bit = 1, + .sck_bit = 0, + .mosi_bit = 6, + .miso_bit = 7, + .preinit = byteblaster_preinit, + .shutdown = byteblaster_shutdown, +}; + static struct rayer_programmer rayer_spi_types[] = { {"rayer", NT, "RayeR SPIPGM", &rayer_spipgm}, {"xilinx", NT, "Xilinx Parallel Cable III (DLC 5)", &xilinx_dlc5}, {"dlc-5b", NT, "Xilinx Parallel Cable III (DLC 5) (buffered)", &xilinx_dlc5b}, + {"byteblastermv", OK, "Altera ByteBlasterMV", &altera_byteblastermv}, {0}, }; @@ -225,6 +238,19 @@ static int dlc5b_shutdown(void * data) { return 0; } +static void byteblaster_preinit(void * data){ + msg_pdbg("byteblaster_preinit\n"); + /* Assert #EN signal. */ + OUTB(2, lpt_iobase + 2 ); +} + +static int byteblaster_shutdown(void * data){ + msg_pdbg("byteblaster_shutdown\n"); + /* De-Assert #EN signal. */ + OUTB(0, lpt_iobase + 2 ); + return 0; +} + #else #error PCI port I/O access is not supported on this architecture yet. #endif -- 1.8.1.1
From: "mmcx@mail.ru" <mmcx@mail.ru> Signed-off-by: Maksim Kuleshov <mmcx@mail.ru> Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Acked-by: Kyösti Mälkki <kyosti.malkki@gmail.com> --- rayer_spi.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/rayer_spi.c b/rayer_spi.c index 28336c6..56ae830 100644 --- a/rayer_spi.c +++ b/rayer_spi.c @@ -96,11 +96,24 @@ static struct rayer_pinout altera_byteblastermv = { .shutdown = byteblaster_shutdown, }; +static void stk200_preinit(void *); +static int stk200_shutdown(void *); + +static struct rayer_pinout atmel_stk200 = { + .cs_bit = 7, + .sck_bit = 4, + .mosi_bit = 5, + .miso_bit = 6, + .preinit = stk200_preinit, + .shutdown = stk200_shutdown, +}; + static struct rayer_programmer rayer_spi_types[] = { {"rayer", NT, "RayeR SPIPGM", &rayer_spipgm}, {"xilinx", NT, "Xilinx Parallel Cable III (DLC 5)", &xilinx_dlc5}, {"dlc-5b", NT, "Xilinx Parallel Cable III (DLC 5) (buffered)", &xilinx_dlc5b}, {"byteblastermv", OK, "Altera ByteBlasterMV", &altera_byteblastermv}, + {"stk200", NT, "Atmel STK200/300 adapter", &atmel_stk200}, {0}, }; @@ -251,6 +264,21 @@ static int byteblaster_shutdown(void * data){ return 0; } +static void stk200_preinit(void *data) { + msg_pdbg("stk200_init\n"); + /* Assert #EN signals, set LED signal. */ + lpt_outbyte = (1 << 6) ; + OUTB(lpt_outbyte, lpt_iobase); +} + +static int stk200_shutdown(void *data) { + msg_pdbg("stk200_shutdown\n"); + /* Assert #EN signals, clear LED signal. */ + lpt_outbyte = (1 << 2) | (1 << 3); + OUTB(lpt_outbyte, lpt_iobase); + return 0; +} + #else #error PCI port I/O access is not supported on this architecture yet. #endif -- 1.8.1.1
From: "mmcx@mail.ru" <mmcx@mail.ru> Signed-off-by: Maksim Kuleshov <mmcx@mail.ru> Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Tested-by: Maksim Kuleshov <mmcx@mail.ru> Acked-by: Kyösti Mälkki <kyosti.malkki@gmail.com> --- rayer_spi.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rayer_spi.c b/rayer_spi.c index 56ae830..037bc12 100644 --- a/rayer_spi.c +++ b/rayer_spi.c @@ -108,12 +108,20 @@ static struct rayer_pinout atmel_stk200 = { .shutdown = stk200_shutdown, }; +static struct rayer_pinout wiggler_lpt = { + .cs_bit = 1, + .sck_bit = 2, + .mosi_bit = 3, + .miso_bit = 7, +}; + static struct rayer_programmer rayer_spi_types[] = { {"rayer", NT, "RayeR SPIPGM", &rayer_spipgm}, {"xilinx", NT, "Xilinx Parallel Cable III (DLC 5)", &xilinx_dlc5}, {"dlc-5b", NT, "Xilinx Parallel Cable III (DLC 5) (buffered)", &xilinx_dlc5b}, {"byteblastermv", OK, "Altera ByteBlasterMV", &altera_byteblastermv}, {"stk200", NT, "Atmel STK200/300 adapter", &atmel_stk200}, + {"wiggler", OK, "Wiggler LPT", &wiggler_lpt}, {0}, }; -- 1.8.1.1
Here is the same patch, with my suggested changes and some other stuff on top (constification, naming the Xilinx DLC-5 cable "dlc5" in anticipation of the buffered DLC-5 variant) to avoid changing things twice. I tried to dig up the history of this patch, hopefully I got it right. Am 01.04.2013 23:55 schrieb Kyösti Mälkki:
Create a list of programmer types with names. This list could be listed with flashrom -L in follow-up patches.
Handle a bit in status register that is inverted, this will be used in different future programmer types.
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Tested-by: Maksim Kuleshov <mmcx@mail.ru> Acked-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
rayer_spi: Rework handling of programmer types Store rayer_spi programmer types with configuration data in an array. Bit 7 of the LPT status register is inverted, automatically handle this for future users. The Xilinx DLC-5 cable is now selected with type=dlc-5 instead of dev=xilinx. Patch originally by Maksim Kuleshov, reworked by Kyösti Mälkki and Carl-Daniel Hailfinger. Maksim/Kyösti, can I get your signoff? Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> Index: flashrom-maksim_rayer_spi_rework_type_selection/rayer_spi.c =================================================================== --- flashrom-maksim_rayer_spi_rework_type_selection/rayer_spi.c (Revision 1667) +++ flashrom-maksim_rayer_spi_rework_type_selection/rayer_spi.c (Arbeitskopie) @@ -19,6 +19,7 @@ /* Driver for the SPIPGM hardware by "RayeR" Martin Rehak. * See http://rayer.ic.cz/elektro/spipgm.htm for schematics and instructions. + * Other LPT-based SPI programming hardware is supported as well. */ /* This driver uses non-portable direct I/O port accesses which won't work on @@ -37,22 +38,50 @@ #include "programmer.h" #include "hwaccess.h" -enum rayer_type { - TYPE_RAYER, - TYPE_XILINX_DLC5, -}; - /* We have two sets of pins, out and in. The numbers for both sets are * independent and are bitshift values, not real pin numbers. * Default settings are for the RayeR hardware. */ -/* Pins for master->slave direction */ -static int rayer_cs_bit = 5; -static int rayer_sck_bit = 6; -static int rayer_mosi_bit = 7; -/* Pins for slave->master direction */ -static int rayer_miso_bit = 6; +struct noid_dev_entry { + const char *type; + const enum test_state status; + const char *description; + const void *dev_data; +}; + +struct rayer_pinout { + uint8_t cs_bit; + uint8_t sck_bit; + uint8_t mosi_bit; + uint8_t miso_bit; + void (*init)(void *); + int (*shutdown)(void *); +}; + +static const struct rayer_pinout rayer_spipgm = { + .cs_bit = 5, + .sck_bit = 6, + .mosi_bit = 7, + .miso_bit = 6, +}; + +static const struct rayer_pinout xilinx_dlc5 = { + .cs_bit = 2, + .sck_bit = 1, + .mosi_bit = 0, + .miso_bit = 4, +}; + +/* List of supported devices, first one is the default. */ +static const struct noid_dev_entry rayer_spi_devs[] = { + {"rayer", NT, "RayeR SPIPGM", &rayer_spipgm}, + {"dlc-5", NT, "Xilinx Parallel Cable III (DLC 5)", &xilinx_dlc5}, + {0}, +}; + +static struct rayer_pinout *pinout = NULL; + static uint16_t lpt_iobase; /* Cached value of last byte sent. */ @@ -60,22 +89,22 @@ static void rayer_bitbang_set_cs(int val) { - lpt_outbyte &= ~(1 << rayer_cs_bit); - lpt_outbyte |= (val << rayer_cs_bit); + lpt_outbyte &= ~(1 << pinout->cs_bit); + lpt_outbyte |= (val << pinout->cs_bit); OUTB(lpt_outbyte, lpt_iobase); } static void rayer_bitbang_set_sck(int val) { - lpt_outbyte &= ~(1 << rayer_sck_bit); - lpt_outbyte |= (val << rayer_sck_bit); + lpt_outbyte &= ~(1 << pinout->sck_bit); + lpt_outbyte |= (val << pinout->sck_bit); OUTB(lpt_outbyte, lpt_iobase); } static void rayer_bitbang_set_mosi(int val) { - lpt_outbyte &= ~(1 << rayer_mosi_bit); - lpt_outbyte |= (val << rayer_mosi_bit); + lpt_outbyte &= ~(1 << pinout->mosi_bit); + lpt_outbyte |= (val << pinout->mosi_bit); OUTB(lpt_outbyte, lpt_iobase); } @@ -83,8 +112,8 @@ { uint8_t tmp; - tmp = INB(lpt_iobase + 1); - tmp = (tmp >> rayer_miso_bit) & 0x1; + tmp = INB(lpt_iobase + 1) ^ 0x80; // bit 7 is inverted + tmp = (tmp >> pinout->miso_bit) & 0x1; return tmp; } @@ -99,8 +128,9 @@ int rayer_spi_init(void) { + /* Pick the first entry in rayer_spi_devs as default. */ + const struct noid_dev_entry *dev = rayer_spi_devs; char *arg = NULL; - enum rayer_type rayer_type = TYPE_RAYER; /* Non-default port requested? */ arg = extract_programmer_param("iobase"); @@ -138,36 +168,18 @@ arg = extract_programmer_param("type"); if (arg) { - if (!strcasecmp(arg, "rayer")) { - rayer_type = TYPE_RAYER; - } else if (!strcasecmp(arg, "xilinx")) { - rayer_type = TYPE_XILINX_DLC5; - } else { - msg_perr("Error: Invalid device type specified.\n"); + for (; dev->type; ++dev) + if (!strcasecmp(arg, dev->type)) + break; + if(!dev->type) { + msg_perr("Error: Invalid device type \"%s\"specified.\n", arg); free(arg); return 1; } + free(arg); } - free(arg); - switch (rayer_type) { - case TYPE_RAYER: - msg_pdbg("Using RayeR SPIPGM pinout.\n"); - /* Bits for master->slave direction */ - rayer_cs_bit = 5; - rayer_sck_bit = 6; - rayer_mosi_bit = 7; - /* Bits for slave->master direction */ - rayer_miso_bit = 6; - break; - case TYPE_XILINX_DLC5: - msg_pdbg("Using Xilinx Parallel Cable III (DLC 5) pinout.\n"); - /* Bits for master->slave direction */ - rayer_cs_bit = 2; - rayer_sck_bit = 1; - rayer_mosi_bit = 0; - /* Bits for slave->master direction */ - rayer_miso_bit = 4; - } + msg_pinfo("Using %s pinout.\n", dev->description); + pinout = (struct rayer_pinout *) dev->dev_data; if (rget_io_perms()) return 1; @@ -175,6 +187,11 @@ /* Get the initial value before writing to any line. */ lpt_outbyte = INB(lpt_iobase); + if (pinout->shutdown) + register_shutdown(pinout->shutdown, pinout); + if (pinout->init) + pinout->init(pinout); + if (bitbang_spi_init(&bitbang_spi_master_rayer)) return 1; Index: flashrom-maksim_rayer_spi_rework_type_selection/flashrom.8 =================================================================== --- flashrom-maksim_rayer_spi_rework_type_selection/flashrom.8 (Revision 1667) +++ flashrom-maksim_rayer_spi_rework_type_selection/flashrom.8 (Arbeitskopie) @@ -715,7 +715,7 @@ syntax where .B model can be -.BR rayer " for the RayeR cable or " xilinx " for the Xilinx Parallel Cable III +.BR rayer " for the RayeR cable or " dlc-5 " for the Xilinx Parallel Cable III (DLC 5). .sp More information about the RayeR hardware is available at -- http://www.hailfinger.org/
On Sat, 2013-04-06 at 03:12 +0200, Carl-Daniel Hailfinger wrote:
Here is the same patch, with my suggested changes and some other stuff on top (constification, naming the Xilinx DLC-5 cable "dlc5" in anticipation of the buffered DLC-5 variant) to avoid changing things twice. I tried to dig up the history of this patch, hopefully I got it right.
Am 01.04.2013 23:55 schrieb Kyösti Mälkki:
Create a list of programmer types with names. This list could be listed with flashrom -L in follow-up patches.
Handle a bit in status register that is inverted, this will be used in different future programmer types.
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Tested-by: Maksim Kuleshov <mmcx@mail.ru> Acked-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
rayer_spi: Rework handling of programmer types
Store rayer_spi programmer types with configuration data in an array. Bit 7 of the LPT status register is inverted, automatically handle this for future users. The Xilinx DLC-5 cable is now selected with type=dlc-5 instead of dev=xilinx.
Patch originally by Maksim Kuleshov, reworked by Kyösti Mälkki and Carl-Daniel Hailfinger.
Maksim/Kyösti, can I get your signoff?
Why did You remove the Signed-off by lines from the patch in the first place? I thought there was a policy to only add at the end of Signed-off-by lines. And if you only do minimal rebase or rework, note that between Your own sign-off. Yes, you can return my Signed-off-by in there. Kyösti
Am 06.04.2013 05:37 schrieb Kyösti Mälkki:
On Sat, 2013-04-06 at 03:12 +0200, Carl-Daniel Hailfinger wrote:
Here is the same patch, with my suggested changes and some other stuff on top (constification, naming the Xilinx DLC-5 cable "dlc5" in anticipation of the buffered DLC-5 variant) to avoid changing things twice. I tried to dig up the history of this patch, hopefully I got it right.
Am 01.04.2013 23:55 schrieb Kyösti Mälkki:
Create a list of programmer types with names. This list could be listed with flashrom -L in follow-up patches.
Handle a bit in status register that is inverted, this will be used in different future programmer types.
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Tested-by: Maksim Kuleshov <mmcx@mail.ru> Acked-by: Kyösti Mälkki <kyosti.malkki@gmail.com> rayer_spi: Rework handling of programmer types
Store rayer_spi programmer types with configuration data in an array. Bit 7 of the LPT status register is inverted, automatically handle this for future users. The Xilinx DLC-5 cable is now selected with type=dlc-5 instead of dev=xilinx.
Patch originally by Maksim Kuleshov, reworked by Kyösti Mälkki and Carl-Daniel Hailfinger.
Maksim/Kyösti, can I get your signoff?
Why did You remove the Signed-off by lines from the patch in the first place? I thought there was a policy to only add at the end of Signed-off-by lines. And if you only do minimal rebase or rework, note that between Your own sign-off.
Indeed. But your comments in response to the "[flashrom] [PATCH] Support device lists for programmers without PCI/USB IDs" thread sounded like you didn't want to be associated with the rework I was doing, and I wanted to avoid a situation where your signoff is associated with a patch you don't like. Do you agree with the patch summary I posted above, or is it incorrect? I tried to dig up all the mails similar to this patch and hope I got the "original by/reworked by" comment right.
Yes, you can return my Signed-off-by in there.
Thanks. I'll wait for Maksim's signoff confirmation before committing. Regards, Carl-Daniel -- http://www.hailfinger.org/
Hi. I don't understand why change "xilinx" on "dlc-5". Changes command line , but does not change the behavior of the program. Users have worked well the current version, now there will be problems. Call flashrom can be integrated into the various programmes, and change the startup options can be very difficult. In the documents of the Xilinx name of the "dlc-5" is very rare. More often adapter is called "Parallel Cable III".It is probably better not to change the name of "xilinx".
Thanks. I'll wait for Maksim's signoff confirmation before committing.
I tested only the ByteblasterMV and Wiggler, but their support is not included in this patch. Maksim
On Sun, 7 Apr 2013 02:02:43 +0400 Maksim Kuleshov <mmcx@mail.ru> wrote:
I don't understand why change "xilinx" on "dlc-5". Changes command line , but does not change the behavior of the program. Users have worked well the current version, now there will be problems. Call flashrom can be integrated into the various programmes, and change the startup options can be very difficult. In the documents of the Xilinx name of the "dlc-5" is very rare. More often adapter is called "Parallel Cable III".It is probably better not to change the name of "xilinx".
Thanks for your considerations regarding the name, that's a valid point. Using some consistent scheme to differentiate the various types is favorable, therefore using the model name instead of the company name generally speaking makes sense. Also, I consider breaking applications that use the CLI instead of libflashrom a good thing. We should do that more often so that either they get so annoyed that their authors review the libflashrom patches or annoy us back enough so that we eventually integrate libflashrom. This was somewhat sarcastic, but my main point is: the CLI should not be used by other programs. Regarding users... that's an excellent point because I am not sure how well the help texts (manpage and --L output) are after this change. Carl-Daniel?
Thanks. I'll wait for Maksim's signoff confirmation before committing.
I tested only the ByteblasterMV and Wiggler, but their support is not included in this patch.
Signing off is about the code, not about testing or reviewing. It indicates that you were authorized to contribute that code under the given license to this project. And in this case Carl-Daniel implies with it general consent to what he has done with your code when refining the patches. I interpret your considerations regarding the type parameter as disagreement, but I might be wrong... -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
Thanks for your considerations regarding the name, that's a valid point. Using some consistent scheme to differentiate the various types is favorable, therefore using the model name instead of the company name generally speaking makes sense. Also, I consider breaking applications that use the CLI instead of libflashrom a good thing. We should do that more often so that either they get so annoyed that their authors review the libflashrom patches or annoy us back enough so that we eventually integrate libflashrom. This was somewhat sarcastic, but my main point is: the CLI should not be used by other programs. Regarding users... that's an excellent point because I am not sure how well the help texts (manpage and --L output) are after this change. Carl-Daniel?
Coercion, is always worse than the possible choices. To use the flashrom not need to be программистом. libflashrom requires C ABI compatibility. How to be with the integration of python, php, perl, ocaml, lisp, etc.? Who write for these language bindings, and will keep them up to date? A good practice in case of need to delete or change the name, is the creation of a new name as a synonym of the old, and the announcement of the old name as "deprecated".
Signing off is about the code, not about testing or reviewing. It indicates that you were authorized to contribute that code under the given license to this project. And in this case Carl-Daniel implies with it general consent to what he has done with your code when refining the patches. I interpret your considerations regarding the type parameter as disagreement, but I might be wrong...
I will sign an agreement with the license for this patch.
On Sat, 2013-04-06 at 19:08 +0200, Carl-Daniel Hailfinger wrote:
Am 06.04.2013 05:37 schrieb Kyösti Mälkki:
On Sat, 2013-04-06 at 03:12 +0200, Carl-Daniel Hailfinger wrote:
Here is the same patch, with my suggested changes and some other stuff on top (constification, naming the Xilinx DLC-5 cable "dlc5" in anticipation of the buffered DLC-5 variant) to avoid changing things twice. I tried to dig up the history of this patch, hopefully I got it right.
Am 01.04.2013 23:55 schrieb Kyösti Mälkki:
Create a list of programmer types with names. This list could be listed with flashrom -L in follow-up patches.
Handle a bit in status register that is inverted, this will be used in different future programmer types.
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Tested-by: Maksim Kuleshov <mmcx@mail.ru> Acked-by: Kyösti Mälkki <kyosti.malkki@gmail.com> rayer_spi: Rework handling of programmer types
Store rayer_spi programmer types with configuration data in an array. Bit 7 of the LPT status register is inverted, automatically handle this for future users. The Xilinx DLC-5 cable is now selected with type=dlc-5 instead of dev=xilinx.
Patch originally by Maksim Kuleshov, reworked by Kyösti Mälkki and Carl-Daniel Hailfinger.
Maksim/Kyösti, can I get your signoff?
Why did You remove the Signed-off by lines from the patch in the first place? I thought there was a policy to only add at the end of Signed-off-by lines. And if you only do minimal rebase or rework, note that between Your own sign-off.
Indeed. But your comments in response to the "[flashrom] [PATCH] Support device lists for programmers without PCI/USB IDs" thread sounded like you didn't want to be associated with the rework I was doing, and I wanted to avoid a situation where your signoff is associated with a patch you don't like.
I have submitted the patch with my Signed-off-by previously. The fact that I do not like the rework You have done here cannot change the "Chain of Trust" or "Certificate of Origin" the Signed-off-by procedure is about. More importantly: For further changes in this rayer_spi patchset, remove the Tested-By and Acked-By lines.
Do you agree with the patch summary I posted above, or is it incorrect? I tried to dig up all the mails similar to this patch and hope I got the "original by/reworked by" comment right.
Summary is fine.
Yes, you can return my Signed-off-by in there.
Thanks. I'll wait for Maksim's signoff confirmation before committing.
Maksim, would you keep track of the effort (in time) You need to put in re-testing the patchsets and commenting on ML.
Regards, Carl-Daniel
Thanks, Kyösti
I have finally committed this set in r1753-r1757 after refining a few details. The most important change to the original is the removal of the distinct buffered xilinx cable. There is no sign that there is an unbuffered version, hence only fix the existing one to handle the pulldown correctly. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
participants (4)
-
Carl-Daniel Hailfinger -
Kyösti Mälkki -
Maksim Kuleshov -
Stefan Tauner