From: Jeff Garzik <jeff@garzik.org>
To: Stefan Assmann <sassmann@redhat.com>
Cc: linux-pci@vger.kernel.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Jesse Barnes <jbarnes@virtuousgeek.org>,
Krzysztof Halasa <khc@pm.waw.pl>, Don Dutile <ddutile@redhat.com>,
kaneshige.kenji@jp.fujitsu.com
Subject: Re: [PATCH] change PCI nomenclature according to PCI-SIG
Date: Sat, 28 Nov 2009 07:43:08 -0500 [thread overview]
Message-ID: <4B111ADC.9020800@garzik.org> (raw)
In-Reply-To: <4B110F79.8080405@redhat.com>
On 11/28/2009 06:54 AM, Stefan Assmann wrote:
> From: Stefan Assmann<sassmann@redhat.com>
>
> Changing occurrences of variants of PCI-X and PCIe to the PCI-SIG
> terms listed in the "Trademark and Logo Usage Guidelines".
> http://www.pcisig.com/developers/procedures/logos/Trademark_and_Logo_Usage_Guidelines_updated_112206.pdf
> Additionally some renames of Gb/s to GT/s where appropriate, concerns PCIe.
>
> This is a followup to the discussion at:
> http://lkml.org/lkml/2009/10/14/107
> Patch is based on 2.6.32-rc8.
>
> Signed-off-by: Stefan Assmann<sassmann@redhat.com>
NAK, this clearly introduces bugs and changes sysfs output (ABI).
Typically this type of change is pointless churn that creates far more
problems than it "solves."
> diff --git a/drivers/edac/ppc4xx_edac.c b/drivers/edac/ppc4xx_edac.c
> index 11f2172..eda4fdf 100644
> --- a/drivers/edac/ppc4xx_edac.c
> +++ b/drivers/edac/ppc4xx_edac.c
> @@ -224,8 +224,8 @@ static const unsigned ppc4xx_edac_nr_chans = 1;
> */
> static const char * const ppc4xx_plb_masters[9] = {
> [SDRAM_PLB_M0ID_ICU] = "ICU",
> - [SDRAM_PLB_M0ID_PCIE0] = "PCI-E 0",
> - [SDRAM_PLB_M0ID_PCIE1] = "PCI-E 1",
> + [SDRAM_PLB_M0ID_PCIE0] = "PCIe 0",
> + [SDRAM_PLB_M0ID_PCIE1] = "PCIe 1",
> [SDRAM_PLB_M0ID_DMA] = "DMA",
> [SDRAM_PLB_M0ID_DCU] = "DCU",
> [SDRAM_PLB_M0ID_OPB] = "OPB",
This data is interpreted programmatically, as the comments at its usage
site indicate. This change is likely to break stuff.
> diff --git a/drivers/firmware/edd.c b/drivers/firmware/edd.c
> index 9e4f59d..ed6b7e3 100644
> --- a/drivers/firmware/edd.c
> +++ b/drivers/firmware/edd.c
> @@ -150,7 +150,7 @@ edd_show_host_bus(struct edd_device *edev, char *buf)
> if (!strncmp(info->params.host_bus_type, "ISA", 3)) {
> p += scnprintf(p, left, "\tbase_address: %x\n",
> info->params.interface_path.isa.base_address);
> - } else if (!strncmp(info->params.host_bus_type, "PCIX", 4) ||
> + } else if (!strncmp(info->params.host_bus_type, "PCI-X", 4) ||
blatantly and obviously wrong:
1) this is a BIOS-provided data structure. this patch just broke PCI-X
detection in edd.
2) the string length comparison is obviously wrong, even if problem #1
was not present
> diff --git a/drivers/hwmon/abituguru3.c b/drivers/hwmon/abituguru3.c
> index 3cf28af..9bbfb02 100644
> --- a/drivers/hwmon/abituguru3.c
> +++ b/drivers/hwmon/abituguru3.c
> @@ -172,7 +172,7 @@ static const struct abituguru3_motherboard_info abituguru3_motherboards[] = {
> { "DDR", 1, 0, 10, 1, 0 },
> { "DDR VTT", 2, 0, 10, 1, 0 },
> { "CPU VTT 1.2V", 3, 0, 10, 1, 0 },
> - { "MCH& PCIE 1.5V", 4, 0, 10, 1, 0 },
> + { "MCH& PCIe 1.5V", 4, 0, 10, 1, 0 },
> { "MCH 2.5V", 5, 0, 20, 1, 0 },
> { "ICH 1.05V", 6, 0, 10, 1, 0 },
> { "ATX +12V (24-Pin)", 7, 0, 60, 1, 0 },
> @@ -194,7 +194,7 @@ static const struct abituguru3_motherboard_info abituguru3_motherboards[] = {
> { "DDR", 1, 0, 10, 1, 0 },
> { "DDR VTT", 2, 0, 10, 1, 0 },
> { "CPU VTT 1.2V", 3, 0, 10, 1, 0 },
> - { "MCH& PCIE 1.5V", 4, 0, 10, 1, 0 },
> + { "MCH& PCIe 1.5V", 4, 0, 10, 1, 0 },
> { "MCH 2.5V", 5, 0, 20, 1, 0 },
> { "ICH 1.05V", 6, 0, 10, 1, 0 },
> { "ATX +12V (24-Pin)", 7, 0, 60, 1, 0 },
> @@ -223,7 +223,7 @@ static const struct abituguru3_motherboard_info abituguru3_motherboards[] = {
> { "DDR", 1, 0, 10, 1, 0 },
> { "DDR VTT", 2, 0, 10, 1, 0 },
> { "CPU VTT 1.2V", 3, 0, 10, 1, 0 },
> - { "MCH& PCIE 1.5V", 4, 0, 10, 1, 0 },
> + { "MCH& PCIe 1.5V", 4, 0, 10, 1, 0 },
> { "MCH 2.5V", 5, 0, 20, 1, 0 },
> { "ICH 1.05V", 6, 0, 10, 1, 0 },
> { "ATX +12V (24-Pin)", 7, 0, 60, 1, 0 },
> @@ -245,7 +245,7 @@ static const struct abituguru3_motherboard_info abituguru3_motherboards[] = {
> { "DDR", 1, 0, 10, 1, 0 },
> { "DDR VTT", 2, 0, 10, 1, 0 },
> { "CPU VTT 1.2V", 3, 0, 10, 1, 0 },
> - { "MCH& PCIE 1.5V", 4, 0, 10, 1, 0 },
> + { "MCH& PCIe 1.5V", 4, 0, 10, 1, 0 },
> { "MCH 2.5V", 5, 0, 20, 1, 0 },
> { "ICH 1.05V", 6, 0, 10, 1, 0 },
> { "ATX +12V (24-Pin)", 7, 0, 60, 1, 0 },
> @@ -291,7 +291,7 @@ static const struct abituguru3_motherboard_info abituguru3_motherboards[] = {
> { "NB 1.8V", 4, 0, 10, 1, 0 },
> { "NB 1.8V Dual", 5, 0, 10, 1, 0 },
> { "HTV 1.2", 3, 0, 10, 1, 0 },
> - { "PCIE 1.2V", 12, 0, 10, 1, 0 },
> + { "PCIe 1.2V", 12, 0, 10, 1, 0 },
> { "NB 1.2V", 13, 0, 10, 1, 0 },
> { "ATX +12V (24-Pin)", 7, 0, 60, 1, 0 },
> { "ATX +12V (4-pin)", 8, 0, 60, 1, 0 },
> @@ -337,7 +337,7 @@ static const struct abituguru3_motherboard_info abituguru3_motherboards[] = {
> { "DDR", 1, 0, 10, 1, 0 },
> { "DDR VTT", 2, 0, 10, 1, 0 },
> { "CPU VTT 1.2V", 3, 0, 10, 1, 0 },
> - { "MCH& PCIE 1.5V", 4, 0, 10, 1, 0 },
> + { "MCH& PCIe 1.5V", 4, 0, 10, 1, 0 },
> { "MCH 2.5V", 5, 0, 20, 1, 0 },
> { "ICH 1.05V", 6, 0, 10, 1, 0 },
> { "ATX +12V (24-Pin)", 7, 0, 60, 1, 0 },
> @@ -366,7 +366,7 @@ static const struct abituguru3_motherboard_info abituguru3_motherboards[] = {
> { "DDR", 1, 0, 10, 1, 0 },
> { "DDR VTT", 2, 0, 10, 1, 0 },
> { "CPU VTT 1.2V", 3, 0, 10, 1, 0 },
> - { "MCH& PCIE 1.5V", 4, 0, 10, 1, 0 },
> + { "MCH& PCIe 1.5V", 4, 0, 10, 1, 0 },
> { "MCH 2.5V", 5, 0, 20, 1, 0 },
> { "ICH 1.05V", 6, 0, 10, 1, 0 },
> { "ATX +12V (24-Pin)", 7, 0, 60, 1, 0 },
> @@ -411,7 +411,7 @@ static const struct abituguru3_motherboard_info abituguru3_motherboards[] = {
> { "DDR2", 1, 0, 20, 1, 0 },
> { "DDR2 VTT", 2, 0, 10, 1, 0 },
> { "CPU VTT 1.2V", 3, 0, 10, 1, 0 },
> - { "MCH& PCIE 1.5V", 4, 0, 10, 1, 0 },
> + { "MCH& PCIe 1.5V", 4, 0, 10, 1, 0 },
> { "MCH 2.5V", 5, 0, 20, 1, 0 },
> { "ICH 1.05V", 6, 0, 10, 1, 0 },
> { "ATX +12V (24-Pin)", 7, 0, 60, 1, 0 },
> @@ -443,7 +443,7 @@ static const struct abituguru3_motherboard_info abituguru3_motherboards[] = {
> { "NB 1.8V", 4, 0, 10, 1, 0 },
> { "NB 1.2V ", 13, 0, 10, 1, 0 },
> { "SB 1.2V", 5, 0, 10, 1, 0 },
> - { "PCIE 1.2V", 12, 0, 10, 1, 0 },
> + { "PCIe 1.2V", 12, 0, 10, 1, 0 },
> { "ATX +12V (24-Pin)", 7, 0, 60, 1, 0 },
> { "ATX +12V (4-pin)", 8, 0, 60, 1, 0 },
> { "ATX +5V", 9, 0, 30, 1, 0 },
bugs galore: these strings match DMI data.
> index fbf8c53..e20cdb8 100644
> --- a/drivers/infiniband/hw/ipath/ipath_iba6120.c
> +++ b/drivers/infiniband/hw/ipath/ipath_iba6120.c
> @@ -639,7 +639,7 @@ static int ipath_pe_boardname(struct ipath_devdata *dd, char *name,
> ipath_dev_err(dd,
> "Don't yet know about board with ID %u\n",
> boardrev);
> - snprintf(name, namelen, "Unknown_InfiniPath_PCIe_%u",
> + snprintf(name, namelen, "Unknown_InfiniPath_PCIE_%u",
> boardrev);
> break;
> }
ABI breakage: this is exported through sysfs.
I stopped reviewing here. I presume there are more bugs, and ton more
pointless churn in the latter 60% of the patch.
This patch introduces big diffs to several drivers for little or no
value AFAICT.
Jeff
next prev parent reply other threads:[~2009-11-28 12:43 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-11-28 11:54 [PATCH] change PCI nomenclature according to PCI-SIG Stefan Assmann
2009-11-28 12:43 ` Jeff Garzik [this message]
2009-11-29 9:09 ` Stefan Assmann
2009-11-29 12:54 ` Jeff Garzik
2009-11-29 15:12 ` Rafael J. Wysocki
2009-11-29 21:09 ` Krzysztof Halasa
2009-11-30 8:57 ` Stefan Assmann
2009-11-30 16:11 ` Jesse Barnes
2009-11-28 13:18 ` Krzysztof Halasa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4B111ADC.9020800@garzik.org \
--to=jeff@garzik.org \
--cc=ddutile@redhat.com \
--cc=jbarnes@virtuousgeek.org \
--cc=kaneshige.kenji@jp.fujitsu.com \
--cc=khc@pm.waw.pl \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=sassmann@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox