Change in coreboot[master]: drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing

List overview All Threads
Download

newer

older

Change in ...coreboot[master]:...

Change in coreboot[master]:...

Patrick Rudolph (Code Review)

29 Aug 2019 29 Aug '19

8 p.m.

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35152 )

Change subject: drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing ......................................................................

drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing

Advertise the register spacing used by the BMC as set by the Kconfig.

Needs test on FB Watson and OCP Weedge100s.

Change-Id: Ib926d30f6a0e78fbf613a6f71f765c5f51eee77d Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/drivers/ipmi/ipmi_kcs_ops.c 1 file changed, 28 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/35152/1

diff --git a/src/drivers/ipmi/ipmi_kcs_ops.c b/src/drivers/ipmi/ipmi_kcs_ops.c index 21102bb..21f927e 100644 --- a/src/drivers/ipmi/ipmi_kcs_ops.c +++ b/src/drivers/ipmi/ipmi_kcs_ops.c @@ -126,10 +126,21 @@ u32 apic_interrupt = 0; acpi_addr_t addr = { .space_id = ACPI_ADDRESS_SPACE_IO, - .access_size = ACPI_ACCESS_SIZE_BYTE_ACCESS, .addrl = dev->path.pnp.port, };

+ switch (CONFIG_IPMI_KCS_REGISTER_SPACING) { + case 4: + addr.access_size = ACPI_ACCESS_SIZE_DWORD_ACCESS; + break; + case 16: + default: /* fallthrough */ + printk(BIOS_ERR, "IPMI: Unsupported register spacing for SPMI\n"); + case 1: /* fallthrough */ + addr.access_size = ACPI_ACCESS_SIZE_BYTE_ACCESS; + break; + } + current = ALIGN_UP(current, 8); printk(BIOS_DEBUG, "ACPI: * SPMI at %lx\n", current); spmi = (struct acpi_spmi *)current; @@ -214,6 +225,8 @@ struct drivers_ipmi_config *conf = NULL; u8 nv_storage = 0xff; u8 i2c_address = 0; + u8 register_spacing; + int len = 0;

if (dev->chip_info) @@ -225,6 +238,20 @@ i2c_address = conf->bmc_i2c_address; }

+ switch (CONFIG_IPMI_KCS_REGISTER_SPACING) { + case 4: + register_spacing = 1 << 6; + break; + case 16: + register_spacing = 2 << 6; + break; + default: + printk(BIOS_ERR, "IPMI: Unsupported register spacing for SMBIOS\n"); + case 1: /* fallthrough */ + register_spacing = 0 << 6; + break; + } + // add IPMI Device Information len += smbios_write_type38( current, handle,

Attachments:

attachment.htm (text/html — 5.2 KB)

Show replies by date

Patrick Rudolph (Code Review)

29 Aug 29 Aug

10:21 p.m.

Hello David Hendricks, Philipp Deppenwiese, build bot (Jenkins), Andrey Petrov,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/35152

to look at the new patch set (#2).

Change subject: drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing ......................................................................

drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing

Advertise the register spacing used by the BMC as set by the Kconfig.

Needs test on OCP Monolake and OCP Weedge100s.

Change-Id: Ib926d30f6a0e78fbf613a6f71f765c5f51eee77d Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/drivers/ipmi/ipmi_kcs_ops.c 1 file changed, 30 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/35152/2

-- To view, visit https://review.coreboot.org/c/coreboot/+/35152 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib926d30f6a0e78fbf613a6f71f765c5f51eee77d Gerrit-Change-Number: 35152 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Andrey Petrov anpetrov@fb.com Gerrit-Reviewer: David Hendricks david.hendricks@gmail.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Name of user not set #1000432 Gerrit-MessageType: newpatchset

Andrey Petrov (Code Review)

31 Aug 31 Aug

1:42 a.m.

Andrey Petrov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35152 )

Change subject: drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing ......................................................................

Patch Set 2:

(4 comments)

https://review.coreboot.org/c/coreboot/+/35152/2/src/drivers/ipmi/ipmi_kcs_o... File src/drivers/ipmi/ipmi_kcs_ops.c:

https://review.coreboot.org/c/coreboot/+/35152/2/src/drivers/ipmi/ipmi_kcs_o... PS2, Line 134: addr.access_size = ACPI_ACCESS_SIZE_DWORD_ACCESS; should be addr.bit_offset, add access_size can be moved to initialization

https://review.coreboot.org/c/coreboot/+/35152/2/src/drivers/ipmi/ipmi_kcs_o... PS2, Line 198: acpigen_write_io16(dev->path.pnp.port, dev->path.pnp.port, 1, 2, 1); should be: acpigen_write_io16(dev->path.pnp.port, dev->path.pnp.port, 1,1, 1); acpigen_write_io16(dev->path.pnp.port + CONFIG_IPMI_KCS_REGISTER_SPACING, dev->path.pnp.port + CONFIG_IPMI_KCS_REGISTER_SPACING, 1, 1, 1);

https://review.coreboot.org/c/coreboot/+/35152/2/src/drivers/ipmi/ipmi_kcs_o... PS2, Line 253: register_spacing = 0 << 6; reg_spacing is not used

https://review.coreboot.org/c/coreboot/+/35152/2/src/drivers/ipmi/ipmi_kcs_o... PS2, Line 265: 0, add reg_spacing here

Patrick Rudolph (Code Review)

1:44 a.m.

Hello David Hendricks, Philipp Deppenwiese, build bot (Jenkins), Andrey Petrov,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/35152

to look at the new patch set (#3).

Change subject: drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing ......................................................................

drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing

Advertise the register spacing used by the BMC as set by the Kconfig.

Needs test on OCP Monolake and OCP Weedge100s.

Change-Id: Ib926d30f6a0e78fbf613a6f71f765c5f51eee77d Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/drivers/ipmi/ipmi_kcs_ops.c 1 file changed, 37 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/35152/3

-- To view, visit https://review.coreboot.org/c/coreboot/+/35152 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib926d30f6a0e78fbf613a6f71f765c5f51eee77d Gerrit-Change-Number: 35152 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Andrey Petrov anpetrov@fb.com Gerrit-Reviewer: David Hendricks david.hendricks@gmail.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Name of user not set #1000432 Gerrit-MessageType: newpatchset

Patrick Rudolph (Code Review)

1:52 a.m.

Hello David Hendricks, Philipp Deppenwiese, build bot (Jenkins), Andrey Petrov,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/35152

to look at the new patch set (#5).

Change subject: drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing ......................................................................

drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing

Advertise the register spacing used by the BMC as set by the Kconfig.

Needs test on OCP Monolake and OCP Weedge100s.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/35152/5

-- To view, visit https://review.coreboot.org/c/coreboot/+/35152 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib926d30f6a0e78fbf613a6f71f765c5f51eee77d Gerrit-Change-Number: 35152 Gerrit-PatchSet: 5 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Andrey Petrov anpetrov@fb.com Gerrit-Reviewer: David Hendricks david.hendricks@gmail.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Name of user not set #1000432 Gerrit-MessageType: newpatchset

Andrey Petrov (Code Review)

2:19 a.m.

Andrey Petrov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35152 )

Change subject: drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing ......................................................................

Patch Set 5: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/35152 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib926d30f6a0e78fbf613a6f71f765c5f51eee77d Gerrit-Change-Number: 35152 Gerrit-PatchSet: 5 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Andrey Petrov anpetrov@fb.com Gerrit-Reviewer: David Hendricks david.hendricks@gmail.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Name of user not set #1000432 Gerrit-Comment-Date: Sat, 31 Aug 2019 00:19:25 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Rudolph (Code Review)

2 Sep 2 Sep

6:50 a.m.

Patrick Rudolph has uploaded a new patch set (#6) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/35152 )

Change subject: drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing ......................................................................

drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing

Advertise the register spacing used by the BMC as set by the Kconfig.

Tested on OCP Monolake.

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/35152/6

-- To view, visit https://review.coreboot.org/c/coreboot/+/35152 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib926d30f6a0e78fbf613a6f71f765c5f51eee77d Gerrit-Change-Number: 35152 Gerrit-PatchSet: 6 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Andrey Petrov anpetrov@fb.com Gerrit-Reviewer: David Hendricks david.hendricks@gmail.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Name of user not set #1000432 Gerrit-MessageType: newpatchset

Philipp Deppenwiese (Code Review)

7:28 a.m.

Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35152 )

Change subject: drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing ......................................................................

Patch Set 7: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/35152 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib926d30f6a0e78fbf613a6f71f765c5f51eee77d Gerrit-Change-Number: 35152 Gerrit-PatchSet: 7 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Andrey Petrov anpetrov@fb.com Gerrit-Reviewer: David Hendricks david.hendricks@gmail.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Name of user not set #1000432 Gerrit-Comment-Date: Mon, 02 Sep 2019 05:28:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Rudolph (Code Review)

7:29 a.m.

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35152 )

Change subject: drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing ......................................................................

Patch Set 7:

(4 comments)

https://review.coreboot.org/c/coreboot/+/35152/2/src/drivers/ipmi/ipmi_kcs_o... File src/drivers/ipmi/ipmi_kcs_ops.c:

https://review.coreboot.org/c/coreboot/+/35152/2/src/drivers/ipmi/ipmi_kcs_o... PS2, Line 134: addr.access_size = ACPI_ACCESS_SIZE_DWORD_ACCESS;

...

should be addr. […]

Done

https://review.coreboot.org/c/coreboot/+/35152/2/src/drivers/ipmi/ipmi_kcs_o... PS2, Line 198: acpigen_write_io16(dev->path.pnp.port, dev->path.pnp.port, 1, 2, 1);

...

should be: […]

Done

https://review.coreboot.org/c/coreboot/+/35152/2/src/drivers/ipmi/ipmi_kcs_o... PS2, Line 253: register_spacing = 0 << 6;

...

reg_spacing is not used

Done

https://review.coreboot.org/c/coreboot/+/35152/2/src/drivers/ipmi/ipmi_kcs_o... PS2, Line 265: 0,

...

add reg_spacing here

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/35152 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib926d30f6a0e78fbf613a6f71f765c5f51eee77d Gerrit-Change-Number: 35152 Gerrit-PatchSet: 7 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Andrey Petrov anpetrov@fb.com Gerrit-Reviewer: David Hendricks david.hendricks@gmail.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Name of user not set #1000432 Gerrit-Comment-Date: Mon, 02 Sep 2019 05:29:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Andrey Petrov anpetrov@fb.com Gerrit-MessageType: comment

Patrick Rudolph (Code Review)

7:30 a.m.

Patrick Rudolph has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/35152 )

Change subject: drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing ......................................................................

drivers/ipmi/ipmi_kcs_ops: Advertise correct register spacing

Advertise the register spacing used by the BMC as set by the Kconfig.

Tested on OCP Monolake.

Change-Id: Ib926d30f6a0e78fbf613a6f71f765c5f51eee77d Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35152 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Philipp Deppenwiese zaolin.daisuki@gmail.com --- M src/drivers/ipmi/ipmi_kcs_ops.c 1 file changed, 37 insertions(+), 2 deletions(-)

Approvals: build bot (Jenkins): Verified Philipp Deppenwiese: Looks good to me, approved

diff --git a/src/drivers/ipmi/ipmi_kcs_ops.c b/src/drivers/ipmi/ipmi_kcs_ops.c index 21102bb..baa72a8 100644 --- a/src/drivers/ipmi/ipmi_kcs_ops.c +++ b/src/drivers/ipmi/ipmi_kcs_ops.c @@ -128,8 +128,24 @@ .space_id = ACPI_ADDRESS_SPACE_IO, .access_size = ACPI_ACCESS_SIZE_BYTE_ACCESS, .addrl = dev->path.pnp.port, + .bit_width = 8, };

+ switch (CONFIG_IPMI_KCS_REGISTER_SPACING) { + case 4: + addr.bit_offset = 32; + break; + case 16: + addr.bit_offset = 128; + break; + default: + printk(BIOS_ERR, "IPMI: Unsupported register spacing for SPMI\n"); + /* fall through */ + case 1: + addr.bit_offset = 8; + break; + } + current = ALIGN_UP(current, 8); printk(BIOS_DEBUG, "ACPI: * SPMI at %lx\n", current); spmi = (struct acpi_spmi *)current; @@ -183,7 +199,9 @@ acpigen_write_STA(0xf); acpigen_write_name("_CRS"); acpigen_write_resourcetemplate_header(); - acpigen_write_io16(dev->path.pnp.port, dev->path.pnp.port, 1, 2, 1); + acpigen_write_io16(dev->path.pnp.port, dev->path.pnp.port, 1, 1, 1); + acpigen_write_io16(dev->path.pnp.port + CONFIG_IPMI_KCS_REGISTER_SPACING, + dev->path.pnp.port + CONFIG_IPMI_KCS_REGISTER_SPACING, 1, 1, 1);

if (conf) { // FIXME: is that correct? @@ -214,6 +232,8 @@ struct drivers_ipmi_config *conf = NULL; u8 nv_storage = 0xff; u8 i2c_address = 0; + u8 register_spacing; + int len = 0;

if (dev->chip_info) @@ -225,6 +245,21 @@ i2c_address = conf->bmc_i2c_address; }

+ switch (CONFIG_IPMI_KCS_REGISTER_SPACING) { + case 4: + register_spacing = 1 << 6; + break; + case 16: + register_spacing = 2 << 6; + break; + default: + printk(BIOS_ERR, "IPMI: Unsupported register spacing for SMBIOS\n"); + /* fall through */ + case 1: + register_spacing = 0 << 6; + break; + } + // add IPMI Device Information len += smbios_write_type38( current, handle, @@ -233,7 +268,7 @@ i2c_address, // I2C address nv_storage, // NV storage dev->path.pnp.port | 1, // IO interface - 0, + register_spacing, 0); // no IRQ

return len;

-- To view, visit https://review.coreboot.org/c/coreboot/+/35152 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib926d30f6a0e78fbf613a6f71f765c5f51eee77d Gerrit-Change-Number: 35152 Gerrit-PatchSet: 8 Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Andrey Petrov anpetrov@fb.com Gerrit-Reviewer: David Hendricks david.hendricks@gmail.com Gerrit-Reviewer: Patrick Rudolph patrick.rudolph@9elements.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Name of user not set #1000432 Gerrit-MessageType: merged

2068

days inactive

2072

days old

coreboot-gerrit@coreboot.org

9 comments

3 participants

tags (0)

participants (3)

Andrey Petrov (Code Review)
Patrick Rudolph (Code Review)
Philipp Deppenwiese (Code Review)